On 10.12.2025 15:11, Andrew Cooper wrote:
>> --- a/xen/arch/x86/include/asm/cpufeature.h
>> +++ b/xen/arch/x86/include/asm/cpufeature.h
>> @@ -115,14 +115,6 @@ static inline bool boot_cpu_has(unsigned
>>  }
>>  
>>  #define CPUID_PM_LEAF                                6
>> -#define CPUID6_EAX_HWP                               BIT(7, U)
>> -#define CPUID6_EAX_HWP_NOTIFICATION                  BIT(8, U)
>> -#define CPUID6_EAX_HWP_ACTIVITY_WINDOW               BIT(9, U)
>> -#define CPUID6_EAX_HWP_ENERGY_PERFORMANCE_PREFERENCE BIT(10, U)
>> -#define CPUID6_EAX_HWP_PACKAGE_LEVEL_REQUEST         BIT(11, U)
>> -#define CPUID6_EAX_HDC                               BIT(13, U)
>> -#define CPUID6_EAX_HWP_PECI                          BIT(16, U)
>> -#define CPUID6_EAX_HW_FEEDBACK                       BIT(19, U)
>>  
>>  /* CPUID level 0x00000001.edx */
>>  #define cpu_has_fpu             1
>> @@ -179,6 +171,14 @@ static inline bool boot_cpu_has(unsigned
>>  /* CPUID level 0x00000006.eax */
>>  #define cpu_has_turbo           host_cpu_policy.basic.turbo
>>  #define cpu_has_arat            host_cpu_policy.basic.arat
>> +#define cpu_has_hwp             host_cpu_policy.basic.hwp
>> +#define cpu_has_hwp_notification host_cpu_policy.basic.hwp_notification
>> +#define cpu_has_hwp_activity_window 
>> host_cpu_policy.basic.hwp_activity_window
>> +#define cpu_has_hwp_epp        host_cpu_policy.basic.hwp_epp
>> +#define cpu_has_hwp_plr        host_cpu_policy.basic.hwp_plr
>> +#define cpu_has_hdc            host_cpu_policy.basic.hdc
>> +#define cpu_has_hwp_peci       host_cpu_policy.basic.hwp_peci
>> +#define cpu_has_hw_feedback    host_cpu_policy.basic.hw_feedback
> 
> The indentation of these final 5 is one-too-few spaces.
> 
> I can't help but feel that notification could be shortened to notify. 
> Except upon looking in the SDM, it's named HWP_INTERRUPT because it
> enumerates MSR_HWP_INTERRUPT.
> 
> Similarly, HWP_PLR is really HWP_REQUEST_PKG because it enumerates
> MSR_HWP_REQUEST_PKG.
> 
> ACTIVITY_WINDOW and EPP are wonky because they're out of order WRT
> PLR/REQUEST_PKG.  It clearly means they all came in together, but have
> SKU controls.
> 
> But I digress.  ACTIVITY_WINDOW can probably be shortened to just
> WINDOW, and that fixes the two egregiously long ones.

To be honest, I see only one of two options: Either we stick to what we had
settled on when the HWP driver went in (switching to acronyms where helpful),
or we strictly follow the SDM. In the latter case I think I will need to
split this patch, for every of the renames to be separate (to be easier to
verify). And the naming decisions then want applying in patch 1 as well.

Unless I hear otherwise (soon), I'll go the "strictly per SDM" route. Still,
the union-or-not question in patch 1 also needs sorting, so I'm dependent
anyway upon you replying in a somewhat timely manner.

Jan

Reply via email to