[Qemu-devel] Re: [PATCH 2/3] virtio network device

[Anthony Liguori <aliguori@us.ibm.com>]

Dor Laor wrote:
> Anthony Liguori wrote:
> Index: qemu/hw/virtio-net.c
> ===================================================================
> --- /dev/null	1970-01-01 00:00:00.000000000 +0000
> +++ qemu/hw/virtio-net.c	2007-12-04 14:17:37.000000000 -0600
>
> +
> +static void virtio_net_receive(void *opaque, const uint8_t *buf, int size)
> +{
> +    VirtIONet *n = opaque;
> +    VirtQueueElement elem;
> +    struct virtio_net_hdr *hdr;
> +    int offset, i;
> +
> +    /* FIXME: the drivers really need to set their status better */
> +    if (n->rx_vq->vring.avail == NULL) {
> +	n->can_receive = 0;
> +	return;
> +    }
> +
> +    if (virtqueue_pop(n->rx_vq, &elem) == 0) {
> +	/* wait until the guest adds some rx bufs */
> +	n->can_receive = 0;
> +	return;
> +    }
> +
> +    hdr = (void *)elem.in_sg[0].iov_base;
> +    hdr->flags = 0;
> +    hdr->gso_type = VIRTIO_NET_HDR_GSO_NONE;
> +
> +    /* copy in packet.  ugh */
> +    offset = 0;
> +    i = 1;
> +    while (offset < size && i < elem.in_num) {
> +	int len = MIN(elem.in_sg[i].iov_len, size - offset);
> +	memcpy(elem.in_sg[i].iov_base, buf + offset, len);
>
>
>
> Actually according to qemu's standard, one should use cpu_physical_memory_write/
> cpu_physical_memory_read functions.
> This is true also for reading the ring values.
>   

Yes, and unfortunately, cpu_physical_memory_{read,write} are copy 
interfaces.  We really don't want that for high speed I/O.

The only down-sides of not using cpu_physical_memory_{read,write} is 
that writes aren't dispatched to MMIO functions and dirty updating isn't 
handled automatically.  I do need to go through an audit the dirty 
updating.  It may be possible to do a 1-time check of MMIO memory and 
then have a fast and slow path.  I'm not sure how much it matter honestly.

Regards,

Anthony Liguori

>
>
> +	offset += len;
> +	i++;
> +    }
> +
> +    /* signal other side */
> +    virtqueue_push(n->rx_vq, &elem, sizeof(*hdr) + offset);
> +    virtio_notify(&n->vdev, n->rx_vq);
> +}
> +
> +/* TX */
> +static void virtio_net_handle_tx(VirtIODevice *vdev, VirtQueue *vq)
> +{
> +    VirtIONet *n = to_virtio_net(vdev);
> +    VirtQueueElement elem;
> +
> +    while (virtqueue_pop(vq, &elem)) {
> +	int i;
> +	size_t len = 0;
> +
> +	/* ignore the header for now */
> +	for (i = 1; i < elem.out_num; i++) {
> +	    qemu_send_packet(n->vc, elem.out_sg[i].iov_base,
> +			     elem.out_sg[i].iov_len);
> +	    len += elem.out_sg[i].iov_len;
> +	}
> +
> +	virtqueue_push(vq, &elem, sizeof(struct virtio_net_hdr) + len);
> +	virtio_notify(&n->vdev, vq);
> +    }
>
>
> The virtio_notify should be left out of the while loop to minimize 
> irq injection.
>
>
> +}
> +
> +void *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn)
> +{
> +    VirtIONet *n;
> +
> +    n = (VirtIONet *)virtio_init_pci(bus, "virtio-net", 6900, 0x1000,
> +				     0, VIRTIO_ID_NET,
> +				     0x02, 0x00, 0x00,
> +				     6, sizeof(VirtIONet));
> +
> +    n->vdev.update_config = virtio_net_update_config;
> +    n->vdev.get_features = virtio_net_get_features;
> +    n->rx_vq = virtio_add_queue(&n->vdev, 512, virtio_net_handle_rx);
> +    n->tx_vq = virtio_add_queue(&n->vdev, 128, virtio_net_handle_tx);
> +    n->can_receive = 0;
> +    memcpy(n->mac, nd->macaddr, 6);
> +    n->vc = qemu_new_vlan_client(nd->vlan, virtio_net_receive,
> +				 virtio_net_can_receive, n);
> +
> +    return &n->vdev;
> +}
> Index: qemu/hw/pc.h
> ===================================================================
> --- qemu.orig/hw/pc.h	2007-12-04 14:15:20.000000000 -0600
> +++ qemu/hw/pc.h	2007-12-04 14:43:57.000000000 -0600
> @@ -142,4 +142,9 @@
>
>  void isa_ne2000_init(int base, qemu_irq irq, NICInfo *nd);
>
> +/* virtio-net.c */
> +
> +void *virtio_net_init(PCIBus *bus, NICInfo *nd, int devfn);
> +
> +
>  #endif
>