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

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

[Chao Liu]

Hi, all:

Currently info mtre prints the memory-region hierarchy using two spaces as
indentation, which is not very clear when there are too many nodes.

```
(qemu) info mtree

memory-region: system
  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
...
    0000000040000000-000000007fffffff (prio 0, i/o): alias ...
    0000000080000000-0000000087ffffff (prio 0, ram): riscv_virt_board.ram
    0000000400000000-00000007ffffffff (prio 0, i/o): alias ...
```

Therefore, I optimized the print format of this command to be similar to the
tree command, so that it can better distinguish multi-level memory-region nodes.

```
(qemu) info mtree

memory-region: system
│  ├── 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
...
│  │   ├── 0000000040000000-000000007fffffff (prio 0, i/o): alias ...
│  │   ├── 0000000080000000-0000000087ffffff (prio 0, ram): riscv_virt_board.ram
│  │   └── 0000000400000000-00000007ffffffff (prio 0, i/o): alias ...
```
--
Regards,
Chao

Chao Liu (1):
  system: optimizing info mtree printing for monitors

 system/memory.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

-- 
2.48.1

[PATCH v1 1/1] system: optimizing info mtree printing for monitors

[Chao Liu]

Make the hierarchical relationship between nodes clearer by adding characters

Signed-off-by: Chao Liu <lc00631@tecorigin.com>
---
 system/memory.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index 71434e7ad0..e723928068 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -3296,6 +3296,22 @@ typedef QTAILQ_HEAD(, MemoryRegionList) MemoryRegionListHead;
                            int128_sub((size), int128_one())) : 0)
 #define MTREE_INDENT "  "
 
+#define PRINT_MTREE_COL(level) do { \
+    if (level == 0) {               \
+        qemu_printf("│  ");         \
+    } else {                        \
+        qemu_printf("│   ");        \
+    }                               \
+} while (0)
+
+#define PRINT_MTREE_NODE(is_tail) do { \
+    if (is_tail) {                     \
+        qemu_printf("└── ");           \
+    } else {                           \
+        qemu_printf("├── ");           \
+    }                                  \
+} while (0)
+
 static void mtree_expand_owner(const char *label, Object *obj)
 {
     DeviceState *dev = (DeviceState *) object_dynamic_cast(obj, TYPE_DEVICE);
@@ -3335,9 +3351,9 @@ 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, bool is_tail)
 {
-    MemoryRegionList *new_ml, *ml, *next_ml;
+    MemoryRegionList *new_ml, *ml, *next_ml, *ml_tail;
     MemoryRegionListHead submr_print_queue;
     const MemoryRegion *submr;
     unsigned int i;
@@ -3376,8 +3392,9 @@ static void mtree_print_mr(const MemoryRegion *mr, unsigned int level,
         }
         if (mr->enabled || display_disabled) {
             for (i = 0; i < level; i++) {
-                qemu_printf(MTREE_INDENT);
+                PRINT_MTREE_COL(i);
             }
+            PRINT_MTREE_NODE(is_tail);
             qemu_printf(HWADDR_FMT_plx "-" HWADDR_FMT_plx
                         " (prio %d, %s%s): alias %s @%s " HWADDR_FMT_plx
                         "-" HWADDR_FMT_plx "%s",
@@ -3398,8 +3415,9 @@ 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);
+                PRINT_MTREE_COL(i);
             }
+            PRINT_MTREE_NODE(is_tail);
             qemu_printf(HWADDR_FMT_plx "-" HWADDR_FMT_plx
                         " (prio %d, %s%s): %s%s",
                         cur_start, cur_end,
@@ -3434,9 +3452,11 @@ static void mtree_print_mr(const MemoryRegion *mr, unsigned int level,
         }
     }
 
+    ml_tail = QTAILQ_LAST(&submr_print_queue);
     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_tail == ml);
     }
 
     QTAILQ_FOREACH_SAFE(ml, &submr_print_queue, mrqueue, next_ml) {
@@ -3614,7 +3634,7 @@ 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, false);
     qemu_printf("\n");
 }
 
@@ -3659,7 +3679,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, false);
         qemu_printf("\n");
     }
 
-- 
2.48.1

Re: [PATCH v1 1/1] system: optimizing info mtree printing for monitors

[David Hildenbrand]

On 30.04.25 06:02, Chao Liu wrote:
> Make the hierarchical relationship between nodes clearer by adding characters
> 
You should probably move most of your cover letter, including the example, in here.

> Signed-off-by: Chao Liu <lc00631@tecorigin.com>
> ---
>   system/memory.c | 34 +++++++++++++++++++++++++++-------
>   1 file changed, 27 insertions(+), 7 deletions(-)
> 
> diff --git a/system/memory.c b/system/memory.c
> index 71434e7ad0..e723928068 100644
> --- a/system/memory.c
> +++ b/system/memory.c
> @@ -3296,6 +3296,22 @@ typedef QTAILQ_HEAD(, MemoryRegionList) MemoryRegionListHead;
>                              int128_sub((size), int128_one())) : 0)
>   #define MTREE_INDENT "  "
>   
> +#define PRINT_MTREE_COL(level) do { \
> +    if (level == 0) {               \
> +        qemu_printf("│  ");         \
> +    } else {                        \
> +        qemu_printf("│   ");        \
> +    }                               \
> +} while (0)
> +
> +#define PRINT_MTREE_NODE(is_tail) do { \
> +    if (is_tail) {                     \
> +        qemu_printf("└── ");           \
> +    } else {                           \
> +        qemu_printf("├── ");           \
> +    }                                  \
> +} while (0)
Just make these static inline functions.

> +
>   static void mtree_expand_owner(const char *label, Object *obj)
>   {
>       DeviceState *dev = (DeviceState *) object_dynamic_cast(obj, TYPE_DEVICE);
> @@ -3335,9 +3351,9 @@ 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, bool is_tail)


The growing number of input booleans is a bit suboptimal, and the hard-coded
"false" in the caller doesn't necessarily make the code easier to read.

We could consider switching to a single flags variable, or maybe convert the
"bool is_tail" into an enum like.

enum mtree_node_type {
     MTREE_NODE_T_INNER,
     MTREE_NODE_T_TAIL,
}

e.g.

mtree_print_mr(mr, 1, 0, asi->ml_head, asi->owner, asi->disabled,
                MTREE_NODE_T_INNER);


and

enum mtree_node_type mtree_node_type = MTREE_NODE_T_INNER;

...

if (ml == QTAILQ_LAST(&submr_print_queue)) {
     mtree_node_type = MTREE_NODE_T_TAIL;
}

-- 
Cheers,

David / dhildenb