Older Xen versions used to expose some CPUID bits which are no longer
exposed by default. In order to keep a compatible behavior with
guests migrated from versions of Xen that don't encode the CPUID data
on the migration stream introduce a function that sets the same bits
as older Xen versions.

This is pulled out from xc_cpuid_apply_policy which already has this
logic present.

No functional change intended.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v1:
 - Move comments and explicitly mention pre-4.14 Xen.
---
 tools/include/xenctrl.h         |  4 +++
 tools/libs/guest/xg_cpuid_x86.c | 58 ++++++++++++++++++++++++---------
 2 files changed, 47 insertions(+), 15 deletions(-)

diff --git a/tools/include/xenctrl.h b/tools/include/xenctrl.h
index c41d794683c..89a73fd6823 100644
--- a/tools/include/xenctrl.h
+++ b/tools/include/xenctrl.h
@@ -2627,6 +2627,10 @@ int xc_cpu_policy_calc_compatible(xc_interface *xch,
                                   const xc_cpu_policy_t p2,
                                   xc_cpu_policy_t out);
 
+/* Make a policy compatible with pre-4.14 Xen versions. */
+int xc_cpu_policy_make_compatible(xc_interface *xch, xc_cpu_policy_t policy,
+                                  bool hvm);
+
 int xc_get_cpu_levelling_caps(xc_interface *xch, uint32_t *caps);
 int xc_get_cpu_featureset(xc_interface *xch, uint32_t index,
                           uint32_t *nr_features, uint32_t *featureset);
diff --git a/tools/libs/guest/xg_cpuid_x86.c b/tools/libs/guest/xg_cpuid_x86.c
index be2056469aa..855d252e067 100644
--- a/tools/libs/guest/xg_cpuid_x86.c
+++ b/tools/libs/guest/xg_cpuid_x86.c
@@ -446,6 +446,7 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore,
     unsigned int i, nr_leaves, nr_msrs;
     xen_cpuid_leaf_t *leaves = NULL;
     struct cpuid_policy *p = NULL;
+    struct xc_cpu_policy policy = { };
     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);
@@ -510,21 +511,9 @@ int xc_cpuid_apply_policy(xc_interface *xch, uint32_t domid, bool restore,
 
     if ( restore )
     {
-        /*
-         * Account for feature which have been disabled by default since Xen 4.13,
-         * so migrated-in VM's don't risk seeing features disappearing.
-         */
-        p->basic.rdrand = test_bit(X86_FEATURE_RDRAND, host_featureset);
-
-        if ( di.hvm )
-        {
-            p->feat.mpx = test_bit(X86_FEATURE_MPX, host_featureset);
-        }
-
-        /* Clamp maximum leaves to the ones supported on 4.12. */
-        p->basic.max_leaf = min(p->basic.max_leaf, 0xdu);
-        p->feat.max_subleaf = 0;
-        p->extd.max_leaf = min(p->extd.max_leaf, 0x1cu);
+        policy.cpuid = *p;
+        xc_cpu_policy_make_compatible(xch, &policy, di.hvm);
+        *p = policy.cpuid;
     }
 
     if ( featureset )
@@ -1086,3 +1075,42 @@ int xc_cpu_policy_calc_compatible(xc_interface *xch,
 
     return rc;
 }
+
+int xc_cpu_policy_make_compatible(xc_interface *xch, xc_cpu_policy_t policy,
+                                  bool hvm)
+{
+    xc_cpu_policy_t host;
+    int rc;
+
+    host = xc_cpu_policy_init();
+    if ( !host )
+    {
+        errno = ENOMEM;
+        return -1;
+    }
+
+    rc = xc_cpu_policy_get_system(xch, XEN_SYSCTL_cpu_policy_host, host);
+    if ( rc )
+    {
+        ERROR("Failed to get host policy");
+        goto out;
+    }
+
+    /*
+     * Account for features which have been disabled by default since Xen 4.13,
+     * so migrated-in VM's don't risk seeing features disappearing.
+     */
+    policy->cpuid.basic.rdrand = host->cpuid.basic.rdrand;
+
+    if ( hvm )
+        policy->cpuid.feat.mpx = host->cpuid.feat.mpx;
+
+    /* Clamp maximum leaves to the ones supported on 4.12. */
+    policy->cpuid.basic.max_leaf = min(policy->cpuid.basic.max_leaf, 0xdu);
+    policy->cpuid.feat.max_subleaf = 0;
+    policy->cpuid.extd.max_leaf = min(policy->cpuid.extd.max_leaf, 0x1cu);
+
+ out:
+    xc_cpu_policy_destroy(host);
+    return rc;
+}
-- 
2.31.1

On 30.04.2021 17:52, Roger Pau Monne wrote:
> @@ -1086,3 +1075,42 @@ int xc_cpu_policy_calc_compatible(xc_interface *xch,
>  
>      return rc;
>  }
> +
> +int xc_cpu_policy_make_compatible(xc_interface *xch, xc_cpu_policy_t policy,
> +                                  bool hvm)

I'm concerned of the naming, and in particular the two very different
meanings of "compatible" for xc_cpu_policy_calc_compatible() and this
new one. I'm afraid I don't have a good suggestion though, short of
making the name even longer and inserting "backwards".

Jan

> +{
> +    xc_cpu_policy_t host;
> +    int rc;
> +
> +    host = xc_cpu_policy_init();
> +    if ( !host )
> +    {
> +        errno = ENOMEM;
> +        return -1;
> +    }
> +
> +    rc = xc_cpu_policy_get_system(xch, XEN_SYSCTL_cpu_policy_host, host);
> +    if ( rc )
> +    {
> +        ERROR("Failed to get host policy");
> +        goto out;
> +    }
> +
> +    /*
> +     * Account for features which have been disabled by default since Xen 4.13,
> +     * so migrated-in VM's don't risk seeing features disappearing.
> +     */
> +    policy->cpuid.basic.rdrand = host->cpuid.basic.rdrand;
> +
> +    if ( hvm )
> +        policy->cpuid.feat.mpx = host->cpuid.feat.mpx;
> +
> +    /* Clamp maximum leaves to the ones supported on 4.12. */
> +    policy->cpuid.basic.max_leaf = min(policy->cpuid.basic.max_leaf, 0xdu);
> +    policy->cpuid.feat.max_subleaf = 0;
> +    policy->cpuid.extd.max_leaf = min(policy->cpuid.extd.max_leaf, 0x1cu);
> +
> + out:
> +    xc_cpu_policy_destroy(host);
> +    return rc;
> +}
> 

On Mon, May 03, 2021 at 01:09:41PM +0200, Jan Beulich wrote:
> On 30.04.2021 17:52, Roger Pau Monne wrote:
> > @@ -1086,3 +1075,42 @@ int xc_cpu_policy_calc_compatible(xc_interface *xch,
> >  
> >      return rc;
> >  }
> > +
> > +int xc_cpu_policy_make_compatible(xc_interface *xch, xc_cpu_policy_t policy,
> > +                                  bool hvm)
> 
> I'm concerned of the naming, and in particular the two very different
> meanings of "compatible" for xc_cpu_policy_calc_compatible() and this
> new one. I'm afraid I don't have a good suggestion though, short of
> making the name even longer and inserting "backwards".

Would xc_cpu_policy_make_compat_412 be acceptable?

That's the more concise one I can think of.

Thanks, Roger.

On 04.05.2021 17:34, Roger Pau Monné wrote:
> On Mon, May 03, 2021 at 01:09:41PM +0200, Jan Beulich wrote:
>> On 30.04.2021 17:52, Roger Pau Monne wrote:
>>> @@ -1086,3 +1075,42 @@ int xc_cpu_policy_calc_compatible(xc_interface *xch,
>>>  
>>>      return rc;
>>>  }
>>> +
>>> +int xc_cpu_policy_make_compatible(xc_interface *xch, xc_cpu_policy_t policy,
>>> +                                  bool hvm)
>>
>> I'm concerned of the naming, and in particular the two very different
>> meanings of "compatible" for xc_cpu_policy_calc_compatible() and this
>> new one. I'm afraid I don't have a good suggestion though, short of
>> making the name even longer and inserting "backwards".
> 
> Would xc_cpu_policy_make_compat_412 be acceptable?
> 
> That's the more concise one I can think of.

Hmm, maybe (perhaps with an underscore inserted between 4 and 12). Yet
(sorry) a comment in the function says "since Xen 4.13", which includes
changes that have happened later. Therefore it's not really clear to me
whether the function really _only_ deals with the 4.12 / 4.13 boundary.

Jan