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

[Akihiko Odaki <akihiko.odaki@daynix.com>]

On 2024/02/05 18: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.

Then that should be documented and the vcpu_index parameter should be 
deleted.