Message ID | 1485251078-10483-1-git-send-email-lvivier@redhat.com |
---|---|
State | Superseded |
Headers | show |
On 24.01.2017 10:44, Laurent Vivier wrote: > From: Laurent Vivier <lvivier@ibm-p8-virt-03.lab.eng.rdu.redhat.com> > > virtioscsi_init() uses the result of virtio_get_vring_avail() whereas > the queue is not enabled: on the first boot, its value is NULL and > the driver uses this to communicate with the device. After a reboot, > its value is the one given by the OS driver, and it works if the > address is in a range SLOF can access. > > In some cases, for instance with NUMA nodes and hotplugged memory, > SLOF cannot access the address set by the kernel, and virtioscsi_init() > fails with a data storage exception. > > To set the vring avail buffer address, we need to enable the queue, what > is done by virtio_set_qaddr(). > > This patch fixes the problem by calling virtio_queue_init_vq() (like other > virtio drivers) in virtioscsi_init() as it allocates memory and enables the > queue. virtio_queue_init_vq() also replaces the calls to virtio_vring_size() > and virtio_get_vring_avail(). > > Signed-off-by: Laurent Vivier <lvivier@redhat.com> > --- > v3: remove setup-virt-queues > > v2: move vqs declarations to the function > only call virtio_cpu_to_modern16() once > add CC: to Nikunj > > board-qemu/slof/virtio-scsi.fs | 17 ----------------- > lib/libvirtio/virtio-scsi.c | 28 +++++++++++++++------------- > 2 files changed, 15 insertions(+), 30 deletions(-) > > diff --git a/board-qemu/slof/virtio-scsi.fs b/board-qemu/slof/virtio-scsi.fs > index 4fedeee..d9f42ac 100644 > --- a/board-qemu/slof/virtio-scsi.fs > +++ b/board-qemu/slof/virtio-scsi.fs > @@ -169,22 +169,6 @@ scsi-close \ no further scsi words required > 0 VALUE queue-event-addr > 0 VALUE queue-cmd-addr > > -: setup-virt-queues > - \ add 3 queues 0-controlq, 1-eventq, 2-cmdq > - \ fixme: do we need to find more than the above 3 queues if exists > - virtiodev 0 virtio-get-qsize virtio-vring-size > - alloc-mem to queue-control-addr > - virtiodev 0 queue-control-addr virtio-set-qaddr > - > - virtiodev 1 virtio-get-qsize virtio-vring-size > - alloc-mem to queue-event-addr > - virtiodev 1 queue-event-addr virtio-set-qaddr > - > - virtiodev 2 virtio-get-qsize virtio-vring-size > - alloc-mem to queue-cmd-addr > - virtiodev 2 queue-cmd-addr virtio-set-qaddr > -; I think you can now also remove the queue-control-addr, queue-event-addr and the queue-cmd-addr variable right before that function. (and maybe also the virtio-get-qsize, virtio-vring-size and virtio-set-qaddr wrappers - but that should likely be done in a separate patch instead) Thomas
diff --git a/board-qemu/slof/virtio-scsi.fs b/board-qemu/slof/virtio-scsi.fs index 4fedeee..d9f42ac 100644 --- a/board-qemu/slof/virtio-scsi.fs +++ b/board-qemu/slof/virtio-scsi.fs @@ -169,22 +169,6 @@ scsi-close \ no further scsi words required 0 VALUE queue-event-addr 0 VALUE queue-cmd-addr -: setup-virt-queues - \ add 3 queues 0-controlq, 1-eventq, 2-cmdq - \ fixme: do we need to find more than the above 3 queues if exists - virtiodev 0 virtio-get-qsize virtio-vring-size - alloc-mem to queue-control-addr - virtiodev 0 queue-control-addr virtio-set-qaddr - - virtiodev 1 virtio-get-qsize virtio-vring-size - alloc-mem to queue-event-addr - virtiodev 1 queue-event-addr virtio-set-qaddr - - virtiodev 2 virtio-get-qsize virtio-vring-size - alloc-mem to queue-cmd-addr - virtiodev 2 queue-cmd-addr virtio-set-qaddr -; - \ Set scsi alias if none is set yet : setup-alias s" scsi" find-alias 0= IF @@ -212,7 +196,6 @@ scsi-close \ no further scsi words required \ Scan the VSCSI bus: virtiodev virtio-scsi-init 0= IF - setup-virt-queues scsi-find-disks setup-alias TRUE to initialized? diff --git a/lib/libvirtio/virtio-scsi.c b/lib/libvirtio/virtio-scsi.c index 36b62d1..e95352d 100644 --- a/lib/libvirtio/virtio-scsi.c +++ b/lib/libvirtio/virtio-scsi.c @@ -99,10 +99,9 @@ int virtioscsi_send(struct virtio_device *dev, */ int virtioscsi_init(struct virtio_device *dev) { - struct vring_avail *vq_avail; - unsigned int idx = 0; - int qsize = 0; + struct vqs vq_ctrl, vq_event, vq_request; int status = VIRTIO_STAT_ACKNOWLEDGE; + uint16_t flags; /* Reset device */ // XXX That will clear the virtq base. We need to move @@ -126,17 +125,20 @@ int virtioscsi_init(struct virtio_device *dev) virtio_set_guest_features(dev, 0); } - while(1) { - qsize = virtio_get_qsize(dev, idx); - if (!qsize) - break; - virtio_vring_size(qsize); + if (virtio_queue_init_vq(dev, &vq_ctrl, VIRTIO_SCSI_CONTROL_VQ) || + virtio_queue_init_vq(dev, &vq_event, VIRTIO_SCSI_EVENT_VQ) || + virtio_queue_init_vq(dev, &vq_request, VIRTIO_SCSI_REQUEST_VQ)) + goto dev_error; - vq_avail = virtio_get_vring_avail(dev, idx); - vq_avail->flags = virtio_cpu_to_modern16(dev, VRING_AVAIL_F_NO_INTERRUPT); - vq_avail->idx = 0; - idx++; - } + flags = virtio_cpu_to_modern16(dev, VRING_AVAIL_F_NO_INTERRUPT); + vq_ctrl.avail->flags = flags; + vq_ctrl.avail->idx = 0; + + vq_event.avail->flags = flags; + vq_event.avail->idx = 0; + + vq_request.avail->flags = flags; + vq_request.avail->idx = 0; /* Tell HV that setup succeeded */ status |= VIRTIO_STAT_DRIVER_OK;