All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/2] Fix incorrect warning from dma-debug
@ 2016-05-08 10:59 ` Niklas Söderlund
  0 siblings, 0 replies; 26+ messages in thread
From: Niklas Söderlund @ 2016-05-08 10:59 UTC (permalink / raw)
  To: linux-arm-kernel, iommu, dmaengine
  Cc: will.deacon, robin.murphy, joro, linux-kernel, linux-renesas-soc,
	Niklas Söderlund

Hi,

While using CONFIG_DMA_API_DEBUG i came across this warning which I
think is a false positive. As shown dma_sync_single_for_device() are
called from the dma_map_single() call path. This triggers the warning
since the dma-debug code have not yet been made aware of the mapping.

I try to solve this by introducing __dma_sync_single_for_device() which
do not call into the dma-debug code. I'm no expert and this might be a
bad way of solving the problem but it allowed me to keep working.

------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at lib/dma-debug.c:1209 check_sync+0x154/0x5e4
ipmmu-vmsa e6740000.mmu: DMA-API: device driver tries to sync DMA memory it has not allocated [device address=0x000000006e89b008] [size=8 bytes]
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.6.0-rc5-00012-g52e78c1-dirty #1
Hardware name: Generic R8A7791 (Flattened Device Tree)
Backtrace:
[<c020a0a4>] (dump_backtrace) from [<c020a250>] (show_stack+0x18/0x1c)
 r7:c03ebad0 r6:00000009 r5:60000093 r4:00000000
[<c020a238>] (show_stack) from [<c03cb7e4>] (dump_stack+0x84/0xa4)
[<c03cb760>] (dump_stack) from [<c021e3ac>] (__warn+0xd0/0x100)
 r5:00000000 r4:ef05dac8
[<c021e2dc>] (__warn) from [<c021e41c>] (warn_slowpath_fmt+0x40/0x48)
 r9:00010000 r8:00000000 r7:c0c69c40 r6:c0c02754 r5:ef05db78 r4:ef222810
[<c021e3e0>] (warn_slowpath_fmt) from [<c03ebad0>] (check_sync+0x154/0x5e4)
 r3:c0945b6f r2:c0936e85
[<c03eb97c>] (check_sync) from [<c03ed594>] (debug_dma_sync_single_for_device+0x64/0x70)
 r10:00000009 r9:0000000c r8:ee89b008 r7:00000000 r6:6e89b008 r5:00000000
 r4:6e89b008
[<c03ed530>] (debug_dma_sync_single_for_device) from [<c0465298>] (__arm_lpae_set_pte.part.0+0x80/0x8c)
 r5:00000001 r4:ef222810
[<c0465218>] (__arm_lpae_set_pte.part.0) from [<c046553c>] (__arm_lpae_map+0x298/0x2f0)
 r7:00000008 r6:ef25e000 r4:ee898500
[<c04652a4>] (__arm_lpae_map) from [<c0465b18>] (arm_lpae_map+0xcc/0xe4)
 r10:c0465f40 r9:00001000 r8:40000000 r7:00000000 r6:6f25c000 r5:00000000
 r4:000008c0
[<c0465a4c>] (arm_lpae_map) from [<c0465f78>] (ipmmu_map+0x38/0x40)
 r7:40000000 r6:00000000 r5:00001000 r4:c0465a4c
[<c0465f40>] (ipmmu_map) from [<c0463d90>] (iommu_map+0xf8/0x15c)
 r4:ee895608
[<c0463c98>] (iommu_map) from [<c0213524>] (arm_coherent_iommu_map_page+0x1d4/0x2d4)
 r10:ef386940 r9:00001000 r8:00000000 r7:00000000 r6:40000000 r5:00000000
 r4:00000000
[<c0213350>] (arm_coherent_iommu_map_page) from [<c0213690>] (arm_iommu_map_page+0x6c/0x74)
 r10:c0c61f44 r9:ef19d210 r8:00000001 r7:00001000 r6:00000000 r5:ec5ddb80
 r4:00000000
[<c0213624>] (arm_iommu_map_page) from [<c04f7d64>] (sh_msiof_spi_probe+0x430/0x7b0)
 r9:00000000 r8:c0213624 r7:ef19d210 r6:ef242800 r5:ef1b4010 r4:ef242af0
[<c04f7934>] (sh_msiof_spi_probe) from [<c049fd3c>] (platform_drv_probe+0x58/0xa8)
 r10:00000000 r9:00000000 r8:c0c1f8d4 r7:c0c7e910 r6:c0c1f8d4 r5:ef1b4010
 r4:c04f7934
[<c049fce4>] (platform_drv_probe) from [<c049e788>] (driver_probe_device+0x13c/0x2a4)
 r7:c0c7e910 r6:00000000 r5:c0c7e900 r4:ef1b4010
[<c049e64c>] (driver_probe_device) from [<c049e978>] (__driver_attach+0x88/0xac)
 r9:c0c34000 r8:c0a3c010 r7:c0c1cf08 r6:c0c1f8d4 r5:ef1b4044 r4:ef1b4010
[<c049e8f0>] (__driver_attach) from [<c049ce44>] (bus_for_each_dev+0x74/0x98)
 r7:c0c1cf08 r6:c049e8f0 r5:c0c1f8d4 r4:00000000
[<c049cdd0>] (bus_for_each_dev) from [<c049ebd0>] (driver_attach+0x20/0x28)
 r6:ef1cc200 r5:00000000 r4:c0c1f8d4
[<c049ebb0>] (driver_attach) from [<c049d5d8>] (bus_add_driver+0xd4/0x1e4)
[<c049d504>] (bus_add_driver) from [<c049f2dc>] (driver_register+0xa4/0xe8)
 r7:c0a3c010 r6:00000000 r5:c0a1e400 r4:c0c1f8d4
[<c049f238>] (driver_register) from [<c04a0778>] (__platform_driver_register+0x38/0x4c)
 r5:c0a1e400 r4:ee958fc0
[<c04a0740>] (__platform_driver_register) from [<c0a1e418>] (sh_msiof_spi_drv_init+0x18/0x20)
[<c0a1e400>] (sh_msiof_spi_drv_init) from [<c0a00e1c>] (do_one_initcall+0x10c/0x1c0)
[<c0a00d10>] (do_one_initcall) from [<c0a00ff8>] (kernel_init_freeable+0x128/0x1f4)
 r9:c0c34000 r8:c0c34000 r7:c0a47e60 r6:c0a3c83c r5:000000bd r4:00000006
[<c0a00ed0>] (kernel_init_freeable) from [<c071067c>] (kernel_init+0x10/0x118)
 r9:00000000[    1.879529] ata1: link resume succeeded after 1 retries
 r8:00000000 r7:00000000 r6:00000000 r5:c071066c r4:00000000
[<c071066c>] (kernel_init) from [<c0206e08>] (ret_from_fork+0x14/0x2c)
 r5:c071066c r4:00000000
---[ end trace 6eb9a3df3009d491 ]---

Niklas Söderlund (2):
  dma-mapping: add __dma_sync_single_for_device()
  iommu/io-pgtable-arm: use __dma_sync_single_for_device()

 drivers/iommu/io-pgtable-arm.c | 2 +-
 include/linux/dma-mapping.h    | 9 ++++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

--
2.8.2

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 0/2] Fix incorrect warning from dma-debug
@ 2016-05-08 10:59 ` Niklas Söderlund
  0 siblings, 0 replies; 26+ messages in thread
From: Niklas Söderlund @ 2016-05-08 10:59 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

While using CONFIG_DMA_API_DEBUG i came across this warning which I
think is a false positive. As shown dma_sync_single_for_device() are
called from the dma_map_single() call path. This triggers the warning
since the dma-debug code have not yet been made aware of the mapping.

I try to solve this by introducing __dma_sync_single_for_device() which
do not call into the dma-debug code. I'm no expert and this might be a
bad way of solving the problem but it allowed me to keep working.

------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at lib/dma-debug.c:1209 check_sync+0x154/0x5e4
ipmmu-vmsa e6740000.mmu: DMA-API: device driver tries to sync DMA memory it has not allocated [device address=0x000000006e89b008] [size=8 bytes]
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.6.0-rc5-00012-g52e78c1-dirty #1
Hardware name: Generic R8A7791 (Flattened Device Tree)
Backtrace:
[<c020a0a4>] (dump_backtrace) from [<c020a250>] (show_stack+0x18/0x1c)
 r7:c03ebad0 r6:00000009 r5:60000093 r4:00000000
[<c020a238>] (show_stack) from [<c03cb7e4>] (dump_stack+0x84/0xa4)
[<c03cb760>] (dump_stack) from [<c021e3ac>] (__warn+0xd0/0x100)
 r5:00000000 r4:ef05dac8
[<c021e2dc>] (__warn) from [<c021e41c>] (warn_slowpath_fmt+0x40/0x48)
 r9:00010000 r8:00000000 r7:c0c69c40 r6:c0c02754 r5:ef05db78 r4:ef222810
[<c021e3e0>] (warn_slowpath_fmt) from [<c03ebad0>] (check_sync+0x154/0x5e4)
 r3:c0945b6f r2:c0936e85
[<c03eb97c>] (check_sync) from [<c03ed594>] (debug_dma_sync_single_for_device+0x64/0x70)
 r10:00000009 r9:0000000c r8:ee89b008 r7:00000000 r6:6e89b008 r5:00000000
 r4:6e89b008
[<c03ed530>] (debug_dma_sync_single_for_device) from [<c0465298>] (__arm_lpae_set_pte.part.0+0x80/0x8c)
 r5:00000001 r4:ef222810
[<c0465218>] (__arm_lpae_set_pte.part.0) from [<c046553c>] (__arm_lpae_map+0x298/0x2f0)
 r7:00000008 r6:ef25e000 r4:ee898500
[<c04652a4>] (__arm_lpae_map) from [<c0465b18>] (arm_lpae_map+0xcc/0xe4)
 r10:c0465f40 r9:00001000 r8:40000000 r7:00000000 r6:6f25c000 r5:00000000
 r4:000008c0
[<c0465a4c>] (arm_lpae_map) from [<c0465f78>] (ipmmu_map+0x38/0x40)
 r7:40000000 r6:00000000 r5:00001000 r4:c0465a4c
[<c0465f40>] (ipmmu_map) from [<c0463d90>] (iommu_map+0xf8/0x15c)
 r4:ee895608
[<c0463c98>] (iommu_map) from [<c0213524>] (arm_coherent_iommu_map_page+0x1d4/0x2d4)
 r10:ef386940 r9:00001000 r8:00000000 r7:00000000 r6:40000000 r5:00000000
 r4:00000000
[<c0213350>] (arm_coherent_iommu_map_page) from [<c0213690>] (arm_iommu_map_page+0x6c/0x74)
 r10:c0c61f44 r9:ef19d210 r8:00000001 r7:00001000 r6:00000000 r5:ec5ddb80
 r4:00000000
[<c0213624>] (arm_iommu_map_page) from [<c04f7d64>] (sh_msiof_spi_probe+0x430/0x7b0)
 r9:00000000 r8:c0213624 r7:ef19d210 r6:ef242800 r5:ef1b4010 r4:ef242af0
[<c04f7934>] (sh_msiof_spi_probe) from [<c049fd3c>] (platform_drv_probe+0x58/0xa8)
 r10:00000000 r9:00000000 r8:c0c1f8d4 r7:c0c7e910 r6:c0c1f8d4 r5:ef1b4010
 r4:c04f7934
[<c049fce4>] (platform_drv_probe) from [<c049e788>] (driver_probe_device+0x13c/0x2a4)
 r7:c0c7e910 r6:00000000 r5:c0c7e900 r4:ef1b4010
[<c049e64c>] (driver_probe_device) from [<c049e978>] (__driver_attach+0x88/0xac)
 r9:c0c34000 r8:c0a3c010 r7:c0c1cf08 r6:c0c1f8d4 r5:ef1b4044 r4:ef1b4010
[<c049e8f0>] (__driver_attach) from [<c049ce44>] (bus_for_each_dev+0x74/0x98)
 r7:c0c1cf08 r6:c049e8f0 r5:c0c1f8d4 r4:00000000
[<c049cdd0>] (bus_for_each_dev) from [<c049ebd0>] (driver_attach+0x20/0x28)
 r6:ef1cc200 r5:00000000 r4:c0c1f8d4
[<c049ebb0>] (driver_attach) from [<c049d5d8>] (bus_add_driver+0xd4/0x1e4)
[<c049d504>] (bus_add_driver) from [<c049f2dc>] (driver_register+0xa4/0xe8)
 r7:c0a3c010 r6:00000000 r5:c0a1e400 r4:c0c1f8d4
[<c049f238>] (driver_register) from [<c04a0778>] (__platform_driver_register+0x38/0x4c)
 r5:c0a1e400 r4:ee958fc0
[<c04a0740>] (__platform_driver_register) from [<c0a1e418>] (sh_msiof_spi_drv_init+0x18/0x20)
[<c0a1e400>] (sh_msiof_spi_drv_init) from [<c0a00e1c>] (do_one_initcall+0x10c/0x1c0)
[<c0a00d10>] (do_one_initcall) from [<c0a00ff8>] (kernel_init_freeable+0x128/0x1f4)
 r9:c0c34000 r8:c0c34000 r7:c0a47e60 r6:c0a3c83c r5:000000bd r4:00000006
[<c0a00ed0>] (kernel_init_freeable) from [<c071067c>] (kernel_init+0x10/0x118)
 r9:00000000[    1.879529] ata1: link resume succeeded after 1 retries
 r8:00000000 r7:00000000 r6:00000000 r5:c071066c r4:00000000
[<c071066c>] (kernel_init) from [<c0206e08>] (ret_from_fork+0x14/0x2c)
 r5:c071066c r4:00000000
---[ end trace 6eb9a3df3009d491 ]---

Niklas S?derlund (2):
  dma-mapping: add __dma_sync_single_for_device()
  iommu/io-pgtable-arm: use __dma_sync_single_for_device()

 drivers/iommu/io-pgtable-arm.c | 2 +-
 include/linux/dma-mapping.h    | 9 ++++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

--
2.8.2

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 1/2] dma-mapping: add __dma_sync_single_for_device()
  2016-05-08 10:59 ` Niklas Söderlund
@ 2016-05-08 10:59   ` Niklas Söderlund
  -1 siblings, 0 replies; 26+ messages in thread
From: Niklas Söderlund @ 2016-05-08 10:59 UTC (permalink / raw)
  To: linux-arm-kernel, iommu, dmaengine
  Cc: will.deacon, robin.murphy, joro, linux-kernel, linux-renesas-soc,
	Niklas Söderlund

Some users of the DMA mapping API calls dma_sync_single_for_device()
from the dma_map_single() call path. This will cause false warning
printouts if CONFIG_DMA_API_DEBUG are enabled.

The reason for the warning are that debug_dma_sync_single_for_device()
will be called before debug_dma_map_page() so the new mapping can't be
found and are believed to be invalid. Add __dma_sync_single_for_device()
that don't call into debug_dma_sync_single_for_device() and can be used
in these situations.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 include/linux/dma-mapping.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 9ea9aba..0e4d3a6 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -224,7 +224,7 @@ static inline void dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
 	debug_dma_sync_single_for_cpu(dev, addr, size, dir);
 }
 
-static inline void dma_sync_single_for_device(struct device *dev,
+static inline void __dma_sync_single_for_device(struct device *dev,
 					      dma_addr_t addr, size_t size,
 					      enum dma_data_direction dir)
 {
@@ -233,6 +233,13 @@ static inline void dma_sync_single_for_device(struct device *dev,
 	BUG_ON(!valid_dma_direction(dir));
 	if (ops->sync_single_for_device)
 		ops->sync_single_for_device(dev, addr, size, dir);
+}
+
+static inline void dma_sync_single_for_device(struct device *dev,
+					      dma_addr_t addr, size_t size,
+					      enum dma_data_direction dir)
+{
+	__dma_sync_single_for_device(dev, addr, size, dir);
 	debug_dma_sync_single_for_device(dev, addr, size, dir);
 }
 
-- 
2.8.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 1/2] dma-mapping: add __dma_sync_single_for_device()
@ 2016-05-08 10:59   ` Niklas Söderlund
  0 siblings, 0 replies; 26+ messages in thread
From: Niklas Söderlund @ 2016-05-08 10:59 UTC (permalink / raw)
  To: linux-arm-kernel

Some users of the DMA mapping API calls dma_sync_single_for_device()
from the dma_map_single() call path. This will cause false warning
printouts if CONFIG_DMA_API_DEBUG are enabled.

The reason for the warning are that debug_dma_sync_single_for_device()
will be called before debug_dma_map_page() so the new mapping can't be
found and are believed to be invalid. Add __dma_sync_single_for_device()
that don't call into debug_dma_sync_single_for_device() and can be used
in these situations.

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
---
 include/linux/dma-mapping.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/dma-mapping.h b/include/linux/dma-mapping.h
index 9ea9aba..0e4d3a6 100644
--- a/include/linux/dma-mapping.h
+++ b/include/linux/dma-mapping.h
@@ -224,7 +224,7 @@ static inline void dma_sync_single_for_cpu(struct device *dev, dma_addr_t addr,
 	debug_dma_sync_single_for_cpu(dev, addr, size, dir);
 }
 
-static inline void dma_sync_single_for_device(struct device *dev,
+static inline void __dma_sync_single_for_device(struct device *dev,
 					      dma_addr_t addr, size_t size,
 					      enum dma_data_direction dir)
 {
@@ -233,6 +233,13 @@ static inline void dma_sync_single_for_device(struct device *dev,
 	BUG_ON(!valid_dma_direction(dir));
 	if (ops->sync_single_for_device)
 		ops->sync_single_for_device(dev, addr, size, dir);
+}
+
+static inline void dma_sync_single_for_device(struct device *dev,
+					      dma_addr_t addr, size_t size,
+					      enum dma_data_direction dir)
+{
+	__dma_sync_single_for_device(dev, addr, size, dir);
 	debug_dma_sync_single_for_device(dev, addr, size, dir);
 }
 
-- 
2.8.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/2] iommu/io-pgtable-arm: use __dma_sync_single_for_device()
  2016-05-08 10:59 ` Niklas Söderlund
@ 2016-05-08 10:59   ` Niklas Söderlund
  -1 siblings, 0 replies; 26+ messages in thread
From: Niklas Söderlund @ 2016-05-08 10:59 UTC (permalink / raw)
  To: linux-arm-kernel, iommu, dmaengine
  Cc: will.deacon, robin.murphy, joro, linux-kernel, linux-renesas-soc,
	Niklas Söderlund

The call to dma_sync_single_for_device() can be reached from
dma_map_single(). If CONFIG_DMA_API_DEBUG is enabled this would result
in a check that the mapping being synced is valid. Since the call to
dma_map_single is not yet completed the mapping is not recorded in
dma-debug and the check fails and a warning is printed. Avoid this
warning by calling __dma_sync_single_for_device() which don't preform
this check.

Signed-off-by: Niklas Söderlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/iommu/io-pgtable-arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index f433b51..3da8102 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -255,7 +255,7 @@ static void __arm_lpae_set_pte(arm_lpae_iopte *ptep, arm_lpae_iopte pte,
 	*ptep = pte;
 
 	if (!selftest_running)
-		dma_sync_single_for_device(cfg->iommu_dev,
+		__dma_sync_single_for_device(cfg->iommu_dev,
 					   __arm_lpae_dma_addr(ptep),
 					   sizeof(pte), DMA_TO_DEVICE);
 }
-- 
2.8.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 2/2] iommu/io-pgtable-arm: use __dma_sync_single_for_device()
@ 2016-05-08 10:59   ` Niklas Söderlund
  0 siblings, 0 replies; 26+ messages in thread
From: Niklas Söderlund @ 2016-05-08 10:59 UTC (permalink / raw)
  To: linux-arm-kernel

The call to dma_sync_single_for_device() can be reached from
dma_map_single(). If CONFIG_DMA_API_DEBUG is enabled this would result
in a check that the mapping being synced is valid. Since the call to
dma_map_single is not yet completed the mapping is not recorded in
dma-debug and the check fails and a warning is printed. Avoid this
warning by calling __dma_sync_single_for_device() which don't preform
this check.

Signed-off-by: Niklas S?derlund <niklas.soderlund+renesas@ragnatech.se>
---
 drivers/iommu/io-pgtable-arm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iommu/io-pgtable-arm.c b/drivers/iommu/io-pgtable-arm.c
index f433b51..3da8102 100644
--- a/drivers/iommu/io-pgtable-arm.c
+++ b/drivers/iommu/io-pgtable-arm.c
@@ -255,7 +255,7 @@ static void __arm_lpae_set_pte(arm_lpae_iopte *ptep, arm_lpae_iopte pte,
 	*ptep = pte;
 
 	if (!selftest_running)
-		dma_sync_single_for_device(cfg->iommu_dev,
+		__dma_sync_single_for_device(cfg->iommu_dev,
 					   __arm_lpae_dma_addr(ptep),
 					   sizeof(pte), DMA_TO_DEVICE);
 }
-- 
2.8.2

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] iommu/io-pgtable-arm: use __dma_sync_single_for_device()
  2016-05-08 10:59   ` Niklas Söderlund
  (?)
@ 2016-05-09  9:35     ` Will Deacon
  -1 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2016-05-09  9:35 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: linux-arm-kernel, iommu, dmaengine, joro, linux-kernel,
	linux-renesas-soc, robin.murphy

On Sun, May 08, 2016 at 12:59:56PM +0200, Niklas Söderlund wrote:
> The call to dma_sync_single_for_device() can be reached from
> dma_map_single(). If CONFIG_DMA_API_DEBUG is enabled this would result
> in a check that the mapping being synced is valid. Since the call to
> dma_map_single is not yet completed the mapping is not recorded in
> dma-debug and the check fails and a warning is printed. Avoid this
> warning by calling __dma_sync_single_for_device() which don't preform
> this check.

Hmm, I don't understand why this would trigger that warning. The memory
being sync'd here is the page table memory, not the buffer being mapped.
The page table memory is "mapped" using dma_map_single in
__arm_lpae_alloc_pages, so it sounds like the issue something else.

Will

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 2/2] iommu/io-pgtable-arm: use __dma_sync_single_for_device()
@ 2016-05-09  9:35     ` Will Deacon
  0 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2016-05-09  9:35 UTC (permalink / raw)
  To: Niklas Söderlund
  Cc: linux-arm-kernel, iommu, dmaengine, joro, linux-kernel,
	linux-renesas-soc, robin.murphy

On Sun, May 08, 2016 at 12:59:56PM +0200, Niklas S�derlund wrote:
> The call to dma_sync_single_for_device() can be reached from
> dma_map_single(). If CONFIG_DMA_API_DEBUG is enabled this would result
> in a check that the mapping being synced is valid. Since the call to
> dma_map_single is not yet completed the mapping is not recorded in
> dma-debug and the check fails and a warning is printed. Avoid this
> warning by calling __dma_sync_single_for_device() which don't preform
> this check.

Hmm, I don't understand why this would trigger that warning. The memory
being sync'd here is the page table memory, not the buffer being mapped.
The page table memory is "mapped" using dma_map_single in
__arm_lpae_alloc_pages, so it sounds like the issue something else.

Will

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 2/2] iommu/io-pgtable-arm: use __dma_sync_single_for_device()
@ 2016-05-09  9:35     ` Will Deacon
  0 siblings, 0 replies; 26+ messages in thread
From: Will Deacon @ 2016-05-09  9:35 UTC (permalink / raw)
  To: linux-arm-kernel

On Sun, May 08, 2016 at 12:59:56PM +0200, Niklas S?derlund wrote:
> The call to dma_sync_single_for_device() can be reached from
> dma_map_single(). If CONFIG_DMA_API_DEBUG is enabled this would result
> in a check that the mapping being synced is valid. Since the call to
> dma_map_single is not yet completed the mapping is not recorded in
> dma-debug and the check fails and a warning is printed. Avoid this
> warning by calling __dma_sync_single_for_device() which don't preform
> this check.

Hmm, I don't understand why this would trigger that warning. The memory
being sync'd here is the page table memory, not the buffer being mapped.
The page table memory is "mapped" using dma_map_single in
__arm_lpae_alloc_pages, so it sounds like the issue something else.

Will

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/2] Fix incorrect warning from dma-debug
  2016-05-08 10:59 ` Niklas Söderlund
@ 2016-05-09  9:37   ` Robin Murphy
  -1 siblings, 0 replies; 26+ messages in thread
From: Robin Murphy @ 2016-05-09  9:37 UTC (permalink / raw)
  To: Niklas Söderlund, linux-arm-kernel, iommu, dmaengine
  Cc: joro, will.deacon, linux-kernel, linux-renesas-soc

Hi Niklas,

On 08/05/16 11:59, Niklas Söderlund wrote:
> Hi,
>
> While using CONFIG_DMA_API_DEBUG i came across this warning which I
> think is a false positive. As shown dma_sync_single_for_device() are
> called from the dma_map_single() call path. This triggers the warning
> since the dma-debug code have not yet been made aware of the mapping.

Almost right ;) The thing being mapped (the SPI device's buffer) and the 
thing being synced (the IOMMU's PTE) are entirely unrelated. Due to the 
current of_iommu_init() setup, the IOMMU is probed long before 
dma_debug_init() gets called, therefore DMA debug is missing entries for 
some of the initial page table mappings and gets confused when we update 
them later.

> I try to solve this by introducing __dma_sync_single_for_device() which
> do not call into the dma-debug code. I'm no expert and this might be a
> bad way of solving the problem but it allowed me to keep working.

The simple fix should be to just call dma_debug_init() from a 
sufficiently earlier initcall level. The best would be to sort out a 
proper device dependency order to avoid the whole early-IOMMU-creation 
thing entirely.

Robin.

> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at lib/dma-debug.c:1209 check_sync+0x154/0x5e4
> ipmmu-vmsa e6740000.mmu: DMA-API: device driver tries to sync DMA memory it has not allocated [device address=0x000000006e89b008] [size=8 bytes]
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.6.0-rc5-00012-g52e78c1-dirty #1
> Hardware name: Generic R8A7791 (Flattened Device Tree)
> Backtrace:
> [<c020a0a4>] (dump_backtrace) from [<c020a250>] (show_stack+0x18/0x1c)
>   r7:c03ebad0 r6:00000009 r5:60000093 r4:00000000
> [<c020a238>] (show_stack) from [<c03cb7e4>] (dump_stack+0x84/0xa4)
> [<c03cb760>] (dump_stack) from [<c021e3ac>] (__warn+0xd0/0x100)
>   r5:00000000 r4:ef05dac8
> [<c021e2dc>] (__warn) from [<c021e41c>] (warn_slowpath_fmt+0x40/0x48)
>   r9:00010000 r8:00000000 r7:c0c69c40 r6:c0c02754 r5:ef05db78 r4:ef222810
> [<c021e3e0>] (warn_slowpath_fmt) from [<c03ebad0>] (check_sync+0x154/0x5e4)
>   r3:c0945b6f r2:c0936e85
> [<c03eb97c>] (check_sync) from [<c03ed594>] (debug_dma_sync_single_for_device+0x64/0x70)
>   r10:00000009 r9:0000000c r8:ee89b008 r7:00000000 r6:6e89b008 r5:00000000
>   r4:6e89b008
> [<c03ed530>] (debug_dma_sync_single_for_device) from [<c0465298>] (__arm_lpae_set_pte.part.0+0x80/0x8c)
>   r5:00000001 r4:ef222810
> [<c0465218>] (__arm_lpae_set_pte.part.0) from [<c046553c>] (__arm_lpae_map+0x298/0x2f0)
>   r7:00000008 r6:ef25e000 r4:ee898500
> [<c04652a4>] (__arm_lpae_map) from [<c0465b18>] (arm_lpae_map+0xcc/0xe4)
>   r10:c0465f40 r9:00001000 r8:40000000 r7:00000000 r6:6f25c000 r5:00000000
>   r4:000008c0
> [<c0465a4c>] (arm_lpae_map) from [<c0465f78>] (ipmmu_map+0x38/0x40)
>   r7:40000000 r6:00000000 r5:00001000 r4:c0465a4c
> [<c0465f40>] (ipmmu_map) from [<c0463d90>] (iommu_map+0xf8/0x15c)
>   r4:ee895608
> [<c0463c98>] (iommu_map) from [<c0213524>] (arm_coherent_iommu_map_page+0x1d4/0x2d4)
>   r10:ef386940 r9:00001000 r8:00000000 r7:00000000 r6:40000000 r5:00000000
>   r4:00000000
> [<c0213350>] (arm_coherent_iommu_map_page) from [<c0213690>] (arm_iommu_map_page+0x6c/0x74)
>   r10:c0c61f44 r9:ef19d210 r8:00000001 r7:00001000 r6:00000000 r5:ec5ddb80
>   r4:00000000
> [<c0213624>] (arm_iommu_map_page) from [<c04f7d64>] (sh_msiof_spi_probe+0x430/0x7b0)
>   r9:00000000 r8:c0213624 r7:ef19d210 r6:ef242800 r5:ef1b4010 r4:ef242af0
> [<c04f7934>] (sh_msiof_spi_probe) from [<c049fd3c>] (platform_drv_probe+0x58/0xa8)
>   r10:00000000 r9:00000000 r8:c0c1f8d4 r7:c0c7e910 r6:c0c1f8d4 r5:ef1b4010
>   r4:c04f7934
> [<c049fce4>] (platform_drv_probe) from [<c049e788>] (driver_probe_device+0x13c/0x2a4)
>   r7:c0c7e910 r6:00000000 r5:c0c7e900 r4:ef1b4010
> [<c049e64c>] (driver_probe_device) from [<c049e978>] (__driver_attach+0x88/0xac)
>   r9:c0c34000 r8:c0a3c010 r7:c0c1cf08 r6:c0c1f8d4 r5:ef1b4044 r4:ef1b4010
> [<c049e8f0>] (__driver_attach) from [<c049ce44>] (bus_for_each_dev+0x74/0x98)
>   r7:c0c1cf08 r6:c049e8f0 r5:c0c1f8d4 r4:00000000
> [<c049cdd0>] (bus_for_each_dev) from [<c049ebd0>] (driver_attach+0x20/0x28)
>   r6:ef1cc200 r5:00000000 r4:c0c1f8d4
> [<c049ebb0>] (driver_attach) from [<c049d5d8>] (bus_add_driver+0xd4/0x1e4)
> [<c049d504>] (bus_add_driver) from [<c049f2dc>] (driver_register+0xa4/0xe8)
>   r7:c0a3c010 r6:00000000 r5:c0a1e400 r4:c0c1f8d4
> [<c049f238>] (driver_register) from [<c04a0778>] (__platform_driver_register+0x38/0x4c)
>   r5:c0a1e400 r4:ee958fc0
> [<c04a0740>] (__platform_driver_register) from [<c0a1e418>] (sh_msiof_spi_drv_init+0x18/0x20)
> [<c0a1e400>] (sh_msiof_spi_drv_init) from [<c0a00e1c>] (do_one_initcall+0x10c/0x1c0)
> [<c0a00d10>] (do_one_initcall) from [<c0a00ff8>] (kernel_init_freeable+0x128/0x1f4)
>   r9:c0c34000 r8:c0c34000 r7:c0a47e60 r6:c0a3c83c r5:000000bd r4:00000006
> [<c0a00ed0>] (kernel_init_freeable) from [<c071067c>] (kernel_init+0x10/0x118)
>   r9:00000000[    1.879529] ata1: link resume succeeded after 1 retries
>   r8:00000000 r7:00000000 r6:00000000 r5:c071066c r4:00000000
> [<c071066c>] (kernel_init) from [<c0206e08>] (ret_from_fork+0x14/0x2c)
>   r5:c071066c r4:00000000
> ---[ end trace 6eb9a3df3009d491 ]---
>
> Niklas Söderlund (2):
>    dma-mapping: add __dma_sync_single_for_device()
>    iommu/io-pgtable-arm: use __dma_sync_single_for_device()
>
>   drivers/iommu/io-pgtable-arm.c | 2 +-
>   include/linux/dma-mapping.h    | 9 ++++++++-
>   2 files changed, 9 insertions(+), 2 deletions(-)
>
> --
> 2.8.2
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 0/2] Fix incorrect warning from dma-debug
@ 2016-05-09  9:37   ` Robin Murphy
  0 siblings, 0 replies; 26+ messages in thread
From: Robin Murphy @ 2016-05-09  9:37 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Niklas,

On 08/05/16 11:59, Niklas S?derlund wrote:
> Hi,
>
> While using CONFIG_DMA_API_DEBUG i came across this warning which I
> think is a false positive. As shown dma_sync_single_for_device() are
> called from the dma_map_single() call path. This triggers the warning
> since the dma-debug code have not yet been made aware of the mapping.

Almost right ;) The thing being mapped (the SPI device's buffer) and the 
thing being synced (the IOMMU's PTE) are entirely unrelated. Due to the 
current of_iommu_init() setup, the IOMMU is probed long before 
dma_debug_init() gets called, therefore DMA debug is missing entries for 
some of the initial page table mappings and gets confused when we update 
them later.

> I try to solve this by introducing __dma_sync_single_for_device() which
> do not call into the dma-debug code. I'm no expert and this might be a
> bad way of solving the problem but it allowed me to keep working.

The simple fix should be to just call dma_debug_init() from a 
sufficiently earlier initcall level. The best would be to sort out a 
proper device dependency order to avoid the whole early-IOMMU-creation 
thing entirely.

Robin.

> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at lib/dma-debug.c:1209 check_sync+0x154/0x5e4
> ipmmu-vmsa e6740000.mmu: DMA-API: device driver tries to sync DMA memory it has not allocated [device address=0x000000006e89b008] [size=8 bytes]
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.6.0-rc5-00012-g52e78c1-dirty #1
> Hardware name: Generic R8A7791 (Flattened Device Tree)
> Backtrace:
> [<c020a0a4>] (dump_backtrace) from [<c020a250>] (show_stack+0x18/0x1c)
>   r7:c03ebad0 r6:00000009 r5:60000093 r4:00000000
> [<c020a238>] (show_stack) from [<c03cb7e4>] (dump_stack+0x84/0xa4)
> [<c03cb760>] (dump_stack) from [<c021e3ac>] (__warn+0xd0/0x100)
>   r5:00000000 r4:ef05dac8
> [<c021e2dc>] (__warn) from [<c021e41c>] (warn_slowpath_fmt+0x40/0x48)
>   r9:00010000 r8:00000000 r7:c0c69c40 r6:c0c02754 r5:ef05db78 r4:ef222810
> [<c021e3e0>] (warn_slowpath_fmt) from [<c03ebad0>] (check_sync+0x154/0x5e4)
>   r3:c0945b6f r2:c0936e85
> [<c03eb97c>] (check_sync) from [<c03ed594>] (debug_dma_sync_single_for_device+0x64/0x70)
>   r10:00000009 r9:0000000c r8:ee89b008 r7:00000000 r6:6e89b008 r5:00000000
>   r4:6e89b008
> [<c03ed530>] (debug_dma_sync_single_for_device) from [<c0465298>] (__arm_lpae_set_pte.part.0+0x80/0x8c)
>   r5:00000001 r4:ef222810
> [<c0465218>] (__arm_lpae_set_pte.part.0) from [<c046553c>] (__arm_lpae_map+0x298/0x2f0)
>   r7:00000008 r6:ef25e000 r4:ee898500
> [<c04652a4>] (__arm_lpae_map) from [<c0465b18>] (arm_lpae_map+0xcc/0xe4)
>   r10:c0465f40 r9:00001000 r8:40000000 r7:00000000 r6:6f25c000 r5:00000000
>   r4:000008c0
> [<c0465a4c>] (arm_lpae_map) from [<c0465f78>] (ipmmu_map+0x38/0x40)
>   r7:40000000 r6:00000000 r5:00001000 r4:c0465a4c
> [<c0465f40>] (ipmmu_map) from [<c0463d90>] (iommu_map+0xf8/0x15c)
>   r4:ee895608
> [<c0463c98>] (iommu_map) from [<c0213524>] (arm_coherent_iommu_map_page+0x1d4/0x2d4)
>   r10:ef386940 r9:00001000 r8:00000000 r7:00000000 r6:40000000 r5:00000000
>   r4:00000000
> [<c0213350>] (arm_coherent_iommu_map_page) from [<c0213690>] (arm_iommu_map_page+0x6c/0x74)
>   r10:c0c61f44 r9:ef19d210 r8:00000001 r7:00001000 r6:00000000 r5:ec5ddb80
>   r4:00000000
> [<c0213624>] (arm_iommu_map_page) from [<c04f7d64>] (sh_msiof_spi_probe+0x430/0x7b0)
>   r9:00000000 r8:c0213624 r7:ef19d210 r6:ef242800 r5:ef1b4010 r4:ef242af0
> [<c04f7934>] (sh_msiof_spi_probe) from [<c049fd3c>] (platform_drv_probe+0x58/0xa8)
>   r10:00000000 r9:00000000 r8:c0c1f8d4 r7:c0c7e910 r6:c0c1f8d4 r5:ef1b4010
>   r4:c04f7934
> [<c049fce4>] (platform_drv_probe) from [<c049e788>] (driver_probe_device+0x13c/0x2a4)
>   r7:c0c7e910 r6:00000000 r5:c0c7e900 r4:ef1b4010
> [<c049e64c>] (driver_probe_device) from [<c049e978>] (__driver_attach+0x88/0xac)
>   r9:c0c34000 r8:c0a3c010 r7:c0c1cf08 r6:c0c1f8d4 r5:ef1b4044 r4:ef1b4010
> [<c049e8f0>] (__driver_attach) from [<c049ce44>] (bus_for_each_dev+0x74/0x98)
>   r7:c0c1cf08 r6:c049e8f0 r5:c0c1f8d4 r4:00000000
> [<c049cdd0>] (bus_for_each_dev) from [<c049ebd0>] (driver_attach+0x20/0x28)
>   r6:ef1cc200 r5:00000000 r4:c0c1f8d4
> [<c049ebb0>] (driver_attach) from [<c049d5d8>] (bus_add_driver+0xd4/0x1e4)
> [<c049d504>] (bus_add_driver) from [<c049f2dc>] (driver_register+0xa4/0xe8)
>   r7:c0a3c010 r6:00000000 r5:c0a1e400 r4:c0c1f8d4
> [<c049f238>] (driver_register) from [<c04a0778>] (__platform_driver_register+0x38/0x4c)
>   r5:c0a1e400 r4:ee958fc0
> [<c04a0740>] (__platform_driver_register) from [<c0a1e418>] (sh_msiof_spi_drv_init+0x18/0x20)
> [<c0a1e400>] (sh_msiof_spi_drv_init) from [<c0a00e1c>] (do_one_initcall+0x10c/0x1c0)
> [<c0a00d10>] (do_one_initcall) from [<c0a00ff8>] (kernel_init_freeable+0x128/0x1f4)
>   r9:c0c34000 r8:c0c34000 r7:c0a47e60 r6:c0a3c83c r5:000000bd r4:00000006
> [<c0a00ed0>] (kernel_init_freeable) from [<c071067c>] (kernel_init+0x10/0x118)
>   r9:00000000[    1.879529] ata1: link resume succeeded after 1 retries
>   r8:00000000 r7:00000000 r6:00000000 r5:c071066c r4:00000000
> [<c071066c>] (kernel_init) from [<c0206e08>] (ret_from_fork+0x14/0x2c)
>   r5:c071066c r4:00000000
> ---[ end trace 6eb9a3df3009d491 ]---
>
> Niklas S?derlund (2):
>    dma-mapping: add __dma_sync_single_for_device()
>    iommu/io-pgtable-arm: use __dma_sync_single_for_device()
>
>   drivers/iommu/io-pgtable-arm.c | 2 +-
>   include/linux/dma-mapping.h    | 9 ++++++++-
>   2 files changed, 9 insertions(+), 2 deletions(-)
>
> --
> 2.8.2
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/2] Fix incorrect warning from dma-debug
  2016-05-09  9:37   ` Robin Murphy
@ 2016-05-09 10:00     ` Robin Murphy
  -1 siblings, 0 replies; 26+ messages in thread
From: Robin Murphy @ 2016-05-09 10:00 UTC (permalink / raw)
  To: Niklas Söderlund, linux-arm-kernel, iommu, dmaengine
  Cc: linux-renesas-soc, will.deacon, linux-kernel

On 09/05/16 10:37, Robin Murphy wrote:
> Hi Niklas,
>
> On 08/05/16 11:59, Niklas Söderlund wrote:
>> Hi,
>>
>> While using CONFIG_DMA_API_DEBUG i came across this warning which I
>> think is a false positive. As shown dma_sync_single_for_device() are
>> called from the dma_map_single() call path. This triggers the warning
>> since the dma-debug code have not yet been made aware of the mapping.
>
> Almost right ;) The thing being mapped (the SPI device's buffer) and the
> thing being synced (the IOMMU's PTE) are entirely unrelated. Due to the
> current of_iommu_init() setup, the IOMMU is probed long before
> dma_debug_init() gets called, therefore DMA debug is missing entries for
> some of the initial page table mappings and gets confused when we update
> them later.
>
>> I try to solve this by introducing __dma_sync_single_for_device() which
>> do not call into the dma-debug code. I'm no expert and this might be a
>> bad way of solving the problem but it allowed me to keep working.
>
> The simple fix should be to just call dma_debug_init() from a
> sufficiently earlier initcall level. The best would be to sort out a
> proper device dependency order to avoid the whole early-IOMMU-creation
> thing entirely.

A tangential idea, as Will just suggested to me, would be to make this 
false-positive situation more explicit, something like (untested):
---8<---
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 4a1515f4b452..e16684a4ce86 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -107,7 +107,8 @@ static bool dma_debug_initialized __read_mostly;

  static inline bool dma_debug_disabled(void)
  {
-	return global_disable || !dma_debug_initialized;
+	return global_disable || WARN_ONCE(!dma_debug_initialized,
+					   "early DMA-API call not tracked");
  }

  /* Global error count */
--->8---

but it seems like this a sufficiently rare case that it's probably not 
worth cluttering up the code for.

Robin.

>> ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 1 at lib/dma-debug.c:1209 check_sync+0x154/0x5e4
>> ipmmu-vmsa e6740000.mmu: DMA-API: device driver tries to sync DMA
>> memory it has not allocated [device address=0x000000006e89b008]
>> [size=8 bytes]
>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted
>> 4.6.0-rc5-00012-g52e78c1-dirty #1
>> Hardware name: Generic R8A7791 (Flattened Device Tree)
>> Backtrace:
>> [<c020a0a4>] (dump_backtrace) from [<c020a250>] (show_stack+0x18/0x1c)
>>   r7:c03ebad0 r6:00000009 r5:60000093 r4:00000000
>> [<c020a238>] (show_stack) from [<c03cb7e4>] (dump_stack+0x84/0xa4)
>> [<c03cb760>] (dump_stack) from [<c021e3ac>] (__warn+0xd0/0x100)
>>   r5:00000000 r4:ef05dac8
>> [<c021e2dc>] (__warn) from [<c021e41c>] (warn_slowpath_fmt+0x40/0x48)
>>   r9:00010000 r8:00000000 r7:c0c69c40 r6:c0c02754 r5:ef05db78 r4:ef222810
>> [<c021e3e0>] (warn_slowpath_fmt) from [<c03ebad0>]
>> (check_sync+0x154/0x5e4)
>>   r3:c0945b6f r2:c0936e85
>> [<c03eb97c>] (check_sync) from [<c03ed594>]
>> (debug_dma_sync_single_for_device+0x64/0x70)
>>   r10:00000009 r9:0000000c r8:ee89b008 r7:00000000 r6:6e89b008
>> r5:00000000
>>   r4:6e89b008
>> [<c03ed530>] (debug_dma_sync_single_for_device) from [<c0465298>]
>> (__arm_lpae_set_pte.part.0+0x80/0x8c)
>>   r5:00000001 r4:ef222810
>> [<c0465218>] (__arm_lpae_set_pte.part.0) from [<c046553c>]
>> (__arm_lpae_map+0x298/0x2f0)
>>   r7:00000008 r6:ef25e000 r4:ee898500
>> [<c04652a4>] (__arm_lpae_map) from [<c0465b18>] (arm_lpae_map+0xcc/0xe4)
>>   r10:c0465f40 r9:00001000 r8:40000000 r7:00000000 r6:6f25c000
>> r5:00000000
>>   r4:000008c0
>> [<c0465a4c>] (arm_lpae_map) from [<c0465f78>] (ipmmu_map+0x38/0x40)
>>   r7:40000000 r6:00000000 r5:00001000 r4:c0465a4c
>> [<c0465f40>] (ipmmu_map) from [<c0463d90>] (iommu_map+0xf8/0x15c)
>>   r4:ee895608
>> [<c0463c98>] (iommu_map) from [<c0213524>]
>> (arm_coherent_iommu_map_page+0x1d4/0x2d4)
>>   r10:ef386940 r9:00001000 r8:00000000 r7:00000000 r6:40000000
>> r5:00000000
>>   r4:00000000
>> [<c0213350>] (arm_coherent_iommu_map_page) from [<c0213690>]
>> (arm_iommu_map_page+0x6c/0x74)
>>   r10:c0c61f44 r9:ef19d210 r8:00000001 r7:00001000 r6:00000000
>> r5:ec5ddb80
>>   r4:00000000
>> [<c0213624>] (arm_iommu_map_page) from [<c04f7d64>]
>> (sh_msiof_spi_probe+0x430/0x7b0)
>>   r9:00000000 r8:c0213624 r7:ef19d210 r6:ef242800 r5:ef1b4010 r4:ef242af0
>> [<c04f7934>] (sh_msiof_spi_probe) from [<c049fd3c>]
>> (platform_drv_probe+0x58/0xa8)
>>   r10:00000000 r9:00000000 r8:c0c1f8d4 r7:c0c7e910 r6:c0c1f8d4
>> r5:ef1b4010
>>   r4:c04f7934
>> [<c049fce4>] (platform_drv_probe) from [<c049e788>]
>> (driver_probe_device+0x13c/0x2a4)
>>   r7:c0c7e910 r6:00000000 r5:c0c7e900 r4:ef1b4010
>> [<c049e64c>] (driver_probe_device) from [<c049e978>]
>> (__driver_attach+0x88/0xac)
>>   r9:c0c34000 r8:c0a3c010 r7:c0c1cf08 r6:c0c1f8d4 r5:ef1b4044 r4:ef1b4010
>> [<c049e8f0>] (__driver_attach) from [<c049ce44>]
>> (bus_for_each_dev+0x74/0x98)
>>   r7:c0c1cf08 r6:c049e8f0 r5:c0c1f8d4 r4:00000000
>> [<c049cdd0>] (bus_for_each_dev) from [<c049ebd0>]
>> (driver_attach+0x20/0x28)
>>   r6:ef1cc200 r5:00000000 r4:c0c1f8d4
>> [<c049ebb0>] (driver_attach) from [<c049d5d8>]
>> (bus_add_driver+0xd4/0x1e4)
>> [<c049d504>] (bus_add_driver) from [<c049f2dc>]
>> (driver_register+0xa4/0xe8)
>>   r7:c0a3c010 r6:00000000 r5:c0a1e400 r4:c0c1f8d4
>> [<c049f238>] (driver_register) from [<c04a0778>]
>> (__platform_driver_register+0x38/0x4c)
>>   r5:c0a1e400 r4:ee958fc0
>> [<c04a0740>] (__platform_driver_register) from [<c0a1e418>]
>> (sh_msiof_spi_drv_init+0x18/0x20)
>> [<c0a1e400>] (sh_msiof_spi_drv_init) from [<c0a00e1c>]
>> (do_one_initcall+0x10c/0x1c0)
>> [<c0a00d10>] (do_one_initcall) from [<c0a00ff8>]
>> (kernel_init_freeable+0x128/0x1f4)
>>   r9:c0c34000 r8:c0c34000 r7:c0a47e60 r6:c0a3c83c r5:000000bd r4:00000006
>> [<c0a00ed0>] (kernel_init_freeable) from [<c071067c>]
>> (kernel_init+0x10/0x118)
>>   r9:00000000[    1.879529] ata1: link resume succeeded after 1 retries
>>   r8:00000000 r7:00000000 r6:00000000 r5:c071066c r4:00000000
>> [<c071066c>] (kernel_init) from [<c0206e08>] (ret_from_fork+0x14/0x2c)
>>   r5:c071066c r4:00000000
>> ---[ end trace 6eb9a3df3009d491 ]---
>>
>> Niklas Söderlund (2):
>>    dma-mapping: add __dma_sync_single_for_device()
>>    iommu/io-pgtable-arm: use __dma_sync_single_for_device()
>>
>>   drivers/iommu/io-pgtable-arm.c | 2 +-
>>   include/linux/dma-mapping.h    | 9 ++++++++-
>>   2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> --
>> 2.8.2
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>
> _______________________________________________
> iommu mailing list
> iommu@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* [PATCH 0/2] Fix incorrect warning from dma-debug
@ 2016-05-09 10:00     ` Robin Murphy
  0 siblings, 0 replies; 26+ messages in thread
From: Robin Murphy @ 2016-05-09 10:00 UTC (permalink / raw)
  To: linux-arm-kernel

On 09/05/16 10:37, Robin Murphy wrote:
> Hi Niklas,
>
> On 08/05/16 11:59, Niklas S?derlund wrote:
>> Hi,
>>
>> While using CONFIG_DMA_API_DEBUG i came across this warning which I
>> think is a false positive. As shown dma_sync_single_for_device() are
>> called from the dma_map_single() call path. This triggers the warning
>> since the dma-debug code have not yet been made aware of the mapping.
>
> Almost right ;) The thing being mapped (the SPI device's buffer) and the
> thing being synced (the IOMMU's PTE) are entirely unrelated. Due to the
> current of_iommu_init() setup, the IOMMU is probed long before
> dma_debug_init() gets called, therefore DMA debug is missing entries for
> some of the initial page table mappings and gets confused when we update
> them later.
>
>> I try to solve this by introducing __dma_sync_single_for_device() which
>> do not call into the dma-debug code. I'm no expert and this might be a
>> bad way of solving the problem but it allowed me to keep working.
>
> The simple fix should be to just call dma_debug_init() from a
> sufficiently earlier initcall level. The best would be to sort out a
> proper device dependency order to avoid the whole early-IOMMU-creation
> thing entirely.

A tangential idea, as Will just suggested to me, would be to make this 
false-positive situation more explicit, something like (untested):
---8<---
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 4a1515f4b452..e16684a4ce86 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -107,7 +107,8 @@ static bool dma_debug_initialized __read_mostly;

  static inline bool dma_debug_disabled(void)
  {
-	return global_disable || !dma_debug_initialized;
+	return global_disable || WARN_ONCE(!dma_debug_initialized,
+					   "early DMA-API call not tracked");
  }

  /* Global error count */
--->8---

but it seems like this a sufficiently rare case that it's probably not 
worth cluttering up the code for.

Robin.

>> ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 1 at lib/dma-debug.c:1209 check_sync+0x154/0x5e4
>> ipmmu-vmsa e6740000.mmu: DMA-API: device driver tries to sync DMA
>> memory it has not allocated [device address=0x000000006e89b008]
>> [size=8 bytes]
>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted
>> 4.6.0-rc5-00012-g52e78c1-dirty #1
>> Hardware name: Generic R8A7791 (Flattened Device Tree)
>> Backtrace:
>> [<c020a0a4>] (dump_backtrace) from [<c020a250>] (show_stack+0x18/0x1c)
>>   r7:c03ebad0 r6:00000009 r5:60000093 r4:00000000
>> [<c020a238>] (show_stack) from [<c03cb7e4>] (dump_stack+0x84/0xa4)
>> [<c03cb760>] (dump_stack) from [<c021e3ac>] (__warn+0xd0/0x100)
>>   r5:00000000 r4:ef05dac8
>> [<c021e2dc>] (__warn) from [<c021e41c>] (warn_slowpath_fmt+0x40/0x48)
>>   r9:00010000 r8:00000000 r7:c0c69c40 r6:c0c02754 r5:ef05db78 r4:ef222810
>> [<c021e3e0>] (warn_slowpath_fmt) from [<c03ebad0>]
>> (check_sync+0x154/0x5e4)
>>   r3:c0945b6f r2:c0936e85
>> [<c03eb97c>] (check_sync) from [<c03ed594>]
>> (debug_dma_sync_single_for_device+0x64/0x70)
>>   r10:00000009 r9:0000000c r8:ee89b008 r7:00000000 r6:6e89b008
>> r5:00000000
>>   r4:6e89b008
>> [<c03ed530>] (debug_dma_sync_single_for_device) from [<c0465298>]
>> (__arm_lpae_set_pte.part.0+0x80/0x8c)
>>   r5:00000001 r4:ef222810
>> [<c0465218>] (__arm_lpae_set_pte.part.0) from [<c046553c>]
>> (__arm_lpae_map+0x298/0x2f0)
>>   r7:00000008 r6:ef25e000 r4:ee898500
>> [<c04652a4>] (__arm_lpae_map) from [<c0465b18>] (arm_lpae_map+0xcc/0xe4)
>>   r10:c0465f40 r9:00001000 r8:40000000 r7:00000000 r6:6f25c000
>> r5:00000000
>>   r4:000008c0
>> [<c0465a4c>] (arm_lpae_map) from [<c0465f78>] (ipmmu_map+0x38/0x40)
>>   r7:40000000 r6:00000000 r5:00001000 r4:c0465a4c
>> [<c0465f40>] (ipmmu_map) from [<c0463d90>] (iommu_map+0xf8/0x15c)
>>   r4:ee895608
>> [<c0463c98>] (iommu_map) from [<c0213524>]
>> (arm_coherent_iommu_map_page+0x1d4/0x2d4)
>>   r10:ef386940 r9:00001000 r8:00000000 r7:00000000 r6:40000000
>> r5:00000000
>>   r4:00000000
>> [<c0213350>] (arm_coherent_iommu_map_page) from [<c0213690>]
>> (arm_iommu_map_page+0x6c/0x74)
>>   r10:c0c61f44 r9:ef19d210 r8:00000001 r7:00001000 r6:00000000
>> r5:ec5ddb80
>>   r4:00000000
>> [<c0213624>] (arm_iommu_map_page) from [<c04f7d64>]
>> (sh_msiof_spi_probe+0x430/0x7b0)
>>   r9:00000000 r8:c0213624 r7:ef19d210 r6:ef242800 r5:ef1b4010 r4:ef242af0
>> [<c04f7934>] (sh_msiof_spi_probe) from [<c049fd3c>]
>> (platform_drv_probe+0x58/0xa8)
>>   r10:00000000 r9:00000000 r8:c0c1f8d4 r7:c0c7e910 r6:c0c1f8d4
>> r5:ef1b4010
>>   r4:c04f7934
>> [<c049fce4>] (platform_drv_probe) from [<c049e788>]
>> (driver_probe_device+0x13c/0x2a4)
>>   r7:c0c7e910 r6:00000000 r5:c0c7e900 r4:ef1b4010
>> [<c049e64c>] (driver_probe_device) from [<c049e978>]
>> (__driver_attach+0x88/0xac)
>>   r9:c0c34000 r8:c0a3c010 r7:c0c1cf08 r6:c0c1f8d4 r5:ef1b4044 r4:ef1b4010
>> [<c049e8f0>] (__driver_attach) from [<c049ce44>]
>> (bus_for_each_dev+0x74/0x98)
>>   r7:c0c1cf08 r6:c049e8f0 r5:c0c1f8d4 r4:00000000
>> [<c049cdd0>] (bus_for_each_dev) from [<c049ebd0>]
>> (driver_attach+0x20/0x28)
>>   r6:ef1cc200 r5:00000000 r4:c0c1f8d4
>> [<c049ebb0>] (driver_attach) from [<c049d5d8>]
>> (bus_add_driver+0xd4/0x1e4)
>> [<c049d504>] (bus_add_driver) from [<c049f2dc>]
>> (driver_register+0xa4/0xe8)
>>   r7:c0a3c010 r6:00000000 r5:c0a1e400 r4:c0c1f8d4
>> [<c049f238>] (driver_register) from [<c04a0778>]
>> (__platform_driver_register+0x38/0x4c)
>>   r5:c0a1e400 r4:ee958fc0
>> [<c04a0740>] (__platform_driver_register) from [<c0a1e418>]
>> (sh_msiof_spi_drv_init+0x18/0x20)
>> [<c0a1e400>] (sh_msiof_spi_drv_init) from [<c0a00e1c>]
>> (do_one_initcall+0x10c/0x1c0)
>> [<c0a00d10>] (do_one_initcall) from [<c0a00ff8>]
>> (kernel_init_freeable+0x128/0x1f4)
>>   r9:c0c34000 r8:c0c34000 r7:c0a47e60 r6:c0a3c83c r5:000000bd r4:00000006
>> [<c0a00ed0>] (kernel_init_freeable) from [<c071067c>]
>> (kernel_init+0x10/0x118)
>>   r9:00000000[    1.879529] ata1: link resume succeeded after 1 retries
>>   r8:00000000 r7:00000000 r6:00000000 r5:c071066c r4:00000000
>> [<c071066c>] (kernel_init) from [<c0206e08>] (ret_from_fork+0x14/0x2c)
>>   r5:c071066c r4:00000000
>> ---[ end trace 6eb9a3df3009d491 ]---
>>
>> Niklas S?derlund (2):
>>    dma-mapping: add __dma_sync_single_for_device()
>>    iommu/io-pgtable-arm: use __dma_sync_single_for_device()
>>
>>   drivers/iommu/io-pgtable-arm.c | 2 +-
>>   include/linux/dma-mapping.h    | 9 ++++++++-
>>   2 files changed, 9 insertions(+), 2 deletions(-)
>>
>> --
>> 2.8.2
>>
>>
>> _______________________________________________
>> linux-arm-kernel mailing list
>> linux-arm-kernel at lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>>
>
> _______________________________________________
> iommu mailing list
> iommu at lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu

^ permalink raw reply related	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/2] Fix incorrect warning from dma-debug
  2016-05-09  9:37   ` Robin Murphy
@ 2017-01-25 16:23     ` Geert Uytterhoeven
  -1 siblings, 0 replies; 26+ messages in thread
From: Geert Uytterhoeven @ 2017-01-25 16:23 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Niklas Söderlund, linux-arm-kernel, iommu, dmaengine,
	Joerg Roedel, Will Deacon, linux-kernel, Linux-Renesas,
	Magnus Damm

Hi Robin,

On Mon, May 9, 2016 at 11:37 AM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 08/05/16 11:59, Niklas Söderlund wrote:
>> While using CONFIG_DMA_API_DEBUG i came across this warning which I
>> think is a false positive. As shown dma_sync_single_for_device() are
>> called from the dma_map_single() call path. This triggers the warning
>> since the dma-debug code have not yet been made aware of the mapping.
>
> Almost right ;) The thing being mapped (the SPI device's buffer) and the
> thing being synced (the IOMMU's PTE) are entirely unrelated. Due to the
> current of_iommu_init() setup, the IOMMU is probed long before
> dma_debug_init() gets called, therefore DMA debug is missing entries for
> some of the initial page table mappings and gets confused when we update
> them later.

I think I've been seeing the same as Niklas since quite a while.
Finally I had a deeper look, and it looks like there is a bug somewhere,
causing the wrong IOMMU PTE to be synced.

>> I try to solve this by introducing __dma_sync_single_for_device() which
>> do not call into the dma-debug code. I'm no expert and this might be a
>> bad way of solving the problem but it allowed me to keep working.
>
> The simple fix should be to just call dma_debug_init() from a sufficiently
> earlier initcall level. The best would be to sort out a proper device
> dependency order to avoid the whole early-IOMMU-creation thing entirely.

And so I did. After disabling the call to dma_debug_fs_init(), you can call
dma_debug_init() quite early. But the warning didn't go away:

    ipmmu-vmsa e67b0000.mmu: DMA-API: device driver tries to sync DMA memory
        it has not allocated [device address=0x000000067bab2ff8] [size=8 bytes]
    ------------[ cut here ]------------
    WARNING: CPU: 0 PID: 174 at lib/dma-debug.c:1235 check_sync+0xcc/0x568
    ...
    [<ffffff800823a3a4>] check_sync+0xcc/0x568
    [<ffffff800823a8d0>] debug_dma_sync_single_for_device+0x44/0x4c
    [<ffffff80082b8d34>] __arm_lpae_set_pte.isra.3+0x6c/0x78
    [<ffffff80082b977c>] __arm_lpae_map+0x318/0x384
    [<ffffff80082b9c58>] arm_lpae_map+0xb0/0xc4
    [<ffffff80082bbc58>] ipmmu_map+0x48/0x58
    [<ffffff80082b6754>] iommu_map+0x120/0x1fc
    [<ffffff80082b7bc8>] __iommu_dma_map+0xb8/0xec
    [<ffffff80082b8514>] iommu_dma_map_page+0x50/0x58
    [<ffffff8008092d28>] __iommu_map_page+0x54/0x98

So, who allocated that memory?

During early kernel init (before fs_initcall(dma_debug_init)):

    arm-lpae io-pgtable: arm_lpae_alloc_pgtable:652: cfg->ias = 32
        data->pg_shift = 12 va_bits = 20
    arm-lpae io-pgtable: arm_lpae_alloc_pgtable:657: data->bits_per_level = 9
        data->levels = 3 pgd_bits = 2
    ipmmu-vmsa e67b0000.mmu: __arm_lpae_alloc_pages:224
        dma_map_single(0xffffffc63bab2000, 32) returned 0x000000067bab2000

Hence 0x67bab2000 is the PGD, which has only 4 entries (32 bytes).
Call stack:

    [<ffffff80082b9240>] __arm_lpae_alloc_pages.isra.11+0x144/0x1e8
    [<ffffff80082b93c0>] arm_64_lpae_alloc_pgtable_s1+0xdc/0x118
    [<ffffff80082b9440>] arm_32_lpae_alloc_pgtable_s1+0x44/0x68
    [<ffffff80082b8b1c>] alloc_io_pgtable_ops+0x4c/0x80
    [<ffffff80082bbf28>] ipmmu_attach_device+0xd0/0x3b0

When starting DMA from the device:

    iommu: map: iova 0xfffffff000 pa 0x000000067a555000 size 0x1000 pgsize 4096
    arm-lpae io-pgtable: __arm_lpae_map:318: iova 0xfffffff000
        phys 0x000000067a555000 size 4096 lvl 1 ptep 0xffffffc63bab2000
    arm-lpae io-pgtable: __arm_lpae_map:320: incr. ptep to 0xffffffc63bab2ff8
    ipmmu-vmsa e67b0000.mmu: __arm_lpae_alloc_pages:224
        dma_map_single(0xffffffc63a490000, 4096) returned 0x000000067a490000
    ipmmu-vmsa e67b0000.mmu: DMA-API: device driver tries to sync DMA memory
        it has not allocated [device address=0x000000067bab2ff8] [size=8 bytes]

__arm_lpae_map() added "ARM_LPAE_LVL_IDX(iova, lvl, data)" == 0xff8 to ptep
(the PGD base address), but the PGD has only 32 bytes, leading to the warning.

Does my analysis make sense?
Do you have a clue?

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 0/2] Fix incorrect warning from dma-debug
@ 2017-01-25 16:23     ` Geert Uytterhoeven
  0 siblings, 0 replies; 26+ messages in thread
From: Geert Uytterhoeven @ 2017-01-25 16:23 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Robin,

On Mon, May 9, 2016 at 11:37 AM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 08/05/16 11:59, Niklas S?derlund wrote:
>> While using CONFIG_DMA_API_DEBUG i came across this warning which I
>> think is a false positive. As shown dma_sync_single_for_device() are
>> called from the dma_map_single() call path. This triggers the warning
>> since the dma-debug code have not yet been made aware of the mapping.
>
> Almost right ;) The thing being mapped (the SPI device's buffer) and the
> thing being synced (the IOMMU's PTE) are entirely unrelated. Due to the
> current of_iommu_init() setup, the IOMMU is probed long before
> dma_debug_init() gets called, therefore DMA debug is missing entries for
> some of the initial page table mappings and gets confused when we update
> them later.

I think I've been seeing the same as Niklas since quite a while.
Finally I had a deeper look, and it looks like there is a bug somewhere,
causing the wrong IOMMU PTE to be synced.

>> I try to solve this by introducing __dma_sync_single_for_device() which
>> do not call into the dma-debug code. I'm no expert and this might be a
>> bad way of solving the problem but it allowed me to keep working.
>
> The simple fix should be to just call dma_debug_init() from a sufficiently
> earlier initcall level. The best would be to sort out a proper device
> dependency order to avoid the whole early-IOMMU-creation thing entirely.

And so I did. After disabling the call to dma_debug_fs_init(), you can call
dma_debug_init() quite early. But the warning didn't go away:

    ipmmu-vmsa e67b0000.mmu: DMA-API: device driver tries to sync DMA memory
        it has not allocated [device address=0x000000067bab2ff8] [size=8 bytes]
    ------------[ cut here ]------------
    WARNING: CPU: 0 PID: 174 at lib/dma-debug.c:1235 check_sync+0xcc/0x568
    ...
    [<ffffff800823a3a4>] check_sync+0xcc/0x568
    [<ffffff800823a8d0>] debug_dma_sync_single_for_device+0x44/0x4c
    [<ffffff80082b8d34>] __arm_lpae_set_pte.isra.3+0x6c/0x78
    [<ffffff80082b977c>] __arm_lpae_map+0x318/0x384
    [<ffffff80082b9c58>] arm_lpae_map+0xb0/0xc4
    [<ffffff80082bbc58>] ipmmu_map+0x48/0x58
    [<ffffff80082b6754>] iommu_map+0x120/0x1fc
    [<ffffff80082b7bc8>] __iommu_dma_map+0xb8/0xec
    [<ffffff80082b8514>] iommu_dma_map_page+0x50/0x58
    [<ffffff8008092d28>] __iommu_map_page+0x54/0x98

So, who allocated that memory?

During early kernel init (before fs_initcall(dma_debug_init)):

    arm-lpae io-pgtable: arm_lpae_alloc_pgtable:652: cfg->ias = 32
        data->pg_shift = 12 va_bits = 20
    arm-lpae io-pgtable: arm_lpae_alloc_pgtable:657: data->bits_per_level = 9
        data->levels = 3 pgd_bits = 2
    ipmmu-vmsa e67b0000.mmu: __arm_lpae_alloc_pages:224
        dma_map_single(0xffffffc63bab2000, 32) returned 0x000000067bab2000

Hence 0x67bab2000 is the PGD, which has only 4 entries (32 bytes).
Call stack:

    [<ffffff80082b9240>] __arm_lpae_alloc_pages.isra.11+0x144/0x1e8
    [<ffffff80082b93c0>] arm_64_lpae_alloc_pgtable_s1+0xdc/0x118
    [<ffffff80082b9440>] arm_32_lpae_alloc_pgtable_s1+0x44/0x68
    [<ffffff80082b8b1c>] alloc_io_pgtable_ops+0x4c/0x80
    [<ffffff80082bbf28>] ipmmu_attach_device+0xd0/0x3b0

When starting DMA from the device:

    iommu: map: iova 0xfffffff000 pa 0x000000067a555000 size 0x1000 pgsize 4096
    arm-lpae io-pgtable: __arm_lpae_map:318: iova 0xfffffff000
        phys 0x000000067a555000 size 4096 lvl 1 ptep 0xffffffc63bab2000
    arm-lpae io-pgtable: __arm_lpae_map:320: incr. ptep to 0xffffffc63bab2ff8
    ipmmu-vmsa e67b0000.mmu: __arm_lpae_alloc_pages:224
        dma_map_single(0xffffffc63a490000, 4096) returned 0x000000067a490000
    ipmmu-vmsa e67b0000.mmu: DMA-API: device driver tries to sync DMA memory
        it has not allocated [device address=0x000000067bab2ff8] [size=8 bytes]

__arm_lpae_map() added "ARM_LPAE_LVL_IDX(iova, lvl, data)" == 0xff8 to ptep
(the PGD base address), but the PGD has only 32 bytes, leading to the warning.

Does my analysis make sense?
Do you have a clue?

Thanks!

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/2] Fix incorrect warning from dma-debug
@ 2017-01-25 17:27       ` Robin Murphy
  0 siblings, 0 replies; 26+ messages in thread
From: Robin Murphy @ 2017-01-25 17:27 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Niklas Söderlund, linux-arm-kernel, iommu, dmaengine,
	Joerg Roedel, Will Deacon, linux-kernel, Linux-Renesas,
	Magnus Damm

On 25/01/17 16:23, Geert Uytterhoeven wrote:
> Hi Robin,

Hi Geert,

> On Mon, May 9, 2016 at 11:37 AM, Robin Murphy <robin.murphy@arm.com> wrote:
>> On 08/05/16 11:59, Niklas Söderlund wrote:
>>> While using CONFIG_DMA_API_DEBUG i came across this warning which I
>>> think is a false positive. As shown dma_sync_single_for_device() are
>>> called from the dma_map_single() call path. This triggers the warning
>>> since the dma-debug code have not yet been made aware of the mapping.
>>
>> Almost right ;) The thing being mapped (the SPI device's buffer) and the
>> thing being synced (the IOMMU's PTE) are entirely unrelated. Due to the
>> current of_iommu_init() setup, the IOMMU is probed long before
>> dma_debug_init() gets called, therefore DMA debug is missing entries for
>> some of the initial page table mappings and gets confused when we update
>> them later.
> 
> I think I've been seeing the same as Niklas since quite a while.
> Finally I had a deeper look, and it looks like there is a bug somewhere,
> causing the wrong IOMMU PTE to be synced.
> 
>>> I try to solve this by introducing __dma_sync_single_for_device() which
>>> do not call into the dma-debug code. I'm no expert and this might be a
>>> bad way of solving the problem but it allowed me to keep working.
>>
>> The simple fix should be to just call dma_debug_init() from a sufficiently
>> earlier initcall level. The best would be to sort out a proper device
>> dependency order to avoid the whole early-IOMMU-creation thing entirely.
> 
> And so I did. After disabling the call to dma_debug_fs_init(), you can call
> dma_debug_init() quite early. But the warning didn't go away:

Yet the underlying reason has subtly changed!

>     ipmmu-vmsa e67b0000.mmu: DMA-API: device driver tries to sync DMA memory
>         it has not allocated [device address=0x000000067bab2ff8] [size=8 bytes]
>     ------------[ cut here ]------------
>     WARNING: CPU: 0 PID: 174 at lib/dma-debug.c:1235 check_sync+0xcc/0x568
>     ...
>     [<ffffff800823a3a4>] check_sync+0xcc/0x568
>     [<ffffff800823a8d0>] debug_dma_sync_single_for_device+0x44/0x4c
>     [<ffffff80082b8d34>] __arm_lpae_set_pte.isra.3+0x6c/0x78
>     [<ffffff80082b977c>] __arm_lpae_map+0x318/0x384
>     [<ffffff80082b9c58>] arm_lpae_map+0xb0/0xc4
>     [<ffffff80082bbc58>] ipmmu_map+0x48/0x58
>     [<ffffff80082b6754>] iommu_map+0x120/0x1fc
>     [<ffffff80082b7bc8>] __iommu_dma_map+0xb8/0xec
>     [<ffffff80082b8514>] iommu_dma_map_page+0x50/0x58
>     [<ffffff8008092d28>] __iommu_map_page+0x54/0x98
> 
> So, who allocated that memory?
> 
> During early kernel init (before fs_initcall(dma_debug_init)):
> 
>     arm-lpae io-pgtable: arm_lpae_alloc_pgtable:652: cfg->ias = 32

Note that you have a 32-bit IAS...

>         data->pg_shift = 12 va_bits = 20
>     arm-lpae io-pgtable: arm_lpae_alloc_pgtable:657: data->bits_per_level = 9
>         data->levels = 3 pgd_bits = 2
>     ipmmu-vmsa e67b0000.mmu: __arm_lpae_alloc_pages:224
>         dma_map_single(0xffffffc63bab2000, 32) returned 0x000000067bab2000
> 
> Hence 0x67bab2000 is the PGD, which has only 4 entries (32 bytes).
> Call stack:
> 
>     [<ffffff80082b9240>] __arm_lpae_alloc_pages.isra.11+0x144/0x1e8
>     [<ffffff80082b93c0>] arm_64_lpae_alloc_pgtable_s1+0xdc/0x118
>     [<ffffff80082b9440>] arm_32_lpae_alloc_pgtable_s1+0x44/0x68
>     [<ffffff80082b8b1c>] alloc_io_pgtable_ops+0x4c/0x80
>     [<ffffff80082bbf28>] ipmmu_attach_device+0xd0/0x3b0
> 
> When starting DMA from the device:
> 
>     iommu: map: iova 0xfffffff000 pa 0x000000067a555000 size 0x1000 pgsize 4096

...then count the f's carefully.

>     arm-lpae io-pgtable: __arm_lpae_map:318: iova 0xfffffff000
>         phys 0x000000067a555000 size 4096 lvl 1 ptep 0xffffffc63bab2000
>     arm-lpae io-pgtable: __arm_lpae_map:320: incr. ptep to 0xffffffc63bab2ff8
>     ipmmu-vmsa e67b0000.mmu: __arm_lpae_alloc_pages:224
>         dma_map_single(0xffffffc63a490000, 4096) returned 0x000000067a490000
>     ipmmu-vmsa e67b0000.mmu: DMA-API: device driver tries to sync DMA memory
>         it has not allocated [device address=0x000000067bab2ff8] [size=8 bytes]
> 
> __arm_lpae_map() added "ARM_LPAE_LVL_IDX(iova, lvl, data)" == 0xff8 to ptep
> (the PGD base address), but the PGD has only 32 bytes, leading to the warning.
> 
> Does my analysis make sense?
> Do you have a clue?

The initial false positive misleads from the fact that this is actually
DMA-debug doing its job admirably. The bug lies in however you ended up
trying to map a 40-bit IOVA in a 32-bit pagetable.

Robin.

> 
> Thanks!
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/2] Fix incorrect warning from dma-debug
@ 2017-01-25 17:27       ` Robin Murphy
  0 siblings, 0 replies; 26+ messages in thread
From: Robin Murphy @ 2017-01-25 17:27 UTC (permalink / raw)
  To: Geert Uytterhoeven
  Cc: Niklas Söderlund, Magnus Damm, Will Deacon,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA, Linux-Renesas,
	iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
	dmaengine-u79uwXL29TY76Z2rM5mHXA,
	linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r

On 25/01/17 16:23, Geert Uytterhoeven wrote:
> Hi Robin,

Hi Geert,

> On Mon, May 9, 2016 at 11:37 AM, Robin Murphy <robin.murphy@arm.com> wrote:
>> On 08/05/16 11:59, Niklas Söderlund wrote:
>>> While using CONFIG_DMA_API_DEBUG i came across this warning which I
>>> think is a false positive. As shown dma_sync_single_for_device() are
>>> called from the dma_map_single() call path. This triggers the warning
>>> since the dma-debug code have not yet been made aware of the mapping.
>>
>> Almost right ;) The thing being mapped (the SPI device's buffer) and the
>> thing being synced (the IOMMU's PTE) are entirely unrelated. Due to the
>> current of_iommu_init() setup, the IOMMU is probed long before
>> dma_debug_init() gets called, therefore DMA debug is missing entries for
>> some of the initial page table mappings and gets confused when we update
>> them later.
> 
> I think I've been seeing the same as Niklas since quite a while.
> Finally I had a deeper look, and it looks like there is a bug somewhere,
> causing the wrong IOMMU PTE to be synced.
> 
>>> I try to solve this by introducing __dma_sync_single_for_device() which
>>> do not call into the dma-debug code. I'm no expert and this might be a
>>> bad way of solving the problem but it allowed me to keep working.
>>
>> The simple fix should be to just call dma_debug_init() from a sufficiently
>> earlier initcall level. The best would be to sort out a proper device
>> dependency order to avoid the whole early-IOMMU-creation thing entirely.
> 
> And so I did. After disabling the call to dma_debug_fs_init(), you can call
> dma_debug_init() quite early. But the warning didn't go away:

Yet the underlying reason has subtly changed!

>     ipmmu-vmsa e67b0000.mmu: DMA-API: device driver tries to sync DMA memory
>         it has not allocated [device address=0x000000067bab2ff8] [size=8 bytes]
>     ------------[ cut here ]------------
>     WARNING: CPU: 0 PID: 174 at lib/dma-debug.c:1235 check_sync+0xcc/0x568
>     ...
>     [<ffffff800823a3a4>] check_sync+0xcc/0x568
>     [<ffffff800823a8d0>] debug_dma_sync_single_for_device+0x44/0x4c
>     [<ffffff80082b8d34>] __arm_lpae_set_pte.isra.3+0x6c/0x78
>     [<ffffff80082b977c>] __arm_lpae_map+0x318/0x384
>     [<ffffff80082b9c58>] arm_lpae_map+0xb0/0xc4
>     [<ffffff80082bbc58>] ipmmu_map+0x48/0x58
>     [<ffffff80082b6754>] iommu_map+0x120/0x1fc
>     [<ffffff80082b7bc8>] __iommu_dma_map+0xb8/0xec
>     [<ffffff80082b8514>] iommu_dma_map_page+0x50/0x58
>     [<ffffff8008092d28>] __iommu_map_page+0x54/0x98
> 
> So, who allocated that memory?
> 
> During early kernel init (before fs_initcall(dma_debug_init)):
> 
>     arm-lpae io-pgtable: arm_lpae_alloc_pgtable:652: cfg->ias = 32

Note that you have a 32-bit IAS...

>         data->pg_shift = 12 va_bits = 20
>     arm-lpae io-pgtable: arm_lpae_alloc_pgtable:657: data->bits_per_level = 9
>         data->levels = 3 pgd_bits = 2
>     ipmmu-vmsa e67b0000.mmu: __arm_lpae_alloc_pages:224
>         dma_map_single(0xffffffc63bab2000, 32) returned 0x000000067bab2000
> 
> Hence 0x67bab2000 is the PGD, which has only 4 entries (32 bytes).
> Call stack:
> 
>     [<ffffff80082b9240>] __arm_lpae_alloc_pages.isra.11+0x144/0x1e8
>     [<ffffff80082b93c0>] arm_64_lpae_alloc_pgtable_s1+0xdc/0x118
>     [<ffffff80082b9440>] arm_32_lpae_alloc_pgtable_s1+0x44/0x68
>     [<ffffff80082b8b1c>] alloc_io_pgtable_ops+0x4c/0x80
>     [<ffffff80082bbf28>] ipmmu_attach_device+0xd0/0x3b0
> 
> When starting DMA from the device:
> 
>     iommu: map: iova 0xfffffff000 pa 0x000000067a555000 size 0x1000 pgsize 4096

...then count the f's carefully.

>     arm-lpae io-pgtable: __arm_lpae_map:318: iova 0xfffffff000
>         phys 0x000000067a555000 size 4096 lvl 1 ptep 0xffffffc63bab2000
>     arm-lpae io-pgtable: __arm_lpae_map:320: incr. ptep to 0xffffffc63bab2ff8
>     ipmmu-vmsa e67b0000.mmu: __arm_lpae_alloc_pages:224
>         dma_map_single(0xffffffc63a490000, 4096) returned 0x000000067a490000
>     ipmmu-vmsa e67b0000.mmu: DMA-API: device driver tries to sync DMA memory
>         it has not allocated [device address=0x000000067bab2ff8] [size=8 bytes]
> 
> __arm_lpae_map() added "ARM_LPAE_LVL_IDX(iova, lvl, data)" == 0xff8 to ptep
> (the PGD base address), but the PGD has only 32 bytes, leading to the warning.
> 
> Does my analysis make sense?
> Do you have a clue?

The initial false positive misleads from the fact that this is actually
DMA-debug doing its job admirably. The bug lies in however you ended up
trying to map a 40-bit IOVA in a 32-bit pagetable.

Robin.

> 
> Thanks!
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 

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

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 0/2] Fix incorrect warning from dma-debug
@ 2017-01-25 17:27       ` Robin Murphy
  0 siblings, 0 replies; 26+ messages in thread
From: Robin Murphy @ 2017-01-25 17:27 UTC (permalink / raw)
  To: linux-arm-kernel

On 25/01/17 16:23, Geert Uytterhoeven wrote:
> Hi Robin,

Hi Geert,

> On Mon, May 9, 2016 at 11:37 AM, Robin Murphy <robin.murphy@arm.com> wrote:
>> On 08/05/16 11:59, Niklas S?derlund wrote:
>>> While using CONFIG_DMA_API_DEBUG i came across this warning which I
>>> think is a false positive. As shown dma_sync_single_for_device() are
>>> called from the dma_map_single() call path. This triggers the warning
>>> since the dma-debug code have not yet been made aware of the mapping.
>>
>> Almost right ;) The thing being mapped (the SPI device's buffer) and the
>> thing being synced (the IOMMU's PTE) are entirely unrelated. Due to the
>> current of_iommu_init() setup, the IOMMU is probed long before
>> dma_debug_init() gets called, therefore DMA debug is missing entries for
>> some of the initial page table mappings and gets confused when we update
>> them later.
> 
> I think I've been seeing the same as Niklas since quite a while.
> Finally I had a deeper look, and it looks like there is a bug somewhere,
> causing the wrong IOMMU PTE to be synced.
> 
>>> I try to solve this by introducing __dma_sync_single_for_device() which
>>> do not call into the dma-debug code. I'm no expert and this might be a
>>> bad way of solving the problem but it allowed me to keep working.
>>
>> The simple fix should be to just call dma_debug_init() from a sufficiently
>> earlier initcall level. The best would be to sort out a proper device
>> dependency order to avoid the whole early-IOMMU-creation thing entirely.
> 
> And so I did. After disabling the call to dma_debug_fs_init(), you can call
> dma_debug_init() quite early. But the warning didn't go away:

Yet the underlying reason has subtly changed!

>     ipmmu-vmsa e67b0000.mmu: DMA-API: device driver tries to sync DMA memory
>         it has not allocated [device address=0x000000067bab2ff8] [size=8 bytes]
>     ------------[ cut here ]------------
>     WARNING: CPU: 0 PID: 174 at lib/dma-debug.c:1235 check_sync+0xcc/0x568
>     ...
>     [<ffffff800823a3a4>] check_sync+0xcc/0x568
>     [<ffffff800823a8d0>] debug_dma_sync_single_for_device+0x44/0x4c
>     [<ffffff80082b8d34>] __arm_lpae_set_pte.isra.3+0x6c/0x78
>     [<ffffff80082b977c>] __arm_lpae_map+0x318/0x384
>     [<ffffff80082b9c58>] arm_lpae_map+0xb0/0xc4
>     [<ffffff80082bbc58>] ipmmu_map+0x48/0x58
>     [<ffffff80082b6754>] iommu_map+0x120/0x1fc
>     [<ffffff80082b7bc8>] __iommu_dma_map+0xb8/0xec
>     [<ffffff80082b8514>] iommu_dma_map_page+0x50/0x58
>     [<ffffff8008092d28>] __iommu_map_page+0x54/0x98
> 
> So, who allocated that memory?
> 
> During early kernel init (before fs_initcall(dma_debug_init)):
> 
>     arm-lpae io-pgtable: arm_lpae_alloc_pgtable:652: cfg->ias = 32

Note that you have a 32-bit IAS...

>         data->pg_shift = 12 va_bits = 20
>     arm-lpae io-pgtable: arm_lpae_alloc_pgtable:657: data->bits_per_level = 9
>         data->levels = 3 pgd_bits = 2
>     ipmmu-vmsa e67b0000.mmu: __arm_lpae_alloc_pages:224
>         dma_map_single(0xffffffc63bab2000, 32) returned 0x000000067bab2000
> 
> Hence 0x67bab2000 is the PGD, which has only 4 entries (32 bytes).
> Call stack:
> 
>     [<ffffff80082b9240>] __arm_lpae_alloc_pages.isra.11+0x144/0x1e8
>     [<ffffff80082b93c0>] arm_64_lpae_alloc_pgtable_s1+0xdc/0x118
>     [<ffffff80082b9440>] arm_32_lpae_alloc_pgtable_s1+0x44/0x68
>     [<ffffff80082b8b1c>] alloc_io_pgtable_ops+0x4c/0x80
>     [<ffffff80082bbf28>] ipmmu_attach_device+0xd0/0x3b0
> 
> When starting DMA from the device:
> 
>     iommu: map: iova 0xfffffff000 pa 0x000000067a555000 size 0x1000 pgsize 4096

...then count the f's carefully.

>     arm-lpae io-pgtable: __arm_lpae_map:318: iova 0xfffffff000
>         phys 0x000000067a555000 size 4096 lvl 1 ptep 0xffffffc63bab2000
>     arm-lpae io-pgtable: __arm_lpae_map:320: incr. ptep to 0xffffffc63bab2ff8
>     ipmmu-vmsa e67b0000.mmu: __arm_lpae_alloc_pages:224
>         dma_map_single(0xffffffc63a490000, 4096) returned 0x000000067a490000
>     ipmmu-vmsa e67b0000.mmu: DMA-API: device driver tries to sync DMA memory
>         it has not allocated [device address=0x000000067bab2ff8] [size=8 bytes]
> 
> __arm_lpae_map() added "ARM_LPAE_LVL_IDX(iova, lvl, data)" == 0xff8 to ptep
> (the PGD base address), but the PGD has only 32 bytes, leading to the warning.
> 
> Does my analysis make sense?
> Do you have a clue?

The initial false positive misleads from the fact that this is actually
DMA-debug doing its job admirably. The bug lies in however you ended up
trying to map a 40-bit IOVA in a 32-bit pagetable.

Robin.

> 
> Thanks!
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org
> 
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/2] Fix incorrect warning from dma-debug
  2017-01-25 17:27       ` Robin Murphy
@ 2017-01-25 20:32         ` Geert Uytterhoeven
  -1 siblings, 0 replies; 26+ messages in thread
From: Geert Uytterhoeven @ 2017-01-25 20:32 UTC (permalink / raw)
  To: Robin Murphy
  Cc: Niklas Söderlund, linux-arm-kernel, iommu, dmaengine,
	Joerg Roedel, Will Deacon, linux-kernel, Linux-Renesas,
	Magnus Damm

Hi Robin,

On Wed, Jan 25, 2017 at 6:27 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 25/01/17 16:23, Geert Uytterhoeven wrote:
>> On Mon, May 9, 2016 at 11:37 AM, Robin Murphy <robin.murphy@arm.com> wrote:
>>> On 08/05/16 11:59, Niklas Söderlund wrote:
>>>> While using CONFIG_DMA_API_DEBUG i came across this warning which I
>>>> think is a false positive. As shown dma_sync_single_for_device() are
>>>> called from the dma_map_single() call path. This triggers the warning
>>>> since the dma-debug code have not yet been made aware of the mapping.
>>>
>>> Almost right ;) The thing being mapped (the SPI device's buffer) and the
>>> thing being synced (the IOMMU's PTE) are entirely unrelated. Due to the
>>> current of_iommu_init() setup, the IOMMU is probed long before
>>> dma_debug_init() gets called, therefore DMA debug is missing entries for
>>> some of the initial page table mappings and gets confused when we update
>>> them later.
>>
>> I think I've been seeing the same as Niklas since quite a while.
>> Finally I had a deeper look, and it looks like there is a bug somewhere,
>> causing the wrong IOMMU PTE to be synced.
>>
>>>> I try to solve this by introducing __dma_sync_single_for_device() which
>>>> do not call into the dma-debug code. I'm no expert and this might be a
>>>> bad way of solving the problem but it allowed me to keep working.
>>>
>>> The simple fix should be to just call dma_debug_init() from a sufficiently
>>> earlier initcall level. The best would be to sort out a proper device
>>> dependency order to avoid the whole early-IOMMU-creation thing entirely.
>>
>> And so I did. After disabling the call to dma_debug_fs_init(), you can call
>> dma_debug_init() quite early. But the warning didn't go away:
>
> Yet the underlying reason has subtly changed!
>
>>     ipmmu-vmsa e67b0000.mmu: DMA-API: device driver tries to sync DMA memory
>>         it has not allocated [device address=0x000000067bab2ff8] [size=8 bytes]
>>     ------------[ cut here ]------------
>>     WARNING: CPU: 0 PID: 174 at lib/dma-debug.c:1235 check_sync+0xcc/0x568
>>     ...
>>     [<ffffff800823a3a4>] check_sync+0xcc/0x568
>>     [<ffffff800823a8d0>] debug_dma_sync_single_for_device+0x44/0x4c
>>     [<ffffff80082b8d34>] __arm_lpae_set_pte.isra.3+0x6c/0x78
>>     [<ffffff80082b977c>] __arm_lpae_map+0x318/0x384
>>     [<ffffff80082b9c58>] arm_lpae_map+0xb0/0xc4
>>     [<ffffff80082bbc58>] ipmmu_map+0x48/0x58
>>     [<ffffff80082b6754>] iommu_map+0x120/0x1fc
>>     [<ffffff80082b7bc8>] __iommu_dma_map+0xb8/0xec
>>     [<ffffff80082b8514>] iommu_dma_map_page+0x50/0x58
>>     [<ffffff8008092d28>] __iommu_map_page+0x54/0x98
>>
>> So, who allocated that memory?
>>
>> During early kernel init (before fs_initcall(dma_debug_init)):
>>
>>     arm-lpae io-pgtable: arm_lpae_alloc_pgtable:652: cfg->ias = 32
>
> Note that you have a 32-bit IAS...

Ah, so that's what it means...

>>         data->pg_shift = 12 va_bits = 20
>>     arm-lpae io-pgtable: arm_lpae_alloc_pgtable:657: data->bits_per_level = 9
>>         data->levels = 3 pgd_bits = 2
>>     ipmmu-vmsa e67b0000.mmu: __arm_lpae_alloc_pages:224
>>         dma_map_single(0xffffffc63bab2000, 32) returned 0x000000067bab2000
>>
>> Hence 0x67bab2000 is the PGD, which has only 4 entries (32 bytes).
>> Call stack:
>>
>>     [<ffffff80082b9240>] __arm_lpae_alloc_pages.isra.11+0x144/0x1e8
>>     [<ffffff80082b93c0>] arm_64_lpae_alloc_pgtable_s1+0xdc/0x118
>>     [<ffffff80082b9440>] arm_32_lpae_alloc_pgtable_s1+0x44/0x68
>>     [<ffffff80082b8b1c>] alloc_io_pgtable_ops+0x4c/0x80
>>     [<ffffff80082bbf28>] ipmmu_attach_device+0xd0/0x3b0
>>
>> When starting DMA from the device:
>>
>>     iommu: map: iova 0xfffffff000 pa 0x000000067a555000 size 0x1000 pgsize 4096
>
> ...then count the f's carefully.

Indeed, 40-bits.

>>     arm-lpae io-pgtable: __arm_lpae_map:318: iova 0xfffffff000
>>         phys 0x000000067a555000 size 4096 lvl 1 ptep 0xffffffc63bab2000
>>     arm-lpae io-pgtable: __arm_lpae_map:320: incr. ptep to 0xffffffc63bab2ff8
>>     ipmmu-vmsa e67b0000.mmu: __arm_lpae_alloc_pages:224
>>         dma_map_single(0xffffffc63a490000, 4096) returned 0x000000067a490000
>>     ipmmu-vmsa e67b0000.mmu: DMA-API: device driver tries to sync DMA memory
>>         it has not allocated [device address=0x000000067bab2ff8] [size=8 bytes]
>>
>> __arm_lpae_map() added "ARM_LPAE_LVL_IDX(iova, lvl, data)" == 0xff8 to ptep
>> (the PGD base address), but the PGD has only 32 bytes, leading to the warning.
>>
>> Does my analysis make sense?
>> Do you have a clue?
>
> The initial false positive misleads from the fact that this is actually
> DMA-debug doing its job admirably. The bug lies in however you ended up
> trying to map a 40-bit IOVA in a 32-bit pagetable.

Right, I completely forgot that I was still running with a patch to
allow the DMAC to use 40-bit addresses:

    dma_set_mask_and_coherent(dmac->dev, DMA_BIT_MASK(40));

which works fine without IOMMU, but fails with, as the IPMMU-VMSA driver
doesn't support that yet.

Thanks a lot, and sorry for the noise...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 0/2] Fix incorrect warning from dma-debug
@ 2017-01-25 20:32         ` Geert Uytterhoeven
  0 siblings, 0 replies; 26+ messages in thread
From: Geert Uytterhoeven @ 2017-01-25 20:32 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Robin,

On Wed, Jan 25, 2017 at 6:27 PM, Robin Murphy <robin.murphy@arm.com> wrote:
> On 25/01/17 16:23, Geert Uytterhoeven wrote:
>> On Mon, May 9, 2016 at 11:37 AM, Robin Murphy <robin.murphy@arm.com> wrote:
>>> On 08/05/16 11:59, Niklas S?derlund wrote:
>>>> While using CONFIG_DMA_API_DEBUG i came across this warning which I
>>>> think is a false positive. As shown dma_sync_single_for_device() are
>>>> called from the dma_map_single() call path. This triggers the warning
>>>> since the dma-debug code have not yet been made aware of the mapping.
>>>
>>> Almost right ;) The thing being mapped (the SPI device's buffer) and the
>>> thing being synced (the IOMMU's PTE) are entirely unrelated. Due to the
>>> current of_iommu_init() setup, the IOMMU is probed long before
>>> dma_debug_init() gets called, therefore DMA debug is missing entries for
>>> some of the initial page table mappings and gets confused when we update
>>> them later.
>>
>> I think I've been seeing the same as Niklas since quite a while.
>> Finally I had a deeper look, and it looks like there is a bug somewhere,
>> causing the wrong IOMMU PTE to be synced.
>>
>>>> I try to solve this by introducing __dma_sync_single_for_device() which
>>>> do not call into the dma-debug code. I'm no expert and this might be a
>>>> bad way of solving the problem but it allowed me to keep working.
>>>
>>> The simple fix should be to just call dma_debug_init() from a sufficiently
>>> earlier initcall level. The best would be to sort out a proper device
>>> dependency order to avoid the whole early-IOMMU-creation thing entirely.
>>
>> And so I did. After disabling the call to dma_debug_fs_init(), you can call
>> dma_debug_init() quite early. But the warning didn't go away:
>
> Yet the underlying reason has subtly changed!
>
>>     ipmmu-vmsa e67b0000.mmu: DMA-API: device driver tries to sync DMA memory
>>         it has not allocated [device address=0x000000067bab2ff8] [size=8 bytes]
>>     ------------[ cut here ]------------
>>     WARNING: CPU: 0 PID: 174 at lib/dma-debug.c:1235 check_sync+0xcc/0x568
>>     ...
>>     [<ffffff800823a3a4>] check_sync+0xcc/0x568
>>     [<ffffff800823a8d0>] debug_dma_sync_single_for_device+0x44/0x4c
>>     [<ffffff80082b8d34>] __arm_lpae_set_pte.isra.3+0x6c/0x78
>>     [<ffffff80082b977c>] __arm_lpae_map+0x318/0x384
>>     [<ffffff80082b9c58>] arm_lpae_map+0xb0/0xc4
>>     [<ffffff80082bbc58>] ipmmu_map+0x48/0x58
>>     [<ffffff80082b6754>] iommu_map+0x120/0x1fc
>>     [<ffffff80082b7bc8>] __iommu_dma_map+0xb8/0xec
>>     [<ffffff80082b8514>] iommu_dma_map_page+0x50/0x58
>>     [<ffffff8008092d28>] __iommu_map_page+0x54/0x98
>>
>> So, who allocated that memory?
>>
>> During early kernel init (before fs_initcall(dma_debug_init)):
>>
>>     arm-lpae io-pgtable: arm_lpae_alloc_pgtable:652: cfg->ias = 32
>
> Note that you have a 32-bit IAS...

Ah, so that's what it means...

>>         data->pg_shift = 12 va_bits = 20
>>     arm-lpae io-pgtable: arm_lpae_alloc_pgtable:657: data->bits_per_level = 9
>>         data->levels = 3 pgd_bits = 2
>>     ipmmu-vmsa e67b0000.mmu: __arm_lpae_alloc_pages:224
>>         dma_map_single(0xffffffc63bab2000, 32) returned 0x000000067bab2000
>>
>> Hence 0x67bab2000 is the PGD, which has only 4 entries (32 bytes).
>> Call stack:
>>
>>     [<ffffff80082b9240>] __arm_lpae_alloc_pages.isra.11+0x144/0x1e8
>>     [<ffffff80082b93c0>] arm_64_lpae_alloc_pgtable_s1+0xdc/0x118
>>     [<ffffff80082b9440>] arm_32_lpae_alloc_pgtable_s1+0x44/0x68
>>     [<ffffff80082b8b1c>] alloc_io_pgtable_ops+0x4c/0x80
>>     [<ffffff80082bbf28>] ipmmu_attach_device+0xd0/0x3b0
>>
>> When starting DMA from the device:
>>
>>     iommu: map: iova 0xfffffff000 pa 0x000000067a555000 size 0x1000 pgsize 4096
>
> ...then count the f's carefully.

Indeed, 40-bits.

>>     arm-lpae io-pgtable: __arm_lpae_map:318: iova 0xfffffff000
>>         phys 0x000000067a555000 size 4096 lvl 1 ptep 0xffffffc63bab2000
>>     arm-lpae io-pgtable: __arm_lpae_map:320: incr. ptep to 0xffffffc63bab2ff8
>>     ipmmu-vmsa e67b0000.mmu: __arm_lpae_alloc_pages:224
>>         dma_map_single(0xffffffc63a490000, 4096) returned 0x000000067a490000
>>     ipmmu-vmsa e67b0000.mmu: DMA-API: device driver tries to sync DMA memory
>>         it has not allocated [device address=0x000000067bab2ff8] [size=8 bytes]
>>
>> __arm_lpae_map() added "ARM_LPAE_LVL_IDX(iova, lvl, data)" == 0xff8 to ptep
>> (the PGD base address), but the PGD has only 32 bytes, leading to the warning.
>>
>> Does my analysis make sense?
>> Do you have a clue?
>
> The initial false positive misleads from the fact that this is actually
> DMA-debug doing its job admirably. The bug lies in however you ended up
> trying to map a 40-bit IOVA in a 32-bit pagetable.

Right, I completely forgot that I was still running with a patch to
allow the DMAC to use 40-bit addresses:

    dma_set_mask_and_coherent(dmac->dev, DMA_BIT_MASK(40));

which works fine without IOMMU, but fails with, as the IPMMU-VMSA driver
doesn't support that yet.

Thanks a lot, and sorry for the noise...

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/2] Fix incorrect warning from dma-debug
  2016-05-09 10:00     ` Robin Murphy
@ 2017-05-07  0:06       ` Jon Masters
  -1 siblings, 0 replies; 26+ messages in thread
From: Jon Masters @ 2017-05-07  0:06 UTC (permalink / raw)
  To: Robin Murphy, Niklas Söderlund, linux-arm-kernel, iommu, dmaengine
  Cc: linux-renesas-soc, will.deacon, linux-kernel

On 05/09/2016 06:00 AM, Robin Murphy wrote:
> On 09/05/16 10:37, Robin Murphy wrote:
>> Hi Niklas,
>>
>> On 08/05/16 11:59, Niklas Söderlund wrote:
>>> Hi,
>>>
>>> While using CONFIG_DMA_API_DEBUG i came across this warning which I
>>> think is a false positive. As shown dma_sync_single_for_device() are
>>> called from the dma_map_single() call path. This triggers the warning
>>> since the dma-debug code have not yet been made aware of the mapping.
>>
>> Almost right ;) The thing being mapped (the SPI device's buffer) and the
>> thing being synced (the IOMMU's PTE) are entirely unrelated. Due to the
>> current of_iommu_init() setup, the IOMMU is probed long before
>> dma_debug_init() gets called, therefore DMA debug is missing entries for
>> some of the initial page table mappings and gets confused when we update
>> them later.

What was the eventual followup/fix for this? We're seeing similar traces
to this during internal testing of 4.11 kernels.

Jon.

-- 
Computer Architect

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 0/2] Fix incorrect warning from dma-debug
@ 2017-05-07  0:06       ` Jon Masters
  0 siblings, 0 replies; 26+ messages in thread
From: Jon Masters @ 2017-05-07  0:06 UTC (permalink / raw)
  To: linux-arm-kernel

On 05/09/2016 06:00 AM, Robin Murphy wrote:
> On 09/05/16 10:37, Robin Murphy wrote:
>> Hi Niklas,
>>
>> On 08/05/16 11:59, Niklas S?derlund wrote:
>>> Hi,
>>>
>>> While using CONFIG_DMA_API_DEBUG i came across this warning which I
>>> think is a false positive. As shown dma_sync_single_for_device() are
>>> called from the dma_map_single() call path. This triggers the warning
>>> since the dma-debug code have not yet been made aware of the mapping.
>>
>> Almost right ;) The thing being mapped (the SPI device's buffer) and the
>> thing being synced (the IOMMU's PTE) are entirely unrelated. Due to the
>> current of_iommu_init() setup, the IOMMU is probed long before
>> dma_debug_init() gets called, therefore DMA debug is missing entries for
>> some of the initial page table mappings and gets confused when we update
>> them later.

What was the eventual followup/fix for this? We're seeing similar traces
to this during internal testing of 4.11 kernels.

Jon.

-- 
Computer Architect

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/2] Fix incorrect warning from dma-debug
  2017-05-07  0:06       ` Jon Masters
@ 2017-05-08  9:19         ` Geert Uytterhoeven
  -1 siblings, 0 replies; 26+ messages in thread
From: Geert Uytterhoeven @ 2017-05-08  9:19 UTC (permalink / raw)
  To: Jon Masters
  Cc: Robin Murphy, Niklas Söderlund, linux-arm-kernel, iommu,
	dmaengine, Linux-Renesas, Will Deacon, linux-kernel

Hi Jon,

On Sun, May 7, 2017 at 2:06 AM, Jon Masters <jcm@jonmasters.org> wrote:
> On 05/09/2016 06:00 AM, Robin Murphy wrote:
>> On 09/05/16 10:37, Robin Murphy wrote:
>>> On 08/05/16 11:59, Niklas Söderlund wrote:
>>>> While using CONFIG_DMA_API_DEBUG i came across this warning which I
>>>> think is a false positive. As shown dma_sync_single_for_device() are
>>>> called from the dma_map_single() call path. This triggers the warning
>>>> since the dma-debug code have not yet been made aware of the mapping.
>>>
>>> Almost right ;) The thing being mapped (the SPI device's buffer) and the
>>> thing being synced (the IOMMU's PTE) are entirely unrelated. Due to the
>>> current of_iommu_init() setup, the IOMMU is probed long before
>>> dma_debug_init() gets called, therefore DMA debug is missing entries for
>>> some of the initial page table mappings and gets confused when we update
>>> them later.
>
> What was the eventual followup/fix for this? We're seeing similar traces
> to this during internal testing of 4.11 kernels.

A quick hack is to change arch/arm64/mm/dma-mapping.c:dma_debug_do_init()
from fs_initcall() to arch_initcall().

However, then you loose the debugfs features. A proper fix would involve
splitting dma_debug_init() in two phases: an early one doing the real work,
and a late one registering the debugfs interface.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 0/2] Fix incorrect warning from dma-debug
@ 2017-05-08  9:19         ` Geert Uytterhoeven
  0 siblings, 0 replies; 26+ messages in thread
From: Geert Uytterhoeven @ 2017-05-08  9:19 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Jon,

On Sun, May 7, 2017 at 2:06 AM, Jon Masters <jcm@jonmasters.org> wrote:
> On 05/09/2016 06:00 AM, Robin Murphy wrote:
>> On 09/05/16 10:37, Robin Murphy wrote:
>>> On 08/05/16 11:59, Niklas S?derlund wrote:
>>>> While using CONFIG_DMA_API_DEBUG i came across this warning which I
>>>> think is a false positive. As shown dma_sync_single_for_device() are
>>>> called from the dma_map_single() call path. This triggers the warning
>>>> since the dma-debug code have not yet been made aware of the mapping.
>>>
>>> Almost right ;) The thing being mapped (the SPI device's buffer) and the
>>> thing being synced (the IOMMU's PTE) are entirely unrelated. Due to the
>>> current of_iommu_init() setup, the IOMMU is probed long before
>>> dma_debug_init() gets called, therefore DMA debug is missing entries for
>>> some of the initial page table mappings and gets confused when we update
>>> them later.
>
> What was the eventual followup/fix for this? We're seeing similar traces
> to this during internal testing of 4.11 kernels.

A quick hack is to change arch/arm64/mm/dma-mapping.c:dma_debug_do_init()
from fs_initcall() to arch_initcall().

However, then you loose the debugfs features. A proper fix would involve
splitting dma_debug_init() in two phases: an early one doing the real work,
and a late one registering the debugfs interface.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert at linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 26+ messages in thread

* Re: [PATCH 0/2] Fix incorrect warning from dma-debug
  2017-05-07  0:06       ` Jon Masters
@ 2017-05-08  9:39         ` Robin Murphy
  -1 siblings, 0 replies; 26+ messages in thread
From: Robin Murphy @ 2017-05-08  9:39 UTC (permalink / raw)
  To: Jon Masters
  Cc: Niklas Söderlund, linux-arm-kernel, iommu, dmaengine,
	linux-renesas-soc, will.deacon, linux-kernel

On 07/05/17 01:06, Jon Masters wrote:
> On 05/09/2016 06:00 AM, Robin Murphy wrote:
>> On 09/05/16 10:37, Robin Murphy wrote:
>>> Hi Niklas,
>>>
>>> On 08/05/16 11:59, Niklas Söderlund wrote:
>>>> Hi,
>>>>
>>>> While using CONFIG_DMA_API_DEBUG i came across this warning which I
>>>> think is a false positive. As shown dma_sync_single_for_device() are
>>>> called from the dma_map_single() call path. This triggers the warning
>>>> since the dma-debug code have not yet been made aware of the mapping.
>>>
>>> Almost right ;) The thing being mapped (the SPI device's buffer) and the
>>> thing being synced (the IOMMU's PTE) are entirely unrelated. Due to the
>>> current of_iommu_init() setup, the IOMMU is probed long before
>>> dma_debug_init() gets called, therefore DMA debug is missing entries for
>>> some of the initial page table mappings and gets confused when we update
>>> them later.
> 
> What was the eventual followup/fix for this? We're seeing similar traces
> to this during internal testing of 4.11 kernels.

The IOMMU probe-deferral patches queued for the current merge window get
rid of the need for early device creation bodges that cause the problem
(of IOMMU pagetables being mapped for DMA before dma-debug is up and
running). In the meantime, I'd suggest either adjusting the initcall
level per 256ff1cf6b44, or turning off the IOMMU driver and/or using
dma-debug's driver-filter option if you're doing more targeted debugging.

Robin.

> 
> Jon.
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

* [PATCH 0/2] Fix incorrect warning from dma-debug
@ 2017-05-08  9:39         ` Robin Murphy
  0 siblings, 0 replies; 26+ messages in thread
From: Robin Murphy @ 2017-05-08  9:39 UTC (permalink / raw)
  To: linux-arm-kernel

On 07/05/17 01:06, Jon Masters wrote:
> On 05/09/2016 06:00 AM, Robin Murphy wrote:
>> On 09/05/16 10:37, Robin Murphy wrote:
>>> Hi Niklas,
>>>
>>> On 08/05/16 11:59, Niklas S?derlund wrote:
>>>> Hi,
>>>>
>>>> While using CONFIG_DMA_API_DEBUG i came across this warning which I
>>>> think is a false positive. As shown dma_sync_single_for_device() are
>>>> called from the dma_map_single() call path. This triggers the warning
>>>> since the dma-debug code have not yet been made aware of the mapping.
>>>
>>> Almost right ;) The thing being mapped (the SPI device's buffer) and the
>>> thing being synced (the IOMMU's PTE) are entirely unrelated. Due to the
>>> current of_iommu_init() setup, the IOMMU is probed long before
>>> dma_debug_init() gets called, therefore DMA debug is missing entries for
>>> some of the initial page table mappings and gets confused when we update
>>> them later.
> 
> What was the eventual followup/fix for this? We're seeing similar traces
> to this during internal testing of 4.11 kernels.

The IOMMU probe-deferral patches queued for the current merge window get
rid of the need for early device creation bodges that cause the problem
(of IOMMU pagetables being mapped for DMA before dma-debug is up and
running). In the meantime, I'd suggest either adjusting the initcall
level per 256ff1cf6b44, or turning off the IOMMU driver and/or using
dma-debug's driver-filter option if you're doing more targeted debugging.

Robin.

> 
> Jon.
> 

^ permalink raw reply	[flat|nested] 26+ messages in thread

end of thread, other threads:[~2017-05-08  9:39 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-08 10:59 [PATCH 0/2] Fix incorrect warning from dma-debug Niklas Söderlund
2016-05-08 10:59 ` Niklas Söderlund
2016-05-08 10:59 ` [PATCH 1/2] dma-mapping: add __dma_sync_single_for_device() Niklas Söderlund
2016-05-08 10:59   ` Niklas Söderlund
2016-05-08 10:59 ` [PATCH 2/2] iommu/io-pgtable-arm: use __dma_sync_single_for_device() Niklas Söderlund
2016-05-08 10:59   ` Niklas Söderlund
2016-05-09  9:35   ` Will Deacon
2016-05-09  9:35     ` Will Deacon
2016-05-09  9:35     ` Will Deacon
2016-05-09  9:37 ` [PATCH 0/2] Fix incorrect warning from dma-debug Robin Murphy
2016-05-09  9:37   ` Robin Murphy
2016-05-09 10:00   ` Robin Murphy
2016-05-09 10:00     ` Robin Murphy
2017-05-07  0:06     ` Jon Masters
2017-05-07  0:06       ` Jon Masters
2017-05-08  9:19       ` Geert Uytterhoeven
2017-05-08  9:19         ` Geert Uytterhoeven
2017-05-08  9:39       ` Robin Murphy
2017-05-08  9:39         ` Robin Murphy
2017-01-25 16:23   ` Geert Uytterhoeven
2017-01-25 16:23     ` Geert Uytterhoeven
2017-01-25 17:27     ` Robin Murphy
2017-01-25 17:27       ` Robin Murphy
2017-01-25 17:27       ` Robin Murphy
2017-01-25 20:32       ` Geert Uytterhoeven
2017-01-25 20:32         ` Geert Uytterhoeven

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.