[v6,06/12] tests/plugin/mem: migrate to new per_vcpu API

Reviewed-by: Luc Michel <luc.michel@amd.com>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
---
 tests/plugin/mem.c | 46 +++++++++++++++++++++++++++++++---------------
 1 file changed, 31 insertions(+), 15 deletions(-)

Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:

> Reviewed-by: Luc Michel <luc.michel@amd.com>
> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> ---
>  tests/plugin/mem.c | 46 +++++++++++++++++++++++++++++++---------------
>  1 file changed, 31 insertions(+), 15 deletions(-)
>
> diff --git a/tests/plugin/mem.c b/tests/plugin/mem.c
> index 44e91065ba7..b650dddcce1 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;
>  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,16 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
>          }
>      }
>  
> +    if (do_inline && do_callback) {
> +        fprintf(stderr,
> +                "can't enable inline and callback counting at the same time\n");
> +        return -1;
> +    }
> +
> +    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;

You need the following to keep check-tcg working:

modified   tests/tcg/Makefile.target
@@ -168,7 +168,7 @@ RUN_TESTS+=$(EXTRA_RUNS)
 
 # Some plugins need additional arguments above the default to fully
 # exercise things. We can define them on a per-test basis here.
-run-plugin-%-with-libmem.so: PLUGIN_ARGS=$(COMMA)inline=true$(COMMA)callback=true
+run-plugin-%-with-libmem.so: PLUGIN_ARGS=$(COMMA)inline=true
 
 ifeq ($(filter %-softmmu, $(TARGET)),)
 run-%: %

On 2/29/24 10:08 PM, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> 
>> Reviewed-by: Luc Michel <luc.michel@amd.com>
>> Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> ---
>>   tests/plugin/mem.c | 46 +++++++++++++++++++++++++++++++---------------
>>   1 file changed, 31 insertions(+), 15 deletions(-)
>>
>> diff --git a/tests/plugin/mem.c b/tests/plugin/mem.c
>> index 44e91065ba7..b650dddcce1 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;
>>   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,16 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id,
>>           }
>>       }
>>   
>> +    if (do_inline && do_callback) {
>> +        fprintf(stderr,
>> +                "can't enable inline and callback counting at the same time\n");
>> +        return -1;
>> +    }
>> +
>> +    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;
> 
> You need the following to keep check-tcg working:
> 
> modified   tests/tcg/Makefile.target
> @@ -168,7 +168,7 @@ RUN_TESTS+=$(EXTRA_RUNS)
>   
>   # Some plugins need additional arguments above the default to fully
>   # exercise things. We can define them on a per-test basis here.
> -run-plugin-%-with-libmem.so: PLUGIN_ARGS=$(COMMA)inline=true$(COMMA)callback=true
> +run-plugin-%-with-libmem.so: PLUGIN_ARGS=$(COMMA)inline=true
>   
>   ifeq ($(filter %-softmmu, $(TARGET)),)
>   run-%: %
> 

Missed that, will fix in next version. Thanks!