[Qemu-devel] [RFC PATCH 0/2] vhost: support for cross endian

[Qemu-devel] [RFC PATCH 0/2] vhost: support for cross endian

[Cédric Le Goater]

These two small patches add a VHOST_VRING_F_BYTESWAP flag to the 
vring which will be used by the kernel to byteswap the different 
vring indexes.

The kernel patchset can be found on the kvm@ and kvm-ppc@ mailing 
lists.

Cédric Le Goater (2):
  vhost: add VHOST_VRING_F_BYTESWAP flag
  vhost_net: re-enable when cross endian

 hw/net/vhost_net.c          |   19 -------------------
 hw/virtio/vhost.c           |   25 ++++++++++++++++++++++++-
 include/hw/virtio/vhost.h   |    1 +
 linux-headers/linux/vhost.h |    3 +++
 4 files changed, 28 insertions(+), 20 deletions(-)

-- 
1.7.10.4

[Qemu-devel] [RFC PATCH 1/2] vhost: add VHOST_VRING_F_BYTESWAP flag

[Cédric Le Goater]

When the guest and the host have a different endian order, the data 
being accessed in the vring queues needs to be byteswapped.

This patch adds a VHOST_VRING_F_BYTESWAP flag to inform the vhost 
kernel backend to byteswap vring data.

Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
 hw/virtio/vhost.c           |   25 ++++++++++++++++++++++++-
 include/hw/virtio/vhost.h   |    1 +
 linux-headers/linux/vhost.h |    3 +++
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 5d7c40ac04d0..75308d127d90 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -539,7 +539,13 @@ static int vhost_virtqueue_set_addr(struct vhost_dev *dev,
         .log_guest_addr = vq->used_phys,
         .flags = enable_log ? (1 << VHOST_VRING_F_LOG) : 0,
     };
-    int r = dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_ADDR, &addr);
+    int r;
+
+    if (vq->byteswap) {
+	addr.flags |= (1 << VHOST_VRING_F_BYTESWAP);
+    }
+
+    r = dev->vhost_ops->vhost_call(dev, VHOST_SET_VRING_ADDR, &addr);
     if (r < 0) {
         return -errno;
     }
@@ -647,6 +653,21 @@ static void vhost_log_stop(MemoryListener *listener,
     /* FIXME: implement */
 }
 
+
+static bool vhost_virtqueue_needs_byteswap(VirtIODevice *vdev)
+{
+#ifdef TARGET_IS_BIENDIAN
+#ifdef HOST_WORDS_BIGENDIAN
+    return !virtio_is_big_endian(vdev);
+#else
+    return virtio_is_big_endian(vdev);
+#endif
+
+#else
+    return false;
+#endif
+}
+
 static int vhost_virtqueue_start(struct vhost_dev *dev,
                                 struct VirtIODevice *vdev,
                                 struct vhost_virtqueue *vq,
@@ -707,6 +728,8 @@ static int vhost_virtqueue_start(struct vhost_dev *dev,
         goto fail_alloc_ring;
     }
 
+    vq->byteswap = vhost_virtqueue_needs_byteswap(vdev);
+
     r = vhost_virtqueue_set_addr(dev, vq, vhost_vq_index, dev->log_enabled);
     if (r < 0) {
         r = -errno;
diff --git a/include/hw/virtio/vhost.h b/include/hw/virtio/vhost.h
index d5593d162079..793f89d71ffb 100644
--- a/include/hw/virtio/vhost.h
+++ b/include/hw/virtio/vhost.h
@@ -20,6 +20,7 @@ struct vhost_virtqueue {
     unsigned long long ring_phys;
     unsigned ring_size;
     EventNotifier masked_notifier;
+    bool byteswap;
 };
 
 typedef unsigned long vhost_log_chunk_t;
diff --git a/linux-headers/linux/vhost.h b/linux-headers/linux/vhost.h
index c656f61cfc75..0bbd3f1209f3 100644
--- a/linux-headers/linux/vhost.h
+++ b/linux-headers/linux/vhost.h
@@ -34,6 +34,9 @@ struct vhost_vring_addr {
 	/* Flag values: */
 	/* Whether log address is valid. If set enables logging. */
 #define VHOST_VRING_F_LOG 0
+	/* Whether vring memory accesses should be byte-swapped.
+	 * required when the guest has a different endianess */
+#define VHOST_VRING_F_BYTESWAP 1
 
 	/* Start of array of descriptors (virtually contiguous) */
 	__u64 desc_user_addr;
-- 
1.7.10.4

[Qemu-devel] [RFC PATCH 2/2] vhost_net: re-enable when cross endian

[Cédric Le Goater]

revert 371df9f5e0f1 "vhost-net: disable when cross-endian"

Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
---
 hw/net/vhost_net.c |   19 -------------------
 1 file changed, 19 deletions(-)

diff --git a/hw/net/vhost_net.c b/hw/net/vhost_net.c
index 4e3a06162291..721fb2da52f8 100644
--- a/hw/net/vhost_net.c
+++ b/hw/net/vhost_net.c
@@ -271,19 +271,6 @@ static void vhost_net_stop_one(struct vhost_net *net,
     vhost_dev_disable_notifiers(&net->dev, dev);
 }
 
-static bool vhost_net_device_endian_ok(VirtIODevice *vdev)
-{
-#ifdef TARGET_IS_BIENDIAN
-#ifdef HOST_WORDS_BIGENDIAN
-    return virtio_is_big_endian(vdev);
-#else
-    return !virtio_is_big_endian(vdev);
-#endif
-#else
-    return true;
-#endif
-}
-
 int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
                     int total_queues)
 {
@@ -292,12 +279,6 @@ int vhost_net_start(VirtIODevice *dev, NetClientState *ncs,
     VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(vbus);
     int r, e, i;
 
-    if (!vhost_net_device_endian_ok(dev)) {
-        error_report("vhost-net does not support cross-endian");
-        r = -ENOSYS;
-        goto err;
-    }
-
     if (!k->set_guest_notifiers) {
         error_report("binding does not support guest notifiers");
         r = -ENOSYS;
-- 
1.7.10.4

Re: [Qemu-devel] [RFC PATCH 1/2] vhost: add VHOST_VRING_F_BYTESWAP flag

[Cornelia Huck]

On Wed, 29 Oct 2014 09:42:09 +0100
Cédric Le Goater <clg@fr.ibm.com> wrote:

> When the guest and the host have a different endian order, the data 
> being accessed in the vring queues needs to be byteswapped.
> 
> This patch adds a VHOST_VRING_F_BYTESWAP flag to inform the vhost 
> kernel backend to byteswap vring data.
> 
> Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
> ---
>  hw/virtio/vhost.c           |   25 ++++++++++++++++++++++++-
>  include/hw/virtio/vhost.h   |    1 +
>  linux-headers/linux/vhost.h |    3 +++
>  3 files changed, 28 insertions(+), 1 deletion(-)

> +static bool vhost_virtqueue_needs_byteswap(VirtIODevice *vdev)
> +{
> +#ifdef TARGET_IS_BIENDIAN
> +#ifdef HOST_WORDS_BIGENDIAN
> +    return !virtio_is_big_endian(vdev);
> +#else
> +    return virtio_is_big_endian(vdev);
> +#endif
> +
> +#else
> +    return false;
> +#endif
> +}

*thinks aloud*

We call this function after features have been negotiated, so we should
be able to reuse this interface for virtio-1 by checking for
VIRTIO_F_VERSION_1, right?

Re: [Qemu-devel] [Qemu-ppc] [RFC PATCH 1/2] vhost: add VHOST_VRING_F_BYTESWAP flag

[Greg Kurz]

On Mon, 3 Nov 2014 16:29:31 +0100
Cornelia Huck <cornelia.huck@de.ibm.com> wrote:
> On Wed, 29 Oct 2014 09:42:09 +0100
> Cédric Le Goater <clg@fr.ibm.com> wrote:
> 
> > When the guest and the host have a different endian order, the data 
> > being accessed in the vring queues needs to be byteswapped.
> > 
> > This patch adds a VHOST_VRING_F_BYTESWAP flag to inform the vhost 
> > kernel backend to byteswap vring data.
> > 
> > Signed-off-by: Cédric Le Goater <clg@fr.ibm.com>
> > ---
> >  hw/virtio/vhost.c           |   25 ++++++++++++++++++++++++-
> >  include/hw/virtio/vhost.h   |    1 +
> >  linux-headers/linux/vhost.h |    3 +++
> >  3 files changed, 28 insertions(+), 1 deletion(-)
> 
> > +static bool vhost_virtqueue_needs_byteswap(VirtIODevice *vdev)
> > +{
> > +#ifdef TARGET_IS_BIENDIAN
> > +#ifdef HOST_WORDS_BIGENDIAN
> > +    return !virtio_is_big_endian(vdev);
> > +#else
> > +    return virtio_is_big_endian(vdev);
> > +#endif
> > +
> > +#else
> > +    return false;
> > +#endif
> > +}
> 
> *thinks aloud*
> 
> We call this function after features have been negotiated, so we should
> be able to reuse this interface for virtio-1 by checking for
> VIRTIO_F_VERSION_1, right?
> 
> 

Yes. Moreover, we're not on a hot path, so we could simply ignore
TARGET_IS_BIENDIAN and only have:

static bool vhost_virtqueue_needs_byteswap(VirtIODevice *vdev)
{
#ifdef HOST_WORDS_BIGENDIAN
    return !virtio_is_big_endian(vdev);
#else
    return virtio_is_big_endian(vdev);
#endif
}

in which case, this would work right away with the changes brought by
Conny's virtio-1 series:

static inline bool virtio_device_is_legacy(VirtIODevice *vdev)
{
    return !(vdev->guest_features[1] & (1 << (VIRTIO_F_VERSION_1 - 32)));
}

static inline bool virtio_is_big_endian(VirtIODevice *vdev)
{
    if (virtio_device_is_legacy(vdev)) {
        assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN);
        return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG;
    }
    /* Devices conforming to VIRTIO 1.0 or later are always LE. */
    return false;
}

Cheers.

--
Greg