* Re: [PATCH v2 1/1] system: optimizing info mtree printing for monitors
2025-04-30 8:31 ` [PATCH v2 1/1] system: optimizing info mtree printing for monitors Chao Liu
@ 2025-04-30 8:46 ` Philippe Mathieu-Daudé
2025-04-30 11:33 ` BALATON Zoltan
2025-04-30 14:03 ` Markus Armbruster
2 siblings, 0 replies; 5+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-04-30 8:46 UTC (permalink / raw)
To: Chao Liu, pbonzini, peterx, david; +Cc: zhangtj, zqz00548, qemu-devel
Hi Chao,
On 30/4/25 10:31, Chao Liu wrote:
> Make the hierarchical relationship between nodes clearer by adding characters
>
> e.g.
>
> qemu-system-riscv64 -M virt -monitor stdio -display none
>
> ```
> (qemu) info mtree
> ...
> memory-region: system
> │ ├── 0000000000000000-ffffffffffffffff (prio 0, i/o): system
> │ │ ├── 0000000003000000-000000000300ffff (prio 0, i/o): gpex_ioport_window
> │ │ │ └── 0000000003000000-000000000300ffff (prio 0, i/o): gpex_ioport
> ...
> │ │ └── 0000000400000000-00000007ffffffff (prio 0, i/o): alias ...
> ```
Nice :)
> 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 | 42 ++++++++++++++++++++++++++++++++++++------
> 1 file changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/system/memory.c b/system/memory.c
> index 71434e7ad0..3a7faeb533 100644
> --- a/system/memory.c
> +++ b/system/memory.c
> @@ -3296,6 +3296,27 @@ typedef QTAILQ_HEAD(, MemoryRegionList) MemoryRegionListHead;
> int128_sub((size), int128_one())) : 0)
> #define MTREE_INDENT " "
>
> +enum mtree_node_type {
> + MTREE_NODE_T_INNER,
> + MTREE_NODE_T_TAIL,
> +};
> +
> +#define PRINT_MTREE_NODE(node_type) do { \
> + if (node_type == MTREE_NODE_T_TAIL) { \
> + qemu_printf("└── "); \
> + } else { \
> + qemu_printf("├── "); \
> + } \
> +} while (0)
> +
> +#define PRINT_MTREE_COL(level) do { \
> + if (level == 0) { \
> + qemu_printf("│ "); \
> + } else { \
> + qemu_printf("│ "); \
> + } \
> +} while (0)
Prefer tiny C functions over macros (easier to maintain):
static void mtree_print_node(bool is_tail)
{
qemu_printf(is_tail ? "└── " : "├── ");
}
static void mtree_print_col(unsigned level)
{
qemu_printf(level ? "│ " : "│ ");
}
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2 1/1] system: optimizing info mtree printing for monitors
2025-04-30 8:31 ` [PATCH v2 1/1] system: optimizing info mtree printing for monitors Chao Liu
2025-04-30 8:46 ` Philippe Mathieu-Daudé
@ 2025-04-30 11:33 ` BALATON Zoltan
2025-04-30 14:03 ` Markus Armbruster
2 siblings, 0 replies; 5+ messages in thread
From: BALATON Zoltan @ 2025-04-30 11:33 UTC (permalink / raw)
To: Chao Liu; +Cc: pbonzini, peterx, david, philmd, zhangtj, zqz00548, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 5443 bytes --]
On Wed, 30 Apr 2025, Chao Liu wrote:
> Make the hierarchical relationship between nodes clearer by adding characters
>
> e.g.
>
> qemu-system-riscv64 -M virt -monitor stdio -display none
>
> ```
> (qemu) info mtree
> ...
> memory-region: system
> │ ├── 0000000000000000-ffffffffffffffff (prio 0, i/o): system
> │ │ ├── 0000000003000000-000000000300ffff (prio 0, i/o): gpex_ioport_window
> │ │ │ └── 0000000003000000-000000000300ffff (prio 0, i/o): gpex_ioport
> ...
> │ │ └── 0000000400000000-00000007ffffffff (prio 0, i/o): alias ...
> ```
>
> 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 | 42 ++++++++++++++++++++++++++++++++++++------
> 1 file changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/system/memory.c b/system/memory.c
> index 71434e7ad0..3a7faeb533 100644
> --- a/system/memory.c
> +++ b/system/memory.c
> @@ -3296,6 +3296,27 @@ typedef QTAILQ_HEAD(, MemoryRegionList) MemoryRegionListHead;
> int128_sub((size), int128_one())) : 0)
> #define MTREE_INDENT " "
>
> +enum mtree_node_type {
> + MTREE_NODE_T_INNER,
> + MTREE_NODE_T_TAIL,
> +};
> +
> +#define PRINT_MTREE_NODE(node_type) do { \
> + if (node_type == MTREE_NODE_T_TAIL) { \
> + qemu_printf("└── ); \
> + } else { \
> + qemu_printf("├── "); \
Could you remove one ─ to make the output less wide? Some memory region
names can be long and with 64bit addresses it easily overflows the line
length of narrower terminals which makes the output wrap and become less
readable. Shorter lines there still shows the tree but takes up less
horizontal space so I think shorter line here would be enough.
Regards,
BALATON Zoltan
> + } \
> +} while (0)
> +
> +#define PRINT_MTREE_COL(level) do { \
> + if (level == 0) { \
> + 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,7 +3356,8 @@ 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,
> + enum mtree_node_type node_type)
> {
> MemoryRegionList *new_ml, *ml, *next_ml;
> MemoryRegionListHead submr_print_queue;
> @@ -3376,8 +3398,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(node_type);
> qemu_printf(HWADDR_FMT_plx "-" HWADDR_FMT_plx
> " (prio %d, %s%s): alias %s @%s " HWADDR_FMT_plx
> "-" HWADDR_FMT_plx "%s",
> @@ -3398,8 +3421,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(node_type);
> qemu_printf(HWADDR_FMT_plx "-" HWADDR_FMT_plx
> " (prio %d, %s%s): %s%s",
> cur_start, cur_end,
> @@ -3435,8 +3459,12 @@ static void mtree_print_mr(const MemoryRegion *mr, unsigned int level,
> }
>
> QTAILQ_FOREACH(ml, &submr_print_queue, mrqueue) {
> + if (ml == QTAILQ_LAST(&submr_print_queue)) {
> + node_type = MTREE_NODE_T_TAIL;
> + }
> mtree_print_mr(ml->mr, level + 1, cur_start,
> - alias_print_queue, owner, display_disabled);
> + alias_print_queue, owner,
> + display_disabled, node_type);
> }
>
> QTAILQ_FOREACH_SAFE(ml, &submr_print_queue, mrqueue, next_ml) {
> @@ -3614,7 +3642,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,
> + MTREE_NODE_T_INNER);
> qemu_printf("\n");
> }
>
> @@ -3659,7 +3688,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,
> + MTREE_NODE_T_INNER);
> qemu_printf("\n");
> }
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v2 1/1] system: optimizing info mtree printing for monitors
2025-04-30 8:31 ` [PATCH v2 1/1] system: optimizing info mtree printing for monitors Chao Liu
2025-04-30 8:46 ` Philippe Mathieu-Daudé
2025-04-30 11:33 ` BALATON Zoltan
@ 2025-04-30 14:03 ` Markus Armbruster
2 siblings, 0 replies; 5+ messages in thread
From: Markus Armbruster @ 2025-04-30 14:03 UTC (permalink / raw)
To: Chao Liu; +Cc: pbonzini, peterx, david, philmd, zhangtj, zqz00548, qemu-devel
Chao Liu <lc00631@tecorigin.com> writes:
> Make the hierarchical relationship between nodes clearer by adding characters
>
> e.g.
>
> qemu-system-riscv64 -M virt -monitor stdio -display none
>
> ```
> (qemu) info mtree
> ...
> memory-region: system
> │ ├── 0000000000000000-ffffffffffffffff (prio 0, i/o): system
> │ │ ├── 0000000003000000-000000000300ffff (prio 0, i/o): gpex_ioport_window
> │ │ │ └── 0000000003000000-000000000300ffff (prio 0, i/o): gpex_ioport
> ...
> │ │ └── 0000000400000000-00000007ffffffff (prio 0, i/o): alias ...
> ```
info qtree and info qom-tree could perhaps use similar treatment.
> 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 | 42 ++++++++++++++++++++++++++++++++++++------
> 1 file changed, 36 insertions(+), 6 deletions(-)
>
> diff --git a/system/memory.c b/system/memory.c
> index 71434e7ad0..3a7faeb533 100644
> --- a/system/memory.c
> +++ b/system/memory.c
> @@ -3296,6 +3296,27 @@ typedef QTAILQ_HEAD(, MemoryRegionList) MemoryRegionListHead;
> int128_sub((size), int128_one())) : 0)
> #define MTREE_INDENT " "
>
> +enum mtree_node_type {
> + MTREE_NODE_T_INNER,
> + MTREE_NODE_T_TAIL,
> +};
> +
> +#define PRINT_MTREE_NODE(node_type) do { \
> + if (node_type == MTREE_NODE_T_TAIL) { \
> + qemu_printf("└── "); \
> + } else { \
> + qemu_printf("├── "); \
> + } \
> +} while (0)
> +
> +#define PRINT_MTREE_COL(level) do { \
> + if (level == 0) { \
> + qemu_printf("│ "); \
> + } else { \
> + qemu_printf("│ "); \
> + } \
> +} while (0)
Printing non-ASCII Unicode characters encoded in UTF-8 assumes the
monitor / stdout uses UTF-8. Are we happy with such an assumption?
For what it's worth, such characters are exceedingly rare in C strings
so far. I found:
contrib/plugins/lockstep.c: g_string_printf(out, "Δ too high, we have diverged, previous insns\n");
Intentional. This is a "plugin for debugging TCG changes" (commit
c81950a2f19).
hw/uefi/var-service-policy.c: fprintf(stderr, " name ´");
This looks like a typo to me.
tests/tcg/plugins/insn.c: " Δ+%"PRId64 " since last match,"
Intentional. Seems to be a plugin used for testing.
ui/sdl2.c: status = " - Press ⌃⌥⇧G to exit grab";
ui/sdl2.c: status = " - Press ⌃⌥G to exit grab";
These are #ifndef CONFIG_DARWIN.
None found in QEMU proper.
The common ASCII equivalent to
│
├──
is
|
+--
Not as pretty, but quite readable.
[...]
^ permalink raw reply [flat|nested] 5+ messages in thread