[PATCH] ARM: PJ4: move coprocessor register access sequences to iwmmxt.S

[PATCH] ARM: PJ4: move coprocessor register access sequences to iwmmxt.S

[Ard Biesheuvel]

The PJ4 inline asm sequences in pj4-cp0.c cannot be built in Thumb-2 mode,
due to the way it performs arithmetic on the program counter, so it is
built in ARM mode instead. However, building C files in ARM mode under
CONFIG_THUMB2_KERNEL is problematic, since the instrumentation performed
by subsystems like ftrace does not expect having to deal with interworking
branches.

So instead, revert to building pj4-cp0.c in Thumb-2 mode, and move the
offending sequence to iwmmxt.S, which is not instrumented anyway, and is
already built in ARM mode unconditionally.

Reported-by: Stephen Boyd <sboyd@codeaurora.org>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 arch/arm/kernel/Makefile  |  1 -
 arch/arm/kernel/iwmmxt.S  | 21 +++++++++++++++++++++
 arch/arm/kernel/pj4-cp0.c | 24 ++----------------------
 3 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index af9e59bf3831..3c789496297f 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -73,7 +73,6 @@ obj-$(CONFIG_IWMMXT)		+= iwmmxt.o
 obj-$(CONFIG_PERF_EVENTS)	+= perf_regs.o perf_callchain.o
 obj-$(CONFIG_HW_PERF_EVENTS)	+= perf_event_xscale.o perf_event_v6.o \
 				   perf_event_v7.o
-CFLAGS_pj4-cp0.o		:= -marm
 AFLAGS_iwmmxt.o			:= -Wa,-mcpu=iwmmxt
 obj-$(CONFIG_ARM_CPU_TOPOLOGY)  += topology.o
 obj-$(CONFIG_VDSO)		+= vdso.o
diff --git a/arch/arm/kernel/iwmmxt.S b/arch/arm/kernel/iwmmxt.S
index 49fadbda8c63..e1776bf52480 100644
--- a/arch/arm/kernel/iwmmxt.S
+++ b/arch/arm/kernel/iwmmxt.S
@@ -14,6 +14,7 @@
  * published by the Free Software Foundation.
  */
 
+#include <linux/init.h>
 #include <linux/linkage.h>
 #include <asm/ptrace.h>
 #include <asm/thread_info.h>
@@ -370,3 +371,23 @@ ENDPROC(iwmmxt_task_release)
 concan_owner:
 	.word	0
 
+#if defined(CONFIG_CPU_PJ4) || defined(CONFIG_CPU_PJ4B)
+
+	__INIT
+
+ENTRY(pj4_cp_access_read)
+	mrc	p15, 0, r0, c1, c0, 2
+	ret	lr
+ENDPROC(pj4_cp_access_read)
+
+ENTRY(pj4_cp_access_write)
+	mcr	p15, 0, r0, c1, c0, 2
+	mrc	p15, 0, r0, c1, c0, 2
+	mov	r0, r0
+	sub	pc, pc, #4
+	ret	lr
+ENDPROC(pj4_cp_access_write)
+
+	__FINIT
+
+#endif
diff --git a/arch/arm/kernel/pj4-cp0.c b/arch/arm/kernel/pj4-cp0.c
index 8153e36b2491..4f226e175734 100644
--- a/arch/arm/kernel/pj4-cp0.c
+++ b/arch/arm/kernel/pj4-cp0.c
@@ -49,28 +49,8 @@ static struct notifier_block __maybe_unused iwmmxt_notifier_block = {
 	.notifier_call	= iwmmxt_do,
 };
 
-
-static u32 __init pj4_cp_access_read(void)
-{
-	u32 value;
-
-	__asm__ __volatile__ (
-		"mrc	p15, 0, %0, c1, c0, 2\n\t"
-		: "=r" (value));
-	return value;
-}
-
-static void __init pj4_cp_access_write(u32 value)
-{
-	u32 temp;
-
-	__asm__ __volatile__ (
-		"mcr	p15, 0, %1, c1, c0, 2\n\t"
-		"mrc	p15, 0, %0, c1, c0, 2\n\t"
-		"mov	%0, %0\n\t"
-		"sub	pc, pc, #4\n\t"
-		: "=r" (temp) : "r" (value));
-}
+asmlinkage u32 pj4_cp_access_read(void);
+asmlinkage void pj4_cp_access_write(u32 value);
 
 static int __init pj4_get_iwmmxt_version(void)
 {
-- 
1.9.1

[PATCH] ARM: PJ4: move coprocessor register access sequences to iwmmxt.S

[Stephen Boyd]

On 11/13, Ard Biesheuvel wrote:
> The PJ4 inline asm sequences in pj4-cp0.c cannot be built in Thumb-2 mode,
> due to the way it performs arithmetic on the program counter, so it is
> built in ARM mode instead. However, building C files in ARM mode under
> CONFIG_THUMB2_KERNEL is problematic, since the instrumentation performed
> by subsystems like ftrace does not expect having to deal with interworking
> branches.
> 
> So instead, revert to building pj4-cp0.c in Thumb-2 mode, and move the
> offending sequence to iwmmxt.S, which is not instrumented anyway, and is
> already built in ARM mode unconditionally.
> 
> Reported-by: Stephen Boyd <sboyd@codeaurora.org>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---

Tested-by: Stephen Boyd <sboyd@codeaurora.org>

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

[PATCH] ARM: PJ4: move coprocessor register access sequences to iwmmxt.S

[Ard Biesheuvel]

On 13 November 2015 at 19:48, Stephen Boyd <sboyd@codeaurora.org> wrote:
> On 11/13, Ard Biesheuvel wrote:
>> The PJ4 inline asm sequences in pj4-cp0.c cannot be built in Thumb-2 mode,
>> due to the way it performs arithmetic on the program counter, so it is
>> built in ARM mode instead. However, building C files in ARM mode under
>> CONFIG_THUMB2_KERNEL is problematic, since the instrumentation performed
>> by subsystems like ftrace does not expect having to deal with interworking
>> branches.
>>
>> So instead, revert to building pj4-cp0.c in Thumb-2 mode, and move the
>> offending sequence to iwmmxt.S, which is not instrumented anyway, and is
>> already built in ARM mode unconditionally.
>>
>> Reported-by: Stephen Boyd <sboyd@codeaurora.org>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>
> Tested-by: Stephen Boyd <sboyd@codeaurora.org>
>

Thanks.

I've put this into the patch system (8452/1)

[PATCH] ARM: PJ4: move coprocessor register access sequences to iwmmxt.S

[Russell King - ARM Linux]

On Fri, Nov 13, 2015 at 08:23:34PM +0100, Ard Biesheuvel wrote:
> On 13 November 2015 at 19:48, Stephen Boyd <sboyd@codeaurora.org> wrote:
> > On 11/13, Ard Biesheuvel wrote:
> >> The PJ4 inline asm sequences in pj4-cp0.c cannot be built in Thumb-2 mode,
> >> due to the way it performs arithmetic on the program counter, so it is
> >> built in ARM mode instead. However, building C files in ARM mode under
> >> CONFIG_THUMB2_KERNEL is problematic, since the instrumentation performed
> >> by subsystems like ftrace does not expect having to deal with interworking
> >> branches.
> >>
> >> So instead, revert to building pj4-cp0.c in Thumb-2 mode, and move the
> >> offending sequence to iwmmxt.S, which is not instrumented anyway, and is
> >> already built in ARM mode unconditionally.
> >>
> >> Reported-by: Stephen Boyd <sboyd@codeaurora.org>
> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >> ---
> >
> > Tested-by: Stephen Boyd <sboyd@codeaurora.org>
> >
> 
> Thanks.
> 
> I've put this into the patch system (8452/1)

And I've dropped it from my tree because it's broken.

Come on guys, you can do better than this:

arch/arm/kernel/built-in.o: In function `pj4_cp0_init':
arch/arm/kernel/psci-call.o:(.init.text+0x22d0): undefined reference to `pj4_cp_access_write'
arch/arm/kernel/psci-call.o:(.init.text+0x22e0): undefined reference to `pj4_cp_access_write'
arch/arm/kernel/psci-call.o:(.init.text+0x2304): undefined reference to `pj4_cp_access_read'
arch/arm/kernel/psci-call.o:(.init.text+0x2310): undefined reference to `pj4_cp_access_write'
arch/arm/kernel/psci-call.o:(.init.text+0x2314): undefined reference to `pj4_cp_access_read'

Let's look at the makefile:

obj-$(CONFIG_CPU_PJ4)           += pj4-cp0.o
obj-$(CONFIG_CPU_PJ4B)          += pj4-cp0.o
obj-$(CONFIG_IWMMXT)            += iwmmxt.o

Now, you're moving code from pj4-cp0.c to iwmmxt.S, and of course, you
checked that IWMMXT would always be enabled if either PJ4 or PJ4B are
enabled.

config IWMMXT
        bool "Enable iWMMXt support"
        depends on CPU_XSCALE || CPU_XSC3 || CPU_MOHAWK || CPU_PJ4 || CPU_PJ4B
        default y if PXA27x || PXA3xx || ARCH_MMP || CPU_PJ4 || CPU_PJ4B

Err, no, IWMMXT can be disabled.  Hence the build errors.

Please, take more time when moving code around to ensure that this kind
of build breakage doesn't creep in.  It's *really* easy to check, it
only takes a couple of additional minutes.

-- 
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCH] ARM: PJ4: move coprocessor register access sequences to iwmmxt.S

[Ard Biesheuvel]

On 17 December 2015 at 11:27, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Fri, Nov 13, 2015 at 08:23:34PM +0100, Ard Biesheuvel wrote:
>> On 13 November 2015 at 19:48, Stephen Boyd <sboyd@codeaurora.org> wrote:
>> > On 11/13, Ard Biesheuvel wrote:
>> >> The PJ4 inline asm sequences in pj4-cp0.c cannot be built in Thumb-2 mode,
>> >> due to the way it performs arithmetic on the program counter, so it is
>> >> built in ARM mode instead. However, building C files in ARM mode under
>> >> CONFIG_THUMB2_KERNEL is problematic, since the instrumentation performed
>> >> by subsystems like ftrace does not expect having to deal with interworking
>> >> branches.
>> >>
>> >> So instead, revert to building pj4-cp0.c in Thumb-2 mode, and move the
>> >> offending sequence to iwmmxt.S, which is not instrumented anyway, and is
>> >> already built in ARM mode unconditionally.
>> >>
>> >> Reported-by: Stephen Boyd <sboyd@codeaurora.org>
>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >> ---
>> >
>> > Tested-by: Stephen Boyd <sboyd@codeaurora.org>
>> >
>>
>> Thanks.
>>
>> I've put this into the patch system (8452/1)
>
> And I've dropped it from my tree because it's broken.
>
> Come on guys, you can do better than this:
>
> arch/arm/kernel/built-in.o: In function `pj4_cp0_init':
> arch/arm/kernel/psci-call.o:(.init.text+0x22d0): undefined reference to `pj4_cp_access_write'
> arch/arm/kernel/psci-call.o:(.init.text+0x22e0): undefined reference to `pj4_cp_access_write'
> arch/arm/kernel/psci-call.o:(.init.text+0x2304): undefined reference to `pj4_cp_access_read'
> arch/arm/kernel/psci-call.o:(.init.text+0x2310): undefined reference to `pj4_cp_access_write'
> arch/arm/kernel/psci-call.o:(.init.text+0x2314): undefined reference to `pj4_cp_access_read'
>
> Let's look at the makefile:
>
> obj-$(CONFIG_CPU_PJ4)           += pj4-cp0.o
> obj-$(CONFIG_CPU_PJ4B)          += pj4-cp0.o
> obj-$(CONFIG_IWMMXT)            += iwmmxt.o
>
> Now, you're moving code from pj4-cp0.c to iwmmxt.S, and of course, you
> checked that IWMMXT would always be enabled if either PJ4 or PJ4B are
> enabled.
>
> config IWMMXT
>         bool "Enable iWMMXt support"
>         depends on CPU_XSCALE || CPU_XSC3 || CPU_MOHAWK || CPU_PJ4 || CPU_PJ4B
>         default y if PXA27x || PXA3xx || ARCH_MMP || CPU_PJ4 || CPU_PJ4B
>
> Err, no, IWMMXT can be disabled.  Hence the build errors.
>
> Please, take more time when moving code around to ensure that this kind
> of build breakage doesn't creep in.  It's *really* easy to check, it
> only takes a couple of additional minutes.
>

Yes, I was just looking into it after Arnd mentioned it over IRC

If you can live with not having the pr_info() that warns you when
running on a PJ4 with iWMMXt while CONFIG_IWMMXT is disabled, I could
just drop pj4-cp0.c from the build entirely, i.e.,

diff --git a/arch/arm/kernel/Makefile b/arch/arm/kernel/Makefile
index af9e59bf3831..9d798fb17359 100644
--- a/arch/arm/kernel/Makefile
+++ b/arch/arm/kernel/Makefile
@@ -67,13 +67,12 @@ obj-$(CONFIG_HAVE_HW_BREAKPOINT)    += hw_breakpoint.o
 obj-$(CONFIG_CPU_XSCALE)       += xscale-cp0.o
 obj-$(CONFIG_CPU_XSC3)         += xscale-cp0.o
 obj-$(CONFIG_CPU_MOHAWK)       += xscale-cp0.o
-obj-$(CONFIG_CPU_PJ4)          += pj4-cp0.o
-obj-$(CONFIG_CPU_PJ4B)         += pj4-cp0.o
-obj-$(CONFIG_IWMMXT)           += iwmmxt.o
+iwmmxt-obj-$(CONFIG_CPU_PJ4)   := pj4-cp0.o
+iwmmxt-obj-$(CONFIG_CPU_PJ4B)  := pj4-cp0.o
+obj-$(CONFIG_IWMMXT)           += iwmmxt.o $(iwmmxt-obj-y)
 obj-$(CONFIG_PERF_EVENTS)      += perf_regs.o perf_callchain.o
 obj-$(CONFIG_HW_PERF_EVENTS)   += perf_event_xscale.o perf_event_v6.o \
                                   perf_event_v7.o