[PATCH v1] vhost: fail device start if iotlb update fails

[PATCH v1] vhost: fail device start if iotlb update fails

[Prasad Pandit]

From: Prasad Pandit <pjp@fedoraproject.org>

While starting a vhost device, updating iotlb entries
via 'vhost_device_iotlb_miss' may return an error.

  qemu-kvm: vhost_device_iotlb_miss:
    700871,700871: Fail to update device iotlb

Fail device start when such an error occurs.

Signed-off-by: Prasad Pandit <pjp@fedoraproject.org>
---
 hw/virtio/vhost.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

v1:
 - Remove VHOST_OPS_DEBUG call.
 - Call vhost_set_iotlb_callback and vhost_dev_start with 'false' argument.

v0:
 - https://lore.kernel.org/qemu-devel/20241105060053.61973-1-ppandit@redhat.com/

diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
index 76f9b2aaad..c40f48ac4d 100644
--- a/hw/virtio/vhost.c
+++ b/hw/virtio/vhost.c
@@ -2095,11 +2095,22 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev, bool vrings)
          * vhost-kernel code requires for this.*/
         for (i = 0; i < hdev->nvqs; ++i) {
             struct vhost_virtqueue *vq = hdev->vqs + i;
-            vhost_device_iotlb_miss(hdev, vq->used_phys, true);
+            r = vhost_device_iotlb_miss(hdev, vq->used_phys, true);
+            if (r) {
+                goto fail_iotlb;
+            }
         }
     }
     vhost_start_config_intr(hdev);
     return 0;
+fail_iotlb:
+    if (vhost_dev_has_iommu(hdev) &&
+        hdev->vhost_ops->vhost_set_iotlb_callback) {
+        hdev->vhost_ops->vhost_set_iotlb_callback(hdev, false);
+    }
+    if (hdev->vhost_ops->vhost_dev_start) {
+        hdev->vhost_ops->vhost_dev_start(hdev, false);
+    }
 fail_start:
     if (vrings) {
         vhost_dev_set_vring_enable(hdev, false);
--
2.47.0

Re: [PATCH v1] vhost: fail device start if iotlb update fails

[Stefano Garzarella]

On Thu, Nov 07, 2024 at 05:02:47PM +0530, Prasad Pandit wrote:
>From: Prasad Pandit <pjp@fedoraproject.org>
>
>While starting a vhost device, updating iotlb entries
>via 'vhost_device_iotlb_miss' may return an error.
>
>  qemu-kvm: vhost_device_iotlb_miss:
>    700871,700871: Fail to update device iotlb
>
>Fail device start when such an error occurs.
>
>Signed-off-by: Prasad Pandit <pjp@fedoraproject.org>
>---
> hw/virtio/vhost.c | 13 ++++++++++++-
> 1 file changed, 12 insertions(+), 1 deletion(-)
>
>v1:
> - Remove VHOST_OPS_DEBUG call.
> - Call vhost_set_iotlb_callback and vhost_dev_start with 'false' argument.

LGTM!

Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>

>
>v0:
> - https://lore.kernel.org/qemu-devel/20241105060053.61973-1-ppandit@redhat.com/
>
>diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>index 76f9b2aaad..c40f48ac4d 100644
>--- a/hw/virtio/vhost.c
>+++ b/hw/virtio/vhost.c
>@@ -2095,11 +2095,22 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev, bool vrings)
>          * vhost-kernel code requires for this.*/
>         for (i = 0; i < hdev->nvqs; ++i) {
>             struct vhost_virtqueue *vq = hdev->vqs + i;
>-            vhost_device_iotlb_miss(hdev, vq->used_phys, true);
>+            r = vhost_device_iotlb_miss(hdev, vq->used_phys, true);
>+            if (r) {
>+                goto fail_iotlb;
>+            }
>         }
>     }
>     vhost_start_config_intr(hdev);
>     return 0;
>+fail_iotlb:
>+    if (vhost_dev_has_iommu(hdev) &&
>+        hdev->vhost_ops->vhost_set_iotlb_callback) {
>+        hdev->vhost_ops->vhost_set_iotlb_callback(hdev, false);
>+    }
>+    if (hdev->vhost_ops->vhost_dev_start) {
>+        hdev->vhost_ops->vhost_dev_start(hdev, false);
>+    }
> fail_start:
>     if (vrings) {
>         vhost_dev_set_vring_enable(hdev, false);
>--
>2.47.0
>

Re: [PATCH v1] vhost: fail device start if iotlb update fails

[Jason Wang]

Hi Prasad:

On Thu, Nov 7, 2024 at 7:33 PM Prasad Pandit <ppandit@redhat.com> wrote:
>
> From: Prasad Pandit <pjp@fedoraproject.org>
>
> While starting a vhost device, updating iotlb entries
> via 'vhost_device_iotlb_miss' may return an error.
>
>   qemu-kvm: vhost_device_iotlb_miss:
>     700871,700871: Fail to update device iotlb

Actually, such updating is a workaround for vhost-net kernel to run.

I wonder what kind of issue would we get if we don't do this?

Thanks

>
> Fail device start when such an error occurs.
>
> Signed-off-by: Prasad Pandit <pjp@fedoraproject.org>
> ---
>  hw/virtio/vhost.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> v1:
>  - Remove VHOST_OPS_DEBUG call.
>  - Call vhost_set_iotlb_callback and vhost_dev_start with 'false' argument.
>
> v0:
>  - https://lore.kernel.org/qemu-devel/20241105060053.61973-1-ppandit@redhat.com/
>
> diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
> index 76f9b2aaad..c40f48ac4d 100644
> --- a/hw/virtio/vhost.c
> +++ b/hw/virtio/vhost.c
> @@ -2095,11 +2095,22 @@ int vhost_dev_start(struct vhost_dev *hdev, VirtIODevice *vdev, bool vrings)
>           * vhost-kernel code requires for this.*/
>          for (i = 0; i < hdev->nvqs; ++i) {
>              struct vhost_virtqueue *vq = hdev->vqs + i;
> -            vhost_device_iotlb_miss(hdev, vq->used_phys, true);
> +            r = vhost_device_iotlb_miss(hdev, vq->used_phys, true);
> +            if (r) {
> +                goto fail_iotlb;
> +            }
>          }
>      }
>      vhost_start_config_intr(hdev);
>      return 0;
> +fail_iotlb:
> +    if (vhost_dev_has_iommu(hdev) &&
> +        hdev->vhost_ops->vhost_set_iotlb_callback) {
> +        hdev->vhost_ops->vhost_set_iotlb_callback(hdev, false);
> +    }
> +    if (hdev->vhost_ops->vhost_dev_start) {
> +        hdev->vhost_ops->vhost_dev_start(hdev, false);
> +    }
>  fail_start:
>      if (vrings) {
>          vhost_dev_set_vring_enable(hdev, false);
> --
> 2.47.0
>

Re: [PATCH v1] vhost: fail device start if iotlb update fails

[Prasad Pandit]

Hello Jason,

On Mon, 11 Nov 2024 at 07:08, Jason Wang <jasowang@redhat.com> wrote:
> > While starting a vhost device, updating iotlb entries
> > via 'vhost_device_iotlb_miss' may return an error.
> >
> >   qemu-kvm: vhost_device_iotlb_miss:
> >     700871,700871: Fail to update device iotlb
>
> Actually, such updating is a workaround for vhost-net kernel to run.
> I wonder what kind of issue would we get if we don't do this?

* During Postcopy migration, while starting the 'vhost-user' device,
update of its iotlb entries would fail and the guest on the
destination would hang, not finishing the migration. There might be
other such scenarios.

Thank you.
---
  - Prasad