在 2021/10/1 下午3:05, Eugenio Pérez 写道:
This will make qemu aware of the device used buffers, allowing it to
write the guest memory with its contents if needed.

Since the use of vhost_virtqueue_start can unmasks and discard call
events, vhost_virtqueue_start should be modified in one of these ways:
* Split in two: One of them uses all logic to start a queue with no
   side effects for the guest, and another one tha actually assumes that
   the guest has just started the device. Vdpa should use just the
   former.
* Actually store and check if the guest notifier is masked, and do it
   conditionally.
* Left as it is, and duplicate all the logic in vhost-vdpa.

Signed-off-by: Eugenio Pérez <epere...@redhat.com>
---
  hw/virtio/vhost-shadow-virtqueue.c | 19 +++++++++++++++
  hw/virtio/vhost-vdpa.c             | 38 +++++++++++++++++++++++++++++-
  2 files changed, 56 insertions(+), 1 deletion(-)

diff --git a/hw/virtio/vhost-shadow-virtqueue.c 
b/hw/virtio/vhost-shadow-virtqueue.c
index 21dc99ab5d..3fe129cf63 100644
--- a/hw/virtio/vhost-shadow-virtqueue.c
+++ b/hw/virtio/vhost-shadow-virtqueue.c
@@ -53,6 +53,22 @@ static void vhost_handle_guest_kick(EventNotifier *n)
      event_notifier_set(&svq->kick_notifier);
  }
+/* Forward vhost notifications */
+static void vhost_svq_handle_call_no_test(EventNotifier *n)
+{
+    VhostShadowVirtqueue *svq = container_of(n, VhostShadowVirtqueue,
+                                             call_notifier);
+
+    event_notifier_set(&svq->guest_call_notifier);
+}
+
+static void vhost_svq_handle_call(EventNotifier *n)
+{
+    if (likely(event_notifier_test_and_clear(n))) {
+        vhost_svq_handle_call_no_test(n);
+    }
+}
+
  /*
   * Obtain the SVQ call notifier, where vhost device notifies SVQ that there
   * exists pending used buffers.
@@ -180,6 +196,8 @@ VhostShadowVirtqueue *vhost_svq_new(struct vhost_dev *dev, 
int idx)
      }
svq->vq = virtio_get_queue(dev->vdev, vq_idx);
+    event_notifier_set_handler(&svq->call_notifier,
+                               vhost_svq_handle_call);
      return g_steal_pointer(&svq);
err_init_call_notifier:
@@ -195,6 +213,7 @@ err_init_kick_notifier:
  void vhost_svq_free(VhostShadowVirtqueue *vq)
  {
      event_notifier_cleanup(&vq->kick_notifier);
+    event_notifier_set_handler(&vq->call_notifier, NULL);
      event_notifier_cleanup(&vq->call_notifier);
      g_free(vq);
  }
diff --git a/hw/virtio/vhost-vdpa.c b/hw/virtio/vhost-vdpa.c
index bc34de2439..6c5f4c98b8 100644
--- a/hw/virtio/vhost-vdpa.c
+++ b/hw/virtio/vhost-vdpa.c
@@ -712,13 +712,40 @@ static bool vhost_vdpa_svq_start_vq(struct vhost_dev 
*dev, unsigned idx)
  {
      struct vhost_vdpa *v = dev->opaque;
      VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, idx);
-    return vhost_svq_start(dev, idx, svq);
+    EventNotifier *vhost_call_notifier = vhost_svq_get_svq_call_notifier(svq);
+    struct vhost_vring_file vhost_call_file = {
+        .index = idx + dev->vq_index,
+        .fd = event_notifier_get_fd(vhost_call_notifier),
+    };
+    int r;
+    bool b;
+
+    /* Set shadow vq -> guest notifier */
+    assert(v->call_fd[idx]);


We need aovid the asser() here. On which case we can hit this?


+    vhost_svq_set_guest_call_notifier(svq, v->call_fd[idx]);
+
+    b = vhost_svq_start(dev, idx, svq);
+    if (unlikely(!b)) {
+        return false;
+    }
+
+    /* Set device -> SVQ notifier */
+    r = vhost_vdpa_set_vring_dev_call(dev, &vhost_call_file);
+    if (unlikely(r)) {
+        error_report("vhost_vdpa_set_vring_call for shadow vq failed");
+        return false;
+    }


Similar to kick, do we need to set_vring_call() before vhost_svq_start()?


+
+    /* Check for pending calls */
+    event_notifier_set(vhost_call_notifier);


Interesting, can this result spurious interrupt?


+    return true;
  }
static unsigned vhost_vdpa_enable_svq(struct vhost_vdpa *v, bool enable)
  {
      struct vhost_dev *hdev = v->dev;
      unsigned n;
+    int r;
if (enable == v->shadow_vqs_enabled) {
          return hdev->nvqs;
@@ -752,9 +779,18 @@ static unsigned vhost_vdpa_enable_svq(struct vhost_vdpa 
*v, bool enable)
      if (!enable) {
          /* Disable all queues or clean up failed start */
          for (n = 0; n < v->shadow_vqs->len; ++n) {
+            struct vhost_vring_file file = {
+                .index = vhost_vdpa_get_vq_index(hdev, n),
+                .fd = v->call_fd[n],
+            };
+
+            r = vhost_vdpa_set_vring_call(hdev, &file);
+            assert(r == 0);
+
              unsigned vq_idx = vhost_vdpa_get_vq_index(hdev, n);
              VhostShadowVirtqueue *svq = g_ptr_array_index(v->shadow_vqs, n);
              vhost_svq_stop(hdev, n, svq);
+            /* TODO: This can unmask or override call fd! */


I don't get this comment. Does this mean the current code can't work with mask_notifiers? If yes, this is something we need to fix.

Thanks


              vhost_virtqueue_start(hdev, hdev->vdev, &hdev->vqs[n], vq_idx);
          }


Reply via email to