From a0d7e2fc61ab54f1be817c9300231a1b48432628 Mon Sep 17 00:00:00 2001 From: Oliver Upton Date: Wed, 26 Feb 2025 10:31:21 -0800 Subject: [PATCH 1/5] KVM: arm64: vgic-v4: Only attempt vLPI mapping for actual MSIs Some 'creative' VMMs out there may assign a VFIO MSI eventfd to an SPI routing entry. And yes, I can already hear you shouting about possibly driving a level interrupt with an edge-sensitive one. You know who you are. This works for the most part, and interrupt injection winds up taking the software path. However, when running on GICv4-enabled hardware, KVM erroneously attempts to setup LPI forwarding, even though the KVM routing isn't an MSI. Thanks to misuse of a union, the MSI destination is unlikely to match any ITS in the VM and kvm_vgic_v4_set_forwarding() bails early. Later on when the VM is being torn down, this half-configured state triggers the WARN_ON() in kvm_vgic_v4_unset_forwarding() due to the fact that no HW IRQ was ever assigned. Avoid the whole mess by preventing SPI routing entries from getting into the LPI forwarding helpers. Reported-by: Sudheer Dantuluri Tested-by: Sudheer Dantuluri Fixes: 196b136498b3 ("KVM: arm/arm64: GICv4: Wire mapping/unmapping of VLPIs in VFIO irq bypass") Acked-by: Marc Zyngier Link: https://lore.kernel.org/r/20250226183124.82094-2-oliver.upton@linux.dev Signed-off-by: Oliver Upton --- arch/arm64/kvm/arm.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index b8e55a441282..36d8dd80b431 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -2717,6 +2717,14 @@ int kvm_arch_irq_bypass_add_producer(struct irq_bypass_consumer *cons, { struct kvm_kernel_irqfd *irqfd = container_of(cons, struct kvm_kernel_irqfd, consumer); + struct kvm_kernel_irq_routing_entry *irq_entry = &irqfd->irq_entry; + + /* + * The only thing we have a chance of directly-injecting is LPIs. Maybe + * one day... + */ + if (irq_entry->type != KVM_IRQ_ROUTING_MSI) + return 0; return kvm_vgic_v4_set_forwarding(irqfd->kvm, prod->irq, &irqfd->irq_entry); @@ -2726,6 +2734,10 @@ void kvm_arch_irq_bypass_del_producer(struct irq_bypass_consumer *cons, { struct kvm_kernel_irqfd *irqfd = container_of(cons, struct kvm_kernel_irqfd, consumer); + struct kvm_kernel_irq_routing_entry *irq_entry = &irqfd->irq_entry; + + if (irq_entry->type != KVM_IRQ_ROUTING_MSI) + return; kvm_vgic_v4_unset_forwarding(irqfd->kvm, prod->irq, &irqfd->irq_entry); From d0b79563fd60c063ee4ad2a76024e60338ed3881 Mon Sep 17 00:00:00 2001 From: Oliver Upton Date: Wed, 26 Feb 2025 10:31:22 -0800 Subject: [PATCH 2/5] KVM: arm64: vgic-v4: Only WARN for HW IRQ mismatch when unmapping vLPI The VMM or guest can easily screw up GICv4 vLPI injection by misconfiguring the MSI or the virtual ITS. Don't fuss over it; limit the WARN in vgic_v4_unset_forwarding() to fire in the worrying case where an unrelated HW IRQ was mapped to a vLPI. Reported-by: Sudheer Dantuluri Tested-by: Sudheer Dantuluri Acked-by: Marc Zyngier Link: https://lore.kernel.org/r/20250226183124.82094-3-oliver.upton@linux.dev Signed-off-by: Oliver Upton --- arch/arm64/kvm/vgic/vgic-v4.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c index eedecbbbcf31..3e1de40720e0 100644 --- a/arch/arm64/kvm/vgic/vgic-v4.c +++ b/arch/arm64/kvm/vgic/vgic-v4.c @@ -512,7 +512,7 @@ int kvm_vgic_v4_unset_forwarding(struct kvm *kvm, int virq, if (ret) goto out; - WARN_ON(!(irq->hw && irq->host_irq == virq)); + WARN_ON(irq->hw && irq->host_irq != virq); if (irq->hw) { atomic_dec(&irq->target_vcpu->arch.vgic_cpu.vgic_v3.its_vpe.vlpi_count); irq->hw = false; From 5c57533eb8c1efeeab4e2e417fa983d65707b925 Mon Sep 17 00:00:00 2001 From: Oliver Upton Date: Wed, 26 Feb 2025 10:31:23 -0800 Subject: [PATCH 3/5] KVM: arm64: vgic-v4: Fall back to software irqbypass if LPI not found Continuing with the theme of broken VMMs and guests, irqbypass registration can fail if the virtual ITS lacks a translation for the MSI. Either the guest hasn't mapped it or userspace may have forgotten to restore the ITS. Exit silently and allow irqbypass configuration to succeed. As a reward for ingenuity, LPIs are demoted to software injection. Tested-by: Sudheer Dantuluri Fixes: 196b136498b3 ("KVM: arm/arm64: GICv4: Wire mapping/unmapping of VLPIs in VFIO irq bypass") Acked-by: Marc Zyngier Link: https://lore.kernel.org/r/20250226183124.82094-4-oliver.upton@linux.dev Signed-off-by: Oliver Upton --- arch/arm64/kvm/vgic/vgic-v4.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/arch/arm64/kvm/vgic/vgic-v4.c b/arch/arm64/kvm/vgic/vgic-v4.c index 3e1de40720e0..9f51d58b5f8a 100644 --- a/arch/arm64/kvm/vgic/vgic-v4.c +++ b/arch/arm64/kvm/vgic/vgic-v4.c @@ -415,7 +415,7 @@ int kvm_vgic_v4_set_forwarding(struct kvm *kvm, int virq, struct vgic_irq *irq; struct its_vlpi_map map; unsigned long flags; - int ret; + int ret = 0; if (!vgic_supports_direct_msis(kvm)) return 0; @@ -430,10 +430,15 @@ int kvm_vgic_v4_set_forwarding(struct kvm *kvm, int virq, mutex_lock(&its->its_lock); - /* Perform the actual DevID/EventID -> LPI translation. */ - ret = vgic_its_resolve_lpi(kvm, its, irq_entry->msi.devid, - irq_entry->msi.data, &irq); - if (ret) + /* + * Perform the actual DevID/EventID -> LPI translation. + * + * Silently exit if translation fails as the guest (or userspace!) has + * managed to do something stupid. Emulated LPI injection will still + * work if the guest figures itself out at a later time. + */ + if (vgic_its_resolve_lpi(kvm, its, irq_entry->msi.devid, + irq_entry->msi.data, &irq)) goto out; /* Silently exit if the vLPI is already mapped */ From d766d87cf4a0a415a45bbb8fc2e3a4cc1995654d Mon Sep 17 00:00:00 2001 From: Oliver Upton Date: Wed, 26 Feb 2025 10:31:24 -0800 Subject: [PATCH 4/5] KVM: arm64: Document ordering requirements for irqbypass One of the not-so-obvious requirements for restoring a VM is ensuring that the vITS has been restored _before_ creating any irqbypass mappings. This is because KVM needs to get the guest translation for MSIs to correctly assemble the vLPI mapping getting installed in the physical ITS. Document the restore ordering requirements necessary for GICv4 vLPI injection to work. Acked-by: Marc Zyngier Link: https://lore.kernel.org/r/20250226183124.82094-5-oliver.upton@linux.dev Signed-off-by: Oliver Upton --- Documentation/virt/kvm/devices/arm-vgic-its.rst | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/virt/kvm/devices/arm-vgic-its.rst b/Documentation/virt/kvm/devices/arm-vgic-its.rst index e053124f77c4..0f971f83205a 100644 --- a/Documentation/virt/kvm/devices/arm-vgic-its.rst +++ b/Documentation/virt/kvm/devices/arm-vgic-its.rst @@ -126,7 +126,8 @@ KVM_DEV_ARM_VGIC_GRP_ITS_REGS ITS Restore Sequence: --------------------- -The following ordering must be followed when restoring the GIC and the ITS: +The following ordering must be followed when restoring the GIC, ITS, and +KVM_IRQFD assignments: a) restore all guest memory and create vcpus b) restore all redistributors @@ -139,6 +140,8 @@ d) restore the ITS in the following order: 3. Load the ITS table data (KVM_DEV_ARM_ITS_RESTORE_TABLES) 4. Restore GITS_CTLR +e) restore KVM_IRQFD assignments for MSIs + Then vcpus can be started. ITS Table ABI REV0: From 250f25367b58d8c65a1b060a2dda037eea09a672 Mon Sep 17 00:00:00 2001 From: Will Deacon Date: Fri, 14 Mar 2025 13:34:09 +0000 Subject: [PATCH 5/5] KVM: arm64: Tear down vGIC on failed vCPU creation If kvm_arch_vcpu_create() fails to share the vCPU page with the hypervisor, we propagate the error back to the ioctl but leave the vGIC vCPU data initialised. Note only does this leak the corresponding memory when the vCPU is destroyed but it can also lead to use-after-free if the redistributor device handling tries to walk into the vCPU. Add the missing cleanup to kvm_arch_vcpu_create(), ensuring that the vGIC vCPU structures are destroyed on error. Cc: Cc: Marc Zyngier Cc: Oliver Upton Cc: Quentin Perret Signed-off-by: Will Deacon Reviewed-by: Marc Zyngier Link: https://lore.kernel.org/r/20250314133409.9123-1-will@kernel.org Signed-off-by: Oliver Upton --- arch/arm64/kvm/arm.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c index 36d8dd80b431..fe1e1a1adc50 100644 --- a/arch/arm64/kvm/arm.c +++ b/arch/arm64/kvm/arm.c @@ -466,7 +466,11 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) if (err) return err; - return kvm_share_hyp(vcpu, vcpu + 1); + err = kvm_share_hyp(vcpu, vcpu + 1); + if (err) + kvm_vgic_vcpu_destroy(vcpu); + + return err; } void kvm_arch_vcpu_postcreate(struct kvm_vcpu *vcpu)