Re: [PATCH] of: Move all FDT reserved-memory handling into of_reserved_mem.c

[Saravana Kannan]

On Mon, Mar 11, 2024 at 11:14 AM Rob Herring <robh@xxxxxxxxxx> wrote:
>
> The split of /reserved-memory handling between fdt.c and
> of_reserved_mem.c makes for reading and restructuring the code
> difficult. As of_reserved_mem.c is only built for
> CONFIG_OF_EARLY_FLATTREE already, move all the code to one spot.

I'm assuming this is a clean copy-paste with maybe some
addition/removal of static keywords.

With that,
Acked-by: Saravana Kannan <saravanak@xxxxxxxxxx>

-Saravana

>
> Signed-off-by: Rob Herring <robh@xxxxxxxxxx>
> ---
> Oreoluwa, Please rebase your series on top of this.
>
>  drivers/of/fdt.c             | 123 +---------------------------------
>  drivers/of/of_private.h      |   5 +-
>  drivers/of/of_reserved_mem.c | 125 ++++++++++++++++++++++++++++++++++-
>  3 files changed, 127 insertions(+), 126 deletions(-)
>
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index e5a385285149..a8a04f27915b 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -17,7 +17,6 @@
>  #include <linux/mutex.h>
>  #include <linux/of.h>
>  #include <linux/of_fdt.h>
> -#include <linux/of_reserved_mem.h>
>  #include <linux/sizes.h>
>  #include <linux/string.h>
>  #include <linux/errno.h>
> @@ -88,7 +87,7 @@ void __init of_fdt_limit_memory(int limit)
>         }
>  }
>
> -static bool of_fdt_device_is_available(const void *blob, unsigned long node)
> +bool of_fdt_device_is_available(const void *blob, unsigned long node)
>  {
>         const char *status = fdt_getprop(blob, node, "status", NULL);
>
> @@ -484,126 +483,6 @@ void *initial_boot_params __ro_after_init;
>
>  static u32 of_fdt_crc32;
>
> -static int __init early_init_dt_reserve_memory(phys_addr_t base,
> -                                              phys_addr_t size, bool nomap)
> -{
> -       if (nomap) {
> -               /*
> -                * If the memory is already reserved (by another region), we
> -                * should not allow it to be marked nomap, but don't worry
> -                * if the region isn't memory as it won't be mapped.
> -                */
> -               if (memblock_overlaps_region(&memblock.memory, base, size) &&
> -                   memblock_is_region_reserved(base, size))
> -                       return -EBUSY;
> -
> -               return memblock_mark_nomap(base, size);
> -       }
> -       return memblock_reserve(base, size);
> -}
> -
> -/*
> - * __reserved_mem_reserve_reg() - reserve all memory described in 'reg' property
> - */
> -static int __init __reserved_mem_reserve_reg(unsigned long node,
> -                                            const char *uname)
> -{
> -       int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> -       phys_addr_t base, size;
> -       int len;
> -       const __be32 *prop;
> -       int first = 1;
> -       bool nomap;
> -
> -       prop = of_get_flat_dt_prop(node, "reg", &len);
> -       if (!prop)
> -               return -ENOENT;
> -
> -       if (len && len % t_len != 0) {
> -               pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
> -                      uname);
> -               return -EINVAL;
> -       }
> -
> -       nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> -
> -       while (len >= t_len) {
> -               base = dt_mem_next_cell(dt_root_addr_cells, &prop);
> -               size = dt_mem_next_cell(dt_root_size_cells, &prop);
> -
> -               if (size &&
> -                   early_init_dt_reserve_memory(base, size, nomap) == 0)
> -                       pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n",
> -                               uname, &base, (unsigned long)(size / SZ_1M));
> -               else
> -                       pr_err("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",
> -                              uname, &base, (unsigned long)(size / SZ_1M));
> -
> -               len -= t_len;
> -               if (first) {
> -                       fdt_reserved_mem_save_node(node, uname, base, size);
> -                       first = 0;
> -               }
> -       }
> -       return 0;
> -}
> -
> -/*
> - * __reserved_mem_check_root() - check if #size-cells, #address-cells provided
> - * in /reserved-memory matches the values supported by the current implementation,
> - * also check if ranges property has been provided
> - */
> -static int __init __reserved_mem_check_root(unsigned long node)
> -{
> -       const __be32 *prop;
> -
> -       prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> -       if (!prop || be32_to_cpup(prop) != dt_root_size_cells)
> -               return -EINVAL;
> -
> -       prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> -       if (!prop || be32_to_cpup(prop) != dt_root_addr_cells)
> -               return -EINVAL;
> -
> -       prop = of_get_flat_dt_prop(node, "ranges", NULL);
> -       if (!prop)
> -               return -EINVAL;
> -       return 0;
> -}
> -
> -/*
> - * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
> - */
> -static int __init fdt_scan_reserved_mem(void)
> -{
> -       int node, child;
> -       const void *fdt = initial_boot_params;
> -
> -       node = fdt_path_offset(fdt, "/reserved-memory");
> -       if (node < 0)
> -               return -ENODEV;
> -
> -       if (__reserved_mem_check_root(node) != 0) {
> -               pr_err("Reserved memory: unsupported node format, ignoring\n");
> -               return -EINVAL;
> -       }
> -
> -       fdt_for_each_subnode(child, fdt, node) {
> -               const char *uname;
> -               int err;
> -
> -               if (!of_fdt_device_is_available(fdt, child))
> -                       continue;
> -
> -               uname = fdt_get_name(fdt, child, NULL);
> -
> -               err = __reserved_mem_reserve_reg(child, uname);
> -               if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL))
> -                       fdt_reserved_mem_save_node(child, uname, 0, 0);
> -       }
> -       return 0;
> -}
> -
>  /*
>   * fdt_reserve_elfcorehdr() - reserves memory for elf core header
>   *
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index f38397c7b582..485483524b7f 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -175,8 +175,9 @@ static inline struct device_node *__of_get_dma_parent(const struct device_node *
>  }
>  #endif
>
> +int fdt_scan_reserved_mem(void);
>  void fdt_init_reserved_mem(void);
> -void fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> -                              phys_addr_t base, phys_addr_t size);
> +
> +bool of_fdt_device_is_available(const void *blob, unsigned long node);
>
>  #endif /* _LINUX_OF_PRIVATE_H */
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 7ec94cfcbddb..8236ecae2953 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -12,6 +12,7 @@
>  #define pr_fmt(fmt)    "OF: reserved mem: " fmt
>
>  #include <linux/err.h>
> +#include <linux/libfdt.h>
>  #include <linux/of.h>
>  #include <linux/of_fdt.h>
>  #include <linux/of_platform.h>
> @@ -58,8 +59,8 @@ static int __init early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
>  /*
>   * fdt_reserved_mem_save_node() - save fdt node for second pass initialization
>   */
> -void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> -                                     phys_addr_t base, phys_addr_t size)
> +static void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> +                                             phys_addr_t base, phys_addr_t size)
>  {
>         struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
>
> @@ -77,6 +78,126 @@ void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
>         return;
>  }
>
> +static int __init early_init_dt_reserve_memory(phys_addr_t base,
> +                                              phys_addr_t size, bool nomap)
> +{
> +       if (nomap) {
> +               /*
> +                * If the memory is already reserved (by another region), we
> +                * should not allow it to be marked nomap, but don't worry
> +                * if the region isn't memory as it won't be mapped.
> +                */
> +               if (memblock_overlaps_region(&memblock.memory, base, size) &&
> +                   memblock_is_region_reserved(base, size))
> +                       return -EBUSY;
> +
> +               return memblock_mark_nomap(base, size);
> +       }
> +       return memblock_reserve(base, size);
> +}
> +
> +/*
> + * __reserved_mem_reserve_reg() - reserve all memory described in 'reg' property
> + */
> +static int __init __reserved_mem_reserve_reg(unsigned long node,
> +                                            const char *uname)
> +{
> +       int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> +       phys_addr_t base, size;
> +       int len;
> +       const __be32 *prop;
> +       int first = 1;
> +       bool nomap;
> +
> +       prop = of_get_flat_dt_prop(node, "reg", &len);
> +       if (!prop)
> +               return -ENOENT;
> +
> +       if (len && len % t_len != 0) {
> +               pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
> +                      uname);
> +               return -EINVAL;
> +       }
> +
> +       nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> +
> +       while (len >= t_len) {
> +               base = dt_mem_next_cell(dt_root_addr_cells, &prop);
> +               size = dt_mem_next_cell(dt_root_size_cells, &prop);
> +
> +               if (size &&
> +                   early_init_dt_reserve_memory(base, size, nomap) == 0)
> +                       pr_debug("Reserved memory: reserved region for node '%s': base %pa, size %lu MiB\n",
> +                               uname, &base, (unsigned long)(size / SZ_1M));
> +               else
> +                       pr_err("Reserved memory: failed to reserve memory for node '%s': base %pa, size %lu MiB\n",
> +                              uname, &base, (unsigned long)(size / SZ_1M));
> +
> +               len -= t_len;
> +               if (first) {
> +                       fdt_reserved_mem_save_node(node, uname, base, size);
> +                       first = 0;
> +               }
> +       }
> +       return 0;
> +}
> +
> +/*
> + * __reserved_mem_check_root() - check if #size-cells, #address-cells provided
> + * in /reserved-memory matches the values supported by the current implementation,
> + * also check if ranges property has been provided
> + */
> +static int __init __reserved_mem_check_root(unsigned long node)
> +{
> +       const __be32 *prop;
> +
> +       prop = of_get_flat_dt_prop(node, "#size-cells", NULL);
> +       if (!prop || be32_to_cpup(prop) != dt_root_size_cells)
> +               return -EINVAL;
> +
> +       prop = of_get_flat_dt_prop(node, "#address-cells", NULL);
> +       if (!prop || be32_to_cpup(prop) != dt_root_addr_cells)
> +               return -EINVAL;
> +
> +       prop = of_get_flat_dt_prop(node, "ranges", NULL);
> +       if (!prop)
> +               return -EINVAL;
> +       return 0;
> +}
> +
> +/*
> + * fdt_scan_reserved_mem() - scan a single FDT node for reserved memory
> + */
> +int __init fdt_scan_reserved_mem(void)
> +{
> +       int node, child;
> +       const void *fdt = initial_boot_params;
> +
> +       node = fdt_path_offset(fdt, "/reserved-memory");
> +       if (node < 0)
> +               return -ENODEV;
> +
> +       if (__reserved_mem_check_root(node) != 0) {
> +               pr_err("Reserved memory: unsupported node format, ignoring\n");
> +               return -EINVAL;
> +       }
> +
> +       fdt_for_each_subnode(child, fdt, node) {
> +               const char *uname;
> +               int err;
> +
> +               if (!of_fdt_device_is_available(fdt, child))
> +                       continue;
> +
> +               uname = fdt_get_name(fdt, child, NULL);
> +
> +               err = __reserved_mem_reserve_reg(child, uname);
> +               if (err == -ENOENT && of_get_flat_dt_prop(child, "size", NULL))
> +                       fdt_reserved_mem_save_node(child, uname, 0, 0);
> +       }
> +       return 0;
> +}
> +
>  /*
>   * __reserved_mem_alloc_in_range() - allocate reserved memory described with
>   *     'alloc-ranges'. Choose bottom-up/top-down depending on nearby existing
> --
> 2.43.0
>