Re: [PATCH v8 02/11] iommu: Add max_pasids field in struct dev_iommu

["Raj, Ashok" <ashok.raj@intel.com>]

On Tue, Jun 07, 2022 at 09:49:33AM +0800, Lu Baolu wrote:
> Use this field to save the number of PASIDs that a device is able to
> consume. It is a generic attribute of a device and lifting it into the
> per-device dev_iommu struct could help to avoid the boilerplate code
> in various IOMMU drivers.
> 
> Signed-off-by: Lu Baolu <baolu.lu@linux.intel.com>
> ---
>  include/linux/iommu.h |  2 ++
>  drivers/iommu/iommu.c | 26 ++++++++++++++++++++++++++
>  2 files changed, 28 insertions(+)
> 
> diff --git a/include/linux/iommu.h b/include/linux/iommu.h
> index 03fbb1b71536..d50afb2c9a09 100644
> --- a/include/linux/iommu.h
> +++ b/include/linux/iommu.h
> @@ -364,6 +364,7 @@ struct iommu_fault_param {
>   * @fwspec:	 IOMMU fwspec data
>   * @iommu_dev:	 IOMMU device this device is linked to
>   * @priv:	 IOMMU Driver private data
> + * @max_pasids:  number of PASIDs device can consume
>   *
>   * TODO: migrate other per device data pointers under iommu_dev_data, e.g.
>   *	struct iommu_group	*iommu_group;
> @@ -375,6 +376,7 @@ struct dev_iommu {
>  	struct iommu_fwspec		*fwspec;
>  	struct iommu_device		*iommu_dev;
>  	void				*priv;
> +	u32				max_pasids;
>  };
>  
>  int iommu_device_register(struct iommu_device *iommu,
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 847ad47a2dfd..adac85ccde73 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -20,6 +20,7 @@
>  #include <linux/idr.h>
>  #include <linux/err.h>
>  #include <linux/pci.h>
> +#include <linux/pci-ats.h>

Is this needed for this patch?

>  #include <linux/bitops.h>
>  #include <linux/property.h>
>  #include <linux/fsl/mc.h>
> @@ -218,6 +219,30 @@ static void dev_iommu_free(struct device *dev)
>  	kfree(param);
>  }
>  
> +static u32 dev_iommu_get_max_pasids(struct device *dev)
> +{
> +	u32 max_pasids = dev->iommu->iommu_dev->max_pasids;
> +	u32 num_bits;
> +	int ret;
> +
> +	if (!max_pasids)
> +		return 0;
> +
> +	if (dev_is_pci(dev)) {
> +		ret = pci_max_pasids(to_pci_dev(dev));
> +		if (ret < 0)
> +			return 0;
> +
> +		return min_t(u32, max_pasids, ret);

Ah.. that answers my other question to consider device pasid-max. I guess
if we need any enforcement of restricting devices that aren't supporting
the full PASID, that will be done by some higher layer?

too many returns in this function, maybe setup all returns to the end of
the function might be elegant?

> +	}
> +
> +	ret = device_property_read_u32(dev, "pasid-num-bits", &num_bits);
> +	if (ret)
> +		return 0;
> +
> +	return min_t(u32, max_pasids, 1UL << num_bits);
> +}
> +
>  static int __iommu_probe_device(struct device *dev, struct list_head *group_list)
>  {
>  	const struct iommu_ops *ops = dev->bus->iommu_ops;
> @@ -243,6 +268,7 @@ static int __iommu_probe_device(struct device *dev, struct list_head *group_list
>  	}
>  
>  	dev->iommu->iommu_dev = iommu_dev;
> +	dev->iommu->max_pasids = dev_iommu_get_max_pasids(dev);
>  
>  	group = iommu_group_get_for_dev(dev);
>  	if (IS_ERR(group)) {
> -- 
> 2.25.1
>