HWP and amd-cppc-epp are both governor-less driver, so we introduce "hw_auto"
flag to together bypass governor-related print in print_cpufreq_para().

In set_cpufreq_para(), a new helper is introduced to help error out when
cpufreq core intialized in governor-less mode.
---
v3 -> v4:
- Include validation check fix here
---
v4 -> v5:
- validation check has beem moved to where XEN_PROCESSOR_PM_CPPC and
XEN_CPPC_INIT have been firstly introduced
- adding "cpufreq_driver.setpolicy == NULL" check to exclude governor-related
para for amd-cppc-epp driver in get/set_cpufreq_para()
---
v5 -> v6:
- add helper cpufreq_is_governorless() to tell whether cpufreq driver is
governor-less
---
 tools/misc/xenpm.c                 | 10 +++++++---
 xen/drivers/acpi/pm-op.c           |  4 ++--
 xen/drivers/cpufreq/cpufreq.c      |  6 ++++++
 xen/include/acpi/cpufreq/cpufreq.h |  1 +
 4 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c
index bdc09f468a..9cb30ea9ce 100644
--- a/tools/misc/xenpm.c
+++ b/tools/misc/xenpm.c
@@ -830,9 +830,13 @@ static void print_cppc_para(unsigned int cpuid,
 /* print out parameters about cpu frequency */
 static void print_cpufreq_para(int cpuid, struct xc_get_cpufreq_para *p_cpufreq)
 {
-    bool hwp = strcmp(p_cpufreq->scaling_driver, XEN_HWP_DRIVER_NAME) == 0;
+    bool hw_auto = false;
     int i;
 
+    if ( !strcmp(p_cpufreq->scaling_driver, XEN_HWP_DRIVER_NAME) ||
+         !strcmp(p_cpufreq->scaling_driver, XEN_AMD_CPPC_EPP_DRIVER_NAME) )
+        hw_auto = true;
+
     printf("cpu id               : %d\n", cpuid);
 
     printf("affected_cpus        :");
@@ -840,7 +844,7 @@ static void print_cpufreq_para(int cpuid, struct xc_get_cpufreq_para *p_cpufreq)
         printf(" %d", p_cpufreq->affected_cpus[i]);
     printf("\n");
 
-    if ( hwp )
+    if ( hw_auto )
         printf("cpuinfo frequency    : base [%"PRIu32"] max [%"PRIu32"]\n",
                p_cpufreq->cpuinfo_min_freq,
                p_cpufreq->cpuinfo_max_freq);
@@ -852,7 +856,7 @@ static void print_cpufreq_para(int cpuid, struct xc_get_cpufreq_para *p_cpufreq)
 
     printf("scaling_driver       : %s\n", p_cpufreq->scaling_driver);
 
-    if ( hwp )
+    if ( hw_auto )
         print_cppc_para(cpuid, &p_cpufreq->u.cppc_para);
     else
     {
diff --git a/xen/drivers/acpi/pm-op.c b/xen/drivers/acpi/pm-op.c
index 077efdfc5c..54815c444b 100644
--- a/xen/drivers/acpi/pm-op.c
+++ b/xen/drivers/acpi/pm-op.c
@@ -244,8 +244,8 @@ static int set_cpufreq_para(struct xen_sysctl_pm_op *op)
     if ( !policy || !policy->governor )
         return -EINVAL;
 
-    if ( hwp_active() )
-        return -EOPNOTSUPP;
+    if ( cpufreq_is_governorless(op->cpuid) )
+         return -EOPNOTSUPP;
 
     switch( op->u.set_para.ctrl_type )
     {
diff --git a/xen/drivers/cpufreq/cpufreq.c b/xen/drivers/cpufreq/cpufreq.c
index 431f2903f8..26aaef6008 100644
--- a/xen/drivers/cpufreq/cpufreq.c
+++ b/xen/drivers/cpufreq/cpufreq.c
@@ -968,3 +968,9 @@ bool cpufreq_in_cppc_passive_mode(unsigned int cpuid)
     return processor_pminfo[cpuid]->init & XEN_CPPC_INIT &&
            cpufreq_driver.target;
 }
+
+bool cpufreq_is_governorless(unsigned int cpuid)
+{
+    return processor_pminfo[cpuid]->init && (hwp_active() ||
+                                             cpufreq_driver.setpolicy);
+}
diff --git a/xen/include/acpi/cpufreq/cpufreq.h b/xen/include/acpi/cpufreq/cpufreq.h
index dd55d268c0..da0456f46d 100644
--- a/xen/include/acpi/cpufreq/cpufreq.h
+++ b/xen/include/acpi/cpufreq/cpufreq.h
@@ -296,5 +296,6 @@ int amd_cppc_cmdline_parse(const char *s, const char *e);
 int amd_cppc_register_driver(void);
 
 bool cpufreq_in_cppc_passive_mode(unsigned int cpuid);
+bool cpufreq_is_governorless(unsigned int cpuid);
 
 #endif /* __XEN_CPUFREQ_PM_H__ */
-- 
2.34.1

On 2025-07-10 23:51, Penny Zheng wrote:
> HWP and amd-cppc-epp are both governor-less driver, so we introduce "hw_auto"
> flag to together bypass governor-related print in print_cpufreq_para().
> 
> In set_cpufreq_para(), a new helper is introduced to help error out when
> cpufreq core intialized in governor-less mode.
> ---
> v3 -> v4:
> - Include validation check fix here
> ---
> v4 -> v5:
> - validation check has beem moved to where XEN_PROCESSOR_PM_CPPC and
> XEN_CPPC_INIT have been firstly introduced
> - adding "cpufreq_driver.setpolicy == NULL" check to exclude governor-related
> para for amd-cppc-epp driver in get/set_cpufreq_para()
> ---
> v5 -> v6:
> - add helper cpufreq_is_governorless() to tell whether cpufreq driver is
> governor-less
> ---

> diff --git a/xen/drivers/acpi/pm-op.c b/xen/drivers/acpi/pm-op.c
> index 077efdfc5c..54815c444b 100644
> --- a/xen/drivers/acpi/pm-op.c
> +++ b/xen/drivers/acpi/pm-op.c
> @@ -244,8 +244,8 @@ static int set_cpufreq_para(struct xen_sysctl_pm_op *op)
>       if ( !policy || !policy->governor )
>           return -EINVAL;
>   
> -    if ( hwp_active() )
> -        return -EOPNOTSUPP;
> +    if ( cpufreq_is_governorless(op->cpuid) )
> +         return -EOPNOTSUPP;

NIT: return indent off by 1.

Regards,
Jason

>   
>       switch( op->u.set_para.ctrl_type )
>       {

On 11.07.2025 05:51, Penny Zheng wrote:
> --- a/xen/drivers/cpufreq/cpufreq.c
> +++ b/xen/drivers/cpufreq/cpufreq.c
> @@ -968,3 +968,9 @@ bool cpufreq_in_cppc_passive_mode(unsigned int cpuid)
>      return processor_pminfo[cpuid]->init & XEN_CPPC_INIT &&
>             cpufreq_driver.target;
>  }
> +
> +bool cpufreq_is_governorless(unsigned int cpuid)
> +{
> +    return processor_pminfo[cpuid]->init && (hwp_active() ||
> +                                             cpufreq_driver.setpolicy);
> +}

The function, by its name, is seemingly generic, yet its implementation
is tailored to the HWP and CPPC drivers. I think such needs calling out
in a comment.

Seeing the XEN_CPPC_INIT check in context, I also wonder why here you
check for ->init just being non-zero.

Jan

[Public]

> -----Original Message-----
> From: Jan Beulich <jbeulich@suse.com>
> Sent: Thursday, July 24, 2025 10:09 PM
> To: Penny, Zheng <penny.zheng@amd.com>
> Cc: Huang, Ray <Ray.Huang@amd.com>; Anthony PERARD
> <anthony.perard@vates.tech>; xen-devel@lists.xenproject.org
> Subject: Re: [PATCH v6 18/19] xen/cpufreq: bypass governor-related para for amd-
> cppc-epp
>
> On 11.07.2025 05:51, Penny Zheng wrote:
> > --- a/xen/drivers/cpufreq/cpufreq.c
> > +++ b/xen/drivers/cpufreq/cpufreq.c
> > @@ -968,3 +968,9 @@ bool cpufreq_in_cppc_passive_mode(unsigned int cpuid)
> >      return processor_pminfo[cpuid]->init & XEN_CPPC_INIT &&
> >             cpufreq_driver.target;
> >  }
> > +
> > +bool cpufreq_is_governorless(unsigned int cpuid) {
> > +    return processor_pminfo[cpuid]->init && (hwp_active() ||
> > +
> > +cpufreq_driver.setpolicy); }
>
> The function, by its name, is seemingly generic, yet its implementation is tailored to
> the HWP and CPPC drivers. I think such needs calling out in a comment.
>
> Seeing the XEN_CPPC_INIT check in context, I also wonder why here you check
> for ->init just being non-zero.
>

Checking ->init being non-zero is to ensure that cpufreq core is initialized successfully, no matter Px mode or CPPC mode.
As non-zero cpufreq_driver.setpolicy callback could only verify that registered cpufreq driver is governorless.
Maybe I shall add comments to explain

> Jan