[PATCH v2] fit: prefer the default configuration on best-match ties

[Carlo Caione <ccaione@baylibre.com>]

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 <sjg@chromium.org>
Reviewed-by: Tom Rini <trini@konsulko.com>
Signed-off-by: Carlo Caione <ccaione@baylibre.com>
---
 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