Re: [Qemu-devel] [PATCH V4 net-next] vhost_net: device IOTLB support

[Michael S. Tsirkin]

On Fri, Jan 13, 2017 at 10:45:09AM +0800, Jason Wang wrote:
> 
> 
> On 2017年01月12日 22:17, Michael S. Tsirkin wrote:
> > On Wed, Jan 11, 2017 at 12:32:12PM +0800, Jason Wang wrote:
> > > This patches implements Device IOTLB support for vhost kernel. This is
> > > done through:
> > > 
> > > 1) switch to use dma helpers when map/unmap vrings from vhost codes
> > > 2) introduce a set of VhostOps to:
> > >     - setting up device IOTLB request callback
> > >     - processing device IOTLB request
> > >     - processing device IOTLB invalidation
> > > 2) kernel support for Device IOTLB API:
> > > 
> > > - allow vhost-net to query the IOMMU IOTLB entry through eventfd
> > > - enable the ability for qemu to update a specified mapping of vhost
> > > - through ioctl.
> > > - enable the ability to invalidate a specified range of iova for the
> > >    device IOTLB of vhost through ioctl. In x86/intel_iommu case this is
> > >    triggered through iommu memory region notifier from device IOTLB
> > >    invalidation descriptor processing routine.
> > > 
> > > With all the above, kernel vhost_net can co-operate with userspace
> > > IOMMU. For vhost-user, the support could be easily done on top by
> > > implementing the VhostOps.
> > > 
> > > Cc: Michael S. Tsirkin<address@hidden>
> > > Signed-off-by: Jason Wang<address@hidden>
> > Applied, thanks!
> > 
> > > ---
> > > Changes from V4:
> > > - set iotlb callback only when IOMMU_PLATFORM is negotiated (fix
> > >    vhost-user qtest failure)
> > In fact this only checks virtio_host_has_feature - which is
> > the right thing to do, we can't trust the guest.
> > 
> > > - whitelist VIRTIO_F_IOMMU_PLATFORM instead of manually add it
> > > - keep cpu_physical_memory_map() in vhost_memory_map()
> > One further enhancement might be to detect that guest disabled
> > iommu (e.g. globally, or using iommu=pt) and disable
> > the iotlb to avoid overhead for guests which use DPDK
> > for assigned devices but not for vhost.
> > 
> > 
> 
> Yes, it's in my todo list.
> 
> Thanks

Something that I just noticed is that when user requests iommu_platform
but vhost can not provide it, this patches will just let vhost continue
without.  I think that's wrong, since iommu_platform is a security
feature, when it's not supported I think we should fail init.

-- 
MST