On 24/11/2025 12:25 pm, Jan Beulich wrote: > There's no need to invoke CPUID yet another time. This way two of the > static booleans can also go away. > > Signed-off-by: Jan Beulich <[email protected]> > --- > v2: Introduce cpu_has_*. > > --- a/xen/arch/x86/acpi/cpufreq/hwp.c > +++ b/xen/arch/x86/acpi/cpufreq/hwp.c > @@ -18,9 +18,6 @@ > > static bool __ro_after_init hwp_in_use; > > -static bool __ro_after_init feature_hwp_notification; > -static bool __ro_after_init feature_hwp_activity_window; > - > static bool __read_mostly feature_hdc; > > static bool __ro_after_init opt_cpufreq_hdc = true; > @@ -165,8 +162,6 @@ bool hwp_active(void) > > static bool __init hwp_available(void) > { > - unsigned int eax; > - > if ( boot_cpu_data.cpuid_level < CPUID_PM_LEAF ) > { > hwp_verbose("cpuid_level (%#x) lacks HWP support\n", > @@ -183,29 +178,22 @@ static bool __init hwp_available(void) > return false; > } > > - eax = cpuid_eax(CPUID_PM_LEAF); > - > hwp_verbose("%d notify: %d act-window: %d energy-perf: %d pkg-level: %d > peci: %d\n", > - !!(eax & CPUID6_EAX_HWP), > - !!(eax & CPUID6_EAX_HWP_NOTIFICATION), > - !!(eax & CPUID6_EAX_HWP_ACTIVITY_WINDOW), > - !!(eax & CPUID6_EAX_HWP_ENERGY_PERFORMANCE_PREFERENCE), > - !!(eax & CPUID6_EAX_HWP_PACKAGE_LEVEL_REQUEST), > - !!(eax & CPUID6_EAX_HWP_PECI)); > + cpu_has_hwp, cpu_has_hwp_notification, > + cpu_has_hwp_activity_window, cpu_has_hwp_epp, > + cpu_has_hwp_plr, cpu_has_hwp_peci); > > - if ( !(eax & CPUID6_EAX_HWP) ) > + if ( !cpu_has_hwp ) > return false; > > - if ( !(eax & CPUID6_EAX_HWP_ENERGY_PERFORMANCE_PREFERENCE) ) > + if ( !cpu_has_hwp_epp ) > { > hwp_verbose("disabled: No energy/performance preference available"); > > return false; > } > > - feature_hwp_notification = eax & CPUID6_EAX_HWP_NOTIFICATION; > - feature_hwp_activity_window = eax & CPUID6_EAX_HWP_ACTIVITY_WINDOW; > - feature_hdc = eax & CPUID6_EAX_HDC; > + feature_hdc = cpu_has_hdc; > > hwp_verbose("Hardware Duty Cycling (HDC) %ssupported%s\n", > feature_hdc ? "" : "not ", > @@ -213,7 +201,7 @@ static bool __init hwp_available(void) > : ""); > > hwp_verbose("HW_FEEDBACK %ssupported\n", > - (eax & CPUID6_EAX_HW_FEEDBACK) ? "" : "not "); > + cpu_has_hw_feedback ? "" : "not "); > > hwp_in_use = true; > > @@ -226,7 +214,8 @@ static int cf_check hwp_cpufreq_verify(s > { > struct hwp_drv_data *data = per_cpu(hwp_drv_data, policy->cpu); > > - if ( !feature_hwp_activity_window && data->activity_window ) > + if ( !cpu_has_hwp_activity_window && > + data->activity_window ) > { > hwp_verbose("HWP activity window not supported\n"); > > @@ -268,7 +257,7 @@ static int cf_check hwp_cpufreq_target(s > hwp_req.max_perf = data->maximum; > hwp_req.desired = data->desired; > hwp_req.energy_perf = data->energy_perf; > - if ( feature_hwp_activity_window ) > + if ( cpu_has_hwp_activity_window ) > hwp_req.activity_window = data->activity_window; > > if ( hwp_req.raw == data->curr_req.raw ) > @@ -365,7 +354,7 @@ static void cf_check hwp_init_msrs(void > } > > /* Ensure we don't generate interrupts */ > - if ( feature_hwp_notification ) > + if ( cpu_has_hwp_notification ) > wrmsr_safe(MSR_HWP_INTERRUPT, 0); > > if ( !(val & PM_ENABLE_HWP_ENABLE) ) > @@ -537,7 +526,8 @@ int get_hwp_para(unsigned int cpu, > return -ENODATA; > > cppc_para->features = > - (feature_hwp_activity_window ? XEN_SYSCTL_CPPC_FEAT_ACT_WINDOW : 0); > + (cpu_has_hwp_activity_window > + ? XEN_SYSCTL_CPPC_FEAT_ACT_WINDOW : 0); > cppc_para->lowest = data->hw.lowest; > cppc_para->lowest_nonlinear = data->hw.most_efficient; > cppc_para->nominal = data->hw.guaranteed; > @@ -585,7 +575,7 @@ int set_hwp_para(struct cpufreq_policy * > > /* Clear out activity window if lacking HW supported. */ > if ( (set_cppc->set_params & XEN_SYSCTL_CPPC_SET_ACT_WINDOW) && > - !feature_hwp_activity_window ) > + !cpu_has_hwp_activity_window ) > { > set_cppc->set_params &= ~XEN_SYSCTL_CPPC_SET_ACT_WINDOW; > cleared_act_window = true; > --- 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. ~Andrew
