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

[Jan Beulich <jbeulich@suse.com>]

On 29.10.2025 17:06, Andrew Cooper wrote:
> On 28/10/2025 9:20 am, Jan Beulich wrote:
>> On 27.10.2025 20:38, Andrew Cooper wrote:
>>> On 27/10/2025 5:26 pm, Teddy Astie wrote:
>>>> I'm not a fan of doing a inline cpuid check here, but I don't have a
>>>> better approach in mind.
>>> I'm not sure if there's enough information in leaf 6 to justify putting
>>> it fully into the CPUID infrastructure.
>>>
>>> But, if you do something like this:
>>>
>>> diff --git a/xen/include/xen/lib/x86/cpu-policy.h b/xen/include/xen/lib/x86/cpu-policy.h
>>> index f94f23e159d2..d02fe4d22151 100644
>>> --- a/xen/include/xen/lib/x86/cpu-policy.h
>>> +++ b/xen/include/xen/lib/x86/cpu-policy.h
>>> @@ -121,7 +121,13 @@ struct cpu_policy
>>>              uint64_t :64, :64; /* Leaf 0x3 - PSN. */
>>>              uint64_t :64, :64; /* Leaf 0x4 - Structured Cache. */
>>>              uint64_t :64, :64; /* Leaf 0x5 - MONITOR. */
>>> -            uint64_t :64, :64; /* Leaf 0x6 - Therm/Perf. */
>>> +
>>> +            /* Leaf 0x6 - Thermal and Perf. */
>>> +            struct {
>>> +                bool /* a */ dts:1;
>>> +                uint32_t /* b */:32, /* c */:32, /* d */:32;
>>> +            };
>>> +
>>>              uint64_t :64, :64; /* Leaf 0x7 - Structured Features. */
>>>              uint64_t :64, :64; /* Leaf 0x8 - rsvd */
>>>              uint64_t :64, :64; /* Leaf 0x9 - DCA */
>> Just to mention, below a patch I have pending as part of a series to
>> e.g. replace the various CPUID6_* values we presently use. As you did
>> indicate when we talked about this, a prereq to then use respective
>> bits from host_policy is an adjustment to cpu-policy.c, which is also
>> part of that series. If we weren't in freeze right now, I would have
>> posted the series already.
>>
>> Jan
>>
>> x86/cpu-policy: define bits of leaf 6
>>
>> ... as far as we presently use them in the codebase.
>>
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Or should we make both parts proper featureset elements? At least
>> APERFMPERF could likely be made visible to guests (in principle).
>>
>> --- a/xen/include/xen/lib/x86/cpu-policy.h
>> +++ b/xen/include/xen/lib/x86/cpu-policy.h
>> @@ -128,7 +128,31 @@ struct cpu_policy
>>              uint64_t :64, :64; /* Leaf 0x3 - PSN. */
>>              uint64_t :64, :64; /* Leaf 0x4 - Structured Cache. */
>>              uint64_t :64, :64; /* Leaf 0x5 - MONITOR. */
>> -            uint64_t :64, :64; /* Leaf 0x6 - Therm/Perf. */
>> +
>> +            /* Leaf 0x6 - Therm/Perf. */
>> +            struct {
>> +                uint32_t /* a */:1,
>> +                    turbo:1,
>> +                    arat:1,
>> +                    :4,
>> +                    hwp:1,
>> +                    hwp_notification:1,
>> +                    hwp_activity_window:1,
>> +                    hwp_epp:1,
>> +                    hwp_plr:1,
>> +                    :1,
>> +                    hdc:1,
>> +                    :2,
>> +                    hwp_peci:1,
>> +                    :2,
>> +                    hw_feedback:1,
>> +                    :12;
>> +                uint32_t /* b */:32;
>> +                uint32_t /* c */ aperfmperf:1,
>> +                    :31;
>> +                uint32_t /* d */:32;
>> +            } pm;
> 
> This works too, although we don't have 'pm' equivalents elsewhere in
> this part of the union.
> 
> APERF/MPERF is a disaster of an interface.  It can't safely be read even
> in root mode, because an NMI/SMI breaks the algorithm in a way that
> isn't easy to spot and retry.  On AMD, it's marginally better because
> GIF can be used to exclude NMIs and non-fatal MCEs while sampling the
> register pair.
> 
> In a VM, it's simply unusable.  Any VMExit, and even a vCPU reschedule,
> breaks reading the pair.
> 
> Until the CPU vendors produce a way of reading the two counters together
> (i.e. atomically, which has been asked for, repeatedly), there's no
> point considering it for use in a VM.

Right now none of this is meant for exposure. It's solely for us to use
the bits in the host policy.

Jan