Re: [PATCH v11 0/3] gdbstub and TCG plugin improvements

[Alex Bennée]

Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:

> On 2/5/24 13:31, Alex Bennée wrote:
>> Akihiko Odaki <akihiko.odaki@daynix.com> writes:
>> 
>>> On 2024/02/03 22:58, Alex Bennée wrote:
>>>> Akihiko Odaki <akihiko.odaki@daynix.com> writes:
>>>>
>>>>> On 2024/02/03 20:08, Alex Bennée wrote:
>>>>>> Akihiko Odaki <akihiko.odaki@daynix.com> writes:
>>>>>>
>>>>>>> This series extracts fixes and refactorings that can be applied
>>>>>>> independently from "[PATCH v9 00/23] plugins: Allow to read registers".
>>>>>>>
>>>>>>> The patch "target/riscv: Move MISA limits to class" was replaced with
>>>>>>> patch "target/riscv: Move misa_mxl_max to class" since I found instances
>>>>>>> may have different misa_ext_mask.
>>>>>> As this is re-based on Alistair's riscv-to-apply.next tree I'll wait
>>>>>> for
>>>>>> this to go through the RiscV trees and then re-base the plugin patches
>>>>>> and dropping the merged riscv patches from my tree.
>>>>>> In the meantime feel free to review:
>>>>>>      Message-Id: <20240122145610.413836-1-alex.bennee@linaro.org>
>>>>>>      Date: Mon, 22 Jan 2024 14:55:49 +0000
>>>>>>      Subject: [PATCH v3 00/21] plugin updates (register access) for 9.0 
>>>>>> (pre-PR?)
>>>>>>      From: =?UTF-8?q?Alex=20Benn=C3=A9e?= <alex.bennee@linaro.org>
>>>>>> For:
>>>>>>      contrib/plugins: extend execlog to track register changes
>>>>>>      gdbstub: expose api to find registers
>>>>>> So I can add this to my maintainer omnibus series for the next PR I
>>>>>> send.
>>>>>
>>>>> I added one trivial comment to: "gdbstub: expose api to find registers"
>>>>>
>>>>> "contrib/plugins: extend execlog to track register changes" depends on
>>>>> "plugins: add an API to read registers". The comments for the patch in
>>>>> the following email are not addressed yet:
>>>>> https://lore.kernel.org/all/4b2156ed-688d-4617-b52d-200413f01156@daynix.com/
>>>> I don't think we need to serialise with the BQL as the structures
>>>> are
>>>> per-CPU (and created on vCPU creation).
>>>
>>> qemu_plugin_get_registers() has vcpu parameter, which can refer to a
>>> different vcpu the caller is on (or the caller may not be in a vcpu
>>> context at all).
>> It should only be called from the current cpu context. We can either
>> assert that or make it implicit like qemu_plugin_insn_disas does.
>> However we will need to ensure current_cpu is set before the vcpu_init
>> callback.
>> Pierrick has had to move these initialisations around for the
>> scoreboard
>> work so they are now run with safe work once the thread starts.
>> 
>
> As a complement, in the series I'll post, the work is run
> asynchronously, but not "safe_async", which means it's not under an
> exclusive section.
>
> If you need this guarantee for registers API, it's better to add this.

We don't. We just want to ensure they line up and are not cross-vCPU.

>
>>>
>>>> As far as the restructuring we can move it into gdbstub later if
>>>> there
>>>> is a need to. At the moment the structure is just housekeeping for
>>>> plugins.
>>>
>>> Certainly we can move it later, but adding the code in the plugin
>>> infrastructure now won't help in that case.
>> 

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro