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

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

And by the way, feel free to integrate this with your other series (as 
it's a very similar topic) in a v3, so we can review both at the same time.

Thanks,
Pierrick

On 8/30/24 12:30, Pierrick Bouvier wrote:
> 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;