* [Qemu-devel] [PATCH 0/2] dataplane: use endian-aware memory accessors @ 2015-01-19 17:04 Stefan Hajnoczi 2015-01-19 17:04 ` [Qemu-devel] [PATCH 1/2] dataplane: move vring_more_avail() into vring.c Stefan Hajnoczi ` (3 more replies) 0 siblings, 4 replies; 8+ messages in thread From: Stefan Hajnoczi @ 2015-01-19 17:04 UTC (permalink / raw) To: qemu-devel; +Cc: David Gibson, Stefan Hajnoczi In commit 0f5d1d2a49778863db54b4b1ac2dc008a8f21f11 ("virtio: memory accessors for endian-ambivalent targets") endian-aware memory accessors were added to support bi-endian targets like recent ppc64 systems. The dataplane vring.c code does not use these accessors and is therefore unable to emulate virtio devices when the endianness differs between the device and host. These patches modify vring.c to use the endian-aware accessors. I have tested that x86_64 guests still function correctly. I have only compile-tested on ppc64 and require help testing that dataplane works. If you have access to a bi-endian system, please try: qemu ... -object iothread,id=iothread0 \ -drive if=none,id=drive0,file=... \ -device virtio-blk-pci,iothread=iothread0,drive=drive0 Stefan Hajnoczi (2): dataplane: move vring_more_avail() into vring.c dataplane: use virtio_ld/st_p() for endian-aware memory access hw/block/dataplane/virtio-blk.c | 2 +- hw/scsi/virtio-scsi-dataplane.c | 2 +- hw/virtio/Makefile.objs | 2 +- hw/virtio/dataplane/Makefile.objs | 2 +- hw/virtio/dataplane/vring.c | 69 ++++++++++++++++++++++++++----------- include/hw/virtio/dataplane/vring.h | 9 ++--- 6 files changed, 55 insertions(+), 31 deletions(-) -- 2.1.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 1/2] dataplane: move vring_more_avail() into vring.c 2015-01-19 17:04 [Qemu-devel] [PATCH 0/2] dataplane: use endian-aware memory accessors Stefan Hajnoczi @ 2015-01-19 17:04 ` Stefan Hajnoczi 2015-01-20 23:56 ` David Gibson 2015-01-21 13:25 ` Greg Kurz 2015-01-19 17:04 ` [Qemu-devel] [PATCH 2/2] dataplane: use virtio_ld/st_p() for endian-aware memory access Stefan Hajnoczi ` (2 subsequent siblings) 3 siblings, 2 replies; 8+ messages in thread From: Stefan Hajnoczi @ 2015-01-19 17:04 UTC (permalink / raw) To: qemu-devel; +Cc: David Gibson, Stefan Hajnoczi vring_more_avail() was an inline function in vring.h. No external callers use it so it's not necessary to export it. Furthermore, we'll need virtio-access.h for endian-aware memory accesses but that only works in per-target object files (obj-y) and not build-once object files (common-obj-y) like the virtio-blk and virtio-scsi devices. Move vring_more_avail() into vring.c so that virtio devices like virtio-blk and virtio-scsi can continue to use vring.h without being built once per target. Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/virtio/dataplane/vring.c | 6 ++++++ include/hw/virtio/dataplane/vring.h | 6 ------ 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c index 61f6d83..cb31d7f 100644 --- a/hw/virtio/dataplane/vring.c +++ b/hw/virtio/dataplane/vring.c @@ -21,6 +21,12 @@ #include "hw/virtio/dataplane/vring.h" #include "qemu/error-report.h" +/* Are there more descriptors available? */ +static inline bool vring_more_avail(Vring *vring) +{ + return vring->vr.avail->idx != vring->last_avail_idx; +} + /* vring_map can be coupled with vring_unmap or (if you still have the * value returned in *mr) memory_region_unref. */ diff --git a/include/hw/virtio/dataplane/vring.h b/include/hw/virtio/dataplane/vring.h index d3e086a..1e871e6 100644 --- a/include/hw/virtio/dataplane/vring.h +++ b/include/hw/virtio/dataplane/vring.h @@ -36,12 +36,6 @@ static inline unsigned int vring_get_num(Vring *vring) return vring->vr.num; } -/* Are there more descriptors available? */ -static inline bool vring_more_avail(Vring *vring) -{ - return vring->vr.avail->idx != vring->last_avail_idx; -} - /* Fail future vring_pop() and vring_push() calls until reset */ static inline void vring_set_broken(Vring *vring) { -- 2.1.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] dataplane: move vring_more_avail() into vring.c 2015-01-19 17:04 ` [Qemu-devel] [PATCH 1/2] dataplane: move vring_more_avail() into vring.c Stefan Hajnoczi @ 2015-01-20 23:56 ` David Gibson 2015-01-21 13:25 ` Greg Kurz 1 sibling, 0 replies; 8+ messages in thread From: David Gibson @ 2015-01-20 23:56 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: qemu-devel [-- Attachment #1: Type: text/plain, Size: 991 bytes --] On Mon, Jan 19, 2015 at 05:04:36PM +0000, Stefan Hajnoczi wrote: > vring_more_avail() was an inline function in vring.h. No external > callers use it so it's not necessary to export it. > > Furthermore, we'll need virtio-access.h for endian-aware memory accesses > but that only works in per-target object files (obj-y) and not > build-once object files (common-obj-y) like the virtio-blk and > virtio-scsi devices. > > Move vring_more_avail() into vring.c so that virtio devices like > virtio-blk and virtio-scsi can continue to use vring.h without being > built once per target. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Looks sensible to me. Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Tested-by: David Gibson <david@gibson.dropbear.id.au> -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 1/2] dataplane: move vring_more_avail() into vring.c 2015-01-19 17:04 ` [Qemu-devel] [PATCH 1/2] dataplane: move vring_more_avail() into vring.c Stefan Hajnoczi 2015-01-20 23:56 ` David Gibson @ 2015-01-21 13:25 ` Greg Kurz 1 sibling, 0 replies; 8+ messages in thread From: Greg Kurz @ 2015-01-21 13:25 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: qemu-devel, David Gibson On Mon, 19 Jan 2015 17:04:36 +0000 Stefan Hajnoczi <stefanha@redhat.com> wrote: > vring_more_avail() was an inline function in vring.h. No external > callers use it so it's not necessary to export it. > > Furthermore, we'll need virtio-access.h for endian-aware memory accesses > but that only works in per-target object files (obj-y) and not > build-once object files (common-obj-y) like the virtio-blk and > virtio-scsi devices. > > Move vring_more_avail() into vring.c so that virtio devices like > virtio-blk and virtio-scsi can continue to use vring.h without being > built once per target. > > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> > --- Yes, this function should definitely be declared in more appropriate scope. And BTW I remember Cornelia had the same concern in the virtio-1 serie. :) Reviewed-by: Greg Kurz <gkurz@linux.vnet.ibm.com> > hw/virtio/dataplane/vring.c | 6 ++++++ > include/hw/virtio/dataplane/vring.h | 6 ------ > 2 files changed, 6 insertions(+), 6 deletions(-) > > diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c > index 61f6d83..cb31d7f 100644 > --- a/hw/virtio/dataplane/vring.c > +++ b/hw/virtio/dataplane/vring.c > @@ -21,6 +21,12 @@ > #include "hw/virtio/dataplane/vring.h" > #include "qemu/error-report.h" > > +/* Are there more descriptors available? */ > +static inline bool vring_more_avail(Vring *vring) > +{ > + return vring->vr.avail->idx != vring->last_avail_idx; > +} > + > /* vring_map can be coupled with vring_unmap or (if you still have the > * value returned in *mr) memory_region_unref. > */ > diff --git a/include/hw/virtio/dataplane/vring.h b/include/hw/virtio/dataplane/vring.h > index d3e086a..1e871e6 100644 > --- a/include/hw/virtio/dataplane/vring.h > +++ b/include/hw/virtio/dataplane/vring.h > @@ -36,12 +36,6 @@ static inline unsigned int vring_get_num(Vring *vring) > return vring->vr.num; > } > > -/* Are there more descriptors available? */ > -static inline bool vring_more_avail(Vring *vring) > -{ > - return vring->vr.avail->idx != vring->last_avail_idx; > -} > - > /* Fail future vring_pop() and vring_push() calls until reset */ > static inline void vring_set_broken(Vring *vring) > { ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH 2/2] dataplane: use virtio_ld/st_p() for endian-aware memory access 2015-01-19 17:04 [Qemu-devel] [PATCH 0/2] dataplane: use endian-aware memory accessors Stefan Hajnoczi 2015-01-19 17:04 ` [Qemu-devel] [PATCH 1/2] dataplane: move vring_more_avail() into vring.c Stefan Hajnoczi @ 2015-01-19 17:04 ` Stefan Hajnoczi 2015-01-21 0:17 ` David Gibson 2015-01-19 17:33 ` [Qemu-devel] [PATCH 0/2] dataplane: use endian-aware memory accessors Cornelia Huck 2015-01-21 13:30 ` Stefan Hajnoczi 3 siblings, 1 reply; 8+ messages in thread From: Stefan Hajnoczi @ 2015-01-19 17:04 UTC (permalink / raw) To: qemu-devel; +Cc: Paolo Bonzini, David Gibson, Fam Zheng, Stefan Hajnoczi The vring.c code was written under the assumption that virtio devices have the same endianness as the host. This is wrong when emulating targets that differ in endianness from the host. It is also wrong when emulating bi-endian targets like POWER 8 processors, which support both little- and big-endian at run-time. In this case the virtio device knows which endianness to use. This change requires the virtio-access.h APIs and therefore builds vring.o for each target (obj-y instead of common-obj-y). Note that $(CONFIG_VIRTIO) for dataplane/ was dropped in hw/virtio/Makefile.objs because hw/Makefile.objs conditionally includes hw/virtio/ on $(CONFIG_VIRTIO) already. Only a small change is needed to the vring.h interface: vring_push() now takes a VirtIODevice *vdev argument. Cc: Paolo Bonzini <pbonzini@redhat.com> Cc: Fam Zheng <famz@redhat.com> Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> --- hw/block/dataplane/virtio-blk.c | 2 +- hw/scsi/virtio-scsi-dataplane.c | 2 +- hw/virtio/Makefile.objs | 2 +- hw/virtio/dataplane/Makefile.objs | 2 +- hw/virtio/dataplane/vring.c | 67 +++++++++++++++++++++++++------------ include/hw/virtio/dataplane/vring.h | 3 +- 6 files changed, 51 insertions(+), 27 deletions(-) diff --git a/hw/block/dataplane/virtio-blk.c b/hw/block/dataplane/virtio-blk.c index 39c5d71..aca2a8d 100644 --- a/hw/block/dataplane/virtio-blk.c +++ b/hw/block/dataplane/virtio-blk.c @@ -75,7 +75,7 @@ static void complete_request_vring(VirtIOBlockReq *req, unsigned char status) VirtIOBlockDataPlane *s = req->dev->dataplane; stb_p(&req->in->status, status); - vring_push(&req->dev->dataplane->vring, &req->elem, + vring_push(s->vdev, &req->dev->dataplane->vring, &req->elem, req->qiov.size + sizeof(*req->in)); /* Suppress notification to guest by BH and its scheduled diff --git a/hw/scsi/virtio-scsi-dataplane.c b/hw/scsi/virtio-scsi-dataplane.c index 03a1e8c..418d73b 100644 --- a/hw/scsi/virtio-scsi-dataplane.c +++ b/hw/scsi/virtio-scsi-dataplane.c @@ -94,7 +94,7 @@ void virtio_scsi_vring_push_notify(VirtIOSCSIReq *req) { VirtIODevice *vdev = VIRTIO_DEVICE(req->vring->parent); - vring_push(&req->vring->vring, &req->elem, + vring_push(vdev, &req->vring->vring, &req->elem, req->qsgl.size + req->resp_iov.size); if (vring_should_notify(vdev, &req->vring->vring)) { diff --git a/hw/virtio/Makefile.objs b/hw/virtio/Makefile.objs index d21c397..3e93a3a 100644 --- a/hw/virtio/Makefile.objs +++ b/hw/virtio/Makefile.objs @@ -2,7 +2,7 @@ common-obj-y += virtio-rng.o common-obj-$(CONFIG_VIRTIO_PCI) += virtio-pci.o common-obj-y += virtio-bus.o common-obj-y += virtio-mmio.o -common-obj-$(CONFIG_VIRTIO) += dataplane/ obj-y += virtio.o virtio-balloon.o obj-$(CONFIG_LINUX) += vhost.o vhost-backend.o vhost-user.o +obj-y += dataplane/ diff --git a/hw/virtio/dataplane/Makefile.objs b/hw/virtio/dataplane/Makefile.objs index 9a8cfc0..753a9ca 100644 --- a/hw/virtio/dataplane/Makefile.objs +++ b/hw/virtio/dataplane/Makefile.objs @@ -1 +1 @@ -common-obj-y += vring.o +obj-y += vring.o diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c index cb31d7f..a36dc19 100644 --- a/hw/virtio/dataplane/vring.c +++ b/hw/virtio/dataplane/vring.c @@ -18,13 +18,14 @@ #include "hw/hw.h" #include "exec/memory.h" #include "exec/address-spaces.h" +#include "hw/virtio/virtio-access.h" #include "hw/virtio/dataplane/vring.h" #include "qemu/error-report.h" /* Are there more descriptors available? */ -static inline bool vring_more_avail(Vring *vring) +static inline bool vring_more_avail(VirtIODevice *vdev, Vring *vring) { - return vring->vr.avail->idx != vring->last_avail_idx; + return virtio_lduw_p(vdev, &vring->vr.avail->idx) != vring->last_avail_idx; } /* vring_map can be coupled with vring_unmap or (if you still have the @@ -89,7 +90,7 @@ bool vring_setup(Vring *vring, VirtIODevice *vdev, int n) vring_init(&vring->vr, virtio_queue_get_num(vdev, n), vring_ptr, 4096); vring->last_avail_idx = virtio_queue_get_last_avail_idx(vdev, n); - vring->last_used_idx = vring->vr.used->idx; + vring->last_used_idx = virtio_lduw_p(vdev, &vring->vr.used->idx); vring->signalled_used = 0; vring->signalled_used_valid = false; @@ -110,7 +111,9 @@ void vring_teardown(Vring *vring, VirtIODevice *vdev, int n) void vring_disable_notification(VirtIODevice *vdev, Vring *vring) { if (!(vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX))) { - vring->vr.used->flags |= VRING_USED_F_NO_NOTIFY; + uint16_t flags = virtio_lduw_p(vdev, &vring->vr.used->flags); + virtio_stw_p(vdev, &vring->vr.used->flags, + flags | VRING_USED_F_NO_NOTIFY); } } @@ -121,18 +124,21 @@ void vring_disable_notification(VirtIODevice *vdev, Vring *vring) bool vring_enable_notification(VirtIODevice *vdev, Vring *vring) { if (vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX)) { - vring_avail_event(&vring->vr) = vring->vr.avail->idx; + virtio_stw_p(vdev, &vring_avail_event(&vring->vr), + virtio_lduw_p(vdev, &vring->vr.avail->idx)); } else { - vring->vr.used->flags &= ~VRING_USED_F_NO_NOTIFY; + uint16_t flags = virtio_lduw_p(vdev, &vring->vr.used->flags); + virtio_stw_p(vdev, &vring->vr.used->flags, + flags & ~VRING_USED_F_NO_NOTIFY); } smp_mb(); /* ensure update is seen before reading avail_idx */ - return !vring_more_avail(vring); + return !vring_more_avail(vdev, vring); } /* This is stolen from linux/drivers/vhost/vhost.c:vhost_notify() */ bool vring_should_notify(VirtIODevice *vdev, Vring *vring) { - uint16_t old, new; + uint16_t used_event_idx, old, new; bool v; /* Flush out used index updates. This is paired * with the barrier that the Guest executes when enabling @@ -140,12 +146,14 @@ bool vring_should_notify(VirtIODevice *vdev, Vring *vring) smp_mb(); if ((vdev->guest_features & VIRTIO_F_NOTIFY_ON_EMPTY) && - unlikely(vring->vr.avail->idx == vring->last_avail_idx)) { + unlikely(virtio_lduw_p(vdev, &vring->vr.avail->idx) == + vring->last_avail_idx)) { return true; } if (!(vdev->guest_features & VIRTIO_RING_F_EVENT_IDX)) { - return !(vring->vr.avail->flags & VRING_AVAIL_F_NO_INTERRUPT); + uint16_t flags = virtio_lduw_p(vdev, &vring->vr.avail->flags); + return !(flags & VRING_AVAIL_F_NO_INTERRUPT); } old = vring->signalled_used; v = vring->signalled_used_valid; @@ -156,7 +164,8 @@ bool vring_should_notify(VirtIODevice *vdev, Vring *vring) return true; } - return vring_need_event(vring_used_event(&vring->vr), new, old); + used_event_idx = virtio_lduw_p(vdev, &vring_used_event(&vring->vr)); + return vring_need_event(used_event_idx, new, old); } @@ -208,8 +217,19 @@ static int get_desc(Vring *vring, VirtQueueElement *elem, return 0; } +static void copy_in_vring_desc(VirtIODevice *vdev, + const struct vring_desc *guest, + struct vring_desc *host) +{ + host->addr = virtio_ldq_p(vdev, &guest->addr); + host->len = virtio_ldl_p(vdev, &guest->len); + host->flags = virtio_lduw_p(vdev, &guest->flags); + host->next = virtio_lduw_p(vdev, &guest->next); +} + /* This is stolen from linux/drivers/vhost/vhost.c. */ -static int get_indirect(Vring *vring, VirtQueueElement *elem, +static int get_indirect(VirtIODevice *vdev, Vring *vring, + VirtQueueElement *elem, struct vring_desc *indirect) { struct vring_desc desc; @@ -250,7 +270,7 @@ static int get_indirect(Vring *vring, VirtQueueElement *elem, vring->broken = true; return -EFAULT; } - desc = *desc_ptr; + copy_in_vring_desc(vdev, desc_ptr, &desc); memory_region_unref(mr); /* Ensure descriptor has been loaded before accessing fields */ @@ -326,7 +346,7 @@ int vring_pop(VirtIODevice *vdev, Vring *vring, /* Check it isn't doing very strange things with descriptor numbers. */ last_avail_idx = vring->last_avail_idx; - avail_idx = vring->vr.avail->idx; + avail_idx = virtio_lduw_p(vdev, &vring->vr.avail->idx); barrier(); /* load indices now and not again later */ if (unlikely((uint16_t)(avail_idx - last_avail_idx) > num)) { @@ -347,7 +367,7 @@ int vring_pop(VirtIODevice *vdev, Vring *vring, /* Grab the next descriptor number they're advertising, and increment * the index we've seen. */ - head = vring->vr.avail->ring[last_avail_idx % num]; + head = virtio_lduw_p(vdev, &vring->vr.avail->ring[last_avail_idx % num]); elem->index = head; @@ -371,13 +391,13 @@ int vring_pop(VirtIODevice *vdev, Vring *vring, ret = -EFAULT; goto out; } - desc = vring->vr.desc[i]; + copy_in_vring_desc(vdev, &vring->vr.desc[i], &desc); /* Ensure descriptor is loaded before accessing fields */ barrier(); if (desc.flags & VRING_DESC_F_INDIRECT) { - ret = get_indirect(vring, elem, &desc); + ret = get_indirect(vdev, vring, elem, &desc); if (ret < 0) { goto out; } @@ -395,7 +415,8 @@ int vring_pop(VirtIODevice *vdev, Vring *vring, /* On success, increment avail index. */ vring->last_avail_idx++; if (vdev->guest_features & (1 << VIRTIO_RING_F_EVENT_IDX)) { - vring_avail_event(&vring->vr) = vring->last_avail_idx; + virtio_stw_p(vdev, &vring_avail_event(&vring->vr), + vring->last_avail_idx); } return head; @@ -413,7 +434,8 @@ out: * * Stolen from linux/drivers/vhost/vhost.c. */ -void vring_push(Vring *vring, VirtQueueElement *elem, int len) +void vring_push(VirtIODevice *vdev, Vring *vring, + VirtQueueElement *elem, int len) { struct vring_used_elem *used; unsigned int head = elem->index; @@ -429,13 +451,14 @@ void vring_push(Vring *vring, VirtQueueElement *elem, int len) /* The virtqueue contains a ring of used buffers. Get a pointer to the * next entry in that used ring. */ used = &vring->vr.used->ring[vring->last_used_idx % vring->vr.num]; - used->id = head; - used->len = len; + virtio_stl_p(vdev, &used->id, head); + virtio_stl_p(vdev, &used->len, len); /* Make sure buffer is written before we update index. */ smp_wmb(); - new = vring->vr.used->idx = ++vring->last_used_idx; + new = ++vring->last_used_idx; + virtio_stw_p(vdev, &vring->vr.used->idx, new); if (unlikely((int16_t)(new - vring->signalled_used) < (uint16_t)1)) { vring->signalled_used_valid = false; } diff --git a/include/hw/virtio/dataplane/vring.h b/include/hw/virtio/dataplane/vring.h index 1e871e6..38e581b 100644 --- a/include/hw/virtio/dataplane/vring.h +++ b/include/hw/virtio/dataplane/vring.h @@ -48,6 +48,7 @@ void vring_disable_notification(VirtIODevice *vdev, Vring *vring); bool vring_enable_notification(VirtIODevice *vdev, Vring *vring); bool vring_should_notify(VirtIODevice *vdev, Vring *vring); int vring_pop(VirtIODevice *vdev, Vring *vring, VirtQueueElement *elem); -void vring_push(Vring *vring, VirtQueueElement *elem, int len); +void vring_push(VirtIODevice *vdev, Vring *vring, + VirtQueueElement *elem, int len); #endif /* VRING_H */ -- 2.1.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 2/2] dataplane: use virtio_ld/st_p() for endian-aware memory access 2015-01-19 17:04 ` [Qemu-devel] [PATCH 2/2] dataplane: use virtio_ld/st_p() for endian-aware memory access Stefan Hajnoczi @ 2015-01-21 0:17 ` David Gibson 0 siblings, 0 replies; 8+ messages in thread From: David Gibson @ 2015-01-21 0:17 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: Paolo Bonzini, Fam Zheng, qemu-devel [-- Attachment #1: Type: text/plain, Size: 1768 bytes --] On Mon, Jan 19, 2015 at 05:04:37PM +0000, Stefan Hajnoczi wrote: > The vring.c code was written under the assumption that virtio devices > have the same endianness as the host. > > This is wrong when emulating targets that differ in endianness from the > host. > > It is also wrong when emulating bi-endian targets like POWER 8 > processors, which support both little- and big-endian at run-time. In > this case the virtio device knows which endianness to use. > > This change requires the virtio-access.h APIs and therefore builds > vring.o for each target (obj-y instead of common-obj-y). > > Note that $(CONFIG_VIRTIO) for dataplane/ was dropped in > hw/virtio/Makefile.objs because hw/Makefile.objs conditionally includes > hw/virtio/ on $(CONFIG_VIRTIO) already. > > Only a small change is needed to the vring.h interface: vring_push() now > takes a VirtIODevice *vdev argument. > > Cc: Paolo Bonzini <pbonzini@redhat.com> > Cc: Fam Zheng <famz@redhat.com> > Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com> Without this patch adding an iothread to a secondary virtio-blk disk caused the kernel to jam up during boot. With this patch it booted up and I was able to do things with the disk. I'm still not able to add an iothread to a primary virtio-blk device: SLOF (guest firmware) gets errors attempting to read the kernel to boot. However, SLOF is always big endian, so I think that's an unrelated bug. Reviewed-by: David Gibson <david@gibson.dropbear.id.au> Tested-by: David Gibson <david@gibson.dropbear.id.au> -- David Gibson | I'll have my music baroque, and my code david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ | _way_ _around_! http://www.ozlabs.org/~dgibson [-- Attachment #2: Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] dataplane: use endian-aware memory accessors 2015-01-19 17:04 [Qemu-devel] [PATCH 0/2] dataplane: use endian-aware memory accessors Stefan Hajnoczi 2015-01-19 17:04 ` [Qemu-devel] [PATCH 1/2] dataplane: move vring_more_avail() into vring.c Stefan Hajnoczi 2015-01-19 17:04 ` [Qemu-devel] [PATCH 2/2] dataplane: use virtio_ld/st_p() for endian-aware memory access Stefan Hajnoczi @ 2015-01-19 17:33 ` Cornelia Huck 2015-01-21 13:30 ` Stefan Hajnoczi 3 siblings, 0 replies; 8+ messages in thread From: Cornelia Huck @ 2015-01-19 17:33 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: qemu-devel, David Gibson On Mon, 19 Jan 2015 17:04:35 +0000 Stefan Hajnoczi <stefanha@redhat.com> wrote: > In commit 0f5d1d2a49778863db54b4b1ac2dc008a8f21f11 ("virtio: memory accessors > for endian-ambivalent targets") endian-aware memory accessors were added to > support bi-endian targets like recent ppc64 systems. > > The dataplane vring.c code does not use these accessors and is therefore unable > to emulate virtio devices when the endianness differs between the device and > host. > > These patches modify vring.c to use the endian-aware accessors. I had modified the accesses in vring.c to consider endianness for virtio-1 support as well. I wonder whether we can find some common ground there? It's been a while since I have had time to work on this, but searching for "dataplane: allow virtio-1 devices" should lead you to the relevant patch. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH 0/2] dataplane: use endian-aware memory accessors 2015-01-19 17:04 [Qemu-devel] [PATCH 0/2] dataplane: use endian-aware memory accessors Stefan Hajnoczi ` (2 preceding siblings ...) 2015-01-19 17:33 ` [Qemu-devel] [PATCH 0/2] dataplane: use endian-aware memory accessors Cornelia Huck @ 2015-01-21 13:30 ` Stefan Hajnoczi 3 siblings, 0 replies; 8+ messages in thread From: Stefan Hajnoczi @ 2015-01-21 13:30 UTC (permalink / raw) To: Stefan Hajnoczi; +Cc: qemu-devel, David Gibson On Mon, Jan 19, 2015 at 5:04 PM, Stefan Hajnoczi <stefanha@redhat.com> wrote: > In commit 0f5d1d2a49778863db54b4b1ac2dc008a8f21f11 ("virtio: memory accessors > for endian-ambivalent targets") endian-aware memory accessors were added to > support bi-endian targets like recent ppc64 systems. > > The dataplane vring.c code does not use these accessors and is therefore unable > to emulate virtio devices when the endianness differs between the device and > host. > > These patches modify vring.c to use the endian-aware accessors. > > I have tested that x86_64 guests still function correctly. > > I have only compile-tested on ppc64 and require help testing that dataplane > works. If you have access to a bi-endian system, please try: > > qemu ... -object iothread,id=iothread0 \ > -drive if=none,id=drive0,file=... \ > -device virtio-blk-pci,iothread=iothread0,drive=drive0 > > Stefan Hajnoczi (2): > dataplane: move vring_more_avail() into vring.c > dataplane: use virtio_ld/st_p() for endian-aware memory access > > hw/block/dataplane/virtio-blk.c | 2 +- > hw/scsi/virtio-scsi-dataplane.c | 2 +- > hw/virtio/Makefile.objs | 2 +- > hw/virtio/dataplane/Makefile.objs | 2 +- > hw/virtio/dataplane/vring.c | 69 ++++++++++++++++++++++++++----------- > include/hw/virtio/dataplane/vring.h | 9 ++--- > 6 files changed, 55 insertions(+), 31 deletions(-) NACK Cornelia Huck already has an equivalent patch. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2015-01-21 13:30 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2015-01-19 17:04 [Qemu-devel] [PATCH 0/2] dataplane: use endian-aware memory accessors Stefan Hajnoczi 2015-01-19 17:04 ` [Qemu-devel] [PATCH 1/2] dataplane: move vring_more_avail() into vring.c Stefan Hajnoczi 2015-01-20 23:56 ` David Gibson 2015-01-21 13:25 ` Greg Kurz 2015-01-19 17:04 ` [Qemu-devel] [PATCH 2/2] dataplane: use virtio_ld/st_p() for endian-aware memory access Stefan Hajnoczi 2015-01-21 0:17 ` David Gibson 2015-01-19 17:33 ` [Qemu-devel] [PATCH 0/2] dataplane: use endian-aware memory accessors Cornelia Huck 2015-01-21 13:30 ` Stefan Hajnoczi
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).