qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins)
@ 2024-10-22 10:55 Alex Bennée
  2024-10-22 10:55 ` [PATCH v2 01/20] tests/docker: Fix microblaze atomics Alex Bennée
                   ` (19 more replies)
  0 siblings, 20 replies; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

This is an aggregation of three of my maintainer trees which you can
also get from their respective branches (testing/next, gdbstub/next
and plugins/next). I didn't include the plugins on the last post as I
hadn't had a chance to do my sweep through patches before travelling.
I've also updated MAINTAINERS to point at my next trees.

For testing we have mostly tweaks and cleanups. I've included some
tracepoints tweaks for cpu_loop_exit_atomic purely as there was no
where else to but it. There are also some cleanups to the tsan support
from Pierrick. The mipsel tweaks have already been applied directly to
the tree.

For gdbstub more cleanups as well as fixing some gdbstub breakage of
the untested aarch64-be linux-user target. I've added a very basic
some test to prevent silly regressions in the future.

For plugins again more cleanups. The GDB trigger patch will probably
not get merged and should be considered an experimental hack for now.

The following still need review:

  plugins: add ability to register a GDB triggered callback
  tests/tcg: enable basic testing for aarch64_be-linux-user
  config/targets: update aarch64_be-linux-user gdb XML list
  MAINTAINERS: mention my gdbstub/next tree
  gitlab: make check-[dco|patch] a little more verbose
  scripts/ci: remove architecture checks for build-environment updates
  MAINTAINERS: mention my testing/next tree
  tests/docker: add NOFETCH env variable for testing
  MAINTAINERS: mention my plugins/next tree

Alex Bennée (10):
  tests/docker: add NOFETCH env variable for testing
  MAINTAINERS: mention my testing/next tree
  scripts/ci: remove architecture checks for build-environment updates
  accel/tcg: add tracepoints for cpu_loop_exit_atomic
  gitlab: make check-[dco|patch] a little more verbose
  MAINTAINERS: mention my gdbstub/next tree
  config/targets: update aarch64_be-linux-user gdb XML list
  tests/tcg: enable basic testing for aarch64_be-linux-user
  MAINTAINERS: mention my plugins/next tree
  plugins: add ability to register a GDB triggered callback

Gustavo Romero (2):
  tests/tcg/aarch64: Use raw strings for regexes in test-mte.py
  testing: Enhance gdb probe script

Ilya Leoshkevich (2):
  tests/docker: Fix microblaze atomics
  tests/tcg/x86_64: Add cross-modifying code test

Pierrick Bouvier (6):
  meson: hide tsan related warnings
  docs/devel: update tsan build documentation
  dockerfiles: fix default targets for debian-loongarch-cross
  meson: build contrib/plugins with meson
  contrib/plugins: remove Makefile for contrib/plugins
  plugins: fix qemu_plugin_reset

 MAINTAINERS                                   |  3 +
 docs/devel/testing/main.rst                   | 26 +++++-
 configure                                     | 23 ++---
 Makefile                                      | 10 ---
 configs/targets/aarch64_be-linux-user.mak     |  2 +-
 meson.build                                   | 14 ++-
 include/qemu/plugin-event.h                   |  1 +
 include/qemu/qemu-plugin.h                    | 16 ++++
 plugins/plugin.h                              |  9 ++
 accel/tcg/plugin-gen.c                        |  4 +
 accel/tcg/user-exec.c                         |  2 +-
 plugins/api.c                                 | 18 ++++
 plugins/core.c                                | 37 ++++++++
 tests/tcg/aarch64_be/hello.c                  | 35 ++++++++
 tests/tcg/plugins/mem.c                       | 11 ++-
 tests/tcg/x86_64/cross-modifying-code.c       | 80 +++++++++++++++++
 accel/tcg/ldst_atomicity.c.inc                |  9 ++
 .gitlab-ci.d/check-dco.py                     |  9 +-
 .gitlab-ci.d/check-patch.py                   |  9 +-
 accel/tcg/trace-events                        | 12 +++
 contrib/plugins/Makefile                      | 87 -------------------
 contrib/plugins/meson.build                   | 23 +++++
 plugins/qemu-plugins.symbols                  |  1 +
 scripts/ci/setup/ubuntu/build-environment.yml |  2 -
 scripts/probe-gdb-support.py                  | 75 ++++++++--------
 tests/docker/Makefile.include                 |  5 +-
 .../dockerfiles/debian-loongarch-cross.docker |  4 +-
 .../build-toolchain.sh                        |  8 ++
 .../dockerfiles/debian-toolchain.docker       |  7 ++
 tests/tcg/Makefile.target                     |  7 +-
 tests/tcg/aarch64/gdbstub/test-mte.py         |  4 +-
 tests/tcg/aarch64_be/Makefile.target          | 17 ++++
 tests/tcg/x86_64/Makefile.target              |  4 +
 33 files changed, 397 insertions(+), 177 deletions(-)
 create mode 100644 tests/tcg/aarch64_be/hello.c
 create mode 100644 tests/tcg/x86_64/cross-modifying-code.c
 delete mode 100644 contrib/plugins/Makefile
 create mode 100644 contrib/plugins/meson.build
 create mode 100644 tests/tcg/aarch64_be/Makefile.target

-- 
2.39.5



^ permalink raw reply	[flat|nested] 43+ messages in thread

* [PATCH v2 01/20] tests/docker: Fix microblaze atomics
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
@ 2024-10-22 10:55 ` Alex Bennée
  2024-10-22 20:30   ` Pierrick Bouvier
  2024-10-22 10:55 ` [PATCH v2 02/20] tests/docker: add NOFETCH env variable for testing Alex Bennée
                   ` (18 subsequent siblings)
  19 siblings, 1 reply; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini, Ilya Leoshkevich

From: Ilya Leoshkevich <iii@linux.ibm.com>

GCC produces invalid code for microblaze atomics.

The fix is unfortunately not upstream, so fetch it from an external
location and apply it locally.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
Message-Id: <20240919152308.10440-1-iii@linux.ibm.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 .../debian-microblaze-cross.d/build-toolchain.sh          | 8 ++++++++
 tests/docker/dockerfiles/debian-toolchain.docker          | 7 +++++++
 2 files changed, 15 insertions(+)

diff --git a/tests/docker/dockerfiles/debian-microblaze-cross.d/build-toolchain.sh b/tests/docker/dockerfiles/debian-microblaze-cross.d/build-toolchain.sh
index 23ec0aa9a7..c5cd0aa931 100755
--- a/tests/docker/dockerfiles/debian-microblaze-cross.d/build-toolchain.sh
+++ b/tests/docker/dockerfiles/debian-microblaze-cross.d/build-toolchain.sh
@@ -10,6 +10,8 @@ TOOLCHAIN_INSTALL=/usr/local
 TOOLCHAIN_BIN=${TOOLCHAIN_INSTALL}/bin
 CROSS_SYSROOT=${TOOLCHAIN_INSTALL}/$TARGET/sys-root
 
+GCC_PATCH0_URL=https://raw.githubusercontent.com/Xilinx/meta-xilinx/refs/tags/xlnx-rel-v2024.1/meta-microblaze/recipes-devtools/gcc/gcc-12/0009-Patch-microblaze-Fix-atomic-boolean-return-value.patch
+
 export PATH=${TOOLCHAIN_BIN}:$PATH
 
 #
@@ -31,6 +33,12 @@ mv gcc-11.2.0 src-gcc
 mv musl-1.2.2 src-musl
 mv linux-5.10.70 src-linux
 
+#
+# Patch gcc
+#
+
+wget -O - ${GCC_PATCH0_URL} | patch -d src-gcc -p1
+
 mkdir -p bld-hdr bld-binu bld-gcc bld-musl
 mkdir -p ${CROSS_SYSROOT}/usr/include
 
diff --git a/tests/docker/dockerfiles/debian-toolchain.docker b/tests/docker/dockerfiles/debian-toolchain.docker
index 687a97fec4..ab4ce29533 100644
--- a/tests/docker/dockerfiles/debian-toolchain.docker
+++ b/tests/docker/dockerfiles/debian-toolchain.docker
@@ -10,6 +10,8 @@ FROM docker.io/library/debian:11-slim
 # ??? The build-dep isn't working, missing a number of
 # minimal build dependiencies, e.g. libmpc.
 
+RUN sed 's/^deb /deb-src /' </etc/apt/sources.list >/etc/apt/sources.list.d/deb-src.list
+
 RUN apt update && \
     DEBIAN_FRONTEND=noninteractive apt install -yy eatmydata && \
     DEBIAN_FRONTEND=noninteractive eatmydata \
@@ -33,6 +35,11 @@ RUN cd /root && ./build-toolchain.sh
 # and the build trees by restoring the original image,
 # then copying the built toolchain from stage 0.
 FROM docker.io/library/debian:11-slim
+RUN apt update && \
+    DEBIAN_FRONTEND=noninteractive apt install -yy eatmydata && \
+    DEBIAN_FRONTEND=noninteractive eatmydata \
+    apt install -y --no-install-recommends \
+        libmpc3
 COPY --from=0 /usr/local /usr/local
 # As a final step configure the user (if env is defined)
 ARG USER
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 02/20] tests/docker: add NOFETCH env variable for testing
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
  2024-10-22 10:55 ` [PATCH v2 01/20] tests/docker: Fix microblaze atomics Alex Bennée
@ 2024-10-22 10:55 ` Alex Bennée
  2024-10-22 20:31   ` Pierrick Bouvier
  2024-10-22 10:55 ` [PATCH v2 03/20] MAINTAINERS: mention my testing/next tree Alex Bennée
                   ` (17 subsequent siblings)
  19 siblings, 1 reply; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

Testing non-auto built docker containers (i.e. custom built compilers)
is a bit fiddly as you couldn't continue a build with a previously
locally built container. While you can play games with REGISTRY its
simpler to allow a NOFETCH that will go through the cached build
process when you run the tests.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/docker/Makefile.include | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
index 681feae744..fead7d3abe 100644
--- a/tests/docker/Makefile.include
+++ b/tests/docker/Makefile.include
@@ -92,10 +92,10 @@ endif
 docker-image-alpine: NOUSER=1
 
 debian-toolchain-run = \
-	$(if $(NOCACHE), 						\
+	$(if $(NOCACHE)$(NOFETCH),					\
 		$(call quiet-command,					\
 			$(DOCKER_SCRIPT) build -t qemu/$1 -f $< 	\
-			$(if $V,,--quiet) --no-cache 			\
+			$(if $V,,--quiet) $(if $(NOCACHE),--no-cache)	\
 			--registry $(DOCKER_REGISTRY) --extra-files	\
 			$(DOCKER_FILES_DIR)/$1.d/build-toolchain.sh,	\
 			"BUILD", $1),				        \
@@ -177,6 +177,7 @@ docker:
 	@echo '    NETWORK=$$BACKEND     Enable virtual network interface with $$BACKEND.'
 	@echo '    NOUSER=1             Define to disable adding current user to containers passwd.'
 	@echo '    NOCACHE=1            Ignore cache when build images.'
+	@echo '    NOFETCH=1            Do not fetch from the registry.'
 	@echo '    EXECUTABLE=<path>    Include executable in image.'
 	@echo '    EXTRA_FILES="<path> [... <path>]"'
 	@echo '                         Include extra files in image.'
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 03/20] MAINTAINERS: mention my testing/next tree
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
  2024-10-22 10:55 ` [PATCH v2 01/20] tests/docker: Fix microblaze atomics Alex Bennée
  2024-10-22 10:55 ` [PATCH v2 02/20] tests/docker: add NOFETCH env variable for testing Alex Bennée
@ 2024-10-22 10:55 ` Alex Bennée
  2024-10-22 11:20   ` Thomas Huth
  2024-10-22 10:55 ` [PATCH v2 04/20] meson: hide tsan related warnings Alex Bennée
                   ` (16 subsequent siblings)
  19 siblings, 1 reply; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

I put it under my name as there may be other maintainer testing trees
as well.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index c21d6a2f9e..b84787ae1b 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4074,6 +4074,7 @@ Build and test automation
 -------------------------
 Build and test automation, general continuous integration
 M: Alex Bennée <alex.bennee@linaro.org>
+T: git https://gitlab.com/stsquad/qemu testing/next
 M: Philippe Mathieu-Daudé <philmd@linaro.org>
 M: Thomas Huth <thuth@redhat.com>
 R: Wainer dos Santos Moschetta <wainersm@redhat.com>
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 04/20] meson: hide tsan related warnings
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
                   ` (2 preceding siblings ...)
  2024-10-22 10:55 ` [PATCH v2 03/20] MAINTAINERS: mention my testing/next tree Alex Bennée
@ 2024-10-22 10:55 ` Alex Bennée
  2024-10-22 10:55 ` [PATCH v2 05/20] docs/devel: update tsan build documentation Alex Bennée
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

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

When building with gcc-12 -fsanitize=thread, gcc reports some
constructions not supported with tsan.
Found on debian stable.

qemu/include/qemu/atomic.h:36:52: error: ‘atomic_thread_fence’ is not supported with ‘-fsanitize=thread’ [-Werror=tsan]
   36 | #define smp_mb()                     ({ barrier(); __atomic_thread_fence(__ATOMIC_SEQ_CST); })
      |                                                    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20240910174013.1433331-2-pierrick.bouvier@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 meson.build | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/meson.build b/meson.build
index d26690ce20..bdd67a2d6d 100644
--- a/meson.build
+++ b/meson.build
@@ -538,7 +538,15 @@ if get_option('tsan')
                          prefix: '#include <sanitizer/tsan_interface.h>')
     error('Cannot enable TSAN due to missing fiber annotation interface')
   endif
-  qemu_cflags = ['-fsanitize=thread'] + qemu_cflags
+  tsan_warn_suppress = []
+  # gcc (>=11) will report constructions not supported by tsan:
+  # "error: ‘atomic_thread_fence’ is not supported with ‘-fsanitize=thread’"
+  # https://gcc.gnu.org/gcc-11/changes.html
+  # However, clang does not support this warning and this triggers an error.
+  if cc.has_argument('-Wno-tsan')
+    tsan_warn_suppress = ['-Wno-tsan']
+  endif
+  qemu_cflags = ['-fsanitize=thread'] + tsan_warn_suppress + qemu_cflags
   qemu_ldflags = ['-fsanitize=thread'] + qemu_ldflags
 endif
 
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 05/20] docs/devel: update tsan build documentation
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
                   ` (3 preceding siblings ...)
  2024-10-22 10:55 ` [PATCH v2 04/20] meson: hide tsan related warnings Alex Bennée
@ 2024-10-22 10:55 ` Alex Bennée
  2024-10-22 10:56 ` [PATCH v2 06/20] scripts/ci: remove architecture checks for build-environment updates Alex Bennée
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:55 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

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

Mention it's now possible to build with gcc, instead of clang, and
explain how to build a sanitized glib version.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20240910174013.1433331-4-pierrick.bouvier@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 docs/devel/testing/main.rst | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/docs/devel/testing/main.rst b/docs/devel/testing/main.rst
index 09725e8ea9..91f4dc61fb 100644
--- a/docs/devel/testing/main.rst
+++ b/docs/devel/testing/main.rst
@@ -628,20 +628,38 @@ Building and Testing with TSan
 It is possible to build and test with TSan, with a few additional steps.
 These steps are normally done automatically in the docker.
 
-There is a one time patch needed in clang-9 or clang-10 at this time:
+TSan is supported for clang and gcc.
+One particularity of sanitizers is that all the code, including shared objects
+dependencies, should be built with it.
+In the case of TSan, any synchronization primitive from glib (GMutex for
+instance) will not be recognized, and will lead to false positives.
+
+To build a tsan version of glib:
 
 .. code::
 
-  sed -i 's/^const/static const/g' \
-      /usr/lib/llvm-10/lib/clang/10.0.0/include/sanitizer/tsan_interface.h
+   $ git clone --depth=1 --branch=2.81.0 https://github.com/GNOME/glib.git
+   $ cd glib
+   $ CFLAGS="-O2 -g -fsanitize=thread" meson build
+   $ ninja -C build
 
 To configure the build for TSan:
 
 .. code::
 
-  ../configure --enable-tsan --cc=clang-10 --cxx=clang++-10 \
+  ../configure --enable-tsan \
                --disable-werror --extra-cflags="-O0"
 
+When executing qemu, don't forget to point to tsan glib:
+
+.. code::
+
+   $ glib_dir=/path/to/glib
+   $ export LD_LIBRARY_PATH=$glib_dir/build/gio:$glib_dir/build/glib:$glib_dir/build/gmodule:$glib_dir/build/gobject:$glib_dir/build/gthread
+   # check correct version is used
+   $ ldd build/qemu-x86_64 | grep glib
+   $ qemu-system-x86_64 ...
+
 The runtime behavior of TSAN is controlled by the TSAN_OPTIONS environment
 variable.
 
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 06/20] scripts/ci: remove architecture checks for build-environment updates
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
                   ` (4 preceding siblings ...)
  2024-10-22 10:55 ` [PATCH v2 05/20] docs/devel: update tsan build documentation Alex Bennée
@ 2024-10-22 10:56 ` Alex Bennée
  2024-10-22 20:32   ` Pierrick Bouvier
  2024-10-22 10:56 ` [PATCH v2 07/20] tests/tcg/x86_64: Add cross-modifying code test Alex Bennée
                   ` (13 subsequent siblings)
  19 siblings, 1 reply; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

We were missing s390x here. There isn't much point testing for the
architecture here as we will fail anyway if the appropriate package
list is missing.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 scripts/ci/setup/ubuntu/build-environment.yml | 2 --
 1 file changed, 2 deletions(-)

diff --git a/scripts/ci/setup/ubuntu/build-environment.yml b/scripts/ci/setup/ubuntu/build-environment.yml
index edf1900b3e..56b51609e3 100644
--- a/scripts/ci/setup/ubuntu/build-environment.yml
+++ b/scripts/ci/setup/ubuntu/build-environment.yml
@@ -39,7 +39,6 @@
       when:
         - ansible_facts['distribution'] == 'Ubuntu'
         - ansible_facts['distribution_version'] == '22.04'
-        - ansible_facts['architecture'] == 'aarch64' or ansible_facts['architecture'] == 'x86_64'
 
     - name: Install packages for QEMU on Ubuntu 22.04
       package:
@@ -47,7 +46,6 @@
       when:
         - ansible_facts['distribution'] == 'Ubuntu'
         - ansible_facts['distribution_version'] == '22.04'
-        - ansible_facts['architecture'] == 'aarch64' or ansible_facts['architecture'] == 'x86_64'
 
     - name: Install armhf cross-compile packages to build QEMU on AArch64 Ubuntu 22.04
       package:
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 07/20] tests/tcg/x86_64: Add cross-modifying code test
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
                   ` (5 preceding siblings ...)
  2024-10-22 10:56 ` [PATCH v2 06/20] scripts/ci: remove architecture checks for build-environment updates Alex Bennée
@ 2024-10-22 10:56 ` Alex Bennée
  2024-10-22 20:36   ` Pierrick Bouvier
  2024-10-22 10:56 ` [PATCH v2 08/20] accel/tcg: add tracepoints for cpu_loop_exit_atomic Alex Bennée
                   ` (12 subsequent siblings)
  19 siblings, 1 reply; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini, Ilya Leoshkevich

From: Ilya Leoshkevich <iii@linux.ibm.com>

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@linux.ibm.com>
Message-Id: <20241001150617.9977-1-iii@linux.ibm.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 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();
+        }
+    }
+}
+
+#define INITIAL 42
+#define COUNT 1000000
+
+int main(void)
+{
+    int command = THREAD_WAIT;
+    pthread_t thread;
+    long x = 0;
+    int err;
+    int i;
+
+    err = pthread_create(&thread, NULL, &thread_func, &command);
+    assert(err == 0);
+
+    __atomic_store_n(&command, THREAD_PATCH, __ATOMIC_SEQ_CST);
+    for (i = 0; i < COUNT; i++) {
+        add1_or_nop(&x);
+    }
+    __atomic_store_n(&command, THREAD_STOP, __ATOMIC_SEQ_CST);
+
+    err = pthread_join(thread, NULL);
+    assert(err == 0);
+
+    assert(x >= INITIAL);
+    assert(x <= INITIAL + COUNT);
+
+    return EXIT_SUCCESS;
+}
diff --git a/tests/tcg/x86_64/Makefile.target b/tests/tcg/x86_64/Makefile.target
index 783ab5b21a..d6dff559c7 100644
--- a/tests/tcg/x86_64/Makefile.target
+++ b/tests/tcg/x86_64/Makefile.target
@@ -17,6 +17,7 @@ X86_64_TESTS += cmpxchg
 X86_64_TESTS += adox
 X86_64_TESTS += test-1648
 X86_64_TESTS += test-2175
+X86_64_TESTS += cross-modifying-code
 TESTS=$(MULTIARCH_TESTS) $(X86_64_TESTS) test-x86_64
 else
 TESTS=$(MULTIARCH_TESTS)
@@ -27,6 +28,9 @@ adox: CFLAGS=-O2
 run-test-i386-ssse3: QEMU_OPTS += -cpu max
 run-plugin-test-i386-ssse3-%: QEMU_OPTS += -cpu max
 
+cross-modifying-code: CFLAGS+=-pthread
+cross-modifying-code: LDFLAGS+=-pthread
+
 test-x86_64: LDFLAGS+=-lm -lc
 test-x86_64: test-i386.c test-i386.h test-i386-shift.h test-i386-muldiv.h
 	$(CC) $(CFLAGS) $< -o $@ $(LDFLAGS)
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 08/20] accel/tcg: add tracepoints for cpu_loop_exit_atomic
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
                   ` (6 preceding siblings ...)
  2024-10-22 10:56 ` [PATCH v2 07/20] tests/tcg/x86_64: Add cross-modifying code test Alex Bennée
@ 2024-10-22 10:56 ` Alex Bennée
  2024-10-22 10:56 ` [PATCH v2 09/20] dockerfiles: fix default targets for debian-loongarch-cross Alex Bennée
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

We try to avoid using cpu_loop_exit_atomic as it brings in an all-core
sync point. However on some cpu/kernel/benchmark combinations it is
starting to show up in the performance profile. To make it easier to
see whats going on add tracepoints for the slow path so we can see
what is triggering the wait.

It seems for a modern CPU it can be quite a bit, for example:

./qemu-system-aarch64 \
           -machine type=virt,virtualization=on,pflash0=rom,pflash1=efivars,gic-version=max \
           -smp 4 \
           -accel tcg \
           -device virtio-net-pci,netdev=unet \
           -device virtio-scsi-pci \
           -device scsi-hd,drive=hd \
           -netdev user,id=unet,hostfwd=tcp::2222-:22 \
           -blockdev driver=raw,node-name=hd,file.driver=host_device,file.filename=/dev/zen-ssd2/trixie-arm64,discard=unmap \
           -serial mon:stdio \
           -blockdev node-name=rom,driver=file,filename=(pwd)/pc-bios/edk2-aarch64-code.fd,read-only=true \
           -blockdev node-name=efivars,driver=file,filename=$HOME/images/qemu-arm64-efivars \
           -m 8192 \
           -object memory-backend-memfd,id=mem,size=8G,share=on \
           -kernel /home/alex/lsrc/linux.git/builds/arm64/arch/arm64/boot/Image -append "root=/dev/sda2 console=ttyAMA0 systemd.unit=benchmark-stress-ng.service" \
           -display none -d trace:load_atom\*_fallback,trace:store_atom\*_fallback

With:

  -cpu neoverse-v1,pauth-impdef=on => 2203343

With:

  -cpu cortex-a76 => 0

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

---
v2
  - 0x prefixes for ra as per checkpatch
---
 accel/tcg/user-exec.c          |  2 +-
 accel/tcg/ldst_atomicity.c.inc |  9 +++++++++
 accel/tcg/trace-events         | 12 ++++++++++++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/accel/tcg/user-exec.c b/accel/tcg/user-exec.c
index 51b2c16dbe..aa8af52cc3 100644
--- a/accel/tcg/user-exec.c
+++ b/accel/tcg/user-exec.c
@@ -29,7 +29,7 @@
 #include "exec/page-protection.h"
 #include "exec/helper-proto.h"
 #include "qemu/atomic128.h"
-#include "trace/trace-root.h"
+#include "trace.h"
 #include "tcg/tcg-ldst.h"
 #include "internal-common.h"
 #include "internal-target.h"
diff --git a/accel/tcg/ldst_atomicity.c.inc b/accel/tcg/ldst_atomicity.c.inc
index 134da3c1da..c735add261 100644
--- a/accel/tcg/ldst_atomicity.c.inc
+++ b/accel/tcg/ldst_atomicity.c.inc
@@ -168,6 +168,7 @@ static uint64_t load_atomic8_or_exit(CPUState *cpu, uintptr_t ra, void *pv)
 #endif
 
     /* Ultimate fallback: re-execute in serial context. */
+    trace_load_atom8_or_exit_fallback(ra);
     cpu_loop_exit_atomic(cpu, ra);
 }
 
@@ -212,6 +213,7 @@ static Int128 load_atomic16_or_exit(CPUState *cpu, uintptr_t ra, void *pv)
     }
 
     /* Ultimate fallback: re-execute in serial context. */
+    trace_load_atom16_or_exit_fallback(ra);
     cpu_loop_exit_atomic(cpu, ra);
 }
 
@@ -519,6 +521,7 @@ static uint64_t load_atom_8(CPUState *cpu, uintptr_t ra,
         if (HAVE_al8) {
             return load_atom_extract_al8x2(pv);
         }
+        trace_load_atom8_fallback(memop, ra);
         cpu_loop_exit_atomic(cpu, ra);
     default:
         g_assert_not_reached();
@@ -563,6 +566,7 @@ static Int128 load_atom_16(CPUState *cpu, uintptr_t ra,
         break;
     case MO_64:
         if (!HAVE_al8) {
+            trace_load_atom16_fallback(memop, ra);
             cpu_loop_exit_atomic(cpu, ra);
         }
         a = load_atomic8(pv);
@@ -570,6 +574,7 @@ static Int128 load_atom_16(CPUState *cpu, uintptr_t ra,
         break;
     case -MO_64:
         if (!HAVE_al8) {
+            trace_load_atom16_fallback(memop, ra);
             cpu_loop_exit_atomic(cpu, ra);
         }
         a = load_atom_extract_al8x2(pv);
@@ -897,6 +902,7 @@ static void store_atom_2(CPUState *cpu, uintptr_t ra,
         g_assert_not_reached();
     }
 
+    trace_store_atom2_fallback(memop, ra);
     cpu_loop_exit_atomic(cpu, ra);
 }
 
@@ -961,6 +967,7 @@ static void store_atom_4(CPUState *cpu, uintptr_t ra,
                 return;
             }
         }
+        trace_store_atom4_fallback(memop, ra);
         cpu_loop_exit_atomic(cpu, ra);
     default:
         g_assert_not_reached();
@@ -1029,6 +1036,7 @@ static void store_atom_8(CPUState *cpu, uintptr_t ra,
     default:
         g_assert_not_reached();
     }
+    trace_store_atom8_fallback(memop, ra);
     cpu_loop_exit_atomic(cpu, ra);
 }
 
@@ -1107,5 +1115,6 @@ static void store_atom_16(CPUState *cpu, uintptr_t ra,
     default:
         g_assert_not_reached();
     }
+    trace_store_atom16_fallback(memop, ra);
     cpu_loop_exit_atomic(cpu, ra);
 }
diff --git a/accel/tcg/trace-events b/accel/tcg/trace-events
index 4e9b450520..14f638810c 100644
--- a/accel/tcg/trace-events
+++ b/accel/tcg/trace-events
@@ -12,3 +12,15 @@ memory_notdirty_set_dirty(uint64_t vaddr) "0x%" PRIx64
 
 # translate-all.c
 translate_block(void *tb, uintptr_t pc, const void *tb_code) "tb:%p, pc:0x%"PRIxPTR", tb_code:%p"
+
+# ldst_atomicity
+load_atom2_fallback(uint32_t memop, uintptr_t ra) "mop:0x%"PRIx32", ra:0x%"PRIxPTR""
+load_atom4_fallback(uint32_t memop, uintptr_t ra) "mop:0x%"PRIx32", ra:0x%"PRIxPTR""
+load_atom8_or_exit_fallback(uintptr_t ra) "ra:0x%"PRIxPTR""
+load_atom8_fallback(uint32_t memop, uintptr_t ra) "mop:0x%"PRIx32", ra:0x%"PRIxPTR""
+load_atom16_fallback(uint32_t memop, uintptr_t ra) "mop:0x%"PRIx32", ra:0x%"PRIxPTR""
+load_atom16_or_exit_fallback(uintptr_t ra) "ra:0x%"PRIxPTR""
+store_atom2_fallback(uint32_t memop, uintptr_t ra) "mop:0x%"PRIx32", ra:0x%"PRIxPTR""
+store_atom4_fallback(uint32_t memop, uintptr_t ra) "mop:0x%"PRIx32", ra:0x%"PRIxPTR""
+store_atom8_fallback(uint32_t memop, uintptr_t ra) "mop:0x%"PRIx32", ra:0x%"PRIxPTR""
+store_atom16_fallback(uint32_t memop, uintptr_t ra) "mop:0x%"PRIx32", ra:0x%"PRIxPTR""
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 09/20] dockerfiles: fix default targets for debian-loongarch-cross
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
                   ` (7 preceding siblings ...)
  2024-10-22 10:56 ` [PATCH v2 08/20] accel/tcg: add tracepoints for cpu_loop_exit_atomic Alex Bennée
@ 2024-10-22 10:56 ` Alex Bennée
  2024-10-22 10:56 ` [PATCH v2 10/20] gitlab: make check-[dco|patch] a little more verbose Alex Bennée
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

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

fix system target name, and remove --disable-system (which deactivates
system target).

Found using: make docker-test-build@debian-loongarch-cross V=1

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Message-Id: <20241020213759.2168248-1-pierrick.bouvier@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/docker/dockerfiles/debian-loongarch-cross.docker | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/docker/dockerfiles/debian-loongarch-cross.docker b/tests/docker/dockerfiles/debian-loongarch-cross.docker
index 79eab5621e..538ab53490 100644
--- a/tests/docker/dockerfiles/debian-loongarch-cross.docker
+++ b/tests/docker/dockerfiles/debian-loongarch-cross.docker
@@ -43,8 +43,8 @@ RUN curl -#SL https://github.com/loongson/build-tools/releases/download/2023.08.
 ENV PATH $PATH:/opt/cross-tools/bin
 ENV LD_LIBRARY_PATH /opt/cross-tools/lib:/opt/cross-tools/loongarch64-unknown-linux-gnu/lib:$LD_LIBRARY_PATH
 
-ENV QEMU_CONFIGURE_OPTS --disable-system --disable-docs --disable-tools
-ENV DEF_TARGET_LIST loongarch64-linux-user,loongarch-softmmu
+ENV QEMU_CONFIGURE_OPTS --disable-docs --disable-tools
+ENV DEF_TARGET_LIST loongarch64-linux-user,loongarch64-softmmu
 ENV MAKE /usr/bin/make
 
 # As a final step configure the user (if env is defined)
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 10/20] gitlab: make check-[dco|patch] a little more verbose
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
                   ` (8 preceding siblings ...)
  2024-10-22 10:56 ` [PATCH v2 09/20] dockerfiles: fix default targets for debian-loongarch-cross Alex Bennée
@ 2024-10-22 10:56 ` Alex Bennée
  2024-10-22 11:04   ` Daniel P. Berrangé
  2024-10-22 11:08   ` Thomas Huth
  2024-10-22 10:56 ` [PATCH v2 11/20] MAINTAINERS: mention my gdbstub/next tree Alex Bennée
                   ` (9 subsequent siblings)
  19 siblings, 2 replies; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

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@linaro.org>
---
 .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)
 
 ancestor = subprocess.check_output(["git", "merge-base",
                                     "check-dco/master", "HEAD"],
diff --git a/.gitlab-ci.d/check-patch.py b/.gitlab-ci.d/check-patch.py
index 39e2b403c9..94afdce555 100755
--- a/.gitlab-ci.d/check-patch.py
+++ b/.gitlab-ci.d/check-patch.py
@@ -19,13 +19,14 @@
 reponame = os.path.basename(cwd)
 repourl = "https://gitlab.com/%s/%s.git" % (namespace, reponame)
 
+print(f"adding upstream git repo @ {repourl}")
 # GitLab CI environment does not give us any direct info about the
 # base for the user's branch. We thus need to figure out a common
 # ancestor between the user's branch and current git master.
-subprocess.check_call(["git", "remote", "add", "check-patch", repourl])
-subprocess.check_call(["git", "fetch", "check-patch", "master"],
-                      stdout=subprocess.DEVNULL,
-                      stderr=subprocess.DEVNULL)
+subprocess.run(["git", "remote", "add", "check-patch", repourl],
+               check=True, capture_output=True)
+subprocess.run(["git", "fetch", "check-patch", "master"],
+               check=True, capture_output=True)
 
 ancestor = subprocess.check_output(["git", "merge-base",
                                     "check-patch/master", "HEAD"],
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 11/20] MAINTAINERS: mention my gdbstub/next tree
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
                   ` (9 preceding siblings ...)
  2024-10-22 10:56 ` [PATCH v2 10/20] gitlab: make check-[dco|patch] a little more verbose Alex Bennée
@ 2024-10-22 10:56 ` Alex Bennée
  2024-10-22 11:29   ` Thomas Huth
  2024-10-22 21:35   ` Philippe Mathieu-Daudé
  2024-10-22 10:56 ` [PATCH v2 12/20] config/targets: update aarch64_be-linux-user gdb XML list Alex Bennée
                   ` (8 subsequent siblings)
  19 siblings, 2 replies; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

Make it easy for people to see what is already queued.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index b84787ae1b..81396c9f15 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2978,6 +2978,7 @@ F: gdb-xml/
 F: tests/tcg/multiarch/gdbstub/*
 F: scripts/feature_to_c.py
 F: scripts/probe-gdb-support.py
+T: git https://gitlab.com/stsquad/qemu gdbstub/next
 
 Memory API
 M: Paolo Bonzini <pbonzini@redhat.com>
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 12/20] config/targets: update aarch64_be-linux-user gdb XML list
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
                   ` (10 preceding siblings ...)
  2024-10-22 10:56 ` [PATCH v2 11/20] MAINTAINERS: mention my gdbstub/next tree Alex Bennée
@ 2024-10-22 10:56 ` Alex Bennée
  2024-10-22 20:37   ` Pierrick Bouvier
  2024-10-22 10:56 ` [PATCH v2 13/20] tests/tcg: enable basic testing for aarch64_be-linux-user Alex Bennée
                   ` (7 subsequent siblings)
  19 siblings, 1 reply; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

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@linaro.org>
---
 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
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 13/20] tests/tcg: enable basic testing for aarch64_be-linux-user
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
                   ` (11 preceding siblings ...)
  2024-10-22 10:56 ` [PATCH v2 12/20] config/targets: update aarch64_be-linux-user gdb XML list Alex Bennée
@ 2024-10-22 10:56 ` Alex Bennée
  2024-10-22 19:12   ` Richard Henderson
                     ` (2 more replies)
  2024-10-22 10:56 ` [PATCH v2 14/20] tests/tcg/aarch64: Use raw strings for regexes in test-mte.py Alex Bennée
                   ` (6 subsequent siblings)
  19 siblings, 3 replies; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

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@linaro.org>

---
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)
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 14/20] tests/tcg/aarch64: Use raw strings for regexes in test-mte.py
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
                   ` (12 preceding siblings ...)
  2024-10-22 10:56 ` [PATCH v2 13/20] tests/tcg: enable basic testing for aarch64_be-linux-user Alex Bennée
@ 2024-10-22 10:56 ` Alex Bennée
  2024-10-22 10:56 ` [PATCH v2 15/20] testing: Enhance gdb probe script Alex Bennée
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini, Gustavo Romero

From: Gustavo Romero <gustavo.romero@linaro.org>

Use Python's raw string notation instead of string literals for regex so
it's not necessary to double backslashes when regex special forms are
used. Raw notation is preferred for regex and easier to read.

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Message-Id: <20241015140806.385449-1-gustavo.romero@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 tests/tcg/aarch64/gdbstub/test-mte.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/tcg/aarch64/gdbstub/test-mte.py b/tests/tcg/aarch64/gdbstub/test-mte.py
index a4cae6caa0..9ad98e7a54 100644
--- a/tests/tcg/aarch64/gdbstub/test-mte.py
+++ b/tests/tcg/aarch64/gdbstub/test-mte.py
@@ -23,8 +23,8 @@
 from test_gdbstub import arg_parser, main, report
 
 
-PATTERN_0 = "Memory tags for address 0x[0-9a-f]+ match \\(0x[0-9a-f]+\\)."
-PATTERN_1 = ".*(0x[0-9a-f]+)"
+PATTERN_0 = r"Memory tags for address 0x[0-9a-f]+ match \(0x[0-9a-f]+\)."
+PATTERN_1 = r".*(0x[0-9a-f]+)"
 
 
 def run_test():
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 15/20] testing: Enhance gdb probe script
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
                   ` (13 preceding siblings ...)
  2024-10-22 10:56 ` [PATCH v2 14/20] tests/tcg/aarch64: Use raw strings for regexes in test-mte.py Alex Bennée
@ 2024-10-22 10:56 ` Alex Bennée
  2024-10-22 20:39   ` Pierrick Bouvier
  2024-10-22 10:56 ` [PATCH v2 16/20] MAINTAINERS: mention my plugins/next tree Alex Bennée
                   ` (4 subsequent siblings)
  19 siblings, 1 reply; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini, Gustavo Romero

From: Gustavo Romero <gustavo.romero@linaro.org>

Use list and set comprehension to simplify code. Also, gently handle
invalid gdb filenames.

Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
Message-Id: <20241015145848.387281-1-gustavo.romero@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 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')
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 16/20] MAINTAINERS: mention my plugins/next tree
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
                   ` (14 preceding siblings ...)
  2024-10-22 10:56 ` [PATCH v2 15/20] testing: Enhance gdb probe script Alex Bennée
@ 2024-10-22 10:56 ` Alex Bennée
  2024-10-22 20:40   ` Pierrick Bouvier
  2024-10-22 10:56 ` [PATCH v2 17/20] plugins: add ability to register a GDB triggered callback Alex Bennée
                   ` (3 subsequent siblings)
  19 siblings, 1 reply; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

Make it easier to find where plugin patches are being staged.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 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@linaro.org>
+T: git https://gitlab.com/stsquad/qemu plugins/next
 R: Alexandre Iooss <erdnaxe@crans.org>
 R: Mahmoud Mandour <ma.mandourr@gmail.com>
 R: Pierrick Bouvier <pierrick.bouvier@linaro.org>
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 17/20] plugins: add ability to register a GDB triggered callback
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
                   ` (15 preceding siblings ...)
  2024-10-22 10:56 ` [PATCH v2 16/20] MAINTAINERS: mention my plugins/next tree Alex Bennée
@ 2024-10-22 10:56 ` Alex Bennée
  2024-10-22 20:47   ` Pierrick Bouvier
  2024-10-22 10:56 ` [PATCH v2 18/20] meson: build contrib/plugins with meson Alex Bennée
                   ` (2 subsequent siblings)
  19 siblings, 1 reply; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

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@linaro.org>
---
 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;
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 18/20] meson: build contrib/plugins with meson
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
                   ` (16 preceding siblings ...)
  2024-10-22 10:56 ` [PATCH v2 17/20] plugins: add ability to register a GDB triggered callback Alex Bennée
@ 2024-10-22 10:56 ` Alex Bennée
  2024-10-23  7:51   ` Pierrick Bouvier
  2024-10-22 10:56 ` [PATCH v2 19/20] contrib/plugins: remove Makefile for contrib/plugins Alex Bennée
  2024-10-22 10:56 ` [PATCH v2 20/20] plugins: fix qemu_plugin_reset Alex Bennée
  19 siblings, 1 reply; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

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

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@linaro.org>
Message-Id: <20240925204845.390689-2-pierrick.bouvier@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 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
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 19/20] contrib/plugins: remove Makefile for contrib/plugins
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
                   ` (17 preceding siblings ...)
  2024-10-22 10:56 ` [PATCH v2 18/20] meson: build contrib/plugins with meson Alex Bennée
@ 2024-10-22 10:56 ` Alex Bennée
  2024-10-22 10:56 ` [PATCH v2 20/20] plugins: fix qemu_plugin_reset Alex Bennée
  19 siblings, 0 replies; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

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

Now replaced by meson build.

Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Message-Id: <20240925204845.390689-3-pierrick.bouvier@linaro.org>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 configure                | 18 ---------
 Makefile                 | 10 -----
 contrib/plugins/Makefile | 87 ----------------------------------------
 3 files changed, 115 deletions(-)
 delete mode 100644 contrib/plugins/Makefile

diff --git a/configure b/configure
index 7dd3400ccb..101ca9ace9 100755
--- a/configure
+++ b/configure
@@ -1073,7 +1073,6 @@ if test "$plugins" != "no" && test $host_bits -eq 64; then
         plugins="no"
     else
         plugins=yes
-        subdirs="$subdirs contrib/plugins"
     fi
 fi
 
@@ -1704,7 +1703,6 @@ LINKS="$LINKS .gdbinit scripts" # scripts needed by relative path in .gdbinit
 LINKS="$LINKS tests/avocado tests/data"
 LINKS="$LINKS tests/qemu-iotests/check tests/qemu-iotests/Makefile"
 LINKS="$LINKS python"
-LINKS="$LINKS contrib/plugins/Makefile "
 for f in $LINKS ; do
     if [ -e "$source_path/$f" ]; then
         symlink "$source_path/$f" "$f"
@@ -1790,22 +1788,6 @@ if test "$default_targets" = "yes"; then
   echo "CONFIG_DEFAULT_TARGETS=y" >> $config_host_mak
 fi
 
-# contrib/plugins configuration
-echo "# Automatically generated by configure - do not modify" > contrib/plugins/$config_host_mak
-echo "SRC_PATH=$source_path/contrib/plugins" >> contrib/plugins/$config_host_mak
-echo "PKG_CONFIG=${pkg_config}" >> contrib/plugins/$config_host_mak
-echo "CC=$cc $CPU_CFLAGS" >> contrib/plugins/$config_host_mak
-echo "CFLAGS=${CFLAGS-$default_cflags} $EXTRA_CFLAGS" >> contrib/plugins/$config_host_mak
-if test "$host_os" = windows; then
-  echo "DLLTOOL=$dlltool" >> contrib/plugins/$config_host_mak
-fi
-if test "$host_os" = darwin; then
-  echo "CONFIG_DARWIN=y" >> contrib/plugins/$config_host_mak
-fi
-if test "$host_os" = windows; then
-  echo "CONFIG_WIN32=y" >> contrib/plugins/$config_host_mak
-fi
-
 # tests/tcg configuration
 mkdir -p tests/tcg
 echo "# Automatically generated by configure - do not modify" > tests/tcg/$config_host_mak
diff --git a/Makefile b/Makefile
index 917c9a34d1..b65b0bd41a 100644
--- a/Makefile
+++ b/Makefile
@@ -187,11 +187,6 @@ SUBDIR_RULES=$(foreach t, all clean distclean, $(addsuffix /$(t), $(SUBDIRS)))
 $(SUBDIR_RULES):
 	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C $(dir $@) V="$(V)" TARGET_DIR="$(dir $@)" $(notdir $@),)
 
-ifneq ($(filter contrib/plugins, $(SUBDIRS)),)
-.PHONY: plugins
-plugins: contrib/plugins/all
-endif
-
 .PHONY: recurse-all recurse-clean
 recurse-all: $(addsuffix /all, $(SUBDIRS))
 recurse-clean: $(addsuffix /clean, $(SUBDIRS))
@@ -307,11 +302,6 @@ help:
 	$(call print-help,cscope,Generate cscope index)
 	$(call print-help,sparse,Run sparse on the QEMU source)
 	@echo  ''
-ifneq ($(filter contrib/plugins, $(SUBDIRS)),)
-	@echo  'Plugin targets:'
-	$(call print-help,plugins,Build the example TCG plugins)
-	@echo  ''
-endif
 	@echo  'Cleaning targets:'
 	$(call print-help,clean,Remove most generated files but keep the config)
 	$(call print-help,distclean,Remove all generated files)
diff --git a/contrib/plugins/Makefile b/contrib/plugins/Makefile
deleted file mode 100644
index bbddd4800f..0000000000
--- a/contrib/plugins/Makefile
+++ /dev/null
@@ -1,87 +0,0 @@
-# -*- Mode: makefile -*-
-#
-# This Makefile example is fairly independent from the main makefile
-# so users can take and adapt it for their build. We only really
-# include config-host.mak so we don't have to repeat probing for
-# programs that the main configure has already done for us.
-#
-
-include config-host.mak
-
-TOP_SRC_PATH = $(SRC_PATH)/../..
-
-VPATH += $(SRC_PATH)
-
-NAMES :=
-NAMES += bbv
-NAMES += execlog
-NAMES += hotblocks
-NAMES += hotpages
-NAMES += howvec
-
-# The lockstep example communicates using unix sockets,
-# and can't be easily made to work on windows.
-ifneq ($(CONFIG_WIN32),y)
-NAMES += lockstep
-endif
-
-NAMES += hwprofile
-NAMES += cache
-NAMES += drcov
-NAMES += ips
-NAMES += stoptrigger
-NAMES += cflow
-
-ifeq ($(CONFIG_WIN32),y)
-SO_SUFFIX := .dll
-LDLIBS += $(shell $(PKG_CONFIG) --libs glib-2.0)
-else
-SO_SUFFIX := .so
-endif
-
-SONAMES := $(addsuffix $(SO_SUFFIX),$(addprefix lib,$(NAMES)))
-
-# The main QEMU uses Glib extensively so it is perfectly fine to use it
-# in plugins (which many example do).
-PLUGIN_CFLAGS := $(shell $(PKG_CONFIG) --cflags glib-2.0)
-PLUGIN_CFLAGS += -fPIC -Wall
-PLUGIN_CFLAGS += -I$(TOP_SRC_PATH)/include/qemu
-
-# Helper that honours V=1 so we get some output when compiling
-quiet-@ = $(if $(V),,@$(if $1,printf "  %-7s %s\n" "$(strip $1)" "$(strip $2)" && ))
-quiet-command = $(call quiet-@,$2,$3)$1
-
-# for including , in command strings
-COMMA := ,
-
-all: $(SONAMES)
-
-%.o: %.c
-	$(call quiet-command, \
-		$(CC) $(CFLAGS) $(PLUGIN_CFLAGS) -c -o $@ $<, \
-	        BUILD, plugin $@)
-
-ifeq ($(CONFIG_WIN32),y)
-lib%$(SO_SUFFIX): %.o win32_linker.o ../../plugins/libqemu_plugin_api.a
-	$(call quiet-command, \
-		$(CC) -shared -o $@ $^ $(LDLIBS), \
-		LINK, plugin $@)
-else ifeq ($(CONFIG_DARWIN),y)
-lib%$(SO_SUFFIX): %.o
-	$(call quiet-command, \
-		$(CC) -bundle -Wl$(COMMA)-undefined$(COMMA)dynamic_lookup -o $@ $^ $(LDLIBS), \
-		LINK, plugin $@)
-else
-lib%$(SO_SUFFIX): %.o
-	$(call quiet-command, \
-		$(CC) -shared -o $@ $^ $(LDLIBS), \
-		LINK, plugin $@)
-endif
-
-
-clean distclean:
-	rm -f *.o *$(SO_SUFFIX) *.d
-	rm -Rf .libs
-
-.PHONY: all clean
-.SECONDARY:
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* [PATCH v2 20/20] plugins: fix qemu_plugin_reset
  2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
                   ` (18 preceding siblings ...)
  2024-10-22 10:56 ` [PATCH v2 19/20] contrib/plugins: remove Makefile for contrib/plugins Alex Bennée
@ 2024-10-22 10:56 ` Alex Bennée
  19 siblings, 0 replies; 43+ messages in thread
From: Alex Bennée @ 2024-10-22 10:56 UTC (permalink / raw)
  To: qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Alex Bennée, Yanan Wang,
	Thomas Huth, John Snow, Marc-André Lureau, qemu-arm,
	Daniel P. Berrangé, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini, Robbin Ehn

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

34e5e1 refactored the plugin context initialization. After this change,
tcg_ctx->plugin_insn is not reset inconditionnally anymore, but only if
one plugin at least is active.

When uninstalling the last plugin active, we stopped reinitializing
tcg_ctx->plugin_insn, which leads to memory callbacks being emitted.
This results in an error as they don't appear in a plugin op sequence as
expected.

The correct fix is to make sure we reset plugin translation variables
after current block translation ends. This way, we can catch any
potential misuse of those after a given block, in more than fixing the
current bug.

Fixes: https://gitlab.com/qemu-project/qemu/-/issues/2570
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>
Tested-by: Robbin Ehn <rehn@rivosinc.com>
Message-Id: <20241015003819.984601-1-pierrick.bouvier@linaro.org>
[AJB: trim patch version details from commit msg]
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 accel/tcg/plugin-gen.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/accel/tcg/plugin-gen.c b/accel/tcg/plugin-gen.c
index 2ee4c22bef..0f47bfbb48 100644
--- a/accel/tcg/plugin-gen.c
+++ b/accel/tcg/plugin-gen.c
@@ -467,4 +467,8 @@ void plugin_gen_tb_end(CPUState *cpu, size_t num_insns)
 
     /* inject the instrumentation at the appropriate places */
     plugin_gen_inject(ptb);
+
+    /* reset plugin translation state (plugin_tb is reused between blocks) */
+    tcg_ctx->plugin_db = NULL;
+    tcg_ctx->plugin_insn = NULL;
 }
-- 
2.39.5



^ permalink raw reply related	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 10/20] gitlab: make check-[dco|patch] a little more verbose
  2024-10-22 10:56 ` [PATCH v2 10/20] gitlab: make check-[dco|patch] a little more verbose Alex Bennée
@ 2024-10-22 11:04   ` Daniel P. Berrangé
  2024-10-22 11:08   ` Thomas Huth
  1 sibling, 0 replies; 43+ messages in thread
From: Daniel P. Berrangé @ 2024-10-22 11:04 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, Beraldo Leal, Laurent Vivier,
	Wainer dos Santos Moschetta, Mahmoud Mandour, Jiaxun Yang,
	Yanan Wang, Thomas Huth, John Snow, Marc-André Lureau,
	qemu-arm, Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

On Tue, Oct 22, 2024 at 11:56:04AM +0100, Alex Bennée wrote:
> 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@linaro.org>
> ---
>  .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)

This is effectively no change - 'capture_output'  means stderr/out
are captured into a buffer which subprocess.run returns, but you're
not using the return value so the captured output is invisible.

If we want to see errors, then just remove the stderr/stdout
args from the check_call function, so they're no longer sent
to /dev/null


With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|



^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 10/20] gitlab: make check-[dco|patch] a little more verbose
  2024-10-22 10:56 ` [PATCH v2 10/20] gitlab: make check-[dco|patch] a little more verbose Alex Bennée
  2024-10-22 11:04   ` Daniel P. Berrangé
@ 2024-10-22 11:08   ` Thomas Huth
  1 sibling, 0 replies; 43+ messages in thread
From: Thomas Huth @ 2024-10-22 11:08 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Yanan Wang, John Snow,
	Marc-André Lureau, qemu-arm, Daniel P. Berrangé,
	Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

On 22/10/2024 12.56, Alex Bennée wrote:
> 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@linaro.org>
> ---
>   .gitlab-ci.d/check-dco.py   | 9 +++++----
>   .gitlab-ci.d/check-patch.py | 9 +++++----
>   2 files changed, 10 insertions(+), 8 deletions(-)

Reviewed-by: Thomas Huth <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 03/20] MAINTAINERS: mention my testing/next tree
  2024-10-22 10:55 ` [PATCH v2 03/20] MAINTAINERS: mention my testing/next tree Alex Bennée
@ 2024-10-22 11:20   ` Thomas Huth
  0 siblings, 0 replies; 43+ messages in thread
From: Thomas Huth @ 2024-10-22 11:20 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Yanan Wang, John Snow,
	Marc-André Lureau, qemu-arm, Daniel P. Berrangé,
	Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

On 22/10/2024 12.55, Alex Bennée wrote:
> I put it under my name as there may be other maintainer testing trees
> as well.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   MAINTAINERS | 1 +
>   1 file changed, 1 insertion(+)

Reviewed-by: Thomas Huth <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 11/20] MAINTAINERS: mention my gdbstub/next tree
  2024-10-22 10:56 ` [PATCH v2 11/20] MAINTAINERS: mention my gdbstub/next tree Alex Bennée
@ 2024-10-22 11:29   ` Thomas Huth
  2024-10-22 21:35   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 43+ messages in thread
From: Thomas Huth @ 2024-10-22 11:29 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Yanan Wang, John Snow,
	Marc-André Lureau, qemu-arm, Daniel P. Berrangé,
	Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Pierrick Bouvier,
	Paolo Bonzini

On 22/10/2024 12.56, Alex Bennée wrote:
> Make it easy for people to see what is already queued.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   MAINTAINERS | 1 +
>   1 file changed, 1 insertion(+)

Reviewed-by: Thomas Huth <thuth@redhat.com>



^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 13/20] tests/tcg: enable basic testing for aarch64_be-linux-user
  2024-10-22 10:56 ` [PATCH v2 13/20] tests/tcg: enable basic testing for aarch64_be-linux-user Alex Bennée
@ 2024-10-22 19:12   ` Richard Henderson
  2024-10-22 20:39   ` Pierrick Bouvier
  2024-10-22 21:37   ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 43+ messages in thread
From: Richard Henderson @ 2024-10-22 19:12 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel

On 10/22/24 03:56, Alex Bennée wrote:
> 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@linaro.org>
> 
> ---
> 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

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 01/20] tests/docker: Fix microblaze atomics
  2024-10-22 10:55 ` [PATCH v2 01/20] tests/docker: Fix microblaze atomics Alex Bennée
@ 2024-10-22 20:30   ` Pierrick Bouvier
  0 siblings, 0 replies; 43+ messages in thread
From: Pierrick Bouvier @ 2024-10-22 20:30 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Yanan Wang, Thomas Huth, John Snow,
	Marc-André Lureau, qemu-arm, Daniel P. Berrangé,
	Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Paolo Bonzini,
	Ilya Leoshkevich

On 10/22/24 03:55, Alex Bennée wrote:
> From: Ilya Leoshkevich <iii@linux.ibm.com>
> 
> GCC produces invalid code for microblaze atomics.
> 
> The fix is unfortunately not upstream, so fetch it from an external
> location and apply it locally.
> 
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> Message-Id: <20240919152308.10440-1-iii@linux.ibm.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   .../debian-microblaze-cross.d/build-toolchain.sh          | 8 ++++++++
>   tests/docker/dockerfiles/debian-toolchain.docker          | 7 +++++++
>   2 files changed, 15 insertions(+)
> 
> diff --git a/tests/docker/dockerfiles/debian-microblaze-cross.d/build-toolchain.sh b/tests/docker/dockerfiles/debian-microblaze-cross.d/build-toolchain.sh
> index 23ec0aa9a7..c5cd0aa931 100755
> --- a/tests/docker/dockerfiles/debian-microblaze-cross.d/build-toolchain.sh
> +++ b/tests/docker/dockerfiles/debian-microblaze-cross.d/build-toolchain.sh
> @@ -10,6 +10,8 @@ TOOLCHAIN_INSTALL=/usr/local
>   TOOLCHAIN_BIN=${TOOLCHAIN_INSTALL}/bin
>   CROSS_SYSROOT=${TOOLCHAIN_INSTALL}/$TARGET/sys-root
>   
> +GCC_PATCH0_URL=https://raw.githubusercontent.com/Xilinx/meta-xilinx/refs/tags/xlnx-rel-v2024.1/meta-microblaze/recipes-devtools/gcc/gcc-12/0009-Patch-microblaze-Fix-atomic-boolean-return-value.patch
> +
>   export PATH=${TOOLCHAIN_BIN}:$PATH
>   
>   #
> @@ -31,6 +33,12 @@ mv gcc-11.2.0 src-gcc
>   mv musl-1.2.2 src-musl
>   mv linux-5.10.70 src-linux
>   
> +#
> +# Patch gcc
> +#
> +
> +wget -O - ${GCC_PATCH0_URL} | patch -d src-gcc -p1
> +
>   mkdir -p bld-hdr bld-binu bld-gcc bld-musl
>   mkdir -p ${CROSS_SYSROOT}/usr/include
>   
> diff --git a/tests/docker/dockerfiles/debian-toolchain.docker b/tests/docker/dockerfiles/debian-toolchain.docker
> index 687a97fec4..ab4ce29533 100644
> --- a/tests/docker/dockerfiles/debian-toolchain.docker
> +++ b/tests/docker/dockerfiles/debian-toolchain.docker
> @@ -10,6 +10,8 @@ FROM docker.io/library/debian:11-slim
>   # ??? The build-dep isn't working, missing a number of
>   # minimal build dependiencies, e.g. libmpc.
>   
> +RUN sed 's/^deb /deb-src /' </etc/apt/sources.list >/etc/apt/sources.list.d/deb-src.list
> +
>   RUN apt update && \
>       DEBIAN_FRONTEND=noninteractive apt install -yy eatmydata && \
>       DEBIAN_FRONTEND=noninteractive eatmydata \
> @@ -33,6 +35,11 @@ RUN cd /root && ./build-toolchain.sh
>   # and the build trees by restoring the original image,
>   # then copying the built toolchain from stage 0.
>   FROM docker.io/library/debian:11-slim
> +RUN apt update && \
> +    DEBIAN_FRONTEND=noninteractive apt install -yy eatmydata && \
> +    DEBIAN_FRONTEND=noninteractive eatmydata \
> +    apt install -y --no-install-recommends \
> +        libmpc3
>   COPY --from=0 /usr/local /usr/local
>   # As a final step configure the user (if env is defined)
>   ARG USER

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 02/20] tests/docker: add NOFETCH env variable for testing
  2024-10-22 10:55 ` [PATCH v2 02/20] tests/docker: add NOFETCH env variable for testing Alex Bennée
@ 2024-10-22 20:31   ` Pierrick Bouvier
  0 siblings, 0 replies; 43+ messages in thread
From: Pierrick Bouvier @ 2024-10-22 20:31 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Yanan Wang, Thomas Huth, John Snow,
	Marc-André Lureau, qemu-arm, Daniel P. Berrangé,
	Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Paolo Bonzini

On 10/22/24 03:55, Alex Bennée wrote:
> Testing non-auto built docker containers (i.e. custom built compilers)
> is a bit fiddly as you couldn't continue a build with a previously
> locally built container. While you can play games with REGISTRY its
> simpler to allow a NOFETCH that will go through the cached build
> process when you run the tests.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   tests/docker/Makefile.include | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include
> index 681feae744..fead7d3abe 100644
> --- a/tests/docker/Makefile.include
> +++ b/tests/docker/Makefile.include
> @@ -92,10 +92,10 @@ endif
>   docker-image-alpine: NOUSER=1
>   
>   debian-toolchain-run = \
> -	$(if $(NOCACHE), 						\
> +	$(if $(NOCACHE)$(NOFETCH),					\
>   		$(call quiet-command,					\
>   			$(DOCKER_SCRIPT) build -t qemu/$1 -f $< 	\
> -			$(if $V,,--quiet) --no-cache 			\
> +			$(if $V,,--quiet) $(if $(NOCACHE),--no-cache)	\
>   			--registry $(DOCKER_REGISTRY) --extra-files	\
>   			$(DOCKER_FILES_DIR)/$1.d/build-toolchain.sh,	\
>   			"BUILD", $1),				        \
> @@ -177,6 +177,7 @@ docker:
>   	@echo '    NETWORK=$$BACKEND     Enable virtual network interface with $$BACKEND.'
>   	@echo '    NOUSER=1             Define to disable adding current user to containers passwd.'
>   	@echo '    NOCACHE=1            Ignore cache when build images.'
> +	@echo '    NOFETCH=1            Do not fetch from the registry.'
>   	@echo '    EXECUTABLE=<path>    Include executable in image.'
>   	@echo '    EXTRA_FILES="<path> [... <path>]"'
>   	@echo '                         Include extra files in image.'

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 06/20] scripts/ci: remove architecture checks for build-environment updates
  2024-10-22 10:56 ` [PATCH v2 06/20] scripts/ci: remove architecture checks for build-environment updates Alex Bennée
@ 2024-10-22 20:32   ` Pierrick Bouvier
  0 siblings, 0 replies; 43+ messages in thread
From: Pierrick Bouvier @ 2024-10-22 20:32 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Yanan Wang, Thomas Huth, John Snow,
	Marc-André Lureau, qemu-arm, Daniel P. Berrangé,
	Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Paolo Bonzini

On 10/22/24 03:56, Alex Bennée wrote:
> We were missing s390x here. There isn't much point testing for the
> architecture here as we will fail anyway if the appropriate package
> list is missing.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   scripts/ci/setup/ubuntu/build-environment.yml | 2 --
>   1 file changed, 2 deletions(-)
> 
> diff --git a/scripts/ci/setup/ubuntu/build-environment.yml b/scripts/ci/setup/ubuntu/build-environment.yml
> index edf1900b3e..56b51609e3 100644
> --- a/scripts/ci/setup/ubuntu/build-environment.yml
> +++ b/scripts/ci/setup/ubuntu/build-environment.yml
> @@ -39,7 +39,6 @@
>         when:
>           - ansible_facts['distribution'] == 'Ubuntu'
>           - ansible_facts['distribution_version'] == '22.04'
> -        - ansible_facts['architecture'] == 'aarch64' or ansible_facts['architecture'] == 'x86_64'
>   
>       - name: Install packages for QEMU on Ubuntu 22.04
>         package:
> @@ -47,7 +46,6 @@
>         when:
>           - ansible_facts['distribution'] == 'Ubuntu'
>           - ansible_facts['distribution_version'] == '22.04'
> -        - ansible_facts['architecture'] == 'aarch64' or ansible_facts['architecture'] == 'x86_64'
>   
>       - name: Install armhf cross-compile packages to build QEMU on AArch64 Ubuntu 22.04
>         package:

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 07/20] tests/tcg/x86_64: Add cross-modifying code test
  2024-10-22 10:56 ` [PATCH v2 07/20] tests/tcg/x86_64: Add cross-modifying code test Alex Bennée
@ 2024-10-22 20:36   ` Pierrick Bouvier
  2024-10-23  0:16     ` Ilya Leoshkevich
  0 siblings, 1 reply; 43+ messages in thread
From: Pierrick Bouvier @ 2024-10-22 20:36 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Yanan Wang, Thomas Huth, John Snow,
	Marc-André Lureau, qemu-arm, Daniel P. Berrangé,
	Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Paolo Bonzini,
	Ilya Leoshkevich

On 10/22/24 03:56, Alex Bennée wrote:
> From: Ilya Leoshkevich <iii@linux.ibm.com>
> 
> 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@linux.ibm.com>
> Message-Id: <20241001150617.9977-1-iii@linux.ibm.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   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.

> +        }
> +    }
> +}
> +
> +#define INITIAL 42
> +#define COUNT 1000000
> +
> +int main(void)
> +{
> +    int command = THREAD_WAIT;
> +    pthread_t thread;
> +    long x = 0;
> +    int err;
> +    int i;
> +
> +    err = pthread_create(&thread, NULL, &thread_func, &command);
> +    assert(err == 0);
> +
> +    __atomic_store_n(&command, THREAD_PATCH, __ATOMIC_SEQ_CST);
> +    for (i = 0; i < COUNT; i++) {
> +        add1_or_nop(&x);
> +    }
> +    __atomic_store_n(&command, THREAD_STOP, __ATOMIC_SEQ_CST);
> +
> +    err = pthread_join(thread, NULL);
> +    assert(err == 0);
> +
> +    assert(x >= INITIAL);
> +    assert(x <= INITIAL + COUNT);
> +
> +    return EXIT_SUCCESS;
> +}
> diff --git a/tests/tcg/x86_64/Makefile.target b/tests/tcg/x86_64/Makefile.target
> index 783ab5b21a..d6dff559c7 100644
> --- a/tests/tcg/x86_64/Makefile.target
> +++ b/tests/tcg/x86_64/Makefile.target
> @@ -17,6 +17,7 @@ X86_64_TESTS += cmpxchg
>   X86_64_TESTS += adox
>   X86_64_TESTS += test-1648
>   X86_64_TESTS += test-2175
> +X86_64_TESTS += cross-modifying-code
>   TESTS=$(MULTIARCH_TESTS) $(X86_64_TESTS) test-x86_64
>   else
>   TESTS=$(MULTIARCH_TESTS)
> @@ -27,6 +28,9 @@ adox: CFLAGS=-O2
>   run-test-i386-ssse3: QEMU_OPTS += -cpu max
>   run-plugin-test-i386-ssse3-%: QEMU_OPTS += -cpu max
>   
> +cross-modifying-code: CFLAGS+=-pthread
> +cross-modifying-code: LDFLAGS+=-pthread
> +
>   test-x86_64: LDFLAGS+=-lm -lc
>   test-x86_64: test-i386.c test-i386.h test-i386-shift.h test-i386-muldiv.h
>   	$(CC) $(CFLAGS) $< -o $@ $(LDFLAGS)

With this change,
Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 12/20] config/targets: update aarch64_be-linux-user gdb XML list
  2024-10-22 10:56 ` [PATCH v2 12/20] config/targets: update aarch64_be-linux-user gdb XML list Alex Bennée
@ 2024-10-22 20:37   ` Pierrick Bouvier
  0 siblings, 0 replies; 43+ messages in thread
From: Pierrick Bouvier @ 2024-10-22 20:37 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Yanan Wang, Thomas Huth, John Snow,
	Marc-André Lureau, qemu-arm, Daniel P. Berrangé,
	Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Paolo Bonzini

On 10/22/24 03:56, Alex Bennée wrote:
> 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@linaro.org>
> ---
>   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

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 13/20] tests/tcg: enable basic testing for aarch64_be-linux-user
  2024-10-22 10:56 ` [PATCH v2 13/20] tests/tcg: enable basic testing for aarch64_be-linux-user Alex Bennée
  2024-10-22 19:12   ` Richard Henderson
@ 2024-10-22 20:39   ` Pierrick Bouvier
  2024-10-22 21:37   ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 43+ messages in thread
From: Pierrick Bouvier @ 2024-10-22 20:39 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Yanan Wang, Thomas Huth, John Snow,
	Marc-André Lureau, qemu-arm, Daniel P. Berrangé,
	Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Paolo Bonzini

On 10/22/24 03:56, Alex Bennée wrote:
> 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@linaro.org>
> 
> ---
> 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.
> +

s/l ibc/ libc/

> +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)

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 15/20] testing: Enhance gdb probe script
  2024-10-22 10:56 ` [PATCH v2 15/20] testing: Enhance gdb probe script Alex Bennée
@ 2024-10-22 20:39   ` Pierrick Bouvier
  0 siblings, 0 replies; 43+ messages in thread
From: Pierrick Bouvier @ 2024-10-22 20:39 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Yanan Wang, Thomas Huth, John Snow,
	Marc-André Lureau, qemu-arm, Daniel P. Berrangé,
	Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Paolo Bonzini,
	Gustavo Romero

On 10/22/24 03:56, Alex Bennée wrote:
> From: Gustavo Romero <gustavo.romero@linaro.org>
> 
> Use list and set comprehension to simplify code. Also, gently handle
> invalid gdb filenames.
> 
> Signed-off-by: Gustavo Romero <gustavo.romero@linaro.org>
> Message-Id: <20241015145848.387281-1-gustavo.romero@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   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')

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 16/20] MAINTAINERS: mention my plugins/next tree
  2024-10-22 10:56 ` [PATCH v2 16/20] MAINTAINERS: mention my plugins/next tree Alex Bennée
@ 2024-10-22 20:40   ` Pierrick Bouvier
  0 siblings, 0 replies; 43+ messages in thread
From: Pierrick Bouvier @ 2024-10-22 20:40 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Yanan Wang, Thomas Huth, John Snow,
	Marc-André Lureau, qemu-arm, Daniel P. Berrangé,
	Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Paolo Bonzini

On 10/22/24 03:56, Alex Bennée wrote:
> Make it easier to find where plugin patches are being staged.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   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@linaro.org>
> +T: git https://gitlab.com/stsquad/qemu plugins/next
>   R: Alexandre Iooss <erdnaxe@crans.org>
>   R: Mahmoud Mandour <ma.mandourr@gmail.com>
>   R: Pierrick Bouvier <pierrick.bouvier@linaro.org>

Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 17/20] plugins: add ability to register a GDB triggered callback
  2024-10-22 10:56 ` [PATCH v2 17/20] plugins: add ability to register a GDB triggered callback Alex Bennée
@ 2024-10-22 20:47   ` Pierrick Bouvier
  0 siblings, 0 replies; 43+ messages in thread
From: Pierrick Bouvier @ 2024-10-22 20:47 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Yanan Wang, Thomas Huth, John Snow,
	Marc-André Lureau, qemu-arm, Daniel P. Berrangé,
	Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Paolo Bonzini

On 10/22/24 03:56, Alex Bennée wrote:
> 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@linaro.org>
> ---
>   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;

This is a very nice addition!
Beyond developers, it can be very useful for plugins because you can 
start to script instrumentation using gdb.

Would that be possible to register several commands, with different 
names? It seems a bit arbitrary to be able to register only one command, 
with a fixed name, when we could have several.

In more, it could be nice to be able to pass a list of parameters (as a 
simple string or string array) to the callback on plugin side.

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 11/20] MAINTAINERS: mention my gdbstub/next tree
  2024-10-22 10:56 ` [PATCH v2 11/20] MAINTAINERS: mention my gdbstub/next tree Alex Bennée
  2024-10-22 11:29   ` Thomas Huth
@ 2024-10-22 21:35   ` Philippe Mathieu-Daudé
  1 sibling, 0 replies; 43+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-10-22 21:35 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Yanan Wang, Thomas Huth, John Snow,
	Marc-André Lureau, qemu-arm, Daniel P. Berrangé,
	Eduardo Habkost, devel, Cleber Rosa, kvm, Alexandre Iooss,
	Peter Maydell, Richard Henderson, Riku Voipio, Zhao Liu,
	Marcelo Tosatti, Edgar E. Iglesias, Marcel Apfelbaum,
	Pierrick Bouvier, Paolo Bonzini

On 22/10/24 07:56, Alex Bennée wrote:
> Make it easy for people to see what is already queued.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   MAINTAINERS | 1 +
>   1 file changed, 1 insertion(+)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>




^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 13/20] tests/tcg: enable basic testing for aarch64_be-linux-user
  2024-10-22 10:56 ` [PATCH v2 13/20] tests/tcg: enable basic testing for aarch64_be-linux-user Alex Bennée
  2024-10-22 19:12   ` Richard Henderson
  2024-10-22 20:39   ` Pierrick Bouvier
@ 2024-10-22 21:37   ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 43+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-10-22 21:37 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Yanan Wang, Thomas Huth, John Snow,
	Marc-André Lureau, qemu-arm, Daniel P. Berrangé,
	Eduardo Habkost, devel, Cleber Rosa, kvm, Alexandre Iooss,
	Peter Maydell, Richard Henderson, Riku Voipio, Zhao Liu,
	Marcelo Tosatti, Edgar E. Iglesias, Marcel Apfelbaum,
	Pierrick Bouvier, Paolo Bonzini

On 22/10/24 07:56, Alex Bennée wrote:
> 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@linaro.org>
> 
> ---
> 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/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

ifeq and include once? Anyhow,

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

> +else
>   -include $(SRC_PATH)/tests/tcg/multiarch/Makefile.target
>   -include $(SRC_PATH)/tests/tcg/$(TARGET_NAME)/Makefile.target
> +endif



^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 07/20] tests/tcg/x86_64: Add cross-modifying code test
  2024-10-22 20:36   ` Pierrick Bouvier
@ 2024-10-23  0:16     ` Ilya Leoshkevich
  2024-10-23  0:33       ` Pierrick Bouvier
  0 siblings, 1 reply; 43+ messages in thread
From: Ilya Leoshkevich @ 2024-10-23  0:16 UTC (permalink / raw)
  To: Pierrick Bouvier, Alex Bennée, qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Yanan Wang, Thomas Huth, John Snow,
	Marc-André Lureau, qemu-arm, Daniel P. Berrangé,
	Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Paolo Bonzini

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@linux.ibm.com>
> > 
> > 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@linux.ibm.com>
> > Message-Id: <20241001150617.9977-1-iii@linux.ibm.com>
> > Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> > ---
> >   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.

[...]




^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 07/20] tests/tcg/x86_64: Add cross-modifying code test
  2024-10-23  0:16     ` Ilya Leoshkevich
@ 2024-10-23  0:33       ` Pierrick Bouvier
  2024-10-23  8:55         ` Alex Bennée
  0 siblings, 1 reply; 43+ messages in thread
From: Pierrick Bouvier @ 2024-10-23  0:33 UTC (permalink / raw)
  To: Ilya Leoshkevich, Alex Bennée, qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Yanan Wang, Thomas Huth, John Snow,
	Marc-André Lureau, qemu-arm, Daniel P. Berrangé,
	Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Paolo Bonzini

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@linux.ibm.com>
>>>
>>> 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@linux.ibm.com>
>>> Message-Id: <20241001150617.9977-1-iii@linux.ibm.com>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> ---
>>>    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@linaro.org>

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 18/20] meson: build contrib/plugins with meson
  2024-10-22 10:56 ` [PATCH v2 18/20] meson: build contrib/plugins with meson Alex Bennée
@ 2024-10-23  7:51   ` Pierrick Bouvier
  2024-10-23  8:57     ` Alex Bennée
  0 siblings, 1 reply; 43+ messages in thread
From: Pierrick Bouvier @ 2024-10-23  7:51 UTC (permalink / raw)
  To: Alex Bennée, qemu-devel
  Cc: Beraldo Leal, Laurent Vivier, Wainer dos Santos Moschetta,
	Mahmoud Mandour, Jiaxun Yang, Yanan Wang, Thomas Huth, John Snow,
	Marc-André Lureau, qemu-arm, Daniel P. Berrangé,
	Eduardo Habkost, devel, Cleber Rosa, kvm,
	Philippe Mathieu-Daudé, Alexandre Iooss, Peter Maydell,
	Richard Henderson, Riku Voipio, Zhao Liu, Marcelo Tosatti,
	Edgar E. Iglesias, Marcel Apfelbaum, Paolo Bonzini

On 10/22/24 03:56, Alex Bennée wrote:
> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
> 
> 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@linaro.org>
> Message-Id: <20240925204845.390689-2-pierrick.bouvier@linaro.org>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>   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.

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

^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 07/20] tests/tcg/x86_64: Add cross-modifying code test
  2024-10-23  0:33       ` Pierrick Bouvier
@ 2024-10-23  8:55         ` Alex Bennée
  0 siblings, 0 replies; 43+ messages in thread
From: Alex Bennée @ 2024-10-23  8:55 UTC (permalink / raw)
  To: Pierrick Bouvier
  Cc: Ilya Leoshkevich, qemu-devel, Beraldo Leal, Laurent Vivier,
	Wainer dos Santos Moschetta, Mahmoud Mandour, Jiaxun Yang,
	Yanan Wang, Thomas Huth, John Snow, Marc-André Lureau,
	qemu-arm, Daniel P. Berrangé, Eduardo Habkost, devel,
	Cleber Rosa, kvm, Philippe Mathieu-Daudé, Alexandre Iooss,
	Peter Maydell, Richard Henderson, Riku Voipio, Zhao Liu,
	Marcelo Tosatti, Edgar E. Iglesias, Marcel Apfelbaum,
	Paolo Bonzini

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

> 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@linux.ibm.com>
>>>>
>>>> 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@linux.ibm.com>
>>>> Message-Id: <20241001150617.9977-1-iii@linux.ibm.com>
>>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>>> ---
>>>>    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.

No we only have glibc for test cases.

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

Its a false positive in this case. We could tech checkpatch not to care
about glib-isms in tests/tcg but that would probaly make keeping it in
sync with the kernel version harder.

>
>> 
>
> As it is,
> Reviewed-by: Pierrick Bouvier <pierrick.bouvier@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 18/20] meson: build contrib/plugins with meson
  2024-10-23  7:51   ` Pierrick Bouvier
@ 2024-10-23  8:57     ` Alex Bennée
  2024-10-23 21:31       ` Pierrick Bouvier
  0 siblings, 1 reply; 43+ messages in thread
From: Alex Bennée @ 2024-10-23  8:57 UTC (permalink / raw)
  To: Pierrick Bouvier
  Cc: qemu-devel, Beraldo Leal, Laurent Vivier,
	Wainer dos Santos Moschetta, Mahmoud Mandour, Jiaxun Yang,
	Yanan Wang, Thomas Huth, John Snow, Marc-André Lureau,
	qemu-arm, Daniel P. Berrangé, Eduardo Habkost, devel,
	Cleber Rosa, kvm, Philippe Mathieu-Daudé, Alexandre Iooss,
	Peter Maydell, Richard Henderson, Riku Voipio, Zhao Liu,
	Marcelo Tosatti, Edgar E. Iglesias, Marcel Apfelbaum,
	Paolo Bonzini

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

> On 10/22/24 03:56, Alex Bennée wrote:
>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> 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@linaro.org>
>> Message-Id: <20240925204845.390689-2-pierrick.bouvier@linaro.org>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>   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.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro


^ permalink raw reply	[flat|nested] 43+ messages in thread

* Re: [PATCH v2 18/20] meson: build contrib/plugins with meson
  2024-10-23  8:57     ` Alex Bennée
@ 2024-10-23 21:31       ` Pierrick Bouvier
  0 siblings, 0 replies; 43+ messages in thread
From: Pierrick Bouvier @ 2024-10-23 21:31 UTC (permalink / raw)
  To: Alex Bennée
  Cc: qemu-devel, Beraldo Leal, Laurent Vivier,
	Wainer dos Santos Moschetta, Mahmoud Mandour, Jiaxun Yang,
	Yanan Wang, Thomas Huth, John Snow, Marc-André Lureau,
	qemu-arm, Daniel P. Berrangé, Eduardo Habkost, devel,
	Cleber Rosa, kvm, Philippe Mathieu-Daudé, Alexandre Iooss,
	Peter Maydell, Richard Henderson, Riku Voipio, Zhao Liu,
	Marcelo Tosatti, Edgar E. Iglesias, Marcel Apfelbaum,
	Paolo Bonzini

On 10/23/24 01:57, Alex Bennée wrote:
> Pierrick Bouvier <pierrick.bouvier@linaro.org> writes:
> 
>> On 10/22/24 03:56, Alex Bennée wrote:
>>> From: Pierrick Bouvier <pierrick.bouvier@linaro.org>
>>> 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@linaro.org>
>>> Message-Id: <20240925204845.390689-2-pierrick.bouvier@linaro.org>
>>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>>> ---
>>>    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.
> 

Sent a v3 with windows fix:
https://lore.kernel.org/qemu-devel/20241023212812.1376972-1-pierrick.bouvier@linaro.org/T/#t

Thanks,
Pierrick

^ permalink raw reply	[flat|nested] 43+ messages in thread

end of thread, other threads:[~2024-10-23 21:32 UTC | newest]

Thread overview: 43+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-22 10:55 [PATCH v2 00/20] maintainer updates (testing, gdbstub, plugins) Alex Bennée
2024-10-22 10:55 ` [PATCH v2 01/20] tests/docker: Fix microblaze atomics Alex Bennée
2024-10-22 20:30   ` Pierrick Bouvier
2024-10-22 10:55 ` [PATCH v2 02/20] tests/docker: add NOFETCH env variable for testing Alex Bennée
2024-10-22 20:31   ` Pierrick Bouvier
2024-10-22 10:55 ` [PATCH v2 03/20] MAINTAINERS: mention my testing/next tree Alex Bennée
2024-10-22 11:20   ` Thomas Huth
2024-10-22 10:55 ` [PATCH v2 04/20] meson: hide tsan related warnings Alex Bennée
2024-10-22 10:55 ` [PATCH v2 05/20] docs/devel: update tsan build documentation Alex Bennée
2024-10-22 10:56 ` [PATCH v2 06/20] scripts/ci: remove architecture checks for build-environment updates Alex Bennée
2024-10-22 20:32   ` Pierrick Bouvier
2024-10-22 10:56 ` [PATCH v2 07/20] tests/tcg/x86_64: Add cross-modifying code test Alex Bennée
2024-10-22 20:36   ` Pierrick Bouvier
2024-10-23  0:16     ` Ilya Leoshkevich
2024-10-23  0:33       ` Pierrick Bouvier
2024-10-23  8:55         ` Alex Bennée
2024-10-22 10:56 ` [PATCH v2 08/20] accel/tcg: add tracepoints for cpu_loop_exit_atomic Alex Bennée
2024-10-22 10:56 ` [PATCH v2 09/20] dockerfiles: fix default targets for debian-loongarch-cross Alex Bennée
2024-10-22 10:56 ` [PATCH v2 10/20] gitlab: make check-[dco|patch] a little more verbose Alex Bennée
2024-10-22 11:04   ` Daniel P. Berrangé
2024-10-22 11:08   ` Thomas Huth
2024-10-22 10:56 ` [PATCH v2 11/20] MAINTAINERS: mention my gdbstub/next tree Alex Bennée
2024-10-22 11:29   ` Thomas Huth
2024-10-22 21:35   ` Philippe Mathieu-Daudé
2024-10-22 10:56 ` [PATCH v2 12/20] config/targets: update aarch64_be-linux-user gdb XML list Alex Bennée
2024-10-22 20:37   ` Pierrick Bouvier
2024-10-22 10:56 ` [PATCH v2 13/20] tests/tcg: enable basic testing for aarch64_be-linux-user Alex Bennée
2024-10-22 19:12   ` Richard Henderson
2024-10-22 20:39   ` Pierrick Bouvier
2024-10-22 21:37   ` Philippe Mathieu-Daudé
2024-10-22 10:56 ` [PATCH v2 14/20] tests/tcg/aarch64: Use raw strings for regexes in test-mte.py Alex Bennée
2024-10-22 10:56 ` [PATCH v2 15/20] testing: Enhance gdb probe script Alex Bennée
2024-10-22 20:39   ` Pierrick Bouvier
2024-10-22 10:56 ` [PATCH v2 16/20] MAINTAINERS: mention my plugins/next tree Alex Bennée
2024-10-22 20:40   ` Pierrick Bouvier
2024-10-22 10:56 ` [PATCH v2 17/20] plugins: add ability to register a GDB triggered callback Alex Bennée
2024-10-22 20:47   ` Pierrick Bouvier
2024-10-22 10:56 ` [PATCH v2 18/20] meson: build contrib/plugins with meson Alex Bennée
2024-10-23  7:51   ` Pierrick Bouvier
2024-10-23  8:57     ` Alex Bennée
2024-10-23 21:31       ` Pierrick Bouvier
2024-10-22 10:56 ` [PATCH v2 19/20] contrib/plugins: remove Makefile for contrib/plugins Alex Bennée
2024-10-22 10:56 ` [PATCH v2 20/20] plugins: fix qemu_plugin_reset Alex Bennée

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 NNTP newsgroup(s).