Attention is currently required from: Furquan Shaikh, Aaron Durbin. Hello Furquan Shaikh, Aaron Durbin,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/49336
to review the following change.
Change subject: cbfs: Remove prog_locate() for payloads (SELF and FIT) ......................................................................
cbfs: Remove prog_locate() for payloads (SELF and FIT)
This patch removes the prog_locate() call for all instances of loading payload formats (SELF and FIT), as the previous patch did for stages.
Signed-off-by: Julius Werner jwerner@chromium.org Change-Id: I582b37f36fe6f9f26975490a823e85b130ba49a2 --- M src/arch/arm64/bl31.c M src/arch/riscv/boot.c M src/include/bootmem.h D src/include/fit_payload.h M src/include/program_loading.h M src/lib/fit_payload.c M src/lib/prog_loaders.c M src/lib/selfboot.c M src/soc/qualcomm/sc7180/aop_load_reset.c 9 files changed, 44 insertions(+), 66 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/36/49336/1
diff --git a/src/arch/arm64/bl31.c b/src/arch/arm64/bl31.c index a387027..06676bc 100644 --- a/src/arch/arm64/bl31.c +++ b/src/arch/arm64/bl31.c @@ -76,9 +76,6 @@ struct prog bl31 = PROG_INIT(PROG_BL31, CONFIG_CBFS_PREFIX"/bl31"); void (*bl31_entry)(bl_params_t *params, void *plat_params) = NULL;
- if (prog_locate(&bl31)) - die("BL31 not found"); - if (!selfload_check(&bl31, BM_MEM_BL31)) die("BL31 load failed"); bl31_entry = prog_entry(&bl31); diff --git a/src/arch/riscv/boot.c b/src/arch/riscv/boot.c index 7cb0a36..119039d 100644 --- a/src/arch/riscv/boot.c +++ b/src/arch/riscv/boot.c @@ -53,9 +53,6 @@ if (ENV_RAMSTAGE && CONFIG(RISCV_OPENSBI)) { struct prog sbi = PROG_INIT(PROG_OPENSBI, CONFIG_CBFS_PREFIX"/opensbi");
- if (prog_locate(&sbi)) - die("OpenSBI not found"); - if (!selfload_check(&sbi, BM_MEM_OPENSBI)) die("OpenSBI load failed");
diff --git a/src/include/bootmem.h b/src/include/bootmem.h index 0625bbb..6ccdd88 100644 --- a/src/include/bootmem.h +++ b/src/include/bootmem.h @@ -17,6 +17,8 @@ * Start at 0x10000 to make sure that the caller doesn't provide LB_MEM tags. */ enum bootmem_type { + BM_MEM_INVALID = 0, /* Invalid type (used in optional arguments). */ + BM_MEM_FIRST = 0x10000, /* First entry in this list */ BM_MEM_RAM, /* Memory anyone can use */ BM_MEM_RESERVED, /* Don't use this memory region */ diff --git a/src/include/fit_payload.h b/src/include/fit_payload.h deleted file mode 100644 index 4ebe3ce..0000000 --- a/src/include/fit_payload.h +++ /dev/null @@ -1,10 +0,0 @@ -/* SPDX-License-Identifier: GPL-2.0-or-later */ - -#ifndef __FIT_PAYLOAD_H_ -#define __FIT_PAYLOAD_H_ - -#include <program_loading.h> - -void fit_payload(struct prog *payload); - -#endif /* __FIT_PAYLOAD_H_ */ diff --git a/src/include/program_loading.h b/src/include/program_loading.h index 7ceecc4..c9ec9c2 100644 --- a/src/include/program_loading.h +++ b/src/include/program_loading.h @@ -45,7 +45,7 @@ * payload. For architectures that use a bounce buffer * then it would represent the bounce buffer. */ enum prog_type type; - uint32_t cbfs_type; + enum cbfs_type cbfs_type; const char *name; struct region_device rdev; /* Entry to program with optional argument. It's up to the architecture @@ -70,7 +70,7 @@ return prog->type; }
-static inline uint32_t prog_cbfs_type(const struct prog *prog) +static inline enum cbfs_type prog_cbfs_type(const struct prog *prog) { return prog->cbfs_type; } @@ -197,5 +197,11 @@ */ bool selfload_check(struct prog *payload, enum bootmem_type dest_type); bool selfload(struct prog *payload); +/* Like selfload_check() but with the payload data already mapped to memory. */ +bool selfload_mapped(struct prog *payload, void *mapping, + enum bootmem_type dest_type); + +/* Load a FIT payload. The payload data must already be mapped to memory. */ +void fit_payload(struct prog *payload, void *data);
#endif /* PROGRAM_LOADING_H */ diff --git a/src/lib/fit_payload.c b/src/lib/fit_payload.c index b3a03a1..dcc4922 100644 --- a/src/lib/fit_payload.c +++ b/src/lib/fit_payload.c @@ -13,7 +13,6 @@ #include <timestamp.h> #include <string.h> #include <lib.h> -#include <fit_payload.h> #include <boardid.h>
/* Pack the device_tree and place it at given position. */ @@ -168,31 +167,24 @@ /* * Parse the uImage FIT, choose a configuration and extract images. */ -void fit_payload(struct prog *payload) +void fit_payload(struct prog *payload, void *data) { struct device_tree *dt = NULL; struct region kernel = {0}, fdt = {0}, initrd = {0}; void *data;
- data = rdev_mmap_full(prog_rdev(payload)); - - if (data == NULL) - return; - printk(BIOS_INFO, "FIT: Examine payload %s\n", payload->name);
struct fit_config_node *config = fit_load(data);
if (!config) { printk(BIOS_ERR, "ERROR: Could not load FIT\n"); - rdev_munmap(prog_rdev(payload), data); return; }
dt = unpack_fdt(config->fdt); if (!dt) { printk(BIOS_ERR, "ERROR: Failed to unflatten the FDT.\n"); - rdev_munmap(prog_rdev(payload), data); return; }
@@ -225,7 +217,6 @@ if (!fit_payload_arch(payload, config, &kernel, &fdt, &initrd)) { printk(BIOS_ERR, "ERROR: Failed to find free memory region\n"); bootmem_dump_ranges(); - rdev_munmap(prog_rdev(payload), data); return; }
@@ -240,7 +231,6 @@ extract(&initrd, config->ramdisk)) { printk(BIOS_ERR, "ERROR: Failed to extract initrd\n"); prog_set_entry(payload, NULL, NULL); - rdev_munmap(prog_rdev(payload), data); return; }
@@ -249,11 +239,8 @@ if (extract(&kernel, config->kernel)) { printk(BIOS_ERR, "ERROR: Failed to extract kernel\n"); prog_set_entry(payload, NULL, NULL); - rdev_munmap(prog_rdev(payload), data); return; }
timestamp_add_now(TS_START_KERNEL); - - rdev_munmap(prog_rdev(payload), data); } diff --git a/src/lib/prog_loaders.c b/src/lib/prog_loaders.c index 964b7f5..5b584fe 100644 --- a/src/lib/prog_loaders.c +++ b/src/lib/prog_loaders.c @@ -15,7 +15,6 @@ #include <stage_cache.h> #include <symbols.h> #include <timestamp.h> -#include <fit_payload.h> #include <security/vboot/vboot_common.h>
/* Only can represent up to 1 byte less than size_t. */ @@ -158,24 +157,30 @@
timestamp_add_now(TS_LOAD_PAYLOAD);
- if (prog_locate(payload)) + if (prog_locate_hook(payload)) + goto out; + + payload->cbfs_type = CBFS_TYPE_QUERY; + void *mapping = cbfs_type_map(prog_name(payload), NULL, &payload->cbfs_type); + if (!mapping) goto out;
switch (prog_cbfs_type(payload)) { case CBFS_TYPE_SELF: /* Simple ELF */ - selfload_check(payload, BM_MEM_RAM); + selfload_mapped(payload, mapping, BM_MEM_RAM); break; case CBFS_TYPE_FIT: /* Flattened image tree */ if (CONFIG(PAYLOAD_FIT_SUPPORT)) { - fit_payload(payload); + fit_payload(payload, mapping); break; } /* else fall-through */ default: die_with_post_code(POST_INVALID_ROM, - "Unsupported payload type.\n"); + "Unsupported payload type %d.\n", payload->cbfs_type); break; }
+ cbfs_unmap(mapping); out: if (prog_entry(payload) == NULL) die_with_post_code(POST_INVALID_ROM, "Payload not loaded.\n"); diff --git a/src/lib/selfboot.c b/src/lib/selfboot.c index c5ad525..a313c7f 100644 --- a/src/lib/selfboot.c +++ b/src/lib/selfboot.c @@ -127,12 +127,14 @@ }
static int check_payload_segments(struct cbfs_payload_segment *cbfssegs, - void *args) + enum bootmem_type dest_type) { uint8_t *dest; size_t memsz; struct cbfs_payload_segment *seg, segment; - enum bootmem_type dest_type = *(enum bootmem_type *)args; + + if (dest_type == BM_MEM_INVALID) + return 0;
for (seg = cbfssegs;; ++seg) { printk(BIOS_DEBUG, "Checking segment from ROM address %p\n", seg); @@ -225,50 +227,45 @@ return 0; }
-static void *selfprepare(struct prog *payload) -{ - void *data; - data = rdev_mmap_full(prog_rdev(payload)); - return data; -} - -static bool _selfload(struct prog *payload, checker_t f, void *args) +bool selfload_mapped(struct prog *payload, void *mapping, + enum bootmem_type dest_type) { uintptr_t entry = 0; struct cbfs_payload_segment *cbfssegs; - void *data;
- data = selfprepare(payload); - if (data == NULL) + cbfssegs = &((struct cbfs_payload *)mapping)->segments; + + if (check_payload_segments(cbfssegs, dest_type)) return false;
- cbfssegs = &((struct cbfs_payload *)data)->segments; - - if (f && f(cbfssegs, args)) - goto out; - if (load_payload_segments(cbfssegs, &entry)) - goto out; + return false;
printk(BIOS_SPEW, "Loaded segments\n");
- rdev_munmap(prog_rdev(payload), data); - /* Pass cbtables to payload if architecture desires it. */ prog_set_entry(payload, (void *)entry, cbmem_find(CBMEM_ID_CBTABLE));
return true; -out: - rdev_munmap(prog_rdev(payload), data); - return false; }
bool selfload_check(struct prog *payload, enum bootmem_type dest_type) { - return _selfload(payload, check_payload_segments, &dest_type); + if (prog_locate_hook(payload)) + return false; + + payload->cbfs_type = CBFS_TYPE_SELF; + void *mapping = cbfs_type_map(prog_name(payload), NULL, &payload->cbfs_type); + if (!mapping) + return false; + + bool ret = selfload_mapped(payload, mapping, dest_type); + + cbfs_unmap(mapping); + return ret; }
bool selfload(struct prog *payload) { - return _selfload(payload, NULL, 0); + return selfload_check(payload, BM_MEM_INVALID); } diff --git a/src/soc/qualcomm/sc7180/aop_load_reset.c b/src/soc/qualcomm/sc7180/aop_load_reset.c index eb90f86..3028646 100644 --- a/src/soc/qualcomm/sc7180/aop_load_reset.c +++ b/src/soc/qualcomm/sc7180/aop_load_reset.c @@ -13,9 +13,6 @@ struct prog aop_fw_prog = PROG_INIT(PROG_PAYLOAD, CONFIG_CBFS_PREFIX "/aop");
- if (prog_locate(&aop_fw_prog)) - die("SOC image: AOP_FW not found"); - aop_fw_entry = selfload(&aop_fw_prog); if (!aop_fw_entry) die("SOC image: AOP load failed");