[U-Boot] [PATCH v8 4/9] x86: slimbootloader: Add memory configuration

Slim Bootloader provides memory map info thru its HOB list pointer. Configure memory size and relocation memory from the HOB data, and provide e820 entries as well. - Get memory size from the memory map info HOB - Set available top memory lower than 4GB for U-Boot relocation - Provide e820 entries from the memory map info HOB
Signed-off-by: Aiden Park aiden.park@intel.com Reviewed-by: Bin Meng bmeng.cn@gmail.com Reviewed-by: Andy Shevchenko andy.shevchenko@gmail.com --- Changes in v8: * Change for_each condition check to avoid ERROR from checkpatch
Changes in v7: * Use for_each macro instead of while_each * Move for_each macro before its first use
Changes in v6: * Change dram.c to sdram.c * Use EFI_GUID * Use a macro to do reversed search in memory map entries * Apply code-review comments
Changes in v3: * Use HOB function from the common HOB library * Add more description
arch/x86/cpu/slimbootloader/Makefile | 2 +- arch/x86/cpu/slimbootloader/sdram.c | 149 ++++++++++++++++++ .../asm/arch-slimbootloader/slimbootloader.h | 53 +++++++ 3 files changed, 203 insertions(+), 1 deletion(-) create mode 100644 arch/x86/cpu/slimbootloader/sdram.c create mode 100644 arch/x86/include/asm/arch-slimbootloader/slimbootloader.h
diff --git a/arch/x86/cpu/slimbootloader/Makefile b/arch/x86/cpu/slimbootloader/Makefile index 627a721e8c..9e6822a6ec 100644 --- a/arch/x86/cpu/slimbootloader/Makefile +++ b/arch/x86/cpu/slimbootloader/Makefile @@ -2,4 +2,4 @@ # # Copyright (C) 2019 Intel Corporation <www.intel.com>
-obj-y += car.o slimbootloader.o +obj-y += car.o slimbootloader.o sdram.o diff --git a/arch/x86/cpu/slimbootloader/sdram.c b/arch/x86/cpu/slimbootloader/sdram.c new file mode 100644 index 0000000000..2b2b220783 --- /dev/null +++ b/arch/x86/cpu/slimbootloader/sdram.c @@ -0,0 +1,149 @@ +// SPDX-License-Identifier: GPL-2.0+ +/* + * Copyright (C) 2019 Intel Corporation <www.intel.com> + */ + +#include <common.h> +#include <linux/sizes.h> +#include <asm/e820.h> +#include <asm/arch/slimbootloader.h> + +DECLARE_GLOBAL_DATA_PTR; + +/** + * This returns a data pointer of memory map info from the guid hob. + * + * @return: A data pointer of memory map info hob + */ +static struct sbl_memory_map_info *get_memory_map_info(void) +{ + struct sbl_memory_map_info *data; + const efi_guid_t guid = SBL_MEMORY_MAP_INFO_GUID; + + if (!gd->arch.hob_list) + return NULL; + + data = hob_get_guid_hob_data(gd->arch.hob_list, NULL, &guid); + if (!data) + panic("memory map info hob not found\n"); + if (!data->count) + panic("invalid number of memory map entries\n"); + + return data; +} + +#define for_each_memory_map_entry_reversed(iter, entries) \ + for (iter = entries->count - 1; iter >= 0; iter--) \ + if (entries->entry[iter].type == E820_RAM) + +/** + * This is to give usable memory region information for u-boot relocation. + * so search usable memory region lower than 4GB. + * The memory map entries from Slim Bootloader hob are already sorted. + * + * @total_size: The memory size that u-boot occupies + * @return : The top available memory address lower than 4GB + */ +ulong board_get_usable_ram_top(ulong total_size) +{ + struct sbl_memory_map_info *data; + int i; + u64 addr_start; + u64 addr_end; + ulong ram_top; + + data = get_memory_map_info(); + + /** + * sorted memory map entries from Slim Bootloader based on physical + * start memory address, from low to high. So do reversed search to + * get highest usable, suitable size, 4KB aligned available memory + * under 4GB. + */ + ram_top = 0; + for_each_memory_map_entry_reversed(i, data) { + addr_start = data->entry[i].addr; + addr_end = addr_start + data->entry[i].size; + + if (addr_start > SZ_4G) + continue; + + if (addr_end > SZ_4G) + addr_end = SZ_4G; + + if (addr_end < total_size) + continue; + + /* to relocate u-boot at 4K aligned memory */ + addr_end = rounddown(addr_end - total_size, SZ_4K); + if (addr_end >= addr_start) { + ram_top = (ulong)addr_end + total_size; + break; + } + } + + if (!ram_top) + panic("failed to find available memory for relocation!"); + + return ram_top; +} + +/** + * The memory initialization has already been done in previous Slim Bootloader + * stage thru FSP-M. Instead, this sets the ram_size from the memory map info + * hob. + */ +int dram_init(void) +{ + struct sbl_memory_map_info *data; + int i; + u64 ram_size; + + data = get_memory_map_info(); + + /** + * sorted memory map entries from Slim Bootloader based on physical + * start memory address, from low to high. So do reversed search to + * simply get highest usable memory address as RAM size + */ + ram_size = 0; + for_each_memory_map_entry_reversed(i, data) { + /* simply use the highest usable memory address as RAM size */ + ram_size = data->entry[i].addr + data->entry[i].size; + break; + } + + if (!ram_size) + panic("failed to detect memory size"); + + gd->ram_size = ram_size; + return 0; +} + +int dram_init_banksize(void) +{ + if (!CONFIG_NR_DRAM_BANKS) + return 0; + + /* simply use a single bank to have whole size for now */ + gd->bd->bi_dram[0].start = 0; + gd->bd->bi_dram[0].size = gd->ram_size; + return 0; +} + +unsigned int install_e820_map(unsigned int max_entries, + struct e820_entry *entries) +{ + struct sbl_memory_map_info *data; + unsigned int i; + + data = get_memory_map_info(); + + for (i = 0; i < data->count; i++) { + entries[i].addr = data->entry[i].addr; + entries[i].size = data->entry[i].size; + entries[i].type = data->entry[i].type; + } + + return i; +} diff --git a/arch/x86/include/asm/arch-slimbootloader/slimbootloader.h b/arch/x86/include/asm/arch-slimbootloader/slimbootloader.h new file mode 100644 index 0000000000..4ee8a0627e --- /dev/null +++ b/arch/x86/include/asm/arch-slimbootloader/slimbootloader.h @@ -0,0 +1,53 @@ +/* SPDX-License-Identifier: GPL-2.0+ */ +/* + * Copyright (C) 2019 Intel Corporation <www.intel.com> + */ + +#ifndef __SLIMBOOTLOADER_ARCH_H__ +#define __SLIMBOOTLOADER_ARCH_H__ + +#include <common.h> +#include <asm/hob.h> + +/** + * A GUID to get MemoryMap info hob which is provided by Slim Bootloader + */ +#define SBL_MEMORY_MAP_INFO_GUID \ + EFI_GUID(0xa1ff7424, 0x7a1a, 0x478e, \ + 0xa9, 0xe4, 0x92, 0xf3, 0x57, 0xd1, 0x28, 0x32) + +/** + * A single entry of memory map information + * + * @addr: start address of a memory map entry + * @size: size of a memory map entry + * @type: usable:1, reserved:2, acpi:3, nvs:4, unusable:5 + * @flag: only used in Slim Bootloader + * @rsvd: padding for alignment + */ +struct sbl_memory_map_entry { + u64 addr; + u64 size; + u8 type; + u8 flag; + u8 rsvd[6]; +}; + +/** + * This includes all memory map entries which is sorted based on physical start + * address, from low to high, and carved out reserved, acpi nvs, acpi reclaim + * and usable memory. + * + * @rev : revision of memory_map_info structure. currently 1. + * @rsvd : padding for alignment + * @count: the number of memory map entries + * @entry: array of all memory map entries + */ +struct sbl_memory_map_info { + u8 rev; + u8 rsvd[3]; + u32 count; + struct sbl_memory_map_entry entry[0]; +}; + +#endif /* __SLIMBOOTLOADER_ARCH_H__ */

On Fri, Aug 2, 2019 at 10:03 AM Park, Aiden aiden.park@intel.com wrote:
Slim Bootloader provides memory map info thru its HOB list pointer. Configure memory size and relocation memory from the HOB data, and provide e820 entries as well.
- Get memory size from the memory map info HOB
- Set available top memory lower than 4GB for U-Boot relocation
- Provide e820 entries from the memory map info HOB
+#define for_each_memory_map_entry_reversed(iter, entries) \
for (iter = entries->count - 1; iter >= 0; iter--) \
if (entries->entry[iter].type == E820_RAM)
It seems you missed my answer to Bin. This is simple incorrect. Checkpatch sometimes is wrong.

On Fri, Aug 2, 2019 at 10:49 AM Andy Shevchenko andy.shevchenko@gmail.com wrote:
On Fri, Aug 2, 2019 at 10:03 AM Park, Aiden aiden.park@intel.com wrote:
Slim Bootloader provides memory map info thru its HOB list pointer. Configure memory size and relocation memory from the HOB data, and provide e820 entries as well.
- Get memory size from the memory map info HOB
- Set available top memory lower than 4GB for U-Boot relocation
- Provide e820 entries from the memory map info HOB
+#define for_each_memory_map_entry_reversed(iter, entries) \
for (iter = entries->count - 1; iter >= 0; iter--) \
if (entries->entry[iter].type == E820_RAM)
It seems you missed my answer to Bin. This is simple incorrect. Checkpatch sometimes is wrong.
Let me elaborate why.
The idea of having
if (foo) {} else
pattern is to avoid weirndess like
for_each_...() { ... } else { ...WTF!.. }

Hi Andy,
-----Original Message----- From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com] Sent: Friday, August 2, 2019 4:51 PM To: Park, Aiden aiden.park@intel.com Cc: Bin Meng bmeng.cn@gmail.com; U-Boot Mailing List <u- boot@lists.denx.de>; Simon Glass sjg@chromium.org Subject: Re: [PATCH v8 4/9] x86: slimbootloader: Add memory configuration
On Fri, Aug 2, 2019 at 10:49 AM Andy Shevchenko andy.shevchenko@gmail.com wrote:
On Fri, Aug 2, 2019 at 10:03 AM Park, Aiden aiden.park@intel.com
wrote:
Slim Bootloader provides memory map info thru its HOB list pointer. Configure memory size and relocation memory from the HOB data, and provide e820 entries as well.
- Get memory size from the memory map info HOB
- Set available top memory lower than 4GB for U-Boot relocation
- Provide e820 entries from the memory map info HOB
+#define for_each_memory_map_entry_reversed(iter, entries) \
for (iter = entries->count - 1; iter >= 0; iter--) \
if (entries->entry[iter].type == E820_RAM)
It seems you missed my answer to Bin. This is simple incorrect. Checkpatch sometimes is wrong.
Let me elaborate why.
The idea of having
if (foo) {} else
pattern is to avoid weirndess like
for_each_...() { ... } else { ...WTF!.. }
Sorry for missing your comment. I understand '{} else' is better way. It looks checkpatch issue is bigger scope than this one. What about this in this series?
#define for_each_if(condition) if (!(condition)) {} else #define for_each_memory_map_entry_reversed(iter, entries) \ for (iter = entries->count - 1; iter >= 0; iter--) \ for_each_if(entries->entry[iter].type == E820_RAM)
tricky, but checkpatch does not report ERROR.
-- With Best Regards, Andy Shevchenko
Best Regards, Aiden

On Fri, Aug 2, 2019 at 1:19 PM Park, Aiden aiden.park@intel.com wrote:
-----Original Message----- From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com] Sent: Friday, August 2, 2019 4:51 PM To: Park, Aiden aiden.park@intel.com Cc: Bin Meng bmeng.cn@gmail.com; U-Boot Mailing List <u- boot@lists.denx.de>; Simon Glass sjg@chromium.org Subject: Re: [PATCH v8 4/9] x86: slimbootloader: Add memory configuration On Fri, Aug 2, 2019 at 10:49 AM Andy Shevchenko andy.shevchenko@gmail.com wrote:
On Fri, Aug 2, 2019 at 10:03 AM Park, Aiden aiden.park@intel.com
wrote:
+#define for_each_memory_map_entry_reversed(iter, entries) \
for (iter = entries->count - 1; iter >= 0; iter--) \
if (entries->entry[iter].type == E820_RAM)
It seems you missed my answer to Bin. This is simple incorrect. Checkpatch sometimes is wrong.
Let me elaborate why.
The idea of having
if (foo) {} else
pattern is to avoid weirndess like
for_each_...() { ... } else { ...WTF!.. }
Sorry for missing your comment. I understand '{} else' is better way. It looks checkpatch issue is bigger scope than this one. What about this in this series?
#define for_each_if(condition) if (!(condition)) {} else #define for_each_memory_map_entry_reversed(iter, entries) \ for (iter = entries->count - 1; iter >= 0; iter--) \ for_each_if(entries->entry[iter].type == E820_RAM)
tricky, but checkpatch does not report ERROR.
Fine for me. If Bin is okay with it, it will be good then.

On Fri, Aug 2, 2019 at 6:29 PM Andy Shevchenko andy.shevchenko@gmail.com wrote:
On Fri, Aug 2, 2019 at 1:19 PM Park, Aiden aiden.park@intel.com wrote:
-----Original Message----- From: Andy Shevchenko [mailto:andy.shevchenko@gmail.com] Sent: Friday, August 2, 2019 4:51 PM To: Park, Aiden aiden.park@intel.com Cc: Bin Meng bmeng.cn@gmail.com; U-Boot Mailing List <u- boot@lists.denx.de>; Simon Glass sjg@chromium.org Subject: Re: [PATCH v8 4/9] x86: slimbootloader: Add memory configuration On Fri, Aug 2, 2019 at 10:49 AM Andy Shevchenko andy.shevchenko@gmail.com wrote:
On Fri, Aug 2, 2019 at 10:03 AM Park, Aiden aiden.park@intel.com
wrote:
+#define for_each_memory_map_entry_reversed(iter, entries) \
for (iter = entries->count - 1; iter >= 0; iter--) \
if (entries->entry[iter].type == E820_RAM)
It seems you missed my answer to Bin. This is simple incorrect. Checkpatch sometimes is wrong.
Let me elaborate why.
The idea of having
if (foo) {} else
pattern is to avoid weirndess like
for_each_...() { ... } else { ...WTF!.. }
Sorry for missing your comment. I understand '{} else' is better way. It looks checkpatch issue is bigger scope than this one. What about this in this series?
#define for_each_if(condition) if (!(condition)) {} else #define for_each_memory_map_entry_reversed(iter, entries) \ for (iter = entries->count - 1; iter >= 0; iter--) \ for_each_if(entries->entry[iter].type == E820_RAM)
tricky, but checkpatch does not report ERROR.
Fine for me. If Bin is okay with it, it will be good then.
I am good too. Thanks for working on fixing checkpatch warnings!
Regards, Bin
participants (3)
-
Andy Shevchenko
-
Bin Meng
-
Park, Aiden