On Tue, 5 Nov 2024 at 16:19, Stefano Garzarella <sgarz...@redhat.com> wrote:
> VHOST_OPS_DEBUG() is usually used in the error path when calling a
> `dev->vhost_ops` callback. In addition vhost_device_iotlb_miss() is
> already reporting error through error_report() in the error path, so I
> think we can remove this line.

* Okay.

> Should we add a new label in the error path and call
> `hdev->vhost_ops->vhost_dev_start` with `false`?
>
> Maybe we need to call also `hdev->vhost_ops->vhost_set_iotlb_callback`
> with `false`.
===
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index a70b7422b5..089eff438e 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -2137,15 +2137,18 @@ int vhost_dev_start(struct vhost_dev *hdev,
VirtIODevice *vdev, bool vrings)
             goto fail_log;
         }
     }
+
+    bool start = true;
+dev_restart:
     if (hdev->vhost_ops->vhost_dev_start) {
-        r = hdev->vhost_ops->vhost_dev_start(hdev, true);
+        r = hdev->vhost_ops->vhost_dev_start(hdev, start);
         if (r) {
             goto fail_start;
         }
     }
     if (vhost_dev_has_iommu(hdev) &&
         hdev->vhost_ops->vhost_set_iotlb_callback) {
-            hdev->vhost_ops->vhost_set_iotlb_callback(hdev, true);
+            hdev->vhost_ops->vhost_set_iotlb_callback(hdev, start);

         /* Update used ring information for IOTLB to work correctly,
          * vhost-kernel code requires for this.*/
@@ -2154,7 +2157,8 @@ int vhost_dev_start(struct vhost_dev *hdev,
VirtIODevice *vdev, bool vrings)
             r = vhost_device_iotlb_miss(hdev, vq->used_phys, true);
             if (r) {
                 VHOST_OPS_DEBUG(r, "vhost_device_iotlb_miss failed");
-                goto fail_start;
+                start = false;
+                goto dev_restart;
             }
         }
     }
===

* Something like above?

===
static int vhost_user_dev_start(struct vhost_dev *dev, bool started)
{
    ...
    if (started) {
        return vhost_user_add_status(dev, VIRTIO_CONFIG_S_ACKNOWLEDGE |
                                          VIRTIO_CONFIG_S_DRIVER |
                                          VIRTIO_CONFIG_S_DRIVER_OK);
    } else {
        return 0;
    }
}

static void vhost_user_set_iotlb_callback(struct vhost_dev *dev, int enabled)
{
    /* No-op as the receive channel is not dedicated to IOTLB messages. */
}
===

* Calling vhost_user_dev_start() and vhost_user_set_iotlb_callback()
with 'false' does not seem to do much. Not sure how'll that help. If
we 'goto fail_start;', libvirtd(8) might restart the guest and thus
start the vhost device again.

...wdyt?

Thank you.
---
  - Prasad


Reply via email to