Thread (25 messages) 25 messages, 2 authors, 2016-06-08
STALE3675d

[PATCH v5 09/13] KVM: arm64: connect LPIs to the VGIC emulation

From: Marc Zyngier <hidden>
Date: 2016-06-08 13:29:54
Also in: kvm, kvmarm

On 03/06/16 15:02, Andre Przywara wrote:
quoted hunk ↗ jump to hunk
LPIs are dynamically created (mapped) at guest runtime and their
actual numbers can be quite high, but is mostly assigned using a very
sparse allocation scheme. So arrays are not an ideal data structure
to hold the information. We use our equivalent of the "Interrupt
Translation Table Entry" (ITTE) to hold the vgic_irq struct for a
virtual LPI embedded in in the ITTE.
Connect the VGIC core code via an accessor function to help it get the
struct vgic_irq for a certain LPI.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 virt/kvm/arm/vgic/vgic-its.c | 34 ++++++++++++++++++++++++++++++++++
 virt/kvm/arm/vgic/vgic.c     |  2 +-
 virt/kvm/arm/vgic/vgic.h     |  6 ++++++
 3 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c
index 3ec12ef..4f248ef 100644
--- a/virt/kvm/arm/vgic/vgic-its.c
+++ b/virt/kvm/arm/vgic/vgic-its.c
@@ -68,11 +68,29 @@ struct its_collection {
 struct its_itte {
 	struct list_head itte_list;
 
+	struct vgic_irq irq;
 	struct its_collection *collection;
 	u32 lpi;
 	u32 event_id;
 };
 
+/* To be used as an iterator this macro misses the enclosing parentheses */
+#define for_each_lpi(dev, itte, its) \
+	list_for_each_entry(dev, &(its)->device_list, dev_list) \
+		list_for_each_entry(itte, &(dev)->itt, itte_list)
Well, this is not really "for each LPI". This is "for each LPI that can
be generated by this ITS". Are you sure that you can always do this on a
per-ITS basis? In other words, while this work for a direct translation,
it doesn't work for a reverse one. Do we have any such case?
quoted hunk ↗ jump to hunk
+
+static struct its_itte *find_itte_by_lpi(struct vgic_its *its, int lpi)
+{
+	struct its_device *device;
+	struct its_itte *itte;
+
+	for_each_lpi(device, itte, its) {
+		if (itte->lpi == lpi)
+			return itte;
+	}
+	return NULL;
+}
+
 #define BASER_BASE_ADDRESS(x) ((x) & 0xfffffffff000ULL)
 
 #define ITS_FRAME(addr) ((addr) & ~(SZ_64K - 1))
@@ -158,6 +176,22 @@ static unsigned long vgic_mmio_read_its_idregs(struct kvm_vcpu *vcpu,
 	return 0;
 }
 
+struct vgic_irq *vgic_its_get_lpi(struct kvm *kvm, u32 intid)
+{
+	struct vgic_its *its;
+	struct its_itte *itte;
+
+	list_for_each_entry(its, &kvm->arch.vits_list, its_list) {
+		itte = find_itte_by_lpi(its, intid);
+		if (!itte)
+			continue;
+
+		return &itte->irq;
Or rather
		if (itte)
			return &itte->irq;

This function implements the case I was worried about above. It would be
worth mentioning that this *only* works because of 6.1.1 in the
architecture spec (an LPI can only be generated by a single EID/DID pair).

What doesn't really work here is that you are allowed to program this
EID/DID->LPI translation on several ITSs (think of a device moving its
doorbell from one ITS to another), which means that you cannot store the
vgic_irq in the ITE. Instead, this must be a pointer to IRQ, and the
interrupt as part of a separate list.
quoted hunk ↗ jump to hunk
+	}
+
+	return NULL;
+}
+
 static void its_free_itte(struct its_itte *itte)
 {
 	list_del(&itte->itte_list);
diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c
index 69b61ab..6812ff1 100644
--- a/virt/kvm/arm/vgic/vgic.c
+++ b/virt/kvm/arm/vgic/vgic.c
@@ -58,7 +58,7 @@ struct vgic_irq *vgic_get_irq(struct kvm *kvm, struct kvm_vcpu *vcpu,
 
 	/* LPIs are not yet covered */
 	if (intid >= VGIC_MIN_LPI)
-		return NULL;
+		return vgic_its_get_lpi(kvm, intid);
 
 	WARN(1, "Looking up struct vgic_irq for reserved INTID");
 	return NULL;
diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h
index 66578d2..6fecd70 100644
--- a/virt/kvm/arm/vgic/vgic.h
+++ b/virt/kvm/arm/vgic/vgic.h
@@ -78,6 +78,7 @@ bool vgic_has_its(struct kvm *kvm);
 int vits_init(struct kvm *kvm, struct vgic_its *its);
 void vits_destroy(struct kvm *kvm, struct vgic_its *its);
 int kvm_vgic_register_its_device(void);
+struct vgic_irq *vgic_its_get_lpi(struct kvm *kvm, u32 intid);
 #else
 static inline void vgic_v3_process_maintenance(struct kvm_vcpu *vcpu)
 {
@@ -148,6 +149,11 @@ static int kvm_vgic_register_its_device(void)
 {
 	return -ENODEV;
 }
+
+static inline struct vgic_irq *vgic_its_get_lpi(struct kvm *kvm, u32 intid)
+{
+	return NULL;
+}
 #endif
 
 int kvm_register_vgic_device(unsigned long type);
Thanks,

	M.
-- 
Jazz is not dead. It just smells funny...
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help