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

Thomas Huth thuth at redhat.com
Tue Jan 24 21:57:03 AEDT 2017


On 24.01.2017 11:54, Laurent Vivier wrote:
> From: Laurent Vivier <lvivier at 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().
> ---
> v4: remove queue-control-addr, queue-event-addr, queue-cmd-addr
> 
> 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 | 21 ---------------------
>  lib/libvirtio/virtio-scsi.c    | 28 +++++++++++++++-------------
>  2 files changed, 15 insertions(+), 34 deletions(-)
> 
> diff --git a/board-qemu/slof/virtio-scsi.fs b/board-qemu/slof/virtio-scsi.fs
> index 4fedeee..d52741e 100644
> --- a/board-qemu/slof/virtio-scsi.fs
> +++ b/board-qemu/slof/virtio-scsi.fs
> @@ -165,26 +165,6 @@ scsi-open
>  
>  scsi-close        \ no further scsi words required
>  
> -0 VALUE queue-control-addr
> -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 +192,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;
> 

Reviewed-by: Thomas Huth <thuth at redhat.com>



More information about the SLOF mailing list