FAILED: patch "[PATCH] KVM: arm64: nvhe: Save the SPE context early" failed to apply to 4.19-stable tree

PATCH In review [email protected] 3 participants 3 messages Mar 15, 2021 – Mar 17, 2021

Subsystems (3)

Stable branch arm64 KVM arm64

lore

Share & tools

lore

[email protected]

author maintainer

2021-03-15 08:49 UTC #0 lore raw

The patch below does not apply to the 4.19-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From b96b0c5de685df82019e16826a282d53d86d112c Mon Sep 17 00:00:00 2001
From: Suzuki K Poulose <[email protected]>
Date: Fri, 5 Mar 2021 18:52:47 +0000
Subject: [PATCH] KVM: arm64: nvhe: Save the SPE context early

The nVHE KVM hyp drains and disables the SPE buffer, before entering the guest, as the EL1&0 translation regime is going to be loaded with that of the guest.

But this operation is performed way too late, because :
- The owning translation regime of the SPE buffer

    is transferred to EL2. (MDCR_EL2_E2PB == 0)

- The guest Stage1 is loaded.

Thus the flush could use the host EL1 virtual address, but use the EL2 translations instead of host EL1, for writing out any cached data.

Fix this by moving the SPE buffer handling early enough. The restore path is doing the right thing.

Fixes 014c4c77aad7 ("KVM: arm64: Improve debug register save/restore flow")

Cc [email protected]

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Will Deacon <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Alexandru Elisei <[email protected]>

Reviewed-by Alexandru Elisei <[email protected]>

Signed-off-by Suzuki K Poulose <[email protected]>

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

Link https://lore.kernel.org/r/[email protected] open here

Message-Id: <[email protected]>

Signed-off-by Paolo Bonzini <[email protected]>

arch/arm64/include/asm/kvm_hyp.h +5 -0

index c0450828378b..385bd7dd3d39 100644
@@ -83,6 +83,11 @@ void sysreg_restore_guest_state_vhe(struct kvm_cpu_context *ctxt);
 void __debug_switch_to_guest(struct kvm_vcpu *vcpu);
 void __debug_switch_to_host(struct kvm_vcpu *vcpu);
 
+#ifdef __KVM_NVHE_HYPERVISOR__
+void __debug_save_host_buffers_nvhe(struct kvm_vcpu *vcpu);
+void __debug_restore_host_buffers_nvhe(struct kvm_vcpu *vcpu);
+#endif
+
 void __fpsimd_save_state(struct user_fpsimd_state *fp_regs);
 void __fpsimd_restore_state(struct user_fpsimd_state *fp_regs);

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

index 91a711aa8382..f401724f12ef 100644
@@ -58,16 +58,24 @@ static void __debug_restore_spe(u64 pmscr_el1)
 	write_sysreg_s(pmscr_el1, SYS_PMSCR_EL1);
 }
 
-void __debug_switch_to_guest(struct kvm_vcpu *vcpu)
+void __debug_save_host_buffers_nvhe(struct kvm_vcpu *vcpu)
 {
 	/* Disable and flush SPE data generation */
 	__debug_save_spe(&vcpu->arch.host_debug_state.pmscr_el1);
+}
+
+void __debug_switch_to_guest(struct kvm_vcpu *vcpu)
+{
 	__debug_switch_to_guest_common(vcpu);
 }
 
-void __debug_switch_to_host(struct kvm_vcpu *vcpu)
+void __debug_restore_host_buffers_nvhe(struct kvm_vcpu *vcpu)
 {
 	__debug_restore_spe(vcpu->arch.host_debug_state.pmscr_el1);
+}
+
+void __debug_switch_to_host(struct kvm_vcpu *vcpu)
+{
 	__debug_switch_to_host_common(vcpu);
 }

arch/arm64/kvm/hyp/nvhe/switch.c +10 -1

index f3d0e9eca56c..59aa1045fdaf 100644
@@ -192,6 +192,14 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 	pmu_switch_needed = __pmu_switch_to_guest(host_ctxt);
 
 	__sysreg_save_state_nvhe(host_ctxt);
+	/*
+	 * We must flush and disable the SPE buffer for nVHE, as
+	 * the translation regime(EL1&0) is going to be loaded with
+	 * that of the guest. And we must do this before we change the
+	 * translation regime to EL2 (via MDCR_EL2_E2PB == 0) and
+	 * before we load guest Stage1.
+	 */
+	__debug_save_host_buffers_nvhe(vcpu);
 
 	__adjust_pc(vcpu);

@@ -234,11 +242,12 @@ int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 	if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED)
 		__fpsimd_save_fpexc32(vcpu);
 
+	__debug_switch_to_host(vcpu);
 	/*
 	 * This must come after restoring the host sysregs, since a non-VHE
 	 * system may enable SPE here and make use of the TTBRs.
 	 */
-	__debug_switch_to_host(vcpu);
+	__debug_restore_host_buffers_nvhe(vcpu);
 
 	if (pmu_switch_needed)
 		__pmu_switch_to_host(host_ctxt);

Suzuki K Poulose

reviewer

2021-03-17 11:51 UTC #1 lore raw

PATCH 1/1 KVM: arm64: nvhe: Save the SPE context early

commit b96b0c5de685df82019e16826a282d53d86d112c upstream

The nVHE KVM hyp drains and disables the SPE buffer, before entering the guest, as the EL1&0 translation regime is going to be loaded with that of the guest.

But this operation is performed way too late, because :
- The owning translation regime of the SPE buffer is transferred to EL2. (MDCR_EL2_E2PB == 0)
- The guest Stage1 is loaded.

Thus the flush could use the host EL1 virtual address, but use the EL2 translations instead of host EL1, for writing out any cached data.

Fix this by moving the SPE buffer handling early enough. The restore path is doing the right thing.

Cc [email protected] # v4.19

Cc Christoffer Dall <[email protected]>

Cc Marc Zyngier <[email protected]>

Cc Will Deacon <[email protected]>

Cc Catalin Marinas <[email protected]>

Cc Mark Rutland <[email protected]>

Cc Alexandru Elisei <[email protected]>

Signed-off-by Suzuki K Poulose <[email protected]>

---
 arch/arm64/include/asm/kvm_hyp.h |  3 +++
 arch/arm64/kvm/hyp/debug-sr.c    | 24 +++++++++++++++---------
 arch/arm64/kvm/hyp/switch.c      |  4 +++-
 3 files changed, 21 insertions(+), 10 deletions(-)

arch/arm64/include/asm/kvm_hyp.h +3 -0

index 384c34397619..5f52d6d670e9 100644
@@ -144,6 +144,9 @@ void __sysreg32_restore_state(struct kvm_vcpu *vcpu);
 
 void __debug_switch_to_guest(struct kvm_vcpu *vcpu);
 void __debug_switch_to_host(struct kvm_vcpu *vcpu);
+void __debug_save_host_buffers_nvhe(struct kvm_vcpu *vcpu);
+void __debug_restore_host_buffers_nvhe(struct kvm_vcpu *vcpu);
+
 
 void __fpsimd_save_state(struct user_fpsimd_state *fp_regs);
 void __fpsimd_restore_state(struct user_fpsimd_state *fp_regs);

arch/arm64/kvm/hyp/debug-sr.c +15 -9

index 50009766e5e5..3c5414633bb7 100644
@@ -149,6 +149,21 @@ static void __hyp_text __debug_restore_state(struct kvm_vcpu *vcpu,
 	write_sysreg(ctxt->sys_regs[MDCCINT_EL1], mdccint_el1);
 }
 
+void __hyp_text __debug_save_host_buffers_nvhe(struct kvm_vcpu *vcpu)
+{
+	/*
+	 * Non-VHE: Disable and flush SPE data generation
+	 * VHE: The vcpu can run, but it can't hide.
+	 */
+	__debug_save_spe_nvhe(&vcpu->arch.host_debug_state.pmscr_el1);
+
+}
+
+void __hyp_text __debug_restore_host_buffers_nvhe(struct kvm_vcpu *vcpu)
+{
+	__debug_restore_spe_nvhe(vcpu->arch.host_debug_state.pmscr_el1);
+}
+
 void __hyp_text __debug_switch_to_guest(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpu_context *host_ctxt;

@@ -156,13 +171,6 @@ void __hyp_text __debug_switch_to_guest(struct kvm_vcpu *vcpu)
 	struct kvm_guest_debug_arch *host_dbg;
 	struct kvm_guest_debug_arch *guest_dbg;
 
-	/*
-	 * Non-VHE: Disable and flush SPE data generation
-	 * VHE: The vcpu can run, but it can't hide.
-	 */
-	if (!has_vhe())
-		__debug_save_spe_nvhe(&vcpu->arch.host_debug_state.pmscr_el1);
-
 	if (!(vcpu->arch.flags & KVM_ARM64_DEBUG_DIRTY))
 		return;

@@ -182,8 +190,6 @@ void __hyp_text __debug_switch_to_host(struct kvm_vcpu *vcpu)
 	struct kvm_guest_debug_arch *host_dbg;
 	struct kvm_guest_debug_arch *guest_dbg;
 
-	if (!has_vhe())
-		__debug_restore_spe_nvhe(vcpu->arch.host_debug_state.pmscr_el1);
 
 	if (!(vcpu->arch.flags & KVM_ARM64_DEBUG_DIRTY))
 		return;

arch/arm64/kvm/hyp/switch.c +3 -1

index 15312e429b7d..1d16ce0b7e0d 100644
@@ -560,6 +560,7 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
 	guest_ctxt = &vcpu->arch.ctxt;
 
 	__sysreg_save_state_nvhe(host_ctxt);
+	__debug_save_host_buffers_nvhe(vcpu);
 
 	__activate_traps(vcpu);
 	__activate_vm(kern_hyp_va(vcpu->kvm));

@@ -599,11 +600,12 @@ int __hyp_text __kvm_vcpu_run_nvhe(struct kvm_vcpu *vcpu)
 	if (vcpu->arch.flags & KVM_ARM64_FP_ENABLED)
 		__fpsimd_save_fpexc32(vcpu);
 
+	__debug_switch_to_host(vcpu);
 	/*
 	 * This must come after restoring the host sysregs, since a non-VHE
 	 * system may enable SPE here and make use of the TTBRs.
 	 */
-	__debug_switch_to_host(vcpu);
+	__debug_restore_host_buffers_nvhe(vcpu);
 
 	return exit_code;
 }

Signature · 1 line

2.24.1

Marc Zyngier

maintainer Acked-by on v1 1/1

2021-03-17 13:59 UTC #2 lore raw

Re: [PATCH] KVM: arm64: nvhe: Save the SPE context early

Quoted 28 lines · Suzuki K Poulose

commit b96b0c5de685df82019e16826a282d53d86d112c upstream
The nVHE KVM hyp drains and disables the SPE buffer, before entering the guest, as the EL1&0 translation regime is going to be loaded with that of the guest.
But this operation is performed way too late, because :
- The owning translation regime of the SPE buffer is transferred to EL2. (MDCR_EL2_E2PB == 0)
- The guest Stage1 is loaded.
Thus the flush could use the host EL1 virtual address, but use the EL2 translations instead of host EL1, for writing out any cached data.
Fix this by moving the SPE buffer handling early enough. The restore path is doing the right thing.
Cc [email protected] # v4.19
Cc Christoffer Dall <[email protected]>
Cc Marc Zyngier <[email protected]>
Cc Will Deacon <[email protected]>
Cc Catalin Marinas <[email protected]>
Cc Mark Rutland <[email protected]>
Cc Alexandru Elisei <[email protected]>
Signed-off-by Suzuki K Poulose <[email protected]>

Quoted 1 line

Signed-off-by Suzuki K Poulose <[email protected]>

Acked-by Marc Zyngier <[email protected]>

Thanks,

M.

Signature · 1 line

Without deviation from the norm, progress is not possible.