| Message ID | 1425534531-6305-9-git-send-email-jasowang@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 5 Mar 2015 13:48:45 +0800 Jason Wang <jasowang@redhat.com> 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 | 12 +++++++----- > include/hw/virtio/virtio.h | 1 - > 2 files changed, 7 insertions(+), 6 deletions(-) > > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > index 04ad532..9a81edf 100644 > --- a/include/hw/virtio/virtio.h > +++ b/include/hw/virtio/virtio.h > @@ -92,7 +92,6 @@ typedef struct VirtQueueElement > struct iovec out_sg[VIRTQUEUE_MAX_SIZE]; > } VirtQueueElement; > > -#define VIRTIO_PCI_QUEUE_MAX 64 > #define VIRTIO_CCW_QUEUE_MAX 64 This hunk makes me think that the ccw limit shouldn't have been added here :) > > #define VIRTIO_NO_VECTOR 0xffff
On Fri, Mar 6, 2015 at 8:28 PM, Cornelia Huck <cornelia.huck@de.ibm.com> wrote: > On Thu, 5 Mar 2015 13:48:45 +0800 > Jason Wang <jasowang@redhat.com> 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 | 12 +++++++----- >> include/hw/virtio/virtio.h | 1 - >> 2 files changed, 7 insertions(+), 6 deletions(-) >> > >> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h >> index 04ad532..9a81edf 100644 >> --- a/include/hw/virtio/virtio.h >> +++ b/include/hw/virtio/virtio.hs >> @@ -92,7 +92,6 @@ typedef struct VirtQueueElement >> struct iovec out_sg[VIRTQUEUE_MAX_SIZE]; >> } VirtQueueElement; >> >> -#define VIRTIO_PCI_QUEUE_MAX 64 >> #define VIRTIO_CCW_QUEUE_MAX 64 > > This hunk makes me think that the ccw limit shouldn't have been added > here :) Yes, fail to find a common header at first try. But looks like I can move this in include/hw/s390x/s390_flic.h.
On Mon, 09 Mar 2015 15:32:51 +0800 Jason Wang <jasowang@redhat.com> wrote: > > > On Fri, Mar 6, 2015 at 8:28 PM, Cornelia Huck > <cornelia.huck@de.ibm.com> wrote: > > On Thu, 5 Mar 2015 13:48:45 +0800 > > Jason Wang <jasowang@redhat.com> 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 | 12 +++++++----- > >> include/hw/virtio/virtio.h | 1 - > >> 2 files changed, 7 insertions(+), 6 deletions(-) > >> > > > >> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > >> index 04ad532..9a81edf 100644 > >> --- a/include/hw/virtio/virtio.h > >> +++ b/include/hw/virtio/virtio.hs > >> @@ -92,7 +92,6 @@ typedef struct VirtQueueElement > >> struct iovec out_sg[VIRTQUEUE_MAX_SIZE]; > >> } VirtQueueElement; > >> > >> -#define VIRTIO_PCI_QUEUE_MAX 64 > >> #define VIRTIO_CCW_QUEUE_MAX 64 > > > > This hunk makes me think that the ccw limit shouldn't have been added > > here :) > > Yes, fail to find a common header at first try. But looks like I can > move this in include/hw/s390x/s390_flic.h. > Hm, I'm not 100% sure about that. Maybe if we introduce a #gsi define there and have virtio-ccw inherit that as queue limit? Having a virtio limit in the flic header feels a bit odd.
On Mon, Mar 9, 2015 at 4:04 PM, Cornelia Huck <cornelia.huck@de.ibm.com> wrote: > On Mon, 09 Mar 2015 15:32:51 +0800 > Jason Wang <jasowang@redhat.com> wrote: > >> >> >> On Fri, Mar 6, 2015 at 8:28 PM, Cornelia Huck >> <cornelia.huck@de.ibm.com> wrote: >> > On Thu, 5 Mar 2015 13:48:45 +0800 >> > Jason Wang <jasowang@redhat.com> 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 | 12 +++++++----- >> >> include/hw/virtio/virtio.h | 1 - >> >> 2 files changed, 7 insertions(+), 6 deletions(-) >> >> >> > >> >> diff --git a/include/hw/virtio/virtio.h >> b/include/hw/virtio/virtio.h >> >> index 04ad532..9a81edf 100644 >> >> --- a/include/hw/virtio/virtio.h >> >> +++ b/include/hw/virtio/virtio.hs >> >> @@ -92,7 +92,6 @@ typedef struct VirtQueueElement >> >> struct iovec out_sg[VIRTQUEUE_MAX_SIZE]; >> >> } VirtQueueElement; >> >> >> >> -#define VIRTIO_PCI_QUEUE_MAX 64 >> >> #define VIRTIO_CCW_QUEUE_MAX 64 >> > >> > This hunk makes me think that the ccw limit shouldn't have been >> added >> > here :) >> >> Yes, fail to find a common header at first try. But looks like I >> can >> move this in include/hw/s390x/s390_flic.h. >> > > Hm, I'm not 100% sure about that. Maybe if we introduce a #gsi define > there and have virtio-ccw inherit that as queue limit? Having a virtio > limit in the flic header feels a bit odd. > Ok, this sounds better. Will do this. Thanks
diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c index 7fa8141..a261515 100644 --- a/hw/virtio/virtio-pci.c +++ b/hw/virtio/virtio-pci.c @@ -86,6 +86,8 @@ * 12 is historical, and due to x86 page size. */ #define VIRTIO_PCI_QUEUE_ADDR_SHIFT 12 +#define VIRTIO_PCI_QUEUE_MAX 64 + static void virtio_pci_bus_new(VirtioBusState *bus, size_t bus_size, VirtIOPCIProxy *dev); @@ -215,7 +217,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 +253,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 +289,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 +794,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. diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index 04ad532..9a81edf 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -92,7 +92,6 @@ typedef struct VirtQueueElement struct iovec out_sg[VIRTQUEUE_MAX_SIZE]; } VirtQueueElement; -#define VIRTIO_PCI_QUEUE_MAX 64 #define VIRTIO_CCW_QUEUE_MAX 64 #define VIRTIO_NO_VECTOR 0xffff
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 | 12 +++++++----- include/hw/virtio/virtio.h | 1 - 2 files changed, 7 insertions(+), 6 deletions(-)