coreboot-gerrit May 2019

coreboot-gerrit@coreboot.org

1 participants
1485 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]: 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: 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

Change in ...coreboot[master]: device_tree: Switch allocations to xzalloc()
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/+/32861 to review the following change. Change subject: device_tree: Switch allocations to xzalloc() ...................................................................... device_tree: Switch allocations to xzalloc() The FIT code is already using xzalloc() everywhere, and that's the only real consumer of device tree code right now. Chances are if you're trying to unflatten an FDT and it doesn't fit into the heap you're pretty much screwed anyway, so all the OOM handling feels a bit unnecessary (and some functions will just silently fail because they don't have a return value, which is bad). Let's just switch this all to die on failed allocations. Change-Id: I738f24d550a776653b2becd3d4f7d4d2cb3cc048 Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- M src/lib/device_tree.c M src/lib/fit.c 2 files changed, 11 insertions(+), 46 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/61/32861/1 diff --git a/src/lib/device_tree.c b/src/lib/device_tree.c index 8dbc510..bb40eee 100644 --- a/src/lib/device_tree.c +++ b/src/lib/device_tree.c @@ -16,6 +16,7 @@ */ #include <assert.h> +#include <commonlib/stdlib.h> #include <console/console.h> #include <device_tree.h> #include <endian.h> @@ -153,24 +154,6 @@ /* * Functions to turn a flattened tree into an unflattened one. */ -static struct device_tree_node *alloc_node(void) -{ - struct device_tree_node *buf = malloc(sizeof(struct device_tree_node)); - if (!buf) - return NULL; - memset(buf, 0, sizeof(*buf)); - return buf; -} - -static struct device_tree_property *alloc_prop(void) -{ - struct device_tree_property *buf = - malloc(sizeof(struct device_tree_property)); - if (!buf) - return NULL; - memset(buf, 0, sizeof(*buf)); - return buf; -} static int fdt_unflatten_node(const void *blob, uint32_t start_offset, struct device_tree_node **new_node) @@ -185,18 +168,14 @@ return 0; offset += size; - struct device_tree_node *node = alloc_node(); + struct device_tree_node *node = xzalloc(sizeof(*node)); *new_node = node; - if (!node) - return 0; node->name = name; struct fdt_property fprop; last = &node->properties; while ((size = fdt_next_property(blob, offset, &fprop))) { - struct device_tree_property *prop = alloc_prop(); - if (!prop) - return 0; + struct device_tree_property *prop = xzalloc(sizeof(*prop)); prop->prop = fprop; list_insert_after(&prop->list_node, last); @@ -227,10 +206,7 @@ if (!size) return 0; - struct device_tree_reserve_map_entry *entry = malloc(sizeof(*entry)); - if (!entry) - return 0; - memset(entry, 0, sizeof(*entry)); + struct device_tree_reserve_map_entry *entry = xzalloc(sizeof(*entry)); *new = entry; entry->start = start; entry->size = size; @@ -240,11 +216,8 @@ struct device_tree *fdt_unflatten(const void *blob) { - struct device_tree *tree = malloc(sizeof(*tree)); + struct device_tree *tree = xzalloc(sizeof(*tree)); const struct fdt_header *header = (const struct fdt_header *)blob; - if (!tree) - return NULL; - memset(tree, 0, sizeof(*tree)); tree->header = header; uint32_t struct_offset = be32toh(header->structure_offset); @@ -534,7 +507,7 @@ if (!create) return NULL; - found = alloc_node(); + found = malloc(sizeof(*found)); if (!found) return NULL; found->name = strdup(*path); @@ -804,9 +777,7 @@ } } - prop = alloc_prop(); - if (!prop) - return; + prop = xzalloc(sizeof(*prop)); list_insert_after(&prop->list_node, &node->properties); prop->prop.name = name; prop->prop.data = data; @@ -878,9 +849,7 @@ */ void dt_add_u32_prop(struct device_tree_node *node, const char *name, u32 val) { - u32 *val_ptr = malloc(sizeof(val)); - if (!val_ptr) - return; + u32 *val_ptr = xmalloc(sizeof(val)); *val_ptr = htobe32(val); dt_add_bin_prop(node, name, val_ptr, sizeof(*val_ptr)); } @@ -894,9 +863,7 @@ */ void dt_add_u64_prop(struct device_tree_node *node, const char *name, u64 val) { - u64 *val_ptr = malloc(sizeof(val)); - if (!val_ptr) - return; + u64 *val_ptr = xmalloc(sizeof(val)); *val_ptr = htobe64(val); dt_add_bin_prop(node, name, val_ptr, sizeof(*val_ptr)); } @@ -916,9 +883,7 @@ { int i; size_t length = (addr_cells + size_cells) * sizeof(u32) * count; - u8 *data = malloc(length); - if (!data) - return; + u8 *data = xmalloc(length); u8 *cur = data; for (i = 0; i < count; i++) { diff --git a/src/lib/fit.c b/src/lib/fit.c index 3aad806..d15641d 100644 --- a/src/lib/fit.c +++ b/src/lib/fit.c @@ -27,7 +27,7 @@ #include <fit.h> #include <boardid.h> #include <commonlib/cbfs_serialized.h> -#include <commonlib/include/commonlib/stdlib.h> +#include <commonlib/stdlib.h> static struct list_node image_nodes; static struct list_node config_nodes; -- To view, visit https://review.coreboot.org/c/coreboot/+/32861 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I738f24d550a776653b2becd3d4f7d4d2cb3cc048 Gerrit-Change-Number: 32861 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

3 6

Change in ...coreboot[master]: soc/intel/cannonlake: Dump ME status info before notify EndOfFirmware
by Bora Guvendik (Code Review) 04 Jun '19

04 Jun '19

Bora Guvendik has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32991 Change subject: soc/intel/cannonlake: Dump ME status info before notify EndOfFirmware ...................................................................... soc/intel/cannonlake: Dump ME status info before notify EndOfFirmware Dumping ME status displays wrong information if we disable Heci1 because it is called after fsp notifies EndOfFirmware and disables Heci1. This patch moves the ME status dump before fsp notify EndOfFirmware. TEST=Boot to OS, check ME dump information Change-Id: Ifd8b18a41c502c4ecfb84698a7669028394589fd Signed-off-by: Bora Guvendik <bora.guvendik(a)intel.com> --- M src/soc/intel/cannonlake/finalize.c M src/soc/intel/cannonlake/include/soc/me.h M src/soc/intel/cannonlake/me.c 3 files changed, 5 insertions(+), 5 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/91/32991/1 diff --git a/src/soc/intel/cannonlake/finalize.c b/src/soc/intel/cannonlake/finalize.c index eb4c5c2..4dfd15b 100644 --- a/src/soc/intel/cannonlake/finalize.c +++ b/src/soc/intel/cannonlake/finalize.c @@ -26,7 +26,6 @@ #include <intelblocks/tco.h> #include <reg_script.h> #include <spi-generic.h> -#include <soc/me.h> #include <soc/p2sb.h> #include <soc/pci_devs.h> #include <soc/pcr_ids.h> @@ -95,8 +94,6 @@ { printk(BIOS_DEBUG, "Finalizing chipset.\n"); - dump_me_status(); - pch_finalize(); printk(BIOS_DEBUG, "Finalizing SMM.\n"); diff --git a/src/soc/intel/cannonlake/include/soc/me.h b/src/soc/intel/cannonlake/include/soc/me.h index 1d782c1..9084209 100644 --- a/src/soc/intel/cannonlake/include/soc/me.h +++ b/src/soc/intel/cannonlake/include/soc/me.h @@ -16,6 +16,6 @@ #ifndef _CANNONLAKE_ME_H_ #define _CANNONLAKE_ME_H_ -void dump_me_status(void); +void dump_me_status(void* unused); #endif /* _CANNONLAKE_ME_H_ */ diff --git a/src/soc/intel/cannonlake/me.c b/src/soc/intel/cannonlake/me.c index 3fedc63..55afbe3 100644 --- a/src/soc/intel/cannonlake/me.c +++ b/src/soc/intel/cannonlake/me.c @@ -235,7 +235,7 @@ } BOOT_STATE_INIT_ENTRY(BS_DEV_ENABLE, BS_ON_EXIT, print_me_version, NULL); -void dump_me_status(void) +void dump_me_status(void* unused) { union hfsts1 hfsts1; union hfsts2 hfsts2; @@ -297,3 +297,6 @@ printk(BIOS_DEBUG, "ME: TXT Support : %s\n", hfsts6.fields.txt_support ? "YES" : "NO"); } + +BOOT_STATE_INIT_ENTRY(BS_PAYLOAD_LOAD, BS_ON_ENTRY, dump_me_status, NULL); +BOOT_STATE_INIT_ENTRY(BS_OS_RESUME_CHECK, BS_ON_EXIT, dump_me_status, NULL); -- To view, visit https://review.coreboot.org/c/coreboot/+/32991 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifd8b18a41c502c4ecfb84698a7669028394589fd Gerrit-Change-Number: 32991 Gerrit-PatchSet: 1 Gerrit-Owner: Bora Guvendik <bora.guvendik(a)intel.com> Gerrit-MessageType: newchange

6 7

Change in ...coreboot[master]: libpayload: Reset PS/2 keyboard
by Paul Menzel (Code Review) 04 Jun '19

04 Jun '19

Paul Menzel has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32951 Change subject: libpayload: Reset PS/2 keyboard ...................................................................... libpayload: Reset PS/2 keyboard Loading a libpayload based payload like coreinfo or FILO from SeaBIOS pressing keys does not give the expected results. For example, pressing F1 gives the character 24 translated to scan code 6a. ESC for example 43 (111). The problem is not reproducible using the payload directly, that means without SeaBIOS. The problem seems to be, that SeaBIOS already initializes the PS/2 controller and AT keyboard. Comparing it with coreboot’s PS/2 keyboard code, the keyboard needs to be reset. That seems to fix the issue, when the keyboard was initialized before. TEST=Build coreboot for QEMU Q35 with SeaBIOS, and coreinfo as secondary payload. Run qemu-system-i386 -M q35 -L /dev/shm -bios build/coreboot.rom -serial stdio press 3 to select the coreinfo payload, and verify that the keys F1 and F2 are working. Change-Id: I2732292ac316d4bc0029ecb5c95fa7d1e7d68947 Signed-off-by: Paul Menzel <pmenzel(a)molgen.mpg.de> --- M payloads/libpayload/drivers/i8042/i8042.h M payloads/libpayload/drivers/i8042/keyboard.c 2 files changed, 8 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/51/32951/1 diff --git a/payloads/libpayload/drivers/i8042/i8042.h b/payloads/libpayload/drivers/i8042/i8042.h index 643167e..e864ac9 100644 --- a/payloads/libpayload/drivers/i8042/i8042.h +++ b/payloads/libpayload/drivers/i8042/i8042.h @@ -63,6 +63,7 @@ #define I8042_KBCMD_EN 0xf4 #define I8042_KBCMD_DEFAULT_DIS 0xf5 #define I8042_KBCMD_SET_DEFAULT 0xf6 +#define I8042_KBCMD_ACK 0xfa #define I8042_KBCMD_RESEND 0xfe #define I8042_KBCMD_RESET 0xff diff --git a/payloads/libpayload/drivers/i8042/keyboard.c b/payloads/libpayload/drivers/i8042/keyboard.c index cded638..fea9e71 100644 --- a/payloads/libpayload/drivers/i8042/keyboard.c +++ b/payloads/libpayload/drivers/i8042/keyboard.c @@ -317,6 +317,13 @@ /* Enable first PS/2 port */ i8042_cmd(I8042_CMD_EN_KB); + /* Reset keyboard and self test (keyboard side) */ + ret = keyboard_cmd(I8042_KBCMD_RESET); + if (ret != I8042_KBCMD_ACK) { + printf("ERROR: Keyboard reset failed ACK: 0x%x\n", ret); + return; + } + /* Set scancode set 1 */ ret = keyboard_cmd(I8042_KBCMD_SET_SCANCODE); if (!ret && !CONFIG(LP_PC_KEYBOARD_IGNORE_INIT_FAILURE)) -- To view, visit https://review.coreboot.org/c/coreboot/+/32951 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I2732292ac316d4bc0029ecb5c95fa7d1e7d68947 Gerrit-Change-Number: 32951 Gerrit-PatchSet: 1 Gerrit-Owner: Paul Menzel <paulepanter(a)users.sourceforge.net> Gerrit-MessageType: newchange

4 7

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2019