On 16.02.2026 23:22, Teddy Astie wrote:
> Le 16/02/2026 à 17:15, Jan Beulich a écrit :
>> On 16.02.2026 16:09, Teddy Astie wrote:
>>> --- 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 ?

The one the SDM mandates CPUID.01H:EDX[22], or - as said for the other one -
a justification for using something else would need providing in a comment.

>>>       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).

How does coretemp matter here? It using a safe rdmsr means it's not Xen-
enabled, i.e. doesn't use the platform-op that the code here is about.

> That may be worth adding a comment eventually.

Yes. However, here and above I wonder whether we'd be doing any good in
using a tighter check than mandated by the SDM. Another reader of the
MSR may appear (in xenpm or elsewhere), and it may be after another field.
Things may then be observed to work fine, until someone tries it on older
hardware.

>>> --- 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.

See version 090 volume 1 section 21.3.

Jan

Reply via email to