Julius Werner submitted this change.

View Change

Approvals: build bot (Jenkins): Verified Aaron Durbin: Looks good to me, approved Furquan Shaikh: Looks good to me, approved
fsp2_0: Replace fspld->get_destination() callback with CBFS allocator

The Intel FSP 2.0 driver contains a custom construct that basically
serves the same purpose as the new CBFS allocator concept: a callback
function to customize placement of a loaded CBFS file whose size is
initially unknown. This patch removes the existing implementation and
replaces it with a CBFS allocator.

Signed-off-by: Julius Werner <jwerner@chromium.org>
Change-Id: I0b7b446a0d2af87ec337fb80ad54f2d404e69668
Reviewed-on: https://review.coreboot.org/c/coreboot/+/52082
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>
Reviewed-by: Aaron Durbin <adurbin@chromium.org>
Reviewed-by: Furquan Shaikh <furquan@google.com>
---
M src/drivers/intel/fsp2_0/include/fsp/util.h
M src/drivers/intel/fsp2_0/memory_init.c
M src/drivers/intel/fsp2_0/silicon_init.c
M src/drivers/intel/fsp2_0/util.c
4 files changed, 27 insertions(+), 89 deletions(-)

diff --git a/src/drivers/intel/fsp2_0/include/fsp/util.h b/src/drivers/intel/fsp2_0/include/fsp/util.h
index 253b74a..6583b64 100644
--- a/src/drivers/intel/fsp2_0/include/fsp/util.h
+++ b/src/drivers/intel/fsp2_0/include/fsp/util.h
@@ -4,6 +4,7 @@
#define _FSP2_0_UTIL_H_

#include <boot/coreboot_tables.h>
+#include <cbfs.h>
#include <commonlib/region.h>
#include <arch/cpu.h>
#include <fsp/api.h>
@@ -111,11 +112,8 @@
/* fsp_prog object will have region_device initialized to final
* load location in memory. */
struct prog fsp_prog;
- /* Fill in destination location given final load size. Return 0 on
- * success, < 0 on error. */
- int (*get_destination)(const struct fsp_load_descriptor *fspld,
- void **dest, size_t final_load_size,
- const struct region_device *source);
+ /* CBFS allocator to place loaded FSP. NULL to map flash directly. */
+ cbfs_allocator_t alloc;
/* Optional argument to be utilized by get_destination() callback. */
void *arg;
};
diff --git a/src/drivers/intel/fsp2_0/memory_init.c b/src/drivers/intel/fsp2_0/memory_init.c
index 0ea5c08..bbc26bc 100644
--- a/src/drivers/intel/fsp2_0/memory_init.c
+++ b/src/drivers/intel/fsp2_0/memory_init.c
@@ -339,40 +339,20 @@
fsp_debug_after_memory_init(status);
}

-static int fspm_get_dest(const struct fsp_load_descriptor *fspld, void **dest,
- size_t size, const struct region_device *source)
+static void *fspm_allocator(void *arg, size_t size, const union cbfs_mdata *unused)
{
+ const struct fsp_load_descriptor *fspld = arg;
struct fspm_context *context = fspld->arg;
- struct fsp_header *hdr = &context->header;
struct memranges *memmap = &context->memmap;
- uintptr_t fspm_begin;
- uintptr_t fspm_end;
-
- if (CONFIG(FSP_M_XIP)) {
- if (fsp_validate_component(hdr, source) != CB_SUCCESS)
- return -1;
-
- *dest = rdev_mmap_full(source);
- if ((uintptr_t)*dest != hdr->image_base) {
- printk(BIOS_CRIT, "FSPM XIP base does not match: %p vs %p\n",
- (void *)(uintptr_t)hdr->image_base, *dest);
- return -1;
- }
- /* Since the component is XIP it's already in the address space.
- Thus, there's no need to rdev_munmap(). */
- return 0;
- }

/* Non XIP FSP-M uses FSP-M address */
- fspm_begin = (uintptr_t)CONFIG_FSP_M_ADDR;
- fspm_end = fspm_begin + size;
+ uintptr_t fspm_begin = (uintptr_t)CONFIG_FSP_M_ADDR;
+ uintptr_t fspm_end = fspm_begin + size;

if (check_region_overlap(memmap, "FSPM", fspm_begin, fspm_end) != CB_SUCCESS)
- return -1;
+ return NULL;

- *dest = (void *)fspm_begin;
-
- return 0;
+ return (void *)fspm_begin;
}

void fsp_memory_init(bool s3wake)
@@ -381,12 +361,15 @@
struct fspm_context context;
struct fsp_load_descriptor fspld = {
.fsp_prog = PROG_INIT(PROG_REFCODE, CONFIG_FSP_M_CBFS),
- .get_destination = fspm_get_dest,
.arg = &context,
};
struct fsp_header *hdr = &context.header;
struct memranges *memmap = &context.memmap;

+ /* For FSP-M XIP we leave alloc NULL to get a direct mapping to flash. */
+ if (!CONFIG(FSP_M_XIP))
+ fspld.alloc = fspm_allocator;
+
elog_boot_notify(s3wake);

/* Build up memory map of romstage address space including CAR. */
@@ -399,6 +382,10 @@
if (fsp_load_component(&fspld, hdr) != CB_SUCCESS)
die("FSPM not available or failed to load!\n");

+ if (CONFIG(FSP_M_XIP) && (uintptr_t)prog_start(&fspld.fsp_prog) != hdr->image_base)
+ die("FSPM XIP base does not match: %p vs %p\n",
+ (void *)(uintptr_t)hdr->image_base, prog_start(&fspld.fsp_prog));
+
timestamp_add_now(TS_BEFORE_INITRAM);

do_fsp_memory_init(&context, s3wake);
diff --git a/src/drivers/intel/fsp2_0/silicon_init.c b/src/drivers/intel/fsp2_0/silicon_init.c
index c9898f5..6326d53 100644
--- a/src/drivers/intel/fsp2_0/silicon_init.c
+++ b/src/drivers/intel/fsp2_0/silicon_init.c
@@ -184,22 +184,16 @@
post_code(POST_FSP_MULTI_PHASE_SI_INIT_EXIT);
}

-static int fsps_get_dest(const struct fsp_load_descriptor *fspld, void **dest,
- size_t size, const struct region_device *source)
+static void *fsps_allocator(void *arg_unused, size_t size, const union cbfs_mdata *mdata_unused)
{
- *dest = cbmem_add(CBMEM_ID_REFCODE, size);
-
- if (*dest == NULL)
- return -1;
-
- return 0;
+ return cbmem_add(CBMEM_ID_REFCODE, size);
}

void fsps_load(void)
{
struct fsp_load_descriptor fspld = {
.fsp_prog = PROG_INIT(PROG_REFCODE, CONFIG_FSP_S_CBFS),
- .get_destination = fsps_get_dest,
+ .alloc = fsps_allocator,
};
struct prog *fsps = &fspld.fsp_prog;
static int load_done;
diff --git a/src/drivers/intel/fsp2_0/util.c b/src/drivers/intel/fsp2_0/util.c
index 9831e64..33bfc11 100644
--- a/src/drivers/intel/fsp2_0/util.c
+++ b/src/drivers/intel/fsp2_0/util.c
@@ -139,65 +139,24 @@
return false;
}

-static void *fsp_get_dest_and_load(struct fsp_load_descriptor *fspld, size_t size,
- const struct region_device *source_rdev,
- uint32_t compression_algo)
-{
- void *dest;
-
- if (fspld->get_destination(fspld, &dest, size, source_rdev) < 0) {
- printk(BIOS_ERR, "FSP Destination not obtained.\n");
- return NULL;
- }
-
- /* Don't load when executing in place. */
- if (fspm_xip())
- return dest;
-
- if (cbfs_load_and_decompress(source_rdev, 0, region_device_sz(source_rdev),
- dest, size, compression_algo) != size) {
- printk(BIOS_ERR, "Failed to load FSP component.\n");
- return NULL;
- }
-
- /* Don't allow FSP-M relocation. */
- if (fspm_env())
- return dest;
-
- if (fsp_component_relocate((uintptr_t)dest, dest, size) < 0) {
- printk(BIOS_ERR, "Unable to relocate FSP component!\n");
- return NULL;
- }
-
- return dest;
-}
-
/* Load the FSP component described by fsp_load_descriptor from cbfs. The FSP
* header object will be validated and filled in on successful load. */
enum cb_err fsp_load_component(struct fsp_load_descriptor *fspld, struct fsp_header *hdr)
{
- struct cbfsf file_desc;
- uint32_t compression_algo;
size_t output_size;
void *dest;
- struct region_device source_rdev, prog_rdev;
+ struct region_device prog_rdev;
struct prog *fsp_prog = &fspld->fsp_prog;

- if (fspld->get_destination == NULL)
+ dest = cbfs_alloc(prog_name(fsp_prog), fspld->alloc, fspld, &output_size);
+ if (!dest)
return CB_ERR;

- if (cbfs_boot_locate(&file_desc, prog_name(fsp_prog), &fsp_prog->cbfs_type) < 0)
+ /* Don't allow FSP-M relocation. */
+ if (!fspm_env() && fsp_component_relocate((uintptr_t)dest, dest, output_size) < 0) {
+ printk(BIOS_ERR, "Unable to relocate FSP component!\n");
return CB_ERR;
-
- if (cbfsf_decompression_info(&file_desc, &compression_algo, &output_size) < 0)
- return CB_ERR;
-
- cbfs_file_data(&source_rdev, &file_desc);
-
- dest = fsp_get_dest_and_load(fspld, output_size, &source_rdev, compression_algo);
-
- if (dest == NULL)
- return CB_ERR;
+ }

prog_set_area(fsp_prog, dest, output_size);


To view, visit change 52082. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: coreboot
Gerrit-Branch: master
Gerrit-Change-Id: I0b7b446a0d2af87ec337fb80ad54f2d404e69668
Gerrit-Change-Number: 52082
Gerrit-PatchSet: 2
Gerrit-Owner: Julius Werner <jwerner@chromium.org>
Gerrit-Reviewer: Aaron Durbin <adurbin@chromium.org>
Gerrit-Reviewer: Andrey Petrov <andrey.petrov@gmail.com>
Gerrit-Reviewer: Furquan Shaikh <furquan@google.com>
Gerrit-Reviewer: Julius Werner <jwerner@chromium.org>
Gerrit-Reviewer: Patrick Rudolph <siro@das-labor.org>
Gerrit-Reviewer: build bot (Jenkins) <no-reply@coreboot.org>
Gerrit-MessageType: merged