With CONFIG_FIT_BEST_MATCH, fit_conf_find_compat() selects the configuration matching the most specific U-Boot compatible string; on equal matches the first listed configuration wins and the configurations node 'default' property is never consulted.
A FIT whose configurations all share the same base devicetree compatible (e.g. one manifest carrying a base tree plus overlay combinations for a single board) therefore always boots the first configuration, silently ignoring the default chosen by the manifest author. Break score ties in favour of the default configuration. A strictly better compatible match still wins over it, and FITs without a default keep the current first-listed behaviour. Reviewed-by: Simon Glass <[email protected]> Reviewed-by: Tom Rini <[email protected]> Signed-off-by: Carlo Caione <[email protected]> --- boot/Kconfig | 2 + boot/image-fit.c | 20 ++++++++- doc/usage/fit/overlay-fdt-boot.rst | 6 +++ include/image.h | 4 ++ test/boot/image.c | 68 ++++++++++++++++++++++++++++++ 5 files changed, 98 insertions(+), 2 deletions(-) diff --git a/boot/Kconfig b/boot/Kconfig index 8e468c56176..c67dc0ba493 100644 --- a/boot/Kconfig +++ b/boot/Kconfig @@ -191,6 +191,8 @@ config FIT_BEST_MATCH U-Boot itself. A match is considered "best" if it matches the most specific compatibility entry of U-Boot's fdt's root node. The order of entries in the configuration's fdt is ignored. + If several configurations match equally well, the one named by + the configurations node 'default' property is preferred. config FIT_IMAGE_POST_PROCESS bool "Enable post-processing of FIT artifacts after loading by U-Boot" diff --git a/boot/image-fit.c b/boot/image-fit.c index 044a40e1910..ef90c5abd18 100644 --- a/boot/image-fit.c +++ b/boot/image-fit.c @@ -1799,6 +1799,8 @@ int fit_conf_find_compat(const void *fit, const void *fdt) int fdt_compat_len; int best_match_offset = 0; int best_match_pos = 0; + const char *default_name; + int default_noffset = -1; confs_noffset = fdt_path_offset(fit, FIT_CONFS_PATH); images_noffset = fdt_path_offset(fit, FIT_IMAGES_PATH); @@ -1813,6 +1815,15 @@ int fit_conf_find_compat(const void *fit, const void *fdt) return -ENXIO; } + /* the default configuration breaks ties between equal matches */ + default_name = fdt_getprop(fit, confs_noffset, FIT_DEFAULT_PROP, NULL); + if (default_name) { + default_noffset = fdt_subnode_offset(fit, confs_noffset, + default_name); + if (default_noffset < 0) + default_noffset = -1; + } + /* * Loop over the configurations in the FIT image. */ @@ -1863,10 +1874,15 @@ int fit_conf_find_compat(const void *fit, const void *fdt) cur_fdt_compat = fdt_compat; /* * Look for a match for each U-Boot compatibility string in - * turn in the compat string property. + * turn in the compat string property. A configuration only + * replaces the current best match on a strictly better + * position, or on an equal position if it is the default + * configuration. */ for (i = 0; len > 0 && - (!best_match_offset || best_match_pos > i); i++) { + (!best_match_offset || best_match_pos > i || + (best_match_pos == i && noffset == default_noffset)); + i++) { int cur_len = strlen(cur_fdt_compat) + 1; if (!fdt_node_check_compatible(fdt, compat_noffset, diff --git a/doc/usage/fit/overlay-fdt-boot.rst b/doc/usage/fit/overlay-fdt-boot.rst index 5df304047c6..e210619a26d 100644 --- a/doc/usage/fit/overlay-fdt-boot.rst +++ b/doc/usage/fit/overlay-fdt-boot.rst @@ -111,6 +111,12 @@ Where config is one of:: This selects the DTB to use when booting. +If no configuration is given, U-Boot picks one automatically: with +``CONFIG_FIT_BEST_MATCH`` it selects the configuration whose fdt is the most +compatible with U-Boot's own control devicetree, and falls back to the +``default`` configuration otherwise. When several configurations match +equally well, the ``default`` one is preferred. + .. _fit_configuration_using_overlays: Configuration using overlays diff --git a/include/image.h b/include/image.h index 9c8a746d576..01a814ed76e 100644 --- a/include/image.h +++ b/include/image.h @@ -1526,6 +1526,10 @@ int fit_check_format(const void *fit, ulong size); * compatible list, "foo,bar", matches a compatible string in the root of fdt1. * "bim,bam" in fdt2 matches the second string which isn't as good as fdt1. * + * If several configurations match at the same position, the one named by the + * 'default' property of the configurations node is preferred, then the first + * one listed. + * * As an optimization, the compatible property from the FDT's root node can be * copied into the configuration node in the FIT image. This is required to * match configurations with compressed FDTs. diff --git a/test/boot/image.c b/test/boot/image.c index 4df7b17ce88..2c6d9dcbc22 100644 --- a/test/boot/image.c +++ b/test/boot/image.c @@ -8,8 +8,76 @@ #include <image.h> #include <test/ut.h> +#include <linux/libfdt.h> #include "bootstd_common.h" +/* Test that the default configuration breaks best-match ties */ +static int test_fit_conf_find_compat(struct unit_test_state *uts) +{ + char fdt[256], fit[1024]; + int confs, images, node; + int ret; + + /* control devicetree with a two-entry compatible list */ + ut_assertok(fdt_create_empty_tree(fdt, sizeof(fdt))); + ut_assertok(fdt_appendprop_string(fdt, 0, "compatible", + "test,board-a")); + ut_assertok(fdt_appendprop_string(fdt, 0, "compatible", + "test,fallback")); + + /* FIT with two configurations matching the same compatible */ + ut_assertok(fdt_create_empty_tree(fit, sizeof(fit))); + images = fdt_add_subnode(fit, 0, "images"); + ut_assert(images >= 0); + confs = fdt_add_subnode(fit, 0, "configurations"); + ut_assert(confs >= 0); + ut_assertok(fdt_setprop_string(fit, confs, FIT_DEFAULT_PROP, "conf-2")); + /* + * fdt_add_subnode() inserts before existing subnodes: create conf-2 + * first so that conf-1 ends up listed first, like an .its compiled + * with the configurations in that order + */ + node = fdt_add_subnode(fit, confs, "conf-2"); + ut_assert(node >= 0); + ut_assertok(fdt_setprop_string(fit, node, "compatible", + "test,board-a")); + node = fdt_add_subnode(fit, confs, "conf-1"); + ut_assert(node >= 0); + ut_assertok(fdt_setprop_string(fit, node, "compatible", + "test,board-a")); + confs = fdt_path_offset(fit, "/configurations"); + node = fdt_first_subnode(fit, confs); + ut_asserteq_str("conf-1", fdt_get_name(fit, node, NULL)); + + /* on a tie, the default configuration wins */ + ret = fit_conf_find_compat(fit, fdt); + ut_assert(ret > 0); + ut_asserteq_str("conf-2", fdt_get_name(fit, ret, NULL)); + + /* without a default, the first listed configuration wins */ + confs = fdt_path_offset(fit, "/configurations"); + ut_assertok(fdt_delprop(fit, confs, FIT_DEFAULT_PROP)); + confs = fdt_path_offset(fit, "/configurations"); + ut_assertnull((void *)fdt_getprop(fit, confs, FIT_DEFAULT_PROP, NULL)); + ret = fit_conf_find_compat(fit, fdt); + ut_assert(ret > 0); + ut_asserteq_str("conf-1", fdt_get_name(fit, ret, NULL)); + + /* a strictly better match still beats the default */ + confs = fdt_path_offset(fit, "/configurations"); + ut_assertok(fdt_setprop_string(fit, confs, FIT_DEFAULT_PROP, "conf-2")); + confs = fdt_path_offset(fit, "/configurations"); + node = fdt_subnode_offset(fit, confs, "conf-2"); + ut_assertok(fdt_setprop_string(fit, node, "compatible", + "test,fallback")); + ret = fit_conf_find_compat(fit, fdt); + ut_assert(ret > 0); + ut_asserteq_str("conf-1", fdt_get_name(fit, ret, NULL)); + + return 0; +} +BOOTSTD_TEST(test_fit_conf_find_compat, 0); + /* Test of image phase */ static int test_image_phase(struct unit_test_state *uts) { -- 2.55.0
