[PATCH 7/9] x86: Merge xc_cpu_policy's cpuid and msr objects

From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Jan Beulich" <JBeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>
Subject: [PATCH 7/9] x86: Merge xc_cpu_policy's cpuid and msr objects
Date: Wed, 29 Mar 2023 21:51:35 +0100	[thread overview]
Message-ID: <20230329205137.323253-8-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <20230329205137.323253-1-andrew.cooper3@citrix.com>

Right now, they're the same underlying type, containing disjoint information.

Use a single object instead.  Also take the opportunity to rename 'entries' to
'msrs' which is more descriptive, and more in line with nr_msrs being the
count of MSR entries in the API.

test-tsx uses xg_private.h to access the internals of xc_cpu_policy, so needs
updating at the same time.

No practical change, but it does reduce the size of struct xc_cpu_policy by
~2k.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Wei Liu <wl@xen.org>
---
 tools/libs/guest/xg_cpuid_x86.c | 36 ++++++++++----------
 tools/libs/guest/xg_private.h   |  5 ++-
 tools/tests/tsx/test-tsx.c      | 58 ++++++++++++++++-----------------
 3 files changed, 48 insertions(+), 51 deletions(-)

diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c
index 5fae06e77804..5061fe357767 100644
--- a/tools/libs/guest/xg_cpuid_x86.c
+++ b/tools/libs/guest/xg_cpuid_x86.c
@@ -431,7 +431,7 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore,
     xc_dominfo_t di;
     unsigned int i, nr_leaves, nr_msrs;
     xen_cpuid_leaf_t *leaves = NULL;
-    struct cpuid_policy *p = NULL;
+    struct cpu_policy *p = NULL;
     uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1;
     uint32_t host_featureset[FEATURESET_NR_ENTRIES] = {};
     uint32_t len = ARRAY_SIZE(host_featureset);
@@ -692,7 +692,7 @@ static int deserialize_policy(xc_interface *xch, xc_cpu_policy_t *policy,
     uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1;
     int rc;
 
-    rc = x86_cpuid_copy_from_buffer(&policy->cpuid, policy->leaves,
+    rc = x86_cpuid_copy_from_buffer(&policy->policy, policy->leaves,
                                     nr_leaves, &err_leaf, &err_subleaf);
     if ( rc )
     {
@@ -702,7 +702,7 @@ static int deserialize_policy(xc_interface *xch, xc_cpu_policy_t *policy,
         return rc;
     }
 
-    rc = x86_msr_copy_from_buffer(&policy->msr, policy->entries,
+    rc = x86_msr_copy_from_buffer(&policy->policy, policy->msrs,
                                   nr_entries, &err_msr);
     if ( rc )
     {
@@ -719,18 +719,18 @@ int xc_cpu_policy_get_system(xc_interface *xch, unsigned int policy_idx,
                              xc_cpu_policy_t *policy)
 {
     unsigned int nr_leaves = ARRAY_SIZE(policy->leaves);
-    unsigned int nr_entries = ARRAY_SIZE(policy->entries);
+    unsigned int nr_msrs = ARRAY_SIZE(policy->msrs);
     int rc;
 
     rc = get_system_cpu_policy(xch, policy_idx, &nr_leaves, policy->leaves,
-                               &nr_entries, policy->entries);
+                               &nr_msrs, policy->msrs);
     if ( rc )
     {
         PERROR("Failed to obtain %u policy", policy_idx);
         return rc;
     }
 
-    rc = deserialize_policy(xch, policy, nr_leaves, nr_entries);
+    rc = deserialize_policy(xch, policy, nr_leaves, nr_msrs);
     if ( rc )
     {
         errno = -rc;
@@ -744,18 +744,18 @@ int xc_cpu_policy_get_domain(xc_interface *xch, uint32_t domid,
                              xc_cpu_policy_t *policy)
 {
     unsigned int nr_leaves = ARRAY_SIZE(policy->leaves);
-    unsigned int nr_entries = ARRAY_SIZE(policy->entries);
+    unsigned int nr_msrs = ARRAY_SIZE(policy->msrs);
     int rc;
 
     rc = get_domain_cpu_policy(xch, domid, &nr_leaves, policy->leaves,
-                               &nr_entries, policy->entries);
+                               &nr_msrs, policy->msrs);
     if ( rc )
     {
         PERROR("Failed to obtain domain %u policy", domid);
         return rc;
     }
 
-    rc = deserialize_policy(xch, policy, nr_leaves, nr_entries);
+    rc = deserialize_policy(xch, policy, nr_leaves, nr_msrs);
     if ( rc )
     {
         errno = -rc;
@@ -770,16 +770,16 @@ int xc_cpu_policy_set_domain(xc_interface *xch, uint32_t domid,
 {
     uint32_t err_leaf = -1, err_subleaf = -1, err_msr = -1;
     unsigned int nr_leaves = ARRAY_SIZE(policy->leaves);
-    unsigned int nr_entries = ARRAY_SIZE(policy->entries);
+    unsigned int nr_msrs = ARRAY_SIZE(policy->msrs);
     int rc;
 
     rc = xc_cpu_policy_serialise(xch, policy, policy->leaves, &nr_leaves,
-                                 policy->entries, &nr_entries);
+                                 policy->msrs, &nr_msrs);
     if ( rc )
         return rc;
 
     rc = xc_set_domain_cpu_policy(xch, domid, nr_leaves, policy->leaves,
-                                  nr_entries, policy->entries,
+                                  nr_msrs, policy->msrs,
                                   &err_leaf, &err_subleaf, &err_msr);
     if ( rc )
     {
@@ -802,7 +802,7 @@ int xc_cpu_policy_serialise(xc_interface *xch, const xc_cpu_policy_t *p,
 
     if ( leaves )
     {
-        rc = x86_cpuid_copy_to_buffer(&p->cpuid, leaves, nr_leaves);
+        rc = x86_cpuid_copy_to_buffer(&p->policy, leaves, nr_leaves);
         if ( rc )
         {
             ERROR("Failed to serialize CPUID policy");
@@ -813,7 +813,7 @@ int xc_cpu_policy_serialise(xc_interface *xch, const xc_cpu_policy_t *p,
 
     if ( msrs )
     {
-        rc = x86_msr_copy_to_buffer(&p->msr, msrs, nr_msrs);
+        rc = x86_msr_copy_to_buffer(&p->policy, msrs, nr_msrs);
         if ( rc )
         {
             ERROR("Failed to serialize MSR policy");
@@ -831,7 +831,7 @@ int xc_cpu_policy_update_cpuid(xc_interface *xch, xc_cpu_policy_t *policy,
                                uint32_t nr)
 {
     unsigned int err_leaf = -1, err_subleaf = -1;
-    int rc = x86_cpuid_copy_from_buffer(&policy->cpuid, leaves, nr,
+    int rc = x86_cpuid_copy_from_buffer(&policy->policy, leaves, nr,
                                         &err_leaf, &err_subleaf);
 
     if ( rc )
@@ -850,7 +850,7 @@ int xc_cpu_policy_update_msrs(xc_interface *xch, xc_cpu_policy_t *policy,
                               const xen_msr_entry_t *msrs, uint32_t nr)
 {
     unsigned int err_msr = -1;
-    int rc = x86_msr_copy_from_buffer(&policy->msr, msrs, nr, &err_msr);
+    int rc = x86_msr_copy_from_buffer(&policy->policy, msrs, nr, &err_msr);
 
     if ( rc )
     {
@@ -868,8 +868,8 @@ bool xc_cpu_policy_is_compatible(xc_interface *xch, xc_cpu_policy_t *host,
                                  xc_cpu_policy_t *guest)
 {
     struct cpu_policy_errors err = INIT_CPU_POLICY_ERRORS;
-    struct old_cpu_policy h = { &host->cpuid, &host->msr };
-    struct old_cpu_policy g = { &guest->cpuid, &guest->msr };
+    struct old_cpu_policy h = { &host->policy, &host->policy };
+    struct old_cpu_policy g = { &guest->policy, &guest->policy };
     int rc = x86_cpu_policies_are_compatible(&h, &g, &err);
 
     if ( !rc )
diff --git a/tools/libs/guest/xg_private.h b/tools/libs/guest/xg_private.h
index 09e24f122760..e729a8106c3e 100644
--- a/tools/libs/guest/xg_private.h
+++ b/tools/libs/guest/xg_private.h
@@ -173,10 +173,9 @@ int pin_table(xc_interface *xch, unsigned int type, unsigned long mfn,
 #include <xen/lib/x86/cpu-policy.h>
 
 struct xc_cpu_policy {
-    struct cpuid_policy cpuid;
-    struct msr_policy msr;
+    struct cpu_policy policy;
     xen_cpuid_leaf_t leaves[CPUID_MAX_SERIALISED_LEAVES];
-    xen_msr_entry_t entries[MSR_MAX_SERIALISED_ENTRIES];
+    xen_msr_entry_t msrs[MSR_MAX_SERIALISED_ENTRIES];
 };
 #endif /* x86 */
 
diff --git a/tools/tests/tsx/test-tsx.c b/tools/tests/tsx/test-tsx.c
index d6d98c299bf9..6164cd86c466 100644
--- a/tools/tests/tsx/test-tsx.c
+++ b/tools/tests/tsx/test-tsx.c
@@ -151,15 +151,15 @@ static void test_tsx_msrs(void)
 {
     printf("Testing MSR_TSX_FORCE_ABORT consistency\n");
     test_tsx_msr_consistency(
-        MSR_TSX_FORCE_ABORT, host.cpuid.feat.tsx_force_abort);
+        MSR_TSX_FORCE_ABORT, host.policy.feat.tsx_force_abort);
 
     printf("Testing MSR_TSX_CTRL consistency\n");
     test_tsx_msr_consistency(
-        MSR_TSX_CTRL, host.msr.arch_caps.tsx_ctrl);
+        MSR_TSX_CTRL, host.policy.arch_caps.tsx_ctrl);
 
     printf("Testing MSR_MCU_OPT_CTRL consistency\n");
     test_tsx_msr_consistency(
-        MSR_MCU_OPT_CTRL, host.cpuid.feat.srbds_ctrl);
+        MSR_MCU_OPT_CTRL, host.policy.feat.srbds_ctrl);
 }
 
 /*
@@ -281,7 +281,7 @@ static void test_rtm_behaviour(void)
     else
         return fail("  Got unexpected behaviour %d\n", rtm_behaviour);
 
-    if ( host.cpuid.feat.rtm )
+    if ( host.policy.feat.rtm )
     {
         if ( rtm_behaviour == RTM_UD )
             fail("  Host reports RTM, but appears unavailable\n");
@@ -297,53 +297,51 @@ static void dump_tsx_details(const struct xc_cpu_policy *p, const char *pref)
 {
     printf("  %s RTM %u, HLE %u, TSX_FORCE_ABORT %u, RTM_ALWAYS_ABORT %u, TSX_CTRL %u\n",
            pref,
-           p->cpuid.feat.rtm,
-           p->cpuid.feat.hle,
-           p->cpuid.feat.tsx_force_abort,
-           p->cpuid.feat.rtm_always_abort,
-           p->msr.arch_caps.tsx_ctrl);
+           p->policy.feat.rtm,
+           p->policy.feat.hle,
+           p->policy.feat.tsx_force_abort,
+           p->policy.feat.rtm_always_abort,
+           p->policy.arch_caps.tsx_ctrl);
 }
 
 /* Sanity test various invariants we expect in the default/max policies. */
 static void test_guest_policies(const struct xc_cpu_policy *max,
                                 const struct xc_cpu_policy *def)
 {
-    const struct cpuid_policy *cm = &max->cpuid;
-    const struct cpuid_policy *cd = &def->cpuid;
-    const struct msr_policy *mm = &max->msr;
-    const struct msr_policy *md = &def->msr;
+    const struct cpu_policy *m = &max->policy;
+    const struct cpu_policy *d = &def->policy;
 
     dump_tsx_details(max, "Max:");
     dump_tsx_details(def, "Def:");
 
-    if ( ((cm->feat.raw[0].d | cd->feat.raw[0].d) &
+    if ( ((m->feat.raw[0].d | d->feat.raw[0].d) &
           (bitmaskof(X86_FEATURE_TSX_FORCE_ABORT) |
            bitmaskof(X86_FEATURE_RTM_ALWAYS_ABORT) |
            bitmaskof(X86_FEATURE_SRBDS_CTRL))) ||
-         ((mm->arch_caps.raw | md->arch_caps.raw) & ARCH_CAPS_TSX_CTRL) )
+         ((m->arch_caps.raw | d->arch_caps.raw) & ARCH_CAPS_TSX_CTRL) )
         fail("  Xen-only TSX controls offered to guest\n");
 
     switch ( rtm_behaviour )
     {
     case RTM_UD:
-        if ( (cm->feat.raw[0].b | cd->feat.raw[0].b) &
+        if ( (m->feat.raw[0].b | d->feat.raw[0].b) &
              (bitmaskof(X86_FEATURE_HLE) | bitmaskof(X86_FEATURE_RTM)) )
              fail("  HLE/RTM offered to guests despite not being available\n");
         break;
 
     case RTM_ABORT:
-        if ( cd->feat.raw[0].b &
+        if ( d->feat.raw[0].b &
              (bitmaskof(X86_FEATURE_HLE) | bitmaskof(X86_FEATURE_RTM)) )
              fail("  HLE/RTM offered to guests by default despite not being usable\n");
         break;
 
     case RTM_OK:
-        if ( !cm->feat.rtm || !cd->feat.rtm )
+        if ( !m->feat.rtm || !d->feat.rtm )
              fail("  RTM not offered to guests despite being available\n");
         break;
     }
 
-    if ( cd->feat.hle )
+    if ( d->feat.hle )
         fail("  Fail: HLE offered in default policy\n");
 }
 
@@ -387,18 +385,18 @@ static void test_guest(struct xen_domctl_createdomain *c)
     /*
      * Check defaults given to the guest.
      */
-    if ( guest_policy.cpuid.feat.rtm != (rtm_behaviour == RTM_OK) )
+    if ( guest_policy.policy.feat.rtm != (rtm_behaviour == RTM_OK) )
         fail("  RTM %u in guest, despite rtm behaviour\n",
-             guest_policy.cpuid.feat.rtm);
+             guest_policy.policy.feat.rtm);
 
-    if ( guest_policy.cpuid.feat.hle ||
-         guest_policy.cpuid.feat.tsx_force_abort ||
-         guest_policy.cpuid.feat.rtm_always_abort ||
-         guest_policy.cpuid.feat.srbds_ctrl ||
-         guest_policy.msr.arch_caps.tsx_ctrl )
+    if ( guest_policy.policy.feat.hle ||
+         guest_policy.policy.feat.tsx_force_abort ||
+         guest_policy.policy.feat.rtm_always_abort ||
+         guest_policy.policy.feat.srbds_ctrl ||
+         guest_policy.policy.arch_caps.tsx_ctrl )
         fail("  Unexpected features advertised\n");
 
-    if ( host.cpuid.feat.rtm )
+    if ( host.policy.feat.rtm )
     {
         unsigned int _7b0;
 
@@ -406,7 +404,7 @@ static void test_guest(struct xen_domctl_createdomain *c)
          * If host RTM is available, all combinations of guest flags should be
          * possible.  Flip both HLE/RTM to check non-default settings.
          */
-        _7b0 = (guest_policy.cpuid.feat.raw[0].b ^=
+        _7b0 = (guest_policy.policy.feat.raw[0].b ^=
                 (bitmaskof(X86_FEATURE_HLE) | bitmaskof(X86_FEATURE_RTM)));
 
         /* Set the new policy. */
@@ -429,10 +427,10 @@ static void test_guest(struct xen_domctl_createdomain *c)
 
         dump_tsx_details(&guest_policy, "Cur:");
 
-        if ( guest_policy.cpuid.feat.raw[0].b != _7b0 )
+        if ( guest_policy.policy.feat.raw[0].b != _7b0 )
         {
             fail("  Expected CPUID.7[1].b 0x%08x differs from actual 0x%08x\n",
-                 _7b0, guest_policy.cpuid.feat.raw[0].b);
+                 _7b0, guest_policy.policy.feat.raw[0].b);
             goto out;
         }
     }
-- 
2.30.2


