lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAL_Jsq+RQaKTqB6hnsCJ_d0zM6FkrMXQ0NF0r1P22q95_ZDM4A@mail.gmail.com>
Date: Thu, 11 Apr 2024 13:59:36 -0500
From: Rob Herring <robh+dt@...nel.org>
To: Oreoluwa Babatunde <quic_obabatun@...cinc.com>
Cc: catalin.marinas@....com, will@...nel.org, frowand.list@...il.com, 
	vgupta@...nel.org, arnd@...db.de, olof@...om.net, soc@...nel.org, 
	guoren@...nel.org, monstr@...str.eu, palmer@...belt.com, 
	aou@...s.berkeley.edu, dinguyen@...nel.org, chenhuacai@...nel.org, 
	tsbogend@...ha.franken.de, jonas@...thpole.se, 
	stefan.kristiansson@...nalahti.fi, shorne@...il.com, mpe@...erman.id.au, 
	ysato@...rs.sourceforge.jp, dalias@...c.org, glaubitz@...sik.fu-berlin.de, 
	richard@....at, anton.ivanov@...bridgegreys.com, johannes@...solutions.net, 
	chris@...kel.net, jcmvbkbc@...il.com, linux-arm-kernel@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org, 
	linux-arm-msm@...r.kernel.org, kernel@...cinc.com
Subject: Re: [PATCH v5 3/4] of: reserved_mem: Use the unflatten_devicetree
 APIs to scan reserved mem. nodes

On Thu, Mar 28, 2024 at 4:16 PM Oreoluwa Babatunde
<quic_obabatun@...cinc.com> wrote:
>
> The unflatten_devicetree APIs have been setup and are available to be
> used by the time the fdt_init_reserved_mem() function is called.
> Since the unflatten_devicetree APIs are a more efficient way of scanning
> through the DT nodes, switch to using these APIs to facilitate the rest
> of the reserved memory processing.

Please use get_maintainers.pl. Specifically, you missed maintainers
for kernel/dma/.

>
> Signed-off-by: Oreoluwa Babatunde <quic_obabatun@...cinc.com>
> ---
>  drivers/of/of_reserved_mem.c    | 77 +++++++++++++++++++++------------
>  include/linux/of_reserved_mem.h |  2 +-
>  kernel/dma/coherent.c           |  8 ++--
>  kernel/dma/contiguous.c         |  8 ++--
>  kernel/dma/swiotlb.c            | 10 ++---
>  5 files changed, 64 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 0aba366eba59..68d1f4cca4bb 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -99,7 +99,7 @@ static void __init alloc_reserved_mem_array(void)
>  /*
>   * fdt_reserved_mem_save_node() - save fdt node for second pass initialization
>   */
> -static void __init fdt_reserved_mem_save_node(unsigned long node, const char *uname,
> +static void __init fdt_reserved_mem_save_node(struct device_node *node, const char *uname,
>                                               phys_addr_t base, phys_addr_t size)
>  {
>         struct reserved_mem *rmem = &reserved_mem[reserved_mem_count];
> @@ -109,7 +109,7 @@ static void __init fdt_reserved_mem_save_node(unsigned long node, const char *un
>                 return;
>         }
>
> -       rmem->fdt_node = node;
> +       rmem->dev_node = node;
>         rmem->name = uname;
>         rmem->base = base;
>         rmem->size = size;
> @@ -178,11 +178,11 @@ static int __init __reserved_mem_reserve_reg(unsigned long node,
>  }
>
>  /*
> - * __reserved_mem_check_root() - check if #size-cells, #address-cells provided
> + * __fdt_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)
> +static int __init __fdt_reserved_mem_check_root(unsigned long node)
>  {
>         const __be32 *prop;
>
> @@ -200,6 +200,29 @@ static int __init __reserved_mem_check_root(unsigned long node)
>         return 0;
>  }
>
> +/*
> + * __dt_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 __dt_reserved_mem_check_root(struct device_node *node)

The normal prefix is 'of', not 'dt'. Weird, right?, but it dates back
to OpenFirmware.

> +{
> +       const __be32 *prop;
> +
> +       prop = of_get_property(node, "#size-cells", NULL);

Throughout, use the appropriate typed function. Here for example,
of_property_read_u32().

> +       if (!prop || be32_to_cpup(prop) != dt_root_size_cells)
> +               return -EINVAL;
> +
> +       prop = of_get_property(node, "#address-cells", NULL);
> +       if (!prop || be32_to_cpup(prop) != dt_root_addr_cells)
> +               return -EINVAL;
> +
> +       prop = of_get_property(node, "ranges", NULL);

Or for presence, just of_property_present().

> +       if (!prop)
> +               return -EINVAL;
> +       return 0;
> +}
> +
>  /**
>   * fdt_scan_reserved_mem_reg_nodes() - Store info for the "reg" defined
>   * reserved memory regions.
> @@ -213,33 +236,38 @@ static int __init __reserved_mem_check_root(unsigned long node)
>  static void __init fdt_scan_reserved_mem_reg_nodes(void)
>  {
>         int t_len = (dt_root_addr_cells + dt_root_size_cells) * sizeof(__be32);
> -       const void *fdt = initial_boot_params;
> +       struct device_node *node, *child;
>         phys_addr_t base, size;
>         const __be32 *prop;
> -       int node, child;
>         int len;
>
> -       node = fdt_path_offset(fdt, "/reserved-memory");
> -       if (node < 0) {
> +       node = of_find_node_by_path("/reserved-memory");
> +       if (!node) {
>                 pr_info("Reserved memory: No reserved-memory node in the DT\n");
>                 return;
>         }
>
> -       if (__reserved_mem_check_root(node)) {
> +       if (__dt_reserved_mem_check_root(node)) {
>                 pr_err("Reserved memory: unsupported node format, ignoring\n");
>                 return;
>         }
>
> -       fdt_for_each_subnode(child, fdt, node) {
> +       for_each_child_of_node(node, child) {
>                 const char *uname;
> +               struct reserved_mem *rmem;
>
> -               prop = of_get_flat_dt_prop(child, "reg", &len);
> -               if (!prop)
> +               if (!of_device_is_available(child))
>                         continue;
> -               if (!of_fdt_device_is_available(fdt, child))
> +
> +               prop = of_get_property(child, "reg", &len);

We have specific unflattened functions for reading 'reg'. Note that
you should use the 'translated' ones even though we have a check to
disallow any real translation. That restriction should be fixed at
some point.

> +               if (!prop) {
> +                       rmem = of_reserved_mem_lookup(child);
> +                       if (rmem)
> +                               rmem->dev_node = child;
>                         continue;
> +               }
>
> -               uname = fdt_get_name(fdt, child, NULL);
> +               uname = of_node_full_name(child);
>                 if (len && len % t_len != 0) {
>                         pr_err("Reserved memory: invalid reg property in '%s', skipping node.\n",
>                                uname);
> @@ -269,7 +297,7 @@ int __init fdt_scan_reserved_mem(void)
>         if (node < 0)
>                 return -ENODEV;
>
> -       if (__reserved_mem_check_root(node) != 0) {
> +       if (__fdt_reserved_mem_check_root(node) != 0) {
>                 pr_err("Reserved memory: unsupported node format, ignoring\n");
>                 return -EINVAL;
>         }
> @@ -447,7 +475,7 @@ static int __init __reserved_mem_alloc_size(unsigned long node, const char *unam
>                        uname, (unsigned long)(size / SZ_1M));
>                 return -ENOMEM;
>         }
> -       fdt_reserved_mem_save_node(node, uname, base, size);
> +       fdt_reserved_mem_save_node(NULL, uname, base, size);
>         return 0;
>  }
>
> @@ -467,7 +495,7 @@ static int __init __reserved_mem_init_node(struct reserved_mem *rmem)
>                 reservedmem_of_init_fn initfn = i->data;
>                 const char *compat = i->compatible;
>
> -               if (!of_flat_dt_is_compatible(rmem->fdt_node, compat))
> +               if (!of_device_is_compatible(rmem->dev_node, compat))
>                         continue;
>
>                 ret = initfn(rmem);
> @@ -500,11 +528,6 @@ static int __init __rmem_cmp(const void *a, const void *b)
>         if (ra->size > rb->size)
>                 return 1;
>
> -       if (ra->fdt_node < rb->fdt_node)
> -               return -1;
> -       if (ra->fdt_node > rb->fdt_node)
> -               return 1;
> -
>         return 0;
>  }
>
> @@ -551,16 +574,16 @@ void __init fdt_init_reserved_mem(void)
>
>         for (i = 0; i < reserved_mem_count; i++) {
>                 struct reserved_mem *rmem = &reserved_mem[i];
> -               unsigned long node = rmem->fdt_node;
> +               struct device_node *node = rmem->dev_node;
>                 int len;
>                 const __be32 *prop;
>                 int err = 0;
>                 bool nomap;
>
> -               nomap = of_get_flat_dt_prop(node, "no-map", NULL) != NULL;
> -               prop = of_get_flat_dt_prop(node, "phandle", &len);
> +               nomap = of_get_property(node, "no-map", NULL) != NULL;
> +               prop = of_get_property(node, "phandle", &len);

We store the phandle in struct device_node, so reading and storing it
here shouldn't be needed I think.

>                 if (!prop)
> -                       prop = of_get_flat_dt_prop(node, "linux,phandle", &len);
> +                       prop = of_get_property(node, "linux,phandle", &len);
>                 if (prop)
>                         rmem->phandle = of_read_number(prop, len/4);
>
> @@ -574,7 +597,7 @@ void __init fdt_init_reserved_mem(void)
>                 } else {
>                         phys_addr_t end = rmem->base + rmem->size - 1;
>                         bool reusable =
> -                               (of_get_flat_dt_prop(node, "reusable", NULL)) != NULL;
> +                               (of_get_property(node, "reusable", NULL)) != NULL;
>
>                         pr_info("%pa..%pa (%lu KiB) %s %s %s\n",
>                                 &rmem->base, &end, (unsigned long)(rmem->size / SZ_1K),
> diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
> index 4de2a24cadc9..b6107a18d170 100644
> --- a/include/linux/of_reserved_mem.h
> +++ b/include/linux/of_reserved_mem.h
> @@ -10,7 +10,7 @@ struct reserved_mem_ops;
>
>  struct reserved_mem {
>         const char                      *name;
> -       unsigned long                   fdt_node;
> +       struct device_node              *dev_node;
>         unsigned long                   phandle;
>         const struct reserved_mem_ops   *ops;
>         phys_addr_t                     base;
> diff --git a/kernel/dma/coherent.c b/kernel/dma/coherent.c
> index ff5683a57f77..0db0aae83102 100644
> --- a/kernel/dma/coherent.c
> +++ b/kernel/dma/coherent.c
> @@ -362,20 +362,20 @@ static const struct reserved_mem_ops rmem_dma_ops = {
>
>  static int __init rmem_dma_setup(struct reserved_mem *rmem)
>  {
> -       unsigned long node = rmem->fdt_node;
> +       struct device_node *node = rmem->dev_node;
>
> -       if (of_get_flat_dt_prop(node, "reusable", NULL))
> +       if (of_get_property(node, "reusable", NULL))
>                 return -EINVAL;
>
>  #ifdef CONFIG_ARM
> -       if (!of_get_flat_dt_prop(node, "no-map", NULL)) {
> +       if (!of_get_property(node, "no-map", NULL)) {

While you are here, convert this to IS_ENABLED():

if (IS_ENABLED(CONFIG_ARM) && !of_property_read_bool(node)) {
  ...
}

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ