Re: [RFC PATCH v5 03/26] virtio: Add VIRTIO_F_QUEUE_STATE

[Jason Wang <jasowang@redhat.com>]

On Sat, Oct 30, 2021 at 2:36 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>
> Implementation of RFC of device state capability:
> https://lists.oasis-open.org/archives/virtio-comment/202012/msg00005.html

Considering this still requires time to be done, we need to think of a
way to go without this.

Thanks



>
> With this capability, vdpa device can reset it's index so it can start
> consuming from guest after disabling shadow virtqueue (SVQ), with state
> not 0.
>
> The use case is to test SVQ with virtio-pci vdpa (vp_vdpa) with nested
> virtualization. Spawning a L0 qemu with a virtio-net device, use
> vp_vdpa driver to handle it in the guest, and then spawn a L1 qemu using
> that vdpa device. When L1 qemu calls device to set a new state though
> vdpa ioctl, vp_vdpa should set each queue state though virtio
> VIRTIO_PCI_COMMON_Q_AVAIL_STATE.
>
> Since this is only for testing vhost-vdpa, it's added here before of
> proposing to kernel code. No effort is done for checking that device
> can actually change its state, its layout, or if the device even
> supports to change state at all. These will be added in the future.
>
> Also, a modified version of vp_vdpa that allows to set these in PCI
> config is needed.
>
> TODO: Check for feature enabled and split in virtio pci config
>
> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>  hw/virtio/virtio-pci.h                         | 1 +
>  include/hw/virtio/virtio.h                     | 4 +++-
>  include/standard-headers/linux/virtio_config.h | 3 +++
>  include/standard-headers/linux/virtio_pci.h    | 2 ++
>  hw/virtio/virtio-pci.c                         | 9 +++++++++
>  5 files changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/hw/virtio/virtio-pci.h b/hw/virtio/virtio-pci.h
> index 2446dcd9ae..019badbd7c 100644
> --- a/hw/virtio/virtio-pci.h
> +++ b/hw/virtio/virtio-pci.h
> @@ -120,6 +120,7 @@ typedef struct VirtIOPCIQueue {
>    uint32_t desc[2];
>    uint32_t avail[2];
>    uint32_t used[2];
> +  uint16_t state;
>  } VirtIOPCIQueue;
>
>  struct VirtIOPCIProxy {
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index 8bab9cfb75..5fe575b8f0 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -289,7 +289,9 @@ typedef struct VirtIORNGConf VirtIORNGConf;
>      DEFINE_PROP_BIT64("iommu_platform", _state, _field, \
>                        VIRTIO_F_IOMMU_PLATFORM, false), \
>      DEFINE_PROP_BIT64("packed", _state, _field, \
> -                      VIRTIO_F_RING_PACKED, false)
> +                      VIRTIO_F_RING_PACKED, false), \
> +    DEFINE_PROP_BIT64("save_restore_q_state", _state, _field, \
> +                      VIRTIO_F_QUEUE_STATE, true)
>
>  hwaddr virtio_queue_get_desc_addr(VirtIODevice *vdev, int n);
>  bool virtio_queue_enabled_legacy(VirtIODevice *vdev, int n);
> diff --git a/include/standard-headers/linux/virtio_config.h b/include/standard-headers/linux/virtio_config.h
> index 22e3a85f67..59fad3eb45 100644
> --- a/include/standard-headers/linux/virtio_config.h
> +++ b/include/standard-headers/linux/virtio_config.h
> @@ -90,4 +90,7 @@
>   * Does the device support Single Root I/O Virtualization?
>   */
>  #define VIRTIO_F_SR_IOV                        37
> +
> +/* Device support save and restore virtqueue state */
> +#define VIRTIO_F_QUEUE_STATE            40
>  #endif /* _LINUX_VIRTIO_CONFIG_H */
> diff --git a/include/standard-headers/linux/virtio_pci.h b/include/standard-headers/linux/virtio_pci.h
> index db7a8e2fcb..c8d9802a87 100644
> --- a/include/standard-headers/linux/virtio_pci.h
> +++ b/include/standard-headers/linux/virtio_pci.h
> @@ -164,6 +164,7 @@ struct virtio_pci_common_cfg {
>         uint32_t queue_avail_hi;                /* read-write */
>         uint32_t queue_used_lo;         /* read-write */
>         uint32_t queue_used_hi;         /* read-write */
> +       uint16_t queue_avail_state;     /* read-write */
>  };
>
>  /* Fields in VIRTIO_PCI_CAP_PCI_CFG: */
> @@ -202,6 +203,7 @@ struct virtio_pci_cfg_cap {
>  #define VIRTIO_PCI_COMMON_Q_AVAILHI    44
>  #define VIRTIO_PCI_COMMON_Q_USEDLO     48
>  #define VIRTIO_PCI_COMMON_Q_USEDHI     52
> +#define VIRTIO_PCI_COMMON_Q_AVAIL_STATE        56
>
>  #endif /* VIRTIO_PCI_NO_MODERN */
>
> diff --git a/hw/virtio/virtio-pci.c b/hw/virtio/virtio-pci.c
> index 750aa47ec1..d7bb549033 100644
> --- a/hw/virtio/virtio-pci.c
> +++ b/hw/virtio/virtio-pci.c
> @@ -1244,6 +1244,9 @@ static uint64_t virtio_pci_common_read(void *opaque, hwaddr addr,
>      case VIRTIO_PCI_COMMON_Q_USEDHI:
>          val = proxy->vqs[vdev->queue_sel].used[1];
>          break;
> +    case VIRTIO_PCI_COMMON_Q_AVAIL_STATE:
> +        val = virtio_queue_get_last_avail_idx(vdev, vdev->queue_sel);
> +        break;
>      default:
>          val = 0;
>      }
> @@ -1330,6 +1333,8 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr,
>                         proxy->vqs[vdev->queue_sel].avail[0],
>                         ((uint64_t)proxy->vqs[vdev->queue_sel].used[1]) << 32 |
>                         proxy->vqs[vdev->queue_sel].used[0]);
> +            virtio_queue_set_last_avail_idx(vdev, vdev->queue_sel,
> +                        proxy->vqs[vdev->queue_sel].state);
>              proxy->vqs[vdev->queue_sel].enabled = 1;
>          } else {
>              virtio_error(vdev, "wrong value for queue_enable %"PRIx64, val);
> @@ -1353,6 +1358,9 @@ static void virtio_pci_common_write(void *opaque, hwaddr addr,
>      case VIRTIO_PCI_COMMON_Q_USEDHI:
>          proxy->vqs[vdev->queue_sel].used[1] = val;
>          break;
> +    case VIRTIO_PCI_COMMON_Q_AVAIL_STATE:
> +        proxy->vqs[vdev->queue_sel].state = val;
> +        break;
>      default:
>          break;
>      }
> @@ -1951,6 +1959,7 @@ static void virtio_pci_reset(DeviceState *qdev)
>          proxy->vqs[i].desc[0] = proxy->vqs[i].desc[1] = 0;
>          proxy->vqs[i].avail[0] = proxy->vqs[i].avail[1] = 0;
>          proxy->vqs[i].used[0] = proxy->vqs[i].used[1] = 0;
> +        proxy->vqs[i].state = 0;
>      }
>
>      if (pci_is_express(dev)) {
> --
> 2.27.0
>