* [PATCH] ARM: cacheflush: Fix exynos build breakage on ARMv6 by using macros for ISB/DSB @ 2014-09-16 12:52 ` Krzysztof Kozlowski 0 siblings, 0 replies; 8+ messages in thread From: Krzysztof Kozlowski @ 2014-09-16 12:52 UTC (permalink / raw) To: linux-arm-kernel This fixes build breakage of platsmp.c if ARMv6 was chosen for compile time options (e.g. by building allmodconfig): $ make allmodconfig $ make CC arch/arm/mach-exynos/platsmp.o /tmp/ccdQM0Eg.s: Assembler messages: /tmp/ccdQM0Eg.s:432: Error: selected processor does not support ARM mode `isb ' /tmp/ccdQM0Eg.s:437: Error: selected processor does not support ARM mode `isb ' /tmp/ccdQM0Eg.s:438: Error: selected processor does not support ARM mode `dsb ' make[1]: *** [arch/arm/mach-exynos/platsmp.o] Error 1 The error was introduced in commit "ARM: EXYNOS: Move code from hotplug.c to platsmp.c". Previously code using v7_exit_coherency_flush() macro was built with '-march=armv7-a' flag but this flag dissapeared during the movement. Use isb() and dsb() macros in v7_exit_coherency_flush() so the proper code will be generated for ARMv6. Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Reported-by: Mark Brown <broonie@kernel.org> Link: http://www.spinics.net/lists/linux-samsung-soc/msg36790.html --- arch/arm/include/asm/cacheflush.h | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h index 79ecb4f34ffb..b74eeec971c0 100644 --- a/arch/arm/include/asm/cacheflush.h +++ b/arch/arm/include/asm/cacheflush.h @@ -464,22 +464,27 @@ static inline void __sync_cache_range_r(volatile void *p, size_t size) * CONFIG_FRAME_POINTER=y. ip is saved as well if ever r12-clobbering * trampoline are inserted by the linker and to keep sp 64-bit aligned. */ -#define v7_exit_coherency_flush(level) \ +#define v7_exit_coherency_flush(level) do { \ asm volatile( \ "stmfd sp!, {fp, ip} \n\t" \ "mrc p15, 0, r0, c1, c0, 0 @ get SCTLR \n\t" \ "bic r0, r0, #"__stringify(CR_C)" \n\t" \ "mcr p15, 0, r0, c1, c0, 0 @ set SCTLR \n\t" \ - "isb \n\t" \ + : : : "r0","r1","r2","r3","r4","r5","r6","r7", \ + "r9","r10","lr","memory" ); \ + isb(); \ + asm volatile( \ "bl v7_flush_dcache_"__stringify(level)" \n\t" \ "mrc p15, 0, r0, c1, c0, 1 @ get ACTLR \n\t" \ "bic r0, r0, #(1 << 6) @ disable local coherency \n\t" \ "mcr p15, 0, r0, c1, c0, 1 @ set ACTLR \n\t" \ - "isb \n\t" \ - "dsb \n\t" \ - "ldmfd sp!, {fp, ip}" \ : : : "r0","r1","r2","r3","r4","r5","r6","r7", \ - "r9","r10","lr","memory" ) + "r9","r10","lr","memory" ); \ + isb(); \ + dsb(); \ + asm volatile( \ + "ldmfd sp!, {fp, ip}" ); \ + } while (0) int set_memory_ro(unsigned long addr, int numpages); int set_memory_rw(unsigned long addr, int numpages); -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] ARM: cacheflush: Fix exynos build breakage on ARMv6 by using macros for ISB/DSB @ 2014-09-16 12:52 ` Krzysztof Kozlowski 0 siblings, 0 replies; 8+ messages in thread From: Krzysztof Kozlowski @ 2014-09-16 12:52 UTC (permalink / raw) To: Russell King, Will Deacon, David A. Long, Mark Rutland, Vinayak Kale, Laura Abbott, Krzysztof Kozlowski, Nicolas Pitre, linux-arm-kernel, linux-kernel Cc: Kyungmin Park, Marek Szyprowski, Bartlomiej Zolnierkiewicz, Tomasz Figa, Kukjin Kim, Mark Brown This fixes build breakage of platsmp.c if ARMv6 was chosen for compile time options (e.g. by building allmodconfig): $ make allmodconfig $ make CC arch/arm/mach-exynos/platsmp.o /tmp/ccdQM0Eg.s: Assembler messages: /tmp/ccdQM0Eg.s:432: Error: selected processor does not support ARM mode `isb ' /tmp/ccdQM0Eg.s:437: Error: selected processor does not support ARM mode `isb ' /tmp/ccdQM0Eg.s:438: Error: selected processor does not support ARM mode `dsb ' make[1]: *** [arch/arm/mach-exynos/platsmp.o] Error 1 The error was introduced in commit "ARM: EXYNOS: Move code from hotplug.c to platsmp.c". Previously code using v7_exit_coherency_flush() macro was built with '-march=armv7-a' flag but this flag dissapeared during the movement. Use isb() and dsb() macros in v7_exit_coherency_flush() so the proper code will be generated for ARMv6. Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com> Reported-by: Mark Brown <broonie@kernel.org> Link: http://www.spinics.net/lists/linux-samsung-soc/msg36790.html --- arch/arm/include/asm/cacheflush.h | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h index 79ecb4f34ffb..b74eeec971c0 100644 --- a/arch/arm/include/asm/cacheflush.h +++ b/arch/arm/include/asm/cacheflush.h @@ -464,22 +464,27 @@ static inline void __sync_cache_range_r(volatile void *p, size_t size) * CONFIG_FRAME_POINTER=y. ip is saved as well if ever r12-clobbering * trampoline are inserted by the linker and to keep sp 64-bit aligned. */ -#define v7_exit_coherency_flush(level) \ +#define v7_exit_coherency_flush(level) do { \ asm volatile( \ "stmfd sp!, {fp, ip} \n\t" \ "mrc p15, 0, r0, c1, c0, 0 @ get SCTLR \n\t" \ "bic r0, r0, #"__stringify(CR_C)" \n\t" \ "mcr p15, 0, r0, c1, c0, 0 @ set SCTLR \n\t" \ - "isb \n\t" \ + : : : "r0","r1","r2","r3","r4","r5","r6","r7", \ + "r9","r10","lr","memory" ); \ + isb(); \ + asm volatile( \ "bl v7_flush_dcache_"__stringify(level)" \n\t" \ "mrc p15, 0, r0, c1, c0, 1 @ get ACTLR \n\t" \ "bic r0, r0, #(1 << 6) @ disable local coherency \n\t" \ "mcr p15, 0, r0, c1, c0, 1 @ set ACTLR \n\t" \ - "isb \n\t" \ - "dsb \n\t" \ - "ldmfd sp!, {fp, ip}" \ : : : "r0","r1","r2","r3","r4","r5","r6","r7", \ - "r9","r10","lr","memory" ) + "r9","r10","lr","memory" ); \ + isb(); \ + dsb(); \ + asm volatile( \ + "ldmfd sp!, {fp, ip}" ); \ + } while (0) int set_memory_ro(unsigned long addr, int numpages); int set_memory_rw(unsigned long addr, int numpages); -- 1.9.1 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH] ARM: cacheflush: Fix exynos build breakage on ARMv6 by using macros for ISB/DSB 2014-09-16 12:52 ` Krzysztof Kozlowski @ 2014-09-23 16:08 ` Kukjin Kim -1 siblings, 0 replies; 8+ messages in thread From: Kukjin Kim @ 2014-09-23 16:08 UTC (permalink / raw) To: linux-arm-kernel On 09/16/14 21:52, Krzysztof Kozlowski wrote: > This fixes build breakage of platsmp.c if ARMv6 was chosen for compile > time options (e.g. by building allmodconfig): > > $ make allmodconfig > $ make > CC arch/arm/mach-exynos/platsmp.o > /tmp/ccdQM0Eg.s: Assembler messages: > /tmp/ccdQM0Eg.s:432: Error: selected processor does not support ARM mode `isb ' > /tmp/ccdQM0Eg.s:437: Error: selected processor does not support ARM mode `isb ' > /tmp/ccdQM0Eg.s:438: Error: selected processor does not support ARM mode `dsb ' > make[1]: *** [arch/arm/mach-exynos/platsmp.o] Error 1 > > The error was introduced in commit "ARM: EXYNOS: Move code from > hotplug.c to platsmp.c". Previously code using > v7_exit_coherency_flush() macro was built with '-march=armv7-a' flag but > this flag dissapeared during the movement. > > Use isb() and dsb() macros in v7_exit_coherency_flush() so the proper > code will be generated for ARMv6. > > Signed-off-by: Krzysztof Kozlowski<k.kozlowski@samsung.com> > Reported-by: Mark Brown<broonie@kernel.org> > Link: http://www.spinics.net/lists/linux-samsung-soc/msg36790.html > --- > arch/arm/include/asm/cacheflush.h | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h > index 79ecb4f34ffb..b74eeec971c0 100644 > --- a/arch/arm/include/asm/cacheflush.h > +++ b/arch/arm/include/asm/cacheflush.h > @@ -464,22 +464,27 @@ static inline void __sync_cache_range_r(volatile void *p, size_t size) > * CONFIG_FRAME_POINTER=y. ip is saved as well if ever r12-clobbering > * trampoline are inserted by the linker and to keep sp 64-bit aligned. > */ > -#define v7_exit_coherency_flush(level) \ > +#define v7_exit_coherency_flush(level) do { \ > asm volatile( \ > "stmfd sp!, {fp, ip} \n\t" \ > "mrc p15, 0, r0, c1, c0, 0 @ get SCTLR \n\t" \ > "bic r0, r0, #"__stringify(CR_C)" \n\t" \ > "mcr p15, 0, r0, c1, c0, 0 @ set SCTLR \n\t" \ > - "isb \n\t" \ > + : : : "r0","r1","r2","r3","r4","r5","r6","r7", \ > + "r9","r10","lr","memory" ); \ > + isb(); \ > + asm volatile( \ > "bl v7_flush_dcache_"__stringify(level)" \n\t" \ > "mrc p15, 0, r0, c1, c0, 1 @ get ACTLR \n\t" \ > "bic r0, r0, #(1<< 6) @ disable local coherency \n\t" \ > "mcr p15, 0, r0, c1, c0, 1 @ set ACTLR \n\t" \ > - "isb \n\t" \ > - "dsb \n\t" \ > - "ldmfd sp!, {fp, ip}" \ > : : : "r0","r1","r2","r3","r4","r5","r6","r7", \ > - "r9","r10","lr","memory" ) > + "r9","r10","lr","memory" ); \ > + isb(); \ > + dsb(); \ > + asm volatile( \ > + "ldmfd sp!, {fp, ip}" ); \ > + } while (0) > > int set_memory_ro(unsigned long addr, int numpages); > int set_memory_rw(unsigned long addr, int numpages); Hi Russell, Can you please check this patch? Unfortunately I'm not sure about this, this should resolve the build error though... - Kukjin ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ARM: cacheflush: Fix exynos build breakage on ARMv6 by using macros for ISB/DSB @ 2014-09-23 16:08 ` Kukjin Kim 0 siblings, 0 replies; 8+ messages in thread From: Kukjin Kim @ 2014-09-23 16:08 UTC (permalink / raw) To: Krzysztof Kozlowski Cc: Russell King, Will Deacon, David A. Long, Mark Rutland, Vinayak Kale, Laura Abbott, Nicolas Pitre, linux-arm-kernel, linux-kernel, Kukjin Kim, Bartlomiej Zolnierkiewicz, Tomasz Figa, Kyungmin Park, Mark Brown, Marek Szyprowski On 09/16/14 21:52, Krzysztof Kozlowski wrote: > This fixes build breakage of platsmp.c if ARMv6 was chosen for compile > time options (e.g. by building allmodconfig): > > $ make allmodconfig > $ make > CC arch/arm/mach-exynos/platsmp.o > /tmp/ccdQM0Eg.s: Assembler messages: > /tmp/ccdQM0Eg.s:432: Error: selected processor does not support ARM mode `isb ' > /tmp/ccdQM0Eg.s:437: Error: selected processor does not support ARM mode `isb ' > /tmp/ccdQM0Eg.s:438: Error: selected processor does not support ARM mode `dsb ' > make[1]: *** [arch/arm/mach-exynos/platsmp.o] Error 1 > > The error was introduced in commit "ARM: EXYNOS: Move code from > hotplug.c to platsmp.c". Previously code using > v7_exit_coherency_flush() macro was built with '-march=armv7-a' flag but > this flag dissapeared during the movement. > > Use isb() and dsb() macros in v7_exit_coherency_flush() so the proper > code will be generated for ARMv6. > > Signed-off-by: Krzysztof Kozlowski<k.kozlowski@samsung.com> > Reported-by: Mark Brown<broonie@kernel.org> > Link: http://www.spinics.net/lists/linux-samsung-soc/msg36790.html > --- > arch/arm/include/asm/cacheflush.h | 17 +++++++++++------ > 1 file changed, 11 insertions(+), 6 deletions(-) > > diff --git a/arch/arm/include/asm/cacheflush.h b/arch/arm/include/asm/cacheflush.h > index 79ecb4f34ffb..b74eeec971c0 100644 > --- a/arch/arm/include/asm/cacheflush.h > +++ b/arch/arm/include/asm/cacheflush.h > @@ -464,22 +464,27 @@ static inline void __sync_cache_range_r(volatile void *p, size_t size) > * CONFIG_FRAME_POINTER=y. ip is saved as well if ever r12-clobbering > * trampoline are inserted by the linker and to keep sp 64-bit aligned. > */ > -#define v7_exit_coherency_flush(level) \ > +#define v7_exit_coherency_flush(level) do { \ > asm volatile( \ > "stmfd sp!, {fp, ip} \n\t" \ > "mrc p15, 0, r0, c1, c0, 0 @ get SCTLR \n\t" \ > "bic r0, r0, #"__stringify(CR_C)" \n\t" \ > "mcr p15, 0, r0, c1, c0, 0 @ set SCTLR \n\t" \ > - "isb \n\t" \ > + : : : "r0","r1","r2","r3","r4","r5","r6","r7", \ > + "r9","r10","lr","memory" ); \ > + isb(); \ > + asm volatile( \ > "bl v7_flush_dcache_"__stringify(level)" \n\t" \ > "mrc p15, 0, r0, c1, c0, 1 @ get ACTLR \n\t" \ > "bic r0, r0, #(1<< 6) @ disable local coherency \n\t" \ > "mcr p15, 0, r0, c1, c0, 1 @ set ACTLR \n\t" \ > - "isb \n\t" \ > - "dsb \n\t" \ > - "ldmfd sp!, {fp, ip}" \ > : : : "r0","r1","r2","r3","r4","r5","r6","r7", \ > - "r9","r10","lr","memory" ) > + "r9","r10","lr","memory" ); \ > + isb(); \ > + dsb(); \ > + asm volatile( \ > + "ldmfd sp!, {fp, ip}" ); \ > + } while (0) > > int set_memory_ro(unsigned long addr, int numpages); > int set_memory_rw(unsigned long addr, int numpages); Hi Russell, Can you please check this patch? Unfortunately I'm not sure about this, this should resolve the build error though... - Kukjin ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] ARM: cacheflush: Fix exynos build breakage on ARMv6 by using macros for ISB/DSB 2014-09-23 16:08 ` Kukjin Kim @ 2014-09-23 16:54 ` Nicolas Pitre -1 siblings, 0 replies; 8+ messages in thread From: Nicolas Pitre @ 2014-09-23 16:54 UTC (permalink / raw) To: linux-arm-kernel On Wed, 24 Sep 2014, Kukjin Kim wrote: > On 09/16/14 21:52, Krzysztof Kozlowski wrote: > > This fixes build breakage of platsmp.c if ARMv6 was chosen for compile > > time options (e.g. by building allmodconfig): > > > > $ make allmodconfig > > $ make > > CC arch/arm/mach-exynos/platsmp.o > > /tmp/ccdQM0Eg.s: Assembler messages: > > /tmp/ccdQM0Eg.s:432: Error: selected processor does not support ARM mode > > `isb ' > > /tmp/ccdQM0Eg.s:437: Error: selected processor does not support ARM mode > > `isb ' > > /tmp/ccdQM0Eg.s:438: Error: selected processor does not support ARM mode > > `dsb ' > > make[1]: *** [arch/arm/mach-exynos/platsmp.o] Error 1 > > > > The error was introduced in commit "ARM: EXYNOS: Move code from > > hotplug.c to platsmp.c". Previously code using > > v7_exit_coherency_flush() macro was built with '-march=armv7-a' flag but > > this flag dissapeared during the movement. > > > > Use isb() and dsb() macros in v7_exit_coherency_flush() so the proper > > code will be generated for ARMv6. > > > > Signed-off-by: Krzysztof Kozlowski<k.kozlowski@samsung.com> > > Reported-by: Mark Brown<broonie@kernel.org> > > Link: http://www.spinics.net/lists/linux-samsung-soc/msg36790.html > > --- > > arch/arm/include/asm/cacheflush.h | 17 +++++++++++------ > > 1 file changed, 11 insertions(+), 6 deletions(-) > > > > diff --git a/arch/arm/include/asm/cacheflush.h > > b/arch/arm/include/asm/cacheflush.h > > index 79ecb4f34ffb..b74eeec971c0 100644 > > --- a/arch/arm/include/asm/cacheflush.h > > +++ b/arch/arm/include/asm/cacheflush.h > > @@ -464,22 +464,27 @@ static inline void __sync_cache_range_r(volatile void > > *p, size_t size) > > * CONFIG_FRAME_POINTER=y. ip is saved as well if ever r12-clobbering > > * trampoline are inserted by the linker and to keep sp 64-bit aligned. > > */ > > -#define v7_exit_coherency_flush(level) \ > > +#define v7_exit_coherency_flush(level) do { \ > > asm volatile( \ > > "stmfd sp!, {fp, ip} \n\t" \ > > "mrc p15, 0, r0, c1, c0, 0 @ get SCTLR \n\t" \ > > "bic r0, r0, #"__stringify(CR_C)" \n\t" \ > > "mcr p15, 0, r0, c1, c0, 0 @ set SCTLR \n\t" \ > > - "isb \n\t" \ > > + : : : "r0","r1","r2","r3","r4","r5","r6","r7", \ > > + "r9","r10","lr","memory" ); \ > > + isb(); \ > > + asm volatile( \ > > "bl v7_flush_dcache_"__stringify(level)" \n\t" \ > > "mrc p15, 0, r0, c1, c0, 1 @ get ACTLR \n\t" \ > > "bic r0, r0, #(1<< 6) @ disable local coherency \n\t" \ > > "mcr p15, 0, r0, c1, c0, 1 @ set ACTLR \n\t" \ > > - "isb \n\t" \ > > - "dsb \n\t" \ > > - "ldmfd sp!, {fp, ip}" \ > > : : : "r0","r1","r2","r3","r4","r5","r6","r7", \ > > - "r9","r10","lr","memory" ) > > + "r9","r10","lr","memory" ); \ > > + isb(); \ > > + dsb(); \ > > + asm volatile( \ > > + "ldmfd sp!, {fp, ip}" ); \ > > + } while (0) > > > > int set_memory_ro(unsigned long addr, int numpages); > > int set_memory_rw(unsigned long addr, int numpages); > > Hi Russell, > > Can you please check this patch? Unfortunately I'm not sure about this, this > should resolve the build error though... I don't like this patch at all. The original assembly sequence was carefully written in a single segment because it is important that the compiler does not attempt to schedule anything in the middle. Some definitions of isb() and dsb() need a temporary register which needs to be allocated and initialized and who knows what the compiler might do (such as spilling to the stack which might be fatal here). Instead, you should insert ".arch armv7-a" in the assembly sequence to tell the assembler the isb and dsb instructions are going to be executed on an ARMv7 capable CPU. Nicolas ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ARM: cacheflush: Fix exynos build breakage on ARMv6 by using macros for ISB/DSB @ 2014-09-23 16:54 ` Nicolas Pitre 0 siblings, 0 replies; 8+ messages in thread From: Nicolas Pitre @ 2014-09-23 16:54 UTC (permalink / raw) To: Kukjin Kim Cc: Krzysztof Kozlowski, Russell King, Will Deacon, David A. Long, Mark Rutland, Vinayak Kale, Laura Abbott, linux-arm-kernel, linux-kernel, Bartlomiej Zolnierkiewicz, Tomasz Figa, Kyungmin Park, Mark Brown, Marek Szyprowski On Wed, 24 Sep 2014, Kukjin Kim wrote: > On 09/16/14 21:52, Krzysztof Kozlowski wrote: > > This fixes build breakage of platsmp.c if ARMv6 was chosen for compile > > time options (e.g. by building allmodconfig): > > > > $ make allmodconfig > > $ make > > CC arch/arm/mach-exynos/platsmp.o > > /tmp/ccdQM0Eg.s: Assembler messages: > > /tmp/ccdQM0Eg.s:432: Error: selected processor does not support ARM mode > > `isb ' > > /tmp/ccdQM0Eg.s:437: Error: selected processor does not support ARM mode > > `isb ' > > /tmp/ccdQM0Eg.s:438: Error: selected processor does not support ARM mode > > `dsb ' > > make[1]: *** [arch/arm/mach-exynos/platsmp.o] Error 1 > > > > The error was introduced in commit "ARM: EXYNOS: Move code from > > hotplug.c to platsmp.c". Previously code using > > v7_exit_coherency_flush() macro was built with '-march=armv7-a' flag but > > this flag dissapeared during the movement. > > > > Use isb() and dsb() macros in v7_exit_coherency_flush() so the proper > > code will be generated for ARMv6. > > > > Signed-off-by: Krzysztof Kozlowski<k.kozlowski@samsung.com> > > Reported-by: Mark Brown<broonie@kernel.org> > > Link: http://www.spinics.net/lists/linux-samsung-soc/msg36790.html > > --- > > arch/arm/include/asm/cacheflush.h | 17 +++++++++++------ > > 1 file changed, 11 insertions(+), 6 deletions(-) > > > > diff --git a/arch/arm/include/asm/cacheflush.h > > b/arch/arm/include/asm/cacheflush.h > > index 79ecb4f34ffb..b74eeec971c0 100644 > > --- a/arch/arm/include/asm/cacheflush.h > > +++ b/arch/arm/include/asm/cacheflush.h > > @@ -464,22 +464,27 @@ static inline void __sync_cache_range_r(volatile void > > *p, size_t size) > > * CONFIG_FRAME_POINTER=y. ip is saved as well if ever r12-clobbering > > * trampoline are inserted by the linker and to keep sp 64-bit aligned. > > */ > > -#define v7_exit_coherency_flush(level) \ > > +#define v7_exit_coherency_flush(level) do { \ > > asm volatile( \ > > "stmfd sp!, {fp, ip} \n\t" \ > > "mrc p15, 0, r0, c1, c0, 0 @ get SCTLR \n\t" \ > > "bic r0, r0, #"__stringify(CR_C)" \n\t" \ > > "mcr p15, 0, r0, c1, c0, 0 @ set SCTLR \n\t" \ > > - "isb \n\t" \ > > + : : : "r0","r1","r2","r3","r4","r5","r6","r7", \ > > + "r9","r10","lr","memory" ); \ > > + isb(); \ > > + asm volatile( \ > > "bl v7_flush_dcache_"__stringify(level)" \n\t" \ > > "mrc p15, 0, r0, c1, c0, 1 @ get ACTLR \n\t" \ > > "bic r0, r0, #(1<< 6) @ disable local coherency \n\t" \ > > "mcr p15, 0, r0, c1, c0, 1 @ set ACTLR \n\t" \ > > - "isb \n\t" \ > > - "dsb \n\t" \ > > - "ldmfd sp!, {fp, ip}" \ > > : : : "r0","r1","r2","r3","r4","r5","r6","r7", \ > > - "r9","r10","lr","memory" ) > > + "r9","r10","lr","memory" ); \ > > + isb(); \ > > + dsb(); \ > > + asm volatile( \ > > + "ldmfd sp!, {fp, ip}" ); \ > > + } while (0) > > > > int set_memory_ro(unsigned long addr, int numpages); > > int set_memory_rw(unsigned long addr, int numpages); > > Hi Russell, > > Can you please check this patch? Unfortunately I'm not sure about this, this > should resolve the build error though... I don't like this patch at all. The original assembly sequence was carefully written in a single segment because it is important that the compiler does not attempt to schedule anything in the middle. Some definitions of isb() and dsb() need a temporary register which needs to be allocated and initialized and who knows what the compiler might do (such as spilling to the stack which might be fatal here). Instead, you should insert ".arch armv7-a" in the assembly sequence to tell the assembler the isb and dsb instructions are going to be executed on an ARMv7 capable CPU. Nicolas ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH] ARM: cacheflush: Fix exynos build breakage on ARMv6 by using macros for ISB/DSB 2014-09-23 16:54 ` Nicolas Pitre @ 2014-09-24 7:51 ` Krzysztof Kozlowski -1 siblings, 0 replies; 8+ messages in thread From: Krzysztof Kozlowski @ 2014-09-24 7:51 UTC (permalink / raw) To: linux-arm-kernel On wto, 2014-09-23 at 12:54 -0400, Nicolas Pitre wrote: > On Wed, 24 Sep 2014, Kukjin Kim wrote: > > > On 09/16/14 21:52, Krzysztof Kozlowski wrote: > > > This fixes build breakage of platsmp.c if ARMv6 was chosen for compile > > > time options (e.g. by building allmodconfig): > > > > > > $ make allmodconfig > > > $ make > > > CC arch/arm/mach-exynos/platsmp.o > > > /tmp/ccdQM0Eg.s: Assembler messages: > > > /tmp/ccdQM0Eg.s:432: Error: selected processor does not support ARM mode > > > `isb ' > > > /tmp/ccdQM0Eg.s:437: Error: selected processor does not support ARM mode > > > `isb ' > > > /tmp/ccdQM0Eg.s:438: Error: selected processor does not support ARM mode > > > `dsb ' > > > make[1]: *** [arch/arm/mach-exynos/platsmp.o] Error 1 > > > > > > The error was introduced in commit "ARM: EXYNOS: Move code from > > > hotplug.c to platsmp.c". Previously code using > > > v7_exit_coherency_flush() macro was built with '-march=armv7-a' flag but > > > this flag dissapeared during the movement. > > > > > > Use isb() and dsb() macros in v7_exit_coherency_flush() so the proper > > > code will be generated for ARMv6. > > > > > > Signed-off-by: Krzysztof Kozlowski<k.kozlowski@samsung.com> > > > Reported-by: Mark Brown<broonie@kernel.org> > > > Link: http://www.spinics.net/lists/linux-samsung-soc/msg36790.html > > > --- > > > arch/arm/include/asm/cacheflush.h | 17 +++++++++++------ > > > 1 file changed, 11 insertions(+), 6 deletions(-) > > > > > > diff --git a/arch/arm/include/asm/cacheflush.h > > > b/arch/arm/include/asm/cacheflush.h > > > index 79ecb4f34ffb..b74eeec971c0 100644 > > > --- a/arch/arm/include/asm/cacheflush.h > > > +++ b/arch/arm/include/asm/cacheflush.h > > > @@ -464,22 +464,27 @@ static inline void __sync_cache_range_r(volatile void > > > *p, size_t size) > > > * CONFIG_FRAME_POINTER=y. ip is saved as well if ever r12-clobbering > > > * trampoline are inserted by the linker and to keep sp 64-bit aligned. > > > */ > > > -#define v7_exit_coherency_flush(level) \ > > > +#define v7_exit_coherency_flush(level) do { \ > > > asm volatile( \ > > > "stmfd sp!, {fp, ip} \n\t" \ > > > "mrc p15, 0, r0, c1, c0, 0 @ get SCTLR \n\t" \ > > > "bic r0, r0, #"__stringify(CR_C)" \n\t" \ > > > "mcr p15, 0, r0, c1, c0, 0 @ set SCTLR \n\t" \ > > > - "isb \n\t" \ > > > + : : : "r0","r1","r2","r3","r4","r5","r6","r7", \ > > > + "r9","r10","lr","memory" ); \ > > > + isb(); \ > > > + asm volatile( \ > > > "bl v7_flush_dcache_"__stringify(level)" \n\t" \ > > > "mrc p15, 0, r0, c1, c0, 1 @ get ACTLR \n\t" \ > > > "bic r0, r0, #(1<< 6) @ disable local coherency \n\t" \ > > > "mcr p15, 0, r0, c1, c0, 1 @ set ACTLR \n\t" \ > > > - "isb \n\t" \ > > > - "dsb \n\t" \ > > > - "ldmfd sp!, {fp, ip}" \ > > > : : : "r0","r1","r2","r3","r4","r5","r6","r7", \ > > > - "r9","r10","lr","memory" ) > > > + "r9","r10","lr","memory" ); \ > > > + isb(); \ > > > + dsb(); \ > > > + asm volatile( \ > > > + "ldmfd sp!, {fp, ip}" ); \ > > > + } while (0) > > > > > > int set_memory_ro(unsigned long addr, int numpages); > > > int set_memory_rw(unsigned long addr, int numpages); > > > > Hi Russell, > > > > Can you please check this patch? Unfortunately I'm not sure about this, this > > should resolve the build error though... > > I don't like this patch at all. > > The original assembly sequence was carefully written in a single segment > because it is important that the compiler does not attempt to schedule > anything in the middle. Some definitions of isb() and dsb() need a > temporary register which needs to be allocated and initialized and who > knows what the compiler might do (such as spilling to the stack which > might be fatal here). > > Instead, you should insert ".arch armv7-a" in the assembly sequence to > tell the assembler the isb and dsb instructions are going to be executed > on an ARMv7 capable CPU. > Thank you for pointing this out. I'll send fixed patch. Best regards, Krzysztof > > Nicolas ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH] ARM: cacheflush: Fix exynos build breakage on ARMv6 by using macros for ISB/DSB @ 2014-09-24 7:51 ` Krzysztof Kozlowski 0 siblings, 0 replies; 8+ messages in thread From: Krzysztof Kozlowski @ 2014-09-24 7:51 UTC (permalink / raw) To: Nicolas Pitre Cc: Kukjin Kim, Russell King, Will Deacon, David A. Long, Mark Rutland, Vinayak Kale, Laura Abbott, linux-arm-kernel, linux-kernel, Bartlomiej Zolnierkiewicz, Tomasz Figa, Kyungmin Park, Mark Brown, Marek Szyprowski On wto, 2014-09-23 at 12:54 -0400, Nicolas Pitre wrote: > On Wed, 24 Sep 2014, Kukjin Kim wrote: > > > On 09/16/14 21:52, Krzysztof Kozlowski wrote: > > > This fixes build breakage of platsmp.c if ARMv6 was chosen for compile > > > time options (e.g. by building allmodconfig): > > > > > > $ make allmodconfig > > > $ make > > > CC arch/arm/mach-exynos/platsmp.o > > > /tmp/ccdQM0Eg.s: Assembler messages: > > > /tmp/ccdQM0Eg.s:432: Error: selected processor does not support ARM mode > > > `isb ' > > > /tmp/ccdQM0Eg.s:437: Error: selected processor does not support ARM mode > > > `isb ' > > > /tmp/ccdQM0Eg.s:438: Error: selected processor does not support ARM mode > > > `dsb ' > > > make[1]: *** [arch/arm/mach-exynos/platsmp.o] Error 1 > > > > > > The error was introduced in commit "ARM: EXYNOS: Move code from > > > hotplug.c to platsmp.c". Previously code using > > > v7_exit_coherency_flush() macro was built with '-march=armv7-a' flag but > > > this flag dissapeared during the movement. > > > > > > Use isb() and dsb() macros in v7_exit_coherency_flush() so the proper > > > code will be generated for ARMv6. > > > > > > Signed-off-by: Krzysztof Kozlowski<k.kozlowski@samsung.com> > > > Reported-by: Mark Brown<broonie@kernel.org> > > > Link: http://www.spinics.net/lists/linux-samsung-soc/msg36790.html > > > --- > > > arch/arm/include/asm/cacheflush.h | 17 +++++++++++------ > > > 1 file changed, 11 insertions(+), 6 deletions(-) > > > > > > diff --git a/arch/arm/include/asm/cacheflush.h > > > b/arch/arm/include/asm/cacheflush.h > > > index 79ecb4f34ffb..b74eeec971c0 100644 > > > --- a/arch/arm/include/asm/cacheflush.h > > > +++ b/arch/arm/include/asm/cacheflush.h > > > @@ -464,22 +464,27 @@ static inline void __sync_cache_range_r(volatile void > > > *p, size_t size) > > > * CONFIG_FRAME_POINTER=y. ip is saved as well if ever r12-clobbering > > > * trampoline are inserted by the linker and to keep sp 64-bit aligned. > > > */ > > > -#define v7_exit_coherency_flush(level) \ > > > +#define v7_exit_coherency_flush(level) do { \ > > > asm volatile( \ > > > "stmfd sp!, {fp, ip} \n\t" \ > > > "mrc p15, 0, r0, c1, c0, 0 @ get SCTLR \n\t" \ > > > "bic r0, r0, #"__stringify(CR_C)" \n\t" \ > > > "mcr p15, 0, r0, c1, c0, 0 @ set SCTLR \n\t" \ > > > - "isb \n\t" \ > > > + : : : "r0","r1","r2","r3","r4","r5","r6","r7", \ > > > + "r9","r10","lr","memory" ); \ > > > + isb(); \ > > > + asm volatile( \ > > > "bl v7_flush_dcache_"__stringify(level)" \n\t" \ > > > "mrc p15, 0, r0, c1, c0, 1 @ get ACTLR \n\t" \ > > > "bic r0, r0, #(1<< 6) @ disable local coherency \n\t" \ > > > "mcr p15, 0, r0, c1, c0, 1 @ set ACTLR \n\t" \ > > > - "isb \n\t" \ > > > - "dsb \n\t" \ > > > - "ldmfd sp!, {fp, ip}" \ > > > : : : "r0","r1","r2","r3","r4","r5","r6","r7", \ > > > - "r9","r10","lr","memory" ) > > > + "r9","r10","lr","memory" ); \ > > > + isb(); \ > > > + dsb(); \ > > > + asm volatile( \ > > > + "ldmfd sp!, {fp, ip}" ); \ > > > + } while (0) > > > > > > int set_memory_ro(unsigned long addr, int numpages); > > > int set_memory_rw(unsigned long addr, int numpages); > > > > Hi Russell, > > > > Can you please check this patch? Unfortunately I'm not sure about this, this > > should resolve the build error though... > > I don't like this patch at all. > > The original assembly sequence was carefully written in a single segment > because it is important that the compiler does not attempt to schedule > anything in the middle. Some definitions of isb() and dsb() need a > temporary register which needs to be allocated and initialized and who > knows what the compiler might do (such as spilling to the stack which > might be fatal here). > > Instead, you should insert ".arch armv7-a" in the assembly sequence to > tell the assembler the isb and dsb instructions are going to be executed > on an ARMv7 capable CPU. > Thank you for pointing this out. I'll send fixed patch. Best regards, Krzysztof > > Nicolas ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2014-09-24 7:51 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2014-09-16 12:52 [PATCH] ARM: cacheflush: Fix exynos build breakage on ARMv6 by using macros for ISB/DSB Krzysztof Kozlowski 2014-09-16 12:52 ` Krzysztof Kozlowski 2014-09-23 16:08 ` Kukjin Kim 2014-09-23 16:08 ` Kukjin Kim 2014-09-23 16:54 ` Nicolas Pitre 2014-09-23 16:54 ` Nicolas Pitre 2014-09-24 7:51 ` Krzysztof Kozlowski 2014-09-24 7:51 ` Krzysztof Kozlowski
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.