KVM: arm64: Synchronise speculative page table walks on translation regime change

When taking an exception between the EL1&0 translation regime and the EL2 translation regime, the page table walker is allowed to complete the walks started from EL0 or EL1 while running at EL2.

It means that altering the system registers that define the EL1&0 translation regime is fraught with danger *unless* we wait for the completion of such walk with a DSB (R_LFHQG and subsequent statements in the ARM ARM). We already did the right thing for other external agents (SPE, TRBE), but not the PTW.

Rework the existing SPE/TRBE synchronisation to include the PTW, and add the missing DSB on guest exit.

Signed-off-by Marc Zyngier <[email protected]>

A TLBI from EL2 impacting EL1 involves messing with the EL1&0 translation regime, and the page table walker may still be performing speculative walks.

Piggyback on the existing DSBs to always have a DSB ISH that will synchronise all load/store operations that the PTW may still have.

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/kvm/hyp/nvhe/tlb.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

arch/arm64/kvm/hyp/nvhe/tlb.c +7 -0

index d296d617f589..e86dd04d49ff 100644
@@ -17,6 +17,23 @@ struct tlb_inv_context {
 static void __tlb_switch_to_guest(struct kvm_s2_mmu *mmu,
 				  struct tlb_inv_context *cxt)
 {
+	/*
+	 * We have two requirements:
+	 *
+	 * - ensure that the page table updates are visible to all
+         *   CPUs, for which a dsb(ishst) is what we need
+	 *
+	 * - complete any speculative page table walk started before

arch/arm64/kvm/hyp/nvhe/tlb.c +0 -4

 	if (cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT)) {
 		u64 val;
 

@@ -60,8 +77,6 @@ void __kvm_tlb_flush_vmid_ipa(struct kvm_s2_mmu *mmu,
 {
 	struct tlb_inv_context cxt;
 
-	dsb(ishst);
-
 	/* Switch to requested VMID */
 	__tlb_switch_to_guest(mmu, &cxt);
 

@@ -113,8 +128,6 @@ void __kvm_tlb_flush_vmid(struct kvm_s2_mmu *mmu)
 {
 	struct tlb_inv_context cxt;
 
-	dsb(ishst);
-
 	/* Switch to requested VMID */
 	__tlb_switch_to_guest(mmu, &cxt);
 

arch/arm64/kvm/hyp/nvhe/tlb.c +1 -0

+	dsb(ish);
 	__tlbi(alle1is);
 
 	/*

Signature · 1 line

2.34.1

Hi Marc,

Quoted 36 lines · Marc Zyngier

A TLBI from EL2 impacting EL1 involves messing with the EL1&0 translation regime, and the page table walker may still be performing speculative walks.

Piggyback on the existing DSBs to always have a DSB ISH that will synchronise all load/store operations that the PTW may still have.

Signed-off-by Marc Zyngier <[email protected]>

---
 arch/arm64/kvm/hyp/nvhe/tlb.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

arch/arm64/kvm/hyp/nvhe/tlb.c +15 -0

index d296d617f589..e86dd04d49ff 100644
@@ -17,6 +17,23 @@ struct tlb_inv_context {
 static void __tlb_switch_to_guest(struct kvm_s2_mmu *mmu,
 				  struct tlb_inv_context *cxt)
 {
+	/*
+	 * We have two requirements:
+	 *
+	 * - ensure that the page table updates are visible to all
+         *   CPUs, for which a dsb(ishst) is what we need
+	 *
+	 * - complete any speculative page table walk started before
+         *   we trapped to EL2 so that we can mess with the MM
+         *   registers out of context, for which dsb(nsh) is enough
+	 *
+	 * The composition of these two barriers is a dsb(ish). This
+	 * might be slightly over the top for non-shareable TLBIs, but
+	 * they are so vanishingly rare that it isn't worth the
+	 * complexity.
+	 */

arch/arm64/kvm/hyp/nvhe/tlb.c struct tlb_inv_context { · line 36

Quoted 2 lines

arch/arm64/kvm/hyp/nvhe/tlb.c +2 -0

+	dsb(ish);
+

Ricardo is carrying a patch for non-shareable TLBIs on permission relaxation [*], and he's found that it produces some rather desirable performance improvements. I appreciate the elegance of your approach, but given what's coming does it make sense to have the TLBI handlers continue to explicitly perform the appropriate DSB?

We rely on the presence of a DSB at the end of kvm_flush_dcache_to_poc() that, on top of ensuring completion of the cache clean, also covers the speculative page table walk started from EL1.

Document this dependency.

Signed-off-by Marc Zyngier <[email protected]>

Contrary to nVHE, VHE is a lot easier when it comes to dealing with speculative page table walks started at EL1. As we only change EL1&0 translation regime when context-switching, we already benefit from the effect of the DSB that sits in the context switch code.

We only need to take care of it in the NV case, where we can flip between between two EL1 contexts (one of them being the virtual EL2) without a context switch.

Signed-off-by Marc Zyngier <[email protected]>

__kvm_vcpu_run_vhe() end on VHE with an isb(). However, this function is only reachable via kvm_call_hyp_ret(), which already contains an isb() in order to mimick the behaviour of nVHE and provide a context synchronisation event.

We thus have two isb()s back to back, which is one too many. Drop the first one and solely rely on the one in the helper.

Signed-off-by Marc Zyngier <[email protected]>