[Qemu-devel] [RFC] qapi: New command query-mtree

[Qemu-devel] [RFC] qapi: New command query-mtree

[Marc Marí]

Add command query-mtree to get the memory tree of the guest.

As we were looking for a flexible solution on accessing the guest memory from
qtests, Stefan came with the idea to implement this new qmp command.

This way, the result can be parsed, and the RAM direction extracted, so only
a generic qtest malloc is necessary and not one per machine, as it is
implemented at the moment (malloc-pc uses fw_cfg).

The actual output is this: http://pastebin.com/nHAH9Jie
Which corresponds to this info mtree: http://pastebin.com/B5vw8DDf

Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
---
 memory.c         |  148 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 qapi-schema.json |   68 +++++++++++++++++++++++++
 qmp-commands.hx  |   19 +++++++
 3 files changed, 235 insertions(+)

diff --git a/memory.c b/memory.c
index 42317a2..6de6fa7 100644
--- a/memory.c
+++ b/memory.c
@@ -20,6 +20,7 @@
 #include "qemu/bitops.h"
 #include "qom/object.h"
 #include "trace.h"
+#include "qmp-commands.h"
 #include <assert.h>
 
 #include "exec/memory-internal.h"
@@ -2100,6 +2101,153 @@ void mtree_info(fprintf_function mon_printf, void *f)
     }
 }
 
+static MemRegion *qmp_mtree_mr(const MemoryRegion *mr, hwaddr base,
+                                   MemoryRegionListHead *alias_queue)
+{
+    MemoryRegionList *new_ml, *ml, *next_ml;
+    MemoryRegionListHead submr_print_queue;
+    MemRegionList *cur_item = NULL, *subregion;
+    MemRegion *region = NULL;
+    const MemoryRegion *submr;
+
+    if (!mr || !mr->enabled) {
+        return region;
+    }
+
+    region = g_malloc0(sizeof(*region));
+
+    if (mr->alias) {
+        MemoryRegionList *ml;
+        bool found = false;
+
+        /* check if the alias is already in the queue */
+        QTAILQ_FOREACH(ml, alias_queue, queue) {
+            if (ml->mr == mr->alias) {
+                found = true;
+            }
+        }
+
+        if (!found) {
+            ml = g_new(MemoryRegionList, 1);
+            ml->mr = mr->alias;
+            QTAILQ_INSERT_TAIL(alias_queue, ml, queue);
+        }
+
+        region->base = base+mr->addr;
+        region->size = int128_get64(int128_sub(mr->size, int128_one()));
+        region->prio = mr->priority;
+        region->read = mr->romd_mode;
+        region->write = !mr->readonly && !(mr->rom_device && mr->romd_mode);
+        region->ram = mr->ram;
+        region->name = g_strdup(memory_region_name(mr));
+        region->has_alias = true;
+        region->alias = g_strdup(memory_region_name(mr->alias));
+    } else {
+        region->base = base+mr->addr;
+        region->size = int128_get64(int128_sub(mr->size, int128_one()));
+        region->prio = mr->priority;
+        region->read = mr->romd_mode;
+        region->write = !mr->readonly && !(mr->rom_device && mr->romd_mode);
+        region->ram = mr->ram;
+        region->name = g_strdup(memory_region_name(mr));
+        region->has_alias = false;
+    }
+
+    QTAILQ_INIT(&submr_print_queue);
+
+    QTAILQ_FOREACH(submr, &mr->subregions, subregions_link) {
+        new_ml = g_new(MemoryRegionList, 1);
+        new_ml->mr = submr;
+        QTAILQ_FOREACH(ml, &submr_print_queue, queue) {
+            if (new_ml->mr->addr < ml->mr->addr ||
+                (new_ml->mr->addr == ml->mr->addr &&
+                 new_ml->mr->priority > ml->mr->priority)) {
+                QTAILQ_INSERT_BEFORE(ml, new_ml, queue);
+                new_ml = NULL;
+                break;
+            }
+        }
+        if (new_ml) {
+            QTAILQ_INSERT_TAIL(&submr_print_queue, new_ml, queue);
+        }
+    }
+
+    QTAILQ_FOREACH(ml, &submr_print_queue, queue) {
+        subregion = g_malloc0(sizeof(*subregion));
+        subregion->value = qmp_mtree_mr(ml->mr, base + mr->addr, alias_queue);
+
+        if (subregion->value != NULL) {
+            if (!cur_item) {
+                region->submr = cur_item = subregion;
+            } else {
+                cur_item->next = subregion;
+                cur_item = subregion;
+            }
+        } else {
+            g_free(subregion);
+        }
+    }
+
+    region->has_submr = (region->submr != NULL);
+
+    QTAILQ_FOREACH_SAFE(ml, &submr_print_queue, queue, next_ml) {
+        g_free(ml);
+    }
+
+    return region;
+}
+
+MemTree *qmp_query_mtree(Error **errp)
+{
+    MemoryRegionListHead ml_head;
+    MemoryRegionList *ml, *ml2;
+    AddressSpace *as;
+    AddrSpaceList *head = NULL, *cur_item = NULL, *space;
+    MemTree *mt = g_malloc0(sizeof(*mt));
+
+    QTAILQ_INIT(&ml_head);
+
+    QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
+        space = g_malloc0(sizeof(*space));
+        space->value = g_malloc0(sizeof(*space->value));
+        space->value->name = g_strdup(as->name);
+        space->value->mem = qmp_mtree_mr(as->root, 0, &ml_head);
+
+        if (!cur_item) {
+            head = cur_item = space;
+        } else {
+            cur_item->next = space;
+            cur_item = space;
+        }
+    }
+
+    mt->spaces = head;
+    head = NULL;
+    cur_item = NULL;
+
+    QTAILQ_FOREACH(ml, &ml_head, queue) {
+        space = g_malloc0(sizeof(*space));
+        space->value = g_malloc0(sizeof(*space->value));
+        space->value->name = g_strdup(memory_region_name(ml->mr));
+        space->value->mem = qmp_mtree_mr(ml->mr, 0, &ml_head);
+
+        if (!cur_item) {
+            head = cur_item = space;
+        } else {
+            cur_item->next = space;
+            cur_item = space;
+        }
+    }
+
+    mt->aliases = head;
+
+    QTAILQ_FOREACH_SAFE(ml, &ml_head, queue, ml2) {
+        g_free(ml);
+    }
+
+    return mt;
+}
+
 static const TypeInfo memory_region_info = {
     .parent             = TYPE_OBJECT,
     .name               = TYPE_MEMORY_REGION,
diff --git a/qapi-schema.json b/qapi-schema.json
index 341f417..bc35bd9 100644
--- a/qapi-schema.json
+++ b/qapi-schema.json
@@ -3481,3 +3481,71 @@
 # Since: 2.1
 ##
 { 'command': 'rtc-reset-reinjection' }
+
+##
+# @MemRegion:
+#
+# Information about a memory region
+#
+# @base: base address
+#
+# @size: size
+#
+# @prio: priority
+#
+# @read: read permitted
+#
+# @write: write permitted
+#
+# @ram: RAM region
+#
+# @name: name of the region
+#
+# @alias: #optional alias of this region
+#
+# @submr: #optional submemory regions of this region
+#
+# Since: 2.x
+##
+{ 'type': 'MemRegion',
+  'data': {'base': 'uint64', 'size': 'uint64', 'prio': 'uint32', 'read': 'bool',
+           'write': 'bool', 'ram': 'bool', 'name': 'str',
+           '*alias': 'str', '*submr': ['MemRegion']} }
+
+##
+# @AddrSpace:
+#
+# An address space of the system
+#
+# @name: name of the address space
+#
+# @mem: a list of memory regions in the address space
+#
+# Since: 2.x
+##
+{ 'type': 'AddrSpace', 'data': {'name': 'str', 'mem': 'MemRegion'} }
+
+##
+# @MemTree:
+#
+# Memory tree of the system
+#
+# @spaces: Address spaces of the system
+#
+# @aliases: Aliased memory regions
+#
+# Since: 2.x
+##
+{ 'type': 'MemTree', 'data': {'spaces': ['AddrSpace'],
+                                'aliases': ['AddrSpace']} }
+
+##
+# @query-mtree:
+#
+# Return the memory distribution of the guest
+#
+# Returns: a list of @AddrSpace
+#
+# Since: 2.x
+##
+{ 'command': 'query-mtree', 'returns': 'MemTree' }
diff --git a/qmp-commands.hx b/qmp-commands.hx
index 4be4765..22f91b0 100644
--- a/qmp-commands.hx
+++ b/qmp-commands.hx
@@ -3755,3 +3755,22 @@ Example:
 <- { "return": {} }
 
 EQMP
+    {
+        .name       = "query-mtree",
+        .args_type  = "",
+        .mhandler.cmd_new = qmp_marshal_input_query_mtree,
+    },
+SQMP
+query-mtree
+---------
+
+Memory tree.
+
+The returned value is a json-array of the memory distribution of the system.
+Each address space is represented by a json-object, which has a name, and a
+json-array of all memory regions that contain. Each memory region is
+represented by a json-object.
+
+(Missing schema and example)
+
+EQMP
-- 
1.7.10.4

Re: [Qemu-devel] [RFC] qapi: New command query-mtree

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 5659 bytes --]

On 08/20/2014 11:46 AM, Marc Marí wrote:
> Add command query-mtree to get the memory tree of the guest.
> 
> As we were looking for a flexible solution on accessing the guest memory from
> qtests, Stefan came with the idea to implement this new qmp command.
> 
> This way, the result can be parsed, and the RAM direction extracted, so only
> a generic qtest malloc is necessary and not one per machine, as it is
> implemented at the moment (malloc-pc uses fw_cfg).
> 
> The actual output is this: http://pastebin.com/nHAH9Jie
> Which corresponds to this info mtree: http://pastebin.com/B5vw8DDf

Alas, these pastebins will expire, even when we still read qemu.git many
years from now.  If it weren't for the fact that 116 lines of mtree is
exploding into 1033 lines of prettified JSON, I'd suggest putting it
here in the commit message.  Or if you can come up with a simpler
testcase, or summarize a sample section here, even that would be nicer
than pointing to what will eventually be a stale URL.

> 
> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
>  memory.c         |  148 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qapi-schema.json |   68 +++++++++++++++++++++++++
>  qmp-commands.hx  |   19 +++++++
>  3 files changed, 235 insertions(+)
> 

>  
> +static MemRegion *qmp_mtree_mr(const MemoryRegion *mr, hwaddr base,
> +                                   MemoryRegionListHead *alias_queue)

Indentation is off.

> +{
> +    MemoryRegionList *new_ml, *ml, *next_ml;
> +    MemoryRegionListHead submr_print_queue;
> +    MemRegionList *cur_item = NULL, *subregion;
> +    MemRegion *region = NULL;
> +    const MemoryRegion *submr;
> +
> +    if (!mr || !mr->enabled) {
> +        return region;
> +    }
> +
> +    region = g_malloc0(sizeof(*region));

g_new0 is safer than g_malloc0.


> +
> +        region->base = base+mr->addr;

Spaces around binary operators.

> +
> +MemTree *qmp_query_mtree(Error **errp)
> +{
> +    MemoryRegionListHead ml_head;
> +    MemoryRegionList *ml, *ml2;
> +    AddressSpace *as;
> +    AddrSpaceList *head = NULL, *cur_item = NULL, *space;
> +    MemTree *mt = g_malloc0(sizeof(*mt));
> +
> +    QTAILQ_INIT(&ml_head);
> +
> +    QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> +        space = g_malloc0(sizeof(*space));
> +        space->value = g_malloc0(sizeof(*space->value));

Several more places where g_new0 is preferred


> +++ b/qapi-schema.json
> @@ -3481,3 +3481,71 @@
>  # Since: 2.1
>  ##
>  { 'command': 'rtc-reset-reinjection' }
> +
> +##
> +# @MemRegion:
> +#
> +# Information about a memory region

> +#
> +# @submr: #optional submemory regions of this region

Bike-shedding - no need to abbreviate.  I'd be fine calling it
'subregions'.  Likewise, s/prio/priority/

> +#
> +# Since: 2.x

s/2.x/2.2/

> +##
> +{ 'type': 'MemRegion',
> +  'data': {'base': 'uint64', 'size': 'uint64', 'prio': 'uint32', 'read': 'bool',
> +           'write': 'bool', 'ram': 'bool', 'name': 'str',
> +           '*alias': 'str', '*submr': ['MemRegion']} }

Looks reasonable (modulo any name changes).

> +
> +##
> +# @AddrSpace:
> +#
> +# An address space of the system
> +#
> +# @name: name of the address space
> +#
> +# @mem: a list of memory regions in the address space
> +#
> +# Since: 2.x

s/2.x/2.2/

> +##
> +{ 'type': 'AddrSpace', 'data': {'name': 'str', 'mem': 'MemRegion'} }
> +
> +##
> +# @MemTree:
> +#
> +# Memory tree of the system
> +#
> +# @spaces: Address spaces of the system
> +#
> +# @aliases: Aliased memory regions
> +#
> +# Since: 2.x

s/2.x/2.2/

> +##
> +{ 'type': 'MemTree', 'data': {'spaces': ['AddrSpace'],
> +                                'aliases': ['AddrSpace']} }

Whitespace doesn't matter, but consistent alignment would look better.

> +
> +##
> +# @query-mtree:
> +#
> +# Return the memory distribution of the guest
> +#
> +# Returns: a list of @AddrSpace
> +#
> +# Since: 2.x

s/2.x/2.2/

> +##
> +{ 'command': 'query-mtree', 'returns': 'MemTree' }

I was expecting ['MemTree'] (an array of structs), but you gave
'MemTreee' (a struct of parallel arrays).  Am I guaranteed that
returns.spaces and returns.aliases are arrays of the same length?  If
not, what's the difference between 'spaces' and 'aliases' (that is, why
do I need two arrays)?  Should the designation of 'space' vs. 'alias' be
part of the 'AddrSpace' struct, rather than having to be learned
indirectly by which of the two arrays it appeared in?

> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 4be4765..22f91b0 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3755,3 +3755,22 @@ Example:
>  <- { "return": {} }
>  
>  EQMP
> +    {
> +        .name       = "query-mtree",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_query_mtree,
> +    },
> +SQMP
> +query-mtree
> +---------
> +
> +Memory tree.
> +
> +The returned value is a json-array of the memory distribution of the system.

Docs don't match your qapi schema.  Let's make sure we get the schema
correct, and then make the docs match.

> +Each address space is represented by a json-object, which has a name, and a
> +json-array of all memory regions that contain.

that contain what?  Did you mean "that it contains"?

> Each memory region is
> +represented by a json-object.
> +
> +(Missing schema and example)

Indeed, hence this being an RFC :)

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [RFC] qapi: New command query-mtree

[Marc Marí]

El Wed, 20 Aug 2014 13:09:20 -0600
Eric Blake <eblake@redhat.com> escribió:
> On 08/20/2014 11:46 AM, Marc Marí wrote:
> > Add command query-mtree to get the memory tree of the guest.
> > 
> > As we were looking for a flexible solution on accessing the guest
> > memory from qtests, Stefan came with the idea to implement this new
> > qmp command.
> > 
> > This way, the result can be parsed, and the RAM direction
> > extracted, so only a generic qtest malloc is necessary and not one
> > per machine, as it is implemented at the moment (malloc-pc uses
> > fw_cfg).
> > 
> > The actual output is this: http://pastebin.com/nHAH9Jie
> > Which corresponds to this info mtree: http://pastebin.com/B5vw8DDf
> 
> Alas, these pastebins will expire, even when we still read qemu.git
> many years from now.  If it weren't for the fact that 116 lines of
> mtree is exploding into 1033 lines of prettified JSON, I'd suggest
> putting it here in the commit message.  Or if you can come up with a
> simpler testcase, or summarize a sample section here, even that would
> be nicer than pointing to what will eventually be a stale URL.

In the final commit message I was thinking in dropping the "why" and the
examples. I put those just for the RFC.

> 
> > +##
> > +{ 'type': 'MemTree', 'data': {'spaces': ['AddrSpace'],
> > +                                'aliases': ['AddrSpace']} }
> 
> Whitespace doesn't matter, but consistent alignment would look better.
> 
> > +
> > +##
> > +# @query-mtree:
> > +#
> > +# Return the memory distribution of the guest
> > +#
> > +# Returns: a list of @AddrSpace
> > +#
> > +# Since: 2.x
> 
> s/2.x/2.2/
> 
> > +##
> > +{ 'command': 'query-mtree', 'returns': 'MemTree' }
> 
> I was expecting ['MemTree'] (an array of structs), but you gave
> 'MemTreee' (a struct of parallel arrays).  Am I guaranteed that
> returns.spaces and returns.aliases are arrays of the same length?  If
> not, what's the difference between 'spaces' and 'aliases' (that is,
> why do I need two arrays)?  Should the designation of 'space' vs.
> 'alias' be part of the 'AddrSpace' struct, rather than having to be
> learned indirectly by which of the two arrays it appeared in?
> 

I put it this way to do it as similar as possible as "info mtree" where
there are two sections, "alias" and "the other". But this could be just
specified inside AddrSpace, and return a ['AddrSpace'] or change its
name to ['MemTree']

> > diff --git a/qmp-commands.hx b/qmp-commands.hx
> > index 4be4765..22f91b0 100644
> > --- a/qmp-commands.hx
> > +++ b/qmp-commands.hx
> > @@ -3755,3 +3755,22 @@ Example:
> >  <- { "return": {} }
> >  
> >  EQMP
> > +    {
> > +        .name       = "query-mtree",
> > +        .args_type  = "",
> > +        .mhandler.cmd_new = qmp_marshal_input_query_mtree,
> > +    },
> > +SQMP
> > +query-mtree
> > +---------
> > +
> > +Memory tree.
> > +
> > +The returned value is a json-array of the memory distribution of
> > the system.
> 
> Docs don't match your qapi schema.  Let's make sure we get the schema
> correct, and then make the docs match. 

I changed the schema after writing the docs, and I didn't update the
docs.

> > +Each address space is represented by a json-object, which has a
> > name, and a +json-array of all memory regions that contain.
> 
> that contain what?  Did you mean "that it contains"?

Yes.

Thanks for your comments
Marc

Re: [Qemu-devel] [RFC] qapi: New command query-mtree

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 2525 bytes --]

On 08/20/2014 01:09 PM, Eric Blake wrote:
> On 08/20/2014 11:46 AM, Marc Marí wrote:
>> Add command query-mtree to get the memory tree of the guest.
>>
>> As we were looking for a flexible solution on accessing the guest memory from
>> qtests, Stefan came with the idea to implement this new qmp command.
>>
>> This way, the result can be parsed, and the RAM direction extracted, so only
>> a generic qtest malloc is necessary and not one per machine, as it is
>> implemented at the moment (malloc-pc uses fw_cfg).
>>
>> The actual output is this: http://pastebin.com/nHAH9Jie
>> Which corresponds to this info mtree: http://pastebin.com/B5vw8DDf
> 

> 
>> +##
>> +{ 'command': 'query-mtree', 'returns': 'MemTree' }
> 
> I was expecting ['MemTree'] (an array of structs), but you gave
> 'MemTreee' (a struct of parallel arrays).  Am I guaranteed that
> returns.spaces and returns.aliases are arrays of the same length?  If
> not, what's the difference between 'spaces' and 'aliases' (that is, why
> do I need two arrays)?  Should the designation of 'space' vs. 'alias' be
> part of the 'AddrSpace' struct, rather than having to be learned
> indirectly by which of the two arrays it appeared in?
> 

Looking at your pastebin, I'm seeing some repetition.  For example:

return.spaces[0].mem.submr[0] is (line 11 in the pastebin):

            {
              u'name': u'ram-below-4g',
              u'prio': 0,
              u'read': True,
              u'ram': False,
              u'write': True,
              u'alias': u'pc.ram',
              u'base': 0,
              u'size': 536870911
            },

Later, return.aliases[0] is (line 858):


        u'mem': {
          u'name': u'pc.ram',
          u'prio': 0,
          u'read': True,
          u'ram': True,
          u'write': True,
          u'base': 0,
          u'size': 536870911
        },
        u'name': u'pc.ram'

Isn't that two descriptions of the same region?  Would it be better if
one of the references in the JSON was just a name, and make the caller
look up that name in the other location, instead of inlining the full
struct at both locations?  On the other hand, the 'ram' field for that
memory region appears to be different depending on whether you access it
via the subregion of system vs. via the pc.ram alias.  Okay, maybe that
much complexity really is necessary.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 539 bytes --]

Re: [Qemu-devel] [RFC] qapi: New command query-mtree

[Paolo Bonzini]

Il 20/08/2014 19:46, Marc Marí ha scritto:
> Add command query-mtree to get the memory tree of the guest.
> 
> As we were looking for a flexible solution on accessing the guest memory from
> qtests, Stefan came with the idea to implement this new qmp command.
> 
> This way, the result can be parsed, and the RAM direction extracted, so only
> a generic qtest malloc is necessary and not one per machine, as it is
> implemented at the moment (malloc-pc uses fw_cfg).
> 
> The actual output is this: http://pastebin.com/nHAH9Jie
> Which corresponds to this info mtree: http://pastebin.com/B5vw8DDf

I don't like this idea very much.  libqos should be using the real
memory map information from the machine.  In the case of x86, that means
fw_cfg; in the case of ARM, that would mean using the device tree.
Getting the information from an out-of-band channel (such as QMP) is
basically cheating. :)

If you had a memory map abstraction in libqos, malloc could be generic.
 Perhaps you can start doing that for PC?

Paolo

> Signed-off-by: Marc Marí <marc.mari.barcelo@gmail.com>
> ---
>  memory.c         |  148 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  qapi-schema.json |   68 +++++++++++++++++++++++++
>  qmp-commands.hx  |   19 +++++++
>  3 files changed, 235 insertions(+)
> 
> diff --git a/memory.c b/memory.c
> index 42317a2..6de6fa7 100644
> --- a/memory.c
> +++ b/memory.c
> @@ -20,6 +20,7 @@
>  #include "qemu/bitops.h"
>  #include "qom/object.h"
>  #include "trace.h"
> +#include "qmp-commands.h"
>  #include <assert.h>
>  
>  #include "exec/memory-internal.h"
> @@ -2100,6 +2101,153 @@ void mtree_info(fprintf_function mon_printf, void *f)
>      }
>  }
>  
> +static MemRegion *qmp_mtree_mr(const MemoryRegion *mr, hwaddr base,
> +                                   MemoryRegionListHead *alias_queue)
> +{
> +    MemoryRegionList *new_ml, *ml, *next_ml;
> +    MemoryRegionListHead submr_print_queue;
> +    MemRegionList *cur_item = NULL, *subregion;
> +    MemRegion *region = NULL;
> +    const MemoryRegion *submr;
> +
> +    if (!mr || !mr->enabled) {
> +        return region;
> +    }
> +
> +    region = g_malloc0(sizeof(*region));
> +
> +    if (mr->alias) {
> +        MemoryRegionList *ml;
> +        bool found = false;
> +
> +        /* check if the alias is already in the queue */
> +        QTAILQ_FOREACH(ml, alias_queue, queue) {
> +            if (ml->mr == mr->alias) {
> +                found = true;
> +            }
> +        }
> +
> +        if (!found) {
> +            ml = g_new(MemoryRegionList, 1);
> +            ml->mr = mr->alias;
> +            QTAILQ_INSERT_TAIL(alias_queue, ml, queue);
> +        }
> +
> +        region->base = base+mr->addr;
> +        region->size = int128_get64(int128_sub(mr->size, int128_one()));
> +        region->prio = mr->priority;
> +        region->read = mr->romd_mode;
> +        region->write = !mr->readonly && !(mr->rom_device && mr->romd_mode);
> +        region->ram = mr->ram;
> +        region->name = g_strdup(memory_region_name(mr));
> +        region->has_alias = true;
> +        region->alias = g_strdup(memory_region_name(mr->alias));
> +    } else {
> +        region->base = base+mr->addr;
> +        region->size = int128_get64(int128_sub(mr->size, int128_one()));
> +        region->prio = mr->priority;
> +        region->read = mr->romd_mode;
> +        region->write = !mr->readonly && !(mr->rom_device && mr->romd_mode);
> +        region->ram = mr->ram;
> +        region->name = g_strdup(memory_region_name(mr));
> +        region->has_alias = false;
> +    }
> +
> +    QTAILQ_INIT(&submr_print_queue);
> +
> +    QTAILQ_FOREACH(submr, &mr->subregions, subregions_link) {
> +        new_ml = g_new(MemoryRegionList, 1);
> +        new_ml->mr = submr;
> +        QTAILQ_FOREACH(ml, &submr_print_queue, queue) {
> +            if (new_ml->mr->addr < ml->mr->addr ||
> +                (new_ml->mr->addr == ml->mr->addr &&
> +                 new_ml->mr->priority > ml->mr->priority)) {
> +                QTAILQ_INSERT_BEFORE(ml, new_ml, queue);
> +                new_ml = NULL;
> +                break;
> +            }
> +        }
> +        if (new_ml) {
> +            QTAILQ_INSERT_TAIL(&submr_print_queue, new_ml, queue);
> +        }
> +    }
> +
> +    QTAILQ_FOREACH(ml, &submr_print_queue, queue) {
> +        subregion = g_malloc0(sizeof(*subregion));
> +        subregion->value = qmp_mtree_mr(ml->mr, base + mr->addr, alias_queue);
> +
> +        if (subregion->value != NULL) {
> +            if (!cur_item) {
> +                region->submr = cur_item = subregion;
> +            } else {
> +                cur_item->next = subregion;
> +                cur_item = subregion;
> +            }
> +        } else {
> +            g_free(subregion);
> +        }
> +    }
> +
> +    region->has_submr = (region->submr != NULL);
> +
> +    QTAILQ_FOREACH_SAFE(ml, &submr_print_queue, queue, next_ml) {
> +        g_free(ml);
> +    }
> +
> +    return region;
> +}
> +
> +MemTree *qmp_query_mtree(Error **errp)
> +{
> +    MemoryRegionListHead ml_head;
> +    MemoryRegionList *ml, *ml2;
> +    AddressSpace *as;
> +    AddrSpaceList *head = NULL, *cur_item = NULL, *space;
> +    MemTree *mt = g_malloc0(sizeof(*mt));
> +
> +    QTAILQ_INIT(&ml_head);
> +
> +    QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) {
> +        space = g_malloc0(sizeof(*space));
> +        space->value = g_malloc0(sizeof(*space->value));
> +        space->value->name = g_strdup(as->name);
> +        space->value->mem = qmp_mtree_mr(as->root, 0, &ml_head);
> +
> +        if (!cur_item) {
> +            head = cur_item = space;
> +        } else {
> +            cur_item->next = space;
> +            cur_item = space;
> +        }
> +    }
> +
> +    mt->spaces = head;
> +    head = NULL;
> +    cur_item = NULL;
> +
> +    QTAILQ_FOREACH(ml, &ml_head, queue) {
> +        space = g_malloc0(sizeof(*space));
> +        space->value = g_malloc0(sizeof(*space->value));
> +        space->value->name = g_strdup(memory_region_name(ml->mr));
> +        space->value->mem = qmp_mtree_mr(ml->mr, 0, &ml_head);
> +
> +        if (!cur_item) {
> +            head = cur_item = space;
> +        } else {
> +            cur_item->next = space;
> +            cur_item = space;
> +        }
> +    }
> +
> +    mt->aliases = head;
> +
> +    QTAILQ_FOREACH_SAFE(ml, &ml_head, queue, ml2) {
> +        g_free(ml);
> +    }
> +
> +    return mt;
> +}
> +
>  static const TypeInfo memory_region_info = {
>      .parent             = TYPE_OBJECT,
>      .name               = TYPE_MEMORY_REGION,
> diff --git a/qapi-schema.json b/qapi-schema.json
> index 341f417..bc35bd9 100644
> --- a/qapi-schema.json
> +++ b/qapi-schema.json
> @@ -3481,3 +3481,71 @@
>  # Since: 2.1
>  ##
>  { 'command': 'rtc-reset-reinjection' }
> +
> +##
> +# @MemRegion:
> +#
> +# Information about a memory region
> +#
> +# @base: base address
> +#
> +# @size: size
> +#
> +# @prio: priority
> +#
> +# @read: read permitted
> +#
> +# @write: write permitted
> +#
> +# @ram: RAM region
> +#
> +# @name: name of the region
> +#
> +# @alias: #optional alias of this region
> +#
> +# @submr: #optional submemory regions of this region
> +#
> +# Since: 2.x
> +##
> +{ 'type': 'MemRegion',
> +  'data': {'base': 'uint64', 'size': 'uint64', 'prio': 'uint32', 'read': 'bool',
> +           'write': 'bool', 'ram': 'bool', 'name': 'str',
> +           '*alias': 'str', '*submr': ['MemRegion']} }
> +
> +##
> +# @AddrSpace:
> +#
> +# An address space of the system
> +#
> +# @name: name of the address space
> +#
> +# @mem: a list of memory regions in the address space
> +#
> +# Since: 2.x
> +##
> +{ 'type': 'AddrSpace', 'data': {'name': 'str', 'mem': 'MemRegion'} }
> +
> +##
> +# @MemTree:
> +#
> +# Memory tree of the system
> +#
> +# @spaces: Address spaces of the system
> +#
> +# @aliases: Aliased memory regions
> +#
> +# Since: 2.x
> +##
> +{ 'type': 'MemTree', 'data': {'spaces': ['AddrSpace'],
> +                                'aliases': ['AddrSpace']} }
> +
> +##
> +# @query-mtree:
> +#
> +# Return the memory distribution of the guest
> +#
> +# Returns: a list of @AddrSpace
> +#
> +# Since: 2.x
> +##
> +{ 'command': 'query-mtree', 'returns': 'MemTree' }
> diff --git a/qmp-commands.hx b/qmp-commands.hx
> index 4be4765..22f91b0 100644
> --- a/qmp-commands.hx
> +++ b/qmp-commands.hx
> @@ -3755,3 +3755,22 @@ Example:
>  <- { "return": {} }
>  
>  EQMP
> +    {
> +        .name       = "query-mtree",
> +        .args_type  = "",
> +        .mhandler.cmd_new = qmp_marshal_input_query_mtree,
> +    },
> +SQMP
> +query-mtree
> +---------
> +
> +Memory tree.
> +
> +The returned value is a json-array of the memory distribution of the system.
> +Each address space is represented by a json-object, which has a name, and a
> +json-array of all memory regions that contain. Each memory region is
> +represented by a json-object.
> +
> +(Missing schema and example)
> +
> +EQMP
> 

Re: [Qemu-devel] [RFC] qapi: New command query-mtree

[Marc Marí]

El Thu, 21 Aug 2014 11:06:51 +0200
Paolo Bonzini <pbonzini@redhat.com> escribió:
> Il 20/08/2014 19:46, Marc Marí ha scritto:
> > Add command query-mtree to get the memory tree of the guest.
> > 
> > As we were looking for a flexible solution on accessing the guest
> > memory from qtests, Stefan came with the idea to implement this new
> > qmp command.
> > 
> > This way, the result can be parsed, and the RAM direction
> > extracted, so only a generic qtest malloc is necessary and not one
> > per machine, as it is implemented at the moment (malloc-pc uses
> > fw_cfg).
> > 
> > The actual output is this: http://pastebin.com/nHAH9Jie
> > Which corresponds to this info mtree: http://pastebin.com/B5vw8DDf
> 
> I don't like this idea very much.  libqos should be using the real
> memory map information from the machine.  In the case of x86, that
> means fw_cfg; in the case of ARM, that would mean using the device
> tree. Getting the information from an out-of-band channel (such as
> QMP) is basically cheating. :)

As we were looking at how to access the device tree, we found that the
device tree is saved in memory with the bootloader or the kernel. So
tests should be using a kernel every time a ARM machine is booted
(and /dev/null, at least in virt machine, does not work). Do you have
any better idea on how to do it?

> If you had a memory map abstraction in libqos, malloc could be
> generic. Perhaps you can start doing that for PC?

Do you mean, start implementing malloc using this "query-mtree"? Or you
have another idea in mind?

Marc

Re: [Qemu-devel] [RFC] qapi: New command query-mtree

[Paolo Bonzini]

Il 21/08/2014 11:18, Marc Marí ha scritto:
> 
>> > If you had a memory map abstraction in libqos, malloc could be
>> > generic. Perhaps you can start doing that for PC?
> Do you mean, start implementing malloc using this "query-mtree"? Or you
> have another idea in mind?

I mean implementing a memory map interface, and implementing it for PC
(that says that the first MB is reserved for example, and gets the
memory size out of fw_cfg).  Then you can implement malloc on top.

Paolo

Re: [Qemu-devel] [RFC] qapi: New command query-mtree

[Paolo Bonzini]

Il 21/08/2014 11:18, Marc Marí ha scritto:
> El Thu, 21 Aug 2014 11:06:51 +0200
> Paolo Bonzini <pbonzini@redhat.com> escribió:
>> Il 20/08/2014 19:46, Marc Marí ha scritto:
>>> Add command query-mtree to get the memory tree of the guest.
>>>
>>> As we were looking for a flexible solution on accessing the guest
>>> memory from qtests, Stefan came with the idea to implement this new
>>> qmp command.
>>>
>>> This way, the result can be parsed, and the RAM direction
>>> extracted, so only a generic qtest malloc is necessary and not one
>>> per machine, as it is implemented at the moment (malloc-pc uses
>>> fw_cfg).
>>>
>>> The actual output is this: http://pastebin.com/nHAH9Jie
>>> Which corresponds to this info mtree: http://pastebin.com/B5vw8DDf
>>
>> I don't like this idea very much.  libqos should be using the real
>> memory map information from the machine.  In the case of x86, that
>> means fw_cfg; in the case of ARM, that would mean using the device
>> tree. Getting the information from an out-of-band channel (such as
>> QMP) is basically cheating. :)
> 
> As we were looking at how to access the device tree, we found that the
> device tree is saved in memory with the bootloader or the kernel. So
> tests should be using a kernel every time a ARM machine is booted
> (and /dev/null, at least in virt machine, does not work). Do you have
> any better idea on how to do it?

It works for me:

$ qemu-system-arm -M virt -S -kernel /dev/null -initrd /dev/null \
     -qtest stdio -qtest-log /dev/null
readl 0x40000014
OK 0x0000000044000000
read 0x44000084 20
OK 0x6c696e75782c64756d6d792d7669727400000000

where this string is "linux,dummy-virt".  QMP could be used to provide
the program counter (via "query-cpus"), but it currently doesn't on ARM.
 Perhaps you could make a simple patch to cpus.c for that?

Paolo