[PATCH v3 0/2] Improve xenpm output under HWP

[PATCH v3 0/2] Improve xenpm output under HWP

[Jason Andryuk]

Have xen return -EOPNOTSUPP for more pmstat hypercalls instead of
returning misleading data under HWP.  Enhance xenpm to handle
-EOPNOTSUPP and provide more informative messages.

Jason Andryuk (2):
  pmstat: Limit hypercalls under HWP
  xenpm: Print message for disabled commands

 tools/misc/xenpm.c        | 19 ++++++++++++++++---
 xen/drivers/acpi/pmstat.c |  5 +++++
 2 files changed, 21 insertions(+), 3 deletions(-)

-- 
2.43.0

[PATCH v3 1/2] pmstat: Limit hypercalls under HWP

[Jason Andryuk]

When HWP is active, the cpufreq P-state information is not updated.  In
that case, return -EOPNOTSUPP instead of bogus, incomplete info.

Similarly, set_cpufreq_para() is not applicable when HWP is active.
Many of the options already checked the governor and were inaccessible,
but SCALING_MIN/MAX_FREQ was still accessible (though it would do
nothing).  Add an ealier HWP check to handle all cases.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
v2:
Use -EOPNOTSUPP in both places

v3:
Add Jan's R-b
---
 xen/drivers/acpi/pmstat.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/xen/drivers/acpi/pmstat.c b/xen/drivers/acpi/pmstat.c
index 85097d463c..86588ddd42 100644
--- a/xen/drivers/acpi/pmstat.c
+++ b/xen/drivers/acpi/pmstat.c
@@ -66,6 +66,8 @@ int do_get_pm_info(struct xen_sysctl_get_pmstat *op)
             return -ENODEV;
         if ( !cpufreq_driver.init )
             return -ENODEV;
+        if ( hwp_active() )
+            return -EOPNOTSUPP;
         if ( !pmpt || !(pmpt->perf.init & XEN_PX_INIT) )
             return -EINVAL;
         break;
@@ -329,6 +331,9 @@ static int set_cpufreq_para(struct xen_sysctl_pm_op *op)
     if ( !policy || !policy->governor )
         return -EINVAL;
 
+    if ( hwp_active() )
+        return -EOPNOTSUPP;
+
     switch(op->u.set_para.ctrl_type)
     {
     case SCALING_MAX_FREQ:
-- 
2.43.0

[PATCH v3 2/2] xenpm: Print message for disabled commands

[Jason Andryuk]

xenpm get-cpufreq-states currently just prints no output when cpufreq is
disabled or HWP is running.  Have it print an appropriate message.  The
cpufreq disabled one mirrors the cpuidle disabled one.

cpufreq disabled:
$ xenpm get-cpufreq-states
Either Xen cpufreq is disabled or no valid information is registered!

Under HWP:
$ xenpm get-cpufreq-states
P-State information not supported.  Try 'get-cpufreq-average' or 'start'.

Also allow xenpm to handle EOPNOTSUPP from the pmstat hypercalls.
EOPNOTSUPP is returned when HWP is active in some cases and allows the
differentiation from cpufreq being disabled.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
---
v2:
New

v3:
Quote subcommands in error message
Fix style errors
s/mirros/mirrors/ in commit message
---
 tools/misc/xenpm.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c
index d982482a3f..336d246346 100644
--- a/tools/misc/xenpm.c
+++ b/tools/misc/xenpm.c
@@ -362,7 +362,15 @@ static int show_pxstat_by_cpuid(xc_interface *xc_handle, int cpuid)
 
     ret = get_pxstat_by_cpuid(xc_handle, cpuid, &pxstatinfo);
     if ( ret )
+    {
+        if ( ret == -ENODEV )
+            fprintf(stderr,
+                    "Either Xen cpufreq is disabled or no valid information is registered!\n");
+        else if ( ret == -EOPNOTSUPP )
+            fprintf(stderr,
+                    "P-State information not supported.  Try 'get-cpufreq-average' or 'start'.\n");
         return ret;
+    }
 
     print_pxstat(cpuid, &pxstatinfo);
 
@@ -383,8 +391,12 @@ void pxstat_func(int argc, char *argv[])
         /* show pxstates on all cpus */
         int i;
         for ( i = 0; i < max_cpu_nr; i++ )
-            if ( show_pxstat_by_cpuid(xc_handle, i) == -ENODEV )
+        {
+            int ret = show_pxstat_by_cpuid(xc_handle, i);
+
+            if ( ret == -ENODEV || ret == -EOPNOTSUPP )
                 break;
+        }
     }
     else
         show_pxstat_by_cpuid(xc_handle, cpuid);
@@ -432,7 +444,7 @@ static uint64_t *sum, *sum_cx, *sum_px;
 
 static void signal_int_handler(int signo)
 {
-    int i, j, k;
+    int i, j, k, ret;
     struct timeval tv;
     int cx_cap = 0, px_cap = 0;
     xc_cputopo_t *cputopo = NULL;
@@ -473,7 +485,8 @@ static void signal_int_handler(int signo)
                 }
     }
 
-    if ( get_pxstat_by_cpuid(xc_handle, 0, NULL) != -ENODEV )
+    ret = get_pxstat_by_cpuid(xc_handle, 0, NULL);
+    if ( ret != -ENODEV && ret != -EOPNOTSUPP )
     {
         px_cap = 1;
         for ( i = 0; i < max_cpu_nr; i++ )
-- 
2.43.0

Re: [PATCH v3 2/2] xenpm: Print message for disabled commands

[Jan Beulich]

On 25.01.2024 19:14, Jason Andryuk wrote:
> xenpm get-cpufreq-states currently just prints no output when cpufreq is
> disabled or HWP is running.  Have it print an appropriate message.  The
> cpufreq disabled one mirrors the cpuidle disabled one.
> 
> cpufreq disabled:
> $ xenpm get-cpufreq-states
> Either Xen cpufreq is disabled or no valid information is registered!
> 
> Under HWP:
> $ xenpm get-cpufreq-states
> P-State information not supported.  Try 'get-cpufreq-average' or 'start'.
> 
> Also allow xenpm to handle EOPNOTSUPP from the pmstat hypercalls.
> EOPNOTSUPP is returned when HWP is active in some cases and allows the
> differentiation from cpufreq being disabled.
> 
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>

Acked-by: Jan Beulich <jbeulich@suse.com>
on the assumption that really xenpm also ought to be listed in "X86
ARCHITECTURE" in ./MAINTAINERS (and be CONFIG_X86-only in the respective
Makefile).

Jan