diff mbox series

[v8,04/15] xen/sysctl: Nest cpufreq scaling options

Message ID 20230807185119.98333-5-jandryuk@gmail.com (mailing list archive)
State New, archived
Headers show
Series Intel Hardware P-States (HWP) support | expand

Commit Message

Jason Andryuk Aug. 7, 2023, 6:51 p.m. UTC 
Add a union and struct so that most of the scaling variables of struct
xen_get_cpufreq_para are within in a binary-compatible layout.  This
allows cppc_para to live in the larger union and use uint32_ts - struct
xen_cppc_para will be 10 uint32_t's.

The new scaling struct is 3 * uint32_t + 16 bytes CPUFREQ_NAME_LEN + 4 *
uint32_t for xen_ondemand = 11 uint32_t.  That means the old size is
retained, int32_t turbo_enabled doesn't move and it's binary compatible.

The out-of-context memcpy() in xc_get_cpufreq_para() now handles the
copying of the fields removed there.

Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
---
v8:
Add BUILD_BUG_ON checks for structs xc_get_cpufreq_para & xen_get_cpufreq_para

v6:
Add Jan's Reviewed-by

v5:
Expand commit message
Change comment to driver/governor
---
 tools/include/xenctrl.h     | 22 ++++++++++--------
 tools/libs/ctrl/xc_pm.c     | 46 ++++++++++++++++++++++++++++++++-----
 tools/misc/xenpm.c          | 24 +++++++++----------
 xen/drivers/acpi/pmstat.c   | 27 +++++++++++-----------
 xen/include/public/sysctl.h | 22 ++++++++++--------
 5 files changed, 92 insertions(+), 49 deletions(-)

Comments

Jan Beulich Aug. 8, 2023, 7:05 a.m. UTC | #1 
On 07.08.2023 20:51, Jason Andryuk wrote:
> Add a union and struct so that most of the scaling variables of struct
> xen_get_cpufreq_para are within in a binary-compatible layout.  This
> allows cppc_para to live in the larger union and use uint32_ts - struct
> xen_cppc_para will be 10 uint32_t's.
> 
> The new scaling struct is 3 * uint32_t + 16 bytes CPUFREQ_NAME_LEN + 4 *
> uint32_t for xen_ondemand = 11 uint32_t.  That means the old size is
> retained, int32_t turbo_enabled doesn't move and it's binary compatible.
> 
> The out-of-context memcpy() in xc_get_cpufreq_para() now handles the
> copying of the fields removed there.
> 
> Signed-off-by: Jason Andryuk <jandryuk@gmail.com>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> ---
> v8:
> Add BUILD_BUG_ON checks for structs xc_get_cpufreq_para & xen_get_cpufreq_para
> 
> v6:
> Add Jan's Reviewed-by
> 
> v5:
> Expand commit message
> Change comment to driver/governor
> ---
>  tools/include/xenctrl.h     | 22 ++++++++++--------
>  tools/libs/ctrl/xc_pm.c     | 46 ++++++++++++++++++++++++++++++++-----
>  tools/misc/xenpm.c          | 24 +++++++++----------
>  xen/drivers/acpi/pmstat.c   | 27 +++++++++++-----------
>  xen/include/public/sysctl.h | 22 ++++++++++--------
>  5 files changed, 92 insertions(+), 49 deletions(-)

Since the note in the cover letter may not be noticed - this wants looking at by
a tool stack person.

Thanks, Jan
Anthony PERARD Aug. 8, 2023, 8:52 a.m. UTC | #2 
On Mon, Aug 07, 2023 at 02:51:08PM -0400, Jason Andryuk wrote:
> diff --git a/tools/libs/ctrl/xc_pm.c b/tools/libs/ctrl/xc_pm.c
> index c3a9864bf7..5ec050982a 100644
> --- a/tools/libs/ctrl/xc_pm.c
> +++ b/tools/libs/ctrl/xc_pm.c
> @@ -245,6 +245,45 @@ int xc_get_cpufreq_para(xc_interface *xch, int cpuid,
>      sys_para->freq_num = user_para->freq_num;
>      sys_para->gov_num  = user_para->gov_num;
>  
> +    /* Sanity check struct layout */
> +    BUILD_BUG_ON(sizeof(*user_para) != sizeof(*sys_para));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), cpu_num) !=
> +                 offsetof(typeof(*sys_para),  cpu_num));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), freq_num) !=
> +                 offsetof(typeof(*sys_para),  freq_num));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), gov_num) !=
> +                 offsetof(typeof(*sys_para),  gov_num));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), affected_cpus) !=
> +                 offsetof(typeof(*sys_para),  affected_cpus));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), scaling_available_frequencies) !=
> +                 offsetof(typeof(*sys_para),  scaling_available_frequencies));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), scaling_available_governors) !=
> +                 offsetof(typeof(*sys_para),  scaling_available_governors));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), scaling_driver) !=
> +                 offsetof(typeof(*sys_para),  scaling_driver));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), cpuinfo_cur_freq) !=
> +                 offsetof(typeof(*sys_para),  cpuinfo_cur_freq));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), cpuinfo_max_freq) !=
> +                 offsetof(typeof(*sys_para),  cpuinfo_max_freq));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), cpuinfo_min_freq) !=
> +                 offsetof(typeof(*sys_para),  cpuinfo_min_freq));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.scaling_cur_freq) !=
> +                 offsetof(typeof(*sys_para),  u.s.scaling_cur_freq));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.scaling_governor) !=
> +                 offsetof(typeof(*sys_para),  u.s.scaling_governor));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.scaling_max_freq) !=
> +                 offsetof(typeof(*sys_para),  u.s.scaling_max_freq));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.scaling_min_freq) !=
> +                 offsetof(typeof(*sys_para),  u.s.scaling_min_freq));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.u.userspace) !=
> +                 offsetof(typeof(*sys_para),  u.s.u.userspace));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.u.ondemand) !=
> +                 offsetof(typeof(*sys_para),  u.s.u.ondemand));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), u.cppc_para) !=
> +                 offsetof(typeof(*sys_para),  u.cppc_para));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), turbo_enabled) !=
> +                 offsetof(typeof(*sys_para),  turbo_enabled));

These could have been done by defining a temporary macro to avoid
repeating the fields name twice, but this sanity check is good and
should prevent anyone from changing one struct without changing the
other one.

Thanks.

Reviewed-by: Anthony PERARD <anthony.perard@citrix.com>

Cheers,
Jan Beulich Aug. 23, 2023, 1:16 p.m. UTC | #3 
On 07.08.2023 20:51, Jason Andryuk wrote:
> --- a/tools/libs/ctrl/xc_pm.c
> +++ b/tools/libs/ctrl/xc_pm.c
> @@ -245,6 +245,45 @@ int xc_get_cpufreq_para(xc_interface *xch, int cpuid,
>      sys_para->freq_num = user_para->freq_num;
>      sys_para->gov_num  = user_para->gov_num;
>  
> +    /* Sanity check struct layout */
> +    BUILD_BUG_ON(sizeof(*user_para) != sizeof(*sys_para));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), cpu_num) !=
> +                 offsetof(typeof(*sys_para),  cpu_num));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), freq_num) !=
> +                 offsetof(typeof(*sys_para),  freq_num));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), gov_num) !=
> +                 offsetof(typeof(*sys_para),  gov_num));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), affected_cpus) !=
> +                 offsetof(typeof(*sys_para),  affected_cpus));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), scaling_available_frequencies) !=
> +                 offsetof(typeof(*sys_para),  scaling_available_frequencies));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), scaling_available_governors) !=
> +                 offsetof(typeof(*sys_para),  scaling_available_governors));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), scaling_driver) !=
> +                 offsetof(typeof(*sys_para),  scaling_driver));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), cpuinfo_cur_freq) !=
> +                 offsetof(typeof(*sys_para),  cpuinfo_cur_freq));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), cpuinfo_max_freq) !=
> +                 offsetof(typeof(*sys_para),  cpuinfo_max_freq));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), cpuinfo_min_freq) !=
> +                 offsetof(typeof(*sys_para),  cpuinfo_min_freq));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.scaling_cur_freq) !=
> +                 offsetof(typeof(*sys_para),  u.s.scaling_cur_freq));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.scaling_governor) !=
> +                 offsetof(typeof(*sys_para),  u.s.scaling_governor));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.scaling_max_freq) !=
> +                 offsetof(typeof(*sys_para),  u.s.scaling_max_freq));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.scaling_min_freq) !=
> +                 offsetof(typeof(*sys_para),  u.s.scaling_min_freq));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.u.userspace) !=
> +                 offsetof(typeof(*sys_para),  u.s.u.userspace));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.u.ondemand) !=
> +                 offsetof(typeof(*sys_para),  u.s.u.ondemand));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), u.cppc_para) !=
> +                 offsetof(typeof(*sys_para),  u.cppc_para));
> +    BUILD_BUG_ON(offsetof(typeof(*user_para), turbo_enabled) !=
> +                 offsetof(typeof(*sys_para),  turbo_enabled));

As the build breakage shows, these checks are too aggressive. The two
layouts can't possibly be the same with 32-bit tool stacks (and hence
pointers being 32 bits wide). But we also don't need all these checks;
what may need checking is only what subsequently is subject to
memcpy(), not anything that's dealt with by field-wise copying.

Jan
diff mbox series

Patch

diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index faec1dd824..de03cfb117 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -1926,16 +1926,20 @@  struct xc_get_cpufreq_para {
     uint32_t cpuinfo_cur_freq;
     uint32_t cpuinfo_max_freq;
     uint32_t cpuinfo_min_freq;
-    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 {
-        xc_userspace_t userspace;
-        xc_ondemand_t ondemand;
+        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 {
+                xc_userspace_t userspace;
+                xc_ondemand_t ondemand;
+            } u;
+        } s;
     } u;
 
     int32_t turbo_enabled;
diff --git a/tools/libs/ctrl/xc_pm.c b/tools/libs/ctrl/xc_pm.c
index c3a9864bf7..5ec050982a 100644
--- a/tools/libs/ctrl/xc_pm.c
+++ b/tools/libs/ctrl/xc_pm.c
@@ -245,6 +245,45 @@  int xc_get_cpufreq_para(xc_interface *xch, int cpuid,
     sys_para->freq_num = user_para->freq_num;
     sys_para->gov_num  = user_para->gov_num;
 
+    /* Sanity check struct layout */
+    BUILD_BUG_ON(sizeof(*user_para) != sizeof(*sys_para));
+    BUILD_BUG_ON(offsetof(typeof(*user_para), cpu_num) !=
+                 offsetof(typeof(*sys_para),  cpu_num));
+    BUILD_BUG_ON(offsetof(typeof(*user_para), freq_num) !=
+                 offsetof(typeof(*sys_para),  freq_num));
+    BUILD_BUG_ON(offsetof(typeof(*user_para), gov_num) !=
+                 offsetof(typeof(*sys_para),  gov_num));
+    BUILD_BUG_ON(offsetof(typeof(*user_para), affected_cpus) !=
+                 offsetof(typeof(*sys_para),  affected_cpus));
+    BUILD_BUG_ON(offsetof(typeof(*user_para), scaling_available_frequencies) !=
+                 offsetof(typeof(*sys_para),  scaling_available_frequencies));
+    BUILD_BUG_ON(offsetof(typeof(*user_para), scaling_available_governors) !=
+                 offsetof(typeof(*sys_para),  scaling_available_governors));
+    BUILD_BUG_ON(offsetof(typeof(*user_para), scaling_driver) !=
+                 offsetof(typeof(*sys_para),  scaling_driver));
+    BUILD_BUG_ON(offsetof(typeof(*user_para), cpuinfo_cur_freq) !=
+                 offsetof(typeof(*sys_para),  cpuinfo_cur_freq));
+    BUILD_BUG_ON(offsetof(typeof(*user_para), cpuinfo_max_freq) !=
+                 offsetof(typeof(*sys_para),  cpuinfo_max_freq));
+    BUILD_BUG_ON(offsetof(typeof(*user_para), cpuinfo_min_freq) !=
+                 offsetof(typeof(*sys_para),  cpuinfo_min_freq));
+    BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.scaling_cur_freq) !=
+                 offsetof(typeof(*sys_para),  u.s.scaling_cur_freq));
+    BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.scaling_governor) !=
+                 offsetof(typeof(*sys_para),  u.s.scaling_governor));
+    BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.scaling_max_freq) !=
+                 offsetof(typeof(*sys_para),  u.s.scaling_max_freq));
+    BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.scaling_min_freq) !=
+                 offsetof(typeof(*sys_para),  u.s.scaling_min_freq));
+    BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.u.userspace) !=
+                 offsetof(typeof(*sys_para),  u.s.u.userspace));
+    BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.u.ondemand) !=
+                 offsetof(typeof(*sys_para),  u.s.u.ondemand));
+    BUILD_BUG_ON(offsetof(typeof(*user_para), u.cppc_para) !=
+                 offsetof(typeof(*sys_para),  u.cppc_para));
+    BUILD_BUG_ON(offsetof(typeof(*user_para), turbo_enabled) !=
+                 offsetof(typeof(*sys_para),  turbo_enabled));
+
     ret = xc_sysctl(xch, &sysctl);
     if ( ret )
     {
@@ -265,17 +304,12 @@  int xc_get_cpufreq_para(xc_interface *xch, int cpuid,
         user_para->cpuinfo_cur_freq = sys_para->cpuinfo_cur_freq;
         user_para->cpuinfo_max_freq = sys_para->cpuinfo_max_freq;
         user_para->cpuinfo_min_freq = sys_para->cpuinfo_min_freq;
-        user_para->scaling_cur_freq = sys_para->scaling_cur_freq;
-        user_para->scaling_max_freq = sys_para->scaling_max_freq;
-        user_para->scaling_min_freq = sys_para->scaling_min_freq;
         user_para->turbo_enabled    = sys_para->turbo_enabled;
 
         memcpy(user_para->scaling_driver,
                 sys_para->scaling_driver, CPUFREQ_NAME_LEN);
-        memcpy(user_para->scaling_governor,
-                sys_para->scaling_governor, CPUFREQ_NAME_LEN);
 
-        /* copy to user_para no matter what cpufreq governor */
+        /* copy to user_para no matter what cpufreq driver/governor */
         BUILD_BUG_ON(sizeof(((struct xc_get_cpufreq_para *)0)->u) !=
 		     sizeof(((struct xen_get_cpufreq_para *)0)->u));
 
diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c
index 1bb6187e56..ee8ce5d5f2 100644
--- a/tools/misc/xenpm.c
+++ b/tools/misc/xenpm.c
@@ -730,39 +730,39 @@  static void print_cpufreq_para(int cpuid, struct xc_get_cpufreq_para *p_cpufreq)
     printf("scaling_avail_gov    : %s\n",
            p_cpufreq->scaling_available_governors);
 
-    printf("current_governor     : %s\n", p_cpufreq->scaling_governor);
-    if ( !strncmp(p_cpufreq->scaling_governor,
+    printf("current_governor     : %s\n", p_cpufreq->u.s.scaling_governor);
+    if ( !strncmp(p_cpufreq->u.s.scaling_governor,
                   "userspace", CPUFREQ_NAME_LEN) )
     {
         printf("  userspace specific :\n");
         printf("    scaling_setspeed : %u\n",
-               p_cpufreq->u.userspace.scaling_setspeed);
+               p_cpufreq->u.s.u.userspace.scaling_setspeed);
     }
-    else if ( !strncmp(p_cpufreq->scaling_governor,
+    else if ( !strncmp(p_cpufreq->u.s.scaling_governor,
                        "ondemand", CPUFREQ_NAME_LEN) )
     {
         printf("  ondemand specific  :\n");
         printf("    sampling_rate    : max [%u] min [%u] cur [%u]\n",
-               p_cpufreq->u.ondemand.sampling_rate_max,
-               p_cpufreq->u.ondemand.sampling_rate_min,
-               p_cpufreq->u.ondemand.sampling_rate);
+               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);
         printf("    up_threshold     : %u\n",
-               p_cpufreq->u.ondemand.up_threshold);
+               p_cpufreq->u.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->scaling_cur_freq )
+             p_cpufreq->u.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->scaling_max_freq,
-           p_cpufreq->scaling_min_freq,
-           p_cpufreq->scaling_cur_freq);
+           p_cpufreq->u.s.scaling_max_freq,
+           p_cpufreq->u.s.scaling_min_freq,
+           p_cpufreq->u.s.scaling_cur_freq);
 
     printf("turbo mode           : %s\n",
            p_cpufreq->turbo_enabled ? "enabled" : "disabled or n/a");
diff --git a/xen/drivers/acpi/pmstat.c b/xen/drivers/acpi/pmstat.c
index 1bae635101..f5a9ac3f1a 100644
--- a/xen/drivers/acpi/pmstat.c
+++ b/xen/drivers/acpi/pmstat.c
@@ -258,37 +258,38 @@  static int get_cpufreq_para(struct xen_sysctl_pm_op *op)
         cpufreq_driver.get ? cpufreq_driver.get(op->cpuid) : policy->cur;
     op->u.get_para.cpuinfo_max_freq = policy->cpuinfo.max_freq;
     op->u.get_para.cpuinfo_min_freq = policy->cpuinfo.min_freq;
-    op->u.get_para.scaling_cur_freq = policy->cur;
-    op->u.get_para.scaling_max_freq = policy->max;
-    op->u.get_para.scaling_min_freq = policy->min;
+
+    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;
 
     if ( cpufreq_driver.name[0] )
-        strlcpy(op->u.get_para.scaling_driver, 
+        strlcpy(op->u.get_para.scaling_driver,
             cpufreq_driver.name, CPUFREQ_NAME_LEN);
     else
         strlcpy(op->u.get_para.scaling_driver, "Unknown", CPUFREQ_NAME_LEN);
 
     if ( policy->governor->name[0] )
-        strlcpy(op->u.get_para.scaling_governor, 
+        strlcpy(op->u.get_para.u.s.scaling_governor,
             policy->governor->name, CPUFREQ_NAME_LEN);
     else
-        strlcpy(op->u.get_para.scaling_governor, "Unknown", CPUFREQ_NAME_LEN);
+        strlcpy(op->u.get_para.u.s.scaling_governor, "Unknown", CPUFREQ_NAME_LEN);
 
     /* governor specific para */
-    if ( !strncasecmp(op->u.get_para.scaling_governor,
+    if ( !strncasecmp(op->u.get_para.u.s.scaling_governor,
                       "userspace", CPUFREQ_NAME_LEN) )
     {
-        op->u.get_para.u.userspace.scaling_setspeed = policy->cur;
+        op->u.get_para.u.s.u.userspace.scaling_setspeed = policy->cur;
     }
 
-    if ( !strncasecmp(op->u.get_para.scaling_governor,
+    if ( !strncasecmp(op->u.get_para.u.s.scaling_governor,
                       "ondemand", CPUFREQ_NAME_LEN) )
     {
         ret = get_cpufreq_ondemand_para(
-            &op->u.get_para.u.ondemand.sampling_rate_max,
-            &op->u.get_para.u.ondemand.sampling_rate_min,
-            &op->u.get_para.u.ondemand.sampling_rate,
-            &op->u.get_para.u.ondemand.up_threshold);
+            &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.turbo_enabled = cpufreq_get_turbo_status(op->cpuid);
 
diff --git a/xen/include/public/sysctl.h b/xen/include/public/sysctl.h
index fa7147de47..c11c0b1a6c 100644
--- a/xen/include/public/sysctl.h
+++ b/xen/include/public/sysctl.h
@@ -317,16 +317,20 @@  struct xen_get_cpufreq_para {
     uint32_t cpuinfo_cur_freq;
     uint32_t cpuinfo_max_freq;
     uint32_t cpuinfo_min_freq;
-    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;
+        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;
     } u;
 
     int32_t turbo_enabled;