[PATCH v4 0/3] memory: Have 'info mtree' remove duplicated Address Space information

[PATCH v4 0/3] memory: Have 'info mtree' remove duplicated Address Space information

[Philippe Mathieu-Daudé]

Follow Peter Maydell suggestions:
- Split mtree_info() as mtree_info_flatview() + mtree_info_as()
- Remove duplicated Address Space information

Since v3:
- Fix typos
- Split mtree_info_flatview() + mtree_info_as() first
- Rebased last patch keeping Peter's R-b tag

Philippe Mathieu-Daudé (3):
  memory: Extract mtree_info_flatview() from mtree_info()
  memory: Extract mtree_info_as() from mtree_info()
  memory: Have 'info mtree' remove duplicated Address Space information

 softmmu/memory.c | 160 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 118 insertions(+), 42 deletions(-)

-- 
2.31.1

[PATCH v4 1/3] memory: Extract mtree_info_flatview() from mtree_info()

[Philippe Mathieu-Daudé]

While mtree_info() handles both ASes and flatviews cases,
the two cases share basically no code. Split mtree_info_flatview()
out of mtree_info() to simplify.

Note: Patch easier to review using 'git-diff --color-moved=blocks'.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 softmmu/memory.c | 72 ++++++++++++++++++++++++++----------------------
 1 file changed, 39 insertions(+), 33 deletions(-)

diff --git a/softmmu/memory.c b/softmmu/memory.c
index bfedaf9c4df..3eb6f52de67 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -3246,6 +3246,44 @@ static gboolean mtree_info_flatview_free(gpointer key, gpointer value,
     return true;
 }
 
+static void mtree_info_flatview(bool dispatch_tree, bool owner)
+{
+    struct FlatViewInfo fvi = {
+        .counter = 0,
+        .dispatch_tree = dispatch_tree,
+        .owner = owner,
+    };
+    AddressSpace *as;
+    FlatView *view;
+    GArray *fv_address_spaces;
+    GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal);
+    AccelClass *ac = ACCEL_GET_CLASS(current_accel());
+
+    if (ac->has_memory) {
+        fvi.ac = ac;
+    }
+
+    /* Gather all FVs in one table */
+    QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
+        view = address_space_get_flatview(as);
+
+        fv_address_spaces = g_hash_table_lookup(views, view);
+        if (!fv_address_spaces) {
+            fv_address_spaces = g_array_new(false, false, sizeof(as));
+            g_hash_table_insert(views, view, fv_address_spaces);
+        }
+
+        g_array_append_val(fv_address_spaces, as);
+    }
+
+    /* Print */
+    g_hash_table_foreach(views, mtree_print_flatview, &fvi);
+
+    /* Free */
+    g_hash_table_foreach_remove(views, mtree_info_flatview_free, 0);
+    g_hash_table_unref(views);
+}
+
 void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
 {
     MemoryRegionListHead ml_head;
@@ -3253,39 +3291,7 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
     AddressSpace *as;
 
     if (flatview) {
-        FlatView *view;
-        struct FlatViewInfo fvi = {
-            .counter = 0,
-            .dispatch_tree = dispatch_tree,
-            .owner = owner,
-        };
-        GArray *fv_address_spaces;
-        GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal);
-        AccelClass *ac = ACCEL_GET_CLASS(current_accel());
-
-        if (ac->has_memory) {
-            fvi.ac = ac;
-        }
-
-        /* Gather all FVs in one table */
-        QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
-            view = address_space_get_flatview(as);
-
-            fv_address_spaces = g_hash_table_lookup(views, view);
-            if (!fv_address_spaces) {
-                fv_address_spaces = g_array_new(false, false, sizeof(as));
-                g_hash_table_insert(views, view, fv_address_spaces);
-            }
-
-            g_array_append_val(fv_address_spaces, as);
-        }
-
-        /* Print */
-        g_hash_table_foreach(views, mtree_print_flatview, &fvi);
-
-        /* Free */
-        g_hash_table_foreach_remove(views, mtree_info_flatview_free, 0);
-        g_hash_table_unref(views);
+        mtree_info_flatview(dispatch_tree, owner);
 
         return;
     }
-- 
2.31.1

Re: [PATCH v4 1/3] memory: Extract mtree_info_flatview() from mtree_info()

[Philippe Mathieu-Daudé]

On 9/1/21 6:19 PM, Philippe Mathieu-Daudé wrote:
> While mtree_info() handles both ASes and flatviews cases,
> the two cases share basically no code. Split mtree_info_flatview()
> out of mtree_info() to simplify.
> 
> Note: Patch easier to review using 'git-diff --color-moved=blocks'.

Surprisingly git-format-patch choose a better algorithm automatically...

> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  softmmu/memory.c | 72 ++++++++++++++++++++++++++----------------------
>  1 file changed, 39 insertions(+), 33 deletions(-)
> 
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index bfedaf9c4df..3eb6f52de67 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -3246,6 +3246,44 @@ static gboolean mtree_info_flatview_free(gpointer key, gpointer value,
>      return true;
>  }
>  
> +static void mtree_info_flatview(bool dispatch_tree, bool owner)
> +{
> +    struct FlatViewInfo fvi = {
> +        .counter = 0,
> +        .dispatch_tree = dispatch_tree,
> +        .owner = owner,
> +    };
> +    AddressSpace *as;
> +    FlatView *view;
> +    GArray *fv_address_spaces;
> +    GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal);
> +    AccelClass *ac = ACCEL_GET_CLASS(current_accel());
> +
> +    if (ac->has_memory) {
> +        fvi.ac = ac;
> +    }
> +
> +    /* Gather all FVs in one table */
> +    QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> +        view = address_space_get_flatview(as);
> +
> +        fv_address_spaces = g_hash_table_lookup(views, view);
> +        if (!fv_address_spaces) {
> +            fv_address_spaces = g_array_new(false, false, sizeof(as));
> +            g_hash_table_insert(views, view, fv_address_spaces);
> +        }
> +
> +        g_array_append_val(fv_address_spaces, as);
> +    }
> +
> +    /* Print */
> +    g_hash_table_foreach(views, mtree_print_flatview, &fvi);
> +
> +    /* Free */
> +    g_hash_table_foreach_remove(views, mtree_info_flatview_free, 0);
> +    g_hash_table_unref(views);
> +}
> +
>  void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
>  {
>      MemoryRegionListHead ml_head;
> @@ -3253,39 +3291,7 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
>      AddressSpace *as;
>  
>      if (flatview) {
> -        FlatView *view;
> -        struct FlatViewInfo fvi = {
> -            .counter = 0,
> -            .dispatch_tree = dispatch_tree,
> -            .owner = owner,
> -        };
> -        GArray *fv_address_spaces;
> -        GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal);
> -        AccelClass *ac = ACCEL_GET_CLASS(current_accel());
> -
> -        if (ac->has_memory) {
> -            fvi.ac = ac;
> -        }
> -
> -        /* Gather all FVs in one table */
> -        QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> -            view = address_space_get_flatview(as);
> -
> -            fv_address_spaces = g_hash_table_lookup(views, view);
> -            if (!fv_address_spaces) {
> -                fv_address_spaces = g_array_new(false, false, sizeof(as));
> -                g_hash_table_insert(views, view, fv_address_spaces);
> -            }
> -
> -            g_array_append_val(fv_address_spaces, as);
> -        }
> -
> -        /* Print */
> -        g_hash_table_foreach(views, mtree_print_flatview, &fvi);
> -
> -        /* Free */
> -        g_hash_table_foreach_remove(views, mtree_info_flatview_free, 0);
> -        g_hash_table_unref(views);
> +        mtree_info_flatview(dispatch_tree, owner);
>  
>          return;
>      }
> 

Re: [PATCH v4 1/3] memory: Extract mtree_info_flatview() from mtree_info()

[David Hildenbrand]

On 01.09.21 18:19, Philippe Mathieu-Daudé wrote:
> While mtree_info() handles both ASes and flatviews cases,
> the two cases share basically no code. Split mtree_info_flatview()
> out of mtree_info() to simplify.
> 
> Note: Patch easier to review using 'git-diff --color-moved=blocks'.
> 
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: David Hildenbrand <david@redhat.com>


-- 
Thanks,

David / dhildenb

Re: [PATCH v4 1/3] memory: Extract mtree_info_flatview() from mtree_info()

[Richard Henderson]

On 9/1/21 6:19 PM, Philippe Mathieu-Daudé wrote:
> While mtree_info() handles both ASes and flatviews cases,
> the two cases share basically no code. Split mtree_info_flatview()
> out of mtree_info() to simplify.
> 
> Note: Patch easier to review using 'git-diff --color-moved=blocks'.
> 
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   softmmu/memory.c | 72 ++++++++++++++++++++++++++----------------------
>   1 file changed, 39 insertions(+), 33 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH v4 2/3] memory: Extract mtree_info_as() from mtree_info()

[Philippe Mathieu-Daudé]

While mtree_info() handles both ASes and flatviews cases,
the two cases share basically no code. Split mtree_info_as()
out of mtree_info() to simplify.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 softmmu/memory.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/softmmu/memory.c b/softmmu/memory.c
index 3eb6f52de67..5be7d5e7412 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -3284,18 +3284,12 @@ static void mtree_info_flatview(bool dispatch_tree, bool owner)
     g_hash_table_unref(views);
 }
 
-void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
+static void mtree_info_as(bool dispatch_tree, bool owner, bool disabled)
 {
     MemoryRegionListHead ml_head;
     MemoryRegionList *ml, *ml2;
     AddressSpace *as;
 
-    if (flatview) {
-        mtree_info_flatview(dispatch_tree, owner);
-
-        return;
-    }
-
     QTAILQ_INIT(&ml_head);
 
     QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
@@ -3316,6 +3310,15 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
     }
 }
 
+void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
+{
+    if (flatview) {
+        mtree_info_flatview(dispatch_tree, owner);
+    } else {
+        mtree_info_as(dispatch_tree, owner, disabled);
+    }
+}
+
 void memory_region_init_ram(MemoryRegion *mr,
                             Object *owner,
                             const char *name,
-- 
2.31.1

Re: [PATCH v4 2/3] memory: Extract mtree_info_as() from mtree_info()

[David Hildenbrand]

On 01.09.21 18:19, Philippe Mathieu-Daudé wrote:
> While mtree_info() handles both ASes and flatviews cases,
> the two cases share basically no code. Split mtree_info_as()
> out of mtree_info() to simplify.
> 
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>   softmmu/memory.c | 17 ++++++++++-------
>   1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index 3eb6f52de67..5be7d5e7412 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -3284,18 +3284,12 @@ static void mtree_info_flatview(bool dispatch_tree, bool owner)
>       g_hash_table_unref(views);
>   }
>   
> -void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
> +static void mtree_info_as(bool dispatch_tree, bool owner, bool disabled)
>   {
>       MemoryRegionListHead ml_head;
>       MemoryRegionList *ml, *ml2;
>       AddressSpace *as;
>   
> -    if (flatview) {
> -        mtree_info_flatview(dispatch_tree, owner);
> -
> -        return;
> -    }
> -
>       QTAILQ_INIT(&ml_head);
>   
>       QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> @@ -3316,6 +3310,15 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
>       }
>   }
>   
> +void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
> +{
> +    if (flatview) {
> +        mtree_info_flatview(dispatch_tree, owner);
> +    } else {
> +        mtree_info_as(dispatch_tree, owner, disabled);
> +    }
> +}
> +
>   void memory_region_init_ram(MemoryRegion *mr,
>                               Object *owner,
>                               const char *name,
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

I'd just have squashed that into #1.

-- 
Thanks,

David / dhildenb

Re: [PATCH v4 2/3] memory: Extract mtree_info_as() from mtree_info()

[Richard Henderson]

On 9/1/21 7:11 PM, David Hildenbrand wrote:
> On 01.09.21 18:19, Philippe Mathieu-Daudé wrote:
>> While mtree_info() handles both ASes and flatviews cases,
>> the two cases share basically no code. Split mtree_info_as()
>> out of mtree_info() to simplify.
>>
>> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>   softmmu/memory.c | 17 ++++++++++-------
>>   1 file changed, 10 insertions(+), 7 deletions(-)
>>
>> diff --git a/softmmu/memory.c b/softmmu/memory.c
>> index 3eb6f52de67..5be7d5e7412 100644
>> --- a/softmmu/memory.c
>> +++ b/softmmu/memory.c
>> @@ -3284,18 +3284,12 @@ static void mtree_info_flatview(bool dispatch_tree, bool owner)
>>       g_hash_table_unref(views);
>>   }
>> -void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
>> +static void mtree_info_as(bool dispatch_tree, bool owner, bool disabled)
>>   {
>>       MemoryRegionListHead ml_head;
>>       MemoryRegionList *ml, *ml2;
>>       AddressSpace *as;
>> -    if (flatview) {
>> -        mtree_info_flatview(dispatch_tree, owner);
>> -
>> -        return;
>> -    }
>> -
>>       QTAILQ_INIT(&ml_head);
>>       QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
>> @@ -3316,6 +3310,15 @@ void mtree_info(bool flatview, bool dispatch_tree, bool owner, 
>> bool disabled)
>>       }
>>   }
>> +void mtree_info(bool flatview, bool dispatch_tree, bool owner, bool disabled)
>> +{
>> +    if (flatview) {
>> +        mtree_info_flatview(dispatch_tree, owner);
>> +    } else {
>> +        mtree_info_as(dispatch_tree, owner, disabled);
>> +    }
>> +}
>> +
>>   void memory_region_init_ram(MemoryRegion *mr,
>>                               Object *owner,
>>                               const char *name,
>>
> 
> Reviewed-by: David Hildenbrand <david@redhat.com>
> 
> I'd just have squashed that into #1.

Agreed, it seems like all part of one change.  Anyway,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

[PATCH v4 3/3] memory: Have 'info mtree' remove duplicated Address Space information

[Philippe Mathieu-Daudé]

Per Peter Maydell [*]:

  'info mtree' monitor command was designed on the assumption that
  there's really only one or two interesting address spaces, and
  with more recent developments that's just not the case any more.

Similarly about how the FlatView are sorted using a GHashTable,
sort the AddressSpace objects to remove the duplications (AS
using the same root MemoryRegion).

This drastically reduces the output of 'info mtree' on some boards.

Before:

  $ (echo info mtree; echo q) \
    | qemu-system-aarch64 -S -monitor stdio -M raspi3b \
    | wc -l
  423

After:

  $ (echo info mtree; echo q) \
    | qemu-system-aarch64 -S -monitor stdio -M raspi3b \
    | wc -l
  108

  (qemu) info mtree
  address-space: I/O
    0000000000000000-000000000000ffff (prio 0, i/o): io

  address-space shared 9 times:
    - cpu-memory-0
    - cpu-memory-1
    - cpu-memory-2
    - cpu-memory-3
    - cpu-secure-memory-0
    - cpu-secure-memory-1
    - cpu-secure-memory-2
    - cpu-secure-memory-3
    - memory
    0000000000000000-ffffffffffffffff (prio 0, i/o): system
      0000000000000000-000000003fffffff (prio 0, ram): ram
      000000003f000000-000000003fffffff (prio 1, i/o): bcm2835-peripherals
        000000003f003000-000000003f00301f (prio 0, i/o): bcm2835-sys-timer
        000000003f004000-000000003f004fff (prio -1000, i/o): bcm2835-txp
        000000003f006000-000000003f006fff (prio 0, i/o): mphi
        000000003f007000-000000003f007fff (prio 0, i/o): bcm2835-dma
        000000003f00b200-000000003f00b3ff (prio 0, i/o): bcm2835-ic
        000000003f00b400-000000003f00b43f (prio -1000, i/o): bcm2835-sp804
        000000003f00b800-000000003f00bbff (prio 0, i/o): bcm2835-mbox
        000000003f100000-000000003f1001ff (prio 0, i/o): bcm2835-powermgt
        000000003f101000-000000003f102fff (prio 0, i/o): bcm2835-cprman
        000000003f104000-000000003f10400f (prio 0, i/o): bcm2835-rng
        000000003f200000-000000003f200fff (prio 0, i/o): bcm2835_gpio
        000000003f201000-000000003f201fff (prio 0, i/o): pl011
        000000003f202000-000000003f202fff (prio 0, i/o): bcm2835-sdhost
        000000003f203000-000000003f2030ff (prio -1000, i/o): bcm2835-i2s
        000000003f204000-000000003f20401f (prio -1000, i/o): bcm2835-spi0
        000000003f205000-000000003f20501f (prio -1000, i/o): bcm2835-i2c0
        000000003f20f000-000000003f20f07f (prio -1000, i/o): bcm2835-otp
        000000003f212000-000000003f212007 (prio 0, i/o): bcm2835-thermal
        000000003f214000-000000003f2140ff (prio -1000, i/o): bcm2835-spis
        000000003f215000-000000003f2150ff (prio 0, i/o): bcm2835-aux
        000000003f300000-000000003f3000ff (prio 0, i/o): sdhci
        000000003f600000-000000003f6000ff (prio -1000, i/o): bcm2835-smi
        000000003f804000-000000003f80401f (prio -1000, i/o): bcm2835-i2c1
        000000003f805000-000000003f80501f (prio -1000, i/o): bcm2835-i2c2
        000000003f900000-000000003f907fff (prio -1000, i/o): bcm2835-dbus
        000000003f910000-000000003f917fff (prio -1000, i/o): bcm2835-ave0
        000000003f980000-000000003f990fff (prio 0, i/o): dwc2
          000000003f980000-000000003f980fff (prio 0, i/o): dwc2-io
          000000003f981000-000000003f990fff (prio 0, i/o): dwc2-fifo
        000000003fc00000-000000003fc00fff (prio -1000, i/o): bcm2835-v3d
        000000003fe00000-000000003fe000ff (prio -1000, i/o): bcm2835-sdramc
        000000003fe05000-000000003fe050ff (prio 0, i/o): bcm2835-dma-chan15
      0000000040000000-00000000400000ff (prio 0, i/o): bcm2836-control

  address-space shared 4 times:
    - bcm2835-dma-memory
    - bcm2835-fb-memory
    - bcm2835-property-memory
    - dwc2
    0000000000000000-00000000ffffffff (prio 0, i/o): bcm2835-gpu
      0000000000000000-000000003fffffff (prio 0, ram): alias bcm2835-gpu-ram-alias[*] @ram 0000000000000000-000000003fffffff
      0000000040000000-000000007fffffff (prio 0, ram): alias bcm2835-gpu-ram-alias[*] @ram 0000000000000000-000000003fffffff
      000000007e000000-000000007effffff (prio 1, i/o): alias bcm2835-peripherals @bcm2835-peripherals 0000000000000000-0000000000ffffff
      0000000080000000-00000000bfffffff (prio 0, ram): alias bcm2835-gpu-ram-alias[*] @ram 0000000000000000-000000003fffffff
      00000000c0000000-00000000ffffffff (prio 0, ram): alias bcm2835-gpu-ram-alias[*] @ram 0000000000000000-000000003fffffff

  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

  memory-region: ram
    0000000000000000-000000003fffffff (prio 0, ram): ram

  memory-region: bcm2835-peripherals
    000000003f000000-000000003fffffff (prio 1, i/o): bcm2835-peripherals
      000000003f003000-000000003f00301f (prio 0, i/o): bcm2835-sys-timer
      000000003f004000-000000003f004fff (prio -1000, i/o): bcm2835-txp
      000000003f006000-000000003f006fff (prio 0, i/o): mphi
      000000003f007000-000000003f007fff (prio 0, i/o): bcm2835-dma
      000000003f00b200-000000003f00b3ff (prio 0, i/o): bcm2835-ic
      000000003f00b400-000000003f00b43f (prio -1000, i/o): bcm2835-sp804
      000000003f00b800-000000003f00bbff (prio 0, i/o): bcm2835-mbox
      000000003f100000-000000003f1001ff (prio 0, i/o): bcm2835-powermgt
      000000003f101000-000000003f102fff (prio 0, i/o): bcm2835-cprman
      000000003f104000-000000003f10400f (prio 0, i/o): bcm2835-rng
      000000003f200000-000000003f200fff (prio 0, i/o): bcm2835_gpio
      000000003f201000-000000003f201fff (prio 0, i/o): pl011
      000000003f202000-000000003f202fff (prio 0, i/o): bcm2835-sdhost
      000000003f203000-000000003f2030ff (prio -1000, i/o): bcm2835-i2s
      000000003f204000-000000003f20401f (prio -1000, i/o): bcm2835-spi0
      000000003f205000-000000003f20501f (prio -1000, i/o): bcm2835-i2c0
      000000003f20f000-000000003f20f07f (prio -1000, i/o): bcm2835-otp
      000000003f212000-000000003f212007 (prio 0, i/o): bcm2835-thermal
      000000003f214000-000000003f2140ff (prio -1000, i/o): bcm2835-spis
      000000003f215000-000000003f2150ff (prio 0, i/o): bcm2835-aux
      000000003f300000-000000003f3000ff (prio 0, i/o): sdhci
      000000003f600000-000000003f6000ff (prio -1000, i/o): bcm2835-smi
      000000003f804000-000000003f80401f (prio -1000, i/o): bcm2835-i2c1
      000000003f805000-000000003f80501f (prio -1000, i/o): bcm2835-i2c2
      000000003f900000-000000003f907fff (prio -1000, i/o): bcm2835-dbus
      000000003f910000-000000003f917fff (prio -1000, i/o): bcm2835-ave0
      000000003f980000-000000003f990fff (prio 0, i/o): dwc2
        000000003f980000-000000003f980fff (prio 0, i/o): dwc2-io
        000000003f981000-000000003f990fff (prio 0, i/o): dwc2-fifo
      000000003fc00000-000000003fc00fff (prio -1000, i/o): bcm2835-v3d
      000000003fe00000-000000003fe000ff (prio -1000, i/o): bcm2835-sdramc
      000000003fe05000-000000003fe050ff (prio 0, i/o): bcm2835-dma-chan15

  (qemu) q

[*] https://www.mail-archive.com/qemu-devel@nongnu.org/msg829821.html

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
Pending question from David on v3:

  We can still distinguish from a completely empty AS, because we don't have an empty line here, correct?

checkpatch warning (81 chars):

  WARNING: line over 80 characters
  #86: FILE: softmmu/memory.c:3359:
  +                                  address_space_compare_name);
---
 softmmu/memory.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 70 insertions(+), 3 deletions(-)

diff --git a/softmmu/memory.c b/softmmu/memory.c
index 5be7d5e7412..9be2017bc38 100644
--- a/softmmu/memory.c
+++ b/softmmu/memory.c
@@ -3282,6 +3282,59 @@ static void mtree_info_flatview(bool dispatch_tree, bool owner)
     /* Free */
     g_hash_table_foreach_remove(views, mtree_info_flatview_free, 0);
     g_hash_table_unref(views);
+
+    return;
+}
+
+struct AddressSpaceInfo {
+    MemoryRegionListHead *ml_head;
+    int counter;
+    bool owner;
+    bool disabled;
+};
+
+/* Returns negative value if a < b; zero if a = b; positive value if a > b. */
+static gint address_space_compare_name(gconstpointer a, gconstpointer b)
+{
+    const AddressSpace *as_a = a;
+    const AddressSpace *as_b = b;
+
+    return g_strcmp0(as_a->name, as_b->name);
+}
+static void mtree_print_as_name(gpointer data, gpointer user_data)
+{
+    AddressSpace *as = data;
+
+    qemu_printf("  - %s\n", as->name);
+}
+
+static void mtree_print_as(gpointer key, gpointer value, gpointer user_data)
+{
+    MemoryRegion *mr = key;
+    GSList *as_same_root_mr_list = value;
+    struct AddressSpaceInfo *asi = user_data;
+    guint same_root_len = g_slist_length(as_same_root_mr_list);
+
+    if (same_root_len == 1) {
+        AddressSpace *as = g_slist_nth_data(as_same_root_mr_list, 0);
+
+        qemu_printf("address-space: %s\n", as->name);
+    } else {
+        qemu_printf("address-space shared %u times:\n", same_root_len);
+        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);
+    qemu_printf("\n");
+}
+
+static gboolean mtree_info_as_free(gpointer key, gpointer value,
+                                   gpointer user_data)
+{
+    GSList *as_same_root_mr_list = value;
+
+    g_slist_free(as_same_root_mr_list);
+
+    return true;
 }
 
 static void mtree_info_as(bool dispatch_tree, bool owner, bool disabled)
@@ -3289,15 +3342,29 @@ static void mtree_info_as(bool dispatch_tree, bool owner, bool disabled)
     MemoryRegionListHead ml_head;
     MemoryRegionList *ml, *ml2;
     AddressSpace *as;
+    GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal);
+    GSList *as_same_root_mr_list;
+    struct AddressSpaceInfo asi = {
+        .ml_head = &ml_head,
+        .owner = owner,
+        .disabled = disabled,
+    };
 
     QTAILQ_INIT(&ml_head);
 
     QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
-        qemu_printf("address-space: %s\n", as->name);
-        mtree_print_mr(as->root, 1, 0, &ml_head, owner, disabled);
-        qemu_printf("\n");
+        /* Create hashtable, key=AS root MR, value = list of AS */
+        as_same_root_mr_list = g_hash_table_lookup(views, as->root);
+        as_same_root_mr_list = g_slist_insert_sorted(as_same_root_mr_list, as,
+                                                     address_space_compare_name);
+        g_hash_table_insert(views, as->root, as_same_root_mr_list);
     }
 
+    /* print address spaces */
+    g_hash_table_foreach(views, mtree_print_as, &asi);
+    g_hash_table_foreach_remove(views, mtree_info_as_free, 0);
+    g_hash_table_unref(views);
+
     /* print aliased regions */
     QTAILQ_FOREACH(ml, &ml_head, mrqueue) {
         qemu_printf("memory-region: %s\n", memory_region_name(ml->mr));
-- 
2.31.1

Re: [PATCH v4 3/3] memory: Have 'info mtree' remove duplicated Address Space information

[David Hildenbrand]

On 01.09.21 18:19, Philippe Mathieu-Daudé wrote:
> Per Peter Maydell [*]:
> 
>    'info mtree' monitor command was designed on the assumption that
>    there's really only one or two interesting address spaces, and
>    with more recent developments that's just not the case any more.
> 
> Similarly about how the FlatView are sorted using a GHashTable,
> sort the AddressSpace objects to remove the duplications (AS
> using the same root MemoryRegion).
> 
> This drastically reduces the output of 'info mtree' on some boards.
> 
> Before:
> 
>    $ (echo info mtree; echo q) \
>      | qemu-system-aarch64 -S -monitor stdio -M raspi3b \
>      | wc -l
>    423
> 
> After:
> 
>    $ (echo info mtree; echo q) \
>      | qemu-system-aarch64 -S -monitor stdio -M raspi3b \
>      | wc -l
>    108
> 
>    (qemu) info mtree
>    address-space: I/O
>      0000000000000000-000000000000ffff (prio 0, i/o): io
> 
>    address-space shared 9 times:
>      - cpu-memory-0
>      - cpu-memory-1
>      - cpu-memory-2
>      - cpu-memory-3
>      - cpu-secure-memory-0
>      - cpu-secure-memory-1
>      - cpu-secure-memory-2
>      - cpu-secure-memory-3
>      - memory
>      0000000000000000-ffffffffffffffff (prio 0, i/o): system
>        0000000000000000-000000003fffffff (prio 0, ram): ram
>        000000003f000000-000000003fffffff (prio 1, i/o): bcm2835-peripherals
>          000000003f003000-000000003f00301f (prio 0, i/o): bcm2835-sys-timer
>          000000003f004000-000000003f004fff (prio -1000, i/o): bcm2835-txp
>          000000003f006000-000000003f006fff (prio 0, i/o): mphi
>          000000003f007000-000000003f007fff (prio 0, i/o): bcm2835-dma
>          000000003f00b200-000000003f00b3ff (prio 0, i/o): bcm2835-ic
>          000000003f00b400-000000003f00b43f (prio -1000, i/o): bcm2835-sp804
>          000000003f00b800-000000003f00bbff (prio 0, i/o): bcm2835-mbox
>          000000003f100000-000000003f1001ff (prio 0, i/o): bcm2835-powermgt
>          000000003f101000-000000003f102fff (prio 0, i/o): bcm2835-cprman
>          000000003f104000-000000003f10400f (prio 0, i/o): bcm2835-rng
>          000000003f200000-000000003f200fff (prio 0, i/o): bcm2835_gpio
>          000000003f201000-000000003f201fff (prio 0, i/o): pl011
>          000000003f202000-000000003f202fff (prio 0, i/o): bcm2835-sdhost
>          000000003f203000-000000003f2030ff (prio -1000, i/o): bcm2835-i2s
>          000000003f204000-000000003f20401f (prio -1000, i/o): bcm2835-spi0
>          000000003f205000-000000003f20501f (prio -1000, i/o): bcm2835-i2c0
>          000000003f20f000-000000003f20f07f (prio -1000, i/o): bcm2835-otp
>          000000003f212000-000000003f212007 (prio 0, i/o): bcm2835-thermal
>          000000003f214000-000000003f2140ff (prio -1000, i/o): bcm2835-spis
>          000000003f215000-000000003f2150ff (prio 0, i/o): bcm2835-aux
>          000000003f300000-000000003f3000ff (prio 0, i/o): sdhci
>          000000003f600000-000000003f6000ff (prio -1000, i/o): bcm2835-smi
>          000000003f804000-000000003f80401f (prio -1000, i/o): bcm2835-i2c1
>          000000003f805000-000000003f80501f (prio -1000, i/o): bcm2835-i2c2
>          000000003f900000-000000003f907fff (prio -1000, i/o): bcm2835-dbus
>          000000003f910000-000000003f917fff (prio -1000, i/o): bcm2835-ave0
>          000000003f980000-000000003f990fff (prio 0, i/o): dwc2
>            000000003f980000-000000003f980fff (prio 0, i/o): dwc2-io
>            000000003f981000-000000003f990fff (prio 0, i/o): dwc2-fifo
>          000000003fc00000-000000003fc00fff (prio -1000, i/o): bcm2835-v3d
>          000000003fe00000-000000003fe000ff (prio -1000, i/o): bcm2835-sdramc
>          000000003fe05000-000000003fe050ff (prio 0, i/o): bcm2835-dma-chan15
>        0000000040000000-00000000400000ff (prio 0, i/o): bcm2836-control
> 
>    address-space shared 4 times:
>      - bcm2835-dma-memory
>      - bcm2835-fb-memory
>      - bcm2835-property-memory
>      - dwc2
>      0000000000000000-00000000ffffffff (prio 0, i/o): bcm2835-gpu
>        0000000000000000-000000003fffffff (prio 0, ram): alias bcm2835-gpu-ram-alias[*] @ram 0000000000000000-000000003fffffff
>        0000000040000000-000000007fffffff (prio 0, ram): alias bcm2835-gpu-ram-alias[*] @ram 0000000000000000-000000003fffffff
>        000000007e000000-000000007effffff (prio 1, i/o): alias bcm2835-peripherals @bcm2835-peripherals 0000000000000000-0000000000ffffff
>        0000000080000000-00000000bfffffff (prio 0, ram): alias bcm2835-gpu-ram-alias[*] @ram 0000000000000000-000000003fffffff
>        00000000c0000000-00000000ffffffff (prio 0, ram): alias bcm2835-gpu-ram-alias[*] @ram 0000000000000000-000000003fffffff
> 
>    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
> 
>    memory-region: ram
>      0000000000000000-000000003fffffff (prio 0, ram): ram
> 
>    memory-region: bcm2835-peripherals
>      000000003f000000-000000003fffffff (prio 1, i/o): bcm2835-peripherals
>        000000003f003000-000000003f00301f (prio 0, i/o): bcm2835-sys-timer
>        000000003f004000-000000003f004fff (prio -1000, i/o): bcm2835-txp
>        000000003f006000-000000003f006fff (prio 0, i/o): mphi
>        000000003f007000-000000003f007fff (prio 0, i/o): bcm2835-dma
>        000000003f00b200-000000003f00b3ff (prio 0, i/o): bcm2835-ic
>        000000003f00b400-000000003f00b43f (prio -1000, i/o): bcm2835-sp804
>        000000003f00b800-000000003f00bbff (prio 0, i/o): bcm2835-mbox
>        000000003f100000-000000003f1001ff (prio 0, i/o): bcm2835-powermgt
>        000000003f101000-000000003f102fff (prio 0, i/o): bcm2835-cprman
>        000000003f104000-000000003f10400f (prio 0, i/o): bcm2835-rng
>        000000003f200000-000000003f200fff (prio 0, i/o): bcm2835_gpio
>        000000003f201000-000000003f201fff (prio 0, i/o): pl011
>        000000003f202000-000000003f202fff (prio 0, i/o): bcm2835-sdhost
>        000000003f203000-000000003f2030ff (prio -1000, i/o): bcm2835-i2s
>        000000003f204000-000000003f20401f (prio -1000, i/o): bcm2835-spi0
>        000000003f205000-000000003f20501f (prio -1000, i/o): bcm2835-i2c0
>        000000003f20f000-000000003f20f07f (prio -1000, i/o): bcm2835-otp
>        000000003f212000-000000003f212007 (prio 0, i/o): bcm2835-thermal
>        000000003f214000-000000003f2140ff (prio -1000, i/o): bcm2835-spis
>        000000003f215000-000000003f2150ff (prio 0, i/o): bcm2835-aux
>        000000003f300000-000000003f3000ff (prio 0, i/o): sdhci
>        000000003f600000-000000003f6000ff (prio -1000, i/o): bcm2835-smi
>        000000003f804000-000000003f80401f (prio -1000, i/o): bcm2835-i2c1
>        000000003f805000-000000003f80501f (prio -1000, i/o): bcm2835-i2c2
>        000000003f900000-000000003f907fff (prio -1000, i/o): bcm2835-dbus
>        000000003f910000-000000003f917fff (prio -1000, i/o): bcm2835-ave0
>        000000003f980000-000000003f990fff (prio 0, i/o): dwc2
>          000000003f980000-000000003f980fff (prio 0, i/o): dwc2-io
>          000000003f981000-000000003f990fff (prio 0, i/o): dwc2-fifo
>        000000003fc00000-000000003fc00fff (prio -1000, i/o): bcm2835-v3d
>        000000003fe00000-000000003fe000ff (prio -1000, i/o): bcm2835-sdramc
>        000000003fe05000-000000003fe050ff (prio 0, i/o): bcm2835-dma-chan15
> 
>    (qemu) q
> 
> [*] https://www.mail-archive.com/qemu-devel@nongnu.org/msg829821.html
> 
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
> Pending question from David on v3:
> 
>    We can still distinguish from a completely empty AS, because we don't have an empty line here, correct?
> 
> checkpatch warning (81 chars):
> 
>    WARNING: line over 80 characters
>    #86: FILE: softmmu/memory.c:3359:
>    +                                  address_space_compare_name);
> ---
>   softmmu/memory.c | 73 ++++++++++++++++++++++++++++++++++++++++++++++--
>   1 file changed, 70 insertions(+), 3 deletions(-)
> 
> diff --git a/softmmu/memory.c b/softmmu/memory.c
> index 5be7d5e7412..9be2017bc38 100644
> --- a/softmmu/memory.c
> +++ b/softmmu/memory.c
> @@ -3282,6 +3282,59 @@ static void mtree_info_flatview(bool dispatch_tree, bool owner)
>       /* Free */
>       g_hash_table_foreach_remove(views, mtree_info_flatview_free, 0);
>       g_hash_table_unref(views);
> +
> +    return;

^ unnecessary

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb

Re: [PATCH v4 3/3] memory: Have 'info mtree' remove duplicated Address Space information

[Richard Henderson]

On 9/1/21 6:19 PM, Philippe Mathieu-Daudé wrote:
> @@ -3282,6 +3282,59 @@ static void mtree_info_flatview(bool dispatch_tree, bool owner)
>       /* Free */
>       g_hash_table_foreach_remove(views, mtree_info_flatview_free, 0);
>       g_hash_table_unref(views);
> +
> +    return;
> +}

Bogus.

> +    return g_strcmp0(as_a->name, as_b->name);
> +}
> +static void mtree_print_as_name(gpointer data, gpointer user_data)

Mind the missing vertical whitespace.

Otherwise,
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~