Re: [PATCH 3/6] x86: Define some Intel vPMU leafs

[Jan Beulich <jbeulich@xxxxxxxx>]

On 25.03.2026 10:48, Teddy Astie wrote:
> Le 24/03/2026 à 10:25, Jan Beulich a écrit :
>> On 10.03.2026 17:44, Teddy Astie wrote:
>>> --- a/xen/include/xen/lib/x86/cpu-policy.h
>>> +++ b/xen/include/xen/lib/x86/cpu-policy.h
>>> @@ -162,7 +162,15 @@ struct cpu_policy
>>>               uint64_t :64, :64; /* Leaf 0x9 - DCA */
>>>   
>>>               /* Leaf 0xa - Intel PMU. */
>>> -            uint8_t pmu_version, _pmu[15];
>>> +            struct {
>>> +                uint8_t /* a */ version, num_gp_ctrs, gp_ctr_width,
>>> +                                event_enum_length;
>>> +                uint32_t /* b */:32;
>>> +                uint32_t /* c */ fixed_ctr_mask;
>>> +                uint32_t /* d */ num_fixed_ctr:5, fixed_ctr_width:8, :1,
>>> +                                 anythread_depreciation:1, slots_per_cyc:4,
>>> +                                 :13;
>>> +            } pmu;
>>
>> Style-wise this looks to follow e.g. the cache leaf, so perhaps okay, even
>> if I would have preferred you to follow what we did for leaf 6. 
> 
> My idea was to put all that as .pmu.*, so I wouldn't need to prefix 
> everything with "pmu_". I'm not sure if you're talking about a different 
> approach.

The "pmu" is fine. I'm talking of what's inside the struct {}.

>  > The named> boolean field, however, wants to be of type bool.
> 
> Which fields ?

There's only one named 1-bit field: anythread_depreciation.

>  > And then the unnamed 1-bit> field really wants to be 2 bits, for 
> anythread_depreciation to be bit 15
>> (etc).
>>
> 
> Ah yes thanks, I got confused with the fields size for a second.
> I also found that slots_per_cyc is 3 bits instead of 4.

Not as far as I can see.

> I think this diff fixes it overall.
> 
> --- a/xen/include/xen/lib/x86/cpu-policy.h
> +++ b/xen/include/xen/lib/x86/cpu-policy.h
> @@ -167,9 +167,9 @@ struct cpu_policy
>                                   event_enum_length;
>                   uint32_t /* b */:32;
>                   uint32_t /* c */ fixed_ctr_mask;
> -                uint32_t /* d */ num_fixed_ctr:5, fixed_ctr_width:8, :1,
> -                                 anythread_depreciation:1, slots_per_cyc:4,
> -                                 :13;
> +                uint32_t /* d */ num_fixed_ctr:5, fixed_ctr_width:8, :2,
> +                                 anythread_depreciation:1, slots_per_cyc:3,
> +                                 :11;

Why 11 all of the sudden?

Jan