From: Sascha Hauer <s.hauer@pengutronix.de>
To: Marco Felsch <m.felsch@pengutronix.de>
Cc: BAREBOX <barebox@lists.infradead.org>
Subject: Re: [PATCH v3 06/10] of: overlay: add FIT overlay support
Date: Mon, 15 Jul 2024 12:18:47 +0200 [thread overview]
Message-ID: <ZpT3h0o089tae2WS@pengutronix.de> (raw)
In-Reply-To: <20240703-v2024-05-0-topic-fit-overlay-v3-6-c1fd766fd31d@pengutronix.de>
On Wed, Jul 03, 2024 at 06:58:34PM +0200, Marco Felsch wrote:
> 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;
The user has explicitly passed a FIT image, but we don't have FIT
support compiled in. Shouldn't this be an error?
> +
> + 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");
Maybe something like:
pr_err("Cannot stat global.of.overlay.path (%s): %pe\n", of_overlay_path, ERR_PTR(ret));
To give the user a better clue what to do about this error?
> + 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");
What other file types are you anticipating here? I would rather assume a
"... is not a FIT image" message to give the user a clue what is
expected here.
I think you could also just drop the filetype detection here and just
call of_overlay_global_fixup_fit() unconditionally. The resulting
"Loading FIT image %s failed with: %pe\n" message when a non FIT image
is to be opened should be enough information for the user.
Note that it's a bit unfortunate that both a FIT image and a dtbo file
are detected as device tree files. With the prvious naming
"global.of.overlay.dir" it was clear that a directory was expected here.
The "global.of.overlay.path" naming might confuse the user into thinking
that a path to a dtbo file could be passed here which would then be
opened as a FIT image. Would be nice to give a meaningful error message
when a user falls into this trap.
Sascha
--
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 |
next prev parent reply other threads:[~2024-07-15 10:19 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-03 16:58 [PATCH v3 00/10] Add FIT image " 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-15 9:46 ` Sascha Hauer
2024-07-03 16:58 ` [PATCH v3 04/10] of: overlay: make search dir/path " Marco Felsch
2024-07-15 9:48 ` Sascha Hauer
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-15 10:18 ` Sascha Hauer [this message]
2024-07-15 11:30 ` Marco Felsch
2024-07-15 11:56 ` Sascha Hauer
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=ZpT3h0o089tae2WS@pengutronix.de \
--to=s.hauer@pengutronix.de \
--cc=barebox@lists.infradead.org \
--cc=m.felsch@pengutronix.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox