Re: [U-Boot] [PATCH 2/3] rockchip: back-to-bootrom: replace assembly-implementation with C-code

Hi Philipp:

On 2017年09月15日 20:02, Philipp Tomsich wrote:

The back-to-bootrom implementation for Rockchip has always relied on the stack-pointer being valid on entry, so there was little reason to have this as an assembly implementation.

This provides a new C-only implementation of save_boot_params and back_to_bootrom (relying on setjmp/longjmp) and removes the older assembly-only implementation.

Signed-off-by: Philipp Tomsich philipp.tomsich@theobroma-systems.com

arch/arm/include/asm/arch-rockchip/bootrom.h | 27 ++++++++--- arch/arm/mach-rockchip/Makefile | 4 +- arch/arm/mach-rockchip/bootrom.c | 52 ++++++++++++++++++++- arch/arm/mach-rockchip/save_boot_param.S | 69 ---------------------------- 4 files changed, 73 insertions(+), 79 deletions(-) delete mode 100644 arch/arm/mach-rockchip/save_boot_param.S

diff --git a/arch/arm/include/asm/arch-rockchip/bootrom.h b/arch/arm/include/asm/arch-rockchip/bootrom.h index 169cc5e..2f61a33 100644 --- a/arch/arm/include/asm/arch-rockchip/bootrom.h +++ b/arch/arm/include/asm/arch-rockchip/bootrom.h @@ -1,5 +1,6 @@ /*

• (C) Copyright 2017 Heiko Stuebner heiko@sntech.de

• • (C) Copyright 2017 Theobroma Systems Design und Consulting GmbH
  • SPDX-License-Identifier: GPL-2.0
  */

@@ -14,15 +15,27 @@ extern u32 SAVE_SP_ADDR;

/**

• • Hand control back to the bootrom to load another
• • boot stage.

• • back_to_bootrom() - return to bootrom (for TPL/SPL), passing a

• •                 result code
    

• • Transfer control back to the Rockchip BROM, restoring necessary
• • register context and passing a command/result code to the BROM
• • to instruct its next actions (e.g. continue boot sequence, enter
• • download mode, ...).
• • This function does not return.
  */

-void back_to_bootrom(void); +enum rockchip_bootrom_cmd {

• /*

• * These can not start at 0, as 0 has a special meaning
  

• * for setjmp().
  

• */
  
  

-/**

• • Assembler component for the above (do not call this directly)
• */

-void _back_to_bootrom_s(void);

• BROM_BOOT_NEXTSTAGE = 1, /* continue boot-sequence */
• BROM_BOOT_ENTER_DNL, /* have BROM enter download-mode */

+};

+void back_to_bootrom(void);

/**

• Boot-device identifiers as used by the BROM

diff --git a/arch/arm/mach-rockchip/Makefile b/arch/arm/mach-rockchip/Makefile index 79e9704..f8b23ea 100644 --- a/arch/arm/mach-rockchip/Makefile +++ b/arch/arm/mach-rockchip/Makefile @@ -8,8 +8,8 @@ # this may have entered from ATF with the stack-pointer pointing to # inaccessible/protected memory (and the bootrom-helper assumes that # the stack-pointer is valid before switching to the U-Boot stack). -obj-spl-$(CONFIG_ROCKCHIP_BROM_HELPER) += bootrom.o save_boot_param.o -obj-tpl-$(CONFIG_ROCKCHIP_BROM_HELPER) += bootrom.o save_boot_param.o +obj-spl-$(CONFIG_ROCKCHIP_BROM_HELPER) += bootrom.o +obj-tpl-$(CONFIG_ROCKCHIP_BROM_HELPER) += bootrom.o

obj-tpl-$(CONFIG_ROCKCHIP_RK3188) += rk3188-board-tpl.o obj-tpl-$(CONFIG_ROCKCHIP_RK3368) += rk3368-board-tpl.o diff --git a/arch/arm/mach-rockchip/bootrom.c b/arch/arm/mach-rockchip/bootrom.c index 8380e4e..7b9b307 100644 --- a/arch/arm/mach-rockchip/bootrom.c +++ b/arch/arm/mach-rockchip/bootrom.c @@ -6,11 +6,61 @@

#include <common.h> #include <asm/arch/bootrom.h> +#include <asm/setjmp.h> +#include <asm/system.h>

+/*

• • Force the jmp_buf to the data-section, as .bss will not be valid
• • when save_boot_params is invoked.
• */

+static jmp_buf brom_ctx __section(".data");

void back_to_bootrom(void) { #if CONFIG_IS_ENABLED(LIBCOMMON_SUPPORT) puts("Returning to boot ROM...\n"); #endif

• _back_to_bootrom_s();

• longjmp(brom_ctx, BROM_BOOT_NEXTSTAGE);

+}

+/*

• • All Rockchip BROM implementations enter with a valid stack-pointer,
• • so this can safely be implemented in C (providing a single
• • implementation both for ARMv7 and AArch64).
• */

+int save_boot_params(void) +{

• int ret = setjmp(brom_ctx);
• switch (ret) {
• case 0:

• /*
  

•  * This is the initial pass through this function
  

•  * (i.e. saving the context), setjmp just setup up the
  

•  * brom_ctx: transfer back into the startup-code at
  

•  * 'save_boot_params_ret' and let the compiler know
  

•  * that this will not return.
  

•  */
  

• save_boot_params_ret();
  

This function works fine on ARM64 platform, But I got problems on ARMv7. When trace the code flow with DS5 I found the core switch to thumb state when jump to save_boot_params_ret[0], but this code can't only execute in arm state as thumb instruction can't access cpsr register. I don't know how to make sure the core in arm state when jump to save_boot_params_ret.

save_boot_params_ret: #ifdef CONFIG_ARMV7_LPAE /* * check for Hypervisor support */ mrc p15, 0, r0, c0, c1, 1 @ read ID_PFR1 and r0, r0, #CPUID_ARM_VIRT_MASK @ mask virtualization bits cmp r0, #(1 << CPUID_ARM_VIRT_SHIFT) beq switch_to_hypervisor switch_to_hypervisor_ret: #endif /* * disable interrupts (FIQ and IRQ), also set the cpu to SVC32 mode, * except if in HYP mode already */ mrs r0, cpsr and r1, r0, #0x1f @ mask mode bits teq r1, #0x1a @ test for HYP mode bicne r0, r0, #0x1f @ clear all mode bits orrne r0, r0, #0x13 @ set SVC mode orr r0, r0, #0xc0 @ disable FIQ and IRQ msr cpsr,r0

• while (true)
  

• 	/* does not return */;
  

• break;
  

• case BROM_BOOT_NEXTSTAGE:

• /*
  

•  * To instruct the BROM to boot the next stage, we
  

•  * need to return 0 to it: i.e. we need to rewrite
  

•  * the return code once more.
  

•  */
  

• ret = 0;
  

• break;
  

• default:

+#if CONFIG_IS_ENABLED(LIBCOMMON_SUPPORT)

• puts("FATAL: unexpected command to back_to_bootrom()\n");
  

+#endif

• hang();
  

• };
• return ret; }

diff --git a/arch/arm/mach-rockchip/save_boot_param.S b/arch/arm/mach-rockchip/save_boot_param.S deleted file mode 100644 index 50fce20..0000000 --- a/arch/arm/mach-rockchip/save_boot_param.S +++ /dev/null @@ -1,69 +0,0 @@ -/*

• • (C) Copyright 2016 Rockchip Electronics Co., Ltd
• • (C) Copyright 2017 Theobroma Systems Design und Consulting GmbH
• • SPDX-License-Identifier: GPL-2.0+
• */

-#include <linux/linkage.h>

-#if defined(CONFIG_ARM64) -.globl SAVE_SP_ADDR -SAVE_SP_ADDR:

• .quad 0

-ENTRY(save_boot_params)

• sub sp, sp, #0x60
• stp x29, x30, [sp, #0x50]
• stp x27, x28, [sp, #0x40]
• stp x25, x26, [sp, #0x30]
• stp x23, x24, [sp, #0x20]
• stp x21, x22, [sp, #0x10]
• stp x19, x20, [sp, #0]
• ldr x8, =SAVE_SP_ADDR
• mov x9, sp
• str x9, [x8]
• b save_boot_params_ret /* back to my caller */

-ENDPROC(save_boot_params)

-.globl _back_to_bootrom_s -ENTRY(_back_to_bootrom_s)

• ldr x0, =SAVE_SP_ADDR
• ldr x0, [x0]
• mov sp, x0
• ldp x29, x30, [sp, #0x50]
• ldp x27, x28, [sp, #0x40]
• ldp x25, x26, [sp, #0x30]
• ldp x23, x24, [sp, #0x20]
• ldp x21, x22, [sp, #0x10]
• ldp x19, x20, [sp]
• add sp, sp, #0x60
• mov x0, xzr
• ret

-ENDPROC(_back_to_bootrom_s) -#else -.globl SAVE_SP_ADDR -SAVE_SP_ADDR:

• .word 0

-/*

• • void save_boot_params
• • Save sp, lr, r1~r12
• */

-ENTRY(save_boot_params)

• push {r1-r12, lr}
• ldr r0, =SAVE_SP_ADDR
• str sp, [r0]
• b save_boot_params_ret @ back to my caller

-ENDPROC(save_boot_params)

-.globl _back_to_bootrom_s -ENTRY(_back_to_bootrom_s)

• ldr r0, =SAVE_SP_ADDR
• ldr sp, [r0]
• mov r0, #0
• pop {r1-r12, pc}

-ENDPROC(_back_to_bootrom_s) -#endif