[PATCH v2 7/7] cmd: Update the meminfo command to show the memory map

[Ilias Apalodimas]

Hi all,


On Sat, 12 Oct 2024 at 04:42, Tom Rini <trini at konsulko.com> wrote:
>
> On Sat, Oct 12, 2024 at 01:18:19AM +0200, Heinrich Schuchardt wrote:
> >
> >
> > Am 11. Oktober 2024 23:40:31 MESZ schrieb Simon Glass <sjg at chromium.org>:
> > >U-Boot has a fairly rigid memory map which is normally not visible
> > >unless debugging is enabled in board_f.c
> > >
> > >Update the 'meminfo' command to show it. This command does not cover
> > >arch-specific pieces but gives a good overview of where things are.
> > >
> > >Signed-off-by: Simon Glass <sjg at chromium.org>
> > >---
> > >
> > >Changes in v2:
> > >- Modify the existing 'meminfo' command instead
> > >
> > > cmd/meminfo.c             |  48 ++++++++++++++-
> > > doc/usage/cmd/meminfo.rst | 126 ++++++++++++++++++++++++++++++++++++++
> > > doc/usage/index.rst       |   1 +
> > > test/cmd/Makefile         |   3 +-
> > > test/cmd/meminfo.c        |  38 ++++++++++++
> > > 5 files changed, 214 insertions(+), 2 deletions(-)
> > > create mode 100644 doc/usage/cmd/meminfo.rst
> > > create mode 100644 test/cmd/meminfo.c
> > >
> > >diff --git a/cmd/meminfo.c b/cmd/meminfo.c
> > >index bb9bcec2e3f..e9a635bf4bf 100644
> > >--- a/cmd/meminfo.c
> > >+++ b/cmd/meminfo.c
> > >@@ -4,18 +4,64 @@
> > >  * Written by Simon Glass <sjg at chromium.org>
> > >  */
> > >
> > >+#include <bloblist.h>
> > >+#include <bootstage.h>
> > > #include <command.h>
> > > #include <display_options.h>
> > >+#include <malloc.h>
> > >+#include <mapmem.h>
> > > #include <asm/global_data.h>
> > >
> > > DECLARE_GLOBAL_DATA_PTR;
> > >
> > >+static void print_region(const char *name, ulong base, ulong size, ulong *uptop)
> > >+{
> > >+    ulong end = base + size;
> > >+
> > >+    printf("%-12s %8lx %8lx %8lx", name, base, size, end);
> > >+    if (*uptop)
> > >+            printf(" %8lx", *uptop - end);
> > >+    putc('\n');
> > >+    *uptop = base;
> > >+}
> > >+
> > > static int do_meminfo(struct cmd_tbl *cmdtp, int flag, int argc,
> > >-                   char *const argv[])
> > >+                  char *const argv[])
> > > {
> > >+    ulong upto, stk_bot;
> > >+
> > >     puts("DRAM:  ");
> > >     print_size(gd->ram_size, "\n");
> > >
> > >+    printf("\n%-12s %8s %8s %8s %8s\n", "Region", "Base", "Size", "End",
> > >+           "Gap");
> > >+    printf("------------------------------------------------\n");
> > >+    upto = 0;
> > >+    if (IS_ENABLED(CONFIG_VIDEO))
> > >+            print_region("video", gd_video_bottom(),
> > >+                         gd_video_size(), &upto);
> > >+    if (IS_ENABLED(CONFIG_TRACE))
> > >+            print_region("trace", map_to_sysmem(gd_trace_buff()),
> > >+                         gd_trace_size(), &upto);
> > >+    print_region("code", gd->relocaddr, gd->mon_len, &upto);
> > >+    print_region("malloc", map_to_sysmem((void *)mem_malloc_start),
> > >+                 mem_malloc_end - mem_malloc_start, &upto);
> > >+    print_region("board_info", map_to_sysmem(gd->bd),
> > >+                 sizeof(struct bd_info), &upto);
> > >+    print_region("global_data", map_to_sysmem((void *)gd),
> > >+                 sizeof(struct global_data), &upto);
> > >+    print_region("devicetree", map_to_sysmem(gd->fdt_blob),
> > >+                 fdt_totalsize(gd->fdt_blob), &upto);
> > >+    if (IS_ENABLED(CONFIG_BOOTSTAGE))
> > >+            print_region("bootstage", map_to_sysmem(gd_bootstage()),
> > >+                         bootstage_get_size(false), &upto);
> > >+    if (IS_ENABLED(CONFIG_BLOBLIST))
> > >+            print_region("bloblist", map_to_sysmem(gd_bloblist()),
> > >+                         bloblist_get_total_size(), &upto);
> > >+    stk_bot = gd->start_addr_sp - CONFIG_STACK_SIZE;
> > >+    print_region("stack", stk_bot, CONFIG_STACK_SIZE, &upto);
> > >+    print_region("free", gd->ram_base, stk_bot, &upto);
> > >+
> > >     return 0;
> > > }
> > >
> > >diff --git a/doc/usage/cmd/meminfo.rst b/doc/usage/cmd/meminfo.rst
> > >new file mode 100644
> > >index 00000000000..c628119787e
> > >--- /dev/null
> > >+++ b/doc/usage/cmd/meminfo.rst
> > >@@ -0,0 +1,126 @@
> > >+.. SPDX-License-Identifier: GPL-2.0+:
> > >+
> > >+.. index::
> > >+   single: meminfo (command)
> > >+
> > >+meminfo command
> > >+===============
> > >+
> > >+Synopsis
> > >+--------
> > >+
> > >+::
> > >+
> > >+    meminfo
> > >+
> > >+Description
> > >+-----------
> > >+
> > >+The meminfo command shows the amount of memory, the layout of memory used by
> > >+U-Boot and the region which is free for use by images.
> >
> > The information offered by this command cannot be used for this purpose as it is incomplete.
> >
> > Memory allocated in LMB cannot be used to load images. And EFI in future will use LMB for allocations.
> >
> > The command should be changed to show LMB allocations.
>
> I agree, this need to include the lmb information as that's quite
> likely to be helpful to anyone wondering "well where can I place things
> then?" when denied being able to load an image somewhere.

I think we all agree, but since the LMB patches are not in place
should we wait to pull this? Or pull it since it's an improvement
anyway and retrifit the LMB and perhaps EFI regions once LMB is good
to merge?

Thanks
/Ilias
>
> --
> Tom