[SLOF] [PATCH v4 22/23] virtio-blk: enable virtio 1.0

[Thomas Huth]

On 28.01.2016 11:24, Nikunj A Dadhania wrote:
> Also add a device file for non-transitional pci device id: 0x1042
> 
> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
> ---
...
> diff --git a/board-qemu/slof/pci-device_1af4_1042.fs b/board-qemu/slof/pci-device_1af4_1042.fs
> new file mode 100644
> index 0000000..5734865
> --- /dev/null
> +++ b/board-qemu/slof/pci-device_1af4_1042.fs
> @@ -0,0 +1 @@
> +s" pci-device_1af4_1001.fs" included

Maybe add a short comment that 1af4:1042 is the virtio-1.0 version of
virtio-block ?

> diff --git a/lib/libvirtio/virtio-blk.c b/lib/libvirtio/virtio-blk.c
> index a3d1df1..616b33f 100644
> --- a/lib/libvirtio/virtio-blk.c
> +++ b/lib/libvirtio/virtio-blk.c
...
> @@ -46,27 +44,35 @@ virtioblk_init(struct virtio_device *dev)
>  	status |= VIRTIO_STAT_DRIVER;
>  	virtio_set_status(dev, status);
>  
> -	/* Device specific setup - we support F_BLK_SIZE */
> -	virtio_set_guest_features(dev,  VIRTIO_BLK_F_BLK_SIZE);
> +	if (dev->is_modern) {
> +		/* Negotiate features and sets FEATURES_OK if successful */
> +		if (virtio_negotiate_guest_features(dev, DRIVER_FEATURE_SUPPORT))
> +			goto dev_error;
> +
> +		virtio_get_status(dev, &status);
> +	} else {
> +		/* Device specific setup - we support F_BLK_SIZE */
> +		virtio_set_guest_features(dev,  VIRTIO_BLK_F_BLK_SIZE);
> +	}
> +
> +	features = virtio_get_host_features(dev);
> +	if (features & VIRTIO_BLK_F_BLK_SIZE) {
> +		blk_size = virtio_get_config(dev,
> +					     offset_of(struct virtio_blk_cfg, blk_size),
> +					     sizeof(blk_size));
> +	}
>  
>  	if (virtio_queue_init_vq(dev, &vq, 0))
>  		goto dev_error;
>  
>  	vq_avail = virtio_get_vring_avail(dev, 0);
> -	vq_avail->flags = VRING_AVAIL_F_NO_INTERRUPT;
> +	vq_avail->flags = virtio_cpu_to_modern16(dev, VRING_AVAIL_F_NO_INTERRUPT);
>  	vq_avail->idx = 0;
>  
>  	/* Tell HV that setup succeeded */
>  	status |= VIRTIO_STAT_DRIVER_OK;
>  	virtio_set_status(dev, status);
>  
> -	features = virtio_get_host_features(dev);
> -	if (features & VIRTIO_BLK_F_BLK_SIZE) {
> -		blk_size = virtio_get_config(dev,
> -				offset_of(struct virtio_blk_cfg, blk_size),
> -				sizeof(blk_size));
> -	}

I think you could now leave the last 6 lines here, no need to move them
around anymore.

Apart from that, the patch looks fine to me now.

 Thomas