All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andre Przywara <andre.przywara@arm.com>
To: Marc Zyngier <marc.zyngier@arm.com>,
	Christoffer Dall <christoffer.dall@linaro.org>,
	Eric Auger <eric.auger@redhat.com>
Cc: linux-arm-kernel@lists.infradead.org,
	kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org
Subject: [PATCH v6 05/15] KVM: arm/arm64: VGIC: add refcounting for IRQs
Date: Fri, 17 Jun 2016 13:08:37 +0100	[thread overview]
Message-ID: <1466165327-32060-6-git-send-email-andre.przywara@arm.com> (raw)
In-Reply-To: <1466165327-32060-1-git-send-email-andre.przywara@arm.com>

In the moment our struct vgic_irq's are statically allocated at guest
creation time. So getting a pointer to an IRQ structure is trivial and
safe. LPIs are more dynamic, they can be mapped and unmapped at any time
during the guest's _runtime_.
In preparation for supporting LPIs we introduce reference counting to
those structures. Since private IRQs and SPIs are statically allocated,
the reqcount never drops to 0 at the moment, but we increase it when an
IRQ gets onto a VCPU list and decrease it when it gets removed.
Also vgic_get_irq() gets changed to take the irq_lock already, this is
later needed to avoid a race between a potential LPI unmap and the
window between us getting the pointer and locking the IRQ.
This introduces vgic_put_irq(), which just does the unlock and makes
the new locking sequence look more symmetrical.
This approach deviates from the classical Linux refcounting with using
atomic_* types and functions, because the users of vgic_get_irq() take
the irq_lock anyway, so we just use an int and adjust the refcount while
holding the lock.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 include/kvm/vgic/vgic.h          |  1 +
 virt/kvm/arm/vgic/vgic-init.c    |  2 +
 virt/kvm/arm/vgic/vgic-mmio-v2.c | 21 +++++------
 virt/kvm/arm/vgic/vgic-mmio-v3.c | 26 ++++++-------
 virt/kvm/arm/vgic/vgic-mmio.c    | 55 ++++++++++++++++-----------
 virt/kvm/arm/vgic/vgic-v2.c      |  4 +-
 virt/kvm/arm/vgic/vgic-v3.c      |  4 +-
 virt/kvm/arm/vgic/vgic.c         | 81 +++++++++++++++++++++++++---------------
 virt/kvm/arm/vgic/vgic.h         |  7 +++-
 9 files changed, 116 insertions(+), 85 deletions(-)

diff --git a/include/kvm/vgic/vgic.h b/include/kvm/vgic/vgic.h
index 2f26f37..e488a369 100644
--- a/include/kvm/vgic/vgic.h
+++ b/include/kvm/vgic/vgic.h
@@ -96,6 +96,7 @@ struct vgic_irq {
 	bool active;			/* not used for LPIs */
 	bool enabled;
 	bool hw;			/* Tied to HW IRQ */
+	int refcnt;			/* Used only for LPIs */
 	u32 hwintid;			/* HW INTID number */
 	union {
 		u8 targets;			/* GICv2 target VCPUs mask */
diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
index 90cae48..c4a8df6 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -177,6 +177,7 @@ static int kvm_vgic_dist_init(struct kvm *kvm, unsigned int nr_spis)
 		spin_lock_init(&irq->irq_lock);
 		irq->vcpu = NULL;
 		irq->target_vcpu = vcpu0;
+		irq->refcnt = 1;
 		if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2)
 			irq->targets = 0;
 		else
@@ -211,6 +212,7 @@ static void kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 		irq->vcpu = NULL;
 		irq->target_vcpu = vcpu;
 		irq->targets = 1U << vcpu->vcpu_id;
+		irq->refcnt = 1;
 		if (vgic_irq_is_sgi(i)) {
 			/* SGIs */
 			irq->enabled = 1;
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c
index a213936..7bb3e94 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v2.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c
@@ -97,11 +97,10 @@ static void vgic_mmio_write_sgir(struct kvm_vcpu *source_vcpu,
 
 		irq = vgic_get_irq(source_vcpu->kvm, vcpu, intid);
 
-		spin_lock(&irq->irq_lock);
 		irq->pending = true;
 		irq->source |= 1U << source_vcpu->vcpu_id;
 
-		vgic_queue_irq_unlock(source_vcpu->kvm, irq);
+		vgic_queue_irq_put(source_vcpu->kvm, irq);
 	}
 }
 
@@ -116,6 +115,8 @@ static unsigned long vgic_mmio_read_target(struct kvm_vcpu *vcpu,
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
 		val |= (u64)irq->targets << (i * 8);
+
+		vgic_put_irq(irq);
 	}
 
 	return val;
@@ -136,13 +137,11 @@ static void vgic_mmio_write_target(struct kvm_vcpu *vcpu,
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, NULL, intid + i);
 		int target;
 
-		spin_lock(&irq->irq_lock);
-
 		irq->targets = (val >> (i * 8)) & 0xff;
 		target = irq->targets ? __ffs(irq->targets) : 0;
 		irq->target_vcpu = kvm_get_vcpu(vcpu->kvm, target);
 
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 	}
 }
 
@@ -157,6 +156,8 @@ static unsigned long vgic_mmio_read_sgipend(struct kvm_vcpu *vcpu,
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
 		val |= (u64)irq->source << (i * 8);
+
+		vgic_put_irq(irq);
 	}
 	return val;
 }
@@ -171,13 +172,11 @@ static void vgic_mmio_write_sgipendc(struct kvm_vcpu *vcpu,
 	for (i = 0; i < len; i++) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
-		spin_lock(&irq->irq_lock);
-
 		irq->source &= ~((val >> (i * 8)) & 0xff);
 		if (!irq->source)
 			irq->pending = false;
 
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 	}
 }
 
@@ -191,15 +190,13 @@ static void vgic_mmio_write_sgipends(struct kvm_vcpu *vcpu,
 	for (i = 0; i < len; i++) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
-		spin_lock(&irq->irq_lock);
-
 		irq->source |= (val >> (i * 8)) & 0xff;
 
 		if (irq->source) {
 			irq->pending = true;
-			vgic_queue_irq_unlock(vcpu->kvm, irq);
+			vgic_queue_irq_put(vcpu->kvm, irq);
 		} else {
-			spin_unlock(&irq->irq_lock);
+			vgic_put_irq(irq);
 		}
 	}
 }
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index fc7b6c9..c38302d 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -80,15 +80,17 @@ static unsigned long vgic_mmio_read_irouter(struct kvm_vcpu *vcpu,
 {
 	int intid = VGIC_ADDR_TO_INTID(addr, 64);
 	struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, NULL, intid);
+	unsigned long ret = 0;
 
 	if (!irq)
 		return 0;
 
 	/* The upper word is RAZ for us. */
-	if (addr & 4)
-		return 0;
+	if (!(addr & 4))
+		ret = extract_bytes(READ_ONCE(irq->mpidr), addr & 7, len);
 
-	return extract_bytes(READ_ONCE(irq->mpidr), addr & 7, len);
+	vgic_put_irq(irq);
+	return ret;
 }
 
 static void vgic_mmio_write_irouter(struct kvm_vcpu *vcpu,
@@ -102,16 +104,13 @@ static void vgic_mmio_write_irouter(struct kvm_vcpu *vcpu,
 		return;
 
 	/* The upper word is WI for us since we don't implement Aff3. */
-	if (addr & 4)
-		return;
-
-	spin_lock(&irq->irq_lock);
-
-	/* We only care about and preserve Aff0, Aff1 and Aff2. */
-	irq->mpidr = val & GENMASK(23, 0);
-	irq->target_vcpu = kvm_mpidr_to_vcpu(vcpu->kvm, irq->mpidr);
+	if (!(addr & 4)) {
+		/* We only care about and preserve Aff0, Aff1 and Aff2. */
+		irq->mpidr = val & GENMASK(23, 0);
+		irq->target_vcpu = kvm_mpidr_to_vcpu(vcpu->kvm, irq->mpidr);
+	}
 
-	spin_unlock(&irq->irq_lock);
+	vgic_put_irq(irq);
 }
 
 static unsigned long vgic_mmio_read_v3r_typer(struct kvm_vcpu *vcpu,
@@ -441,9 +440,8 @@ void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg)
 
 		irq = vgic_get_irq(vcpu->kvm, c_vcpu, sgi);
 
-		spin_lock(&irq->irq_lock);
 		irq->pending = true;
 
-		vgic_queue_irq_unlock(vcpu->kvm, irq);
+		vgic_queue_irq_put(vcpu->kvm, irq);
 	}
 }
diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
index 9f6fab7..4050c1c 100644
--- a/virt/kvm/arm/vgic/vgic-mmio.c
+++ b/virt/kvm/arm/vgic/vgic-mmio.c
@@ -56,6 +56,8 @@ unsigned long vgic_mmio_read_enable(struct kvm_vcpu *vcpu,
 
 		if (irq->enabled)
 			value |= (1U << i);
+
+		vgic_put_irq(irq);
 	}
 
 	return value;
@@ -71,9 +73,9 @@ void vgic_mmio_write_senable(struct kvm_vcpu *vcpu,
 	for_each_set_bit(i, &val, len * 8) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
-		spin_lock(&irq->irq_lock);
 		irq->enabled = true;
-		vgic_queue_irq_unlock(vcpu->kvm, irq);
+
+		vgic_queue_irq_put(vcpu->kvm, irq);
 	}
 }
 
@@ -87,11 +89,9 @@ void vgic_mmio_write_cenable(struct kvm_vcpu *vcpu,
 	for_each_set_bit(i, &val, len * 8) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
-		spin_lock(&irq->irq_lock);
-
 		irq->enabled = false;
 
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 	}
 }
 
@@ -108,6 +108,8 @@ unsigned long vgic_mmio_read_pending(struct kvm_vcpu *vcpu,
 
 		if (irq->pending)
 			value |= (1U << i);
+
+		vgic_put_irq(irq);
 	}
 
 	return value;
@@ -123,12 +125,11 @@ void vgic_mmio_write_spending(struct kvm_vcpu *vcpu,
 	for_each_set_bit(i, &val, len * 8) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
-		spin_lock(&irq->irq_lock);
 		irq->pending = true;
 		if (irq->config == VGIC_CONFIG_LEVEL)
 			irq->soft_pending = true;
 
-		vgic_queue_irq_unlock(vcpu->kvm, irq);
+		vgic_queue_irq_put(vcpu->kvm, irq);
 	}
 }
 
@@ -142,8 +143,6 @@ void vgic_mmio_write_cpending(struct kvm_vcpu *vcpu,
 	for_each_set_bit(i, &val, len * 8) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
-		spin_lock(&irq->irq_lock);
-
 		if (irq->config == VGIC_CONFIG_LEVEL) {
 			irq->soft_pending = false;
 			irq->pending = irq->line_level;
@@ -151,7 +150,7 @@ void vgic_mmio_write_cpending(struct kvm_vcpu *vcpu,
 			irq->pending = false;
 		}
 
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 	}
 }
 
@@ -168,15 +167,17 @@ unsigned long vgic_mmio_read_active(struct kvm_vcpu *vcpu,
 
 		if (irq->active)
 			value |= (1U << i);
+
+		vgic_put_irq(irq);
 	}
 
 	return value;
 }
 
-static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
-				    bool new_active_state)
+static void vgic_mmio_change_active_put(struct kvm_vcpu *vcpu,
+					struct vgic_irq *irq,
+					bool new_active_state)
 {
-	spin_lock(&irq->irq_lock);
 	/*
 	 * If this virtual IRQ was written into a list register, we
 	 * have to make sure the CPU that runs the VCPU thread has
@@ -190,15 +191,17 @@ static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
 	 * IRQ, so we release and re-acquire the spin_lock to let the
 	 * other thread sync back the IRQ.
 	 */
+	irq->refcnt++;
 	while (irq->vcpu && /* IRQ may have state in an LR somewhere */
 	       irq->vcpu->cpu != -1) /* VCPU thread is running */
 		cond_resched_lock(&irq->irq_lock);
 
+	irq->refcnt--;
 	irq->active = new_active_state;
 	if (new_active_state)
-		vgic_queue_irq_unlock(vcpu->kvm, irq);
+		vgic_queue_irq_put(vcpu->kvm, irq);
 	else
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 }
 
 /*
@@ -241,7 +244,8 @@ void vgic_mmio_write_cactive(struct kvm_vcpu *vcpu,
 	vgic_change_active_prepare(vcpu, intid);
 	for_each_set_bit(i, &val, len * 8) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
-		vgic_mmio_change_active(vcpu, irq, false);
+
+		vgic_mmio_change_active_put(vcpu, irq, false);
 	}
 	vgic_change_active_finish(vcpu, intid);
 }
@@ -256,7 +260,8 @@ void vgic_mmio_write_sactive(struct kvm_vcpu *vcpu,
 	vgic_change_active_prepare(vcpu, intid);
 	for_each_set_bit(i, &val, len * 8) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
-		vgic_mmio_change_active(vcpu, irq, true);
+
+		vgic_mmio_change_active_put(vcpu, irq, true);
 	}
 	vgic_change_active_finish(vcpu, intid);
 }
@@ -272,6 +277,8 @@ unsigned long vgic_mmio_read_priority(struct kvm_vcpu *vcpu,
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
 		val |= (u64)irq->priority << (i * 8);
+
+		vgic_put_irq(irq);
 	}
 
 	return val;
@@ -294,10 +301,10 @@ void vgic_mmio_write_priority(struct kvm_vcpu *vcpu,
 	for (i = 0; i < len; i++) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
-		spin_lock(&irq->irq_lock);
 		/* Narrow the priority range to what we actually support */
 		irq->priority = (val >> (i * 8)) & GENMASK(7, 8 - VGIC_PRI_BITS);
-		spin_unlock(&irq->irq_lock);
+
+		vgic_put_irq(irq);
 	}
 }
 
@@ -313,6 +320,8 @@ unsigned long vgic_mmio_read_config(struct kvm_vcpu *vcpu,
 
 		if (irq->config == VGIC_CONFIG_EDGE)
 			value |= (2U << (i * 2));
+
+		vgic_put_irq(irq);
 	}
 
 	return value;
@@ -326,7 +335,7 @@ void vgic_mmio_write_config(struct kvm_vcpu *vcpu,
 	int i;
 
 	for (i = 0; i < len * 4; i++) {
-		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
+		struct vgic_irq *irq;
 
 		/*
 		 * The configuration cannot be changed for SGIs in general,
@@ -337,14 +346,16 @@ void vgic_mmio_write_config(struct kvm_vcpu *vcpu,
 		if (intid + i < VGIC_NR_PRIVATE_IRQS)
 			continue;
 
-		spin_lock(&irq->irq_lock);
+		irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
+
 		if (test_bit(i * 2 + 1, &val)) {
 			irq->config = VGIC_CONFIG_EDGE;
 		} else {
 			irq->config = VGIC_CONFIG_LEVEL;
 			irq->pending = irq->line_level | irq->soft_pending;
 		}
-		spin_unlock(&irq->irq_lock);
+
+		vgic_put_irq(irq);
 	}
 }
 
diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c
index 80313de..2147576 100644
--- a/virt/kvm/arm/vgic/vgic-v2.c
+++ b/virt/kvm/arm/vgic/vgic-v2.c
@@ -94,8 +94,6 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)
 
 		irq = vgic_get_irq(vcpu->kvm, vcpu, intid);
 
-		spin_lock(&irq->irq_lock);
-
 		/* Always preserve the active bit */
 		irq->active = !!(val & GICH_LR_ACTIVE_BIT);
 
@@ -123,7 +121,7 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)
 			irq->pending = irq->line_level || irq->soft_pending;
 		}
 
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 	}
 }
 
diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
index e48a22e..21d84e9 100644
--- a/virt/kvm/arm/vgic/vgic-v3.c
+++ b/virt/kvm/arm/vgic/vgic-v3.c
@@ -82,8 +82,6 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
 			intid = val & GICH_LR_VIRTUALID;
 		irq = vgic_get_irq(vcpu->kvm, vcpu, intid);
 
-		spin_lock(&irq->irq_lock);
-
 		/* Always preserve the active bit */
 		irq->active = !!(val & ICH_LR_ACTIVE_BIT);
 
@@ -112,7 +110,7 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
 			irq->pending = irq->line_level || irq->soft_pending;
 		}
 
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 	}
 }
 
diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index 69b61ab..90f2543 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -48,13 +48,20 @@ struct vgic_global __section(.hyp.text) kvm_vgic_global_state;
 struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct kvm_vcpu *vcpu,
 			      u32 intid)
 {
-	/* SGIs and PPIs */
-	if (intid <= VGIC_MAX_PRIVATE)
-		return &vcpu->arch.vgic_cpu.private_irqs[intid];
+	struct vgic_dist *dist = &kvm->arch.vgic;
+	struct vgic_irq *irq;
 
-	/* SPIs */
-	if (intid <= VGIC_MAX_SPI)
-		return &kvm->arch.vgic.spis[intid - VGIC_NR_PRIVATE_IRQS];
+	if (intid <= VGIC_MAX_PRIVATE) {        /* SGIs and PPIs */
+		irq = &vcpu->arch.vgic_cpu.private_irqs[intid];
+		spin_lock(&irq->irq_lock);
+		return irq;
+	}
+
+	if (intid <= VGIC_MAX_SPI) {            /* SPIs */
+		irq = &dist->spis[intid - VGIC_NR_PRIVATE_IRQS];
+		spin_lock(&irq->irq_lock);
+		return irq;
+	}
 
 	/* LPIs are not yet covered */
 	if (intid >= VGIC_MIN_LPI)
@@ -183,9 +190,10 @@ static bool vgic_validate_injection(struct vgic_irq *irq, bool level)
  * Needs to be entered with the IRQ lock already held, but will return
  * with all locks dropped.
  */
-bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq)
+bool vgic_queue_irq_put(struct kvm *kvm, struct vgic_irq *irq)
 {
-	struct kvm_vcpu *vcpu;
+	struct kvm_vcpu *vcpu, *irq_vcpu = irq->target_vcpu;
+	u32 intid = irq->intid;
 
 	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&irq->irq_lock));
 
@@ -201,7 +209,7 @@ retry:
 		 * not need to be inserted into an ap_list and there is also
 		 * no more work for us to do.
 		 */
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 		return false;
 	}
 
@@ -209,12 +217,18 @@ retry:
 	 * We must unlock the irq lock to take the ap_list_lock where
 	 * we are going to insert this new pending interrupt.
 	 */
-	spin_unlock(&irq->irq_lock);
+	vgic_put_irq(irq);
 
 	/* someone can do stuff here, which we re-check below */
 
 	spin_lock(&vcpu->arch.vgic_cpu.ap_list_lock);
-	spin_lock(&irq->irq_lock);
+	irq = vgic_get_irq(kvm, irq_vcpu, intid);
+
+	if (!irq) {
+		/* The LPI has been unmapped, nothing left to do. */
+		spin_unlock(&vcpu->arch.vgic_cpu.ap_list_lock);
+		return false;
+	}
 
 	/*
 	 * Did something change behind our backs?
@@ -229,17 +243,21 @@ retry:
 	 */
 
 	if (unlikely(irq->vcpu || vcpu != vgic_target_oracle(irq))) {
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 		spin_unlock(&vcpu->arch.vgic_cpu.ap_list_lock);
 
-		spin_lock(&irq->irq_lock);
+		irq = vgic_get_irq(kvm, irq_vcpu, intid);
+		if (!irq)
+			return false;
+
 		goto retry;
 	}
 
 	list_add_tail(&irq->ap_list, &vcpu->arch.vgic_cpu.ap_list_head);
+	irq->refcnt++;
 	irq->vcpu = vcpu;
 
-	spin_unlock(&irq->irq_lock);
+	vgic_put_irq(irq);
 	spin_unlock(&vcpu->arch.vgic_cpu.ap_list_lock);
 
 	kvm_vcpu_kick(vcpu);
@@ -269,14 +287,14 @@ static int vgic_update_irq_pending(struct kvm *kvm, int cpuid,
 	if (!irq)
 		return -EINVAL;
 
-	if (irq->hw != mapped_irq)
+	if (irq->hw != mapped_irq) {
+		vgic_put_irq(irq);
 		return -EINVAL;
-
-	spin_lock(&irq->irq_lock);
+	}
 
 	if (!vgic_validate_injection(irq, level)) {
 		/* Nothing to see here, move along... */
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 		return 0;
 	}
 
@@ -287,7 +305,7 @@ static int vgic_update_irq_pending(struct kvm *kvm, int cpuid,
 		irq->pending = true;
 	}
 
-	vgic_queue_irq_unlock(kvm, irq);
+	vgic_queue_irq_put(kvm, irq);
 
 	return 0;
 }
@@ -324,31 +342,28 @@ int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, u32 virt_irq, u32 phys_irq)
 
 	BUG_ON(!irq);
 
-	spin_lock(&irq->irq_lock);
-
 	irq->hw = true;
 	irq->hwintid = phys_irq;
 
-	spin_unlock(&irq->irq_lock);
+	vgic_put_irq(irq);
 
 	return 0;
 }
 
 int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, unsigned int virt_irq)
 {
-	struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, virt_irq);
-
-	BUG_ON(!irq);
+	struct vgic_irq *irq;
 
 	if (!vgic_initialized(vcpu->kvm))
 		return -EAGAIN;
 
-	spin_lock(&irq->irq_lock);
+	irq = vgic_get_irq(vcpu->kvm, vcpu, virt_irq);
+	BUG_ON(!irq);
 
 	irq->hw = false;
 	irq->hwintid = 0;
 
-	spin_unlock(&irq->irq_lock);
+	vgic_put_irq(irq);
 
 	return 0;
 }
@@ -378,14 +393,21 @@ retry:
 
 		target_vcpu = vgic_target_oracle(irq);
 
-		if (!target_vcpu) {
+		if (!target_vcpu || irq->refcnt == 1) {
+			bool free_irq = false;
+
 			/*
 			 * We don't need to process this interrupt any
 			 * further, move it off the list.
 			 */
 			list_del(&irq->ap_list);
 			irq->vcpu = NULL;
+			irq->refcnt--;
+			if (!irq->refcnt)
+				free_irq = true;
 			spin_unlock(&irq->irq_lock);
+			if (free_irq)
+				kfree(irq);
 			continue;
 		}
 
@@ -611,9 +633,8 @@ bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, unsigned int virt_irq)
 	struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, virt_irq);
 	bool map_is_active;
 
-	spin_lock(&irq->irq_lock);
 	map_is_active = irq->hw && irq->active;
-	spin_unlock(&irq->irq_lock);
+	vgic_put_irq(irq);
 
 	return map_is_active;
 }
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index c752152..fa2d225 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -38,7 +38,12 @@ struct vgic_vmcr {
 
 struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct kvm_vcpu *vcpu,
 			      u32 intid);
-bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq);
+static inline void vgic_put_irq(struct vgic_irq *irq)
+{
+	spin_unlock(&irq->irq_lock);
+}
+
+bool vgic_queue_irq_put(struct kvm *kvm, struct vgic_irq *irq);
 void vgic_kick_vcpus(struct kvm *kvm);
 
 void vgic_v2_process_maintenance(struct kvm_vcpu *vcpu);
-- 
2.8.2

WARNING: multiple messages have this Message-ID (diff)
From: andre.przywara@arm.com (Andre Przywara)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v6 05/15] KVM: arm/arm64: VGIC: add refcounting for IRQs
Date: Fri, 17 Jun 2016 13:08:37 +0100	[thread overview]
Message-ID: <1466165327-32060-6-git-send-email-andre.przywara@arm.com> (raw)
In-Reply-To: <1466165327-32060-1-git-send-email-andre.przywara@arm.com>

In the moment our struct vgic_irq's are statically allocated at guest
creation time. So getting a pointer to an IRQ structure is trivial and
safe. LPIs are more dynamic, they can be mapped and unmapped at any time
during the guest's _runtime_.
In preparation for supporting LPIs we introduce reference counting to
those structures. Since private IRQs and SPIs are statically allocated,
the reqcount never drops to 0 at the moment, but we increase it when an
IRQ gets onto a VCPU list and decrease it when it gets removed.
Also vgic_get_irq() gets changed to take the irq_lock already, this is
later needed to avoid a race between a potential LPI unmap and the
window between us getting the pointer and locking the IRQ.
This introduces vgic_put_irq(), which just does the unlock and makes
the new locking sequence look more symmetrical.
This approach deviates from the classical Linux refcounting with using
atomic_* types and functions, because the users of vgic_get_irq() take
the irq_lock anyway, so we just use an int and adjust the refcount while
holding the lock.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 include/kvm/vgic/vgic.h          |  1 +
 virt/kvm/arm/vgic/vgic-init.c    |  2 +
 virt/kvm/arm/vgic/vgic-mmio-v2.c | 21 +++++------
 virt/kvm/arm/vgic/vgic-mmio-v3.c | 26 ++++++-------
 virt/kvm/arm/vgic/vgic-mmio.c    | 55 ++++++++++++++++-----------
 virt/kvm/arm/vgic/vgic-v2.c      |  4 +-
 virt/kvm/arm/vgic/vgic-v3.c      |  4 +-
 virt/kvm/arm/vgic/vgic.c         | 81 +++++++++++++++++++++++++---------------
 virt/kvm/arm/vgic/vgic.h         |  7 +++-
 9 files changed, 116 insertions(+), 85 deletions(-)

diff --git a/include/kvm/vgic/vgic.h b/include/kvm/vgic/vgic.h
index 2f26f37..e488a369 100644
--- a/include/kvm/vgic/vgic.h
+++ b/include/kvm/vgic/vgic.h
@@ -96,6 +96,7 @@ struct vgic_irq {
 	bool active;			/* not used for LPIs */
 	bool enabled;
 	bool hw;			/* Tied to HW IRQ */
+	int refcnt;			/* Used only for LPIs */
 	u32 hwintid;			/* HW INTID number */
 	union {
 		u8 targets;			/* GICv2 target VCPUs mask */
diff --git a/virt/kvm/arm/vgic/vgic-init.c b/virt/kvm/arm/vgic/vgic-init.c
index 90cae48..c4a8df6 100644
--- a/virt/kvm/arm/vgic/vgic-init.c
+++ b/virt/kvm/arm/vgic/vgic-init.c
@@ -177,6 +177,7 @@ static int kvm_vgic_dist_init(struct kvm *kvm, unsigned int nr_spis)
 		spin_lock_init(&irq->irq_lock);
 		irq->vcpu = NULL;
 		irq->target_vcpu = vcpu0;
+		irq->refcnt = 1;
 		if (dist->vgic_model == KVM_DEV_TYPE_ARM_VGIC_V2)
 			irq->targets = 0;
 		else
@@ -211,6 +212,7 @@ static void kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu)
 		irq->vcpu = NULL;
 		irq->target_vcpu = vcpu;
 		irq->targets = 1U << vcpu->vcpu_id;
+		irq->refcnt = 1;
 		if (vgic_irq_is_sgi(i)) {
 			/* SGIs */
 			irq->enabled = 1;
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c
index a213936..7bb3e94 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v2.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c
@@ -97,11 +97,10 @@ static void vgic_mmio_write_sgir(struct kvm_vcpu *source_vcpu,
 
 		irq = vgic_get_irq(source_vcpu->kvm, vcpu, intid);
 
-		spin_lock(&irq->irq_lock);
 		irq->pending = true;
 		irq->source |= 1U << source_vcpu->vcpu_id;
 
-		vgic_queue_irq_unlock(source_vcpu->kvm, irq);
+		vgic_queue_irq_put(source_vcpu->kvm, irq);
 	}
 }
 
@@ -116,6 +115,8 @@ static unsigned long vgic_mmio_read_target(struct kvm_vcpu *vcpu,
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
 		val |= (u64)irq->targets << (i * 8);
+
+		vgic_put_irq(irq);
 	}
 
 	return val;
@@ -136,13 +137,11 @@ static void vgic_mmio_write_target(struct kvm_vcpu *vcpu,
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, NULL, intid + i);
 		int target;
 
-		spin_lock(&irq->irq_lock);
-
 		irq->targets = (val >> (i * 8)) & 0xff;
 		target = irq->targets ? __ffs(irq->targets) : 0;
 		irq->target_vcpu = kvm_get_vcpu(vcpu->kvm, target);
 
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 	}
 }
 
@@ -157,6 +156,8 @@ static unsigned long vgic_mmio_read_sgipend(struct kvm_vcpu *vcpu,
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
 		val |= (u64)irq->source << (i * 8);
+
+		vgic_put_irq(irq);
 	}
 	return val;
 }
@@ -171,13 +172,11 @@ static void vgic_mmio_write_sgipendc(struct kvm_vcpu *vcpu,
 	for (i = 0; i < len; i++) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
-		spin_lock(&irq->irq_lock);
-
 		irq->source &= ~((val >> (i * 8)) & 0xff);
 		if (!irq->source)
 			irq->pending = false;
 
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 	}
 }
 
@@ -191,15 +190,13 @@ static void vgic_mmio_write_sgipends(struct kvm_vcpu *vcpu,
 	for (i = 0; i < len; i++) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
-		spin_lock(&irq->irq_lock);
-
 		irq->source |= (val >> (i * 8)) & 0xff;
 
 		if (irq->source) {
 			irq->pending = true;
-			vgic_queue_irq_unlock(vcpu->kvm, irq);
+			vgic_queue_irq_put(vcpu->kvm, irq);
 		} else {
-			spin_unlock(&irq->irq_lock);
+			vgic_put_irq(irq);
 		}
 	}
 }
diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c
index fc7b6c9..c38302d 100644
--- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
+++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
@@ -80,15 +80,17 @@ static unsigned long vgic_mmio_read_irouter(struct kvm_vcpu *vcpu,
 {
 	int intid = VGIC_ADDR_TO_INTID(addr, 64);
 	struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, NULL, intid);
+	unsigned long ret = 0;
 
 	if (!irq)
 		return 0;
 
 	/* The upper word is RAZ for us. */
-	if (addr & 4)
-		return 0;
+	if (!(addr & 4))
+		ret = extract_bytes(READ_ONCE(irq->mpidr), addr & 7, len);
 
-	return extract_bytes(READ_ONCE(irq->mpidr), addr & 7, len);
+	vgic_put_irq(irq);
+	return ret;
 }
 
 static void vgic_mmio_write_irouter(struct kvm_vcpu *vcpu,
@@ -102,16 +104,13 @@ static void vgic_mmio_write_irouter(struct kvm_vcpu *vcpu,
 		return;
 
 	/* The upper word is WI for us since we don't implement Aff3. */
-	if (addr & 4)
-		return;
-
-	spin_lock(&irq->irq_lock);
-
-	/* We only care about and preserve Aff0, Aff1 and Aff2. */
-	irq->mpidr = val & GENMASK(23, 0);
-	irq->target_vcpu = kvm_mpidr_to_vcpu(vcpu->kvm, irq->mpidr);
+	if (!(addr & 4)) {
+		/* We only care about and preserve Aff0, Aff1 and Aff2. */
+		irq->mpidr = val & GENMASK(23, 0);
+		irq->target_vcpu = kvm_mpidr_to_vcpu(vcpu->kvm, irq->mpidr);
+	}
 
-	spin_unlock(&irq->irq_lock);
+	vgic_put_irq(irq);
 }
 
 static unsigned long vgic_mmio_read_v3r_typer(struct kvm_vcpu *vcpu,
@@ -441,9 +440,8 @@ void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg)
 
 		irq = vgic_get_irq(vcpu->kvm, c_vcpu, sgi);
 
-		spin_lock(&irq->irq_lock);
 		irq->pending = true;
 
-		vgic_queue_irq_unlock(vcpu->kvm, irq);
+		vgic_queue_irq_put(vcpu->kvm, irq);
 	}
 }
diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c
index 9f6fab7..4050c1c 100644
--- a/virt/kvm/arm/vgic/vgic-mmio.c
+++ b/virt/kvm/arm/vgic/vgic-mmio.c
@@ -56,6 +56,8 @@ unsigned long vgic_mmio_read_enable(struct kvm_vcpu *vcpu,
 
 		if (irq->enabled)
 			value |= (1U << i);
+
+		vgic_put_irq(irq);
 	}
 
 	return value;
@@ -71,9 +73,9 @@ void vgic_mmio_write_senable(struct kvm_vcpu *vcpu,
 	for_each_set_bit(i, &val, len * 8) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
-		spin_lock(&irq->irq_lock);
 		irq->enabled = true;
-		vgic_queue_irq_unlock(vcpu->kvm, irq);
+
+		vgic_queue_irq_put(vcpu->kvm, irq);
 	}
 }
 
@@ -87,11 +89,9 @@ void vgic_mmio_write_cenable(struct kvm_vcpu *vcpu,
 	for_each_set_bit(i, &val, len * 8) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
-		spin_lock(&irq->irq_lock);
-
 		irq->enabled = false;
 
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 	}
 }
 
@@ -108,6 +108,8 @@ unsigned long vgic_mmio_read_pending(struct kvm_vcpu *vcpu,
 
 		if (irq->pending)
 			value |= (1U << i);
+
+		vgic_put_irq(irq);
 	}
 
 	return value;
@@ -123,12 +125,11 @@ void vgic_mmio_write_spending(struct kvm_vcpu *vcpu,
 	for_each_set_bit(i, &val, len * 8) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
-		spin_lock(&irq->irq_lock);
 		irq->pending = true;
 		if (irq->config == VGIC_CONFIG_LEVEL)
 			irq->soft_pending = true;
 
-		vgic_queue_irq_unlock(vcpu->kvm, irq);
+		vgic_queue_irq_put(vcpu->kvm, irq);
 	}
 }
 
@@ -142,8 +143,6 @@ void vgic_mmio_write_cpending(struct kvm_vcpu *vcpu,
 	for_each_set_bit(i, &val, len * 8) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
-		spin_lock(&irq->irq_lock);
-
 		if (irq->config == VGIC_CONFIG_LEVEL) {
 			irq->soft_pending = false;
 			irq->pending = irq->line_level;
@@ -151,7 +150,7 @@ void vgic_mmio_write_cpending(struct kvm_vcpu *vcpu,
 			irq->pending = false;
 		}
 
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 	}
 }
 
@@ -168,15 +167,17 @@ unsigned long vgic_mmio_read_active(struct kvm_vcpu *vcpu,
 
 		if (irq->active)
 			value |= (1U << i);
+
+		vgic_put_irq(irq);
 	}
 
 	return value;
 }
 
-static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
-				    bool new_active_state)
+static void vgic_mmio_change_active_put(struct kvm_vcpu *vcpu,
+					struct vgic_irq *irq,
+					bool new_active_state)
 {
-	spin_lock(&irq->irq_lock);
 	/*
 	 * If this virtual IRQ was written into a list register, we
 	 * have to make sure the CPU that runs the VCPU thread has
@@ -190,15 +191,17 @@ static void vgic_mmio_change_active(struct kvm_vcpu *vcpu, struct vgic_irq *irq,
 	 * IRQ, so we release and re-acquire the spin_lock to let the
 	 * other thread sync back the IRQ.
 	 */
+	irq->refcnt++;
 	while (irq->vcpu && /* IRQ may have state in an LR somewhere */
 	       irq->vcpu->cpu != -1) /* VCPU thread is running */
 		cond_resched_lock(&irq->irq_lock);
 
+	irq->refcnt--;
 	irq->active = new_active_state;
 	if (new_active_state)
-		vgic_queue_irq_unlock(vcpu->kvm, irq);
+		vgic_queue_irq_put(vcpu->kvm, irq);
 	else
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 }
 
 /*
@@ -241,7 +244,8 @@ void vgic_mmio_write_cactive(struct kvm_vcpu *vcpu,
 	vgic_change_active_prepare(vcpu, intid);
 	for_each_set_bit(i, &val, len * 8) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
-		vgic_mmio_change_active(vcpu, irq, false);
+
+		vgic_mmio_change_active_put(vcpu, irq, false);
 	}
 	vgic_change_active_finish(vcpu, intid);
 }
@@ -256,7 +260,8 @@ void vgic_mmio_write_sactive(struct kvm_vcpu *vcpu,
 	vgic_change_active_prepare(vcpu, intid);
 	for_each_set_bit(i, &val, len * 8) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
-		vgic_mmio_change_active(vcpu, irq, true);
+
+		vgic_mmio_change_active_put(vcpu, irq, true);
 	}
 	vgic_change_active_finish(vcpu, intid);
 }
@@ -272,6 +277,8 @@ unsigned long vgic_mmio_read_priority(struct kvm_vcpu *vcpu,
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
 		val |= (u64)irq->priority << (i * 8);
+
+		vgic_put_irq(irq);
 	}
 
 	return val;
@@ -294,10 +301,10 @@ void vgic_mmio_write_priority(struct kvm_vcpu *vcpu,
 	for (i = 0; i < len; i++) {
 		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
 
-		spin_lock(&irq->irq_lock);
 		/* Narrow the priority range to what we actually support */
 		irq->priority = (val >> (i * 8)) & GENMASK(7, 8 - VGIC_PRI_BITS);
-		spin_unlock(&irq->irq_lock);
+
+		vgic_put_irq(irq);
 	}
 }
 
@@ -313,6 +320,8 @@ unsigned long vgic_mmio_read_config(struct kvm_vcpu *vcpu,
 
 		if (irq->config == VGIC_CONFIG_EDGE)
 			value |= (2U << (i * 2));
+
+		vgic_put_irq(irq);
 	}
 
 	return value;
@@ -326,7 +335,7 @@ void vgic_mmio_write_config(struct kvm_vcpu *vcpu,
 	int i;
 
 	for (i = 0; i < len * 4; i++) {
-		struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
+		struct vgic_irq *irq;
 
 		/*
 		 * The configuration cannot be changed for SGIs in general,
@@ -337,14 +346,16 @@ void vgic_mmio_write_config(struct kvm_vcpu *vcpu,
 		if (intid + i < VGIC_NR_PRIVATE_IRQS)
 			continue;
 
-		spin_lock(&irq->irq_lock);
+		irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i);
+
 		if (test_bit(i * 2 + 1, &val)) {
 			irq->config = VGIC_CONFIG_EDGE;
 		} else {
 			irq->config = VGIC_CONFIG_LEVEL;
 			irq->pending = irq->line_level | irq->soft_pending;
 		}
-		spin_unlock(&irq->irq_lock);
+
+		vgic_put_irq(irq);
 	}
 }
 
diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c
index 80313de..2147576 100644
--- a/virt/kvm/arm/vgic/vgic-v2.c
+++ b/virt/kvm/arm/vgic/vgic-v2.c
@@ -94,8 +94,6 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)
 
 		irq = vgic_get_irq(vcpu->kvm, vcpu, intid);
 
-		spin_lock(&irq->irq_lock);
-
 		/* Always preserve the active bit */
 		irq->active = !!(val & GICH_LR_ACTIVE_BIT);
 
@@ -123,7 +121,7 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu)
 			irq->pending = irq->line_level || irq->soft_pending;
 		}
 
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 	}
 }
 
diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c
index e48a22e..21d84e9 100644
--- a/virt/kvm/arm/vgic/vgic-v3.c
+++ b/virt/kvm/arm/vgic/vgic-v3.c
@@ -82,8 +82,6 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
 			intid = val & GICH_LR_VIRTUALID;
 		irq = vgic_get_irq(vcpu->kvm, vcpu, intid);
 
-		spin_lock(&irq->irq_lock);
-
 		/* Always preserve the active bit */
 		irq->active = !!(val & ICH_LR_ACTIVE_BIT);
 
@@ -112,7 +110,7 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu)
 			irq->pending = irq->line_level || irq->soft_pending;
 		}
 
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 	}
 }
 
diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index 69b61ab..90f2543 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -48,13 +48,20 @@ struct vgic_global __section(.hyp.text) kvm_vgic_global_state;
 struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct kvm_vcpu *vcpu,
 			      u32 intid)
 {
-	/* SGIs and PPIs */
-	if (intid <= VGIC_MAX_PRIVATE)
-		return &vcpu->arch.vgic_cpu.private_irqs[intid];
+	struct vgic_dist *dist = &kvm->arch.vgic;
+	struct vgic_irq *irq;
 
-	/* SPIs */
-	if (intid <= VGIC_MAX_SPI)
-		return &kvm->arch.vgic.spis[intid - VGIC_NR_PRIVATE_IRQS];
+	if (intid <= VGIC_MAX_PRIVATE) {        /* SGIs and PPIs */
+		irq = &vcpu->arch.vgic_cpu.private_irqs[intid];
+		spin_lock(&irq->irq_lock);
+		return irq;
+	}
+
+	if (intid <= VGIC_MAX_SPI) {            /* SPIs */
+		irq = &dist->spis[intid - VGIC_NR_PRIVATE_IRQS];
+		spin_lock(&irq->irq_lock);
+		return irq;
+	}
 
 	/* LPIs are not yet covered */
 	if (intid >= VGIC_MIN_LPI)
@@ -183,9 +190,10 @@ static bool vgic_validate_injection(struct vgic_irq *irq, bool level)
  * Needs to be entered with the IRQ lock already held, but will return
  * with all locks dropped.
  */
-bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq)
+bool vgic_queue_irq_put(struct kvm *kvm, struct vgic_irq *irq)
 {
-	struct kvm_vcpu *vcpu;
+	struct kvm_vcpu *vcpu, *irq_vcpu = irq->target_vcpu;
+	u32 intid = irq->intid;
 
 	DEBUG_SPINLOCK_BUG_ON(!spin_is_locked(&irq->irq_lock));
 
@@ -201,7 +209,7 @@ retry:
 		 * not need to be inserted into an ap_list and there is also
 		 * no more work for us to do.
 		 */
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 		return false;
 	}
 
@@ -209,12 +217,18 @@ retry:
 	 * We must unlock the irq lock to take the ap_list_lock where
 	 * we are going to insert this new pending interrupt.
 	 */
-	spin_unlock(&irq->irq_lock);
+	vgic_put_irq(irq);
 
 	/* someone can do stuff here, which we re-check below */
 
 	spin_lock(&vcpu->arch.vgic_cpu.ap_list_lock);
-	spin_lock(&irq->irq_lock);
+	irq = vgic_get_irq(kvm, irq_vcpu, intid);
+
+	if (!irq) {
+		/* The LPI has been unmapped, nothing left to do. */
+		spin_unlock(&vcpu->arch.vgic_cpu.ap_list_lock);
+		return false;
+	}
 
 	/*
 	 * Did something change behind our backs?
@@ -229,17 +243,21 @@ retry:
 	 */
 
 	if (unlikely(irq->vcpu || vcpu != vgic_target_oracle(irq))) {
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 		spin_unlock(&vcpu->arch.vgic_cpu.ap_list_lock);
 
-		spin_lock(&irq->irq_lock);
+		irq = vgic_get_irq(kvm, irq_vcpu, intid);
+		if (!irq)
+			return false;
+
 		goto retry;
 	}
 
 	list_add_tail(&irq->ap_list, &vcpu->arch.vgic_cpu.ap_list_head);
+	irq->refcnt++;
 	irq->vcpu = vcpu;
 
-	spin_unlock(&irq->irq_lock);
+	vgic_put_irq(irq);
 	spin_unlock(&vcpu->arch.vgic_cpu.ap_list_lock);
 
 	kvm_vcpu_kick(vcpu);
@@ -269,14 +287,14 @@ static int vgic_update_irq_pending(struct kvm *kvm, int cpuid,
 	if (!irq)
 		return -EINVAL;
 
-	if (irq->hw != mapped_irq)
+	if (irq->hw != mapped_irq) {
+		vgic_put_irq(irq);
 		return -EINVAL;
-
-	spin_lock(&irq->irq_lock);
+	}
 
 	if (!vgic_validate_injection(irq, level)) {
 		/* Nothing to see here, move along... */
-		spin_unlock(&irq->irq_lock);
+		vgic_put_irq(irq);
 		return 0;
 	}
 
@@ -287,7 +305,7 @@ static int vgic_update_irq_pending(struct kvm *kvm, int cpuid,
 		irq->pending = true;
 	}
 
-	vgic_queue_irq_unlock(kvm, irq);
+	vgic_queue_irq_put(kvm, irq);
 
 	return 0;
 }
@@ -324,31 +342,28 @@ int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, u32 virt_irq, u32 phys_irq)
 
 	BUG_ON(!irq);
 
-	spin_lock(&irq->irq_lock);
-
 	irq->hw = true;
 	irq->hwintid = phys_irq;
 
-	spin_unlock(&irq->irq_lock);
+	vgic_put_irq(irq);
 
 	return 0;
 }
 
 int kvm_vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, unsigned int virt_irq)
 {
-	struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, virt_irq);
-
-	BUG_ON(!irq);
+	struct vgic_irq *irq;
 
 	if (!vgic_initialized(vcpu->kvm))
 		return -EAGAIN;
 
-	spin_lock(&irq->irq_lock);
+	irq = vgic_get_irq(vcpu->kvm, vcpu, virt_irq);
+	BUG_ON(!irq);
 
 	irq->hw = false;
 	irq->hwintid = 0;
 
-	spin_unlock(&irq->irq_lock);
+	vgic_put_irq(irq);
 
 	return 0;
 }
@@ -378,14 +393,21 @@ retry:
 
 		target_vcpu = vgic_target_oracle(irq);
 
-		if (!target_vcpu) {
+		if (!target_vcpu || irq->refcnt == 1) {
+			bool free_irq = false;
+
 			/*
 			 * We don't need to process this interrupt any
 			 * further, move it off the list.
 			 */
 			list_del(&irq->ap_list);
 			irq->vcpu = NULL;
+			irq->refcnt--;
+			if (!irq->refcnt)
+				free_irq = true;
 			spin_unlock(&irq->irq_lock);
+			if (free_irq)
+				kfree(irq);
 			continue;
 		}
 
@@ -611,9 +633,8 @@ bool kvm_vgic_map_is_active(struct kvm_vcpu *vcpu, unsigned int virt_irq)
 	struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, virt_irq);
 	bool map_is_active;
 
-	spin_lock(&irq->irq_lock);
 	map_is_active = irq->hw && irq->active;
-	spin_unlock(&irq->irq_lock);
+	vgic_put_irq(irq);
 
 	return map_is_active;
 }
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index c752152..fa2d225 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -38,7 +38,12 @@ struct vgic_vmcr {
 
 struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct kvm_vcpu *vcpu,
 			      u32 intid);
-bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq);
+static inline void vgic_put_irq(struct vgic_irq *irq)
+{
+	spin_unlock(&irq->irq_lock);
+}
+
+bool vgic_queue_irq_put(struct kvm *kvm, struct vgic_irq *irq);
 void vgic_kick_vcpus(struct kvm *kvm);
 
 void vgic_v2_process_maintenance(struct kvm_vcpu *vcpu);
-- 
2.8.2

  parent reply	other threads:[~2016-06-17 12:08 UTC|newest]

Thread overview: 56+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-06-17 12:08 [PATCH v6 00/15] KVM: arm64: GICv3 ITS emulation Andre Przywara
2016-06-17 12:08 ` Andre Przywara
2016-06-17 12:08 ` [PATCH v6 01/15] KVM: arm/arm64: move redistributor kvm_io_devices Andre Przywara
2016-06-17 12:08   ` Andre Przywara
2016-06-17 12:08 ` [PATCH v6 02/15] KVM: arm/arm64: check return value for kvm_register_vgic_device Andre Przywara
2016-06-17 12:08   ` Andre Przywara
2016-06-17 12:08 ` [PATCH v6 03/15] KVM: extend struct kvm_msi to hold a 32-bit device ID Andre Przywara
2016-06-17 12:08   ` Andre Przywara
2016-06-17 12:08 ` [PATCH v6 04/15] KVM: arm/arm64: extend arch CAP checks to allow per-VM capabilities Andre Przywara
2016-06-17 12:08   ` Andre Przywara
2016-06-17 12:08 ` Andre Przywara [this message]
2016-06-17 12:08   ` [PATCH v6 05/15] KVM: arm/arm64: VGIC: add refcounting for IRQs Andre Przywara
2016-06-22  8:18   ` Andre Przywara
2016-06-22  8:18     ` Andre Przywara
2016-06-22  8:26     ` Marc Zyngier
2016-06-22  8:26       ` Marc Zyngier
2016-06-17 12:08 ` [PATCH v6 06/15] KVM: arm64: handle ITS related GICv3 redistributor registers Andre Przywara
2016-06-17 12:08   ` Andre Przywara
2016-06-22 14:07   ` Marc Zyngier
2016-06-22 14:07     ` Marc Zyngier
2016-06-22 14:39     ` Andre Przywara
2016-06-22 14:39       ` Andre Przywara
2016-06-22 14:59       ` Marc Zyngier
2016-06-22 14:59         ` Marc Zyngier
2016-06-17 12:08 ` [PATCH v6 07/15] KVM: arm64: introduce ITS emulation file with MMIO framework Andre Przywara
2016-06-17 12:08   ` Andre Przywara
2016-06-22 14:48   ` Marc Zyngier
2016-06-22 14:48     ` Marc Zyngier
2016-06-22 15:03     ` Andre Przywara
2016-06-22 15:03       ` Andre Przywara
2016-06-22 15:24       ` Marc Zyngier
2016-06-22 15:24         ` Marc Zyngier
2016-06-17 12:08 ` [PATCH v6 08/15] KVM: arm64: introduce new KVM ITS device Andre Przywara
2016-06-17 12:08   ` Andre Przywara
2016-06-22 15:23   ` Marc Zyngier
2016-06-22 15:23     ` Marc Zyngier
2016-06-17 12:08 ` [PATCH v6 09/15] KVM: arm64: implement basic ITS register handlers Andre Przywara
2016-06-17 12:08   ` Andre Przywara
2016-06-22 16:19   ` Marc Zyngier
2016-06-22 16:19     ` Marc Zyngier
2016-06-17 12:08 ` [PATCH v6 10/15] KVM: arm64: connect LPIs to the VGIC emulation Andre Przywara
2016-06-17 12:08   ` Andre Przywara
2016-06-22 16:26   ` Marc Zyngier
2016-06-22 16:26     ` Marc Zyngier
2016-06-22 17:02     ` Marc Zyngier
2016-06-22 17:02       ` Marc Zyngier
2016-06-17 12:08 ` [PATCH v6 11/15] KVM: arm64: read initial LPI pending table Andre Przywara
2016-06-17 12:08   ` Andre Przywara
2016-06-17 12:08 ` [PATCH v6 12/15] KVM: arm64: allow updates of LPI configuration table Andre Przywara
2016-06-17 12:08   ` Andre Przywara
2016-06-17 12:08 ` [PATCH v6 13/15] KVM: arm64: implement ITS command queue command handlers Andre Przywara
2016-06-17 12:08   ` Andre Przywara
2016-06-17 12:08 ` [PATCH v6 14/15] KVM: arm64: implement MSI injection in ITS emulation Andre Przywara
2016-06-17 12:08   ` Andre Przywara
2016-06-17 12:08 ` [PATCH v6 15/15] KVM: arm64: enable ITS emulation as a virtual MSI controller Andre Przywara
2016-06-17 12:08   ` Andre Przywara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1466165327-32060-6-git-send-email-andre.przywara@arm.com \
    --to=andre.przywara@arm.com \
    --cc=christoffer.dall@linaro.org \
    --cc=eric.auger@redhat.com \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=marc.zyngier@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.