[PULL 03/12] tests/qtest: Use qtest_add_data_func_full()

2024-07-02 Thread Thomas Huth
From: Akihiko Odaki 

A test function may not be executed depending on the test command line
so it is wrong to free data with a test function. Use
qtest_add_data_func_full() to register a function to free data.

Signed-off-by: Akihiko Odaki 
Reviewed-by: Michael S. Tsirkin 
Message-ID: <20240627-san-v2-10-750bb0946...@daynix.com>
Reviewed-by: Thomas Huth 
Signed-off-by: Thomas Huth 
---
 tests/qtest/device-introspect-test.c | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/device-introspect-test.c 
b/tests/qtest/device-introspect-test.c
index 5b0ffe43f5..587da59623 100644
--- a/tests/qtest/device-introspect-test.c
+++ b/tests/qtest/device-introspect-test.c
@@ -266,7 +266,6 @@ static void test_device_intro_concrete(const void *args)
 
 qobject_unref(types);
 qtest_quit(qts);
-g_free((void *)args);
 }
 
 static void test_abstract_interfaces(void)
@@ -310,12 +309,12 @@ static void add_machine_test_case(const char *mname)
 
 path = g_strdup_printf("device/introspect/concrete/defaults/%s", mname);
 args = g_strdup_printf("-M %s", mname);
-qtest_add_data_func(path, args, test_device_intro_concrete);
+qtest_add_data_func_full(path, args, test_device_intro_concrete, g_free);
 g_free(path);
 
 path = g_strdup_printf("device/introspect/concrete/nodefaults/%s", mname);
 args = g_strdup_printf("-nodefaults -M %s", mname);
-qtest_add_data_func(path, args, test_device_intro_concrete);
+qtest_add_data_func_full(path, args, test_device_intro_concrete, g_free);
 g_free(path);
 }
 
@@ -330,7 +329,7 @@ int main(int argc, char **argv)
 qtest_add_func("device/introspect/abstract-interfaces", 
test_abstract_interfaces);
 if (g_test_quick()) {
 qtest_add_data_func("device/introspect/concrete/defaults/none",
-g_strdup(common_args), test_device_intro_concrete);
+common_args, test_device_intro_concrete);
 } else {
 qtest_cb_for_every_machine(add_machine_test_case, true);
 }
-- 
2.45.2




[PULL 05/12] tests/qtest: Free old machine variable name

2024-07-02 Thread Thomas Huth
From: Akihiko Odaki 

This fixes LeakSanitizer warnings.

Signed-off-by: Akihiko Odaki 
Reviewed-by: Peter Maydell 
Reviewed-by: Michael S. Tsirkin 
Message-ID: <20240627-san-v2-12-750bb0946...@daynix.com>
Signed-off-by: Thomas Huth 
---
 tests/qtest/libqtest.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index ca46b1f8d0..1326e34291 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1514,6 +1514,7 @@ static struct MachInfo *qtest_get_machines(const char 
*var)
 int idx;
 
 if (g_strcmp0(qemu_var, var)) {
+g_free(qemu_var);
 qemu_var = g_strdup(var);
 
 /* new qemu, clear the cache */
-- 
2.45.2




[PULL 04/12] tests/qtest: Free unused QMP response

2024-07-02 Thread Thomas Huth
From: Akihiko Odaki 

This fixes LeakSanitizer warnings.

Signed-off-by: Akihiko Odaki 
Reviewed-by: Michael S. Tsirkin 
Message-ID: <20240627-san-v2-11-750bb0946...@daynix.com>
Signed-off-by: Thomas Huth 
---
 tests/qtest/libqtest.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index c7f6897d78..ca46b1f8d0 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -749,6 +749,8 @@ QDict *qtest_qmp_receive(QTestState *s)
 response, s->eventData)) {
 /* Stash the event for a later consumption */
 s->pending_events = g_list_append(s->pending_events, response);
+} else {
+qobject_unref(response);
 }
 }
 }
-- 
2.45.2




[PULL 10/12] tests/avocado: add hotplug_blk test

2024-07-02 Thread Thomas Huth
From: Vladimir Sementsov-Ogievskiy 

Introduce a test, that checks that plug/unplug of virtio-blk device
works.

(the test is developed by copying hotplug_cpu.py, so keep original
copyright)

Signed-off-by: Vladimir Sementsov-Ogievskiy 
Reviewed-by: Thomas Huth 
Message-ID: <20240409065854.366856-1-vsement...@yandex-team.ru>
Signed-off-by: Thomas Huth 
---
 tests/avocado/hotplug_blk.py | 69 
 1 file changed, 69 insertions(+)
 create mode 100644 tests/avocado/hotplug_blk.py

diff --git a/tests/avocado/hotplug_blk.py b/tests/avocado/hotplug_blk.py
new file mode 100644
index 00..5dc30f6616
--- /dev/null
+++ b/tests/avocado/hotplug_blk.py
@@ -0,0 +1,69 @@
+# Functional test that hotplugs a virtio blk disk and checks it on a Linux
+# guest
+#
+# Copyright (c) 2021 Red Hat, Inc.
+# Copyright (c) Yandex
+#
+# 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 time
+
+from avocado_qemu import LinuxTest
+
+
+class HotPlug(LinuxTest):
+def blockdev_add(self) -> None:
+self.vm.cmd('blockdev-add', **{
+'driver': 'null-co',
+'size': 1073741824,
+'node-name': 'disk'
+})
+
+def assert_vda(self) -> None:
+self.ssh_command('test -e /sys/block/vda')
+
+def assert_no_vda(self) -> None:
+with self.assertRaises(AssertionError):
+self.assert_vda()
+
+def plug(self) -> None:
+args = {
+'driver': 'virtio-blk-pci',
+'drive': 'disk',
+'id': 'virtio-disk0',
+'bus': 'pci.1',
+'addr': 1
+}
+
+self.assert_no_vda()
+self.vm.cmd('device_add', args)
+try:
+self.assert_vda()
+except AssertionError:
+time.sleep(1)
+self.assert_vda()
+
+def unplug(self) -> None:
+self.vm.cmd('device_del', id='virtio-disk0')
+
+self.vm.event_wait('DEVICE_DELETED', 1.0,
+   match={'data': {'device': 'virtio-disk0'}})
+
+self.assert_no_vda()
+
+def test(self) -> None:
+"""
+:avocado: tags=arch:x86_64
+:avocado: tags=machine:q35
+:avocado: tags=accel:kvm
+"""
+self.require_accelerator('kvm')
+self.vm.add_args('-accel', 'kvm')
+self.vm.add_args('-device', 'pcie-pci-bridge,id=pci.1,bus=pcie.0')
+
+self.launch_and_wait()
+self.blockdev_add()
+
+self.plug()
+self.unplug()
-- 
2.45.2




[PULL 09/12] hw/s390x: Attach default virtio-net devices to the /machine/virtual-css-bridge

2024-07-02 Thread Thomas Huth
The initial virtio-net-ccw devices currently do not have a proper parent
in the QOM tree, so they show up under /machine/unattached - which is
somewhat ugly. Let's attach them to /machine/virtual-css-bridge/virtual-css
instead.

Message-ID: <20240701200108.154271-1-th...@redhat.com>
Reviewed-by: Cédric Le Goater 
Reviewed-by: Philippe Mathieu-Daudé 
Signed-off-by: Thomas Huth 
---
 hw/s390x/s390-virtio-ccw.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index f87ca36264..c1edbd9131 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -216,8 +216,11 @@ static void s390_init_ipl_dev(const char *kernel_filename,
 static void s390_create_virtio_net(BusState *bus, const char *name)
 {
 DeviceState *dev;
+int cnt = 0;
 
 while ((dev = qemu_create_nic_device(name, true, "virtio"))) {
+g_autofree char *childname = g_strdup_printf("%s[%d]", name, cnt++);
+object_property_add_child(OBJECT(bus), childname, OBJECT(dev));
 qdev_realize_and_unref(dev, bus, _fatal);
 }
 }
-- 
2.45.2




[PULL 11/12] .travis.yml: Install python3-tomli in all build jobs

2024-07-02 Thread Thomas Huth
Since commit 1f97715c83 ('Revert "python: use vendored tomli"')
this package is a hard requirement for compiling QEMU, so install
it now in all Travis jobs, too.

Message-ID: <20240624094807.182313-1-th...@redhat.com>
Acked-by: Alex Bennée 
Signed-off-by: Thomas Huth 
---
 .travis.yml | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index cef0308952..8fc1ae0cf2 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -106,6 +106,7 @@ jobs:
   - libvdeplug-dev
   - libvte-2.91-dev
   - ninja-build
+  - python3-tomli
   # Tests dependencies
   - genisoimage
   env:
@@ -141,6 +142,7 @@ jobs:
   - libvdeplug-dev
   - libvte-2.91-dev
   - ninja-build
+  - python3-tomli
   # Tests dependencies
   - genisoimage
   env:
@@ -175,6 +177,7 @@ jobs:
   - libvdeplug-dev
   - libvte-2.91-dev
   - ninja-build
+  - python3-tomli
   # Tests dependencies
   - genisoimage
   env:
@@ -215,6 +218,7 @@ jobs:
   - libzstd-dev
   - nettle-dev
   - ninja-build
+  - python3-tomli
   # Tests dependencies
   - genisoimage
   env:
@@ -231,6 +235,7 @@ jobs:
   - ninja-build
   - flex
   - bison
+  - python3-tomli
   env:
 - TEST_CMD="make check check-tcg V=1"
 - CONFIG="--disable-containers --disable-system"
@@ -263,6 +268,7 @@ jobs:
   - libvdeplug-dev
   - libvte-2.91-dev
   - ninja-build
+  - python3-tomli
   env:
 - TEST_CMD="make check-unit"
 - CONFIG="--disable-containers --disable-tcg --enable-kvm 
--disable-tools
-- 
2.45.2




[PULL 02/12] tests/qtest/migration-test: enable on s390x with TCG

2024-07-02 Thread Thomas Huth
From: Nicholas Piggin 

s390x with TCG is more stable now. Enable it.

Signed-off-by: Nicholas Piggin 
Message-Id: <20240525131241.378473-3-npig...@gmail.com>
Reviewed-by: Prasad Pandit 
[thuth: Added "with TCG" to the commit message]
Signed-off-by: Thomas Huth 
---
 tests/qtest/migration-test.c | 12 
 1 file changed, 12 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 571fc1334c..70b606b888 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -3823,16 +3823,6 @@ int main(int argc, char **argv)
test_vmstate_checker_script);
 #endif
 
-/*
- * On s390x with TCG, migration is observed to hang due to the 'pending'
- * state of the flic interrupt controller not being migrated or
- * reconstructed post-migration. Disable it until the problem is resolved.
- */
-if (g_str_equal(arch, "s390x") && !has_kvm) {
-g_test_message("Skipping tests: s390x host with KVM is required");
-goto test_add_done;
-}
-
 if (is_x86) {
 migration_test_add("/migration/precopy/unix/suspend/live",
test_precopy_unix_suspend_live);
@@ -4036,8 +4026,6 @@ int main(int argc, char **argv)
test_vcpu_dirty_limit);
 }
 
-test_add_done:
-
 ret = g_test_run();
 
 g_assert_cmpint(ret, ==, 0);
-- 
2.45.2




[PULL 12/12] pc-bios/s390-ccw: Remove duplicated LDFLAGS

2024-07-02 Thread Thomas Huth
The -Wl,-pie and -nostdlib flags are added to LDFLAGS twice. Merge
the two lines to get rid of the duplicates.

Message-ID: <20240621082422.136217-2-th...@redhat.com>
Signed-off-by: Thomas Huth 
---
 pc-bios/s390-ccw/Makefile | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index acfcd1e71a..6207911b53 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -40,7 +40,7 @@ EXTRA_CFLAGS += -ffreestanding 
-fno-delete-null-pointer-checks -fno-common -fPIE
 EXTRA_CFLAGS += -fwrapv -fno-strict-aliasing -fno-asynchronous-unwind-tables
 EXTRA_CFLAGS += -msoft-float
 EXTRA_CFLAGS += -std=gnu99
-LDFLAGS += -Wl,-pie -nostdlib
+LDFLAGS += -Wl,-pie -nostdlib -z noexecstack
 
 cc-test = $(CC) -Werror $1 -c -o /dev/null -xc /dev/null >/dev/null 2>/dev/null
 cc-option = if $(call cc-test, $1); then \
@@ -55,8 +55,6 @@ config-cc.mak: Makefile
$(call cc-option,-march=z900,-march=z10)) 3> config-cc.mak
 -include config-cc.mak
 
-LDFLAGS += -Wl,-pie -nostdlib -z noexecstack
-
 build-all: s390-ccw.img s390-netboot.img
 
 s390-ccw.elf: $(OBJECTS)
-- 
2.45.2




[PULL 07/12] tests/qtest: Free GThread

2024-07-02 Thread Thomas Huth
From: Akihiko Odaki 

These GThreads are never referenced.

Signed-off-by: Akihiko Odaki 
Reviewed-by: Peter Maydell 
Reviewed-by: Michael S. Tsirkin 
Message-ID: <20240627-san-v2-15-750bb0946...@daynix.com>
Signed-off-by: Thomas Huth 
---
 tests/qtest/vhost-user-test.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/qtest/vhost-user-test.c b/tests/qtest/vhost-user-test.c
index d4e437265f..929af5c183 100644
--- a/tests/qtest/vhost-user-test.c
+++ b/tests/qtest/vhost-user-test.c
@@ -928,7 +928,7 @@ static void *vhost_user_test_setup_reconnect(GString 
*cmd_line, void *arg)
 {
 TestServer *s = test_server_new("reconnect", arg);
 
-g_thread_new("connect", connect_thread, s);
+g_thread_unref(g_thread_new("connect", connect_thread, s));
 append_mem_opts(s, cmd_line, 256, TEST_MEMFD_AUTO);
 s->vu_ops->append_opts(s, cmd_line, ",server=on");
 
@@ -965,7 +965,7 @@ static void *vhost_user_test_setup_connect_fail(GString 
*cmd_line, void *arg)
 
 s->test_fail = true;
 
-g_thread_new("connect", connect_thread, s);
+g_thread_unref(g_thread_new("connect", connect_thread, s));
 append_mem_opts(s, cmd_line, 256, TEST_MEMFD_AUTO);
 s->vu_ops->append_opts(s, cmd_line, ",server=on");
 
@@ -980,7 +980,7 @@ static void *vhost_user_test_setup_flags_mismatch(GString 
*cmd_line, void *arg)
 
 s->test_flags = TEST_FLAGS_DISCONNECT;
 
-g_thread_new("connect", connect_thread, s);
+g_thread_unref(g_thread_new("connect", connect_thread, s));
 append_mem_opts(s, cmd_line, 256, TEST_MEMFD_AUTO);
 s->vu_ops->append_opts(s, cmd_line, ",server=on");
 
-- 
2.45.2




[PULL 08/12] docs: add precision about capstone for execlog plugin

2024-07-02 Thread Thomas Huth
From: Alexandre Iooss 

Some people are wondering why they get an empty string as disassembly.
Most of the time, they configured QEMU without Capstone support.
Let's document this behaviour to help users.

Signed-off-by: Alexandre Iooss 
Reviewed-by: Pierrick Bouvier 
Message-ID: <20240620135731.977377-1-erdn...@crans.org>
Signed-off-by: Thomas Huth 
---
 docs/devel/tcg-plugins.rst | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/docs/devel/tcg-plugins.rst b/docs/devel/tcg-plugins.rst
index 9cc09d8c3d..f7d7b9e3a4 100644
--- a/docs/devel/tcg-plugins.rst
+++ b/docs/devel/tcg-plugins.rst
@@ -539,7 +539,9 @@ which will output an execution trace following this 
structure::
   0, 0xd34, 0xf9c8f000, "bl #0x10c8"
   0, 0x10c8, 0xfff96c43, "ldr r3, [r0, #0x44]", load, 0x20e4, RAM
 
-the output can be filtered to only track certain instructions or
+Please note that you need to configure QEMU with Capstone support to get 
disassembly.
+
+The output can be filtered to only track certain instructions or
 addresses using the ``ifilter`` or ``afilter`` options. You can stack the
 arguments if required::
 
-- 
2.45.2




[PULL 06/12] tests/qtest: Free paths

2024-07-02 Thread Thomas Huth
From: Akihiko Odaki 

This fixes LeakSanitizer warnings.

Signed-off-by: Akihiko Odaki 
Reviewed-by: Michael S. Tsirkin 
Message-ID: <20240627-san-v2-14-750bb0946...@daynix.com>
Signed-off-by: Thomas Huth 
---
 tests/qtest/qos-test.c | 16 
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/tests/qtest/qos-test.c b/tests/qtest/qos-test.c
index 5da4091ec3..114f6bef27 100644
--- a/tests/qtest/qos-test.c
+++ b/tests/qtest/qos-test.c
@@ -33,7 +33,6 @@
 static char *old_path;
 
 
-
 /**
  * qos_set_machines_devices_available(): sets availability of qgraph
  * machines and devices.
@@ -191,6 +190,12 @@ static void subprocess_run_one_test(const void *arg)
 g_test_trap_assert_passed();
 }
 
+static void destroy_pathv(void *arg)
+{
+g_free(((char **)arg)[0]);
+g_free(arg);
+}
+
 /*
  * in this function, 2 path will be built:
  * path_str, a one-string path (ex "pc/i440FX-pcihost/...")
@@ -295,10 +300,13 @@ static void walk_path(QOSGraphNode *orig_path, int len)
 if (path->u.test.subprocess) {
 gchar *subprocess_path = g_strdup_printf("/%s/%s/subprocess",
  qtest_get_arch(), path_str);
-qtest_add_data_func(path_str, subprocess_path, 
subprocess_run_one_test);
-g_test_add_data_func(subprocess_path, path_vec, run_one_test);
+qtest_add_data_func_full(path_str, subprocess_path,
+ subprocess_run_one_test, g_free);
+g_test_add_data_func_full(subprocess_path, path_vec,
+  run_one_test, destroy_pathv);
 } else {
-qtest_add_data_func(path_str, path_vec, run_one_test);
+qtest_add_data_func_full(path_str, path_vec,
+ run_one_test, destroy_pathv);
 }
 
 g_free(path_str);
-- 
2.45.2




[PULL 01/12] hw/intc/s390_flic: Fix interrupt controller migration on s390x with TCG

2024-07-02 Thread Thomas Huth
Migration of a s390x guest with TCG was long known to be very unstable,
so the tests in tests/qtest/migration-test.c are disabled if running
with TCG instead of KVM.

Nicholas Piggin did a great analysis of the problem:

"The flic pending state is not migrated, so if the machine is migrated
 while an interrupt is pending, it can be lost. This shows up in
 qtest migration test, an extint is pending (due to console writes?)
 and the CPU waits via s390_cpu_set_psw and expects the interrupt to
 wake it. However when the flic pending state is lost, s390_cpu_has_int
 returns false, so s390_cpu_exec_interrupt falls through to halting
 again."

Thus let's finally migrate the pending state, and to be on the safe
side, also the other state variables of the QEMUS390FLICState structure.

Message-ID: <20240619144421.261342-1-th...@redhat.com>
Signed-off-by: Thomas Huth 
---
 include/hw/s390x/s390_flic.h |  1 +
 hw/intc/s390_flic.c  | 75 ++--
 hw/s390x/s390-virtio-ccw.c   |  5 +++
 3 files changed, 78 insertions(+), 3 deletions(-)

diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h
index 382d9833f1..4d66c5e42e 100644
--- a/include/hw/s390x/s390_flic.h
+++ b/include/hw/s390x/s390_flic.h
@@ -116,6 +116,7 @@ struct QEMUS390FLICState {
 uint8_t simm;
 uint8_t nimm;
 QLIST_HEAD(, QEMUS390FlicIO) io[8];
+bool migrate_all_state;
 };
 
 uint32_t qemu_s390_flic_dequeue_service(QEMUS390FLICState *flic);
diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
index 6771645699..a91a4a47e8 100644
--- a/hw/intc/s390_flic.c
+++ b/hw/intc/s390_flic.c
@@ -361,15 +361,77 @@ bool ais_needed(void *opaque)
 return s->ais_supported;
 }
 
+static bool ais_needed_v(void *opaque, int version_id)
+{
+return ais_needed(opaque);
+}
+
+static bool qemu_s390_flic_full_state_needed(void *opaque)
+{
+QEMUS390FLICState *s = opaque;
+
+return s->migrate_all_state;
+}
+
+static bool qemu_s390_flic_state_needed(void *opaque)
+{
+return ais_needed(opaque) || qemu_s390_flic_full_state_needed(opaque);
+}
+
+static const VMStateDescription vmstate_qemu_s390_flic_io = {
+ .name = "qemu-s390-flic-io",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (const VMStateField[]) {
+ VMSTATE_UINT16(id, QEMUS390FlicIO),
+ VMSTATE_UINT16(nr, QEMUS390FlicIO),
+ VMSTATE_UINT32(parm, QEMUS390FlicIO),
+ VMSTATE_UINT32(word, QEMUS390FlicIO),
+ VMSTATE_END_OF_LIST()
+ },
+};
+
+static const VMStateDescription vmstate_qemu_s390_flic_full = {
+.name = "qemu-s390-flic-full",
+.version_id = 1,
+.minimum_version_id = 1,
+.needed = qemu_s390_flic_full_state_needed,
+.fields = (const VMStateField[]) {
+VMSTATE_UINT32(pending, QEMUS390FLICState),
+VMSTATE_UINT32(service_param, QEMUS390FLICState),
+VMSTATE_QLIST_V(io[0], QEMUS390FLICState, 1,
+vmstate_qemu_s390_flic_io, QEMUS390FlicIO, next),
+VMSTATE_QLIST_V(io[1], QEMUS390FLICState, 1,
+vmstate_qemu_s390_flic_io, QEMUS390FlicIO, next),
+VMSTATE_QLIST_V(io[2], QEMUS390FLICState, 1,
+vmstate_qemu_s390_flic_io, QEMUS390FlicIO, next),
+VMSTATE_QLIST_V(io[3], QEMUS390FLICState, 1,
+vmstate_qemu_s390_flic_io, QEMUS390FlicIO, next),
+VMSTATE_QLIST_V(io[4], QEMUS390FLICState, 1,
+vmstate_qemu_s390_flic_io, QEMUS390FlicIO, next),
+VMSTATE_QLIST_V(io[5], QEMUS390FLICState, 1,
+vmstate_qemu_s390_flic_io, QEMUS390FlicIO, next),
+VMSTATE_QLIST_V(io[6], QEMUS390FLICState, 1,
+vmstate_qemu_s390_flic_io, QEMUS390FlicIO, next),
+VMSTATE_QLIST_V(io[7], QEMUS390FLICState, 1,
+vmstate_qemu_s390_flic_io, QEMUS390FlicIO, next),
+VMSTATE_END_OF_LIST()
+}
+};
+
 static const VMStateDescription qemu_s390_flic_vmstate = {
 .name = "qemu-s390-flic",
 .version_id = 1,
 .minimum_version_id = 1,
-.needed = ais_needed,
+.needed = qemu_s390_flic_state_needed,
 .fields = (const VMStateField[]) {
-VMSTATE_UINT8(simm, QEMUS390FLICState),
-VMSTATE_UINT8(nimm, QEMUS390FLICState),
+VMSTATE_UINT8_TEST(simm, QEMUS390FLICState, ais_needed_v),
+VMSTATE_UINT8_TEST(nimm, QEMUS390FLICState, ais_needed_v),
 VMSTATE_END_OF_LIST()
+},
+.subsections = (const VMStateDescription * const []) {
+_qemu_s390_flic_full,
+NULL
 }
 };
 
@@ -383,11 +445,18 @@ static void qemu_s390_flic_instance_init(Object *obj)
 }
 }
 
+static Property qemu_s390_flic_properties[] = {
+DEFINE_PROP_BOOL("migrate-all-state", QEMUS390FLICState,
+ migrate_all_state, true),
+DEFINE_PROP_END_OF_LIST(),
+};
+
 static void qemu_s390_flic_class_init(ObjectCl

[PULL 00/12] qtest, s390x, avocado and doc patches

2024-07-02 Thread Thomas Huth
 Hi Richard!

The following changes since commit c80a339587fe4148292c260716482dd2f86d4476:

  Merge tag 'pull-target-arm-20240701' of 
https://git.linaro.org/people/pmaydell/qemu-arm into staging (2024-07-01 
10:41:45 -0700)

are available in the Git repository at:

  https://gitlab.com/thuth/qemu.git tags/pull-request-2024-07-02

for you to fetch changes up to e3e2708fee10e6df413c36a71b100c59710e727e:

  pc-bios/s390-ccw: Remove duplicated LDFLAGS (2024-07-02 09:52:38 +0200)


* Fix interrupt controller migration on s390x with TCG and enable qtest
* Fix memory leaks in qtests
* Use a proper qom-tree parent for s390x virtio-net devices
* Add hotplug avocado test for virtio-blk
* Fix Travis jobs (need python3-tomli now)


Akihiko Odaki (5):
  tests/qtest: Use qtest_add_data_func_full()
  tests/qtest: Free unused QMP response
  tests/qtest: Free old machine variable name
  tests/qtest: Free paths
  tests/qtest: Free GThread

Alexandre Iooss (1):
  docs: add precision about capstone for execlog plugin

Nicholas Piggin (1):
  tests/qtest/migration-test: enable on s390x with TCG

Thomas Huth (4):
  hw/intc/s390_flic: Fix interrupt controller migration on s390x with TCG
  hw/s390x: Attach default virtio-net devices to the 
/machine/virtual-css-bridge
  .travis.yml: Install python3-tomli in all build jobs
  pc-bios/s390-ccw: Remove duplicated LDFLAGS

Vladimir Sementsov-Ogievskiy (1):
  tests/avocado: add hotplug_blk test

 docs/devel/tcg-plugins.rst   |  4 +-
 include/hw/s390x/s390_flic.h |  1 +
 hw/intc/s390_flic.c  | 75 ++--
 hw/s390x/s390-virtio-ccw.c   |  8 
 tests/qtest/device-introspect-test.c |  7 ++--
 tests/qtest/libqtest.c   |  3 ++
 tests/qtest/migration-test.c | 12 --
 tests/qtest/qos-test.c   | 16 ++--
 tests/qtest/vhost-user-test.c|  6 +--
 .travis.yml  |  6 +++
 pc-bios/s390-ccw/Makefile|  4 +-
 tests/avocado/hotplug_blk.py | 69 +
 12 files changed, 181 insertions(+), 30 deletions(-)
 create mode 100644 tests/avocado/hotplug_blk.py




Re: [PATCH v2 13/15] tests/qtest: Delete previous boot file

2024-07-02 Thread Thomas Huth

On 27/06/2024 15.37, Akihiko Odaki wrote:

A test run may create boot files several times. Delete the previous boot
file before creating a new one.

Signed-off-by: Akihiko Odaki 
---
  tests/qtest/migration-test.c | 18 +++---
  1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index b7e3406471a6..5c0d669b6df3 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -129,12 +129,23 @@ static char *bootpath;
  #include "tests/migration/aarch64/a-b-kernel.h"
  #include "tests/migration/s390x/a-b-bios.h"
  
+static void bootfile_delete(void)

+{
+unlink(bootpath);
+g_free(bootpath);
+bootpath = NULL;
+}
+
  static void bootfile_create(char *dir, bool suspend_me)
  {
  const char *arch = qtest_get_arch();
  unsigned char *content;
  size_t len;
  
+if (bootpath) {

+bootfile_delete();
+}
+
  bootpath = g_strdup_printf("%s/bootsect", dir);
  if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
  /* the assembled x86 boot sector should be exactly one sector large */
@@ -164,13 +175,6 @@ static void bootfile_create(char *dir, bool suspend_me)
  fclose(bootfile);
  }
  
-static void bootfile_delete(void)

-{
-unlink(bootpath);
-g_free(bootpath);
-bootpath = NULL;
-}
-
  /*
   * Wait for some output in the serial output file,
   * we get an 'A' followed by an endless string of 'B's



I think the better fix would be to call bootfile_create() only once from 
main() since we don't have to create the bootfile multiple times, do we?


 Thomas




Re: [PATCH v2 00/15] Fix check-qtest-ppc64 sanitizer errors

2024-07-02 Thread Thomas Huth

On 02/07/2024 00.23, BALATON Zoltan wrote:

On Mon, 1 Jul 2024, Michael S. Tsirkin wrote:

On Thu, Jun 27, 2024 at 10:37:43PM +0900, Akihiko Odaki wrote:
Based-on: 
<3ad18bc590ef28e1526e8053568086b453e7ffde.1718211878.git.quic_mathb...@quicinc.com>

("[PATCH] cpu: fix memleak of 'halt_cond' and 'thread'")

I saw various sanitizer errors when running check-qtest-ppc64. While
I could just turn off sanitizers, I decided to tackle them this time.

Unfortunately, GLib does not free test data in some cases so some
sanitizer errors remain. All sanitizer errors will be gone with this
patch series combined with the following change for GLib:
https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4120

Signed-off-by: Akihiko Odaki 



Reviewed-by: Michael S. Tsirkin 

who's merging all this?


Maybe needs to be split. Mark had an alternative for macio which was picked 
up by Philippe if I'm not mistaken. I've sent an alternative for vt82c686 
which is still discussed but could belong to Philippe as well. PPC parts 
could be taken by the PPC maintainers if there were any active at the moment 
and I don't know who maintains tests normally or other misc areas.


I can take the qtest patches through my tree.

 Thomas





Re: [PATCH v2 10/15] tests/qtest: Use qtest_add_data_func_full()

2024-07-02 Thread Thomas Huth

On 27/06/2024 15.37, Akihiko Odaki wrote:

A test function may not be executed depending on the test command line
so it is wrong to free data with a test function. Use
qtest_add_data_func_full() to register a function to free data.

Signed-off-by: Akihiko Odaki 
---
  tests/qtest/device-introspect-test.c | 7 +++
  1 file changed, 3 insertions(+), 4 deletions(-)


Reviewed-by: Thomas Huth 




[PATCH] hw/s390x: Attach default virtio-net devices to the /machine/virtual-css-bridge

2024-07-01 Thread Thomas Huth
The initial virtio-net-ccw devices currently do not have a proper parent
in the QOM tree, so they show up under /machine/unattached - which is
somewhat ugly. Let's attach them to /machine/virtual-css-bridge/virtual-css
instead.

Signed-off-by: Thomas Huth 
---
 hw/s390x/s390-virtio-ccw.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index cd063f8b64..0d58e5ab75 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -216,8 +216,11 @@ static void s390_init_ipl_dev(const char *kernel_filename,
 static void s390_create_virtio_net(BusState *bus, const char *name)
 {
 DeviceState *dev;
+int cnt = 0;
 
 while ((dev = qemu_create_nic_device(name, true, "virtio"))) {
+g_autofree char *childname = g_strdup_printf("%s[%d]", name, cnt++);
+object_property_add_child(OBJECT(bus), childname, OBJECT(dev));
 qdev_realize_and_unref(dev, bus, _fatal);
 }
 }
-- 
2.45.2




Re: [PATCH] hw/virtio: Fix the de-initialization of vhost-user devices

2024-07-01 Thread Thomas Huth

On 01/07/2024 17.06, Michael S. Tsirkin wrote:

On Mon, Jul 01, 2024 at 04:07:56PM +0200, Thomas Huth wrote:

On 18/06/2024 14.19, Thomas Huth wrote:

The unrealize functions of the various vhost-user devices are
calling the corresponding vhost_*_set_status() functions with a
status of 0 to shut down the device correctly.

Now these vhost_*_set_status() functions all follow this scheme:

  bool should_start = virtio_device_should_start(vdev, status);

  if (vhost_dev_is_started(>vhost_dev) == should_start) {
  return;
  }

  if (should_start) {
  /* ... do the initialization stuff ... */
  } else {
  /* ... do the cleanup stuff ... */
  }

The problem here is virtio_device_should_start(vdev, 0) currently
always returns "true" since it internally only looks at vdev->started
instead of looking at the "status" parameter. Thus once the device
got started once, virtio_device_should_start() always returns true
and thus the vhost_*_set_status() functions return early, without
ever doing any clean-up when being called with status == 0. This
causes e.g. problems when trying to hot-plug and hot-unplug a vhost
user devices multiple times since the de-initialization step is
completely skipped during the unplug operation.

This bug has been introduced in commit 9f6bcfd99f ("hw/virtio: move
vm_running check to virtio_device_started") which replaced

   should_start = status & VIRTIO_CONFIG_S_DRIVER_OK;

with

   should_start = virtio_device_started(vdev, status);

which later got replaced by virtio_device_should_start(). This blocked
the possibility to set should_start to false in case the status flag
VIRTIO_CONFIG_S_DRIVER_OK was not set.

Fix it by adjusting the virtio_device_should_start() function to
only consider the status flag instead of vdev->started. Since this
function is only used in the various vhost_*_set_status() functions
for exactly the same purpose, it should be fine to fix it in this
central place there without any risk to change the behavior of other
code.

Fixes: 9f6bcfd99f ("hw/virtio: move vm_running check to virtio_device_started")
Buglink: https://issues.redhat.com/browse/RHEL-40708
Signed-off-by: Thomas Huth 
---
   include/hw/virtio/virtio.h | 8 
   1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 7d5ffdc145..2eafad17b8 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -470,9 +470,9 @@ static inline bool virtio_device_started(VirtIODevice 
*vdev, uint8_t status)
* @vdev - the VirtIO device
* @status - the devices status bits
*
- * This is similar to virtio_device_started() but also encapsulates a
- * check on the VM status which would prevent a device starting
- * anyway.
+ * This is similar to virtio_device_started() but ignores vdev->started
+ * and also encapsulates a check on the VM status which would prevent a
+ * device from starting anyway.
*/
   static inline bool virtio_device_should_start(VirtIODevice *vdev, uint8_t 
status)
   {
@@ -480,7 +480,7 @@ static inline bool virtio_device_should_start(VirtIODevice 
*vdev, uint8_t status
   return false;
   }
-return virtio_device_started(vdev, status);
+return status & VIRTIO_CONFIG_S_DRIVER_OK;
   }


Michael, any concerns or comments about this patch?

If not, I could also take it via my s390x tree since this fixes vhost-ccw
devices on s390x.

  Thomas


I'm working on a pull request with this today.
I can drop it if you prefer ...


Ah, perfect, please include it in your PR then!

 Thanks,
  Thomas




Re: [PATCH] hw/virtio: Fix the de-initialization of vhost-user devices

2024-07-01 Thread Thomas Huth

On 18/06/2024 14.19, Thomas Huth wrote:

The unrealize functions of the various vhost-user devices are
calling the corresponding vhost_*_set_status() functions with a
status of 0 to shut down the device correctly.

Now these vhost_*_set_status() functions all follow this scheme:

 bool should_start = virtio_device_should_start(vdev, status);

 if (vhost_dev_is_started(>vhost_dev) == should_start) {
 return;
 }

 if (should_start) {
 /* ... do the initialization stuff ... */
 } else {
 /* ... do the cleanup stuff ... */
 }

The problem here is virtio_device_should_start(vdev, 0) currently
always returns "true" since it internally only looks at vdev->started
instead of looking at the "status" parameter. Thus once the device
got started once, virtio_device_should_start() always returns true
and thus the vhost_*_set_status() functions return early, without
ever doing any clean-up when being called with status == 0. This
causes e.g. problems when trying to hot-plug and hot-unplug a vhost
user devices multiple times since the de-initialization step is
completely skipped during the unplug operation.

This bug has been introduced in commit 9f6bcfd99f ("hw/virtio: move
vm_running check to virtio_device_started") which replaced

  should_start = status & VIRTIO_CONFIG_S_DRIVER_OK;

with

  should_start = virtio_device_started(vdev, status);

which later got replaced by virtio_device_should_start(). This blocked
the possibility to set should_start to false in case the status flag
VIRTIO_CONFIG_S_DRIVER_OK was not set.

Fix it by adjusting the virtio_device_should_start() function to
only consider the status flag instead of vdev->started. Since this
function is only used in the various vhost_*_set_status() functions
for exactly the same purpose, it should be fine to fix it in this
central place there without any risk to change the behavior of other
code.

Fixes: 9f6bcfd99f ("hw/virtio: move vm_running check to virtio_device_started")
Buglink: https://issues.redhat.com/browse/RHEL-40708
Signed-off-by: Thomas Huth 
---
  include/hw/virtio/virtio.h | 8 
  1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 7d5ffdc145..2eafad17b8 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -470,9 +470,9 @@ static inline bool virtio_device_started(VirtIODevice 
*vdev, uint8_t status)
   * @vdev - the VirtIO device
   * @status - the devices status bits
   *
- * This is similar to virtio_device_started() but also encapsulates a
- * check on the VM status which would prevent a device starting
- * anyway.
+ * This is similar to virtio_device_started() but ignores vdev->started
+ * and also encapsulates a check on the VM status which would prevent a
+ * device from starting anyway.
   */
  static inline bool virtio_device_should_start(VirtIODevice *vdev, uint8_t 
status)
  {
@@ -480,7 +480,7 @@ static inline bool virtio_device_should_start(VirtIODevice 
*vdev, uint8_t status
  return false;
  }
  
-return virtio_device_started(vdev, status);

+return status & VIRTIO_CONFIG_S_DRIVER_OK;
  }


Michael, any concerns or comments about this patch?

If not, I could also take it via my s390x tree since this fixes vhost-ccw 
devices on s390x.


 Thomas





[PATCH] Remove inclusion of hw/hw.h from files that don't need it

2024-07-01 Thread Thomas Huth
hw/hw.h only contains the prototype of hw_error() nowadays, so
files that don't use this function don't need to include this
header.

Signed-off-by: Thomas Huth 
---
 include/hw/misc/xlnx-cfi-if.h | 1 -
 hw/misc/edu.c | 1 -
 hw/vfio/container.c   | 1 -
 3 files changed, 3 deletions(-)

diff --git a/include/hw/misc/xlnx-cfi-if.h b/include/hw/misc/xlnx-cfi-if.h
index f9bd12292d..5010401517 100644
--- a/include/hw/misc/xlnx-cfi-if.h
+++ b/include/hw/misc/xlnx-cfi-if.h
@@ -11,7 +11,6 @@
 #define XLNX_CFI_IF_H 1
 
 #include "qemu/help-texts.h"
-#include "hw/hw.h"
 #include "qom/object.h"
 
 #define TYPE_XLNX_CFI_IF "xlnx-cfi-if"
diff --git a/hw/misc/edu.c b/hw/misc/edu.c
index fa052c44db..504178b4a2 100644
--- a/hw/misc/edu.c
+++ b/hw/misc/edu.c
@@ -26,7 +26,6 @@
 #include "qemu/log.h"
 #include "qemu/units.h"
 #include "hw/pci/pci.h"
-#include "hw/hw.h"
 #include "hw/pci/msi.h"
 #include "qemu/timer.h"
 #include "qom/object.h"
diff --git a/hw/vfio/container.c b/hw/vfio/container.c
index 2e7ecdf10e..88ede913d6 100644
--- a/hw/vfio/container.c
+++ b/hw/vfio/container.c
@@ -26,7 +26,6 @@
 #include "exec/address-spaces.h"
 #include "exec/memory.h"
 #include "exec/ram_addr.h"
-#include "hw/hw.h"
 #include "qemu/error-report.h"
 #include "qemu/range.h"
 #include "sysemu/reset.h"
-- 
2.45.2




Re: [PATCH v2 07/14] hw/i386: convert 'q35' machine definitions to use new macros

2024-07-01 Thread Thomas Huth

On 20/06/2024 18.57, Daniel P. Berrangé wrote:

This changes the DEFINE_Q35_MACHINE macro to use the common
helpers for constructing versioned symbol names and strings,
bringing greater consistency across targets.

The added benefit is that it avoids the need to repeat the
version number thrice in three different formats in the calls
to DEFINE_Q35_MACHINE.

Due to the odd-ball '4.0.1' machine type version, this
commit introduces a DEFINE_Q35_BUGFIX helper, to allow
defining of "bugfix" machine types which have a three
digit version.

Signed-off-by: Daniel P. Berrangé 
---
  hw/i386/pc_q35.c | 215 ---
  1 file changed, 90 insertions(+), 125 deletions(-)


Reviewed-by: Thomas Huth 





Re: [PATCH v2 06/14] hw/i386: convert 'i440fx' machine definitions to use new macros

2024-07-01 Thread Thomas Huth

On 20/06/2024 18.57, Daniel P. Berrangé wrote:

This changes the DEFINE_I440FX_MACHINE macro to use the common
helpers for constructing versioned symbol names and strings,
bringing greater consistency across targets.

The added benefit is that it avoids the need to repeat the
version number thrice in three different formats in the calls
to DEFINE_I440FX_MACHINE.

Signed-off-by: Daniel P. Berrangé 
---
  hw/i386/pc_piix.c| 219 +++
  include/hw/i386/pc.h |  26 +
  2 files changed, 122 insertions(+), 123 deletions(-)



Reviewed-by: Thomas Huth 




Re: [PATCH 0/7] pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img

2024-07-01 Thread Thomas Huth

On 28/06/2024 20.01, Jared Rossi wrote:



On 6/24/24 1:55 AM, Thomas Huth wrote:

[...]

I think it should be fine, both functions are basically just a wrapper 
around the write() function in sclp.c, with sclp_print() being rather dumb 
while printf() is doing the usual string formatting before writing it out. 
I think in the long run, it would be nice to get rid of sclp_print() and 
replace it by puts() or printf() in the whole code, but doing that right 
now would likely cause quite some conflicts for Jared with his patch 
series, so I'd rather postpone that to a later point in time.


Hi Thomas,

Converting the panics to returns will require me to modify/move some of the 
sclp_print() calls.  Shall I go ahead and change them to printf() and puts() 
while I'm at it, or would you rather preserve the sclp_print() for now and 
then have a dedicated patch for the all replacements later?  I'm not sure if 
we want to try to maintain some amount of consistency until we do a total 
conversion, or if you are OK with a mix of sclp_print() and printf() 
throughout in the meantime.


 Hi,

I'm ok if we mix them for a while, so I'd say go ahead and use printf or 
puts for the new code.


 Thomas





Re: [PATCH v3 15/17] tests/qtest: Disable npcm7xx_sdhci tests using hardcoded RCA

2024-06-27 Thread Thomas Huth

On 27/06/2024 18.22, Philippe Mathieu-Daudé wrote:

Disable tests using 0x4567 hardcoded RCA otherwise when
using random RCA we get:

   ERROR:../../tests/qtest/npcm7xx_sdhci-test.c:69:write_sdread: assertion 
failed: (ret == len)
   not ok /arm/npcm7xx_sdhci/read_sd - 
ERROR:../../tests/qtest/npcm7xx_sdhci-test.c:69:write_sdread: assertion failed: 
(ret == len)
   Bail out!

See 
https://lore.kernel.org/qemu-devel/37f83be9-deb5-42a1-b704-14984351d...@linaro.org/

Signed-off-by: Philippe Mathieu-Daudé 
---
Cc: Hao Wu 
Cc: Shengtan Mao 
Cc: Tyrone Ting 
---
  tests/qtest/npcm7xx_sdhci-test.c | 7 +++
  1 file changed, 7 insertions(+)

diff --git a/tests/qtest/npcm7xx_sdhci-test.c b/tests/qtest/npcm7xx_sdhci-test.c
index 5d68540e52..6a42b142ad 100644
--- a/tests/qtest/npcm7xx_sdhci-test.c
+++ b/tests/qtest/npcm7xx_sdhci-test.c
@@ -44,6 +44,7 @@ static QTestState *setup_sd_card(void)
  sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x4120, 0, (41 << 8));
  sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_ALL_SEND_CID);
  sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0, 0, SDHC_SEND_RELATIVE_ADDR);
+g_test_skip("hardcoded 0x4567 card address");


This g_test_skip here does not make too much sense (since you're doing it in 
the caller site, too) ... could you please replace it with a proper comment 
why this code needs to be reworked? Thanks!


 Thomas



  sdhci_cmd_regs(qts, NPCM7XX_MMC_BA, 0, 0, 0x4567, 0,
 SDHC_SELECT_DESELECT_CARD);
  
@@ -76,6 +77,9 @@ static void test_read_sd(void)

  {
  QTestState *qts = setup_sd_card();
  
+g_test_skip("hardcoded 0x4567 card address used in setup_sd_card()");

+return;
+
  write_sdread(qts, "hello world");
  write_sdread(qts, "goodbye");
  
@@ -108,6 +112,9 @@ static void test_write_sd(void)

  {
  QTestState *qts = setup_sd_card();
  
+g_test_skip("hardcoded 0x4567 card address used in setup_sd_card()");

+return;
+
  sdwrite_read(qts, "hello world");
  sdwrite_read(qts, "goodbye");
  





[PATCH] docs/system/devices/usb: Replace the non-existing "qemu" binary

2024-06-26 Thread Thomas Huth
We don't ship a binary that is simply called "qemu", so we should
avoid this in the documentation. Use the configurable binary name
via "|qemu_system|" instead.

Signed-off-by: Thomas Huth 
---
 docs/system/devices/usb.rst | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/system/devices/usb.rst b/docs/system/devices/usb.rst
index a6ca7b0c37..dc694d23c2 100644
--- a/docs/system/devices/usb.rst
+++ b/docs/system/devices/usb.rst
@@ -18,7 +18,7 @@ emulation uses less resources (especially CPU).  So if your 
guest
 supports XHCI (which should be the case for any operating system
 released around 2010 or later) we recommend using it:
 
-qemu -device qemu-xhci
+|qemu_system| -device qemu-xhci
 
 XHCI supports USB 1.1, USB 2.0 and USB 3.0 devices, so this is the
 only controller you need.  With only a single USB controller (and
-- 
2.45.2




Re: [RFC PATCH v3 2/2] tests/qtest: QTest example for RISC-V CSR register

2024-06-26 Thread Thomas Huth

On 25/06/2024 17.35, Ivan Klokov wrote:

Added demo for reading CSR register from qtest environment.

Signed-off-by: Ivan Klokov 
---
  tests/qtest/meson.build  |  2 +
  tests/qtest/riscv-csr-test.c | 85 
  2 files changed, 87 insertions(+)
  create mode 100644 tests/qtest/riscv-csr-test.c

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 12792948ff..45d651da99 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -259,6 +259,8 @@ qtests_s390x = \
  qtests_riscv32 = \
(config_all_devices.has_key('CONFIG_SIFIVE_E_AON') ? 
['sifive-e-aon-watchdog-test'] : [])
  
+qtests_riscv32 += ['riscv-csr-test']

+
  qos_test_ss = ss.source_set()
  qos_test_ss.add(
'ac97-test.c',
diff --git a/tests/qtest/riscv-csr-test.c b/tests/qtest/riscv-csr-test.c
new file mode 100644
index 00..21a3646ae9
--- /dev/null
+++ b/tests/qtest/riscv-csr-test.c
@@ -0,0 +1,85 @@
+/*
+ * QTest testcase for RISC-V CSRs
+ *
+ * Copyright (c) 2024 Syntacore.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest-single.h"
+#include "libqtest.h"
+
+static uint64_t qcsr_call(QTestState *qts, const char *name, uint64_t cpu,
+   int csrno, uint64_t *val)
+{
+uint64_t res = 0;
+
+res = qtest_csr_call(qts, name, cpu, csrno, val);
+
+return res;
+}


This wrapper function looks completely unnecessary, just call 
qtest_csr_call() everywhere directly instead?


 Thomas


+static int qcsr_get_csr(QTestState *qts, uint64_t cpu,
+int csrno, uint64_t *val)
+{
+int res;
+
+res = qcsr_call(qts, "get_csr", cpu, csrno, val);
+
+return res;
+}
+
+static int qcsr_set_csr(QTestState *qts, uint64_t cpu,
+int csrno, uint64_t *val)
+{
+int res;
+
+res = qcsr_call(qts, "set_csr", cpu, csrno, val);
+
+return res;
+}
+
+static void run_test_csr(void)
+{
+
+uint64_t res;
+uint64_t val = 0;
+
+res = qcsr_get_csr(global_qtest, 0, 0xf11, );
+
+g_assert_cmpint(res, ==, 0);
+g_assert_cmpint(val, ==, 0x100);
+
+val = 0xff;
+res = qcsr_set_csr(global_qtest, 0, 0x342, );
+g_assert_cmpint(res, ==, 0);
+
+val = 0;
+res = qcsr_get_csr(global_qtest, 0, 0x342, );
+
+g_assert_cmpint(res, ==, 0);
+g_assert_cmpint(val, ==, 0xff);
+
+qtest_quit(global_qtest);
+}
+
+int main(int argc, char **argv)
+{
+g_test_init(, , NULL);
+
+qtest_add_func("/cpu/csr", run_test_csr);
+
+qtest_start("-machine virt -cpu any,mvendorid=0x100");
+
+return g_test_run();
+
+}





Re: [RFC PATCH v3 1/2] target/riscv: Add RISC-V CSR qtest support

2024-06-26 Thread Thomas Huth

On 25/06/2024 17.35, Ivan Klokov wrote:

The RISC-V architecture supports the creation of custom
CSR-mapped devices. It would be convenient to test them in the same way
as MMIO-mapped devices. To do this, a new call has been added
to read/write CSR registers.

Signed-off-by: Ivan Klokov 
---
  target/riscv/cpu.c | 14 +++
  target/riscv/cpu.h |  3 +++
  target/riscv/csr.c | 53 +-
  tests/qtest/libqtest.c | 27 +
  tests/qtest/libqtest.h | 14 +++
  5 files changed, 110 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 69a08e8c2c..55cc01bfb3 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -1148,7 +1148,17 @@ void riscv_cpu_finalize_features(RISCVCPU *cpu, Error 
**errp)
  }
  }
  }
+#ifndef CONFIG_USER_ONLY
+static void riscv_cpu_register_csr_qtest_callback(void)
+{
+static gsize reinit_done;
+if (g_once_init_enter(_done)) {
+qtest_set_command_cb(csr_qtest_callback);
  
+g_once_init_leave(_done, 1);

+}
+}
+#endif
  static void riscv_cpu_realize(DeviceState *dev, Error **errp)


Could you please add an empty line before the #ifndef and after the #endif 
line? Thanks!




diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index c7f6897d78..f8c3ff15a9 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -1205,6 +1205,33 @@ uint64_t qtest_rtas_call(QTestState *s, const char *name,
  return 0;
  }
  
+static void qtest_rsp_csr(QTestState *s, uint64_t *val)

+{
+gchar **args;
+uint64_t ret;
+int rc;
+
+args = qtest_rsp_args(s, 3);
+
+rc = qemu_strtou64(args[1], NULL, 16, );
+g_assert(rc == 0);
+rc = qemu_strtou64(args[2], NULL, 16, val);
+g_assert(rc == 0);
+
+g_strfreev(args);
+}
+
+uint64_t qtest_csr_call(QTestState *s, const char *name,
+ uint64_t cpu, int csr,
+ uint64_t *val)
+{
+qtest_sendf(s, "csr %s 0x%"PRIx64" %d 0x%"PRIx64"\n",
+name, cpu, csr, *val);
+
+qtest_rsp_csr(s, val);
+return 0;
+}
+
  void qtest_add_func(const char *str, void (*fn)(void))
  {
  gchar *path = g_strdup_printf("/%s/%s", qtest_get_arch(), str);
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index c261b7e0b3..53cd8fe9f0 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -577,6 +577,20 @@ uint64_t qtest_rtas_call(QTestState *s, const char *name,
   uint32_t nargs, uint64_t args,
   uint32_t nret, uint64_t ret);
  
+/**

+ * qtest_csr_call:
+ * @s: #QTestState instance to operate on.
+ * @name: name of the command to call.
+ * @cpu: hart number.
+ * @csr: CSR number.
+ * @val: Value for reading/writing.
+ *
+ * Call an CSR function


Maybe mention RISC-V here in the comment?


+ */
+uint64_t qtest_csr_call(QTestState *s, const char *name,
+ uint64_t cpu, int csr,
+ unsigned long *val);
+
  /**
   * qtest_bufread:
   * @s: #QTestState instance to operate on.


For the tests/qtest part:
Acked-by: Thomas Huth 




Re: [PATCH] tests/avocado: add hotplug_blk test

2024-06-26 Thread Thomas Huth

On 09/04/2024 08.58, Vladimir Sementsov-Ogievskiy wrote:

Introduce a test, that checks that plug/unplug of virtio-blk device
works.

(the test is developed by copying hotplug_cpu.py, so keep original
copyright)

Signed-off-by: Vladimir Sementsov-Ogievskiy 
---
  tests/avocado/hotplug_blk.py | 69 
  1 file changed, 69 insertions(+)
  create mode 100644 tests/avocado/hotplug_blk.py

diff --git a/tests/avocado/hotplug_blk.py b/tests/avocado/hotplug_blk.py
new file mode 100644
index 00..5dc30f6616
--- /dev/null
+++ b/tests/avocado/hotplug_blk.py
@@ -0,0 +1,69 @@
+# Functional test that hotplugs a virtio blk disk and checks it on a Linux
+# guest
+#
+# Copyright (c) 2021 Red Hat, Inc.
+# Copyright (c) Yandex
+#
+# 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 time
+
+from avocado_qemu import LinuxTest
+
+
+class HotPlug(LinuxTest):
+def blockdev_add(self) -> None:
+self.vm.cmd('blockdev-add', **{
+'driver': 'null-co',
+'size': 1073741824,
+'node-name': 'disk'
+})
+
+def assert_vda(self) -> None:
+self.ssh_command('test -e /sys/block/vda')
+
+def assert_no_vda(self) -> None:
+with self.assertRaises(AssertionError):
+self.assert_vda()
+
+def plug(self) -> None:
+args = {
+'driver': 'virtio-blk-pci',
+'drive': 'disk',
+'id': 'virtio-disk0',
+'bus': 'pci.1',
+'addr': 1
+}
+
+self.assert_no_vda()
+self.vm.cmd('device_add', args)
+try:
+self.assert_vda()
+except AssertionError:
+time.sleep(1)
+self.assert_vda()
+
+def unplug(self) -> None:
+self.vm.cmd('device_del', id='virtio-disk0')
+
+self.vm.event_wait('DEVICE_DELETED', 1.0,
+   match={'data': {'device': 'virtio-disk0'}})
+
+self.assert_no_vda()
+
+def test(self) -> None:
+"""
+:avocado: tags=arch:x86_64
+:avocado: tags=machine:q35
+:avocado: tags=accel:kvm
+"""
+self.require_accelerator('kvm')
+self.vm.add_args('-accel', 'kvm')
+self.vm.add_args('-device', 'pcie-pci-bridge,id=pci.1,bus=pcie.0')
+
+self.launch_and_wait()
+self.blockdev_add()
+
+self.plug()
+self.unplug()


Reviewed-by: Thomas Huth 




Re: [RFC PATCH] testing: restore some testing for i686

2024-06-25 Thread Thomas Huth

On 25/06/2024 16.54, Alex Bennée wrote:

The commit 4f9a8315e6 (gitlab-ci.d/crossbuilds: Drop the i386 system
emulation job) was a little too aggressive dropping testing for 32 bit
system builds.


FWIW: It was necessary at that point in time since we were constantly 
running out of CI minutes, and that job was one of the jobs that was running 
for the longest time, consuming lots of CI minutes, while only testing a 
host environment that hardly anybody uses anymore.


But now we've got our own custom runners, so we're not that badly limited in 
runtime anymore I guess, so it should be ok to re-introduce this job.



Partially revert but using the debian-i686 cross build
images this time as fedora has deprecated the 32 bit stuff.

Reported-by: Richard Henderson 
Suggested-by: Thomas Huth 
Signed-off-by: Alex Bennée 
---
  .gitlab-ci.d/crossbuilds.yml | 10 ++
  1 file changed, 10 insertions(+)

diff --git a/.gitlab-ci.d/crossbuilds.yml b/.gitlab-ci.d/crossbuilds.yml
index 47bdb99b5b..5ba728f641 100644
--- a/.gitlab-ci.d/crossbuilds.yml
+++ b/.gitlab-ci.d/crossbuilds.yml
@@ -37,6 +37,16 @@ cross-arm64-kvm-only:
  IMAGE: debian-arm64-cross
  EXTRA_CONFIGURE_OPTS: --disable-tcg --without-default-features
  
+cross-i686-system:

+  extends:
+- .cross_system_build_job
+- .cross_test_artifacts
+  needs:
+job: i686-debian-cross-container
+  variables:
+IMAGE: debian-i686-cross
+MAKE_CHECK_ARGS: check-qtest


Reviewed-by: Thomas Huth 





Re: Help improve 32-bit testing

2024-06-25 Thread Thomas Huth

On 25/06/2024 01.33, Richard Henderson wrote:

Hiya,

I've just discovered a 32-bit build issue that is probably 3 weeks old.

While we still support 32-bit builds at all, I would request that we improve 
our cross-i686 testing.  For instance: we have cross-i686-user and 
cross-i686-tci.  There is some system build testing in the tci job, but 
(rightfully) not everything.


System emulation on 32-bit x86 hosts is marked as deprecated since QEMU 8.0 
... maybe we could finally remove it instead?


I would like a full cross-i686-system target that builds all targets, and I 
would like the debian-i686-cross image on which we base these to be more 
complete -- ideally, exactly matching x86_64.  In particular, CONFIG_SEV is 
not detected within the current docker image, which is where the current 
build error is located.


Do you have time to look at this?


If you really, really want to go a step backwards, then basically just 
revert commit 4f9a8315e65561bafa03651518aa5d22af09bdee and use the 
i686-debian-cross-container image instead of the removed 
i386-fedora-cross-container image.


 HTH,
  Thomas




Re: [PATCH 1/1] include/qemu: Provide a C++ compatible version of typeof_strip_qual

2024-06-25 Thread Thomas Huth

On 25/06/2024 04.32, Roman Kiryanov wrote:

Hi Philippe, thank you for looking.

On Mon, Jun 24, 2024 at 7:27 PM Philippe Mathieu-Daudé
 wrote:

In particular this patch seems contained well enough
to be carried in forks were C++ _is_ used.


Will you agree to take #ifdef __cplusplus  and #error to the QEMU side
in atomic.h and
we will keep atomic.hpp on our side?


Well, from an upstream QEMU perspective, it doesn't make sense to have an 
#error with a message that references a file that does not exist in the 
upstream repository, so I think that patch also rather belongs to your 
C++-enabled downstream repository.


 Thomas





Re: [PATCH] .travis.yml: Install python3-tomli in all build jobs

2024-06-24 Thread Thomas Huth

On 24/06/2024 12.09, Alex Bennée wrote:

Thomas Huth  writes:


Since commit 1f97715c83 ('Revert "python: use vendored tomli"')
this package is a hard requirement for compiling QEMU, so install
it now in all Travis jobs, too.


AFAICT the only repo currently running these tests is your github
mirror:

   https://app.travis-ci.com/github/huth/qemu/builds?serverType=git

Because both the official github mirror and the gitlab project haven't
run anything for a while:

   https://app.travis-ci.com/gitlab/qemu-project/qemu/branches
   https://app.travis-ci.com/github/qemu/qemu/branches


AFAIK the gitlab integration broke completely at one point in time. But as 
you can see with my account, it still works fine for the github repo - I 
just need to write a short mail to the travis support once a year to ask 
them to extend the open source credits for my repo. So apart from that minor 
effort, it's a very convenient way to test QEMU on non-x86 hosts.



FWIW the changes themselves look fine:

Acked-by: Alex Bennée 


Thanks!

 Thomas





Re: [PATCH v2] scsi-disk: Fix crash for VM configured with USB CDROM after live migration

2024-06-24 Thread Thomas Huth

On 10/06/2024 19.02, Paolo Bonzini wrote:

From: Hyman Huang 

For VMs configured with the USB CDROM device:

-drive file=/path/to/local/file,id=drive-usb-disk0,media=cdrom,readonly=on...
-device usb-storage,drive=drive-usb-disk0,id=usb-disk0...

QEMU process may crash after live migration, to reproduce the issue,
configure VM (Guest OS ubuntu 20.04 or 21.10) with the following XML:


   
   
   
   
   



Do the live migration repeatedly, crash may happen after live migratoin,
trace log at the source before live migration is as follows:

324808@1711972823.521945:usb_uhci_frame_start nr 319
324808@1711972823.521978:usb_uhci_qh_load qh 0x35cb5400
324808@1711972823.521989:usb_uhci_qh_load qh 0x35cb5480
324808@1711972823.521997:usb_uhci_td_load qh 0x35cb5480, td 0x35cbe000, ctrl 
0x0, token 0xffe07f69
324808@1711972823.522010:usb_uhci_td_nextqh qh 0x35cb5480, td 0x35cbe000
324808@1711972823.522022:usb_uhci_qh_load qh 0x35cb5680
324808@1711972823.522030:usb_uhci_td_load qh 0x35cb5680, td 0x75ac5180, ctrl 
0x1980, token 0x3c903e1
324808@1711972823.522045:usb_uhci_packet_add token 0x103e1, td 0x75ac5180
324808@1711972823.522056:usb_packet_state_change bus 0, port 2, ep 2, packet 
0x559f9ba14b00, state undef -> setup
324808@1711972823.522079:usb_msd_cmd_submit lun 0, tag 0x472, flags 0x0080, 
len 10, data-len 8
324808@1711972823.522107:scsi_req_parsed target 0 lun 0 tag 1138 command 74 dir 
1 length 8
324808@1711972823.522124:scsi_req_parsed_lba target 0 lun 0 tag 1138 command 74 
lba 4096
324808@1711972823.522139:scsi_req_alloc target 0 lun 0 tag 1138
324808@1711972823.522169:scsi_req_continue target 0 lun 0 tag 1138
324808@1711972823.522181:scsi_req_data target 0 lun 0 tag 1138 len 8
324808@1711972823.522194:usb_packet_state_change bus 0, port 2, ep 2, packet 
0x559f9ba14b00, state setup -> complete
324808@1711972823.522209:usb_uhci_packet_complete_success token 0x103e1, td 
0x75ac5180
324808@1711972823.522219:usb_uhci_packet_del token 0x103e1, td 0x75ac5180
324808@1711972823.522232:usb_uhci_td_complete qh 0x35cb5680, td 0x75ac5180

trace log at the destination after live migration is as follows:

3286206@1711972823.951646:usb_uhci_frame_start nr 320
3286206@1711972823.951663:usb_uhci_qh_load qh 0x35cb5100
3286206@1711972823.951671:usb_uhci_qh_load qh 0x35cb5480
3286206@1711972823.951680:usb_uhci_td_load qh 0x35cb5480, td 0x35cbe000, ctrl 
0x100, token 0xffe07f69
3286206@1711972823.951693:usb_uhci_td_nextqh qh 0x35cb5480, td 0x35cbe000
3286206@1711972823.951702:usb_uhci_qh_load qh 0x35cb5700
3286206@1711972823.951709:usb_uhci_td_load qh 0x35cb5700, td 0x75ac5240, ctrl 
0x3980, token 0xe08369
3286206@1711972823.951727:usb_uhci_queue_add token 0x8369
3286206@1711972823.951735:usb_uhci_packet_add token 0x8369, td 0x75ac5240
3286206@1711972823.951746:usb_packet_state_change bus 0, port 2, ep 1, packet 
0x56066b2fb5a0, state undef -> setup
3286206@1711972823.951766:usb_msd_data_in 8/8 (scsi 8)
2024-04-01 12:00:24.665+: shutting down, reason=crashed

The backtrace reveals the following:

Program terminated with signal SIGSEGV, Segmentation fault.
0  __memmove_sse2_unaligned_erms () at 
../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:312
312movq-8(%rsi,%rdx), %rcx
[Current thread is 1 (Thread 0x7f0a9025fc00 (LWP 3286206))]
(gdb) bt
0  __memmove_sse2_unaligned_erms () at 
../sysdeps/x86_64/multiarch/memmove-vec-unaligned-erms.S:312
1  memcpy (__len=8, __src=, __dest=) at 
/usr/include/bits/string_fortified.h:34
2  iov_from_buf_full (iov=, iov_cnt=, 
offset=, buf=0x0, bytes=bytes@entry=8) at ../util/iov.c:33
3  iov_from_buf (bytes=8, buf=, offset=, 
iov_cnt=, iov=)
at 
/usr/src/debug/qemu-6-6.2.0-75.7.oe1.smartx.git.40.x86_64/include/qemu/iov.h:49
4  usb_packet_copy (p=p@entry=0x56066b2fb5a0, ptr=, 
bytes=bytes@entry=8) at ../hw/usb/core.c:636
5  usb_msd_copy_data (s=s@entry=0x56066c62c770, p=p@entry=0x56066b2fb5a0) at 
../hw/usb/dev-storage.c:186
6  usb_msd_handle_data (dev=0x56066c62c770, p=0x56066b2fb5a0) at 
../hw/usb/dev-storage.c:496
7  usb_handle_packet (dev=0x56066c62c770, p=p@entry=0x56066b2fb5a0) at 
../hw/usb/core.c:455
8  uhci_handle_td (s=s@entry=0x56066bd5f210, q=0x56066bb7fbd0, q@entry=0x0, 
qh_addr=qh_addr@entry=902518530, td=td@entry=0x7fffe6e788f0, td_addr=,
int_mask=int_mask@entry=0x7fffe6e788e4) at ../hw/usb/hcd-uhci.c:885
9  uhci_process_frame (s=s@entry=0x56066bd5f210) at ../hw/usb/hcd-uhci.c:1061
10 uhci_frame_timer (opaque=opaque@entry=0x56066bd5f210) at 
../hw/usb/hcd-uhci.c:1159
11 timerlist_run_timers (timer_list=0x56066af26bd0) at ../util/qemu-timer.c:642
12 qemu_clock_run_timers (type=QEMU_CLOCK_VIRTUAL) at ../util/qemu-timer.c:656
13 qemu_clock_run_all_timers () at ../util/qemu-timer.c:738
14 main_loop_wait (nonblocking=nonblocking@entry=0) at ../util/main-loop.c:542
15 qemu_main_loop () at ../softmmu/runstate.c:739
16 main (argc=, argv=, envp=) at 
../softmmu/main.c:52
(gdb) frame 5
(gdb) p ((SCSIDiskReq *)s->req)->iov
$1 = {iov_base = 

[PATCH] .travis.yml: Install python3-tomli in all build jobs

2024-06-24 Thread Thomas Huth
Since commit 1f97715c83 ('Revert "python: use vendored tomli"')
this package is a hard requirement for compiling QEMU, so install
it now in all Travis jobs, too.

Signed-off-by: Thomas Huth 
---
 .travis.yml | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/.travis.yml b/.travis.yml
index cef0308952..8fc1ae0cf2 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -106,6 +106,7 @@ jobs:
   - libvdeplug-dev
   - libvte-2.91-dev
   - ninja-build
+  - python3-tomli
   # Tests dependencies
   - genisoimage
   env:
@@ -141,6 +142,7 @@ jobs:
   - libvdeplug-dev
   - libvte-2.91-dev
   - ninja-build
+  - python3-tomli
   # Tests dependencies
   - genisoimage
   env:
@@ -175,6 +177,7 @@ jobs:
   - libvdeplug-dev
   - libvte-2.91-dev
   - ninja-build
+  - python3-tomli
   # Tests dependencies
   - genisoimage
   env:
@@ -215,6 +218,7 @@ jobs:
   - libzstd-dev
   - nettle-dev
   - ninja-build
+  - python3-tomli
   # Tests dependencies
   - genisoimage
   env:
@@ -231,6 +235,7 @@ jobs:
   - ninja-build
   - flex
   - bison
+  - python3-tomli
   env:
 - TEST_CMD="make check check-tcg V=1"
 - CONFIG="--disable-containers --disable-system"
@@ -263,6 +268,7 @@ jobs:
   - libvdeplug-dev
   - libvte-2.91-dev
   - ninja-build
+  - python3-tomli
   env:
 - TEST_CMD="make check-unit"
 - CONFIG="--disable-containers --disable-tcg --enable-kvm 
--disable-tools
-- 
2.45.2




[PULL 05/11] vfio/ccw: Use the 'Error **errp' argument of vfio_ccw_realize()

2024-06-24 Thread Thomas Huth
From: Cédric Le Goater 

The local error variable is kept for vfio_ccw_register_irq_notifier()
because it is not considered as a failing condition. We will change
how error reporting is done in following changes.

Remove the error_propagate() call.

Cc: Zhenzhong Duan 
Signed-off-by: Cédric Le Goater 
Reviewed-by: Zhenzhong Duan 
Reviewed-by: Anthony Krowiak 
Reviewed-by: Eric Farman 
Reviewed-by: Thomas Huth 
Message-ID: <20240522170107.289532-6-...@redhat.com>
Signed-off-by: Thomas Huth 
---
 hw/vfio/ccw.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 9a8e052711..a468fa2342 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -582,8 +582,8 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp)
 
 /* Call the class init function for subchannel. */
 if (cdc->realize) {
-if (!cdc->realize(cdev, vcdev->vdev.sysfsdev, )) {
-goto out_err_propagate;
+if (!cdc->realize(cdev, vcdev->vdev.sysfsdev, errp)) {
+return;
 }
 }
 
@@ -596,17 +596,17 @@ static void vfio_ccw_realize(DeviceState *dev, Error 
**errp)
 goto out_attach_dev_err;
 }
 
-if (!vfio_ccw_get_region(vcdev, )) {
+if (!vfio_ccw_get_region(vcdev, errp)) {
 goto out_region_err;
 }
 
-if (!vfio_ccw_register_irq_notifier(vcdev, VFIO_CCW_IO_IRQ_INDEX, )) {
+if (!vfio_ccw_register_irq_notifier(vcdev, VFIO_CCW_IO_IRQ_INDEX, errp)) {
 goto out_io_notifier_err;
 }
 
 if (vcdev->crw_region) {
 if (!vfio_ccw_register_irq_notifier(vcdev, VFIO_CCW_CRW_IRQ_INDEX,
-)) {
+errp)) {
 goto out_irq_notifier_err;
 }
 }
@@ -634,8 +634,6 @@ out_attach_dev_err:
 if (cdc->unrealize) {
 cdc->unrealize(cdev);
 }
-out_err_propagate:
-error_propagate(errp, err);
 }
 
 static void vfio_ccw_unrealize(DeviceState *dev)
-- 
2.45.2




[PULL 04/11] s390x/css: Make S390CCWDeviceClass::realize return bool

2024-06-24 Thread Thomas Huth
From: Cédric Le Goater 

Since the realize() handler of S390CCWDeviceClass takes an 'Error **'
argument, best practices suggest to return a bool. See the api/error.h
Rules section. While at it, modify the call in vfio_ccw_realize().

Signed-off-by: Cédric Le Goater 
Reviewed-by: Zhenzhong Duan 
Reviewed-by: Anthony Krowiak 
Reviewed-by: Eric Farman 
Reviewed-by: Thomas Huth 
Message-ID: <20240522170107.289532-5-...@redhat.com>
Signed-off-by: Thomas Huth 
---
 include/hw/s390x/s390-ccw.h | 2 +-
 hw/s390x/s390-ccw.c | 7 ---
 hw/vfio/ccw.c   | 3 +--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/hw/s390x/s390-ccw.h b/include/hw/s390x/s390-ccw.h
index 2c807ee3a1..2e0a709981 100644
--- a/include/hw/s390x/s390-ccw.h
+++ b/include/hw/s390x/s390-ccw.h
@@ -31,7 +31,7 @@ struct S390CCWDevice {
 
 struct S390CCWDeviceClass {
 CCWDeviceClass parent_class;
-void (*realize)(S390CCWDevice *dev, char *sysfsdev, Error **errp);
+bool (*realize)(S390CCWDevice *dev, char *sysfsdev, Error **errp);
 void (*unrealize)(S390CCWDevice *dev);
 IOInstEnding (*handle_request) (SubchDev *sch);
 int (*handle_halt) (SubchDev *sch);
diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c
index b3d14c61d7..3c09750550 100644
--- a/hw/s390x/s390-ccw.c
+++ b/hw/s390x/s390-ccw.c
@@ -108,7 +108,7 @@ static bool s390_ccw_get_dev_info(S390CCWDevice *cdev,
 return true;
 }
 
-static void s390_ccw_realize(S390CCWDevice *cdev, char *sysfsdev, Error **errp)
+static bool s390_ccw_realize(S390CCWDevice *cdev, char *sysfsdev, Error **errp)
 {
 CcwDevice *ccw_dev = CCW_DEVICE(cdev);
 CCWDeviceClass *ck = CCW_DEVICE_GET_CLASS(ccw_dev);
@@ -117,7 +117,7 @@ static void s390_ccw_realize(S390CCWDevice *cdev, char 
*sysfsdev, Error **errp)
 int ret;
 
 if (!s390_ccw_get_dev_info(cdev, sysfsdev, errp)) {
-return;
+return false;
 }
 
 sch = css_create_sch(ccw_dev->devno, errp);
@@ -142,7 +142,7 @@ static void s390_ccw_realize(S390CCWDevice *cdev, char 
*sysfsdev, Error **errp)
 
 css_generate_sch_crws(sch->cssid, sch->ssid, sch->schid,
   parent->hotplugged, 1);
-return;
+return true;
 
 out_err:
 css_subch_assign(sch->cssid, sch->ssid, sch->schid, sch->devno, NULL);
@@ -150,6 +150,7 @@ out_err:
 g_free(sch);
 out_mdevid_free:
 g_free(cdev->mdevid);
+return false;
 }
 
 static void s390_ccw_unrealize(S390CCWDevice *cdev)
diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 2600e62e37..9a8e052711 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -582,8 +582,7 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp)
 
 /* Call the class init function for subchannel. */
 if (cdc->realize) {
-cdc->realize(cdev, vcdev->vdev.sysfsdev, );
-if (err) {
+if (!cdc->realize(cdev, vcdev->vdev.sysfsdev, )) {
 goto out_err_propagate;
 }
 }
-- 
2.45.2




[PULL 08/11] tests/qtest/fuzz: fix memleak in qos_fuzz.c

2024-06-24 Thread Thomas Huth
From: Dmitry Frolov 

Found with fuzzing for qemu-8.2, but also relevant for master

Signed-off-by: Dmitry Frolov 
Reviewed-by: Thomas Huth 
Reviewed-by: Alexander Bulekov 
Message-ID: <20240521103106.119021-3-fro...@swemel.ru>
Signed-off-by: Thomas Huth 
---
 tests/qtest/fuzz/qos_fuzz.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
index b71e945c5f..d3839bf999 100644
--- a/tests/qtest/fuzz/qos_fuzz.c
+++ b/tests/qtest/fuzz/qos_fuzz.c
@@ -180,6 +180,7 @@ static void walk_path(QOSGraphNode *orig_path, int len)
 
 fuzz_path_vec = path_vec;
 } else {
+g_string_free(cmd_line, true);
 g_free(path_vec);
 }
 
-- 
2.45.2




[PULL 07/11] vfio/{ap, ccw}: Use warn_report_err() for IRQ notifier registration errors

2024-06-24 Thread Thomas Huth
From: Cédric Le Goater 

vfio_ccw_register_irq_notifier() and vfio_ap_register_irq_notifier()
errors are currently reported using error_report_err(). Since they are
not considered as failing conditions, using warn_report_err() is more
appropriate.

Signed-off-by: Cédric Le Goater 
Reviewed-by: Zhenzhong Duan 
Reviewed-by: Anthony Krowiak 
Reviewed-by: Eric Farman 
Reviewed-by: Thomas Huth 
Message-ID: <20240522170107.289532-8-...@redhat.com>
Signed-off-by: Thomas Huth 
---
 hw/vfio/ap.c  | 2 +-
 hw/vfio/ccw.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
index c12531a788..0c4354e3e7 100644
--- a/hw/vfio/ap.c
+++ b/hw/vfio/ap.c
@@ -172,7 +172,7 @@ static void vfio_ap_realize(DeviceState *dev, Error **errp)
  * Report this error, but do not make it a failing condition.
  * Lack of this IRQ in the host does not prevent normal operation.
  */
-error_report_err(err);
+warn_report_err(err);
 }
 
 return;
diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index 36f2677a44..1f8e1272c7 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -616,7 +616,7 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp)
  * Report this error, but do not make it a failing condition.
  * Lack of this IRQ in the host does not prevent normal operation.
  */
-error_report_err(err);
+warn_report_err(err);
 }
 
 return;
-- 
2.45.2




[PULL 09/11] target/s390x/arch_dump: use correct byte order for pid

2024-06-24 Thread Thomas Huth
From: Omar Sandoval 

The pid field of prstatus needs to be big endian like all of the other
fields.

Fixes: f738f296eaae ("s390x/arch_dump: pass cpuid into notes sections")
Signed-off-by: Omar Sandoval 
Reviewed-by: Thomas Huth 
Message-ID: 
<5929f76d536d355afd04af51bf293695a1065118.1718771802.git.osan...@osandov.com>
Signed-off-by: Thomas Huth 
---
 target/s390x/arch_dump.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/arch_dump.c b/target/s390x/arch_dump.c
index 7e8a1b4fc0..029d91d93a 100644
--- a/target/s390x/arch_dump.c
+++ b/target/s390x/arch_dump.c
@@ -102,7 +102,7 @@ static void s390x_write_elf64_prstatus(Note *note, S390CPU 
*cpu, int id)
 regs->acrs[i] = cpu_to_be32(cpu->env.aregs[i]);
 regs->gprs[i] = cpu_to_be64(cpu->env.regs[i]);
 }
-note->contents.prstatus.pid = id;
+note->contents.prstatus.pid = cpu_to_be32(id);
 }
 
 static void s390x_write_elf64_fpregset(Note *note, S390CPU *cpu, int id)
-- 
2.45.2




[PULL 11/11] target/s390x: Add a CONFIG switch to disable legacy CPUs

2024-06-24 Thread Thomas Huth
The oldest model that IBM still supports is the z13. Considering
that each generation can "emulate" the previous two generations
in hardware (via the "IBC" feature of the CPUs), this means that
everything that is older than z114/196 is not an officially supported
CPU model anymore. The Linux kernel still support the z10, so if
we also take this into account, everything older than that can
definitely be considered as a legacy CPU model.

For downstream builds of QEMU, we would like to be able to disable
these legacy CPUs in the build. Thus add a CONFIG switch that can be
used to disable them (and old machine types that use them by default).

Message-Id: <20240614125019.588928-1-th...@redhat.com>
Signed-off-by: Thomas Huth 
---
 hw/s390x/s390-virtio-ccw.c | 5 +
 target/s390x/cpu_models.c  | 9 +
 target/s390x/Kconfig   | 5 +
 3 files changed, 19 insertions(+)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 3d0bc3e7f2..cd063f8b64 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -47,6 +47,7 @@
 #include "migration/blocker.h"
 #include "qapi/visitor.h"
 #include "hw/s390x/cpu-topology.h"
+#include CONFIG_DEVICES
 
 static Error *pv_mig_blocker;
 
@@ -1126,6 +1127,8 @@ static void ccw_machine_2_12_class_options(MachineClass 
*mc)
 }
 DEFINE_CCW_MACHINE(2_12, "2.12", false);
 
+#ifdef CONFIG_S390X_LEGACY_CPUS
+
 static void ccw_machine_2_11_instance_options(MachineState *machine)
 {
 static const S390FeatInit qemu_cpu_feat = { S390_FEAT_LIST_QEMU_V2_11 };
@@ -1272,6 +1275,8 @@ static void ccw_machine_2_4_class_options(MachineClass 
*mc)
 }
 DEFINE_CCW_MACHINE(2_4, "2.4", false);
 
+#endif
+
 static void ccw_machine_register_types(void)
 {
 type_register_static(_machine_info);
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index efb508cd2e..a27f4b6f79 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -25,6 +25,7 @@
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/sysemu.h"
 #include "target/s390x/kvm/pv.h"
+#include CONFIG_DEVICES
 #endif
 
 #define CPUDEF_INIT(_type, _gen, _ec_ga, _mha_pow, _hmfai, _name, _desc) \
@@ -47,6 +48,13 @@
  * generation 15 one base feature and one optional feature have been 
deprecated.
  */
 static S390CPUDef s390_cpu_defs[] = {
+/*
+ * Linux requires at least z10 nowadays, and IBM only supports recent CPUs
+ * (see 
https://www.ibm.com/support/pages/ibm-mainframe-life-cycle-history),
+ * so we consider older CPUs as legacy that can optionally be disabled via
+ * the CONFIG_S390X_LEGACY_CPUS config switch.
+ */
+#if defined(CONFIG_S390X_LEGACY_CPUS) || defined(CONFIG_USER_ONLY)
 CPUDEF_INIT(0x2064, 7, 1, 38, 0xU, "z900", "IBM zSeries 900 GA1"),
 CPUDEF_INIT(0x2064, 7, 2, 38, 0xU, "z900.2", "IBM zSeries 900 
GA2"),
 CPUDEF_INIT(0x2064, 7, 3, 38, 0xU, "z900.3", "IBM zSeries 900 
GA3"),
@@ -64,6 +72,7 @@ static S390CPUDef s390_cpu_defs[] = {
 CPUDEF_INIT(0x2096, 9, 2, 40, 0xU, "z9BC", "IBM System z9 BC GA1"),
 CPUDEF_INIT(0x2094, 9, 3, 40, 0xU, "z9EC.3", "IBM System z9 EC 
GA3"),
 CPUDEF_INIT(0x2096, 9, 3, 40, 0xU, "z9BC.2", "IBM System z9 BC 
GA2"),
+#endif
 CPUDEF_INIT(0x2097, 10, 1, 43, 0xU, "z10EC", "IBM System z10 EC 
GA1"),
 CPUDEF_INIT(0x2097, 10, 2, 43, 0xU, "z10EC.2", "IBM System z10 EC 
GA2"),
 CPUDEF_INIT(0x2098, 10, 2, 43, 0xU, "z10BC", "IBM System z10 BC 
GA1"),
diff --git a/target/s390x/Kconfig b/target/s390x/Kconfig
index d886be48b4..8a95f2bc3f 100644
--- a/target/s390x/Kconfig
+++ b/target/s390x/Kconfig
@@ -2,3 +2,8 @@ config S390X
 bool
 select PCI
 select S390_FLIC
+
+config S390X_LEGACY_CPUS
+bool
+default y
+depends on S390X
-- 
2.45.2




[PULL 00/11] s390x and qtest patches 2024-06-24

2024-06-24 Thread Thomas Huth
The following changes since commit c9ba79baca7c673098361e3a687f72d458e0d18a:

  Merge tag 'pull-target-arm-20240622' of 
https://git.linaro.org/people/pmaydell/qemu-arm into staging (2024-06-22 
09:56:49 -0700)

are available in the Git repository at:

  https://gitlab.com/thuth/qemu.git tags/pull-request-2024-06-24

for you to fetch changes up to d6a7c3f44cf3f60c066dbf087ef79d4b12acc642:

  target/s390x: Add a CONFIG switch to disable legacy CPUs (2024-06-24 08:22:30 
+0200)


* s390x error reporting clean ups
* fix memleak in qos_fuzz.c
* use correct byte order for pid field in s390x dumps
* Add a CONFIG switch to disable legacy s390x CPUs


Cédric Le Goater (6):
  hw/s390x/ccw: Make s390_ccw_get_dev_info() return a bool
  s390x/css: Make CCWDeviceClass::realize return bool
  hw/s390x/ccw: Remove local Error variable from s390_ccw_realize()
  s390x/css: Make S390CCWDeviceClass::realize return bool
  vfio/ccw: Use the 'Error **errp' argument of vfio_ccw_realize()
  vfio/{ap, ccw}: Use warn_report_err() for IRQ notifier registration errors

Dmitry Frolov (1):
  tests/qtest/fuzz: fix memleak in qos_fuzz.c

Omar Sandoval (1):
  target/s390x/arch_dump: use correct byte order for pid

Thomas Huth (2):
  MAINTAINERS: Cover all tests/qtest/migration-* files
  target/s390x: Add a CONFIG switch to disable legacy CPUs

Zhenzhong Duan (1):
  vfio/ccw: Fix the missed unrealize() call in error path

 MAINTAINERS |  3 ++-
 hw/s390x/ccw-device.h   |  2 +-
 include/hw/s390x/s390-ccw.h |  2 +-
 hw/s390x/ccw-device.c   |  3 ++-
 hw/s390x/s390-ccw.c | 29 +
 hw/s390x/s390-virtio-ccw.c  |  5 +
 hw/vfio/ap.c|  2 +-
 hw/vfio/ccw.c   | 18 --
 target/s390x/arch_dump.c|  2 +-
 target/s390x/cpu_models.c   |  9 +
 tests/qtest/fuzz/qos_fuzz.c |  1 +
 target/s390x/Kconfig|  5 +
 12 files changed, 49 insertions(+), 32 deletions(-)




[PULL 06/11] vfio/ccw: Fix the missed unrealize() call in error path

2024-06-24 Thread Thomas Huth
From: Zhenzhong Duan 

When get name failed, we should call unrealize() so that
vfio_ccw_realize() is self contained.

Fixes: 909a6254eda ("vfio/ccw: Make vfio cdev pre-openable by passing a file 
handle")
Signed-off-by: Zhenzhong Duan 
Reviewed-by: Cédric Le Goater 
Reviewed-by: Eric Farman 
Reviewed-by: Thomas Huth 
Message-ID: <20240522170107.289532-7-...@redhat.com>
Signed-off-by: Thomas Huth 
---
 hw/vfio/ccw.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/hw/vfio/ccw.c b/hw/vfio/ccw.c
index a468fa2342..36f2677a44 100644
--- a/hw/vfio/ccw.c
+++ b/hw/vfio/ccw.c
@@ -588,7 +588,7 @@ static void vfio_ccw_realize(DeviceState *dev, Error **errp)
 }
 
 if (!vfio_device_get_name(vbasedev, errp)) {
-return;
+goto out_unrealize;
 }
 
 if (!vfio_attach_device(cdev->mdevid, vbasedev,
@@ -631,6 +631,7 @@ out_region_err:
 vfio_detach_device(vbasedev);
 out_attach_dev_err:
 g_free(vbasedev->name);
+out_unrealize:
 if (cdc->unrealize) {
 cdc->unrealize(cdev);
 }
-- 
2.45.2




[PULL 03/11] hw/s390x/ccw: Remove local Error variable from s390_ccw_realize()

2024-06-24 Thread Thomas Huth
From: Cédric Le Goater 

Use the 'Error **errp' argument of s390_ccw_realize() instead and
remove the error_propagate() call.

Signed-off-by: Cédric Le Goater 
Reviewed-by: Zhenzhong Duan 
Reviewed-by: Anthony Krowiak 
Reviewed-by: Eric Farman 
Reviewed-by: Thomas Huth 
Message-ID: <20240522170107.289532-4-...@redhat.com>
Signed-off-by: Thomas Huth 
---
 hw/s390x/s390-ccw.c | 13 +
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c
index 4b8ede701d..b3d14c61d7 100644
--- a/hw/s390x/s390-ccw.c
+++ b/hw/s390x/s390-ccw.c
@@ -115,13 +115,12 @@ static void s390_ccw_realize(S390CCWDevice *cdev, char 
*sysfsdev, Error **errp)
 DeviceState *parent = DEVICE(ccw_dev);
 SubchDev *sch;
 int ret;
-Error *err = NULL;
 
-if (!s390_ccw_get_dev_info(cdev, sysfsdev, )) {
-goto out_err_propagate;
+if (!s390_ccw_get_dev_info(cdev, sysfsdev, errp)) {
+return;
 }
 
-sch = css_create_sch(ccw_dev->devno, );
+sch = css_create_sch(ccw_dev->devno, errp);
 if (!sch) {
 goto out_mdevid_free;
 }
@@ -132,12 +131,12 @@ static void s390_ccw_realize(S390CCWDevice *cdev, char 
*sysfsdev, Error **errp)
 ccw_dev->sch = sch;
 ret = css_sch_build_schib(sch, >hostid);
 if (ret) {
-error_setg_errno(, -ret, "%s: Failed to build initial schib",
+error_setg_errno(errp, -ret, "%s: Failed to build initial schib",
  __func__);
 goto out_err;
 }
 
-if (!ck->realize(ccw_dev, )) {
+if (!ck->realize(ccw_dev, errp)) {
 goto out_err;
 }
 
@@ -151,8 +150,6 @@ out_err:
 g_free(sch);
 out_mdevid_free:
 g_free(cdev->mdevid);
-out_err_propagate:
-error_propagate(errp, err);
 }
 
 static void s390_ccw_unrealize(S390CCWDevice *cdev)
-- 
2.45.2




[PULL 10/11] MAINTAINERS: Cover all tests/qtest/migration-* files

2024-06-24 Thread Thomas Huth
Beside migration-test.c, there is nowadays migration-helpers.[ch],
too, so update the entry in the migration section to also cover these
files now.
While we're at it, exclude these files in the common qtest section,
since the migration test is well covered by the migration maintainers
already. Since the test is under very active development, it was causing
a lot of distraction to the generic qtest maintainers with regards to
the patches that need to be reviewed by the migration maintainers anyway.

Message-ID: <20240619055447.129943-1-th...@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé 
Acked-by: Fabiano Rosas 
Reviewed-by: Peter Xu 
Signed-off-by: Thomas Huth 
---
 MAINTAINERS | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index cef54de759..f144b5af44 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3312,6 +3312,7 @@ F: tests/qtest/
 F: docs/devel/qgraph.rst
 F: docs/devel/qtest.rst
 X: tests/qtest/bios-tables-test*
+X: tests/qtest/migration-*
 
 Device Fuzzing
 M: Alexander Bulekov 
@@ -3408,7 +3409,7 @@ F: include/qemu/userfaultfd.h
 F: migration/
 F: scripts/vmstate-static-checker.py
 F: tests/vmstate-static-checker-data/
-F: tests/qtest/migration-test.c
+F: tests/qtest/migration-*
 F: docs/devel/migration/
 F: qapi/migration.json
 F: tests/migration/
-- 
2.45.2




[PULL 01/11] hw/s390x/ccw: Make s390_ccw_get_dev_info() return a bool

2024-06-24 Thread Thomas Huth
From: Cédric Le Goater 

Since s390_ccw_get_dev_info() takes an 'Error **' argument, best
practices suggest to return a bool. See the qapi/error.h Rules
section. While at it, modify the call in s390_ccw_realize().

Signed-off-by: Cédric Le Goater 
Reviewed-by: Zhenzhong Duan 
Reviewed-by: Anthony Krowiak 
Reviewed-by: Eric Farman 
Reviewed-by: Thomas Huth 
Message-ID: <20240522170107.289532-2-...@redhat.com>
Signed-off-by: Thomas Huth 
---
 hw/s390x/s390-ccw.c | 12 ++--
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c
index 5261e66724..a06e91dfb3 100644
--- a/hw/s390x/s390-ccw.c
+++ b/hw/s390x/s390-ccw.c
@@ -71,7 +71,7 @@ IOInstEnding s390_ccw_store(SubchDev *sch)
 return ret;
 }
 
-static void s390_ccw_get_dev_info(S390CCWDevice *cdev,
+static bool s390_ccw_get_dev_info(S390CCWDevice *cdev,
   char *sysfsdev,
   Error **errp)
 {
@@ -84,12 +84,12 @@ static void s390_ccw_get_dev_info(S390CCWDevice *cdev,
 error_setg(errp, "No host device provided");
 error_append_hint(errp,
   "Use -device vfio-ccw,sysfsdev=PATH_TO_DEVICE\n");
-return;
+return false;
 }
 
 if (!realpath(sysfsdev, dev_path)) {
 error_setg_errno(errp, errno, "Host device '%s' not found", sysfsdev);
-return;
+return false;
 }
 
 cdev->mdevid = g_path_get_basename(dev_path);
@@ -98,13 +98,14 @@ static void s390_ccw_get_dev_info(S390CCWDevice *cdev,
 tmp = g_path_get_basename(tmp_dir);
 if (sscanf(tmp, "%2x.%1x.%4x", , , ) != 3) {
 error_setg_errno(errp, errno, "Failed to read %s", tmp);
-return;
+return false;
 }
 
 cdev->hostid.cssid = cssid;
 cdev->hostid.ssid = ssid;
 cdev->hostid.devid = devid;
 cdev->hostid.valid = true;
+return true;
 }
 
 static void s390_ccw_realize(S390CCWDevice *cdev, char *sysfsdev, Error **errp)
@@ -116,8 +117,7 @@ static void s390_ccw_realize(S390CCWDevice *cdev, char 
*sysfsdev, Error **errp)
 int ret;
 Error *err = NULL;
 
-s390_ccw_get_dev_info(cdev, sysfsdev, );
-if (err) {
+if (!s390_ccw_get_dev_info(cdev, sysfsdev, )) {
 goto out_err_propagate;
 }
 
-- 
2.45.2




[PULL 02/11] s390x/css: Make CCWDeviceClass::realize return bool

2024-06-24 Thread Thomas Huth
From: Cédric Le Goater 

Since the realize() handler of CCWDeviceClass takes an 'Error **'
argument, best practices suggest to return a bool. See the api/error.h
Rules section. While at it, modify the call in s390_ccw_realize().

Signed-off-by: Cédric Le Goater 
Reviewed-by: Zhenzhong Duan 
Reviewed-by: Anthony Krowiak 
Reviewed-by: Eric Farman 
Reviewed-by: Thomas Huth 
Message-ID: <20240522170107.289532-3-...@redhat.com>
Signed-off-by: Thomas Huth 
---
 hw/s390x/ccw-device.h | 2 +-
 hw/s390x/ccw-device.c | 3 ++-
 hw/s390x/s390-ccw.c   | 3 +--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/hw/s390x/ccw-device.h b/hw/s390x/ccw-device.h
index 6dff95225d..5feeb0ee7a 100644
--- a/hw/s390x/ccw-device.h
+++ b/hw/s390x/ccw-device.h
@@ -36,7 +36,7 @@ extern const VMStateDescription vmstate_ccw_dev;
 struct CCWDeviceClass {
 DeviceClass parent_class;
 void (*unplug)(HotplugHandler *, DeviceState *, Error **);
-void (*realize)(CcwDevice *, Error **);
+bool (*realize)(CcwDevice *, Error **);
 void (*refill_ids)(CcwDevice *);
 };
 
diff --git a/hw/s390x/ccw-device.c b/hw/s390x/ccw-device.c
index fb8c1acc64..a7d682e5af 100644
--- a/hw/s390x/ccw-device.c
+++ b/hw/s390x/ccw-device.c
@@ -31,9 +31,10 @@ static void ccw_device_refill_ids(CcwDevice *dev)
 dev->subch_id.valid = true;
 }
 
-static void ccw_device_realize(CcwDevice *dev, Error **errp)
+static bool ccw_device_realize(CcwDevice *dev, Error **errp)
 {
 ccw_device_refill_ids(dev);
+return true;
 }
 
 static Property ccw_device_properties[] = {
diff --git a/hw/s390x/s390-ccw.c b/hw/s390x/s390-ccw.c
index a06e91dfb3..4b8ede701d 100644
--- a/hw/s390x/s390-ccw.c
+++ b/hw/s390x/s390-ccw.c
@@ -137,8 +137,7 @@ static void s390_ccw_realize(S390CCWDevice *cdev, char 
*sysfsdev, Error **errp)
 goto out_err;
 }
 
-ck->realize(ccw_dev, );
-if (err) {
+if (!ck->realize(ccw_dev, )) {
 goto out_err;
 }
 
-- 
2.45.2




Re: [PATCH 0/2] arch_dump: fix prstatus pid on s390x and ppc

2024-06-24 Thread Thomas Huth

On 19/06/2024 07.00, Omar Sandoval wrote:

Hello,

I maintain drgn [1], a debugger for the Linux kernel. I ran into a quirk
of the NT_PRSTATUS note in kernel core dumps [2], so I looked into how
QEMU's dump-guest-memory command generates NT_PRSTATUS. I noticed that
on most architectures, the note's PID field is set to the CPU ID plus 1.
There are two exceptions: on s390x, there's an endianness bug (it's not
byte swapped if the host is little endian), and on ppc, it's not set at
all (it defaults to zero). They're both easy fixes.

Thanks,
Omar

1: https://github.com/osandov/drgn
2: https://github.com/osandov/drgn/issues/404

Omar Sandoval (2):
   target/s390x/arch_dump: use correct byte order for pid
   target/ppc/arch_dump: set prstatus pid to cpuid

  target/ppc/arch_dump.c   | 21 -
  target/s390x/arch_dump.c |  2 +-
  2 files changed, 13 insertions(+), 10 deletions(-)


I'm going to pick up patch 1 for my s390x tree ... for the second patch, it 
would be good if someone of the ppc guys could have a look at it first.


 Thomas





Re: [PATCH 0/7] pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img

2024-06-23 Thread Thomas Huth

On 21/06/2024 22.51, Eric Farman wrote:

On Fri, 2024-06-21 at 10:24 +0200, Thomas Huth wrote:

We originally built a separate binary for the netboot code since it
was considered as experimental and we could not be sure that the
necessary SLOF module had been checked out. Time passed, the netboot
code proved its usefulness, and the build system nowadays makes sure
that the SLOF module is checked out if you have a s390x compiler
available
for building the s390-ccw bios. In fact, the possibility to build the
s390-ccw.img without s390-netboot.img has been removed in commit
bf6903f6944f ("pc-bios/s390-ccw: always build network bootloader")
already.

So it does not make too much sense anymore to keep the netboot code
in a separate binary. To make it easier to support a more flexible
boot process soon that supports more than one boot device via the
bootindex properties, let's finally merge the netboot code into the
main s390-ccw.img binary now.


Hi Thomas,

I find myself wondering about the side effects of the
s/sclp_print/printf/ changes, but I haven't come up with anything I can
put my finger on. Maybe something will come to me over the weekend, but
all-in-all I like the looks of this.


I think it should be fine, both functions are basically just a wrapper 
around the write() function in sclp.c, with sclp_print() being rather dumb 
while printf() is doing the usual string formatting before writing it out. I 
think in the long run, it would be nice to get rid of sclp_print() and 
replace it by puts() or printf() in the whole code, but doing that right now 
would likely cause quite some conflicts for Jared with his patch series, so 
I'd rather postpone that to a later point in time.



Reviewed-by: Eric Farman 


Thanks!

 Thomas




Re: [PATCH 0/7] pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img

2024-06-21 Thread Thomas Huth

On 21/06/2024 11.39, Christian Borntraeger wrote:

[...]

  docs/system/s390x/bootdevices.rst |  20 +++
  pc-bios/s390-ccw/netboot.mak  |  62 -
  hw/s390x/ipl.h    |  12 ++--
  pc-bios/s390-ccw/cio.h    |   2 +
  pc-bios/s390-ccw/iplb.h   |   4 +-
  pc-bios/s390-ccw/libc.h   |  89 --
  pc-bios/s390-ccw/s390-ccw.h   |  10 +++-
  pc-bios/s390-ccw/virtio.h |   1 -
  hw/s390x/ipl.c    |  65 +++---
  hw/s390x/s390-virtio-ccw.c    |  10 +---
  pc-bios/s390-ccw/bootmap.c    |   4 +-
  pc-bios/s390-ccw/cio.c    |   2 +-
  pc-bios/s390-ccw/dasd-ipl.c   |   2 +-
  pc-bios/s390-ccw/jump2ipl.c   |   2 +-
  pc-bios/s390-ccw/libc.c   |  88 -
  pc-bios/s390-ccw/main.c   |  15 +++--
  pc-bios/s390-ccw/menu.c   |  25 -
  pc-bios/s390-ccw/netmain.c    |  15 +
  pc-bios/s390-ccw/sclp.c   |   2 +-
  pc-bios/s390-ccw/virtio-blkdev.c  |   1 -
  pc-bios/s390-ccw/virtio-scsi.c    |   2 +-
  pc-bios/s390-ccw/virtio.c |   2 +-
  pc-bios/meson.build   |   1 -
  pc-bios/s390-ccw/Makefile |  69 +++
  pc-bios/s390-netboot.img  | Bin 67232 -> 0 bytes


Shouldnt you also update the s390-ccw.img file?


Sure, but I didn't want to spam the mailing list with a binary blob as long 
as this is not final yet (not sure whether we want to commit this separately 
or wait 'til Jared finished his series, too). Sorry, I should have mentioned 
it in the cover letter.


 Thomas




Re: How to use designware-root-port and designware-root-host devices ?

2024-06-21 Thread Thomas Huth

On 21/06/2024 11.16, Peter Maydell wrote:

On Fri, 21 Jun 2024 at 08:07, Arthur Tumanyan  wrote:


Hi,

I just tried to run mcimx7d-sabre machine this way:

${HOME}/cosim/usr/local/bin/qemu-system-arm -M mcimx7d-sabre -m 2G \
-kernel ${HOME}/cosim-arm/buildroot/output/images/uImage \
 --initrd ${HOME}/cosim-arm/buildroot/output/images/rootfs.cpio.gz \
-nographic \
-net nic -net user

and it just prints this and do nothing: qemu-system-arm: warning: nic 
imx.enet.1 has no peer

Based on what I see in the mcimx7d-sabre.c , it configures just very basic 
things, no PCIe at all (may be I'm wrong ;) )


The machine model in mcimx7d-sabre.c creates the SoC object
(TYPE_FSL_IMX7). It's the code for that in hw/arm/fsl-imx7.c
that creates all the SoC devices including the PCIe controller.
(This structure is similar to real hardware where you have a
board, which has one or two chips on it like RAM but most of
the complicated stuff is inside the one big SoC chip.)


Is there any idea what goes wrong here ? Maybe someone has experience with 
running this machine ?


"No output" usually means "your guest kernel is not configured
correctly for this machine type", and/or possibly "you didn't
tell the kernel to output on the serial port".


By the way, it seems like we don't even have an avocado test for that 
machine available. Peter, do you know whether there is a kernel for this 
machine available somewhere that we could use for testing?


 Thomas





[PATCH 7/7] docs/system/s390x/bootdevices: Update the documentation about network booting

2024-06-21 Thread Thomas Huth
Remove the information about the separate s390-netboot.img from
the documentation.

Signed-off-by: Thomas Huth 
---
 docs/system/s390x/bootdevices.rst | 20 +++-
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/docs/system/s390x/bootdevices.rst 
b/docs/system/s390x/bootdevices.rst
index 1a7a18b43b..c97efb8fc0 100644
--- a/docs/system/s390x/bootdevices.rst
+++ b/docs/system/s390x/bootdevices.rst
@@ -82,23 +82,17 @@ Note that ``0`` can be used to boot the default entry.
 Booting from a network device
 -
 
-Beside the normal guest firmware (which is loaded from the file 
``s390-ccw.img``
-in the data directory of QEMU, or via the ``-bios`` option), QEMU ships with
-a small TFTP network bootloader firmware for virtio-net-ccw devices, too. This
-firmware is loaded from a file called ``s390-netboot.img`` in the QEMU data
-directory. In case you want to load it from a different filename instead,
-you can specify it via the ``-global s390-ipl.netboot_fw=filename``
-command line option.
-
-The ``bootindex`` property is especially important for booting via the network.
-If you don't specify the ``bootindex`` property here, the network bootloader
-firmware code won't get loaded into the guest memory so that the network boot
-will fail. For a successful network boot, try something like this::
+The firmware that ships with QEMU includes a small TFTP network bootloader
+for virtio-net-ccw devices.  The ``bootindex`` property is especially
+important for booting via the network. If you don't specify the ``bootindex``
+property here, the network bootloader won't be taken into consideration and
+the network boot will fail. For a successful network boot, try something
+like this::
 
  qemu-system-s390x -netdev user,id=n1,tftp=...,bootfile=... \
-device virtio-net-ccw,netdev=n1,bootindex=1
 
-The network bootloader firmware also has basic support for pxelinux.cfg-style
+The network bootloader also has basic support for pxelinux.cfg-style
 configuration files. See the `PXELINUX Configuration page
 <https://wiki.syslinux.org/wiki/index.php?title=PXELINUX#Configuration>`__
 for details how to set up the configuration file on your TFTP server.
-- 
2.45.2




[PATCH 5/7] hw/s390x: Remove the possibility to load the s390-netboot.img binary

2024-06-21 Thread Thomas Huth
Since the netboot code has now been merged into the main s390-ccw.img
binary, we don't need the separate s390-netboot.img anymore. Remove
it and the code that was responsible for loading it.

Signed-off-by: Thomas Huth 
---
 hw/s390x/ipl.h |  12 +++-
 hw/s390x/ipl.c |  55 -
 hw/s390x/s390-virtio-ccw.c |  10 ++-
 pc-bios/meson.build|   1 -
 pc-bios/s390-netboot.img   | Bin 67232 -> 0 bytes
 5 files changed, 6 insertions(+), 72 deletions(-)
 delete mode 100644 pc-bios/s390-netboot.img

diff --git a/hw/s390x/ipl.h b/hw/s390x/ipl.h
index 57cd125769..b2105b616a 100644
--- a/hw/s390x/ipl.h
+++ b/hw/s390x/ipl.h
@@ -134,11 +134,8 @@ void s390_ipl_clear_reset_request(void);
 /*
  * The QEMU IPL Parameters will be stored at absolute address
  * 204 (0xcc) which means it is 32-bit word aligned but not
- * double-word aligned.
- * Placement of data fields in this area must account for
- * their alignment needs. E.g., netboot_start_address must
- * have an offset of 4 + n * 8 bytes within the struct in order
- * to keep it double-word aligned.
+ * double-word aligned. Placement of 64-bit data fields in this
+ * area must account for their alignment needs.
  * The total size of the struct must never exceed 28 bytes.
  * This definition must be kept in sync with the definition
  * in pc-bios/s390-ccw/iplb.h.
@@ -146,9 +143,9 @@ void s390_ipl_clear_reset_request(void);
 struct QemuIplParameters {
 uint8_t  qipl_flags;
 uint8_t  reserved1[3];
-uint64_t netboot_start_addr;
+uint64_t reserved2;
 uint32_t boot_menu_timeout;
-uint8_t  reserved2[12];
+uint8_t  reserved3[12];
 } QEMU_PACKED;
 typedef struct QemuIplParameters QemuIplParameters;
 
@@ -178,7 +175,6 @@ struct S390IPLState {
 char *initrd;
 char *cmdline;
 char *firmware;
-char *netboot_fw;
 uint8_t cssid;
 uint8_t ssid;
 uint16_t devno;
diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index 9362de0b6f..8a0a3e6961 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -288,7 +288,6 @@ static Property s390_ipl_properties[] = {
 DEFINE_PROP_STRING("initrd", S390IPLState, initrd),
 DEFINE_PROP_STRING("cmdline", S390IPLState, cmdline),
 DEFINE_PROP_STRING("firmware", S390IPLState, firmware),
-DEFINE_PROP_STRING("netboot_fw", S390IPLState, netboot_fw),
 DEFINE_PROP_BOOL("enforce_bios", S390IPLState, enforce_bios, false),
 DEFINE_PROP_BOOL("iplbext_migration", S390IPLState, iplbext_migration,
  true),
@@ -480,56 +479,6 @@ int s390_ipl_set_loadparm(uint8_t *loadparm)
 return -1;
 }
 
-static int load_netboot_image(Error **errp)
-{
-MachineState *ms = MACHINE(qdev_get_machine());
-S390IPLState *ipl = get_ipl_device();
-char *netboot_filename;
-MemoryRegion *sysmem =  get_system_memory();
-MemoryRegion *mr = NULL;
-void *ram_ptr = NULL;
-int img_size = -1;
-
-mr = memory_region_find(sysmem, 0, 1).mr;
-if (!mr) {
-error_setg(errp, "Failed to find memory region at address 0");
-return -1;
-}
-
-ram_ptr = memory_region_get_ram_ptr(mr);
-if (!ram_ptr) {
-error_setg(errp, "No RAM found");
-goto unref_mr;
-}
-
-netboot_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, ipl->netboot_fw);
-if (netboot_filename == NULL) {
-error_setg(errp, "Could not find network bootloader '%s'",
-   ipl->netboot_fw);
-goto unref_mr;
-}
-
-img_size = load_elf_ram(netboot_filename, NULL, NULL, NULL,
->start_addr,
-NULL, NULL, NULL, 1, EM_S390, 0, 0, NULL,
-false);
-
-if (img_size < 0) {
-img_size = load_image_size(netboot_filename, ram_ptr, ms->ram_size);
-ipl->start_addr = KERN_IMAGE_START;
-}
-
-if (img_size < 0) {
-error_setg(errp, "Failed to load network bootloader");
-}
-
-g_free(netboot_filename);
-
-unref_mr:
-memory_region_unref(mr);
-return img_size;
-}
-
 static bool is_virtio_ccw_device_of_type(IplParameterBlock *iplb,
  int virtio_id)
 {
@@ -754,10 +703,6 @@ void s390_ipl_prepare_cpu(S390CPU *cpu)
 ipl->iplb_valid = s390_gen_initial_iplb(ipl);
 }
 }
-if (ipl->netboot) {
-load_netboot_image(_fatal);
-ipl->qipl.netboot_start_addr = cpu_to_be64(ipl->start_addr);
-}
 s390_ipl_set_boot_menu(ipl);
 s390_ipl_prepare_qipl(cpu);
 }
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 3d0bc3e7f2..779366d551 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -186,11 +186,10 @@ static void s390_memory_init(MemoryRegion *ram)
 static void s390_init_ipl_dev(const char *kernel_filename,

[PATCH 3/7] pc-bios/s390-ccw: Use the libc from SLOF for the main s390-ccw.img binary, too

2024-06-21 Thread Thomas Huth
We are already using the libc from SLOF for the s390-netboot.img, and
this libc implementation is way more complete and accurate than the
simple implementation that we currently use for the s390-ccw.img binary.
Since we are now always assuming that the SLOF submodule is available
when building the s390-ccw bios (see commit bf6903f6944f), we can drop
the simple implementation and use the SLOF libc for the s390-ccw.img
binary, too.

Signed-off-by: Thomas Huth 
---
 pc-bios/s390-ccw/netboot.mak |  3 --
 pc-bios/s390-ccw/libc.h  | 89 
 pc-bios/s390-ccw/s390-ccw.h  |  7 +--
 pc-bios/s390-ccw/bootmap.c   |  2 +-
 pc-bios/s390-ccw/cio.c   |  2 +-
 pc-bios/s390-ccw/dasd-ipl.c  |  2 +-
 pc-bios/s390-ccw/jump2ipl.c  |  2 +-
 pc-bios/s390-ccw/libc.c  | 88 ---
 pc-bios/s390-ccw/main.c  |  5 +-
 pc-bios/s390-ccw/menu.c  | 25 -
 pc-bios/s390-ccw/sclp.c  |  2 +-
 pc-bios/s390-ccw/virtio-blkdev.c |  1 -
 pc-bios/s390-ccw/virtio-scsi.c   |  2 +-
 pc-bios/s390-ccw/virtio.c|  2 +-
 pc-bios/s390-ccw/Makefile| 15 --
 15 files changed, 34 insertions(+), 213 deletions(-)
 delete mode 100644 pc-bios/s390-ccw/libc.h
 delete mode 100644 pc-bios/s390-ccw/libc.c

diff --git a/pc-bios/s390-ccw/netboot.mak b/pc-bios/s390-ccw/netboot.mak
index 046aa35587..d2b3d8ee74 100644
--- a/pc-bios/s390-ccw/netboot.mak
+++ b/pc-bios/s390-ccw/netboot.mak
@@ -1,9 +1,6 @@
 
-SLOF_DIR := $(SRC_PATH)/../../roms/SLOF
-
 NETOBJS := start.o sclp.o cio.o virtio.o virtio-net.o jump2ipl.o netmain.o
 
-LIBC_INC := -nostdinc -I$(SLOF_DIR)/lib/libc/include
 LIBNET_INC := -I$(SLOF_DIR)/lib/libnet
 
 NETLDFLAGS := $(LDFLAGS) -Wl,-Ttext=0x780
diff --git a/pc-bios/s390-ccw/libc.h b/pc-bios/s390-ccw/libc.h
deleted file mode 100644
index bcdc45732d..00
--- a/pc-bios/s390-ccw/libc.h
+++ /dev/null
@@ -1,89 +0,0 @@
-/*
- * libc-style definitions and functions
- *
- * Copyright (c) 2013 Alexander Graf 
- *
- * This code is free software; you can redistribute it and/or modify it
- * under the terms of the GNU General Public License as published by the
- * Free Software Foundation; either version 2 of the License, or (at your
- * option) any later version.
- */
-
-#ifndef S390_CCW_LIBC_H
-#define S390_CCW_LIBC_H
-
-typedef unsigned long  size_t;
-typedef intbool;
-typedef unsigned char  uint8_t;
-typedef unsigned short uint16_t;
-typedef unsigned int   uint32_t;
-typedef unsigned long long uint64_t;
-
-static inline void *memset(void *s, int c, size_t n)
-{
-size_t i;
-unsigned char *p = s;
-
-for (i = 0; i < n; i++) {
-p[i] = c;
-}
-
-return s;
-}
-
-static inline void *memcpy(void *s1, const void *s2, size_t n)
-{
-uint8_t *dest = s1;
-const uint8_t *src = s2;
-size_t i;
-
-for (i = 0; i < n; i++) {
-dest[i] = src[i];
-}
-
-return s1;
-}
-
-static inline int memcmp(const void *s1, const void *s2, size_t n)
-{
-size_t i;
-const uint8_t *p1 = s1, *p2 = s2;
-
-for (i = 0; i < n; i++) {
-if (p1[i] != p2[i]) {
-return p1[i] > p2[i] ? 1 : -1;
-}
-}
-
-return 0;
-}
-
-static inline size_t strlen(const char *str)
-{
-size_t i;
-for (i = 0; *str; i++) {
-str++;
-}
-return i;
-}
-
-static inline char *strcat(char *dest, const char *src)
-{
-int i;
-char *dest_end = dest + strlen(dest);
-
-for (i = 0; i <= strlen(src); i++) {
-dest_end[i] = src[i];
-}
-return dest;
-}
-
-static inline int isdigit(int c)
-{
-return (c >= '0') && (c <= '9');
-}
-
-uint64_t atoui(const char *str);
-char *uitoa(uint64_t num, char *str, size_t len);
-
-#endif
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index c977a52b50..b911ebe6d2 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -13,6 +13,10 @@
 
 /* #define DEBUG */
 
+#include 
+#include 
+#include 
+
 typedef unsigned char  u8;
 typedef unsigned short u16;
 typedef unsigned int   u32;
@@ -26,9 +30,6 @@ typedef unsigned long long u64;
 #define EBUSY   2
 #define ENODEV  3
 
-#ifndef NULL
-#define NULL0
-#endif
 #ifndef MIN
 #define MIN(a, b) (((a) < (b)) ? (a) : (b))
 #endif
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index a2137449dc..bf85a9c5fe 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -8,7 +8,7 @@
  * directory.
  */
 
-#include "libc.h"
+#include 
 #include "s390-ccw.h"
 #include "s390-arch.h"
 #include "bootmap.h"
diff --git a/pc-bios/s390-ccw/cio.c b/pc-bios/s390-ccw/cio.c
index 83ca27ab41..11f0387ff4 100644
--- a/pc-bios/s390-ccw/cio.c
+++ b/pc-bios/s390-ccw/cio.c
@@ -11,7 +11,7 @@
  * directory.
  */
 
-#include "libc.h"
+#include 
 #include "s390-ccw.h"
 #include &q

[PATCH 4/7] pc-bios/s390-ccw: Link the netboot code into the main s390-ccw.img binary

2024-06-21 Thread Thomas Huth
We originally built a separate binary for the netboot code since it
was considered as experimental and we could not be sure that the
necessary SLOF module had been checked out. Time passed, the code
proved its usefulness, and the build system nowadays makes sure that
the SLOF module is checked out if you have a s390x compiler available
for building the s390-ccw bios. So there is no real compelling reason
anymore to keep the netboot code in a separate binary. Linking the
code together with the main s390-ccw.img will make future enhancements
much easier, like supporting more than one boot device.

Signed-off-by: Thomas Huth 
---
 pc-bios/s390-ccw/netboot.mak | 14 --
 pc-bios/s390-ccw/cio.h   |  2 ++
 pc-bios/s390-ccw/iplb.h  |  4 ++--
 pc-bios/s390-ccw/s390-ccw.h  |  3 +++
 pc-bios/s390-ccw/virtio.h|  1 -
 pc-bios/s390-ccw/bootmap.c   |  2 +-
 pc-bios/s390-ccw/main.c  | 10 +++---
 pc-bios/s390-ccw/netmain.c   | 15 ++-
 pc-bios/s390-ccw/Makefile| 13 +++--
 9 files changed, 24 insertions(+), 40 deletions(-)

diff --git a/pc-bios/s390-ccw/netboot.mak b/pc-bios/s390-ccw/netboot.mak
index d2b3d8ee74..0a24257ff4 100644
--- a/pc-bios/s390-ccw/netboot.mak
+++ b/pc-bios/s390-ccw/netboot.mak
@@ -1,18 +1,4 @@
 
-NETOBJS := start.o sclp.o cio.o virtio.o virtio-net.o jump2ipl.o netmain.o
-
-LIBNET_INC := -I$(SLOF_DIR)/lib/libnet
-
-NETLDFLAGS := $(LDFLAGS) -Wl,-Ttext=0x780
-
-$(NETOBJS): EXTRA_CFLAGS += $(LIBC_INC) $(LIBNET_INC)
-
-s390-netboot.elf: $(NETOBJS) libnet.a libc.a
-   $(call quiet-command,$(CC) $(NETLDFLAGS) -o $@ $^,Linking)
-
-s390-netboot.img: s390-netboot.elf
-   $(call quiet-command,$(STRIP) --strip-unneeded $< -o $@,Stripping $< 
into)
-
 # libc files:
 
 LIBC_CFLAGS = $(EXTRA_CFLAGS) $(CFLAGS) $(LIBC_INC) $(LIBNET_INC) \
diff --git a/pc-bios/s390-ccw/cio.h b/pc-bios/s390-ccw/cio.h
index 8b18153deb..6a5e86ba01 100644
--- a/pc-bios/s390-ccw/cio.h
+++ b/pc-bios/s390-ccw/cio.h
@@ -361,6 +361,8 @@ typedef struct CcwSearchIdData {
 uint8_t record;
 } __attribute__((packed)) CcwSearchIdData;
 
+extern SubChannelId net_schid;
+
 int enable_mss_facility(void);
 void enable_subchannel(SubChannelId schid);
 uint16_t cu_type(SubChannelId schid);
diff --git a/pc-bios/s390-ccw/iplb.h b/pc-bios/s390-ccw/iplb.h
index cb6ac8a880..3758698468 100644
--- a/pc-bios/s390-ccw/iplb.h
+++ b/pc-bios/s390-ccw/iplb.h
@@ -87,9 +87,9 @@ extern IplParameterBlock iplb 
__attribute__((__aligned__(PAGE_SIZE)));
 struct QemuIplParameters {
 uint8_t  qipl_flags;
 uint8_t  reserved1[3];
-uint64_t netboot_start_addr;
+uint64_t reserved2;
 uint32_t boot_menu_timeout;
-uint8_t  reserved2[12];
+uint8_t  reserved3[12];
 } __attribute__ ((packed));
 typedef struct QemuIplParameters QemuIplParameters;
 
diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index b911ebe6d2..b960f85b90 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -54,6 +54,9 @@ void write_iplb_location(void);
 unsigned int get_loadparm_index(void);
 void main(void);
 
+/* netmain.c */
+void netmain(void);
+
 /* sclp.c */
 void sclp_print(const char *string);
 void sclp_set_write_mask(uint32_t receive_mask, uint32_t send_mask);
diff --git a/pc-bios/s390-ccw/virtio.h b/pc-bios/s390-ccw/virtio.h
index 85bd9d1695..6f9a558ff5 100644
--- a/pc-bios/s390-ccw/virtio.h
+++ b/pc-bios/s390-ccw/virtio.h
@@ -253,7 +253,6 @@ struct VDev {
 uint8_t scsi_dev_heads;
 bool scsi_device_selected;
 ScsiDevice selected_scsi_device;
-uint64_t netboot_start_addr;
 uint32_t max_transfer;
 uint32_t guest_features[2];
 };
diff --git a/pc-bios/s390-ccw/bootmap.c b/pc-bios/s390-ccw/bootmap.c
index bf85a9c5fe..3c56d547ca 100644
--- a/pc-bios/s390-ccw/bootmap.c
+++ b/pc-bios/s390-ccw/bootmap.c
@@ -928,7 +928,7 @@ void zipl_load(void)
 }
 
 if (virtio_get_device_type() == VIRTIO_ID_NET) {
-jump_to_IPL_code(vdev->netboot_start_addr);
+netmain();
 }
 
 ipl_scsi();
diff --git a/pc-bios/s390-ccw/main.c b/pc-bios/s390-ccw/main.c
index b8bf9f5fcc..558c26c21b 100644
--- a/pc-bios/s390-ccw/main.c
+++ b/pc-bios/s390-ccw/main.c
@@ -37,8 +37,13 @@ LowCore *lowcore; /* Yes, this *is* a pointer to address 0 */
  */
 void write_subsystem_identification(void)
 {
-lowcore->subchannel_id = blk_schid.sch_id;
-lowcore->subchannel_nr = blk_schid.sch_no;
+if (cutype == CU_TYPE_VIRTIO && virtio_get_device_type() == VIRTIO_ID_NET) 
{
+lowcore->subchannel_id = net_schid.sch_id;
+lowcore->subchannel_nr = net_schid.sch_no;
+} else {
+lowcore->subchannel_id = blk_schid.sch_id;
+lowcore->subchannel_nr = blk_schid.sch_no;
+}
 lowcore->io_int_parm = 0;
 }
 
@@ -230,7 +235,6 @@ static int virtio_setup(void)
 switch (vdev->senseid.cu_model) {
 case VIRTIO_ID_NET:
 sclp_print("Network boot device detected\n");
-   

[PATCH 2/7] hw/s390x/ipl: Provide more memory to the s390-ccw.img firmware

2024-06-21 Thread Thomas Huth
We are going to link the SLOF libc into the s390-ccw.img, and this
libc needs more memory for providing space for malloc() and friends.
Thus bump the memory size that we reserve for the bios to 3 MiB
instead of only 2 MiB. While we're at it, add a proper check that
there is really enough memory assigned to the machine before blindly
using it.

Signed-off-by: Thomas Huth 
---
 hw/s390x/ipl.c | 10 +-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/hw/s390x/ipl.c b/hw/s390x/ipl.c
index e934bf89d1..9362de0b6f 100644
--- a/hw/s390x/ipl.c
+++ b/hw/s390x/ipl.c
@@ -45,6 +45,7 @@
 #define INITRD_PARM_START   0x010408UL
 #define PARMFILE_START  0x001000UL
 #define ZIPL_IMAGE_START0x009000UL
+#define BIOS_MAX_SIZE   0x30UL
 #define IPL_PSW_MASK(PSW_MASK_32 | PSW_MASK_64)
 
 static bool iplb_extended_needed(void *opaque)
@@ -144,7 +145,14 @@ static void s390_ipl_realize(DeviceState *dev, Error 
**errp)
  * even if an external kernel has been defined.
  */
 if (!ipl->kernel || ipl->enforce_bios) {
-uint64_t fwbase = (MIN(ms->ram_size, 0x8000U) - 0x20) & 
~0xUL;
+uint64_t fwbase;
+
+if (ms->ram_size < BIOS_MAX_SIZE) {
+error_setg(errp, "not enough RAM to load the BIOS file");
+return;
+}
+
+fwbase = (MIN(ms->ram_size, 0x8000U) - BIOS_MAX_SIZE) & ~0xUL;
 
 bios_filename = qemu_find_file(QEMU_FILE_TYPE_BIOS, ipl->firmware);
 if (bios_filename == NULL) {
-- 
2.45.2




[PATCH 6/7] pc-bios/s390-ccw: Merge netboot.mak into the main Makefile

2024-06-21 Thread Thomas Huth
Now that the netboot code has been merged into the main s390-ccw.img,
it also does not make sense to keep the build rules in a separate
file. Thus let's merge netboot.mak into the main Makefile.

Signed-off-by: Thomas Huth 
---
 pc-bios/s390-ccw/netboot.mak | 45 --
 pc-bios/s390-ccw/Makefile| 47 +++-
 2 files changed, 46 insertions(+), 46 deletions(-)
 delete mode 100644 pc-bios/s390-ccw/netboot.mak

diff --git a/pc-bios/s390-ccw/netboot.mak b/pc-bios/s390-ccw/netboot.mak
deleted file mode 100644
index 0a24257ff4..00
--- a/pc-bios/s390-ccw/netboot.mak
+++ /dev/null
@@ -1,45 +0,0 @@
-
-# libc files:
-
-LIBC_CFLAGS = $(EXTRA_CFLAGS) $(CFLAGS) $(LIBC_INC) $(LIBNET_INC) \
- -MMD -MP -MT $@ -MF $(@:%.o=%.d)
-
-CTYPE_OBJS = isdigit.o isxdigit.o toupper.o
-%.o : $(SLOF_DIR)/lib/libc/ctype/%.c
-   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
-
-STRING_OBJS = strcat.o strchr.o strrchr.o strcpy.o strlen.o strncpy.o \
- strcmp.o strncmp.o strcasecmp.o strncasecmp.o strstr.o \
- memset.o memcpy.o memmove.o memcmp.o
-%.o : $(SLOF_DIR)/lib/libc/string/%.c
-   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
-
-STDLIB_OBJS = atoi.o atol.o strtoul.o strtol.o rand.o malloc.o free.o
-%.o : $(SLOF_DIR)/lib/libc/stdlib/%.c
-   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
-
-STDIO_OBJS = sprintf.o snprintf.o vfprintf.o vsnprintf.o vsprintf.o fprintf.o \
-printf.o putc.o puts.o putchar.o stdchnls.o fileno.o
-%.o : $(SLOF_DIR)/lib/libc/stdio/%.c
-   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
-
-sbrk.o: $(SLOF_DIR)/slof/sbrk.c
-   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
-
-LIBCOBJS := $(STRING_OBJS) $(CTYPE_OBJS) $(STDLIB_OBJS) $(STDIO_OBJS) sbrk.o
-
-libc.a: $(LIBCOBJS)
-   $(call quiet-command,$(AR) -rc $@ $^,Creating static library)
-
-# libnet files:
-
-LIBNETOBJS := args.o dhcp.o dns.o icmpv6.o ipv6.o tcp.o udp.o bootp.o \
- dhcpv6.o ethernet.o ipv4.o ndp.o tftp.o pxelinux.o
-LIBNETCFLAGS = $(EXTRA_CFLAGS) $(CFLAGS) $(LIBC_INC) $(LIBNET_INC) \
-  -DDHCPARCH=0x1F -MMD -MP -MT $@ -MF $(@:%.o=%.d)
-
-%.o : $(SLOF_DIR)/lib/libnet/%.c
-   $(call quiet-command,$(CC) $(LIBNETCFLAGS) -c -o $@ $<,Compiling)
-
-libnet.a: $(LIBNETOBJS)
-   $(call quiet-command,$(AR) -rc $@ $^,Creating static library)
diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index cf6859823a..27cbb354af 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -61,7 +61,52 @@ config-cc.mak: Makefile
$(call cc-option,-march=z900,-march=z10)) 3> config-cc.mak
 -include config-cc.mak
 
-include $(SRC_PATH)/netboot.mak
+# libc files:
+
+LIBC_CFLAGS = $(EXTRA_CFLAGS) $(CFLAGS) $(LIBC_INC) $(LIBNET_INC) \
+ -MMD -MP -MT $@ -MF $(@:%.o=%.d)
+
+CTYPE_OBJS = isdigit.o isxdigit.o toupper.o
+%.o : $(SLOF_DIR)/lib/libc/ctype/%.c
+   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
+
+STRING_OBJS = strcat.o strchr.o strrchr.o strcpy.o strlen.o strncpy.o \
+ strcmp.o strncmp.o strcasecmp.o strncasecmp.o strstr.o \
+ memset.o memcpy.o memmove.o memcmp.o
+%.o : $(SLOF_DIR)/lib/libc/string/%.c
+   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
+
+STDLIB_OBJS = atoi.o atol.o strtoul.o strtol.o rand.o malloc.o free.o
+%.o : $(SLOF_DIR)/lib/libc/stdlib/%.c
+   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
+
+STDIO_OBJS = sprintf.o snprintf.o vfprintf.o vsnprintf.o vsprintf.o fprintf.o \
+printf.o putc.o puts.o putchar.o stdchnls.o fileno.o
+%.o : $(SLOF_DIR)/lib/libc/stdio/%.c
+   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
+
+sbrk.o: $(SLOF_DIR)/slof/sbrk.c
+   $(call quiet-command,$(CC) $(LIBC_CFLAGS) -c -o $@ $<,Compiling)
+
+LIBCOBJS := $(STRING_OBJS) $(CTYPE_OBJS) $(STDLIB_OBJS) $(STDIO_OBJS) sbrk.o
+
+libc.a: $(LIBCOBJS)
+   $(call quiet-command,$(AR) -rc $@ $^,Creating static library)
+
+# libnet files:
+
+LIBNETOBJS := args.o dhcp.o dns.o icmpv6.o ipv6.o tcp.o udp.o bootp.o \
+ dhcpv6.o ethernet.o ipv4.o ndp.o tftp.o pxelinux.o
+LIBNETCFLAGS = $(EXTRA_CFLAGS) $(CFLAGS) $(LIBC_INC) $(LIBNET_INC) \
+  -DDHCPARCH=0x1F -MMD -MP -MT $@ -MF $(@:%.o=%.d)
+
+%.o : $(SLOF_DIR)/lib/libnet/%.c
+   $(call quiet-command,$(CC) $(LIBNETCFLAGS) -c -o $@ $<,Compiling)
+
+libnet.a: $(LIBNETOBJS)
+   $(call quiet-command,$(AR) -rc $@ $^,Creating static library)
+
+# Main targets:
 
 build-all: s390-ccw.img
 
-- 
2.45.2




[PATCH 1/7] pc-bios/s390-ccw: Remove duplicated LDFLAGS

2024-06-21 Thread Thomas Huth
The -Wl,-pie and -nostdlib flags are added to LDFLAGS twice. Merge
the two lines to get rid of the duplicates.

Signed-off-by: Thomas Huth 
---
 pc-bios/s390-ccw/Makefile | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/pc-bios/s390-ccw/Makefile b/pc-bios/s390-ccw/Makefile
index acfcd1e71a..6207911b53 100644
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -40,7 +40,7 @@ EXTRA_CFLAGS += -ffreestanding 
-fno-delete-null-pointer-checks -fno-common -fPIE
 EXTRA_CFLAGS += -fwrapv -fno-strict-aliasing -fno-asynchronous-unwind-tables
 EXTRA_CFLAGS += -msoft-float
 EXTRA_CFLAGS += -std=gnu99
-LDFLAGS += -Wl,-pie -nostdlib
+LDFLAGS += -Wl,-pie -nostdlib -z noexecstack
 
 cc-test = $(CC) -Werror $1 -c -o /dev/null -xc /dev/null >/dev/null 2>/dev/null
 cc-option = if $(call cc-test, $1); then \
@@ -55,8 +55,6 @@ config-cc.mak: Makefile
$(call cc-option,-march=z900,-march=z10)) 3> config-cc.mak
 -include config-cc.mak
 
-LDFLAGS += -Wl,-pie -nostdlib -z noexecstack
-
 build-all: s390-ccw.img s390-netboot.img
 
 s390-ccw.elf: $(OBJECTS)
-- 
2.45.2




[PATCH 0/7] pc-bios/s390-ccw: Merge the netboot loader into s390-ccw.img

2024-06-21 Thread Thomas Huth
We originally built a separate binary for the netboot code since it
was considered as experimental and we could not be sure that the
necessary SLOF module had been checked out. Time passed, the netboot
code proved its usefulness, and the build system nowadays makes sure
that the SLOF module is checked out if you have a s390x compiler available
for building the s390-ccw bios. In fact, the possibility to build the
s390-ccw.img without s390-netboot.img has been removed in commit
bf6903f6944f ("pc-bios/s390-ccw: always build network bootloader")
already.

So it does not make too much sense anymore to keep the netboot code
in a separate binary. To make it easier to support a more flexible
boot process soon that supports more than one boot device via the
bootindex properties, let's finally merge the netboot code into the
main s390-ccw.img binary now.

Thomas Huth (7):
  pc-bios/s390-ccw: Remove duplicated LDFLAGS
  hw/s390x/ipl: Provide more memory to the s390-ccw.img firmware
  pc-bios/s390-ccw: Use the libc from SLOF for the main s390-ccw.img
binary, too
  pc-bios/s390-ccw: Link the netboot code into the main s390-ccw.img
binary
  hw/s390x: Remove the possibility to load the s390-netboot.img binary
  pc-bios/s390-ccw: Merge netboot.mak into the main Makefile
  docs/system/s390x/bootdevices: Update the documentation about network
booting

 docs/system/s390x/bootdevices.rst |  20 +++
 pc-bios/s390-ccw/netboot.mak  |  62 -
 hw/s390x/ipl.h|  12 ++--
 pc-bios/s390-ccw/cio.h|   2 +
 pc-bios/s390-ccw/iplb.h   |   4 +-
 pc-bios/s390-ccw/libc.h   |  89 --
 pc-bios/s390-ccw/s390-ccw.h   |  10 +++-
 pc-bios/s390-ccw/virtio.h |   1 -
 hw/s390x/ipl.c|  65 +++---
 hw/s390x/s390-virtio-ccw.c|  10 +---
 pc-bios/s390-ccw/bootmap.c|   4 +-
 pc-bios/s390-ccw/cio.c|   2 +-
 pc-bios/s390-ccw/dasd-ipl.c   |   2 +-
 pc-bios/s390-ccw/jump2ipl.c   |   2 +-
 pc-bios/s390-ccw/libc.c   |  88 -
 pc-bios/s390-ccw/main.c   |  15 +++--
 pc-bios/s390-ccw/menu.c   |  25 -
 pc-bios/s390-ccw/netmain.c|  15 +
 pc-bios/s390-ccw/sclp.c   |   2 +-
 pc-bios/s390-ccw/virtio-blkdev.c  |   1 -
 pc-bios/s390-ccw/virtio-scsi.c|   2 +-
 pc-bios/s390-ccw/virtio.c |   2 +-
 pc-bios/meson.build   |   1 -
 pc-bios/s390-ccw/Makefile |  69 +++
 pc-bios/s390-netboot.img  | Bin 67232 -> 0 bytes
 25 files changed, 122 insertions(+), 383 deletions(-)
 delete mode 100644 pc-bios/s390-ccw/netboot.mak
 delete mode 100644 pc-bios/s390-ccw/libc.h
 delete mode 100644 pc-bios/s390-ccw/libc.c
 delete mode 100644 pc-bios/s390-netboot.img

-- 
2.45.2




Re: How to use designware-root-port and designware-root-host devices ?

2024-06-20 Thread Thomas Huth

On 20/06/2024 10.28, Arthur Tumanyan wrote:

Hi all,

My question may sound stupid, however...


 Hi Arthur,

no worries, the question how to use which device in QEMU can be quite tricky ;-)

Currently I'm trying to make 
available designware-root-{port,host} devices  in linux when I run it in qemu.


I try the following way to run:

qemu-system-arm -M virt -m 2G \
      -kernel images/Image \
      -append "rootwait root=/dev/vda ro" \
      -drive file=images/rootfs.ext2,format=raw,id=hd0 \
      -device designware-root-port,id=rp0,chassis=1,slot=0,bus=pcie.0,addr=1 \
      -device e1000,netdev=net0,mac=52:54:00:12:34:56,bus=rp0,addr=0 \
      -netdev user,id=net0

but it seems designware device is not enabled by default: qemu-system-arm: 
-device designware-root-port,id=rp0,chassis=1,slot=0,bus=pcie.0,addr=1: 
'designware-root-port' is not a valid device model name


Are you sure about the names?

$ grep -r 'designware' *
...
include/hw/pci-host/designware.h:#define TYPE_DESIGNWARE_PCIE_HOST 
"designware-pcie-host"
include/hw/pci-host/designware.h:#define TYPE_DESIGNWARE_PCIE_ROOT 
"designware-pcie-root"


when I enable it from Kconfig/meson.build it says the device is already 
registered and exits with abort().


 From the other hand the device is declared as non pluggable: 
dc->user_creatable = false;


Well, that means that you cannot use those with "-device". They can only be 
instantiated via the code that creates the machine.



Can you please help me to use designware-root-host/port devices ?


It seems like the i.MX7 SABRE machine is using this device, so instead of 
"-M virt", you could have a try with "-M mcimx7d-sabre" (and a kernel that 
supports this machine) instead.


 HTH,
  Thomas




Re: [PATCH v2 09/14] include/hw: temporarily disable deletion of versioned machine types

2024-06-20 Thread Thomas Huth

On 20/06/2024 18.57, Daniel P. Berrangé wrote:

The new deprecation and deletion policy for versioned machine types is
being introduced in QEMU 9.1.0.

Under the new policy a number of old machine types (any prior to 2.12)
would be liable for immediate deletion which would be a violation of our
historical deprecation and removal policy

Thus automatic deletions (by skipping QOM registration) are temporarily
gated on existance of the env variable "QEMU_DELETE_MACHINES" / QEMU
version number >= 10.1.0. This allows opt-in testing of the automatic
deletion logic, while activating it fully in QEMU >= 10.1.0.

This whole commit should be reverted in the 10.1.0 dev cycle or shortly
thereafter.

Signed-off-by: Daniel P. Berrangé 
---
  include/hw/boards.h | 19 ++-
  1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/include/hw/boards.h b/include/hw/boards.h
index 187ed76646..ef6f18f2c1 100644
--- a/include/hw/boards.h
+++ b/include/hw/boards.h
@@ -686,11 +686,28 @@ struct MachineState {
   * suitable period of time has passed, it will cause
   * execution of the method to return, avoiding registration
   * of the machine
+ *
+ * The new deprecation and deletion policy for versioned
+ * machine types was introduced in QEMU 9.1.0.
+ *
+ * Under the new policy a number of old machine types (any
+ * prior to 2.12) would be liable for immediate deletion
+ * which would be a violation of our historical deprecation
+ * and removal policy
+ *
+ * Thus deletions are temporarily gated on existance of
+ * the env variable "QEMU_DELETE_MACHINES" / QEMU version
+ * number >= 10.1.0. This gate can be deleted in the 10.1.0
+ * dev cycle
   */
  #define MACHINE_VER_DELETION(...) \
  do { \
  if (MACHINE_VER_SHOULD_DELETE(__VA_ARGS__)) { \
-return; \
+if (getenv("QEMU_DELETE_MACHINES") || \
+QEMU_VERSION_MAJOR > 10 || (QEMU_VERSION_MAJOR == 10 && \
+QEMU_VERSION_MINOR >= 1)) { \
+return; \
+} \
      } \
  } while (0)
  


Reviewed-by: Thomas Huth 




Re: [PATCH v2 09/12] plugins: add migration blocker

2024-06-20 Thread Thomas Huth

On 20/06/2024 17.22, Alex Bennée wrote:

If the plugin in controlling time there is some state that might be
missing from the plugin tracking it. Migration is unlikely to work in
this case so lets put a migration blocker in to let the user know if
they try.

Signed-off-by: Alex Bennée 
Suggested-by: "Dr. David Alan Gilbert" 
---
  plugins/api.c | 8 
  1 file changed, 8 insertions(+)

diff --git a/plugins/api.c b/plugins/api.c
index 4431a0ea7e..c4239153af 100644
--- a/plugins/api.c
+++ b/plugins/api.c
@@ -47,6 +47,8 @@
  #include "disas/disas.h"
  #include "plugin.h"
  #ifndef CONFIG_USER_ONLY
+#include "qapi/error.h"
+#include "migration/blocker.h"
  #include "exec/ram_addr.h"
  #include "qemu/plugin-memory.h"
  #include "hw/boards.h"
@@ -589,11 +591,17 @@ uint64_t qemu_plugin_u64_sum(qemu_plugin_u64 entry)
   * Time control
   */
  static bool has_control;
+Error *migration_blocker;
  
  const void *qemu_plugin_request_time_control(void)

  {
  if (!has_control) {
  has_control = true;
+#ifdef CONFIG_SOFTMMU
+error_setg(_blocker,
+   "TCG plugin time control does not support migration");
+migrate_add_blocker(_blocker, NULL);
+#endif
  return _control;
  }
  return NULL;


Reviewed-by: Thomas Huth 




Re: [PATCH 5/5] s390x: Enable and document boot device fallback on panic

2024-06-20 Thread Thomas Huth

On 17/06/2024 16.49, Christian Borntraeger wrote:



Am 05.06.24 um 15:37 schrieb Thomas Huth:

On 29/05/2024 17.43, jro...@linux.ibm.com wrote:

From: Jared Rossi 

On a panic during IPL (i.e. a device failed to boot) check for another 
device

to boot from, as indicated by the presence of an unused IPLB.

If an IPLB is successfully loaded, then jump to the start of BIOS, 
restarting

IPL using the updated IPLB.  Otherwise enter disabled wait.

Signed-off-by: Jared Rossi 
---
  docs/system/bootindex.rst | 7 ---
  docs/system/s390x/bootdevices.rst | 9 ++---
  pc-bios/s390-ccw/s390-ccw.h   | 6 ++
  3 files changed, 16 insertions(+), 6 deletions(-)


Could you please split the documentation changes into a separate patch in 
v2 ? ... I think that would be cleaner.



diff --git a/docs/system/bootindex.rst b/docs/system/bootindex.rst
index 8b057f812f..de597561bd 100644
--- a/docs/system/bootindex.rst
+++ b/docs/system/bootindex.rst
@@ -50,9 +50,10 @@ Limitations
  Some firmware has limitations on which devices can be considered for
  booting.  For instance, the PC BIOS boot specification allows only one
-disk to be bootable.  If boot from disk fails for some reason, the BIOS
-won't retry booting from other disk.  It can still try to boot from
-floppy or net, though.
+disk to be bootable, except for on s390x machines. If boot from disk 
fails for
+some reason, the BIOS won't retry booting from other disk.  It can still 
try to
+boot from floppy or net, though.  In the case of s390x, the BIOS will 
try up to

+8 total devices, any number of which may be disks.


Since the old text was already talking about "PC BIOS", I'd rather leave 
that paragraph as it is (maybe just replace "PC BIOS" with "x86 PC BIOS"), 
and add a separate paragraph afterwards about s390x instead.



diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index c977a52b50..de3d1f0d5a 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -43,6 +43,7 @@ typedef unsigned long long u64;
  #include "iplb.h"
  /* start.s */
+extern char _start[];
  void disabled_wait(void) __attribute__ ((__noreturn__));
  void consume_sclp_int(void);
  void consume_io_int(void);
@@ -88,6 +89,11 @@ __attribute__ ((__noreturn__))
  static inline void panic(const char *string)
  {
  sclp_print(string);
+    if (load_next_iplb()) {
+    sclp_print("\nTrying next boot device...");
+    jump_to_IPL_code((long)_start);
+    }
+
  disabled_wait();
  }


Honestly, I am unsure whether this is a really cool idea or a very ugly 
hack ... but I think I tend towards the latter, sorry. Jumping back to the 
startup code might cause various problem, e.g. pre-initialized variables 
don't get their values reset, causing different behavior when the s390-ccw 
bios runs a function a second time this way. 


We jump back to _start and to me it looks like that this code does the 
resetting of bss segment.
So anything that has a zero value this should be fine. But static variables 
!= 0 are indeed tricky.

As far as I can see we do have some of those :-(

So instead of jumping, is there a way that remember somewhere at which 
device we are and then trigger a re-ipl to reload the BIOS?


If there is an easy way, this could maybe an option, but in the long run, 
I'd really prefer if we'd merge the binaries and get rid of such tricks, 
since this makes the code flow quite hard to understand and maybe also more 
difficult to debug if you run into problems later.


 Thomas




Re: [PATCH 5/5] s390x: Enable and document boot device fallback on panic

2024-06-20 Thread Thomas Huth

On 17/06/2024 01.44, Jared Rossi wrote:



On 6/7/24 1:57 AM, Thomas Huth wrote:

On 05/06/2024 16.48, Jared Rossi wrote:



diff --git a/pc-bios/s390-ccw/s390-ccw.h b/pc-bios/s390-ccw/s390-ccw.h
index c977a52b50..de3d1f0d5a 100644
--- a/pc-bios/s390-ccw/s390-ccw.h
+++ b/pc-bios/s390-ccw/s390-ccw.h
@@ -43,6 +43,7 @@ typedef unsigned long long u64;
  #include "iplb.h"
    /* start.s */
+extern char _start[];
  void disabled_wait(void) __attribute__ ((__noreturn__));
  void consume_sclp_int(void);
  void consume_io_int(void);
@@ -88,6 +89,11 @@ __attribute__ ((__noreturn__))
  static inline void panic(const char *string)
  {
  sclp_print(string);
+    if (load_next_iplb()) {
+    sclp_print("\nTrying next boot device...");
+    jump_to_IPL_code((long)_start);
+    }
+
  disabled_wait();
  }


Honestly, I am unsure whether this is a really cool idea or a very ugly 
hack ... but I think I tend towards the latter, sorry. Jumping back to 
the startup code might cause various problem, e.g. pre-initialized 
variables don't get their values reset, causing different behavior when 
the s390-ccw bios runs a function a second time this way. Thus this 
sounds very fragile. Could we please try to get things cleaned up 
correctly, so that functions return with error codes instead of 
panicking when we can continue with another boot device? Even if its 
more work right now, I think this will be much more maintainable in the 
future.


 Thomas



Thanks Thomas, I appreciate your insight.  Your hesitation is perfectly 
understandable as well.  My initial design was like you suggest, where 
the functions return instead of panic, but the issue I ran into is that 
netboot uses a separate image, which we jump in to at the start of IPL 
from a network device (see zipl_load() in pc-bios/s390-ccw/bootmap.c). I 
wasn't able to come up with a simple way to return to the main BIOS code 
if a netboot fails other than by jumping back.  So, it seems to me that 
netboot kind of throws a monkeywrench into the basic idea of reworking 
the panics into returns.


I'm open to suggestions on a better way to recover from a failed netboot, 
and it's certainly possible I've overlooked something, but as far as I 
can tell a jump is necessary in that particular case at least. Netboot 
could perhaps be handled as a special case where the jump back is 
permitted whereas other device types return, but I don't think that 
actually solves the main issue.


What are your thoughts on this?


Yes, I agree that jumping is currently required to get back from the 
netboot code. So if you could rework your patches in a way that limits the 
jumping to a failed netboot, that would be acceptable, I think.


Apart from that: We originally decided to put the netboot code into a 
separate binary since the required roms/SLOF module might not always have 
been checked out (it needed to be done manually), so we were not able to 
compile it in all cases. But nowadays, this is handled in a much nicer 
way, the submodule is automatically checked out once you compile the 
s390x-softmmu target and have a s390x compiler available, so I wonder 
whether we should maybe do the next step and integrate the netboot code 
into the main s390-ccw.img now? Anybody got an opinion on this?


 Thomas



Hi Thomas,

I would generally defer the decision about integrating the netboot code to 
someone with more insight than me, but for what it's worth, I am of the 
opinion that if we want to rework all of panics into returns, then it would 
make the most sense to also do the integration now so that we can avoid 
using jump altogether.  Unless I'm missing something simple, I don't think 
the panic/return conversion will be trivial, and actually I think it will be 
quite invasive since there are dozens of calls to panic and assert that will 
need to be changed.   It doesn't seem worthwhile to do all of these 
conversions in order to avoid using jump, but then still being exposed to 
possible problems caused by jumping due to netboot requiring it anyway.


Agreed, we should either do it right and merge the two binaries, or it does 
not make too much sense to only partly convert the code.


I can look into merging the two binaries, but it might also take some time. 
So for the time being, I'm fine if we include the panic-jumping hack for 
now, we can still then clean it up later.


 Thomas




Re: [PATCH] configure: detect --cpu=mipsisa64r6

2024-06-19 Thread Thomas Huth

On 19/06/2024 15.34, Paolo Bonzini wrote:

On Wed, Jun 19, 2024 at 2:49 PM Thomas Huth  wrote:


On 19/06/2024 13.46, Paolo Bonzini wrote:

Treat it as a MIPS64 machine.

...

diff --git a/configure b/configure
index d0703ea279d..3669eec86e5 100755
--- a/configure
+++ b/configure
@@ -452,7 +452,7 @@ case "$cpu" in
   linux_arch=loongarch
   ;;

-  mips64*)
+  mips64*|mipsisa64*)


Maybe simply switch to mips*64*) ?


Not sure if it's a good idea, since we know the exact prefixes.


Fair point.

Reviewed-by: Thomas Huth 




Re: [PATCH] hw/core: Rename CpuTopology to CPUTopology

2024-06-19 Thread Thomas Huth

On 19/06/2024 16.49, Zhao Liu wrote:

Hi maintainers,

Per my communication with Markus, it seems this renaming matches the
"local consistency" principle in (include/hw/boards.h). :-)

So do you think this change is acceptable?


I don't care too much, both ways of naming look acceptable to me...
... but in case somebody else wants to merge this, FWIW:

s390x parts
Acked-by: Thomas Huth 



On Mon, May 27, 2024 at 09:18:37PM +0800, Zhao Liu wrote:

Date: Mon, 27 May 2024 21:18:37 +0800
From: Zhao Liu 
Subject: [PATCH] hw/core: Rename CpuTopology to CPUTopology
X-Mailer: git-send-email 2.34.1

Use CPUTopology to honor the generic style of CPU capitalization
abbreviations.

Signed-off-by: Zhao Liu 
---
  * Split from the previous SMP cache RFC:

https://lore.kernel.org/qemu-devel/20240220092504.726064-2-zhao1@linux.intel.com/
---
  hw/s390x/cpu-topology.c |  6 +++---
  include/hw/boards.h |  8 
  include/hw/s390x/cpu-topology.h |  6 +++---
  tests/unit/test-smp-parse.c | 14 +++---
  4 files changed, 17 insertions(+), 17 deletions(-)








[PATCH] hw/intc/s390_flic: Fix interrupt controller migration on s390x with TCG

2024-06-19 Thread Thomas Huth
Migration of a s390x guest with TCG was long known to be very unstable,
so the tests in tests/qtest/migration-test.c are disabled if running
with TCG instead of KVM.

Nicholas Piggin did a great analysis of the problem:

"The flic pending state is not migrated, so if the machine is migrated
 while an interrupt is pending, it can be lost. This shows up in
 qtest migration test, an extint is pending (due to console writes?)
 and the CPU waits via s390_cpu_set_psw and expects the interrupt to
 wake it. However when the flic pending state is lost, s390_cpu_has_int
 returns false, so s390_cpu_exec_interrupt falls through to halting
 again."

Thus let's finally migrate the pending state, and to be on the safe
side, also the other state variables of the QEMUS390FLICState structure.

Signed-off-by: Thomas Huth 
---
 Once this has been merged, we can enable the migration-test again
 with Nicholas' patch here:
 https://lore.kernel.org/qemu-devel/20240525131241.378473-3-npig...@gmail.com/

 include/hw/s390x/s390_flic.h |  1 +
 hw/intc/s390_flic.c  | 75 ++--
 hw/s390x/s390-virtio-ccw.c   |  5 +++
 3 files changed, 78 insertions(+), 3 deletions(-)

diff --git a/include/hw/s390x/s390_flic.h b/include/hw/s390x/s390_flic.h
index 382d9833f1..4d66c5e42e 100644
--- a/include/hw/s390x/s390_flic.h
+++ b/include/hw/s390x/s390_flic.h
@@ -116,6 +116,7 @@ struct QEMUS390FLICState {
 uint8_t simm;
 uint8_t nimm;
 QLIST_HEAD(, QEMUS390FlicIO) io[8];
+bool migrate_all_state;
 };
 
 uint32_t qemu_s390_flic_dequeue_service(QEMUS390FLICState *flic);
diff --git a/hw/intc/s390_flic.c b/hw/intc/s390_flic.c
index 6771645699..a91a4a47e8 100644
--- a/hw/intc/s390_flic.c
+++ b/hw/intc/s390_flic.c
@@ -361,15 +361,77 @@ bool ais_needed(void *opaque)
 return s->ais_supported;
 }
 
+static bool ais_needed_v(void *opaque, int version_id)
+{
+return ais_needed(opaque);
+}
+
+static bool qemu_s390_flic_full_state_needed(void *opaque)
+{
+QEMUS390FLICState *s = opaque;
+
+return s->migrate_all_state;
+}
+
+static bool qemu_s390_flic_state_needed(void *opaque)
+{
+return ais_needed(opaque) || qemu_s390_flic_full_state_needed(opaque);
+}
+
+static const VMStateDescription vmstate_qemu_s390_flic_io = {
+ .name = "qemu-s390-flic-io",
+ .version_id = 1,
+ .minimum_version_id = 1,
+ .fields = (const VMStateField[]) {
+ VMSTATE_UINT16(id, QEMUS390FlicIO),
+ VMSTATE_UINT16(nr, QEMUS390FlicIO),
+ VMSTATE_UINT32(parm, QEMUS390FlicIO),
+ VMSTATE_UINT32(word, QEMUS390FlicIO),
+ VMSTATE_END_OF_LIST()
+ },
+};
+
+static const VMStateDescription vmstate_qemu_s390_flic_full = {
+.name = "qemu-s390-flic-full",
+.version_id = 1,
+.minimum_version_id = 1,
+.needed = qemu_s390_flic_full_state_needed,
+.fields = (const VMStateField[]) {
+VMSTATE_UINT32(pending, QEMUS390FLICState),
+VMSTATE_UINT32(service_param, QEMUS390FLICState),
+VMSTATE_QLIST_V(io[0], QEMUS390FLICState, 1,
+vmstate_qemu_s390_flic_io, QEMUS390FlicIO, next),
+VMSTATE_QLIST_V(io[1], QEMUS390FLICState, 1,
+vmstate_qemu_s390_flic_io, QEMUS390FlicIO, next),
+VMSTATE_QLIST_V(io[2], QEMUS390FLICState, 1,
+vmstate_qemu_s390_flic_io, QEMUS390FlicIO, next),
+VMSTATE_QLIST_V(io[3], QEMUS390FLICState, 1,
+vmstate_qemu_s390_flic_io, QEMUS390FlicIO, next),
+VMSTATE_QLIST_V(io[4], QEMUS390FLICState, 1,
+vmstate_qemu_s390_flic_io, QEMUS390FlicIO, next),
+VMSTATE_QLIST_V(io[5], QEMUS390FLICState, 1,
+vmstate_qemu_s390_flic_io, QEMUS390FlicIO, next),
+VMSTATE_QLIST_V(io[6], QEMUS390FLICState, 1,
+vmstate_qemu_s390_flic_io, QEMUS390FlicIO, next),
+VMSTATE_QLIST_V(io[7], QEMUS390FLICState, 1,
+vmstate_qemu_s390_flic_io, QEMUS390FlicIO, next),
+VMSTATE_END_OF_LIST()
+}
+};
+
 static const VMStateDescription qemu_s390_flic_vmstate = {
 .name = "qemu-s390-flic",
 .version_id = 1,
 .minimum_version_id = 1,
-.needed = ais_needed,
+.needed = qemu_s390_flic_state_needed,
 .fields = (const VMStateField[]) {
-VMSTATE_UINT8(simm, QEMUS390FLICState),
-VMSTATE_UINT8(nimm, QEMUS390FLICState),
+VMSTATE_UINT8_TEST(simm, QEMUS390FLICState, ais_needed_v),
+VMSTATE_UINT8_TEST(nimm, QEMUS390FLICState, ais_needed_v),
 VMSTATE_END_OF_LIST()
+},
+.subsections = (const VMStateDescription * const []) {
+_qemu_s390_flic_full,
+NULL
 }
 };
 
@@ -383,11 +445,18 @@ static void qemu_s390_flic_instance_init(Object *obj)
 }
 }
 
+static Property qemu_s390_flic_properties[] = {
+DEFINE_PROP_BOOL("migrate-all-state", QEMUS390FLICState,
+   

Re: [PATCH] configure: detect --cpu=mipsisa64r6

2024-06-19 Thread Thomas Huth

On 19/06/2024 13.46, Paolo Bonzini wrote:

Treat it as a MIPS64 machine.


Where did you encounter it?


Signed-off-by: Paolo Bonzini 
---
  configure | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure b/configure
index d0703ea279d..3669eec86e5 100755
--- a/configure
+++ b/configure
@@ -452,7 +452,7 @@ case "$cpu" in
  linux_arch=loongarch
  ;;
  
-  mips64*)

+  mips64*|mipsisa64*)


Maybe simply switch to mips*64*) ?


  cpu=mips64
  host_arch=mips
  linux_arch=mips


 Thomas




Re: [PATCH 2/2] target/ppc/arch_dump: set prstatus pid to cpuid

2024-06-19 Thread Thomas Huth

On 19/06/2024 07.00, Omar Sandoval wrote:

Every other architecture does this, and debuggers need it to be able to
identify which prstatus note corresponds to which CPU.

Signed-off-by: Omar Sandoval 
---
  target/ppc/arch_dump.c | 21 -
  1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/target/ppc/arch_dump.c b/target/ppc/arch_dump.c
index a8315659d9..78b4205319 100644
--- a/target/ppc/arch_dump.c
+++ b/target/ppc/arch_dump.c
@@ -47,9 +47,11 @@ struct PPCUserRegStruct {
  } QEMU_PACKED;
  
  struct PPCElfPrstatus {

-char pad1[112];
+char pad1[32];
+uint32_t pid;
+uint8_t pad2[76];
  struct PPCUserRegStruct pr_reg;
-char pad2[40];
+char pad3[40];
  } QEMU_PACKED;
  
  
@@ -96,7 +98,7 @@ typedef struct NoteFuncArg {

  DumpState *state;
  } NoteFuncArg;
  
-static void ppc_write_elf_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu)

+static void ppc_write_elf_prstatus(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
  {
  int i;
  reg_t cr;
@@ -109,6 +111,7 @@ static void ppc_write_elf_prstatus(NoteFuncArg *arg, 
PowerPCCPU *cpu)
  
  prstatus = >contents.prstatus;

  memset(prstatus, 0, sizeof(*prstatus));
+prstatus->pid = cpu_to_dump32(s, id);
  reg = >pr_reg;
  
  for (i = 0; i < 32; i++) {

@@ -127,7 +130,7 @@ static void ppc_write_elf_prstatus(NoteFuncArg *arg, 
PowerPCCPU *cpu)
  reg->ccr = cpu_to_dump_reg(s, cr);
  }
  
-static void ppc_write_elf_fpregset(NoteFuncArg *arg, PowerPCCPU *cpu)

+static void ppc_write_elf_fpregset(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
  {
  int i;
  struct PPCElfFpregset  *fpregset;
@@ -146,7 +149,7 @@ static void ppc_write_elf_fpregset(NoteFuncArg *arg, 
PowerPCCPU *cpu)
  fpregset->fpscr = cpu_to_dump_reg(s, cpu->env.fpscr);
  }
  
-static void ppc_write_elf_vmxregset(NoteFuncArg *arg, PowerPCCPU *cpu)

+static void ppc_write_elf_vmxregset(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
  {
  int i;
  struct PPCElfVmxregset *vmxregset;
@@ -178,7 +181,7 @@ static void ppc_write_elf_vmxregset(NoteFuncArg *arg, 
PowerPCCPU *cpu)
  vmxregset->vscr.u32[3] = cpu_to_dump32(s, ppc_get_vscr(>env));
  }
  
-static void ppc_write_elf_vsxregset(NoteFuncArg *arg, PowerPCCPU *cpu)

+static void ppc_write_elf_vsxregset(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
  {
  int i;
  struct PPCElfVsxregset *vsxregset;
@@ -195,7 +198,7 @@ static void ppc_write_elf_vsxregset(NoteFuncArg *arg, 
PowerPCCPU *cpu)
  }
  }
  
-static void ppc_write_elf_speregset(NoteFuncArg *arg, PowerPCCPU *cpu)

+static void ppc_write_elf_speregset(NoteFuncArg *arg, PowerPCCPU *cpu, int id)
  {
  struct PPCElfSperegset *speregset;
  Note *note = >note;
@@ -211,7 +214,7 @@ static void ppc_write_elf_speregset(NoteFuncArg *arg, 
PowerPCCPU *cpu)
  
  static const struct NoteFuncDescStruct {

  int contents_size;
-void (*note_contents_func)(NoteFuncArg *arg, PowerPCCPU *cpu);
+void (*note_contents_func)(NoteFuncArg *arg, PowerPCCPU *cpu, int id);
  } note_func[] = {
  {sizeof_field(Note, contents.prstatus),  ppc_write_elf_prstatus},
  {sizeof_field(Note, contents.fpregset),  ppc_write_elf_fpregset},
@@ -282,7 +285,7 @@ static int ppc_write_all_elf_notes(const char *note_name,
  arg.note.hdr.n_descsz = cpu_to_dump32(s, nf->contents_size);
  strncpy(arg.note.name, note_name, sizeof(arg.note.name));
  
-(*nf->note_contents_func)(, cpu);

+(*nf->note_contents_func)(, cpu, id);
  
  note_size =

  sizeof(arg.note) - sizeof(arg.note.contents) + nf->contents_size;


Reviewed-by: Thomas Huth 




Re: [PATCH 1/2] target/s390x/arch_dump: use correct byte order for pid

2024-06-18 Thread Thomas Huth

On 19/06/2024 07.00, Omar Sandoval wrote:

The pid field of prstatus needs to be big endian like all of the other
fields.

Fixes: f738f296eaae ("s390x/arch_dump: pass cpuid into notes sections")
Signed-off-by: Omar Sandoval 
---
  target/s390x/arch_dump.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/s390x/arch_dump.c b/target/s390x/arch_dump.c
index 7e8a1b4fc0..029d91d93a 100644
--- a/target/s390x/arch_dump.c
+++ b/target/s390x/arch_dump.c
@@ -102,7 +102,7 @@ static void s390x_write_elf64_prstatus(Note *note, S390CPU 
*cpu, int id)
  regs->acrs[i] = cpu_to_be32(cpu->env.aregs[i]);
  regs->gprs[i] = cpu_to_be64(cpu->env.regs[i]);
  }
-note->contents.prstatus.pid = id;
+note->contents.prstatus.pid = cpu_to_be32(id);
  }
  
  static void s390x_write_elf64_fpregset(Note *note, S390CPU *cpu, int id)


Reviewed-by: Thomas Huth 




[PATCH] MAINTAINERS: Cover all tests/qtest/migration-* files

2024-06-18 Thread Thomas Huth
Beside migration-test.c, there is nowadays migration-helpers.[ch],
too, so update the entry in the migration section to also cover these
files now.
While we're at it, exclude these files in the common qtest section,
since the migration test is well covered by the migration maintainers
already. Since the test is under very active development, it was causing
a lot of distraction to the generic qtest maintainers with regards to
the patches that need to be reviewed by the migration maintainers anyway.

Signed-off-by: Thomas Huth 
---
 MAINTAINERS | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index 0f63bcdc7d..32691302db 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3316,6 +3316,7 @@ F: tests/qtest/
 F: docs/devel/qgraph.rst
 F: docs/devel/qtest.rst
 X: tests/qtest/bios-tables-test*
+X: tests/qtest/migration-*
 
 Device Fuzzing
 M: Alexander Bulekov 
@@ -3412,7 +3413,7 @@ F: include/qemu/userfaultfd.h
 F: migration/
 F: scripts/vmstate-static-checker.py
 F: tests/vmstate-static-checker-data/
-F: tests/qtest/migration-test.c
+F: tests/qtest/migration-*
 F: docs/devel/migration/
 F: qapi/migration.json
 F: tests/migration/
-- 
2.45.2




Re: [PATCH v2 0/2] target/s390x: Fix tracing header path in TCG mem_helper.c

2024-06-18 Thread Thomas Huth

On 13/06/2024 12.44, Philippe Mathieu-Daudé wrote:

In order to keep trace event headers local to their
directory, introduce s390_skeys_get/s390_skeys_set
helpers, fixing:

   In file included from ../../target/s390x/tcg/mem_helper.c:33:
   ../../target/s390x/tcg/trace.h:1:10: fatal error: 
'trace/trace-target_s390x_tcg.h' file not found
   #include "trace/trace-target_s390x_tcg.h"
^~~~
   1 error generated.
   ninja: build stopped: subcommand failed.

Philippe Mathieu-Daudé (2):
   hw/s390x: Introduce s390_skeys_get|set() helpers
   target/s390x: Use s390_skeys_get|set() helper

  include/hw/s390x/storage-keys.h | 10 ++
  hw/s390x/s390-skeys.c   | 27 +++
  target/s390x/mmu_helper.c   | 11 ++-
  target/s390x/tcg/mem_helper.c   | 16 
  hw/s390x/trace-events   |  4 
  target/s390x/trace-events   |  4 
  6 files changed, 47 insertions(+), 25 deletions(-)



Series
Reviewed-by: Thomas Huth 




[PATCH] hw/virtio: Fix the de-initialization of vhost-user devices

2024-06-18 Thread Thomas Huth
The unrealize functions of the various vhost-user devices are
calling the corresponding vhost_*_set_status() functions with a
status of 0 to shut down the device correctly.

Now these vhost_*_set_status() functions all follow this scheme:

bool should_start = virtio_device_should_start(vdev, status);

if (vhost_dev_is_started(>vhost_dev) == should_start) {
return;
}

if (should_start) {
/* ... do the initialization stuff ... */
} else {
/* ... do the cleanup stuff ... */
}

The problem here is virtio_device_should_start(vdev, 0) currently
always returns "true" since it internally only looks at vdev->started
instead of looking at the "status" parameter. Thus once the device
got started once, virtio_device_should_start() always returns true
and thus the vhost_*_set_status() functions return early, without
ever doing any clean-up when being called with status == 0. This
causes e.g. problems when trying to hot-plug and hot-unplug a vhost
user devices multiple times since the de-initialization step is
completely skipped during the unplug operation.

This bug has been introduced in commit 9f6bcfd99f ("hw/virtio: move
vm_running check to virtio_device_started") which replaced

 should_start = status & VIRTIO_CONFIG_S_DRIVER_OK;

with

 should_start = virtio_device_started(vdev, status);

which later got replaced by virtio_device_should_start(). This blocked
the possibility to set should_start to false in case the status flag
VIRTIO_CONFIG_S_DRIVER_OK was not set.

Fix it by adjusting the virtio_device_should_start() function to
only consider the status flag instead of vdev->started. Since this
function is only used in the various vhost_*_set_status() functions
for exactly the same purpose, it should be fine to fix it in this
central place there without any risk to change the behavior of other
code.

Fixes: 9f6bcfd99f ("hw/virtio: move vm_running check to virtio_device_started")
Buglink: https://issues.redhat.com/browse/RHEL-40708
Signed-off-by: Thomas Huth 
---
 include/hw/virtio/virtio.h | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index 7d5ffdc145..2eafad17b8 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -470,9 +470,9 @@ static inline bool virtio_device_started(VirtIODevice 
*vdev, uint8_t status)
  * @vdev - the VirtIO device
  * @status - the devices status bits
  *
- * This is similar to virtio_device_started() but also encapsulates a
- * check on the VM status which would prevent a device starting
- * anyway.
+ * This is similar to virtio_device_started() but ignores vdev->started
+ * and also encapsulates a check on the VM status which would prevent a
+ * device from starting anyway.
  */
 static inline bool virtio_device_should_start(VirtIODevice *vdev, uint8_t 
status)
 {
@@ -480,7 +480,7 @@ static inline bool virtio_device_should_start(VirtIODevice 
*vdev, uint8_t status
 return false;
 }
 
-return virtio_device_started(vdev, status);
+return status & VIRTIO_CONFIG_S_DRIVER_OK;
 }
 
 static inline void virtio_set_started(VirtIODevice *vdev, bool started)
-- 
2.45.2




Re: [RFC PATCH v2 1/2] Add RISC-V CSR qtest support

2024-06-18 Thread Thomas Huth

On 18/06/2024 08.44, Ivan Klokov wrote:

The RISC-V architecture supports the creation of custom
CSR-mapped devices. It would be convenient to test them in the same way
as MMIO-mapped devices. To do this, a new call has been added
to read/write CSR registers.

Signed-off-by: Ivan Klokov 
---

...

diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index 58ef7079dc..82540ae5dc 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -29,7 +29,7 @@
  #include "sysemu/cpu-timers.h"
  #include "qemu/guest-random.h"
  #include "qapi/error.h"
-
+#include "tests/qtest/libqtest.h"
  
  /* CSR function table public API */

  void riscv_get_csr_ops(int csrno, riscv_csr_operations *ops)
@@ -4549,6 +4549,53 @@ static RISCVException write_jvt(CPURISCVState *env, int 
csrno,
  return RISCV_EXCP_NONE;
  }
  
+static uint64_t csr_call(char *cmd, uint64_t cpu_num, int csrno,

+uint64_t *val)
+{
+RISCVCPU *cpu = RISCV_CPU(cpu_by_arch_id(cpu_num));
+CPURISCVState *env = >env;
+
+int ret = RISCV_EXCP_NONE;
+if (strcmp(cmd, "get_csr") == 0) {
+ret = riscv_csrrw(env, csrno, (target_ulong *)val, 0, 0);
+
+} else if (strcmp(cmd, "set_csr") == 0) {
+ret = riscv_csrrw(env, csrno, NULL, *(target_ulong *)val, 
MAKE_64BIT_MASK(0, TARGET_LONG_BITS));
+}
+
+if (ret == RISCV_EXCP_NONE) {
+ret = 0;
+}


Is there a reason for ignoring errors here? If not, I'd rather replace that 
final if-statement with:


else {
g_assert_not_reached();
}

to make sure that mistakes in setting the right sub-command don't get 
ignored without any error message.



+return ret;
+}
+
+bool csr_qtest_callback(CharBackend *chr, gchar **words)
+{
+if (strcmp(words[0], "csr") == 0) {
+
+uint64_t res, cpu;
+
+uint64_t val;
+int rc, csr;
+
+rc = qemu_strtou64(words[2], NULL, 0, );
+g_assert(rc == 0);
+rc = qemu_strtoi(words[3], NULL, 0, );
+g_assert(rc == 0);
+rc = qemu_strtou64(words[4], NULL, 0, );
+g_assert(rc == 0);
+res = csr_call(words[1], cpu, csr, );
+
+qtest_send_prefix(chr);
+qtest_sendf(chr, "OK %"PRIx64" "TARGET_FMT_lx"\n", res, 
(target_ulong)val);
+
+return true;
+}
+
+return false;
+}
+
  /*
   * Control and Status Register function table
   * riscv_csr_operations::predicate() must be provided for an implemented CSR
diff --git a/tests/qtest/libqos/csr.c b/tests/qtest/libqos/csr.c
new file mode 100644
index 00..2dc52fc442
--- /dev/null
+++ b/tests/qtest/libqos/csr.c
@@ -0,0 +1,42 @@
+/*
+ * QTest RISC-V CSR driver
+ *
+ * Copyright (c) 2024 Syntacore
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "../libqtest.h"
+#include "csr.h"
+
+static uint64_t qcsr_call(QTestState *qts, const char *name, uint64_t cpu,
+   int csrno, uint64_t *val)
+{
+uint64_t res = 0;
+
+res = qtest_csr_call(qts, name, cpu, csrno, val);
+
+return res;
+}
+
+int qcsr_get_csr(QTestState *qts, uint64_t cpu,
+int csrno, uint64_t *val)
+{
+int res;
+
+res = qcsr_call(qts, "get_csr", cpu, csrno, val);
+
+return res;
+}
+
+int qcsr_set_csr(QTestState *qts, uint64_t cpu,
+int csrno, uint64_t *val)
+{
+int res;
+
+res = qcsr_call(qts, "set_csr", cpu, csrno, val);
+
+return res;
+}


Technically, there does not seem to be anything related to libqos in your 
patch set. libqos is a framework for executing tests on various buses, e.g. 
to test PCI devices on various host PCI bus implementations. All that is 
triggered via qos-test.c. Your CSR test does not seem to fit into that 
catogory, so please put that code rather directly in your riscv-csr-test.c 
file instead. (unless you want to use it in a lot of other tests in the 
future, too, then maybe you could move them as static inlines into the csr.h 
header instead).



diff --git a/tests/qtest/libqos/csr.h b/tests/qtest/libqos/csr.h
new file mode 100644
index 00..d953735fe8
--- /dev/null
+++ b/tests/qtest/libqos/csr.h


Again, not related to libqos, please move it up to the qtest folder itself.


@@ -0,0 +1,16 @@
+/*
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef LIBQOS_CSR_H
+#define LIBQOS_CSR_H
+
+int qcsr_get_csr(QTestState *qts, uint64_t cpu,
+int csrno, uint64_t *val);
+
+int qcsr_set_csr(QTestState *qts, uint64_t cpu,
+int csrno, uint64_t *val);
+
+
+#endif /* LIBQOS_CSR_H */
diff --git a/tests/qtest/libqos/meson.build b/tests/qtest/libqos/meson.build
index 558eb4c24b..a944febbd8 100644
--- a/tests/qtest/libqos/meson.build
+++ b/tests/qtest/libqos/meson.build
@@ -25,6 +25,9 @@ libqos_srcs = files(
  # usb
  'usb.c',
  
+#riscv csr

+

[PATCH v2] target/s390x: Add a CONFIG switch to disable legacy CPUs

2024-06-14 Thread Thomas Huth
The oldest model that IBM still supports is the z13. Considering
that each generation can "emulate" the previous two generations
in hardware (via the "IBC" feature of the CPUs), this means that
everything that is older than z114/196 is not an officially supported
CPU model anymore. The Linux kernel still support the z10, so if
we also take this into account, everything older than that can
definitely be considered as a legacy CPU model.

For downstream builds of QEMU, we would like to be able to disable
these legacy CPUs in the build. Thus add a CONFIG switch that can be
used to disable them (and old machine types that use them by default).

Signed-off-by: Thomas Huth 
---
v2:
 - Add comment in source code
 - Only consider z9 and older as legacy

 hw/s390x/s390-virtio-ccw.c | 5 +
 target/s390x/cpu_models.c  | 9 +
 target/s390x/Kconfig   | 5 +
 3 files changed, 19 insertions(+)

diff --git a/target/s390x/Kconfig b/target/s390x/Kconfig
index d886be48b4..8a95f2bc3f 100644
--- a/target/s390x/Kconfig
+++ b/target/s390x/Kconfig
@@ -2,3 +2,8 @@ config S390X
 bool
 select PCI
 select S390_FLIC
+
+config S390X_LEGACY_CPUS
+bool
+default y
+depends on S390X
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index efb508cd2e..a6bafe153e 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -22,6 +22,7 @@
 #include "qemu/module.h"
 #include "qemu/hw-version.h"
 #include "qemu/qemu-print.h"
+#include CONFIG_DEVICES
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/sysemu.h"
 #include "target/s390x/kvm/pv.h"
@@ -47,6 +48,13 @@
  * generation 15 one base feature and one optional feature have been 
deprecated.
  */
 static S390CPUDef s390_cpu_defs[] = {
+/*
+ * Linux requires at least z10 nowadays, and IBM only supports recent CPUs
+ * (see 
https://www.ibm.com/support/pages/ibm-mainframe-life-cycle-history),
+ * so we consider older CPUs as legacy that can optionally be disabled via
+ * the CONFIG_S390X_LEGACY_CPUS config switch.
+ */
+#ifdef CONFIG_S390X_LEGACY_CPUS
 CPUDEF_INIT(0x2064, 7, 1, 38, 0xU, "z900", "IBM zSeries 900 GA1"),
 CPUDEF_INIT(0x2064, 7, 2, 38, 0xU, "z900.2", "IBM zSeries 900 
GA2"),
 CPUDEF_INIT(0x2064, 7, 3, 38, 0xU, "z900.3", "IBM zSeries 900 
GA3"),
@@ -64,6 +72,7 @@ static S390CPUDef s390_cpu_defs[] = {
 CPUDEF_INIT(0x2096, 9, 2, 40, 0xU, "z9BC", "IBM System z9 BC GA1"),
 CPUDEF_INIT(0x2094, 9, 3, 40, 0xU, "z9EC.3", "IBM System z9 EC 
GA3"),
 CPUDEF_INIT(0x2096, 9, 3, 40, 0xU, "z9BC.2", "IBM System z9 BC 
GA2"),
+#endif
 CPUDEF_INIT(0x2097, 10, 1, 43, 0xU, "z10EC", "IBM System z10 EC 
GA1"),
 CPUDEF_INIT(0x2097, 10, 2, 43, 0xU, "z10EC.2", "IBM System z10 EC 
GA2"),
 CPUDEF_INIT(0x2098, 10, 2, 43, 0xU, "z10BC", "IBM System z10 BC 
GA1"),
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 3d0bc3e7f2..cd063f8b64 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -47,6 +47,7 @@
 #include "migration/blocker.h"
 #include "qapi/visitor.h"
 #include "hw/s390x/cpu-topology.h"
+#include CONFIG_DEVICES
 
 static Error *pv_mig_blocker;
 
@@ -1126,6 +1127,8 @@ static void ccw_machine_2_12_class_options(MachineClass 
*mc)
 }
 DEFINE_CCW_MACHINE(2_12, "2.12", false);
 
+#ifdef CONFIG_S390X_LEGACY_CPUS
+
 static void ccw_machine_2_11_instance_options(MachineState *machine)
 {
 static const S390FeatInit qemu_cpu_feat = { S390_FEAT_LIST_QEMU_V2_11 };
@@ -1272,6 +1275,8 @@ static void ccw_machine_2_4_class_options(MachineClass 
*mc)
 }
 DEFINE_CCW_MACHINE(2_4, "2.4", false);
 
+#endif
+
 static void ccw_machine_register_types(void)
 {
 type_register_static(_machine_info);
-- 
2.45.2




Re: [PATCH] target/s390x: Add a CONFIG switch to disable legacy CPUs

2024-06-14 Thread Thomas Huth

On 14/06/2024 10.17, Christian Borntraeger wrote:



Am 14.06.24 um 09:15 schrieb Thomas Huth:

On 14/06/2024 08.07, Christian Borntraeger wrote:



Am 13.06.24 um 19:07 schrieb Thomas Huth:

Old CPU models are not officially supported anymore by IBM, and for
downstream builds of QEMU, we would like to be able to disable these
CPUs in the build. Thus add a CONFIG switch that can be used to
disable these CPUs (and old machine types that use them by default).

Signed-off-by: Thomas Huth 
---
  If you're interested, the PDF that can be downloaded from
  https://www.ibm.com/support/pages/ibm-mainframe-life-cycle-history
  shows the supported CPUs in a nice diagram


z13 is still supported so the patch needs to be fixed at least.


Oh, drat, I misread the diagram, indeed. 'should have looked at the table 
instead :-/



Furthermore, z14 has the IBC/VAL cabability to behave like a z13,
same for z15. (we do support VAL to N-2)


Hmm, so if z13 is still supported, and also has the possibility to do N-2, 
I assume the z114/196 and z12 should still be considered as non-legacy, too?


Yes. z9 and older is no longer relevant (only for people that collect old 
HW) but the upstream kernel has an minimum requirement for z10 so maybe we 
still want to support that for testing purposes.


Ok, fair point, kernel support is a good hint, too.

For upstream I prefer to keep the full list but I would be ok to hide those 
ancient things behind a config switch.


That's what this patch is trying to do - by default, all CPUs are still 
enabled, you have actively disable the switch to get rid of the old ones.


 Thomas





Re: [PATCH] target/s390x: Add a CONFIG switch to disable legacy CPUs

2024-06-14 Thread Thomas Huth

On 14/06/2024 08.07, Christian Borntraeger wrote:



Am 13.06.24 um 19:07 schrieb Thomas Huth:

Old CPU models are not officially supported anymore by IBM, and for
downstream builds of QEMU, we would like to be able to disable these
CPUs in the build. Thus add a CONFIG switch that can be used to
disable these CPUs (and old machine types that use them by default).

Signed-off-by: Thomas Huth 
---
  If you're interested, the PDF that can be downloaded from
  https://www.ibm.com/support/pages/ibm-mainframe-life-cycle-history
  shows the supported CPUs in a nice diagram


z13 is still supported so the patch needs to be fixed at least.


Oh, drat, I misread the diagram, indeed. 'should have looked at the table 
instead :-/



Furthermore, z14 has the IBC/VAL cabability to behave like a z13,
same for z15. (we do support VAL to N-2)


Hmm, so if z13 is still supported, and also has the possibility to do N-2, I 
assume the z114/196 and z12 should still be considered as non-legacy, too?



I fail to see the value of this given how stable this code is.


As mentioned in the patch description, it's meant for downstream builds of 
QEMU where we'd like to avoid legacy devices and CPUs in the builds (it 
doesn't make sense to have support for e.g. z900 CPUs there).


 Thomas




Re: [PATCH] target/s390x: Add a CONFIG switch to disable legacy CPUs

2024-06-13 Thread Thomas Huth

On 13/06/2024 19.17, Philippe Mathieu-Daudé wrote:

Hi Thomas,

On 13/6/24 19:07, Thomas Huth wrote:

Old CPU models are not officially supported anymore by IBM, and for
downstream builds of QEMU, we would like to be able to disable these
CPUs in the build. Thus add a CONFIG switch that can be used to
disable these CPUs (and old machine types that use them by default).

Signed-off-by: Thomas Huth 
---
  If you're interested, the PDF that can be downloaded from
  https://www.ibm.com/support/pages/ibm-mainframe-life-cycle-history
  shows the supported CPUs in a nice diagram


I'd add this link ...


  hw/s390x/s390-virtio-ccw.c | 9 +
  target/s390x/cpu_models.c  | 3 +++
  target/s390x/Kconfig   | 5 +
  3 files changed, 17 insertions(+)

diff --git a/target/s390x/Kconfig b/target/s390x/Kconfig
index d886be48b4..8a95f2bc3f 100644
--- a/target/s390x/Kconfig
+++ b/target/s390x/Kconfig
@@ -2,3 +2,8 @@ config S390X
  bool
  select PCI
  select S390_FLIC
+
+config S390X_LEGACY_CPUS
+    bool
+    default y
+    depends on S390X
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index efb508cd2e..ffae95dcb3 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -22,6 +22,7 @@
  #include "qemu/module.h"
  #include "qemu/hw-version.h"
  #include "qemu/qemu-print.h"
+#include CONFIG_DEVICES
  #ifndef CONFIG_USER_ONLY
  #include "sysemu/sysemu.h"
  #include "target/s390x/kvm/pv.h"
@@ -47,6 +48,7 @@
   * generation 15 one base feature and one optional feature have been 
deprecated.

   */
  static S390CPUDef s390_cpu_defs[] = {
+#ifdef CONFIG_S390X_LEGACY_CPUS


... here :)


Can do ... let's just hope that the link is stable in the course of time!

  CPUDEF_INIT(0x2064, 7, 1, 38, 0xU, "z900", "IBM zSeries 900 
GA1"),
  CPUDEF_INIT(0x2064, 7, 2, 38, 0xU, "z900.2", "IBM zSeries 
900 GA2"),
  CPUDEF_INIT(0x2064, 7, 3, 38, 0xU, "z900.3", "IBM zSeries 
900 GA3"),

@@ -78,6 +80,7 @@ static S390CPUDef s390_cpu_defs[] = {
  CPUDEF_INIT(0x2964, 13, 1, 47, 0x0800U, "z13", "IBM z13 GA1"),
  CPUDEF_INIT(0x2964, 13, 2, 47, 0x0800U, "z13.2", "IBM z13 GA2"),
  CPUDEF_INIT(0x2965, 13, 2, 47, 0x0800U, "z13s", "IBM z13s GA1"),
+#endif
  CPUDEF_INIT(0x3906, 14, 1, 47, 0x0800U, "z14", "IBM z14 GA1"),
  CPUDEF_INIT(0x3906, 14, 2, 47, 0x0800U, "z14.2", "IBM z14 GA2"),
  CPUDEF_INIT(0x3907, 14, 1, 47, 0x0800U, "z14ZR1", "IBM z14 Model 
ZR1 GA1"),

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 3d0bc3e7f2..7529d2fba8 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -47,6 +47,7 @@
  #include "migration/blocker.h"
  #include "qapi/visitor.h"
  #include "hw/s390x/cpu-topology.h"
+#include CONFIG_DEVICES
  static Error *pv_mig_blocker;
@@ -603,6 +604,8 @@ static void s390_nmi(NMIState *n, int cpu_index, Error 
**errp)

  s390_cpu_restart(S390_CPU(cs));
  }
+#ifdef CONFIG_S390X_LEGACY_CPUS
+
  static ram_addr_t s390_fixup_ram_size(ram_addr_t sz)
  {
  /* same logic as in sclp.c */
@@ -623,6 +626,8 @@ static ram_addr_t s390_fixup_ram_size(ram_addr_t sz)
  return newsz;
  }
+#endif
+
  static inline bool machine_get_aes_key_wrap(Object *obj, Error **errp)
  {
  S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
@@ -989,6 +994,8 @@ static void ccw_machine_6_1_class_options(MachineClass 
*mc)

  }
  DEFINE_CCW_MACHINE(6_1, "6.1", false);
+#ifdef CONFIG_S390X_LEGACY_CPUS
+
  static void ccw_machine_6_0_instance_options(MachineState *machine)
  {
  static const S390FeatInit qemu_cpu_feat = { S390_FEAT_LIST_QEMU_V6_0 };


Should we deprecate machines up to v6.0?


I'm still hoping that Daniel will be able to get his auto-deprecation 
patches merged in this cycle - then we shouldn't derive from that, I think.


By the way, what's up with your i440fx removal series? ... it would be good 
to get this finally merged now...?


 Thomas




Re: [PATCH] tests/qtest/fuzz/virtio_net_fuzz.c: fix virtio_net_fuzz_multi

2024-06-13 Thread Thomas Huth

On 13/06/2024 13.59, Дмитрий Фролов wrote:



On 13.06.2024 13:08, Thomas Huth wrote:

On 23/05/2024 12.28, Dmitry Frolov wrote:

If QTestState was already CLOSED due to error, calling qtest_clock_step()
afterwards makes no sense and only raises false-crash with message:
"assertion timer != NULL failed".

Signed-off-by: Dmitry Frolov 
---
  tests/qtest/fuzz/virtio_net_fuzz.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/tests/qtest/fuzz/virtio_net_fuzz.c 
b/tests/qtest/fuzz/virtio_net_fuzz.c

index e239875e3b..2f57a8ddd8 100644
--- a/tests/qtest/fuzz/virtio_net_fuzz.c
+++ b/tests/qtest/fuzz/virtio_net_fuzz.c
@@ -81,6 +81,9 @@ static void virtio_net_fuzz_multi(QTestState *s,
  /* Run the main loop */
  qtest_clock_step(s, 100);
  flush_events(s);
+    if (!qtest_probe_child(s)) {
+    return;
+    }


According to your patch description, it rather sounds like the check 
should be done before the qtest_clock_step() ? ... or where does the 
QTestState get closed? During flush_events() ?
To my understanding, the main loop is executed during flush_events(), where 
an error may occur. This behavior is legit and should not produce any crash 
report.
Without the check, the test continues to wait on used descriptors, and 
finally fails with message: "assertion timer != NULL failed".

Thus, any invalid input data produces a meaningless crash report.


Ok, makes sense now, thanks!

There seems to be another while loop with a flush_events() call later in 
this file, does it maybe need the same treatment, too?


 Thomas




[PATCH] target/s390x: Add a CONFIG switch to disable legacy CPUs

2024-06-13 Thread Thomas Huth
Old CPU models are not officially supported anymore by IBM, and for
downstream builds of QEMU, we would like to be able to disable these
CPUs in the build. Thus add a CONFIG switch that can be used to
disable these CPUs (and old machine types that use them by default).

Signed-off-by: Thomas Huth 
---
 If you're interested, the PDF that can be downloaded from
 https://www.ibm.com/support/pages/ibm-mainframe-life-cycle-history
 shows the supported CPUs in a nice diagram

 hw/s390x/s390-virtio-ccw.c | 9 +
 target/s390x/cpu_models.c  | 3 +++
 target/s390x/Kconfig   | 5 +
 3 files changed, 17 insertions(+)

diff --git a/target/s390x/Kconfig b/target/s390x/Kconfig
index d886be48b4..8a95f2bc3f 100644
--- a/target/s390x/Kconfig
+++ b/target/s390x/Kconfig
@@ -2,3 +2,8 @@ config S390X
 bool
 select PCI
 select S390_FLIC
+
+config S390X_LEGACY_CPUS
+bool
+default y
+depends on S390X
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index efb508cd2e..ffae95dcb3 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -22,6 +22,7 @@
 #include "qemu/module.h"
 #include "qemu/hw-version.h"
 #include "qemu/qemu-print.h"
+#include CONFIG_DEVICES
 #ifndef CONFIG_USER_ONLY
 #include "sysemu/sysemu.h"
 #include "target/s390x/kvm/pv.h"
@@ -47,6 +48,7 @@
  * generation 15 one base feature and one optional feature have been 
deprecated.
  */
 static S390CPUDef s390_cpu_defs[] = {
+#ifdef CONFIG_S390X_LEGACY_CPUS
 CPUDEF_INIT(0x2064, 7, 1, 38, 0xU, "z900", "IBM zSeries 900 GA1"),
 CPUDEF_INIT(0x2064, 7, 2, 38, 0xU, "z900.2", "IBM zSeries 900 
GA2"),
 CPUDEF_INIT(0x2064, 7, 3, 38, 0xU, "z900.3", "IBM zSeries 900 
GA3"),
@@ -78,6 +80,7 @@ static S390CPUDef s390_cpu_defs[] = {
 CPUDEF_INIT(0x2964, 13, 1, 47, 0x0800U, "z13", "IBM z13 GA1"),
 CPUDEF_INIT(0x2964, 13, 2, 47, 0x0800U, "z13.2", "IBM z13 GA2"),
 CPUDEF_INIT(0x2965, 13, 2, 47, 0x0800U, "z13s", "IBM z13s GA1"),
+#endif
 CPUDEF_INIT(0x3906, 14, 1, 47, 0x0800U, "z14", "IBM z14 GA1"),
 CPUDEF_INIT(0x3906, 14, 2, 47, 0x0800U, "z14.2", "IBM z14 GA2"),
 CPUDEF_INIT(0x3907, 14, 1, 47, 0x0800U, "z14ZR1", "IBM z14 Model ZR1 
GA1"),
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 3d0bc3e7f2..7529d2fba8 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -47,6 +47,7 @@
 #include "migration/blocker.h"
 #include "qapi/visitor.h"
 #include "hw/s390x/cpu-topology.h"
+#include CONFIG_DEVICES
 
 static Error *pv_mig_blocker;
 
@@ -603,6 +604,8 @@ static void s390_nmi(NMIState *n, int cpu_index, Error 
**errp)
 s390_cpu_restart(S390_CPU(cs));
 }
 
+#ifdef CONFIG_S390X_LEGACY_CPUS
+
 static ram_addr_t s390_fixup_ram_size(ram_addr_t sz)
 {
 /* same logic as in sclp.c */
@@ -623,6 +626,8 @@ static ram_addr_t s390_fixup_ram_size(ram_addr_t sz)
 return newsz;
 }
 
+#endif
+
 static inline bool machine_get_aes_key_wrap(Object *obj, Error **errp)
 {
 S390CcwMachineState *ms = S390_CCW_MACHINE(obj);
@@ -989,6 +994,8 @@ static void ccw_machine_6_1_class_options(MachineClass *mc)
 }
 DEFINE_CCW_MACHINE(6_1, "6.1", false);
 
+#ifdef CONFIG_S390X_LEGACY_CPUS
+
 static void ccw_machine_6_0_instance_options(MachineState *machine)
 {
 static const S390FeatInit qemu_cpu_feat = { S390_FEAT_LIST_QEMU_V6_0 };
@@ -1272,6 +1279,8 @@ static void ccw_machine_2_4_class_options(MachineClass 
*mc)
 }
 DEFINE_CCW_MACHINE(2_4, "2.4", false);
 
+#endif
+
 static void ccw_machine_register_types(void)
 {
 type_register_static(_machine_info);
-- 
2.45.2




Re: [PATCH] tests/qtest/fuzz/virtio_net_fuzz.c: fix virtio_net_fuzz_multi

2024-06-13 Thread Thomas Huth

On 23/05/2024 12.28, Dmitry Frolov wrote:

If QTestState was already CLOSED due to error, calling qtest_clock_step()
afterwards makes no sense and only raises false-crash with message:
"assertion timer != NULL failed".

Signed-off-by: Dmitry Frolov 
---
  tests/qtest/fuzz/virtio_net_fuzz.c | 3 +++
  1 file changed, 3 insertions(+)

diff --git a/tests/qtest/fuzz/virtio_net_fuzz.c 
b/tests/qtest/fuzz/virtio_net_fuzz.c
index e239875e3b..2f57a8ddd8 100644
--- a/tests/qtest/fuzz/virtio_net_fuzz.c
+++ b/tests/qtest/fuzz/virtio_net_fuzz.c
@@ -81,6 +81,9 @@ static void virtio_net_fuzz_multi(QTestState *s,
  /* Run the main loop */
  qtest_clock_step(s, 100);
  flush_events(s);
+if (!qtest_probe_child(s)) {
+return;
+}


According to your patch description, it rather sounds like the check should 
be done before the qtest_clock_step() ? ... or where does the QTestState get 
closed? During flush_events() ?


 Thomas




Re: [PATCH 2/2] QTest example for RISC-V CSR register

2024-06-13 Thread Thomas Huth

On 13/06/2024 11.56, Ivan Klokov wrote:

Added demo for reading CSR register from qtest environment.

Signed-off-by: Ivan Klokov 
---
  tests/qtest/meson.build  |  2 ++
  tests/qtest/riscv-csr-test.c | 68 
  2 files changed, 70 insertions(+)
  create mode 100644 tests/qtest/riscv-csr-test.c

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 12792948ff..45d651da99 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -259,6 +259,8 @@ qtests_s390x = \
  qtests_riscv32 = \
(config_all_devices.has_key('CONFIG_SIFIVE_E_AON') ? 
['sifive-e-aon-watchdog-test'] : [])
  
+qtests_riscv32 += ['riscv-csr-test']

+
  qos_test_ss = ss.source_set()
  qos_test_ss.add(
'ac97-test.c',
diff --git a/tests/qtest/riscv-csr-test.c b/tests/qtest/riscv-csr-test.c
new file mode 100644
index 00..715d5fe4b7
--- /dev/null
+++ b/tests/qtest/riscv-csr-test.c
@@ -0,0 +1,68 @@
+/*
+ * QTest testcase for RISC-V CSRs
+ *
+ * Copyright (c) 2024 Syntacore.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the
+ * Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+ * for more details.
+ */
+
+#include "qemu/osdep.h"
+#include "libqtest-single.h"
+#include "qemu/error-report.h"
+
+#include "qapi/qmp/qdict.h"
+#include "qapi/qmp/qjson.h"
+#include "qapi/qmp/qlist.h"
+#include "qapi/qmp/qstring.h"
+#include "qapi/qmp/qobject.h"
+#include "qapi/qobject-input-visitor.h"
+#include "qapi/qobject-output-visitor.h"
+#include "qom/object_interfaces.h"


Do you really need all these headers for the short code below? Please double 
check and trim the list.



+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "qemu/error-report.h"
+#include "hw/qdev-properties.h"
+
+#include "qemu/osdep.h"


Duplicate include statement, please remove.


+#include "qemu/cutils.h"
+#include "libqtest.h"
+
+#include "libqos/csr.h"
+#include "libqos/libqos.h"
+
+static void run_test_csr(void)
+{
+
+uint64_t res;
+uint64_t val = 0;
+
+res = qcsr_get_csr(global_qtest, 0, 0xf11, );
+
+g_assert_cmpint(res, ==, 0);
+g_assert_cmpint(val, ==, 0x100);
+}
+
+int main(int argc, char **argv)
+{
+g_test_init(, , NULL);
+
+qtest_add_func("/cpu/csr", run_test_csr);
+
+qtest_start("--nographic -machine virt -cpu any,mvendorid=0x100");


You don't need --nographic, it's been taken care off by the libqtest 
framework already.



+g_test_run();
+
+qtest_quit(global_qtest);
+
+return 0;


You should return the result of g_test_run() here, otherwise your test will 
look like it always succeeds.



+}


 Thomas




Re: [PATCH] tests/qtest/fuzz: fix memleak in qos_fuzz.c

2024-06-13 Thread Thomas Huth

On 21/05/2024 12.31, Dmitry Frolov wrote:

Found with fuzzing for qemu-8.2, but also relevant for master

Signed-off-by: Dmitry Frolov 
---
  tests/qtest/fuzz/qos_fuzz.c | 1 +
  1 file changed, 1 insertion(+)

diff --git a/tests/qtest/fuzz/qos_fuzz.c b/tests/qtest/fuzz/qos_fuzz.c
index b71e945c5f..d3839bf999 100644
--- a/tests/qtest/fuzz/qos_fuzz.c
+++ b/tests/qtest/fuzz/qos_fuzz.c
@@ -180,6 +180,7 @@ static void walk_path(QOSGraphNode *orig_path, int len)
  
  fuzz_path_vec = path_vec;

  } else {
+g_string_free(cmd_line, true);
  g_free(path_vec);
  }
  


Reviewed-by: Thomas Huth 




Re: Qemu License question

2024-06-13 Thread Thomas Huth

On 13/06/2024 07.22, Markus Armbruster wrote:

Manos Pitsidianakis  writes:


On Thu, 13 Jun 2024 06:26, Peng Fan  wrote:

Hi All,

The following files are marked as GPL-3.0-or-later. Will these
Conflict with Qemu LICENSE?

Should we update the files to GPL-2.0?

./tests/tcg/aarch64/semicall.h:7: * SPDX-License-Identifier: GPL-3.0-or-later
./tests/tcg/x86_64/system/boot.S:13: * SPDX-License-Identifier: GPL-3.0-or-later
./tests/tcg/riscv64/semicall.h:7: * SPDX-License-Identifier: GPL-3.0-or-later
./tests/tcg/multiarch/float_convs.c:6: * SPDX-License-Identifier: 
GPL-3.0-or-later
./tests/tcg/multiarch/float_helpers.h:6: * SPDX-License-Identifier: 
GPL-3.0-or-later
./tests/tcg/multiarch/libs/float_helpers.c:10: * SPDX-License-Identifier: 
GPL-3.0-or-later
./tests/tcg/multiarch/arm-compat-semi/semihosting.c:7: * 
SPDX-License-Identifier: GPL-3.0-or-later
./tests/tcg/multiarch/arm-compat-semi/semiconsole.c:7: * 
SPDX-License-Identifier: GPL-3.0-or-later
./tests/tcg/multiarch/float_convd.c:6: * SPDX-License-Identifier: 
GPL-3.0-or-later
./tests/tcg/multiarch/float_madds.c:6: * SPDX-License-Identifier: 
GPL-3.0-or-later
./tests/tcg/i386/system/boot.S:10: * SPDX-License-Identifier: GPL-3.0-or-later
./tests/tcg/arm/semicall.h:7: * SPDX-License-Identifier: GPL-3.0-or-later

Thanks,
Peng.


Hello Peng,

These are all actually GPL-2.0-or-later, in fact I can't find the string 
GPL-3.0-or-later in the current master at all.


See commit 542b10bd148a (tests/tcg: update licenses to GPLv2 as intended).


Maybe it could be included in the stable releases before 9.0, too?
CC:-ing qemu-stable for this now.

 Thomas




[PULL 13/15] tests/unit/test-smp-parse: Test "modules" and "dies" combination case

2024-06-12 Thread Thomas Huth
From: Zhao Liu 

Since i386 PC machine supports both "modules" and "dies" in -smp, add the
"modules" and "dies" combination test case to match the actual topology
usage scenario.

Signed-off-by: Zhao Liu 
Tested-by: Yongwei Ma 
Message-ID: <20240529061925.350323-8-zhao1@intel.com>
Signed-off-by: Thomas Huth 
---
 tests/unit/test-smp-parse.c | 103 
 1 file changed, 103 insertions(+)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 01832e5eda..2ca8530e93 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -445,6 +445,33 @@ static const struct SMPTestData data_with_dies_invalid[] = 
{
 },
 };
 
+static const struct SMPTestData data_with_modules_dies_invalid[] = {
+{
+/*
+ * config: -smp 200,sockets=3,dies=5,modules=2,cores=4,\
+ * threads=2,maxcpus=200
+ */
+.config = SMP_CONFIG_WITH_MODS_DIES(T, 200, T, 3, T, 5, T,
+2, T, 4, T, 2, T, 200),
+.expect_error = "Invalid CPU topology: "
+"product of the hierarchy must match maxcpus: "
+"sockets (3) * dies (5) * modules (2) * "
+"cores (4) * threads (2) != maxcpus (200)",
+}, {
+/*
+ * config: -smp 242,sockets=3,dies=5,modules=2,cores=4,\
+ * threads=2,maxcpus=240
+ */
+.config = SMP_CONFIG_WITH_MODS_DIES(T, 242, T, 3, T, 5, T,
+2, T, 4, T, 2, T, 240),
+.expect_error = "Invalid CPU topology: "
+"maxcpus must be equal to or greater than smp: "
+"sockets (3) * dies (5) * modules (2) * "
+"cores (4) * threads (2) "
+"== maxcpus (240) < smp_cpus (242)",
+},
+};
+
 static const struct SMPTestData data_with_clusters_invalid[] = {
 {
 /* config: -smp 16,sockets=2,clusters=2,cores=4,threads=2,maxcpus=16 */
@@ -905,6 +932,14 @@ static void machine_with_dies_class_init(ObjectClass *oc, 
void *data)
 mc->smp_props.dies_supported = true;
 }
 
+static void machine_with_modules_dies_class_init(ObjectClass *oc, void *data)
+{
+MachineClass *mc = MACHINE_CLASS(oc);
+
+mc->smp_props.modules_supported = true;
+mc->smp_props.dies_supported = true;
+}
+
 static void machine_with_clusters_class_init(ObjectClass *oc, void *data)
 {
 MachineClass *mc = MACHINE_CLASS(oc);
@@ -1082,6 +1117,67 @@ static void test_with_dies(const void *opaque)
 object_unref(obj);
 }
 
+static void test_with_modules_dies(const void *opaque)
+{
+const char *machine_type = opaque;
+Object *obj = object_new(machine_type);
+MachineState *ms = MACHINE(obj);
+MachineClass *mc = MACHINE_GET_CLASS(obj);
+SMPTestData data = {};
+unsigned int num_modules = 5, num_dies = 3;
+int i;
+
+for (i = 0; i < ARRAY_SIZE(data_generic_valid); i++) {
+data = data_generic_valid[i];
+unsupported_params_init(mc, );
+
+/*
+ * when modules and dies parameters are omitted, they will
+ * be both set as 1.
+ */
+data.expect_prefer_sockets.modules = 1;
+data.expect_prefer_sockets.dies = 1;
+data.expect_prefer_cores.modules = 1;
+data.expect_prefer_cores.dies = 1;
+
+smp_parse_test(ms, , true);
+
+/* when modules and dies parameters are both specified */
+data.config.has_modules = true;
+data.config.modules = num_modules;
+data.config.has_dies = true;
+data.config.dies = num_dies;
+
+if (data.config.has_cpus) {
+data.config.cpus *= num_modules * num_dies;
+}
+if (data.config.has_maxcpus) {
+data.config.maxcpus *= num_modules * num_dies;
+}
+
+data.expect_prefer_sockets.modules = num_modules;
+data.expect_prefer_sockets.dies = num_dies;
+data.expect_prefer_sockets.cpus *= num_modules * num_dies;
+data.expect_prefer_sockets.max_cpus *= num_modules * num_dies;
+
+data.expect_prefer_cores.modules = num_modules;
+data.expect_prefer_cores.dies = num_dies;
+data.expect_prefer_cores.cpus *= num_modules * num_dies;
+data.expect_prefer_cores.max_cpus *= num_modules * num_dies;
+
+smp_parse_test(ms, , true);
+}
+
+for (i = 0; i < ARRAY_SIZE(data_with_modules_dies_invalid); i++) {
+data = data_with_modules_dies_invalid[i];
+unsupported_params_init(mc, );
+
+smp_parse_test(ms, , false);
+}
+
+object_unref(obj);
+}
+
 static void test_with_clusters(const void *opaque)
 {
 const char *machine_type = opaque;
@@ -1398,6 +1494,10 @@ static const TypeInfo smp_machine_types[] = {
 

[PULL 03/15] tests/qtest/libqtest: add qtest_has_cpu_model() api

2024-06-12 Thread Thomas Huth
From: Ani Sinha 

Added a new test api qtest_has_cpu_model() in order to check availability of
some cpu models in the current QEMU binary. The specific architecture of the
QEMU binary is selected using the QTEST_QEMU_BINARY environment variable.
This api would be useful to run tests against some older cpu models after
checking if QEMU actually supported these models.

Signed-off-by: Ani Sinha 
Reviewed-by: Reviewed-by: Daniel P. Berrangé 
Message-ID: <20240610155303.7933-3-anisi...@redhat.com>
Signed-off-by: Thomas Huth 
---
 tests/qtest/libqtest.h |  8 
 tests/qtest/libqtest.c | 83 ++
 2 files changed, 91 insertions(+)

diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index 6e3d3525bf..beb96b18eb 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -949,6 +949,14 @@ bool qtest_has_machine(const char *machine);
  */
 bool qtest_has_machine_with_env(const char *var, const char *machine);
 
+/**
+ * qtest_has_cpu_model:
+ * @cpu: The cpu to look for
+ *
+ * Returns: true if the cpu is available in the target binary.
+ */
+bool qtest_has_cpu_model(const char *cpu);
+
 /**
  * qtest_has_device:
  * @device: The device to look for
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index d8f80d335e..18e2f7f282 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -37,6 +37,7 @@
 #include "qapi/qmp/qjson.h"
 #include "qapi/qmp/qlist.h"
 #include "qapi/qmp/qstring.h"
+#include "qapi/qmp/qbool.h"
 
 #define MAX_IRQ 256
 
@@ -1471,6 +1472,12 @@ struct MachInfo {
 char *alias;
 };
 
+struct CpuModel {
+char *name;
+char *alias_of;
+bool deprecated;
+};
+
 static void qtest_free_machine_list(struct MachInfo *machines)
 {
 if (machines) {
@@ -1550,6 +1557,82 @@ static struct MachInfo *qtest_get_machines(const char 
*var)
 return machines;
 }
 
+static struct CpuModel *qtest_get_cpu_models(void)
+{
+static struct CpuModel *cpus;
+QDict *response, *minfo;
+QList *list;
+const QListEntry *p;
+QObject *qobj;
+QString *qstr;
+QBool *qbool;
+QTestState *qts;
+int idx;
+
+if (cpus) {
+return cpus;
+}
+
+silence_spawn_log = !g_test_verbose();
+
+qts = qtest_init_with_env(NULL, "-machine none");
+response = qtest_qmp(qts, "{ 'execute': 'query-cpu-definitions' }");
+g_assert(response);
+list = qdict_get_qlist(response, "return");
+g_assert(list);
+
+cpus = g_new0(struct CpuModel, qlist_size(list) + 1);
+
+for (p = qlist_first(list), idx = 0; p; p = qlist_next(p), idx++) {
+minfo = qobject_to(QDict, qlist_entry_obj(p));
+g_assert(minfo);
+
+qobj = qdict_get(minfo, "name");
+g_assert(qobj);
+qstr = qobject_to(QString, qobj);
+g_assert(qstr);
+cpus[idx].name = g_strdup(qstring_get_str(qstr));
+
+qobj = qdict_get(minfo, "alias_of");
+if (qobj) { /* old machines do not report aliases */
+qstr = qobject_to(QString, qobj);
+g_assert(qstr);
+cpus[idx].alias_of = g_strdup(qstring_get_str(qstr));
+} else {
+cpus[idx].alias_of = NULL;
+}
+
+qobj = qdict_get(minfo, "deprecated");
+qbool = qobject_to(QBool, qobj);
+g_assert(qbool);
+cpus[idx].deprecated = qbool_get_bool(qbool);
+}
+
+qtest_quit(qts);
+qobject_unref(response);
+
+silence_spawn_log = false;
+
+return cpus;
+}
+
+bool qtest_has_cpu_model(const char *cpu)
+{
+struct CpuModel *cpus;
+int i;
+
+cpus = qtest_get_cpu_models();
+
+for (i = 0; cpus[i].name != NULL; i++) {
+if (g_str_equal(cpu, cpus[i].name) ||
+(cpus[i].alias_of && g_str_equal(cpu, cpus[i].alias_of))) {
+return true;
+}
+}
+
+return false;
+}
+
 void qtest_cb_for_every_machine(void (*cb)(const char *machine),
 bool skip_old_versioned)
 {
-- 
2.45.2




[PULL 15/15] tests/tcg/s390x: Allow specifying extra QEMU options on the command line

2024-06-12 Thread Thomas Huth
From: Ilya Leoshkevich 

The use case for this is `make check-tcg EXTFLAGS="-accel kvm"`,
which allows validating the system TCG testcases on real hardware.
EXTFLAGS name is borrowed from tests/tcg/xtensa/Makefile.softmmu-target.
While at it, use += instead of = in order to be consistent with the
other architectures.

Signed-off-by: Ilya Leoshkevich 
Reviewed-by: Thomas Huth 
Message-ID: <20240522184116.35975-1-...@linux.ibm.com>
Signed-off-by: Thomas Huth 
---
 tests/tcg/s390x/Makefile.softmmu-target | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/tcg/s390x/Makefile.softmmu-target 
b/tests/tcg/s390x/Makefile.softmmu-target
index 80159cccf5..4c8e15e625 100644
--- a/tests/tcg/s390x/Makefile.softmmu-target
+++ b/tests/tcg/s390x/Makefile.softmmu-target
@@ -1,6 +1,6 @@
 S390X_SRC=$(SRC_PATH)/tests/tcg/s390x
 VPATH+=$(S390X_SRC)
-QEMU_OPTS=-action panic=exit-failure -nographic -kernel
+QEMU_OPTS+=-action panic=exit-failure -nographic $(EXTFLAGS) -kernel
 LINK_SCRIPT=$(S390X_SRC)/softmmu.ld
 CFLAGS+=-ggdb -O0
 LDFLAGS=-nostdlib -static
-- 
2.45.2




[PULL 04/15] tests/qtest/x86: check for availability of older cpu models before running tests

2024-06-12 Thread Thomas Huth
From: Ani Sinha 

It is better to check if some older cpu models like 486, athlon, pentium,
penryn, phenom, core2duo etc are available before running their corresponding
tests. Some downstream distributions may no longer support these older cpu
models.

Signature of add_feature_test() has been modified to return void as
FeatureTestArgs* was not used by the caller.

One minor correction. Replaced 'phenom' with '486' in the test
'x86/cpuid/auto-level/phenom/arat' matching the cpu used.

Signed-off-by: Ani Sinha 
Reviewed-by: Daniel P. Berrangé 
Message-ID: <20240610155303.7933-4-anisi...@redhat.com>
Signed-off-by: Thomas Huth 
---
 tests/qtest/test-x86-cpuid-compat.c | 170 ++--
 1 file changed, 108 insertions(+), 62 deletions(-)

diff --git a/tests/qtest/test-x86-cpuid-compat.c 
b/tests/qtest/test-x86-cpuid-compat.c
index 6a39454fce..b9e7e5ef7b 100644
--- a/tests/qtest/test-x86-cpuid-compat.c
+++ b/tests/qtest/test-x86-cpuid-compat.c
@@ -67,10 +67,29 @@ static void test_cpuid_prop(const void *data)
 g_free(path);
 }
 
-static void add_cpuid_test(const char *name, const char *cmdline,
+static void add_cpuid_test(const char *name, const char *cpu,
+   const char *cpufeat, const char *machine,
const char *property, int64_t expected_value)
 {
 CpuidTestArgs *args = g_new0(CpuidTestArgs, 1);
+char *cmdline;
+char *save;
+
+if (!qtest_has_cpu_model(cpu)) {
+return;
+}
+cmdline = g_strdup_printf("-cpu %s", cpu);
+
+if (cpufeat) {
+save = cmdline;
+cmdline = g_strdup_printf("%s,%s", cmdline, cpufeat);
+g_free(save);
+}
+if (machine) {
+save = cmdline;
+cmdline = g_strdup_printf("-machine %s %s", machine, cmdline);
+g_free(save);
+}
 args->cmdline = cmdline;
 args->property = property;
 args->expected_value = expected_value;
@@ -149,12 +168,24 @@ static void test_feature_flag(const void *data)
  * either "feature-words" or "filtered-features", when running QEMU
  * using cmdline
  */
-static FeatureTestArgs *add_feature_test(const char *name, const char *cmdline,
- uint32_t eax, uint32_t ecx,
- const char *reg, int bitnr,
- bool expected_value)
+static void add_feature_test(const char *name, const char *cpu,
+ const char *cpufeat, uint32_t eax,
+ uint32_t ecx, const char *reg,
+ int bitnr, bool expected_value)
 {
 FeatureTestArgs *args = g_new0(FeatureTestArgs, 1);
+char *cmdline;
+
+if (!qtest_has_cpu_model(cpu)) {
+return;
+}
+
+if (cpufeat) {
+cmdline = g_strdup_printf("-cpu %s,%s", cpu, cpufeat);
+} else {
+cmdline = g_strdup_printf("-cpu %s", cpu);
+}
+
 args->cmdline = cmdline;
 args->in_eax = eax;
 args->in_ecx = ecx;
@@ -162,13 +193,17 @@ static FeatureTestArgs *add_feature_test(const char 
*name, const char *cmdline,
 args->bitnr = bitnr;
 args->expected_value = expected_value;
 qtest_add_data_func(name, args, test_feature_flag);
-return args;
+return;
 }
 
 static void test_plus_minus_subprocess(void)
 {
 char *path;
 
+if (!qtest_has_cpu_model("pentium")) {
+return;
+}
+
 /* Rules:
  * 1)"-foo" overrides "+foo"
  * 2) "[+-]foo" overrides "foo=..."
@@ -198,6 +233,10 @@ static void test_plus_minus_subprocess(void)
 
 static void test_plus_minus(void)
 {
+if (!qtest_has_cpu_model("pentium")) {
+return;
+}
+
 g_test_trap_subprocess("/x86/cpuid/parsing-plus-minus/subprocess", 0, 0);
 g_test_trap_assert_passed();
 g_test_trap_assert_stderr("*Ambiguous CPU model string. "
@@ -217,99 +256,105 @@ int main(int argc, char **argv)
 
 /* Original level values for CPU models: */
 add_cpuid_test("x86/cpuid/phenom/level",
-   "-cpu phenom", "level", 5);
+   "phenom", NULL, NULL, "level", 5);
 add_cpuid_test("x86/cpuid/Conroe/level",
-   "-cpu Conroe", "level", 10);
+   "Conroe", NULL, NULL, "level", 10);
 add_cpuid_test("x86/cpuid/SandyBridge/level",
-   "-cpu SandyBridge", "level", 0xd);
+   "SandyBridge", NULL, NULL, "level", 0xd);
 add_cpuid_test("x86/cpuid/486/xlevel",
-   "-cpu 486", "xlevel", 0);
+   "486", NULL, NULL, "xlevel", 0);
 add_cpuid_test("x86/cpuid/co

[PULL 14/15] tests/unit/test-smp-parse: Test the full 8-levels topology hierarchy

2024-06-12 Thread Thomas Huth
From: Zhao Liu 

With module level, QEMU now support 8-levels topology hierarchy.
Cover "modules" in SMP_CONFIG_WITH_FULL_TOPO related cases.

Signed-off-by: Zhao Liu 
Tested-by: Yongwei Ma 
Message-ID: <20240529061925.350323-9-zhao1@intel.com>
Signed-off-by: Thomas Huth 
---
 tests/unit/test-smp-parse.c | 129 
 1 file changed, 85 insertions(+), 44 deletions(-)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 2ca8530e93..f9bccb56ab 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -94,11 +94,11 @@
 }
 
 /*
- * Currently QEMU supports up to a 7-level topology hierarchy, which is the
+ * Currently QEMU supports up to a 8-level topology hierarchy, which is the
  * QEMU's unified abstract representation of CPU topology.
- *  -drawers/books/sockets/dies/clusters/cores/threads
+ *  -drawers/books/sockets/dies/clusters/modules/cores/threads
  */
-#define SMP_CONFIG_WITH_FULL_TOPO(a, b, c, d, e, f, g, h, i)\
+#define SMP_CONFIG_WITH_FULL_TOPO(a, b, c, d, e, f, g, h, i, j) \
 {   \
 .has_cpus = true, .cpus = a,\
 .has_drawers  = true, .drawers  = b,\
@@ -106,9 +106,10 @@
 .has_sockets  = true, .sockets  = d,\
 .has_dies = true, .dies = e,\
 .has_clusters = true, .clusters = f,\
-.has_cores= true, .cores= g,\
-.has_threads  = true, .threads  = h,\
-.has_maxcpus  = true, .maxcpus  = i,\
+.has_modules  = true, .modules  = g,\
+.has_cores= true, .cores= h,\
+.has_threads  = true, .threads  = i,\
+.has_maxcpus  = true, .maxcpus  = j,\
 }
 
 /**
@@ -336,10 +337,10 @@ static const struct SMPTestData data_generic_valid[] = {
 /*
  * Unsupported parameters are always allowed to be set to '1'
  * config:
- *   -smp 8,drawers=1,books=1,sockets=2,dies=1,clusters=1,cores=2,\
- *threads=2,maxcpus=8
+ *   -smp 8,drawers=1,books=1,sockets=2,dies=1,clusters=1,modules=1,\
+ *cores=2,threads=2,maxcpus=8
  * expect: cpus=8,sockets=2,cores=2,threads=2,maxcpus=8 */
-.config = SMP_CONFIG_WITH_FULL_TOPO(8, 1, 1, 2, 1, 1, 2, 2, 8),
+.config = SMP_CONFIG_WITH_FULL_TOPO(8, 1, 1, 2, 1, 1, 1, 2, 2, 8),
 .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 2, 2, 8),
 .expect_prefer_cores   = CPU_TOPOLOGY_GENERIC(8, 2, 2, 2, 8),
 },
@@ -561,32 +562,37 @@ static const struct SMPTestData data_full_topo_invalid[] 
= {
 {
 /*
  * config: -smp 200,drawers=3,books=5,sockets=2,dies=4,\
- *  clusters=2,cores=7,threads=2,maxcpus=200
+ *  clusters=2,modules=3,cores=7,threads=2,\
+ *  maxcpus=200
  */
-.config = SMP_CONFIG_WITH_FULL_TOPO(200, 3, 5, 2, 4, 2, 7, 2, 200),
+.config = SMP_CONFIG_WITH_FULL_TOPO(200, 3, 5, 2, 4, 2, 3, 7, 2, 200),
 .expect_error = "Invalid CPU topology: "
 "product of the hierarchy must match maxcpus: "
 "drawers (3) * books (5) * sockets (2) * dies (4) * "
-"clusters (2) * cores (7) * threads (2) "
+"clusters (2) * modules (3) * cores (7) * threads (2) "
 "!= maxcpus (200)",
 }, {
 /*
- * config: -smp 3361,drawers=3,books=5,sockets=2,dies=4,\
- *  clusters=2,cores=7,threads=2,maxcpus=3360
+ * config: -smp 2881,drawers=3,books=5,sockets=2,dies=4,\
+ *  clusters=2,modules=3,cores=2,threads=2,
+ *  maxcpus=2880
  */
-.config = SMP_CONFIG_WITH_FULL_TOPO(3361, 3, 5, 2, 4, 2, 7, 2, 3360),
+.config = SMP_CONFIG_WITH_FULL_TOPO(2881, 3, 5, 2, 4,
+2, 3, 2, 2, 2880),
 .expect_error = "Invalid CPU topology: "
 "maxcpus must be equal to or greater than smp: "
-"drawers (3) * books (5) * sockets (2) * dies (4) * "
-"clusters (2) * cores (7) * threads (2) "
-"== maxcpus (3360) < smp_cpus (3361)",
+"drawers (3) * books (5) * sockets (2) * "
+"dies (4) * clusters (2) * modules (3) * "
+"cores (2) * threads (2) == maxcpus (2880) "
+"< smp

[PULL 12/15] tests/unit/test-smp-parse: Test "modules" parameter in -smp

2024-06-12 Thread Thomas Huth
From: Zhao Liu 

Cover the module cases in test-smp-parse.

Signed-off-by: Zhao Liu 
Tested-by: Yongwei Ma 
Message-ID: <20240529061925.350323-7-zhao1@intel.com>
Signed-off-by: Thomas Huth 
---
 tests/unit/test-smp-parse.c | 112 +---
 1 file changed, 103 insertions(+), 9 deletions(-)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 2214e47ba9..01832e5eda 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -48,17 +48,19 @@
 }
 
 /*
- * Currently a 4-level topology hierarchy is supported on PC machines
- *  -sockets/dies/cores/threads
+ * Currently a 5-level topology hierarchy is supported on PC machines
+ *  -sockets/dies/modules/cores/threads
  */
-#define SMP_CONFIG_WITH_DIES(ha, a, hb, b, hc, c, hd, d, he, e, hf, f) \
+#define SMP_CONFIG_WITH_MODS_DIES(ha, a, hb, b, hc, c, hd, d, \
+  he, e, hf, f, hg, g)\
 { \
 .has_cpus= ha, .cpus= a,  \
 .has_sockets = hb, .sockets = b,  \
 .has_dies= hc, .dies= c,  \
-.has_cores   = hd, .cores   = d,  \
-.has_threads = he, .threads = e,  \
-.has_maxcpus = hf, .maxcpus = f,  \
+.has_modules = hd, .modules = d,  \
+.has_cores   = he, .cores   = e,  \
+.has_threads = hf, .threads = f,  \
+.has_maxcpus = hg, .maxcpus = g,  \
 }
 
 /*
@@ -345,8 +347,14 @@ static const struct SMPTestData data_generic_valid[] = {
 
 static const struct SMPTestData data_generic_invalid[] = {
 {
+/* config: -smp 2,modules=2 */
+.config = SMP_CONFIG_WITH_MODS_DIES(T, 2, F, 0, F, 0, T, 2,
+F, 0, F, 0, F, 0),
+.expect_error = "modules > 1 not supported by this machine's CPU 
topology",
+}, {
 /* config: -smp 2,dies=2 */
-.config = SMP_CONFIG_WITH_DIES(T, 2, F, 0, T, 2, F, 0, F, 0, F, 0),
+.config = SMP_CONFIG_WITH_MODS_DIES(T, 2, F, 0, T, 2, F, 0,
+F, 0, F, 0, F, 0),
 .expect_error = "dies > 1 not supported by this machine's CPU 
topology",
 }, {
 /* config: -smp 2,clusters=2 */
@@ -397,17 +405,39 @@ static const struct SMPTestData data_generic_invalid[] = {
 },
 };
 
+static const struct SMPTestData data_with_modules_invalid[] = {
+{
+/* config: -smp 16,sockets=2,modules=2,cores=4,threads=2,maxcpus=16 */
+.config = SMP_CONFIG_WITH_MODS_DIES(T, 16, T, 2, F, 0, T, 2,
+T, 4, T, 2, T, 16),
+.expect_error = "Invalid CPU topology: "
+"product of the hierarchy must match maxcpus: "
+"sockets (2) * modules (2) * cores (4) * threads (2) "
+"!= maxcpus (16)",
+}, {
+/* config: -smp 34,sockets=2,modules=2,cores=4,threads=2,maxcpus=32 */
+.config = SMP_CONFIG_WITH_MODS_DIES(T, 34, T, 2, F, 0, T, 2,
+T, 4, T, 2, T, 32),
+.expect_error = "Invalid CPU topology: "
+"maxcpus must be equal to or greater than smp: "
+"sockets (2) * modules (2) * cores (4) * threads (2) "
+"== maxcpus (32) < smp_cpus (34)",
+},
+};
+
 static const struct SMPTestData data_with_dies_invalid[] = {
 {
 /* config: -smp 16,sockets=2,dies=2,cores=4,threads=2,maxcpus=16 */
-.config = SMP_CONFIG_WITH_DIES(T, 16, T, 2, T, 2, T, 4, T, 2, T, 16),
+.config = SMP_CONFIG_WITH_MODS_DIES(T, 16, T, 2, T, 2, F, 0,
+T, 4, T, 2, T, 16),
 .expect_error = "Invalid CPU topology: "
 "product of the hierarchy must match maxcpus: "
 "sockets (2) * dies (2) * cores (4) * threads (2) "
 "!= maxcpus (16)",
 }, {
 /* config: -smp 34,sockets=2,dies=2,cores=4,threads=2,maxcpus=32 */
-.config = SMP_CONFIG_WITH_DIES(T, 34, T, 2, T, 2, T, 4, T, 2, T, 32),
+.config = SMP_CONFIG_WITH_MODS_DIES(T, 34, T, 2, T, 2, F, 0,
+T, 4, T, 2, T, 32),
 .expect_error = "Invalid CPU topology: "
 "maxcpus must be equal to or greater than smp: "
 "sockets (2) * dies (2) * cores (4) * threads (2) "
@@ -861,6 +891,13 @@ static void machine_generic_invalid_class_init(ObjectClass 

[PULL 00/15] CPU-related test updates

2024-06-12 Thread Thomas Huth
The following changes since commit 80e8f0602168f451a93e71cbb1d59e93d745e62e:

  Merge tag 'bsd-user-misc-2024q2-pull-request' of gitlab.com:bsdimp/qemu into 
staging (2024-06-09 11:21:55 -0700)

are available in the Git repository at:

  https://gitlab.com/thuth/qemu.git tags/pull-request-2024-06-12

for you to fetch changes up to 26a09ead7351f117ae780781b347f014da03c20b:

  tests/tcg/s390x: Allow specifying extra QEMU options on the command line 
(2024-06-12 12:12:28 +0200)


* Fix loongarch64 avocado test
* Make qtests more flexible with regards to non-available CPU models
* Improvements for the test-smp-parse unit test


Ani Sinha (3):
  qtest/x86/numa-test: do not use the obsolete 'pentium' cpu
  tests/qtest/libqtest: add qtest_has_cpu_model() api
  tests/qtest/x86: check for availability of older cpu models before 
running tests

Ilya Leoshkevich (1):
  tests/tcg/s390x: Allow specifying extra QEMU options on the command line

Song Gao (1):
  tests/avocado: Update LoongArch bios file

Zhao Liu (8):
  tests/unit/test-smp-parse: Fix comments of drawers and books case
  tests/unit/test-smp-parse: Fix comment of parameters=1 case
  tests/unit/test-smp-parse: Fix an invalid topology case
  tests/unit/test-smp-parse: Use default parameters=0 when not set in -smp
  tests/unit/test-smp-parse: Make test cases aware of module level
  tests/unit/test-smp-parse: Test "modules" parameter in -smp
  tests/unit/test-smp-parse: Test "modules" and "dies" combination case
  tests/unit/test-smp-parse: Test the full 8-levels topology hierarchy

Zhenwei Pi (2):
  meson: Remove libibumad dependence
  test: Remove libibumad dependence

 meson.build|   4 +-
 tests/qtest/libqtest.h |   8 +
 tests/qtest/libqtest.c |  83 +
 tests/qtest/numa-test.c|   3 +-
 tests/qtest/test-x86-cpuid-compat.c| 170 ++
 tests/unit/test-smp-parse.c| 373 +
 scripts/ci/setup/ubuntu/ubuntu-2204-aarch64.yaml   |   1 -
 scripts/ci/setup/ubuntu/ubuntu-2204-s390x.yaml |   1 -
 tests/avocado/machine_loongarch.py |   8 +-
 tests/docker/dockerfiles/debian-amd64-cross.docker |   1 -
 tests/docker/dockerfiles/debian-arm64-cross.docker |   1 -
 tests/docker/dockerfiles/debian-armel-cross.docker |   1 -
 tests/docker/dockerfiles/debian-armhf-cross.docker |   1 -
 tests/docker/dockerfiles/debian-i686-cross.docker  |   1 -
 .../dockerfiles/debian-mips64el-cross.docker   |   1 -
 .../docker/dockerfiles/debian-mipsel-cross.docker  |   1 -
 .../docker/dockerfiles/debian-ppc64el-cross.docker |   1 -
 tests/docker/dockerfiles/debian-s390x-cross.docker |   1 -
 tests/docker/dockerfiles/debian.docker |   1 -
 tests/docker/dockerfiles/ubuntu2204.docker |   1 -
 tests/lcitool/projects/qemu.yml|   1 -
 tests/tcg/s390x/Makefile.softmmu-target|   2 +-
 22 files changed, 518 insertions(+), 147 deletions(-)




[PULL 08/15] tests/unit/test-smp-parse: Fix comment of parameters=1 case

2024-06-12 Thread Thomas Huth
From: Zhao Liu 

SMP_CONFIG_WITH_FULL_TOPO hasn't support module level, so the parameter
should indicate the "clusters".

Additionally, reorder the parameters of -smp to match the topology
hierarchy order.

Signed-off-by: Zhao Liu 
Reviewed-by: Thomas Huth 
Tested-by: Yongwei Ma 
Message-ID: <20240529061925.350323-3-zhao1@intel.com>
Signed-off-by: Thomas Huth 
---
 tests/unit/test-smp-parse.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index fa8e7d83a7..c9cbc89c21 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -333,7 +333,9 @@ static const struct SMPTestData data_generic_valid[] = {
 }, {
 /*
  * Unsupported parameters are always allowed to be set to '1'
- * config: -smp 
8,books=1,drawers=1,sockets=2,modules=1,dies=1,cores=2,threads=2,maxcpus=8
+ * config:
+ *   -smp 8,drawers=1,books=1,sockets=2,dies=1,clusters=1,cores=2,\
+ *threads=2,maxcpus=8
  * expect: cpus=8,sockets=2,cores=2,threads=2,maxcpus=8 */
 .config = SMP_CONFIG_WITH_FULL_TOPO(8, 1, 1, 2, 1, 1, 2, 2, 8),
 .expect_prefer_sockets = CPU_TOPOLOGY_GENERIC(8, 2, 2, 2, 8),
-- 
2.45.2




[PULL 02/15] qtest/x86/numa-test: do not use the obsolete 'pentium' cpu

2024-06-12 Thread Thomas Huth
From: Ani Sinha 

'pentium' cpu is old and obsolete and should be avoided for running tests if
its not strictly needed. Use 'max' cpu instead for generic non-cpu specific
numa test.

Reviewed-by: Thomas Huth 
Reviewed-by: Igor Mammedov 
Tested-by: Mario Casquero 
Signed-off-by: Ani Sinha 
Message-ID: <20240610155303.7933-2-anisi...@redhat.com>
Signed-off-by: Thomas Huth 
---
 tests/qtest/numa-test.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/qtest/numa-test.c b/tests/qtest/numa-test.c
index 5518f6596b..ede418963c 100644
--- a/tests/qtest/numa-test.c
+++ b/tests/qtest/numa-test.c
@@ -125,7 +125,8 @@ static void pc_numa_cpu(const void *data)
 QTestState *qts;
 g_autofree char *cli = NULL;
 
-cli = make_cli(data, "-cpu pentium -machine 
smp.cpus=8,smp.sockets=2,smp.cores=2,smp.threads=2 "
+cli = make_cli(data,
+"-cpu max -machine smp.cpus=8,smp.sockets=2,smp.cores=2,smp.threads=2 "
 "-numa node,nodeid=0,memdev=ram -numa node,nodeid=1 "
 "-numa cpu,node-id=1,socket-id=0 "
 "-numa cpu,node-id=0,socket-id=1,core-id=0 "
-- 
2.45.2




[PULL 11/15] tests/unit/test-smp-parse: Make test cases aware of module level

2024-06-12 Thread Thomas Huth
From: Zhao Liu 

Currently, -smp supports module level.

It is necessary to consider the effects of module in the test cases to
ensure that the calculations are correct. This is also the preparation
to add module test cases.

Signed-off-by: Zhao Liu 
Reviewed-by: Thomas Huth 
Tested-by: Yongwei Ma 
Message-ID: <20240529061925.350323-6-zhao1@intel.com>
Signed-off-by: Thomas Huth 
---
 tests/unit/test-smp-parse.c | 13 +++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index e3a0a9d12d..2214e47ba9 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -629,6 +629,7 @@ static char *smp_config_to_string(const SMPConfiguration 
*config)
 ".has_sockets  = %5s, sockets  = %" PRId64 ",\n"
 ".has_dies = %5s, dies = %" PRId64 ",\n"
 ".has_clusters = %5s, clusters = %" PRId64 ",\n"
+".has_modules  = %5s, modules  = %" PRId64 ",\n"
 ".has_cores= %5s, cores= %" PRId64 ",\n"
 ".has_threads  = %5s, threads  = %" PRId64 ",\n"
 ".has_maxcpus  = %5s, maxcpus  = %" PRId64 ",\n"
@@ -639,6 +640,7 @@ static char *smp_config_to_string(const SMPConfiguration 
*config)
 config->has_sockets ? "true" : "false", config->sockets,
 config->has_dies ? "true" : "false", config->dies,
 config->has_clusters ? "true" : "false", config->clusters,
+config->has_modules ? "true" : "false", config->modules,
 config->has_cores ? "true" : "false", config->cores,
 config->has_threads ? "true" : "false", config->threads,
 config->has_maxcpus ? "true" : "false", config->maxcpus);
@@ -679,6 +681,7 @@ static char *cpu_topology_to_string(const CpuTopology *topo,
 ".sockets= %u,\n"
 ".dies   = %u,\n"
 ".clusters   = %u,\n"
+".modules= %u,\n"
 ".cores  = %u,\n"
 ".threads= %u,\n"
 ".max_cpus   = %u,\n"
@@ -688,8 +691,8 @@ static char *cpu_topology_to_string(const CpuTopology *topo,
 "}",
 topo->cpus, topo->drawers, topo->books,
 topo->sockets, topo->dies, topo->clusters,
-topo->cores, topo->threads, topo->max_cpus,
-threads_per_socket, cores_per_socket,
+topo->modules, topo->cores, topo->threads,
+topo->max_cpus, threads_per_socket, cores_per_socket,
 has_clusters ? "true" : "false");
 }
 
@@ -732,6 +735,7 @@ static void check_parse(MachineState *ms, const 
SMPConfiguration *config,
 (ms->smp.sockets == expect_topo->sockets) &&
 (ms->smp.dies == expect_topo->dies) &&
 (ms->smp.clusters == expect_topo->clusters) &&
+(ms->smp.modules == expect_topo->modules) &&
 (ms->smp.cores == expect_topo->cores) &&
 (ms->smp.threads == expect_topo->threads) &&
 (ms->smp.max_cpus == expect_topo->max_cpus) &&
@@ -812,6 +816,11 @@ static void smp_parse_test(MachineState *ms, SMPTestData 
*data, bool is_valid)
 /* The parsed results of the unsupported parameters should be 1 */
 static void unsupported_params_init(const MachineClass *mc, SMPTestData *data)
 {
+if (!mc->smp_props.modules_supported) {
+data->expect_prefer_sockets.modules = 1;
+data->expect_prefer_cores.modules = 1;
+}
+
 if (!mc->smp_props.dies_supported) {
 data->expect_prefer_sockets.dies = 1;
 data->expect_prefer_cores.dies = 1;
-- 
2.45.2




[PULL 07/15] tests/unit/test-smp-parse: Fix comments of drawers and books case

2024-06-12 Thread Thomas Huth
From: Zhao Liu 

Fix the comments to match the actual configurations.

Signed-off-by: Zhao Liu 
Reviewed-by: Thomas Huth 
Tested-by: Yongwei Ma 
Message-ID: <20240529061925.350323-2-zhao1@intel.com>
Signed-off-by: Thomas Huth 
---
 tests/unit/test-smp-parse.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 9fdba24fce..fa8e7d83a7 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -474,8 +474,8 @@ static const struct SMPTestData data_with_drawers_invalid[] 
= {
 static const struct SMPTestData data_with_drawers_books_invalid[] = {
 {
 /*
- * config: -smp 200,drawers=2,books=2,sockets=2,cores=4,\
- * threads=2,maxcpus=200
+ * config: -smp 200,drawers=3,books=5,sockets=2,cores=4,\
+ *  threads=2,maxcpus=200
  */
 .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 200, T, 3, T, 5, T,
 2, T, 4, T, 2, T, 200),
@@ -485,8 +485,8 @@ static const struct SMPTestData 
data_with_drawers_books_invalid[] = {
 "cores (4) * threads (2) != maxcpus (200)",
 }, {
 /*
- * config: -smp 242,drawers=2,books=2,sockets=2,cores=4,\
- * threads=2,maxcpus=240
+ * config: -smp 242,drawers=3,books=5,sockets=2,cores=4,\
+ *  threads=2,maxcpus=240
  */
 .config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 242, T, 3, T, 5, T,
 2, T, 4, T, 2, T, 240),
-- 
2.45.2




[PULL 01/15] tests/avocado: Update LoongArch bios file

2024-06-12 Thread Thomas Huth
From: Song Gao 

The VM uses old bios to boot up only 1 cpu, causing the test case to fail.
Update the bios to solve this problem.

Reported-by: Thomas Huth 
Signed-off-by: Song Gao 
Message-ID: <20240604030058.2327145-1-gaos...@loongson.cn>
Signed-off-by: Thomas Huth 
---
 tests/avocado/machine_loongarch.py | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/avocado/machine_loongarch.py 
b/tests/avocado/machine_loongarch.py
index 7d8a3c1fa5..8de308f2d6 100644
--- a/tests/avocado/machine_loongarch.py
+++ b/tests/avocado/machine_loongarch.py
@@ -27,18 +27,18 @@ def test_loongarch64_devices(self):
 """
 
 kernel_url = ('https://github.com/yangxiaojuan-loongson/qemu-binary/'
-  'releases/download/binary-files/vmlinuz.efi')
+  'releases/download/2024-05-30/vmlinuz.efi')
 kernel_hash = '951b485b16e3788b6db03a3e1793c067009e31a2'
 kernel_path = self.fetch_asset(kernel_url, asset_hash=kernel_hash)
 
 initrd_url = ('https://github.com/yangxiaojuan-loongson/qemu-binary/'
-  'releases/download/binary-files/ramdisk')
+  'releases/download/2024-05-30/ramdisk')
 initrd_hash = 'c67658d9b2a447ce7db2f73ba3d373c9b2b90ab2'
 initrd_path = self.fetch_asset(initrd_url, asset_hash=initrd_hash)
 
 bios_url = ('https://github.com/yangxiaojuan-loongson/qemu-binary/'
-'releases/download/binary-files/QEMU_EFI.fd')
-bios_hash = ('dfc1bfba4853cd763b9d392d0031827e8addbca8')
+'releases/download/2024-05-30/QEMU_EFI.fd')
+bios_hash = ('f4d0966b5117d4cd82327c050dd668741046be69')
 bios_path = self.fetch_asset(bios_url, asset_hash=bios_hash)
 
 self.vm.set_console()
-- 
2.45.2




[PULL 09/15] tests/unit/test-smp-parse: Fix an invalid topology case

2024-06-12 Thread Thomas Huth
From: Zhao Liu 

Adjust the "cpus" parameter to match the comment configuration.

Signed-off-by: Zhao Liu 
Reviewed-by: Thomas Huth 
Tested-by: Yongwei Ma 
Message-ID: <20240529061925.350323-4-zhao1@intel.com>
Signed-off-by: Thomas Huth 
---
 tests/unit/test-smp-parse.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index c9cbc89c21..5d99e0d923 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -528,7 +528,7 @@ static const struct SMPTestData data_full_topo_invalid[] = {
  * config: -smp 1,drawers=3,books=5,sockets=2,dies=4,\
  *  clusters=2,cores=7,threads=3,maxcpus=5040
  */
-.config = SMP_CONFIG_WITH_FULL_TOPO(3361, 3, 5, 2, 4, 2, 7, 3, 5040),
+.config = SMP_CONFIG_WITH_FULL_TOPO(1, 3, 5, 2, 4, 2, 7, 3, 5040),
 .expect_error = "Invalid SMP CPUs 5040. The max CPUs supported "
 "by machine '" SMP_MACHINE_NAME "' is 4096",
 },
-- 
2.45.2




[PULL 05/15] meson: Remove libibumad dependence

2024-06-12 Thread Thomas Huth
From: zhenwei pi 

RDMA based migration has no dependence on libumad. libibverbs and
librdmacm are enough.
libumad was used by rdmacm-mux which has been already removed. It's
remained mistakenly.

Fixes: 1dfd42c4264b ("hw/rdma: Remove deprecated pvrdma device and rdmacm-mux 
helper")
Cc: Philippe Mathieu-Daudé 
Signed-off-by: zhenwei pi 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240611105427.61395-2-pizhen...@bytedance.com>
Signed-off-by: Thomas Huth 
---
 meson.build | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/meson.build b/meson.build
index ec59effca2..226b97ea26 100644
--- a/meson.build
+++ b/meson.build
@@ -1885,11 +1885,9 @@ endif
 
 rdma = not_found
 if not get_option('rdma').auto() or have_system
-  libumad = cc.find_library('ibumad', required: get_option('rdma'))
   rdma_libs = [cc.find_library('rdmacm', has_headers: ['rdma/rdma_cma.h'],
required: get_option('rdma')),
-   cc.find_library('ibverbs', required: get_option('rdma')),
-   libumad]
+   cc.find_library('ibverbs', required: get_option('rdma'))]
   rdma = declare_dependency(dependencies: rdma_libs)
   foreach lib: rdma_libs
 if not lib.found()
-- 
2.45.2




[PULL 10/15] tests/unit/test-smp-parse: Use default parameters=0 when not set in -smp

2024-06-12 Thread Thomas Huth
From: Zhao Liu 

Since -smp allows parameters=1 whether the level is supported by
machine, to avoid the test scenarios where the parameter defaults to 1
cause some errors to be masked, explicitly set undesired parameters to
0.

Signed-off-by: Zhao Liu 
Reviewed-by: Thomas Huth 
Tested-by: Yongwei Ma 
Message-ID: <20240529061925.350323-5-zhao1@intel.com>
Signed-off-by: Thomas Huth 
---
 tests/unit/test-smp-parse.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/tests/unit/test-smp-parse.c b/tests/unit/test-smp-parse.c
index 5d99e0d923..e3a0a9d12d 100644
--- a/tests/unit/test-smp-parse.c
+++ b/tests/unit/test-smp-parse.c
@@ -436,7 +436,7 @@ static const struct SMPTestData 
data_with_clusters_invalid[] = {
 static const struct SMPTestData data_with_books_invalid[] = {
 {
 /* config: -smp 16,books=2,sockets=2,cores=4,threads=2,maxcpus=16 */
-.config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 16, F, 1, T, 2, T,
+.config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 16, F, 0, T, 2, T,
 2, T, 4, T, 2, T, 16),
 .expect_error = "Invalid CPU topology: "
 "product of the hierarchy must match maxcpus: "
@@ -444,7 +444,7 @@ static const struct SMPTestData data_with_books_invalid[] = 
{
 "!= maxcpus (16)",
 }, {
 /* config: -smp 34,books=2,sockets=2,cores=4,threads=2,maxcpus=32 */
-.config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 34, F, 1, T, 2, T,
+.config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 34, F, 0, T, 2, T,
 2, T, 4, T, 2, T, 32),
 .expect_error = "Invalid CPU topology: "
 "maxcpus must be equal to or greater than smp: "
@@ -456,7 +456,7 @@ static const struct SMPTestData data_with_books_invalid[] = 
{
 static const struct SMPTestData data_with_drawers_invalid[] = {
 {
 /* config: -smp 16,drawers=2,sockets=2,cores=4,threads=2,maxcpus=16 */
-.config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 16, T, 2, F, 1, T,
+.config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 16, T, 2, F, 0, T,
 2, T, 4, T, 2, T, 16),
 .expect_error = "Invalid CPU topology: "
 "product of the hierarchy must match maxcpus: "
@@ -464,7 +464,7 @@ static const struct SMPTestData data_with_drawers_invalid[] 
= {
 "!= maxcpus (16)",
 }, {
 /* config: -smp 34,drawers=2,sockets=2,cores=4,threads=2,maxcpus=32 */
-.config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 34, T, 2, F, 1, T,
+.config = SMP_CONFIG_WITH_BOOKS_DRAWERS(T, 34, T, 2, F, 0, T,
 2, T, 4, T, 2, T, 32),
 .expect_error = "Invalid CPU topology: "
 "maxcpus must be equal to or greater than smp: "
-- 
2.45.2




[PULL 06/15] test: Remove libibumad dependence

2024-06-12 Thread Thomas Huth
From: zhenwei pi 

Remove libibumad dependence from the test environment.

Suggested-by: Philippe Mathieu-Daudé 
Signed-off-by: zhenwei pi 
Reviewed-by: Philippe Mathieu-Daudé 
Message-ID: <20240611105427.61395-3-pizhen...@bytedance.com>
Signed-off-by: Thomas Huth 
---
 scripts/ci/setup/ubuntu/ubuntu-2204-aarch64.yaml  | 1 -
 scripts/ci/setup/ubuntu/ubuntu-2204-s390x.yaml| 1 -
 tests/docker/dockerfiles/debian-amd64-cross.docker| 1 -
 tests/docker/dockerfiles/debian-arm64-cross.docker| 1 -
 tests/docker/dockerfiles/debian-armel-cross.docker| 1 -
 tests/docker/dockerfiles/debian-armhf-cross.docker| 1 -
 tests/docker/dockerfiles/debian-i686-cross.docker | 1 -
 tests/docker/dockerfiles/debian-mips64el-cross.docker | 1 -
 tests/docker/dockerfiles/debian-mipsel-cross.docker   | 1 -
 tests/docker/dockerfiles/debian-ppc64el-cross.docker  | 1 -
 tests/docker/dockerfiles/debian-s390x-cross.docker| 1 -
 tests/docker/dockerfiles/debian.docker| 1 -
 tests/docker/dockerfiles/ubuntu2204.docker| 1 -
 tests/lcitool/projects/qemu.yml   | 1 -
 14 files changed, 14 deletions(-)

diff --git a/scripts/ci/setup/ubuntu/ubuntu-2204-aarch64.yaml 
b/scripts/ci/setup/ubuntu/ubuntu-2204-aarch64.yaml
index 8d7d8725fb..fd5489cd82 100644
--- a/scripts/ci/setup/ubuntu/ubuntu-2204-aarch64.yaml
+++ b/scripts/ci/setup/ubuntu/ubuntu-2204-aarch64.yaml
@@ -49,7 +49,6 @@ packages:
   - libglusterfs-dev
   - libgnutls28-dev
   - libgtk-3-dev
-  - libibumad-dev
   - libibverbs-dev
   - libiscsi-dev
   - libjemalloc-dev
diff --git a/scripts/ci/setup/ubuntu/ubuntu-2204-s390x.yaml 
b/scripts/ci/setup/ubuntu/ubuntu-2204-s390x.yaml
index 16050a5058..afa04502cf 100644
--- a/scripts/ci/setup/ubuntu/ubuntu-2204-s390x.yaml
+++ b/scripts/ci/setup/ubuntu/ubuntu-2204-s390x.yaml
@@ -49,7 +49,6 @@ packages:
   - libglusterfs-dev
   - libgnutls28-dev
   - libgtk-3-dev
-  - libibumad-dev
   - libibverbs-dev
   - libiscsi-dev
   - libjemalloc-dev
diff --git a/tests/docker/dockerfiles/debian-amd64-cross.docker 
b/tests/docker/dockerfiles/debian-amd64-cross.docker
index f8c61d1191..8058695979 100644
--- a/tests/docker/dockerfiles/debian-amd64-cross.docker
+++ b/tests/docker/dockerfiles/debian-amd64-cross.docker
@@ -105,7 +105,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
   libglusterfs-dev:amd64 \
   libgnutls28-dev:amd64 \
   libgtk-3-dev:amd64 \
-  libibumad-dev:amd64 \
   libibverbs-dev:amd64 \
   libiscsi-dev:amd64 \
   libjemalloc-dev:amd64 \
diff --git a/tests/docker/dockerfiles/debian-arm64-cross.docker 
b/tests/docker/dockerfiles/debian-arm64-cross.docker
index 6510872279..15457d7657 100644
--- a/tests/docker/dockerfiles/debian-arm64-cross.docker
+++ b/tests/docker/dockerfiles/debian-arm64-cross.docker
@@ -105,7 +105,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
   libglusterfs-dev:arm64 \
   libgnutls28-dev:arm64 \
   libgtk-3-dev:arm64 \
-  libibumad-dev:arm64 \
   libibverbs-dev:arm64 \
   libiscsi-dev:arm64 \
   libjemalloc-dev:arm64 \
diff --git a/tests/docker/dockerfiles/debian-armel-cross.docker 
b/tests/docker/dockerfiles/debian-armel-cross.docker
index f227d42987..c26ffc2e9e 100644
--- a/tests/docker/dockerfiles/debian-armel-cross.docker
+++ b/tests/docker/dockerfiles/debian-armel-cross.docker
@@ -108,7 +108,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
   libglusterfs-dev:armel \
   libgnutls28-dev:armel \
   libgtk-3-dev:armel \
-  libibumad-dev:armel \
   libibverbs-dev:armel \
   libiscsi-dev:armel \
   libjemalloc-dev:armel \
diff --git a/tests/docker/dockerfiles/debian-armhf-cross.docker 
b/tests/docker/dockerfiles/debian-armhf-cross.docker
index 58bdf07223..8f87656d89 100644
--- a/tests/docker/dockerfiles/debian-armhf-cross.docker
+++ b/tests/docker/dockerfiles/debian-armhf-cross.docker
@@ -105,7 +105,6 @@ RUN export DEBIAN_FRONTEND=noninteractive && \
   libglusterfs-dev:armhf \
   libgnutls28-dev:armhf \
   libgtk-3-dev:armhf \
-  libibumad-dev:armhf \
   libibverbs-dev:armhf \
   libiscsi-dev:armhf \
   libjemalloc-dev:armhf \
diff --git a/tests/docker/dockerfiles/debian-i686-cross.docker 
b/tests/docker/dockerfiles/debian-i686-cross.docker
index 9f4102be8f..f1e5b0b877 100644
--- a/tests/docker/dockerfiles/debian-i686-cross.docker
+++ b/tests/docker/dockerfiles/debian-i686-cross.docker
@@ -108,7 +108,6 @@ RUN export DEBIAN_FRONTEND=noninteractive &a

Re: [PATCH 5/8] tests/unit/test-smp-parse: Make test cases aware of module level

2024-06-12 Thread Thomas Huth

On 29/05/2024 08.19, Zhao Liu wrote:

Currently, -smp supports module level.

It is necessary to consider the effects of module in the test cases to
ensure that the calculations are correct. This is also the preparation
to add module test cases.

Signed-off-by: Zhao Liu 
---
  tests/unit/test-smp-parse.c | 13 +++--
  1 file changed, 11 insertions(+), 2 deletions(-)


Reviewed-by: Thomas Huth 




  1   2   3   4   5   6   7   8   9   10   >