linux-kernel - Re: [PATCH] of: reserved-mem: expose reserved-mem details via debugfs

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_JsqKA8pBess-zLA_2n0p8q=NBuY0EGA7qALKabwJA8ZRwrA@mail.gmail.com>
Date:   Mon, 6 Feb 2023 09:12:34 -0600
From:   Rob Herring <robh+dt@...nel.org>
To:     Martin Liu <liumartin@...gle.com>
Cc:     frowand.list@...il.com, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org, surenb@...gle.com,
        minchan@...nel.org, tkjos@...gle.com
Subject: Re: [PATCH] of: reserved-mem: expose reserved-mem details via debugfs

On Mon, Feb 6, 2023 at 8:27 AM Martin Liu <liumartin@...gle.com> wrote:
>
> It's important to know reserved-mem information in mobile world
> since reserved memory via device tree keeps increased in platform
> (e.g., 45% in our platform). Therefore, it's crucial to know the
> reserved memory sizes breakdown for the memory accounting.
>
> This patch shows the reserved memory breakdown under debugfs to
> make them visible.
>
> Below is an example output:
> cat $debugfs/reserved_mem/show
> 0x00000009fc400000..0x00000009ffffffff (   61440 KB )   map     reusable test1
> 0x00000009f9000000..0x00000009fc3fffff (   53248 KB )   map     reusable test2
> 0x00000000ffdf0000..0x00000000ffffffff (    2112 KB )   map non-reusable test3
> 0x00000009f6000000..0x00000009f8ffffff (   49152 KB )   map     reusable test4
> ...
> 0x00000000fd902000..0x00000000fd909fff (      32 KB ) nomap non-reusable test38
> 0x00000000fd90a000..0x00000000fd90bfff (       8 KB ) nomap non-reusable test39
> Total 39 regions, 1446140 KB

This information is pretty much static, why not just print it during
boot? It's also just spitting out information that's straight from the
DT which is also available to userspace (flattened and unflattened).

Is there not something in memblock that provides the same info in a
firmware agnostic way?


> Signed-off-by: Martin Liu <liumartin@...gle.com>
> ---
>  drivers/of/of_reserved_mem.c | 39 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index 65f3b02a0e4e..a73228e07c8c 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -23,6 +23,7 @@
>  #include <linux/memblock.h>
>  #include <linux/kmemleak.h>
>  #include <linux/cma.h>
> +#include <linux/debugfs.h>
>
>  #include "of_private.h"
>
> @@ -446,3 +447,41 @@ struct reserved_mem *of_reserved_mem_lookup(struct device_node *np)
>         return NULL;
>  }
>  EXPORT_SYMBOL_GPL(of_reserved_mem_lookup);
> +
> +#if defined(CONFIG_DEBUG_FS)
> +static int of_reserved_mem_debug_show(struct seq_file *m, void *private)
> +{
> +       unsigned int i;
> +       size_t sum = 0;
> +
> +       for (i = 0; i < reserved_mem_count; i++) {
> +               const struct reserved_mem *rmem = &reserved_mem[i];
> +               unsigned long node = rmem->fdt_node;
> +               phys_addr_t end = rmem->base + rmem->size - 1;
> +               bool nomap = (of_get_flat_dt_prop(node, "no-map", NULL)) != NULL;
> +               bool reusable = (of_get_flat_dt_prop(node, "reusable", NULL)) != NULL;

There is no reason to read the flat DT at this point in time after we
have an unflattened tree.

> +
> +               sum += rmem->size;
> +               seq_printf(m, "%pa..%pa ( %7lu KB ) %5s %12s %s\n", &rmem->base,
> +                          &end, rmem->size / 1024,
> +                          nomap ? "nomap" : "map",
> +                          reusable ? "reusable" : "non-reusable",
> +                          rmem->name ? rmem->name : "unknown");
> +       }
> +       seq_printf(m, "Total %d regions, %zu KB\n",
> +                  reserved_mem_count,
> +                  sum / 1024);
> +       return 0;
> +}
> +DEFINE_SHOW_ATTRIBUTE(of_reserved_mem_debug);
> +
> +static int __init of_reserved_mem_init_debugfs(void)
> +{
> +       struct dentry *root = debugfs_create_dir("reserved_mem", NULL);
> +
> +       debugfs_create_file("show", 0444, root,
> +                           NULL, &of_reserved_mem_debug_fops);
> +       return 0;
> +}
> +device_initcall(of_reserved_mem_init_debugfs);

We already have a DT init hook, don't add another random one. Plus,
why does this need to be an early device_initcall?

Rob