x86/cpuid: Enable CPUID Faulting for PV control domains by default

The domain builder no longer uses local CPUID instructions for policy
decisions.  This resolves a key issue for PVH dom0's.  However, as PV dom0's
have never had faulting enforced, leave a command line option to restore the
old behaviour.

Advertise virtualised faulting support to control domains unless the opt-out
has been used.

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Release-acked-by: Juergen Gross <jgross@suse.com>

docs/misc/xen-command-line.pandoc

@@ -658,7 +658,8 @@ The debug trace feature is only enabled in debugging builds of Xen.
 Specify the bit width of the DMA heap.
 
 ### dom0
-    = List of [ pv | pvh, shadow=<bool>, verbose=<bool> ]
+    = List of [ pv | pvh, shadow=<bool>, verbose=<bool>,
+                cpuid-faulting=<bool> ]
 
     Applicability: x86
 
@@ -691,6 +692,22 @@ Controls for how dom0 is constructed on x86 systems.
     information during the dom0 build.  It defaults to the compile time choice
     of `CONFIG_VERBOSE_DEBUG`.
 
+*   The `cpuid-faulting` boolean is an interim option, is only applicable to
+    PV dom0, and defaults to true.
+
+    Before Xen 4.13, the domain builder logic for guest construction depended
+    on seeing host CPUID values to function correctly.  As a result, CPUID
+    Faulting was never activated for PV dom0's, even on capable hardware.
+
+    In Xen 4.13, the domain builder logic has been fixed, and no longer has
+    this dependency.  As a consequence, CPUID Faulting is activated by default
+    even for PV dom0's.
+
+    However, as PV dom0's have always seen host CPUID data in the past, there
+    is a chance that further dependencies exist.  This boolean can be used to
+    restore the pre-4.13 behaviour.  If specifying `no-cpuid-faulting` fixes
+    an issue in dom0, please report a bug.
+
 ### dom0-iommu
     = List of [ passthrough=<bool>, strict=<bool>, map-inclusive=<bool>,
                 map-reserved=<bool>, none ]

xen/arch/x86/cpu/common.c

@@ -10,12 +10,15 @@
 #include <asm/io.h>
 #include <asm/mpspec.h>
 #include <asm/apic.h>
+#include <asm/setup.h>
 #include <mach_apic.h>
 #include <public/sysctl.h> /* for XEN_INVALID_{SOCKET,CORE}_ID */
 
 #include "cpu.h"
 #include "mcheck/x86_mca.h"
 
+bool __read_mostly opt_dom0_cpuid_faulting = true;
+
 bool_t opt_arat = 1;
 boolean_param("arat", opt_arat);
 
@@ -171,20 +174,19 @@ void ctxt_switch_levelling(const struct vcpu *next)
 		/*
 		 * We *should* be enabling faulting for PV control domains.
 		 *
-		 * Unfortunately, the domain builder (having only ever been a
+		 * The domain builder has now been updated to not depend on
-		 * PV guest) expects to be able to see host cpuid state in a
+		 * seeing host CPUID values.  This makes it compatible with
-		 * native CPUID instruction, to correctly build a CPUID policy
+		 * PVH toolstack domains, and lets us enable faulting by
-		 * for HVM guests (notably the xstate leaves).
+		 * default for all PV domains.
 		 *
-		 * This logic is fundimentally broken for HVM toolstack
+		 * However, as PV control domains have never had faulting
-		 * domains, and faulting causes PV guests to behave like HVM
+		 * enforced on them before, there might plausibly be other
-		 * guests from their point of view.
+		 * dependenices on host CPUID data.  Therefore, we have left
-		 *
+		 * an interim escape hatch in the form of
-		 * Future development plans will move responsibility for
+		 * `dom0=no-cpuid-faulting` to restore the older behaviour.
-		 * generating the maximum full cpuid policy into Xen, at which
-		 * this problem will disappear.
 		 */
-		set_cpuid_faulting(nextd && (!is_control_domain(nextd) ||
+		set_cpuid_faulting(nextd && (opt_dom0_cpuid_faulting ||
+					     !is_control_domain(nextd) ||
 					     !is_pv_domain(nextd)) &&
 				   (is_pv_domain(nextd) ||
 				    next->arch.msrs->

xen/arch/x86/dom0_build.c

@@ -305,6 +305,9 @@ static int __init parse_dom0_param(const char *s)
 #endif
         else if ( (val = parse_boolean("verbose", s, ss)) >= 0 )
             opt_dom0_verbose = val;
+        else if ( IS_ENABLED(CONFIG_PV) &&
+                  (val = parse_boolean("cpuid-faulting", s, ss)) >= 0 )
+            opt_dom0_cpuid_faulting = val;
         else
             rc = -EINVAL;
 

xen/arch/x86/msr.c

@@ -26,6 +26,7 @@
 
 #include <asm/debugreg.h>
 #include <asm/msr.h>
+#include <asm/setup.h>
 
 DEFINE_PER_CPU(uint32_t, tsc_aux);
 
@@ -92,7 +93,7 @@ int init_domain_msr_policy(struct domain *d)
         return -ENOMEM;
 
     /* See comment in ctxt_switch_levelling() */
-    if ( is_control_domain(d) && is_pv_domain(d) )
+    if ( !opt_dom0_cpuid_faulting && is_control_domain(d) && is_pv_domain(d) )
         mp->platform_info.cpuid_faulting = false;
 
     d->arch.msr = mp;

xen/include/asm-x86/setup.h

@@ -66,6 +66,7 @@ extern bool opt_dom0_shadow;
 #endif
 extern bool opt_dom0_pvh;
 extern bool opt_dom0_verbose;
+extern bool opt_dom0_cpuid_faulting;
 
 #define max_init_domid (0)