From: Akihiko Odaki <akihiko.odaki@daynix.com>
To: "Alex Bennée" <alex.bennee@linaro.org>
Cc: qemu-devel@nongnu.org, "Nicholas Piggin" <npiggin@gmail.com>,
"Liu Zhiwei" <zhiwei_liu@linux.alibaba.com>,
"Cédric Le Goater" <clg@kaod.org>,
"Peter Maydell" <peter.maydell@linaro.org>,
qemu-riscv@nongnu.org, "Michael Rolnik" <mrolnik@gmail.com>,
"Song Gao" <gaosong@loongson.cn>,
"Alexandre Iooss" <erdnaxe@crans.org>,
"Marcel Apfelbaum" <marcel.apfelbaum@gmail.com>,
"David Hildenbrand" <david@redhat.com>,
"Pierrick Bouvier" <pierrick.bouvier@linaro.org>,
"John Snow" <jsnow@redhat.com>,
"Eduardo Habkost" <eduardo@habkost.net>,
"Mahmoud Mandour" <ma.mandourr@gmail.com>,
qemu-arm@nongnu.org, "Philippe Mathieu-Daudé" <philmd@linaro.org>,
"Paolo Bonzini" <pbonzini@redhat.com>,
"Cleber Rosa" <crosa@redhat.com>,
qemu-ppc@nongnu.org,
"Richard Henderson" <richard.henderson@linaro.org>,
qemu-s390x@nongnu.org,
"Daniel Henrique Barboza" <dbarboza@ventanamicro.com>,
"Thomas Huth" <thuth@redhat.com>,
"Yoshinori Sato" <ysato@users.sourceforge.jp>,
"Yanan Wang" <wangyanan55@huawei.com>,
"Brian Cain" <bcain@quicinc.com>,
"Ilya Leoshkevich" <iii@linux.ibm.com>,
"Weiwei Li" <liwei1518@gmail.com>,
"Edgar E. Iglesias" <edgar.iglesias@gmail.com>,
"Bin Meng" <bin.meng@windriver.com>,
"Laurent Vivier" <laurent@vivier.eu>,
"Palmer Dabbelt" <palmer@dabbelt.com>,
"Daniel Henrique Barboza" <danielhb413@gmail.com>,
"Alistair Francis" <alistair.francis@wdc.com>
Subject: Re: [PATCH v3 21/27] plugins: add an API to read registers
Date: Tue, 27 Feb 2024 19:58:43 +0900 [thread overview]
Message-ID: <20e4981d-6378-4293-8625-7c24bb499f41@daynix.com> (raw)
In-Reply-To: <8734tedwun.fsf@draig.linaro.org>
On 2024/02/27 19:29, Alex Bennée wrote:
> 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.
I'm not sure why you would want to short-circuiting array iteration.
Iterating an empty array is often simpler.
In any case, the same logic also applies to gdb_get_register_list(), so
you should change gdb_get_register_list() to return NULL instead of an
empty array if you think that's more reasonable.
next prev parent reply other threads:[~2024-02-27 10:59 UTC|newest]
Thread overview: 33+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-26 16:56 [PATCH v3 00/27] maintainer updates for 9.0 pre-PR (tests, plugin register support) Alex Bennée
2024-02-26 16:56 ` [PATCH v3 01/27] tests/tcg: update licenses to GPLv2 as intended Alex Bennée
2024-02-26 16:56 ` [PATCH v3 02/27] tests/tcg: bump TCG test timeout to 120s Alex Bennée
2024-02-26 16:56 ` [PATCH v3 03/27] target/arm: Use GDBFeature for dynamic XML Alex Bennée
2024-02-26 16:56 ` [PATCH v3 04/27] target/ppc: " Alex Bennée
2024-02-26 16:56 ` [PATCH v3 05/27] target/riscv: " Alex Bennée
2024-02-26 16:56 ` [PATCH v3 06/27] gdbstub: Use GDBFeature for gdb_register_coprocessor Alex Bennée
2024-02-26 16:56 ` [PATCH v3 07/27] gdbstub: Use GDBFeature for GDBRegisterState Alex Bennée
2024-02-26 16:56 ` [PATCH v3 08/27] gdbstub: Change gdb_get_reg_cb and gdb_set_reg_cb Alex Bennée
2024-02-26 16:56 ` [PATCH v3 09/27] gdbstub: Simplify XML lookup Alex Bennée
2024-02-26 16:56 ` [PATCH v3 10/27] gdbstub: Infer number of core registers from XML Alex Bennée
2024-02-26 16:56 ` [PATCH v3 11/27] hw/core/cpu: Remove gdb_get_dynamic_xml member Alex Bennée
2024-02-26 16:56 ` [PATCH v3 12/27] gdbstub: Add members to identify registers to GDBFeature Alex Bennée
2024-02-26 16:56 ` [PATCH v3 13/27] plugins: remove previous n_vcpus functions from API Alex Bennée
2024-02-26 16:56 ` [PATCH v3 14/27] plugins: add qemu_plugin_num_vcpus function Alex Bennée
2024-02-26 16:56 ` [PATCH v3 15/27] plugins: fix order of init/idle/resume callback Alex Bennée
2024-02-26 16:56 ` [PATCH v3 16/27] linux-user: ensure nios2 processes queued work Alex Bennée
2024-02-26 16:56 ` [PATCH v3 17/27] cpu: call plugin init hook asynchronously Alex Bennée
2024-02-26 16:56 ` [PATCH v3 18/27] plugins: Use different helpers when reading registers Alex Bennée
2024-02-26 16:56 ` [PATCH v3 19/27] gdbstub: expose api to find registers Alex Bennée
2024-02-26 16:56 ` [PATCH v3 20/27] plugins: create CPUPluginState and migrate plugin_mask Alex Bennée
2024-02-26 16:56 ` [PATCH v3 21/27] plugins: add an API to read registers Alex Bennée
2024-02-27 7:47 ` Akihiko Odaki
2024-02-27 10:08 ` Alex Bennée
2024-02-27 10:15 ` Akihiko Odaki
2024-02-27 10:29 ` Alex Bennée
2024-02-27 10:58 ` Akihiko Odaki [this message]
2024-02-26 16:56 ` [PATCH v3 22/27] tests/tcg: expand insn test case to exercise register API Alex Bennée
2024-02-26 16:56 ` [PATCH v3 23/27] contrib/plugins: fix imatch Alex Bennée
2024-02-26 16:56 ` [PATCH v3 24/27] contrib/plugins: extend execlog to track register changes Alex Bennée
2024-02-26 16:56 ` [PATCH v3 25/27] docs/devel: lift example and plugin API sections up Alex Bennée
2024-02-26 16:56 ` [PATCH v3 26/27] docs/devel: document some plugin assumptions Alex Bennée
2024-02-26 16:56 ` [PATCH v3 27/27] docs/devel: plugins can trigger a tb flush Alex Bennée
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20e4981d-6378-4293-8625-7c24bb499f41@daynix.com \
--to=akihiko.odaki@daynix.com \
--cc=alex.bennee@linaro.org \
--cc=alistair.francis@wdc.com \
--cc=bcain@quicinc.com \
--cc=bin.meng@windriver.com \
--cc=clg@kaod.org \
--cc=crosa@redhat.com \
--cc=danielhb413@gmail.com \
--cc=david@redhat.com \
--cc=dbarboza@ventanamicro.com \
--cc=edgar.iglesias@gmail.com \
--cc=eduardo@habkost.net \
--cc=erdnaxe@crans.org \
--cc=gaosong@loongson.cn \
--cc=iii@linux.ibm.com \
--cc=jsnow@redhat.com \
--cc=laurent@vivier.eu \
--cc=liwei1518@gmail.com \
--cc=ma.mandourr@gmail.com \
--cc=marcel.apfelbaum@gmail.com \
--cc=mrolnik@gmail.com \
--cc=npiggin@gmail.com \
--cc=palmer@dabbelt.com \
--cc=pbonzini@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=philmd@linaro.org \
--cc=pierrick.bouvier@linaro.org \
--cc=qemu-arm@nongnu.org \
--cc=qemu-devel@nongnu.org \
--cc=qemu-ppc@nongnu.org \
--cc=qemu-riscv@nongnu.org \
--cc=qemu-s390x@nongnu.org \
--cc=richard.henderson@linaro.org \
--cc=thuth@redhat.com \
--cc=wangyanan55@huawei.com \
--cc=ysato@users.sourceforge.jp \
--cc=zhiwei_liu@linux.alibaba.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).