| Message ID | 20181105143617.120602-5-marc.zyngier@arm.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Workaround for Cortex-A76 erratum 1165522 | expand |
Hi Marc, On 05/11/2018 14:36, Marc Zyngier wrote: > Early versions of Cortex-A76 can end-up with corrupt TLBs if they > speculate an AT instruction in during a guest switch while the (in during?) > S1/S2 system registers are in an inconsistent state. > > Work around it by: > - Mandating VHE > - Make sure that S1 and S2 system registers are consistent before > clearing HCR_EL2.TGE, which allows AT to target the EL1 translation > regime > > These two things together ensure that we cannot hit this erratum. > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c > index 51d5d966d9e5..322109183853 100644 > --- a/arch/arm64/kvm/hyp/switch.c > +++ b/arch/arm64/kvm/hyp/switch.c > @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void) > { > extern char vectors[]; /* kernel exception vectors */ > write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2); > + > + /* > + * ARM erratum 1165522 requires the actual execution of the > + * above before we can switch to the host translation regime. > + */ > + asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522)); > + Host regime too ... does __tlb_switch_to_host_vhe() need the same treatment? It writes vttbr_el2 and hcr_el2 back to back. Thanks, James
On Mon, Nov 05, 2018 at 02:36:16PM +0000, Marc Zyngier wrote: > Early versions of Cortex-A76 can end-up with corrupt TLBs if they > speculate an AT instruction in during a guest switch while the > S1/S2 system registers are in an inconsistent state. > > Work around it by: > - Mandating VHE > - Make sure that S1 and S2 system registers are consistent before > clearing HCR_EL2.TGE, which allows AT to target the EL1 translation > regime > > These two things together ensure that we cannot hit this erratum. > > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > --- > Documentation/arm64/silicon-errata.txt | 1 + > arch/arm64/Kconfig | 12 ++++++++++++ > arch/arm64/include/asm/cpucaps.h | 3 ++- > arch/arm64/include/asm/kvm_host.h | 3 +++ > arch/arm64/include/asm/kvm_hyp.h | 6 ++++++ > arch/arm64/kernel/cpu_errata.c | 8 ++++++++ > arch/arm64/kvm/hyp/switch.c | 14 ++++++++++++++ > 7 files changed, 46 insertions(+), 1 deletion(-) > > diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt > index 76ccded8b74c..04f0bc4690c6 100644 > --- a/Documentation/arm64/silicon-errata.txt > +++ b/Documentation/arm64/silicon-errata.txt > @@ -57,6 +57,7 @@ stable kernels. > | ARM | Cortex-A73 | #858921 | ARM64_ERRATUM_858921 | > | ARM | Cortex-A55 | #1024718 | ARM64_ERRATUM_1024718 | > | ARM | Cortex-A76 | #1188873 | ARM64_ERRATUM_1188873 | > +| ARM | Cortex-A76 | #1165522 | ARM64_ERRATUM_1165522 | > | ARM | MMU-500 | #841119,#826419 | N/A | > | | | | | > | Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 | > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 787d7850e064..a68bc6cc2167 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873 > > If unsure, say Y. > > +config ARM64_ERRATUM_1165522 > + bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation" > + default y > + help > + This option adds work arounds for ARM Cortex-A76 erratum 1165522 > + > + Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with > + corrupted TLBs by speculating an AT instruction during a guest > + context switch. > + > + If unsure, say Y. > + > config CAVIUM_ERRATUM_22375 > bool "Cavium erratum 22375, 24313" > default y > diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h > index 6e2d254c09eb..62d8cd15fdf2 100644 > --- a/arch/arm64/include/asm/cpucaps.h > +++ b/arch/arm64/include/asm/cpucaps.h > @@ -54,7 +54,8 @@ > #define ARM64_HAS_CRC32 33 > #define ARM64_SSBS 34 > #define ARM64_WORKAROUND_1188873 35 > +#define ARM64_WORKAROUND_1165522 36 > > -#define ARM64_NCAPS 36 > +#define ARM64_NCAPS 37 > > #endif /* __ASM_CPUCAPS_H */ > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 7d6e974d024a..8f486026ac87 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -435,6 +435,9 @@ static inline bool kvm_arch_sve_requires_vhe(void) > static inline bool kvm_arch_impl_requires_vhe(void) > { > /* Some implementations have defects that confine them to VHE */ > + if (cpus_have_cap(ARM64_WORKAROUND_1165522)) > + return true; > + > return false; > } > > diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h > index 23aca66767f9..9681360d0959 100644 > --- a/arch/arm64/include/asm/kvm_hyp.h > +++ b/arch/arm64/include/asm/kvm_hyp.h > @@ -163,6 +163,12 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm) > { > write_sysreg(kvm->arch.vtcr, vtcr_el2); > write_sysreg(kvm->arch.vttbr, vttbr_el2); > + > + /* > + * ARM erratum 1165522 requires the actual execution of the > + * above before we can switch to the guest translation regime. > + */ Is it about a guest translation 'regime' or should this just say before we can enable stage 2 translation? > + asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522)); > } > > #endif /* __ARM64_KVM_HYP_H__ */ > diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c > index a509e35132d2..476e738e6c46 100644 > --- a/arch/arm64/kernel/cpu_errata.c > +++ b/arch/arm64/kernel/cpu_errata.c > @@ -739,6 +739,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = { > .capability = ARM64_WORKAROUND_1188873, > ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0), > }, > +#endif > +#ifdef CONFIG_ARM64_ERRATUM_1165522 > + { > + /* Cortex-A76 r0p0 to r2p0 */ > + .desc = "ARM erratum 1165522", > + .capability = ARM64_WORKAROUND_1165522, > + ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0), > + }, > #endif > { > } > diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c > index 51d5d966d9e5..322109183853 100644 > --- a/arch/arm64/kvm/hyp/switch.c > +++ b/arch/arm64/kvm/hyp/switch.c > @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void) > { > extern char vectors[]; /* kernel exception vectors */ > write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2); > + > + /* > + * ARM erratum 1165522 requires the actual execution of the > + * above before we can switch to the host translation regime. > + */ same here, is it not rather about disabling stage 2 translation than about the host (EL2 and EL0 stage 1) translation regimes? > + asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522)); > + > write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1); > write_sysreg(vectors, vbar_el1); > } > @@ -499,6 +506,13 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu) > > sysreg_save_host_state_vhe(host_ctxt); > > + /* > + * ARM erratum 1165522 requires us to have all the translation > + * context in place before we clear HCR_TGE. All the offending > + * guest sysregs are loaded in kvm_vcpu_load_sysregs, and > + * __activate_vm has the stage-2 configuration. Once this is > + * done, __activate_trap clears HCR_TGE (among other things). > + */ I'm not this comment is needed or is helpful here. For example, I don't understand what you mean with the offending guest sysregs and how that relates to the problem of configuring stage 2 before clearing TGE. Is this about gettting the stage 1 configuration in place first? If so, could I suggest a reword along the lines of: /* * ARM erratum 1165522 requires us to configure both stage 1 and * stage 2 translation for the guest context before we clear * HCR_EL2.TGE. * * We have already configured the guest's stage 1 translation in * kvm_vcpu_load_sysregs above. We must now call __activate_vm * before __activate_traps, because __activate_vm configures * stage 2 translation, and __activate_traps clear HCR_EL2.TGE * (among other things). */ > __activate_vm(vcpu->kvm); > __activate_traps(vcpu); > > -- > 2.19.1 > Thanks, Christoffer
On 05/11/18 18:34, James Morse wrote: > Hi Marc, > > On 05/11/2018 14:36, Marc Zyngier wrote: >> Early versions of Cortex-A76 can end-up with corrupt TLBs if they >> speculate an AT instruction in during a guest switch while the > > (in during?) > >> S1/S2 system registers are in an inconsistent state. >> >> Work around it by: >> - Mandating VHE >> - Make sure that S1 and S2 system registers are consistent before >> clearing HCR_EL2.TGE, which allows AT to target the EL1 translation >> regime >> >> These two things together ensure that we cannot hit this erratum. > > >> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c >> index 51d5d966d9e5..322109183853 100644 >> --- a/arch/arm64/kvm/hyp/switch.c >> +++ b/arch/arm64/kvm/hyp/switch.c >> @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void) >> { >> extern char vectors[]; /* kernel exception vectors */ >> write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2); >> + >> + /* >> + * ARM erratum 1165522 requires the actual execution of the >> + * above before we can switch to the host translation regime. >> + */ >> + asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522)); >> + > > Host regime too ... does __tlb_switch_to_host_vhe() need the same > treatment? It writes vttbr_el2 and hcr_el2 back to back. It turns out that our VHE TLB invalidation are a tiny bit broken, and that's before we work around this very erratum. You're perfectly right that we're mitting an ISB in __tlb_switch_to_host_vhe(). We also have the problem that we can perfectly take an interrupt here, and maybe schedule another process from there (very unlikely, but I couldn't fully convince myself that it couldn't happen). What I'm planning to do is to make these TLB invalidation sequence atomic by disabling interrupts. Yes, this is quite a hammer, but that' no different from !VHE, and that's a very rare event anyway. Thanks, M.
On 06/11/18 08:15, Christoffer Dall wrote: > On Mon, Nov 05, 2018 at 02:36:16PM +0000, Marc Zyngier wrote: >> Early versions of Cortex-A76 can end-up with corrupt TLBs if they >> speculate an AT instruction in during a guest switch while the >> S1/S2 system registers are in an inconsistent state. >> >> Work around it by: >> - Mandating VHE >> - Make sure that S1 and S2 system registers are consistent before >> clearing HCR_EL2.TGE, which allows AT to target the EL1 translation >> regime >> >> These two things together ensure that we cannot hit this erratum. >> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >> --- >> Documentation/arm64/silicon-errata.txt | 1 + >> arch/arm64/Kconfig | 12 ++++++++++++ >> arch/arm64/include/asm/cpucaps.h | 3 ++- >> arch/arm64/include/asm/kvm_host.h | 3 +++ >> arch/arm64/include/asm/kvm_hyp.h | 6 ++++++ >> arch/arm64/kernel/cpu_errata.c | 8 ++++++++ >> arch/arm64/kvm/hyp/switch.c | 14 ++++++++++++++ >> 7 files changed, 46 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt >> index 76ccded8b74c..04f0bc4690c6 100644 >> --- a/Documentation/arm64/silicon-errata.txt >> +++ b/Documentation/arm64/silicon-errata.txt >> @@ -57,6 +57,7 @@ stable kernels. >> | ARM | Cortex-A73 | #858921 | ARM64_ERRATUM_858921 | >> | ARM | Cortex-A55 | #1024718 | ARM64_ERRATUM_1024718 | >> | ARM | Cortex-A76 | #1188873 | ARM64_ERRATUM_1188873 | >> +| ARM | Cortex-A76 | #1165522 | ARM64_ERRATUM_1165522 | >> | ARM | MMU-500 | #841119,#826419 | N/A | >> | | | | | >> | Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 | >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >> index 787d7850e064..a68bc6cc2167 100644 >> --- a/arch/arm64/Kconfig >> +++ b/arch/arm64/Kconfig >> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873 >> >> If unsure, say Y. >> >> +config ARM64_ERRATUM_1165522 >> + bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation" >> + default y >> + help >> + This option adds work arounds for ARM Cortex-A76 erratum 1165522 >> + >> + Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with >> + corrupted TLBs by speculating an AT instruction during a guest >> + context switch. >> + >> + If unsure, say Y. >> + >> config CAVIUM_ERRATUM_22375 >> bool "Cavium erratum 22375, 24313" >> default y >> diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h >> index 6e2d254c09eb..62d8cd15fdf2 100644 >> --- a/arch/arm64/include/asm/cpucaps.h >> +++ b/arch/arm64/include/asm/cpucaps.h >> @@ -54,7 +54,8 @@ >> #define ARM64_HAS_CRC32 33 >> #define ARM64_SSBS 34 >> #define ARM64_WORKAROUND_1188873 35 >> +#define ARM64_WORKAROUND_1165522 36 >> >> -#define ARM64_NCAPS 36 >> +#define ARM64_NCAPS 37 >> >> #endif /* __ASM_CPUCAPS_H */ >> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h >> index 7d6e974d024a..8f486026ac87 100644 >> --- a/arch/arm64/include/asm/kvm_host.h >> +++ b/arch/arm64/include/asm/kvm_host.h >> @@ -435,6 +435,9 @@ static inline bool kvm_arch_sve_requires_vhe(void) >> static inline bool kvm_arch_impl_requires_vhe(void) >> { >> /* Some implementations have defects that confine them to VHE */ >> + if (cpus_have_cap(ARM64_WORKAROUND_1165522)) >> + return true; >> + >> return false; >> } >> >> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h >> index 23aca66767f9..9681360d0959 100644 >> --- a/arch/arm64/include/asm/kvm_hyp.h >> +++ b/arch/arm64/include/asm/kvm_hyp.h >> @@ -163,6 +163,12 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm) >> { >> write_sysreg(kvm->arch.vtcr, vtcr_el2); >> write_sysreg(kvm->arch.vttbr, vttbr_el2); >> + >> + /* >> + * ARM erratum 1165522 requires the actual execution of the >> + * above before we can switch to the guest translation regime. >> + */ > > Is it about a guest translation 'regime' or should this just say before > we can enable stage 2 translation? No, this isn't strictly about enabling stage-2 translation. This is about making sure that anything that impacts the guest translations is actually executed. I wonder if it would be clearer to move this outside of __load_guest_stage2 and make it explicit in the callers of this helper... Thoughts? > >> + asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522)); >> } >> >> #endif /* __ARM64_KVM_HYP_H__ */ >> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c >> index a509e35132d2..476e738e6c46 100644 >> --- a/arch/arm64/kernel/cpu_errata.c >> +++ b/arch/arm64/kernel/cpu_errata.c >> @@ -739,6 +739,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = { >> .capability = ARM64_WORKAROUND_1188873, >> ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0), >> }, >> +#endif >> +#ifdef CONFIG_ARM64_ERRATUM_1165522 >> + { >> + /* Cortex-A76 r0p0 to r2p0 */ >> + .desc = "ARM erratum 1165522", >> + .capability = ARM64_WORKAROUND_1165522, >> + ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0), >> + }, >> #endif >> { >> } >> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c >> index 51d5d966d9e5..322109183853 100644 >> --- a/arch/arm64/kvm/hyp/switch.c >> +++ b/arch/arm64/kvm/hyp/switch.c >> @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void) >> { >> extern char vectors[]; /* kernel exception vectors */ >> write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2); >> + >> + /* >> + * ARM erratum 1165522 requires the actual execution of the >> + * above before we can switch to the host translation regime. >> + */ > > same here, is it not rather about disabling stage 2 translation than > about the host (EL2 and EL0 stage 1) translation regimes? > >> + asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522)); >> + >> write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1); >> write_sysreg(vectors, vbar_el1); >> } >> @@ -499,6 +506,13 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu) >> >> sysreg_save_host_state_vhe(host_ctxt); >> >> + /* >> + * ARM erratum 1165522 requires us to have all the translation >> + * context in place before we clear HCR_TGE. All the offending >> + * guest sysregs are loaded in kvm_vcpu_load_sysregs, and >> + * __activate_vm has the stage-2 configuration. Once this is >> + * done, __activate_trap clears HCR_TGE (among other things). >> + */ > > I'm not this comment is needed or is helpful here. For example, I don't > understand what you mean with the offending guest sysregs and how that TTBR*, TCR, SCTLR... Anything that deals with stage-1 translations. > relates to the problem of configuring stage 2 before clearing TGE. Is > this about gettting the stage 1 configuration in place first? Not only stage-1. Stage-2 is involved as well, as the CPU could otherwise end-up with the wrong translations (bypassing stage-2 altogether). > > If so, could I suggest a reword along the lines of: > > /* > * ARM erratum 1165522 requires us to configure both stage 1 and > * stage 2 translation for the guest context before we clear > * HCR_EL2.TGE. > * > * We have already configured the guest's stage 1 translation in > * kvm_vcpu_load_sysregs above. We must now call __activate_vm > * before __activate_traps, because __activate_vm configures > * stage 2 translation, and __activate_traps clear HCR_EL2.TGE > * (among other things). > */ Works for me (and shows that contrary to what you wrote above, you have perfectly understood the problem)!. Thanks, M.
On Thu, Nov 08, 2018 at 06:05:55PM +0000, Marc Zyngier wrote: > On 06/11/18 08:15, Christoffer Dall wrote: > > On Mon, Nov 05, 2018 at 02:36:16PM +0000, Marc Zyngier wrote: > >> Early versions of Cortex-A76 can end-up with corrupt TLBs if they > >> speculate an AT instruction in during a guest switch while the > >> S1/S2 system registers are in an inconsistent state. > >> > >> Work around it by: > >> - Mandating VHE > >> - Make sure that S1 and S2 system registers are consistent before > >> clearing HCR_EL2.TGE, which allows AT to target the EL1 translation > >> regime > >> > >> These two things together ensure that we cannot hit this erratum. > >> > >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > >> --- > >> Documentation/arm64/silicon-errata.txt | 1 + > >> arch/arm64/Kconfig | 12 ++++++++++++ > >> arch/arm64/include/asm/cpucaps.h | 3 ++- > >> arch/arm64/include/asm/kvm_host.h | 3 +++ > >> arch/arm64/include/asm/kvm_hyp.h | 6 ++++++ > >> arch/arm64/kernel/cpu_errata.c | 8 ++++++++ > >> arch/arm64/kvm/hyp/switch.c | 14 ++++++++++++++ > >> 7 files changed, 46 insertions(+), 1 deletion(-) > >> > >> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt > >> index 76ccded8b74c..04f0bc4690c6 100644 > >> --- a/Documentation/arm64/silicon-errata.txt > >> +++ b/Documentation/arm64/silicon-errata.txt > >> @@ -57,6 +57,7 @@ stable kernels. > >> | ARM | Cortex-A73 | #858921 | ARM64_ERRATUM_858921 | > >> | ARM | Cortex-A55 | #1024718 | ARM64_ERRATUM_1024718 | > >> | ARM | Cortex-A76 | #1188873 | ARM64_ERRATUM_1188873 | > >> +| ARM | Cortex-A76 | #1165522 | ARM64_ERRATUM_1165522 | > >> | ARM | MMU-500 | #841119,#826419 | N/A | > >> | | | | | > >> | Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 | > >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > >> index 787d7850e064..a68bc6cc2167 100644 > >> --- a/arch/arm64/Kconfig > >> +++ b/arch/arm64/Kconfig > >> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873 > >> > >> If unsure, say Y. > >> > >> +config ARM64_ERRATUM_1165522 > >> + bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation" > >> + default y > >> + help > >> + This option adds work arounds for ARM Cortex-A76 erratum 1165522 > >> + > >> + Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with > >> + corrupted TLBs by speculating an AT instruction during a guest > >> + context switch. > >> + > >> + If unsure, say Y. > >> + > >> config CAVIUM_ERRATUM_22375 > >> bool "Cavium erratum 22375, 24313" > >> default y > >> diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h > >> index 6e2d254c09eb..62d8cd15fdf2 100644 > >> --- a/arch/arm64/include/asm/cpucaps.h > >> +++ b/arch/arm64/include/asm/cpucaps.h > >> @@ -54,7 +54,8 @@ > >> #define ARM64_HAS_CRC32 33 > >> #define ARM64_SSBS 34 > >> #define ARM64_WORKAROUND_1188873 35 > >> +#define ARM64_WORKAROUND_1165522 36 > >> > >> -#define ARM64_NCAPS 36 > >> +#define ARM64_NCAPS 37 > >> > >> #endif /* __ASM_CPUCAPS_H */ > >> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > >> index 7d6e974d024a..8f486026ac87 100644 > >> --- a/arch/arm64/include/asm/kvm_host.h > >> +++ b/arch/arm64/include/asm/kvm_host.h > >> @@ -435,6 +435,9 @@ static inline bool kvm_arch_sve_requires_vhe(void) > >> static inline bool kvm_arch_impl_requires_vhe(void) > >> { > >> /* Some implementations have defects that confine them to VHE */ > >> + if (cpus_have_cap(ARM64_WORKAROUND_1165522)) > >> + return true; > >> + > >> return false; > >> } > >> > >> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h > >> index 23aca66767f9..9681360d0959 100644 > >> --- a/arch/arm64/include/asm/kvm_hyp.h > >> +++ b/arch/arm64/include/asm/kvm_hyp.h > >> @@ -163,6 +163,12 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm) > >> { > >> write_sysreg(kvm->arch.vtcr, vtcr_el2); > >> write_sysreg(kvm->arch.vttbr, vttbr_el2); > >> + > >> + /* > >> + * ARM erratum 1165522 requires the actual execution of the > >> + * above before we can switch to the guest translation regime. > >> + */ > > > > Is it about a guest translation 'regime' or should this just say before > > we can enable stage 2 translation? > > No, this isn't strictly about enabling stage-2 translation. This is > about making sure that anything that impacts the guest translations is > actually executed. > > I wonder if it would be clearer to move this outside of > __load_guest_stage2 and make it explicit in the callers of this helper... I think it makes sense to have this here to explain the alternative. But it's the 'switch to guest translation regime' thing that bothers me a bit. Is that an architectural concept (I thought we only had EL1 and EL2 translation regimes in stage 1, and then stage 2 translations). So When you say 'guest translation regime' I'm just not entirely sure what that means, unless I'm missing something. > > > >> + asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522)); > >> } > >> > >> #endif /* __ARM64_KVM_HYP_H__ */ > >> diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c > >> index a509e35132d2..476e738e6c46 100644 > >> --- a/arch/arm64/kernel/cpu_errata.c > >> +++ b/arch/arm64/kernel/cpu_errata.c > >> @@ -739,6 +739,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = { > >> .capability = ARM64_WORKAROUND_1188873, > >> ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0), > >> }, > >> +#endif > >> +#ifdef CONFIG_ARM64_ERRATUM_1165522 > >> + { > >> + /* Cortex-A76 r0p0 to r2p0 */ > >> + .desc = "ARM erratum 1165522", > >> + .capability = ARM64_WORKAROUND_1165522, > >> + ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0), > >> + }, > >> #endif > >> { > >> } > >> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c > >> index 51d5d966d9e5..322109183853 100644 > >> --- a/arch/arm64/kvm/hyp/switch.c > >> +++ b/arch/arm64/kvm/hyp/switch.c > >> @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void) > >> { > >> extern char vectors[]; /* kernel exception vectors */ > >> write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2); > >> + > >> + /* > >> + * ARM erratum 1165522 requires the actual execution of the > >> + * above before we can switch to the host translation regime. > >> + */ > > > > same here, is it not rather about disabling stage 2 translation than > > about the host (EL2 and EL0 stage 1) translation regimes? > > > >> + asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522)); > >> + > >> write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1); > >> write_sysreg(vectors, vbar_el1); > >> } > >> @@ -499,6 +506,13 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu) > >> > >> sysreg_save_host_state_vhe(host_ctxt); > >> > >> + /* > >> + * ARM erratum 1165522 requires us to have all the translation > >> + * context in place before we clear HCR_TGE. All the offending > >> + * guest sysregs are loaded in kvm_vcpu_load_sysregs, and > >> + * __activate_vm has the stage-2 configuration. Once this is > >> + * done, __activate_trap clears HCR_TGE (among other things). > >> + */ > > > > I'm not this comment is needed or is helpful here. For example, I don't > > understand what you mean with the offending guest sysregs and how that > > TTBR*, TCR, SCTLR... Anything that deals with stage-1 translations. > > > relates to the problem of configuring stage 2 before clearing TGE. Is > > this about gettting the stage 1 configuration in place first? > > Not only stage-1. Stage-2 is involved as well, as the CPU could > otherwise end-up with the wrong translations (bypassing stage-2 altogether). > > > > > If so, could I suggest a reword along the lines of: > > > > /* > > * ARM erratum 1165522 requires us to configure both stage 1 and > > * stage 2 translation for the guest context before we clear > > * HCR_EL2.TGE. > > * > > * We have already configured the guest's stage 1 translation in > > * kvm_vcpu_load_sysregs above. We must now call __activate_vm > > * before __activate_traps, because __activate_vm configures > > * stage 2 translation, and __activate_traps clear HCR_EL2.TGE > > * (among other things). > > */ > > Works for me (and shows that contrary to what you wrote above, you have > perfectly understood the problem)!. > I may have actually understood the problem by writing up that piece of commentary. Great! Thanks, Christoffer
Hi Marc, On 08/11/2018 17:18, Marc Zyngier wrote: > On 05/11/18 18:34, James Morse wrote: >> On 05/11/2018 14:36, Marc Zyngier wrote: >>> Early versions of Cortex-A76 can end-up with corrupt TLBs if they >>> speculate an AT instruction in during a guest switch while the >> >> (in during?) >> >>> S1/S2 system registers are in an inconsistent state. >>> >>> Work around it by: >>> - Mandating VHE >>> - Make sure that S1 and S2 system registers are consistent before >>> clearing HCR_EL2.TGE, which allows AT to target the EL1 translation >>> regime >>> >>> These two things together ensure that we cannot hit this erratum. >> >> >>> diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c >>> index 51d5d966d9e5..322109183853 100644 >>> --- a/arch/arm64/kvm/hyp/switch.c >>> +++ b/arch/arm64/kvm/hyp/switch.c >>> @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void) >>> { >>> extern char vectors[]; /* kernel exception vectors */ >>> write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2); >>> + >>> + /* >>> + * ARM erratum 1165522 requires the actual execution of the >>> + * above before we can switch to the host translation regime. >>> + */ >>> + asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522)); >>> + >> >> Host regime too ... does __tlb_switch_to_host_vhe() need the same >> treatment? It writes vttbr_el2 and hcr_el2 back to back. > > It turns out that our VHE TLB invalidation are a tiny bit broken, and > that's before we work around this very erratum. > > You're perfectly right that we're mitting an ISB in > __tlb_switch_to_host_vhe(). I thought that would only be needed for this erratum workaround, in the regular case don't we rely on the isb hiding in __vhe_hyp_call()? > We also have the problem that we can > perfectly take an interrupt here, and maybe schedule another process > from there (very unlikely, but I couldn't fully convince myself that it > couldn't happen). > What I'm planning to do is to make these TLB invalidation sequence > atomic by disabling interrupts. Yes, this is quite a hammer, but that' > no different from !VHE, and that's a very rare event anyway. Anything running on the back of an IRQ should not be allowed to see HCR_EL2.TGE being clear. I think this is the right thing to do. One example is TGE changes PANs behaviour, which could become inexplicably-trappy if we're also using the LDRT instructions in the uaccess helpers from an IRQ. (who would do such a thing? perf). Thanks, James
Hi Marc, On 05/11/2018 14:36, Marc Zyngier wrote: > Early versions of Cortex-A76 can end-up with corrupt TLBs if they > speculate an AT instruction in during a guest switch while the > S1/S2 system registers are in an inconsistent state. > > Work around it by: > - Mandating VHE > - Make sure that S1 and S2 system registers are consistent before > clearing HCR_EL2.TGE, which allows AT to target the EL1 translation > regime > > These two things together ensure that we cannot hit this erratum. > > Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > --- > Documentation/arm64/silicon-errata.txt | 1 + > arch/arm64/Kconfig | 12 ++++++++++++ > arch/arm64/include/asm/cpucaps.h | 3 ++- > arch/arm64/include/asm/kvm_host.h | 3 +++ > arch/arm64/include/asm/kvm_hyp.h | 6 ++++++ > arch/arm64/kernel/cpu_errata.c | 8 ++++++++ > arch/arm64/kvm/hyp/switch.c | 14 ++++++++++++++ > 7 files changed, 46 insertions(+), 1 deletion(-) > > diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt > index 76ccded8b74c..04f0bc4690c6 100644 > --- a/Documentation/arm64/silicon-errata.txt > +++ b/Documentation/arm64/silicon-errata.txt > @@ -57,6 +57,7 @@ stable kernels. > | ARM | Cortex-A73 | #858921 | ARM64_ERRATUM_858921 | > | ARM | Cortex-A55 | #1024718 | ARM64_ERRATUM_1024718 | > | ARM | Cortex-A76 | #1188873 | ARM64_ERRATUM_1188873 | > +| ARM | Cortex-A76 | #1165522 | ARM64_ERRATUM_1165522 | > | ARM | MMU-500 | #841119,#826419 | N/A | > | | | | | > | Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 | > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 787d7850e064..a68bc6cc2167 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873 > > If unsure, say Y. > > +config ARM64_ERRATUM_1165522 > + bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation" > + default y > + help > + This option adds work arounds for ARM Cortex-A76 erratum 1165522 > + > + Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with > + corrupted TLBs by speculating an AT instruction during a guest > + context switch. > + > + If unsure, say Y. Most of the code in the patch is not guarded by #ifdef ARM64_*. So is there any benefits to add a Kconfig for this option? Cheers,
On 21/11/2018 12:23, Julien Grall wrote: > Hi Marc, > > On 05/11/2018 14:36, Marc Zyngier wrote: >> Early versions of Cortex-A76 can end-up with corrupt TLBs if they >> speculate an AT instruction in during a guest switch while the >> S1/S2 system registers are in an inconsistent state. >> >> Work around it by: >> - Mandating VHE >> - Make sure that S1 and S2 system registers are consistent before >> clearing HCR_EL2.TGE, which allows AT to target the EL1 translation >> regime >> >> These two things together ensure that we cannot hit this erratum. >> >> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >> --- >> Documentation/arm64/silicon-errata.txt | 1 + >> arch/arm64/Kconfig | 12 ++++++++++++ >> arch/arm64/include/asm/cpucaps.h | 3 ++- >> arch/arm64/include/asm/kvm_host.h | 3 +++ >> arch/arm64/include/asm/kvm_hyp.h | 6 ++++++ >> arch/arm64/kernel/cpu_errata.c | 8 ++++++++ >> arch/arm64/kvm/hyp/switch.c | 14 ++++++++++++++ >> 7 files changed, 46 insertions(+), 1 deletion(-) >> >> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt >> index 76ccded8b74c..04f0bc4690c6 100644 >> --- a/Documentation/arm64/silicon-errata.txt >> +++ b/Documentation/arm64/silicon-errata.txt >> @@ -57,6 +57,7 @@ stable kernels. >> | ARM | Cortex-A73 | #858921 | ARM64_ERRATUM_858921 | >> | ARM | Cortex-A55 | #1024718 | ARM64_ERRATUM_1024718 | >> | ARM | Cortex-A76 | #1188873 | ARM64_ERRATUM_1188873 | >> +| ARM | Cortex-A76 | #1165522 | ARM64_ERRATUM_1165522 | >> | ARM | MMU-500 | #841119,#826419 | N/A | >> | | | | | >> | Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 | >> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >> index 787d7850e064..a68bc6cc2167 100644 >> --- a/arch/arm64/Kconfig >> +++ b/arch/arm64/Kconfig >> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873 >> >> If unsure, say Y. >> >> +config ARM64_ERRATUM_1165522 >> + bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation" >> + default y >> + help >> + This option adds work arounds for ARM Cortex-A76 erratum 1165522 >> + >> + Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with >> + corrupted TLBs by speculating an AT instruction during a guest >> + context switch. >> + >> + If unsure, say Y. > > Most of the code in the patch is not guarded by #ifdef ARM64_*. So is there any > benefits to add a Kconfig for this option? The detection code is guarded by this config option, which is the important thing. In general, we try to compile everything, all the time, unless this is too big to be the case. It drastically simplify the maintenance. See the VHE code for example, which is always compiled in, and is only gated by the detection code that gets compiled out if the option isn't selected. Thanks, M.
On 08/11/2018 20:10, Christoffer Dall wrote: > On Thu, Nov 08, 2018 at 06:05:55PM +0000, Marc Zyngier wrote: >> On 06/11/18 08:15, Christoffer Dall wrote: >>> On Mon, Nov 05, 2018 at 02:36:16PM +0000, Marc Zyngier wrote: >>>> Early versions of Cortex-A76 can end-up with corrupt TLBs if they >>>> speculate an AT instruction in during a guest switch while the >>>> S1/S2 system registers are in an inconsistent state. >>>> >>>> Work around it by: >>>> - Mandating VHE >>>> - Make sure that S1 and S2 system registers are consistent before >>>> clearing HCR_EL2.TGE, which allows AT to target the EL1 translation >>>> regime >>>> >>>> These two things together ensure that we cannot hit this erratum. >>>> >>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> >>>> --- >>>> Documentation/arm64/silicon-errata.txt | 1 + >>>> arch/arm64/Kconfig | 12 ++++++++++++ >>>> arch/arm64/include/asm/cpucaps.h | 3 ++- >>>> arch/arm64/include/asm/kvm_host.h | 3 +++ >>>> arch/arm64/include/asm/kvm_hyp.h | 6 ++++++ >>>> arch/arm64/kernel/cpu_errata.c | 8 ++++++++ >>>> arch/arm64/kvm/hyp/switch.c | 14 ++++++++++++++ >>>> 7 files changed, 46 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt >>>> index 76ccded8b74c..04f0bc4690c6 100644 >>>> --- a/Documentation/arm64/silicon-errata.txt >>>> +++ b/Documentation/arm64/silicon-errata.txt >>>> @@ -57,6 +57,7 @@ stable kernels. >>>> | ARM | Cortex-A73 | #858921 | ARM64_ERRATUM_858921 | >>>> | ARM | Cortex-A55 | #1024718 | ARM64_ERRATUM_1024718 | >>>> | ARM | Cortex-A76 | #1188873 | ARM64_ERRATUM_1188873 | >>>> +| ARM | Cortex-A76 | #1165522 | ARM64_ERRATUM_1165522 | >>>> | ARM | MMU-500 | #841119,#826419 | N/A | >>>> | | | | | >>>> | Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 | >>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig >>>> index 787d7850e064..a68bc6cc2167 100644 >>>> --- a/arch/arm64/Kconfig >>>> +++ b/arch/arm64/Kconfig >>>> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873 >>>> >>>> If unsure, say Y. >>>> >>>> +config ARM64_ERRATUM_1165522 >>>> + bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation" >>>> + default y >>>> + help >>>> + This option adds work arounds for ARM Cortex-A76 erratum 1165522 >>>> + >>>> + Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with >>>> + corrupted TLBs by speculating an AT instruction during a guest >>>> + context switch. >>>> + >>>> + If unsure, say Y. >>>> + >>>> config CAVIUM_ERRATUM_22375 >>>> bool "Cavium erratum 22375, 24313" >>>> default y >>>> diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h >>>> index 6e2d254c09eb..62d8cd15fdf2 100644 >>>> --- a/arch/arm64/include/asm/cpucaps.h >>>> +++ b/arch/arm64/include/asm/cpucaps.h >>>> @@ -54,7 +54,8 @@ >>>> #define ARM64_HAS_CRC32 33 >>>> #define ARM64_SSBS 34 >>>> #define ARM64_WORKAROUND_1188873 35 >>>> +#define ARM64_WORKAROUND_1165522 36 >>>> >>>> -#define ARM64_NCAPS 36 >>>> +#define ARM64_NCAPS 37 >>>> >>>> #endif /* __ASM_CPUCAPS_H */ >>>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h >>>> index 7d6e974d024a..8f486026ac87 100644 >>>> --- a/arch/arm64/include/asm/kvm_host.h >>>> +++ b/arch/arm64/include/asm/kvm_host.h >>>> @@ -435,6 +435,9 @@ static inline bool kvm_arch_sve_requires_vhe(void) >>>> static inline bool kvm_arch_impl_requires_vhe(void) >>>> { >>>> /* Some implementations have defects that confine them to VHE */ >>>> + if (cpus_have_cap(ARM64_WORKAROUND_1165522)) >>>> + return true; >>>> + >>>> return false; >>>> } >>>> >>>> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h >>>> index 23aca66767f9..9681360d0959 100644 >>>> --- a/arch/arm64/include/asm/kvm_hyp.h >>>> +++ b/arch/arm64/include/asm/kvm_hyp.h >>>> @@ -163,6 +163,12 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm) >>>> { >>>> write_sysreg(kvm->arch.vtcr, vtcr_el2); >>>> write_sysreg(kvm->arch.vttbr, vttbr_el2); >>>> + >>>> + /* >>>> + * ARM erratum 1165522 requires the actual execution of the >>>> + * above before we can switch to the guest translation regime. >>>> + */ >>> >>> Is it about a guest translation 'regime' or should this just say before >>> we can enable stage 2 translation? >> >> No, this isn't strictly about enabling stage-2 translation. This is >> about making sure that anything that impacts the guest translations is >> actually executed. >> >> I wonder if it would be clearer to move this outside of >> __load_guest_stage2 and make it explicit in the callers of this helper... > > I think it makes sense to have this here to explain the alternative. > > But it's the 'switch to guest translation regime' thing that bothers me > a bit. Is that an architectural concept (I thought we only had EL1 and > EL2 translation regimes in stage 1, and then stage 2 translations). So > When you say 'guest translation regime' I'm just not entirely sure what > that means, unless I'm missing something. [coming back to this as I'm preparing the next round] What I'm trying to convey by "guest translation regime" is the following from the ARM ARM: C5.4.7 AT S1E1R, Address Translate Stage 1 EL1 Read [...] When EL2 is implemented and enabled in the current Security state: — If HCR_EL2.{E2H, TGE} is not {1, 1}, the EL1&0 translation regime, accessed from EL1. — If HCR_EL2.{E2H, TGE} is {1, 1}, the EL2&0 translation regime, accessed from EL2. So maybe I should just bite the bullet and call out EL1/EL2 translation regime instead of guest/host. Thoughts? M.
On Thu, Nov 22, 2018 at 04:13:16PM +0000, Marc Zyngier wrote: > On 08/11/2018 20:10, Christoffer Dall wrote: > > On Thu, Nov 08, 2018 at 06:05:55PM +0000, Marc Zyngier wrote: > >> On 06/11/18 08:15, Christoffer Dall wrote: > >>> On Mon, Nov 05, 2018 at 02:36:16PM +0000, Marc Zyngier wrote: > >>>> Early versions of Cortex-A76 can end-up with corrupt TLBs if they > >>>> speculate an AT instruction in during a guest switch while the > >>>> S1/S2 system registers are in an inconsistent state. > >>>> > >>>> Work around it by: > >>>> - Mandating VHE > >>>> - Make sure that S1 and S2 system registers are consistent before > >>>> clearing HCR_EL2.TGE, which allows AT to target the EL1 translation > >>>> regime > >>>> > >>>> These two things together ensure that we cannot hit this erratum. > >>>> > >>>> Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> > >>>> --- > >>>> Documentation/arm64/silicon-errata.txt | 1 + > >>>> arch/arm64/Kconfig | 12 ++++++++++++ > >>>> arch/arm64/include/asm/cpucaps.h | 3 ++- > >>>> arch/arm64/include/asm/kvm_host.h | 3 +++ > >>>> arch/arm64/include/asm/kvm_hyp.h | 6 ++++++ > >>>> arch/arm64/kernel/cpu_errata.c | 8 ++++++++ > >>>> arch/arm64/kvm/hyp/switch.c | 14 ++++++++++++++ > >>>> 7 files changed, 46 insertions(+), 1 deletion(-) > >>>> > >>>> diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt > >>>> index 76ccded8b74c..04f0bc4690c6 100644 > >>>> --- a/Documentation/arm64/silicon-errata.txt > >>>> +++ b/Documentation/arm64/silicon-errata.txt > >>>> @@ -57,6 +57,7 @@ stable kernels. > >>>> | ARM | Cortex-A73 | #858921 | ARM64_ERRATUM_858921 | > >>>> | ARM | Cortex-A55 | #1024718 | ARM64_ERRATUM_1024718 | > >>>> | ARM | Cortex-A76 | #1188873 | ARM64_ERRATUM_1188873 | > >>>> +| ARM | Cortex-A76 | #1165522 | ARM64_ERRATUM_1165522 | > >>>> | ARM | MMU-500 | #841119,#826419 | N/A | > >>>> | | | | | > >>>> | Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 | > >>>> diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > >>>> index 787d7850e064..a68bc6cc2167 100644 > >>>> --- a/arch/arm64/Kconfig > >>>> +++ b/arch/arm64/Kconfig > >>>> @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873 > >>>> > >>>> If unsure, say Y. > >>>> > >>>> +config ARM64_ERRATUM_1165522 > >>>> + bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation" > >>>> + default y > >>>> + help > >>>> + This option adds work arounds for ARM Cortex-A76 erratum 1165522 > >>>> + > >>>> + Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with > >>>> + corrupted TLBs by speculating an AT instruction during a guest > >>>> + context switch. > >>>> + > >>>> + If unsure, say Y. > >>>> + > >>>> config CAVIUM_ERRATUM_22375 > >>>> bool "Cavium erratum 22375, 24313" > >>>> default y > >>>> diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h > >>>> index 6e2d254c09eb..62d8cd15fdf2 100644 > >>>> --- a/arch/arm64/include/asm/cpucaps.h > >>>> +++ b/arch/arm64/include/asm/cpucaps.h > >>>> @@ -54,7 +54,8 @@ > >>>> #define ARM64_HAS_CRC32 33 > >>>> #define ARM64_SSBS 34 > >>>> #define ARM64_WORKAROUND_1188873 35 > >>>> +#define ARM64_WORKAROUND_1165522 36 > >>>> > >>>> -#define ARM64_NCAPS 36 > >>>> +#define ARM64_NCAPS 37 > >>>> > >>>> #endif /* __ASM_CPUCAPS_H */ > >>>> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > >>>> index 7d6e974d024a..8f486026ac87 100644 > >>>> --- a/arch/arm64/include/asm/kvm_host.h > >>>> +++ b/arch/arm64/include/asm/kvm_host.h > >>>> @@ -435,6 +435,9 @@ static inline bool kvm_arch_sve_requires_vhe(void) > >>>> static inline bool kvm_arch_impl_requires_vhe(void) > >>>> { > >>>> /* Some implementations have defects that confine them to VHE */ > >>>> + if (cpus_have_cap(ARM64_WORKAROUND_1165522)) > >>>> + return true; > >>>> + > >>>> return false; > >>>> } > >>>> > >>>> diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h > >>>> index 23aca66767f9..9681360d0959 100644 > >>>> --- a/arch/arm64/include/asm/kvm_hyp.h > >>>> +++ b/arch/arm64/include/asm/kvm_hyp.h > >>>> @@ -163,6 +163,12 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm) > >>>> { > >>>> write_sysreg(kvm->arch.vtcr, vtcr_el2); > >>>> write_sysreg(kvm->arch.vttbr, vttbr_el2); > >>>> + > >>>> + /* > >>>> + * ARM erratum 1165522 requires the actual execution of the > >>>> + * above before we can switch to the guest translation regime. > >>>> + */ > >>> > >>> Is it about a guest translation 'regime' or should this just say before > >>> we can enable stage 2 translation? > >> > >> No, this isn't strictly about enabling stage-2 translation. This is > >> about making sure that anything that impacts the guest translations is > >> actually executed. > >> > >> I wonder if it would be clearer to move this outside of > >> __load_guest_stage2 and make it explicit in the callers of this helper... > > > > I think it makes sense to have this here to explain the alternative. > > > > But it's the 'switch to guest translation regime' thing that bothers me > > a bit. Is that an architectural concept (I thought we only had EL1 and > > EL2 translation regimes in stage 1, and then stage 2 translations). So > > When you say 'guest translation regime' I'm just not entirely sure what > > that means, unless I'm missing something. > > [coming back to this as I'm preparing the next round] > > What I'm trying to convey by "guest translation regime" is the following > from the ARM ARM: > > C5.4.7 AT S1E1R, Address Translate Stage 1 EL1 Read > [...] > > When EL2 is implemented and enabled in the current Security state: > — If HCR_EL2.{E2H, TGE} is not {1, 1}, the EL1&0 translation regime, > accessed from EL1. > — If HCR_EL2.{E2H, TGE} is {1, 1}, the EL2&0 translation regime, > accessed from EL2. > > So maybe I should just bite the bullet and call out EL1/EL2 translation > regime instead of guest/host. > How about the following, does that work? /* * ARM erratum 1165522 requires the actual execution of the * above before we can switch to the EL1/EL0 translation regime * used by the guest. */ Thanks, Christoffer
diff --git a/Documentation/arm64/silicon-errata.txt b/Documentation/arm64/silicon-errata.txt index 76ccded8b74c..04f0bc4690c6 100644 --- a/Documentation/arm64/silicon-errata.txt +++ b/Documentation/arm64/silicon-errata.txt @@ -57,6 +57,7 @@ stable kernels. | ARM | Cortex-A73 | #858921 | ARM64_ERRATUM_858921 | | ARM | Cortex-A55 | #1024718 | ARM64_ERRATUM_1024718 | | ARM | Cortex-A76 | #1188873 | ARM64_ERRATUM_1188873 | +| ARM | Cortex-A76 | #1165522 | ARM64_ERRATUM_1165522 | | ARM | MMU-500 | #841119,#826419 | N/A | | | | | | | Cavium | ThunderX ITS | #22375, #24313 | CAVIUM_ERRATUM_22375 | diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig index 787d7850e064..a68bc6cc2167 100644 --- a/arch/arm64/Kconfig +++ b/arch/arm64/Kconfig @@ -497,6 +497,18 @@ config ARM64_ERRATUM_1188873 If unsure, say Y. +config ARM64_ERRATUM_1165522 + bool "Cortex-A76: Speculative AT instruction using out-of-context translation regime could cause subsequent request to generate an incorrect translation" + default y + help + This option adds work arounds for ARM Cortex-A76 erratum 1165522 + + Affected Cortex-A76 cores (r0p0, r1p0, r2p0) could end-up with + corrupted TLBs by speculating an AT instruction during a guest + context switch. + + If unsure, say Y. + config CAVIUM_ERRATUM_22375 bool "Cavium erratum 22375, 24313" default y diff --git a/arch/arm64/include/asm/cpucaps.h b/arch/arm64/include/asm/cpucaps.h index 6e2d254c09eb..62d8cd15fdf2 100644 --- a/arch/arm64/include/asm/cpucaps.h +++ b/arch/arm64/include/asm/cpucaps.h @@ -54,7 +54,8 @@ #define ARM64_HAS_CRC32 33 #define ARM64_SSBS 34 #define ARM64_WORKAROUND_1188873 35 +#define ARM64_WORKAROUND_1165522 36 -#define ARM64_NCAPS 36 +#define ARM64_NCAPS 37 #endif /* __ASM_CPUCAPS_H */ diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h index 7d6e974d024a..8f486026ac87 100644 --- a/arch/arm64/include/asm/kvm_host.h +++ b/arch/arm64/include/asm/kvm_host.h @@ -435,6 +435,9 @@ static inline bool kvm_arch_sve_requires_vhe(void) static inline bool kvm_arch_impl_requires_vhe(void) { /* Some implementations have defects that confine them to VHE */ + if (cpus_have_cap(ARM64_WORKAROUND_1165522)) + return true; + return false; } diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h index 23aca66767f9..9681360d0959 100644 --- a/arch/arm64/include/asm/kvm_hyp.h +++ b/arch/arm64/include/asm/kvm_hyp.h @@ -163,6 +163,12 @@ static __always_inline void __hyp_text __load_guest_stage2(struct kvm *kvm) { write_sysreg(kvm->arch.vtcr, vtcr_el2); write_sysreg(kvm->arch.vttbr, vttbr_el2); + + /* + * ARM erratum 1165522 requires the actual execution of the + * above before we can switch to the guest translation regime. + */ + asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522)); } #endif /* __ARM64_KVM_HYP_H__ */ diff --git a/arch/arm64/kernel/cpu_errata.c b/arch/arm64/kernel/cpu_errata.c index a509e35132d2..476e738e6c46 100644 --- a/arch/arm64/kernel/cpu_errata.c +++ b/arch/arm64/kernel/cpu_errata.c @@ -739,6 +739,14 @@ const struct arm64_cpu_capabilities arm64_errata[] = { .capability = ARM64_WORKAROUND_1188873, ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0), }, +#endif +#ifdef CONFIG_ARM64_ERRATUM_1165522 + { + /* Cortex-A76 r0p0 to r2p0 */ + .desc = "ARM erratum 1165522", + .capability = ARM64_WORKAROUND_1165522, + ERRATA_MIDR_RANGE(MIDR_CORTEX_A76, 0, 0, 2, 0), + }, #endif { } diff --git a/arch/arm64/kvm/hyp/switch.c b/arch/arm64/kvm/hyp/switch.c index 51d5d966d9e5..322109183853 100644 --- a/arch/arm64/kvm/hyp/switch.c +++ b/arch/arm64/kvm/hyp/switch.c @@ -143,6 +143,13 @@ static void deactivate_traps_vhe(void) { extern char vectors[]; /* kernel exception vectors */ write_sysreg(HCR_HOST_VHE_FLAGS, hcr_el2); + + /* + * ARM erratum 1165522 requires the actual execution of the + * above before we can switch to the host translation regime. + */ + asm(ALTERNATIVE("nop", "isb", ARM64_WORKAROUND_1165522)); + write_sysreg(CPACR_EL1_DEFAULT, cpacr_el1); write_sysreg(vectors, vbar_el1); } @@ -499,6 +506,13 @@ int kvm_vcpu_run_vhe(struct kvm_vcpu *vcpu) sysreg_save_host_state_vhe(host_ctxt); + /* + * ARM erratum 1165522 requires us to have all the translation + * context in place before we clear HCR_TGE. All the offending + * guest sysregs are loaded in kvm_vcpu_load_sysregs, and + * __activate_vm has the stage-2 configuration. Once this is + * done, __activate_trap clears HCR_TGE (among other things). + */ __activate_vm(vcpu->kvm); __activate_traps(vcpu);
Early versions of Cortex-A76 can end-up with corrupt TLBs if they speculate an AT instruction in during a guest switch while the S1/S2 system registers are in an inconsistent state. Work around it by: - Mandating VHE - Make sure that S1 and S2 system registers are consistent before clearing HCR_EL2.TGE, which allows AT to target the EL1 translation regime These two things together ensure that we cannot hit this erratum. Signed-off-by: Marc Zyngier <marc.zyngier@arm.com> --- Documentation/arm64/silicon-errata.txt | 1 + arch/arm64/Kconfig | 12 ++++++++++++ arch/arm64/include/asm/cpucaps.h | 3 ++- arch/arm64/include/asm/kvm_host.h | 3 +++ arch/arm64/include/asm/kvm_hyp.h | 6 ++++++ arch/arm64/kernel/cpu_errata.c | 8 ++++++++ arch/arm64/kvm/hyp/switch.c | 14 ++++++++++++++ 7 files changed, 46 insertions(+), 1 deletion(-)