[PATCH 0/3] Fixes for kprobes test issues

[PATCH 0/3] Fixes for kprobes test issues

[Jon Medhurst]

This small series fixes issues found with the kprobes tests during
testing of Dave Long's latest ARM uprobe patches [1]. Whilst these fixes
are based on top of that series all the bugs are already present in the
kernel tree.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2014-March/238862.html

Contents:

Jon Medhurst (3):
      ARM: kprobes: Prevent known test failures stopping other tests running
      ARM: kprobes: Disallow instructions with PC and register specified shift
      ARM: kprobes: Fix test code compilation errors for ARMv4 targets

 arch/arm/kernel/kprobes-test-arm.c |   33 +++++++++++++++++++++------------
 arch/arm/kernel/kprobes-test.c     |   10 ++++++++++
 arch/arm/kernel/probes-arm.c       |    6 +++---

[PATCH 1/3] ARM: kprobes: Prevent known test failures stopping other tests running

[Jon Medhurst]

Due to a long-standing issue with Thumb symbol lookup [1] the jprobes
tests fail when built into a kernel compiled as Thumb mode. (They work
fine for ARM mode kernels or for Thumb when built as a loadable module.)

Rather than have this problem terminate testing prematurely lets instead
emit an error message and carry on with the main kprobes tests, delaying
the final failure report until the end.

[1] http://lists.infradead.org/pipermail/linux-arm-kernel/2011-August/063026.html

Signed-off-by: Jon Medhurst <tixy@linaro.org>
---
 arch/arm/kernel/kprobes-test.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/arch/arm/kernel/kprobes-test.c b/arch/arm/kernel/kprobes-test.c
index c2fd06b..6de5e94 100644
--- a/arch/arm/kernel/kprobes-test.c
+++ b/arch/arm/kernel/kprobes-test.c
@@ -225,6 +225,7 @@ static int pre_handler_called;
 static int post_handler_called;
 static int jprobe_func_called;
 static int kretprobe_handler_called;
+static int tests_failed;
 
 #define FUNC_ARG1 0x12345678
 #define FUNC_ARG2 0xabcdef
@@ -461,6 +462,13 @@ static int run_api_tests(long (*func)(long, long))
 
 	pr_info("    jprobe\n");
 	ret = test_jprobe(func);
+#if defined(CONFIG_THUMB2_KERNEL) && !defined(MODULE)
+	if (ret == -EINVAL) {
+		pr_err("FAIL: Known longtime bug with jprobe on Thumb kernels\n");
+		tests_failed = ret;
+		ret = 0;
+	}
+#endif
 	if (ret < 0)
 		return ret;
 
@@ -1671,6 +1679,8 @@ static int __init run_all_tests(void)
 
 out:
 	if (ret == 0)
+		ret = tests_failed;
+	if (ret == 0)
 		pr_info("Finished kprobe tests OK\n");
 	else
 		pr_err("kprobe tests failed\n");
-- 
1.7.10.4

[PATCH 1/3] ARM: kprobes: Prevent known test failures stopping other tests running

[David Long]

On 03/11/14 12:54, Jon Medhurst wrote:
> Due to a long-standing issue with Thumb symbol lookup [1] the jprobes
> tests fail when built into a kernel compiled as Thumb mode. (They work
> fine for ARM mode kernels or for Thumb when built as a loadable module.)
>
> Rather than have this problem terminate testing prematurely lets instead
> emit an error message and carry on with the main kprobes tests, delaying
> the final failure report until the end.
>
> [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2011-August/063026.html
>
> Signed-off-by: Jon Medhurst <tixy@linaro.org>
> ---
>   arch/arm/kernel/kprobes-test.c |   10 ++++++++++
>   1 file changed, 10 insertions(+)
>
> diff --git a/arch/arm/kernel/kprobes-test.c b/arch/arm/kernel/kprobes-test.c
> index c2fd06b..6de5e94 100644
> --- a/arch/arm/kernel/kprobes-test.c
> +++ b/arch/arm/kernel/kprobes-test.c
> @@ -225,6 +225,7 @@ static int pre_handler_called;
>   static int post_handler_called;
>   static int jprobe_func_called;
>   static int kretprobe_handler_called;
> +static int tests_failed;
>
>   #define FUNC_ARG1 0x12345678
>   #define FUNC_ARG2 0xabcdef
> @@ -461,6 +462,13 @@ static int run_api_tests(long (*func)(long, long))
>
>   	pr_info("    jprobe\n");
>   	ret = test_jprobe(func);
> +#if defined(CONFIG_THUMB2_KERNEL) && !defined(MODULE)
> +	if (ret == -EINVAL) {
> +		pr_err("FAIL: Known longtime bug with jprobe on Thumb kernels\n");
> +		tests_failed = ret;
> +		ret = 0;
> +	}
> +#endif
>   	if (ret < 0)
>   		return ret;
>
> @@ -1671,6 +1679,8 @@ static int __init run_all_tests(void)
>
>   out:
>   	if (ret == 0)
> +		ret = tests_failed;
> +	if (ret == 0)
>   		pr_info("Finished kprobe tests OK\n");
>   	else
>   		pr_err("kprobe tests failed\n");
>

Tixy,

Sorry for the delay in responding.

Because this (correctly) was marked as having no interdependencies it 
got marked as "applied" in the patch-tracker.  That is why I omitted it 
from my v7 patches (although I should have just submitted it separately 
in the first place).  I don't see it in the linux-arm tree though, so I 
guess we do still need it.  If you haven't yet you might want to double 
check that.

I'm happy ack'ing this (if that's useful to you :-).

-dl

[PATCH 1/3] ARM: kprobes: Prevent known test failures stopping other tests running

[Jon Medhurst (Tixy)]

On Mon, 2014-03-24 at 11:18 -0400, David Long wrote:
> On 03/11/14 12:54, Jon Medhurst wrote:
> > Due to a long-standing issue with Thumb symbol lookup [1] the jprobes
> > tests fail when built into a kernel compiled as Thumb mode. (They work
> > fine for ARM mode kernels or for Thumb when built as a loadable module.)
> >
> > Rather than have this problem terminate testing prematurely lets instead
> > emit an error message and carry on with the main kprobes tests, delaying
> > the final failure report until the end.
> >
> > [1] http://lists.infradead.org/pipermail/linux-arm-kernel/2011-August/063026.html
> >
> > Signed-off-by: Jon Medhurst <tixy@linaro.org>
> > ---
> >   arch/arm/kernel/kprobes-test.c |   10 ++++++++++
> >   1 file changed, 10 insertions(+)
> >
> > diff --git a/arch/arm/kernel/kprobes-test.c b/arch/arm/kernel/kprobes-test.c
> > index c2fd06b..6de5e94 100644
> > --- a/arch/arm/kernel/kprobes-test.c
> > +++ b/arch/arm/kernel/kprobes-test.c
> > @@ -225,6 +225,7 @@ static int pre_handler_called;
> >   static int post_handler_called;
> >   static int jprobe_func_called;
> >   static int kretprobe_handler_called;
> > +static int tests_failed;
> >
> >   #define FUNC_ARG1 0x12345678
> >   #define FUNC_ARG2 0xabcdef
> > @@ -461,6 +462,13 @@ static int run_api_tests(long (*func)(long, long))
> >
> >   	pr_info("    jprobe\n");
> >   	ret = test_jprobe(func);
> > +#if defined(CONFIG_THUMB2_KERNEL) && !defined(MODULE)
> > +	if (ret == -EINVAL) {
> > +		pr_err("FAIL: Known longtime bug with jprobe on Thumb kernels\n");
> > +		tests_failed = ret;
> > +		ret = 0;
> > +	}
> > +#endif
> >   	if (ret < 0)
> >   		return ret;
> >
> > @@ -1671,6 +1679,8 @@ static int __init run_all_tests(void)
> >
> >   out:
> >   	if (ret == 0)
> > +		ret = tests_failed;
> > +	if (ret == 0)
> >   		pr_info("Finished kprobe tests OK\n");
> >   	else
> >   		pr_err("kprobe tests failed\n");
> >
[...]
> Because this (correctly) was marked as having no interdependencies it 
> got marked as "applied" in the patch-tracker.  That is why I omitted it 
> from my v7 patches.

Yes, I now see this in Russell's patch tracker (with a less verbose
commit message) as patch 7978, and it's marked as "Applied to git-curr
(uprobes branch)". Perhaps it got dropped along with the rest of the
uprobes branch when the config problems were discovered, Russell?

I have two other kprobes test fixes in this patches series, and was
planning on sending a pull request, or submitting to the patch tracker,
after the merge window has closed, so they can head into 3.16. (I posted
these a bit late for 3.15 and as the bugs they fix have been there since
day one I didn't think it warranted expediting).

-- 
Tixy

[PATCH 1/3] ARM: kprobes: Prevent known test failures stopping other tests running

[Russell King - ARM Linux]

On Mon, Mar 24, 2014 at 04:49:05PM +0000, Jon Medhurst (Tixy) wrote:
> Yes, I now see this in Russell's patch tracker (with a less verbose
> commit message) as patch 7978, and it's marked as "Applied to git-curr
> (uprobes branch)". Perhaps it got dropped along with the rest of the
> uprobes branch when the config problems were discovered, Russell?

If it was applied to the uprobes branch, and it wasn't part of David's
uprobes git pull request, then yes, it doesn't exist anymore.

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH 1/3] ARM: kprobes: Prevent known test failures stopping other tests running

[David Long]

On 03/24/14 12:56, Russell King - ARM Linux wrote:
> On Mon, Mar 24, 2014 at 04:49:05PM +0000, Jon Medhurst (Tixy) wrote:
>> Yes, I now see this in Russell's patch tracker (with a less verbose
>> commit message) as patch 7978, and it's marked as "Applied to git-curr
>> (uprobes branch)". Perhaps it got dropped along with the rest of the
>> uprobes branch when the config problems were discovered, Russell?
>
> If it was applied to the uprobes branch, and it wasn't part of David's
> uprobes git pull request, then yes, it doesn't exist anymore.
>

OK.  Just to be absolutely clear:  this doesn't create any new problem, 
but we do then need it in Tixy's new patchset in order to fix this older 
bug.

-dl

[PATCH 1/3] ARM: kprobes: Prevent known test failures stopping other tests running

[Russell King - ARM Linux]

On Mon, Mar 24, 2014 at 02:34:01PM -0400, David Long wrote:
> On 03/24/14 12:56, Russell King - ARM Linux wrote:
>> On Mon, Mar 24, 2014 at 04:49:05PM +0000, Jon Medhurst (Tixy) wrote:
>>> Yes, I now see this in Russell's patch tracker (with a less verbose
>>> commit message) as patch 7978, and it's marked as "Applied to git-curr
>>> (uprobes branch)". Perhaps it got dropped along with the rest of the
>>> uprobes branch when the config problems were discovered, Russell?
>>
>> If it was applied to the uprobes branch, and it wasn't part of David's
>> uprobes git pull request, then yes, it doesn't exist anymore.
>>
>
> OK.  Just to be absolutely clear:  this doesn't create any new problem,  
> but we do then need it in Tixy's new patchset in order to fix this older  
> bug.

Okay, so I'll just wait for Tixy's patch set then?

-- 
FTTC broadband for 0.8mile line: now at 9.7Mbps down 460kbps up... slowly
improving, and getting towards what was expected from it.

[PATCH 1/3] ARM: kprobes: Prevent known test failures stopping other tests running

[David Long]

On 03/25/14 10:02, Russell King - ARM Linux wrote:
> On Mon, Mar 24, 2014 at 02:34:01PM -0400, David Long wrote:
>> On 03/24/14 12:56, Russell King - ARM Linux wrote:
>>> On Mon, Mar 24, 2014 at 04:49:05PM +0000, Jon Medhurst (Tixy) wrote:
>>>> Yes, I now see this in Russell's patch tracker (with a less verbose
>>>> commit message) as patch 7978, and it's marked as "Applied to git-curr
>>>> (uprobes branch)". Perhaps it got dropped along with the rest of the
>>>> uprobes branch when the config problems were discovered, Russell?
>>>
>>> If it was applied to the uprobes branch, and it wasn't part of David's
>>> uprobes git pull request, then yes, it doesn't exist anymore.
>>>
>>
>> OK.  Just to be absolutely clear:  this doesn't create any new problem,
>> but we do then need it in Tixy's new patchset in order to fix this older
>> bug.
>
> Okay, so I'll just wait for Tixy's patch set then?
>

You don't have to delay anything, just eventually (e.g.: v3.16) take 
Tixy's patch as  a separate patch fixing this old bug.  The uprobes 
stuff is fine just as it is.  There are no interdependencies.

-dl

[PATCH 1/3] ARM: kprobes: Prevent known test failures stopping other tests running

[Jon Medhurst (Tixy)]

On Tue, 2014-03-25 at 10:08 -0400, David Long wrote:
> On 03/25/14 10:02, Russell King - ARM Linux wrote:
> > On Mon, Mar 24, 2014 at 02:34:01PM -0400, David Long wrote:
> >> On 03/24/14 12:56, Russell King - ARM Linux wrote:
> >>> On Mon, Mar 24, 2014 at 04:49:05PM +0000, Jon Medhurst (Tixy) wrote:
> >>>> Yes, I now see this in Russell's patch tracker (with a less verbose
> >>>> commit message) as patch 7978, and it's marked as "Applied to git-curr
> >>>> (uprobes branch)". Perhaps it got dropped along with the rest of the
> >>>> uprobes branch when the config problems were discovered, Russell?
> >>>
> >>> If it was applied to the uprobes branch, and it wasn't part of David's
> >>> uprobes git pull request, then yes, it doesn't exist anymore.
> >>>
> >>
> >> OK.  Just to be absolutely clear:  this doesn't create any new problem,
> >> but we do then need it in Tixy's new patchset in order to fix this older
> >> bug.
> >
> > Okay, so I'll just wait for Tixy's patch set then?
> >
> 
> You don't have to delay anything, just eventually (e.g.: v3.16) take 
> Tixy's patch as  a separate patch fixing this old bug.  The uprobes 
> stuff is fine just as it is.  There are no interdependencies.

In case you are talking at crossed purposes. The uprobes code which
Russell has already pulled into his tree, and is in linux-next, is fine,
nothing need changing, and is safe to go into 3.15.

The 3 patches I posted, which this is in reply to, are for fixes for
minor bugs in kprobes which have been there for 3 years, and can wait
for 3.16.

-- 
Tixy

[PATCH 2/3] ARM: kprobes: Disallow instructions with PC and register specified shift

[Jon Medhurst]

ARM data processing instructions which have a register specified shift
are defined as UNPREDICTABLE if PC is used for any register, not just
the shift value as the code was previous assuming. This issue manifests
on A15 devices as either test case failures or undefined instructions
aborts.

Reported-by: David Long <dave.long@linaro.org>
Signed-off-by: Jon Medhurst <tixy@linaro.org>
---
 arch/arm/kernel/kprobes-test-arm.c |   25 +++++++++++++------------
 arch/arm/kernel/probes-arm.c       |    6 +++---
 2 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/arch/arm/kernel/kprobes-test-arm.c b/arch/arm/kernel/kprobes-test-arm.c
index 87839de..8a7428b 100644
--- a/arch/arm/kernel/kprobes-test-arm.c
+++ b/arch/arm/kernel/kprobes-test-arm.c
@@ -73,12 +73,9 @@ void kprobe_arm_test_cases(void)
 	TEST_RRR( op "lt" s "	r11, r",11,VAL1,", r",14,N(val),", asr r",7, 6,"")\
 	TEST_RR(  op "gt" s "	r12, r13"       ", r",14,val, ", ror r",14,7,"")\
 	TEST_RR(  op "le" s "	r14, r",0, val, ", r13"       ", lsl r",14,8,"")\
-	TEST_RR(  op s "	r12, pc"        ", r",14,val, ", ror r",14,7,"")\
-	TEST_RR(  op s "	r14, r",0, val, ", pc"        ", lsl r",14,8,"")\
 	TEST_R(   op "eq" s "	r0,  r",11,VAL1,", #0xf5")			\
 	TEST_R(   op "ne" s "	r11, r",0, VAL1,", #0xf5000000")		\
-	TEST_R(   op s "	r7,  r",8, VAL2,", #0x000af000")		\
-	TEST(     op s "	r4,  pc"        ", #0x00005a00")
+	TEST_R(   op s "	r7,  r",8, VAL2,", #0x000af000")
 
 #define DATA_PROCESSING_DNM(op,val)		\
 	_DATA_PROCESSING_DNM(op,"",val)		\
@@ -102,8 +99,6 @@ void kprobe_arm_test_cases(void)
 	TEST_RRR( op "ge	r",11,VAL1,", r",14,N(val),", asr r",7, 6,"")	\
 	TEST_RR(  op "le	r13"       ", r",14,val, ", ror r",14,7,"")	\
 	TEST_RR(  op "gt	r",0, val, ", r13"       ", lsl r",14,8,"")	\
-	TEST_RR(  op "	pc"        ", r",14,val, ", ror r",14,7,"")		\
-	TEST_RR(  op "	r",0, val, ", pc"        ", lsl r",14,8,"")		\
 	TEST_R(   op "eq	r",11,VAL1,", #0xf5")				\
 	TEST_R(   op "ne	r",0, VAL1,", #0xf5000000")			\
 	TEST_R(   op "	r",8, VAL2,", #0x000af000")
@@ -124,7 +119,6 @@ void kprobe_arm_test_cases(void)
 	TEST_RR(  op "ge" s "	r11, r",11,N(val),", asr r",7, 6,"")	\
 	TEST_RR(  op "lt" s "	r12, r",11,val, ", ror r",14,7,"")	\
 	TEST_R(   op "gt" s "	r14, r13"       ", lsl r",14,8,"")	\
-	TEST_R(   op "le" s "	r14, pc"        ", lsl r",14,8,"")	\
 	TEST(     op "eq" s "	r0,  #0xf5")				\
 	TEST(     op "ne" s "	r11, #0xf5000000")			\
 	TEST(     op s "	r7,  #0x000af000")			\
@@ -158,12 +152,19 @@ void kprobe_arm_test_cases(void)
 	TEST_SUPPORTED("cmp	pc, #0x1000");
 	TEST_SUPPORTED("cmp	sp, #0x1000");
 
-	/* Data-processing with PC as shift*/
+	/* Data-processing with PC and a shift count in a register */
 	TEST_UNSUPPORTED(".word 0xe15c0f1e	@ cmp	r12, r14, asl pc")
 	TEST_UNSUPPORTED(".word 0xe1a0cf1e	@ mov	r12, r14, asl pc")
 	TEST_UNSUPPORTED(".word 0xe08caf1e	@ add	r10, r12, r14, asl pc")
-
-	/* Data-processing with PC as shift*/
+	TEST_UNSUPPORTED(".word 0xe151021f	@ cmp	r1, pc, lsl r2")
+	TEST_UNSUPPORTED(".word 0xe17f0211	@ cmn	pc, r1, lsl r2")
+	TEST_UNSUPPORTED(".word 0xe1a0121f	@ mov	r1, pc, lsl r2")
+	TEST_UNSUPPORTED(".word 0xe1a0f211	@ mov	pc, r1, lsl r2")
+	TEST_UNSUPPORTED(".word 0xe042131f	@ sub	r1, r2, pc, lsl r3")
+	TEST_UNSUPPORTED(".word 0xe1cf1312	@ bic	r1, pc, r2, lsl r3")
+	TEST_UNSUPPORTED(".word 0xe081f312	@ add	pc, r1, r2, lsl r3")
+
+	/* Data-processing with PC as a target a status registers updated */
 	TEST_UNSUPPORTED("movs	pc, r1")
 	TEST_UNSUPPORTED("movs	pc, r1, lsl r2")
 	TEST_UNSUPPORTED("movs	pc, #0x10000")
@@ -186,14 +187,14 @@ void kprobe_arm_test_cases(void)
 	TEST_BF_R ("add	pc, pc, r",14,2f-1f-8,"")
 	TEST_BF_R ("add	pc, r",14,2f-1f-8,", pc")
 	TEST_BF_R ("mov	pc, r",0,2f,"")
-	TEST_BF_RR("mov	pc, r",0,2f,", asl r",1,0,"")
+	TEST_BF_R ("add	pc, pc, r",14,(2f-1f-8)*2,", asr #1")
 	TEST_BB(   "sub	pc, pc, #1b-2b+8")
 #if __LINUX_ARM_ARCH__ == 6 && !defined(CONFIG_CPU_V7)
 	TEST_BB(   "sub	pc, pc, #1b-2b+8-2") /* UNPREDICTABLE before and after ARMv6 */
 #endif
 	TEST_BB_R( "sub	pc, pc, r",14, 1f-2f+8,"")
 	TEST_BB_R( "rsb	pc, r",14,1f-2f+8,", pc")
-	TEST_RR(   "add	pc, pc, r",10,-2,", asl r",11,1,"")
+	TEST_R(    "add	pc, pc, r",10,-2,", asl #1")
 #ifdef CONFIG_THUMB2_KERNEL
 	TEST_ARM_TO_THUMB_INTERWORK_R("add	pc, pc, r",0,3f-1f-8+1,"")
 	TEST_ARM_TO_THUMB_INTERWORK_R("sub	pc, r",0,3f+8+1,", #8")
diff --git a/arch/arm/kernel/probes-arm.c b/arch/arm/kernel/probes-arm.c
index 51a13a0..8eaef81 100644
--- a/arch/arm/kernel/probes-arm.c
+++ b/arch/arm/kernel/probes-arm.c
@@ -341,12 +341,12 @@ static const union decode_item arm_cccc_000x_table[] = {
 	/* CMP (reg-shift reg)	cccc 0001 0101 xxxx xxxx xxxx 0xx1 xxxx */
 	/* CMN (reg-shift reg)	cccc 0001 0111 xxxx xxxx xxxx 0xx1 xxxx */
 	DECODE_EMULATEX	(0x0f900090, 0x01100010, PROBES_DATA_PROCESSING_REG,
-						 REGS(ANY, 0, NOPC, 0, ANY)),
+						 REGS(NOPC, 0, NOPC, 0, NOPC)),
 
 	/* MOV (reg-shift reg)	cccc 0001 101x xxxx xxxx xxxx 0xx1 xxxx */
 	/* MVN (reg-shift reg)	cccc 0001 111x xxxx xxxx xxxx 0xx1 xxxx */
 	DECODE_EMULATEX	(0x0fa00090, 0x01a00010, PROBES_DATA_PROCESSING_REG,
-						 REGS(0, ANY, NOPC, 0, ANY)),
+						 REGS(0, NOPC, NOPC, 0, NOPC)),
 
 	/* AND (reg-shift reg)	cccc 0000 000x xxxx xxxx xxxx 0xx1 xxxx */
 	/* EOR (reg-shift reg)	cccc 0000 001x xxxx xxxx xxxx 0xx1 xxxx */
@@ -359,7 +359,7 @@ static const union decode_item arm_cccc_000x_table[] = {
 	/* ORR (reg-shift reg)	cccc 0001 100x xxxx xxxx xxxx 0xx1 xxxx */
 	/* BIC (reg-shift reg)	cccc 0001 110x xxxx xxxx xxxx 0xx1 xxxx */
 	DECODE_EMULATEX	(0x0e000090, 0x00000010, PROBES_DATA_PROCESSING_REG,
-						 REGS(ANY, ANY, NOPC, 0, ANY)),
+						 REGS(NOPC, NOPC, NOPC, 0, NOPC)),
 
 	DECODE_END
 };
-- 
1.7.10.4

[PATCH 2/3] ARM: kprobes: Disallow instructions with PC and register specified shift

[David Long]

On 03/11/14 12:54, Jon Medhurst wrote:
> ARM data processing instructions which have a register specified shift
> are defined as UNPREDICTABLE if PC is used for any register, not just
> the shift value as the code was previous assuming. This issue manifests
> on A15 devices as either test case failures or undefined instructions
> aborts.
>
> Reported-by: David Long <dave.long@linaro.org>
> Signed-off-by: Jon Medhurst <tixy@linaro.org>
> ---
>   arch/arm/kernel/kprobes-test-arm.c |   25 +++++++++++++------------
>   arch/arm/kernel/probes-arm.c       |    6 +++---
>   2 files changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/arch/arm/kernel/kprobes-test-arm.c b/arch/arm/kernel/kprobes-test-arm.c
> index 87839de..8a7428b 100644
> --- a/arch/arm/kernel/kprobes-test-arm.c
> +++ b/arch/arm/kernel/kprobes-test-arm.c
> @@ -73,12 +73,9 @@ void kprobe_arm_test_cases(void)
>   	TEST_RRR( op "lt" s "	r11, r",11,VAL1,", r",14,N(val),", asr r",7, 6,"")\
>   	TEST_RR(  op "gt" s "	r12, r13"       ", r",14,val, ", ror r",14,7,"")\
>   	TEST_RR(  op "le" s "	r14, r",0, val, ", r13"       ", lsl r",14,8,"")\
> -	TEST_RR(  op s "	r12, pc"        ", r",14,val, ", ror r",14,7,"")\
> -	TEST_RR(  op s "	r14, r",0, val, ", pc"        ", lsl r",14,8,"")\
>   	TEST_R(   op "eq" s "	r0,  r",11,VAL1,", #0xf5")			\
>   	TEST_R(   op "ne" s "	r11, r",0, VAL1,", #0xf5000000")		\
> -	TEST_R(   op s "	r7,  r",8, VAL2,", #0x000af000")		\
> -	TEST(     op s "	r4,  pc"        ", #0x00005a00")

The last two lines above confuse me.  Can you explain why those needed 
to be removed?  Is there somehow a shift involved with those instructions?

The rest looked OK to me.  I'm omitting it for the sake of brevity.

-dl

[PATCH 2/3] ARM: kprobes: Disallow instructions with PC and register specified shift

[Jon Medhurst (Tixy)]

On Mon, 2014-03-24 at 15:49 -0400, David Long wrote:
> On 03/11/14 12:54, Jon Medhurst wrote:
> > ARM data processing instructions which have a register specified shift
> > are defined as UNPREDICTABLE if PC is used for any register, not just
> > the shift value as the code was previous assuming. This issue manifests
> > on A15 devices as either test case failures or undefined instructions
> > aborts.
> >
> > Reported-by: David Long <dave.long@linaro.org>
> > Signed-off-by: Jon Medhurst <tixy@linaro.org>
> > ---
> >   arch/arm/kernel/kprobes-test-arm.c |   25 +++++++++++++------------
> >   arch/arm/kernel/probes-arm.c       |    6 +++---
> >   2 files changed, 16 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/arm/kernel/kprobes-test-arm.c b/arch/arm/kernel/kprobes-test-arm.c
> > index 87839de..8a7428b 100644
> > --- a/arch/arm/kernel/kprobes-test-arm.c
> > +++ b/arch/arm/kernel/kprobes-test-arm.c
> > @@ -73,12 +73,9 @@ void kprobe_arm_test_cases(void)
> >   	TEST_RRR( op "lt" s "	r11, r",11,VAL1,", r",14,N(val),", asr r",7, 6,"")\
> >   	TEST_RR(  op "gt" s "	r12, r13"       ", r",14,val, ", ror r",14,7,"")\
> >   	TEST_RR(  op "le" s "	r14, r",0, val, ", r13"       ", lsl r",14,8,"")\
> > -	TEST_RR(  op s "	r12, pc"        ", r",14,val, ", ror r",14,7,"")\
> > -	TEST_RR(  op s "	r14, r",0, val, ", pc"        ", lsl r",14,8,"")\
> >   	TEST_R(   op "eq" s "	r0,  r",11,VAL1,", #0xf5")			\
> >   	TEST_R(   op "ne" s "	r11, r",0, VAL1,", #0xf5000000")		\
> > -	TEST_R(   op s "	r7,  r",8, VAL2,", #0x000af000")		\
> > -	TEST(     op s "	r4,  pc"        ", #0x00005a00")
> 
> The last two lines above confuse me.  Can you explain why those needed 
> to be removed?  Is there somehow a shift involved with those instructions?
> 
> The rest looked OK to me.  I'm omitting it for the sake of brevity.

The next line in the patch was

+	TEST_R(   op s "	r7,  r",8, VAL2,", #0x000af000")

so the change actually only removed the last test case. However, as you
say, this doesn't involve a shift by a register and so shouldn't have
been removed by this patch, I'll fix that. Thanks for spotting the
error.

-- 
Tixy

[PATCH 3/3] ARM: kprobes: Fix test code compilation errors for ARMv4 targets

[Jon Medhurst]

Conditionally compile kprobes test cases for ARMv5 instructions to avoid
compilation errors with ARMv4 targets like:

/tmp/cc7Tx8ST.s:16740: Error: selected processor does not support ARM mode `clz r0,r0'

Signed-off-by: Jon Medhurst <tixy@linaro.org>
---
 arch/arm/kernel/kprobes-test-arm.c |    8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm/kernel/kprobes-test-arm.c b/arch/arm/kernel/kprobes-test-arm.c
index 8a7428b..f9ed7cf 100644
--- a/arch/arm/kernel/kprobes-test-arm.c
+++ b/arch/arm/kernel/kprobes-test-arm.c
@@ -216,6 +216,7 @@ void kprobe_arm_test_cases(void)
 	TEST_BB_R("bx	r",7,2f,"")
 	TEST_BF_R("bxeq	r",14,2f,"")
 
+#if __LINUX_ARM_ARCH__ >= 5
 	TEST_R("clz	r0, r",0, 0x0,"")
 	TEST_R("clzeq	r7, r",14,0x1,"")
 	TEST_R("clz	lr, r",7, 0xffffffff,"")
@@ -337,6 +338,7 @@ void kprobe_arm_test_cases(void)
 	TEST_UNSUPPORTED(".word 0xe16f02e1 @ smultt pc, r1, r2")
 	TEST_UNSUPPORTED(".word 0xe16002ef @ smultt r0, pc, r2")
 	TEST_UNSUPPORTED(".word 0xe1600fe1 @ smultt r0, r1, pc")
+#endif
 
 	TEST_GROUP("Multiply and multiply-accumulate")
 
@@ -559,6 +561,7 @@ void kprobe_arm_test_cases(void)
 	TEST_UNSUPPORTED("ldrsht	r1, [r2], #48")
 #endif
 
+#if __LINUX_ARM_ARCH__ >= 5
 	TEST_RPR(  "strd	r",0, VAL1,", [r",1, 48,", -r",2,24,"]")
 	TEST_RPR(  "strccd	r",8, VAL2,", [r",13,0, ", r",12,48,"]")
 	TEST_RPR(  "strd	r",4, VAL1,", [r",2, 24,", r",3, 48,"]!")
@@ -595,6 +598,7 @@ void kprobe_arm_test_cases(void)
 	TEST_UNSUPPORTED(".word 0xe1efc3d0	@ ldrd r12, [pc, #48]!")
 	TEST_UNSUPPORTED(".word 0xe0c9f3d0	@ ldrd pc, [r9], #48")
 	TEST_UNSUPPORTED(".word 0xe0c9e3d0	@ ldrd lr, [r9], #48")
+#endif
 
 	TEST_GROUP("Miscellaneous")
 
@@ -1227,7 +1231,9 @@ void kprobe_arm_test_cases(void)
 	TEST_COPROCESSOR( "mrc"two"	0, 0, r0, cr0, cr0, 0")
 
 	COPROCESSOR_INSTRUCTIONS_ST_LD("","e")
+#if __LINUX_ARM_ARCH__ >= 5
 	COPROCESSOR_INSTRUCTIONS_MC_MR("","e")
+#endif
 	TEST_UNSUPPORTED("svc	0")
 	TEST_UNSUPPORTED("svc	0xffffff")
 
@@ -1287,7 +1293,9 @@ void kprobe_arm_test_cases(void)
 	TEST(	"blx	__dummy_thumb_subroutine_odd")
 #endif /* __LINUX_ARM_ARCH__ >= 6 */
 
+#if __LINUX_ARM_ARCH__ >= 5
 	COPROCESSOR_INSTRUCTIONS_ST_LD("2","f")
+#endif
 #if __LINUX_ARM_ARCH__ >= 6
 	COPROCESSOR_INSTRUCTIONS_MC_MR("2","f")
 #endif
-- 
1.7.10.4

[PATCH 3/3] ARM: kprobes: Fix test code compilation errors for ARMv4 targets

[David Long]

On 03/11/14 12:54, Jon Medhurst wrote:
> Conditionally compile kprobes test cases for ARMv5 instructions to avoid
> compilation errors with ARMv4 targets like:
>
> /tmp/cc7Tx8ST.s:16740: Error: selected processor does not support ARM mode `clz r0,r0'
>
> Signed-off-by: Jon Medhurst <tixy@linaro.org>
> ---
>   arch/arm/kernel/kprobes-test-arm.c |    8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/arch/arm/kernel/kprobes-test-arm.c b/arch/arm/kernel/kprobes-test-arm.c
> index 8a7428b..f9ed7cf 100644
> --- a/arch/arm/kernel/kprobes-test-arm.c
> +++ b/arch/arm/kernel/kprobes-test-arm.c
> @@ -216,6 +216,7 @@ void kprobe_arm_test_cases(void)
>   	TEST_BB_R("bx	r",7,2f,"")
>   	TEST_BF_R("bxeq	r",14,2f,"")
>
> +#if __LINUX_ARM_ARCH__ >= 5
>   	TEST_R("clz	r0, r",0, 0x0,"")
>   	TEST_R("clzeq	r7, r",14,0x1,"")
>   	TEST_R("clz	lr, r",7, 0xffffffff,"")
> @@ -337,6 +338,7 @@ void kprobe_arm_test_cases(void)
>   	TEST_UNSUPPORTED(".word 0xe16f02e1 @ smultt pc, r1, r2")
>   	TEST_UNSUPPORTED(".word 0xe16002ef @ smultt r0, pc, r2")
>   	TEST_UNSUPPORTED(".word 0xe1600fe1 @ smultt r0, r1, pc")
> +#endif
>
>   	TEST_GROUP("Multiply and multiply-accumulate")
>
> @@ -559,6 +561,7 @@ void kprobe_arm_test_cases(void)
>   	TEST_UNSUPPORTED("ldrsht	r1, [r2], #48")
>   #endif
>
> +#if __LINUX_ARM_ARCH__ >= 5
>   	TEST_RPR(  "strd	r",0, VAL1,", [r",1, 48,", -r",2,24,"]")
>   	TEST_RPR(  "strccd	r",8, VAL2,", [r",13,0, ", r",12,48,"]")
>   	TEST_RPR(  "strd	r",4, VAL1,", [r",2, 24,", r",3, 48,"]!")
> @@ -595,6 +598,7 @@ void kprobe_arm_test_cases(void)
>   	TEST_UNSUPPORTED(".word 0xe1efc3d0	@ ldrd r12, [pc, #48]!")
>   	TEST_UNSUPPORTED(".word 0xe0c9f3d0	@ ldrd pc, [r9], #48")
>   	TEST_UNSUPPORTED(".word 0xe0c9e3d0	@ ldrd lr, [r9], #48")
> +#endif
>
>   	TEST_GROUP("Miscellaneous")
>
> @@ -1227,7 +1231,9 @@ void kprobe_arm_test_cases(void)
>   	TEST_COPROCESSOR( "mrc"two"	0, 0, r0, cr0, cr0, 0")
>
>   	COPROCESSOR_INSTRUCTIONS_ST_LD("","e")
> +#if __LINUX_ARM_ARCH__ >= 5
>   	COPROCESSOR_INSTRUCTIONS_MC_MR("","e")
> +#endif
>   	TEST_UNSUPPORTED("svc	0")
>   	TEST_UNSUPPORTED("svc	0xffffff")
>
> @@ -1287,7 +1293,9 @@ void kprobe_arm_test_cases(void)
>   	TEST(	"blx	__dummy_thumb_subroutine_odd")
>   #endif /* __LINUX_ARM_ARCH__ >= 6 */
>
> +#if __LINUX_ARM_ARCH__ >= 5
>   	COPROCESSOR_INSTRUCTIONS_ST_LD("2","f")
> +#endif
>   #if __LINUX_ARM_ARCH__ >= 6
>   	COPROCESSOR_INSTRUCTIONS_MC_MR("2","f")
>   #endif
>

This looks OK to me.  Feel free to add my ack.

-dl

[PATCH 3/3] ARM: kprobes: Fix test code compilation errors for ARMv4 targets

[Arnd Bergmann]

On Tuesday 25 March 2014 09:27:29 David Long wrote:
> On 03/11/14 12:54, Jon Medhurst wrote:
> > Conditionally compile kprobes test cases for ARMv5 instructions to avoid
> > compilation errors with ARMv4 targets like:
> >
> > /tmp/cc7Tx8ST.s:16740: Error: selected processor does not support ARM mode `clz r0,r0'
> >
> > Signed-off-by: Jon Medhurst <tixy@linaro.org>
> 
> This looks OK to me.  Feel free to add my ack.

Ah, I had a similar patch in my 'randconfig-fixes' series. I noticed three
other configurations that are broken with kprobes-test:

- ARMv3 (enabled by ARCH_RPC)
- ARMv7-M (enabled by ARCH_EFM32)
- CPU_ENDIAN_BE32 (enabled by building a big-endian kernel on ARMv5 or older)

Should we treat those the same way, or just disable Kprobes for this case
if nobody cares?

	Arnd

[PATCH 3/3] ARM: kprobes: Fix test code compilation errors for ARMv4 targets

[Jon Medhurst (Tixy)]

On Tue, 2014-03-25 at 14:42 +0100, Arnd Bergmann wrote:
> On Tuesday 25 March 2014 09:27:29 David Long wrote:
> > On 03/11/14 12:54, Jon Medhurst wrote:
> > > Conditionally compile kprobes test cases for ARMv5 instructions to avoid
> > > compilation errors with ARMv4 targets like:
> > >
> > > /tmp/cc7Tx8ST.s:16740: Error: selected processor does not support ARM mode `clz r0,r0'
> > >
> > > Signed-off-by: Jon Medhurst <tixy@linaro.org>
> > 
> > This looks OK to me.  Feel free to add my ack.
> 
> Ah, I had a similar patch in my 'randconfig-fixes' series.

Where's that?

>  I noticed three
> other configurations that are broken with kprobes-test:
> 
> - ARMv3 (enabled by ARCH_RPC)

Didn't know we support < ARMv4!

> - ARMv7-M (enabled by ARCH_EFM32)

I guess that problem is because randconfig is enabling HAVE_KPROBES,
even though it wouldn't normally be selected, because we have

   config ARCH_ARM
	select HAVE_KPROBES if !XIP_KERNEL

and ARCH_EFM32 is XIP_KERNEL.

I think this problem goes all the way back to the commit which added
HAVE_KPROBES to all arches (3f550096dede4430f83b16457da83bf429155ac2)
That replaced 

   config KPROBES
	depends on ... (ARM && !XIP_KERNEL)

with the current select if !XIP_KERNEL, which made it possible to enable
kprobes for XIP when before we couldn't. And presumably that was for a
good reason like it doesn't work on XIP?

Not sure at the moment how best to fix that. (Making ARM_KPROBES_TEST
depend on !XIP_KERNEL doesn't solve the underlying problem of KPROBES
feature still being enabled for XIP kernels.)

> - CPU_ENDIAN_BE32 (enabled by building a big-endian kernel on ARMv5 or older)

> Should we treat those the same way, or just disable Kprobes for this case
> if nobody cares?

For CPU_ENDIAN_BE32 I have a feeling that the kprobes code wouldn't
work, would have to think more about why I have that feeling. If it
doesn't, we come back to the problem that arch code can't add
dependencies to CONFIG_KPROBES as things stand.

-- 
Tixy

[PATCH 3/3] ARM: kprobes: Fix test code compilation errors for ARMv4 targets

[Arnd Bergmann]

On Tuesday 25 March 2014 14:54:44 Jon Medhurst wrote:
> On Tue, 2014-03-25 at 14:42 +0100, Arnd Bergmann wrote:
> > On Tuesday 25 March 2014 09:27:29 David Long wrote:
> > > On 03/11/14 12:54, Jon Medhurst wrote:
> > > > Conditionally compile kprobes test cases for ARMv5 instructions to avoid
> > > > compilation errors with ARMv4 targets like:
> > > >
> > > > /tmp/cc7Tx8ST.s:16740: Error: selected processor does not support ARM mode `clz r0,r0'
> > > >
> > > > Signed-off-by: Jon Medhurst <tixy@linaro.org>
> > > 
> > > This looks OK to me.  Feel free to add my ack.
> > 
> > Ah, I had a similar patch in my 'randconfig-fixes' series.
> 
> Where's that?

http://git.kernel.org/cgit/linux/kernel/git/arnd/playground.git/commit/?h=arm-randconfig

> >  I noticed three
> > other configurations that are broken with kprobes-test:
> > 
> > - ARMv3 (enabled by ARCH_RPC)
> 
> Didn't know we support < ARMv4!

We don't really (any more). RPC is a v4 CPU, but we build the kernel
pretending it is v3 to work around some issue I don't remember.

> > - ARMv7-M (enabled by ARCH_EFM32)
> 
> I guess that problem is because randconfig is enabling HAVE_KPROBES,
> even though it wouldn't normally be selected, because we have
> 
>    config ARCH_ARM
> 	select HAVE_KPROBES if !XIP_KERNEL
> 
> and ARCH_EFM32 is XIP_KERNEL.

Right. EFM32 can in theory run without XIP_KERNEL, but the only
board we support doesn't have enough RAM for that.

> I think this problem goes all the way back to the commit which added
> HAVE_KPROBES to all arches (3f550096dede4430f83b16457da83bf429155ac2)
> That replaced 
> 
>    config KPROBES
> 	depends on ... (ARM && !XIP_KERNEL)
> 
> with the current select if !XIP_KERNEL, which made it possible to enable
> kprobes for XIP when before we couldn't. And presumably that was for a
> good reason like it doesn't work on XIP?

The text segment is very read-only in XIP_KERNEL, so anything that tries
to overwrite instructions can't work.

> Not sure at the moment how best to fix that. (Making ARM_KPROBES_TEST
> depend on !XIP_KERNEL doesn't solve the underlying problem of KPROBES
> feature still being enabled for XIP kernels.)

Regardless of XIP, there is the other problem of ARMv7-M support, which
in particular only has THUMB2 instructions:

  CC      arch/arm/kernel/kprobes.o
/tmp/cclbjgU2.s: Assembler messages:
	/tmp/cclbjgU2.s:701: Error: selected processor does not support Thumb mode `rfeia sp!'
make[3]: *** [arch/arm/kernel/kprobes.o] Error 1
  CC      arch/arm/kernel/kprobes-thumb.o
/tmp/ccwlnR1o.s: Assembler messages:
/tmp/ccwlnR1o.s:315: Error: selected processor does not support requested special purpose register -- `msr cpsr_fs,r0'
/tmp/ccwlnR1o.s:317: Error: selected processor does not support requested special purpose register -- `mrs r0,cpsr'
/tmp/ccwlnR1o.s:673: Error: selected processor does not support requested special purpose register -- `msr cpsr_fs,r3'
/tmp/ccwlnR1o.s:675: Error: selected processor does not support requested special purpose register -- `mrs r3,cpsr'
/tmp/ccwlnR1o.s:919: Error: selected processor does not support requested special purpose register -- `msr cpsr_fs,r8'
/tmp/ccwlnR1o.s:923: Error: selected processor does not support requested special purpose register -- `mrs r9,cpsr'
/tmp/ccwlnR1o.s:960: Error: selected processor does not support requested special purpose register -- `msr cpsr_fs,r8'
/tmp/ccwlnR1o.s:964: Error: selected processor does not support requested special purpose register -- `mrs r9,cpsr'
make[3]: *** [arch/arm/kernel/kprobes-thumb.o] Error 1
  CC      arch/arm/kernel/kprobes-test.o
/tmp/ccVIP6jA.s: Assembler messages:
/tmp/ccVIP6jA.s:1354: Error: selected processor does not support ARM opcodes
/tmp/ccVIP6jA.s:1355: Error: attempt to use an ARM instruction on a Thumb-only processor -- `orr lr,lr,#1'
/tmp/ccVIP6jA.s:1356: Error: attempt to use an ARM instruction on a Thumb-only processor -- `ldr pc,1f'
make[3]: *** [arch/arm/kernel/kprobes-test.o] Error 1
  CC      arch/arm/kernel/kprobes-test-thumb.o
/tmp/ccN8WTB4.s: Assembler messages:
/tmp/ccN8WTB4.s:2032: Error: selected processor does not support ARM opcodes
/tmp/ccN8WTB4.s:2033: Error: attempt to use an ARM instruction on a Thumb-only processor -- `adr lr,2f+1'
/tmp/ccN8WTB4.s:2034: Error: attempt to use an ARM instruction on a Thumb-only processor -- `bx lr'
/tmp/ccN8WTB4.s:4183: Error: selected processor does not support ARM opcodes
/tmp/ccN8WTB4.s:4184: Error: attempt to use an ARM instruction on a Thumb-only processor -- `adr lr,2f+1'
/tmp/ccN8WTB4.s:4185: Error: attempt to use an ARM instruction on a Thumb-only processor -- `bx lr'
/tmp/ccN8WTB4.s:4222: Error: selected processor does not support ARM opcodes
/tmp/ccN8WTB4.s:4223: Error: attempt to use an ARM instruction on a Thumb-only processor -- `adr lr,2f+1'
/tmp/ccN8WTB4.s:4224: Error: attempt to use an ARM instruction on a Thumb-only processor -- `bx lr'
/tmp/ccN8WTB4.s:5618: Error: selected processor does not support Thumb mode `rfedb sp'
/tmp/ccN8WTB4.s:5639: Error: selected processor does not support Thumb mode `rfeia sp'
/tmp/ccN8WTB4.s:5660: Error: selected processor does not support Thumb mode `rfedb sp!'
/tmp/ccN8WTB4.s:5681: Error: selected processor does not support Thumb mode `rfeia sp!'
/tmp/ccN8WTB4.s:6489: Error: selected processor does not support ARM opcodes
/tmp/ccN8WTB4.s:6490: Error: attempt to use an ARM instruction on a Thumb-only processor -- `adr lr,2f+1'
/tmp/ccN8WTB4.s:6491: Error: attempt to use an ARM instruction on a Thumb-only processor -- `bx lr'
/tmp/ccN8WTB4.s:6528: Error: selected processor does not support ARM opcodes
/tmp/ccN8WTB4.s:6529: Error: attempt to use an ARM instruction on a Thumb-only processor -- `adr lr,2f+1'
/tmp/ccN8WTB4.s:6530: Error: attempt to use an ARM instruction on a Thumb-only processor -- `bx lr'
/tmp/ccN8WTB4.s:7521: Error: selected processor does not support Thumb mode `strexd r0,r1,[r2]'
/tmp/ccN8WTB4.s:7584: Error: selected processor does not support Thumb mode `ldrexd r0,[r1]'
/tmp/ccN8WTB4.s:15321: Error: selected processor does not support Thumb mode `pkhbt r0,r0,r1'
/tmp/ccN8WTB4.s:15350: Error: selected processor does not support Thumb mode `pkhbt r14,r12,r10,lsl#2'
/tmp/ccN8WTB4.s:15379: Error: selected processor does not support Thumb mode `pkhtb r0,r0,r1'
/tmp/ccN8WTB4.s:15408: Error: selected processor does not support Thumb mode `pkhtb r14,r12,r10,asr#2'
/tmp/ccN8WTB4.s:18355: Error: selected processor does not support Thumb mode `ssat16 r0,#12,r0'

> > - CPU_ENDIAN_BE32 (enabled by building a big-endian kernel on ARMv5 or older)
> 
> > Should we treat those the same way, or just disable Kprobes for this case
> > if nobody cares?
> 
> For CPU_ENDIAN_BE32 I have a feeling that the kprobes code wouldn't
> work, would have to think more about why I have that feeling. If it
> doesn't, we come back to the problem that arch code can't add
> dependencies to CONFIG_KPROBES as things stand.

The error message I get here is

arnd at wuerfel:~/arm-soc$ make O=build/0xCD5BF01B_defconfig/  -skj40
/git/arm-soc/Makefile:629: Cannot use CONFIG_CC_STACKPROTECTOR_STRONG: -fstack-protector-strong not supported by compiler
/git/arm-soc/arch/arm/kernel/patch.c: In function '__patch_text':
/git/arm-soc/arch/arm/kernel/patch.c:35:4: error: implicit declaration of function '__opcode_to_mem_thumb32' [-Werror=implicit-function-declaration]
    insn = __opcode_to_mem_thumb32(insn);
    ^


I've filed that for now in the "wontfix" category, but I though I'd bring
it up anyway.

	Arnd