Re: [RFC PATCH v2 14/22] iommufd: Add TIO calls

[Xu Yilun <yilun.xu@linux.intel.com>]

On Wed, Feb 26, 2025 at 09:12:02AM -0400, Jason Gunthorpe wrote:
> On Wed, Feb 26, 2025 at 06:49:18PM +0800, Xu Yilun wrote:
> 
> > E.g. I don't think VFIO driver would expect its MMIO access suddenly
> > failed without knowing what happened.
> 
> What do people expect to happen here anyhow? Do you still intend to
> mmap any of the MMIO into the hypervisor? No, right? It is all locked

Not expecting mmap the MMIO, but I switched to another way. VFIO doesn't
disallow mmap until bind, and if there is mmap on bind, bind failed.
That's my understanding of your comments.

https://lore.kernel.org/kvm/Z4Hp9jvJbhW0cqWY@yilunxu-OptiPlex-7050/#t


Another concern is about dma-buf importer (e.g. KVM) mapping the MMIO.
Recall we are working on the VFIO dma-buf solution, on bind/unbind the
MMIO accessibility is being changed and importers should be notified to
remove their mapping beforehand, and rebuild later if possible.
An immediate requirement for Intel TDX is, KVM should remove secure EPT
mapping for MMIO before unbind.

So I think device is all locked down into CC mode AFTER bind and BEFORE
unbind. It doesn't seems viommu/vdevice could control bind/unbind.

There are other bus error handling cases, like AER when TDISP/SPDM/IDE
state broken, that I don't have a clear solution now. But I cannot
imagine they could be correctly handled without pci_driver support.

> down?
> 
> So perhaps the answer is that the VFIO side has to put the device into
> CC mode which disables MMAP/etc, then the viommu/vdevice iommufd
> object can control it.
> 
> > Back to your concern, I don't think it is a problem. From your patch,
> > vIOMMU doesn't know the guest BDFn by nature, it is just the user
> > stores the id in vdevice via iommufd_vdevice_alloc_ioctl(). A proper
> > VFIO API could also do this work.
> 
> We don't want duplication though. If the viommu/vdevice/vbdf are owned
> and lifecycle controlled by iommufd then the operations against them
> must go through iommufd and through it's locking regime.
> > 
> > The implementation is basically no difference from:
> > 
> > +       vdev = container_of(iommufd_get_object(ucmd->ictx, cmd->vdevice_id,
> > +                                              IOMMUFD_OBJ_VDEVICE),
> > 
> > The real concern is the device owner, VFIO, should initiate the bind.
> 
> There is a big different, the above has correct locking, the other
> does not :)

Could you elaborate more on that? Any locking problem if we implement
bind/unbind outside iommufd. Thanks in advance.

Thanks,
Yilun

> 
> Jason