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

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

[Chao Liu]

Hi, all:

Thanks to BALATON, and David for their reviews.

In PATCH v6:
1. Replaced the bool type with enum mtree_node_type to improve code readability.
2. Modified the output to use only one horizontal dash instead of two, and
   aligned character printing for a cleaner look.

like this:

```
$ ./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 v5 changelog:
1. Fix one comment, level 0 no longer prints line characters.

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;

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 v5:
https://lore.kernel.org/qemu-devel/9859fa86-2fb7-2380-2320-8b268d900fe9@eik.bme.hu/

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/
https://lore.kernel.org/qemu-devel/f5f540a2-a6d6-47fd-9b7c-8fd4a4927684@redhat.com/

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 | 132 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 121 insertions(+), 11 deletions(-)

-- 
2.48.1

[PATCH v6 1/1] system: improve visual representation of node hierarchy in 'info mtree' output for qemu monitor

[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 | 132 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 121 insertions(+), 11 deletions(-)

diff --git a/system/memory.c b/system/memory.c
index 71434e7ad0..1ab020a1bf 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -3283,10 +3283,25 @@ static const char *memory_region_type(MemoryRegion *mr)
     }
 }
 
+enum mtree_node_type {
+    MTREE_NODE_T_INNER,
+    MTREE_NODE_T_TAIL,
+};
+
+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 +3311,98 @@ 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(enum mtree_node_type node_type)
+{
+    if (node_type == MTREE_NODE_T_TAIL) {
+        qemu_printf("`- ");
+    } else {
+        qemu_printf("|- ");
+    }
+}
+
+static void mtree_print_head(PrintColHead *col_string, unsigned int level,
+                             enum mtree_node_type node_type)
+{
+    for (int i = 0; i < level; i++) {
+        mtree_print_col(col_string, i);
+    }
+    mtree_print_node(node_type);
+}
+
+static PrintColHead *mtree_col_string_new(PrintColHead *col_string, int level,
+                                          enum mtree_node_type node_type)
+{
+    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);
+        if ((i == (level - 1)) && (node_type == MTREE_NODE_T_TAIL)) {
+            new_col->print_col = false;
+        } else {
+            new_col->print_col = 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 +3442,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,
+                           enum mtree_node_type node_type)
 {
     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 +3483,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, node_type);
             qemu_printf(HWADDR_FMT_plx "-" HWADDR_FMT_plx
                         " (prio %d, %s%s): alias %s @%s " HWADDR_FMT_plx
                         "-" HWADDR_FMT_plx "%s",
@@ -3397,9 +3503,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, node_type);
             qemu_printf(HWADDR_FMT_plx "-" HWADDR_FMT_plx
                         " (prio %d, %s%s): %s%s",
                         cur_start, cur_end,
@@ -3420,6 +3524,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, node_type);
         QTAILQ_FOREACH(ml, &submr_print_queue, mrqueue) {
             if (new_ml->mr->addr < ml->mr->addr ||
                 (new_ml->mr->addr == ml->mr->addr &&
@@ -3436,10 +3542,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 +3722,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, MTREE_NODE_T_TAIL);
     qemu_printf("\n");
 }
 
@@ -3659,7 +3768,8 @@ 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, MTREE_NODE_T_TAIL);
         qemu_printf("\n");
     }
 
-- 
2.48.1

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

[Philippe Mathieu-Daudé]

Hi Chao,

On 1/5/25 04:24, Chao Liu wrote:
> Hi, all:
> 
> Thanks to BALATON, and David for their reviews.
> 
> In PATCH v6:
> 1. Replaced the bool type with enum mtree_node_type to improve code readability.
> 2. Modified the output to use only one horizontal dash instead of two, and
>     aligned character printing for a cleaner look.
> 
> like this:
> 
> ```
> $ ./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
> ```

Could we keep the address ranges aligned? I.e.:

 >   |--+     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

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

[BALATON Zoltan]

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

On Thu, 1 May 2025, Philippe Mathieu-Daudé wrote:
> Hi Chao,
>
> On 1/5/25 04:24, Chao Liu wrote:
>> Hi, all:
>> 
>> Thanks to BALATON, and David for their reviews.
>> 
>> In PATCH v6:
>> 1. Replaced the bool type with enum mtree_node_type to improve code 
>> readability.
>> 2. Modified the output to use only one horizontal dash instead of two, and
>>     aligned character printing for a cleaner look.
>> 
>> like this:
>> 
>> ```
>> $ ./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
>> ```
>
> Could we keep the address ranges aligned? I.e.:
>
>>   |--+     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

I don't think that helps. I was OK with just indents and adding graphics 
does not change that but this would actually make it harder to see what is 
below what as you'd have to trace back to the beginning of the line and 
not just look at the right end where it would not be clear where a sub 
region starts so it't less readable even if it looks more organised. So 
I'd keep the indent.

Regards,
BALATON Zoltan

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

[Philippe Mathieu-Daudé]

On 1/5/25 15:49, BALATON Zoltan wrote:
> On Thu, 1 May 2025, Philippe Mathieu-Daudé wrote:
>> Hi Chao,
>>
>> On 1/5/25 04:24, Chao Liu wrote:
>>> Hi, all:
>>>
>>> Thanks to BALATON, and David for their reviews.
>>>
>>> In PATCH v6:
>>> 1. Replaced the bool type with enum mtree_node_type to improve code 
>>> readability.
>>> 2. Modified the output to use only one horizontal dash instead of 
>>> two, and
>>>     aligned character printing for a cleaner look.
>>>
>>> like this:
>>>
>>> ```
>>> $ ./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
>>> ```
>>
>> Could we keep the address ranges aligned? I.e.:
>>
>>>   |--+     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
> 
> I don't think that helps. I was OK with just indents and adding graphics 
> does not change that but this would actually make it harder to see what 
> is below what as you'd have to trace back to the beginning of the line 
> and not just look at the right end where it would not be clear where a 
> sub region starts so it't less readable even if it looks more organised. 
> So I'd keep the indent.

Maybe add the '-t' option then, to display as tree, and not disturb
the previous command output.

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

[BALATON Zoltan]

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

On Thu, 1 May 2025, Philippe Mathieu-Daudé wrote:
> On 1/5/25 15:49, BALATON Zoltan wrote:
>> On Thu, 1 May 2025, Philippe Mathieu-Daudé wrote:
>>> Hi Chao,
>>> 
>>> On 1/5/25 04:24, Chao Liu wrote:
>>>> Hi, all:
>>>> 
>>>> Thanks to BALATON, and David for their reviews.
>>>> 
>>>> In PATCH v6:
>>>> 1. Replaced the bool type with enum mtree_node_type to improve code 
>>>> readability.
>>>> 2. Modified the output to use only one horizontal dash instead of two, 
>>>> and
>>>>     aligned character printing for a cleaner look.
>>>> 
>>>> like this:
>>>> 
>>>> ```
>>>> $ ./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
>>>> ```
>>> 
>>> Could we keep the address ranges aligned? I.e.:
>>> 
>>>>   |--+     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
>> 
>> I don't think that helps. I was OK with just indents and adding graphics 
>> does not change that but this would actually make it harder to see what is 
>> below what as you'd have to trace back to the beginning of the line and not 
>> just look at the right end where it would not be clear where a sub region 
>> starts so it't less readable even if it looks more organised. So I'd keep 
>> the indent.
>
> Maybe add the '-t' option then, to display as tree, and not disturb
> the previous command output.

That seems to be a good compromise and works for me if you want aligned 
tree but if you're OK with indented tree I don't mind the additional chars 
at the front either (as long as they are not too wide) so not necessary to 
add an option in that case. On the other hand if anything tries to parse 
this output then an option may be better to cause less disruption although 
it's human monitor command so format can change without notice.

Regards,
BALATON Zoltan