[v6] Optimizing the print format of the QEMU monitor 'info mtree'

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

Posted by Chao Liu 6 months, 2 weeks ago

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

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

Posted by Philippe Mathieu-Daudé 6 months, 2 weeks ago

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'

Posted by BALATON Zoltan 6 months, 2 weeks ago

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'

Posted by Philippe Mathieu-Daudé 6 months, 2 weeks ago

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'

Posted by BALATON Zoltan 6 months, 2 weeks ago

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