xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / 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 8/8] x86/cpuid: Enable CPUID Faulting for the control domain by default
Date: Thu, 12 Sep 2019 19:55:00 +0100	[thread overview]
Message-ID: <20190912185500.21621-1-andrew.cooper3@citrix.com> (raw)
In-Reply-To: <20190911200504.5693-9-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.

In ctxt_switch_levelling(), invert the first cpu_has_cpuid_faulting condition
to reduce the indentation for the CPUID faulting logic.

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>

opt_dom0_cpuid_faulting would ideally live in dom0_build.c next to
opt_dom0_verbose, but the file is currently .init

v2:
 * Introduce a command line option to retain old behaviour.
 * Advertise virtualised faulting support to dom0 when it is used.

RFC: The previous logic was slightly buggy in that even PVH dom0's had
virtualised faulting support hidden from them.  Given that this case always
hits the CPUID intercept, how much do we care about retaining the old
behaviour?
---
 xen/arch/x86/cpu/common.c   | 59 +++++++++++++++++++++++----------------------
 xen/arch/x86/dom0_build.c   |  2 ++
 xen/arch/x86/msr.c          |  3 ++-
 xen/include/asm-x86/setup.h |  1 +
 4 files changed, 35 insertions(+), 30 deletions(-)

diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 937d8e82a8..b8d6c4967e 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);
 
@@ -161,38 +164,36 @@ void ctxt_switch_levelling(const struct vcpu *next)
 {
 	const struct domain *nextd = next ? next->domain : NULL;
 
-	if (cpu_has_cpuid_faulting) {
-		/*
-		 * No need to alter the faulting setting if we are switching
-		 * to idle; it won't affect any code running in idle context.
-		 */
-		if (nextd && is_idle_domain(nextd))
-			return;
-		/*
-		 * We *should* be enabling faulting for the control domain.
-		 *
-		 * 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.
-		 *
-		 * Future development plans will move responsibility for
-		 * generating the maximum full cpuid policy into Xen, at which
-		 * this problem will disappear.
-		 */
-		set_cpuid_faulting(nextd && !is_control_domain(nextd) &&
-				   (is_pv_domain(nextd) ||
-				    next->arch.msrs->
-				    misc_features_enables.cpuid_faulting));
+	if (!cpu_has_cpuid_faulting) {
+		if (ctxt_switch_masking)
+			alternative_vcall(ctxt_switch_masking, next);
 		return;
 	}
 
-	if (ctxt_switch_masking)
-		alternative_vcall(ctxt_switch_masking, next);
+	/*
+	 * No need to alter the faulting setting if we are switching
+	 * to idle; it won't affect any code running in idle context.
+	 */
+	if (nextd && is_idle_domain(nextd))
+		return;
+
+	/*
+	 * We *should* be enabling faulting for the control domain.
+	 *
+	 * 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.
+	 *
+	 * 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 && (opt_dom0_cpuid_faulting ||
+				     !is_control_domain(nextd)) &&
+			   (is_pv_domain(nextd) ||
+			    next->arch.msrs->
+			    misc_features_enables.cpuid_faulting));
 }
 
 bool_t opt_cpu_info;
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 e65961fccb..88b882c8cc 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 intel_ctxt_switch_levelling() */
-    if ( is_control_domain(d) )
+    if ( !opt_dom0_cpuid_faulting && is_control_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	other threads:[~2019-09-12 18:55 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-11 20:04 [Xen-devel] [PATCH 0/8] x86/cpuid: Switch to using XEN_DOMCTL_set_cpumsr_policy Andrew Cooper
2019-09-11 20:04 ` [Xen-devel] [PATCH 1/8] libx86: Introduce x86_cpu_policies_are_compatible() Andrew Cooper
2019-09-12  7:43   ` Jan Beulich
2019-09-12  7:59     ` Andrew Cooper
2019-09-12  8:22       ` Jan Beulich
2019-09-12 11:41         ` Andrew Cooper
2019-09-11 20:04 ` [Xen-devel] [PATCH 2/8] x86/cpuid: Split update_domain_cpuid_info() in half Andrew Cooper
2019-09-12  7:52   ` Jan Beulich
2019-09-12  8:07     ` Andrew Cooper
2019-09-11 20:04 ` [Xen-devel] [PATCH 3/8] x86/domctl: Implement XEN_DOMCTL_set_cpumsr_policy Andrew Cooper
2019-09-12  8:06   ` Jan Beulich
2019-09-12 13:15     ` Andrew Cooper
2019-09-12 13:20       ` Jan Beulich
2019-09-12 16:34       ` Andrew Cooper
2019-09-11 20:05 ` [Xen-devel] [PATCH 4/8] tools/libxc: Pre-cleanup for xc_cpuid_{set, apply_policy}() Andrew Cooper
2019-09-12  8:09   ` Jan Beulich
2019-09-12  8:17   ` Jan Beulich
2019-09-12  8:38     ` Andrew Cooper
2019-09-11 20:05 ` [Xen-devel] [PATCH 5/8] tools/libxc: Rework xc_cpuid_set() to use {get, set}_cpu_policy() Andrew Cooper
2019-09-12  8:19   ` Jan Beulich
2019-09-12  8:36     ` Andrew Cooper
2019-09-12  9:11       ` Jan Beulich
2019-09-12 13:21         ` Andrew Cooper
2019-09-11 20:05 ` [Xen-devel] [PATCH 6/8] tools/libxc: Rework xc_cpuid_apply_policy() " Andrew Cooper
2019-09-12  9:02   ` Jan Beulich
2019-09-12 13:47     ` Andrew Cooper
2019-09-11 20:05 ` [Xen-devel] [PATCH 7/8] x86/domctl: Drop XEN_DOMCTL_set_cpuid Andrew Cooper
2019-09-12  9:04   ` Jan Beulich
2019-09-11 20:05 ` [Xen-devel] [PATCH 8/8] x86/cpuid: Enable CPUID Faulting for the control domain Andrew Cooper
2019-09-12  9:07   ` Jan Beulich
2019-09-12  9:28     ` Andrew Cooper
2019-09-12 18:55   ` Andrew Cooper [this message]
2019-09-13  6:38     ` [Xen-devel] [PATCH v2 8/8] x86/cpuid: Enable CPUID Faulting for the control domain by default Jan Beulich
2019-09-13 14:56       ` Andrew Cooper
2019-09-12 18:55 ` [Xen-devel] [PATCH v2 0.5/8] libx86: Proactively initialise error pointers Andrew Cooper
2019-09-13  6:20   ` Jan Beulich

Reply instructions:

You may reply publicly 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=20190912185500.21621-1-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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).