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

[Simon Glass]

Hi Ilias,

On Mon, 14 Oct 2024 at 11:57, Ilias Apalodimas
<ilias.apalodimas at linaro.org> wrote:
>
> 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?

I think it can go in now.

But I will say that I am very-much in favour of a setup where the EFI
memory appears in the 'meminfo' table, but within the U-Boot area.

Showing lmb is kind-of useless since the areas don't have any names. I
am working on a series for that, but it might be a few days...

Regards,
Simon