Re: [RFC PATCH v9 12/23] vhost: Add opaque member to SVQElement

[Jason Wang]

在 2022/7/7 02:39, Eugenio Pérez 写道:
When qemu injects buffers to the vdpa device it will be used to maintain
contextual data. If SVQ has no operation, it will be used to maintain
the VirtQueueElement pointer.

Signed-off-by: Eugenio Pérez <epere...@redhat.com>
---
  hw/virtio/vhost-shadow-virtqueue.h |  3 ++-
  hw/virtio/vhost-shadow-virtqueue.c | 13 +++++++------
  2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/hw/virtio/vhost-shadow-virtqueue.h 
b/hw/virtio/vhost-shadow-virtqueue.h
index 0e434e9fd0..a811f90e01 100644
--- a/hw/virtio/vhost-shadow-virtqueue.h
+++ b/hw/virtio/vhost-shadow-virtqueue.h
@@ -16,7 +16,8 @@
  #include "hw/virtio/vhost-iova-tree.h"
  
  typedef struct SVQElement {
-    VirtQueueElement *elem;
+    /* Opaque data */
+    void *opaque;


So I wonder if we can simply:

1) introduce a opaque to VirtQueueElement
2) store pointers to ring_id_maps

Since

1) VirtQueueElement's member looks general
2) help to reduce the tricky codes like vhost_svq_empty_elem() and 
vhost_svq_empty_elem().

Thanks


  
      /* Last descriptor of the chain */
      uint32_t last_chain_id;
diff --git a/hw/virtio/vhost-shadow-virtqueue.c 
b/hw/virtio/vhost-shadow-virtqueue.c
index c5e49e51c5..492bb12b5f 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -237,7 +237,7 @@ static uint16_t vhost_svq_last_desc_of_chain(const 
VhostShadowVirtqueue *svq,
   */
  static bool vhost_svq_add(VhostShadowVirtqueue *svq, const struct iovec 
*out_sg,
                            size_t out_num, const struct iovec *in_sg,
-                          size_t in_num, VirtQueueElement *elem)
+                          size_t in_num, void *opaque)
  {
      SVQElement *svq_elem;
      unsigned qemu_head;
@@ -245,13 +245,12 @@ static bool vhost_svq_add(VhostShadowVirtqueue *svq, 
const struct iovec *out_sg,
      bool ok = vhost_svq_add_split(svq, out_sg, out_num, in_sg, in_num,
                                    &qemu_head);
      if (unlikely(!ok)) {
-        g_free(elem);
          return false;
      }
  
      n = out_num + in_num;
      svq_elem = &svq->ring_id_maps[qemu_head];
-    svq_elem->elem = elem;
+    svq_elem->opaque = opaque;
      svq_elem->last_chain_id = vhost_svq_last_desc_of_chain(svq, n, qemu_head);
      return true;
  }
@@ -277,6 +276,8 @@ static bool vhost_svq_add_element(VhostShadowVirtqueue *svq,
                              elem->in_num, elem);
      if (ok) {
          vhost_svq_kick(svq);
+    } else {
+        g_free(elem);
      }
  
      return ok;
@@ -392,7 +393,7 @@ static void 
vhost_svq_disable_notification(VhostShadowVirtqueue *svq)
  
  static bool vhost_svq_is_empty_elem(SVQElement elem)
  {
-    return elem.elem == NULL;
+    return elem.opaque == NULL;
  }
  
  static SVQElement vhost_svq_empty_elem(void)
@@ -483,7 +484,7 @@ static void vhost_svq_flush(VhostShadowVirtqueue *svq,
                  break;
              }
  
-            elem = g_steal_pointer(&svq_elem.elem);
+            elem = g_steal_pointer(&svq_elem.opaque);
              virtqueue_fill(vq, elem, len, i++);
          }
  
@@ -651,7 +652,7 @@ void vhost_svq_stop(VhostShadowVirtqueue *svq)
  
      for (unsigned i = 0; i < svq->vring.num; ++i) {
          g_autofree VirtQueueElement *elem = NULL;
-        elem = g_steal_pointer(&svq->ring_id_maps[i].elem);
+        elem = g_steal_pointer(&svq->ring_id_maps[i].opaque);
          if (elem) {
              virtqueue_detach_element(svq->vq, elem, 0);
          }