coreboot-gerrit June 2019

coreboot-gerrit@coreboot.org

1 participants
1454 discussions

Change in ...coreboot[master]: soc/intel/braswell: Use common cpu/intel/car code
by Arthur Heymans (Code Review) 04 Jun '19

04 Jun '19

Arthur Heymans has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32963 Change subject: soc/intel/braswell: Use common cpu/intel/car code ...................................................................... soc/intel/braswell: Use common cpu/intel/car code The code in cpu/intel/car/romstage.c Does most of the things like setting up timestamps, stack guards, entering postcar. A functional difference is that the FSP header is searched for twice instead of passed from the CAR entry to the C code. When using C_ENVIRONMENT_BOOTBLOCK this needs to be done anyway (or a special linker symbol kept across multiple stages is needed, which is likely not worth the speedup). Change-Id: I0f03e5a808f00157fdd807b104417a54e4bde7b2 Signed-off-by: Arthur Heymans <arthur(a)aheymans.xyz> --- M src/drivers/intel/fsp1_1/cache_as_ram.inc M src/drivers/intel/fsp1_1/car.c M src/drivers/intel/fsp1_1/include/fsp/car.h M src/soc/intel/braswell/romstage/Makefile.inc 4 files changed, 29 insertions(+), 89 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/32963/1 diff --git a/src/drivers/intel/fsp1_1/cache_as_ram.inc b/src/drivers/intel/fsp1_1/cache_as_ram.inc index f50641e..e2bc852 100644 --- a/src/drivers/intel/fsp1_1/cache_as_ram.inc +++ b/src/drivers/intel/fsp1_1/cache_as_ram.inc @@ -117,35 +117,33 @@ * mm1: high 32-bits of TSC value */ + /* coreboot assumes stack/heap region will be zero */ + cld + movl %ecx, %edi + neg %ecx + /* Clear up to Temp Ram top. */ + add %edx, %ecx + shrl $2, %ecx + xorl %eax, %eax + rep stosl + + /* Need to align stack to 16 bytes at call instruction. Account for + the pushes below. */ + andl $0xfffffff0, %esp + subl $8, %esp + /* Create cache_as_ram_params on stack */ - pushl %edx /* bootloader CAR end */ - pushl %ecx /* bootloader CAR begin */ - pushl %ebp /* FSP_INFO_HEADER */ pushl %edi /* bist */ movd %mm1, %eax pushl %eax /* tsc[63:32] */ movd %mm0, %eax pushl %eax /* tsc[31:0] */ - pushl %esp /* pointer to cache_as_ram_params */ - - /* Save FSP_INFO_HEADER location in ebx */ - mov %ebp, %ebx - - /* coreboot assumes stack/heap region will be zero */ - cld - movl %ecx, %edi - neg %ecx - /* Only clear up to current stack value. */ - add %esp, %ecx - shrl $2, %ecx - xorl %eax, %eax - rep stosl before_romstage: post_code(0x2A) /* Call cache_as_ram_main(struct cache_as_ram_params *) */ - call cache_as_ram_main + call bootblock_c_entry_bist movb $0x69, %ah jmp .Lhlt diff --git a/src/drivers/intel/fsp1_1/car.c b/src/drivers/intel/fsp1_1/car.c index 2202121..f5faf72 100644 --- a/src/drivers/intel/fsp1_1/car.c +++ b/src/drivers/intel/fsp1_1/car.c @@ -94,66 +94,6 @@ run_postcar_phase(&pcf); } -/* This is the romstage C entry for platforms without - CONFIG_C_ENVIRONMENT_BOOTBLOCK */ -asmlinkage void cache_as_ram_main(struct cache_as_ram_params *car_params) -{ - int i; - const int num_guards = 4; - const u32 stack_guard = 0xdeadbeef; - u32 *stack_base; - u32 size; - - /* Size of unallocated CAR. */ - size = _car_region_end - _car_relocatable_data_end; - size = ALIGN_DOWN(size, 16); - - stack_base = (u32 *)(_car_region_end - size); - - for (i = 0; i < num_guards; i++) - stack_base[i] = stack_guard; - - /* Initialize timestamp book keeping only once. */ - timestamp_init(car_params->tsc); - - /* Call into pre-console init code then initialize console. */ - car_soc_pre_console_init(); - car_mainboard_pre_console_init(); - console_init(); - - printk(BIOS_DEBUG, "FSP TempRamInit successful\n"); - - printk(BIOS_SPEW, "bist: 0x%08x\n", car_params->bist); - printk(BIOS_SPEW, "tsc: 0x%016llx\n", car_params->tsc); - - display_mtrrs(); - - if (car_params->bootloader_car_start != CONFIG_DCACHE_RAM_BASE - || car_params->bootloader_car_end != (CONFIG_DCACHE_RAM_BASE - + CONFIG_DCACHE_RAM_SIZE)) { - printk(BIOS_INFO, "CAR mismatch: %08x--%08x vs %08lx--%08lx\n", - CONFIG_DCACHE_RAM_BASE, - CONFIG_DCACHE_RAM_BASE + CONFIG_DCACHE_RAM_SIZE, - (long)car_params->bootloader_car_start, - (long)car_params->bootloader_car_end); - } - - car_soc_post_console_init(); - car_mainboard_post_console_init(); - - cache_as_ram_stage_main(car_params->fih); - - /* Check the stack. */ - for (i = 0; i < num_guards; i++) { - if (stack_base[i] == stack_guard) - continue; - printk(BIOS_DEBUG, "Smashed stack detected in romstage!\n"); - } - - /* we don't return here */ - platform_enter_postcar(); -} - /* This is the romstage C entry for platforms with CONFIG_C_ENVIRONMENT_BOOTBLOCK */ void mainboard_romstage_entry(unsigned long bist) @@ -172,6 +112,18 @@ fih = find_fsp((uintptr_t)rdev_mmap_full(prog_rdev(&fsp))); } + if (!CONFIG(C_ENVIRONMENT_BOOTBLOCK)) { + /* Call into pre-console init code then initialize console. */ + car_soc_pre_console_init(); + car_mainboard_pre_console_init(); + console_init(); + + display_mtrrs(); + + car_soc_post_console_init(); + car_mainboard_post_console_init(); + } + cache_as_ram_stage_main(fih); } diff --git a/src/drivers/intel/fsp1_1/include/fsp/car.h b/src/drivers/intel/fsp1_1/include/fsp/car.h index c051392..8d7a683 100644 --- a/src/drivers/intel/fsp1_1/include/fsp/car.h +++ b/src/drivers/intel/fsp1_1/include/fsp/car.h @@ -20,17 +20,6 @@ #include <fsp/api.h> #include <stdint.h> -/* cache-as-ram support for FSP 1.1. */ -struct cache_as_ram_params { - uint64_t tsc; - uint32_t bist; - FSP_INFO_HEADER *fih; - uintptr_t bootloader_car_start; - uintptr_t bootloader_car_end; -}; - -/* Entry points from the cache-as-ram assembly code. */ -asmlinkage void cache_as_ram_main(struct cache_as_ram_params *car_params); /* Per stage calls from the above two functions. The void * return from * cache_as_ram_stage_main() is the stack pointer to use in RAM after * exiting cache-as-ram mode. */ diff --git a/src/soc/intel/braswell/romstage/Makefile.inc b/src/soc/intel/braswell/romstage/Makefile.inc index c3ed415..b9c46b0 100644 --- a/src/soc/intel/braswell/romstage/Makefile.inc +++ b/src/soc/intel/braswell/romstage/Makefile.inc @@ -1,3 +1,4 @@ +romstage-y += ../../../../cpu/intel/car/romstage.c romstage-y += early_spi.c romstage-y += pmc.c romstage-y += romstage.c -- To view, visit https://review.coreboot.org/c/coreboot/+/32963 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0f03e5a808f00157fdd807b104417a54e4bde7b2 Gerrit-Change-Number: 32963 Gerrit-PatchSet: 1 Gerrit-Owner: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com> Gerrit-Reviewer: Lee Leahy <leroy.p.leahy(a)intel.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-MessageType: newchange

4 17

Change in ...coreboot[master]: sb/i82801ix: Fix flashconsole on platform
by Felix Held (Code Review) 04 Jun '19

04 Jun '19

Felix Held has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/21107 ) Change subject: sb/i82801ix: Fix flashconsole on platform ...................................................................... Patch Set 17: Code-Review+2 -- To view, visit https://review.coreboot.org/c/coreboot/+/21107 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifd3e8621fa4cb349b7e0e07118cab0380f24ff55 Gerrit-Change-Number: 21107 Gerrit-PatchSet: 17 Gerrit-Owner: Nicola Corna <nicola(a)corna.info> Gerrit-Reviewer: Aaron Durbin <adurbin(a)chromium.org> Gerrit-Reviewer: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-Reviewer: Felix Held <felix-coreboot(a)felixheld.de> Gerrit-Reviewer: Furquan Shaikh <furquan(a)google.com> Gerrit-Reviewer: Huang Jin <huang.jin(a)intel.com> Gerrit-Reviewer: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Matt DeVillier <matt.devillier(a)gmail.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Nicola Corna <nicola(a)corna.info> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-Reviewer: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Reviewer: Werner Zeh <werner.zeh(a)siemens.com> Gerrit-Reviewer: Youness Alaoui <snifikino(a)gmail.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-Comment-Date: Tue, 04 Jun 2019 11:25:09 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: mainboard/facebook/fbg1701: Do initial mainboard commit
by David Hendricks (Code Review) 04 Jun '19

04 Jun '19

David Hendricks has posted comments on this change. ( https://review.coreboot.org/c/coreboot/+/30414 ) Change subject: mainboard/facebook/fbg1701: Do initial mainboard commit ...................................................................... Patch Set 25: Code-Review+1 (1 comment) almost there... https://review.coreboot.org/#/c/30414/25/src/mainboard/facebook/fbg1701/rom… File src/mainboard/facebook/fbg1701/romstage.c: https://review.coreboot.org/#/c/30414/25/src/mainboard/facebook/fbg1701/rom… PS25, Line 28: /* First call into mainboard. */ : void mainboard_romstage_entry(struct romstage_params *params) : { : romstage_common(params); : } : This seems to be leftover from earlier baytrail --> braswell porting, and is no longer needed. If I remove it then it will compile locally. I don't think romstage_common() exists for braswell, so removing this should have no effect (aside from fixing compilation). -- To view, visit https://review.coreboot.org/c/coreboot/+/30414 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I28ac78a630ee705b1e546031f024bfe7f952ab39 Gerrit-Change-Number: 30414 Gerrit-PatchSet: 25 Gerrit-Owner: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Alex Thiessen <alex.thiessen.de+coreboot(a)gmail.com> Gerrit-Reviewer: David Hendricks <david.hendricks(a)gmail.com> Gerrit-Reviewer: Frans Hendriks <fhendriks(a)eltan.com> Gerrit-Reviewer: Martin Roth <martinroth(a)google.com> Gerrit-Reviewer: Michał Żygowski <michal.zygowski(a)3mdeb.com> Gerrit-Reviewer: Nico Huber <nico.h(a)gmx.de> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: Patrick Rudolph <siro(a)das-labor.org> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Angel Pons <th3fanbus(a)gmail.com> Gerrit-CC: Arthur Heymans <arthur(a)aheymans.xyz> Gerrit-CC: Kyösti Mälkki <kyosti.malkki(a)gmail.com> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-CC: Philipp Deppenwiese <zaolin.daisuki(a)gmail.com> Gerrit-Comment-Date: Tue, 04 Jun 2019 11:25:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

Change in ...coreboot[master]: device_tree: Make FDT property data non-const
by Julius Werner (Code Review) 04 Jun '19

04 Jun '19

Hello Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/32868 to review the following change. Change subject: device_tree: Make FDT property data non-const ...................................................................... device_tree: Make FDT property data non-const FDT property data should not be const -- sometimes we need to update it, for example when fixing up phandles in an overlay. On the other hand it's occasionally desirable to put a string constant in there without having to strdup() it all the time... let's just live with the tiny implicit assumption that the data we'd want to modify (phandle references, mostly) will never be added from string constants, and put a cast in dt_add_string_prop(). Change-Id: Ifac103fcff0520cc427ab9a2aa141c65e12507ac Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- M src/include/device_tree.h M src/lib/device_tree.c 2 files changed, 4 insertions(+), 4 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/68/32868/1 diff --git a/src/include/device_tree.h b/src/include/device_tree.h index e3723c8..d9d9613 100644 --- a/src/include/device_tree.h +++ b/src/include/device_tree.h @@ -52,7 +52,7 @@ struct fdt_property { const char *name; - const void *data; + void *data; uint32_t size; }; @@ -165,7 +165,7 @@ void dt_delete_prop(struct device_tree_node *node, const char *name); // Add different kinds of properties to a node, or update existing ones. void dt_add_bin_prop(struct device_tree_node *node, const char *name, - const void *data, size_t size); + void *data, size_t size); void dt_add_string_prop(struct device_tree_node *node, const char *name, const char *str); void dt_add_u32_prop(struct device_tree_node *node, const char *name, u32 val); diff --git a/src/lib/device_tree.c b/src/lib/device_tree.c index 3c4bd24..a5021ca 100644 --- a/src/lib/device_tree.c +++ b/src/lib/device_tree.c @@ -883,7 +883,7 @@ * @param size The size of data in bytes. */ void dt_add_bin_prop(struct device_tree_node *node, const char *name, - const void *data, size_t size) + void *data, size_t size) { struct device_tree_property *prop; @@ -955,7 +955,7 @@ void dt_add_string_prop(struct device_tree_node *node, const char *name, const char *str) { - dt_add_bin_prop(node, name, str, strlen(str) + 1); + dt_add_bin_prop(node, name, (char *)str, strlen(str) + 1); } /* -- To view, visit https://review.coreboot.org/c/coreboot/+/32868 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifac103fcff0520cc427ab9a2aa141c65e12507ac Gerrit-Change-Number: 32868 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

3 3

Change in ...coreboot[master]: device_tree: Match debug output format to dtc -O dts output
by Julius Werner (Code Review) 04 Jun '19

04 Jun '19

Hello Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/32867 to review the following change. Change subject: device_tree: Match debug output format to dtc -O dts output ...................................................................... device_tree: Match debug output format to dtc -O dts output This patch updates the device tree dumping functions (not compiled by default but available for debugging) to output properties and nodes in a format similar to .dts files that is very close to what dtc outputs when you decompile a .dtb with it. This makes it easier to match device tree dumps from coreboot with device tree dumps generated by other device tree tooling. This patch was adapted from depthcharge's http://crosreview.com/1536386 Change-Id: Ib40e50d906aff05473a70c4fc9b124d63232558c Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- M src/lib/device_tree.c 1 file changed, 38 insertions(+), 11 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/67/32867/1 diff --git a/src/lib/device_tree.c b/src/lib/device_tree.c index bbcd7c0..3c4bd24 100644 --- a/src/lib/device_tree.c +++ b/src/lib/device_tree.c @@ -80,21 +80,36 @@ static void print_indent(int depth) { - while (depth--) - printk(BIOS_DEBUG, " "); + printk(BIOS_DEBUG, "%*s", depth * 8, ""); } static void print_property(const struct fdt_property *prop, int depth) { + int is_string = prop->size > 0 && + ((char *)prop->data)[prop->size - 1] == '\0'; + + if (is_string) + for (const char *c = prop->data; *c != '\0'; c++) + if (!isprint(*c)) + is_string = 0; + print_indent(depth); - printk(BIOS_DEBUG, "prop \"%s\" (%d bytes).\n", prop->name, prop->size); - print_indent(depth + 1); - for (int i = 0; i < MIN(25, prop->size); i++) { - printk(BIOS_DEBUG, "%02x ", ((uint8_t *)prop->data)[i]); + if (is_string) { + printk(BIOS_DEBUG, "%s = \"%s\";\n", + prop->name, (const char *)prop->data); + } else { + printk(BIOS_DEBUG, "%s = < ", prop->name); + for (int i = 0; i < MIN(128, prop->size); i += 4) { + uint32_t val = 0; + for (int j = 0; j < MIN(4, prop->size - i); j++) + val |= ((uint8_t *)prop->data)[i + j] << + (24 - j * 8); + printk(BIOS_DEBUG, "%#.2x ", val); + } + if (prop->size > 128) + printk(BIOS_DEBUG, "..."); + printk(BIOS_DEBUG, ">;\n"); } - if (prop->size > 25) - printk(BIOS_DEBUG, "..."); - printk(BIOS_DEBUG, "\n"); } static int print_flat_node(const void *blob, uint32_t start_offset, int depth) @@ -109,7 +124,7 @@ offset += size; print_indent(depth); - printk(BIOS_DEBUG, "name = %s\n", name); + printk(BIOS_DEBUG, "%s {\n", name); struct fdt_property prop; while ((size = fdt_next_property(blob, offset, &prop))) { @@ -118,9 +133,14 @@ offset += size; } + printk(BIOS_DEBUG, "\n"); // empty line between props and nodes + while ((size = print_flat_node(blob, offset, depth + 1))) offset += size; + print_indent(depth); + printk(BIOS_DEBUG, "}\n"); + return offset - start_offset + sizeof(uint32_t); } @@ -458,15 +478,22 @@ static void print_node(const struct device_tree_node *node, int depth) { print_indent(depth); - printk(BIOS_DEBUG, "name = %s\n", node->name); + if (depth == 0) // root node has no name, print a starting slash + printk(BIOS_DEBUG, "/"); + printk(BIOS_DEBUG, "%s {\n", node->name); struct device_tree_property *prop; list_for_each(prop, node->properties, list_node) print_property(&prop->prop, depth + 1); + printk(BIOS_DEBUG, "\n"); // empty line between props and nodes + struct device_tree_node *child; list_for_each(child, node->children, list_node) print_node(child, depth + 1); + + print_indent(depth); + printk(BIOS_DEBUG, "};\n"); } void dt_print_node(const struct device_tree_node *node) -- To view, visit https://review.coreboot.org/c/coreboot/+/32867 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ib40e50d906aff05473a70c4fc9b124d63232558c Gerrit-Change-Number: 32867 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

3 5

Change in ...coreboot[master]: device_tree: Add support for aliases
by Patrick Georgi (Code Review) 04 Jun '19

04 Jun '19

Patrick Georgi has submitted this change and it was merged. ( https://review.coreboot.org/c/coreboot/+/32866 ) Change subject: device_tree: Add support for aliases ...................................................................... device_tree: Add support for aliases This patch adds support to lookup nodes via the "/aliases" mechanism in device trees. This may be required for overlay support (don't quite remember tbh) and is also just a generally useful feature. It was adapted from depthcharge's http://crosreview.com/1249703 and http://crosreview.com/1542702. Change-Id: I1289ab2f02c4877a2d0111040384827e2b48a34a Signed-off-by: Julius Werner <jwerner(a)chromium.org> Reviewed-on: https://review.coreboot.org/c/coreboot/+/32866 Reviewed-by: Hung-Te Lin <hungte(a)chromium.org> Tested-by: build bot (Jenkins) <no-reply(a)coreboot.org> --- M src/include/device_tree.h M src/lib/device_tree.c 2 files changed, 77 insertions(+), 12 deletions(-) Approvals: build bot (Jenkins): Verified Hung-Te Lin: Looks good to me, approved diff --git a/src/include/device_tree.h b/src/include/device_tree.h index 6eaeacd..e3723c8 100644 --- a/src/include/device_tree.h +++ b/src/include/device_tree.h @@ -146,6 +146,9 @@ // represented as a string of '/' separated node names. struct device_tree_node *dt_find_node_by_path(struct device_tree *tree, const char *path, u32 *addrcp, u32 *sizecp, int create); +// Look up a node through an alias. +struct device_tree_node *dt_find_node_by_alias(struct device_tree *tree, + const char *alias); // Look up a node relative to a parent node, through its compatible string. struct device_tree_node *dt_find_compat(struct device_tree_node *parent, const char *compatible); // Look up the next child of a parent node, through its compatible string. It diff --git a/src/lib/device_tree.c b/src/lib/device_tree.c index e26021e..bbcd7c0 100644 --- a/src/lib/device_tree.c +++ b/src/lib/device_tree.c @@ -564,27 +564,65 @@ * @param create 1: Create node(s) if not found. 0: Return NULL instead. * @return The found/created node, or NULL. * - * It is the caller responsibility to provide the correct path string, namely - * starting with a '/', not ending in a '/' and not having "//" anywhere in it. - */ + * It is the caller responsibility to provide a path string that doesn't end + * with a '/' and doesn't contain any "//". If the path does not start with a + * '/', the first segment is interpreted as an alias. */ struct device_tree_node *dt_find_node_by_path(struct device_tree *tree, const char *path, u32 *addrcp, u32 *sizecp, int create) { - char *dup_path = strdup(&path[1]); /* remove leading '/' */ + char *sub_path; + char *duped_str; + struct device_tree_node *parent; + char *next_slash; /* Hopefully enough depth for any node. */ const char *path_array[15]; int i; - char *next_slash; struct device_tree_node *node = NULL; - if (!dup_path) - return NULL; + if (path[0] == '/') { // regular path + if (path[1] == '\0') { // special case: "/" is root node + dt_read_cell_props(tree->root, addrcp, sizecp); + return tree->root; + } - next_slash = dup_path; - path_array[0] = dup_path; + sub_path = duped_str = strdup(&path[1]); + if (!sub_path) + return NULL; + + parent = tree->root; + } else { // alias + char *alias; + + alias = duped_str = strdup(path); + if (!alias) + return NULL; + + sub_path = strchr(alias, '/'); + if (sub_path) + *sub_path = '\0'; + + parent = dt_find_node_by_alias(tree, alias); + if (!parent) { + printk(BIOS_DEBUG, + "Could not find node '%s', alias '%s' does not exist\n", + path, alias); + free(duped_str); + return NULL; + } + + if (!sub_path) { + // it's just the alias, no sub-path + free(duped_str); + return parent; + } + + sub_path++; + } + + next_slash = sub_path; + path_array[0] = sub_path; for (i = 1; i < (ARRAY_SIZE(path_array) - 1); i++) { - next_slash = strchr(next_slash, '/'); if (!next_slash) break; @@ -595,14 +633,38 @@ if (!next_slash) { path_array[i] = NULL; - node = dt_find_node(tree->root, path_array, + node = dt_find_node(parent, path_array, addrcp, sizecp, create); } - free(dup_path); + free(duped_str); return node; } +/* + * Find a node from an alias + * + * @param tree The device tree. + * @param alias The alias name. + * @return The found node, or NULL. + */ +struct device_tree_node *dt_find_node_by_alias(struct device_tree *tree, + const char *alias) +{ + struct device_tree_node *node; + const char *alias_path; + + node = dt_find_node_by_path(tree, "/aliases", NULL, NULL, 0); + if (!node) + return NULL; + + alias_path = dt_find_string_prop(node, alias); + if (!alias_path) + return NULL; + + return dt_find_node_by_path(tree, alias_path, NULL, NULL, 0); +} + struct device_tree_node *dt_find_node_by_phandle(struct device_tree_node *root, uint32_t phandle) { -- To view, visit https://review.coreboot.org/c/coreboot/+/32866 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I1289ab2f02c4877a2d0111040384827e2b48a34a Gerrit-Change-Number: 32866 Gerrit-PatchSet: 6 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Hung-Te Lin <hungte(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Georgi <pgeorgi(a)google.com> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-Reviewer: build bot (Jenkins) <no-reply(a)coreboot.org> Gerrit-CC: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: merged

1 0

Change in ...coreboot[master]: device_tree: Have absolute paths start with '/'
by Julius Werner (Code Review) 04 Jun '19

04 Jun '19

Hello Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/32865 to review the following change. Change subject: device_tree: Have absolute paths start with '/' ...................................................................... device_tree: Have absolute paths start with '/' Currently DT paths are *not* expected to start with '/'. This is not what the spec says (see Devicetree Specification v0.2, 2.2.3 Path Names) and also not what is done by Linux. Change dt_find_node_by_path() to expect paths to start with '/' and add a leading '/' to all DT path strings. Besides the compatibility with the spec this change is also needed to support aliases in the future. This patch was adapted from depthcharge's http://crosreview.com/1252770 Change-Id: Ibdf59ccbb4ead38c6193b630642fd1f1e847dd89 Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- M src/lib/device_tree.c M src/soc/cavium/cn81xx/soc.c 2 files changed, 9 insertions(+), 9 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/65/32865/1 diff --git a/src/lib/device_tree.c b/src/lib/device_tree.c index 3e1dd35..e26021e 100644 --- a/src/lib/device_tree.c +++ b/src/lib/device_tree.c @@ -556,7 +556,7 @@ * * @param tree The device tree to search. * @param path A string representing a path in the device tree, with - * nodes separated by '/'. Example: "soc/firmware/coreboot" + * nodes separated by '/'. Example: "/firmware/coreboot" * @param addrcp Pointer that will be updated with any #address-cells * value found in the path. May be NULL to ignore. * @param sizecp Pointer that will be updated with any #size-cells @@ -565,13 +565,13 @@ * @return The found/created node, or NULL. * * It is the caller responsibility to provide the correct path string, namely - * not starting or ending with a '/', and not having "//" anywhere in it. + * starting with a '/', not ending in a '/' and not having "//" anywhere in it. */ struct device_tree_node *dt_find_node_by_path(struct device_tree *tree, const char *path, u32 *addrcp, u32 *sizecp, int create) { - char *dup_path = strdup(path); + char *dup_path = strdup(&path[1]); /* remove leading '/' */ /* Hopefully enough depth for any node. */ const char *path_array[15]; int i; @@ -995,7 +995,7 @@ struct device_tree_node *reserved; u32 addr = 0, size = 0; - reserved = dt_find_node_by_path(tree, "reserved-memory", &addr, + reserved = dt_find_node_by_path(tree, "/reserved-memory", &addr, &size, 1); if (!reserved) return NULL; diff --git a/src/soc/cavium/cn81xx/soc.c b/src/soc/cavium/cn81xx/soc.c index 20542e5..efe7d6d 100644 --- a/src/soc/cavium/cn81xx/soc.c +++ b/src/soc/cavium/cn81xx/soc.c @@ -185,7 +185,7 @@ size_t i; /* Set the sclk clock rate. */ - dt_node = dt_find_node_by_path(tree, "soc@0/sclk", NULL, NULL, 0); + dt_node = dt_find_node_by_path(tree, "/soc@0/sclk", NULL, NULL, 0); if (dt_node) { const u32 freq = thunderx_get_io_clock(); printk(BIOS_INFO, "%s: Set SCLK to %u Hz\n", __func__, freq); @@ -195,7 +195,7 @@ __func__); /* Set refclkuaa clock rate. */ - dt_node = dt_find_node_by_path(tree, "soc@0/refclkuaa", NULL, + dt_node = dt_find_node_by_path(tree, "/soc@0/refclkuaa", NULL, NULL, 0); if (dt_node) { const u32 freq = uart_platform_refclk(); @@ -211,7 +211,7 @@ char path[32]; const uint64_t addr = UAAx_PF_BAR0(i); /* Remove the node */ - snprintf(path, sizeof(path), "soc@0/serial@%llx", addr); + snprintf(path, sizeof(path), "/soc@0/serial@%llx", addr); dt_node = dt_find_node_by_path(tree, path, NULL, NULL, 0); if (!dt_node || uart_is_enabled(i)) { printk(BIOS_INFO, "%s: ignoring %s\n", __func__, path); @@ -227,7 +227,7 @@ u32 phandle = 0; const uint64_t addr = PEM_PEMX_PF_BAR0(i); /* Remove the node */ - snprintf(path, sizeof(path), "soc@0/pci@%llx", addr); + snprintf(path, sizeof(path), "/soc@0/pci@%llx", addr); dt_node = dt_find_node_by_path(tree, path, NULL, NULL, 0); if (!dt_node || bdk_pcie_is_running(0, i)) { printk(BIOS_INFO, "%s: ignoring %s\n", __func__, path); @@ -239,7 +239,7 @@ list_remove(&dt_node->list_node); /* Remove phandle to non existing nodes */ - snprintf(path, sizeof(path), "soc@0/smmu0@%llx", SMMU_PF_BAR0); + snprintf(path, sizeof(path), "/soc@0/smmu0@%llx", SMMU_PF_BAR0); dt_node = dt_find_node_by_path(tree, path, NULL, NULL, 0); if (!dt_node) { printk(BIOS_ERR, "%s: SMMU entry not found\n", -- To view, visit https://review.coreboot.org/c/coreboot/+/32865 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibdf59ccbb4ead38c6193b630642fd1f1e847dd89 Gerrit-Change-Number: 32865 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

3 4

Change in ...coreboot[master]: device_tree: Drop sub-node path lookup from dt_find_node_by_path()
by Julius Werner (Code Review) 04 Jun '19

04 Jun '19

Hello Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/32864 to review the following change. Change subject: device_tree: Drop sub-node path lookup from dt_find_node_by_path() ...................................................................... device_tree: Drop sub-node path lookup from dt_find_node_by_path() Besides looking up a node with an absolute path dt_find_node_by_path() currently also supports finding a sub-node of a non-root node. All callers of the function pass the root node though, so it seems there is no real need for this functionality. Also it is planned to support DT path names with aliases, which would become messy in combination with the lookup from a sub-node. Change the interface of dt_find_node_by_path() to receive the DT tree object instead of a parent node and adapt all callers accordingly. This patch was adapted from depthcharge's http://crosreview.com/1252769 Change-Id: Iff56be4da2461ae73a7301dcaa315758d2a8c999 Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- M src/include/device_tree.h M src/lib/device_tree.c M src/soc/cavium/cn81xx/soc.c 3 files changed, 14 insertions(+), 14 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/64/32864/1 diff --git a/src/include/device_tree.h b/src/include/device_tree.h index 8280dad..6eaeacd 100644 --- a/src/include/device_tree.h +++ b/src/include/device_tree.h @@ -142,10 +142,10 @@ u32 *addrcp, u32 *sizecp, int create); struct device_tree_node *dt_find_node_by_phandle(struct device_tree_node *root, uint32_t phandle); -// Look up or create a node relative to a parent node, through its path +// Look up or create a node in the tree, through its path // represented as a string of '/' separated node names. -struct device_tree_node *dt_find_node_by_path(struct device_tree_node *parent, const char *path, - u32 *addrcp, u32 *sizecp, int create); +struct device_tree_node *dt_find_node_by_path(struct device_tree *tree, + const char *path, u32 *addrcp, u32 *sizecp, int create); // Look up a node relative to a parent node, through its compatible string. struct device_tree_node *dt_find_compat(struct device_tree_node *parent, const char *compatible); // Look up the next child of a parent node, through its compatible string. It diff --git a/src/lib/device_tree.c b/src/lib/device_tree.c index 44eca4e..3e1dd35 100644 --- a/src/lib/device_tree.c +++ b/src/lib/device_tree.c @@ -552,9 +552,9 @@ } /* - * Find a node from a string device tree path, relative to a parent node. + * Find a node in the tree from a string device tree path. * - * @param parent The node from which to start the relative path lookup. + * @param tree The device tree to search. * @param path A string representing a path in the device tree, with * nodes separated by '/'. Example: "soc/firmware/coreboot" * @param addrcp Pointer that will be updated with any #address-cells @@ -567,7 +567,7 @@ * It is the caller responsibility to provide the correct path string, namely * not starting or ending with a '/', and not having "//" anywhere in it. */ -struct device_tree_node *dt_find_node_by_path(struct device_tree_node *parent, +struct device_tree_node *dt_find_node_by_path(struct device_tree *tree, const char *path, u32 *addrcp, u32 *sizecp, int create) { @@ -595,7 +595,7 @@ if (!next_slash) { path_array[i] = NULL; - node = dt_find_node(parent, path_array, + node = dt_find_node(tree->root, path_array, addrcp, sizecp, create); } @@ -965,7 +965,7 @@ *prop_name++ = '\0'; /* Separate path from the property name. */ - dt_node = dt_find_node_by_path(tree->root, path_copy, NULL, + dt_node = dt_find_node_by_path(tree, path_copy, NULL, NULL, create); if (!dt_node) { @@ -995,7 +995,7 @@ struct device_tree_node *reserved; u32 addr = 0, size = 0; - reserved = dt_find_node_by_path(tree->root, "reserved-memory", &addr, + reserved = dt_find_node_by_path(tree, "reserved-memory", &addr, &size, 1); if (!reserved) return NULL; diff --git a/src/soc/cavium/cn81xx/soc.c b/src/soc/cavium/cn81xx/soc.c index 0500749..20542e5 100644 --- a/src/soc/cavium/cn81xx/soc.c +++ b/src/soc/cavium/cn81xx/soc.c @@ -185,7 +185,7 @@ size_t i; /* Set the sclk clock rate. */ - dt_node = dt_find_node_by_path(tree->root, "soc@0/sclk", NULL, NULL, 0); + dt_node = dt_find_node_by_path(tree, "soc@0/sclk", NULL, NULL, 0); if (dt_node) { const u32 freq = thunderx_get_io_clock(); printk(BIOS_INFO, "%s: Set SCLK to %u Hz\n", __func__, freq); @@ -195,7 +195,7 @@ __func__); /* Set refclkuaa clock rate. */ - dt_node = dt_find_node_by_path(tree->root, "soc@0/refclkuaa", NULL, + dt_node = dt_find_node_by_path(tree, "soc@0/refclkuaa", NULL, NULL, 0); if (dt_node) { const u32 freq = uart_platform_refclk(); @@ -212,7 +212,7 @@ const uint64_t addr = UAAx_PF_BAR0(i); /* Remove the node */ snprintf(path, sizeof(path), "soc@0/serial@%llx", addr); - dt_node = dt_find_node_by_path(tree->root, path, NULL, NULL, 0); + dt_node = dt_find_node_by_path(tree, path, NULL, NULL, 0); if (!dt_node || uart_is_enabled(i)) { printk(BIOS_INFO, "%s: ignoring %s\n", __func__, path); continue; @@ -228,7 +228,7 @@ const uint64_t addr = PEM_PEMX_PF_BAR0(i); /* Remove the node */ snprintf(path, sizeof(path), "soc@0/pci@%llx", addr); - dt_node = dt_find_node_by_path(tree->root, path, NULL, NULL, 0); + dt_node = dt_find_node_by_path(tree, path, NULL, NULL, 0); if (!dt_node || bdk_pcie_is_running(0, i)) { printk(BIOS_INFO, "%s: ignoring %s\n", __func__, path); continue; @@ -240,7 +240,7 @@ /* Remove phandle to non existing nodes */ snprintf(path, sizeof(path), "soc@0/smmu0@%llx", SMMU_PF_BAR0); - dt_node = dt_find_node_by_path(tree->root, path, NULL, NULL, 0); + dt_node = dt_find_node_by_path(tree, path, NULL, NULL, 0); if (!dt_node) { printk(BIOS_ERR, "%s: SMMU entry not found\n", __func__); -- To view, visit https://review.coreboot.org/c/coreboot/+/32864 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Iff56be4da2461ae73a7301dcaa315758d2a8c999 Gerrit-Change-Number: 32864 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

3 2

Change in ...coreboot[master]: device_tree: Add phandle caching and lookups
by Julius Werner (Code Review) 04 Jun '19

04 Jun '19

Hello Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/32863 to review the following change. Change subject: device_tree: Add phandle caching and lookups ...................................................................... device_tree: Add phandle caching and lookups This patch caches phandles when unflattening the device tree, so we don't have to look up the phandle property again every time we're trying to find the phandle of a node. This is especially important when supporting phandle lookups, which are also added. In addition we keep track of the highest phandle in the whole tree, which will be important for applying overlays later. With this, dt_get_phandle(node) becomes obsolete because the phandle is already available as a member variable in the node. This patch was adapted from depthcharge's http://crosreview.com/1536385 Change-Id: I9cbd67d1d13e57c25d068b3db18bb75c709d7ebe Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- M src/include/device_tree.h M src/lib/device_tree.c M src/soc/cavium/cn81xx/soc.c 3 files changed, 42 insertions(+), 27 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/32863/1 diff --git a/src/include/device_tree.h b/src/include/device_tree.h index 96902b8..8280dad 100644 --- a/src/include/device_tree.h +++ b/src/include/device_tree.h @@ -47,6 +47,7 @@ #define FDT_TOKEN_END_NODE 2 #define FDT_TOKEN_PROPERTY 3 #define FDT_TOKEN_END 9 +#define FDT_PHANDLE_ILLEGAL 0xdeadbeef struct fdt_property { @@ -71,6 +72,8 @@ struct device_tree_node { const char *name; + uint32_t phandle; + // List of struct device_tree_property-s. struct list_node properties; // List of struct device_tree_nodes. @@ -91,6 +94,7 @@ { const void *header; uint32_t header_size; + uint32_t max_phandle; struct list_node reserve_map; @@ -136,6 +140,8 @@ // represented as an array of strings. struct device_tree_node *dt_find_node(struct device_tree_node *parent, const char **path, u32 *addrcp, u32 *sizecp, int create); +struct device_tree_node *dt_find_node_by_phandle(struct device_tree_node *root, + uint32_t phandle); // Look up or create a node relative to a parent node, through its path // represented as a string of '/' separated node names. struct device_tree_node *dt_find_node_by_path(struct device_tree_node *parent, const char *path, @@ -150,8 +156,6 @@ // Look up a node relative to a parent node, through its property value. struct device_tree_node *dt_find_prop_value(struct device_tree_node *parent, const char *name, void *data, size_t size); -// Return the phandle -uint32_t dt_get_phandle(const struct device_tree_node *node); // Write src into *dest as a 'length'-byte big-endian integer. void dt_write_int(u8 *dest, u64 src, size_t length); // Delete a property diff --git a/src/lib/device_tree.c b/src/lib/device_tree.c index 7a3128e..44eca4e 100644 --- a/src/lib/device_tree.c +++ b/src/lib/device_tree.c @@ -66,6 +66,12 @@ return ALIGN_UP(strlen((char *)ptr) + 1, sizeof(uint32_t)) + 4; } +static int dt_prop_is_phandle(struct device_tree_property *prop) +{ + return !(strcmp("phandle", prop->prop.name) && + strcmp("linux,phandle", prop->prop.name)); +} + /* @@ -156,6 +162,7 @@ */ static int fdt_unflatten_node(const void *blob, uint32_t start_offset, + struct device_tree *tree, struct device_tree_node **new_node) { struct list_node *last; @@ -178,6 +185,12 @@ struct device_tree_property *prop = xzalloc(sizeof(*prop)); prop->prop = fprop; + if (dt_prop_is_phandle(prop)) { + node->phandle = be32dec(prop->prop.data); + if (node->phandle > tree->max_phandle) + tree->max_phandle = node->phandle; + } + list_insert_after(&prop->list_node, last); last = &prop->list_node; @@ -186,7 +199,7 @@ struct device_tree_node *child; last = &node->children; - while ((size = fdt_unflatten_node(blob, offset, &child))) { + while ((size = fdt_unflatten_node(blob, offset, tree, &child))) { list_insert_after(&child->list_node, last); last = &child->list_node; @@ -260,7 +273,7 @@ offset += size; } - fdt_unflatten_node(blob, struct_offset, &tree->root); + fdt_unflatten_node(blob, struct_offset, tree, &tree->root); return tree; } @@ -590,6 +603,26 @@ return node; } +struct device_tree_node *dt_find_node_by_phandle(struct device_tree_node *root, + uint32_t phandle) +{ + if (!root) + return NULL; + + if (root->phandle == phandle) + return root; + + struct device_tree_node *node; + struct device_tree_node *result; + list_for_each(node, root->children, list_node) { + result = dt_find_node_by_phandle(node, phandle); + if (result) + return result; + } + + return NULL; +} + /* * Check if given node is compatible. * @@ -719,28 +752,6 @@ return NULL; } -/** - * Find the phandle of a node. - * - * @param node Pointer to node containing the phandle - * @return Zero on error, the phandle on success - */ -uint32_t dt_get_phandle(const struct device_tree_node *node) -{ - const uint32_t *phandle; - size_t len; - - dt_find_bin_prop(node, "phandle", (const void **)&phandle, &len); - if (phandle != NULL && len == sizeof(*phandle)) - return be32_to_cpu(*phandle); - - dt_find_bin_prop(node, "linux,phandle", (const void **)&phandle, &len); - if (phandle != NULL && len == sizeof(*phandle)) - return be32_to_cpu(*phandle); - - return 0; -} - /* * Write an arbitrary sized big-endian integer into a pointer. * diff --git a/src/soc/cavium/cn81xx/soc.c b/src/soc/cavium/cn81xx/soc.c index 98166b0..0500749 100644 --- a/src/soc/cavium/cn81xx/soc.c +++ b/src/soc/cavium/cn81xx/soc.c @@ -234,7 +234,7 @@ continue; } /* Store the phandle */ - phandle = dt_get_phandle(dt_node); + phandle = dt_node->phandle; printk(BIOS_INFO, "%s: Removing node %s\n", __func__, path); list_remove(&dt_node->list_node); -- To view, visit https://review.coreboot.org/c/coreboot/+/32863 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I9cbd67d1d13e57c25d068b3db18bb75c709d7ebe Gerrit-Change-Number: 32863 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

3 2

Change in ...coreboot[master]: device_tree: Add version checks
by Julius Werner (Code Review) 04 Jun '19

04 Jun '19

Hello Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/32862 to review the following change. Change subject: device_tree: Add version checks ...................................................................... device_tree: Add version checks This patch adds a few more sanity checks to the FDT header parsing to make sure that our code can support the version that is passed in. This patch was adapted from depthcharge's http://crosreview.com/1536384 Change-Id: I06c112f540213c8db7c2455c2e8a4e8e4f337b78 Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- M src/include/device_tree.h M src/lib/device_tree.c M src/lib/fit.c 3 files changed, 23 insertions(+), 6 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/62/32862/1 diff --git a/src/include/device_tree.h b/src/include/device_tree.h index 14be832..96902b8 100644 --- a/src/include/device_tree.h +++ b/src/include/device_tree.h @@ -33,7 +33,7 @@ uint32_t reserve_map_offset; uint32_t version; - uint32_t last_compatible_version; + uint32_t last_comp_version; uint32_t boot_cpuid_phys; @@ -42,6 +42,7 @@ }; #define FDT_HEADER_MAGIC 0xd00dfeed +#define FDT_SUPPORTED_VERSION 17 #define FDT_TOKEN_BEGIN_NODE 1 #define FDT_TOKEN_END_NODE 2 #define FDT_TOKEN_PROPERTY 3 diff --git a/src/lib/device_tree.c b/src/lib/device_tree.c index bb40eee..7a3128e 100644 --- a/src/lib/device_tree.c +++ b/src/lib/device_tree.c @@ -220,6 +220,24 @@ const struct fdt_header *header = (const struct fdt_header *)blob; tree->header = header; + uint32_t magic = be32toh(header->magic); + uint32_t version = be32toh(header->version); + uint32_t last_comp_version = be32toh(header->last_comp_version); + + if (magic != FDT_HEADER_MAGIC) { + printk(BIOS_DEBUG, "Invalid device tree magic %#.8x!\n", magic); + return NULL; + } + if (last_comp_version > FDT_SUPPORTED_VERSION) { + printk(BIOS_DEBUG, "Unsupported device tree version %u(>=%u)\n", + version, last_comp_version); + return NULL; + } + if (version > FDT_SUPPORTED_VERSION) + printk(BIOS_DEBUG, + "NOTE: FDT version %u too new, should add support!\n", + version); + uint32_t struct_offset = be32toh(header->structure_offset); uint32_t strings_offset = be32toh(header->strings_offset); uint32_t reserve_offset = be32toh(header->reserve_map_offset); diff --git a/src/lib/fit.c b/src/lib/fit.c index d15641d..c98ba2f 100644 --- a/src/lib/fit.c +++ b/src/lib/fit.c @@ -423,19 +423,17 @@ struct fit_config_node *fit_load(void *fit) { - struct fdt_header *header = (struct fdt_header *)fit; struct fit_image_node *image; struct fit_config_node *config; struct compat_string_entry *compat_node; printk(BIOS_DEBUG, "FIT: Loading FIT from %p\n", fit); - if (be32toh(header->magic) != FDT_HEADER_MAGIC) { - printk(BIOS_ERR, "FIT: Bad header magic value 0x%08x.\n", - be32toh(header->magic)); + struct device_tree *tree = fdt_unflatten(fit); + if (!tree) { + printk(BIOS_ERR, "ERROR: Failed to unflatten FIT image!\n"); return NULL; } - struct device_tree *tree = fdt_unflatten(fit); const char *default_config_name = NULL; struct fit_config_node *default_config = NULL; -- To view, visit https://review.coreboot.org/c/coreboot/+/32862 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I06c112f540213c8db7c2455c2e8a4e8e4f337b78 Gerrit-Change-Number: 32862 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

4 5

Jump to page:

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit June 2019