Le 16/02/2026 à 17:15, Jan Beulich a écrit :
> On 16.02.2026 16:09, Teddy Astie wrote:
>> Package-related MSR is actually gated behind "PTM" CPUID flag rather than
>> "DTS" one. Make sure we check the right CPUID for package-related MSR.
>>
>> Check for either DTS or PTM for MSR_TEMPERATURE_TARGET.
>>
>> The only visible difference in practice would be that EPERM would now
>> be reported instead of EFAULT if we tried accessing the package MSR on
>> a platform that doesn't have it.
>>
>> Amends: 615c9f3f820 ("x86/platform: Expose DTS sensors MSR")
>
> Imo this really addresses a bug, so wants to be Fixes:.
>I wasn't so sure between Fixes and Amends; but Fixes is ok for me. >> --- a/xen/arch/x86/platform_hypercall.c >> +++ b/xen/arch/x86/platform_hypercall.c >> @@ -89,9 +89,12 @@ static bool msr_read_allowed(unsigned int msr) >> return cpu_has_srbds_ctrl; >> >> case MSR_IA32_THERM_STATUS: >> + return host_cpu_policy.basic.digital_temp_sensor; > > As per the SDM this doesn't look right either - it's CPUID.01H:EDX[22] > (acpi) instead. It is the field you're after in xenpm which is tied to > CPUID.06H:EAX[0] (digital_temp_sensor). > I'm not sure to follow exactly what you mean here. Which CPUID should we check ? >> case MSR_TEMPERATURE_TARGET: >> + return host_cpu_policy.basic.digital_temp_sensor || >> + host_cpu_policy.basic.package_therm_management; > > Where in the SDM did you find this connection? (Anything made up wants > commenting upon.) > To me, we are interested in the MSR_TEMPERATURE_TARGET only with dts or ptm, and the only used probing method in practice is performing a safe rdmsr (at least in Linux coretemp). That may be worth adding a comment eventually. >> case MSR_PACKAGE_THERM_STATUS: >> - return host_cpu_policy.basic.digital_temp_sensor; >> + return host_cpu_policy.basic.package_therm_management; >> } > > And of course with this splitting of cases, blank lines want inserting > between the case blocks. > >> --- a/xen/include/xen/lib/x86/cpu-policy.h >> +++ b/xen/include/xen/lib/x86/cpu-policy.h >> @@ -132,7 +132,7 @@ struct cpu_policy >> :1, >> :1, >> :1, >> - :1, >> + package_therm_management:1, > > The SDM calls this PKG_THERM_MGMT; I think our naming would better match now > that we decided to have everything else here named according to the SDM. > I can't find PKG_THERM_MGMT in my SDM version; but overall I don't have a strong opinion on naming and am fine with pkg_therm_mgmt. > Jan > Teddy -- Teddy Astie | Vates XCP-ng Developer XCP-ng & Xen Orchestra - Vates solutions web: https://vates.tech
