Hi Jean-Philippe, On Fri, Oct 19, 2018 at 07:11:54PM +0100, Jean-Philippe Brucker wrote: > The allocator doesn't really belong in drivers/iommu because some > drivers would like to allocate PASIDs for devices that aren't managed by > an IOMMU, using the same ID space as IOMMU. It doesn't really belong in > drivers/pci either since platform device also support PASID. Add the > allocator in drivers/base.
I don't really buy this argument, in the end it is the IOMMU that enforces the PASID mappings for a device. Why should a device not behind an IOMMU need to allocate a pasid? Do you have examples of such devices which also don't have their own iommu built-in? > +int ioasid_for_each(struct ioasid_set *set, ioasid_iter_t func, void *data) > +{ > + int ret; > + struct ioasid_iter_data iter_data = { > + .set = set, > + .func = func, > + .data = data, > + }; > + > + idr_lock(&ioasid_idr); > + ret = idr_for_each(&ioasid_idr, ioasid_iter, &iter_data); > + idr_unlock(&ioasid_idr); > + > + return ret; > +} > +EXPORT_SYMBOL_GPL(ioasid_for_each); What is the intended use-case for this function? > +void *ioasid_find(struct ioasid_set *set, ioasid_t ioasid) > +{ > + void *priv = NULL; > + struct ioasid_data *ioasid_data; > + > + idr_lock(&ioasid_idr); > + ioasid_data = idr_find(&ioasid_idr, ioasid); > + if (ioasid_data && ioasid_data->set == set) > + priv = ioasid_data->private; > + idr_unlock(&ioasid_idr); > + > + return priv; > +} I think using the idr_lock here will become a performance problem, as we need this function in the SVA page-fault path to retrieve the mm_struct belonging to a PASID. The head comment in lib/idr.c states that it should be safe to use rcu_readlock() on read-only iterations. If so, this should be used here so that concurrent lookups are possible. Regards, Joerg _______________________________________________ iommu mailing list iommu@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/iommu