[Qemu-devel] [PATCH V2 00/11] Support more virtio queues

[Qemu-devel] [PATCH V2 00/11] Support more virtio queues

[Jason Wang]

We current limit the max virtio queues to 64. This is not sufficient
to support some multiqueue deivces (e.g recent Linux support up to 256
tap queues). So this series try to let virtio to support more queues.

No much works needs to be done except:

- Patch 1 add a check to validate the queues supported by backend
  against the virtio queue limitation
- Patch 2 fixes a bug in virtio_del_queue()
- Patch 3 introdues a bus specific queue limitation
- Patch 4 - Patch 8 let each transport to use bus specific queue
  limitation instead of VIRTIO_PCI_QUEUE_MAX
- Patch 9 increase the maximum number of queues supported by
  virtio-pci from 64 to 513, and deal with the migration compatibility
  with the changes.
- Patch 10 tries to remove the hard coded MSI-X bar size (4096) and
  allow up to 2048 MSI-X entries.
- Patch 11 add a boolean property to let the virtio-net can calculate
  the MSI-X bar size based on the number of MSI-X vectors and keep
  migration compatibility with legacy version whose bar size is 4096.

With this patch, we can support up to 256 queues. Since x86 can only
allow about 240 interrupt vectors for MSI-X, current Linux driver can
only have about 80 queue pairs has their private MSI-X interrupt
vectors. With sharing IRQ with queue pairs (RFC posted in
https://lkml.org/lkml/2014/12/25/169), Linux driver can have up
to about 186 queue pairs has their private MSI-X interrupt vectors.

Stress/migration test on virtio-pci, compile test/make check on other
s390-softmmu.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Anthony Liguori <aliguori@amazon.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: Keith Busch <keith.busch@intel.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Anthony Liguori <aliguori@amazon.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Amit Shah <amit.shah@redhat.com>

Please review

Thanks

Changes from V1:
- add a validation against the bus limitation
- switch to use a bus specific queue limit instead of a global one,
  this will allow us to just increase the limit of one transport
  without disturbing others.
- only increase the queue limit of virtio-pci
- limit the maximum number of virtio queues to 64 for legacy machine
  types

Jason Wang (11):
  virtio-net: validate backend queue numbers against bus limitation
  virtio-net: fix the upper bound when trying to delete queues
  virito: introduce bus specific queue limit
  virtio-ccw: introduce ccw specific queue limit
  virtio-serial-bus: switch to bus specific queue limit
  virtio-s390: switch to bus specific queue limit
  virtio-mmio: switch to bus specific queue limit
  virtio-pci: switch to use bus specific queue limit
  virtio-pci: increase the maximum number of virtqueues to 513
  pci: remove hard-coded bar size in msix_init_exclusive_bar()
  virtio-pci: introduce auto_msix_bar_size property

 hw/block/nvme.c                |  2 +-
 hw/char/virtio-serial-bus.c    |  4 +++-
 hw/i386/pc_piix.c              | 10 ++++++++
 hw/i386/pc_q35.c               |  9 ++++++++
 hw/misc/ivshmem.c              |  2 +-
 hw/net/virtio-net.c            |  9 +++++++-
 hw/pci/msix.c                  | 18 ++++++---------
 hw/ppc/spapr.c                 |  5 ++++
 hw/s390x/s390-virtio-bus.c     |  5 ++--
 hw/s390x/s390-virtio-ccw.c     |  7 ++++--
 hw/s390x/virtio-ccw.c          | 12 ++++++----
 hw/scsi/virtio-scsi.c          |  4 ++--
 hw/virtio/virtio-mmio.c        |  5 ++--
 hw/virtio/virtio-pci.c         | 28 +++++++++++++++++------
 hw/virtio/virtio-pci.h         |  3 +++
 hw/virtio/virtio.c             | 52 +++++++++++++++++++++++++++++++-----------
 include/hw/compat.h            |  8 +++++++
 include/hw/pci/msix.h          |  2 +-
 include/hw/s390x/s390_flic.h   |  2 +-
 include/hw/virtio/virtio-bus.h |  1 +
 include/hw/virtio/virtio.h     |  6 ++++-
 21 files changed, 143 insertions(+), 51 deletions(-)

-- 
1.9.1

[Qemu-devel] [PATCH V2 01/11] virtio-net: validate backend queue numbers against bus limitation

[Jason Wang]

We don't validate the backend queue numbers against bus limitation,
this will easily crash qemu if it exceeds the limitation. Fixing this
by doing the validation and fail early.

Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/virtio-net.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index 45da34a..b4ac2b3 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1585,6 +1585,13 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
     virtio_init(vdev, "virtio-net", VIRTIO_ID_NET, n->config_size);
 
     n->max_queues = MAX(n->nic_conf.peers.queues, 1);
+    if (n->max_queues * 2 + 1 > VIRTIO_PCI_QUEUE_MAX) {
+        error_setg(errp, "Invalid number of queues (= %" PRIu32 "), "
+                   "must be a postive integer less than %d.",
+                   n->max_queues, (VIRTIO_PCI_QUEUE_MAX - 1) / 2);
+        virtio_cleanup(vdev);
+        return;
+    }
     n->vqs = g_malloc0(sizeof(VirtIONetQueue) * n->max_queues);
     n->vqs[0].rx_vq = virtio_add_queue(vdev, 256, virtio_net_handle_rx);
     n->curr_queues = 1;
-- 
1.9.1

[Qemu-devel] [PATCH V2 02/11] virtio-net: fix the upper bound when trying to delete queues

[Jason Wang]

Virtqueue were indexed from zero, so don't delete virtqueue whose
index is n->max_queues * 2 + 1.

Cc: Anthony Liguori <aliguori@amazon.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/virtio-net.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index b4ac2b3..c8d2cca 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1306,7 +1306,7 @@ static void virtio_net_set_multiqueue(VirtIONet *n, int multiqueue)
 
     n->multiqueue = multiqueue;
 
-    for (i = 2; i <= n->max_queues * 2 + 1; i++) {
+    for (i = 2; i < n->max_queues * 2 + 1; i++) {
         virtio_del_queue(vdev, i);
     }
 
-- 
1.9.1

[Qemu-devel] [PATCH V2 03/11] virito: introduce bus specific queue limit

[Jason Wang]

This patch introduces a bus specific queue limitation. It will be
useful for increasing the limit for one of the bus without disturbing
other buses.

Cc: Anthony Liguori <aliguori@amazon.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/net/virtio-net.c            |  4 ++--
 hw/s390x/s390-virtio-bus.c     |  1 +
 hw/s390x/virtio-ccw.c          |  1 +
 hw/scsi/virtio-scsi.c          |  4 ++--
 hw/virtio/virtio-mmio.c        |  1 +
 hw/virtio/virtio-pci.c         |  1 +
 hw/virtio/virtio.c             | 52 +++++++++++++++++++++++++++++++-----------
 include/hw/virtio/virtio-bus.h |  1 +
 include/hw/virtio/virtio.h     |  1 +
 9 files changed, 49 insertions(+), 17 deletions(-)

diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
index c8d2cca..260345c 100644
--- a/hw/net/virtio-net.c
+++ b/hw/net/virtio-net.c
@@ -1585,10 +1585,10 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
     virtio_init(vdev, "virtio-net", VIRTIO_ID_NET, n->config_size);
 
     n->max_queues = MAX(n->nic_conf.peers.queues, 1);
-    if (n->max_queues * 2 + 1 > VIRTIO_PCI_QUEUE_MAX) {
+    if (n->max_queues * 2 + 1 > virtio_get_queue_max(vdev)) {
         error_setg(errp, "Invalid number of queues (= %" PRIu32 "), "
                    "must be a postive integer less than %d.",
-                   n->max_queues, (VIRTIO_PCI_QUEUE_MAX - 1) / 2);
+                   n->max_queues, (virtio_get_queue_max(vdev) - 1) / 2);
         virtio_cleanup(vdev);
         return;
     }
diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index 39dc201..d48590a 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -707,6 +707,7 @@ static void virtio_s390_bus_class_init(ObjectClass *klass, void *data)
     bus_class->max_dev = 1;
     k->notify = virtio_s390_notify;
     k->get_features = virtio_s390_get_features;
+    k->queue_max = VIRTIO_PCI_QUEUE_MAX;
 }
 
 static const TypeInfo virtio_s390_bus_info = {
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index ea236c9..4874622 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -1695,6 +1695,7 @@ static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data)
     k->load_queue = virtio_ccw_load_queue;
     k->save_config = virtio_ccw_save_config;
     k->load_config = virtio_ccw_load_config;
+    k->queue_max = VIRTIO_PCI_QUEUE_MAX;
 }
 
 static const TypeInfo virtio_ccw_bus_info = {
diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
index 9e2c718..91d49f1 100644
--- a/hw/scsi/virtio-scsi.c
+++ b/hw/scsi/virtio-scsi.c
@@ -822,10 +822,10 @@ void virtio_scsi_common_realize(DeviceState *dev, Error **errp,
                 sizeof(VirtIOSCSIConfig));
 
     if (s->conf.num_queues == 0 ||
-            s->conf.num_queues > VIRTIO_PCI_QUEUE_MAX - 2) {
+            s->conf.num_queues > virtio_get_queue_max(vdev) - 2) {
         error_setg(errp, "Invalid number of queues (= %" PRIu32 "), "
                          "must be a positive integer less than %d.",
-                   s->conf.num_queues, VIRTIO_PCI_QUEUE_MAX - 2);
+                   s->conf.num_queues, virtio_get_queue_max(vdev) - 2);
         virtio_cleanup(vdev);
         return;
     }
diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index 2450c13..ad03218 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -403,6 +403,7 @@ static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data)
     k->device_plugged = virtio_mmio_device_plugged;
     k->has_variable_vring_alignment = true;
     bus_class->max_dev = 1;
+    k->queue_max = VIRTIO_PCI_QUEUE_MAX;
 }
 
 static const TypeInfo virtio_mmio_bus_info = {
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index dde1d73..7fa8141 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -1559,6 +1559,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
     k->vmstate_change = virtio_pci_vmstate_change;
     k->device_plugged = virtio_pci_device_plugged;
     k->device_unplugged = virtio_pci_device_unplugged;
+    k->queue_max = VIRTIO_PCI_QUEUE_MAX;
 }
 
 static const TypeInfo virtio_pci_bus_info = {
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
index ffc22e8..5a806b5 100644
--- a/hw/virtio/virtio.c
+++ b/hw/virtio/virtio.c
@@ -541,6 +541,14 @@ void virtio_update_irq(VirtIODevice *vdev)
     virtio_notify_vector(vdev, VIRTIO_NO_VECTOR);
 }
 
+int virtio_get_queue_max(VirtIODevice *vdev)
+{
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+
+    return k->queue_max;
+}
+
 void virtio_set_status(VirtIODevice *vdev, uint8_t val)
 {
     VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
@@ -576,6 +584,8 @@ static enum virtio_device_endian virtio_current_cpu_endian(void)
 void virtio_reset(void *opaque)
 {
     VirtIODevice *vdev = opaque;
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusClass *bk = VIRTIO_BUS_GET_CLASS(qbus);
     VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
     int i;
 
@@ -599,7 +609,7 @@ void virtio_reset(void *opaque)
     vdev->config_vector = VIRTIO_NO_VECTOR;
     virtio_notify_vector(vdev, vdev->config_vector);
 
-    for(i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) {
+    for(i = 0; i < bk->queue_max; i++) {
         vdev->vq[i].vring.desc = 0;
         vdev->vq[i].vring.avail = 0;
         vdev->vq[i].vring.used = 0;
@@ -738,7 +748,10 @@ int virtio_queue_get_num(VirtIODevice *vdev, int n)
 int virtio_queue_get_id(VirtQueue *vq)
 {
     VirtIODevice *vdev = vq->vdev;
-    assert(vq >= &vdev->vq[0] && vq < &vdev->vq[VIRTIO_PCI_QUEUE_MAX]);
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+
+    assert(vq >= &vdev->vq[0] && vq < &vdev->vq[k->queue_max]);
     return vq - &vdev->vq[0];
 }
 
@@ -777,27 +790,35 @@ void virtio_queue_notify(VirtIODevice *vdev, int n)
 
 uint16_t virtio_queue_vector(VirtIODevice *vdev, int n)
 {
-    return n < VIRTIO_PCI_QUEUE_MAX ? vdev->vq[n].vector :
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+
+    return n < k->queue_max ? vdev->vq[n].vector :
         VIRTIO_NO_VECTOR;
 }
 
 void virtio_queue_set_vector(VirtIODevice *vdev, int n, uint16_t vector)
 {
-    if (n < VIRTIO_PCI_QUEUE_MAX)
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+
+    if (n < k->queue_max)
         vdev->vq[n].vector = vector;
 }
 
 VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
                             void (*handle_output)(VirtIODevice *, VirtQueue *))
 {
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
     int i;
 
-    for (i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) {
+    for (i = 0; i < k->queue_max; i++) {
         if (vdev->vq[i].vring.num == 0)
             break;
     }
 
-    if (i == VIRTIO_PCI_QUEUE_MAX || queue_size > VIRTQUEUE_MAX_SIZE)
+    if (i == k->queue_max || queue_size > VIRTQUEUE_MAX_SIZE)
         abort();
 
     vdev->vq[i].vring.num = queue_size;
@@ -809,7 +830,10 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
 
 void virtio_del_queue(VirtIODevice *vdev, int n)
 {
-    if (n < 0 || n >= VIRTIO_PCI_QUEUE_MAX) {
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
+
+    if (n < 0 || n >= k->queue_max) {
         abort();
     }
 
@@ -932,14 +956,14 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
     qemu_put_be32(f, vdev->config_len);
     qemu_put_buffer(f, vdev->config, vdev->config_len);
 
-    for (i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) {
+    for (i = 0; i < k->queue_max; i++) {
         if (vdev->vq[i].vring.num == 0)
             break;
     }
 
     qemu_put_be32(f, i);
 
-    for (i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) {
+    for (i = 0; i < k->queue_max; i++) {
         if (vdev->vq[i].vring.num == 0)
             break;
 
@@ -1004,7 +1028,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
     qemu_get_8s(f, &vdev->status);
     qemu_get_8s(f, &vdev->isr);
     qemu_get_be16s(f, &vdev->queue_sel);
-    if (vdev->queue_sel >= VIRTIO_PCI_QUEUE_MAX) {
+    if (vdev->queue_sel >= k->queue_max) {
         return -1;
     }
     qemu_get_be32s(f, &features);
@@ -1031,7 +1055,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
 
     num = qemu_get_be32(f);
 
-    if (num > VIRTIO_PCI_QUEUE_MAX) {
+    if (num > k->queue_max) {
         error_report("Invalid number of PCI queues: 0x%x", num);
         return -1;
     }
@@ -1141,15 +1165,17 @@ void virtio_instance_init_common(Object *proxy_obj, void *data,
 void virtio_init(VirtIODevice *vdev, const char *name,
                  uint16_t device_id, size_t config_size)
 {
+    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
+    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
     int i;
     vdev->device_id = device_id;
     vdev->status = 0;
     vdev->isr = 0;
     vdev->queue_sel = 0;
     vdev->config_vector = VIRTIO_NO_VECTOR;
-    vdev->vq = g_malloc0(sizeof(VirtQueue) * VIRTIO_PCI_QUEUE_MAX);
+    vdev->vq = g_malloc0(sizeof(VirtQueue) * k->queue_max);
     vdev->vm_running = runstate_is_running();
-    for (i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) {
+    for (i = 0; i < k->queue_max; i++) {
         vdev->vq[i].vector = VIRTIO_NO_VECTOR;
         vdev->vq[i].vdev = vdev;
         vdev->vq[i].queue_index = i;
diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
index 0756545..979835c 100644
--- a/include/hw/virtio/virtio-bus.h
+++ b/include/hw/virtio/virtio-bus.h
@@ -68,6 +68,7 @@ typedef struct VirtioBusClass {
      * Note that changing this will break migration for this transport.
      */
     bool has_variable_vring_alignment;
+    uint16_t queue_max;
 } VirtioBusClass;
 
 struct VirtioBusState {
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index f24997d..9fe0d92 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -223,6 +223,7 @@ void virtio_queue_notify(VirtIODevice *vdev, int n);
 uint16_t virtio_queue_vector(VirtIODevice *vdev, int n);
 void virtio_queue_set_vector(VirtIODevice *vdev, int n, uint16_t vector);
 void virtio_set_status(VirtIODevice *vdev, uint8_t val);
+int virtio_get_queue_max(VirtIODevice *vdev);
 void virtio_reset(void *opaque);
 void virtio_update_irq(VirtIODevice *vdev);
 int virtio_set_features(VirtIODevice *vdev, uint32_t val);
-- 
1.9.1

Re: [Qemu-devel] [PATCH V2 03/11] virito: introduce bus specific queue limit

[Michael S. Tsirkin]

On Thu, Feb 26, 2015 at 03:04:38PM +0800, Jason Wang wrote:
> This patch introduces a bus specific queue limitation. It will be
> useful for increasing the limit for one of the bus without disturbing
> other buses.

Is this about s390 only supporting up to 64 queues?


> 
> Cc: Anthony Liguori <aliguori@amazon.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  hw/net/virtio-net.c            |  4 ++--
>  hw/s390x/s390-virtio-bus.c     |  1 +
>  hw/s390x/virtio-ccw.c          |  1 +
>  hw/scsi/virtio-scsi.c          |  4 ++--
>  hw/virtio/virtio-mmio.c        |  1 +
>  hw/virtio/virtio-pci.c         |  1 +
>  hw/virtio/virtio.c             | 52 +++++++++++++++++++++++++++++++-----------
>  include/hw/virtio/virtio-bus.h |  1 +
>  include/hw/virtio/virtio.h     |  1 +
>  9 files changed, 49 insertions(+), 17 deletions(-)
> 
> diff --git a/hw/net/virtio-net.c b/hw/net/virtio-net.c
> index c8d2cca..260345c 100644
> --- a/hw/net/virtio-net.c
> +++ b/hw/net/virtio-net.c
> @@ -1585,10 +1585,10 @@ static void virtio_net_device_realize(DeviceState *dev, Error **errp)
>      virtio_init(vdev, "virtio-net", VIRTIO_ID_NET, n->config_size);
>  
>      n->max_queues = MAX(n->nic_conf.peers.queues, 1);
> -    if (n->max_queues * 2 + 1 > VIRTIO_PCI_QUEUE_MAX) {
> +    if (n->max_queues * 2 + 1 > virtio_get_queue_max(vdev)) {
>          error_setg(errp, "Invalid number of queues (= %" PRIu32 "), "
>                     "must be a postive integer less than %d.",
> -                   n->max_queues, (VIRTIO_PCI_QUEUE_MAX - 1) / 2);
> +                   n->max_queues, (virtio_get_queue_max(vdev) - 1) / 2);
>          virtio_cleanup(vdev);
>          return;
>      }
> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
> index 39dc201..d48590a 100644
> --- a/hw/s390x/s390-virtio-bus.c
> +++ b/hw/s390x/s390-virtio-bus.c
> @@ -707,6 +707,7 @@ static void virtio_s390_bus_class_init(ObjectClass *klass, void *data)
>      bus_class->max_dev = 1;
>      k->notify = virtio_s390_notify;
>      k->get_features = virtio_s390_get_features;
> +    k->queue_max = VIRTIO_PCI_QUEUE_MAX;
>  }
>  
>  static const TypeInfo virtio_s390_bus_info = {
> diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
> index ea236c9..4874622 100644
> --- a/hw/s390x/virtio-ccw.c
> +++ b/hw/s390x/virtio-ccw.c
> @@ -1695,6 +1695,7 @@ static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data)
>      k->load_queue = virtio_ccw_load_queue;
>      k->save_config = virtio_ccw_save_config;
>      k->load_config = virtio_ccw_load_config;
> +    k->queue_max = VIRTIO_PCI_QUEUE_MAX;
>  }
>  
>  static const TypeInfo virtio_ccw_bus_info = {
> diff --git a/hw/scsi/virtio-scsi.c b/hw/scsi/virtio-scsi.c
> index 9e2c718..91d49f1 100644
> --- a/hw/scsi/virtio-scsi.c
> +++ b/hw/scsi/virtio-scsi.c
> @@ -822,10 +822,10 @@ void virtio_scsi_common_realize(DeviceState *dev, Error **errp,
>                  sizeof(VirtIOSCSIConfig));
>  
>      if (s->conf.num_queues == 0 ||
> -            s->conf.num_queues > VIRTIO_PCI_QUEUE_MAX - 2) {
> +            s->conf.num_queues > virtio_get_queue_max(vdev) - 2) {
>          error_setg(errp, "Invalid number of queues (= %" PRIu32 "), "
>                           "must be a positive integer less than %d.",
> -                   s->conf.num_queues, VIRTIO_PCI_QUEUE_MAX - 2);
> +                   s->conf.num_queues, virtio_get_queue_max(vdev) - 2);
>          virtio_cleanup(vdev);
>          return;
>      }
> diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
> index 2450c13..ad03218 100644
> --- a/hw/virtio/virtio-mmio.c
> +++ b/hw/virtio/virtio-mmio.c
> @@ -403,6 +403,7 @@ static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data)
>      k->device_plugged = virtio_mmio_device_plugged;
>      k->has_variable_vring_alignment = true;
>      bus_class->max_dev = 1;
> +    k->queue_max = VIRTIO_PCI_QUEUE_MAX;
>  }
>  
>  static const TypeInfo virtio_mmio_bus_info = {
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index dde1d73..7fa8141 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1559,6 +1559,7 @@ static void virtio_pci_bus_class_init(ObjectClass *klass, void *data)
>      k->vmstate_change = virtio_pci_vmstate_change;
>      k->device_plugged = virtio_pci_device_plugged;
>      k->device_unplugged = virtio_pci_device_unplugged;
> +    k->queue_max = VIRTIO_PCI_QUEUE_MAX;
>  }
>  
>  static const TypeInfo virtio_pci_bus_info = {
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index ffc22e8..5a806b5 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -541,6 +541,14 @@ void virtio_update_irq(VirtIODevice *vdev)
>      virtio_notify_vector(vdev, VIRTIO_NO_VECTOR);
>  }
>  
> +int virtio_get_queue_max(VirtIODevice *vdev)
> +{
> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +
> +    return k->queue_max;
> +}
> +
>  void virtio_set_status(VirtIODevice *vdev, uint8_t val)
>  {
>      VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> @@ -576,6 +584,8 @@ static enum virtio_device_endian virtio_current_cpu_endian(void)
>  void virtio_reset(void *opaque)
>  {
>      VirtIODevice *vdev = opaque;
> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> +    VirtioBusClass *bk = VIRTIO_BUS_GET_CLASS(qbus);
>      VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
>      int i;
>  
> @@ -599,7 +609,7 @@ void virtio_reset(void *opaque)
>      vdev->config_vector = VIRTIO_NO_VECTOR;
>      virtio_notify_vector(vdev, vdev->config_vector);
>  
> -    for(i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) {
> +    for(i = 0; i < bk->queue_max; i++) {
>          vdev->vq[i].vring.desc = 0;
>          vdev->vq[i].vring.avail = 0;
>          vdev->vq[i].vring.used = 0;
> @@ -738,7 +748,10 @@ int virtio_queue_get_num(VirtIODevice *vdev, int n)
>  int virtio_queue_get_id(VirtQueue *vq)
>  {
>      VirtIODevice *vdev = vq->vdev;
> -    assert(vq >= &vdev->vq[0] && vq < &vdev->vq[VIRTIO_PCI_QUEUE_MAX]);
> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +
> +    assert(vq >= &vdev->vq[0] && vq < &vdev->vq[k->queue_max]);
>      return vq - &vdev->vq[0];
>  }
>  
> @@ -777,27 +790,35 @@ void virtio_queue_notify(VirtIODevice *vdev, int n)
>  
>  uint16_t virtio_queue_vector(VirtIODevice *vdev, int n)
>  {
> -    return n < VIRTIO_PCI_QUEUE_MAX ? vdev->vq[n].vector :
> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +
> +    return n < k->queue_max ? vdev->vq[n].vector :
>          VIRTIO_NO_VECTOR;
>  }
>  
>  void virtio_queue_set_vector(VirtIODevice *vdev, int n, uint16_t vector)
>  {
> -    if (n < VIRTIO_PCI_QUEUE_MAX)
> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +
> +    if (n < k->queue_max)
>          vdev->vq[n].vector = vector;
>  }
>  
>  VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
>                              void (*handle_output)(VirtIODevice *, VirtQueue *))
>  {
> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>      int i;
>  
> -    for (i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) {
> +    for (i = 0; i < k->queue_max; i++) {
>          if (vdev->vq[i].vring.num == 0)
>              break;
>      }
>  
> -    if (i == VIRTIO_PCI_QUEUE_MAX || queue_size > VIRTQUEUE_MAX_SIZE)
> +    if (i == k->queue_max || queue_size > VIRTQUEUE_MAX_SIZE)
>          abort();
>  
>      vdev->vq[i].vring.num = queue_size;
> @@ -809,7 +830,10 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size,
>  
>  void virtio_del_queue(VirtIODevice *vdev, int n)
>  {
> -    if (n < 0 || n >= VIRTIO_PCI_QUEUE_MAX) {
> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +
> +    if (n < 0 || n >= k->queue_max) {
>          abort();
>      }
>  
> @@ -932,14 +956,14 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
>      qemu_put_be32(f, vdev->config_len);
>      qemu_put_buffer(f, vdev->config, vdev->config_len);
>  
> -    for (i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) {
> +    for (i = 0; i < k->queue_max; i++) {
>          if (vdev->vq[i].vring.num == 0)
>              break;
>      }
>  
>      qemu_put_be32(f, i);
>  
> -    for (i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) {
> +    for (i = 0; i < k->queue_max; i++) {
>          if (vdev->vq[i].vring.num == 0)
>              break;
>  
> @@ -1004,7 +1028,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
>      qemu_get_8s(f, &vdev->status);
>      qemu_get_8s(f, &vdev->isr);
>      qemu_get_be16s(f, &vdev->queue_sel);
> -    if (vdev->queue_sel >= VIRTIO_PCI_QUEUE_MAX) {
> +    if (vdev->queue_sel >= k->queue_max) {
>          return -1;
>      }
>      qemu_get_be32s(f, &features);
> @@ -1031,7 +1055,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int version_id)
>  
>      num = qemu_get_be32(f);
>  
> -    if (num > VIRTIO_PCI_QUEUE_MAX) {
> +    if (num > k->queue_max) {
>          error_report("Invalid number of PCI queues: 0x%x", num);
>          return -1;
>      }
> @@ -1141,15 +1165,17 @@ void virtio_instance_init_common(Object *proxy_obj, void *data,
>  void virtio_init(VirtIODevice *vdev, const char *name,
>                   uint16_t device_id, size_t config_size)
>  {
> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>      int i;
>      vdev->device_id = device_id;
>      vdev->status = 0;
>      vdev->isr = 0;
>      vdev->queue_sel = 0;
>      vdev->config_vector = VIRTIO_NO_VECTOR;
> -    vdev->vq = g_malloc0(sizeof(VirtQueue) * VIRTIO_PCI_QUEUE_MAX);
> +    vdev->vq = g_malloc0(sizeof(VirtQueue) * k->queue_max);
>      vdev->vm_running = runstate_is_running();
> -    for (i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) {
> +    for (i = 0; i < k->queue_max; i++) {
>          vdev->vq[i].vector = VIRTIO_NO_VECTOR;
>          vdev->vq[i].vdev = vdev;
>          vdev->vq[i].queue_index = i;
> diff --git a/include/hw/virtio/virtio-bus.h b/include/hw/virtio/virtio-bus.h
> index 0756545..979835c 100644
> --- a/include/hw/virtio/virtio-bus.h
> +++ b/include/hw/virtio/virtio-bus.h
> @@ -68,6 +68,7 @@ typedef struct VirtioBusClass {
>       * Note that changing this will break migration for this transport.
>       */
>      bool has_variable_vring_alignment;
> +    uint16_t queue_max;
>  } VirtioBusClass;
>  
>  struct VirtioBusState {
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index f24997d..9fe0d92 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -223,6 +223,7 @@ void virtio_queue_notify(VirtIODevice *vdev, int n);
>  uint16_t virtio_queue_vector(VirtIODevice *vdev, int n);
>  void virtio_queue_set_vector(VirtIODevice *vdev, int n, uint16_t vector);
>  void virtio_set_status(VirtIODevice *vdev, uint8_t val);
> +int virtio_get_queue_max(VirtIODevice *vdev);
>  void virtio_reset(void *opaque);
>  void virtio_update_irq(VirtIODevice *vdev);
>  int virtio_set_features(VirtIODevice *vdev, uint32_t val);
> -- 
> 1.9.1

Re: [Qemu-devel] [PATCH V2 03/11] virito: introduce bus specific queue limit

[Jason Wang]

On Thu, Feb 26, 2015 at 6:19 PM, Michael S. Tsirkin <mst@redhat.com> 
wrote:
> On Thu, Feb 26, 2015 at 03:04:38PM +0800, Jason Wang wrote:
>>  This patch introduces a bus specific queue limitation. It will be
>>  useful for increasing the limit for one of the bus without 
>> disturbing
>>  other buses.
> 
> Is this about s390 only supporting up to 64 queues?

Not specific to s390. It just introduces a queue_max to each bus and 
main changes were done for virito core. For each specific transport, it 
just set queue_max to VIRTIO_PCI_QUEUE_MAX(64).

Re: [Qemu-devel] [PATCH V2 03/11] virito: introduce bus specific queue limit

[Cornelia Huck]

On Thu, 26 Feb 2015 15:04:38 +0800
Jason Wang <jasowang@redhat.com> wrote:

> This patch introduces a bus specific queue limitation. It will be
> useful for increasing the limit for one of the bus without disturbing
> other buses.
> 
> Cc: Anthony Liguori <aliguori@amazon.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  hw/net/virtio-net.c            |  4 ++--
>  hw/s390x/s390-virtio-bus.c     |  1 +
>  hw/s390x/virtio-ccw.c          |  1 +
>  hw/scsi/virtio-scsi.c          |  4 ++--
>  hw/virtio/virtio-mmio.c        |  1 +
>  hw/virtio/virtio-pci.c         |  1 +
>  hw/virtio/virtio.c             | 52 +++++++++++++++++++++++++++++++-----------
>  include/hw/virtio/virtio-bus.h |  1 +
>  include/hw/virtio/virtio.h     |  1 +
>  9 files changed, 49 insertions(+), 17 deletions(-)
> 

> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index ffc22e8..5a806b5 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -541,6 +541,14 @@ void virtio_update_irq(VirtIODevice *vdev)
>      virtio_notify_vector(vdev, VIRTIO_NO_VECTOR);
>  }
> 
> +int virtio_get_queue_max(VirtIODevice *vdev)
> +{
> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +
> +    return k->queue_max;
> +}
> +

Are all callers of this in the slow path? So we don't introduce
processing overhead.

>  void virtio_set_status(VirtIODevice *vdev, uint8_t val)
>  {
>      VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);

(...)


> @@ -777,27 +790,35 @@ void virtio_queue_notify(VirtIODevice *vdev, int n)
> 
>  uint16_t virtio_queue_vector(VirtIODevice *vdev, int n)
>  {
> -    return n < VIRTIO_PCI_QUEUE_MAX ? vdev->vq[n].vector :
> +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> +
> +    return n < k->queue_max ? vdev->vq[n].vector :

Any reason you're not using the newly introduced virtio_get_queue_max()?

>          VIRTIO_NO_VECTOR;
>  }

Do we need to care about migration if the target supports a different
number of queues?

I'm also not sure whether it would be sufficient to allow transports to
limit queues to a lower number than the core supports. We'd basically
only need to block off queue creation in the individual transports, I
think.

Re: [Qemu-devel] [PATCH V2 03/11] virito: introduce bus specific queue limit

[Jason Wang]

On Thu, Feb 26, 2015 at 8:57 PM, Cornelia Huck 
<cornelia.huck@de.ibm.com> wrote:
> On Thu, 26 Feb 2015 15:04:38 +0800
> Jason Wang <jasowang@redhat.com> wrote:
> 
>>  This patch introduces a bus specific queue limitation. It will be
>>  useful for increasing the limit for one of the bus without 
>> disturbing
>>  other buses.
>>  
>>  Cc: Anthony Liguori <aliguori@amazon.com>
>>  Cc: Michael S. Tsirkin <mst@redhat.com>
>>  Cc: Alexander Graf <agraf@suse.de>
>>  Cc: Richard Henderson <rth@twiddle.net>
>>  Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
>>  Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>>  Cc: Paolo Bonzini <pbonzini@redhat.com>
>>  Signed-off-by: Jason Wang <jasowang@redhat.com>
>>  ---
>>   hw/net/virtio-net.c            |  4 ++--
>>   hw/s390x/s390-virtio-bus.c     |  1 +
>>   hw/s390x/virtio-ccw.c          |  1 +
>>   hw/scsi/virtio-scsi.c          |  4 ++--
>>   hw/virtio/virtio-mmio.c        |  1 +
>>   hw/virtio/virtio-pci.c         |  1 +
>>   hw/virtio/virtio.c             | 52 
>> +++++++++++++++++++++++++++++++-----------
>>   include/hw/virtio/virtio-bus.h |  1 +
>>   include/hw/virtio/virtio.h     |  1 +
>>   9 files changed, 49 insertions(+), 17 deletions(-)
>>  
> 
>>  diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>>  index ffc22e8..5a806b5 100644
>>  --- a/hw/virtio/virtio.c
>>  +++ b/hw/virtio/virtio.c
>>  @@ -541,6 +541,14 @@ void virtio_update_irq(VirtIODevice *vdev)
>>       virtio_notify_vector(vdev, VIRTIO_NO_VECTOR);
>>   }
>>  
>>  +int virtio_get_queue_max(VirtIODevice *vdev)
>>  +{
>>  +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>>  +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>>  +
>>  +    return k->queue_max;
>>  +}
>>  +
> 
> Are all callers of this in the slow path? So we don't introduce
> processing overhead.

Looks not. For overhead, do you mean one introduced by 
VIRTIO_BUS_GET_CLASS()? Not sure how much it will affact but we've 
already used something like this in the datapath, e.g 
virtio_notify_vector(). 
> 
> 
>>   void virtio_set_status(VirtIODevice *vdev, uint8_t val)
>>   {
>>       VirtioDeviceClass *k = VIRTIO_DEVICE_GET_CLASS(vdev);
> 
> (...)
> 
> 
>>  @@ -777,27 +790,35 @@ void virtio_queue_notify(VirtIODevice *vdev, 
>> int n)
>>  
>>   uint16_t virtio_queue_vector(VirtIODevice *vdev, int n)
>>   {
>>  -    return n < VIRTIO_PCI_QUEUE_MAX ? vdev->vq[n].vector :
>>  +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>>  +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>>  +
>>  +    return n < k->queue_max ? vdev->vq[n].vector :
> 
> Any reason you're not using the newly introduced 
> virtio_get_queue_max()?

Yes, better use virtio_get_queue_max().
> 
> 
>>           VIRTIO_NO_VECTOR;
>>   }
> 
> Do we need to care about migration if the target supports a different
> number of queues?

See patch 9, if a target support a different number of queues, it can 
patch the k->queue_max during its initialization. 
> 
> 
> I'm also not sure whether it would be sufficient to allow transports 
> to
> limit queues to a lower number than the core supports. We'd basically
> only need to block off queue creation in the individual transports, I
> think.

Not sure I understand the issue correctly, with this series. There's 
not core limitation but only transport limitation.

Re: [Qemu-devel] [PATCH V2 03/11] virito: introduce bus specific queue limit

[Cornelia Huck]

On Fri, 27 Feb 2015 03:42:00 +0008
Jason Wang <jasowang@redhat.com> wrote:

> On Thu, Feb 26, 2015 at 8:57 PM, Cornelia Huck 
> <cornelia.huck@de.ibm.com> wrote:
> > On Thu, 26 Feb 2015 15:04:38 +0800
> > Jason Wang <jasowang@redhat.com> wrote:
> > 
> >>  This patch introduces a bus specific queue limitation. It will be
> >>  useful for increasing the limit for one of the bus without 
> >> disturbing
> >>  other buses.

> >>  diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> >>  index ffc22e8..5a806b5 100644
> >>  --- a/hw/virtio/virtio.c
> >>  +++ b/hw/virtio/virtio.c
> >>  @@ -541,6 +541,14 @@ void virtio_update_irq(VirtIODevice *vdev)
> >>       virtio_notify_vector(vdev, VIRTIO_NO_VECTOR);
> >>   }
> >>  
> >>  +int virtio_get_queue_max(VirtIODevice *vdev)
> >>  +{
> >>  +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
> >>  +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
> >>  +
> >>  +    return k->queue_max;
> >>  +}
> >>  +
> > 
> > Are all callers of this in the slow path? So we don't introduce
> > processing overhead.
> 
> Looks not. For overhead, do you mean one introduced by 
> VIRTIO_BUS_GET_CLASS()? Not sure how much it will affact but we've 
> already used something like this in the datapath, e.g 
> virtio_notify_vector(). 

I may have misremembered how much overhead those types of operation
introduce.

But it made me think: This function is basically introducing a
per-VirtIODevice queue limit. We set it once in the VirtioBusClass
during initialization, but don't expect it to change. Why don't we just
propagate it to a new member of VirtIODevice during initialization
instead?

Re: [Qemu-devel] [PATCH V2 03/11] virito: introduce bus specific queue limit

[Jason Wang]

On Fri, Feb 27, 2015 at 5:34 PM, Cornelia Huck 
<cornelia.huck@de.ibm.com> wrote:
> On Fri, 27 Feb 2015 03:42:00 +0008
> Jason Wang <jasowang@redhat.com> wrote:
> 
>>  On Thu, Feb 26, 2015 at 8:57 PM, Cornelia Huck 
>>  <cornelia.huck@de.ibm.com> wrote:
>>  > On Thu, 26 Feb 2015 15:04:38 +0800
>>  > Jason Wang <jasowang@redhat.com> wrote:
>>  > 
>>  >>  This patch introduces a bus specific queue limitation. It will 
>> be
>>  >>  useful for increasing the limit for one of the bus without 
>>  >> disturbing
>>  >>  other buses.
> 
>>  >>  diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
>>  >>  index ffc22e8..5a806b5 100644
>>  >>  --- a/hw/virtio/virtio.c
>>  >>  +++ b/hw/virtio/virtio.c
>>  >>  @@ -541,6 +541,14 @@ void virtio_update_irq(VirtIODevice *vdev)
>>  >>       virtio_notify_vector(vdev, VIRTIO_NO_VECTOR);
>>  >>   }
>>  >>  
>>  >>  +int virtio_get_queue_max(VirtIODevice *vdev)
>>  >>  +{
>>  >>  +    BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>>  >>  +    VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>>  >>  +
>>  >>  +    return k->queue_max;
>>  >>  +}
>>  >>  +
>>  > 
>>  > Are all callers of this in the slow path? So we don't introduce
>>  > processing overhead.
>>  
>>  Looks not. For overhead, do you mean one introduced by 
>>  VIRTIO_BUS_GET_CLASS()? Not sure how much it will affact but we've 
>>  already used something like this in the datapath, e.g 
>>  virtio_notify_vector(). 
> 
> I may have misremembered how much overhead those types of operation
> introduce.
> 
> But it made me think: This function is basically introducing a
> per-VirtIODevice queue limit. We set it once in the VirtioBusClass
> during initialization, but don't expect it to change. Why don't we 
> just
> propagate it to a new member of VirtIODevice during initialization
> instead?

The limit may be changed. Consider that patch 9 increases the pci limit 
from 64 to 513. But we want to keep the migration compatibility for 
legacy machine types e.g machines earlier than pc-q35-2.3 or 
pc-i440fx-2.3, so the limit was changed back to 64 in pc_compat_2_2(). 
More work will be done if we want to propagate it to VirtIODevice, and 
it looks unnecessary.

[Qemu-devel] [PATCH V2 04/11] virtio-ccw: introduce ccw specific queue limit

[Jason Wang]

Instead of depending on marco, using a bus specific limit.

Cc: Alexander Graf <agraf@suse.de>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Richard Henderson <rth@twiddle.net>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/s390x/s390-virtio-ccw.c |  7 +++++--
 hw/s390x/virtio-ccw.c      | 13 +++++++------
 include/hw/virtio/virtio.h |  1 +
 3 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 71bafe0..6aeb815 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -43,6 +43,7 @@ void io_subsystem_reset(void)
 
 static int virtio_ccw_hcall_notify(const uint64_t *args)
 {
+    VirtIODevice *vdev;
     uint64_t subch_id = args[0];
     uint64_t queue = args[1];
     SubchDev *sch;
@@ -55,10 +56,12 @@ static int virtio_ccw_hcall_notify(const uint64_t *args)
     if (!sch || !css_subch_visible(sch)) {
         return -EINVAL;
     }
-    if (queue >= VIRTIO_PCI_QUEUE_MAX) {
+
+    vdev = virtio_ccw_get_vdev(sch);
+    if (queue >= virtio_get_queue_max(vdev)) {
         return -EINVAL;
     }
-    virtio_queue_notify(virtio_ccw_get_vdev(sch), queue);
+    virtio_queue_notify(vdev, queue);
     return 0;
 
 }
diff --git a/hw/s390x/virtio-ccw.c b/hw/s390x/virtio-ccw.c
index 4874622..98a1a90 100644
--- a/hw/s390x/virtio-ccw.c
+++ b/hw/s390x/virtio-ccw.c
@@ -170,7 +170,7 @@ static void virtio_ccw_start_ioeventfd(VirtioCcwDevice *dev)
         return;
     }
     vdev = virtio_bus_get_device(&dev->bus);
-    for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
+    for (n = 0; n < virtio_get_queue_max(vdev); n++) {
         if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
@@ -205,7 +205,7 @@ static void virtio_ccw_stop_ioeventfd(VirtioCcwDevice *dev)
         return;
     }
     vdev = virtio_bus_get_device(&dev->bus);
-    for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
+    for (n = 0; n < virtio_get_queue_max(vdev); n++) {
         if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
@@ -265,8 +265,9 @@ static int virtio_ccw_set_vqs(SubchDev *sch, uint64_t addr, uint32_t align,
                               uint16_t index, uint16_t num)
 {
     VirtIODevice *vdev = virtio_ccw_get_vdev(sch);
+    int queue_max = virtio_get_queue_max(vdev);
 
-    if (index > VIRTIO_PCI_QUEUE_MAX) {
+    if (index > queue_max) {
         return -EINVAL;
     }
 
@@ -291,7 +292,7 @@ static int virtio_ccw_set_vqs(SubchDev *sch, uint64_t addr, uint32_t align,
         virtio_queue_set_vector(vdev, index, index);
     }
     /* tell notify handler in case of config change */
-    vdev->config_vector = VIRTIO_PCI_QUEUE_MAX;
+    vdev->config_vector = queue_max;
     return 0;
 }
 
@@ -1026,7 +1027,7 @@ static void virtio_ccw_notify(DeviceState *d, uint16_t vector)
         return;
     }
 
-    if (vector < VIRTIO_PCI_QUEUE_MAX) {
+    if (vector < virtio_get_queue_max(virtio_bus_get_device(&dev->bus))) {
         if (!dev->indicators) {
             return;
         }
@@ -1695,7 +1696,7 @@ static void virtio_ccw_bus_class_init(ObjectClass *klass, void *data)
     k->load_queue = virtio_ccw_load_queue;
     k->save_config = virtio_ccw_save_config;
     k->load_config = virtio_ccw_load_config;
-    k->queue_max = VIRTIO_PCI_QUEUE_MAX;
+    k->queue_max = VIRTIO_CCW_QUEUE_MAX;
 }
 
 static const TypeInfo virtio_ccw_bus_info = {
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 9fe0d92..04ad532 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -93,6 +93,7 @@ typedef struct VirtQueueElement
 } VirtQueueElement;
 
 #define VIRTIO_PCI_QUEUE_MAX 64
+#define VIRTIO_CCW_QUEUE_MAX 64
 
 #define VIRTIO_NO_VECTOR 0xffff
 
-- 
1.9.1

Re: [Qemu-devel] [PATCH V2 04/11] virtio-ccw: introduce ccw specific queue limit

[Cornelia Huck]

On Thu, 26 Feb 2015 15:04:39 +0800
Jason Wang <jasowang@redhat.com> wrote:

> Instead of depending on marco, using a bus specific limit.
> 
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Richard Henderson <rth@twiddle.net>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  hw/s390x/s390-virtio-ccw.c |  7 +++++--
>  hw/s390x/virtio-ccw.c      | 13 +++++++------
>  include/hw/virtio/virtio.h |  1 +
>  3 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 71bafe0..6aeb815 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -43,6 +43,7 @@ void io_subsystem_reset(void)
> 
>  static int virtio_ccw_hcall_notify(const uint64_t *args)
>  {
> +    VirtIODevice *vdev;
>      uint64_t subch_id = args[0];
>      uint64_t queue = args[1];
>      SubchDev *sch;
> @@ -55,10 +56,12 @@ static int virtio_ccw_hcall_notify(const uint64_t *args)
>      if (!sch || !css_subch_visible(sch)) {
>          return -EINVAL;
>      }
> -    if (queue >= VIRTIO_PCI_QUEUE_MAX) {
> +
> +    vdev = virtio_ccw_get_vdev(sch);
> +    if (queue >= virtio_get_queue_max(vdev)) {

But we already know we have a virtio_ccw device, right? So why not just
check against VIRTIO_CCW_QUEUE_MAX?

>          return -EINVAL;
>      }
> -    virtio_queue_notify(virtio_ccw_get_vdev(sch), queue);
> +    virtio_queue_notify(vdev, queue);
>      return 0;
> 
>  }

(...)

> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 9fe0d92..04ad532 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -93,6 +93,7 @@ typedef struct VirtQueueElement
>  } VirtQueueElement;
> 
>  #define VIRTIO_PCI_QUEUE_MAX 64
> +#define VIRTIO_CCW_QUEUE_MAX 64

Does generic code need to know about the ccw limit?

> 
>  #define VIRTIO_NO_VECTOR 0xffff
> 

Re: [Qemu-devel] [PATCH V2 04/11] virtio-ccw: introduce ccw specific queue limit

[Jason Wang]

On Thu, Feb 26, 2015 at 9:02 PM, Cornelia Huck 
<cornelia.huck@de.ibm.com> wrote:
> On Thu, 26 Feb 2015 15:04:39 +0800
> Jason Wang <jasowang@redhat.com> wrote:
> 
>>  Instead of depending on marco, using a bus specific limit.
>>  
>>  Cc: Alexander Graf <agraf@suse.de>
>>  Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
>>  Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>>  Cc: Richard Henderson <rth@twiddle.net>
>>  Signed-off-by: Jason Wang <jasowang@redhat.com>
>>  ---
>>   hw/s390x/s390-virtio-ccw.c |  7 +++++--
>>   hw/s390x/virtio-ccw.c      | 13 +++++++------
>>   include/hw/virtio/virtio.h |  1 +
>>   3 files changed, 13 insertions(+), 8 deletions(-)
>>  
>>  diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>  index 71bafe0..6aeb815 100644
>>  --- a/hw/s390x/s390-virtio-ccw.c
>>  +++ b/hw/s390x/s390-virtio-ccw.c
>>  @@ -43,6 +43,7 @@ void io_subsystem_reset(void)
>>  
>>   static int virtio_ccw_hcall_notify(const uint64_t *args)
>>   {
>>  +    VirtIODevice *vdev;
>>       uint64_t subch_id = args[0];
>>       uint64_t queue = args[1];
>>       SubchDev *sch;
>>  @@ -55,10 +56,12 @@ static int virtio_ccw_hcall_notify(const 
>> uint64_t *args)
>>       if (!sch || !css_subch_visible(sch)) {
>>           return -EINVAL;
>>       }
>>  -    if (queue >= VIRTIO_PCI_QUEUE_MAX) {
>>  +
>>  +    vdev = virtio_ccw_get_vdev(sch);
>>  +    if (queue >= virtio_get_queue_max(vdev)) {
> 
> But we already know we have a virtio_ccw device, right? So why not 
> just
> check against VIRTIO_CCW_QUEUE_MAX?

The problem is whether or not you want to increase the max queue for 
ccw. If yes, for migration compatibility, you could not just use a 
marco here since legacy machine type will also get increased. Something 
dynamically like this is need so the machine initialization code can 
change this limit. 
> 
> 
>>           return -EINVAL;
>>       }
>>  -    virtio_queue_notify(virtio_ccw_get_vdev(sch), queue);
>>  +    virtio_queue_notify(vdev, queue);
>>       return 0;
>>  
>>   }
> 
> (...)
> 
>>  diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>>  index 9fe0d92..04ad532 100644
>>  --- a/include/hw/virtio/virtio.h
>>  +++ b/include/hw/virtio/virtio.h
>>  @@ -93,6 +93,7 @@ typedef struct VirtQueueElement
>>   } VirtQueueElement;
>>  
>>   #define VIRTIO_PCI_QUEUE_MAX 64
>>  +#define VIRTIO_CCW_QUEUE_MAX 64
> 
> Does generic code need to know about the ccw limit?

Seems not, will move it to ccw file.
> 
> 
>>  
>>   #define VIRTIO_NO_VECTOR 0xffff
>>  
> 

Re: [Qemu-devel] [PATCH V2 04/11] virtio-ccw: introduce ccw specific queue limit

[Cornelia Huck]

On Fri, 27 Feb 2015 03:46:25 +0008
Jason Wang <jasowang@redhat.com> wrote:

> On Thu, Feb 26, 2015 at 9:02 PM, Cornelia Huck 
> <cornelia.huck@de.ibm.com> wrote:
> > On Thu, 26 Feb 2015 15:04:39 +0800
> > Jason Wang <jasowang@redhat.com> wrote:
> > 
> >>  Instead of depending on marco, using a bus specific limit.
> >>  
> >>  Cc: Alexander Graf <agraf@suse.de>
> >>  Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> >>  Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> >>  Cc: Richard Henderson <rth@twiddle.net>
> >>  Signed-off-by: Jason Wang <jasowang@redhat.com>
> >>  ---
> >>   hw/s390x/s390-virtio-ccw.c |  7 +++++--
> >>   hw/s390x/virtio-ccw.c      | 13 +++++++------
> >>   include/hw/virtio/virtio.h |  1 +
> >>   3 files changed, 13 insertions(+), 8 deletions(-)
> >>  
> >>  diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> >>  index 71bafe0..6aeb815 100644
> >>  --- a/hw/s390x/s390-virtio-ccw.c
> >>  +++ b/hw/s390x/s390-virtio-ccw.c
> >>  @@ -43,6 +43,7 @@ void io_subsystem_reset(void)
> >>  
> >>   static int virtio_ccw_hcall_notify(const uint64_t *args)
> >>   {
> >>  +    VirtIODevice *vdev;
> >>       uint64_t subch_id = args[0];
> >>       uint64_t queue = args[1];
> >>       SubchDev *sch;
> >>  @@ -55,10 +56,12 @@ static int virtio_ccw_hcall_notify(const 
> >> uint64_t *args)
> >>       if (!sch || !css_subch_visible(sch)) {
> >>           return -EINVAL;
> >>       }
> >>  -    if (queue >= VIRTIO_PCI_QUEUE_MAX) {
> >>  +
> >>  +    vdev = virtio_ccw_get_vdev(sch);
> >>  +    if (queue >= virtio_get_queue_max(vdev)) {
> > 
> > But we already know we have a virtio_ccw device, right? So why not 
> > just
> > check against VIRTIO_CCW_QUEUE_MAX?
> 
> The problem is whether or not you want to increase the max queue for 
> ccw. If yes, for migration compatibility, you could not just use a 
> marco here since legacy machine type will also get increased. 

Confused. With "legacy machine type", do you mean s390-virtio? It does
not register this hcall.

> Something 
> dynamically like this is need so the machine initialization code can 
> change this limit. 

I fear I don't understand how the machine code comes into play here. Is
the virtio-ccw device code supposed to inherit the limit from the
machine?

Anyway, if we move the queue limit to the VirtIODevice, checking the
limit there instead of using a #define should work out fine.

Re: [Qemu-devel] [PATCH V2 04/11] virtio-ccw: introduce ccw specific queue limit

[Jason Wang]

On Fri, Feb 27, 2015 at 5:41 PM, Cornelia Huck 
<cornelia.huck@de.ibm.com> wrote:
> On Fri, 27 Feb 2015 03:46:25 +0008
> Jason Wang <jasowang@redhat.com> wrote:
> 
>>  On Thu, Feb 26, 2015 at 9:02 PM, Cornelia Huck 
>>  <cornelia.huck@de.ibm.com> wrote:
>>  > On Thu, 26 Feb 2015 15:04:39 +0800
>>  > Jason Wang <jasowang@redhat.com> wrote:
>>  > 
>>  >>  Instead of depending on marco, using a bus specific limit.
>>  >>  
>>  >>  Cc: Alexander Graf <agraf@suse.de>
>>  >>  Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
>>  >>  Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>>  >>  Cc: Richard Henderson <rth@twiddle.net>
>>  >>  Signed-off-by: Jason Wang <jasowang@redhat.com>
>>  >>  ---
>>  >>   hw/s390x/s390-virtio-ccw.c |  7 +++++--
>>  >>   hw/s390x/virtio-ccw.c      | 13 +++++++------
>>  >>   include/hw/virtio/virtio.h |  1 +
>>  >>   3 files changed, 13 insertions(+), 8 deletions(-)
>>  >>  
>>  >>  diff --git a/hw/s390x/s390-virtio-ccw.c 
>> b/hw/s390x/s390-virtio-ccw.c
>>  >>  index 71bafe0..6aeb815 100644
>>  >>  --- a/hw/s390x/s390-virtio-ccw.c
>>  >>  +++ b/hw/s390x/s390-virtio-ccw.c
>>  >>  @@ -43,6 +43,7 @@ void io_subsystem_reset(void)
>>  >>  
>>  >>   static int virtio_ccw_hcall_notify(const uint64_t *args)
>>  >>   {
>>  >>  +    VirtIODevice *vdev;
>>  >>       uint64_t subch_id = args[0];
>>  >>       uint64_t queue = args[1];
>>  >>       SubchDev *sch;
>>  >>  @@ -55,10 +56,12 @@ static int virtio_ccw_hcall_notify(const 
>>  >> uint64_t *args)
>>  >>       if (!sch || !css_subch_visible(sch)) {
>>  >>           return -EINVAL;
>>  >>       }
>>  >>  -    if (queue >= VIRTIO_PCI_QUEUE_MAX) {
>>  >>  +
>>  >>  +    vdev = virtio_ccw_get_vdev(sch);
>>  >>  +    if (queue >= virtio_get_queue_max(vdev)) {
>>  > 
>>  > But we already know we have a virtio_ccw device, right? So why 
>> not 
>>  > just
>>  > check against VIRTIO_CCW_QUEUE_MAX?
>>  
>>  The problem is whether or not you want to increase the max queue 
>> for 
>>  ccw. If yes, for migration compatibility, you could not just use a 
>>  marco here since legacy machine type will also get increased. 
> 
> Confused. With "legacy machine type", do you mean s390-virtio? It does
> not register this hcall.

Ok, I thought s390 may have something like pc had to keep the migration 
compatibility for migration. But seems not.

> 
> 
>>  Something 
>>  dynamically like this is need so the machine initialization code 
>> can 
>>  change this limit. 
> 
> I fear I don't understand how the machine code comes into play here. 
> Is
> the virtio-ccw device code supposed to inherit the limit from the
> machine?

As I said in the previous reply. The machine code is used to keep 
migration compatibility. We don't want to break the migration from 2.3 
to 2.2. If the limit of ccw will be increased in the future and we want 
to keep migration compatibility. The machine codes may do something 
similar to virtio pci.


> 
> Anyway, if we move the queue limit to the VirtIODevice, checking the
> limit there instead of using a #define should work out fine.
> 
> 

[Qemu-devel] [PATCH V2 05/11] virtio-serial-bus: switch to bus specific queue limit

[Jason Wang]

Cc: Amit Shah <amit.shah@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/char/virtio-serial-bus.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/hw/char/virtio-serial-bus.c b/hw/char/virtio-serial-bus.c
index 37a6f44..f280e95 100644
--- a/hw/char/virtio-serial-bus.c
+++ b/hw/char/virtio-serial-bus.c
@@ -26,6 +26,8 @@
 #include "hw/virtio/virtio-serial.h"
 #include "hw/virtio/virtio-access.h"
 
+#define VIRTIO_SERIAL_BUS_QUEUE_MAX 64
+
 struct VirtIOSerialDevices {
     QLIST_HEAD(, VirtIOSerial) devices;
 } vserdevices;
@@ -942,7 +944,7 @@ static void virtio_serial_device_realize(DeviceState *dev, Error **errp)
     }
 
     /* Each port takes 2 queues, and one pair is for the control queue */
-    max_supported_ports = VIRTIO_PCI_QUEUE_MAX / 2 - 1;
+    max_supported_ports = VIRTIO_SERIAL_BUS_QUEUE_MAX / 2 - 1;
 
     if (vser->serial.max_virtserial_ports > max_supported_ports) {
         error_setg(errp, "maximum ports supported: %u", max_supported_ports);
-- 
1.9.1

[Qemu-devel] [PATCH V2 06/11] virtio-s390: switch to bus specific queue limit

[Jason Wang]

Instead of depending on marco, switch to use a bus specific queue
limit. Left is AdapterRouters->gsi[], this could be done in the future
if we want to increase s390's queue limit really.

Cc: Alexander Graf <agraf@suse.de>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Christian Borntraeger <borntraeger@de.ibm.com>
Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/s390x/s390-virtio-bus.c   | 6 +++---
 include/hw/s390x/s390_flic.h | 2 +-
 include/hw/virtio/virtio.h   | 1 +
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
index d48590a..4e2bbbc 100644
--- a/hw/s390x/s390-virtio-bus.c
+++ b/hw/s390x/s390-virtio-bus.c
@@ -331,7 +331,7 @@ static ram_addr_t s390_virtio_device_num_vq(VirtIOS390Device *dev)
     VirtIODevice *vdev = dev->vdev;
     int num_vq;
 
-    for (num_vq = 0; num_vq < VIRTIO_PCI_QUEUE_MAX; num_vq++) {
+    for (num_vq = 0; num_vq < virtio_get_queue_max(vdev); num_vq++) {
         if (!virtio_queue_get_num(vdev, num_vq)) {
             break;
         }
@@ -438,7 +438,7 @@ VirtIOS390Device *s390_virtio_bus_find_vring(VirtIOS390Bus *bus,
     QTAILQ_FOREACH(kid, &bus->bus.children, sibling) {
         VirtIOS390Device *dev = (VirtIOS390Device *)kid->child;
 
-        for(i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) {
+        for(i = 0; i < virtio_get_queue_max(dev->vdev); i++) {
             if (!virtio_queue_get_addr(dev->vdev, i))
                 break;
             if (virtio_queue_get_addr(dev->vdev, i) == mem) {
@@ -707,7 +707,7 @@ static void virtio_s390_bus_class_init(ObjectClass *klass, void *data)
     bus_class->max_dev = 1;
     k->notify = virtio_s390_notify;
     k->get_features = virtio_s390_get_features;
-    k->queue_max = VIRTIO_PCI_QUEUE_MAX;
+    k->queue_max = VIRTIO_S390_QUEUE_MAX;
 }
 
 static const TypeInfo virtio_s390_bus_info = {
diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h
index 489d73b..7a3ada2 100644
--- a/include/hw/s390x/s390_flic.h
+++ b/include/hw/s390x/s390_flic.h
@@ -20,7 +20,7 @@
 typedef struct AdapterRoutes {
     AdapterInfo adapter;
     int num_routes;
-    int gsi[VIRTIO_PCI_QUEUE_MAX];
+    int gsi[VIRTIO_S390_QUEUE_MAX];
 } AdapterRoutes;
 
 #define TYPE_S390_FLIC_COMMON "s390-flic"
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 04ad532..8f4da77 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -94,6 +94,7 @@ typedef struct VirtQueueElement
 
 #define VIRTIO_PCI_QUEUE_MAX 64
 #define VIRTIO_CCW_QUEUE_MAX 64
+#define VIRTIO_S390_QUEUE_MAX 64
 
 #define VIRTIO_NO_VECTOR 0xffff
 
-- 
1.9.1

Re: [Qemu-devel] [PATCH V2 06/11] virtio-s390: switch to bus specific queue limit

[Cornelia Huck]

On Thu, 26 Feb 2015 15:04:41 +0800
Jason Wang <jasowang@redhat.com> wrote:

> Instead of depending on marco, switch to use a bus specific queue
> limit. Left is AdapterRouters->gsi[], this could be done in the future
> if we want to increase s390's queue limit really.
> 
> Cc: Alexander Graf <agraf@suse.de>
> Cc: Richard Henderson <rth@twiddle.net>
> Cc: Christian Borntraeger <borntraeger@de.ibm.com>
> Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  hw/s390x/s390-virtio-bus.c   | 6 +++---
>  include/hw/s390x/s390_flic.h | 2 +-
>  include/hw/virtio/virtio.h   | 1 +
>  3 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
> index d48590a..4e2bbbc 100644
> --- a/hw/s390x/s390-virtio-bus.c
> +++ b/hw/s390x/s390-virtio-bus.c
> @@ -331,7 +331,7 @@ static ram_addr_t s390_virtio_device_num_vq(VirtIOS390Device *dev)
>      VirtIODevice *vdev = dev->vdev;
>      int num_vq;
> 
> -    for (num_vq = 0; num_vq < VIRTIO_PCI_QUEUE_MAX; num_vq++) {
> +    for (num_vq = 0; num_vq < virtio_get_queue_max(vdev); num_vq++) {
>          if (!virtio_queue_get_num(vdev, num_vq)) {
>              break;
>          }
> @@ -438,7 +438,7 @@ VirtIOS390Device *s390_virtio_bus_find_vring(VirtIOS390Bus *bus,
>      QTAILQ_FOREACH(kid, &bus->bus.children, sibling) {
>          VirtIOS390Device *dev = (VirtIOS390Device *)kid->child;
> 
> -        for(i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) {
> +        for(i = 0; i < virtio_get_queue_max(dev->vdev); i++) {

Again, I do not see any advantage over s/PCI/S390/ here.

>              if (!virtio_queue_get_addr(dev->vdev, i))
>                  break;
>              if (virtio_queue_get_addr(dev->vdev, i) == mem) {

(...)

> diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h
> index 489d73b..7a3ada2 100644
> --- a/include/hw/s390x/s390_flic.h
> +++ b/include/hw/s390x/s390_flic.h
> @@ -20,7 +20,7 @@
>  typedef struct AdapterRoutes {
>      AdapterInfo adapter;
>      int num_routes;
> -    int gsi[VIRTIO_PCI_QUEUE_MAX];
> +    int gsi[VIRTIO_S390_QUEUE_MAX];

Adapter routes are only applicable for the ccw transport, not for the
old s390 transport.

(I'm also wondering whether this should be the generic limit instead.)

>  } AdapterRoutes;
> 
>  #define TYPE_S390_FLIC_COMMON "s390-flic"

Re: [Qemu-devel] [PATCH V2 06/11] virtio-s390: switch to bus specific queue limit

[Jason Wang]

On Thu, Feb 26, 2015 at 9:05 PM, Cornelia Huck 
<cornelia.huck@de.ibm.com> wrote:
> On Thu, 26 Feb 2015 15:04:41 +0800
> Jason Wang <jasowang@redhat.com> wrote:
> 
>>  Instead of depending on marco, switch to use a bus specific queue
>>  limit. Left is AdapterRouters->gsi[], this could be done in the 
>> future
>>  if we want to increase s390's queue limit really.
>>  
>>  Cc: Alexander Graf <agraf@suse.de>
>>  Cc: Richard Henderson <rth@twiddle.net>
>>  Cc: Christian Borntraeger <borntraeger@de.ibm.com>
>>  Cc: Cornelia Huck <cornelia.huck@de.ibm.com>
>>  Signed-off-by: Jason Wang <jasowang@redhat.com>
>>  ---
>>   hw/s390x/s390-virtio-bus.c   | 6 +++---
>>   include/hw/s390x/s390_flic.h | 2 +-
>>   include/hw/virtio/virtio.h   | 1 +
>>   3 files changed, 5 insertions(+), 4 deletions(-)
>>  
>>  diff --git a/hw/s390x/s390-virtio-bus.c b/hw/s390x/s390-virtio-bus.c
>>  index d48590a..4e2bbbc 100644
>>  --- a/hw/s390x/s390-virtio-bus.c
>>  +++ b/hw/s390x/s390-virtio-bus.c
>>  @@ -331,7 +331,7 @@ static ram_addr_t 
>> s390_virtio_device_num_vq(VirtIOS390Device *dev)
>>       VirtIODevice *vdev = dev->vdev;
>>       int num_vq;
>>  
>>  -    for (num_vq = 0; num_vq < VIRTIO_PCI_QUEUE_MAX; num_vq++) {
>>  +    for (num_vq = 0; num_vq < virtio_get_queue_max(vdev); 
>> num_vq++) {
>>           if (!virtio_queue_get_num(vdev, num_vq)) {
>>               break;
>>           }
>>  @@ -438,7 +438,7 @@ VirtIOS390Device 
>> *s390_virtio_bus_find_vring(VirtIOS390Bus *bus,
>>       QTAILQ_FOREACH(kid, &bus->bus.children, sibling) {
>>           VirtIOS390Device *dev = (VirtIOS390Device *)kid->child;
>>  
>>  -        for(i = 0; i < VIRTIO_PCI_QUEUE_MAX; i++) {
>>  +        for(i = 0; i < virtio_get_queue_max(dev->vdev); i++) {
> 
> Again, I do not see any advantage over s/PCI/S390/ here.
> 
>>               if (!virtio_queue_get_addr(dev->vdev, i))
>>                   break;
>>               if (virtio_queue_get_addr(dev->vdev, i) == mem) {
> 
> (...)
> 
>>  diff --git a/include/hw/s390x/s390_flic.h 
>> b/include/hw/s390x/s390_flic.h
>>  index 489d73b..7a3ada2 100644
>>  --- a/include/hw/s390x/s390_flic.h
>>  +++ b/include/hw/s390x/s390_flic.h
>>  @@ -20,7 +20,7 @@
>>   typedef struct AdapterRoutes {
>>       AdapterInfo adapter;
>>       int num_routes;
>>  -    int gsi[VIRTIO_PCI_QUEUE_MAX];
>>  +    int gsi[VIRTIO_S390_QUEUE_MAX];
> 
> Adapter routes are only applicable for the ccw transport, not for the
> old s390 transport.

Sure, will fix this.

> 
> 
> (I'm also wondering whether this should be the generic limit instead.)

As you pointed out in V1, there will be more issues if we just increase 
the generic limit. So I switch to use per transport limit. Since the 
limit was not changed for both s390 and ccw, it should be ok.

>>   } AdapterRoutes;
>>  
>>   #define TYPE_S390_FLIC_COMMON "s390-flic"
> 
> 

Re: [Qemu-devel] [PATCH V2 06/11] virtio-s390: switch to bus specific queue limit

[Cornelia Huck]

On Fri, 27 Feb 2015 06:42:57 +0008
Jason Wang <jasowang@redhat.com> wrote:

> On Thu, Feb 26, 2015 at 9:05 PM, Cornelia Huck 
> <cornelia.huck@de.ibm.com> wrote:
> > On Thu, 26 Feb 2015 15:04:41 +0800
> > Jason Wang <jasowang@redhat.com> wrote:
> > 

> >>   typedef struct AdapterRoutes {
> >>       AdapterInfo adapter;
> >>       int num_routes;
> >>  -    int gsi[VIRTIO_PCI_QUEUE_MAX];
> >>  +    int gsi[VIRTIO_S390_QUEUE_MAX];
> > 
> > Adapter routes are only applicable for the ccw transport, not for the
> > old s390 transport.
> 
> Sure, will fix this.
> 
> > 
> > 
> > (I'm also wondering whether this should be the generic limit instead.)
> 
> As you pointed out in V1, there will be more issues if we just increase 
> the generic limit. So I switch to use per transport limit. Since the 
> limit was not changed for both s390 and ccw, it should be ok.

I'm just wondering how many gsis we want to support for adapter routes.
They were introduced for virtio-ccw, but recently s390 pci has started
to use them as well, so a virtio limit seems silly here. I'll switch
them to some kind of generic limit instead, I think.

> 
> >>   } AdapterRoutes;
> >>  
> >>   #define TYPE_S390_FLIC_COMMON "s390-flic"
> > 
> > 
> 

Re: [Qemu-devel] [PATCH V2 06/11] virtio-s390: switch to bus specific queue limit

[Jason Wang]

On Fri, Feb 27, 2015 at 5:49 PM, Cornelia Huck 
<cornelia.huck@de.ibm.com> wrote:
> On Fri, 27 Feb 2015 06:42:57 +0008
> Jason Wang <jasowang@redhat.com> wrote:
> 
>>  On Thu, Feb 26, 2015 at 9:05 PM, Cornelia Huck 
>>  <cornelia.huck@de.ibm.com> wrote:
>>  > On Thu, 26 Feb 2015 15:04:41 +0800
>>  > Jason Wang <jasowang@redhat.com> wrote:
>>  > 
> 
>>  >>   typedef struct AdapterRoutes {
>>  >>       AdapterInfo adapter;
>>  >>       int num_routes;
>>  >>  -    int gsi[VIRTIO_PCI_QUEUE_MAX];
>>  >>  +    int gsi[VIRTIO_S390_QUEUE_MAX];
>>  > 
>>  > Adapter routes are only applicable for the ccw transport, not for 
>> the
>>  > old s390 transport.
>>  
>>  Sure, will fix this.
>>  
>>  > 
>>  > 
>>  > (I'm also wondering whether this should be the generic limit 
>> instead.)
>>  
>>  As you pointed out in V1, there will be more issues if we just 
>> increase 
>>  the generic limit. So I switch to use per transport limit. Since 
>> the 
>>  limit was not changed for both s390 and ccw, it should be ok.
> 
> I'm just wondering how many gsis we want to support for adapter 
> routes.
> They were introduced for virtio-ccw, but recently s390 pci has started
> to use them as well, so a virtio limit seems silly here. I'll switch
> them to some kind of generic limit instead, I think.

Get your point. My understanding is you can do this on top of this 
series.

Thanks

Re: [Qemu-devel] [PATCH V2 06/11] virtio-s390: switch to bus specific queue limit

[Cornelia Huck]

On Sat, 28 Feb 2015 11:31:21 +0800
Jason Wang <jasowang@redhat.com> wrote:

> 
> 
> On Fri, Feb 27, 2015 at 5:49 PM, Cornelia Huck 
> <cornelia.huck@de.ibm.com> wrote:
> > On Fri, 27 Feb 2015 06:42:57 +0008
> > Jason Wang <jasowang@redhat.com> wrote:
> > 
> >>  On Thu, Feb 26, 2015 at 9:05 PM, Cornelia Huck 
> >>  <cornelia.huck@de.ibm.com> wrote:
> >>  > On Thu, 26 Feb 2015 15:04:41 +0800
> >>  > Jason Wang <jasowang@redhat.com> wrote:
> >>  > 
> > 
> >>  >>   typedef struct AdapterRoutes {
> >>  >>       AdapterInfo adapter;
> >>  >>       int num_routes;
> >>  >>  -    int gsi[VIRTIO_PCI_QUEUE_MAX];
> >>  >>  +    int gsi[VIRTIO_S390_QUEUE_MAX];
> >>  > 
> >>  > Adapter routes are only applicable for the ccw transport, not for 
> >> the
> >>  > old s390 transport.
> >>  
> >>  Sure, will fix this.
> >>  
> >>  > 
> >>  > 
> >>  > (I'm also wondering whether this should be the generic limit 
> >> instead.)
> >>  
> >>  As you pointed out in V1, there will be more issues if we just 
> >> increase 
> >>  the generic limit. So I switch to use per transport limit. Since 
> >> the 
> >>  limit was not changed for both s390 and ccw, it should be ok.
> > 
> > I'm just wondering how many gsis we want to support for adapter 
> > routes.
> > They were introduced for virtio-ccw, but recently s390 pci has started
> > to use them as well, so a virtio limit seems silly here. I'll switch
> > them to some kind of generic limit instead, I think.
> 
> Get your point. My understanding is you can do this on top of this 
> series.

Yup, that will work.

[Qemu-devel] [PATCH V2 07/11] virtio-mmio: switch to bus specific queue limit

[Jason Wang]

Cc: Anthony Liguori <aliguori@amazon.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio/virtio-mmio.c    | 6 +++---
 include/hw/virtio/virtio.h | 1 +
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/virtio-mmio.c b/hw/virtio/virtio-mmio.c
index ad03218..3be2ce8 100644
--- a/hw/virtio/virtio-mmio.c
+++ b/hw/virtio/virtio-mmio.c
@@ -237,7 +237,7 @@ static void virtio_mmio_write(void *opaque, hwaddr offset, uint64_t value,
                 proxy->guest_page_shift);
         break;
     case VIRTIO_MMIO_QUEUESEL:
-        if (value < VIRTIO_PCI_QUEUE_MAX) {
+	    if (value < virtio_get_queue_max(vdev)) {
             vdev->queue_sel = value;
         }
         break;
@@ -257,7 +257,7 @@ static void virtio_mmio_write(void *opaque, hwaddr offset, uint64_t value,
         }
         break;
     case VIRTIO_MMIO_QUEUENOTIFY:
-        if (value < VIRTIO_PCI_QUEUE_MAX) {
+        if (value < virtio_get_queue_max(vdev)) {
             virtio_queue_notify(vdev, value);
         }
         break;
@@ -403,7 +403,7 @@ static void virtio_mmio_bus_class_init(ObjectClass *klass, void *data)
     k->device_plugged = virtio_mmio_device_plugged;
     k->has_variable_vring_alignment = true;
     bus_class->max_dev = 1;
-    k->queue_max = VIRTIO_PCI_QUEUE_MAX;
+    k->queue_max = VIRTIO_MMIO_QUEUE_MAX;
 }
 
 static const TypeInfo virtio_mmio_bus_info = {
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 8f4da77..e538f5a 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -95,6 +95,7 @@ typedef struct VirtQueueElement
 #define VIRTIO_PCI_QUEUE_MAX 64
 #define VIRTIO_CCW_QUEUE_MAX 64
 #define VIRTIO_S390_QUEUE_MAX 64
+#define VIRTIO_MMIO_QUEUE_MAX 64
 
 #define VIRTIO_NO_VECTOR 0xffff
 
-- 
1.9.1

[Qemu-devel] [PATCH V2 08/11] virtio-pci: switch to use bus specific queue limit

[Jason Wang]

Instead of depending on a macro, switch to use a bus specific queue
limit.

Cc: Anthony Liguori <aliguori@amazon.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/virtio/virtio-pci.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 7fa8141..23c4649 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -215,7 +215,7 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
         return;
     }
 
-    for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
+    for (n = 0; n < virtio_get_queue_max(vdev); n++) {
         if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
@@ -251,7 +251,7 @@ static void virtio_pci_stop_ioeventfd(VirtIOPCIProxy *proxy)
         return;
     }
 
-    for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
+    for (n = 0; n < virtio_get_queue_max(vdev); n++) {
         if (!virtio_queue_get_num(vdev, n)) {
             continue;
         }
@@ -287,11 +287,11 @@ static void virtio_ioport_write(void *opaque, uint32_t addr, uint32_t val)
             virtio_queue_set_addr(vdev, vdev->queue_sel, pa);
         break;
     case VIRTIO_PCI_QUEUE_SEL:
-        if (val < VIRTIO_PCI_QUEUE_MAX)
+        if (val < virtio_get_queue_max(vdev))
             vdev->queue_sel = val;
         break;
     case VIRTIO_PCI_QUEUE_NOTIFY:
-        if (val < VIRTIO_PCI_QUEUE_MAX) {
+        if (val < virtio_get_queue_max(vdev)) {
             virtio_queue_notify(vdev, val);
         }
         break;
@@ -792,7 +792,7 @@ static int virtio_pci_set_guest_notifiers(DeviceState *d, int nvqs, bool assign)
     bool with_irqfd = msix_enabled(&proxy->pci_dev) &&
         kvm_msi_via_irqfd_enabled();
 
-    nvqs = MIN(nvqs, VIRTIO_PCI_QUEUE_MAX);
+    nvqs = MIN(nvqs, virtio_get_queue_max(vdev));
 
     /* When deassigning, pass a consistent nvqs value
      * to avoid leaking notifiers.
-- 
1.9.1

Re: [Qemu-devel] [PATCH V2 08/11] virtio-pci: switch to use bus specific queue limit

[Michael S. Tsirkin]

On Thu, Feb 26, 2015 at 03:04:43PM +0800, Jason Wang wrote:
> Instead of depending on a macro, switch to use a bus specific queue
> limit.
> 
> Cc: Anthony Liguori <aliguori@amazon.com>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Signed-off-by: Jason Wang <jasowang@redhat.com>
> ---
>  hw/virtio/virtio-pci.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 7fa8141..23c4649 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -215,7 +215,7 @@ static void virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
>          return;
>      }
>  
> -    for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
> +    for (n = 0; n < virtio_get_queue_max(vdev); n++) {
>          if (!virtio_queue_get_num(vdev, n)) {
>              continue;
>          }

This is done on guest IO, and I think after applying the
next patch which increases the number to >500 for pci, it's too much
work: VCPU is blocked meanwhile. Same applies to other places.

At minimum, we'll need a faster way to locate active VQs.


-- 
MST

Re: [Qemu-devel] [PATCH V2 08/11] virtio-pci: switch to use bus specific queue limit

[Jason Wang]

On Thu, Feb 26, 2015 at 6:20 PM, Michael S. Tsirkin <mst@redhat.com> 
wrote:
> On Thu, Feb 26, 2015 at 03:04:43PM +0800, Jason Wang wrote:
>>  Instead of depending on a macro, switch to use a bus specific queue
>>  limit.
>>  
>>  Cc: Anthony Liguori <aliguori@amazon.com>
>>  Cc: Michael S. Tsirkin <mst@redhat.com>
>>  Signed-off-by: Jason Wang <jasowang@redhat.com>
>>  ---
>>   hw/virtio/virtio-pci.c | 10 +++++-----
>>   1 file changed, 5 insertions(+), 5 deletions(-)
>>  
>>  diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
>>  index 7fa8141..23c4649 100644
>>  --- a/hw/virtio/virtio-pci.c
>>  +++ b/hw/virtio/virtio-pci.c
>>  @@ -215,7 +215,7 @@ static void 
>> virtio_pci_start_ioeventfd(VirtIOPCIProxy *proxy)
>>           return;
>>       }
>>   
>>  -    for (n = 0; n < VIRTIO_PCI_QUEUE_MAX; n++) {
>>  +    for (n = 0; n < virtio_get_queue_max(vdev); n++) {
>>           if (!virtio_queue_get_num(vdev, n)) {
>>               continue;
>>           }
> 
> This is done on guest IO, and I think after applying the
> next patch which increases the number to >500 for pci, it's too much
> work: VCPU is blocked meanwhile. Same applies to other places.
> 
> At minimum, we'll need a faster way to locate active VQs.
> 

Maybe just replace the continue with break in above. Since it looks 
like virtqueue index used by all virtio devices are contiguous?

[Qemu-devel] [PATCH V2 09/11] virtio-pci: increase the maximum number of virtqueues to 513

[Jason Wang]

This patch increases the maximum number of virtqueues for pci from 64
to 513. This will allow booting a virtio-net-pci device with 256 queue
pairs.

To keep migration compatibility, 64 was kept for legacy machine types.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Anthony Liguori <aliguori@amazon.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/i386/pc_piix.c          | 6 ++++++
 hw/i386/pc_q35.c           | 5 +++++
 include/hw/virtio/virtio.h | 2 +-
 3 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 38b42b0..821d44c 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -49,6 +49,7 @@
 #include "hw/acpi/acpi.h"
 #include "cpu.h"
 #include "qemu/error-report.h"
+#include "include/hw/virtio/virtio-bus.h"
 #ifdef CONFIG_XEN
 #  include <xen/hvm/hvm_info_table.h>
 #endif
@@ -310,6 +311,11 @@ static void pc_init_pci(MachineState *machine)
 
 static void pc_compat_2_2(MachineState *machine)
 {
+    ObjectClass *klass = object_class_by_name("virtio-pci-bus");
+    VirtioBusClass *k = VIRTIO_BUS_CLASS(klass);
+
+    k->queue_max = 64;
+
     x86_cpu_compat_set_features("kvm64", FEAT_1_EDX, 0, CPUID_VME);
     x86_cpu_compat_set_features("kvm32", FEAT_1_EDX, 0, CPUID_VME);
     x86_cpu_compat_set_features("Conroe", FEAT_1_EDX, 0, CPUID_VME);
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index 63027ee..dc5ada4 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -45,6 +45,7 @@
 #include "hw/usb.h"
 #include "hw/cpu/icc_bus.h"
 #include "qemu/error-report.h"
+#include "include/hw/virtio/virtio-bus.h"
 
 /* ICH9 AHCI has 6 ports */
 #define MAX_SATA_PORTS     6
@@ -289,6 +290,10 @@ static void pc_q35_init(MachineState *machine)
 
 static void pc_compat_2_2(MachineState *machine)
 {
+    ObjectClass *klass = object_class_by_name("virtio-pci-bus");
+    VirtioBusClass *k = VIRTIO_BUS_CLASS(klass);
+
+    k->queue_max = 64;
     x86_cpu_compat_set_features("kvm64", FEAT_1_EDX, 0, CPUID_VME);
     x86_cpu_compat_set_features("kvm32", FEAT_1_EDX, 0, CPUID_VME);
     x86_cpu_compat_set_features("Conroe", FEAT_1_EDX, 0, CPUID_VME);
diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index e538f5a..c653ad1 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -92,7 +92,7 @@ typedef struct VirtQueueElement
     struct iovec out_sg[VIRTQUEUE_MAX_SIZE];
 } VirtQueueElement;
 
-#define VIRTIO_PCI_QUEUE_MAX 64
+#define VIRTIO_PCI_QUEUE_MAX 513
 #define VIRTIO_CCW_QUEUE_MAX 64
 #define VIRTIO_S390_QUEUE_MAX 64
 #define VIRTIO_MMIO_QUEUE_MAX 64
-- 
1.9.1

[Qemu-devel] [PATCH V2 10/11] pci: remove hard-coded bar size in msix_init_exclusive_bar()

[Jason Wang]

Let msix_init_exclusive_bar() can accept bar_size parameter other than
a hard-coded limit 4096. Then caller can specify a bar_size depends on
msix entries and can use up to 2048 msix entries as PCI spec
allows. To keep migration compatibility, 4096 is used for all callers
and pba were start from half of bar size.

Cc: Keith Busch <keith.busch@intel.com>
Cc: Kevin Wolf <kwolf@redhat.com>
Cc: Stefan Hajnoczi <stefanha@redhat.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Anthony Liguori <aliguori@amazon.com>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/block/nvme.c        |  2 +-
 hw/misc/ivshmem.c      |  2 +-
 hw/pci/msix.c          | 18 +++++++-----------
 hw/virtio/virtio-pci.c |  2 +-
 include/hw/pci/msix.h  |  2 +-
 5 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index ce079ae..0fa1396 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -786,7 +786,7 @@ static int nvme_init(PCIDevice *pci_dev)
     pci_register_bar(&n->parent_obj, 0,
         PCI_BASE_ADDRESS_SPACE_MEMORY | PCI_BASE_ADDRESS_MEM_TYPE_64,
         &n->iomem);
-    msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4);
+    msix_init_exclusive_bar(&n->parent_obj, n->num_queues, 4, 4096);
 
     id->vid = cpu_to_le16(pci_get_word(pci_conf + PCI_VENDOR_ID));
     id->ssvid = cpu_to_le16(pci_get_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID));
diff --git a/hw/misc/ivshmem.c b/hw/misc/ivshmem.c
index 5d272c8..3e2a2d4 100644
--- a/hw/misc/ivshmem.c
+++ b/hw/misc/ivshmem.c
@@ -631,7 +631,7 @@ static uint64_t ivshmem_get_size(IVShmemState * s) {
 
 static void ivshmem_setup_msi(IVShmemState * s)
 {
-    if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1)) {
+    if (msix_init_exclusive_bar(PCI_DEVICE(s), s->vectors, 1, 4096)) {
         IVSHMEM_DPRINTF("msix initialization failed\n");
         exit(1);
     }
diff --git a/hw/pci/msix.c b/hw/pci/msix.c
index 24de260..9a1894f 100644
--- a/hw/pci/msix.c
+++ b/hw/pci/msix.c
@@ -291,33 +291,29 @@ int msix_init(struct PCIDevice *dev, unsigned short nentries,
 }
 
 int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries,
-                            uint8_t bar_nr)
+                            uint8_t bar_nr, uint32_t bar_size)
 {
     int ret;
     char *name;
+    uint32_t bar_pba_offset = bar_size / 2;
 
     /*
      * Migration compatibility dictates that this remains a 4k
      * BAR with the vector table in the lower half and PBA in
      * the upper half.  Do not use these elsewhere!
      */
-#define MSIX_EXCLUSIVE_BAR_SIZE 4096
-#define MSIX_EXCLUSIVE_BAR_TABLE_OFFSET 0
-#define MSIX_EXCLUSIVE_BAR_PBA_OFFSET (MSIX_EXCLUSIVE_BAR_SIZE / 2)
-#define MSIX_EXCLUSIVE_CAP_OFFSET 0
-
-    if (nentries * PCI_MSIX_ENTRY_SIZE > MSIX_EXCLUSIVE_BAR_PBA_OFFSET) {
+    if (nentries * PCI_MSIX_ENTRY_SIZE > bar_pba_offset) {
         return -EINVAL;
     }
 
     name = g_strdup_printf("%s-msix", dev->name);
-    memory_region_init(&dev->msix_exclusive_bar, OBJECT(dev), name, MSIX_EXCLUSIVE_BAR_SIZE);
+    memory_region_init(&dev->msix_exclusive_bar, OBJECT(dev), name, bar_size);
     g_free(name);
 
     ret = msix_init(dev, nentries, &dev->msix_exclusive_bar, bar_nr,
-                    MSIX_EXCLUSIVE_BAR_TABLE_OFFSET, &dev->msix_exclusive_bar,
-                    bar_nr, MSIX_EXCLUSIVE_BAR_PBA_OFFSET,
-                    MSIX_EXCLUSIVE_CAP_OFFSET);
+                    0, &dev->msix_exclusive_bar,
+                    bar_nr, bar_pba_offset,
+                    0);
     if (ret) {
         return ret;
     }
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index 23c4649..a287b2a 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -973,7 +973,7 @@ static void virtio_pci_device_plugged(DeviceState *d)
     config[PCI_INTERRUPT_PIN] = 1;
 
     if (proxy->nvectors &&
-        msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 1)) {
+        msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 1, 4096)) {
         error_report("unable to init msix vectors to %" PRIu32,
                      proxy->nvectors);
         proxy->nvectors = 0;
diff --git a/include/hw/pci/msix.h b/include/hw/pci/msix.h
index 954d82b..43edebc 100644
--- a/include/hw/pci/msix.h
+++ b/include/hw/pci/msix.h
@@ -11,7 +11,7 @@ int msix_init(PCIDevice *dev, unsigned short nentries,
               unsigned table_offset, MemoryRegion *pba_bar,
               uint8_t pba_bar_nr, unsigned pba_offset, uint8_t cap_pos);
 int msix_init_exclusive_bar(PCIDevice *dev, unsigned short nentries,
-                            uint8_t bar_nr);
+                            uint8_t bar_nr, uint32_t bar_size);
 
 void msix_write_config(PCIDevice *dev, uint32_t address, uint32_t val, int len);
 
-- 
1.9.1

[Qemu-devel] [PATCH V2 11/11] virtio-pci: introduce auto_msix_bar_size property

[Jason Wang]

This patch introduces boolean auto_msix_bar_size property for virito-pci
devices. Enable this will let the device calculate the msix bar size
based on the number of MSI-X entries instead of previous 4096
hard-coded limit.

This is a must to let virtio-net can up to 256 queues and each queue
were associated with a specific MSI-X entry.

Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Richard Henderson <rth@twiddle.net>
Cc: Anthony Liguori <aliguori@amazon.com>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Alexander Graf <agraf@suse.de>
Cc: qemu-ppc@nongnu.org
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 hw/i386/pc_piix.c      |  4 ++++
 hw/i386/pc_q35.c       |  4 ++++
 hw/ppc/spapr.c         |  5 +++++
 hw/virtio/virtio-pci.c | 17 +++++++++++++++--
 hw/virtio/virtio-pci.h |  3 +++
 include/hw/compat.h    |  8 ++++++++
 6 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/hw/i386/pc_piix.c b/hw/i386/pc_piix.c
index 821d44c..07bf0b4 100644
--- a/hw/i386/pc_piix.c
+++ b/hw/i386/pc_piix.c
@@ -537,6 +537,10 @@ static QEMUMachine pc_i440fx_machine_v2_2 = {
     PC_I440FX_2_2_MACHINE_OPTIONS,
     .name = "pc-i440fx-2.2",
     .init = pc_init_pci_2_2,
+    .compat_props = (GlobalProperty[]) {
+        HW_COMPAT_2_2,
+        { /* end of list */ }
+    },
 };
 
 #define PC_I440FX_2_1_MACHINE_OPTIONS                           \
diff --git a/hw/i386/pc_q35.c b/hw/i386/pc_q35.c
index dc5ada4..42799ab 100644
--- a/hw/i386/pc_q35.c
+++ b/hw/i386/pc_q35.c
@@ -433,6 +433,10 @@ static QEMUMachine pc_q35_machine_v2_2 = {
     PC_Q35_2_2_MACHINE_OPTIONS,
     .name = "pc-q35-2.2",
     .init = pc_q35_init_2_2,
+    .compat_props = (GlobalProperty[]) {
+        HW_COMPAT_2_2,
+        { /* end of list */ }
+    },
 };
 
 #define PC_Q35_2_1_MACHINE_OPTIONS                      \
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index b560459..b50d600 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -1777,11 +1777,16 @@ static const TypeInfo spapr_machine_2_1_info = {
 static void spapr_machine_2_2_class_init(ObjectClass *oc, void *data)
 {
     MachineClass *mc = MACHINE_CLASS(oc);
+    static GlobalProperty compat_props[] = {
+        HW_COMPAT_2_2,
+        { /* end of list */ }
+    };
 
     mc->name = "pseries-2.2";
     mc->desc = "pSeries Logical Partition (PAPR compliant) v2.2";
     mc->alias = "pseries";
     mc->is_default = 1;
+    mc->compat_props = compat_props;
 }
 
 static const TypeInfo spapr_machine_2_2_info = {
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
index a287b2a..fc16826 100644
--- a/hw/virtio/virtio-pci.c
+++ b/hw/virtio/virtio-pci.c
@@ -961,7 +961,7 @@ static void virtio_pci_device_plugged(DeviceState *d)
     VirtIOPCIProxy *proxy = VIRTIO_PCI(d);
     VirtioBusState *bus = &proxy->bus;
     uint8_t *config;
-    uint32_t size;
+    uint32_t size, bar_size;
 
     config = proxy->pci_dev.config;
     if (proxy->class_code) {
@@ -972,8 +972,19 @@ static void virtio_pci_device_plugged(DeviceState *d)
     pci_set_word(config + PCI_SUBSYSTEM_ID, virtio_bus_get_vdev_id(bus));
     config[PCI_INTERRUPT_PIN] = 1;
 
+    if (proxy->flags & VIRTIO_PCI_FLAG_AUTO_MSIX_SIZE) {
+        bar_size = proxy->nvectors * PCI_MSIX_ENTRY_SIZE * 2;
+        if (bar_size & (bar_size - 1)) {
+            bar_size = 1 << qemu_fls(bar_size);
+        }
+    } else {
+        /* For migration compatibility */
+        bar_size = 4096;
+    }
+
     if (proxy->nvectors &&
-        msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 1, 4096)) {
+        msix_init_exclusive_bar(&proxy->pci_dev, proxy->nvectors, 1,
+                                bar_size)) {
         error_report("unable to init msix vectors to %" PRIu32,
                      proxy->nvectors);
         proxy->nvectors = 0;
@@ -1418,6 +1429,8 @@ static const TypeInfo virtio_serial_pci_info = {
 static Property virtio_net_properties[] = {
     DEFINE_PROP_BIT("ioeventfd", VirtIOPCIProxy, flags,
                     VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT, false),
+    DEFINE_PROP_BIT("auto_msix_bar_size", VirtIOPCIProxy, flags,
+                    VIRTIO_PCI_FLAG_AUTO_MSIX_SIZE_BIT, true),
     DEFINE_PROP_UINT32("vectors", VirtIOPCIProxy, nvectors, 3),
     DEFINE_VIRTIO_NET_FEATURES(VirtIOPCIProxy, host_features),
     DEFINE_PROP_END_OF_LIST(),
diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
index 8873b6d..96dea82 100644
--- a/hw/virtio/virtio-pci.h
+++ b/hw/virtio/virtio-pci.h
@@ -62,6 +62,9 @@ typedef struct VirtioBusClass VirtioPCIBusClass;
  * vcpu thread using ioeventfd for some devices. */
 #define VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT 1
 #define VIRTIO_PCI_FLAG_USE_IOEVENTFD   (1 << VIRTIO_PCI_FLAG_USE_IOEVENTFD_BIT)
+#define VIRTIO_PCI_FLAG_AUTO_MSIX_SIZE_BIT 2
+#define VIRTIO_PCI_FLAG_AUTO_MSIX_SIZE \
+    (1 << VIRTIO_PCI_FLAG_AUTO_MSIX_SIZE_BIT)
 
 typedef struct {
     MSIMessage msg;
diff --git a/include/hw/compat.h b/include/hw/compat.h
index 313682a..3e5c5ae 100644
--- a/include/hw/compat.h
+++ b/include/hw/compat.h
@@ -1,7 +1,15 @@
 #ifndef HW_COMPAT_H
 #define HW_COMPAT_H
 
+#define HW_COMPAT_2_2 \
+        {\
+            .driver   = "virtio-net-pci",\
+            .property = "auto_msix_bar_size",\
+            .value    = "off",\
+        }
+
 #define HW_COMPAT_2_1 \
+        HW_COMPAT_2_2, \
         {\
             .driver   = "intel-hda",\
             .property = "old_msi_addr",\
-- 
1.9.1