Hi Tom, thank you for reviewing.

(2012/06/28 2:40), Tom Rini wrote:

On Mon, Jun 25, 2012 at 09:42:29PM +0900, Tetsuyuki Kobayashi wrote:

...

save_boot_params_default() in cpu.c accesses uninitialized stack area when it compiled with -O0 (not optimized).

Signed-off-by: Tetsuyuki Kobayashikoba@kmckk.co.jp

arch/arm/cpu/armv7/cpu.c | 3 +++ 1 file changed, 3 insertions(+)

diff --git a/arch/arm/cpu/armv7/cpu.c b/arch/arm/cpu/armv7/cpu.c index c6fa8ef..6104cb2 100644 --- a/arch/arm/cpu/armv7/cpu.c +++ b/arch/arm/cpu/armv7/cpu.c @@ -37,8 +37,11 @@ #include<asm/cache.h> #include<asm/armv7.h>

+__attribute__((naked)) /* don't save anything to stack even if compiled with -O0 */ void save_boot_params_default(u32 r0, u32 r1, u32 r2, u32 r3) {

• /* stack is not yet initialized */
• asm("bx lr"); }

Please add<linux/compiler.h> and use __naked instead. Thanks!

OK. I will post V2 patch.

On 06/28/2012 04:35 AM, Tetsuyuki Kobayashi wrote:

save_boot_params_default() in cpu.c accesses uninitialized stack area when it compiled with -O0 (not optimized).

Signed-off-by: Tetsuyuki Kobayashi koba@kmckk.co.jp

Changes for v2:

• include <linux/compiler.h> and use __naked instead of __attribute__((naked))

arch/arm/cpu/armv7/cpu.c | 4 ++++ 1 file changed, 4 insertions(+)

diff --git a/arch/arm/cpu/armv7/cpu.c b/arch/arm/cpu/armv7/cpu.c index c6fa8ef..3e2a75c 100644 --- a/arch/arm/cpu/armv7/cpu.c +++ b/arch/arm/cpu/armv7/cpu.c @@ -36,9 +36,13 @@ #include <asm/system.h> #include <asm/cache.h> #include <asm/armv7.h> +#include <linux/compiler.h>

+__naked /* don't save anything to stack even if compiled with -O0 */ void save_boot_params_default(u32 r0, u32 r1, u32 r2, u32 r3)

The usual form (here and kernel) is: void __naked save_boot_params_default(...)

Same for __weak and so on. Thanks!

save_boot_params_default() in cpu.c accesses uninitialized stack area when it compiled with -O0 (not optimized).

Signed-off-by: Tetsuyuki Kobayashi koba@kmckk.co.jp --- Changes for v2: - include <linux/compiler.h> and use __naked instead of __attribute__((naked))

Changes for v3: - move __naked after void - reformat comments

arch/arm/cpu/armv7/cpu.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/arch/arm/cpu/armv7/cpu.c b/arch/arm/cpu/armv7/cpu.c index c6fa8ef..9eb484a 100644 --- a/arch/arm/cpu/armv7/cpu.c +++ b/arch/arm/cpu/armv7/cpu.c @@ -36,9 +36,15 @@ #include <asm/system.h> #include <asm/cache.h> #include <asm/armv7.h> +#include <linux/compiler.h>

-void save_boot_params_default(u32 r0, u32 r1, u32 r2, u32 r3) +void __naked save_boot_params_default(u32 r0, u32 r1, u32 r2, u32 r3) { + /* + * Stack pointer is not yet initialized + * Don't save anything to stack even if compiled with -O0 + */ + asm("bx lr"); }

void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3) -- 1.7.9.5

save_boot_params_default() in cpu.c accesses uninitialized stack area when it compiled with -O0 (not optimized).

Signed-off-by: Tetsuyuki Kobayashi koba@kmckk.co.jp --- Hi Tom, Albert,

I rewrite them in asm language and put it to start.S. No warning now. I tested it quickly on my kzm9g board.

Changes for v2: - include <linux/compiler.h> and use __naked instead of __attribute__((naked))

Changes for v3: - move __naked after void - reformat comments

Changes for v4: - v3 causes following warnings cpu.c: In function 'save_boot_params_default': cpu.c:48:1: warning: -fstack-usage not supported for this target [enabled by default] - move save_boot_params_default() and save_boot_params() from cpu.c to start.S and write them in asm language

arch/arm/cpu/armv7/cpu.c | 7 ------- arch/arm/cpu/armv7/start.S | 15 +++++++++++++++ 2 files changed, 15 insertions(+), 7 deletions(-)

diff --git a/arch/arm/cpu/armv7/cpu.c b/arch/arm/cpu/armv7/cpu.c index c6fa8ef..b0677f4 100644 --- a/arch/arm/cpu/armv7/cpu.c +++ b/arch/arm/cpu/armv7/cpu.c @@ -37,13 +37,6 @@ #include <asm/cache.h> #include <asm/armv7.h>

-void save_boot_params_default(u32 r0, u32 r1, u32 r2, u32 r3) -{ -} - -void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3) - __attribute__((weak, alias("save_boot_params_default"))); - int cleanup_before_linux(void) { /* diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 261835b..4feade5 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -350,6 +350,21 @@ ENTRY(cpu_init_crit) ENDPROC(cpu_init_crit) #endif

+/************************************************************************* + * + * void save_boot_params_default(u32 r0, u32 r1, u32 r2, u32 r3) + * + * Stack pointer is not yet initialized + * Don't save anything to stack even if compiled with -O0 + * + *************************************************************************/ +ENTRY(save_boot_params_default) + bx lr @ back to my caller +ENDPROC(save_boot_params_default) + + .weak save_boot_params + .set save_boot_params, save_boot_params_default + #ifndef CONFIG_SPL_BUILD /* ************************************************************************* -- 1.7.9.5

save_boot_params_default() in cpu.c accesses uninitialized stack area when it compiled with -O0 (not optimized). This patch removes save_boot_params_default() and put the equivalent in start.S

Signed-off-by: Tetsuyuki Kobayashi koba@kmckk.co.jp --- Hi Tom, Albert,

I rewrite it again. I tested it quickly on my kzm9g board, and also build it for omap4_panda and checked the generated code by objdump command.

Changes for v2: - include <linux/compiler.h> and use __naked instead of __attribute__((naked))

Changes for v3: - move __naked after void - reformat comments

Changes for v4: - v3 causes following warnings cpu.c: In function 'save_boot_params_default': cpu.c:48:1: warning: -fstack-usage not supported for this target [enabled by default] - move save_boot_params_default() and save_boot_params() from cpu.c to start.S and write them in asm language

Changes for v5 - rename save_boot_parames_default() to save_boot_params() and drop aliasing - move the code after relocate_code (nearer to callee) - modify commit log

arch/arm/cpu/armv7/cpu.c | 7 ------- arch/arm/cpu/armv7/start.S | 14 ++++++++++++++ 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/arch/arm/cpu/armv7/cpu.c b/arch/arm/cpu/armv7/cpu.c index c6fa8ef..b0677f4 100644 --- a/arch/arm/cpu/armv7/cpu.c +++ b/arch/arm/cpu/armv7/cpu.c @@ -37,13 +37,6 @@ #include <asm/cache.h> #include <asm/armv7.h>

-void save_boot_params_default(u32 r0, u32 r1, u32 r2, u32 r3) -{ -} - -void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3) - __attribute__((weak, alias("save_boot_params_default"))); - int cleanup_before_linux(void) { /* diff --git a/arch/arm/cpu/armv7/start.S b/arch/arm/cpu/armv7/start.S index 261835b..bf734fb 100644 --- a/arch/arm/cpu/armv7/start.S +++ b/arch/arm/cpu/armv7/start.S @@ -293,6 +293,20 @@ ENDPROC(relocate_code)

/************************************************************************* * + * void save_boot_params(u32 r0, u32 r1, u32 r2, u32 r3) + * __attribute__((weak)); + * + * Stack pointer is not yet initialized at this moment + * Don't save anything to stack even if compiled with -O0 + * + *************************************************************************/ +ENTRY(save_boot_params) + bx lr @ back to my caller +ENDPROC(save_boot_params) + .weak save_boot_params + +/************************************************************************* + * * cpu_init_cp15 * * Setup CP15 registers (cache, MMU, TLBs). The I-cache is turned on unless