| Message ID | 20221130081120.2620722-1-lulu@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | vhost: Not return fail while the device does not support send_iotlb_msg | expand |
On Wed, Nov 30, 2022 at 4:11 PM Cindy Lu <lulu@redhat.com> wrote: > > Some device does not support vhost_send_device_iotlb_msg() > such as vDPA device, which is as expected. So we should not > return fail here. Please explain in which case you may hit the -ENODEV and what's the side effect of this. Thanks > > Signed-off-by: Cindy Lu <lulu@redhat.com> > --- > hw/virtio/vhost-backend.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c > index 8e581575c9..9321ed9031 100644 > --- a/hw/virtio/vhost-backend.c > +++ b/hw/virtio/vhost-backend.c > @@ -360,7 +360,7 @@ int vhost_backend_update_device_iotlb(struct vhost_dev *dev, > if (dev->vhost_ops && dev->vhost_ops->vhost_send_device_iotlb_msg) > return dev->vhost_ops->vhost_send_device_iotlb_msg(dev, &imsg); > > - return -ENODEV; > + return 0; > } > > int vhost_backend_invalidate_device_iotlb(struct vhost_dev *dev, > @@ -375,7 +375,7 @@ int vhost_backend_invalidate_device_iotlb(struct vhost_dev *dev, > if (dev->vhost_ops && dev->vhost_ops->vhost_send_device_iotlb_msg) > return dev->vhost_ops->vhost_send_device_iotlb_msg(dev, &imsg); > > - return -ENODEV; > + return 0; > } > > int vhost_backend_handle_iotlb_msg(struct vhost_dev *dev, > -- > 2.34.3 >
On Thu, 1 Dec 2022 at 16:49, Jason Wang <jasowang@redhat.com> wrote: > > On Wed, Nov 30, 2022 at 4:11 PM Cindy Lu <lulu@redhat.com> wrote: > > > > Some device does not support vhost_send_device_iotlb_msg() > > such as vDPA device, which is as expected. So we should not > > return fail here. > > Please explain in which case you may hit the -ENODEV and what's the > side effect of this. > > Thanks > this issue was found during the test of virtio-iommu the step is 1. while load the VM with qemu -device virtio-iommu-pci \ -device virtio-net-pci,netdev=vhost-vdpa0,disable-legacy=on,disable-modern=off,iommu_platform=on\ -netdev type=vhost-vdpa,vhostdev=/dev/vhost-vdpa-0,id=vhost-vdpa0\ 2. the guest vm's CMDLINE //proc/cmdline don't have iommu=pt there will be a lot error message during loading VM/runing traffic ysteqemu-system-x86_64: Fail to invalidate device iotlb qemu-system-x86_64: Fail to invalidate device iotlb qemu-system-x86_64: Fail to invalidate device iotlb qemu-system-x86_64: Fail to invalidate device iotlb mqemu-system-x86_64: Fail to invalidate device iotlb qemu-system-x86_64: Fail to invalidate device iotlb qemu-system-x86_64: Fail to invalidate device iotlb qemu-system-x86_64: Fail to invalidate device iotlb qemu-system-x86_64: Fail to invalidate device iotlb qemu-system-x86_64: Fail to invalidate device iotlb dqemu-system-x86_64: Fail to invalidate device iotlb qemu-system-x86_64: Fail to invalidate device iotlb -resolved.…e - Network Name Resolution...qemu-system-x86_64: Fail to invalidate device iotlb qemu-system-x86_64: Fail to invalidate device iotlb ..... and the vdpa port in guest VM doesn't working well With this fix the guest VM load without error message and the vdpa port working correctly at the same setting [root@ubuntunew ~]# ping 111.1.1.2 PING 111.1.1.2 (111.1.1.2) 56(84) bytes of data. 64 bytes from 111.1.1.2: icmp_seq=1 ttl=64 time=25.0 ms 64 bytes from 111.1.1.2: icmp_seq=2 ttl=64 time=22.0 ms 64 bytes from 111.1.1.2: icmp_seq=3 ttl=64 time=24.3 ms Thansk Cindy > > > > Signed-off-by: Cindy Lu <lulu@redhat.com> > > --- > > hw/virtio/vhost-backend.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c > > index 8e581575c9..9321ed9031 100644 > > --- a/hw/virtio/vhost-backend.c > > +++ b/hw/virtio/vhost-backend.c > > @@ -360,7 +360,7 @@ int vhost_backend_update_device_iotlb(struct vhost_dev *dev, > > if (dev->vhost_ops && dev->vhost_ops->vhost_send_device_iotlb_msg) > > return dev->vhost_ops->vhost_send_device_iotlb_msg(dev, &imsg); > > > > - return -ENODEV; > > + return 0; > > } > > > > int vhost_backend_invalidate_device_iotlb(struct vhost_dev *dev, > > @@ -375,7 +375,7 @@ int vhost_backend_invalidate_device_iotlb(struct vhost_dev *dev, > > if (dev->vhost_ops && dev->vhost_ops->vhost_send_device_iotlb_msg) > > return dev->vhost_ops->vhost_send_device_iotlb_msg(dev, &imsg); > > > > - return -ENODEV; > > + return 0; > > } > > > > int vhost_backend_handle_iotlb_msg(struct vhost_dev *dev, > > -- > > 2.34.3 > > >
On Sat, Dec 3, 2022 at 3:38 PM Cindy Lu <lulu@redhat.com> wrote: > > On Thu, 1 Dec 2022 at 16:49, Jason Wang <jasowang@redhat.com> wrote: > > > > On Wed, Nov 30, 2022 at 4:11 PM Cindy Lu <lulu@redhat.com> wrote: > > > > > > Some device does not support vhost_send_device_iotlb_msg() > > > such as vDPA device, which is as expected. So we should not > > > return fail here. I don't see why vDPA doesn't support this function? > > > > Please explain in which case you may hit the -ENODEV and what's the > > side effect of this. > > > > Thanks > > > this issue was found during the test of virtio-iommu > the step is > 1. while load the VM with qemu > -device virtio-iommu-pci \ > -device virtio-net-pci,netdev=vhost-vdpa0,disable-legacy=on,disable-modern=off,iommu_platform=on\ > -netdev type=vhost-vdpa,vhostdev=/dev/vhost-vdpa-0,id=vhost-vdpa0\ > 2. the guest vm's CMDLINE //proc/cmdline don't have iommu=pt > there will be a lot error message during loading VM/runing traffic > ysteqemu-system-x86_64: Fail to invalidate device iotlb > qemu-system-x86_64: Fail to invalidate device iotlb > qemu-system-x86_64: Fail to invalidate device iotlb > qemu-system-x86_64: Fail to invalidate device iotlb > mqemu-system-x86_64: Fail to invalidate device iotlb > qemu-system-x86_64: Fail to invalidate device iotlb > qemu-system-x86_64: Fail to invalidate device iotlb > qemu-system-x86_64: Fail to invalidate device iotlb > qemu-system-x86_64: Fail to invalidate device iotlb > qemu-system-x86_64: Fail to invalidate device iotlb > dqemu-system-x86_64: Fail to invalidate device iotlb > qemu-system-x86_64: Fail to invalidate device iotlb > -resolved.…e - Network Name Resolution...qemu-system-x86_64: Fail to > invalidate device iotlb > qemu-system-x86_64: Fail to invalidate device iotlb > ..... > and the vdpa port in guest VM doesn't working well > > With this fix the guest VM load without error message and the vdpa > port working correctly at the > same setting > [root@ubuntunew ~]# ping 111.1.1.2 > PING 111.1.1.2 (111.1.1.2) 56(84) bytes of data. > 64 bytes from 111.1.1.2: icmp_seq=1 ttl=64 time=25.0 ms > 64 bytes from 111.1.1.2: icmp_seq=2 ttl=64 time=22.0 ms > 64 bytes from 111.1.1.2: icmp_seq=3 ttl=64 time=24.3 ms > > Thansk > Cindy > > > > > > > Signed-off-by: Cindy Lu <lulu@redhat.com> > > > --- > > > hw/virtio/vhost-backend.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c > > > index 8e581575c9..9321ed9031 100644 > > > --- a/hw/virtio/vhost-backend.c > > > +++ b/hw/virtio/vhost-backend.c > > > @@ -360,7 +360,7 @@ int vhost_backend_update_device_iotlb(struct vhost_dev *dev, > > > if (dev->vhost_ops && dev->vhost_ops->vhost_send_device_iotlb_msg) > > > return dev->vhost_ops->vhost_send_device_iotlb_msg(dev, &imsg); We need to figure out why we can get vhost_iotlb_miss() here. And if it is needed, we need to implement the ops or document why it is not needed here. It seems to be here: /* Update used ring information for IOTLB to work correctly, * 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); } The code is only needed for the kernel backend (vhost_init_used() requires it), so let's try to skip it for other backeds here. Thanks > > > > > > - return -ENODEV; > > > + return 0; > > > } > > > > > > int vhost_backend_invalidate_device_iotlb(struct vhost_dev *dev, > > > @@ -375,7 +375,7 @@ int vhost_backend_invalidate_device_iotlb(struct vhost_dev *dev, > > > if (dev->vhost_ops && dev->vhost_ops->vhost_send_device_iotlb_msg) > > > return dev->vhost_ops->vhost_send_device_iotlb_msg(dev, &imsg); > > > > > > - return -ENODEV; > > > + return 0; > > > } > > > > > > int vhost_backend_handle_iotlb_msg(struct vhost_dev *dev, > > > -- > > > 2.34.3 > > > > > >
diff --git a/hw/virtio/vhost-backend.c b/hw/virtio/vhost-backend.c index 8e581575c9..9321ed9031 100644 --- a/hw/virtio/vhost-backend.c +++ b/hw/virtio/vhost-backend.c @@ -360,7 +360,7 @@ int vhost_backend_update_device_iotlb(struct vhost_dev *dev, if (dev->vhost_ops && dev->vhost_ops->vhost_send_device_iotlb_msg) return dev->vhost_ops->vhost_send_device_iotlb_msg(dev, &imsg); - return -ENODEV; + return 0; } int vhost_backend_invalidate_device_iotlb(struct vhost_dev *dev, @@ -375,7 +375,7 @@ int vhost_backend_invalidate_device_iotlb(struct vhost_dev *dev, if (dev->vhost_ops && dev->vhost_ops->vhost_send_device_iotlb_msg) return dev->vhost_ops->vhost_send_device_iotlb_msg(dev, &imsg); - return -ENODEV; + return 0; } int vhost_backend_handle_iotlb_msg(struct vhost_dev *dev,
Some device does not support vhost_send_device_iotlb_msg() such as vDPA device, which is as expected. So we should not return fail here. Signed-off-by: Cindy Lu <lulu@redhat.com> --- hw/virtio/vhost-backend.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)