Skip to contentSkip to conversation

KVM: arm64: Fix SPE and TRBE nVHE world switch

PATCH Applied Will Deacon 3 participants 6 messages Mar 27 – Mar 28

Subsystems (2)
lore
Share & tools
lore

Will Deacon

author maintainer
#0 lore raw

v4 cover KVM: arm64: Fix SPE and TRBE nVHE world switch

Hi all,

I got the Sashiko treatment on v3, so here's a quick respin to address the BRBE thinko it found in the last patch.

Previous versions of the series are available at:

v1: https://lore.kernel.org/r/[email protected]
v2: https://lore.kernel.org/r/[email protected]
v3: https://lore.kernel.org/r/[email protected]

Changes since v3 include:

* Retain zeroing of *brbcr_el1 when saving BRBE state
* Dropped R-b / T-b tags on the BRBE patch

Cheers,

Will

Cc Marc Zyngier <[email protected]>
Cc Oliver Upton <[email protected]>
Cc James Clark <[email protected]>
Cc Leo Yan <[email protected]>
Cc Suzuki K Poulose <[email protected]>
Cc Fuad Tabba <[email protected]>
Cc Alexandru Elisei <[email protected]>
Cc Yabin Cui <[email protected]>

--->8

Will Deacon (3):
KVM: arm64: Disable TRBE Trace Buffer Unit when running in guest
context
KVM: arm64: Disable SPE Profiling Buffer when running in guest context
KVM: arm64: Don't pass host_debug_state to BRBE world-switch routines

 arch/arm64/include/asm/kvm_host.h  |   2 +
 arch/arm64/kvm/hyp/nvhe/debug-sr.c | 116 +++++++++++++++++++++++------
 arch/arm64/kvm/hyp/nvhe/switch.c   |   2 +-
 3 files changed, 95 insertions(+), 25 deletions(-)
Signature · 1 line
2.53.0.1018.g2bb0e51243-goog
author maintainer
#1 lore raw

PATCH v4 1/3 KVM: arm64: Disable TRBE Trace Buffer Unit when running in guest context

The nVHE world-switch code relies on zeroing TRFCR_EL1 to disable trace generation in guest context when self-hosted TRBE is in use by the host.

Per D3.2.1 ("Controls to prohibit trace at Exception levels"), clearing TRFCR_EL1 means that trace generation is prohibited at EL1 and EL0 but per R_YCHKJ the Trace Buffer Unit will still be enabled if TRBLIMITR_EL1.E is set. R_SJFRQ goes on to state that, when enabled, the Trace Buffer Unit can perform address translation for the "owning exception level" even when it is out of context.

Consequently, we can end up in a state where TRBE performs speculative page-table walks for a host VA/IPA in guest/hypervisor context depending on the value of MDCR_EL2.E2TB, which changes over world-switch. The potential result appears to be a heady mixture of SErrors, data corruption and hardware lockups.

Extend the TRBE world-switch code to clear TRBLIMITR_EL1.E after draining the buffer, restoring the register on return to the host. This unfortunately means we need to tackle CPU errata #2064142 and #2038923 which add additional synchronisation requirements around manipulations of the limit register. Hopefully this doesn't need to be fast.

Cc Marc Zyngier <[email protected]>
Cc Oliver Upton <[email protected]>
Cc James Clark <[email protected]>
Cc Leo Yan <[email protected]>
Cc Suzuki K Poulose <[email protected]>
Cc Fuad Tabba <[email protected]>
Cc Alexandru Elisei <[email protected]>
Fixes a1319260bf62 ("arm64: KVM: Enable access to TRBE support for host")
---
 arch/arm64/include/asm/kvm_host.h  |  1 +
 arch/arm64/kvm/hyp/nvhe/debug-sr.c | 71 ++++++++++++++++++++++++++----
 arch/arm64/kvm/hyp/nvhe/switch.c   |  2 +-
 3 files changed, 64 insertions(+), 10 deletions(-)
arch/arm64/include/asm/kvm_host.h +1 -0
index 70cb9cfd760a..b1335c55dbef 100644
@@ -770,6 +770,7 @@ struct kvm_host_data {
 		u64 pmscr_el1;
 		/* Self-hosted trace */
 		u64 trfcr_el1;
+		u64 trblimitr_el1;
 		/* Values of trap registers for the host before guest entry. */
 		u64 mdcr_el2;
 		u64 brbcr_el1;
arch/arm64/kvm/hyp/nvhe/debug-sr.c +62 -9

89 lines not on this page · raw message

arch/arm64/kvm/hyp/nvhe/switch.c +1 -1
index 779089e42681..f00688e69d88 100644
@@ -278,7 +278,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 	 * We're about to restore some new MMU state. Make sure
 	 * ongoing page-table walks that have started before we
 	 * trapped to EL2 have completed. This also synchronises the
-	 * above disabling of BRBE, SPE and TRBE.
+	 * above disabling of BRBE and SPE.
 	 *
 	 * See DDI0487I.a D8.1.5 "Out-of-context translation regimes",
 	 * rule R_LFHQG and subsequent information statements.
Signature · 1 line
2.53.0.1018.g2bb0e51243-goog
author maintainer
#2 lore raw

PATCH v4 2/3 KVM: arm64: Disable SPE Profiling Buffer when running in guest context

In reply to Will Deacon

The nVHE world-switch code relies on zeroing PMSCR_EL1 to disable profiling data generation in guest context when SPE is in use by the host.

Unfortunately, this may leave PMBLIMITR_EL1.E set and consequently we can end up running in guest/hypervisor context with the Profiling Buffer enabled. The current "known issues" document for Rev M.a of the Arm ARM states that this can lead to speculative, out-of-context translations:

  | 2.18 D23136:
  |
  | When the Profiling Buffer is enabled, profiling is not stopped, and
  | Discard mode is not enabled, the Statistical Profiling Unit might
  | cause speculative translations for the owning translation regime,
  | including when the owning translation regime is out-of-context.

In a similar fashion to TRBE, ensure that the Profiling Buffer is disabled during the nVHE world switch before we start messing with the stage-2 MMU and trap configuration.

Cc Marc Zyngier <[email protected]>
Cc Oliver Upton <[email protected]>
Cc James Clark <[email protected]>
Cc Leo Yan <[email protected]>
Cc Suzuki K Poulose <[email protected]>
Cc Fuad Tabba <[email protected]>
Cc Alexandru Elisei <[email protected]>
Fixes f85279b4bd48 ("arm64: KVM: Save/restore the host SPE state when entering/leaving a VM")
---
 arch/arm64/include/asm/kvm_host.h  |  1 +
 arch/arm64/kvm/hyp/nvhe/debug-sr.c | 33 ++++++++++++++++++++----------
 arch/arm64/kvm/hyp/nvhe/switch.c   |  2 +-
 3 files changed, 24 insertions(+), 12 deletions(-)
arch/arm64/include/asm/kvm_host.h +1 -0
index b1335c55dbef..fe588760fe62 100644
@@ -768,6 +768,7 @@ struct kvm_host_data {
 		struct kvm_guest_debug_arch regs;
 		/* Statistical profiling extension */
 		u64 pmscr_el1;
+		u64 pmblimitr_el1;
 		/* Self-hosted trace */
 		u64 trfcr_el1;
 		u64 trblimitr_el1;
arch/arm64/kvm/hyp/nvhe/debug-sr.c +22 -11
index 0955af771ad1..84bc80f4e36b 100644
@@ -14,20 +14,20 @@
 #include <asm/kvm_hyp.h>
 #include <asm/kvm_mmu.h>
 
-static void __debug_save_spe(u64 *pmscr_el1)
+static void __debug_save_spe(void)
 {
-	u64 reg;
+	u64 *pmscr_el1, *pmblimitr_el1;
 
-	/* Clear pmscr in case of early return */
-	*pmscr_el1 = 0;
+	pmscr_el1 = host_data_ptr(host_debug_state.pmscr_el1);
+	pmblimitr_el1 = host_data_ptr(host_debug_state.pmblimitr_el1);
 
 	/*
 	 * At this point, we know that this CPU implements
 	 * SPE and is available to the host.
 	 * Check if the host is actually using it ?
 	 */
-	reg = read_sysreg_s(SYS_PMBLIMITR_EL1);
-	if (!(reg & BIT(PMBLIMITR_EL1_E_SHIFT)))
+	*pmblimitr_el1 = read_sysreg_s(SYS_PMBLIMITR_EL1);
+	if (!(*pmblimitr_el1 & BIT(PMBLIMITR_EL1_E_SHIFT)))
 		return;
 
 	/* Yes; save the control register and disable data generation */
@@ -37,18 +37,29 @@ static void __debug_save_spe(u64 *pmscr_el1)
 
 	/* Now drain all buffered data to memory */
 	psb_csync();
+	dsb(nsh);
+
+	/* And disable the profiling buffer */
+	write_sysreg_s(0, SYS_PMBLIMITR_EL1);
+	isb();
 }
 
-static void __debug_restore_spe(u64 pmscr_el1)
+static void __debug_restore_spe(void)
 {
-	if (!pmscr_el1)
+	u64 pmblimitr_el1 = *host_data_ptr(host_debug_state.pmblimitr_el1);
+
+	if (!(pmblimitr_el1 & BIT(PMBLIMITR_EL1_E_SHIFT)))
 		return;
 
 	/* The host page table is installed, but not yet synchronised */
 	isb();
 
+	/* Re-enable the profiling buffer. */
+	write_sysreg_s(pmblimitr_el1, SYS_PMBLIMITR_EL1);
+	isb();
+
 	/* Re-enable data generation */
-	write_sysreg_el1(pmscr_el1, SYS_PMSCR);
+	write_sysreg_el1(*host_data_ptr(host_debug_state.pmscr_el1), SYS_PMSCR);
 }
 
 static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr)
@@ -175,7 +186,7 @@ void __debug_save_host_buffers_nvhe(struct kvm_vcpu *vcpu)
 {
 	/* Disable and flush SPE data generation */
 	if (host_data_test_flag(HAS_SPE))
-		__debug_save_spe(host_data_ptr(host_debug_state.pmscr_el1));
+		__debug_save_spe();
 
 	/* Disable BRBE branch records */
 	if (host_data_test_flag(HAS_BRBE))
@@ -193,7 +204,7 @@ void __debug_switch_to_guest(struct kvm_vcpu *vcpu)
 void __debug_restore_host_buffers_nvhe(struct kvm_vcpu *vcpu)
 {
 	if (host_data_test_flag(HAS_SPE))
-		__debug_restore_spe(*host_data_ptr(host_debug_state.pmscr_el1));
+		__debug_restore_spe();
 	if (host_data_test_flag(HAS_BRBE))
 		__debug_restore_brbe(*host_data_ptr(host_debug_state.brbcr_el1));
 	if (__trace_needs_switch())
arch/arm64/kvm/hyp/nvhe/switch.c +1 -1
index f00688e69d88..9b6e87dac3b9 100644
@@ -278,7 +278,7 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 	 * We're about to restore some new MMU state. Make sure
 	 * ongoing page-table walks that have started before we
 	 * trapped to EL2 have completed. This also synchronises the
-	 * above disabling of BRBE and SPE.
+	 * above disabling of BRBE.
 	 *
 	 * See DDI0487I.a D8.1.5 "Out-of-context translation regimes",
 	 * rule R_LFHQG and subsequent information statements.
Signature · 1 line
2.53.0.1018.g2bb0e51243-goog
author maintainer
#3 lore raw

PATCH v4 3/3 KVM: arm64: Don't pass host_debug_state to BRBE world-switch routines

In reply to Will Deacon

Now that the SPE and BRBE nVHE world-switch routines operate on the host_debug_state directly, tweak the BRBE code to do the same for consistency.

This is purely cosmetic.

Cc Marc Zyngier <[email protected]>
Cc Oliver Upton <[email protected]>
Cc James Clark <[email protected]>
Cc Leo Yan <[email protected]>
Cc Suzuki K Poulose <[email protected]>
Cc Fuad Tabba <[email protected]>
Cc Alexandru Elisei <[email protected]>
---
 arch/arm64/kvm/hyp/nvhe/debug-sr.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
arch/arm64/kvm/hyp/nvhe/debug-sr.c +8 -4
index 84bc80f4e36b..f8904391c125 100644
@@ -156,8 +156,10 @@ static void __trace_switch_to_host(void)
 			  *host_data_ptr(host_debug_state.trfcr_el1));
 }
 
-static void __debug_save_brbe(u64 *brbcr_el1)
+static void __debug_save_brbe(void)
 {
+	u64 *brbcr_el1 = host_data_ptr(host_debug_state.brbcr_el1);
+
 	*brbcr_el1 = 0;
 
 	/* Check if the BRBE is enabled */
@@ -173,8 +175,10 @@ static void __debug_save_brbe(u64 *brbcr_el1)
 	write_sysreg_el1(0, SYS_BRBCR);
 }
 
-static void __debug_restore_brbe(u64 brbcr_el1)
+static void __debug_restore_brbe(void)
 {
+	u64 brbcr_el1 = *host_data_ptr(host_debug_state.brbcr_el1);
+
 	if (!brbcr_el1)
 		return;
 
@@ -190,7 +194,7 @@ void __debug_save_host_buffers_nvhe(struct kvm_vcpu *vcpu)
 
 	/* Disable BRBE branch records */
 	if (host_data_test_flag(HAS_BRBE))
-		__debug_save_brbe(host_data_ptr(host_debug_state.brbcr_el1));
+		__debug_save_brbe();
 
 	if (__trace_needs_switch())
 		__trace_switch_to_guest();
@@ -206,7 +210,7 @@ void __debug_restore_host_buffers_nvhe(struct kvm_vcpu *vcpu)
 	if (host_data_test_flag(HAS_SPE))
 		__debug_restore_spe();
 	if (host_data_test_flag(HAS_BRBE))
-		__debug_restore_brbe(*host_data_ptr(host_debug_state.brbcr_el1));
+		__debug_restore_brbe();
 	if (__trace_needs_switch())
 		__trace_switch_to_host();
 }
Signature · 1 line
2.53.0.1018.g2bb0e51243-goog
Tested-by Reviewed-by on v4 3/3
#4 lore raw

Re: [PATCH v4 3/3] KVM: arm64: Don't pass host_debug_state to BRBE world-switch routines

Quoted 6 lines · Will Deacon

Now that the SPE and BRBE nVHE world-switch routines operate on the host_debug_state directly, tweak the BRBE code to do the same for consistency.

This is purely cosmetic.

It is indeed... until sashiko proves me wrong again :)

Cheers,
/fuad

Quoted 62 lines
Cc Marc Zyngier <[email protected]>
Cc Oliver Upton <[email protected]>
Cc James Clark <[email protected]>
Cc Leo Yan <[email protected]>
Cc Suzuki K Poulose <[email protected]>
Cc Fuad Tabba <[email protected]>
Cc Alexandru Elisei <[email protected]>
---
 arch/arm64/kvm/hyp/nvhe/debug-sr.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)
arch/arm64/kvm/hyp/nvhe/debug-sr.c +8 -5
index 84bc80f4e36b..f8904391c125 100644
@@ -156,8 +156,10 @@ static void __trace_switch_to_host(void)
                          *host_data_ptr(host_debug_state.trfcr_el1));
 }
 
-static void __debug_save_brbe(u64 *brbcr_el1)
+static void __debug_save_brbe(void)
 {
+       u64 *brbcr_el1 = host_data_ptr(host_debug_state.brbcr_el1);
+
        *brbcr_el1 = 0;
 
        /* Check if the BRBE is enabled */
@@ -173,8 +175,10 @@ static void __debug_save_brbe(u64 *brbcr_el1)
        write_sysreg_el1(0, SYS_BRBCR);
 }
 
-static void __debug_restore_brbe(u64 brbcr_el1)
+static void __debug_restore_brbe(void)
 {
+       u64 brbcr_el1 = *host_data_ptr(host_debug_state.brbcr_el1);
+
        if (!brbcr_el1)
                return;
 
@@ -190,7 +194,7 @@ void __debug_save_host_buffers_nvhe(struct kvm_vcpu *vcpu)
 
        /* Disable BRBE branch records */
        if (host_data_test_flag(HAS_BRBE))
-               __debug_save_brbe(host_data_ptr(host_debug_state.brbcr_el1));
+               __debug_save_brbe();
 
        if (__trace_needs_switch())
                __trace_switch_to_guest();
@@ -206,7 +210,7 @@ void __debug_restore_host_buffers_nvhe(struct kvm_vcpu *vcpu)
        if (host_data_test_flag(HAS_SPE))
                __debug_restore_spe();
        if (host_data_test_flag(HAS_BRBE))
-               __debug_restore_brbe(*host_data_ptr(host_debug_state.brbcr_el1));
+               __debug_restore_brbe();
        if (__trace_needs_switch())
                __trace_switch_to_host();
 }
--

2.53.0.1018.g2bb0e51243-goog

maintainer Applied
#5 lore raw
In reply to Will Deacon
Quoted 10 lines · Will Deacon

I got the Sashiko treatment on v3, so here's a quick respin to address the BRBE thinko it found in the last patch.

Previous versions of the series are available at:

v1: https://lore.kernel.org/r/[email protected]
v2: https://lore.kernel.org/r/[email protected]
v3: https://lore.kernel.org/r/[email protected]

[...]

Applied to next, thanks!

[1/3] KVM: arm64: Disable TRBE Trace Buffer Unit when running in guest context

      commit: d133aa75e39dd72e0b8577ab1f5fc17c72246536
[2/3] KVM: arm64: Disable SPE Profiling Buffer when running in guest context
      commit: 07695f7dc1e141601254057a00bf4e23301eb0b2
[3/3] KVM: arm64: Don't pass host_debug_state to BRBE world-switch routines
      commit: 7aba10efef1d972fc82b00b84911f07f6afbdb78

Cheers,

	M.
Signature · 1 line
Without deviation from the norm, progress is not possible.