Re: [Qemu-devel] [PATCH v8 11/20] virtio: add endian-ambivalent support to VirtIODevice

[Alexander Graf <agraf@suse.de>]

On 13.06.14 13:23, Greg Kurz wrote:
> Some CPU families can dynamically change their endianness. This means we
> can have little endian ppc or big endian arm guests for example. This has
> an impact on legacy virtio data structures since they are target endian.
> We hence introduce a new property to track the endianness of each virtio
> device. It is reasonnably assumed that endianness won't change while the
> device is in use : we hence capture the device endianness when it gets
> reset.
>
> We migrate this property in a subsection, after the device descriptor. This
> means the load code must not rely on it until it is restored. As a consequence,
> the vring sanity checks had to be moved after the call to vmstate_load_state().
> We enforce paranoia by poisoning the property at the begining of virtio_load().
>
> Signed-off-by: Greg Kurz <gkurz@linux.vnet.ibm.com>
> ---
>
>   Changes since last version (virtio migration RFC):
>   - endianness cached at device reset time
>   - reworked migration support
>
>   hw/virtio/virtio-pci.c     |    8 ++--
>   hw/virtio/virtio.c         |  100 +++++++++++++++++++++++++++++++++++++++-----
>   include/hw/virtio/virtio.h |   12 ++++-
>   3 files changed, 102 insertions(+), 18 deletions(-)
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 390c8d2..2ffceb8 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -406,13 +406,13 @@ static uint64_t virtio_pci_config_read(void *opaque, hwaddr addr,
>           break;
>       case 2:
>           val = virtio_config_readw(vdev, addr);
> -        if (virtio_is_big_endian()) {
> +        if (virtio_is_big_endian(vdev)) {
>               val = bswap16(val);
>           }
>           break;
>       case 4:
>           val = virtio_config_readl(vdev, addr);
> -        if (virtio_is_big_endian()) {
> +        if (virtio_is_big_endian(vdev)) {
>               val = bswap32(val);
>           }
>           break;
> @@ -440,13 +440,13 @@ static void virtio_pci_config_write(void *opaque, hwaddr addr,
>           virtio_config_writeb(vdev, addr, val);
>           break;
>       case 2:
> -        if (virtio_is_big_endian()) {
> +        if (virtio_is_big_endian(vdev)) {
>               val = bswap16(val);
>           }
>           virtio_config_writew(vdev, addr, val);
>           break;
>       case 4:
> -        if (virtio_is_big_endian()) {
> +        if (virtio_is_big_endian(vdev)) {
>               val = bswap32(val);
>           }
>           virtio_config_writel(vdev, addr, val);
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index 16b73d9..6235df8 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -545,6 +545,24 @@ void virtio_set_status(VirtIODevice *vdev, uint8_t val)
>       vdev->status = val;
>   }
>   
> +static void virtio_set_endian_target_default(VirtIODevice *vdev)
> +{
> +#ifdef TARGET_WORDS_BIGENDIAN
> +    vdev->device_endian = VIRTIO_DEVICE_ENDIAN_BIG;
> +#else
> +    vdev->device_endian = VIRTIO_DEVICE_ENDIAN_LITTLE;
> +#endif
> +}
> +
> +static void virtio_set_endian_cpu(VirtIODevice *vdev, CPUState *cpu)
> +{
> +    if (cpu_virtio_is_big_endian(cpu)) {
> +        vdev->device_endian = VIRTIO_DEVICE_ENDIAN_BIG;
> +    } else {
> +        vdev->device_endian = VIRTIO_DEVICE_ENDIAN_LITTLE;
> +    }
> +}
> +
>   void virtio_reset(void *opaque)
>   {
>       VirtIODevice *vdev = opaque;
> @@ -552,6 +570,13 @@ void virtio_reset(void *opaque)
>       int i;
>   
>       virtio_set_status(vdev, 0);
> +    if (current_cpu) {
> +        /* Guest initiated reset */
> +        virtio_set_endian_cpu(vdev, current_cpu);
> +    } else {
> +        /* System reset */
> +        virtio_set_endian_target_default(vdev);
> +    }
>   
>       if (k->reset) {
>           k->reset(vdev);
> @@ -840,6 +865,28 @@ void virtio_notify_config(VirtIODevice *vdev)
>       virtio_notify_vector(vdev, vdev->config_vector);
>   }
>   
> +static bool virtio_device_endian_needed(void *opaque)
> +{
> +    VirtIODevice *vdev = opaque;
> +
> +    assert(vdev->device_endian != VIRTIO_DEVICE_ENDIAN_UNKNOWN);
> +#ifdef TARGET_WORDS_BIGENDIAN
> +    return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_LITTLE;
> +#else
> +    return vdev->device_endian == VIRTIO_DEVICE_ENDIAN_BIG;
> +#endif

if (target_words_bigendian()) {
     return virtio_is_big_endian();
} else {
     return !virtio_is_big_endian();
}

Then we have one less dependency on TARGET_WORDS_BIGENDIAN which means a 
step towards compiling virtio once ;).


Alex