Avi Kivity wrote:
Anthony Liguori wrote:
For merging virtio, I thought I'd try a different approach from the
fix out of tree and merge all at once approach I took with live migration.
What I would like to do is make some minimal changes to virtio in kvm-userspace
so that some form of virtio could be merged in upstream QEMU. We'll have to
maintain a small diff in the kvm-userspace tree until we work out some of the
issues, but I'd rather work out those issues in the QEMU tree instead of fixing
them all outside of the tree first.
The attached patch uses USE_KVM guards to separate KVM specific code. This is
not KVM code, per say, but rather routines that aren't mergable right now. I
think using the guards make it more clear and easier to deal with merges.
When we submit virtio to qemu-devel and eventually commit, we'll strip out any
ifdef USE_KVM block.
The only real significant change in this patch is the use of accessors for
the virtio queues. We've discussed patches like this before.
The point of this patch is to make no functional change in the KVM tree. I've
confirmed that performance does not regress in virtio-net and that both
virtio-blk and virtio-net seem to be functional.
Signed-off-by: Anthony Liguori <[EMAIL PROTECTED]>
diff --git a/qemu/hw/virtio-blk.c b/qemu/hw/virtio-blk.c
index 727119b..fb703eb 100644
--- a/qemu/hw/virtio-blk.c
+++ b/qemu/hw/virtio-blk.c
@@ -276,7 +276,9 @@ void *virtio_blk_init(PCIBus *bus, uint16_t vendor,
uint16_t device,
BlockDriverState *bs)
{
VirtIOBlock *s;
+#ifdef USE_KVM
int cylinders, heads, secs;
+#endif
static int virtio_blk_id;
s = (VirtIOBlock *)virtio_init_pci(bus, "virtio-blk", vendor, device,
@@ -290,9 +292,12 @@ void *virtio_blk_init(PCIBus *bus, uint16_t vendor,
uint16_t device,
s->vdev.get_features = virtio_blk_get_features;
s->vdev.reset = virtio_blk_reset;
s->bs = bs;
+#ifdef USE_KVM
bs->devfn = s->vdev.pci_dev.devfn;
+ /* FIXME this can definitely go in upstream QEMU */
bdrv_guess_geometry(s->bs, &cylinders, &heads, &secs);
bdrv_set_geometry_hint(s->bs, cylinders, heads, secs);
+#endif
Why not merge these bits prior to merging virtio? They aren't kvm
specific and would be good in mainline qemu.
I'd rather have a consumer of an interface before merging the actual
infrastructure.
static int virtio_net_can_receive(void *opaque)
{
VirtIONet *n = opaque;
- if (n->rx_vq->vring.avail == NULL ||
+ if (!virtio_queue_ready(n->rx_vq) ||
!(n->vdev.status & VIRTIO_CONFIG_S_DRIVER_OK))
return 0;
- if (n->rx_vq->vring.avail->idx == n->rx_vq->last_avail_idx) {
- n->rx_vq->vring.used->flags &= ~VRING_USED_F_NO_NOTIFY;
+ if (virtio_queue_empty(n->rx_vq)) {
+ virtio_queue_set_notification(n->rx_vq, 1);
return 0;
}
- n->rx_vq->vring.used->flags |= VRING_USED_F_NO_NOTIFY;
+ virtio_queue_set_notification(n->rx_vq, 0);
return 1;
}
This should be in a separate patch.
Sure.
+#ifdef USE_KVM
/* dhclient uses AF_PACKET but doesn't pass auxdata to the kernel so
* it never finds out that the packets don't have valid checksums. This
* causes dhclient to get upset. Fedora's carried a patch for ages to
@@ -181,6 +190,7 @@ static void work_around_broken_dhclient(struct
virtio_net_hdr *hdr,
hdr->flags &= ~VIRTIO_NET_HDR_F_NEEDS_CSUM;
}
}
+#endif
Is this kvm specific?
It is because the GSO stuff has not gone into upstream QEMU yet. I'd
rather merge a crippled virtio implementation, and then merge each of
the optimizations than vice-versa. Primarily because I'm concerned some
of these optimizations will require refactoring.
static void virtio_net_receive(void *opaque, const uint8_t *buf, int size)
{
@@ -190,6 +200,11 @@ static void virtio_net_receive(void *opaque, const uint8_t
*buf, int size)
int offset, i;
int total;
+#ifndef USE_KVM
+ if (!virtio_queue_ready(n->rx_vq))
+ return;
+#endif
+
Or this?
This is necessary because of the net optimizations, which yes, should go
upstream into QEMU.
Why not merge qemu_sendv_packet? Perhaps with the alternate code as the
body if some infrastructure in qemu is missing?
I will, but I'd rather have a consumer first.
+#ifdef USE_KVM
+ VRingDesc *desc;
+ VRingAvail *avail;
+ VRingUsed *used;
+#else
+ target_phys_addr_t desc;
+ target_phys_addr_t avail;
+ target_phys_addr_t used;
+#endif
+} VRing;
Stumped. Why?
In KVM, desc points directly to guest memory. The non-KVM accessors use
stl/ldl et al so they only need the physical address base.
If you agree with this approach, I'll clean up the patch and send again.
Regards,
Anthony Liguori
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
