* [Qemu-devel] [PATCH qemu v2 0/2] memory/hmp: Print owners/parents in "info mtree" @ 2018-04-30 6:25 Alexey Kardashevskiy 2018-04-30 6:25 ` [Qemu-devel] [PATCH qemu v2 1/2] " Alexey Kardashevskiy 2018-04-30 6:25 ` [Qemu-devel] [PATCH qemu v2 2/2] object: Handle objects with no parents Alexey Kardashevskiy 0 siblings, 2 replies; 8+ messages in thread From: Alexey Kardashevskiy @ 2018-04-30 6:25 UTC (permalink / raw) To: qemu-devel Cc: Alexey Kardashevskiy, Dr. David Alan Gilbert, Markus Armbruster, Paolo Bonzini, Igor Mammedov This is a debug extension to "into mtree" to print a memory region owner/parent. This is based on sha1 4743c23 Peter Maydell "Update version for v2.12.0 release". Please comment. Thanks. Alexey Kardashevskiy (2): memory/hmp: Print owners/parents in "info mtree" object: Handle objects with no parents include/exec/memory.h | 2 +- memory.c | 69 +++++++++++++++++++++++++++++++++++++++++++-------- monitor.c | 4 ++- qom/object.c | 2 +- hmp-commands-info.hx | 7 +++--- 5 files changed, 68 insertions(+), 16 deletions(-) -- 2.11.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH qemu v2 1/2] memory/hmp: Print owners/parents in "info mtree" 2018-04-30 6:25 [Qemu-devel] [PATCH qemu v2 0/2] memory/hmp: Print owners/parents in "info mtree" Alexey Kardashevskiy @ 2018-04-30 6:25 ` Alexey Kardashevskiy 2018-04-30 9:53 ` Paolo Bonzini 2018-04-30 6:25 ` [Qemu-devel] [PATCH qemu v2 2/2] object: Handle objects with no parents Alexey Kardashevskiy 1 sibling, 1 reply; 8+ messages in thread From: Alexey Kardashevskiy @ 2018-04-30 6:25 UTC (permalink / raw) To: qemu-devel Cc: Alexey Kardashevskiy, Dr. David Alan Gilbert, Markus Armbruster, Paolo Bonzini, Igor Mammedov This adds owners/parents (which are the same, just occasionally owner==NULL) printing for memory regions; a new '-o' flag enabled new output. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- Changes: v2: * cleanups --- include/exec/memory.h | 2 +- memory.c | 69 +++++++++++++++++++++++++++++++++++++++++++-------- monitor.c | 4 ++- hmp-commands-info.hx | 7 +++--- 4 files changed, 67 insertions(+), 15 deletions(-) diff --git a/include/exec/memory.h b/include/exec/memory.h index 31eae0a..3e9d67c 100644 --- a/include/exec/memory.h +++ b/include/exec/memory.h @@ -1541,7 +1541,7 @@ void memory_global_dirty_log_start(void); void memory_global_dirty_log_stop(void); void mtree_info(fprintf_function mon_printf, void *f, bool flatview, - bool dispatch_tree); + bool dispatch_tree, bool owner); /** * memory_region_request_mmio_ptr: request a pointer to an mmio diff --git a/memory.c b/memory.c index e70b64b..230d905 100644 --- a/memory.c +++ b/memory.c @@ -2830,10 +2830,46 @@ typedef QTAILQ_HEAD(mrqueue, MemoryRegionList) MemoryRegionListHead; int128_sub((size), int128_one())) : 0) #define MTREE_INDENT " " +static void mtree_expand_owner(fprintf_function mon_printf, void *f, + const char *label, Object *obj) +{ + DeviceState *dev = (DeviceState *) object_dynamic_cast(obj, TYPE_DEVICE); + const char *id = object_property_print(obj, "id", true, NULL); + + mon_printf(f, " %s:{%s", label, dev ? "dev" : "obj"); + if (dev ? dev->id : id) { + mon_printf(f, " id=%s", dev ? dev->id : id); + } else { + gchar *canonical_path = object_get_canonical_path(obj); + mon_printf(f, " path=%s", canonical_path); + g_free(canonical_path); + } + mon_printf(f, "}"); +} + +static void mtree_print_mr_owner(fprintf_function mon_printf, void *f, + const MemoryRegion *mr) +{ + Object *owner = mr->owner; + Object *parent = memory_region_owner((MemoryRegion *)mr); + + if (!owner && !parent) { + mon_printf(f, " orphan"); + return; + } + if (owner) { + mtree_expand_owner(mon_printf, f, "owner", owner); + } + if (parent && parent != owner) { + mtree_expand_owner(mon_printf, f, "parent", parent); + } +} + static void mtree_print_mr(fprintf_function mon_printf, void *f, const MemoryRegion *mr, unsigned int level, hwaddr base, - MemoryRegionListHead *alias_print_queue) + MemoryRegionListHead *alias_print_queue, + bool owner) { MemoryRegionList *new_ml, *ml, *next_ml; MemoryRegionListHead submr_print_queue; @@ -2879,7 +2915,7 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f, } mon_printf(f, TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): alias %s @%s " TARGET_FMT_plx - "-" TARGET_FMT_plx "%s\n", + "-" TARGET_FMT_plx "%s", cur_start, cur_end, mr->priority, memory_region_type((MemoryRegion *)mr), @@ -2888,15 +2924,22 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f, mr->alias_offset, mr->alias_offset + MR_SIZE(mr->size), mr->enabled ? "" : " [disabled]"); + if (owner) { + mtree_print_mr_owner(mon_printf, f, mr); + } } else { mon_printf(f, - TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): %s%s\n", + TARGET_FMT_plx "-" TARGET_FMT_plx " (prio %d, %s): %s%s", cur_start, cur_end, mr->priority, memory_region_type((MemoryRegion *)mr), memory_region_name(mr), mr->enabled ? "" : " [disabled]"); + if (owner) { + mtree_print_mr_owner(mon_printf, f, mr); + } } + mon_printf(f, "\n"); QTAILQ_INIT(&submr_print_queue); @@ -2919,7 +2962,7 @@ static void mtree_print_mr(fprintf_function mon_printf, void *f, QTAILQ_FOREACH(ml, &submr_print_queue, mrqueue) { mtree_print_mr(mon_printf, f, ml->mr, level + 1, cur_start, - alias_print_queue); + alias_print_queue, owner); } QTAILQ_FOREACH_SAFE(ml, &submr_print_queue, mrqueue, next_ml) { @@ -2932,6 +2975,7 @@ struct FlatViewInfo { void *f; int counter; bool dispatch_tree; + bool owner; }; static void mtree_print_flatview(gpointer key, gpointer value, @@ -2972,7 +3016,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, mr = range->mr; if (range->offset_in_region) { p(f, MTREE_INDENT TARGET_FMT_plx "-" - TARGET_FMT_plx " (prio %d, %s): %s @" TARGET_FMT_plx "\n", + TARGET_FMT_plx " (prio %d, %s): %s @" TARGET_FMT_plx, int128_get64(range->addr.start), int128_get64(range->addr.start) + MR_SIZE(range->addr.size), mr->priority, @@ -2981,13 +3025,17 @@ static void mtree_print_flatview(gpointer key, gpointer value, range->offset_in_region); } else { p(f, MTREE_INDENT TARGET_FMT_plx "-" - TARGET_FMT_plx " (prio %d, %s): %s\n", + TARGET_FMT_plx " (prio %d, %s): %s", int128_get64(range->addr.start), int128_get64(range->addr.start) + MR_SIZE(range->addr.size), mr->priority, range->readonly ? "rom" : memory_region_type(mr), memory_region_name(mr)); } + if (fvi->owner) { + mtree_print_mr_owner(p, f, mr); + } + p(f, "\n"); range++; } @@ -3013,7 +3061,7 @@ static gboolean mtree_info_flatview_free(gpointer key, gpointer value, } void mtree_info(fprintf_function mon_printf, void *f, bool flatview, - bool dispatch_tree) + bool dispatch_tree, bool owner) { MemoryRegionListHead ml_head; MemoryRegionList *ml, *ml2; @@ -3025,7 +3073,8 @@ void mtree_info(fprintf_function mon_printf, void *f, bool flatview, .mon_printf = mon_printf, .f = f, .counter = 0, - .dispatch_tree = dispatch_tree + .dispatch_tree = dispatch_tree, + .owner = owner, }; GArray *fv_address_spaces; GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal); @@ -3057,14 +3106,14 @@ void mtree_info(fprintf_function mon_printf, void *f, bool flatview, QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) { mon_printf(f, "address-space: %s\n", as->name); - mtree_print_mr(mon_printf, f, as->root, 1, 0, &ml_head); + mtree_print_mr(mon_printf, f, as->root, 1, 0, &ml_head, owner); mon_printf(f, "\n"); } /* print aliased regions */ QTAILQ_FOREACH(ml, &ml_head, mrqueue) { mon_printf(f, "memory-region: %s\n", memory_region_name(ml->mr)); - mtree_print_mr(mon_printf, f, ml->mr, 1, 0, &ml_head); + mtree_print_mr(mon_printf, f, ml->mr, 1, 0, &ml_head, owner); mon_printf(f, "\n"); } diff --git a/monitor.c b/monitor.c index 39f8ee1..81a534d 100644 --- a/monitor.c +++ b/monitor.c @@ -1969,8 +1969,10 @@ static void hmp_info_mtree(Monitor *mon, const QDict *qdict) { bool flatview = qdict_get_try_bool(qdict, "flatview", false); bool dispatch_tree = qdict_get_try_bool(qdict, "dispatch_tree", false); + bool owner = qdict_get_try_bool(qdict, "owner", false); - mtree_info((fprintf_function)monitor_printf, mon, flatview, dispatch_tree); + mtree_info((fprintf_function)monitor_printf, mon, flatview, dispatch_tree, + owner); } static void hmp_info_numa(Monitor *mon, const QDict *qdict) diff --git a/hmp-commands-info.hx b/hmp-commands-info.hx index ddfcd5a..5956495 100644 --- a/hmp-commands-info.hx +++ b/hmp-commands-info.hx @@ -250,10 +250,11 @@ ETEXI { .name = "mtree", - .args_type = "flatview:-f,dispatch_tree:-d", - .params = "[-f][-d]", + .args_type = "flatview:-f,dispatch_tree:-d,owner:-o", + .params = "[-f][-d][-o]", .help = "show memory tree (-f: dump flat view for address spaces;" - "-d: dump dispatch tree, valid with -f only)", + "-d: dump dispatch tree, valid with -f only);" + "-o: dump region owners/parents", .cmd = hmp_info_mtree, }, -- 2.11.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2 1/2] memory/hmp: Print owners/parents in "info mtree" 2018-04-30 6:25 ` [Qemu-devel] [PATCH qemu v2 1/2] " Alexey Kardashevskiy @ 2018-04-30 9:53 ` Paolo Bonzini 2018-05-03 5:40 ` Alexey Kardashevskiy 0 siblings, 1 reply; 8+ messages in thread From: Paolo Bonzini @ 2018-04-30 9:53 UTC (permalink / raw) To: Alexey Kardashevskiy, qemu-devel Cc: Dr. David Alan Gilbert, Markus Armbruster, Igor Mammedov On 30/04/2018 08:25, Alexey Kardashevskiy wrote: > + DeviceState *dev = (DeviceState *) object_dynamic_cast(obj, TYPE_DEVICE); > + const char *id = object_property_print(obj, "id", true, NULL); The only objects that have an "id" property are memdevs. If you want to special case their printing too, it's probably a good idea (that is, print one of "dev id=ID"/"memdev id=ID"/"obj path=PATH"). Otherwise, I can also queue this patch as is, but I'd remove the "id" property handling because I'm going to submit a small series to remove the "id" property altogether. Let me know what you prefer! Thanks, Paolo > + mon_printf(f, " %s:{%s", label, dev ? "dev" : "obj"); > + if (dev ? dev->id : id) { > + mon_printf(f, " id=%s", dev ? dev->id : id); > + } else { > + gchar *canonical_path = object_get_canonical_path(obj); > + mon_printf(f, " path=%s", canonical_path); > + g_free(canonical_path); > + } > + mon_printf(f, "}"); ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2 1/2] memory/hmp: Print owners/parents in "info mtree" 2018-04-30 9:53 ` Paolo Bonzini @ 2018-05-03 5:40 ` Alexey Kardashevskiy 2018-05-30 4:57 ` Alexey Kardashevskiy 0 siblings, 1 reply; 8+ messages in thread From: Alexey Kardashevskiy @ 2018-05-03 5:40 UTC (permalink / raw) To: Paolo Bonzini, qemu-devel Cc: Dr. David Alan Gilbert, Markus Armbruster, Igor Mammedov On 30/4/18 7:53 pm, Paolo Bonzini wrote: > On 30/04/2018 08:25, Alexey Kardashevskiy wrote: >> + DeviceState *dev = (DeviceState *) object_dynamic_cast(obj, TYPE_DEVICE); >> + const char *id = object_property_print(obj, "id", true, NULL); > > The only objects that have an "id" property are memdevs. If you want to > special case their printing too, it's probably a good idea (that is, > print one of "dev id=ID"/"memdev id=ID"/"obj path=PATH"). > > Otherwise, I can also queue this patch as is, but I'd remove the "id" > property handling because I'm going to submit a small series to remove > the "id" property altogether. > > Let me know what you prefer! I choose to wait and repost, thanks :) > > Thanks, > > Paolo > >> + mon_printf(f, " %s:{%s", label, dev ? "dev" : "obj"); >> + if (dev ? dev->id : id) { >> + mon_printf(f, " id=%s", dev ? dev->id : id); >> + } else { >> + gchar *canonical_path = object_get_canonical_path(obj); >> + mon_printf(f, " path=%s", canonical_path); >> + g_free(canonical_path); >> + } >> + mon_printf(f, "}"); > -- Alexey ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2 1/2] memory/hmp: Print owners/parents in "info mtree" 2018-05-03 5:40 ` Alexey Kardashevskiy @ 2018-05-30 4:57 ` Alexey Kardashevskiy 2018-05-30 10:30 ` Paolo Bonzini 0 siblings, 1 reply; 8+ messages in thread From: Alexey Kardashevskiy @ 2018-05-30 4:57 UTC (permalink / raw) To: Paolo Bonzini, qemu-devel Cc: Dr. David Alan Gilbert, Markus Armbruster, Igor Mammedov On 3/5/18 3:40 pm, Alexey Kardashevskiy wrote: > On 30/4/18 7:53 pm, Paolo Bonzini wrote: >> On 30/04/2018 08:25, Alexey Kardashevskiy wrote: >>> + DeviceState *dev = (DeviceState *) object_dynamic_cast(obj, TYPE_DEVICE); >>> + const char *id = object_property_print(obj, "id", true, NULL); >> >> The only objects that have an "id" property are memdevs. If you want to >> special case their printing too, it's probably a good idea (that is, >> print one of "dev id=ID"/"memdev id=ID"/"obj path=PATH"). >> >> Otherwise, I can also queue this patch as is, but I'd remove the "id" >> property handling because I'm going to submit a small series to remove >> the "id" property altogether. >> >> Let me know what you prefer! > > > I choose to wait and repost, thanks :) I waited, checked https://git.qemu.org/?p=qemu.git;a=commitdiff;h=29de4e that "id" is no more and then grepped: hw/intc/apic_common.c|489| object_property_add(obj, "id", "uint32", hw/ppc/spapr_drc.c|557| object_property_add_uint32_ptr(obj, "id", &drc->id, NULL); This does not look like "remove the "id" property altogether" :) Does this mean we still rather want to print QOM's "id"? spapr_drc does not own MRs and APIC seems not to either. > > >> >> Thanks, >> >> Paolo >> >>> + mon_printf(f, " %s:{%s", label, dev ? "dev" : "obj"); >>> + if (dev ? dev->id : id) { >>> + mon_printf(f, " id=%s", dev ? dev->id : id); >>> + } else { >>> + gchar *canonical_path = object_get_canonical_path(obj); >>> + mon_printf(f, " path=%s", canonical_path); >>> + g_free(canonical_path); >>> + } >>> + mon_printf(f, "}"); >> > > -- Alexey ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2 1/2] memory/hmp: Print owners/parents in "info mtree" 2018-05-30 4:57 ` Alexey Kardashevskiy @ 2018-05-30 10:30 ` Paolo Bonzini 0 siblings, 0 replies; 8+ messages in thread From: Paolo Bonzini @ 2018-05-30 10:30 UTC (permalink / raw) To: Alexey Kardashevskiy, qemu-devel Cc: Dr. David Alan Gilbert, Markus Armbruster, Igor Mammedov On 30/05/2018 06:57, Alexey Kardashevskiy wrote: > hw/intc/apic_common.c|489| object_property_add(obj, "id", "uint32", > hw/ppc/spapr_drc.c|557| object_property_add_uint32_ptr(obj, "id", &drc->id, > NULL); > > This does not look like "remove the "id" property altogether" :) Does this > mean we still rather want to print QOM's "id"? spapr_drc does not own MRs > and APIC seems not to either. No, those properties are specific to some devices (and in fact they are integers rather than strings). The id property that mirrors the path component is gone. Paolo ^ permalink raw reply [flat|nested] 8+ messages in thread
* [Qemu-devel] [PATCH qemu v2 2/2] object: Handle objects with no parents 2018-04-30 6:25 [Qemu-devel] [PATCH qemu v2 0/2] memory/hmp: Print owners/parents in "info mtree" Alexey Kardashevskiy 2018-04-30 6:25 ` [Qemu-devel] [PATCH qemu v2 1/2] " Alexey Kardashevskiy @ 2018-04-30 6:25 ` Alexey Kardashevskiy 2018-04-30 9:51 ` Paolo Bonzini 1 sibling, 1 reply; 8+ messages in thread From: Alexey Kardashevskiy @ 2018-04-30 6:25 UTC (permalink / raw) To: qemu-devel Cc: Alexey Kardashevskiy, Dr. David Alan Gilbert, Markus Armbruster, Paolo Bonzini, Igor Mammedov At the moment object_get_canonical_path_component() crashes on assert() if the object does not have a parent. Usually it is not called for orphan objects but various HMP/QMP commands can do that (info mtree, qom-get). This adds few more tests in object_get_canonical_path() to prevent QEMU from crashing. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- qom/object.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/qom/object.c b/qom/object.c index 4677951..e0e300b 100644 --- a/qom/object.c +++ b/qom/object.c @@ -1668,7 +1668,7 @@ gchar *object_get_canonical_path(Object *obj) Object *root = object_get_root(); char *newpath, *path = NULL; - while (obj != root) { + while (obj && obj->parent && obj != root) { char *component = object_get_canonical_path_component(obj); if (path) { -- 2.11.0 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2 2/2] object: Handle objects with no parents 2018-04-30 6:25 ` [Qemu-devel] [PATCH qemu v2 2/2] object: Handle objects with no parents Alexey Kardashevskiy @ 2018-04-30 9:51 ` Paolo Bonzini 0 siblings, 0 replies; 8+ messages in thread From: Paolo Bonzini @ 2018-04-30 9:51 UTC (permalink / raw) To: Alexey Kardashevskiy, qemu-devel Cc: Dr. David Alan Gilbert, Markus Armbruster, Igor Mammedov On 30/04/2018 08:25, Alexey Kardashevskiy wrote: > At the moment object_get_canonical_path_component() crashes on assert() > if the object does not have a parent. Usually it is not called for > orphan objects but various HMP/QMP commands can do that (info mtree, > qom-get). > > This adds few more tests in object_get_canonical_path() to prevent QEMU > from crashing. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > qom/object.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/qom/object.c b/qom/object.c > index 4677951..e0e300b 100644 > --- a/qom/object.c > +++ b/qom/object.c > @@ -1668,7 +1668,7 @@ gchar *object_get_canonical_path(Object *obj) > Object *root = object_get_root(); > char *newpath, *path = NULL; > > - while (obj != root) { > + while (obj && obj->parent && obj != root) { > char *component = object_get_canonical_path_component(obj); > > if (path) { I think the patch is a good idea, but as it is written it is incorrect, because it will return an invalid canonical path. You should return NULL instead. Also, checking both obj and obj->parent is unnecessary; if obj->parent is NULL, obj will be NULL on the next iteration. Thanks, Paolo ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-05-30 10:30 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-04-30 6:25 [Qemu-devel] [PATCH qemu v2 0/2] memory/hmp: Print owners/parents in "info mtree" Alexey Kardashevskiy 2018-04-30 6:25 ` [Qemu-devel] [PATCH qemu v2 1/2] " Alexey Kardashevskiy 2018-04-30 9:53 ` Paolo Bonzini 2018-05-03 5:40 ` Alexey Kardashevskiy 2018-05-30 4:57 ` Alexey Kardashevskiy 2018-05-30 10:30 ` Paolo Bonzini 2018-04-30 6:25 ` [Qemu-devel] [PATCH qemu v2 2/2] object: Handle objects with no parents Alexey Kardashevskiy 2018-04-30 9:51 ` Paolo Bonzini
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.