Re: [PATCH v5 03/19] iommu/of: Handle iommu-map property for PCI

[Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>]

On Tue, Aug 23, 2016 at 08:05:14PM +0100, Robin Murphy wrote:
> Now that we have a way to pick up the RID translation and target IOMMU,
> hook up of_iommu_configure() to bring PCI devices into the of_xlate
> mechanism and allow them IOMMU-backed DMA ops without the need for
> driver-specific handling.
> 
> Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
> ---
>  drivers/iommu/of_iommu.c | 43 ++++++++++++++++++++++++++++++++++++-------
>  1 file changed, 36 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/iommu/of_iommu.c b/drivers/iommu/of_iommu.c
> index 57f23eaaa2f9..1a65cc806898 100644
> --- a/drivers/iommu/of_iommu.c
> +++ b/drivers/iommu/of_iommu.c
> @@ -22,6 +22,7 @@
>  #include <linux/limits.h>
>  #include <linux/of.h>
>  #include <linux/of_iommu.h>
> +#include <linux/of_pci.h>
>  #include <linux/slab.h>
>  
>  static const struct of_device_id __iommu_of_table_sentinel
> @@ -134,20 +135,48 @@ const struct iommu_ops *of_iommu_get_ops(struct device_node *np)
>  	return ops;
>  }
>  
> +static int __get_pci_rid(struct pci_dev *pdev, u16 alias, void *data)
> +{
> +	struct of_phandle_args *iommu_spec = data;
> +
> +	iommu_spec->args[0] = alias;
> +	return iommu_spec->np == pdev->bus->dev.of_node;
> +}
> +
>  const struct iommu_ops *of_iommu_configure(struct device *dev,
>  					   struct device_node *master_np)
>  {
>  	struct of_phandle_args iommu_spec;
> -	struct device_node *np;
> +	struct device_node *np = NULL;
>  	const struct iommu_ops *ops = NULL;
>  	int idx = 0;
>  
> -	/*
> -	 * We can't do much for PCI devices without knowing how
> -	 * device IDs are wired up from the PCI bus to the IOMMU.
> -	 */
> -	if (dev_is_pci(dev))
> -		return NULL;
> +	if (dev_is_pci(dev)) {
> +		/*
> +		 * Start by tracing the RID alias down the PCI topology as
> +		 * far as the host bridge whose OF node we have...
> +		 */
> +		iommu_spec.np = master_np;
> +		pci_for_each_dma_alias(to_pci_dev(dev), __get_pci_rid,
> +				       &iommu_spec);
> +		/*
> +		 * ...then find out what that becomes once it escapes the PCI
> +		 * bus into the system beyond, and which IOMMU it ends up at.
> +		 */
> +		if (of_pci_map_rid(master_np, iommu_spec.args[0], "iommu-map",
> +				    "iommu-map-mask", &np, iommu_spec.args))
> +			return NULL;
> +
> +		/* We're not attempting to handle multi-alias devices yet */
> +		iommu_spec.np = np;
> +		iommu_spec.args_count = 1;
> +		ops = of_iommu_get_ops(np);
> +		if (!ops || !ops->of_xlate || ops->of_xlate(dev, &iommu_spec))
> +			ops = NULL;
> +
> +		of_node_put(np);
> +		return ops;
> +	}

I think you should stick this in a separate function, rather than inline
it into of_iommu_configure, otherwise the control flow is pretty whacky
until you realise that the PCI path and the platform device path are
mutually exclusive.

With that:

Reviewed-by: Will Deacon <will.deacon-5wv7dgnIgG8@public.gmane.org>

Will