kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] qemu-kvm: Fixes and improvements for device assigment
@ 2010-11-02 14:55 Jan Kiszka
  2010-11-02 14:55 ` [PATCH 1/4] pci-assign: Convert iommu property to booleam Jan Kiszka
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Jan Kiszka @ 2010-11-02 14:55 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm

Mostly usability fixes, but also a patch to allow using devices that
have problems with MSIs.

Jan Kiszka (4):
  pci-assign: Convert iommu property to booleam
  pci-assign: Allow to disable MSI perference for host IRQ
  pci-assign: Issue warning when running w/o IOMMU
  pci-assign: Remove broken -pcidevice and pci_add host

 hmp-commands.hx        |    2 +-
 hw/device-assignment.c |   81 ++++++++---------------------------------------
 hw/device-assignment.h |   15 ++++-----
 hw/ipf.c               |    6 ---
 hw/pc.c                |    6 ---
 hw/pci-hotplug.c       |   22 -------------
 qemu-options.hx        |    5 ---
 vl.c                   |   14 --------
 8 files changed, 22 insertions(+), 129 deletions(-)


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

* [PATCH 1/4] pci-assign: Convert iommu property to booleam
  2010-11-02 14:55 [PATCH 0/4] qemu-kvm: Fixes and improvements for device assigment Jan Kiszka
@ 2010-11-02 14:55 ` Jan Kiszka
  2010-11-02 14:55 ` [PATCH 2/4] pci-assign: Allow to disable MSI perference for host IRQ Jan Kiszka
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Jan Kiszka @ 2010-11-02 14:55 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm

Defining the iommu property as an integer opens the door for confusion
(Is it an index or a switch?). Fix this by converting it to a bit
property that can be on or off, nothing else.

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

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 2605bd1..349e864 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -902,7 +902,7 @@ static int assign_device(AssignedDevice *dev)
 
 #ifdef KVM_CAP_IOMMU
     /* We always enable the IOMMU unless disabled on the command line */
-    if (dev->use_iommu) {
+    if (dev->features & ASSIGNED_DEVICE_USE_IOMMU_MASK) {
         if (!kvm_check_extension(kvm_state, KVM_CAP_IOMMU)) {
             fprintf(stderr, "No IOMMU found.  Unable to assign device \"%s\"\n",
                     dev->dev.qdev.id);
@@ -911,7 +911,7 @@ static int assign_device(AssignedDevice *dev)
         assigned_dev_data.flags |= KVM_DEV_ASSIGN_ENABLE_IOMMU;
     }
 #else
-    dev->use_iommu = 0;
+    dev->features &= ~ASSIGNED_DEVICE_USE_IOMMU_MASK;
 #endif
 
     r = kvm_assign_pci_device(kvm_context, &assigned_dev_data);
@@ -1538,7 +1538,8 @@ static PCIDeviceInfo assign_info = {
     .config_write = assigned_dev_pci_write_config,
     .qdev.props   = (Property[]) {
         DEFINE_PROP("host", AssignedDevice, host, qdev_prop_hostaddr, PCIHostDevice),
-        DEFINE_PROP_UINT32("iommu", AssignedDevice, use_iommu, 1),
+        DEFINE_PROP_BIT("iommu", AssignedDevice, features,
+                        ASSIGNED_DEVICE_USE_IOMMU_BIT, true),
         DEFINE_PROP_STRING("configfd", AssignedDevice, configfd_name),
         DEFINE_PROP_END_OF_LIST(),
     },
diff --git a/hw/device-assignment.h b/hw/device-assignment.h
index 2f5fa17..4eb5835 100644
--- a/hw/device-assignment.h
+++ b/hw/device-assignment.h
@@ -74,10 +74,14 @@ typedef struct {
     PCIRegion *region;
 } AssignedDevRegion;
 
+#define ASSIGNED_DEVICE_USE_IOMMU_BIT	0
+
+#define ASSIGNED_DEVICE_USE_IOMMU_MASK	(1 << ASSIGNED_DEVICE_USE_IOMMU_BIT)
+
 typedef struct AssignedDevice {
     PCIDevice dev;
     PCIHostDevice host;
-    uint32_t use_iommu;
+    uint32_t features;
     int intpin;
     uint8_t debug_flags;
     AssignedDevRegion v_addrs[PCI_NUM_REGIONS - 1];
-- 
1.7.1


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

* [PATCH 2/4] pci-assign: Allow to disable MSI perference for host IRQ
  2010-11-02 14:55 [PATCH 0/4] qemu-kvm: Fixes and improvements for device assigment Jan Kiszka
  2010-11-02 14:55 ` [PATCH 1/4] pci-assign: Convert iommu property to booleam Jan Kiszka
@ 2010-11-02 14:55 ` Jan Kiszka
  2010-11-03 12:54   ` Markus Armbruster
  2010-11-02 14:55 ` [PATCH 3/4] pci-assign: Issue warning when running w/o IOMMU Jan Kiszka
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 11+ messages in thread
From: Jan Kiszka @ 2010-11-02 14:55 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm

Some devices (e.g. the ath9k) claim to support MSI but actually do not
work when this is enabled. We must not blindly switch such devices to
MSI but rather provide the user a way to pass control back to the
guest driver. This can be done by turning the new property "prefer_msi"
off (default remains on).

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

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 349e864..73d8afd 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -964,7 +964,8 @@ static int assign_irq(AssignedDevice *dev)
     }
 
     assigned_irq_data.flags = KVM_DEV_IRQ_GUEST_INTX;
-    if (dev->cap.available & ASSIGNED_DEVICE_CAP_MSI)
+    if (dev->features & ASSIGNED_DEVICE_PREFER_MSI_MASK &&
+        dev->cap.available & ASSIGNED_DEVICE_CAP_MSI)
         assigned_irq_data.flags |= KVM_DEV_IRQ_HOST_MSI;
     else
         assigned_irq_data.flags |= KVM_DEV_IRQ_HOST_INTX;
@@ -1541,6 +1542,8 @@ static PCIDeviceInfo assign_info = {
         DEFINE_PROP_BIT("iommu", AssignedDevice, features,
                         ASSIGNED_DEVICE_USE_IOMMU_BIT, true),
         DEFINE_PROP_STRING("configfd", AssignedDevice, configfd_name),
+        DEFINE_PROP_BIT("prefer_msi", AssignedDevice, features,
+                        ASSIGNED_DEVICE_PREFER_MSI_BIT, true),
         DEFINE_PROP_END_OF_LIST(),
     },
 };
diff --git a/hw/device-assignment.h b/hw/device-assignment.h
index 4eb5835..56b7076 100644
--- a/hw/device-assignment.h
+++ b/hw/device-assignment.h
@@ -75,8 +75,10 @@ typedef struct {
 } AssignedDevRegion;
 
 #define ASSIGNED_DEVICE_USE_IOMMU_BIT	0
+#define ASSIGNED_DEVICE_PREFER_MSI_BIT	1
 
 #define ASSIGNED_DEVICE_USE_IOMMU_MASK	(1 << ASSIGNED_DEVICE_USE_IOMMU_BIT)
+#define ASSIGNED_DEVICE_PREFER_MSI_MASK	(1 << ASSIGNED_DEVICE_PREFER_MSI_BIT)
 
 typedef struct AssignedDevice {
     PCIDevice dev;
-- 
1.7.1


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

* [PATCH 3/4] pci-assign: Issue warning when running w/o IOMMU
  2010-11-02 14:55 [PATCH 0/4] qemu-kvm: Fixes and improvements for device assigment Jan Kiszka
  2010-11-02 14:55 ` [PATCH 1/4] pci-assign: Convert iommu property to booleam Jan Kiszka
  2010-11-02 14:55 ` [PATCH 2/4] pci-assign: Allow to disable MSI perference for host IRQ Jan Kiszka
@ 2010-11-02 14:55 ` Jan Kiszka
  2010-11-02 14:55 ` [PATCH 4/4] pci-assign: Remove broken -pcidevice and pci_add host Jan Kiszka
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Jan Kiszka @ 2010-11-02 14:55 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm

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

diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index 73d8afd..d8c565c 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -913,6 +913,12 @@ static int assign_device(AssignedDevice *dev)
 #else
     dev->features &= ~ASSIGNED_DEVICE_USE_IOMMU_MASK;
 #endif
+    if (!(dev->features & ASSIGNED_DEVICE_USE_IOMMU_MASK)) {
+        fprintf(stderr,
+                "WARNING: Assigning a device without IOMMU protection can "
+                "cause host memory corruption if the device issues DMA write "
+                "requests!\n");
+    }
 
     r = kvm_assign_pci_device(kvm_context, &assigned_dev_data);
     if (r < 0) {
-- 
1.7.1


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

* [PATCH 4/4] pci-assign: Remove broken -pcidevice and pci_add host
  2010-11-02 14:55 [PATCH 0/4] qemu-kvm: Fixes and improvements for device assigment Jan Kiszka
                   ` (2 preceding siblings ...)
  2010-11-02 14:55 ` [PATCH 3/4] pci-assign: Issue warning when running w/o IOMMU Jan Kiszka
@ 2010-11-02 14:55 ` Jan Kiszka
  2010-11-03 13:02   ` Markus Armbruster
       [not found] ` <4CD119D6.2050602@web.de>
  2010-11-03 20:03 ` [PATCH 0/4] qemu-kvm: Fixes and improvements for device assigment Marcelo Tosatti
  5 siblings, 1 reply; 11+ messages in thread
From: Jan Kiszka @ 2010-11-02 14:55 UTC (permalink / raw)
  To: Avi Kivity, Marcelo Tosatti; +Cc: kvm, Markus Armbruster, Daniel P. Berrange

These qemu-kvm-only interfaces were broken by b560a9ab9b, but no one
complained loud enough to get them fixed again. As we have properly
working "-device pci-assign"/"device_add" and we won't push this
upstream anyway, there is likely no point in restoring the interface.

Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
CC: Markus Armbruster <armbru@redhat.com>
CC: Daniel P. Berrange <berrange@redhat.com>
---
 hmp-commands.hx        |    2 +-
 hw/device-assignment.c |   63 ------------------------------------------------
 hw/device-assignment.h |    7 -----
 hw/ipf.c               |    6 ----
 hw/pc.c                |    6 ----
 hw/pci-hotplug.c       |   22 ----------------
 qemu-options.hx        |    5 ----
 vl.c                   |   14 ----------
 8 files changed, 1 insertions(+), 124 deletions(-)

diff --git a/hmp-commands.hx b/hmp-commands.hx
index 9406496..ba6de28 100644
--- a/hmp-commands.hx
+++ b/hmp-commands.hx
@@ -806,7 +806,7 @@ ETEXI
     {
         .name       = "pci_add",
         .args_type  = "pci_addr:s,type:s,opts:s?",
-        .params     = "auto|[[<domain>:]<bus>:]<slot> nic|storage|host [[vlan=n][,macaddr=addr][,model=type]] [file=file][,if=type][,bus=nr]... [host=02:00.0[,name=string][,dma=none]",
+        .params     = "auto|[[<domain>:]<bus>:]<slot> nic|storage [[vlan=n][,macaddr=addr][,model=type]] [file=file][,if=type][,bus=nr]...",
         .help       = "hot-add PCI device",
         .mhandler.cmd = pci_device_hot_add,
     },
diff --git a/hw/device-assignment.c b/hw/device-assignment.c
index d8c565c..0bbf9d9 100644
--- a/hw/device-assignment.c
+++ b/hw/device-assignment.c
@@ -1561,69 +1561,6 @@ static void assign_register_devices(void)
 
 device_init(assign_register_devices)
 
-
-/*
- * Syntax to assign device:
- *
- * -pcidevice host=bus:dev.func[,dma=none][,name=Foo]
- *
- * Example:
- * -pcidevice host=00:13.0,dma=pvdma
- *
- * dma can currently only be 'none' to disable iommu support.
- */
-QemuOpts *add_assigned_device(const char *arg)
-{
-    QemuOpts *opts = NULL;
-    char host[64], id[64], dma[8];
-    int r;
-
-    r = get_param_value(host, sizeof(host), "host", arg);
-    if (!r)
-         goto bad;
-    r = get_param_value(id, sizeof(id), "id", arg);
-    if (!r)
-        r = get_param_value(id, sizeof(id), "name", arg);
-    if (!r)
-        r = get_param_value(id, sizeof(id), "host", arg);
-
-    opts = qemu_opts_create(qemu_find_opts("device"), id, 0);
-    if (!opts)
-        goto bad;
-    qemu_opt_set(opts, "driver", "pci-assign");
-    qemu_opt_set(opts, "host", host);
-
-#ifdef KVM_CAP_IOMMU
-    r = get_param_value(dma, sizeof(dma), "dma", arg);
-    if (r && !strncmp(dma, "none", 4))
-        qemu_opt_set(opts, "iommu", "0");
-#endif
-    qemu_opts_print(opts, NULL);
-    return opts;
-
-bad:
-    fprintf(stderr, "pcidevice argument parse error; "
-            "please check the help text for usage\n");
-    if (opts)
-        qemu_opts_del(opts);
-    return NULL;
-}
-
-void add_assigned_devices(PCIBus *bus, const char **devices, int n_devices)
-{
-    QemuOpts *opts;
-    int i;
-
-    for (i = 0; i < n_devices; i++) {
-        opts = add_assigned_device(devices[i]);
-        if (opts == NULL) {
-            fprintf(stderr, "Could not add assigned device %s\n", devices[i]);
-            exit(1);
-        }
-        /* generic code will call qdev_device_add() for the device */
-    }
-}
-
 /*
  * Scan the assigned devices for the devices that have an option ROM, and then
  * load the corresponding ROM data to RAM. If an error occurs while loading an
diff --git a/hw/device-assignment.h b/hw/device-assignment.h
index 56b7076..c94a730 100644
--- a/hw/device-assignment.h
+++ b/hw/device-assignment.h
@@ -114,13 +114,6 @@ typedef struct AssignedDevice {
     QLIST_ENTRY(AssignedDevice) next;
 } AssignedDevice;
 
-QemuOpts *add_assigned_device(const char *arg);
-void add_assigned_devices(PCIBus *bus, const char **devices, int n_devices);
 void assigned_dev_update_irqs(void);
 
-#define MAX_DEV_ASSIGN_CMDLINE 8
-
-extern const char *assigned_devices[MAX_DEV_ASSIGN_CMDLINE];
-extern int assigned_devices_index;
-
 #endif              /* __DEVICE_ASSIGNMENT_H__ */
diff --git a/hw/ipf.c b/hw/ipf.c
index 21cff72..10c21eb 100644
--- a/hw/ipf.c
+++ b/hw/ipf.c
@@ -635,12 +635,6 @@ static void ipf_init1(ram_addr_t ram_size,
 	    unit_id++;
 	}
     }
-
-#ifdef CONFIG_KVM_DEVICE_ASSIGNMENT
-    if (kvm_enabled())
-	add_assigned_devices(pci_bus, assigned_devices, assigned_devices_index);
-#endif /* CONFIG_KVM_DEVICE_ASSIGNMENT */
-
 }
 
 static void ipf_init_pci(ram_addr_t ram_size,
diff --git a/hw/pc.c b/hw/pc.c
index b624873..3bf3862 100644
--- a/hw/pc.c
+++ b/hw/pc.c
@@ -1105,10 +1105,4 @@ void pc_pci_device_init(PCIBus *pci_bus)
 
         extboot_init(info->bdrv);
     }
-
-#ifdef CONFIG_KVM_DEVICE_ASSIGNMENT
-    if (kvm_enabled()) {
-        add_assigned_devices(pci_bus, assigned_devices, assigned_devices_index);
-    }
-#endif /* CONFIG_KVM_DEVICE_ASSIGNMENT */
 }
diff --git a/hw/pci-hotplug.c b/hw/pci-hotplug.c
index b370b9b..a2d4bb2 100644
--- a/hw/pci-hotplug.c
+++ b/hw/pci-hotplug.c
@@ -230,24 +230,6 @@ static PCIDevice *qemu_pci_hot_add_storage(Monitor *mon,
     return dev;
 }
 
-#ifdef CONFIG_KVM_DEVICE_ASSIGNMENT
-static PCIDevice *qemu_pci_hot_assign_device(Monitor *mon,
-                                             const char *devaddr,
-                                             const char *opts_str)
-{
-    QemuOpts *opts;
-    DeviceState *dev;
-
-    opts = add_assigned_device(opts_str);
-    if (opts == NULL) {
-        monitor_printf(mon, "Error adding device; check syntax\n");
-        return NULL;
-    }
-    dev = qdev_device_add(opts);
-    return DO_UPCAST(PCIDevice, qdev, dev);
-}
-#endif /* CONFIG_KVM_DEVICE_ASSIGNMENT */
-
 void pci_device_hot_add(Monitor *mon, const QDict *qdict)
 {
     PCIDevice *dev = NULL;
@@ -271,10 +253,6 @@ void pci_device_hot_add(Monitor *mon, const QDict *qdict)
         dev = qemu_pci_hot_add_nic(mon, pci_addr, opts);
     } else if (strcmp(type, "storage") == 0) {
         dev = qemu_pci_hot_add_storage(mon, pci_addr, opts);
-#ifdef CONFIG_KVM_DEVICE_ASSIGNMENT
-    } else if (strcmp(type, "host") == 0) {
-        dev = qemu_pci_hot_assign_device(mon, pci_addr, opts);
-#endif /* CONFIG_KVM_DEVICE_ASSIGNMENT */
     } else {
         monitor_printf(mon, "invalid type: %s\n", type);
     }
diff --git a/qemu-options.hx b/qemu-options.hx
index 1c9a1e5..699b653 100644
--- a/qemu-options.hx
+++ b/qemu-options.hx
@@ -2278,11 +2278,6 @@ DEF("no-kvm-pit-reinjection", 0, QEMU_OPTION_no_kvm_pit_reinjection,
     "-no-kvm-pit-reinjection\n"
     "                disable KVM kernel mode PIT interrupt reinjection\n",
     QEMU_ARCH_I386)
-DEF("pcidevice", HAS_ARG, QEMU_OPTION_pcidevice,
-    "-pcidevice host=[seg:]bus:dev.func[,dma=none][,name=string]\n"
-    "                expose a PCI device to the guest OS\n"
-    "                dma=none: don't perform any dma translations (default is to use an iommu)\n"
-    "                'string' is used in log output\n", QEMU_ARCH_I386)
 DEF("enable-nesting", 0, QEMU_OPTION_enable_nesting,
     "-enable-nesting enable support for running a VM inside the VM (AMD only)\n", QEMU_ARCH_I386)
 DEF("nvram", HAS_ARG, QEMU_OPTION_nvram,
diff --git a/vl.c b/vl.c
index 42617c2..b5da216 100644
--- a/vl.c
+++ b/vl.c
@@ -204,8 +204,6 @@ int win2k_install_hack = 0;
 int rtc_td_hack = 0;
 int usb_enabled = 0;
 int singlestep = 0;
-const char *assigned_devices[MAX_DEV_ASSIGN_CMDLINE];
-int assigned_devices_index;
 int smp_cpus = 1;
 int max_cpus = 0;
 int smp_cores = 1;
@@ -1885,8 +1883,6 @@ int main(int argc, char **argv, char **envp)
         node_cpumask[i] = 0;
     }
 
-    assigned_devices_index = 0;
-
     nb_numa_nodes = 0;
     nb_nics = 0;
 
@@ -2502,16 +2498,6 @@ int main(int argc, char **argv, char **envp)
 		break;
 	    }
 #endif
-#if defined(TARGET_I386) || defined(TARGET_X86_64) || defined(TARGET_IA64) || defined(__linux__)
-            case QEMU_OPTION_pcidevice:
-		if (assigned_devices_index >= MAX_DEV_ASSIGN_CMDLINE) {
-                    fprintf(stderr, "Too many assigned devices\n");
-                    exit(1);
-		}
-		assigned_devices[assigned_devices_index] = optarg;
-		assigned_devices_index++;
-                break;
-#endif
             case QEMU_OPTION_usb:
                 usb_enabled = 1;
                 break;
-- 
1.7.1


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

* Re: [PATCH 5/4] pci-assign: Use PCI-2.3-based shared legacy interrupts by default
       [not found]     ` <4CD120E9.7050306@web.de>
@ 2010-11-03  9:11       ` Michael S. Tsirkin
  0 siblings, 0 replies; 11+ messages in thread
From: Michael S. Tsirkin @ 2010-11-03  9:11 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Avi Kivity, Marcelo Tosatti, kvm

On Wed, Nov 03, 2010 at 09:44:25AM +0100, Jan Kiszka wrote:
> Am 03.11.2010 09:33, Michael S. Tsirkin wrote:
> > On Wed, Nov 03, 2010 at 09:14:14AM +0100, Jan Kiszka wrote:
> >> From: Jan Kiszka <jan.kiszka@siemens.com>
> >>
> >> Make use of the new KVM feature that allows legacy interrupt sharing
> >> for PCI-2.3-compliant devices. Exclusive mode (with IRQ masking at
> >> interrupt controller level) can be re-enabled via disabling the
> >> property "pci_2_3".
> >>
> >> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> >> ---
> >>  hw/device-assignment.c |    7 +++++++
> >>  hw/device-assignment.h |    2 ++
> >>  2 files changed, 9 insertions(+), 0 deletions(-)
> >>
> >> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> >> index 0bbf9d9..5c7e958 100644
> >> --- a/hw/device-assignment.c
> >> +++ b/hw/device-assignment.c
> >> @@ -970,6 +970,11 @@ static int assign_irq(AssignedDevice *dev)
> >>      }
> >>  
> >>      assigned_irq_data.flags = KVM_DEV_IRQ_GUEST_INTX;
> >> +#ifdef KVM_CAP_PCI_2_3
> >> +    if (dev->features & ASSIGNED_DEVICE_USE_PCI_2_3_MASK) {
> >> +        assigned_irq_data.flags |= KVM_DEV_IRQ_PCI_2_3;
> >> +    }
> >> +#endif /* KVM_CAP_PCI_2_3 */
> >>      if (dev->features & ASSIGNED_DEVICE_PREFER_MSI_MASK &&
> >>          dev->cap.available & ASSIGNED_DEVICE_CAP_MSI)
> >>          assigned_irq_data.flags |= KVM_DEV_IRQ_HOST_MSI;
> >> @@ -1550,6 +1555,8 @@ static PCIDeviceInfo assign_info = {
> >>          DEFINE_PROP_STRING("configfd", AssignedDevice, configfd_name),
> >>          DEFINE_PROP_BIT("prefer_msi", AssignedDevice, features,
> >>                          ASSIGNED_DEVICE_PREFER_MSI_BIT, true),
> >> +        DEFINE_PROP_BIT("pci_2_3", AssignedDevice, features,
> >> +                        ASSIGNED_DEVICE_USE_PCI_2_3_BIT, true),
> > 
> > I'd prefer some other name that hints this is a kvm feature.
> > Reason is, we might want to have pci_2_3 to mean that
> > INTx is supported *on the guest side*.
> 
> I did not get that use case yet, but I could call it "host_pci_2_3" if
> preferred. Or provide a better name.
> 
> Jan
> 

Sounds good.

-- 
MST

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

* Re: [PATCH 2/4] pci-assign: Allow to disable MSI perference for host IRQ
  2010-11-02 14:55 ` [PATCH 2/4] pci-assign: Allow to disable MSI perference for host IRQ Jan Kiszka
@ 2010-11-03 12:54   ` Markus Armbruster
  0 siblings, 0 replies; 11+ messages in thread
From: Markus Armbruster @ 2010-11-03 12:54 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Avi Kivity, Marcelo Tosatti, kvm

Jan Kiszka <jan.kiszka@siemens.com> writes:

> Some devices (e.g. the ath9k) claim to support MSI but actually do not
> work when this is enabled. We must not blindly switch such devices to
> MSI but rather provide the user a way to pass control back to the
> guest driver. This can be done by turning the new property "prefer_msi"
> off (default remains on).
>
> Signed-off-by: Jan Kiszka <jan.kiszka@siemens.com>
> ---
>  hw/device-assignment.c |    5 ++++-
>  hw/device-assignment.h |    2 ++
>  2 files changed, 6 insertions(+), 1 deletions(-)
>
> diff --git a/hw/device-assignment.c b/hw/device-assignment.c
> index 349e864..73d8afd 100644
> --- a/hw/device-assignment.c
> +++ b/hw/device-assignment.c
> @@ -964,7 +964,8 @@ static int assign_irq(AssignedDevice *dev)
>      }
>  
>      assigned_irq_data.flags = KVM_DEV_IRQ_GUEST_INTX;
> -    if (dev->cap.available & ASSIGNED_DEVICE_CAP_MSI)
> +    if (dev->features & ASSIGNED_DEVICE_PREFER_MSI_MASK &&
> +        dev->cap.available & ASSIGNED_DEVICE_CAP_MSI)
>          assigned_irq_data.flags |= KVM_DEV_IRQ_HOST_MSI;
>      else
>          assigned_irq_data.flags |= KVM_DEV_IRQ_HOST_INTX;
> @@ -1541,6 +1542,8 @@ static PCIDeviceInfo assign_info = {
>          DEFINE_PROP_BIT("iommu", AssignedDevice, features,
>                          ASSIGNED_DEVICE_USE_IOMMU_BIT, true),
>          DEFINE_PROP_STRING("configfd", AssignedDevice, configfd_name),
> +        DEFINE_PROP_BIT("prefer_msi", AssignedDevice, features,
> +                        ASSIGNED_DEVICE_PREFER_MSI_BIT, true),

Suggest to define the feature bit properties next to each other.

>          DEFINE_PROP_END_OF_LIST(),
>      },
>  };
[...]

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

* Re: [PATCH 4/4] pci-assign: Remove broken -pcidevice and pci_add host
  2010-11-02 14:55 ` [PATCH 4/4] pci-assign: Remove broken -pcidevice and pci_add host Jan Kiszka
@ 2010-11-03 13:02   ` Markus Armbruster
  2010-11-03 13:12     ` Jan Kiszka
  0 siblings, 1 reply; 11+ messages in thread
From: Markus Armbruster @ 2010-11-03 13:02 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Avi Kivity, Marcelo Tosatti, kvm, Daniel P. Berrange

Jan Kiszka <jan.kiszka@siemens.com> writes:

> These qemu-kvm-only interfaces were broken by b560a9ab9b, but no one
> complained loud enough to get them fixed again. As we have properly
> working "-device pci-assign"/"device_add" and we won't push this
> upstream anyway, there is likely no point in restoring the interface.

Agree.  Dan wrote re libvirt:

    As of libvirt >= 0.8.1 & QEMU >= 0.12.x we use switched to using
    -device for everything. Older libvirt versions had rather broken
    checking for PCI device topology, so I think it is fine to require
    libvirt >= 0.8.1 for latest QEMU releases if users want PCI dev
    assignment. Thus -pcidevice and pci_add can both be killed from our
    POV.

Note that this patch keeps pci_add nic|storage around.  Unlikely to make
it upstream.

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

* Re: [PATCH 4/4] pci-assign: Remove broken -pcidevice and pci_add host
  2010-11-03 13:02   ` Markus Armbruster
@ 2010-11-03 13:12     ` Jan Kiszka
  2010-11-03 15:22       ` Markus Armbruster
  0 siblings, 1 reply; 11+ messages in thread
From: Jan Kiszka @ 2010-11-03 13:12 UTC (permalink / raw)
  To: Markus Armbruster
  Cc: Avi Kivity, Marcelo Tosatti, kvm@vger.kernel.org,
	Daniel P. Berrange

Am 03.11.2010 14:02, Markus Armbruster wrote:
> Jan Kiszka <jan.kiszka@siemens.com> writes:
> 
>> These qemu-kvm-only interfaces were broken by b560a9ab9b, but no one
>> complained loud enough to get them fixed again. As we have properly
>> working "-device pci-assign"/"device_add" and we won't push this
>> upstream anyway, there is likely no point in restoring the interface.
> 
> Agree.  Dan wrote re libvirt:
> 
>     As of libvirt >= 0.8.1 & QEMU >= 0.12.x we use switched to using
>     -device for everything. Older libvirt versions had rather broken
>     checking for PCI device topology, so I think it is fine to require
>     libvirt >= 0.8.1 for latest QEMU releases if users want PCI dev
>     assignment. Thus -pcidevice and pci_add can both be killed from our
>     POV.
> 
> Note that this patch keeps pci_add nic|storage around.  Unlikely to make
> it upstream.

pci_add is already in upstream.

Jan

-- 
Siemens AG, Corporate Technology, CT T DE IT 1
Corporate Competence Center Embedded Linux

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

* Re: [PATCH 4/4] pci-assign: Remove broken -pcidevice and pci_add host
  2010-11-03 13:12     ` Jan Kiszka
@ 2010-11-03 15:22       ` Markus Armbruster
  0 siblings, 0 replies; 11+ messages in thread
From: Markus Armbruster @ 2010-11-03 15:22 UTC (permalink / raw)
  To: Jan Kiszka
  Cc: Avi Kivity, Marcelo Tosatti, kvm@vger.kernel.org,
	Daniel P. Berrange

Jan Kiszka <jan.kiszka@siemens.com> writes:

> Am 03.11.2010 14:02, Markus Armbruster wrote:
>> Jan Kiszka <jan.kiszka@siemens.com> writes:
>> 
>>> These qemu-kvm-only interfaces were broken by b560a9ab9b, but no one
>>> complained loud enough to get them fixed again. As we have properly
>>> working "-device pci-assign"/"device_add" and we won't push this
>>> upstream anyway, there is likely no point in restoring the interface.
>> 
>> Agree.  Dan wrote re libvirt:
>> 
>>     As of libvirt >= 0.8.1 & QEMU >= 0.12.x we use switched to using
>>     -device for everything. Older libvirt versions had rather broken
>>     checking for PCI device topology, so I think it is fine to require
>>     libvirt >= 0.8.1 for latest QEMU releases if users want PCI dev
>>     assignment. Thus -pcidevice and pci_add can both be killed from our
>>     POV.
>> 
>> Note that this patch keeps pci_add nic|storage around.  Unlikely to make
>> it upstream.
>
> pci_add is already in upstream.

You're right.  Can't think straight today.

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

* Re: [PATCH 0/4] qemu-kvm: Fixes and improvements for device assigment
  2010-11-02 14:55 [PATCH 0/4] qemu-kvm: Fixes and improvements for device assigment Jan Kiszka
                   ` (4 preceding siblings ...)
       [not found] ` <4CD119D6.2050602@web.de>
@ 2010-11-03 20:03 ` Marcelo Tosatti
  5 siblings, 0 replies; 11+ messages in thread
From: Marcelo Tosatti @ 2010-11-03 20:03 UTC (permalink / raw)
  To: Jan Kiszka; +Cc: Avi Kivity, kvm

On Tue, Nov 02, 2010 at 03:55:33PM +0100, Jan Kiszka wrote:
> Mostly usability fixes, but also a patch to allow using devices that
> have problems with MSIs.
> 
> Jan Kiszka (4):
>   pci-assign: Convert iommu property to booleam
>   pci-assign: Allow to disable MSI perference for host IRQ
>   pci-assign: Issue warning when running w/o IOMMU
>   pci-assign: Remove broken -pcidevice and pci_add host
> 
>  hmp-commands.hx        |    2 +-
>  hw/device-assignment.c |   81 ++++++++---------------------------------------
>  hw/device-assignment.h |   15 ++++-----
>  hw/ipf.c               |    6 ---
>  hw/pc.c                |    6 ---
>  hw/pci-hotplug.c       |   22 -------------
>  qemu-options.hx        |    5 ---
>  vl.c                   |   14 --------
>  8 files changed, 22 insertions(+), 129 deletions(-)

Applied, thanks.


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

end of thread, other threads:[~2010-11-03 22:14 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-11-02 14:55 [PATCH 0/4] qemu-kvm: Fixes and improvements for device assigment Jan Kiszka
2010-11-02 14:55 ` [PATCH 1/4] pci-assign: Convert iommu property to booleam Jan Kiszka
2010-11-02 14:55 ` [PATCH 2/4] pci-assign: Allow to disable MSI perference for host IRQ Jan Kiszka
2010-11-03 12:54   ` Markus Armbruster
2010-11-02 14:55 ` [PATCH 3/4] pci-assign: Issue warning when running w/o IOMMU Jan Kiszka
2010-11-02 14:55 ` [PATCH 4/4] pci-assign: Remove broken -pcidevice and pci_add host Jan Kiszka
2010-11-03 13:02   ` Markus Armbruster
2010-11-03 13:12     ` Jan Kiszka
2010-11-03 15:22       ` Markus Armbruster
     [not found] ` <4CD119D6.2050602@web.de>
     [not found]   ` <20101103083321.GE6772@redhat.com>
     [not found]     ` <4CD120E9.7050306@web.de>
2010-11-03  9:11       ` [PATCH 5/4] pci-assign: Use PCI-2.3-based shared legacy interrupts by default Michael S. Tsirkin
2010-11-03 20:03 ` [PATCH 0/4] qemu-kvm: Fixes and improvements for device assigment Marcelo Tosatti

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).