| Message ID | 20180605152303.14450-6-julien.grall@arm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | xen/arm: SSBD (aka Spectre-v4) mitigation (XSA-263) | expand |
On Tue, 5 Jun 2018, Julien Grall wrote: > On a system where the firmware implements ARCH_WORKAROUND_2, it may be > useful to either permanently enable or disable the workaround for cases > where the user decides that they'd rather not get a trap overhead, and > keep the mitigation permanently on or off instead of switching it on > exception entry/exit. In any case, default to mitigation being enabled. > > The new command line option is implemented as list of one option to > follow x86 option and also allow to extend it more easily in the future. > > Note that for convenience, the full implemention of the workaround is > done in the .matches callback. > > Lastly, a accessor is provided to know the state of the mitigation. > > After this patch, there are 3 methods complementing each other to find the > state of the mitigation: > - The capability ARM_SSBD indicates the platform is affected by the > vulnerability. This will also return false if the user decide to force > disabled the mitigation (spec-ctrl="ssbd=force-disable"). The > capability is useful for putting shortcut in place using alternative. > - ssbd_state indicates the global state of the mitigation (e.g > unknown, force enable...). The global state is required to report > the state to a guest. > - The per-cpu ssbd_callback_required indicates whether a pCPU > requires to call the SMC. This allows to shortcut SMC call > and save an entry/exit to EL3. > > This is part of XSA-263. > > Signed-off-by: Julien Grall <julien.grall@arm.com> > > --- > Changes in v2: > - Move out some code to the previous patch. > - Update the commit message with more background > --- > docs/misc/xen-command-line.markdown | 18 ++++++++ > xen/arch/arm/cpuerrata.c | 91 +++++++++++++++++++++++++++++++++---- > xen/include/asm-arm/cpuerrata.h | 21 +++++++++ > 3 files changed, 122 insertions(+), 8 deletions(-) > > diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown > index 8712a833a2..962028b6ed 100644 > --- a/docs/misc/xen-command-line.markdown > +++ b/docs/misc/xen-command-line.markdown > @@ -1756,6 +1756,24 @@ 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 (Arm) > +> `= List of [ ssbd=force-disable|runtime|force-enable ]` > + > +Controls for speculative execution sidechannel mitigations. > + > +The option `ssbd=` is used to control the state of Speculative Store > +Bypass Disable (SSBD) mitigation. > + > +* `ssbd=force-disable` will keep the mitigation permanently off. The guest > +will not be able to control the state of the mitigation. > +* `ssbd=runtime` will always turn on the mitigation when running in the > +hypervisor context. The guest will be to turn on/off the mitigation for > +itself by using the firmware interface ARCH\_WORKAROUND\_2. > +* `ssbd=force-enable` will keep the mitigation permanently on. The guest will > +not be able to control the state of the mitigation. > + > +By default SSBD will be mitigated at runtime (i.e `ssbd=runtime`). > + > ### spec-ctrl (x86) > > `= List of [ <bool>, xen=<bool>, {pv,hvm,msr-sc,rsb}=<bool>, > > bti-thunk=retpoline|lfence|jmp, {ibrs,ibpb,ssbd}=<bool> ]` > diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c > index aa86c7c0fe..4292008692 100644 > --- a/xen/arch/arm/cpuerrata.c > +++ b/xen/arch/arm/cpuerrata.c > @@ -237,6 +237,41 @@ static int enable_ic_inv_hardening(void *data) > > #ifdef CONFIG_ARM_SSBD > > +enum ssbd_state ssbd_state = ARM_SSBD_RUNTIME; > + > +static int __init parse_spec_ctrl(const char *s) > +{ > + const char *ss; > + int rc = 0; > + > + do { > + ss = strchr(s, ','); > + if ( !ss ) > + ss = strchr(s, '\0'); > + > + if ( !strncmp(s, "ssbd=", 5) ) > + { > + s += 5; > + > + if ( !strncmp(s, "force-disable", ss - s) ) > + ssbd_state = ARM_SSBD_FORCE_DISABLE; > + else if ( !strncmp(s, "runtime", ss - s) ) > + ssbd_state = ARM_SSBD_RUNTIME; > + else if ( !strncmp(s, "force-enable", ss - s) ) > + ssbd_state = ARM_SSBD_FORCE_ENABLE; > + else > + rc = -EINVAL; > + } > + else > + rc = -EINVAL; > + > + s = ss + 1; > + } while ( *ss ); > + > + return rc; > +} > +custom_param("spec-ctrl", parse_spec_ctrl); > + > /* > * Assembly code may use the variable directly, so we need to make sure > * it fits in a register. > @@ -251,19 +286,17 @@ static bool has_ssbd_mitigation(const struct arm_cpu_capabilities *entry) > if ( smccc_ver < SMCCC_VERSION(1, 1) ) > return false; > > - /* > - * The probe function return value is either negative (unsupported > - * or mitigated), positive (unaffected), or zero (requires > - * mitigation). We only need to do anything in the last case. > - */ > arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FID, > ARM_SMCCC_ARCH_WORKAROUND_2_FID, &res); > + spurious change > switch ( (int)res.a0 ) > { > case ARM_SMCCC_NOT_SUPPORTED: > + ssbd_state = ARM_SSBD_UNKNOWN; > return false; > > case ARM_SMCCC_NOT_REQUIRED: > + ssbd_state = ARM_SSBD_MITIGATED; > return false; > > case ARM_SMCCC_SUCCESS: > @@ -271,7 +304,7 @@ static bool has_ssbd_mitigation(const struct arm_cpu_capabilities *entry) > break; > > case 1: /* Mitigation not required on this CPU. */ > - required = true; > + required = false; > break; > > default: > @@ -279,8 +312,49 @@ static bool has_ssbd_mitigation(const struct arm_cpu_capabilities *entry) > return false; > } > > - if ( required ) > - this_cpu(ssbd_callback_required) = 1; > + switch ( ssbd_state ) > + { > + case ARM_SSBD_FORCE_DISABLE: > + { > + static bool once = true; > + > + if ( once ) > + printk("%s disabled from command-line\n", entry->desc); > + once = false; > + > + arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 0, NULL); > + required = false; > + > + break; > + } > + > + case ARM_SSBD_RUNTIME: > + if ( required ) > + { > + this_cpu(ssbd_callback_required) = 1; > + arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL); > + } > + > + break; > + > + case ARM_SSBD_FORCE_ENABLE: > + { > + static bool once = true; > + > + if ( once ) > + printk("%s forced from command-line\n", entry->desc); > + once = false; > + > + arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL); > + required = true; > + > + break; > + } > + > + default: > + ASSERT_UNREACHABLE(); > + return false; > + } > > return required; > } > @@ -389,6 +463,7 @@ static const struct arm_cpu_capabilities arm_errata[] = { > #endif > #ifdef CONFIG_ARM_SSBD > { > + .desc = "Speculative Store Bypass Disabled", > .capability = ARM_SSBD, > .matches = has_ssbd_mitigation, > }, > diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h > index e628d3ff56..7fbb3dc0be 100644 > --- a/xen/include/asm-arm/cpuerrata.h > +++ b/xen/include/asm-arm/cpuerrata.h > @@ -31,10 +31,26 @@ CHECK_WORKAROUND_HELPER(ssbd, ARM_SSBD, CONFIG_ARM_SSBD) > > #undef CHECK_WORKAROUND_HELPER > > +enum ssbd_state > +{ > + ARM_SSBD_UNKNOWN, > + ARM_SSBD_FORCE_DISABLE, > + ARM_SSBD_RUNTIME, > + ARM_SSBD_FORCE_ENABLE, > + ARM_SSBD_MITIGATED, > +}; > + > #ifdef CONFIG_ARM_SSBD > > #include <asm/current.h> > > +extern enum ssbd_state ssbd_state; > + > +static inline enum ssbd_state get_ssbd_state(void) > +{ > + return ssbd_state; > +} > + > DECLARE_PER_CPU(register_t, ssbd_callback_required); > > static inline bool cpu_require_ssbd_mitigation(void) > @@ -49,6 +65,11 @@ static inline bool cpu_require_ssbd_mitigation(void) > return false; > } > > +static inline enum ssbd_state get_sbdd_state(void) the mistype is still present > +{ > + return ARM_SSBD_UNKNOWN; > +} > + > #endif > > #endif /* __ARM_CPUERRATA_H__ */ > -- > 2.11.0 >
Hi Stefano, On 12/06/18 00:15, Stefano Stabellini wrote: > On Tue, 5 Jun 2018, Julien Grall wrote: >> /* >> * Assembly code may use the variable directly, so we need to make sure >> * it fits in a register. >> @@ -251,19 +286,17 @@ static bool has_ssbd_mitigation(const struct arm_cpu_capabilities *entry) >> if ( smccc_ver < SMCCC_VERSION(1, 1) ) >> return false; >> >> - /* >> - * The probe function return value is either negative (unsupported >> - * or mitigated), positive (unaffected), or zero (requires >> - * mitigation). We only need to do anything in the last case. >> - */ >> arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FID, >> ARM_SMCCC_ARCH_WORKAROUND_2_FID, &res); >> + > > spurious change It just belongs to the previous patch. This is due to the code reshuffling you requested. [...] >> diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h >> index e628d3ff56..7fbb3dc0be 100644 >> --- a/xen/include/asm-arm/cpuerrata.h >> +++ b/xen/include/asm-arm/cpuerrata.h >> @@ -31,10 +31,26 @@ CHECK_WORKAROUND_HELPER(ssbd, ARM_SSBD, CONFIG_ARM_SSBD) >> >> #undef CHECK_WORKAROUND_HELPER >> >> +enum ssbd_state >> +{ >> + ARM_SSBD_UNKNOWN, >> + ARM_SSBD_FORCE_DISABLE, >> + ARM_SSBD_RUNTIME, >> + ARM_SSBD_FORCE_ENABLE, >> + ARM_SSBD_MITIGATED, >> +}; >> + >> #ifdef CONFIG_ARM_SSBD >> >> #include <asm/current.h> >> >> +extern enum ssbd_state ssbd_state; >> + >> +static inline enum ssbd_state get_ssbd_state(void) >> +{ >> + return ssbd_state; >> +} >> + >> DECLARE_PER_CPU(register_t, ssbd_callback_required); >> >> static inline bool cpu_require_ssbd_mitigation(void) >> @@ -49,6 +65,11 @@ static inline bool cpu_require_ssbd_mitigation(void) >> return false; >> } >> >> +static inline enum ssbd_state get_sbdd_state(void) > > the mistype is still present Fixed now. Cheers,
diff --git a/docs/misc/xen-command-line.markdown b/docs/misc/xen-command-line.markdown index 8712a833a2..962028b6ed 100644 --- a/docs/misc/xen-command-line.markdown +++ b/docs/misc/xen-command-line.markdown @@ -1756,6 +1756,24 @@ 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 (Arm) +> `= List of [ ssbd=force-disable|runtime|force-enable ]` + +Controls for speculative execution sidechannel mitigations. + +The option `ssbd=` is used to control the state of Speculative Store +Bypass Disable (SSBD) mitigation. + +* `ssbd=force-disable` will keep the mitigation permanently off. The guest +will not be able to control the state of the mitigation. +* `ssbd=runtime` will always turn on the mitigation when running in the +hypervisor context. The guest will be to turn on/off the mitigation for +itself by using the firmware interface ARCH\_WORKAROUND\_2. +* `ssbd=force-enable` will keep the mitigation permanently on. The guest will +not be able to control the state of the mitigation. + +By default SSBD will be mitigated at runtime (i.e `ssbd=runtime`). + ### spec-ctrl (x86) > `= List of [ <bool>, xen=<bool>, {pv,hvm,msr-sc,rsb}=<bool>, > bti-thunk=retpoline|lfence|jmp, {ibrs,ibpb,ssbd}=<bool> ]` diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c index aa86c7c0fe..4292008692 100644 --- a/xen/arch/arm/cpuerrata.c +++ b/xen/arch/arm/cpuerrata.c @@ -237,6 +237,41 @@ static int enable_ic_inv_hardening(void *data) #ifdef CONFIG_ARM_SSBD +enum ssbd_state ssbd_state = ARM_SSBD_RUNTIME; + +static int __init parse_spec_ctrl(const char *s) +{ + const char *ss; + int rc = 0; + + do { + ss = strchr(s, ','); + if ( !ss ) + ss = strchr(s, '\0'); + + if ( !strncmp(s, "ssbd=", 5) ) + { + s += 5; + + if ( !strncmp(s, "force-disable", ss - s) ) + ssbd_state = ARM_SSBD_FORCE_DISABLE; + else if ( !strncmp(s, "runtime", ss - s) ) + ssbd_state = ARM_SSBD_RUNTIME; + else if ( !strncmp(s, "force-enable", ss - s) ) + ssbd_state = ARM_SSBD_FORCE_ENABLE; + else + rc = -EINVAL; + } + else + rc = -EINVAL; + + s = ss + 1; + } while ( *ss ); + + return rc; +} +custom_param("spec-ctrl", parse_spec_ctrl); + /* * Assembly code may use the variable directly, so we need to make sure * it fits in a register. @@ -251,19 +286,17 @@ static bool has_ssbd_mitigation(const struct arm_cpu_capabilities *entry) if ( smccc_ver < SMCCC_VERSION(1, 1) ) return false; - /* - * The probe function return value is either negative (unsupported - * or mitigated), positive (unaffected), or zero (requires - * mitigation). We only need to do anything in the last case. - */ arm_smccc_1_1_smc(ARM_SMCCC_ARCH_FEATURES_FID, ARM_SMCCC_ARCH_WORKAROUND_2_FID, &res); + switch ( (int)res.a0 ) { case ARM_SMCCC_NOT_SUPPORTED: + ssbd_state = ARM_SSBD_UNKNOWN; return false; case ARM_SMCCC_NOT_REQUIRED: + ssbd_state = ARM_SSBD_MITIGATED; return false; case ARM_SMCCC_SUCCESS: @@ -271,7 +304,7 @@ static bool has_ssbd_mitigation(const struct arm_cpu_capabilities *entry) break; case 1: /* Mitigation not required on this CPU. */ - required = true; + required = false; break; default: @@ -279,8 +312,49 @@ static bool has_ssbd_mitigation(const struct arm_cpu_capabilities *entry) return false; } - if ( required ) - this_cpu(ssbd_callback_required) = 1; + switch ( ssbd_state ) + { + case ARM_SSBD_FORCE_DISABLE: + { + static bool once = true; + + if ( once ) + printk("%s disabled from command-line\n", entry->desc); + once = false; + + arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 0, NULL); + required = false; + + break; + } + + case ARM_SSBD_RUNTIME: + if ( required ) + { + this_cpu(ssbd_callback_required) = 1; + arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL); + } + + break; + + case ARM_SSBD_FORCE_ENABLE: + { + static bool once = true; + + if ( once ) + printk("%s forced from command-line\n", entry->desc); + once = false; + + arm_smccc_1_1_smc(ARM_SMCCC_ARCH_WORKAROUND_2_FID, 1, NULL); + required = true; + + break; + } + + default: + ASSERT_UNREACHABLE(); + return false; + } return required; } @@ -389,6 +463,7 @@ static const struct arm_cpu_capabilities arm_errata[] = { #endif #ifdef CONFIG_ARM_SSBD { + .desc = "Speculative Store Bypass Disabled", .capability = ARM_SSBD, .matches = has_ssbd_mitigation, }, diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h index e628d3ff56..7fbb3dc0be 100644 --- a/xen/include/asm-arm/cpuerrata.h +++ b/xen/include/asm-arm/cpuerrata.h @@ -31,10 +31,26 @@ CHECK_WORKAROUND_HELPER(ssbd, ARM_SSBD, CONFIG_ARM_SSBD) #undef CHECK_WORKAROUND_HELPER +enum ssbd_state +{ + ARM_SSBD_UNKNOWN, + ARM_SSBD_FORCE_DISABLE, + ARM_SSBD_RUNTIME, + ARM_SSBD_FORCE_ENABLE, + ARM_SSBD_MITIGATED, +}; + #ifdef CONFIG_ARM_SSBD #include <asm/current.h> +extern enum ssbd_state ssbd_state; + +static inline enum ssbd_state get_ssbd_state(void) +{ + return ssbd_state; +} + DECLARE_PER_CPU(register_t, ssbd_callback_required); static inline bool cpu_require_ssbd_mitigation(void) @@ -49,6 +65,11 @@ static inline bool cpu_require_ssbd_mitigation(void) return false; } +static inline enum ssbd_state get_sbdd_state(void) +{ + return ARM_SSBD_UNKNOWN; +} + #endif #endif /* __ARM_CPUERRATA_H__ */
On a system where the firmware implements ARCH_WORKAROUND_2, it may be useful to either permanently enable or disable the workaround for cases where the user decides that they'd rather not get a trap overhead, and keep the mitigation permanently on or off instead of switching it on exception entry/exit. In any case, default to mitigation being enabled. The new command line option is implemented as list of one option to follow x86 option and also allow to extend it more easily in the future. Note that for convenience, the full implemention of the workaround is done in the .matches callback. Lastly, a accessor is provided to know the state of the mitigation. After this patch, there are 3 methods complementing each other to find the state of the mitigation: - The capability ARM_SSBD indicates the platform is affected by the vulnerability. This will also return false if the user decide to force disabled the mitigation (spec-ctrl="ssbd=force-disable"). The capability is useful for putting shortcut in place using alternative. - ssbd_state indicates the global state of the mitigation (e.g unknown, force enable...). The global state is required to report the state to a guest. - The per-cpu ssbd_callback_required indicates whether a pCPU requires to call the SMC. This allows to shortcut SMC call and save an entry/exit to EL3. This is part of XSA-263. Signed-off-by: Julien Grall <julien.grall@arm.com> --- Changes in v2: - Move out some code to the previous patch. - Update the commit message with more background --- docs/misc/xen-command-line.markdown | 18 ++++++++ xen/arch/arm/cpuerrata.c | 91 +++++++++++++++++++++++++++++++++---- xen/include/asm-arm/cpuerrata.h | 21 +++++++++ 3 files changed, 122 insertions(+), 8 deletions(-)