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. Signed-off-by: Carlo Caione <[email protected]> --- boot/Kconfig | 2 ++ boot/image-fit.c | 20 ++++++++++++-- include/image.h | 4 +++ test/boot/image.c | 68 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 92 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/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
