[PATCH v5 0/1] Optimizing the print format of the QEMU monitor 'info mtree'

[PATCH v5 0/1] Optimizing the print format of the QEMU monitor 'info mtree'

[Chao Liu]

Hi, all:

Thanks to Markus, BALATON, and Philippe for their reviews

In PATCH v5:

Fix one comment, level 0 no longer prints line characters.

```
-    /* Level 0 always has a col. */
+    /* Level 0 always has not a col. */
    if (level == 0 || col_string == NULL) {
```

PATCH v4 changelog:
1. When printing the child nodes of a single node, the line characters from the
parent node's level are no longer printed, making the output more concise
and clear overall;

2. Use more commonly used ASCII characters, such as '|--' instead of '├──',
and '`--' instead of '└──';

3. Control the number of prefix characters to reduce unnecessary output;

The result is as follows:

```
$ ./qemu-system-aarch64 -S -monitor stdio -M raspi4b
(qemu) info mtree
address-space: memory
`-- 0000000000000000-ffffffffffffffff (prio 0, i/o): system
    |-- 0000000000000000-000000007fffffff (prio 0, ram): ram
...
    |-- 00000000fe000000-00000000ff7fffff (prio 1, i/o): bcm2835-peripherals
    |   |-- 00000000fe900000-00000000fe907fff (prio -1000, i/o): bcm2835-dbus
    |   |-- 00000000fe910000-00000000fe917fff (prio -1000, i/o): bcm2835-ave0
    |   |-- 00000000fe980000-00000000fe990fff (prio 0, i/o): dwc2
    |   |   |-- 00000000fe980000-00000000fe980fff (prio 0, i/o): dwc2-io
    |   |   `-- 00000000fe981000-00000000fe990fff (prio 0, i/o): dwc2-fifo
    |   |-- 00000000fec00000-00000000fec00fff (prio -1000, i/o): bcm2835-v3d
    |   |-- 00000000fec11000-00000000fec110ff (prio -1000, i/o): bcm2835-clkisp
    |   |-- 00000000fee00000-00000000fee000ff (prio -1000, i/o): bcm2835-sdramc
    |   `-- 00000000fee05000-00000000fee050ff (prio 0, i/o): bcm2835-dma-chan15
    |-- 00000000ff800000-00000000ff8000ff (prio 0, i/o): bcm2836-control
...
    |-- 00000000ff845600-00000000ff8456ff (prio 0, i/o): gic_cpu
    `-- 00000000ff846000-00000000ff847fff (prio 0, i/o): gic_vcpu
```

PATCH v3 changelog:
1.Use more maintainable c functions instead of macros, as per review comments.

PATCH v2 changelog:
1. Enrich the commit message, add 'info mtree' print example.
2. Optimize the code implementation according to the review comments.

PATCH v4:
https://lore.kernel.org/qemu-devel/3dd1e9e3eeedc70e1f1878bd17cc779a79084e58.1746030617.git.lc00631@tecorigin.com/

PATCH v3:
https://lore.kernel.org/qemu-devel/15227d0a-c459-4bea-bec7-13dc88d22c3c@linaro.org/

PATCH v2:
https://lore.kernel.org/qemu-devel/72b2d911-112e-48e3-9ba4-017a11758060@linaro.org/
https://lore.kernel.org/qemu-devel/7ec1e581-3919-fdf5-499a-279cba99d43d@eik.bme.hu/
https://lore.kernel.org/qemu-devel/874iy5d9v7.fsf@pond.sub.org/

PATCH v1:
https://lore.kernel.org/qemu-devel/210c69d9-803e-41a5-b40c-bc8372e582fa@redhat.com/

--
Regards,
Chao

Chao Liu (1):
  system: improve visual representation of node hierarchy in 'info
    mtree' output for qemu monitor

 system/memory.c | 122 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 111 insertions(+), 11 deletions(-)

-- 
2.48.1

[PATCH v5 1/1] system: improve visual representation of node hierarchy

[Chao Liu]

Make the hierarchical relationship between nodes clearer by adding characters.

e.g.

```
$ qemu-system-riscv64 -M virt -monitor stdio -display none

(qemu) info mtree

...
address-space: memory
`-- 0000000000000000-ffffffffffffffff (prio 0, i/o): system
    |-- 0000000000001000-000000000000ffff (prio 0, rom): riscv_virt_board.mrom
...
    |-- 0000000003000000-000000000300ffff (prio 0, i/o): gpex_ioport_window
    |   `-- 0000000003000000-000000000300ffff (prio 0, i/o): gpex_ioport
    |-- 0000000004000000-0000000005ffffff (prio 0, i/o): platform bus
...
    |-- 0000000080000000-0000000087ffffff (prio 0, ram): riscv_virt_board.ram
    `-- 0000000400000000-00000007ffffffff (prio 0, i/o): alias pcie-mmio-high @gpex_mmio_window
```

Signed-off-by: Chao Liu <lc00631@tecorigin.com>
Reviewed-by: Qingze Zhao <zqz00548@tecorigin.com>
Reviewed-by: Tingjian Zhang <zhangtj@tecorigin.com>
---
 system/memory.c | 122 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 111 insertions(+), 11 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index 71434e7ad0..ec40ef09aa 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -3283,10 +3283,20 @@ static const char *memory_region_type(MemoryRegion *mr)
     }
 }
 
+typedef struct PrintCol PrintCol;
+
+struct PrintCol {
+    bool print_col;
+    QTAILQ_ENTRY(PrintCol) queue;
+};
+
+typedef QTAILQ_HEAD(, PrintCol) PrintColHead;
+
 typedef struct MemoryRegionList MemoryRegionList;
 
 struct MemoryRegionList {
     const MemoryRegion *mr;
+    PrintColHead *col_string;
     QTAILQ_ENTRY(MemoryRegionList) mrqueue;
 };
 
@@ -3296,6 +3306,94 @@ typedef QTAILQ_HEAD(, MemoryRegionList) MemoryRegionListHead;
                            int128_sub((size), int128_one())) : 0)
 #define MTREE_INDENT "  "
 
+static void mtree_print_col(PrintColHead *col_string, unsigned int level)
+{
+    PrintCol *col = NULL;
+    int i = 0;
+
+    /* Level 0 always has not a col. */
+    if (level == 0 || col_string == NULL) {
+        return;
+    }
+
+    /*
+     * If the parent node is not a tail node,
+     * print a column at the corresponding level.
+     */
+    if (col_string != NULL) {
+        QTAILQ_FOREACH(col, col_string, queue) {
+            if (i++ == level) {
+                break;
+            }
+        }
+    }
+
+    if (col != NULL && col->print_col) {
+        qemu_printf("|");
+    } else {
+        qemu_printf(" ");
+    }
+
+    /* Align with the first character of the parent node. */
+    qemu_printf("   ");
+}
+
+static void mtree_print_node(bool is_tail)
+{
+    if (is_tail) {
+        qemu_printf("`-- ");
+    } else {
+        qemu_printf("|-- ");
+    }
+}
+
+static void mtree_print_head(PrintColHead *col_string, unsigned int level,
+                             bool is_tail)
+{
+    for (int i = 0; i < level; i++) {
+        mtree_print_col(col_string, i);
+    }
+    mtree_print_node(is_tail);
+}
+
+static PrintColHead *mtree_col_string_new(PrintColHead *col_string, int level,
+                                          bool is_tail)
+{
+    PrintColHead *new_col_string = g_new(PrintColHead, 1);
+    PrintCol *col, *new_col;
+    int i = 0;
+
+    QTAILQ_INIT(new_col_string);
+    if (col_string != NULL) {
+        QTAILQ_FOREACH(col, col_string, queue) {
+            new_col = g_new(PrintCol, 1);
+            new_col->print_col = col->print_col;
+            QTAILQ_INSERT_TAIL(new_col_string, new_col, queue);
+            i++;
+        }
+    } else {
+        new_col = g_new(PrintCol, 1);
+        new_col->print_col = true;
+        QTAILQ_INSERT_TAIL(new_col_string, new_col, queue);
+        i++;
+    }
+    for (; i < level; i++) {
+        new_col = g_new(PrintCol, 1);
+        new_col->print_col = ((i == (level - 1)) && is_tail) ? false : true;
+        QTAILQ_INSERT_TAIL(new_col_string, new_col, queue);
+    }
+    return new_col_string;
+}
+
+static void mtree_col_string_free(PrintColHead *col_string)
+{
+    PrintCol *col, *next_col;
+    QTAILQ_FOREACH_SAFE(col, col_string, queue, next_col) {
+        g_free(col);
+    }
+    g_free(col_string);
+}
+
 static void mtree_expand_owner(const char *label, Object *obj)
 {
     DeviceState *dev = (DeviceState *) object_dynamic_cast(obj, TYPE_DEVICE);
@@ -3335,12 +3433,13 @@ static void mtree_print_mr_owner(const MemoryRegion *mr)
 static void mtree_print_mr(const MemoryRegion *mr, unsigned int level,
                            hwaddr base,
                            MemoryRegionListHead *alias_print_queue,
-                           bool owner, bool display_disabled)
+                           bool owner, bool display_disabled,
+                           PrintColHead *col_string,
+                           bool is_tail)
 {
     MemoryRegionList *new_ml, *ml, *next_ml;
     MemoryRegionListHead submr_print_queue;
     const MemoryRegion *submr;
-    unsigned int i;
     hwaddr cur_start, cur_end;
 
     if (!mr) {
@@ -3375,9 +3474,7 @@ static void mtree_print_mr(const MemoryRegion *mr, unsigned int level,
             QTAILQ_INSERT_TAIL(alias_print_queue, ml, mrqueue);
         }
         if (mr->enabled || display_disabled) {
-            for (i = 0; i < level; i++) {
-                qemu_printf(MTREE_INDENT);
-            }
+            mtree_print_head(col_string, level, is_tail);
             qemu_printf(HWADDR_FMT_plx "-" HWADDR_FMT_plx
                         " (prio %d, %s%s): alias %s @%s " HWADDR_FMT_plx
                         "-" HWADDR_FMT_plx "%s",
@@ -3397,9 +3494,7 @@ static void mtree_print_mr(const MemoryRegion *mr, unsigned int level,
         }
     } else {
         if (mr->enabled || display_disabled) {
-            for (i = 0; i < level; i++) {
-                qemu_printf(MTREE_INDENT);
-            }
+            mtree_print_head(col_string, level, is_tail);
             qemu_printf(HWADDR_FMT_plx "-" HWADDR_FMT_plx
                         " (prio %d, %s%s): %s%s",
                         cur_start, cur_end,
@@ -3420,6 +3515,8 @@ static void mtree_print_mr(const MemoryRegion *mr, unsigned int level,
     QTAILQ_FOREACH(submr, &mr->subregions, subregions_link) {
         new_ml = g_new(MemoryRegionList, 1);
         new_ml->mr = submr;
+        new_ml->col_string = mtree_col_string_new(col_string,
+                                                  level + 1, is_tail);
         QTAILQ_FOREACH(ml, &submr_print_queue, mrqueue) {
             if (new_ml->mr->addr < ml->mr->addr ||
                 (new_ml->mr->addr == ml->mr->addr &&
@@ -3436,10 +3533,12 @@ static void mtree_print_mr(const MemoryRegion *mr, unsigned int level,
 
     QTAILQ_FOREACH(ml, &submr_print_queue, mrqueue) {
         mtree_print_mr(ml->mr, level + 1, cur_start,
-                       alias_print_queue, owner, display_disabled);
+                       alias_print_queue, owner, display_disabled,
+                       ml->col_string, ml == QTAILQ_LAST(&submr_print_queue));
     }
 
     QTAILQ_FOREACH_SAFE(ml, &submr_print_queue, mrqueue, next_ml) {
+        mtree_col_string_free(ml->col_string);
         g_free(ml);
     }
 }
@@ -3614,7 +3713,8 @@ static void mtree_print_as(gpointer key, gpointer value, gpointer user_data)
     struct AddressSpaceInfo *asi = user_data;
 
     g_slist_foreach(as_same_root_mr_list, mtree_print_as_name, NULL);
-    mtree_print_mr(mr, 1, 0, asi->ml_head, asi->owner, asi->disabled);
+    mtree_print_mr(mr, 1, 0, asi->ml_head, asi->owner, asi->disabled,
+                   NULL, true);
     qemu_printf("\n");
 }
 
@@ -3659,7 +3759,7 @@ static void mtree_info_as(bool dispatch_tree, bool owner, bool disabled)
     /* print aliased regions */
     QTAILQ_FOREACH(ml, &ml_head, mrqueue) {
         qemu_printf("memory-region: %s\n", memory_region_name(ml->mr));
-        mtree_print_mr(ml->mr, 1, 0, &ml_head, owner, disabled);
+        mtree_print_mr(ml->mr, 1, 0, &ml_head, owner, disabled, NULL, true);
         qemu_printf("\n");
     }
 
-- 
2.48.1

Re: [PATCH v5 0/1] Optimizing the print format of the QEMU monitor 'info mtree'

[BALATON Zoltan]

[-- Attachment #1: Type: text/plain, Size: 874 bytes --]

On Thu, 1 May 2025, Chao Liu wrote:
> Hi, all:
>
> Thanks to Markus, BALATON, and Philippe for their reviews
>
> In PATCH v5:
>
> Fix one comment, level 0 no longer prints line characters.
>
> ```
> -    /* Level 0 always has a col. */
> +    /* Level 0 always has not a col. */
>    if (level == 0 || col_string == NULL) {
> ```
>
> PATCH v4 changelog:
> 1. When printing the child nodes of a single node, the line characters from the
> parent node's level are no longer printed, making the output more concise
> and clear overall;
>
> 2. Use more commonly used ASCII characters, such as '|--' instead of '├──',
> and '`--' instead of '└──';

Thanks but I meant only 1 horizontal dash instead of two to keep output 
fit in narrower terminal. So instead of '--' just '-' and instead of '|--' 
with one dash such as '|-'. Would that work?

Regards,
BALATON Zoltan