[Qemu-devel] [PATCH v2 0/4] virtio-pci: a few proxy tweaks

[Qemu-devel] [PATCH v2 0/4] virtio-pci: a few proxy tweaks

[Gerd Hoffmann]

  Hi,

A few patches for virtio, applying cleanly on top of
mst's virtio-1.0 branch.

cheers,
  Gerd

Gerd Hoffmann (4):
  virtio-pci: add flags to enable/disable legacy/modern
  virtio-pci: make QEMU_VIRTIO_PCI_QUEUE_MEM_MULT smaller
  virtio-pci: make pci bars configurable
  virtio-pci: make modern bar 64bit prefetchable

 hw/virtio/virtio-pci.c | 81 ++++++++++++++++++++++++++++++++++++++------------
 hw/virtio/virtio-pci.h |  8 +++++
 2 files changed, 70 insertions(+), 19 deletions(-)

-- 
1.8.3.1

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

[Gerd Hoffmann]

Add VIRTIO_PCI_FLAG_DISABLE_LEGACY and VIRTIO_PCI_FLAG_DISABLE_MODERN
for VirtIOPCIProxy->flags.  Also add properties for them.  They can be
used to disable modern (virtio 1.0) or legacy (virtio 0.9) modes.  By
default both are advertized and the guest driver can choose.

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 4c9a0b8..6c0c650 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1233,6 +1233,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;
 
@@ -1240,13 +1242,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,
@@ -1359,17 +1372,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;
@@ -1416,6 +1432,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 2cddd6a..3068a63 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -63,6 +63,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

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

[Fam Zheng]

On Mon, 02/16 17:33, Gerd Hoffmann wrote:
> Add VIRTIO_PCI_FLAG_DISABLE_LEGACY and VIRTIO_PCI_FLAG_DISABLE_MODERN
> for VirtIOPCIProxy->flags.  Also add properties for them.  They can be
> used to disable modern (virtio 1.0) or legacy (virtio 0.9) modes.  By
> default both are advertized and the guest driver can choose.
> 
> 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 4c9a0b8..6c0c650 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1233,6 +1233,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;
>  
> @@ -1240,13 +1242,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);
> +    }

Should we forbid setting both legacy and modern at the same time?

Fam

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

[Gerd Hoffmann]

  Hi,

> Should we forbid setting both legacy and modern at the same time?

No, this is explicitly allowed in the spec and it is needed for the
transition from legacy to modern.

cheers,
  Gerd

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

[Michael S. Tsirkin]

On Tue, Feb 17, 2015 at 09:23:35AM +0100, Gerd Hoffmann wrote:
>   Hi,
> 
> > Should we forbid setting both legacy and modern at the same time?
> 
> No, this is explicitly allowed in the spec and it is needed for the
> transition from legacy to modern.
> 
> cheers,
>   Gerd

But !legacy && !modern aren't legal together, correct?

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

[Gerd Hoffmann]

On Di, 2015-02-17 at 09:42 +0100, Michael S. Tsirkin wrote:
> On Tue, Feb 17, 2015 at 09:23:35AM +0100, Gerd Hoffmann wrote:
> >   Hi,
> > 
> > > Should we forbid setting both legacy and modern at the same time?
> > 
> > No, this is explicitly allowed in the spec and it is needed for the
> > transition from legacy to modern.
> > 
> > cheers,
> >   Gerd
> 
> But !legacy && !modern aren't legal together, correct?

That doesn't make sense indeed.

cheers,
  Gerd

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

[Gerd Hoffmann]

On Di, 2015-02-17 at 10:03 +0100, Gerd Hoffmann wrote:
> On Di, 2015-02-17 at 09:42 +0100, Michael S. Tsirkin wrote:
> > On Tue, Feb 17, 2015 at 09:23:35AM +0100, Gerd Hoffmann wrote:
> > >   Hi,
> > > 
> > > > Should we forbid setting both legacy and modern at the same time?
> > > 
> > > No, this is explicitly allowed in the spec and it is needed for the
> > > transition from legacy to modern.
> > > 
> > > cheers,
> > >   Gerd
> > 
> > But !legacy && !modern aren't legal together, correct?
> 
> That doesn't make sense indeed.

What is the state of markus "convert pci to realize" patch series btw?  

Convert virtio-pci too, then throw an error in virtio_pci_realize would
be the best way to catch this I think.

cheers,
  Gerd

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

[Michael S. Tsirkin]

On Wed, Feb 18, 2015 at 11:32:55AM +0100, Gerd Hoffmann wrote:
> On Di, 2015-02-17 at 10:03 +0100, Gerd Hoffmann wrote:
> > On Di, 2015-02-17 at 09:42 +0100, Michael S. Tsirkin wrote:
> > > On Tue, Feb 17, 2015 at 09:23:35AM +0100, Gerd Hoffmann wrote:
> > > >   Hi,
> > > > 
> > > > > Should we forbid setting both legacy and modern at the same time?
> > > > 
> > > > No, this is explicitly allowed in the spec and it is needed for the
> > > > transition from legacy to modern.
> > > > 
> > > > cheers,
> > > >   Gerd
> > > 
> > > But !legacy && !modern aren't legal together, correct?
> > 
> > That doesn't make sense indeed.
> 
> What is the state of markus "convert pci to realize" patch series btw?  
> 
> Convert virtio-pci too, then throw an error in virtio_pci_realize would
> be the best way to catch this I think.
> 
> cheers,
>   Gerd
> 

It's on my tree, will be upstream soon.

[Qemu-devel] [PATCH v2 2/4] 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 6c0c650..cd7c777 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -975,7 +975,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

[Qemu-devel] [PATCH v2 3/4] virtio-pci: make pci bars configurable

[Gerd Hoffmann]

Add msix_bar and modern_mem_bar fields to VirtIOPCIProxy.  They can be
used to configure which pci regions are used for the virtio 1.0 memory
bar and the msix bar.

For legacy/transitional devices the legacy bar is region 0 and the msix
bar is region 1.  Only the modern bar can be configured, and it must be
2 or larger.  Default is 2.

For legacy-free devices the modern bar is region 0 by default and the
msix bar is 2 by default.

Use case: For VirtIOPCIProxy subclasses which need additional pci bars,
such as virtio-vga.  With the new fields they can make sure the regions
do not conflict.

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

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index cd7c777..f97baf2 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -965,8 +965,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);
 
@@ -1243,11 +1241,21 @@ static void virtio_pci_device_plugged(DeviceState *d)
         pci_config_set_class(config, proxy->class_code);
     }
 
+    if (proxy->modern_mem_bar > 5) {
+        proxy->modern_mem_bar = 5;
+    }
+    if (proxy->msix_bar > 5) {
+        proxy->msix_bar = 5;
+    }
     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));
+        proxy->msix_bar = 1;
+        if (proxy->modern_mem_bar < 2) {
+            proxy->modern_mem_bar = 2;
+        }
     } else {
         /* pure virtio-1.0 */
         pci_set_word(config + PCI_VENDOR_ID,
@@ -1255,6 +1263,9 @@ 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);
+        if (proxy->msix_bar == proxy->modern_mem_bar) {
+            proxy->msix_bar = (proxy->msix_bar + 2) % 6;
+        }
     }
     config[PCI_INTERRUPT_PIN] = 1;
 
@@ -1263,24 +1274,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 = proxy->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 = proxy->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 = proxy->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 = proxy->modern_mem_bar,
             .cap.offset = cpu_to_le32(0x3000),
             .cap.length = cpu_to_le32(QEMU_VIRTIO_PCI_QUEUE_MEM_MULT *
                                       VIRTIO_PCI_QUEUE_MAX),
@@ -1359,12 +1374,14 @@ 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, proxy->modern_mem_bar,
+                         PCI_BASE_ADDRESS_SPACE_MEMORY,
                          &proxy->modern_bar);
     }
 
     if (proxy->nvectors &&
-        msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 1)) {
+        msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors,
+                                proxy->msix_bar)) {
         error_report("unable to init msix vectors to %" PRIu32,
                      proxy->nvectors);
         proxy->nvectors = 0;
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 3068a63..a273c33 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -102,6 +102,8 @@ struct VirtIOPCIProxy {
     uint32_t flags;
     uint32_t class_code;
     uint32_t nvectors;
+    uint32_t msix_bar;
+    uint32_t modern_mem_bar;
     uint64_t host_features;
     uint32_t dfselect;
     uint32_t gfselect;
-- 
1.8.3.1

[Qemu-devel] [PATCH v2 4/4] virtio-pci: make modern bar 64bit prefetchable

[Gerd Hoffmann]

Modern bar is made prefetchable.  In case it is configured to use one of
the bars 0, 2, or 4 (which by default is the case) it is also configured
as 64bit region.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 f97baf2..96b3692 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1263,7 +1263,8 @@ 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);
-        if (proxy->msix_bar == proxy->modern_mem_bar) {
+        if ((proxy->msix_bar == proxy->modern_mem_bar) ||
+            (proxy->msix_bar == proxy->modern_mem_bar+1)) {
             proxy->msix_bar = (proxy->msix_bar + 2) % 6;
         }
     }
@@ -1301,6 +1302,7 @@ static void virtio_pci_device_plugged(DeviceState *d)
                                       VIRTIO_PCI_QUEUE_MAX),
             .notify_off_multiplier = cpu_to_le32(QEMU_VIRTIO_PCI_QUEUE_MEM_MULT),
         };
+        uint32_t modern_mem_attr;
 
         static const MemoryRegionOps common_ops = {
             .read = virtio_pci_common_read,
@@ -1374,9 +1376,13 @@ 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);
+        modern_mem_attr = (PCI_BASE_ADDRESS_SPACE_MEMORY |
+                           PCI_BASE_ADDRESS_MEM_PREFETCH);
+        if (!(proxy->modern_mem_bar % 2)) {
+            modern_mem_attr |= PCI_BASE_ADDRESS_MEM_TYPE_64;
+        }
         pci_register_bar(&proxy->pci_dev, proxy->modern_mem_bar,
-                         PCI_BASE_ADDRESS_SPACE_MEMORY,
-                         &proxy->modern_bar);
+                         modern_mem_attr, &proxy->modern_bar);
     }
 
     if (proxy->nvectors &&
-- 
1.8.3.1