Hello Patrick Rudolph,
I'd like you to do a code review. Please visit
https://review.coreboot.org/c/coreboot/+/32870
to review the following change.
Change subject: fit: Refactor config node handling ......................................................................
fit: Refactor config node handling
This patch makes some minor refactoring to the way the FIT parser handles config nodes. A lot of this code was written in the dawn age of depthcharge when its device tree library wasn't as well-stocked yet, so some of it can be rewritten nicer with more high-level primitives. There's no point in storing both the string name and the actual FDT node of a FIT image node separately, since the latter also contains the former, so remove that. Also eliminate code for the case of not having an FDT (which makes no sense), and move some more FDT validity/compat checking into fit_update_compat() (mostly in anticipation of later changes).
This patch was adapted from depthcharge's http://crosreview.com/1553456 with a couple of modifications specific to coreboot's custom FIT loading code.
Change-Id: Ia79e0fd0e1159c4aca64c453b82a0379b133350d Signed-off-by: Julius Werner jwerner@chromium.org --- M src/arch/arm64/fit_payload.c M src/include/fit.h M src/lib/fit.c M src/lib/fit_payload.c 4 files changed, 92 insertions(+), 117 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/32870/1
diff --git a/src/arch/arm64/fit_payload.c b/src/arch/arm64/fit_payload.c index c4bbcee..fd1bae1 100644 --- a/src/arch/arm64/fit_payload.c +++ b/src/arch/arm64/fit_payload.c @@ -184,20 +184,14 @@ bool place_anywhere; void *arg = NULL;
- if (!config->fdt || !fdt) { - printk(BIOS_CRIT, "CRIT: Providing a valid FDT is mandatory to " - "boot an ARM64 kernel!\n"); - return false; - } - - if (!decompress_kernel_header(config->kernel_node)) { + if (!decompress_kernel_header(config->kernel)) { printk(BIOS_CRIT, "CRIT: Payload doesn't look like an ARM64" " kernel Image.\n"); return false; }
/* Update kernel size from image header, if possible */ - kernel->size = get_kernel_size(config->kernel_node); + kernel->size = get_kernel_size(config->kernel); printk(BIOS_DEBUG, "FIT: Using kernel size of 0x%zx bytes\n", kernel->size);
diff --git a/src/include/fit.h b/src/include/fit.h index 6e0667f..758ee70 100644 --- a/src/include/fit.h +++ b/src/include/fit.h @@ -27,7 +27,7 @@ struct fit_image_node { const char *name; - const void *data; + void *data; uint32_t size; int compression;
@@ -37,12 +37,9 @@ struct fit_config_node { const char *name; - const char *kernel; - struct fit_image_node *kernel_node; - const char *fdt; - struct fit_image_node *fdt_node; - const char *ramdisk; - struct fit_image_node *ramdisk_node; + struct fit_image_node *kernel; + struct fit_image_node *fdt; + struct fit_image_node *ramdisk; struct fdt_property compat; int compat_rank; int compat_pos; diff --git a/src/lib/fit.c b/src/lib/fit.c index c98ba2f..aa3fee7 100644 --- a/src/lib/fit.c +++ b/src/lib/fit.c @@ -85,6 +85,17 @@ fit_add_compat_string(compat_string); }
+static struct fit_image_node *find_image(const char *name) +{ + struct fit_image_node *image; + list_for_each(image, image_nodes, list_node) { + if (!strcmp(image->name, name)) + return image; + } + printk(BIOS_ERR, "ERROR: Cannot find image node %s!\n", name); + return NULL; +} + static void image_node(struct device_tree_node *node) { struct fit_image_node *image = xzalloc(sizeof(*image)); @@ -120,11 +131,11 @@ struct device_tree_property *prop; list_for_each(prop, node->properties, list_node) { if (!strcmp("kernel", prop->prop.name)) - config->kernel = prop->prop.data; + config->kernel = find_image(prop->prop.data); else if (!strcmp("fdt", prop->prop.name)) - config->fdt = prop->prop.data; + config->fdt = find_image(prop->prop.data); else if (!strcmp("ramdisk", prop->prop.name)) - config->ramdisk = prop->prop.data; + config->ramdisk = find_image(prop->prop.data); }
list_insert_after(&config->list_node, &config_nodes); @@ -132,40 +143,22 @@
static void fit_unpack(struct device_tree *tree, const char **default_config) { - assert(tree && tree->root); + struct device_tree_node *child; + struct device_tree_node *images = dt_find_node_by_path(tree, "/images", + NULL, NULL, 0); + if (images) + list_for_each(child, images->children, list_node) + image_node(child);
- struct device_tree_node *top; - list_for_each(top, tree->root->children, list_node) { - struct device_tree_node *child; - if (!strcmp("images", top->name)) { - - list_for_each(child, top->children, list_node) - image_node(child); - - } else if (!strcmp("configurations", top->name)) { - struct device_tree_property *prop; - list_for_each(prop, top->properties, list_node) { - if (!strcmp("default", prop->prop.name) && - default_config) - *default_config = prop->prop.data; - } - - list_for_each(child, top->children, list_node) - config_node(child); - } + struct device_tree_node *configs = dt_find_node_by_path(tree, + "/configurations", NULL, NULL, 0); + if (configs) { + *default_config = dt_find_string_prop(configs, "default"); + list_for_each(child, configs->children, list_node) + config_node(child); } }
-static struct fit_image_node *find_image(const char *name) -{ - struct fit_image_node *image; - list_for_each(image, image_nodes, list_node) { - if (!strcmp(image->name, name)) - return image; - } - return NULL; -} - static int fdt_find_compat(const void *blob, uint32_t start_offset, struct fdt_property *prop) { @@ -393,18 +386,27 @@
/* * Finds a compat string and updates the compat position and rank. - * @param fdt_blob Pointer to FDT * @param config The current config node to operate on + * @return 0 if compat updated, -1 if this FDT cannot be used. */ -static void fit_update_compat(const void *fdt_blob, - struct fit_config_node *config) +static int fit_update_compat(struct fit_config_node *config) { + if (config->fdt->compression != CBFS_COMPRESS_NONE) { + printk(BIOS_ERR, + "FDT compression not yet supported, skipping %s.\n", + config->name); + return -1; + } + + void *fdt_blob = config->fdt->data; struct compat_string_entry *compat_node; const struct fdt_header *fdt_header = (const struct fdt_header *)fdt_blob; uint32_t fdt_offset = be32_to_cpu(fdt_header->structure_offset); size_t i = 0;
+ config->compat_pos = -1; + config->compat_rank = -1; if (!fdt_find_compat(fdt_blob, fdt_offset, &config->compat)) { list_for_each(compat_node, compat_strings, list_node) { int pos = fit_check_compat(&config->compat, @@ -419,6 +421,8 @@ i++; } } + + return 0; }
struct fit_config_node *fit_load(void *fit) @@ -457,55 +461,40 @@ printk(BIOS_DEBUG, "\n"); /* Process and list the configs. */ list_for_each(config, config_nodes, list_node) { - if (config->kernel) - config->kernel_node = find_image(config->kernel); - if (config->fdt) - config->fdt_node = find_image(config->fdt); - if (config->ramdisk) - config->ramdisk_node = find_image(config->ramdisk); + if (!config->kernel) { + printk(BIOS_ERR, + "ERROR: config %s has no kernel, skipping.\n", + config->name); + continue; + } + if (!config->fdt) { + printk(BIOS_ERR, + "ERROR: config %s has no FDT, skipping.\n", + config->name); + continue; + }
- if (config->ramdisk_node && - config->ramdisk_node->compression < 0) { + if (config->ramdisk && + config->ramdisk->compression < 0) { printk(BIOS_WARNING, "WARN: Ramdisk is compressed with " "an unsupported algorithm, discarding config %s." "\n", config->name); - list_remove(&config->list_node); continue; }
- if (!config->kernel_node || - (config->fdt && !config->fdt_node)) { - printk(BIOS_DEBUG, "FIT: Missing image, discarding " - "config %s.\n", config->name); - list_remove(&config->list_node); + if (fit_update_compat(config)) continue; - }
- if (config->fdt_node) { - if (config->fdt_node->compression != - CBFS_COMPRESS_NONE) { - printk(BIOS_DEBUG, - "FIT: FDT compression not yet supported," - " skipping config %s.\n", config->name); - list_remove(&config->list_node); - continue; - } - - config->compat_pos = -1; - config->compat_rank = -1; - - fit_update_compat(config->fdt_node->data, config); - } printk(BIOS_DEBUG, "FIT: config %s", config->name); if (default_config_name && !strcmp(config->name, default_config_name)) { printk(BIOS_DEBUG, " (default)"); default_config = config; } - if (config->fdt) - printk(BIOS_DEBUG, ", fdt %s", config->fdt); - if (config->ramdisk) - printk(BIOS_DEBUG, ", ramdisk %s", config->ramdisk); + printk(BIOS_DEBUG, ", kernel %s", config->kernel->name); + printk(BIOS_DEBUG, ", fdt %s", config->fdt->name); + if (config->ramdisk) printk(BIOS_DEBUG, + ", ramdisk %s", config->ramdisk->name); if (config->compat.name) { printk(BIOS_DEBUG, ", compat"); int bytes = config->compat.size; diff --git a/src/lib/fit_payload.c b/src/lib/fit_payload.c index e0158f2..4bc6622 100644 --- a/src/lib/fit_payload.c +++ b/src/lib/fit_payload.c @@ -176,37 +176,34 @@
struct fit_config_node *config = fit_load(data);
- if (!config || !config->kernel_node) { + if (!config) { printk(BIOS_ERR, "ERROR: Could not load FIT\n"); rdev_munmap(prog_rdev(payload), data); return; }
- if (config->fdt_node) { - dt = fdt_unflatten(config->fdt_node->data); - if (!dt) { - printk(BIOS_ERR, - "ERROR: Failed to unflatten the FDT.\n"); - rdev_munmap(prog_rdev(payload), data); - return; - } - - dt_apply_fixups(dt); - - /* Insert coreboot specific information */ - add_cb_fdt_data(dt); - - /* Update device_tree */ -#if defined(CONFIG_LINUX_COMMAND_LINE) - fit_update_chosen(dt, (char *)CONFIG_LINUX_COMMAND_LINE); -#endif - fit_update_memory(dt); + dt = fdt_unflatten(config->fdt->data); + if (!dt) { + printk(BIOS_ERR, "ERROR: Failed to unflatten the FDT.\n"); + rdev_munmap(prog_rdev(payload), data); + return; }
+ dt_apply_fixups(dt); + + /* Insert coreboot specific information */ + add_cb_fdt_data(dt); + + /* Update device_tree */ +#if defined(CONFIG_LINUX_COMMAND_LINE) + fit_update_chosen(dt, (char *)CONFIG_LINUX_COMMAND_LINE); +#endif + fit_update_memory(dt); + /* Collect infos for fit_payload_arch */ - kernel.size = config->kernel_node->size; + kernel.size = config->kernel->size; fdt.size = dt ? dt_flat_size(dt) : 0; - initrd.size = config->ramdisk_node ? config->ramdisk_node->size : 0; + initrd.size = config->ramdisk ? config->ramdisk->size : 0;
/* Invoke arch specific payload placement and fixups */ if (!fit_payload_arch(payload, config, &kernel, &fdt, &initrd)) { @@ -216,17 +213,15 @@ return; }
- /* Load the images to given position */ - if (config->fdt_node) { - /* Update device_tree */ - if (config->ramdisk_node) - fit_add_ramdisk(dt, (void *)initrd.offset, initrd.size); + /* Update ramdisk location in FDT */ + if (config->ramdisk) + fit_add_ramdisk(dt, (void *)initrd.offset, initrd.size);
- pack_fdt(&fdt, dt); - } + /* Repack FDT for handoff to kernel */ + pack_fdt(&fdt, dt);
- if (config->ramdisk_node && - extract(&initrd, config->ramdisk_node)) { + if (config->ramdisk && + 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); @@ -235,7 +230,7 @@
timestamp_add_now(TS_KERNEL_DECOMPRESSION);
- if (extract(&kernel, config->kernel_node)) { + 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);
build bot (Jenkins) has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32870 )
Change subject: fit: Refactor config node handling ......................................................................
Patch Set 1:
(1 comment)
https://review.coreboot.org/#/c/32870/1/src/lib/fit.c File src/lib/fit.c:
https://review.coreboot.org/#/c/32870/1/src/lib/fit.c@496 PS1, Line 496: if (config->ramdisk) printk(BIOS_DEBUG, trailing statements should be on next line
Hello Patrick Rudolph,
I'd like you to reexamine a change. Please visit
https://review.coreboot.org/c/coreboot/+/32870
to look at the new patch set (#2).
Change subject: fit: Refactor config node handling ......................................................................
fit: Refactor config node handling
This patch makes some minor refactoring to the way the FIT parser handles config nodes. A lot of this code was written in the dawn age of depthcharge when its device tree library wasn't as well-stocked yet, so some of it can be rewritten nicer with more high-level primitives. There's no point in storing both the string name and the actual FDT node of a FIT image node separately, since the latter also contains the former, so remove that. Also eliminate code for the case of not having an FDT (which makes no sense), and move some more FDT validity/compat checking into fit_update_compat() (mostly in anticipation of later changes).
This patch was adapted from depthcharge's http://crosreview.com/1553456 with a couple of modifications specific to coreboot's custom FIT loading code.
Change-Id: Ia79e0fd0e1159c4aca64c453b82a0379b133350d Signed-off-by: Julius Werner jwerner@chromium.org --- M src/arch/arm64/fit_payload.c M src/include/fit.h M src/lib/fit.c M src/lib/fit_payload.c 4 files changed, 92 insertions(+), 116 deletions(-)
git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/32870/2
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32870 )
Change subject: fit: Refactor config node handling ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/32870/5/src/lib/fit_payload.c File src/lib/fit_payload.c:
https://review.coreboot.org/#/c/32870/5/src/lib/fit_payload.c@198 PS5, Line 198: #if defined(CONFIG_LINUX_COMMAND_LINE) : fit_update_chosen(dt, (char *)CONFIG_LINUX_COMMAND_LINE); : #endif can we do
if (CONFIG(...)) ?
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32870 )
Change subject: fit: Refactor config node handling ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/32870/5/src/lib/fit_payload.c File src/lib/fit_payload.c:
https://review.coreboot.org/#/c/32870/5/src/lib/fit_payload.c@198 PS5, Line 198: #if defined(CONFIG_LINUX_COMMAND_LINE) : fit_update_chosen(dt, (char *)CONFIG_LINUX_COMMAND_LINE); : #endif
can we do […]
No, because this is a string Kconfig, CONFIG() is only for boolean Kconfigs.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32870 )
Change subject: fit: Refactor config node handling ......................................................................
Patch Set 5: Code-Review+2
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32870 )
Change subject: fit: Refactor config node handling ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/32870/5/src/lib/fit_payload.c File src/lib/fit_payload.c:
https://review.coreboot.org/#/c/32870/5/src/lib/fit_payload.c@198 PS5, Line 198: #if defined(CONFIG_LINUX_COMMAND_LINE) : fit_update_chosen(dt, (char *)CONFIG_LINUX_COMMAND_LINE); : #endif
No, because this is a string Kconfig, CONFIG() is only for boolean Kconfigs.
not blocking +2, but will it help if we for example define the config as NULL and check that?
simply trying to eliminate conditional compile.
Julius Werner has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32870 )
Change subject: fit: Refactor config node handling ......................................................................
Patch Set 5:
(1 comment)
https://review.coreboot.org/#/c/32870/5/src/lib/fit_payload.c File src/lib/fit_payload.c:
https://review.coreboot.org/#/c/32870/5/src/lib/fit_payload.c@198 PS5, Line 198: #if defined(CONFIG_LINUX_COMMAND_LINE) : fit_update_chosen(dt, (char *)CONFIG_LINUX_COMMAND_LINE); : #endif
not blocking +2, but will it help if we for example define the config as NULL and check that? […]
Kconfig can't do that, if the string symbol is not visible it will just not be defined at all in config.h. I guess we could hack kconfig further to change that, but I'm not sure if we want that. We use string symbols quite rarely anyway... I think doing this in the few cases that we do is fine.
Hung-Te Lin has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/32870 )
Change subject: fit: Refactor config node handling ......................................................................
Patch Set 5:
agree. let's keep current version.
Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32870 )
Change subject: fit: Refactor config node handling ......................................................................
fit: Refactor config node handling
This patch makes some minor refactoring to the way the FIT parser handles config nodes. A lot of this code was written in the dawn age of depthcharge when its device tree library wasn't as well-stocked yet, so some of it can be rewritten nicer with more high-level primitives. There's no point in storing both the string name and the actual FDT node of a FIT image node separately, since the latter also contains the former, so remove that. Also eliminate code for the case of not having an FDT (which makes no sense), and move some more FDT validity/compat checking into fit_update_compat() (mostly in anticipation of later changes).
This patch was adapted from depthcharge's http://crosreview.com/1553456 with a couple of modifications specific to coreboot's custom FIT loading code.
Change-Id: Ia79e0fd0e1159c4aca64c453b82a0379b133350d Signed-off-by: Julius Werner jwerner@chromium.org Reviewed-on: https://review.coreboot.org/c/coreboot/+/32870 Reviewed-by: Hung-Te Lin hungte@chromium.org Tested-by: build bot (Jenkins) no-reply@coreboot.org --- M src/arch/arm64/fit_payload.c M src/include/fit.h M src/lib/fit.c M src/lib/fit_payload.c 4 files changed, 92 insertions(+), 116 deletions(-)
Approvals: build bot (Jenkins): Verified Hung-Te Lin: Looks good to me, approved
diff --git a/src/arch/arm64/fit_payload.c b/src/arch/arm64/fit_payload.c index c4bbcee..fd1bae1 100644 --- a/src/arch/arm64/fit_payload.c +++ b/src/arch/arm64/fit_payload.c @@ -184,20 +184,14 @@ bool place_anywhere; void *arg = NULL;
- if (!config->fdt || !fdt) { - printk(BIOS_CRIT, "CRIT: Providing a valid FDT is mandatory to " - "boot an ARM64 kernel!\n"); - return false; - } - - if (!decompress_kernel_header(config->kernel_node)) { + if (!decompress_kernel_header(config->kernel)) { printk(BIOS_CRIT, "CRIT: Payload doesn't look like an ARM64" " kernel Image.\n"); return false; }
/* Update kernel size from image header, if possible */ - kernel->size = get_kernel_size(config->kernel_node); + kernel->size = get_kernel_size(config->kernel); printk(BIOS_DEBUG, "FIT: Using kernel size of 0x%zx bytes\n", kernel->size);
diff --git a/src/include/fit.h b/src/include/fit.h index 6e0667f..758ee70 100644 --- a/src/include/fit.h +++ b/src/include/fit.h @@ -27,7 +27,7 @@ struct fit_image_node { const char *name; - const void *data; + void *data; uint32_t size; int compression;
@@ -37,12 +37,9 @@ struct fit_config_node { const char *name; - const char *kernel; - struct fit_image_node *kernel_node; - const char *fdt; - struct fit_image_node *fdt_node; - const char *ramdisk; - struct fit_image_node *ramdisk_node; + struct fit_image_node *kernel; + struct fit_image_node *fdt; + struct fit_image_node *ramdisk; struct fdt_property compat; int compat_rank; int compat_pos; diff --git a/src/lib/fit.c b/src/lib/fit.c index c98ba2f..5fbcd77 100644 --- a/src/lib/fit.c +++ b/src/lib/fit.c @@ -85,6 +85,17 @@ fit_add_compat_string(compat_string); }
+static struct fit_image_node *find_image(const char *name) +{ + struct fit_image_node *image; + list_for_each(image, image_nodes, list_node) { + if (!strcmp(image->name, name)) + return image; + } + printk(BIOS_ERR, "ERROR: Cannot find image node %s!\n", name); + return NULL; +} + static void image_node(struct device_tree_node *node) { struct fit_image_node *image = xzalloc(sizeof(*image)); @@ -120,11 +131,11 @@ struct device_tree_property *prop; list_for_each(prop, node->properties, list_node) { if (!strcmp("kernel", prop->prop.name)) - config->kernel = prop->prop.data; + config->kernel = find_image(prop->prop.data); else if (!strcmp("fdt", prop->prop.name)) - config->fdt = prop->prop.data; + config->fdt = find_image(prop->prop.data); else if (!strcmp("ramdisk", prop->prop.name)) - config->ramdisk = prop->prop.data; + config->ramdisk = find_image(prop->prop.data); }
list_insert_after(&config->list_node, &config_nodes); @@ -132,40 +143,22 @@
static void fit_unpack(struct device_tree *tree, const char **default_config) { - assert(tree && tree->root); + struct device_tree_node *child; + struct device_tree_node *images = dt_find_node_by_path(tree, "/images", + NULL, NULL, 0); + if (images) + list_for_each(child, images->children, list_node) + image_node(child);
- struct device_tree_node *top; - list_for_each(top, tree->root->children, list_node) { - struct device_tree_node *child; - if (!strcmp("images", top->name)) { - - list_for_each(child, top->children, list_node) - image_node(child); - - } else if (!strcmp("configurations", top->name)) { - struct device_tree_property *prop; - list_for_each(prop, top->properties, list_node) { - if (!strcmp("default", prop->prop.name) && - default_config) - *default_config = prop->prop.data; - } - - list_for_each(child, top->children, list_node) - config_node(child); - } + struct device_tree_node *configs = dt_find_node_by_path(tree, + "/configurations", NULL, NULL, 0); + if (configs) { + *default_config = dt_find_string_prop(configs, "default"); + list_for_each(child, configs->children, list_node) + config_node(child); } }
-static struct fit_image_node *find_image(const char *name) -{ - struct fit_image_node *image; - list_for_each(image, image_nodes, list_node) { - if (!strcmp(image->name, name)) - return image; - } - return NULL; -} - static int fdt_find_compat(const void *blob, uint32_t start_offset, struct fdt_property *prop) { @@ -393,18 +386,27 @@
/* * Finds a compat string and updates the compat position and rank. - * @param fdt_blob Pointer to FDT * @param config The current config node to operate on + * @return 0 if compat updated, -1 if this FDT cannot be used. */ -static void fit_update_compat(const void *fdt_blob, - struct fit_config_node *config) +static int fit_update_compat(struct fit_config_node *config) { + if (config->fdt->compression != CBFS_COMPRESS_NONE) { + printk(BIOS_ERR, + "FDT compression not yet supported, skipping %s.\n", + config->name); + return -1; + } + + void *fdt_blob = config->fdt->data; struct compat_string_entry *compat_node; const struct fdt_header *fdt_header = (const struct fdt_header *)fdt_blob; uint32_t fdt_offset = be32_to_cpu(fdt_header->structure_offset); size_t i = 0;
+ config->compat_pos = -1; + config->compat_rank = -1; if (!fdt_find_compat(fdt_blob, fdt_offset, &config->compat)) { list_for_each(compat_node, compat_strings, list_node) { int pos = fit_check_compat(&config->compat, @@ -419,6 +421,8 @@ i++; } } + + return 0; }
struct fit_config_node *fit_load(void *fit) @@ -457,55 +461,41 @@ printk(BIOS_DEBUG, "\n"); /* Process and list the configs. */ list_for_each(config, config_nodes, list_node) { - if (config->kernel) - config->kernel_node = find_image(config->kernel); - if (config->fdt) - config->fdt_node = find_image(config->fdt); - if (config->ramdisk) - config->ramdisk_node = find_image(config->ramdisk); + if (!config->kernel) { + printk(BIOS_ERR, + "ERROR: config %s has no kernel, skipping.\n", + config->name); + continue; + } + if (!config->fdt) { + printk(BIOS_ERR, + "ERROR: config %s has no FDT, skipping.\n", + config->name); + continue; + }
- if (config->ramdisk_node && - config->ramdisk_node->compression < 0) { + if (config->ramdisk && + config->ramdisk->compression < 0) { printk(BIOS_WARNING, "WARN: Ramdisk is compressed with " "an unsupported algorithm, discarding config %s." "\n", config->name); - list_remove(&config->list_node); continue; }
- if (!config->kernel_node || - (config->fdt && !config->fdt_node)) { - printk(BIOS_DEBUG, "FIT: Missing image, discarding " - "config %s.\n", config->name); - list_remove(&config->list_node); + if (fit_update_compat(config)) continue; - }
- if (config->fdt_node) { - if (config->fdt_node->compression != - CBFS_COMPRESS_NONE) { - printk(BIOS_DEBUG, - "FIT: FDT compression not yet supported," - " skipping config %s.\n", config->name); - list_remove(&config->list_node); - continue; - } - - config->compat_pos = -1; - config->compat_rank = -1; - - fit_update_compat(config->fdt_node->data, config); - } printk(BIOS_DEBUG, "FIT: config %s", config->name); if (default_config_name && !strcmp(config->name, default_config_name)) { printk(BIOS_DEBUG, " (default)"); default_config = config; } - if (config->fdt) - printk(BIOS_DEBUG, ", fdt %s", config->fdt); + printk(BIOS_DEBUG, ", kernel %s", config->kernel->name); + printk(BIOS_DEBUG, ", fdt %s", config->fdt->name); if (config->ramdisk) - printk(BIOS_DEBUG, ", ramdisk %s", config->ramdisk); + printk(BIOS_DEBUG, ", ramdisk %s", + config->ramdisk->name); if (config->compat.name) { printk(BIOS_DEBUG, ", compat"); int bytes = config->compat.size; diff --git a/src/lib/fit_payload.c b/src/lib/fit_payload.c index e0158f2..4bc6622 100644 --- a/src/lib/fit_payload.c +++ b/src/lib/fit_payload.c @@ -176,37 +176,34 @@
struct fit_config_node *config = fit_load(data);
- if (!config || !config->kernel_node) { + if (!config) { printk(BIOS_ERR, "ERROR: Could not load FIT\n"); rdev_munmap(prog_rdev(payload), data); return; }
- if (config->fdt_node) { - dt = fdt_unflatten(config->fdt_node->data); - if (!dt) { - printk(BIOS_ERR, - "ERROR: Failed to unflatten the FDT.\n"); - rdev_munmap(prog_rdev(payload), data); - return; - } - - dt_apply_fixups(dt); - - /* Insert coreboot specific information */ - add_cb_fdt_data(dt); - - /* Update device_tree */ -#if defined(CONFIG_LINUX_COMMAND_LINE) - fit_update_chosen(dt, (char *)CONFIG_LINUX_COMMAND_LINE); -#endif - fit_update_memory(dt); + dt = fdt_unflatten(config->fdt->data); + if (!dt) { + printk(BIOS_ERR, "ERROR: Failed to unflatten the FDT.\n"); + rdev_munmap(prog_rdev(payload), data); + return; }
+ dt_apply_fixups(dt); + + /* Insert coreboot specific information */ + add_cb_fdt_data(dt); + + /* Update device_tree */ +#if defined(CONFIG_LINUX_COMMAND_LINE) + fit_update_chosen(dt, (char *)CONFIG_LINUX_COMMAND_LINE); +#endif + fit_update_memory(dt); + /* Collect infos for fit_payload_arch */ - kernel.size = config->kernel_node->size; + kernel.size = config->kernel->size; fdt.size = dt ? dt_flat_size(dt) : 0; - initrd.size = config->ramdisk_node ? config->ramdisk_node->size : 0; + initrd.size = config->ramdisk ? config->ramdisk->size : 0;
/* Invoke arch specific payload placement and fixups */ if (!fit_payload_arch(payload, config, &kernel, &fdt, &initrd)) { @@ -216,17 +213,15 @@ return; }
- /* Load the images to given position */ - if (config->fdt_node) { - /* Update device_tree */ - if (config->ramdisk_node) - fit_add_ramdisk(dt, (void *)initrd.offset, initrd.size); + /* Update ramdisk location in FDT */ + if (config->ramdisk) + fit_add_ramdisk(dt, (void *)initrd.offset, initrd.size);
- pack_fdt(&fdt, dt); - } + /* Repack FDT for handoff to kernel */ + pack_fdt(&fdt, dt);
- if (config->ramdisk_node && - extract(&initrd, config->ramdisk_node)) { + if (config->ramdisk && + 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); @@ -235,7 +230,7 @@
timestamp_add_now(TS_KERNEL_DECOMPRESSION);
- if (extract(&kernel, config->kernel_node)) { + 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);
-
build bot (Jenkins) (Code Review) -
Hung-Te Lin (Code Review)
-
Julius Werner (Code Review)
-
Patrick Georgi (Code Review)