Re: [PATCH 25/29] contrib/plugins: extend execlog to track register changes

[Alex Bennée]

Alex Bennée <alex.bennee@linaro.org> writes:

> With the new plugin register API we can now track changes to register
> values. Currently the implementation is fairly dumb which will slow
> down if a large number of register values are being tracked. This
> could be improved by only instrumenting instructions which mention
> registers we are interested in tracking.
>
> Example usage:
>
>   ./qemu-aarch64 -D plugin.log -d plugin \
>      -cpu max,sve256=on \
>      -plugin contrib/plugins/libexeclog.so,reg=sp,reg=z\* \
>      ./tests/tcg/aarch64-linux-user/sha512-sve
>
> will display in the execlog any changes to the stack pointer (sp) and
> the SVE Z registers.
>
> Cc: Akihiko Odaki <akihiko.odaki@daynix.com>
> Based-On: <20231025093128.33116-19-akihiko.odaki@daynix.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>
> ---
> vAJB:
>
> Changes for the new API with a simpler glob based "reg" specifier
> which can be specified multiple times.
> +/*
> + * Initialise a new vcpu/thread with:
> + *   - last_exec tracking data
> + *   - list of tracked registers
> + *   - initial value of registers
> + *
> + * As we could have multiple threads trying to do this we need to
> + * serialise the expansion under a lock.
> + */
> +static void vcpu_init(qemu_plugin_id_t id, unsigned int vcpu_index)
> +{
> +    g_rw_lock_writer_lock(&expand_array_lock);
> +
> +    if (vcpu_index >= num_cpus) {
> +        cpus = g_realloc_n(cpus, vcpu_index + 1, sizeof(*cpus));
> +        while (vcpu_index >= num_cpus) {
> +            cpus[num_cpus].last_exec = g_string_new(NULL);
> +
> +            /* Any registers to track? */
> +            if (rmatches && rmatches->len) {
> +                GPtrArray *registers = g_ptr_array_new();
> +
> +                /* For each pattern add the register definitions */
> +                for (int p = 0; p < rmatches->len; p++) {
> +                    g_autoptr(GArray) reg_list =
> +                        qemu_plugin_find_registers(vcpu_index, 
> rmatches->pdata[p]);
> +                    if (reg_list && reg_list->len) {
> +                        for (int r = 0; r < reg_list->len; r++) {
> +                            Register *reg =
> +                                init_vcpu_register(vcpu_index,
> +                                                   &g_array_index(reg_list,
> +                                                                  
> qemu_plugin_reg_descriptor, r));
> +                            g_ptr_array_add(registers, reg);
> +                        }
> +                    }
> +                }
> +                cpus[num_cpus].registers = registers;

Note to self:

modified   contrib/plugins/execlog.c
@@ -250,6 +250,8 @@ static void vcpu_init(qemu_plugin_id_t id, unsigned int 
vcpu_index)
                     }
                 }
                 cpus[num_cpus].registers = registers;
+            } else {
+                cpus[num_cpus].registers = NULL;
             }
             num_cpus++;
         }


> +            }
> +            num_cpus++;
> +        }
> +    }
> +
> +    g_rw_lock_writer_unlock(&expand_array_lock);
> +}
> +
>  /**
>   * On plugin exit, print last instruction in cache
>   */
>  static void plugin_exit(qemu_plugin_id_t id, void *p)
>  {
>      guint i;
> -    GString *s;
> -    for (i = 0; i < last_exec->len; i++) {
> -        s = g_ptr_array_index(last_exec, i);
> -        if (s->str) {
> -            qemu_plugin_outs(s->str);
> +    for (i = 0; i < num_cpus; i++) {
> +        if (cpus[i].last_exec->str) {
> +            qemu_plugin_outs(cpus[i].last_exec->str);
>              qemu_plugin_outs("\n");
>          }
>      }
> @@ -212,6 +291,18 @@ static void parse_vaddr_match(char *match)
>      g_array_append_val(amatches, v);
>  }
-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro