[SLOF] [PATCH v1 21/27] virtio-blk: enable virtio 1.0

Nikunj A Dadhania nikunj at linux.vnet.ibm.com
Thu Jan 14 22:17:06 AEDT 2016


Alexey Kardashevskiy <aik at ozlabs.ru> writes:

> On 01/13/2016 10:17 PM, Nikunj A Dadhania wrote:
>> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
>> ---
>>   lib/libvirtio/virtio-blk.c | 118 ++++++++++++++++++++++++++++++---------------
>>   1 file changed, 80 insertions(+), 38 deletions(-)
>>
>> diff --git a/lib/libvirtio/virtio-blk.c b/lib/libvirtio/virtio-blk.c
>> index 19819cf..e6d4040 100644
>> --- a/lib/libvirtio/virtio-blk.c
>> +++ b/lib/libvirtio/virtio-blk.c
>> @@ -13,10 +13,12 @@
>>   #include <stdio.h>
>>   #include <cpu.h>
>>   #include <helpers.h>
>> +#include <byteorder.h>
>>   #include "virtio.h"
>>   #include "virtio-blk.h"
>>
>>   #define DEFAULT_SECTOR_SIZE 512
>> +#define DRIVER_FEATURE_SUPPORT  (VIRTIO_BLK_F_BLK_SIZE | VIRTIO_F_VERSION_1)
>>
>>   struct vqs vq;
>>
>> @@ -30,41 +32,61 @@ virtioblk_init(struct virtio_device *dev)
>>   	struct vring_avail *vq_avail;
>>   	int blk_size = DEFAULT_SECTOR_SIZE;
>>   	int features;
>> +	unsigned int status = VIRTIO_STAT_ACKNOWLEDGE;
>>
>>   	/* Reset device */
>>   	virtio_reset_device(dev);
>>
>>   	/* Acknowledge device. */
>> -	virtio_set_status(dev, VIRTIO_STAT_ACKNOWLEDGE);
>> -
>> -	/* Tell HV that we know how to drive the device. */
>> -	virtio_set_status(dev, VIRTIO_STAT_ACKNOWLEDGE|VIRTIO_STAT_DRIVER);
>> -
>> -	/* Device specific setup - we support F_BLK_SIZE */
>> -	virtio_set_guest_features(dev,  VIRTIO_BLK_F_BLK_SIZE);
>> -
>> -	if(!virtio_queue_init_vq(dev, &vq, 0)) {
>> -		virtio_set_status(dev, VIRTIO_STAT_ACKNOWLEDGE|VIRTIO_STAT_DRIVER
>> -				  |VIRTIO_STAT_FAILED);
>> -		return 0;
>> +	virtio_set_status(dev, status);
>> +
>> +	/* Tell HV that we know how t drive the device. */
>
>
> s/how t /how to /

Sure.

>
>
>> +	status |= VIRTIO_STAT_DRIVER;
>> +	virtio_set_status(dev, status);
>> +
>> +	if (dev->is_modern) {
>> +		/* Negotiate features */
>> +		if(!virtio_negotiate_guest_features(dev, DRIVER_FEATURE_SUPPORT))
>> +			goto dev_error;
>> +
>> +		status |= VIRTIO_STAT_FEATURES_OK;
>> +		virtio_set_status(dev, status);
>> +
>> +		if (features & VIRTIO_BLK_F_BLK_SIZE) {
>> +			blk_size = virtio_get_config(dev,
>> +						     offset_of(struct virtio_blk_cfg, blk_size),
>> +						     sizeof(blk_size));
>> +		}
>> +
>> +	} else {
>> +		/* Device specific setup - we support F_BLK_SIZE */
>> +		virtio_set_guest_features(dev,  VIRTIO_BLK_F_BLK_SIZE);
>> +
>> +		virtio_get_host_features(dev, &features);
>> +		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 = dev->is_modern ? cpu_to_le16(VRING_AVAIL_F_NO_INTERRUPT) :
>> +		VRING_AVAIL_F_NO_INTERRUPT;
>>   	vq_avail->idx = 0;
>>
>>   	/* Tell HV that setup succeeded */
>> -	virtio_set_status(dev, VIRTIO_STAT_ACKNOWLEDGE|VIRTIO_STAT_DRIVER
>> -				|VIRTIO_STAT_DRIVER_OK);
>> -
>> -	virtio_get_host_features(dev, &features);
>> -	if (features & VIRTIO_BLK_F_BLK_SIZE) {
>> -		blk_size = virtio_get_config(dev,
>> -				offset_of(struct virtio_blk_cfg, blk_size),
>> -				sizeof(blk_size));
>> -	}
>> +	status |= VIRTIO_STAT_DRIVER_OK;
>> +	virtio_set_status(dev, status);
>>
>>   	return blk_size;
>> +dev_error:
>> +	fprintf(stderr, "Device Error \n");
>> +	status |= VIRTIO_STAT_FAILED;
>> +	virtio_set_status(dev, status);
>> +	return 0;
>>   }
>>
>>
>> @@ -82,12 +104,18 @@ virtioblk_shutdown(struct virtio_device *dev)
>>   	virtio_reset_device(dev);
>>   }
>>
>> -static void fill_blk_hdr(struct virtio_blk_req *blkhdr,
>> -			 uint32_t type, uint32_t ioprio, uint32_t sector)
>> +static void fill_blk_hdr(struct virtio_blk_req *blkhdr, uint32_t is_modern,
>> +                         uint32_t type, uint32_t ioprio, uint32_t sector)
>
>
>
> This prototype change could also be merged into 03/27, 20/27, etc. Also, 
> use "bool".

is_modern is not available at that point. Only comes after virtio_device
setup patches. Merging will jumble up the patches. Anyways, this patch also
does the conversion to virtio 1.0. So makes sense to have this here.

>
>
>>   {
>> -	blkhdr->type = type;
>> -	blkhdr->ioprio = ioprio;
>> -	blkhdr->sector = sector;
>> +	if (is_modern) {
>> +		blkhdr->type = cpu_to_le32(type);
>> +		blkhdr->ioprio = cpu_to_le32(ioprio);
>> +		blkhdr->sector = cpu_to_le64(sector);
>> +	} else {
>> +		blkhdr->type = type;
>> +		blkhdr->ioprio = ioprio;
>> +		blkhdr->sector = sector;
>> +	}
>>   }
>>
>>   /**
>> @@ -112,7 +140,7 @@ virtioblk_read(struct virtio_device *dev, char *buf, long blocknum, long cnt)
>>   	struct vring_used *vq_used;		/* "Used" vring */
>>   	volatile uint8_t status = -1;
>>   	volatile uint16_t *current_used_idx;
>> -	uint16_t last_used_idx;
>> +	uint16_t last_used_idx, avail_idx;
>>   	int blk_size = DEFAULT_SECTOR_SIZE;
>>
>>   	//printf("virtioblk_read: dev=%p buf=%p blocknum=%li count=%li\n",
>> @@ -140,33 +168,47 @@ virtioblk_read(struct virtio_device *dev, char *buf, long blocknum, long cnt)
>>   	vq_avail = virtio_get_vring_avail(dev, 0);
>>   	vq_used = virtio_get_vring_used(dev, 0);
>>
>> +	if (dev->is_modern)
>> +		avail_idx = le16_to_cpu(vq_avail->idx);
>> +	else
>> +		avail_idx = vq_avail->idx;
>> +
>>   	last_used_idx = vq_used->idx;
>>   	current_used_idx = &vq_used->idx;
>>
>>   	/* Set up header */
>> -	fill_blk_hdr(&blkhdr, VIRTIO_BLK_T_IN | VIRTIO_BLK_T_BARRIER,
>> +	fill_blk_hdr(&blkhdr, dev->is_modern, VIRTIO_BLK_T_IN | VIRTIO_BLK_T_BARRIER,
>>   		     1, blocknum * blk_size / DEFAULT_SECTOR_SIZE);
>>
>>   	/* Determine descriptor index */
>> -	id = (vq_avail->idx * 3) % vq_size;
>> +	id = (avail_idx * 3) % vq_size;
>>
>>   	/* Set up virtqueue descriptor for header */
>>   	desc = &vq_desc[id];
>> -	virtio_fill_desc(desc, 0, (uint64_t)&blkhdr, sizeof(struct virtio_blk_req),
>> -		  VRING_DESC_F_NEXT, (id + 1) % vq_size);
>> +	virtio_fill_desc(desc, dev->is_modern, (uint64_t)&blkhdr,
>> +			 sizeof(struct virtio_blk_req),
>> +			 VRING_DESC_F_NEXT, (id + 1) % vq_size);
>>
>>   	/* Set up virtqueue descriptor for data */
>>   	desc = &vq_desc[(id + 1) % vq_size];
>> -	virtio_fill_desc(desc, 0, (uint64_t)buf, cnt * blk_size,
>> -		  VRING_DESC_F_NEXT | VRING_DESC_F_WRITE, (id + 2) % vq_size);
>> +	virtio_fill_desc(desc, dev->is_modern, (uint64_t)buf, cnt * blk_size,
>> +			 VRING_DESC_F_NEXT | VRING_DESC_F_WRITE,
>> +			 (id + 2) % vq_size);
>>
>>   	/* Set up virtqueue descriptor for status */
>>   	desc = &vq_desc[(id + 2) % vq_size];
>> -	virtio_fill_desc(desc, 0, (uint64_t)&status, 1, VRING_DESC_F_WRITE, 0);
>> +	virtio_fill_desc(desc, dev->is_modern, (uint64_t)&status, 1,
>> +			 VRING_DESC_F_WRITE, 0);
>>
>> -	vq_avail->ring[vq_avail->idx % vq_size] = id;
>> -	mb();
>> -	vq_avail->idx += 1;
>> +	if (dev->is_modern) {
>> +		vq_avail->ring[avail_idx % vq_size] = cpu_to_le16(id);
>> +		mb();
>> +		vq_avail->idx = cpu_to_le16(avail_idx + 1);
>> +	} else {
>> +		vq_avail->ring[vq_avail->idx % vq_size] = id;
>> +		mb();
>> +		vq_avail->idx += 1;
>> +	}
>>
>>   	/* Tell HV that the queue is ready */
>>   	virtio_queue_notify(dev, 0);
>>
>
>
> -- 
> Alexey



More information about the SLOF mailing list