| Message ID | 20150625152609.16840.52299.stgit@bahia.lab.toulouse-stg.fr.ibm.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Jun 25, 2015 at 05:26:09PM +0200, Greg Kurz wrote: > During early virtio 1.0 devel, there were several proposals about how to > deal endianness of the vring descriptor fields: > - convert the decriptor to host endianness in a single place, and use its > fields directly in the code > - keep the descriptor untouched and use virtio memory helpers to access its > fields with the appropriate endianness > > It seems like both approaches got merged: commit f5a5628cf0b6 introduces > an extra swap that negates the one brought by commit b0e5d90ebc3e. This > breaks cross-endian setups with the following error: > > Failed to map descriptor addr 0x18e2517e00000000 len 268435456 > > A solution could be to revert f5a5628cf0b6, but dropping copy_in_vring_desc() > is equivalent and gives a smaller patch. Cornelia already sent "[PATCH] Revert "dataplane: allow virtio-1 devices" to revert f5a5628cf0b. I acked it but the patch is going through Michael Tsirkin.
On Fri, 26 Jun 2015 11:00:45 +0100 Stefan Hajnoczi <stefanha@gmail.com> wrote: > On Thu, Jun 25, 2015 at 05:26:09PM +0200, Greg Kurz wrote: > > During early virtio 1.0 devel, there were several proposals about how to > > deal endianness of the vring descriptor fields: > > - convert the decriptor to host endianness in a single place, and use its > > fields directly in the code > > - keep the descriptor untouched and use virtio memory helpers to access its > > fields with the appropriate endianness > > > > It seems like both approaches got merged: commit f5a5628cf0b6 introduces > > an extra swap that negates the one brought by commit b0e5d90ebc3e. This > > breaks cross-endian setups with the following error: > > > > Failed to map descriptor addr 0x18e2517e00000000 len 268435456 > > > > A solution could be to revert f5a5628cf0b6, but dropping copy_in_vring_desc() > > is equivalent and gives a smaller patch. > > Cornelia already sent "[PATCH] Revert "dataplane: allow virtio-1 > devices" to revert f5a5628cf0b. I acked it but the patch is going > through Michael Tsirkin. Indeed... my bad, I overlooked that thread... -- G
diff --git a/hw/virtio/dataplane/vring.c b/hw/virtio/dataplane/vring.c index 35891856ee06..3fa421b9d773 100644 --- a/hw/virtio/dataplane/vring.c +++ b/hw/virtio/dataplane/vring.c @@ -207,16 +207,6 @@ static int get_desc(VirtIODevice *vdev, 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(VirtIODevice *vdev, Vring *vring, VirtQueueElement *elem, struct vring_desc *indirect) @@ -261,7 +251,7 @@ static int get_indirect(VirtIODevice *vdev, Vring *vring, vring->broken = true; return -EFAULT; } - copy_in_vring_desc(vdev, desc_ptr, &desc); + desc = *desc_ptr; memory_region_unref(mr); /* Ensure descriptor has been loaded before accessing fields */ @@ -383,7 +373,7 @@ int vring_pop(VirtIODevice *vdev, Vring *vring, ret = -EFAULT; goto out; } - copy_in_vring_desc(vdev, &vring->vr.desc[i], &desc); + desc = vring->vr.desc[i]; /* Ensure descriptor is loaded before accessing fields */ barrier();
During early virtio 1.0 devel, there were several proposals about how to deal endianness of the vring descriptor fields: - convert the decriptor to host endianness in a single place, and use its fields directly in the code - keep the descriptor untouched and use virtio memory helpers to access its fields with the appropriate endianness It seems like both approaches got merged: commit f5a5628cf0b6 introduces an extra swap that negates the one brought by commit b0e5d90ebc3e. This breaks cross-endian setups with the following error: Failed to map descriptor addr 0x18e2517e00000000 len 268435456 A solution could be to revert f5a5628cf0b6, but dropping copy_in_vring_desc() is equivalent and gives a smaller patch. Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com> --- hw/virtio/dataplane/vring.c | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-)