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 | 124 +++++++++++++++++++++++++++
xen/drivers/acpi/pmstat.c | 20 ++++-
xen/include/acpi/cpufreq/cpufreq.h | 5 ++
3 files changed, 145 insertions(+), 4 deletions(-)
diff --git a/xen/arch/x86/acpi/cpufreq/amd-cppc.c b/xen/arch/x86/acpi/cpufreq/amd-cppc.c
index 606bb648b3..28c13b09c8 100644
--- a/xen/arch/x86/acpi/cpufreq/amd-cppc.c
+++ b/xen/arch/x86/acpi/cpufreq/amd-cppc.c
@@ -32,6 +32,7 @@
static bool __ro_after_init opt_active_mode;
static DEFINE_PER_CPU_READ_MOSTLY(uint8_t, epp_init);
+static bool __ro_after_init amd_cppc_in_use;
struct amd_cppc_drv_data
{
@@ -513,6 +514,123 @@ 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, epp;
+
+ if ( data == NULL )
+ return -ENOENT;
+
+ /* Validate all parameters - Disallow reserved bits. */
+ if ( set_cppc->minimum > UINT8_MAX || set_cppc->maximum > UINT8_MAX ||
+ set_cppc->desired > UINT8_MAX || set_cppc->energy_perf > UINT8_MAX )
+ 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 in MSR */
+ if ( set_cppc->set_params & XEN_SYSCTL_CPPC_SET_ACT_WINDOW )
+ return -EOPNOTSUPP;
+
+ /* Return if there is nothing to do. */
+ if ( set_cppc->set_params == 0 )
+ return 0;
+
+ epp = per_cpu(epp_init, cpu);
+ /* Apply presets */
+ /*
+ * XEN_SYSCTL_CPPC_SET_PRESET_POWERSAVE/PERFORMANCE/BALANCE are
+ * for amd-cppc in active mode, min_perf could be set with lowest_perf
+ * representing the T-state range of performance levels, while
+ * XEN_SYSCTL_CPPC_SET_PRESET_NONE is for amd-cppc in passive mode, it
+ * depends on governor to do performance scaling, setting with
+ * lowest_nonlinear_perf to ensures performance in P-state range.
+ */
+ 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 =
{
@@ -533,6 +651,11 @@ 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)
{
int ret;
@@ -552,6 +675,7 @@ int __init amd_cppc_register_driver(void)
/* Remove possible fallback option */
xen_processor_pmbits &= ~XEN_PROCESSOR_PM_PX;
+ amd_cppc_in_use = true;
return ret;
}
diff --git a/xen/drivers/acpi/pmstat.c b/xen/drivers/acpi/pmstat.c
index 7f432be761..9c96020d69 100644
--- a/xen/drivers/acpi/pmstat.c
+++ b/xen/drivers/acpi/pmstat.c
@@ -261,7 +261,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)) )
@@ -417,10 +426,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 7c36634d40..0a2eb2a26f 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.1
On 06.03.2025 09:39, Penny Zheng wrote:
> 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 | 124 +++++++++++++++++++++++++++
> xen/drivers/acpi/pmstat.c | 20 ++++-
> xen/include/acpi/cpufreq/cpufreq.h | 5 ++
> 3 files changed, 145 insertions(+), 4 deletions(-)
>
> diff --git a/xen/arch/x86/acpi/cpufreq/amd-cppc.c b/xen/arch/x86/acpi/cpufreq/amd-cppc.c
> index 606bb648b3..28c13b09c8 100644
> --- a/xen/arch/x86/acpi/cpufreq/amd-cppc.c
> +++ b/xen/arch/x86/acpi/cpufreq/amd-cppc.c
> @@ -32,6 +32,7 @@
>
> static bool __ro_after_init opt_active_mode;
> static DEFINE_PER_CPU_READ_MOSTLY(uint8_t, epp_init);
> +static bool __ro_after_init amd_cppc_in_use;
>
> struct amd_cppc_drv_data
> {
> @@ -513,6 +514,123 @@ 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, epp;
> +
> + if ( data == NULL )
> + return -ENOENT;
> +
> + /* Validate all parameters - Disallow reserved bits. */
> + if ( set_cppc->minimum > UINT8_MAX || set_cppc->maximum > UINT8_MAX ||
> + set_cppc->desired > UINT8_MAX || set_cppc->energy_perf > UINT8_MAX )
> + 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 in MSR */
> + if ( set_cppc->set_params & XEN_SYSCTL_CPPC_SET_ACT_WINDOW )
> + return -EOPNOTSUPP;
> +
> + /* Return if there is nothing to do. */
> + if ( set_cppc->set_params == 0 )
> + return 0;
> +
> + epp = per_cpu(epp_init, cpu);
> + /* Apply presets */
> + /*
> + * XEN_SYSCTL_CPPC_SET_PRESET_POWERSAVE/PERFORMANCE/BALANCE are
> + * for amd-cppc in active mode, min_perf could be set with lowest_perf
> + * representing the T-state range of performance levels, while
> + * XEN_SYSCTL_CPPC_SET_PRESET_NONE is for amd-cppc in passive mode, it
> + * depends on governor to do performance scaling, setting with
> + * lowest_nonlinear_perf to ensures performance in P-state range.
> + */
Nit: There are again two consecutive comments here.
The active / passive mode distinction mentioned in the comment isn't
reflected anywhere in the code. It's the XEN_SYSCTL_CPPC_SET_DESIRED
which distinguishes them, yet that flag isn't mentioned in the comment.
> + 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 are still not not both ".lowest_perf", and I still don't understand
- due to the lack of a comment - why that is.
> + 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;
Considering these I even less understand what the comment further up is
about.
> + return amd_cppc_write_request(cpu, min_perf, des_perf, max_perf, epp);
> +}
> +
> +
Nit (not for the first time, I think): No double blank lines please.
> @@ -533,6 +651,11 @@ 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)
> {
> int ret;
> @@ -552,6 +675,7 @@ int __init amd_cppc_register_driver(void)
>
> /* Remove possible fallback option */
> xen_processor_pmbits &= ~XEN_PROCESSOR_PM_PX;
> + amd_cppc_in_use = true;
Is this separate flag really needed? Can't you go from xen_processor_pmbits?
> --- a/xen/drivers/acpi/pmstat.c
> +++ b/xen/drivers/acpi/pmstat.c
> @@ -261,7 +261,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,
Overlong line again.
Jan
© 2016 - 2026 Red Hat, Inc.