On Thu, Dec 11, 2014 at 02:25:08PM +0100, Cornelia Huck wrote: > Add code that checks for the VERSION_1 feature bit in order to make > decisions about the device's endianness. This allows us to support > transitional devices. > > Signed-off-by: Cornelia Huck <[email protected]> > --- > hw/virtio/virtio.c | 6 +++++- > include/hw/virtio/virtio-access.h | 4 ++++ > include/hw/virtio/virtio.h | 8 ++++++-- > 3 files changed, 15 insertions(+), 3 deletions(-) > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 7f74ae5..8f69ffa 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -881,7 +881,11 @@ static bool virtio_device_endian_needed(void *opaque) > VirtIODevice *vdev = opaque; > > assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN); > - return vdev->device_endian != virtio_default_endian(); > + if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) { > + return vdev->device_endian != virtio_default_endian(); > + } > + /* Devices conforming to VIRTIO 1.0 or later are always LE. */ > + return vdev->device_endian != VIRTIO_DEVICE_ENDIAN_LITTLE;
This doesn't seem quite right. Since virtio 1.0 is always LE, this
should just assert that device_endian == LE and return false,
right?
> }
>
> static const VMStateDescription vmstate_virtio_device_endian = {
> diff --git a/include/hw/virtio/virtio-access.h
> b/include/hw/virtio/virtio-access.h
> index 46456fd..ee28c21 100644
> --- a/include/hw/virtio/virtio-access.h
> +++ b/include/hw/virtio/virtio-access.h
> @@ -19,6 +19,10 @@
>
> static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
> {
> + if (virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> + /* Devices conforming to VIRTIO 1.0 or later are always LE. */
> + return false;
> + }
> #if defined(TARGET_IS_BIENDIAN)
> return virtio_is_big_endian(vdev);
> #elif defined(TARGET_WORDS_BIGENDIAN)
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 08141c7..68c40db 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -297,7 +297,11 @@ static inline bool virtio_has_feature(VirtIODevice
> *vdev, unsigned int fbit)
>
> static inline bool virtio_is_big_endian(VirtIODevice *vdev)
> {
> - assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN);
> - return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG;
> + if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> + 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;
> }
> #endif
AFAICT, the only real difference between virtio_is_big_endian() and
virtio_access_is_big_endian() is that the latter will become
compile-time constant on targets that don't do bi-endian.
With virtio 1.0 support, that's no longer true, so those two macros
should just be merged, I think.
--
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
pgpj_tNTjUrSz.pgp
Description: PGP signature
_______________________________________________ Virtualization mailing list [email protected] https://lists.linuxfoundation.org/mailman/listinfo/virtualization
