[PATCH RFC 19/31] tools/libxc: Sanitise guest featuresets

[Andrew Cooper <andrew.cooper3@citrix.com>]

When generating a VM featureset, clearing individual features is problematic
if a feature has dependent features.

Instead of disabling individual features, collect all disabling together in
sanitise_featureset(), and perform deep dependency removal on the result, to
remove all impacted features.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Ian Campbell <Ian.Campbell@citrix.com>
CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
CC: Wei Liu <wei.liu2@citrix.com>
---
 tools/libxc/xc_cpuid_x86.c | 144 +++++++++++++++++++++------------------------
 1 file changed, 67 insertions(+), 77 deletions(-)

diff --git a/tools/libxc/xc_cpuid_x86.c b/tools/libxc/xc_cpuid_x86.c
index 3f39306..6c8995f 100644
--- a/tools/libxc/xc_cpuid_x86.c
+++ b/tools/libxc/xc_cpuid_x86.c
@@ -21,14 +21,15 @@
 
 #include <stdlib.h>
 #include <stdbool.h>
+#include <limits.h>
 #include "xc_private.h"
-#include <xen/arch-x86/featureset.h>
+#include "cpuid-private.h"
 #include <xen/hvm/params.h>
 #include <xen/sysctl.h>
 
 #define bitmaskof(idx)      (1u << ((idx) & 31))
-#define clear_bit(idx, dst) ((dst) &= ~bitmaskof(idx))
-#define set_bit(idx, dst)   ((dst) |=  bitmaskof(idx))
+#define clear_feature(idx, dst) ((dst) &= ~bitmaskof(idx))
+#define set_feature(idx, dst)   ((dst) |=  bitmaskof(idx))
 
 #define DEF_MAX_BASE 0x0000000du
 #define DEF_MAX_INTELEXT  0x80000008u
@@ -212,20 +213,6 @@ static void amd_xc_cpuid_policy(xc_interface *xch,
             regs[0] = DEF_MAX_AMDEXT;
         break;
 
-    case 0x80000001:
-        if ( !info->pae )
-            clear_bit(X86_FEATURE_PAE, regs[3]);
-
-        if ( !info->nestedhvm )
-            clear_bit(X86_FEATURE_SVM, regs[2]);
-
-        if ( info->xfeature_mask == 0 )
-        {
-            clear_bit(X86_FEATURE_FMA4, regs[2]);
-            clear_bit(X86_FEATURE_LWP, regs[2]);
-        }
-        break;
-
     case 0x80000008:
         /*
          * ECX[15:12] is ApicIdCoreSize: ECX[7:0] is NumberOfCores (minus one).
@@ -272,11 +259,6 @@ static void intel_xc_cpuid_policy(xc_interface *xch,
 {
     switch ( input[0] )
     {
-    case 0x00000001:
-        if ( !info->nestedhvm )
-            clear_bit(X86_FEATURE_VMXE, regs[2]);
-        break;
-
     case 0x00000004:
         /*
          * EAX[31:26] is Maximum Cores Per Package (minus one).
@@ -375,20 +357,6 @@ static void xc_cpuid_hvm_policy(xc_interface *xch,
 
         regs[2] = info->featureset[XEN_FEATURESET_1c];
         regs[3] = info->featureset[XEN_FEATURESET_1d];
-
-        if ( info->xfeature_mask == 0 )
-        {
-            clear_bit(X86_FEATURE_AVX, regs[2]);
-            clear_bit(X86_FEATURE_F16C, regs[2]);
-            clear_bit(X86_FEATURE_FMA, regs[2]);
-            clear_bit(X86_FEATURE_XSAVE, regs[2]);
-        }
-
-        if ( !info->pae )
-        {
-            clear_bit(X86_FEATURE_PAE, regs[3]);
-            clear_bit(X86_FEATURE_PSE36, regs[3]);
-        }
         break;
 
     case 0x00000007: /* Intel-defined CPU features */
@@ -416,15 +384,6 @@ static void xc_cpuid_hvm_policy(xc_interface *xch,
     case 0x80000001:
         regs[2] = info->featureset[XEN_FEATURESET_e1c];
         regs[3] = info->featureset[XEN_FEATURESET_e1d];
-
-        if ( !info->pae )
-        {
-            clear_bit(X86_FEATURE_LAHF_LM, regs[2]);
-            clear_bit(X86_FEATURE_LM, regs[3]);
-            clear_bit(X86_FEATURE_NX, regs[3]);
-            clear_bit(X86_FEATURE_PSE36, regs[3]);
-            clear_bit(X86_FEATURE_PAGE1GB, regs[3]);
-        }
         break;
 
     case 0x80000007:
@@ -473,23 +432,6 @@ static void xc_cpuid_pv_policy(xc_interface *xch,
     case 0x00000001:
         regs[2] = info->featureset[XEN_FEATURESET_1c];
         regs[3] = info->featureset[XEN_FEATURESET_1d];
-
-        if ( !info->pv64 )
-            clear_bit(X86_FEATURE_CX16, regs[2]);
-
-        if ( info->xfeature_mask == 0 )
-        {
-            clear_bit(X86_FEATURE_XSAVE, regs[2]);
-            clear_bit(X86_FEATURE_AVX, regs[2]);
-            clear_bit(X86_FEATURE_F16C, regs[2]);
-            clear_bit(X86_FEATURE_FMA, regs[2]);
-        }
-
-        if ( !info->pvh )
-        {
-            clear_bit(X86_FEATURE_PSE, regs[3]);
-            clear_bit(X86_FEATURE_PGE, regs[3]);
-        }
         break;
 
     case 0x00000007:
@@ -513,19 +455,6 @@ static void xc_cpuid_pv_policy(xc_interface *xch,
     case 0x80000001:
         regs[2] = info->featureset[XEN_FEATURESET_e1c];
         regs[3] = info->featureset[XEN_FEATURESET_e1d];
-
-        if ( !info->pv64 )
-        {
-            clear_bit(X86_FEATURE_LM, regs[3]);
-            clear_bit(X86_FEATURE_LAHF_LM, regs[2]);
-        }
-
-        if ( !info->pvh )
-        {
-            clear_bit(X86_FEATURE_PSE, regs[3]);
-            clear_bit(X86_FEATURE_PGE, regs[3]);
-            clear_bit(X86_FEATURE_PAGE1GB, regs[3]);
-        }
         break;
 
     case 0x00000005: /* MONITOR/MWAIT */
@@ -605,6 +534,65 @@ void xc_cpuid_to_str(const unsigned int *regs, char **strs)
     }
 }
 
+static void sanitise_featureset(struct cpuid_domain_info *info)
+{
+    uint32_t disabled_features[XEN_NR_FEATURESET_ENTRIES];
+    unsigned int i, b;
+
+    if ( info->hvm )
+    {
+        /* HVM Guest */
+
+        if ( !info->pae )
+            clear_bit(X86_FEATURE_PAE, info->featureset);
+
+        if ( !info->nestedhvm )
+        {
+            clear_bit(X86_FEATURE_SVM, info->featureset);
+            clear_bit(X86_FEATURE_VMXE, info->featureset);
+        }
+
+    }
+    else
+    {
+        /* PV or PVH Guest */
+
+        if ( !info->pv64 )
+            clear_bit(X86_FEATURE_LM, info->featureset);
+
+        if ( !info->pvh )
+        {
+            clear_bit(X86_FEATURE_PSE, info->featureset);
+            clear_bit(X86_FEATURE_PSE36, info->featureset);
+            clear_bit(X86_FEATURE_PGE, info->featureset);
+            clear_bit(X86_FEATURE_PAGE1GB, info->featureset);
+        }
+    }
+
+    if ( info->xfeature_mask == 0 )
+        clear_bit(X86_FEATURE_XSAVE, info->featureset);
+
+    /* Disable deep dependencies of disabled features. */
+
+    for ( i = 0; i < ARRAY_SIZE(disabled_features); ++i )
+        disabled_features[i] = ~info->featureset[i] & deep_dep_features[i];
+
+    for ( b = 0; b < sizeof(disabled_features) * CHAR_BIT; ++b )
+    {
+        const struct tagged_featureset *dfs;
+
+        if ( !test_bit(b, disabled_features) ||
+             !(dfs = lookup_deep_deps(b)) )
+             continue;
+
+        for ( i = 0; i < ARRAY_SIZE(disabled_features); ++i )
+        {
+            info->featureset[i] &= ~dfs->fs[i];
+            disabled_features[i] &= ~dfs->fs[i];
+        }
+    }
+}
+
 static int __xc_cpuid_apply_policy(xc_interface *xch, domid_t domid,
                                    uint32_t *featureset,
                                    unsigned int nr_features)
@@ -628,6 +616,8 @@ static int __xc_cpuid_apply_policy(xc_interface *xch, domid_t domid,
     else
         ext_max = (regs[0] <= DEF_MAX_INTELEXT) ? regs[0] : DEF_MAX_INTELEXT;
 
+    sanitise_featureset(&info);
+
     input[0] = 0;
     input[1] = XEN_CPUID_INPUT_UNUSED;
     for ( ; ; )
@@ -811,9 +801,9 @@ int xc_cpuid_set(
                 val = polval;
 
             if ( val )
-                set_bit(31 - j, regs[i]);
+                set_feature(31 - j, regs[i]);
             else
-                clear_bit(31 - j, regs[i]);
+                clear_feature(31 - j, regs[i]);
 
             config_transformed[i][j] = config[i][j];
             if ( config[i][j] == 's' )
-- 
2.1.4