* [PATCH 01/10] x86/spec_ctrl: Read MSR_ARCH_CAPABILITIES only once
2018-05-11 10:38 [PATCH for-4.11 00/10] x86: Improvements and fixes to Spectre handling Andrew Cooper
@ 2018-05-11 10:38 ` Andrew Cooper
2018-05-11 14:32 ` Konrad Rzeszutek Wilk
2018-05-14 9:23 ` Wei Liu
2018-05-11 10:38 ` [PATCH 02/10] x86/spec_ctrl: Express Xen's choice of MSR_SPEC_CTRL value as a variable Andrew Cooper
` (11 subsequent siblings)
12 siblings, 2 replies; 34+ messages in thread
From: Andrew Cooper @ 2018-05-11 10:38 UTC (permalink / raw)
To: Xen-devel
Cc: Juergen Gross, Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné
Make it available from the beginning of init_speculation_mitigations(), and
pass it into appropriate functions. Fix an RSBA typo while moving the
affected comment.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Juergen Gross <jgross@suse.com>
---
xen/arch/x86/spec_ctrl.c | 34 ++++++++++++++--------------------
1 file changed, 14 insertions(+), 20 deletions(-)
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 037e84d..4ab0f50 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -97,18 +97,15 @@ static int __init parse_bti(const char *s)
}
custom_param("bti", parse_bti);
-static void __init print_details(enum ind_thunk thunk)
+static void __init print_details(enum ind_thunk thunk, uint64_t caps)
{
unsigned int _7d0 = 0, e8b = 0, tmp;
- uint64_t caps = 0;
/* Collect diagnostics about available mitigations. */
if ( boot_cpu_data.cpuid_level >= 7 )
cpuid_count(7, 0, &tmp, &tmp, &tmp, &_7d0);
if ( boot_cpu_data.extended_cpuid_level >= 0x80000008 )
cpuid(0x80000008, &tmp, &e8b, &tmp, &tmp);
- if ( _7d0 & cpufeat_mask(X86_FEATURE_ARCH_CAPS) )
- rdmsrl(MSR_ARCH_CAPABILITIES, caps);
printk(XENLOG_DEBUG "Speculative mitigation facilities:\n");
@@ -142,7 +139,7 @@ static void __init print_details(enum ind_thunk thunk)
}
/* Calculate whether Retpoline is known-safe on this CPU. */
-static bool __init retpoline_safe(void)
+static bool __init retpoline_safe(uint64_t caps)
{
unsigned int ucode_rev = this_cpu(ucode_cpu_info).cpu_sig.rev;
@@ -153,19 +150,12 @@ static bool __init retpoline_safe(void)
boot_cpu_data.x86 != 6 )
return false;
- if ( boot_cpu_has(X86_FEATURE_ARCH_CAPS) )
- {
- uint64_t caps;
-
- rdmsrl(MSR_ARCH_CAPABILITIES, caps);
-
- /*
- * RBSA may be set by a hypervisor to indicate that we may move to a
- * processor which isn't retpoline-safe.
- */
- if ( caps & ARCH_CAPS_RSBA )
- return false;
- }
+ /*
+ * RSBA may be set by a hypervisor to indicate that we may move to a
+ * processor which isn't retpoline-safe.
+ */
+ if ( caps & ARCH_CAPS_RSBA )
+ return false;
switch ( boot_cpu_data.x86_model )
{
@@ -299,6 +289,10 @@ void __init init_speculation_mitigations(void)
{
enum ind_thunk thunk = THUNK_DEFAULT;
bool ibrs = false;
+ uint64_t caps = 0;
+
+ if ( boot_cpu_has(X86_FEATURE_ARCH_CAPS) )
+ rdmsrl(MSR_ARCH_CAPABILITIES, caps);
/*
* Has the user specified any custom BTI mitigations? If so, follow their
@@ -327,7 +321,7 @@ void __init init_speculation_mitigations(void)
* On Intel hardware, we'd like to use retpoline in preference to
* IBRS, but only if it is safe on this hardware.
*/
- else if ( retpoline_safe() )
+ else if ( retpoline_safe(caps) )
thunk = THUNK_RETPOLINE;
else if ( boot_cpu_has(X86_FEATURE_IBRSB) )
ibrs = true;
@@ -418,7 +412,7 @@ void __init init_speculation_mitigations(void)
else
setup_clear_cpu_cap(X86_FEATURE_NO_XPTI);
- print_details(thunk);
+ print_details(thunk, caps);
}
static void __init __maybe_unused build_assertions(void)
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 01/10] x86/spec_ctrl: Read MSR_ARCH_CAPABILITIES only once
2018-05-11 10:38 ` [PATCH 01/10] x86/spec_ctrl: Read MSR_ARCH_CAPABILITIES only once Andrew Cooper
@ 2018-05-11 14:32 ` Konrad Rzeszutek Wilk
2018-05-14 9:23 ` Wei Liu
1 sibling, 0 replies; 34+ messages in thread
From: Konrad Rzeszutek Wilk @ 2018-05-11 14:32 UTC (permalink / raw)
To: Andrew Cooper
Cc: Juergen Gross, Roger Pau Monné, Wei Liu, Jan Beulich, Xen-devel
On Fri, May 11, 2018 at 11:38:05AM +0100, Andrew Cooper wrote:
> Make it available from the beginning of init_speculation_mitigations(), and
> pass it into appropriate functions. Fix an RSBA typo while moving the
> affected comment.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Thank you!
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Juergen Gross <jgross@suse.com>
> ---
> xen/arch/x86/spec_ctrl.c | 34 ++++++++++++++--------------------
> 1 file changed, 14 insertions(+), 20 deletions(-)
>
> diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
> index 037e84d..4ab0f50 100644
> --- a/xen/arch/x86/spec_ctrl.c
> +++ b/xen/arch/x86/spec_ctrl.c
> @@ -97,18 +97,15 @@ static int __init parse_bti(const char *s)
> }
> custom_param("bti", parse_bti);
>
> -static void __init print_details(enum ind_thunk thunk)
> +static void __init print_details(enum ind_thunk thunk, uint64_t caps)
> {
> unsigned int _7d0 = 0, e8b = 0, tmp;
> - uint64_t caps = 0;
>
> /* Collect diagnostics about available mitigations. */
> if ( boot_cpu_data.cpuid_level >= 7 )
> cpuid_count(7, 0, &tmp, &tmp, &tmp, &_7d0);
> if ( boot_cpu_data.extended_cpuid_level >= 0x80000008 )
> cpuid(0x80000008, &tmp, &e8b, &tmp, &tmp);
> - if ( _7d0 & cpufeat_mask(X86_FEATURE_ARCH_CAPS) )
> - rdmsrl(MSR_ARCH_CAPABILITIES, caps);
>
> printk(XENLOG_DEBUG "Speculative mitigation facilities:\n");
>
> @@ -142,7 +139,7 @@ static void __init print_details(enum ind_thunk thunk)
> }
>
> /* Calculate whether Retpoline is known-safe on this CPU. */
> -static bool __init retpoline_safe(void)
> +static bool __init retpoline_safe(uint64_t caps)
> {
> unsigned int ucode_rev = this_cpu(ucode_cpu_info).cpu_sig.rev;
>
> @@ -153,19 +150,12 @@ static bool __init retpoline_safe(void)
> boot_cpu_data.x86 != 6 )
> return false;
>
> - if ( boot_cpu_has(X86_FEATURE_ARCH_CAPS) )
> - {
> - uint64_t caps;
> -
> - rdmsrl(MSR_ARCH_CAPABILITIES, caps);
> -
> - /*
> - * RBSA may be set by a hypervisor to indicate that we may move to a
> - * processor which isn't retpoline-safe.
> - */
> - if ( caps & ARCH_CAPS_RSBA )
> - return false;
> - }
> + /*
> + * RSBA may be set by a hypervisor to indicate that we may move to a
> + * processor which isn't retpoline-safe.
> + */
> + if ( caps & ARCH_CAPS_RSBA )
> + return false;
>
> switch ( boot_cpu_data.x86_model )
> {
> @@ -299,6 +289,10 @@ void __init init_speculation_mitigations(void)
> {
> enum ind_thunk thunk = THUNK_DEFAULT;
> bool ibrs = false;
> + uint64_t caps = 0;
> +
> + if ( boot_cpu_has(X86_FEATURE_ARCH_CAPS) )
> + rdmsrl(MSR_ARCH_CAPABILITIES, caps);
>
> /*
> * Has the user specified any custom BTI mitigations? If so, follow their
> @@ -327,7 +321,7 @@ void __init init_speculation_mitigations(void)
> * On Intel hardware, we'd like to use retpoline in preference to
> * IBRS, but only if it is safe on this hardware.
> */
> - else if ( retpoline_safe() )
> + else if ( retpoline_safe(caps) )
> thunk = THUNK_RETPOLINE;
> else if ( boot_cpu_has(X86_FEATURE_IBRSB) )
> ibrs = true;
> @@ -418,7 +412,7 @@ void __init init_speculation_mitigations(void)
> else
> setup_clear_cpu_cap(X86_FEATURE_NO_XPTI);
>
> - print_details(thunk);
> + print_details(thunk, caps);
> }
>
> static void __init __maybe_unused build_assertions(void)
> --
> 2.1.4
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xenproject.org
> https://lists.xenproject.org/mailman/listinfo/xen-devel
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 01/10] x86/spec_ctrl: Read MSR_ARCH_CAPABILITIES only once
2018-05-11 10:38 ` [PATCH 01/10] x86/spec_ctrl: Read MSR_ARCH_CAPABILITIES only once Andrew Cooper
2018-05-11 14:32 ` Konrad Rzeszutek Wilk
@ 2018-05-14 9:23 ` Wei Liu
1 sibling, 0 replies; 34+ messages in thread
From: Wei Liu @ 2018-05-14 9:23 UTC (permalink / raw)
To: Andrew Cooper
Cc: Juergen Gross, Roger Pau Monné, Wei Liu, Jan Beulich, Xen-devel
On Fri, May 11, 2018 at 11:38:05AM +0100, Andrew Cooper wrote:
> Make it available from the beginning of init_speculation_mitigations(), and
> pass it into appropriate functions. Fix an RSBA typo while moving the
> affected comment.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 02/10] x86/spec_ctrl: Express Xen's choice of MSR_SPEC_CTRL value as a variable
2018-05-11 10:38 [PATCH for-4.11 00/10] x86: Improvements and fixes to Spectre handling Andrew Cooper
2018-05-11 10:38 ` [PATCH 01/10] x86/spec_ctrl: Read MSR_ARCH_CAPABILITIES only once Andrew Cooper
@ 2018-05-11 10:38 ` Andrew Cooper
2018-05-14 10:15 ` Wei Liu
2018-05-11 10:38 ` [PATCH 03/10] x86/spec_ctrl: Merge bti_ist_info and use_shadow_spec_ctrl into spec_ctrl_flags Andrew Cooper
` (10 subsequent siblings)
12 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2018-05-11 10:38 UTC (permalink / raw)
To: Xen-devel
Cc: Juergen Gross, Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné
At the moment, we have two different encodings of Xen's MSR_SPEC_CTRL value,
which is a side effect of how the Spectre series developed. One encoding is
via an alias with the bottom bit of bti_ist_info, and can encode IBRS or not,
but not other configuraitons such as STIBP.
Break Xen's value out into a separate variable (in the top of stack block for
XPTI reasons) and use this instead of bti_ist_info in the IST path.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Juergen Gross <jgross@suse.com>
---
xen/arch/x86/spec_ctrl.c | 8 +++++---
xen/arch/x86/x86_64/asm-offsets.c | 1 +
xen/include/asm-x86/current.h | 1 +
xen/include/asm-x86/spec_ctrl.h | 2 ++
xen/include/asm-x86/spec_ctrl_asm.h | 8 ++------
5 files changed, 11 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 4ab0f50..6633c64 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -38,6 +38,7 @@ static int8_t __initdata opt_ibrs = -1;
static bool __initdata opt_rsb_native = true;
static bool __initdata opt_rsb_vmexit = true;
bool __read_mostly opt_ibpb = true;
+uint8_t __read_mostly default_xen_spec_ctrl;
uint8_t __read_mostly default_bti_ist_info;
static int __init parse_bti(const char *s)
@@ -366,11 +367,14 @@ void __init init_speculation_mitigations(void)
* guests.
*/
if ( ibrs )
+ {
+ default_xen_spec_ctrl |= SPEC_CTRL_IBRS;
setup_force_cpu_cap(X86_FEATURE_XEN_IBRS_SET);
+ }
else
setup_force_cpu_cap(X86_FEATURE_XEN_IBRS_CLEAR);
- default_bti_ist_info |= BTI_IST_WRMSR | ibrs;
+ default_bti_ist_info |= BTI_IST_WRMSR;
}
/*
@@ -417,8 +421,6 @@ void __init init_speculation_mitigations(void)
static void __init __maybe_unused build_assertions(void)
{
- /* The optimised assembly relies on this alias. */
- BUILD_BUG_ON(BTI_IST_IBRS != SPEC_CTRL_IBRS);
}
/*
diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c
index 06028fe..f80d3b7 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -134,6 +134,7 @@ void __dummy__(void)
OFFSET(CPUINFO_xen_cr3, struct cpu_info, xen_cr3);
OFFSET(CPUINFO_pv_cr3, struct cpu_info, pv_cr3);
OFFSET(CPUINFO_shadow_spec_ctrl, struct cpu_info, shadow_spec_ctrl);
+ OFFSET(CPUINFO_xen_spec_ctrl, struct cpu_info, xen_spec_ctrl);
OFFSET(CPUINFO_use_shadow_spec_ctrl, struct cpu_info, use_shadow_spec_ctrl);
OFFSET(CPUINFO_bti_ist_info, struct cpu_info, bti_ist_info);
OFFSET(CPUINFO_root_pgt_changed, struct cpu_info, root_pgt_changed);
diff --git a/xen/include/asm-x86/current.h b/xen/include/asm-x86/current.h
index 43bdec1..200e935 100644
--- a/xen/include/asm-x86/current.h
+++ b/xen/include/asm-x86/current.h
@@ -54,6 +54,7 @@ struct cpu_info {
/* See asm-x86/spec_ctrl_asm.h for usage. */
unsigned int shadow_spec_ctrl;
+ uint8_t xen_spec_ctrl;
bool use_shadow_spec_ctrl;
uint8_t bti_ist_info;
diff --git a/xen/include/asm-x86/spec_ctrl.h b/xen/include/asm-x86/spec_ctrl.h
index b4fa432..0c7663a 100644
--- a/xen/include/asm-x86/spec_ctrl.h
+++ b/xen/include/asm-x86/spec_ctrl.h
@@ -27,6 +27,7 @@
void init_speculation_mitigations(void);
extern bool opt_ibpb;
+extern uint8_t default_xen_spec_ctrl;
extern uint8_t default_bti_ist_info;
extern uint8_t opt_xpti;
@@ -38,6 +39,7 @@ static inline void init_shadow_spec_ctrl_state(void)
struct cpu_info *info = get_cpu_info();
info->shadow_spec_ctrl = info->use_shadow_spec_ctrl = 0;
+ info->xen_spec_ctrl = default_xen_spec_ctrl;
info->bti_ist_info = default_bti_ist_info;
}
diff --git a/xen/include/asm-x86/spec_ctrl_asm.h b/xen/include/asm-x86/spec_ctrl_asm.h
index 1623fc0..e8e8f9a 100644
--- a/xen/include/asm-x86/spec_ctrl_asm.h
+++ b/xen/include/asm-x86/spec_ctrl_asm.h
@@ -21,7 +21,6 @@
#define __X86_SPEC_CTRL_ASM_H__
/* Encoding of the bottom bits in cpuinfo.bti_ist_info */
-#define BTI_IST_IBRS (1 << 0)
#define BTI_IST_WRMSR (1 << 1)
#define BTI_IST_RSB (1 << 2)
@@ -283,12 +282,9 @@
setz %dl
and %dl, STACK_CPUINFO_FIELD(use_shadow_spec_ctrl)(%r14)
- /*
- * Load Xen's intended value. SPEC_CTRL_IBRS vs 0 is encoded in the
- * bottom bit of bti_ist_info, via a deliberate alias with BTI_IST_IBRS.
- */
+ /* Load Xen's intended value. */
mov $MSR_SPEC_CTRL, %ecx
- and $BTI_IST_IBRS, %eax
+ movzbl STACK_CPUINFO_FIELD(xen_spec_ctrl)(%r14), %eax
xor %edx, %edx
wrmsr
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 02/10] x86/spec_ctrl: Express Xen's choice of MSR_SPEC_CTRL value as a variable
2018-05-11 10:38 ` [PATCH 02/10] x86/spec_ctrl: Express Xen's choice of MSR_SPEC_CTRL value as a variable Andrew Cooper
@ 2018-05-14 10:15 ` Wei Liu
0 siblings, 0 replies; 34+ messages in thread
From: Wei Liu @ 2018-05-14 10:15 UTC (permalink / raw)
To: Andrew Cooper
Cc: Juergen Gross, Roger Pau Monné, Wei Liu, Jan Beulich, Xen-devel
On Fri, May 11, 2018 at 11:38:06AM +0100, Andrew Cooper wrote:
> At the moment, we have two different encodings of Xen's MSR_SPEC_CTRL value,
> which is a side effect of how the Spectre series developed. One encoding is
> via an alias with the bottom bit of bti_ist_info, and can encode IBRS or not,
> but not other configuraitons such as STIBP.
>
> Break Xen's value out into a separate variable (in the top of stack block for
> XPTI reasons) and use this instead of bti_ist_info in the IST path.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
>
> /*
> @@ -417,8 +421,6 @@ void __init init_speculation_mitigations(void)
>
> static void __init __maybe_unused build_assertions(void)
> {
> - /* The optimised assembly relies on this alias. */
> - BUILD_BUG_ON(BTI_IST_IBRS != SPEC_CTRL_IBRS);
I was about to suggest removing this function entirely but it is in fact
filled in later, so this is fine.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 03/10] x86/spec_ctrl: Merge bti_ist_info and use_shadow_spec_ctrl into spec_ctrl_flags
2018-05-11 10:38 [PATCH for-4.11 00/10] x86: Improvements and fixes to Spectre handling Andrew Cooper
2018-05-11 10:38 ` [PATCH 01/10] x86/spec_ctrl: Read MSR_ARCH_CAPABILITIES only once Andrew Cooper
2018-05-11 10:38 ` [PATCH 02/10] x86/spec_ctrl: Express Xen's choice of MSR_SPEC_CTRL value as a variable Andrew Cooper
@ 2018-05-11 10:38 ` Andrew Cooper
2018-05-14 15:13 ` Wei Liu
2018-05-11 10:38 ` [PATCH 04/10] x86/spec_ctrl: Fold the XEN_IBRS_{SET, CLEAR} ALTERNATIVES together Andrew Cooper
` (9 subsequent siblings)
12 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2018-05-11 10:38 UTC (permalink / raw)
To: Xen-devel
Cc: Juergen Gross, Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné
All 3 bits of information here are control flags for the entry/exit code
behaviour. Treat them as such, rather than having two different variables.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Juergen Gross <jgross@suse.com>
---
xen/arch/x86/acpi/power.c | 4 ++--
xen/arch/x86/spec_ctrl.c | 10 ++++----
xen/arch/x86/x86_64/asm-offsets.c | 3 +--
xen/include/asm-x86/current.h | 3 +--
xen/include/asm-x86/spec_ctrl.h | 10 ++++----
xen/include/asm-x86/spec_ctrl_asm.h | 46 ++++++++++++++++++++-----------------
6 files changed, 40 insertions(+), 36 deletions(-)
diff --git a/xen/arch/x86/acpi/power.c b/xen/arch/x86/acpi/power.c
index bb0d095..a704c7c 100644
--- a/xen/arch/x86/acpi/power.c
+++ b/xen/arch/x86/acpi/power.c
@@ -215,7 +215,7 @@ static int enter_state(u32 state)
ci = get_cpu_info();
spec_ctrl_enter_idle(ci);
/* Avoid NMI/#MC using MSR_SPEC_CTRL until we've reloaded microcode. */
- ci->bti_ist_info = 0;
+ ci->spec_ctrl_flags &= ~SCF_ist_wrmsr;
ACPI_FLUSH_CPU_CACHE();
@@ -259,7 +259,7 @@ static int enter_state(u32 state)
panic("Missing previously available feature(s).");
/* Re-enabled default NMI/#MC use of MSR_SPEC_CTRL. */
- ci->bti_ist_info = default_bti_ist_info;
+ ci->spec_ctrl_flags |= (default_spec_ctrl_flags & SCF_ist_wrmsr);
spec_ctrl_exit_idle(ci);
done:
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 6633c64..1ad3ff5 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -39,7 +39,7 @@ static bool __initdata opt_rsb_native = true;
static bool __initdata opt_rsb_vmexit = true;
bool __read_mostly opt_ibpb = true;
uint8_t __read_mostly default_xen_spec_ctrl;
-uint8_t __read_mostly default_bti_ist_info;
+uint8_t __read_mostly default_spec_ctrl_flags;
static int __init parse_bti(const char *s)
{
@@ -374,7 +374,7 @@ void __init init_speculation_mitigations(void)
else
setup_force_cpu_cap(X86_FEATURE_XEN_IBRS_CLEAR);
- default_bti_ist_info |= BTI_IST_WRMSR;
+ default_spec_ctrl_flags |= SCF_ist_wrmsr;
}
/*
@@ -393,7 +393,7 @@ void __init init_speculation_mitigations(void)
if ( opt_rsb_native )
{
setup_force_cpu_cap(X86_FEATURE_RSB_NATIVE);
- default_bti_ist_info |= BTI_IST_RSB;
+ default_spec_ctrl_flags |= SCF_ist_rsb;
}
/*
@@ -407,7 +407,7 @@ void __init init_speculation_mitigations(void)
if ( !boot_cpu_has(X86_FEATURE_IBRSB) && !boot_cpu_has(X86_FEATURE_IBPB) )
opt_ibpb = false;
- /* (Re)init BSP state now that default_bti_ist_info has been calculated. */
+ /* (Re)init BSP state now that default_spec_ctrl_flags has been calculated. */
init_shadow_spec_ctrl_state();
xpti_init_default(false);
@@ -421,6 +421,8 @@ void __init init_speculation_mitigations(void)
static void __init __maybe_unused build_assertions(void)
{
+ /* The optimised assembly relies on this alias. */
+ BUILD_BUG_ON(SCF_use_shadow != 1);
}
/*
diff --git a/xen/arch/x86/x86_64/asm-offsets.c b/xen/arch/x86/x86_64/asm-offsets.c
index f80d3b7..5957c76 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -135,8 +135,7 @@ void __dummy__(void)
OFFSET(CPUINFO_pv_cr3, struct cpu_info, pv_cr3);
OFFSET(CPUINFO_shadow_spec_ctrl, struct cpu_info, shadow_spec_ctrl);
OFFSET(CPUINFO_xen_spec_ctrl, struct cpu_info, xen_spec_ctrl);
- OFFSET(CPUINFO_use_shadow_spec_ctrl, struct cpu_info, use_shadow_spec_ctrl);
- OFFSET(CPUINFO_bti_ist_info, struct cpu_info, bti_ist_info);
+ OFFSET(CPUINFO_spec_ctrl_flags, struct cpu_info, spec_ctrl_flags);
OFFSET(CPUINFO_root_pgt_changed, struct cpu_info, root_pgt_changed);
OFFSET(CPUINFO_use_pv_cr3, struct cpu_info, use_pv_cr3);
DEFINE(CPUINFO_sizeof, sizeof(struct cpu_info));
diff --git a/xen/include/asm-x86/current.h b/xen/include/asm-x86/current.h
index 200e935..5bd64b2 100644
--- a/xen/include/asm-x86/current.h
+++ b/xen/include/asm-x86/current.h
@@ -55,8 +55,7 @@ struct cpu_info {
/* See asm-x86/spec_ctrl_asm.h for usage. */
unsigned int shadow_spec_ctrl;
uint8_t xen_spec_ctrl;
- bool use_shadow_spec_ctrl;
- uint8_t bti_ist_info;
+ uint8_t spec_ctrl_flags;
/*
* The following field controls copying of the L4 page table of 64-bit
diff --git a/xen/include/asm-x86/spec_ctrl.h b/xen/include/asm-x86/spec_ctrl.h
index 0c7663a..d5bd4df 100644
--- a/xen/include/asm-x86/spec_ctrl.h
+++ b/xen/include/asm-x86/spec_ctrl.h
@@ -28,7 +28,7 @@ void init_speculation_mitigations(void);
extern bool opt_ibpb;
extern uint8_t default_xen_spec_ctrl;
-extern uint8_t default_bti_ist_info;
+extern uint8_t default_spec_ctrl_flags;
extern uint8_t opt_xpti;
#define OPT_XPTI_DOM0 0x01
@@ -38,9 +38,9 @@ static inline void init_shadow_spec_ctrl_state(void)
{
struct cpu_info *info = get_cpu_info();
- info->shadow_spec_ctrl = info->use_shadow_spec_ctrl = 0;
+ info->shadow_spec_ctrl = 0;
info->xen_spec_ctrl = default_xen_spec_ctrl;
- info->bti_ist_info = default_bti_ist_info;
+ info->spec_ctrl_flags = default_spec_ctrl_flags;
}
/* WARNING! `ret`, `call *`, `jmp *` not safe after this call. */
@@ -54,7 +54,7 @@ static always_inline void spec_ctrl_enter_idle(struct cpu_info *info)
*/
info->shadow_spec_ctrl = val;
barrier();
- info->use_shadow_spec_ctrl = true;
+ info->spec_ctrl_flags |= SCF_use_shadow;
barrier();
asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_XEN_IBRS_SET)
:: "a" (val), "c" (MSR_SPEC_CTRL), "d" (0) : "memory" );
@@ -69,7 +69,7 @@ static always_inline void spec_ctrl_exit_idle(struct cpu_info *info)
* Disable shadowing before updating the MSR. There are no SMP issues
* here; only local processor ordering concerns.
*/
- info->use_shadow_spec_ctrl = false;
+ info->spec_ctrl_flags &= ~SCF_use_shadow;
barrier();
asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_XEN_IBRS_SET)
:: "a" (val), "c" (MSR_SPEC_CTRL), "d" (0) : "memory" );
diff --git a/xen/include/asm-x86/spec_ctrl_asm.h b/xen/include/asm-x86/spec_ctrl_asm.h
index e8e8f9a..97da08b 100644
--- a/xen/include/asm-x86/spec_ctrl_asm.h
+++ b/xen/include/asm-x86/spec_ctrl_asm.h
@@ -20,9 +20,10 @@
#ifndef __X86_SPEC_CTRL_ASM_H__
#define __X86_SPEC_CTRL_ASM_H__
-/* Encoding of the bottom bits in cpuinfo.bti_ist_info */
-#define BTI_IST_WRMSR (1 << 1)
-#define BTI_IST_RSB (1 << 2)
+/* Encoding of cpuinfo.spec_ctrl_flags */
+#define SCF_use_shadow (1 << 0)
+#define SCF_ist_wrmsr (1 << 1)
+#define SCF_ist_rsb (1 << 2)
#ifdef __ASSEMBLY__
#include <asm/msr-index.h>
@@ -49,20 +50,20 @@
* after VMEXIT. The VMEXIT-specific code reads MSR_SPEC_CTRL and updates
* current before loading Xen's MSR_SPEC_CTRL setting.
*
- * Factor 2 is harder. We maintain a shadow_spec_ctrl value, and
- * use_shadow_spec_ctrl boolean per cpu. The synchronous use is:
+ * Factor 2 is harder. We maintain a shadow_spec_ctrl value, and a use_shadow
+ * boolean in the per cpu spec_ctrl_flags. The synchronous use is:
*
* 1) Store guest value in shadow_spec_ctrl
- * 2) Set use_shadow_spec_ctrl boolean
+ * 2) Set the use_shadow boolean
* 3) Load guest value into MSR_SPEC_CTRL
* 4) Exit to guest
* 5) Entry from guest
- * 6) Clear use_shadow_spec_ctrl boolean
+ * 6) Clear the use_shadow boolean
* 7) Load Xen's value into MSR_SPEC_CTRL
*
* The asynchronous use for interrupts/exceptions is:
* - Set/clear IBRS on entry to Xen
- * - On exit to Xen, check use_shadow_spec_ctrl
+ * - On exit to Xen, check use_shadow
* - If set, load shadow_spec_ctrl
*
* Therefore, an interrupt/exception which hits the synchronous path between
@@ -134,7 +135,7 @@
xor %edx, %edx
/* Clear SPEC_CTRL shadowing *before* loading Xen's value. */
- movb %dl, CPUINFO_use_shadow_spec_ctrl(%rsp)
+ andb $~SCF_use_shadow, CPUINFO_spec_ctrl_flags(%rsp)
/* Load Xen's intended value. */
mov $\ibrs_val, %eax
@@ -160,12 +161,14 @@
* block so calculate the position directly.
*/
.if \maybexen
+ xor %eax, %eax
/* Branchless `if ( !xen ) clear_shadowing` */
testb $3, UREGS_cs(%rsp)
- setz %al
- and %al, STACK_CPUINFO_FIELD(use_shadow_spec_ctrl)(%r14)
+ setnz %al
+ not %eax
+ and %al, STACK_CPUINFO_FIELD(spec_ctrl_flags)(%r14)
.else
- movb %dl, CPUINFO_use_shadow_spec_ctrl(%rsp)
+ andb $~SCF_use_shadow, CPUINFO_spec_ctrl_flags(%rsp)
.endif
/* Load Xen's intended value. */
@@ -184,8 +187,8 @@
*/
xor %edx, %edx
- cmpb %dl, STACK_CPUINFO_FIELD(use_shadow_spec_ctrl)(%rbx)
- je .L\@_skip
+ testb $SCF_use_shadow, STACK_CPUINFO_FIELD(spec_ctrl_flags)(%rbx)
+ jz .L\@_skip
mov STACK_CPUINFO_FIELD(shadow_spec_ctrl)(%rbx), %eax
mov $MSR_SPEC_CTRL, %ecx
@@ -206,7 +209,7 @@
mov %eax, CPUINFO_shadow_spec_ctrl(%rsp)
/* Set SPEC_CTRL shadowing *before* loading the guest value. */
- movb $1, CPUINFO_use_shadow_spec_ctrl(%rsp)
+ orb $SCF_use_shadow, CPUINFO_spec_ctrl_flags(%rsp)
mov $MSR_SPEC_CTRL, %ecx
xor %edx, %edx
@@ -265,22 +268,23 @@
* This is logical merge of DO_OVERWRITE_RSB and DO_SPEC_CTRL_ENTRY
* maybexen=1, but with conditionals rather than alternatives.
*/
- movzbl STACK_CPUINFO_FIELD(bti_ist_info)(%r14), %eax
+ movzbl STACK_CPUINFO_FIELD(spec_ctrl_flags)(%r14), %eax
- testb $BTI_IST_RSB, %al
+ test $SCF_ist_rsb, %al
jz .L\@_skip_rsb
DO_OVERWRITE_RSB tmp=rdx /* Clobbers %rcx/%rdx */
.L\@_skip_rsb:
- testb $BTI_IST_WRMSR, %al
+ test $SCF_ist_wrmsr, %al
jz .L\@_skip_wrmsr
xor %edx, %edx
testb $3, UREGS_cs(%rsp)
- setz %dl
- and %dl, STACK_CPUINFO_FIELD(use_shadow_spec_ctrl)(%r14)
+ setnz %dl
+ not %edx
+ and %dl, STACK_CPUINFO_FIELD(spec_ctrl_flags)(%r14)
/* Load Xen's intended value. */
mov $MSR_SPEC_CTRL, %ecx
@@ -307,7 +311,7 @@ UNLIKELY_DISPATCH_LABEL(\@_serialise):
* Requires %rbx=stack_end
* Clobbers %rax, %rcx, %rdx
*/
- testb $BTI_IST_WRMSR, STACK_CPUINFO_FIELD(bti_ist_info)(%rbx)
+ testb $SCF_ist_wrmsr, STACK_CPUINFO_FIELD(spec_ctrl_flags)(%rbx)
jz .L\@_skip
DO_SPEC_CTRL_EXIT_TO_XEN
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 03/10] x86/spec_ctrl: Merge bti_ist_info and use_shadow_spec_ctrl into spec_ctrl_flags
2018-05-11 10:38 ` [PATCH 03/10] x86/spec_ctrl: Merge bti_ist_info and use_shadow_spec_ctrl into spec_ctrl_flags Andrew Cooper
@ 2018-05-14 15:13 ` Wei Liu
0 siblings, 0 replies; 34+ messages in thread
From: Wei Liu @ 2018-05-14 15:13 UTC (permalink / raw)
To: Andrew Cooper
Cc: Juergen Gross, Roger Pau Monné, Wei Liu, Jan Beulich, Xen-devel
On Fri, May 11, 2018 at 11:38:07AM +0100, Andrew Cooper wrote:
> All 3 bits of information here are control flags for the entry/exit code
> behaviour. Treat them as such, rather than having two different variables.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
This patch does what it says on the tin.
FWIW:
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
> diff --git a/xen/include/asm-x86/spec_ctrl_asm.h b/xen/include/asm-x86/spec_ctrl_asm.h
> index e8e8f9a..97da08b 100644
> --- a/xen/include/asm-x86/spec_ctrl_asm.h
> +++ b/xen/include/asm-x86/spec_ctrl_asm.h
> @@ -20,9 +20,10 @@
> #ifndef __X86_SPEC_CTRL_ASM_H__
> #define __X86_SPEC_CTRL_ASM_H__
>
> -/* Encoding of the bottom bits in cpuinfo.bti_ist_info */
> -#define BTI_IST_WRMSR (1 << 1)
> -#define BTI_IST_RSB (1 << 2)
> +/* Encoding of cpuinfo.spec_ctrl_flags */
> +#define SCF_use_shadow (1 << 0)
> +#define SCF_ist_wrmsr (1 << 1)
> +#define SCF_ist_rsb (1 << 2)
>
Fancy turning them into 1u << X while you're at it?
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 04/10] x86/spec_ctrl: Fold the XEN_IBRS_{SET, CLEAR} ALTERNATIVES together
2018-05-11 10:38 [PATCH for-4.11 00/10] x86: Improvements and fixes to Spectre handling Andrew Cooper
` (2 preceding siblings ...)
2018-05-11 10:38 ` [PATCH 03/10] x86/spec_ctrl: Merge bti_ist_info and use_shadow_spec_ctrl into spec_ctrl_flags Andrew Cooper
@ 2018-05-11 10:38 ` Andrew Cooper
2018-05-14 15:20 ` Wei Liu
2018-05-11 10:38 ` [PATCH 05/10] x86/spec_ctrl: Rename bits of infrastructure to avoid NATIVE and VMEXIT Andrew Cooper
` (8 subsequent siblings)
12 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2018-05-11 10:38 UTC (permalink / raw)
To: Xen-devel
Cc: Juergen Gross, Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné
Currently, the SPEC_CTRL_{ENTRY,EXIT}_* macros encode Xen's choice of
MSR_SPEC_CTRL as an immediate constant, and chooses between IBRS or not by
doubling up the entire alternative block.
There is now a variable holding Xen's choice of value, so use that and
simplify the alternatives.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Juergen Gross <jgross@suse.com>
---
xen/arch/x86/spec_ctrl.c | 12 +++++-----
xen/include/asm-x86/cpufeatures.h | 3 +--
xen/include/asm-x86/spec_ctrl.h | 6 ++---
xen/include/asm-x86/spec_ctrl_asm.h | 45 +++++++++++++------------------------
4 files changed, 24 insertions(+), 42 deletions(-)
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 1ad3ff5..13e426c 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -128,8 +128,9 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
thunk == THUNK_RETPOLINE ? "RETPOLINE" :
thunk == THUNK_LFENCE ? "LFENCE" :
thunk == THUNK_JMP ? "JMP" : "?",
- boot_cpu_has(X86_FEATURE_XEN_IBRS_SET) ? " IBRS+" :
- boot_cpu_has(X86_FEATURE_XEN_IBRS_CLEAR) ? " IBRS-" : "",
+ boot_cpu_has(X86_FEATURE_SC_MSR) ?
+ default_xen_spec_ctrl & SPEC_CTRL_IBRS ? " IBRS+" :
+ " IBRS-" : "",
opt_ibpb ? " IBPB" : "",
boot_cpu_has(X86_FEATURE_RSB_NATIVE) ? " RSB_NATIVE" : "",
boot_cpu_has(X86_FEATURE_RSB_VMEXIT) ? " RSB_VMEXIT" : "");
@@ -366,13 +367,10 @@ void __init init_speculation_mitigations(void)
* need the IBRS entry/exit logic to virtualise IBRS support for
* guests.
*/
+ setup_force_cpu_cap(X86_FEATURE_SC_MSR);
+
if ( ibrs )
- {
default_xen_spec_ctrl |= SPEC_CTRL_IBRS;
- setup_force_cpu_cap(X86_FEATURE_XEN_IBRS_SET);
- }
- else
- setup_force_cpu_cap(X86_FEATURE_XEN_IBRS_CLEAR);
default_spec_ctrl_flags |= SCF_ist_wrmsr;
}
diff --git a/xen/include/asm-x86/cpufeatures.h b/xen/include/asm-x86/cpufeatures.h
index c9b1a48..ca58b0e 100644
--- a/xen/include/asm-x86/cpufeatures.h
+++ b/xen/include/asm-x86/cpufeatures.h
@@ -26,8 +26,7 @@ XEN_CPUFEATURE(LFENCE_DISPATCH, (FSCAPINTS+0)*32+12) /* lfence set as Dispatch S
XEN_CPUFEATURE(IND_THUNK_LFENCE,(FSCAPINTS+0)*32+13) /* Use IND_THUNK_LFENCE */
XEN_CPUFEATURE(IND_THUNK_JMP, (FSCAPINTS+0)*32+14) /* Use IND_THUNK_JMP */
XEN_CPUFEATURE(XEN_IBPB, (FSCAPINTS+0)*32+15) /* IBRSB || IBPB */
-XEN_CPUFEATURE(XEN_IBRS_SET, (FSCAPINTS+0)*32+16) /* IBRSB && IRBS set in Xen */
-XEN_CPUFEATURE(XEN_IBRS_CLEAR, (FSCAPINTS+0)*32+17) /* IBRSB && IBRS clear in Xen */
+XEN_CPUFEATURE(SC_MSR, (FSCAPINTS+0)*32+16) /* MSR_SPEC_CTRL used by Xen */
XEN_CPUFEATURE(RSB_NATIVE, (FSCAPINTS+0)*32+18) /* RSB overwrite needed for native */
XEN_CPUFEATURE(RSB_VMEXIT, (FSCAPINTS+0)*32+19) /* RSB overwrite needed for vmexit */
XEN_CPUFEATURE(NO_XPTI, (FSCAPINTS+0)*32+20) /* XPTI mitigation not in use */
diff --git a/xen/include/asm-x86/spec_ctrl.h b/xen/include/asm-x86/spec_ctrl.h
index d5bd4df..86a3dfe 100644
--- a/xen/include/asm-x86/spec_ctrl.h
+++ b/xen/include/asm-x86/spec_ctrl.h
@@ -56,14 +56,14 @@ static always_inline void spec_ctrl_enter_idle(struct cpu_info *info)
barrier();
info->spec_ctrl_flags |= SCF_use_shadow;
barrier();
- asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_XEN_IBRS_SET)
+ asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_SC_MSR)
:: "a" (val), "c" (MSR_SPEC_CTRL), "d" (0) : "memory" );
}
/* WARNING! `ret`, `call *`, `jmp *` not safe before this call. */
static always_inline void spec_ctrl_exit_idle(struct cpu_info *info)
{
- uint32_t val = SPEC_CTRL_IBRS;
+ uint32_t val = info->xen_spec_ctrl;
/*
* Disable shadowing before updating the MSR. There are no SMP issues
@@ -71,7 +71,7 @@ static always_inline void spec_ctrl_exit_idle(struct cpu_info *info)
*/
info->spec_ctrl_flags &= ~SCF_use_shadow;
barrier();
- asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_XEN_IBRS_SET)
+ asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_SC_MSR)
:: "a" (val), "c" (MSR_SPEC_CTRL), "d" (0) : "memory" );
}
diff --git a/xen/include/asm-x86/spec_ctrl_asm.h b/xen/include/asm-x86/spec_ctrl_asm.h
index 97da08b..730c998 100644
--- a/xen/include/asm-x86/spec_ctrl_asm.h
+++ b/xen/include/asm-x86/spec_ctrl_asm.h
@@ -117,7 +117,7 @@
mov %\tmp, %rsp /* Restore old %rsp */
.endm
-.macro DO_SPEC_CTRL_ENTRY_FROM_VMEXIT ibrs_val:req
+.macro DO_SPEC_CTRL_ENTRY_FROM_VMEXIT
/*
* Requires %rbx=current, %rsp=regs/cpuinfo
* Clobbers %rax, %rcx, %rdx
@@ -138,11 +138,11 @@
andb $~SCF_use_shadow, CPUINFO_spec_ctrl_flags(%rsp)
/* Load Xen's intended value. */
- mov $\ibrs_val, %eax
+ movzbl CPUINFO_xen_spec_ctrl(%rsp), %eax
wrmsr
.endm
-.macro DO_SPEC_CTRL_ENTRY maybexen:req ibrs_val:req
+.macro DO_SPEC_CTRL_ENTRY maybexen:req
/*
* Requires %rsp=regs (also cpuinfo if !maybexen)
* Requires %r14=stack_end (if maybexen)
@@ -167,12 +167,12 @@
setnz %al
not %eax
and %al, STACK_CPUINFO_FIELD(spec_ctrl_flags)(%r14)
+ movzbl STACK_CPUINFO_FIELD(xen_spec_ctrl)(%r14), %eax
.else
andb $~SCF_use_shadow, CPUINFO_spec_ctrl_flags(%rsp)
+ movzbl CPUINFO_xen_spec_ctrl(%rsp), %eax
.endif
- /* Load Xen's intended value. */
- mov $\ibrs_val, %eax
wrmsr
.endm
@@ -219,45 +219,30 @@
/* Use after a VMEXIT from an HVM guest. */
#define SPEC_CTRL_ENTRY_FROM_VMEXIT \
ALTERNATIVE "", DO_OVERWRITE_RSB, X86_FEATURE_RSB_VMEXIT; \
- ALTERNATIVE_2 "", \
- __stringify(DO_SPEC_CTRL_ENTRY_FROM_VMEXIT \
- ibrs_val=SPEC_CTRL_IBRS), \
- X86_FEATURE_XEN_IBRS_SET, \
- __stringify(DO_SPEC_CTRL_ENTRY_FROM_VMEXIT \
- ibrs_val=0), \
- X86_FEATURE_XEN_IBRS_CLEAR
+ ALTERNATIVE "", DO_SPEC_CTRL_ENTRY_FROM_VMEXIT, \
+ X86_FEATURE_SC_MSR
/* Use after an entry from PV context (syscall/sysenter/int80/int82/etc). */
#define SPEC_CTRL_ENTRY_FROM_PV \
ALTERNATIVE "", DO_OVERWRITE_RSB, X86_FEATURE_RSB_NATIVE; \
- ALTERNATIVE_2 "", \
- __stringify(DO_SPEC_CTRL_ENTRY maybexen=0 \
- ibrs_val=SPEC_CTRL_IBRS), \
- X86_FEATURE_XEN_IBRS_SET, \
- __stringify(DO_SPEC_CTRL_ENTRY maybexen=0 ibrs_val=0), \
- X86_FEATURE_XEN_IBRS_CLEAR
+ ALTERNATIVE "", __stringify(DO_SPEC_CTRL_ENTRY maybexen=0), \
+ X86_FEATURE_SC_MSR
/* Use in interrupt/exception context. May interrupt Xen or PV context. */
#define SPEC_CTRL_ENTRY_FROM_INTR \
ALTERNATIVE "", DO_OVERWRITE_RSB, X86_FEATURE_RSB_NATIVE; \
- ALTERNATIVE_2 "", \
- __stringify(DO_SPEC_CTRL_ENTRY maybexen=1 \
- ibrs_val=SPEC_CTRL_IBRS), \
- X86_FEATURE_XEN_IBRS_SET, \
- __stringify(DO_SPEC_CTRL_ENTRY maybexen=1 ibrs_val=0), \
- X86_FEATURE_XEN_IBRS_CLEAR
+ ALTERNATIVE "", __stringify(DO_SPEC_CTRL_ENTRY maybexen=1), \
+ X86_FEATURE_SC_MSR
/* Use when exiting to Xen context. */
#define SPEC_CTRL_EXIT_TO_XEN \
- ALTERNATIVE_2 "", \
- DO_SPEC_CTRL_EXIT_TO_XEN, X86_FEATURE_XEN_IBRS_SET, \
- DO_SPEC_CTRL_EXIT_TO_XEN, X86_FEATURE_XEN_IBRS_CLEAR
+ ALTERNATIVE "", \
+ DO_SPEC_CTRL_EXIT_TO_XEN, X86_FEATURE_SC_MSR
/* Use when exiting to guest context. */
#define SPEC_CTRL_EXIT_TO_GUEST \
- ALTERNATIVE_2 "", \
- DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_XEN_IBRS_SET, \
- DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_XEN_IBRS_CLEAR
+ ALTERNATIVE "", \
+ DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_SC_MSR
/* TODO: Drop these when the alternatives infrastructure is NMI/#MC safe. */
.macro SPEC_CTRL_ENTRY_FROM_INTR_IST
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 04/10] x86/spec_ctrl: Fold the XEN_IBRS_{SET, CLEAR} ALTERNATIVES together
2018-05-11 10:38 ` [PATCH 04/10] x86/spec_ctrl: Fold the XEN_IBRS_{SET, CLEAR} ALTERNATIVES together Andrew Cooper
@ 2018-05-14 15:20 ` Wei Liu
0 siblings, 0 replies; 34+ messages in thread
From: Wei Liu @ 2018-05-14 15:20 UTC (permalink / raw)
To: Andrew Cooper
Cc: Juergen Gross, Roger Pau Monné, Wei Liu, Jan Beulich, Xen-devel
On Fri, May 11, 2018 at 11:38:08AM +0100, Andrew Cooper wrote:
> Currently, the SPEC_CTRL_{ENTRY,EXIT}_* macros encode Xen's choice of
> MSR_SPEC_CTRL as an immediate constant, and chooses between IBRS or not by
> doubling up the entire alternative block.
>
> There is now a variable holding Xen's choice of value, so use that and
> simplify the alternatives.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 05/10] x86/spec_ctrl: Rename bits of infrastructure to avoid NATIVE and VMEXIT
2018-05-11 10:38 [PATCH for-4.11 00/10] x86: Improvements and fixes to Spectre handling Andrew Cooper
` (3 preceding siblings ...)
2018-05-11 10:38 ` [PATCH 04/10] x86/spec_ctrl: Fold the XEN_IBRS_{SET, CLEAR} ALTERNATIVES together Andrew Cooper
@ 2018-05-11 10:38 ` Andrew Cooper
2018-05-14 15:21 ` Wei Liu
2018-05-11 10:38 ` [PATCH 06/10] x86/spec_ctrl: Split X86_FEATURE_SC_MSR into PV and HVM variants Andrew Cooper
` (7 subsequent siblings)
12 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2018-05-11 10:38 UTC (permalink / raw)
To: Xen-devel
Cc: Juergen Gross, Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné
In hindsight, using NATIVE and VMEXIT as naming terminology was not clever.
A future change wants to split SPEC_CTRL_EXIT_TO_GUEST into PV and HVM
specific implementations, and using VMEXIT as a term is completely wrong.
Take the opportunity to fix some stale documentation in spec_ctrl_asm.h. The
IST helpers were missing from the large comment block, and since
SPEC_CTRL_ENTRY_FROM_INTR_IST was introduced, we've gained a new piece of
functionality which currently depends on the fine grain control, which exists
in lieu of livepatching. Note this in the comment.
No functional change.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Juergen Gross <jgross@suse.com>
---
xen/arch/x86/hvm/svm/entry.S | 4 ++--
xen/arch/x86/hvm/vmx/entry.S | 4 ++--
xen/arch/x86/spec_ctrl.c | 28 ++++++++++++++--------------
xen/arch/x86/x86_64/compat/entry.S | 2 +-
xen/arch/x86/x86_64/entry.S | 2 +-
xen/include/asm-x86/cpufeatures.h | 4 ++--
xen/include/asm-x86/spec_ctrl_asm.h | 36 +++++++++++++++++++++++++-----------
7 files changed, 47 insertions(+), 33 deletions(-)
diff --git a/xen/arch/x86/hvm/svm/entry.S b/xen/arch/x86/hvm/svm/entry.S
index 0fa5501..2d540f9 100644
--- a/xen/arch/x86/hvm/svm/entry.S
+++ b/xen/arch/x86/hvm/svm/entry.S
@@ -68,7 +68,7 @@ __UNLIKELY_END(nsvm_hap)
mov VCPUMSR_spec_ctrl_raw(%rax), %eax
/* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
- SPEC_CTRL_EXIT_TO_GUEST /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */
+ SPEC_CTRL_EXIT_TO_HVM /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */
pop %r15
pop %r14
@@ -93,7 +93,7 @@ __UNLIKELY_END(nsvm_hap)
GET_CURRENT(bx)
- SPEC_CTRL_ENTRY_FROM_VMEXIT /* Req: b=curr %rsp=regs/cpuinfo, Clob: acd */
+ SPEC_CTRL_ENTRY_FROM_HVM /* Req: b=curr %rsp=regs/cpuinfo, Clob: acd */
/* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
STGI
diff --git a/xen/arch/x86/hvm/vmx/entry.S b/xen/arch/x86/hvm/vmx/entry.S
index e750544..aa2f103 100644
--- a/xen/arch/x86/hvm/vmx/entry.S
+++ b/xen/arch/x86/hvm/vmx/entry.S
@@ -38,7 +38,7 @@ ENTRY(vmx_asm_vmexit_handler)
movb $1,VCPU_vmx_launched(%rbx)
mov %rax,VCPU_hvm_guest_cr2(%rbx)
- SPEC_CTRL_ENTRY_FROM_VMEXIT /* Req: b=curr %rsp=regs/cpuinfo, Clob: acd */
+ SPEC_CTRL_ENTRY_FROM_HVM /* Req: b=curr %rsp=regs/cpuinfo, Clob: acd */
/* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
mov %rsp,%rdi
@@ -76,7 +76,7 @@ UNLIKELY_END(realmode)
mov VCPUMSR_spec_ctrl_raw(%rax), %eax
/* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
- SPEC_CTRL_EXIT_TO_GUEST /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */
+ SPEC_CTRL_EXIT_TO_HVM /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */
mov VCPU_hvm_guest_cr2(%rbx),%rax
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 13e426c..f489f79 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -35,8 +35,8 @@ static enum ind_thunk {
THUNK_JMP,
} opt_thunk __initdata = THUNK_DEFAULT;
static int8_t __initdata opt_ibrs = -1;
-static bool __initdata opt_rsb_native = true;
-static bool __initdata opt_rsb_vmexit = true;
+static bool __initdata opt_rsb_pv = true;
+static bool __initdata opt_rsb_hvm = true;
bool __read_mostly opt_ibpb = true;
uint8_t __read_mostly default_xen_spec_ctrl;
uint8_t __read_mostly default_spec_ctrl_flags;
@@ -57,8 +57,8 @@ static int __init parse_bti(const char *s)
opt_thunk = THUNK_JMP;
opt_ibrs = 0;
opt_ibpb = false;
- opt_rsb_native = false;
- opt_rsb_vmexit = false;
+ opt_rsb_pv = false;
+ opt_rsb_hvm = false;
}
else if ( val > 0 )
rc = -EINVAL;
@@ -80,13 +80,13 @@ static int __init parse_bti(const char *s)
else if ( (val = parse_boolean("ibpb", s, ss)) >= 0 )
opt_ibpb = val;
else if ( (val = parse_boolean("rsb_native", s, ss)) >= 0 )
- opt_rsb_native = val;
+ opt_rsb_pv = val;
else if ( (val = parse_boolean("rsb_vmexit", s, ss)) >= 0 )
- opt_rsb_vmexit = val;
+ opt_rsb_hvm = val;
else if ( (val = parse_boolean("rsb", s, ss)) >= 0 )
{
- opt_rsb_native = val;
- opt_rsb_vmexit = val;
+ opt_rsb_pv = val;
+ opt_rsb_hvm = val;
}
else
rc = -EINVAL;
@@ -132,8 +132,8 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
default_xen_spec_ctrl & SPEC_CTRL_IBRS ? " IBRS+" :
" IBRS-" : "",
opt_ibpb ? " IBPB" : "",
- boot_cpu_has(X86_FEATURE_RSB_NATIVE) ? " RSB_NATIVE" : "",
- boot_cpu_has(X86_FEATURE_RSB_VMEXIT) ? " RSB_VMEXIT" : "");
+ boot_cpu_has(X86_FEATURE_SC_RSB_PV) ? " RSB_NATIVE" : "",
+ boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ? " RSB_VMEXIT" : "");
printk("XPTI (64-bit PV only): Dom0 %s, DomU %s\n",
opt_xpti & OPT_XPTI_DOM0 ? "enabled" : "disabled",
@@ -388,9 +388,9 @@ void __init init_speculation_mitigations(void)
* If a processors speculates to 32bit PV guest kernel mappings, it is
* speculating in 64bit supervisor mode, and can leak data.
*/
- if ( opt_rsb_native )
+ if ( opt_rsb_pv )
{
- setup_force_cpu_cap(X86_FEATURE_RSB_NATIVE);
+ setup_force_cpu_cap(X86_FEATURE_SC_RSB_PV);
default_spec_ctrl_flags |= SCF_ist_rsb;
}
@@ -398,8 +398,8 @@ void __init init_speculation_mitigations(void)
* HVM guests can always poison the RSB to point at Xen supervisor
* mappings.
*/
- if ( opt_rsb_vmexit )
- setup_force_cpu_cap(X86_FEATURE_RSB_VMEXIT);
+ if ( opt_rsb_hvm )
+ setup_force_cpu_cap(X86_FEATURE_SC_RSB_HVM);
/* Check we have hardware IBPB support before using it... */
if ( !boot_cpu_has(X86_FEATURE_IBRSB) && !boot_cpu_has(X86_FEATURE_IBPB) )
diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
index cd1a9544..f697e05 100644
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -158,7 +158,7 @@ ENTRY(compat_restore_all_guest)
mov VCPUMSR_spec_ctrl_raw(%rax), %eax
/* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
- SPEC_CTRL_EXIT_TO_GUEST /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */
+ SPEC_CTRL_EXIT_TO_PV /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */
RESTORE_ALL adj=8 compat=1
.Lft0: iretq
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 141848a..58054ed 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -177,7 +177,7 @@ restore_all_guest:
mov %r15d, %eax
/* WARNING! `ret`, `call *`, `jmp *` not safe beyond this point. */
- SPEC_CTRL_EXIT_TO_GUEST /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */
+ SPEC_CTRL_EXIT_TO_PV /* Req: a=spec_ctrl %rsp=regs/cpuinfo, Clob: cd */
RESTORE_ALL
testw $TRAP_syscall,4(%rsp)
diff --git a/xen/include/asm-x86/cpufeatures.h b/xen/include/asm-x86/cpufeatures.h
index ca58b0e..f9aa5d7 100644
--- a/xen/include/asm-x86/cpufeatures.h
+++ b/xen/include/asm-x86/cpufeatures.h
@@ -27,6 +27,6 @@ XEN_CPUFEATURE(IND_THUNK_LFENCE,(FSCAPINTS+0)*32+13) /* Use IND_THUNK_LFENCE */
XEN_CPUFEATURE(IND_THUNK_JMP, (FSCAPINTS+0)*32+14) /* Use IND_THUNK_JMP */
XEN_CPUFEATURE(XEN_IBPB, (FSCAPINTS+0)*32+15) /* IBRSB || IBPB */
XEN_CPUFEATURE(SC_MSR, (FSCAPINTS+0)*32+16) /* MSR_SPEC_CTRL used by Xen */
-XEN_CPUFEATURE(RSB_NATIVE, (FSCAPINTS+0)*32+18) /* RSB overwrite needed for native */
-XEN_CPUFEATURE(RSB_VMEXIT, (FSCAPINTS+0)*32+19) /* RSB overwrite needed for vmexit */
+XEN_CPUFEATURE(SC_RSB_PV, (FSCAPINTS+0)*32+18) /* RSB overwrite needed for PV */
+XEN_CPUFEATURE(SC_RSB_HVM, (FSCAPINTS+0)*32+19) /* RSB overwrite needed for HVM */
XEN_CPUFEATURE(NO_XPTI, (FSCAPINTS+0)*32+20) /* XPTI mitigation not in use */
diff --git a/xen/include/asm-x86/spec_ctrl_asm.h b/xen/include/asm-x86/spec_ctrl_asm.h
index 730c998..bf36b5a 100644
--- a/xen/include/asm-x86/spec_ctrl_asm.h
+++ b/xen/include/asm-x86/spec_ctrl_asm.h
@@ -72,11 +72,14 @@
*
* The following ASM fragments implement this algorithm. See their local
* comments for further details.
- * - SPEC_CTRL_ENTRY_FROM_VMEXIT
+ * - SPEC_CTRL_ENTRY_FROM_HVM
* - SPEC_CTRL_ENTRY_FROM_PV
* - SPEC_CTRL_ENTRY_FROM_INTR
+ * - SPEC_CTRL_ENTRY_FROM_INTR_IST
+ * - SPEC_CTRL_EXIT_TO_XEN_IST
* - SPEC_CTRL_EXIT_TO_XEN
- * - SPEC_CTRL_EXIT_TO_GUEST
+ * - SPEC_CTRL_EXIT_TO_PV
+ * - SPEC_CTRL_EXIT_TO_HVM
*/
.macro DO_OVERWRITE_RSB tmp=rax
@@ -117,7 +120,7 @@
mov %\tmp, %rsp /* Restore old %rsp */
.endm
-.macro DO_SPEC_CTRL_ENTRY_FROM_VMEXIT
+.macro DO_SPEC_CTRL_ENTRY_FROM_HVM
/*
* Requires %rbx=current, %rsp=regs/cpuinfo
* Clobbers %rax, %rcx, %rdx
@@ -217,20 +220,20 @@
.endm
/* Use after a VMEXIT from an HVM guest. */
-#define SPEC_CTRL_ENTRY_FROM_VMEXIT \
- ALTERNATIVE "", DO_OVERWRITE_RSB, X86_FEATURE_RSB_VMEXIT; \
- ALTERNATIVE "", DO_SPEC_CTRL_ENTRY_FROM_VMEXIT, \
+#define SPEC_CTRL_ENTRY_FROM_HVM \
+ ALTERNATIVE "", DO_OVERWRITE_RSB, X86_FEATURE_SC_RSB_HVM; \
+ ALTERNATIVE "", DO_SPEC_CTRL_ENTRY_FROM_HVM, \
X86_FEATURE_SC_MSR
/* Use after an entry from PV context (syscall/sysenter/int80/int82/etc). */
#define SPEC_CTRL_ENTRY_FROM_PV \
- ALTERNATIVE "", DO_OVERWRITE_RSB, X86_FEATURE_RSB_NATIVE; \
+ ALTERNATIVE "", DO_OVERWRITE_RSB, X86_FEATURE_SC_RSB_PV; \
ALTERNATIVE "", __stringify(DO_SPEC_CTRL_ENTRY maybexen=0), \
X86_FEATURE_SC_MSR
/* Use in interrupt/exception context. May interrupt Xen or PV context. */
#define SPEC_CTRL_ENTRY_FROM_INTR \
- ALTERNATIVE "", DO_OVERWRITE_RSB, X86_FEATURE_RSB_NATIVE; \
+ ALTERNATIVE "", DO_OVERWRITE_RSB, X86_FEATURE_SC_RSB_PV; \
ALTERNATIVE "", __stringify(DO_SPEC_CTRL_ENTRY maybexen=1), \
X86_FEATURE_SC_MSR
@@ -239,12 +242,22 @@
ALTERNATIVE "", \
DO_SPEC_CTRL_EXIT_TO_XEN, X86_FEATURE_SC_MSR
-/* Use when exiting to guest context. */
-#define SPEC_CTRL_EXIT_TO_GUEST \
+/* Use when exiting to PV guest context. */
+#define SPEC_CTRL_EXIT_TO_PV \
ALTERNATIVE "", \
DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_SC_MSR
-/* TODO: Drop these when the alternatives infrastructure is NMI/#MC safe. */
+/* Use when exiting to HVM guest context. */
+#define SPEC_CTRL_EXIT_TO_HVM \
+ ALTERNATIVE "", \
+ DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_SC_MSR
+
+/*
+ * Use in IST interrupt/exception context. May interrupt Xen or PV context.
+ * Fine grain control of SCF_ist_wrmsr is needed for safety in the S3 resume
+ * path to avoid using MSR_SPEC_CTRL before the microcode introducing it has
+ * been reloaded.
+ */
.macro SPEC_CTRL_ENTRY_FROM_INTR_IST
/*
* Requires %rsp=regs, %r14=stack_end
@@ -291,6 +304,7 @@ UNLIKELY_DISPATCH_LABEL(\@_serialise):
UNLIKELY_END(\@_serialise)
.endm
+/* Use when exiting to Xen in IST context. */
.macro SPEC_CTRL_EXIT_TO_XEN_IST
/*
* Requires %rbx=stack_end
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 05/10] x86/spec_ctrl: Rename bits of infrastructure to avoid NATIVE and VMEXIT
2018-05-11 10:38 ` [PATCH 05/10] x86/spec_ctrl: Rename bits of infrastructure to avoid NATIVE and VMEXIT Andrew Cooper
@ 2018-05-14 15:21 ` Wei Liu
0 siblings, 0 replies; 34+ messages in thread
From: Wei Liu @ 2018-05-14 15:21 UTC (permalink / raw)
To: Andrew Cooper
Cc: Juergen Gross, Roger Pau Monné, Wei Liu, Jan Beulich, Xen-devel
On Fri, May 11, 2018 at 11:38:09AM +0100, Andrew Cooper wrote:
> In hindsight, using NATIVE and VMEXIT as naming terminology was not clever.
> A future change wants to split SPEC_CTRL_EXIT_TO_GUEST into PV and HVM
> specific implementations, and using VMEXIT as a term is completely wrong.
>
> Take the opportunity to fix some stale documentation in spec_ctrl_asm.h. The
> IST helpers were missing from the large comment block, and since
> SPEC_CTRL_ENTRY_FROM_INTR_IST was introduced, we've gained a new piece of
> functionality which currently depends on the fine grain control, which exists
> in lieu of livepatching. Note this in the comment.
>
> No functional change.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 06/10] x86/spec_ctrl: Split X86_FEATURE_SC_MSR into PV and HVM variants
2018-05-11 10:38 [PATCH for-4.11 00/10] x86: Improvements and fixes to Spectre handling Andrew Cooper
` (4 preceding siblings ...)
2018-05-11 10:38 ` [PATCH 05/10] x86/spec_ctrl: Rename bits of infrastructure to avoid NATIVE and VMEXIT Andrew Cooper
@ 2018-05-11 10:38 ` Andrew Cooper
2018-05-14 15:22 ` Wei Liu
2018-05-14 15:27 ` Jan Beulich
2018-05-11 10:38 ` [PATCH 07/10] x86/spec_ctrl: Explicitly set Xen's default MSR_SPEC_CTRL value Andrew Cooper
` (6 subsequent siblings)
12 siblings, 2 replies; 34+ messages in thread
From: Andrew Cooper @ 2018-05-11 10:38 UTC (permalink / raw)
To: Xen-devel
Cc: Juergen Gross, Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné
In order to separately control whether MSR_SPEC_CTRL is virtualised for PV and
HVM guests, split the feature used to control runtime alternatives into two.
Xen will use MSR_SPEC_CTRL itself if either of these features are active.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Juergen Gross <jgross@suse.com>
---
xen/arch/x86/spec_ctrl.c | 6 ++++--
xen/include/asm-x86/cpufeatures.h | 3 ++-
xen/include/asm-x86/spec_ctrl.h | 8 ++++++--
xen/include/asm-x86/spec_ctrl_asm.h | 12 ++++++------
4 files changed, 18 insertions(+), 11 deletions(-)
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index f489f79..0404962 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -128,7 +128,8 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
thunk == THUNK_RETPOLINE ? "RETPOLINE" :
thunk == THUNK_LFENCE ? "LFENCE" :
thunk == THUNK_JMP ? "JMP" : "?",
- boot_cpu_has(X86_FEATURE_SC_MSR) ?
+ (boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
+ boot_cpu_has(X86_FEATURE_SC_MSR_HVM)) ?
default_xen_spec_ctrl & SPEC_CTRL_IBRS ? " IBRS+" :
" IBRS-" : "",
opt_ibpb ? " IBPB" : "",
@@ -367,7 +368,8 @@ void __init init_speculation_mitigations(void)
* need the IBRS entry/exit logic to virtualise IBRS support for
* guests.
*/
- setup_force_cpu_cap(X86_FEATURE_SC_MSR);
+ setup_force_cpu_cap(X86_FEATURE_SC_MSR_PV);
+ setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM);
if ( ibrs )
default_xen_spec_ctrl |= SPEC_CTRL_IBRS;
diff --git a/xen/include/asm-x86/cpufeatures.h b/xen/include/asm-x86/cpufeatures.h
index f9aa5d7..9d5d81e 100644
--- a/xen/include/asm-x86/cpufeatures.h
+++ b/xen/include/asm-x86/cpufeatures.h
@@ -26,7 +26,8 @@ XEN_CPUFEATURE(LFENCE_DISPATCH, (FSCAPINTS+0)*32+12) /* lfence set as Dispatch S
XEN_CPUFEATURE(IND_THUNK_LFENCE,(FSCAPINTS+0)*32+13) /* Use IND_THUNK_LFENCE */
XEN_CPUFEATURE(IND_THUNK_JMP, (FSCAPINTS+0)*32+14) /* Use IND_THUNK_JMP */
XEN_CPUFEATURE(XEN_IBPB, (FSCAPINTS+0)*32+15) /* IBRSB || IBPB */
-XEN_CPUFEATURE(SC_MSR, (FSCAPINTS+0)*32+16) /* MSR_SPEC_CTRL used by Xen */
+XEN_CPUFEATURE(SC_MSR_PV, (FSCAPINTS+0)*32+16) /* MSR_SPEC_CTRL used by Xen for PV */
+XEN_CPUFEATURE(SC_MSR_HVM, (FSCAPINTS+0)*32+17) /* MSR_SPEC_CTRL used by Xen for HVM */
XEN_CPUFEATURE(SC_RSB_PV, (FSCAPINTS+0)*32+18) /* RSB overwrite needed for PV */
XEN_CPUFEATURE(SC_RSB_HVM, (FSCAPINTS+0)*32+19) /* RSB overwrite needed for HVM */
XEN_CPUFEATURE(NO_XPTI, (FSCAPINTS+0)*32+20) /* XPTI mitigation not in use */
diff --git a/xen/include/asm-x86/spec_ctrl.h b/xen/include/asm-x86/spec_ctrl.h
index 86a3dfe..9880e19 100644
--- a/xen/include/asm-x86/spec_ctrl.h
+++ b/xen/include/asm-x86/spec_ctrl.h
@@ -56,7 +56,9 @@ static always_inline void spec_ctrl_enter_idle(struct cpu_info *info)
barrier();
info->spec_ctrl_flags |= SCF_use_shadow;
barrier();
- asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_SC_MSR)
+ asm volatile ( ALTERNATIVE_2(ASM_NOP3,
+ "wrmsr", X86_FEATURE_SC_MSR_PV,
+ "wrmsr", X86_FEATURE_SC_MSR_HVM)
:: "a" (val), "c" (MSR_SPEC_CTRL), "d" (0) : "memory" );
}
@@ -71,7 +73,9 @@ static always_inline void spec_ctrl_exit_idle(struct cpu_info *info)
*/
info->spec_ctrl_flags &= ~SCF_use_shadow;
barrier();
- asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_SC_MSR)
+ asm volatile ( ALTERNATIVE_2(ASM_NOP3,
+ "wrmsr", X86_FEATURE_SC_MSR_PV,
+ "wrmsr", X86_FEATURE_SC_MSR_HVM)
:: "a" (val), "c" (MSR_SPEC_CTRL), "d" (0) : "memory" );
}
diff --git a/xen/include/asm-x86/spec_ctrl_asm.h b/xen/include/asm-x86/spec_ctrl_asm.h
index bf36b5a..edace2a 100644
--- a/xen/include/asm-x86/spec_ctrl_asm.h
+++ b/xen/include/asm-x86/spec_ctrl_asm.h
@@ -223,34 +223,34 @@
#define SPEC_CTRL_ENTRY_FROM_HVM \
ALTERNATIVE "", DO_OVERWRITE_RSB, X86_FEATURE_SC_RSB_HVM; \
ALTERNATIVE "", DO_SPEC_CTRL_ENTRY_FROM_HVM, \
- X86_FEATURE_SC_MSR
+ X86_FEATURE_SC_MSR_HVM
/* Use after an entry from PV context (syscall/sysenter/int80/int82/etc). */
#define SPEC_CTRL_ENTRY_FROM_PV \
ALTERNATIVE "", DO_OVERWRITE_RSB, X86_FEATURE_SC_RSB_PV; \
ALTERNATIVE "", __stringify(DO_SPEC_CTRL_ENTRY maybexen=0), \
- X86_FEATURE_SC_MSR
+ X86_FEATURE_SC_MSR_PV
/* Use in interrupt/exception context. May interrupt Xen or PV context. */
#define SPEC_CTRL_ENTRY_FROM_INTR \
ALTERNATIVE "", DO_OVERWRITE_RSB, X86_FEATURE_SC_RSB_PV; \
ALTERNATIVE "", __stringify(DO_SPEC_CTRL_ENTRY maybexen=1), \
- X86_FEATURE_SC_MSR
+ X86_FEATURE_SC_MSR_PV
/* Use when exiting to Xen context. */
#define SPEC_CTRL_EXIT_TO_XEN \
ALTERNATIVE "", \
- DO_SPEC_CTRL_EXIT_TO_XEN, X86_FEATURE_SC_MSR
+ DO_SPEC_CTRL_EXIT_TO_XEN, X86_FEATURE_SC_MSR_PV
/* Use when exiting to PV guest context. */
#define SPEC_CTRL_EXIT_TO_PV \
ALTERNATIVE "", \
- DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_SC_MSR
+ DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_SC_MSR_PV
/* Use when exiting to HVM guest context. */
#define SPEC_CTRL_EXIT_TO_HVM \
ALTERNATIVE "", \
- DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_SC_MSR
+ DO_SPEC_CTRL_EXIT_TO_GUEST, X86_FEATURE_SC_MSR_HVM
/*
* Use in IST interrupt/exception context. May interrupt Xen or PV context.
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 06/10] x86/spec_ctrl: Split X86_FEATURE_SC_MSR into PV and HVM variants
2018-05-11 10:38 ` [PATCH 06/10] x86/spec_ctrl: Split X86_FEATURE_SC_MSR into PV and HVM variants Andrew Cooper
@ 2018-05-14 15:22 ` Wei Liu
2018-05-14 15:27 ` Jan Beulich
1 sibling, 0 replies; 34+ messages in thread
From: Wei Liu @ 2018-05-14 15:22 UTC (permalink / raw)
To: Andrew Cooper
Cc: Juergen Gross, Roger Pau Monné, Wei Liu, Jan Beulich, Xen-devel
On Fri, May 11, 2018 at 11:38:10AM +0100, Andrew Cooper wrote:
> In order to separately control whether MSR_SPEC_CTRL is virtualised for PV and
> HVM guests, split the feature used to control runtime alternatives into two.
> Xen will use MSR_SPEC_CTRL itself if either of these features are active.
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Wei Liu <wei.liu2@citrix.com>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 06/10] x86/spec_ctrl: Split X86_FEATURE_SC_MSR into PV and HVM variants
2018-05-11 10:38 ` [PATCH 06/10] x86/spec_ctrl: Split X86_FEATURE_SC_MSR into PV and HVM variants Andrew Cooper
2018-05-14 15:22 ` Wei Liu
@ 2018-05-14 15:27 ` Jan Beulich
2018-05-15 19:52 ` Andrew Cooper
1 sibling, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2018-05-14 15:27 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Juergen Gross, Xen-devel, Wei Liu, Roger Pau Monne
>>> On 11.05.18 at 12:38, <andrew.cooper3@citrix.com> wrote:
> --- a/xen/arch/x86/spec_ctrl.c
> +++ b/xen/arch/x86/spec_ctrl.c
> @@ -128,7 +128,8 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
> thunk == THUNK_RETPOLINE ? "RETPOLINE" :
> thunk == THUNK_LFENCE ? "LFENCE" :
> thunk == THUNK_JMP ? "JMP" : "?",
> - boot_cpu_has(X86_FEATURE_SC_MSR) ?
> + (boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
> + boot_cpu_has(X86_FEATURE_SC_MSR_HVM)) ?
> default_xen_spec_ctrl & SPEC_CTRL_IBRS ? " IBRS+" :
> " IBRS-" : "",
> opt_ibpb ? " IBPB" : "",
> @@ -367,7 +368,8 @@ void __init init_speculation_mitigations(void)
> * need the IBRS entry/exit logic to virtualise IBRS support for
> * guests.
> */
> - setup_force_cpu_cap(X86_FEATURE_SC_MSR);
> + setup_force_cpu_cap(X86_FEATURE_SC_MSR_PV);
> + setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM);
Besides these sort of open coding alternative_io_2() (you'd really want an
output-less variant here, I agree) these are slightly bending the rules of
when/how to use multiple alternatives: The above ends up correct only
because of both replacements being identical.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 06/10] x86/spec_ctrl: Split X86_FEATURE_SC_MSR into PV and HVM variants
2018-05-14 15:27 ` Jan Beulich
@ 2018-05-15 19:52 ` Andrew Cooper
2018-05-16 6:38 ` Jan Beulich
0 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2018-05-15 19:52 UTC (permalink / raw)
To: Jan Beulich; +Cc: Juergen Gross, Xen-devel, Wei Liu, Roger Pau Monne
On 14/05/18 16:27, Jan Beulich wrote:
>>>> On 11.05.18 at 12:38, <andrew.cooper3@citrix.com> wrote:
>> --- a/xen/arch/x86/spec_ctrl.c
>> +++ b/xen/arch/x86/spec_ctrl.c
>> @@ -128,7 +128,8 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
>> thunk == THUNK_RETPOLINE ? "RETPOLINE" :
>> thunk == THUNK_LFENCE ? "LFENCE" :
>> thunk == THUNK_JMP ? "JMP" : "?",
>> - boot_cpu_has(X86_FEATURE_SC_MSR) ?
>> + (boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
>> + boot_cpu_has(X86_FEATURE_SC_MSR_HVM)) ?
>> default_xen_spec_ctrl & SPEC_CTRL_IBRS ? " IBRS+" :
>> " IBRS-" : "",
>> opt_ibpb ? " IBPB" : "",
>> @@ -367,7 +368,8 @@ void __init init_speculation_mitigations(void)
>> * need the IBRS entry/exit logic to virtualise IBRS support for
>> * guests.
>> */
>> - setup_force_cpu_cap(X86_FEATURE_SC_MSR);
>> + setup_force_cpu_cap(X86_FEATURE_SC_MSR_PV);
>> + setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM);
> Besides these sort of open coding alternative_io_2() (you'd really want an
> output-less variant here, I agree) these are slightly bending the rules of
> when/how to use multiple alternatives: The above ends up correct only
> because of both replacements being identical.
Actually, by reordering patch 10 ahead of this patch, we never get to
needing the ALTERNATIVE_2()'s in the first place, and lose any concerns
with bending the rules along the series.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 06/10] x86/spec_ctrl: Split X86_FEATURE_SC_MSR into PV and HVM variants
2018-05-15 19:52 ` Andrew Cooper
@ 2018-05-16 6:38 ` Jan Beulich
2018-05-16 10:28 ` Andrew Cooper
0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2018-05-16 6:38 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Juergen Gross, Xen-devel, Wei Liu, Roger Pau Monne
>>> On 15.05.18 at 21:52, <andrew.cooper3@citrix.com> wrote:
> On 14/05/18 16:27, Jan Beulich wrote:
>>>>> On 11.05.18 at 12:38, <andrew.cooper3@citrix.com> wrote:
>>> --- a/xen/arch/x86/spec_ctrl.c
>>> +++ b/xen/arch/x86/spec_ctrl.c
>>> @@ -128,7 +128,8 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
>>> thunk == THUNK_RETPOLINE ? "RETPOLINE" :
>>> thunk == THUNK_LFENCE ? "LFENCE" :
>>> thunk == THUNK_JMP ? "JMP" : "?",
>>> - boot_cpu_has(X86_FEATURE_SC_MSR) ?
>>> + (boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
>>> + boot_cpu_has(X86_FEATURE_SC_MSR_HVM)) ?
>>> default_xen_spec_ctrl & SPEC_CTRL_IBRS ? " IBRS+" :
>>> " IBRS-" : "",
>>> opt_ibpb ? " IBPB" : "",
>>> @@ -367,7 +368,8 @@ void __init init_speculation_mitigations(void)
>>> * need the IBRS entry/exit logic to virtualise IBRS support for
>>> * guests.
>>> */
>>> - setup_force_cpu_cap(X86_FEATURE_SC_MSR);
>>> + setup_force_cpu_cap(X86_FEATURE_SC_MSR_PV);
>>> + setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM);
>> Besides these sort of open coding alternative_io_2() (you'd really want an
>> output-less variant here, I agree) these are slightly bending the rules of
>> when/how to use multiple alternatives: The above ends up correct only
>> because of both replacements being identical.
>
> Actually, by reordering patch 10 ahead of this patch, we never get to
> needing the ALTERNATIVE_2()'s in the first place, and lose any concerns
> with bending the rules along the series.
Ah yes, indeed. And you would better use alternative_input() there then,
instead of open coding it.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 06/10] x86/spec_ctrl: Split X86_FEATURE_SC_MSR into PV and HVM variants
2018-05-16 6:38 ` Jan Beulich
@ 2018-05-16 10:28 ` Andrew Cooper
2018-05-16 10:49 ` Jan Beulich
0 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2018-05-16 10:28 UTC (permalink / raw)
To: Jan Beulich; +Cc: Juergen Gross, Xen-devel, Wei Liu, Roger Pau Monne
On 16/05/18 07:38, Jan Beulich wrote:
>>>> On 15.05.18 at 21:52, <andrew.cooper3@citrix.com> wrote:
>> On 14/05/18 16:27, Jan Beulich wrote:
>>>>>> On 11.05.18 at 12:38, <andrew.cooper3@citrix.com> wrote:
>>>> --- a/xen/arch/x86/spec_ctrl.c
>>>> +++ b/xen/arch/x86/spec_ctrl.c
>>>> @@ -128,7 +128,8 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
>>>> thunk == THUNK_RETPOLINE ? "RETPOLINE" :
>>>> thunk == THUNK_LFENCE ? "LFENCE" :
>>>> thunk == THUNK_JMP ? "JMP" : "?",
>>>> - boot_cpu_has(X86_FEATURE_SC_MSR) ?
>>>> + (boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
>>>> + boot_cpu_has(X86_FEATURE_SC_MSR_HVM)) ?
>>>> default_xen_spec_ctrl & SPEC_CTRL_IBRS ? " IBRS+" :
>>>> " IBRS-" : "",
>>>> opt_ibpb ? " IBPB" : "",
>>>> @@ -367,7 +368,8 @@ void __init init_speculation_mitigations(void)
>>>> * need the IBRS entry/exit logic to virtualise IBRS support for
>>>> * guests.
>>>> */
>>>> - setup_force_cpu_cap(X86_FEATURE_SC_MSR);
>>>> + setup_force_cpu_cap(X86_FEATURE_SC_MSR_PV);
>>>> + setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM);
>>> Besides these sort of open coding alternative_io_2() (you'd really want an
>>> output-less variant here, I agree) these are slightly bending the rules of
>>> when/how to use multiple alternatives: The above ends up correct only
>>> because of both replacements being identical.
>> Actually, by reordering patch 10 ahead of this patch, we never get to
>> needing the ALTERNATIVE_2()'s in the first place, and lose any concerns
>> with bending the rules along the series.
> Ah yes, indeed. And you would better use alternative_input() there then,
> instead of open coding it.
The reason this doesn't use alternative_input() at the moment is because
of the memory clobber. (And the lack of a memory clobber is called out
as a peculiarity in comment). The current code looks dangerously
inconsistent WRT barriers.
As for bending the rules, I now disagree with your assessment. The
alternative_*() wrappers do nothing but make it harder to express the
parameters, as perfectly demonstrated by the ASM_OUTPUT2() bodge.
I don't see their value, and they have a cost of making an asm volatile
statement not look and work quite as an asm volatile statement does in
all other callsites.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 06/10] x86/spec_ctrl: Split X86_FEATURE_SC_MSR into PV and HVM variants
2018-05-16 10:28 ` Andrew Cooper
@ 2018-05-16 10:49 ` Jan Beulich
2018-05-16 10:56 ` Andrew Cooper
0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2018-05-16 10:49 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Juergen Gross, Xen-devel, Wei Liu, Roger Pau Monne
>>> On 16.05.18 at 12:28, <andrew.cooper3@citrix.com> wrote:
> On 16/05/18 07:38, Jan Beulich wrote:
>>>>> On 15.05.18 at 21:52, <andrew.cooper3@citrix.com> wrote:
>>> On 14/05/18 16:27, Jan Beulich wrote:
>>>>>>> On 11.05.18 at 12:38, <andrew.cooper3@citrix.com> wrote:
>>>>> --- a/xen/arch/x86/spec_ctrl.c
>>>>> +++ b/xen/arch/x86/spec_ctrl.c
>>>>> @@ -128,7 +128,8 @@ static void __init print_details(enum ind_thunk thunk,
> uint64_t caps)
>>>>> thunk == THUNK_RETPOLINE ? "RETPOLINE" :
>>>>> thunk == THUNK_LFENCE ? "LFENCE" :
>>>>> thunk == THUNK_JMP ? "JMP" : "?",
>>>>> - boot_cpu_has(X86_FEATURE_SC_MSR) ?
>>>>> + (boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
>>>>> + boot_cpu_has(X86_FEATURE_SC_MSR_HVM)) ?
>>>>> default_xen_spec_ctrl & SPEC_CTRL_IBRS ? " IBRS+" :
>>>>> " IBRS-" : "",
>>>>> opt_ibpb ? " IBPB" : "",
>>>>> @@ -367,7 +368,8 @@ void __init init_speculation_mitigations(void)
>>>>> * need the IBRS entry/exit logic to virtualise IBRS support for
>>>>> * guests.
>>>>> */
>>>>> - setup_force_cpu_cap(X86_FEATURE_SC_MSR);
>>>>> + setup_force_cpu_cap(X86_FEATURE_SC_MSR_PV);
>>>>> + setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM);
>>>> Besides these sort of open coding alternative_io_2() (you'd really want an
>>>> output-less variant here, I agree) these are slightly bending the rules of
>>>> when/how to use multiple alternatives: The above ends up correct only
>>>> because of both replacements being identical.
>>> Actually, by reordering patch 10 ahead of this patch, we never get to
>>> needing the ALTERNATIVE_2()'s in the first place, and lose any concerns
>>> with bending the rules along the series.
>> Ah yes, indeed. And you would better use alternative_input() there then,
>> instead of open coding it.
>
> The reason this doesn't use alternative_input() at the moment is because
> of the memory clobber. (And the lack of a memory clobber is called out
> as a peculiarity in comment). The current code looks dangerously
> inconsistent WRT barriers.
>
> As for bending the rules, I now disagree with your assessment. The
> alternative_*() wrappers do nothing but make it harder to express the
> parameters, as perfectly demonstrated by the ASM_OUTPUT2() bodge.
The "bending the rules" comment was unrelated to alternative_*() vs
ALTERNATIVE*() use, and instead was solely related to there being a
dependency here on both pieces of replacement code being identical.
> I don't see their value, and they have a cost of making an asm volatile
> statement not look and work quite as an asm volatile statement does in
> all other callsites.
I don't mind consistency being achieved to other way around (i.e. by
dropping those wrappers). But I'd prefer if we didn't mix things unless
there's a compelling reason to do so.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH 06/10] x86/spec_ctrl: Split X86_FEATURE_SC_MSR into PV and HVM variants
2018-05-16 10:49 ` Jan Beulich
@ 2018-05-16 10:56 ` Andrew Cooper
0 siblings, 0 replies; 34+ messages in thread
From: Andrew Cooper @ 2018-05-16 10:56 UTC (permalink / raw)
To: Jan Beulich; +Cc: Juergen Gross, Xen-devel, Wei Liu, Roger Pau Monne
On 16/05/18 11:49, Jan Beulich wrote:
>>>> On 16.05.18 at 12:28, <andrew.cooper3@citrix.com> wrote:
>> On 16/05/18 07:38, Jan Beulich wrote:
>>>>>> On 15.05.18 at 21:52, <andrew.cooper3@citrix.com> wrote:
>>>> On 14/05/18 16:27, Jan Beulich wrote:
>>>>>>>> On 11.05.18 at 12:38, <andrew.cooper3@citrix.com> wrote:
>>>>>> --- a/xen/arch/x86/spec_ctrl.c
>>>>>> +++ b/xen/arch/x86/spec_ctrl.c
>>>>>> @@ -128,7 +128,8 @@ static void __init print_details(enum ind_thunk thunk,
>> uint64_t caps)
>>>>>> thunk == THUNK_RETPOLINE ? "RETPOLINE" :
>>>>>> thunk == THUNK_LFENCE ? "LFENCE" :
>>>>>> thunk == THUNK_JMP ? "JMP" : "?",
>>>>>> - boot_cpu_has(X86_FEATURE_SC_MSR) ?
>>>>>> + (boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
>>>>>> + boot_cpu_has(X86_FEATURE_SC_MSR_HVM)) ?
>>>>>> default_xen_spec_ctrl & SPEC_CTRL_IBRS ? " IBRS+" :
>>>>>> " IBRS-" : "",
>>>>>> opt_ibpb ? " IBPB" : "",
>>>>>> @@ -367,7 +368,8 @@ void __init init_speculation_mitigations(void)
>>>>>> * need the IBRS entry/exit logic to virtualise IBRS support for
>>>>>> * guests.
>>>>>> */
>>>>>> - setup_force_cpu_cap(X86_FEATURE_SC_MSR);
>>>>>> + setup_force_cpu_cap(X86_FEATURE_SC_MSR_PV);
>>>>>> + setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM);
>>>>> Besides these sort of open coding alternative_io_2() (you'd really want an
>>>>> output-less variant here, I agree) these are slightly bending the rules of
>>>>> when/how to use multiple alternatives: The above ends up correct only
>>>>> because of both replacements being identical.
>>>> Actually, by reordering patch 10 ahead of this patch, we never get to
>>>> needing the ALTERNATIVE_2()'s in the first place, and lose any concerns
>>>> with bending the rules along the series.
>>> Ah yes, indeed. And you would better use alternative_input() there then,
>>> instead of open coding it.
>> The reason this doesn't use alternative_input() at the moment is because
>> of the memory clobber. (And the lack of a memory clobber is called out
>> as a peculiarity in comment). The current code looks dangerously
>> inconsistent WRT barriers.
>>
>> As for bending the rules, I now disagree with your assessment. The
>> alternative_*() wrappers do nothing but make it harder to express the
>> parameters, as perfectly demonstrated by the ASM_OUTPUT2() bodge.
> The "bending the rules" comment was unrelated to alternative_*() vs
> ALTERNATIVE*() use, and instead was solely related to there being a
> dependency here on both pieces of replacement code being identical.
>
>> I don't see their value, and they have a cost of making an asm volatile
>> statement not look and work quite as an asm volatile statement does in
>> all other callsites.
> I don't mind consistency being achieved to other way around (i.e. by
> dropping those wrappers). But I'd prefer if we didn't mix things unless
> there's a compelling reason to do so.
I'll see about doing some cleanup of the overall tree for 4.12. For
now, its already mixed, and this doesn't make anything worse.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 07/10] x86/spec_ctrl: Explicitly set Xen's default MSR_SPEC_CTRL value
2018-05-11 10:38 [PATCH for-4.11 00/10] x86: Improvements and fixes to Spectre handling Andrew Cooper
` (5 preceding siblings ...)
2018-05-11 10:38 ` [PATCH 06/10] x86/spec_ctrl: Split X86_FEATURE_SC_MSR into PV and HVM variants Andrew Cooper
@ 2018-05-11 10:38 ` Andrew Cooper
2018-05-14 15:39 ` Wei Liu
2018-05-11 10:38 ` [PATCH 08/10] x86/cpuid: Improvements to guest policies for speculative sidechannel features Andrew Cooper
` (5 subsequent siblings)
12 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2018-05-11 10:38 UTC (permalink / raw)
To: Xen-devel
Cc: Juergen Gross, Wei Liu, Andrew Cooper, Zhenzhong Duan,
Jan Beulich, Boris Ostrovsky, Roger Pau Monné
With the impending ability to disable MSR_SPEC_CTRL handling on a
per-guest-type basis, the first exit-from-guest may not have the side effect
of loading Xen's choice of value. Explicitly set Xen's default during the BSP
and AP boot paths.
For the BSP however, delay setting a non-zero MSR_SPEC_CTRL default until
after dom0 has been constructed when safe to do so. Oracle report that this
speeds up boots of some hardware by 50s.
Reported-by: Zhenzhong Duan <zhenzhong.duan@oracle.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Zhenzhong Duan <zhenzhong.duan@oracle.com>
CC: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
CC: Boris Ostrovsky <boris.ostrovsky@oracle.com>
CC: Juergen Gross <jgross@suse.com>
---
xen/arch/x86/setup.c | 7 +++++++
xen/arch/x86/smpboot.c | 8 ++++++++
xen/arch/x86/spec_ctrl.c | 28 ++++++++++++++++++++++++++++
xen/include/asm-x86/spec_ctrl.h | 2 ++
4 files changed, 45 insertions(+)
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 164c42c..a3172ca 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1743,6 +1743,13 @@ void __init noreturn __start_xen(unsigned long mbi_p)
setup_io_bitmap(dom0);
+ if ( bsp_delay_spec_ctrl )
+ {
+ get_cpu_info()->spec_ctrl_flags &= ~SCF_use_shadow;
+ barrier();
+ wrmsrl(MSR_SPEC_CTRL, default_xen_spec_ctrl);
+ }
+
/* Jump to the 1:1 virtual mappings of cpu0_stack. */
asm volatile ("mov %[stk], %%rsp; jmp %c[fn]" ::
[stk] "g" (__va(__pa(get_stack_bottom()))),
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 86fa410..fd9050e 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -358,6 +358,14 @@ void start_secondary(void *unused)
else
microcode_resume_cpu(cpu);
+ /*
+ * If MSR_SPEC_CTRL is available, apply Xen's default setting and discard
+ * any firmware settings. Note: MSR_SPEC_CTRL may only become available
+ * after loading microcode.
+ */
+ if ( boot_cpu_has(X86_FEATURE_IBRSB) )
+ wrmsrl(MSR_SPEC_CTRL, default_xen_spec_ctrl);
+
if ( xen_guest )
hypervisor_ap_setup();
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index 0404962..de8b35f 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -38,6 +38,8 @@ static int8_t __initdata opt_ibrs = -1;
static bool __initdata opt_rsb_pv = true;
static bool __initdata opt_rsb_hvm = true;
bool __read_mostly opt_ibpb = true;
+
+bool __initdata bsp_delay_spec_ctrl;
uint8_t __read_mostly default_xen_spec_ctrl;
uint8_t __read_mostly default_spec_ctrl_flags;
@@ -417,6 +419,32 @@ void __init init_speculation_mitigations(void)
setup_clear_cpu_cap(X86_FEATURE_NO_XPTI);
print_details(thunk, caps);
+
+ /*
+ * If MSR_SPEC_CTRL is available, apply Xen's default setting and discard
+ * any firmware settings. For performance reasons on native hardware, we
+ * delay applying non-zero settings until after dom0 has been constructed.
+ */
+ if ( boot_cpu_has(X86_FEATURE_IBRSB) )
+ {
+ bsp_delay_spec_ctrl = !cpu_has_hypervisor && default_xen_spec_ctrl;
+
+ /*
+ * If delaying MSR_SPEC_CTRL setup, use the same mechanism as
+ * spec_ctrl_enter_idle(), by using a shadow value of zero.
+ */
+ if ( bsp_delay_spec_ctrl )
+ {
+ struct cpu_info *info = get_cpu_info();
+
+ info->shadow_spec_ctrl = 0;
+ barrier();
+ info->spec_ctrl_flags |= SCF_use_shadow;
+ barrier();
+ }
+
+ wrmsrl(MSR_SPEC_CTRL, bsp_delay_spec_ctrl ? 0 : default_xen_spec_ctrl);
+ }
}
static void __init __maybe_unused build_assertions(void)
diff --git a/xen/include/asm-x86/spec_ctrl.h b/xen/include/asm-x86/spec_ctrl.h
index 9880e19..bb4e7b2 100644
--- a/xen/include/asm-x86/spec_ctrl.h
+++ b/xen/include/asm-x86/spec_ctrl.h
@@ -27,6 +27,8 @@
void init_speculation_mitigations(void);
extern bool opt_ibpb;
+
+extern bool bsp_delay_spec_ctrl;
extern uint8_t default_xen_spec_ctrl;
extern uint8_t default_spec_ctrl_flags;
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 07/10] x86/spec_ctrl: Explicitly set Xen's default MSR_SPEC_CTRL value
2018-05-11 10:38 ` [PATCH 07/10] x86/spec_ctrl: Explicitly set Xen's default MSR_SPEC_CTRL value Andrew Cooper
@ 2018-05-14 15:39 ` Wei Liu
2018-05-14 15:52 ` Jan Beulich
0 siblings, 1 reply; 34+ messages in thread
From: Wei Liu @ 2018-05-14 15:39 UTC (permalink / raw)
To: Andrew Cooper
Cc: Juergen Gross, Wei Liu, Zhenzhong Duan, Xen-devel, Jan Beulich,
Boris Ostrovsky, Roger Pau Monné
On Fri, May 11, 2018 at 11:38:11AM +0100, Andrew Cooper wrote:
> @@ -417,6 +419,32 @@ void __init init_speculation_mitigations(void)
> setup_clear_cpu_cap(X86_FEATURE_NO_XPTI);
>
> print_details(thunk, caps);
> +
> + /*
> + * If MSR_SPEC_CTRL is available, apply Xen's default setting and discard
> + * any firmware settings. For performance reasons on native hardware, we
> + * delay applying non-zero settings until after dom0 has been constructed.
> + */
> + if ( boot_cpu_has(X86_FEATURE_IBRSB) )
> + {
> + bsp_delay_spec_ctrl = !cpu_has_hypervisor && default_xen_spec_ctrl;
> +
Why is cpu_has_hypervisor needed here? This should help nested case as
well. And it wouldn't make the setup less secure, right?
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 07/10] x86/spec_ctrl: Explicitly set Xen's default MSR_SPEC_CTRL value
2018-05-14 15:39 ` Wei Liu
@ 2018-05-14 15:52 ` Jan Beulich
2018-05-16 11:08 ` Andrew Cooper
0 siblings, 1 reply; 34+ messages in thread
From: Jan Beulich @ 2018-05-14 15:52 UTC (permalink / raw)
To: Andrew Cooper, Wei Liu
Cc: Juergen Gross, Zhenzhong Duan, Xen-devel, Boris Ostrovsky,
Roger Pau Monne
>>> On 14.05.18 at 17:39, <wei.liu2@citrix.com> wrote:
> On Fri, May 11, 2018 at 11:38:11AM +0100, Andrew Cooper wrote:
>> @@ -417,6 +419,32 @@ void __init init_speculation_mitigations(void)
>> setup_clear_cpu_cap(X86_FEATURE_NO_XPTI);
>>
>> print_details(thunk, caps);
>> +
>> + /*
>> + * If MSR_SPEC_CTRL is available, apply Xen's default setting and discard
>> + * any firmware settings. For performance reasons on native hardware, we
>> + * delay applying non-zero settings until after dom0 has been constructed.
>> + */
>> + if ( boot_cpu_has(X86_FEATURE_IBRSB) )
>> + {
>> + bsp_delay_spec_ctrl = !cpu_has_hypervisor && default_xen_spec_ctrl;
>> +
>
> Why is cpu_has_hypervisor needed here? This should help nested case as
> well. And it wouldn't make the setup less secure, right?
Ah, yes, Andrew, this should indeed be explained in at least one of comment
or commit message.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 07/10] x86/spec_ctrl: Explicitly set Xen's default MSR_SPEC_CTRL value
2018-05-14 15:52 ` Jan Beulich
@ 2018-05-16 11:08 ` Andrew Cooper
2018-05-16 11:12 ` Wei Liu
0 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2018-05-16 11:08 UTC (permalink / raw)
To: Jan Beulich, Wei Liu
Cc: Juergen Gross, Zhenzhong Duan, Xen-devel, Boris Ostrovsky,
Roger Pau Monne
On 14/05/18 16:52, Jan Beulich wrote:
>>>> On 14.05.18 at 17:39, <wei.liu2@citrix.com> wrote:
>> On Fri, May 11, 2018 at 11:38:11AM +0100, Andrew Cooper wrote:
>>> @@ -417,6 +419,32 @@ void __init init_speculation_mitigations(void)
>>> setup_clear_cpu_cap(X86_FEATURE_NO_XPTI);
>>>
>>> print_details(thunk, caps);
>>> +
>>> + /*
>>> + * If MSR_SPEC_CTRL is available, apply Xen's default setting and discard
>>> + * any firmware settings. For performance reasons on native hardware, we
>>> + * delay applying non-zero settings until after dom0 has been constructed.
>>> + */
>>> + if ( boot_cpu_has(X86_FEATURE_IBRSB) )
>>> + {
>>> + bsp_delay_spec_ctrl = !cpu_has_hypervisor && default_xen_spec_ctrl;
>>> +
>> Why is cpu_has_hypervisor needed here? This should help nested case as
>> well. And it wouldn't make the setup less secure, right?
> Ah, yes, Andrew, this should indeed be explained in at least one of comment
> or commit message.
I've adjusted this comment to read:
/*
* If MSR_SPEC_CTRL is available, apply Xen's default setting and discard
* any firmware settings. For performance reasons, when safe to do so, we
* delay applying non-zero settings until after dom0 has been constructed.
*
* "when safe to do so" is based on whether we are virtualised. A native
* boot won't have any other code running in a position to mount an
* attack.
*/
and added the same second paragraph to the commit message.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 07/10] x86/spec_ctrl: Explicitly set Xen's default MSR_SPEC_CTRL value
2018-05-16 11:08 ` Andrew Cooper
@ 2018-05-16 11:12 ` Wei Liu
0 siblings, 0 replies; 34+ messages in thread
From: Wei Liu @ 2018-05-16 11:12 UTC (permalink / raw)
To: Andrew Cooper
Cc: Juergen Gross, Wei Liu, Zhenzhong Duan, Xen-devel, Jan Beulich,
Boris Ostrovsky, Roger Pau Monne
On Wed, May 16, 2018 at 12:08:02PM +0100, Andrew Cooper wrote:
> On 14/05/18 16:52, Jan Beulich wrote:
> >>>> On 14.05.18 at 17:39, <wei.liu2@citrix.com> wrote:
> >> On Fri, May 11, 2018 at 11:38:11AM +0100, Andrew Cooper wrote:
> >>> @@ -417,6 +419,32 @@ void __init init_speculation_mitigations(void)
> >>> setup_clear_cpu_cap(X86_FEATURE_NO_XPTI);
> >>>
> >>> print_details(thunk, caps);
> >>> +
> >>> + /*
> >>> + * If MSR_SPEC_CTRL is available, apply Xen's default setting and discard
> >>> + * any firmware settings. For performance reasons on native hardware, we
> >>> + * delay applying non-zero settings until after dom0 has been constructed.
> >>> + */
> >>> + if ( boot_cpu_has(X86_FEATURE_IBRSB) )
> >>> + {
> >>> + bsp_delay_spec_ctrl = !cpu_has_hypervisor && default_xen_spec_ctrl;
> >>> +
> >> Why is cpu_has_hypervisor needed here? This should help nested case as
> >> well. And it wouldn't make the setup less secure, right?
> > Ah, yes, Andrew, this should indeed be explained in at least one of comment
> > or commit message.
>
> I've adjusted this comment to read:
>
> /*
> * If MSR_SPEC_CTRL is available, apply Xen's default setting and discard
> * any firmware settings. For performance reasons, when safe to do so, we
> * delay applying non-zero settings until after dom0 has been constructed.
> *
> * "when safe to do so" is based on whether we are virtualised. A native
> * boot won't have any other code running in a position to mount an
> * attack.
> */
>
> and added the same second paragraph to the commit message.
LGTM. Thanks!
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* [PATCH 08/10] x86/cpuid: Improvements to guest policies for speculative sidechannel features
2018-05-11 10:38 [PATCH for-4.11 00/10] x86: Improvements and fixes to Spectre handling Andrew Cooper
` (6 preceding siblings ...)
2018-05-11 10:38 ` [PATCH 07/10] x86/spec_ctrl: Explicitly set Xen's default MSR_SPEC_CTRL value Andrew Cooper
@ 2018-05-11 10:38 ` Andrew Cooper
2018-05-11 10:38 ` [PATCH 09/10] x86/spec_ctrl: Introduce a new `spec-ctrl=` command line argument to replace `bti=` Andrew Cooper
` (4 subsequent siblings)
12 siblings, 0 replies; 34+ messages in thread
From: Andrew Cooper @ 2018-05-11 10:38 UTC (permalink / raw)
To: Xen-devel
Cc: Juergen Gross, Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné
If Xen isn't virtualising MSR_SPEC_CTRL for guests, IBRSB shouldn't be
advertised. It is not currently possible to express this via the existing
command line options, but such an ability will be introduced.
Another useful option in some usecases is to offer IBPB without IBRS. When a
guest kernel is known to be compatible (uses retpoline and knows about the AMD
IBPB feature bit), an administrator with pre-Skylake hardware may wish to hide
IBRS. This allows the VM to have full protection, without Xen or the VM
needing to touch MSR_SPEC_CTRL, which can reduce the overhead of Spectre
mitigations.
Break the logic common to both PV and HVM CPUID calculations into a common
helper, to avoid duplication.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Juergen Gross <jgross@suse.com>
---
xen/arch/x86/cpuid.c | 60 ++++++++++++++++++++++++++++++++--------------------
1 file changed, 37 insertions(+), 23 deletions(-)
diff --git a/xen/arch/x86/cpuid.c b/xen/arch/x86/cpuid.c
index cff1a26..827b6c5 100644
--- a/xen/arch/x86/cpuid.c
+++ b/xen/arch/x86/cpuid.c
@@ -368,6 +368,28 @@ static void __init calculate_host_policy(void)
}
}
+static void __init guest_common_feature_adjustments(uint32_t *fs)
+{
+ /* Unconditionally claim to be able to set the hypervisor bit. */
+ __set_bit(X86_FEATURE_HYPERVISOR, fs);
+
+ /*
+ * If IBRS is offered to the guest, unconditionally offer STIBP. It is a
+ * nop on non-HT hardware, and has this behaviour to make heterogeneous
+ * setups easier to manage.
+ */
+ if ( test_bit(X86_FEATURE_IBRSB, fs) )
+ __set_bit(X86_FEATURE_STIBP, fs);
+
+ /*
+ * On hardware which supports IBRS/IBPB, we can offer IBPB independently
+ * of IBRS by using the AMD feature bit. An administrator may wish for
+ * performance reasons to offer IBPB without IBRS.
+ */
+ if ( host_cpuid_policy.feat.ibrsb )
+ __set_bit(X86_FEATURE_IBPB, fs);
+}
+
static void __init calculate_pv_max_policy(void)
{
struct cpuid_policy *p = &pv_max_cpuid_policy;
@@ -380,18 +402,14 @@ static void __init calculate_pv_max_policy(void)
for ( i = 0; i < ARRAY_SIZE(pv_featureset); ++i )
pv_featureset[i] &= pv_featuremask[i];
- /* Unconditionally claim to be able to set the hypervisor bit. */
- __set_bit(X86_FEATURE_HYPERVISOR, pv_featureset);
-
- /* On hardware with IBRS/IBPB support, there are further adjustments. */
- if ( test_bit(X86_FEATURE_IBRSB, pv_featureset) )
- {
- /* Offer STIBP unconditionally. It is a nop on non-HT hardware. */
- __set_bit(X86_FEATURE_STIBP, pv_featureset);
+ /*
+ * If Xen isn't virtualising MSR_SPEC_CTRL for PV guests because of
+ * administrator choice, hide the feature.
+ */
+ if ( !boot_cpu_has(X86_FEATURE_SC_MSR_PV) )
+ __clear_bit(X86_FEATURE_IBRSB, pv_featureset);
- /* AMD's IBPB is a subset of IBRS/IBPB. */
- __set_bit(X86_FEATURE_IBPB, pv_featureset);
- }
+ guest_common_feature_adjustments(pv_featureset);
sanitise_featureset(pv_featureset);
cpuid_featureset_to_policy(pv_featureset, p);
@@ -419,9 +437,6 @@ static void __init calculate_hvm_max_policy(void)
for ( i = 0; i < ARRAY_SIZE(hvm_featureset); ++i )
hvm_featureset[i] &= hvm_featuremask[i];
- /* Unconditionally claim to be able to set the hypervisor bit. */
- __set_bit(X86_FEATURE_HYPERVISOR, hvm_featureset);
-
/*
* Xen can provide an APIC emulation to HVM guests even if the host's APIC
* isn't enabled.
@@ -438,6 +453,13 @@ static void __init calculate_hvm_max_policy(void)
__set_bit(X86_FEATURE_SEP, hvm_featureset);
/*
+ * If Xen isn't virtualising MSR_SPEC_CTRL for HVM guests because of
+ * administrator choice, hide the feature.
+ */
+ if ( !boot_cpu_has(X86_FEATURE_SC_MSR_HVM) )
+ __clear_bit(X86_FEATURE_IBRSB, hvm_featureset);
+
+ /*
* With VT-x, some features are only supported by Xen if dedicated
* hardware support is also available.
*/
@@ -450,15 +472,7 @@ static void __init calculate_hvm_max_policy(void)
__clear_bit(X86_FEATURE_XSAVES, hvm_featureset);
}
- /* On hardware with IBRS/IBPB support, there are further adjustments. */
- if ( test_bit(X86_FEATURE_IBRSB, hvm_featureset) )
- {
- /* Offer STIBP unconditionally. It is a nop on non-HT hardware. */
- __set_bit(X86_FEATURE_STIBP, hvm_featureset);
-
- /* AMD's IBPB is a subset of IBRS/IBPB. */
- __set_bit(X86_FEATURE_IBPB, hvm_featureset);
- }
+ guest_common_feature_adjustments(hvm_featureset);
sanitise_featureset(hvm_featureset);
cpuid_featureset_to_policy(hvm_featureset, p);
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 09/10] x86/spec_ctrl: Introduce a new `spec-ctrl=` command line argument to replace `bti=`
2018-05-11 10:38 [PATCH for-4.11 00/10] x86: Improvements and fixes to Spectre handling Andrew Cooper
` (7 preceding siblings ...)
2018-05-11 10:38 ` [PATCH 08/10] x86/cpuid: Improvements to guest policies for speculative sidechannel features Andrew Cooper
@ 2018-05-11 10:38 ` Andrew Cooper
2018-05-11 10:38 ` [PATCH 10/10] x86/spec_ctrl: Elide MSR_SPEC_CTRL handling in idle context when possible Andrew Cooper
` (3 subsequent siblings)
12 siblings, 0 replies; 34+ messages in thread
From: Andrew Cooper @ 2018-05-11 10:38 UTC (permalink / raw)
To: Xen-devel
Cc: Juergen Gross, Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné
In hindsight, the options for `bti=` aren't as flexible or useful as expected
(including several options which don't appear to behave as intended).
Changing the behaviour of an existing option is problematic for compatibility,
so introduce a new `spec-ctrl=` in the hopes that we can do better.
One common way of deploying Xen is with a single PV dom0 and all domUs being
HVM domains. In such a setup, an administrator who has weighed up the risks
may wish to forgo protection against malicious PV domains, to reduce the
overall performance hit. To cater for this usecase, `spec-ctrl=no-pv` will
disable all speculative protection for PV domains, while leaving all
speculative protection for HVM domains intact.
For coding clarity as much as anything else, the suboptions are grouped by
logical area; those which affect the alternatives blocks, and those which
affect Xen's in-hypervisor settings. See the xen-command-line.markdown for
full details of the new options.
While changing the command line options, take the time to change how the data
is reported to the user. The three DEBUG printks are upgraded to unilateral,
as they are all relevant pieces of information, and the old "mitigations:"
line is split in the two logical areas described above.
Sample output from booting with `spec-ctrl=no-pv` looks like:
(XEN) Speculative mitigation facilities:
(XEN) Hardware features: IBRS/IBPB STIBP IBPB
(XEN) Compiled-in support: INDIRECT_THUNK
(XEN) Xen settings: BTI-Thunk RETPOLINE, SPEC_CTRL: IBRS-, Other: IBPB
(XEN) Support for VMs: PV: None, HVM: MSR_SPEC_CTRL RSB
(XEN) XPTI (64-bit PV only): Dom0 enabled, DomU enabled
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Juergen Gross <jgross@suse.com>
---
docs/misc/xen-command-line.markdown | 49 +++++++++++
xen/arch/x86/spec_ctrl.c | 164 ++++++++++++++++++++++++++++++------
2 files changed, 188 insertions(+), 25 deletions(-)
diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown
index 5b6571a..b6b1530 100644
--- a/docs/misc/xen-command-line.markdown
+++ b/docs/misc/xen-command-line.markdown
@@ -248,6 +248,9 @@ the NMI watchdog is also enabled.
### bti (x86)
> `= List of [ <bool>, thunk=retpoline|lfence|jmp, ibrs=<bool>, ibpb=<bool>, rsb=<bool>, rsb_{vmexit,native}=<bool> ]`
+**WARNING: This command line option is deprecated, and superseded by
+_spec-ctrl=_ - using both options in combination is undefined.**
+
Branch Target Injection controls. By default, Xen will pick the most
appropriate BTI mitigations based on compiled in support, loaded microcode,
and hardware details.
@@ -1752,6 +1755,52 @@ enforces the maximum theoretically necessary timeout of 670ms. Any number
is being interpreted as a custom timeout in milliseconds. Zero or boolean
false disable the quirk workaround, which is also the default.
+### spec-ctrl (x86)
+> `= List of [ <bool>, xen=<bool>, {pv,hvm,msr-sc,rsb}=<bool>,
+> bti-thunk=retpoline|lfence|jmp, {ibrs,ibpb}=<bool> ]`
+
+Controls for speculative execution sidechannel mitigations. By default, Xen
+will pick the most appropriate mitigations based on compiled in support,
+loaded microcode, and hardware details, and will virtualise appropriate
+mitigations for guests to use.
+
+**WARNING: Any use of this option may interfere with heuristics. Use with
+extreme care.**
+
+An overall boolean value, `spec-ctrl=no`, can be specified to turn off all
+mitigations, including pieces of infrastructure used to virtualise certain
+mitigation features for guests. Alternatively, a slightly more restricted
+`spec-ctrl=no-xen` can be used to turn off all of Xen's mitigations, while
+leaving the virtualisation support in place for guests to use. Use of a
+positive boolean value for either of these options is invalid.
+
+The booleans `pv=`, `hvm=`, `msr-sc=` and `rsb=` offer fine grained control
+over the alternative blocks used by Xen. These impact Xen's ability to
+protect itself, and Xen's ability to virtualise support for guests to use.
+
+* `pv=` and `hvm=` offer control over all suboptions for PV and HVM guests
+ respectively.
+* `msr-sc=` offers control over Xen's support for manipulating MSR\_SPEC\_CTRL
+ on entry and exit. These blocks are necessary to virtualise support for
+ guests and if disabled, guests will be unable to use IBRS/STIBP/etc.
+* `rsb=` offers control over whether to overwrite the Return Stack Buffer /
+ Return Address Stack on entry to Xen.
+
+If Xen was compiled with INDIRECT\_THUNK support, `bti-thunk=` can be used to
+select which of the thunks gets patched into the `__x86_indirect_thunk_%reg`
+locations. The default thunk is `retpoline` (generally preferred for Intel
+hardware), with the alternatives being `jmp` (a `jmp *%reg` gadget, minimal
+overhead), and `lfence` (an `lfence; jmp *%reg` gadget, preferred for AMD).
+
+On hardware supporting IBRS (Indirect Branch Restricted Speculation), the
+`ibrs=` option can be used to force or prevent Xen using the feature itself.
+If Xen is not using IBRS itself, functionality is still set up so IBRS can be
+virtualised for guests.
+
+On hardware supporting IBPB (Indirect Branch Prediction Barrier), the `ibpb=`
+option can be used to force (the default) or prevent Xen from issuing branch
+prediction barriers on vcpu context switches.
+
### sync\_console
> `= <boolean>`
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index de8b35f..a2328bd 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -26,6 +26,13 @@
#include <asm/spec_ctrl.h>
#include <asm/spec_ctrl_asm.h>
+/* Cmdline controls for Xen's alternative blocks. */
+static bool __initdata opt_msr_sc_pv = true;
+static bool __initdata opt_msr_sc_hvm = true;
+static bool __initdata opt_rsb_pv = true;
+static bool __initdata opt_rsb_hvm = true;
+
+/* Cmdline controls for Xen's speculative settings. */
static enum ind_thunk {
THUNK_DEFAULT, /* Decide which thunk to use at boot time. */
THUNK_NONE, /* Missing compiler support for thunks. */
@@ -35,8 +42,6 @@ static enum ind_thunk {
THUNK_JMP,
} opt_thunk __initdata = THUNK_DEFAULT;
static int8_t __initdata opt_ibrs = -1;
-static bool __initdata opt_rsb_pv = true;
-static bool __initdata opt_rsb_hvm = true;
bool __read_mostly opt_ibpb = true;
bool __initdata bsp_delay_spec_ctrl;
@@ -100,8 +105,95 @@ static int __init parse_bti(const char *s)
}
custom_param("bti", parse_bti);
+static int __init parse_spec_ctrl(const char *s)
+{
+ const char *ss;
+ int val, rc = 0;
+
+ do {
+ ss = strchr(s, ',');
+ if ( !ss )
+ ss = strchr(s, '\0');
+
+ /* Global and Xen-wide disable. */
+ val = parse_bool(s, ss);
+ if ( !val )
+ {
+ opt_msr_sc_pv = false;
+ opt_msr_sc_hvm = false;
+
+ disable_common:
+ opt_rsb_pv = false;
+ opt_rsb_hvm = false;
+
+ opt_thunk = THUNK_JMP;
+ opt_ibrs = 0;
+ opt_ibpb = false;
+ }
+ else if ( val > 0 )
+ rc = -EINVAL;
+ else if ( (val = parse_boolean("xen", s, ss)) >= 0 )
+ {
+ if ( !val )
+ goto disable_common;
+
+ rc = -EINVAL;
+ }
+
+ /* Xen's alternative blocks. */
+ else if ( (val = parse_boolean("pv", s, ss)) >= 0 )
+ {
+ opt_msr_sc_pv = val;
+ opt_rsb_pv = val;
+ }
+ else if ( (val = parse_boolean("hvm", s, ss)) >= 0 )
+ {
+ opt_msr_sc_hvm = val;
+ opt_rsb_hvm = val;
+ }
+ else if ( (val = parse_boolean("msr-sc", s, ss)) >= 0 )
+ {
+ opt_msr_sc_pv = val;
+ opt_msr_sc_hvm = val;
+ }
+ else if ( (val = parse_boolean("rsb", s, ss)) >= 0 )
+ {
+ opt_rsb_pv = val;
+ opt_rsb_hvm = val;
+ }
+
+ /* Xen's speculative sidechannel mitigation settings. */
+ else if ( !strncmp(s, "bti-thunk=", 10) )
+ {
+ s += 10;
+
+ if ( !strncmp(s, "retpoline", ss - s) )
+ opt_thunk = THUNK_RETPOLINE;
+ else if ( !strncmp(s, "lfence", ss - s) )
+ opt_thunk = THUNK_LFENCE;
+ else if ( !strncmp(s, "jmp", ss - s) )
+ opt_thunk = THUNK_JMP;
+ else
+ rc = -EINVAL;
+ }
+ else if ( (val = parse_boolean("ibrs", s, ss)) >= 0 )
+ opt_ibrs = val;
+ else if ( (val = parse_boolean("ibpb", s, ss)) >= 0 )
+ opt_ibpb = val;
+ else
+ rc = -EINVAL;
+
+ s = ss + 1;
+ } while ( *ss );
+
+ return rc;
+}
+custom_param("spec-ctrl", parse_spec_ctrl);
+
static void __init print_details(enum ind_thunk thunk, uint64_t caps)
{
+ bool use_spec_ctrl = (boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
+ boot_cpu_has(X86_FEATURE_SC_MSR_HVM));
unsigned int _7d0 = 0, e8b = 0, tmp;
/* Collect diagnostics about available mitigations. */
@@ -110,10 +202,10 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
if ( boot_cpu_data.extended_cpuid_level >= 0x80000008 )
cpuid(0x80000008, &tmp, &e8b, &tmp, &tmp);
- printk(XENLOG_DEBUG "Speculative mitigation facilities:\n");
+ printk("Speculative mitigation facilities:\n");
/* Hardware features which pertain to speculative mitigations. */
- printk(XENLOG_DEBUG " Hardware features:%s%s%s%s%s%s\n",
+ printk(" Hardware features:%s%s%s%s%s%s\n",
(_7d0 & cpufeat_mask(X86_FEATURE_IBRSB)) ? " IBRS/IBPB" : "",
(_7d0 & cpufeat_mask(X86_FEATURE_STIBP)) ? " STIBP" : "",
(e8b & cpufeat_mask(X86_FEATURE_IBPB)) ? " IBPB" : "",
@@ -123,22 +215,33 @@ static void __init print_details(enum ind_thunk thunk, uint64_t caps)
/* Compiled-in support which pertains to BTI mitigations. */
if ( IS_ENABLED(CONFIG_INDIRECT_THUNK) )
- printk(XENLOG_DEBUG " Compiled-in support: INDIRECT_THUNK\n");
+ printk(" Compiled-in support: INDIRECT_THUNK\n");
- printk("BTI mitigations: Thunk %s, Others:%s%s%s%s\n",
+ /* Settings for Xen's protection, irrespective of guests. */
+ printk(" Xen settings: BTI-Thunk %s, SPEC_CTRL: %s, Other:%s\n",
thunk == THUNK_NONE ? "N/A" :
thunk == THUNK_RETPOLINE ? "RETPOLINE" :
thunk == THUNK_LFENCE ? "LFENCE" :
thunk == THUNK_JMP ? "JMP" : "?",
+ !use_spec_ctrl ? "No" :
+ (default_xen_spec_ctrl & SPEC_CTRL_IBRS) ? "IBRS+" : "IBRS-",
+ opt_ibpb ? " IBPB" : "");
+
+ /*
+ * Alternatives blocks for protecting against and/or virtualising
+ * mitigation support for guests.
+ */
+ printk(" Support for VMs: PV:%s%s%s, HVM:%s%s%s\n",
(boot_cpu_has(X86_FEATURE_SC_MSR_PV) ||
- boot_cpu_has(X86_FEATURE_SC_MSR_HVM)) ?
- default_xen_spec_ctrl & SPEC_CTRL_IBRS ? " IBRS+" :
- " IBRS-" : "",
- opt_ibpb ? " IBPB" : "",
- boot_cpu_has(X86_FEATURE_SC_RSB_PV) ? " RSB_NATIVE" : "",
- boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ? " RSB_VMEXIT" : "");
-
- printk("XPTI (64-bit PV only): Dom0 %s, DomU %s\n",
+ boot_cpu_has(X86_FEATURE_SC_RSB_PV)) ? "" : " None",
+ boot_cpu_has(X86_FEATURE_SC_MSR_PV) ? " MSR_SPEC_CTRL" : "",
+ boot_cpu_has(X86_FEATURE_SC_RSB_PV) ? " RSB" : "",
+ (boot_cpu_has(X86_FEATURE_SC_MSR_HVM) ||
+ boot_cpu_has(X86_FEATURE_SC_RSB_HVM)) ? "" : " None",
+ boot_cpu_has(X86_FEATURE_SC_MSR_HVM) ? " MSR_SPEC_CTRL" : "",
+ boot_cpu_has(X86_FEATURE_SC_RSB_HVM) ? " RSB" : "");
+
+ printk(" XPTI (64-bit PV only): Dom0 %s, DomU %s\n",
opt_xpti & OPT_XPTI_DOM0 ? "enabled" : "disabled",
opt_xpti & OPT_XPTI_DOMU ? "enabled" : "disabled");
}
@@ -293,7 +396,7 @@ custom_param("xpti", parse_xpti);
void __init init_speculation_mitigations(void)
{
enum ind_thunk thunk = THUNK_DEFAULT;
- bool ibrs = false;
+ bool use_spec_ctrl = false, ibrs = false;
uint64_t caps = 0;
if ( boot_cpu_has(X86_FEATURE_ARCH_CAPS) )
@@ -363,20 +466,31 @@ void __init init_speculation_mitigations(void)
else if ( thunk == THUNK_JMP )
setup_force_cpu_cap(X86_FEATURE_IND_THUNK_JMP);
+ /*
+ * If we are on hardware supporting MSR_SPEC_CTRL, see about setting up
+ * the alternatives blocks so we can virtualise support for guests.
+ */
if ( boot_cpu_has(X86_FEATURE_IBRSB) )
{
- /*
- * Even if we've chosen to not have IBRS set in Xen context, we still
- * need the IBRS entry/exit logic to virtualise IBRS support for
- * guests.
- */
- setup_force_cpu_cap(X86_FEATURE_SC_MSR_PV);
- setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM);
+ if ( opt_msr_sc_pv )
+ {
+ use_spec_ctrl = true;
+ setup_force_cpu_cap(X86_FEATURE_SC_MSR_PV);
+ }
- if ( ibrs )
- default_xen_spec_ctrl |= SPEC_CTRL_IBRS;
+ if ( opt_msr_sc_hvm )
+ {
+ use_spec_ctrl = true;
+ setup_force_cpu_cap(X86_FEATURE_SC_MSR_HVM);
+ }
- default_spec_ctrl_flags |= SCF_ist_wrmsr;
+ if ( use_spec_ctrl )
+ {
+ if ( ibrs )
+ default_xen_spec_ctrl |= SPEC_CTRL_IBRS;
+
+ default_spec_ctrl_flags |= SCF_ist_wrmsr;
+ }
}
/*
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread* [PATCH 10/10] x86/spec_ctrl: Elide MSR_SPEC_CTRL handling in idle context when possible
2018-05-11 10:38 [PATCH for-4.11 00/10] x86: Improvements and fixes to Spectre handling Andrew Cooper
` (8 preceding siblings ...)
2018-05-11 10:38 ` [PATCH 09/10] x86/spec_ctrl: Introduce a new `spec-ctrl=` command line argument to replace `bti=` Andrew Cooper
@ 2018-05-11 10:38 ` Andrew Cooper
2018-05-14 15:48 ` Wei Liu
2018-05-14 9:23 ` [PATCH for-4.11 00/10] x86: Improvements and fixes to Spectre handling Wei Liu
` (2 subsequent siblings)
12 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2018-05-11 10:38 UTC (permalink / raw)
To: Xen-devel
Cc: Juergen Gross, Andrew Cooper, Wei Liu, Jan Beulich, Roger Pau Monné
If Xen is virtualising MSR_SPEC_CTRL handling for guests, but using 0 as its
own MSR_SPEC_CTRL value, spec_ctrl_{enter,exit}_idle() need not write to the
MSR.
Requested-by: Jan Beulich <JBeulich@suse.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Juergen Gross <jgross@suse.com>
---
xen/arch/x86/spec_ctrl.c | 4 ++++
xen/include/asm-x86/cpufeatures.h | 1 +
xen/include/asm-x86/spec_ctrl.h | 8 ++------
3 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
index a2328bd..f4a3165 100644
--- a/xen/arch/x86/spec_ctrl.c
+++ b/xen/arch/x86/spec_ctrl.c
@@ -526,6 +526,10 @@ void __init init_speculation_mitigations(void)
/* (Re)init BSP state now that default_spec_ctrl_flags has been calculated. */
init_shadow_spec_ctrl_state();
+ /* If Xen is using any MSR_SPEC_CTRL settings, adjust the idle path. */
+ if ( default_xen_spec_ctrl )
+ setup_force_cpu_cap(X86_FEATURE_SC_MSR_IDLE);
+
xpti_init_default(false);
if ( opt_xpti == 0 )
setup_force_cpu_cap(X86_FEATURE_NO_XPTI);
diff --git a/xen/include/asm-x86/cpufeatures.h b/xen/include/asm-x86/cpufeatures.h
index 9d5d81e..b90aa2d 100644
--- a/xen/include/asm-x86/cpufeatures.h
+++ b/xen/include/asm-x86/cpufeatures.h
@@ -31,3 +31,4 @@ XEN_CPUFEATURE(SC_MSR_HVM, (FSCAPINTS+0)*32+17) /* MSR_SPEC_CTRL used by Xe
XEN_CPUFEATURE(SC_RSB_PV, (FSCAPINTS+0)*32+18) /* RSB overwrite needed for PV */
XEN_CPUFEATURE(SC_RSB_HVM, (FSCAPINTS+0)*32+19) /* RSB overwrite needed for HVM */
XEN_CPUFEATURE(NO_XPTI, (FSCAPINTS+0)*32+20) /* XPTI mitigation not in use */
+XEN_CPUFEATURE(SC_MSR_IDLE, (FSCAPINTS+0)*32+21) /* (SC_MSR_PV || SC_MSR_HVM) && default_xen_spec_ctrl */
diff --git a/xen/include/asm-x86/spec_ctrl.h b/xen/include/asm-x86/spec_ctrl.h
index bb4e7b2..993b958 100644
--- a/xen/include/asm-x86/spec_ctrl.h
+++ b/xen/include/asm-x86/spec_ctrl.h
@@ -58,9 +58,7 @@ static always_inline void spec_ctrl_enter_idle(struct cpu_info *info)
barrier();
info->spec_ctrl_flags |= SCF_use_shadow;
barrier();
- asm volatile ( ALTERNATIVE_2(ASM_NOP3,
- "wrmsr", X86_FEATURE_SC_MSR_PV,
- "wrmsr", X86_FEATURE_SC_MSR_HVM)
+ asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_SC_MSR_IDLE)
:: "a" (val), "c" (MSR_SPEC_CTRL), "d" (0) : "memory" );
}
@@ -75,9 +73,7 @@ static always_inline void spec_ctrl_exit_idle(struct cpu_info *info)
*/
info->spec_ctrl_flags &= ~SCF_use_shadow;
barrier();
- asm volatile ( ALTERNATIVE_2(ASM_NOP3,
- "wrmsr", X86_FEATURE_SC_MSR_PV,
- "wrmsr", X86_FEATURE_SC_MSR_HVM)
+ asm volatile ( ALTERNATIVE(ASM_NOP3, "wrmsr", X86_FEATURE_SC_MSR_IDLE)
:: "a" (val), "c" (MSR_SPEC_CTRL), "d" (0) : "memory" );
}
--
2.1.4
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply related [flat|nested] 34+ messages in thread* Re: [PATCH 10/10] x86/spec_ctrl: Elide MSR_SPEC_CTRL handling in idle context when possible
2018-05-11 10:38 ` [PATCH 10/10] x86/spec_ctrl: Elide MSR_SPEC_CTRL handling in idle context when possible Andrew Cooper
@ 2018-05-14 15:48 ` Wei Liu
2018-05-16 11:27 ` Andrew Cooper
0 siblings, 1 reply; 34+ messages in thread
From: Wei Liu @ 2018-05-14 15:48 UTC (permalink / raw)
To: Andrew Cooper
Cc: Juergen Gross, Roger Pau Monné, Wei Liu, Jan Beulich, Xen-devel
On Fri, May 11, 2018 at 11:38:14AM +0100, Andrew Cooper wrote:
> If Xen is virtualising MSR_SPEC_CTRL handling for guests, but using 0 as its
> own MSR_SPEC_CTRL value, spec_ctrl_{enter,exit}_idle() need not write to the
> MSR.
>
> Requested-by: Jan Beulich <JBeulich@suse.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Juergen Gross <jgross@suse.com>
> ---
> xen/arch/x86/spec_ctrl.c | 4 ++++
> xen/include/asm-x86/cpufeatures.h | 1 +
> xen/include/asm-x86/spec_ctrl.h | 8 ++------
> 3 files changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
> index a2328bd..f4a3165 100644
> --- a/xen/arch/x86/spec_ctrl.c
> +++ b/xen/arch/x86/spec_ctrl.c
> @@ -526,6 +526,10 @@ void __init init_speculation_mitigations(void)
> /* (Re)init BSP state now that default_spec_ctrl_flags has been calculated. */
> init_shadow_spec_ctrl_state();
>
> + /* If Xen is using any MSR_SPEC_CTRL settings, adjust the idle path. */
> + if ( default_xen_spec_ctrl )
> + setup_force_cpu_cap(X86_FEATURE_SC_MSR_IDLE);
> +
> xpti_init_default(false);
> if ( opt_xpti == 0 )
> setup_force_cpu_cap(X86_FEATURE_NO_XPTI);
> diff --git a/xen/include/asm-x86/cpufeatures.h b/xen/include/asm-x86/cpufeatures.h
> index 9d5d81e..b90aa2d 100644
> --- a/xen/include/asm-x86/cpufeatures.h
> +++ b/xen/include/asm-x86/cpufeatures.h
> @@ -31,3 +31,4 @@ XEN_CPUFEATURE(SC_MSR_HVM, (FSCAPINTS+0)*32+17) /* MSR_SPEC_CTRL used by Xe
> XEN_CPUFEATURE(SC_RSB_PV, (FSCAPINTS+0)*32+18) /* RSB overwrite needed for PV */
> XEN_CPUFEATURE(SC_RSB_HVM, (FSCAPINTS+0)*32+19) /* RSB overwrite needed for HVM */
> XEN_CPUFEATURE(NO_XPTI, (FSCAPINTS+0)*32+20) /* XPTI mitigation not in use */
> +XEN_CPUFEATURE(SC_MSR_IDLE, (FSCAPINTS+0)*32+21) /* (SC_MSR_PV || SC_MSR_HVM) && default_xen_spec_ctrl */
I don't follow: the code above only depends on default_xen_spec_ctrl but
the comment says SC_MSR_PV and SC_MSR_HVM are also taken into
consideration.
The rest looks fine.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 10/10] x86/spec_ctrl: Elide MSR_SPEC_CTRL handling in idle context when possible
2018-05-14 15:48 ` Wei Liu
@ 2018-05-16 11:27 ` Andrew Cooper
2018-05-16 11:28 ` Wei Liu
0 siblings, 1 reply; 34+ messages in thread
From: Andrew Cooper @ 2018-05-16 11:27 UTC (permalink / raw)
To: Wei Liu; +Cc: Juergen Gross, Roger Pau Monné, Jan Beulich, Xen-devel
On 14/05/18 16:48, Wei Liu wrote:
> On Fri, May 11, 2018 at 11:38:14AM +0100, Andrew Cooper wrote:
>> If Xen is virtualising MSR_SPEC_CTRL handling for guests, but using 0 as its
>> own MSR_SPEC_CTRL value, spec_ctrl_{enter,exit}_idle() need not write to the
>> MSR.
>>
>> Requested-by: Jan Beulich <JBeulich@suse.com>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> ---
>> CC: Jan Beulich <JBeulich@suse.com>
>> CC: Wei Liu <wei.liu2@citrix.com>
>> CC: Roger Pau Monné <roger.pau@citrix.com>
>> CC: Juergen Gross <jgross@suse.com>
>> ---
>> xen/arch/x86/spec_ctrl.c | 4 ++++
>> xen/include/asm-x86/cpufeatures.h | 1 +
>> xen/include/asm-x86/spec_ctrl.h | 8 ++------
>> 3 files changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
>> index a2328bd..f4a3165 100644
>> --- a/xen/arch/x86/spec_ctrl.c
>> +++ b/xen/arch/x86/spec_ctrl.c
>> @@ -526,6 +526,10 @@ void __init init_speculation_mitigations(void)
>> /* (Re)init BSP state now that default_spec_ctrl_flags has been calculated. */
>> init_shadow_spec_ctrl_state();
>>
>> + /* If Xen is using any MSR_SPEC_CTRL settings, adjust the idle path. */
>> + if ( default_xen_spec_ctrl )
>> + setup_force_cpu_cap(X86_FEATURE_SC_MSR_IDLE);
>> +
>> xpti_init_default(false);
>> if ( opt_xpti == 0 )
>> setup_force_cpu_cap(X86_FEATURE_NO_XPTI);
>> diff --git a/xen/include/asm-x86/cpufeatures.h b/xen/include/asm-x86/cpufeatures.h
>> index 9d5d81e..b90aa2d 100644
>> --- a/xen/include/asm-x86/cpufeatures.h
>> +++ b/xen/include/asm-x86/cpufeatures.h
>> @@ -31,3 +31,4 @@ XEN_CPUFEATURE(SC_MSR_HVM, (FSCAPINTS+0)*32+17) /* MSR_SPEC_CTRL used by Xe
>> XEN_CPUFEATURE(SC_RSB_PV, (FSCAPINTS+0)*32+18) /* RSB overwrite needed for PV */
>> XEN_CPUFEATURE(SC_RSB_HVM, (FSCAPINTS+0)*32+19) /* RSB overwrite needed for HVM */
>> XEN_CPUFEATURE(NO_XPTI, (FSCAPINTS+0)*32+20) /* XPTI mitigation not in use */
>> +XEN_CPUFEATURE(SC_MSR_IDLE, (FSCAPINTS+0)*32+21) /* (SC_MSR_PV || SC_MSR_HVM) && default_xen_spec_ctrl */
> I don't follow: the code above only depends on default_xen_spec_ctrl but
> the comment says SC_MSR_PV and SC_MSR_HVM are also taken into
> consideration.
default_xen_spec_ctrl is only ever nonzero when SC_MSR_PV or SC_MSR_HVM
is set. In principle, I could assert that one of the two is set.
I have phrased it like this because it is going to become rather more
complicated (than != 0) when adding IBRS_ATT mode.
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH 10/10] x86/spec_ctrl: Elide MSR_SPEC_CTRL handling in idle context when possible
2018-05-16 11:27 ` Andrew Cooper
@ 2018-05-16 11:28 ` Wei Liu
0 siblings, 0 replies; 34+ messages in thread
From: Wei Liu @ 2018-05-16 11:28 UTC (permalink / raw)
To: Andrew Cooper
Cc: Juergen Gross, Roger Pau Monné, Wei Liu, Jan Beulich, Xen-devel
On Wed, May 16, 2018 at 12:27:29PM +0100, Andrew Cooper wrote:
> On 14/05/18 16:48, Wei Liu wrote:
> > On Fri, May 11, 2018 at 11:38:14AM +0100, Andrew Cooper wrote:
> >> If Xen is virtualising MSR_SPEC_CTRL handling for guests, but using 0 as its
> >> own MSR_SPEC_CTRL value, spec_ctrl_{enter,exit}_idle() need not write to the
> >> MSR.
> >>
> >> Requested-by: Jan Beulich <JBeulich@suse.com>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> >> ---
> >> CC: Jan Beulich <JBeulich@suse.com>
> >> CC: Wei Liu <wei.liu2@citrix.com>
> >> CC: Roger Pau Monné <roger.pau@citrix.com>
> >> CC: Juergen Gross <jgross@suse.com>
> >> ---
> >> xen/arch/x86/spec_ctrl.c | 4 ++++
> >> xen/include/asm-x86/cpufeatures.h | 1 +
> >> xen/include/asm-x86/spec_ctrl.h | 8 ++------
> >> 3 files changed, 7 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/xen/arch/x86/spec_ctrl.c b/xen/arch/x86/spec_ctrl.c
> >> index a2328bd..f4a3165 100644
> >> --- a/xen/arch/x86/spec_ctrl.c
> >> +++ b/xen/arch/x86/spec_ctrl.c
> >> @@ -526,6 +526,10 @@ void __init init_speculation_mitigations(void)
> >> /* (Re)init BSP state now that default_spec_ctrl_flags has been calculated. */
> >> init_shadow_spec_ctrl_state();
> >>
> >> + /* If Xen is using any MSR_SPEC_CTRL settings, adjust the idle path. */
> >> + if ( default_xen_spec_ctrl )
> >> + setup_force_cpu_cap(X86_FEATURE_SC_MSR_IDLE);
> >> +
> >> xpti_init_default(false);
> >> if ( opt_xpti == 0 )
> >> setup_force_cpu_cap(X86_FEATURE_NO_XPTI);
> >> diff --git a/xen/include/asm-x86/cpufeatures.h b/xen/include/asm-x86/cpufeatures.h
> >> index 9d5d81e..b90aa2d 100644
> >> --- a/xen/include/asm-x86/cpufeatures.h
> >> +++ b/xen/include/asm-x86/cpufeatures.h
> >> @@ -31,3 +31,4 @@ XEN_CPUFEATURE(SC_MSR_HVM, (FSCAPINTS+0)*32+17) /* MSR_SPEC_CTRL used by Xe
> >> XEN_CPUFEATURE(SC_RSB_PV, (FSCAPINTS+0)*32+18) /* RSB overwrite needed for PV */
> >> XEN_CPUFEATURE(SC_RSB_HVM, (FSCAPINTS+0)*32+19) /* RSB overwrite needed for HVM */
> >> XEN_CPUFEATURE(NO_XPTI, (FSCAPINTS+0)*32+20) /* XPTI mitigation not in use */
> >> +XEN_CPUFEATURE(SC_MSR_IDLE, (FSCAPINTS+0)*32+21) /* (SC_MSR_PV || SC_MSR_HVM) && default_xen_spec_ctrl */
> > I don't follow: the code above only depends on default_xen_spec_ctrl but
> > the comment says SC_MSR_PV and SC_MSR_HVM are also taken into
> > consideration.
>
> default_xen_spec_ctrl is only ever nonzero when SC_MSR_PV or SC_MSR_HVM
> is set. In principle, I could assert that one of the two is set.
>
> I have phrased it like this because it is going to become rather more
> complicated (than != 0) when adding IBRS_ATT mode.
OK, thanks for explaining.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread
* Re: [PATCH for-4.11 00/10] x86: Improvements and fixes to Spectre handling
2018-05-11 10:38 [PATCH for-4.11 00/10] x86: Improvements and fixes to Spectre handling Andrew Cooper
` (9 preceding siblings ...)
2018-05-11 10:38 ` [PATCH 10/10] x86/spec_ctrl: Elide MSR_SPEC_CTRL handling in idle context when possible Andrew Cooper
@ 2018-05-14 9:23 ` Wei Liu
2018-05-14 15:31 ` Jan Beulich
2018-05-15 18:25 ` Juergen Gross
12 siblings, 0 replies; 34+ messages in thread
From: Wei Liu @ 2018-05-14 9:23 UTC (permalink / raw)
To: Andrew Cooper
Cc: Juergen Gross, Wei Liu, Anthony Liguori, Zhenzhong Duan,
Xen-devel, Martin Pohlack, Jan Beulich, Boris Ostrovsky,
David Woodhouse, Roger Pau Monné
On Fri, May 11, 2018 at 11:38:04AM +0100, Andrew Cooper wrote:
> In hindsight, the end result of the Spectre mitigations aren't as great as I'd
> hoped, and have several inefficiencies. Also, the `bti=` command line option
> isn't as flexible as intended.
I think this is a good argument for this series to be in for 4.11.
Wei.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH for-4.11 00/10] x86: Improvements and fixes to Spectre handling
2018-05-11 10:38 [PATCH for-4.11 00/10] x86: Improvements and fixes to Spectre handling Andrew Cooper
` (10 preceding siblings ...)
2018-05-14 9:23 ` [PATCH for-4.11 00/10] x86: Improvements and fixes to Spectre handling Wei Liu
@ 2018-05-14 15:31 ` Jan Beulich
2018-05-15 18:25 ` Juergen Gross
12 siblings, 0 replies; 34+ messages in thread
From: Jan Beulich @ 2018-05-14 15:31 UTC (permalink / raw)
To: Andrew Cooper
Cc: Juergen Gross, Wei Liu, Zhenzhong Duan, Xen-devel,
Martin Pohlack, Anthony Liguori, Boris Ostrovsky,
David Woodhouse, Roger Pau Monne
>>> On 11.05.18 at 12:38, <andrew.cooper3@citrix.com> wrote:
> In hindsight, the end result of the Spectre mitigations aren't as great as I'd
> hoped, and have several inefficiencies. Also, the `bti=` command line option
> isn't as flexible as intended.
>
> This series does four things:
>
> 1) Some internal cleanup, for clarity and to help the other features
> 2) Introduce `spec-ctrl=no-pv` mode. XenServer's performance measurements
> see a 10% net/disk performance improvement in some production scenarios.
> 3) Introduce the ability to use IBPB-only mode for guests. This was
> discussed by Amazon during the Spectre work, but I don't have any
> performance numbers to hand.
> 4) Avoid imposing IBRS mode while dom0 is booting. This was reported by
> Oracle on the list, and speeds up boot time on some servers by 50s.
>
> I know this series is rather late for 4.11, but seeing as I've managed to
> complete it before 4.12 opens, it should be considered at this point, as all
> of the Spectre code is new in 4.11.
>
> Andrew Cooper (10):
> x86/spec_ctrl: Read MSR_ARCH_CAPABILITIES only once
> x86/spec_ctrl: Express Xen's choice of MSR_SPEC_CTRL value as a variable
> x86/spec_ctrl: Merge bti_ist_info and use_shadow_spec_ctrl into spec_ctrl_flags
> x86/spec_ctrl: Fold the XEN_IBRS_{SET,CLEAR} ALTERNATIVES together
> x86/spec_ctrl: Rename bits of infrastructure to avoid NATIVE and VMEXIT
> x86/spec_ctrl: Split X86_FEATURE_SC_MSR into PV and HVM variants
> x86/spec_ctrl: Explicitly set Xen's default MSR_SPEC_CTRL value
> x86/cpuid: Improvements to guest policies for speculative sidechannel features
> x86/spec_ctrl: Introduce a new `spec-ctrl=` command line argument to replace `bti=`
> x86/spec_ctrl: Elide MSR_SPEC_CTRL handling in idle context when possible
Irrespective of the small comment just sent for patch 6:
Reviewed-by: Jan Beulich <jbeulich@suse.com>
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread* Re: [PATCH for-4.11 00/10] x86: Improvements and fixes to Spectre handling
2018-05-11 10:38 [PATCH for-4.11 00/10] x86: Improvements and fixes to Spectre handling Andrew Cooper
` (11 preceding siblings ...)
2018-05-14 15:31 ` Jan Beulich
@ 2018-05-15 18:25 ` Juergen Gross
12 siblings, 0 replies; 34+ messages in thread
From: Juergen Gross @ 2018-05-15 18:25 UTC (permalink / raw)
To: Andrew Cooper, Xen-devel
Cc: Wei Liu, Anthony Liguori, Zhenzhong Duan, Martin Pohlack,
Jan Beulich, Boris Ostrovsky, David Woodhouse,
Roger Pau Monné
On 11/05/18 12:38, Andrew Cooper wrote:
> In hindsight, the end result of the Spectre mitigations aren't as great as I'd
> hoped, and have several inefficiencies. Also, the `bti=` command line option
> isn't as flexible as intended.
>
> This series does four things:
>
> 1) Some internal cleanup, for clarity and to help the other features
> 2) Introduce `spec-ctrl=no-pv` mode. XenServer's performance measurements
> see a 10% net/disk performance improvement in some production scenarios.
> 3) Introduce the ability to use IBPB-only mode for guests. This was
> discussed by Amazon during the Spectre work, but I don't have any
> performance numbers to hand.
> 4) Avoid imposing IBRS mode while dom0 is booting. This was reported by
> Oracle on the list, and speeds up boot time on some servers by 50s.
>
> I know this series is rather late for 4.11, but seeing as I've managed to
> complete it before 4.12 opens, it should be considered at this point, as all
> of the Spectre code is new in 4.11.
>
> Andrew Cooper (10):
> x86/spec_ctrl: Read MSR_ARCH_CAPABILITIES only once
> x86/spec_ctrl: Express Xen's choice of MSR_SPEC_CTRL value as a variable
> x86/spec_ctrl: Merge bti_ist_info and use_shadow_spec_ctrl into spec_ctrl_flags
> x86/spec_ctrl: Fold the XEN_IBRS_{SET,CLEAR} ALTERNATIVES together
> x86/spec_ctrl: Rename bits of infrastructure to avoid NATIVE and VMEXIT
> x86/spec_ctrl: Split X86_FEATURE_SC_MSR into PV and HVM variants
> x86/spec_ctrl: Explicitly set Xen's default MSR_SPEC_CTRL value
> x86/cpuid: Improvements to guest policies for speculative sidechannel features
> x86/spec_ctrl: Introduce a new `spec-ctrl=` command line argument to replace `bti=`
> x86/spec_ctrl: Elide MSR_SPEC_CTRL handling in idle context when possible
>
> docs/misc/xen-command-line.markdown | 49 +++++++
> xen/arch/x86/acpi/power.c | 4 +-
> xen/arch/x86/cpuid.c | 60 +++++----
> xen/arch/x86/hvm/svm/entry.S | 4 +-
> xen/arch/x86/hvm/vmx/entry.S | 4 +-
> xen/arch/x86/setup.c | 7 +
> xen/arch/x86/smpboot.c | 8 ++
> xen/arch/x86/spec_ctrl.c | 258 ++++++++++++++++++++++++++++--------
> xen/arch/x86/x86_64/asm-offsets.c | 4 +-
> xen/arch/x86/x86_64/compat/entry.S | 2 +-
> xen/arch/x86/x86_64/entry.S | 2 +-
> xen/include/asm-x86/cpufeatures.h | 9 +-
> xen/include/asm-x86/current.h | 4 +-
> xen/include/asm-x86/spec_ctrl.h | 20 +--
> xen/include/asm-x86/spec_ctrl_asm.h | 131 +++++++++---------
> 15 files changed, 396 insertions(+), 170 deletions(-)
>
For the series:
Release-acked-by: Juergen Gross <jgross@suse.com>
Juergen
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 34+ messages in thread