| Message ID | 1418389545-11254-12-git-send-email-bmeng.cn@gmail.com |
|---|---|
| State | Accepted |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Bin, On 12 December 2014 at 06:05, Bin Meng <bmeng.cn@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@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
Hi Simon, On Sat, Dec 13, 2014 at 2:49 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Bin, > > On 12 December 2014 at 06:05, Bin Meng <bmeng.cn@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@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. Regards, Bin
Hi Simon, On Sat, Dec 13, 2014 at 12:50 PM, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Sat, Dec 13, 2014 at 2:49 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Bin, >> >> On 12 December 2014 at 06:05, Bin Meng <bmeng.cn@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@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/ Regards, Bin
On 13 December 2014 at 21:17, Bin Meng <bmeng.cn@gmail.com> wrote: > Hi Simon, > > On Sat, Dec 13, 2014 at 12:50 PM, Bin Meng <bmeng.cn@gmail.com> wrote: >> Hi Simon, >> >> On Sat, Dec 13, 2014 at 2:49 AM, Simon Glass <sjg@chromium.org> wrote: >>> Hi Bin, >>> >>> On 12 December 2014 at 06:05, Bin Meng <bmeng.cn@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@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!
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 { void asm_continuation(void); -void bl_main_continue(void *hob_list, struct shared_data_t *shared_data); +void fsp_init_done(void *hob_list); /** * FSP Continuation function
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@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(-)