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 
<address@hidden> wrote:
> On Thu, 26 Feb 2015 15:04:41 +0800
> Jason Wang <address@hidden> 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 <address@hidden>
> >  Cc: Richard Henderson <address@hidden>
> >  Cc: Christian Borntraeger <address@hidden>
> >  Cc: Cornelia Huck <address@hidden>
> >  Signed-off-by: Jason Wang <address@hidden>
> >  ---
> >   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"