KVM: arm64: nv: Fix nested IRQ exception storm on NV+GICv4
cover KVM: arm64: nv: Fix nested IRQ exception storm on NV+GICv4
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
Signature · 1 line
2.46.0.295.g3b9ea8a38a-goog
PATCH 1/3 KVM: arm64: nv: Leave vPE nonresident in nested vgic state
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.
--- arch/arm64/kvm/vgic/vgic-v3.c | 15 ++++++++++++++- include/kvm/arm_vgic.h | 5 +++++ 2 files changed, 19 insertions(+), 1 deletion(-)
arch/arm64/kvm/vgic/vgic-v3.c +14 -1
@@ -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 ())
include/kvm/arm_vgic.h +5 -0
@@ -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 );
Signature · 1 line
2.46.0.295.g3b9ea8a38a-goog
PATCH 2/3 KVM: arm64: Move host SVE/SME state flags out of vCPU
In reply to Oliver UptonWe'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.
--- arch/arm64/include/asm/kvm_host.h | 26 ++++++++++++++++---------- arch/arm64/kvm/fpsimd.c | 12 ++++++------ 2 files changed, 22 insertions(+), 16 deletions(-)
arch/arm64/include/asm/kvm_host.h +16 -10
@@ -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 ) {
arch/arm64/kvm/fpsimd.c +6 -6
@@ -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 (¤t->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 );
Signature · 1 line
2.46.0.295.g3b9ea8a38a-goog
PATCH 3/3 KVM: arm64: nv: Request vPE doorbell upon nested ERET to L2
In reply to Oliver UptonRunning 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.
--- 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(-)
arch/arm64/include/asm/kvm_host.h +2 -0
@@ -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() */
arch/arm64/kvm/emulate-nested.c +2 -0
@@ -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 (); }
arch/arm64/kvm/vgic/vgic-v4.c +17 -1
@@ -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)
Signature · 1 line
2.46.0.295.g3b9ea8a38a-goog