On 13 December 2014 at 21:17, Bin Meng <bmeng...@gmail.com> wrote: > Hi Simon, > > On Sat, Dec 13, 2014 at 12:50 PM, Bin Meng <bmeng...@gmail.com> wrote: >> Hi Simon, >> >> On Sat, Dec 13, 2014 at 2:49 AM, Simon Glass <s...@chromium.org> wrote: >>> Hi Bin, >>> >>> On 12 December 2014 at 06:05, Bin Meng <bmeng...@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...@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(¶ms); >>>> + 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. >> > > These two issues are fixed in the follow-on patch @ > http://patchwork.ozlabs.org/patch/420827/
Applied to u-boot-x86, thanks! _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot