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

Akihiko Odaki posted 3 patches 2 months, 3 weeks ago
Failed in applying to current master (apply log)
target/riscv/cpu.h         |   4 +-
hw/riscv/boot.c            |   3 +-
target/riscv/cpu.c         | 181 ++++++++++++++++++++++++++-------------------
target/riscv/gdbstub.c     |  12 ++-
target/riscv/kvm/kvm-cpu.c |  10 +--
target/riscv/machine.c     |   7 +-
target/riscv/tcg/tcg-cpu.c |  44 ++---------
target/riscv/translate.c   |   3 +-
8 files changed, 133 insertions(+), 131 deletions(-)
[PATCH v11 0/3] gdbstub and TCG plugin improvements
Posted by Akihiko Odaki 2 months, 3 weeks ago
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.

V6 -> V7:
  Rebased.

V5 -> V6:
  Added patch "default-configs: Add TARGET_XML_FILES definition".
  Rebased.

V4 -> V5:
  Added patch "hw/riscv: Use misa_mxl instead of misa_mxl_max".

V3 -> V4:
  Added patch "gdbstub: Check if gdb_regs is NULL".

V2 -> V3:
  Restored patch sets from the previous version.
  Rebased to commit 800485762e6564e04e2ab315132d477069562d91.

V1 -> V2:
  Added patch "target/riscv: Do not allow MXL_RV32 for TARGET_RISCV64".
  Added patch "target/riscv: Initialize gdb_core_xml_file only once".
  Dropped patch "target/riscv: Remove misa_mxl validation".
  Dropped patch "target/riscv: Move misa_mxl_max to class".
  Dropped patch "target/riscv: Validate misa_mxl_max only once".

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
Changes in v11:
- Rebased on: https://github.com/alistair23/qemu/tree/riscv-to-apply.next
- Link to v10: https://lore.kernel.org/r/20240128-riscv-v10-0-fdbe593976e9@daynix.com

Changes in v10:
- Dropped patch "hw/riscv: Use misa_mxl instead of misa_mxl_max" due to
  invalid assumption that the relevant code is only used for kernel
  loading.
- Link to v9: https://lore.kernel.org/r/20240115-riscv-v9-0-ff171e1aedc8@daynix.com

Changes in v9:
- Rebased to commit 977542ded7e6b28d2bc077bcda24568c716e393c.
- Link to v8: https://lore.kernel.org/r/20231218-riscv-v8-0-c9bf2b1582d7@daynix.com

Changes in v8:
- Added a more detailed explanation for patch "hw/riscv: Use misa_mxl
  instead of misa_mxl_max". (Alistair Francis)
- Link to v7: https://lore.kernel.org/r/20231213-riscv-v7-0-a760156a337f@daynix.com

---
Akihiko Odaki (3):
      target/riscv: Remove misa_mxl validation
      target/riscv: Move misa_mxl_max to class
      target/riscv: Validate misa_mxl_max only once

 target/riscv/cpu.h         |   4 +-
 hw/riscv/boot.c            |   3 +-
 target/riscv/cpu.c         | 181 ++++++++++++++++++++++++++-------------------
 target/riscv/gdbstub.c     |  12 ++-
 target/riscv/kvm/kvm-cpu.c |  10 +--
 target/riscv/machine.c     |   7 +-
 target/riscv/tcg/tcg-cpu.c |  44 ++---------
 target/riscv/translate.c   |   3 +-
 8 files changed, 133 insertions(+), 131 deletions(-)
---
base-commit: 0c9d286cf791cdda76fd57e4562e2cb18d4a79e2
change-id: 20231213-riscv-fcc9640986cf

Best regards,
-- 
Akihiko Odaki <akihiko.odaki@daynix.com>
Re: [PATCH v11 0/3] gdbstub and TCG plugin improvements
Posted by Alistair Francis 2 months, 3 weeks ago
On Sat, Feb 3, 2024 at 8:12 PM Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>
> 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.
>
> V6 -> V7:
>   Rebased.
>
> V5 -> V6:
>   Added patch "default-configs: Add TARGET_XML_FILES definition".
>   Rebased.
>
> V4 -> V5:
>   Added patch "hw/riscv: Use misa_mxl instead of misa_mxl_max".
>
> V3 -> V4:
>   Added patch "gdbstub: Check if gdb_regs is NULL".
>
> V2 -> V3:
>   Restored patch sets from the previous version.
>   Rebased to commit 800485762e6564e04e2ab315132d477069562d91.
>
> V1 -> V2:
>   Added patch "target/riscv: Do not allow MXL_RV32 for TARGET_RISCV64".
>   Added patch "target/riscv: Initialize gdb_core_xml_file only once".
>   Dropped patch "target/riscv: Remove misa_mxl validation".
>   Dropped patch "target/riscv: Move misa_mxl_max to class".
>   Dropped patch "target/riscv: Validate misa_mxl_max only once".
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
> Changes in v11:
> - Rebased on: https://github.com/alistair23/qemu/tree/riscv-to-apply.next
> - Link to v10: https://lore.kernel.org/r/20240128-riscv-v10-0-fdbe593976e9@daynix.com
>
> Changes in v10:
> - Dropped patch "hw/riscv: Use misa_mxl instead of misa_mxl_max" due to
>   invalid assumption that the relevant code is only used for kernel
>   loading.
> - Link to v9: https://lore.kernel.org/r/20240115-riscv-v9-0-ff171e1aedc8@daynix.com
>
> Changes in v9:
> - Rebased to commit 977542ded7e6b28d2bc077bcda24568c716e393c.
> - Link to v8: https://lore.kernel.org/r/20231218-riscv-v8-0-c9bf2b1582d7@daynix.com
>
> Changes in v8:
> - Added a more detailed explanation for patch "hw/riscv: Use misa_mxl
>   instead of misa_mxl_max". (Alistair Francis)
> - Link to v7: https://lore.kernel.org/r/20231213-riscv-v7-0-a760156a337f@daynix.com
>
> ---
> Akihiko Odaki (3):
>       target/riscv: Remove misa_mxl validation
>       target/riscv: Move misa_mxl_max to class
>       target/riscv: Validate misa_mxl_max only once

Thanks!

Applied to riscv-to-apply.next

Alistair

>
>  target/riscv/cpu.h         |   4 +-
>  hw/riscv/boot.c            |   3 +-
>  target/riscv/cpu.c         | 181 ++++++++++++++++++++++++++-------------------
>  target/riscv/gdbstub.c     |  12 ++-
>  target/riscv/kvm/kvm-cpu.c |  10 +--
>  target/riscv/machine.c     |   7 +-
>  target/riscv/tcg/tcg-cpu.c |  44 ++---------
>  target/riscv/translate.c   |   3 +-
>  8 files changed, 133 insertions(+), 131 deletions(-)
> ---
> base-commit: 0c9d286cf791cdda76fd57e4562e2cb18d4a79e2
> change-id: 20231213-riscv-fcc9640986cf
>
> Best regards,
> --
> Akihiko Odaki <akihiko.odaki@daynix.com>
>
>
Re: [PATCH v11 0/3] gdbstub and TCG plugin improvements
Posted by Alex Bennée 2 months, 3 weeks ago
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.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH v11 0/3] gdbstub and TCG plugin improvements
Posted by Akihiko Odaki 2 months, 3 weeks ago
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/

Please check them out.

Re: [PATCH v11 0/3] gdbstub and TCG plugin improvements
Posted by Alex Bennée 2 months, 3 weeks ago
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).

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.


>
> Please check them out.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro
Re: [PATCH v11 0/3] gdbstub and TCG plugin improvements
Posted by Akihiko Odaki 2 months, 3 weeks ago
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).

> 
> 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.

Re: [PATCH v11 0/3] gdbstub and TCG plugin improvements
Posted by Alex Bennée 2 months, 3 weeks ago
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 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
Re: [PATCH v11 0/3] gdbstub and TCG plugin improvements
Posted by Akihiko Odaki 2 months, 3 weeks ago

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.

Re: [PATCH v11 0/3] gdbstub and TCG plugin improvements
Posted by Pierrick Bouvier 2 months, 3 weeks ago
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.

>>
>>> 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.
> 
Re: [PATCH v11 0/3] gdbstub and TCG plugin improvements
Posted by Alex Bennée 2 months, 3 weeks ago
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