[SLOF] [PATCH] virtio-scsi: initialize vring avail queue buffers

[Nikunj A Dadhania]

Laurent Vivier <lvivier at redhat.com> writes:

> 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.

Good find.

>
> 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().

That is done in setup-virt-queues, which will be late as
virtioscsi_init() is already accessing vq_avail.

> 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().


board-qemu/slof/virtio-scsi.fs


: virtio-scsi-init-and-scan  ( -- )
[...]
    virtiodev virtio-scsi-init
    0= IF
        setup-virt-queues
[...]
;

We will need to get rid of this in slof code.

>
> Signed-off-by: Laurent Vivier <lvivier at redhat.com>
> ---
>  lib/libvirtio/virtio-scsi.c | 32 +++++++++++++++++++-------------
>  1 file changed, 19 insertions(+), 13 deletions(-)
>
> diff --git a/lib/libvirtio/virtio-scsi.c b/lib/libvirtio/virtio-scsi.c
> index 36b62d1..4de5b39 100644
> --- a/lib/libvirtio/virtio-scsi.c
> +++ b/lib/libvirtio/virtio-scsi.c
> @@ -18,6 +18,10 @@
>  #include "virtio-internal.h"
>  #include "virtio-scsi.h"
>  
> +static struct vqs vq_ctrl;
> +static struct vqs vq_event;
> +static struct vqs vq_request;
> +
>  int virtioscsi_send(struct virtio_device *dev,
>  		    struct virtio_scsi_req_cmd *req,
>  		    struct virtio_scsi_resp_cmd *resp,
> @@ -99,9 +103,6 @@ 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;
>  	int status = VIRTIO_STAT_ACKNOWLEDGE;
>  
>  	/* Reset device */
> @@ -126,17 +127,22 @@ 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++;
> -	}
> +	vq_ctrl.avail->flags = virtio_cpu_to_modern16(dev,
> +                                                    VRING_AVAIL_F_NO_INTERRUPT);
> +	vq_ctrl.avail->idx = 0;
> +
> +	vq_event.avail->flags = virtio_cpu_to_modern16(dev,
> +                                                    VRING_AVAIL_F_NO_INTERRUPT);
> +	vq_event.avail->idx = 0;
> +
> +	vq_request.avail->flags = virtio_cpu_to_modern16(dev,
> +                                                    VRING_AVAIL_F_NO_INTERRUPT);
> +	vq_request.avail->idx = 0;
>  
>  	/* Tell HV that setup succeeded */
>  	status |= VIRTIO_STAT_DRIVER_OK;
> -- 
> 2.7.4
>
> _______________________________________________
> SLOF mailing list
> SLOF at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/slof