| Message ID | 20241107113247.46532-1-ppandit@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [v1] vhost: fail device start if iotlb update fails | expand |
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 >
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 >
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
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);