From 6c5a1467f8d0a9e840c8aa193bc110cc76ee80e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 28 Feb 2025 10:27:32 +0000 Subject: [PATCH 01/15] tests/functional: remove unused 'bin_prefix' variable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This was copied over from avocado but has not been used in the new functional tests. Reviewed-by: Thomas Huth Signed-off-by: Daniel P. Berrangé Reviewed-by: Richard Henderson Message-ID: <20250228102738.3064045-2-berrange@redhat.com> Signed-off-by: Thomas Huth --- tests/functional/qemu_test/testcase.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py index 869f3949fe..9d5611c4d7 100644 --- a/tests/functional/qemu_test/testcase.py +++ b/tests/functional/qemu_test/testcase.py @@ -192,7 +192,7 @@ class QemuBaseTest(unittest.TestCase): return False return True - def setUp(self, bin_prefix): + def setUp(self): self.assertIsNotNone(self.qemu_bin, 'QEMU_TEST_QEMU_BINARY must be set') self.arch = self.qemu_bin.split('-')[-1] self.socketdir = None @@ -254,7 +254,7 @@ class QemuBaseTest(unittest.TestCase): class QemuUserTest(QemuBaseTest): def setUp(self): - super().setUp('qemu-') + super().setUp() self._ldpath = [] def add_ldpath(self, ldpath): @@ -277,7 +277,7 @@ class QemuSystemTest(QemuBaseTest): def setUp(self): self._vms = {} - super().setUp('qemu-system-') + super().setUp() console_log = logging.getLogger('console') console_log.setLevel(logging.DEBUG) From 8188356a260ca0201c42d128d8fa86f40160b513 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 28 Feb 2025 10:27:33 +0000 Subject: [PATCH 02/15] tests/functional: set 'qemu_bin' as an object level field MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 'qemu_bin' field is currently set on the class, despite being accessed as if it were an object instance field with 'self.qemu_bin'. This is no obvious need to have it as a class field, so move it into the object instance. Reviewed-by: Thomas Huth Signed-off-by: Daniel P. Berrangé Reviewed-by: Richard Henderson Message-ID: <20250228102738.3064045-3-berrange@redhat.com> Signed-off-by: Thomas Huth --- docs/devel/testing/functional.rst | 2 +- tests/functional/qemu_test/testcase.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/devel/testing/functional.rst b/docs/devel/testing/functional.rst index ecc738922b..bcb5509512 100644 --- a/docs/devel/testing/functional.rst +++ b/docs/devel/testing/functional.rst @@ -173,7 +173,7 @@ QEMU binary selection ^^^^^^^^^^^^^^^^^^^^^ The QEMU binary used for the ``self.vm`` QEMUMachine instance will -primarily depend on the value of the ``qemu_bin`` class attribute. +primarily depend on the value of the ``qemu_bin`` instance attribute. If it is not explicitly set by the test code, its default value will be the result the QEMU_TEST_QEMU_BINARY environment variable. diff --git a/tests/functional/qemu_test/testcase.py b/tests/functional/qemu_test/testcase.py index 9d5611c4d7..058bf270ec 100644 --- a/tests/functional/qemu_test/testcase.py +++ b/tests/functional/qemu_test/testcase.py @@ -33,7 +33,6 @@ from .uncompress import uncompress class QemuBaseTest(unittest.TestCase): - qemu_bin = os.getenv('QEMU_TEST_QEMU_BINARY') arch = None workdir = None @@ -193,6 +192,7 @@ class QemuBaseTest(unittest.TestCase): return True def setUp(self): + self.qemu_bin = os.getenv('QEMU_TEST_QEMU_BINARY') self.assertIsNotNone(self.qemu_bin, 'QEMU_TEST_QEMU_BINARY must be set') self.arch = self.qemu_bin.split('-')[-1] self.socketdir = None From 188f71929520940048ec5ba85ea30588e0566e6e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 28 Feb 2025 10:27:35 +0000 Subject: [PATCH 03/15] tests/functional: reduce tuxrun maxmem to work on 32-bit hosts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit maxmem=4G is too large to address on 32-bit hosts, so reduce it to 2G since the tuxrun tests don't actually need such an elevated memory limit. Signed-off-by: Daniel P. Berrangé Reviewed-by: Thomas Huth Reviewed-by: Richard Henderson Message-ID: <20250228102738.3064045-5-berrange@redhat.com> Signed-off-by: Thomas Huth --- tests/functional/test_ppc64_tuxrun.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/functional/test_ppc64_tuxrun.py b/tests/functional/test_ppc64_tuxrun.py index 05c6162b5e..e8f79c676e 100755 --- a/tests/functional/test_ppc64_tuxrun.py +++ b/tests/functional/test_ppc64_tuxrun.py @@ -64,7 +64,7 @@ class TuxRunPPC64Test(TuxRunBaselineTest): ',"index":1,"id":"pci.1"}') self.vm.add_args('-device', '{"driver":"spapr-vscsi","id":"scsi1"' ',"reg":12288}') - self.vm.add_args('-m', '2G,slots=32,maxmem=4G', + self.vm.add_args('-m', '1G,slots=32,maxmem=2G', '-object', 'memory-backend-ram,id=ram1,size=1G', '-device', 'pc-dimm,id=dimm1,memdev=ram1') From 42ea7f782a32df4ac58e7d9d73e736def3057ef7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 28 Feb 2025 10:27:36 +0000 Subject: [PATCH 04/15] tests/functional: skip memaddr tests on 32-bit builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the QEMU binary was built for a 32-bit ELF target we cannot run the memory address space tests as they all require ability to address more RAM that can be represented on 32-bit. We can't use a decorator to skip the tests as we need setUp() to run to pick the QEMU binary, thus we must call a method at the start of each test to check and skip it. The functional result is effectively the same as using a decorator, just less pretty. This code will go away when 32-bit hosts are full dropped from QEMU. The code allows any non-ELF target since all macOS versions supported at 64-bit only and we already dropped support for 32-bit Windows. Signed-off-by: Daniel P. Berrangé Message-ID: <20250228102738.3064045-6-berrange@redhat.com> [thuth: Add missing byteorder='little' to from_bytes()] Signed-off-by: Thomas Huth --- tests/functional/test_mem_addr_space.py | 34 +++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/tests/functional/test_mem_addr_space.py b/tests/functional/test_mem_addr_space.py index bb0cf062ca..2d9d31efb5 100755 --- a/tests/functional/test_mem_addr_space.py +++ b/tests/functional/test_mem_addr_space.py @@ -20,6 +20,25 @@ class MemAddrCheck(QemuSystemTest): # this reason. DELAY_Q35_BOOT_SEQUENCE = 1 + # This helper can go away when the 32-bit host deprecation + # turns into full & final removal of support. + def ensure_64bit_binary(self): + with open(self.qemu_bin, "rb") as fh: + ident = fh.read(4) + + # "\x7fELF" + if ident != bytes([0x7f, 0x45, 0x4C, 0x46]): + # Non-ELF file implies macOS or Windows which + # we already assume to be 64-bit only + return + + # bits == 1 -> 32-bit; bits == 2 -> 64-bit + bits = int.from_bytes(fh.read(1), byteorder='little') + if bits != 2: + # 32-bit ELF builds won't be able to address sufficient + # RAM to run the tests + self.skipTest("64-bit build host is required") + # first, lets test some 32-bit processors. # for all 32-bit cases, pci64_hole_size is 0. def test_phybits_low_pse36(self): @@ -38,6 +57,7 @@ class MemAddrCheck(QemuSystemTest): If maxmem is set to 59.5G with all other QEMU parameters identical, QEMU should start fine. """ + self.ensure_64bit_binary() self.vm.add_args('-S', '-machine', 'q35', '-m', '512,slots=1,maxmem=59.6G', '-cpu', 'pentium,pse36=on', '-display', 'none', @@ -55,6 +75,7 @@ class MemAddrCheck(QemuSystemTest): access up to a maximum of 64GiB of memory. Rest is the same as the case with pse36 above. """ + self.ensure_64bit_binary() self.vm.add_args('-S', '-machine', 'q35', '-m', '512,slots=1,maxmem=59.6G', '-cpu', 'pentium,pae=on', '-display', 'none', @@ -71,6 +92,7 @@ class MemAddrCheck(QemuSystemTest): Setting maxmem to 59.5G and making sure that QEMU can start with the same options as the failing case above with pse36 cpu feature. """ + self.ensure_64bit_binary() self.vm.add_args('-machine', 'q35', '-m', '512,slots=1,maxmem=59.5G', '-cpu', 'pentium,pse36=on', '-display', 'none', @@ -88,6 +110,7 @@ class MemAddrCheck(QemuSystemTest): Setting maxmem to 59.5G and making sure that QEMU can start fine with the same options as the case above. """ + self.ensure_64bit_binary() self.vm.add_args('-machine', 'q35', '-m', '512,slots=1,maxmem=59.5G', '-cpu', 'pentium,pae=on', '-display', 'none', @@ -104,6 +127,7 @@ class MemAddrCheck(QemuSystemTest): Pentium2 has 36 bits of addressing, so its same as pentium with pse36 ON. """ + self.ensure_64bit_binary() self.vm.add_args('-machine', 'q35', '-m', '512,slots=1,maxmem=59.5G', '-cpu', 'pentium2', '-display', 'none', @@ -123,6 +147,7 @@ class MemAddrCheck(QemuSystemTest): message because the region for memory hotplug is always placed above 4 GiB due to the PCI hole and simplicity. """ + self.ensure_64bit_binary() self.vm.add_args('-S', '-machine', 'q35', '-m', '512,slots=1,maxmem=4G', '-cpu', 'pentium', '-display', 'none', @@ -150,6 +175,7 @@ class MemAddrCheck(QemuSystemTest): which is equal to 987.5 GiB. Setting the value to 988 GiB should make QEMU fail with the error message. """ + self.ensure_64bit_binary() self.vm.add_args('-S', '-machine', 'pc-q35-7.0', '-m', '512,slots=1,maxmem=988G', '-display', 'none', @@ -170,6 +196,7 @@ class MemAddrCheck(QemuSystemTest): Make sure QEMU fails when maxmem size is 976 GiB (12 GiB less than 988 GiB). """ + self.ensure_64bit_binary() self.vm.add_args('-S', '-machine', 'pc-q35-7.1', '-m', '512,slots=1,maxmem=976G', '-display', 'none', @@ -186,6 +213,7 @@ class MemAddrCheck(QemuSystemTest): Same as q35-7.0 AMD case except that here we check that QEMU can successfully start when maxmem is < 988G. """ + self.ensure_64bit_binary() self.vm.add_args('-S', '-machine', 'pc-q35-7.0', '-m', '512,slots=1,maxmem=987.5G', '-display', 'none', @@ -202,6 +230,7 @@ class MemAddrCheck(QemuSystemTest): Same as q35-7.1 AMD case except that here we check that QEMU can successfully start when maxmem is < 976G. """ + self.ensure_64bit_binary() self.vm.add_args('-S', '-machine', 'pc-q35-7.1', '-m', '512,slots=1,maxmem=975.5G', '-display', 'none', @@ -219,6 +248,7 @@ class MemAddrCheck(QemuSystemTest): Intel cpu instead. QEMU should start fine in this case as "above_4G" memory starts at 4G. """ + self.ensure_64bit_binary() self.vm.add_args('-S', '-cpu', 'Skylake-Server', '-machine', 'pc-q35-7.1', '-m', '512,slots=1,maxmem=976G', @@ -243,6 +273,7 @@ class MemAddrCheck(QemuSystemTest): memory for the VM (1024 - 32 - 1 + 0.5). With 992 GiB, QEMU should fail to start. """ + self.ensure_64bit_binary() self.vm.add_args('-S', '-cpu', 'EPYC-v4,phys-bits=41', '-machine', 'pc-q35-7.1', '-m', '512,slots=1,maxmem=992G', @@ -261,6 +292,7 @@ class MemAddrCheck(QemuSystemTest): Same as above but by setting maxram between 976 GiB and 992 Gib, QEMU should start fine. """ + self.ensure_64bit_binary() self.vm.add_args('-S', '-cpu', 'EPYC-v4,phys-bits=41', '-machine', 'pc-q35-7.1', '-m', '512,slots=1,maxmem=990G', @@ -281,6 +313,7 @@ class MemAddrCheck(QemuSystemTest): So maxmem here should be at most 986 GiB considering all memory boundary alignment constraints with 40 bits (1 TiB) of processor physical bits. """ + self.ensure_64bit_binary() self.vm.add_args('-S', '-cpu', 'Skylake-Server,phys-bits=40', '-machine', 'q35,cxl=on', '-m', '512,slots=1,maxmem=987G', @@ -299,6 +332,7 @@ class MemAddrCheck(QemuSystemTest): with the exact same parameters as above, QEMU should start fine even with cxl enabled. """ + self.ensure_64bit_binary() self.vm.add_args('-S', '-cpu', 'Skylake-Server,phys-bits=40', '-machine', 'q35,cxl=on', '-m', '512,slots=1,maxmem=987G', From 5ad2c8f357a76bbc502452c60076a4b36708f46a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 28 Feb 2025 10:27:37 +0000 Subject: [PATCH 05/15] tests/functional: drop unused 'get_tag' method MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Daniel P. Berrangé Reviewed-by: Thomas Huth Message-ID: <20250228102738.3064045-7-berrange@redhat.com> Signed-off-by: Thomas Huth --- tests/functional/qemu_test/tuxruntest.py | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/tests/functional/qemu_test/tuxruntest.py b/tests/functional/qemu_test/tuxruntest.py index 41a4945a14..ad74156f9c 100644 --- a/tests/functional/qemu_test/tuxruntest.py +++ b/tests/functional/qemu_test/tuxruntest.py @@ -24,17 +24,6 @@ class TuxRunBaselineTest(QemuSystemTest): # Tests are ~10-40s, allow for --debug/--enable-gcov overhead timeout = 100 - def get_tag(self, tagname, default=None): - """ - Get the metadata tag or return the default. - """ - utag = self._get_unique_tag_val(tagname) - print(f"{tagname}/{default} -> {utag}") - if utag: - return utag - - return default - def setUp(self): super().setUp() From 981395889201f556c37e18c7a896d2555ffa4373 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 28 Feb 2025 10:27:38 +0000 Subject: [PATCH 06/15] tests/functional: stop output from zstd command when uncompressing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The zstd command will print incremental decompression progress to stderr when running. Fortunately it is not on stdout as that would confuse the TAP parsing, but we should still not have this printed. By switching from 'check_call' to 'run' with the check=True and capture_output=True we'll get the desired silence on success, and on failure the raised exception will automatically include stdout/stderr data for diagnosis purposes. Signed-off-by: Daniel P. Berrangé Reviewed-by: Thomas Huth Message-ID: <20250228102738.3064045-8-berrange@redhat.com> Signed-off-by: Thomas Huth --- tests/functional/qemu_test/uncompress.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/functional/qemu_test/uncompress.py b/tests/functional/qemu_test/uncompress.py index 76dcf22385..ce79da1b68 100644 --- a/tests/functional/qemu_test/uncompress.py +++ b/tests/functional/qemu_test/uncompress.py @@ -13,7 +13,7 @@ import os import stat import shutil from urllib.parse import urlparse -from subprocess import check_call, CalledProcessError +from subprocess import run, CalledProcessError, DEVNULL from .asset import Asset @@ -46,8 +46,8 @@ def zstd_uncompress(zstd_path, output_path): return try: - check_call(['zstd', "-f", "-d", zstd_path, - "-o", output_path]) + run(['zstd', "-f", "-d", zstd_path, + "-o", output_path], capture_output=True, check=True) except CalledProcessError as e: os.remove(output_path) raise Exception( From 2c92ecb678cddf4bf3ced98f94acd2f3691c21bc Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Thu, 27 Feb 2025 11:39:10 +0100 Subject: [PATCH 07/15] tests/functional: Move the code for testing HTTP downloads to a common function We are going to use this code in other tests, too, so let's move it to the qemu_test module to be able to re-use it more easily. Message-ID: <20250227103915.19795-2-thuth@redhat.com> Signed-off-by: Thomas Huth --- tests/functional/qemu_test/linuxkernel.py | 26 ++++++++++++++++++++++- tests/functional/test_intel_iommu.py | 22 +------------------ 2 files changed, 26 insertions(+), 22 deletions(-) diff --git a/tests/functional/qemu_test/linuxkernel.py b/tests/functional/qemu_test/linuxkernel.py index 2c9598102d..2aca0ee3cd 100644 --- a/tests/functional/qemu_test/linuxkernel.py +++ b/tests/functional/qemu_test/linuxkernel.py @@ -3,8 +3,12 @@ # This work is licensed under the terms of the GNU GPL, version 2 or # later. See the COPYING file in the top-level directory. +import hashlib +import urllib.request + +from .cmd import wait_for_console_pattern, exec_command_and_wait_for_pattern from .testcase import QemuSystemTest -from .cmd import wait_for_console_pattern +from .utils import get_usernet_hostfwd_port class LinuxKernelTest(QemuSystemTest): @@ -26,3 +30,23 @@ class LinuxKernelTest(QemuSystemTest): self.vm.launch() if wait_for: self.wait_for_console_pattern(wait_for) + + def check_http_download(self, filename, hashsum, guestport=8080, + pythoncmd='python3 -m http.server'): + exec_command_and_wait_for_pattern(self, + f'{pythoncmd} {guestport} & sleep 1', + f'Serving HTTP on 0.0.0.0 port {guestport}') + hl = hashlib.sha256() + hostport = get_usernet_hostfwd_port(self.vm) + url = f'http://localhost:{hostport}{filename}' + self.log.info(f'Downloading {url} ...') + with urllib.request.urlopen(url) as response: + while True: + chunk = response.read(1 << 20) + if not chunk: + break + hl.update(chunk) + + digest = hl.hexdigest() + self.log.info(f'sha256sum of download is {digest}.') + self.assertEqual(digest, hashsum) diff --git a/tests/functional/test_intel_iommu.py b/tests/functional/test_intel_iommu.py index a9e8f82ab5..62268d6f27 100755 --- a/tests/functional/test_intel_iommu.py +++ b/tests/functional/test_intel_iommu.py @@ -10,11 +10,7 @@ # This work is licensed under the terms of the GNU GPL, version 2 or # later. See the COPYING file in the top-level directory. -import hashlib -import urllib.request - from qemu_test import LinuxKernelTest, Asset, exec_command_and_wait_for_pattern -from qemu_test.utils import get_usernet_hostfwd_port class IntelIOMMU(LinuxKernelTest): @@ -125,23 +121,7 @@ class IntelIOMMU(LinuxKernelTest): # Check virtio-net via HTTP: exec_command_and_wait_for_pattern(self, 'dhclient eth0', prompt) - exec_command_and_wait_for_pattern(self, - f'python3 -m http.server {self.GUEST_PORT} & sleep 1', - f'Serving HTTP on 0.0.0.0 port {self.GUEST_PORT}') - hl = hashlib.sha256() - hostport = get_usernet_hostfwd_port(self.vm) - url = f'http://localhost:{hostport}{filename}' - self.log.info(f'Downloading {url} ...') - with urllib.request.urlopen(url) as response: - while True: - chunk = response.read(1 << 20) - if not chunk: - break - hl.update(chunk) - - digest = hl.hexdigest() - self.log.info(f'sha256sum of download is {digest}.') - self.assertEqual(digest, hashsum) + self.check_http_download(filename, hashsum, self.GUEST_PORT) def test_intel_iommu(self): self.common_vm_setup() From 7b7f98efa7628a58fae594d3aa51b3c8a10293b3 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Thu, 27 Feb 2025 11:39:11 +0100 Subject: [PATCH 08/15] tests/functional/test_mips_malta: Add a network test via the pcnet NIC The kernel has a driver for the pcnet NIC included, and the initrd has a "tftp" command, so we can test a simple network transfer here, too. Message-ID: <20250227103915.19795-3-thuth@redhat.com> Signed-off-by: Thomas Huth --- tests/functional/test_mips_malta.py | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/functional/test_mips_malta.py b/tests/functional/test_mips_malta.py index eaf372255b..9697c7d63f 100755 --- a/tests/functional/test_mips_malta.py +++ b/tests/functional/test_mips_malta.py @@ -45,12 +45,15 @@ class MaltaMachineConsole(LinuxKernelTest): 'dcfe3a7fe3200da3a00d176b95caaa086495eb158f2bff64afc67d7e1eb2cddc') def test_mips_malta_cpio(self): + self.require_netdev('user') + self.set_machine('malta') + self.require_device('pcnet') + kernel_path = self.archive_extract( self.ASSET_KERNEL_4_5_0, member='boot/vmlinux-4.5.0-2-4kc-malta') initrd_path = self.uncompress(self.ASSET_INITRD) - self.set_machine('malta') self.vm.set_console() kernel_command_line = (self.KERNEL_COMMON_COMMAND_LINE + 'console=ttyS0 console=tty ' @@ -58,6 +61,8 @@ class MaltaMachineConsole(LinuxKernelTest): self.vm.add_args('-kernel', kernel_path, '-initrd', initrd_path, '-append', kernel_command_line, + '-netdev', 'user,id=n1,tftp=' + self.scratch_file('boot'), + '-device', 'pcnet,netdev=n1', '-no-reboot') self.vm.launch() self.wait_for_console_pattern('Boot successful.') @@ -66,6 +71,19 @@ class MaltaMachineConsole(LinuxKernelTest): 'BogoMIPS') exec_command_and_wait_for_pattern(self, 'uname -a', 'Debian') + + exec_command_and_wait_for_pattern(self, 'ip link set eth0 up', + 'eth0: link up') + exec_command_and_wait_for_pattern(self, + 'ip addr add 10.0.2.15 dev eth0', + '#') + exec_command_and_wait_for_pattern(self, 'route add default eth0', '#') + exec_command_and_wait_for_pattern(self, + 'tftp -g -r vmlinux-4.5.0-2-4kc-malta 10.0.2.2', '#') + exec_command_and_wait_for_pattern(self, + 'md5sum vmlinux-4.5.0-2-4kc-malta', + 'a98218a7efbdefb2dfdf9ecd08c98318') + exec_command_and_wait_for_pattern(self, 'reboot', 'reboot: Restarting system') # Wait for VM to shut down gracefully From a31001b1c807cc59b2a9aa99845783cb40a27d0c Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Wed, 5 Mar 2025 08:43:53 +0100 Subject: [PATCH 09/15] tests/functional: Increase the timeout of the mips64el_replay test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We run the gitlab-CI with the untrusted tests enabled, and the test_replay_mips64el_malta_5KEc_cpio subtest is rather slow, so this already hit the standard 90 seconds timeout in the CI. Increase the timeout for more headroom. Reported-by: Stefan Hajnoczi Message-ID: <20250305074353.52552-1-thuth@redhat.com> Reviewed-by: Daniel P. Berrangé Reviewed-by: Stefan Hajnoczi Signed-off-by: Thomas Huth --- tests/functional/meson.build | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/functional/meson.build b/tests/functional/meson.build index 3fd2652c07..97c3f4ad4e 100644 --- a/tests/functional/meson.build +++ b/tests/functional/meson.build @@ -36,6 +36,7 @@ test_timeouts = { 'intel_iommu': 300, 'mips_malta' : 120, 'mipsel_replay' : 480, + 'mips64el_replay' : 180, 'netdev_ethtool' : 180, 'ppc_40p' : 240, 'ppc64_hv' : 1000, From 842721581fdbed45fa4738d02df8d28b1eaf28dd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Tue, 4 Mar 2025 18:33:40 +0000 Subject: [PATCH 10/15] tests/functional: fix race in virtio balloon test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are two race conditions in the recently added virtio balloon test * The /dev/vda device node is not ready * The virtio-balloon driver has not issued the first stats refresh To fix the former, monitor dmesg for a line about 'vda'. To fix the latter, retry the stats query until seeing fresh data. Adding 'quiet' to the kernel command line reduces serial output which otherwise slows boot, making it less likely to hit the former race too. Signed-off-by: Daniel P. Berrangé Message-ID: <20250304183340.3749797-1-berrange@redhat.com> Reviewed-by: Thomas Huth Reviewed-by: David Hildenbrand [thuth: Break long line to avoid checkpatch error] Signed-off-by: Thomas Huth --- tests/functional/test_virtio_balloon.py | 26 ++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/tests/functional/test_virtio_balloon.py b/tests/functional/test_virtio_balloon.py index 67b48e1b4e..082bf08c4e 100755 --- a/tests/functional/test_virtio_balloon.py +++ b/tests/functional/test_virtio_balloon.py @@ -32,7 +32,7 @@ class VirtioBalloonx86(QemuSystemTest): 'e3c1b309d9203604922d6e255c2c5d098a309c2d46215d8fc026954f3c5c27a0') DEFAULT_KERNEL_PARAMS = ('root=/dev/vda1 console=ttyS0 net.ifnames=0 ' - 'rd.rescue') + 'rd.rescue quiet') def wait_for_console_pattern(self, success_message, vm=None): wait_for_console_pattern( @@ -47,6 +47,11 @@ class VirtioBalloonx86(QemuSystemTest): prompt = '# ' self.wait_for_console_pattern(prompt) + # Synchronize on virtio-block driver creating the root device + exec_command_and_wait_for_pattern(self, + "while ! (dmesg -c | grep vda:) ; do sleep 1 ; done", + "vda1") + exec_command_and_wait_for_pattern(self, 'mount /dev/vda1 /sysroot', prompt) exec_command_and_wait_for_pattern(self, 'chroot /sysroot', @@ -65,10 +70,21 @@ class VirtioBalloonx86(QemuSystemTest): assert val == UNSET_STATS_VALUE def assert_running_stats(self, then): - ret = self.vm.qmp('qom-get', - {'path': '/machine/peripheral/balloon', - 'property': 'guest-stats'})['return'] - when = ret.get('last-update') + # We told the QEMU to refresh stats every 100ms, but + # there can be a delay between virtio-ballon driver + # being modprobed and seeing the first stats refresh + # Retry a few times for robustness under heavy load + retries = 10 + when = 0 + while when == 0 and retries: + ret = self.vm.qmp('qom-get', + {'path': '/machine/peripheral/balloon', + 'property': 'guest-stats'})['return'] + when = ret.get('last-update') + if when == 0: + retries = retries - 1 + time.sleep(0.5) + now = time.time() assert when > then and when < now From 4dc11ee468df3ffdaa312a16b4ded3378133bb39 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Fri, 7 Mar 2025 07:25:37 +0100 Subject: [PATCH 11/15] tests/functional/test_virtio_balloon: Only use KVM for running this test The virtio_balloon test is currently hanging for unknown reasons when being run on the shared gitlab CI runners (which don't provide KVM, thus it's running in TCG mode there). All other functional tests that use the same asset (the Fedora 31 kernel) have already been marked to work only with KVM in the past, so those other tests are skipped on the shared gitlab CI runners. As long as the problem isn't fully understood and fixed, let's do the same with the virtio_balloon test to avoid that the CI is failing here. Message-ID: <20250307063904.1081961-1-thuth@redhat.com> Reviewed-by: David Hildenbrand Signed-off-by: Thomas Huth --- tests/functional/test_virtio_balloon.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/functional/test_virtio_balloon.py b/tests/functional/test_virtio_balloon.py index 082bf08c4e..5877b6c408 100755 --- a/tests/functional/test_virtio_balloon.py +++ b/tests/functional/test_virtio_balloon.py @@ -110,6 +110,7 @@ class VirtioBalloonx86(QemuSystemTest): def test_virtio_balloon_stats(self): self.set_machine('q35') + self.require_accelerator("kvm") kernel_path = self.ASSET_KERNEL.fetch() initrd_path = self.ASSET_INITRD.fetch() diskimage_path = self.ASSET_DISKIMAGE.fetch() @@ -122,7 +123,7 @@ class VirtioBalloonx86(QemuSystemTest): # reset, we can reliably catch the clean stats again in BIOS # phase before the guest OS launches self.vm.add_args("-boot", "menu=on") - self.vm.add_args("-machine", "q35,accel=kvm:tcg") + self.vm.add_args("-accel", "kvm") self.vm.add_args("-device", "virtio-balloon,id=balloon") self.vm.add_args('-drive', f'file={diskimage_path},if=none,id=drv0,snapshot=on') From e7f091d0c1750a57fd7ab39db50d1aae1c7647b0 Mon Sep 17 00:00:00 2001 From: Aditya Gupta Date: Thu, 6 Mar 2025 11:37:06 +0530 Subject: [PATCH 12/15] doc: add missing 'Asset' type in function test doc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Seems 'Asset' got missed in the documentation by mistake. Also fix the one spellcheck issue pointed by spellcheck Signed-off-by: Aditya Gupta Reviewed-by: Philippe Mathieu-Daudé Message-ID: <20250306060706.1982992-1-adityag@linux.ibm.com> Signed-off-by: Thomas Huth --- docs/devel/testing/functional.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/devel/testing/functional.rst b/docs/devel/testing/functional.rst index bcb5509512..a9fa45eac1 100644 --- a/docs/devel/testing/functional.rst +++ b/docs/devel/testing/functional.rst @@ -251,7 +251,7 @@ Many functional tests download assets (e.g. Linux kernels, initrds, firmware images, etc.) from the internet to be able to run tests with them. This imposes additional challenges to the test framework. -First there is the the problem that some people might not have an +First there is the problem that some people might not have an unconstrained internet connection, so such tests should not be run by default when running ``make check``. To accomplish this situation, the tests that download files should only be added to the "thorough" @@ -274,7 +274,9 @@ the tests are run. This pre-caching is done with the qemu_test.Asset class. To use it in your test, declare an asset in your test class with its URL and SHA256 checksum like this:: - ASSET_somename = ( + from qemu_test import Asset + + ASSET_somename = Asset( ('https://www.qemu.org/assets/images/qemu_head_200.png'), '34b74cad46ea28a2966c1d04e102510daf1fd73e6582b6b74523940d5da029dd') From 9cbff6f29ee099e7cb331802a1bf9b179c4c3934 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Thu, 6 Mar 2025 11:51:23 +0100 Subject: [PATCH 13/15] MAINTAINERS: Add docs/devel/testing/functional.rst to the functional section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add an entry for docs/devel/testing/functional.rst to get notified on patches that change this file. Message-ID: <20250306105124.702131-1-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 692628cd78..51f424ee84 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -4228,6 +4228,7 @@ Functional testing framework M: Thomas Huth R: Philippe Mathieu-Daudé R: Daniel P. Berrange +F: docs/devel/testing/functional.rst F: tests/functional/qemu_test/ Windows Hosted Continuous Integration From dfcee1ea4c52ac60e0a06221eafb7b6253eb10c3 Mon Sep 17 00:00:00 2001 From: Matthew Rosato Date: Wed, 26 Feb 2025 16:00:12 -0500 Subject: [PATCH 14/15] s390x/pci: add support for guests that request direct mapping When receiving a guest mpcifc(4) or mpcifc(6) instruction without the T bit set, treat this as a request to perform direct mapping instead of address translation. In order to facilitate this, pin the entirety of guest memory into the host iommu. Pinning for the direct mapping case is handled via vfio and its memory listener. Additionally, ram discard settings are inherited from vfio: coordinated discards (e.g. virtio-mem) are allowed while uncoordinated discards (e.g. virtio-balloon) are disabled. Subsequent guest DMA operations are all expected to be of the format guest_phys+sdma, allowing them to be used as lookup into the host iommu table. Signed-off-by: Matthew Rosato Reviewed-by: David Hildenbrand Message-ID: <20250226210013.238349-2-mjrosato@linux.ibm.com> Signed-off-by: Thomas Huth --- hw/s390x/s390-pci-bus.c | 39 +++++++++++++++++++++++++++++++-- hw/s390x/s390-pci-inst.c | 13 +++++++++-- hw/s390x/s390-pci-vfio.c | 25 ++++++++++++++++----- hw/s390x/s390-virtio-ccw.c | 5 +++++ include/hw/s390x/s390-pci-bus.h | 3 +++ 5 files changed, 76 insertions(+), 9 deletions(-) diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c index 913d72cc74..9d7b0f7540 100644 --- a/hw/s390x/s390-pci-bus.c +++ b/hw/s390x/s390-pci-bus.c @@ -18,6 +18,8 @@ #include "hw/s390x/s390-pci-inst.h" #include "hw/s390x/s390-pci-kvm.h" #include "hw/s390x/s390-pci-vfio.h" +#include "hw/s390x/s390-virtio-ccw.h" +#include "hw/boards.h" #include "hw/pci/pci_bus.h" #include "hw/qdev-properties.h" #include "hw/pci/pci_bridge.h" @@ -724,12 +726,42 @@ void s390_pci_iommu_enable(S390PCIIOMMU *iommu) g_free(name); } +void s390_pci_iommu_direct_map_enable(S390PCIIOMMU *iommu) +{ + MachineState *ms = MACHINE(qdev_get_machine()); + S390CcwMachineState *s390ms = S390_CCW_MACHINE(ms); + + /* + * For direct-mapping we must map the entire guest address space. Rather + * than using an iommu, create a memory region alias that maps GPA X to + * IOVA X + SDMA. VFIO will handle pinning via its memory listener. + */ + g_autofree char *name = g_strdup_printf("iommu-dm-s390-%04x", + iommu->pbdev->uid); + + iommu->dm_mr = g_malloc0(sizeof(*iommu->dm_mr)); + memory_region_init_alias(iommu->dm_mr, OBJECT(&iommu->mr), name, + get_system_memory(), 0, + s390_get_memory_limit(s390ms)); + iommu->enabled = true; + memory_region_add_subregion(&iommu->mr, iommu->pbdev->zpci_fn.sdma, + iommu->dm_mr); +} + void s390_pci_iommu_disable(S390PCIIOMMU *iommu) { iommu->enabled = false; g_hash_table_remove_all(iommu->iotlb); - memory_region_del_subregion(&iommu->mr, MEMORY_REGION(&iommu->iommu_mr)); - object_unparent(OBJECT(&iommu->iommu_mr)); + if (iommu->dm_mr) { + memory_region_del_subregion(&iommu->mr, iommu->dm_mr); + object_unparent(OBJECT(iommu->dm_mr)); + g_free(iommu->dm_mr); + iommu->dm_mr = NULL; + } else { + memory_region_del_subregion(&iommu->mr, + MEMORY_REGION(&iommu->iommu_mr)); + object_unparent(OBJECT(&iommu->iommu_mr)); + } } static void s390_pci_iommu_free(S390pciState *s, PCIBus *bus, int32_t devfn) @@ -1145,6 +1177,7 @@ static void s390_pcihost_plug(HotplugHandler *hotplug_dev, DeviceState *dev, /* Always intercept emulated devices */ pbdev->interp = false; pbdev->forwarding_assist = false; + pbdev->rtr_avail = false; } if (s390_pci_msix_init(pbdev) && !pbdev->interp) { @@ -1510,6 +1543,8 @@ static const Property s390_pci_device_properties[] = { DEFINE_PROP_BOOL("interpret", S390PCIBusDevice, interp, true), DEFINE_PROP_BOOL("forwarding-assist", S390PCIBusDevice, forwarding_assist, true), + DEFINE_PROP_BOOL("relaxed-translation", S390PCIBusDevice, rtr_avail, + true), }; static const VMStateDescription s390_pci_device_vmstate = { diff --git a/hw/s390x/s390-pci-inst.c b/hw/s390x/s390-pci-inst.c index e386d75d58..8cdeb6cb7f 100644 --- a/hw/s390x/s390-pci-inst.c +++ b/hw/s390x/s390-pci-inst.c @@ -16,6 +16,7 @@ #include "exec/memory.h" #include "qemu/error-report.h" #include "system/hw_accel.h" +#include "hw/boards.h" #include "hw/pci/pci_device.h" #include "hw/s390x/s390-pci-inst.h" #include "hw/s390x/s390-pci-bus.h" @@ -1008,17 +1009,25 @@ static int reg_ioat(CPUS390XState *env, S390PCIBusDevice *pbdev, ZpciFib fib, } /* currently we only support designation type 1 with translation */ - if (!(dt == ZPCI_IOTA_RTTO && t)) { + if (t && dt != ZPCI_IOTA_RTTO) { error_report("unsupported ioat dt %d t %d", dt, t); s390_program_interrupt(env, PGM_OPERAND, ra); return -EINVAL; + } else if (!t && !pbdev->rtr_avail) { + error_report("relaxed translation not allowed"); + s390_program_interrupt(env, PGM_OPERAND, ra); + return -EINVAL; } iommu->pba = pba; iommu->pal = pal; iommu->g_iota = g_iota; - s390_pci_iommu_enable(iommu); + if (t) { + s390_pci_iommu_enable(iommu); + } else { + s390_pci_iommu_direct_map_enable(iommu); + } return 0; } diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c index 7dbbc76823..443e222912 100644 --- a/hw/s390x/s390-pci-vfio.c +++ b/hw/s390x/s390-pci-vfio.c @@ -132,12 +132,27 @@ static void s390_pci_read_base(S390PCIBusDevice *pbdev, pbdev->pft = cap->pft; /* - * If appropriate, reduce the size of the supported DMA aperture reported - * to the guest based upon the vfio DMA limit. + * If the device is a passthrough ISM device, disallow relaxed + * translation. */ - vfio_size = pbdev->iommu->max_dma_limit << TARGET_PAGE_BITS; - if (vfio_size > 0 && vfio_size < cap->end_dma - cap->start_dma + 1) { - pbdev->zpci_fn.edma = cap->start_dma + vfio_size - 1; + if (pbdev->pft == ZPCI_PFT_ISM) { + pbdev->rtr_avail = false; + } + + /* + * If appropriate, reduce the size of the supported DMA aperture reported + * to the guest based upon the vfio DMA limit. This is applicable for + * devices that are guaranteed to not use relaxed translation. If the + * device is capable of relaxed translation then we must advertise the + * full aperture. In this case, if translation is used then we will + * rely on the vfio DMA limit counting and use RPCIT CC1 / status 16 + * to request that the guest free DMA mappings as necessary. + */ + if (!pbdev->rtr_avail) { + vfio_size = pbdev->iommu->max_dma_limit << TARGET_PAGE_BITS; + if (vfio_size > 0 && vfio_size < cap->end_dma - cap->start_dma + 1) { + pbdev->zpci_fn.edma = cap->start_dma + vfio_size - 1; + } } } diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 51ae0c133d..a9b3db19f6 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -936,8 +936,13 @@ static void ccw_machine_9_2_instance_options(MachineState *machine) static void ccw_machine_9_2_class_options(MachineClass *mc) { + static GlobalProperty compat[] = { + { TYPE_S390_PCI_DEVICE, "relaxed-translation", "off", }, + }; + ccw_machine_10_0_class_options(mc); compat_props_add(mc->compat_props, hw_compat_9_2, hw_compat_9_2_len); + compat_props_add(mc->compat_props, compat, G_N_ELEMENTS(compat)); } DEFINE_CCW_MACHINE(9, 2); diff --git a/include/hw/s390x/s390-pci-bus.h b/include/hw/s390x/s390-pci-bus.h index 2c43ea123f..04944d4fed 100644 --- a/include/hw/s390x/s390-pci-bus.h +++ b/include/hw/s390x/s390-pci-bus.h @@ -277,6 +277,7 @@ struct S390PCIIOMMU { AddressSpace as; MemoryRegion mr; IOMMUMemoryRegion iommu_mr; + MemoryRegion *dm_mr; bool enabled; uint64_t g_iota; uint64_t pba; @@ -362,6 +363,7 @@ struct S390PCIBusDevice { bool interp; bool forwarding_assist; bool aif; + bool rtr_avail; QTAILQ_ENTRY(S390PCIBusDevice) link; }; @@ -389,6 +391,7 @@ int pci_chsc_sei_nt2_have_event(void); void s390_pci_sclp_configure(SCCB *sccb); void s390_pci_sclp_deconfigure(SCCB *sccb); void s390_pci_iommu_enable(S390PCIIOMMU *iommu); +void s390_pci_iommu_direct_map_enable(S390PCIIOMMU *iommu); void s390_pci_iommu_disable(S390PCIIOMMU *iommu); void s390_pci_generate_error_event(uint16_t pec, uint32_t fh, uint32_t fid, uint64_t faddr, uint32_t e); From d9b5dfc7122559e5b5959ecf534788b90c3dd102 Mon Sep 17 00:00:00 2001 From: Matthew Rosato Date: Wed, 26 Feb 2025 16:00:13 -0500 Subject: [PATCH 15/15] s390x/pci: indicate QEMU supports relaxed translation for passthrough Specifying this bit in the guest CLP response indicates that the guest can optionally choose to skip translation and instead use identity-mapped operations. Tested-by: Niklas Schnelle Reviewed-by: Niklas Schnelle Signed-off-by: Matthew Rosato Message-ID: <20250226210013.238349-3-mjrosato@linux.ibm.com> Signed-off-by: Thomas Huth --- hw/s390x/s390-pci-vfio.c | 5 ++++- include/hw/s390x/s390-pci-clp.h | 1 + 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/s390x/s390-pci-vfio.c b/hw/s390x/s390-pci-vfio.c index 443e222912..6236ac7f1e 100644 --- a/hw/s390x/s390-pci-vfio.c +++ b/hw/s390x/s390-pci-vfio.c @@ -238,8 +238,11 @@ static void s390_pci_read_group(S390PCIBusDevice *pbdev, pbdev->pci_group = s390_group_create(pbdev->zpci_fn.pfgid, start_gid); resgrp = &pbdev->pci_group->zpci_group; + if (pbdev->rtr_avail) { + resgrp->fr |= CLP_RSP_QPCIG_MASK_RTR; + } if (cap->flags & VFIO_DEVICE_INFO_ZPCI_FLAG_REFRESH) { - resgrp->fr = 1; + resgrp->fr |= CLP_RSP_QPCIG_MASK_REFRESH; } resgrp->dasm = cap->dasm; resgrp->msia = cap->msi_addr; diff --git a/include/hw/s390x/s390-pci-clp.h b/include/hw/s390x/s390-pci-clp.h index 03b7f9ba5f..6a635d693b 100644 --- a/include/hw/s390x/s390-pci-clp.h +++ b/include/hw/s390x/s390-pci-clp.h @@ -158,6 +158,7 @@ typedef struct ClpRspQueryPciGrp { #define CLP_RSP_QPCIG_MASK_NOI 0xfff uint16_t i; uint8_t version; +#define CLP_RSP_QPCIG_MASK_RTR 0x20 #define CLP_RSP_QPCIG_MASK_FRAME 0x2 #define CLP_RSP_QPCIG_MASK_REFRESH 0x1 uint8_t fr;