Re: [PATCH] iommu: silence iommu group prints

2020-03-04 Thread Laurentiu Tudor




On 04.03.2020 12:52, Russell King - ARM Linux admin wrote:

On Wed, Mar 04, 2020 at 12:33:14PM +0200, Laurentiu Tudor wrote:



On 04.03.2020 12:07, Russell King - ARM Linux admin wrote:

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 
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.


Yes, thanks.


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.


The thing is that probably this workaround will never make it in the kernel
because it questionable to say the least, e.g. see [1]. My plan is to give
this approach [2] a try sometime soon.


So, if we want to reduce the iommu noise, we need to completely break
the ability to use a mainline kernel on the LX2160A.  This doesn't
seem practical nor sensible.  Someone has to give.



Well, I think it's a bit too early for such conclusions. I'd consider 
this stuff early / experimental work, probably will take quite a while 
for the dust to settle. Anyway, I'll take care not to break the kernel 
when I'll start submitting more official patches.

For now, I'm just hoping that this stuff helps fixing your local tree.

---
Best Regards, Laurentiu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu: silence iommu group prints

2020-03-04 Thread Russell King - ARM Linux admin
On Wed, Mar 04, 2020 at 12:33:14PM +0200, Laurentiu Tudor wrote:
> 
> 
> On 04.03.2020 12:07, Russell King - ARM Linux admin wrote:
> > 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 
> > > 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.
> 
> Yes, thanks.
> 
> > > 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.
> 
> The thing is that probably this workaround will never make it in the kernel
> because it questionable to say the least, e.g. see [1]. My plan is to give
> this approach [2] a try sometime soon.
> 
> [1] https://patchwork.kernel.org/comment/23149049/
> [2] https://patchwork.kernel.org/cover/11279577/

So, if we want to reduce the iommu noise, we need to completely break
the ability to use a mainline kernel on the LX2160A.  This doesn't
seem practical nor sensible.  Someone has to give.

-- 
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


Re: [PATCH] iommu: silence iommu group prints

2020-03-04 Thread Laurentiu Tudor




On 04.03.2020 12:07, Russell King - ARM Linux admin wrote:

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 
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.


Yes, thanks.


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.


The thing is that probably this workaround will never make it in the 
kernel because it questionable to say the least, e.g. see [1]. My plan 
is to give this approach [2] a try sometime soon.


[1] https://patchwork.kernel.org/comment/23149049/
[2] https://patchwork.kernel.org/cover/11279577/

---
Best Regards, Laurentiu


[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 
---
  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 
  #include 
  #include 
+#include 
+#include 
  
  #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(>dev);
+   if (error)
+   dev_warn(>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(>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(>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(_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, );
+   if (error)
+   goto error_cleanup_dprc_driver;
+   fsl_mc_regs = ioremap(res.start, resource_size());
+   if (!fsl_mc_regs) {
+   error = -ENXIO;
+   goto error_cleanup_dprc_driver;
+  

Re: [PATCH] iommu: silence iommu group prints

2020-03-04 Thread Laurentiu Tudor



On 04.03.2020 11:51, Russell King - ARM Linux admin wrote:

On Wed, Mar 04, 2020 at 11:42:16AM +0200, Laurentiu Tudor wrote:

On 04.03.2020 11:33, Russell King - ARM Linux admin wrote:

On Wed, Mar 04, 2020 at 10:56:06AM +0200, Laurentiu Tudor wrote:


On 04.03.2020 00:17, Russell King - ARM Linux admin wrote:

On Tue, Mar 03, 2020 at 05:55:05PM +0200, Laurentiu Tudor wrote:

   From c98dc05cdd45ae923654f2427985bd28bcde4bb2 Mon Sep 17 00:00:00 2001
From: Laurentiu Tudor 
Date: Thu, 13 Feb 2020 11:59:12 +0200
Subject: [PATCH 1/4] bus: fsl-mc: add custom .dma_configure implementation
Content-Type: text/plain; charset="us-ascii"

The devices on this bus are not discovered by way of device tree
but by queries to the firmware. It makes little sense to trick the
generic of layer into thinking that these devices are of related so
that we can get our dma configuration. Instead of doing that, add
our custom dma configuration implementation.


Firstly, applying this to v5.5 results in a build failure, due to a
missing linux/iommu.h include.

Secondly, this on its own appears to make the DPAA2 network interfaces
completely disappear.  Looking in /sys/bus/fsl-mc/drivers/*, none of
the DPAA2 drivers are bound to anything, and looking in
/sys/bus/fsl-mc/devices/, there is:

lrwxrwxrwx 1 root root 0 Mar  3 22:06 dprc.1 -> 
../../../devices/platform/soc/80c00.fsl-mc/dprc.1

This is booting with u-boot, so using DT rather than ACPI.


Signed-off-by: Laurentiu Tudor 
---
drivers/bus/fsl-mc/fsl-mc-bus.c | 42 -
1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
index 36eb25f82c8e..3df015eedae4 100644
--- a/drivers/bus/fsl-mc/fsl-mc-bus.c
+++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
@@ -132,11 +132,51 @@ static int fsl_mc_bus_uevent(struct device *dev, struct 
kobj_uevent_env *env)
static int fsl_mc_dma_configure(struct device *dev)
{
struct device *dma_dev = dev;
+   struct iommu_fwspec *fwspec;
+   const struct iommu_ops *iommu_ops;
+   struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
+   int ret;
+   u32 icid;
+
+   /* Skip DMA setup for devices that are not DMA masters */
+   if (dev->type == _mc_bus_dpmcp_type ||
+   dev->type == _mc_bus_dpbp_type ||
+   dev->type == _mc_bus_dpcon_type ||
+   dev->type == _mc_bus_dpio_type)
+   return 0;
while (dev_is_fsl_mc(dma_dev))
dma_dev = dma_dev->parent;
-   return of_dma_configure(dev, dma_dev->of_node, 0);
+   fwspec = dev_iommu_fwspec_get(dma_dev);
+   if (!fwspec)
+   return -ENODEV;


The problem appears to be here - fwspec is NULL for dprc.1.


Ok, that's because the iommu config is missing from the DT node that's
exposing the MC firmware. I've attached a fresh set of patches that include
on top the missing config and a workaround that makes MC work over SMMU.
Also added the missing #include, thanks for pointing out.
Let me know how it goes.


Shouldn't patch 6 should be first in the series, so that patch 1 does
not cause a regression and bisectability damage?



Correct, width one comment: both 5 and 6 should go first. Once iommu-map is
added in the device tree the workaround for MC with the
arm-smmu.disable_bypass boot arg will no longer work.


So, wouldn't it be reasonable to arrange the patch series like that?



Sure, please see attached.

---
Best Regards, Laurentiu
>From 44ae46501b5379bd0890df87fd3827248626ed03 Mon Sep 17 00:00:00 2001
From: Laurentiu Tudor 
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.
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

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

Signed-off-by: Laurentiu Tudor 
---
 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 
 #include 
 #include 
+#include 
+#include 
 
 #include 

Re: [PATCH] iommu: silence iommu group prints

2020-03-04 Thread Russell King - ARM Linux admin
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 
> 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 
> ---
>  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 
>  #include 
>  #include 
> +#include 
> +#include 
>  
>  #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(>dev);
> + if (error)
> + dev_warn(>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(>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(>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(_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, );
> + if (error)
> + goto error_cleanup_dprc_driver;
> + fsl_mc_regs = ioremap(res.start, resource_size());
> + 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))

Re: [PATCH] iommu: silence iommu group prints

2020-03-04 Thread Russell King - ARM Linux admin
On Wed, Mar 04, 2020 at 11:42:16AM +0200, Laurentiu Tudor wrote:
> On 04.03.2020 11:33, Russell King - ARM Linux admin wrote:
> > On Wed, Mar 04, 2020 at 10:56:06AM +0200, Laurentiu Tudor wrote:
> > > 
> > > On 04.03.2020 00:17, Russell King - ARM Linux admin wrote:
> > > > On Tue, Mar 03, 2020 at 05:55:05PM +0200, Laurentiu Tudor wrote:
> > > > >   From c98dc05cdd45ae923654f2427985bd28bcde4bb2 Mon Sep 17 00:00:00 
> > > > > 2001
> > > > > From: Laurentiu Tudor 
> > > > > Date: Thu, 13 Feb 2020 11:59:12 +0200
> > > > > Subject: [PATCH 1/4] bus: fsl-mc: add custom .dma_configure 
> > > > > implementation
> > > > > Content-Type: text/plain; charset="us-ascii"
> > > > > 
> > > > > The devices on this bus are not discovered by way of device tree
> > > > > but by queries to the firmware. It makes little sense to trick the
> > > > > generic of layer into thinking that these devices are of related so
> > > > > that we can get our dma configuration. Instead of doing that, add
> > > > > our custom dma configuration implementation.
> > > > 
> > > > Firstly, applying this to v5.5 results in a build failure, due to a
> > > > missing linux/iommu.h include.
> > > > 
> > > > Secondly, this on its own appears to make the DPAA2 network interfaces
> > > > completely disappear.  Looking in /sys/bus/fsl-mc/drivers/*, none of
> > > > the DPAA2 drivers are bound to anything, and looking in
> > > > /sys/bus/fsl-mc/devices/, there is:
> > > > 
> > > > lrwxrwxrwx 1 root root 0 Mar  3 22:06 dprc.1 -> 
> > > > ../../../devices/platform/soc/80c00.fsl-mc/dprc.1
> > > > 
> > > > This is booting with u-boot, so using DT rather than ACPI.
> > > > 
> > > > > Signed-off-by: Laurentiu Tudor 
> > > > > ---
> > > > >drivers/bus/fsl-mc/fsl-mc-bus.c | 42 
> > > > > -
> > > > >1 file changed, 41 insertions(+), 1 deletion(-)
> > > > > 
> > > > > diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c 
> > > > > b/drivers/bus/fsl-mc/fsl-mc-bus.c
> > > > > index 36eb25f82c8e..3df015eedae4 100644
> > > > > --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> > > > > +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> > > > > @@ -132,11 +132,51 @@ static int fsl_mc_bus_uevent(struct device 
> > > > > *dev, struct kobj_uevent_env *env)
> > > > >static int fsl_mc_dma_configure(struct device *dev)
> > > > >{
> > > > >   struct device *dma_dev = dev;
> > > > > + struct iommu_fwspec *fwspec;
> > > > > + const struct iommu_ops *iommu_ops;
> > > > > + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
> > > > > + int ret;
> > > > > + u32 icid;
> > > > > +
> > > > > + /* Skip DMA setup for devices that are not DMA masters */
> > > > > + if (dev->type == _mc_bus_dpmcp_type ||
> > > > > + dev->type == _mc_bus_dpbp_type ||
> > > > > + dev->type == _mc_bus_dpcon_type ||
> > > > > + dev->type == _mc_bus_dpio_type)
> > > > > + return 0;
> > > > >   while (dev_is_fsl_mc(dma_dev))
> > > > >   dma_dev = dma_dev->parent;
> > > > > - return of_dma_configure(dev, dma_dev->of_node, 0);
> > > > > + fwspec = dev_iommu_fwspec_get(dma_dev);
> > > > > + if (!fwspec)
> > > > > + return -ENODEV;
> > > > 
> > > > The problem appears to be here - fwspec is NULL for dprc.1.
> > > 
> > > Ok, that's because the iommu config is missing from the DT node that's
> > > exposing the MC firmware. I've attached a fresh set of patches that 
> > > include
> > > on top the missing config and a workaround that makes MC work over SMMU.
> > > Also added the missing #include, thanks for pointing out.
> > > Let me know how it goes.
> > 
> > Shouldn't patch 6 should be first in the series, so that patch 1 does
> > not cause a regression and bisectability damage?
> > 
> 
> Correct, width one comment: both 5 and 6 should go first. Once iommu-map is
> added in the device tree the workaround for MC with the
> arm-smmu.disable_bypass boot arg will no longer work.

So, wouldn't it be reasonable to arrange the patch series like that?

-- 
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


Re: [PATCH] iommu: silence iommu group prints

2020-03-04 Thread Laurentiu Tudor




On 04.03.2020 11:33, Russell King - ARM Linux admin wrote:

On Wed, Mar 04, 2020 at 10:56:06AM +0200, Laurentiu Tudor wrote:


On 04.03.2020 00:17, Russell King - ARM Linux admin wrote:

On Tue, Mar 03, 2020 at 05:55:05PM +0200, Laurentiu Tudor wrote:

  From c98dc05cdd45ae923654f2427985bd28bcde4bb2 Mon Sep 17 00:00:00 2001
From: Laurentiu Tudor 
Date: Thu, 13 Feb 2020 11:59:12 +0200
Subject: [PATCH 1/4] bus: fsl-mc: add custom .dma_configure implementation
Content-Type: text/plain; charset="us-ascii"

The devices on this bus are not discovered by way of device tree
but by queries to the firmware. It makes little sense to trick the
generic of layer into thinking that these devices are of related so
that we can get our dma configuration. Instead of doing that, add
our custom dma configuration implementation.


Firstly, applying this to v5.5 results in a build failure, due to a
missing linux/iommu.h include.

Secondly, this on its own appears to make the DPAA2 network interfaces
completely disappear.  Looking in /sys/bus/fsl-mc/drivers/*, none of
the DPAA2 drivers are bound to anything, and looking in
/sys/bus/fsl-mc/devices/, there is:

lrwxrwxrwx 1 root root 0 Mar  3 22:06 dprc.1 -> 
../../../devices/platform/soc/80c00.fsl-mc/dprc.1

This is booting with u-boot, so using DT rather than ACPI.


Signed-off-by: Laurentiu Tudor 
---
   drivers/bus/fsl-mc/fsl-mc-bus.c | 42 -
   1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
index 36eb25f82c8e..3df015eedae4 100644
--- a/drivers/bus/fsl-mc/fsl-mc-bus.c
+++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
@@ -132,11 +132,51 @@ static int fsl_mc_bus_uevent(struct device *dev, struct 
kobj_uevent_env *env)
   static int fsl_mc_dma_configure(struct device *dev)
   {
struct device *dma_dev = dev;
+   struct iommu_fwspec *fwspec;
+   const struct iommu_ops *iommu_ops;
+   struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
+   int ret;
+   u32 icid;
+
+   /* Skip DMA setup for devices that are not DMA masters */
+   if (dev->type == _mc_bus_dpmcp_type ||
+   dev->type == _mc_bus_dpbp_type ||
+   dev->type == _mc_bus_dpcon_type ||
+   dev->type == _mc_bus_dpio_type)
+   return 0;
while (dev_is_fsl_mc(dma_dev))
dma_dev = dma_dev->parent;
-   return of_dma_configure(dev, dma_dev->of_node, 0);
+   fwspec = dev_iommu_fwspec_get(dma_dev);
+   if (!fwspec)
+   return -ENODEV;


The problem appears to be here - fwspec is NULL for dprc.1.


Ok, that's because the iommu config is missing from the DT node that's
exposing the MC firmware. I've attached a fresh set of patches that include
on top the missing config and a workaround that makes MC work over SMMU.
Also added the missing #include, thanks for pointing out.
Let me know how it goes.


Shouldn't patch 6 should be first in the series, so that patch 1 does
not cause a regression and bisectability damage?



Correct, width one comment: both 5 and 6 should go first. Once iommu-map 
is added in the device tree the workaround for MC with the 
arm-smmu.disable_bypass boot arg will no longer work.


---
Best Regards, Laurentiu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu: silence iommu group prints

2020-03-04 Thread Russell King - ARM Linux admin
On Wed, Mar 04, 2020 at 10:56:06AM +0200, Laurentiu Tudor wrote:
> 
> On 04.03.2020 00:17, Russell King - ARM Linux admin wrote:
> > On Tue, Mar 03, 2020 at 05:55:05PM +0200, Laurentiu Tudor wrote:
> > >  From c98dc05cdd45ae923654f2427985bd28bcde4bb2 Mon Sep 17 00:00:00 2001
> > > From: Laurentiu Tudor 
> > > Date: Thu, 13 Feb 2020 11:59:12 +0200
> > > Subject: [PATCH 1/4] bus: fsl-mc: add custom .dma_configure implementation
> > > Content-Type: text/plain; charset="us-ascii"
> > > 
> > > The devices on this bus are not discovered by way of device tree
> > > but by queries to the firmware. It makes little sense to trick the
> > > generic of layer into thinking that these devices are of related so
> > > that we can get our dma configuration. Instead of doing that, add
> > > our custom dma configuration implementation.
> > 
> > Firstly, applying this to v5.5 results in a build failure, due to a
> > missing linux/iommu.h include.
> > 
> > Secondly, this on its own appears to make the DPAA2 network interfaces
> > completely disappear.  Looking in /sys/bus/fsl-mc/drivers/*, none of
> > the DPAA2 drivers are bound to anything, and looking in
> > /sys/bus/fsl-mc/devices/, there is:
> > 
> > lrwxrwxrwx 1 root root 0 Mar  3 22:06 dprc.1 -> 
> > ../../../devices/platform/soc/80c00.fsl-mc/dprc.1
> > 
> > This is booting with u-boot, so using DT rather than ACPI.
> > 
> > > Signed-off-by: Laurentiu Tudor 
> > > ---
> > >   drivers/bus/fsl-mc/fsl-mc-bus.c | 42 -
> > >   1 file changed, 41 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c 
> > > b/drivers/bus/fsl-mc/fsl-mc-bus.c
> > > index 36eb25f82c8e..3df015eedae4 100644
> > > --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> > > +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> > > @@ -132,11 +132,51 @@ static int fsl_mc_bus_uevent(struct device *dev, 
> > > struct kobj_uevent_env *env)
> > >   static int fsl_mc_dma_configure(struct device *dev)
> > >   {
> > >   struct device *dma_dev = dev;
> > > + struct iommu_fwspec *fwspec;
> > > + const struct iommu_ops *iommu_ops;
> > > + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
> > > + int ret;
> > > + u32 icid;
> > > +
> > > + /* Skip DMA setup for devices that are not DMA masters */
> > > + if (dev->type == _mc_bus_dpmcp_type ||
> > > + dev->type == _mc_bus_dpbp_type ||
> > > + dev->type == _mc_bus_dpcon_type ||
> > > + dev->type == _mc_bus_dpio_type)
> > > + return 0;
> > >   while (dev_is_fsl_mc(dma_dev))
> > >   dma_dev = dma_dev->parent;
> > > - return of_dma_configure(dev, dma_dev->of_node, 0);
> > > + fwspec = dev_iommu_fwspec_get(dma_dev);
> > > + if (!fwspec)
> > > + return -ENODEV;
> > 
> > The problem appears to be here - fwspec is NULL for dprc.1.
> 
> Ok, that's because the iommu config is missing from the DT node that's
> exposing the MC firmware. I've attached a fresh set of patches that include
> on top the missing config and a workaround that makes MC work over SMMU.
> Also added the missing #include, thanks for pointing out.
> Let me know how it goes.

Shouldn't patch 6 should be first in the series, so that patch 1 does
not cause a regression and bisectability damage?

-- 
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


Re: [PATCH] iommu: silence iommu group prints

2020-03-04 Thread Laurentiu Tudor


On 04.03.2020 00:17, Russell King - ARM Linux admin wrote:

On Tue, Mar 03, 2020 at 05:55:05PM +0200, Laurentiu Tudor wrote:

 From c98dc05cdd45ae923654f2427985bd28bcde4bb2 Mon Sep 17 00:00:00 2001
From: Laurentiu Tudor 
Date: Thu, 13 Feb 2020 11:59:12 +0200
Subject: [PATCH 1/4] bus: fsl-mc: add custom .dma_configure implementation
Content-Type: text/plain; charset="us-ascii"

The devices on this bus are not discovered by way of device tree
but by queries to the firmware. It makes little sense to trick the
generic of layer into thinking that these devices are of related so
that we can get our dma configuration. Instead of doing that, add
our custom dma configuration implementation.


Firstly, applying this to v5.5 results in a build failure, due to a
missing linux/iommu.h include.

Secondly, this on its own appears to make the DPAA2 network interfaces
completely disappear.  Looking in /sys/bus/fsl-mc/drivers/*, none of
the DPAA2 drivers are bound to anything, and looking in
/sys/bus/fsl-mc/devices/, there is:

lrwxrwxrwx 1 root root 0 Mar  3 22:06 dprc.1 -> 
../../../devices/platform/soc/80c00.fsl-mc/dprc.1

This is booting with u-boot, so using DT rather than ACPI.


Signed-off-by: Laurentiu Tudor 
---
  drivers/bus/fsl-mc/fsl-mc-bus.c | 42 -
  1 file changed, 41 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
index 36eb25f82c8e..3df015eedae4 100644
--- a/drivers/bus/fsl-mc/fsl-mc-bus.c
+++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
@@ -132,11 +132,51 @@ static int fsl_mc_bus_uevent(struct device *dev, struct 
kobj_uevent_env *env)
  static int fsl_mc_dma_configure(struct device *dev)
  {
struct device *dma_dev = dev;
+   struct iommu_fwspec *fwspec;
+   const struct iommu_ops *iommu_ops;
+   struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
+   int ret;
+   u32 icid;
+
+   /* Skip DMA setup for devices that are not DMA masters */
+   if (dev->type == _mc_bus_dpmcp_type ||
+   dev->type == _mc_bus_dpbp_type ||
+   dev->type == _mc_bus_dpcon_type ||
+   dev->type == _mc_bus_dpio_type)
+   return 0;
  
  	while (dev_is_fsl_mc(dma_dev))

dma_dev = dma_dev->parent;
  
-	return of_dma_configure(dev, dma_dev->of_node, 0);

+   fwspec = dev_iommu_fwspec_get(dma_dev);
+   if (!fwspec)
+   return -ENODEV;


The problem appears to be here - fwspec is NULL for dprc.1.


Ok, that's because the iommu config is missing from the DT node that's 
exposing the MC firmware. I've attached a fresh set of patches that 
include on top the missing config and a workaround that makes MC work 
over SMMU. Also added the missing #include, thanks for pointing out.

Let me know how it goes.

---
Best Regards, Laurentiu
>From 3d418f04fed9c10b64b3b861d66378a8f7514cc4 Mon Sep 17 00:00:00 2001
From: Laurentiu Tudor 
Date: Thu, 13 Feb 2020 11:59:12 +0200
Subject: [PATCH 1/6] bus: fsl-mc: add custom .dma_configure implementation
Content-Type: text/plain; charset="us-ascii"

The devices on this bus are not discovered by way of device tree
but by queries to the firmware. It makes little sense to trick the
generic of layer into thinking that these devices are of related so
that we can get our dma configuration. Instead of doing that, add
our custom dma configuration implementation.

Signed-off-by: Laurentiu Tudor 
---
 drivers/bus/fsl-mc/fsl-mc-bus.c | 43 -
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
index a0f8854acb3a..e2682fbefb42 100644
--- a/drivers/bus/fsl-mc/fsl-mc-bus.c
+++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
@@ -18,6 +18,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "fsl-mc-private.h"
 
@@ -130,11 +131,51 @@ static int fsl_mc_bus_uevent(struct device *dev, struct kobj_uevent_env *env)
 static int fsl_mc_dma_configure(struct device *dev)
 {
 	struct device *dma_dev = dev;
+	struct iommu_fwspec *fwspec;
+	const struct iommu_ops *iommu_ops;
+	struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
+	int ret;
+	u32 icid;
+
+	/* Skip DMA setup for devices that are not DMA masters */
+	if (dev->type == _mc_bus_dpmcp_type ||
+	dev->type == _mc_bus_dpbp_type ||
+	dev->type == _mc_bus_dpcon_type ||
+	dev->type == _mc_bus_dpio_type)
+		return 0;
 
 	while (dev_is_fsl_mc(dma_dev))
 		dma_dev = dma_dev->parent;
 
-	return of_dma_configure(dev, dma_dev->of_node, 0);
+	fwspec = dev_iommu_fwspec_get(dma_dev);
+	if (!fwspec)
+		return -ENODEV;
+	iommu_ops = iommu_ops_from_fwnode(fwspec->iommu_fwnode);
+	if (!iommu_ops)
+		return -ENODEV;
+
+	ret = iommu_fwspec_init(dev, fwspec->iommu_fwnode, iommu_ops);
+	if (ret)
+		return ret;
+
+	icid = mc_dev->icid;
+	ret = iommu_fwspec_add_ids(dev, , 1);
+	if (ret) {
+		iommu_fwspec_free(dev);
+		return ret;
+	}
+
+	if (!device_iommu_mapped(dev)) {
+		ret = 

Re: [PATCH] iommu: silence iommu group prints

2020-03-03 Thread Russell King - ARM Linux admin
On Tue, Mar 03, 2020 at 05:55:05PM +0200, Laurentiu Tudor wrote:
> From c98dc05cdd45ae923654f2427985bd28bcde4bb2 Mon Sep 17 00:00:00 2001
> From: Laurentiu Tudor 
> Date: Thu, 13 Feb 2020 11:59:12 +0200
> Subject: [PATCH 1/4] bus: fsl-mc: add custom .dma_configure implementation
> Content-Type: text/plain; charset="us-ascii"
> 
> The devices on this bus are not discovered by way of device tree
> but by queries to the firmware. It makes little sense to trick the
> generic of layer into thinking that these devices are of related so
> that we can get our dma configuration. Instead of doing that, add
> our custom dma configuration implementation.

Firstly, applying this to v5.5 results in a build failure, due to a
missing linux/iommu.h include.

Secondly, this on its own appears to make the DPAA2 network interfaces
completely disappear.  Looking in /sys/bus/fsl-mc/drivers/*, none of
the DPAA2 drivers are bound to anything, and looking in
/sys/bus/fsl-mc/devices/, there is:

lrwxrwxrwx 1 root root 0 Mar  3 22:06 dprc.1 -> 
../../../devices/platform/soc/80c00.fsl-mc/dprc.1

This is booting with u-boot, so using DT rather than ACPI.

> Signed-off-by: Laurentiu Tudor 
> ---
>  drivers/bus/fsl-mc/fsl-mc-bus.c | 42 -
>  1 file changed, 41 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/bus/fsl-mc/fsl-mc-bus.c b/drivers/bus/fsl-mc/fsl-mc-bus.c
> index 36eb25f82c8e..3df015eedae4 100644
> --- a/drivers/bus/fsl-mc/fsl-mc-bus.c
> +++ b/drivers/bus/fsl-mc/fsl-mc-bus.c
> @@ -132,11 +132,51 @@ static int fsl_mc_bus_uevent(struct device *dev, struct 
> kobj_uevent_env *env)
>  static int fsl_mc_dma_configure(struct device *dev)
>  {
>   struct device *dma_dev = dev;
> + struct iommu_fwspec *fwspec;
> + const struct iommu_ops *iommu_ops;
> + struct fsl_mc_device *mc_dev = to_fsl_mc_device(dev);
> + int ret;
> + u32 icid;
> +
> + /* Skip DMA setup for devices that are not DMA masters */
> + if (dev->type == _mc_bus_dpmcp_type ||
> + dev->type == _mc_bus_dpbp_type ||
> + dev->type == _mc_bus_dpcon_type ||
> + dev->type == _mc_bus_dpio_type)
> + return 0;
>  
>   while (dev_is_fsl_mc(dma_dev))
>   dma_dev = dma_dev->parent;
>  
> - return of_dma_configure(dev, dma_dev->of_node, 0);
> + fwspec = dev_iommu_fwspec_get(dma_dev);
> + if (!fwspec)
> + return -ENODEV;

The problem appears to be here - fwspec is NULL for dprc.1.

> + iommu_ops = iommu_ops_from_fwnode(fwspec->iommu_fwnode);
> + if (!iommu_ops)
> + return -ENODEV;
> +
> + ret = iommu_fwspec_init(dev, fwspec->iommu_fwnode, iommu_ops);
> + if (ret)
> + return ret;
> +
> + icid = mc_dev->icid;
> + ret = iommu_fwspec_add_ids(dev, , 1);
> + if (ret) {
> + iommu_fwspec_free(dev);
> + return ret;
> + }
> +
> + if (!device_iommu_mapped(dev)) {
> + ret = iommu_probe_device(dev);
> + if (ret) {
> + iommu_fwspec_free(dev);
> + return ret;
> + }
> + }
> +
> + arch_setup_dma_ops(dev, 0, *dma_dev->dma_mask + 1, iommu_ops, true);
> +
> + return 0;
>  }
>  
>  static ssize_t modalias_show(struct device *dev, struct device_attribute 
> *attr,
> -- 
> 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


Re: [PATCH] iommu: silence iommu group prints

2020-03-03 Thread Laurentiu Tudor

Hi Russell,

On 03.03.2020 17:49, Russell King - ARM Linux admin wrote:

On Tue, Mar 03, 2020 at 04:18:57PM +0200, Laurentiu Tudor wrote:



On 28.02.2020 20:32, Robin Murphy wrote:

[ +Laurentiu ]

Hi Russell,

Thanks for sharing a log, now I properly understand what's up... further
comments at the end (for context).

On 28/02/2020 10:06 am, Russell King - ARM Linux admin wrote:

On Fri, Feb 28, 2020 at 09:33:40AM +, John Garry wrote:

On 28/02/2020 02:16, Lu Baolu wrote:

Hi,

On 2020/2/27 19:57, Russell King wrote:

On the LX2160A, there are lots (about 160) of IOMMU messages produced
during boot; this is excessive.  Reduce the severity of these messages
to debug level.

Signed-off-by: Russell King 
---
    drivers/iommu/iommu.c | 4 ++--
    1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3ead597e1c57..304281ec623b 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -741,7 +741,7 @@ int iommu_group_add_device(struct iommu_group
*group, struct device *dev)
    trace_add_device_to_group(group->id, dev);
-    dev_info(dev, "Adding to iommu group %d\n", group->id);
+    dev_dbg(dev, "Adding to iommu group %d\n", group->id);


I'm not strongly against this. But to me this message seems
to be a good
indicator that a device was probed successfully by the iommu subsystem.
Keeping it in the default kernel message always helps to the kernel
debugging.



I would tend to agree.


Here's the boot messages.  Notice how many of these "Adding to iommu
group" messages there are:

[    0.00] Booting Linux on physical CPU 0x00 [0x410fd083]
[    0.00] Linux version 5.5.0+ (rmk@rmk-PC) (gcc version 4.9.2
(GCC)) #655 SMP PREEMPT Fri Feb 28 09:54:47 GMT 2020
[    0.00] Machine model: SolidRun LX2160A Clearfog CX
[    0.00] earlycon: pl11 at MMIO32 0x021c (options '')
[    0.00] printk: bootconsole [pl11] enabled
[    0.00] efi: Getting EFI parameters from FDT:
[    0.00] efi: UEFI not found.
[    0.00] cma: Reserved 32 MiB at 0xf9c0
[    0.00] On node 0 totalpages: 1555968
[    0.00]   DMA zone: 4096 pages used for memmap
[    0.00]   DMA zone: 0 pages reserved
[    0.00]   DMA zone: 262144 pages, LIFO batch:63
[    0.00]   DMA32 zone: 3832 pages used for memmap
[    0.00]   DMA32 zone: 245248 pages, LIFO batch:63
[    0.00]   Normal zone: 16384 pages used for memmap
[    0.00]   Normal zone: 1048576 pages, LIFO batch:63
[    0.00] psci: probing for conduit method from DT.
[    0.00] psci: PSCIv1.1 detected in firmware.
[    0.00] psci: Using standard PSCI v0.2 function IDs
[    0.00] psci: MIGRATE_INFO_TYPE not supported.
[    0.00] psci: SMC Calling Convention v1.1
[    0.00] percpu: Embedded 31 pages/cpu s88968 r8192 d29816 u126976
[    0.00] pcpu-alloc: s88968 r8192 d29816 u126976 alloc=31*4096
[    0.00] pcpu-alloc: [0] 00 [0] 01 [0] 02 [0] 03 [0] 04 [0] 05
[0] 06 [0] 07
[    0.00] pcpu-alloc: [0] 08 [0] 09 [0] 10 [0] 11 [0] 12 [0] 13
[0] 14 [0] 15
[    0.00] Detected PIPT I-cache on CPU0
[    0.00] CPU features: detected: GIC system register CPU interface
[    0.00] CPU features: detected: EL2 vector hardening
[    0.00] Speculative Store Bypass Disable mitigation not required
[    0.00] CPU features: detected: ARM erratum 1319367
[    0.00] Built 1 zonelists, mobility grouping on.  Total
pages: 1531656
[    0.00] Kernel command line: console=ttyAMA0,115200
root=PARTUUID=c7837e2f-02 rootwait earlycon=pl011,mmio32,0x21c
ramdisk_size=0 pci=pcie_bus_perf arm_smmu.disable_bypass=0
iommu.passthrough=0
[    0.00] Dentry cache hash table entries: 1048576 (order: 11,
8388608 bytes, linear)
[    0.00] Inode-cache hash table entries: 524288 (order: 10,
4194304 bytes, linear)
[    0.00] mem auto-init: stack:off, heap alloc:off, heap free:off
[    0.00] software IO TLB: mapped [mem 0xbbfff000-0xb000] (64MB)
[    0.00] Memory: 5991500K/6223872K available (10172K kernel
code, 1376K rwdata, 3888K rodata, 960K init, 4326K bss, 199604K
reserved, 32768K cma-reserved)
[    0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=16,
Nodes=1
[    0.00] rcu: Preemptible hierarchical RCU implementation.
[    0.00] rcu: RCU restricting CPUs from NR_CPUS=64 to
nr_cpu_ids=16.
[    0.00] Tasks RCU enabled.
[    0.00] rcu: RCU calculated value of scheduler-enlistment
delay is 25 jiffies.
[    0.00] rcu: Adjusting geometry for rcu_fanout_leaf=16,
nr_cpu_ids=16
[    0.00] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
[    0.00] GICv3: GIC: Using split EOI/Deactivate mode
[    0.00] GICv3: 256 SPIs implemented
[    0.00] GICv3: 0 Extended SPIs implemented
[    0.00] GICv3: Distributor has no Range Selector support
[    0.00] GICv3: 16 PPIs implemented
[    0.00] GICv3: no VLPI support, no direct LPI support
[    

Re: [PATCH] iommu: silence iommu group prints

2020-03-03 Thread Laurentiu Tudor



On 28.02.2020 20:32, Robin Murphy wrote:

[ +Laurentiu ]

Hi Russell,

Thanks for sharing a log, now I properly understand what's up... further 
comments at the end (for context).


On 28/02/2020 10:06 am, Russell King - ARM Linux admin wrote:

On Fri, Feb 28, 2020 at 09:33:40AM +, John Garry wrote:

On 28/02/2020 02:16, Lu Baolu wrote:

Hi,

On 2020/2/27 19:57, Russell King wrote:

On the LX2160A, there are lots (about 160) of IOMMU messages produced
during boot; this is excessive.  Reduce the severity of these messages
to debug level.

Signed-off-by: Russell King 
---
   drivers/iommu/iommu.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3ead597e1c57..304281ec623b 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -741,7 +741,7 @@ int iommu_group_add_device(struct iommu_group
*group, struct device *dev)
   trace_add_device_to_group(group->id, dev);
-    dev_info(dev, "Adding to iommu group %d\n", group->id);
+    dev_dbg(dev, "Adding to iommu group %d\n", group->id);


I'm not strongly against this. But to me this message seems to be a 
good

indicator that a device was probed successfully by the iommu subsystem.
Keeping it in the default kernel message always helps to the kernel
debugging.



I would tend to agree.


Here's the boot messages.  Notice how many of these "Adding to iommu
group" messages there are:

[    0.00] Booting Linux on physical CPU 0x00 [0x410fd083]
[    0.00] Linux version 5.5.0+ (rmk@rmk-PC) (gcc version 4.9.2 
(GCC)) #655 SMP PREEMPT Fri Feb 28 09:54:47 GMT 2020

[    0.00] Machine model: SolidRun LX2160A Clearfog CX
[    0.00] earlycon: pl11 at MMIO32 0x021c (options '')
[    0.00] printk: bootconsole [pl11] enabled
[    0.00] efi: Getting EFI parameters from FDT:
[    0.00] efi: UEFI not found.
[    0.00] cma: Reserved 32 MiB at 0xf9c0
[    0.00] On node 0 totalpages: 1555968
[    0.00]   DMA zone: 4096 pages used for memmap
[    0.00]   DMA zone: 0 pages reserved
[    0.00]   DMA zone: 262144 pages, LIFO batch:63
[    0.00]   DMA32 zone: 3832 pages used for memmap
[    0.00]   DMA32 zone: 245248 pages, LIFO batch:63
[    0.00]   Normal zone: 16384 pages used for memmap
[    0.00]   Normal zone: 1048576 pages, LIFO batch:63
[    0.00] psci: probing for conduit method from DT.
[    0.00] psci: PSCIv1.1 detected in firmware.
[    0.00] psci: Using standard PSCI v0.2 function IDs
[    0.00] psci: MIGRATE_INFO_TYPE not supported.
[    0.00] psci: SMC Calling Convention v1.1
[    0.00] percpu: Embedded 31 pages/cpu s88968 r8192 d29816 u126976
[    0.00] pcpu-alloc: s88968 r8192 d29816 u126976 alloc=31*4096
[    0.00] pcpu-alloc: [0] 00 [0] 01 [0] 02 [0] 03 [0] 04 [0] 05 
[0] 06 [0] 07
[    0.00] pcpu-alloc: [0] 08 [0] 09 [0] 10 [0] 11 [0] 12 [0] 13 
[0] 14 [0] 15

[    0.00] Detected PIPT I-cache on CPU0
[    0.00] CPU features: detected: GIC system register CPU interface
[    0.00] CPU features: detected: EL2 vector hardening
[    0.00] Speculative Store Bypass Disable mitigation not required
[    0.00] CPU features: detected: ARM erratum 1319367
[    0.00] Built 1 zonelists, mobility grouping on.  Total pages: 
1531656
[    0.00] Kernel command line: console=ttyAMA0,115200 
root=PARTUUID=c7837e2f-02 rootwait earlycon=pl011,mmio32,0x21c 
ramdisk_size=0 pci=pcie_bus_perf arm_smmu.disable_bypass=0 
iommu.passthrough=0
[    0.00] Dentry cache hash table entries: 1048576 (order: 11, 
8388608 bytes, linear)
[    0.00] Inode-cache hash table entries: 524288 (order: 10, 
4194304 bytes, linear)

[    0.00] mem auto-init: stack:off, heap alloc:off, heap free:off
[    0.00] software IO TLB: mapped [mem 0xbbfff000-0xb000] (64MB)
[    0.00] Memory: 5991500K/6223872K available (10172K kernel 
code, 1376K rwdata, 3888K rodata, 960K init, 4326K bss, 199604K 
reserved, 32768K cma-reserved)
[    0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=16, 
Nodes=1

[    0.00] rcu: Preemptible hierarchical RCU implementation.
[    0.00] rcu: RCU restricting CPUs from NR_CPUS=64 to 
nr_cpu_ids=16.

[    0.00] Tasks RCU enabled.
[    0.00] rcu: RCU calculated value of scheduler-enlistment delay 
is 25 jiffies.
[    0.00] rcu: Adjusting geometry for rcu_fanout_leaf=16, 
nr_cpu_ids=16

[    0.00] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
[    0.00] GICv3: GIC: Using split EOI/Deactivate mode
[    0.00] GICv3: 256 SPIs implemented
[    0.00] GICv3: 0 Extended SPIs implemented
[    0.00] GICv3: Distributor has no Range Selector support
[    0.00] GICv3: 16 PPIs implemented
[    0.00] GICv3: no VLPI support, no direct LPI support
[    0.00] GICv3: CPU0: found redistributor 0 region 
0:0x0620

[    0.00] ITS [mem 0x0602-0x0603]
[ 

Re: [PATCH] iommu: silence iommu group prints

2020-03-03 Thread Russell King - ARM Linux admin
On Tue, Mar 03, 2020 at 04:18:57PM +0200, Laurentiu Tudor wrote:
> 
> 
> On 28.02.2020 20:32, Robin Murphy wrote:
> > [ +Laurentiu ]
> > 
> > Hi Russell,
> > 
> > Thanks for sharing a log, now I properly understand what's up... further
> > comments at the end (for context).
> > 
> > On 28/02/2020 10:06 am, Russell King - ARM Linux admin wrote:
> > > On Fri, Feb 28, 2020 at 09:33:40AM +, John Garry wrote:
> > > > On 28/02/2020 02:16, Lu Baolu wrote:
> > > > > Hi,
> > > > > 
> > > > > On 2020/2/27 19:57, Russell King wrote:
> > > > > > On the LX2160A, there are lots (about 160) of IOMMU messages 
> > > > > > produced
> > > > > > during boot; this is excessive.  Reduce the severity of these 
> > > > > > messages
> > > > > > to debug level.
> > > > > > 
> > > > > > Signed-off-by: Russell King 
> > > > > > ---
> > > > > >    drivers/iommu/iommu.c | 4 ++--
> > > > > >    1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > > 
> > > > > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > > > > > index 3ead597e1c57..304281ec623b 100644
> > > > > > --- a/drivers/iommu/iommu.c
> > > > > > +++ b/drivers/iommu/iommu.c
> > > > > > @@ -741,7 +741,7 @@ int iommu_group_add_device(struct iommu_group
> > > > > > *group, struct device *dev)
> > > > > >    trace_add_device_to_group(group->id, dev);
> > > > > > -    dev_info(dev, "Adding to iommu group %d\n", group->id);
> > > > > > +    dev_dbg(dev, "Adding to iommu group %d\n", group->id);
> > > > > 
> > > > > I'm not strongly against this. But to me this message seems
> > > > > to be a good
> > > > > indicator that a device was probed successfully by the iommu 
> > > > > subsystem.
> > > > > Keeping it in the default kernel message always helps to the kernel
> > > > > debugging.
> > > > > 
> > > > 
> > > > I would tend to agree.
> > > 
> > > Here's the boot messages.  Notice how many of these "Adding to iommu
> > > group" messages there are:
> > > 
> > > [    0.00] Booting Linux on physical CPU 0x00 [0x410fd083]
> > > [    0.00] Linux version 5.5.0+ (rmk@rmk-PC) (gcc version 4.9.2
> > > (GCC)) #655 SMP PREEMPT Fri Feb 28 09:54:47 GMT 2020
> > > [    0.00] Machine model: SolidRun LX2160A Clearfog CX
> > > [    0.00] earlycon: pl11 at MMIO32 0x021c (options '')
> > > [    0.00] printk: bootconsole [pl11] enabled
> > > [    0.00] efi: Getting EFI parameters from FDT:
> > > [    0.00] efi: UEFI not found.
> > > [    0.00] cma: Reserved 32 MiB at 0xf9c0
> > > [    0.00] On node 0 totalpages: 1555968
> > > [    0.00]   DMA zone: 4096 pages used for memmap
> > > [    0.00]   DMA zone: 0 pages reserved
> > > [    0.00]   DMA zone: 262144 pages, LIFO batch:63
> > > [    0.00]   DMA32 zone: 3832 pages used for memmap
> > > [    0.00]   DMA32 zone: 245248 pages, LIFO batch:63
> > > [    0.00]   Normal zone: 16384 pages used for memmap
> > > [    0.00]   Normal zone: 1048576 pages, LIFO batch:63
> > > [    0.00] psci: probing for conduit method from DT.
> > > [    0.00] psci: PSCIv1.1 detected in firmware.
> > > [    0.00] psci: Using standard PSCI v0.2 function IDs
> > > [    0.00] psci: MIGRATE_INFO_TYPE not supported.
> > > [    0.00] psci: SMC Calling Convention v1.1
> > > [    0.00] percpu: Embedded 31 pages/cpu s88968 r8192 d29816 u126976
> > > [    0.00] pcpu-alloc: s88968 r8192 d29816 u126976 alloc=31*4096
> > > [    0.00] pcpu-alloc: [0] 00 [0] 01 [0] 02 [0] 03 [0] 04 [0] 05
> > > [0] 06 [0] 07
> > > [    0.00] pcpu-alloc: [0] 08 [0] 09 [0] 10 [0] 11 [0] 12 [0] 13
> > > [0] 14 [0] 15
> > > [    0.00] Detected PIPT I-cache on CPU0
> > > [    0.00] CPU features: detected: GIC system register CPU interface
> > > [    0.00] CPU features: detected: EL2 vector hardening
> > > [    0.00] Speculative Store Bypass Disable mitigation not required
> > > [    0.00] CPU features: detected: ARM erratum 1319367
> > > [    0.00] Built 1 zonelists, mobility grouping on.  Total
> > > pages: 1531656
> > > [    0.00] Kernel command line: console=ttyAMA0,115200
> > > root=PARTUUID=c7837e2f-02 rootwait earlycon=pl011,mmio32,0x21c
> > > ramdisk_size=0 pci=pcie_bus_perf arm_smmu.disable_bypass=0
> > > iommu.passthrough=0
> > > [    0.00] Dentry cache hash table entries: 1048576 (order: 11,
> > > 8388608 bytes, linear)
> > > [    0.00] Inode-cache hash table entries: 524288 (order: 10,
> > > 4194304 bytes, linear)
> > > [    0.00] mem auto-init: stack:off, heap alloc:off, heap free:off
> > > [    0.00] software IO TLB: mapped [mem 0xbbfff000-0xb000] (64MB)
> > > [    0.00] Memory: 5991500K/6223872K available (10172K kernel
> > > code, 1376K rwdata, 3888K rodata, 960K init, 4326K bss, 199604K
> > > reserved, 32768K cma-reserved)
> > > [    0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=16,
> > > Nodes=1
> > > [    0.00] rcu: Preemptible hierarchical RCU implementation.
> > > [ 

Re: [PATCH] iommu: silence iommu group prints

2020-03-02 Thread Joerg Roedel
On Thu, Feb 27, 2020 at 11:57:52AM +, Russell King wrote:
> On the LX2160A, there are lots (about 160) of IOMMU messages produced
> during boot; this is excessive.  Reduce the severity of these messages
> to debug level.

No, these messages are a very useful tool when it comes to debugging
IOMMU issues on other platforms, and the user only has to provide dmesg
output. When your system has 160 devices, so be it.

Regards,

Joerg
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


RE: [PATCH] iommu: silence iommu group prints

2020-03-02 Thread Laurentiu Tudor
Hi Robin,

> -Original Message-
> From: Robin Murphy 
> Sent: Friday, February 28, 2020 8:32 PM
> 
> [ +Laurentiu ]
> 
> Hi Russell,
> 
> Thanks for sharing a log, now I properly understand what's up... further
> comments at the end (for context).
> 
> On 28/02/2020 10:06 am, Russell King - ARM Linux admin wrote:
> > On Fri, Feb 28, 2020 at 09:33:40AM +, John Garry wrote:
> >> On 28/02/2020 02:16, Lu Baolu wrote:
> >>> Hi,
> >>>
> >>> On 2020/2/27 19:57, Russell King wrote:
>  On the LX2160A, there are lots (about 160) of IOMMU messages produced
>  during boot; this is excessive.  Reduce the severity of these
> messages
>  to debug level.
> 
>  Signed-off-by: Russell King 
>  ---
>     drivers/iommu/iommu.c | 4 ++--
>     1 file changed, 2 insertions(+), 2 deletions(-)
> 
>  diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
>  index 3ead597e1c57..304281ec623b 100644
>  --- a/drivers/iommu/iommu.c
>  +++ b/drivers/iommu/iommu.c
>  @@ -741,7 +741,7 @@ int iommu_group_add_device(struct iommu_group
>  *group, struct device *dev)
>     trace_add_device_to_group(group->id, dev);
>  -    dev_info(dev, "Adding to iommu group %d\n", group->id);
>  +    dev_dbg(dev, "Adding to iommu group %d\n", group->id);
> >>>
> >>> I'm not strongly against this. But to me this message seems to be a
> good
> >>> indicator that a device was probed successfully by the iommu subsystem.
> >>> Keeping it in the default kernel message always helps to the kernel
> >>> debugging.
> >>>
> >>
> >> I would tend to agree.
> >

[snip]

> >
> > # dmesg |grep 'Adding to iommu' | wc -l
> > 164
> > # dmesg |grep -v 'Adding to iommu' | wc -l
> > 551
> >
> > So, 23% of the kernel messages on this platform are "Adding to iommu",
> > which is excessive.
> 
> Indeed, however I would note that on most platforms bringing up a
> network interface involves hot-adding 0 devices, so hot-adding 19
> devices as full-blown DMA masters is arguably the root of "excessive"
> already. Per the concern I initially raised, each of those messages
> represents a whole bunch of internal allocation and bookkeeping going
> on, which if it isn't necessary would be far better avoided altogether,
> than simply done more quietly.
> 
> Laurentiu, I guess at the moment the nature of the of_dma_configure()
> integration means we end up treating all DPAA2 objects identically, but
> do you think we have scope to be a bit cleverer in that regard?
> Presumably not every type of object that shows up on the fsl_mc bus is
> really an independent DMA master, so if we could skip doing the full
> DMA/IOMMU/MSI setup for the ones that don't need it, it would work out
> nicer all round. In fact your .dma_configure proposal (which I'll try to
> take a proper look at next week) couldn't have come at a better time for
> that argument :)
 
Thanks! That's a very good point - I'll check on which devices we actually use 
dma apis and filter the rest out. Will keep in mind for the next spin of the 
patches.

---
Best Regards, Laurentiu

___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu: silence iommu group prints

2020-02-28 Thread Robin Murphy

[ +Laurentiu ]

Hi Russell,

Thanks for sharing a log, now I properly understand what's up... further 
comments at the end (for context).


On 28/02/2020 10:06 am, Russell King - ARM Linux admin wrote:

On Fri, Feb 28, 2020 at 09:33:40AM +, John Garry wrote:

On 28/02/2020 02:16, Lu Baolu wrote:

Hi,

On 2020/2/27 19:57, Russell King wrote:

On the LX2160A, there are lots (about 160) of IOMMU messages produced
during boot; this is excessive.  Reduce the severity of these messages
to debug level.

Signed-off-by: Russell King 
---
   drivers/iommu/iommu.c | 4 ++--
   1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3ead597e1c57..304281ec623b 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -741,7 +741,7 @@ int iommu_group_add_device(struct iommu_group
*group, struct device *dev)
   trace_add_device_to_group(group->id, dev);
-    dev_info(dev, "Adding to iommu group %d\n", group->id);
+    dev_dbg(dev, "Adding to iommu group %d\n", group->id);


I'm not strongly against this. But to me this message seems to be a good
indicator that a device was probed successfully by the iommu subsystem.
Keeping it in the default kernel message always helps to the kernel
debugging.



I would tend to agree.


Here's the boot messages.  Notice how many of these "Adding to iommu
group" messages there are:

[0.00] Booting Linux on physical CPU 0x00 [0x410fd083]
[0.00] Linux version 5.5.0+ (rmk@rmk-PC) (gcc version 4.9.2 (GCC)) #655 
SMP PREEMPT Fri Feb 28 09:54:47 GMT 2020
[0.00] Machine model: SolidRun LX2160A Clearfog CX
[0.00] earlycon: pl11 at MMIO32 0x021c (options '')
[0.00] printk: bootconsole [pl11] enabled
[0.00] efi: Getting EFI parameters from FDT:
[0.00] efi: UEFI not found.
[0.00] cma: Reserved 32 MiB at 0xf9c0
[0.00] On node 0 totalpages: 1555968
[0.00]   DMA zone: 4096 pages used for memmap
[0.00]   DMA zone: 0 pages reserved
[0.00]   DMA zone: 262144 pages, LIFO batch:63
[0.00]   DMA32 zone: 3832 pages used for memmap
[0.00]   DMA32 zone: 245248 pages, LIFO batch:63
[0.00]   Normal zone: 16384 pages used for memmap
[0.00]   Normal zone: 1048576 pages, LIFO batch:63
[0.00] psci: probing for conduit method from DT.
[0.00] psci: PSCIv1.1 detected in firmware.
[0.00] psci: Using standard PSCI v0.2 function IDs
[0.00] psci: MIGRATE_INFO_TYPE not supported.
[0.00] psci: SMC Calling Convention v1.1
[0.00] percpu: Embedded 31 pages/cpu s88968 r8192 d29816 u126976
[0.00] pcpu-alloc: s88968 r8192 d29816 u126976 alloc=31*4096
[0.00] pcpu-alloc: [0] 00 [0] 01 [0] 02 [0] 03 [0] 04 [0] 05 [0] 06 [0] 
07
[0.00] pcpu-alloc: [0] 08 [0] 09 [0] 10 [0] 11 [0] 12 [0] 13 [0] 14 [0] 
15
[0.00] Detected PIPT I-cache on CPU0
[0.00] CPU features: detected: GIC system register CPU interface
[0.00] CPU features: detected: EL2 vector hardening
[0.00] Speculative Store Bypass Disable mitigation not required
[0.00] CPU features: detected: ARM erratum 1319367
[0.00] Built 1 zonelists, mobility grouping on.  Total pages: 1531656
[0.00] Kernel command line: console=ttyAMA0,115200 
root=PARTUUID=c7837e2f-02 rootwait earlycon=pl011,mmio32,0x21c 
ramdisk_size=0 pci=pcie_bus_perf arm_smmu.disable_bypass=0 iommu.passthrough=0
[0.00] Dentry cache hash table entries: 1048576 (order: 11, 8388608 
bytes, linear)
[0.00] Inode-cache hash table entries: 524288 (order: 10, 4194304 
bytes, linear)
[0.00] mem auto-init: stack:off, heap alloc:off, heap free:off
[0.00] software IO TLB: mapped [mem 0xbbfff000-0xb000] (64MB)
[0.00] Memory: 5991500K/6223872K available (10172K kernel code, 1376K 
rwdata, 3888K rodata, 960K init, 4326K bss, 199604K reserved, 32768K 
cma-reserved)
[0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=16, Nodes=1
[0.00] rcu: Preemptible hierarchical RCU implementation.
[0.00] rcu: RCU restricting CPUs from NR_CPUS=64 to nr_cpu_ids=16.
[0.00]  Tasks RCU enabled.
[0.00] rcu: RCU calculated value of scheduler-enlistment delay is 25 
jiffies.
[0.00] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=16
[0.00] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
[0.00] GICv3: GIC: Using split EOI/Deactivate mode
[0.00] GICv3: 256 SPIs implemented
[0.00] GICv3: 0 Extended SPIs implemented
[0.00] GICv3: Distributor has no Range Selector support
[0.00] GICv3: 16 PPIs implemented
[0.00] GICv3: no VLPI support, no direct LPI support
[0.00] GICv3: CPU0: found redistributor 0 region 0:0x0620
[0.00] ITS [mem 0x0602-0x0603]
[0.00] ITS@0x0602: allocated 65536 Devices 

Re: [PATCH] iommu: silence iommu group prints

2020-02-28 Thread Russell King - ARM Linux admin
On Fri, Feb 28, 2020 at 09:33:40AM +, John Garry wrote:
> On 28/02/2020 02:16, Lu Baolu wrote:
> > Hi,
> > 
> > On 2020/2/27 19:57, Russell King wrote:
> > > On the LX2160A, there are lots (about 160) of IOMMU messages produced
> > > during boot; this is excessive.  Reduce the severity of these messages
> > > to debug level.
> > > 
> > > Signed-off-by: Russell King 
> > > ---
> > >   drivers/iommu/iommu.c | 4 ++--
> > >   1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > > index 3ead597e1c57..304281ec623b 100644
> > > --- a/drivers/iommu/iommu.c
> > > +++ b/drivers/iommu/iommu.c
> > > @@ -741,7 +741,7 @@ int iommu_group_add_device(struct iommu_group
> > > *group, struct device *dev)
> > >   trace_add_device_to_group(group->id, dev);
> > > -    dev_info(dev, "Adding to iommu group %d\n", group->id);
> > > +    dev_dbg(dev, "Adding to iommu group %d\n", group->id);
> > 
> > I'm not strongly against this. But to me this message seems to be a good
> > indicator that a device was probed successfully by the iommu subsystem.
> > Keeping it in the default kernel message always helps to the kernel
> > debugging.
> > 
> 
> I would tend to agree.

Here's the boot messages.  Notice how many of these "Adding to iommu
group" messages there are:

[0.00] Booting Linux on physical CPU 0x00 [0x410fd083]
[0.00] Linux version 5.5.0+ (rmk@rmk-PC) (gcc version 4.9.2 (GCC)) #655 
SMP PREEMPT Fri Feb 28 09:54:47 GMT 2020
[0.00] Machine model: SolidRun LX2160A Clearfog CX
[0.00] earlycon: pl11 at MMIO32 0x021c (options '')
[0.00] printk: bootconsole [pl11] enabled
[0.00] efi: Getting EFI parameters from FDT:
[0.00] efi: UEFI not found.
[0.00] cma: Reserved 32 MiB at 0xf9c0
[0.00] On node 0 totalpages: 1555968
[0.00]   DMA zone: 4096 pages used for memmap
[0.00]   DMA zone: 0 pages reserved
[0.00]   DMA zone: 262144 pages, LIFO batch:63
[0.00]   DMA32 zone: 3832 pages used for memmap
[0.00]   DMA32 zone: 245248 pages, LIFO batch:63
[0.00]   Normal zone: 16384 pages used for memmap
[0.00]   Normal zone: 1048576 pages, LIFO batch:63
[0.00] psci: probing for conduit method from DT.
[0.00] psci: PSCIv1.1 detected in firmware.
[0.00] psci: Using standard PSCI v0.2 function IDs
[0.00] psci: MIGRATE_INFO_TYPE not supported.
[0.00] psci: SMC Calling Convention v1.1
[0.00] percpu: Embedded 31 pages/cpu s88968 r8192 d29816 u126976
[0.00] pcpu-alloc: s88968 r8192 d29816 u126976 alloc=31*4096
[0.00] pcpu-alloc: [0] 00 [0] 01 [0] 02 [0] 03 [0] 04 [0] 05 [0] 06 [0] 
07 
[0.00] pcpu-alloc: [0] 08 [0] 09 [0] 10 [0] 11 [0] 12 [0] 13 [0] 14 [0] 
15 
[0.00] Detected PIPT I-cache on CPU0
[0.00] CPU features: detected: GIC system register CPU interface
[0.00] CPU features: detected: EL2 vector hardening
[0.00] Speculative Store Bypass Disable mitigation not required
[0.00] CPU features: detected: ARM erratum 1319367
[0.00] Built 1 zonelists, mobility grouping on.  Total pages: 1531656
[0.00] Kernel command line: console=ttyAMA0,115200 
root=PARTUUID=c7837e2f-02 rootwait earlycon=pl011,mmio32,0x21c 
ramdisk_size=0 pci=pcie_bus_perf arm_smmu.disable_bypass=0 iommu.passthrough=0
[0.00] Dentry cache hash table entries: 1048576 (order: 11, 8388608 
bytes, linear)
[0.00] Inode-cache hash table entries: 524288 (order: 10, 4194304 
bytes, linear)
[0.00] mem auto-init: stack:off, heap alloc:off, heap free:off
[0.00] software IO TLB: mapped [mem 0xbbfff000-0xb000] (64MB)
[0.00] Memory: 5991500K/6223872K available (10172K kernel code, 1376K 
rwdata, 3888K rodata, 960K init, 4326K bss, 199604K reserved, 32768K 
cma-reserved)
[0.00] SLUB: HWalign=64, Order=0-3, MinObjects=0, CPUs=16, Nodes=1
[0.00] rcu: Preemptible hierarchical RCU implementation.
[0.00] rcu: RCU restricting CPUs from NR_CPUS=64 to nr_cpu_ids=16.
[0.00]  Tasks RCU enabled.
[0.00] rcu: RCU calculated value of scheduler-enlistment delay is 25 
jiffies.
[0.00] rcu: Adjusting geometry for rcu_fanout_leaf=16, nr_cpu_ids=16
[0.00] NR_IRQS: 64, nr_irqs: 64, preallocated irqs: 0
[0.00] GICv3: GIC: Using split EOI/Deactivate mode
[0.00] GICv3: 256 SPIs implemented
[0.00] GICv3: 0 Extended SPIs implemented
[0.00] GICv3: Distributor has no Range Selector support
[0.00] GICv3: 16 PPIs implemented
[0.00] GICv3: no VLPI support, no direct LPI support
[0.00] GICv3: CPU0: found redistributor 0 region 0:0x0620
[0.00] ITS [mem 0x0602-0x0603]
[0.00] ITS@0x0602: allocated 65536 Devices @2178d0 
(flat, esz 8, psz 64K, shr 0)
[0.00] 

Re: [PATCH] iommu: silence iommu group prints

2020-02-28 Thread John Garry

On 28/02/2020 02:16, Lu Baolu wrote:

Hi,

On 2020/2/27 19:57, Russell King wrote:

On the LX2160A, there are lots (about 160) of IOMMU messages produced
during boot; this is excessive.  Reduce the severity of these messages
to debug level.

Signed-off-by: Russell King 
---
  drivers/iommu/iommu.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3ead597e1c57..304281ec623b 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -741,7 +741,7 @@ int iommu_group_add_device(struct iommu_group 
*group, struct device *dev)

  trace_add_device_to_group(group->id, dev);
-    dev_info(dev, "Adding to iommu group %d\n", group->id);
+    dev_dbg(dev, "Adding to iommu group %d\n", group->id);


I'm not strongly against this. But to me this message seems to be a good
indicator that a device was probed successfully by the iommu subsystem.
Keeping it in the default kernel message always helps to the kernel
debugging.



I would tend to agree.


Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu
.


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu

Re: [PATCH] iommu: silence iommu group prints

2020-02-27 Thread Lu Baolu

Hi,

On 2020/2/27 19:57, Russell King wrote:

On the LX2160A, there are lots (about 160) of IOMMU messages produced
during boot; this is excessive.  Reduce the severity of these messages
to debug level.

Signed-off-by: Russell King 
---
  drivers/iommu/iommu.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3ead597e1c57..304281ec623b 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -741,7 +741,7 @@ int iommu_group_add_device(struct iommu_group *group, 
struct device *dev)
  
  	trace_add_device_to_group(group->id, dev);
  
-	dev_info(dev, "Adding to iommu group %d\n", group->id);

+   dev_dbg(dev, "Adding to iommu group %d\n", group->id);


I'm not strongly against this. But to me this message seems to be a good
indicator that a device was probed successfully by the iommu subsystem.
Keeping it in the default kernel message always helps to the kernel
debugging.

Best regards,
baolu
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu: silence iommu group prints

2020-02-27 Thread Russell King - ARM Linux admin
On Thu, Feb 27, 2020 at 06:19:10PM +, Robin Murphy wrote:
> On 27/02/2020 1:48 pm, Russell King - ARM Linux admin wrote:
> > On Thu, Feb 27, 2020 at 01:44:56PM +, Robin Murphy wrote:
> > > On 27/02/2020 11:57 am, Russell King wrote:
> > > > On the LX2160A, there are lots (about 160) of IOMMU messages produced
> > > > during boot; this is excessive.  Reduce the severity of these messages
> > > > to debug level.
> > > 
> > > That's... a lot. Does the system really have that many devices, or is some
> > > driver being stupid and repeatedly populating and destroying an entire bus
> > > in a probe-deferral dance?
> > 
> > It's all the devices created by for the mc-bus for the DPAA2
> > networking support.  I don't know the technicalities, just that
> > the boot is spammed with these messages.
> 
> Well, the "technicalities" are really just whether the thing before the
> colon on each message is unique or not. If you're seeing multiple add and
> remove calls pertaining to the same device (or frankly any remove calls at
> all during boot) then it smacks of something somewhere wasting time and
> resources with unnecessary busywork, which is indicative of either poor
> design or an actual bug, either of which would deserve fixing.

It's not the same device.

-- 
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


Re: [PATCH] iommu: silence iommu group prints

2020-02-27 Thread Robin Murphy

On 27/02/2020 1:48 pm, Russell King - ARM Linux admin wrote:

On Thu, Feb 27, 2020 at 01:44:56PM +, Robin Murphy wrote:

On 27/02/2020 11:57 am, Russell King wrote:

On the LX2160A, there are lots (about 160) of IOMMU messages produced
during boot; this is excessive.  Reduce the severity of these messages
to debug level.


That's... a lot. Does the system really have that many devices, or is some
driver being stupid and repeatedly populating and destroying an entire bus
in a probe-deferral dance?


It's all the devices created by for the mc-bus for the DPAA2
networking support.  I don't know the technicalities, just that
the boot is spammed with these messages.


Well, the "technicalities" are really just whether the thing before the 
colon on each message is unique or not. If you're seeing multiple add 
and remove calls pertaining to the same device (or frankly any remove 
calls at all during boot) then it smacks of something somewhere wasting 
time and resources with unnecessary busywork, which is indicative of 
either poor design or an actual bug, either of which would deserve fixing.


Robin.
___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu


Re: [PATCH] iommu: silence iommu group prints

2020-02-27 Thread Russell King - ARM Linux admin
On Thu, Feb 27, 2020 at 01:44:56PM +, Robin Murphy wrote:
> On 27/02/2020 11:57 am, Russell King wrote:
> > On the LX2160A, there are lots (about 160) of IOMMU messages produced
> > during boot; this is excessive.  Reduce the severity of these messages
> > to debug level.
> 
> That's... a lot. Does the system really have that many devices, or is some
> driver being stupid and repeatedly populating and destroying an entire bus
> in a probe-deferral dance?

It's all the devices created by for the mc-bus for the DPAA2
networking support.  I don't know the technicalities, just that
the boot is spammed with these messages.

-- 
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


Re: [PATCH] iommu: silence iommu group prints

2020-02-27 Thread Robin Murphy

On 27/02/2020 11:57 am, Russell King wrote:

On the LX2160A, there are lots (about 160) of IOMMU messages produced
during boot; this is excessive.  Reduce the severity of these messages
to debug level.


That's... a lot. Does the system really have that many devices, or is 
some driver being stupid and repeatedly populating and destroying an 
entire bus in a probe-deferral dance?


I have no real objection to the change itself - it can be handy to see 
these when diagnosing dmesg dumps over email, but it's nothing that 
can't be gleaned from sysfs later even without dynamic debug - I'm just 
curious as to what leads to such an unusually obnoxious spamming.


Robin.


Signed-off-by: Russell King 
---
  drivers/iommu/iommu.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 3ead597e1c57..304281ec623b 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -741,7 +741,7 @@ int iommu_group_add_device(struct iommu_group *group, 
struct device *dev)
  
  	trace_add_device_to_group(group->id, dev);
  
-	dev_info(dev, "Adding to iommu group %d\n", group->id);

+   dev_dbg(dev, "Adding to iommu group %d\n", group->id);
  
  	return 0;
  
@@ -775,7 +775,7 @@ void iommu_group_remove_device(struct device *dev)

struct iommu_group *group = dev->iommu_group;
struct group_device *tmp_device, *device = NULL;
  
-	dev_info(dev, "Removing from iommu group %d\n", group->id);

+   dev_dbg(dev, "Removing from iommu group %d\n", group->id);
  
  	/* Pre-notify listeners that a device is being removed. */

blocking_notifier_call_chain(>notifier,


___
iommu mailing list
iommu@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/iommu