All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/19] pci-assign: Refactor for upstream merge
@ 2012-08-16 13:54 Jan Kiszka
  2012-08-16 13:54 ` [PATCH 01/19] pci-assign: Only clean up registered IO resources Jan Kiszka
                   ` (20 more replies)
  0 siblings, 21 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

With this series, we are getting very close to obsoleting qemu-kvm. It
refactors hw/device-assignment.c and the associated KVM helper functions
into a form that should allow merging them into QEMU. Once the series is
acceptable for qemu-kvm, I will break out the necessary uq/master
patches and push pci-assign to upstream.

The major step of this series is to define a regular set of kvm_device_*
services that encapsulate classic (i.e. KVM-based, non-VFIO) device
assignment features and export them to i386 targets only. There will
never be another arch using them, therefore I pushed them into this
corner. Moreover, the device assignment device now makes use of the new
KVM IRQ/MSI routing API and no longer pokes into the internals of that
layer. Finally, I moved the code into hw/kvm/pci-assign.c, dropped the
superfluous configure option and did some basic code cleanups (mostly
coding style) to bring things in shape.

Note that patch 1 is a simple bug fix that should likely be applied for
qemu-kvm-1.2 independently.

This series depends on [1] and [2] and QEMU upstream (2b97f88c92) being
merged into qemu-kvm.

Please review.

[1] http://thread.gmane.org/gmane.comp.emulators.kvm.devel/95528
[2] http://thread.gmane.org/gmane.comp.emulators.kvm.devel/95522

Jan Kiszka (19):
  pci-assign: Only clean up registered IO resources
  pci-assign: Factor out kvm_device_pci_assign/deassign
  pci-assign: Rename assign_irq to assign_intx
  pci-assign: Refactor interrupt deassignment
  pci-assign: Factor out kvm_device_intx_assign
  qemu-kvm: Move kvm_device_intx_set_mask service
  pci-assign: Rework MSI assignment
  pci-assign: Factor out kvm_device_msix_supported
  pci-assign: Replace kvm_assign_set_msix_nr with
    kvm_device_msix_init_vectors
  pci-assign: Replace kvm_assign_set_msix_entry with
    kvm_device_msix_set_vector
  pci-assign: Rework MSI-X route setup
  pci-assign: Factor out kvm_device_msix_assign
  qemu-kvm: Kill qemu-kvm.[ch]
  pci-assign: Drop configure switches
  pci-assign: Move and rename source file
  pci-assign: Fix coding style issues
  pci-assign: Replace exit() with hw_error()
  pci-assign: Drop unused or write-only variables
  pci-assign: Gracefully handle missing in-kernel irqchip support

 configure                                    |   11 -
 hw/i386/Makefile.objs                        |    3 -
 hw/kvm/Makefile.objs                         |    2 +-
 hw/{device-assignment.c => kvm/pci-assign.c} |  502 +++++++++++++-------------
 kvm-all.c                                    |   54 +++-
 kvm-stub.c                                   |    9 -
 kvm.h                                        |   12 +-
 qemu-kvm.c                                   |  233 ------------
 qemu-kvm.h                                   |  112 ------
 target-i386/kvm.c                            |  142 ++++++++
 target-i386/kvm_i386.h                       |   22 ++
 11 files changed, 461 insertions(+), 641 deletions(-)
 rename hw/{device-assignment.c => kvm/pci-assign.c} (84%)
 delete mode 100644 qemu-kvm.c
 delete mode 100644 qemu-kvm.h

-- 
1.7.3.4


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

* [PATCH 01/19] pci-assign: Only clean up registered IO resources
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 13:54 ` [PATCH 02/19] pci-assign: Factor out kvm_device_pci_assign/deassign Jan Kiszka
                   ` (19 subsequent siblings)
  20 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

If we fail before or inside assigned_dev_register_regions, we so far
unconditionally unregistered IO resources, triggering an assertion in
the memory layer. Non-null u.r_baseport tells us if the region has been
registered.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/device-assignment.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 7b55ac9..529e229 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -720,10 +720,12 @@ static void free_assigned_device(AssignedDevice *dev)
             continue;
         }
         if (pci_region->type & IORESOURCE_IO) {
-            memory_region_del_subregion(&region->container,
-                                        &region->real_iomem);
-            memory_region_destroy(&region->real_iomem);
-            memory_region_destroy(&region->container);
+            if (region->u.r_baseport) {
+                memory_region_del_subregion(&region->container,
+                                            &region->real_iomem);
+                memory_region_destroy(&region->real_iomem);
+                memory_region_destroy(&region->container);
+            }
         } else if (pci_region->type & IORESOURCE_MEM) {
             if (region->u.r_virtbase) {
                 memory_region_del_subregion(&region->container,
-- 
1.7.3.4


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

* [PATCH 02/19] pci-assign: Factor out kvm_device_pci_assign/deassign
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
  2012-08-16 13:54 ` [PATCH 01/19] pci-assign: Only clean up registered IO resources Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 13:54 ` [PATCH 03/19] pci-assign: Rename assign_irq to assign_intx Jan Kiszka
                   ` (18 subsequent siblings)
  20 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

In contrast to the old wrappers, the new ones only take the required
parameters instead of a preinitialized kvm_assigned_pci_dev structure.
We also move the dev_id generation into kvm_device_pci_assign and store
the result in the AssignedDevice structure. It will serve as handle for
all upcoming kvm_device_* functions.

While refactoring these services, start moving KVM services where they
should finally end up in upstream QEMU: in the i386-specific KVM layer.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/device-assignment.c |   46 ++++++++++++----------------------------------
 qemu-kvm.c             |   14 --------------
 qemu-kvm.h             |   24 ------------------------
 target-i386/kvm.c      |   36 ++++++++++++++++++++++++++++++++++++
 target-i386/kvm_i386.h |    6 ++++++
 5 files changed, 54 insertions(+), 72 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 529e229..3f6196a 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -41,6 +41,7 @@
 #include "range.h"
 #include "sysemu.h"
 #include "pci.h"
+#include "kvm_i386.h"
 
 #define MSIX_PAGE_SIZE 0x1000
 
@@ -108,6 +109,7 @@ typedef struct {
 typedef struct AssignedDevice {
     PCIDevice dev;
     PCIHostDeviceAddress host;
+    uint32_t dev_id;
     uint32_t features;
     int intpin;
     uint8_t debug_flags;
@@ -115,9 +117,6 @@ typedef struct AssignedDevice {
     PCIDevRegions real_device;
     int run;
     PCIINTxRoute intx_route;
-    uint16_t h_segnr;
-    uint8_t h_busnr;
-    uint8_t h_devfn;
     int irq_requested_type;
     int bound;
     struct {
@@ -761,12 +760,6 @@ static void free_assigned_device(AssignedDevice *dev)
     free_dev_irq_entries(dev);
 }
 
-static uint32_t calc_assigned_dev_id(AssignedDevice *dev)
-{
-    return (uint32_t)dev->h_segnr << 16 | (uint32_t)dev->h_busnr << 8 |
-           (uint32_t)dev->h_devfn;
-}
-
 static void assign_failed_examine(AssignedDevice *dev)
 {
     char name[PATH_MAX], dir[PATH_MAX], driver[PATH_MAX] = {}, *ns;
@@ -820,24 +813,17 @@ fail:
 
 static int assign_device(AssignedDevice *dev)
 {
-    struct kvm_assigned_pci_dev assigned_dev_data;
+    uint32_t flags = KVM_DEV_ASSIGN_ENABLE_IOMMU;
     int r;
 
     /* Only pass non-zero PCI segment to capable module */
     if (!kvm_check_extension(kvm_state, KVM_CAP_PCI_SEGMENT) &&
-        dev->h_segnr) {
+        dev->host.domain) {
         fprintf(stderr, "Can't assign device inside non-zero PCI segment "
                 "as this KVM module doesn't support it.\n");
         return -ENODEV;
     }
 
-    memset(&assigned_dev_data, 0, sizeof(assigned_dev_data));
-    assigned_dev_data.assigned_dev_id = calc_assigned_dev_id(dev);
-    assigned_dev_data.segnr = dev->h_segnr;
-    assigned_dev_data.busnr = dev->h_busnr;
-    assigned_dev_data.devfn = dev->h_devfn;
-
-    assigned_dev_data.flags = KVM_DEV_ASSIGN_ENABLE_IOMMU;
     if (!kvm_check_extension(kvm_state, KVM_CAP_IOMMU)) {
         fprintf(stderr, "No IOMMU found.  Unable to assign device \"%s\"\n",
                 dev->dev.qdev.id);
@@ -846,10 +832,10 @@ static int assign_device(AssignedDevice *dev)
 
     if (dev->features & ASSIGNED_DEVICE_SHARE_INTX_MASK &&
         kvm_has_intx_set_mask()) {
-        assigned_dev_data.flags |= KVM_DEV_ASSIGN_PCI_2_3;
+        flags |= KVM_DEV_ASSIGN_PCI_2_3;
     }
 
-    r = kvm_assign_pci_device(kvm_state, &assigned_dev_data);
+    r = kvm_device_pci_assign(kvm_state, &dev->host, flags, &dev->dev_id);
     if (r < 0) {
         fprintf(stderr, "Failed to assign device \"%s\" : %s\n",
                 dev->dev.qdev.id, strerror(-r));
@@ -889,7 +875,7 @@ static int assign_irq(AssignedDevice *dev)
     }
 
     memset(&assigned_irq_data, 0, sizeof(assigned_irq_data));
-    assigned_irq_data.assigned_dev_id = calc_assigned_dev_id(dev);
+    assigned_irq_data.assigned_dev_id = dev->dev_id;
     assigned_irq_data.guest_irq = intx_route.irq;
     if (dev->irq_requested_type) {
         assigned_irq_data.flags = dev->irq_requested_type;
@@ -940,13 +926,9 @@ retry:
 
 static void deassign_device(AssignedDevice *dev)
 {
-    struct kvm_assigned_pci_dev assigned_dev_data;
     int r;
 
-    memset(&assigned_dev_data, 0, sizeof(assigned_dev_data));
-    assigned_dev_data.assigned_dev_id = calc_assigned_dev_id(dev);
-
-    r = kvm_deassign_pci_device(kvm_state, &assigned_dev_data);
+    r = kvm_device_pci_deassign(kvm_state, dev->dev_id);
     if (r < 0)
 	fprintf(stderr, "Failed to deassign device \"%s\" : %s\n",
                 dev->dev.qdev.id, strerror(-r));
@@ -977,7 +959,7 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev)
     int r;
 
     memset(&assigned_irq_data, 0, sizeof assigned_irq_data);
-    assigned_irq_data.assigned_dev_id = calc_assigned_dev_id(assigned_dev);
+    assigned_irq_data.assigned_dev_id = assigned_dev->dev_id;
 
     /* Some guests gratuitously disable MSI even if they're not using it,
      * try to catch this by only deassigning irqs if the guest is using
@@ -1059,7 +1041,7 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
         return 0;
     }
 
-    msix_nr.assigned_dev_id = calc_assigned_dev_id(adev);
+    msix_nr.assigned_dev_id = adev->dev_id;
     msix_nr.entry_nr = entries_nr;
     r = kvm_assign_set_msix_nr(kvm_state, &msix_nr);
     if (r != 0) {
@@ -1121,7 +1103,7 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev)
     int r;
 
     memset(&assigned_irq_data, 0, sizeof assigned_irq_data);
-    assigned_irq_data.assigned_dev_id = calc_assigned_dev_id(assigned_dev);
+    assigned_irq_data.assigned_dev_id = assigned_dev->dev_id;
 
     /* Some guests gratuitously disable MSIX even if they're not using it,
      * try to catch this by only deassigning irqs if the guest is using
@@ -1200,8 +1182,7 @@ static void assigned_dev_pci_write_config(PCIDevice *pci_dev, uint32_t address,
                             PCI_COMMAND_INTX_DISABLE);
 
         if (intx_masked != !!(old_cmd & PCI_COMMAND_INTX_DISABLE)) {
-            ret = kvm_device_intx_set_mask(kvm_state,
-                                           calc_assigned_dev_id(assigned_dev),
+            ret = kvm_device_intx_set_mask(kvm_state, assigned_dev->dev_id,
                                            intx_masked);
             if (ret) {
                 perror("assigned_dev_pci_write_config: set intx mask");
@@ -1782,9 +1763,6 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
     dev->run = 0;
     dev->intx_route.mode = PCI_INTX_DISABLED;
     dev->intx_route.irq = -1;
-    dev->h_segnr = dev->host.domain;
-    dev->h_busnr = dev->host.bus;
-    dev->h_devfn = PCI_DEVFN(dev->host.slot, dev->host.function);
 
     /* assign device to guest */
     r = assign_device(dev);
diff --git a/qemu-kvm.c b/qemu-kvm.c
index efdeff1..7bf20d1 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -31,12 +31,6 @@
 #define ALIGN(x, y) (((x)+(y)-1) & ~((y)-1))
 
 #ifdef KVM_CAP_DEVICE_ASSIGNMENT
-int kvm_assign_pci_device(KVMState *s,
-                          struct kvm_assigned_pci_dev *assigned_dev)
-{
-    return kvm_vm_ioctl(s, KVM_ASSIGN_PCI_DEVICE, assigned_dev);
-}
-
 static int kvm_old_assign_irq(KVMState *s,
                               struct kvm_assigned_irq *assigned_irq)
 {
@@ -77,14 +71,6 @@ int kvm_assign_irq(KVMState *s, struct kvm_assigned_irq *assigned_irq)
 #endif
 #endif
 
-#ifdef KVM_CAP_DEVICE_DEASSIGNMENT
-int kvm_deassign_pci_device(KVMState *s,
-                            struct kvm_assigned_pci_dev *assigned_dev)
-{
-    return kvm_vm_ioctl(s, KVM_DEASSIGN_PCI_DEVICE, assigned_dev);
-}
-#endif
-
 int kvm_del_routing_entry(struct kvm_irq_routing_entry *entry)
 {
 #ifdef KVM_CAP_IRQ_ROUTING
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 3ebbbb6..54e615a 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -33,18 +33,6 @@
 #include "kvm.h"
 
 /*!
- * \brief Notifies host kernel about a PCI device to be assigned to a guest
- *
- * Used for PCI device assignment, this function notifies the host
- * kernel about the assigning of the physical PCI device to a guest.
- *
- * \param kvm Pointer to the current kvm_context
- * \param assigned_dev Parameters, like bus, devfn number, etc
- */
-int kvm_assign_pci_device(KVMState *s,
-                          struct kvm_assigned_pci_dev *assigned_dev);
-
-/*!
  * \brief Assign IRQ for an assigned device
  *
  * Used for PCI device assignment, this function assigns IRQ numbers for
@@ -68,18 +56,6 @@ int kvm_deassign_irq(KVMState *s, struct kvm_assigned_irq *assigned_irq);
 
 int kvm_device_intx_set_mask(KVMState *s, uint32_t dev_id, bool masked);
 
-/*!
- * \brief Notifies host kernel about a PCI device to be deassigned from a guest
- *
- * Used for hot remove PCI device, this function notifies the host
- * kernel about the deassigning of the physical PCI device from a guest.
- *
- * \param kvm Pointer to the current kvm_context
- * \param assigned_dev Parameters, like bus, devfn number, etc
- */
-int kvm_deassign_pci_device(KVMState *s,
-                            struct kvm_assigned_pci_dev *assigned_dev);
-
 struct kvm_irq_routing_entry;
 
 void kvm_add_routing_entry(KVMState *s, struct kvm_irq_routing_entry *entry);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 696b14a..f944f83 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -31,6 +31,7 @@
 #include "hw/apic.h"
 #include "ioport.h"
 #include "hyperv.h"
+#include "hw/pci.h"
 
 //#define DEBUG_KVM
 
@@ -2055,3 +2056,38 @@ void kvm_arch_init_irq_routing(KVMState *s)
     kvm_msi_via_irqfd_allowed = true;
     kvm_gsi_routing_allowed = true;
 }
+
+/* Classic KVM device assignment interface. Will remain x86 only. */
+int kvm_device_pci_assign(KVMState *s, PCIHostDeviceAddress *dev_addr,
+                          uint32_t flags, uint32_t *dev_id)
+{
+    struct kvm_assigned_pci_dev dev_data;
+    int ret;
+
+    memset(&dev_data, 0, sizeof(dev_data));
+    dev_data.segnr = dev_addr->domain;
+    dev_data.busnr = dev_addr->bus;
+    dev_data.devfn = PCI_DEVFN(dev_addr->slot, dev_addr->function);
+    dev_data.assigned_dev_id =
+        (dev_addr->domain << 16) | (dev_addr->bus << 8) | dev_data.devfn;
+    dev_data.flags = flags;
+
+    ret = kvm_vm_ioctl(s, KVM_ASSIGN_PCI_DEVICE, &dev_data);
+    if (ret < 0) {
+        return ret;
+    }
+
+    *dev_id = dev_data.assigned_dev_id;
+
+    return 0;
+}
+
+int kvm_device_pci_deassign(KVMState *s, uint32_t dev_id)
+{
+    struct kvm_assigned_pci_dev dev_data;
+
+    memset(&dev_data, 0, sizeof(dev_data));
+    dev_data.assigned_dev_id = dev_id;
+
+    return kvm_vm_ioctl(s, KVM_DEASSIGN_PCI_DEVICE, &dev_data);
+}
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index b82bbf4..5161f67 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -11,6 +11,12 @@
 #ifndef QEMU_KVM_I386_H
 #define QEMU_KVM_I386_H
 
+#include "kvm.h"
+
 bool kvm_allows_irq0_override(void);
 
+int kvm_device_pci_assign(KVMState *s, PCIHostDeviceAddress *dev_addr,
+                          uint32_t flags, uint32_t *dev_id);
+int kvm_device_pci_deassign(KVMState *s, uint32_t dev_id);
+
 #endif
-- 
1.7.3.4


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

* [PATCH 03/19] pci-assign: Rename assign_irq to assign_intx
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
  2012-08-16 13:54 ` [PATCH 01/19] pci-assign: Only clean up registered IO resources Jan Kiszka
  2012-08-16 13:54 ` [PATCH 02/19] pci-assign: Factor out kvm_device_pci_assign/deassign Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 13:54 ` [PATCH 04/19] pci-assign: Refactor interrupt deassignment Jan Kiszka
                   ` (17 subsequent siblings)
  20 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

The previous name may incorrectly suggest that this function assigns all
types of IRQs though it's only dealing with legacy interrupts.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/device-assignment.c |   14 +++++++-------
 1 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 3f6196a..ee64c33 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -851,7 +851,7 @@ static int assign_device(AssignedDevice *dev)
     return r;
 }
 
-static int assign_irq(AssignedDevice *dev)
+static int assign_intx(AssignedDevice *dev)
 {
     struct kvm_assigned_irq assigned_irq_data;
     PCIINTxRoute intx_route;
@@ -881,7 +881,7 @@ static int assign_irq(AssignedDevice *dev)
         assigned_irq_data.flags = dev->irq_requested_type;
         r = kvm_deassign_irq(kvm_state, &assigned_irq_data);
         if (r) {
-            perror("assign_irq: deassign");
+            perror("assign_intx: deassign");
         }
         dev->irq_requested_type = 0;
     }
@@ -943,7 +943,7 @@ static void assigned_dev_update_irq_routing(PCIDevice *dev)
     Error *err = NULL;
     int r;
 
-    r = assign_irq(assigned_dev);
+    r = assign_intx(assigned_dev);
     if (r < 0) {
         qdev_unplug(&dev->qdev, &err);
         assert(!err);
@@ -1008,7 +1008,7 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev)
         assigned_dev->intx_route.irq = -1;
         assigned_dev->irq_requested_type = assigned_irq_data.flags;
     } else {
-        assign_irq(assigned_dev);
+        assign_intx(assigned_dev);
     }
 }
 
@@ -1141,7 +1141,7 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev)
         assigned_dev->intx_route.irq = -1;
         assigned_dev->irq_requested_type = assigned_irq_data.flags;
     } else {
-        assign_irq(assigned_dev);
+        assign_intx(assigned_dev);
     }
 }
 
@@ -1769,8 +1769,8 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
     if (r < 0)
         goto out;
 
-    /* assign irq for the device */
-    r = assign_irq(dev);
+    /* assign legacy INTx to the device */
+    r = assign_intx(dev);
     if (r < 0)
         goto assigned_out;
 
-- 
1.7.3.4


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

* [PATCH 04/19] pci-assign: Refactor interrupt deassignment
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
                   ` (2 preceding siblings ...)
  2012-08-16 13:54 ` [PATCH 03/19] pci-assign: Rename assign_irq to assign_intx Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 13:54 ` [PATCH 05/19] pci-assign: Factor out kvm_device_intx_assign Jan Kiszka
                   ` (16 subsequent siblings)
  20 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

Introduce three new KVM services, kvm_device_intx/msi/msix_deassign, to
release assigned interrupts. These no longer require to pass
pre-initialized kvm_assigned_irq structures but only request required
parameters.

To trace which type of interrupt is currently assigned, use a new enum
AssignedIRQType instead of KVM constants.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/device-assignment.c |   80 ++++++++++++++++++++++++++++++-----------------
 qemu-kvm.c             |    5 ---
 qemu-kvm.h             |   11 ------
 target-i386/kvm.c      |   29 +++++++++++++++++
 target-i386/kvm_i386.h |    6 +++
 5 files changed, 86 insertions(+), 45 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index ee64c33..0b33c04 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -106,6 +106,14 @@ typedef struct {
     uint32_t ctrl;
 } MSIXTableEntry;
 
+typedef enum AssignedIRQType {
+    ASSIGNED_IRQ_NONE = 0,
+    ASSIGNED_IRQ_INTX_HOST_INTX,
+    ASSIGNED_IRQ_INTX_HOST_MSI,
+    ASSIGNED_IRQ_MSI,
+    ASSIGNED_IRQ_MSIX
+} AssignedIRQType;
+
 typedef struct AssignedDevice {
     PCIDevice dev;
     PCIHostDeviceAddress host;
@@ -117,7 +125,7 @@ typedef struct AssignedDevice {
     PCIDevRegions real_device;
     int run;
     PCIINTxRoute intx_route;
-    int irq_requested_type;
+    AssignedIRQType assigned_irq_type;
     int bound;
     struct {
 #define ASSIGNED_DEVICE_CAP_MSI (1 << 0)
@@ -854,7 +862,9 @@ static int assign_device(AssignedDevice *dev)
 static int assign_intx(AssignedDevice *dev)
 {
     struct kvm_assigned_irq assigned_irq_data;
+    AssignedIRQType new_type;
     PCIINTxRoute intx_route;
+    bool intx_host_msi;
     int r = 0;
 
     /* Interrupt PIN 0 means don't use INTx */
@@ -874,17 +884,26 @@ static int assign_intx(AssignedDevice *dev)
         return r;
     }
 
-    memset(&assigned_irq_data, 0, sizeof(assigned_irq_data));
-    assigned_irq_data.assigned_dev_id = dev->dev_id;
-    assigned_irq_data.guest_irq = intx_route.irq;
-    if (dev->irq_requested_type) {
-        assigned_irq_data.flags = dev->irq_requested_type;
-        r = kvm_deassign_irq(kvm_state, &assigned_irq_data);
-        if (r) {
-            perror("assign_intx: deassign");
-        }
-        dev->irq_requested_type = 0;
+    switch (dev->assigned_irq_type) {
+    case ASSIGNED_IRQ_INTX_HOST_INTX:
+    case ASSIGNED_IRQ_INTX_HOST_MSI:
+        intx_host_msi = dev->assigned_irq_type == ASSIGNED_IRQ_INTX_HOST_MSI;
+        r = kvm_device_intx_deassign(kvm_state, dev->dev_id, intx_host_msi);
+        break;
+    case ASSIGNED_IRQ_MSI:
+        r = kvm_device_msi_deassign(kvm_state, dev->dev_id);
+        break;
+    case ASSIGNED_IRQ_MSIX:
+        r = kvm_device_msix_deassign(kvm_state, dev->dev_id);
+        break;
+    default:
+        r = 0;
+        break;
     }
+    if (r) {
+        perror("assign_intx: deassign");
+    }
+    dev->assigned_irq_type = ASSIGNED_IRQ_NONE;
 
     if (intx_route.mode == PCI_INTX_DISABLED) {
         dev->intx_route = intx_route;
@@ -892,12 +911,18 @@ static int assign_intx(AssignedDevice *dev)
     }
 
 retry:
+    memset(&assigned_irq_data, 0, sizeof(assigned_irq_data));
+    assigned_irq_data.assigned_dev_id = dev->dev_id;
+    assigned_irq_data.guest_irq = intx_route.irq;
     assigned_irq_data.flags = KVM_DEV_IRQ_GUEST_INTX;
     if (dev->features & ASSIGNED_DEVICE_PREFER_MSI_MASK &&
-        dev->cap.available & ASSIGNED_DEVICE_CAP_MSI)
+        dev->cap.available & ASSIGNED_DEVICE_CAP_MSI) {
         assigned_irq_data.flags |= KVM_DEV_IRQ_HOST_MSI;
-    else
+        new_type = ASSIGNED_IRQ_INTX_HOST_MSI;
+    } else {
         assigned_irq_data.flags |= KVM_DEV_IRQ_HOST_INTX;
+        new_type = ASSIGNED_IRQ_INTX_HOST_INTX;
+    }
 
     r = kvm_assign_irq(kvm_state, &assigned_irq_data);
     if (r < 0) {
@@ -920,7 +945,7 @@ retry:
     }
 
     dev->intx_route = intx_route;
-    dev->irq_requested_type = assigned_irq_data.flags;
+    dev->assigned_irq_type = new_type;
     return r;
 }
 
@@ -958,23 +983,19 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev)
                                      PCI_MSI_FLAGS);
     int r;
 
-    memset(&assigned_irq_data, 0, sizeof assigned_irq_data);
-    assigned_irq_data.assigned_dev_id = assigned_dev->dev_id;
-
     /* Some guests gratuitously disable MSI even if they're not using it,
      * try to catch this by only deassigning irqs if the guest is using
      * MSI or intends to start. */
-    if ((assigned_dev->irq_requested_type & KVM_DEV_IRQ_GUEST_MSI) ||
+    if (assigned_dev->assigned_irq_type == ASSIGNED_IRQ_MSI ||
         (ctrl_byte & PCI_MSI_FLAGS_ENABLE)) {
 
-        assigned_irq_data.flags = assigned_dev->irq_requested_type;
         free_dev_irq_entries(assigned_dev);
-        r = kvm_deassign_irq(kvm_state, &assigned_irq_data);
+        r = kvm_device_msi_deassign(kvm_state, assigned_dev->dev_id);
         /* -ENXIO means no assigned irq */
         if (r && r != -ENXIO)
             perror("assigned_dev_update_msi: deassign irq");
 
-        assigned_dev->irq_requested_type = 0;
+        assigned_dev->assigned_irq_type = ASSIGNED_IRQ_NONE;
         pci_device_set_intx_routing_notifier(pci_dev, NULL);
     }
 
@@ -998,6 +1019,8 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev)
         kvm_irqchip_commit_routes(kvm_state);
 	assigned_dev->irq_entries_nr = 1;
 
+        memset(&assigned_irq_data, 0, sizeof assigned_irq_data);
+        assigned_irq_data.assigned_dev_id = assigned_dev->dev_id;
         assigned_irq_data.guest_irq = assigned_dev->entry->gsi;
 	assigned_irq_data.flags = KVM_DEV_IRQ_HOST_MSI | KVM_DEV_IRQ_GUEST_MSI;
         if (kvm_assign_irq(kvm_state, &assigned_irq_data) < 0) {
@@ -1006,7 +1029,7 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev)
 
         assigned_dev->intx_route.mode = PCI_INTX_DISABLED;
         assigned_dev->intx_route.irq = -1;
-        assigned_dev->irq_requested_type = assigned_irq_data.flags;
+        assigned_dev->assigned_irq_type = ASSIGNED_IRQ_MSI;
     } else {
         assign_intx(assigned_dev);
     }
@@ -1108,17 +1131,16 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev)
     /* Some guests gratuitously disable MSIX even if they're not using it,
      * try to catch this by only deassigning irqs if the guest is using
      * MSIX or intends to start. */
-    if ((assigned_dev->irq_requested_type & KVM_DEV_IRQ_GUEST_MSIX) ||
+    if ((assigned_dev->assigned_irq_type == ASSIGNED_IRQ_MSIX) ||
         (ctrl_word & PCI_MSIX_FLAGS_ENABLE)) {
 
-        assigned_irq_data.flags = assigned_dev->irq_requested_type;
         free_dev_irq_entries(assigned_dev);
-        r = kvm_deassign_irq(kvm_state, &assigned_irq_data);
+        r = kvm_device_msix_deassign(kvm_state, assigned_dev->dev_id);
         /* -ENXIO means no assigned irq */
         if (r && r != -ENXIO)
             perror("assigned_dev_update_msix: deassign irq");
 
-        assigned_dev->irq_requested_type = 0;
+        assigned_dev->assigned_irq_type = ASSIGNED_IRQ_NONE;
         pci_device_set_intx_routing_notifier(pci_dev, NULL);
     }
 
@@ -1139,7 +1161,7 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev)
         }
         assigned_dev->intx_route.mode = PCI_INTX_DISABLED;
         assigned_dev->intx_route.irq = -1;
-        assigned_dev->irq_requested_type = assigned_irq_data.flags;
+        assigned_dev->assigned_irq_type = ASSIGNED_IRQ_MSIX;
     } else {
         assign_intx(assigned_dev);
     }
@@ -1655,14 +1677,14 @@ static void reset_assigned_device(DeviceState *dev)
      * enabled since it lives in MMIO space, which is about to get
      * disabled.
      */
-    if (adev->irq_requested_type & KVM_DEV_IRQ_GUEST_MSIX) {
+    if (adev->assigned_irq_type == ASSIGNED_IRQ_MSIX) {
         uint16_t ctrl = pci_get_word(pci_dev->config +
                                      pci_dev->msix_cap + PCI_MSIX_FLAGS);
 
         pci_set_word(pci_dev->config + pci_dev->msix_cap + PCI_MSIX_FLAGS,
                      ctrl & ~PCI_MSIX_FLAGS_ENABLE);
         assigned_dev_update_msix(pci_dev);
-    } else if (adev->irq_requested_type & KVM_DEV_IRQ_GUEST_MSI) {
+    } else if (adev->assigned_irq_type == ASSIGNED_IRQ_MSI) {
         uint8_t ctrl = pci_get_byte(pci_dev->config +
                                     pci_dev->msi_cap + PCI_MSI_FLAGS);
 
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 7bf20d1..8bc9857 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -58,11 +58,6 @@ int kvm_assign_irq(KVMState *s, struct kvm_assigned_irq *assigned_irq)
 
     return kvm_old_assign_irq(s, assigned_irq);
 }
-
-int kvm_deassign_irq(KVMState *s, struct kvm_assigned_irq *assigned_irq)
-{
-    return kvm_vm_ioctl(s, KVM_DEASSIGN_DEV_IRQ, assigned_irq);
-}
 #else
 int kvm_assign_irq(KVMState *s, struct kvm_assigned_irq *assigned_irq)
 {
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 54e615a..1cdface 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -43,17 +43,6 @@
  */
 int kvm_assign_irq(KVMState *s, struct kvm_assigned_irq *assigned_irq);
 
-/*!
- * \brief Deassign IRQ for an assigned device
- *
- * Used for PCI device assignment, this function deassigns IRQ numbers
- * for an assigned device.
- *
- * \param kvm Pointer to the current kvm_context
- * \param assigned_irq Parameters, like dev id, host irq, guest irq, etc
- */
-int kvm_deassign_irq(KVMState *s, struct kvm_assigned_irq *assigned_irq);
-
 int kvm_device_intx_set_mask(KVMState *s, uint32_t dev_id, bool masked);
 
 struct kvm_irq_routing_entry;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index f944f83..058ed3f 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2091,3 +2091,32 @@ int kvm_device_pci_deassign(KVMState *s, uint32_t dev_id)
 
     return kvm_vm_ioctl(s, KVM_DEASSIGN_PCI_DEVICE, &dev_data);
 }
+
+static int kvm_deassign_irq_internal(KVMState *s, uint32_t dev_id,
+                                     uint32_t type)
+{
+    struct kvm_assigned_irq assigned_irq = {
+        .assigned_dev_id = dev_id,
+        .flags = type,
+    };
+
+    return kvm_vm_ioctl(s, KVM_DEASSIGN_DEV_IRQ, &assigned_irq);
+}
+
+int kvm_device_intx_deassign(KVMState *s, uint32_t dev_id, bool use_host_msi)
+{
+    return kvm_deassign_irq_internal(s, dev_id, KVM_DEV_IRQ_GUEST_INTX |
+        (use_host_msi ? KVM_DEV_IRQ_HOST_MSI : KVM_DEV_IRQ_HOST_INTX));
+}
+
+int kvm_device_msi_deassign(KVMState *s, uint32_t dev_id)
+{
+    return kvm_deassign_irq_internal(s, dev_id, KVM_DEV_IRQ_GUEST_MSI |
+                                                KVM_DEV_IRQ_HOST_MSI);
+}
+
+int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id)
+{
+    return kvm_deassign_irq_internal(s, dev_id, KVM_DEV_IRQ_GUEST_MSIX |
+                                                KVM_DEV_IRQ_HOST_MSIX);
+}
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index 5161f67..fdecdd5 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -19,4 +19,10 @@ int kvm_device_pci_assign(KVMState *s, PCIHostDeviceAddress *dev_addr,
                           uint32_t flags, uint32_t *dev_id);
 int kvm_device_pci_deassign(KVMState *s, uint32_t dev_id);
 
+int kvm_device_intx_deassign(KVMState *s, uint32_t dev_id, bool use_host_msi);
+
+int kvm_device_msi_deassign(KVMState *s, uint32_t dev_id);
+
+int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id);
+
 #endif
-- 
1.7.3.4


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

* [PATCH 05/19] pci-assign: Factor out kvm_device_intx_assign
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
                   ` (3 preceding siblings ...)
  2012-08-16 13:54 ` [PATCH 04/19] pci-assign: Refactor interrupt deassignment Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 13:54 ` [PATCH 06/19] qemu-kvm: Move kvm_device_intx_set_mask service Jan Kiszka
                   ` (15 subsequent siblings)
  20 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

Avoid passing kvm_assigned_irq on INTx assignment and separate this
function from (to-be-refactored) MSI/MSI-X assignment.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/device-assignment.c |   16 ++++++----------
 target-i386/kvm.c      |   24 ++++++++++++++++++++++++
 target-i386/kvm_i386.h |    2 ++
 3 files changed, 32 insertions(+), 10 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 0b33c04..d448fdc 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -861,11 +861,10 @@ static int assign_device(AssignedDevice *dev)
 
 static int assign_intx(AssignedDevice *dev)
 {
-    struct kvm_assigned_irq assigned_irq_data;
     AssignedIRQType new_type;
     PCIINTxRoute intx_route;
     bool intx_host_msi;
-    int r = 0;
+    int r;
 
     /* Interrupt PIN 0 means don't use INTx */
     if (assigned_dev_pci_read_byte(&dev->dev, PCI_INTERRUPT_PIN) == 0) {
@@ -881,7 +880,7 @@ static int assign_intx(AssignedDevice *dev)
 
     if (dev->intx_route.mode == intx_route.mode &&
         dev->intx_route.irq == intx_route.irq) {
-        return r;
+        return 0;
     }
 
     switch (dev->assigned_irq_type) {
@@ -911,20 +910,17 @@ static int assign_intx(AssignedDevice *dev)
     }
 
 retry:
-    memset(&assigned_irq_data, 0, sizeof(assigned_irq_data));
-    assigned_irq_data.assigned_dev_id = dev->dev_id;
-    assigned_irq_data.guest_irq = intx_route.irq;
-    assigned_irq_data.flags = KVM_DEV_IRQ_GUEST_INTX;
     if (dev->features & ASSIGNED_DEVICE_PREFER_MSI_MASK &&
         dev->cap.available & ASSIGNED_DEVICE_CAP_MSI) {
-        assigned_irq_data.flags |= KVM_DEV_IRQ_HOST_MSI;
+        intx_host_msi = true;
         new_type = ASSIGNED_IRQ_INTX_HOST_MSI;
     } else {
-        assigned_irq_data.flags |= KVM_DEV_IRQ_HOST_INTX;
+        intx_host_msi = false;
         new_type = ASSIGNED_IRQ_INTX_HOST_INTX;
     }
 
-    r = kvm_assign_irq(kvm_state, &assigned_irq_data);
+    r = kvm_device_intx_assign(kvm_state, dev->dev_id, intx_host_msi,
+                               intx_route.irq);
     if (r < 0) {
         if (r == -EIO && !(dev->features & ASSIGNED_DEVICE_PREFER_MSI_MASK) &&
             dev->cap.available & ASSIGNED_DEVICE_CAP_MSI) {
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 058ed3f..e2041f4 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2092,6 +2092,30 @@ int kvm_device_pci_deassign(KVMState *s, uint32_t dev_id)
     return kvm_vm_ioctl(s, KVM_DEASSIGN_PCI_DEVICE, &dev_data);
 }
 
+static int kvm_assign_irq_internal(KVMState *s, uint32_t dev_id,
+                                   uint32_t irq_type, uint32_t guest_irq)
+{
+    struct kvm_assigned_irq assigned_irq;
+
+    memset(&assigned_irq, 0, sizeof(assigned_irq));
+    assigned_irq.assigned_dev_id = dev_id;
+    assigned_irq.guest_irq = guest_irq;
+    assigned_irq.flags = irq_type;
+    if (kvm_check_extension(s, KVM_CAP_ASSIGN_DEV_IRQ)) {
+        return kvm_vm_ioctl(s, KVM_ASSIGN_DEV_IRQ, &assigned_irq);
+    } else {
+        return kvm_vm_ioctl(s, KVM_ASSIGN_IRQ, &assigned_irq);
+    }
+}
+
+int kvm_device_intx_assign(KVMState *s, uint32_t dev_id, bool use_host_msi,
+                           uint32_t guest_irq)
+{
+    uint32_t irq_type = KVM_DEV_IRQ_GUEST_INTX |
+        (use_host_msi ? KVM_DEV_IRQ_HOST_MSI : KVM_DEV_IRQ_HOST_INTX);
+    return kvm_assign_irq_internal(s, dev_id, irq_type, guest_irq);
+}
+
 static int kvm_deassign_irq_internal(KVMState *s, uint32_t dev_id,
                                      uint32_t type)
 {
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index fdecdd5..5a24168 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -19,6 +19,8 @@ int kvm_device_pci_assign(KVMState *s, PCIHostDeviceAddress *dev_addr,
                           uint32_t flags, uint32_t *dev_id);
 int kvm_device_pci_deassign(KVMState *s, uint32_t dev_id);
 
+int kvm_device_intx_assign(KVMState *s, uint32_t dev_id,
+                           bool use_host_msi, uint32_t guest_irq);
 int kvm_device_intx_deassign(KVMState *s, uint32_t dev_id, bool use_host_msi);
 
 int kvm_device_msi_deassign(KVMState *s, uint32_t dev_id);
-- 
1.7.3.4


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

* [PATCH 06/19] qemu-kvm: Move kvm_device_intx_set_mask service
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
                   ` (4 preceding siblings ...)
  2012-08-16 13:54 ` [PATCH 05/19] pci-assign: Factor out kvm_device_intx_assign Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 13:54 ` [PATCH 07/19] pci-assign: Rework MSI assignment Jan Kiszka
                   ` (14 subsequent siblings)
  20 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

Move kvm_device_intx_set_mask prototype and implementation to their
upstream positions.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 qemu-kvm.c             |    9 ---------
 qemu-kvm.h             |    2 --
 target-i386/kvm.c      |    9 +++++++++
 target-i386/kvm_i386.h |    1 +
 4 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/qemu-kvm.c b/qemu-kvm.c
index 8bc9857..8416a8d 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -37,15 +37,6 @@ static int kvm_old_assign_irq(KVMState *s,
     return kvm_vm_ioctl(s, KVM_ASSIGN_IRQ, assigned_irq);
 }
 
-int kvm_device_intx_set_mask(KVMState *s, uint32_t dev_id, bool masked)
-{
-    struct kvm_assigned_pci_dev assigned_dev;
-
-    assigned_dev.assigned_dev_id = dev_id;
-    assigned_dev.flags = masked ? KVM_DEV_ASSIGN_MASK_INTX : 0;
-    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_INTX_MASK, &assigned_dev);
-}
-
 #ifdef KVM_CAP_ASSIGN_DEV_IRQ
 int kvm_assign_irq(KVMState *s, struct kvm_assigned_irq *assigned_irq)
 {
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 1cdface..c247ad0 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -43,8 +43,6 @@
  */
 int kvm_assign_irq(KVMState *s, struct kvm_assigned_irq *assigned_irq);
 
-int kvm_device_intx_set_mask(KVMState *s, uint32_t dev_id, bool masked);
-
 struct kvm_irq_routing_entry;
 
 void kvm_add_routing_entry(KVMState *s, struct kvm_irq_routing_entry *entry);
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index e2041f4..4941744 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2116,6 +2116,15 @@ int kvm_device_intx_assign(KVMState *s, uint32_t dev_id, bool use_host_msi,
     return kvm_assign_irq_internal(s, dev_id, irq_type, guest_irq);
 }
 
+int kvm_device_intx_set_mask(KVMState *s, uint32_t dev_id, bool masked)
+{
+    struct kvm_assigned_pci_dev assigned_dev;
+
+    assigned_dev.assigned_dev_id = dev_id;
+    assigned_dev.flags = masked ? KVM_DEV_ASSIGN_MASK_INTX : 0;
+    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_INTX_MASK, &assigned_dev);
+}
+
 static int kvm_deassign_irq_internal(KVMState *s, uint32_t dev_id,
                                      uint32_t type)
 {
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index 5a24168..28f26bb 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -21,6 +21,7 @@ int kvm_device_pci_deassign(KVMState *s, uint32_t dev_id);
 
 int kvm_device_intx_assign(KVMState *s, uint32_t dev_id,
                            bool use_host_msi, uint32_t guest_irq);
+int kvm_device_intx_set_mask(KVMState *s, uint32_t dev_id, bool masked);
 int kvm_device_intx_deassign(KVMState *s, uint32_t dev_id, bool use_host_msi);
 
 int kvm_device_msi_deassign(KVMState *s, uint32_t dev_id);
-- 
1.7.3.4


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

* [PATCH 07/19] pci-assign: Rework MSI assignment
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
                   ` (5 preceding siblings ...)
  2012-08-16 13:54 ` [PATCH 06/19] qemu-kvm: Move kvm_device_intx_set_mask service Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 13:54 ` [PATCH 08/19] pci-assign: Factor out kvm_device_msix_supported Jan Kiszka
                   ` (13 subsequent siblings)
  20 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

Introduce kvm_device_msi_assign and use upstream's
kvm_irqchip_add_msi_route/kvm_irqchip_release_virq to provide MSI
support for assigned devices.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/device-assignment.c |   54 +++++++++++++++++++++++++----------------------
 target-i386/kvm.c      |    6 +++++
 target-i386/kvm_i386.h |    1 +
 3 files changed, 36 insertions(+), 25 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index d448fdc..1d0af34 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -41,6 +41,7 @@
 #include "range.h"
 #include "sysemu.h"
 #include "pci.h"
+#include "msi.h"
 #include "kvm_i386.h"
 
 #define MSIX_PAGE_SIZE 0x1000
@@ -138,6 +139,8 @@ typedef struct AssignedDevice {
     } cap;
     uint8_t emulate_config_read[PCI_CONFIG_SPACE_SIZE];
     uint8_t emulate_config_write[PCI_CONFIG_SPACE_SIZE];
+    int msi_virq_nr;
+    int *msi_virq;
     int irq_entries_nr;
     struct kvm_irq_routing_entry *entry;
     MSIXTableEntry *msix_table;
@@ -702,6 +705,16 @@ static void free_dev_irq_entries(AssignedDevice *dev)
 {
     int i;
 
+    for (i = 0; i < dev->msi_virq_nr; i++) {
+        if (dev->msi_virq[i] >= 0) {
+            kvm_irqchip_release_virq(kvm_state, dev->msi_virq[i]);
+            dev->msi_virq[i] = -1;
+        }
+    }
+    g_free(dev->msi_virq);
+    dev->msi_virq = NULL;
+    dev->msi_virq_nr = 0;
+
     for (i = 0; i < dev->irq_entries_nr; i++) {
         if (dev->entry[i].type) {
             kvm_del_routing_entry(&dev->entry[i]);
@@ -973,7 +986,6 @@ static void assigned_dev_update_irq_routing(PCIDevice *dev)
 
 static void assigned_dev_update_msi(PCIDevice *pci_dev)
 {
-    struct kvm_assigned_irq assigned_irq_data;
     AssignedDevice *assigned_dev = DO_UPCAST(AssignedDevice, dev, pci_dev);
     uint8_t ctrl_byte = pci_get_byte(pci_dev->config + pci_dev->msi_cap +
                                      PCI_MSI_FLAGS);
@@ -984,43 +996,35 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev)
      * MSI or intends to start. */
     if (assigned_dev->assigned_irq_type == ASSIGNED_IRQ_MSI ||
         (ctrl_byte & PCI_MSI_FLAGS_ENABLE)) {
-
-        free_dev_irq_entries(assigned_dev);
         r = kvm_device_msi_deassign(kvm_state, assigned_dev->dev_id);
         /* -ENXIO means no assigned irq */
         if (r && r != -ENXIO)
             perror("assigned_dev_update_msi: deassign irq");
 
+        free_dev_irq_entries(assigned_dev);
+
         assigned_dev->assigned_irq_type = ASSIGNED_IRQ_NONE;
         pci_device_set_intx_routing_notifier(pci_dev, NULL);
     }
 
     if (ctrl_byte & PCI_MSI_FLAGS_ENABLE) {
         uint8_t *pos = pci_dev->config + pci_dev->msi_cap;
-
-        assigned_dev->entry = g_malloc0(sizeof(*(assigned_dev->entry)));
-        assigned_dev->entry->u.msi.address_lo =
-            pci_get_long(pos + PCI_MSI_ADDRESS_LO);
-        assigned_dev->entry->u.msi.address_hi = 0;
-        assigned_dev->entry->u.msi.data = pci_get_word(pos + PCI_MSI_DATA_32);
-        assigned_dev->entry->type = KVM_IRQ_ROUTING_MSI;
-        r = kvm_get_irq_route_gsi();
-        if (r < 0) {
-            perror("assigned_dev_update_msi: kvm_get_irq_route_gsi");
+        MSIMessage msg;
+        int virq;
+
+        msg.address = pci_get_long(pos + PCI_MSI_ADDRESS_LO);
+        msg.data = pci_get_word(pos + PCI_MSI_DATA_32);
+        virq = kvm_irqchip_add_msi_route(kvm_state, msg);
+        if (virq < 0) {
+            perror("assigned_dev_update_msi: kvm_irqchip_add_msi_route");
             return;
         }
-        assigned_dev->entry->gsi = r;
 
-        kvm_add_routing_entry(kvm_state, assigned_dev->entry);
-        kvm_irqchip_commit_routes(kvm_state);
-	assigned_dev->irq_entries_nr = 1;
-
-        memset(&assigned_irq_data, 0, sizeof assigned_irq_data);
-        assigned_irq_data.assigned_dev_id = assigned_dev->dev_id;
-        assigned_irq_data.guest_irq = assigned_dev->entry->gsi;
-	assigned_irq_data.flags = KVM_DEV_IRQ_HOST_MSI | KVM_DEV_IRQ_GUEST_MSI;
-        if (kvm_assign_irq(kvm_state, &assigned_irq_data) < 0) {
-            perror("assigned_dev_enable_msi: assign irq");
+        assigned_dev->msi_virq = g_malloc(sizeof(*assigned_dev->msi_virq));
+        assigned_dev->msi_virq_nr = 1;
+        assigned_dev->msi_virq[0] = virq;
+        if (kvm_device_msi_assign(kvm_state, assigned_dev->dev_id, virq) < 0) {
+            perror("assigned_dev_update_msi: kvm_device_msi_assign");
         }
 
         assigned_dev->intx_route.mode = PCI_INTX_DISABLED;
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 4941744..04d1c7d 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2142,6 +2142,12 @@ int kvm_device_intx_deassign(KVMState *s, uint32_t dev_id, bool use_host_msi)
         (use_host_msi ? KVM_DEV_IRQ_HOST_MSI : KVM_DEV_IRQ_HOST_INTX));
 }
 
+int kvm_device_msi_assign(KVMState *s, uint32_t dev_id, int virq)
+{
+    return kvm_assign_irq_internal(s, dev_id, KVM_DEV_IRQ_HOST_MSI |
+                                              KVM_DEV_IRQ_GUEST_MSI, virq);
+}
+
 int kvm_device_msi_deassign(KVMState *s, uint32_t dev_id)
 {
     return kvm_deassign_irq_internal(s, dev_id, KVM_DEV_IRQ_GUEST_MSI |
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index 28f26bb..e827f5b 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -24,6 +24,7 @@ int kvm_device_intx_assign(KVMState *s, uint32_t dev_id,
 int kvm_device_intx_set_mask(KVMState *s, uint32_t dev_id, bool masked);
 int kvm_device_intx_deassign(KVMState *s, uint32_t dev_id, bool use_host_msi);
 
+int kvm_device_msi_assign(KVMState *s, uint32_t dev_id, int virq);
 int kvm_device_msi_deassign(KVMState *s, uint32_t dev_id);
 
 int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id);
-- 
1.7.3.4


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

* [PATCH 08/19] pci-assign: Factor out kvm_device_msix_supported
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
                   ` (6 preceding siblings ...)
  2012-08-16 13:54 ` [PATCH 07/19] pci-assign: Rework MSI assignment Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 13:54 ` [PATCH 09/19] pci-assign: Replace kvm_assign_set_msix_nr with kvm_device_msix_init_vectors Jan Kiszka
                   ` (12 subsequent siblings)
  20 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

Encapsulate the ugly check if MSI-X assignment is supported in a
separate helper function.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/device-assignment.c |    5 +----
 target-i386/kvm.c      |    7 +++++++
 target-i386/kvm_i386.h |    1 +
 3 files changed, 9 insertions(+), 4 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 1d0af34..80ac2fc 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -1283,10 +1283,7 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
     }
     /* Expose MSI-X capability */
     pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSIX, 0);
-    /* Would really like to test kvm_check_extension(, KVM_CAP_DEVICE_MSIX),
-     * but the kernel doesn't expose it.  Instead do a dummy call to
-     * KVM_ASSIGN_SET_MSIX_NR to see if it exists. */
-    if (pos != 0 && kvm_assign_set_msix_nr(kvm_state, NULL) == -EFAULT) {
+    if (pos != 0 && kvm_device_msix_supported(kvm_state)) {
         int bar_nr;
         uint32_t msix_table_entry;
 
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 04d1c7d..677a791 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2154,6 +2154,13 @@ int kvm_device_msi_deassign(KVMState *s, uint32_t dev_id)
                                                 KVM_DEV_IRQ_HOST_MSI);
 }
 
+bool kvm_device_msix_supported(KVMState *s)
+{
+    /* The kernel lacks a corresponding KVM_CAP, so we probe by calling
+     * KVM_ASSIGN_SET_MSIX_NR with an invalid parameter. */
+    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, NULL) == -EFAULT;
+}
+
 int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id)
 {
     return kvm_deassign_irq_internal(s, dev_id, KVM_DEV_IRQ_GUEST_MSIX |
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index e827f5b..6f66b6d 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -27,6 +27,7 @@ int kvm_device_intx_deassign(KVMState *s, uint32_t dev_id, bool use_host_msi);
 int kvm_device_msi_assign(KVMState *s, uint32_t dev_id, int virq);
 int kvm_device_msi_deassign(KVMState *s, uint32_t dev_id);
 
+bool kvm_device_msix_supported(KVMState *s);
 int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id);
 
 #endif
-- 
1.7.3.4


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

* [PATCH 09/19] pci-assign: Replace kvm_assign_set_msix_nr with kvm_device_msix_init_vectors
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
                   ` (7 preceding siblings ...)
  2012-08-16 13:54 ` [PATCH 08/19] pci-assign: Factor out kvm_device_msix_supported Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 13:54 ` [PATCH 10/19] pci-assign: Replace kvm_assign_set_msix_entry with kvm_device_msix_set_vector Jan Kiszka
                   ` (11 subsequent siblings)
  20 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

The refactored version cleanly hides the KVM IOCTL structure from the
users and also zeros out the padding field.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/device-assignment.c |    7 ++-----
 qemu-kvm.c             |    5 -----
 qemu-kvm.h             |    1 -
 target-i386/kvm.c      |   12 ++++++++++++
 target-i386/kvm_i386.h |    2 ++
 5 files changed, 16 insertions(+), 11 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 80ac2fc..0e2f8e6 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -1045,7 +1045,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
     AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
     uint16_t entries_nr = 0;
     int i, r = 0;
-    struct kvm_assigned_msix_nr msix_nr;
     struct kvm_assigned_msix_entry msix_entry;
     MSIXTableEntry *entry = adev->msix_table;
 
@@ -1064,9 +1063,7 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
         return 0;
     }
 
-    msix_nr.assigned_dev_id = adev->dev_id;
-    msix_nr.entry_nr = entries_nr;
-    r = kvm_assign_set_msix_nr(kvm_state, &msix_nr);
+    r = kvm_device_msix_init_vectors(kvm_state, adev->dev_id, entries_nr);
     if (r != 0) {
         fprintf(stderr, "fail to set MSI-X entry number for MSIX! %s\n",
 			strerror(-r));
@@ -1078,7 +1075,7 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
     adev->irq_entries_nr = adev->msix_max;
     adev->entry = g_malloc0(adev->msix_max * sizeof(*(adev->entry)));
 
-    msix_entry.assigned_dev_id = msix_nr.assigned_dev_id;
+    msix_entry.assigned_dev_id = adev->dev_id;
     entry = adev->msix_table;
     for (i = 0; i < adev->msix_max; i++, entry++) {
         if (msix_masked(entry)) {
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 8416a8d..1a2a4fd 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -186,11 +186,6 @@ int kvm_get_irq_route_gsi(void)
 }
 
 #ifdef KVM_CAP_DEVICE_MSIX
-int kvm_assign_set_msix_nr(KVMState *s, struct kvm_assigned_msix_nr *msix_nr)
-{
-    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, msix_nr);
-}
-
 int kvm_assign_set_msix_entry(KVMState *s,
                               struct kvm_assigned_msix_entry *entry)
 {
diff --git a/qemu-kvm.h b/qemu-kvm.h
index c247ad0..3fd6046 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -66,7 +66,6 @@ int kvm_update_routing_entry(struct kvm_irq_routing_entry *entry,
                              struct kvm_irq_routing_entry *newentry);
 
 
-int kvm_assign_set_msix_nr(KVMState *s, struct kvm_assigned_msix_nr *msix_nr);
 int kvm_assign_set_msix_entry(KVMState *s,
                               struct kvm_assigned_msix_entry *entry);
 
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 677a791..676f45b 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2161,6 +2161,18 @@ bool kvm_device_msix_supported(KVMState *s)
     return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, NULL) == -EFAULT;
 }
 
+int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
+                                 uint32_t nr_vectors)
+{
+    struct kvm_assigned_msix_nr msix_nr = {
+        .assigned_dev_id = dev_id,
+        .entry_nr = nr_vectors,
+        .padding = 0,
+    };
+
+    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, &msix_nr);
+}
+
 int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id)
 {
     return kvm_deassign_irq_internal(s, dev_id, KVM_DEV_IRQ_GUEST_MSIX |
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index 6f66b6d..aac14eb 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -28,6 +28,8 @@ int kvm_device_msi_assign(KVMState *s, uint32_t dev_id, int virq);
 int kvm_device_msi_deassign(KVMState *s, uint32_t dev_id);
 
 bool kvm_device_msix_supported(KVMState *s);
+int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
+                                 uint32_t nr_vectors);
 int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id);
 
 #endif
-- 
1.7.3.4


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

* [PATCH 10/19] pci-assign: Replace kvm_assign_set_msix_entry with kvm_device_msix_set_vector
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
                   ` (8 preceding siblings ...)
  2012-08-16 13:54 ` [PATCH 09/19] pci-assign: Replace kvm_assign_set_msix_nr with kvm_device_msix_init_vectors Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 16:21   ` Alex Williamson
  2012-08-16 13:54 ` [PATCH 11/19] pci-assign: Rework MSI-X route setup Jan Kiszka
                   ` (10 subsequent siblings)
  20 siblings, 1 reply; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

The refactored version cleanly hides the KVM IOCTL structure from the
users and also zeros out the padding field.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/device-assignment.c |    7 ++-----
 qemu-kvm.c             |    8 --------
 qemu-kvm.h             |    4 ----
 target-i386/kvm.c      |   13 +++++++++++++
 target-i386/kvm_i386.h |    2 ++
 5 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 0e2f8e6..af8a5aa 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -1045,7 +1045,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
     AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
     uint16_t entries_nr = 0;
     int i, r = 0;
-    struct kvm_assigned_msix_entry msix_entry;
     MSIXTableEntry *entry = adev->msix_table;
 
     /* Get the usable entry number for allocating */
@@ -1075,7 +1074,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
     adev->irq_entries_nr = adev->msix_max;
     adev->entry = g_malloc0(adev->msix_max * sizeof(*(adev->entry)));
 
-    msix_entry.assigned_dev_id = adev->dev_id;
     entry = adev->msix_table;
     for (i = 0; i < adev->msix_max; i++, entry++) {
         if (msix_masked(entry)) {
@@ -1098,9 +1096,8 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
 
         kvm_add_routing_entry(kvm_state, &adev->entry[i]);
 
-        msix_entry.gsi = adev->entry[i].gsi;
-        msix_entry.entry = i;
-        r = kvm_assign_set_msix_entry(kvm_state, &msix_entry);
+        r = kvm_device_msix_set_vector(kvm_state, adev->dev_id, i,
+                                       adev->entry[i].gsi);
         if (r) {
             fprintf(stderr, "fail to set MSI-X entry! %s\n", strerror(-r));
             break;
diff --git a/qemu-kvm.c b/qemu-kvm.c
index 1a2a4fd..ec1911f 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -185,14 +185,6 @@ int kvm_get_irq_route_gsi(void)
 #endif
 }
 
-#ifdef KVM_CAP_DEVICE_MSIX
-int kvm_assign_set_msix_entry(KVMState *s,
-                              struct kvm_assigned_msix_entry *entry)
-{
-    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_ENTRY, entry);
-}
-#endif
-
 #if !defined(TARGET_I386)
 void kvm_arch_init_irq_routing(KVMState *s)
 {
diff --git a/qemu-kvm.h b/qemu-kvm.h
index 3fd6046..ad628d5 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -65,10 +65,6 @@ int kvm_del_routing_entry(struct kvm_irq_routing_entry *entry);
 int kvm_update_routing_entry(struct kvm_irq_routing_entry *entry,
                              struct kvm_irq_routing_entry *newentry);
 
-
-int kvm_assign_set_msix_entry(KVMState *s,
-                              struct kvm_assigned_msix_entry *entry);
-
 #endif /* CONFIG_KVM */
 
 #endif
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index 676f45b..e9353ed 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2173,6 +2173,19 @@ int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
     return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, &msix_nr);
 }
 
+int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
+                               int virq)
+{
+    struct kvm_assigned_msix_entry msix_entry = {
+        .assigned_dev_id = dev_id,
+        .gsi = virq,
+        .entry = vector,
+    };
+
+    memset(msix_entry.padding, 0, sizeof(msix_entry.padding));
+    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_ENTRY, &msix_entry);
+}
+
 int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id)
 {
     return kvm_deassign_irq_internal(s, dev_id, KVM_DEV_IRQ_GUEST_MSIX |
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index aac14eb..bd3b398 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -30,6 +30,8 @@ int kvm_device_msi_deassign(KVMState *s, uint32_t dev_id);
 bool kvm_device_msix_supported(KVMState *s);
 int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
                                  uint32_t nr_vectors);
+int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
+                               int virq);
 int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id);
 
 #endif
-- 
1.7.3.4


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

* [PATCH 11/19] pci-assign: Rework MSI-X route setup
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
                   ` (9 preceding siblings ...)
  2012-08-16 13:54 ` [PATCH 10/19] pci-assign: Replace kvm_assign_set_msix_entry with kvm_device_msix_set_vector Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 13:54 ` [PATCH 12/19] pci-assign: Factor out kvm_device_msix_assign Jan Kiszka
                   ` (9 subsequent siblings)
  20 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

Use kvm_irqchip_add_msi_route and introduce kvm_irqchip_update_msi_route
to set up the required IRQ routes for MSI-X injections. This removes the
last direct interaction with the IRQ routing API of the KVM kernel so
that we can remove/unexport related services.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/device-assignment.c |   71 ++++++++++-----------------
 kvm-all.c              |   51 ++++++++++++++++---
 kvm-stub.c             |    9 ---
 kvm.h                  |    5 +--
 qemu-kvm.c             |  128 ------------------------------------------------
 qemu-kvm.h             |   22 --------
 6 files changed, 71 insertions(+), 215 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index af8a5aa..7ffd26c 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -141,8 +141,6 @@ typedef struct AssignedDevice {
     uint8_t emulate_config_write[PCI_CONFIG_SPACE_SIZE];
     int msi_virq_nr;
     int *msi_virq;
-    int irq_entries_nr;
-    struct kvm_irq_routing_entry *entry;
     MSIXTableEntry *msix_table;
     target_phys_addr_t msix_table_addr;
     uint16_t msix_max;
@@ -701,7 +699,7 @@ again:
 
 static QLIST_HEAD(, AssignedDevice) devs = QLIST_HEAD_INITIALIZER(devs);
 
-static void free_dev_irq_entries(AssignedDevice *dev)
+static void free_msi_virqs(AssignedDevice *dev)
 {
     int i;
 
@@ -714,15 +712,6 @@ static void free_dev_irq_entries(AssignedDevice *dev)
     g_free(dev->msi_virq);
     dev->msi_virq = NULL;
     dev->msi_virq_nr = 0;
-
-    for (i = 0; i < dev->irq_entries_nr; i++) {
-        if (dev->entry[i].type) {
-            kvm_del_routing_entry(&dev->entry[i]);
-        }
-    }
-    g_free(dev->entry);
-    dev->entry = NULL;
-    dev->irq_entries_nr = 0;
 }
 
 static void free_assigned_device(AssignedDevice *dev)
@@ -778,7 +767,7 @@ static void free_assigned_device(AssignedDevice *dev)
         close(dev->real_device.config_fd);
     }
 
-    free_dev_irq_entries(dev);
+    free_msi_virqs(dev);
 }
 
 static void assign_failed_examine(AssignedDevice *dev)
@@ -1001,7 +990,7 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev)
         if (r && r != -ENXIO)
             perror("assigned_dev_update_msi: deassign irq");
 
-        free_dev_irq_entries(assigned_dev);
+        free_msi_virqs(assigned_dev);
 
         assigned_dev->assigned_irq_type = ASSIGNED_IRQ_NONE;
         pci_device_set_intx_routing_notifier(pci_dev, NULL);
@@ -1046,6 +1035,7 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
     uint16_t entries_nr = 0;
     int i, r = 0;
     MSIXTableEntry *entry = adev->msix_table;
+    MSIMessage msg;
 
     /* Get the usable entry number for allocating */
     for (i = 0; i < adev->msix_max; i++, entry++) {
@@ -1069,45 +1059,38 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
         return r;
     }
 
-    free_dev_irq_entries(adev);
+    free_msi_virqs(adev);
 
-    adev->irq_entries_nr = adev->msix_max;
-    adev->entry = g_malloc0(adev->msix_max * sizeof(*(adev->entry)));
+    adev->msi_virq_nr = adev->msix_max;
+    adev->msi_virq = g_malloc(adev->msix_max * sizeof(*adev->msi_virq));
 
     entry = adev->msix_table;
     for (i = 0; i < adev->msix_max; i++, entry++) {
+        adev->msi_virq[i] = -1;
+
         if (msix_masked(entry)) {
             continue;
         }
 
-        r = kvm_get_irq_route_gsi();
-        if (r < 0)
+        msg.address = entry->addr_lo | ((uint64_t)entry->addr_hi << 32);
+        msg.data = entry->data;
+        r = kvm_irqchip_add_msi_route(kvm_state, msg);
+        if (r < 0) {
             return r;
-
-        adev->entry[i].gsi = r;
-        adev->entry[i].type = KVM_IRQ_ROUTING_MSI;
-        adev->entry[i].flags = 0;
-        adev->entry[i].u.msi.address_lo = entry->addr_lo;
-        adev->entry[i].u.msi.address_hi = entry->addr_hi;
-        adev->entry[i].u.msi.data = entry->data;
+        }
+        adev->msi_virq[i] = r;
 
         DEBUG("MSI-X vector %d, gsi %d, addr %08x_%08x, data %08x\n", i,
               r, entry->addr_hi, entry->addr_lo, entry->data);
 
-        kvm_add_routing_entry(kvm_state, &adev->entry[i]);
-
         r = kvm_device_msix_set_vector(kvm_state, adev->dev_id, i,
-                                       adev->entry[i].gsi);
+                                       adev->msi_virq[i]);
         if (r) {
             fprintf(stderr, "fail to set MSI-X entry! %s\n", strerror(-r));
             break;
         }
     }
 
-    if (r == 0) {
-        kvm_irqchip_commit_routes(kvm_state);
-    }
-
     return r;
 }
 
@@ -1127,13 +1110,13 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev)
      * MSIX or intends to start. */
     if ((assigned_dev->assigned_irq_type == ASSIGNED_IRQ_MSIX) ||
         (ctrl_word & PCI_MSIX_FLAGS_ENABLE)) {
-
-        free_dev_irq_entries(assigned_dev);
         r = kvm_device_msix_deassign(kvm_state, assigned_dev->dev_id);
         /* -ENXIO means no assigned irq */
         if (r && r != -ENXIO)
             perror("assigned_dev_update_msix: deassign irq");
 
+        free_msi_virqs(assigned_dev);
+
         assigned_dev->assigned_irq_type = ASSIGNED_IRQ_NONE;
         pci_device_set_intx_routing_notifier(pci_dev, NULL);
     }
@@ -1147,7 +1130,7 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev)
             return;
         }
 
-        if (assigned_dev->irq_entries_nr) {
+        if (assigned_dev->msi_virq_nr > 0) {
             if (kvm_assign_irq(kvm_state, &assigned_irq_data) < 0) {
                 perror("assigned_dev_enable_msix: assign irq");
                 return;
@@ -1561,27 +1544,25 @@ static void msix_mmio_write(void *opaque, target_phys_addr_t addr,
              */
         } else if (msix_masked(&orig) && !msix_masked(entry)) {
             /* Vector unmasked */
-            if (i >= adev->irq_entries_nr || !adev->entry[i].type) {
+            if (i >= adev->msi_virq_nr || adev->msi_virq[i] < 0) {
                 /* Previously unassigned vector, start from scratch */
                 assigned_dev_update_msix(pdev);
                 return;
             } else {
                 /* Update an existing, previously masked vector */
-                struct kvm_irq_routing_entry orig = adev->entry[i];
+                MSIMessage msg;
                 int ret;
 
-                adev->entry[i].u.msi.address_lo = entry->addr_lo;
-                adev->entry[i].u.msi.address_hi = entry->addr_hi;
-                adev->entry[i].u.msi.data = entry->data;
+                msg.address = entry->addr_lo |
+                    ((uint64_t)entry->addr_hi << 32);
+                msg.data = entry->data;
 
-                ret = kvm_update_routing_entry(&orig, &adev->entry[i]);
+                ret = kvm_irqchip_update_msi_route(kvm_state,
+                                                   adev->msi_virq[i], msg);
                 if (ret) {
                     fprintf(stderr,
                             "Error updating irq routing entry (%d)\n", ret);
-                    return;
                 }
-
-                kvm_irqchip_commit_routes(kvm_state);
             }
         }
     }
diff --git a/kvm-all.c b/kvm-all.c
index 979c8d7..8ab47f1 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -922,7 +922,7 @@ static void kvm_init_irq_routing(KVMState *s)
     kvm_arch_init_irq_routing(s);
 }
 
-void kvm_irqchip_commit_routes(KVMState *s)
+static void kvm_irqchip_commit_routes(KVMState *s)
 {
     int ret;
 
@@ -931,7 +931,7 @@ void kvm_irqchip_commit_routes(KVMState *s)
     assert(ret == 0);
 }
 
-void kvm_add_routing_entry(KVMState *s,
+static void kvm_add_routing_entry(KVMState *s,
                                   struct kvm_irq_routing_entry *entry)
 {
     struct kvm_irq_routing_entry *new;
@@ -960,6 +960,30 @@ void kvm_add_routing_entry(KVMState *s,
     kvm_irqchip_commit_routes(s);
 }
 
+static int kvm_update_routing_entry(KVMState *s,
+                                    struct kvm_irq_routing_entry *new_entry)
+{
+    struct kvm_irq_routing_entry *entry;
+    int n;
+
+    for (n = 0; n < s->irq_routes->nr; n++) {
+        entry = &s->irq_routes->entries[n];
+        if (entry->gsi != new_entry->gsi) {
+            continue;
+        }
+
+        entry->type = new_entry->type;
+        entry->flags = new_entry->flags;
+        entry->u = new_entry->u;
+
+        kvm_irqchip_commit_routes(s);
+
+        return 0;
+    }
+
+    return -ESRCH;
+}
+
 void kvm_irqchip_add_irq_route(KVMState *s, int irq, int irqchip, int pin)
 {
     struct kvm_irq_routing_entry e;
@@ -1122,6 +1146,24 @@ int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg)
     return virq;
 }
 
+int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg)
+{
+    struct kvm_irq_routing_entry kroute;
+
+    if (!kvm_irqchip_in_kernel()) {
+        return -ENOSYS;
+    }
+
+    kroute.gsi = virq;
+    kroute.type = KVM_IRQ_ROUTING_MSI;
+    kroute.flags = 0;
+    kroute.u.msi.address_lo = (uint32_t)msg.address;
+    kroute.u.msi.address_hi = msg.address >> 32;
+    kroute.u.msi.data = msg.data;
+
+    return kvm_update_routing_entry(s, &kroute);
+}
+
 static int kvm_irqchip_assign_irqfd(KVMState *s, int fd, int virq, bool assign)
 {
     struct kvm_irqfd irqfd = {
@@ -1143,11 +1185,6 @@ static void kvm_init_irq_routing(KVMState *s)
 {
 }
 
-int kvm_irqchip_commit_routes(KVMState *s)
-{
-    return -ENOSYS;
-}
-
 void kvm_irqchip_release_virq(KVMState *s, int virq)
 {
 }
diff --git a/kvm-stub.c b/kvm-stub.c
index 6c8d980..94c9ea1 100644
--- a/kvm-stub.c
+++ b/kvm-stub.c
@@ -122,15 +122,6 @@ int kvm_set_ioeventfd_mmio(int fd, uint32_t adr, uint32_t val, bool assign, uint
     return -ENOSYS;
 }
 
-int kvm_get_irq_route_gsi(void)
-{
-    return -ENOSYS;
-}
-
-void kvm_irqchip_commit_routes(KVMState *s)
-{
-}
-
 int kvm_on_sigbus_vcpu(CPUArchState *env, int code, void *addr)
 {
     return 1;
diff --git a/kvm.h b/kvm.h
index cdfbb82..0c09be8 100644
--- a/kvm.h
+++ b/kvm.h
@@ -273,6 +273,7 @@ int kvm_set_ioeventfd_mmio(int fd, uint32_t adr, uint32_t val, bool assign,
 int kvm_set_ioeventfd_pio_word(int fd, uint16_t adr, uint16_t val, bool assign);
 
 int kvm_irqchip_add_msi_route(KVMState *s, MSIMessage msg);
+int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg);
 void kvm_irqchip_release_virq(KVMState *s, int virq);
 
 int kvm_irqchip_add_irqfd(KVMState *s, int fd, int virq);
@@ -280,10 +281,6 @@ int kvm_irqchip_remove_irqfd(KVMState *s, int fd, int virq);
 int kvm_irqchip_add_irq_notifier(KVMState *s, EventNotifier *n, int virq);
 int kvm_irqchip_remove_irq_notifier(KVMState *s, EventNotifier *n, int virq);
 
-int kvm_get_irq_route_gsi(void);
-
-void kvm_irqchip_commit_routes(KVMState *s);
-
 #ifdef NEED_CPU_H
 #include "qemu-kvm.h"
 #endif
diff --git a/qemu-kvm.c b/qemu-kvm.c
index ec1911f..e45e4a7 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -57,134 +57,6 @@ int kvm_assign_irq(KVMState *s, struct kvm_assigned_irq *assigned_irq)
 #endif
 #endif
 
-int kvm_del_routing_entry(struct kvm_irq_routing_entry *entry)
-{
-#ifdef KVM_CAP_IRQ_ROUTING
-    KVMState *s = kvm_state;
-    struct kvm_irq_routing_entry *e, *p;
-    int i, gsi, found = 0;
-
-    gsi = entry->gsi;
-
-    for (i = 0; i < s->irq_routes->nr; ++i) {
-        e = &s->irq_routes->entries[i];
-        if (e->type == entry->type && e->gsi == gsi) {
-            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) {
-                        p = &s->irq_routes->entries[--s->irq_routes->nr];
-                        *e = *p;
-                        found = 1;
-                    }
-                    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) {
-                        p = &s->irq_routes->entries[--s->irq_routes->nr];
-                        *e = *p;
-                        found = 1;
-                    }
-                    break;
-                }
-            default:
-                break;
-            }
-            if (found) {
-                /* If there are no other users of this GSI
-                 * mark it available in the bitmap */
-                for (i = 0; i < s->irq_routes->nr; i++) {
-                    e = &s->irq_routes->entries[i];
-                    if (e->gsi == gsi)
-                        break;
-                }
-                if (i == s->irq_routes->nr) {
-                    clear_gsi(s, gsi);
-                }
-
-                return 0;
-            }
-        }
-    }
-    return -ESRCH;
-#else
-    return -ENOSYS;
-#endif
-}
-
-int kvm_update_routing_entry(struct kvm_irq_routing_entry *entry,
-                             struct kvm_irq_routing_entry *newentry)
-{
-#ifdef KVM_CAP_IRQ_ROUTING
-    KVMState *s = kvm_state;
-    struct kvm_irq_routing_entry *e;
-    int i;
-
-    if (entry->gsi != newentry->gsi || entry->type != newentry->type) {
-        return -EINVAL;
-    }
-
-    for (i = 0; i < s->irq_routes->nr; ++i) {
-        e = &s->irq_routes->entries[i];
-        if (e->type != entry->type || e->gsi != entry->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) {
-                memcpy(&e->u.irqchip, &newentry->u.irqchip,
-                       sizeof e->u.irqchip);
-                return 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) {
-                memcpy(&e->u.msi, &newentry->u.msi, sizeof e->u.msi);
-                return 0;
-            }
-            break;
-        default:
-            break;
-        }
-    }
-    return -ESRCH;
-#else
-    return -ENOSYS;
-#endif
-}
-
-int kvm_get_irq_route_gsi(void)
-{
-#ifdef KVM_CAP_IRQ_ROUTING
-    KVMState *s = kvm_state;
-    int max_words = ALIGN(s->gsi_count, 32) / 32;
-    int i, bit;
-    uint32_t *buf = s->used_gsi_bitmap;
-
-    /* Return the lowest unused GSI in the bitmap */
-    for (i = 0; i < max_words; i++) {
-        bit = ffs(~buf[i]);
-        if (!bit) {
-            continue;
-        }
-
-        return bit - 1 + i * 32;
-    }
-
-    return -ENOSPC;
-#else
-    return -ENOSYS;
-#endif
-}
-
 #if !defined(TARGET_I386)
 void kvm_arch_init_irq_routing(KVMState *s)
 {
diff --git a/qemu-kvm.h b/qemu-kvm.h
index ad628d5..ae7a33c 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -43,28 +43,6 @@
  */
 int kvm_assign_irq(KVMState *s, struct kvm_assigned_irq *assigned_irq);
 
-struct kvm_irq_routing_entry;
-
-void kvm_add_routing_entry(KVMState *s, struct kvm_irq_routing_entry *entry);
-
-/*!
- * \brief Removes a routing from the temporary irq routing table
- *
- * Remove a routing to the temporary irq routing table.  Nothing is
- * committed to the running VM.
- */
-int kvm_del_routing_entry(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.
- */
-int kvm_update_routing_entry(struct kvm_irq_routing_entry *entry,
-                             struct kvm_irq_routing_entry *newentry);
-
 #endif /* CONFIG_KVM */
 
 #endif
-- 
1.7.3.4


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

* [PATCH 12/19] pci-assign: Factor out kvm_device_msix_assign
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
                   ` (10 preceding siblings ...)
  2012-08-16 13:54 ` [PATCH 11/19] pci-assign: Rework MSI-X route setup Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 13:54 ` [PATCH 13/19] qemu-kvm: Kill qemu-kvm.[ch] Jan Kiszka
                   ` (8 subsequent siblings)
  20 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

Avoid passing kvm_assigned_irq on MSI-X assignment. Drop kvm_assign_irq
as it's now no longer used.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/device-assignment.c |    9 +--------
 qemu-kvm.c             |   27 ---------------------------
 qemu-kvm.h             |   11 -----------
 target-i386/kvm.c      |    6 ++++++
 target-i386/kvm_i386.h |    1 +
 5 files changed, 8 insertions(+), 46 deletions(-)

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 7ffd26c..32a082d 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -1096,15 +1096,11 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
 
 static void assigned_dev_update_msix(PCIDevice *pci_dev)
 {
-    struct kvm_assigned_irq assigned_irq_data;
     AssignedDevice *assigned_dev = DO_UPCAST(AssignedDevice, dev, pci_dev);
     uint16_t ctrl_word = pci_get_word(pci_dev->config + pci_dev->msix_cap +
                                       PCI_MSIX_FLAGS);
     int r;
 
-    memset(&assigned_irq_data, 0, sizeof assigned_irq_data);
-    assigned_irq_data.assigned_dev_id = assigned_dev->dev_id;
-
     /* Some guests gratuitously disable MSIX even if they're not using it,
      * try to catch this by only deassigning irqs if the guest is using
      * MSIX or intends to start. */
@@ -1122,16 +1118,13 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev)
     }
 
     if (ctrl_word & PCI_MSIX_FLAGS_ENABLE) {
-        assigned_irq_data.flags = KVM_DEV_IRQ_HOST_MSIX |
-                                  KVM_DEV_IRQ_GUEST_MSIX;
-
         if (assigned_dev_update_msix_mmio(pci_dev) < 0) {
             perror("assigned_dev_update_msix_mmio");
             return;
         }
 
         if (assigned_dev->msi_virq_nr > 0) {
-            if (kvm_assign_irq(kvm_state, &assigned_irq_data) < 0) {
+            if (kvm_device_msix_assign(kvm_state, assigned_dev->dev_id) < 0) {
                 perror("assigned_dev_enable_msix: assign irq");
                 return;
             }
diff --git a/qemu-kvm.c b/qemu-kvm.c
index e45e4a7..3dc56ea 100644
--- a/qemu-kvm.c
+++ b/qemu-kvm.c
@@ -30,33 +30,6 @@
 
 #define ALIGN(x, y) (((x)+(y)-1) & ~((y)-1))
 
-#ifdef KVM_CAP_DEVICE_ASSIGNMENT
-static int kvm_old_assign_irq(KVMState *s,
-                              struct kvm_assigned_irq *assigned_irq)
-{
-    return kvm_vm_ioctl(s, KVM_ASSIGN_IRQ, assigned_irq);
-}
-
-#ifdef KVM_CAP_ASSIGN_DEV_IRQ
-int kvm_assign_irq(KVMState *s, struct kvm_assigned_irq *assigned_irq)
-{
-    int ret;
-
-    ret = kvm_ioctl(s, KVM_CHECK_EXTENSION, KVM_CAP_ASSIGN_DEV_IRQ);
-    if (ret > 0) {
-        return kvm_vm_ioctl(s, KVM_ASSIGN_DEV_IRQ, assigned_irq);
-    }
-
-    return kvm_old_assign_irq(s, assigned_irq);
-}
-#else
-int kvm_assign_irq(KVMState *s, struct kvm_assigned_irq *assigned_irq)
-{
-    return kvm_old_assign_irq(s, assigned_irq);
-}
-#endif
-#endif
-
 #if !defined(TARGET_I386)
 void kvm_arch_init_irq_routing(KVMState *s)
 {
diff --git a/qemu-kvm.h b/qemu-kvm.h
index ae7a33c..f7d9cd5 100644
--- a/qemu-kvm.h
+++ b/qemu-kvm.h
@@ -32,17 +32,6 @@
 
 #include "kvm.h"
 
-/*!
- * \brief Assign IRQ for an assigned device
- *
- * Used for PCI device assignment, this function assigns IRQ numbers for
- * an physical device and guest IRQ handling.
- *
- * \param kvm Pointer to the current kvm_context
- * \param assigned_irq Parameters, like dev id, host irq, guest irq, etc
- */
-int kvm_assign_irq(KVMState *s, struct kvm_assigned_irq *assigned_irq);
-
 #endif /* CONFIG_KVM */
 
 #endif
diff --git a/target-i386/kvm.c b/target-i386/kvm.c
index e9353ed..67635af 100644
--- a/target-i386/kvm.c
+++ b/target-i386/kvm.c
@@ -2186,6 +2186,12 @@ int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
     return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_ENTRY, &msix_entry);
 }
 
+int kvm_device_msix_assign(KVMState *s, uint32_t dev_id)
+{
+    return kvm_assign_irq_internal(s, dev_id, KVM_DEV_IRQ_HOST_MSIX |
+                                              KVM_DEV_IRQ_GUEST_MSIX, 0);
+}
+
 int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id)
 {
     return kvm_deassign_irq_internal(s, dev_id, KVM_DEV_IRQ_GUEST_MSIX |
diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
index bd3b398..f6ab82f 100644
--- a/target-i386/kvm_i386.h
+++ b/target-i386/kvm_i386.h
@@ -32,6 +32,7 @@ int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
                                  uint32_t nr_vectors);
 int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
                                int virq);
+int kvm_device_msix_assign(KVMState *s, uint32_t dev_id);
 int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id);
 
 #endif
-- 
1.7.3.4


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

* [PATCH 13/19] qemu-kvm: Kill qemu-kvm.[ch]
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
                   ` (11 preceding siblings ...)
  2012-08-16 13:54 ` [PATCH 12/19] pci-assign: Factor out kvm_device_msix_assign Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 13:54 ` [PATCH 14/19] pci-assign: Drop configure switches Jan Kiszka
                   ` (7 subsequent siblings)
  20 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

Hurray!

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/device-assignment.c |    1 -
 kvm-all.c              |    3 ---
 kvm.h                  |    7 -------
 qemu-kvm.c             |   37 -------------------------------------
 qemu-kvm.h             |   37 -------------------------------------
 5 files changed, 0 insertions(+), 85 deletions(-)
 delete mode 100644 qemu-kvm.c
 delete mode 100644 qemu-kvm.h

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 32a082d..5ef5629 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -31,7 +31,6 @@
 #include <sys/mman.h>
 #include <sys/types.h>
 #include <sys/stat.h>
-#include "qemu-kvm.h"
 #include "hw.h"
 #include "pc.h"
 #include "qemu-error.h"
diff --git a/kvm-all.c b/kvm-all.c
index 8ab47f1..badf1d8 100644
--- a/kvm-all.c
+++ b/kvm-all.c
@@ -2043,6 +2043,3 @@ int kvm_on_sigbus(int code, void *addr)
 {
     return kvm_arch_on_sigbus(code, addr);
 }
-
-#undef PAGE_SIZE
-#include "qemu-kvm.c"
diff --git a/kvm.h b/kvm.h
index 0c09be8..2a68a52 100644
--- a/kvm.h
+++ b/kvm.h
@@ -146,7 +146,6 @@ int kvm_set_signal_mask(CPUArchState *env, const sigset_t *sigset);
 
 int kvm_on_sigbus_vcpu(CPUArchState *env, int code, void *addr);
 int kvm_on_sigbus(int code, void *addr);
-#endif /* NEED_CPU_H */
 
 /* internal API */
 
@@ -154,7 +153,6 @@ int kvm_ioctl(KVMState *s, int type, ...);
 
 int kvm_vm_ioctl(KVMState *s, int type, ...);
 
-#ifdef NEED_CPU_H
 int kvm_vcpu_ioctl(CPUArchState *env, int type, ...);
 
 /* Arch specific hooks */
@@ -280,9 +278,4 @@ int kvm_irqchip_add_irqfd(KVMState *s, int fd, int virq);
 int kvm_irqchip_remove_irqfd(KVMState *s, int fd, int virq);
 int kvm_irqchip_add_irq_notifier(KVMState *s, EventNotifier *n, int virq);
 int kvm_irqchip_remove_irq_notifier(KVMState *s, EventNotifier *n, int virq);
-
-#ifdef NEED_CPU_H
-#include "qemu-kvm.h"
-#endif
-
 #endif
diff --git a/qemu-kvm.c b/qemu-kvm.c
deleted file mode 100644
index 3dc56ea..0000000
--- a/qemu-kvm.c
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
- * qemu/kvm integration
- *
- * Copyright (C) 2006-2008 Qumranet Technologies
- *
- * Licensed under the terms of the GNU GPL version 2 or higher.
- */
-#include "config.h"
-#include "config-host.h"
-
-#include <assert.h>
-#include <string.h>
-#include "hw/hw.h"
-#include "sysemu.h"
-#include "qemu-common.h"
-#include "console.h"
-#include "block.h"
-#include "compatfd.h"
-#include "gdbstub.h"
-#include "monitor.h"
-#include "cpus.h"
-
-#include "qemu-kvm.h"
-
-#define EXPECTED_KVM_API_VERSION 12
-
-#if EXPECTED_KVM_API_VERSION != KVM_API_VERSION
-#error libkvm: userspace and kernel version mismatch
-#endif
-
-#define ALIGN(x, y) (((x)+(y)-1) & ~((y)-1))
-
-#if !defined(TARGET_I386)
-void kvm_arch_init_irq_routing(KVMState *s)
-{
-}
-#endif
diff --git a/qemu-kvm.h b/qemu-kvm.h
deleted file mode 100644
index f7d9cd5..0000000
--- a/qemu-kvm.h
+++ /dev/null
@@ -1,37 +0,0 @@
-/*
- * qemu/kvm integration
- *
- * Copyright (C) 2006-2008 Qumranet Technologies
- *
- * Licensed under the terms of the GNU GPL version 2 or higher.
- */
-#ifndef THE_ORIGINAL_AND_TRUE_QEMU_KVM_H
-#define THE_ORIGINAL_AND_TRUE_QEMU_KVM_H
-
-#include "cpu.h"
-
-#include <signal.h>
-#include <stdlib.h>
-
-#ifdef CONFIG_KVM
-
-#include <stdint.h>
-
-#ifndef __user
-#define __user       /* temporary, until installed via make headers_install */
-#endif
-
-#include <linux/kvm.h>
-
-#include <signal.h>
-
-/* FIXME: share this number with kvm */
-/* FIXME: or dynamically alloc/realloc regions */
-#define KVM_MAX_NUM_MEM_REGIONS 32u
-#define MAX_VCPUS 16
-
-#include "kvm.h"
-
-#endif /* CONFIG_KVM */
-
-#endif
-- 
1.7.3.4


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

* [PATCH 14/19] pci-assign: Drop configure switches
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
                   ` (12 preceding siblings ...)
  2012-08-16 13:54 ` [PATCH 13/19] qemu-kvm: Kill qemu-kvm.[ch] Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 13:54 ` [PATCH 15/19] pci-assign: Move and rename source file Jan Kiszka
                   ` (6 subsequent siblings)
  20 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

There are no other dependencies of device assignment except for
CONFIG_KVM and an x86 target. So there is also no point in controlling
this feature via configure.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 configure             |   11 -----------
 hw/i386/Makefile.objs |    2 +-
 2 files changed, 1 insertions(+), 12 deletions(-)

diff --git a/configure b/configure
index 5c3ad51..87424ce 100755
--- a/configure
+++ b/configure
@@ -211,7 +211,6 @@ bsd_user="no"
 guest_base=""
 uname_release=""
 mixemu="no"
-kvm_cap_device_assignment="yes"
 aix="no"
 blobs="yes"
 pkgversion=" ($(kvm_version))"
@@ -729,10 +728,6 @@ for opt do
   ;;
   --enable-tcg-interpreter) tcg_interpreter="yes"
   ;;
-  --disable-kvm-device-assignment) kvm_cap_device_assignment="no"
-  ;;
-  --enable-kvm-device-assignment) kvm_cap_device_assignment="yes"
-  ;;
   --disable-cap-ng)  cap_ng="no"
   ;;
   --enable-cap-ng) cap_ng="yes"
@@ -1091,8 +1086,6 @@ echo "  --disable-slirp          disable SLIRP userspace network connectivity"
 echo "  --disable-kvm            disable KVM acceleration support"
 echo "  --enable-kvm             enable KVM acceleration support"
 echo "  --enable-tcg-interpreter enable TCG with bytecode interpreter (TCI)"
-echo "  --disable-kvm-device-assignment  disable KVM device assignment support"
-echo "  --enable-kvm-device-assignment   enable KVM device assignment support"
 echo "  --disable-nptl           disable usermode NPTL support"
 echo "  --enable-nptl            enable usermode NPTL support"
 echo "  --enable-system          enable all system emulation targets"
@@ -3118,7 +3111,6 @@ echo "ATTR/XATTR support $attr"
 echo "Install blobs     $blobs"
 echo "KVM support       $kvm"
 echo "TCG interpreter   $tcg_interpreter"
-echo "KVM device assig. $kvm_cap_device_assignment"
 echo "fdt support       $fdt"
 echo "preadv support    $preadv"
 echo "fdatasync         $fdatasync"
@@ -3845,9 +3837,6 @@ case "$target_arch2" in
       if test "$vhost_net" = "yes" ; then
         echo "CONFIG_VHOST_NET=y" >> $config_target_mak
       fi
-      if test $kvm_cap_device_assignment = "yes" ; then
-        echo "CONFIG_KVM_DEVICE_ASSIGNMENT=y" >> $config_target_mak
-      fi
     fi
 esac
 case "$target_arch2" in
diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index ad52387..29f3e6f 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -14,7 +14,7 @@ obj-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
 obj-y += testdev.o
 obj-y += acpi.o acpi_piix4.o
 
-obj-$(CONFIG_KVM_DEVICE_ASSIGNMENT) += device-assignment.o
+obj-$(CONFIG_KVM) += device-assignment.o
 
 
 obj-y := $(addprefix ../,$(obj-y))
-- 
1.7.3.4


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

* [PATCH 15/19] pci-assign: Move and rename source file
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
                   ` (13 preceding siblings ...)
  2012-08-16 13:54 ` [PATCH 14/19] pci-assign: Drop configure switches Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 13:54 ` [PATCH 16/19] pci-assign: Fix coding style issues Jan Kiszka
                   ` (5 subsequent siblings)
  20 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

Move device-assignment.c into hw/kvm, calling it pci-assign.c now, just
like its device name.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/i386/Makefile.objs                        |    3 ---
 hw/kvm/Makefile.objs                         |    2 +-
 hw/{device-assignment.c => kvm/pci-assign.c} |   10 +++++-----
 3 files changed, 6 insertions(+), 9 deletions(-)
 rename hw/{device-assignment.c => kvm/pci-assign.c} (99%)

diff --git a/hw/i386/Makefile.objs b/hw/i386/Makefile.objs
index 29f3e6f..523e224 100644
--- a/hw/i386/Makefile.objs
+++ b/hw/i386/Makefile.objs
@@ -14,7 +14,4 @@ obj-$(CONFIG_SPICE) += qxl.o qxl-logger.o qxl-render.o
 obj-y += testdev.o
 obj-y += acpi.o acpi_piix4.o
 
-obj-$(CONFIG_KVM) += device-assignment.o
-
-
 obj-y := $(addprefix ../,$(obj-y))
diff --git a/hw/kvm/Makefile.objs b/hw/kvm/Makefile.objs
index 226497a..f620d7f 100644
--- a/hw/kvm/Makefile.objs
+++ b/hw/kvm/Makefile.objs
@@ -1 +1 @@
-obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o
+obj-$(CONFIG_KVM) += clock.o apic.o i8259.o ioapic.o i8254.o pci-assign.o
diff --git a/hw/device-assignment.c b/hw/kvm/pci-assign.c
similarity index 99%
rename from hw/device-assignment.c
rename to hw/kvm/pci-assign.c
index 5ef5629..3611539 100644
--- a/hw/device-assignment.c
+++ b/hw/kvm/pci-assign.c
@@ -31,16 +31,16 @@
 #include <sys/mman.h>
 #include <sys/types.h>
 #include <sys/stat.h>
-#include "hw.h"
-#include "pc.h"
+#include "hw/hw.h"
+#include "hw/pc.h"
 #include "qemu-error.h"
 #include "console.h"
-#include "loader.h"
+#include "hw/loader.h"
 #include "monitor.h"
 #include "range.h"
 #include "sysemu.h"
-#include "pci.h"
-#include "msi.h"
+#include "hw/pci.h"
+#include "hw/msi.h"
 #include "kvm_i386.h"
 
 #define MSIX_PAGE_SIZE 0x1000
-- 
1.7.3.4


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

* [PATCH 16/19] pci-assign: Fix coding style issues
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
                   ` (14 preceding siblings ...)
  2012-08-16 13:54 ` [PATCH 15/19] pci-assign: Move and rename source file Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 13:54 ` [PATCH 17/19] pci-assign: Replace exit() with hw_error() Jan Kiszka
                   ` (4 subsequent siblings)
  20 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

No functional changes.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/kvm/pci-assign.c |  181 +++++++++++++++++++++++++++++---------------------
 1 files changed, 105 insertions(+), 76 deletions(-)

diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c
index 3611539..cfd859e 100644
--- a/hw/kvm/pci-assign.c
+++ b/hw/kvm/pci-assign.c
@@ -57,10 +57,10 @@
 #ifdef DEVICE_ASSIGNMENT_DEBUG
 #define DEBUG(fmt, ...)                                       \
     do {                                                      \
-      fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__);    \
+        fprintf(stderr, "%s: " fmt, __func__ , __VA_ARGS__);  \
     } while (0)
 #else
-#define DEBUG(fmt, ...) do { } while(0)
+#define DEBUG(fmt, ...)
 #endif
 
 typedef struct {
@@ -186,27 +186,27 @@ static uint64_t assigned_dev_ioport_rw(AssignedDevRegion *dev_region,
             DEBUG("out data=%lx, size=%d, e_phys=%lx, host=%x\n",
                   *data, size, addr, port);
             switch (size) {
-                case 1:
-                    outb(*data, port);
-                    break;
-                case 2:
-                    outw(*data, port);
-                    break;
-                case 4:
-                    outl(*data, port);
-                    break;
+            case 1:
+                outb(*data, port);
+                break;
+            case 2:
+                outw(*data, port);
+                break;
+            case 4:
+                outl(*data, port);
+                break;
             }
         } else {
             switch (size) {
-                case 1:
-                    val = inb(port);
-                    break;
-                case 2:
-                    val = inw(port);
-                    break;
-                case 4:
-                    val = inl(port);
-                    break;
+            case 1:
+                val = inb(port);
+                break;
+            case 2:
+                val = inw(port);
+                break;
+            case 4:
+                val = inl(port);
+                break;
             }
             DEBUG("in data=%lx, size=%d, e_phys=%lx, host=%x\n",
                   val, size, addr, port);
@@ -354,13 +354,14 @@ static uint32_t assigned_dev_pci_read(PCIDevice *d, int pos, int len)
 again:
     ret = pread(fd, &val, len, pos);
     if (ret != len) {
-	if ((ret < 0) && (errno == EINTR || errno == EAGAIN))
-	    goto again;
+        if ((ret < 0) && (errno == EINTR || errno == EAGAIN)) {
+            goto again;
+        }
 
-	fprintf(stderr, "%s: pread failed, ret = %zd errno = %d\n",
-		__func__, ret, errno);
+        fprintf(stderr, "%s: pread failed, ret = %zd errno = %d\n",
+                __func__, ret, errno);
 
-	exit(1);
+        exit(1);
     }
 
     return val;
@@ -380,16 +381,15 @@ static void assigned_dev_pci_write(PCIDevice *d, int pos, uint32_t val, int len)
 again:
     ret = pwrite(fd, &val, len, pos);
     if (ret != len) {
-	if ((ret < 0) && (errno == EINTR || errno == EAGAIN))
-	    goto again;
+        if ((ret < 0) && (errno == EINTR || errno == EAGAIN)) {
+            goto again;
+        }
 
-	fprintf(stderr, "%s: pwrite failed, ret = %zd errno = %d\n",
-		__func__, ret, errno);
+        fprintf(stderr, "%s: pwrite failed, ret = %zd errno = %d\n",
+                __func__, ret, errno);
 
-	exit(1);
+        exit(1);
     }
-
-    return;
 }
 
 static void assigned_dev_emulate_config_read(AssignedDevice *dev,
@@ -418,21 +418,25 @@ static uint8_t pci_find_cap_offset(PCIDevice *d, uint8_t cap, uint8_t start)
     int status;
 
     status = assigned_dev_pci_read_byte(d, PCI_STATUS);
-    if ((status & PCI_STATUS_CAP_LIST) == 0)
+    if ((status & PCI_STATUS_CAP_LIST) == 0) {
         return 0;
+    }
 
     while (max_cap--) {
         pos = assigned_dev_pci_read_byte(d, pos);
-        if (pos < 0x40)
+        if (pos < 0x40) {
             break;
+        }
 
         pos &= ~3;
         id = assigned_dev_pci_read_byte(d, pos + PCI_CAP_LIST_ID);
 
-        if (id == 0xff)
+        if (id == 0xff) {
             break;
-        if (id == cap)
+        }
+        if (id == cap) {
             return pos;
+        }
 
         pos += PCI_CAP_LIST_NEXT;
     }
@@ -447,8 +451,9 @@ static int assigned_dev_register_regions(PCIRegion *io_regions,
     PCIRegion *cur_region = io_regions;
 
     for (i = 0; i < regions_num; i++, cur_region++) {
-        if (!cur_region->valid)
+        if (!cur_region->valid) {
             continue;
+        }
 
         /* handle memory io regions */
         if (cur_region->type & IORESOURCE_MEM) {
@@ -587,7 +592,7 @@ static int get_real_device(AssignedDevice *pci_dev, uint16_t r_seg,
     dev->region_number = 0;
 
     snprintf(dir, sizeof(dir), "/sys/bus/pci/devices/%04x:%02x:%02x.%x/",
-	     r_seg, r_bus, r_dev, r_func);
+             r_seg, r_bus, r_dev, r_func);
 
     snprintf(name, sizeof(name), "%sconfig", dir);
 
@@ -614,8 +619,9 @@ again:
     r = read(dev->config_fd, pci_dev->dev.config,
              pci_config_size(&pci_dev->dev));
     if (r < 0) {
-        if (errno == EINTR || errno == EAGAIN)
+        if (errno == EINTR || errno == EAGAIN) {
             goto again;
+        }
         fprintf(stderr, "%s: read failed, errno = %d\n", __func__, errno);
     }
 
@@ -641,16 +647,18 @@ again:
     }
 
     for (r = 0; r < PCI_ROM_SLOT; r++) {
-	if (fscanf(f, "%lli %lli %lli\n", &start, &end, &flags) != 3)
-	    break;
+        if (fscanf(f, "%lli %lli %lli\n", &start, &end, &flags) != 3) {
+            break;
+        }
 
         rp = dev->regions + r;
         rp->valid = 0;
         rp->resource_fd = -1;
         size = end - start + 1;
         flags &= IORESOURCE_IO | IORESOURCE_MEM | IORESOURCE_PREFETCH;
-        if (size == 0 || (flags & ~IORESOURCE_PREFETCH) == 0)
+        if (size == 0 || (flags & ~IORESOURCE_PREFETCH) == 0) {
             continue;
+        }
         if (flags & IORESOURCE_MEM) {
             flags &= ~IORESOURCE_IO;
         } else {
@@ -658,8 +666,9 @@ again:
         }
         snprintf(name, sizeof(name), "%sresource%d", dir, r);
         fd = open(name, O_RDWR);
-        if (fd == -1)
+        if (fd == -1) {
             continue;
+        }
         rp->resource_fd = fd;
 
         rp->type = flags;
@@ -782,7 +791,12 @@ static void assign_failed_examine(AssignedDevice *dev)
     sprintf(name, "%sdriver", dir);
 
     r = readlink(name, driver, sizeof(driver));
-    if ((r <= 0) || r >= sizeof(driver) || !(ns = strrchr(driver, '/'))) {
+    if ((r <= 0) || r >= sizeof(driver)) {
+        goto fail;
+    }
+
+    ns = strrchr(driver, '/');
+    if (!ns) {
         goto fail;
     }
 
@@ -850,11 +864,11 @@ static int assign_device(AssignedDevice *dev)
                 dev->dev.qdev.id, strerror(-r));
 
         switch (r) {
-            case -EBUSY:
-                assign_failed_examine(dev);
-                break;
-            default:
-                break;
+        case -EBUSY:
+            assign_failed_examine(dev);
+            break;
+        default:
+            break;
         }
     }
     return r;
@@ -951,9 +965,10 @@ static void deassign_device(AssignedDevice *dev)
     int r;
 
     r = kvm_device_pci_deassign(kvm_state, dev->dev_id);
-    if (r < 0)
-	fprintf(stderr, "Failed to deassign device \"%s\" : %s\n",
+    if (r < 0) {
+        fprintf(stderr, "Failed to deassign device \"%s\" : %s\n",
                 dev->dev.qdev.id, strerror(-r));
+    }
 }
 
 /* The pci config space got updated. Check if irq numbers have changed
@@ -986,8 +1001,9 @@ static void assigned_dev_update_msi(PCIDevice *pci_dev)
         (ctrl_byte & PCI_MSI_FLAGS_ENABLE)) {
         r = kvm_device_msi_deassign(kvm_state, assigned_dev->dev_id);
         /* -ENXIO means no assigned irq */
-        if (r && r != -ENXIO)
+        if (r && r != -ENXIO) {
             perror("assigned_dev_update_msi: deassign irq");
+        }
 
         free_msi_virqs(assigned_dev);
 
@@ -1054,7 +1070,7 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
     r = kvm_device_msix_init_vectors(kvm_state, adev->dev_id, entries_nr);
     if (r != 0) {
         fprintf(stderr, "fail to set MSI-X entry number for MSIX! %s\n",
-			strerror(-r));
+                strerror(-r));
         return r;
     }
 
@@ -1107,8 +1123,9 @@ static void assigned_dev_update_msix(PCIDevice *pci_dev)
         (ctrl_word & PCI_MSIX_FLAGS_ENABLE)) {
         r = kvm_device_msix_deassign(kvm_state, assigned_dev->dev_id);
         /* -ENXIO means no assigned irq */
-        if (r && r != -ENXIO)
+        if (r && r != -ENXIO) {
             perror("assigned_dev_update_msix: deassign irq");
+        }
 
         free_msi_virqs(assigned_dev);
 
@@ -1233,7 +1250,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
     if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_ASSIGN_DEV_IRQ)) {
         dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI;
         /* Only 32-bit/no-mask currently supported */
-        if ((ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSI, pos, 10)) < 0) {
+        ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSI, pos, 10);
+        if (ret < 0) {
             return ret;
         }
         pci_dev->msi_cap = pos;
@@ -1257,7 +1275,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
         uint32_t msix_table_entry;
 
         dev->cap.available |= ASSIGNED_DEVICE_CAP_MSIX;
-        if ((ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSIX, pos, 12)) < 0) {
+        ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSIX, pos, 12);
+        if (ret < 0) {
             return ret;
         }
         pci_dev->msix_cap = pos;
@@ -1280,10 +1299,12 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
     }
 
     /* Minimal PM support, nothing writable, device appears to NAK changes */
-    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_PM, 0))) {
+    pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_PM, 0);
+    if (pos) {
         uint16_t pmc;
-        if ((ret = pci_add_capability(pci_dev, PCI_CAP_ID_PM, pos,
-                                      PCI_PM_SIZEOF)) < 0) {
+
+        ret = pci_add_capability(pci_dev, PCI_CAP_ID_PM, pos, PCI_PM_SIZEOF);
+        if (ret < 0) {
             return ret;
         }
 
@@ -1302,7 +1323,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
         pci_set_byte(pci_dev->config + pos + PCI_PM_DATA_REGISTER, 0);
     }
 
-    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_EXP, 0))) {
+    pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_EXP, 0);
+    if (pos) {
         uint8_t version, size = 0;
         uint16_t type, devctl, lnksta;
         uint32_t devcap, lnkcap;
@@ -1321,13 +1343,13 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
             size = MIN(0x3c, PCI_CONFIG_SPACE_SIZE - pos);
             if (size < 0x34) {
                 fprintf(stderr,
-                        "%s: Invalid size PCIe cap-id 0x%x \n",
+                        "%s: Invalid size PCIe cap-id 0x%x\n",
                         __func__, PCI_CAP_ID_EXP);
                 return -EINVAL;
             } else if (size != 0x3c) {
                 fprintf(stderr,
                         "WARNING, %s: PCIe cap-id 0x%x has "
-                        "non-standard size 0x%x; std size should be 0x3c \n",
+                        "non-standard size 0x%x; std size should be 0x3c\n",
                          __func__, PCI_CAP_ID_EXP, size);
             }
         } else if (version == 0) {
@@ -1350,8 +1372,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
             return -EINVAL;
         }
 
-        if ((ret = pci_add_capability(pci_dev, PCI_CAP_ID_EXP,
-                                      pos, size)) < 0) {
+        ret = pci_add_capability(pci_dev, PCI_CAP_ID_EXP, pos, size);
+        if (ret < 0) {
             return ret;
         }
 
@@ -1419,12 +1441,14 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
         }
     }
 
-    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_PCIX, 0))) {
+    pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_PCIX, 0);
+    if (pos) {
         uint16_t cmd;
         uint32_t status;
 
         /* Only expose the minimum, 8 byte capability */
-        if ((ret = pci_add_capability(pci_dev, PCI_CAP_ID_PCIX, pos, 8)) < 0) {
+        ret = pci_add_capability(pci_dev, PCI_CAP_ID_PCIX, pos, 8);
+        if (ret < 0) {
             return ret;
         }
 
@@ -1446,9 +1470,11 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
         pci_set_long(pci_dev->config + pos + PCI_X_STATUS, status);
     }
 
-    if ((pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_VPD, 0))) {
+    pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_VPD, 0);
+    if (pos) {
         /* Direct R/W passthrough */
-        if ((ret = pci_add_capability(pci_dev, PCI_CAP_ID_VPD, pos, 8)) < 0) {
+        ret = pci_add_capability(pci_dev, PCI_CAP_ID_VPD, pos, 8);
+        if (ret < 0) {
             return ret;
         }
 
@@ -1463,8 +1489,8 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
         pos += PCI_CAP_LIST_NEXT) {
         uint8_t len = pci_get_byte(pci_dev->config + pos + PCI_CAP_FLAGS);
         /* Direct R/W passthrough */
-        if ((ret = pci_add_capability(pci_dev, PCI_CAP_ID_VNDR,
-                                      pos, len)) < 0) {
+        ret = pci_add_capability(pci_dev, PCI_CAP_ID_VNDR, pos, len);
+        if (ret < 0) {
             return ret;
         }
 
@@ -1740,8 +1766,9 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
     /* handle real device's MMIO/PIO BARs */
     if (assigned_dev_register_regions(dev->real_device.regions,
                                       dev->real_device.region_number,
-                                      dev))
+                                      dev)) {
         goto out;
+    }
 
     /* handle interrupt routing */
     e_intx = dev->dev.config[0x3d] - 1;
@@ -1752,13 +1779,15 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
 
     /* assign device to guest */
     r = assign_device(dev);
-    if (r < 0)
+    if (r < 0) {
         goto out;
+    }
 
     /* assign legacy INTx to the device */
     r = assign_intx(dev);
-    if (r < 0)
+    if (r < 0) {
         goto assigned_out;
+    }
 
     assigned_dev_load_option_rom(dev);
     QLIST_INSERT_HEAD(&devs, dev, next);
@@ -1783,8 +1812,7 @@ static void assigned_exitfn(struct PCIDevice *pci_dev)
     free_assigned_device(dev);
 }
 
-static Property da_properties[] =
-{
+static Property da_properties[] = {
     DEFINE_PROP_PCI_HOST_DEVADDR("host", AssignedDevice, host),
     DEFINE_PROP_BIT("prefer_msi", AssignedDevice, features,
                     ASSIGNED_DEVICE_PREFER_MSI_BIT, false),
@@ -1837,8 +1865,9 @@ static void assigned_dev_load_option_rom(AssignedDevice *dev)
     void *ptr;
 
     /* If loading ROM from file, pci handles it */
-    if (dev->dev.romfile || !dev->dev.rom_bar)
+    if (dev->dev.romfile || !dev->dev.rom_bar) {
         return;
+    }
 
     snprintf(rom_file, sizeof(rom_file),
              "/sys/bus/pci/devices/%04x:%02x:%02x.%01x/rom",
-- 
1.7.3.4


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

* [PATCH 17/19] pci-assign: Replace exit() with hw_error()
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
                   ` (15 preceding siblings ...)
  2012-08-16 13:54 ` [PATCH 16/19] pci-assign: Fix coding style issues Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 13:54 ` [PATCH 18/19] pci-assign: Drop unused or write-only variables Jan Kiszka
                   ` (3 subsequent siblings)
  20 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

This is more appropriate and allows central handling.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/kvm/pci-assign.c |   10 ++--------
 1 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c
index cfd859e..29a4671 100644
--- a/hw/kvm/pci-assign.c
+++ b/hw/kvm/pci-assign.c
@@ -358,10 +358,7 @@ again:
             goto again;
         }
 
-        fprintf(stderr, "%s: pread failed, ret = %zd errno = %d\n",
-                __func__, ret, errno);
-
-        exit(1);
+        hw_error("pci read failed, ret = %zd errno = %d\n", ret, errno);
     }
 
     return val;
@@ -385,10 +382,7 @@ again:
             goto again;
         }
 
-        fprintf(stderr, "%s: pwrite failed, ret = %zd errno = %d\n",
-                __func__, ret, errno);
-
-        exit(1);
+        hw_error("pci write failed, ret = %zd errno = %d\n", ret, errno);
     }
 }
 
-- 
1.7.3.4


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

* [PATCH 18/19] pci-assign: Drop unused or write-only variables
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
                   ` (16 preceding siblings ...)
  2012-08-16 13:54 ` [PATCH 17/19] pci-assign: Replace exit() with hw_error() Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 13:54 ` [PATCH 19/19] pci-assign: Gracefully handle missing in-kernel irqchip support Jan Kiszka
                   ` (2 subsequent siblings)
  20 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

Remove remainders of previous refactorings.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/kvm/pci-assign.c |    9 ---------
 1 files changed, 0 insertions(+), 9 deletions(-)

diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c
index 29a4671..4f5daf3 100644
--- a/hw/kvm/pci-assign.c
+++ b/hw/kvm/pci-assign.c
@@ -120,13 +120,10 @@ typedef struct AssignedDevice {
     uint32_t dev_id;
     uint32_t features;
     int intpin;
-    uint8_t debug_flags;
     AssignedDevRegion v_addrs[PCI_NUM_REGIONS - 1];
     PCIDevRegions real_device;
-    int run;
     PCIINTxRoute intx_route;
     AssignedIRQType assigned_irq_type;
-    int bound;
     struct {
 #define ASSIGNED_DEVICE_CAP_MSI (1 << 0)
 #define ASSIGNED_DEVICE_CAP_MSIX (1 << 1)
@@ -146,7 +143,6 @@ typedef struct AssignedDevice {
     MemoryRegion mmio;
     char *configfd_name;
     int32_t bootindex;
-    QLIST_ENTRY(AssignedDevice) next;
 } AssignedDevice;
 
 static void assigned_dev_update_irq_routing(PCIDevice *dev);
@@ -699,8 +695,6 @@ again:
     return 0;
 }
 
-static QLIST_HEAD(, AssignedDevice) devs = QLIST_HEAD_INITIALIZER(devs);
-
 static void free_msi_virqs(AssignedDevice *dev)
 {
     int i;
@@ -1767,7 +1761,6 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
     /* handle interrupt routing */
     e_intx = dev->dev.config[0x3d] - 1;
     dev->intpin = e_intx;
-    dev->run = 0;
     dev->intx_route.mode = PCI_INTX_DISABLED;
     dev->intx_route.irq = -1;
 
@@ -1784,7 +1777,6 @@ static int assigned_initfn(struct PCIDevice *pci_dev)
     }
 
     assigned_dev_load_option_rom(dev);
-    QLIST_INSERT_HEAD(&devs, dev, next);
 
     add_boot_device_path(dev->bootindex, &pci_dev->qdev, NULL);
 
@@ -1801,7 +1793,6 @@ static void assigned_exitfn(struct PCIDevice *pci_dev)
 {
     AssignedDevice *dev = DO_UPCAST(AssignedDevice, dev, pci_dev);
 
-    QLIST_REMOVE(dev, next);
     deassign_device(dev);
     free_assigned_device(dev);
 }
-- 
1.7.3.4


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

* [PATCH 19/19] pci-assign: Gracefully handle missing in-kernel irqchip support
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
                   ` (17 preceding siblings ...)
  2012-08-16 13:54 ` [PATCH 18/19] pci-assign: Drop unused or write-only variables Jan Kiszka
@ 2012-08-16 13:54 ` Jan Kiszka
  2012-08-16 14:34 ` [PATCH 00/19] pci-assign: Refactor for upstream merge Avi Kivity
  2012-08-16 16:33 ` Alex Williamson
  20 siblings, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 13:54 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Alex Williamson

Inform the user properly when trying to register an IRQ-using device
without in-kernel irqchip enabled.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
---
 hw/kvm/pci-assign.c |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)

diff --git a/hw/kvm/pci-assign.c b/hw/kvm/pci-assign.c
index 4f5daf3..9cce02c 100644
--- a/hw/kvm/pci-assign.c
+++ b/hw/kvm/pci-assign.c
@@ -862,6 +862,16 @@ static int assign_device(AssignedDevice *dev)
     return r;
 }
 
+static bool check_irqchip_in_kernel(void)
+{
+    if (kvm_irqchip_in_kernel()) {
+        return true;
+    }
+    error_report("pci-assign: error: requires KVM with in-kernel irqchip "
+                 "enabled");
+    return false;
+}
+
 static int assign_intx(AssignedDevice *dev)
 {
     AssignedIRQType new_type;
@@ -875,6 +885,10 @@ static int assign_intx(AssignedDevice *dev)
         return 0;
     }
 
+    if (!check_irqchip_in_kernel()) {
+        return -ENOTSUP;
+    }
+
     pci_device_set_intx_routing_notifier(&dev->dev,
                                          assigned_dev_update_irq_routing);
 
@@ -1236,6 +1250,9 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
      * MSI capability is the 1st capability in capability config */
     pos = pci_find_cap_offset(pci_dev, PCI_CAP_ID_MSI, 0);
     if (pos != 0 && kvm_check_extension(kvm_state, KVM_CAP_ASSIGN_DEV_IRQ)) {
+        if (!check_irqchip_in_kernel()) {
+            return -ENOTSUP;
+        }
         dev->cap.available |= ASSIGNED_DEVICE_CAP_MSI;
         /* Only 32-bit/no-mask currently supported */
         ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSI, pos, 10);
@@ -1262,6 +1279,9 @@ static int assigned_device_pci_cap_init(PCIDevice *pci_dev)
         int bar_nr;
         uint32_t msix_table_entry;
 
+        if (!check_irqchip_in_kernel()) {
+            return -ENOTSUP;
+        }
         dev->cap.available |= ASSIGNED_DEVICE_CAP_MSIX;
         ret = pci_add_capability(pci_dev, PCI_CAP_ID_MSIX, pos, 12);
         if (ret < 0) {
-- 
1.7.3.4


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

* Re: [PATCH 00/19] pci-assign: Refactor for upstream merge
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
                   ` (18 preceding siblings ...)
  2012-08-16 13:54 ` [PATCH 19/19] pci-assign: Gracefully handle missing in-kernel irqchip support Jan Kiszka
@ 2012-08-16 14:34 ` Avi Kivity
  2012-08-16 14:49   ` Jan Kiszka
  2012-08-16 16:07   ` Michael S. Tsirkin
  2012-08-16 16:33 ` Alex Williamson
  20 siblings, 2 replies; 30+ messages in thread
From: Avi Kivity @ 2012-08-16 14:34 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Marcelo Tosatti, kvm, Alex Williamson

On 08/16/2012 04:54 PM, Jan Kiszka wrote:
> With this series, we are getting very close to obsoleting qemu-kvm. It
> refactors hw/device-assignment.c and the associated KVM helper functions
> into a form that should allow merging them into QEMU. Once the series is
> acceptable for qemu-kvm, I will break out the necessary uq/master
> patches and push pci-assign to upstream.
> 
> The major step of this series is to define a regular set of kvm_device_*
> services that encapsulate classic (i.e. KVM-based, non-VFIO) device
> assignment features and export them to i386 targets only. There will
> never be another arch using them, therefore I pushed them into this
> corner. Moreover, the device assignment device now makes use of the new
> KVM IRQ/MSI routing API and no longer pokes into the internals of that
> layer. Finally, I moved the code into hw/kvm/pci-assign.c, dropped the
> superfluous configure option and did some basic code cleanups (mostly
> coding style) to bring things in shape.
> 
> Note that patch 1 is a simple bug fix that should likely be applied for
> qemu-kvm-1.2 independently.
> 
> This series depends on [1] and [2] and QEMU upstream (2b97f88c92) being
> merged into qemu-kvm.
> 
> Please review.

>From a quick review it looks ready to merge.  Of course I'd appreciate a
review from Alex or Michael as well.

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

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

* Re: [PATCH 00/19] pci-assign: Refactor for upstream merge
  2012-08-16 14:34 ` [PATCH 00/19] pci-assign: Refactor for upstream merge Avi Kivity
@ 2012-08-16 14:49   ` Jan Kiszka
  2012-08-16 16:07   ` Michael S. Tsirkin
  1 sibling, 0 replies; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 14:49 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Marcelo Tosatti, kvm, Alex Williamson, Michael S. Tsirkin

On 2012-08-16 16:34, Avi Kivity wrote:
> On 08/16/2012 04:54 PM, Jan Kiszka wrote:
>> With this series, we are getting very close to obsoleting qemu-kvm. It
>> refactors hw/device-assignment.c and the associated KVM helper functions
>> into a form that should allow merging them into QEMU. Once the series is
>> acceptable for qemu-kvm, I will break out the necessary uq/master
>> patches and push pci-assign to upstream.
>>
>> The major step of this series is to define a regular set of kvm_device_*
>> services that encapsulate classic (i.e. KVM-based, non-VFIO) device
>> assignment features and export them to i386 targets only. There will
>> never be another arch using them, therefore I pushed them into this
>> corner. Moreover, the device assignment device now makes use of the new
>> KVM IRQ/MSI routing API and no longer pokes into the internals of that
>> layer. Finally, I moved the code into hw/kvm/pci-assign.c, dropped the
>> superfluous configure option and did some basic code cleanups (mostly
>> coding style) to bring things in shape.
>>
>> Note that patch 1 is a simple bug fix that should likely be applied for
>> qemu-kvm-1.2 independently.
>>
>> This series depends on [1] and [2] and QEMU upstream (2b97f88c92) being
>> merged into qemu-kvm.
>>
>> Please review.
> 
>>From a quick review it looks ready to merge.  Of course I'd appreciate a
> review from Alex or Michael as well.

Great, thanks.

FWIW, as the upstream integration is technically trivial now, I pushed a
preview to

git://git.kiszka.org/qemu-kvm.git queues/kvm-upstream

Will send out once reviews for this series are done.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

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

* Re: [PATCH 00/19] pci-assign: Refactor for upstream merge
  2012-08-16 14:34 ` [PATCH 00/19] pci-assign: Refactor for upstream merge Avi Kivity
  2012-08-16 14:49   ` Jan Kiszka
@ 2012-08-16 16:07   ` Michael S. Tsirkin
  1 sibling, 0 replies; 30+ messages in thread
From: Michael S. Tsirkin @ 2012-08-16 16:07 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Jan Kiszka, Marcelo Tosatti, kvm, Alex Williamson

On Thu, Aug 16, 2012 at 05:34:51PM +0300, Avi Kivity wrote:
> On 08/16/2012 04:54 PM, Jan Kiszka wrote:
> > With this series, we are getting very close to obsoleting qemu-kvm. It
> > refactors hw/device-assignment.c and the associated KVM helper functions
> > into a form that should allow merging them into QEMU. Once the series is
> > acceptable for qemu-kvm, I will break out the necessary uq/master
> > patches and push pci-assign to upstream.
> > 
> > The major step of this series is to define a regular set of kvm_device_*
> > services that encapsulate classic (i.e. KVM-based, non-VFIO) device
> > assignment features and export them to i386 targets only. There will
> > never be another arch using them, therefore I pushed them into this
> > corner. Moreover, the device assignment device now makes use of the new
> > KVM IRQ/MSI routing API and no longer pokes into the internals of that
> > layer. Finally, I moved the code into hw/kvm/pci-assign.c, dropped the
> > superfluous configure option and did some basic code cleanups (mostly
> > coding style) to bring things in shape.
> > 
> > Note that patch 1 is a simple bug fix that should likely be applied for
> > qemu-kvm-1.2 independently.
> > 
> > This series depends on [1] and [2] and QEMU upstream (2b97f88c92) being
> > merged into qemu-kvm.
> > 
> > Please review.
> 
> >From a quick review it looks ready to merge.  Of course I'd appreciate a
> review from Alex or Michael as well.

Looks good to me too.

Acked-by: Michael S. Tsirkin <mst@redhat.com>



> -- 
> error compiling committee.c: too many arguments to function
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

* Re: [PATCH 10/19] pci-assign: Replace kvm_assign_set_msix_entry with kvm_device_msix_set_vector
  2012-08-16 13:54 ` [PATCH 10/19] pci-assign: Replace kvm_assign_set_msix_entry with kvm_device_msix_set_vector Jan Kiszka
@ 2012-08-16 16:21   ` Alex Williamson
  2012-08-16 16:30     ` Jan Kiszka
  2012-08-16 16:34     ` Avi Kivity
  0 siblings, 2 replies; 30+ messages in thread
From: Alex Williamson @ 2012-08-16 16:21 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Avi Kivity, Marcelo Tosatti, kvm

On Thu, 2012-08-16 at 15:54 +0200, Jan Kiszka wrote:
> The refactored version cleanly hides the KVM IOCTL structure from the
> users and also zeros out the padding field.
> 
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  hw/device-assignment.c |    7 ++-----
>  qemu-kvm.c             |    8 --------
>  qemu-kvm.h             |    4 ----
>  target-i386/kvm.c      |   13 +++++++++++++
>  target-i386/kvm_i386.h |    2 ++
>  5 files changed, 17 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> index 0e2f8e6..af8a5aa 100644
> --- a/hw/device-assignment.c
> +++ b/hw/device-assignment.c
> @@ -1045,7 +1045,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
>      AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
>      uint16_t entries_nr = 0;
>      int i, r = 0;
> -    struct kvm_assigned_msix_entry msix_entry;
>      MSIXTableEntry *entry = adev->msix_table;
>  
>      /* Get the usable entry number for allocating */
> @@ -1075,7 +1074,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
>      adev->irq_entries_nr = adev->msix_max;
>      adev->entry = g_malloc0(adev->msix_max * sizeof(*(adev->entry)));
>  
> -    msix_entry.assigned_dev_id = adev->dev_id;
>      entry = adev->msix_table;
>      for (i = 0; i < adev->msix_max; i++, entry++) {
>          if (msix_masked(entry)) {
> @@ -1098,9 +1096,8 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
>  
>          kvm_add_routing_entry(kvm_state, &adev->entry[i]);
>  
> -        msix_entry.gsi = adev->entry[i].gsi;
> -        msix_entry.entry = i;
> -        r = kvm_assign_set_msix_entry(kvm_state, &msix_entry);
> +        r = kvm_device_msix_set_vector(kvm_state, adev->dev_id, i,
> +                                       adev->entry[i].gsi);
>          if (r) {
>              fprintf(stderr, "fail to set MSI-X entry! %s\n", strerror(-r));
>              break;
> diff --git a/qemu-kvm.c b/qemu-kvm.c
> index 1a2a4fd..ec1911f 100644
> --- a/qemu-kvm.c
> +++ b/qemu-kvm.c
> @@ -185,14 +185,6 @@ int kvm_get_irq_route_gsi(void)
>  #endif
>  }
>  
> -#ifdef KVM_CAP_DEVICE_MSIX
> -int kvm_assign_set_msix_entry(KVMState *s,
> -                              struct kvm_assigned_msix_entry *entry)
> -{
> -    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_ENTRY, entry);
> -}
> -#endif
> -
>  #if !defined(TARGET_I386)
>  void kvm_arch_init_irq_routing(KVMState *s)
>  {
> diff --git a/qemu-kvm.h b/qemu-kvm.h
> index 3fd6046..ad628d5 100644
> --- a/qemu-kvm.h
> +++ b/qemu-kvm.h
> @@ -65,10 +65,6 @@ int kvm_del_routing_entry(struct kvm_irq_routing_entry *entry);
>  int kvm_update_routing_entry(struct kvm_irq_routing_entry *entry,
>                               struct kvm_irq_routing_entry *newentry);
>  
> -
> -int kvm_assign_set_msix_entry(KVMState *s,
> -                              struct kvm_assigned_msix_entry *entry);
> -
>  #endif /* CONFIG_KVM */
>  
>  #endif
> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> index 676f45b..e9353ed 100644
> --- a/target-i386/kvm.c
> +++ b/target-i386/kvm.c
> @@ -2173,6 +2173,19 @@ int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
>      return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, &msix_nr);
>  }
>  
> +int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
> +                               int virq)
> +{
> +    struct kvm_assigned_msix_entry msix_entry = {
> +        .assigned_dev_id = dev_id,
> +        .gsi = virq,
> +        .entry = vector,
> +    };
> +
> +    memset(msix_entry.padding, 0, sizeof(msix_entry.padding));

nit, I think this can be done w/o a memset.  .padding = { 0 }?  Thanks,

Alex


> +    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_ENTRY, &msix_entry);
> +}
> +
>  int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id)
>  {
>      return kvm_deassign_irq_internal(s, dev_id, KVM_DEV_IRQ_GUEST_MSIX |
> diff --git a/target-i386/kvm_i386.h b/target-i386/kvm_i386.h
> index aac14eb..bd3b398 100644
> --- a/target-i386/kvm_i386.h
> +++ b/target-i386/kvm_i386.h
> @@ -30,6 +30,8 @@ int kvm_device_msi_deassign(KVMState *s, uint32_t dev_id);
>  bool kvm_device_msix_supported(KVMState *s);
>  int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
>                                   uint32_t nr_vectors);
> +int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
> +                               int virq);
>  int kvm_device_msix_deassign(KVMState *s, uint32_t dev_id);
>  
>  #endif




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

* Re: [PATCH 10/19] pci-assign: Replace kvm_assign_set_msix_entry with kvm_device_msix_set_vector
  2012-08-16 16:21   ` Alex Williamson
@ 2012-08-16 16:30     ` Jan Kiszka
  2012-08-16 17:30       ` Jan Kiszka
  2012-08-16 16:34     ` Avi Kivity
  1 sibling, 1 reply; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 16:30 UTC (permalink / raw)
  To: Alex Williamson; +Cc: Avi Kivity, Marcelo Tosatti, kvm

On 2012-08-16 18:21, Alex Williamson wrote:
> On Thu, 2012-08-16 at 15:54 +0200, Jan Kiszka wrote:
>> The refactored version cleanly hides the KVM IOCTL structure from the
>> users and also zeros out the padding field.
>>
>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>> ---
>>  hw/device-assignment.c |    7 ++-----
>>  qemu-kvm.c             |    8 --------
>>  qemu-kvm.h             |    4 ----
>>  target-i386/kvm.c      |   13 +++++++++++++
>>  target-i386/kvm_i386.h |    2 ++
>>  5 files changed, 17 insertions(+), 17 deletions(-)
>>
>> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
>> index 0e2f8e6..af8a5aa 100644
>> --- a/hw/device-assignment.c
>> +++ b/hw/device-assignment.c
>> @@ -1045,7 +1045,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
>>      AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
>>      uint16_t entries_nr = 0;
>>      int i, r = 0;
>> -    struct kvm_assigned_msix_entry msix_entry;
>>      MSIXTableEntry *entry = adev->msix_table;
>>  
>>      /* Get the usable entry number for allocating */
>> @@ -1075,7 +1074,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
>>      adev->irq_entries_nr = adev->msix_max;
>>      adev->entry = g_malloc0(adev->msix_max * sizeof(*(adev->entry)));
>>  
>> -    msix_entry.assigned_dev_id = adev->dev_id;
>>      entry = adev->msix_table;
>>      for (i = 0; i < adev->msix_max; i++, entry++) {
>>          if (msix_masked(entry)) {
>> @@ -1098,9 +1096,8 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
>>  
>>          kvm_add_routing_entry(kvm_state, &adev->entry[i]);
>>  
>> -        msix_entry.gsi = adev->entry[i].gsi;
>> -        msix_entry.entry = i;
>> -        r = kvm_assign_set_msix_entry(kvm_state, &msix_entry);
>> +        r = kvm_device_msix_set_vector(kvm_state, adev->dev_id, i,
>> +                                       adev->entry[i].gsi);
>>          if (r) {
>>              fprintf(stderr, "fail to set MSI-X entry! %s\n", strerror(-r));
>>              break;
>> diff --git a/qemu-kvm.c b/qemu-kvm.c
>> index 1a2a4fd..ec1911f 100644
>> --- a/qemu-kvm.c
>> +++ b/qemu-kvm.c
>> @@ -185,14 +185,6 @@ int kvm_get_irq_route_gsi(void)
>>  #endif
>>  }
>>  
>> -#ifdef KVM_CAP_DEVICE_MSIX
>> -int kvm_assign_set_msix_entry(KVMState *s,
>> -                              struct kvm_assigned_msix_entry *entry)
>> -{
>> -    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_ENTRY, entry);
>> -}
>> -#endif
>> -
>>  #if !defined(TARGET_I386)
>>  void kvm_arch_init_irq_routing(KVMState *s)
>>  {
>> diff --git a/qemu-kvm.h b/qemu-kvm.h
>> index 3fd6046..ad628d5 100644
>> --- a/qemu-kvm.h
>> +++ b/qemu-kvm.h
>> @@ -65,10 +65,6 @@ int kvm_del_routing_entry(struct kvm_irq_routing_entry *entry);
>>  int kvm_update_routing_entry(struct kvm_irq_routing_entry *entry,
>>                               struct kvm_irq_routing_entry *newentry);
>>  
>> -
>> -int kvm_assign_set_msix_entry(KVMState *s,
>> -                              struct kvm_assigned_msix_entry *entry);
>> -
>>  #endif /* CONFIG_KVM */
>>  
>>  #endif
>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>> index 676f45b..e9353ed 100644
>> --- a/target-i386/kvm.c
>> +++ b/target-i386/kvm.c
>> @@ -2173,6 +2173,19 @@ int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
>>      return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, &msix_nr);
>>  }
>>  
>> +int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
>> +                               int virq)
>> +{
>> +    struct kvm_assigned_msix_entry msix_entry = {
>> +        .assigned_dev_id = dev_id,
>> +        .gsi = virq,
>> +        .entry = vector,
>> +    };
>> +
>> +    memset(msix_entry.padding, 0, sizeof(msix_entry.padding));
> 
> nit, I think this can be done w/o a memset.  .padding = { 0 }?  Thanks,

I think to remember it has to be .padding = { 0, 0, 0 } (due to three
padding elements) to be standard conforming, but that would still be
nicer than the memset, indeed.

Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

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

* Re: [PATCH 00/19] pci-assign: Refactor for upstream merge
  2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
                   ` (19 preceding siblings ...)
  2012-08-16 14:34 ` [PATCH 00/19] pci-assign: Refactor for upstream merge Avi Kivity
@ 2012-08-16 16:33 ` Alex Williamson
  20 siblings, 0 replies; 30+ messages in thread
From: Alex Williamson @ 2012-08-16 16:33 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Avi Kivity, Marcelo Tosatti, kvm

On Thu, 2012-08-16 at 15:54 +0200, Jan Kiszka wrote:
> With this series, we are getting very close to obsoleting qemu-kvm. It
> refactors hw/device-assignment.c and the associated KVM helper functions
> into a form that should allow merging them into QEMU. Once the series is
> acceptable for qemu-kvm, I will break out the necessary uq/master
> patches and push pci-assign to upstream.
> 
> The major step of this series is to define a regular set of kvm_device_*
> services that encapsulate classic (i.e. KVM-based, non-VFIO) device
> assignment features and export them to i386 targets only. There will
> never be another arch using them, therefore I pushed them into this
> corner. Moreover, the device assignment device now makes use of the new
> KVM IRQ/MSI routing API and no longer pokes into the internals of that
> layer. Finally, I moved the code into hw/kvm/pci-assign.c, dropped the
> superfluous configure option and did some basic code cleanups (mostly
> coding style) to bring things in shape.
> 
> Note that patch 1 is a simple bug fix that should likely be applied for
> qemu-kvm-1.2 independently.
> 
> This series depends on [1] and [2] and QEMU upstream (2b97f88c92) being
> merged into qemu-kvm.
> 
> Please review.
> 
> [1] http://thread.gmane.org/gmane.comp.emulators.kvm.devel/95528
> [2] http://thread.gmane.org/gmane.comp.emulators.kvm.devel/95522
> 
> Jan Kiszka (19):
>   pci-assign: Only clean up registered IO resources
>   pci-assign: Factor out kvm_device_pci_assign/deassign
>   pci-assign: Rename assign_irq to assign_intx
>   pci-assign: Refactor interrupt deassignment
>   pci-assign: Factor out kvm_device_intx_assign
>   qemu-kvm: Move kvm_device_intx_set_mask service
>   pci-assign: Rework MSI assignment
>   pci-assign: Factor out kvm_device_msix_supported
>   pci-assign: Replace kvm_assign_set_msix_nr with
>     kvm_device_msix_init_vectors
>   pci-assign: Replace kvm_assign_set_msix_entry with
>     kvm_device_msix_set_vector
>   pci-assign: Rework MSI-X route setup
>   pci-assign: Factor out kvm_device_msix_assign
>   qemu-kvm: Kill qemu-kvm.[ch]
>   pci-assign: Drop configure switches
>   pci-assign: Move and rename source file
>   pci-assign: Fix coding style issues
>   pci-assign: Replace exit() with hw_error()
>   pci-assign: Drop unused or write-only variables
>   pci-assign: Gracefully handle missing in-kernel irqchip support
> 
>  configure                                    |   11 -
>  hw/i386/Makefile.objs                        |    3 -
>  hw/kvm/Makefile.objs                         |    2 +-
>  hw/{device-assignment.c => kvm/pci-assign.c} |  502 +++++++++++++-------------
>  kvm-all.c                                    |   54 +++-
>  kvm-stub.c                                   |    9 -
>  kvm.h                                        |   12 +-
>  qemu-kvm.c                                   |  233 ------------
>  qemu-kvm.h                                   |  112 ------
>  target-i386/kvm.c                            |  142 ++++++++
>  target-i386/kvm_i386.h                       |   22 ++
>  11 files changed, 461 insertions(+), 641 deletions(-)
>  rename hw/{device-assignment.c => kvm/pci-assign.c} (84%)
>  delete mode 100644 qemu-kvm.c
>  delete mode 100644 qemu-kvm.h

Really nice overall

Acked-by: Alex Williamson <alex.williamson@redhat.com>


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

* Re: [PATCH 10/19] pci-assign: Replace kvm_assign_set_msix_entry with kvm_device_msix_set_vector
  2012-08-16 16:21   ` Alex Williamson
  2012-08-16 16:30     ` Jan Kiszka
@ 2012-08-16 16:34     ` Avi Kivity
  2012-08-16 16:43       ` Alex Williamson
  1 sibling, 1 reply; 30+ messages in thread
From: Avi Kivity @ 2012-08-16 16:34 UTC (permalink / raw)
  To: Alex Williamson; +Cc: Jan Kiszka, Marcelo Tosatti, kvm

On 08/16/2012 07:21 PM, Alex Williamson wrote:
>>  
>> +int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
>> +                               int virq)
>> +{
>> +    struct kvm_assigned_msix_entry msix_entry = {
>> +        .assigned_dev_id = dev_id,
>> +        .gsi = virq,
>> +        .entry = vector,
>> +    };
>> +
>> +    memset(msix_entry.padding, 0, sizeof(msix_entry.padding));
> 
> nit, I think this can be done w/o a memset.  .padding = { 0 }?  Thanks,

It can be done with a null statement.  The msix_entry initialization
above zero-initializes all fields that are not mentioned in the initializer.

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

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

* Re: [PATCH 10/19] pci-assign: Replace kvm_assign_set_msix_entry with kvm_device_msix_set_vector
  2012-08-16 16:34     ` Avi Kivity
@ 2012-08-16 16:43       ` Alex Williamson
  0 siblings, 0 replies; 30+ messages in thread
From: Alex Williamson @ 2012-08-16 16:43 UTC (permalink / raw)
  To: Avi Kivity; +Cc: Jan Kiszka, Marcelo Tosatti, kvm

On Thu, 2012-08-16 at 19:34 +0300, Avi Kivity wrote:
> On 08/16/2012 07:21 PM, Alex Williamson wrote:
> >>  
> >> +int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
> >> +                               int virq)
> >> +{
> >> +    struct kvm_assigned_msix_entry msix_entry = {
> >> +        .assigned_dev_id = dev_id,
> >> +        .gsi = virq,
> >> +        .entry = vector,
> >> +    };
> >> +
> >> +    memset(msix_entry.padding, 0, sizeof(msix_entry.padding));
> > 
> > nit, I think this can be done w/o a memset.  .padding = { 0 }?  Thanks,
> 
> It can be done with a null statement.  The msix_entry initialization
> above zero-initializes all fields that are not mentioned in the initializer.

Yeah, I thought that was probably the case as well.  Thanks for
confirming.  The .padding = 0 in the previous patch could also be
dropped then.

Alex


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

* Re: [PATCH 10/19] pci-assign: Replace kvm_assign_set_msix_entry with kvm_device_msix_set_vector
  2012-08-16 16:30     ` Jan Kiszka
@ 2012-08-16 17:30       ` Jan Kiszka
  2012-08-16 18:23         ` Alex Williamson
  0 siblings, 1 reply; 30+ messages in thread
From: Jan Kiszka @ 2012-08-16 17:30 UTC (permalink / raw)
  To: Alex Williamson; +Cc: Avi Kivity, Marcelo Tosatti, kvm

On 2012-08-16 18:30, Jan Kiszka wrote:
> On 2012-08-16 18:21, Alex Williamson wrote:
>> On Thu, 2012-08-16 at 15:54 +0200, Jan Kiszka wrote:
>>> The refactored version cleanly hides the KVM IOCTL structure from the
>>> users and also zeros out the padding field.
>>>
>>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
>>> ---
>>>  hw/device-assignment.c |    7 ++-----
>>>  qemu-kvm.c             |    8 --------
>>>  qemu-kvm.h             |    4 ----
>>>  target-i386/kvm.c      |   13 +++++++++++++
>>>  target-i386/kvm_i386.h |    2 ++
>>>  5 files changed, 17 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
>>> index 0e2f8e6..af8a5aa 100644
>>> --- a/hw/device-assignment.c
>>> +++ b/hw/device-assignment.c
>>> @@ -1045,7 +1045,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
>>>      AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
>>>      uint16_t entries_nr = 0;
>>>      int i, r = 0;
>>> -    struct kvm_assigned_msix_entry msix_entry;
>>>      MSIXTableEntry *entry = adev->msix_table;
>>>  
>>>      /* Get the usable entry number for allocating */
>>> @@ -1075,7 +1074,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
>>>      adev->irq_entries_nr = adev->msix_max;
>>>      adev->entry = g_malloc0(adev->msix_max * sizeof(*(adev->entry)));
>>>  
>>> -    msix_entry.assigned_dev_id = adev->dev_id;
>>>      entry = adev->msix_table;
>>>      for (i = 0; i < adev->msix_max; i++, entry++) {
>>>          if (msix_masked(entry)) {
>>> @@ -1098,9 +1096,8 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
>>>  
>>>          kvm_add_routing_entry(kvm_state, &adev->entry[i]);
>>>  
>>> -        msix_entry.gsi = adev->entry[i].gsi;
>>> -        msix_entry.entry = i;
>>> -        r = kvm_assign_set_msix_entry(kvm_state, &msix_entry);
>>> +        r = kvm_device_msix_set_vector(kvm_state, adev->dev_id, i,
>>> +                                       adev->entry[i].gsi);
>>>          if (r) {
>>>              fprintf(stderr, "fail to set MSI-X entry! %s\n", strerror(-r));
>>>              break;
>>> diff --git a/qemu-kvm.c b/qemu-kvm.c
>>> index 1a2a4fd..ec1911f 100644
>>> --- a/qemu-kvm.c
>>> +++ b/qemu-kvm.c
>>> @@ -185,14 +185,6 @@ int kvm_get_irq_route_gsi(void)
>>>  #endif
>>>  }
>>>  
>>> -#ifdef KVM_CAP_DEVICE_MSIX
>>> -int kvm_assign_set_msix_entry(KVMState *s,
>>> -                              struct kvm_assigned_msix_entry *entry)
>>> -{
>>> -    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_ENTRY, entry);
>>> -}
>>> -#endif
>>> -
>>>  #if !defined(TARGET_I386)
>>>  void kvm_arch_init_irq_routing(KVMState *s)
>>>  {
>>> diff --git a/qemu-kvm.h b/qemu-kvm.h
>>> index 3fd6046..ad628d5 100644
>>> --- a/qemu-kvm.h
>>> +++ b/qemu-kvm.h
>>> @@ -65,10 +65,6 @@ int kvm_del_routing_entry(struct kvm_irq_routing_entry *entry);
>>>  int kvm_update_routing_entry(struct kvm_irq_routing_entry *entry,
>>>                               struct kvm_irq_routing_entry *newentry);
>>>  
>>> -
>>> -int kvm_assign_set_msix_entry(KVMState *s,
>>> -                              struct kvm_assigned_msix_entry *entry);
>>> -
>>>  #endif /* CONFIG_KVM */
>>>  
>>>  #endif
>>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
>>> index 676f45b..e9353ed 100644
>>> --- a/target-i386/kvm.c
>>> +++ b/target-i386/kvm.c
>>> @@ -2173,6 +2173,19 @@ int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
>>>      return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, &msix_nr);
>>>  }
>>>  
>>> +int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
>>> +                               int virq)
>>> +{
>>> +    struct kvm_assigned_msix_entry msix_entry = {
>>> +        .assigned_dev_id = dev_id,
>>> +        .gsi = virq,
>>> +        .entry = vector,
>>> +    };
>>> +
>>> +    memset(msix_entry.padding, 0, sizeof(msix_entry.padding));
>>
>> nit, I think this can be done w/o a memset.  .padding = { 0 }?  Thanks,
> 
> I think to remember it has to be .padding = { 0, 0, 0 } (due to three
> padding elements) to be standard conforming, but that would still be
> nicer than the memset, indeed.

I've found some minor inconsistencies in the IOCTL struct
zero-initializations. Will send v2, just waiting in case you have more
comments.

Thanks,
Jan

-- 
Siemens AG, Corporate Technology, CT RTC ITP SDP-DE
Corporate Competence Center Embedded Linux

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

* Re: [PATCH 10/19] pci-assign: Replace kvm_assign_set_msix_entry with kvm_device_msix_set_vector
  2012-08-16 17:30       ` Jan Kiszka
@ 2012-08-16 18:23         ` Alex Williamson
  0 siblings, 0 replies; 30+ messages in thread
From: Alex Williamson @ 2012-08-16 18:23 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Avi Kivity, Marcelo Tosatti, kvm

On Thu, 2012-08-16 at 19:30 +0200, Jan Kiszka wrote:
> On 2012-08-16 18:30, Jan Kiszka wrote:
> > On 2012-08-16 18:21, Alex Williamson wrote:
> >> On Thu, 2012-08-16 at 15:54 +0200, Jan Kiszka wrote:
> >>> The refactored version cleanly hides the KVM IOCTL structure from the
> >>> users and also zeros out the padding field.
> >>>
> >>> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >>> ---
> >>>  hw/device-assignment.c |    7 ++-----
> >>>  qemu-kvm.c             |    8 --------
> >>>  qemu-kvm.h             |    4 ----
> >>>  target-i386/kvm.c      |   13 +++++++++++++
> >>>  target-i386/kvm_i386.h |    2 ++
> >>>  5 files changed, 17 insertions(+), 17 deletions(-)
> >>>
> >>> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> >>> index 0e2f8e6..af8a5aa 100644
> >>> --- a/hw/device-assignment.c
> >>> +++ b/hw/device-assignment.c
> >>> @@ -1045,7 +1045,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
> >>>      AssignedDevice *adev = DO_UPCAST(AssignedDevice, dev, pci_dev);
> >>>      uint16_t entries_nr = 0;
> >>>      int i, r = 0;
> >>> -    struct kvm_assigned_msix_entry msix_entry;
> >>>      MSIXTableEntry *entry = adev->msix_table;
> >>>  
> >>>      /* Get the usable entry number for allocating */
> >>> @@ -1075,7 +1074,6 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
> >>>      adev->irq_entries_nr = adev->msix_max;
> >>>      adev->entry = g_malloc0(adev->msix_max * sizeof(*(adev->entry)));
> >>>  
> >>> -    msix_entry.assigned_dev_id = adev->dev_id;
> >>>      entry = adev->msix_table;
> >>>      for (i = 0; i < adev->msix_max; i++, entry++) {
> >>>          if (msix_masked(entry)) {
> >>> @@ -1098,9 +1096,8 @@ static int assigned_dev_update_msix_mmio(PCIDevice *pci_dev)
> >>>  
> >>>          kvm_add_routing_entry(kvm_state, &adev->entry[i]);
> >>>  
> >>> -        msix_entry.gsi = adev->entry[i].gsi;
> >>> -        msix_entry.entry = i;
> >>> -        r = kvm_assign_set_msix_entry(kvm_state, &msix_entry);
> >>> +        r = kvm_device_msix_set_vector(kvm_state, adev->dev_id, i,
> >>> +                                       adev->entry[i].gsi);
> >>>          if (r) {
> >>>              fprintf(stderr, "fail to set MSI-X entry! %s\n", strerror(-r));
> >>>              break;
> >>> diff --git a/qemu-kvm.c b/qemu-kvm.c
> >>> index 1a2a4fd..ec1911f 100644
> >>> --- a/qemu-kvm.c
> >>> +++ b/qemu-kvm.c
> >>> @@ -185,14 +185,6 @@ int kvm_get_irq_route_gsi(void)
> >>>  #endif
> >>>  }
> >>>  
> >>> -#ifdef KVM_CAP_DEVICE_MSIX
> >>> -int kvm_assign_set_msix_entry(KVMState *s,
> >>> -                              struct kvm_assigned_msix_entry *entry)
> >>> -{
> >>> -    return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_ENTRY, entry);
> >>> -}
> >>> -#endif
> >>> -
> >>>  #if !defined(TARGET_I386)
> >>>  void kvm_arch_init_irq_routing(KVMState *s)
> >>>  {
> >>> diff --git a/qemu-kvm.h b/qemu-kvm.h
> >>> index 3fd6046..ad628d5 100644
> >>> --- a/qemu-kvm.h
> >>> +++ b/qemu-kvm.h
> >>> @@ -65,10 +65,6 @@ int kvm_del_routing_entry(struct kvm_irq_routing_entry *entry);
> >>>  int kvm_update_routing_entry(struct kvm_irq_routing_entry *entry,
> >>>                               struct kvm_irq_routing_entry *newentry);
> >>>  
> >>> -
> >>> -int kvm_assign_set_msix_entry(KVMState *s,
> >>> -                              struct kvm_assigned_msix_entry *entry);
> >>> -
> >>>  #endif /* CONFIG_KVM */
> >>>  
> >>>  #endif
> >>> diff --git a/target-i386/kvm.c b/target-i386/kvm.c
> >>> index 676f45b..e9353ed 100644
> >>> --- a/target-i386/kvm.c
> >>> +++ b/target-i386/kvm.c
> >>> @@ -2173,6 +2173,19 @@ int kvm_device_msix_init_vectors(KVMState *s, uint32_t dev_id,
> >>>      return kvm_vm_ioctl(s, KVM_ASSIGN_SET_MSIX_NR, &msix_nr);
> >>>  }
> >>>  
> >>> +int kvm_device_msix_set_vector(KVMState *s, uint32_t dev_id, uint32_t vector,
> >>> +                               int virq)
> >>> +{
> >>> +    struct kvm_assigned_msix_entry msix_entry = {
> >>> +        .assigned_dev_id = dev_id,
> >>> +        .gsi = virq,
> >>> +        .entry = vector,
> >>> +    };
> >>> +
> >>> +    memset(msix_entry.padding, 0, sizeof(msix_entry.padding));
> >>
> >> nit, I think this can be done w/o a memset.  .padding = { 0 }?  Thanks,
> > 
> > I think to remember it has to be .padding = { 0, 0, 0 } (due to three
> > padding elements) to be standard conforming, but that would still be
> > nicer than the memset, indeed.
> 
> I've found some minor inconsistencies in the IOCTL struct
> zero-initializations. Will send v2, just waiting in case you have more
> comments.

Nope, I didn't see anything else.  Thanks,

Alex




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

end of thread, other threads:[~2012-08-16 18:23 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-08-16 13:54 [PATCH 00/19] pci-assign: Refactor for upstream merge Jan Kiszka
2012-08-16 13:54 ` [PATCH 01/19] pci-assign: Only clean up registered IO resources Jan Kiszka
2012-08-16 13:54 ` [PATCH 02/19] pci-assign: Factor out kvm_device_pci_assign/deassign Jan Kiszka
2012-08-16 13:54 ` [PATCH 03/19] pci-assign: Rename assign_irq to assign_intx Jan Kiszka
2012-08-16 13:54 ` [PATCH 04/19] pci-assign: Refactor interrupt deassignment Jan Kiszka
2012-08-16 13:54 ` [PATCH 05/19] pci-assign: Factor out kvm_device_intx_assign Jan Kiszka
2012-08-16 13:54 ` [PATCH 06/19] qemu-kvm: Move kvm_device_intx_set_mask service Jan Kiszka
2012-08-16 13:54 ` [PATCH 07/19] pci-assign: Rework MSI assignment Jan Kiszka
2012-08-16 13:54 ` [PATCH 08/19] pci-assign: Factor out kvm_device_msix_supported Jan Kiszka
2012-08-16 13:54 ` [PATCH 09/19] pci-assign: Replace kvm_assign_set_msix_nr with kvm_device_msix_init_vectors Jan Kiszka
2012-08-16 13:54 ` [PATCH 10/19] pci-assign: Replace kvm_assign_set_msix_entry with kvm_device_msix_set_vector Jan Kiszka
2012-08-16 16:21   ` Alex Williamson
2012-08-16 16:30     ` Jan Kiszka
2012-08-16 17:30       ` Jan Kiszka
2012-08-16 18:23         ` Alex Williamson
2012-08-16 16:34     ` Avi Kivity
2012-08-16 16:43       ` Alex Williamson
2012-08-16 13:54 ` [PATCH 11/19] pci-assign: Rework MSI-X route setup Jan Kiszka
2012-08-16 13:54 ` [PATCH 12/19] pci-assign: Factor out kvm_device_msix_assign Jan Kiszka
2012-08-16 13:54 ` [PATCH 13/19] qemu-kvm: Kill qemu-kvm.[ch] Jan Kiszka
2012-08-16 13:54 ` [PATCH 14/19] pci-assign: Drop configure switches Jan Kiszka
2012-08-16 13:54 ` [PATCH 15/19] pci-assign: Move and rename source file Jan Kiszka
2012-08-16 13:54 ` [PATCH 16/19] pci-assign: Fix coding style issues Jan Kiszka
2012-08-16 13:54 ` [PATCH 17/19] pci-assign: Replace exit() with hw_error() Jan Kiszka
2012-08-16 13:54 ` [PATCH 18/19] pci-assign: Drop unused or write-only variables Jan Kiszka
2012-08-16 13:54 ` [PATCH 19/19] pci-assign: Gracefully handle missing in-kernel irqchip support Jan Kiszka
2012-08-16 14:34 ` [PATCH 00/19] pci-assign: Refactor for upstream merge Avi Kivity
2012-08-16 14:49   ` Jan Kiszka
2012-08-16 16:07   ` Michael S. Tsirkin
2012-08-16 16:33 ` Alex Williamson

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.