When HWP is active, the cpufreq P-state information is not updated.  In
that case, return -ENODEV instead of bogus, incomplete info.  The xenpm
command already supports skipping results when -ENODEV is returned, so
it is re-used when -EOPNOTSUPP might be more accurate.

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>
---
`xenpm get-cpufreq-states` now doesn't return any output.  It also exits
successfully since xenpm doesn't check the returns there.  Other
commands will fail:
xenpm set-scaling-maxfreq 11 1100000
failed to set scaling max freq (95 - Operation not supported)

 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..4c4d298d1c 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 -ENODEV;
         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

On 22.01.2024 20:09, Jason Andryuk wrote:
> When HWP is active, the cpufreq P-state information is not updated.  In
> that case, return -ENODEV instead of bogus, incomplete info.  The xenpm
> command already supports skipping results when -ENODEV is returned, so
> it is re-used when -EOPNOTSUPP might be more accurate.
> 
> 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>
> ---
> `xenpm get-cpufreq-states` now doesn't return any output.  It also exits
> successfully since xenpm doesn't check the returns there.

This isn't very nice. Is there nothing sensible that can be output
instead in the HWP case? If not, I think it would help if
inapplicability of the command would be indicated by at least one line
of output. Or might it make sense to at least fall back to
get-cpufreq-average in that case?

>  Other
> commands will fail:
> xenpm set-scaling-maxfreq 11 1100000
> failed to set scaling max freq (95 - Operation not supported)

This is fine imo.

Jan

On Tue, Jan 23, 2024 at 3:17 AM Jan Beulich <jbeulich@suse.com> wrote:
>
> On 22.01.2024 20:09, Jason Andryuk wrote:
> > When HWP is active, the cpufreq P-state information is not updated.  In
> > that case, return -ENODEV instead of bogus, incomplete info.  The xenpm
> > command already supports skipping results when -ENODEV is returned, so
> > it is re-used when -EOPNOTSUPP might be more accurate.
> >
> > 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>
> > ---
> > `xenpm get-cpufreq-states` now doesn't return any output.  It also exits
> > successfully since xenpm doesn't check the returns there.
>
> This isn't very nice. Is there nothing sensible that can be output
> instead in the HWP case? If not, I think it would help if
> inapplicability of the command would be indicated by at least one line
> of output. Or might it make sense to at least fall back to
> get-cpufreq-average in that case?

Sorry, I should have explained more, but, yes, not nice.  "No output
and exits with success" is how xenpm works if -ENODEV is received -
which I guess occurs when cpufreq is disabled (regardless of HWP).  I
found that surprising, but that behaviour is matched under HWP with
this patch.

Yes, `xenpm get-cpufreq-states` can be enhanced.  The re-use of ENODEV
was useful to make `xenpm get-cpufreq-average` output C-states but
skip P-states.  If EOPNOTSUPP is used, then that can differentiate
when HWP is used.  So `xenpm get-cpufreq-states` can print a message
when cpufreq is disabled, and a different one when P-States  are
unavailable.

Regards,
Jason