[Qemu-devel] [PATCH 0/3] virtio-pci: 1.0 tweaks

[Qemu-devel] [PATCH 0/3] virtio-pci: 1.0 tweaks

[Gerd Hoffmann]

  Hi,

Continued to look at virtio-1.0.  Added flags to turn off legacy/modern
mode for virtio devices.

Figured notification seems to not be adapted to virtio 1.0 yet:  Apply
this patchset, boot with -device virtio-net-pci,disable-legacy=on, watch
qemu blow up in virtio_pci_set_host_notifier_internal, because it uses
uninitialited proxy->bar without checking.  Looked closer, seems the
virtio 1.0 notification memory region isn't wired up yet ...

Gerd Hoffmann (3):
  virtio-pci: add flags to enable/disable legacy/modern
  virtio-pci: use bar0 for non-transitional devices
  virtio-pci: make QEMU_VIRTIO_PCI_QUEUE_MEM_MULT smaller

 hw/virtio/virtio-pci.c | 60 ++++++++++++++++++++++++++++++++++++--------------
 hw/virtio/virtio-pci.h |  6 +++++
 2 files changed, 49 insertions(+), 17 deletions(-)

-- 
1.8.3.1

[Qemu-devel] [PATCH 1/3] virtio-pci: add flags to enable/disable legacy/modern

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/virtio/virtio-pci.c | 46 +++++++++++++++++++++++++++++++++-------------
 hw/virtio/virtio-pci.h |  6 ++++++
 2 files changed, 39 insertions(+), 13 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index a6ce3fe..407063c 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1234,6 +1234,8 @@ static void virtio_pci_device_plugged(DeviceState *d)
 {
     VirtIOPCIProxy *proxy = VIRTIO_PCI(d);
     VirtioBusState *bus = &proxy->bus;
+    bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY);
+    bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN);
     uint8_t *config;
     uint32_t size;
 
@@ -1241,13 +1243,24 @@ static void virtio_pci_device_plugged(DeviceState *d)
     if (proxy->class_code) {
         pci_config_set_class(config, proxy->class_code);
     }
-    pci_set_word(config + PCI_SUBSYSTEM_VENDOR_ID,
-                 pci_get_word(config + PCI_VENDOR_ID));
-    pci_set_word(config + PCI_SUBSYSTEM_ID, virtio_bus_get_vdev_id(bus));
+
+    if (legacy) {
+        /* legacy and transitional */
+        pci_set_word(config + PCI_SUBSYSTEM_VENDOR_ID,
+                     pci_get_word(config + PCI_VENDOR_ID));
+        pci_set_word(config + PCI_SUBSYSTEM_ID, virtio_bus_get_vdev_id(bus));
+    } else {
+        /* pure virtio-1.0 */
+        pci_set_word(config + PCI_VENDOR_ID,
+                     PCI_VENDOR_ID_REDHAT_QUMRANET);
+        pci_set_word(config + PCI_DEVICE_ID,
+                     0x1040 + virtio_bus_get_vdev_id(bus));
+        pci_config_set_revision(config, 1);
+    }
     config[PCI_INTERRUPT_PIN] = 1;
 
 
-    if (1) { /* TODO: Make this optional, dependent on virtio 1.0 */
+    if (modern) {
         struct virtio_pci_cap common = {
             .cfg_type = VIRTIO_PCI_CAP_COMMON_CFG,
             .cap_len = sizeof common,
@@ -1360,17 +1373,20 @@ static void virtio_pci_device_plugged(DeviceState *d)
 
     proxy->pci_dev.config_write = virtio_write_config;
 
-    size = VIRTIO_PCI_REGION_SIZE(&proxy->pci_dev)
-         + virtio_bus_get_vdev_config_len(bus);
-    if (size & (size - 1)) {
-        size = 1 << qemu_fls(size);
-    }
+    if (legacy) {
+        size = VIRTIO_PCI_REGION_SIZE(&proxy->pci_dev)
+            + virtio_bus_get_vdev_config_len(bus);
+        if (size & (size - 1)) {
+            size = 1 << qemu_fls(size);
+        }
 
-    memory_region_init_io(&proxy->bar, OBJECT(proxy), &virtio_pci_config_ops,
-                          proxy, "virtio-pci", size);
+        memory_region_init_io(&proxy->bar, OBJECT(proxy),
+                              &virtio_pci_config_ops,
+                              proxy, "virtio-pci", size);
 
-    pci_register_bar(&proxy->pci_dev, 0, PCI_BASE_ADDRESS_SPACE_IO,
-                     &proxy->bar);
+        pci_register_bar(&proxy->pci_dev, 0, PCI_BASE_ADDRESS_SPACE_IO,
+                         &proxy->bar);
+    }
 
     if (!kvm_has_many_ioeventfds()) {
         proxy->flags &= ~VIRTIO_PCI_FLAG_USE_IOEVENTFD;
@@ -1417,6 +1433,10 @@ static void virtio_pci_reset(DeviceState *qdev)
 static Property virtio_pci_properties[] = {
     DEFINE_PROP_BIT("virtio-pci-bus-master-bug-migration", VirtIOPCIProxy, flags,
                     VIRTIO_PCI_FLAG_BUS_MASTER_BUG_MIGRATION_BIT, false),
+    DEFINE_PROP_BIT("disable-legacy", VirtIOPCIProxy, flags,
+                    VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT, false),
+    DEFINE_PROP_BIT("disable-modern", VirtIOPCIProxy, flags,
+                    VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT, false),
     DEFINE_VIRTIO_COMMON_FEATURES(VirtIOPCIProxy, host_features),
     DEFINE_PROP_END_OF_LIST(),
 };
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 271a73a..938eeff 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -68,6 +68,12 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
 #define VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT 1
 #define VIRTIO_PCI_FLAG_USE_IOEVENTFD   (1 << VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT)
 
+/* virtio version flags */
+#define VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT 2
+#define VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT 3
+#define VIRTIO_PCI_FLAG_DISABLE_LEGACY (1 << VIRTIO_PCI_FLAG_DISABLE_LEGACY_BIT)
+#define VIRTIO_PCI_FLAG_DISABLE_MODERN (1 << VIRTIO_PCI_FLAG_DISABLE_MODERN_BIT)
+
 typedef struct {
     MSIMessage msg;
     int virq;
-- 
1.8.3.1

[Qemu-devel] [PATCH 2/3] virtio-pci: use bar0 for non-transitional devices

[Gerd Hoffmann]

---
 hw/virtio/virtio-pci.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 407063c..ca325fb 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -966,8 +966,6 @@ static void virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
     PCIDevice *dev = &proxy->pci_dev;
     int offset;
 
-    cap->bar = 2;
-
     offset = pci_add_capability(dev, PCI_CAP_ID_VNDR, 0, cap->cap_len);
     assert(offset > 0);
 
@@ -1236,6 +1234,7 @@ static void virtio_pci_device_plugged(DeviceState *d)
     VirtioBusState *bus = &proxy->bus;
     bool legacy = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_LEGACY);
     bool modern = !(proxy->flags & VIRTIO_PCI_FLAG_DISABLE_MODERN);
+    int modern_mem_bar;
     uint8_t *config;
     uint32_t size;
 
@@ -1249,6 +1248,7 @@ static void virtio_pci_device_plugged(DeviceState *d)
         pci_set_word(config + PCI_SUBSYSTEM_VENDOR_ID,
                      pci_get_word(config + PCI_VENDOR_ID));
         pci_set_word(config + PCI_SUBSYSTEM_ID, virtio_bus_get_vdev_id(bus));
+        modern_mem_bar = 2;
     } else {
         /* pure virtio-1.0 */
         pci_set_word(config + PCI_VENDOR_ID,
@@ -1256,6 +1256,7 @@ static void virtio_pci_device_plugged(DeviceState *d)
         pci_set_word(config + PCI_DEVICE_ID,
                      0x1040 + virtio_bus_get_vdev_id(bus));
         pci_config_set_revision(config, 1);
+        modern_mem_bar = 0;
     }
     config[PCI_INTERRUPT_PIN] = 1;
 
@@ -1264,24 +1265,28 @@ static void virtio_pci_device_plugged(DeviceState *d)
         struct virtio_pci_cap common = {
             .cfg_type = VIRTIO_PCI_CAP_COMMON_CFG,
             .cap_len = sizeof common,
+            .bar = modern_mem_bar,
             .offset = cpu_to_le32(0x0),
             .length = cpu_to_le32(0x1000),
         };
         struct virtio_pci_cap isr = {
             .cfg_type = VIRTIO_PCI_CAP_ISR_CFG,
             .cap_len = sizeof isr,
+            .bar = modern_mem_bar,
             .offset = cpu_to_le32(0x1000),
             .length = cpu_to_le32(0x1000),
         };
         struct virtio_pci_cap device = {
             .cfg_type = VIRTIO_PCI_CAP_DEVICE_CFG,
             .cap_len = sizeof device,
+            .bar = modern_mem_bar,
             .offset = cpu_to_le32(0x2000),
             .length = cpu_to_le32(0x1000),
         };
         struct virtio_pci_notify_cap notify = {
             .cap.cfg_type = VIRTIO_PCI_CAP_NOTIFY_CFG,
             .cap.cap_len = sizeof notify,
+            .cap.bar = modern_mem_bar,
             .cap.offset = cpu_to_le32(0x3000),
             .cap.length = cpu_to_le32(QEMU_VIRTIO_PCI_QUEUE_MEM_MULT *
                                       VIRTIO_PCI_QUEUE_MAX),
@@ -1360,7 +1365,8 @@ static void virtio_pci_device_plugged(DeviceState *d)
                               QEMU_VIRTIO_PCI_QUEUE_MEM_MULT *
                               VIRTIO_PCI_QUEUE_MAX);
         memory_region_add_subregion(&proxy->modern_bar, 0x3000, &proxy->notify);
-        pci_register_bar(&proxy->pci_dev, 2, PCI_BASE_ADDRESS_SPACE_MEMORY,
+        pci_register_bar(&proxy->pci_dev, modern_mem_bar,
+                         PCI_BASE_ADDRESS_SPACE_MEMORY,
                          &proxy->modern_bar);
     }
 
-- 
1.8.3.1

[Qemu-devel] [PATCH 3/3] virtio-pci: make QEMU_VIRTIO_PCI_QUEUE_MEM_MULT smaller

[Gerd Hoffmann]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/virtio/virtio-pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index ca325fb..01f8bb7 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -974,7 +974,7 @@ static void virtio_pci_add_mem_cap(VirtIOPCIProxy *proxy,
            cap->cap_len - PCI_CAP_FLAGS);
 }
 
-#define QEMU_VIRTIO_PCI_QUEUE_MEM_MULT 0x10000
+#define QEMU_VIRTIO_PCI_QUEUE_MEM_MULT 0x1000
 
 static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
                                        unsigned size)
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH 0/3] virtio-pci: 1.0 tweaks

[Gerd Hoffmann]

  Hi,

> Figured notification seems to not be adapted to virtio 1.0 yet:  Apply
> this patchset, boot with -device virtio-net-pci,disable-legacy=on, watch
> qemu blow up in virtio_pci_set_host_notifier_internal, because it uses
> uninitialited proxy->bar without checking.  Looked closer, seems the
> virtio 1.0 notification memory region isn't wired up yet ...

Oh, looking at the stacktrace again I see vhost in there, which IIRC
isn't ready yet.  Turned off vhost -- things are working nicely.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 0/3] virtio-pci: 1.0 tweaks

[Michael S. Tsirkin]

On Fri, Jan 23, 2015 at 09:31:29AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > Figured notification seems to not be adapted to virtio 1.0 yet:  Apply
> > this patchset, boot with -device virtio-net-pci,disable-legacy=on, watch
> > qemu blow up in virtio_pci_set_host_notifier_internal, because it uses
> > uninitialited proxy->bar without checking.  Looked closer, seems the
> > virtio 1.0 notification memory region isn't wired up yet ...
> 
> Oh, looking at the stacktrace again I see vhost in there, which IIRC
> isn't ready yet.  Turned off vhost -- things are working nicely.
> 
> cheers,
>   Gerd


yes - merely need to propage the new feature bit
in both directions.