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

[Pierrick Bouvier]

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;