From 57822f71ddd64e79933539376fa1139433edcc4f Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Tue, 2 May 2023 12:57:21 +0200 Subject: [PATCH 01/21] tests/avocado/virtio-gpu: Fix the URLs of the test_virtio_vga_virgl test MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The URLs here are not valid anymore - looks like the assets got moved into the pub/archive/ subfolder instead. Message-Id: <20230502105721.1661930-1-thuth@redhat.com> Reviewed-by: Marc-André Lureau Signed-off-by: Thomas Huth --- tests/avocado/virtio-gpu.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/avocado/virtio-gpu.py b/tests/avocado/virtio-gpu.py index 2a249a3a2c..e3b58fe799 100644 --- a/tests/avocado/virtio-gpu.py +++ b/tests/avocado/virtio-gpu.py @@ -36,13 +36,13 @@ class VirtioGPUx86(QemuSystemTest): KERNEL_COMMAND_LINE = "printk.time=0 console=ttyS0 rdinit=/bin/bash" KERNEL_URL = ( - "https://archives.fedoraproject.org/pub/fedora" + "https://archives.fedoraproject.org/pub/archive/fedora" "/linux/releases/33/Everything/x86_64/os/images" "/pxeboot/vmlinuz" ) KERNEL_HASH = '1433cfe3f2ffaa44de4ecfb57ec25dc2399cdecf' INITRD_URL = ( - "https://archives.fedoraproject.org/pub/fedora" + "https://archives.fedoraproject.org/pub/archive/fedora" "/linux/releases/33/Everything/x86_64/os/images" "/pxeboot/initrd.img" ) From 1e05888ab5e0f78adb29c4344586fb7a6df572c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Philippe=20Mathieu-Daud=C3=A9?= Date: Wed, 5 Apr 2023 18:04:45 +0200 Subject: [PATCH 02/21] sysemu/kvm: Remove unused headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All types used are forward-declared in "qemu/typedefs.h". Signed-off-by: Philippe Mathieu-Daudé Message-Id: <20230405160454.97436-2-philmd@linaro.org> [thuth: Add hw/core/cpu.h to migration/dirtyrate.c to fix compile failure] Signed-off-by: Thomas Huth --- include/sysemu/kvm.h | 3 --- migration/dirtyrate.c | 1 + 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h index c8281c07a7..88f5ccfbce 100644 --- a/include/sysemu/kvm.h +++ b/include/sysemu/kvm.h @@ -14,9 +14,6 @@ #ifndef QEMU_KVM_H #define QEMU_KVM_H -#include "qemu/queue.h" -#include "hw/core/cpu.h" -#include "exec/memattrs.h" #include "qemu/accel.h" #include "qom/object.h" diff --git a/migration/dirtyrate.c b/migration/dirtyrate.c index c06f12c39d..0220db82ec 100644 --- a/migration/dirtyrate.c +++ b/migration/dirtyrate.c @@ -13,6 +13,7 @@ #include "qemu/osdep.h" #include "qemu/error-report.h" #include +#include "hw/core/cpu.h" #include "qapi/error.h" #include "exec/ramblock.h" #include "exec/target_page.h" From eb96660507ecbe479c6a38639861f65b23d067d7 Mon Sep 17 00:00:00 2001 From: Laurent Vivier Date: Wed, 3 May 2023 11:41:09 +0200 Subject: [PATCH 03/21] net: stream: test reconnect option with an unix socket We can have failure with the inet type test because the port address is not allocated atomically and can be taken by another test between its selection and the start of QEMU. To avoid that, use an unix socket with a path that is unique Signed-off-by: Laurent Vivier Message-Id: <20230503094109.1198248-1-lvivier@redhat.com> Reviewed-by: Thomas Huth Signed-off-by: Thomas Huth --- tests/qtest/netdev-socket.c | 39 +++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 21 deletions(-) diff --git a/tests/qtest/netdev-socket.c b/tests/qtest/netdev-socket.c index 9cf1b0698e..097abc0230 100644 --- a/tests/qtest/netdev-socket.c +++ b/tests/qtest/netdev-socket.c @@ -189,28 +189,26 @@ static void wait_stream_disconnected(QTestState *qts, const char *id) qobject_unref(resp); } -static void test_stream_inet_reconnect(void) +static void test_stream_unix_reconnect(void) { QTestState *qts0, *qts1; - int port; SocketAddress *addr; + gchar *path; - port = inet_get_free_port(false); + path = g_strconcat(tmpdir, "/stream_unix_reconnect", NULL); qts0 = qtest_initf("-nodefaults -M none " - "-netdev stream,id=st0,server=true,addr.type=inet," - "addr.ipv4=on,addr.ipv6=off," - "addr.host=127.0.0.1,addr.port=%d", port); + "-netdev stream,id=st0,server=true,addr.type=unix," + "addr.path=%s", path); EXPECT_STATE(qts0, "st0: index=0,type=stream,\r\n", 0); qts1 = qtest_initf("-nodefaults -M none " - "-netdev stream,server=false,id=st0,addr.type=inet," - "addr.ipv4=on,addr.ipv6=off,reconnect=1," - "addr.host=127.0.0.1,addr.port=%d", port); + "-netdev stream,server=false,id=st0,addr.type=unix," + "addr.path=%s,reconnect=1", path); wait_stream_connected(qts0, "st0", &addr); - g_assert_cmpint(addr->type, ==, SOCKET_ADDRESS_TYPE_INET); - g_assert_cmpstr(addr->u.inet.host, ==, "127.0.0.1"); + g_assert_cmpint(addr->type, ==, SOCKET_ADDRESS_TYPE_UNIX); + g_assert_cmpstr(addr->u.q_unix.path, ==, path); qapi_free_SocketAddress(addr); /* kill server */ @@ -221,24 +219,23 @@ static void test_stream_inet_reconnect(void) /* restart server */ qts0 = qtest_initf("-nodefaults -M none " - "-netdev stream,id=st0,server=true,addr.type=inet," - "addr.ipv4=on,addr.ipv6=off," - "addr.host=127.0.0.1,addr.port=%d", port); + "-netdev stream,id=st0,server=true,addr.type=unix," + "addr.path=%s", path); /* wait connection events*/ wait_stream_connected(qts0, "st0", &addr); - g_assert_cmpint(addr->type, ==, SOCKET_ADDRESS_TYPE_INET); - g_assert_cmpstr(addr->u.inet.host, ==, "127.0.0.1"); + g_assert_cmpint(addr->type, ==, SOCKET_ADDRESS_TYPE_UNIX); + g_assert_cmpstr(addr->u.q_unix.path, ==, path); qapi_free_SocketAddress(addr); wait_stream_connected(qts1, "st0", &addr); - g_assert_cmpint(addr->type, ==, SOCKET_ADDRESS_TYPE_INET); - g_assert_cmpstr(addr->u.inet.host, ==, "127.0.0.1"); - g_assert_cmpint(atoi(addr->u.inet.port), ==, port); + g_assert_cmpint(addr->type, ==, SOCKET_ADDRESS_TYPE_UNIX); + g_assert_cmpstr(addr->u.q_unix.path, ==, path); qapi_free_SocketAddress(addr); qtest_quit(qts1); qtest_quit(qts0); + g_free(path); } static void test_stream_inet_ipv6(void) @@ -517,8 +514,6 @@ int main(int argc, char **argv) #ifndef _WIN32 qtest_add_func("/netdev/dgram/mcast", test_dgram_mcast); #endif - qtest_add_func("/netdev/stream/inet/reconnect", - test_stream_inet_reconnect); } if (has_ipv6) { qtest_add_func("/netdev/stream/inet/ipv6", test_stream_inet_ipv6); @@ -530,6 +525,8 @@ int main(int argc, char **argv) qtest_add_func("/netdev/dgram/unix", test_dgram_unix); #endif qtest_add_func("/netdev/stream/unix", test_stream_unix); + qtest_add_func("/netdev/stream/unix/reconnect", + test_stream_unix_reconnect); #ifdef CONFIG_LINUX qtest_add_func("/netdev/stream/unix/abstract", test_stream_unix_abstract); From 855436dbf756014a024f3e415001ead37301ef95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Fri, 21 Apr 2023 18:14:06 +0100 Subject: [PATCH 04/21] tests/qtest: replace qmp_discard_response with qtest_qmp_assert_success MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The qmp_discard_response method simply ignores the result of the QMP command, merely unref'ing the object. This is a bad idea for tests as it leaves no trace if the QMP command unexpectedly failed. The qtest_qmp_assert_success method will validate that the QMP command returned without error, and if errors occur, it will print a message on the console aiding debugging. Signed-off-by: Daniel P. Berrangé Message-Id: <20230421171411.566300-2-berrange@redhat.com> Reviewed-by: Juan Quintela Reviewed-by: Zhang Chen Signed-off-by: Thomas Huth --- tests/qtest/ahci-test.c | 31 ++++++++++++++-------------- tests/qtest/boot-order-test.c | 5 +---- tests/qtest/fdc-test.c | 15 +++++++------- tests/qtest/ide-test.c | 5 +---- tests/qtest/migration-test.c | 5 +---- tests/qtest/test-filter-mirror.c | 5 +---- tests/qtest/test-filter-redirector.c | 7 ++----- tests/qtest/virtio-blk-test.c | 24 ++++++++++----------- 8 files changed, 40 insertions(+), 57 deletions(-) diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c index 1967cd5898..abab761c26 100644 --- a/tests/qtest/ahci-test.c +++ b/tests/qtest/ahci-test.c @@ -36,9 +36,6 @@ #include "hw/pci/pci_ids.h" #include "hw/pci/pci_regs.h" -/* TODO actually test the results and get rid of this */ -#define qmp_discard_response(s, ...) qobject_unref(qtest_qmp(s, __VA_ARGS__)) - /* Test images sizes in MB */ #define TEST_IMAGE_SIZE_MB_LARGE (200 * 1024) #define TEST_IMAGE_SIZE_MB_SMALL 64 @@ -1595,9 +1592,9 @@ static void test_atapi_tray(void) rsp = qtest_qmp_receive(ahci->parent->qts); qobject_unref(rsp); - qmp_discard_response(ahci->parent->qts, - "{'execute': 'blockdev-remove-medium', " - "'arguments': {'id': 'cd0'}}"); + qtest_qmp_assert_success(ahci->parent->qts, + "{'execute': 'blockdev-remove-medium', " + "'arguments': {'id': 'cd0'}}"); /* Test the tray without a medium */ ahci_atapi_load(ahci, port); @@ -1607,16 +1604,18 @@ static void test_atapi_tray(void) atapi_wait_tray(ahci, true); /* Re-insert media */ - qmp_discard_response(ahci->parent->qts, - "{'execute': 'blockdev-add', " - "'arguments': {'node-name': 'node0', " - "'driver': 'raw', " - "'file': { 'driver': 'file', " - "'filename': %s }}}", iso); - qmp_discard_response(ahci->parent->qts, - "{'execute': 'blockdev-insert-medium'," - "'arguments': { 'id': 'cd0', " - "'node-name': 'node0' }}"); + qtest_qmp_assert_success( + ahci->parent->qts, + "{'execute': 'blockdev-add', " + "'arguments': {'node-name': 'node0', " + "'driver': 'raw', " + "'file': { 'driver': 'file', " + "'filename': %s }}}", iso); + qtest_qmp_assert_success( + ahci->parent->qts, + "{'execute': 'blockdev-insert-medium'," + "'arguments': { 'id': 'cd0', " + "'node-name': 'node0' }}"); /* Again, the event shows up first */ qtest_qmp_send(ahci->parent->qts, "{'execute': 'blockdev-close-tray', " diff --git a/tests/qtest/boot-order-test.c b/tests/qtest/boot-order-test.c index 0680d79d6d..8f2b6ef05a 100644 --- a/tests/qtest/boot-order-test.c +++ b/tests/qtest/boot-order-test.c @@ -16,9 +16,6 @@ #include "qapi/qmp/qdict.h" #include "standard-headers/linux/qemu_fw_cfg.h" -/* TODO actually test the results and get rid of this */ -#define qmp_discard_response(qs, ...) qobject_unref(qtest_qmp(qs, __VA_ARGS__)) - typedef struct { const char *args; uint64_t expected_boot; @@ -43,7 +40,7 @@ static void test_a_boot_order(const char *machine, machine ?: "", test_args); actual = read_boot_order(qts); g_assert_cmphex(actual, ==, expected_boot); - qmp_discard_response(qts, "{ 'execute': 'system_reset' }"); + qtest_qmp_assert_success(qts, "{ 'execute': 'system_reset' }"); /* * system_reset only requests reset. We get a RESET event after * the actual reset completes. Need to wait for that. diff --git a/tests/qtest/fdc-test.c b/tests/qtest/fdc-test.c index 1f9b99ad6d..5e8fbda9df 100644 --- a/tests/qtest/fdc-test.c +++ b/tests/qtest/fdc-test.c @@ -28,9 +28,6 @@ #include "libqtest-single.h" #include "qapi/qmp/qdict.h" -/* TODO actually test the results and get rid of this */ -#define qmp_discard_response(...) qobject_unref(qmp(__VA_ARGS__)) - #define DRIVE_FLOPPY_BLANK \ "-drive if=floppy,file=null-co://,file.read-zeroes=on,format=raw,size=1440k" @@ -304,9 +301,10 @@ static void test_media_insert(void) /* Insert media in drive. DSKCHK should not be reset until a step pulse * is sent. */ - qmp_discard_response("{'execute':'blockdev-change-medium', 'arguments':{" - " 'id':'floppy0', 'filename': %s, 'format': 'raw' }}", - test_image); + qtest_qmp_assert_success(global_qtest, + "{'execute':'blockdev-change-medium', 'arguments':{" + " 'id':'floppy0', 'filename': %s, 'format': 'raw' }}", + test_image); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); @@ -335,8 +333,9 @@ static void test_media_change(void) /* Eject the floppy and check that DSKCHG is set. Reading it out doesn't * reset the bit. */ - qmp_discard_response("{'execute':'eject', 'arguments':{" - " 'id':'floppy0' }}"); + qtest_qmp_assert_success(global_qtest, + "{'execute':'eject', 'arguments':{" + " 'id':'floppy0' }}"); dir = inb(FLOPPY_BASE + reg_dir); assert_bit_set(dir, DSKCHG); diff --git a/tests/qtest/ide-test.c b/tests/qtest/ide-test.c index dcb050bf9b..d6b4f6e36a 100644 --- a/tests/qtest/ide-test.c +++ b/tests/qtest/ide-test.c @@ -34,9 +34,6 @@ #include "hw/pci/pci_ids.h" #include "hw/pci/pci_regs.h" -/* TODO actually test the results and get rid of this */ -#define qmp_discard_response(q, ...) qobject_unref(qtest_qmp(q, __VA_ARGS__)) - #define TEST_IMAGE_SIZE 64 * 1024 * 1024 #define IDE_PCI_DEV 1 @@ -766,7 +763,7 @@ static void test_pci_retry_flush(void) qtest_qmp_eventwait(qts, "STOP"); /* Complete the command */ - qmp_discard_response(qts, "{'execute':'cont' }"); + qtest_qmp_assert_success(qts, "{'execute':'cont' }"); /* Check registers */ data = qpci_io_readb(dev, ide_bar, reg_device); diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c index 8a5df84624..b99b49a314 100644 --- a/tests/qtest/migration-test.c +++ b/tests/qtest/migration-test.c @@ -40,9 +40,6 @@ #include "linux/kvm.h" #endif -/* TODO actually test the results and get rid of this */ -#define qtest_qmp_discard_response(...) qobject_unref(qtest_qmp(__VA_ARGS__)) - unsigned start_address; unsigned end_address; static bool uffd_feature_thread_id; @@ -766,7 +763,7 @@ static void test_migrate_end(QTestState *from, QTestState *to, bool test_dest) usleep(1000 * 10); } while (dest_byte_a == dest_byte_b); - qtest_qmp_discard_response(to, "{ 'execute' : 'stop'}"); + qtest_qmp_assert_success(to, "{ 'execute' : 'stop'}"); /* With it stopped, check nothing changes */ qtest_memread(to, start_address, &dest_byte_c, 1); diff --git a/tests/qtest/test-filter-mirror.c b/tests/qtest/test-filter-mirror.c index 248fc88699..adeada3eb8 100644 --- a/tests/qtest/test-filter-mirror.c +++ b/tests/qtest/test-filter-mirror.c @@ -16,9 +16,6 @@ #include "qemu/error-report.h" #include "qemu/main-loop.h" -/* TODO actually test the results and get rid of this */ -#define qmp_discard_response(qs, ...) qobject_unref(qtest_qmp(qs, __VA_ARGS__)) - static void test_mirror(void) { int send_sock[2], recv_sock[2]; @@ -52,7 +49,7 @@ static void test_mirror(void) }; /* send a qmp command to guarantee that 'connected' is setting to true. */ - qmp_discard_response(qts, "{ 'execute' : 'query-status'}"); + qtest_qmp_assert_success(qts, "{ 'execute' : 'query-status'}"); ret = iov_send(send_sock[0], iov, 2, 0, sizeof(size) + sizeof(send_buf)); g_assert_cmpint(ret, ==, sizeof(send_buf) + sizeof(size)); close(send_sock[0]); diff --git a/tests/qtest/test-filter-redirector.c b/tests/qtest/test-filter-redirector.c index 24ca9280f8..e72e3b7873 100644 --- a/tests/qtest/test-filter-redirector.c +++ b/tests/qtest/test-filter-redirector.c @@ -58,9 +58,6 @@ #include "qemu/error-report.h" #include "qemu/main-loop.h" -/* TODO actually test the results and get rid of this */ -#define qmp_discard_response(qs, ...) qobject_unref(qtest_qmp(qs, __VA_ARGS__)) - static void test_redirector_tx(void) { int backend_sock[2], recv_sock; @@ -98,7 +95,7 @@ static void test_redirector_tx(void) g_assert_cmpint(recv_sock, !=, -1); /* send a qmp command to guarantee that 'connected' is setting to true. */ - qmp_discard_response(qts, "{ 'execute' : 'query-status'}"); + qtest_qmp_assert_success(qts, "{ 'execute' : 'query-status'}"); struct iovec iov[] = { { @@ -176,7 +173,7 @@ static void test_redirector_rx(void) send_sock = unix_connect(sock_path1, NULL); g_assert_cmpint(send_sock, !=, -1); /* send a qmp command to guarantee that 'connected' is setting to true. */ - qmp_discard_response(qts, "{ 'execute' : 'query-status'}"); + qtest_qmp_assert_success(qts, "{ 'execute' : 'query-status'}"); ret = iov_send(send_sock, iov, 2, 0, sizeof(size) + sizeof(send_buf)); g_assert_cmpint(ret, ==, sizeof(send_buf) + sizeof(size)); diff --git a/tests/qtest/virtio-blk-test.c b/tests/qtest/virtio-blk-test.c index 19c01f808b..98c906ebb4 100644 --- a/tests/qtest/virtio-blk-test.c +++ b/tests/qtest/virtio-blk-test.c @@ -17,9 +17,6 @@ #include "libqos/qgraph.h" #include "libqos/virtio-blk.h" -/* TODO actually test the results and get rid of this */ -#define qmp_discard_response(...) qobject_unref(qmp(__VA_ARGS__)) - #define TEST_IMAGE_SIZE (64 * 1024 * 1024) #define QVIRTIO_BLK_TIMEOUT_US (30 * 1000 * 1000) #define PCI_SLOT_HP 0x06 @@ -453,9 +450,10 @@ static void config(void *obj, void *data, QGuestAllocator *t_alloc) qvirtio_set_driver_ok(dev); - qmp_discard_response("{ 'execute': 'block_resize', " - " 'arguments': { 'device': 'drive0', " - " 'size': %d } }", n_size); + qtest_qmp_assert_success(global_qtest, + "{ 'execute': 'block_resize', " + " 'arguments': { 'device': 'drive0', " + " 'size': %d } }", n_size); qvirtio_wait_config_isr(dev, QVIRTIO_BLK_TIMEOUT_US); capacity = qvirtio_config_readq(dev, 0); @@ -502,9 +500,10 @@ static void msix(void *obj, void *u_data, QGuestAllocator *t_alloc) qvirtio_set_driver_ok(dev); - qmp_discard_response("{ 'execute': 'block_resize', " - " 'arguments': { 'device': 'drive0', " - " 'size': %d } }", n_size); + qtest_qmp_assert_success(global_qtest, + "{ 'execute': 'block_resize', " + " 'arguments': { 'device': 'drive0', " + " 'size': %d } }", n_size); qvirtio_wait_config_isr(dev, QVIRTIO_BLK_TIMEOUT_US); @@ -758,9 +757,10 @@ static void resize(void *obj, void *data, QGuestAllocator *t_alloc) vq = test_basic(dev, t_alloc); - qmp_discard_response("{ 'execute': 'block_resize', " - " 'arguments': { 'device': 'drive0', " - " 'size': %d } }", n_size); + qtest_qmp_assert_success(global_qtest, + "{ 'execute': 'block_resize', " + " 'arguments': { 'device': 'drive0', " + " 'size': %d } }", n_size); qvirtio_wait_queue_isr(qts, dev, vq, QVIRTIO_BLK_TIMEOUT_US); From b2999ed8ad6d9793b8310a1c7138a0232f637d1d Mon Sep 17 00:00:00 2001 From: Jonathan Cameron Date: Fri, 21 Apr 2023 13:25:50 +0100 Subject: [PATCH 05/21] hw/pci-bridge: Fix release ordering by embedding PCIBridgeWindows within PCIBridge The lifetime of the PCIBridgeWindows instance accessed via the windows pointer in struct PCIBridge is managed separately from the PCIBridge itself. Triggered by ./qemu-system-x86_64 -M x-remote -display none -monitor stdio QEMU monitor: device_add cxl-downstream In some error handling paths (such as the above due to attaching a cxl-downstream port anything other than a cxl-upstream port) the g_free() of the PCIBridge windows in pci_bridge_region_cleanup() is called before the final call of flatview_uref() in address_space_set_flatview() ultimately from drain_call_rcu() At one stage this resulted in a crash, currently can still be observed using valgrind which records a use after free. When present, only one instance is allocated. pci_bridge_update_mappings() can operate directly on an instance rather than creating a new one and swapping it in. Thus there appears to be no reason to not directly couple the lifetimes of the two structures by embedding the PCIBridgeWindows within the PCIBridge removing the need for the problematic separate free. Patch is same as was posted deep in the discussion. https://lore.kernel.org/qemu-devel/20230403171232.000020bb@huawei.com/ Reported-by: Thomas Huth Signed-off-by: Jonathan Cameron Message-Id: <20230421122550.28234-1-Jonathan.Cameron@huawei.com> Reviewed-by: Thomas Huth Signed-off-by: Thomas Huth --- hw/pci/pci_bridge.c | 19 ++++++++----------- include/hw/pci/pci_bridge.h | 2 +- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/hw/pci/pci_bridge.c b/hw/pci/pci_bridge.c index dd5af508f9..e7b9345615 100644 --- a/hw/pci/pci_bridge.c +++ b/hw/pci/pci_bridge.c @@ -184,11 +184,11 @@ static void pci_bridge_init_vga_aliases(PCIBridge *br, PCIBus *parent, } } -static PCIBridgeWindows *pci_bridge_region_init(PCIBridge *br) +static void pci_bridge_region_init(PCIBridge *br) { PCIDevice *pd = PCI_DEVICE(br); PCIBus *parent = pci_get_bus(pd); - PCIBridgeWindows *w = g_new(PCIBridgeWindows, 1); + PCIBridgeWindows *w = &br->windows; uint16_t cmd = pci_get_word(pd->config + PCI_COMMAND); pci_bridge_init_alias(br, &w->alias_pref_mem, @@ -211,8 +211,6 @@ static PCIBridgeWindows *pci_bridge_region_init(PCIBridge *br) cmd & PCI_COMMAND_IO); pci_bridge_init_vga_aliases(br, parent, w->alias_vga); - - return w; } static void pci_bridge_region_del(PCIBridge *br, PCIBridgeWindows *w) @@ -234,19 +232,18 @@ static void pci_bridge_region_cleanup(PCIBridge *br, PCIBridgeWindows *w) object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_IO_LO])); object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_IO_HI])); object_unparent(OBJECT(&w->alias_vga[QEMU_PCI_VGA_MEM])); - g_free(w); } void pci_bridge_update_mappings(PCIBridge *br) { - PCIBridgeWindows *w = br->windows; + PCIBridgeWindows *w = &br->windows; /* Make updates atomic to: handle the case of one VCPU updating the bridge * while another accesses an unaffected region. */ memory_region_transaction_begin(); - pci_bridge_region_del(br, br->windows); + pci_bridge_region_del(br, w); pci_bridge_region_cleanup(br, w); - br->windows = pci_bridge_region_init(br); + pci_bridge_region_init(br); memory_region_transaction_commit(); } @@ -385,7 +382,7 @@ void pci_bridge_initfn(PCIDevice *dev, const char *typename) sec_bus->address_space_io = &br->address_space_io; memory_region_init(&br->address_space_io, OBJECT(br), "pci_bridge_io", 4 * GiB); - br->windows = pci_bridge_region_init(br); + pci_bridge_region_init(br); QLIST_INIT(&sec_bus->child); QLIST_INSERT_HEAD(&parent->child, sec_bus, sibling); } @@ -396,8 +393,8 @@ void pci_bridge_exitfn(PCIDevice *pci_dev) PCIBridge *s = PCI_BRIDGE(pci_dev); assert(QLIST_EMPTY(&s->sec_bus.child)); QLIST_REMOVE(&s->sec_bus, sibling); - pci_bridge_region_del(s, s->windows); - pci_bridge_region_cleanup(s, s->windows); + pci_bridge_region_del(s, &s->windows); + pci_bridge_region_cleanup(s, &s->windows); /* object_unparent() is called automatically during device deletion */ } diff --git a/include/hw/pci/pci_bridge.h b/include/hw/pci/pci_bridge.h index 01670e9e65..ea54a81a15 100644 --- a/include/hw/pci/pci_bridge.h +++ b/include/hw/pci/pci_bridge.h @@ -73,7 +73,7 @@ struct PCIBridge { MemoryRegion address_space_mem; MemoryRegion address_space_io; - PCIBridgeWindows *windows; + PCIBridgeWindows windows; pci_map_irq_fn map_irq; const char *bus_name; From 1f8da027ddc73c6ddbffc84f7183c1ce98f2a3b1 Mon Sep 17 00:00:00 2001 From: Mateusz Krawczuk Date: Thu, 4 May 2023 23:11:01 +0200 Subject: [PATCH 06/21] Add information how to fix common build error on Windows in symlink-install-tree By default, Windows doesn't allow to create soft links for user account and only administrator is allowed to do this. To fix this problem you have to raise your permissions or enable Developer Mode, which available since Windows 10. Additional explanation when build fails will allow developer to fix the problem on his computer faster. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/1386 Signed-off-by: Mateusz Krawczuk Message-Id: <20230504211101.1386-1-mat.krawczuk@gmail.com> [thuth: Drop the hunk with the white space changes] Signed-off-by: Thomas Huth --- scripts/symlink-install-tree.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/scripts/symlink-install-tree.py b/scripts/symlink-install-tree.py index 67cb86dd52..8ed97e3c94 100644 --- a/scripts/symlink-install-tree.py +++ b/scripts/symlink-install-tree.py @@ -28,5 +28,8 @@ for source, dest in json.loads(out).items(): os.symlink(source, bundle_dest) except BaseException as e: if not isinstance(e, OSError) or e.errno != errno.EEXIST: + if os.name == 'nt': + print('Please enable Developer Mode to support soft link ' + 'without Administrator permission') print(f'error making symbolic link {dest}', file=sys.stderr) raise e From a19b119bd79f2c4b521c5969c9ffdabd0b6b546b Mon Sep 17 00:00:00 2001 From: Ani Sinha Date: Thu, 4 May 2023 21:16:10 +0530 Subject: [PATCH 07/21] tests: libvirt-ci: Update to commit 'c8971e90ac' to pull in mformat and xorriso Pull in the following changes from lcitool: * tests/lcitool/libvirt-ci 85487e1...c8971e9 (18): > mappings: add new package mappings for mformat and xorriso > docs: testing: Update contents with tox > .gitlab-ci.yml: Always test against installed lcitool > gitlab-ci.yml: Start using tox for testing > tox: Allow running with custom pytest options with {posargs} > gitignore: Add the default .tox directory > dev-requirements: Reference VM requirements > requirements: Add tox to dev-requirements.txt and drop pytest and flake > test-requirements: Rename to dev-requirements.txt > Add tox.ini configuration file > tests: commands: Consolidate the installed package/run from git tests > Add a pytest.ini > facts: targets: Drop Fedora 36 target > gitlab-ci.yml: Add Fedora 38 target > facts: targets: Add Fedora 38 > facts: mappings: Drop 'zstd' mapping > facts: projects: nbdkit: Replace zstd mapping with libzstd > docs: mappings: Add a section on the preferred mapping naming scheme Signed-off-by: Ani Sinha Message-Id: <20230504154611.85854-2-anisinha@redhat.com> Reviewed-by: Thomas Huth Reviewed-by: Michael S. Tsirkin Signed-off-by: Thomas Huth --- tests/lcitool/libvirt-ci | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/lcitool/libvirt-ci b/tests/lcitool/libvirt-ci index 85487e1404..c8971e90ac 160000 --- a/tests/lcitool/libvirt-ci +++ b/tests/lcitool/libvirt-ci @@ -1 +1 @@ -Subproject commit 85487e140415b2ac54b01a9a6b600fd7c21edc2f +Subproject commit c8971e90ac169ee2b539c747f74d96c876debdf9 From da9000784c90d2b645eedaa865b5cf778cb4d37f Mon Sep 17 00:00:00 2001 From: Ani Sinha Date: Thu, 4 May 2023 21:16:11 +0530 Subject: [PATCH 08/21] tests/lcitool: Add mtools and xorriso and remove genisoimage as dependencies Bios bits avocado tests need mformat (provided by the mtools package) and xorriso tools in order to run within gitlab CI containers. Add those dependencies within the Dockerfiles so that containers can be built with those tools present and bios bits avocado tests can be run there. xorriso package conflicts with genisoimage package on some distributions. Therefore, it is not possible to have both the packages at the same time in the container image uniformly for all distribution flavors. Further, on some distributions like RHEL, both xorriso and genisoimage packages provide /usr/bin/genisoimage and on some other distributions like Fedora, only genisoimage package provides the same utility. Therefore, this change removes the dependency on geninsoimage for building container images altogether keeping only xorriso package. At the same time, cdrom-test.c is updated to use and check for existence of only xorrisofs. Signed-off-by: Ani Sinha Message-Id: <20230504154611.85854-3-anisinha@redhat.com> Reviewed-by: Thomas Huth Reviewed-by: Michael S. Tsirkin Signed-off-by: Thomas Huth --- .gitlab-ci.d/cirrus/freebsd-13.vars | 2 +- .gitlab-ci.d/cirrus/macos-12.vars | 2 +- tests/docker/dockerfiles/alpine.docker | 3 ++- tests/docker/dockerfiles/centos8.docker | 3 ++- tests/docker/dockerfiles/debian-amd64-cross.docker | 3 ++- tests/docker/dockerfiles/debian-amd64.docker | 3 ++- tests/docker/dockerfiles/debian-arm64-cross.docker | 3 ++- tests/docker/dockerfiles/debian-armel-cross.docker | 3 ++- tests/docker/dockerfiles/debian-armhf-cross.docker | 3 ++- .../dockerfiles/debian-mips64el-cross.docker | 3 ++- .../docker/dockerfiles/debian-mipsel-cross.docker | 3 ++- .../docker/dockerfiles/debian-ppc64el-cross.docker | 3 ++- tests/docker/dockerfiles/debian-s390x-cross.docker | 3 ++- tests/docker/dockerfiles/fedora-win32-cross.docker | 3 ++- tests/docker/dockerfiles/fedora-win64-cross.docker | 3 ++- tests/docker/dockerfiles/fedora.docker | 3 ++- tests/docker/dockerfiles/opensuse-leap.docker | 3 ++- tests/docker/dockerfiles/ubuntu2004.docker | 3 ++- tests/docker/dockerfiles/ubuntu2204.docker | 3 ++- tests/lcitool/projects/qemu.yml | 3 ++- tests/qtest/cdrom-test.c | 14 +++++++------- 21 files changed, 45 insertions(+), 27 deletions(-) diff --git a/.gitlab-ci.d/cirrus/freebsd-13.vars b/.gitlab-ci.d/cirrus/freebsd-13.vars index 7622c849b2..facb649f5b 100644 --- a/.gitlab-ci.d/cirrus/freebsd-13.vars +++ b/.gitlab-ci.d/cirrus/freebsd-13.vars @@ -11,6 +11,6 @@ MAKE='/usr/local/bin/gmake' NINJA='/usr/local/bin/ninja' PACKAGING_COMMAND='pkg' PIP3='/usr/local/bin/pip-3.8' -PKGS='alsa-lib bash bison bzip2 ca_root_nss capstone4 ccache cdrkit-genisoimage cmocka ctags curl cyrus-sasl dbus diffutils dtc flex fusefs-libs3 gettext git glib gmake gnutls gsed gtk3 json-c libepoxy libffi libgcrypt libjpeg-turbo libnfs libslirp libspice-server libssh libtasn1 llvm lzo2 meson ncurses nettle ninja opencv pixman pkgconf png py39-numpy py39-pillow py39-pip py39-sphinx py39-sphinx_rtd_theme py39-yaml python3 rpm2cpio sdl2 sdl2_image snappy sndio socat spice-protocol tesseract usbredir virglrenderer vte3 zstd' +PKGS='alsa-lib bash bison bzip2 ca_root_nss capstone4 ccache cmocka ctags curl cyrus-sasl dbus diffutils dtc flex fusefs-libs3 gettext git glib gmake gnutls gsed gtk3 json-c libepoxy libffi libgcrypt libjpeg-turbo libnfs libslirp libspice-server libssh libtasn1 llvm lzo2 meson mtools ncurses nettle ninja opencv pixman pkgconf png py39-numpy py39-pillow py39-pip py39-sphinx py39-sphinx_rtd_theme py39-yaml python3 rpm2cpio sdl2 sdl2_image snappy sndio socat spice-protocol tesseract usbredir virglrenderer vte3 xorriso zstd' PYPI_PKGS='' PYTHON='/usr/local/bin/python3' diff --git a/.gitlab-ci.d/cirrus/macos-12.vars b/.gitlab-ci.d/cirrus/macos-12.vars index da6aa6469b..ceb294e153 100644 --- a/.gitlab-ci.d/cirrus/macos-12.vars +++ b/.gitlab-ci.d/cirrus/macos-12.vars @@ -11,6 +11,6 @@ MAKE='/opt/homebrew/bin/gmake' NINJA='/opt/homebrew/bin/ninja' PACKAGING_COMMAND='brew' PIP3='/opt/homebrew/bin/pip3' -PKGS='bash bc bison bzip2 capstone ccache cmocka ctags curl dbus diffutils dtc flex gcovr gettext git glib gnu-sed gnutls gtk+3 jemalloc jpeg-turbo json-c libepoxy libffi libgcrypt libiscsi libnfs libpng libslirp libssh libtasn1 libusb llvm lzo make meson ncurses nettle ninja pixman pkg-config python3 rpm2cpio sdl2 sdl2_image snappy socat sparse spice-protocol tesseract usbredir vde vte3 zlib zstd' +PKGS='bash bc bison bzip2 capstone ccache cmocka ctags curl dbus diffutils dtc flex gcovr gettext git glib gnu-sed gnutls gtk+3 jemalloc jpeg-turbo json-c libepoxy libffi libgcrypt libiscsi libnfs libpng libslirp libssh libtasn1 libusb llvm lzo make meson mtools ncurses nettle ninja pixman pkg-config python3 rpm2cpio sdl2 sdl2_image snappy socat sparse spice-protocol tesseract usbredir vde vte3 xorriso zlib zstd' PYPI_PKGS='PyYAML numpy pillow sphinx sphinx-rtd-theme' PYTHON='/opt/homebrew/bin/python3' diff --git a/tests/docker/dockerfiles/alpine.docker b/tests/docker/dockerfiles/alpine.docker index 81c70aeaf9..0097637dca 100644 --- a/tests/docker/dockerfiles/alpine.docker +++ b/tests/docker/dockerfiles/alpine.docker @@ -19,7 +19,6 @@ RUN apk update && \ ca-certificates \ capstone-dev \ ccache \ - cdrkit \ ceph-dev \ clang \ cmocka-dev \ @@ -67,6 +66,7 @@ RUN apk update && \ make \ mesa-dev \ meson \ + mtools \ multipath-tools \ musl-dev \ ncurses-dev \ @@ -108,6 +108,7 @@ RUN apk update && \ which \ xen-dev \ xfsprogs-dev \ + xorriso \ zlib-dev \ zlib-static \ zstd \ diff --git a/tests/docker/dockerfiles/centos8.docker b/tests/docker/dockerfiles/centos8.docker index 1a6a9087c1..78f454b782 100644 --- a/tests/docker/dockerfiles/centos8.docker +++ b/tests/docker/dockerfiles/centos8.docker @@ -36,7 +36,6 @@ RUN dnf distro-sync -y && \ fuse3-devel \ gcc \ gcc-c++ \ - genisoimage \ gettext \ git \ glib2-devel \ @@ -82,6 +81,7 @@ RUN dnf distro-sync -y && \ lzo-devel \ make \ mesa-libgbm-devel \ + mtools \ ncurses-devel \ nettle-devel \ ninja-build \ @@ -114,6 +114,7 @@ RUN dnf distro-sync -y && \ vte291-devel \ which \ xfsprogs-devel \ + xorriso \ zlib-devel \ zlib-static \ zstd && \ diff --git a/tests/docker/dockerfiles/debian-amd64-cross.docker b/tests/docker/dockerfiles/debian-amd64-cross.docker index 2e7eb445f1..40a2b6acc4 100644 --- a/tests/docker/dockerfiles/debian-amd64-cross.docker +++ b/tests/docker/dockerfiles/debian-amd64-cross.docker @@ -25,7 +25,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ findutils \ flex \ gcovr \ - genisoimage \ gettext \ git \ hostname \ @@ -37,6 +36,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ locales \ make \ meson \ + mtools \ ncat \ ninja-build \ openssh-client \ @@ -57,6 +57,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ tar \ tesseract-ocr \ tesseract-ocr-eng \ + xorriso \ zstd && \ eatmydata apt-get autoremove -y && \ eatmydata apt-get autoclean -y && \ diff --git a/tests/docker/dockerfiles/debian-amd64.docker b/tests/docker/dockerfiles/debian-amd64.docker index 28e2fa81b1..e39871c7bb 100644 --- a/tests/docker/dockerfiles/debian-amd64.docker +++ b/tests/docker/dockerfiles/debian-amd64.docker @@ -28,7 +28,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ g++ \ gcc \ gcovr \ - genisoimage \ gettext \ git \ hostname \ @@ -103,6 +102,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ locales \ make \ meson \ + mtools \ multipath-tools \ ncat \ nettle-dev \ @@ -127,6 +127,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ tesseract-ocr \ tesseract-ocr-eng \ xfslibs-dev \ + xorriso \ zlib1g-dev \ zstd && \ eatmydata apt-get autoremove -y && \ diff --git a/tests/docker/dockerfiles/debian-arm64-cross.docker b/tests/docker/dockerfiles/debian-arm64-cross.docker index f558770f84..c99300bbfa 100644 --- a/tests/docker/dockerfiles/debian-arm64-cross.docker +++ b/tests/docker/dockerfiles/debian-arm64-cross.docker @@ -25,7 +25,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ findutils \ flex \ gcovr \ - genisoimage \ gettext \ git \ hostname \ @@ -37,6 +36,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ locales \ make \ meson \ + mtools \ ncat \ ninja-build \ openssh-client \ @@ -57,6 +57,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ tar \ tesseract-ocr \ tesseract-ocr-eng \ + xorriso \ zstd && \ eatmydata apt-get autoremove -y && \ eatmydata apt-get autoclean -y && \ diff --git a/tests/docker/dockerfiles/debian-armel-cross.docker b/tests/docker/dockerfiles/debian-armel-cross.docker index f3d7e07cce..5db5c78b31 100644 --- a/tests/docker/dockerfiles/debian-armel-cross.docker +++ b/tests/docker/dockerfiles/debian-armel-cross.docker @@ -25,7 +25,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ findutils \ flex \ gcovr \ - genisoimage \ gettext \ git \ hostname \ @@ -37,6 +36,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ locales \ make \ meson \ + mtools \ ncat \ ninja-build \ openssh-client \ @@ -57,6 +57,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ tar \ tesseract-ocr \ tesseract-ocr-eng \ + xorriso \ zstd && \ eatmydata apt-get autoremove -y && \ eatmydata apt-get autoclean -y && \ diff --git a/tests/docker/dockerfiles/debian-armhf-cross.docker b/tests/docker/dockerfiles/debian-armhf-cross.docker index 531c556ad5..ae6600b25f 100644 --- a/tests/docker/dockerfiles/debian-armhf-cross.docker +++ b/tests/docker/dockerfiles/debian-armhf-cross.docker @@ -25,7 +25,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ findutils \ flex \ gcovr \ - genisoimage \ gettext \ git \ hostname \ @@ -37,6 +36,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ locales \ make \ meson \ + mtools \ ncat \ ninja-build \ openssh-client \ @@ -57,6 +57,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ tar \ tesseract-ocr \ tesseract-ocr-eng \ + xorriso \ zstd && \ eatmydata apt-get autoremove -y && \ eatmydata apt-get autoclean -y && \ diff --git a/tests/docker/dockerfiles/debian-mips64el-cross.docker b/tests/docker/dockerfiles/debian-mips64el-cross.docker index 816dbd2911..daa2d48e36 100644 --- a/tests/docker/dockerfiles/debian-mips64el-cross.docker +++ b/tests/docker/dockerfiles/debian-mips64el-cross.docker @@ -25,7 +25,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ findutils \ flex \ gcovr \ - genisoimage \ gettext \ git \ hostname \ @@ -37,6 +36,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ locales \ make \ meson \ + mtools \ ncat \ ninja-build \ openssh-client \ @@ -57,6 +57,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ tar \ tesseract-ocr \ tesseract-ocr-eng \ + xorriso \ zstd && \ eatmydata apt-get autoremove -y && \ eatmydata apt-get autoclean -y && \ diff --git a/tests/docker/dockerfiles/debian-mipsel-cross.docker b/tests/docker/dockerfiles/debian-mipsel-cross.docker index b115b29af3..5af04e2054 100644 --- a/tests/docker/dockerfiles/debian-mipsel-cross.docker +++ b/tests/docker/dockerfiles/debian-mipsel-cross.docker @@ -25,7 +25,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ findutils \ flex \ gcovr \ - genisoimage \ gettext \ git \ hostname \ @@ -37,6 +36,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ locales \ make \ meson \ + mtools \ ncat \ ninja-build \ openssh-client \ @@ -57,6 +57,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ tar \ tesseract-ocr \ tesseract-ocr-eng \ + xorriso \ zstd && \ eatmydata apt-get autoremove -y && \ eatmydata apt-get autoclean -y && \ diff --git a/tests/docker/dockerfiles/debian-ppc64el-cross.docker b/tests/docker/dockerfiles/debian-ppc64el-cross.docker index 301bddb536..1eeba7fcab 100644 --- a/tests/docker/dockerfiles/debian-ppc64el-cross.docker +++ b/tests/docker/dockerfiles/debian-ppc64el-cross.docker @@ -25,7 +25,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ findutils \ flex \ gcovr \ - genisoimage \ gettext \ git \ hostname \ @@ -37,6 +36,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ locales \ make \ meson \ + mtools \ ncat \ ninja-build \ openssh-client \ @@ -57,6 +57,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ tar \ tesseract-ocr \ tesseract-ocr-eng \ + xorriso \ zstd && \ eatmydata apt-get autoremove -y && \ eatmydata apt-get autoclean -y && \ diff --git a/tests/docker/dockerfiles/debian-s390x-cross.docker b/tests/docker/dockerfiles/debian-s390x-cross.docker index 5d27c91c17..52e89a6dab 100644 --- a/tests/docker/dockerfiles/debian-s390x-cross.docker +++ b/tests/docker/dockerfiles/debian-s390x-cross.docker @@ -25,7 +25,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ findutils \ flex \ gcovr \ - genisoimage \ gettext \ git \ hostname \ @@ -37,6 +36,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ locales \ make \ meson \ + mtools \ ncat \ ninja-build \ openssh-client \ @@ -57,6 +57,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ tar \ tesseract-ocr \ tesseract-ocr-eng \ + xorriso \ zstd && \ eatmydata apt-get autoremove -y && \ eatmydata apt-get autoclean -y && \ diff --git a/tests/docker/dockerfiles/fedora-win32-cross.docker b/tests/docker/dockerfiles/fedora-win32-cross.docker index e7966ec7fd..dc72ae9cc9 100644 --- a/tests/docker/dockerfiles/fedora-win32-cross.docker +++ b/tests/docker/dockerfiles/fedora-win32-cross.docker @@ -30,7 +30,6 @@ exec "$@"\n' > /usr/bin/nosync && \ findutils \ flex \ gcovr \ - genisoimage \ git \ glib2-devel \ glibc-langpack-en \ @@ -38,6 +37,7 @@ exec "$@"\n' > /usr/bin/nosync && \ llvm \ make \ meson \ + mtools \ ninja-build \ nmap-ncat \ openssh-clients \ @@ -59,6 +59,7 @@ exec "$@"\n' > /usr/bin/nosync && \ tesseract-langpack-eng \ util-linux \ which \ + xorriso \ zstd && \ nosync dnf autoremove -y && \ nosync dnf clean all -y diff --git a/tests/docker/dockerfiles/fedora-win64-cross.docker b/tests/docker/dockerfiles/fedora-win64-cross.docker index 86c3a8f2ac..7eb4a5dba2 100644 --- a/tests/docker/dockerfiles/fedora-win64-cross.docker +++ b/tests/docker/dockerfiles/fedora-win64-cross.docker @@ -30,7 +30,6 @@ exec "$@"\n' > /usr/bin/nosync && \ findutils \ flex \ gcovr \ - genisoimage \ git \ glib2-devel \ glibc-langpack-en \ @@ -38,6 +37,7 @@ exec "$@"\n' > /usr/bin/nosync && \ llvm \ make \ meson \ + mtools \ ninja-build \ nmap-ncat \ openssh-clients \ @@ -59,6 +59,7 @@ exec "$@"\n' > /usr/bin/nosync && \ tesseract-langpack-eng \ util-linux \ which \ + xorriso \ zstd && \ nosync dnf autoremove -y && \ nosync dnf clean all -y diff --git a/tests/docker/dockerfiles/fedora.docker b/tests/docker/dockerfiles/fedora.docker index b698b7595d..3a69eefdda 100644 --- a/tests/docker/dockerfiles/fedora.docker +++ b/tests/docker/dockerfiles/fedora.docker @@ -43,7 +43,6 @@ exec "$@"\n' > /usr/bin/nosync && \ gcc \ gcc-c++ \ gcovr \ - genisoimage \ gettext \ git \ glib2-devel \ @@ -90,6 +89,7 @@ exec "$@"\n' > /usr/bin/nosync && \ make \ mesa-libgbm-devel \ meson \ + mtools \ ncurses-devel \ nettle-devel \ ninja-build \ @@ -128,6 +128,7 @@ exec "$@"\n' > /usr/bin/nosync && \ which \ xen-devel \ xfsprogs-devel \ + xorriso \ zlib-devel \ zlib-static \ zstd && \ diff --git a/tests/docker/dockerfiles/opensuse-leap.docker b/tests/docker/dockerfiles/opensuse-leap.docker index afb9f5419f..185abe57d8 100644 --- a/tests/docker/dockerfiles/opensuse-leap.docker +++ b/tests/docker/dockerfiles/opensuse-leap.docker @@ -81,7 +81,7 @@ RUN zypper update -y && \ lttng-ust-devel \ lzo-devel \ make \ - mkisofs \ + mtools \ ncat \ ncurses-devel \ ninja \ @@ -111,6 +111,7 @@ RUN zypper update -y && \ which \ xen-devel \ xfsprogs-devel \ + xorriso \ zlib-devel \ zlib-devel-static \ zstd && \ diff --git a/tests/docker/dockerfiles/ubuntu2004.docker b/tests/docker/dockerfiles/ubuntu2004.docker index aa2f5ca7b4..8f864d19e6 100644 --- a/tests/docker/dockerfiles/ubuntu2004.docker +++ b/tests/docker/dockerfiles/ubuntu2004.docker @@ -28,7 +28,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ g++ \ gcc \ gcovr \ - genisoimage \ gettext \ git \ hostname \ @@ -100,6 +99,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ llvm \ locales \ make \ + mtools \ multipath-tools \ ncat \ nettle-dev \ @@ -126,6 +126,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ tesseract-ocr \ tesseract-ocr-eng \ xfslibs-dev \ + xorriso \ zlib1g-dev \ zstd && \ eatmydata apt-get autoremove -y && \ diff --git a/tests/docker/dockerfiles/ubuntu2204.docker b/tests/docker/dockerfiles/ubuntu2204.docker index 3f7d30e5d0..1d442cdfe6 100644 --- a/tests/docker/dockerfiles/ubuntu2204.docker +++ b/tests/docker/dockerfiles/ubuntu2204.docker @@ -28,7 +28,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ g++ \ gcc \ gcovr \ - genisoimage \ gettext \ git \ hostname \ @@ -103,6 +102,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ locales \ make \ meson \ + mtools \ multipath-tools \ ncat \ nettle-dev \ @@ -127,6 +127,7 @@ RUN export DEBIAN_FRONTEND=noninteractive && \ tesseract-ocr \ tesseract-ocr-eng \ xfslibs-dev \ + xorriso \ zlib1g-dev \ zstd && \ eatmydata apt-get autoremove -y && \ diff --git a/tests/lcitool/projects/qemu.yml b/tests/lcitool/projects/qemu.yml index af3700379a..566db8313b 100644 --- a/tests/lcitool/projects/qemu.yml +++ b/tests/lcitool/projects/qemu.yml @@ -26,7 +26,6 @@ packages: - gcc - gcovr - gettext - - genisoimage - glib2 - glib2-native - glib2-static @@ -73,6 +72,7 @@ packages: - llvm - lttng-ust - lzo + - mtools - netcat - nettle - ninja @@ -116,6 +116,7 @@ packages: - which - xen - xfsprogs + - xorriso - zstdtools - zlib - zlib-static diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c index 31d3bacd8c..2b7e10d920 100644 --- a/tests/qtest/cdrom-test.c +++ b/tests/qtest/cdrom-test.c @@ -17,7 +17,7 @@ static char isoimage[] = "cdrom-boot-iso-XXXXXX"; -static int exec_genisoimg(const char **args) +static int exec_xorrisofs(const char **args) { gchar *out_err = NULL; gint exit_status = -1; @@ -43,7 +43,7 @@ static int prepare_image(const char *arch, char *isoimage) char *codefile = NULL; int ifh, ret = -1; const char *args[] = { - "genisoimage", "-quiet", "-l", "-no-emul-boot", + "xorrisofs", "-quiet", "-l", "-no-emul-boot", "-b", NULL, "-o", isoimage, srcdir, NULL }; @@ -75,9 +75,9 @@ static int prepare_image(const char *arch, char *isoimage) } args[5] = strchr(codefile, '/') + 1; - ret = exec_genisoimg(args); + ret = exec_xorrisofs(args); if (ret) { - fprintf(stderr, "genisoimage failed: %i\n", ret); + fprintf(stderr, "xorrisofs failed: %i\n", ret); } unlink(codefile); @@ -211,12 +211,12 @@ int main(int argc, char **argv) { int ret; const char *arch = qtest_get_arch(); - const char *genisocheck[] = { "genisoimage", "-version", NULL }; + const char *xorrisocheck[] = { "xorrisofs", "-version", NULL }; g_test_init(&argc, &argv, NULL); - if (exec_genisoimg(genisocheck)) { - /* genisoimage not available - so can't run tests */ + if (exec_xorrisofs(xorrisocheck)) { + /* xorrisofs not available - so can't run tests */ return g_test_run(); } From 80bd81cadd127c1e2fc784612a52abe392670ba4 Mon Sep 17 00:00:00 2001 From: Claudio Imbrenda Date: Fri, 5 May 2023 14:00:51 +0200 Subject: [PATCH 09/21] util/async-teardown: wire up query-command-line-options Add new -run-with option with an async-teardown=on|off parameter. It is visible in the output of query-command-line-options QMP command, so it can be discovered and used by libvirt. The option -async-teardown is now redundant, deprecate it. Reported-by: Boris Fiuczynski Fixes: c891c24b1a ("os-posix: asynchronous teardown for shutdown on Linux") Signed-off-by: Claudio Imbrenda Message-Id: <20230505120051.36605-2-imbrenda@linux.ibm.com> [thuth: Add curly braces to fix error with GCC 8.5, fix bug in deprecated.rst] Signed-off-by: Thomas Huth --- docs/about/deprecated.rst | 5 +++++ os-posix.c | 14 ++++++++++++++ qemu-options.hx | 34 +++++++++++++++++++++++----------- util/async-teardown.c | 21 +++++++++++++++++++++ 4 files changed, 63 insertions(+), 11 deletions(-) diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst index 4c7f08803e..7bb4d2f4f6 100644 --- a/docs/about/deprecated.rst +++ b/docs/about/deprecated.rst @@ -111,6 +111,11 @@ Use ``-machine acpi=off`` instead. The HAXM project has been retired (see https://github.com/intel/haxm#status). Use "whpx" (on Windows) or "hvf" (on macOS) instead. +``-async-teardown`` (since 8.1) +''''''''''''''''''''''''''''''' + +Use ``-run-with async-teardown=on`` instead. + ``-singlestep`` (since 8.1) ''''''''''''''''''''''''''' diff --git a/os-posix.c b/os-posix.c index 5adc69f560..90ea71725f 100644 --- a/os-posix.c +++ b/os-posix.c @@ -36,6 +36,8 @@ #include "qemu/log.h" #include "sysemu/runstate.h" #include "qemu/cutils.h" +#include "qemu/config-file.h" +#include "qemu/option.h" #ifdef CONFIG_LINUX #include @@ -152,9 +154,21 @@ int os_parse_cmd_args(int index, const char *optarg) daemonize = 1; break; #if defined(CONFIG_LINUX) + /* deprecated */ case QEMU_OPTION_asyncteardown: init_async_teardown(); break; + case QEMU_OPTION_run_with: { + QemuOpts *opts = qemu_opts_parse_noisily(qemu_find_opts("run-with"), + optarg, false); + if (!opts) { + exit(1); + } + if (qemu_opt_get_bool(opts, "async-teardown", false)) { + init_async_teardown(); + } + break; + } #endif default: return -1; diff --git a/qemu-options.hx b/qemu-options.hx index 42b9094c10..30690d9c3f 100644 --- a/qemu-options.hx +++ b/qemu-options.hx @@ -4828,20 +4828,32 @@ DEF("qtest-log", HAS_ARG, QEMU_OPTION_qtest_log, "", QEMU_ARCH_ALL) DEF("async-teardown", 0, QEMU_OPTION_asyncteardown, "-async-teardown enable asynchronous teardown\n", QEMU_ARCH_ALL) -#endif SRST ``-async-teardown`` - Enable asynchronous teardown. A new process called "cleanup/" - will be created at startup sharing the address space with the main qemu - process, using clone. It will wait for the main qemu process to - terminate completely, and then exit. - This allows qemu to terminate very quickly even if the guest was - huge, leaving the teardown of the address space to the cleanup - process. Since the cleanup process shares the same cgroups as the - main qemu process, accounting is performed correctly. This only - works if the cleanup process is not forcefully killed with SIGKILL - before the main qemu process has terminated completely. + This option is deprecated and should no longer be used. The new option + ``-run-with async-teardown=on`` is a replacement. ERST +DEF("run-with", HAS_ARG, QEMU_OPTION_run_with, + "-run-with async-teardown[=on|off]\n" + " misc QEMU process lifecycle options\n" + " async-teardown=on enables asynchronous teardown\n", + QEMU_ARCH_ALL) +SRST +``-run-with`` + Set QEMU process lifecycle options. + + ``async-teardown=on`` enables asynchronous teardown. A new process called + "cleanup/" will be created at startup sharing the address + space with the main QEMU process, using clone. It will wait for the + main QEMU process to terminate completely, and then exit. This allows + QEMU to terminate very quickly even if the guest was huge, leaving the + teardown of the address space to the cleanup process. Since the cleanup + process shares the same cgroups as the main QEMU process, accounting is + performed correctly. This only works if the cleanup process is not + forcefully killed with SIGKILL before the main QEMU process has + terminated completely. +ERST +#endif DEF("msg", HAS_ARG, QEMU_OPTION_msg, "-msg [timestamp[=on|off]][,guest-name=[on|off]]\n" diff --git a/util/async-teardown.c b/util/async-teardown.c index 62cdeb0f20..3ab19c8740 100644 --- a/util/async-teardown.c +++ b/util/async-teardown.c @@ -12,6 +12,9 @@ */ #include "qemu/osdep.h" +#include "qemu/config-file.h" +#include "qemu/option.h" +#include "qemu/module.h" #include #include #include @@ -144,3 +147,21 @@ void init_async_teardown(void) clone(async_teardown_fn, new_stack_for_clone(), CLONE_VM, NULL); sigprocmask(SIG_SETMASK, &old_signals, NULL); } + +static QemuOptsList qemu_run_with_opts = { + .name = "run-with", + .head = QTAILQ_HEAD_INITIALIZER(qemu_run_with_opts.head), + .desc = { + { + .name = "async-teardown", + .type = QEMU_OPT_BOOL, + }, + { /* end of list */ } + }, +}; + +static void register_teardown(void) +{ + qemu_add_opts(&qemu_run_with_opts); +} +opts_init(register_teardown); From 88693ab2a53f2f3d25cb39a7b5034ab391bc5a81 Mon Sep 17 00:00:00 2001 From: Claudio Imbrenda Date: Wed, 10 May 2023 12:55:31 +0200 Subject: [PATCH 10/21] s390x/pv: Fix spurious warning with asynchronous teardown Kernel commit 292a7d6fca33 ("KVM: s390: pv: fix asynchronous teardown for small VMs") causes the KVM_PV_ASYNC_CLEANUP_PREPARE ioctl to fail if the VM is not larger than 2GiB. QEMU would attempt it and fail, print an error message, and then proceed with a normal teardown. Avoid attempting to use asynchronous teardown altogether when the VM is not larger than 2 GiB. This will avoid triggering the error message and also avoid pointless overhead; normal teardown is fast enough for small VMs. Reported-by: Marc Hartmayer Fixes: c3a073c610 ("s390x/pv: Add support for asynchronous teardown for reboot") Link: https://lore.kernel.org/all/20230421085036.52511-2-imbrenda@linux.ibm.com/ Signed-off-by: Claudio Imbrenda Message-Id: <20230510105531.30623-2-imbrenda@linux.ibm.com> Reviewed-by: Thomas Huth [thuth: Fix inline function parameter in pv.h] Signed-off-by: Thomas Huth --- hw/s390x/pv.c | 10 ++++++++-- hw/s390x/s390-virtio-ccw.c | 2 +- include/hw/s390x/pv.h | 6 +++--- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/hw/s390x/pv.c b/hw/s390x/pv.c index 49ea38236c..b63f3784c6 100644 --- a/hw/s390x/pv.c +++ b/hw/s390x/pv.c @@ -13,6 +13,7 @@ #include +#include "qemu/units.h" #include "qapi/error.h" #include "qemu/error-report.h" #include "sysemu/kvm.h" @@ -115,7 +116,7 @@ static void *s390_pv_do_unprot_async_fn(void *p) return NULL; } -bool s390_pv_vm_try_disable_async(void) +bool s390_pv_vm_try_disable_async(S390CcwMachineState *ms) { /* * t is only needed to create the thread; once qemu_thread_create @@ -123,7 +124,12 @@ bool s390_pv_vm_try_disable_async(void) */ QemuThread t; - if (!kvm_check_extension(kvm_state, KVM_CAP_S390_PROTECTED_ASYNC_DISABLE)) { + /* + * If the feature is not present or if the VM is not larger than 2 GiB, + * KVM_PV_ASYNC_CLEANUP_PREPARE fill fail; no point in attempting it. + */ + if ((MACHINE(ms)->maxram_size <= 2 * GiB) || + !kvm_check_extension(kvm_state, KVM_CAP_S390_PROTECTED_ASYNC_DISABLE)) { return false; } if (s390_pv_cmd(KVM_PV_ASYNC_CLEANUP_PREPARE, NULL) != 0) { diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index e6f2c62625..2516b89b32 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -330,7 +330,7 @@ static inline void s390_do_cpu_ipl(CPUState *cs, run_on_cpu_data arg) static void s390_machine_unprotect(S390CcwMachineState *ms) { - if (!s390_pv_vm_try_disable_async()) { + if (!s390_pv_vm_try_disable_async(ms)) { s390_pv_vm_disable(); } ms->pv = false; diff --git a/include/hw/s390x/pv.h b/include/hw/s390x/pv.h index 966306a9db..7b935e2246 100644 --- a/include/hw/s390x/pv.h +++ b/include/hw/s390x/pv.h @@ -14,10 +14,10 @@ #include "qapi/error.h" #include "sysemu/kvm.h" +#include "hw/s390x/s390-virtio-ccw.h" #ifdef CONFIG_KVM #include "cpu.h" -#include "hw/s390x/s390-virtio-ccw.h" static inline bool s390_is_pv(void) { @@ -41,7 +41,7 @@ static inline bool s390_is_pv(void) int s390_pv_query_info(void); int s390_pv_vm_enable(void); void s390_pv_vm_disable(void); -bool s390_pv_vm_try_disable_async(void); +bool s390_pv_vm_try_disable_async(S390CcwMachineState *ms); int s390_pv_set_sec_parms(uint64_t origin, uint64_t length); int s390_pv_unpack(uint64_t addr, uint64_t size, uint64_t tweak); void s390_pv_prep_reset(void); @@ -61,7 +61,7 @@ static inline bool s390_is_pv(void) { return false; } static inline int s390_pv_query_info(void) { return 0; } static inline int s390_pv_vm_enable(void) { return 0; } static inline void s390_pv_vm_disable(void) {} -static inline bool s390_pv_vm_try_disable_async(void) { return false; } +static inline bool s390_pv_vm_try_disable_async(S390CcwMachineState *ms) { return false; } static inline int s390_pv_set_sec_parms(uint64_t origin, uint64_t length) { return 0; } static inline int s390_pv_unpack(uint64_t addr, uint64_t size, uint64_t tweak) { return 0; } static inline void s390_pv_prep_reset(void) {} From 2a851fca9fcf6e1af53b7d494802b05069597787 Mon Sep 17 00:00:00 2001 From: Ani Sinha Date: Sat, 6 May 2023 12:50:12 +0530 Subject: [PATCH 11/21] docs/devel: remind developers to run CI container pipeline when updating images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When new dependencies and packages are added to containers, its important to run CI container generation pipelines on gitlab to make sure that there are no obvious conflicts between packages that are being added and those that are already present. Running CI container pipelines will make sure that there are no such breakages before we commit the change updating the containers. Add a line in the documentation reminding developers to run the pipeline before submitting the change. It will also ease the life of the maintainers. Signed-off-by: Ani Sinha Reviewed-by: Daniel P. Berrangé Message-Id: <20230506072012.10350-1-anisinha@redhat.com> Signed-off-by: Thomas Huth --- docs/devel/testing.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/docs/devel/testing.rst b/docs/devel/testing.rst index 4071e72710..203facb417 100644 --- a/docs/devel/testing.rst +++ b/docs/devel/testing.rst @@ -479,6 +479,12 @@ first to contribute the mapping to the ``libvirt-ci`` project: contains the ``mappings.yml`` update. Then add the prerequisite and run ``make lcitool-refresh``. + * Please also trigger gitlab container generation pipelines on your change + for as many OS distros as practical to make sure that there are no + obvious breakages when adding the new pre-requisite. Please see + `CI `__ documentation + page on how to trigger gitlab CI pipelines on your change. + For enterprise distros that default to old, end-of-life versions of the Python runtime, QEMU uses a separate set of mappings that work with more recent versions. These can be found in ``tests/lcitool/mappings.yml``. From c70bb9a771d467302d1c7df5c5bd56b48f42716e Mon Sep 17 00:00:00 2001 From: Lizhi Yang Date: Thu, 11 May 2023 16:01:19 +0800 Subject: [PATCH 12/21] docs/about/emulation: fix typo MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Duplicated word "are". Signed-off-by: Lizhi Yang Reviewed-by: Philippe Mathieu-Daudé Message-Id: <20230511080119.99018-1-sledgeh4w@gmail.com> Signed-off-by: Thomas Huth --- docs/about/emulation.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/about/emulation.rst b/docs/about/emulation.rst index b510a54418..0ad0b86f0d 100644 --- a/docs/about/emulation.rst +++ b/docs/about/emulation.rst @@ -99,7 +99,7 @@ depending on the guest architecture. - Yes - A configurable 32 bit soft core now owned by Cadence -A number of features are are only available when running under +A number of features are only available when running under emulation including :ref:`Record/Replay` and :ref:`TCG Plugins`. .. _Semihosting: From 5503da4a0c9e7f6853a175f5e273897680cd12df Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Mon, 24 Apr 2023 18:04:32 +0200 Subject: [PATCH 13/21] hw/core: Use a callback for target specific query-cpus-fast information For being able to create a universal QEMU binary one day, core files like machine-qmp-cmds.c must not contain any "#ifdef TARGET_..." parts. Thus let's provide the target specific function via a function pointer in CPUClass instead, as a first step towards making this file target independent. Message-Id: <20230424160434.331175-2-thuth@redhat.com> Signed-off-by: Thomas Huth --- hw/core/machine-qmp-cmds.c | 16 ++-------------- include/hw/core/cpu.h | 4 ++++ include/qemu/typedefs.h | 1 + target/s390x/cpu.c | 8 ++++++++ 4 files changed, 15 insertions(+), 14 deletions(-) diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c index b98ff15089..c158c02aa3 100644 --- a/hw/core/machine-qmp-cmds.c +++ b/hw/core/machine-qmp-cmds.c @@ -28,18 +28,6 @@ #include "sysemu/runstate.h" #include "sysemu/sysemu.h" -static void cpustate_to_cpuinfo_s390(CpuInfoS390 *info, const CPUState *cpu) -{ -#ifdef TARGET_S390X - S390CPU *s390_cpu = S390_CPU(cpu); - CPUS390XState *env = &s390_cpu->env; - - info->cpu_state = env->cpu_state; -#else - abort(); -#endif -} - /* * fast means: we NEVER interrupt vCPU threads to retrieve * information from KVM. @@ -68,8 +56,8 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp) } value->target = target; - if (target == SYS_EMU_TARGET_S390X) { - cpustate_to_cpuinfo_s390(&value->u.s390x, cpu); + if (cpu->cc->query_cpu_fast) { + cpu->cc->query_cpu_fast(cpu, value); } QAPI_LIST_APPEND(tail, value); diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index 397fd3ac68..5a019a27bc 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -106,6 +106,9 @@ struct SysemuCPUOps; * @has_work: Callback for checking if there is work to do. * @memory_rw_debug: Callback for GDB memory access. * @dump_state: Callback for dumping state. + * @query_cpu_fast: + * Fill in target specific information for the "query-cpus-fast" + * QAPI call. * @get_arch_id: Callback for getting architecture-dependent CPU ID. * @set_pc: Callback for setting the Program Counter register. This * should have the semantics used by the target architecture when @@ -151,6 +154,7 @@ struct CPUClass { int (*memory_rw_debug)(CPUState *cpu, vaddr addr, uint8_t *buf, int len, bool is_write); void (*dump_state)(CPUState *cpu, FILE *, int flags); + void (*query_cpu_fast)(CPUState *cpu, CpuInfoFast *value); int64_t (*get_arch_id)(CPUState *cpu); void (*set_pc)(CPUState *cpu, vaddr value); vaddr (*get_pc)(CPUState *cpu); diff --git a/include/qemu/typedefs.h b/include/qemu/typedefs.h index df4b55ac65..8e9ef252f5 100644 --- a/include/qemu/typedefs.h +++ b/include/qemu/typedefs.h @@ -41,6 +41,7 @@ typedef struct CompatProperty CompatProperty; typedef struct ConfidentialGuestSupport ConfidentialGuestSupport; typedef struct CPUAddressSpace CPUAddressSpace; typedef struct CPUArchState CPUArchState; +typedef struct CpuInfoFast CpuInfoFast; typedef struct CPUJumpCache CPUJumpCache; typedef struct CPUState CPUState; typedef struct CPUTLBEntryFull CPUTLBEntryFull; diff --git a/target/s390x/cpu.c b/target/s390x/cpu.c index 40fdeaa905..df167493c3 100644 --- a/target/s390x/cpu.c +++ b/target/s390x/cpu.c @@ -140,6 +140,13 @@ static bool s390_cpu_has_work(CPUState *cs) return s390_cpu_has_int(cpu); } +static void s390_query_cpu_fast(CPUState *cpu, CpuInfoFast *value) +{ + S390CPU *s390_cpu = S390_CPU(cpu); + + value->u.s390x.cpu_state = s390_cpu->env.cpu_state; +} + /* S390CPUClass::reset() */ static void s390_cpu_reset(CPUState *s, cpu_reset_type type) { @@ -332,6 +339,7 @@ static void s390_cpu_class_init(ObjectClass *oc, void *data) cc->class_by_name = s390_cpu_class_by_name, cc->has_work = s390_cpu_has_work; cc->dump_state = s390_cpu_dump_state; + cc->query_cpu_fast = s390_query_cpu_fast; cc->set_pc = s390_cpu_set_pc; cc->get_pc = s390_cpu_get_pc; cc->gdb_read_register = s390_cpu_gdb_read_register; From 1077f50b23065a1d07cbbc6bad1141d50a5d25b9 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Mon, 24 Apr 2023 18:04:33 +0200 Subject: [PATCH 14/21] cpu: Introduce a wrapper for being able to use TARGET_NAME in common code In some spots, it would be helpful to be able to use TARGET_NAME in common (target independent) code, too. Thus introduce a wrapper that can be called from common code, too, just like we already have one for target_words_bigendian(). Message-Id: <20230424160434.331175-3-thuth@redhat.com> Signed-off-by: Thomas Huth --- cpu.c | 5 +++++ include/hw/core/cpu.h | 2 ++ 2 files changed, 7 insertions(+) diff --git a/cpu.c b/cpu.c index 9105c85404..65ebaf8159 100644 --- a/cpu.c +++ b/cpu.c @@ -427,6 +427,11 @@ bool target_words_bigendian(void) #endif } +const char *target_name(void) +{ + return TARGET_NAME; +} + void page_size_init(void) { /* NOTE: we can always suppose that qemu_host_page_size >= diff --git a/include/hw/core/cpu.h b/include/hw/core/cpu.h index 5a019a27bc..39150cf8f8 100644 --- a/include/hw/core/cpu.h +++ b/include/hw/core/cpu.h @@ -1013,6 +1013,8 @@ void cpu_exec_unrealizefn(CPUState *cpu); */ bool target_words_bigendian(void); +const char *target_name(void); + void page_size_init(void); #ifdef NEED_CPU_H From 89c81b3d4c1ccafa22f58a6169c9410003654b8e Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Mon, 24 Apr 2023 18:04:34 +0200 Subject: [PATCH 15/21] hw/core: Move machine-qmp-cmds.c into the target independent source set The only target specific code that is left in here are two spots that use TARGET_NAME. Change them to use the new target_name() wrapper function instead, so we can move the file into the common softmmu_ss source set. That way we only have to compile this file once, and not for each target anymore. Message-Id: <20230424160434.331175-4-thuth@redhat.com> Signed-off-by: Thomas Huth --- hw/core/machine-qmp-cmds.c | 4 ++-- hw/core/meson.build | 5 +---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/hw/core/machine-qmp-cmds.c b/hw/core/machine-qmp-cmds.c index c158c02aa3..3860a50c3b 100644 --- a/hw/core/machine-qmp-cmds.c +++ b/hw/core/machine-qmp-cmds.c @@ -37,7 +37,7 @@ CpuInfoFastList *qmp_query_cpus_fast(Error **errp) MachineState *ms = MACHINE(qdev_get_machine()); MachineClass *mc = MACHINE_GET_CLASS(ms); CpuInfoFastList *head = NULL, **tail = &head; - SysEmuTarget target = qapi_enum_parse(&SysEmuTarget_lookup, TARGET_NAME, + SysEmuTarget target = qapi_enum_parse(&SysEmuTarget_lookup, target_name(), -1, &error_abort); CPUState *cpu; @@ -117,7 +117,7 @@ TargetInfo *qmp_query_target(Error **errp) { TargetInfo *info = g_malloc0(sizeof(*info)); - info->arch = qapi_enum_parse(&SysEmuTarget_lookup, TARGET_NAME, -1, + info->arch = qapi_enum_parse(&SysEmuTarget_lookup, target_name(), -1, &error_abort); return info; diff --git a/hw/core/meson.build b/hw/core/meson.build index ae977c9396..959bc924d4 100644 --- a/hw/core/meson.build +++ b/hw/core/meson.build @@ -41,6 +41,7 @@ softmmu_ss.add(files( 'gpio.c', 'loader.c', 'machine-hmp-cmds.c', + 'machine-qmp-cmds.c', 'machine.c', 'nmi.c', 'null-machine.c', @@ -51,7 +52,3 @@ softmmu_ss.add(files( 'vm-change-state-handler.c', 'clock-vmstate.c', )) - -specific_ss.add(when: 'CONFIG_SOFTMMU', if_true: files( - 'machine-qmp-cmds.c', -)) From a9ea0a9c930eedbc6356869099468cbe27cc7705 Mon Sep 17 00:00:00 2001 From: Thomas Huth Date: Mon, 8 May 2023 14:03:14 +0200 Subject: [PATCH 16/21] hw/net: Move xilinx_ethlite.c to the target-independent source set Now that the tswap() functions are available for target-independent code, too, we can move xilinx_ethlite.c from specific_ss to softmmu_ss to avoid that we have to compile this file multiple times. Message-Id: <20230508120314.59274-1-thuth@redhat.com> Reviewed-by: Francisco Iglesias Reviewed-by: Alistair Francis Signed-off-by: Thomas Huth --- hw/net/meson.build | 2 +- hw/net/xilinx_ethlite.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/hw/net/meson.build b/hw/net/meson.build index e2be0654a1..a7860c5efe 100644 --- a/hw/net/meson.build +++ b/hw/net/meson.build @@ -43,7 +43,7 @@ softmmu_ss.add(when: 'CONFIG_NPCM7XX', if_true: files('npcm7xx_emc.c')) softmmu_ss.add(when: 'CONFIG_ETRAXFS', if_true: files('etraxfs_eth.c')) softmmu_ss.add(when: 'CONFIG_COLDFIRE', if_true: files('mcf_fec.c')) specific_ss.add(when: 'CONFIG_PSERIES', if_true: files('spapr_llan.c')) -specific_ss.add(when: 'CONFIG_XILINX_ETHLITE', if_true: files('xilinx_ethlite.c')) +softmmu_ss.add(when: 'CONFIG_XILINX_ETHLITE', if_true: files('xilinx_ethlite.c')) softmmu_ss.add(when: 'CONFIG_VIRTIO_NET', if_true: files('net_rx_pkt.c')) specific_ss.add(when: 'CONFIG_VIRTIO_NET', if_true: files('virtio-net.c')) diff --git a/hw/net/xilinx_ethlite.c b/hw/net/xilinx_ethlite.c index 99c22819ea..89f4f3b254 100644 --- a/hw/net/xilinx_ethlite.c +++ b/hw/net/xilinx_ethlite.c @@ -25,7 +25,7 @@ #include "qemu/osdep.h" #include "qemu/module.h" #include "qom/object.h" -#include "cpu.h" /* FIXME should not use tswap* */ +#include "exec/tswap.h" #include "hw/sysbus.h" #include "hw/irq.h" #include "hw/qdev-properties.h" From 970641de01908dd09b569965e78f13842e5854bc Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Thu, 11 May 2023 15:47:26 +0200 Subject: [PATCH 17/21] s390x/tcg: Fix LDER instruction format It's RRE, not RXE. Found by running valgrind's none/tests/s390x/bfp-2. Fixes: 86b59624c4aa ("s390x/tcg: Implement LOAD LENGTHENED short HFP to long HFP") Reviewed-by: David Hildenbrand Signed-off-by: Ilya Leoshkevich Message-Id: <20230511134726.469651-1-iii@linux.ibm.com> Reviewed-by: Richard Henderson Signed-off-by: Thomas Huth --- target/s390x/tcg/insn-data.h.inc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/target/s390x/tcg/insn-data.h.inc b/target/s390x/tcg/insn-data.h.inc index 597d968b0e..1f1ac742a9 100644 --- a/target/s390x/tcg/insn-data.h.inc +++ b/target/s390x/tcg/insn-data.h.inc @@ -606,7 +606,7 @@ F(0xed04, LDEB, RXE, Z, 0, m2_32u, new, f1, ldeb, 0, IF_BFP) F(0xed05, LXDB, RXE, Z, 0, m2_64, new_x, x1, lxdb, 0, IF_BFP) F(0xed06, LXEB, RXE, Z, 0, m2_32u, new_x, x1, lxeb, 0, IF_BFP) - F(0xb324, LDER, RXE, Z, 0, e2, new, f1, lde, 0, IF_AFP1) + F(0xb324, LDER, RRE, Z, 0, e2, new, f1, lde, 0, IF_AFP1) F(0xed24, LDE, RXE, Z, 0, m2_32u, new, f1, lde, 0, IF_AFP1) /* LOAD ROUNDED */ F(0xb344, LEDBR, RRF_e, Z, 0, f2, new, e1, ledb, 0, IF_BFP) From f8d7c90f836370d2a0c8fee8e6c43d49d35ad770 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Thu, 11 May 2023 13:46:50 +0200 Subject: [PATCH 18/21] tests/tcg/multiarch: Make the system memory test work on big-endian MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Store the bytes in descending order on big-endian. Invert the logic in the multi-byte signed tests on big-endian. Make the checks in the multi-byte signed tests stricter. Signed-off-by: Ilya Leoshkevich Reviewed-by: Alex Bennée Message-Id: <20230511114651.439872-2-iii@linux.ibm.com> Signed-off-by: Thomas Huth --- tests/tcg/multiarch/system/memory.c | 67 ++++++++++++++++++----------- 1 file changed, 43 insertions(+), 24 deletions(-) diff --git a/tests/tcg/multiarch/system/memory.c b/tests/tcg/multiarch/system/memory.c index 214f7d4f54..e29786ae55 100644 --- a/tests/tcg/multiarch/system/memory.c +++ b/tests/tcg/multiarch/system/memory.c @@ -40,18 +40,21 @@ static void pdot(int count) } /* - * Helper macros for shift/extract so we can keep our endian handling - * in one place. + * Helper macros for endian handling. */ -#define BYTE_SHIFT(b, pos) ((uint64_t)b << (pos * 8)) -#define BYTE_EXTRACT(b, pos) ((b >> (pos * 8)) & 0xff) +#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__ +#define BYTE_SHIFT(b, pos) (b << (pos * 8)) +#define BYTE_NEXT(b) ((b)++) +#elif __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ +#define BYTE_SHIFT(b, pos) (b << ((sizeof(b) - 1 - (pos)) * 8)) +#define BYTE_NEXT(b) (--(b)) +#else +#error Unsupported __BYTE_ORDER__ +#endif /* - * Fill the data with ascending value bytes. - * - * Currently we only support Little Endian machines so write in - * ascending address order. When we read higher address bytes should - * either be zero or higher than the lower bytes. + * Fill the data with ascending (for little-endian) or descending (for + * big-endian) value bytes. */ static void init_test_data_u8(int unused_offset) @@ -62,14 +65,14 @@ static void init_test_data_u8(int unused_offset) ml_printf("Filling test area with u8:"); for (i = 0; i < TEST_SIZE; i++) { - *ptr++ = count++; + *ptr++ = BYTE_NEXT(count); pdot(i); } ml_printf("done\n"); } /* - * Full the data with alternating positive and negative bytes. This + * Fill the data with alternating positive and negative bytes. This * should mean for reads larger than a byte all subsequent reads will * stay either negative or positive. We never write 0. */ @@ -119,7 +122,7 @@ static void init_test_data_u16(int offset) reset_start_data(offset); for (i = 0; i < max; i++) { - uint8_t low = count++, high = count++; + uint16_t low = BYTE_NEXT(count), high = BYTE_NEXT(count); word = BYTE_SHIFT(high, 1) | BYTE_SHIFT(low, 0); *ptr++ = word; pdot(i); @@ -139,9 +142,10 @@ static void init_test_data_u32(int offset) reset_start_data(offset); for (i = 0; i < max; i++) { - uint8_t b4 = count++, b3 = count++; - uint8_t b2 = count++, b1 = count++; - word = BYTE_SHIFT(b1, 3) | BYTE_SHIFT(b2, 2) | BYTE_SHIFT(b3, 1) | b4; + uint32_t b4 = BYTE_NEXT(count), b3 = BYTE_NEXT(count); + uint32_t b2 = BYTE_NEXT(count), b1 = BYTE_NEXT(count); + word = BYTE_SHIFT(b1, 3) | BYTE_SHIFT(b2, 2) | BYTE_SHIFT(b3, 1) | + BYTE_SHIFT(b4, 0); *ptr++ = word; pdot(i); } @@ -160,13 +164,13 @@ static void init_test_data_u64(int offset) reset_start_data(offset); for (i = 0; i < max; i++) { - uint8_t b8 = count++, b7 = count++; - uint8_t b6 = count++, b5 = count++; - uint8_t b4 = count++, b3 = count++; - uint8_t b2 = count++, b1 = count++; + uint64_t b8 = BYTE_NEXT(count), b7 = BYTE_NEXT(count); + uint64_t b6 = BYTE_NEXT(count), b5 = BYTE_NEXT(count); + uint64_t b4 = BYTE_NEXT(count), b3 = BYTE_NEXT(count); + uint64_t b2 = BYTE_NEXT(count), b1 = BYTE_NEXT(count); word = BYTE_SHIFT(b1, 7) | BYTE_SHIFT(b2, 6) | BYTE_SHIFT(b3, 5) | BYTE_SHIFT(b4, 4) | BYTE_SHIFT(b5, 3) | BYTE_SHIFT(b6, 2) | - BYTE_SHIFT(b7, 1) | b8; + BYTE_SHIFT(b7, 1) | BYTE_SHIFT(b8, 0); *ptr++ = word; pdot(i); } @@ -374,12 +378,20 @@ static bool read_test_data_s16(int offset, bool neg_first) ml_printf("Reading s16 from %#lx (offset %d, %s):", ptr, offset, neg_first ? "neg" : "pos"); + /* + * If the first byte is negative, then the last byte is positive. + * Therefore the logic below must be flipped for big-endian. + */ +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ + neg_first = !neg_first; +#endif + for (i = 0; i < max; i++) { int32_t data = *ptr++; if (neg_first && data < 0) { pdot(i); - } else if (data > 0) { + } else if (!neg_first && data > 0) { pdot(i); } else { ml_printf("Error %d %c 0\n", data, neg_first ? '<' : '>'); @@ -399,12 +411,20 @@ static bool read_test_data_s32(int offset, bool neg_first) ml_printf("Reading s32 from %#lx (offset %d, %s):", ptr, offset, neg_first ? "neg" : "pos"); + /* + * If the first byte is negative, then the last byte is positive. + * Therefore the logic below must be flipped for big-endian. + */ +#if __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__ + neg_first = !neg_first; +#endif + for (i = 0; i < max; i++) { int64_t data = *ptr++; if (neg_first && data < 0) { pdot(i); - } else if (data > 0) { + } else if (!neg_first && data > 0) { pdot(i); } else { ml_printf("Error %d %c 0\n", data, neg_first ? '<' : '>'); @@ -419,8 +439,7 @@ static bool read_test_data_s32(int offset, bool neg_first) * Read the test data and verify at various offsets * * For everything except bytes all our reads should be either positive - * or negative depending on what offset we are reading from. Currently - * we only handle LE systems. + * or negative depending on what offset we are reading from. */ read_sfn read_sfns[] = { read_test_data_s8, read_test_data_s16, From c2485ea402449dd19d51b2d7db7060afb2330b79 Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Thu, 11 May 2023 13:46:51 +0200 Subject: [PATCH 19/21] tests/tcg/s390x: Enable the multiarch system tests Multiarch tests are written in C and need support for printing characters. Instead of implementing the runtime from scratch, just reuse the pc-bios/s390-ccw one. Run tests with -nographic in order to enable SCLP (enable this for the existing tests as well, since it does not hurt). Use the default linker script for the new tests. Signed-off-by: Ilya Leoshkevich Reviewed-by: Richard Henderson Message-Id: <20230511114651.439872-3-iii@linux.ibm.com> Signed-off-by: Thomas Huth --- tests/tcg/s390x/Makefile.softmmu-target | 40 ++++++++++++++++--------- tests/tcg/s390x/console.c | 12 ++++++++ tests/tcg/s390x/head64.S | 31 +++++++++++++++++++ 3 files changed, 69 insertions(+), 14 deletions(-) create mode 100644 tests/tcg/s390x/console.c create mode 100644 tests/tcg/s390x/head64.S diff --git a/tests/tcg/s390x/Makefile.softmmu-target b/tests/tcg/s390x/Makefile.softmmu-target index 192315dd20..44dfd71629 100644 --- a/tests/tcg/s390x/Makefile.softmmu-target +++ b/tests/tcg/s390x/Makefile.softmmu-target @@ -1,28 +1,40 @@ S390X_SRC=$(SRC_PATH)/tests/tcg/s390x VPATH+=$(S390X_SRC) -QEMU_OPTS=-action panic=exit-failure -kernel +QEMU_OPTS=-action panic=exit-failure -nographic -kernel LINK_SCRIPT=$(S390X_SRC)/softmmu.ld -LDFLAGS=-nostdlib -static -Wl,-T$(LINK_SCRIPT) -Wl,--build-id=none +CFLAGS+=-ggdb -O0 +LDFLAGS=-nostdlib -static %.o: %.S $(CC) -march=z13 -m64 -c $< -o $@ -%: %.o $(LINK_SCRIPT) +%.o: %.c + $(CC) $(CFLAGS) $(EXTRA_CFLAGS) -march=z13 -m64 -c $< -o $@ + +%: %.o $(CC) $< -o $@ $(LDFLAGS) -TESTS += unaligned-lowcore -TESTS += bal -TESTS += sam -TESTS += lpsw -TESTS += lpswe-early -TESTS += ssm-early -TESTS += stosm-early -TESTS += exrl-ssm-early +ASM_TESTS = \ + bal \ + exrl-ssm-early \ + sam \ + lpsw \ + lpswe-early \ + ssm-early \ + stosm-early \ + unaligned-lowcore include $(S390X_SRC)/pgm-specification.mak $(PGM_SPECIFICATION_TESTS): pgm-specification-softmmu.o $(PGM_SPECIFICATION_TESTS): LDFLAGS+=pgm-specification-softmmu.o -TESTS += $(PGM_SPECIFICATION_TESTS) +ASM_TESTS += $(PGM_SPECIFICATION_TESTS) -# We don't currently support the multiarch system tests -undefine MULTIARCH_TESTS +$(ASM_TESTS): LDFLAGS += -Wl,-T$(LINK_SCRIPT) -Wl,--build-id=none +$(ASM_TESTS): $(LINK_SCRIPT) +TESTS += $(ASM_TESTS) + +S390X_MULTIARCH_RUNTIME_OBJS = head64.o console.o $(MINILIB_OBJS) +$(MULTIARCH_TESTS): $(S390X_MULTIARCH_RUNTIME_OBJS) +$(MULTIARCH_TESTS): LDFLAGS += $(S390X_MULTIARCH_RUNTIME_OBJS) +$(MULTIARCH_TESTS): CFLAGS += $(MINILIB_INC) +memory: CFLAGS += -DCHECK_UNALIGNED=0 diff --git a/tests/tcg/s390x/console.c b/tests/tcg/s390x/console.c new file mode 100644 index 0000000000..d43ce3f44b --- /dev/null +++ b/tests/tcg/s390x/console.c @@ -0,0 +1,12 @@ +/* + * Console code for multiarch tests. + * Reuses the pc-bios/s390-ccw implementation. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ +#include "../../../pc-bios/s390-ccw/sclp.c" + +void __sys_outc(char c) +{ + write(1, &c, sizeof(c)); +} diff --git a/tests/tcg/s390x/head64.S b/tests/tcg/s390x/head64.S new file mode 100644 index 0000000000..c6f36dfea4 --- /dev/null +++ b/tests/tcg/s390x/head64.S @@ -0,0 +1,31 @@ +/* + * Startup code for multiarch tests. + * Reuses the pc-bios/s390-ccw implementation. + * + * SPDX-License-Identifier: GPL-2.0-or-later + */ +#define main main_pre +#include "../../../pc-bios/s390-ccw/start.S" +#undef main + +main_pre: + aghi %r15,-160 /* reserve stack for C code */ + brasl %r14,sclp_setup + brasl %r14,main + larl %r1,success_psw /* check main() return code */ + ltgr %r2,%r2 + je 0f + larl %r1,failure_psw +0: + lpswe 0(%r1) + + .align 8 +success_psw: + .quad 0x2000180000000,0xfff /* see is_special_wait_psw() */ +failure_psw: + .quad 0x2000180000000,0 /* disabled wait */ + + .section .bss + .align 0x1000 +stack: + .skip 0x8000 From e8ecdfeb30f087574191cde523e846e023911c8d Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Thu, 27 Apr 2023 01:58:12 +0200 Subject: [PATCH 20/21] target/s390x: Fix EXECUTE of relative branches Fix a problem similar to the one fixed by commit 703d03a4aaf3 ("target/s390x: Fix EXECUTE of relative long instructions"), but now for relative branches. Reported-by: Nina Schoetterl-Glausch Signed-off-by: Ilya Leoshkevich Reviewed-by: Richard Henderson Message-Id: <20230426235813.198183-2-iii@linux.ibm.com> Signed-off-by: Thomas Huth --- target/s390x/tcg/translate.c | 81 ++++++++++++++++++++++++++---------- 1 file changed, 58 insertions(+), 23 deletions(-) diff --git a/target/s390x/tcg/translate.c b/target/s390x/tcg/translate.c index a05205beb1..d6670e6a87 100644 --- a/target/s390x/tcg/translate.c +++ b/target/s390x/tcg/translate.c @@ -1534,18 +1534,51 @@ static DisasJumpType op_bal(DisasContext *s, DisasOps *o) } } +/* + * Disassemble the target of a branch. The results are returned in a form + * suitable for passing into help_branch(): + * + * - bool IS_IMM reflects whether the target is fixed or computed. Non-EXECUTEd + * branches, whose DisasContext *S contains the relative immediate field RI, + * are considered fixed. All the other branches are considered computed. + * - int IMM is the value of RI. + * - TCGv_i64 CDEST is the address of the computed target. + */ +#define disas_jdest(s, ri, is_imm, imm, cdest) do { \ + if (have_field(s, ri)) { \ + if (unlikely(s->ex_value)) { \ + cdest = tcg_temp_new_i64(); \ + tcg_gen_ld_i64(cdest, cpu_env, offsetof(CPUS390XState, ex_target));\ + tcg_gen_addi_i64(cdest, cdest, (int64_t)get_field(s, ri) * 2); \ + is_imm = false; \ + } else { \ + is_imm = true; \ + } \ + } else { \ + is_imm = false; \ + } \ + imm = is_imm ? get_field(s, ri) : 0; \ +} while (false) + static DisasJumpType op_basi(DisasContext *s, DisasOps *o) { + DisasCompare c; + bool is_imm; + int imm; + pc_to_link_info(o->out, s, s->pc_tmp); - return help_goto_direct(s, s->base.pc_next + (int64_t)get_field(s, i2) * 2); + + disas_jdest(s, i2, is_imm, imm, o->in2); + disas_jcc(s, &c, 0xf); + return help_branch(s, &c, is_imm, imm, o->in2); } static DisasJumpType op_bc(DisasContext *s, DisasOps *o) { int m1 = get_field(s, m1); - bool is_imm = have_field(s, i2); - int imm = is_imm ? get_field(s, i2) : 0; DisasCompare c; + bool is_imm; + int imm; /* BCR with R2 = 0 causes no branching */ if (have_field(s, r2) && get_field(s, r2) == 0) { @@ -1562,6 +1595,7 @@ static DisasJumpType op_bc(DisasContext *s, DisasOps *o) return DISAS_NEXT; } + disas_jdest(s, i2, is_imm, imm, o->in2); disas_jcc(s, &c, m1); return help_branch(s, &c, is_imm, imm, o->in2); } @@ -1569,10 +1603,10 @@ static DisasJumpType op_bc(DisasContext *s, DisasOps *o) static DisasJumpType op_bct32(DisasContext *s, DisasOps *o) { int r1 = get_field(s, r1); - bool is_imm = have_field(s, i2); - int imm = is_imm ? get_field(s, i2) : 0; DisasCompare c; + bool is_imm; TCGv_i64 t; + int imm; c.cond = TCG_COND_NE; c.is_64 = false; @@ -1584,6 +1618,7 @@ static DisasJumpType op_bct32(DisasContext *s, DisasOps *o) c.u.s32.b = tcg_constant_i32(0); tcg_gen_extrl_i64_i32(c.u.s32.a, t); + disas_jdest(s, i2, is_imm, imm, o->in2); return help_branch(s, &c, is_imm, imm, o->in2); } @@ -1611,9 +1646,9 @@ static DisasJumpType op_bcth(DisasContext *s, DisasOps *o) static DisasJumpType op_bct64(DisasContext *s, DisasOps *o) { int r1 = get_field(s, r1); - bool is_imm = have_field(s, i2); - int imm = is_imm ? get_field(s, i2) : 0; DisasCompare c; + bool is_imm; + int imm; c.cond = TCG_COND_NE; c.is_64 = true; @@ -1622,6 +1657,7 @@ static DisasJumpType op_bct64(DisasContext *s, DisasOps *o) c.u.s64.a = regs[r1]; c.u.s64.b = tcg_constant_i64(0); + disas_jdest(s, i2, is_imm, imm, o->in2); return help_branch(s, &c, is_imm, imm, o->in2); } @@ -1629,10 +1665,10 @@ static DisasJumpType op_bx32(DisasContext *s, DisasOps *o) { int r1 = get_field(s, r1); int r3 = get_field(s, r3); - bool is_imm = have_field(s, i2); - int imm = is_imm ? get_field(s, i2) : 0; DisasCompare c; + bool is_imm; TCGv_i64 t; + int imm; c.cond = (s->insn->data ? TCG_COND_LE : TCG_COND_GT); c.is_64 = false; @@ -1645,6 +1681,7 @@ static DisasJumpType op_bx32(DisasContext *s, DisasOps *o) tcg_gen_extrl_i64_i32(c.u.s32.b, regs[r3 | 1]); store_reg32_i64(r1, t); + disas_jdest(s, i2, is_imm, imm, o->in2); return help_branch(s, &c, is_imm, imm, o->in2); } @@ -1652,9 +1689,9 @@ static DisasJumpType op_bx64(DisasContext *s, DisasOps *o) { int r1 = get_field(s, r1); int r3 = get_field(s, r3); - bool is_imm = have_field(s, i2); - int imm = is_imm ? get_field(s, i2) : 0; DisasCompare c; + bool is_imm; + int imm; c.cond = (s->insn->data ? TCG_COND_LE : TCG_COND_GT); c.is_64 = true; @@ -1668,6 +1705,7 @@ static DisasJumpType op_bx64(DisasContext *s, DisasOps *o) tcg_gen_add_i64(regs[r1], regs[r1], regs[r3]); c.u.s64.a = regs[r1]; + disas_jdest(s, i2, is_imm, imm, o->in2); return help_branch(s, &c, is_imm, imm, o->in2); } @@ -1685,10 +1723,9 @@ static DisasJumpType op_cj(DisasContext *s, DisasOps *o) c.u.s64.a = o->in1; c.u.s64.b = o->in2; - is_imm = have_field(s, i4); - if (is_imm) { - imm = get_field(s, i4); - } else { + o->out = NULL; + disas_jdest(s, i4, is_imm, imm, o->out); + if (!is_imm && !o->out) { imm = 0; o->out = get_address(s, 0, get_field(s, b4), get_field(s, d4)); @@ -5764,15 +5801,13 @@ static void in2_a2(DisasContext *s, DisasOps *o) static TCGv gen_ri2(DisasContext *s) { - int64_t delta = (int64_t)get_field(s, i2) * 2; - TCGv ri2; + TCGv ri2 = NULL; + bool is_imm; + int imm; - if (unlikely(s->ex_value)) { - ri2 = tcg_temp_new_i64(); - tcg_gen_ld_i64(ri2, cpu_env, offsetof(CPUS390XState, ex_target)); - tcg_gen_addi_i64(ri2, ri2, delta); - } else { - ri2 = tcg_constant_i64(s->base.pc_next + delta); + disas_jdest(s, i2, is_imm, imm, ri2); + if (is_imm) { + ri2 = tcg_constant_i64(s->base.pc_next + imm * 2); } return ri2; From bfa72590df14e4c94c03d2464f3abe18bf2e5dac Mon Sep 17 00:00:00 2001 From: Ilya Leoshkevich Date: Thu, 27 Apr 2023 01:58:13 +0200 Subject: [PATCH 21/21] tests/tcg/s390x: Test EXECUTE of relative branches Add a small test to prevent regressions. Signed-off-by: Ilya Leoshkevich Acked-by: Richard Henderson Message-Id: <20230426235813.198183-3-iii@linux.ibm.com> Signed-off-by: Thomas Huth --- tests/tcg/s390x/Makefile.target | 1 + tests/tcg/s390x/ex-branch.c | 158 ++++++++++++++++++++++++++++++++ 2 files changed, 159 insertions(+) create mode 100644 tests/tcg/s390x/ex-branch.c diff --git a/tests/tcg/s390x/Makefile.target b/tests/tcg/s390x/Makefile.target index 0031868b13..23dc8b6a63 100644 --- a/tests/tcg/s390x/Makefile.target +++ b/tests/tcg/s390x/Makefile.target @@ -34,6 +34,7 @@ TESTS+=cdsg TESTS+=chrl TESTS+=rxsbg TESTS+=ex-relative-long +TESTS+=ex-branch cdsg: CFLAGS+=-pthread cdsg: LDFLAGS+=-pthread diff --git a/tests/tcg/s390x/ex-branch.c b/tests/tcg/s390x/ex-branch.c new file mode 100644 index 0000000000..c606719152 --- /dev/null +++ b/tests/tcg/s390x/ex-branch.c @@ -0,0 +1,158 @@ +/* Check EXECUTE with relative branch instructions as targets. */ +#include +#include +#include +#include +#include + +struct test { + const char *name; + void (*func)(long *link, long *magic); + long exp_link; +}; + +/* Branch instructions and their expected effects. */ +#define LINK_64(test) ((long)test ## _exp_link) +#define LINK_NONE(test) -1L +#define FOR_EACH_INSN(F) \ + F(bras, "%[link]", LINK_64) \ + F(brasl, "%[link]", LINK_64) \ + F(brc, "0x8", LINK_NONE) \ + F(brcl, "0x8", LINK_NONE) \ + F(brct, "%%r0", LINK_NONE) \ + F(brctg, "%%r0", LINK_NONE) \ + F(brxh, "%%r2,%%r0", LINK_NONE) \ + F(brxhg, "%%r2,%%r0", LINK_NONE) \ + F(brxle, "%%r0,%%r1", LINK_NONE) \ + F(brxlg, "%%r0,%%r1", LINK_NONE) \ + F(crj, "%%r0,%%r0,8", LINK_NONE) \ + F(cgrj, "%%r0,%%r0,8", LINK_NONE) \ + F(cij, "%%r0,0,8", LINK_NONE) \ + F(cgij, "%%r0,0,8", LINK_NONE) \ + F(clrj, "%%r0,%%r0,8", LINK_NONE) \ + F(clgrj, "%%r0,%%r0,8", LINK_NONE) \ + F(clij, "%%r0,0,8", LINK_NONE) \ + F(clgij, "%%r0,0,8", LINK_NONE) + +#define INIT_TEST \ + "xgr %%r0,%%r0\n" /* %r0 = 0; %cc = 0 */ \ + "lghi %%r1,1\n" /* %r1 = 1 */ \ + "lghi %%r2,2\n" /* %r2 = 2 */ + +#define CLOBBERS_TEST "cc", "0", "1", "2" + +#define DEFINE_TEST(insn, args, exp_link) \ + extern char insn ## _exp_link[]; \ + static void test_ ## insn(long *link, long *magic) \ + { \ + asm(INIT_TEST \ + #insn " " args ",0f\n" \ + ".globl " #insn "_exp_link\n" \ + #insn "_exp_link:\n" \ + ".org . + 90\n" \ + "0: lgfi %[magic],0x12345678\n" \ + : [link] "+r" (*link) \ + , [magic] "+r" (*magic) \ + : : CLOBBERS_TEST); \ + } \ + extern char ex_ ## insn ## _exp_link[]; \ + static void test_ex_ ## insn(long *link, long *magic) \ + { \ + unsigned long target; \ + \ + asm(INIT_TEST \ + "larl %[target],0f\n" \ + "ex %%r0,0(%[target])\n" \ + ".globl ex_" #insn "_exp_link\n" \ + "ex_" #insn "_exp_link:\n" \ + ".org . + 60\n" \ + "0: " #insn " " args ",1f\n" \ + ".org . + 120\n" \ + "1: lgfi %[magic],0x12345678\n" \ + : [target] "=r" (target) \ + , [link] "+r" (*link) \ + , [magic] "+r" (*magic) \ + : : CLOBBERS_TEST); \ + } \ + extern char exrl_ ## insn ## _exp_link[]; \ + static void test_exrl_ ## insn(long *link, long *magic) \ + { \ + asm(INIT_TEST \ + "exrl %%r0,0f\n" \ + ".globl exrl_" #insn "_exp_link\n" \ + "exrl_" #insn "_exp_link:\n" \ + ".org . + 60\n" \ + "0: " #insn " " args ",1f\n" \ + ".org . + 120\n" \ + "1: lgfi %[magic],0x12345678\n" \ + : [link] "+r" (*link) \ + , [magic] "+r" (*magic) \ + : : CLOBBERS_TEST); \ + } + +/* Test functions. */ +FOR_EACH_INSN(DEFINE_TEST) + +/* Test definitions. */ +#define REGISTER_TEST(insn, args, _exp_link) \ + { \ + .name = #insn, \ + .func = test_ ## insn, \ + .exp_link = (_exp_link(insn)), \ + }, \ + { \ + .name = "ex " #insn, \ + .func = test_ex_ ## insn, \ + .exp_link = (_exp_link(ex_ ## insn)), \ + }, \ + { \ + .name = "exrl " #insn, \ + .func = test_exrl_ ## insn, \ + .exp_link = (_exp_link(exrl_ ## insn)), \ + }, + +static const struct test tests[] = { + FOR_EACH_INSN(REGISTER_TEST) +}; + +int main(int argc, char **argv) +{ + const struct test *test; + int ret = EXIT_SUCCESS; + bool verbose = false; + long link, magic; + size_t i; + + for (i = 1; i < argc; i++) { + if (strcmp(argv[i], "-v") == 0) { + verbose = true; + } + } + + for (i = 0; i < sizeof(tests) / sizeof(tests[0]); i++) { + test = &tests[i]; + if (verbose) { + fprintf(stderr, "[ RUN ] %s\n", test->name); + } + link = -1; + magic = -1; + test->func(&link, &magic); +#define ASSERT_EQ(expected, actual) do { \ + if (expected != actual) { \ + fprintf(stderr, "%s: " #expected " (0x%lx) != " #actual " (0x%lx)\n", \ + test->name, expected, actual); \ + ret = EXIT_FAILURE; \ + } \ +} while (0) + ASSERT_EQ(test->exp_link, link); + ASSERT_EQ(0x12345678L, magic); +#undef ASSERT_EQ + } + + if (verbose) { + fprintf(stderr, ret == EXIT_SUCCESS ? "[ PASSED ]\n" : + "[ FAILED ]\n"); + } + + return ret; +}