[PATCH v3 00/11] x86: xen cpuid() cleanup

[PATCH v3 00/11] x86: xen cpuid() cleanup

[Juergen Gross]

Reduce special casing of xen_cpuid() by using cpu capabilities instead
of faked cpuid nodes.

This cleanup enables us remove the hypervisor specific set_cpu_features
callback as the same effect can be reached via
setup_[clear|force]_cpu_cap().

Removing the rest faked nodes from xen_cpuid() requires some more work
as the remaining cases (mwait leafs and extended topology info) have
to be handled at the consumer sides of this information.

Changes in V3:
- rewrite patch 9 (xsave) to use xgetbv instruction to test for osxsave
  availability (Andrew Cooper)

Changes in V2:
- added several features to this scheme
- removed hypervisor specific set_cpu_features() callbacks

Cc: Alok Kataria <akataria@vmware.com> 
Cc: Thomas Gleixner <tglx@linutronix.de> 
Cc: Ingo Molnar <mingo@redhat.com> 
Cc: "H. Peter Anvin" <hpa@zytor.com> 
Cc: x86@kernel.org 
Cc: virtualization@lists.linux-foundation.org

Juergen Gross (11):
  xen: set cpu capabilities from xen_start_kernel()
  x86/xen: don't indicate DCA support in pv domains
  x86/xen: use capabilities instead of fake cpuid values for aperf
  x86/xen: use capabilities instead of fake cpuid values for mtrr
  x86/xen: use capabilities instead of fake cpuid values for acc
  x86/xen: use capabilities instead of fake cpuid values for acpi
  x86/xen: use capabilities instead of fake cpuid values for mwait
  x86/xen: use capabilities instead of fake cpuid values for x2apic
  x86/xen: use capabilities instead of fake cpuid values for xsave
  vmware: set cpu capabilities during platform initialization
  x86/cpu: remove hypervisor specific set_cpu_features

 arch/x86/include/asm/hypervisor.h |  5 ---
 arch/x86/kernel/cpu/common.c      |  1 -
 arch/x86/kernel/cpu/hypervisor.c  |  8 ----
 arch/x86/kernel/cpu/vmware.c      | 39 ++++++++--------
 arch/x86/xen/enlighten_pv.c       | 95 +++++++++++++++++----------------------
 5 files changed, 61 insertions(+), 87 deletions(-)

-- 
2.12.0

[PATCH v3 01/11] xen: set cpu capabilities from xen_start_kernel()

[Juergen Gross]

There is no need to set the same capabilities for each cpu
individually. This can easily be done for all cpus when starting the
kernel.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 arch/x86/xen/enlighten_pv.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index b226f67fb741..6dc922e3848a 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -327,6 +327,12 @@ static void __init xen_init_cpuid_mask(void)
 		cpuid_leaf1_ecx_set_mask = (1 << (X86_FEATURE_MWAIT % 32));
 }
 
+static void __init xen_init_capabilities(void)
+{
+	setup_clear_cpu_cap(X86_BUG_SYSRET_SS_ATTRS);
+	setup_force_cpu_cap(X86_FEATURE_XENPV);
+}
+
 static void xen_set_debugreg(int reg, unsigned long val)
 {
 	HYPERVISOR_set_debugreg(reg, val);
@@ -1318,6 +1324,7 @@ asmlinkage __visible void __init xen_start_kernel(void)
 
 	xen_init_irq_ops();
 	xen_init_cpuid_mask();
+	xen_init_capabilities();
 
 #ifdef CONFIG_X86_LOCAL_APIC
 	/*
@@ -1506,16 +1513,9 @@ static uint32_t __init xen_platform_pv(void)
 	return 0;
 }
 
-static void xen_set_cpu_features(struct cpuinfo_x86 *c)
-{
-	clear_cpu_bug(c, X86_BUG_SYSRET_SS_ATTRS);
-	set_cpu_cap(c, X86_FEATURE_XENPV);
-}
-
 const struct hypervisor_x86 x86_hyper_xen_pv = {
 	.name                   = "Xen PV",
 	.detect                 = xen_platform_pv,
-	.set_cpu_features       = xen_set_cpu_features,
 	.pin_vcpu               = xen_pin_vcpu,
 };
 EXPORT_SYMBOL(x86_hyper_xen_pv);
-- 
2.12.0

[PATCH v3 02/11] x86/xen: don't indicate DCA support in pv domains

[Juergen Gross]

Xen doesn't support DCA (direct cache access) for pv domains. Clear
the corresponding capability indicator.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 arch/x86/xen/enlighten_pv.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 6dc922e3848a..bd69868909b4 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -331,6 +331,7 @@ static void __init xen_init_capabilities(void)
 {
 	setup_clear_cpu_cap(X86_BUG_SYSRET_SS_ATTRS);
 	setup_force_cpu_cap(X86_FEATURE_XENPV);
+	setup_clear_cpu_cap(X86_FEATURE_DCA);
 }
 
 static void xen_set_debugreg(int reg, unsigned long val)
-- 
2.12.0

[PATCH v3 03/11] x86/xen: use capabilities instead of fake cpuid values for aperf

[Juergen Gross]

When running as pv domain xen_cpuid() is being used instead of
native_cpuid(). In xen_cpuid() the aperf/mperf feature is indicated
as not being present by special casing the related cpuid leaf.

Instead of delivering fake cpuid values clear the cpu capability bit
for aperf/mperf instead.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 arch/x86/xen/enlighten_pv.c | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index bd69868909b4..4c8cd7278189 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -165,9 +165,6 @@ xen_running_on_version_or_later(unsigned int major, unsigned int minor)
 	return false;
 }
 
-#define CPUID_THERM_POWER_LEAF 6
-#define APERFMPERF_PRESENT 0
-
 static __read_mostly unsigned int cpuid_leaf1_edx_mask = ~0;
 static __read_mostly unsigned int cpuid_leaf1_ecx_mask = ~0;
 
@@ -201,11 +198,6 @@ static void xen_cpuid(unsigned int *ax, unsigned int *bx,
 		*dx = cpuid_leaf5_edx_val;
 		return;
 
-	case CPUID_THERM_POWER_LEAF:
-		/* Disabling APERFMPERF for kernel usage */
-		maskecx = ~(1 << APERFMPERF_PRESENT);
-		break;
-
 	case 0xb:
 		/* Suppress extended topology stuff */
 		maskebx = 0;
@@ -332,6 +324,7 @@ static void __init xen_init_capabilities(void)
 	setup_clear_cpu_cap(X86_BUG_SYSRET_SS_ATTRS);
 	setup_force_cpu_cap(X86_FEATURE_XENPV);
 	setup_clear_cpu_cap(X86_FEATURE_DCA);
+	setup_clear_cpu_cap(X86_FEATURE_APERFMPERF);
 }
 
 static void xen_set_debugreg(int reg, unsigned long val)
-- 
2.12.0

[PATCH v3 04/11] x86/xen: use capabilities instead of fake cpuid values for mtrr

[Juergen Gross]

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

Instead of delivering fake cpuid values clear the cpu capability bit
for mtrr instead.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 arch/x86/xen/enlighten_pv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 4c8cd7278189..37e174d9009b 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -295,8 +295,7 @@ static void __init xen_init_cpuid_mask(void)
 	unsigned int xsave_mask;
 
 	cpuid_leaf1_edx_mask =
-		~((1 << X86_FEATURE_MTRR) |  /* disable MTRR */
-		  (1 << X86_FEATURE_ACC));   /* thermal monitoring */
+		~(1 << X86_FEATURE_ACC);   /* thermal monitoring */
 
 	if (!xen_initial_domain())
 		cpuid_leaf1_edx_mask &=
@@ -325,6 +324,7 @@ static void __init xen_init_capabilities(void)
 	setup_force_cpu_cap(X86_FEATURE_XENPV);
 	setup_clear_cpu_cap(X86_FEATURE_DCA);
 	setup_clear_cpu_cap(X86_FEATURE_APERFMPERF);
+	setup_clear_cpu_cap(X86_FEATURE_MTRR);
 }
 
 static void xen_set_debugreg(int reg, unsigned long val)
-- 
2.12.0

[PATCH v3 05/11] x86/xen: use capabilities instead of fake cpuid values for acc

[Juergen Gross]

When running as pv domain xen_cpuid() is being used instead of
native_cpuid(). In xen_cpuid() the acc feature (thermal monitoring)
is indicated as not being present by special casing the related
cpuid leaf.

Instead of delivering fake cpuid values clear the cpu capability bit
for acc instead.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 arch/x86/xen/enlighten_pv.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 37e174d9009b..53fe97bd720f 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -294,9 +294,6 @@ static void __init xen_init_cpuid_mask(void)
 	unsigned int ax, bx, cx, dx;
 	unsigned int xsave_mask;
 
-	cpuid_leaf1_edx_mask =
-		~(1 << X86_FEATURE_ACC);   /* thermal monitoring */
-
 	if (!xen_initial_domain())
 		cpuid_leaf1_edx_mask &=
 			~((1 << X86_FEATURE_ACPI));  /* disable ACPI */
@@ -325,6 +322,7 @@ static void __init xen_init_capabilities(void)
 	setup_clear_cpu_cap(X86_FEATURE_DCA);
 	setup_clear_cpu_cap(X86_FEATURE_APERFMPERF);
 	setup_clear_cpu_cap(X86_FEATURE_MTRR);
+	setup_clear_cpu_cap(X86_FEATURE_ACC);
 }
 
 static void xen_set_debugreg(int reg, unsigned long val)
-- 
2.12.0

[PATCH v3 06/11] x86/xen: use capabilities instead of fake cpuid values for acpi

[Juergen Gross]

When running as pv domain xen_cpuid() is being used instead of
native_cpuid(). In xen_cpuid() the acpi feature is indicated as not
being present by special casing the related cpuid leaf in case we
are not the initial domain.

Instead of delivering fake cpuid values clear the cpu capability bit
for acpi instead.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 arch/x86/xen/enlighten_pv.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 53fe97bd720f..3196961862bc 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -165,7 +165,6 @@ xen_running_on_version_or_later(unsigned int major, unsigned int minor)
 	return false;
 }
 
-static __read_mostly unsigned int cpuid_leaf1_edx_mask = ~0;
 static __read_mostly unsigned int cpuid_leaf1_ecx_mask = ~0;
 
 static __read_mostly unsigned int cpuid_leaf1_ecx_set_mask;
@@ -177,7 +176,6 @@ static void xen_cpuid(unsigned int *ax, unsigned int *bx,
 {
 	unsigned maskebx = ~0;
 	unsigned maskecx = ~0;
-	unsigned maskedx = ~0;
 	unsigned setecx = 0;
 	/*
 	 * Mask out inconvenient features, to try and disable as many
@@ -187,7 +185,6 @@ static void xen_cpuid(unsigned int *ax, unsigned int *bx,
 	case 1:
 		maskecx = cpuid_leaf1_ecx_mask;
 		setecx = cpuid_leaf1_ecx_set_mask;
-		maskedx = cpuid_leaf1_edx_mask;
 		break;
 
 	case CPUID_MWAIT_LEAF:
@@ -214,7 +211,6 @@ static void xen_cpuid(unsigned int *ax, unsigned int *bx,
 	*bx &= maskebx;
 	*cx &= maskecx;
 	*cx |= setecx;
-	*dx &= maskedx;
 }
 STACK_FRAME_NON_STANDARD(xen_cpuid); /* XEN_EMULATE_PREFIX */
 
@@ -294,10 +290,6 @@ static void __init xen_init_cpuid_mask(void)
 	unsigned int ax, bx, cx, dx;
 	unsigned int xsave_mask;
 
-	if (!xen_initial_domain())
-		cpuid_leaf1_edx_mask &=
-			~((1 << X86_FEATURE_ACPI));  /* disable ACPI */
-
 	cpuid_leaf1_ecx_mask &= ~(1 << (X86_FEATURE_X2APIC % 32));
 
 	ax = 1;
@@ -323,6 +315,9 @@ static void __init xen_init_capabilities(void)
 	setup_clear_cpu_cap(X86_FEATURE_APERFMPERF);
 	setup_clear_cpu_cap(X86_FEATURE_MTRR);
 	setup_clear_cpu_cap(X86_FEATURE_ACC);
+
+	if (!xen_initial_domain())
+		setup_clear_cpu_cap(X86_FEATURE_ACPI);
 }
 
 static void xen_set_debugreg(int reg, unsigned long val)
-- 
2.12.0

[PATCH v3 07/11] x86/xen: use capabilities instead of fake cpuid values for mwait

[Juergen Gross]

When running as pv domain xen_cpuid() is being used instead of
native_cpuid(). In xen_cpuid() the mwait feature is indicated to be
present or not by special casing the related cpuid leaf.

Instead of delivering fake cpuid values use the cpu capability bit
for mwait instead.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 arch/x86/xen/enlighten_pv.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 3196961862bc..d43382284ada 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -167,7 +167,6 @@ xen_running_on_version_or_later(unsigned int major, unsigned int minor)
 
 static __read_mostly unsigned int cpuid_leaf1_ecx_mask = ~0;
 
-static __read_mostly unsigned int cpuid_leaf1_ecx_set_mask;
 static __read_mostly unsigned int cpuid_leaf5_ecx_val;
 static __read_mostly unsigned int cpuid_leaf5_edx_val;
 
@@ -176,7 +175,6 @@ static void xen_cpuid(unsigned int *ax, unsigned int *bx,
 {
 	unsigned maskebx = ~0;
 	unsigned maskecx = ~0;
-	unsigned setecx = 0;
 	/*
 	 * Mask out inconvenient features, to try and disable as many
 	 * unsupported kernel subsystems as possible.
@@ -184,7 +182,6 @@ static void xen_cpuid(unsigned int *ax, unsigned int *bx,
 	switch (*ax) {
 	case 1:
 		maskecx = cpuid_leaf1_ecx_mask;
-		setecx = cpuid_leaf1_ecx_set_mask;
 		break;
 
 	case CPUID_MWAIT_LEAF:
@@ -210,7 +207,6 @@ static void xen_cpuid(unsigned int *ax, unsigned int *bx,
 
 	*bx &= maskebx;
 	*cx &= maskecx;
-	*cx |= setecx;
 }
 STACK_FRAME_NON_STANDARD(xen_cpuid); /* XEN_EMULATE_PREFIX */
 
@@ -303,8 +299,6 @@ static void __init xen_init_cpuid_mask(void)
 	/* 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 */
-	if (xen_check_mwait())
-		cpuid_leaf1_ecx_set_mask = (1 << (X86_FEATURE_MWAIT % 32));
 }
 
 static void __init xen_init_capabilities(void)
@@ -318,6 +312,11 @@ static void __init xen_init_capabilities(void)
 
 	if (!xen_initial_domain())
 		setup_clear_cpu_cap(X86_FEATURE_ACPI);
+
+	if (xen_check_mwait())
+		setup_force_cpu_cap(X86_FEATURE_MWAIT);
+	else
+		setup_clear_cpu_cap(X86_FEATURE_MWAIT);
 }
 
 static void xen_set_debugreg(int reg, unsigned long val)
-- 
2.12.0

[PATCH v3 08/11] x86/xen: use capabilities instead of fake cpuid values for x2apic

[Juergen Gross]

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

Instead of delivering fake cpuid values clear the cpu capability bit
for x2apic instead.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 arch/x86/xen/enlighten_pv.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index d43382284ada..38dec28a8e6d 100644
--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -286,8 +286,6 @@ static void __init xen_init_cpuid_mask(void)
 	unsigned int ax, bx, cx, dx;
 	unsigned int xsave_mask;
 
-	cpuid_leaf1_ecx_mask &= ~(1 << (X86_FEATURE_X2APIC % 32));
-
 	ax = 1;
 	cx = 0;
 	cpuid(1, &ax, &bx, &cx, &dx);
@@ -309,6 +307,7 @@ static void __init xen_init_capabilities(void)
 	setup_clear_cpu_cap(X86_FEATURE_APERFMPERF);
 	setup_clear_cpu_cap(X86_FEATURE_MTRR);
 	setup_clear_cpu_cap(X86_FEATURE_ACC);
+	setup_clear_cpu_cap(X86_FEATURE_X2APIC);
 
 	if (!xen_initial_domain())
 		setup_clear_cpu_cap(X86_FEATURE_ACPI);
-- 
2.12.0

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

[Juergen Gross]

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 <jgross@suse.com>
---
 arch/x86/xen/enlighten_pv.c | 46 +++++++++++++++++++++++----------------------
 1 file changed, 24 insertions(+), 22 deletions(-)

diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c
index 38dec28a8e6d..e6bf71d76e10 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,24 @@ 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 err, eax, edx;
 
-	ax = 1;
-	cx = 0;
-	cpuid(1, &ax, &bx, &cx, &dx);
+	/* Test OSXSAVE capability via xgetbv instruction. */
+	asm volatile("1: .byte 0x0f,0x01,0xd0\n\t" /* xgetbv */
+		     "xor %[err], %[err]\n"
+		     "2:\n\t"
+		     ".pushsection .fixup,\"ax\"\n\t"
+		     "3: movl $1,%[err]\n\t"
+		     "jmp 2b\n\t"
+		     ".popsection\n\t"
+		     _ASM_EXTABLE(1b, 3b)
+		     : [err] "=r" (err), "=a" (eax), "=d" (edx)
+		     : "c" (0));
 
-	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 err == 0;
 }
 
 static void __init xen_init_capabilities(void)
@@ -316,6 +311,14 @@ 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_force_cpu_cap(X86_FEATURE_XSAVE);
+		setup_force_cpu_cap(X86_FEATURE_OSXSAVE);
+	} else {
+		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 +1311,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
-- 
2.12.0

Re: [Xen-devel] [PATCH v3 09/11] x86/xen: use capabilities instead of fake cpuid values for xsave

[Andrew Cooper]

On 18/04/17 07:31, Juergen Gross wrote:
> @@ -281,22 +274,24 @@ 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 err, eax, edx;
>  
> -	ax = 1;
> -	cx = 0;
> -	cpuid(1, &ax, &bx, &cx, &dx);
> +	/* Test OSXSAVE capability via xgetbv instruction. */

The code is fine, but this comment isn't going to be any help to people
reading this code in 6 months time.

How about this:

"Xen 4.0 and older accidentally leaked the host XSAVE flag into guest
view, despite not being able to support guests using the functionality. 
Probe for the actual availability of XSAVE by seeing whether xgetbv
executes successfully or raises #UD."

Everything else is fine, so Reviewed-by: Andrew Cooper
<andrew.cooper3@citrix.com>

~Andrew

Re: [Xen-devel] [PATCH v3 09/11] x86/xen: use capabilities instead of fake cpuid values for xsave

[Juergen Gross]

On 18/04/17 12:02, Andrew Cooper wrote:
> On 18/04/17 07:31, Juergen Gross wrote:
>> @@ -281,22 +274,24 @@ 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 err, eax, edx;
>>  
>> -	ax = 1;
>> -	cx = 0;
>> -	cpuid(1, &ax, &bx, &cx, &dx);
>> +	/* Test OSXSAVE capability via xgetbv instruction. */
> 
> The code is fine, but this comment isn't going to be any help to people
> reading this code in 6 months time.
> 
> How about this:
> 
> "Xen 4.0 and older accidentally leaked the host XSAVE flag into guest
> view, despite not being able to support guests using the functionality. 
> Probe for the actual availability of XSAVE by seeing whether xgetbv
> executes successfully or raises #UD."

I'll update the comment.

> Everything else is fine, so Reviewed-by: Andrew Cooper
> <andrew.cooper3@citrix.com>

Thanks,

Juergen

Re: [PATCH v3 09/11] x86/xen: use capabilities instead of fake cpuid values for xsave

[Boris Ostrovsky]

> +static bool __init xen_check_xsave(void)
>  {
> -	unsigned int ax, bx, cx, dx;
> -	unsigned int xsave_mask;
> +	unsigned int err, eax, edx;
>  
> -	ax = 1;
> -	cx = 0;
> -	cpuid(1, &ax, &bx, &cx, &dx);
> +	/* Test OSXSAVE capability via xgetbv instruction. */
> +	asm volatile("1: .byte 0x0f,0x01,0xd0\n\t" /* xgetbv */
> +		     "xor %[err], %[err]\n"
> +		     "2:\n\t"
> +		     ".pushsection .fixup,\"ax\"\n\t"
> +		     "3: movl $1,%[err]\n\t"
> +		     "jmp 2b\n\t"
> +		     ".popsection\n\t"
> +		     _ASM_EXTABLE(1b, 3b)
> +		     : [err] "=r" (err), "=a" (eax), "=d" (edx)
> +		     : "c" (0));

Have you tested this on processors where we actually trap on xgetbv?

I have an AMD box without XSAVE support and this is a fatal error. I
suspect it's too early to use exception fixup framework here.

-boris

Re: [PATCH v3 09/11] x86/xen: use capabilities instead of fake cpuid values for xsave

[Juergen Gross]

On 21/04/17 16:24, Boris Ostrovsky wrote:
> 
>> +static bool __init xen_check_xsave(void)
>>  {
>> -	unsigned int ax, bx, cx, dx;
>> -	unsigned int xsave_mask;
>> +	unsigned int err, eax, edx;
>>  
>> -	ax = 1;
>> -	cx = 0;
>> -	cpuid(1, &ax, &bx, &cx, &dx);
>> +	/* Test OSXSAVE capability via xgetbv instruction. */
>> +	asm volatile("1: .byte 0x0f,0x01,0xd0\n\t" /* xgetbv */
>> +		     "xor %[err], %[err]\n"
>> +		     "2:\n\t"
>> +		     ".pushsection .fixup,\"ax\"\n\t"
>> +		     "3: movl $1,%[err]\n\t"
>> +		     "jmp 2b\n\t"
>> +		     ".popsection\n\t"
>> +		     _ASM_EXTABLE(1b, 3b)
>> +		     : [err] "=r" (err), "=a" (eax), "=d" (edx)
>> +		     : "c" (0));
> 
> Have you tested this on processors where we actually trap on xgetbv?
> 
> I have an AMD box without XSAVE support and this is a fatal error. I
> suspect it's too early to use exception fixup framework here.

Uuh, too bad.

Then I fear we must use the other solution Andrew didn't like. :-(
Andrew, would you be okay with that?


Juergen

Re: [PATCH v3 09/11] x86/xen: use capabilities instead of fake cpuid values for xsave

[Andrew Cooper]

On 21/04/17 15:38, Juergen Gross wrote:
> On 21/04/17 16:24, Boris Ostrovsky wrote:
>>> +static bool __init xen_check_xsave(void)
>>>  {
>>> -	unsigned int ax, bx, cx, dx;
>>> -	unsigned int xsave_mask;
>>> +	unsigned int err, eax, edx;
>>>  
>>> -	ax = 1;
>>> -	cx = 0;
>>> -	cpuid(1, &ax, &bx, &cx, &dx);
>>> +	/* Test OSXSAVE capability via xgetbv instruction. */
>>> +	asm volatile("1: .byte 0x0f,0x01,0xd0\n\t" /* xgetbv */
>>> +		     "xor %[err], %[err]\n"
>>> +		     "2:\n\t"
>>> +		     ".pushsection .fixup,\"ax\"\n\t"
>>> +		     "3: movl $1,%[err]\n\t"
>>> +		     "jmp 2b\n\t"
>>> +		     ".popsection\n\t"
>>> +		     _ASM_EXTABLE(1b, 3b)
>>> +		     : [err] "=r" (err), "=a" (eax), "=d" (edx)
>>> +		     : "c" (0));
>> Have you tested this on processors where we actually trap on xgetbv?
>>
>> I have an AMD box without XSAVE support and this is a fatal error. I
>> suspect it's too early to use exception fixup framework here.
> Uuh, too bad.
>
> Then I fear we must use the other solution Andrew didn't like. :-(
> Andrew, would you be okay with that?

Hmm fine.  The status quo is probably best then to unblock this series.

As an independent question, why are exceptions set up so late?  They
really should be the very first thing done.

~Andrew

Re: [PATCH v3 09/11] x86/xen: use capabilities instead of fake cpuid values for xsave

[Boris Ostrovsky]

On 04/21/2017 10:45 AM, Andrew Cooper wrote:
> On 21/04/17 15:38, Juergen Gross wrote:
>> On 21/04/17 16:24, Boris Ostrovsky wrote:
>>>> +static bool __init xen_check_xsave(void)
>>>>  {
>>>> -	unsigned int ax, bx, cx, dx;
>>>> -	unsigned int xsave_mask;
>>>> +	unsigned int err, eax, edx;
>>>>  
>>>> -	ax = 1;
>>>> -	cx = 0;
>>>> -	cpuid(1, &ax, &bx, &cx, &dx);
>>>> +	/* Test OSXSAVE capability via xgetbv instruction. */
>>>> +	asm volatile("1: .byte 0x0f,0x01,0xd0\n\t" /* xgetbv */
>>>> +		     "xor %[err], %[err]\n"
>>>> +		     "2:\n\t"
>>>> +		     ".pushsection .fixup,\"ax\"\n\t"
>>>> +		     "3: movl $1,%[err]\n\t"
>>>> +		     "jmp 2b\n\t"
>>>> +		     ".popsection\n\t"
>>>> +		     _ASM_EXTABLE(1b, 3b)
>>>> +		     : [err] "=r" (err), "=a" (eax), "=d" (edx)
>>>> +		     : "c" (0));
>>> Have you tested this on processors where we actually trap on xgetbv?
>>>
>>> I have an AMD box without XSAVE support and this is a fatal error. I
>>> suspect it's too early to use exception fixup framework here.
>> Uuh, too bad.
>>
>> Then I fear we must use the other solution Andrew didn't like. :-(
>> Andrew, would you be okay with that?
> Hmm fine.  The status quo is probably best then to unblock this series.
>
> As an independent question, why are exceptions set up so late?  They
> really should be the very first thing done

It's exception fixup that is not set up yet --- we are executing here
before "main" kernel's entry point.

This is somewhat similar to what
arch/x86/kernel/head_64.S:early_idt_handler_common() does --- it has
special handling for early fixup --- early_fixup_exception().

I wonder though --- can this feature masking be deferred until a bit later?

-boris

Re: [PATCH v3 09/11] x86/xen: use capabilities instead of fake cpuid values for xsave

[Juergen Gross]

On 21/04/17 16:51, Boris Ostrovsky wrote:
> On 04/21/2017 10:45 AM, Andrew Cooper wrote:
>> On 21/04/17 15:38, Juergen Gross wrote:
>>> On 21/04/17 16:24, Boris Ostrovsky wrote:
>>>>> +static bool __init xen_check_xsave(void)
>>>>>  {
>>>>> -	unsigned int ax, bx, cx, dx;
>>>>> -	unsigned int xsave_mask;
>>>>> +	unsigned int err, eax, edx;
>>>>>  
>>>>> -	ax = 1;
>>>>> -	cx = 0;
>>>>> -	cpuid(1, &ax, &bx, &cx, &dx);
>>>>> +	/* Test OSXSAVE capability via xgetbv instruction. */
>>>>> +	asm volatile("1: .byte 0x0f,0x01,0xd0\n\t" /* xgetbv */
>>>>> +		     "xor %[err], %[err]\n"
>>>>> +		     "2:\n\t"
>>>>> +		     ".pushsection .fixup,\"ax\"\n\t"
>>>>> +		     "3: movl $1,%[err]\n\t"
>>>>> +		     "jmp 2b\n\t"
>>>>> +		     ".popsection\n\t"
>>>>> +		     _ASM_EXTABLE(1b, 3b)
>>>>> +		     : [err] "=r" (err), "=a" (eax), "=d" (edx)
>>>>> +		     : "c" (0));
>>>> Have you tested this on processors where we actually trap on xgetbv?
>>>>
>>>> I have an AMD box without XSAVE support and this is a fatal error. I
>>>> suspect it's too early to use exception fixup framework here.
>>> Uuh, too bad.
>>>
>>> Then I fear we must use the other solution Andrew didn't like. :-(
>>> Andrew, would you be okay with that?
>> Hmm fine.  The status quo is probably best then to unblock this series.
>>
>> As an independent question, why are exceptions set up so late?  They
>> really should be the very first thing done
> 
> It's exception fixup that is not set up yet --- we are executing here
> before "main" kernel's entry point.
> 
> This is somewhat similar to what
> arch/x86/kernel/head_64.S:early_idt_handler_common() does --- it has
> special handling for early fixup --- early_fixup_exception().
> 
> I wonder though --- can this feature masking be deferred until a bit later?

At least the xsave feature is tested rather early: it is needed in
early_cpu_init() being called way before trap_init().


Juergen

[PATCH v3 10/11] vmware: set cpu capabilities during platform initialization

[Juergen Gross]

There is no need to set the same capabilities for each cpu
individually. This can be done for all cpus in platform initialization.

Cc: Alok Kataria <akataria@vmware.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Cc: virtualization@lists.linux-foundation.org
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
Acked-by: Alok Kataria <akataria@vmware.com>
---
 arch/x86/kernel/cpu/vmware.c | 39 ++++++++++++++++++++-------------------
 1 file changed, 20 insertions(+), 19 deletions(-)

diff --git a/arch/x86/kernel/cpu/vmware.c b/arch/x86/kernel/cpu/vmware.c
index 22403a28caf5..40ed26852ebd 100644
--- a/arch/x86/kernel/cpu/vmware.c
+++ b/arch/x86/kernel/cpu/vmware.c
@@ -113,6 +113,24 @@ static void __init vmware_paravirt_ops_setup(void)
 #define vmware_paravirt_ops_setup() do {} while (0)
 #endif
 
+/*
+ * VMware hypervisor takes care of exporting a reliable TSC to the guest.
+ * Still, due to timing difference when running on virtual cpus, the TSC can
+ * be marked as unstable in some cases. For example, the TSC sync check at
+ * bootup can fail due to a marginal offset between vcpus' TSCs (though the
+ * TSCs do not drift from each other).  Also, the ACPI PM timer clocksource
+ * is not suitable as a watchdog when running on a hypervisor because the
+ * kernel may miss a wrap of the counter if the vcpu is descheduled for a
+ * long time. To skip these checks at runtime we set these capability bits,
+ * so that the kernel could just trust the hypervisor with providing a
+ * reliable virtual TSC that is suitable for timekeeping.
+ */
+static void __init vmware_set_capabilities(void)
+{
+	setup_force_cpu_cap(X86_FEATURE_CONSTANT_TSC);
+	setup_force_cpu_cap(X86_FEATURE_TSC_RELIABLE);
+}
+
 static void __init vmware_platform_setup(void)
 {
 	uint32_t eax, ebx, ecx, edx;
@@ -152,6 +170,8 @@ static void __init vmware_platform_setup(void)
 #ifdef CONFIG_X86_IO_APIC
 	no_timer_check = 1;
 #endif
+
+	vmware_set_capabilities();
 }
 
 /*
@@ -176,24 +196,6 @@ static uint32_t __init vmware_platform(void)
 	return 0;
 }
 
-/*
- * VMware hypervisor takes care of exporting a reliable TSC to the guest.
- * Still, due to timing difference when running on virtual cpus, the TSC can
- * be marked as unstable in some cases. For example, the TSC sync check at
- * bootup can fail due to a marginal offset between vcpus' TSCs (though the
- * TSCs do not drift from each other).  Also, the ACPI PM timer clocksource
- * is not suitable as a watchdog when running on a hypervisor because the
- * kernel may miss a wrap of the counter if the vcpu is descheduled for a
- * long time. To skip these checks at runtime we set these capability bits,
- * so that the kernel could just trust the hypervisor with providing a
- * reliable virtual TSC that is suitable for timekeeping.
- */
-static void vmware_set_cpu_features(struct cpuinfo_x86 *c)
-{
-	set_cpu_cap(c, X86_FEATURE_CONSTANT_TSC);
-	set_cpu_cap(c, X86_FEATURE_TSC_RELIABLE);
-}
-
 /* Checks if hypervisor supports x2apic without VT-D interrupt remapping. */
 static bool __init vmware_legacy_x2apic_available(void)
 {
@@ -206,7 +208,6 @@ static bool __init vmware_legacy_x2apic_available(void)
 const __refconst struct hypervisor_x86 x86_hyper_vmware = {
 	.name			= "VMware",
 	.detect			= vmware_platform,
-	.set_cpu_features	= vmware_set_cpu_features,
 	.init_platform		= vmware_platform_setup,
 	.x2apic_available	= vmware_legacy_x2apic_available,
 };
-- 
2.12.0

[PATCH v3 11/11] x86/cpu: remove hypervisor specific set_cpu_features

[Juergen Gross]

There is no user of x86_hyper->set_cpu_features() any more. Remove it.

Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com>
---
 arch/x86/include/asm/hypervisor.h | 5 -----
 arch/x86/kernel/cpu/common.c      | 1 -
 arch/x86/kernel/cpu/hypervisor.c  | 8 --------
 3 files changed, 14 deletions(-)

diff --git a/arch/x86/include/asm/hypervisor.h b/arch/x86/include/asm/hypervisor.h
index 6f7545c6e9cf..21126155a739 100644
--- a/arch/x86/include/asm/hypervisor.h
+++ b/arch/x86/include/asm/hypervisor.h
@@ -35,9 +35,6 @@ struct hypervisor_x86 {
 	/* Detection routine */
 	uint32_t	(*detect)(void);
 
-	/* Adjust CPU feature bits (run once per CPU) */
-	void		(*set_cpu_features)(struct cpuinfo_x86 *);
-
 	/* Platform setup (run once per boot) */
 	void		(*init_platform)(void);
 
@@ -57,12 +54,10 @@ extern const struct hypervisor_x86 x86_hyper_xen_pv;
 extern const struct hypervisor_x86 x86_hyper_xen_hvm;
 extern const struct hypervisor_x86 x86_hyper_kvm;
 
-extern void init_hypervisor(struct cpuinfo_x86 *c);
 extern void init_hypervisor_platform(void);
 extern bool hypervisor_x2apic_available(void);
 extern void hypervisor_pin_vcpu(int cpu);
 #else
-static inline void init_hypervisor(struct cpuinfo_x86 *c) { }
 static inline void init_hypervisor_platform(void) { }
 static inline bool hypervisor_x2apic_available(void) { return false; }
 #endif /* CONFIG_HYPERVISOR_GUEST */
diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 8ee32119144d..c8b39870f33e 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -1149,7 +1149,6 @@ static void identify_cpu(struct cpuinfo_x86 *c)
 	detect_ht(c);
 #endif
 
-	init_hypervisor(c);
 	x86_init_rdrand(c);
 	x86_init_cache_qos(c);
 	setup_pku(c);
diff --git a/arch/x86/kernel/cpu/hypervisor.c b/arch/x86/kernel/cpu/hypervisor.c
index ce6fcc30f2ad..4fa90006ac68 100644
--- a/arch/x86/kernel/cpu/hypervisor.c
+++ b/arch/x86/kernel/cpu/hypervisor.c
@@ -63,12 +63,6 @@ detect_hypervisor_vendor(void)
 		pr_info("Hypervisor detected: %s\n", x86_hyper->name);
 }
 
-void init_hypervisor(struct cpuinfo_x86 *c)
-{
-	if (x86_hyper && x86_hyper->set_cpu_features)
-		x86_hyper->set_cpu_features(c);
-}
-
 void __init init_hypervisor_platform(void)
 {
 
@@ -77,8 +71,6 @@ void __init init_hypervisor_platform(void)
 	if (!x86_hyper)
 		return;
 
-	init_hypervisor(&boot_cpu_data);
-
 	if (x86_hyper->init_platform)
 		x86_hyper->init_platform();
 }
-- 
2.12.0