Skip to contentSkip to conversation

qemu-kvm: enable msi with irqchip

PATCH Superseded Michael S. Tsirkin 2 participants 6 messages Jul 5, 2009 – Jul 7, 2009 KVM

v3 of 4 Superseded Latest is v4 →

lore
Share & tools
lore
#0 lore raw

PATCH v3 1/1 qemu-kvm: enable msi with irqchip

Support msi-x with irqchip in kernel: allocate entries when they are used, and update when they are unmasked.

---

Changes since v2:
move all kvm-specific code into kvm_msi_XXX helpers
Changes since v1:
add braces as per CODING_STYLE

 hw/apic.c    |    4 +-
 hw/msix.c    |  123 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
 hw/pci.h     |    3 +
 libkvm-all.h |   14 +++++++
 qemu-kvm.c   |   54 +++++++++++++++++++++++++
 5 files changed, 193 insertions(+), 5 deletions(-)
hw/apic.c +1 -3
index 778a853..cdb5972 100644
@@ -1088,9 +1088,7 @@ int apic_init(CPUState *env)
     s->cpu_env = env;
 
     apic_reset(s);
-    if (!kvm_enabled() || !qemu_kvm_irqchip_in_kernel()) {
-        msix_supported = 1;
-    }
+    msix_supported = 1;
 
     /* XXX: mapping more APICs at the same memory location */
     if (apic_io_memory == 0) {
hw/msix.c +121 -2

177 lines not on this page · raw message

hw/pci.h +3 -0
index 7ae9c93..66235e4 100644
@@ -5,6 +5,8 @@
 
 #include "qdev.h"
 
+struct kvm_irq_routing_entry;
+
 /* PCI includes legacy ISA access.  */
 #include "isa.h"
 
@@ -232,6 +234,7 @@ struct PCIDevice {
     unsigned *msix_entry_used;
     /* Region including the MSI-X table */
     uint32_t msix_bar_size;
+    struct kvm_irq_routing_entry *msix_irq_entries;
 
     /* Device capability configuration space */
     struct {
libkvm-all.h +14 -0
index ecd3065..4a98bcb 100644
@@ -898,6 +898,20 @@ int kvm_del_routing_entry(kvm_context_t kvm,
 		          struct kvm_irq_routing_entry* entry);
 
 /*!
+ * \brief Updates a routing in the temporary irq routing table
+ *
+ * Update a routing in the temporary irq routing table
+ * with a new value. entry type and GSI can not be changed.
+ * Nothing is committed to the running VM.
+ *
+ * \param kvm Pointer to the current kvm_context
+ */
+int kvm_update_routing_entry(kvm_context_t kvm,
+                             struct kvm_irq_routing_entry* entry,
+                             struct kvm_irq_routing_entry* newentry
+);
+
+/*!
  * \brief Commit the temporary irq routing table
  *
  * Commit the temporary irq routing table to the running VM.
qemu-kvm.c +54 -0

60 lines not on this page · raw message

Signature · 1 line
1.6.2.5
#1 lore raw

hw/msix.c void msix_notify(PCIDevice *dev, unsigned vector) · line 452

Quoted 10 lines · Michael S. Tsirkin

Support msi-x with irqchip in kernel: allocate entries when they are used, and update when they are unmasked.

hw/msix.c +2 -0
@@ -340,6 +447,10 @@ void msix_notify(PCIDevice *dev, unsigned vector)
          msix_set_pending(dev, vector);
          return;
      }
+    if (kvm_enabled()&&  qemu_kvm_irqchip_in_kernel()) {
+        kvm_set_irq(dev->msix_irq_entries[vector].gsi, 1, NULL);
 

Toggle back to zero after setting to one, for consistency.

Quoted 25 lines
qemu-kvm.c +19 -0
@@ -1448,6 +1448,60 @@ int kvm_del_routing_entry(kvm_context_t kvm,
  #endif
  }
 
+int kvm_update_routing_entry(kvm_context_t kvm,
+	                  struct kvm_irq_routing_entry* entry,
+                          struct kvm_irq_routing_entry* newentry)
+{
+#ifdef KVM_CAP_IRQ_ROUTING
+	struct kvm_irq_routing_entry *e;
+	int i, gsi, found = 0;
+
+        if (entry->gsi != newentry->gsi ||
+            entry->type != newentry->type) {
+            return -EINVAL;
+        }
+	gsi = entry->gsi;
+
+	for (i = 0; i<  kvm->irq_routes->nr; ++i) {
+		e =&kvm->irq_routes->entries[i];
+		if (e->type != entry->type || e->gsi != gsi) {
+                    continue;
+                }

qemu-kvm.c int kvm_del_routing_entry(kvm_context_t kvm, · line 1479

Quoted 10 lines
qemu-kvm.c +9 -0
+		switch (e->type)
+		{
+		case KVM_IRQ_ROUTING_IRQCHIP: {
+			if (e->u.irqchip.irqchip ==
+			    entry->u.irqchip.irqchip
+			&&  e->u.irqchip.pin ==
+			    entry->u.irqchip.pin) {
+			    found = 1;
+                        }
 

this
is
not
readable

Quoted 16 lines
qemu-kvm.c +16 -0
+			break;
+		}
+		case KVM_IRQ_ROUTING_MSI: {
+			if (e->u.msi.address_lo ==
+			    entry->u.msi.address_lo
+			&&  e->u.msi.address_hi ==
+			    entry->u.msi.address_hi
+			&&  e->u.msi.data == entry->u.msi.data) {
+			    found = 1;
+                        }
+			break;
+		}
+		default:
+			break;
+		}
+		if (found) {

qemu-kvm.c int kvm_del_routing_entry(kvm_context_t kvm,

Quoted 10 lines
qemu-kvm.c +9 -0
+			memcpy(e, entry, sizeof *e);
+			return 0;
+		}
+	}
+	return -ESRCH;
+#else
+	return -ENOSYS;
+#endif
+}
 

Please detab the whole thing.

You use perror() on functions that return -errno; please fix.

Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
#2 lore raw
Quoted 4 lines · Avi Kivity
Quoted 3 lines · Michael S. Tsirkin

Support msi-x with irqchip in kernel: allocate entries when they are used, and update when they are unmasked.

Quoted 9 lines

hw/msix.c void msix_notify(PCIDevice *dev, unsigned vector) · line 452

Quoted 7 lines
hw/msix.c +2 -0
@@ -340,6 +447,10 @@ void msix_notify(PCIDevice *dev, unsigned vector)
          msix_set_pending(dev, vector);
          return;
      }
+    if (kvm_enabled()&&  qemu_kvm_irqchip_in_kernel()) {
+        kvm_set_irq(dev->msix_irq_entries[vector].gsi, 1, NULL);
 

Toggle back to zero after setting to one, for consistency.

You don't really want that. It's an extra system call, on a data path operation.
KVM knows it is MSI vector, and so can be relied upon to do the right thing. Only value 1 should be legal for MSI, I'll send a patch to fail on 0.

Quoted 74 lines
Quoted 25 lines
qemu-kvm.c +19 -0
@@ -1448,6 +1448,60 @@ int kvm_del_routing_entry(kvm_context_t kvm,
  #endif
  }
 
+int kvm_update_routing_entry(kvm_context_t kvm,
+	                  struct kvm_irq_routing_entry* entry,
+                          struct kvm_irq_routing_entry* newentry)
+{
+#ifdef KVM_CAP_IRQ_ROUTING
+	struct kvm_irq_routing_entry *e;
+	int i, gsi, found = 0;
+
+        if (entry->gsi != newentry->gsi ||
+            entry->type != newentry->type) {
+            return -EINVAL;
+        }
+	gsi = entry->gsi;
+
+	for (i = 0; i<  kvm->irq_routes->nr; ++i) {
+		e =&kvm->irq_routes->entries[i];
+		if (e->type != entry->type || e->gsi != gsi) {
+                    continue;
+                }

qemu-kvm.c int kvm_del_routing_entry(kvm_context_t kvm, · line 1479

Quoted 10 lines
qemu-kvm.c +9 -0
+		switch (e->type)
+		{
+		case KVM_IRQ_ROUTING_IRQCHIP: {
+			if (e->u.irqchip.irqchip ==
+			    entry->u.irqchip.irqchip
+			&&  e->u.irqchip.pin ==
+			    entry->u.irqchip.pin) {
+			    found = 1;
+                        }
 

this
is
not
readable

Quoted 16 lines
qemu-kvm.c +16 -0
+			break;
+		}
+		case KVM_IRQ_ROUTING_MSI: {
+			if (e->u.msi.address_lo ==
+			    entry->u.msi.address_lo
+			&&  e->u.msi.address_hi ==
+			    entry->u.msi.address_hi
+			&&  e->u.msi.data == entry->u.msi.data) {
+			    found = 1;
+                        }
+			break;
+		}
+		default:
+			break;
+		}
+		if (found) {

qemu-kvm.c int kvm_del_routing_entry(kvm_context_t kvm,

Quoted 10 lines
qemu-kvm.c +9 -0
+			memcpy(e, entry, sizeof *e);
+			return 0;
+		}
+	}
+	return -ESRCH;
+#else
+	return -ENOSYS;
+#endif
+}
 

Please detab the whole thing.

You use perror() on functions that return -errno; please fix.

Signature · 1 line
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
#3 lore raw
Quoted 18 lines · Michael S. Tsirkin
Quoted 5 lines · Avi Kivity
Quoted 3 lines · Michael S. Tsirkin

Support msi-x with irqchip in kernel: allocate entries when they are used, and update when they are unmasked.

Quoted 10 lines

hw/msix.c void msix_notify(PCIDevice *dev, unsigned vector) · line 453

Quoted 8 lines
hw/msix.c +2 -0
@@ -340,6 +447,10 @@ void msix_notify(PCIDevice *dev, unsigned vector)
           msix_set_pending(dev, vector);
           return;
       }
+    if (kvm_enabled()&&   qemu_kvm_irqchip_in_kernel()) {
+        kvm_set_irq(dev->msix_irq_entries[vector].gsi, 1, NULL);
 
 

Toggle back to zero after setting to one, for consistency.

Quoted 3 lines

You don't really want that. It's an extra system call, on a data path operation.

Yes I do. I don't want 'level' to mean one thing for msi and another for other interrupts.

If the extra system call ends up hurting, we can add KVM_TOGGLE_IRQ or define a new level that means 'toggle'.

Signature · 1 line
error compiling committee.c: too many arguments to function
#4 lore raw
In reply to Avi Kivity
Quoted 46 lines · Avi Kivity

hw/msix.c void msix_notify(PCIDevice *dev, unsigned vector) · line 452

Quoted 10 lines · Michael S. Tsirkin

Support msi-x with irqchip in kernel: allocate entries when they are used, and update when they are unmasked.

hw/msix.c +2 -0
@@ -340,6 +447,10 @@ void msix_notify(PCIDevice *dev, unsigned vector)
          msix_set_pending(dev, vector);
          return;
      }
+    if (kvm_enabled()&&  qemu_kvm_irqchip_in_kernel()) {
+        kvm_set_irq(dev->msix_irq_entries[vector].gsi, 1, NULL);
 

Toggle back to zero after setting to one, for consistency.

Quoted 32 lines
qemu-kvm.c +26 -0
@@ -1448,6 +1448,60 @@ int kvm_del_routing_entry(kvm_context_t kvm,
  #endif
  }
 
+int kvm_update_routing_entry(kvm_context_t kvm,
+	                  struct kvm_irq_routing_entry* entry,
+                          struct kvm_irq_routing_entry* newentry)
+{
+#ifdef KVM_CAP_IRQ_ROUTING
+	struct kvm_irq_routing_entry *e;
+	int i, gsi, found = 0;
+
+        if (entry->gsi != newentry->gsi ||
+            entry->type != newentry->type) {
+            return -EINVAL;
+        }
+	gsi = entry->gsi;
+
+	for (i = 0; i<  kvm->irq_routes->nr; ++i) {
+		e =&kvm->irq_routes->entries[i];
+		if (e->type != entry->type || e->gsi != gsi) {
+                    continue;
+                }
+		switch (e->type)
+		{
+		case KVM_IRQ_ROUTING_IRQCHIP: {
+			if (e->u.irqchip.irqchip ==
+			    entry->u.irqchip.irqchip
+			&&  e->u.irqchip.pin ==
+			    entry->u.irqchip.pin) {
Quoted 8 lines

qemu-kvm.c int kvm_del_routing_entry(kvm_context_t kvm,

Quoted 3 lines
qemu-kvm.c +2 -0
+			    found = 1;
+                        }
 

this
is
not
readable

OK, but other functions in this file look the same.

Quoted 29 lines
Quoted 16 lines
qemu-kvm.c +16 -0
+			break;
+		}
+		case KVM_IRQ_ROUTING_MSI: {
+			if (e->u.msi.address_lo ==
+			    entry->u.msi.address_lo
+			&&  e->u.msi.address_hi ==
+			    entry->u.msi.address_hi
+			&&  e->u.msi.data == entry->u.msi.data) {
+			    found = 1;
+                        }
+			break;
+		}
+		default:
+			break;
+		}
+		if (found) {

qemu-kvm.c int kvm_del_routing_entry(kvm_context_t kvm,

Quoted 10 lines
qemu-kvm.c +9 -0
+			memcpy(e, entry, sizeof *e);
+			return 0;
+		}
+	}
+	return -ESRCH;
+#else
+	return -ENOSYS;
+#endif
+}
 

Please detab the whole thing.

Quoted 1 line

You use perror() on functions that return -errno; please fix.

Should I send output to stderr or to console, btw?

Quoted 3 lines
Signature · 2 lines
-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.
#5 lore raw
Quoted 18 lines · Michael S. Tsirkin
Quoted 17 lines

qemu-kvm.c int kvm_del_routing_entry(kvm_context_t kvm, · line 1478

Quoted 12 lines
qemu-kvm.c +9 -0
+		switch (e->type)
+		{
+		case KVM_IRQ_ROUTING_IRQCHIP: {
+			if (e->u.irqchip.irqchip ==
+			    entry->u.irqchip.irqchip
+			&&   e->u.irqchip.pin ==
+			    entry->u.irqchip.pin) {
+			    found = 1;
+                        }

this
is
not
readable

Quoted 2 lines

OK, but other functions in this file look the same.

I don't know how they sneaked them past me. Ignore the others.

Quoted 5 lines
Quoted 2 lines

You use perror() on functions that return -errno; please fix.

Should I send output to stderr or to console, btw?

stderr for now. When we have the new monitor protocol we can define a notification for logging random errors.

Signature · 1 line
error compiling committee.c: too many arguments to function