Re: [RFC PATCH v7 18/25] vdpa: Add map/unmap operation callback to SVQ

[Jason Wang]

On Thu, Apr 14, 2022 at 12:32 AM Eugenio Pérez <eperezma@redhat.com> wrote:
>

Let's document the motivation here. It looks to me we don't have more
than one kind of map ops implemented in this series.

Thanks

> Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> ---
>  hw/virtio/vhost-shadow-virtqueue.h | 21 +++++++++++++++++++--
>  hw/virtio/vhost-shadow-virtqueue.c |  8 +++++++-
>  hw/virtio/vhost-vdpa.c             | 20 +++++++++++++++++++-
>  3 files changed, 45 insertions(+), 4 deletions(-)
>
> diff --git a/hw/virtio/vhost-shadow-virtqueue.h 
> b/hw/virtio/vhost-shadow-virtqueue.h
> index 2809dee27b..e06ac52158 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.h
> +++ b/hw/virtio/vhost-shadow-virtqueue.h
> @@ -26,6 +26,15 @@ typedef struct VhostShadowVirtqueueOps {
>      VirtQueueElementCallback used_elem_handler;
>  } VhostShadowVirtqueueOps;
>
> +typedef int (*vhost_svq_map_op)(hwaddr iova, hwaddr size, void *vaddr,
> +                                bool readonly, void *opaque);
> +typedef int (*vhost_svq_unmap_op)(hwaddr iova, hwaddr size, void *opaque);
> +
> +typedef struct VhostShadowVirtqueueMapOps {
> +    vhost_svq_map_op map;
> +    vhost_svq_unmap_op unmap;
> +} VhostShadowVirtqueueMapOps;
> +
>  /* Shadow virtqueue to relay notifications */
>  typedef struct VhostShadowVirtqueue {
>      /* Shadow vring */
> @@ -73,6 +82,12 @@ typedef struct VhostShadowVirtqueue {
>      /* Optional callbacks */
>      const VhostShadowVirtqueueOps *ops;
>
> +    /* Device memory mapping callbacks */
> +    const VhostShadowVirtqueueMapOps *map_ops;
> +
> +    /* Device memory mapping callbacks opaque */
> +    void *map_ops_opaque;
> +
>      /* Optional custom used virtqueue element handler */
>      VirtQueueElementCallback used_elem_cb;
>
> @@ -102,8 +117,10 @@ void vhost_svq_start(VhostShadowVirtqueue *svq, 
> VirtIODevice *vdev,
>                       VirtQueue *vq);
>  void vhost_svq_stop(VhostShadowVirtqueue *svq);
>
> -VhostShadowVirtqueue *vhost_svq_new(VhostIOVATree *iova_tree,
> -                                    const VhostShadowVirtqueueOps *ops);
> +VhostShadowVirtqueue *vhost_svq_new(VhostIOVATree *iova_map,
> +                                    const VhostShadowVirtqueueOps *ops,
> +                                    const VhostShadowVirtqueueMapOps 
> *map_ops,
> +                                    void *map_ops_opaque);
>
>  void vhost_svq_free(gpointer vq);
>  G_DEFINE_AUTOPTR_CLEANUP_FUNC(VhostShadowVirtqueue, vhost_svq_free);
> diff --git a/hw/virtio/vhost-shadow-virtqueue.c 
> b/hw/virtio/vhost-shadow-virtqueue.c
> index 72a403d90b..87980e2a9c 100644
> --- a/hw/virtio/vhost-shadow-virtqueue.c
> +++ b/hw/virtio/vhost-shadow-virtqueue.c
> @@ -612,13 +612,17 @@ void vhost_svq_stop(VhostShadowVirtqueue *svq)
>   *
>   * @iova_tree: Tree to perform descriptors translations
>   * @ops: SVQ operations hooks
> + * @map_ops: SVQ mapping operation hooks
> + * @map_ops_opaque: Opaque data to pass to mapping operations
>   *
>   * Returns the new virtqueue or NULL.
>   *
>   * In case of error, reason is reported through error_report.
>   */
>  VhostShadowVirtqueue *vhost_svq_new(VhostIOVATree *iova_tree,
> -                                    const VhostShadowVirtqueueOps *ops)
> +                                    const VhostShadowVirtqueueOps *ops,
> +                                    const VhostShadowVirtqueueMapOps 
> *map_ops,
> +                                    void *map_ops_opaque)
>  {
>      g_autofree VhostShadowVirtqueue *svq = g_new0(VhostShadowVirtqueue, 1);
>      int r;
> @@ -641,6 +645,8 @@ VhostShadowVirtqueue *vhost_svq_new(VhostIOVATree 
> *iova_tree,
>      event_notifier_set_handler(&svq->hdev_call, vhost_svq_handle_call);
>      svq->iova_tree = iova_tree;
>      svq->ops = ops;
> +    svq->map_ops = map_ops;
> +    svq->map_ops_opaque = map_ops_opaque;
>      return g_steal_pointer(&svq);
>
>  err_init_hdev_call:
> diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
> index 9e62f3280d..1948c5ca7d 100644
> --- a/hw/virtio/vhost-vdpa.c
> +++ b/hw/virtio/vhost-vdpa.c
> @@ -384,6 +384,22 @@ static int vhost_vdpa_get_dev_features(struct vhost_dev 
> *dev,
>      return ret;
>  }
>
> +static int vhost_vdpa_svq_map(hwaddr iova, hwaddr size, void *vaddr,
> +                              bool readonly, void *opaque)
> +{
> +    return vhost_vdpa_dma_map(opaque, iova, size, vaddr, readonly);
> +}
> +
> +static int vhost_vdpa_svq_unmap(hwaddr iova, hwaddr size, void *opaque)
> +{
> +    return vhost_vdpa_dma_unmap(opaque, iova, size);
> +}
> +
> +static const VhostShadowVirtqueueMapOps vhost_vdpa_svq_map_ops = {
> +    .map = vhost_vdpa_svq_map,
> +    .unmap = vhost_vdpa_svq_unmap,
> +};
> +
>  static int vhost_vdpa_init_svq(struct vhost_dev *hdev, struct vhost_vdpa *v,
>                                 Error **errp)
>  {
> @@ -411,7 +427,9 @@ static int vhost_vdpa_init_svq(struct vhost_dev *hdev, 
> struct vhost_vdpa *v,
>      shadow_vqs = g_ptr_array_new_full(hdev->nvqs, vhost_svq_free);
>      for (unsigned n = 0; n < hdev->nvqs; ++n) {
>          g_autoptr(VhostShadowVirtqueue) svq = vhost_svq_new(v->iova_tree,
> -                                                            
> v->shadow_vq_ops);
> +                                                       v->shadow_vq_ops,
> +                                                       
> &vhost_vdpa_svq_map_ops,
> +                                                       v);
>
>          if (unlikely(!svq)) {
>              error_setg(errp, "Cannot create svq %u", n);
> --
> 2.27.0
>