Re: [PATCH 0/3] Disable vhost device IOTLB is IOMMU is not enabled

[Jason Wang]

在 2021/8/5 上午12:08, Peter Xu 写道:
> On Wed, Aug 04, 2021 at 11:48:00AM +0800, Jason Wang wrote:
> > Hi:
> >
> > We currently try to enable device IOTLB when iommu_platform is
> > set. This may lead unnecessary trasnsactions between qemu and vhost
> > when vIOMMU is not used (which is the typical case for the encrypted
> > VM).
> >
> > So patch tries to use transport specific method to detect the enalbing
> > of vIOMMU and enable the device IOTLB only if vIOMMU is enalbed.
> Just to mention that there's also the ordering requirement for e.g. vfio-pci
> and the iommu device so far because vfio_realize() depends on vIOMMU being
> realized too, so specifying "-device vfio-pci" before "-device intel-iommu"
> will stop working, I think (see the similar pci_device_iommu_address_space()
> call in vfio_realize()).


Right, I actually try to google and end up with one patch that you 
posted to make sure vtd is initialized first.


> Do you think vhost can do the same to assume vIOMMU must be specified before
> vhost?


See below and I think it's not user friendly. I think maybe there should 
a general way to handle the order/dependency of device initialization in 
Qemu. But until that is implemented, I tend to use the "workaround" like 
this.


> Then vhost can call pci_device_iommu_address_space() freely.  It'll be
> more gentle for vhost even when it's used wrong: instead of not working at all
> (vfio-pci), vhost runs slower.


It's not about the slower, if pci_device_iommu_address_space() is used 
we will end up a wrong dma_as which breaks virtio DMA.


> Currently libvirt should be able to guarantee that ordering so libvirt users
> shouldn't need to bother.  I think libvirt should also guarantee the vIOMMU
> device to be created before all the rest devices, including virtio/vhost.  But
> need to check.  If that's the case libvirt will naturally work for vhost too.
>
> For the long term we may need to think about making device creation to be not
> ordered by user cmdline input but still has a priority specified in the code
> itself.


I fully agree.


>   Migration has something like that (MigrationPriority).  I think we
> just need something similar for general device realizations.  Since vhost
> raised the same need, I think that priority should bump up too.


Yes.


> The other concern is right now vhost has vhost_dev.dma_as but now we're not
> using it for vhost_dev_has_iommu().


If I understand correctly, both of us means using 
pci_device_iommu_address_space() in get_dma_as. If this is true, it's 
not he vhost_dev.dma_as but virtio_dev.dma_as.

So it breaks virtio actually (see above).



>    It's just a bit confusing as when to use
> which.


Yes, but I don't think a better approach.

Thanks


> What do you think?
>
> Thanks,