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

[Greg Kurz <gkurz@linux.vnet.ibm.com>]

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