| Message ID | 20240122190934.52080-1-jandryuk@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | pmstat: Limit hypercalls under HWP | expand |
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
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:
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(+)