Re: [RFC PATCH for-4.22 v2 2/3] x86/platform: Expose DTS sensors MSR

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

Le 30/10/2025 à 14:54, Jan Beulich a écrit :
> On 29.10.2025 16:59, Teddy Astie wrote:
>> --- a/xen/arch/x86/include/asm/msr-index.h
>> +++ b/xen/arch/x86/include/asm/msr-index.h
>> @@ -115,6 +115,9 @@
>>   #define  MCU_OPT_CTRL_GDS_MIT_DIS           (_AC(1, ULL) <<  4)
>>   #define  MCU_OPT_CTRL_GDS_MIT_LOCK          (_AC(1, ULL) <<  5)
>>
>> +#define MSR_DTS_TEMPERATURE_TARGET          0x000001a2
>> +#define MSR_DTS_PACKAGE_THERM_STATUS        0x000001b1
>
> Where are the DTS infixes coming from? The SDM doesn't have such. We try to
> stay as close as reasonable to the SDM / PM names, with the exception that
> typically we omit IA32 infixes.
>

I got confused with the naming of MSRs (and their lack of IA32 prefix)
in the upper list of MSR. I guess it should be MSR_PACKAGE_THERM_STATUS
and MSR_TEMPERATURE_TARGET.

> I'd also like to note that unlike the two THERM_STATUS, MSR_TEMPERATURE_TARGET
> (as per the absence if an IA32 infix in the SDM) isn't an architectural MSR,
> and hence I'm not entirely convinced we can "blindly" expose it. 
> (Interestingly
> in Linux code there is an IA32 infix.)
>

We only perform rdmsr_safe on this MSR, so I don't think there is much
problem with it as I don't expect Intel to reuse this MSR number for
something else (at worst, it is going to not be implemented and would
gracefully fail).

Some parts of this MSR slightly change, but the one (tjmax) that is
interesting in here is consistent across the architectures.

>> @@ -86,6 +87,11 @@ static bool msr_read_allowed(unsigned int msr)
>>
>>       case MSR_MCU_OPT_CTRL:
>>           return cpu_has_srbds_ctrl;
>> +
>> +    case MSR_IA32_THERM_STATUS:
>> +    case MSR_DTS_TEMPERATURE_TARGET:
>> +    case MSR_DTS_PACKAGE_THERM_STATUS:
>> +        return raw_cpu_policy.basic.pm.dts;
>
> This, aiui (and according to related comments I got from Andrew on remotely
> similar changes I was doing) wants to use host_policy. Hence why that other
> prereq change is needed that I talked about (and that iirc I reproduced on
> the other sub-thread).
>

yes

> Jan



--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech