Introduce helper set_amd_cppc_para and get_amd_cppc_para to
SET/GET CPPC-related para for amd-cppc/amd-cppc-epp driver.
Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
---
v1 -> v2:
- Give the variable des_perf an initializer of 0
- Use the strncmp()s directly in the if()
---
xen/arch/x86/acpi/cpufreq/amd-cppc.c | 115 +++++++++++++++++++++++++++
xen/drivers/acpi/pmstat.c | 20 ++++-
xen/include/acpi/cpufreq/cpufreq.h | 5 ++
3 files changed, 136 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/acpi/cpufreq/amd-cppc.c b/xen/arch/x86/acpi/cpufreq/amd-cppc.c
index 241cce330b..0e43e32979 100644
--- a/xen/arch/x86/acpi/cpufreq/amd-cppc.c
+++ b/xen/arch/x86/acpi/cpufreq/amd-cppc.c
@@ -35,6 +35,7 @@
static bool __ro_after_init opt_cpufreq_active;
static uint8_t __read_mostly epp_init;
+static bool __ro_after_init amd_cppc_in_use;
struct amd_cppc_drv_data
{
@@ -533,6 +534,114 @@ static int cf_check amd_cppc_epp_set_policy(struct cpufreq_policy *policy)
return amd_cppc_epp_update_limit(policy);
}
+int get_amd_cppc_para(unsigned int cpu,
+ struct xen_cppc_para *cppc_para)
+{
+ const struct amd_cppc_drv_data *data = per_cpu(amd_cppc_drv_data, cpu);
+
+ if ( data == NULL )
+ return -ENODATA;
+
+ cppc_para->features = 0;
+ cppc_para->lowest = data->caps.lowest_perf;
+ cppc_para->lowest_nonlinear = data->caps.lowest_nonlinear_perf;
+ cppc_para->nominal = data->caps.nominal_perf;
+ cppc_para->highest = data->caps.highest_perf;
+ cppc_para->minimum = data->req.min_perf;
+ cppc_para->maximum = data->req.max_perf;
+ cppc_para->desired = data->req.des_perf;
+ cppc_para->energy_perf = data->req.epp;
+
+ return 0;
+}
+
+int set_amd_cppc_para(const struct cpufreq_policy *policy,
+ const struct xen_set_cppc_para *set_cppc)
+{
+ unsigned int cpu = policy->cpu;
+ struct amd_cppc_drv_data *data = per_cpu(amd_cppc_drv_data, cpu);
+ uint8_t max_perf, min_perf, des_perf = 0;
+ int epp = -1;
+
+ if ( data == NULL )
+ return -ENOENT;
+
+ /* Validate all parameters - Disallow reserved bits. */
+ if ( set_cppc->minimum > 255 || set_cppc->maximum > 255 ||
+ set_cppc->desired > 255 || set_cppc->energy_perf > 255 )
+ return -EINVAL;
+
+ /* Only allow values if params bit is set. */
+ if ( (!(set_cppc->set_params & XEN_SYSCTL_CPPC_SET_DESIRED) &&
+ set_cppc->desired) ||
+ (!(set_cppc->set_params & XEN_SYSCTL_CPPC_SET_MINIMUM) &&
+ set_cppc->minimum) ||
+ (!(set_cppc->set_params & XEN_SYSCTL_CPPC_SET_MAXIMUM) &&
+ set_cppc->maximum) ||
+ (!(set_cppc->set_params & XEN_SYSCTL_CPPC_SET_ENERGY_PERF) &&
+ set_cppc->energy_perf) )
+ return -EINVAL;
+
+ /* Activity window not supported */
+ if ( set_cppc->set_params & XEN_SYSCTL_CPPC_SET_ACT_WINDOW )
+ return -EINVAL;
+
+ /* Return if there is nothing to do. */
+ if ( set_cppc->set_params == 0 )
+ return 0;
+
+ /* Apply presets */
+ switch ( set_cppc->set_params & XEN_SYSCTL_CPPC_SET_PRESET_MASK )
+ {
+ case XEN_SYSCTL_CPPC_SET_PRESET_POWERSAVE:
+ if ( set_cppc->set_params & XEN_SYSCTL_CPPC_SET_DESIRED )
+ return -EINVAL;
+ min_perf = data->caps.lowest_perf;
+ max_perf = data->caps.highest_perf;
+ epp = CPPC_ENERGY_PERF_MAX_POWERSAVE;
+ break;
+
+ case XEN_SYSCTL_CPPC_SET_PRESET_PERFORMANCE:
+ if ( set_cppc->set_params & XEN_SYSCTL_CPPC_SET_DESIRED )
+ return -EINVAL;
+ min_perf = data->caps.highest_perf;
+ max_perf = data->caps.highest_perf;
+ epp = CPPC_ENERGY_PERF_MAX_PERFORMANCE;
+ break;
+
+ case XEN_SYSCTL_CPPC_SET_PRESET_BALANCE:
+ if ( set_cppc->set_params & XEN_SYSCTL_CPPC_SET_DESIRED )
+ return -EINVAL;
+ min_perf = data->caps.lowest_perf;
+ max_perf = data->caps.highest_perf;
+ epp = CPPC_ENERGY_PERF_BALANCE;
+ break;
+
+ case XEN_SYSCTL_CPPC_SET_PRESET_NONE:
+ min_perf = data->caps.lowest_nonlinear_perf;
+ max_perf = data->caps.highest_perf;
+ break;
+
+ default:
+ return -EINVAL;
+ }
+
+ /* Further customize presets if needed */
+ if ( set_cppc->set_params & XEN_SYSCTL_CPPC_SET_MINIMUM )
+ min_perf = set_cppc->minimum;
+
+ if ( set_cppc->set_params & XEN_SYSCTL_CPPC_SET_MAXIMUM )
+ max_perf = set_cppc->maximum;
+
+ if ( set_cppc->set_params & XEN_SYSCTL_CPPC_SET_ENERGY_PERF )
+ epp = set_cppc->energy_perf;
+
+ if ( set_cppc->set_params & XEN_SYSCTL_CPPC_SET_DESIRED )
+ des_perf = set_cppc->desired;
+
+ return amd_cppc_write_request(cpu, min_perf, des_perf, max_perf, epp);
+}
+
static const struct cpufreq_driver __initconst_cf_clobber amd_cppc_cpufreq_driver =
{
.name = XEN_AMD_CPPC_DRIVER_NAME,
@@ -551,11 +660,17 @@ static const struct cpufreq_driver __initconst_cf_clobber amd_cppc_epp_driver =
.exit = amd_cppc_cpufreq_cpu_exit,
};
+bool amd_cppc_active(void)
+{
+ return amd_cppc_in_use;
+}
+
int __init amd_cppc_register_driver(void)
{
if ( !cpu_has_cppc )
return -ENODEV;
+ amd_cppc_in_use = true;
if ( !opt_cpufreq_active )
return cpufreq_register_driver(&amd_cppc_cpufreq_driver);
else
diff --git a/xen/drivers/acpi/pmstat.c b/xen/drivers/acpi/pmstat.c
index df309e27b4..46899d99d7 100644
--- a/xen/drivers/acpi/pmstat.c
+++ b/xen/drivers/acpi/pmstat.c
@@ -258,7 +258,16 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
!strncmp(op->u.get_para.scaling_driver, XEN_HWP_DRIVER_NAME,
CPUFREQ_NAME_LEN) )
ret = get_hwp_para(policy->cpu, &op->u.get_para.u.cppc_para);
- else
+ else if ( !strncmp(op->u.get_para.scaling_driver, XEN_AMD_CPPC_DRIVER_NAME,
+ CPUFREQ_NAME_LEN) ||
+ !strncmp(op->u.get_para.scaling_driver, XEN_AMD_CPPC_EPP_DRIVER_NAME,
+ CPUFREQ_NAME_LEN) )
+ ret = get_amd_cppc_para(policy->cpu, &op->u.get_para.u.cppc_para);
+
+ if ( strncmp(op->u.get_para.scaling_driver, XEN_HWP_DRIVER_NAME,
+ CPUFREQ_NAME_LEN) &&
+ strncmp(op->u.get_para.scaling_driver, XEN_AMD_CPPC_EPP_DRIVER_NAME,
+ CPUFREQ_NAME_LEN) )
{
if ( !(scaling_available_governors =
xzalloc_array(char, gov_num * CPUFREQ_NAME_LEN)) )
@@ -414,10 +423,13 @@ static int set_cpufreq_cppc(struct xen_sysctl_pm_op *op)
if ( !policy || !policy->governor )
return -ENOENT;
- if ( !hwp_active() )
- return -EOPNOTSUPP;
+ if ( hwp_active() )
+ return set_hwp_para(policy, &op->u.set_cppc);
+
+ if ( amd_cppc_active() )
+ return set_amd_cppc_para(policy, &op->u.set_cppc);
- return set_hwp_para(policy, &op->u.set_cppc);
+ return -EOPNOTSUPP;
}
int do_pm_op(struct xen_sysctl_pm_op *op)
diff --git a/xen/include/acpi/cpufreq/cpufreq.h b/xen/include/acpi/cpufreq/cpufreq.h
index fb42d61567..cc5a248193 100644
--- a/xen/include/acpi/cpufreq/cpufreq.h
+++ b/xen/include/acpi/cpufreq/cpufreq.h
@@ -292,5 +292,10 @@ int acpi_cpufreq_register(void);
int amd_cppc_cmdline_parse(const char *s, const char *e);
int amd_cppc_register_driver(void);
+bool amd_cppc_active(void);
+int get_amd_cppc_para(unsigned int cpu,
+ struct xen_cppc_para *cppc_para);
+int set_amd_cppc_para(const struct cpufreq_policy *policy,
+ const struct xen_set_cppc_para *set_cppc);
#endif /* __XEN_CPUFREQ_PM_H__ */
--
2.34.1On 06.02.2025 09:32, Penny Zheng wrote:
> @@ -533,6 +534,114 @@ static int cf_check amd_cppc_epp_set_policy(struct cpufreq_policy *policy)
> return amd_cppc_epp_update_limit(policy);
> }
>
> +int get_amd_cppc_para(unsigned int cpu,
> + struct xen_cppc_para *cppc_para)
> +{
> + const struct amd_cppc_drv_data *data = per_cpu(amd_cppc_drv_data, cpu);
> +
> + if ( data == NULL )
> + return -ENODATA;
> +
> + cppc_para->features = 0;
> + cppc_para->lowest = data->caps.lowest_perf;
> + cppc_para->lowest_nonlinear = data->caps.lowest_nonlinear_perf;
> + cppc_para->nominal = data->caps.nominal_perf;
> + cppc_para->highest = data->caps.highest_perf;
> + cppc_para->minimum = data->req.min_perf;
> + cppc_para->maximum = data->req.max_perf;
> + cppc_para->desired = data->req.des_perf;
> + cppc_para->energy_perf = data->req.epp;
> +
> + return 0;
> +}
> +
> +int set_amd_cppc_para(const struct cpufreq_policy *policy,
> + const struct xen_set_cppc_para *set_cppc)
> +{
> + unsigned int cpu = policy->cpu;
> + struct amd_cppc_drv_data *data = per_cpu(amd_cppc_drv_data, cpu);
> + uint8_t max_perf, min_perf, des_perf = 0;
> + int epp = -1;
> +
> + if ( data == NULL )
> + return -ENOENT;
> +
> + /* Validate all parameters - Disallow reserved bits. */
> + if ( set_cppc->minimum > 255 || set_cppc->maximum > 255 ||
> + set_cppc->desired > 255 || set_cppc->energy_perf > 255 )
> + return -EINVAL;
In an earlier patch I just looked at you use UINT8_MAX for bounds checking.
I'm not overly fussed which of the two its is, but I'd like to ask for it
to be consistent throughout the driver. Unless of course there's a reason
for the difference.
> + /* Only allow values if params bit is set. */
> + if ( (!(set_cppc->set_params & XEN_SYSCTL_CPPC_SET_DESIRED) &&
> + set_cppc->desired) ||
> + (!(set_cppc->set_params & XEN_SYSCTL_CPPC_SET_MINIMUM) &&
> + set_cppc->minimum) ||
> + (!(set_cppc->set_params & XEN_SYSCTL_CPPC_SET_MAXIMUM) &&
> + set_cppc->maximum) ||
> + (!(set_cppc->set_params & XEN_SYSCTL_CPPC_SET_ENERGY_PERF) &&
> + set_cppc->energy_perf) )
> + return -EINVAL;
> +
> + /* Activity window not supported */
> + if ( set_cppc->set_params & XEN_SYSCTL_CPPC_SET_ACT_WINDOW )
> + return -EINVAL;
"not supported" as in "support may appear later"? The -EOPNOTSUPP may be
more appropriate. Else the comment may want re-wording.
> + /* Return if there is nothing to do. */
> + if ( set_cppc->set_params == 0 )
> + return 0;
> +
> + /* Apply presets */
> + switch ( set_cppc->set_params & XEN_SYSCTL_CPPC_SET_PRESET_MASK )
> + {
> + case XEN_SYSCTL_CPPC_SET_PRESET_POWERSAVE:
> + if ( set_cppc->set_params & XEN_SYSCTL_CPPC_SET_DESIRED )
> + return -EINVAL;
> + min_perf = data->caps.lowest_perf;
> + max_perf = data->caps.highest_perf;
These match ...
> + epp = CPPC_ENERGY_PERF_MAX_POWERSAVE;
> + break;
> +
> + case XEN_SYSCTL_CPPC_SET_PRESET_PERFORMANCE:
> + if ( set_cppc->set_params & XEN_SYSCTL_CPPC_SET_DESIRED )
> + return -EINVAL;
> + min_perf = data->caps.highest_perf;
> + max_perf = data->caps.highest_perf;
> + epp = CPPC_ENERGY_PERF_MAX_PERFORMANCE;
> + break;
> +
> + case XEN_SYSCTL_CPPC_SET_PRESET_BALANCE:
> + if ( set_cppc->set_params & XEN_SYSCTL_CPPC_SET_DESIRED )
> + return -EINVAL;
> + min_perf = data->caps.lowest_perf;
> + max_perf = data->caps.highest_perf;
... these, which doesn't seem quite right. It feels like I had asked about this
on v1 already. If that's really intended, please add a clarifying comment to
the POWERSAVE block.
> + epp = CPPC_ENERGY_PERF_BALANCE;
> + break;
> +
> + case XEN_SYSCTL_CPPC_SET_PRESET_NONE:
> + min_perf = data->caps.lowest_nonlinear_perf;
> + max_perf = data->caps.highest_perf;
> + break;
Similarly I think the use of lowest_nonlinear_perf deserves a comment here.
> @@ -551,11 +660,17 @@ static const struct cpufreq_driver __initconst_cf_clobber amd_cppc_epp_driver =
> .exit = amd_cppc_cpufreq_cpu_exit,
> };
>
> +bool amd_cppc_active(void)
> +{
> + return amd_cppc_in_use;
> +}
> +
> int __init amd_cppc_register_driver(void)
> {
> if ( !cpu_has_cppc )
> return -ENODEV;
>
> + amd_cppc_in_use = true;
Isn't this permature? I.e. wouldn't you better do so only ...
> if ( !opt_cpufreq_active )
> return cpufreq_register_driver(&amd_cppc_cpufreq_driver);
> else
... after successful driver registration?
Jan
© 2016 - 2025 Red Hat, Inc.