Florian Hofhammer <[email protected]> writes:
> Some registers should be marked as read-only from a plugin API
> perspective, as writing to them via qemu_plugin_write_register has no
> effect. This includes the program counter, and we expose this fact to
> the plugins with this patch.
>
> Signed-off-by: Florian Hofhammer <[email protected]>
> ---
> include/qemu/qemu-plugin.h | 2 ++
> plugins/api.c | 17 +++++++++++++++++
> 2 files changed, 19 insertions(+)
>
> diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h
> index f976b62030..1f25fb2b40 100644
> --- a/include/qemu/qemu-plugin.h
> +++ b/include/qemu/qemu-plugin.h
> @@ -942,11 +942,13 @@ struct qemu_plugin_register;
> * writing value with qemu_plugin_write_register
> * @name: register name
> * @feature: optional feature descriptor, can be NULL
> + * @is_readonly: true if the register cannot be written via
> qemu_plugin_write_register
> */
> typedef struct {
> struct qemu_plugin_register *handle;
> const char *name;
> const char *feature;
> + bool is_readonly;
> } qemu_plugin_reg_descriptor;
>
> /**
> diff --git a/plugins/api.c b/plugins/api.c
> index fc19bdb40b..de8c32db50 100644
> --- a/plugins/api.c
> +++ b/plugins/api.c
> @@ -403,6 +403,12 @@ bool qemu_plugin_bool_parse(const char *name, const char
> *value, bool *ret)
> * ancillary data the plugin might find useful.
> */
>
> +static const char pc_str[] = "pc"; // generic name for program counter
> +static const char eip_str[] = "eip"; // x86 specific name for program counter
> +static const char rip_str[] = "rip"; // x86_64 specific name for program
> counter
> +static const char pswa_str[] = "pswa"; // s390x specific name for program
> counter
> +static const char iaoq_str[] = "iaoq"; // HP/PA specific name for program
> counter
> +static const char rpc_str[] = "rpc"; // microblaze specific name for
> program counter
It's ugly but I can't think of anything better as you say in the commit message.
> static GArray *create_register_handles(GArray *gdbstub_regs)
> {
> GArray *find_data = g_array_new(true, true,
> @@ -417,9 +423,20 @@ static GArray *create_register_handles(GArray
> *gdbstub_regs)
> continue;
> }
>
> + gint plugin_ro_bit = 0;
> /* Create a record for the plugin */
> desc.handle = GINT_TO_POINTER(grd->gdb_reg + 1);
> desc.name = g_intern_string(grd->name);
Lets just set desc.is_readonly to false here.
> + if (!strcmp(desc.name, pc_str)
> + || !strcmp(desc.name, eip_str)
> + || !strcmp(desc.name, rip_str)
> + || !strcmp(desc.name, pswa_str)
> + || !strcmp(desc.name, iaoq_str)
> + || !strcmp(desc.name, rpc_str)
> + ) {
> + plugin_ro_bit = 1;
> + }
> + desc.is_readonly = plugin_ro_bit == 1 ? true : false;
And fold setting it to true into the if statement. I have a marginal
preference for g_strcmp0(desc.name, eip_str) == 0 style tests.
> desc.feature = g_intern_string(grd->feature_name);
> g_array_append_val(find_data, desc);
> }
Otherwise:
Reviewed-by: Alex Bennée <[email protected]>
--
Alex Bennée
Virtualisation Tech Lead @ Linaro
