All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] msix/kvm integration cleanups
@ 2010-09-20 15:06 Avi Kivity
  2010-09-20 15:06 ` [PATCH 1/9] msix: avoid leaking kvm data on init failure Avi Kivity
                   ` (9 more replies)
  0 siblings, 10 replies; 19+ messages in thread
From: Avi Kivity @ 2010-09-20 15:06 UTC (permalink / raw)
  To: Marcelo Tosatti, kvm, Michael S. Tsirkin

This cleans up msix/kvm integration a bit.  The really important patch is the
last one, which allows msix.o to be part of non-target-specific build.

Avi Kivity (9):
  msix: avoid leaking kvm data on init failure
  msix: make kvm specific initialization a function
  msix: move kvm specific msix notify into a function
  msix: move definitions from msix.c to msix.h
  Add missing #include to hw/irq.h
  msix: move kvm support functions to kvm-all.c and kvm-stub.c
  msix: Move kvm_enabled() guards to kvm-all.c functions
  Protect qemu-kvm.h declarations with NEED_CPU_H
  Move msix.o build back to Makefile.objs

 Makefile.objs   |    4 +-
 Makefile.target |    4 -
 hw/irq.h        |    2 +
 hw/msix.c       |  173 ++++---------------------------------------------------
 hw/msix.h       |   26 ++++++++
 kvm-all.c       |  142 +++++++++++++++++++++++++++++++++++++++++++++
 kvm-stub.c      |   29 +++++++++
 kvm.h           |   16 +++++
 qemu-kvm.h      |   21 ++++++-
 9 files changed, 247 insertions(+), 170 deletions(-)

-- 
1.7.2.3


^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH 1/9] msix: avoid leaking kvm data on init failure
  2010-09-20 15:06 [PATCH 0/9] msix/kvm integration cleanups Avi Kivity
@ 2010-09-20 15:06 ` Avi Kivity
  2010-09-20 15:06 ` [PATCH 2/9] msix: make kvm specific initialization a function Avi Kivity
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Avi Kivity @ 2010-09-20 15:06 UTC (permalink / raw)
  To: Marcelo Tosatti, kvm, Michael S. Tsirkin

Move initialization after we're certain to succeed, so we don't leak
memory on failure.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 hw/msix.c |   13 +++++++------
 1 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index 3dd0456..312439a 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -373,12 +373,6 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries,
     if (nentries > MSIX_MAX_ENTRIES)
         return -EINVAL;
 
-#ifdef KVM_CAP_IRQCHIP
-    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
-        dev->msix_irq_entries = qemu_malloc(nentries *
-                                            sizeof *dev->msix_irq_entries);
-    }
-#endif
     dev->msix_mask_notifier_opaque =
         qemu_mallocz(nentries * sizeof *dev->msix_mask_notifier_opaque);
     dev->msix_mask_notifier = NULL;
@@ -400,6 +394,13 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries,
     if (ret)
         goto err_config;
 
+#ifdef KVM_CAP_IRQCHIP
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
+        dev->msix_irq_entries = qemu_malloc(nentries *
+                                            sizeof *dev->msix_irq_entries);
+    }
+#endif
+
     dev->cap_present |= QEMU_PCI_CAP_MSIX;
     return 0;
 
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 2/9] msix: make kvm specific initialization a function
  2010-09-20 15:06 [PATCH 0/9] msix/kvm integration cleanups Avi Kivity
  2010-09-20 15:06 ` [PATCH 1/9] msix: avoid leaking kvm data on init failure Avi Kivity
@ 2010-09-20 15:06 ` Avi Kivity
  2010-09-20 15:06 ` [PATCH 3/9] msix: move kvm specific msix notify into " Avi Kivity
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Avi Kivity @ 2010-09-20 15:06 UTC (permalink / raw)
  To: Marcelo Tosatti, kvm, Michael S. Tsirkin

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 hw/msix.c |   19 +++++++++++++------
 1 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index 312439a..2e2ce5a 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -48,6 +48,17 @@ int msix_supported;
 
 #ifdef CONFIG_KVM
 /* KVM specific MSIX helpers */
+
+static void kvm_msix_init(PCIDevice *dev)
+{
+#ifdef KVM_CAP_IRQCHIP
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
+        dev->msix_irq_entries = qemu_malloc(dev->msix_entries_nr *
+                                            sizeof *dev->msix_irq_entries);
+    }
+#endif
+}
+
 static void kvm_msix_free(PCIDevice *dev)
 {
     int vector, changed = 0;
@@ -150,6 +161,7 @@ static void kvm_msix_del(PCIDevice *dev, unsigned vector)
 }
 #else
 
+static void kvm_msix_init(PCIDevice *dev) {}
 static void kvm_msix_free(PCIDevice *dev) {}
 static void kvm_msix_update(PCIDevice *dev, int vector,
                             int was_masked, int is_masked) {}
@@ -394,12 +406,7 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries,
     if (ret)
         goto err_config;
 
-#ifdef KVM_CAP_IRQCHIP
-    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
-        dev->msix_irq_entries = qemu_malloc(nentries *
-                                            sizeof *dev->msix_irq_entries);
-    }
-#endif
+    kvm_msix_init(dev);
 
     dev->cap_present |= QEMU_PCI_CAP_MSIX;
     return 0;
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 3/9] msix: move kvm specific msix notify into a function
  2010-09-20 15:06 [PATCH 0/9] msix/kvm integration cleanups Avi Kivity
  2010-09-20 15:06 ` [PATCH 1/9] msix: avoid leaking kvm data on init failure Avi Kivity
  2010-09-20 15:06 ` [PATCH 2/9] msix: make kvm specific initialization a function Avi Kivity
@ 2010-09-20 15:06 ` Avi Kivity
  2010-09-20 15:06 ` [PATCH 4/9] msix: move definitions from msix.c to msix.h Avi Kivity
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Avi Kivity @ 2010-09-20 15:06 UTC (permalink / raw)
  To: Marcelo Tosatti, kvm, Michael S. Tsirkin

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 hw/msix.c |   19 +++++++++++++++----
 1 files changed, 15 insertions(+), 4 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index 2e2ce5a..d762870 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -159,6 +159,18 @@ static void kvm_msix_del(PCIDevice *dev, unsigned vector)
     kvm_del_routing_entry(kvm_context, &dev->msix_irq_entries[vector]);
     kvm_commit_irq_routes(kvm_context);
 }
+
+static bool kvm_msix_notify(PCIDevice *dev, unsigned vector)
+{
+#ifdef KVM_CAP_IRQCHIP
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
+        kvm_set_irq(dev->msix_irq_entries[vector].gsi, 1, NULL);
+        return true;
+    }
+#endif
+    return false;
+}
+
 #else
 
 static void kvm_msix_init(PCIDevice *dev) {}
@@ -167,6 +179,8 @@ static void kvm_msix_update(PCIDevice *dev, int vector,
                             int was_masked, int is_masked) {}
 static int kvm_msix_add(PCIDevice *dev, unsigned vector) { return -1; }
 static void kvm_msix_del(PCIDevice *dev, unsigned vector) {}
+static bool kvm_msix_notify(PCIDevice *dev, unsigned vector) { return false; }
+
 #endif
 
 /* Add MSI-X capability to the config space for the device. */
@@ -525,12 +539,9 @@ void msix_notify(PCIDevice *dev, unsigned vector)
         return;
     }
 
-#ifdef KVM_CAP_IRQCHIP
-    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
-        kvm_set_irq(dev->msix_irq_entries[vector].gsi, 1, NULL);
+    if (kvm_msix_notify(dev, vector)) {
         return;
     }
-#endif
 
     address = pci_get_long(table_entry + MSIX_MSG_UPPER_ADDR);
     address = (address << 32) | pci_get_long(table_entry + MSIX_MSG_ADDR);
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 4/9] msix: move definitions from msix.c to msix.h
  2010-09-20 15:06 [PATCH 0/9] msix/kvm integration cleanups Avi Kivity
                   ` (2 preceding siblings ...)
  2010-09-20 15:06 ` [PATCH 3/9] msix: move kvm specific msix notify into " Avi Kivity
@ 2010-09-20 15:06 ` Avi Kivity
  2010-09-20 16:56   ` Michael S. Tsirkin
  2010-09-20 15:06 ` [PATCH 5/9] Add missing #include to hw/irq.h Avi Kivity
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 19+ messages in thread
From: Avi Kivity @ 2010-09-20 15:06 UTC (permalink / raw)
  To: Marcelo Tosatti, kvm, Michael S. Tsirkin

This allows us to reuse them from the kvm support code.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 hw/msix.c |   27 ---------------------------
 hw/msix.h |   26 ++++++++++++++++++++++++++
 2 files changed, 26 insertions(+), 27 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index d762870..dda1a24 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -16,33 +16,6 @@
 #include "pci.h"
 #include "kvm.h"
 
-/* MSI-X capability structure */
-#define MSIX_TABLE_OFFSET 4
-#define MSIX_PBA_OFFSET 8
-#define MSIX_CAP_LENGTH 12
-
-/* MSI enable bit and maskall bit are in byte 1 in FLAGS register */
-#define MSIX_CONTROL_OFFSET (PCI_MSIX_FLAGS + 1)
-#define MSIX_ENABLE_MASK (PCI_MSIX_FLAGS_ENABLE >> 8)
-#define MSIX_MASKALL_MASK (PCI_MSIX_FLAGS_MASKALL >> 8)
-
-/* MSI-X table format */
-#define MSIX_MSG_ADDR 0
-#define MSIX_MSG_UPPER_ADDR 4
-#define MSIX_MSG_DATA 8
-#define MSIX_VECTOR_CTRL 12
-#define MSIX_ENTRY_SIZE 16
-#define MSIX_VECTOR_MASK 0x1
-
-/* How much space does an MSIX table need. */
-/* The spec requires giving the table structure
- * a 4K aligned region all by itself. */
-#define MSIX_PAGE_SIZE 0x1000
-/* Reserve second half of the page for pending bits */
-#define MSIX_PAGE_PENDING (MSIX_PAGE_SIZE / 2)
-#define MSIX_MAX_ENTRIES 32
-
-
 /* Flag for interrupt controller to declare MSI-X support */
 int msix_supported;
 
diff --git a/hw/msix.h b/hw/msix.h
index 6b21ffb..5c84b3e 100644
--- a/hw/msix.h
+++ b/hw/msix.h
@@ -4,6 +4,32 @@
 #include "qemu-common.h"
 #include "pci.h"
 
+/* MSI-X capability structure */
+#define MSIX_TABLE_OFFSET 4
+#define MSIX_PBA_OFFSET 8
+#define MSIX_CAP_LENGTH 12
+
+/* MSI enable bit and maskall bit are in byte 1 in FLAGS register */
+#define MSIX_CONTROL_OFFSET (PCI_MSIX_FLAGS + 1)
+#define MSIX_ENABLE_MASK (PCI_MSIX_FLAGS_ENABLE >> 8)
+#define MSIX_MASKALL_MASK (PCI_MSIX_FLAGS_MASKALL >> 8)
+
+/* MSI-X table format */
+#define MSIX_MSG_ADDR 0
+#define MSIX_MSG_UPPER_ADDR 4
+#define MSIX_MSG_DATA 8
+#define MSIX_VECTOR_CTRL 12
+#define MSIX_ENTRY_SIZE 16
+#define MSIX_VECTOR_MASK 0x1
+
+/* How much space does an MSIX table need. */
+/* The spec requires giving the table structure
+ * a 4K aligned region all by itself. */
+#define MSIX_PAGE_SIZE 0x1000
+/* Reserve second half of the page for pending bits */
+#define MSIX_PAGE_PENDING (MSIX_PAGE_SIZE / 2)
+#define MSIX_MAX_ENTRIES 32
+
 int msix_init(PCIDevice *pdev, unsigned short nentries,
               unsigned bar_nr, unsigned bar_size);
 
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 5/9] Add missing #include to hw/irq.h
  2010-09-20 15:06 [PATCH 0/9] msix/kvm integration cleanups Avi Kivity
                   ` (3 preceding siblings ...)
  2010-09-20 15:06 ` [PATCH 4/9] msix: move definitions from msix.c to msix.h Avi Kivity
@ 2010-09-20 15:06 ` Avi Kivity
  2010-09-20 15:06 ` [PATCH 6/9] msix: move kvm support functions to kvm-all.c and kvm-stub.c Avi Kivity
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Avi Kivity @ 2010-09-20 15:06 UTC (permalink / raw)
  To: Marcelo Tosatti, kvm, Michael S. Tsirkin

Needs qemu-common.h for qemu_irq.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 hw/irq.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/hw/irq.h b/hw/irq.h
index 5daae44..e9f0542 100644
--- a/hw/irq.h
+++ b/hw/irq.h
@@ -1,6 +1,8 @@
 #ifndef QEMU_IRQ_H
 #define QEMU_IRQ_H
 
+#include "qemu-common.h"
+
 /* Generic IRQ/GPIO pin infrastructure.  */
 
 /* FIXME: Rmove one of these.  */
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 6/9] msix: move kvm support functions to kvm-all.c and kvm-stub.c
  2010-09-20 15:06 [PATCH 0/9] msix/kvm integration cleanups Avi Kivity
                   ` (4 preceding siblings ...)
  2010-09-20 15:06 ` [PATCH 5/9] Add missing #include to hw/irq.h Avi Kivity
@ 2010-09-20 15:06 ` Avi Kivity
  2010-09-20 15:06 ` [PATCH 7/9] msix: Move kvm_enabled() guards to kvm-all.c functions Avi Kivity
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Avi Kivity @ 2010-09-20 15:06 UTC (permalink / raw)
  To: Marcelo Tosatti, kvm, Michael S. Tsirkin

This removes the dependency on CONFIG_KVM.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 hw/msix.c  |  137 ------------------------------------------------------------
 kvm-all.c  |  124 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 kvm-stub.c |   28 ++++++++++++
 kvm.h      |   16 +++++++
 4 files changed, 168 insertions(+), 137 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index dda1a24..bdec0b3 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -19,143 +19,6 @@
 /* Flag for interrupt controller to declare MSI-X support */
 int msix_supported;
 
-#ifdef CONFIG_KVM
-/* KVM specific MSIX helpers */
-
-static void kvm_msix_init(PCIDevice *dev)
-{
-#ifdef KVM_CAP_IRQCHIP
-    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
-        dev->msix_irq_entries = qemu_malloc(dev->msix_entries_nr *
-                                            sizeof *dev->msix_irq_entries);
-    }
-#endif
-}
-
-static void kvm_msix_free(PCIDevice *dev)
-{
-    int vector, changed = 0;
-    for (vector = 0; vector < dev->msix_entries_nr; ++vector) {
-        if (dev->msix_entry_used[vector]) {
-            kvm_del_routing_entry(kvm_context, &dev->msix_irq_entries[vector]);
-            changed = 1;
-        }
-    }
-    if (changed) {
-        kvm_commit_irq_routes(kvm_context);
-    }
-}
-
-static void kvm_msix_routing_entry(PCIDevice *dev, unsigned vector,
-                                   struct kvm_irq_routing_entry *entry)
-{
-    uint8_t *table_entry = dev->msix_table_page + vector * MSIX_ENTRY_SIZE;
-    entry->type = KVM_IRQ_ROUTING_MSI;
-    entry->flags = 0;
-    entry->u.msi.address_lo = pci_get_long(table_entry + MSIX_MSG_ADDR);
-    entry->u.msi.address_hi = pci_get_long(table_entry + MSIX_MSG_UPPER_ADDR);
-    entry->u.msi.data = pci_get_long(table_entry + MSIX_MSG_DATA);
-}
-
-static void kvm_msix_update(PCIDevice *dev, int vector,
-                            int was_masked, int is_masked)
-{
-    struct kvm_irq_routing_entry e = {}, *entry;
-    int mask_cleared = was_masked && !is_masked;
-    /* It is only legal to change an entry when it is masked. Therefore, it is
-     * enough to update the routing in kernel when mask is being cleared. */
-    if (!mask_cleared) {
-        return;
-    }
-    if (!dev->msix_entry_used[vector]) {
-        return;
-    }
-    entry = dev->msix_irq_entries + vector;
-    e.gsi = entry->gsi;
-    kvm_msix_routing_entry(dev, vector, &e);
-    if (memcmp(&entry->u.msi, &e.u.msi, sizeof entry->u.msi)) {
-        int r;
-        r = kvm_update_routing_entry(kvm_context, entry, &e);
-        if (r) {
-            fprintf(stderr, "%s: kvm_update_routing_entry failed: %s\n", __func__,
-		    strerror(-r));
-            exit(1);
-        }
-        memcpy(&entry->u.msi, &e.u.msi, sizeof entry->u.msi);
-        r = kvm_commit_irq_routes(kvm_context);
-        if (r) {
-            fprintf(stderr, "%s: kvm_commit_irq_routes failed: %s\n", __func__,
-		    strerror(-r));
-            exit(1);
-        }
-    }
-}
-
-static int kvm_msix_add(PCIDevice *dev, unsigned vector)
-{
-    struct kvm_irq_routing_entry *entry = dev->msix_irq_entries + vector;
-    int r;
-
-    if (!kvm_has_gsi_routing(kvm_context)) {
-        fprintf(stderr, "Warning: no MSI-X support found. "
-                "At least kernel 2.6.30 is required for MSI-X support.\n"
-               );
-        return -EOPNOTSUPP;
-    }
-
-    r = kvm_get_irq_route_gsi(kvm_context);
-    if (r < 0) {
-        fprintf(stderr, "%s: kvm_get_irq_route_gsi failed: %s\n", __func__, strerror(-r));
-        return r;
-    }
-    entry->gsi = r;
-    kvm_msix_routing_entry(dev, vector, entry);
-    r = kvm_add_routing_entry(kvm_context, entry);
-    if (r < 0) {
-        fprintf(stderr, "%s: kvm_add_routing_entry failed: %s\n", __func__, strerror(-r));
-        return r;
-    }
-
-    r = kvm_commit_irq_routes(kvm_context);
-    if (r < 0) {
-        fprintf(stderr, "%s: kvm_commit_irq_routes failed: %s\n", __func__, strerror(-r));
-        return r;
-    }
-    return 0;
-}
-
-static void kvm_msix_del(PCIDevice *dev, unsigned vector)
-{
-    if (dev->msix_entry_used[vector]) {
-        return;
-    }
-    kvm_del_routing_entry(kvm_context, &dev->msix_irq_entries[vector]);
-    kvm_commit_irq_routes(kvm_context);
-}
-
-static bool kvm_msix_notify(PCIDevice *dev, unsigned vector)
-{
-#ifdef KVM_CAP_IRQCHIP
-    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
-        kvm_set_irq(dev->msix_irq_entries[vector].gsi, 1, NULL);
-        return true;
-    }
-#endif
-    return false;
-}
-
-#else
-
-static void kvm_msix_init(PCIDevice *dev) {}
-static void kvm_msix_free(PCIDevice *dev) {}
-static void kvm_msix_update(PCIDevice *dev, int vector,
-                            int was_masked, int is_masked) {}
-static int kvm_msix_add(PCIDevice *dev, unsigned vector) { return -1; }
-static void kvm_msix_del(PCIDevice *dev, unsigned vector) {}
-static bool kvm_msix_notify(PCIDevice *dev, unsigned vector) { return false; }
-
-#endif
-
 /* Add MSI-X capability to the config space for the device. */
 /* Given a bar and its size, add MSI-X table on top of it
  * and fill MSI-X capability in the config space.
diff --git a/kvm-all.c b/kvm-all.c
index d4b0861..0860a16 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -24,6 +24,8 @@
 #include "qemu-barrier.h"
 #include "sysemu.h"
 #include "hw/hw.h"
+#include "hw/pci.h"
+#include "hw/msix.h"
 #include "gdbstub.h"
 #include "kvm.h"
 #include "bswap.h"
@@ -1338,5 +1340,127 @@ int kvm_set_irqfd(int gsi, int fd, bool assigned)
 }
 #endif
 
+void kvm_msix_init(PCIDevice *dev)
+{
+#ifdef KVM_CAP_IRQCHIP
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
+        dev->msix_irq_entries = qemu_malloc(dev->msix_entries_nr *
+                                            sizeof *dev->msix_irq_entries);
+    }
+#endif
+}
+
+void kvm_msix_free(PCIDevice *dev)
+{
+    int vector, changed = 0;
+    for (vector = 0; vector < dev->msix_entries_nr; ++vector) {
+        if (dev->msix_entry_used[vector]) {
+            kvm_del_routing_entry(kvm_context, &dev->msix_irq_entries[vector]);
+            changed = 1;
+        }
+    }
+    if (changed) {
+        kvm_commit_irq_routes(kvm_context);
+    }
+}
+
+static void kvm_msix_routing_entry(PCIDevice *dev, unsigned vector,
+                                   struct kvm_irq_routing_entry *entry)
+{
+    uint8_t *table_entry = dev->msix_table_page + vector * MSIX_ENTRY_SIZE;
+    entry->type = KVM_IRQ_ROUTING_MSI;
+    entry->flags = 0;
+    entry->u.msi.address_lo = pci_get_long(table_entry + MSIX_MSG_ADDR);
+    entry->u.msi.address_hi = pci_get_long(table_entry + MSIX_MSG_UPPER_ADDR);
+    entry->u.msi.data = pci_get_long(table_entry + MSIX_MSG_DATA);
+}
+
+void kvm_msix_update(PCIDevice *dev, int vector,
+                            int was_masked, int is_masked)
+{
+    struct kvm_irq_routing_entry e = {}, *entry;
+    int mask_cleared = was_masked && !is_masked;
+    /* It is only legal to change an entry when it is masked. Therefore, it is
+     * enough to update the routing in kernel when mask is being cleared. */
+    if (!mask_cleared) {
+        return;
+    }
+    if (!dev->msix_entry_used[vector]) {
+        return;
+    }
+    entry = dev->msix_irq_entries + vector;
+    e.gsi = entry->gsi;
+    kvm_msix_routing_entry(dev, vector, &e);
+    if (memcmp(&entry->u.msi, &e.u.msi, sizeof entry->u.msi)) {
+        int r;
+        r = kvm_update_routing_entry(kvm_context, entry, &e);
+        if (r) {
+            fprintf(stderr, "%s: kvm_update_routing_entry failed: %s\n", __func__,
+		    strerror(-r));
+            exit(1);
+        }
+        memcpy(&entry->u.msi, &e.u.msi, sizeof entry->u.msi);
+        r = kvm_commit_irq_routes(kvm_context);
+        if (r) {
+            fprintf(stderr, "%s: kvm_commit_irq_routes failed: %s\n", __func__,
+		    strerror(-r));
+            exit(1);
+        }
+    }
+}
+
+int kvm_msix_add(PCIDevice *dev, unsigned vector)
+{
+    struct kvm_irq_routing_entry *entry = dev->msix_irq_entries + vector;
+    int r;
+
+    if (!kvm_has_gsi_routing(kvm_context)) {
+        fprintf(stderr, "Warning: no MSI-X support found. "
+                "At least kernel 2.6.30 is required for MSI-X support.\n"
+               );
+        return -EOPNOTSUPP;
+    }
+
+    r = kvm_get_irq_route_gsi(kvm_context);
+    if (r < 0) {
+        fprintf(stderr, "%s: kvm_get_irq_route_gsi failed: %s\n", __func__, strerror(-r));
+        return r;
+    }
+    entry->gsi = r;
+    kvm_msix_routing_entry(dev, vector, entry);
+    r = kvm_add_routing_entry(kvm_context, entry);
+    if (r < 0) {
+        fprintf(stderr, "%s: kvm_add_routing_entry failed: %s\n", __func__, strerror(-r));
+        return r;
+    }
+
+    r = kvm_commit_irq_routes(kvm_context);
+    if (r < 0) {
+        fprintf(stderr, "%s: kvm_commit_irq_routes failed: %s\n", __func__, strerror(-r));
+        return r;
+    }
+    return 0;
+}
+
+void kvm_msix_del(PCIDevice *dev, unsigned vector)
+{
+    if (dev->msix_entry_used[vector]) {
+        return;
+    }
+    kvm_del_routing_entry(kvm_context, &dev->msix_irq_entries[vector]);
+    kvm_commit_irq_routes(kvm_context);
+}
+
+bool kvm_msix_notify(PCIDevice *dev, unsigned vector)
+{
+#ifdef KVM_CAP_IRQCHIP
+    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
+        kvm_set_irq(dev->msix_irq_entries[vector].gsi, 1, NULL);
+        return true;
+    }
+#endif
+    return false;
+}
+
 #undef PAGE_SIZE
 #include "qemu-kvm.c"
diff --git a/kvm-stub.c b/kvm-stub.c
index d45f9fa..1c54020 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -13,6 +13,7 @@
 #include "qemu-common.h"
 #include "sysemu.h"
 #include "hw/hw.h"
+#include "hw/pci.h"
 #include "exec-all.h"
 #include "gdbstub.h"
 #include "kvm.h"
@@ -141,3 +142,30 @@ int kvm_set_ioeventfd_mmio_long(int fd, uint32_t adr, uint32_t val, bool assign)
 {
     return -ENOSYS;
 }
+
+void kvm_msix_init(PCIDevice *dev)
+{
+}
+
+void kvm_msix_free(PCIDevice *dev)
+{
+}
+
+void kvm_msix_update(PCIDevice *dev, int vector,
+                     int was_masked, int is_masked)
+{
+}
+
+int kvm_msix_add(PCIDevice *dev, unsigned vector)
+{
+    return -1;
+}
+
+void kvm_msix_del(PCIDevice *dev, unsigned vector)
+{
+}
+
+bool kvm_msix_notify(PCIDevice *dev, unsigned vector)
+{
+    return false;
+}
diff --git a/kvm.h b/kvm.h
index 56236ae..62dc039 100644
--- a/kvm.h
+++ b/kvm.h
@@ -18,6 +18,9 @@
 #include "config-host.h"
 #include "qemu-queue.h"
 #include "qemu-kvm.h"
+#ifndef __DYNGEN_EXEC_H__
+#include "hw/pci.h"
+#endif
 
 #ifdef CONFIG_KVM
 #include <linux/kvm.h>
@@ -194,5 +197,18 @@ int kvm_set_irqfd(int gsi, int fd, bool assigned)
 }
 #endif
 
+#ifndef __DYNGEN_EXEC_H__
+
 int kvm_set_ioeventfd_pio_word(int fd, uint16_t adr, uint16_t val, bool assign);
+
+void kvm_msix_init(PCIDevice *dev);
+void kvm_msix_free(PCIDevice *dev);
+void kvm_msix_update(PCIDevice *dev, int vector,
+                     int was_masked, int is_masked);
+int kvm_msix_add(PCIDevice *dev, unsigned vector);
+void kvm_msix_del(PCIDevice *dev, unsigned vector);
+bool kvm_msix_notify(PCIDevice *dev, unsigned vector);
+
+#endif
+
 #endif
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 7/9] msix: Move kvm_enabled() guards to kvm-all.c functions
  2010-09-20 15:06 [PATCH 0/9] msix/kvm integration cleanups Avi Kivity
                   ` (5 preceding siblings ...)
  2010-09-20 15:06 ` [PATCH 6/9] msix: move kvm support functions to kvm-all.c and kvm-stub.c Avi Kivity
@ 2010-09-20 15:06 ` Avi Kivity
  2010-09-20 15:06 ` [PATCH 8/9] Protect qemu-kvm.h declarations with NEED_CPU_H Avi Kivity
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 19+ messages in thread
From: Avi Kivity @ 2010-09-20 15:06 UTC (permalink / raw)
  To: Marcelo Tosatti, kvm, Michael S. Tsirkin

Simplifies the msix code.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 hw/msix.c  |   22 ++++++++--------------
 kvm-all.c  |   18 ++++++++++++++++++
 kvm-stub.c |    2 +-
 3 files changed, 27 insertions(+), 15 deletions(-)

diff --git a/hw/msix.c b/hw/msix.c
index bdec0b3..fc4cbb1 100644
--- a/hw/msix.c
+++ b/hw/msix.c
@@ -159,9 +159,7 @@ static void msix_mmio_writel(void *opaque, target_phys_addr_t addr,
     int vector = offset / MSIX_ENTRY_SIZE;
     int was_masked = msix_is_masked(dev, vector);
     pci_set_long(dev->msix_table_page + offset, val);
-    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
-        kvm_msix_update(dev, vector, was_masked, msix_is_masked(dev, vector));
-    }
+    kvm_msix_update(dev, vector, was_masked, msix_is_masked(dev, vector));
     if (was_masked != msix_is_masked(dev, vector) &&
         dev->msix_mask_notifier && dev->msix_mask_notifier_opaque[vector]) {
         int r = dev->msix_mask_notifier(dev, vector,
@@ -276,9 +274,7 @@ static void msix_free_irq_entries(PCIDevice *dev)
 {
     int vector;
 
-    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
-        kvm_msix_free(dev);
-    }
+    kvm_msix_free(dev);
 
     for (vector = 0; vector < dev->msix_entries_nr; ++vector) {
         dev->msix_entry_used[vector] = 0;
@@ -413,12 +409,12 @@ int msix_vector_use(PCIDevice *dev, unsigned vector)
     if (dev->msix_entry_used[vector]) {
         return 0;
     }
-    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
-        ret = kvm_msix_add(dev, vector);
-        if (ret) {
-            return ret;
-        }
+
+    ret = kvm_msix_add(dev, vector);
+    if (ret) {
+        return ret;
     }
+
     ++dev->msix_entry_used[vector];
     return 0;
 }
@@ -432,9 +428,7 @@ void msix_vector_unuse(PCIDevice *dev, unsigned vector)
     if (--dev->msix_entry_used[vector]) {
         return;
     }
-    if (kvm_enabled() && kvm_irqchip_in_kernel()) {
-        kvm_msix_del(dev, vector);
-    }
+    kvm_msix_del(dev, vector);
     msix_clr_pending(dev, vector);
 }
 
diff --git a/kvm-all.c b/kvm-all.c
index 0860a16..a5e7545 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -1353,6 +1353,11 @@ void kvm_msix_init(PCIDevice *dev)
 void kvm_msix_free(PCIDevice *dev)
 {
     int vector, changed = 0;
+
+    if (!(kvm_enabled() && kvm_irqchip_in_kernel())) {
+        return;
+    }
+
     for (vector = 0; vector < dev->msix_entries_nr; ++vector) {
         if (dev->msix_entry_used[vector]) {
             kvm_del_routing_entry(kvm_context, &dev->msix_irq_entries[vector]);
@@ -1380,6 +1385,11 @@ void kvm_msix_update(PCIDevice *dev, int vector,
 {
     struct kvm_irq_routing_entry e = {}, *entry;
     int mask_cleared = was_masked && !is_masked;
+
+    if (!(kvm_enabled() && kvm_irqchip_in_kernel())) {
+        return;
+    }
+
     /* It is only legal to change an entry when it is masked. Therefore, it is
      * enough to update the routing in kernel when mask is being cleared. */
     if (!mask_cleared) {
@@ -1414,6 +1424,10 @@ int kvm_msix_add(PCIDevice *dev, unsigned vector)
     struct kvm_irq_routing_entry *entry = dev->msix_irq_entries + vector;
     int r;
 
+    if (!(kvm_enabled() && kvm_irqchip_in_kernel())) {
+        return 0;
+    }
+
     if (!kvm_has_gsi_routing(kvm_context)) {
         fprintf(stderr, "Warning: no MSI-X support found. "
                 "At least kernel 2.6.30 is required for MSI-X support.\n"
@@ -1444,6 +1458,10 @@ int kvm_msix_add(PCIDevice *dev, unsigned vector)
 
 void kvm_msix_del(PCIDevice *dev, unsigned vector)
 {
+    if (!(kvm_enabled() && kvm_irqchip_in_kernel())) {
+        return;
+    }
+
     if (dev->msix_entry_used[vector]) {
         return;
     }
diff --git a/kvm-stub.c b/kvm-stub.c
index 1c54020..37d2b7a 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -158,7 +158,7 @@ void kvm_msix_update(PCIDevice *dev, int vector,
 
 int kvm_msix_add(PCIDevice *dev, unsigned vector)
 {
-    return -1;
+    return 0;
 }
 
 void kvm_msix_del(PCIDevice *dev, unsigned vector)
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 8/9] Protect qemu-kvm.h declarations with NEED_CPU_H
  2010-09-20 15:06 [PATCH 0/9] msix/kvm integration cleanups Avi Kivity
                   ` (6 preceding siblings ...)
  2010-09-20 15:06 ` [PATCH 7/9] msix: Move kvm_enabled() guards to kvm-all.c functions Avi Kivity
@ 2010-09-20 15:06 ` Avi Kivity
  2010-09-20 17:05   ` Michael S. Tsirkin
  2010-09-20 15:06 ` [PATCH 9/9] Move msix.o build back to Makefile.objs Avi Kivity
  2010-09-20 17:02 ` [PATCH 0/9] msix/kvm integration cleanups Michael S. Tsirkin
  9 siblings, 1 reply; 19+ messages in thread
From: Avi Kivity @ 2010-09-20 15:06 UTC (permalink / raw)
  To: Marcelo Tosatti, kvm, Michael S. Tsirkin

Target-specific definitions need to be qualified with NEED_CPU_H so kvm.h
can be included from non-target-specific files.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 kvm-stub.c |    1 +
 qemu-kvm.h |   21 ++++++++++++++++++++-
 2 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/kvm-stub.c b/kvm-stub.c
index 37d2b7a..2e4bf00 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -169,3 +169,4 @@ bool kvm_msix_notify(PCIDevice *dev, unsigned vector)
 {
     return false;
 }
+
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 9809574..66ae18f 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -8,7 +8,9 @@
 #ifndef THE_ORIGINAL_AND_TRUE_QEMU_KVM_H
 #define THE_ORIGINAL_AND_TRUE_QEMU_KVM_H
 
+#ifdef NEED_CPU_H
 #include "cpu.h"
+#endif
 
 #include <signal.h>
 #include <stdlib.h>
@@ -794,6 +796,8 @@ typedef struct kvm_vcpu_context *kvm_vcpu_context_t;
 struct kvm_pit_state {
 };
 
+#ifdef NEED_CPU_H
+
 static inline void kvm_inject_x86_mce(CPUState *cenv, int bank,
                                       uint64_t status, uint64_t mcg_status,
                                       uint64_t addr, uint64_t misc,
@@ -803,8 +807,9 @@ static inline void kvm_inject_x86_mce(CPUState *cenv, int bank,
         abort();
 }
 
-#endif                          /* !CONFIG_KVM */
+#endif
 
+#endif                          /* !CONFIG_KVM */
 
 /*!
  * \brief Create new KVM context
@@ -820,9 +825,11 @@ int kvm_init(int smp_cpus);
 
 int kvm_main_loop(void);
 int kvm_init_ap(void);
+#ifdef NEED_CPU_H
 int kvm_vcpu_inited(CPUState *env);
 void kvm_save_lapic(CPUState *env);
 void kvm_load_lapic(CPUState *env);
+#endif
 
 void kvm_hpet_enable_kpit(void);
 void kvm_hpet_disable_kpit(void);
@@ -830,11 +837,13 @@ int kvm_set_irq(int irq, int level, int *status);
 
 int kvm_physical_memory_set_dirty_tracking(int enable);
 
+#ifdef NEED_CPU_H
 void qemu_kvm_call_with_env(void (*func)(void *), void *data, CPUState *env);
 void qemu_kvm_cpuid_on_env(CPUState *env);
 void kvm_inject_interrupt(CPUState *env, int mask);
 void kvm_update_after_sipi(CPUState *env);
 void kvm_update_interrupt_request(CPUState *env);
+#endif
 #ifndef CONFIG_USER_ONLY
 void *kvm_cpu_create_phys_mem(target_phys_addr_t start_addr, unsigned long size,
                               int log, int writable);
@@ -850,6 +859,7 @@ int kvm_qemu_destroy_memory_alias(uint64_t phys_start);
 
 int kvm_arch_qemu_create_context(void);
 
+#ifdef NEED_CPU_H
 void kvm_arch_save_regs(CPUState *env);
 void kvm_arch_load_regs(CPUState *env, int level);
 int kvm_arch_has_work(CPUState *env);
@@ -858,12 +868,15 @@ int kvm_arch_try_push_interrupts(void *opaque);
 void kvm_arch_push_nmi(void *opaque);
 void kvm_arch_cpu_reset(CPUState *env);
 int kvm_set_boot_cpu_id(uint32_t id);
+#endif
 
 void qemu_kvm_aio_wait_start(void);
 void qemu_kvm_aio_wait(void);
 void qemu_kvm_aio_wait_end(void);
 
+#ifdef NEED_CPU_H
 void kvm_tpr_access_report(CPUState *env, uint64_t rip, int is_write);
+#endif
 
 int kvm_arch_init_irq_routing(void);
 
@@ -873,15 +886,20 @@ int kvm_mmio_write(void *opaque, uint64_t addr, uint8_t * data, int len);
 #ifdef CONFIG_KVM_DEVICE_ASSIGNMENT
 struct ioperm_data;
 
+#ifdef NEED_CPU_H
 void kvm_ioperm(CPUState *env, void *data);
 void kvm_add_ioperm_data(struct ioperm_data *data);
 void kvm_remove_ioperm_data(unsigned long start_port, unsigned long num);
 void kvm_arch_do_ioperm(void *_data);
 #endif
+#endif
 
 #define ALIGN(x, y)  (((x)+(y)-1) & ~((y)-1))
+#ifdef NEED_CPU_H
 #define BITMAP_SIZE(m) (ALIGN(((m)>>TARGET_PAGE_BITS), HOST_LONG_BITS) / 8)
+#endif
 
+#ifdef NEED_CPU_H
 #ifdef CONFIG_KVM
 #include "qemu-queue.h"
 
@@ -915,6 +933,7 @@ int handle_tpr_access(void *opaque, CPUState *env, uint64_t rip,
 #endif
 #define qemu_kvm_cpu_stop(env) do {} while(0)
 #endif
+#endif
 
 #ifdef CONFIG_KVM
 
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH 9/9] Move msix.o build back to Makefile.objs
  2010-09-20 15:06 [PATCH 0/9] msix/kvm integration cleanups Avi Kivity
                   ` (7 preceding siblings ...)
  2010-09-20 15:06 ` [PATCH 8/9] Protect qemu-kvm.h declarations with NEED_CPU_H Avi Kivity
@ 2010-09-20 15:06 ` Avi Kivity
  2010-09-20 17:02 ` [PATCH 0/9] msix/kvm integration cleanups Michael S. Tsirkin
  9 siblings, 0 replies; 19+ messages in thread
From: Avi Kivity @ 2010-09-20 15:06 UTC (permalink / raw)
  To: Marcelo Tosatti, kvm, Michael S. Tsirkin

Now that hw/msix.c does not depend on CONFIG_KVM, remove it from
Makefile.target and put it back in Makefile.objs.

Signed-off-by: Avi Kivity <avi@redhat.com>
---
 Makefile.objs   |    4 +---
 Makefile.target |    4 ----
 2 files changed, 1 insertions(+), 7 deletions(-)

diff --git a/Makefile.objs b/Makefile.objs
index dbee210..44b2399 100644
--- a/Makefile.objs
+++ b/Makefile.objs
@@ -196,9 +196,7 @@ hw-obj-$(CONFIG_PIIX4) += piix4.o
 # PCI watchdog devices
 hw-obj-y += wdt_i6300esb.o
 
-# MSI-X depends on kvm for interrupt injection,
-# so moved it from Makefile.objs to Makefile.target for now
-# hw-obj-y += msix.o
+hw-obj-y += msix.o
 
 # PCI network cards
 hw-obj-y += ne2000.o
diff --git a/Makefile.target b/Makefile.target
index 5c1eec4..7c8f19c 100644
--- a/Makefile.target
+++ b/Makefile.target
@@ -183,10 +183,6 @@ obj-y += rwhandler.o
 obj-$(CONFIG_KVM) += kvm.o kvm-all.o
 obj-$(CONFIG_NO_KVM) += kvm-stub.o
 
-# MSI-X depends on kvm for interrupt injection,
-# so moved it from Makefile.objs to Makefile.target for now
-obj-y += msix.o
-
 LIBS+=-lz
 
 QEMU_CFLAGS += $(VNC_TLS_CFLAGS)
-- 
1.7.2.3


^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/9] msix: move definitions from msix.c to msix.h
  2010-09-20 15:06 ` [PATCH 4/9] msix: move definitions from msix.c to msix.h Avi Kivity
@ 2010-09-20 16:56   ` Michael S. Tsirkin
  2010-09-21 16:03     ` Avi Kivity
  0 siblings, 1 reply; 19+ messages in thread
From: Michael S. Tsirkin @ 2010-09-20 16:56 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm

On Mon, Sep 20, 2010 at 05:06:45PM +0200, Avi Kivity wrote:
> This allows us to reuse them from the kvm support code.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>

I would rather all dealings with MSI-X table stayed in one place. All we
need is just the entry, so let's add APIs to retrieve MSIX address and
data:

uint64_t msix_get_address(dev, vector)
uint32_t msix_get_data(dev, vector)

and that will be enough for KVM.

> ---
>  hw/msix.c |   27 ---------------------------
>  hw/msix.h |   26 ++++++++++++++++++++++++++
>  2 files changed, 26 insertions(+), 27 deletions(-)
> 
> diff --git a/hw/msix.c b/hw/msix.c
> index d762870..dda1a24 100644
> --- a/hw/msix.c
> +++ b/hw/msix.c
> @@ -16,33 +16,6 @@
>  #include "pci.h"
>  #include "kvm.h"
>  
> -/* MSI-X capability structure */
> -#define MSIX_TABLE_OFFSET 4
> -#define MSIX_PBA_OFFSET 8
> -#define MSIX_CAP_LENGTH 12
> -
> -/* MSI enable bit and maskall bit are in byte 1 in FLAGS register */
> -#define MSIX_CONTROL_OFFSET (PCI_MSIX_FLAGS + 1)
> -#define MSIX_ENABLE_MASK (PCI_MSIX_FLAGS_ENABLE >> 8)
> -#define MSIX_MASKALL_MASK (PCI_MSIX_FLAGS_MASKALL >> 8)
> -
> -/* MSI-X table format */
> -#define MSIX_MSG_ADDR 0
> -#define MSIX_MSG_UPPER_ADDR 4
> -#define MSIX_MSG_DATA 8
> -#define MSIX_VECTOR_CTRL 12
> -#define MSIX_ENTRY_SIZE 16
> -#define MSIX_VECTOR_MASK 0x1
> -
> -/* How much space does an MSIX table need. */
> -/* The spec requires giving the table structure
> - * a 4K aligned region all by itself. */
> -#define MSIX_PAGE_SIZE 0x1000
> -/* Reserve second half of the page for pending bits */
> -#define MSIX_PAGE_PENDING (MSIX_PAGE_SIZE / 2)
> -#define MSIX_MAX_ENTRIES 32
> -
> -
>  /* Flag for interrupt controller to declare MSI-X support */
>  int msix_supported;
>  
> diff --git a/hw/msix.h b/hw/msix.h
> index 6b21ffb..5c84b3e 100644
> --- a/hw/msix.h
> +++ b/hw/msix.h
> @@ -4,6 +4,32 @@
>  #include "qemu-common.h"
>  #include "pci.h"
>  
> +/* MSI-X capability structure */
> +#define MSIX_TABLE_OFFSET 4
> +#define MSIX_PBA_OFFSET 8
> +#define MSIX_CAP_LENGTH 12
> +
> +/* MSI enable bit and maskall bit are in byte 1 in FLAGS register */
> +#define MSIX_CONTROL_OFFSET (PCI_MSIX_FLAGS + 1)
> +#define MSIX_ENABLE_MASK (PCI_MSIX_FLAGS_ENABLE >> 8)
> +#define MSIX_MASKALL_MASK (PCI_MSIX_FLAGS_MASKALL >> 8)
> +
> +/* MSI-X table format */
> +#define MSIX_MSG_ADDR 0
> +#define MSIX_MSG_UPPER_ADDR 4
> +#define MSIX_MSG_DATA 8
> +#define MSIX_VECTOR_CTRL 12
> +#define MSIX_ENTRY_SIZE 16
> +#define MSIX_VECTOR_MASK 0x1
> +
> +/* How much space does an MSIX table need. */
> +/* The spec requires giving the table structure
> + * a 4K aligned region all by itself. */
> +#define MSIX_PAGE_SIZE 0x1000
> +/* Reserve second half of the page for pending bits */
> +#define MSIX_PAGE_PENDING (MSIX_PAGE_SIZE / 2)
> +#define MSIX_MAX_ENTRIES 32
> +
>  int msix_init(PCIDevice *pdev, unsigned short nentries,
>                unsigned bar_nr, unsigned bar_size);
>  
> -- 
> 1.7.2.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/9] msix/kvm integration cleanups
  2010-09-20 15:06 [PATCH 0/9] msix/kvm integration cleanups Avi Kivity
                   ` (8 preceding siblings ...)
  2010-09-20 15:06 ` [PATCH 9/9] Move msix.o build back to Makefile.objs Avi Kivity
@ 2010-09-20 17:02 ` Michael S. Tsirkin
  2010-09-21 16:05   ` Avi Kivity
  9 siblings, 1 reply; 19+ messages in thread
From: Michael S. Tsirkin @ 2010-09-20 17:02 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm

On Mon, Sep 20, 2010 at 05:06:41PM +0200, Avi Kivity wrote:
> This cleans up msix/kvm integration a bit.  The really important patch is the
> last one, which allows msix.o to be part of non-target-specific build.

I actually thoought this later move should be done in a different way:
- add all functions msix uses to kvm-stub.c
- kvm_irq_routing_entry should also have a stub

I sent some minor comments in case you have a reason
to prefer this way.

> Avi Kivity (9):
>   msix: avoid leaking kvm data on init failure
>   msix: make kvm specific initialization a function
>   msix: move kvm specific msix notify into a function
>   msix: move definitions from msix.c to msix.h
>   Add missing #include to hw/irq.h
>   msix: move kvm support functions to kvm-all.c and kvm-stub.c
>   msix: Move kvm_enabled() guards to kvm-all.c functions
>   Protect qemu-kvm.h declarations with NEED_CPU_H
>   Move msix.o build back to Makefile.objs
> 
>  Makefile.objs   |    4 +-
>  Makefile.target |    4 -
>  hw/irq.h        |    2 +
>  hw/msix.c       |  173 ++++---------------------------------------------------
>  hw/msix.h       |   26 ++++++++
>  kvm-all.c       |  142 +++++++++++++++++++++++++++++++++++++++++++++
>  kvm-stub.c      |   29 +++++++++
>  kvm.h           |   16 +++++
>  qemu-kvm.h      |   21 ++++++-
>  9 files changed, 247 insertions(+), 170 deletions(-)
> 
> -- 
> 1.7.2.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/9] Protect qemu-kvm.h declarations with NEED_CPU_H
  2010-09-20 15:06 ` [PATCH 8/9] Protect qemu-kvm.h declarations with NEED_CPU_H Avi Kivity
@ 2010-09-20 17:05   ` Michael S. Tsirkin
  2010-09-21 16:03     ` Avi Kivity
  0 siblings, 1 reply; 19+ messages in thread
From: Michael S. Tsirkin @ 2010-09-20 17:05 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm

On Mon, Sep 20, 2010 at 05:06:49PM +0200, Avi Kivity wrote:
> Target-specific definitions need to be qualified with NEED_CPU_H so kvm.h
> can be included from non-target-specific files.
> 
> Signed-off-by: Avi Kivity <avi@redhat.com>

Long term, would be cleaner to split this into two files ...

> ---
>  kvm-stub.c |    1 +
>  qemu-kvm.h |   21 ++++++++++++++++++++-
>  2 files changed, 21 insertions(+), 1 deletions(-)
> 
> diff --git a/kvm-stub.c b/kvm-stub.c
> index 37d2b7a..2e4bf00 100644
> --- a/kvm-stub.c
> +++ b/kvm-stub.c
> @@ -169,3 +169,4 @@ bool kvm_msix_notify(PCIDevice *dev, unsigned vector)
>  {
>      return false;
>  }
> +

intentional?

> diff --git a/qemu-kvm.h b/qemu-kvm.h
> index 9809574..66ae18f 100644
> --- a/qemu-kvm.h
> +++ b/qemu-kvm.h
> @@ -8,7 +8,9 @@
>  #ifndef THE_ORIGINAL_AND_TRUE_QEMU_KVM_H
>  #define THE_ORIGINAL_AND_TRUE_QEMU_KVM_H
>  
> +#ifdef NEED_CPU_H
>  #include "cpu.h"
> +#endif
>  
>  #include <signal.h>
>  #include <stdlib.h>
> @@ -794,6 +796,8 @@ typedef struct kvm_vcpu_context *kvm_vcpu_context_t;
>  struct kvm_pit_state {
>  };
>  
> +#ifdef NEED_CPU_H
> +
>  static inline void kvm_inject_x86_mce(CPUState *cenv, int bank,
>                                        uint64_t status, uint64_t mcg_status,
>                                        uint64_t addr, uint64_t misc,
> @@ -803,8 +807,9 @@ static inline void kvm_inject_x86_mce(CPUState *cenv, int bank,
>          abort();
>  }
>  
> -#endif                          /* !CONFIG_KVM */
> +#endif
>  
> +#endif                          /* !CONFIG_KVM */
>  
>  /*!
>   * \brief Create new KVM context
> @@ -820,9 +825,11 @@ int kvm_init(int smp_cpus);
>  
>  int kvm_main_loop(void);
>  int kvm_init_ap(void);
> +#ifdef NEED_CPU_H
>  int kvm_vcpu_inited(CPUState *env);
>  void kvm_save_lapic(CPUState *env);
>  void kvm_load_lapic(CPUState *env);
> +#endif
>  
>  void kvm_hpet_enable_kpit(void);
>  void kvm_hpet_disable_kpit(void);
> @@ -830,11 +837,13 @@ int kvm_set_irq(int irq, int level, int *status);
>  
>  int kvm_physical_memory_set_dirty_tracking(int enable);
>  
> +#ifdef NEED_CPU_H
>  void qemu_kvm_call_with_env(void (*func)(void *), void *data, CPUState *env);
>  void qemu_kvm_cpuid_on_env(CPUState *env);
>  void kvm_inject_interrupt(CPUState *env, int mask);
>  void kvm_update_after_sipi(CPUState *env);
>  void kvm_update_interrupt_request(CPUState *env);
> +#endif
>  #ifndef CONFIG_USER_ONLY
>  void *kvm_cpu_create_phys_mem(target_phys_addr_t start_addr, unsigned long size,
>                                int log, int writable);
> @@ -850,6 +859,7 @@ int kvm_qemu_destroy_memory_alias(uint64_t phys_start);
>  
>  int kvm_arch_qemu_create_context(void);
>  
> +#ifdef NEED_CPU_H
>  void kvm_arch_save_regs(CPUState *env);
>  void kvm_arch_load_regs(CPUState *env, int level);
>  int kvm_arch_has_work(CPUState *env);
> @@ -858,12 +868,15 @@ int kvm_arch_try_push_interrupts(void *opaque);
>  void kvm_arch_push_nmi(void *opaque);
>  void kvm_arch_cpu_reset(CPUState *env);
>  int kvm_set_boot_cpu_id(uint32_t id);
> +#endif
>  
>  void qemu_kvm_aio_wait_start(void);
>  void qemu_kvm_aio_wait(void);
>  void qemu_kvm_aio_wait_end(void);
>  
> +#ifdef NEED_CPU_H
>  void kvm_tpr_access_report(CPUState *env, uint64_t rip, int is_write);
> +#endif
>  
>  int kvm_arch_init_irq_routing(void);
>  
> @@ -873,15 +886,20 @@ int kvm_mmio_write(void *opaque, uint64_t addr, uint8_t * data, int len);
>  #ifdef CONFIG_KVM_DEVICE_ASSIGNMENT
>  struct ioperm_data;
>  
> +#ifdef NEED_CPU_H
>  void kvm_ioperm(CPUState *env, void *data);
>  void kvm_add_ioperm_data(struct ioperm_data *data);
>  void kvm_remove_ioperm_data(unsigned long start_port, unsigned long num);
>  void kvm_arch_do_ioperm(void *_data);
>  #endif
> +#endif
>  
>  #define ALIGN(x, y)  (((x)+(y)-1) & ~((y)-1))
> +#ifdef NEED_CPU_H
>  #define BITMAP_SIZE(m) (ALIGN(((m)>>TARGET_PAGE_BITS), HOST_LONG_BITS) / 8)
> +#endif
>  
> +#ifdef NEED_CPU_H
>  #ifdef CONFIG_KVM
>  #include "qemu-queue.h"
>  
> @@ -915,6 +933,7 @@ int handle_tpr_access(void *opaque, CPUState *env, uint64_t rip,
>  #endif
>  #define qemu_kvm_cpu_stop(env) do {} while(0)
>  #endif
> +#endif
>  
>  #ifdef CONFIG_KVM
>  
> -- 
> 1.7.2.3

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 4/9] msix: move definitions from msix.c to msix.h
  2010-09-20 16:56   ` Michael S. Tsirkin
@ 2010-09-21 16:03     ` Avi Kivity
  0 siblings, 0 replies; 19+ messages in thread
From: Avi Kivity @ 2010-09-21 16:03 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Marcelo Tosatti, kvm

  On 09/20/2010 06:56 PM, Michael S. Tsirkin wrote:
> On Mon, Sep 20, 2010 at 05:06:45PM +0200, Avi Kivity wrote:
> >  This allows us to reuse them from the kvm support code.
> >
> >  Signed-off-by: Avi Kivity<avi@redhat.com>
>
> I would rather all dealings with MSI-X table stayed in one place. All we
> need is just the entry, so let's add APIs to retrieve MSIX address and
> data:
>
> uint64_t msix_get_address(dev, vector)
> uint32_t msix_get_data(dev, vector)
>
> and that will be enough for KVM.
>

Ok, will do.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 8/9] Protect qemu-kvm.h declarations with NEED_CPU_H
  2010-09-20 17:05   ` Michael S. Tsirkin
@ 2010-09-21 16:03     ` Avi Kivity
  0 siblings, 0 replies; 19+ messages in thread
From: Avi Kivity @ 2010-09-21 16:03 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Marcelo Tosatti, kvm

  On 09/20/2010 07:05 PM, Michael S. Tsirkin wrote:
> On Mon, Sep 20, 2010 at 05:06:49PM +0200, Avi Kivity wrote:
> >  Target-specific definitions need to be qualified with NEED_CPU_H so kvm.h
> >  can be included from non-target-specific files.
> >
> >  Signed-off-by: Avi Kivity<avi@redhat.com>
>
> Long term, would be cleaner to split this into two files ...

Yes, this is a pain to deal with.

> >  ---
> >   kvm-stub.c |    1 +
> >   qemu-kvm.h |   21 ++++++++++++++++++++-
> >   2 files changed, 21 insertions(+), 1 deletions(-)
> >
> >  diff --git a/kvm-stub.c b/kvm-stub.c
> >  index 37d2b7a..2e4bf00 100644
> >  --- a/kvm-stub.c
> >  +++ b/kvm-stub.c
> >  @@ -169,3 +169,4 @@ bool kvm_msix_notify(PCIDevice *dev, unsigned vector)
> >   {
> >       return false;
> >   }
> >  +
>
> intentional?


Nope.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/9] msix/kvm integration cleanups
  2010-09-20 17:02 ` [PATCH 0/9] msix/kvm integration cleanups Michael S. Tsirkin
@ 2010-09-21 16:05   ` Avi Kivity
  2010-09-21 16:14     ` Michael S. Tsirkin
  2010-10-06  9:39     ` Michael S. Tsirkin
  0 siblings, 2 replies; 19+ messages in thread
From: Avi Kivity @ 2010-09-21 16:05 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Marcelo Tosatti, kvm

  On 09/20/2010 07:02 PM, Michael S. Tsirkin wrote:
> On Mon, Sep 20, 2010 at 05:06:41PM +0200, Avi Kivity wrote:
> >  This cleans up msix/kvm integration a bit.  The really important patch is the
> >  last one, which allows msix.o to be part of non-target-specific build.
>
> I actually thoought this later move should be done in a different way:
> - add all functions msix uses to kvm-stub.c

Isn't that what I did?

> - kvm_irq_routing_entry should also have a stub
>
> I sent some minor comments in case you have a reason
> to prefer this way.

My motivation is really the last patch.  If you explain what you'd like 
to see I'll try to do it.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/9] msix/kvm integration cleanups
  2010-09-21 16:05   ` Avi Kivity
@ 2010-09-21 16:14     ` Michael S. Tsirkin
  2010-10-06  9:39     ` Michael S. Tsirkin
  1 sibling, 0 replies; 19+ messages in thread
From: Michael S. Tsirkin @ 2010-09-21 16:14 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm

On Tue, Sep 21, 2010 at 06:05:10PM +0200, Avi Kivity wrote:
>  On 09/20/2010 07:02 PM, Michael S. Tsirkin wrote:
> >On Mon, Sep 20, 2010 at 05:06:41PM +0200, Avi Kivity wrote:
> >>  This cleans up msix/kvm integration a bit.  The really important patch is the
> >>  last one, which allows msix.o to be part of non-target-specific build.
> >
> >I actually thoought this later move should be done in a different way:
> >- add all functions msix uses to kvm-stub.c
> 
> Isn't that what I did?
> 
> >- kvm_irq_routing_entry should also have a stub
> >
> >I sent some minor comments in case you have a reason
> >to prefer this way.
> 
> My motivation is really the last patch.  If you explain what you'd
> like to see I'll try to do it.

Basically my idea was to avoid all ifdefs in msix.c
*without changing it*, by stubbing out kvm APIs
and structures we use there.

-- 
MST

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/9] msix/kvm integration cleanups
  2010-09-21 16:05   ` Avi Kivity
  2010-09-21 16:14     ` Michael S. Tsirkin
@ 2010-10-06  9:39     ` Michael S. Tsirkin
  2010-10-06  9:50       ` Avi Kivity
  1 sibling, 1 reply; 19+ messages in thread
From: Michael S. Tsirkin @ 2010-10-06  9:39 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm

On Tue, Sep 21, 2010 at 06:05:10PM +0200, Avi Kivity wrote:
>  On 09/20/2010 07:02 PM, Michael S. Tsirkin wrote:
> >On Mon, Sep 20, 2010 at 05:06:41PM +0200, Avi Kivity wrote:
> >>  This cleans up msix/kvm integration a bit.  The really important patch is the
> >>  last one, which allows msix.o to be part of non-target-specific build.
> >
> >I actually thoought this later move should be done in a different way:
> >- add all functions msix uses to kvm-stub.c
> 
> Isn't that what I did?
> 
> >- kvm_irq_routing_entry should also have a stub
> >
> >I sent some minor comments in case you have a reason
> >to prefer this way.
> 
> My motivation is really the last patch.  If you explain what you'd
> like to see I'll try to do it.

Still looking at this?

> -- 
> error compiling committee.c: too many arguments to function

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH 0/9] msix/kvm integration cleanups
  2010-10-06  9:39     ` Michael S. Tsirkin
@ 2010-10-06  9:50       ` Avi Kivity
  0 siblings, 0 replies; 19+ messages in thread
From: Avi Kivity @ 2010-10-06  9:50 UTC (permalink / raw)
  To: Michael S. Tsirkin; +Cc: Marcelo Tosatti, kvm

  On 10/06/2010 11:39 AM, Michael S. Tsirkin wrote:
> On Tue, Sep 21, 2010 at 06:05:10PM +0200, Avi Kivity wrote:
> >   On 09/20/2010 07:02 PM, Michael S. Tsirkin wrote:
> >  >On Mon, Sep 20, 2010 at 05:06:41PM +0200, Avi Kivity wrote:
> >  >>   This cleans up msix/kvm integration a bit.  The really important patch is the
> >  >>   last one, which allows msix.o to be part of non-target-specific build.
> >  >
> >  >I actually thoought this later move should be done in a different way:
> >  >- add all functions msix uses to kvm-stub.c
> >
> >  Isn't that what I did?
> >
> >  >- kvm_irq_routing_entry should also have a stub
> >  >
> >  >I sent some minor comments in case you have a reason
> >  >to prefer this way.
> >
> >  My motivation is really the last patch.  If you explain what you'd
> >  like to see I'll try to do it.
>
> Still looking at this?
>

I plan to do this yes, when I get a bit of time.

-- 
error compiling committee.c: too many arguments to function


^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2010-10-06  9:50 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-20 15:06 [PATCH 0/9] msix/kvm integration cleanups Avi Kivity
2010-09-20 15:06 ` [PATCH 1/9] msix: avoid leaking kvm data on init failure Avi Kivity
2010-09-20 15:06 ` [PATCH 2/9] msix: make kvm specific initialization a function Avi Kivity
2010-09-20 15:06 ` [PATCH 3/9] msix: move kvm specific msix notify into " Avi Kivity
2010-09-20 15:06 ` [PATCH 4/9] msix: move definitions from msix.c to msix.h Avi Kivity
2010-09-20 16:56   ` Michael S. Tsirkin
2010-09-21 16:03     ` Avi Kivity
2010-09-20 15:06 ` [PATCH 5/9] Add missing #include to hw/irq.h Avi Kivity
2010-09-20 15:06 ` [PATCH 6/9] msix: move kvm support functions to kvm-all.c and kvm-stub.c Avi Kivity
2010-09-20 15:06 ` [PATCH 7/9] msix: Move kvm_enabled() guards to kvm-all.c functions Avi Kivity
2010-09-20 15:06 ` [PATCH 8/9] Protect qemu-kvm.h declarations with NEED_CPU_H Avi Kivity
2010-09-20 17:05   ` Michael S. Tsirkin
2010-09-21 16:03     ` Avi Kivity
2010-09-20 15:06 ` [PATCH 9/9] Move msix.o build back to Makefile.objs Avi Kivity
2010-09-20 17:02 ` [PATCH 0/9] msix/kvm integration cleanups Michael S. Tsirkin
2010-09-21 16:05   ` Avi Kivity
2010-09-21 16:14     ` Michael S. Tsirkin
2010-10-06  9:39     ` Michael S. Tsirkin
2010-10-06  9:50       ` Avi Kivity

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.