Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Andrew Cooper <andrew.cooper3@citrix.com>
To: Xen-devel <xen-devel@lists.xenproject.org>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Wei Liu" <wl@xen.org>, "Jan Beulich" <JBeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>
Subject: [Xen-devel] [PATCH v2 10/10] x86/cpuid: Enable CPUID Faulting for PV control domains by default
Date: Fri, 13 Sep 2019 20:27:59 +0100
Message-ID: <20190913192759.10795-11-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <20190913192759.10795-1-andrew.cooper3@citrix.com>

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>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wl@xen.org>
CC: Roger Pau Monné <roger.pau@citrix.com>

v2:
 * Introduce a command line option to retain old behaviour.
 * Advertise virtualised faulting support to dom0 when it is used.
v2.1:
 * Split the PVH adjustment out.  Rebase.
 * Recover the docs/ hunk which was accidentally missing.
---
 docs/misc/xen-command-line.pandoc | 19 ++++++++++++++++++-
 xen/arch/x86/cpu/common.c         | 26 ++++++++++++++------------
 xen/arch/x86/dom0_build.c         |  2 ++
 xen/arch/x86/msr.c                |  3 ++-
 xen/include/asm-x86/setup.h       |  1 +
 5 files changed, 37 insertions(+), 14 deletions(-)

diff --git a/docs/misc/xen-command-line.pandoc b/docs/misc/xen-command-line.pandoc
index 832797e2e2..fc64429064 100644
--- 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 4bf852c948..6c6bd63301 100644
--- 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 c69570920c..4b75166db3 100644
--- a/xen/arch/x86/dom0_build.c
+++ b/xen/arch/x86/dom0_build.c
@@ -305,6 +305,8 @@ 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 ( (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 a6c8cc7627..4698d2bba1 100644
--- 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 15d6363022..861d46d6ac 100644
--- 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)
 
-- 
2.11.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  parent reply index

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-13 19:27 [Xen-devel] [PATCH v2 00/10] x86/cpuid: Switch to using XEN_DOMCTL_set_cpumsr_policy Andrew Cooper
2019-09-13 19:27 ` [Xen-devel] [PATCH v2 01/10] x86/msr: Offer CPUID Faulting to PVH control domains Andrew Cooper
2019-09-16 10:53   ` Jan Beulich
2019-09-13 19:27 ` [Xen-devel] [PATCH v2 02/10] libx86: Proactively initialise error pointers Andrew Cooper
     [not found]   ` <527f33ad-3de1-15c7-eb4b-603eaf65f3c5@suse.com>
     [not found]     ` <65f18521-15c5-72a9-29f6-cd5d621e1283@citrix.com>
2019-09-16 15:46       ` Jan Beulich
2019-09-13 19:27 ` [Xen-devel] [PATCH v2 03/10] libx86: Introduce x86_cpu_policies_are_compatible() Andrew Cooper
2019-09-16 10:59   ` Jan Beulich
2019-09-16 15:31     ` Andrew Cooper
2019-09-13 19:27 ` [Xen-devel] [PATCH v2 04/10] x86/cpuid: Split update_domain_cpuid_info() in half Andrew Cooper
2019-09-13 19:27 ` [Xen-devel] [PATCH v2 05/10] x86/domctl: Implement XEN_DOMCTL_set_cpumsr_policy Andrew Cooper
2019-09-16 11:04   ` Jan Beulich
2019-09-16 15:40     ` Andrew Cooper
2019-09-13 19:27 ` [Xen-devel] [PATCH v2 06/10] tools/libxc: Pre-cleanup for xc_cpuid_{set, apply_policy}() Andrew Cooper
2019-09-16 11:09   ` Jan Beulich
2019-09-16 15:42     ` Andrew Cooper
2019-09-13 19:27 ` [Xen-devel] [PATCH v2 07/10] tools/libxc: Rework xc_cpuid_set() to use {get, set}_cpu_policy() Andrew Cooper
2019-09-13 19:27 ` [Xen-devel] [PATCH v2 08/10] tools/libxc: Rework xc_cpuid_apply_policy() " Andrew Cooper
2019-09-16 11:17   ` Jan Beulich
2019-09-16 13:41     ` Wei Liu
2019-09-16 15:49     ` Andrew Cooper
2019-09-16 16:05       ` Jan Beulich
2019-09-18 16:09   ` Jan Beulich
2019-09-19  8:48     ` Andrew Cooper
2019-09-25 18:11   ` [Xen-devel] [PATCH v3 " Andrew Cooper
2019-09-26  8:04     ` Jan Beulich
2019-09-26 12:25       ` Andrew Cooper
2019-09-13 19:27 ` [Xen-devel] [PATCH v2 09/10] x86/domctl: Drop XEN_DOMCTL_set_cpuid Andrew Cooper
2019-09-13 19:27 ` Andrew Cooper [this message]
2019-09-16 11:22   ` [Xen-devel] [PATCH v2 10/10] x86/cpuid: Enable CPUID Faulting for PV control domains by default Jan Beulich
2019-09-16 15:52     ` Andrew Cooper

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190913192759.10795-11-andrew.cooper3@citrix.com \
    --to=andrew.cooper3@citrix.com \
    --cc=JBeulich@suse.com \
    --cc=roger.pau@citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git