| Message ID | 20180116142337.24942-6-julien.grall@linaro.org |
|---|---|
| State | Accepted |
| Commit | e730f8e41e8537f1db9770b9464f9523c28857b9 |
| Headers | show |
| Series | xen/arm64: Branch predictor hardening (XSA-254 variant 2) | expand |
On Tue, 16 Jan 2018, Julien Grall wrote: > Cortex-A57, A72, A73 and A75 are susceptible to branch predictor > aliasing and can theoritically be attacked by malicious code. > > This patch implements a PSCI-based mitigation for these CPUs when > available. The call into firmware will invalidate the branch predictor > state, preventing any malicious entries from affection other victim > contexts. > > Ported from Linux git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git > branch kpti. > > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > Signed-off-by: Will Deacon <will.deacon@arm.com> > > This is part of XSA-254. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> > --- > xen/arch/arm/arm64/bpi.S | 25 ++++++++++++++++++++++++ > xen/arch/arm/cpuerrata.c | 49 ++++++++++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 74 insertions(+) > > diff --git a/xen/arch/arm/arm64/bpi.S b/xen/arch/arm/arm64/bpi.S > index 6cc2f17529..4b7f1dc21f 100644 > --- a/xen/arch/arm/arm64/bpi.S > +++ b/xen/arch/arm/arm64/bpi.S > @@ -56,6 +56,31 @@ ENTRY(__bp_harden_hyp_vecs_start) > .endr > ENTRY(__bp_harden_hyp_vecs_end) > > +ENTRY(__psci_hyp_bp_inval_start) > + sub sp, sp, #(8 * 18) > + stp x16, x17, [sp, #(16 * 0)] > + stp x14, x15, [sp, #(16 * 1)] > + stp x12, x13, [sp, #(16 * 2)] > + stp x10, x11, [sp, #(16 * 3)] > + stp x8, x9, [sp, #(16 * 4)] > + stp x6, x7, [sp, #(16 * 5)] > + stp x4, x5, [sp, #(16 * 6)] > + stp x2, x3, [sp, #(16 * 7)] > + stp x0, x1, [sp, #(16 * 8)] > + mov x0, #0x84000000 > + smc #0 > + ldp x16, x17, [sp, #(16 * 0)] > + ldp x14, x15, [sp, #(16 * 1)] > + ldp x12, x13, [sp, #(16 * 2)] > + ldp x10, x11, [sp, #(16 * 3)] > + ldp x8, x9, [sp, #(16 * 4)] > + ldp x6, x7, [sp, #(16 * 5)] > + ldp x4, x5, [sp, #(16 * 6)] > + ldp x2, x3, [sp, #(16 * 7)] > + ldp x0, x1, [sp, #(16 * 8)] > + add sp, sp, #(8 * 18) > +ENTRY(__psci_hyp_bp_inval_end) > + > /* > * Local variables: > * mode: ASM > diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c > index 76d98e771d..f1ea7f3c5b 100644 > --- a/xen/arch/arm/cpuerrata.c > +++ b/xen/arch/arm/cpuerrata.c > @@ -4,8 +4,10 @@ > #include <xen/smp.h> > #include <xen/spinlock.h> > #include <xen/vmap.h> > +#include <xen/warning.h> > #include <asm/cpufeature.h> > #include <asm/cpuerrata.h> > +#include <asm/psci.h> > > /* Override macros from asm/page.h to make them work with mfn_t */ > #undef virt_to_mfn > @@ -141,6 +143,31 @@ install_bp_hardening_vec(const struct arm_cpu_capabilities *entry, > return ret; > } > > +extern char __psci_hyp_bp_inval_start[], __psci_hyp_bp_inval_end[]; > + > +static int enable_psci_bp_hardening(void *data) > +{ > + bool ret = true; > + static bool warned = false; > + > + /* > + * The mitigation is using PSCI version function to invalidate the > + * branch predictor. This function is only available with PSCI 0.2 > + * and later. > + */ > + if ( psci_ver >= PSCI_VERSION(0, 2) ) > + ret = install_bp_hardening_vec(data, __psci_hyp_bp_inval_start, > + __psci_hyp_bp_inval_end); > + else if ( !warned ) > + { > + ASSERT(system_state < SYS_STATE_active); > + warning_add("PSCI 0.2 or later is required for the branch predictor hardening.\n"); > + warned = true; > + } > + > + return !ret; > +} > + > #endif /* CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR */ > > #define MIDR_RANGE(model, min, max) \ > @@ -205,6 +232,28 @@ static const struct arm_cpu_capabilities arm_errata[] = { > (1 << MIDR_VARIANT_SHIFT) | 2), > }, > #endif > +#ifdef CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR > + { > + .capability = ARM_HARDEN_BRANCH_PREDICTOR, > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A57), > + .enable = enable_psci_bp_hardening, > + }, > + { > + .capability = ARM_HARDEN_BRANCH_PREDICTOR, > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A72), > + .enable = enable_psci_bp_hardening, > + }, > + { > + .capability = ARM_HARDEN_BRANCH_PREDICTOR, > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A73), > + .enable = enable_psci_bp_hardening, > + }, > + { > + .capability = ARM_HARDEN_BRANCH_PREDICTOR, > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A75), > + .enable = enable_psci_bp_hardening, > + }, We need to add a basic description in the desc field as it is printed by update_cpu_capabilities. > +#endif > {}, > }; > > -- > 2.11.0 >
Hi Stefano, On 17/01/18 00:42, Stefano Stabellini wrote: > On Tue, 16 Jan 2018, Julien Grall wrote: >> #define MIDR_RANGE(model, min, max) \ >> @@ -205,6 +232,28 @@ static const struct arm_cpu_capabilities arm_errata[] = { >> (1 << MIDR_VARIANT_SHIFT) | 2), >> }, >> #endif >> +#ifdef CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR >> + { >> + .capability = ARM_HARDEN_BRANCH_PREDICTOR, >> + MIDR_ALL_VERSIONS(MIDR_CORTEX_A57), >> + .enable = enable_psci_bp_hardening, >> + }, >> + { >> + .capability = ARM_HARDEN_BRANCH_PREDICTOR, >> + MIDR_ALL_VERSIONS(MIDR_CORTEX_A72), >> + .enable = enable_psci_bp_hardening, >> + }, >> + { >> + .capability = ARM_HARDEN_BRANCH_PREDICTOR, >> + MIDR_ALL_VERSIONS(MIDR_CORTEX_A73), >> + .enable = enable_psci_bp_hardening, >> + }, >> + { >> + .capability = ARM_HARDEN_BRANCH_PREDICTOR, >> + MIDR_ALL_VERSIONS(MIDR_CORTEX_A75), >> + .enable = enable_psci_bp_hardening, >> + }, > > We need to add a basic description in the desc field as it is printed by > update_cpu_capabilities. desc field is not mandatory, and in that case I think the print would be confusing. At the difference of the other errata, we have more check in install_bp_hardening_vec that may result to skip the hardening. The errata here is covering all variant/revision of A75 models for safety reason. Arm has introduced a new field ID_AA64PFR0_EL1.CSV2 to tell whether a branch predictor trained in one context will affect speculative execution in another context. This field is checked in install_bp_hardening_vec so you avoid to harden the vector tables and small performance hit. IHMO, it would be better to have a per-CPU message in install_bp_hardening_vec announcing whether the vector tables has been harden and the kind of hardening. What do you think? Cheers, > > >> +#endif >> {}, >> }; >> >> -- >> 2.11.0 >>
On Wed, 17 Jan 2018, Julien Grall wrote: > Hi Stefano, > > On 17/01/18 00:42, Stefano Stabellini wrote: > > On Tue, 16 Jan 2018, Julien Grall wrote: > > > #define MIDR_RANGE(model, min, max) \ > > > @@ -205,6 +232,28 @@ static const struct arm_cpu_capabilities arm_errata[] > > > = { > > > (1 << MIDR_VARIANT_SHIFT) | 2), > > > }, > > > #endif > > > +#ifdef CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR > > > + { > > > + .capability = ARM_HARDEN_BRANCH_PREDICTOR, > > > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A57), > > > + .enable = enable_psci_bp_hardening, > > > + }, > > > + { > > > + .capability = ARM_HARDEN_BRANCH_PREDICTOR, > > > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A72), > > > + .enable = enable_psci_bp_hardening, > > > + }, > > > + { > > > + .capability = ARM_HARDEN_BRANCH_PREDICTOR, > > > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A73), > > > + .enable = enable_psci_bp_hardening, > > > + }, > > > + { > > > + .capability = ARM_HARDEN_BRANCH_PREDICTOR, > > > + MIDR_ALL_VERSIONS(MIDR_CORTEX_A75), > > > + .enable = enable_psci_bp_hardening, > > > + }, > > > > We need to add a basic description in the desc field as it is printed by > > update_cpu_capabilities. > > desc field is not mandatory, and in that case I think the print would be > confusing. At the difference of the other errata, we have more check in > install_bp_hardening_vec that may result to skip the hardening. > > The errata here is covering all variant/revision of A75 models for safety > reason. Arm has introduced a new field ID_AA64PFR0_EL1.CSV2 to tell whether a > branch predictor trained in one context will affect speculative execution in > another context. This field is checked in install_bp_hardening_vec so you > avoid to harden the vector tables and small performance hit. > > IHMO, it would be better to have a per-CPU message in install_bp_hardening_vec > announcing whether the vector tables has been harden and the kind of > hardening. > > What do you think? I understand what you are saying and I agree with you. Maybe we should change update_cpu_capabilities to print "detected need for workaround: blah" but you don't have to do it in this series.
Hi, On 17/01/18 17:11, Stefano Stabellini wrote: > On Wed, 17 Jan 2018, Julien Grall wrote: >> Hi Stefano, >> >> On 17/01/18 00:42, Stefano Stabellini wrote: >>> On Tue, 16 Jan 2018, Julien Grall wrote: >>>> #define MIDR_RANGE(model, min, max) \ >>>> @@ -205,6 +232,28 @@ static const struct arm_cpu_capabilities arm_errata[] >>>> = { >>>> (1 << MIDR_VARIANT_SHIFT) | 2), >>>> }, >>>> #endif >>>> +#ifdef CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR >>>> + { >>>> + .capability = ARM_HARDEN_BRANCH_PREDICTOR, >>>> + MIDR_ALL_VERSIONS(MIDR_CORTEX_A57), >>>> + .enable = enable_psci_bp_hardening, >>>> + }, >>>> + { >>>> + .capability = ARM_HARDEN_BRANCH_PREDICTOR, >>>> + MIDR_ALL_VERSIONS(MIDR_CORTEX_A72), >>>> + .enable = enable_psci_bp_hardening, >>>> + }, >>>> + { >>>> + .capability = ARM_HARDEN_BRANCH_PREDICTOR, >>>> + MIDR_ALL_VERSIONS(MIDR_CORTEX_A73), >>>> + .enable = enable_psci_bp_hardening, >>>> + }, >>>> + { >>>> + .capability = ARM_HARDEN_BRANCH_PREDICTOR, >>>> + MIDR_ALL_VERSIONS(MIDR_CORTEX_A75), >>>> + .enable = enable_psci_bp_hardening, >>>> + }, >>> >>> We need to add a basic description in the desc field as it is printed by >>> update_cpu_capabilities. >> >> desc field is not mandatory, and in that case I think the print would be >> confusing. At the difference of the other errata, we have more check in >> install_bp_hardening_vec that may result to skip the hardening. >> >> The errata here is covering all variant/revision of A75 models for safety >> reason. Arm has introduced a new field ID_AA64PFR0_EL1.CSV2 to tell whether a >> branch predictor trained in one context will affect speculative execution in >> another context. This field is checked in install_bp_hardening_vec so you >> avoid to harden the vector tables and small performance hit. >> >> IHMO, it would be better to have a per-CPU message in install_bp_hardening_vec >> announcing whether the vector tables has been harden and the kind of >> hardening. >> >> What do you think? > > I understand what you are saying and I agree with you. Maybe we should > change update_cpu_capabilities to print "detected need for workaround: > blah" but you don't have to do it in this series. "need for workaround..." is not entirely true for the branch predictor hardening. It is more a "may need". I still prefer the per-CPU message "CPU%u will %s on guest exit" %u is the CPU number. %s will be the name of the call/instruction to execute. The rationale behind is branch predictor hardening may be different on each CPU (think big.LITTLE) so code executed will be different. This differ from the other errata that will be applied for all CPUs. Cheers,
diff --git a/xen/arch/arm/arm64/bpi.S b/xen/arch/arm/arm64/bpi.S index 6cc2f17529..4b7f1dc21f 100644 --- a/xen/arch/arm/arm64/bpi.S +++ b/xen/arch/arm/arm64/bpi.S @@ -56,6 +56,31 @@ ENTRY(__bp_harden_hyp_vecs_start) .endr ENTRY(__bp_harden_hyp_vecs_end) +ENTRY(__psci_hyp_bp_inval_start) + sub sp, sp, #(8 * 18) + stp x16, x17, [sp, #(16 * 0)] + stp x14, x15, [sp, #(16 * 1)] + stp x12, x13, [sp, #(16 * 2)] + stp x10, x11, [sp, #(16 * 3)] + stp x8, x9, [sp, #(16 * 4)] + stp x6, x7, [sp, #(16 * 5)] + stp x4, x5, [sp, #(16 * 6)] + stp x2, x3, [sp, #(16 * 7)] + stp x0, x1, [sp, #(16 * 8)] + mov x0, #0x84000000 + smc #0 + ldp x16, x17, [sp, #(16 * 0)] + ldp x14, x15, [sp, #(16 * 1)] + ldp x12, x13, [sp, #(16 * 2)] + ldp x10, x11, [sp, #(16 * 3)] + ldp x8, x9, [sp, #(16 * 4)] + ldp x6, x7, [sp, #(16 * 5)] + ldp x4, x5, [sp, #(16 * 6)] + ldp x2, x3, [sp, #(16 * 7)] + ldp x0, x1, [sp, #(16 * 8)] + add sp, sp, #(8 * 18) +ENTRY(__psci_hyp_bp_inval_end) + /* * Local variables: * mode: ASM diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c index 76d98e771d..f1ea7f3c5b 100644 --- a/xen/arch/arm/cpuerrata.c +++ b/xen/arch/arm/cpuerrata.c @@ -4,8 +4,10 @@ #include <xen/smp.h> #include <xen/spinlock.h> #include <xen/vmap.h> +#include <xen/warning.h> #include <asm/cpufeature.h> #include <asm/cpuerrata.h> +#include <asm/psci.h> /* Override macros from asm/page.h to make them work with mfn_t */ #undef virt_to_mfn @@ -141,6 +143,31 @@ install_bp_hardening_vec(const struct arm_cpu_capabilities *entry, return ret; } +extern char __psci_hyp_bp_inval_start[], __psci_hyp_bp_inval_end[]; + +static int enable_psci_bp_hardening(void *data) +{ + bool ret = true; + static bool warned = false; + + /* + * The mitigation is using PSCI version function to invalidate the + * branch predictor. This function is only available with PSCI 0.2 + * and later. + */ + if ( psci_ver >= PSCI_VERSION(0, 2) ) + ret = install_bp_hardening_vec(data, __psci_hyp_bp_inval_start, + __psci_hyp_bp_inval_end); + else if ( !warned ) + { + ASSERT(system_state < SYS_STATE_active); + warning_add("PSCI 0.2 or later is required for the branch predictor hardening.\n"); + warned = true; + } + + return !ret; +} + #endif /* CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR */ #define MIDR_RANGE(model, min, max) \ @@ -205,6 +232,28 @@ static const struct arm_cpu_capabilities arm_errata[] = { (1 << MIDR_VARIANT_SHIFT) | 2), }, #endif +#ifdef CONFIG_ARM64_HARDEN_BRANCH_PREDICTOR + { + .capability = ARM_HARDEN_BRANCH_PREDICTOR, + MIDR_ALL_VERSIONS(MIDR_CORTEX_A57), + .enable = enable_psci_bp_hardening, + }, + { + .capability = ARM_HARDEN_BRANCH_PREDICTOR, + MIDR_ALL_VERSIONS(MIDR_CORTEX_A72), + .enable = enable_psci_bp_hardening, + }, + { + .capability = ARM_HARDEN_BRANCH_PREDICTOR, + MIDR_ALL_VERSIONS(MIDR_CORTEX_A73), + .enable = enable_psci_bp_hardening, + }, + { + .capability = ARM_HARDEN_BRANCH_PREDICTOR, + MIDR_ALL_VERSIONS(MIDR_CORTEX_A75), + .enable = enable_psci_bp_hardening, + }, +#endif {}, };