Re: [PATCH v2] x86/cpufreq: Add Kconfig option for CPU frequency scaling

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 9 Feb 2026, Jan Beulich wrote:
> On 07.02.2026 00:30, Stefano Stabellini wrote:> --- a/xen/common/Kconfig
> > +++ b/xen/common/Kconfig
> > @@ -652,7 +652,7 @@ endmenu
> >  
> >  config PM_OP
> >     bool "Enable Performance Management Operation"
> > -   depends on ACPI && HAS_CPUFREQ && SYSCTL
> > +   depends on ACPI && CPUFREQ && SYSCTL
> >     default y
> >     help
> >       This option shall enable userspace performance management control
> > @@ -660,7 +660,7 @@ config PM_OP
> >  
> >  config PM_STATS
> >     bool "Enable Performance Management Statistics"
> > -   depends on ACPI && HAS_CPUFREQ && SYSCTL
> > +   depends on ACPI && CPUFREQ && SYSCTL
> >     default y
> >     help
> >       Enable collection of performance management statistics to aid in
> 
> Is the original HAS_CPUFREQ misleading here? do_pm_op() (in pm-op.c) is also
> doing some C-state related work. You may not compile that out just because of
> CPUFREQ=n. Same for pmstat.c.

I managed to resolve this with little changes thanks to DCE


> > --- a/xen/drivers/cpufreq/Kconfig
> > +++ b/xen/drivers/cpufreq/Kconfig
> > @@ -1,3 +1,17 @@
> > -
> >  config HAS_CPUFREQ
> >     bool
> > +
> > +config CPUFREQ
> > +   bool "CPU Frequency scaling"
> > +   default y
> > +   depends on HAS_CPUFREQ
> > +   help
> > +     Enable CPU frequency scaling and power management governors.
> > +     This allows Xen to dynamically adjust CPU P-states (performance
> > +     states) based on system load.
> > +
> > +     Disabling this option removes all cpufreq governors and power
> > +     management interfaces. This is useful for real-time systems or
> > +     minimal hypervisor builds.
> > +
> > +     If unsure, say Y.
> 
> Looks like we're trying to get rid of such re-stating of what the default
> is.

OK


> > --- a/xen/include/acpi/cpufreq/cpufreq.h
> > +++ b/xen/include/acpi/cpufreq/cpufreq.h
> > @@ -381,8 +381,19 @@ int write_ondemand_up_threshold(unsigned int 
> > up_threshold);
> >  
> >  int write_userspace_scaling_setspeed(unsigned int cpu, unsigned int freq);
> >  
> > +#ifdef CONFIG_CPUFREQ
> > +int  cpufreq_add_cpu(unsigned int cpu);
> > +int  cpufreq_del_cpu(unsigned int cpu);
> 
> If already you move these, please also get rid of the double blanks.

OK


> >  void cpufreq_dbs_timer_suspend(void);
> >  void cpufreq_dbs_timer_resume(void);
> > +#else
> > +static inline int  cpufreq_add_cpu(unsigned int cpu) { return -ENOSYS; }
> > +static inline int  cpufreq_del_cpu(unsigned int cpu) { return -ENOSYS; }
> 
> Here and below - no use of ENOSYS, please. EOPNOTSUPP it is everywhere else
> (unless dating back very far).

OK


> > --- a/xen/include/xen/acpi.h
> > +++ b/xen/include/xen/acpi.h
> > @@ -185,8 +185,14 @@ static inline unsigned int 
> > acpi_get_csubstate_limit(void) { return 0; }
> >  static inline void acpi_set_csubstate_limit(unsigned int new_limit) { 
> > return; }
> >  #endif
> >  
> > -#ifdef XEN_GUEST_HANDLE
> 
> If you leave this as-is, ...
> 
> > +#if defined(XEN_GUEST_HANDLE) && defined(CONFIG_CPUFREQ)
> >  int acpi_set_pdc_bits(unsigned int acpi_id, XEN_GUEST_HANDLE(uint32));
> > +#elif defined(XEN_GUEST_HANDLE)
> > +static inline int acpi_set_pdc_bits(unsigned int acpi_id,
> > +                                    XEN_GUEST_HANDLE(uint32) pdc)
> > +{
> > +    return -ENOSYS;
> > +}
> >  #endif
> 
> ... the overall result may be a tiny bit tidier.

OK


> > --- a/xen/include/xen/sched.h
> > +++ b/xen/include/xen/sched.h
> > @@ -1255,9 +1255,14 @@ static always_inline bool is_iommu_enabled(const 
> > struct domain *d)
> >  extern bool sched_smt_power_savings;
> >  extern bool sched_disable_smt_switching;
> >  
> > -extern enum cpufreq_controller {
> > +enum cpufreq_controller {
> >      FREQCTL_none, FREQCTL_dom0_kernel, FREQCTL_xen
> > -} cpufreq_controller;
> 
> This enum would better ...
> 
> > +};
> > +#ifdef CONFIG_CPUFREQ
> > +extern enum cpufreq_controller cpufreq_controller;
> 
> ... stay inside here, then also making the split of type and var decl 
> unnecessary.
>
> The two affected platform-ops likely want compiling out, too.
 
I am not sure I understood your suggestion. If this is what you are
thinking about, it doesn't seem like an improvement.

diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 1b431fc726..e8f5dfd473 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -1255,13 +1255,15 @@ static always_inline bool is_iommu_enabled(const struct 
domain *d)
 extern bool sched_smt_power_savings;
 extern bool sched_disable_smt_switching;
 
+#ifdef CONFIG_CPUFREQ
 enum cpufreq_controller {
     FREQCTL_none, FREQCTL_dom0_kernel, FREQCTL_xen
 };
-#ifdef CONFIG_CPUFREQ
 extern enum cpufreq_controller cpufreq_controller;
 #else
-#define cpufreq_controller FREQCTL_none
+#define FREQCTL_none        0
+#define FREQCTL_dom0_kernel 1
+#define cpufreq_controller  FREQCTL_none
 #endif