[SLOF] [PATCH v4 23/23] virtio-net: 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: 0x1041
> 
> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
> ---
>  board-qemu/slof/Makefile                |  1 +
>  board-qemu/slof/pci-device_1af4_1041.fs |  1 +
>  lib/libvirtio/virtio-net.c              | 92 +++++++++++++++++++++------------
>  lib/libvirtio/virtio-net.h              |  3 ++
>  4 files changed, 63 insertions(+), 34 deletions(-)
>  create mode 100644 board-qemu/slof/pci-device_1af4_1041.fs
> 
> diff --git a/board-qemu/slof/Makefile b/board-qemu/slof/Makefile
> index b7d923c..44706b1 100644
> --- a/board-qemu/slof/Makefile
> +++ b/board-qemu/slof/Makefile
> @@ -66,6 +66,7 @@ USB_FFS_FILES = \
>  
>  VIO_FFS_FILES = \
>  	$(SLOFBRDDIR)/pci-device_1af4_1000.fs \
> +	$(SLOFBRDDIR)/pci-device_1af4_1041.fs \
>  	$(SLOFBRDDIR)/pci-device_1af4_1001.fs \
>  	$(SLOFBRDDIR)/pci-device_1af4_1042.fs \
>  	$(SLOFBRDDIR)/pci-device_1af4_1004.fs \
> diff --git a/board-qemu/slof/pci-device_1af4_1041.fs b/board-qemu/slof/pci-device_1af4_1041.fs
> new file mode 100644
> index 0000000..5aefead
> --- /dev/null
> +++ b/board-qemu/slof/pci-device_1af4_1041.fs
> @@ -0,0 +1 @@
> +s" pci-device_1af4_1000.fs" included

Maybe also add a short comment in here that 1041 is the virtio-1.0
version of virtio-net ?

> diff --git a/lib/libvirtio/virtio-net.c b/lib/libvirtio/virtio-net.c
> index ffc6cf6..26396ce 100644
> --- a/lib/libvirtio/virtio-net.c
> +++ b/lib/libvirtio/virtio-net.c
> @@ -37,6 +37,8 @@
>  
>  #define sync()  asm volatile (" sync \n" ::: "memory")
>  
> +#define DRIVER_FEATURE_SUPPORT  (VIRTIO_NET_F_MAC | VIRTIO_F_VERSION_1)
> +
>  struct virtio_device virtiodev;
>  static struct vqs vq_rx;     /* Information about receive virtqueues */
>  static struct vqs vq_tx;     /* Information about transmit virtqueues */
> @@ -54,6 +56,16 @@ struct virtio_net_hdr {
>  
>  static unsigned int net_hdr_size;
>  
> +struct virtio_net_hdr_v1 {
> +	uint8_t  flags;
> +	uint8_t  gso_type;
> +	le16  hdr_len;
> +	le16  gso_size;
> +	le16  csum_start;
> +	le16  csum_offset;
> +	le16  num_buffers;
> +};
> +
>  static uint16_t last_rx_idx;	/* Last index in RX "used" ring */
>  
>  /**
> @@ -68,10 +80,7 @@ static int virtionet_init_pci(struct virtio_device *dev)
>  	if (!dev)
>  		return -1;
>  
> -	virtiodev.base = dev->base;
> -
> -	/* Keep it disabled until the driver is 1.0 capable */
> -	virtiodev.is_modern = false;
> +	memcpy(&virtiodev, dev, sizeof(struct virtio_device));
>  
>  	/* Reset device */
>  	virtio_reset_device(&virtiodev);
> @@ -116,9 +125,16 @@ static int virtionet_init(net_driver_t *driver)
>  	virtio_set_status(&virtiodev, status);
>  
>  	/* Device specific setup - we do not support special features right now */

I think the above comment is not appropriate anymore ... the code now
negotiates the VIRTIO_NET_F_MAC feature (which the driver depends on).
So I'd maybe simply remove that comment.

> -	virtio_set_guest_features(&virtiodev,  0);
> +	if (virtiodev.is_modern) {
> +		if (virtio_negotiate_guest_features(&virtiodev, DRIVER_FEATURE_SUPPORT))
> +			goto dev_error;
> +		net_hdr_size = sizeof(struct virtio_net_hdr_v1);
> +		virtio_get_status(&virtiodev, &status);
> +	} else {
> +		net_hdr_size = sizeof(struct virtio_net_hdr);
> +		virtio_set_guest_features(&virtiodev,  0);
> +	}
>  
> -	net_hdr_size = sizeof(struct virtio_net_hdr);
>  	/* Allocate memory for one transmit an multiple receive buffers */
>  	vq_rx.buf_mem = SLOF_alloc_mem((BUFFER_ENTRY_SIZE+net_hdr_size)
>  				   * RX_QUEUE_SIZE);

The rest of the patch looks fine to me.

 Thomas