* [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator @ 2018-12-14 2:58 Alexey Kardashevskiy 2018-12-14 11:07 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 19+ messages in thread From: Alexey Kardashevskiy @ 2018-12-14 2:58 UTC (permalink / raw) To: qemu-devel Cc: Alexey Kardashevskiy, Paolo Bonzini, Daniel Henrique Barboza, Dr . David Alan Gilbert This adds an accelerator name to the "into mtree -f" to tell the user if a particular memory section is registered with the accelerator; the primary user for this is KVM and such information is useful for debugging purposes. This adds a has_memory() callback to the accelerator class allowing any accelerator to have a label in that memory tree dump. Since memory sections are passed to memory listeners and get registered in accelerators (rather than memory regions), this only prints new labels for flatviews attached to the system address space. An example: Root memory region: system 0000000000000000-0000002fffffffff (prio 0, ram): /objects/mem0 kvm 0000003000000000-0000005fffffffff (prio 0, ram): /objects/mem1 kvm 0000200000000020-000020000000003f (prio 1, i/o): virtio-pci 0000200080000000-000020008000003f (prio 0, i/o): capabilities Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- This supercedes "[PATCH qemu] hmp: Print if memory section is registered in KVM" --- Changes: v2: * added an accelerator callback instead of hardcoding it to kvm only --- include/sysemu/accel.h | 2 ++ accel/kvm/kvm-all.c | 10 ++++++++++ memory.c | 22 ++++++++++++++++++++++ 3 files changed, 34 insertions(+) diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h index 637358f..30b456d 100644 --- a/include/sysemu/accel.h +++ b/include/sysemu/accel.h @@ -25,6 +25,7 @@ #include "qom/object.h" #include "hw/qdev-properties.h" +#include "exec/hwaddr.h" typedef struct AccelState { /*< private >*/ @@ -41,6 +42,7 @@ typedef struct AccelClass { int (*available)(void); int (*init_machine)(MachineState *ms); void (*setup_post)(MachineState *ms, AccelState *accel); + bool (*has_memory)(MachineState *ms, hwaddr start_addr, hwaddr size); bool *allowed; /* * Array of global properties that would be applied when specific diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index 4880a05..634f386 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -2589,11 +2589,21 @@ int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target) return r; } +static bool kvm_accel_has_memory(MachineState *ms, hwaddr start_addr, + hwaddr size) +{ + KVMState *kvm = KVM_STATE(ms->accelerator); + KVMMemoryListener *kml = &kvm->memory_listener; + + return NULL != kvm_lookup_matching_slot(kml, start_addr, size); +} + static void kvm_accel_class_init(ObjectClass *oc, void *data) { AccelClass *ac = ACCEL_CLASS(oc); ac->name = "KVM"; ac->init_machine = kvm_init; + ac->has_memory = kvm_accel_has_memory; ac->allowed = &kvm_allowed; } diff --git a/memory.c b/memory.c index d14c6de..61e758a 100644 --- a/memory.c +++ b/memory.c @@ -29,7 +29,9 @@ #include "exec/ram_addr.h" #include "sysemu/kvm.h" #include "sysemu/sysemu.h" +#include "sysemu/accel.h" #include "hw/qdev-properties.h" +#include "hw/boards.h" #include "migration/vmstate.h" //#define DEBUG_UNASSIGNED @@ -2924,6 +2926,8 @@ struct FlatViewInfo { int counter; bool dispatch_tree; bool owner; + AccelClass *ac; + const char *ac_name; }; static void mtree_print_flatview(gpointer key, gpointer value, @@ -2939,6 +2943,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, int n = view->nr; int i; AddressSpace *as; + bool system_as = false; p(f, "FlatView #%d\n", fvi->counter); ++fvi->counter; @@ -2950,6 +2955,9 @@ static void mtree_print_flatview(gpointer key, gpointer value, p(f, ", alias %s", memory_region_name(as->root->alias)); } p(f, "\n"); + if (as == &address_space_memory) { + system_as = true; + } } p(f, " Root memory region: %s\n", @@ -2985,6 +2993,13 @@ static void mtree_print_flatview(gpointer key, gpointer value, if (fvi->owner) { mtree_print_mr_owner(p, f, mr); } + + if (system_as && fvi->ac && + fvi->ac->has_memory(current_machine, + int128_get64(range->addr.start), + MR_SIZE(range->addr.size) + 1)) { + p(f, " %s", fvi->ac_name); + } p(f, "\n"); range++; } @@ -3028,6 +3043,13 @@ void mtree_info(fprintf_function mon_printf, void *f, bool flatview, }; GArray *fv_address_spaces; GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal); + AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator); + + if (ac->has_memory) { + fvi.ac = ac; + fvi.ac_name = current_machine->accel ? current_machine->accel : + object_class_get_name(OBJECT_CLASS(ac)); + } /* Gather all FVs in one table */ QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) { -- 2.17.1 ^ permalink raw reply related [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator 2018-12-14 2:58 [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator Alexey Kardashevskiy @ 2018-12-14 11:07 ` Philippe Mathieu-Daudé 2018-12-17 1:27 ` Alexey Kardashevskiy 0 siblings, 1 reply; 19+ messages in thread From: Philippe Mathieu-Daudé @ 2018-12-14 11:07 UTC (permalink / raw) To: Alexey Kardashevskiy, qemu-devel Cc: Paolo Bonzini, Daniel Henrique Barboza, Dr . David Alan Gilbert Hi Alexey, On 12/14/18 3:58 AM, Alexey Kardashevskiy wrote: > This adds an accelerator name to the "into mtree -f" to tell the user if > a particular memory section is registered with the accelerator; > the primary user for this is KVM and such information is useful > for debugging purposes. > > This adds a has_memory() callback to the accelerator class allowing any > accelerator to have a label in that memory tree dump. > > Since memory sections are passed to memory listeners and get registered > in accelerators (rather than memory regions), this only prints new labels > for flatviews attached to the system address space. > > An example: > Root memory region: system > 0000000000000000-0000002fffffffff (prio 0, ram): /objects/mem0 kvm > 0000003000000000-0000005fffffffff (prio 0, ram): /objects/mem1 kvm > 0000200000000020-000020000000003f (prio 1, i/o): virtio-pci > 0000200080000000-000020008000003f (prio 0, i/o): capabilities > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > > This supercedes "[PATCH qemu] hmp: Print if memory section is registered in KVM" > > --- > Changes: > v2: > * added an accelerator callback instead of hardcoding it to kvm only > --- > include/sysemu/accel.h | 2 ++ > accel/kvm/kvm-all.c | 10 ++++++++++ > memory.c | 22 ++++++++++++++++++++++ > 3 files changed, 34 insertions(+) > > diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h > index 637358f..30b456d 100644 > --- a/include/sysemu/accel.h > +++ b/include/sysemu/accel.h > @@ -25,6 +25,7 @@ > > #include "qom/object.h" > #include "hw/qdev-properties.h" > +#include "exec/hwaddr.h" > > typedef struct AccelState { > /*< private >*/ > @@ -41,6 +42,7 @@ typedef struct AccelClass { > int (*available)(void); > int (*init_machine)(MachineState *ms); > void (*setup_post)(MachineState *ms, AccelState *accel); > + bool (*has_memory)(MachineState *ms, hwaddr start_addr, hwaddr size); > bool *allowed; > /* > * Array of global properties that would be applied when specific > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > index 4880a05..634f386 100644 > --- a/accel/kvm/kvm-all.c > +++ b/accel/kvm/kvm-all.c > @@ -2589,11 +2589,21 @@ int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target) > return r; > } > > +static bool kvm_accel_has_memory(MachineState *ms, hwaddr start_addr, > + hwaddr size) > +{ > + KVMState *kvm = KVM_STATE(ms->accelerator); > + KVMMemoryListener *kml = &kvm->memory_listener; > + > + return NULL != kvm_lookup_matching_slot(kml, start_addr, size); > +} > + > static void kvm_accel_class_init(ObjectClass *oc, void *data) > { > AccelClass *ac = ACCEL_CLASS(oc); > ac->name = "KVM"; > ac->init_machine = kvm_init; > + ac->has_memory = kvm_accel_has_memory; > ac->allowed = &kvm_allowed; > } > > diff --git a/memory.c b/memory.c > index d14c6de..61e758a 100644 > --- a/memory.c > +++ b/memory.c > @@ -29,7 +29,9 @@ > #include "exec/ram_addr.h" > #include "sysemu/kvm.h" > #include "sysemu/sysemu.h" > +#include "sysemu/accel.h" > #include "hw/qdev-properties.h" > +#include "hw/boards.h" > #include "migration/vmstate.h" > > //#define DEBUG_UNASSIGNED > @@ -2924,6 +2926,8 @@ struct FlatViewInfo { > int counter; > bool dispatch_tree; > bool owner; > + AccelClass *ac; > + const char *ac_name; > }; > > static void mtree_print_flatview(gpointer key, gpointer value, > @@ -2939,6 +2943,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, > int n = view->nr; > int i; > AddressSpace *as; > + bool system_as = false; > > p(f, "FlatView #%d\n", fvi->counter); > ++fvi->counter; > @@ -2950,6 +2955,9 @@ static void mtree_print_flatview(gpointer key, gpointer value, > p(f, ", alias %s", memory_region_name(as->root->alias)); > } > p(f, "\n"); > + if (as == &address_space_memory) { > + system_as = true; > + } > } > > p(f, " Root memory region: %s\n", > @@ -2985,6 +2993,13 @@ static void mtree_print_flatview(gpointer key, gpointer value, > if (fvi->owner) { > mtree_print_mr_owner(p, f, mr); > } > + > + if (system_as && fvi->ac && > + fvi->ac->has_memory(current_machine, > + int128_get64(range->addr.start), > + MR_SIZE(range->addr.size) + 1)) { > + p(f, " %s", fvi->ac_name); Why not simply display fvi->ac->name? You could avoid to add the ac_name field. > + } > p(f, "\n"); > range++; > } > @@ -3028,6 +3043,13 @@ void mtree_info(fprintf_function mon_printf, void *f, bool flatview, > }; > GArray *fv_address_spaces; > GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal); > + AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator); > + > + if (ac->has_memory) { > + fvi.ac = ac; > + fvi.ac_name = current_machine->accel ? current_machine->accel : > + object_class_get_name(OBJECT_CLASS(ac)); > + } > > /* Gather all FVs in one table */ > QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) { > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator 2018-12-14 11:07 ` Philippe Mathieu-Daudé @ 2018-12-17 1:27 ` Alexey Kardashevskiy 2018-12-17 12:47 ` Philippe Mathieu-Daudé 0 siblings, 1 reply; 19+ messages in thread From: Alexey Kardashevskiy @ 2018-12-17 1:27 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel Cc: Paolo Bonzini, Daniel Henrique Barboza, Dr . David Alan Gilbert On 14/12/2018 22:07, Philippe Mathieu-Daudé wrote: > Hi Alexey, > > On 12/14/18 3:58 AM, Alexey Kardashevskiy wrote: >> This adds an accelerator name to the "into mtree -f" to tell the user if >> a particular memory section is registered with the accelerator; >> the primary user for this is KVM and such information is useful >> for debugging purposes. >> >> This adds a has_memory() callback to the accelerator class allowing any >> accelerator to have a label in that memory tree dump. >> >> Since memory sections are passed to memory listeners and get registered >> in accelerators (rather than memory regions), this only prints new labels >> for flatviews attached to the system address space. >> >> An example: >> Root memory region: system >> 0000000000000000-0000002fffffffff (prio 0, ram): /objects/mem0 kvm >> 0000003000000000-0000005fffffffff (prio 0, ram): /objects/mem1 kvm >> 0000200000000020-000020000000003f (prio 1, i/o): virtio-pci >> 0000200080000000-000020008000003f (prio 0, i/o): capabilities >> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> --- >> >> This supercedes "[PATCH qemu] hmp: Print if memory section is registered in KVM" >> >> --- >> Changes: >> v2: >> * added an accelerator callback instead of hardcoding it to kvm only >> --- >> include/sysemu/accel.h | 2 ++ >> accel/kvm/kvm-all.c | 10 ++++++++++ >> memory.c | 22 ++++++++++++++++++++++ >> 3 files changed, 34 insertions(+) >> >> diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h >> index 637358f..30b456d 100644 >> --- a/include/sysemu/accel.h >> +++ b/include/sysemu/accel.h >> @@ -25,6 +25,7 @@ >> >> #include "qom/object.h" >> #include "hw/qdev-properties.h" >> +#include "exec/hwaddr.h" >> >> typedef struct AccelState { >> /*< private >*/ >> @@ -41,6 +42,7 @@ typedef struct AccelClass { >> int (*available)(void); >> int (*init_machine)(MachineState *ms); >> void (*setup_post)(MachineState *ms, AccelState *accel); >> + bool (*has_memory)(MachineState *ms, hwaddr start_addr, hwaddr size); >> bool *allowed; >> /* >> * Array of global properties that would be applied when specific >> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c >> index 4880a05..634f386 100644 >> --- a/accel/kvm/kvm-all.c >> +++ b/accel/kvm/kvm-all.c >> @@ -2589,11 +2589,21 @@ int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target) >> return r; >> } >> >> +static bool kvm_accel_has_memory(MachineState *ms, hwaddr start_addr, >> + hwaddr size) >> +{ >> + KVMState *kvm = KVM_STATE(ms->accelerator); >> + KVMMemoryListener *kml = &kvm->memory_listener; >> + >> + return NULL != kvm_lookup_matching_slot(kml, start_addr, size); >> +} >> + >> static void kvm_accel_class_init(ObjectClass *oc, void *data) >> { >> AccelClass *ac = ACCEL_CLASS(oc); >> ac->name = "KVM"; >> ac->init_machine = kvm_init; >> + ac->has_memory = kvm_accel_has_memory; >> ac->allowed = &kvm_allowed; >> } >> >> diff --git a/memory.c b/memory.c >> index d14c6de..61e758a 100644 >> --- a/memory.c >> +++ b/memory.c >> @@ -29,7 +29,9 @@ >> #include "exec/ram_addr.h" >> #include "sysemu/kvm.h" >> #include "sysemu/sysemu.h" >> +#include "sysemu/accel.h" >> #include "hw/qdev-properties.h" >> +#include "hw/boards.h" >> #include "migration/vmstate.h" >> >> //#define DEBUG_UNASSIGNED >> @@ -2924,6 +2926,8 @@ struct FlatViewInfo { >> int counter; >> bool dispatch_tree; >> bool owner; >> + AccelClass *ac; >> + const char *ac_name; >> }; >> >> static void mtree_print_flatview(gpointer key, gpointer value, >> @@ -2939,6 +2943,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, >> int n = view->nr; >> int i; >> AddressSpace *as; >> + bool system_as = false; >> >> p(f, "FlatView #%d\n", fvi->counter); >> ++fvi->counter; >> @@ -2950,6 +2955,9 @@ static void mtree_print_flatview(gpointer key, gpointer value, >> p(f, ", alias %s", memory_region_name(as->root->alias)); >> } >> p(f, "\n"); >> + if (as == &address_space_memory) { >> + system_as = true; >> + } >> } >> >> p(f, " Root memory region: %s\n", >> @@ -2985,6 +2993,13 @@ static void mtree_print_flatview(gpointer key, gpointer value, >> if (fvi->owner) { >> mtree_print_mr_owner(p, f, mr); >> } >> + >> + if (system_as && fvi->ac && >> + fvi->ac->has_memory(current_machine, >> + int128_get64(range->addr.start), >> + MR_SIZE(range->addr.size) + 1)) { >> + p(f, " %s", fvi->ac_name); > > Why not simply display fvi->ac->name? > You could avoid to add the ac_name field. Well, I thought I better print whatever the user passed via the command line (which is current_machine->accel and equals to "kvm" in my case) rather than robotic, dry and excessive "kvm-accel". > >> + } >> p(f, "\n"); >> range++; >> } >> @@ -3028,6 +3043,13 @@ void mtree_info(fprintf_function mon_printf, void *f, bool flatview, >> }; >> GArray *fv_address_spaces; >> GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal); >> + AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator); >> + >> + if (ac->has_memory) { >> + fvi.ac = ac; >> + fvi.ac_name = current_machine->accel ? current_machine->accel : >> + object_class_get_name(OBJECT_CLASS(ac)); >> + } >> >> /* Gather all FVs in one table */ >> QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) { >> -- Alexey ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator 2018-12-17 1:27 ` Alexey Kardashevskiy @ 2018-12-17 12:47 ` Philippe Mathieu-Daudé 2018-12-18 0:58 ` Alexey Kardashevskiy 0 siblings, 1 reply; 19+ messages in thread From: Philippe Mathieu-Daudé @ 2018-12-17 12:47 UTC (permalink / raw) To: Alexey Kardashevskiy, qemu-devel Cc: Paolo Bonzini, Daniel Henrique Barboza, Dr . David Alan Gilbert On 12/17/18 2:27 AM, Alexey Kardashevskiy wrote: > On 14/12/2018 22:07, Philippe Mathieu-Daudé wrote: >> Hi Alexey, >> >> On 12/14/18 3:58 AM, Alexey Kardashevskiy wrote: >>> This adds an accelerator name to the "into mtree -f" to tell the user if >>> a particular memory section is registered with the accelerator; >>> the primary user for this is KVM and such information is useful >>> for debugging purposes. >>> >>> This adds a has_memory() callback to the accelerator class allowing any >>> accelerator to have a label in that memory tree dump. >>> >>> Since memory sections are passed to memory listeners and get registered >>> in accelerators (rather than memory regions), this only prints new labels >>> for flatviews attached to the system address space. >>> >>> An example: >>> Root memory region: system >>> 0000000000000000-0000002fffffffff (prio 0, ram): /objects/mem0 kvm >>> 0000003000000000-0000005fffffffff (prio 0, ram): /objects/mem1 kvm >>> 0000200000000020-000020000000003f (prio 1, i/o): virtio-pci >>> 0000200080000000-000020008000003f (prio 0, i/o): capabilities >>> >>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >>> --- >>> >>> This supercedes "[PATCH qemu] hmp: Print if memory section is registered in KVM" >>> >>> --- >>> Changes: >>> v2: >>> * added an accelerator callback instead of hardcoding it to kvm only >>> --- >>> include/sysemu/accel.h | 2 ++ >>> accel/kvm/kvm-all.c | 10 ++++++++++ >>> memory.c | 22 ++++++++++++++++++++++ >>> 3 files changed, 34 insertions(+) >>> >>> diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h >>> index 637358f..30b456d 100644 >>> --- a/include/sysemu/accel.h >>> +++ b/include/sysemu/accel.h >>> @@ -25,6 +25,7 @@ >>> >>> #include "qom/object.h" >>> #include "hw/qdev-properties.h" >>> +#include "exec/hwaddr.h" >>> >>> typedef struct AccelState { >>> /*< private >*/ >>> @@ -41,6 +42,7 @@ typedef struct AccelClass { >>> int (*available)(void); >>> int (*init_machine)(MachineState *ms); >>> void (*setup_post)(MachineState *ms, AccelState *accel); >>> + bool (*has_memory)(MachineState *ms, hwaddr start_addr, hwaddr size); >>> bool *allowed; >>> /* >>> * Array of global properties that would be applied when specific >>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c >>> index 4880a05..634f386 100644 >>> --- a/accel/kvm/kvm-all.c >>> +++ b/accel/kvm/kvm-all.c >>> @@ -2589,11 +2589,21 @@ int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target) >>> return r; >>> } >>> >>> +static bool kvm_accel_has_memory(MachineState *ms, hwaddr start_addr, >>> + hwaddr size) >>> +{ >>> + KVMState *kvm = KVM_STATE(ms->accelerator); >>> + KVMMemoryListener *kml = &kvm->memory_listener; >>> + >>> + return NULL != kvm_lookup_matching_slot(kml, start_addr, size); >>> +} >>> + >>> static void kvm_accel_class_init(ObjectClass *oc, void *data) >>> { >>> AccelClass *ac = ACCEL_CLASS(oc); >>> ac->name = "KVM"; >>> ac->init_machine = kvm_init; >>> + ac->has_memory = kvm_accel_has_memory; >>> ac->allowed = &kvm_allowed; >>> } >>> >>> diff --git a/memory.c b/memory.c >>> index d14c6de..61e758a 100644 >>> --- a/memory.c >>> +++ b/memory.c >>> @@ -29,7 +29,9 @@ >>> #include "exec/ram_addr.h" >>> #include "sysemu/kvm.h" >>> #include "sysemu/sysemu.h" >>> +#include "sysemu/accel.h" >>> #include "hw/qdev-properties.h" >>> +#include "hw/boards.h" >>> #include "migration/vmstate.h" >>> >>> //#define DEBUG_UNASSIGNED >>> @@ -2924,6 +2926,8 @@ struct FlatViewInfo { >>> int counter; >>> bool dispatch_tree; >>> bool owner; >>> + AccelClass *ac; >>> + const char *ac_name; >>> }; >>> >>> static void mtree_print_flatview(gpointer key, gpointer value, >>> @@ -2939,6 +2943,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>> int n = view->nr; >>> int i; >>> AddressSpace *as; >>> + bool system_as = false; >>> >>> p(f, "FlatView #%d\n", fvi->counter); >>> ++fvi->counter; >>> @@ -2950,6 +2955,9 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>> p(f, ", alias %s", memory_region_name(as->root->alias)); >>> } >>> p(f, "\n"); >>> + if (as == &address_space_memory) { >>> + system_as = true; >>> + } >>> } >>> >>> p(f, " Root memory region: %s\n", >>> @@ -2985,6 +2993,13 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>> if (fvi->owner) { >>> mtree_print_mr_owner(p, f, mr); >>> } >>> + >>> + if (system_as && fvi->ac && >>> + fvi->ac->has_memory(current_machine, >>> + int128_get64(range->addr.start), >>> + MR_SIZE(range->addr.size) + 1)) { >>> + p(f, " %s", fvi->ac_name); >> >> Why not simply display fvi->ac->name? >> You could avoid to add the ac_name field. > > > Well, I thought I better print whatever the user passed via the command > line (which is current_machine->accel and equals to "kvm" in my case) > rather than robotic, dry and excessive "kvm-accel". I have no hit for 'kvm-accel': $ git grep kvm-accel $ Names looks human friendly: $ git grep 'ac->name =' accel/kvm/kvm-all.c:2595: ac->name = "KVM"; accel/tcg/tcg-all.c:74: ac->name = "tcg"; hw/xen/xen-common.c:184: ac->name = "Xen"; target/i386/hax-all.c:1088: ac->name = "HAX"; target/i386/hvf/hvf.c:959: ac->name = "HVF"; target/i386/whpx-all.c:1477: ac->name = "WHPX"; qtest.c:755: ac->name = "QTest"; >> >>> + } >>> p(f, "\n"); >>> range++; >>> } >>> @@ -3028,6 +3043,13 @@ void mtree_info(fprintf_function mon_printf, void *f, bool flatview, >>> }; >>> GArray *fv_address_spaces; >>> GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal); >>> + AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator); >>> + >>> + if (ac->has_memory) { >>> + fvi.ac = ac; >>> + fvi.ac_name = current_machine->accel ? current_machine->accel : >>> + object_class_get_name(OBJECT_CLASS(ac)); >>> + } >>> >>> /* Gather all FVs in one table */ >>> QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) { >>> ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator 2018-12-17 12:47 ` Philippe Mathieu-Daudé @ 2018-12-18 0:58 ` Alexey Kardashevskiy 2019-01-03 17:37 ` Dr. David Alan Gilbert 0 siblings, 1 reply; 19+ messages in thread From: Alexey Kardashevskiy @ 2018-12-18 0:58 UTC (permalink / raw) To: Philippe Mathieu-Daudé, qemu-devel Cc: Paolo Bonzini, Daniel Henrique Barboza, Dr . David Alan Gilbert On 17/12/2018 23:47, Philippe Mathieu-Daudé wrote: > On 12/17/18 2:27 AM, Alexey Kardashevskiy wrote: >> On 14/12/2018 22:07, Philippe Mathieu-Daudé wrote: >>> Hi Alexey, >>> >>> On 12/14/18 3:58 AM, Alexey Kardashevskiy wrote: >>>> This adds an accelerator name to the "into mtree -f" to tell the user if >>>> a particular memory section is registered with the accelerator; >>>> the primary user for this is KVM and such information is useful >>>> for debugging purposes. >>>> >>>> This adds a has_memory() callback to the accelerator class allowing any >>>> accelerator to have a label in that memory tree dump. >>>> >>>> Since memory sections are passed to memory listeners and get registered >>>> in accelerators (rather than memory regions), this only prints new labels >>>> for flatviews attached to the system address space. >>>> >>>> An example: >>>> Root memory region: system >>>> 0000000000000000-0000002fffffffff (prio 0, ram): /objects/mem0 kvm >>>> 0000003000000000-0000005fffffffff (prio 0, ram): /objects/mem1 kvm >>>> 0000200000000020-000020000000003f (prio 1, i/o): virtio-pci >>>> 0000200080000000-000020008000003f (prio 0, i/o): capabilities >>>> >>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >>>> --- >>>> >>>> This supercedes "[PATCH qemu] hmp: Print if memory section is registered in KVM" >>>> >>>> --- >>>> Changes: >>>> v2: >>>> * added an accelerator callback instead of hardcoding it to kvm only >>>> --- >>>> include/sysemu/accel.h | 2 ++ >>>> accel/kvm/kvm-all.c | 10 ++++++++++ >>>> memory.c | 22 ++++++++++++++++++++++ >>>> 3 files changed, 34 insertions(+) >>>> >>>> diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h >>>> index 637358f..30b456d 100644 >>>> --- a/include/sysemu/accel.h >>>> +++ b/include/sysemu/accel.h >>>> @@ -25,6 +25,7 @@ >>>> >>>> #include "qom/object.h" >>>> #include "hw/qdev-properties.h" >>>> +#include "exec/hwaddr.h" >>>> >>>> typedef struct AccelState { >>>> /*< private >*/ >>>> @@ -41,6 +42,7 @@ typedef struct AccelClass { >>>> int (*available)(void); >>>> int (*init_machine)(MachineState *ms); >>>> void (*setup_post)(MachineState *ms, AccelState *accel); >>>> + bool (*has_memory)(MachineState *ms, hwaddr start_addr, hwaddr size); >>>> bool *allowed; >>>> /* >>>> * Array of global properties that would be applied when specific >>>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c >>>> index 4880a05..634f386 100644 >>>> --- a/accel/kvm/kvm-all.c >>>> +++ b/accel/kvm/kvm-all.c >>>> @@ -2589,11 +2589,21 @@ int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target) >>>> return r; >>>> } >>>> >>>> +static bool kvm_accel_has_memory(MachineState *ms, hwaddr start_addr, >>>> + hwaddr size) >>>> +{ >>>> + KVMState *kvm = KVM_STATE(ms->accelerator); >>>> + KVMMemoryListener *kml = &kvm->memory_listener; >>>> + >>>> + return NULL != kvm_lookup_matching_slot(kml, start_addr, size); >>>> +} >>>> + >>>> static void kvm_accel_class_init(ObjectClass *oc, void *data) >>>> { >>>> AccelClass *ac = ACCEL_CLASS(oc); >>>> ac->name = "KVM"; >>>> ac->init_machine = kvm_init; >>>> + ac->has_memory = kvm_accel_has_memory; >>>> ac->allowed = &kvm_allowed; >>>> } >>>> >>>> diff --git a/memory.c b/memory.c >>>> index d14c6de..61e758a 100644 >>>> --- a/memory.c >>>> +++ b/memory.c >>>> @@ -29,7 +29,9 @@ >>>> #include "exec/ram_addr.h" >>>> #include "sysemu/kvm.h" >>>> #include "sysemu/sysemu.h" >>>> +#include "sysemu/accel.h" >>>> #include "hw/qdev-properties.h" >>>> +#include "hw/boards.h" >>>> #include "migration/vmstate.h" >>>> >>>> //#define DEBUG_UNASSIGNED >>>> @@ -2924,6 +2926,8 @@ struct FlatViewInfo { >>>> int counter; >>>> bool dispatch_tree; >>>> bool owner; >>>> + AccelClass *ac; >>>> + const char *ac_name; >>>> }; >>>> >>>> static void mtree_print_flatview(gpointer key, gpointer value, >>>> @@ -2939,6 +2943,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>> int n = view->nr; >>>> int i; >>>> AddressSpace *as; >>>> + bool system_as = false; >>>> >>>> p(f, "FlatView #%d\n", fvi->counter); >>>> ++fvi->counter; >>>> @@ -2950,6 +2955,9 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>> p(f, ", alias %s", memory_region_name(as->root->alias)); >>>> } >>>> p(f, "\n"); >>>> + if (as == &address_space_memory) { >>>> + system_as = true; >>>> + } >>>> } >>>> >>>> p(f, " Root memory region: %s\n", >>>> @@ -2985,6 +2993,13 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>> if (fvi->owner) { >>>> mtree_print_mr_owner(p, f, mr); >>>> } >>>> + >>>> + if (system_as && fvi->ac && >>>> + fvi->ac->has_memory(current_machine, >>>> + int128_get64(range->addr.start), >>>> + MR_SIZE(range->addr.size) + 1)) { >>>> + p(f, " %s", fvi->ac_name); >>> >>> Why not simply display fvi->ac->name? >>> You could avoid to add the ac_name field. >> >> >> Well, I thought I better print whatever the user passed via the command >> line (which is current_machine->accel and equals to "kvm" in my case) >> rather than robotic, dry and excessive "kvm-accel". > > I have no hit for 'kvm-accel': > > $ git grep kvm-accel > $ > > Names looks human friendly: Ah, I confused with object_class_get_name(). Anyway, I'd still show the user provided accelerator name than some internal name. > > $ git grep 'ac->name =' > accel/kvm/kvm-all.c:2595: ac->name = "KVM"; > accel/tcg/tcg-all.c:74: ac->name = "tcg"; > hw/xen/xen-common.c:184: ac->name = "Xen"; > target/i386/hax-all.c:1088: ac->name = "HAX"; > target/i386/hvf/hvf.c:959: ac->name = "HVF"; > target/i386/whpx-all.c:1477: ac->name = "WHPX"; > qtest.c:755: ac->name = "QTest"; > >>> >>>> + } >>>> p(f, "\n"); >>>> range++; >>>> } >>>> @@ -3028,6 +3043,13 @@ void mtree_info(fprintf_function mon_printf, void *f, bool flatview, >>>> }; >>>> GArray *fv_address_spaces; >>>> GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal); >>>> + AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator); >>>> + >>>> + if (ac->has_memory) { >>>> + fvi.ac = ac; >>>> + fvi.ac_name = current_machine->accel ? current_machine->accel : >>>> + object_class_get_name(OBJECT_CLASS(ac)); >>>> + } >>>> >>>> /* Gather all FVs in one table */ >>>> QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) { >>>> -- Alexey ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator 2018-12-18 0:58 ` Alexey Kardashevskiy @ 2019-01-03 17:37 ` Dr. David Alan Gilbert 2019-01-14 1:43 ` Alexey Kardashevskiy 0 siblings, 1 reply; 19+ messages in thread From: Dr. David Alan Gilbert @ 2019-01-03 17:37 UTC (permalink / raw) To: Alexey Kardashevskiy Cc: Philippe Mathieu-Daudé, qemu-devel, Paolo Bonzini, Daniel Henrique Barboza * Alexey Kardashevskiy (aik@ozlabs.ru) wrote: > > > On 17/12/2018 23:47, Philippe Mathieu-Daudé wrote: > > On 12/17/18 2:27 AM, Alexey Kardashevskiy wrote: > >> On 14/12/2018 22:07, Philippe Mathieu-Daudé wrote: > >>> Hi Alexey, > >>> > >>> On 12/14/18 3:58 AM, Alexey Kardashevskiy wrote: > >>>> This adds an accelerator name to the "into mtree -f" to tell the user if > >>>> a particular memory section is registered with the accelerator; > >>>> the primary user for this is KVM and such information is useful > >>>> for debugging purposes. > >>>> > >>>> This adds a has_memory() callback to the accelerator class allowing any > >>>> accelerator to have a label in that memory tree dump. > >>>> > >>>> Since memory sections are passed to memory listeners and get registered > >>>> in accelerators (rather than memory regions), this only prints new labels > >>>> for flatviews attached to the system address space. > >>>> > >>>> An example: > >>>> Root memory region: system > >>>> 0000000000000000-0000002fffffffff (prio 0, ram): /objects/mem0 kvm > >>>> 0000003000000000-0000005fffffffff (prio 0, ram): /objects/mem1 kvm > >>>> 0000200000000020-000020000000003f (prio 1, i/o): virtio-pci > >>>> 0000200080000000-000020008000003f (prio 0, i/o): capabilities > >>>> > >>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > >>>> --- > >>>> > >>>> This supercedes "[PATCH qemu] hmp: Print if memory section is registered in KVM" > >>>> > >>>> --- > >>>> Changes: > >>>> v2: > >>>> * added an accelerator callback instead of hardcoding it to kvm only > >>>> --- > >>>> include/sysemu/accel.h | 2 ++ > >>>> accel/kvm/kvm-all.c | 10 ++++++++++ > >>>> memory.c | 22 ++++++++++++++++++++++ > >>>> 3 files changed, 34 insertions(+) > >>>> > >>>> diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h > >>>> index 637358f..30b456d 100644 > >>>> --- a/include/sysemu/accel.h > >>>> +++ b/include/sysemu/accel.h > >>>> @@ -25,6 +25,7 @@ > >>>> > >>>> #include "qom/object.h" > >>>> #include "hw/qdev-properties.h" > >>>> +#include "exec/hwaddr.h" > >>>> > >>>> typedef struct AccelState { > >>>> /*< private >*/ > >>>> @@ -41,6 +42,7 @@ typedef struct AccelClass { > >>>> int (*available)(void); > >>>> int (*init_machine)(MachineState *ms); > >>>> void (*setup_post)(MachineState *ms, AccelState *accel); > >>>> + bool (*has_memory)(MachineState *ms, hwaddr start_addr, hwaddr size); > >>>> bool *allowed; > >>>> /* > >>>> * Array of global properties that would be applied when specific > >>>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > >>>> index 4880a05..634f386 100644 > >>>> --- a/accel/kvm/kvm-all.c > >>>> +++ b/accel/kvm/kvm-all.c > >>>> @@ -2589,11 +2589,21 @@ int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target) > >>>> return r; > >>>> } > >>>> > >>>> +static bool kvm_accel_has_memory(MachineState *ms, hwaddr start_addr, > >>>> + hwaddr size) > >>>> +{ > >>>> + KVMState *kvm = KVM_STATE(ms->accelerator); > >>>> + KVMMemoryListener *kml = &kvm->memory_listener; > >>>> + > >>>> + return NULL != kvm_lookup_matching_slot(kml, start_addr, size); > >>>> +} > >>>> + > >>>> static void kvm_accel_class_init(ObjectClass *oc, void *data) > >>>> { > >>>> AccelClass *ac = ACCEL_CLASS(oc); > >>>> ac->name = "KVM"; > >>>> ac->init_machine = kvm_init; > >>>> + ac->has_memory = kvm_accel_has_memory; > >>>> ac->allowed = &kvm_allowed; > >>>> } > >>>> > >>>> diff --git a/memory.c b/memory.c > >>>> index d14c6de..61e758a 100644 > >>>> --- a/memory.c > >>>> +++ b/memory.c > >>>> @@ -29,7 +29,9 @@ > >>>> #include "exec/ram_addr.h" > >>>> #include "sysemu/kvm.h" > >>>> #include "sysemu/sysemu.h" > >>>> +#include "sysemu/accel.h" > >>>> #include "hw/qdev-properties.h" > >>>> +#include "hw/boards.h" > >>>> #include "migration/vmstate.h" > >>>> > >>>> //#define DEBUG_UNASSIGNED > >>>> @@ -2924,6 +2926,8 @@ struct FlatViewInfo { > >>>> int counter; > >>>> bool dispatch_tree; > >>>> bool owner; > >>>> + AccelClass *ac; > >>>> + const char *ac_name; > >>>> }; > >>>> > >>>> static void mtree_print_flatview(gpointer key, gpointer value, > >>>> @@ -2939,6 +2943,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, > >>>> int n = view->nr; > >>>> int i; > >>>> AddressSpace *as; > >>>> + bool system_as = false; > >>>> > >>>> p(f, "FlatView #%d\n", fvi->counter); > >>>> ++fvi->counter; > >>>> @@ -2950,6 +2955,9 @@ static void mtree_print_flatview(gpointer key, gpointer value, > >>>> p(f, ", alias %s", memory_region_name(as->root->alias)); > >>>> } > >>>> p(f, "\n"); > >>>> + if (as == &address_space_memory) { > >>>> + system_as = true; > >>>> + } > >>>> } > >>>> > >>>> p(f, " Root memory region: %s\n", > >>>> @@ -2985,6 +2993,13 @@ static void mtree_print_flatview(gpointer key, gpointer value, > >>>> if (fvi->owner) { > >>>> mtree_print_mr_owner(p, f, mr); > >>>> } > >>>> + > >>>> + if (system_as && fvi->ac && > >>>> + fvi->ac->has_memory(current_machine, > >>>> + int128_get64(range->addr.start), > >>>> + MR_SIZE(range->addr.size) + 1)) { > >>>> + p(f, " %s", fvi->ac_name); > >>> > >>> Why not simply display fvi->ac->name? > >>> You could avoid to add the ac_name field. > >> > >> > >> Well, I thought I better print whatever the user passed via the command > >> line (which is current_machine->accel and equals to "kvm" in my case) > >> rather than robotic, dry and excessive "kvm-accel". > > > > I have no hit for 'kvm-accel': > > > > $ git grep kvm-accel > > $ > > > > Names looks human friendly: > > > Ah, I confused with object_class_get_name(). Anyway, I'd still show the > user provided accelerator name than some internal name. Aren't they exactly the same? I think the accel= thing from the user is parsed by comparing it with the ACCEL_CLASS_NAME declarations: ./target/i386/whpx-all.c: .name = ACCEL_CLASS_NAME("whpx"), ./target/i386/hax-all.c: .name = ACCEL_CLASS_NAME("hax"), ./hw/xen/xen-common.c:#define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen") ./qtest.c:#define TYPE_QTEST_ACCEL ACCEL_CLASS_NAME("qtest") ./accel/tcg/tcg-all.c:#define TYPE_TCG_ACCEL ACCEL_CLASS_NAME("tcg") ./include/sysemu/hvf.h:#define TYPE_HVF_ACCEL ACCEL_CLASS_NAME("hvf") ./include/sysemu/kvm_int.h:#define TYPE_KVM_ACCEL ACCEL_CLASS_NAME("kvm") and the code in accel/accel.c accel_find Dave > > > > > $ git grep 'ac->name =' > > accel/kvm/kvm-all.c:2595: ac->name = "KVM"; > > accel/tcg/tcg-all.c:74: ac->name = "tcg"; > > hw/xen/xen-common.c:184: ac->name = "Xen"; > > target/i386/hax-all.c:1088: ac->name = "HAX"; > > target/i386/hvf/hvf.c:959: ac->name = "HVF"; > > target/i386/whpx-all.c:1477: ac->name = "WHPX"; > > qtest.c:755: ac->name = "QTest"; > > > >>> > >>>> + } > >>>> p(f, "\n"); > >>>> range++; > >>>> } > >>>> @@ -3028,6 +3043,13 @@ void mtree_info(fprintf_function mon_printf, void *f, bool flatview, > >>>> }; > >>>> GArray *fv_address_spaces; > >>>> GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal); > >>>> + AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator); > >>>> + > >>>> + if (ac->has_memory) { > >>>> + fvi.ac = ac; > >>>> + fvi.ac_name = current_machine->accel ? current_machine->accel : > >>>> + object_class_get_name(OBJECT_CLASS(ac)); > >>>> + } > >>>> > >>>> /* Gather all FVs in one table */ > >>>> QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) { > >>>> > > -- > Alexey -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator 2019-01-03 17:37 ` Dr. David Alan Gilbert @ 2019-01-14 1:43 ` Alexey Kardashevskiy 2019-01-29 2:30 ` Alexey Kardashevskiy 2019-02-07 11:49 ` Dr. David Alan Gilbert 0 siblings, 2 replies; 19+ messages in thread From: Alexey Kardashevskiy @ 2019-01-14 1:43 UTC (permalink / raw) To: Dr. David Alan Gilbert Cc: Philippe Mathieu-Daudé, qemu-devel, Paolo Bonzini, Daniel Henrique Barboza On 04/01/2019 04:37, Dr. David Alan Gilbert wrote: > * Alexey Kardashevskiy (aik@ozlabs.ru) wrote: >> >> >> On 17/12/2018 23:47, Philippe Mathieu-Daudé wrote: >>> On 12/17/18 2:27 AM, Alexey Kardashevskiy wrote: >>>> On 14/12/2018 22:07, Philippe Mathieu-Daudé wrote: >>>>> Hi Alexey, >>>>> >>>>> On 12/14/18 3:58 AM, Alexey Kardashevskiy wrote: >>>>>> This adds an accelerator name to the "into mtree -f" to tell the user if >>>>>> a particular memory section is registered with the accelerator; >>>>>> the primary user for this is KVM and such information is useful >>>>>> for debugging purposes. >>>>>> >>>>>> This adds a has_memory() callback to the accelerator class allowing any >>>>>> accelerator to have a label in that memory tree dump. >>>>>> >>>>>> Since memory sections are passed to memory listeners and get registered >>>>>> in accelerators (rather than memory regions), this only prints new labels >>>>>> for flatviews attached to the system address space. >>>>>> >>>>>> An example: >>>>>> Root memory region: system >>>>>> 0000000000000000-0000002fffffffff (prio 0, ram): /objects/mem0 kvm >>>>>> 0000003000000000-0000005fffffffff (prio 0, ram): /objects/mem1 kvm >>>>>> 0000200000000020-000020000000003f (prio 1, i/o): virtio-pci >>>>>> 0000200080000000-000020008000003f (prio 0, i/o): capabilities >>>>>> >>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >>>>>> --- >>>>>> >>>>>> This supercedes "[PATCH qemu] hmp: Print if memory section is registered in KVM" >>>>>> >>>>>> --- >>>>>> Changes: >>>>>> v2: >>>>>> * added an accelerator callback instead of hardcoding it to kvm only >>>>>> --- >>>>>> include/sysemu/accel.h | 2 ++ >>>>>> accel/kvm/kvm-all.c | 10 ++++++++++ >>>>>> memory.c | 22 ++++++++++++++++++++++ >>>>>> 3 files changed, 34 insertions(+) >>>>>> >>>>>> diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h >>>>>> index 637358f..30b456d 100644 >>>>>> --- a/include/sysemu/accel.h >>>>>> +++ b/include/sysemu/accel.h >>>>>> @@ -25,6 +25,7 @@ >>>>>> >>>>>> #include "qom/object.h" >>>>>> #include "hw/qdev-properties.h" >>>>>> +#include "exec/hwaddr.h" >>>>>> >>>>>> typedef struct AccelState { >>>>>> /*< private >*/ >>>>>> @@ -41,6 +42,7 @@ typedef struct AccelClass { >>>>>> int (*available)(void); >>>>>> int (*init_machine)(MachineState *ms); >>>>>> void (*setup_post)(MachineState *ms, AccelState *accel); >>>>>> + bool (*has_memory)(MachineState *ms, hwaddr start_addr, hwaddr size); >>>>>> bool *allowed; >>>>>> /* >>>>>> * Array of global properties that would be applied when specific >>>>>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c >>>>>> index 4880a05..634f386 100644 >>>>>> --- a/accel/kvm/kvm-all.c >>>>>> +++ b/accel/kvm/kvm-all.c >>>>>> @@ -2589,11 +2589,21 @@ int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target) >>>>>> return r; >>>>>> } >>>>>> >>>>>> +static bool kvm_accel_has_memory(MachineState *ms, hwaddr start_addr, >>>>>> + hwaddr size) >>>>>> +{ >>>>>> + KVMState *kvm = KVM_STATE(ms->accelerator); >>>>>> + KVMMemoryListener *kml = &kvm->memory_listener; >>>>>> + >>>>>> + return NULL != kvm_lookup_matching_slot(kml, start_addr, size); >>>>>> +} >>>>>> + >>>>>> static void kvm_accel_class_init(ObjectClass *oc, void *data) >>>>>> { >>>>>> AccelClass *ac = ACCEL_CLASS(oc); >>>>>> ac->name = "KVM"; >>>>>> ac->init_machine = kvm_init; >>>>>> + ac->has_memory = kvm_accel_has_memory; >>>>>> ac->allowed = &kvm_allowed; >>>>>> } >>>>>> >>>>>> diff --git a/memory.c b/memory.c >>>>>> index d14c6de..61e758a 100644 >>>>>> --- a/memory.c >>>>>> +++ b/memory.c >>>>>> @@ -29,7 +29,9 @@ >>>>>> #include "exec/ram_addr.h" >>>>>> #include "sysemu/kvm.h" >>>>>> #include "sysemu/sysemu.h" >>>>>> +#include "sysemu/accel.h" >>>>>> #include "hw/qdev-properties.h" >>>>>> +#include "hw/boards.h" >>>>>> #include "migration/vmstate.h" >>>>>> >>>>>> //#define DEBUG_UNASSIGNED >>>>>> @@ -2924,6 +2926,8 @@ struct FlatViewInfo { >>>>>> int counter; >>>>>> bool dispatch_tree; >>>>>> bool owner; >>>>>> + AccelClass *ac; >>>>>> + const char *ac_name; >>>>>> }; >>>>>> >>>>>> static void mtree_print_flatview(gpointer key, gpointer value, >>>>>> @@ -2939,6 +2943,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>>>> int n = view->nr; >>>>>> int i; >>>>>> AddressSpace *as; >>>>>> + bool system_as = false; >>>>>> >>>>>> p(f, "FlatView #%d\n", fvi->counter); >>>>>> ++fvi->counter; >>>>>> @@ -2950,6 +2955,9 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>>>> p(f, ", alias %s", memory_region_name(as->root->alias)); >>>>>> } >>>>>> p(f, "\n"); >>>>>> + if (as == &address_space_memory) { >>>>>> + system_as = true; >>>>>> + } >>>>>> } >>>>>> >>>>>> p(f, " Root memory region: %s\n", >>>>>> @@ -2985,6 +2993,13 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>>>> if (fvi->owner) { >>>>>> mtree_print_mr_owner(p, f, mr); >>>>>> } >>>>>> + >>>>>> + if (system_as && fvi->ac && >>>>>> + fvi->ac->has_memory(current_machine, >>>>>> + int128_get64(range->addr.start), >>>>>> + MR_SIZE(range->addr.size) + 1)) { >>>>>> + p(f, " %s", fvi->ac_name); >>>>> >>>>> Why not simply display fvi->ac->name? >>>>> You could avoid to add the ac_name field. >>>> >>>> >>>> Well, I thought I better print whatever the user passed via the command >>>> line (which is current_machine->accel and equals to "kvm" in my case) >>>> rather than robotic, dry and excessive "kvm-accel". >>> >>> I have no hit for 'kvm-accel': >>> >>> $ git grep kvm-accel >>> $ >>> >>> Names looks human friendly: >> >> >> Ah, I confused with object_class_get_name(). Anyway, I'd still show the >> user provided accelerator name than some internal name. > > Aren't they exactly the same? Nope. "kvm" vs. "kvm-accel". > I think the accel= thing from the user is > parsed by comparing it with the ACCEL_CLASS_NAME declarations: > > ./target/i386/whpx-all.c: .name = ACCEL_CLASS_NAME("whpx"), > ./target/i386/hax-all.c: .name = ACCEL_CLASS_NAME("hax"), > ./hw/xen/xen-common.c:#define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen") > ./qtest.c:#define TYPE_QTEST_ACCEL ACCEL_CLASS_NAME("qtest") > ./accel/tcg/tcg-all.c:#define TYPE_TCG_ACCEL ACCEL_CLASS_NAME("tcg") > ./include/sysemu/hvf.h:#define TYPE_HVF_ACCEL ACCEL_CLASS_NAME("hvf") > ./include/sysemu/kvm_int.h:#define TYPE_KVM_ACCEL ACCEL_CLASS_NAME("kvm") > > and the code in accel/accel.c accel_find https://git.qemu.org/?p=qemu.git;a=blob;f=include/sysemu/accel.h;h=5565e00a96136bc5024c17c401116ef6a497138a;hb=HEAD#l58 55 #define TYPE_ACCEL "accel" 56 57 #define ACCEL_CLASS_SUFFIX "-" TYPE_ACCEL 58 #define ACCEL_CLASS_NAME(a) (a ACCEL_CLASS_SUFFIX) > > Dave > >> >>> >>> $ git grep 'ac->name =' >>> accel/kvm/kvm-all.c:2595: ac->name = "KVM"; >>> accel/tcg/tcg-all.c:74: ac->name = "tcg"; >>> hw/xen/xen-common.c:184: ac->name = "Xen"; >>> target/i386/hax-all.c:1088: ac->name = "HAX"; >>> target/i386/hvf/hvf.c:959: ac->name = "HVF"; >>> target/i386/whpx-all.c:1477: ac->name = "WHPX"; >>> qtest.c:755: ac->name = "QTest"; >>> >>>>> >>>>>> + } >>>>>> p(f, "\n"); >>>>>> range++; >>>>>> } >>>>>> @@ -3028,6 +3043,13 @@ void mtree_info(fprintf_function mon_printf, void *f, bool flatview, >>>>>> }; >>>>>> GArray *fv_address_spaces; >>>>>> GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal); >>>>>> + AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator); >>>>>> + >>>>>> + if (ac->has_memory) { >>>>>> + fvi.ac = ac; >>>>>> + fvi.ac_name = current_machine->accel ? current_machine->accel : >>>>>> + object_class_get_name(OBJECT_CLASS(ac)); >>>>>> + } >>>>>> >>>>>> /* Gather all FVs in one table */ >>>>>> QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) { >>>>>> >> >> -- >> Alexey > -- > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > -- Alexey ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator 2019-01-14 1:43 ` Alexey Kardashevskiy @ 2019-01-29 2:30 ` Alexey Kardashevskiy 2019-02-07 5:20 ` Alexey Kardashevskiy 2019-02-07 11:49 ` Dr. David Alan Gilbert 1 sibling, 1 reply; 19+ messages in thread From: Alexey Kardashevskiy @ 2019-01-29 2:30 UTC (permalink / raw) To: Dr. David Alan Gilbert Cc: Philippe Mathieu-Daudé, qemu-devel, Paolo Bonzini, Daniel Henrique Barboza On 14/01/2019 12:43, Alexey Kardashevskiy wrote: > > > On 04/01/2019 04:37, Dr. David Alan Gilbert wrote: >> * Alexey Kardashevskiy (aik@ozlabs.ru) wrote: >>> >>> >>> On 17/12/2018 23:47, Philippe Mathieu-Daudé wrote: >>>> On 12/17/18 2:27 AM, Alexey Kardashevskiy wrote: >>>>> On 14/12/2018 22:07, Philippe Mathieu-Daudé wrote: >>>>>> Hi Alexey, >>>>>> >>>>>> On 12/14/18 3:58 AM, Alexey Kardashevskiy wrote: >>>>>>> This adds an accelerator name to the "into mtree -f" to tell the user if >>>>>>> a particular memory section is registered with the accelerator; >>>>>>> the primary user for this is KVM and such information is useful >>>>>>> for debugging purposes. >>>>>>> >>>>>>> This adds a has_memory() callback to the accelerator class allowing any >>>>>>> accelerator to have a label in that memory tree dump. >>>>>>> >>>>>>> Since memory sections are passed to memory listeners and get registered >>>>>>> in accelerators (rather than memory regions), this only prints new labels >>>>>>> for flatviews attached to the system address space. >>>>>>> >>>>>>> An example: >>>>>>> Root memory region: system >>>>>>> 0000000000000000-0000002fffffffff (prio 0, ram): /objects/mem0 kvm >>>>>>> 0000003000000000-0000005fffffffff (prio 0, ram): /objects/mem1 kvm >>>>>>> 0000200000000020-000020000000003f (prio 1, i/o): virtio-pci >>>>>>> 0000200080000000-000020008000003f (prio 0, i/o): capabilities >>>>>>> >>>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >>>>>>> --- >>>>>>> >>>>>>> This supercedes "[PATCH qemu] hmp: Print if memory section is registered in KVM" >>>>>>> >>>>>>> --- >>>>>>> Changes: >>>>>>> v2: >>>>>>> * added an accelerator callback instead of hardcoding it to kvm only >>>>>>> --- >>>>>>> include/sysemu/accel.h | 2 ++ >>>>>>> accel/kvm/kvm-all.c | 10 ++++++++++ >>>>>>> memory.c | 22 ++++++++++++++++++++++ >>>>>>> 3 files changed, 34 insertions(+) >>>>>>> >>>>>>> diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h >>>>>>> index 637358f..30b456d 100644 >>>>>>> --- a/include/sysemu/accel.h >>>>>>> +++ b/include/sysemu/accel.h >>>>>>> @@ -25,6 +25,7 @@ >>>>>>> >>>>>>> #include "qom/object.h" >>>>>>> #include "hw/qdev-properties.h" >>>>>>> +#include "exec/hwaddr.h" >>>>>>> >>>>>>> typedef struct AccelState { >>>>>>> /*< private >*/ >>>>>>> @@ -41,6 +42,7 @@ typedef struct AccelClass { >>>>>>> int (*available)(void); >>>>>>> int (*init_machine)(MachineState *ms); >>>>>>> void (*setup_post)(MachineState *ms, AccelState *accel); >>>>>>> + bool (*has_memory)(MachineState *ms, hwaddr start_addr, hwaddr size); >>>>>>> bool *allowed; >>>>>>> /* >>>>>>> * Array of global properties that would be applied when specific >>>>>>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c >>>>>>> index 4880a05..634f386 100644 >>>>>>> --- a/accel/kvm/kvm-all.c >>>>>>> +++ b/accel/kvm/kvm-all.c >>>>>>> @@ -2589,11 +2589,21 @@ int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target) >>>>>>> return r; >>>>>>> } >>>>>>> >>>>>>> +static bool kvm_accel_has_memory(MachineState *ms, hwaddr start_addr, >>>>>>> + hwaddr size) >>>>>>> +{ >>>>>>> + KVMState *kvm = KVM_STATE(ms->accelerator); >>>>>>> + KVMMemoryListener *kml = &kvm->memory_listener; >>>>>>> + >>>>>>> + return NULL != kvm_lookup_matching_slot(kml, start_addr, size); >>>>>>> +} >>>>>>> + >>>>>>> static void kvm_accel_class_init(ObjectClass *oc, void *data) >>>>>>> { >>>>>>> AccelClass *ac = ACCEL_CLASS(oc); >>>>>>> ac->name = "KVM"; >>>>>>> ac->init_machine = kvm_init; >>>>>>> + ac->has_memory = kvm_accel_has_memory; >>>>>>> ac->allowed = &kvm_allowed; >>>>>>> } >>>>>>> >>>>>>> diff --git a/memory.c b/memory.c >>>>>>> index d14c6de..61e758a 100644 >>>>>>> --- a/memory.c >>>>>>> +++ b/memory.c >>>>>>> @@ -29,7 +29,9 @@ >>>>>>> #include "exec/ram_addr.h" >>>>>>> #include "sysemu/kvm.h" >>>>>>> #include "sysemu/sysemu.h" >>>>>>> +#include "sysemu/accel.h" >>>>>>> #include "hw/qdev-properties.h" >>>>>>> +#include "hw/boards.h" >>>>>>> #include "migration/vmstate.h" >>>>>>> >>>>>>> //#define DEBUG_UNASSIGNED >>>>>>> @@ -2924,6 +2926,8 @@ struct FlatViewInfo { >>>>>>> int counter; >>>>>>> bool dispatch_tree; >>>>>>> bool owner; >>>>>>> + AccelClass *ac; >>>>>>> + const char *ac_name; >>>>>>> }; >>>>>>> >>>>>>> static void mtree_print_flatview(gpointer key, gpointer value, >>>>>>> @@ -2939,6 +2943,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>>>>> int n = view->nr; >>>>>>> int i; >>>>>>> AddressSpace *as; >>>>>>> + bool system_as = false; >>>>>>> >>>>>>> p(f, "FlatView #%d\n", fvi->counter); >>>>>>> ++fvi->counter; >>>>>>> @@ -2950,6 +2955,9 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>>>>> p(f, ", alias %s", memory_region_name(as->root->alias)); >>>>>>> } >>>>>>> p(f, "\n"); >>>>>>> + if (as == &address_space_memory) { >>>>>>> + system_as = true; >>>>>>> + } >>>>>>> } >>>>>>> >>>>>>> p(f, " Root memory region: %s\n", >>>>>>> @@ -2985,6 +2993,13 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>>>>> if (fvi->owner) { >>>>>>> mtree_print_mr_owner(p, f, mr); >>>>>>> } >>>>>>> + >>>>>>> + if (system_as && fvi->ac && >>>>>>> + fvi->ac->has_memory(current_machine, >>>>>>> + int128_get64(range->addr.start), >>>>>>> + MR_SIZE(range->addr.size) + 1)) { >>>>>>> + p(f, " %s", fvi->ac_name); >>>>>> >>>>>> Why not simply display fvi->ac->name? >>>>>> You could avoid to add the ac_name field. >>>>> >>>>> >>>>> Well, I thought I better print whatever the user passed via the command >>>>> line (which is current_machine->accel and equals to "kvm" in my case) >>>>> rather than robotic, dry and excessive "kvm-accel". >>>> >>>> I have no hit for 'kvm-accel': >>>> >>>> $ git grep kvm-accel >>>> $ >>>> >>>> Names looks human friendly: >>> >>> >>> Ah, I confused with object_class_get_name(). Anyway, I'd still show the >>> user provided accelerator name than some internal name. >> >> Aren't they exactly the same? > > Nope. "kvm" vs. "kvm-accel". > >> I think the accel= thing from the user is >> parsed by comparing it with the ACCEL_CLASS_NAME declarations: >> >> ./target/i386/whpx-all.c: .name = ACCEL_CLASS_NAME("whpx"), >> ./target/i386/hax-all.c: .name = ACCEL_CLASS_NAME("hax"), >> ./hw/xen/xen-common.c:#define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen") >> ./qtest.c:#define TYPE_QTEST_ACCEL ACCEL_CLASS_NAME("qtest") >> ./accel/tcg/tcg-all.c:#define TYPE_TCG_ACCEL ACCEL_CLASS_NAME("tcg") >> ./include/sysemu/hvf.h:#define TYPE_HVF_ACCEL ACCEL_CLASS_NAME("hvf") >> ./include/sysemu/kvm_int.h:#define TYPE_KVM_ACCEL ACCEL_CLASS_NAME("kvm") >> >> and the code in accel/accel.c accel_find > > > https://git.qemu.org/?p=qemu.git;a=blob;f=include/sysemu/accel.h;h=5565e00a96136bc5024c17c401116ef6a497138a;hb=HEAD#l58 > > 55 #define TYPE_ACCEL "accel" > 56 > 57 #define ACCEL_CLASS_SUFFIX "-" TYPE_ACCEL > 58 #define ACCEL_CLASS_NAME(a) (a ACCEL_CLASS_SUFFIX) Ping, anybody? > > > >> >> Dave >> >>> >>>> >>>> $ git grep 'ac->name =' >>>> accel/kvm/kvm-all.c:2595: ac->name = "KVM"; >>>> accel/tcg/tcg-all.c:74: ac->name = "tcg"; >>>> hw/xen/xen-common.c:184: ac->name = "Xen"; >>>> target/i386/hax-all.c:1088: ac->name = "HAX"; >>>> target/i386/hvf/hvf.c:959: ac->name = "HVF"; >>>> target/i386/whpx-all.c:1477: ac->name = "WHPX"; >>>> qtest.c:755: ac->name = "QTest"; >>>> >>>>>> >>>>>>> + } >>>>>>> p(f, "\n"); >>>>>>> range++; >>>>>>> } >>>>>>> @@ -3028,6 +3043,13 @@ void mtree_info(fprintf_function mon_printf, void *f, bool flatview, >>>>>>> }; >>>>>>> GArray *fv_address_spaces; >>>>>>> GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal); >>>>>>> + AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator); >>>>>>> + >>>>>>> + if (ac->has_memory) { >>>>>>> + fvi.ac = ac; >>>>>>> + fvi.ac_name = current_machine->accel ? current_machine->accel : >>>>>>> + object_class_get_name(OBJECT_CLASS(ac)); >>>>>>> + } >>>>>>> >>>>>>> /* Gather all FVs in one table */ >>>>>>> QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) { >>>>>>> >>> >>> -- >>> Alexey >> -- >> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >> > -- Alexey ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator 2019-01-29 2:30 ` Alexey Kardashevskiy @ 2019-02-07 5:20 ` Alexey Kardashevskiy 0 siblings, 0 replies; 19+ messages in thread From: Alexey Kardashevskiy @ 2019-02-07 5:20 UTC (permalink / raw) To: Dr. David Alan Gilbert Cc: Philippe Mathieu-Daudé, qemu-devel, Paolo Bonzini, Daniel Henrique Barboza On 29/01/2019 13:30, Alexey Kardashevskiy wrote: > > > On 14/01/2019 12:43, Alexey Kardashevskiy wrote: >> >> >> On 04/01/2019 04:37, Dr. David Alan Gilbert wrote: >>> * Alexey Kardashevskiy (aik@ozlabs.ru) wrote: >>>> >>>> >>>> On 17/12/2018 23:47, Philippe Mathieu-Daudé wrote: >>>>> On 12/17/18 2:27 AM, Alexey Kardashevskiy wrote: >>>>>> On 14/12/2018 22:07, Philippe Mathieu-Daudé wrote: >>>>>>> Hi Alexey, >>>>>>> >>>>>>> On 12/14/18 3:58 AM, Alexey Kardashevskiy wrote: >>>>>>>> This adds an accelerator name to the "into mtree -f" to tell the user if >>>>>>>> a particular memory section is registered with the accelerator; >>>>>>>> the primary user for this is KVM and such information is useful >>>>>>>> for debugging purposes. >>>>>>>> >>>>>>>> This adds a has_memory() callback to the accelerator class allowing any >>>>>>>> accelerator to have a label in that memory tree dump. >>>>>>>> >>>>>>>> Since memory sections are passed to memory listeners and get registered >>>>>>>> in accelerators (rather than memory regions), this only prints new labels >>>>>>>> for flatviews attached to the system address space. >>>>>>>> >>>>>>>> An example: >>>>>>>> Root memory region: system >>>>>>>> 0000000000000000-0000002fffffffff (prio 0, ram): /objects/mem0 kvm >>>>>>>> 0000003000000000-0000005fffffffff (prio 0, ram): /objects/mem1 kvm >>>>>>>> 0000200000000020-000020000000003f (prio 1, i/o): virtio-pci >>>>>>>> 0000200080000000-000020008000003f (prio 0, i/o): capabilities >>>>>>>> >>>>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >>>>>>>> --- >>>>>>>> >>>>>>>> This supercedes "[PATCH qemu] hmp: Print if memory section is registered in KVM" >>>>>>>> >>>>>>>> --- >>>>>>>> Changes: >>>>>>>> v2: >>>>>>>> * added an accelerator callback instead of hardcoding it to kvm only >>>>>>>> --- >>>>>>>> include/sysemu/accel.h | 2 ++ >>>>>>>> accel/kvm/kvm-all.c | 10 ++++++++++ >>>>>>>> memory.c | 22 ++++++++++++++++++++++ >>>>>>>> 3 files changed, 34 insertions(+) >>>>>>>> >>>>>>>> diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h >>>>>>>> index 637358f..30b456d 100644 >>>>>>>> --- a/include/sysemu/accel.h >>>>>>>> +++ b/include/sysemu/accel.h >>>>>>>> @@ -25,6 +25,7 @@ >>>>>>>> >>>>>>>> #include "qom/object.h" >>>>>>>> #include "hw/qdev-properties.h" >>>>>>>> +#include "exec/hwaddr.h" >>>>>>>> >>>>>>>> typedef struct AccelState { >>>>>>>> /*< private >*/ >>>>>>>> @@ -41,6 +42,7 @@ typedef struct AccelClass { >>>>>>>> int (*available)(void); >>>>>>>> int (*init_machine)(MachineState *ms); >>>>>>>> void (*setup_post)(MachineState *ms, AccelState *accel); >>>>>>>> + bool (*has_memory)(MachineState *ms, hwaddr start_addr, hwaddr size); >>>>>>>> bool *allowed; >>>>>>>> /* >>>>>>>> * Array of global properties that would be applied when specific >>>>>>>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c >>>>>>>> index 4880a05..634f386 100644 >>>>>>>> --- a/accel/kvm/kvm-all.c >>>>>>>> +++ b/accel/kvm/kvm-all.c >>>>>>>> @@ -2589,11 +2589,21 @@ int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target) >>>>>>>> return r; >>>>>>>> } >>>>>>>> >>>>>>>> +static bool kvm_accel_has_memory(MachineState *ms, hwaddr start_addr, >>>>>>>> + hwaddr size) >>>>>>>> +{ >>>>>>>> + KVMState *kvm = KVM_STATE(ms->accelerator); >>>>>>>> + KVMMemoryListener *kml = &kvm->memory_listener; >>>>>>>> + >>>>>>>> + return NULL != kvm_lookup_matching_slot(kml, start_addr, size); >>>>>>>> +} >>>>>>>> + >>>>>>>> static void kvm_accel_class_init(ObjectClass *oc, void *data) >>>>>>>> { >>>>>>>> AccelClass *ac = ACCEL_CLASS(oc); >>>>>>>> ac->name = "KVM"; >>>>>>>> ac->init_machine = kvm_init; >>>>>>>> + ac->has_memory = kvm_accel_has_memory; >>>>>>>> ac->allowed = &kvm_allowed; >>>>>>>> } >>>>>>>> >>>>>>>> diff --git a/memory.c b/memory.c >>>>>>>> index d14c6de..61e758a 100644 >>>>>>>> --- a/memory.c >>>>>>>> +++ b/memory.c >>>>>>>> @@ -29,7 +29,9 @@ >>>>>>>> #include "exec/ram_addr.h" >>>>>>>> #include "sysemu/kvm.h" >>>>>>>> #include "sysemu/sysemu.h" >>>>>>>> +#include "sysemu/accel.h" >>>>>>>> #include "hw/qdev-properties.h" >>>>>>>> +#include "hw/boards.h" >>>>>>>> #include "migration/vmstate.h" >>>>>>>> >>>>>>>> //#define DEBUG_UNASSIGNED >>>>>>>> @@ -2924,6 +2926,8 @@ struct FlatViewInfo { >>>>>>>> int counter; >>>>>>>> bool dispatch_tree; >>>>>>>> bool owner; >>>>>>>> + AccelClass *ac; >>>>>>>> + const char *ac_name; >>>>>>>> }; >>>>>>>> >>>>>>>> static void mtree_print_flatview(gpointer key, gpointer value, >>>>>>>> @@ -2939,6 +2943,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>>>>>> int n = view->nr; >>>>>>>> int i; >>>>>>>> AddressSpace *as; >>>>>>>> + bool system_as = false; >>>>>>>> >>>>>>>> p(f, "FlatView #%d\n", fvi->counter); >>>>>>>> ++fvi->counter; >>>>>>>> @@ -2950,6 +2955,9 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>>>>>> p(f, ", alias %s", memory_region_name(as->root->alias)); >>>>>>>> } >>>>>>>> p(f, "\n"); >>>>>>>> + if (as == &address_space_memory) { >>>>>>>> + system_as = true; >>>>>>>> + } >>>>>>>> } >>>>>>>> >>>>>>>> p(f, " Root memory region: %s\n", >>>>>>>> @@ -2985,6 +2993,13 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>>>>>> if (fvi->owner) { >>>>>>>> mtree_print_mr_owner(p, f, mr); >>>>>>>> } >>>>>>>> + >>>>>>>> + if (system_as && fvi->ac && >>>>>>>> + fvi->ac->has_memory(current_machine, >>>>>>>> + int128_get64(range->addr.start), >>>>>>>> + MR_SIZE(range->addr.size) + 1)) { >>>>>>>> + p(f, " %s", fvi->ac_name); >>>>>>> >>>>>>> Why not simply display fvi->ac->name? >>>>>>> You could avoid to add the ac_name field. >>>>>> >>>>>> >>>>>> Well, I thought I better print whatever the user passed via the command >>>>>> line (which is current_machine->accel and equals to "kvm" in my case) >>>>>> rather than robotic, dry and excessive "kvm-accel". >>>>> >>>>> I have no hit for 'kvm-accel': >>>>> >>>>> $ git grep kvm-accel >>>>> $ >>>>> >>>>> Names looks human friendly: >>>> >>>> >>>> Ah, I confused with object_class_get_name(). Anyway, I'd still show the >>>> user provided accelerator name than some internal name. >>> >>> Aren't they exactly the same? >> >> Nope. "kvm" vs. "kvm-accel". >> >>> I think the accel= thing from the user is >>> parsed by comparing it with the ACCEL_CLASS_NAME declarations: >>> >>> ./target/i386/whpx-all.c: .name = ACCEL_CLASS_NAME("whpx"), >>> ./target/i386/hax-all.c: .name = ACCEL_CLASS_NAME("hax"), >>> ./hw/xen/xen-common.c:#define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen") >>> ./qtest.c:#define TYPE_QTEST_ACCEL ACCEL_CLASS_NAME("qtest") >>> ./accel/tcg/tcg-all.c:#define TYPE_TCG_ACCEL ACCEL_CLASS_NAME("tcg") >>> ./include/sysemu/hvf.h:#define TYPE_HVF_ACCEL ACCEL_CLASS_NAME("hvf") >>> ./include/sysemu/kvm_int.h:#define TYPE_KVM_ACCEL ACCEL_CLASS_NAME("kvm") >>> >>> and the code in accel/accel.c accel_find >> >> >> https://git.qemu.org/?p=qemu.git;a=blob;f=include/sysemu/accel.h;h=5565e00a96136bc5024c17c401116ef6a497138a;hb=HEAD#l58 >> >> 55 #define TYPE_ACCEL "accel" >> 56 >> 57 #define ACCEL_CLASS_SUFFIX "-" TYPE_ACCEL >> 58 #define ACCEL_CLASS_NAME(a) (a ACCEL_CLASS_SUFFIX) > > > Ping, anybody? Yet another friendly ping. Any comments? Thanks. > > >> >> >> >>> >>> Dave >>> >>>> >>>>> >>>>> $ git grep 'ac->name =' >>>>> accel/kvm/kvm-all.c:2595: ac->name = "KVM"; >>>>> accel/tcg/tcg-all.c:74: ac->name = "tcg"; >>>>> hw/xen/xen-common.c:184: ac->name = "Xen"; >>>>> target/i386/hax-all.c:1088: ac->name = "HAX"; >>>>> target/i386/hvf/hvf.c:959: ac->name = "HVF"; >>>>> target/i386/whpx-all.c:1477: ac->name = "WHPX"; >>>>> qtest.c:755: ac->name = "QTest"; >>>>> >>>>>>> >>>>>>>> + } >>>>>>>> p(f, "\n"); >>>>>>>> range++; >>>>>>>> } >>>>>>>> @@ -3028,6 +3043,13 @@ void mtree_info(fprintf_function mon_printf, void *f, bool flatview, >>>>>>>> }; >>>>>>>> GArray *fv_address_spaces; >>>>>>>> GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal); >>>>>>>> + AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator); >>>>>>>> + >>>>>>>> + if (ac->has_memory) { >>>>>>>> + fvi.ac = ac; >>>>>>>> + fvi.ac_name = current_machine->accel ? current_machine->accel : >>>>>>>> + object_class_get_name(OBJECT_CLASS(ac)); >>>>>>>> + } >>>>>>>> >>>>>>>> /* Gather all FVs in one table */ >>>>>>>> QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) { >>>>>>>> >>>> >>>> -- >>>> Alexey >>> -- >>> Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK >>> >> > -- Alexey ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator 2019-01-14 1:43 ` Alexey Kardashevskiy 2019-01-29 2:30 ` Alexey Kardashevskiy @ 2019-02-07 11:49 ` Dr. David Alan Gilbert 2019-02-08 17:26 ` Paolo Bonzini 1 sibling, 1 reply; 19+ messages in thread From: Dr. David Alan Gilbert @ 2019-02-07 11:49 UTC (permalink / raw) To: Alexey Kardashevskiy Cc: Philippe Mathieu-Daudé, qemu-devel, Paolo Bonzini, Daniel Henrique Barboza * Alexey Kardashevskiy (aik@ozlabs.ru) wrote: > > > On 04/01/2019 04:37, Dr. David Alan Gilbert wrote: > > * Alexey Kardashevskiy (aik@ozlabs.ru) wrote: > >> > >> > >> On 17/12/2018 23:47, Philippe Mathieu-Daudé wrote: > >>> On 12/17/18 2:27 AM, Alexey Kardashevskiy wrote: > >>>> On 14/12/2018 22:07, Philippe Mathieu-Daudé wrote: > >>>>> Hi Alexey, > >>>>> > >>>>> On 12/14/18 3:58 AM, Alexey Kardashevskiy wrote: > >>>>>> This adds an accelerator name to the "into mtree -f" to tell the user if > >>>>>> a particular memory section is registered with the accelerator; > >>>>>> the primary user for this is KVM and such information is useful > >>>>>> for debugging purposes. > >>>>>> > >>>>>> This adds a has_memory() callback to the accelerator class allowing any > >>>>>> accelerator to have a label in that memory tree dump. > >>>>>> > >>>>>> Since memory sections are passed to memory listeners and get registered > >>>>>> in accelerators (rather than memory regions), this only prints new labels > >>>>>> for flatviews attached to the system address space. > >>>>>> > >>>>>> An example: > >>>>>> Root memory region: system > >>>>>> 0000000000000000-0000002fffffffff (prio 0, ram): /objects/mem0 kvm > >>>>>> 0000003000000000-0000005fffffffff (prio 0, ram): /objects/mem1 kvm > >>>>>> 0000200000000020-000020000000003f (prio 1, i/o): virtio-pci > >>>>>> 0000200080000000-000020008000003f (prio 0, i/o): capabilities > >>>>>> > >>>>>> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > >>>>>> --- > >>>>>> > >>>>>> This supercedes "[PATCH qemu] hmp: Print if memory section is registered in KVM" > >>>>>> > >>>>>> --- > >>>>>> Changes: > >>>>>> v2: > >>>>>> * added an accelerator callback instead of hardcoding it to kvm only > >>>>>> --- > >>>>>> include/sysemu/accel.h | 2 ++ > >>>>>> accel/kvm/kvm-all.c | 10 ++++++++++ > >>>>>> memory.c | 22 ++++++++++++++++++++++ > >>>>>> 3 files changed, 34 insertions(+) > >>>>>> > >>>>>> diff --git a/include/sysemu/accel.h b/include/sysemu/accel.h > >>>>>> index 637358f..30b456d 100644 > >>>>>> --- a/include/sysemu/accel.h > >>>>>> +++ b/include/sysemu/accel.h > >>>>>> @@ -25,6 +25,7 @@ > >>>>>> > >>>>>> #include "qom/object.h" > >>>>>> #include "hw/qdev-properties.h" > >>>>>> +#include "exec/hwaddr.h" > >>>>>> > >>>>>> typedef struct AccelState { > >>>>>> /*< private >*/ > >>>>>> @@ -41,6 +42,7 @@ typedef struct AccelClass { > >>>>>> int (*available)(void); > >>>>>> int (*init_machine)(MachineState *ms); > >>>>>> void (*setup_post)(MachineState *ms, AccelState *accel); > >>>>>> + bool (*has_memory)(MachineState *ms, hwaddr start_addr, hwaddr size); > >>>>>> bool *allowed; > >>>>>> /* > >>>>>> * Array of global properties that would be applied when specific > >>>>>> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > >>>>>> index 4880a05..634f386 100644 > >>>>>> --- a/accel/kvm/kvm-all.c > >>>>>> +++ b/accel/kvm/kvm-all.c > >>>>>> @@ -2589,11 +2589,21 @@ int kvm_get_one_reg(CPUState *cs, uint64_t id, void *target) > >>>>>> return r; > >>>>>> } > >>>>>> > >>>>>> +static bool kvm_accel_has_memory(MachineState *ms, hwaddr start_addr, > >>>>>> + hwaddr size) > >>>>>> +{ > >>>>>> + KVMState *kvm = KVM_STATE(ms->accelerator); > >>>>>> + KVMMemoryListener *kml = &kvm->memory_listener; > >>>>>> + > >>>>>> + return NULL != kvm_lookup_matching_slot(kml, start_addr, size); > >>>>>> +} > >>>>>> + > >>>>>> static void kvm_accel_class_init(ObjectClass *oc, void *data) > >>>>>> { > >>>>>> AccelClass *ac = ACCEL_CLASS(oc); > >>>>>> ac->name = "KVM"; > >>>>>> ac->init_machine = kvm_init; > >>>>>> + ac->has_memory = kvm_accel_has_memory; > >>>>>> ac->allowed = &kvm_allowed; > >>>>>> } > >>>>>> > >>>>>> diff --git a/memory.c b/memory.c > >>>>>> index d14c6de..61e758a 100644 > >>>>>> --- a/memory.c > >>>>>> +++ b/memory.c > >>>>>> @@ -29,7 +29,9 @@ > >>>>>> #include "exec/ram_addr.h" > >>>>>> #include "sysemu/kvm.h" > >>>>>> #include "sysemu/sysemu.h" > >>>>>> +#include "sysemu/accel.h" > >>>>>> #include "hw/qdev-properties.h" > >>>>>> +#include "hw/boards.h" > >>>>>> #include "migration/vmstate.h" > >>>>>> > >>>>>> //#define DEBUG_UNASSIGNED > >>>>>> @@ -2924,6 +2926,8 @@ struct FlatViewInfo { > >>>>>> int counter; > >>>>>> bool dispatch_tree; > >>>>>> bool owner; > >>>>>> + AccelClass *ac; > >>>>>> + const char *ac_name; > >>>>>> }; > >>>>>> > >>>>>> static void mtree_print_flatview(gpointer key, gpointer value, > >>>>>> @@ -2939,6 +2943,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, > >>>>>> int n = view->nr; > >>>>>> int i; > >>>>>> AddressSpace *as; > >>>>>> + bool system_as = false; > >>>>>> > >>>>>> p(f, "FlatView #%d\n", fvi->counter); > >>>>>> ++fvi->counter; > >>>>>> @@ -2950,6 +2955,9 @@ static void mtree_print_flatview(gpointer key, gpointer value, > >>>>>> p(f, ", alias %s", memory_region_name(as->root->alias)); > >>>>>> } > >>>>>> p(f, "\n"); > >>>>>> + if (as == &address_space_memory) { > >>>>>> + system_as = true; > >>>>>> + } > >>>>>> } > >>>>>> > >>>>>> p(f, " Root memory region: %s\n", > >>>>>> @@ -2985,6 +2993,13 @@ static void mtree_print_flatview(gpointer key, gpointer value, > >>>>>> if (fvi->owner) { > >>>>>> mtree_print_mr_owner(p, f, mr); > >>>>>> } > >>>>>> + > >>>>>> + if (system_as && fvi->ac && > >>>>>> + fvi->ac->has_memory(current_machine, > >>>>>> + int128_get64(range->addr.start), > >>>>>> + MR_SIZE(range->addr.size) + 1)) { > >>>>>> + p(f, " %s", fvi->ac_name); > >>>>> > >>>>> Why not simply display fvi->ac->name? > >>>>> You could avoid to add the ac_name field. > >>>> > >>>> > >>>> Well, I thought I better print whatever the user passed via the command > >>>> line (which is current_machine->accel and equals to "kvm" in my case) > >>>> rather than robotic, dry and excessive "kvm-accel". > >>> > >>> I have no hit for 'kvm-accel': > >>> > >>> $ git grep kvm-accel > >>> $ > >>> > >>> Names looks human friendly: > >> > >> > >> Ah, I confused with object_class_get_name(). Anyway, I'd still show the > >> user provided accelerator name than some internal name. > > > > Aren't they exactly the same? > > Nope. "kvm" vs. "kvm-accel". > > > I think the accel= thing from the user is > > parsed by comparing it with the ACCEL_CLASS_NAME declarations: > > > > ./target/i386/whpx-all.c: .name = ACCEL_CLASS_NAME("whpx"), > > ./target/i386/hax-all.c: .name = ACCEL_CLASS_NAME("hax"), > > ./hw/xen/xen-common.c:#define TYPE_XEN_ACCEL ACCEL_CLASS_NAME("xen") > > ./qtest.c:#define TYPE_QTEST_ACCEL ACCEL_CLASS_NAME("qtest") > > ./accel/tcg/tcg-all.c:#define TYPE_TCG_ACCEL ACCEL_CLASS_NAME("tcg") > > ./include/sysemu/hvf.h:#define TYPE_HVF_ACCEL ACCEL_CLASS_NAME("hvf") > > ./include/sysemu/kvm_int.h:#define TYPE_KVM_ACCEL ACCEL_CLASS_NAME("kvm") > > > > and the code in accel/accel.c accel_find > > > https://git.qemu.org/?p=qemu.git;a=blob;f=include/sysemu/accel.h;h=5565e00a96136bc5024c17c401116ef6a497138a;hb=HEAD#l58 > > 55 #define TYPE_ACCEL "accel" > 56 > 57 #define ACCEL_CLASS_SUFFIX "-" TYPE_ACCEL > 58 #define ACCEL_CLASS_NAME(a) (a ACCEL_CLASS_SUFFIX) (Following up from your pings). I'm OK from the HMP side, but I agree it would be nice to avoid adding the ac_name field in FlatViewInfo if it's only reason is to get rid of the '-accel'. If that's the only reason why don't you just truncate the fvi->ac->name when you display it? You could do a build-time check that TYPE_ACCEL and ACCEL_CLASS_SUFFIX haven't changed. Dave > > > > > > Dave > > > >> > >>> > >>> $ git grep 'ac->name =' > >>> accel/kvm/kvm-all.c:2595: ac->name = "KVM"; > >>> accel/tcg/tcg-all.c:74: ac->name = "tcg"; > >>> hw/xen/xen-common.c:184: ac->name = "Xen"; > >>> target/i386/hax-all.c:1088: ac->name = "HAX"; > >>> target/i386/hvf/hvf.c:959: ac->name = "HVF"; > >>> target/i386/whpx-all.c:1477: ac->name = "WHPX"; > >>> qtest.c:755: ac->name = "QTest"; > >>> > >>>>> > >>>>>> + } > >>>>>> p(f, "\n"); > >>>>>> range++; > >>>>>> } > >>>>>> @@ -3028,6 +3043,13 @@ void mtree_info(fprintf_function mon_printf, void *f, bool flatview, > >>>>>> }; > >>>>>> GArray *fv_address_spaces; > >>>>>> GHashTable *views = g_hash_table_new(g_direct_hash, g_direct_equal); > >>>>>> + AccelClass *ac = ACCEL_GET_CLASS(current_machine->accelerator); > >>>>>> + > >>>>>> + if (ac->has_memory) { > >>>>>> + fvi.ac = ac; > >>>>>> + fvi.ac_name = current_machine->accel ? current_machine->accel : > >>>>>> + object_class_get_name(OBJECT_CLASS(ac)); > >>>>>> + } > >>>>>> > >>>>>> /* Gather all FVs in one table */ > >>>>>> QTAILQ_FOREACH(as, &address_spaces, address_spaces_link) { > >>>>>> > >> > >> -- > >> Alexey > > -- > > Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK > > > > -- > Alexey -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator 2019-02-07 11:49 ` Dr. David Alan Gilbert @ 2019-02-08 17:26 ` Paolo Bonzini 2019-02-11 4:56 ` Alexey Kardashevskiy 0 siblings, 1 reply; 19+ messages in thread From: Paolo Bonzini @ 2019-02-08 17:26 UTC (permalink / raw) To: Dr. David Alan Gilbert, Alexey Kardashevskiy Cc: Philippe Mathieu-Daudé, qemu-devel, Daniel Henrique Barboza On 07/02/19 12:49, Dr. David Alan Gilbert wrote: > //#define DEBUG_UNASSIGNED > @@ -2924,6 +2926,8 @@ struct FlatViewInfo { > int counter; > bool dispatch_tree; > bool owner; > + AccelClass *ac; > + const char *ac_name; > }; > > static void mtree_print_flatview(gpointer key, gpointer value, > @@ -2939,6 +2943,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, > int n = view->nr; > int i; > AddressSpace *as; > + bool system_as = false; > > p(f, "FlatView #%d\n", fvi->counter); > ++fvi->counter; > @@ -2950,6 +2955,9 @@ static void mtree_print_flatview(gpointer key, gpointer value, > p(f, ", alias %s", memory_region_name(as->root->alias)); > } > p(f, "\n"); > + if (as == &address_space_memory) { > + system_as = true; > + } > } > > p(f, " Root memory region: %s\n", > @@ -2985,6 +2993,13 @@ static void mtree_print_flatview(gpointer key, gpointer value, > if (fvi->owner) { > mtree_print_mr_owner(p, f, mr); > } > + > + if (system_as && fvi->ac && > + fvi->ac->has_memory(current_machine, > + int128_get64(range->addr.start), > + MR_SIZE(range->addr.size) + 1)) { > + p(f, " %s", fvi->ac_name); I don't understand this. This doesn't check that the memory range actually matches the memory registered with the accelerator, only that there is something in that range. Why isn't it enough to use "info mtree" and look at the KVM address space? Perhaps you could add instead an argument to "info mtree" that prints only the AddressSpace with a given name? Paolo ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator 2019-02-08 17:26 ` Paolo Bonzini @ 2019-02-11 4:56 ` Alexey Kardashevskiy [not found] ` <f346fdcb-1849-3397-7f4c-2d6ee07fcd7c@ozlabs.ru> 0 siblings, 1 reply; 19+ messages in thread From: Alexey Kardashevskiy @ 2019-02-11 4:56 UTC (permalink / raw) To: Paolo Bonzini, Dr. David Alan Gilbert Cc: Philippe Mathieu-Daudé, qemu-devel, Daniel Henrique Barboza On 09/02/2019 04:26, Paolo Bonzini wrote: > On 07/02/19 12:49, Dr. David Alan Gilbert wrote: >> //#define DEBUG_UNASSIGNED >> @@ -2924,6 +2926,8 @@ struct FlatViewInfo { >> int counter; >> bool dispatch_tree; >> bool owner; >> + AccelClass *ac; >> + const char *ac_name; >> }; >> >> static void mtree_print_flatview(gpointer key, gpointer value, >> @@ -2939,6 +2943,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, >> int n = view->nr; >> int i; >> AddressSpace *as; >> + bool system_as = false; >> >> p(f, "FlatView #%d\n", fvi->counter); >> ++fvi->counter; >> @@ -2950,6 +2955,9 @@ static void mtree_print_flatview(gpointer key, gpointer value, >> p(f, ", alias %s", memory_region_name(as->root->alias)); >> } >> p(f, "\n"); >> + if (as == &address_space_memory) { >> + system_as = true; >> + } >> } >> >> p(f, " Root memory region: %s\n", >> @@ -2985,6 +2993,13 @@ static void mtree_print_flatview(gpointer key, gpointer value, >> if (fvi->owner) { >> mtree_print_mr_owner(p, f, mr); >> } >> + >> + if (system_as && fvi->ac && >> + fvi->ac->has_memory(current_machine, >> + int128_get64(range->addr.start), >> + MR_SIZE(range->addr.size) + 1)) { >> + p(f, " %s", fvi->ac_name); > > I don't understand this. This doesn't check that the memory range > actually matches the memory registered with the accelerator, only that > there is something in that range. It is checking that a flat range (i.e. what actually works) has a corresponding KVM slot: https://git.qemu.org/?p=qemu.git;a=blob;f=accel/kvm/kvm-all.c;h=4e1de942ce554c734ac2673030031c228a752ac9;hb=HEAD#l201 > Why isn't it enough to use "info > mtree" and look at the KVM address space? There is no such thing in my QEMU, did you mean "KVM-SMRAM" (which is missing on spapr)? I am not sure I understand its purpose for the task - it prints all same ranges on my x86 laptop, not just ones which we told KVM about. My task is that if let's say "0000:00:1a.0 BAR 0 mmaps[0]" is split into several sections because MSIX happens to be in a middle of that BAR and it is not system page size aligned, then it is going to be several ranges with no clear indication whether or not these were registered as KVM slots. A memory chunk can be "ram" and not a KVM slot if it is 4K on PPC with 64K system pages, for example. FlatView #0 AS "memory", root: system AS "cpu-memory-0", root: system AS "cpu-memory-1", root: system AS "cpu-memory-2", root: system AS "cpu-memory-3", root: system AS "piix3-ide", root: bus master container AS "virtio-net-pci", root: bus master container AS "vfio-pci", root: bus master container Root memory region: system 0000000000000000-00000000000bffff (prio 0, ram): pc.ram kvm 00000000000c0000-00000000000c0fff (prio 0, rom): pc.ram @00000000000c0000 kvm 00000000000c1000-00000000000c3fff (prio 0, ram): pc.ram @00000000000c1000 kvm 00000000000c4000-00000000000e7fff (prio 0, rom): pc.ram @00000000000c4000 kvm 00000000000e8000-00000000000effff (prio 0, ram): pc.ram @00000000000e8000 kvm 00000000000f0000-00000000000fffff (prio 0, rom): pc.ram @00000000000f0000 kvm 0000000000100000-00000000bfffffff (prio 0, ram): pc.ram @0000000000100000 kvm 00000000febc0000-00000000febc0fff (prio 0, ramd): 0000:00:1a.0 BAR 0 mmaps[0] kvm 00000000febc1000-00000000febc102f (prio 0, i/o): msix-table 00000000febc1800-00000000febc1807 (prio 0, i/o): msix-pba 00000000febfc000-00000000febfcfff (prio 0, i/o): virtio-pci-common 00000000febfd000-00000000febfdfff (prio 0, i/o): virtio-pci-isr 00000000febfe000-00000000febfefff (prio 0, i/o): virtio-pci-device 00000000febff000-00000000febfffff (prio 0, i/o): virtio-pci-notify 00000000fec00000-00000000fec00fff (prio 0, i/o): kvm-ioapic 00000000fed00000-00000000fed003ff (prio 0, i/o): hpet 00000000fee00000-00000000feefffff (prio 4096, i/o): kvm-apic-msi 00000000fffc0000-00000000ffffffff (prio 0, rom): pc.bios kvm 0000000100000000-000000013fffffff (prio 0, ram): pc.ram @00000000c0000000 kvm FlatView #3 AS "KVM-SMRAM", root: mem-container-smram Root memory region: mem-container-smram 0000000000000000-00000000000bffff (prio 0, ram): pc.ram 00000000000c0000-00000000000c0fff (prio 0, rom): pc.ram @00000000000c0000 00000000000c1000-00000000000c3fff (prio 0, ram): pc.ram @00000000000c1000 00000000000c4000-00000000000e7fff (prio 0, rom): pc.ram @00000000000c4000 00000000000e8000-00000000000effff (prio 0, ram): pc.ram @00000000000e8000 00000000000f0000-00000000000fffff (prio 0, rom): pc.ram @00000000000f0000 0000000000100000-00000000bfffffff (prio 0, ram): pc.ram @0000000000100000 00000000febc0000-00000000febc0fff (prio 0, ramd): 0000:00:1a.0 BAR 0 mmaps[0] 00000000febc1000-00000000febc102f (prio 0, i/o): msix-table 00000000febc1800-00000000febc1807 (prio 0, i/o): msix-pba 00000000febfc000-00000000febfcfff (prio 0, i/o): virtio-pci-common 00000000febfd000-00000000febfdfff (prio 0, i/o): virtio-pci-isr 00000000febfe000-00000000febfefff (prio 0, i/o): virtio-pci-device 00000000febff000-00000000febfffff (prio 0, i/o): virtio-pci-notify 00000000fec00000-00000000fec00fff (prio 0, i/o): kvm-ioapic 00000000fed00000-00000000fed003ff (prio 0, i/o): hpet 00000000fee00000-00000000feefffff (prio 4096, i/o): kvm-apic-msi 00000000fffc0000-00000000ffffffff (prio 0, rom): pc.bios 0000000100000000-000000013fffffff (prio 0, ram): pc.ram @00000000c0000000 > Perhaps you could add instead an argument to "info mtree" that prints > only the AddressSpace with a given name? Nah, this is not what it is about. -- Alexey ^ permalink raw reply [flat|nested] 19+ messages in thread
[parent not found: <f346fdcb-1849-3397-7f4c-2d6ee07fcd7c@ozlabs.ru>]
* Re: [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator @ 2019-04-24 5:32 ` Alexey Kardashevskiy 0 siblings, 0 replies; 19+ messages in thread From: Alexey Kardashevskiy @ 2019-04-24 5:32 UTC (permalink / raw) To: Paolo Bonzini Cc: Dr. David Alan Gilbert, Philippe Mathieu-Daudé, qemu-devel, Daniel Henrique Barboza Paolo, ping? On 19/03/2019 18:05, Alexey Kardashevskiy wrote: > > > On 11/02/2019 15:56, Alexey Kardashevskiy wrote: >> >> >> On 09/02/2019 04:26, Paolo Bonzini wrote: >>> On 07/02/19 12:49, Dr. David Alan Gilbert wrote: >>>> //#define DEBUG_UNASSIGNED >>>> @@ -2924,6 +2926,8 @@ struct FlatViewInfo { >>>> int counter; >>>> bool dispatch_tree; >>>> bool owner; >>>> + AccelClass *ac; >>>> + const char *ac_name; >>>> }; >>>> >>>> static void mtree_print_flatview(gpointer key, gpointer value, >>>> @@ -2939,6 +2943,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>> int n = view->nr; >>>> int i; >>>> AddressSpace *as; >>>> + bool system_as = false; >>>> >>>> p(f, "FlatView #%d\n", fvi->counter); >>>> ++fvi->counter; >>>> @@ -2950,6 +2955,9 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>> p(f, ", alias %s", memory_region_name(as->root->alias)); >>>> } >>>> p(f, "\n"); >>>> + if (as == &address_space_memory) { >>>> + system_as = true; >>>> + } >>>> } >>>> >>>> p(f, " Root memory region: %s\n", >>>> @@ -2985,6 +2993,13 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>> if (fvi->owner) { >>>> mtree_print_mr_owner(p, f, mr); >>>> } >>>> + >>>> + if (system_as && fvi->ac && >>>> + fvi->ac->has_memory(current_machine, >>>> + int128_get64(range->addr.start), >>>> + MR_SIZE(range->addr.size) + 1)) { >>>> + p(f, " %s", fvi->ac_name); >>> >>> I don't understand this. This doesn't check that the memory range >>> actually matches the memory registered with the accelerator, only that >>> there is something in that range. >> >> >> It is checking that a flat range (i.e. what actually works) has a >> corresponding KVM slot: >> https://git.qemu.org/?p=qemu.git;a=blob;f=accel/kvm/kvm-all.c;h=4e1de942ce554c734ac2673030031c228a752ac9;hb=HEAD#l201 >> >> >>> Why isn't it enough to use "info >>> mtree" and look at the KVM address space? >> >> >> There is no such thing in my QEMU, did you mean "KVM-SMRAM" (which is >> missing on spapr)? I am not sure I understand its purpose for the task - >> it prints all same ranges on my x86 laptop, not just ones which we told >> KVM about. >> >> My task is that if let's say "0000:00:1a.0 BAR 0 mmaps[0]" is split into >> several sections because MSIX happens to be in a middle of that BAR and >> it is not system page size aligned, then it is going to be several >> ranges with no clear indication whether or not these were registered as >> KVM slots. A memory chunk can be "ram" and not a KVM slot if it is 4K on >> PPC with 64K system pages, for example. >> >> >> FlatView #0 >> AS "memory", root: system >> AS "cpu-memory-0", root: system >> AS "cpu-memory-1", root: system >> AS "cpu-memory-2", root: system >> AS "cpu-memory-3", root: system >> AS "piix3-ide", root: bus master container >> AS "virtio-net-pci", root: bus master container >> AS "vfio-pci", root: bus master container >> Root memory region: system >> 0000000000000000-00000000000bffff (prio 0, ram): pc.ram kvm >> 00000000000c0000-00000000000c0fff (prio 0, rom): pc.ram >> @00000000000c0000 kvm >> 00000000000c1000-00000000000c3fff (prio 0, ram): pc.ram >> @00000000000c1000 kvm >> 00000000000c4000-00000000000e7fff (prio 0, rom): pc.ram >> @00000000000c4000 kvm >> 00000000000e8000-00000000000effff (prio 0, ram): pc.ram >> @00000000000e8000 kvm >> 00000000000f0000-00000000000fffff (prio 0, rom): pc.ram >> @00000000000f0000 kvm >> 0000000000100000-00000000bfffffff (prio 0, ram): pc.ram >> @0000000000100000 kvm >> 00000000febc0000-00000000febc0fff (prio 0, ramd): 0000:00:1a.0 BAR 0 >> mmaps[0] kvm >> 00000000febc1000-00000000febc102f (prio 0, i/o): msix-table >> 00000000febc1800-00000000febc1807 (prio 0, i/o): msix-pba >> 00000000febfc000-00000000febfcfff (prio 0, i/o): virtio-pci-common >> 00000000febfd000-00000000febfdfff (prio 0, i/o): virtio-pci-isr >> 00000000febfe000-00000000febfefff (prio 0, i/o): virtio-pci-device >> 00000000febff000-00000000febfffff (prio 0, i/o): virtio-pci-notify >> 00000000fec00000-00000000fec00fff (prio 0, i/o): kvm-ioapic >> 00000000fed00000-00000000fed003ff (prio 0, i/o): hpet >> 00000000fee00000-00000000feefffff (prio 4096, i/o): kvm-apic-msi >> 00000000fffc0000-00000000ffffffff (prio 0, rom): pc.bios kvm >> 0000000100000000-000000013fffffff (prio 0, ram): pc.ram >> @00000000c0000000 kvm >> >> >> >> FlatView #3 >> AS "KVM-SMRAM", root: mem-container-smram >> Root memory region: mem-container-smram >> 0000000000000000-00000000000bffff (prio 0, ram): pc.ram >> 00000000000c0000-00000000000c0fff (prio 0, rom): pc.ram @00000000000c0000 >> 00000000000c1000-00000000000c3fff (prio 0, ram): pc.ram @00000000000c1000 >> 00000000000c4000-00000000000e7fff (prio 0, rom): pc.ram @00000000000c4000 >> 00000000000e8000-00000000000effff (prio 0, ram): pc.ram @00000000000e8000 >> 00000000000f0000-00000000000fffff (prio 0, rom): pc.ram @00000000000f0000 >> 0000000000100000-00000000bfffffff (prio 0, ram): pc.ram @0000000000100000 >> 00000000febc0000-00000000febc0fff (prio 0, ramd): 0000:00:1a.0 BAR 0 >> mmaps[0] >> 00000000febc1000-00000000febc102f (prio 0, i/o): msix-table >> 00000000febc1800-00000000febc1807 (prio 0, i/o): msix-pba >> 00000000febfc000-00000000febfcfff (prio 0, i/o): virtio-pci-common >> 00000000febfd000-00000000febfdfff (prio 0, i/o): virtio-pci-isr >> 00000000febfe000-00000000febfefff (prio 0, i/o): virtio-pci-device >> 00000000febff000-00000000febfffff (prio 0, i/o): virtio-pci-notify >> 00000000fec00000-00000000fec00fff (prio 0, i/o): kvm-ioapic >> 00000000fed00000-00000000fed003ff (prio 0, i/o): hpet >> 00000000fee00000-00000000feefffff (prio 4096, i/o): kvm-apic-msi >> 00000000fffc0000-00000000ffffffff (prio 0, rom): pc.bios >> 0000000100000000-000000013fffffff (prio 0, ram): pc.ram @00000000c0000000 >> >> >> >> >>> Perhaps you could add instead an argument to "info mtree" that prints >>> only the AddressSpace with a given name? >> >> Nah, this is not what it is about. > > > > Still a nack? :) -- Alexey ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator @ 2019-04-24 5:32 ` Alexey Kardashevskiy 0 siblings, 0 replies; 19+ messages in thread From: Alexey Kardashevskiy @ 2019-04-24 5:32 UTC (permalink / raw) To: Paolo Bonzini Cc: Daniel Henrique Barboza, Philippe Mathieu-Daudé, Dr. David Alan Gilbert, qemu-devel Paolo, ping? On 19/03/2019 18:05, Alexey Kardashevskiy wrote: > > > On 11/02/2019 15:56, Alexey Kardashevskiy wrote: >> >> >> On 09/02/2019 04:26, Paolo Bonzini wrote: >>> On 07/02/19 12:49, Dr. David Alan Gilbert wrote: >>>> //#define DEBUG_UNASSIGNED >>>> @@ -2924,6 +2926,8 @@ struct FlatViewInfo { >>>> int counter; >>>> bool dispatch_tree; >>>> bool owner; >>>> + AccelClass *ac; >>>> + const char *ac_name; >>>> }; >>>> >>>> static void mtree_print_flatview(gpointer key, gpointer value, >>>> @@ -2939,6 +2943,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>> int n = view->nr; >>>> int i; >>>> AddressSpace *as; >>>> + bool system_as = false; >>>> >>>> p(f, "FlatView #%d\n", fvi->counter); >>>> ++fvi->counter; >>>> @@ -2950,6 +2955,9 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>> p(f, ", alias %s", memory_region_name(as->root->alias)); >>>> } >>>> p(f, "\n"); >>>> + if (as == &address_space_memory) { >>>> + system_as = true; >>>> + } >>>> } >>>> >>>> p(f, " Root memory region: %s\n", >>>> @@ -2985,6 +2993,13 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>> if (fvi->owner) { >>>> mtree_print_mr_owner(p, f, mr); >>>> } >>>> + >>>> + if (system_as && fvi->ac && >>>> + fvi->ac->has_memory(current_machine, >>>> + int128_get64(range->addr.start), >>>> + MR_SIZE(range->addr.size) + 1)) { >>>> + p(f, " %s", fvi->ac_name); >>> >>> I don't understand this. This doesn't check that the memory range >>> actually matches the memory registered with the accelerator, only that >>> there is something in that range. >> >> >> It is checking that a flat range (i.e. what actually works) has a >> corresponding KVM slot: >> https://git.qemu.org/?p=qemu.git;a=blob;f=accel/kvm/kvm-all.c;h=4e1de942ce554c734ac2673030031c228a752ac9;hb=HEAD#l201 >> >> >>> Why isn't it enough to use "info >>> mtree" and look at the KVM address space? >> >> >> There is no such thing in my QEMU, did you mean "KVM-SMRAM" (which is >> missing on spapr)? I am not sure I understand its purpose for the task - >> it prints all same ranges on my x86 laptop, not just ones which we told >> KVM about. >> >> My task is that if let's say "0000:00:1a.0 BAR 0 mmaps[0]" is split into >> several sections because MSIX happens to be in a middle of that BAR and >> it is not system page size aligned, then it is going to be several >> ranges with no clear indication whether or not these were registered as >> KVM slots. A memory chunk can be "ram" and not a KVM slot if it is 4K on >> PPC with 64K system pages, for example. >> >> >> FlatView #0 >> AS "memory", root: system >> AS "cpu-memory-0", root: system >> AS "cpu-memory-1", root: system >> AS "cpu-memory-2", root: system >> AS "cpu-memory-3", root: system >> AS "piix3-ide", root: bus master container >> AS "virtio-net-pci", root: bus master container >> AS "vfio-pci", root: bus master container >> Root memory region: system >> 0000000000000000-00000000000bffff (prio 0, ram): pc.ram kvm >> 00000000000c0000-00000000000c0fff (prio 0, rom): pc.ram >> @00000000000c0000 kvm >> 00000000000c1000-00000000000c3fff (prio 0, ram): pc.ram >> @00000000000c1000 kvm >> 00000000000c4000-00000000000e7fff (prio 0, rom): pc.ram >> @00000000000c4000 kvm >> 00000000000e8000-00000000000effff (prio 0, ram): pc.ram >> @00000000000e8000 kvm >> 00000000000f0000-00000000000fffff (prio 0, rom): pc.ram >> @00000000000f0000 kvm >> 0000000000100000-00000000bfffffff (prio 0, ram): pc.ram >> @0000000000100000 kvm >> 00000000febc0000-00000000febc0fff (prio 0, ramd): 0000:00:1a.0 BAR 0 >> mmaps[0] kvm >> 00000000febc1000-00000000febc102f (prio 0, i/o): msix-table >> 00000000febc1800-00000000febc1807 (prio 0, i/o): msix-pba >> 00000000febfc000-00000000febfcfff (prio 0, i/o): virtio-pci-common >> 00000000febfd000-00000000febfdfff (prio 0, i/o): virtio-pci-isr >> 00000000febfe000-00000000febfefff (prio 0, i/o): virtio-pci-device >> 00000000febff000-00000000febfffff (prio 0, i/o): virtio-pci-notify >> 00000000fec00000-00000000fec00fff (prio 0, i/o): kvm-ioapic >> 00000000fed00000-00000000fed003ff (prio 0, i/o): hpet >> 00000000fee00000-00000000feefffff (prio 4096, i/o): kvm-apic-msi >> 00000000fffc0000-00000000ffffffff (prio 0, rom): pc.bios kvm >> 0000000100000000-000000013fffffff (prio 0, ram): pc.ram >> @00000000c0000000 kvm >> >> >> >> FlatView #3 >> AS "KVM-SMRAM", root: mem-container-smram >> Root memory region: mem-container-smram >> 0000000000000000-00000000000bffff (prio 0, ram): pc.ram >> 00000000000c0000-00000000000c0fff (prio 0, rom): pc.ram @00000000000c0000 >> 00000000000c1000-00000000000c3fff (prio 0, ram): pc.ram @00000000000c1000 >> 00000000000c4000-00000000000e7fff (prio 0, rom): pc.ram @00000000000c4000 >> 00000000000e8000-00000000000effff (prio 0, ram): pc.ram @00000000000e8000 >> 00000000000f0000-00000000000fffff (prio 0, rom): pc.ram @00000000000f0000 >> 0000000000100000-00000000bfffffff (prio 0, ram): pc.ram @0000000000100000 >> 00000000febc0000-00000000febc0fff (prio 0, ramd): 0000:00:1a.0 BAR 0 >> mmaps[0] >> 00000000febc1000-00000000febc102f (prio 0, i/o): msix-table >> 00000000febc1800-00000000febc1807 (prio 0, i/o): msix-pba >> 00000000febfc000-00000000febfcfff (prio 0, i/o): virtio-pci-common >> 00000000febfd000-00000000febfdfff (prio 0, i/o): virtio-pci-isr >> 00000000febfe000-00000000febfefff (prio 0, i/o): virtio-pci-device >> 00000000febff000-00000000febfffff (prio 0, i/o): virtio-pci-notify >> 00000000fec00000-00000000fec00fff (prio 0, i/o): kvm-ioapic >> 00000000fed00000-00000000fed003ff (prio 0, i/o): hpet >> 00000000fee00000-00000000feefffff (prio 4096, i/o): kvm-apic-msi >> 00000000fffc0000-00000000ffffffff (prio 0, rom): pc.bios >> 0000000100000000-000000013fffffff (prio 0, ram): pc.ram @00000000c0000000 >> >> >> >> >>> Perhaps you could add instead an argument to "info mtree" that prints >>> only the AddressSpace with a given name? >> >> Nah, this is not what it is about. > > > > Still a nack? :) -- Alexey ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator 2019-04-24 5:32 ` Alexey Kardashevskiy (?) @ 2019-05-21 6:37 ` Alexey Kardashevskiy 2019-06-13 5:07 ` Alexey Kardashevskiy -1 siblings, 1 reply; 19+ messages in thread From: Alexey Kardashevskiy @ 2019-05-21 6:37 UTC (permalink / raw) To: Paolo Bonzini Cc: Daniel Henrique Barboza, Philippe Mathieu-Daudé, Dr. David Alan Gilbert, qemu-devel Ping, anyone? I still enjoy seeing "kvm" next to MRs in "info mtree -f" in my local QEMU :) On 24/04/2019 15:32, Alexey Kardashevskiy wrote: > Paolo, ping? > > > On 19/03/2019 18:05, Alexey Kardashevskiy wrote: >> >> >> On 11/02/2019 15:56, Alexey Kardashevskiy wrote: >>> >>> >>> On 09/02/2019 04:26, Paolo Bonzini wrote: >>>> On 07/02/19 12:49, Dr. David Alan Gilbert wrote: >>>>> //#define DEBUG_UNASSIGNED >>>>> @@ -2924,6 +2926,8 @@ struct FlatViewInfo { >>>>> int counter; >>>>> bool dispatch_tree; >>>>> bool owner; >>>>> + AccelClass *ac; >>>>> + const char *ac_name; >>>>> }; >>>>> >>>>> static void mtree_print_flatview(gpointer key, gpointer value, >>>>> @@ -2939,6 +2943,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>>> int n = view->nr; >>>>> int i; >>>>> AddressSpace *as; >>>>> + bool system_as = false; >>>>> >>>>> p(f, "FlatView #%d\n", fvi->counter); >>>>> ++fvi->counter; >>>>> @@ -2950,6 +2955,9 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>>> p(f, ", alias %s", memory_region_name(as->root->alias)); >>>>> } >>>>> p(f, "\n"); >>>>> + if (as == &address_space_memory) { >>>>> + system_as = true; >>>>> + } >>>>> } >>>>> >>>>> p(f, " Root memory region: %s\n", >>>>> @@ -2985,6 +2993,13 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>>> if (fvi->owner) { >>>>> mtree_print_mr_owner(p, f, mr); >>>>> } >>>>> + >>>>> + if (system_as && fvi->ac && >>>>> + fvi->ac->has_memory(current_machine, >>>>> + int128_get64(range->addr.start), >>>>> + MR_SIZE(range->addr.size) + 1)) { >>>>> + p(f, " %s", fvi->ac_name); >>>> >>>> I don't understand this. This doesn't check that the memory range >>>> actually matches the memory registered with the accelerator, only that >>>> there is something in that range. >>> >>> >>> It is checking that a flat range (i.e. what actually works) has a >>> corresponding KVM slot: >>> https://git.qemu.org/?p=qemu.git;a=blob;f=accel/kvm/kvm-all.c;h=4e1de942ce554c734ac2673030031c228a752ac9;hb=HEAD#l201 >>> >>> >>>> Why isn't it enough to use "info >>>> mtree" and look at the KVM address space? >>> >>> >>> There is no such thing in my QEMU, did you mean "KVM-SMRAM" (which is >>> missing on spapr)? I am not sure I understand its purpose for the task - >>> it prints all same ranges on my x86 laptop, not just ones which we told >>> KVM about. >>> >>> My task is that if let's say "0000:00:1a.0 BAR 0 mmaps[0]" is split into >>> several sections because MSIX happens to be in a middle of that BAR and >>> it is not system page size aligned, then it is going to be several >>> ranges with no clear indication whether or not these were registered as >>> KVM slots. A memory chunk can be "ram" and not a KVM slot if it is 4K on >>> PPC with 64K system pages, for example. >>> >>> >>> FlatView #0 >>> AS "memory", root: system >>> AS "cpu-memory-0", root: system >>> AS "cpu-memory-1", root: system >>> AS "cpu-memory-2", root: system >>> AS "cpu-memory-3", root: system >>> AS "piix3-ide", root: bus master container >>> AS "virtio-net-pci", root: bus master container >>> AS "vfio-pci", root: bus master container >>> Root memory region: system >>> 0000000000000000-00000000000bffff (prio 0, ram): pc.ram kvm >>> 00000000000c0000-00000000000c0fff (prio 0, rom): pc.ram >>> @00000000000c0000 kvm >>> 00000000000c1000-00000000000c3fff (prio 0, ram): pc.ram >>> @00000000000c1000 kvm >>> 00000000000c4000-00000000000e7fff (prio 0, rom): pc.ram >>> @00000000000c4000 kvm >>> 00000000000e8000-00000000000effff (prio 0, ram): pc.ram >>> @00000000000e8000 kvm >>> 00000000000f0000-00000000000fffff (prio 0, rom): pc.ram >>> @00000000000f0000 kvm >>> 0000000000100000-00000000bfffffff (prio 0, ram): pc.ram >>> @0000000000100000 kvm >>> 00000000febc0000-00000000febc0fff (prio 0, ramd): 0000:00:1a.0 BAR 0 >>> mmaps[0] kvm >>> 00000000febc1000-00000000febc102f (prio 0, i/o): msix-table >>> 00000000febc1800-00000000febc1807 (prio 0, i/o): msix-pba >>> 00000000febfc000-00000000febfcfff (prio 0, i/o): virtio-pci-common >>> 00000000febfd000-00000000febfdfff (prio 0, i/o): virtio-pci-isr >>> 00000000febfe000-00000000febfefff (prio 0, i/o): virtio-pci-device >>> 00000000febff000-00000000febfffff (prio 0, i/o): virtio-pci-notify >>> 00000000fec00000-00000000fec00fff (prio 0, i/o): kvm-ioapic >>> 00000000fed00000-00000000fed003ff (prio 0, i/o): hpet >>> 00000000fee00000-00000000feefffff (prio 4096, i/o): kvm-apic-msi >>> 00000000fffc0000-00000000ffffffff (prio 0, rom): pc.bios kvm >>> 0000000100000000-000000013fffffff (prio 0, ram): pc.ram >>> @00000000c0000000 kvm >>> >>> >>> >>> FlatView #3 >>> AS "KVM-SMRAM", root: mem-container-smram >>> Root memory region: mem-container-smram >>> 0000000000000000-00000000000bffff (prio 0, ram): pc.ram >>> 00000000000c0000-00000000000c0fff (prio 0, rom): pc.ram @00000000000c0000 >>> 00000000000c1000-00000000000c3fff (prio 0, ram): pc.ram @00000000000c1000 >>> 00000000000c4000-00000000000e7fff (prio 0, rom): pc.ram @00000000000c4000 >>> 00000000000e8000-00000000000effff (prio 0, ram): pc.ram @00000000000e8000 >>> 00000000000f0000-00000000000fffff (prio 0, rom): pc.ram @00000000000f0000 >>> 0000000000100000-00000000bfffffff (prio 0, ram): pc.ram @0000000000100000 >>> 00000000febc0000-00000000febc0fff (prio 0, ramd): 0000:00:1a.0 BAR 0 >>> mmaps[0] >>> 00000000febc1000-00000000febc102f (prio 0, i/o): msix-table >>> 00000000febc1800-00000000febc1807 (prio 0, i/o): msix-pba >>> 00000000febfc000-00000000febfcfff (prio 0, i/o): virtio-pci-common >>> 00000000febfd000-00000000febfdfff (prio 0, i/o): virtio-pci-isr >>> 00000000febfe000-00000000febfefff (prio 0, i/o): virtio-pci-device >>> 00000000febff000-00000000febfffff (prio 0, i/o): virtio-pci-notify >>> 00000000fec00000-00000000fec00fff (prio 0, i/o): kvm-ioapic >>> 00000000fed00000-00000000fed003ff (prio 0, i/o): hpet >>> 00000000fee00000-00000000feefffff (prio 4096, i/o): kvm-apic-msi >>> 00000000fffc0000-00000000ffffffff (prio 0, rom): pc.bios >>> 0000000100000000-000000013fffffff (prio 0, ram): pc.ram @00000000c0000000 >>> >>> >>> >>> >>>> Perhaps you could add instead an argument to "info mtree" that prints >>>> only the AddressSpace with a given name? >>> >>> Nah, this is not what it is about. >> >> >> >> Still a nack? :) > > > -- Alexey ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator 2019-05-21 6:37 ` Alexey Kardashevskiy @ 2019-06-13 5:07 ` Alexey Kardashevskiy 2019-06-13 11:49 ` Dr. David Alan Gilbert ` (2 more replies) 0 siblings, 3 replies; 19+ messages in thread From: Alexey Kardashevskiy @ 2019-06-13 5:07 UTC (permalink / raw) To: Paolo Bonzini Cc: Daniel Henrique Barboza, Philippe Mathieu-Daudé, Dr. David Alan Gilbert, qemu-devel On 21/05/2019 16:37, Alexey Kardashevskiy wrote: > Ping, anyone? I still enjoy seeing "kvm" next to MRs in "info mtree -f" > in my local QEMU :) "Still feeling it" :) Ping? > > > On 24/04/2019 15:32, Alexey Kardashevskiy wrote: >> Paolo, ping? >> >> >> On 19/03/2019 18:05, Alexey Kardashevskiy wrote: >>> >>> >>> On 11/02/2019 15:56, Alexey Kardashevskiy wrote: >>>> >>>> >>>> On 09/02/2019 04:26, Paolo Bonzini wrote: >>>>> On 07/02/19 12:49, Dr. David Alan Gilbert wrote: >>>>>> //#define DEBUG_UNASSIGNED >>>>>> @@ -2924,6 +2926,8 @@ struct FlatViewInfo { >>>>>> int counter; >>>>>> bool dispatch_tree; >>>>>> bool owner; >>>>>> + AccelClass *ac; >>>>>> + const char *ac_name; >>>>>> }; >>>>>> >>>>>> static void mtree_print_flatview(gpointer key, gpointer value, >>>>>> @@ -2939,6 +2943,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>>>> int n = view->nr; >>>>>> int i; >>>>>> AddressSpace *as; >>>>>> + bool system_as = false; >>>>>> >>>>>> p(f, "FlatView #%d\n", fvi->counter); >>>>>> ++fvi->counter; >>>>>> @@ -2950,6 +2955,9 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>>>> p(f, ", alias %s", memory_region_name(as->root->alias)); >>>>>> } >>>>>> p(f, "\n"); >>>>>> + if (as == &address_space_memory) { >>>>>> + system_as = true; >>>>>> + } >>>>>> } >>>>>> >>>>>> p(f, " Root memory region: %s\n", >>>>>> @@ -2985,6 +2993,13 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>>>> if (fvi->owner) { >>>>>> mtree_print_mr_owner(p, f, mr); >>>>>> } >>>>>> + >>>>>> + if (system_as && fvi->ac && >>>>>> + fvi->ac->has_memory(current_machine, >>>>>> + int128_get64(range->addr.start), >>>>>> + MR_SIZE(range->addr.size) + 1)) { >>>>>> + p(f, " %s", fvi->ac_name); >>>>> >>>>> I don't understand this. This doesn't check that the memory range >>>>> actually matches the memory registered with the accelerator, only that >>>>> there is something in that range. >>>> >>>> >>>> It is checking that a flat range (i.e. what actually works) has a >>>> corresponding KVM slot: >>>> https://git.qemu.org/?p=qemu.git;a=blob;f=accel/kvm/kvm-all.c;h=4e1de942ce554c734ac2673030031c228a752ac9;hb=HEAD#l201 >>>> >>>> >>>>> Why isn't it enough to use "info >>>>> mtree" and look at the KVM address space? >>>> >>>> >>>> There is no such thing in my QEMU, did you mean "KVM-SMRAM" (which is >>>> missing on spapr)? I am not sure I understand its purpose for the task - >>>> it prints all same ranges on my x86 laptop, not just ones which we told >>>> KVM about. >>>> >>>> My task is that if let's say "0000:00:1a.0 BAR 0 mmaps[0]" is split into >>>> several sections because MSIX happens to be in a middle of that BAR and >>>> it is not system page size aligned, then it is going to be several >>>> ranges with no clear indication whether or not these were registered as >>>> KVM slots. A memory chunk can be "ram" and not a KVM slot if it is 4K on >>>> PPC with 64K system pages, for example. >>>> >>>> >>>> FlatView #0 >>>> AS "memory", root: system >>>> AS "cpu-memory-0", root: system >>>> AS "cpu-memory-1", root: system >>>> AS "cpu-memory-2", root: system >>>> AS "cpu-memory-3", root: system >>>> AS "piix3-ide", root: bus master container >>>> AS "virtio-net-pci", root: bus master container >>>> AS "vfio-pci", root: bus master container >>>> Root memory region: system >>>> 0000000000000000-00000000000bffff (prio 0, ram): pc.ram kvm >>>> 00000000000c0000-00000000000c0fff (prio 0, rom): pc.ram >>>> @00000000000c0000 kvm >>>> 00000000000c1000-00000000000c3fff (prio 0, ram): pc.ram >>>> @00000000000c1000 kvm >>>> 00000000000c4000-00000000000e7fff (prio 0, rom): pc.ram >>>> @00000000000c4000 kvm >>>> 00000000000e8000-00000000000effff (prio 0, ram): pc.ram >>>> @00000000000e8000 kvm >>>> 00000000000f0000-00000000000fffff (prio 0, rom): pc.ram >>>> @00000000000f0000 kvm >>>> 0000000000100000-00000000bfffffff (prio 0, ram): pc.ram >>>> @0000000000100000 kvm >>>> 00000000febc0000-00000000febc0fff (prio 0, ramd): 0000:00:1a.0 BAR 0 >>>> mmaps[0] kvm >>>> 00000000febc1000-00000000febc102f (prio 0, i/o): msix-table >>>> 00000000febc1800-00000000febc1807 (prio 0, i/o): msix-pba >>>> 00000000febfc000-00000000febfcfff (prio 0, i/o): virtio-pci-common >>>> 00000000febfd000-00000000febfdfff (prio 0, i/o): virtio-pci-isr >>>> 00000000febfe000-00000000febfefff (prio 0, i/o): virtio-pci-device >>>> 00000000febff000-00000000febfffff (prio 0, i/o): virtio-pci-notify >>>> 00000000fec00000-00000000fec00fff (prio 0, i/o): kvm-ioapic >>>> 00000000fed00000-00000000fed003ff (prio 0, i/o): hpet >>>> 00000000fee00000-00000000feefffff (prio 4096, i/o): kvm-apic-msi >>>> 00000000fffc0000-00000000ffffffff (prio 0, rom): pc.bios kvm >>>> 0000000100000000-000000013fffffff (prio 0, ram): pc.ram >>>> @00000000c0000000 kvm >>>> >>>> >>>> >>>> FlatView #3 >>>> AS "KVM-SMRAM", root: mem-container-smram >>>> Root memory region: mem-container-smram >>>> 0000000000000000-00000000000bffff (prio 0, ram): pc.ram >>>> 00000000000c0000-00000000000c0fff (prio 0, rom): pc.ram @00000000000c0000 >>>> 00000000000c1000-00000000000c3fff (prio 0, ram): pc.ram @00000000000c1000 >>>> 00000000000c4000-00000000000e7fff (prio 0, rom): pc.ram @00000000000c4000 >>>> 00000000000e8000-00000000000effff (prio 0, ram): pc.ram @00000000000e8000 >>>> 00000000000f0000-00000000000fffff (prio 0, rom): pc.ram @00000000000f0000 >>>> 0000000000100000-00000000bfffffff (prio 0, ram): pc.ram @0000000000100000 >>>> 00000000febc0000-00000000febc0fff (prio 0, ramd): 0000:00:1a.0 BAR 0 >>>> mmaps[0] >>>> 00000000febc1000-00000000febc102f (prio 0, i/o): msix-table >>>> 00000000febc1800-00000000febc1807 (prio 0, i/o): msix-pba >>>> 00000000febfc000-00000000febfcfff (prio 0, i/o): virtio-pci-common >>>> 00000000febfd000-00000000febfdfff (prio 0, i/o): virtio-pci-isr >>>> 00000000febfe000-00000000febfefff (prio 0, i/o): virtio-pci-device >>>> 00000000febff000-00000000febfffff (prio 0, i/o): virtio-pci-notify >>>> 00000000fec00000-00000000fec00fff (prio 0, i/o): kvm-ioapic >>>> 00000000fed00000-00000000fed003ff (prio 0, i/o): hpet >>>> 00000000fee00000-00000000feefffff (prio 4096, i/o): kvm-apic-msi >>>> 00000000fffc0000-00000000ffffffff (prio 0, rom): pc.bios >>>> 0000000100000000-000000013fffffff (prio 0, ram): pc.ram @00000000c0000000 >>>> >>>> >>>> >>>> >>>>> Perhaps you could add instead an argument to "info mtree" that prints >>>>> only the AddressSpace with a given name? >>>> >>>> Nah, this is not what it is about. >>> >>> >>> >>> Still a nack? :) >> >> >> > -- Alexey ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator 2019-06-13 5:07 ` Alexey Kardashevskiy @ 2019-06-13 11:49 ` Dr. David Alan Gilbert 2019-06-13 13:59 ` Paolo Bonzini 2019-06-13 14:04 ` Paolo Bonzini 2 siblings, 0 replies; 19+ messages in thread From: Dr. David Alan Gilbert @ 2019-06-13 11:49 UTC (permalink / raw) To: Alexey Kardashevskiy Cc: Paolo Bonzini, Daniel Henrique Barboza, Philippe Mathieu-Daudé, qemu-devel * Alexey Kardashevskiy (aik@ozlabs.ru) wrote: > > > On 21/05/2019 16:37, Alexey Kardashevskiy wrote: > > Ping, anyone? I still enjoy seeing "kvm" next to MRs in "info mtree -f" > > in my local QEMU :) > > > "Still feeling it" :) > Ping? Fundamentally you've got to convince Paolo that it's the right approach to identifying KVM RAM. Dave > > > > > > > On 24/04/2019 15:32, Alexey Kardashevskiy wrote: > >> Paolo, ping? > >> > >> > >> On 19/03/2019 18:05, Alexey Kardashevskiy wrote: > >>> > >>> > >>> On 11/02/2019 15:56, Alexey Kardashevskiy wrote: > >>>> > >>>> > >>>> On 09/02/2019 04:26, Paolo Bonzini wrote: > >>>>> On 07/02/19 12:49, Dr. David Alan Gilbert wrote: > >>>>>> //#define DEBUG_UNASSIGNED > >>>>>> @@ -2924,6 +2926,8 @@ struct FlatViewInfo { > >>>>>> int counter; > >>>>>> bool dispatch_tree; > >>>>>> bool owner; > >>>>>> + AccelClass *ac; > >>>>>> + const char *ac_name; > >>>>>> }; > >>>>>> > >>>>>> static void mtree_print_flatview(gpointer key, gpointer value, > >>>>>> @@ -2939,6 +2943,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, > >>>>>> int n = view->nr; > >>>>>> int i; > >>>>>> AddressSpace *as; > >>>>>> + bool system_as = false; > >>>>>> > >>>>>> p(f, "FlatView #%d\n", fvi->counter); > >>>>>> ++fvi->counter; > >>>>>> @@ -2950,6 +2955,9 @@ static void mtree_print_flatview(gpointer key, gpointer value, > >>>>>> p(f, ", alias %s", memory_region_name(as->root->alias)); > >>>>>> } > >>>>>> p(f, "\n"); > >>>>>> + if (as == &address_space_memory) { > >>>>>> + system_as = true; > >>>>>> + } > >>>>>> } > >>>>>> > >>>>>> p(f, " Root memory region: %s\n", > >>>>>> @@ -2985,6 +2993,13 @@ static void mtree_print_flatview(gpointer key, gpointer value, > >>>>>> if (fvi->owner) { > >>>>>> mtree_print_mr_owner(p, f, mr); > >>>>>> } > >>>>>> + > >>>>>> + if (system_as && fvi->ac && > >>>>>> + fvi->ac->has_memory(current_machine, > >>>>>> + int128_get64(range->addr.start), > >>>>>> + MR_SIZE(range->addr.size) + 1)) { > >>>>>> + p(f, " %s", fvi->ac_name); > >>>>> > >>>>> I don't understand this. This doesn't check that the memory range > >>>>> actually matches the memory registered with the accelerator, only that > >>>>> there is something in that range. > >>>> > >>>> > >>>> It is checking that a flat range (i.e. what actually works) has a > >>>> corresponding KVM slot: > >>>> https://git.qemu.org/?p=qemu.git;a=blob;f=accel/kvm/kvm-all.c;h=4e1de942ce554c734ac2673030031c228a752ac9;hb=HEAD#l201 > >>>> > >>>> > >>>>> Why isn't it enough to use "info > >>>>> mtree" and look at the KVM address space? > >>>> > >>>> > >>>> There is no such thing in my QEMU, did you mean "KVM-SMRAM" (which is > >>>> missing on spapr)? I am not sure I understand its purpose for the task - > >>>> it prints all same ranges on my x86 laptop, not just ones which we told > >>>> KVM about. > >>>> > >>>> My task is that if let's say "0000:00:1a.0 BAR 0 mmaps[0]" is split into > >>>> several sections because MSIX happens to be in a middle of that BAR and > >>>> it is not system page size aligned, then it is going to be several > >>>> ranges with no clear indication whether or not these were registered as > >>>> KVM slots. A memory chunk can be "ram" and not a KVM slot if it is 4K on > >>>> PPC with 64K system pages, for example. > >>>> > >>>> > >>>> FlatView #0 > >>>> AS "memory", root: system > >>>> AS "cpu-memory-0", root: system > >>>> AS "cpu-memory-1", root: system > >>>> AS "cpu-memory-2", root: system > >>>> AS "cpu-memory-3", root: system > >>>> AS "piix3-ide", root: bus master container > >>>> AS "virtio-net-pci", root: bus master container > >>>> AS "vfio-pci", root: bus master container > >>>> Root memory region: system > >>>> 0000000000000000-00000000000bffff (prio 0, ram): pc.ram kvm > >>>> 00000000000c0000-00000000000c0fff (prio 0, rom): pc.ram > >>>> @00000000000c0000 kvm > >>>> 00000000000c1000-00000000000c3fff (prio 0, ram): pc.ram > >>>> @00000000000c1000 kvm > >>>> 00000000000c4000-00000000000e7fff (prio 0, rom): pc.ram > >>>> @00000000000c4000 kvm > >>>> 00000000000e8000-00000000000effff (prio 0, ram): pc.ram > >>>> @00000000000e8000 kvm > >>>> 00000000000f0000-00000000000fffff (prio 0, rom): pc.ram > >>>> @00000000000f0000 kvm > >>>> 0000000000100000-00000000bfffffff (prio 0, ram): pc.ram > >>>> @0000000000100000 kvm > >>>> 00000000febc0000-00000000febc0fff (prio 0, ramd): 0000:00:1a.0 BAR 0 > >>>> mmaps[0] kvm > >>>> 00000000febc1000-00000000febc102f (prio 0, i/o): msix-table > >>>> 00000000febc1800-00000000febc1807 (prio 0, i/o): msix-pba > >>>> 00000000febfc000-00000000febfcfff (prio 0, i/o): virtio-pci-common > >>>> 00000000febfd000-00000000febfdfff (prio 0, i/o): virtio-pci-isr > >>>> 00000000febfe000-00000000febfefff (prio 0, i/o): virtio-pci-device > >>>> 00000000febff000-00000000febfffff (prio 0, i/o): virtio-pci-notify > >>>> 00000000fec00000-00000000fec00fff (prio 0, i/o): kvm-ioapic > >>>> 00000000fed00000-00000000fed003ff (prio 0, i/o): hpet > >>>> 00000000fee00000-00000000feefffff (prio 4096, i/o): kvm-apic-msi > >>>> 00000000fffc0000-00000000ffffffff (prio 0, rom): pc.bios kvm > >>>> 0000000100000000-000000013fffffff (prio 0, ram): pc.ram > >>>> @00000000c0000000 kvm > >>>> > >>>> > >>>> > >>>> FlatView #3 > >>>> AS "KVM-SMRAM", root: mem-container-smram > >>>> Root memory region: mem-container-smram > >>>> 0000000000000000-00000000000bffff (prio 0, ram): pc.ram > >>>> 00000000000c0000-00000000000c0fff (prio 0, rom): pc.ram @00000000000c0000 > >>>> 00000000000c1000-00000000000c3fff (prio 0, ram): pc.ram @00000000000c1000 > >>>> 00000000000c4000-00000000000e7fff (prio 0, rom): pc.ram @00000000000c4000 > >>>> 00000000000e8000-00000000000effff (prio 0, ram): pc.ram @00000000000e8000 > >>>> 00000000000f0000-00000000000fffff (prio 0, rom): pc.ram @00000000000f0000 > >>>> 0000000000100000-00000000bfffffff (prio 0, ram): pc.ram @0000000000100000 > >>>> 00000000febc0000-00000000febc0fff (prio 0, ramd): 0000:00:1a.0 BAR 0 > >>>> mmaps[0] > >>>> 00000000febc1000-00000000febc102f (prio 0, i/o): msix-table > >>>> 00000000febc1800-00000000febc1807 (prio 0, i/o): msix-pba > >>>> 00000000febfc000-00000000febfcfff (prio 0, i/o): virtio-pci-common > >>>> 00000000febfd000-00000000febfdfff (prio 0, i/o): virtio-pci-isr > >>>> 00000000febfe000-00000000febfefff (prio 0, i/o): virtio-pci-device > >>>> 00000000febff000-00000000febfffff (prio 0, i/o): virtio-pci-notify > >>>> 00000000fec00000-00000000fec00fff (prio 0, i/o): kvm-ioapic > >>>> 00000000fed00000-00000000fed003ff (prio 0, i/o): hpet > >>>> 00000000fee00000-00000000feefffff (prio 4096, i/o): kvm-apic-msi > >>>> 00000000fffc0000-00000000ffffffff (prio 0, rom): pc.bios > >>>> 0000000100000000-000000013fffffff (prio 0, ram): pc.ram @00000000c0000000 > >>>> > >>>> > >>>> > >>>> > >>>>> Perhaps you could add instead an argument to "info mtree" that prints > >>>>> only the AddressSpace with a given name? > >>>> > >>>> Nah, this is not what it is about. > >>> > >>> > >>> > >>> Still a nack? :) > >> > >> > >> > > > > -- > Alexey > -- Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator 2019-06-13 5:07 ` Alexey Kardashevskiy 2019-06-13 11:49 ` Dr. David Alan Gilbert @ 2019-06-13 13:59 ` Paolo Bonzini 2019-06-13 14:04 ` Paolo Bonzini 2 siblings, 0 replies; 19+ messages in thread From: Paolo Bonzini @ 2019-06-13 13:59 UTC (permalink / raw) To: Alexey Kardashevskiy Cc: Daniel Henrique Barboza, Philippe Mathieu-Daudé, Dr. David Alan Gilbert, qemu-devel On 13/06/19 07:07, Alexey Kardashevskiy wrote: > > > On 21/05/2019 16:37, Alexey Kardashevskiy wrote: >> Ping, anyone? I still enjoy seeing "kvm" next to MRs in "info mtree -f" >> in my local QEMU :) > > > "Still feeling it" :) > Ping? Queued for persistence. :) Paolo > >> >> >> On 24/04/2019 15:32, Alexey Kardashevskiy wrote: >>> Paolo, ping? >>> >>> >>> On 19/03/2019 18:05, Alexey Kardashevskiy wrote: >>>> >>>> >>>> On 11/02/2019 15:56, Alexey Kardashevskiy wrote: >>>>> >>>>> >>>>> On 09/02/2019 04:26, Paolo Bonzini wrote: >>>>>> On 07/02/19 12:49, Dr. David Alan Gilbert wrote: >>>>>>> //#define DEBUG_UNASSIGNED >>>>>>> @@ -2924,6 +2926,8 @@ struct FlatViewInfo { >>>>>>> int counter; >>>>>>> bool dispatch_tree; >>>>>>> bool owner; >>>>>>> + AccelClass *ac; >>>>>>> + const char *ac_name; >>>>>>> }; >>>>>>> >>>>>>> static void mtree_print_flatview(gpointer key, gpointer value, >>>>>>> @@ -2939,6 +2943,7 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>>>>> int n = view->nr; >>>>>>> int i; >>>>>>> AddressSpace *as; >>>>>>> + bool system_as = false; >>>>>>> >>>>>>> p(f, "FlatView #%d\n", fvi->counter); >>>>>>> ++fvi->counter; >>>>>>> @@ -2950,6 +2955,9 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>>>>> p(f, ", alias %s", memory_region_name(as->root->alias)); >>>>>>> } >>>>>>> p(f, "\n"); >>>>>>> + if (as == &address_space_memory) { >>>>>>> + system_as = true; >>>>>>> + } >>>>>>> } >>>>>>> >>>>>>> p(f, " Root memory region: %s\n", >>>>>>> @@ -2985,6 +2993,13 @@ static void mtree_print_flatview(gpointer key, gpointer value, >>>>>>> if (fvi->owner) { >>>>>>> mtree_print_mr_owner(p, f, mr); >>>>>>> } >>>>>>> + >>>>>>> + if (system_as && fvi->ac && >>>>>>> + fvi->ac->has_memory(current_machine, >>>>>>> + int128_get64(range->addr.start), >>>>>>> + MR_SIZE(range->addr.size) + 1)) { >>>>>>> + p(f, " %s", fvi->ac_name); >>>>>> >>>>>> I don't understand this. This doesn't check that the memory range >>>>>> actually matches the memory registered with the accelerator, only that >>>>>> there is something in that range. >>>>> >>>>> >>>>> It is checking that a flat range (i.e. what actually works) has a >>>>> corresponding KVM slot: >>>>> https://git.qemu.org/?p=qemu.git;a=blob;f=accel/kvm/kvm-all.c;h=4e1de942ce554c734ac2673030031c228a752ac9;hb=HEAD#l201 >>>>> >>>>> >>>>>> Why isn't it enough to use "info >>>>>> mtree" and look at the KVM address space? >>>>> >>>>> >>>>> There is no such thing in my QEMU, did you mean "KVM-SMRAM" (which is >>>>> missing on spapr)? I am not sure I understand its purpose for the task - >>>>> it prints all same ranges on my x86 laptop, not just ones which we told >>>>> KVM about. >>>>> >>>>> My task is that if let's say "0000:00:1a.0 BAR 0 mmaps[0]" is split into >>>>> several sections because MSIX happens to be in a middle of that BAR and >>>>> it is not system page size aligned, then it is going to be several >>>>> ranges with no clear indication whether or not these were registered as >>>>> KVM slots. A memory chunk can be "ram" and not a KVM slot if it is 4K on >>>>> PPC with 64K system pages, for example. >>>>> >>>>> >>>>> FlatView #0 >>>>> AS "memory", root: system >>>>> AS "cpu-memory-0", root: system >>>>> AS "cpu-memory-1", root: system >>>>> AS "cpu-memory-2", root: system >>>>> AS "cpu-memory-3", root: system >>>>> AS "piix3-ide", root: bus master container >>>>> AS "virtio-net-pci", root: bus master container >>>>> AS "vfio-pci", root: bus master container >>>>> Root memory region: system >>>>> 0000000000000000-00000000000bffff (prio 0, ram): pc.ram kvm >>>>> 00000000000c0000-00000000000c0fff (prio 0, rom): pc.ram >>>>> @00000000000c0000 kvm >>>>> 00000000000c1000-00000000000c3fff (prio 0, ram): pc.ram >>>>> @00000000000c1000 kvm >>>>> 00000000000c4000-00000000000e7fff (prio 0, rom): pc.ram >>>>> @00000000000c4000 kvm >>>>> 00000000000e8000-00000000000effff (prio 0, ram): pc.ram >>>>> @00000000000e8000 kvm >>>>> 00000000000f0000-00000000000fffff (prio 0, rom): pc.ram >>>>> @00000000000f0000 kvm >>>>> 0000000000100000-00000000bfffffff (prio 0, ram): pc.ram >>>>> @0000000000100000 kvm >>>>> 00000000febc0000-00000000febc0fff (prio 0, ramd): 0000:00:1a.0 BAR 0 >>>>> mmaps[0] kvm >>>>> 00000000febc1000-00000000febc102f (prio 0, i/o): msix-table >>>>> 00000000febc1800-00000000febc1807 (prio 0, i/o): msix-pba >>>>> 00000000febfc000-00000000febfcfff (prio 0, i/o): virtio-pci-common >>>>> 00000000febfd000-00000000febfdfff (prio 0, i/o): virtio-pci-isr >>>>> 00000000febfe000-00000000febfefff (prio 0, i/o): virtio-pci-device >>>>> 00000000febff000-00000000febfffff (prio 0, i/o): virtio-pci-notify >>>>> 00000000fec00000-00000000fec00fff (prio 0, i/o): kvm-ioapic >>>>> 00000000fed00000-00000000fed003ff (prio 0, i/o): hpet >>>>> 00000000fee00000-00000000feefffff (prio 4096, i/o): kvm-apic-msi >>>>> 00000000fffc0000-00000000ffffffff (prio 0, rom): pc.bios kvm >>>>> 0000000100000000-000000013fffffff (prio 0, ram): pc.ram >>>>> @00000000c0000000 kvm >>>>> >>>>> >>>>> >>>>> FlatView #3 >>>>> AS "KVM-SMRAM", root: mem-container-smram >>>>> Root memory region: mem-container-smram >>>>> 0000000000000000-00000000000bffff (prio 0, ram): pc.ram >>>>> 00000000000c0000-00000000000c0fff (prio 0, rom): pc.ram @00000000000c0000 >>>>> 00000000000c1000-00000000000c3fff (prio 0, ram): pc.ram @00000000000c1000 >>>>> 00000000000c4000-00000000000e7fff (prio 0, rom): pc.ram @00000000000c4000 >>>>> 00000000000e8000-00000000000effff (prio 0, ram): pc.ram @00000000000e8000 >>>>> 00000000000f0000-00000000000fffff (prio 0, rom): pc.ram @00000000000f0000 >>>>> 0000000000100000-00000000bfffffff (prio 0, ram): pc.ram @0000000000100000 >>>>> 00000000febc0000-00000000febc0fff (prio 0, ramd): 0000:00:1a.0 BAR 0 >>>>> mmaps[0] >>>>> 00000000febc1000-00000000febc102f (prio 0, i/o): msix-table >>>>> 00000000febc1800-00000000febc1807 (prio 0, i/o): msix-pba >>>>> 00000000febfc000-00000000febfcfff (prio 0, i/o): virtio-pci-common >>>>> 00000000febfd000-00000000febfdfff (prio 0, i/o): virtio-pci-isr >>>>> 00000000febfe000-00000000febfefff (prio 0, i/o): virtio-pci-device >>>>> 00000000febff000-00000000febfffff (prio 0, i/o): virtio-pci-notify >>>>> 00000000fec00000-00000000fec00fff (prio 0, i/o): kvm-ioapic >>>>> 00000000fed00000-00000000fed003ff (prio 0, i/o): hpet >>>>> 00000000fee00000-00000000feefffff (prio 4096, i/o): kvm-apic-msi >>>>> 00000000fffc0000-00000000ffffffff (prio 0, rom): pc.bios >>>>> 0000000100000000-000000013fffffff (prio 0, ram): pc.ram @00000000c0000000 >>>>> >>>>> >>>>> >>>>> >>>>>> Perhaps you could add instead an argument to "info mtree" that prints >>>>>> only the AddressSpace with a given name? >>>>> >>>>> Nah, this is not what it is about. >>>> >>>> >>>> >>>> Still a nack? :) >>> >>> >>> >> > ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator 2019-06-13 5:07 ` Alexey Kardashevskiy 2019-06-13 11:49 ` Dr. David Alan Gilbert 2019-06-13 13:59 ` Paolo Bonzini @ 2019-06-13 14:04 ` Paolo Bonzini 2 siblings, 0 replies; 19+ messages in thread From: Paolo Bonzini @ 2019-06-13 14:04 UTC (permalink / raw) To: Alexey Kardashevskiy Cc: Daniel Henrique Barboza, Philippe Mathieu-Daudé, Dr. David Alan Gilbert, qemu-devel On 13/06/19 07:07, Alexey Kardashevskiy wrote: > + if (as == &address_space_memory) { > + system_as = true; > + } > } Ah no, wait... On x86, there are two KVM address spaces, so you have to pass "as" to has_memory. To detect KVM address spaces, you can allocate an array with kvm_check_extension(kvm, KVM_CAP_MULTI_ADDRESS_SPACE) elements (default 1 if the capability is not available), and fill it in kvm_memory_listener_register. Then you an search for the address space in that array, it's very small and cheap. Promise to merge quickly with that change. Thanks, Paolo ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2019-06-13 15:29 UTC | newest]
Thread overview: 19+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-12-14 2:58 [Qemu-devel] [PATCH qemu v2] hmp: Print if memory section is registered with an accelerator Alexey Kardashevskiy
2018-12-14 11:07 ` Philippe Mathieu-Daudé
2018-12-17 1:27 ` Alexey Kardashevskiy
2018-12-17 12:47 ` Philippe Mathieu-Daudé
2018-12-18 0:58 ` Alexey Kardashevskiy
2019-01-03 17:37 ` Dr. David Alan Gilbert
2019-01-14 1:43 ` Alexey Kardashevskiy
2019-01-29 2:30 ` Alexey Kardashevskiy
2019-02-07 5:20 ` Alexey Kardashevskiy
2019-02-07 11:49 ` Dr. David Alan Gilbert
2019-02-08 17:26 ` Paolo Bonzini
2019-02-11 4:56 ` Alexey Kardashevskiy
[not found] ` <f346fdcb-1849-3397-7f4c-2d6ee07fcd7c@ozlabs.ru>
2019-04-24 5:32 ` Alexey Kardashevskiy
2019-04-24 5:32 ` Alexey Kardashevskiy
2019-05-21 6:37 ` Alexey Kardashevskiy
2019-06-13 5:07 ` Alexey Kardashevskiy
2019-06-13 11:49 ` Dr. David Alan Gilbert
2019-06-13 13:59 ` Paolo Bonzini
2019-06-13 14:04 ` 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.