Re: [PATCH] iommu: silence iommu group prints

[Russell King - ARM Linux admin <linux@armlinux.org.uk>]

On Wed, Mar 04, 2020 at 11:56:53AM +0200, Laurentiu Tudor wrote:
> From 44ae46501b5379bd0890df87fd3827248626ed03 Mon Sep 17 00:00:00 2001
> From: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> Date: Tue, 1 Oct 2019 16:22:48 +0300
> Subject: [PATCH 1/6] bus: fsl-mc: make mc work with smmu disable bypass on
> Content-Type: text/plain; charset="us-ascii"
> 
> Since this commit [1] booting kernel on MC based chips started to
> fail because this firmware starts before the kernel and as soon as
> SMMU is probed it starts to trigger contiguous faults.

I think you mean "continuous" here.

> This is a workaround that allows MC firmware to run with SMMU
> in disable bypass mode. It consists of the following steps:
>  1. pause the firmware at early boot to get a chance to setup SMMU
>  2. request direct mapping for MC device
>  3. resume the firmware
> The workaround relies on the fact that no state is lost when
> pausing / resuming firmware, as per the docs.
> With this patch, platforms with MC firmware can now boot without
> having to change the default config to set:
>   CONFIG_ARM_SMMU_DISABLE_BYPASS_BY_DEFAULT=n

This alone is a definite improvement, and has been needed for a while.
Please submit this patch with an appropriate Fixes: tag so stable trees
can pick it up.

> [1] 954a03be033 ("iommu/arm-smmu: Break insecure users by disabling bypass by default")

Please put this where you're referencing it above - it's fine to wrap
the description of the commit when using it in the body of the commit
message.  However, that should _never_ when providing a Fixes: tag
(linux-next has a script which will detect and complain about broken
Fixes: tags.)

Thanks.

> 
> Signed-off-by: Laurentiu Tudor <laurentiu.tudor@nxp.com>
> ---
>  drivers/bus/fsl-mc/fsl-mc-bus.c | 53 +++++++++++++++++++++++++++++++++
>  1 file changed, 53 insertions(+)
> 
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index a0f8854acb3a..683a6401ffe8 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -18,6 +18,8 @@
>  #include <linux/bitops.h>
>  #include <linux/msi.h>
>  #include <linux/dma-mapping.h>
> +#include <linux/iommu.h>
> +#include <linux/io.h>
>  
>  #include "fsl-mc-private.h"
>  
> @@ -889,6 +891,12 @@ static int get_mc_addr_translation_ranges(struct device *dev,
>  	return 0;
>  }
>  
> +#define FSL_MC_GCR1	0x0
> +#define GCR1_P1_STOP	BIT(31)
> +
> +static u32 boot_gcr1;
> +static void __iomem *fsl_mc_regs;
> +
>  /**
>   * fsl_mc_bus_probe - callback invoked when the root MC bus is being
>   * added
> @@ -906,6 +914,21 @@ static int fsl_mc_bus_probe(struct platform_device *pdev)
>  	struct mc_version mc_version;
>  	struct resource res;
>  
> +	/*
> +	 * The MC firmware requires full access to the whole address space plus
> +	 * it has no way of dealing with any kind of address translation, so
> +	 * request direct mapping for it.
> +	 */
> +	error = iommu_request_dm_for_dev(&pdev->dev);
> +	if (error)
> +		dev_warn(&pdev->dev, "iommu_request_dm_for_dev() failed: %d\n",
> +			 error);
> +
> +	if (fsl_mc_regs) {
> +		/* Resume the firmware */
> +		writel(boot_gcr1 & ~GCR1_P1_STOP, fsl_mc_regs + FSL_MC_GCR1);
> +	}
> +
>  	mc = devm_kzalloc(&pdev->dev, sizeof(*mc), GFP_KERNEL);
>  	if (!mc)
>  		return -ENOMEM;
> @@ -990,6 +1013,13 @@ static int fsl_mc_bus_remove(struct platform_device *pdev)
>  	if (!fsl_mc_is_root_dprc(&mc->root_mc_bus_dev->dev))
>  		return -EINVAL;
>  
> +	/*
> +	 * Pause back the firmware so that it doesn't trigger faults by the
> +	 * time SMMU gets brought down.
> +	 */
> +	writel(boot_gcr1 | GCR1_P1_STOP, fsl_mc_regs + FSL_MC_GCR1);
> +	iounmap(fsl_mc_regs);
> +
>  	fsl_mc_device_remove(mc->root_mc_bus_dev);
>  
>  	fsl_destroy_mc_io(mc->root_mc_bus_dev->mc_io);
> @@ -1018,6 +1048,8 @@ static struct platform_driver fsl_mc_bus_driver = {
>  static int __init fsl_mc_bus_driver_init(void)
>  {
>  	int error;
> +	struct device_node *np;
> +	struct resource res;
>  
>  	error = bus_register(&fsl_mc_bus_type);
>  	if (error < 0) {
> @@ -1039,9 +1071,30 @@ static int __init fsl_mc_bus_driver_init(void)
>  	if (error < 0)
>  		goto error_cleanup_dprc_driver;
>  
> +	np = of_find_matching_node(NULL, fsl_mc_bus_match_table);
> +	if (np && of_device_is_available(np)) {
> +		error = of_address_to_resource(np, 1, &res);
> +		if (error)
> +			goto error_cleanup_dprc_driver;
> +		fsl_mc_regs = ioremap(res.start, resource_size(&res));
> +		if (!fsl_mc_regs) {
> +			error = -ENXIO;
> +			goto error_cleanup_dprc_driver;
> +		}
> +
> +		boot_gcr1 = readl(fsl_mc_regs + FSL_MC_GCR1);
> +		/*
> +		 * If found running, pause MC firmware in order to get a chance
> +		 * to setup SMMU for it.
> +		 */
> +		if (!(boot_gcr1 & GCR1_P1_STOP))
> +			writel(boot_gcr1 | GCR1_P1_STOP,  fsl_mc_regs + FSL_MC_GCR1);
> +	}
> +
>  	return 0;
>  
>  error_cleanup_dprc_driver:
> +	iounmap(fsl_mc_regs);
>  	dprc_driver_exit();
>  
>  error_cleanup_driver:
> -- 
> 2.17.1
> 


-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu