[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 3/6] virtio for QEMU
From: |
Dor Laor |
Subject: |
Re: [Qemu-devel] [PATCH 3/6] virtio for QEMU |
Date: |
Sun, 30 Mar 2008 20:25:41 +0300 |
On Sat, 2008-03-29 at 16:55 -0500, Anthony Liguori wrote:
> This patch introduces virtio support over PCI. virtio is a generic virtual IO
> framework for Linux first introduced in 2.6.23. Since 2.6.25, virtio has
> supported a PCI transport which this patch implements.
>
> Since the last time these patches were posted to qemu-devel, I've reworked it
> to use the proper access functions to manipulate guest memory.
>
> Signed-off-by: Anthony Liguori <address@hidden>
It's will be great to drop the nasty hacks :)
Do you still get 1G net performance using the extra copy from tap
(memcpy_to_iovector)?
[snip]
> +static uint32_t vring_desc_len(VirtQueue *vq, unsigned int i)
> +{
Below there were place you did use offsetof(vq->vring.desc[i], len) so
we better be consistent + its nicer
> + return ldl_phys(vq->vring.desc + i * sizeof(VRingDesc) +
> + offsetof(VRingDesc, len));
> +}
> +
[snip]
> +VirtQueueElement *virtqueue_pop(VirtQueue *vq)
> +{
> + unsigned int i, head;
> + unsigned int position;
> + VirtQueueElement *elem;
> +
> + /* Check it isn't doing very strange things with descriptor numbers. */
> + if ((uint16_t)(vring_avail_idx(vq) - vq->last_avail_idx) > vq->vring.num)
> + errx(1, "Guest moved used index from %u to %u",
> + vq->last_avail_idx, vring_avail_idx(vq));
> +
> + /* If there's nothing new since last we looked, return invalid. */
> + if (vring_avail_idx(vq) == vq->last_avail_idx)
> + return NULL;
> +
> + /* Grab the next descriptor number they're advertising, and increment
> + * the index we've seen. */
> + head = vring_avail_ring(vq, vq->last_avail_idx++ % vq->vring.num);
> +
> + /* If their number is silly, that's a fatal mistake. */
> + if (head >= vq->vring.num)
> + errx(1, "Guest says index %u is available", head);
> +
> + /* When we start there are none of either input nor output. */
> + position = 0;
> +
> + elem = qemu_mallocz(sizeof(VirtQueueElement));
> +
> + elem->phys_in = qemu_mallocz(sizeof(PhysIOVector) +
> + vq->vring.num * sizeof(PhysIOVectorElement));
> + elem->phys_out = qemu_mallocz(sizeof(PhysIOVector) +
> + vq->vring.num * sizeof(PhysIOVectorElement));
I was wondering whether it can be optimized since vring.num is sometimes
512 so and we can either use a pool of these or calculate the vring.num
from the descriptors but it seems like your way is the best.
> +
> + i = head;
> + do {
> + PhysIOVectorElement *sge;
> +
> + if (vring_desc_flags(vq, i) & VRING_DESC_F_WRITE)
> + sge = &elem->phys_in->sg[elem->phys_in->num++];
> + else
> + sge = &elem->phys_out->sg[elem->phys_out->num++];
> +
> + /* Grab the first descriptor, and check it's OK. */
> + sge->len = vring_desc_len(vq, i);
> + sge->base = vring_desc_addr(vq, i);
> +
> + /* If we've got too many, that implies a descriptor loop. */
> + if ((elem->phys_in->num + elem->phys_out->num) > vq->vring.num)
> + errx(1, "Looped descriptor");
> + } while ((i = virtqueue_next_desc(vq, i)) != vq->vring.num);
> +
> + elem->virt_in = pci_device_dma_map(&vq->vdev->pci_dev, elem->phys_in);
> + elem->virt_out = pci_device_dma_map(&vq->vdev->pci_dev, elem->phys_out);
> + elem->index = head;
> +
> + if (elem->virt_in == NULL || elem->virt_out == NULL)
> + errx(1, "Bad DMA");
> +
> + return elem;
> +}
> +
> +
The name below is a bit misleading since when enable is true you
actually set no_notify.
So I name it something like virtio_vring_set_no_notify(...) or similar.
> +void virtio_ring_set_used_notify(VirtQueue *vq, int enable)
> +{
> + if (enable)
> + vring_used_set_flag(vq, VRING_USED_F_NO_NOTIFY);
> + else
> + vring_used_unset_flag(vq, VRING_USED_F_NO_NOTIFY);
> +}
> +
Cheers,
Dor
Re: [Qemu-devel] [PATCH 2/6] PCI DMA API, Paul Brook, 2008/03/30