Re: [RFC PATCH 3/3] iommufd/tsm: Add tsm_bind/unbind iommufd ioctls

[Jason Gunthorpe <jgg@nvidia.com>]

On Tue, Jul 15, 2025 at 06:29:35PM +0800, Xu Yilun wrote:
> On Thu, May 29, 2025 at 07:07:56PM +0530, Aneesh Kumar K.V (Arm) wrote:
> > Signed-off-by: Aneesh Kumar K.V (Arm) <aneesh.kumar@kernel.org>
> > ---
> >  drivers/iommu/iommufd/iommufd_private.h |  3 +
> >  drivers/iommu/iommufd/main.c            |  5 ++
> >  drivers/iommu/iommufd/viommu.c          | 78 +++++++++++++++++++++++++
> >  include/uapi/linux/iommufd.h            | 15 +++++
> >  4 files changed, 101 insertions(+)
> > 
> > diff --git a/drivers/iommu/iommufd/iommufd_private.h b/drivers/iommu/iommufd/iommufd_private.h
> > index 80e8c76d25f2..a323e8b18125 100644
> > --- a/drivers/iommu/iommufd/iommufd_private.h
> > +++ b/drivers/iommu/iommufd/iommufd_private.h
> > @@ -606,6 +606,8 @@ int iommufd_viommu_alloc_ioctl(struct iommufd_ucmd *ucmd);
> >  void iommufd_viommu_destroy(struct iommufd_object *obj);
> >  int iommufd_vdevice_alloc_ioctl(struct iommufd_ucmd *ucmd);
> >  void iommufd_vdevice_destroy(struct iommufd_object *obj);
> > +int iommufd_vdevice_tsm_bind_ioctl(struct iommufd_ucmd *ucmd);
> > +int iommufd_vdevice_tsm_unbind_ioctl(struct iommufd_ucmd *ucmd);
> 
> Hello:
> 
> I recently have another concern about the vdevice tsm bind/unbind API.
> And need your inputs.
> 
> According to this:
> https://lore.kernel.org/all/aC9QPoEUw_nLHhV4@google.com/
> 
> Sean illustrates the memory in-place conversion, that the memory
> owner - gmemfd should own & control the memory shareability) and
> the conversion. I.e. For in-place conversion,
> KVM_SET_MEMORY_ATTRIBUTES should be disabled.
> 
> Private/shared MMIO must be of in-place conversion, similarly it's
> the MMIO owner should be responsible for MMIO shareability, maybe adding
> some new ioctls like MMIO_CONVERT_SHARED/PRIVATE.

Except it doesn't work like that for MMIO. Shared/private is a TDI
operation only and effects the whole device. We shouldn't split it
into two actions.

I also don't think it needs to be strictly 'in-place' as we expect the
VM to be idle on the MMIO during this change over. Faulting would be OK.

> From previous discussion, VFIO is the MMIO owner (implement as dmabuf
> exporter), so manages MMIO shareability. And IOMMUFD vdevice is the TDI
> state owner for TSM bind/unbind. But MMIO shareability & TDI state are
> actually correlated, do we really want to manage them in 2 components?

Yes, we've been over this. There are two components, we have to split
it somehow. It makes more sense for iommufd to lead the ioctls because
it has more information about the full system.

Any case where we need to get back to vfio for something needs to be
managed with a callback of some kind. We need to get a list of what
those things are.

What do all the arches need here?
 - ARM I suspect has the TDI locking operation install the MMIO in the
   secure S2, not KVM?
 - AMD just leaves the MMIO mapped all the time?
 - x86 presumably needs to carefully map/unmap to KVM and iommu in the
   right sequence or you get a MCE?

So what is the plan? You want to wrap this in DMABUF, but will there
be two DMABUFS, one for secure and one for non-secure? Is userspace
expected to map/unmap in the right sequence?

Something else?

Jason