[v2,09/11] x86/xen: use capabilities instead of fake cpuid values for xsave

Commit Message

Jürgen Groß April 13, 2017, 10:11 a.m. UTC

When running as pv domain xen_cpuid() is being used instead of
native_cpuid(). In xen_cpuid() the xsave feature availability is
indicated by special casing the related cpuid leaf.

Instead of delivering fake cpuid values set or clear the cpu
capability bits for xsave instead.

Signed-off-by: Juergen Gross <[e-mail]>
---
 arch/x86/xen/enlighten_pv.c | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

Comments

Andrew Cooper April 13, 2017, 10:35 a.m. UTC | #1

On 13/04/17 11:11, Juergen Gross wrote:
> @@ -281,22 +274,19 @@ static bool __init xen_check_mwait(void)
>  	return false;
>  #endif
>  }
> -static void __init xen_init_cpuid_mask(void)
> +
> +static bool __init xen_check_xsave(void)
>  {
> -	unsigned int ax, bx, cx, dx;
> -	unsigned int xsave_mask;
> +	unsigned int cx, xsave_mask;
>  
> -	ax = 1;
> -	cx = 0;
> -	cpuid(1, &ax, &bx, &cx, &dx);
> +	cx = cpuid_ecx(1);
>  
>  	xsave_mask =
>  		(1 << (X86_FEATURE_XSAVE % 32)) |
>  		(1 << (X86_FEATURE_OSXSAVE % 32));
>  
>  	/* Xen will set CR4.OSXSAVE if supported and not disabled by force */
> -	if ((cx & xsave_mask) != xsave_mask)
> -		cpuid_leaf1_ecx_mask &= ~xsave_mask; /* disable XSAVE & OSXSAVE */
> +	return (cx & xsave_mask) == xsave_mask;

As you are taking the time to change this logic, could we see about
making it work in a way which doesn't require maintaining
non-architectural hackary in the hypervisor?

(For anyone reading this who isn't familiar with the history,
http://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/x86/cpuid.c;h=d359e090f33bb5793524186dc5f1c8bb620f17b3;hb=17cd6621688bce9972d9242611114fd7aba44c31#l726
ought to cover it)

In the presence of the bugs which caused this issue in the first place,
the best way to probe whether xsave is actually available is to try and
execute an xgetbv instruction, and use #UD as a signal that support
isn't actually available.

This also avoids Linux participating in the non-architectural behaviour
of forcing OXSAVE to be something other than a reflection of CR4.

~Andrew

Patch

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 38dec28a8e6d..dad1db61e40b 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -165,8 +165,6 @@  xen_running_on_version_or_later(unsigned int major, unsigned int minor)
 	return false;
 }
 
-static __read_mostly unsigned int cpuid_leaf1_ecx_mask = ~0;
-
 static __read_mostly unsigned int cpuid_leaf5_ecx_val;
 static __read_mostly unsigned int cpuid_leaf5_edx_val;
 
@@ -174,16 +172,12 @@  static void xen_cpuid(unsigned int *ax, unsigned int *bx,
 		      unsigned int *cx, unsigned int *dx)
 {
 	unsigned maskebx = ~0;
-	unsigned maskecx = ~0;
+
 	/*
 	 * Mask out inconvenient features, to try and disable as many
 	 * unsupported kernel subsystems as possible.
 	 */
 	switch (*ax) {
-	case 1:
-		maskecx = cpuid_leaf1_ecx_mask;
-		break;
-
 	case CPUID_MWAIT_LEAF:
 		/* Synthesize the values.. */
 		*ax = 0;
@@ -206,7 +200,6 @@  static void xen_cpuid(unsigned int *ax, unsigned int *bx,
 		: "0" (*ax), "2" (*cx));
 
 	*bx &= maskebx;
-	*cx &= maskecx;
 }
 STACK_FRAME_NON_STANDARD(xen_cpuid); /* XEN_EMULATE_PREFIX */
 
@@ -281,22 +274,19 @@  static bool __init xen_check_mwait(void)
 	return false;
 #endif
 }
-static void __init xen_init_cpuid_mask(void)
+
+static bool __init xen_check_xsave(void)
 {
-	unsigned int ax, bx, cx, dx;
-	unsigned int xsave_mask;
+	unsigned int cx, xsave_mask;
 
-	ax = 1;
-	cx = 0;
-	cpuid(1, &ax, &bx, &cx, &dx);
+	cx = cpuid_ecx(1);
 
 	xsave_mask =
 		(1 << (X86_FEATURE_XSAVE % 32)) |
 		(1 << (X86_FEATURE_OSXSAVE % 32));
 
 	/* Xen will set CR4.OSXSAVE if supported and not disabled by force */
-	if ((cx & xsave_mask) != xsave_mask)
-		cpuid_leaf1_ecx_mask &= ~xsave_mask; /* disable XSAVE & OSXSAVE */
+	return (cx & xsave_mask) == xsave_mask;
 }
 
 static void __init xen_init_capabilities(void)
@@ -316,6 +306,11 @@  static void __init xen_init_capabilities(void)
 		setup_force_cpu_cap(X86_FEATURE_MWAIT);
 	else
 		setup_clear_cpu_cap(X86_FEATURE_MWAIT);
+
+	if (!xen_check_xsave()) {
+		setup_clear_cpu_cap(X86_FEATURE_XSAVE);
+		setup_clear_cpu_cap(X86_FEATURE_OSXSAVE);
+	}
 }
 
 static void xen_set_debugreg(int reg, unsigned long val)
@@ -1308,7 +1303,6 @@  asmlinkage __visible void __init xen_start_kernel(void)
 	xen_setup_gdt(0);
 
 	xen_init_irq_ops();
-	xen_init_cpuid_mask();
 	xen_init_capabilities();
 
 #ifdef CONFIG_X86_LOCAL_APIC