Hi Bin,
On 6 August 2015 at 01:16, Bin Meng bmeng.cn@gmail.com wrote:
Hi Simon,
On Mon, Aug 3, 2015 at 8:10 AM, Simon Glass sjg@chromium.org wrote:
There is quite a bit of assembler code that can be removed if we use the generic global_data setup. Less arch-specific code makes it easier to add new features and maintain the start-up code.
Drop the unneeded code and adjust the hooks in board_f.c to cope.
Signed-off-by: Simon Glass sjg@chromium.org
arch/x86/cpu/cpu.c | 4 ++- arch/x86/cpu/start.S | 86 ++++++---------------------------------------------- common/board_f.c | 3 +- 3 files changed, 15 insertions(+), 78 deletions(-)
diff --git a/arch/x86/cpu/cpu.c b/arch/x86/cpu/cpu.c index 4f57145..b2e0323 100644 --- a/arch/x86/cpu/cpu.c +++ b/arch/x86/cpu/cpu.c @@ -136,8 +136,10 @@ static void load_gdt(const u64 *boot_gdt, u16 num_entries) asm volatile("lgdtl %0\n" : : "m" (gdt)); }
-void setup_gdt(gd_t *new_gd, u64 *gdt_addr) +void arch_setup_gdt(gd_t *new_gd) {
u64 *gdt_addr;gdt_addr = new_gd->arch.gdt; /* CS: code, read/execute, 4 GB, base 0 */diff --git a/arch/x86/cpu/start.S b/arch/x86/cpu/start.S index ac711ed..d93bdfc 100644 --- a/arch/x86/cpu/start.S +++ b/arch/x86/cpu/start.S @@ -104,8 +104,7 @@ car_init_ret: * * top-> CONFIG_SYS_CAR_ADDR + CONFIG_SYS_CAR_SIZE * MRC area
* global_data* x86 global descriptor table
* global_data with x86 global descriptor table * early malloc area * stack * bottom-> CONFIG_SYS_CAR_ADDR@@ -120,13 +119,10 @@ car_init_ret: * and esi holds the HOB list address returned by the FSP. */ #endif
/* Reserve space on stack for global data */subl $GENERATED_GBL_DATA_SIZE, %esp/* Align global data to 16-byte boundary */andl $0xfffffff0, %esppost_code(POST_START_STACK)
/* Set up global data */mov %esp, %eaxcall board_init_f_memmov %eax, %esp /* * Debug UART is available here although it may not be plumbed out@@ -137,19 +133,12 @@ car_init_ret: * call printch */
/* Zero the global data since it won't happen later */xorl %eax, %eaxmovl $GENERATED_GBL_DATA_SIZE, %ecxmovl %esp, %edirep stosb#ifdef CONFIG_HAVE_FSP
/* Store the HOB list if we have one */ test %esi, %esi jz skip_hob
/* Store HOB list */movl %esp, %edxaddl $GD_HOB_LIST, %edx+fs addl $GD_HOB_LIST, %edx
I don't see %edx is initialized anywhere?
What I really want to do is:
movl fs:GD_HOB_LIST, %edx
That doesn't seem to do the right thing, although I'm still testing fiddling with it.
movl %esi, (%edx)skip_hob: @@ -159,35 +148,10 @@ skip_hob: addl $GD_TABLE, %edx movl %esi, (%edx) #endif
/* Setup first parameter to setup_gdt, pointer to global_data */movl %esp, %eax/* Reserve space for global descriptor table */subl $X86_GDT_SIZE, %esp/* Align temporary global descriptor table to 16-byte boundary */andl $0xfffffff0, %espmovl %esp, %ecx-#if defined(CONFIG_SYS_MALLOC_F_LEN)
/* Set up the pre-relocation malloc pool */subl $CONFIG_SYS_MALLOC_F_LEN, %espmovl %eax, %edxaddl $GD_MALLOC_BASE, %edxmovl %esp, (%edx)-#endif
/* Store BIST into global_data */movl %eax, %edxaddl $GD_BIST, %edx
/* Store BIST */+fs addl $GD_BIST, %edx
Ditto.
movl %ebp, (%edx)
/* Set second parameter to setup_gdt() */movl %ecx, %edx/* Setup global descriptor table so gd->xyz works */call setup_gdt/* Set parameter to board_init_f() to boot flags */ post_code(POST_START_DONE) xorl %eax, %eax@@ -213,37 +177,7 @@ board_init_f_r_trampoline: /* Stack grows down from top of SDRAM */ movl %eax, %esp
/* Reserve space on stack for global data */subl $GENERATED_GBL_DATA_SIZE, %esp/* Align global data to 16-byte boundary */andl $0xfffffff0, %esp/* Setup first parameter to memcpy() and setup_gdt() */movl %esp, %eax/* Setup second parameter to memcpy() */fs movl 0, %edx/* Set third parameter to memcpy() */movl $GENERATED_GBL_DATA_SIZE, %ecx/* Copy global data from CAR to SDRAM stack */call memcpy/* Reserve space for global descriptor table */subl $X86_GDT_SIZE, %esp/* Align global descriptor table to 16-byte boundary */andl $0xfffffff0, %esp/* Set second parameter to setup_gdt() */movl %esp, %edx/* Setup global descriptor table so gd->xyz works */call setup_gdt/* Set if we need to disable CAR */
/* See if we need to disable CAR */.weak car_uninit movl $car_uninit, %eax cmpl $0, %eax diff --git a/common/board_f.c b/common/board_f.c index 3d1f305..ec9d2a8 100644 --- a/common/board_f.c +++ b/common/board_f.c @@ -711,6 +711,7 @@ static int jump_to_copy(void) * with the stack in SDRAM and Global Data in temporary memory * (CPU cache) */
arch_setup_gdt(gd->new_gd); board_init_f_r_trampoline(gd->start_addr_sp);#else relocate_code(gd->start_addr_sp, gd->new_gd, gd->relocaddr); @@ -1028,6 +1029,7 @@ __weak void arch_setup_gdt(struct global_data *gd_ptr) { gd = gd_ptr; } +#endif /* !CONFIG_X86 */
ulong board_init_f_mem(ulong top, ulong reserve_size) { @@ -1049,4 +1051,3 @@ ulong board_init_f_mem(ulong top, ulong reserve_size)
return top;} -#endif /* !CONFIG_X86 */
Can we remove the CONFIG_X86 around arch_set_gdt() given it is a __weak and any arch can implement their own?
Unfortunately I get a compile error when assigning to gd - it is not a register like on ARM, etc.
Regards, Simon