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
[email protected]
https://lists.linuxfoundation.org/mailman/listinfo/iommu
