[S] Change in coreboot[main]: [WIP] commonlib/device_tree.c,lib/fit.c: Consider address-cells and s...

[Benjamin Doron (Code Review)]

Benjamin Doron has uploaded this change for review. ( https://review.coreboot.org/c/coreboot/+/85643?usp=email )

Change subject: [WIP] commonlib/device_tree.c,lib/fit.c: Consider address-cells and size-cells
......................................................................

[WIP] commonlib/device_tree.c,lib/fit.c: Consider address-cells and size-cells

Follow the recommendation at
https://review.coreboot.org/c/coreboot/+/84796/comment/21f615a2_99a41147/
and implement support for reading integer properties generically, using their
size to determine how much to read. This is used for reading `load`,
`entry`, etc.

I still need to confirm with the FIT spec if we should find these cells in the
root, or /images?

TODO: I want to confirm the maths, but I don't have a known-good FIT to
use. This is broken for EDK2's UPL: they're not compliant, there is no
{address,size}-cells prop there.

Change-Id: I02d27eb5e23dfbfc1404d209ee8d60968e22bb80
Signed-off-by: Benjamin Doron 
---
M src/commonlib/device_tree.c
M src/commonlib/include/commonlib/device_tree.h
M src/lib/fit.c
3 files changed, 27 insertions(+), 7 deletions(-)

git pull ssh://review.coreboot.org:29418/coreboot refs/changes/43/85643/1

diff --git a/src/commonlib/device_tree.c b/src/commonlib/device_tree.c
index b2daac9..5997201 100644
--- a/src/commonlib/device_tree.c
+++ b/src/commonlib/device_tree.c
@@ -260,6 +260,23 @@
    return offset;
 }

+uint64_t fdt_read_int_prop(struct fdt_property *prop, u32 cells)
+{
+	printk(BIOS_DEBUG, "cells %d, prop->size %d\n", cells, prop->size);
+	if (false) {// FIXME: Confirm the math, I suspect the other order: cells != prop->size * 4) {
+		printk(BIOS_ERR, "FDT integer property of size %u @%p doesn't match expected cell count %u\n", prop->size, prop->data, cells);
+		return 0;
+	}
+
+	if (cells == 2)
+		return be64dec(prop->data);
+	else if (cells == 1)
+		return be32dec(prop->data);
+
+	printk(BIOS_ERR, "Illegal FDT integer property size %u @%p\n", prop->size, prop);
+	return 0;
+}
+
 /*
  * fdt_find_node searches for a node relative to another node
  *
diff --git a/src/commonlib/include/commonlib/device_tree.h b/src/commonlib/include/commonlib/device_tree.h
index c15d7ae..df17a1a 100644
--- a/src/commonlib/include/commonlib/device_tree.h
+++ b/src/commonlib/include/commonlib/device_tree.h
@@ -118,6 +118,8 @@
 /* Read reg property and save regions inside 'regions'. Returns number of regions read */
 u32 fdt_read_reg_prop(const void *blob, u32 node_offset, u32 addr_cells, u32 size_cells,
    	      struct device_tree_region regions[], size_t regions_count);
+/* Reads value for a fdt_prop, considering the cells */
+uint64_t fdt_read_int_prop(struct fdt_property *prop, u32 cells);
 /* Find a node by a given path and return the offset */
 u32 fdt_find_node_by_path(const void *blob, const char *path, u32 *addrcp, u32 *sizecp);
 /* Find multiple nodes matching a given pattern. Returns number of nodes found */
diff --git a/src/lib/fit.c b/src/lib/fit.c
index 3e384d7..25e9ee6 100644
--- a/src/lib/fit.c
+++ b/src/lib/fit.c
@@ -107,7 +107,7 @@
    return base;
 }

-static void image_node(struct device_tree_node *node)
+static void image_node(struct device_tree_node *node, u32 addr_cells, u32 size_cells)
 {
    struct fit_image_node *image = xzalloc(sizeof(*image));

@@ -117,9 +117,9 @@
    struct device_tree_property *prop;
    list_for_each(prop, node->properties, list_node) {
    	if (!strcmp("data-offset", prop->prop.name))
-			image->data = fit_bin_base + be32dec(prop->prop.data);
+			image->data = fit_bin_base + fdt_read_int_prop(&prop->prop, addr_cells);
    	else if (!strcmp("data-size", prop->prop.name))
-			image->size = be32dec(prop->prop.data);
+			image->size = fdt_read_int_prop(&prop->prop, size_cells);
    	else if (!strcmp("data", prop->prop.name)) {
    		image->data = prop->prop.data;
    		image->size = prop->prop.size;
@@ -133,9 +133,9 @@
    		else
    			image->compression = -1;
    	} else if (!strcmp("load", prop->prop.name))
-			image->load_address = be64dec(prop->prop.data);
+			image->load_address = fdt_read_int_prop(&prop->prop, addr_cells);
    	else if (!strcmp("entry", prop->prop.name))
-			image->entrypoint_address = be64dec(prop->prop.data);
+			image->entrypoint_address = fdt_read_int_prop(&prop->prop, addr_cells);
    }

list_insert_after(&image->list_node, &image_nodes);
@@ -166,12 +166,13 @@

static void fit_unpack(struct device_tree *tree, const char **default_config)
 {
+	u32 addr_cells, size_cells;
    struct device_tree_node *child;
    struct device_tree_node *images = dt_find_node_by_path(tree, "/images",
-							       NULL, NULL, 0);
+							       &addr_cells, &size_cells, 0);
    if (images)
    	list_for_each(child, images->children, list_node)
-			image_node(child);
+			image_node(child, addr_cells, size_cells);

struct device_tree_node *configs = dt_find_node_by_path(tree,
    	"/configurations", NULL, NULL, 0);