KVM: arm64: vgic-v4: Make the doorbell request robust w.r.t preemption
authorMarc Zyngier <maz@kernel.org>
Thu, 13 Jul 2023 07:06:57 +0000 (08:06 +0100)
committerOliver Upton <oliver.upton@linux.dev>
Thu, 13 Jul 2023 22:23:34 +0000 (22:23 +0000)
Xiang reports that VMs occasionally fail to boot on GICv4.1 systems when
running a preemptible kernel, as it is possible that a vCPU is blocked
without requesting a doorbell interrupt.

The issue is that any preemption that occurs between vgic_v4_put() and
schedule() on the block path will mark the vPE as nonresident and *not*
request a doorbell irq. This occurs because when the vcpu thread is
resumed on its way to block, vcpu_load() will make the vPE resident
again. Once the vcpu actually blocks, we don't request a doorbell
anymore, and the vcpu won't be woken up on interrupt delivery.

Fix it by tracking that we're entering WFI, and key the doorbell
request on that flag. This allows us not to make the vPE resident
when going through a preempt/schedule cycle, meaning we don't lose
any state.

Cc: stable@vger.kernel.org
Fixes: 8e01d9a396e6 ("KVM: arm64: vgic-v4: Move the GICv4 residency flow to be driven by vcpu_load/put")
Reported-by: Xiang Chen <chenxiang66@hisilicon.com>
Suggested-by: Zenghui Yu <yuzenghui@huawei.com>
Tested-by: Xiang Chen <chenxiang66@hisilicon.com>
Co-developed-by: Oliver Upton <oliver.upton@linux.dev>
Signed-off-by: Marc Zyngier <maz@kernel.org>
Acked-by: Zenghui Yu <yuzenghui@huawei.com>
Link: https://lore.kernel.org/r/20230713070657.3873244-1-maz@kernel.org
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
arch/arm64/include/asm/kvm_host.h
arch/arm64/kvm/arm.c
arch/arm64/kvm/vgic/vgic-v3.c
arch/arm64/kvm/vgic/vgic-v4.c
include/kvm/arm_vgic.h

index 8b6096753740ccce477ae60aa195bd8ff088da20..d3dd05bbfe23fc07ccd6ce49786b7ad22fc35bb6 100644 (file)
@@ -727,6 +727,8 @@ struct kvm_vcpu_arch {
 #define DBG_SS_ACTIVE_PENDING  __vcpu_single_flag(sflags, BIT(5))
 /* PMUSERENR for the guest EL0 is on physical CPU */
 #define PMUSERENR_ON_CPU       __vcpu_single_flag(sflags, BIT(6))
+/* WFI instruction trapped */
+#define IN_WFI                 __vcpu_single_flag(sflags, BIT(7))
 
 
 /* Pointer to the vcpu's SVE FFR for sve_{save,load}_state() */
index a402ea5511f3292e2ed8b0c6d924e8b38eb23778..72dc53a75d1c8b09b512f11d06fee248dbe991d3 100644 (file)
@@ -718,13 +718,15 @@ void kvm_vcpu_wfi(struct kvm_vcpu *vcpu)
         */
        preempt_disable();
        kvm_vgic_vmcr_sync(vcpu);
-       vgic_v4_put(vcpu, true);
+       vcpu_set_flag(vcpu, IN_WFI);
+       vgic_v4_put(vcpu);
        preempt_enable();
 
        kvm_vcpu_halt(vcpu);
        vcpu_clear_flag(vcpu, IN_WFIT);
 
        preempt_disable();
+       vcpu_clear_flag(vcpu, IN_WFI);
        vgic_v4_load(vcpu);
        preempt_enable();
 }
@@ -792,7 +794,7 @@ static int check_vcpu_requests(struct kvm_vcpu *vcpu)
                if (kvm_check_request(KVM_REQ_RELOAD_GICv4, vcpu)) {
                        /* The distributor enable bits were changed */
                        preempt_disable();
-                       vgic_v4_put(vcpu, false);
+                       vgic_v4_put(vcpu);
                        vgic_v4_load(vcpu);
                        preempt_enable();
                }
index c3b8e132d5992616ca1f308456ababce125c6c01..3dfc8b84e03e67868ff49cb72a97695a9222ef2b 100644 (file)
@@ -749,7 +749,7 @@ void vgic_v3_put(struct kvm_vcpu *vcpu)
 {
        struct vgic_v3_cpu_if *cpu_if = &vcpu->arch.vgic_cpu.vgic_v3;
 
-       WARN_ON(vgic_v4_put(vcpu, false));
+       WARN_ON(vgic_v4_put(vcpu));
 
        vgic_v3_vmcr_sync(vcpu);
 
index c1c28fe680ba317e9b8712de9a04b264991cdd0a..339a55194b2c63e78a6c8083fe7acb34a3cfa5af 100644 (file)
@@ -336,14 +336,14 @@ void vgic_v4_teardown(struct kvm *kvm)
        its_vm->vpes = NULL;
 }
 
-int vgic_v4_put(struct kvm_vcpu *vcpu, bool need_db)
+int vgic_v4_put(struct kvm_vcpu *vcpu)
 {
        struct its_vpe *vpe = &vcpu->arch.vgic_cpu.vgic_v3.its_vpe;
 
        if (!vgic_supports_direct_msis(vcpu->kvm) || !vpe->resident)
                return 0;
 
-       return its_make_vpe_non_resident(vpe, need_db);
+       return its_make_vpe_non_resident(vpe, !!vcpu_get_flag(vcpu, IN_WFI));
 }
 
 int vgic_v4_load(struct kvm_vcpu *vcpu)
@@ -354,6 +354,9 @@ int vgic_v4_load(struct kvm_vcpu *vcpu)
        if (!vgic_supports_direct_msis(vcpu->kvm) || vpe->resident)
                return 0;
 
+       if (vcpu_get_flag(vcpu, IN_WFI))
+               return 0;
+
        /*
         * Before making the VPE resident, make sure the redistributor
         * corresponding to our current CPU expects us here. See the
index 402b545959af7b2e4e7aa75b5f4271896d2d78d9..5b27f94d4fad6a5bc16d7f5fb737435dc0972bd2 100644 (file)
@@ -431,7 +431,7 @@ int kvm_vgic_v4_unset_forwarding(struct kvm *kvm, int irq,
 
 int vgic_v4_load(struct kvm_vcpu *vcpu);
 void vgic_v4_commit(struct kvm_vcpu *vcpu);
-int vgic_v4_put(struct kvm_vcpu *vcpu, bool need_db);
+int vgic_v4_put(struct kvm_vcpu *vcpu);
 
 /* CPU HP callbacks */
 void kvm_vgic_cpu_up(void);