Attention is currently required from: Christian Walter, Johnny Lin, Jonathan Zhang, Jérémy Compostella, Lean Sheng Tan, Patrick Rudolph, Shuo Liu, Tim Chu.
Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/84039?usp=email )
Change subject: cbmem.h: Change signature of cbmem_get_region ......................................................................
cbmem.h: Change signature of cbmem_get_region
The underlying IMD function already returns an integer which indicates success or failure.
This removes the need to have initialized variables that need to be checked for NULL later.
Dying is appropriate if cbmem is not found as it is essential to the bootflow.
Change-Id: Ib3e09a75380faf9f533601368993261f042422ef Signed-off-by: Arthur Heymans arthur@aheymans.xyz --- M src/arch/x86/postcar_loader.c M src/include/cbmem.h M src/lib/fit_payload.c M src/lib/imd_cbmem.c M src/security/memory/memory_clear.c M src/soc/intel/xeon_sp/memmap.c 6 files changed, 20 insertions(+), 18 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/39/84039/1
diff --git a/src/arch/x86/postcar_loader.c b/src/arch/x86/postcar_loader.c index 2d7bb48..aac4af8 100644 --- a/src/arch/x86/postcar_loader.c +++ b/src/arch/x86/postcar_loader.c @@ -147,7 +147,8 @@ uintptr_t stage_cache_base; size_t stage_cache_size;
- cbmem_get_region((void **)&cbmem_base, &cbmem_size); + if (cbmem_get_region((void **)&cbmem_base, &cbmem_size)) + die("Could not find cbmem region"); prog_segment_loaded(cbmem_base, cbmem_size, SEG_FINAL); if (CONFIG(TSEG_STAGE_CACHE) && !romstage_handoff_is_resume()) { stage_cache_external_region((void **)&stage_cache_base, &stage_cache_size); diff --git a/src/include/cbmem.h b/src/include/cbmem.h index de305ae..fd258cd 100644 --- a/src/include/cbmem.h +++ b/src/include/cbmem.h @@ -102,7 +102,7 @@ /* Add the cbmem memory used to the memory map at boot. */ void cbmem_add_bootmem(void); /* Return the cbmem memory used */ -void cbmem_get_region(void **baseptr, size_t *size); +int cbmem_get_region(void **baseptr, size_t *size); void cbmem_list(void); void cbmem_add_records_to_cbtable(struct lb_header *header);
diff --git a/src/lib/fit_payload.c b/src/lib/fit_payload.c index 21bc4e8..9ca5f09 100644 --- a/src/lib/fit_payload.c +++ b/src/lib/fit_payload.c @@ -110,8 +110,8 @@ { u32 addr_cells = 1, size_cells = 1; u64 reg_addrs[2], reg_sizes[2]; - void *baseptr = NULL; - size_t size = 0; + void *baseptr; + size_t size;
static const char *firmware_path[] = {"firmware", NULL}; struct device_tree_node *firmware_node = dt_find_node(tree->root, @@ -140,11 +140,8 @@
/* Second is the CBMEM area (which usually includes the coreboot table). */ - cbmem_get_region(&baseptr, &size); - if (!baseptr || size == 0) { - printk(BIOS_WARNING, "FIT: CBMEM pointer/size not found!\n"); - return; - } + if (cbmem_get_region(&baseptr, &size)) + die("Could not find cbmem region");
reg_addrs[1] = (uintptr_t)baseptr; reg_sizes[1] = size; diff --git a/src/lib/imd_cbmem.c b/src/lib/imd_cbmem.c index a88cf98..6e333f6 100644 --- a/src/lib/imd_cbmem.c +++ b/src/lib/imd_cbmem.c @@ -193,16 +193,18 @@
void cbmem_add_bootmem(void) { - void *baseptr = NULL; - size_t size = 0; + void *baseptr; + size_t size;
- cbmem_get_region(&baseptr, &size); + if (cbmem_get_region(&baseptr, &size)) { + die("Could not find cbmem region"); + } bootmem_add_range((uintptr_t)baseptr, size, BM_MEM_TABLE); }
-void cbmem_get_region(void **baseptr, size_t *size) +int cbmem_get_region(void **baseptr, size_t *size) { - imd_region_used(&imd, baseptr, size); + return imd_region_used(&imd, baseptr, size); }
#if ENV_PAYLOAD_LOADER || (CONFIG(EARLY_CBMEM_LIST) && ENV_HAS_CBMEM) diff --git a/src/security/memory/memory_clear.c b/src/security/memory/memory_clear.c index 9af3205..9a8a23a 100644 --- a/src/security/memory/memory_clear.c +++ b/src/security/memory/memory_clear.c @@ -74,12 +74,13 @@ BM_MEM_RAM);
/* Add reserved entries */ - void *baseptr = NULL; - size_t size = 0; + void *baseptr; + size_t size;
/* Only skip CBMEM, stage program, stack and heap are included there. */
- cbmem_get_region(&baseptr, &size); + if (cbmem_get_region(&baseptr, &size)) + die("Could not find cbmem region"); memranges_insert(&mem, (uintptr_t)baseptr, size, BM_MEM_TABLE);
if (ENV_X86) { diff --git a/src/soc/intel/xeon_sp/memmap.c b/src/soc/intel/xeon_sp/memmap.c index bf4c13e..b70c1cc 100644 --- a/src/soc/intel/xeon_sp/memmap.c +++ b/src/soc/intel/xeon_sp/memmap.c @@ -35,7 +35,8 @@ size_t cbmem_size;
/* Try account for the CBMEM region currently used and for future use */ - cbmem_get_region((void **)&cbmem_base, &cbmem_size); + if (cbmem_get_region((void **)&cbmem_base, &cbmem_size)) + die("Could not find cbmem region"); printk(BIOS_DEBUG, "top_of_ram = 0x%lx\n", top_of_ram); printk(BIOS_DEBUG, "cbmem base_ptr: 0x%lx, size: 0x%zx\n", cbmem_base, cbmem_size); /* Assume 4MiB will be enough for future cbmem objects (FSP-S, ramstage, ...) */