From 3df437c737984069fbc12fd0d2e0f2f2c5ba5d4c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Tue, 19 May 2020 09:22:48 -0400 Subject: [PATCH 01/12] configure: add alternate binary for genisoimage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Not all distros ship genisoimage which is a Debian fork from the original cdrtools. As the options are pretty much the same support it as a fallback binary. Signed-off-by: Alex Bennée Signed-off-by: Robert Foley Message-Id: <20200519132259.405-2-robert.foley@linaro.org> --- configure | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure b/configure index b969dee675..af2ba83f0e 100755 --- a/configure +++ b/configure @@ -941,7 +941,7 @@ done # Check for ancillary tools used in testing genisoimage= -for binary in genisoimage +for binary in genisoimage mkisofs do if has $binary then From 92fecad3d3ed3cbfd0a6b4408bb80920de803dca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Tue, 19 May 2020 09:22:49 -0400 Subject: [PATCH 02/12] tests/vm: pass --genisoimage to basevm script MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If we have an alternative to genisoimage we really need to tell the script about it as well so it can use it. It will still default to genisoimage in case it is run outside our build machinery. Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Robert Foley Message-Id: <20200519132259.405-3-robert.foley@linaro.org> --- tests/vm/Makefile.include | 1 + tests/vm/basevm.py | 16 ++++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/tests/vm/Makefile.include b/tests/vm/Makefile.include index 1bf9693d19..74ab522c55 100644 --- a/tests/vm/Makefile.include +++ b/tests/vm/Makefile.include @@ -56,6 +56,7 @@ $(IMAGES_DIR)/%.img: $(SRC_PATH)/tests/vm/% \ $(call quiet-command, \ $(PYTHON) $< \ $(if $(V)$(DEBUG), --debug) \ + $(if $(GENISOIMAGE),--genisoimage $(GENISOIMAGE)) \ --image "$@" \ --force \ --build-image $@, \ diff --git a/tests/vm/basevm.py b/tests/vm/basevm.py index 756ccf7aca..a2d4054d72 100644 --- a/tests/vm/basevm.py +++ b/tests/vm/basevm.py @@ -61,8 +61,9 @@ class BaseVM(object): # 4 is arbitrary, but greater than 2, # since we found we need to wait more than twice as long. tcg_ssh_timeout_multiplier = 4 - def __init__(self, debug=False, vcpus=None): + def __init__(self, debug=False, vcpus=None, genisoimage=None): self._guest = None + self._genisoimage = genisoimage self._tmpdir = os.path.realpath(tempfile.mkdtemp(prefix="vm-test-", suffix=".tmp", dir=".")) @@ -381,12 +382,12 @@ def gen_cloud_init_iso(self): udata.writelines(["apt:\n", " proxy: %s" % proxy]) udata.close() - subprocess.check_call(["genisoimage", "-output", "cloud-init.iso", + subprocess.check_call([self._genisoimage, "-output", "cloud-init.iso", "-volid", "cidata", "-joliet", "-rock", "user-data", "meta-data"], - cwd=cidir, - stdin=self._devnull, stdout=self._stdout, - stderr=self._stdout) + cwd=cidir, + stdin=self._devnull, stdout=self._stdout, + stderr=self._stdout) return os.path.join(cidir, "cloud-init.iso") @@ -424,6 +425,8 @@ def get_default_jobs(): help="Interactively run command") parser.add_option("--snapshot", "-s", action="store_true", help="run tests with a snapshot") + parser.add_option("--genisoimage", default="genisoimage", + help="iso imaging tool") parser.disable_interspersed_args() return parser.parse_args() @@ -435,7 +438,8 @@ def main(vmcls): return 1 logging.basicConfig(level=(logging.DEBUG if args.debug else logging.WARN)) - vm = vmcls(debug=args.debug, vcpus=args.jobs) + vm = vmcls(debug=args.debug, vcpus=args.jobs, + genisoimage=args.genisoimage) if args.build_image: if os.path.exists(args.image) and not args.force: sys.stderr.writelines(["Image file exists: %s\n" % args.image, From 6f83cf88f02a98e857b6cadd4b4e27401b901bba Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Wed, 20 May 2020 15:05:29 +0100 Subject: [PATCH 03/12] travis.yml: Use clang++ in the Clang tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Our configure script does not look for clang++ automatically, so we should use --cxx=clang++ to make sure that we test our C++ code with Clang, too. And while we're at it, also use --host-cc=clang here to avoid that we use the normal "cc" as host C compiler. Signed-off-by: Thomas Huth Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20200518083316.25065-1-thuth@redhat.com> Message-Id: <20200520140541.30256-4-alex.bennee@linaro.org> --- .travis.yml | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/.travis.yml b/.travis.yml index 1ec8a7b465..564be50a3c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -205,14 +205,15 @@ jobs: # Test with Clang for compile portability (Travis uses clang-5.0) - name: "Clang (user)" env: - - CONFIG="--disable-system" + - CONFIG="--disable-system --host-cc=clang --cxx=clang++" - CACHE_NAME="${TRAVIS_BRANCH}-linux-clang-default" compiler: clang - name: "Clang (main-softmmu)" env: - - CONFIG="--target-list=${MAIN_SOFTMMU_TARGETS} " + - CONFIG="--target-list=${MAIN_SOFTMMU_TARGETS} + --host-cc=clang --cxx=clang++" - CACHE_NAME="${TRAVIS_BRANCH}-linux-clang-sanitize" compiler: clang before_script: @@ -222,7 +223,8 @@ jobs: - name: "Clang (other-softmmu)" env: - - CONFIG="--disable-user --target-list-exclude=${MAIN_SOFTMMU_TARGETS}" + - CONFIG="--disable-user --target-list-exclude=${MAIN_SOFTMMU_TARGETS} + --host-cc=clang --cxx=clang++" - CACHE_NAME="${TRAVIS_BRANCH}-linux-clang-default" compiler: clang From be9bc1b73a12f1ebfb2ce2f9edfacc5d1304561a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Wed, 20 May 2020 15:05:30 +0100 Subject: [PATCH 04/12] tests/tcg: fix invocation of the memory record/replay tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I'm not sure when this broke but we should use EXTRA_RUNS for "virtual" tests which are not generated from the binary names. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Message-Id: <20200520140541.30256-5-alex.bennee@linaro.org> --- tests/tcg/aarch64/Makefile.softmmu-target | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/tcg/aarch64/Makefile.softmmu-target b/tests/tcg/aarch64/Makefile.softmmu-target index 71f72cfbe3..1057a8ac49 100644 --- a/tests/tcg/aarch64/Makefile.softmmu-target +++ b/tests/tcg/aarch64/Makefile.softmmu-target @@ -61,7 +61,7 @@ run-memory-replay: memory-replay run-memory-record $(QEMU_OPTS) memory, \ "$< on $(TARGET_NAME)") -EXTRA_TESTS+=memory-record memory-replay +EXTRA_RUNS+=run-memory-replay ifneq ($(DOCKER_IMAGE)$(CROSS_CC_HAS_ARMV8_3),) pauth-3: CFLAGS += -march=armv8.3-a From 91fa8b64cbef14855dc3bd24702416313dea6481 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Wed, 20 May 2020 15:05:31 +0100 Subject: [PATCH 05/12] tests/fp: enable extf80_le_quite tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These have been fixed now so we no longer need a special version of the le_quiet rule to skip the test. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Message-Id: <20200520140541.30256-6-alex.bennee@linaro.org> --- tests/Makefile.include | 7 ------- 1 file changed, 7 deletions(-) diff --git a/tests/Makefile.include b/tests/Makefile.include index 03a74b60f6..e6d87fcbf0 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -741,13 +741,6 @@ check-softfloat-%: $(FP_TEST_BIN) SF_COMPARE_OPS=eq eq_signaling le le_quiet lt_quiet SF_COMPARE_RULES=$(patsubst %,check-softfloat-%, $(SF_COMPARE_OPS)) -# FIXME: extF80_le_quiet (broken) -check-softfloat-le_quiet: $(FP_TEST_BIN) - $(call test-softfloat, \ - f16_le_quiet f32_le_quiet f64_le_quiet \ - f128_le_quiet, \ - le_quiet) - # FIXME: extF80_lt_quiet (broken) check-softfloat-lt_quiet: $(FP_TEST_BIN) $(call test-softfloat, \ From 8281a157c5d01c08e1d684bf7a28c3e2bebf1a96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Wed, 20 May 2020 15:05:32 +0100 Subject: [PATCH 06/12] tests/fp: split and audit the conversion tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Split the float conversion tests into separate groups and audit the tests to check what is still broken. I was able to enable a bunch of tests that had been missed before: all the float to float conversions ui32_to_extF80 ui64_to_extF80 extF80_to_ui32 extF80_to_ui32_r_minMag extF80_to_ui64 extF80_to_ui64_r_minMag Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Message-Id: <20200520140541.30256-7-alex.bennee@linaro.org> --- tests/Makefile.include | 37 ++++++++++++++++++++++++++++++++----- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/tests/Makefile.include b/tests/Makefile.include index e6d87fcbf0..a00ccc94b8 100644 --- a/tests/Makefile.include +++ b/tests/Makefile.include @@ -687,11 +687,26 @@ test-softfloat = $(call quiet-command, \ (cat $2.out && exit 1;), \ "FLOAT TEST", $2) -# Conversion Routines: +# Conversion Routines: Float to Float +# FIXME: f32_to_f128 (broken), f64_to_f128 (broken) +# FIXME: f128_to_f32(broken), f128_to_f64 (broken) +# FIXME: f128_to_extF80 (broken) +check-softfloat-conv-f2f: $(FP_TEST_BIN) + $(call test-softfloat, \ + f16_to_f32 f16_to_f64 \ + f16_to_extF80 f16_to_f128 \ + f32_to_f16 f32_to_f64 \ + f32_to_extF80 \ + f64_to_f16 f64_to_f32 \ + extF80_to_f16 extF80_to_f32 \ + extF80_to_f64 extF80_to_f128 \ + f128_to_f16, \ + float-to-float) + +# Conversion Routines: Int and Uint to Float # FIXME: i32_to_extF80 (broken), i64_to_extF80 (broken) -# ui32_to_f128 (not implemented), extF80_roundToInt (broken) -# -check-softfloat-conv: $(FP_TEST_BIN) +# ui32_to_f128 (not implemented) +check-softfloat-conv-to-float: $(FP_TEST_BIN) $(call test-softfloat, \ i32_to_f16 i64_to_f16 \ i32_to_f32 i64_to_f32 \ @@ -701,7 +716,12 @@ check-softfloat-conv: $(FP_TEST_BIN) ui32_to_f16 ui64_to_f16 \ ui32_to_f32 ui64_to_f32 \ ui32_to_f64 ui64_to_f64 \ + ui32_to_extF80 ui64_to_extF80 \ ui64_to_f128, uint-to-float) + +# Conversion Routines: Float to integers +# FIXME: extF80_roundToInt (broken) +check-softfloat-conv-to-int: $(FP_TEST_BIN) $(call test-softfloat, \ f16_to_i32 f16_to_i32_r_minMag \ f32_to_i32 f32_to_i32_r_minMag \ @@ -718,10 +738,12 @@ check-softfloat-conv: $(FP_TEST_BIN) f16_to_ui32 f16_to_ui32_r_minMag \ f32_to_ui32 f32_to_ui32_r_minMag \ f64_to_ui32 f64_to_ui32_r_minMag \ + extF80_to_ui32 extF80_to_ui32_r_minMag \ f128_to_ui32 f128_to_ui32_r_minMag \ f16_to_ui64 f16_to_ui64_r_minMag \ f32_to_ui64 f32_to_ui64_r_minMag \ f64_to_ui64 f64_to_ui64_r_minMag \ + extF80_to_ui64 extF80_to_ui64_r_minMag \ f128_to_ui64 f128_to_ui64_r_minMag, \ float-to-uint) $(call test-softfloat, \ @@ -729,9 +751,14 @@ check-softfloat-conv: $(FP_TEST_BIN) f64_roundToInt f128_roundToInt, \ round-to-integer) +.PHONY: check-softfloat-conv +check-softfloat-conv: check-softfloat-conv-f2f +check-softfloat-conv: check-softfloat-conv-to-float +check-softfloat-conv: check-softfloat-conv-to-int + # Generic rule for all float operations # -# Some patterns are overidden due to broken or missing tests. +# Some patterns are overridden due to broken or missing tests. # Hopefully these can be removed over time. check-softfloat-%: $(FP_TEST_BIN) From 8ec6f3315118ef75d37e9433dc8faa8383130fb6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Wed, 20 May 2020 15:05:33 +0100 Subject: [PATCH 07/12] tests/tcg: better detect confused gdb which can't connect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit While we may gamely give the right information it can still confuse the wide range of GDBs out there. For example ppc64abi32-linux-user reports: warning: Selected architecture powerpc:common is not compatible with reported target architecture powerpc:common64 warning: Architecture rejected target-supplied description but still connects. Add a test for a 0 pc and exit early if that is the case. This may actually be a bug we need to fix? Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Message-Id: <20200520140541.30256-8-alex.bennee@linaro.org> --- tests/tcg/multiarch/gdbstub/sha1.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/tcg/multiarch/gdbstub/sha1.py b/tests/tcg/multiarch/gdbstub/sha1.py index 734553b98b..2bfde49633 100644 --- a/tests/tcg/multiarch/gdbstub/sha1.py +++ b/tests/tcg/multiarch/gdbstub/sha1.py @@ -65,6 +65,10 @@ def run_test(): print("SKIPPING (not connected)", file=sys.stderr) exit(0) +if gdb.parse_and_eval('$pc') == 0: + print("SKIP: PC not set") + exit(0) + try: # These are not very useful in scripts gdb.execute("set pagination off") From 086f269cf4eb0d759d7f716dcd239e6d310bb955 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Wed, 20 May 2020 15:05:35 +0100 Subject: [PATCH 08/12] tests/docker: add debian11 base image MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We won't use this for building QEMU but we do need newer GCC's and binutils for building some of our test cases. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Message-Id: <20200520140541.30256-10-alex.bennee@linaro.org> --- tests/docker/Makefile.include | 2 +- tests/docker/dockerfiles/debian11.docker | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) create mode 100644 tests/docker/dockerfiles/debian11.docker diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index 43a8678688..3596b58930 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -7,7 +7,7 @@ HOST_ARCH = $(if $(ARCH),$(ARCH),$(shell uname -m)) DOCKER_SUFFIX := .docker DOCKER_FILES_DIR := $(SRC_PATH)/tests/docker/dockerfiles # we don't run tests on intermediate images (used as base by another image) -DOCKER_PARTIAL_IMAGES := debian9 debian10 +DOCKER_PARTIAL_IMAGES := debian9 debian10 debian11 DOCKER_PARTIAL_IMAGES += debian9-mxe debian-bootstrap DOCKER_IMAGES := $(sort $(notdir $(basename $(wildcard $(DOCKER_FILES_DIR)/*.docker)))) DOCKER_TARGETS := $(patsubst %,docker-image-%,$(DOCKER_IMAGES)) diff --git a/tests/docker/dockerfiles/debian11.docker b/tests/docker/dockerfiles/debian11.docker new file mode 100644 index 0000000000..5adfd62d55 --- /dev/null +++ b/tests/docker/dockerfiles/debian11.docker @@ -0,0 +1,18 @@ +# +# Docker multiarch cross-compiler target +# +# This docker target uses the current development version of Debian as +# a base for cross compilers for building test binaries. We won't +# attempt to build QEMU on it yet given it is still in development. +# +# On its own you can't build much but the docker-foo-cross targets +# build on top of the base debian image. +# +FROM debian:bullseye-slim + +# Duplicate deb line as deb-src +RUN cat /etc/apt/sources.list | sed "s/^deb\ /deb-src /" >> /etc/apt/sources.list + +# Install common build utilities +RUN apt update && \ + DEBIAN_FRONTEND=noninteractive apt install -yy eatmydata From c729a99d27018b8d619544b18926b234b010b733 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Wed, 20 May 2020 15:05:36 +0100 Subject: [PATCH 09/12] tests/docker: use a gcc-10 based image for arm64 tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit As we enable newer features that we want to test on arm64 targets we need newer compilers. Split off a new debian-arm64-test-cross image which we can use to build these new tests. Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Message-Id: <20200520140541.30256-11-alex.bennee@linaro.org> --- tests/docker/Makefile.include | 2 ++ .../dockerfiles/debian-arm64-test-cross.docker | 13 +++++++++++++ tests/tcg/configure.sh | 4 ++-- 3 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 tests/docker/dockerfiles/debian-arm64-test-cross.docker diff --git a/tests/docker/Makefile.include b/tests/docker/Makefile.include index 3596b58930..ed46bd98eb 100644 --- a/tests/docker/Makefile.include +++ b/tests/docker/Makefile.include @@ -131,9 +131,11 @@ docker-image-travis: NOUSER=1 # Specialist build images, sometimes very limited tools docker-image-tricore-cross: docker-image-debian9 +docker-image-debian-arm64-test-cross: docker-image-debian11 # These images may be good enough for building tests but not for test builds DOCKER_PARTIAL_IMAGES += debian-alpha-cross +DOCKER_PARTIAL_IMAGES += debian-arm64-test-cross DOCKER_PARTIAL_IMAGES += debian-hppa-cross DOCKER_PARTIAL_IMAGES += debian-m68k-cross debian-mips64-cross DOCKER_PARTIAL_IMAGES += debian-powerpc-cross debian-ppc64-cross diff --git a/tests/docker/dockerfiles/debian-arm64-test-cross.docker b/tests/docker/dockerfiles/debian-arm64-test-cross.docker new file mode 100644 index 0000000000..a44e76d942 --- /dev/null +++ b/tests/docker/dockerfiles/debian-arm64-test-cross.docker @@ -0,0 +1,13 @@ +# +# Docker arm64 cross-compiler target (tests only) +# +# This docker target builds on the debian Bullseye base image. +# +FROM qemu:debian11 + +# Add the foreign architecture we want and install dependencies +RUN dpkg --add-architecture arm64 +RUN apt update && \ + DEBIAN_FRONTEND=noninteractive eatmydata \ + apt install -y --no-install-recommends \ + crossbuild-essential-arm64 gcc-10-aarch64-linux-gnu diff --git a/tests/tcg/configure.sh b/tests/tcg/configure.sh index eaaaff6233..2326f97856 100755 --- a/tests/tcg/configure.sh +++ b/tests/tcg/configure.sh @@ -97,8 +97,8 @@ for target in $target_list; do case $target in aarch64-*) # We don't have any bigendian build tools so we only use this for AArch64 - container_image=debian-arm64-cross - container_cross_cc=aarch64-linux-gnu-gcc + container_image=debian-arm64-test-cross + container_cross_cc=aarch64-linux-gnu-gcc-10 ;; alpha-*) container_image=debian-alpha-cross From 716386e397fabbbf9915d49f8bc79673fd2831bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Wed, 20 May 2020 15:05:38 +0100 Subject: [PATCH 10/12] cpus-common: ensure auto-assigned cpu_indexes don't clash MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Basing the cpu_index on the number of currently allocated vCPUs fails when vCPUs aren't removed in a LIFO manner. This is especially true when we are allocating a cpu_index for each guest thread in linux-user where there is no ordering constraint on their allocation and de-allocation. [I've dropped the assert which is there to guard against out-of-order removal as this should probably be caught higher up the stack. Maybe we could just ifdef CONFIG_SOFTTMU it?] Signed-off-by: Alex Bennée Reviewed-by: Richard Henderson Acked-by: Igor Mammedow Cc: Nikolay Igotti Cc: Paolo Bonzini Cc: Eduardo Habkost Message-Id: <20200520140541.30256-13-alex.bennee@linaro.org> --- cpus-common.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cpus-common.c b/cpus-common.c index 55d5df8923..70a9d12981 100644 --- a/cpus-common.c +++ b/cpus-common.c @@ -61,13 +61,15 @@ static bool cpu_index_auto_assigned; static int cpu_get_free_index(void) { CPUState *some_cpu; - int cpu_index = 0; + int max_cpu_index = 0; cpu_index_auto_assigned = true; CPU_FOREACH(some_cpu) { - cpu_index++; + if (some_cpu->cpu_index >= max_cpu_index) { + max_cpu_index = some_cpu->cpu_index + 1; + } } - return cpu_index; + return max_cpu_index; } void cpu_list_add(CPUState *cpu) @@ -90,8 +92,6 @@ void cpu_list_remove(CPUState *cpu) return; } - assert(!(cpu_index_auto_assigned && cpu != QTAILQ_LAST(&cpus))); - QTAILQ_REMOVE_RCU(&cpus, cpu, node); cpu->cpu_index = UNASSIGNED_CPU_INDEX; } From 1f81ce90e31ef338ee53a0cea02344237bc470cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Wed, 20 May 2020 15:05:39 +0100 Subject: [PATCH 11/12] linux-user: properly "unrealize" vCPU object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We shouldn't be messing around with the CPU list in linux-user save for the very special case of do_fork(). When threads end we need to properly follow QOM object lifetime handling and allow the eventual cpu_common_unrealizefn to both remove the CPU and ensure any clean-up actions are taken place, for example calling plugin exit hooks. There is still a race condition to avoid so use the linux-user specific clone_lock instead of the cpu_list_lock to avoid it. Signed-off-by: Alex Bennée Reviewed-by: Philippe Mathieu-Daudé Cc: Nikolay Igotti Cc: Paolo Bonzini Cc: Daniel P. Berrange Cc: Eduardo Habkost Cc: Markus Armbruster Message-Id: <20200520140541.30256-14-alex.bennee@linaro.org> --- linux-user/syscall.c | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/linux-user/syscall.c b/linux-user/syscall.c index 05f03919ff..7f6700c54e 100644 --- a/linux-user/syscall.c +++ b/linux-user/syscall.c @@ -7635,30 +7635,33 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1, return -TARGET_ERESTARTSYS; } - cpu_list_lock(); + pthread_mutex_lock(&clone_lock); if (CPU_NEXT(first_cpu)) { - TaskState *ts; + TaskState *ts = cpu->opaque; - /* Remove the CPU from the list. */ - QTAILQ_REMOVE_RCU(&cpus, cpu, node); + object_property_set_bool(OBJECT(cpu), false, "realized", NULL); + object_unref(OBJECT(cpu)); + /* + * At this point the CPU should be unrealized and removed + * from cpu lists. We can clean-up the rest of the thread + * data without the lock held. + */ - cpu_list_unlock(); + pthread_mutex_unlock(&clone_lock); - ts = cpu->opaque; if (ts->child_tidptr) { put_user_u32(0, ts->child_tidptr); do_sys_futex(g2h(ts->child_tidptr), FUTEX_WAKE, INT_MAX, NULL, NULL, 0); } thread_cpu = NULL; - object_unref(OBJECT(cpu)); g_free(ts); rcu_unregister_thread(); pthread_exit(NULL); } - cpu_list_unlock(); + pthread_mutex_unlock(&clone_lock); preexit_cleanup(cpu_env, arg1); _exit(arg1); return 0; /* avoid warning */ From 919bfbf5d6569b63a374332292cf3d2355a6d6c3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alex=20Benn=C3=A9e?= Date: Wed, 20 May 2020 15:05:40 +0100 Subject: [PATCH 12/12] tests/tcg: add new threadcount test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Based on the original testcase by Nikolay Igotti. Message-ID: Signed-off-by: Nikolay Igotti Signed-off-by: Alex Bennée Message-Id: <20200520140541.30256-15-alex.bennee@linaro.org> --- tests/tcg/multiarch/Makefile.target | 2 + tests/tcg/multiarch/threadcount.c | 64 +++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 tests/tcg/multiarch/threadcount.c diff --git a/tests/tcg/multiarch/Makefile.target b/tests/tcg/multiarch/Makefile.target index 51fb75ecfd..cb49cc9ccb 100644 --- a/tests/tcg/multiarch/Makefile.target +++ b/tests/tcg/multiarch/Makefile.target @@ -28,6 +28,8 @@ run-float_%: float_% testthread: LDFLAGS+=-lpthread +threadcount: LDFLAGS+=-lpthread + # We define the runner for test-mmap after the individual # architectures have defined their supported pages sizes. If no # additional page sizes are defined we only run the default test. diff --git a/tests/tcg/multiarch/threadcount.c b/tests/tcg/multiarch/threadcount.c new file mode 100644 index 0000000000..545a1c8146 --- /dev/null +++ b/tests/tcg/multiarch/threadcount.c @@ -0,0 +1,64 @@ +/* + * Thread Exerciser + * + * Unlike testthread which is mainly concerned about testing thread + * semantics this test is used to exercise the thread creation and + * accounting. A version of this test found a problem with clashing + * cpu_indexes which caused a break in plugin handling. + * + * Based on the original test case by Nikolay Igotti. + * + * Copyright (c) 2020 Linaro Ltd + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ + +#include +#include +#include +#include +#include + +int max_threads = 10; + +typedef struct { + int delay; +} ThreadArg; + +static void *thread_fn(void* varg) +{ + ThreadArg *arg = varg; + usleep(arg->delay); + free(arg); + return NULL; +} + +int main(int argc, char **argv) +{ + int i; + pthread_t *threads; + + if (argc > 1) { + max_threads = atoi(argv[1]); + } + threads = calloc(sizeof(pthread_t), max_threads); + + for (i = 0; i < max_threads; i++) { + ThreadArg *arg = calloc(sizeof(ThreadArg), 1); + arg->delay = i * 100; + pthread_create(threads + i, NULL, thread_fn, arg); + } + + printf("Created %d threads\n", max_threads); + + /* sleep until roughly half the threads have "finished" */ + usleep(max_threads * 50); + + for (i = 0; i < max_threads; i++) { + pthread_join(threads[i], NULL); + } + + printf("Done\n"); + + return 0; +}