| Message ID | 1424934286-7099-7-git-send-email-jasowang@redhat.com |
|---|---|
| State | New |
| Headers | show |
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"
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" > >
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" > > > > >
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
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.
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
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(-)