[U-Boot] [PATCH v3 11/27] x86: queensbay: Adapt FSP support codes

[Bin Meng]

Hi Simon,

On Sat, Dec 13, 2014 at 2:49 AM, Simon Glass <sjg at chromium.org> wrote:
> Hi Bin,
>
> On 12 December 2014 at 06:05, Bin Meng <bmeng.cn at gmail.com> wrote:
>> Use inline assembly codes to call FspNotify() to make sure parameters
>> are passed on the stack as required by the FSP calling convention.
>>
>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>>
>> ---
>>
>> Changes in v3:
>> - Move license header changes to last commit
>>
>> Changes in v2:
>> - Update the codes to use U-Boot coding style
>>
>>  arch/x86/cpu/queensbay/fsp_configs.c               |  5 ++-
>>  arch/x86/cpu/queensbay/fsp_support.c               | 39 ++++++++++++++--------
>>  .../include/asm/arch-queensbay/fsp/fsp_support.h   |  2 +-
>>  3 files changed, 28 insertions(+), 18 deletions(-)
>>
>> diff --git a/arch/x86/cpu/queensbay/fsp_configs.c b/arch/x86/cpu/queensbay/fsp_configs.c
>> index a7bb314..aef18fc 100644
>> --- a/arch/x86/cpu/queensbay/fsp_configs.c
>> +++ b/arch/x86/cpu/queensbay/fsp_configs.c
>> @@ -5,9 +5,8 @@
>>   * SPDX-License-Identifier:    Intel
>>   */
>>
>> -#include <types.h>
>> -#include <string.h>
>> -#include "fsp_support.h"
>> +#include <common.h>
>> +#include <asm/arch/fsp/fsp_support.h>
>>
>>  void update_fsp_upd(struct upd_region_t *fsp_upd)
>>  {
>> diff --git a/arch/x86/cpu/queensbay/fsp_support.c b/arch/x86/cpu/queensbay/fsp_support.c
>> index 2048030..df3bbd0 100644
>> --- a/arch/x86/cpu/queensbay/fsp_support.c
>> +++ b/arch/x86/cpu/queensbay/fsp_support.c
>> @@ -5,9 +5,9 @@
>>   * SPDX-License-Identifier:    Intel
>>   */
>>
>> -#include <types.h>
>> -#include <string.h>
>> -#include "fsp_support.h"
>> +#include <common.h>
>> +#include <asm/arch/fsp/fsp_support.h>
>> +#include <asm/post.h>
>>
>>  /**
>>   * Reads a 64-bit value from memory that may be unaligned.
>> @@ -99,13 +99,14 @@ u32 __attribute__((optimize("O0"))) find_fsp_header(void)
>>         return (u32)fsp;
>>  }
>>
>> -#ifdef __PRE_RAM__
>>  void fsp_continue(struct shared_data_t *shared_data, u32 status, void *hob_list)
>>  {
>>         u32 stack_len;
>>         u32 stack_base;
>>         u32 stack_top;
>>
>> +       post_code(POST_MRC);
>> +
>>         ASSERT(status == 0);
>>
>>         /* Get the migrated stack in normal memory */
>> @@ -121,7 +122,7 @@ void fsp_continue(struct shared_data_t *shared_data, u32 status, void *hob_list)
>>                         ((u32)shared_data - *(u32 *)stack_top));
>>
>>         /* The boot loader main function entry */
>> -       bl_main_continue(hob_list, shared_data);
>> +       fsp_init_done(hob_list);
>>  }
>>
>>  void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>> @@ -178,6 +179,8 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>>         shared_data.fsp_hdr = fsp_hdr;
>>         shared_data.stack_top = (u32 *)stack_top;
>>
>> +       post_code(POST_PRE_MRC);
>> +
>>         /*
>>          * Use ASM code to ensure the register value in EAX & ECX
>>          * will be passed into BlContinuationFunc
>> @@ -187,11 +190,11 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>>                 "call   *%%eax;"
>>                 ".global asm_continuation;"
>>                 "asm_continuation:;"
>> -               "popl   %%eax;" /* pop out return address */
>> -               "pushl  %%ecx;" /* push shared_data pointer */
>> -               "pushl  %%eax;" /* push back return address */
>> +               "movl   %%ebx, %%eax;"          /* shared_data */
>> +               "movl   4(%%esp), %%edx;"       /* status */
>> +               "movl   8(%%esp), %%ecx;"       /* hob_list */
>>                 "jmp    fsp_continue;"
>> -               : : "m"(params_ptr), "a"(init), "c"(&shared_data)
>> +               : : "m"(params_ptr), "a"(init), "b"(&shared_data)
>>         );
>>
>>         /*
>> @@ -209,12 +212,11 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>>         ASSERT(FALSE);
>>  }
>>
>> -#else
>> -
>>  u32 fsp_notify(struct fsp_header_t *fsp_hdr, u32 phase)
>>  {
>>         fsp_notify_f notify;
>>         struct fsp_notify_params_t params;
>> +       struct fsp_notify_params_t *params_ptr;
>>         u32 status;
>>
>>         if (!fsp_hdr)
>> @@ -227,13 +229,22 @@ u32 fsp_notify(struct fsp_header_t *fsp_hdr, u32 phase)
>>
>>         notify = (fsp_notify_f)(fsp_hdr->img_base + fsp_hdr->fsp_notify);
>>         params.phase = phase;
>> -       status = notify(&params);
>> +       params_ptr = ¶ms;
>> +
>> +       /*
>> +        * Use ASM code to ensure correct parameter is on the stack for
>> +        * FspNotify as U-Boot is using different ABI from FSP
>> +        */
>> +       asm volatile (
>> +               "pushl  %1;"            /* push notify phase */
>> +               "call   *%%eax;"        /* call FspNotify */
>> +               "addl   $4, %%esp;"     /* clean up the stack */
>> +               : "=a"(status) : "m"(params_ptr), "a"(notify), "m"(*params_ptr)
>> +       );
>>
>>         return status;
>>  }
>>
>> -#endif  /* __PRE_RAM__ */
>> -
>>  u32 get_usable_lowmem_top(const void *hob_list)
>>  {
>>         union hob_pointers_t hob;
>> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_support.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_support.h
>> index 3e53ea1..3296a2b 100644
>> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_support.h
>> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_support.h
>> @@ -26,7 +26,7 @@ struct shared_data_t {
>
> Can we get rid of the _t suffix on these structures? A follow-on patch
> would be OK if you prefer.

Yes, I will send a follow-on patch.

>>
>>  void asm_continuation(void);
>>
>> -void bl_main_continue(void *hob_list, struct shared_data_t *shared_data);
>> +void fsp_init_done(void *hob_list);
>
> Function comments for these?
>

Will be fixed in the follow-on patch.

Regards,
Bin