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