[PATCH 1/2] KVM: arm64: vgic: Don't forward LPIs to a redistributor with LPIs disabled

[Fuad Tabba]

Clearing GICR_CTLR.EnableLPIs retires the LPIs on the target vCPU's
ap_list, then invalidates the ITS translation caches. However, an MSI
that hits the translation cache can still queue an LPI to that vCPU
after the retire, and so can an INVALL that enables an LPI already
pending. vgic_target_oracle() only checks that the distributor is
enabled, so the LPI goes on the ap_list and the vCPU takes it with
EnableLPIs clear.

Make the oracle also return NULL for an LPI whose redistributor has LPIs
disabled. vgic_queue_irq_unlock() rechecks it under the ap_list lock,
which the retire takes after clearing EnableLPIs, so either the retire
finds the LPI or the queueing reads EnableLPIs clear. The disable path
pauses the vCPU after clearing EnableLPIs, and with this change the
prune on that vCPU's exit would take its pending LPIs off the ap_list
before the retire walks it. Pause the vCPU before clearing EnableLPIs,
so that the retire still clears their pending state.

Fixes: 0e4e82f154e38 ("KVM: arm64: vgic-its: Enable ITS emulation as a virtual MSI controller")
Suggested-by: Oliver Upton <oupton@xxxxxxxxxx>
Link: https://lore.kernel.org/r/asf87i2RDh-WksL-@xxxxxxxxxx
Signed-off-by: Fuad Tabba <fuad.tabba@xxxxxxxxx>
---
 arch/arm64/kvm/vgic/vgic-mmio-v3.c | 31 ++++++++++++++++--------------
 arch/arm64/kvm/vgic/vgic.c         | 12 ++++++++----
 2 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/arch/arm64/kvm/vgic/vgic-mmio-v3.c b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
index f5bd29fbe3a90..9117cd85aa593 100644
--- a/arch/arm64/kvm/vgic/vgic-mmio-v3.c
+++ b/arch/arm64/kvm/vgic/vgic-mmio-v3.c
@@ -293,6 +293,18 @@ static void vgic_mmio_write_v3r_ctlr(struct kvm_vcpu *vcpu,
 		return;
 
 	if (!(val & GICR_CTLR_ENABLE_LPIS)) {
+		/*
+		 * Yes, disabling LPIs is painful, since it can be done from
+		 * a *remote* vcpu! So let's not take any chance, and make
+		 * sure that the target vCPU has written their LRs back to the
+		 * irq structures, and release any reference they would have.
+		 * Pause it before its LPIs read as disabled, so that the exit's
+		 * prune leaves them for vgic_flush_pending_lpis() to retire.
+		 *
+		 * If it hurts, don't do it.
+		 */
+		kvm_pause_vcpu(vcpu);
+
 		/*
 		 * Don't disable if RWP is set, as there already an
 		 * ongoing disable. Funky guest...
@@ -300,21 +312,12 @@ static void vgic_mmio_write_v3r_ctlr(struct kvm_vcpu *vcpu,
 		ctlr = atomic_cmpxchg_acquire(&vgic_cpu->ctlr,
 					      GICR_CTLR_ENABLE_LPIS,
 					      GICR_CTLR_RWP);
-		if (ctlr != GICR_CTLR_ENABLE_LPIS)
-			return;
+		if (ctlr == GICR_CTLR_ENABLE_LPIS) {
+			vgic_flush_pending_lpis(vcpu);
+			vgic_its_invalidate_all_caches(vcpu->kvm);
+			atomic_set_release(&vgic_cpu->ctlr, 0);
+		}
 
-		/*
-		 * Yes, disabling LPIs is painful, since it can be done from
-		 * a *remote* vcpu! So let's not take any chance, and make
-		 * sure that the target vCPU has written their LRs back to the
-		 * irq structures, and release any reference they would have.
-		 *
-		 * If it hurts, don't do it.
-		 */
-		kvm_pause_vcpu(vcpu);
-		vgic_flush_pending_lpis(vcpu);
-		vgic_its_invalidate_all_caches(vcpu->kvm);
-		atomic_set_release(&vgic_cpu->ctlr, 0);
 		kvm_resume_vcpu(vcpu);
 	} else {
 		ctlr = atomic_cmpxchg_acquire(&vgic_cpu->ctlr, 0,
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index 7061b4ab206c9..e1a6a5aa61a8e 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -273,14 +273,18 @@ struct kvm_vcpu *vgic_target_oracle(struct vgic_irq *irq)
 	 * If the IRQ is not active but enabled and pending, we should direct
 	 * it to its configured target VCPU.
 	 * If the distributor is disabled, pending interrupts shouldn't be
-	 * forwarded.
+	 * forwarded, nor should LPIs whose redistributor has them disabled.
 	 */
 	if (irq->enabled && irq_is_pending(irq)) {
-		if (unlikely(irq->target_vcpu &&
-			     !irq->target_vcpu->kvm->arch.vgic.enabled))
+		struct kvm_vcpu *vcpu = irq->target_vcpu;
+
+		if (unlikely(vcpu && !vcpu->kvm->arch.vgic.enabled))
 			return NULL;
 
-		return irq->target_vcpu;
+		if (unlikely(vcpu && irq_is_lpi(vcpu->kvm, irq->intid) && !vgic_lpis_enabled(vcpu)))
+			return NULL;
+
+		return vcpu;
 	}
 
 	/* If neither active nor pending and enabled, then this IRQ should not
-- 
2.47.3