| Message ID | 0f6e8a602fb3106d6b064582ca50d3d5fd4f1174.1719832871.git.Sergiy_Kibrik@epam.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86: separate powernow/hwp/acpi cpufreq code | expand |
On 01.07.2024 14:19, Sergiy Kibrik wrote: > --- a/xen/drivers/acpi/pmstat.c > +++ b/xen/drivers/acpi/pmstat.c > @@ -255,7 +255,7 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op) > strlcpy(op->u.get_para.scaling_driver, "Unknown", CPUFREQ_NAME_LEN); > > if ( !strncmp(op->u.get_para.scaling_driver, XEN_HWP_DRIVER_NAME, > - CPUFREQ_NAME_LEN) ) > + CPUFREQ_NAME_LEN) && IS_ENABLED(CONFIG_INTEL) ) Wrapping like this is confusing, not just because of the flawed indentation. Please can this be if ( !strncmp(op->u.get_para.scaling_driver, XEN_HWP_DRIVER_NAME, CPUFREQ_NAME_LEN) && IS_ENABLED(CONFIG_INTEL) ) ? Perhaps the IS_ENABLED() would also better be first (not just here). > --- a/xen/drivers/cpufreq/utility.c > +++ b/xen/drivers/cpufreq/utility.c > @@ -379,7 +379,7 @@ int cpufreq_driver_getavg(unsigned int cpu, unsigned int flag) > if (!cpu_online(cpu) || !(policy = per_cpu(cpufreq_cpu_policy, cpu))) > return 0; > > - freq_avg = get_measured_perf(cpu, flag); > + freq_avg = IS_ENABLED(CONFIG_INTEL) ? get_measured_perf(cpu, flag) : 0; > if ( freq_avg > 0 ) > return freq_avg; Why is this? APERF/MPERF aren't Intel-only MSRs. > --- a/xen/include/acpi/cpufreq/cpufreq.h > +++ b/xen/include/acpi/cpufreq/cpufreq.h > @@ -254,11 +254,20 @@ void intel_feature_detect(struct cpufreq_policy *policy); > > int hwp_cmdline_parse(const char *s, const char *e); > int hwp_register_driver(void); > +#ifdef CONFIG_INTEL > bool hwp_active(void); > +#else > +static inline bool hwp_active(void) > +{ > + return false; > +} > +#endif > + > int get_hwp_para(unsigned int cpu, > struct xen_cppc_para *cppc_para); > int set_hwp_para(struct cpufreq_policy *policy, > struct xen_set_cppc_para *set_cppc); > > int acpi_register_driver(void); > + > #endif /* __XEN_CPUFREQ_PM_H__ */ Nit: This adding of a blank line should be part of the earlier patch. Jan
03.07.24 18:14, Jan Beulich: > On 01.07.2024 14:19, Sergiy Kibrik wrote: >> --- a/xen/drivers/acpi/pmstat.c >> +++ b/xen/drivers/acpi/pmstat.c >> @@ -255,7 +255,7 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op) >> strlcpy(op->u.get_para.scaling_driver, "Unknown", CPUFREQ_NAME_LEN); >> >> if ( !strncmp(op->u.get_para.scaling_driver, XEN_HWP_DRIVER_NAME, >> - CPUFREQ_NAME_LEN) ) >> + CPUFREQ_NAME_LEN) && IS_ENABLED(CONFIG_INTEL) ) > Wrapping like this is confusing, not just because of the flawed indentation. > Please can this be > > if ( !strncmp(op->u.get_para.scaling_driver, XEN_HWP_DRIVER_NAME, > CPUFREQ_NAME_LEN) && > IS_ENABLED(CONFIG_INTEL) ) > > ? Perhaps the IS_ENABLED() would also better be first (not just here). sure, will fix that > >> --- a/xen/drivers/cpufreq/utility.c >> +++ b/xen/drivers/cpufreq/utility.c >> @@ -379,7 +379,7 @@ int cpufreq_driver_getavg(unsigned int cpu, unsigned int flag) >> if (!cpu_online(cpu) || !(policy = per_cpu(cpufreq_cpu_policy, cpu))) >> return 0; >> >> - freq_avg = get_measured_perf(cpu, flag); >> + freq_avg = IS_ENABLED(CONFIG_INTEL) ? get_measured_perf(cpu, flag) : 0; >> if ( freq_avg > 0 ) >> return freq_avg; > Why is this? APERF/MPERF aren't Intel-only MSRs. yes, it seems to be a mistake.. -Sergiy
diff --git a/xen/arch/x86/acpi/cpufreq/Makefile b/xen/arch/x86/acpi/cpufreq/Makefile index 44d4c0b497..e7dbe434a8 100644 --- a/xen/arch/x86/acpi/cpufreq/Makefile +++ b/xen/arch/x86/acpi/cpufreq/Makefile @@ -1,4 +1,4 @@ -obj-y += acpi.o +obj-$(CONFIG_INTEL) += acpi.o obj-y += cpufreq.o -obj-y += hwp.o -obj-y += powernow.o +obj-$(CONFIG_INTEL) += hwp.o +obj-$(CONFIG_AMD) += powernow.o diff --git a/xen/arch/x86/acpi/cpufreq/cpufreq.c b/xen/arch/x86/acpi/cpufreq/cpufreq.c index c1a842e959..59ea1f41d4 100644 --- a/xen/arch/x86/acpi/cpufreq/cpufreq.c +++ b/xen/arch/x86/acpi/cpufreq/cpufreq.c @@ -50,10 +50,12 @@ static int __init cf_check cpufreq_driver_init(void) switch ( cpufreq_xen_opts[i] ) { case CPUFREQ_xen: - ret = acpi_register_driver(); + ret = IS_ENABLED(CONFIG_INTEL) ? + acpi_register_driver() : -ENODEV; break; case CPUFREQ_hwp: - ret = hwp_register_driver(); + ret = IS_ENABLED(CONFIG_INTEL) ? + hwp_register_driver() : -ENODEV; break; case CPUFREQ_none: ret = 0; @@ -67,7 +69,7 @@ static int __init cf_check cpufreq_driver_init(void) case X86_VENDOR_AMD: case X86_VENDOR_HYGON: - ret = powernow_register_driver(); + ret = IS_ENABLED(CONFIG_AMD) ? powernow_register_driver() : -ENODEV; break; } } diff --git a/xen/drivers/acpi/pmstat.c b/xen/drivers/acpi/pmstat.c index 998d2e3c65..ff6b930794 100644 --- a/xen/drivers/acpi/pmstat.c +++ b/xen/drivers/acpi/pmstat.c @@ -255,7 +255,7 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op) strlcpy(op->u.get_para.scaling_driver, "Unknown", CPUFREQ_NAME_LEN); if ( !strncmp(op->u.get_para.scaling_driver, XEN_HWP_DRIVER_NAME, - CPUFREQ_NAME_LEN) ) + CPUFREQ_NAME_LEN) && IS_ENABLED(CONFIG_INTEL) ) ret = get_hwp_para(policy->cpu, &op->u.get_para.u.cppc_para); else { diff --git a/xen/drivers/cpufreq/cpufreq.c b/xen/drivers/cpufreq/cpufreq.c index 8659ad3aee..e61482e5b0 100644 --- a/xen/drivers/cpufreq/cpufreq.c +++ b/xen/drivers/cpufreq/cpufreq.c @@ -120,7 +120,8 @@ static int __init cf_check setup_cpufreq_option(const char *str) if ( arg[0] && arg[1] ) ret = cpufreq_cmdline_parse(arg + 1, end); } - else if ( choice < 0 && !cmdline_strcmp(str, "hwp") ) + else if ( choice < 0 && !cmdline_strcmp(str, "hwp") && + IS_ENABLED(CONFIG_INTEL) ) { xen_processor_pmbits |= XEN_PROCESSOR_PM_PX; cpufreq_controller = FREQCTL_xen; diff --git a/xen/drivers/cpufreq/utility.c b/xen/drivers/cpufreq/utility.c index e690a484f1..aab0b78454 100644 --- a/xen/drivers/cpufreq/utility.c +++ b/xen/drivers/cpufreq/utility.c @@ -379,7 +379,7 @@ int cpufreq_driver_getavg(unsigned int cpu, unsigned int flag) if (!cpu_online(cpu) || !(policy = per_cpu(cpufreq_cpu_policy, cpu))) return 0; - freq_avg = get_measured_perf(cpu, flag); + freq_avg = IS_ENABLED(CONFIG_INTEL) ? get_measured_perf(cpu, flag) : 0; if ( freq_avg > 0 ) return freq_avg; diff --git a/xen/include/acpi/cpufreq/cpufreq.h b/xen/include/acpi/cpufreq/cpufreq.h index 1a8ba3756c..b2e7a592df 100644 --- a/xen/include/acpi/cpufreq/cpufreq.h +++ b/xen/include/acpi/cpufreq/cpufreq.h @@ -254,11 +254,20 @@ void intel_feature_detect(struct cpufreq_policy *policy); int hwp_cmdline_parse(const char *s, const char *e); int hwp_register_driver(void); +#ifdef CONFIG_INTEL bool hwp_active(void); +#else +static inline bool hwp_active(void) +{ + return false; +} +#endif + int get_hwp_para(unsigned int cpu, struct xen_cppc_para *cppc_para); int set_hwp_para(struct cpufreq_policy *policy, struct xen_set_cppc_para *set_cppc); int acpi_register_driver(void); + #endif /* __XEN_CPUFREQ_PM_H__ */
Build AMD Architectural P-state driver when CONFIG_AMD is on, and Intel Hardware P-States driver together with ACPI Processor P-States driver when CONFIG_INTEL is on respectively, allowing for a platform-specific build. Signed-off-by: Sergiy Kibrik <Sergiy_Kibrik@epam.com> CC: Jason Andryuk <jason.andryuk@amd.com> CC: Jan Beulich <jbeulich@suse.com> --- changes in v2: - disable acpi-cpufreq driver as well when !INTEL - leave a stub for hwp_active() only when !INTEL - updated patch description --- xen/arch/x86/acpi/cpufreq/Makefile | 6 +++--- xen/arch/x86/acpi/cpufreq/cpufreq.c | 8 +++++--- xen/drivers/acpi/pmstat.c | 2 +- xen/drivers/cpufreq/cpufreq.c | 3 ++- xen/drivers/cpufreq/utility.c | 2 +- xen/include/acpi/cpufreq/cpufreq.h | 9 +++++++++ 6 files changed, 21 insertions(+), 9 deletions(-)