Re: [PATCH] vhost: Not return fail while the device does not support send_iotlb_msg

[Jason Wang]

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
> > >
> >
>