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

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

On Tue, Jul 15, 2025 at 10:09:49AM -0300, Jason Gunthorpe wrote:
> 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.

OK. IIUC you want 1 uAPI, TSM Bind, to finish all secure configuration,
including MMIO sharebility. I think it is possible, the MMIO shareability
is fixed after TSM Bind. iommufd could fetch TDI report to get the
private/shared MMIO ranges and callback to VFIO.

> 
> I also don't think it needs to be strictly 'in-place' as we expect the

When I said "must be in-place", I mean the MMIO resource (hpa) for one gfn is
fixed, can't have 2 copies of backend as the current private/shared
memory does.

> VM to be idle on the MMIO during this change over. Faulting would be OK.

Sorry, I don't get your point about 'strictly in-place' here?

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

Yeah, for Intel TDX, basically 2 things, zap the mapping on opposite side
page table, mark the correct shareability for correct fault in.

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

No I don't expect 2 dmabufs. I image shareability could be an physical
attribute of dmabuf and the callback to VFIO changes the shareability.
And VFIO, the dmabuf exporter, could notify KVM/iommufd about the
shareability change. Then KVM/iommufd unmaps their page tables.

Thanks,
Yilun

> expected to map/unmap in the right sequence?
> 
> Something else?
> 
> Jason