Re: [PATCH v3 2/8] Add register write API

[Julian Ganz <neither@nut.email>]

Hi Rowan,

> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index 3a850aa216..68c8632fd7 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -893,6 +891,41 @@ typedef struct {
>  QEMU_PLUGIN_API
>  GArray *qemu_plugin_get_registers(void);
> 
> +/**
> + * qemu_plugin_read_register() - read register for current vCPU
> + *
> + * @handle: a @qemu_plugin_reg_handle handle
> + * @buf: A GByteArray for the data owned by the plugin
> + *
> + * This function is only available in a context that register read access is
> + * explicitly requested via the QEMU_PLUGIN_CB_R_REGS flag.
> + *
> + * Returns the size of the read register. The content of @buf is in target byte
> + * order. On failure returns -1.
> + */
> +QEMU_PLUGIN_API
> +int qemu_plugin_read_register(struct qemu_plugin_register *handle,
> +                              GByteArray *buf);
> +
> +/**
> + * qemu_plugin_write_register() - write register for current vCPU
> + *
> + * @handle: a @qemu_plugin_reg_handle handle
> + * @buf: A GByteArray for the data owned by the plugin
> + *
> + * This function is only available in a context that register write access is
> + * explicitly requested via the QEMU_PLUGIN_CB_W_REGS flag.
> + *
> + * The size of @buf must be at least the size of the requested register.
> + * Attempting to write a register with @buf smaller than the register size
> + * will result in a crash or other undesired behavior.
> + *
> + * Returns the number of bytes written. On failure returns 0.
> + */
> +QEMU_PLUGIN_API
> +int qemu_plugin_write_register(struct qemu_plugin_register *handle,
> +                              GByteArray *buf);
> +
>  /**
>   * qemu_plugin_read_memory_vaddr() - read from memory using a virtual address
>   *
> @@ -915,22 +948,6 @@ QEMU_PLUGIN_API
>  bool qemu_plugin_read_memory_vaddr(uint64_t addr,
>                                     GByteArray *data, size_t len);
> 
> -/**
> - * qemu_plugin_read_register() - read register for current vCPU
> - *
> - * @handle: a @qemu_plugin_reg_handle handle
> - * @buf: A GByteArray for the data owned by the plugin
> - *
> - * This function is only available in a context that register read access is
> - * explicitly requested via the QEMU_PLUGIN_CB_R_REGS flag.
> - *
> - * Returns the size of the read register. The content of @buf is in target byte
> - * order. On failure returns -1.
> - */
> -QEMU_PLUGIN_API
> -int qemu_plugin_read_register(struct qemu_plugin_register *handle,
> -                              GByteArray *buf);
> -

Why the code move?

> diff --git a/plugins/api.c b/plugins/api.c
> index 3c9d4832e9..79b2dc20b8 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -433,6 +433,25 @@ GArray *qemu_plugin_get_registers(void)
>      return create_register_handles(regs);
>  }
> 
> +int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
> +{
> +    g_assert(current_cpu);
> +
> +    return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1);
> +}
> +
> +int qemu_plugin_write_register(struct qemu_plugin_register *reg, 
> +                               GByteArray *buf)
> +{
> +    g_assert(current_cpu);
> +
> +    if (buf->len == 0) {
> +        return 0;
> +    }
> +
> +    return gdb_write_register(current_cpu, buf->data, GPOINTER_TO_INT(reg) - 1);
> +}
> +
>  bool qemu_plugin_read_memory_vaddr(uint64_t addr, GByteArray *data, size_t len)
>  {
>      g_assert(current_cpu);
> @@ -453,13 +472,6 @@ bool qemu_plugin_read_memory_vaddr(uint64_t addr, GByteArray *data, size_t len)
>      return true;
>  }
>  
> -int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
> -{
> -    g_assert(current_cpu);
> -
> -    return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1);
> -}
> -

Again, what was the reason for moving `qemu_plugin_read_register`?

Reviewed-By: Julian Ganz <neither@nut.email>

Regards,
Julian