mail archive of the barebox mailing list
 help / color / mirror / Atom feed
* [PATCH v3 00/10] Add FIT image overlay support
@ 2024-07-03 16:58 Marco Felsch
  2024-07-03 16:58 ` [PATCH v3 01/10] FIT: fix missing free in fit_open error path Marco Felsch
                   ` (9 more replies)
  0 siblings, 10 replies; 14+ messages in thread
From: Marco Felsch @ 2024-07-03 16:58 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Marco Felsch

Hi,

this series add the support to load FIT image supplied devicetree overlays.
The v1 of this series can be found here [1].
                                                                                                                                                       
The overlay loading wasn't coupled to bootm due to the following
reasons:
 - By making use of the common overlay handling we can specifiy a
   different/separate FIT image which provides only overlays.
 - It should be possible to apply FIT image overlay to the barebox
   live-tree (not implemented yet).
 - Loading a single overlay takes ~20ms (depending on the overlay size)
   if the same FIT image is used to supply the kernel, initrd,
   devicetree and devicetree-overlays. This is an improvement compared to
   the v1 of this series which required ~1sec.

Regards,
  Marco

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
Changes in v3:
- Link to v2: https://lore.barebox.org/barebox/20240613125818.30499-1-m.felsch@pengutronix.de/
- drop of.overlay.fitconfigpattern usage and make use of an match-fn
  instead
- fit_get_handle(): make use of strcmp
- open_fits: make it static
- fit_close(): use free unconditional

Changes in v2:
- Link to v1: https://lore.pengutronix.de/barebox/20240322164953.1772129-1-m.felsch@pengutronix.de/
- add caching

---
Marco Felsch (10):
      FIT: fix missing free in fit_open error path
      FIT: fit_open_configuration: add match function support
      of: overlay: make the pattern match function more generic
      of: overlay: make search dir/path more generic
      FIT: expose useful helpers
      of: overlay: add FIT overlay support
      of: overlay: drop unnecessary empty check in of_overlay_global_fixup_dir
      of: overlay: replace filename with an more unique name
      FIT: save filename during fit_open
      FIT: add support to cache opened fit images

 arch/arm/mach-layerscape/ppa.c |   2 +-
 common/bootm.c                 |  13 ++-
 common/image-fit.c             |  57 +++++++++--
 drivers/of/overlay.c           | 222 +++++++++++++++++++++++++++++++++--------
 include/image-fit.h            |  10 +-
 include/of.h                   |   3 +-
 6 files changed, 255 insertions(+), 52 deletions(-)
---
base-commit: bafdf4b35d777d159ac4058efc86d36622ce5ccf
change-id: 20240703-v2024-05-0-topic-fit-overlay-76d5176c1d4e

Best regards,
-- 
Marco Felsch <m.felsch@pengutronix.de>




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 01/10] FIT: fix missing free in fit_open error path
  2024-07-03 16:58 [PATCH v3 00/10] Add FIT image overlay support Marco Felsch
@ 2024-07-03 16:58 ` Marco Felsch
  2024-07-03 18:30   ` Ahmad Fatoum
  2024-07-03 16:58 ` [PATCH v3 02/10] FIT: fit_open_configuration: add match function support Marco Felsch
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 14+ messages in thread
From: Marco Felsch @ 2024-07-03 16:58 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Marco Felsch

Free the handle if read_file_2() fails.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 common/image-fit.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/common/image-fit.c b/common/image-fit.c
index 251fda97b3fc..008804e6a6c3 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -922,6 +922,7 @@ struct fit_handle *fit_open(const char *filename, bool verbose,
 			  max_size);
 	if (ret && ret != -EFBIG) {
 		pr_err("unable to read %s: %s\n", filename, strerror(-ret));
+		free(handle);
 		return ERR_PTR(ret);
 	}
 

-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 02/10] FIT: fit_open_configuration: add match function support
  2024-07-03 16:58 [PATCH v3 00/10] Add FIT image overlay support Marco Felsch
  2024-07-03 16:58 ` [PATCH v3 01/10] FIT: fix missing free in fit_open error path Marco Felsch
@ 2024-07-03 16:58 ` Marco Felsch
  2024-07-03 16:58 ` [PATCH v3 03/10] of: overlay: make the pattern match function more generic Marco Felsch
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Marco Felsch @ 2024-07-03 16:58 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Marco Felsch

The FIT spec is not very specific and was growing over the past years.
E.g. according U-Boot (doc/usage/fit/source_file_format.rst) either a
"kernel" or "firmware" property is mandatory whereas the fit-overlay
example (doc/usage/fit/overlay-fdt-boot.rst) doesn't fulfil this and
instead allows nodes which do contain only a "fdt" property.

This inconsistency makes it hard to detect the purpose of an
configuration node. So far we have three different configuration nodes

- bootable nodes (the usual use-case):

|	config-0 {
|		compatible = "machine-compatible";
|		kernel = "kernel-img-name";
|		fdt = "fdt-img-name";
|	}

- firmware only nodes like (doc/usage/fit/sec_firmware_ppa.rst):

|	config-1 {
|		description = "PPA Secure firmware";
|		firmware = "firmware@1";
|		loadables = "trustedOS@1", "fuse_scr";
|	};

- overlay only nodes:

|	config-2 {
|		fdt = "fdt-overlay-img-name";
|	}

This commit adds an optional match function which can be passed to the
fit_open_configuration() to sort out config nodes which are not
interessting. E.g. the bootm code is only interested in config nodes
which do provide an "kernel" image. The new match function is only
called if no explicit configuration node name was specified.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 arch/arm/mach-layerscape/ppa.c |  2 +-
 common/bootm.c                 | 13 ++++++++++++-
 common/image-fit.c             | 20 +++++++++++++++-----
 include/image-fit.h            |  4 +++-
 4 files changed, 31 insertions(+), 8 deletions(-)

diff --git a/arch/arm/mach-layerscape/ppa.c b/arch/arm/mach-layerscape/ppa.c
index 21efaae3ab32..96877712e737 100644
--- a/arch/arm/mach-layerscape/ppa.c
+++ b/arch/arm/mach-layerscape/ppa.c
@@ -69,7 +69,7 @@ static int ppa_init(void *ppa, size_t ppa_size, void *sec_firmware_addr)
 		return PTR_ERR(fit);
 	}
 
-	conf = fit_open_configuration(fit, NULL);
+	conf = fit_open_configuration(fit, NULL, NULL);
 	if (IS_ERR(conf)) {
 		pr_err("Cannot open default config in ppa FIT image: %pe\n", conf);
 		ret = PTR_ERR(conf);
diff --git a/common/bootm.c b/common/bootm.c
index c851ab0456b8..2f1aabc3388c 100644
--- a/common/bootm.c
+++ b/common/bootm.c
@@ -554,6 +554,16 @@ static int bootm_open_os_uimage(struct image_data *data)
 	return 0;
 }
 
+static bool bootm_fit_config_valid(struct fit_handle *fit,
+				   struct device_node *config)
+{
+	/*
+	 * Consider only FIT configurations which do provide a loadable kernel
+	 * image.
+	 */
+	return !!fit_has_image(fit, config, "kernel");
+}
+
 static int bootm_open_fit(struct image_data *data)
 {
 	struct fit_handle *fit;
@@ -579,7 +589,8 @@ static int bootm_open_fit(struct image_data *data)
 	data->os_fit = fit;
 
 	data->fit_config = fit_open_configuration(data->os_fit,
-						  data->os_part);
+						  data->os_part,
+						  bootm_fit_config_valid);
 	if (IS_ERR(data->fit_config)) {
 		pr_err("Cannot open FIT image configuration '%s'\n",
 		       data->os_part ? data->os_part : "default");
diff --git a/common/image-fit.c b/common/image-fit.c
index 008804e6a6c3..24a733cd6b6e 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -717,7 +717,9 @@ static int fit_config_verify_signature(struct fit_handle *handle, struct device_
 
 static int fit_find_compatible_unit(struct fit_handle *handle,
 				    struct device_node *conf_node,
-				    const char **unit)
+				    const char **unit,
+				    bool (*config_node_valid)(struct fit_handle *handle,
+							      struct device_node *config))
 {
 	struct device_node *child = NULL;
 	struct device_node *barebox_root;
@@ -734,7 +736,12 @@ static int fit_find_compatible_unit(struct fit_handle *handle,
 		return -ENOENT;
 
 	for_each_child_of_node(conf_node, child) {
-		int score = of_device_is_compatible(child, machine);
+		int score;
+
+		if (config_node_valid && !config_node_valid(handle, child))
+			continue;
+
+		score = of_device_is_compatible(child, machine);
 
 		if (!score && !of_property_present(child, "compatible") &&
 		    of_property_present(child, "fdt")) {
@@ -802,7 +809,9 @@ static int fit_find_compatible_unit(struct fit_handle *handle,
  * Return: If successful a pointer to a valid configuration node,
  *         otherwise a ERR_PTR()
  */
-void *fit_open_configuration(struct fit_handle *handle, const char *name)
+void *fit_open_configuration(struct fit_handle *handle, const char *name,
+			     bool (*match_valid)(struct fit_handle *handle,
+						 struct device_node *config))
 {
 	struct device_node *conf_node = handle->configurations;
 	const char *unit, *desc = "(no description)";
@@ -814,7 +823,8 @@ void *fit_open_configuration(struct fit_handle *handle, const char *name)
 	if (name) {
 		unit = name;
 	} else {
-		ret = fit_find_compatible_unit(handle, conf_node, &unit);
+		ret = fit_find_compatible_unit(handle, conf_node, &unit,
+					       match_valid);
 		if (ret) {
 			pr_info("Couldn't get a valid configuration. Aborting.\n");
 			return ERR_PTR(ret);
@@ -958,7 +968,7 @@ static int do_bootm_sandbox_fit(struct image_data *data)
 	if (IS_ERR(handle))
 		return PTR_ERR(handle);
 
-	ret = fit_open_configuration(handle, data->os_part);
+	ret = fit_open_configuration(handle, data->os_part, NULL);
 	if (ret)
 		goto out;
 
diff --git a/include/image-fit.h b/include/image-fit.h
index 0b8e94bf4635..416f1f2c1896 100644
--- a/include/image-fit.h
+++ b/include/image-fit.h
@@ -26,7 +26,9 @@ struct fit_handle *fit_open(const char *filename, bool verbose,
 			    enum bootm_verify verify, loff_t max_size);
 struct fit_handle *fit_open_buf(const void *buf, size_t len, bool verbose,
 				enum bootm_verify verify);
-void *fit_open_configuration(struct fit_handle *handle, const char *name);
+void *fit_open_configuration(struct fit_handle *handle, const char *name,
+			     bool (*match_valid)(struct fit_handle *handle,
+						 struct device_node *config));
 int fit_has_image(struct fit_handle *handle, void *configuration,
 		  const char *name);
 int fit_open_image(struct fit_handle *handle, void *configuration,

-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 03/10] of: overlay: make the pattern match function more generic
  2024-07-03 16:58 [PATCH v3 00/10] Add FIT image overlay support Marco Felsch
  2024-07-03 16:58 ` [PATCH v3 01/10] FIT: fix missing free in fit_open error path Marco Felsch
  2024-07-03 16:58 ` [PATCH v3 02/10] FIT: fit_open_configuration: add match function support Marco Felsch
@ 2024-07-03 16:58 ` Marco Felsch
  2024-07-03 16:58 ` [PATCH v3 04/10] of: overlay: make search dir/path " Marco Felsch
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Marco Felsch @ 2024-07-03 16:58 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Marco Felsch

The current overlay mechanism can handle files only, so filepattern was
obvious. With the next commit this will change, so overlays can also
supplied via FIT images. To prepare the pattern filter to match FIT
config-nodes make the filter and global variable handling the pattern
more generic by dropping the "file" prefix.

Keep the backward compatibility by still providing the filepattern
filter and the global of.overlay.filepattern variable but notice the
user that the usage of those are deprecated.

No functional change.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/of/overlay.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 73c7a91db9b5..f0588076ff10 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -394,7 +394,7 @@ int of_register_overlay(struct device_node *overlay)
 	return of_register_fixup(of_overlay_fixup, overlay);
 }
 
-static char *of_overlay_filepattern;
+static char *of_overlay_pattern;
 static char *of_overlay_dir;
 static char *of_overlay_basedir;
 
@@ -491,10 +491,10 @@ int of_overlay_register_filter(struct of_overlay_filter *filter)
 }
 
 /**
- * of_overlay_filter_filename - A filter that matches on the filename of
+ * of_overlay_filter_pattern - A filter that matches on the filename or
  *                                an overlay
  * @f: The filter
- * @filename: The filename of the overlay
+ * @pattern: The filename of the overlay
  *
  * This filter matches when the filename matches one of the patterns given
  * in global.of.overlay.filepattern. global.of.overlay.filepattern shall
@@ -502,20 +502,20 @@ int of_overlay_register_filter(struct of_overlay_filter *filter)
  *
  * @return: True when the overlay shall be applied, false otherwise.
  */
-static bool of_overlay_filter_filename(struct of_overlay_filter *f,
-				       const char *filename)
+static bool of_overlay_filter_pattern(struct of_overlay_filter *f,
+				      const char *pattern)
 {
 	char *p, *path, *n;
 	int ret;
 	bool apply;
 
-	p = path = strdup(of_overlay_filepattern);
+	p = path = strdup(of_overlay_pattern);
 
 	while ((n = strsep_unescaped(&p, " "))) {
 		if (!*n)
 			continue;
 
-		ret = fnmatch(n, filename, 0);
+		ret = fnmatch(n, pattern, 0);
 
 		if (!ret) {
 			apply = true;
@@ -530,6 +530,18 @@ static bool of_overlay_filter_filename(struct of_overlay_filter *f,
 	return apply;
 }
 
+static struct of_overlay_filter of_overlay_pattern_filter = {
+	.name = "pattern",
+	.filter_filename = of_overlay_filter_pattern,
+};
+
+static bool of_overlay_filter_filename(struct of_overlay_filter *f,
+				       const char *filename)
+{
+	pr_warn("'filepattern' filter is marked as deprecated, convert to 'pattern' filter\n");
+	return of_overlay_filter_pattern(f, filename);
+}
+
 static struct of_overlay_filter of_overlay_filepattern_filter = {
 	.name = "filepattern",
 	.filter_filename = of_overlay_filter_filename,
@@ -584,15 +596,18 @@ static struct of_overlay_filter of_overlay_compatible_filter = {
 
 static int of_overlay_init(void)
 {
-	of_overlay_filepattern = strdup("*");
-	of_overlay_filter = strdup("filepattern compatible");
+	of_overlay_pattern = strdup("*");
+	of_overlay_filter = strdup("pattern compatible");
 	of_overlay_set_basedir("/");
 
 	globalvar_add_simple_string("of.overlay.compatible", &of_overlay_compatible);
-	globalvar_add_simple_string("of.overlay.filepattern", &of_overlay_filepattern);
+	globalvar_add_simple_string("of.overlay.pattern", &of_overlay_pattern);
 	globalvar_add_simple_string("of.overlay.filter", &of_overlay_filter);
 	globalvar_add_simple_string("of.overlay.dir", &of_overlay_dir);
 
+	globalvar_alias_deprecated("of.overlay.filepattern", "of.overlay.pattern");
+
+	of_overlay_register_filter(&of_overlay_pattern_filter);
 	of_overlay_register_filter(&of_overlay_filepattern_filter);
 	of_overlay_register_filter(&of_overlay_compatible_filter);
 
@@ -603,6 +618,6 @@ static int of_overlay_init(void)
 device_initcall(of_overlay_init);
 
 BAREBOX_MAGICVAR(global.of.overlay.compatible, "space separated list of compatibles an overlay must match");
-BAREBOX_MAGICVAR(global.of.overlay.filepattern, "space separated list of filepatterns an overlay must match");
+BAREBOX_MAGICVAR(global.of.overlay.pattern, "space separated list of filepatterns an overlay must match");
 BAREBOX_MAGICVAR(global.of.overlay.dir, "Directory to look for dt overlays");
 BAREBOX_MAGICVAR(global.of.overlay.filter, "space separated list of filters");

-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 04/10] of: overlay: make search dir/path more generic
  2024-07-03 16:58 [PATCH v3 00/10] Add FIT image overlay support Marco Felsch
                   ` (2 preceding siblings ...)
  2024-07-03 16:58 ` [PATCH v3 03/10] of: overlay: make the pattern match function more generic Marco Felsch
@ 2024-07-03 16:58 ` Marco Felsch
  2024-07-03 16:58 ` [PATCH v3 05/10] FIT: expose useful helpers Marco Felsch
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Marco Felsch @ 2024-07-03 16:58 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Marco Felsch

In preparation of adding support for FIT image overlay handling the
global.of.overlay.dir can be reused if we rename it to a more generic
global.of.overlay.path since it does not imply an directory.

This commit adds the ground work by deprecating the old
global.of.overlay.dir and moving the directory handling into a separate
of_overlay_global_fixup_dir() helper.

No functional change.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/of/overlay.c | 22 ++++++++++++++--------
 1 file changed, 14 insertions(+), 8 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index f0588076ff10..5617f322ddca 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -395,7 +395,7 @@ int of_register_overlay(struct device_node *overlay)
 }
 
 static char *of_overlay_pattern;
-static char *of_overlay_dir;
+static char *of_overlay_path;
 static char *of_overlay_basedir;
 
 /**
@@ -450,18 +450,18 @@ static int of_overlay_apply_dir(struct device_node *root, const char *dirname,
 	return ret;
 }
 
-static int of_overlay_global_fixup(struct device_node *root, void *data)
+static int of_overlay_global_fixup_dir(struct device_node *root, const char *ovl_dir)
 {
 	char *dir;
 	int ret;
 
-	if (*of_overlay_dir == '/')
-		return of_overlay_apply_dir(root, of_overlay_dir, true);
+	if (*ovl_dir == '/')
+		return of_overlay_apply_dir(root, ovl_dir, true);
 
-	if (*of_overlay_dir == '\0')
+	if (*ovl_dir == '\0')
 		return 0;
 
-	dir = concat_path_file(of_overlay_basedir, of_overlay_dir);
+	dir = concat_path_file(of_overlay_basedir, ovl_dir);
 
 	ret = of_overlay_apply_dir(root, dir, true);
 
@@ -470,6 +470,11 @@ static int of_overlay_global_fixup(struct device_node *root, void *data)
 	return ret;
 }
 
+static int of_overlay_global_fixup(struct device_node *root, void *data)
+{
+	return of_overlay_global_fixup_dir(root, of_overlay_path);
+}
+
 /**
  * of_overlay_register_filter - register a new overlay filter
  * @filter: The new filter
@@ -603,9 +608,10 @@ static int of_overlay_init(void)
 	globalvar_add_simple_string("of.overlay.compatible", &of_overlay_compatible);
 	globalvar_add_simple_string("of.overlay.pattern", &of_overlay_pattern);
 	globalvar_add_simple_string("of.overlay.filter", &of_overlay_filter);
-	globalvar_add_simple_string("of.overlay.dir", &of_overlay_dir);
+	globalvar_add_simple_string("of.overlay.path", &of_overlay_path);
 
 	globalvar_alias_deprecated("of.overlay.filepattern", "of.overlay.pattern");
+	globalvar_alias_deprecated("of.overlay.dir", "of.overlay.path");
 
 	of_overlay_register_filter(&of_overlay_pattern_filter);
 	of_overlay_register_filter(&of_overlay_filepattern_filter);
@@ -619,5 +625,5 @@ device_initcall(of_overlay_init);
 
 BAREBOX_MAGICVAR(global.of.overlay.compatible, "space separated list of compatibles an overlay must match");
 BAREBOX_MAGICVAR(global.of.overlay.pattern, "space separated list of filepatterns an overlay must match");
-BAREBOX_MAGICVAR(global.of.overlay.dir, "Directory to look for dt overlays");
+BAREBOX_MAGICVAR(global.of.overlay.path, "Path to look for dt overlays");
 BAREBOX_MAGICVAR(global.of.overlay.filter, "space separated list of filters");

-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 05/10] FIT: expose useful helpers
  2024-07-03 16:58 [PATCH v3 00/10] Add FIT image overlay support Marco Felsch
                   ` (3 preceding siblings ...)
  2024-07-03 16:58 ` [PATCH v3 04/10] of: overlay: make search dir/path " Marco Felsch
@ 2024-07-03 16:58 ` Marco Felsch
  2024-07-03 16:58 ` [PATCH v3 06/10] of: overlay: add FIT overlay support Marco Felsch
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Marco Felsch @ 2024-07-03 16:58 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Marco Felsch

The upcoming FIT overlay support requires these helpers for performing
check on a fit image.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 common/image-fit.c  | 2 +-
 include/image-fit.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/common/image-fit.c b/common/image-fit.c
index 24a733cd6b6e..9480d7963f2a 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -675,7 +675,7 @@ int fit_open_image(struct fit_handle *handle, void *configuration,
 	return 0;
 }
 
-static int fit_config_verify_signature(struct fit_handle *handle, struct device_node *conf_node)
+int fit_config_verify_signature(struct fit_handle *handle, struct device_node *conf_node)
 {
 	struct device_node *sig_node;
 	int ret = -EINVAL;
diff --git a/include/image-fit.h b/include/image-fit.h
index 416f1f2c1896..f8321aa87e11 100644
--- a/include/image-fit.h
+++ b/include/image-fit.h
@@ -37,6 +37,7 @@ int fit_open_image(struct fit_handle *handle, void *configuration,
 int fit_get_image_address(struct fit_handle *handle, void *configuration,
 			  const char *name, const char *property,
 			  unsigned long *address);
+int fit_config_verify_signature(struct fit_handle *handle, struct device_node *conf_node);
 
 void fit_close(struct fit_handle *handle);
 

-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 06/10] of: overlay: add FIT overlay support
  2024-07-03 16:58 [PATCH v3 00/10] Add FIT image overlay support Marco Felsch
                   ` (4 preceding siblings ...)
  2024-07-03 16:58 ` [PATCH v3 05/10] FIT: expose useful helpers Marco Felsch
@ 2024-07-03 16:58 ` Marco Felsch
  2024-07-03 16:58 ` [PATCH v3 07/10] of: overlay: drop unnecessary empty check in of_overlay_global_fixup_dir Marco Felsch
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Marco Felsch @ 2024-07-03 16:58 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Marco Felsch

This adds the support to load devicetree overlays from an FIT image.
There are quite a few options to handle FIT overlays since the FIT
overlay spec is not very strict.

This patch implement the most configurable case where each overlay does
have it's own config node (including the optional signature).

- The "name" filter check is performed on the config-node name (the node
  under the configurations) and not the FIT overlay image name (the node
  name under the images node).
- The "content" filter check does not differ from the file based overlay
  handling.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/of/overlay.c | 131 ++++++++++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 124 insertions(+), 7 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 5617f322ddca..a980e7aa5e02 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -8,10 +8,13 @@
  */
 #define pr_fmt(fmt) "of_overlay: " fmt
 
+#include <bootm.h>
 #include <common.h>
 #include <of.h>
 #include <errno.h>
+#include <filetype.h>
 #include <globalvar.h>
+#include <image-fit.h>
 #include <magicvar.h>
 #include <string.h>
 #include <libfile.h>
@@ -470,9 +473,123 @@ static int of_overlay_global_fixup_dir(struct device_node *root, const char *ovl
 	return ret;
 }
 
+static int of_overlay_apply_fit(struct device_node *root, struct fit_handle *fit,
+				struct device_node *config)
+{
+	const char *name = config->name;
+	struct device_node *overlay;
+	unsigned long ovl_sz;
+	const void *ovl;
+	int ret;
+
+	if (!of_overlay_matches_filter(name, NULL))
+		return 0;
+
+	ret = fit_open_image(fit, config, "fdt", &ovl, &ovl_sz);
+	if (ret)
+		return ret;
+
+	overlay = of_unflatten_dtb(ovl, ovl_sz);
+
+	if (!of_overlay_matches_filter(NULL, overlay)) {
+		ret = 0;
+		goto out;
+	}
+
+	ret = of_overlay_apply_tree(root, overlay);
+	if (ret == -ENODEV)
+		pr_debug("Not applied %s (not compatible)\n", name);
+	else if (ret)
+		pr_err("Cannot apply %s: %s\n", name, strerror(-ret));
+	else
+		pr_info("Applied %s\n", name);
+
+out:
+	of_delete_node(overlay);
+
+	return ret;
+}
+
+static bool of_overlay_valid_config(struct fit_handle *fit,
+				    struct device_node *config)
+{
+	/*
+	 * Either kernel or firmware is marked as mandatory by U-Boot
+	 * (doc/usage/fit/source_file_format.rst) except for overlays
+	 * (doc/usage/fit/overlay-fdt-boot.rst). Therefore we need to ensure
+	 * that only "fdt" config nodes are recognized as overlay config node.
+	 */
+	if (!fit_has_image(fit, config, "fdt") ||
+	    fit_has_image(fit, config, "kernel") ||
+	    fit_has_image(fit, config, "firmware"))
+		return false;
+
+	return true;
+}
+
+static int of_overlay_global_fixup_fit(struct device_node *root,
+				       const char *fit_path, loff_t fit_size)
+{
+	enum bootm_verify verify = bootm_get_verify_mode();
+	struct device_node *conf_node;
+	struct fit_handle *fit;
+	int ret;
+
+	if (!IS_ENABLED(CONFIG_FITIMAGE))
+		return 0;
+
+	fit = fit_open(fit_path, 0, verify, fit_size);
+	if (IS_ERR(fit)) {
+		pr_err("Loading FIT image %s failed with: %pe\n", fit_path, fit);
+		return PTR_ERR(fit);
+	}
+
+	for_each_child_of_node(fit->configurations, conf_node) {
+		if (!of_overlay_valid_config(fit, conf_node))
+			continue;
+
+		ret = fit_config_verify_signature(fit, conf_node);
+		if (ret)
+			goto out;
+
+		ret = of_overlay_apply_fit(root, fit, conf_node);
+		if (ret)
+			goto out;
+	}
+
+out:
+	fit_close(fit);
+	return ret;
+}
+
 static int of_overlay_global_fixup(struct device_node *root, void *data)
 {
-	return of_overlay_global_fixup_dir(root, of_overlay_path);
+	enum filetype type;
+	struct stat s;
+	int ret;
+
+	if (isempty(of_overlay_path))
+		return 0;
+
+	if (stat(of_overlay_path, &s)) {
+		pr_err("Failed to detect file status\n");
+		return -errno;
+	}
+
+	if (S_ISDIR(s.st_mode))
+		return of_overlay_global_fixup_dir(root, of_overlay_path);
+
+	ret = file_name_detect_type(of_overlay_path, &type);
+	if (ret)
+		return ret;
+
+	if (type == filetype_oftree)
+		return of_overlay_global_fixup_fit(root, of_overlay_path,
+						   s.st_size);
+
+	pr_err("No suitable overlay provider found!\n");
+
+	return -EINVAL;
 }
 
 /**
@@ -497,13 +614,13 @@ int of_overlay_register_filter(struct of_overlay_filter *filter)
 
 /**
  * of_overlay_filter_pattern - A filter that matches on the filename or
- *                                an overlay
+ *			       FIT config-node name of an overlay
  * @f: The filter
- * @pattern: The filename of the overlay
+ * @pattern: The filename or FIT config-node name of the overlay
  *
- * This filter matches when the filename matches one of the patterns given
- * in global.of.overlay.filepattern. global.of.overlay.filepattern shall
- * contain a space separated list of wildcard patterns.
+ * This filter matches when the filename or FIT config-node name matches one of
+ * the patterns given in global.of.overlay.pattern. global.of.overlay.pattern
+ * shall contain a space separated list of wildcard patterns.
  *
  * @return: True when the overlay shall be applied, false otherwise.
  */
@@ -624,6 +741,6 @@ static int of_overlay_init(void)
 device_initcall(of_overlay_init);
 
 BAREBOX_MAGICVAR(global.of.overlay.compatible, "space separated list of compatibles an overlay must match");
-BAREBOX_MAGICVAR(global.of.overlay.pattern, "space separated list of filepatterns an overlay must match");
+BAREBOX_MAGICVAR(global.of.overlay.pattern, "space separated list of filename or fit config-node name patterns an overlay must match");
 BAREBOX_MAGICVAR(global.of.overlay.path, "Path to look for dt overlays");
 BAREBOX_MAGICVAR(global.of.overlay.filter, "space separated list of filters");

-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 07/10] of: overlay: drop unnecessary empty check in of_overlay_global_fixup_dir
  2024-07-03 16:58 [PATCH v3 00/10] Add FIT image overlay support Marco Felsch
                   ` (5 preceding siblings ...)
  2024-07-03 16:58 ` [PATCH v3 06/10] of: overlay: add FIT overlay support Marco Felsch
@ 2024-07-03 16:58 ` Marco Felsch
  2024-07-03 16:58 ` [PATCH v3 08/10] of: overlay: replace filename with an more unique name Marco Felsch
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 14+ messages in thread
From: Marco Felsch @ 2024-07-03 16:58 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Marco Felsch

This is now done during of_overlay_global_fixup() so we can drop it
here.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/of/overlay.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index a980e7aa5e02..822487908ede 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -461,9 +461,6 @@ static int of_overlay_global_fixup_dir(struct device_node *root, const char *ovl
 	if (*ovl_dir == '/')
 		return of_overlay_apply_dir(root, ovl_dir, true);
 
-	if (*ovl_dir == '\0')
-		return 0;
-
 	dir = concat_path_file(of_overlay_basedir, ovl_dir);
 
 	ret = of_overlay_apply_dir(root, dir, true);

-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 08/10] of: overlay: replace filename with an more unique name
  2024-07-03 16:58 [PATCH v3 00/10] Add FIT image overlay support Marco Felsch
                   ` (6 preceding siblings ...)
  2024-07-03 16:58 ` [PATCH v3 07/10] of: overlay: drop unnecessary empty check in of_overlay_global_fixup_dir Marco Felsch
@ 2024-07-03 16:58 ` Marco Felsch
  2024-07-03 16:58 ` [PATCH v3 09/10] FIT: save filename during fit_open Marco Felsch
  2024-07-03 16:58 ` [PATCH v3 10/10] FIT: add support to cache opened fit images Marco Felsch
  9 siblings, 0 replies; 14+ messages in thread
From: Marco Felsch @ 2024-07-03 16:58 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Marco Felsch

Since we do support FIT image overlays and file/dir based overlays the
filename variable is not sufficient anylonger. Therefore rename the
local variables as well as the filter function to hook.

To not break any systems keep the old filename based name too but mark
them as deprecated.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 drivers/of/overlay.c | 39 +++++++++++++++++++++------------------
 include/of.h         |  3 ++-
 2 files changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
index 822487908ede..6b76eafc966e 100644
--- a/drivers/of/overlay.c
+++ b/drivers/of/overlay.c
@@ -234,12 +234,12 @@ static struct of_overlay_filter *of_overlay_find_filter(const char *name)
 	return NULL;
 }
 
-static bool of_overlay_matches_filter(const char *filename, struct device_node *ovl)
+static bool of_overlay_matches_filter(const char *pattern, struct device_node *ovl)
 {
 	struct of_overlay_filter *filter;
 	char *p, *path, *n;
 	bool apply = false;
-	bool have_filename_filter = false;
+	bool have_pattern_filter = false;
 	bool have_content_filter = false;
 
 	p = path = strdup(of_overlay_filter);
@@ -256,14 +256,16 @@ static bool of_overlay_matches_filter(const char *filename, struct device_node *
 			continue;
 		}
 
-		if (filter->filter_filename)
-			have_filename_filter = true;
+		if (filter->filter_pattern || filter->filter_filename)
+			have_pattern_filter = true;
 		if (filter->filter_content)
 			have_content_filter = true;
 
-		if (filename) {
-			if (filter->filter_filename &&
-			    filter->filter_filename(filter, kbasename(filename)))
+		if (pattern) {
+			if ((filter->filter_pattern &&
+			     filter->filter_pattern(filter, kbasename(pattern))) ||
+			    (filter->filter_filename &&
+			     filter->filter_filename(filter, kbasename(pattern))))
 				score++;
 		} else {
 			score++;
@@ -286,11 +288,11 @@ static bool of_overlay_matches_filter(const char *filename, struct device_node *
 	free(path);
 
 	/* No filter found at all, no match */
-	if (!have_filename_filter && !have_content_filter)
+	if (!have_pattern_filter && !have_content_filter)
 		return false;
 
-	/* Want to match filename, but we do not have a filename_filter */
-	if (filename && !have_filename_filter)
+	/* Want to match pattern, but we do not have a filename_filter */
+	if (pattern && !have_pattern_filter)
 		return true;
 
 	/* Want to match content, but we do not have a content_filter */
@@ -298,12 +300,12 @@ static bool of_overlay_matches_filter(const char *filename, struct device_node *
 		return true;
 
 	if (apply)
-		pr_debug("filename %s, overlay %p: match against filter %s\n",
-			 filename ?: "<NONE>",
+		pr_debug("pattern %s, overlay %p: match against filter %s\n",
+			 pattern ?: "<NONE>",
 			 ovl, filter->name);
 	else
-		pr_debug("filename %s, overlay %p: no match\n",
-			 filename ?: "<NONE>", ovl);
+		pr_debug("pattern %s, overlay %p: no match\n",
+			 pattern ?: "<NONE>", ovl);
 
 	return apply;
 }
@@ -594,14 +596,15 @@ static int of_overlay_global_fixup(struct device_node *root, void *data)
  * @filter: The new filter
  *
  * Register a new overlay filter. A filter can either match on
- * the filename or on the content of an overlay, but not on both.
+ * a pattern or on the content of an overlay, but not on both.
  * If that's desired two filters have to be registered.
  *
  * @return: 0 for success, negative error code otherwise
  */
 int of_overlay_register_filter(struct of_overlay_filter *filter)
 {
-	if (filter->filter_filename && filter->filter_content)
+	if ((filter->filter_pattern || filter->filter_filename) &&
+	    filter->filter_content)
 		return -EINVAL;
 
 	list_add_tail(&filter->list, &of_overlay_filters);
@@ -651,7 +654,7 @@ static bool of_overlay_filter_pattern(struct of_overlay_filter *f,
 
 static struct of_overlay_filter of_overlay_pattern_filter = {
 	.name = "pattern",
-	.filter_filename = of_overlay_filter_pattern,
+	.filter_pattern = of_overlay_filter_pattern,
 };
 
 static bool of_overlay_filter_filename(struct of_overlay_filter *f,
@@ -663,7 +666,7 @@ static bool of_overlay_filter_filename(struct of_overlay_filter *f,
 
 static struct of_overlay_filter of_overlay_filepattern_filter = {
 	.name = "filepattern",
-	.filter_filename = of_overlay_filter_filename,
+	.filter_pattern = of_overlay_filter_filename,
 };
 
 /**
diff --git a/include/of.h b/include/of.h
index f99cff5cf54a..34cb535e1924 100644
--- a/include/of.h
+++ b/include/of.h
@@ -1298,7 +1298,8 @@ static inline struct device_node *of_find_root_node(struct device_node *node)
 }
 
 struct of_overlay_filter {
-	bool (*filter_filename)(struct of_overlay_filter *, const char *filename);
+	bool (*filter_filename)(struct of_overlay_filter *, const char *filename); /* deprecated */
+	bool (*filter_pattern)(struct of_overlay_filter *, const char *pattern);
 	bool (*filter_content)(struct of_overlay_filter *, struct device_node *);
 	char *name;
 	struct list_head list;

-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 09/10] FIT: save filename during fit_open
  2024-07-03 16:58 [PATCH v3 00/10] Add FIT image overlay support Marco Felsch
                   ` (7 preceding siblings ...)
  2024-07-03 16:58 ` [PATCH v3 08/10] of: overlay: replace filename with an more unique name Marco Felsch
@ 2024-07-03 16:58 ` Marco Felsch
  2024-07-03 16:58 ` [PATCH v3 10/10] FIT: add support to cache opened fit images Marco Felsch
  9 siblings, 0 replies; 14+ messages in thread
From: Marco Felsch @ 2024-07-03 16:58 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Marco Felsch

This is in preparation of buffering fit_open() calls to not load the
same fit twice if it is still open.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 common/image-fit.c  | 2 ++
 include/image-fit.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/common/image-fit.c b/common/image-fit.c
index 9480d7963f2a..4839d8dd33a4 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -937,6 +937,7 @@ struct fit_handle *fit_open(const char *filename, bool verbose,
 	}
 
 	handle->fit = handle->fit_alloc;
+	handle->filename = xstrdup(filename);
 
 	ret = fit_do_open(handle);
 	if (ret) {
@@ -952,6 +953,7 @@ void fit_close(struct fit_handle *handle)
 	if (handle->root)
 		of_delete_node(handle->root);
 
+	free(handle->filename);
 	free(handle->fit_alloc);
 	free(handle);
 }
diff --git a/include/image-fit.h b/include/image-fit.h
index f8321aa87e11..68f70f4365cb 100644
--- a/include/image-fit.h
+++ b/include/image-fit.h
@@ -13,6 +13,7 @@ struct fit_handle {
 	const void *fit;
 	void *fit_alloc;
 	size_t size;
+	char *filename;
 
 	bool verbose;
 	enum bootm_verify verify;

-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v3 10/10] FIT: add support to cache opened fit images
  2024-07-03 16:58 [PATCH v3 00/10] Add FIT image overlay support Marco Felsch
                   ` (8 preceding siblings ...)
  2024-07-03 16:58 ` [PATCH v3 09/10] FIT: save filename during fit_open Marco Felsch
@ 2024-07-03 16:58 ` Marco Felsch
  2024-07-03 18:46   ` Ahmad Fatoum
  9 siblings, 1 reply; 14+ messages in thread
From: Marco Felsch @ 2024-07-03 16:58 UTC (permalink / raw)
  To: Sascha Hauer, BAREBOX; +Cc: Marco Felsch

Cache the FIT image fit_open() calls to avoid loading the same FIT image
twice. This is very useful if the same FIT image is used to provide the
base devicetree, kernel and initrd as well as devicetree overlays.

Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
---
 common/image-fit.c  | 32 ++++++++++++++++++++++++++++++++
 include/image-fit.h |  4 ++++
 2 files changed, 36 insertions(+)

diff --git a/common/image-fit.c b/common/image-fit.c
index 4839d8dd33a4..a75bb79d1761 100644
--- a/common/image-fit.c
+++ b/common/image-fit.c
@@ -16,6 +16,7 @@
 #include <fs.h>
 #include <malloc.h>
 #include <linux/ctype.h>
+#include <linux/refcount.h>
 #include <asm/byteorder.h>
 #include <errno.h>
 #include <linux/err.h>
@@ -32,6 +33,8 @@
 #define CHECK_LEVEL_SIG 2
 #define CHECK_LEVEL_MAX 3
 
+static LIST_HEAD(open_fits);
+
 static uint32_t dt_struct_advance(struct fdt_header *f, uint32_t dt, int size)
 {
 	dt += size;
@@ -847,6 +850,18 @@ void *fit_open_configuration(struct fit_handle *handle, const char *name,
 	return conf_node;
 }
 
+static struct fit_handle *fit_get_handle(const char *filename)
+{
+	struct fit_handle *handle;
+
+	list_for_each_entry(handle, &open_fits, entry) {
+		if (!strcmp(filename, handle->filename))
+			return handle;
+	}
+
+	return NULL;
+}
+
 static int fit_do_open(struct fit_handle *handle)
 {
 	const char *desc = "(no description)";
@@ -896,6 +911,8 @@ struct fit_handle *fit_open_buf(const void *buf, size_t size, bool verbose,
 	handle->size = size;
 	handle->verify = verify;
 
+	refcount_set(&handle->users, 1);
+
 	ret = fit_do_open(handle);
 	if (ret) {
 		fit_close(handle);
@@ -923,6 +940,12 @@ struct fit_handle *fit_open(const char *filename, bool verbose,
 	struct fit_handle *handle;
 	int ret;
 
+	handle = fit_get_handle(filename);
+	if (handle) {
+		refcount_inc(&handle->users);
+		return handle;
+	}
+
 	handle = xzalloc(sizeof(struct fit_handle));
 
 	handle->verbose = verbose;
@@ -939,6 +962,9 @@ struct fit_handle *fit_open(const char *filename, bool verbose,
 	handle->fit = handle->fit_alloc;
 	handle->filename = xstrdup(filename);
 
+	refcount_set(&handle->users, 1);
+	list_add(&handle->entry, &open_fits);
+
 	ret = fit_do_open(handle);
 	if (ret) {
 		fit_close(handle);
@@ -950,9 +976,15 @@ struct fit_handle *fit_open(const char *filename, bool verbose,
 
 void fit_close(struct fit_handle *handle)
 {
+	if (!refcount_dec_and_test(&handle->users))
+		return;
+
 	if (handle->root)
 		of_delete_node(handle->root);
 
+	if (handle->filename)
+		list_del(&handle->entry);
+
 	free(handle->filename);
 	free(handle->fit_alloc);
 	free(handle);
diff --git a/include/image-fit.h b/include/image-fit.h
index 68f70f4365cb..f9791ff251c5 100644
--- a/include/image-fit.h
+++ b/include/image-fit.h
@@ -7,6 +7,7 @@
 #define __IMAGE_FIT_H__
 
 #include <linux/types.h>
+#include <linux/refcount.h>
 #include <bootm.h>
 
 struct fit_handle {
@@ -15,6 +16,9 @@ struct fit_handle {
 	size_t size;
 	char *filename;
 
+	struct list_head entry;
+	refcount_t users;
+
 	bool verbose;
 	enum bootm_verify verify;
 

-- 
2.39.2




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 01/10] FIT: fix missing free in fit_open error path
  2024-07-03 16:58 ` [PATCH v3 01/10] FIT: fix missing free in fit_open error path Marco Felsch
@ 2024-07-03 18:30   ` Ahmad Fatoum
  0 siblings, 0 replies; 14+ messages in thread
From: Ahmad Fatoum @ 2024-07-03 18:30 UTC (permalink / raw)
  To: Marco Felsch, Sascha Hauer, BAREBOX

On 03.07.24 18:58, Marco Felsch wrote:
> Free the handle if read_file_2() fails.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>

Reviewed-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

> ---
>  common/image-fit.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/common/image-fit.c b/common/image-fit.c
> index 251fda97b3fc..008804e6a6c3 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -922,6 +922,7 @@ struct fit_handle *fit_open(const char *filename, bool verbose,
>  			  max_size);
>  	if (ret && ret != -EFBIG) {
>  		pr_err("unable to read %s: %s\n", filename, strerror(-ret));
> +		free(handle);
>  		return ERR_PTR(ret);
>  	}
>  
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 10/10] FIT: add support to cache opened fit images
  2024-07-03 16:58 ` [PATCH v3 10/10] FIT: add support to cache opened fit images Marco Felsch
@ 2024-07-03 18:46   ` Ahmad Fatoum
  2024-07-04  8:57     ` Marco Felsch
  0 siblings, 1 reply; 14+ messages in thread
From: Ahmad Fatoum @ 2024-07-03 18:46 UTC (permalink / raw)
  To: Marco Felsch, Sascha Hauer, BAREBOX

Hello Marco,

On 03.07.24 18:58, Marco Felsch wrote:
> Cache the FIT image fit_open() calls to avoid loading the same FIT image
> twice. This is very useful if the same FIT image is used to provide the
> base devicetree, kernel and initrd as well as devicetree overlays.
> 
> Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> ---
>  common/image-fit.c  | 32 ++++++++++++++++++++++++++++++++
>  include/image-fit.h |  4 ++++
>  2 files changed, 36 insertions(+)
> 
> diff --git a/common/image-fit.c b/common/image-fit.c
> index 4839d8dd33a4..a75bb79d1761 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -16,6 +16,7 @@
>  #include <fs.h>
>  #include <malloc.h>
>  #include <linux/ctype.h>
> +#include <linux/refcount.h>
>  #include <asm/byteorder.h>
>  #include <errno.h>
>  #include <linux/err.h>
> @@ -32,6 +33,8 @@
>  #define CHECK_LEVEL_SIG 2
>  #define CHECK_LEVEL_MAX 3
>  
> +static LIST_HEAD(open_fits);
> +
>  static uint32_t dt_struct_advance(struct fdt_header *f, uint32_t dt, int size)
>  {
>  	dt += size;
> @@ -847,6 +850,18 @@ void *fit_open_configuration(struct fit_handle *handle, const char *name,
>  	return conf_node;
>  }
>  
> +static struct fit_handle *fit_get_handle(const char *filename)
> +{
> +	struct fit_handle *handle;
> +
> +	list_for_each_entry(handle, &open_fits, entry) {
> +		if (!strcmp(filename, handle->filename))

This wouldn't handle identical relative file names correctly, right?
You'll need to make the filename absolute, I think.

> +			return handle;
> +	}
> +
> +	return NULL;
> +}
> +
>  static int fit_do_open(struct fit_handle *handle)
>  {
>  	const char *desc = "(no description)";
> @@ -896,6 +911,8 @@ struct fit_handle *fit_open_buf(const void *buf, size_t size, bool verbose,
>  	handle->size = size;
>  	handle->verify = verify;
>  
> +	refcount_set(&handle->users, 1);
> +
>  	ret = fit_do_open(handle);
>  	if (ret) {
>  		fit_close(handle);
> @@ -923,6 +940,12 @@ struct fit_handle *fit_open(const char *filename, bool verbose,
>  	struct fit_handle *handle;
>  	int ret;
>  
> +	handle = fit_get_handle(filename);
> +	if (handle) {
> +		refcount_inc(&handle->users);
> +		return handle;
> +	}
> +
>  	handle = xzalloc(sizeof(struct fit_handle));
>  
>  	handle->verbose = verbose;
> @@ -939,6 +962,9 @@ struct fit_handle *fit_open(const char *filename, bool verbose,
>  	handle->fit = handle->fit_alloc;
>  	handle->filename = xstrdup(filename);
>  
> +	refcount_set(&handle->users, 1);
> +	list_add(&handle->entry, &open_fits);
> +
>  	ret = fit_do_open(handle);
>  	if (ret) {
>  		fit_close(handle);
> @@ -950,9 +976,15 @@ struct fit_handle *fit_open(const char *filename, bool verbose,
>  
>  void fit_close(struct fit_handle *handle)
>  {
> +	if (!refcount_dec_and_test(&handle->users))
> +		return;
> +
>  	if (handle->root)
>  		of_delete_node(handle->root);
>  
> +	if (handle->filename)
> +		list_del(&handle->entry);
> +
>  	free(handle->filename);
>  	free(handle->fit_alloc);
>  	free(handle);
> diff --git a/include/image-fit.h b/include/image-fit.h
> index 68f70f4365cb..f9791ff251c5 100644
> --- a/include/image-fit.h
> +++ b/include/image-fit.h
> @@ -7,6 +7,7 @@
>  #define __IMAGE_FIT_H__
>  
>  #include <linux/types.h>
> +#include <linux/refcount.h>
>  #include <bootm.h>
>  
>  struct fit_handle {
> @@ -15,6 +16,9 @@ struct fit_handle {
>  	size_t size;
>  	char *filename;
>  
> +	struct list_head entry;
> +	refcount_t users;
> +
>  	bool verbose;
>  	enum bootm_verify verify;
>  
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v3 10/10] FIT: add support to cache opened fit images
  2024-07-03 18:46   ` Ahmad Fatoum
@ 2024-07-04  8:57     ` Marco Felsch
  0 siblings, 0 replies; 14+ messages in thread
From: Marco Felsch @ 2024-07-04  8:57 UTC (permalink / raw)
  To: Ahmad Fatoum; +Cc: BAREBOX

Hi Ahmad,

On 24-07-03, Ahmad Fatoum wrote:
> Hello Marco,
> 
> On 03.07.24 18:58, Marco Felsch wrote:
> > Cache the FIT image fit_open() calls to avoid loading the same FIT image
> > twice. This is very useful if the same FIT image is used to provide the
> > base devicetree, kernel and initrd as well as devicetree overlays.
> > 
> > Signed-off-by: Marco Felsch <m.felsch@pengutronix.de>
> > ---
> >  common/image-fit.c  | 32 ++++++++++++++++++++++++++++++++
> >  include/image-fit.h |  4 ++++
> >  2 files changed, 36 insertions(+)
> > 
> > diff --git a/common/image-fit.c b/common/image-fit.c
> > index 4839d8dd33a4..a75bb79d1761 100644
> > --- a/common/image-fit.c
> > +++ b/common/image-fit.c
> > @@ -16,6 +16,7 @@
> >  #include <fs.h>
> >  #include <malloc.h>
> >  #include <linux/ctype.h>
> > +#include <linux/refcount.h>
> >  #include <asm/byteorder.h>
> >  #include <errno.h>
> >  #include <linux/err.h>
> > @@ -32,6 +33,8 @@
> >  #define CHECK_LEVEL_SIG 2
> >  #define CHECK_LEVEL_MAX 3
> >  
> > +static LIST_HEAD(open_fits);
> > +
> >  static uint32_t dt_struct_advance(struct fdt_header *f, uint32_t dt, int size)
> >  {
> >  	dt += size;
> > @@ -847,6 +850,18 @@ void *fit_open_configuration(struct fit_handle *handle, const char *name,
> >  	return conf_node;
> >  }
> >  
> > +static struct fit_handle *fit_get_handle(const char *filename)
> > +{
> > +	struct fit_handle *handle;
> > +
> > +	list_for_each_entry(handle, &open_fits, entry) {
> > +		if (!strcmp(filename, handle->filename))
> 
> This wouldn't handle identical relative file names correctly, right?

Just be sure you mean somthing like:

  filename1: ./fit
  filename2: /fit

right? No this isn't something I considered to happen and I'm not sure
if this is even working without this patchset.

> You'll need to make the filename absolute, I think.

Yes, I will add an resolve step within the fit_open() call. Thank you
for the feedback!

Regards,
  Marco

> > +			return handle;
> > +	}
> > +
> > +	return NULL;
> > +}
> > +
> >  static int fit_do_open(struct fit_handle *handle)
> >  {
> >  	const char *desc = "(no description)";
> > @@ -896,6 +911,8 @@ struct fit_handle *fit_open_buf(const void *buf, size_t size, bool verbose,
> >  	handle->size = size;
> >  	handle->verify = verify;
> >  
> > +	refcount_set(&handle->users, 1);
> > +
> >  	ret = fit_do_open(handle);
> >  	if (ret) {
> >  		fit_close(handle);
> > @@ -923,6 +940,12 @@ struct fit_handle *fit_open(const char *filename, bool verbose,
> >  	struct fit_handle *handle;
> >  	int ret;
> >  
> > +	handle = fit_get_handle(filename);
> > +	if (handle) {
> > +		refcount_inc(&handle->users);
> > +		return handle;
> > +	}
> > +
> >  	handle = xzalloc(sizeof(struct fit_handle));
> >  
> >  	handle->verbose = verbose;
> > @@ -939,6 +962,9 @@ struct fit_handle *fit_open(const char *filename, bool verbose,
> >  	handle->fit = handle->fit_alloc;
> >  	handle->filename = xstrdup(filename);
> >  
> > +	refcount_set(&handle->users, 1);
> > +	list_add(&handle->entry, &open_fits);
> > +
> >  	ret = fit_do_open(handle);
> >  	if (ret) {
> >  		fit_close(handle);
> > @@ -950,9 +976,15 @@ struct fit_handle *fit_open(const char *filename, bool verbose,
> >  
> >  void fit_close(struct fit_handle *handle)
> >  {
> > +	if (!refcount_dec_and_test(&handle->users))
> > +		return;
> > +
> >  	if (handle->root)
> >  		of_delete_node(handle->root);
> >  
> > +	if (handle->filename)
> > +		list_del(&handle->entry);
> > +
> >  	free(handle->filename);
> >  	free(handle->fit_alloc);
> >  	free(handle);
> > diff --git a/include/image-fit.h b/include/image-fit.h
> > index 68f70f4365cb..f9791ff251c5 100644
> > --- a/include/image-fit.h
> > +++ b/include/image-fit.h
> > @@ -7,6 +7,7 @@
> >  #define __IMAGE_FIT_H__
> >  
> >  #include <linux/types.h>
> > +#include <linux/refcount.h>
> >  #include <bootm.h>
> >  
> >  struct fit_handle {
> > @@ -15,6 +16,9 @@ struct fit_handle {
> >  	size_t size;
> >  	char *filename;
> >  
> > +	struct list_head entry;
> > +	refcount_t users;
> > +
> >  	bool verbose;
> >  	enum bootm_verify verify;
> >  
> > 
> 
> -- 
> Pengutronix e.K.                           |                             |
> Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
> 31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
> Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |
> 
> 



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2024-07-04  8:57 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-07-03 16:58 [PATCH v3 00/10] Add FIT image overlay support Marco Felsch
2024-07-03 16:58 ` [PATCH v3 01/10] FIT: fix missing free in fit_open error path Marco Felsch
2024-07-03 18:30   ` Ahmad Fatoum
2024-07-03 16:58 ` [PATCH v3 02/10] FIT: fit_open_configuration: add match function support Marco Felsch
2024-07-03 16:58 ` [PATCH v3 03/10] of: overlay: make the pattern match function more generic Marco Felsch
2024-07-03 16:58 ` [PATCH v3 04/10] of: overlay: make search dir/path " Marco Felsch
2024-07-03 16:58 ` [PATCH v3 05/10] FIT: expose useful helpers Marco Felsch
2024-07-03 16:58 ` [PATCH v3 06/10] of: overlay: add FIT overlay support Marco Felsch
2024-07-03 16:58 ` [PATCH v3 07/10] of: overlay: drop unnecessary empty check in of_overlay_global_fixup_dir Marco Felsch
2024-07-03 16:58 ` [PATCH v3 08/10] of: overlay: replace filename with an more unique name Marco Felsch
2024-07-03 16:58 ` [PATCH v3 09/10] FIT: save filename during fit_open Marco Felsch
2024-07-03 16:58 ` [PATCH v3 10/10] FIT: add support to cache opened fit images Marco Felsch
2024-07-03 18:46   ` Ahmad Fatoum
2024-07-04  8:57     ` Marco Felsch

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox