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

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

Le 25/03/2026 à 12:43, Jan Beulich a écrit :
> 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 {}.
>

Is it regarding having union and _aa, _ab, (...) fields or prefixing
fields with pmu_ ?

>>   > 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?
>
Okay, I think I finally figured out field sizes (5-bit field confuses me).

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, :2;
     bool             anythread_depreciation:1;
     uint32_t         slots_per_cyc:4, :12;
} pmu;


> Jan
>

Teddy


--
Teddy Astie | Vates XCP-ng Developer

XCP-ng & Xen Orchestra - Vates solutions

web: https://vates.tech