Luc Michel <luc.mic...@amd.com> writes:
> Hi Pierrick,
>
> On 13:14 Mon 26 Feb , Pierrick Bouvier wrote:
>> Signed-off-by: Pierrick Bouvier <pierrick.bouv...@linaro.org>
>> ---
>> tests/plugin/mem.c | 40 +++++++++++++++++++++++++---------------
>> 1 file changed, 25 insertions(+), 15 deletions(-)
>>
>> diff --git a/tests/plugin/mem.c b/tests/plugin/mem.c
>> index 44e91065ba7..d4729f5e015 100644
>> --- a/tests/plugin/mem.c
>> +++ b/tests/plugin/mem.c
>> @@ -16,9 +16,14 @@
>>
>> QEMU_PLUGIN_EXPORT int qemu_plugin_version = QEMU_PLUGIN_VERSION;
>>
>> -static uint64_t inline_mem_count;
>> -static uint64_t cb_mem_count;
>> -static uint64_t io_count;
>> +typedef struct {
>> + uint64_t mem_count;
>> + uint64_t io_count;
>> +} CPUCount;
>> +
>> +static struct qemu_plugin_scoreboard *counts;
>> +static qemu_plugin_u64 mem_count;
>> +static qemu_plugin_u64 io_count;
>
> I see that you merged inline and callback counts into the same variable.
>
> I wonder... For this test don't you want to keep a plain uint64_t for
> callback counts? I have the feeling that this test was made so one can
> make sure inline and callback counts match.
Indeed the problem now is double counting:
➜ ./qemu-hppa -plugin ./tests/plugin/libmem.so,inline=true -d plugin
./tests/tcg/hppa-linux-user/sha512
1..10
ok 1 - do_test(&tests[i])
ok 2 - do_test(&tests[i])
ok 3 - do_test(&tests[i])
ok 4 - do_test(&tests[i])
ok 5 - do_test(&tests[i])
ok 6 - do_test(&tests[i])
ok 7 - do_test(&tests[i])
ok 8 - do_test(&tests[i])
ok 9 - do_test(&tests[i])
ok 10 - do_test(&tests[i])
mem accesses: 262917
🕙22:06:57 alex@draig:qemu.git/builds/all on plugins/next [$?]
➜ ./qemu-hppa -plugin ./tests/plugin/libmem.so,inline=true,callback=true -d
plugin ./tests/tcg/hppa-linux-user/sha512
1..10
ok 1 - do_test(&tests[i])
ok 2 - do_test(&tests[i])
ok 3 - do_test(&tests[i])
ok 4 - do_test(&tests[i])
ok 5 - do_test(&tests[i])
ok 6 - do_test(&tests[i])
ok 7 - do_test(&tests[i])
ok 8 - do_test(&tests[i])
ok 9 - do_test(&tests[i])
ok 10 - do_test(&tests[i])
mem accesses: 525834
although perhaps it would just be simpler for the plugin to only accept
one or the other method.
>
> Luc
>
>> static bool do_inline, do_callback;
>> static bool do_haddr;
>> static enum qemu_plugin_mem_rw rw = QEMU_PLUGIN_MEM_RW;
>> @@ -27,16 +32,16 @@ static void plugin_exit(qemu_plugin_id_t id, void *p)
>> {
>> g_autoptr(GString) out = g_string_new("");
>>
>> - if (do_inline) {
>> - g_string_printf(out, "inline mem accesses: %" PRIu64 "\n",
>> inline_mem_count);
>> - }
>> - if (do_callback) {
>> - g_string_append_printf(out, "callback mem accesses: %" PRIu64 "\n",
>> cb_mem_count);
>> + if (do_inline || do_callback) {
>> + g_string_printf(out, "mem accesses: %" PRIu64 "\n",
>> + qemu_plugin_u64_sum(mem_count));
>> }
>> if (do_haddr) {
>> - g_string_append_printf(out, "io accesses: %" PRIu64 "\n", io_count);
>> + g_string_append_printf(out, "io accesses: %" PRIu64 "\n",
>> + qemu_plugin_u64_sum(io_count));
>> }
>> qemu_plugin_outs(out->str);
>> + qemu_plugin_scoreboard_free(counts);
>> }
>>
>> static void vcpu_mem(unsigned int cpu_index, qemu_plugin_meminfo_t meminfo,
>> @@ -46,12 +51,12 @@ static void vcpu_mem(unsigned int cpu_index,
>> qemu_plugin_meminfo_t meminfo,
>> struct qemu_plugin_hwaddr *hwaddr;
>> hwaddr = qemu_plugin_get_hwaddr(meminfo, vaddr);
>> if (qemu_plugin_hwaddr_is_io(hwaddr)) {
>> - io_count++;
>> + qemu_plugin_u64_add(io_count, cpu_index, 1);
>> } else {
>> - cb_mem_count++;
>> + qemu_plugin_u64_add(mem_count, cpu_index, 1);
>> }
>> } else {
>> - cb_mem_count++;
>> + qemu_plugin_u64_add(mem_count, cpu_index, 1);
>> }
>> }
>>
>> @@ -64,9 +69,10 @@ static void vcpu_tb_trans(qemu_plugin_id_t id, struct
>> qemu_plugin_tb *tb)
>> struct qemu_plugin_insn *insn = qemu_plugin_tb_get_insn(tb, i);
>>
>> if (do_inline) {
>> - qemu_plugin_register_vcpu_mem_inline(insn, rw,
>> - QEMU_PLUGIN_INLINE_ADD_U64,
>> - &inline_mem_count, 1);
>> + qemu_plugin_register_vcpu_mem_inline_per_vcpu(
>> + insn, rw,
>> + QEMU_PLUGIN_INLINE_ADD_U64,
>> + mem_count, 1);
>> }
>> if (do_callback) {
>> qemu_plugin_register_vcpu_mem_cb(insn, vcpu_mem,
>> @@ -117,6 +123,10 @@ QEMU_PLUGIN_EXPORT int
>> qemu_plugin_install(qemu_plugin_id_t id,
>> }
>> }
>>
>> + counts = qemu_plugin_scoreboard_new(sizeof(CPUCount));
>> + mem_count = qemu_plugin_scoreboard_u64_in_struct(
>> + counts, CPUCount, mem_count);
>> + io_count = qemu_plugin_scoreboard_u64_in_struct(counts, CPUCount,
>> io_count);
>> qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans);
>> qemu_plugin_register_atexit_cb(id, plugin_exit, NULL);
>> return 0;
>> --
>> 2.43.0
>>
>>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
