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

[Anthony Liguori]

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
>