Re: [PATCH] x86/platform: Consider PTM for exposing package-related MSR

["Teddy Astie" <teddy.astie@xxxxxxxxxx>]

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