| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-id: <alpine.LFD.2.00.1012201638150.10437@xanadu.home>
Date: Mon, 20 Dec 2010 16:39:10 -0500 (EST)
From: Nicolas Pitre <nico@...xnic.net>
To: Stephen Boyd <sboyd@...eaurora.org>
Cc: Greg Kroah-Hartman <gregkh@...e.de>, linux-kernel@...r.kernel.org,
linux-arm-msm@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org,
Tony Lindgren <tony@...mide.com>,
Arnd Bergmann <arnd@...db.de>,
Daniel Walker <dwalker@...eaurora.org>
Subject: Re: [PATCH 1/3] hvc_dcc: Fix bad code generation by marking assembly
volatile
On Mon, 20 Dec 2010, Stephen Boyd wrote:
> Without marking the asm __dcc_getstatus() volatile my compiler
> decides it can cache the value of __ret in a register and then
> check the value of it continually in hvc_dcc_put_chars() (I had
> to replace get_wait/put_wait with 1 and fixup the branch
> otherwise my disassembler barfed on __dcc_(get|put)char).
>
> 00000000 <hvc_dcc_put_chars>:
> 0: ee103e11 mrc 14, 0, r3, cr0, cr1, {0}
> 4: e3a0c000 mov ip, #0 ; 0x0
> 8: e2033202 and r3, r3, #536870912 ; 0x20000000
> c: ea000006 b 2c <hvc_dcc_put_chars+0x2c>
> 10: e3530000 cmp r3, #0 ; 0x0
> 14: 1afffffd bne 10 <hvc_dcc_put_chars+0x10>
> 18: e7d1000c ldrb r0, [r1, ip]
> 1c: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0}
> 20: 2afffffd bcs 1c <hvc_dcc_put_chars+0x1c>
> 24: ee000e15 mcr 14, 0, r0, cr0, cr5, {0}
> 28: e28cc001 add ip, ip, #1 ; 0x1
> 2c: e15c0002 cmp ip, r2
> 30: bafffff6 blt 10 <hvc_dcc_put_chars+0x10>
> 34: e1a00002 mov r0, r2
> 38: e12fff1e bx lr
>
> As you can see, the value of the mrc is checked against
> DCC_STATUS_TX (bit 29) and then stored in r3 for later use.
> Marking the asm volatile produces the following:
>
> 00000000 <hvc_dcc_put_chars>:
> 0: e3a03000 mov r3, #0 ; 0x0
> 4: ea000007 b 28 <hvc_dcc_put_chars+0x28>
> 8: ee100e11 mrc 14, 0, r0, cr0, cr1, {0}
> c: e3100202 tst r0, #536870912 ; 0x20000000
> 10: 1afffffc bne 8 <hvc_dcc_put_chars+0x8>
> 14: e7d10003 ldrb r0, [r1, r3]
> 18: ee10fe11 mrc 14, 0, pc, cr0, cr1, {0}
> 1c: 2afffffd bcs 18 <hvc_dcc_put_chars+0x18>
> 20: ee000e15 mcr 14, 0, r0, cr0, cr5, {0}
> 24: e2833001 add r3, r3, #1 ; 0x1
> 28: e1530002 cmp r3, r2
> 2c: bafffff5 blt 8 <hvc_dcc_put_chars+0x8>
> 30: e1a00002 mov r0, r2
> 34: e12fff1e bx lr
>
> which looks better and actually works. Mark all the inline
> assembly in this file as volatile since we don't want the
> compiler to optimize away these statements or move them around
> in any way.
>
> Cc: Tony Lindgren <tony@...mide.com>
> Cc: Arnd Bergmann <arnd@...db.de>
> Cc: Nicolas Pitre <nicolas.pitre@...aro.org>
> Cc: Daniel Walker <dwalker@...eaurora.org>
> Signed-off-by: Stephen Boyd <sboyd@...eaurora.org>
Acked-by: Nicolas Pitre <nicolas.pitre@...aro.org>
> ---
> drivers/char/hvc_dcc.c | 11 +++++------
> 1 files changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/char/hvc_dcc.c b/drivers/char/hvc_dcc.c
> index 6470f63..155ec10 100644
> --- a/drivers/char/hvc_dcc.c
> +++ b/drivers/char/hvc_dcc.c
> @@ -33,8 +33,7 @@
> static inline u32 __dcc_getstatus(void)
> {
> u32 __ret;
> -
> - asm("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg"
> + asm volatile("mrc p14, 0, %0, c0, c1, 0 @ read comms ctrl reg"
> : "=r" (__ret) : : "cc");
>
> return __ret;
> @@ -46,7 +45,7 @@ static inline char __dcc_getchar(void)
> {
> char __c;
>
> - asm("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\
> + asm volatile("get_wait: mrc p14, 0, pc, c0, c1, 0 \n\
> bne get_wait \n\
> mrc p14, 0, %0, c0, c5, 0 @ read comms data reg"
> : "=r" (__c) : : "cc");
> @@ -58,7 +57,7 @@ static inline char __dcc_getchar(void)
> {
> char __c;
>
> - asm("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg"
> + asm volatile("mrc p14, 0, %0, c0, c5, 0 @ read comms data reg"
> : "=r" (__c));
>
> return __c;
> @@ -68,7 +67,7 @@ static inline char __dcc_getchar(void)
> #if defined(CONFIG_CPU_V7)
> static inline void __dcc_putchar(char c)
> {
> - asm("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\
> + asm volatile("put_wait: mrc p14, 0, pc, c0, c1, 0 \n\
> bcs put_wait \n\
> mcr p14, 0, %0, c0, c5, 0 "
> : : "r" (c) : "cc");
> @@ -76,7 +75,7 @@ static inline void __dcc_putchar(char c)
> #else
> static inline void __dcc_putchar(char c)
> {
> - asm("mcr p14, 0, %0, c0, c5, 0 @ write a char"
> + asm volatile("mcr p14, 0, %0, c0, c5, 0 @ write a char"
> : /* no output register */
> : "r" (c));
> }
> --
> Sent by an employee of the Qualcomm Innovation Center, Inc.
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum.
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists