Re: [PATCH v3 21/27] plugins: add an API to read registers

[Alex Bennée]

Akihiko Odaki <akihiko.odaki@daynix.com> writes:

> On 2024/02/27 19:08, Alex Bennée wrote:
>> Akihiko Odaki <akihiko.odaki@daynix.com> writes:
>> 
>>> On 2024/02/27 1:56, Alex Bennée wrote:
>>>> We can only request a list of registers once the vCPU has been
>>>> initialised so the user needs to use either call the get function on
>>>> vCPU initialisation or during the translation phase.
>>>> We don't expose the reg number to the plugin instead hiding it
>>>> behind
>>>> an opaque handle. For now this is just the gdb_regnum encapsulated in
>>>> an anonymous GPOINTER but in future as we add more state for plugins
>>>> to track we can expand it.
>>>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1706
>>>> Cc: Akihiko Odaki <akihiko.odaki@daynix.com>
>>>> Message-Id: <20240103173349.398526-39-alex.bennee@linaro.org>
>>>> Based-on: <20231025093128.33116-18-akihiko.odaki@daynix.com>
>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>>> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>>
>>> Hi,
>>>
>>> Mostly looks good. I have a few trivial comments so please have a look
>>> at them.
>> Done
>> <snip>
>>>> +        g_array_append_val(find_data, desc);
>>>> +    }
>>>> +
>>>> +    return find_data;
>>>> +}
>>>> +
>>>> +GArray *qemu_plugin_get_registers(void)
>>>> +{
>>>> +    g_assert(current_cpu);
>>>> +
>>>> +    g_autoptr(GArray) regs = gdb_get_register_list(current_cpu);
>>>> +    return regs->len ? create_register_handles(current_cpu, regs) : NULL;
>>>
>>> Why do you need regs->len check?
>> Not all guests expose register to gdb so we need to catch that:
>>    TEST    catch-syscalls-with-libinsn.so on alpha
>> **
>> ERROR:../../plugins/api.c:459:qemu_plugin_get_registers: assertion failed: 
>> (regs->len)
>> Aborted
> Certainly regs->len can be 0, but why do you need to return NULL in
> that case? Can't qemu_plugin_get_registers() return an empty array
> just as gdb_get_register_list() does?

That seems more troublesome to handle the other end. NULL is nothing is
a fairly common pattern here which makes short-circuiting the array
iteration simple.


-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro