[PATCH v5 02/18] xen/cpufreq: move "init" flag into common structure

Penny Zheng posted 18 patches 5 months, 1 week ago
There is a newer version of this series
[PATCH v5 02/18] xen/cpufreq: move "init" flag into common structure
Posted by Penny Zheng 5 months, 1 week ago
AMD cpufreq cores will be intialized in two modes, legacy P-state mode,
and CPPC mode. So "init" flag shall be extracted from px-specific
"struct xen_processor_perf", and placed in the common
"struct processor_pminfo". Otherwise, later when introducing a new
sub-hypercall to propagate CPPC data, we need to pass irrelevant px-specific
"struct xen_processor_perf" to just set init flag.

Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
---
v3 -> v4:
- new commit
---
v4 -> v5:
- commit message refactor
---
 xen/drivers/acpi/pmstat.c                 | 6 +++---
 xen/drivers/cpufreq/cpufreq.c             | 8 ++++----
 xen/include/acpi/cpufreq/processor_perf.h | 4 ++--
 3 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/xen/drivers/acpi/pmstat.c b/xen/drivers/acpi/pmstat.c
index b7fcc02db9..6a1a900f78 100644
--- a/xen/drivers/acpi/pmstat.c
+++ b/xen/drivers/acpi/pmstat.c
@@ -68,7 +68,7 @@ int do_get_pm_info(struct xen_sysctl_get_pmstat *op)
             return -ENODEV;
         if ( hwp_active() )
             return -EOPNOTSUPP;
-        if ( !pmpt || !(pmpt->perf.init & XEN_PX_INIT) )
+        if ( !pmpt || !(pmpt->init & XEN_PX_INIT) )
             return -EINVAL;
         break;
     default:
@@ -228,7 +228,7 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
     ret = copy_to_guest(op->u.get_para.affected_cpus,
                         data, op->u.get_para.cpu_num);
 
-    if ( pmpt->perf.init & XEN_PX_INIT )
+    if ( pmpt->init & XEN_PX_INIT )
     {
         for ( i = 0; i < op->u.get_para.freq_num; i++ )
             data[i] = pmpt->perf.states[i].core_frequency * 1000;
@@ -466,7 +466,7 @@ int do_pm_op(struct xen_sysctl_pm_op *op)
     case CPUFREQ_PARA:
         if ( !(xen_processor_pmbits & XEN_PROCESSOR_PM_PX) )
             return -ENODEV;
-        if ( !pmpt || !(pmpt->perf.init & XEN_PX_INIT) )
+        if ( !pmpt || !(pmpt->init & XEN_PX_INIT) )
             return -EINVAL;
         break;
     }
diff --git a/xen/drivers/cpufreq/cpufreq.c b/xen/drivers/cpufreq/cpufreq.c
index 19e2992335..08d027317c 100644
--- a/xen/drivers/cpufreq/cpufreq.c
+++ b/xen/drivers/cpufreq/cpufreq.c
@@ -209,7 +209,7 @@ int cpufreq_add_cpu(unsigned int cpu)
 
     perf = &processor_pminfo[cpu]->perf;
 
-    if ( !(perf->init & XEN_PX_INIT) )
+    if ( !(processor_pminfo[cpu]->init & XEN_PX_INIT) )
         return -EINVAL;
 
     if (!cpufreq_driver.init)
@@ -373,7 +373,7 @@ int cpufreq_del_cpu(unsigned int cpu)
 
     perf = &processor_pminfo[cpu]->perf;
 
-    if ( !(perf->init & XEN_PX_INIT) )
+    if ( !(processor_pminfo[cpu]->init & XEN_PX_INIT) )
         return -EINVAL;
 
     if (!per_cpu(cpufreq_cpu_policy, cpu))
@@ -569,7 +569,7 @@ int set_px_pminfo(uint32_t acpi_id, struct xen_processor_performance *perf)
         if ( cpufreq_verbose )
             print_PPC(pxpt->platform_limit);
 
-        if ( pxpt->init == XEN_PX_INIT )
+        if ( pmpt->init == XEN_PX_INIT )
         {
             ret = cpufreq_limit_change(cpu);
             goto out;
@@ -578,7 +578,7 @@ int set_px_pminfo(uint32_t acpi_id, struct xen_processor_performance *perf)
 
     if ( perf->flags == ( XEN_PX_PCT | XEN_PX_PSS | XEN_PX_PSD | XEN_PX_PPC ) )
     {
-        pxpt->init = XEN_PX_INIT;
+        pmpt->init = XEN_PX_INIT;
 
         ret = cpufreq_cpu_init(cpu);
         goto out;
diff --git a/xen/include/acpi/cpufreq/processor_perf.h b/xen/include/acpi/cpufreq/processor_perf.h
index 301104e16f..5f2612b15a 100644
--- a/xen/include/acpi/cpufreq/processor_perf.h
+++ b/xen/include/acpi/cpufreq/processor_perf.h
@@ -29,14 +29,14 @@ struct processor_performance {
     struct xen_processor_px *states;
     struct xen_psd_package domain_info;
     uint32_t shared_type;
-
-    uint32_t init;
 };
 
 struct processor_pminfo {
     uint32_t acpi_id;
     uint32_t id;
     struct processor_performance    perf;
+
+    uint32_t init;
 };
 
 extern struct processor_pminfo *processor_pminfo[NR_CPUS];
-- 
2.34.1
Re: [PATCH v5 02/18] xen/cpufreq: move "init" flag into common structure
Posted by Jan Beulich 4 months, 3 weeks ago
On 27.05.2025 10:48, Penny Zheng wrote:
> AMD cpufreq cores will be intialized in two modes, legacy P-state mode,
> and CPPC mode. So "init" flag shall be extracted from px-specific
> "struct xen_processor_perf", and placed in the common
> "struct processor_pminfo". Otherwise, later when introducing a new
> sub-hypercall to propagate CPPC data, we need to pass irrelevant px-specific
> "struct xen_processor_perf" to just set init flag.
> 
> Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>

Acked-by: Jan Beulich <jbeulich@suse.com>

I'd be happy to put this in right away, but aiui ...

> @@ -228,7 +228,7 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
>      ret = copy_to_guest(op->u.get_para.affected_cpus,
>                          data, op->u.get_para.cpu_num);
>  
> -    if ( pmpt->perf.init & XEN_PX_INIT )
> +    if ( pmpt->init & XEN_PX_INIT )
>      {
>          for ( i = 0; i < op->u.get_para.freq_num; i++ )
>              data[i] = pmpt->perf.states[i].core_frequency * 1000;

... this hunk needs dropping then, as the modified code is only introduced
by patch 1. Which raises the question why you didn't order the patches the
other way around, avoiding to immediately need to touch the same (new) code
again. Please clarify.

Jan