On Tue, 5 Nov 2024 at 16:19, Stefano Garzarella <[email protected]> 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
