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í <address@hidden>
> ---
>  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
>