[PATCHv4 0/3] ARM: implement workaround for Cortex-A9/PL310/PCIe deadlock

[PATCHv4 0/3] ARM: implement workaround for Cortex-A9/PL310/PCIe deadlock

[Thomas Petazzoni]

Russell, Will, Catalin,

This patch series adresses a problem that affects the newer Marvell
Armada 375 and 38x SOCs, based on Cortex-A9+PL310, combined with the
Marvell PCIe hardware unit. When the hardware I/O coherency is
enabled, the combination of Cortex-A9/PL310/Marvell PCIe hardware unit
will quickly cause a deadlock when the PCIe bus is stressed.

The workaround for this problem has been suggested by ARM, and
consists in two things:

 (1) Map the PCIe regions as strongly-ordered

 (2) Disable the outer cache sync of the PL310 when hardware I/O
     coherency is used, since it is unneeded and causes the deadlock.

The following three patches address the problem in the following way:

 * PATCH 1/3 switches the PCI I/O mappings to always be mapped
   MT_UNCACHED instead of MT_DEVICE. As Arnd Bergmann said: "The PCI
   I/O space semantics require non-posted writes, which is the main
   difference between device and strongly-ordered mappings". This
   patch should be routed through Russell's tree.

 * PATCH 2/3 extends the l2x0 cache driver with a new property
   "arm,io-coherent", valid for the PL310, which makes the driver
   disable the outer cache sync operation. This patch should be routed
   through Russell's tree.

 * PATCH 3/3 actually implements the Armada 375/38x workaround, by
   using MT_UNCACHED for PCI memory mappings, and adding the
   "arm,io-coherent" property to the cache controller Device Tree node
   when appropriate (i.e, when hardware I/O coherency is
   enabled). This patch has no build dependency on the two previous
   patches. It should be routed through the mvebu maintainers tree.

Changes since v3:

 - Withdrawn all Acked-by tags since the changes compared to v3 are
   quite significant.

 - Instead of introducing a small mechanism to allow each
   sub-architecture to override the memory type used for PCI I/O
   mappings, simply make all of them mapped MT_UNCACHED instead of
   MT_DEVICE, as suggested by Arnd Bergmann. This also has the nice
   consequence that there is no longer a build dependency between
   PATCH 3/3 and PATCH 1/3. Suggested by Arnd Bergmann.

 - Change the name of the new property of the PL310 DT binding from
   the too generic 'dma-coherent' to 'arm,io-coherent'. Suggested by
   Rob Herring.

 - Instead of adding a complete set of L2 cache operations in
   cache-l2x0.c, simply nullify the outer_cache.sync operation when
   'arm,io-coherent' is specified. Suggested by Rob Herring.

 - Move the Armada 375/38x specific code from mach-mvebu/board-v7.c to
   mach-mvebu/coherency.c, which makes more sense. Suggested by Arnd
   Bergmann.

Changes since v2:

 - Added Acked-by from Catalin on "ARM: mm: allow sub-architectures to
   override PCI I/O memory type".

 - Dropped the patch fixing the of_update_property() function, since
   we're no longer using it.

 - Instead of using a different compatible string to identify PL310
   used in an I/O coherent configuration, use a separate boolean
   property. Suggested by Catalin.

 - Rework the mach-mvebu/coherency.c to add the boolean property
   "dma-coherent" when needed instead of updating the compatible
   string of the cache controller.

Changes since v1:

 - Instead of introducing separate l2x0 initialization functions, rely
   on a separate compatible string to identify whether we're coherent
   or not. The compatible string *has* to be modified at runtime,
   because Armada 375 and 38x are only I/O coherent when in SMP
   mode. In non-SMP mode, they are not I/O coherent, so we cannot
   change the DT to 'arm,pl310-coherent-cache'.

 - Addition of the drivers/of fix to be able to use
   of_update_property() early and fix up the PL310 compatible string,
   as explained in the previous item.

Thanks!

Thomas

Thomas Petazzoni (3):
  ARM: mm: use MT_UNCACHED as the memory type for PCI I/O mappings
  ARM: mm: add support for HW coherent systems in PL310
  ARM: mvebu: implement L2/PCIe deadlock workaround

 Documentation/devicetree/bindings/arm/l2cc.txt |  3 ++
 arch/arm/mach-mvebu/coherency.c                | 39 ++++++++++++++++++++++++++
 arch/arm/mm/cache-l2x0.c                       | 14 +++++++++
 arch/arm/mm/ioremap.c                          |  2 +-
 4 files changed, 57 insertions(+), 1 deletion(-)

-- 
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 0/3] ARM: implement workaround for Cortex-A9/PL310/PCIe deadlock

[Thomas Petazzoni]

Russell, Will, Catalin,

This patch series adresses a problem that affects the newer Marvell
Armada 375 and 38x SOCs, based on Cortex-A9+PL310, combined with the
Marvell PCIe hardware unit. When the hardware I/O coherency is
enabled, the combination of Cortex-A9/PL310/Marvell PCIe hardware unit
will quickly cause a deadlock when the PCIe bus is stressed.

The workaround for this problem has been suggested by ARM, and
consists in two things:

 (1) Map the PCIe regions as strongly-ordered

 (2) Disable the outer cache sync of the PL310 when hardware I/O
     coherency is used, since it is unneeded and causes the deadlock.

The following three patches address the problem in the following way:

 * PATCH 1/3 switches the PCI I/O mappings to always be mapped
   MT_UNCACHED instead of MT_DEVICE. As Arnd Bergmann said: "The PCI
   I/O space semantics require non-posted writes, which is the main
   difference between device and strongly-ordered mappings". This
   patch should be routed through Russell's tree.

 * PATCH 2/3 extends the l2x0 cache driver with a new property
   "arm,io-coherent", valid for the PL310, which makes the driver
   disable the outer cache sync operation. This patch should be routed
   through Russell's tree.

 * PATCH 3/3 actually implements the Armada 375/38x workaround, by
   using MT_UNCACHED for PCI memory mappings, and adding the
   "arm,io-coherent" property to the cache controller Device Tree node
   when appropriate (i.e, when hardware I/O coherency is
   enabled). This patch has no build dependency on the two previous
   patches. It should be routed through the mvebu maintainers tree.

Changes since v3:

 - Withdrawn all Acked-by tags since the changes compared to v3 are
   quite significant.

 - Instead of introducing a small mechanism to allow each
   sub-architecture to override the memory type used for PCI I/O
   mappings, simply make all of them mapped MT_UNCACHED instead of
   MT_DEVICE, as suggested by Arnd Bergmann. This also has the nice
   consequence that there is no longer a build dependency between
   PATCH 3/3 and PATCH 1/3. Suggested by Arnd Bergmann.

 - Change the name of the new property of the PL310 DT binding from
   the too generic 'dma-coherent' to 'arm,io-coherent'. Suggested by
   Rob Herring.

 - Instead of adding a complete set of L2 cache operations in
   cache-l2x0.c, simply nullify the outer_cache.sync operation when
   'arm,io-coherent' is specified. Suggested by Rob Herring.

 - Move the Armada 375/38x specific code from mach-mvebu/board-v7.c to
   mach-mvebu/coherency.c, which makes more sense. Suggested by Arnd
   Bergmann.

Changes since v2:

 - Added Acked-by from Catalin on "ARM: mm: allow sub-architectures to
   override PCI I/O memory type".

 - Dropped the patch fixing the of_update_property() function, since
   we're no longer using it.

 - Instead of using a different compatible string to identify PL310
   used in an I/O coherent configuration, use a separate boolean
   property. Suggested by Catalin.

 - Rework the mach-mvebu/coherency.c to add the boolean property
   "dma-coherent" when needed instead of updating the compatible
   string of the cache controller.

Changes since v1:

 - Instead of introducing separate l2x0 initialization functions, rely
   on a separate compatible string to identify whether we're coherent
   or not. The compatible string *has* to be modified at runtime,
   because Armada 375 and 38x are only I/O coherent when in SMP
   mode. In non-SMP mode, they are not I/O coherent, so we cannot
   change the DT to 'arm,pl310-coherent-cache'.

 - Addition of the drivers/of fix to be able to use
   of_update_property() early and fix up the PL310 compatible string,
   as explained in the previous item.

Thanks!

Thomas

Thomas Petazzoni (3):
  ARM: mm: use MT_UNCACHED as the memory type for PCI I/O mappings
  ARM: mm: add support for HW coherent systems in PL310
  ARM: mvebu: implement L2/PCIe deadlock workaround

 Documentation/devicetree/bindings/arm/l2cc.txt |  3 ++
 arch/arm/mach-mvebu/coherency.c                | 39 ++++++++++++++++++++++++++
 arch/arm/mm/cache-l2x0.c                       | 14 +++++++++
 arch/arm/mm/ioremap.c                          |  2 +-
 4 files changed, 57 insertions(+), 1 deletion(-)

-- 
1.9.3

[PATCHv4 1/3] ARM: mm: use MT_UNCACHED as the memory type for PCI I/O mappings

[Thomas Petazzoni]

Arnd Bergmann explains: "The PCI I/O space semantics require
non-posted writes, which is the main difference between device and
strongly-ordered mappings."

This patch therefore changes the memory type used for PCI I/O mappings
from MT_DEVICE to MT_UNCACHED. On Marvell EBU platforms, it is part of
a workaround for a deadlock situation occuring on Armada 375/38x
between PCIe/PL310 and hardware I/O coherency in SMP situations.

As Arnd Bergmann indicates: "the same fix is required both for mvebu
as a workaround for the deadlock as well as for everyone else as a fix
for an incorrect PCI behavior."

Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/mm/ioremap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index f9c32ba..631b0e6 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -445,7 +445,7 @@ int pci_ioremap_io(unsigned int offset, phys_addr_t phys_addr)
 	return ioremap_page_range(PCI_IO_VIRT_BASE + offset,
 				  PCI_IO_VIRT_BASE + offset + SZ_64K,
 				  phys_addr,
-				  __pgprot(get_mem_type(MT_DEVICE)->prot_pte));
+				  __pgprot(get_mem_type(MT_UNCACHED)->prot_pte));
 }
 EXPORT_SYMBOL_GPL(pci_ioremap_io);
 #endif
-- 
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 1/3] ARM: mm: use MT_UNCACHED as the memory type for PCI I/O mappings

[Thomas Petazzoni]

Arnd Bergmann explains: "The PCI I/O space semantics require
non-posted writes, which is the main difference between device and
strongly-ordered mappings."

This patch therefore changes the memory type used for PCI I/O mappings
from MT_DEVICE to MT_UNCACHED. On Marvell EBU platforms, it is part of
a workaround for a deadlock situation occuring on Armada 375/38x
between PCIe/PL310 and hardware I/O coherency in SMP situations.

As Arnd Bergmann indicates: "the same fix is required both for mvebu
as a workaround for the deadlock as well as for everyone else as a fix
for an incorrect PCI behavior."

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/mm/ioremap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/mm/ioremap.c b/arch/arm/mm/ioremap.c
index f9c32ba..631b0e6 100644
--- a/arch/arm/mm/ioremap.c
+++ b/arch/arm/mm/ioremap.c
@@ -445,7 +445,7 @@ int pci_ioremap_io(unsigned int offset, phys_addr_t phys_addr)
 	return ioremap_page_range(PCI_IO_VIRT_BASE + offset,
 				  PCI_IO_VIRT_BASE + offset + SZ_64K,
 				  phys_addr,
-				  __pgprot(get_mem_type(MT_DEVICE)->prot_pte));
+				  __pgprot(get_mem_type(MT_UNCACHED)->prot_pte));
 }
 EXPORT_SYMBOL_GPL(pci_ioremap_io);
 #endif
-- 
1.9.3

[PATCHv4 2/3] ARM: mm: add support for HW coherent systems in PL310

[Thomas Petazzoni]

When a PL310 cache is used on a system that provides hardware
coherency, the outer cache sync operation is useless, and can be
skipped. Moreover, on some systems, it is harmful as it causes
deadlocks between the Marvell coherency mechanism, the Marvell PCIe
controller and the Cortex-A9.

To avoid this, this commit introduces a new Device Tree property
'arm,io-coherent' for the L2 cache controller node, valid only for the
PL310 cache. It identifies the usage of the PL310 cache in an I/O
coherent configuration. Internally, it makes the driver disable the
outer cache sync operation.

Note that technically speaking, a fully coherent system wouldn't
require any of the other .outer_cache operations. However, in
practice, when booting secondary CPUs, these are not yet coherent, and
therefore a set of cache maintenance operations are necessary at this
point. This explains why we keep the other .outer_cache operations and
only ->sync is disabled.

While in theory any write to a PL310 register could cause the
deadlock, in practice, disabling ->sync is sufficient to workaround
the deadlock, since the other cache maintenance operations are only
used in very specific situations.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 Documentation/devicetree/bindings/arm/l2cc.txt |  3 +++
 arch/arm/mm/cache-l2x0.c                       | 14 ++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt
index b513cb8..af527ee 100644
--- a/Documentation/devicetree/bindings/arm/l2cc.txt
+++ b/Documentation/devicetree/bindings/arm/l2cc.txt
@@ -40,6 +40,9 @@ Optional properties:
 - arm,filter-ranges : <start length> Starting address and length of window to
   filter. Addresses in the filter window are directed to the M1 port. Other
   addresses will go to the M0 port.
+- arm,io-coherent : indicates that the system is operating in an hardware
+  I/O coherent mode. Valid only when the arm,pl310-cache compatible
+  string is used.
 - interrupts : 1 combined interrupt.
 - cache-id-part: cache id part number to be used if it is not present
   on hardware
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 7abde2ce..199a745 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -1005,6 +1005,20 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
 
 	of_init = true;
 	memcpy(&outer_cache, &data->outer_cache, sizeof(outer_cache));
+
+	/*
+	 * outer sync operations are not needed when the system is I/O
+	 * coherent, and potentially harmful in certain situations
+	 * (PCIe/PL310 deadlock on Armada 375/38x due to hardware I/O
+	 * coherency). The other operations are kept because they are
+	 * infrequent (therefore do not cause the deadlock) and needed
+	 * for secondary CPU boot and other power management
+	 * activities.
+	 */
+	if (of_device_is_compatible(np, "arm,pl310-cache") &&
+	    of_property_read_bool(np, "arm,io-coherent"))
+		outer_cache.sync = NULL;
+
 	l2x0_init(l2x0_base, aux_val, aux_mask);
 
 	return 0;
-- 
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 2/3] ARM: mm: add support for HW coherent systems in PL310

[Thomas Petazzoni]

When a PL310 cache is used on a system that provides hardware
coherency, the outer cache sync operation is useless, and can be
skipped. Moreover, on some systems, it is harmful as it causes
deadlocks between the Marvell coherency mechanism, the Marvell PCIe
controller and the Cortex-A9.

To avoid this, this commit introduces a new Device Tree property
'arm,io-coherent' for the L2 cache controller node, valid only for the
PL310 cache. It identifies the usage of the PL310 cache in an I/O
coherent configuration. Internally, it makes the driver disable the
outer cache sync operation.

Note that technically speaking, a fully coherent system wouldn't
require any of the other .outer_cache operations. However, in
practice, when booting secondary CPUs, these are not yet coherent, and
therefore a set of cache maintenance operations are necessary at this
point. This explains why we keep the other .outer_cache operations and
only ->sync is disabled.

While in theory any write to a PL310 register could cause the
deadlock, in practice, disabling ->sync is sufficient to workaround
the deadlock, since the other cache maintenance operations are only
used in very specific situations.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 Documentation/devicetree/bindings/arm/l2cc.txt |  3 +++
 arch/arm/mm/cache-l2x0.c                       | 14 ++++++++++++++
 2 files changed, 17 insertions(+)

diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt
index b513cb8..af527ee 100644
--- a/Documentation/devicetree/bindings/arm/l2cc.txt
+++ b/Documentation/devicetree/bindings/arm/l2cc.txt
@@ -40,6 +40,9 @@ Optional properties:
 - arm,filter-ranges : <start length> Starting address and length of window to
   filter. Addresses in the filter window are directed to the M1 port. Other
   addresses will go to the M0 port.
+- arm,io-coherent : indicates that the system is operating in an hardware
+  I/O coherent mode. Valid only when the arm,pl310-cache compatible
+  string is used.
 - interrupts : 1 combined interrupt.
 - cache-id-part: cache id part number to be used if it is not present
   on hardware
diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
index 7abde2ce..199a745 100644
--- a/arch/arm/mm/cache-l2x0.c
+++ b/arch/arm/mm/cache-l2x0.c
@@ -1005,6 +1005,20 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
 
 	of_init = true;
 	memcpy(&outer_cache, &data->outer_cache, sizeof(outer_cache));
+
+	/*
+	 * outer sync operations are not needed when the system is I/O
+	 * coherent, and potentially harmful in certain situations
+	 * (PCIe/PL310 deadlock on Armada 375/38x due to hardware I/O
+	 * coherency). The other operations are kept because they are
+	 * infrequent (therefore do not cause the deadlock) and needed
+	 * for secondary CPU boot and other power management
+	 * activities.
+	 */
+	if (of_device_is_compatible(np, "arm,pl310-cache") &&
+	    of_property_read_bool(np, "arm,io-coherent"))
+		outer_cache.sync = NULL;
+
 	l2x0_init(l2x0_base, aux_val, aux_mask);
 
 	return 0;
-- 
1.9.3

[PATCHv4 3/3] ARM: mvebu: implement L2/PCIe deadlock workaround

[Thomas Petazzoni]

The Marvell Armada 375 and Armada 38x SOCs, which use the Cortex-A9
CPU core, the PL310 cache and the Marvell PCIe hardware block are
affected a L2/PCIe deadlock caused by a system erratum when hardware
I/O coherency is used.

This deadlock can be avoided by mapping the PCIe memory areas as
strongly-ordered (note: MT_UNCACHED is strongly-ordered), and by
removing the outer cache sync done in software. This is implemented in
this patch by:

 * Registering a custom arch_ioremap_caller function that allows to
   make sure PCI memory regions are mapped MT_UNCACHED.

 * Adding at runtime the 'arm,io-coherent' property to the PL310 cache
   controller. This cannot be done permanently in the DT, because the
   hardware I/O coherency can only be enabled when CONFIG_SMP is
   enabled, in the current kernel situation.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
---
 arch/arm/mach-mvebu/coherency.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/arm/mach-mvebu/coherency.c b/arch/arm/mach-mvebu/coherency.c
index d5a975b..f6be9c6 100644
--- a/arch/arm/mach-mvebu/coherency.c
+++ b/arch/arm/mach-mvebu/coherency.c
@@ -31,6 +31,7 @@
 #include <linux/clk.h>
 #include <asm/smp_plat.h>
 #include <asm/cacheflush.h>
+#include <asm/mach/map.h>
 #include "armada-370-xp.h"
 #include "coherency.h"
 #include "mvebu-soc-id.h"
@@ -308,9 +309,47 @@ static void __init armada_370_coherency_init(struct device_node *np)
 	set_cpu_coherent();
 }
 
+/*
+ * This ioremap hook is used on Armada 375/38x to ensure that PCIe
+ * memory areas are mapped as MT_UNCACHED instead of MT_DEVICE. This
+ * is needed as a workaround for a deadlock issue between the PCIe
+ * interface and the cache controller.
+ */
+static void __iomem *
+armada_pcie_wa_ioremap_caller(phys_addr_t phys_addr, size_t size,
+			      unsigned int mtype, void *caller)
+{
+	struct resource pcie_mem;
+
+	mvebu_mbus_get_pcie_mem_aperture(&pcie_mem);
+
+	if (pcie_mem.start <= phys_addr && (phys_addr + size) <= pcie_mem.end)
+		mtype = MT_UNCACHED;
+
+	return __arm_ioremap_caller(phys_addr, size, mtype, caller);
+}
+
 static void __init armada_375_380_coherency_init(struct device_node *np)
 {
+	struct device_node *cache_dn;
+
 	coherency_cpu_base = of_iomap(np, 0);
+	arch_ioremap_caller = armada_pcie_wa_ioremap_caller;
+
+	/*
+	 * Add the PL310 property "arm,io-coherent". This makes sure the
+	 * outer sync operation is not used, which allows to
+	 * workaround the system erratum that causes deadlocks when
+	 * doing PCIe in an SMP situation on Armada 375 and Armada
+	 * 38x.
+	 */
+	for_each_compatible_node(cache_dn, NULL, "arm,pl310-cache") {
+		struct property *p;
+
+		p = kzalloc(sizeof(*p), GFP_KERNEL);
+		p->name = kstrdup("arm,io-coherent", GFP_KERNEL);
+		of_add_property(cache_dn, p);
+	}
 }
 
 static int coherency_type(void)
-- 
1.9.3

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 3/3] ARM: mvebu: implement L2/PCIe deadlock workaround

[Thomas Petazzoni]

The Marvell Armada 375 and Armada 38x SOCs, which use the Cortex-A9
CPU core, the PL310 cache and the Marvell PCIe hardware block are
affected a L2/PCIe deadlock caused by a system erratum when hardware
I/O coherency is used.

This deadlock can be avoided by mapping the PCIe memory areas as
strongly-ordered (note: MT_UNCACHED is strongly-ordered), and by
removing the outer cache sync done in software. This is implemented in
this patch by:

 * Registering a custom arch_ioremap_caller function that allows to
   make sure PCI memory regions are mapped MT_UNCACHED.

 * Adding at runtime the 'arm,io-coherent' property to the PL310 cache
   controller. This cannot be done permanently in the DT, because the
   hardware I/O coherency can only be enabled when CONFIG_SMP is
   enabled, in the current kernel situation.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
 arch/arm/mach-mvebu/coherency.c | 39 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/arm/mach-mvebu/coherency.c b/arch/arm/mach-mvebu/coherency.c
index d5a975b..f6be9c6 100644
--- a/arch/arm/mach-mvebu/coherency.c
+++ b/arch/arm/mach-mvebu/coherency.c
@@ -31,6 +31,7 @@
 #include <linux/clk.h>
 #include <asm/smp_plat.h>
 #include <asm/cacheflush.h>
+#include <asm/mach/map.h>
 #include "armada-370-xp.h"
 #include "coherency.h"
 #include "mvebu-soc-id.h"
@@ -308,9 +309,47 @@ static void __init armada_370_coherency_init(struct device_node *np)
 	set_cpu_coherent();
 }
 
+/*
+ * This ioremap hook is used on Armada 375/38x to ensure that PCIe
+ * memory areas are mapped as MT_UNCACHED instead of MT_DEVICE. This
+ * is needed as a workaround for a deadlock issue between the PCIe
+ * interface and the cache controller.
+ */
+static void __iomem *
+armada_pcie_wa_ioremap_caller(phys_addr_t phys_addr, size_t size,
+			      unsigned int mtype, void *caller)
+{
+	struct resource pcie_mem;
+
+	mvebu_mbus_get_pcie_mem_aperture(&pcie_mem);
+
+	if (pcie_mem.start <= phys_addr && (phys_addr + size) <= pcie_mem.end)
+		mtype = MT_UNCACHED;
+
+	return __arm_ioremap_caller(phys_addr, size, mtype, caller);
+}
+
 static void __init armada_375_380_coherency_init(struct device_node *np)
 {
+	struct device_node *cache_dn;
+
 	coherency_cpu_base = of_iomap(np, 0);
+	arch_ioremap_caller = armada_pcie_wa_ioremap_caller;
+
+	/*
+	 * Add the PL310 property "arm,io-coherent". This makes sure the
+	 * outer sync operation is not used, which allows to
+	 * workaround the system erratum that causes deadlocks when
+	 * doing PCIe in an SMP situation on Armada 375 and Armada
+	 * 38x.
+	 */
+	for_each_compatible_node(cache_dn, NULL, "arm,pl310-cache") {
+		struct property *p;
+
+		p = kzalloc(sizeof(*p), GFP_KERNEL);
+		p->name = kstrdup("arm,io-coherent", GFP_KERNEL);
+		of_add_property(cache_dn, p);
+	}
 }
 
 static int coherency_type(void)
-- 
1.9.3

Re: [PATCHv4 2/3] ARM: mm: add support for HW coherent systems in PL310

[Rob Herring]

On Thu, May 15, 2014 at 9:59 AM, Thomas Petazzoni
<thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> When a PL310 cache is used on a system that provides hardware
> coherency, the outer cache sync operation is useless, and can be
> skipped. Moreover, on some systems, it is harmful as it causes
> deadlocks between the Marvell coherency mechanism, the Marvell PCIe
> controller and the Cortex-A9.
>
> To avoid this, this commit introduces a new Device Tree property
> 'arm,io-coherent' for the L2 cache controller node, valid only for the
> PL310 cache. It identifies the usage of the PL310 cache in an I/O
> coherent configuration. Internally, it makes the driver disable the
> outer cache sync operation.
>
> Note that technically speaking, a fully coherent system wouldn't
> require any of the other .outer_cache operations. However, in
> practice, when booting secondary CPUs, these are not yet coherent, and
> therefore a set of cache maintenance operations are necessary at this
> point. This explains why we keep the other .outer_cache operations and
> only ->sync is disabled.
>
> While in theory any write to a PL310 register could cause the
> deadlock, in practice, disabling ->sync is sufficient to workaround
> the deadlock, since the other cache maintenance operations are only
> used in very specific situations.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  Documentation/devicetree/bindings/arm/l2cc.txt |  3 +++
>  arch/arm/mm/cache-l2x0.c                       | 14 ++++++++++++++
>  2 files changed, 17 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt
> index b513cb8..af527ee 100644
> --- a/Documentation/devicetree/bindings/arm/l2cc.txt
> +++ b/Documentation/devicetree/bindings/arm/l2cc.txt
> @@ -40,6 +40,9 @@ Optional properties:
>  - arm,filter-ranges : <start length> Starting address and length of window to
>    filter. Addresses in the filter window are directed to the M1 port. Other
>    addresses will go to the M0 port.
> +- arm,io-coherent : indicates that the system is operating in an hardware
> +  I/O coherent mode. Valid only when the arm,pl310-cache compatible
> +  string is used.
>  - interrupts : 1 combined interrupt.
>  - cache-id-part: cache id part number to be used if it is not present
>    on hardware
> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
> index 7abde2ce..199a745 100644
> --- a/arch/arm/mm/cache-l2x0.c
> +++ b/arch/arm/mm/cache-l2x0.c
> @@ -1005,6 +1005,20 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
>
>         of_init = true;
>         memcpy(&outer_cache, &data->outer_cache, sizeof(outer_cache));
> +
> +       /*
> +        * outer sync operations are not needed when the system is I/O
> +        * coherent, and potentially harmful in certain situations
> +        * (PCIe/PL310 deadlock on Armada 375/38x due to hardware I/O
> +        * coherency). The other operations are kept because they are
> +        * infrequent (therefore do not cause the deadlock) and needed
> +        * for secondary CPU boot and other power management
> +        * activities.
> +        */
> +       if (of_device_is_compatible(np, "arm,pl310-cache") &&

Don't we have a pl310 specific init function this can be placed in
removing this check? We could probably remove this check here anyway
because we're never going to see this on an L210 or L220.

> +           of_property_read_bool(np, "arm,io-coherent"))
> +               outer_cache.sync = NULL;
> +
>         l2x0_init(l2x0_base, aux_val, aux_mask);
>
>         return 0;
> --
> 1.9.3
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 2/3] ARM: mm: add support for HW coherent systems in PL310

[Rob Herring]

On Thu, May 15, 2014 at 9:59 AM, Thomas Petazzoni
<thomas.petazzoni@free-electrons.com> wrote:
> When a PL310 cache is used on a system that provides hardware
> coherency, the outer cache sync operation is useless, and can be
> skipped. Moreover, on some systems, it is harmful as it causes
> deadlocks between the Marvell coherency mechanism, the Marvell PCIe
> controller and the Cortex-A9.
>
> To avoid this, this commit introduces a new Device Tree property
> 'arm,io-coherent' for the L2 cache controller node, valid only for the
> PL310 cache. It identifies the usage of the PL310 cache in an I/O
> coherent configuration. Internally, it makes the driver disable the
> outer cache sync operation.
>
> Note that technically speaking, a fully coherent system wouldn't
> require any of the other .outer_cache operations. However, in
> practice, when booting secondary CPUs, these are not yet coherent, and
> therefore a set of cache maintenance operations are necessary at this
> point. This explains why we keep the other .outer_cache operations and
> only ->sync is disabled.
>
> While in theory any write to a PL310 register could cause the
> deadlock, in practice, disabling ->sync is sufficient to workaround
> the deadlock, since the other cache maintenance operations are only
> used in very specific situations.
>
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  Documentation/devicetree/bindings/arm/l2cc.txt |  3 +++
>  arch/arm/mm/cache-l2x0.c                       | 14 ++++++++++++++
>  2 files changed, 17 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/arm/l2cc.txt b/Documentation/devicetree/bindings/arm/l2cc.txt
> index b513cb8..af527ee 100644
> --- a/Documentation/devicetree/bindings/arm/l2cc.txt
> +++ b/Documentation/devicetree/bindings/arm/l2cc.txt
> @@ -40,6 +40,9 @@ Optional properties:
>  - arm,filter-ranges : <start length> Starting address and length of window to
>    filter. Addresses in the filter window are directed to the M1 port. Other
>    addresses will go to the M0 port.
> +- arm,io-coherent : indicates that the system is operating in an hardware
> +  I/O coherent mode. Valid only when the arm,pl310-cache compatible
> +  string is used.
>  - interrupts : 1 combined interrupt.
>  - cache-id-part: cache id part number to be used if it is not present
>    on hardware
> diff --git a/arch/arm/mm/cache-l2x0.c b/arch/arm/mm/cache-l2x0.c
> index 7abde2ce..199a745 100644
> --- a/arch/arm/mm/cache-l2x0.c
> +++ b/arch/arm/mm/cache-l2x0.c
> @@ -1005,6 +1005,20 @@ int __init l2x0_of_init(u32 aux_val, u32 aux_mask)
>
>         of_init = true;
>         memcpy(&outer_cache, &data->outer_cache, sizeof(outer_cache));
> +
> +       /*
> +        * outer sync operations are not needed when the system is I/O
> +        * coherent, and potentially harmful in certain situations
> +        * (PCIe/PL310 deadlock on Armada 375/38x due to hardware I/O
> +        * coherency). The other operations are kept because they are
> +        * infrequent (therefore do not cause the deadlock) and needed
> +        * for secondary CPU boot and other power management
> +        * activities.
> +        */
> +       if (of_device_is_compatible(np, "arm,pl310-cache") &&

Don't we have a pl310 specific init function this can be placed in
removing this check? We could probably remove this check here anyway
because we're never going to see this on an L210 or L220.

> +           of_property_read_bool(np, "arm,io-coherent"))
> +               outer_cache.sync = NULL;
> +
>         l2x0_init(l2x0_base, aux_val, aux_mask);
>
>         return 0;
> --
> 1.9.3
>

Re: [PATCHv4 3/3] ARM: mvebu: implement L2/PCIe deadlock workaround

[Jason Cooper]

On Thu, May 15, 2014 at 04:59:34PM +0200, Thomas Petazzoni wrote:
> The Marvell Armada 375 and Armada 38x SOCs, which use the Cortex-A9
> CPU core, the PL310 cache and the Marvell PCIe hardware block are
> affected a L2/PCIe deadlock caused by a system erratum when hardware
> I/O coherency is used.
> 
> This deadlock can be avoided by mapping the PCIe memory areas as
> strongly-ordered (note: MT_UNCACHED is strongly-ordered), and by
> removing the outer cache sync done in software. This is implemented in
> this patch by:
> 
>  * Registering a custom arch_ioremap_caller function that allows to
>    make sure PCI memory regions are mapped MT_UNCACHED.
> 
>  * Adding at runtime the 'arm,io-coherent' property to the PL310 cache
>    controller. This cannot be done permanently in the DT, because the
>    hardware I/O coherency can only be enabled when CONFIG_SMP is
>    enabled, in the current kernel situation.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  arch/arm/mach-mvebu/coherency.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)

Applied to mvebu/soc.

thx,

Jason.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 3/3] ARM: mvebu: implement L2/PCIe deadlock workaround

[Jason Cooper]

On Thu, May 15, 2014 at 04:59:34PM +0200, Thomas Petazzoni wrote:
> The Marvell Armada 375 and Armada 38x SOCs, which use the Cortex-A9
> CPU core, the PL310 cache and the Marvell PCIe hardware block are
> affected a L2/PCIe deadlock caused by a system erratum when hardware
> I/O coherency is used.
> 
> This deadlock can be avoided by mapping the PCIe memory areas as
> strongly-ordered (note: MT_UNCACHED is strongly-ordered), and by
> removing the outer cache sync done in software. This is implemented in
> this patch by:
> 
>  * Registering a custom arch_ioremap_caller function that allows to
>    make sure PCI memory regions are mapped MT_UNCACHED.
> 
>  * Adding at runtime the 'arm,io-coherent' property to the PL310 cache
>    controller. This cannot be done permanently in the DT, because the
>    hardware I/O coherency can only be enabled when CONFIG_SMP is
>    enabled, in the current kernel situation.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  arch/arm/mach-mvebu/coherency.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)

Applied to mvebu/soc.

thx,

Jason.

Re: [PATCHv4 3/3] ARM: mvebu: implement L2/PCIe deadlock workaround

[Thomas Petazzoni]

Dear Jason Cooper,

On Fri, 16 May 2014 02:08:51 -0400, Jason Cooper wrote:
> On Thu, May 15, 2014 at 04:59:34PM +0200, Thomas Petazzoni wrote:
> > The Marvell Armada 375 and Armada 38x SOCs, which use the Cortex-A9
> > CPU core, the PL310 cache and the Marvell PCIe hardware block are
> > affected a L2/PCIe deadlock caused by a system erratum when hardware
> > I/O coherency is used.
> > 
> > This deadlock can be avoided by mapping the PCIe memory areas as
> > strongly-ordered (note: MT_UNCACHED is strongly-ordered), and by
> > removing the outer cache sync done in software. This is implemented in
> > this patch by:
> > 
> >  * Registering a custom arch_ioremap_caller function that allows to
> >    make sure PCI memory regions are mapped MT_UNCACHED.
> > 
> >  * Adding at runtime the 'arm,io-coherent' property to the PL310 cache
> >    controller. This cannot be done permanently in the DT, because the
> >    hardware I/O coherency can only be enabled when CONFIG_SMP is
> >    enabled, in the current kernel situation.
> > 
> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > ---
> >  arch/arm/mach-mvebu/coherency.c | 39 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> 
> Applied to mvebu/soc.

Hum, well, I believe it's OK, but notice that I will very likely have
to do a followup patch, because the solution of making all PCI I/O
mappings use the MT_UNCACHED memory type, suggested by Arnd, has not
been accepted by Will Deacon. So very likely I will have to change
again the mach-mvebu/coherency.c code to call a function that makes PCI
I/O mappings MT_UNCACHED specifically for the mach-mvebu platform. But
that can indeed be a followup patch.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 3/3] ARM: mvebu: implement L2/PCIe deadlock workaround

[Thomas Petazzoni]

Dear Jason Cooper,

On Fri, 16 May 2014 02:08:51 -0400, Jason Cooper wrote:
> On Thu, May 15, 2014 at 04:59:34PM +0200, Thomas Petazzoni wrote:
> > The Marvell Armada 375 and Armada 38x SOCs, which use the Cortex-A9
> > CPU core, the PL310 cache and the Marvell PCIe hardware block are
> > affected a L2/PCIe deadlock caused by a system erratum when hardware
> > I/O coherency is used.
> > 
> > This deadlock can be avoided by mapping the PCIe memory areas as
> > strongly-ordered (note: MT_UNCACHED is strongly-ordered), and by
> > removing the outer cache sync done in software. This is implemented in
> > this patch by:
> > 
> >  * Registering a custom arch_ioremap_caller function that allows to
> >    make sure PCI memory regions are mapped MT_UNCACHED.
> > 
> >  * Adding at runtime the 'arm,io-coherent' property to the PL310 cache
> >    controller. This cannot be done permanently in the DT, because the
> >    hardware I/O coherency can only be enabled when CONFIG_SMP is
> >    enabled, in the current kernel situation.
> > 
> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > ---
> >  arch/arm/mach-mvebu/coherency.c | 39 +++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 39 insertions(+)
> 
> Applied to mvebu/soc.

Hum, well, I believe it's OK, but notice that I will very likely have
to do a followup patch, because the solution of making all PCI I/O
mappings use the MT_UNCACHED memory type, suggested by Arnd, has not
been accepted by Will Deacon. So very likely I will have to change
again the mach-mvebu/coherency.c code to call a function that makes PCI
I/O mappings MT_UNCACHED specifically for the mach-mvebu platform. But
that can indeed be a followup patch.

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCHv4 3/3] ARM: mvebu: implement L2/PCIe deadlock workaround

[Jason Cooper]

On Fri, May 16, 2014 at 09:07:27AM +0200, Thomas Petazzoni wrote:
> Dear Jason Cooper,
> 
> On Fri, 16 May 2014 02:08:51 -0400, Jason Cooper wrote:
> > On Thu, May 15, 2014 at 04:59:34PM +0200, Thomas Petazzoni wrote:
> > > The Marvell Armada 375 and Armada 38x SOCs, which use the Cortex-A9
> > > CPU core, the PL310 cache and the Marvell PCIe hardware block are
> > > affected a L2/PCIe deadlock caused by a system erratum when hardware
> > > I/O coherency is used.
> > > 
> > > This deadlock can be avoided by mapping the PCIe memory areas as
> > > strongly-ordered (note: MT_UNCACHED is strongly-ordered), and by
> > > removing the outer cache sync done in software. This is implemented in
> > > this patch by:
> > > 
> > >  * Registering a custom arch_ioremap_caller function that allows to
> > >    make sure PCI memory regions are mapped MT_UNCACHED.
> > > 
> > >  * Adding at runtime the 'arm,io-coherent' property to the PL310 cache
> > >    controller. This cannot be done permanently in the DT, because the
> > >    hardware I/O coherency can only be enabled when CONFIG_SMP is
> > >    enabled, in the current kernel situation.
> > > 
> > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > > ---
> > >  arch/arm/mach-mvebu/coherency.c | 39 +++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 39 insertions(+)
> > 
> > Applied to mvebu/soc.
> 
> Hum, well, I believe it's OK, but notice that I will very likely have
> to do a followup patch, because the solution of making all PCI I/O
> mappings use the MT_UNCACHED memory type, suggested by Arnd, has not
> been accepted by Will Deacon. So very likely I will have to change
> again the mach-mvebu/coherency.c code to call a function that makes PCI
> I/O mappings MT_UNCACHED specifically for the mach-mvebu platform. But
> that can indeed be a followup patch.

Ahh, my mistake.  It was late.  I saw the arm,io-coherent was settled
and mistakenly thought the other proposed changes wouldn't affect this
patch.

I could easily drop it if you like.  It's currently the tip of mvebu/soc
and I haven't done a signed tag for it yet.

thx,

Jason.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 3/3] ARM: mvebu: implement L2/PCIe deadlock workaround

[Jason Cooper]

On Fri, May 16, 2014 at 09:07:27AM +0200, Thomas Petazzoni wrote:
> Dear Jason Cooper,
> 
> On Fri, 16 May 2014 02:08:51 -0400, Jason Cooper wrote:
> > On Thu, May 15, 2014 at 04:59:34PM +0200, Thomas Petazzoni wrote:
> > > The Marvell Armada 375 and Armada 38x SOCs, which use the Cortex-A9
> > > CPU core, the PL310 cache and the Marvell PCIe hardware block are
> > > affected a L2/PCIe deadlock caused by a system erratum when hardware
> > > I/O coherency is used.
> > > 
> > > This deadlock can be avoided by mapping the PCIe memory areas as
> > > strongly-ordered (note: MT_UNCACHED is strongly-ordered), and by
> > > removing the outer cache sync done in software. This is implemented in
> > > this patch by:
> > > 
> > >  * Registering a custom arch_ioremap_caller function that allows to
> > >    make sure PCI memory regions are mapped MT_UNCACHED.
> > > 
> > >  * Adding at runtime the 'arm,io-coherent' property to the PL310 cache
> > >    controller. This cannot be done permanently in the DT, because the
> > >    hardware I/O coherency can only be enabled when CONFIG_SMP is
> > >    enabled, in the current kernel situation.
> > > 
> > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > > ---
> > >  arch/arm/mach-mvebu/coherency.c | 39 +++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 39 insertions(+)
> > 
> > Applied to mvebu/soc.
> 
> Hum, well, I believe it's OK, but notice that I will very likely have
> to do a followup patch, because the solution of making all PCI I/O
> mappings use the MT_UNCACHED memory type, suggested by Arnd, has not
> been accepted by Will Deacon. So very likely I will have to change
> again the mach-mvebu/coherency.c code to call a function that makes PCI
> I/O mappings MT_UNCACHED specifically for the mach-mvebu platform. But
> that can indeed be a followup patch.

Ahh, my mistake.  It was late.  I saw the arm,io-coherent was settled
and mistakenly thought the other proposed changes wouldn't affect this
patch.

I could easily drop it if you like.  It's currently the tip of mvebu/soc
and I haven't done a signed tag for it yet.

thx,

Jason.

Re: [PATCHv4 3/3] ARM: mvebu: implement L2/PCIe deadlock workaround

[Thomas Petazzoni]

Dear Jason Cooper,

On Fri, 16 May 2014 08:58:47 -0400, Jason Cooper wrote:

> > Hum, well, I believe it's OK, but notice that I will very likely have
> > to do a followup patch, because the solution of making all PCI I/O
> > mappings use the MT_UNCACHED memory type, suggested by Arnd, has not
> > been accepted by Will Deacon. So very likely I will have to change
> > again the mach-mvebu/coherency.c code to call a function that makes PCI
> > I/O mappings MT_UNCACHED specifically for the mach-mvebu platform. But
> > that can indeed be a followup patch.
> 
> Ahh, my mistake.  It was late.  I saw the arm,io-coherent was settled
> and mistakenly thought the other proposed changes wouldn't affect this
> patch.
> 
> I could easily drop it if you like.  It's currently the tip of mvebu/soc
> and I haven't done a signed tag for it yet.

I think it's OK, you can keep it: the PCI I/O mapping thing will come
as an addition.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 3/3] ARM: mvebu: implement L2/PCIe deadlock workaround

[Thomas Petazzoni]

Dear Jason Cooper,

On Fri, 16 May 2014 08:58:47 -0400, Jason Cooper wrote:

> > Hum, well, I believe it's OK, but notice that I will very likely have
> > to do a followup patch, because the solution of making all PCI I/O
> > mappings use the MT_UNCACHED memory type, suggested by Arnd, has not
> > been accepted by Will Deacon. So very likely I will have to change
> > again the mach-mvebu/coherency.c code to call a function that makes PCI
> > I/O mappings MT_UNCACHED specifically for the mach-mvebu platform. But
> > that can indeed be a followup patch.
> 
> Ahh, my mistake.  It was late.  I saw the arm,io-coherent was settled
> and mistakenly thought the other proposed changes wouldn't affect this
> patch.
> 
> I could easily drop it if you like.  It's currently the tip of mvebu/soc
> and I haven't done a signed tag for it yet.

I think it's OK, you can keep it: the PCI I/O mapping thing will come
as an addition.

Best regards,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

Re: [PATCHv4 3/3] ARM: mvebu: implement L2/PCIe deadlock workaround

[Jason Cooper]

On Thu, May 15, 2014 at 04:59:34PM +0200, Thomas Petazzoni wrote:
> The Marvell Armada 375 and Armada 38x SOCs, which use the Cortex-A9
> CPU core, the PL310 cache and the Marvell PCIe hardware block are
> affected a L2/PCIe deadlock caused by a system erratum when hardware
> I/O coherency is used.
> 
> This deadlock can be avoided by mapping the PCIe memory areas as
> strongly-ordered (note: MT_UNCACHED is strongly-ordered), and by
> removing the outer cache sync done in software. This is implemented in
> this patch by:
> 
>  * Registering a custom arch_ioremap_caller function that allows to
>    make sure PCI memory regions are mapped MT_UNCACHED.
> 
>  * Adding at runtime the 'arm,io-coherent' property to the PL310 cache
>    controller. This cannot be done permanently in the DT, because the
>    hardware I/O coherency can only be enabled when CONFIG_SMP is
>    enabled, in the current kernel situation.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> ---
>  arch/arm/mach-mvebu/coherency.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)

Applied to mvebu/soc.

thx,

Jason.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 3/3] ARM: mvebu: implement L2/PCIe deadlock workaround

[Jason Cooper]

On Thu, May 15, 2014 at 04:59:34PM +0200, Thomas Petazzoni wrote:
> The Marvell Armada 375 and Armada 38x SOCs, which use the Cortex-A9
> CPU core, the PL310 cache and the Marvell PCIe hardware block are
> affected a L2/PCIe deadlock caused by a system erratum when hardware
> I/O coherency is used.
> 
> This deadlock can be avoided by mapping the PCIe memory areas as
> strongly-ordered (note: MT_UNCACHED is strongly-ordered), and by
> removing the outer cache sync done in software. This is implemented in
> this patch by:
> 
>  * Registering a custom arch_ioremap_caller function that allows to
>    make sure PCI memory regions are mapped MT_UNCACHED.
> 
>  * Adding at runtime the 'arm,io-coherent' property to the PL310 cache
>    controller. This cannot be done permanently in the DT, because the
>    hardware I/O coherency can only be enabled when CONFIG_SMP is
>    enabled, in the current kernel situation.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> ---
>  arch/arm/mach-mvebu/coherency.c | 39 +++++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)

Applied to mvebu/soc.

thx,

Jason.

Re: [PATCHv4 3/3] ARM: mvebu: implement L2/PCIe deadlock workaround

[Jason Cooper]

On Fri, May 16, 2014 at 09:07:27AM +0200, Thomas Petazzoni wrote:
> Dear Jason Cooper,
> 
> On Fri, 16 May 2014 02:08:51 -0400, Jason Cooper wrote:
> > On Thu, May 15, 2014 at 04:59:34PM +0200, Thomas Petazzoni wrote:
> > > The Marvell Armada 375 and Armada 38x SOCs, which use the Cortex-A9
> > > CPU core, the PL310 cache and the Marvell PCIe hardware block are
> > > affected a L2/PCIe deadlock caused by a system erratum when hardware
> > > I/O coherency is used.
> > > 
> > > This deadlock can be avoided by mapping the PCIe memory areas as
> > > strongly-ordered (note: MT_UNCACHED is strongly-ordered), and by
> > > removing the outer cache sync done in software. This is implemented in
> > > this patch by:
> > > 
> > >  * Registering a custom arch_ioremap_caller function that allows to
> > >    make sure PCI memory regions are mapped MT_UNCACHED.
> > > 
> > >  * Adding at runtime the 'arm,io-coherent' property to the PL310 cache
> > >    controller. This cannot be done permanently in the DT, because the
> > >    hardware I/O coherency can only be enabled when CONFIG_SMP is
> > >    enabled, in the current kernel situation.
> > > 
> > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> > > ---
> > >  arch/arm/mach-mvebu/coherency.c | 39 +++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 39 insertions(+)
> > 
> > Applied to mvebu/soc.
> 
> Hum, well, I believe it's OK, but notice that I will very likely have
> to do a followup patch, because the solution of making all PCI I/O
> mappings use the MT_UNCACHED memory type, suggested by Arnd, has not
> been accepted by Will Deacon. So very likely I will have to change
> again the mach-mvebu/coherency.c code to call a function that makes PCI
> I/O mappings MT_UNCACHED specifically for the mach-mvebu platform. But
> that can indeed be a followup patch.

Ahh, my mistake.  It was late.  I saw the arm,io-coherent was settled
and mistakenly thought the other proposed changes wouldn't affect this
patch.

I could easily drop it if you like.  It's currently the tip of mvebu/soc
and I haven't done a signed tag for it yet.

thx,

Jason.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv4 3/3] ARM: mvebu: implement L2/PCIe deadlock workaround

[Jason Cooper]

On Fri, May 16, 2014 at 09:07:27AM +0200, Thomas Petazzoni wrote:
> Dear Jason Cooper,
> 
> On Fri, 16 May 2014 02:08:51 -0400, Jason Cooper wrote:
> > On Thu, May 15, 2014 at 04:59:34PM +0200, Thomas Petazzoni wrote:
> > > The Marvell Armada 375 and Armada 38x SOCs, which use the Cortex-A9
> > > CPU core, the PL310 cache and the Marvell PCIe hardware block are
> > > affected a L2/PCIe deadlock caused by a system erratum when hardware
> > > I/O coherency is used.
> > > 
> > > This deadlock can be avoided by mapping the PCIe memory areas as
> > > strongly-ordered (note: MT_UNCACHED is strongly-ordered), and by
> > > removing the outer cache sync done in software. This is implemented in
> > > this patch by:
> > > 
> > >  * Registering a custom arch_ioremap_caller function that allows to
> > >    make sure PCI memory regions are mapped MT_UNCACHED.
> > > 
> > >  * Adding at runtime the 'arm,io-coherent' property to the PL310 cache
> > >    controller. This cannot be done permanently in the DT, because the
> > >    hardware I/O coherency can only be enabled when CONFIG_SMP is
> > >    enabled, in the current kernel situation.
> > > 
> > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> > > ---
> > >  arch/arm/mach-mvebu/coherency.c | 39 +++++++++++++++++++++++++++++++++++++++
> > >  1 file changed, 39 insertions(+)
> > 
> > Applied to mvebu/soc.
> 
> Hum, well, I believe it's OK, but notice that I will very likely have
> to do a followup patch, because the solution of making all PCI I/O
> mappings use the MT_UNCACHED memory type, suggested by Arnd, has not
> been accepted by Will Deacon. So very likely I will have to change
> again the mach-mvebu/coherency.c code to call a function that makes PCI
> I/O mappings MT_UNCACHED specifically for the mach-mvebu platform. But
> that can indeed be a followup patch.

Ahh, my mistake.  It was late.  I saw the arm,io-coherent was settled
and mistakenly thought the other proposed changes wouldn't affect this
patch.

I could easily drop it if you like.  It's currently the tip of mvebu/soc
and I haven't done a signed tag for it yet.

thx,

Jason.

Re: [PATCHv4 2/3] ARM: mm: add support for HW coherent systems in PL310

[Thomas Petazzoni]

Dear Rob Herring,

On Thu, 15 May 2014 14:08:39 -0500, Rob Herring wrote:

> >         of_init = true;
> >         memcpy(&outer_cache, &data->outer_cache, sizeof(outer_cache));
> > +
> > +       /*
> > +        * outer sync operations are not needed when the system is I/O
> > +        * coherent, and potentially harmful in certain situations
> > +        * (PCIe/PL310 deadlock on Armada 375/38x due to hardware I/O
> > +        * coherency). The other operations are kept because they are
> > +        * infrequent (therefore do not cause the deadlock) and needed
> > +        * for secondary CPU boot and other power management
> > +        * activities.
> > +        */
> > +       if (of_device_is_compatible(np, "arm,pl310-cache") &&
> 
> Don't we have a pl310 specific init function this can be placed in
> removing this check? We could probably remove this check here anyway
> because we're never going to see this on an L210 or L220.

We have a pl310 specific init function called pl310_of_init(). However
it gets called *before* the global outer_cache structure gets memcpy()
from the specific L2 cache type chosen by the compatible string.
Therefore, it cannot null-ify the .sync operation. It is also unable to
do it directly in the pl310_data structure, because 1/ this structure
is const and 2/ I believe it would be ugly.

Therefore, we don't have much other choice than to do that where I'm
doing at the moment. I'll respin the patch series to remove the
"arm,pl310-cache" check, as you suggested.

Thanks for the review,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com