fit: prefer the default configuration on best-match ties

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>

diff --git a/boot/Kconfig b/boot/Kconfig
index 8e468c5617657988c3865b9b3ca3209291316efc..c67dc0ba4932e2b01b91873aff29f3b3de4a6b01 100644 (file)
--- 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 044a40e19100e2e78c4da5cc67bdd8fb58ab5bea..ef90c5abd182a16b61000d5042659b8024a0f940 100644 (file)
--- 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 5df304047c6cb67ba5d5ad1e3803b879a98bdecb..e210619a26deb9230f64e897aae1d8fc51372068 100644 (file)
--- 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 4b3c9c87bf5e44d1b6030bfec3128e85b151cc15..4149ebbcce99c688373b0cd41baa480a35bf9fbe 100644 (file)
--- a/include/image.h
+++ b/include/image.h
@@ -1535,6 +1535,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 4df7b17ce883b5ddfdfef7ccba0578d8065f647f..2c6d9dcbc223ddaf8dc6c610d7bc3a49b7176db8 100644 (file)
--- 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)
 {