[U-Boot] [PATCH 5/5] x86: Switch to using generic global_data setup

Simon Glass sjg at chromium.org
Mon Aug 10 20:51:53 CEST 2015


Hi Bin,

On 6 August 2015 at 01:16, Bin Meng <bmeng.cn at gmail.com> wrote:
> Hi Simon,
>
> On Mon, Aug 3, 2015 at 8:10 AM, Simon Glass <sjg at 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 at 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, %esp
>> -       post_code(POST_START_STACK)
>> +       /* Set up global data */
>> +       mov     %esp, %eax
>> +       call    board_init_f_mem
>> +       mov     %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, %eax
>> -       movl    $GENERATED_GBL_DATA_SIZE, %ecx
>> -       movl    %esp, %edi
>> -       rep     stosb
>> -
>>  #ifdef CONFIG_HAVE_FSP
>> +       /* Store the HOB list if we have one */
>>         test    %esi, %esi
>>         jz      skip_hob
>>
>> -       /* Store HOB list */
>> -       movl    %esp, %edx
>> -       addl    $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, %esp
>> -       movl    %esp, %ecx
>> -
>> -#if defined(CONFIG_SYS_MALLOC_F_LEN)
>> -       /* Set up the pre-relocation malloc pool */
>> -       subl    $CONFIG_SYS_MALLOC_F_LEN, %esp
>> -       movl    %eax, %edx
>> -       addl    $GD_MALLOC_BASE, %edx
>> -       movl    %esp, (%edx)
>> -#endif
>> -       /* Store BIST into global_data */
>> -       movl    %eax, %edx
>> -       addl    $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


More information about the U-Boot mailing list