From 34aee9c94691f529cd952f9483a6b357ca098042 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Wed, 8 Nov 2023 09:59:54 +0100 Subject: [PATCH 01/12] host/include/generic/host/atomic128: Fix compilation problem with Clang 17 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When compiling QEMU with Clang 17 on a s390x, the compilation fails: In file included from ../accel/tcg/cputlb.c:32: In file included from /root/qemu/include/exec/helper-proto-common.h:10: In file included from /root/qemu/include/qemu/atomic128.h:62: /root/qemu/host/include/generic/host/atomic128-ldst.h:68:15: error: __sync builtin operation MUST have natural alignment (consider using __ atomic). [-Werror,-Wsync-alignment] 68 | } while (!__sync_bool_compare_and_swap_16(ptr_align, old, new.i)); | ^ In file included from ../accel/tcg/cputlb.c:32: In file included from /root/qemu/include/exec/helper-proto-common.h:10: In file included from /root/qemu/include/qemu/atomic128.h:61: /root/qemu/host/include/generic/host/atomic128-cas.h:36:11: error: __sync builtin operation MUST have natural alignment (consider using __a tomic). [-Werror,-Wsync-alignment] 36 | r.i = __sync_val_compare_and_swap_16(ptr_align, c.i, n.i); | ^ 2 errors generated. It's arguably a bug in Clang since we already use __builtin_assume_aligned() to tell the compiler that the pointer is properly aligned. But according to https://github.com/llvm/llvm-project/issues/69146 it seems like the Clang folks don't see an easy fix on their side and recommend to use a type declared with __attribute__((aligned(16))) to work around this problem. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1934 Message-ID: <20231108085954.313071-1-thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Reviewed-by: Richard Henderson Signed-off-by: Thomas Huth --- host/include/generic/host/atomic128-cas.h | 2 +- host/include/generic/host/atomic128-ldst.h | 2 +- include/qemu/int128.h | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/host/include/generic/host/atomic128-cas.h b/host/include/generic/host/atomic128-cas.h index 991d3da082..6b40cc2271 100644 --- a/host/include/generic/host/atomic128-cas.h +++ b/host/include/generic/host/atomic128-cas.h @@ -28,7 +28,7 @@ atomic16_cmpxchg(Int128 *ptr, Int128 cmp, Int128 new) static inline Int128 ATTRIBUTE_ATOMIC128_OPT atomic16_cmpxchg(Int128 *ptr, Int128 cmp, Int128 new) { - __int128_t *ptr_align = __builtin_assume_aligned(ptr, 16); + Int128Aligned *ptr_align = __builtin_assume_aligned(ptr, 16); Int128Alias r, c, n; c.s = cmp; diff --git a/host/include/generic/host/atomic128-ldst.h b/host/include/generic/host/atomic128-ldst.h index 80fff0643a..691e6a8531 100644 --- a/host/include/generic/host/atomic128-ldst.h +++ b/host/include/generic/host/atomic128-ldst.h @@ -58,7 +58,7 @@ atomic16_read_rw(Int128 *ptr) static inline void ATTRIBUTE_ATOMIC128_OPT atomic16_set(Int128 *ptr, Int128 val) { - __int128_t *ptr_align = __builtin_assume_aligned(ptr, 16); + Int128Aligned *ptr_align = __builtin_assume_aligned(ptr, 16); __int128_t old; Int128Alias new; diff --git a/include/qemu/int128.h b/include/qemu/int128.h index 73624e8be7..174bd7dafb 100644 --- a/include/qemu/int128.h +++ b/include/qemu/int128.h @@ -10,6 +10,7 @@ */ #if defined(CONFIG_INT128) && !defined(CONFIG_TCG_INTERPRETER) typedef __int128_t Int128; +typedef __int128_t __attribute__((aligned(16))) Int128Aligned; static inline Int128 int128_make64(uint64_t a) { @@ -224,6 +225,7 @@ static inline Int128 int128_rems(Int128 a, Int128 b) #else /* !CONFIG_INT128 */ typedef struct Int128 Int128; +typedef struct Int128 __attribute__((aligned(16))) Int128Aligned; /* * We guarantee that the in-memory byte representation of an From 0ab35658401a2e0a411ce0d1836a4f509bde717a Mon Sep 17 00:00:00 2001 From: Matthew Rosato Date: Fri, 10 Nov 2023 12:51:07 -0500 Subject: [PATCH 02/12] s390x/pci: bypass vfio DMA counting when using cdev The current code assumes that there is always a vfio group, but that's no longer guaranteed with the iommufd backend when using cdev. In this case, we don't need to track the vfio dma limit anyway. Signed-off-by: Matthew Rosato Message-ID: <20231110175108.465851-2-mjrosato@linux.ibm.com> Signed-off-by: Thomas Huth --- hw/s390x/s390-pci-vfio.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c index 59a2e03873..e28573b593 100644 --- a/hw/s390x/s390-pci-vfio.c +++ b/hw/s390x/s390-pci-vfio.c @@ -66,6 +66,10 @@ S390PCIDMACount *s390_pci_start_dma_count(S390pciState *s, assert(vpdev); + if (!vpdev->vbasedev.group) { + return NULL; + } + id = vpdev->vbasedev.group->container->fd; if (!s390_pci_update_dma_avail(id, &avail)) { From 8011b508cf0ddbdbda03820f4fa6cd484a6d9aed Mon Sep 17 00:00:00 2001 From: Matthew Rosato Date: Fri, 10 Nov 2023 12:51:08 -0500 Subject: [PATCH 03/12] s390x/pci: only limit DMA aperture if vfio DMA limit reported If the host kernel lacks vfio DMA limit reporting, do not attempt to shrink the guest DMA aperture. Fixes: df202e3ff3 ("s390x/pci: shrink DMA aperture to be bound by vfio DMA limit") Signed-off-by: Matthew Rosato Message-ID: <20231110175108.465851-3-mjrosato@linux.ibm.com> Signed-off-by: Thomas Huth --- hw/s390x/s390-pci-vfio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c index e28573b593..7dbbc76823 100644 --- a/hw/s390x/s390-pci-vfio.c +++ b/hw/s390x/s390-pci-vfio.c @@ -136,7 +136,7 @@ static void s390_pci_read_base(S390PCIBusDevice *pbdev, * to the guest based upon the vfio DMA limit. */ vfio_size = pbdev->iommu->max_dma_limit << TARGET_PAGE_BITS; - if (vfio_size < (cap->end_dma - cap->start_dma + 1)) { + if (vfio_size > 0 && vfio_size < cap->end_dma - cap->start_dma + 1) { pbdev->zpci_fn.edma = cap->start_dma + vfio_size - 1; } } From 4940da2096f969cb21fc23e0fd6f52e766bf4fdf Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 20 Oct 2023 08:09:33 +0200 Subject: [PATCH 04/12] MAINTAINERS: Add include/hw/input/pl050.h to the PrimeCell/CMSDK section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The corresponding pl050.c file is already listed here, so we should mention the header here, too. Message-ID: <20231020060936.524988-2-thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index e73a3ff544..ccc7aa3e3e 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -657,6 +657,7 @@ F: include/hw/dma/pl080.h F: hw/dma/pl330.c F: hw/gpio/pl061.c F: hw/input/pl050.c +F: include/hw/input/pl050.h F: hw/intc/pl190.c F: hw/sd/pl181.c F: hw/ssi/pl022.c From 261c1281e87e030ba8c1ceb45b34ae03d4c7972f Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 20 Oct 2023 08:09:34 +0200 Subject: [PATCH 05/12] MAINTAINERS: Add hw/input/ads7846.c to the PXA2XX section The code from hw/input/ads7846.c is only used by hw/arm/spitz.c, so add this file to the same section where hw/arm/spitz.c is listed. Message-ID: <20231020060936.524988-3-thuth@redhat.com> Reviewed-by: Peter Maydell Signed-off-by: Thomas Huth --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index ccc7aa3e3e..e82b64d1ca 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -928,6 +928,7 @@ F: hw/*/pxa2xx* F: hw/display/tc6393xb.c F: hw/gpio/max7310.c F: hw/gpio/zaurus.c +F: hw/input/ads7846.c F: hw/misc/mst_fpga.c F: hw/adc/max111x.c F: include/hw/adc/max111x.h From 42c31682bae341bc16a92e7eb94587a6b4d84dfa Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 20 Oct 2023 08:09:35 +0200 Subject: [PATCH 06/12] MAINTAINERS: Add hw/display/sii9022.c to the Versatile Express section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This graphics adapter is only used by the Versatile Express machine, so add it to the corresponding section in MAINTAINERS. Message-ID: <20231020060936.524988-4-thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index e82b64d1ca..cc13f15996 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -996,6 +996,7 @@ M: Peter Maydell L: qemu-arm@nongnu.org S: Maintained F: hw/arm/vexpress.c +F: hw/display/sii9022.c F: docs/system/arm/vexpress.rst Versatile PB From 7c7e1f6017ece833499ff459fea1e6d32f543940 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 20 Oct 2023 08:09:36 +0200 Subject: [PATCH 07/12] MAINTAINERS: Extend the Stellaris section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This header include/hw/timer/stellaris-gptm.h obviously belongs to the Stellaris machines, so let's add it to the corresponding section. And hw/display/ssd0303.c and hw/display/ssd0323.c are only used by hw/arm/stellaris.c, so add them to the corresponding section in the MAINTAINERS file, too. Message-ID: <20231020060936.524988-5-thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- MAINTAINERS | 2 ++ 1 file changed, 2 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index cc13f15996..c127a373ab 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -981,7 +981,9 @@ M: Peter Maydell L: qemu-arm@nongnu.org S: Maintained F: hw/*/stellaris* +F: hw/display/ssd03* F: include/hw/input/gamepad.h +F: include/hw/timer/stellaris-gptm.h F: docs/system/arm/stellaris.rst STM32VLDISCOVERY From d229996b402e6d08764f3da5a49aa3a25193db47 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 29 Sep 2023 15:45:51 +0200 Subject: [PATCH 08/12] MAINTAINERS: Add a general architecture section for x86 It's a little bit weird that the files in target/i386/ which are not in a subfolder there do not have any associated maintainer (and thus nobody might be CC:-ed on changes to these files). We should have a general x86 section for these files, similar to what we already have for s390x and mips. Since Paolo is already listed as maintainer for both, the x86 KVM and TCG CPUs, I'd like to suggest him as maintainer for the general files, too. Message-ID: <20230929134551.395438-1-thuth@redhat.com> Signed-off-by: Thomas Huth --- MAINTAINERS | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/MAINTAINERS b/MAINTAINERS index c127a373ab..6999b26a0f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -131,6 +131,17 @@ K: ^Subject:.*(?i)mips F: docs/system/target-mips.rst F: configs/targets/mips* +X86 general architecture support +M: Paolo Bonzini +S: Maintained +F: configs/devices/i386-softmmu/default.mak +F: configs/targets/i386-softmmu.mak +F: configs/targets/x86_64-softmmu.mak +F: docs/system/target-i386* +F: target/i386/*.[ch] +F: target/i386/Kconfig +F: target/i386/meson.build + Guest CPU cores (TCG) --------------------- Overall TCG CPUs From 00ac955b06b28803319159551bfed6a130f8ec2f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Thu, 9 Nov 2023 16:09:00 +0100 Subject: [PATCH 09/12] tests/vm/netbsd: Use Python v3.11 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We requiere the 'ninja-build', which depends on 'python311': $ pkgin show-deps ninja-build direct dependencies for ninja-build-1.11.1nb1 python311>=3.11.0 So we end up installing both Python v3.10 and v3.11: [31/76] installing python311-3.11.5... [54/76] installing python310-3.10.13... [74/76] installing py310-expat-3.10.13nb1... Then the build system picks Python v3.11, and doesn't find py-expat because we only installed the 3.10 version: python determined to be '/usr/pkg/bin/python3.11' python version: Python 3.11.5 *** Ouch! *** Python's pyexpat module is not found. It's normally part of the Python standard library, maybe your distribution packages it separately? Either install pyexpat, or alleviate the need for it in the first place by installing pip and setuptools for '/usr/pkg/bin/python3.11'. (Hint: NetBSD's pkgsrc debundles this to e.g. 'py310-expat'.) ERROR: python venv creation failed Fix by installing py-expat for v3.11. Remove the v3.10 packages since we aren't using them anymore. Signed-off-by: Philippe Mathieu-Daudé Tested-by: Thomas Huth Message-ID: <20231109150900.91186-1-philmd@linaro.org> Signed-off-by: Thomas Huth --- tests/vm/netbsd | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/vm/netbsd b/tests/vm/netbsd index 40b27a3469..649fcad353 100755 --- a/tests/vm/netbsd +++ b/tests/vm/netbsd @@ -30,8 +30,8 @@ class NetBSDVM(basevm.BaseVM): "git-base", "pkgconf", "xz", - "python310", - "py310-expat", + "python311", + "py311-expat", "ninja-build", # gnu tools From 2e990d81d9813628148f64b944691b5cbc251fab Mon Sep 17 00:00:00 2001 From: Eric Auger Date: Fri, 10 Nov 2023 09:36:54 +0100 Subject: [PATCH 10/12] test-resv-mem: Fix CID 1523911 Coverity complains about passing "&expected" to "run_range_inverse_array", which dereferences null "expected". I guess the problem is that the compare_ranges() loop dereferences 'e' without testing it. However the loop condition is based on 'ranges' which is garanteed to have the same length as 'expected' given the g_assert_cmpint() just before the loop. So the code looks safe to me. Nevertheless adding a test on expected before the loop to get rid of the warning. Fixes: CID 1523901 Signed-off-by: Eric Auger Reported-by: Coverity (CID 1523901) Message-ID: <20231110083654.277345-1-eric.auger@redhat.com> Reviewed-by: Thomas Huth Signed-off-by: Thomas Huth --- tests/unit/test-resv-mem.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/unit/test-resv-mem.c b/tests/unit/test-resv-mem.c index 5963274e2c..cd8f7318cc 100644 --- a/tests/unit/test-resv-mem.c +++ b/tests/unit/test-resv-mem.c @@ -44,6 +44,10 @@ static void compare_ranges(const char *prefix, GList *ranges, print_ranges("out", ranges); print_ranges("expected", expected); #endif + if (!expected) { + g_assert_true(!ranges); + return; + } g_assert_cmpint(g_list_length(ranges), ==, g_list_length(expected)); for (l = ranges, e = expected; l ; l = l->next, e = e->next) { Range *r = (Range *)l->data; From f9a19bd8d23f0048315a60d695857c705988dc05 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Thu, 9 Nov 2023 18:47:20 +0100 Subject: [PATCH 11/12] tests/tsan: Rename the file with the entries that should be ignored MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Let's use a better file name here. Message-ID: <20231109174720.375873-1-thuth@redhat.com> Reviewed-by: Philippe Mathieu-Daudé Signed-off-by: Thomas Huth --- docs/devel/testing.rst | 4 ++-- tests/tsan/{blacklist.tsan => ignore.tsan} | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) rename tests/tsan/{blacklist.tsan => ignore.tsan} (57%) diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index b0680cbb22..fef64accc1 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst @@ -668,11 +668,11 @@ suppressing it. More information on the file format can be found here: https://github.com/google/sanitizers/wiki/ThreadSanitizerSuppressions -tests/tsan/blacklist.tsan - Has TSan warnings we wish to disable +tests/tsan/ignore.tsan - Has TSan warnings we wish to disable at compile time for test or debug. Add flags to configure to enable: -"--extra-cflags=-fsanitize-blacklist=/tests/tsan/blacklist.tsan" +"--extra-cflags=-fsanitize-blacklist=/tests/tsan/ignore.tsan" More information on the file format can be found here under "Blacklist Format": diff --git a/tests/tsan/blacklist.tsan b/tests/tsan/ignore.tsan similarity index 57% rename from tests/tsan/blacklist.tsan rename to tests/tsan/ignore.tsan index 75e444f5dc..423e482d2f 100644 --- a/tests/tsan/blacklist.tsan +++ b/tests/tsan/ignore.tsan @@ -1,6 +1,6 @@ -# This is an example blacklist. -# To enable use of the blacklist add this to configure: -# "--extra-cflags=-fsanitize-blacklist=/tests/tsan/blacklist.tsan" +# This is an example ignore list. +# To enable use of the ignore list add this to configure: +# "--extra-cflags=-fsanitize-blacklist=/tests/tsan/ignore.tsan" # The eventual goal would be to fix these warnings. # TSan is not happy about setting/getting of dirty bits, From 4409a6d85522925df580554d476161a570bb1ed9 Mon Sep 17 00:00:00 2001 From: Peter Maydell Date: Fri, 10 Nov 2023 16:43:18 +0000 Subject: [PATCH 12/12] hw/audio/es1370: Clean up comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace a sweary comment with one that's a bit more helpful to future readers of the code. Signed-off-by: Peter Maydell Reviewed-by: Volker Rümelin Message-ID: <20231110164318.2197569-1-peter.maydell@linaro.org> Signed-off-by: Thomas Huth --- hw/audio/es1370.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/hw/audio/es1370.c b/hw/audio/es1370.c index 91c47330ad..fad5541211 100644 --- a/hw/audio/es1370.c +++ b/hw/audio/es1370.c @@ -670,8 +670,13 @@ static void es1370_transfer_audio (ES1370State *s, struct chan *d, int loop_sel, cnt += (transferred + d->leftover) >> 2; if (s->sctl & loop_sel) { - /* Bah, how stupid is that having a 0 represent true value? - i just spent few hours on this shit */ + /* + * loop_sel tells us which bit in the SCTL register to look at + * (either P1_LOOP_SEL, P2_LOOP_SEL or R1_LOOP_SEL). The sense + * of these bits is 0 for loop mode (set interrupt and keep recording + * when the sample count reaches zero) or 1 for stop mode (set + * interrupt and stop recording). + */ AUD_log ("es1370: warning", "non looping mode\n"); } else { d->frame_cnt = size;