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

[Jan Beulich <jbeulich@xxxxxxxx>]

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.

> --- 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.

> --- 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.

>  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).

> --- 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.

> --- 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.

Jan