On 18.11.2025 16:30, Andrew Cooper wrote:
> On 18/11/2025 3:06 pm, Jan Beulich wrote:
>> --- a/xen/include/xen/lib/x86/cpu-policy.h
>> +++ b/xen/include/xen/lib/x86/cpu-policy.h
>> @@ -121,7 +121,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;
>
> Elsewhere, single bit fields are bool foo:1, and these want to match for
> consistency.
Oh, yes, will change.
> In particular using uint32_t:1 creates a latent bug in
> patch 8.
I don't see where that would be.
> One problem with bool bitfields is that your :4 needs to become 4x :1.
> Right now his hidden in the macros that gen-cpuid.py makes.
>
> Given that b is of type uint32_t, you can omit the :12 from the end of a
> and leave a comment. Similarly, the trailing :31 on c can be dropped.
We have these in many other places, and omitting in particular the :31
would also feel somewhat fragile / misleading. It'll need to be
bool /* c */ aperfmperf:1;
uint32_t :31;
or something along these lines.
>> + } pm;
>
> Nothing else is sub-scoped. I'd prefer that you drop the 'pm'.
Wouldn't that require the use of the very extension you just talked about
at the committer's call?
Jan