On Fri, Mar 22, 2024 at 05:49:51PM +0100, 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@xxxxxxxxxxxxxx>
> ---
> drivers/of/overlay.c | 110 ++++++++++++++++++++++++++++++++++++++++---
> 1 file changed, 103 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index e9fd5c0a1f7d..c8e70ab00091 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -8,10 +8,12 @@
> */
> #define pr_fmt(fmt) "of_overlay: " fmt
>
> +#include <bootm.h>
> #include <common.h>
> #include <of.h>
> #include <errno.h>
> #include <globalvar.h>
> +#include <image-fit.h>
> #include <magicvar.h>
> #include <string.h>
> #include <libfile.h>
> @@ -473,9 +475,103 @@ 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 (!fit_has_image(fit, config, "fdt"))
> + return 0;
> +
> + 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 int of_overlay_global_fixup_fit(struct device_node *root, const char *ovl_dev)
> +{
> + enum bootm_verify verify = bootm_get_verify_mode();
> + struct device_node *conf_node;
> + struct fit_handle *fit;
> + struct stat s;
> + int ret;
> +
> + if (!IS_ENABLED(CONFIG_FITIMAGE))
> + return 0;
> +
> + if (stat(of_overlay_path, &s))
> + return -errno;
Why this? The caller already checked for existence of of_overlay_path.
Besides, it is not even used in this function.
> +
> + fit = fit_open(ovl_dev, 0, verify, s.st_size);
> + if (IS_ERR(fit)) {
> + pr_err("Loading FIT image %s failed with: %pe\n", ovl_dev, fit);
> + return PTR_ERR(fit);
> + }
Are you anticipating taking only the overlays from a FIT image and the
kernel coming from somewhere else? Otherwise I would expect the
integration to happen in the bootm and FIT code where we already have a
handle to the opened FIT image. It seems wasteful to open the same FIT
image here again.
> +
> + for_each_child_of_node(fit->configurations, conf_node) {
> + if (!fit_config_is_overlay(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);
> + struct stat s;
> +
> + 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);
> + else if (S_ISCHR(s.st_mode) || S_ISBLK(s.st_mode))
> + return of_overlay_global_fixup_fit(root, of_overlay_path);
Why must the FIT image providing overlays be on a plain block device?
Shouldn't we allow FIT images to live in a filesystem?
Anyway, as said I think this is the wrong place to implement this. When
opening a FIT image it's already clear that we should take the overlays
from that image, and not open some image again.
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 |
