Linux KVM/arm64 development list
 help / color / mirror / Atom feed
* [PATCH 0/3] KVM: arm64: nv: Fix nested IRQ exception storm on NV+GICv4
@ 2024-08-23 21:27 Oliver Upton
  2024-08-23 21:27 ` [PATCH 1/3] KVM: arm64: nv: Leave vPE nonresident in nested vgic state Oliver Upton
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Oliver Upton @ 2024-08-23 21:27 UTC (permalink / raw)
  To: kvmarm
  Cc: Marc Zyngier, James Morse, Suzuki K Poulose, Zenghui Yu,
	Ganapatrao Kulkarni, Oliver Upton

Hi,

There's a rather annoying collision between nested virt and GICv4 that
leads to a storm of nested excpetions taken to the L1, meaning the L2
can never get off the ground.

Here's a small series to address the immediate issue + ensure IRQs
destined for the L1 aren't erroneously masked by running an L2.

Marc, I've deliberately based this on top of upstream rather than the NV
series. I did this to avoid stacking more crap on top of the out of tree
series. The fact we don't virtualize GICv4 for the L1 isn't really
up for discussion right now, so taking that limitation ahead of full
nested VGIC support seems OK.

Note that I spun off a small portion of the nested VGIC patch which avoids
making the vPE resident in nested state to give the full picture of how
the vPE is managed while nested.

Oliver Upton (3):
  KVM: arm64: nv: Leave vPE nonresident in nested vgic state
  KVM: arm64: Move host SVE/SME state flags out of vCPU
  KVM: arm64: nv: Request vPE doorbell upon nested ERET to L2

 arch/arm64/include/asm/kvm_host.h | 28 ++++++++++++++++++----------
 arch/arm64/kvm/emulate-nested.c   |  2 ++
 arch/arm64/kvm/fpsimd.c           | 12 ++++++------
 arch/arm64/kvm/vgic/vgic-v3.c     | 15 ++++++++++++++-
 arch/arm64/kvm/vgic/vgic-v4.c     | 18 +++++++++++++++++-
 include/kvm/arm_vgic.h            |  5 +++++
 6 files changed, 62 insertions(+), 18 deletions(-)


base-commit: 7c626ce4bae1ac14f60076d00eafe71af30450ba
-- 
2.46.0.295.g3b9ea8a38a-goog


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/3] KVM: arm64: nv: Leave vPE nonresident in nested vgic state
  2024-08-23 21:27 [PATCH 0/3] KVM: arm64: nv: Fix nested IRQ exception storm on NV+GICv4 Oliver Upton
@ 2024-08-23 21:27 ` Oliver Upton
  2024-08-23 21:27 ` [PATCH 2/3] KVM: arm64: Move host SVE/SME state flags out of vCPU Oliver Upton
  2024-08-23 21:27 ` [PATCH 3/3] KVM: arm64: nv: Request vPE doorbell upon nested ERET to L2 Oliver Upton
  2 siblings, 0 replies; 4+ messages in thread
From: Oliver Upton @ 2024-08-23 21:27 UTC (permalink / raw)
  To: kvmarm
  Cc: Marc Zyngier, James Morse, Suzuki K Poulose, Zenghui Yu,
	Ganapatrao Kulkarni, Oliver Upton

Nested VGIC support hasn't taken shape upstream yet, although one thing
that's already clear is KVM will not virtualize GICv4 for the guest
hypervisor. Nope. Nada. Maybe we can revisit the topic for the next GIC
architecture.

Leave the vPE unloaded in nested state and document why we do it. This
has been spun off from the nested VGIC patch that Marc has been carrying
as part of the NV series.

Co-developed-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/vgic/vgic-v3.c | 15 ++++++++++++++-
 include/kvm/arm_vgic.h        |  5 +++++
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/vgic/vgic-v3.c b/arch/arm64/kvm/vgic/vgic-v3.c
index ed6e412cd74b..27b42664bb1c 100644
--- a/arch/arm64/kvm/vgic/vgic-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-v3.c
@@ -727,7 +727,14 @@ void vgic_v3_load(struct kvm_vcpu *vcpu)
 	if (has_vhe())
 		__vgic_v3_activate_traps(cpu_if);
 
-	WARN_ON(vgic_v4_load(vcpu));
+	/*
+	 * KVM does not virtualize GICv4 for the guest hypervisor, so there's no
+	 * vPE to load when in a nested state. The L1 vPE remains nonresident
+	 * so the GIC will generate a doorbell when a vLPI/vSGI becomes pending
+	 * for the L1.
+	 */
+	if (!vgic_is_nested_state(vcpu))
+		WARN_ON(vgic_v4_load(vcpu));
 }
 
 void vgic_v3_put(struct kvm_vcpu *vcpu)
@@ -735,6 +742,12 @@ void vgic_v3_put(struct kvm_vcpu *vcpu)
 	struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3;
 
 	kvm_call_hyp(__vgic_v3_save_vmcr_aprs, cpu_if);
+
+	/*
+	 * The vPE may already be nonresident if we're blocking (i.e. already
+	 * called vgic_v4_put()) or in a nested state. Calling vgic_v4_put() on
+	 * an already nonresdent vCPU is benign.
+	 */
 	WARN_ON(vgic_v4_put(vcpu));
 
 	if (has_vhe())
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index f5172549f9ba..0b1b7b706227 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -434,6 +434,11 @@ int vgic_v4_load(struct kvm_vcpu *vcpu);
 void vgic_v4_commit(struct kvm_vcpu *vcpu);
 int vgic_v4_put(struct kvm_vcpu *vcpu);
 
+static inline bool vgic_is_nested_state(struct kvm_vcpu *vcpu)
+{
+	return false;
+}
+
 /* CPU HP callbacks */
 void kvm_vgic_cpu_up(void);
 void kvm_vgic_cpu_down(void);
-- 
2.46.0.295.g3b9ea8a38a-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/3] KVM: arm64: Move host SVE/SME state flags out of vCPU
  2024-08-23 21:27 [PATCH 0/3] KVM: arm64: nv: Fix nested IRQ exception storm on NV+GICv4 Oliver Upton
  2024-08-23 21:27 ` [PATCH 1/3] KVM: arm64: nv: Leave vPE nonresident in nested vgic state Oliver Upton
@ 2024-08-23 21:27 ` Oliver Upton
  2024-08-23 21:27 ` [PATCH 3/3] KVM: arm64: nv: Request vPE doorbell upon nested ERET to L2 Oliver Upton
  2 siblings, 0 replies; 4+ messages in thread
From: Oliver Upton @ 2024-08-23 21:27 UTC (permalink / raw)
  To: kvmarm
  Cc: Marc Zyngier, James Morse, Suzuki K Poulose, Zenghui Yu,
	Ganapatrao Kulkarni, Oliver Upton

We're running a bit tight on vCPU state flags, and there's no good
reason for tracking bits of host state in the vCPU. Rather than do the
unspeakable and add another byte of sflags, move some of the obvious
host bits over to kvm_host_data where they really belong.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/include/asm/kvm_host.h | 26 ++++++++++++++++----------
 arch/arm64/kvm/fpsimd.c           | 12 ++++++------
 2 files changed, 22 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index a33f5996ca9f..f959d03decfc 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -584,6 +584,12 @@ struct cpu_sve_state {
  * field.
  */
 struct kvm_host_data {
+	/* SVE enabled for EL0 */
+#define HOST_SVE_ENABLED	0
+	/* SME enabled for EL0 */
+#define HOST_SME_ENABLED	1
+	unsigned long flags;
+
 	struct kvm_cpu_context host_ctxt;
 
 	/*
@@ -879,22 +885,18 @@ struct kvm_vcpu_arch {
 /* Save TRBE context if active  */
 #define DEBUG_STATE_SAVE_TRBE	__vcpu_single_flag(iflags, BIT(6))
 
-/* SVE enabled for host EL0 */
-#define HOST_SVE_ENABLED	__vcpu_single_flag(sflags, BIT(0))
-/* SME enabled for EL0 */
-#define HOST_SME_ENABLED	__vcpu_single_flag(sflags, BIT(1))
 /* Physical CPU not in supported_cpus */
-#define ON_UNSUPPORTED_CPU	__vcpu_single_flag(sflags, BIT(2))
+#define ON_UNSUPPORTED_CPU	__vcpu_single_flag(sflags, BIT(0))
 /* WFIT instruction trapped */
-#define IN_WFIT			__vcpu_single_flag(sflags, BIT(3))
+#define IN_WFIT			__vcpu_single_flag(sflags, BIT(1))
 /* vcpu system registers loaded on physical CPU */
-#define SYSREGS_ON_CPU		__vcpu_single_flag(sflags, BIT(4))
+#define SYSREGS_ON_CPU		__vcpu_single_flag(sflags, BIT(2))
 /* Software step state is Active-pending */
-#define DBG_SS_ACTIVE_PENDING	__vcpu_single_flag(sflags, BIT(5))
+#define DBG_SS_ACTIVE_PENDING	__vcpu_single_flag(sflags, BIT(3))
 /* PMUSERENR for the guest EL0 is on physical CPU */
-#define PMUSERENR_ON_CPU	__vcpu_single_flag(sflags, BIT(6))
+#define PMUSERENR_ON_CPU	__vcpu_single_flag(sflags, BIT(4))
 /* WFI instruction trapped */
-#define IN_WFI			__vcpu_single_flag(sflags, BIT(7))
+#define IN_WFI			__vcpu_single_flag(sflags, BIT(5))
 
 
 /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
@@ -1265,6 +1267,10 @@ DECLARE_KVM_HYP_PER_CPU(struct kvm_host_data, kvm_host_data);
 	 &this_cpu_ptr_hyp_sym(kvm_host_data)->f)
 #endif
 
+#define host_data_set_flag(nr)		set_bit(nr, host_data_ptr(flags))
+#define host_data_test_flag(nr)		test_bit(nr, host_data_ptr(flags))
+#define host_data_clear_flag(nr)	clear_bit(nr, host_data_ptr(flags))
+
 /* Check whether the FP regs are owned by the guest */
 static inline bool guest_owns_fp_regs(void)
 {
diff --git a/arch/arm64/kvm/fpsimd.c b/arch/arm64/kvm/fpsimd.c
index c53e5b14038d..f7712c89adef 100644
--- a/arch/arm64/kvm/fpsimd.c
+++ b/arch/arm64/kvm/fpsimd.c
@@ -64,14 +64,14 @@ void kvm_arch_vcpu_load_fp(struct kvm_vcpu *vcpu)
 	*host_data_ptr(fp_owner) = FP_STATE_HOST_OWNED;
 	*host_data_ptr(fpsimd_state) = kern_hyp_va(&current->thread.uw.fpsimd_state);
 
-	vcpu_clear_flag(vcpu, HOST_SVE_ENABLED);
+	host_data_clear_flag(HOST_SVE_ENABLED);
 	if (read_sysreg(cpacr_el1) & CPACR_EL1_ZEN_EL0EN)
-		vcpu_set_flag(vcpu, HOST_SVE_ENABLED);
+		host_data_set_flag(HOST_SVE_ENABLED);
 
 	if (system_supports_sme()) {
-		vcpu_clear_flag(vcpu, HOST_SME_ENABLED);
+		host_data_clear_flag(HOST_SME_ENABLED);
 		if (read_sysreg(cpacr_el1) & CPACR_EL1_SMEN_EL0EN)
-			vcpu_set_flag(vcpu, HOST_SME_ENABLED);
+			host_data_set_flag(HOST_SME_ENABLED);
 
 		/*
 		 * If PSTATE.SM is enabled then save any pending FP
@@ -167,7 +167,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
 	 */
 	if (has_vhe() && system_supports_sme()) {
 		/* Also restore EL0 state seen on entry */
-		if (vcpu_get_flag(vcpu, HOST_SME_ENABLED))
+		if (host_data_test_flag(HOST_SME_ENABLED))
 			sysreg_clear_set(CPACR_EL1, 0, CPACR_ELx_SMEN);
 		else
 			sysreg_clear_set(CPACR_EL1,
@@ -226,7 +226,7 @@ void kvm_arch_vcpu_put_fp(struct kvm_vcpu *vcpu)
 		 * for EL0.  To avoid spurious traps, restore the trap state
 		 * seen by kvm_arch_vcpu_load_fp():
 		 */
-		if (vcpu_get_flag(vcpu, HOST_SVE_ENABLED))
+		if (host_data_test_flag(HOST_SVE_ENABLED))
 			sysreg_clear_set(CPACR_EL1, 0, CPACR_EL1_ZEN_EL0EN);
 		else
 			sysreg_clear_set(CPACR_EL1, CPACR_EL1_ZEN_EL0EN, 0);
-- 
2.46.0.295.g3b9ea8a38a-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 3/3] KVM: arm64: nv: Request vPE doorbell upon nested ERET to L2
  2024-08-23 21:27 [PATCH 0/3] KVM: arm64: nv: Fix nested IRQ exception storm on NV+GICv4 Oliver Upton
  2024-08-23 21:27 ` [PATCH 1/3] KVM: arm64: nv: Leave vPE nonresident in nested vgic state Oliver Upton
  2024-08-23 21:27 ` [PATCH 2/3] KVM: arm64: Move host SVE/SME state flags out of vCPU Oliver Upton
@ 2024-08-23 21:27 ` Oliver Upton
  2 siblings, 0 replies; 4+ messages in thread
From: Oliver Upton @ 2024-08-23 21:27 UTC (permalink / raw)
  To: kvmarm
  Cc: Marc Zyngier, James Morse, Suzuki K Poulose, Zenghui Yu,
	Ganapatrao Kulkarni, Oliver Upton

Running an L2 guest with GICv4 enabled goes absolutely nowhere, and gets
into a vicious cycle of nested ERET followed by nested exception entry
into the L1.

When KVM does a put on a runnable vCPU, it marks the vPE as nonresident
but does not request a doorbell IRQ. Behind the scenes in the ITS
driver's view of the vCPU, its_vpe::pending_last gets set to true to
indicate that context is still runnable.

This comes to a head when doing the nested ERET into L2. The vPE doesn't
get scheduled on the redistributor as it is exclusively part of the L1's
VGIC context. kvm_vgic_vcpu_pending_irq() returns true because the vPE
appears runnable, and KVM does a nested exception entry into the L1
before L2 ever gets off the ground.

This issue can be papered over by requesting a doorbell IRQ when
descheduling a vPE as part of a nested ERET. KVM needs this anyway to
kick the vCPU out of the L2 when an IRQ becomes pending for the L1.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/include/asm/kvm_host.h |  2 ++
 arch/arm64/kvm/emulate-nested.c   |  2 ++
 arch/arm64/kvm/vgic/vgic-v4.c     | 18 +++++++++++++++++-
 3 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index f959d03decfc..7d561f0bd9bf 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -897,6 +897,8 @@ struct kvm_vcpu_arch {
 #define PMUSERENR_ON_CPU	__vcpu_single_flag(sflags, BIT(4))
 /* WFI instruction trapped */
 #define IN_WFI			__vcpu_single_flag(sflags, BIT(5))
+/* KVM is currently emulating a nested ERET */
+#define IN_NESTED_ERET		__vcpu_single_flag(sflags, BIT(6))
 
 
 /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
diff --git a/arch/arm64/kvm/emulate-nested.c b/arch/arm64/kvm/emulate-nested.c
index 05166eccea0a..fd3d6275b777 100644
--- a/arch/arm64/kvm/emulate-nested.c
+++ b/arch/arm64/kvm/emulate-nested.c
@@ -2310,6 +2310,7 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)
 	}
 
 	preempt_disable();
+	vcpu_set_flag(vcpu, IN_NESTED_ERET);
 	kvm_arch_vcpu_put(vcpu);
 
 	if (!esr_iss_is_eretax(esr))
@@ -2321,6 +2322,7 @@ void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)
 	*vcpu_cpsr(vcpu) = spsr;
 
 	kvm_arch_vcpu_load(vcpu, smp_processor_id());
+	vcpu_clear_flag(vcpu, IN_NESTED_ERET);
 	preempt_enable();
 }
 
diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c
index 74a67ad87f29..9f3f06ac76cc 100644
--- a/arch/arm64/kvm/vgic/vgic-v4.c
+++ b/arch/arm64/kvm/vgic/vgic-v4.c
@@ -336,6 +336,22 @@ void vgic_v4_teardown(struct kvm *kvm)
 	its_vm->vpes = NULL;
 }
 
+static inline bool vgic_v4_want_doorbell(struct kvm_vcpu *vcpu)
+{
+	if (vcpu_get_flag(vcpu, IN_WFI))
+		return true;
+
+	if (likely(!vcpu_has_nv(vcpu)))
+		return false;
+
+	/*
+	 * GICv4 hardware is only ever used for the L1. Mark the vPE (i.e. the
+	 * L1 context) nonresident and request a doorbell to kick us out of the
+	 * L2 when an IRQ becomes pending.
+	 */
+	return vcpu_get_flag(vcpu, IN_NESTED_ERET);
+}
+
 int vgic_v4_put(struct kvm_vcpu *vcpu)
 {
 	struct its_vpe *vpe = &vcpu->arch.vgic_cpu.vgic_v3.its_vpe;
@@ -343,7 +359,7 @@ int vgic_v4_put(struct kvm_vcpu *vcpu)
 	if (!vgic_supports_direct_msis(vcpu->kvm) || !vpe->resident)
 		return 0;
 
-	return its_make_vpe_non_resident(vpe, !!vcpu_get_flag(vcpu, IN_WFI));
+	return its_make_vpe_non_resident(vpe, vgic_v4_want_doorbell(vcpu));
 }
 
 int vgic_v4_load(struct kvm_vcpu *vcpu)
-- 
2.46.0.295.g3b9ea8a38a-goog


^ permalink raw reply related	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2024-08-23 21:27 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-08-23 21:27 [PATCH 0/3] KVM: arm64: nv: Fix nested IRQ exception storm on NV+GICv4 Oliver Upton
2024-08-23 21:27 ` [PATCH 1/3] KVM: arm64: nv: Leave vPE nonresident in nested vgic state Oliver Upton
2024-08-23 21:27 ` [PATCH 2/3] KVM: arm64: Move host SVE/SME state flags out of vCPU Oliver Upton
2024-08-23 21:27 ` [PATCH 3/3] KVM: arm64: nv: Request vPE doorbell upon nested ERET to L2 Oliver Upton

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox