| Message ID | E1Z07tA-0001TV-As@rmk-PC.arm.linux.org.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jun 03, 2015 at 01:35:20PM +0100, Russell King wrote: > Add an extension to the heavy barrier code to allow a SoC specific > memory barrier function to be provided. This is needed for platforms > where the interconnect has weak ordering, and thus needs assistance > to ensure that memory writes are properly visible in the correct order > to other parts of the system. Do you have an example of where this is needed? Were they previously handled by hijacking outer_cache.sync?
On Wed, Jun 03, 2015 at 02:15:25PM +0100, Catalin Marinas wrote: > On Wed, Jun 03, 2015 at 01:35:20PM +0100, Russell King wrote: > > Add an extension to the heavy barrier code to allow a SoC specific > > memory barrier function to be provided. This is needed for platforms > > where the interconnect has weak ordering, and thus needs assistance > > to ensure that memory writes are properly visible in the correct order > > to other parts of the system. > > Do you have an example of where this is needed? Were they previously > handled by hijacking outer_cache.sync? Look for omap_bus_sync() - you need to go back through the kernel history as it has been half-heartedly removed (OMAP's mach/barriers.h still exists but it isn't used by anything, and omap_bus_sync() is now just an assembly stub.) Basically, OMAP used to set CONFIG_ARCH_HAS_BARRIERS, which caused OMAPs mach/barriers.h to be picked up. This contained: #define rmb() dsb() #define wmb() do { dsb(); outer_sync(); omap_bus_sync(); } while (0) #define mb() wmb() and omap_bus_sync() was implemented arch/arm/mach-omap2/omap4-common.c as: void __iomem *dram_sync, *sram_sync; void omap_bus_sync(void) { if (dram_sync && sram_sync) { writel_relaxed(readl_relaxed(dram_sync), dram_sync); writel_relaxed(readl_relaxed(sram_sync), sram_sync); isb(); } } EXPORT_SYMBOL(omap_bus_sync); with the initialisation: dram_io_desc[0].virtual = OMAP4_DRAM_BARRIER_VA; dram_io_desc[0].pfn = __phys_to_pfn(paddr); dram_io_desc[0].length = size; dram_io_desc[0].type = MT_MEMORY_RW_SO; iotable_init(dram_io_desc, ARRAY_SIZE(dram_io_desc)); dram_sync = (void __iomem *) dram_io_desc[0].virtual; and: sram_sync = (void *)gen_pool_alloc(sram_pool, PAGE_SIZE); which has the effect of issuing a strongly ordered access out to memory, which cause the OMAP interconnects to be flushed. The message I'm hearing from TI is that the removal of this from mainline is a mistake (it sounds to me like it's caused a regression), and it needs to be restored, otherwise data accesses via various paths to DRAM can end up being seen out of order.
* Russell King - ARM Linux <linux@arm.linux.org.uk> [150603 06:44]: > On Wed, Jun 03, 2015 at 02:15:25PM +0100, Catalin Marinas wrote: > > On Wed, Jun 03, 2015 at 01:35:20PM +0100, Russell King wrote: > > > Add an extension to the heavy barrier code to allow a SoC specific > > > memory barrier function to be provided. This is needed for platforms > > > where the interconnect has weak ordering, and thus needs assistance > > > to ensure that memory writes are properly visible in the correct order > > > to other parts of the system. > > > > Do you have an example of where this is needed? Were they previously > > handled by hijacking outer_cache.sync? > > Look for omap_bus_sync() - you need to go back through the kernel history > as it has been half-heartedly removed (OMAP's mach/barriers.h still exists > but it isn't used by anything, and omap_bus_sync() is now just an assembly > stub.) With multiarch support, the code for errata i688 became unselectable. That's mostly commit 137d105d50f6 ("ARM: OMAP4: Fix errata i688 with MPU interconnect barriers."). > Basically, OMAP used to set CONFIG_ARCH_HAS_BARRIERS, which caused OMAPs > mach/barriers.h to be picked up. This contained: > > #define rmb() dsb() > #define wmb() do { dsb(); outer_sync(); omap_bus_sync(); } while (0) > #define mb() wmb() > > and omap_bus_sync() was implemented arch/arm/mach-omap2/omap4-common.c > as: > > void __iomem *dram_sync, *sram_sync; > > void omap_bus_sync(void) > { > if (dram_sync && sram_sync) { > writel_relaxed(readl_relaxed(dram_sync), dram_sync); > writel_relaxed(readl_relaxed(sram_sync), sram_sync); > isb(); > } > } > EXPORT_SYMBOL(omap_bus_sync); > > with the initialisation: > > dram_io_desc[0].virtual = OMAP4_DRAM_BARRIER_VA; > dram_io_desc[0].pfn = __phys_to_pfn(paddr); > dram_io_desc[0].length = size; > dram_io_desc[0].type = MT_MEMORY_RW_SO; > iotable_init(dram_io_desc, ARRAY_SIZE(dram_io_desc)); > dram_sync = (void __iomem *) dram_io_desc[0].virtual; > > and: > > sram_sync = (void *)gen_pool_alloc(sram_pool, PAGE_SIZE); > > > which has the effect of issuing a strongly ordered access out to memory, > which cause the OMAP interconnects to be flushed. > > The message I'm hearing from TI is that the removal of this from mainline > is a mistake (it sounds to me like it's caused a regression), and it needs > to be restored, otherwise data accesses via various paths to DRAM can end > up being seen out of order. Yes we need a multiarch safe way to do the interconnect barriers, it's not limited to just erratum i688 on omaps. Regards, Tony
diff --git a/arch/arm/include/asm/barrier.h b/arch/arm/include/asm/barrier.h index 646539a903a2..1676007d9549 100644 --- a/arch/arm/include/asm/barrier.h +++ b/arch/arm/include/asm/barrier.h @@ -37,6 +37,7 @@ #endif #ifdef CONFIG_ARM_HEAVY_MB +extern void (*soc_mb)(void); extern void arm_heavy_mb(void); #define __arm_heavy_mb(x...) do { dsb(x); arm_heavy_mb(); } while (0) #else diff --git a/arch/arm/mm/flush.c b/arch/arm/mm/flush.c index ce6c2960d5ac..1ec8e7590fc6 100644 --- a/arch/arm/mm/flush.c +++ b/arch/arm/mm/flush.c @@ -22,12 +22,16 @@ #include "mm.h" #ifdef CONFIG_ARM_HEAVY_MB +void (*soc_mb)(void); + void arm_heavy_mb(void) { #ifdef CONFIG_OUTER_CACHE_SYNC if (outer_cache.sync) outer_cache.sync(); #endif + if (soc_mb) + soc_mb(); } EXPORT_SYMBOL(arm_heavy_mb); #endif
Add an extension to the heavy barrier code to allow a SoC specific memory barrier function to be provided. This is needed for platforms where the interconnect has weak ordering, and thus needs assistance to ensure that memory writes are properly visible in the correct order to other parts of the system. Signed-off-by: Russell King <rmk+kernel@arm.linux.org.uk> --- arch/arm/include/asm/barrier.h | 1 + arch/arm/mm/flush.c | 4 ++++ 2 files changed, 5 insertions(+)