Re: [PATCH v3 11/20] xen/sysctl: introduce CONFIG_PM_STATS

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 21 Apr 2025, Penny Zheng wrote:
> We introduce a new Kconfig CONFIG_PM_STATS for wrapping all operations
> regarding performance management statistics.
> The major codes reside in xen/drivers/acpi/pmstat.c, including the
> pm-statistic-related sysctl op: do_get_pm_info().
> CONFIG_PM_STATS also shall depend on CONFIG_SYSCTL
> 
> Signed-off-by: Penny Zheng <Penny.Zheng@xxxxxxx>

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
> v1 -> v2:
> - rename to CONFIG_PM_STATS
> - fix indention and stray semicolon
> - make code movements into a new commit
> - No need to wrap inline functions and declarations
> ---
> v2 -> v3:
> - sepearte functions related to do_pm_op() into a new commit
> - both braces shall be moved to the line with the closing parenthesis
> ---
>  xen/arch/x86/acpi/cpu_idle.c              |  2 ++
>  xen/common/Kconfig                        |  8 ++++++++
>  xen/common/sysctl.c                       |  4 ++--
>  xen/drivers/acpi/Makefile                 |  2 +-
>  xen/include/acpi/cpufreq/processor_perf.h | 10 ++++++++++
>  5 files changed, 23 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/x86/acpi/cpu_idle.c b/xen/arch/x86/acpi/cpu_idle.c
> index 420198406d..b537ac4cd6 100644
> --- a/xen/arch/x86/acpi/cpu_idle.c
> +++ b/xen/arch/x86/acpi/cpu_idle.c
> @@ -1487,6 +1487,7 @@ static void amd_cpuidle_init(struct 
> acpi_processor_power *power)
>          vendor_override = -1;
>  }
>  
> +#ifdef CONFIG_PM_STATS
>  uint32_t pmstat_get_cx_nr(unsigned int cpu)
>  {
>      return processor_powers[cpu] ? processor_powers[cpu]->count : 0;
> @@ -1606,6 +1607,7 @@ int pmstat_reset_cx_stat(unsigned int cpu)
>  {
>      return 0;
>  }
> +#endif /* CONFIG_PM_STATS */
>  
>  void cpuidle_disable_deep_cstate(void)
>  {
> diff --git a/xen/common/Kconfig b/xen/common/Kconfig
> index ca1f692487..d8e242eebc 100644
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -600,4 +600,12 @@ config PM_OP
>       help
>         This option shall enable userspace performance management control
>         to do power/performance analyzing and tuning.
> +
> +config PM_STATS
> +     bool "Enable Performance Management Statistics"
> +     depends on ACPI && HAS_CPUFREQ && SYSCTL
> +     default y
> +     help
> +       Enable collection of performance management statistics to aid in
> +       analyzing and tuning power/performance characteristics of the system
>  endmenu
> diff --git a/xen/common/sysctl.c b/xen/common/sysctl.c
> index 4ab827b694..baaad3bd42 100644
> --- a/xen/common/sysctl.c
> +++ b/xen/common/sysctl.c
> @@ -177,11 +177,11 @@ long do_sysctl(XEN_GUEST_HANDLE_PARAM(xen_sysctl_t) 
> u_sysctl)
>          op->u.availheap.avail_bytes <<= PAGE_SHIFT;
>          break;
>  
> -#if defined (CONFIG_ACPI) && defined (CONFIG_HAS_CPUFREQ)
> +#ifdef CONFIG_PM_STATS
>      case XEN_SYSCTL_get_pmstat:
>          ret = do_get_pm_info(&op->u.get_pmstat);
>          break;
> -#endif
> +#endif /* CONFIG_PM_STATS */
>  
>  #ifdef CONFIG_PM_OP
>      case XEN_SYSCTL_pm_op:
> diff --git a/xen/drivers/acpi/Makefile b/xen/drivers/acpi/Makefile
> index e1f84a4468..b52b006100 100644
> --- a/xen/drivers/acpi/Makefile
> +++ b/xen/drivers/acpi/Makefile
> @@ -5,7 +5,7 @@ obj-$(CONFIG_X86) += apei/
>  obj-bin-y += tables.init.o
>  obj-$(CONFIG_ACPI_NUMA) += numa.o
>  obj-y += osl.o
> -obj-$(CONFIG_HAS_CPUFREQ) += pmstat.o
> +obj-$(CONFIG_PM_STATS) += pmstat.o
>  obj-$(CONFIG_PM_OP) += pm_op.o
>  
>  obj-$(CONFIG_X86) += hwregs.o
> diff --git a/xen/include/acpi/cpufreq/processor_perf.h 
> b/xen/include/acpi/cpufreq/processor_perf.h
> index 6de43f8602..a9a3b7a372 100644
> --- a/xen/include/acpi/cpufreq/processor_perf.h
> +++ b/xen/include/acpi/cpufreq/processor_perf.h
> @@ -9,9 +9,19 @@
>  
>  unsigned int powernow_register_driver(void);
>  unsigned int get_measured_perf(unsigned int cpu, unsigned int flag);
> +#ifdef CONFIG_PM_STATS
>  void cpufreq_statistic_update(unsigned int cpu, uint8_t from, uint8_t to);
>  int  cpufreq_statistic_init(unsigned int cpu);
>  void cpufreq_statistic_exit(unsigned int cpu);
> +#else
> +static inline void cpufreq_statistic_update(unsigned int cpu, uint8_t from,
> +                                            uint8_t to) {}
> +static inline int cpufreq_statistic_init(unsigned int cpu)
> +{
> +    return 0;
> +}
> +static inline void cpufreq_statistic_exit(unsigned int cpu) {}
> +#endif /* CONFIG_PM_STATS */
>  
>  int  cpufreq_limit_change(unsigned int cpu);
>  
> -- 
> 2.34.1
>