Re: [PATCH] TCG plugin API extension to read guest memory content by an address

[Alex Bennée]

Mikhail Tyutin <m.tyutin@yadro.com> writes:

> TCG plugin API extension to read guest memory content. 
> qemu_plugin_vcpu_read_phys_mem()
> function can be used by TCG plugin inside of qemu_plugin_vcpu_mem_cb_t 
> callback to adjust
> received address according to internal memory mappings and read content of 
> guest memory.
> Works for both user-level and system-level emulation modes.
>
> Signed-off-by: Mikhail Tyutin <m.tyutin@yadro.com>
> Signed-off-by: Aleksey Titov <a.titov@yadro.com>
> ---
> QEMU_PLUGIN_READ_PHYS_MEM_ENABLED define below is to let plugins to check if 
> this API
> is available in Qemu build.
>
>   include/qemu/qemu-plugin.h   | 18 ++++++++++++++++++
>   plugins/api.c                | 20 ++++++++++++++++++++
>   plugins/qemu-plugins.symbols |  1 +
>   3 files changed, 39 insertions(+)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index d0e9d03adf..576597f601 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -625,4 +625,22 @@ uint64_t qemu_plugin_end_code(void);
>    */
>   uint64_t qemu_plugin_entry_code(void);
>   
> +
> +#define QEMU_PLUGIN_READ_PHYS_MEM_ENABLED

We don't need this - we should update the plugin API version instead.

> +/**
> + * qemu_plugin_vcpu_read_phys_mem() - reads guest's memory content
> + *
> + * @vcpu_index: vcpu index
> + * @addr: guest's virtual address
> + * @buf: destination buffer to read data to
> + * @len: number of bytes to read
> + *
> + * Adjusts address according to internal memory mapping and reads
> + * content of guest memory.
> + */
> +void qemu_plugin_vcpu_read_phys_mem(unsigned int vcpu_index,
> +                                    uint64_t addr,
> +                                    void *buf,
> +                                    uint64_t len);
> +
>   #endif /* QEMU_QEMU_PLUGIN_H */
> diff --git a/plugins/api.c b/plugins/api.c
> index 2078b16edb..95753bce95 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -442,3 +442,23 @@ uint64_t qemu_plugin_entry_code(void)
>   #endif
>       return entry;
>   }
> +
> +void qemu_plugin_vcpu_read_phys_mem(unsigned int vcpu_index,
> +                                    uint64_t addr,
> +                                    void *buf,
> +                                    uint64_t len) {
> +#ifndef CONFIG_USER_ONLY
> +    cpu_physical_memory_rw(addr, buf, len, false);
> +#else
> +    CPUClass *cc;
> +    CPUState *cpu;
> +
> +    cpu = qemu_get_cpu(vcpu_index);
> +    cc = CPU_GET_CLASS(cpu);
> +    if (cc->memory_rw_debug) {
> +        cc->memory_rw_debug(cpu, addr, buf, len, false);
> +    } else {
> +        cpu_memory_rw_debug(cpu, addr, buf, len, false);
> +    }
> +#endif
> +}

I'm wary about this because what memory is the plugin going to be
looking at? There are certainly no guarantees the memory contain the
value that has just been loaded or stored by the instrumented
instruction.

I certainly prefer to see example plugins that use new APIs in the same
patch series to help in assessing if the API is useful or flawed for the
purpose its being suggested for.

> \ No newline at end of file
> diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols
> index 71f6c90549..f0ce8c730f 100644
> --- a/plugins/qemu-plugins.symbols
> +++ b/plugins/qemu-plugins.symbols
> @@ -42,4 +42,5 @@
>     qemu_plugin_tb_vaddr;
>     qemu_plugin_uninstall;
>     qemu_plugin_vcpu_for_each;
> +  qemu_plugin_vcpu_read_phys_mem;
>   };


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro