diff mbox

[RFC,Quantal] Replace pv-ops fix for legacy Xen [v2]

Message ID 1339754099-12023-1-git-send-email-stefan.bader@canonical.com
State New
Headers show

Commit Message

Stefan Bader June 15, 2012, 9:54 a.m. UTC 
> Just a nit, how about making the version check a documented macro
> instead of open coding '(version >> 16) < 4)' twice ?

You know, I was already wondering about that just after sending
out the initial version. So how about that? Just using an inline
function instead of a macro. Not sure why, but I find inline
a bit more attractive. Should not make much difference...

-Stefan

From 590ef72644debfb8c910eda68f5264610d128f74 Mon Sep 17 00:00:00 2001
From: Stefan Bader <stefan.bader@canonical.com>
Date: Mon, 4 Jun 2012 19:12:51 +0200
Subject: [PATCH] UBUNTU: SAUCE: Mask CR4 writes on older Xen hypervisors

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>
---
 arch/x86/xen/enlighten.c |   17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

Comments

Tim Gardner June 15, 2012, 12:09 p.m. UTC | #1
Stefan Bader June 15, 2012, 1:42 p.m. UTC | #2 
On 15.06.2012 14:09, Tim Gardner wrote:
> 
Sorry I was not clear enough before. This of course replaces the old SAUCE
patch. I took the liberty of pushing that revert to master-next myself.

Leann can squash the old patch plus revert out of existence on next rebase.

-Stefan
Tim Gardner June 15, 2012, 3:34 p.m. UTC | #3 
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On 06/15/2012 07:42 AM, Stefan Bader wrote:
> On 15.06.2012 14:09, Tim Gardner wrote:
>> 
> Sorry I was not clear enough before. This of course replaces the
> old SAUCE patch. I took the liberty of pushing that revert to
> master-next myself.
> 
> Leann can squash the old patch plus revert out of existence on next
> rebase.
> 
> -Stefan
> 

Done - rebased out of existence.

- -- 
Tim Gardner tim.gardner@canonical.com
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQIcBAEBCgAGBQJP21X+AAoJED12yEX6FEfKi80P/RbFH5nzJNiJEOgcFd7ylAhC
D9CEIbrdlix+7e21jLi1eY92dxst1CojChmZ6BORsdzyW0dTzjHn8Nyh6KTXabn9
ll1+bQRJnces7DJ6yqftr6j3P3ENWUa8sz+zx3VJVvoSnQ5OOSQEbg2J8OWb7tOH
o1e4p5h1wZurTiNaXeNgPAQxI7NnO/CIJ+DYUUazg3Wb4LGq2EeJNTHo6LaUk0Fd
MTOmZ+F+oFF5d8evVkJk0gPP863Bu0W3NsF1AEebCelNhEgnRStZ2GcG3qY20TrL
DAzHM9U8MScMJ3suREkrOjQn1MCDih+ERE7k2jcnMJYVWhRttJLvqIz8dTUShPEv
h+ENfpy7ce4CwZF/1XdQeRJXZzrL0cbENhW0I8EJLhdWr3sOqs2F8pbbQx/K7ogD
B/m0RckZRKvgXlvCc/89Gmu+p06Mub+nlqeBCgp4n51NHCTpUhqqccueOKTVk9De
S7Kn5DynjhmvLbAR1uxLWd8CKPnoblStX1Z3Rh2y3SufyS5IfccA13SIp+yOyeVc
IcSecaFwifGfPuM+9fkofIsUWKTKKxyOiKMGVixeO55rnBWelmHLzoWfqdTa4gid
iyWc7mU7x/uExtqvz3474hZ9v6HGPbjbyGlj/CNnW9rWY4SQx5AlNrtEWS0WT0jc
s99GPpV8shGwHf7UnOnA
=5vBQ
-----END PGP SIGNATURE-----
diff mbox

Patch

diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index e74df95..d465026 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -196,6 +196,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);
@@ -207,6 +219,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");
 }
 
 static __read_mostly unsigned int cpuid_leaf1_edx_mask = ~0;
@@ -329,6 +343,7 @@  static bool __init xen_check_mwait(void)
 }
 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;
 
@@ -351,7 +366,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 */
 	if (xen_check_mwait())
 		cpuid_leaf1_ecx_set_mask = (1 << (X86_FEATURE_MWAIT % 32));