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>

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index 832797e2e2b82d331e3193b2c0781608edc6d038..fc6442906451b547233e019b7f199a453a3a6bc2 100644 (file)
--- a/docs/misc/xen-command-line.pandoc
+++ b/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 ]

diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 4bf852c948f963ecaa8f23878dedb25514828277..6c6bd6330129b0454d5df605978db335f30c74e0 100644 (file)
--- a/xen/arch/x86/cpu/common.c
+++ b/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
-                * PV guest) expects to be able to see host cpuid state in a
-                * native CPUID instruction, to correctly build a CPUID policy
-                * for HVM guests (notably the xstate leaves).
-                *
-                * This logic is fundimentally broken for HVM toolstack
-                * domains, and faulting causes PV guests to behave like HVM
-                * guests from their point of view.
+                * The domain builder has now been updated to not depend on
+                * seeing host CPUID values.  This makes it compatible with
+                * PVH toolstack domains, and lets us enable faulting by
+                * default for all PV domains.
                 *
-                * Future development plans will move responsibility for
-                * generating the maximum full cpuid policy into Xen, at which
-                * this problem will disappear.
+                * However, as PV control domains have never had faulting
+                * enforced on them before, there might plausibly be other
+                * dependenices on host CPUID data.  Therefore, we have left
+                * an interim escape hatch in the form of
+                * `dom0=no-cpuid-faulting` to restore the older behaviour.
                 */
-               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->

diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
index 7cfab2dc25f129bc78c1b6e79fcff268a35a97dd..454cf632d7ded0ad53a9f433563d306a3e2f84e1 100644 (file)
--- a/xen/arch/x86/dom0_build.c
+++ b/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;
 

diff --git a/xen/arch/x86/msr.c b/xen/arch/x86/msr.c
index a6c8cc7627e6305f93d0036724b6706455319fc7..4698d2bba1c96485bcb6f7475348fd8ac738315c 100644 (file)
--- a/xen/arch/x86/msr.c
+++ b/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;

diff --git a/xen/include/asm-x86/setup.h b/xen/include/asm-x86/setup.h
index 15d6363022374da6f38a208722ee8b4bf6245a96..861d46d6ac1e19ee64d5458624deed3640b50e16 100644 (file)
--- a/xen/include/asm-x86/setup.h
+++ b/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)