On 18 Sep 2017, at 10:17, Andy Yan <andy....@rock-chips.com> wrote:
>
> 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.toms...@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 <he...@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.
Thanks for the detailed diagnosis. I’ll take a look on how to best force
the compiler to emit this function in ARM state: I hope to be able to use
a target(arm) function attribute (see
https://gcc.gnu.org/onlinedocs/gcc/ARM-Function-Attributes.html)
>
>
> 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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
