Patchwork [v2] memory: simple memory tree printer

login
register
mail settings
Submitter Blue Swirl
Date Sept. 17, 2011, 7:27 p.m.
Message ID <CAAu8pHtzoajmvbQ9+Cx-_GVt5wwL34N+eTNj2V6oN+d3kQk1ow@mail.gmail.com>
Download mbox | patch
Permalink /patch/115159/
State New
Headers show

Comments

Blue Swirl - Sept. 17, 2011, 7:27 p.m.
Add a monitor command 'info mtree' to show the memory hierarchy
much like /proc/iomem in Linux.

Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
---
v1->v2: use /proc/iomem format.
---
 memory.c  |   27 +++++++++++++++++++++++++++
 memory.h  |    2 ++
 monitor.c |    7 +++++++
 3 files changed, 36 insertions(+), 0 deletions(-)
Jan Kiszka - Sept. 18, 2011, 12:57 p.m.
On 2011-09-17 21:27, Blue Swirl wrote:
> Add a monitor command 'info mtree' to show the memory hierarchy
> much like /proc/iomem in Linux.
> 
> Signed-off-by: Blue Swirl <blauwirbel@gmail.com>
> ---
> v1->v2: use /proc/iomem format.
> ---
>  memory.c  |   27 +++++++++++++++++++++++++++
>  memory.h  |    2 ++
>  monitor.c |    7 +++++++
>  3 files changed, 36 insertions(+), 0 deletions(-)
> 
> diff --git a/memory.c b/memory.c
> index 101b67c..275f5cf 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -17,6 +17,7 @@
>  #include "bitops.h"
>  #include "kvm.h"
>  #include <assert.h>
> +#include "monitor.h"
> 
>  unsigned memory_region_transaction_depth = 0;
> 
> @@ -1253,3 +1254,29 @@ void set_system_io_map(MemoryRegion *mr)
>      address_space_io.root = mr;
>      memory_region_update_topology();
>  }
> +
> +static void mtree_print_mr(Monitor *mon, MemoryRegion *mr, unsigned int level)
> +{
> +    MemoryRegion *submr;
> +    unsigned int i;
> +
> +    for (i = 0; i < level; i++) {
> +        monitor_printf(mon, "  ");
> +    }
> +    monitor_printf(mon, TARGET_FMT_plx "-" TARGET_FMT_plx " : %s\n",
> +                   mr->addr, mr->addr + (target_phys_addr_t)mr->size - 1,
> +                   mr->name);

I would prefer absolute addresses here. And the priority field needs to
be dumped as well.

Jan
Avi Kivity - Sept. 18, 2011, 1:53 p.m.
On 09/17/2011 10:27 PM, Blue Swirl wrote:
> Add a monitor command 'info mtree' to show the memory hierarchy
> much like /proc/iomem in Linux.
>
>

Still missing alias support.  PCI would be invisible on a PC (or any 
machine which has PCI holes implemented properly).

Maybe we need to dump both the memory tree and the flat view - the 
memory tree for the logical hierarchy and the flat view to see what 
actually happens (I have an address, where does it go?)
Blue Swirl - Sept. 18, 2011, 2:07 p.m.
On Sun, Sep 18, 2011 at 1:53 PM, Avi Kivity <avi@redhat.com> wrote:
> On 09/17/2011 10:27 PM, Blue Swirl wrote:
>>
>> Add a monitor command 'info mtree' to show the memory hierarchy
>> much like /proc/iomem in Linux.
>>
>>
>
> Still missing alias support.  PCI would be invisible on a PC (or any machine
> which has PCI holes implemented properly).

Yes, that annoyed me too when debugging the PPC patch. But how should
that look like? Consider for example that in the PPC case, range 0 to
0x80000000 is RAM from CPU point of view but only PCI MMIO space when
looking after the PCI bridge. I/O shouldn't need separate handling if
the CPU does not have PIO instructions, but instead PIO space is
mapped as MMIO as on Sparc64 and PPC. I/O should be visible there.

> Maybe we need to dump both the memory tree and the flat view - the memory
> tree for the logical hierarchy and the flat view to see what actually
> happens (I have an address, where does it go?)

I have some trouble thinking about how to print fully converted,
per-CPU memory trees. Also, if the memory API is fully embraced and
extended to handle DMA and IOMMUs, each device could have a different
view on the system memory. Perhaps the tool should take a device ID
(also CPU ID) as a parameter to give it a starting point.
Avi Kivity - Sept. 18, 2011, 2:26 p.m.
On 09/18/2011 05:07 PM, Blue Swirl wrote:
> On Sun, Sep 18, 2011 at 1:53 PM, Avi Kivity<avi@redhat.com>  wrote:
> >  On 09/17/2011 10:27 PM, Blue Swirl wrote:
> >>
> >>  Add a monitor command 'info mtree' to show the memory hierarchy
> >>  much like /proc/iomem in Linux.
> >>
> >>
> >
> >  Still missing alias support.  PCI would be invisible on a PC (or any machine
> >  which has PCI holes implemented properly).
>
> Yes, that annoyed me too when debugging the PPC patch. But how should
> that look like? Consider for example that in the PPC case, range 0 to
> 0x80000000 is RAM from CPU point of view but only PCI MMIO space when
> looking after the PCI bridge. I/O shouldn't need separate handling if
> the CPU does not have PIO instructions, but instead PIO space is
> mapped as MMIO as on Sparc64 and PPC. I/O should be visible there.

Have some notation for a reference.  Example for PC:

Memory:
00000000-7fffffffffffffff system-memory container
   00000000-0009ffff alias @ram 00000000-0009ffff
   000a0000-000bffff alias @pci 000a0000-000bffff
   ...
   e0000000-ffffffff alias @pci e0000000-ffffffff

pci:
00000000-ffffffffff pci container
   000a0000-000bffff alias @vgam 00000000-0001ffff
   e0000000-e1ffffff alias @vram 00000000-01ffffff
   e2000000-e2001000 e1000-mmio

vram:
00000000-01ffffff vram ram

(each time you encounter a new alias target, add it to the print queue, 
it should work itself out naturally)

>
> >  Maybe we need to dump both the memory tree and the flat view - the memory
> >  tree for the logical hierarchy and the flat view to see what actually
> >  happens (I have an address, where does it go?)
>
> I have some trouble thinking about how to print fully converted,
> per-CPU memory trees. Also, if the memory API is fully embraced and
> extended to handle DMA and IOMMUs, each device could have a different
> view on the system memory. Perhaps the tool should take a device ID
> (also CPU ID) as a parameter to give it a starting point.

The possibilities are endless.

Patch

diff --git a/memory.c b/memory.c
index 101b67c..275f5cf 100644
--- a/memory.c
+++ b/memory.c
@@ -17,6 +17,7 @@ 
 #include "bitops.h"
 #include "kvm.h"
 #include <assert.h>
+#include "monitor.h"

 unsigned memory_region_transaction_depth = 0;

@@ -1253,3 +1254,29 @@  void set_system_io_map(MemoryRegion *mr)
     address_space_io.root = mr;
     memory_region_update_topology();
 }
+
+static void mtree_print_mr(Monitor *mon, MemoryRegion *mr, unsigned int level)
+{
+    MemoryRegion *submr;
+    unsigned int i;
+
+    for (i = 0; i < level; i++) {
+        monitor_printf(mon, "  ");
+    }
+    monitor_printf(mon, TARGET_FMT_plx "-" TARGET_FMT_plx " : %s\n",
+                   mr->addr, mr->addr + (target_phys_addr_t)mr->size - 1,
+                   mr->name);
+
+    QTAILQ_FOREACH(submr, &mr->subregions, subregions_link) {
+        mtree_print_mr(mon, submr, level + 1);
+    }
+}
+
+void mtree_info(Monitor *mon)
+{
+    monitor_printf(mon, "memory\n");
+    mtree_print_mr(mon, address_space_memory.root, 0);
+
+    monitor_printf(mon, "I/O\n");
+    mtree_print_mr(mon, address_space_io.root, 0);
+}
diff --git a/memory.h b/memory.h
index 06b83ae..09d8e29 100644
--- a/memory.h
+++ b/memory.h
@@ -500,6 +500,8 @@  void memory_region_transaction_begin(void);
  */
 void memory_region_transaction_commit(void);

+void mtree_info(Monitor *mon);
+
 #endif

 #endif
diff --git a/monitor.c b/monitor.c
index 03ae997..0302446 100644
--- a/monitor.c
+++ b/monitor.c
@@ -2968,6 +2968,13 @@  static const mon_cmd_t info_cmds[] = {
     },
 #endif
     {
+        .name       = "mtree",
+        .args_type  = "",
+        .params     = "",
+        .help       = "show memory tree",
+        .mhandler.info = mtree_info,
+    },
+    {
         .name       = "jit",
         .args_type  = "",
         .params     = "",