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 +++---
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.htm...
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");
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.htm...
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
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.htm...
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).
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.
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
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?
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
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.
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 };
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
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.
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
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
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
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.
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=ar...
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@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
linaro-kernel@lists.linaro.org
-
Arnd Bergmann -
David Long -
Jon Medhurst -
Jon Medhurst (Tixy)
-
Russell King - ARM Linux