Re: [Qemu-devel] [PATCH 2.4] virtio: fix 1.0 virtqueue migration

[Michael S. Tsirkin]

On Wed, Aug 05, 2015 at 02:12:26PM +0800, Jason Wang wrote:
> 1.0 does not requires physically-contiguous pages layout for a
> virtqueue. So we could not infer avail and used from desc. This means
> we need to migrate vring.avail and vring.used when host support virtio
> 1.0. This fixes malfunction of virtio 1.0 device after migration.
> 
> Cc: Michael S. Tsirkin <address@hidden>
> Cc: Cornelia Huck <address@hidden>
> Signed-off-by: Jason Wang <address@hidden>

Reviewed-by: Michael S. Tsirkin <address@hidden>

I agree we want this in 2.4.

> ---
>  hw/virtio/virtio.c         | 17 ++++++++++++++---
>  include/hw/virtio/virtio.h |  6 ++++++
>  2 files changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c
> index ee4e07c..ae78253 100644
> --- a/hw/virtio/virtio.c
> +++ b/hw/virtio/virtio.c
> @@ -1092,6 +1092,7 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
>      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>      VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
>      uint32_t guest_features_lo = (vdev->guest_features & 0xffffffff);
> +    bool version_1 = virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1);
>      int i;
>  
>      if (k->save_config) {
> @@ -1120,8 +1121,12 @@ void virtio_save(VirtIODevice *vdev, QEMUFile *f)
>          if (k->has_variable_vring_alignment) {
>              qemu_put_be32(f, vdev->vq[i].vring.align);
>          }
> -        /* XXX virtio-1 devices */
>          qemu_put_be64(f, vdev->vq[i].vring.desc);
> +        if (version_1) {
> +            qemu_put_be64(f, vdev->vq[i].vring.avail);
> +            qemu_put_be64(f, vdev->vq[i].vring.used);
> +        }
> +
>          qemu_put_be16s(f, &vdev->vq[i].last_avail_idx);
>          if (k->save_queue) {
>              k->save_queue(qbus->parent, i, f);
> @@ -1170,6 +1175,7 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int 
> version_id)
>      BusState *qbus = qdev_get_parent_bus(DEVICE(vdev));
>      VirtioBusClass *k = VIRTIO_BUS_GET_CLASS(qbus);
>      VirtioDeviceClass *vdc = VIRTIO_DEVICE_GET_CLASS(vdev);
> +    bool version_1 = virtio_host_has_feature(vdev, VIRTIO_F_VERSION_1);
>  
>      /*
>       * We poison the endianness to ensure it does not get used before
> @@ -1218,13 +1224,18 @@ int virtio_load(VirtIODevice *vdev, QEMUFile *f, int 
> version_id)
>              vdev->vq[i].vring.align = qemu_get_be32(f);
>          }
>          vdev->vq[i].vring.desc = qemu_get_be64(f);
> +        if (version_1) {
> +            vdev->vq[i].vring.avail = qemu_get_be64(f);
> +            vdev->vq[i].vring.used = qemu_get_be64(f);
> +        }
>          qemu_get_be16s(f, &vdev->vq[i].last_avail_idx);
>          vdev->vq[i].signalled_used_valid = false;
>          vdev->vq[i].notification = true;
>  
>          if (vdev->vq[i].vring.desc) {
> -            /* XXX virtio-1 devices */
> -            virtio_queue_update_rings(vdev, i);
> +            if (!version_1) {
> +                virtio_queue_update_rings(vdev, i);
> +            }
>          } else if (vdev->vq[i].last_avail_idx) {
>              error_report("VQ %d address 0x0 "
>                           "inconsistent with Host index 0x%x",
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 59f0763..cccae89 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -272,6 +272,12 @@ static inline bool virtio_has_feature(VirtIODevice 
> *vdev, unsigned int fbit)
>      return __virtio_has_feature(vdev->guest_features, fbit);
>  }
>  
> +static inline bool virtio_host_has_feature(VirtIODevice *vdev,
> +                                           unsigned int fbit)
> +{
> +    return __virtio_has_feature(vdev->host_features, fbit);
> +}
> +
>  static inline bool virtio_is_big_endian(VirtIODevice *vdev)
>  {
>      if (!virtio_has_feature(vdev, VIRTIO_F_VERSION_1)) {
> -- 
> 2.1.4