Re: [PATCH-for-6.0 1/3] hw/virtio: Add configure switch to disable legacy VIRTIO

[Cornelia Huck]

On Thu,  5 Nov 2020 13:43:51 +0100
Philippe Mathieu-Daudé <philmd@redhat.com> wrote:

> Per [1] (Terminology):
> 
>   Legacy interfaces are not required; ie. don’t implement them
>   unless you have a need for backwards compatibility!
> 
> [2] (Version 1.0):
> 
>   The device configuration space uses the little-endian format
>   for multi-byte fields.
> 
> and [3] (Legacy Interface):
> 
>   for legacy interfaces, device configuration space is generally
>   the guest’s native endian, rather than PCI’s little-endian.
>   The correct endian-ness is documented for each device.
> 
> Add the --disable-virtio-legacy configure flag to produce builds
> with VIRTIO 1.0 only, and the --enable-virtio-legacy to include
> legacy VIRTIO support (supporting legacy VIRTIO is the default).

This is only dealing with endianess issues; there are other differences
on the control plane as well.

Currently, virtio-pci has the option to make devices non-transitional,
but virtio-ccw has not (only for device types). For virtio-mmio, you
need to select one of legacy or non-transitional, IIRC.

> 
> [1] 
> http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-60001
> [2] 
> http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-170003
> [3] 
> http://docs.oasis-open.org/virtio/virtio/v1.0/cs04/virtio-v1.0-cs04.html#x1-200003
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  configure                         | 10 ++++++++++
>  meson.build                       |  1 +
>  include/hw/virtio/virtio-access.h | 19 +++++--------------
>  hw/virtio/virtio-legacy.c         | 29 +++++++++++++++++++++++++++++
>  hw/virtio/meson.build             |  1 +
>  5 files changed, 46 insertions(+), 14 deletions(-)
>  create mode 100644 hw/virtio/virtio-legacy.c

(...)

> diff --git a/include/hw/virtio/virtio-access.h 
> b/include/hw/virtio/virtio-access.h
> index 6818a23a2d3..b6c060f8cc6 100644
> --- a/include/hw/virtio/virtio-access.h
> +++ b/include/hw/virtio/virtio-access.h
> @@ -20,24 +20,15 @@
>  #include "hw/virtio/virtio.h"
>  #include "hw/virtio/virtio-bus.h"
>  
> -#if defined(TARGET_PPC64) || defined(TARGET_ARM)
> -#define LEGACY_VIRTIO_IS_BIENDIAN 1
> -#endif
> -
> +#ifdef CONFIG_VIRTIO_LEGACY
> +bool virtio_access_is_big_endian(VirtIODevice *vdev);
> +#else
>  static inline bool virtio_access_is_big_endian(VirtIODevice *vdev)
>  {
> -#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)) {
> -        /* Devices conforming to VIRTIO 1.0 or later are always LE. */
> -        return false;
> -    }
> -    return true;
> -#else
> +    /* Devices conforming to VIRTIO 1.0 or later are always LE. */
>      return false;

This will make migration from a QEMU that has devices for which 1.0 has
not been negotiated fail.

> -#endif
>  }
> +#endif
>  
>  static inline uint16_t virtio_lduw_phys(VirtIODevice *vdev, hwaddr pa)
>  {