| Message ID | 20241105060053.61973-1-ppandit@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | vhost: fail device start if iotlb update fails | expand |
On Tue, Nov 05, 2024 at 11:30:53AM +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 | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > >Note: > - Earlier this patch was submitted as part of a series to fix vhost device > related issue. It remained unreviewed, because the other patch in that > series did not fix the issue entirely. > > - The error fixed in this patch is fairly independent of the other issue. > It can be reviewed as is, without waiting for the other patch in that > series. Not sure when the other patch will be revised again. > So sending this one alone, instead of holding it back. > >* https://lore.kernel.org/qemu-devel/20240808095147.291626-2-ppandit@redhat.com/ >* https://lore.kernel.org/qemu-devel/20240711131424.181615-3-ppandit@redhat.com/ > >diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c >index 06fc71746e..a70b7422b5 100644 >--- a/hw/virtio/vhost.c >+++ b/hw/virtio/vhost.c >@@ -2151,7 +2151,11 @@ 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) { >+ VHOST_OPS_DEBUG(r, "vhost_device_iotlb_miss failed"); 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. >+ goto fail_start; 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`. Thanks, Stefano >+ } > } > } > vhost_start_config_intr(hdev); >-- >2.47.0 >
diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c index 06fc71746e..a70b7422b5 100644 --- a/hw/virtio/vhost.c +++ b/hw/virtio/vhost.c @@ -2151,7 +2151,11 @@ 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) { + VHOST_OPS_DEBUG(r, "vhost_device_iotlb_miss failed"); + goto fail_start; + } } } vhost_start_config_intr(hdev);