We extract cppc info from "struct xen_get_cpufreq_para", where it acts as
a member of union, and share the space with governor info.
However, it may fail in amd-cppc passive mode, in which governor info and
CPPC info could co-exist, and both need to be printed together via xenpm tool.
If we tried to still put it in "struct xen_get_cpufreq_para" (e.g. just move
out of union), "struct xen_get_cpufreq_para" will enlarge too much to further
make xen_sysctl.u exceed 128 bytes.
So we introduce a new sub-field GET_CPUFREQ_CPPC to dedicatedly acquire
CPPC-related para, and make get-cpufreq-para invoke GET_CPUFREQ_CPPC
if available.
New helpers print_cppc_para() and get_cpufreq_cppc() are introduced to
extract CPPC-related parameters process from cpufreq para.
Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
Acked-by: Jan Beulich <jbeulich@suse.com> # hypervisor
Acked-by: Anthony PERARD <anthony.perard@vates.tech>
---
v4 -> v5:
- new commit
---
v5 -> v6:
- remove the changes for get-cpufreq-para
---
v6 -> v7:
- make get-cpufreq-para invoke GET_CPUFREQ_CPPC
---
v7 -> v8:
- use structure assignment as it is a alias
- add errno info to the error print
---
v9 -> v10
- drop the outer union
---
tools/include/xenctrl.h | 27 +++++-----
tools/libs/ctrl/xc_pm.c | 47 +++++++++++-----
tools/misc/xenpm.c | 103 ++++++++++++++++++++++--------------
xen/drivers/acpi/pm-op.c | 43 +++++++++------
xen/include/public/sysctl.h | 31 ++++++-----
5 files changed, 155 insertions(+), 96 deletions(-)
diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index 965d3b585a..c14ecd66aa 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -1924,22 +1924,19 @@ struct xc_get_cpufreq_para {
uint32_t cpuinfo_cur_freq;
uint32_t cpuinfo_max_freq;
uint32_t cpuinfo_min_freq;
- union {
- struct {
- uint32_t scaling_cur_freq;
+ struct {
+ uint32_t scaling_cur_freq;
- char scaling_governor[CPUFREQ_NAME_LEN];
- uint32_t scaling_max_freq;
- uint32_t scaling_min_freq;
+ char scaling_governor[CPUFREQ_NAME_LEN];
+ uint32_t scaling_max_freq;
+ uint32_t scaling_min_freq;
- /* for specific governor */
- union {
- xc_userspace_t userspace;
- xc_ondemand_t ondemand;
- } u;
- } s;
- xc_cppc_para_t cppc_para;
- } u;
+ /* for specific governor */
+ union {
+ xc_userspace_t userspace;
+ xc_ondemand_t ondemand;
+ } u;
+ } s;
int32_t turbo_enabled;
};
@@ -1953,6 +1950,8 @@ int xc_set_cpufreq_para(xc_interface *xch, int cpuid,
int ctrl_type, int ctrl_value);
int xc_set_cpufreq_cppc(xc_interface *xch, int cpuid,
xc_set_cppc_para_t *set_cppc);
+int xc_get_cppc_para(xc_interface *xch, unsigned int cpuid,
+ xc_cppc_para_t *cppc_para);
int xc_get_cpufreq_avgfreq(xc_interface *xch, int cpuid, int *avg_freq);
int xc_set_sched_opt_smt(xc_interface *xch, uint32_t value);
diff --git a/tools/libs/ctrl/xc_pm.c b/tools/libs/ctrl/xc_pm.c
index 6fda973f1f..5b4e489acf 100644
--- a/tools/libs/ctrl/xc_pm.c
+++ b/tools/libs/ctrl/xc_pm.c
@@ -274,25 +274,24 @@ int xc_get_cpufreq_para(xc_interface *xch, int cpuid,
/*
* Copy to user_para no matter what cpufreq driver/governor.
*
- * First sanity check layout of the union subject to memcpy() below.
+ * First sanity check layout of the struct subject to memcpy() below.
*/
- BUILD_BUG_ON(sizeof(user_para->u) != sizeof(sys_para->u));
+ BUILD_BUG_ON(sizeof(user_para->s) != sizeof(sys_para->s));
#define CHK_FIELD(fld) \
- BUILD_BUG_ON(offsetof(typeof(user_para->u), fld) != \
- offsetof(typeof(sys_para->u), fld))
+ BUILD_BUG_ON(offsetof(typeof(user_para->s), fld) != \
+ offsetof(typeof(sys_para->s), fld))
- CHK_FIELD(s.scaling_cur_freq);
- CHK_FIELD(s.scaling_governor);
- CHK_FIELD(s.scaling_max_freq);
- CHK_FIELD(s.scaling_min_freq);
- CHK_FIELD(s.u.userspace);
- CHK_FIELD(s.u.ondemand);
- CHK_FIELD(cppc_para);
+ CHK_FIELD(scaling_cur_freq);
+ CHK_FIELD(scaling_governor);
+ CHK_FIELD(scaling_max_freq);
+ CHK_FIELD(scaling_min_freq);
+ CHK_FIELD(u.userspace);
+ CHK_FIELD(u.ondemand);
#undef CHK_FIELD
- memcpy(&user_para->u, &sys_para->u, sizeof(sys_para->u));
+ memcpy(&user_para->s, &sys_para->s, sizeof(sys_para->s));
}
unlock_4:
@@ -366,6 +365,30 @@ int xc_set_cpufreq_cppc(xc_interface *xch, int cpuid,
return ret;
}
+int xc_get_cppc_para(xc_interface *xch, unsigned int cpuid,
+ xc_cppc_para_t *cppc_para)
+{
+ int ret;
+ struct xen_sysctl sysctl = {};
+
+ if ( !xch || !cppc_para )
+ {
+ errno = EINVAL;
+ return -1;
+ }
+
+ sysctl.cmd = XEN_SYSCTL_pm_op;
+ sysctl.u.pm_op.cmd = GET_CPUFREQ_CPPC;
+ sysctl.u.pm_op.cpuid = cpuid;
+
+ ret = xc_sysctl(xch, &sysctl);
+ if ( ret )
+ return ret;
+
+ *cppc_para = sysctl.u.pm_op.u.get_cppc;
+ return ret;
+}
+
int xc_get_cpufreq_avgfreq(xc_interface *xch, int cpuid, int *avg_freq)
{
int ret = 0;
diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c
index 6b054b10a4..c7f19cea28 100644
--- a/tools/misc/xenpm.c
+++ b/tools/misc/xenpm.c
@@ -801,6 +801,34 @@ static unsigned int calculate_activity_window(const xc_cppc_para_t *cppc,
return mantissa * multiplier;
}
+/* print out parameters about cpu cppc */
+static void print_cppc_para(unsigned int cpuid,
+ const xc_cppc_para_t *cppc)
+{
+ printf("cppc variables :\n");
+ printf(" hardware limits : lowest [%"PRIu32"] lowest nonlinear [%"PRIu32"]\n",
+ cppc->lowest, cppc->lowest_nonlinear);
+ printf(" : nominal [%"PRIu32"] highest [%"PRIu32"]\n",
+ cppc->nominal, cppc->highest);
+ printf(" configured limits : min [%"PRIu32"] max [%"PRIu32"] energy perf [%"PRIu32"]\n",
+ cppc->minimum, cppc->maximum, cppc->energy_perf);
+
+ if ( cppc->features & XEN_SYSCTL_CPPC_FEAT_ACT_WINDOW )
+ {
+ unsigned int activity_window;
+ const char *units;
+
+ activity_window = calculate_activity_window(cppc, &units);
+ printf(" : activity_window [%"PRIu32" %s]\n",
+ activity_window, units);
+ }
+
+ printf(" : desired [%"PRIu32"%s]\n",
+ cppc->desired,
+ cppc->desired ? "" : " hw autonomous");
+ printf("\n");
+}
+
/* print out parameters about cpu frequency */
static void print_cpufreq_para(int cpuid, struct xc_get_cpufreq_para *p_cpufreq)
{
@@ -826,71 +854,45 @@ static void print_cpufreq_para(int cpuid, struct xc_get_cpufreq_para *p_cpufreq)
printf("scaling_driver : %s\n", p_cpufreq->scaling_driver);
- if ( hwp )
- {
- const xc_cppc_para_t *cppc = &p_cpufreq->u.cppc_para;
-
- printf("cppc variables :\n");
- printf(" hardware limits : lowest [%"PRIu32"] lowest nonlinear [%"PRIu32"]\n",
- cppc->lowest, cppc->lowest_nonlinear);
- printf(" : nominal [%"PRIu32"] highest [%"PRIu32"]\n",
- cppc->nominal, cppc->highest);
- printf(" configured limits : min [%"PRIu32"] max [%"PRIu32"] energy perf [%"PRIu32"]\n",
- cppc->minimum, cppc->maximum, cppc->energy_perf);
-
- if ( cppc->features & XEN_SYSCTL_CPPC_FEAT_ACT_WINDOW )
- {
- unsigned int activity_window;
- const char *units;
-
- activity_window = calculate_activity_window(cppc, &units);
- printf(" : activity_window [%"PRIu32" %s]\n",
- activity_window, units);
- }
-
- printf(" : desired [%"PRIu32"%s]\n",
- cppc->desired,
- cppc->desired ? "" : " hw autonomous");
- }
- else
+ if ( !hwp )
{
if ( p_cpufreq->gov_num )
printf("scaling_avail_gov : %s\n",
p_cpufreq->scaling_available_governors);
- printf("current_governor : %s\n", p_cpufreq->u.s.scaling_governor);
- if ( !strncmp(p_cpufreq->u.s.scaling_governor,
+ printf("current_governor : %s\n", p_cpufreq->s.scaling_governor);
+ if ( !strncmp(p_cpufreq->s.scaling_governor,
"userspace", CPUFREQ_NAME_LEN) )
{
printf(" userspace specific :\n");
printf(" scaling_setspeed : %u\n",
- p_cpufreq->u.s.u.userspace.scaling_setspeed);
+ p_cpufreq->s.u.userspace.scaling_setspeed);
}
- else if ( !strncmp(p_cpufreq->u.s.scaling_governor,
+ else if ( !strncmp(p_cpufreq->s.scaling_governor,
"ondemand", CPUFREQ_NAME_LEN) )
{
printf(" ondemand specific :\n");
printf(" sampling_rate : max [%u] min [%u] cur [%u]\n",
- p_cpufreq->u.s.u.ondemand.sampling_rate_max,
- p_cpufreq->u.s.u.ondemand.sampling_rate_min,
- p_cpufreq->u.s.u.ondemand.sampling_rate);
+ p_cpufreq->s.u.ondemand.sampling_rate_max,
+ p_cpufreq->s.u.ondemand.sampling_rate_min,
+ p_cpufreq->s.u.ondemand.sampling_rate);
printf(" up_threshold : %u\n",
- p_cpufreq->u.s.u.ondemand.up_threshold);
+ p_cpufreq->s.u.ondemand.up_threshold);
}
printf("scaling_avail_freq :");
for ( i = 0; i < p_cpufreq->freq_num; i++ )
if ( p_cpufreq->scaling_available_frequencies[i] ==
- p_cpufreq->u.s.scaling_cur_freq )
+ p_cpufreq->s.scaling_cur_freq )
printf(" *%d", p_cpufreq->scaling_available_frequencies[i]);
else
printf(" %d", p_cpufreq->scaling_available_frequencies[i]);
printf("\n");
printf("scaling frequency : max [%u] min [%u] cur [%u]\n",
- p_cpufreq->u.s.scaling_max_freq,
- p_cpufreq->u.s.scaling_min_freq,
- p_cpufreq->u.s.scaling_cur_freq);
+ p_cpufreq->s.scaling_max_freq,
+ p_cpufreq->s.scaling_min_freq,
+ p_cpufreq->s.scaling_cur_freq);
}
printf("turbo mode : %s\n",
@@ -898,6 +900,24 @@ static void print_cpufreq_para(int cpuid, struct xc_get_cpufreq_para *p_cpufreq)
printf("\n");
}
+/* show cpu cppc parameters information on CPU cpuid */
+static int show_cppc_para_by_cpuid(xc_interface *xc_handle, unsigned int cpuid)
+{
+ int ret;
+ xc_cppc_para_t cppc_para;
+
+ ret = xc_get_cppc_para(xc_handle, cpuid, &cppc_para);
+ if ( !ret )
+ print_cppc_para(cpuid, &cppc_para);
+ else if ( errno == ENODEV )
+ ret = 0; /* Ignore unsupported platform */
+ else
+ fprintf(stderr, "[CPU%u] failed to get cppc parameter: %s\n",
+ cpuid, strerror(errno));
+
+ return ret;
+}
+
/* show cpu frequency parameters information on CPU cpuid */
static int show_cpufreq_para_by_cpuid(xc_interface *xc_handle, int cpuid)
{
@@ -957,7 +977,12 @@ static int show_cpufreq_para_by_cpuid(xc_interface *xc_handle, int cpuid)
} while ( ret && errno == EAGAIN );
if ( ret == 0 )
+ {
print_cpufreq_para(cpuid, p_cpufreq);
+
+ /* Show CPPC parameters if available */
+ ret = show_cppc_para_by_cpuid(xc_handle, cpuid);
+ }
else if ( errno == ENODEV )
{
ret = -ENODEV;
diff --git a/xen/drivers/acpi/pm-op.c b/xen/drivers/acpi/pm-op.c
index a7eaf29c31..f50acd7088 100644
--- a/xen/drivers/acpi/pm-op.c
+++ b/xen/drivers/acpi/pm-op.c
@@ -77,6 +77,17 @@ static int read_scaling_available_governors(char *scaling_available_governors,
return 0;
}
+static int get_cpufreq_cppc(unsigned int cpu,
+ struct xen_get_cppc_para *cppc_para)
+{
+ int ret = -ENODEV;
+
+ if ( hwp_active() )
+ ret = get_hwp_para(cpu, cppc_para);
+
+ return ret;
+}
+
static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
{
uint32_t ret = 0;
@@ -143,9 +154,7 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
else
strlcpy(op->u.get_para.scaling_driver, "Unknown", CPUFREQ_NAME_LEN);
- if ( hwp_active() )
- ret = get_hwp_para(policy->cpu, &op->u.get_para.u.cppc_para);
- else
+ if ( !hwp_active() )
{
if ( !(scaling_available_governors =
xzalloc_array(char, gov_num * CPUFREQ_NAME_LEN)) )
@@ -165,29 +174,29 @@ static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
if ( ret )
return -EFAULT;
- op->u.get_para.u.s.scaling_cur_freq = policy->cur;
- op->u.get_para.u.s.scaling_max_freq = policy->max;
- op->u.get_para.u.s.scaling_min_freq = policy->min;
+ op->u.get_para.s.scaling_cur_freq = policy->cur;
+ op->u.get_para.s.scaling_max_freq = policy->max;
+ op->u.get_para.s.scaling_min_freq = policy->min;
if ( policy->governor->name[0] )
- strlcpy(op->u.get_para.u.s.scaling_governor,
+ strlcpy(op->u.get_para.s.scaling_governor,
policy->governor->name, CPUFREQ_NAME_LEN);
else
- strlcpy(op->u.get_para.u.s.scaling_governor, "Unknown",
+ strlcpy(op->u.get_para.s.scaling_governor, "Unknown",
CPUFREQ_NAME_LEN);
/* governor specific para */
- if ( !strncasecmp(op->u.get_para.u.s.scaling_governor,
+ if ( !strncasecmp(op->u.get_para.s.scaling_governor,
"userspace", CPUFREQ_NAME_LEN) )
- op->u.get_para.u.s.u.userspace.scaling_setspeed = policy->cur;
+ op->u.get_para.s.u.userspace.scaling_setspeed = policy->cur;
- if ( !strncasecmp(op->u.get_para.u.s.scaling_governor,
+ if ( !strncasecmp(op->u.get_para.s.scaling_governor,
"ondemand", CPUFREQ_NAME_LEN) )
ret = get_cpufreq_ondemand_para(
- &op->u.get_para.u.s.u.ondemand.sampling_rate_max,
- &op->u.get_para.u.s.u.ondemand.sampling_rate_min,
- &op->u.get_para.u.s.u.ondemand.sampling_rate,
- &op->u.get_para.u.s.u.ondemand.up_threshold);
+ &op->u.get_para.s.u.ondemand.sampling_rate_max,
+ &op->u.get_para.s.u.ondemand.sampling_rate_min,
+ &op->u.get_para.s.u.ondemand.sampling_rate,
+ &op->u.get_para.s.u.ondemand.up_threshold);
}
return ret;
@@ -385,6 +394,10 @@ int do_pm_op(struct xen_sysctl_pm_op *op)
ret = set_cpufreq_para(op);
break;
+ case GET_CPUFREQ_CPPC:
+ ret = get_cpufreq_cppc(op->cpuid, &op->u.get_cppc);
+ break;
+
case SET_CPUFREQ_CPPC:
ret = set_cpufreq_cppc(op);
break;
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index eb3a23b038..66c9b65465 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -478,22 +478,19 @@ struct xen_get_cpufreq_para {
uint32_t cpuinfo_cur_freq;
uint32_t cpuinfo_max_freq;
uint32_t cpuinfo_min_freq;
- union {
- struct {
- uint32_t scaling_cur_freq;
-
- char scaling_governor[CPUFREQ_NAME_LEN];
- uint32_t scaling_max_freq;
- uint32_t scaling_min_freq;
-
- /* for specific governor */
- union {
- struct xen_userspace userspace;
- struct xen_ondemand ondemand;
- } u;
- } s;
- struct xen_get_cppc_para cppc_para;
- } u;
+ struct {
+ uint32_t scaling_cur_freq;
+
+ char scaling_governor[CPUFREQ_NAME_LEN];
+ uint32_t scaling_max_freq;
+ uint32_t scaling_min_freq;
+
+ /* for specific governor */
+ union {
+ struct xen_userspace userspace;
+ struct xen_ondemand ondemand;
+ } u;
+ } s;
int32_t turbo_enabled;
};
@@ -523,6 +520,7 @@ struct xen_sysctl_pm_op {
#define SET_CPUFREQ_PARA (CPUFREQ_PARA | 0x03)
#define GET_CPUFREQ_AVGFREQ (CPUFREQ_PARA | 0x04)
#define SET_CPUFREQ_CPPC (CPUFREQ_PARA | 0x05)
+ #define GET_CPUFREQ_CPPC (CPUFREQ_PARA | 0x06)
/* set/reset scheduler power saving option */
#define XEN_SYSCTL_pm_op_set_sched_opt_smt 0x21
@@ -547,6 +545,7 @@ struct xen_sysctl_pm_op {
uint32_t cpuid;
union {
struct xen_get_cpufreq_para get_para;
+ struct xen_get_cppc_para get_cppc;
struct xen_set_cpufreq_gov set_gov;
struct xen_set_cpufreq_para set_para;
struct xen_set_cppc_para set_cppc;
--
2.34.1On 23.09.2025 06:38, Penny Zheng wrote:
> We extract cppc info from "struct xen_get_cpufreq_para", where it acts as
> a member of union, and share the space with governor info.
> However, it may fail in amd-cppc passive mode, in which governor info and
> CPPC info could co-exist, and both need to be printed together via xenpm tool.
> If we tried to still put it in "struct xen_get_cpufreq_para" (e.g. just move
> out of union), "struct xen_get_cpufreq_para" will enlarge too much to further
> make xen_sysctl.u exceed 128 bytes.
>
> So we introduce a new sub-field GET_CPUFREQ_CPPC to dedicatedly acquire
> CPPC-related para, and make get-cpufreq-para invoke GET_CPUFREQ_CPPC
> if available.
> New helpers print_cppc_para() and get_cpufreq_cppc() are introduced to
> extract CPPC-related parameters process from cpufreq para.
>
> Signed-off-by: Penny Zheng <Penny.Zheng@amd.com>
> Acked-by: Jan Beulich <jbeulich@suse.com> # hypervisor
> Acked-by: Anthony PERARD <anthony.perard@vates.tech>
> ---
> v4 -> v5:
> - new commit
> ---
> v5 -> v6:
> - remove the changes for get-cpufreq-para
> ---
> v6 -> v7:
> - make get-cpufreq-para invoke GET_CPUFREQ_CPPC
> ---
> v7 -> v8:
> - use structure assignment as it is a alias
> - add errno info to the error print
> ---
> v9 -> v10
> - drop the outer union
In the interest of getting this series in I think we will want to take
this patch as is (I yet have to see the other, related patch though),
but ...
> --- a/xen/include/public/sysctl.h
> +++ b/xen/include/public/sysctl.h
> @@ -478,22 +478,19 @@ struct xen_get_cpufreq_para {
> uint32_t cpuinfo_cur_freq;
> uint32_t cpuinfo_max_freq;
> uint32_t cpuinfo_min_freq;
> - union {
> - struct {
> - uint32_t scaling_cur_freq;
> -
> - char scaling_governor[CPUFREQ_NAME_LEN];
> - uint32_t scaling_max_freq;
> - uint32_t scaling_min_freq;
> -
> - /* for specific governor */
> - union {
> - struct xen_userspace userspace;
> - struct xen_ondemand ondemand;
> - } u;
> - } s;
> - struct xen_get_cppc_para cppc_para;
> - } u;
> + struct {
> + uint32_t scaling_cur_freq;
> +
> + char scaling_governor[CPUFREQ_NAME_LEN];
> + uint32_t scaling_max_freq;
> + uint32_t scaling_min_freq;
> +
> + /* for specific governor */
> + union {
> + struct xen_userspace userspace;
> + struct xen_ondemand ondemand;
> + } u;
> + } s;
... I don't quite see why we'd need to retain the nested struct now
either. Imo we ought to be cleaning this up for 4.22.
Jan
© 2016 - 2025 Red Hat, Inc.