* [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
0 siblings, 1 reply; 4+ 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] 4+ messages in thread
* [PATCH] ARM: cacheflush: Fix exynos build breakage on ARMv6 by using macros for ISB/DSB
2014-09-16 12:52 [PATCH] ARM: cacheflush: Fix exynos build breakage on ARMv6 by using macros for ISB/DSB Krzysztof Kozlowski
@ 2014-09-23 16:08 ` Kukjin Kim
2014-09-23 16:54 ` Nicolas Pitre
0 siblings, 1 reply; 4+ 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] 4+ 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
2014-09-24 7:51 ` Krzysztof Kozlowski
0 siblings, 1 reply; 4+ 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] 4+ 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
0 siblings, 0 replies; 4+ 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] 4+ messages in thread
end of thread, other threads:[~2014-09-24 7:51 UTC | newest]
Thread overview: 4+ 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-23 16:08 ` Kukjin Kim
2014-09-23 16:54 ` Nicolas Pitre
2014-09-24 7:51 ` Krzysztof Kozlowski
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).