Re: [Qemu-arm] [Qemu-devel] [PATCH] virtio: move bi-endian target support to a single location

[Cédric Le Goater]

On 05/31/2016 10:09 AM, Greg Kurz wrote:
> Paolo's recent cpu.h cleanups broke legacy virtio for ppc64 LE guests (and
> arm BE guests as well, even if I have not verified that). Especially, commit
> "33c11879fd42 qemu-common: push cpu.h inclusion out of qemu-common.h" has
> the side-effect of silently hiding the TARGET_IS_BIENDIAN macro from the
> virtio memory accessors, and thus fully disabling support of endian changing
> targets.
> 
> To be sure this cannot happen again, let's gather all the bi-endian bits
> where they belong in include/hw/virtio/virtio-access.h.
> 
> The changes in hw/virtio/vhost.c are safe because vhost_needs_vring_endian()
> is not called on a hot path and non bi-endian targets will return false
> anyway.
> 
> While here, also rename TARGET_IS_BIENDIAN to be more precise: it is only for
> legacy virtio and bi-endian guests.
> 
> Signed-off-by: Greg Kurz <address@hidden>

biendian-ness being only used by virtio devices, I think this is a good place 
where to put it.

Acked-by: Cédric Le Goater <address@hidden>

> ---
>  hw/virtio/vhost.c                 |    4 ----
>  include/hw/virtio/virtio-access.h |    6 +++++-
>  target-arm/cpu.h                  |    2 --
>  target-ppc/cpu.h                  |    2 --
>  4 files changed, 5 insertions(+), 9 deletions(-)
> 
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 440071815408..81cc5b0ae35c 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -767,15 +767,11 @@ static inline bool 
> vhost_needs_vring_endian(VirtIODevice *vdev)
>      if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
>          return false;
>      }
> -#ifdef TARGET_IS_BIENDIAN
>  #ifdef HOST_WORDS_BIGENDIAN
>      return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_LITTLE;
>  #else
>      return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG;
>  #endif
> -#else
> -    return false;
> -#endif
>  }
> 
>  static int vhost_virtqueue_set_vring_endian_legacy(struct vhost_dev *dev,
> diff --git a/include/hw/virtio/virtio-access.h 
> b/include/hw/virtio/virtio-access.h
> index 8dc84f520316..4b2803814642 100644
> --- a/include/hw/virtio/virtio-access.h
> +++ b/include/hw/virtio/virtio-access.h
> @@ -17,9 +17,13 @@
>  #include "hw/virtio/virtio.h"
>  #include "exec/address-spaces.h"
> 
> +#if defined(TARGET_PPC64) || defined(TARGET_ARM)
> +#define LEGACY_VIRTIO_IS_BIENDIAN 1
> +#endif
> +
>  static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
>  {
> -#if defined(TARGET_IS_BIENDIAN)
> +#if defined(LEGACY_VIRTIO_IS_BIENDIAN)
>      return virtio_is_big_endian(vdev);
>  #elif defined(TARGET_WORDS_BIGENDIAN)
>      if (virtio_vdev_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> diff --git a/target-arm/cpu.h b/target-arm/cpu.h
> index c741b53ad45f..60971e16f7a4 100644
> --- a/target-arm/cpu.h
> +++ b/target-arm/cpu.h
> @@ -29,8 +29,6 @@
>  #  define TARGET_LONG_BITS 32
>  #endif
> 
> -#define TARGET_IS_BIENDIAN 1
> -
>  #define CPUArchState struct CPUARMState
> 
>  #include "qemu-common.h"
> diff --git a/target-ppc/cpu.h b/target-ppc/cpu.h
> index cd33539d1ce9..556d66c39d11 100644
> --- a/target-ppc/cpu.h
> +++ b/target-ppc/cpu.h
> @@ -28,8 +28,6 @@
>  #define TARGET_LONG_BITS 64
>  #define TARGET_PAGE_BITS 12
> 
> -#define TARGET_IS_BIENDIAN 1
> -
>  /* Note that the official physical address space bits is 62-M where M
>     is implementation dependent.  I've not looked up M for the set of
>     cpus we emulate at the system level.  */
> 
>