* [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
* 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.