* [PATCH v3 0/1] Optimizing the print format of the QEMU monitor 'info mtree'
@ 2025-04-30 9:02 Chao Liu
2025-04-30 9:02 ` [PATCH v3 1/1] system: optimizing info mtree printing for monitors Chao Liu
0 siblings, 1 reply; 6+ messages in thread
From: Chao Liu @ 2025-04-30 9:02 UTC (permalink / raw)
To: pbonzini, peterx, david, philmd; +Cc: zhangtj, zqz00548, lc00631, qemu-devel
Hi, all:
Thanks to philmd's review, I've made the following improvements in PATCH v3:
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 v2 (reviewed):
https://lore.kernel.org/qemu-devel/72b2d911-112e-48e3-9ba4-017a11758060@linaro.org/T/#u
PATCH v1 (reviewed):
https://lore.kernel.org/qemu-devel/210c69d9-803e-41a5-b40c-bc8372e582fa@redhat.com/T/#u
--
Regards,
Chao
Chao Liu (1):
system: optimizing info mtree printing for monitors
system/memory.c | 36 ++++++++++++++++++++++++++++++------
1 file changed, 30 insertions(+), 6 deletions(-)
--
2.48.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v3 1/1] system: optimizing info mtree printing for monitors
2025-04-30 9:02 [PATCH v3 0/1] Optimizing the print format of the QEMU monitor 'info mtree' Chao Liu
@ 2025-04-30 9:02 ` Chao Liu
2025-04-30 9:58 ` Philippe Mathieu-Daudé
2025-05-05 17:20 ` Mario Casquero
0 siblings, 2 replies; 6+ messages in thread
From: Chao Liu @ 2025-04-30 9:02 UTC (permalink / raw)
To: pbonzini, peterx, david, philmd; +Cc: zhangtj, zqz00548, lc00631, qemu-devel
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 | 36 ++++++++++++++++++++++++++++++------
1 file changed, 30 insertions(+), 6 deletions(-)
diff --git a/system/memory.c b/system/memory.c
index 71434e7ad0..cf91718bcc 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -3296,6 +3296,21 @@ 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,
+};
+
+static void mtree_print_node(enum mtree_node_type is_tail)
+{
+ qemu_printf((is_tail == MTREE_NODE_T_TAIL) ? "└── " : "├── ");
+}
+
+static void mtree_print_col(unsigned level)
+{
+ qemu_printf(level ? "│ " : "│ ");
+}
+
static void mtree_expand_owner(const char *label, Object *obj)
{
DeviceState *dev = (DeviceState *) object_dynamic_cast(obj, TYPE_DEVICE);
@@ -3335,7 +3350,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 +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);
+ mtree_print_col(i);
}
+ mtree_print_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 +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);
+ mtree_print_col(i);
}
+ mtree_print_node(node_type);
qemu_printf(HWADDR_FMT_plx "-" HWADDR_FMT_plx
" (prio %d, %s%s): %s%s",
cur_start, cur_end,
@@ -3435,8 +3453,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 +3636,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 +3682,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");
}
--
2.48.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] system: optimizing info mtree printing for monitors
2025-04-30 9:02 ` [PATCH v3 1/1] system: optimizing info mtree printing for monitors Chao Liu
@ 2025-04-30 9:58 ` Philippe Mathieu-Daudé
2025-04-30 21:38 ` David Hildenbrand
2025-05-05 17:20 ` Mario Casquero
1 sibling, 1 reply; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-04-30 9:58 UTC (permalink / raw)
To: Chao Liu, pbonzini, peterx, david; +Cc: zhangtj, zqz00548, qemu-devel
Hi Chao,
On 30/4/25 11:02, Chao Liu wrote:
> Make the hierarchical relationship between nodes clearer by adding characters
W.r.t. $Subject, it isn't really about "optimization" but making
output clearer, as you describe.
>
> 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 | 36 ++++++++++++++++++++++++++++++------
> 1 file changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/system/memory.c b/system/memory.c
> index 71434e7ad0..cf91718bcc 100644
> --- a/system/memory.c
> +++ b/system/memory.c
> @@ -3296,6 +3296,21 @@ 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,
> +};
> +
> +static void mtree_print_node(enum mtree_node_type is_tail)
Either rename 'is_tail' as 'node_type', or really use a boolean
(maybe '[is_]last_node' is clearer?), since the mtree_node_type enum
doesn't bring much (see [*] below).
> +{
> + qemu_printf((is_tail == MTREE_NODE_T_TAIL) ? "└── " : "├── ");
> +}
> +
> +static void mtree_print_col(unsigned level)
> +{
> + qemu_printf(level ? "│ " : "│ ");
> +}
> +
> static void mtree_expand_owner(const char *label, Object *obj)
> {
> DeviceState *dev = (DeviceState *) object_dynamic_cast(obj, TYPE_DEVICE);
> @@ -3335,7 +3350,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 +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);
> + mtree_print_col(i);
> }
> + mtree_print_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 +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);
> + mtree_print_col(i);
> }
> + mtree_print_node(node_type);
> qemu_printf(HWADDR_FMT_plx "-" HWADDR_FMT_plx
> " (prio %d, %s%s): %s%s",
> cur_start, cur_end,
> @@ -3435,8 +3453,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 +3636,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 +3682,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");
> }
>
Trying this patch:
$ ./qemu-system-aarch64 -S -monitor stdio -M raspi4b
QEMU 10.0.50 monitor - type 'help' for more information
(qemu) info mtree
address-space: cpu-memory-0
address-space: cpu-memory-1
address-space: cpu-memory-2
address-space: cpu-memory-3
address-space: cpu-secure-memory-0
address-space: cpu-secure-memory-1
address-space: cpu-secure-memory-2
address-space: cpu-secure-memory-3
address-space: memory
│ ├── 0000000000000000-ffffffffffffffff (prio 0, i/o): system
│ │ ├── 0000000000000000-000000007fffffff (prio 0, ram): ram
│ │ ├── 00000000fc000000-00000000fdffffff (prio 1, i/o):
bcm2838-peripherals
│ │ ├── 00000000fe000000-00000000ff7fffff (prio 1, i/o):
bcm2835-peripherals
│ │ │ ├── 00000000fe003000-00000000fe00301f (prio 0, i/o):
bcm2835-sys-timer
│ │ │ ├── 00000000fe004000-00000000fe004fff (prio -1000, i/o):
bcm2835-txp
│ │ │ ├── 00000000fe007000-00000000fe007fff (prio 0, i/o): bcm2835-dma
│ │ │ ├── 00000000fe00a000-00000000fe00a023 (prio -1000, i/o):
bcm2838-asb
│ │ │ ├── 00000000fe00b200-00000000fe00b3ff (prio 0, i/o): alias
mphi @mphi 0000000000000000-00000000000001ff
│ │ │ ├── 00000000fe00b200-00000000fe00b3ff (prio 0, i/o): bcm2835-ic
│ │ │ ├── 00000000fe00b400-00000000fe00b43f (prio -1000, i/o):
bcm2835-sp804
│ │ │ ├── 00000000fe00b800-00000000fe00bbff (prio 0, i/o): bcm2835-mbox
│ │ │ ├── 00000000fe100000-00000000fe1001ff (prio 0, i/o):
bcm2835-powermgt
│ │ │ ├── 00000000fe101000-00000000fe102fff (prio 0, i/o):
bcm2835-cprman
│ │ │ ├── 00000000fe200000-00000000fe200fff (prio 0, i/o): bcm2838_gpio
│ │ │ ├── 00000000fe201000-00000000fe201fff (prio 0, i/o): pl011
│ │ │ ├── 00000000fe202000-00000000fe202fff (prio 0, i/o):
bcm2835-sdhost
│ │ │ ├── 00000000fe203000-00000000fe2030ff (prio -1000, i/o):
bcm2835-i2s
│ │ │ ├── 00000000fe204000-00000000fe204017 (prio 0, i/o): bcm2835-spi
│ │ │ ├── 00000000fe205000-00000000fe205023 (prio 0, i/o): bcm2835-i2c
│ │ │ ├── 00000000fe20f000-00000000fe20f07f (prio 0, i/o): bcm2835-otp
│ │ │ ├── 00000000fe214000-00000000fe2140ff (prio -1000, i/o):
bcm2835-spis
│ │ │ ├── 00000000fe215000-00000000fe2150ff (prio 0, i/o): bcm2835-aux
│ │ │ ├── 00000000fe300000-00000000fe3000ff (prio 0, i/o): sdhci
│ │ │ ├── 00000000fe340000-00000000fe3400ff (prio 0, i/o): sdhci
│ │ │ ├── 00000000fe600000-00000000fe6000ff (prio -1000, i/o):
bcm2835-smi
│ │ │ ├── 00000000fe804000-00000000fe804023 (prio 0, i/o): bcm2835-i2c
│ │ │ ├── 00000000fe805000-00000000fe805023 (prio 0, i/o): bcm2835-i2c
│ │ │ ├── 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
│ │ ├── 00000000ff841000-00000000ff841fff (prio 0, i/o): gic_dist
│ │ ├── 00000000ff842000-00000000ff843fff (prio 0, i/o): gic_cpu
│ │ ├── 00000000ff844000-00000000ff844fff (prio 0, i/o): gic_viface
│ │ ├── 00000000ff845000-00000000ff8450ff (prio 0, i/o): gic_cpu
│ │ ├── 00000000ff845200-00000000ff8452ff (prio 0, i/o): gic_cpu
│ │ ├── 00000000ff845400-00000000ff8454ff (prio 0, i/o): gic_cpu
│ │ ├── 00000000ff845600-00000000ff8456ff (prio 0, i/o): gic_cpu
│ │ └── 00000000ff846000-00000000ff847fff (prio 0, i/o): gic_vcpu
address-space: bcm2835-mbox-memory
│ ├── 0000000000000000-000000000000008f (prio 0, i/o): bcm2835-mbox
│ │ ├── 0000000000000010-000000000000001f (prio 0, i/o): bcm2835-fb
│ │ └── 0000000000000080-000000000000008f (prio 0, i/o): bcm2835-property
address-space: I/O
│ ├── 0000000000000000-000000000000ffff (prio 0, i/o): io
address-space: bcm2835-dma-memory
address-space: bcm2835-fb-memory
address-space: bcm2835-property-memory
address-space: dwc2
│ ├── 0000000000000000-00000000ffffffff (prio 0, i/o): bcm2835-gpu
│ │ ├── 0000000000000000-000000007fffffff (prio 0, ram): alias
bcm2835-gpu-ram-alias[*] @ram 0000000000000000-000000007fffffff
│ │ ├── 0000000040000000-00000000bfffffff (prio 0, ram): alias
bcm2835-gpu-ram-alias[*] @ram 0000000000000000-000000007fffffff
│ │ ├── 000000007c000000-000000007dffffff (prio 1, i/o): alias
bcm2838-peripherals @bcm2838-peripherals 0000000000000000-0000000001ffffff
│ │ ├── 000000007e000000-000000007f7fffff (prio 1, i/o): alias
bcm2835-peripherals @bcm2835-peripherals 0000000000000000-00000000017fffff
│ │ ├── 0000000080000000-00000000ffffffff (prio 0, ram): alias
bcm2835-gpu-ram-alias[*] @ram 0000000000000000-000000007fffffff
│ │ └── 00000000c0000000-000000013fffffff (prio 0, ram): alias
bcm2835-gpu-ram-alias[*] @ram 0000000000000000-000000007fffffff
memory-region: mphi
│ ├── 0000000000000000-0000000000000fff (prio 0, i/o): mphi
memory-region: ram
│ ├── 0000000000000000-000000007fffffff (prio 0, ram): ram
memory-region: bcm2838-peripherals
│ ├── 00000000fc000000-00000000fdffffff (prio 1, i/o): bcm2838-peripherals
memory-region: bcm2835-peripherals
│ ├── 00000000fe000000-00000000ff7fffff (prio 1, i/o): bcm2835-peripherals
│ │ ├── 00000000fe003000-00000000fe00301f (prio 0, i/o):
bcm2835-sys-timer
│ │ ├── 00000000fe004000-00000000fe004fff (prio -1000, i/o): bcm2835-txp
│ │ ├── 00000000fe007000-00000000fe007fff (prio 0, i/o): bcm2835-dma
│ │ ├── 00000000fe00a000-00000000fe00a023 (prio -1000, i/o): bcm2838-asb
│ │ ├── 00000000fe00b200-00000000fe00b3ff (prio 0, i/o): alias mphi
@mphi 0000000000000000-00000000000001ff
│ │ ├── 00000000fe00b200-00000000fe00b3ff (prio 0, i/o): bcm2835-ic
│ │ ├── 00000000fe00b400-00000000fe00b43f (prio -1000, i/o):
bcm2835-sp804
│ │ ├── 00000000fe00b800-00000000fe00bbff (prio 0, i/o): bcm2835-mbox
│ │ ├── 00000000fe100000-00000000fe1001ff (prio 0, i/o): bcm2835-powermgt
│ │ ├── 00000000fe101000-00000000fe102fff (prio 0, i/o): bcm2835-cprman
│ │ ├── 00000000fe200000-00000000fe200fff (prio 0, i/o): bcm2838_gpio
│ │ ├── 00000000fe201000-00000000fe201fff (prio 0, i/o): pl011
│ │ ├── 00000000fe202000-00000000fe202fff (prio 0, i/o): bcm2835-sdhost
│ │ ├── 00000000fe203000-00000000fe2030ff (prio -1000, i/o): bcm2835-i2s
│ │ ├── 00000000fe204000-00000000fe204017 (prio 0, i/o): bcm2835-spi
│ │ ├── 00000000fe205000-00000000fe205023 (prio 0, i/o): bcm2835-i2c
│ │ ├── 00000000fe20f000-00000000fe20f07f (prio 0, i/o): bcm2835-otp
│ │ ├── 00000000fe214000-00000000fe2140ff (prio -1000, i/o): bcm2835-spis
│ │ ├── 00000000fe215000-00000000fe2150ff (prio 0, i/o): bcm2835-aux
│ │ ├── 00000000fe300000-00000000fe3000ff (prio 0, i/o): sdhci
│ │ ├── 00000000fe340000-00000000fe3400ff (prio 0, i/o): sdhci
│ │ ├── 00000000fe600000-00000000fe6000ff (prio -1000, i/o): bcm2835-smi
│ │ ├── 00000000fe804000-00000000fe804023 (prio 0, i/o): bcm2835-i2c
│ │ ├── 00000000fe805000-00000000fe805023 (prio 0, i/o): bcm2835-i2c
│ │ ├── 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
(qemu)
1/ For leaves it is OK, but root branch looks odd being displayed all
along without leaf.
2/ Why single region is displayed unfinished (├)? We shouldn't display
any prefix at all, see:
address-space: I/O
│ ├── 0000000000000000-000000000000ffff (prio 0, i/o): io
[*] removing enum mtree_node_type:
-- >8 --
diff --git a/system/memory.c b/system/memory.c
index cf91718bcc1..20aa4a8e008 100644
--- a/system/memory.c
+++ b/system/memory.c
@@ -3298,10 +3298,5 @@ typedef QTAILQ_HEAD(, MemoryRegionList)
MemoryRegionListHead;
-enum mtree_node_type {
- MTREE_NODE_T_INNER,
- MTREE_NODE_T_TAIL,
-};
-
-static void mtree_print_node(enum mtree_node_type is_tail)
+static void mtree_print_node(bool last_node)
{
- qemu_printf((is_tail == MTREE_NODE_T_TAIL) ? "└── " : "├── ");
+ qemu_printf(last_node ? "└── " : "├── ");
}
@@ -3353,3 +3348,3 @@ static void mtree_print_mr(const MemoryRegion *mr,
unsigned int level,
bool owner, bool display_disabled,
- enum mtree_node_type node_type)
+ bool last_node)
{
@@ -3396,3 +3391,3 @@ static void mtree_print_mr(const MemoryRegion *mr,
unsigned int level,
}
- mtree_print_node(node_type);
+ mtree_print_node(last_node);
qemu_printf(HWADDR_FMT_plx "-" HWADDR_FMT_plx
@@ -3419,3 +3414,3 @@ static void mtree_print_mr(const MemoryRegion *mr,
unsigned int level,
}
- mtree_print_node(node_type);
+ mtree_print_node(last_node);
qemu_printf(HWADDR_FMT_plx "-" HWADDR_FMT_plx
@@ -3456,3 +3451,3 @@ static void mtree_print_mr(const MemoryRegion *mr,
unsigned int level,
if (ml == QTAILQ_LAST(&submr_print_queue)) {
- node_type = MTREE_NODE_T_TAIL;
+ last_node = true;
}
@@ -3460,3 +3455,3 @@ static void mtree_print_mr(const MemoryRegion *mr,
unsigned int level,
alias_print_queue, owner,
- display_disabled, node_type);
+ display_disabled, last_node);
}
@@ -3638,4 +3633,3 @@ static void mtree_print_as(gpointer key, gpointer
value, gpointer 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_NODE_T_INNER);
+ mtree_print_mr(mr, 1, 0, asi->ml_head, asi->owner, asi->disabled,
false);
qemu_printf("\n");
@@ -3684,4 +3678,3 @@ static void mtree_info_as(bool dispatch_tree, bool
owner, bool disabled)
qemu_printf("memory-region: %s\n", memory_region_name(ml->mr));
- mtree_print_mr(ml->mr, 1, 0, &ml_head, owner, disabled,
- MTREE_NODE_T_INNER);
+ mtree_print_mr(ml->mr, 1, 0, &ml_head, owner, disabled, false);
qemu_printf("\n");
---
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] system: optimizing info mtree printing for monitors
2025-04-30 9:58 ` Philippe Mathieu-Daudé
@ 2025-04-30 21:38 ` David Hildenbrand
2025-05-01 12:51 ` Philippe Mathieu-Daudé
0 siblings, 1 reply; 6+ messages in thread
From: David Hildenbrand @ 2025-04-30 21:38 UTC (permalink / raw)
To: Philippe Mathieu-Daudé, Chao Liu, pbonzini, peterx
Cc: zhangtj, zqz00548, qemu-devel
On 30.04.25 11:58, Philippe Mathieu-Daudé wrote:
> Hi Chao,
>
> On 30/4/25 11:02, Chao Liu wrote:
>> Make the hierarchical relationship between nodes clearer by adding characters
>
> W.r.t. $Subject, it isn't really about "optimization" but making
> output clearer, as you describe.
>
>>
>> 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 | 36 ++++++++++++++++++++++++++++++------
>> 1 file changed, 30 insertions(+), 6 deletions(-)
>>
>> diff --git a/system/memory.c b/system/memory.c
>> index 71434e7ad0..cf91718bcc 100644
>> --- a/system/memory.c
>> +++ b/system/memory.c
>> @@ -3296,6 +3296,21 @@ 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,
>> +};
>> +
>> +static void mtree_print_node(enum mtree_node_type is_tail)
>
> Either rename 'is_tail' as 'node_type', or really use a boolean
> (maybe '[is_]last_node' is clearer?), since the mtree_node_type enum
> doesn't bring much (see [*] below).
>
Note that I suggested that, because the passing or true/false in the
code is horrible for readability.
--
Cheers,
David / dhildenb
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] system: optimizing info mtree printing for monitors
2025-04-30 21:38 ` David Hildenbrand
@ 2025-05-01 12:51 ` Philippe Mathieu-Daudé
0 siblings, 0 replies; 6+ messages in thread
From: Philippe Mathieu-Daudé @ 2025-05-01 12:51 UTC (permalink / raw)
To: David Hildenbrand, Chao Liu, pbonzini, peterx
Cc: zhangtj, zqz00548, qemu-devel
On 30/4/25 23:38, David Hildenbrand wrote:
> On 30.04.25 11:58, Philippe Mathieu-Daudé wrote:
>> Hi Chao,
>>
>> On 30/4/25 11:02, Chao Liu wrote:
>>> Make the hierarchical relationship between nodes clearer by adding
>>> characters
>>
>> W.r.t. $Subject, it isn't really about "optimization" but making
>> output clearer, as you describe.
>>
>>>
>>> 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 | 36 ++++++++++++++++++++++++++++++------
>>> 1 file changed, 30 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/system/memory.c b/system/memory.c
>>> index 71434e7ad0..cf91718bcc 100644
>>> --- a/system/memory.c
>>> +++ b/system/memory.c
>>> @@ -3296,6 +3296,21 @@ 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,
>>> +};
>>> +
>>> +static void mtree_print_node(enum mtree_node_type is_tail)
>>
>> Either rename 'is_tail' as 'node_type', or really use a boolean
>> (maybe '[is_]last_node' is clearer?), since the mtree_node_type enum
>> doesn't bring much (see [*] below).
>>
>
> Note that I suggested that, because the passing or true/false in the
> code is horrible for readability.
Oh good point. Sorry I missed previous version reviews.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3 1/1] system: optimizing info mtree printing for monitors
2025-04-30 9:02 ` [PATCH v3 1/1] system: optimizing info mtree printing for monitors Chao Liu
2025-04-30 9:58 ` Philippe Mathieu-Daudé
@ 2025-05-05 17:20 ` Mario Casquero
1 sibling, 0 replies; 6+ messages in thread
From: Mario Casquero @ 2025-05-05 17:20 UTC (permalink / raw)
To: Chao Liu; +Cc: pbonzini, peterx, david, philmd, zhangtj, zqz00548, qemu-devel
This patch has been successfully tested. Differences are clear,
without the patch:
/usr/libexec/qemu-kvm -m 4G -monitor stdio
(qemu) info mtree
...
memory-region: system
0000000000000000-ffffffffffffffff (prio 0, i/o): system
0000000000000000-00000000bfffffff (prio 0, ram): alias
ram-below-4g @pc.ram 0000000000000000-00000000bfffffff
0000000000000000-ffffffffffffffff (prio -1, i/o): pci
...
After applying the patch
./qemu/build/qemu-system-x86_64 -m 4G -monitor stdio
(qemu) info mtree
...
memory-region: system
│ ├── 0000000000000000-ffffffffffffffff (prio 0, i/o): system
│ │ ├── 0000000000000000-00000000bfffffff (prio 0, ram): alias
ram-below-4g @pc.ram 0000000000000000-00000000bfffffff
│ │ ├── 0000000000000000-ffffffffffffffff (prio -1, i/o): pci
...
Tested-by: Mario Casquero <mcasquer@redhat.com>
On Wed, Apr 30, 2025 at 11:08 AM Chao Liu <lc00631@tecorigin.com> 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 | 36 ++++++++++++++++++++++++++++++------
> 1 file changed, 30 insertions(+), 6 deletions(-)
>
> diff --git a/system/memory.c b/system/memory.c
> index 71434e7ad0..cf91718bcc 100644
> --- a/system/memory.c
> +++ b/system/memory.c
> @@ -3296,6 +3296,21 @@ 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,
> +};
> +
> +static void mtree_print_node(enum mtree_node_type is_tail)
> +{
> + qemu_printf((is_tail == MTREE_NODE_T_TAIL) ? "└── " : "├── ");
> +}
> +
> +static void mtree_print_col(unsigned level)
> +{
> + qemu_printf(level ? "│ " : "│ ");
> +}
> +
> static void mtree_expand_owner(const char *label, Object *obj)
> {
> DeviceState *dev = (DeviceState *) object_dynamic_cast(obj, TYPE_DEVICE);
> @@ -3335,7 +3350,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 +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);
> + mtree_print_col(i);
> }
> + mtree_print_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 +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);
> + mtree_print_col(i);
> }
> + mtree_print_node(node_type);
> qemu_printf(HWADDR_FMT_plx "-" HWADDR_FMT_plx
> " (prio %d, %s%s): %s%s",
> cur_start, cur_end,
> @@ -3435,8 +3453,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 +3636,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 +3682,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");
> }
>
> --
> 2.48.1
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-05-05 17:22 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-30 9:02 [PATCH v3 0/1] Optimizing the print format of the QEMU monitor 'info mtree' Chao Liu
2025-04-30 9:02 ` [PATCH v3 1/1] system: optimizing info mtree printing for monitors Chao Liu
2025-04-30 9:58 ` Philippe Mathieu-Daudé
2025-04-30 21:38 ` David Hildenbrand
2025-05-01 12:51 ` Philippe Mathieu-Daudé
2025-05-05 17:20 ` Mario Casquero
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).