[U-Boot] [PATCH v2 7/9] libfdt: Add overlay application function

Pantelis Antoniou pantelis.antoniou at konsulko.com
Fri Jun 10 16:28:11 CEST 2016


Hi Maxime,

> On May 27, 2016, at 12:13 , Maxime Ripard <maxime.ripard at free-electrons.com> wrote:
> 
> The device tree overlays are a good way to deal with user-modifyable
> boards or boards with some kind of an expansion mechanism where we can
> easily plug new board in (like the BBB, the Raspberry Pi or the CHIP).
> 
> Add a new function to merge overlays with a base device tree.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard at free-electrons.com>
> ---
> include/libfdt.h         |  30 ++++
> lib/libfdt/Makefile      |   2 +-
> lib/libfdt/fdt_overlay.c | 414 +++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 445 insertions(+), 1 deletion(-)
> create mode 100644 lib/libfdt/fdt_overlay.c
> 
> diff --git a/include/libfdt.h b/include/libfdt.h
> index 1e01b2c7ebdf..783067e841a1 100644
> --- a/include/libfdt.h
> +++ b/include/libfdt.h
> @@ -1698,6 +1698,36 @@ int fdt_add_subnode(void *fdt, int parentoffset, const char *name);
>  */
> int fdt_del_node(void *fdt, int nodeoffset);
> 
> +/**
> + * fdt_overlay_apply - Applies a DT overlay on a base DT
> + * @fdt: pointer to the base device tree blob
> + * @fdto: pointer to the device tree overlay blob
> + *
> + * fdt_overlay_apply() will apply the given device tree overlay on the
> + * given base device tree.
> + *
> + * Expect the base device tree to be modified, even if the function
> + * returns an error.
> + *

If the base tree has been modified on an error it is unsafe to use it
for booting. A valid strategy would be to scribble over the DT magic
number so that that blob is invalidated.

What are the other people’s opinion on this?

> + * returns:
> + *	0, on success
> + *	-FDT_ERR_NOSPACE, there's not enough space in the base device tree
> + *	-FDT_ERR_NOTFOUND, the overlay points to some inexistant nodes or
> + *		properties in the base DT
> + *	-FDT_ERR_BADPHANDLE, the phandles in the overlay do not have the right
> + *		magic
> + *	-FDT_ERR_INTERNAL,
> + *	-FDT_ERR_BADLAYOUT,
> + *	-FDT_ERR_BADMAGIC,
> + *	-FDT_ERR_BADOFFSET,
> + *	-FDT_ERR_BADPATH,
> + *	-FDT_ERR_BADVERSION,
> + *	-FDT_ERR_BADSTRUCTURE,
> + *	-FDT_ERR_BADSTATE,
> + *	-FDT_ERR_TRUNCATED, standard meanings
> + */
> +int fdt_overlay_apply(void *fdt, void *fdto);
> +
> /**********************************************************************/
> /* Debugging / informational functions                                */
> /**********************************************************************/
> diff --git a/lib/libfdt/Makefile b/lib/libfdt/Makefile
> index 934d6142c3e9..4935bf012a75 100644
> --- a/lib/libfdt/Makefile
> +++ b/lib/libfdt/Makefile
> @@ -6,4 +6,4 @@
> #
> 
> obj-y += fdt.o fdt_ro.o fdt_rw.o fdt_strerror.o fdt_sw.o fdt_wip.o \
> -	fdt_empty_tree.o fdt_addresses.o fdt_region.o
> +	fdt_empty_tree.o fdt_addresses.o fdt_region.o fdt_overlay.o
> diff --git a/lib/libfdt/fdt_overlay.c b/lib/libfdt/fdt_overlay.c
> new file mode 100644
> index 000000000000..1e68e903c734
> --- /dev/null
> +++ b/lib/libfdt/fdt_overlay.c
> @@ -0,0 +1,414 @@
> +#include "libfdt_env.h"
> +
> +#include <fdt.h>
> +#include <libfdt.h>
> +
> +#include "libfdt_internal.h"
> +
> +static uint32_t fdt_overlay_get_target_phandle(const void *fdto, int node)
> +{
> +	const uint32_t *val;
> +	int len;
> +
> +	val = fdt_getprop(fdto, node, "target", &len);
> +	if (!val || (len != sizeof(*val)))
> +		return 0;
> +
> +	return fdt32_to_cpu(*val);
> +}
> +
> +static int fdt_overlay_get_target(const void *fdt, const void *fdto,
> +				  int fragment)
> +{
> +	uint32_t phandle;
> +	const char *path;
> +
> +	/* Try first to do a phandle based lookup */
> +	phandle = fdt_overlay_get_target_phandle(fdto, fragment);
> +	if (phandle)
> +		return fdt_node_offset_by_phandle(fdt, phandle);
> +
> +	/* And then a path based lookup */
> +	path = fdt_getprop(fdto, fragment, "target-path", NULL);
> +	if (!path)
> +		return -FDT_ERR_NOTFOUND;
> +
> +	return fdt_path_offset(fdt, path);
> +}
> +

Those targets are fine; beware there are patches with more target options.

> +static int fdt_overlay_adjust_node_phandles(void *fdto, int node,
> +					    uint32_t delta)
> +{
> +	int property;
> +	int child;
> +
> +	fdt_for_each_property(fdto, node, property) {
> +		const uint32_t *val;
> +		const char *name;
> +		uint32_t adj_val;
> +		int len;
> +		int ret;
> +
> +		val = fdt_getprop_by_offset(fdto, property,
> +					    &name, &len);
> +		if (!val || (len != sizeof(*val)))

Superfluous parentheses.

> +			continue;
> +
> +		if (strcmp(name, "phandle") && strcmp(name, "linux,phandle"))
> +			continue;
> +
> +		adj_val = fdt32_to_cpu(*val);
> +		adj_val += delta;
> +		ret = fdt_setprop_inplace_u32(fdto, node, name, adj_val);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	fdt_for_each_subnode(fdto, child, node)
> +		fdt_overlay_adjust_node_phandles(fdto, child, delta);
> +
> +	return 0;
> +}
> +
> +static int fdt_overlay_adjust_local_phandles(void *fdto, uint32_t delta)
> +{
> +	int root;
> +
> +	root = fdt_path_offset(fdto, "/");
> +	if (root < 0)
> +		return root;
> +
> +	return fdt_overlay_adjust_node_phandles(fdto, root, delta);
> +}
> +
> +static int _fdt_overlay_update_local_references(void *fdto,
> +						int tree_node,
> +						int fixup_node,
> +						uint32_t delta)
> +{
> +	int fixup_prop;
> +	int fixup_child;
> +
> +	fdt_for_each_property(fdto, fixup_node, fixup_prop) {
> +		const char *fixup_name, *tree_name;
> +		const uint32_t *val = NULL;
> +		uint32_t adj_val;
> +		int fixup_len;
> +		int tree_prop;
> +		int tree_len;
> +		int ret;
> +
> +		fdt_getprop_by_offset(fdto, fixup_prop,
> +				      &fixup_name, &fixup_len);
> +
> +		if (!strcmp(fixup_name, "phandle") ||
> +		    !strcmp(fixup_name, "linux,phandle"))
> +			continue;
> +
> +		fdt_for_each_property(fdto, tree_node, tree_prop) {
> +			val = fdt_getprop_by_offset(fdto, tree_prop,
> +						    &tree_name, &tree_len);
> +
> +			if (!strcmp(tree_name, fixup_name))
> +				break;
> +		}
> +
> +		if (!val || !tree_len)
> +			return -FDT_ERR_NOTFOUND;
> +
> +		if (!tree_name)
> +			return -FDT_ERR_NOTFOUND;
> +
> +		if (tree_len != 4)
> +			return -FDT_ERR_NOTFOUND;
> +
> +		adj_val = fdt32_to_cpu(*val);
> +		adj_val += delta;
> +		ret = fdt_setprop_inplace_u32(fdto, tree_node, fixup_name,
> +					      adj_val);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	fdt_for_each_subnode(fdto, fixup_child, fixup_node) {
> +		const char *fixup_child_name = fdt_get_name(fdto,
> +							    fixup_child, NULL);
> +		int tree_child;
> +
> +		fdt_for_each_subnode(fdto, tree_child, tree_node) {
> +			const char *tree_child_name = fdt_get_name(fdto,
> +								   tree_child,
> +								   NULL);
> +
> +			if (!strcmp(fixup_child_name, tree_child_name))
> +				break;
> +		}
> +
> +		_fdt_overlay_update_local_references(fdto,
> +						     tree_child,
> +						     fixup_child,
> +						     delta);
> +	}
> +
> +	return 0;
> +}
> +
> +static int fdt_overlay_update_local_references(void *dto, uint32_t delta)
> +{
> +	int root, fixups;
> +
> +	root = fdt_path_offset(dto, "/");
> +	if (root < 0)
> +		return root;
> +
> +	fixups = fdt_path_offset(dto, "/__local_fixups__");
> +	if (root < 0)
> +		return root;
> +
> +	return _fdt_overlay_update_local_references(dto, root, fixups,
> +						    delta);
> +}
> +
> +static int _fdt_overlay_fixup_phandle(void *fdt, void *fdto,
> +				      int symbols_off,
> +				      const char *path, const char *name,
> +				      int index, const char *label)
> +{
> +	const uint32_t *prop_val;
> +	const char *symbol_path;
> +	uint32_t *fixup_val;
> +	uint32_t phandle;
> +	int symbol_off, fixup_off;
> +	int prop_len;
> +	int ret;
> +
> +	symbol_path = fdt_getprop(fdt, symbols_off, label,
> +				  &prop_len);
> +	if (!symbol_path)
> +		return -FDT_ERR_NOTFOUND;
> +
> +	symbol_off = fdt_path_offset(fdt, symbol_path);
> +	if (symbol_off < 0)
> +		return symbol_off;
> +
> +	phandle = fdt_get_phandle(fdt, symbol_off);
> +	if (!phandle)
> +		return -FDT_ERR_NOTFOUND;
> +
> +	fixup_off = fdt_path_offset(fdto, path);
> +	if (fixup_off < 0)
> +		return fixup_off;
> +
> +	prop_val = fdt_getprop(fdto, fixup_off, name,
> +			       &prop_len);
> +	if (!prop_val)
> +		return -FDT_ERR_NOTFOUND;
> +
> +	fixup_val = malloc(prop_len);
> +	if (!fixup_val)
> +		return -FDT_ERR_INTERNAL;
> +	memcpy(fixup_val, prop_val, prop_len);
> +
> +	if (fdt32_to_cpu(fixup_val[index]) != 0xdeadbeef) {
> +		ret = -FDT_ERR_BADPHANDLE;
> +		goto out;
> +	}
> +
> +	fixup_val[index] = cpu_to_fdt32(phandle);
> +	ret = fdt_setprop_inplace(fdto, fixup_off,
> +				  name, fixup_val,
> +				  prop_len);
> +
> +out:
> +	free(fixup_val);
> +	return ret;
> +};
> +
> +static int fdt_overlay_fixup_phandle(void *fdt, void *fdto, int symbols_off,
> +				     int property)
> +{
> +	const char *value, *tmp_value;
> +	const char *label;
> +	int tmp_len, len, next;
> +	int table = 0;
> +	int i;
> +
> +	value = fdt_getprop_by_offset(fdto, property,
> +				      &label, &len);
> +	tmp_value = value;
> +	tmp_len = len;
> +
> +	do {
> +		next = strlen(tmp_value) + 1;
> +		tmp_len -= next;
> +		tmp_value += next;
> +		table++;
> +	} while (tmp_len > 0);
> +
> +	for (i = 0; i < table; i++) {
> +		const char *prop_string = value;
> +		const char *path, *name;
> +		char *prop_cpy, *prop_tmp;
> +		int index, stlen;
> +		char *sep;
> +
> +		stlen = strlen(prop_string);
> +		prop_cpy = malloc(stlen + 1);
> +		if (!prop_cpy)
> +			return -FDT_ERR_INTERNAL;
> +
> +		strncpy(prop_cpy, prop_string, stlen);
> +		prop_cpy[stlen] = '\0';
> +
> +		for (prop_tmp = prop_cpy; (sep = strchr(prop_tmp, ':'));
> +		     prop_tmp += ((sep - prop_tmp) + 1))
> +			*sep = '\0';
> +
> +		prop_tmp = prop_cpy;
> +		path = prop_tmp;
> +		prop_tmp += strlen(prop_tmp) + 1;
> +
> +		name = prop_tmp;
> +		prop_tmp += strlen(prop_tmp) + 1;
> +
> +		index = strtol(prop_tmp, NULL, 10);
> +		prop_tmp += strlen(prop_tmp) + 1;
> +
> +		value += stlen + 1;
> +		len -= stlen + 1;
> +
> +		_fdt_overlay_fixup_phandle(fdt, fdto, symbols_off,
> +					   path, name, index, label);
> +
> +		free(prop_cpy);
> +	}
> +
> +	return 0;
> +}
> +
> +static int fdt_overlay_fixup_phandles(void *dt, void *dto)
> +{
> +	int fixups_off, symbols_off;
> +	int property;
> +
> +	symbols_off = fdt_path_offset(dt, "/__symbols__");
> +	fixups_off = fdt_path_offset(dto, "/__fixups__");
> +
> +	fdt_for_each_property(dto, fixups_off, property)
> +		fdt_overlay_fixup_phandle(dt, dto, symbols_off, property);
> +
> +	return 0;
> +}
> +
> +static int fdt_apply_overlay_node(void *dt, void *dto,
> +				  int target, int overlay)
> +{
> +	int property;
> +	int node;
> +
> +	fdt_for_each_property(dto, overlay, property) {
> +		const char *name;
> +		const void *prop;
> +		int prop_len;
> +		int ret;
> +
> +		prop = fdt_getprop_by_offset(dto, property, &name,
> +					     &prop_len);
> +		if (!prop)
> +			return -FDT_ERR_INTERNAL;
> +
> +		ret = fdt_setprop(dt, target, name, prop, prop_len);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	fdt_for_each_subnode(dto, node, overlay) {
> +		const char *name = fdt_get_name(dto, node, NULL);
> +		int nnode;
> +		int ret;
> +
> +		nnode = fdt_add_subnode(dt, target, name);
> +		if (nnode < 0)
> +			return nnode;
> +
> +		ret = fdt_apply_overlay_node(dt, dto, nnode, node);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +static int fdt_overlay_merge(void *dt, void *dto)
> +{
> +	int root, fragment;
> +
> +	root = fdt_path_offset(dto, "/");
> +	if (root < 0)
> +		return root;
> +
> +	fdt_for_each_subnode(dto, fragment, root) {
> +		const char *name = fdt_get_name(dto, fragment, NULL);
> +		uint32_t target;
> +		int overlay;
> +		int ret;
> +
> +		if (strncmp(name, "fragment", 8))
> +			continue;
> +

This is incorrect. The use of “fragment” is a convention only.
The real test whether the node is an overlay fragment is that
it contains a target property.

> +		target = fdt_overlay_get_target(dt, dto, fragment);
> +		if (target < 0)
> +			return target;
> +

So you could do ‘if (target < 0) continue;’ or handle a more complex error code.

> +		overlay = fdt_subnode_offset(dto, fragment, "__overlay__");
> +		if (overlay < 0)
> +			return overlay;
> +
> +		ret = fdt_apply_overlay_node(dt, dto, target, overlay);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	return 0;
> +}
> +
> +int fdt_overlay_apply(void *fdt, void *fdto)
> +{
> +	uint32_t delta = fdt_get_max_phandle(fdt) + 1;
> +	void *fdto_copy;
> +	int ret;
> +
> +	FDT_CHECK_HEADER(fdt);
> +	FDT_CHECK_HEADER(fdto);
> +
> +	/*
> +	 * We're going to modify the overlay so that we can apply it.
> +	 *
> +	 * Make sure sure we don't destroy the original
> +	 */
> +	fdto_copy = malloc(fdt_totalsize(fdto));
> +	if (!fdto_copy)
> +		return -FDT_ERR_INTERNAL;
> +
> +	ret = fdt_move(fdto, fdto_copy, fdt_totalsize(fdto));
> +	if (ret)
> +		goto out;
> +
> +	ret = fdt_overlay_adjust_local_phandles(fdto_copy, delta);
> +	if (ret)
> +		goto out;
> +
> +	ret = fdt_overlay_update_local_references(fdto_copy, delta);
> +	if (ret)
> +		goto out;
> +
> +	ret = fdt_overlay_fixup_phandles(fdt, fdto_copy);
> +	if (ret)
> +		goto out;
> +
> +	ret = fdt_overlay_merge(fdt, fdto_copy);
> +
> +out:
> +	free(fdto_copy);
> +	return ret;
> +}
> -- 
> 2.8.2
> 

I would caution against the liberal use of malloc in libfdt. We’re possibly running in a constrained
environment; a custom extents based (non freeing) allocator should be better.

We need some figures about memory consumption when this is enabled, and a CONFIG option to disable it.

Regards

— Pantelis




More information about the U-Boot mailing list