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 result appears to be a heady mixture of 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.

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]>

Fixes a1319260bf62 ("arm64: KVM: Enable access to TRBE support for host")

Signed-off-by Will Deacon <[email protected]>

---

NOTE: This is *untested* as I don't have a TRBE-capable device that can run upstream but I noticed this by inspection when triaging occasional hardware lockups on systems using a 6.12-based kernel with TRBE running at the same time as a vCPU is loaded. This code has changed quite a bit over time, so stable backports are not entirely straightforward. Hopefully James/Leo/Suzuki can help us test if folks agree with the general approach taken here.

 arch/arm64/include/asm/kvm_host.h  |  1 +
 arch/arm64/kvm/hyp/nvhe/debug-sr.c | 36 ++++++++++++++++++++++--------
 2 files changed, 28 insertions(+), 9 deletions(-)

arch/arm64/include/asm/kvm_host.h +1 -0

index ac7f970c7883..a932cf043b83 100644
@@ -746,6 +746,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 +9 -4

index 2a1c0f49792b..fd389a26bc59 100644
@@ -57,12 +57,27 @@ static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr)
 	write_sysreg_el1(new_trfcr, SYS_TRFCR);
 }
 
-static bool __trace_needs_drain(void)
+static void __trace_drain_and_disable(void)
 {
-	if (is_protected_kvm_enabled() && host_data_test_flag(HAS_TRBE))
-		return read_sysreg_s(SYS_TRBLIMITR_EL1) & TRBLIMITR_EL1_E;
+	u64 *trblimitr_el1 = host_data_ptr(host_debug_state.trblimitr_el1);
 
-	return host_data_test_flag(TRBE_ENABLED);
+	*trblimitr_el1 = 0;
+
+	if (is_protected_kvm_enabled()) {
+		if (!host_data_test_flag(HAS_TRBE))
+			return;
+	} else {
+		if (!host_data_test_flag(TRBE_ENABLED))

Quoted 10 lines · Will Deacon

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.

Great. So TRBE violates all the principles that we hold true in the architecture. Does SPE suffer from the same level of brokenness?

Quoted 67 lines

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 result appears to be a heady mixture of 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.

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]>

Fixes a1319260bf62 ("arm64: KVM: Enable access to TRBE support for host")

Signed-off-by Will Deacon <[email protected]>

---

NOTE: This is *untested* as I don't have a TRBE-capable device that can run upstream but I noticed this by inspection when triaging occasional hardware lockups on systems using a 6.12-based kernel with TRBE running at the same time as a vCPU is loaded. This code has changed quite a bit over time, so stable backports are not entirely straightforward. Hopefully James/Leo/Suzuki can help us test if folks agree with the general approach taken here.

  arch/arm64/include/asm/kvm_host.h  |  1 +
  arch/arm64/kvm/hyp/nvhe/debug-sr.c | 36 ++++++++++++++++++++++--------
  2 files changed, 28 insertions(+), 9 deletions(-)

arch/arm64/include/asm/kvm_host.h +1 -0

index ac7f970c7883..a932cf043b83 100644
@@ -746,6 +746,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 +10 -4

index 2a1c0f49792b..fd389a26bc59 100644
@@ -57,12 +57,27 @@ static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr)
  	write_sysreg_el1(new_trfcr, SYS_TRFCR);
  }
 
-static bool __trace_needs_drain(void)
+static void __trace_drain_and_disable(void)
  {
-	if (is_protected_kvm_enabled() && host_data_test_flag(HAS_TRBE))
-		return read_sysreg_s(SYS_TRBLIMITR_EL1) & TRBLIMITR_EL1_E;
+	u64 *trblimitr_el1 = host_data_ptr(host_debug_state.trblimitr_el1);
 
-	return host_data_test_flag(TRBE_ENABLED);
+	*trblimitr_el1 = 0;
+
+	if (is_protected_kvm_enabled()) {
+		if (!host_data_test_flag(HAS_TRBE))
+			return;
+	} else {
+		if (!host_data_test_flag(TRBE_ENABLED))
+			return;

arch/arm64/kvm/hyp/nvhe/debug-sr.c static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr) · line 79

Quoted 8 lines

arch/arm64/kvm/hyp/nvhe/debug-sr.c +8 -0

+	}
+
+	*trblimitr_el1 = read_sysreg_s(SYS_TRBLIMITR_EL1);
+	if (*trblimitr_el1 & TRBLIMITR_EL1_E) {
+		isb();
+		tsb_csync();
+		write_sysreg_s(0, SYS_TRBLIMITR_EL1);
+		isb();

arch/arm64/kvm/hyp/nvhe/debug-sr.c static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr) · line 80

Quoted 1 line

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

+	}

Doesn't this mean we should be able to get rid of most of the TRFCR messing about that litters the entry/exit code and leave that to VHE

only? And even then, I'm tempted to simply get rid of any sort of guest-only tracing, given that TRBE is not capable of representing exceptions that are synthesised by the host, making it the resulting traces useless.

I'm still trying to get my hands on a TRBE-enabled system that has some actual firmware tables (my O6 seems to have the HW, but no description of the required coresight infra).

Thanks,

	M.

Quoted 87 lines · Marc Zyngier

Quoted 10 lines · Will Deacon

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.

Great. So TRBE violates all the principles that we hold true in the architecture. Does SPE suffer from the same level of brokenness?

Quoted 71 lines

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 result appears to be a heady mixture of 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.

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]>

Fixes a1319260bf62 ("arm64: KVM: Enable access to TRBE support for host")

Signed-off-by Will Deacon <[email protected]>

---

NOTE: This is *untested* as I don't have a TRBE-capable device that can run upstream but I noticed this by inspection when triaging occasional hardware lockups on systems using a 6.12-based kernel with TRBE running at the same time as a vCPU is loaded. This code has changed quite a bit over time, so stable backports are not entirely straightforward. Hopefully James/Leo/Suzuki can help us test if folks agree with the general approach taken here.

  arch/arm64/include/asm/kvm_host.h  |  1 +
  arch/arm64/kvm/hyp/nvhe/debug-sr.c | 36 ++++++++++++++++++++++--------
  2 files changed, 28 insertions(+), 9 deletions(-)

arch/arm64/include/asm/kvm_host.h +1 -0

index ac7f970c7883..a932cf043b83 100644
@@ -746,6 +746,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 +16 -2

index 2a1c0f49792b..fd389a26bc59 100644
@@ -57,12 +57,27 @@ static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr)
  	write_sysreg_el1(new_trfcr, SYS_TRFCR);
  }
  -static bool __trace_needs_drain(void)
+static void __trace_drain_and_disable(void)
  {
-	if (is_protected_kvm_enabled() && host_data_test_flag(HAS_TRBE))
-		return read_sysreg_s(SYS_TRBLIMITR_EL1) & TRBLIMITR_EL1_E;
+	u64 *trblimitr_el1 = host_data_ptr(host_debug_state.trblimitr_el1);
  -	return host_data_test_flag(TRBE_ENABLED);
+	*trblimitr_el1 = 0;
+
+	if (is_protected_kvm_enabled()) {
+		if (!host_data_test_flag(HAS_TRBE))
+			return;
+	} else {
+		if (!host_data_test_flag(TRBE_ENABLED))
+			return;
+	}
+
+	*trblimitr_el1 = read_sysreg_s(SYS_TRBLIMITR_EL1);
+	if (*trblimitr_el1 & TRBLIMITR_EL1_E) {
+		isb();
+		tsb_csync();

Quoted 2 lines

arch/arm64/kvm/hyp/nvhe/debug-sr.c static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr) · line 79

Quoted 2 lines

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

+		write_sysreg_s(0, SYS_TRBLIMITR_EL1);
+		isb();

The TRBE driver might do an extra drain here as a workaround. Hard to tell if it's actually required in this case (seems like probably not) but it might be worth doing it anyway to avoid hitting the issue. Especially if we add guest support later where some of the affected registers might start being used.

See:

    if (trbe_needs_drain_after_disable(cpudata))
        trbe_drain_buffer();

Quoted 4 lines

arch/arm64/kvm/hyp/nvhe/debug-sr.c static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr) · line 80

Quoted 1 line

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

+	}

Doesn't this mean we should be able to get rid of most of the TRFCR messing about that litters the entry/exit code and leave that to VHE

Technically you could have ETMs that and are connected to sinks other than TRBE. Unless you somehow switch off those sinks you still need to do the TRFCR switching stuff.

Quoted 4 lines

only? And even then, I'm tempted to simply get rid of any sort of guest-only tracing, given that TRBE is not capable of representing exceptions that are synthesised by the host, making it the resulting traces useless.

I haven't heard of anyone tracing a guest from the host, but until we add support for guests to be able to trace themselves it's the only way of doing it, so it could be useful.

Although all the messing
around with TRFCR before was from a request to disable guest trace rather than enable it, as it was always on in nVHE.

Quoted 3 lines

I'm still trying to get my hands on a TRBE-enabled system that has some actual firmware tables (my O6 seems to have the HW, but no description of the required coresight infra).

Leo has a guide, I haven't tried it myself yet but it should be working. I'll send it to you.

Quoted 91 lines · James Clark

Quoted 87 lines · Marc Zyngier

Quoted 10 lines · Will Deacon

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.

Great. So TRBE violates all the principles that we hold true in the architecture. Does SPE suffer from the same level of brokenness?

Quoted 71 lines

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 result appears to be a heady mixture of 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.

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]>

Fixes a1319260bf62 ("arm64: KVM: Enable access to TRBE support for host")

Signed-off-by Will Deacon <[email protected]>

---

NOTE: This is *untested* as I don't have a TRBE-capable device that can run upstream but I noticed this by inspection when triaging occasional hardware lockups on systems using a 6.12-based kernel with TRBE running at the same time as a vCPU is loaded. This code has changed quite a bit over time, so stable backports are not entirely straightforward. Hopefully James/Leo/Suzuki can help us test if folks agree with the general approach taken here.

   arch/arm64/include/asm/kvm_host.h  |  1 +
   arch/arm64/kvm/hyp/nvhe/debug-sr.c | 36 ++++++++++++++++++++++--------
   2 files changed, 28 insertions(+), 9 deletions(-)

arch/arm64/include/asm/kvm_host.h +1 -0

index ac7f970c7883..a932cf043b83 100644
@@ -746,6 +746,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 +16 -2

index 2a1c0f49792b..fd389a26bc59 100644
@@ -57,12 +57,27 @@ static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr)
   	write_sysreg_el1(new_trfcr, SYS_TRFCR);
   }
   -static bool __trace_needs_drain(void)
+static void __trace_drain_and_disable(void)
   {
-	if (is_protected_kvm_enabled() && host_data_test_flag(HAS_TRBE))
-		return read_sysreg_s(SYS_TRBLIMITR_EL1) & TRBLIMITR_EL1_E;
+	u64 *trblimitr_el1 = host_data_ptr(host_debug_state.trblimitr_el1);
   -	return host_data_test_flag(TRBE_ENABLED);
+	*trblimitr_el1 = 0;
+
+	if (is_protected_kvm_enabled()) {
+		if (!host_data_test_flag(HAS_TRBE))
+			return;
+	} else {
+		if (!host_data_test_flag(TRBE_ENABLED))
+			return;
+	}
+
+	*trblimitr_el1 = read_sysreg_s(SYS_TRBLIMITR_EL1);
+	if (*trblimitr_el1 & TRBLIMITR_EL1_E) {
+		isb();
+		tsb_csync();

Quoted 8 lines

Quoted 2 lines

arch/arm64/kvm/hyp/nvhe/debug-sr.c static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr) · line 79

Quoted 2 lines

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

+		write_sysreg_s(0, SYS_TRBLIMITR_EL1);
+		isb();

The TRBE driver might do an extra drain here as a workaround. Hard to tell if it's actually required in this case (seems like probably not) but it might be worth doing it anyway to avoid hitting the issue. Especially if we add guest support later where some of the affected registers might start being used.

Just to set the expectations: guest TRBE support is not happening until the architecture is fixed. It cannot reliably give a trace that includes emulated exceptions, and until then, no TRBE for you.

Quoted 20 lines

See:

     if (trbe_needs_drain_after_disable(cpudata))
         trbe_drain_buffer();

Quoted 4 lines

arch/arm64/kvm/hyp/nvhe/debug-sr.c static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr) · line 80

Quoted 1 line

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

+	}

Doesn't this mean we should be able to get rid of most of the TRFCR messing about that litters the entry/exit code and leave that to VHE

Technically you could have ETMs that and are connected to sinks other than TRBE. Unless you somehow switch off those sinks you still need to do the TRFCR switching stuff.

Quoted 4 lines

only? And even then, I'm tempted to simply get rid of any sort of guest-only tracing, given that TRBE is not capable of representing exceptions that are synthesised by the host, making it the resulting traces useless.

Quoted 3 lines

I haven't heard of anyone tracing a guest from the host, but until we add support for guests to be able to trace themselves it's the only way of doing it, so it could be useful.

But that's *not* working. If you trace EL1 only, even with a VHE host, the result is not usable.

Quoted 3 lines

Although all the messing
around with TRFCR before was from a request to disable guest trace rather than enable it, as it was always on in nVHE.

I can't see how it could ever be "always on", unless you were happy to randomly corrupt memory?

Quoted 6 lines

Quoted 3 lines

I'm still trying to get my hands on a TRBE-enabled system that has some actual firmware tables (my O6 seems to have the HW, but no description of the required coresight infra).

Leo has a guide, I haven't tried it myself yet but it should be working. I'll send it to you.

Thanks. But why isn't that stuff publicly documented? Sight...

	M.

Quoted 130 lines · Marc Zyngier

Quoted 91 lines · James Clark

Quoted 87 lines · Marc Zyngier

Quoted 10 lines · Will Deacon

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.

Great. So TRBE violates all the principles that we hold true in the architecture. Does SPE suffer from the same level of brokenness?

Quoted 71 lines

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 result appears to be a heady mixture of 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.

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]>

Fixes a1319260bf62 ("arm64: KVM: Enable access to TRBE support for host")

Signed-off-by Will Deacon <[email protected]>

---

NOTE: This is *untested* as I don't have a TRBE-capable device that can run upstream but I noticed this by inspection when triaging occasional hardware lockups on systems using a 6.12-based kernel with TRBE running at the same time as a vCPU is loaded. This code has changed quite a bit over time, so stable backports are not entirely straightforward. Hopefully James/Leo/Suzuki can help us test if folks agree with the general approach taken here.

   arch/arm64/include/asm/kvm_host.h  |  1 +
   arch/arm64/kvm/hyp/nvhe/debug-sr.c | 36 ++++++++++++++++++++++--------
   2 files changed, 28 insertions(+), 9 deletions(-)

arch/arm64/include/asm/kvm_host.h +1 -0

index ac7f970c7883..a932cf043b83 100644
@@ -746,6 +746,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 +16 -2

index 2a1c0f49792b..fd389a26bc59 100644
@@ -57,12 +57,27 @@ static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr)
   	write_sysreg_el1(new_trfcr, SYS_TRFCR);
   }
   -static bool __trace_needs_drain(void)
+static void __trace_drain_and_disable(void)
   {
-	if (is_protected_kvm_enabled() && host_data_test_flag(HAS_TRBE))
-		return read_sysreg_s(SYS_TRBLIMITR_EL1) & TRBLIMITR_EL1_E;
+	u64 *trblimitr_el1 = host_data_ptr(host_debug_state.trblimitr_el1);
   -	return host_data_test_flag(TRBE_ENABLED);
+	*trblimitr_el1 = 0;
+
+	if (is_protected_kvm_enabled()) {
+		if (!host_data_test_flag(HAS_TRBE))
+			return;
+	} else {
+		if (!host_data_test_flag(TRBE_ENABLED))
+			return;
+	}
+
+	*trblimitr_el1 = read_sysreg_s(SYS_TRBLIMITR_EL1);
+	if (*trblimitr_el1 & TRBLIMITR_EL1_E) {
+		isb();
+		tsb_csync();

Quoted 8 lines

Quoted 2 lines

arch/arm64/kvm/hyp/nvhe/debug-sr.c static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr) · line 79

Quoted 2 lines

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

+		write_sysreg_s(0, SYS_TRBLIMITR_EL1);
+		isb();

The TRBE driver might do an extra drain here as a workaround. Hard to tell if it's actually required in this case (seems like probably not) but it might be worth doing it anyway to avoid hitting the issue. Especially if we add guest support later where some of the affected registers might start being used.

Just to set the expectations: guest TRBE support is not happening until the architecture is fixed. It cannot reliably give a trace that includes emulated exceptions, and until then, no TRBE for you.

Quoted 23 lines

See:

     if (trbe_needs_drain_after_disable(cpudata))
         trbe_drain_buffer();

Quoted 4 lines

arch/arm64/kvm/hyp/nvhe/debug-sr.c static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr) · line 80

Quoted 1 line

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

+	}

Doesn't this mean we should be able to get rid of most of the TRFCR messing about that litters the entry/exit code and leave that to VHE

Technically you could have ETMs that and are connected to sinks other than TRBE. Unless you somehow switch off those sinks you still need to do the TRFCR switching stuff.

Quoted 4 lines

only? And even then, I'm tempted to simply get rid of any sort of guest-only tracing, given that TRBE is not capable of representing exceptions that are synthesised by the host, making it the resulting traces useless.

I haven't heard of anyone tracing a guest from the host, but until we add support for guests to be able to trace themselves it's the only way of doing it, so it could be useful.

Quoted 3 lines

But that's *not* working. If you trace EL1 only, even with a VHE host, the result is not usable.

Do you mean not working because of the missing exceptions? I did a bit of testing before and the trace did seem somewhat usable to me. It had EL1 and EL0 atoms in there.

All you need is the mmap records from the
guest which you can get by running Perf in the guest and it's possible to decode it. Maybe it's not complete but I don't think all use cases require complete trace. AutoFDO for example just needs lots of small snippets of execution history.

Quoted 134 lines · James Clark

Quoted 130 lines · Marc Zyngier

Quoted 91 lines · James Clark

Quoted 87 lines · Marc Zyngier

Quoted 10 lines · Will Deacon

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.

Great. So TRBE violates all the principles that we hold true in the architecture. Does SPE suffer from the same level of brokenness?

Quoted 71 lines

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 result appears to be a heady mixture of 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.

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]>

Fixes a1319260bf62 ("arm64: KVM: Enable access to TRBE support for host")

Signed-off-by Will Deacon <[email protected]>

---

NOTE: This is *untested* as I don't have a TRBE-capable device that can run upstream but I noticed this by inspection when triaging occasional hardware lockups on systems using a 6.12-based kernel with TRBE running at the same time as a vCPU is loaded. This code has changed quite a bit over time, so stable backports are not entirely straightforward. Hopefully James/Leo/Suzuki can help us test if folks agree with the general approach taken here.

    arch/arm64/include/asm/kvm_host.h  |  1 +
    arch/arm64/kvm/hyp/nvhe/debug-sr.c | 36 ++++++++++++++++++++++--------
    2 files changed, 28 insertions(+), 9 deletions(-)

arch/arm64/include/asm/kvm_host.h +1 -0

index ac7f970c7883..a932cf043b83 100644
@@ -746,6 +746,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 +16 -2

index 2a1c0f49792b..fd389a26bc59 100644
@@ -57,12 +57,27 @@ static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr)
    	write_sysreg_el1(new_trfcr, SYS_TRFCR);
    }
    -static bool __trace_needs_drain(void)
+static void __trace_drain_and_disable(void)
    {
-	if (is_protected_kvm_enabled() && host_data_test_flag(HAS_TRBE))
-		return read_sysreg_s(SYS_TRBLIMITR_EL1) & TRBLIMITR_EL1_E;
+	u64 *trblimitr_el1 = host_data_ptr(host_debug_state.trblimitr_el1);
    -	return host_data_test_flag(TRBE_ENABLED);
+	*trblimitr_el1 = 0;
+
+	if (is_protected_kvm_enabled()) {
+		if (!host_data_test_flag(HAS_TRBE))
+			return;
+	} else {
+		if (!host_data_test_flag(TRBE_ENABLED))
+			return;
+	}
+
+	*trblimitr_el1 = read_sysreg_s(SYS_TRBLIMITR_EL1);
+	if (*trblimitr_el1 & TRBLIMITR_EL1_E) {
+		isb();
+		tsb_csync();

Quoted 8 lines

Quoted 2 lines

arch/arm64/kvm/hyp/nvhe/debug-sr.c static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr) · line 79

Quoted 2 lines

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

+		write_sysreg_s(0, SYS_TRBLIMITR_EL1);
+		isb();

The TRBE driver might do an extra drain here as a workaround. Hard to tell if it's actually required in this case (seems like probably not) but it might be worth doing it anyway to avoid hitting the issue. Especially if we add guest support later where some of the affected registers might start being used.

Just to set the expectations: guest TRBE support is not happening until the architecture is fixed. It cannot reliably give a trace that includes emulated exceptions, and until then, no TRBE for you.

Quoted 23 lines

See:

      if (trbe_needs_drain_after_disable(cpudata))
          trbe_drain_buffer();

Quoted 4 lines

arch/arm64/kvm/hyp/nvhe/debug-sr.c static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr) · line 80

Quoted 1 line

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

+	}

Doesn't this mean we should be able to get rid of most of the TRFCR messing about that litters the entry/exit code and leave that to VHE

Technically you could have ETMs that and are connected to sinks other than TRBE. Unless you somehow switch off those sinks you still need to do the TRFCR switching stuff.

Quoted 4 lines

only? And even then, I'm tempted to simply get rid of any sort of guest-only tracing, given that TRBE is not capable of representing exceptions that are synthesised by the host, making it the resulting traces useless.

I haven't heard of anyone tracing a guest from the host, but until we add support for guests to be able to trace themselves it's the only way of doing it, so it could be useful.

Quoted 7 lines

Quoted 3 lines

But that's *not* working. If you trace EL1 only, even with a VHE host, the result is not usable.

Do you mean not working because of the missing exceptions? I did a bit of testing before and the trace did seem somewhat usable to me. It had EL1 and EL0 atoms in there.

Sure. Now try to look at what that means for NV, where all the EL1->EL2 exceptions are emulated, where all the EL2->EL1 exception returns are emulated.

What does it give you? A bag of nonsense.

Same thing for EL2->EL0, by the way, so you can't even correctly profile an EL0 program that performs a syscall, or that gets interrupted. And while without NV, these exceptions are rare, having a trace that is unreliable has the potential of being worse than no trace at all.

Until the architecture grows a way for KVM to inject the missing information into the trace, TRBE support for guest will stay out.

Quoted 5 lines

All you need is the mmap records from the
guest which you can get by running Perf in the guest and it's possible to decode it. Maybe it's not complete but I don't think all use cases require complete trace. AutoFDO for example just needs lots of small snippets of execution history.

I don't think it is OK to feed an FDO with traces that are known to be incomplete. Maybe that goes under the radar today, but my crystal ball is telling me things could be very different in the future, and I'm not going to take any bet.

Thanks,

	M.

Quoted 150 lines · Marc Zyngier

Quoted 134 lines · James Clark

Quoted 130 lines · Marc Zyngier

Quoted 91 lines · James Clark

Quoted 87 lines · Marc Zyngier

Quoted 10 lines · Will Deacon

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.

Great. So TRBE violates all the principles that we hold true in the architecture. Does SPE suffer from the same level of brokenness?

Quoted 71 lines

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 result appears to be a heady mixture of 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.

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]>

Fixes a1319260bf62 ("arm64: KVM: Enable access to TRBE support for host")

Signed-off-by Will Deacon <[email protected]>

---

NOTE: This is *untested* as I don't have a TRBE-capable device that can run upstream but I noticed this by inspection when triaging occasional hardware lockups on systems using a 6.12-based kernel with TRBE running at the same time as a vCPU is loaded. This code has changed quite a bit over time, so stable backports are not entirely straightforward. Hopefully James/Leo/Suzuki can help us test if folks agree with the general approach taken here.

    arch/arm64/include/asm/kvm_host.h  |  1 +
    arch/arm64/kvm/hyp/nvhe/debug-sr.c | 36 ++++++++++++++++++++++--------
    2 files changed, 28 insertions(+), 9 deletions(-)

arch/arm64/include/asm/kvm_host.h +1 -0

index ac7f970c7883..a932cf043b83 100644
@@ -746,6 +746,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 +16 -2

index 2a1c0f49792b..fd389a26bc59 100644
@@ -57,12 +57,27 @@ static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr)
    	write_sysreg_el1(new_trfcr, SYS_TRFCR);
    }
    -static bool __trace_needs_drain(void)
+static void __trace_drain_and_disable(void)
    {
-	if (is_protected_kvm_enabled() && host_data_test_flag(HAS_TRBE))
-		return read_sysreg_s(SYS_TRBLIMITR_EL1) & TRBLIMITR_EL1_E;
+	u64 *trblimitr_el1 = host_data_ptr(host_debug_state.trblimitr_el1);
    -	return host_data_test_flag(TRBE_ENABLED);
+	*trblimitr_el1 = 0;
+
+	if (is_protected_kvm_enabled()) {
+		if (!host_data_test_flag(HAS_TRBE))
+			return;
+	} else {
+		if (!host_data_test_flag(TRBE_ENABLED))
+			return;
+	}
+
+	*trblimitr_el1 = read_sysreg_s(SYS_TRBLIMITR_EL1);
+	if (*trblimitr_el1 & TRBLIMITR_EL1_E) {
+		isb();
+		tsb_csync();

Quoted 8 lines

Quoted 2 lines

arch/arm64/kvm/hyp/nvhe/debug-sr.c static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr) · line 79

Quoted 2 lines

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

+		write_sysreg_s(0, SYS_TRBLIMITR_EL1);
+		isb();

The TRBE driver might do an extra drain here as a workaround. Hard to tell if it's actually required in this case (seems like probably not) but it might be worth doing it anyway to avoid hitting the issue. Especially if we add guest support later where some of the affected registers might start being used.

Just to set the expectations: guest TRBE support is not happening until the architecture is fixed. It cannot reliably give a trace that includes emulated exceptions, and until then, no TRBE for you.

Quoted 23 lines

See:

      if (trbe_needs_drain_after_disable(cpudata))
          trbe_drain_buffer();

Quoted 4 lines

arch/arm64/kvm/hyp/nvhe/debug-sr.c static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr) · line 80

Quoted 1 line

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

+	}

Doesn't this mean we should be able to get rid of most of the TRFCR messing about that litters the entry/exit code and leave that to VHE

Technically you could have ETMs that and are connected to sinks other than TRBE. Unless you somehow switch off those sinks you still need to do the TRFCR switching stuff.

Quoted 4 lines

only? And even then, I'm tempted to simply get rid of any sort of guest-only tracing, given that TRBE is not capable of representing exceptions that are synthesised by the host, making it the resulting traces useless.

I haven't heard of anyone tracing a guest from the host, but until we add support for guests to be able to trace themselves it's the only way of doing it, so it could be useful.

Quoted 7 lines

Quoted 3 lines

But that's *not* working. If you trace EL1 only, even with a VHE host, the result is not usable.

Do you mean not working because of the missing exceptions? I did a bit of testing before and the trace did seem somewhat usable to me. It had EL1 and EL0 atoms in there.

Sure. Now try to look at what that means for NV, where all the EL1->EL2 exceptions are emulated, where all the EL2->EL1 exception returns are emulated.

What does it give you? A bag of nonsense.

Quoted 5 lines

Same thing for EL2->EL0, by the way, so you can't even correctly profile an EL0 program that performs a syscall, or that gets interrupted. And while without NV, these exceptions are rare, having a trace that is unreliable has the potential of being worse than no trace at all.

If there are issues with NV perhaps we can skip it for the initial trace virtualisation implementation?

I'm not familiar with it but isn't NV still an experimental feature anyway?

I can't imagine actual users who want to do tracing in guests would accept that they can't do tracing on a non-NV guest because there is something that doesn't work in NV.

Also do you have an example of these exceptions that you mean without NV so I can have a look?

Quoted 7 lines

Quoted 3 lines

But that's *not* working. If you trace EL1 only, even with a VHE host, the result is not usable.

Do you mean not working because of the missing exceptions? I did a bit of testing before and the trace did seem somewhat usable to me. It had EL1 and EL0 atoms in there.

Sure. Now try to look at what that means for NV, where all the EL1->EL2 exceptions are emulated, where all the EL2->EL1 exception returns are emulated.

What does it give you? A bag of nonsense.

Same thing for EL2->EL0, by the way, so you can't even correctly profile an EL0 program that performs a syscall, or that gets interrupted. And while without NV, these exceptions are rare, having a trace that is unreliable has the potential of being worse than no trace at all.

Until the architecture grows a way for KVM to inject the missing information into the trace, TRBE support for guest will stay out.

On Mon, Feb 16, 2026 at 04:49:02PM +0000, Marc Zyngier wrote:

[...]

Quoted 12 lines

Quoted 7 lines

Quoted 3 lines

But that's *not* working. If you trace EL1 only, even with a VHE host, the result is not usable.

Do you mean not working because of the missing exceptions? I did a bit of testing before and the trace did seem somewhat usable to me. It had EL1 and EL0 atoms in there.

Sure. Now try to look at what that means for NV, where all the EL1->EL2 exceptions are emulated, where all the EL2->EL1 exception returns are emulated.

Quoted 1 line

What does it give you? A bag of nonsense.

Sorry for jumping in.

If we enable TRBE in a VM, whether nested or not, why is it necessary to capture trace data for the exception transition between the VM and its higher level host(s)?

Seems to me, regardless of what happens during exception emulation, once the VM is switched out, tracing will be stopped, and then re-enabled when the VM is switched back. In that case, we should be able to record the complete trace data for whatever occurs while the VM is running.

Quoted 24 lines · Marc Zyngier

Quoted 22 lines · Will Deacon

arch/arm64/kvm/hyp/nvhe/debug-sr.c +10 -4

index 2a1c0f49792b..fd389a26bc59 100644
@@ -57,12 +57,27 @@ static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr)
  	write_sysreg_el1(new_trfcr, SYS_TRFCR);
  }
-static bool __trace_needs_drain(void)
+static void __trace_drain_and_disable(void)
  {
-	if (is_protected_kvm_enabled() && host_data_test_flag(HAS_TRBE))
-		return read_sysreg_s(SYS_TRBLIMITR_EL1) & TRBLIMITR_EL1_E;
+	u64 *trblimitr_el1 = host_data_ptr(host_debug_state.trblimitr_el1);
-	return host_data_test_flag(TRBE_ENABLED);
+	*trblimitr_el1 = 0;
+
+	if (is_protected_kvm_enabled()) {
+		if (!host_data_test_flag(HAS_TRBE))
+			return;
+	} else {
+		if (!host_data_test_flag(TRBE_ENABLED))
+			return;

Quoted 8 lines

arch/arm64/kvm/hyp/nvhe/debug-sr.c static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr) · line 79

Quoted 8 lines

arch/arm64/kvm/hyp/nvhe/debug-sr.c +8 -0

+	}
+
+	*trblimitr_el1 = read_sysreg_s(SYS_TRBLIMITR_EL1);
+	if (*trblimitr_el1 & TRBLIMITR_EL1_E) {
+		isb();
+		tsb_csync();
+		write_sysreg_s(0, SYS_TRBLIMITR_EL1);
+		isb();

The TRBE driver might do an extra drain here as a workaround. Hard to tell if it's actually required in this case (seems like probably not) but it might be worth doing it anyway to avoid hitting the issue. Especially if we add guest support later where some of the affected registers might start being used. See:

    if (trbe_needs_drain_after_disable(cpudata))
        trbe_drain_buffer();

Quoted 8 lines

The TRBE driver might do an extra drain here as a workaround. Hard to tell if it's actually required in this case (seems like probably not) but it might be worth doing it anyway to avoid hitting the issue. Especially if we add guest support later where some of the affected registers might start being used. See:

    if (trbe_needs_drain_after_disable(cpudata))
        trbe_drain_buffer();

Oh great, this thing sucks even more than I realised!

But thanks for pointing that out... this is presumably erratum #2064142, but we probably need to look at #2038923 as well :/

I can't find any public documentation for the problems, but based on the kconfig text then I think we care about #2064142 so that the TRBE register writes when restoring the host context are effective and we care about #2038923 to avoid corrupting trace when re-enabling for the host.

It also looks like we can't rely on the dsb(nsh) in the vcpu_run() path if that needs to be before the write to TRBLIMITR_EL1.

In which case, the host->guest something hideous like:

	isb();
	tsb_csync();	// Executes twice if ARM64_WORKAROUND_TSB_FLUSH_FAILURE!
	dsb(nsh);	// I missed this in my patch
	write_sysreg_s(0, SYS_TRBLIMITR_EL1);
	if (2064142) {
		tsb_csync();
		dsb(nsh);
	}
	isb();

and then the guest->host part is:

	write_sysreg_s(trblimitr_el1, SYS_TRBLIMITR_EL1);
	isb();
	if (2038923)
		isb();

Does that look right to you?

On Mon, Feb 16, 2026 at 06:14:11PM +0000, Will Deacon wrote:

[...]

Quoted 11 lines

Quoted 8 lines

The TRBE driver might do an extra drain here as a workaround. Hard to tell if it's actually required in this case (seems like probably not) but it might be worth doing it anyway to avoid hitting the issue. Especially if we add guest support later where some of the affected registers might start being used. See:

    if (trbe_needs_drain_after_disable(cpudata))
        trbe_drain_buffer();

Oh great, this thing sucks even more than I realised!

Quoted 8 lines

But thanks for pointing that out... this is presumably erratum #2064142, but we probably need to look at #2038923 as well :/

I can't find any public documentation for the problems, but based on the kconfig text then I think we care about #2064142 so that the TRBE register writes when restoring the host context are effective and we care about #2038923 to avoid corrupting trace when re-enabling for the host.

Seems to me, this is correct.

Quoted 5 lines

It also looks like we can't rely on the dsb(nsh) in the vcpu_run() path if that needs to be before the write to TRBLIMITR_EL1.

In which case, the host->guest something hideous like:

Quoted 9 lines

	isb();
	tsb_csync();	// Executes twice if ARM64_WORKAROUND_TSB_FLUSH_FAILURE!
	dsb(nsh);	// I missed this in my patch
	write_sysreg_s(0, SYS_TRBLIMITR_EL1);
	if (2064142) {
		tsb_csync();
		dsb(nsh);
	}
	isb();

As I_QXJZX suggests, the section K10.5.10 "Context switching" gives the flow. I'd suggest the VM context switch is also aligned to the description in S_VKHHY.

When switching from host to guest, we need to clear TRCPRGCTLR.EN to zero. As the doc states "ETE trace compression logic is stateful, and disabling the ETE resets this compression state".

Quoted 2 lines

and then the guest->host part is:

Quoted 6 lines

	write_sysreg_s(trblimitr_el1, SYS_TRBLIMITR_EL1);
	isb();
	if (2038923)
		isb();

Does that look right to you?

S_PKLXF gives the flow for switching in.

Quoted 15 lines

Quoted 14 lines

It also looks like we can't rely on the dsb(nsh) in the vcpu_run() path if that needs to be before the write to TRBLIMITR_EL1.

In which case, the host->guest something hideous like:

	isb();
	tsb_csync();	// Executes twice if ARM64_WORKAROUND_TSB_FLUSH_FAILURE!
	dsb(nsh);	// I missed this in my patch
	write_sysreg_s(0, SYS_TRBLIMITR_EL1);
	if (2064142) {
		tsb_csync();
		dsb(nsh);
	}
	isb();

Quoted 3 lines

As I_QXJZX suggests, the section K10.5.10 "Context switching" gives the flow. I'd suggest the VM context switch is also aligned to the description in S_VKHHY.

I honestly have a hard time believing the sequence in S_VKHHY as the DSB seems to be in the wrong place which means the TSB CSYNC can float. It also isn't aligned with what the EL1 driver does...

Quoted 11 lines

When switching from host to guest, we need to clear TRCPRGCTLR.EN to zero. As the doc states "ETE trace compression logic is stateful, and disabling the ETE resets this compression state".

Quoted 7 lines

and then the guest->host part is:

	write_sysreg_s(trblimitr_el1, SYS_TRBLIMITR_EL1);
	isb();
	if (2038923)
		isb();

Quoted 3 lines

Quoted 1 line

Does that look right to you?

S_PKLXF gives the flow for switching in.

Well, modulo errata, sure. I don't have access to the errata document so I was more interested in whether I got that right...

On Tue, Feb 17, 2026 at 02:52:32PM +0000, Will Deacon wrote:

[...]

Quoted 15 lines

Quoted 15 lines

Quoted 14 lines

It also looks like we can't rely on the dsb(nsh) in the vcpu_run() path if that needs to be before the write to TRBLIMITR_EL1.

In which case, the host->guest something hideous like:

	isb();
	tsb_csync();	// Executes twice if ARM64_WORKAROUND_TSB_FLUSH_FAILURE!
	dsb(nsh);	// I missed this in my patch
	write_sysreg_s(0, SYS_TRBLIMITR_EL1);
	if (2064142) {
		tsb_csync();
		dsb(nsh);
	}
	isb();

Quoted 7 lines

Quoted 3 lines

As I_QXJZX suggests, the section K10.5.10 "Context switching" gives the flow. I'd suggest the VM context switch is also aligned to the description in S_VKHHY.

I honestly have a hard time believing the sequence in S_VKHHY as the DSB seems to be in the wrong place which means the TSB CSYNC can float. It also isn't aligned with what the EL1 driver does...

Sorry for confusion. I am checking internally for the flow suggested in S_VKHHY.

Quoted 13 lines

Quoted 13 lines

When switching from host to guest, we need to clear TRCPRGCTLR.EN to zero. As the doc states "ETE trace compression logic is stateful, and disabling the ETE resets this compression state".

Quoted 8 lines

and then the guest->host part is:

	write_sysreg_s(trblimitr_el1, SYS_TRBLIMITR_EL1);
	isb();
	if (2038923)
		isb();

Does that look right to you?

Quoted 4 lines

Quoted 1 line

S_PKLXF gives the flow for switching in.

Well, modulo errata, sure. I don't have access to the errata document so I was more interested in whether I got that right...

Please see the doc:
https://developer.arm.com/documentation/SDEN-1873351/1900/?lang=en

Quoted 16 lines

Quoted 16 lines

Quoted 15 lines

Quoted 11 lines

In which case, the host->guest something hideous like:

	isb();
	tsb_csync();	// Executes twice if ARM64_WORKAROUND_TSB_FLUSH_FAILURE!
	dsb(nsh);	// I missed this in my patch
	write_sysreg_s(0, SYS_TRBLIMITR_EL1);
	if (2064142) {
		tsb_csync();
		dsb(nsh);
	}
	isb();

As I_QXJZX suggests, the section K10.5.10 "Context switching" gives the flow. I'd suggest the VM context switch is also aligned to the description in S_VKHHY.

Quoted 6 lines

Quoted 3 lines

I honestly have a hard time believing the sequence in S_VKHHY as the DSB seems to be in the wrong place which means the TSB CSYNC can float. It also isn't aligned with what the EL1 driver does...

Sorry for confusion. I am checking internally for the flow suggested in S_VKHHY.

Quoted 15 lines

Quoted 15 lines

Quoted 14 lines

When switching from host to guest, we need to clear TRCPRGCTLR.EN to zero. As the doc states "ETE trace compression logic is stateful, and disabling the ETE resets this compression state".

Quoted 8 lines

and then the guest->host part is:

	write_sysreg_s(trblimitr_el1, SYS_TRBLIMITR_EL1);
	isb();
	if (2038923)
		isb();

Does that look right to you?

S_PKLXF gives the flow for switching in.

Quoted 5 lines

Quoted 2 lines

Well, modulo errata, sure. I don't have access to the errata document so I was more interested in whether I got that right...

Please see the doc:
https://developer.arm.com/documentation/SDEN-1873351/1900/?lang=en

Aha, thank you, Leo!

I swear you used to be able to google the erratum number and get the doc, but that doesn't seem to be the case any more. In fact, if you type the erratum number into the search box on developer.arm.com it doesn't even work, so cheers for pointing me to the right place.

The flow for guest->host might be easy one, anyway, I try to summary for review:

Restore TRFCR_EL1;

// Ensure restored sysreg is visible
isb();

// Enable trace buffer
TRBLIMITR_EL1.E = 0b1

// Enable trace unit
TRCPRGCTLR.EN = 0b1

  if (2038923)
     isb();

Based on these conclusions, let me summarize the flow:

  // Prohibit trace
  TRFCR_EL1 = 0;

  // No new program-flow trace
  isb();

// Trace operation and trace unit are flushed
tsb_csync(); // Executes twice if ARM64_WORKAROUND_TSB_FLUSH_FAILURE!

// Disable trace unit
TRCPRGCTLR.EN = 0b0

[ + Yabin ]

Thanks for Suzuki's reminding, I should mention that Yabin reported another lockup issue caused by missing CPU PM support in TRBE driver.

We have a patch series to fix the issue:
https://lore.kernel.org/linux-arm-kernel/20251119-arm_coresight_path_power_management_improvement-v5-16-f615a301ad0b@arm.com/

Besides your fix the translation regime issue, I'd also suggest applying the CoreSight PM patch series to fix lockup caused by CPU idle.

Quoted 5 lines · Leo Yan

[ + Yabin ]

Thanks for Suzuki's reminding, I should mention that Yabin reported another lockup issue caused by missing CPU PM support in TRBE driver.

Quoted 2 lines

We have a patch series to fix the issue:
https://lore.kernel.org/linux-arm-kernel/20251119-arm_coresight_path_power_management_improvement-v5-16-f615a301ad0b@arm.com/

Two nits on that series:

1. It seems a bit weird to me for the ETE driver to manage TRFCR but for the TRBE driver to manage the other registers

2. Are you sure you don't need to save/restore the TRBE state when LIMITR.E is clear? Maybe the driver is fine with that, but I'm worried that we could suspend in a half-programmed state and lose some of that configuration.

Quoted 2 lines

Besides your fix the translation regime issue, I'd also suggest applying the CoreSight PM patch series to fix lockup caused by CPU idle.

Yes, we definitely need something like that in the android kernel trees. I've previously bodged a hack into the ETE PM notifiers, but if you have backports of your series to 6.12, 6.6 and 6.1 then we should merge them into Android. As it stands, I don't have a TRBE-capable device running mainline.

Quoted 11 lines · Will Deacon

Quoted 7 lines · Leo Yan

[ + Yabin ]

Thanks for Suzuki's reminding, I should mention that Yabin reported another lockup issue caused by missing CPU PM support in TRBE driver.

We have a patch series to fix the issue:
https://lore.kernel.org/linux-arm-kernel/20251119-arm_coresight_path_power_management_improvement-v5-16-f615a301ad0b@arm.com/

Two nits on that series:

Quoted 2 lines

1. It seems a bit weird to me for the ETE driver to manage TRFCR but for the TRBE driver to manage the other registers

TRFCR_ELx is introduced by FEAT_TRF, which is a separate feature from TRBE, and it can be used for other sinks (like ETR). I think this is the main reason that it is implemented in ETE driver rather than TRBE driver.

Quoted 5 lines

2. Are you sure you don't need to save/restore the TRBE state when LIMITR.E is clear? Maybe the driver is fine with that, but I'm worried that we could suspend in a half-programmed state and lose some of that configuration.

If the TRBLIMITR_EL1.E bit is cleared during the CPU is idle, then the next time the TRBE trace buffer is re-enabled, trbe_enable_hw() must be called to reconfigure the TRBE registers (including TRBSR_EL1).

One concern is that after a CPU power cycle, some fields in the TRBE registers may be in the following state:

"On a cold reset, this field resets to an architecturally UNKNOWN value."

I will change to always save/restore TRBE state. Thanks for suggestions.

Quoted 8 lines

Quoted 2 lines

Besides your fix the translation regime issue, I'd also suggest applying the CoreSight PM patch series to fix lockup caused by CPU idle.

Yes, we definitely need something like that in the android kernel trees. I've previously bodged a hack into the ETE PM notifiers, but if you have backports of your series to 6.12, 6.6 and 6.1 then we should merge them into Android. As it stands, I don't have a TRBE-capable device running mainline.

Let us first merge the series on the master :)

After that, we can consider backporting (I assume Yabin already has a plan for this). I'd be happy to help with backporting to v6.12. However, I cannot commit to backporting to v6.6 or v6.1 at this stage, as many dependencies are likely to be involved.

Thanks,
Leo

Quoted 10 lines · Will Deacon

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.

Great. So TRBE violates all the principles that we hold true in the architecture. Does SPE suffer from the same level of brokenness?

Quoted 67 lines

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 result appears to be a heady mixture of 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.

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]>

Fixes a1319260bf62 ("arm64: KVM: Enable access to TRBE support for host")

Signed-off-by Will Deacon <[email protected]>

---

NOTE: This is *untested* as I don't have a TRBE-capable device that can run upstream but I noticed this by inspection when triaging occasional hardware lockups on systems using a 6.12-based kernel with TRBE running at the same time as a vCPU is loaded. This code has changed quite a bit over time, so stable backports are not entirely straightforward. Hopefully James/Leo/Suzuki can help us test if folks agree with the general approach taken here.

 arch/arm64/include/asm/kvm_host.h  |  1 +
 arch/arm64/kvm/hyp/nvhe/debug-sr.c | 36 ++++++++++++++++++++++--------
 2 files changed, 28 insertions(+), 9 deletions(-)

arch/arm64/include/asm/kvm_host.h +1 -0

index ac7f970c7883..a932cf043b83 100644
@@ -746,6 +746,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 +9 -4

index 2a1c0f49792b..fd389a26bc59 100644
@@ -57,12 +57,27 @@ static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr)
 	write_sysreg_el1(new_trfcr, SYS_TRFCR);
 }
 
-static bool __trace_needs_drain(void)
+static void __trace_drain_and_disable(void)
 {
-	if (is_protected_kvm_enabled() && host_data_test_flag(HAS_TRBE))
-		return read_sysreg_s(SYS_TRBLIMITR_EL1) & TRBLIMITR_EL1_E;
+	u64 *trblimitr_el1 = host_data_ptr(host_debug_state.trblimitr_el1);
 
-	return host_data_test_flag(TRBE_ENABLED);
+	*trblimitr_el1 = 0;
+
+	if (is_protected_kvm_enabled()) {
+		if (!host_data_test_flag(HAS_TRBE))
+			return;
+	} else {
+		if (!host_data_test_flag(TRBE_ENABLED))

arch/arm64/kvm/hyp/nvhe/debug-sr.c static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr) · line 80

Quoted 10 lines

arch/arm64/kvm/hyp/nvhe/debug-sr.c +10 -0

+			return;
+	}
+
+	*trblimitr_el1 = read_sysreg_s(SYS_TRBLIMITR_EL1);
+	if (*trblimitr_el1 & TRBLIMITR_EL1_E) {
+		isb();
+		tsb_csync();
+		write_sysreg_s(0, SYS_TRBLIMITR_EL1);
+		isb();
+	}

Doesn't this mean we should be able to get rid of most of the TRFCR messing about that litters the entry/exit code and leave that to VHE only? And even then, I'm tempted to simply get rid of any sort of guest-only tracing, given that TRBE is not capable of representing exceptions that are synthesised by the host, making it the resulting traces useless.

I'm still trying to get my hands on a TRBE-enabled system that has some actual firmware tables (my O6 seems to have the HW, but no description of the required coresight infra).

Thanks,

	M.

Signature · 1 line

Without deviation from the norm, progress is not possible.

Hi,

Quoted 13 lines · Marc Zyngier

Quoted 10 lines · Will Deacon

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.

Quoted 2 lines

Great. So TRBE violates all the principles that we hold true in the architecture. Does SPE suffer from the same level of brokenness?

I think not currently - I_JZRDG from DDI0487M.a.a says that after a PSB + DSB 'no new memory accesses using the lower Exception level translation table entries occur'.

But looks like the behaviour will be changed so that it will be similar to TRBE, according to the Arm known issues document [1], added in 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'.

Quoted 23 lines · Alexandru Elisei

Hi,

Quoted 13 lines · Marc Zyngier

Quoted 10 lines · Will Deacon

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.

Quoted 2 lines

Great. So TRBE violates all the principles that we hold true in the architecture. Does SPE suffer from the same level of brokenness?

I think not currently - I_JZRDG from DDI0487M.a.a says that after a PSB + DSB 'no new memory accesses using the lower Exception level translation table entries occur'.

Quoted 7 lines

But looks like the behaviour will be changed so that it will be similar to TRBE, according to the Arm known issues document [1], added in 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'.

I think SPE is ok, as __debug_save_spe() clears PMSCR_EL1 and (unlike TRBE) PMSCR_EL1.ExSPE _are_ factored into whether or not profiling is "enabled".

So there's a funny asymmetry between SPE and TRBE, which I assume is due to the coresight much associated with the latter.

Quoted 10 lines · Will Deacon

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.

Great. So TRBE violates all the principles that we hold true in the architecture. Does SPE suffer from the same level of brokenness?

Quoted 72 lines

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 result appears to be a heady mixture of 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.

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]>

Fixes a1319260bf62 ("arm64: KVM: Enable access to TRBE support for host")

Signed-off-by Will Deacon <[email protected]>

---

NOTE: This is *untested* as I don't have a TRBE-capable device that can run upstream but I noticed this by inspection when triaging occasional hardware lockups on systems using a 6.12-based kernel with TRBE running at the same time as a vCPU is loaded. This code has changed quite a bit over time, so stable backports are not entirely straightforward. Hopefully James/Leo/Suzuki can help us test if folks agree with the general approach taken here.

 arch/arm64/include/asm/kvm_host.h  |  1 +
 arch/arm64/kvm/hyp/nvhe/debug-sr.c | 36 ++++++++++++++++++++++--------
 2 files changed, 28 insertions(+), 9 deletions(-)

arch/arm64/include/asm/kvm_host.h +1 -0

index ac7f970c7883..a932cf043b83 100644
@@ -746,6 +746,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 +15 -4

index 2a1c0f49792b..fd389a26bc59 100644
@@ -57,12 +57,27 @@ static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr)
 	write_sysreg_el1(new_trfcr, SYS_TRFCR);
 }
 
-static bool __trace_needs_drain(void)
+static void __trace_drain_and_disable(void)
 {
-	if (is_protected_kvm_enabled() && host_data_test_flag(HAS_TRBE))
-		return read_sysreg_s(SYS_TRBLIMITR_EL1) & TRBLIMITR_EL1_E;
+	u64 *trblimitr_el1 = host_data_ptr(host_debug_state.trblimitr_el1);
 
-	return host_data_test_flag(TRBE_ENABLED);
+	*trblimitr_el1 = 0;
+
+	if (is_protected_kvm_enabled()) {
+		if (!host_data_test_flag(HAS_TRBE))
+			return;
+	} else {
+		if (!host_data_test_flag(TRBE_ENABLED))
+			return;
+	}
+
+	*trblimitr_el1 = read_sysreg_s(SYS_TRBLIMITR_EL1);
+	if (*trblimitr_el1 & TRBLIMITR_EL1_E) {
+		isb();

arch/arm64/kvm/hyp/nvhe/debug-sr.c static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr) · line 80

Quoted 4 lines

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

+		tsb_csync();
+		write_sysreg_s(0, SYS_TRBLIMITR_EL1);
+		isb();
+	}

Doesn't this mean we should be able to get rid of most of the TRFCR messing about that litters the entry/exit code and leave that to VHE only?

And even then, I'm tempted to simply get rid of any sort of guest-only tracing, given that TRBE is not capable of representing exceptions that are synthesised by the host, making it the resulting traces useless.

Quoted 88 lines · Marc Zyngier

Quoted 10 lines · Will Deacon

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.

Great. So TRBE violates all the principles that we hold true in the architecture. Does SPE suffer from the same level of brokenness?

Quoted 72 lines

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 result appears to be a heady mixture of 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.

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]>

Fixes a1319260bf62 ("arm64: KVM: Enable access to TRBE support for host")

Signed-off-by Will Deacon <[email protected]>

---

NOTE: This is *untested* as I don't have a TRBE-capable device that can run upstream but I noticed this by inspection when triaging occasional hardware lockups on systems using a 6.12-based kernel with TRBE running at the same time as a vCPU is loaded. This code has changed quite a bit over time, so stable backports are not entirely straightforward. Hopefully James/Leo/Suzuki can help us test if folks agree with the general approach taken here.

  arch/arm64/include/asm/kvm_host.h  |  1 +
  arch/arm64/kvm/hyp/nvhe/debug-sr.c | 36 ++++++++++++++++++++++--------
  2 files changed, 28 insertions(+), 9 deletions(-)

arch/arm64/include/asm/kvm_host.h +1 -0

index ac7f970c7883..a932cf043b83 100644
@@ -746,6 +746,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 +15 -4

index 2a1c0f49792b..fd389a26bc59 100644
@@ -57,12 +57,27 @@ static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr)
  	write_sysreg_el1(new_trfcr, SYS_TRFCR);
  }
 
-static bool __trace_needs_drain(void)
+static void __trace_drain_and_disable(void)
  {
-	if (is_protected_kvm_enabled() && host_data_test_flag(HAS_TRBE))
-		return read_sysreg_s(SYS_TRBLIMITR_EL1) & TRBLIMITR_EL1_E;
+	u64 *trblimitr_el1 = host_data_ptr(host_debug_state.trblimitr_el1);
 
-	return host_data_test_flag(TRBE_ENABLED);
+	*trblimitr_el1 = 0;
+
+	if (is_protected_kvm_enabled()) {
+		if (!host_data_test_flag(HAS_TRBE))
+			return;
+	} else {
+		if (!host_data_test_flag(TRBE_ENABLED))
+			return;
+	}
+
+	*trblimitr_el1 = read_sysreg_s(SYS_TRBLIMITR_EL1);
+	if (*trblimitr_el1 & TRBLIMITR_EL1_E) {
+		isb();

Quoted 8 lines

arch/arm64/kvm/hyp/nvhe/debug-sr.c static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr) · line 80

Quoted 4 lines

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

+		tsb_csync();
+		write_sysreg_s(0, SYS_TRBLIMITR_EL1);
+		isb();
+	}

Doesn't this mean we should be able to get rid of most of the TRFCR messing about that litters the entry/exit code and leave that to VHE only?

I'm not sure we can get rid of an awful lot: if the host is using TRBE, then we still need to stop trace generation, drain the buffer and disable the buffer. Or are you thinking of some other TRFCR accesses?

Looking at the TRBE driver, I _think_ the idea is that the trace hardware can generate trace to ETM/Coresight instead of memory in some cases and so you can enable it at boot time or via sysfs and then profile the whole machine, presumably using an expensive external box + cable or via some other coresight "sink" component. But I'm really guessing based on the driver; James and Leo will know for sure.

I've tried (and failed) to reconcile the above with what is written in the Arm ARM regarding self-hosted trace with TRBE.

Quoted 4 lines

And even then, I'm tempted to simply get rid of any sort of guest-only tracing, given that TRBE is not capable of representing exceptions that are synthesised by the host, making it the resulting traces useless.

I think that effectively means reverting the series merged from here:

https://lore.kernel.org/all/[email protected]/

but then we still need to clear TRBLIMITR_EL1.E.

Will

Quoted 8 lines · Will Deacon

Quoted 4 lines · Marc Zyngier

And even then, I'm tempted to simply get rid of any sort of guest-only tracing, given that TRBE is not capable of representing exceptions that are synthesised by the host, making it the resulting traces useless.

I think that effectively means reverting the series merged from here:

Quoted 4 lines

https://lore.kernel.org/all/[email protected]/

but then we still need to clear TRBLIMITR_EL1.E.

Removing that series would actually have the effect of turning guest trace on in nVHE for non-TRBE sinks. The reason for implementing the filtering was to turn guest trace off because a user didn't want to see it.

Quoted 9 lines · James Clark

Quoted 8 lines · Will Deacon

Quoted 4 lines · Marc Zyngier

And even then, I'm tempted to simply get rid of any sort of guest-only tracing, given that TRBE is not capable of representing exceptions that are synthesised by the host, making it the resulting traces useless.

I think that effectively means reverting the series merged from here:

Quoted 8 lines

Quoted 4 lines

https://lore.kernel.org/all/[email protected]/

but then we still need to clear TRBLIMITR_EL1.E.

Removing that series would actually have the effect of turning guest trace on in nVHE for non-TRBE sinks. The reason for implementing the filtering was to turn guest trace off because a user didn't want to see it.

What I meant was, revert that series and then also ensure that both TRFCR and TRBLIMITR are always zero while running in the guest. Is that not sufficient?

Will

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 result appears to be a heady mixture of 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.

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]>

Fixes a1319260bf62 ("arm64: KVM: Enable access to TRBE support for host")

Signed-off-by Will Deacon <[email protected]>

---

NOTE: This is *untested* as I don't have a TRBE-capable device that can run upstream but I noticed this by inspection when triaging occasional hardware lockups on systems using a 6.12-based kernel with TRBE running at the same time as a vCPU is loaded. This code has changed quite a bit over time, so stable backports are not entirely straightforward. Hopefully James/Leo/Suzuki can help us test if folks agree with the general approach taken here.

  arch/arm64/include/asm/kvm_host.h  |  1 +
  arch/arm64/kvm/hyp/nvhe/debug-sr.c | 36 ++++++++++++++++++++++--------
  2 files changed, 28 insertions(+), 9 deletions(-)

arch/arm64/include/asm/kvm_host.h +1 -0

index ac7f970c7883..a932cf043b83 100644
@@ -746,6 +746,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 +19 -8

index 2a1c0f49792b..fd389a26bc59 100644
@@ -57,12 +57,27 @@ static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr)
  	write_sysreg_el1(new_trfcr, SYS_TRFCR);
  }
 
-static bool __trace_needs_drain(void)
+static void __trace_drain_and_disable(void)
  {
-	if (is_protected_kvm_enabled() && host_data_test_flag(HAS_TRBE))
-		return read_sysreg_s(SYS_TRBLIMITR_EL1) & TRBLIMITR_EL1_E;
+	u64 *trblimitr_el1 = host_data_ptr(host_debug_state.trblimitr_el1);
 
-	return host_data_test_flag(TRBE_ENABLED);
+	*trblimitr_el1 = 0;
+
+	if (is_protected_kvm_enabled()) {
+		if (!host_data_test_flag(HAS_TRBE))
+			return;
+	} else {
+		if (!host_data_test_flag(TRBE_ENABLED))
+			return;
+	}
+
+	*trblimitr_el1 = read_sysreg_s(SYS_TRBLIMITR_EL1);
+	if (*trblimitr_el1 & TRBLIMITR_EL1_E) {
+		isb();
+		tsb_csync();
+		write_sysreg_s(0, SYS_TRBLIMITR_EL1);
+		isb();
+	}
  }
 
  static bool __trace_needs_switch(void)

@@ -79,15 +94,18 @@ static void __trace_switch_to_guest(void)
 
  	__trace_do_switch(host_data_ptr(host_debug_state.trfcr_el1),
  			  *host_data_ptr(trfcr_while_in_guest));
-
-	if (__trace_needs_drain()) {
-		isb();
-		tsb_csync();

arch/arm64/kvm/hyp/nvhe/debug-sr.c +3 -0

+		isb();
+	}
+
  	__trace_do_switch(host_data_ptr(trfcr_while_in_guest),
  			  *host_data_ptr(host_debug_state.trfcr_el1));
  }

Quoted 99 lines · Will Deacon

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 result appears to be a heady mixture of 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.

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]>

Fixes a1319260bf62 ("arm64: KVM: Enable access to TRBE support for host")

Signed-off-by Will Deacon <[email protected]>

---

NOTE: This is *untested* as I don't have a TRBE-capable device that can run upstream but I noticed this by inspection when triaging occasional hardware lockups on systems using a 6.12-based kernel with TRBE running at the same time as a vCPU is loaded. This code has changed quite a bit over time, so stable backports are not entirely straightforward. Hopefully James/Leo/Suzuki can help us test if folks agree with the general approach taken here.

  arch/arm64/include/asm/kvm_host.h  |  1 +
  arch/arm64/kvm/hyp/nvhe/debug-sr.c | 36 ++++++++++++++++++++++--------
  2 files changed, 28 insertions(+), 9 deletions(-)

arch/arm64/include/asm/kvm_host.h +1 -0

index ac7f970c7883..a932cf043b83 100644
@@ -746,6 +746,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 +21 -9

index 2a1c0f49792b..fd389a26bc59 100644
@@ -57,12 +57,27 @@ static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr)
  	write_sysreg_el1(new_trfcr, SYS_TRFCR);
  }
-static bool __trace_needs_drain(void)
+static void __trace_drain_and_disable(void)
  {
-	if (is_protected_kvm_enabled() && host_data_test_flag(HAS_TRBE))
-		return read_sysreg_s(SYS_TRBLIMITR_EL1) & TRBLIMITR_EL1_E;
+	u64 *trblimitr_el1 = host_data_ptr(host_debug_state.trblimitr_el1);
-	return host_data_test_flag(TRBE_ENABLED);
+	*trblimitr_el1 = 0;
+
+	if (is_protected_kvm_enabled()) {
+		if (!host_data_test_flag(HAS_TRBE))
+			return;
+	} else {
+		if (!host_data_test_flag(TRBE_ENABLED))
+			return;
+	}
+
+	*trblimitr_el1 = read_sysreg_s(SYS_TRBLIMITR_EL1);
+	if (*trblimitr_el1 & TRBLIMITR_EL1_E) {
+		isb();
+		tsb_csync();
+		write_sysreg_s(0, SYS_TRBLIMITR_EL1);
+		isb();
+	}
  }
  static bool __trace_needs_switch(void)

@@ -79,15 +94,18 @@ static void __trace_switch_to_guest(void)
  	__trace_do_switch(host_data_ptr(host_debug_state.trfcr_el1),
  			  *host_data_ptr(trfcr_while_in_guest));
-
-	if (__trace_needs_drain()) {
-		isb();
-		tsb_csync();
-	}
+	__trace_drain_and_disable();
  }
  static void __trace_switch_to_host(void)
  {
+	u64 trblimitr_el1 = *host_data_ptr(host_debug_state.trblimitr_el1);

arch/arm64/kvm/hyp/nvhe/debug-sr.c static void __trace_switch_to_guest(void) · line 105

Quoted 3 lines

arch/arm64/kvm/hyp/nvhe/debug-sr.c +3 -0

+
+	if (trblimitr_el1 & TRBLIMITR_EL1_E) {
+		write_sysreg_s(trblimitr_el1, SYS_TRBLIMITR_EL1);

Will this restore a stale value if you do kvm_enable_trbe() then later kvm_disable_trbe()? Looks like the read and save will be skipped unless host_data_test_flag(TRBE_ENABLED) is true, so it will never save a disabled value.

kvm_disable_trbe() might need to clear host_debug_state.trblimitr_el1.

Quoted 102 lines · James Clark

Quoted 99 lines · Will Deacon

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 result appears to be a heady mixture of 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.

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]>

Fixes a1319260bf62 ("arm64: KVM: Enable access to TRBE support for host")

Signed-off-by Will Deacon <[email protected]>

---

NOTE: This is *untested* as I don't have a TRBE-capable device that can run upstream but I noticed this by inspection when triaging occasional hardware lockups on systems using a 6.12-based kernel with TRBE running at the same time as a vCPU is loaded. This code has changed quite a bit over time, so stable backports are not entirely straightforward. Hopefully James/Leo/Suzuki can help us test if folks agree with the general approach taken here.

   arch/arm64/include/asm/kvm_host.h  |  1 +
   arch/arm64/kvm/hyp/nvhe/debug-sr.c | 36 ++++++++++++++++++++++--------
   2 files changed, 28 insertions(+), 9 deletions(-)

arch/arm64/include/asm/kvm_host.h +1 -0

index ac7f970c7883..a932cf043b83 100644
@@ -746,6 +746,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 +21 -9

index 2a1c0f49792b..fd389a26bc59 100644
@@ -57,12 +57,27 @@ static void __trace_do_switch(u64 *saved_trfcr, u64 new_trfcr)
   	write_sysreg_el1(new_trfcr, SYS_TRFCR);
   }
-static bool __trace_needs_drain(void)
+static void __trace_drain_and_disable(void)
   {
-	if (is_protected_kvm_enabled() && host_data_test_flag(HAS_TRBE))
-		return read_sysreg_s(SYS_TRBLIMITR_EL1) & TRBLIMITR_EL1_E;
+	u64 *trblimitr_el1 = host_data_ptr(host_debug_state.trblimitr_el1);
-	return host_data_test_flag(TRBE_ENABLED);
+	*trblimitr_el1 = 0;
+
+	if (is_protected_kvm_enabled()) {
+		if (!host_data_test_flag(HAS_TRBE))
+			return;
+	} else {
+		if (!host_data_test_flag(TRBE_ENABLED))
+			return;
+	}
+
+	*trblimitr_el1 = read_sysreg_s(SYS_TRBLIMITR_EL1);
+	if (*trblimitr_el1 & TRBLIMITR_EL1_E) {
+		isb();
+		tsb_csync();
+		write_sysreg_s(0, SYS_TRBLIMITR_EL1);
+		isb();
+	}
   }
   static bool __trace_needs_switch(void)

@@ -79,15 +94,18 @@ static void __trace_switch_to_guest(void)
   	__trace_do_switch(host_data_ptr(host_debug_state.trfcr_el1),
   			  *host_data_ptr(trfcr_while_in_guest));
-
-	if (__trace_needs_drain()) {
-		isb();
-		tsb_csync();
-	}
+	__trace_drain_and_disable();
   }
   static void __trace_switch_to_host(void)
   {
+	u64 trblimitr_el1 = *host_data_ptr(host_debug_state.trblimitr_el1);

Quoted 8 lines

arch/arm64/kvm/hyp/nvhe/debug-sr.c static void __trace_switch_to_guest(void) · line 105

Quoted 3 lines

arch/arm64/kvm/hyp/nvhe/debug-sr.c +3 -0

+
+	if (trblimitr_el1 & TRBLIMITR_EL1_E) {
+		write_sysreg_s(trblimitr_el1, SYS_TRBLIMITR_EL1);

Will this restore a stale value if you do kvm_enable_trbe() then later kvm_disable_trbe()? Looks like the read and save will be skipped unless host_data_test_flag(TRBE_ENABLED) is true, so it will never save a disabled value.

__trace_drain_and_disable() sets the saved limit to 0 if TRBE_ENABLE is not set, so this shouldn't do anything in that case. Or did I misunderstand your scenario?

Quoted 1 line

kvm_disable_trbe() might need to clear host_debug_state.trblimitr_el1.

pKVM can't rely on that thing being called, so the context switch still needs to be self-contained there.

Will