[U-Boot] [PATCH v2] x86: Clean up the FSP support codes
Bin Meng
bmeng.cn at gmail.com
Wed Dec 17 04:12:45 CET 2014
Hi Simon,
On Tue, Dec 16, 2014 at 1:18 PM, Simon Glass <sjg at chromium.org> wrote:
> Hi Bin,
>
> On 15 December 2014 at 08:03, Bin Meng <bmeng.cn at gmail.com> wrote:
>> This is the follow-on patch to clean up the FSP support codes:
>>
>> - Remove the _t suffix on the structures defines
>> - Use U-Boot's assert()
>> - Use standard bool true/false
>> - Remove read_unaligned64()
>> - Use memcmp() in the compare_guid()
>> - Remove the cast in the memset() call
>> - Replace some magic numbers with macros
>> - Use panic() when no valid FSP image header is found
>> - Change some FSP utility routines to use an fsp_ prefix
>> - Add comment blocks for asm_continuation and fsp_init_done
>> - Add comments to mention find_fsp_header() may be called in a
>> stackless environment
>> - Add comments to mention init(¶ms) in fsp_init() cannot
>> be removed
>>
>> Signed-off-by: Bin Meng <bmeng.cn at gmail.com>
>
> This looks pretty good to me now. I've got a few more comments that
> you can hopefully roll into your final version.
>
> Also I've pushed some updates to u-boot-x86.git branch 'working'.
> Patches 2-7 are the v2 series I just send. Can send a series that
> incorporates those, or put them as your base, perhaps dropping or
> squashing the 'Convert microcode format' patch then resend your
> series? Everything up to your microcode patch is applied to x86/master
> now.
I will rebase my remaining patches on top of u-boot-x86/master and
send the patches soon.
> Then I will retest and push.
>
> BTW I wonder if your series would work with Minnowboard Max?
There are two versions of the Minnow boards. One is Minnow board based
on Atom E6xx (the same Queensbay platform) and with my series it
should be pretty easy to get U-Boot up and running on that board. The
other version is a newer version called Minnow board Max which has an
Intel Atom E38xx SoC (BayTrail platform). Luckily Intel has released
FSP for BayTrail as well. So it should not take too much effort
supporting Minnow board Max with Intel FSP on top of my series.
Minnow board: http://www.minnowboard.org/technical-features/
Minnow board Max: http://www.minnowboard.org/meet-minnowboard-max/
>>
>> ---
>>
>> Changes in v2:
>> - Remove some casts in find_fsp_header()
>> - Change HOB access macros to static inline routines
>>
>> arch/x86/cpu/queensbay/fsp_configs.c | 2 +-
>> arch/x86/cpu/queensbay/fsp_support.c | 253 ++++++++++-----------
>> arch/x86/cpu/queensbay/tnc_dram.c | 18 +-
>> arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h | 14 +-
>> arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h | 24 +-
>> arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h | 14 +-
>> arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h | 112 +++++----
>> .../asm/arch-queensbay/fsp/fsp_infoheader.h | 2 +-
>> .../include/asm/arch-queensbay/fsp/fsp_platform.h | 4 +-
>> .../include/asm/arch-queensbay/fsp/fsp_support.h | 63 ++---
>> .../x86/include/asm/arch-queensbay/fsp/fsp_types.h | 17 +-
>> arch/x86/include/asm/arch-queensbay/fsp/fsp_vpd.h | 6 +-
>> arch/x86/lib/cmd_hob.c | 12 +-
>> 13 files changed, 275 insertions(+), 266 deletions(-)
>>
>> diff --git a/arch/x86/cpu/queensbay/fsp_configs.c b/arch/x86/cpu/queensbay/fsp_configs.c
>> index aef18fc..af28e45 100644
>> --- a/arch/x86/cpu/queensbay/fsp_configs.c
>> +++ b/arch/x86/cpu/queensbay/fsp_configs.c
>> @@ -8,7 +8,7 @@
>> #include <common.h>
>> #include <asm/arch/fsp/fsp_support.h>
>>
>> -void update_fsp_upd(struct upd_region_t *fsp_upd)
>> +void update_fsp_upd(struct upd_region *fsp_upd)
>> {
>> /* Override any UPD setting if required */
>>
>> diff --git a/arch/x86/cpu/queensbay/fsp_support.c b/arch/x86/cpu/queensbay/fsp_support.c
>> index f830eeb..ef1916b 100644
>> --- a/arch/x86/cpu/queensbay/fsp_support.c
>> +++ b/arch/x86/cpu/queensbay/fsp_support.c
>> @@ -10,67 +10,48 @@
>> #include <asm/post.h>
>>
>> /**
>> - * Reads a 64-bit value from memory that may be unaligned.
>> - *
>> - * This function returns the 64-bit value pointed to by buf. The function
>> - * guarantees that the read operation does not produce an alignment fault.
>> - *
>> - * If the buf is NULL, then ASSERT().
>> - *
>> - * @buf: Pointer to a 64-bit value that may be unaligned.
>> - *
>> - * @return: The 64-bit value read from buf.
>> - */
>> -static u64 read_unaligned64(const u64 *buf)
>> -{
>> - ASSERT(buf != NULL);
>> -
>> - return *buf;
>> -}
>> -
>> -/**
>> * Compares two GUIDs
>> *
>> - * If the GUIDs are identical then TRUE is returned.
>> - * If there are any bit differences in the two GUIDs, then FALSE is returned.
>> - *
>> - * If guid1 is NULL, then ASSERT().
>> - * If guid2 is NULL, then ASSERT().
>> + * If the GUIDs are identical then true is returned.
>> + * If there are any bit differences in the two GUIDs, then false is returned.
>> *
>> * @guid1: A pointer to a 128 bit GUID.
>> * @guid2: A pointer to a 128 bit GUID.
>> *
>> - * @retval TRUE: guid1 and guid2 are identical.
>> - * @retval FALSE: guid1 and guid2 are not identical.
>> + * @retval true: guid1 and guid2 are identical.
>> + * @retval false: guid1 and guid2 are not identical.
>> */
>> -static unsigned char compare_guid(const struct efi_guid_t *guid1,
>> - const struct efi_guid_t *guid2)
>> +static bool compare_guid(const struct efi_guid *guid1,
>> + const struct efi_guid *guid2)
>> {
>> - u64 guid1_low;
>> - u64 guid2_low;
>> - u64 guid1_high;
>> - u64 guid2_high;
>> -
>> - guid1_low = read_unaligned64((const u64 *)guid1);
>> - guid2_low = read_unaligned64((const u64 *)guid2);
>> - guid1_high = read_unaligned64((const u64 *)guid1 + 1);
>> - guid2_high = read_unaligned64((const u64 *)guid2 + 1);
>> -
>> - return (unsigned char)(guid1_low == guid2_low && guid1_high == guid2_high);
>> + if (memcmp(guid1, guid2, sizeof(struct efi_guid)) == 0)
>> + return true;
>> + else
>> + return false;
>> }
>>
>> u32 __attribute__((optimize("O0"))) find_fsp_header(void)
>> {
>> + /*
>> + * This function may be called before the a stack is established,
>> + * so special care must be taken. First, it cannot declare any local
>> + * variable using stack. Only register variable can be used here.
>> + * Secondly, some compiler version will add prolog or epilog code
>> + * for the C function. If so the function call may not work before
>> + * stack is ready.
>> + *
>> + * GCC 4.8.1 has been verified to be working for the following codes.
>> + */
>> volatile register u8 *fsp asm("eax");
>>
>> /* Initalize the FSP base */
>> fsp = (u8 *)CONFIG_FSP_ADDR;
>>
>> /* Check the FV signature, _FVH */
>> - if (((struct fv_header_t *)fsp)->sign == 0x4856465F) {
>> + if (((struct fv_header *)fsp)->sign == EFI_FVH_SIGNATURE) {
>> /* Go to the end of the FV header and align the address */
>> - fsp += ((struct fv_header_t *)fsp)->ext_hdr_off;
>> - fsp += ((struct fv_ext_header_t *)fsp)->ext_hdr_size;
>> + fsp += ((struct fv_header *)fsp)->ext_hdr_off;
>> + fsp += ((struct fv_ext_header *)fsp)->ext_hdr_size;
>> fsp = (u8 *)(((u32)fsp + 7) & 0xFFFFFFF8);
>> } else {
>> fsp = 0;
>> @@ -78,20 +59,27 @@ u32 __attribute__((optimize("O0"))) find_fsp_header(void)
>>
>> /* Check the FFS GUID */
>> if (fsp &&
>> - (((u32 *)&(((struct ffs_file_header_t *)fsp)->name))[0] == 0x912740BE) &&
>> - (((u32 *)&(((struct ffs_file_header_t *)fsp)->name))[1] == 0x47342284) &&
>> - (((u32 *)&(((struct ffs_file_header_t *)fsp)->name))[2] == 0xB08471B9) &&
>> - (((u32 *)&(((struct ffs_file_header_t *)fsp)->name))[3] == 0x0C3F3527)) {
>> + ((struct ffs_file_header *)fsp)->name.data1 == FSP_GUID_DATA1 &&
>> + ((struct ffs_file_header *)fsp)->name.data2 == FSP_GUID_DATA2 &&
>> + ((struct ffs_file_header *)fsp)->name.data3 == FSP_GUID_DATA3 &&
>> + ((struct ffs_file_header *)fsp)->name.data4[0] == FSP_GUID_DATA4_0 &&
>> + ((struct ffs_file_header *)fsp)->name.data4[1] == FSP_GUID_DATA4_1 &&
>> + ((struct ffs_file_header *)fsp)->name.data4[2] == FSP_GUID_DATA4_2 &&
>> + ((struct ffs_file_header *)fsp)->name.data4[3] == FSP_GUID_DATA4_3 &&
>> + ((struct ffs_file_header *)fsp)->name.data4[4] == FSP_GUID_DATA4_4 &&
>> + ((struct ffs_file_header *)fsp)->name.data4[5] == FSP_GUID_DATA4_5 &&
>> + ((struct ffs_file_header *)fsp)->name.data4[6] == FSP_GUID_DATA4_6 &&
>> + ((struct ffs_file_header *)fsp)->name.data4[7] == FSP_GUID_DATA4_7) {
>> /* Add the FFS header size to find the raw section header */
>> - fsp += sizeof(struct ffs_file_header_t);
>> + fsp += sizeof(struct ffs_file_header);
>> } else {
>> fsp = 0;
>> }
>>
>> if (fsp &&
>> - ((struct raw_section_t *)fsp)->type == EFI_SECTION_RAW) {
>> + ((struct raw_section *)fsp)->type == EFI_SECTION_RAW) {
>> /* Add the raw section header size to find the FSP header */
>> - fsp += sizeof(struct raw_section_t);
>> + fsp += sizeof(struct raw_section);
>> } else {
>> fsp = 0;
>> }
>> @@ -99,7 +87,7 @@ u32 __attribute__((optimize("O0"))) find_fsp_header(void)
>> return (u32)fsp;
>> }
>>
>> -void fsp_continue(struct shared_data_t *shared_data, u32 status, void *hob_list)
>> +void fsp_continue(struct shared_data *shared_data, u32 status, void *hob_list)
>> {
>> u32 stack_len;
>> u32 stack_base;
>> @@ -107,18 +95,18 @@ void fsp_continue(struct shared_data_t *shared_data, u32 status, void *hob_list)
>>
>> post_code(POST_MRC);
>>
>> - ASSERT(status == 0);
>> + assert(status == 0);
>>
>> /* Get the migrated stack in normal memory */
>> - stack_base = (u32)get_bootloader_tmp_mem(hob_list, &stack_len);
>> - ASSERT(stack_base != 0);
>> + stack_base = (u32)fsp_get_bootloader_tmp_mem(hob_list, &stack_len);
>> + assert(stack_base != 0);
>> stack_top = stack_base + stack_len - sizeof(u32);
>>
>> /*
>> * Old stack base is stored at the very end of the stack top,
>> * use it to calculate the migrated shared data base
>> */
>> - shared_data = (struct shared_data_t *)(stack_base +
>> + shared_data = (struct shared_data *)(stack_base +
>> ((u32)shared_data - *(u32 *)stack_top));
>>
>> /* The boot loader main function entry */
>> @@ -127,50 +115,50 @@ void fsp_continue(struct shared_data_t *shared_data, u32 status, void *hob_list)
>>
>> void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>> {
>> - struct shared_data_t shared_data;
>> + struct shared_data shared_data;
>> fsp_init_f init;
>> - struct fsp_init_params_t params;
>> - struct fspinit_rtbuf_t rt_buf;
>> - struct vpd_region_t *fsp_vpd;
>> - struct fsp_header_t *fsp_hdr;
>> - struct fsp_init_params_t *params_ptr;
>> - struct upd_region_t *fsp_upd;
>> -
>> - fsp_hdr = (struct fsp_header_t *)find_fsp_header();
>> + struct fsp_init_params params;
>> + struct fspinit_rtbuf rt_buf;
>> + struct vpd_region *fsp_vpd;
>> + struct fsp_header *fsp_hdr;
>> + struct fsp_init_params *params_ptr;
>> + struct upd_region *fsp_upd;
>> +
>> + fsp_hdr = (struct fsp_header *)find_fsp_header();
>> if (fsp_hdr == NULL) {
>> /* No valid FSP info header was found */
>> - ASSERT(FALSE);
>> + panic("Invalid FSP header");
>> }
>>
>> - fsp_upd = (struct upd_region_t *)&shared_data.fsp_upd;
>> - memset((void *)&rt_buf, 0, sizeof(struct fspinit_rtbuf_t));
>> + fsp_upd = (struct upd_region *)&shared_data.fsp_upd;
>> + memset(&rt_buf, 0, sizeof(struct fspinit_rtbuf));
>>
>> /* Reserve a gap in stack top */
>> rt_buf.common.stack_top = (u32 *)stack_top - 32;
>> rt_buf.common.boot_mode = boot_mode;
>> - rt_buf.common.upd_data = (struct upd_region_t *)fsp_upd;
>> + rt_buf.common.upd_data = (struct upd_region *)fsp_upd;
>>
>> /* Get VPD region start */
>> - fsp_vpd = (struct vpd_region_t *)(fsp_hdr->img_base +
>> + fsp_vpd = (struct vpd_region *)(fsp_hdr->img_base +
>> fsp_hdr->cfg_region_off);
>>
>> /* Verifify the VPD data region is valid */
>> - ASSERT((fsp_vpd->img_rev == VPD_IMAGE_REV) &&
>> + assert((fsp_vpd->img_rev == VPD_IMAGE_REV) &&
>> (fsp_vpd->sign == VPD_IMAGE_ID));
>>
>> /* Copy default data from Flash */
>> memcpy(fsp_upd, (void *)(fsp_hdr->img_base + fsp_vpd->upd_offset),
>> - sizeof(struct upd_region_t));
>> + sizeof(struct upd_region));
>>
>> /* Verifify the UPD data region is valid */
>> - ASSERT(fsp_upd->terminator == 0x55AA);
>> + assert(fsp_upd->terminator == UPD_TERMINATOR);
>>
>> /* Override any UPD setting if required */
>> update_fsp_upd(fsp_upd);
>>
>> - memset((void *)¶ms, 0, sizeof(struct fsp_init_params_t));
>> + memset(¶ms, 0, sizeof(struct fsp_init_params));
>> params.nvs_buf = nvs_buf;
>> - params.rt_buf = (struct fspinit_rtbuf_t *)&rt_buf;
>> + params.rt_buf = (struct fspinit_rtbuf *)&rt_buf;
>> params.continuation = (fsp_continuation_f)asm_continuation;
>>
>> init = (fsp_init_f)(fsp_hdr->img_base + fsp_hdr->fsp_init);
>> @@ -199,32 +187,28 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf)
>>
>> /*
>> * Should never get here.
>> - * Control will continue from romstage_main_continue_asm.
>> + * Control will continue from fsp_continue.
>> * This line below is to prevent the compiler from optimizing
>> * structure intialization.
>> + *
>> + * DO NOT REMOVE!
>> */
>> init(¶ms);
>> -
>> - /*
>> - * Should never return.
>> - * Control will continue from ContinuationFunc
>> - */
>> - ASSERT(FALSE);
>> }
>>
>> -u32 fsp_notify(struct fsp_header_t *fsp_hdr, u32 phase)
>> +u32 fsp_notify(struct fsp_header *fsp_hdr, u32 phase)
>> {
>> fsp_notify_f notify;
>> - struct fsp_notify_params_t params;
>> - struct fsp_notify_params_t *params_ptr;
>> + struct fsp_notify_params params;
>> + struct fsp_notify_params *params_ptr;
>> u32 status;
>>
>> if (!fsp_hdr)
>> - fsp_hdr = (struct fsp_header_t *)find_fsp_header();
>> + fsp_hdr = (struct fsp_header *)find_fsp_header();
>>
>> if (fsp_hdr == NULL) {
>> /* No valid FSP info header */
>> - ASSERT(FALSE);
>> + panic("Invalid FSP header");
>> }
>>
>> notify = (fsp_notify_f)(fsp_hdr->img_base + fsp_hdr->fsp_notify);
>> @@ -245,9 +229,9 @@ u32 fsp_notify(struct fsp_header_t *fsp_hdr, u32 phase)
>> return status;
>> }
>>
>> -u32 get_usable_lowmem_top(const void *hob_list)
>> +u32 fsp_get_usable_lowmem_top(const void *hob_list)
>> {
>> - union hob_pointers_t hob;
>> + union hob_pointers hob;
>> phys_addr_t phys_start;
>> u32 top;
>>
>> @@ -255,26 +239,26 @@ u32 get_usable_lowmem_top(const void *hob_list)
>> hob.raw = (void *)hob_list;
>>
>> /* * Collect memory ranges */
>> - top = 0x100000;
>> - while (!END_OF_HOB(hob)) {
>> - if (hob.hdr->type == HOB_TYPE_RES_DESC) {
>> + top = FSP_LOWMEM_BASE;
>> + while (!end_of_hob(hob)) {
>> + if (get_hob_type(hob) == HOB_TYPE_RES_DESC) {
>> if (hob.res_desc->type == RES_SYS_MEM) {
>> phys_start = hob.res_desc->phys_start;
>> /* Need memory above 1MB to be collected here */
>> - if (phys_start >= 0x100000 &&
>> - phys_start < (phys_addr_t)0x100000000)
>> + if (phys_start >= FSP_LOWMEM_BASE &&
>> + phys_start < (phys_addr_t)FSP_HIGHMEM_BASE)
>> top += (u32)(hob.res_desc->len);
>> }
>> }
>> - hob.raw = GET_NEXT_HOB(hob);
>> + hob.raw = get_next_hob(hob);
>> }
>>
>> return top;
>> }
>>
>> -u64 get_usable_highmem_top(const void *hob_list)
>> +u64 fsp_get_usable_highmem_top(const void *hob_list)
>> {
>> - union hob_pointers_t hob;
>> + union hob_pointers hob;
>> phys_addr_t phys_start;
>> u64 top;
>>
>> @@ -282,33 +266,33 @@ u64 get_usable_highmem_top(const void *hob_list)
>> hob.raw = (void *)hob_list;
>>
>> /* Collect memory ranges */
>> - top = 0x100000000;
>> - while (!END_OF_HOB(hob)) {
>> - if (hob.hdr->type == HOB_TYPE_RES_DESC) {
>> + top = FSP_HIGHMEM_BASE;
>> + while (!end_of_hob(hob)) {
>> + if (get_hob_type(hob) == HOB_TYPE_RES_DESC) {
>> if (hob.res_desc->type == RES_SYS_MEM) {
>> phys_start = hob.res_desc->phys_start;
>> /* Need memory above 1MB to be collected here */
>> - if (phys_start >= (phys_addr_t)0x100000000)
>> + if (phys_start >= (phys_addr_t)FSP_HIGHMEM_BASE)
>> top += (u32)(hob.res_desc->len);
>> }
>> }
>> - hob.raw = GET_NEXT_HOB(hob);
>> + hob.raw = get_next_hob(hob);
>> }
>>
>> return top;
>> }
>>
>> -u64 get_fsp_reserved_mem_from_guid(const void *hob_list, u64 *len,
>> - struct efi_guid_t *guid)
>> +u64 fsp_get_reserved_mem_from_guid(const void *hob_list, u64 *len,
>> + struct efi_guid *guid)
>> {
>> - union hob_pointers_t hob;
>> + union hob_pointers hob;
>>
>> /* Get the HOB list for processing */
>> hob.raw = (void *)hob_list;
>>
>> /* Collect memory ranges */
>> - while (!END_OF_HOB(hob)) {
>> - if (hob.hdr->type == HOB_TYPE_RES_DESC) {
>> + while (!end_of_hob(hob)) {
>> + if (get_hob_type(hob) == HOB_TYPE_RES_DESC) {
>> if (hob.res_desc->type == RES_MEM_RESERVED) {
>> if (compare_guid(&hob.res_desc->owner, guid)) {
>> if (len)
>> @@ -318,99 +302,100 @@ u64 get_fsp_reserved_mem_from_guid(const void *hob_list, u64 *len,
>> }
>> }
>> }
>> - hob.raw = GET_NEXT_HOB(hob);
>> + hob.raw = get_next_hob(hob);
>> }
>>
>> return 0;
>> }
>>
>> -u32 get_fsp_reserved_mem(const void *hob_list, u32 *len)
>> +u32 fsp_get_fsp_reserved_mem(const void *hob_list, u32 *len)
>> {
>> - const struct efi_guid_t guid = FSP_HOB_RESOURCE_OWNER_FSP_GUID;
>> + const struct efi_guid guid = FSP_HOB_RESOURCE_OWNER_FSP_GUID;
>> u64 length;
>> u32 base;
>>
>> - base = (u32)get_fsp_reserved_mem_from_guid(hob_list,
>> - &length, (struct efi_guid_t *)&guid);
>> + base = (u32)fsp_get_reserved_mem_from_guid(hob_list,
>> + &length, (struct efi_guid *)&guid);
>> if ((len != 0) && (base != 0))
>> *len = (u32)length;
>>
>> return base;
>> }
>>
>> -u32 get_tseg_reserved_mem(const void *hob_list, u32 *len)
>> +u32 fsp_get_tseg_reserved_mem(const void *hob_list, u32 *len)
>> {
>> - const struct efi_guid_t guid = FSP_HOB_RESOURCE_OWNER_TSEG_GUID;
>> + const struct efi_guid guid = FSP_HOB_RESOURCE_OWNER_TSEG_GUID;
>> u64 length;
>> u32 base;
>>
>> - base = (u32)get_fsp_reserved_mem_from_guid(hob_list,
>> - &length, (struct efi_guid_t *)&guid);
>> + base = (u32)fsp_get_reserved_mem_from_guid(hob_list,
>> + &length, (struct efi_guid *)&guid);
>> if ((len != 0) && (base != 0))
>> *len = (u32)length;
>>
>> return base;
>> }
>>
>> -void *get_next_hob(u16 type, const void *hob_list)
>> +void *fsp_get_next_hob(u16 type, const void *hob_list)
>> {
>> - union hob_pointers_t hob;
>> + union hob_pointers hob;
>>
>> - ASSERT(hob_list != NULL);
>> + assert(hob_list != NULL);
>>
>> hob.raw = (u8 *)hob_list;
>>
>> /* Parse the HOB list until end of list or matching type is found */
>> - while (!END_OF_HOB(hob)) {
>> - if (hob.hdr->type == type)
>> + while (!end_of_hob(hob)) {
>> + if (get_hob_type(hob) == type)
>> return hob.raw;
>>
>> - hob.raw = GET_NEXT_HOB(hob);
>> + hob.raw = get_next_hob(hob);
>> }
>>
>> return NULL;
>> }
>>
>> -void *get_next_guid_hob(const struct efi_guid_t *guid, const void *hob_list)
>> +void *fsp_get_next_guid_hob(const struct efi_guid *guid, const void *hob_list)
>> {
>> - union hob_pointers_t hob;
>> + union hob_pointers hob;
>>
>> hob.raw = (u8 *)hob_list;
>> - while ((hob.raw = get_next_hob(HOB_TYPE_GUID_EXT,
>> + while ((hob.raw = fsp_get_next_hob(HOB_TYPE_GUID_EXT,
>> hob.raw)) != NULL) {
>> if (compare_guid(guid, &hob.guid->name))
>> break;
>> - hob.raw = GET_NEXT_HOB(hob);
>> + hob.raw = get_next_hob(hob);
>> }
>>
>> return hob.raw;
>> }
>>
>> -void *get_guid_hob_data(const void *hob_list, u32 *len, struct efi_guid_t *guid)
>> +void *fsp_get_guid_hob_data(const void *hob_list, u32 *len,
>> + struct efi_guid *guid)
>> {
>> u8 *guid_hob;
>>
>> - guid_hob = get_next_guid_hob(guid, hob_list);
>> + guid_hob = fsp_get_next_guid_hob(guid, hob_list);
>> if (guid_hob == NULL) {
>> return NULL;
>> } else {
>> if (len)
>> - *len = GET_GUID_HOB_DATA_SIZE(guid_hob);
>> + *len = get_guid_hob_data_size(guid_hob);
>>
>> - return GET_GUID_HOB_DATA(guid_hob);
>> + return get_guid_hob_data(guid_hob);
>> }
>> }
>>
>> -void *get_fsp_nvs_data(const void *hob_list, u32 *len)
>> +void *fsp_get_nvs_data(const void *hob_list, u32 *len)
>> {
>> - const struct efi_guid_t guid = FSP_NON_VOLATILE_STORAGE_HOB_GUID;
>> + const struct efi_guid guid = FSP_NON_VOLATILE_STORAGE_HOB_GUID;
>>
>> - return get_guid_hob_data(hob_list, len, (struct efi_guid_t *)&guid);
>> + return fsp_get_guid_hob_data(hob_list, len, (struct efi_guid *)&guid);
>> }
>>
>> -void *get_bootloader_tmp_mem(const void *hob_list, u32 *len)
>> +void *fsp_get_bootloader_tmp_mem(const void *hob_list, u32 *len)
>> {
>> - const struct efi_guid_t guid = FSP_BOOTLOADER_TEMP_MEM_HOB_GUID;
>> + const struct efi_guid guid = FSP_BOOTLOADER_TEMP_MEM_HOB_GUID;
>>
>> - return get_guid_hob_data(hob_list, len, (struct efi_guid_t *)&guid);
>> + return fsp_get_guid_hob_data(hob_list, len, (struct efi_guid *)&guid);
>> }
>> diff --git a/arch/x86/cpu/queensbay/tnc_dram.c b/arch/x86/cpu/queensbay/tnc_dram.c
>> index dbc1710..8e97c9b 100644
>> --- a/arch/x86/cpu/queensbay/tnc_dram.c
>> +++ b/arch/x86/cpu/queensbay/tnc_dram.c
>> @@ -14,17 +14,17 @@ DECLARE_GLOBAL_DATA_PTR;
>> int dram_init(void)
>> {
>> phys_size_t ram_size = 0;
>> - union hob_pointers_t hob;
>> + union hob_pointers hob;
>>
>> hob.raw = gd->arch.hob_list;
>> - while (!END_OF_HOB(hob)) {
>> - if (hob.hdr->type == HOB_TYPE_RES_DESC) {
>> + while (!end_of_hob(hob)) {
>> + if (get_hob_type(hob) == HOB_TYPE_RES_DESC) {
>> if (hob.res_desc->type == RES_SYS_MEM ||
>> hob.res_desc->type == RES_MEM_RESERVED) {
>> ram_size += hob.res_desc->len;
>> }
>> }
>> - hob.raw = GET_NEXT_HOB(hob);
>> + hob.raw = get_next_hob(hob);
>> }
>>
>> gd->ram_size = ram_size;
>> @@ -49,19 +49,19 @@ void dram_init_banksize(void)
>> */
>> ulong board_get_usable_ram_top(ulong total_size)
>> {
>> - return get_usable_lowmem_top(gd->arch.hob_list);
>> + return fsp_get_usable_lowmem_top(gd->arch.hob_list);
>> }
>>
>> unsigned install_e820_map(unsigned max_entries, struct e820entry *entries)
>> {
>> unsigned num_entries = 0;
>>
>> - union hob_pointers_t hob;
>> + union hob_pointers hob;
>>
>> hob.raw = gd->arch.hob_list;
>>
>> - while (!END_OF_HOB(hob)) {
>> - if (hob.hdr->type == HOB_TYPE_RES_DESC) {
>> + while (!end_of_hob(hob)) {
>> + if (get_hob_type(hob) == HOB_TYPE_RES_DESC) {
>> entries[num_entries].addr = hob.res_desc->phys_start;
>> entries[num_entries].size = hob.res_desc->len;
>>
>> @@ -70,7 +70,7 @@ unsigned install_e820_map(unsigned max_entries, struct e820entry *entries)
>> else if (hob.res_desc->type == RES_MEM_RESERVED)
>> entries[num_entries].type = E820_RESERVED;
>> }
>> - hob.raw = GET_NEXT_HOB(hob);
>> + hob.raw = get_next_hob(hob);
>> num_entries++;
>> }
>>
>> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h
>> index 25b938f..95f0e06 100644
>> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h
>> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_api.h
>> @@ -16,7 +16,7 @@ typedef void (*fsp_continuation_f)(u32 status, void *hob_list);
>>
>> #pragma pack(1)
>
> Can we used __packed on each structure instead? Please fix elsewhere
> too as this seems to be the U-Boot standard.
Sure, will fix it in next version.
>>
>> -struct fsp_init_params_t {
>> +struct fsp_init_params {
>> /* Non-volatile storage buffer pointer */
>> void *nvs_buf;
>> /* Runtime buffer pointer */
>> @@ -25,7 +25,7 @@ struct fsp_init_params_t {
>> fsp_continuation_f continuation;
>> };
>>
>> -struct common_buf_t {
>> +struct common_buf {
>> /*
>> * Stack top pointer used by the bootloader. The new stack frame will be
>> * set up at this location after FspInit API call.
>> @@ -36,24 +36,24 @@ struct common_buf_t {
>> u32 reserved[7]; /* Reserved */
>> };
>>
>> -enum fsp_phase_t {
>> +enum fsp_phase {
>> /* Notification code for post PCI enuermation */
>> INIT_PHASE_PCI = 0x20,
>> /* Notification code before transfering control to the payload */
>> INIT_PHASE_BOOT = 0x40
>> };
>>
>> -struct fsp_notify_params_t {
>> +struct fsp_notify_params {
>> /* Notification phase used for NotifyPhase API */
>> - enum fsp_phase_t phase;
>> + enum fsp_phase phase;
>> };
>>
>> #pragma pack()
>>
>> /* FspInit API function prototype */
>> -typedef u32 (*fsp_init_f)(struct fsp_init_params_t *param);
>> +typedef u32 (*fsp_init_f)(struct fsp_init_params *params);
>>
>> /* FspNotify API function prototype */
>> -typedef u32 (*fsp_notify_f)(struct fsp_notify_params_t *param);
>> +typedef u32 (*fsp_notify_f)(struct fsp_notify_params *params);
>>
>> #endif
>> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h
>> index 1f73680..7fef3e7 100644
>> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h
>> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_ffs.h
>> @@ -11,7 +11,7 @@
>> #pragma pack(1)
>>
>> /* Used to verify the integrity of the file */
>> -union ffs_integrity_t {
>> +union ffs_integrity {
>> struct {
>> /*
>> * The IntegrityCheck.checksum.header field is an 8-bit
>> @@ -43,14 +43,14 @@ union ffs_integrity_t {
>> * Each file begins with the header that describe the
>> * contents and state of the files.
>> */
>> -struct ffs_file_header_t {
>> +struct ffs_file_header {
>> /*
>> * This GUID is the file name.
>> * It is used to uniquely identify the file.
>> */
>> - struct efi_guid_t name;
>> + struct efi_guid name;
>> /* Used to verify the integrity of the file */
>> - union ffs_integrity_t integrity;
>> + union ffs_integrity integrity;
>> /* Identifies the type of file */
>> u8 type;
>> /* Declares various file attribute bits */
>> @@ -64,16 +64,16 @@ struct ffs_file_header_t {
>> u8 state;
>> };
>>
>> -struct ffs_file_header2_t {
>> +struct ffs_file_header2 {
>> /*
>> * This GUID is the file name. It is used to uniquely identify the file.
>> * There may be only one instance of a file with the file name GUID of
>> * Name in any given firmware volume, except if the file type is
>> * EFI_FV_FILE_TYPE_FFS_PAD.
>> */
>> - struct efi_guid_t name;
>> + struct efi_guid name;
>> /* Used to verify the integrity of the file */
>> - union ffs_integrity_t integrity;
>> + union ffs_integrity integrity;
>> /* Identifies the type of file */
>> u8 type;
>> /* Declares various file attribute bits */
>> @@ -81,9 +81,9 @@ struct ffs_file_header2_t {
>> /*
>> * The length of the file in bytes, including the FFS header.
>> * The length of the file data is either
>> - * (size - sizeof(struct ffs_file_header_t)). This calculation means a
>> + * (size - sizeof(struct ffs_file_header)). This calculation means a
>> * zero-length file has a size of 24 bytes, which is
>> - * sizeof(struct ffs_file_header_t). Size is not required to be a
>> + * sizeof(struct ffs_file_header). Size is not required to be a
>> * multiple of 8 bytes. Given a file F, the next file header is located
>> * at the next 8-byte aligned firmware volume offset following the last
>> * byte of the file F.
>> @@ -98,7 +98,7 @@ struct ffs_file_header2_t {
>> * If FFS_ATTRIB_LARGE_FILE is set in attr, then ext_size exists
>> * and size must be set to zero.
>> * If FFS_ATTRIB_LARGE_FILE is not set then
>> - * struct ffs_file_header_t is used.
>> + * struct ffs_file_header is used.
>> */
>> u32 ext_size;
>> };
>> @@ -129,7 +129,7 @@ struct ffs_file_header2_t {
>> #define EFI_SECTION_SMM_DEPEX 0x1C
>>
>> /* Common section header */
>> -struct raw_section_t {
>> +struct raw_section {
>> /*
>> * A 24-bit unsigned integer that contains the total size of
>> * the section in bytes, including the EFI_COMMON_SECTION_HEADER.
>> @@ -138,7 +138,7 @@ struct raw_section_t {
>> u8 type;
>> };
>>
>> -struct raw_section2_t {
>> +struct raw_section2 {
>> /*
>> * A 24-bit unsigned integer that contains the total size of
>> * the section in bytes, including the EFI_COMMON_SECTION_HEADER.
>> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h
>> index 01300db..a024451 100644
>> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h
>> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_fv.h
>> @@ -63,7 +63,7 @@
>> #define EFI_FVB2_ALIGNMENT_1G 0x001E0000
>> #define EFI_FVB2_ALIGNMENT_2G 0x001F0000
>>
>> -struct fv_blkmap_entry_t {
>> +struct fv_blkmap_entry {
>> /* The number of sequential blocks which are of the same size */
>> u32 num_blocks;
>> /* The size of the blocks */
>> @@ -71,7 +71,7 @@ struct fv_blkmap_entry_t {
>> };
>>
>> /* Describes the features and layout of the firmware volume */
>> -struct fv_header_t {
>> +struct fv_header {
>> /*
>> * The first 16 bytes are reserved to allow for the reset vector of
>> * processors whose reset vector is at address 0.
>> @@ -81,7 +81,7 @@ struct fv_header_t {
>> * Declares the file system with which the firmware volume
>> * is formatted.
>> */
>> - struct efi_guid_t fs_guid;
>> + struct efi_guid fs_guid;
>
> space before tab here I think.
No, actually there are two tabs after efi_guid.
>> /*
>> * Length in bytes of the complete firmware volume, including
>> * the header.
>> @@ -118,18 +118,18 @@ struct fv_header_t {
>> * An array of run-length encoded FvBlockMapEntry structures.
>> * The array is terminated with an entry of {0,0}.
>> */
>> - struct fv_blkmap_entry_t block_map[1];
>> + struct fv_blkmap_entry block_map[1];
>> };
>>
>> -#define EFI_FVH_SIGNATURE SIGNATURE_32('_', 'F', 'V', 'H')
>> +#define EFI_FVH_SIGNATURE SIGNATURE_32('_', 'F', 'V', 'H')
>>
>> /* Firmware Volume Header Revision definition */
>> #define EFI_FVH_REVISION 0x02
>>
>> /* Extension header pointed by ExtHeaderOffset of volume header */
>> -struct fv_ext_header_t {
>> +struct fv_ext_header {
>> /* firmware volume name */
>> - struct efi_guid_t fv_name;
>> + struct efi_guid fv_name;
>
> and here? Actually it seems to be after each efi_guid.
No, actually there are two tabs after efi_guid.
>> /* Size of the rest of the extension header including this structure */
>> u32 ext_hdr_size;
>> };
>> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h
>> index 44c0f90..380b64e 100644
>> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h
>> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_hob.h
>> @@ -19,14 +19,14 @@
>> * Describes the format and size of the data inside the HOB.
>> * All HOBs must contain this generic HOB header.
>> */
>> -struct hob_header_t {
>> +struct hob_header {
>> u16 type; /* HOB type */
>> u16 len; /* HOB length */
>> u32 reserved; /* always zero */
>> };
>>
>> /* Enumeration of memory types introduced in UEFI */
>> -enum efi_mem_type_t {
>> +enum efi_mem_type {
>> EFI_RESERVED_MEMORY_TYPE,
>> /*
>> * The code portions of a loaded application.
>> @@ -87,16 +87,16 @@ enum efi_mem_type_t {
>> * exist outside the HOB list. This HOB type describes how memory is used,
>> * not the physical attributes of memory.
>> */
>> -struct hob_mem_alloc_t {
>> - struct hob_header_t hdr;
>> +struct hob_mem_alloc {
>> + struct hob_header hdr;
>> /*
>> * A GUID that defines the memory allocation region's type and purpose,
>> * as well as other fields within the memory allocation HOB. This GUID
>> * is used to define the additional data within the HOB that may be
>> - * present for the memory allocation HOB. Type efi_guid_t is defined in
>> + * present for the memory allocation HOB. Type efi_guid is defined in
>> * InstallProtocolInterface() in the UEFI 2.0 specification.
>> */
>> - struct efi_guid_t name;
>> + struct efi_guid name;
>> /*
>> * The base address of memory allocated by this HOB.
>> * Type phys_addr_t is defined in AllocatePages() in the UEFI 2.0
>> @@ -111,7 +111,7 @@ struct hob_mem_alloc_t {
>> * Type EFI_MEMORY_TYPE is defined in AllocatePages() in the UEFI 2.0
>> * specification.
>> */
>> - enum efi_mem_type_t mem_type;
>> + enum efi_mem_type mem_type;
>> /* padding */
>> u8 reserved[4];
>> };
>> @@ -155,14 +155,14 @@ struct hob_mem_alloc_t {
>> * Describes the resource properties of all fixed, nonrelocatable resource
>> * ranges found on the processor host bus during the HOB producer phase.
>> */
>> -struct hob_res_desc_t {
>> - struct hob_header_t hdr;
>> +struct hob_res_desc {
>> + struct hob_header hdr;
>> /*
>> * A GUID representing the owner of the resource. This GUID is
>> * used by HOB consumer phase components to correlate device
>> * ownership of a resource.
>> */
>> - struct efi_guid_t owner;
>> + struct efi_guid owner;
>> u32 type;
>> u32 attr;
>> /* The physical start address of the resource region */
>> @@ -175,24 +175,24 @@ struct hob_res_desc_t {
>> * Allows writers of executable content in the HOB producer phase to
>> * maintain and manage HOBs with specific GUID.
>> */
>> -struct hob_guid_t {
>> - struct hob_header_t hdr;
>> +struct hob_guid {
>> + struct hob_header hdr;
>> /* A GUID that defines the contents of this HOB */
>> - struct efi_guid_t name;
>> + struct efi_guid name;
>> /* GUID specific data goes here */
>> };
>>
>> /* Union of all the possible HOB Types */
>> -union hob_pointers_t {
>> - struct hob_header_t *hdr;
>> - struct hob_mem_alloc_t *mem_alloc;
>> - struct hob_res_desc_t *res_desc;
>> - struct hob_guid_t *guid;
>> +union hob_pointers {
>> + struct hob_header *hdr;
>> + struct hob_mem_alloc *mem_alloc;
>> + struct hob_res_desc *res_desc;
>> + struct hob_guid *guid;
>> u8 *raw;
>> };
>>
>> /**
>> - * Returns the type of a HOB.
>> + * get_hob_type() - return the type of a HOB
>> *
>> * This macro returns the type field from the HOB header for the
>> * HOB specified by hob.
>> @@ -201,11 +201,13 @@ union hob_pointers_t {
>> *
>> * @return: HOB type.
>> */
>> -#define GET_HOB_TYPE(hob) \
>> - ((*(struct hob_header_t **)&(hob))->type)
>> +static inline u16 get_hob_type(union hob_pointers hob)
>> +{
>> + return hob.hdr->type;
>> +}
>
> Now that you have tidied these up I really question the value of these
> functions. Should we just drop them or are they important. It just
> looks like obfuscation to me, but maybe I'm missing a hidden value?!
Yep, those routines are very simple, but I believe they are of some
value in the codes.
>>
>> /**
>> - * Returns the length, in bytes, of a HOB.
>> + * get_hob_length() - return the length, in bytes, of a HOB
>> *
>> * This macro returns the len field from the HOB header for the
>> * HOB specified by hob.
>> @@ -214,11 +216,13 @@ union hob_pointers_t {
>> *
>> * @return: HOB length.
>> */
>> -#define GET_HOB_LENGTH(hob) \
>> - ((*(struct hob_header_t **)&(hob))->len)
>> +static inline u16 get_hob_length(union hob_pointers hob)
>> +{
>> + return hob.hdr->len;
>> +}
>>
>> /**
>> - * Returns a pointer to the next HOB in the HOB list.
>> + * get_next_hob() - return a pointer to the next HOB in the HOB list
>> *
>> * This macro returns a pointer to HOB that follows the HOB specified by hob
>> * in the HOB List.
>> @@ -227,25 +231,31 @@ union hob_pointers_t {
>> *
>> * @return: A pointer to the next HOB in the HOB list.
>> */
>> -#define GET_NEXT_HOB(hob) \
>> - (void *)(*(u8 **)&(hob) + GET_HOB_LENGTH(hob))
>> +static inline void *get_next_hob(union hob_pointers hob)
>> +{
>> + return (void *)(*(u8 **)&(hob) + get_hob_length(hob));
>> +}
>>
>> /**
>> - * Determines if a HOB is the last HOB in the HOB list.
>> + * end_of_hob() - determine if a HOB is the last HOB in the HOB list
>> *
>> * This macro determine if the HOB specified by hob is the last HOB in the
>> - * HOB list. If hob is last HOB in the HOB list, then TRUE is returned.
>> - * Otherwise, FALSE is returned.
>> + * HOB list. If hob is last HOB in the HOB list, then true is returned.
>> + * Otherwise, false is returned.
>> *
>> * @hob: A pointer to a HOB.
>> *
>> - * @retval TRUE: The HOB specified by hob is the last HOB in the HOB list.
>> - * @retval FALSE: The HOB specified by hob is not the last HOB in the HOB list.
>> + * @retval true: The HOB specified by hob is the last HOB in the HOB list.
>> + * @retval false: The HOB specified by hob is not the last HOB in the HOB list.
>> */
>> -#define END_OF_HOB(hob) (GET_HOB_TYPE(hob) == (u16)HOB_TYPE_EOH)
>> +static inline bool end_of_hob(union hob_pointers hob)
>> +{
>> + return get_hob_type(hob) == HOB_TYPE_EOH;
>> +}
>>
>> /**
>> - * Returns a pointer to data buffer from a HOB of type HOB_TYPE_GUID_EXT.
>> + * get_guid_hob_data() - return a pointer to data buffer from a HOB of
>> + * type HOB_TYPE_GUID_EXT
>> *
>> * This macro returns a pointer to the data buffer in a HOB specified by hob.
>> * hob is assumed to be a HOB of type HOB_TYPE_GUID_EXT.
>> @@ -254,11 +264,14 @@ union hob_pointers_t {
>> *
>> * @return: A pointer to the data buffer in a HOB.
>> */
>> -#define GET_GUID_HOB_DATA(hob) \
>> - (void *)(*(u8 **)&(hob) + sizeof(struct hob_guid_t))
>> +static inline void *get_guid_hob_data(u8 *hob)
>> +{
>> + return (void *)(hob + sizeof(struct hob_guid));
>> +}
>
> This one is probably of some value.
>
>>
>> /**
>> - * Returns the size of the data buffer from a HOB of type HOB_TYPE_GUID_EXT.
>> + * get_guid_hob_data_size() - return the size of the data buffer from a HOB
>> + * of type HOB_TYPE_GUID_EXT
>> *
>> * This macro returns the size, in bytes, of the data buffer in a HOB
>> * specified by hob. hob is assumed to be a HOB of type HOB_TYPE_GUID_EXT.
>> @@ -267,14 +280,31 @@ union hob_pointers_t {
>> *
>> * @return: The size of the data buffer.
>> */
>> -#define GET_GUID_HOB_DATA_SIZE(hob) \
>> - (u16)(GET_HOB_LENGTH(hob) - sizeof(struct hob_guid_t))
>> +static inline u16 get_guid_hob_data_size(u8 *hob)
>> +{
>> + union hob_pointers hob_p = *(union hob_pointers *)hob;
>> + return get_hob_length(hob_p) - sizeof(struct hob_guid);
>> +}
>
> It feels like this one should take a union hob_pointers instead of u8
> *. Also it is only called in one place I think. Is it possibly useful
> elsewhere later?
Yes, currently only one place call this routine, but I think it might be useful.
>>
>> /* FSP specific GUID HOB definitions */
>> +#define FSP_GUID_DATA1 0x912740be
>> +#define FSP_GUID_DATA2 0x2284
>> +#define FSP_GUID_DATA3 0x4734
>> +#define FSP_GUID_DATA4_0 0xb9
>> +#define FSP_GUID_DATA4_1 0x71
>> +#define FSP_GUID_DATA4_2 0x84
>> +#define FSP_GUID_DATA4_3 0xb0
>> +#define FSP_GUID_DATA4_4 0x27
>> +#define FSP_GUID_DATA4_5 0x35
>> +#define FSP_GUID_DATA4_6 0x3f
>> +#define FSP_GUID_DATA4_7 0x0c
>> +
>> #define FSP_HEADER_GUID \
>> { \
>> - 0x912740be, 0x2284, 0x4734, \
>> - {0xb9, 0x71, 0x84, 0xb0, 0x27, 0x35, 0x3f, 0x0c} \
>> + FSP_GUID_DATA1, FSP_GUID_DATA2, FSP_GUID_DATA3, \
>> + { FSP_GUID_DATA4_0, FSP_GUID_DATA4_1, FSP_GUID_DATA4_2, \
>> + FSP_GUID_DATA4_3, FSP_GUID_DATA4_4, FSP_GUID_DATA4_5, \
>> + FSP_GUID_DATA4_6, FSP_GUID_DATA4_7 } \
>> }
>>
>> #define FSP_NON_VOLATILE_STORAGE_HOB_GUID \
>> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_infoheader.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_infoheader.h
>> index ad78bcd..a5edfef 100644
>> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_infoheader.h
>> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_infoheader.h
>> @@ -12,7 +12,7 @@
>>
>> #pragma pack(1)
>>
>> -struct fsp_header_t {
>> +struct fsp_header {
>> u32 sign; /* 'FSPH' */
>> u32 hdr_len; /* header length */
>> u8 reserved1[3];
>> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_platform.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_platform.h
>> index a7b6e6b..67e4b1e 100644
>> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_platform.h
>> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_platform.h
>> @@ -10,8 +10,8 @@
>>
>> #pragma pack(1)
>>
>> -struct fspinit_rtbuf_t {
>> - struct common_buf_t common; /* FSP common runtime data structure */
>> +struct fspinit_rtbuf {
>> + struct common_buf common; /* FSP common runtime data structure */
>> };
>>
>> #pragma pack()
>> 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 3296a2b..3ae1b66 100644
>> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_support.h
>> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_support.h
>> @@ -18,14 +18,30 @@
>> #include "fsp_bootmode.h"
>> #include "fsp_vpd.h"
>>
>> -struct shared_data_t {
>> - struct fsp_header_t *fsp_hdr;
>> +struct shared_data {
>> + struct fsp_header *fsp_hdr;
>> u32 *stack_top;
>> - struct upd_region_t fsp_upd;
>> + struct upd_region fsp_upd;
>> };
>>
>> +#define FSP_LOWMEM_BASE 0x100000UL
>> +#define FSP_HIGHMEM_BASE 0x100000000ULL
>> +
>> +/**
>> + * FSP Continuation assembly helper routine
>> + *
>> + * This routine jumps to the C version of FSP continuation function
>> + */
>> void asm_continuation(void);
>>
>> +/**
>> + * FSP initialization complete
>> + *
>> + * This is the function that indicates FSP initialization is complete and jumps
>> + * back to the bootloader with HOB list pointer as the parameter.
>> + *
>> + * @hob_list: HOB list pointer
>> + */
>> void fsp_init_done(void *hob_list);
>>
>> /**
>> @@ -37,19 +53,12 @@ void fsp_init_done(void *hob_list);
>> *
>> * @retval: Never returns
>> */
>> -void fsp_continue(struct shared_data_t *shared_data, u32 status,
>> +void fsp_continue(struct shared_data *shared_data, u32 status,
>> void *hob_list);
>>
>> /**
>> * Find FSP header offset in FSP image
>> *
>> - * If this function is called before the a stack is established, special care
>> - * must be taken. First, it cannot declare any local variable using stack.
>> - * Only register variable can be used here. Secondly, some compiler version
>> - * will add prolog or epilog code for the C function. If so the function call
>> - * may not work before stack is ready. GCC 4.8.1 has been verified to be
>> - * working for the following code.
>> - *
>> * @retval: the offset of FSP header. If signature is invalid, returns 0.
>> */
>> u32 find_fsp_header(void);
>> @@ -67,11 +76,11 @@ void fsp_init(u32 stack_top, u32 boot_mode, void *nvs_buf);
>> * FSP notification wrapper function
>> *
>> * @fsp_hdr: Pointer to FSP information header
>> - * @phase: FSP initialization phase defined in enum fsp_phase_t
>> + * @phase: FSP initialization phase defined in enum fsp_phase
>> *
>> * @retval: compatible status code with EFI_STATUS defined in PI spec
>> */
>> -u32 fsp_notify(struct fsp_header_t *fsp_hdr, u32 phase);
>> +u32 fsp_notify(struct fsp_header *fsp_hdr, u32 phase);
>>
>> /**
>> * This function retrieves the top of usable low memory.
>> @@ -80,7 +89,7 @@ u32 fsp_notify(struct fsp_header_t *fsp_hdr, u32 phase);
>> *
>> * @retval: Usable low memory top.
>> */
>> -u32 get_usable_lowmem_top(const void *hob_list);
>> +u32 fsp_get_usable_lowmem_top(const void *hob_list);
>>
>> /**
>> * This function retrieves the top of usable high memory.
>> @@ -89,7 +98,7 @@ u32 get_usable_lowmem_top(const void *hob_list);
>> *
>> * @retval: Usable high memory top.
>> */
>> -u64 get_usable_highmem_top(const void *hob_list);
>> +u64 fsp_get_usable_highmem_top(const void *hob_list);
>>
>> /**
>> * This function retrieves a special reserved memory region.
>> @@ -102,8 +111,8 @@ u64 get_usable_highmem_top(const void *hob_list);
>> * @retval: Reserved region start address.
>> * 0 if this region does not exist.
>> */
>> -u64 get_fsp_reserved_mem_from_guid(const void *hob_list,
>> - u64 *len, struct efi_guid_t *guid);
>> +u64 fsp_get_reserved_mem_from_guid(const void *hob_list,
>> + u64 *len, struct efi_guid *guid);
>>
>> /**
>> * This function retrieves the FSP reserved normal memory.
>> @@ -114,7 +123,7 @@ u64 get_fsp_reserved_mem_from_guid(const void *hob_list,
>> * @retval: FSP reserved memory base
>> * 0 if this region does not exist.
>> */
>> -u32 get_fsp_reserved_mem(const void *hob_list, u32 *len);
>> +u32 fsp_get_fsp_reserved_mem(const void *hob_list, u32 *len);
>
> fsp_get_reserved_mem
No, this routine is to get the memory reserved by the FSP itself.
There are memory reserved by some other owners like TSEC (see
fsp_get_tseg_reserved_mem below).
>>
>> /**
>> * This function retrieves the TSEG reserved normal memory.
>> @@ -126,7 +135,7 @@ u32 get_fsp_reserved_mem(const void *hob_list, u32 *len);
>> * @retval NULL: Failed to find the TSEG reserved memory.
>> * @retval others: TSEG reserved memory base.
>> */
>> -u32 get_tseg_reserved_mem(const void *hob_list, u32 *len);
>> +u32 fsp_get_tseg_reserved_mem(const void *hob_list, u32 *len);
>>
>> /**
>> * Returns the next instance of a HOB type from the starting HOB.
>> @@ -136,7 +145,7 @@ u32 get_tseg_reserved_mem(const void *hob_list, u32 *len);
>> *
>> * @retval: A HOB object with matching type; Otherwise NULL.
>> */
>> -void *get_next_hob(u16 type, const void *hob_list);
>> +void *fsp_get_next_hob(u16 type, const void *hob_list);
>>
>> /**
>> * Returns the next instance of the matched GUID HOB from the starting HOB.
>> @@ -146,7 +155,7 @@ void *get_next_hob(u16 type, const void *hob_list);
>> *
>> * @retval: A HOB object with matching GUID; Otherwise NULL.
>> */
>> -void *get_next_guid_hob(const struct efi_guid_t *guid, const void *hob_list);
>> +void *fsp_get_next_guid_hob(const struct efi_guid *guid, const void *hob_list);
>>
>> /**
>> * This function retrieves a GUID HOB data buffer and size.
>> @@ -159,8 +168,8 @@ void *get_next_guid_hob(const struct efi_guid_t *guid, const void *hob_list);
>> * @retval NULL: Failed to find the GUID HOB.
>> * @retval others: GUID HOB data buffer pointer.
>> */
>> -void *get_guid_hob_data(const void *hob_list, u32 *len,
>> - struct efi_guid_t *guid);
>> +void *fsp_get_guid_hob_data(const void *hob_list, u32 *len,
>> + struct efi_guid *guid);
>>
>> /**
>> * This function retrieves FSP Non-volatile Storage HOB buffer and size.
>> @@ -172,7 +181,7 @@ void *get_guid_hob_data(const void *hob_list, u32 *len,
>> * @retval NULL: Failed to find the NVS HOB.
>> * @retval others: FSP NVS data buffer pointer.
>> */
>> -void *get_fsp_nvs_data(const void *hob_list, u32 *len);
>> +void *fsp_get_nvs_data(const void *hob_list, u32 *len);
>>
>> /**
>> * This function retrieves Bootloader temporary stack buffer and size.
>> @@ -184,15 +193,15 @@ void *get_fsp_nvs_data(const void *hob_list, u32 *len);
>> * @retval NULL: Failed to find the bootloader temporary stack HOB.
>> * @retval others: Bootloader temporary stackbuffer pointer.
>> */
>> -void *get_bootloader_tmp_mem(const void *hob_list, u32 *len);
>> +void *fsp_get_bootloader_tmp_mem(const void *hob_list, u32 *len);
>>
>> /**
>> * This function overrides the default configurations in the UPD data region.
>> *
>> - * @fsp_upd: A pointer to the upd_region_t data strcture
>> + * @fsp_upd: A pointer to the upd_region data strcture
>> *
>> * @return: None
>> */
>> -void update_fsp_upd(struct upd_region_t *fsp_upd);
>> +void update_fsp_upd(struct upd_region *fsp_upd);
>>
>> #endif
>> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_types.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_types.h
>> index 12ebbfd..f32d827 100644
>> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_types.h
>> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_types.h
>> @@ -8,20 +8,8 @@
>> #ifndef __FSP_TYPES_H__
>> #define __FSP_TYPES_H__
>>
>> -/*
>> - * Boolean true value. UEFI Specification defines this value to be 1,
>> - * but this form is more portable.
>> - */
>> -#define TRUE ((unsigned char)(1 == 1))
>> -
>> -/*
>> - * Boolean false value. UEFI Specification defines this value to be 0,
>> - * but this form is more portable.
>> - */
>> -#define FALSE ((unsigned char)(0 == 1))
>> -
>> /* 128 bit buffer containing a unique identifier value */
>> -struct efi_guid_t {
>> +struct efi_guid {
>> u32 data1;
>> u16 data2;
>> u16 data3;
>> @@ -80,9 +68,6 @@ struct efi_guid_t {
>> #define SIGNATURE_64(A, B, C, D, E, F, G, H) \
>> (SIGNATURE_32(A, B, C, D) | ((u64)(SIGNATURE_32(E, F, G, H)) << 32))
>>
>> -/* Assertion for debug */
>> -#define ASSERT(exp) do { if (!(exp)) for (;;); } while (FALSE)
>> -
>> /*
>> * Define FSP API return status code.
>> * Compatiable with EFI_STATUS defined in PI Spec.
>> diff --git a/arch/x86/include/asm/arch-queensbay/fsp/fsp_vpd.h b/arch/x86/include/asm/arch-queensbay/fsp/fsp_vpd.h
>> index 11cc32f..ff2bb63 100644
>> --- a/arch/x86/include/asm/arch-queensbay/fsp/fsp_vpd.h
>> +++ b/arch/x86/include/asm/arch-queensbay/fsp/fsp_vpd.h
>> @@ -12,7 +12,9 @@
>>
>> #pragma pack(1)
>>
>> -struct upd_region_t {
>> +#define UPD_TERMINATOR 0x55AA
>> +
>> +struct upd_region {
>> u64 sign; /* Offset 0x0000 */
>> u64 reserved; /* Offset 0x0008 */
>> u8 dummy[240]; /* Offset 0x0010 */
>> @@ -39,7 +41,7 @@ struct upd_region_t {
>> #define VPD_IMAGE_ID 0x445056574F4E4E4D /* 'MNNOWVPD' */
>> #define VPD_IMAGE_REV 0x00000301
>>
>> -struct vpd_region_t {
>> +struct vpd_region {
>> u64 sign; /* Offset 0x0000 */
>> u32 img_rev; /* Offset 0x0008 */
>> u32 upd_offset; /* Offset 0x000C */
>> diff --git a/arch/x86/lib/cmd_hob.c b/arch/x86/lib/cmd_hob.c
>> index 2fdff2b..bfa2863 100644
>> --- a/arch/x86/lib/cmd_hob.c
>> +++ b/arch/x86/lib/cmd_hob.c
>> @@ -28,7 +28,7 @@ static char *hob_type[] = {
>>
>> int do_hob(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>> {
>> - union hob_pointers_t hob;
>> + union hob_pointers hob;
>> u16 type;
>> char *desc;
>> int i = 0;
>> @@ -39,9 +39,9 @@ int do_hob(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>
>> printf("No. | Address | Type | Length in Bytes\n");
>> printf("----|----------|---------------------|----------------\n");
>> - while (!END_OF_HOB(hob)) {
>> + while (!end_of_hob(hob)) {
>> printf("%-3d | %08x | ", i, (unsigned int)hob.raw);
>> - type = hob.hdr->type;
>> + type = get_hob_type(hob);
>> if (type == HOB_TYPE_UNUSED)
>> desc = "*Unused*";
>> else if (type == HOB_TYPE_EOH)
>> @@ -50,16 +50,14 @@ int do_hob(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>> desc = hob_type[type];
>> else
>> desc = "!!!Invalid Type!!!";
>> - printf("%-19s | %-15d\n", desc, hob.hdr->len);
>> - hob.raw = GET_NEXT_HOB(hob);
>> + printf("%-19s | %-15d\n", desc, get_hob_length(hob));
>> + hob.raw = get_next_hob(hob);
>> i++;
>> }
>>
>> return 0;
>> }
>>
>> -/* -------------------------------------------------------------------- */
>> -
>> U_BOOT_CMD(
>> hob, 1, 1, do_hob,
>> "print FSP Hand-Off Block information",
>> --
>> 1.8.2.1
Regards,
Bin
More information about the U-Boot
mailing list