Re: [PATCH RFCv1 08/14] iommufd: Add IOMMU_VIOMMU_SET_DEV_ID ioctl

[Nicolin Chen <nicolinc@nvidia.com>]

On Tue, May 21, 2024 at 03:30:11PM -0300, Jason Gunthorpe wrote:
> On Thu, May 16, 2024 at 10:14:38PM -0700, Nicolin Chen wrote:
> > I also moved xa_cmpxchg to the driver, as I feel that the vdev_id
> > here is very driver/iommu specific. There can be some complication
> > if iommufd core handles this u64 vdev_id: most likely we will use
> > this u64 vdev_id to index the xarray that takes an unsigned-long
> > xa_index for a fast vdev_id-to-pdev_id lookup, while only a driver
> > knows whether u64 vdev_id is compatible with unsigned long or not.
> 
> This seems like the most general part.. The core code should just have
> a check like:
> 
>   if (vdevid >= ULONG_MAX) return -EINVAL;

Ack.

> And if someone wants to make 32 bit kernels support a 64bit vdevid
> then they can sort it out someday :) I think this is not a big issue
> as all iommus seem to have some kind of radix lookup for vdevid and
> want it to be small.
> 
> Matthew has been talking about support for 64bit indexes in
> maple/xarray or something for a bit so it might sort itself out.

OK. In that case, the core can do the xarray maintenance.

And then..

> > And, we have a list_head in the structure idev, so a device unbind
> > will for-each the list and unset all the vdev_ids in it, meanwhile
> > the viommu stays. I wonder if we need to add another list_head in
> > the structure viommu, so a viommu tear down will for-each its list
> > and unset all the vdev_ids on its side while a device (idev) stays.
> > I don't see a use case of that though..any thought?
> 
> I think you need to support viommu teardown, at least from a
> correctness perspective. The API permits it. But isn't it just
> list_del everything in the xarray and that will clean it up enough?

... viommu tear down can xa_for_each to call unset_vdev_id().

Thanks
Nicolin