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