Re: [PATCH 1/1] plugins: add API to read guest CPU memory from hwaddr

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

Hi Rowan,

thanks for this good complement on the virt address read function.

However, to be able to merge a new plugins API function, we must have a 
concrete usage of it, through one of the existing plugin.
What could be a good demonstration of value brought by being able to 
read a physical address?

Thanks,
Pierrick

On 8/27/24 23:32, Rowan Hart wrote:
> Signed-off-by: Rowan Hart <rowanbhart@gmail.com>
> ---
>   include/qemu/qemu-plugin.h   | 22 ++++++++++++++++++++++
>   plugins/api.c                | 17 +++++++++++++++++
>   plugins/qemu-plugins.symbols |  2 ++
>   3 files changed, 41 insertions(+)
> 
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index c71c705b69..25f39c0960 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -868,6 +868,28 @@ QEMU_PLUGIN_API
>   int qemu_plugin_read_register(struct qemu_plugin_register *handle,
>                                 GByteArray *buf);
>   
> +/**
> + * qemu_plugin_read_cpu_memory_hwaddr() - read CPU memory from hwaddr
> + *
> + * @addr: A virtual address to read from
> + * @data: A byte array to store data into
> + * @len: The number of bytes to read, starting from @addr
> + *
> + * @len bytes of data is read starting at @addr and stored into @data. If @data
> + * is not large enough to hold @len bytes, it will be expanded to the necessary
> + * size, reallocating if necessary. @len must be greater than 0.
> + *
> + * This function does not ensure writes are flushed prior to reading, so
> + * callers should take care when calling this function in plugin callbacks to
> + * avoid attempting to read data which may not yet be written and should use
> + * the memory callback API instead.
> + *
> + * Returns true on success and false on failure.
> + */
> +QEMU_PLUGIN_API
> +bool qemu_plugin_read_cpu_memory_hwaddr(uint64_t addr,
> +                                          GByteArray *data, size_t len);
> +
>   /**
>    * qemu_plugin_scoreboard_new() - alloc a new scoreboard
>    *
> diff --git a/plugins/api.c b/plugins/api.c
> index 2ff13d09de..c87bed6641 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -527,6 +527,22 @@ GArray *qemu_plugin_get_registers(void)
>       return create_register_handles(regs);
>   }
>   
> +bool qemu_plugin_read_cpu_memory_hwaddr(uint64_t addr,
> +                                        GByteArray *data, uint64_t len)
> +{
> +#ifndef CONFIG_USER_ONLY
> +    if (len == 0) {
> +        return false;
> +    }
> +
> +    g_byte_array_set_size(data, len);
> +    cpu_physical_memory_rw(addr, (void *)data->data, len, 0);
> +    return true;
> +#else
> +    return false;
> +#endif
> +}
> +
>   int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
>   {
>       g_assert(current_cpu);
> @@ -534,6 +550,7 @@ int qemu_plugin_read_register(struct qemu_plugin_register *reg, GByteArray *buf)
>       return gdb_read_register(current_cpu, buf, GPOINTER_TO_INT(reg) - 1);
>   }
>   
> +
>   struct qemu_plugin_scoreboard *qemu_plugin_scoreboard_new(size_t element_size)
>   {
>       return plugin_scoreboard_new(element_size);
> diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
> index ca773d8d9f..5d9cfd71bb 100644
> --- a/plugins/qemu-plugins.symbols
> +++ b/plugins/qemu-plugins.symbols
> @@ -20,6 +20,8 @@
>     qemu_plugin_num_vcpus;
>     qemu_plugin_outs;
>     qemu_plugin_path_to_binary;
> +  qemu_plugin_read_cpu_memory_hwaddr;
> +  qemu_plugin_read_io_memory_hwaddr;

As you mentioned, you can remove the second one for v2.

>     qemu_plugin_read_register;
>     qemu_plugin_register_atexit_cb;
>     qemu_plugin_register_flush_cb;