Re: [PATCH/RFC 1/2] arm64: mm: Silently allow devices lacking IOMMU group

[Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>]

Hi Magnus,

On 23/01/17 12:12, Magnus Damm wrote:
> From: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
> 
> Consider failure of iommu_get_domain_for_dev() as non-critical and
> get rid of the warning printout. This allows IOMMU properties to be
> included in the DTB even though the kernel is configured with
> CONFIG_IOMMU_API=n or in case a particular IOMMU driver refuses to
> enable IOMMU support for a certain slave device and returns error
> from the ->add_device() callback.
> 
> This is only a cosmetic change that removes console warning printouts.

The warning is there for a reason - at this point, we *expected* the
device to be using an IOMMU for DMA, so a failure is significant. Rather
than masking genuine failures in other cases because your case
deliberately breaks that expectation, simply change the expectation -
i.e. rather than letting of_xlate() succeed then failing add_device()
later, reject the of_xlate() call up-front such that the DMA layer never
gets told about the IOMMU in the first place.

Robin.

> Signed-off-by: Magnus Damm <damm+renesas-yzvPICuk2ACczHhG9Qg4qA@public.gmane.org>
> ---
> 
>  arch/arm64/mm/dma-mapping.c |   10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> --- 0001/arch/arm64/mm/dma-mapping.c
> +++ work/arch/arm64/mm/dma-mapping.c	2017-01-23 20:54:40.060607110 +0900
> @@ -827,11 +827,19 @@ static bool do_iommu_attach(struct devic
>  	struct iommu_domain *domain = iommu_get_domain_for_dev(dev);
>  
>  	/*
> +	 * In case IOMMU support is excluded from the kernel or if the device
> +	 * is not hooked up to any IOMMU group then be silent and keep the
> +	 * old dma_ops.
> +	 */
> +	if (!domain)
> +		return false;
> +
> +	/*
>  	 * If the IOMMU driver has the DMA domain support that we require,
>  	 * then the IOMMU core will have already configured a group for this
>  	 * device, and allocated the default domain for that group.
>  	 */
> -	if (!domain || iommu_dma_init_domain(domain, dma_base, size, dev)) {
> +	if (iommu_dma_init_domain(domain, dma_base, size, dev)) {
>  		pr_warn("Failed to set up IOMMU for device %s; retaining platform DMA ops\n",
>  			dev_name(dev));
>  		return false;
> _______________________________________________
> iommu mailing list
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>