x86: Merge xc_cpu_policy's cpuid and msr objects

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.  Take the opportunity to improve the code clarity
by passing a cpu_policy rather than an xc_cpu_policy into some functions.

No practical change.  This undoes the transient doubling of storage space from
earlier patches.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit c9985233ca663fea20fc8807cf509d2e3fef0dca)

diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c
index 8d5c68c96726dc3707137b29b5cc9d007126ba79..c1e33613cf186fa0737150facd1dcb4a0cbbb581 100644 (file)
--- a/tools/libs/guest/xg_cpuid_x86.c
+++ b/tools/libs/guest/xg_cpuid_x86.c
@@ -432,7 +432,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);
@@ -693,7 +693,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 )
     {
@@ -703,7 +703,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 )
     {
@@ -720,18 +720,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;
@@ -745,18 +745,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;
@@ -771,16 +771,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 )
     {
@@ -803,7 +803,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");
@@ -814,7 +814,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");
@@ -832,7 +832,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 )
@@ -851,7 +851,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 )
     {
@@ -869,8 +869,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 28441ee13ff77027b2d19d3a5bdaa8d4ec697533..d01e49df80ef092c25fd35776554ceedca4d6d7e 100644 (file)
--- a/tools/libs/guest/xg_private.h
+++ b/tools/libs/guest/xg_private.h
@@ -172,10 +172,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 f11e8c54e0def121f008aa85bc000eddad332e29..0f4ea5f9c462ddcd641f28f317003f11428c5bda 100644 (file)
--- 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");
@@ -293,57 +293,52 @@ static void test_rtm_behaviour(void)
     }
 }
 
-static void dump_tsx_details(const struct xc_cpu_policy *p, const char *pref)
+static void dump_tsx_details(const struct 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->feat.rtm,
+           p->feat.hle,
+           p->feat.tsx_force_abort,
+           p->feat.rtm_always_abort,
+           p->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)
+static void test_guest_policies(const struct cpu_policy *max,
+                                const struct 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;
-
     dump_tsx_details(max, "Max:");
     dump_tsx_details(def, "Def:");
 
-    if ( ((cm->feat.raw[0].d | cd->feat.raw[0].d) &
+    if ( ((max->feat.raw[0].d | def->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) )
+         ((max->arch_caps.raw | def->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 ( (max->feat.raw[0].b | def->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 ( def->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 ( !max->feat.rtm || !def->feat.rtm )
              fail("  RTM not offered to guests despite being available\n");
         break;
     }
 
-    if ( cd->feat.hle )
+    if ( def->feat.hle )
         fail("  Fail: HLE offered in default policy\n");
 }
 
@@ -352,13 +347,13 @@ static void test_def_max_policies(void)
     if ( xen_has_pv )
     {
         printf("Testing PV default/max policies\n");
-        test_guest_policies(&pv_max, &pv_default);
+        test_guest_policies(&pv_max.policy, &pv_default.policy);
     }
 
     if ( xen_has_hvm )
     {
         printf("Testing HVM default/max policies\n");
-        test_guest_policies(&hvm_max, &hvm_default);
+        test_guest_policies(&hvm_max.policy, &hvm_default.policy);
     }
 }
 
@@ -382,23 +377,23 @@ static void test_guest(struct xen_domctl_createdomain *c)
         goto out;
     }
 
-    dump_tsx_details(&guest_policy, "Cur:");
+    dump_tsx_details(&guest_policy.policy, "Cur:");
 
     /*
      * 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 +401,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. */
@@ -427,12 +422,12 @@ static void test_guest(struct xen_domctl_createdomain *c)
             goto out;
         }
 
-        dump_tsx_details(&guest_policy, "Cur:");
+        dump_tsx_details(&guest_policy.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;
         }
     }