* [PATCH] kprobes: only test 'sub pc, pc, #1b-2b+8-2' on ARMv6
@ 2012-06-12 12:07 Rabin Vincent
2012-06-13 7:18 ` Tixy
0 siblings, 1 reply; 6+ messages in thread
From: Rabin Vincent @ 2012-06-12 12:07 UTC (permalink / raw)
To: linux-arm-kernel
'sub pc, pc, #1b-2b+8-2' results in address<1:0> == '10'.
sub pc, pc, #const (== ADR pc, #const) performs an interworking branch
(BXWritePC()) on ARMv7+ and a simple branch (BranchWritePC()) on earlier
versions.
In ARM state, BXWritePC() is UNPREDICTABLE when address<1:0> == '10'.
In ARM state on ARMv6+, BranchWritePC() ignores address<1:0>. Before
ARMv6, BranchWritePC() is UNPREDICTABLE if address<1:0> != '00'
So the instruction is UNPREDICTABLE both before and after v6.
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
arch/arm/kernel/kprobes-test-arm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/kernel/kprobes-test-arm.c b/arch/arm/kernel/kprobes-test-arm.c
index ba32b39..6c926d5 100644
--- a/arch/arm/kernel/kprobes-test-arm.c
+++ b/arch/arm/kernel/kprobes-test-arm.c
@@ -187,8 +187,8 @@ void kprobe_arm_test_cases(void)
TEST_BF_R ("mov pc, r",0,2f,"")
TEST_BF_RR("mov pc, r",0,2f,", asl r",1,0,"")
TEST_BB( "sub pc, pc, #1b-2b+8")
-#if __LINUX_ARM_ARCH__ >= 6
- TEST_BB( "sub pc, pc, #1b-2b+8-2") /* UNPREDICTABLE before ARMv6 */
+#if __LINUX_ARM_ARCH__ == 6
+ 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")
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] kprobes: only test 'sub pc, pc, #1b-2b+8-2' on ARMv6
2012-06-12 12:07 [PATCH] kprobes: only test 'sub pc, pc, #1b-2b+8-2' on ARMv6 Rabin Vincent
@ 2012-06-13 7:18 ` Tixy
2012-06-15 10:03 ` Rabin Vincent
0 siblings, 1 reply; 6+ messages in thread
From: Tixy @ 2012-06-13 7:18 UTC (permalink / raw)
To: linux-arm-kernel
On Tue, 2012-06-12 at 17:37 +0530, Rabin Vincent wrote:
> 'sub pc, pc, #1b-2b+8-2' results in address<1:0> == '10'.
>
> sub pc, pc, #const (== ADR pc, #const) performs an interworking branch
> (BXWritePC()) on ARMv7+ and a simple branch (BranchWritePC()) on earlier
> versions.
>
> In ARM state, BXWritePC() is UNPREDICTABLE when address<1:0> == '10'.
>
> In ARM state on ARMv6+, BranchWritePC() ignores address<1:0>. Before
> ARMv6, BranchWritePC() is UNPREDICTABLE if address<1:0> != '00'
>
> So the instruction is UNPREDICTABLE both before and after v6.
I agree with this analysis. However, it is possible to have a kernel
built to support both ARM v6 and v7 (e.g OMAP2+) in which case
__LINUX_ARM_ARCH__ == 6 will be true but the code could be running on v7
hardware. Therefore a compile time check for ARM architecture isn't
sufficient to fix the problem.
I can only suggest to remove the test case. That feels a bit wrong, but
I'm not sure it's worth the effort to get the test code infrastructure
to support runtime checks for architecture version.
--
Tixy
> Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
> ---
> arch/arm/kernel/kprobes-test-arm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/kernel/kprobes-test-arm.c b/arch/arm/kernel/kprobes-test-arm.c
> index ba32b39..6c926d5 100644
> --- a/arch/arm/kernel/kprobes-test-arm.c
> +++ b/arch/arm/kernel/kprobes-test-arm.c
> @@ -187,8 +187,8 @@ void kprobe_arm_test_cases(void)
> TEST_BF_R ("mov pc, r",0,2f,"")
> TEST_BF_RR("mov pc, r",0,2f,", asl r",1,0,"")
> TEST_BB( "sub pc, pc, #1b-2b+8")
> -#if __LINUX_ARM_ARCH__ >= 6
> - TEST_BB( "sub pc, pc, #1b-2b+8-2") /* UNPREDICTABLE before ARMv6 */
> +#if __LINUX_ARM_ARCH__ == 6
> + 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")
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] kprobes: only test 'sub pc, pc, #1b-2b+8-2' on ARMv6
2012-06-13 7:18 ` Tixy
@ 2012-06-15 10:03 ` Rabin Vincent
2012-06-15 13:36 ` Tixy
0 siblings, 1 reply; 6+ messages in thread
From: Rabin Vincent @ 2012-06-15 10:03 UTC (permalink / raw)
To: linux-arm-kernel
On Wed, Jun 13, 2012 at 12:48 PM, Tixy <tixy@yxit.co.uk> wrote:
> I agree with this analysis. However, it is possible to have a kernel
> built to support both ARM v6 and v7 (e.g OMAP2+) in which case
> __LINUX_ARM_ARCH__ == 6 will be true but the code could be running on v7
> hardware. Therefore a compile time check for ARM architecture isn't
> sufficient to fix the problem.
How about the following check instead? At least one other place uses
it.
8<---
>From f1e1794c0c12e39e99d4169c934fd43feb889d7b Mon Sep 17 00:00:00 2001
From: Rabin Vincent <rabin.vincent@stericsson.com>
Date: Mon, 11 Jun 2012 16:55:37 +0530
Subject: [PATCH] ARM: kprobes: only test 'sub pc, pc, #1b-2b+8-2' on ARMv6
'sub pc, pc, #1b-2b+8-2' results in address<1:0> == '10'.
sub pc, pc, #const (== ADR pc, #const) performs an interworking branch
(BXWritePC()) on ARMv7+ and a simple branch (BranchWritePC()) on earlier
versions.
In ARM state, BXWritePC() is UNPREDICTABLE when address<1:0> == '10'.
In ARM state on ARMv6+, BranchWritePC() ignores address<1:0>. Before
ARMv6, BranchWritePC() is UNPREDICTABLE if address<1:0> != '00'
So the instruction is UNPREDICTABLE both before and after v6.
Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
---
arch/arm/kernel/kprobes-test-arm.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/arm/kernel/kprobes-test-arm.c
b/arch/arm/kernel/kprobes-test-arm.c
index ba32b39..38c1a3b 100644
--- a/arch/arm/kernel/kprobes-test-arm.c
+++ b/arch/arm/kernel/kprobes-test-arm.c
@@ -187,8 +187,8 @@ void kprobe_arm_test_cases(void)
TEST_BF_R ("mov pc, r",0,2f,"")
TEST_BF_RR("mov pc, r",0,2f,", asl r",1,0,"")
TEST_BB( "sub pc, pc, #1b-2b+8")
-#if __LINUX_ARM_ARCH__ >= 6
- TEST_BB( "sub pc, pc, #1b-2b+8-2") /* UNPREDICTABLE before ARMv6 */
+#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")
--
1.7.9.5
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH] kprobes: only test 'sub pc, pc, #1b-2b+8-2' on ARMv6
2012-06-15 10:03 ` Rabin Vincent
@ 2012-06-15 13:36 ` Tixy
2012-06-15 14:00 ` Rabin Vincent
0 siblings, 1 reply; 6+ messages in thread
From: Tixy @ 2012-06-15 13:36 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 2012-06-15 at 15:33 +0530, Rabin Vincent wrote:
> On Wed, Jun 13, 2012 at 12:48 PM, Tixy <tixy@yxit.co.uk> wrote:
> > I agree with this analysis. However, it is possible to have a kernel
> > built to support both ARM v6 and v7 (e.g OMAP2+) in which case
> > __LINUX_ARM_ARCH__ == 6 will be true but the code could be running on v7
> > hardware. Therefore a compile time check for ARM architecture isn't
> > sufficient to fix the problem.
>
> How about the following check instead? At least one other place uses
> it.
Can't you point out the other place which uses it, I couldn't find it.
The proposed patch below would work, but feels slightly off as it
assumes that no one will build a kernel to support V6 and V8, but
without V7. That does seem highly unlikely though, so I personally
wouldn't object to the patch.
--
Tixy
>
> 8<---
> From f1e1794c0c12e39e99d4169c934fd43feb889d7b Mon Sep 17 00:00:00 2001
> From: Rabin Vincent <rabin.vincent@stericsson.com>
> Date: Mon, 11 Jun 2012 16:55:37 +0530
> Subject: [PATCH] ARM: kprobes: only test 'sub pc, pc, #1b-2b+8-2' on ARMv6
>
> 'sub pc, pc, #1b-2b+8-2' results in address<1:0> == '10'.
>
> sub pc, pc, #const (== ADR pc, #const) performs an interworking branch
> (BXWritePC()) on ARMv7+ and a simple branch (BranchWritePC()) on earlier
> versions.
>
> In ARM state, BXWritePC() is UNPREDICTABLE when address<1:0> == '10'.
>
> In ARM state on ARMv6+, BranchWritePC() ignores address<1:0>. Before
> ARMv6, BranchWritePC() is UNPREDICTABLE if address<1:0> != '00'
>
> So the instruction is UNPREDICTABLE both before and after v6.
>
> Signed-off-by: Rabin Vincent <rabin.vincent@stericsson.com>
> ---
> arch/arm/kernel/kprobes-test-arm.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/kernel/kprobes-test-arm.c
> b/arch/arm/kernel/kprobes-test-arm.c
> index ba32b39..38c1a3b 100644
> --- a/arch/arm/kernel/kprobes-test-arm.c
> +++ b/arch/arm/kernel/kprobes-test-arm.c
> @@ -187,8 +187,8 @@ void kprobe_arm_test_cases(void)
> TEST_BF_R ("mov pc, r",0,2f,"")
> TEST_BF_RR("mov pc, r",0,2f,", asl r",1,0,"")
> TEST_BB( "sub pc, pc, #1b-2b+8")
> -#if __LINUX_ARM_ARCH__ >= 6
> - TEST_BB( "sub pc, pc, #1b-2b+8-2") /* UNPREDICTABLE before ARMv6 */
> +#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")
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH] kprobes: only test 'sub pc, pc, #1b-2b+8-2' on ARMv6
2012-06-15 13:36 ` Tixy
@ 2012-06-15 14:00 ` Rabin Vincent
2012-06-15 14:53 ` Tixy
0 siblings, 1 reply; 6+ messages in thread
From: Rabin Vincent @ 2012-06-15 14:00 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, Jun 15, 2012 at 7:06 PM, Tixy <tixy@yxit.co.uk> wrote:
>> How about the following check instead? ?At least one other place uses
>> it.
>
> Can't you point out the other place which uses it, I couldn't find it.
arch/arm/kernel/entry-armv.S:445
#if CONFIG_ARM_THUMB && __LINUX_ARM_ARCH__ >= 6 && CONFIG_CPU_V7
> The proposed patch below would work, but feels slightly off as it
> assumes that no one will build a kernel to support V6 and V8, but
> without V7. That does seem highly unlikely though, so I personally
> wouldn't object to the patch.
I've no preference either way, so if a deletion of the test case would get your
acked-by instead of just a non-objection, I'll do that.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] kprobes: only test 'sub pc, pc, #1b-2b+8-2' on ARMv6
2012-06-15 14:00 ` Rabin Vincent
@ 2012-06-15 14:53 ` Tixy
0 siblings, 0 replies; 6+ messages in thread
From: Tixy @ 2012-06-15 14:53 UTC (permalink / raw)
To: linux-arm-kernel
On Fri, 2012-06-15 at 19:30 +0530, Rabin Vincent wrote:
> On Fri, Jun 15, 2012 at 7:06 PM, Tixy <tixy@yxit.co.uk> wrote:
> >> How about the following check instead? At least one other place uses
> >> it.
> >
> > Can't you point out the other place which uses it, I couldn't find it.
>
> arch/arm/kernel/entry-armv.S:445
> #if CONFIG_ARM_THUMB && __LINUX_ARM_ARCH__ >= 6 && CONFIG_CPU_V7
Yes, that's going to need looking at too when V8 arrives.
> > The proposed patch below would work, but feels slightly off as it
> > assumes that no one will build a kernel to support V6 and V8, but
> > without V7. That does seem highly unlikely though, so I personally
> > wouldn't object to the patch.
>
> I've no preference either way, so if a deletion of the test case would get your
> acked-by instead of just a non-objection, I'll do that.
Actually, the kprobes implementation and test code will need looking at
again when V8 hits mainline, so lets not worry about it for the moment.
So, in that case, I agree with your revised patch which tests for not
CONFIG_CPU_V7 and you have:
Acked-by: Jon Medhurst <tixy@yxit.co.uk>
Thanks for looking at kprobes and contributing fixes.
As Russell King is the official maintainer of this code, can you add
your patches to his patch system?
Thanks again.
--
Tixy
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-06-15 14:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-12 12:07 [PATCH] kprobes: only test 'sub pc, pc, #1b-2b+8-2' on ARMv6 Rabin Vincent
2012-06-13 7:18 ` Tixy
2012-06-15 10:03 ` Rabin Vincent
2012-06-15 13:36 ` Tixy
2012-06-15 14:00 ` Rabin Vincent
2012-06-15 14:53 ` Tixy
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).