On 10/22/24 03:56, Alex Bennée wrote: > From: Ilya Leoshkevich <iii(a)linux.ibm.com&gt; > > commit f025692c992c ("accel/tcg: Clear PAGE_WRITE before > translation") > fixed cross-modifying code handling, but did not add a test. The > changed code was further improved recently [1], and I was not sure > whether these modifications were safe (spoiler: they were fine). > > Add a test to make sure there are no regressions. > > [1] > https://lists.gnu.org/archive/html/qemu-devel/2022-09/msg00034.html > > Signed-off-by: Ilya Leoshkevich <iii(a)linux.ibm.com&gt; > Message-Id: <20241001150617.9977-1-iii(a)linux.ibm.com&gt; > Signed-off-by: Alex Bennée <alex.bennee(a)linaro.org&gt; > --- >   tests/tcg/x86_64/cross-modifying-code.c | 80 > +++++++++++++++++++++++++ >   tests/tcg/x86_64/Makefile.target        |  4 ++ >   2 files changed, 84 insertions(+) >   create mode 100644 tests/tcg/x86_64/cross-modifying-code.c > > diff --git a/tests/tcg/x86_64/cross-modifying-code.c > b/tests/tcg/x86_64/cross-modifying-code.c > new file mode 100644 > index 0000000000..2704df6061 > --- /dev/null > +++ b/tests/tcg/x86_64/cross-modifying-code.c > @@ -0,0 +1,80 @@ > +/* > + * Test patching code, running in one thread, from another thread. > + * > + * Intel SDM calls this "cross-modifying code" and recommends a > special > + * sequence, which requires both threads to cooperate. > + * > + * Linux kernel uses a different sequence that does not require > cooperation and > + * involves patching the first byte with int3. > + * > + * Finally, there is user-mode software out there that simply uses > atomics, and > + * that seems to be good enough in practice. Test that QEMU has no > problems > + * with this as well. > + */ > + > +#include <assert.h> > +#include <pthread.h> > +#include <stdbool.h> > +#include <stdlib.h> > + > +void add1_or_nop(long *x); > +asm(".pushsection .rwx,\"awx\",@progbits\n" > +    ".globl add1_or_nop\n" > +    /* addq $0x1,(%rdi) */ > +    "add1_or_nop: .byte 0x48, 0x83, 0x07, 0x01\n" > +    "ret\n" > +    ".popsection\n"); > + > +#define THREAD_WAIT 0 > +#define THREAD_PATCH 1 > +#define THREAD_STOP 2 > + > +static void *thread_func(void *arg) > +{ > +    int val = 0x0026748d; /* nop */ > + > +    while (true) { > +        switch (__atomic_load_n((int *)arg, __ATOMIC_SEQ_CST)) { > +        case THREAD_WAIT: > +            break; > +        case THREAD_PATCH: > +            val = __atomic_exchange_n((int *)&add1_or_nop, val, > +                                      __ATOMIC_SEQ_CST); > +            break; > +        case THREAD_STOP: > +            return NULL; > +        default: > +            assert(false); > +            __builtin_unreachable(); Use g_assert_not_reached() instead. checkpatch emits an error for it now.

On 10/22/24 17:16, Ilya Leoshkevich wrote: > On Tue, 2024-10-22 at 13:36 -0700, Pierrick Bouvier wrote: >> On 10/22/24 03:56, Alex Bennée wrote: >>> From: Ilya Leoshkevich <iii(a)linux.ibm.com&gt; >>> >>> commit f025692c992c ("accel/tcg: Clear PAGE_WRITE before >>> translation") >>> fixed cross-modifying code handling, but did not add a test. The >>> changed code was further improved recently [1], and I was not sure >>> whether these modifications were safe (spoiler: they were fine). >>> >>> Add a test to make sure there are no regressions. >>> >>> [1] >>> https://lists.gnu.org/archive/html/qemu-devel/2022-09/msg00034.html >>> >>> Signed-off-by: Ilya Leoshkevich <iii(a)linux.ibm.com&gt; >>> Message-Id: <20241001150617.9977-1-iii(a)linux.ibm.com&gt; >>> Signed-off-by: Alex Bennée <alex.bennee(a)linaro.org&gt; >>> --- >>>   tests/tcg/x86_64/cross-modifying-code.c | 80 >>> +++++++++++++++++++++++++ >>>   tests/tcg/x86_64/Makefile.target        |  4 ++ >>>   2 files changed, 84 insertions(+) >>>   create mode 100644 tests/tcg/x86_64/cross-modifying-code.c >>> >>> diff --git a/tests/tcg/x86_64/cross-modifying-code.c >>> b/tests/tcg/x86_64/cross-modifying-code.c >>> new file mode 100644 >>> index 0000000000..2704df6061 >>> --- /dev/null >>> +++ b/tests/tcg/x86_64/cross-modifying-code.c >>> @@ -0,0 +1,80 @@ >>> +/* >>> + * Test patching code, running in one thread, from another thread. >>> + * >>> + * Intel SDM calls this "cross-modifying code" and recommends a >>> special >>> + * sequence, which requires both threads to cooperate. >>> + * >>> + * Linux kernel uses a different sequence that does not require >>> cooperation and >>> + * involves patching the first byte with int3. >>> + * >>> + * Finally, there is user-mode software out there that simply uses >>> atomics, and >>> + * that seems to be good enough in practice. Test that QEMU has no >>> problems >>> + * with this as well. >>> + */ >>> + >>> +#include <assert.h> >>> +#include <pthread.h> >>> +#include <stdbool.h> >>> +#include <stdlib.h> >>> + >>> +void add1_or_nop(long *x); >>> +asm(".pushsection .rwx,\"awx\",@progbits\n" >>> +    ".globl add1_or_nop\n" >>> +    /* addq $0x1,(%rdi) */ >>> +    "add1_or_nop: .byte 0x48, 0x83, 0x07, 0x01\n" >>> +    "ret\n" >>> +    ".popsection\n"); >>> + >>> +#define THREAD_WAIT 0 >>> +#define THREAD_PATCH 1 >>> +#define THREAD_STOP 2 >>> + >>> +static void *thread_func(void *arg) >>> +{ >>> +    int val = 0x0026748d; /* nop */ >>> + >>> +    while (true) { >>> +        switch (__atomic_load_n((int *)arg, __ATOMIC_SEQ_CST)) { >>> +        case THREAD_WAIT: >>> +            break; >>> +        case THREAD_PATCH: >>> +            val = __atomic_exchange_n((int *)&add1_or_nop, val, >>> +                                      __ATOMIC_SEQ_CST); >>> +            break; >>> +        case THREAD_STOP: >>> +            return NULL; >>> +        default: >>> +            assert(false); >>> +            __builtin_unreachable(); >> >> Use g_assert_not_reached() instead. >> checkpatch emits an error for it now. > Is there an easy way to include glib from testcases? > It's located using meson, and I can't immediately see how to push the > respective compiler flags to the test Makefiles - this seems to be > currently handled by configure writing to $config_target_mak. > [...] > Sorry you're right, I missed the fact tests don't have the deps we have in QEMU itself. I don't think any test case include any extra dependency for now (and would make it hard to cross compile them too), so it's not worth trying to get the right glib header for this.

I don't now if it will be a problem when merging the series regarding checkpatch, but if it is, we can always replace this by abort, or exit.

> As it is, Reviewed-by: Pierrick Bouvier <pierrick.bouvier(a)linaro.org&gt;

When git fails the rather terse backtrace only indicates it failed without some useful context. Add some to make the log a little more useful. Signed-off-by: Alex Bennée <alex.bennee(a)linaro.org&gt; --- .gitlab-ci.d/check-dco.py | 9 +++++---- .gitlab-ci.d/check-patch.py | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/.gitlab-ci.d/check-dco.py b/.gitlab-ci.d/check-dco.py index 632c8bcce8..d29c580d63 100755 --- a/.gitlab-ci.d/check-dco.py +++ b/.gitlab-ci.d/check-dco.py @@ -19,10 +19,11 @@ reponame = os.path.basename(cwd) repourl = "https://gitlab.com/%s/%s.git" % (namespace, reponame) -subprocess.check_call(["git", "remote", "add", "check-dco", repourl]) -subprocess.check_call(["git", "fetch", "check-dco", "master"], - stdout=subprocess.DEVNULL, - stderr=subprocess.DEVNULL) +print(f"adding upstream git repo @ {repourl}") +subprocess.run(["git", "remote", "add", "check-dco", repourl], + check=True, capture_output=True) +subprocess.run(["git", "fetch", "check-dco", "master"], + check=True, capture_output=True)

Attempting to run the binary asserts when it can't find the XML entry. We can fix it so we don't although I suspect other stuff is broken. Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2580 Signed-off-by: Alex Bennée <alex.bennee(a)linaro.org&gt; --- configs/targets/aarch64_be-linux-user.mak | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configs/targets/aarch64_be-linux-user.mak b/configs/targets/aarch64_be-linux-user.mak index 778d22b2a9..dcef597a80 100644 --- a/configs/targets/aarch64_be-linux-user.mak +++ b/configs/targets/aarch64_be-linux-user.mak @@ -1,7 +1,7 @@ TARGET_ARCH=aarch64 TARGET_BASE_ARCH=arm TARGET_BIG_ENDIAN=y -TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml gdb-xml/aarch64-pauth.xml +TARGET_XML_FILES= gdb-xml/aarch64-core.xml gdb-xml/aarch64-fpu.xml gdb-xml/aarch64-pauth.xml gdb-xml/aarch64-mte.xml TARGET_HAS_BFLT=y CONFIG_SEMIHOSTING=y CONFIG_ARM_COMPATIBLE_SEMIHOSTING=y

We didn't notice breakage of aarch64_be because we don't have any TCG tests for it. However while the existing aarch64 compiler can target big-endian builds no one packages a BE libc. Instead we bang some rocks together to do the most basic of hello world with a nostdlib syscall test. Signed-off-by: Alex Bennée <alex.bennee(a)linaro.org&gt; --- v2 - fix checkpatch complaints --- configure | 5 ++++ tests/tcg/aarch64_be/hello.c | 35 ++++++++++++++++++++++++++++ tests/tcg/Makefile.target | 7 +++++- tests/tcg/aarch64_be/Makefile.target | 17 ++++++++++++++ 4 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 tests/tcg/aarch64_be/hello.c create mode 100644 tests/tcg/aarch64_be/Makefile.target diff --git a/configure b/configure index 72d1a94225..7dd3400ccb 100755 --- a/configure +++ b/configure @@ -1418,6 +1418,7 @@ probe_target_compiler() { target_arch=${1%%-*} case $target_arch in aarch64) container_hosts="x86_64 aarch64" ;; + aarch64_be) container_hosts="x86_64 aarch64" ;; alpha) container_hosts=x86_64 ;; arm) container_hosts="x86_64 aarch64" ;; hexagon) container_hosts=x86_64 ;; @@ -1447,6 +1448,10 @@ probe_target_compiler() { case $target_arch in # debian-all-test-cross architectures + aarch64_be) + container_image=debian-all-test-cross + container_cross_prefix=aarch64-linux-gnu- + ;; hppa|m68k|mips|riscv64|sparc64) container_image=debian-all-test-cross ;; diff --git a/tests/tcg/aarch64_be/hello.c b/tests/tcg/aarch64_be/hello.c new file mode 100644 index 0000000000..a9b2ab45de --- /dev/null +++ b/tests/tcg/aarch64_be/hello.c @@ -0,0 +1,35 @@ +/* + * Non-libc syscall hello world for Aarch64 BE + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#define __NR_write 64 +#define __NR_exit 93 + +int write(int fd, char *buf, int len) +{ + register int x0 __asm__("x0") = fd; + register char *x1 __asm__("x1") = buf; + register int x2 __asm__("x2") = len; + register int x8 __asm__("x8") = __NR_write; + + asm volatile("svc #0" : : "r"(x0), "r"(x1), "r"(x2), "r"(x8)); + + return len; +} + +void exit(int ret) +{ + register int x0 __asm__("x0") = ret; + register int x8 __asm__("x8") = __NR_exit; + + asm volatile("svc #0" : : "r"(x0), "r"(x8)); + __builtin_unreachable(); +} + +void _start(void) +{ + write(1, "Hello World\n", 12); + exit(0); +} diff --git a/tests/tcg/Makefile.target b/tests/tcg/Makefile.target index 2da70b2fcf..9722145b97 100644 --- a/tests/tcg/Makefile.target +++ b/tests/tcg/Makefile.target @@ -103,9 +103,14 @@ ifeq ($(filter %-softmmu, $(TARGET)),) # then the target. If there are common tests shared between # sub-targets (e.g. ARM & AArch64) then it is up to # $(TARGET_NAME)/Makefile.target to include the common parent -# architecture in its VPATH. +# architecture in its VPATH. However some targets are so minimal we +# can't even build the multiarch tests. +ifneq ($(filter $(TARGET_NAME),aarch64_be),) +-include $(SRC_PATH)/tests/tcg/$(TARGET_NAME)/Makefile.target +else -include $(SRC_PATH)/tests/tcg/multiarch/Makefile.target -include $(SRC_PATH)/tests/tcg/$(TARGET_NAME)/Makefile.target +endif # Add the common build options CFLAGS+=-Wall -Werror -O0 -g -fno-strict-aliasing diff --git a/tests/tcg/aarch64_be/Makefile.target b/tests/tcg/aarch64_be/Makefile.target new file mode 100644 index 0000000000..297d2cf71c --- /dev/null +++ b/tests/tcg/aarch64_be/Makefile.target @@ -0,0 +1,17 @@ +# -*- Mode: makefile -*- +# +# A super basic AArch64 BE makefile. As we don't have any big-endian +#l ibc available the best we can do is a basic Hello World. +

+AARCH64BE_SRC=$(SRC_PATH)/tests/tcg/aarch64_be +VPATH += $(AARCH64BE_SRC) + +AARCH64BE_TEST_SRCS=$(notdir $(wildcard $(AARCH64BE_SRC)/*.c)) +AARCH64BE_TESTS=$(AARCH64BE_TEST_SRCS:.c=) +#MULTIARCH_TESTS = $(MULTIARCH_SRCS:.c=) + +# We need to specify big-endian cflags +CFLAGS +=-mbig-endian -ffreestanding +LDFLAGS +=-nostdlib + +TESTS += $(AARCH64BE_TESTS)

From: Gustavo Romero <gustavo.romero(a)linaro.org&gt; Use list and set comprehension to simplify code. Also, gently handle invalid gdb filenames. Signed-off-by: Gustavo Romero <gustavo.romero(a)linaro.org&gt; Message-Id: <20241015145848.387281-1-gustavo.romero(a)linaro.org&gt; Signed-off-by: Alex Bennée <alex.bennee(a)linaro.org&gt; --- scripts/probe-gdb-support.py | 75 +++++++++++++++++++----------------- 1 file changed, 39 insertions(+), 36 deletions(-) diff --git a/scripts/probe-gdb-support.py b/scripts/probe-gdb-support.py index 6dc58d06c7..6bcadce150 100644 --- a/scripts/probe-gdb-support.py +++ b/scripts/probe-gdb-support.py @@ -19,58 +19,61 @@ import argparse import re -from subprocess import check_output, STDOUT +from subprocess import check_output, STDOUT, CalledProcessError +import sys -# mappings from gdb arch to QEMU target -mappings = { - "alpha" : "alpha", +# Mappings from gdb arch to QEMU target +MAP = { + "alpha" : ["alpha"], "aarch64" : ["aarch64", "aarch64_be"], - "armv7": "arm", + "armv7": ["arm"], "armv8-a" : ["aarch64", "aarch64_be"], - "avr" : "avr", + "avr" : ["avr"], # no hexagon in upstream gdb - "hppa1.0" : "hppa", - "i386" : "i386", - "i386:x86-64" : "x86_64", - "Loongarch64" : "loongarch64", - "m68k" : "m68k", - "MicroBlaze" : "microblaze", + "hppa1.0" : ["hppa"], + "i386" : ["i386"], + "i386:x86-64" : ["x86_64"], + "Loongarch64" : ["loongarch64"], + "m68k" : ["m68k"], + "MicroBlaze" : ["microblaze"], "mips:isa64" : ["mips64", "mips64el"], - "or1k" : "or1k", - "powerpc:common" : "ppc", + "or1k" : ["or1k"], + "powerpc:common" : ["ppc"], "powerpc:common64" : ["ppc64", "ppc64le"], - "riscv:rv32" : "riscv32", - "riscv:rv64" : "riscv64", - "s390:64-bit" : "s390x", + "riscv:rv32" : ["riscv32"], + "riscv:rv64" : ["riscv64"], + "s390:64-bit" : ["s390x"], "sh4" : ["sh4", "sh4eb"], - "sparc": "sparc", - "sparc:v8plus": "sparc32plus", - "sparc:v9a" : "sparc64", + "sparc": ["sparc"], + "sparc:v8plus": ["sparc32plus"], + "sparc:v9a" : ["sparc64"], # no tricore in upstream gdb "xtensa" : ["xtensa", "xtensaeb"] } + def do_probe(gdb): - gdb_out = check_output([gdb, - "-ex", "set architecture", - "-ex", "quit"], stderr=STDOUT) + try: + gdb_out = check_output([gdb, + "-ex", "set architecture", + "-ex", "quit"], stderr=STDOUT, encoding="utf-8") + except (OSError) as e: + sys.exit(e) + except CalledProcessError as e: + sys.exit(f'{e}. Output:\n\n{e.output}') + + found_gdb_archs = re.search(r'Valid arguments are (.*)', gdb_out) - m = re.search(r"Valid arguments are (.*)", - gdb_out.decode("utf-8")) + targets = set() + if found_gdb_archs: + gdb_archs = found_gdb_archs.group(1).split(", ") + mapped_gdb_archs = [arch for arch in gdb_archs if arch in MAP] - valid_arches = set() + targets = {target for arch in mapped_gdb_archs for target in MAP[arch]} - if m.group(1): - for arch in m.group(1).split(", "): - if arch in mappings: - mapping = mappings[arch] - if isinstance(mapping, str): - valid_arches.add(mapping) - else: - for entry in mapping: - valid_arches.add(entry) + # QEMU targets + return targets - return valid_arches def main() -> None: parser = argparse.ArgumentParser(description='Probe GDB Architectures')

Make it easier to find where plugin patches are being staged. Signed-off-by: Alex Bennée <alex.bennee(a)linaro.org&gt; --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 81396c9f15..02b8b2dfd6 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -3702,6 +3702,7 @@ F: include/tcg/ TCG Plugins M: Alex Bennée <alex.bennee(a)linaro.org&gt; +T: git https://gitlab.com/stsquad/qemu plugins/next R: Alexandre Iooss <erdnaxe(a)crans.org&gt; R: Mahmoud Mandour <ma.mandourr(a)gmail.com&gt; R: Pierrick Bouvier <pierrick.bouvier(a)linaro.org&gt;

Now gdbstub has gained the ability to extend its command tables we can allow it to trigger plugin callbacks. This is probably most useful for QEMU developers debugging plugins themselves but might be useful for other stuff. Trigger the callback by sending: maintenance packet Qqemu.plugin_cb I've extended the memory plugin to report on the packet. Signed-off-by: Alex Bennée <alex.bennee(a)linaro.org&gt; --- include/qemu/plugin-event.h | 1 + include/qemu/qemu-plugin.h | 16 ++++++++++++++++ plugins/plugin.h | 9 +++++++++ plugins/api.c | 18 ++++++++++++++++++ plugins/core.c | 37 ++++++++++++++++++++++++++++++++++++ tests/tcg/plugins/mem.c | 11 +++++++++-- plugins/qemu-plugins.symbols | 1 + 7 files changed, 91 insertions(+), 2 deletions(-) diff --git a/include/qemu/plugin-event.h b/include/qemu/plugin-event.h index 7056d8427b..d9aa56cf4f 100644 --- a/include/qemu/plugin-event.h +++ b/include/qemu/plugin-event.h @@ -20,6 +20,7 @@ enum qemu_plugin_event { QEMU_PLUGIN_EV_VCPU_SYSCALL_RET, QEMU_PLUGIN_EV_FLUSH, QEMU_PLUGIN_EV_ATEXIT, + QEMU_PLUGIN_EV_GDBSTUB, QEMU_PLUGIN_EV_MAX, /* total number of plugin events we support */ }; diff --git a/include/qemu/qemu-plugin.h b/include/qemu/qemu-plugin.h index 622c9a0232..99c3b365aa 100644 --- a/include/qemu/qemu-plugin.h +++ b/include/qemu/qemu-plugin.h @@ -802,6 +802,22 @@ QEMU_PLUGIN_API void qemu_plugin_register_atexit_cb(qemu_plugin_id_t id, qemu_plugin_udata_cb_t cb, void *userdata); + +/** + * qemu_plugin_register_gdb_cb() - register a gdb callback + * @id: plugin ID + * @cb: callback + * @userdata: user data for callback + * + * When using the gdbstub to debug a guest you can send a command that + * will trigger the callback. This is useful if you want the plugin to + * print out collected state at particular points when debugging a + * program. + */ +QEMU_PLUGIN_API +void qemu_plugin_register_gdb_cb(qemu_plugin_id_t id, + qemu_plugin_udata_cb_t cb, void *userdata); + /* returns how many vcpus were started at this point */ int qemu_plugin_num_vcpus(void); diff --git a/plugins/plugin.h b/plugins/plugin.h index 30e2299a54..f37667c9fb 100644 --- a/plugins/plugin.h +++ b/plugins/plugin.h @@ -118,4 +118,13 @@ struct qemu_plugin_scoreboard *plugin_scoreboard_new(size_t element_size); void plugin_scoreboard_free(struct qemu_plugin_scoreboard *score); +/** + * plugin_register_gdbstub_commands - register gdbstub commands + * + * This should only be called once to register gdbstub commands so we + * can trigger callbacks if needed. + */ +void plugin_register_gdbstub_commands(void); + + #endif /* PLUGIN_H */ diff --git a/plugins/api.c b/plugins/api.c index 24ea64e2de..62141616f4 100644 --- a/plugins/api.c +++ b/plugins/api.c @@ -681,3 +681,21 @@ void qemu_plugin_update_ns(const void *handle, int64_t new_time) } #endif } + +/* + * gdbstub hooks + */ + +static bool gdbstub_callbacks; + +void qemu_plugin_register_gdb_cb(qemu_plugin_id_t id, + qemu_plugin_udata_cb_t cb, + void *udata) +{ + plugin_register_cb_udata(id, QEMU_PLUGIN_EV_GDBSTUB, cb, udata); + + if (!gdbstub_callbacks) { + plugin_register_gdbstub_commands(); + gdbstub_callbacks = true; + } +} diff --git a/plugins/core.c b/plugins/core.c index bb105e8e68..e7fce08799 100644 --- a/plugins/core.c +++ b/plugins/core.c @@ -23,6 +23,7 @@ #include "qemu/xxhash.h" #include "qemu/rcu.h" #include "hw/core/cpu.h" +#include "gdbstub/commands.h" #include "exec/exec-all.h" #include "exec/tb-flush.h" @@ -147,6 +148,7 @@ static void plugin_cb__udata(enum qemu_plugin_event ev) switch (ev) { case QEMU_PLUGIN_EV_ATEXIT: + case QEMU_PLUGIN_EV_GDBSTUB: QLIST_FOREACH_SAFE_RCU(cb, &plugin.cb_lists[ev], entry, next) { qemu_plugin_udata_cb_t func = cb->f.udata; @@ -768,3 +770,38 @@ void plugin_scoreboard_free(struct qemu_plugin_scoreboard *score) g_array_free(score->data, TRUE); g_free(score); } + +/* + * gdbstub integration + */ + +static void handle_plugin_cb(GArray *params, void *user_ctx) +{ + plugin_cb__udata(QEMU_PLUGIN_EV_GDBSTUB); + gdb_put_packet("OK"); +} + +enum Command { + PluginCallback, + NUM_GDB_CMDS +}; + +static const GdbCmdParseEntry cmd_handler_table[NUM_GDB_CMDS] = { + [PluginCallback] = { + .handler = handle_plugin_cb, + .cmd_startswith = true, + .cmd = "qemu.plugin_cb", + .schema = "s?", + }, +}; + +void plugin_register_gdbstub_commands(void) +{ + g_autoptr(GPtrArray) set_table = g_ptr_array_new(); + + for (int i = 0; i < NUM_GDB_CMDS; i++) { + g_ptr_array_add(set_table, (gpointer) &cmd_handler_table[PluginCallback]); + } + + gdb_extend_set_table(set_table); +} diff --git a/tests/tcg/plugins/mem.c b/tests/tcg/plugins/mem.c index b0fa8a9f27..d416d92fc2 100644 --- a/tests/tcg/plugins/mem.c +++ b/tests/tcg/plugins/mem.c @@ -75,8 +75,7 @@ static gint addr_order(gconstpointer a, gconstpointer b) return na->region_address > nb->region_address ? 1 : -1; } - -static void plugin_exit(qemu_plugin_id_t id, void *p) +static void plugin_report(qemu_plugin_id_t id, void *p) { g_autoptr(GString) out = g_string_new(""); @@ -90,6 +89,7 @@ static void plugin_exit(qemu_plugin_id_t id, void *p) } qemu_plugin_outs(out->str); + g_mutex_lock(&lock); if (do_region_summary) { GList *counts = g_hash_table_get_values(regions); @@ -114,6 +114,12 @@ static void plugin_exit(qemu_plugin_id_t id, void *p) qemu_plugin_outs(out->str); } + g_mutex_unlock(&lock); +} + +static void plugin_exit(qemu_plugin_id_t id, void *p) +{ + plugin_report(id, p); qemu_plugin_scoreboard_free(counts); } @@ -400,6 +406,7 @@ QEMU_PLUGIN_EXPORT int qemu_plugin_install(qemu_plugin_id_t id, counts, CPUCount, mem_count); io_count = qemu_plugin_scoreboard_u64_in_struct(counts, CPUCount, io_count); qemu_plugin_register_vcpu_tb_trans_cb(id, vcpu_tb_trans); + qemu_plugin_register_gdb_cb(id, plugin_report, NULL); qemu_plugin_register_atexit_cb(id, plugin_exit, NULL); return 0; } diff --git a/plugins/qemu-plugins.symbols b/plugins/qemu-plugins.symbols index 032661f9ea..d272e8e0f3 100644 --- a/plugins/qemu-plugins.symbols +++ b/plugins/qemu-plugins.symbols @@ -25,6 +25,7 @@ qemu_plugin_read_register; qemu_plugin_register_atexit_cb; qemu_plugin_register_flush_cb; + qemu_plugin_register_gdb_cb; qemu_plugin_register_vcpu_exit_cb; qemu_plugin_register_vcpu_idle_cb; qemu_plugin_register_vcpu_init_cb;

From: Pierrick Bouvier <pierrick.bouvier(a)linaro.org&gt; Tried to unify this meson.build with tests/tcg/plugins/meson.build but the resulting modules are not output in the right directory. Originally proposed by Anton Kochkov, thank you! Solves: https://gitlab.com/qemu-project/qemu/-/issues/1710 Signed-off-by: Pierrick Bouvier <pierrick.bouvier(a)linaro.org&gt; Message-Id: <20240925204845.390689-2-pierrick.bouvier(a)linaro.org&gt; Signed-off-by: Alex Bennée <alex.bennee(a)linaro.org&gt; --- meson.build | 4 ++++ contrib/plugins/meson.build | 23 +++++++++++++++++++++++ 2 files changed, 27 insertions(+) create mode 100644 contrib/plugins/meson.build diff --git a/meson.build b/meson.build index bdd67a2d6d..3ea03c451b 100644 --- a/meson.build +++ b/meson.build @@ -3678,6 +3678,10 @@ subdir('accel') subdir('plugins') subdir('ebpf') +if 'CONFIG_TCG' in config_all_accel + subdir('contrib/plugins') +endif + common_user_inc = [] subdir('common-user') diff --git a/contrib/plugins/meson.build b/contrib/plugins/meson.build new file mode 100644 index 0000000000..a0e026d25e --- /dev/null +++ b/contrib/plugins/meson.build @@ -0,0 +1,23 @@ +t = [] +if get_option('plugins') + foreach i : ['cache', 'drcov', 'execlog', 'hotblocks', 'hotpages', 'howvec', + 'hwprofile', 'ips', 'lockstep', 'stoptrigger']

+ if host_os == 'windows' + t += shared_module(i, files(i + '.c') + 'win32_linker.c', + include_directories: '../../include/qemu', + link_depends: [win32_qemu_plugin_api_lib], + link_args: ['-Lplugins', '-lqemu_plugin_api'], + dependencies: glib) + + else + t += shared_module(i, files(i + '.c'), + include_directories: '../../include/qemu', + dependencies: glib) + endif + endforeach +endif +if t.length() > 0 + alias_target('contrib-plugins', t) +else + run_target('contrib-plugins', command: find_program('true')) +endif

Pierrick Bouvier <pierrick.bouvier(a)linaro.org&gt; writes: > On 10/22/24 03:56, Alex Bennée wrote: >> From: Pierrick Bouvier <pierrick.bouvier(a)linaro.org&gt; >> Tried to unify this meson.build with tests/tcg/plugins/meson.build >> but >> the resulting modules are not output in the right directory. >> Originally proposed by Anton Kochkov, thank you! >> Solves: https://gitlab.com/qemu-project/qemu/-/issues/1710 >> Signed-off-by: Pierrick Bouvier <pierrick.bouvier(a)linaro.org&gt; >> Message-Id: <20240925204845.390689-2-pierrick.bouvier(a)linaro.org&gt; >> Signed-off-by: Alex Bennée <alex.bennee(a)linaro.org&gt; >> --- >> meson.build | 4 ++++ >> contrib/plugins/meson.build | 23 +++++++++++++++++++++++ >> 2 files changed, 27 insertions(+) >> create mode 100644 contrib/plugins/meson.build >> diff --git a/meson.build b/meson.build >> index bdd67a2d6d..3ea03c451b 100644 >> --- a/meson.build >> +++ b/meson.build >> @@ -3678,6 +3678,10 @@ subdir('accel') >> subdir('plugins') >> subdir('ebpf') >> +if 'CONFIG_TCG' in config_all_accel >> + subdir('contrib/plugins') >> +endif >> + >> common_user_inc = [] >> subdir('common-user') >> diff --git a/contrib/plugins/meson.build b/contrib/plugins/meson.build >> new file mode 100644 >> index 0000000000..a0e026d25e >> --- /dev/null >> +++ b/contrib/plugins/meson.build >> @@ -0,0 +1,23 @@ >> +t = [] >> +if get_option('plugins') >> + foreach i : ['cache', 'drcov', 'execlog', 'hotblocks', 'hotpages', 'howvec', >> + 'hwprofile', 'ips', 'lockstep', 'stoptrigger'] > > lockstep does not build under Windows (it uses sockets), so it should > be conditionnally not built on this platform. > @Alex, if you feel like modifying this, you can. If not, you can drop > the meson build patches from this series to not block it. I'll drop from the PR and let you re-submit.