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. > > 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? Regards, Simon _______________________________________________ U-Boot mailing list U-Boot@lists.denx.de http://lists.denx.de/mailman/listinfo/u-boot