Patchwork [2/2] UBUNTU: SAUCE: Mask CR4 writes on older Xen hypervisors

login
register
mail settings
Submitter Stefan Bader
Date Sept. 4, 2012, 3:08 p.m.
Message ID <1346771283-8881-3-git-send-email-stefan.bader@canonical.com>
Download mbox | patch
Permalink /patch/181602/
State New
Headers show

Comments

Stefan Bader - Sept. 4, 2012, 3:08 p.m.
Older Xen hypervisors (like RHEL5 versions found to be used
on Amazon's EC2) did have a bug which would crash the domain
when trying to write unsupported CR4 values.
Newer versions do handle this correctly. But some probes (in
particular one that tries to pass the OSXSAVE bit set) were
causing pv-ops kernels to crash early on boot when running on EC2.

We were using a patch that did always filter the OSXSAVE off the
values written to CR4 when running as Xen PV guest. While not
completely wrong this creates an inconsistency between the cpuid
bits a guest sees and the CR4 settings. And this did recently
cause problems because user-space was not testing all bits when
deciding to use certain features.

This patch will actually mask off the cpuid bits for XSAVE and
OSXSAVE, so generic code will not even try to set CR4. It is
limited to PV guests and (since we do not actually know the
exact version) Xen hypervisors before version 4.

[v2: make the version check an inline function]
Signed-off-by: Stefan Bader <stefan.bader@canonical.com>

BugLink: http://bugs.launchpad.net/bugs/1044550

Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
---
 arch/x86/xen/enlighten.c |   17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)
Stefan Bader - Sept. 4, 2012, 3:16 p.m.
While the patch itself is ok, I realize that the summary/subject is not really
correct (it is the same as it is currently in quantal, though). The patch does
not mask CR4 writes but masks off the cpu feature bits. The title must have
stuck while coming up with the alternate solution. So maybe for Precise the
title should become

UBUNTU: SAUCE: Force xsave off on older Xen hypervisors

-Stefan

Patch

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index bdf0883..5811718 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -188,6 +188,18 @@  void xen_vcpu_restore(void)
 	}
 }
 
+/*
+ * Older (with no clear statement about what old means) Xen hypervisors
+ * will crash a PV guest that tries to store OSXSAVE into CR4.
+ * To prevent this, we force the feature bits related to this off in the
+ * xen cpuid call. This inline function serves as a centralized test
+ * on whether the quirk should be done.
+ */
+static inline needs_xsave_quirk(unsigned version)
+{
+	return (xen_pv_domain() && ((version >> 16) < 4)) ? 1 : 0;
+}
+
 static void __init xen_banner(void)
 {
 	unsigned version = HYPERVISOR_xen_version(XENVER_version, NULL);
@@ -199,6 +211,8 @@  static void __init xen_banner(void)
 	printk(KERN_INFO "Xen version: %d.%d%s%s\n",
 	       version >> 16, version & 0xffff, extra.extraversion,
 	       xen_feature(XENFEAT_mmu_pt_update_preserve_ad) ? " (preserve-AD)" : "");
+	if (needs_xsave_quirk(version))
+		printk(KERN_INFO "Forcing xsave off due to Xen version.\n");
 }
 
 #define CPUID_THERM_POWER_LEAF 6
@@ -249,6 +263,7 @@  static void xen_cpuid(unsigned int *ax, unsigned int *bx,
 
 static void __init xen_init_cpuid_mask(void)
 {
+	unsigned version = HYPERVISOR_xen_version(XENVER_version, NULL);
 	unsigned int ax, bx, cx, dx;
 	unsigned int xsave_mask;
 
@@ -271,7 +286,7 @@  static void __init xen_init_cpuid_mask(void)
 		(1 << (X86_FEATURE_OSXSAVE % 32));
 
 	/* Xen will set CR4.OSXSAVE if supported and not disabled by force */
-	if ((cx & xsave_mask) != xsave_mask)
+	if (((cx & xsave_mask) != xsave_mask) || needs_xsave_quirk(version))
 		cpuid_leaf1_ecx_mask &= ~xsave_mask; /* disable XSAVE & OSXSAVE */
 }