coreboot-gerrit May 2019

coreboot-gerrit@coreboot.org

1 participants
1485 discussions

Change in ...coreboot[master]: fit: Check all compat strings for highest match
by Julius Werner (Code Review) 21 Jun '19

21 Jun '19

Hello Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/32874 to review the following change. Change subject: fit: Check all compat strings for highest match ...................................................................... fit: Check all compat strings for highest match The compat string matching code was mostly copied from depthcharge. One of the few differences is that we now store the list of compat strings we're willing to match in a list rather than an array. Since our lists insert at the front, that means the strings are now ordered lowest to highest (not highest to lowest like in depthcharge). We did rewrite the compat_rank matching code to accomodate for that... however, what we didn't do is remove the break-statement in the loop that matches all compat strings. When we search the lowest priority first, we can't abort the search as soon as we found a match -- we have to keep looking because we might find a higher priority match later. This patch fixes the issue so that my Kevin can actually match for google,kevin-rev5 (and doesn't just jump at the first best google,kevin match). Change-Id: Ibe3d84bbce6de3cd49c746a667ae1ccfdc843105 Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- M src/lib/fit.c 1 file changed, 0 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/74/32874/1 diff --git a/src/lib/fit.c b/src/lib/fit.c index 1b02c6a..0d3cb2d 100644 --- a/src/lib/fit.c +++ b/src/lib/fit.c @@ -463,7 +463,6 @@ config->compat_rank = i; config->compat_string = compat_node->compat_string; - break; } i++; } -- To view, visit https://review.coreboot.org/c/coreboot/+/32874 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibe3d84bbce6de3cd49c746a667ae1ccfdc843105 Gerrit-Change-Number: 32874 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]: fit: Add overlay support
by Julius Werner (Code Review) 21 Jun '19

21 Jun '19

Hello Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/32873 to review the following change. Change subject: fit: Add overlay support ...................................................................... fit: Add overlay support This patch adds support to boot FIT image configurations consisting of a base device tree and one or more overlays. Since extracting the right compatible string from overlay FDTs is problematic, we'll only support this for FIT images that have the compatible string pulled out into the config node. This patch was adapted from depthcharge's http://crosreview.com/1555293 Change-Id: I0943f9a1869c9e416887c7ff16e33f7d91b74989 Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- M src/include/fit.h M src/lib/fit.c M src/lib/fit_payload.c 3 files changed, 54 insertions(+), 1 deletion(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/73/32873/1 diff --git a/src/include/fit.h b/src/include/fit.h index 758ee70..a5a9e69 100644 --- a/src/include/fit.h +++ b/src/include/fit.h @@ -39,6 +39,7 @@ const char *name; struct fit_image_node *kernel; struct fit_image_node *fdt; + struct list_node overlays; struct fit_image_node *ramdisk; struct fdt_property compat; int compat_rank; @@ -48,6 +49,12 @@ struct list_node list_node; }; +struct fit_overlay_chain +{ + struct fit_image_node *overlay; + struct list_node list_node; +}; + /* * Updates the cmdline in the devicetree. */ diff --git a/src/lib/fit.c b/src/lib/fit.c index 464be3a..1b02c6a 100644 --- a/src/lib/fit.c +++ b/src/lib/fit.c @@ -96,6 +96,31 @@ return NULL; } +static struct fit_image_node *find_image_with_overlays(const char *name, + int bytes, struct list_node *prev) +{ + struct fit_image_node *base = find_image(name); + if (!base) + return NULL; + + int len = strnlen(name, bytes) + 1; + bytes -= len; + name += len; + while (bytes > 0) { + struct fit_overlay_chain *next = xzalloc(sizeof(*next)); + next->overlay = find_image(name); + if (!next->overlay) + return NULL; + list_insert_after(&next->list_node, prev); + prev = &next->list_node; + len = strnlen(name, bytes) + 1; + bytes -= len; + name += len; + } + + return base; +} + static void image_node(struct device_tree_node *node) { struct fit_image_node *image = xzalloc(sizeof(*image)); @@ -133,7 +158,8 @@ if (!strcmp("kernel", prop->prop.name)) config->kernel = find_image(prop->prop.data); else if (!strcmp("fdt", prop->prop.name)) - config->fdt = find_image(prop->prop.data); + config->fdt = find_image_with_overlays(prop->prop.data, + prop->prop.size, &config->overlays); else if (!strcmp("ramdisk", prop->prop.name)) config->ramdisk = find_image(prop->prop.data); else if (!strcmp("compatible", prop->prop.name)) @@ -408,6 +434,14 @@ return -1; } + // FDT overlays are not supported in legacy FIT images. + if (config->overlays.next) { + printk(BIOS_ERR, + "ERROR: config %s has overlay but no compat!\n", + config->name); + return -1; + } + if (fdt_find_compat(fdt_blob, fdt_offset, &config->compat)) { printk(BIOS_ERR, "ERROR: Can't find compat string in FDT %s " @@ -442,6 +476,7 @@ struct fit_image_node *image; struct fit_config_node *config; struct compat_string_entry *compat_node; + struct fit_overlay_chain *overlay_chain; printk(BIOS_DEBUG, "FIT: Loading FIT from %p\n", fit); @@ -505,6 +540,8 @@ } printk(BIOS_DEBUG, ", kernel %s", config->kernel->name); printk(BIOS_DEBUG, ", fdt %s", config->fdt->name); + list_for_each(overlay_chain, config->overlays, list_node) + printk(BIOS_DEBUG, " %s", overlay_chain->overlay->name); if (config->ramdisk) printk(BIOS_DEBUG, ", ramdisk %s", config->ramdisk->name); if (config->compat.name) { diff --git a/src/lib/fit_payload.c b/src/lib/fit_payload.c index b049188..8e75915 100644 --- a/src/lib/fit_payload.c +++ b/src/lib/fit_payload.c @@ -206,6 +206,15 @@ return; } + struct fit_overlay_chain *chain; + list_for_each(chain, config->overlays, list_node) { + struct device_tree *overlay = unpack_fdt(chain->overlay); + if (!overlay || dt_apply_overlay(dt, overlay)) { + printk(BIOS_ERR, "ERROR: Failed to apply overlay %s!\n", + chain->overlay->name); + } + } + dt_apply_fixups(dt); /* Insert coreboot specific information */ -- To view, visit https://review.coreboot.org/c/coreboot/+/32873 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I0943f9a1869c9e416887c7ff16e33f7d91b74989 Gerrit-Change-Number: 32873 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

4 4

Change in ...coreboot[master]: fit: Add device tree compression
by Julius Werner (Code Review) 21 Jun '19

21 Jun '19

Hello Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/32872 to review the following change. Change subject: fit: Add device tree compression ...................................................................... fit: Add device tree compression This patch adds support for compressing individual device trees in the FIT image. In order to make this efficient, we'll have to pull the compatible property out of the FDT and store it directly in the config node of the FIT image, so that we don't have to scan (and therefore decompress) every single FDT on boot. Device tree compression is only supported for FIT images that have this external compatible property. For older images with no compression, we still support fallback to scanning the FDT for the property. This patch was adapted from depthcharge's http://crosreview.com/1553458 Change-Id: Ifcb6997782c480c8ef6692df17b66ad96264e623 Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- M src/lib/fit.c M src/lib/fit_payload.c 2 files changed, 55 insertions(+), 25 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/72/32872/1 diff --git a/src/lib/fit.c b/src/lib/fit.c index aa3fee7..464be3a 100644 --- a/src/lib/fit.c +++ b/src/lib/fit.c @@ -136,6 +136,8 @@ config->fdt = find_image(prop->prop.data); else if (!strcmp("ramdisk", prop->prop.name)) config->ramdisk = find_image(prop->prop.data); + else if (!strcmp("compatible", prop->prop.name)) + config->compat = prop->prop; } list_insert_after(&config->list_node, &config_nodes); @@ -391,35 +393,45 @@ */ static int fit_update_compat(struct fit_config_node *config) { - if (config->fdt->compression != CBFS_COMPRESS_NONE) { - printk(BIOS_ERR, - "FDT compression not yet supported, skipping %s.\n", - config->name); - return -1; - } + // If there was no "compatible" property in config node, this is a + // legacy FIT image. Must extract compat prop from FDT itself. + if (!config->compat.name) { + void *fdt_blob = config->fdt->data; + const struct fdt_header *fdt_header = fdt_blob; + uint32_t fdt_offset = be32_to_cpu(fdt_header->structure_offset); - void *fdt_blob = config->fdt->data; - struct compat_string_entry *compat_node; - const struct fdt_header *fdt_header = - (const struct fdt_header *)fdt_blob; - uint32_t fdt_offset = be32_to_cpu(fdt_header->structure_offset); - size_t i = 0; + if (config->fdt->compression != CBFS_COMPRESS_NONE) { + printk(BIOS_ERR, + "ERROR: config %s has a compressed FDT without " + "external compatible property, skipping.\n", + config->name); + return -1; + } + + if (fdt_find_compat(fdt_blob, fdt_offset, &config->compat)) { + printk(BIOS_ERR, + "ERROR: Can't find compat string in FDT %s " + "for config %s, skipping.\n", + config->fdt->name, config->name); + return -1; + } + } config->compat_pos = -1; config->compat_rank = -1; - if (!fdt_find_compat(fdt_blob, fdt_offset, &config->compat)) { - list_for_each(compat_node, compat_strings, list_node) { - int pos = fit_check_compat(&config->compat, - compat_node->compat_string); - if (pos >= 0) { - config->compat_pos = pos; - config->compat_rank = i; - config->compat_string = - compat_node->compat_string; - break; - } - i++; + size_t i = 0; + struct compat_string_entry *compat_node; + list_for_each(compat_node, compat_strings, list_node) { + int pos = fit_check_compat(&config->compat, + compat_node->compat_string); + if (pos >= 0) { + config->compat_pos = pos; + config->compat_rank = i; + config->compat_string = + compat_node->compat_string; + break; } + i++; } return 0; diff --git a/src/lib/fit_payload.c b/src/lib/fit_payload.c index c977903..b049188 100644 --- a/src/lib/fit_payload.c +++ b/src/lib/fit_payload.c @@ -96,6 +96,24 @@ return false; } +static struct device_tree *unpack_fdt(struct fit_image_node *image_node) +{ + void *data = image_node->data; + + if (image_node->compression != CBFS_COMPRESS_NONE) { + /* TODO: This is an ugly heuristic for how much the size will + expand on decompression, fix once FIT images support storing + the real uncompressed size. */ + struct region r = { .offset = 0, .size = image_node->size * 5 }; + data = malloc(r.size); + r.offset = (uintptr_t)data; + if (!data || extract(&r, image_node)) + return NULL; + } + + return fdt_unflatten(data); +} + /** * Add coreboot tables, CBMEM information and optional board specific strapping * IDs to the device tree loaded via FIT. @@ -181,7 +199,7 @@ return; } - dt = fdt_unflatten(config->fdt->data); + dt = unpack_fdt(config->fdt); if (!dt) { printk(BIOS_ERR, "ERROR: Failed to unflatten the FDT.\n"); rdev_munmap(prog_rdev(payload), data); -- To view, visit https://review.coreboot.org/c/coreboot/+/32872 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ifcb6997782c480c8ef6692df17b66ad96264e623 Gerrit-Change-Number: 32872 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]: fit_payload: Don't call prog_segment_loaded() on extracted images
by Julius Werner (Code Review) 21 Jun '19

21 Jun '19

Hello Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/32871 to review the following change. Change subject: fit_payload: Don't call prog_segment_loaded() on extracted images ...................................................................... fit_payload: Don't call prog_segment_loaded() on extracted images Kernel handoff on all architectures supporting FIT images already includes flushing and disabling the cache. No need to waste any more time flushing individual components (especially since in the case of compressed DT overlays they will still get accessed again afterwards). Change-Id: I7b483e920c5a71663b024b5b50804ffc84939830 Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- M src/lib/fit_payload.c 1 file changed, 2 insertions(+), 3 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/71/32871/1 diff --git a/src/lib/fit_payload.c b/src/lib/fit_payload.c index 4bc6622..c977903 100644 --- a/src/lib/fit_payload.c +++ b/src/lib/fit_payload.c @@ -88,12 +88,11 @@ } if (!true_size) { - printk(BIOS_ERR, "ERROR: %s node failed!\n", comp_name); + printk(BIOS_ERR, "ERROR: %s decompression failed!\n", + comp_name); return true; } - prog_segment_loaded(region->offset, true_size, 0); - return false; } -- To view, visit https://review.coreboot.org/c/coreboot/+/32871 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I7b483e920c5a71663b024b5b50804ffc84939830 Gerrit-Change-Number: 32871 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]: fit: Refactor config node handling
by Julius Werner (Code Review) 21 Jun '19

21 Jun '19

Hello Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/32870 to review the following change. Change subject: fit: Refactor config node handling ...................................................................... fit: Refactor config node handling This patch makes some minor refactoring to the way the FIT parser handles config nodes. A lot of this code was written in the dawn age of depthcharge when its device tree library wasn't as well-stocked yet, so some of it can be rewritten nicer with more high-level primitives. There's no point in storing both the string name and the actual FDT node of a FIT image node separately, since the latter also contains the former, so remove that. Also eliminate code for the case of not having an FDT (which makes no sense), and move some more FDT validity/compat checking into fit_update_compat() (mostly in anticipation of later changes). This patch was adapted from depthcharge's http://crosreview.com/1553456 with a couple of modifications specific to coreboot's custom FIT loading code. Change-Id: Ia79e0fd0e1159c4aca64c453b82a0379b133350d Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- M src/arch/arm64/fit_payload.c M src/include/fit.h M src/lib/fit.c M src/lib/fit_payload.c 4 files changed, 92 insertions(+), 117 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/70/32870/1 diff --git a/src/arch/arm64/fit_payload.c b/src/arch/arm64/fit_payload.c index c4bbcee..fd1bae1 100644 --- a/src/arch/arm64/fit_payload.c +++ b/src/arch/arm64/fit_payload.c @@ -184,20 +184,14 @@ bool place_anywhere; void *arg = NULL; - if (!config->fdt || !fdt) { - printk(BIOS_CRIT, "CRIT: Providing a valid FDT is mandatory to " - "boot an ARM64 kernel!\n"); - return false; - } - - if (!decompress_kernel_header(config->kernel_node)) { + if (!decompress_kernel_header(config->kernel)) { printk(BIOS_CRIT, "CRIT: Payload doesn't look like an ARM64" " kernel Image.\n"); return false; } /* Update kernel size from image header, if possible */ - kernel->size = get_kernel_size(config->kernel_node); + kernel->size = get_kernel_size(config->kernel); printk(BIOS_DEBUG, "FIT: Using kernel size of 0x%zx bytes\n", kernel->size); diff --git a/src/include/fit.h b/src/include/fit.h index 6e0667f..758ee70 100644 --- a/src/include/fit.h +++ b/src/include/fit.h @@ -27,7 +27,7 @@ struct fit_image_node { const char *name; - const void *data; + void *data; uint32_t size; int compression; @@ -37,12 +37,9 @@ struct fit_config_node { const char *name; - const char *kernel; - struct fit_image_node *kernel_node; - const char *fdt; - struct fit_image_node *fdt_node; - const char *ramdisk; - struct fit_image_node *ramdisk_node; + struct fit_image_node *kernel; + struct fit_image_node *fdt; + struct fit_image_node *ramdisk; struct fdt_property compat; int compat_rank; int compat_pos; diff --git a/src/lib/fit.c b/src/lib/fit.c index c98ba2f..aa3fee7 100644 --- a/src/lib/fit.c +++ b/src/lib/fit.c @@ -85,6 +85,17 @@ fit_add_compat_string(compat_string); } +static struct fit_image_node *find_image(const char *name) +{ + struct fit_image_node *image; + list_for_each(image, image_nodes, list_node) { + if (!strcmp(image->name, name)) + return image; + } + printk(BIOS_ERR, "ERROR: Cannot find image node %s!\n", name); + return NULL; +} + static void image_node(struct device_tree_node *node) { struct fit_image_node *image = xzalloc(sizeof(*image)); @@ -120,11 +131,11 @@ struct device_tree_property *prop; list_for_each(prop, node->properties, list_node) { if (!strcmp("kernel", prop->prop.name)) - config->kernel = prop->prop.data; + config->kernel = find_image(prop->prop.data); else if (!strcmp("fdt", prop->prop.name)) - config->fdt = prop->prop.data; + config->fdt = find_image(prop->prop.data); else if (!strcmp("ramdisk", prop->prop.name)) - config->ramdisk = prop->prop.data; + config->ramdisk = find_image(prop->prop.data); } list_insert_after(&config->list_node, &config_nodes); @@ -132,40 +143,22 @@ static void fit_unpack(struct device_tree *tree, const char **default_config) { - assert(tree && tree->root); + struct device_tree_node *child; + struct device_tree_node *images = dt_find_node_by_path(tree, "/images", + NULL, NULL, 0); + if (images) + list_for_each(child, images->children, list_node) + image_node(child); - struct device_tree_node *top; - list_for_each(top, tree->root->children, list_node) { - struct device_tree_node *child; - if (!strcmp("images", top->name)) { - - list_for_each(child, top->children, list_node) - image_node(child); - - } else if (!strcmp("configurations", top->name)) { - struct device_tree_property *prop; - list_for_each(prop, top->properties, list_node) { - if (!strcmp("default", prop->prop.name) && - default_config) - *default_config = prop->prop.data; - } - - list_for_each(child, top->children, list_node) - config_node(child); - } + struct device_tree_node *configs = dt_find_node_by_path(tree, + "/configurations", NULL, NULL, 0); + if (configs) { + *default_config = dt_find_string_prop(configs, "default"); + list_for_each(child, configs->children, list_node) + config_node(child); } } -static struct fit_image_node *find_image(const char *name) -{ - struct fit_image_node *image; - list_for_each(image, image_nodes, list_node) { - if (!strcmp(image->name, name)) - return image; - } - return NULL; -} - static int fdt_find_compat(const void *blob, uint32_t start_offset, struct fdt_property *prop) { @@ -393,18 +386,27 @@ /* * Finds a compat string and updates the compat position and rank. - * @param fdt_blob Pointer to FDT * @param config The current config node to operate on + * @return 0 if compat updated, -1 if this FDT cannot be used. */ -static void fit_update_compat(const void *fdt_blob, - struct fit_config_node *config) +static int fit_update_compat(struct fit_config_node *config) { + if (config->fdt->compression != CBFS_COMPRESS_NONE) { + printk(BIOS_ERR, + "FDT compression not yet supported, skipping %s.\n", + config->name); + return -1; + } + + void *fdt_blob = config->fdt->data; struct compat_string_entry *compat_node; const struct fdt_header *fdt_header = (const struct fdt_header *)fdt_blob; uint32_t fdt_offset = be32_to_cpu(fdt_header->structure_offset); size_t i = 0; + config->compat_pos = -1; + config->compat_rank = -1; if (!fdt_find_compat(fdt_blob, fdt_offset, &config->compat)) { list_for_each(compat_node, compat_strings, list_node) { int pos = fit_check_compat(&config->compat, @@ -419,6 +421,8 @@ i++; } } + + return 0; } struct fit_config_node *fit_load(void *fit) @@ -457,55 +461,40 @@ printk(BIOS_DEBUG, "\n"); /* Process and list the configs. */ list_for_each(config, config_nodes, list_node) { - if (config->kernel) - config->kernel_node = find_image(config->kernel); - if (config->fdt) - config->fdt_node = find_image(config->fdt); - if (config->ramdisk) - config->ramdisk_node = find_image(config->ramdisk); + if (!config->kernel) { + printk(BIOS_ERR, + "ERROR: config %s has no kernel, skipping.\n", + config->name); + continue; + } + if (!config->fdt) { + printk(BIOS_ERR, + "ERROR: config %s has no FDT, skipping.\n", + config->name); + continue; + } - if (config->ramdisk_node && - config->ramdisk_node->compression < 0) { + if (config->ramdisk && + config->ramdisk->compression < 0) { printk(BIOS_WARNING, "WARN: Ramdisk is compressed with " "an unsupported algorithm, discarding config %s." "\n", config->name); - list_remove(&config->list_node); continue; } - if (!config->kernel_node || - (config->fdt && !config->fdt_node)) { - printk(BIOS_DEBUG, "FIT: Missing image, discarding " - "config %s.\n", config->name); - list_remove(&config->list_node); + if (fit_update_compat(config)) continue; - } - if (config->fdt_node) { - if (config->fdt_node->compression != - CBFS_COMPRESS_NONE) { - printk(BIOS_DEBUG, - "FIT: FDT compression not yet supported," - " skipping config %s.\n", config->name); - list_remove(&config->list_node); - continue; - } - - config->compat_pos = -1; - config->compat_rank = -1; - - fit_update_compat(config->fdt_node->data, config); - } printk(BIOS_DEBUG, "FIT: config %s", config->name); if (default_config_name && !strcmp(config->name, default_config_name)) { printk(BIOS_DEBUG, " (default)"); default_config = config; } - if (config->fdt) - printk(BIOS_DEBUG, ", fdt %s", config->fdt); - if (config->ramdisk) - printk(BIOS_DEBUG, ", ramdisk %s", config->ramdisk); + printk(BIOS_DEBUG, ", kernel %s", config->kernel->name); + printk(BIOS_DEBUG, ", fdt %s", config->fdt->name); + if (config->ramdisk) printk(BIOS_DEBUG, + ", ramdisk %s", config->ramdisk->name); if (config->compat.name) { printk(BIOS_DEBUG, ", compat"); int bytes = config->compat.size; diff --git a/src/lib/fit_payload.c b/src/lib/fit_payload.c index e0158f2..4bc6622 100644 --- a/src/lib/fit_payload.c +++ b/src/lib/fit_payload.c @@ -176,37 +176,34 @@ struct fit_config_node *config = fit_load(data); - if (!config || !config->kernel_node) { + if (!config) { printk(BIOS_ERR, "ERROR: Could not load FIT\n"); rdev_munmap(prog_rdev(payload), data); return; } - if (config->fdt_node) { - dt = fdt_unflatten(config->fdt_node->data); - if (!dt) { - printk(BIOS_ERR, - "ERROR: Failed to unflatten the FDT.\n"); - rdev_munmap(prog_rdev(payload), data); - return; - } - - dt_apply_fixups(dt); - - /* Insert coreboot specific information */ - add_cb_fdt_data(dt); - - /* Update device_tree */ -#if defined(CONFIG_LINUX_COMMAND_LINE) - fit_update_chosen(dt, (char *)CONFIG_LINUX_COMMAND_LINE); -#endif - fit_update_memory(dt); + dt = fdt_unflatten(config->fdt->data); + if (!dt) { + printk(BIOS_ERR, "ERROR: Failed to unflatten the FDT.\n"); + rdev_munmap(prog_rdev(payload), data); + return; } + dt_apply_fixups(dt); + + /* Insert coreboot specific information */ + add_cb_fdt_data(dt); + + /* Update device_tree */ +#if defined(CONFIG_LINUX_COMMAND_LINE) + fit_update_chosen(dt, (char *)CONFIG_LINUX_COMMAND_LINE); +#endif + fit_update_memory(dt); + /* Collect infos for fit_payload_arch */ - kernel.size = config->kernel_node->size; + kernel.size = config->kernel->size; fdt.size = dt ? dt_flat_size(dt) : 0; - initrd.size = config->ramdisk_node ? config->ramdisk_node->size : 0; + initrd.size = config->ramdisk ? config->ramdisk->size : 0; /* Invoke arch specific payload placement and fixups */ if (!fit_payload_arch(payload, config, &kernel, &fdt, &initrd)) { @@ -216,17 +213,15 @@ return; } - /* Load the images to given position */ - if (config->fdt_node) { - /* Update device_tree */ - if (config->ramdisk_node) - fit_add_ramdisk(dt, (void *)initrd.offset, initrd.size); + /* Update ramdisk location in FDT */ + if (config->ramdisk) + fit_add_ramdisk(dt, (void *)initrd.offset, initrd.size); - pack_fdt(&fdt, dt); - } + /* Repack FDT for handoff to kernel */ + pack_fdt(&fdt, dt); - if (config->ramdisk_node && - extract(&initrd, config->ramdisk_node)) { + if (config->ramdisk && + extract(&initrd, config->ramdisk)) { printk(BIOS_ERR, "ERROR: Failed to extract initrd\n"); prog_set_entry(payload, NULL, NULL); rdev_munmap(prog_rdev(payload), data); @@ -235,7 +230,7 @@ timestamp_add_now(TS_KERNEL_DECOMPRESSION); - if (extract(&kernel, config->kernel_node)) { + if (extract(&kernel, config->kernel)) { printk(BIOS_ERR, "ERROR: Failed to extract kernel\n"); prog_set_entry(payload, NULL, NULL); rdev_munmap(prog_rdev(payload), data); -- To view, visit https://review.coreboot.org/c/coreboot/+/32870 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ia79e0fd0e1159c4aca64c453b82a0379b133350d Gerrit-Change-Number: 32870 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

4 9

Change in ...coreboot[master]: device_tree: Add overlay support
by Julius Werner (Code Review) 21 Jun '19

21 Jun '19

Hello Patrick Rudolph, I'd like you to do a code review. Please visit https://review.coreboot.org/c/coreboot/+/32869 to review the following change. Change subject: device_tree: Add overlay support ...................................................................... device_tree: Add overlay support This patch adds support for merging a device tree overlay (as defined in Documentation/dt-object-internal.txt in the dtc repository) into a base device tree. It was adapted from depthcharge's http://crosreview.com/1536387. Change-Id: Ibec833cd471201bcc7a79eebf360d5f12adb8ff9 Signed-off-by: Julius Werner <jwerner(a)chromium.org> --- M src/include/device_tree.h M src/lib/device_tree.c 2 files changed, 475 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/69/32869/1 diff --git a/src/include/device_tree.h b/src/include/device_tree.h index d9d9613..8d86bc5 100644 --- a/src/include/device_tree.h +++ b/src/include/device_tree.h @@ -180,6 +180,10 @@ const char *dt_find_string_prop(const struct device_tree_node *node, const char *name); +/* Apply an overlay to a base device tree. Ownership of the overlay data passes + to the newly combined base tree -- do not free() or access it afterwards! */ +int dt_apply_overlay(struct device_tree *tree, struct device_tree *overlay); + /* * Fixups to apply to a kernel's device tree before booting it. */ diff --git a/src/lib/device_tree.c b/src/lib/device_tree.c index a5021ca..05f2ea9 100644 --- a/src/lib/device_tree.c +++ b/src/lib/device_tree.c @@ -1098,3 +1098,474 @@ return reserved; } + +/* + * Increment a single phandle in prop at a given offset by a given adjustment. + * + * @param prop Property whose phandle should be adjusted. + * @param adjustment Value that should be added to the existing phandle. + * @param offset Byte offset of the phandle in the property data. + * + * @return New phandle value, or 0 on error. + */ +static uint32_t dt_adjust_phandle(struct device_tree_property *prop, + uint32_t adjustment, uint32_t offset) +{ + if (offset + 4 > prop->prop.size) + return 0; + + uint32_t phandle = be32dec(prop->prop.data + offset); + if (phandle == 0 || + phandle == FDT_PHANDLE_ILLEGAL || + phandle == 0xffffffff) + return 0; + + phandle += adjustment; + if(phandle >= FDT_PHANDLE_ILLEGAL) + return 0; + + be32enc(prop->prop.data + offset, phandle); + return phandle; +} + +/* + * Adjust all phandles in subtree by adding a new base offset. + * + * @param node Root node of the subtree to work on. + * @param base New phandle base to be added to all phandles. + * + * @return New highest phandle in the subtree, or 0 on error. + */ +static uint32_t dt_adjust_all_phandles(struct device_tree_node *node, + uint32_t base) +{ + uint32_t new_max = MAX(base, 1); // make sure we don't return 0 + struct device_tree_property *prop; + struct device_tree_node *child; + + if (!node) + return new_max; + + list_for_each(prop, node->properties, list_node) + if (dt_prop_is_phandle(prop)) { + node->phandle = dt_adjust_phandle(prop, base, 0); + if (!node->phandle) + return 0; + new_max = MAX(new_max, node->phandle); + } // no break -- can have more than one phandle prop + + list_for_each(child, node->children, list_node) + new_max = MAX(new_max, dt_adjust_all_phandles(child, base)); + + return new_max; +} + +/* + * Apply a /__local_fixup__ subtree to the corresponding overlay subtree. + * + * @param node Root node of the overlay subtree to fix up. + * @param node Root node of the /__local_fixup__ subtree. + * @param base Adjustment that was added to phandles in the overlay. + * + * @return 0 on success, -1 on error. + */ +static int dt_fixup_locals(struct device_tree_node *node, + struct device_tree_node *fixup, uint32_t base) +{ + struct device_tree_property *prop; + struct device_tree_property *fixup_prop; + struct device_tree_node *child; + struct device_tree_node *fixup_child; + int i; + + // For local fixups the /__local_fixup__ subtree contains the same node + // hierarchy as the main tree we're fixing up. Each property contains + // the fixup offsets for the respective property in the main tree. For + // each property in the fixup node, find the corresponding property in + // the base node and apply fixups to all offsets it specifies. + list_for_each(fixup_prop, fixup->properties, list_node) { + struct device_tree_property *base_prop = NULL; + list_for_each(prop, node->properties, list_node) + if (!strcmp(prop->prop.name, fixup_prop->prop.name)) { + base_prop = prop; + break; + } + + // We should always find a corresponding base prop for a fixup, + // and fixup props contain a list of 32-bit fixup offsets. + if (!base_prop || fixup_prop->prop.size % sizeof(uint32_t)) + return -1; + + for (i = 0; i < fixup_prop->prop.size; i += sizeof(uint32_t)) + if (!dt_adjust_phandle(base_prop, base, be32dec( + fixup_prop->prop.data + i))) + return -1; + } + + // Now recursively descend both the base tree and the /__local_fixups__ + // subtree in sync to apply all fixups. + list_for_each(fixup_child, fixup->children, list_node) { + struct device_tree_node *base_child = NULL; + list_for_each(child, node->children, list_node) + if (!strcmp(child->name, fixup_child->name)) { + base_child = child; + break; + } + + // All fixup nodes should have a corresponding base node. + if (!base_child) + return -1; + + if (dt_fixup_locals(base_child, fixup_child, base) < 0) + return -1; + } + + return 0; +} + +/* + * Update all /__symbols__ properties in an overlay that start with + * "/fragment@X/__overlay__" with corresponding path prefix in the base tree. + * + * @param symbols /__symbols__ done to update. + * @param fragment /fragment@X node that references to should be updated. + * @param base_path Path of base tree node that the fragment overlaid. + */ +static void dt_fix_symbols(struct device_tree_node *symbols, + struct device_tree_node *fragment, + const char *base_path) +{ + struct device_tree_property *prop; + char buf[512]; // Should be enough for maximum DT path length? + char node_path[64]; // easily enough for /fragment@XXXX/__overlay__ + + if (!symbols) // If the overlay has no /__symbols__ node, we're done! + return; + + int len = snprintf(node_path, sizeof(node_path), "/%s/__overlay__", + fragment->name); + + list_for_each(prop, symbols->properties, list_node) + if (!strncmp(prop->prop.data, node_path, len)) { + prop->prop.size = snprintf(buf, sizeof(buf), "%s%s", + base_path, (char *)prop->prop.data + len) + 1; + free(prop->prop.data); + prop->prop.data = strdup(buf); + } +} + +/* + * Fix up overlay according to a property in /__fixup__. If the fixed property + * is a /fragment@X:target, also update /__symbols__ references to fragment. + * + * @params overlay Overlay to fix up. + * @params fixup /__fixup__ property. + * @params phandle phandle value to insert where the fixup points to. + * @params base_path Path to the base DT node that the fixup points to. + * @params overlay_symbols /__symbols__ node of the overlay. + * + * @return 0 on success, -1 on error. + */ +static int dt_fixup_external(struct device_tree *overlay, + struct device_tree_property *fixup, + uint32_t phandle, const char *base_path, + struct device_tree_node *overlay_symbols) +{ + struct device_tree_property *prop; + + // External fixup properties are encoded as "<path>:<prop>:<offset>". + char *entry = fixup->prop.data; + while ((void *)entry < fixup->prop.data + fixup->prop.size) { + // okay to destroy fixup property value, won't be needed again + char *node_path = entry; + entry = strchr(node_path, ':'); + if (!entry) + return -1; + *entry++ = '\0'; + + char *prop_name = entry; + entry = strchr(prop_name, ':'); + if (!entry) + return -1; + *entry++ = '\0'; + + struct device_tree_node *ovl_node = dt_find_node_by_path( + overlay, node_path, NULL, NULL, 0); + if (!ovl_node || !isdigit(*entry)) + return -1; + + struct device_tree_property *ovl_prop = NULL; + list_for_each(prop, ovl_node->properties, list_node) + if (!strcmp(prop->prop.name, prop_name)) { + ovl_prop = prop; + break; + } + + // Move entry to first char after number, must be a '\0'. + uint32_t offset = skip_atoi((const char **)&entry); + if (!ovl_prop || offset + 4 > ovl_prop->prop.size || entry[0]) + return -1; + entry++; // jump over '\0' to potential next fixup + + be32enc(ovl_prop->prop.data + offset, phandle); + + // If this is a /fragment@X:target property, update + // references to this fragment in the overlay __symbols__ now. + if (offset == 0 && !strcmp(prop_name, "target") && + !strchr(node_path + 1, '/')) // only toplevel nodes + dt_fix_symbols(overlay_symbols, ovl_node, base_path); + } + + return 0; +} + +/* + * Apply all /__fixup__ properties in the overlay. This will destroy the + * property data in /__fixup__ and it should not be accessed again. + * + * @params tree Base device tree that the overlay updates. + * @params symbols /__symbols__ node of the base device tree. + * @params overlay Overlay to fix up. + * @params fixups /__fixup__ node in the overlay. + * @params overlay_symbols /__symbols__ node of the overlay. + * + * @return 0 on success, -1 on error. + */ +static int dt_fixup_all_externals(struct device_tree *tree, + struct device_tree_node *symbols, + struct device_tree *overlay, + struct device_tree_node *fixups, + struct device_tree_node *overlay_symbols) +{ + struct device_tree_property *fix; + + // If we have any external fixups, the base tree must have /__symbols__. + if (!symbols) + return -1; + + // Unlike /__local_fixups__, /__fixups__ is not a whole subtree that + // mirrors the node hierarchy. It's just a directory of fixup properties + // that each directly contain all information necessary to apply them. + list_for_each(fix, fixups->properties, list_node) { + // The name of a fixup property is the label of the node we want + // a property to phandle-reference. Look it up in /__symbols__. + const char *path = dt_find_string_prop(symbols, fix->prop.name); + if (!path) + return -1; + + // Find the node the label pointed to to figure out its phandle. + struct device_tree_node *node = dt_find_node_by_path(tree, path, + NULL, NULL, 0); + if (!node) + return -1; + + // Write it into the overlay property(s) pointing to that node. + if (dt_fixup_external(overlay, fix, node->phandle, + path, overlay_symbols) < 0) + return -1; + } + + return 0; +} + +/* + * Copy all nodes and properties from one DT subtree into another. This is a + * shallow copy so both trees will point to the same property data afterwards. + * + * @params dst Destination subtree to copy into. + * @params src Source subtree to copy from. + * @params upd 1 to overwrite same-name properties, 0 to discard them. + */ +static void dt_copy_subtree(struct device_tree_node *dst, + struct device_tree_node *src, int upd) +{ + struct device_tree_property *prop; + struct device_tree_property *src_prop; + list_for_each (src_prop, src->properties, list_node) { + if (dt_prop_is_phandle(src_prop) || + !strcmp(src_prop->prop.name, "name")) { + printk(BIOS_DEBUG, + "WARNING: ignoring illegal overlay prop '%s'\n", + src_prop->prop.name); + continue; + } + + struct device_tree_property *dst_prop = NULL; + list_for_each(prop, dst->properties, list_node) + if (!strcmp(prop->prop.name, src_prop->prop.name)) { + dst_prop = prop; + break; + } + + if (dst_prop) { + if (!upd) { + printk(BIOS_DEBUG, + "WARNING: ignoring prop update '%s'\n", + src_prop->prop.name); + continue; + } + } else { + dst_prop = xzalloc(sizeof(*dst_prop)); + list_insert_after(&dst_prop->list_node, + &dst->properties); + } + + dst_prop->prop = src_prop->prop; + } + + struct device_tree_node *node; + struct device_tree_node *src_node; + list_for_each (src_node, src->children, list_node) { + struct device_tree_node *dst_node = NULL; + list_for_each (node, dst->children, list_node) + if (!strcmp(node->name, src_node->name)) { + dst_node = node; + break; + } + + if (!dst_node) { + dst_node = xzalloc(sizeof(*dst_node)); + *dst_node = *src_node; + list_insert_after(&dst_node->list_node, &dst->children); + } else { + dt_copy_subtree(dst_node, src_node, upd); + } + } +} + +/* + * Apply an overlay /fragment@X node to a base device tree. + * + * @param tree Base device tree. + * @param fragment /fragment@X node. + * @params overlay_symbols /__symbols__ node of the overlay. + * + * @return 0 on success, -1 on error. + */ +static int dt_import_fragment(struct device_tree *tree, + struct device_tree_node *fragment, + struct device_tree_node *overlay_symbols) +{ + // The actually overlaid nodes/props are in an __overlay__ child node. + static const char *overlay_path[] = { "__overlay__", NULL }; + struct device_tree_node *overlay = dt_find_node(fragment, overlay_path, + NULL, NULL, 0); + + // If it doesn't have an __overlay__ child, it's not a fragment. + if (!overlay) + return 0; + + // The target node of the fragment can be given by path or by phandle. + struct device_tree_property *prop; + struct device_tree_property *phandle = NULL; + struct device_tree_property *path = NULL; + list_for_each(prop, fragment->properties, list_node) { + if (!strcmp(prop->prop.name, "target")) { + phandle = prop; + break; // phandle target has priority, stop looking here + } + if (!strcmp(prop->prop.name, "target-path")) + path = prop; + } + + struct device_tree_node *target = NULL; + if (phandle) { + if (phandle->prop.size != sizeof(uint32_t)) + return -1; + target = dt_find_node_by_phandle(tree->root, + be32dec(phandle->prop.data)); + // Symbols already updated as part of dt_fixup_external(target). + } else if (path) { + target = dt_find_node_by_path(tree, path->prop.data, + NULL, NULL, 0); + dt_fix_symbols(overlay_symbols, fragment, path->prop.data); + } + if (!target) + return -1; + + dt_copy_subtree(target, overlay, 1); + return 0; +} + +/* + * Apply a device tree overlay to a base device tree. This will + * destroy/incorporate the overlay data, so it should not be freed or reused. + * See dtc.git/Documentation/dt-object-internal.txt for overlay format details. + * + * @param tree Unflattened base device tree to add the overlay into. + * @param overlay Unflattened overlay device tree to apply to the base. + * + * @return 0 on success, -1 on error. + */ +int dt_apply_overlay(struct device_tree *tree, struct device_tree *overlay) +{ + // First, we need to make sure phandles inside the overlay don't clash + // with those in the base tree. We just define the highest phandle value + // in the base tree as the "phandle offset" for this overlay and + // increment all phandles in it by that value. + uint32_t phandle_base = tree->max_phandle; + uint32_t new_max = dt_adjust_all_phandles(overlay->root, phandle_base); + if (!new_max) { + printk(BIOS_DEBUG, "ERROR: invalid phandles in overlay\n"); + return -1; + } + tree->max_phandle = new_max; + + // Now that we changed phandles in the overlay, we need to update any + // nodes referring to them. Those are listed in /__local_fixups__. + struct device_tree_node *local_fixups = dt_find_node_by_path(overlay, + "/__local_fixups__", NULL, NULL, 0); + if (local_fixups && dt_fixup_locals(overlay->root, local_fixups, + phandle_base) < 0) { + printk(BIOS_DEBUG, "ERROR: invalid local fixups in overlay\n"); + return -1; + } + + // Besides local phandle references (from nodes within the overlay to + // other nodes within the overlay), the overlay may also contain phandle + // references to the base tree. These are stored with invalid values and + // must be updated now. /__symbols__ contains a list of all labels in + // the base tree, and /__fixups__ describes all nodes in the overlay + // that contain external phandle references. + // We also take this opportunity to update all /fragment@X/__overlay__/ + // prefixes in the overlay's /__symbols__ node to the correct path that + // the fragment will be placed in later, since this is the only step + // where we have all necessary information for that easily available. + struct device_tree_node *symbols = dt_find_node_by_path(tree, + "/__symbols__", NULL, NULL, 0); + struct device_tree_node *fixups = dt_find_node_by_path(overlay, + "/__fixups__", NULL, NULL, 0); + struct device_tree_node *overlay_symbols = dt_find_node_by_path(overlay, + "/__symbols__", NULL, NULL, 0); + if (fixups && dt_fixup_all_externals(tree, symbols, overlay, + fixups, overlay_symbols) < 0) { + printk(BIOS_DEBUG, + "ERROR: cannot match external fixups from overlay\n"); + return -1; + } + + // After all this fixing up, we can finally merge the overlay into the + // tree (one fragment at a time, because for some reason it's split up). + struct device_tree_node *fragment; + list_for_each(fragment, overlay->root->children, list_node) + if (dt_import_fragment(tree, fragment, overlay_symbols) < 0) { + printk(BIOS_DEBUG, "ERROR: bad DT fragment '%s'\n", + fragment->name); + return -1; + } + + // We need to also update /__symbols__ to include labels from this + // overlay, in case we want to load further overlays with external + // phandle references to it. If the base tree already has a /__symbols__ + // we merge them together, otherwise we just insert the overlay's + // /__symbols__ node into the base tree root. + if (overlay_symbols) { + if (symbols) + dt_copy_subtree(symbols, overlay_symbols, 0); + else + list_insert_after(&overlay_symbols->list_node, + &tree->root->children); + } + + return 0; +} -- To view, visit https://review.coreboot.org/c/coreboot/+/32869 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: Ibec833cd471201bcc7a79eebf360d5f12adb8ff9 Gerrit-Change-Number: 32869 Gerrit-PatchSet: 1 Gerrit-Owner: Julius Werner <jwerner(a)chromium.org> Gerrit-Reviewer: Patrick Rudolph <patrick.rudolph(a)9elements.com> Gerrit-MessageType: newchange

4 10

Change in ...coreboot[master]: Add support for Hygon Dhyana processor
by Name of user not set (Code Review) 21 Jun '19

21 Jun '19

Name of user not set #1002411 has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32876 Change subject: Add support for Hygon Dhyana processor ...................................................................... Add support for Hygon Dhyana processor src/arch/x86/cpu.c: Add hygon vendor string verify. Background: Chengdu Haiguang IC Design Co., Ltd (Hygon) is a Joint Venture between AMD and Haiguang Information Technology Co.,Ltd., aims at providing high performance x86 processor for China server market. Its first generation processor codename is Dhyana, which originates from AMD technology and shares most of the architecture with AMD's family 17h, but with different CPU Vendor ID("HygonGenuine")/Family series number(Family 18h). More details can be found on: http://lkml.kernel.org/r/5ce86123a7b9dad925ac583d88d2f921040e859b.153858328… Change-Id: I8af8b0f0675f978ac07522029696e43651a3153f Signed-off-by: Jinke Fan <fanjinke(a)hygon.cn> --- M src/arch/x86/cpu.c M src/arch/x86/include/arch/cpu.h 2 files changed, 3 insertions(+), 0 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/76/32876/1 diff --git a/src/arch/x86/cpu.c b/src/arch/x86/cpu.c index e6c9435..cfab219 100644 --- a/src/arch/x86/cpu.c +++ b/src/arch/x86/cpu.c @@ -113,6 +113,7 @@ { X86_VENDOR_TRANSMETA, "TransmetaCPU", }, { X86_VENDOR_NSC, "Geode by NSC", }, { X86_VENDOR_SIS, "SiS SiS SiS ", }, + { X86_VENDOR_HYGON, "HygonGenuine", }, }; static const char *const x86_vendor_name[] = { @@ -126,6 +127,7 @@ [X86_VENDOR_TRANSMETA] = "Transmeta", [X86_VENDOR_NSC] = "NSC", [X86_VENDOR_SIS] = "SiS", + [X86_VENDOR_HYGON] = "Hygon", }; static const char *cpu_vendor_name(int vendor) diff --git a/src/arch/x86/include/arch/cpu.h b/src/arch/x86/include/arch/cpu.h index 078ea17..ff1a33b 100644 --- a/src/arch/x86/include/arch/cpu.h +++ b/src/arch/x86/include/arch/cpu.h @@ -152,6 +152,7 @@ #define X86_VENDOR_TRANSMETA 8 #define X86_VENDOR_NSC 9 #define X86_VENDOR_SIS 10 +#define X86_VENDOR_HYGON 11 #define X86_VENDOR_ANY 0xfe #define X86_VENDOR_UNKNOWN 0xff -- To view, visit https://review.coreboot.org/c/coreboot/+/32876 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I8af8b0f0675f978ac07522029696e43651a3153f Gerrit-Change-Number: 32876 Gerrit-PatchSet: 1 Gerrit-Owner: Name of user not set #1002411 Gerrit-MessageType: newchange

7 12

Change in ...coreboot[master]: sb/nvidia/ck804/lpc.c: Remove variable set but not used
by HAOUAS Elyes (Code Review) 21 Jun '19

21 Jun '19

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33063 Change subject: sb/nvidia/ck804/lpc.c: Remove variable set but not used ...................................................................... sb/nvidia/ck804/lpc.c: Remove variable set but not used Change-Id: I19b16bc2052440ca191cf3e30810ddc58a485a60 Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/southbridge/nvidia/ck804/lpc.c 1 file changed, 1 insertion(+), 2 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/63/33063/1 diff --git a/src/southbridge/nvidia/ck804/lpc.c b/src/southbridge/nvidia/ck804/lpc.c index 2123554..8caa8ed 100644 --- a/src/southbridge/nvidia/ck804/lpc.c +++ b/src/southbridge/nvidia/ck804/lpc.c @@ -124,10 +124,9 @@ get_option(&on, "slow_cpu"); if (on) { u16 pm10_bar; - u32 dword; pm10_bar = (pci_read_config16(dev, 0x60) & 0xff00); outl(((on << 1) + 0x10), (pm10_bar + 0x10)); - dword = inl(pm10_bar + 0x10); + inl(pm10_bar + 0x10); on = 8 - on; printk(BIOS_DEBUG, "Throttling CPU %2d.%1.1d percent.\n", (on * 12) + (on >> 1), (on & 1) * 5); -- To view, visit https://review.coreboot.org/c/coreboot/+/33063 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I19b16bc2052440ca191cf3e30810ddc58a485a60 Gerrit-Change-Number: 33063 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-MessageType: newchange

2 2

Change in ...coreboot[master]: nb/amd/amdfam10/northbridge.c: Remove variable set but not used
by HAOUAS Elyes (Code Review) 21 Jun '19

21 Jun '19

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/33013 Change subject: nb/amd/amdfam10/northbridge.c: Remove variable set but not used ...................................................................... nb/amd/amdfam10/northbridge.c: Remove variable set but not used Change-Id: I62a51b794dedcf320b8054125e75aa041035ce33 Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/northbridge/amd/amdfam10/northbridge.c 1 file changed, 0 insertions(+), 7 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/13/33013/1 diff --git a/src/northbridge/amd/amdfam10/northbridge.c b/src/northbridge/amd/amdfam10/northbridge.c index 1d071c1..db40267 100644 --- a/src/northbridge/amd/amdfam10/northbridge.c +++ b/src/northbridge/amd/amdfam10/northbridge.c @@ -1690,7 +1690,6 @@ uint8_t fam15h = 0; uint8_t rev_gte_d = 0; - unsigned nb_cfg_54; uint32_t family; uint32_t model; @@ -1700,7 +1699,6 @@ if (is_fam15h()) { /* Family 15h or later */ fam15h = 1; - nb_cfg_54 = 1; } if ((model >= 0x8) || fam15h) @@ -1862,7 +1860,6 @@ uint8_t compute_unit_count = 0; uint32_t f3xe8; - uint8_t dual_node = 0; for (i = 0; i < sysconf.nodes; i++) { struct device *f3x_dev = pcidev_on_root(0x18 + i, 3); @@ -1871,10 +1868,6 @@ f3xe8 = pci_read_config32(f3x_dev, 0xe8); - /* Check for dual node capability */ - if (f3xe8 & 0x20000000) - dual_node = 1; - /* Determine the number of active compute units on this node */ f5x80 = pci_read_config32(f5x_dev, 0x80); cu_enabled = f5x80 & 0xf; -- To view, visit https://review.coreboot.org/c/coreboot/+/33013 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I62a51b794dedcf320b8054125e75aa041035ce33 Gerrit-Change-Number: 33013 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-MessageType: newchange

3 15

Change in ...coreboot[master]: nb/intel/nehalem/raminit.c: Remove variable set but not used
by HAOUAS Elyes (Code Review) 21 Jun '19

21 Jun '19

HAOUAS Elyes has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/32944 Change subject: nb/intel/nehalem/raminit.c: Remove variable set but not used ...................................................................... nb/intel/nehalem/raminit.c: Remove variable set but not used Change-Id: I216d4d00375fcff9e740583b9e17013e0495360c Signed-off-by: Elyes HAOUAS <ehaouas(a)noos.fr> --- M src/northbridge/intel/nehalem/raminit.c 1 file changed, 12 insertions(+), 17 deletions(-) git pull ssh://review.coreboot.org:29418/coreboot refs/changes/44/32944/1 diff --git a/src/northbridge/intel/nehalem/raminit.c b/src/northbridge/intel/nehalem/raminit.c index 5b6077f..e54350c 100644 --- a/src/northbridge/intel/nehalem/raminit.c +++ b/src/northbridge/intel/nehalem/raminit.c @@ -3740,11 +3740,6 @@ u16 deven; int cbmem_wasnot_inited; - /* only used for dummy reads */ - volatile u8 tmp8; - volatile u16 tmp16; - volatile u32 tmp32; - x2ca8 = MCHBAR8(0x2ca8); deven = pci_read_config16(NORTHBRIDGE, D0F0_DEVEN); @@ -4058,10 +4053,10 @@ pci_read_config8(PCI_DEV (0, 0x2, 0x0), 0x4c); pci_read_config8(PCI_DEV (0, 0x2, 0x0), 0x4e); - tmp8 = MCHBAR8(0x1150); - tmp8 = MCHBAR8(0x1151); - tmp8 = MCHBAR8(0x1022); - tmp8 = MCHBAR8(0x16d0); + MCHBAR8(0x1150); + MCHBAR8(0x1151); + MCHBAR8(0x1022); + MCHBAR8(0x16d0); MCHBAR32(0x1300) = 0x60606060; MCHBAR32(0x1304) = 0x60606060; MCHBAR32(0x1308) = 0x78797a7b; @@ -4249,10 +4244,10 @@ } MCHBAR32_AND_OR(0x2c80, 0, 0x1053688); // !!!! - tmp32 = MCHBAR32(0x1c04); // !!!! + MCHBAR32(0x1c04); // !!!! MCHBAR32(0x1804) = 0x406080; - tmp8 = MCHBAR8(0x2ca8); + MCHBAR8(0x2ca8); if (x2ca8 == 0) { MCHBAR8_AND(0x2ca8, ~3); @@ -4264,9 +4259,9 @@ MCHBAR8(0x2ca8) = MCHBAR8(0x2ca8); MCHBAR32_AND_OR(0x2c80, 0, 0x53688); // !!!! pci_write_config32(PCI_DEV (0xff, 0, 0), 0x60, 0x20220); - tmp16 = MCHBAR16(0x2c20); // !!!! - tmp16 = MCHBAR16(0x2c10); // !!!! - tmp16 = MCHBAR16(0x2c00); // !!!! + MCHBAR16(0x2c20); // !!!! + MCHBAR16(0x2c10); // !!!! + MCHBAR16(0x2c00); // !!!! MCHBAR16(0x2c00) = 0x8c0; udelay(1000); write_1d0(0, 0x33d, 0, 0); @@ -4372,7 +4367,7 @@ MCHBAR32_AND_OR(0x1f4, 0, 0x20000); // !!!! MCHBAR32(0x1f0) = 0x1d000200; MCHBAR8_AND_OR(0x1f0, 0, 0x1); // !!!! - tmp8 = MCHBAR8(0x1f0); // !!!! + MCHBAR8(0x1f0); // !!!! program_board_delay(&info); @@ -4431,7 +4426,7 @@ info.populated_ranks[channel][0][0] ? 9 : 1); rmw_1d0(0x116, 0xe, 1, 4, 1); // = 0x4040432 // !!!! - tmp32 = MCHBAR32(0x144); // !!!! + MCHBAR32(0x144); // !!!! write_1d0(2, 0xae, 6, 1); write_1d0(2, 0x300, 6, 1); write_1d0(2, 0x121, 3, 1); @@ -4681,7 +4676,7 @@ reg1c = read32p(DEFAULT_EPBAR | 0x01c); // = 0x8001 // OK pci_read_config32(NORTHBRIDGE, 0x40); // = DEFAULT_EPBAR | 0x001 // OK write32p(DEFAULT_EPBAR | 0x01c, reg1c); // OK - tmp8 = MCHBAR8(0xe08); // = 0x0 + MCHBAR8(0xe08); // = 0x0 pci_read_config32(NORTHBRIDGE, 0xe4); // = 0x316126 MCHBAR8_OR(0x1210, 2); MCHBAR32(0x1200) = 0x8800440; -- To view, visit https://review.coreboot.org/c/coreboot/+/32944 To unsubscribe, or for help writing mail filters, visit https://review.coreboot.org/settings Gerrit-Project: coreboot Gerrit-Branch: master Gerrit-Change-Id: I216d4d00375fcff9e740583b9e17013e0495360c Gerrit-Change-Number: 32944 Gerrit-PatchSet: 1 Gerrit-Owner: HAOUAS Elyes <ehaouas(a)noos.fr> Gerrit-MessageType: newchange

4 5

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

coreboot-gerrit May 2019