[PATCH 0/8] Allow check-qtest with "--without-default-devices"

[PATCH 0/8] Allow check-qtest with "--without-default-devices"

[Thomas Huth]

With these fixes in place the qtests now work when QEMU has been
configured with "--without-default-devices". The final patch enables
most targets in the related test in the CI.

The riscv patch is necessary for fixing a build failure that occurs
when compiling this target without the default devices.

Thomas Huth (8):
  tests/qtest/migration: Add a check for the availability of the "pc"
    machine
  tests/qtest/cdrom-test: Improve the machine detection in the cdrom
    test
  tests/qtest/boot-order-test: Make the machine name mandatory in this
    test
  tests/qtest/hd-geo-test: Check for availability of "pc" machine before
    using it
  tests/qtest/meson.build: Add more CONFIG switches checks for the x86
    tests
  tests/qtest: Disable numa-test if the default machine is not available
  Revert "target/riscv: Restrict semihosting to TCG"
  .gitlab-ci.d/buildtest: Build most targets in the
    build-without-defaults job

 tests/qtest/boot-order-test.c |  4 +-
 tests/qtest/cdrom-test.c      | 77 +++++++++++++++++++----------------
 tests/qtest/hd-geo-test.c     | 71 +++++++++++++++++---------------
 tests/qtest/migration-test.c  |  6 ++-
 .gitlab-ci.d/buildtest.yml    |  9 +---
 target/riscv/Kconfig          |  4 +-
 tests/qtest/meson.build       | 31 +++++++-------
 7 files changed, 106 insertions(+), 96 deletions(-)

-- 
2.46.0

[PATCH 1/8] tests/qtest/migration: Add a check for the availability of the "pc" machine

[Thomas Huth]

The test_vcpu_dirty_limit is the only test that does not check for the
availability of the machine before starting the test, so it fails when
QEMU has been configured with --without-default-devices. Add a check for
the "pc" machine type to fix it.

Reviewed-by: Fabiano Rosas <farosas@suse.de>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/migration-test.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/migration-test.c b/tests/qtest/migration-test.c
index 6c06100d91..8fee18dfbe 100644
--- a/tests/qtest/migration-test.c
+++ b/tests/qtest/migration-test.c
@@ -4026,8 +4026,10 @@ int main(int argc, char **argv)
     if (g_str_equal(arch, "x86_64") && has_kvm && kvm_dirty_ring_supported()) {
         migration_test_add("/migration/dirty_ring",
                            test_precopy_unix_dirty_ring);
-        migration_test_add("/migration/vcpu_dirty_limit",
-                           test_vcpu_dirty_limit);
+        if (qtest_has_machine("pc")) {
+            migration_test_add("/migration/vcpu_dirty_limit",
+                               test_vcpu_dirty_limit);
+        }
     }
 
     ret = g_test_run();
-- 
2.46.0

[PATCH 2/8] tests/qtest/cdrom-test: Improve the machine detection in the cdrom test

[Thomas Huth]

When configuring QEMU with the --without-default-devices switch, these
tests are currently failing since they assume that the "pc" and "q35"
machines are always available. Add some proper checks to make the test
work without these machines, too.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/cdrom-test.c | 77 ++++++++++++++++++++++------------------
 1 file changed, 42 insertions(+), 35 deletions(-)

diff --git a/tests/qtest/cdrom-test.c b/tests/qtest/cdrom-test.c
index 5d89e62515..9d72b24e4b 100644
--- a/tests/qtest/cdrom-test.c
+++ b/tests/qtest/cdrom-test.c
@@ -135,13 +135,35 @@ static void add_x86_tests(void)
         return;
     }
 
-    qtest_add_data_func("cdrom/boot/default", "-cdrom ", test_cdboot);
-    if (qtest_has_device("virtio-scsi-ccw")) {
-        qtest_add_data_func("cdrom/boot/virtio-scsi",
-                            "-device virtio-scsi -device scsi-cd,drive=cdr "
-                            "-blockdev file,node-name=cdr,filename=",
-                            test_cdboot);
+    if (qtest_has_machine("pc")) {
+        qtest_add_data_func("cdrom/boot/default", "-cdrom ", test_cdboot);
+        if (qtest_has_device("virtio-scsi-ccw")) {
+            qtest_add_data_func("cdrom/boot/virtio-scsi",
+                                "-device virtio-scsi -device scsi-cd,drive=cdr "
+                                "-blockdev file,node-name=cdr,filename=",
+                                test_cdboot);
+        }
+
+        if (qtest_has_device("am53c974")) {
+            qtest_add_data_func("cdrom/boot/am53c974",
+                                "-device am53c974 -device scsi-cd,drive=cd1 "
+                                "-drive if=none,id=cd1,format=raw,file=",
+                                test_cdboot);
+        }
+        if (qtest_has_device("dc390")) {
+            qtest_add_data_func("cdrom/boot/dc390",
+                                "-device dc390 -device scsi-cd,drive=cd1 "
+                                "-blockdev file,node-name=cd1,filename=",
+                                test_cdboot);
+        }
+        if (qtest_has_device("lsi53c895a")) {
+            qtest_add_data_func("cdrom/boot/lsi53c895a",
+                                "-device lsi53c895a -device scsi-cd,drive=cd1 "
+                                "-blockdev file,node-name=cd1,filename=",
+                                test_cdboot);
+        }
     }
+
     /*
      * Unstable CI test under load
      * See https://lists.gnu.org/archive/html/qemu-devel/2019-02/msg05509.html
@@ -150,35 +172,20 @@ static void add_x86_tests(void)
         qtest_add_data_func("cdrom/boot/isapc", "-M isapc "
                             "-drive if=ide,media=cdrom,file=", test_cdboot);
     }
-    if (qtest_has_device("am53c974")) {
-        qtest_add_data_func("cdrom/boot/am53c974",
-                            "-device am53c974 -device scsi-cd,drive=cd1 "
-                            "-drive if=none,id=cd1,format=raw,file=",
-                            test_cdboot);
-    }
-    if (qtest_has_device("dc390")) {
-        qtest_add_data_func("cdrom/boot/dc390",
-                            "-device dc390 -device scsi-cd,drive=cd1 "
-                            "-blockdev file,node-name=cd1,filename=",
-                            test_cdboot);
-    }
-    if (qtest_has_device("lsi53c895a")) {
-        qtest_add_data_func("cdrom/boot/lsi53c895a",
-                            "-device lsi53c895a -device scsi-cd,drive=cd1 "
-                            "-blockdev file,node-name=cd1,filename=",
-                            test_cdboot);
-    }
-    if (qtest_has_device("megasas")) {
-        qtest_add_data_func("cdrom/boot/megasas", "-M q35 "
-                            "-device megasas -device scsi-cd,drive=cd1 "
-                            "-blockdev file,node-name=cd1,filename=",
-                            test_cdboot);
-    }
-    if (qtest_has_device("megasas-gen2")) {
-        qtest_add_data_func("cdrom/boot/megasas-gen2", "-M q35 "
-                            "-device megasas-gen2 -device scsi-cd,drive=cd1 "
-                            "-blockdev file,node-name=cd1,filename=",
-                            test_cdboot);
+
+    if (qtest_has_machine("q35")) {
+        if (qtest_has_device("megasas")) {
+            qtest_add_data_func("cdrom/boot/megasas", "-M q35 "
+                                "-device megasas -device scsi-cd,drive=cd1 "
+                                "-blockdev file,node-name=cd1,filename=",
+                                test_cdboot);
+        }
+        if (qtest_has_device("megasas-gen2")) {
+            qtest_add_data_func("cdrom/boot/megasas-gen2", "-M q35 "
+                                "-device megasas-gen2 -device scsi-cd,drive=cd1 "
+                                "-blockdev file,node-name=cd1,filename=",
+                                test_cdboot);
+        }
     }
 }
 
-- 
2.46.0

[PATCH 3/8] tests/qtest/boot-order-test: Make the machine name mandatory in this test

[Thomas Huth]

Let's make sure that we always pass a machine name to the test_boot_orders()
function, so we can check whether the machine is available in the binary
and skip the test in case it is not included in the build.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/boot-order-test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/boot-order-test.c b/tests/qtest/boot-order-test.c
index 8f2b6ef05a..c67b8cfe16 100644
--- a/tests/qtest/boot-order-test.c
+++ b/tests/qtest/boot-order-test.c
@@ -31,7 +31,7 @@ static void test_a_boot_order(const char *machine,
     uint64_t actual;
     QTestState *qts;
 
-    if (machine && !qtest_has_machine(machine)) {
+    if (!qtest_has_machine(machine)) {
         g_test_skip("Machine is not available");
         return;
     }
@@ -107,7 +107,7 @@ static const boot_order_test test_cases_pc[] = {
 
 static void test_pc_boot_order(void)
 {
-    test_boot_orders(NULL, read_boot_order_pc, test_cases_pc);
+    test_boot_orders("pc", read_boot_order_pc, test_cases_pc);
 }
 
 static uint64_t read_boot_order_pmac(QTestState *qts)
-- 
2.46.0

[PATCH 4/8] tests/qtest/hd-geo-test: Check for availability of "pc" machine before using it

[Thomas Huth]

In case QEMU has been configured with "--without-default-devices", the
"pc" machine type might be missing in the binary. We should check for
its availability before using it.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/hd-geo-test.c | 71 +++++++++++++++++++++------------------
 1 file changed, 38 insertions(+), 33 deletions(-)

diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
index d08bffad91..85eb8d7668 100644
--- a/tests/qtest/hd-geo-test.c
+++ b/tests/qtest/hd-geo-test.c
@@ -1074,17 +1074,26 @@ int main(int argc, char **argv)
         }
     }
 
-    qtest_add_func("hd-geo/ide/none", test_ide_none);
-    qtest_add_func("hd-geo/ide/drive/mbr/blank", test_ide_drive_mbr_blank);
-    qtest_add_func("hd-geo/ide/drive/mbr/lba", test_ide_drive_mbr_lba);
-    qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs);
-    qtest_add_func("hd-geo/ide/drive/cd_0", test_ide_drive_cd_0);
-    qtest_add_func("hd-geo/ide/device/mbr/blank", test_ide_device_mbr_blank);
-    qtest_add_func("hd-geo/ide/device/mbr/lba", test_ide_device_mbr_lba);
-    qtest_add_func("hd-geo/ide/device/mbr/chs", test_ide_device_mbr_chs);
-    qtest_add_func("hd-geo/ide/device/user/chs", test_ide_device_user_chs);
-    qtest_add_func("hd-geo/ide/device/user/chst", test_ide_device_user_chst);
-    if (have_qemu_img()) {
+    if (qtest_has_machine("pc")) {
+        qtest_add_func("hd-geo/ide/none", test_ide_none);
+        qtest_add_func("hd-geo/ide/drive/mbr/blank", test_ide_drive_mbr_blank);
+        qtest_add_func("hd-geo/ide/drive/mbr/lba", test_ide_drive_mbr_lba);
+        qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs);
+        qtest_add_func("hd-geo/ide/drive/cd_0", test_ide_drive_cd_0);
+        qtest_add_func("hd-geo/ide/device/mbr/blank", test_ide_device_mbr_blank);
+        qtest_add_func("hd-geo/ide/device/mbr/lba", test_ide_device_mbr_lba);
+        qtest_add_func("hd-geo/ide/device/mbr/chs", test_ide_device_mbr_chs);
+        qtest_add_func("hd-geo/ide/device/user/chs", test_ide_device_user_chs);
+        qtest_add_func("hd-geo/ide/device/user/chst", test_ide_device_user_chst);
+    }
+
+    if (!have_qemu_img()) {
+        g_test_message("QTEST_QEMU_IMG not set or qemu-img missing; "
+                       "skipping hd-geo/override/* tests");
+        goto test_add_done;
+    }
+
+    if (qtest_has_machine("pc")) {
         qtest_add_func("hd-geo/override/ide", test_override_ide);
         if (qtest_has_device("lsi53c895a")) {
             qtest_add_func("hd-geo/override/scsi", test_override_scsi);
@@ -1104,30 +1113,26 @@ int main(int argc, char **argv)
             qtest_add_func("hd-geo/override/virtio_blk",
                            test_override_virtio_blk);
         }
+    }
 
-        if (qtest_has_machine("q35")) {
-            qtest_add_func("hd-geo/override/sata", test_override_sata);
-            qtest_add_func("hd-geo/override/zero_chs_q35",
-                           test_override_zero_chs_q35);
-            if (qtest_has_device("lsi53c895a")) {
-                qtest_add_func("hd-geo/override/scsi_q35",
-                               test_override_scsi_q35);
-            }
-            if (qtest_has_device("virtio-scsi-pci")) {
-                qtest_add_func("hd-geo/override/scsi_hot_unplug_q35",
-                               test_override_scsi_hot_unplug_q35);
-            }
-            if (qtest_has_device("virtio-blk-pci")) {
-                qtest_add_func("hd-geo/override/virtio_hot_unplug_q35",
-                               test_override_virtio_hot_unplug_q35);
-                qtest_add_func("hd-geo/override/virtio_blk_q35",
-                               test_override_virtio_blk_q35);
-            }
-
+    if (qtest_has_machine("q35")) {
+        qtest_add_func("hd-geo/override/sata", test_override_sata);
+        qtest_add_func("hd-geo/override/zero_chs_q35",
+                       test_override_zero_chs_q35);
+        if (qtest_has_device("lsi53c895a")) {
+            qtest_add_func("hd-geo/override/scsi_q35",
+                           test_override_scsi_q35);
+        }
+        if (qtest_has_device("virtio-scsi-pci")) {
+            qtest_add_func("hd-geo/override/scsi_hot_unplug_q35",
+                           test_override_scsi_hot_unplug_q35);
+        }
+        if (qtest_has_device("virtio-blk-pci")) {
+            qtest_add_func("hd-geo/override/virtio_hot_unplug_q35",
+                           test_override_virtio_hot_unplug_q35);
+            qtest_add_func("hd-geo/override/virtio_blk_q35",
+                           test_override_virtio_blk_q35);
         }
-    } else {
-        g_test_message("QTEST_QEMU_IMG not set or qemu-img missing; "
-                       "skipping hd-geo/override/* tests");
     }
 
 test_add_done:
-- 
2.46.0

[PATCH 5/8] tests/qtest/meson.build: Add more CONFIG switches checks for the x86 tests

[Thomas Huth]

When configuring QEMU with "--without-default-devices", currently a lot
of the x86 qtests are failing since they silently assume that a certain
device or the i440fx pc machine is available. Add more checks for CONFIG
switches here to not run those tests in case the corresponding device is
not available.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/meson.build | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 2f0d3ef080..587e1dc47e 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -49,7 +49,15 @@ qtests_filter = \
 qtests_i386 = \
   (slirp.found() ? ['pxe-test'] : []) + \
   qtests_filter + \
-  (have_tools ? ['ahci-test'] : []) +                                                       \
+  (config_all_devices.has_key('CONFIG_ACPI_VMGENID') ? ['vmgenid-test'] : []) +             \
+  (config_all_devices.has_key('CONFIG_AHCI_ICH9') and have_tools ? ['ahci-test'] : []) +    \
+  (config_all_devices.has_key('CONFIG_AHCI_ICH9') ? ['tco-test'] : []) +                    \
+  (config_all_devices.has_key('CONFIG_FDC_ISA') ? ['fdc-test'] : []) +                      \
+  (config_all_devices.has_key('CONFIG_I440FX') ? ['fw_cfg-test'] : []) +                    \
+  (config_all_devices.has_key('CONFIG_I440FX') ? ['i440fx-test'] : []) +                    \
+  (config_all_devices.has_key('CONFIG_I440FX') ? ['ide-test'] : []) +                       \
+  (config_all_devices.has_key('CONFIG_I440FX') ? ['numa-test'] : []) +                      \
+  (config_all_devices.has_key('CONFIG_I440FX') ? ['test-x86-cpuid-compat'] : []) +          \
   (config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] : []) +           \
   (config_all_devices.has_key('CONFIG_SGA') ? ['boot-serial-test'] : []) +                  \
   (config_all_devices.has_key('CONFIG_ISA_IPMI_KCS') ? ['ipmi-kcs-test'] : []) +            \
@@ -63,6 +71,7 @@ qtests_i386 = \
   (config_all_devices.has_key('CONFIG_I82801B11') ? ['i82801b11-test'] : []) +             \
   (config_all_devices.has_key('CONFIG_IOH3420') ? ['ioh3420-test'] : []) +                  \
   (config_all_devices.has_key('CONFIG_LPC_ICH9') ? ['lpc-ich9-test'] : []) +              \
+  (config_all_devices.has_key('CONFIG_MC146818RTC') ? ['rtc-test'] : []) +                  \
   (config_all_devices.has_key('CONFIG_USB_UHCI') ? ['usb-hcd-uhci-test'] : []) +            \
   (config_all_devices.has_key('CONFIG_USB_UHCI') and                                        \
    config_all_devices.has_key('CONFIG_USB_EHCI') ? ['usb-hcd-ehci-test'] : []) +            \
@@ -76,6 +85,7 @@ qtests_i386 = \
   (config_all_devices.has_key('CONFIG_MEGASAS_SCSI_PCI') ? ['fuzz-megasas-test'] : []) +    \
   (config_all_devices.has_key('CONFIG_LSI_SCSI_PCI') ? ['fuzz-lsi53c895a-test'] : []) +     \
   (config_all_devices.has_key('CONFIG_VIRTIO_SCSI') ? ['fuzz-virtio-scsi-test'] : []) +     \
+  (config_all_devices.has_key('CONFIG_Q35') ? ['q35-test'] : []) +                          \
   (config_all_devices.has_key('CONFIG_SB16') ? ['fuzz-sb16-test'] : []) +                   \
   (config_all_devices.has_key('CONFIG_SDHCI_PCI') ? ['fuzz-sdcard-test'] : []) +            \
   (config_all_devices.has_key('CONFIG_ESP_PCI') ? ['am53c974-test'] : []) +                 \
@@ -91,25 +101,16 @@ qtests_i386 = \
    config_all_devices.has_key('CONFIG_PARALLEL') ? ['bios-tables-test'] : []) +             \
   qtests_pci +                                                                              \
   qtests_cxl +                                                                              \
-  ['fdc-test',
-   'ide-test',
+  [
    'hd-geo-test',
    'boot-order-test',
-   'rtc-test',
-   'i440fx-test',
-   'fw_cfg-test',
    'device-plug-test',
    'drive_del-test',
-   'tco-test',
    'cpu-plug-test',
-   'q35-test',
-   'vmgenid-test',
    'migration-test',
-   'test-x86-cpuid-compat',
-   'numa-test'
   ]
 
-if dbus_display
+if dbus_display and config_all_devices.has_key('CONFIG_VGA')
   qtests_i386 += ['dbus-display-test']
 endif
 
-- 
2.46.0

[PATCH 6/8] tests/qtest: Disable numa-test if the default machine is not available

[Thomas Huth]

The numa-test needs a default machine in the target binary to work
successfully, so don't try to run this test if the corresponding
machine has not been enabled, e.g. when QEMU has been configured with
"--without-default-devices".

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/meson.build | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/tests/qtest/meson.build b/tests/qtest/meson.build
index 587e1dc47e..70f240e8b1 100644
--- a/tests/qtest/meson.build
+++ b/tests/qtest/meson.build
@@ -141,7 +141,8 @@ qtests_hppa = ['boot-serial-test'] + \
   (config_all_devices.has_key('CONFIG_VGA') ? ['display-vga-test'] : [])
 
 qtests_loongarch64 = qtests_filter + \
-  ['boot-serial-test', 'numa-test']
+  (config_all_devices.has_key('CONFIG_LOONGARCH_VIRT') ? ['numa-test'] : []) + \
+  ['boot-serial-test']
 
 qtests_m68k = ['boot-serial-test'] + \
   qtests_filter
@@ -174,11 +175,12 @@ qtests_ppc64 = \
   (config_all_devices.has_key('CONFIG_POWERNV') ? ['pnv-xscom-test'] : []) +                 \
   (config_all_devices.has_key('CONFIG_POWERNV') ? ['pnv-spi-seeprom-test'] : []) +           \
   (config_all_devices.has_key('CONFIG_POWERNV') ? ['pnv-host-i2c-test'] : []) +              \
+  (config_all_devices.has_key('CONFIG_PSERIES') ? ['numa-test'] : []) +                      \
   (config_all_devices.has_key('CONFIG_PSERIES') ? ['rtas-test'] : []) +                      \
   (slirp.found() ? ['pxe-test'] : []) +              \
   (config_all_devices.has_key('CONFIG_USB_UHCI') ? ['usb-hcd-uhci-test'] : []) +             \
   (config_all_devices.has_key('CONFIG_USB_XHCI_NEC') ? ['usb-hcd-xhci-test'] : []) +         \
-  qtests_pci + ['migration-test', 'numa-test', 'cpu-plug-test', 'drive_del-test']
+  qtests_pci + ['migration-test', 'cpu-plug-test', 'drive_del-test']
 
 qtests_sh4 = (config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] : [])
 qtests_sh4eb = (config_all_devices.has_key('CONFIG_ISA_TESTDEV') ? ['endianness-test'] : [])
-- 
2.46.0

[PATCH 7/8] Revert "target/riscv: Restrict semihosting to TCG"

[Thomas Huth]

This reverts commit 10425887ba54241be1ce97f8935fc320332b531c.

Using "imply" instead of "select" is causing a build failure:

 /usr/bin/ld: libqemu-riscv32-softmmu.a.p/target_riscv_cpu_helper.c.o: in function `riscv_cpu_do_interrupt':
 .../qemu/target/riscv/cpu_helper.c:1678:(.text+0x2214): undefined reference to `do_common_semihosting'

Thus revert to fix the build.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 target/riscv/Kconfig | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/target/riscv/Kconfig b/target/riscv/Kconfig
index c332616d36..5f30df22f2 100644
--- a/target/riscv/Kconfig
+++ b/target/riscv/Kconfig
@@ -1,9 +1,9 @@
 config RISCV32
     bool
-    imply ARM_COMPATIBLE_SEMIHOSTING if TCG
+    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
     select DEVICE_TREE # needed by boot.c
 
 config RISCV64
     bool
-    imply ARM_COMPATIBLE_SEMIHOSTING if TCG
+    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
     select DEVICE_TREE # needed by boot.c
-- 
2.46.0

[PATCH 8/8] .gitlab-ci.d/buildtest: Build most targets in the build-without-defaults job

[Thomas Huth]

Now that all the qtests are able to deal with builds that use the
"--without-default-devices" configuration switch, we can add all
targets to the build-without-defaults job. But to avoid burning too
much CI cycles in this job, exclude some targets where we already
have similar test coverage by a related target.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 .gitlab-ci.d/buildtest.yml | 9 +--------
 1 file changed, 1 insertion(+), 8 deletions(-)

diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
index 0c624813cf..dd1edf88db 100644
--- a/.gitlab-ci.d/buildtest.yml
+++ b/.gitlab-ci.d/buildtest.yml
@@ -653,9 +653,6 @@ build-tci:
     - make check-tcg
 
 # Check our reduced build configurations
-# requires libfdt: aarch64, arm, loongarch64, microblaze, microblazeel,
-#   or1k, ppc64, riscv32, riscv64, rx
-# fails qtest without boards: i386, x86_64
 build-without-defaults:
   extends: .native_build_job_template
   needs:
@@ -669,11 +666,7 @@ build-without-defaults:
       --disable-pie
       --disable-qom-cast-debug
       --disable-strip
-    TARGETS: alpha-softmmu avr-softmmu cris-softmmu hppa-softmmu m68k-softmmu
-      mips-softmmu mips64-softmmu mipsel-softmmu mips64el-softmmu
-      ppc-softmmu s390x-softmmu sh4-softmmu sh4eb-softmmu sparc-softmmu
-      sparc64-softmmu tricore-softmmu xtensa-softmmu xtensaeb-softmmu
-      hexagon-linux-user i386-linux-user s390x-linux-user
+      --target-list-exclude=aarch64-softmmu,microblaze-softmmu,mips64-softmmu,mipsel-softmmu,ppc64-softmmu,sh4el-softmmu,xtensa-softmmu,x86_64-softmmu
     MAKE_CHECK_ARGS: check
 
 build-libvhost-user:
-- 
2.46.0

Re: [PATCH 7/8] Revert "target/riscv: Restrict semihosting to TCG"

[Peter Maydell]

On Thu, 5 Sept 2024 at 20:16, Thomas Huth <thuth@redhat.com> wrote:
>
> This reverts commit 10425887ba54241be1ce97f8935fc320332b531c.
>
> Using "imply" instead of "select" is causing a build failure:
>
>  /usr/bin/ld: libqemu-riscv32-softmmu.a.p/target_riscv_cpu_helper.c.o: in function `riscv_cpu_do_interrupt':
>  .../qemu/target/riscv/cpu_helper.c:1678:(.text+0x2214): undefined reference to `do_common_semihosting'
>
> Thus revert to fix the build.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  target/riscv/Kconfig | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/target/riscv/Kconfig b/target/riscv/Kconfig
> index c332616d36..5f30df22f2 100644
> --- a/target/riscv/Kconfig
> +++ b/target/riscv/Kconfig
> @@ -1,9 +1,9 @@
>  config RISCV32
>      bool
> -    imply ARM_COMPATIBLE_SEMIHOSTING if TCG
> +    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
>      select DEVICE_TREE # needed by boot.c
>
>  config RISCV64
>      bool
> -    imply ARM_COMPATIBLE_SEMIHOSTING if TCG
> +    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
>      select DEVICE_TREE # needed by boot.c

This will break the intended "we don't need semihosting if
this is a KVM-only compile", though. Can we fix the
build problem use see with
 "select ARM_COMPATIBLE_SEMIHOSTING if TCG"

?

-- PMM

Re: [PATCH 4/8] tests/qtest/hd-geo-test: Check for availability of "pc" machine before using it

[Philippe Mathieu-Daudé]

On 5/9/24 21:14, Thomas Huth wrote:
> In case QEMU has been configured with "--without-default-devices", the
> "pc" machine type might be missing in the binary. We should check for
> its availability before using it.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/qtest/hd-geo-test.c | 71 +++++++++++++++++++++------------------
>   1 file changed, 38 insertions(+), 33 deletions(-)
> 
> diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
> index d08bffad91..85eb8d7668 100644
> --- a/tests/qtest/hd-geo-test.c
> +++ b/tests/qtest/hd-geo-test.c
> @@ -1074,17 +1074,26 @@ int main(int argc, char **argv)
>           }
>       }
>   
> -    qtest_add_func("hd-geo/ide/none", test_ide_none);
> -    qtest_add_func("hd-geo/ide/drive/mbr/blank", test_ide_drive_mbr_blank);
> -    qtest_add_func("hd-geo/ide/drive/mbr/lba", test_ide_drive_mbr_lba);
> -    qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs);
> -    qtest_add_func("hd-geo/ide/drive/cd_0", test_ide_drive_cd_0);
> -    qtest_add_func("hd-geo/ide/device/mbr/blank", test_ide_device_mbr_blank);
> -    qtest_add_func("hd-geo/ide/device/mbr/lba", test_ide_device_mbr_lba);
> -    qtest_add_func("hd-geo/ide/device/mbr/chs", test_ide_device_mbr_chs);
> -    qtest_add_func("hd-geo/ide/device/user/chs", test_ide_device_user_chs);
> -    qtest_add_func("hd-geo/ide/device/user/chst", test_ide_device_user_chst);
> -    if (have_qemu_img()) {
> +    if (qtest_has_machine("pc")) {
> +        qtest_add_func("hd-geo/ide/none", test_ide_none);
> +        qtest_add_func("hd-geo/ide/drive/mbr/blank", test_ide_drive_mbr_blank);
> +        qtest_add_func("hd-geo/ide/drive/mbr/lba", test_ide_drive_mbr_lba);
> +        qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs);
> +        qtest_add_func("hd-geo/ide/drive/cd_0", test_ide_drive_cd_0);
> +        qtest_add_func("hd-geo/ide/device/mbr/blank", test_ide_device_mbr_blank);
> +        qtest_add_func("hd-geo/ide/device/mbr/lba", test_ide_device_mbr_lba);
> +        qtest_add_func("hd-geo/ide/device/mbr/chs", test_ide_device_mbr_chs);
> +        qtest_add_func("hd-geo/ide/device/user/chs", test_ide_device_user_chs);
> +        qtest_add_func("hd-geo/ide/device/user/chst", test_ide_device_user_chst);

Just wondering, could a qtest_add_machine_func() method be helpful?
Maybe not if we want to check for multiple machines?

Re: [PATCH 4/8] tests/qtest/hd-geo-test: Check for availability of "pc" machine before using it

[Thomas Huth]

On 06/09/2024 09.50, Philippe Mathieu-Daudé wrote:
> On 5/9/24 21:14, Thomas Huth wrote:
>> In case QEMU has been configured with "--without-default-devices", the
>> "pc" machine type might be missing in the binary. We should check for
>> its availability before using it.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   tests/qtest/hd-geo-test.c | 71 +++++++++++++++++++++------------------
>>   1 file changed, 38 insertions(+), 33 deletions(-)
>>
>> diff --git a/tests/qtest/hd-geo-test.c b/tests/qtest/hd-geo-test.c
>> index d08bffad91..85eb8d7668 100644
>> --- a/tests/qtest/hd-geo-test.c
>> +++ b/tests/qtest/hd-geo-test.c
>> @@ -1074,17 +1074,26 @@ int main(int argc, char **argv)
>>           }
>>       }
>> -    qtest_add_func("hd-geo/ide/none", test_ide_none);
>> -    qtest_add_func("hd-geo/ide/drive/mbr/blank", test_ide_drive_mbr_blank);
>> -    qtest_add_func("hd-geo/ide/drive/mbr/lba", test_ide_drive_mbr_lba);
>> -    qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs);
>> -    qtest_add_func("hd-geo/ide/drive/cd_0", test_ide_drive_cd_0);
>> -    qtest_add_func("hd-geo/ide/device/mbr/blank", 
>> test_ide_device_mbr_blank);
>> -    qtest_add_func("hd-geo/ide/device/mbr/lba", test_ide_device_mbr_lba);
>> -    qtest_add_func("hd-geo/ide/device/mbr/chs", test_ide_device_mbr_chs);
>> -    qtest_add_func("hd-geo/ide/device/user/chs", test_ide_device_user_chs);
>> -    qtest_add_func("hd-geo/ide/device/user/chst", 
>> test_ide_device_user_chst);
>> -    if (have_qemu_img()) {
>> +    if (qtest_has_machine("pc")) {
>> +        qtest_add_func("hd-geo/ide/none", test_ide_none);
>> +        qtest_add_func("hd-geo/ide/drive/mbr/blank", 
>> test_ide_drive_mbr_blank);
>> +        qtest_add_func("hd-geo/ide/drive/mbr/lba", test_ide_drive_mbr_lba);
>> +        qtest_add_func("hd-geo/ide/drive/mbr/chs", test_ide_drive_mbr_chs);
>> +        qtest_add_func("hd-geo/ide/drive/cd_0", test_ide_drive_cd_0);
>> +        qtest_add_func("hd-geo/ide/device/mbr/blank", 
>> test_ide_device_mbr_blank);
>> +        qtest_add_func("hd-geo/ide/device/mbr/lba", 
>> test_ide_device_mbr_lba);
>> +        qtest_add_func("hd-geo/ide/device/mbr/chs", 
>> test_ide_device_mbr_chs);
>> +        qtest_add_func("hd-geo/ide/device/user/chs", 
>> test_ide_device_user_chs);
>> +        qtest_add_func("hd-geo/ide/device/user/chst", 
>> test_ide_device_user_chst);
> 
> Just wondering, could a qtest_add_machine_func() method be helpful?
> Maybe not if we want to check for multiple machines?

For adding multiple tests, I think a check at the beginning of the block is 
better than checking it over and over again.

  Thomas

Re: [PATCH 3/8] tests/qtest/boot-order-test: Make the machine name mandatory in this test

[Philippe Mathieu-Daudé]

Hi Thomas,

On 5/9/24 21:14, Thomas Huth wrote:
> Let's make sure that we always pass a machine name to the test_boot_orders()
> function, so we can check whether the machine is available in the binary
> and skip the test in case it is not included in the build.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   tests/qtest/boot-order-test.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/qtest/boot-order-test.c b/tests/qtest/boot-order-test.c
> index 8f2b6ef05a..c67b8cfe16 100644
> --- a/tests/qtest/boot-order-test.c
> +++ b/tests/qtest/boot-order-test.c
> @@ -31,7 +31,7 @@ static void test_a_boot_order(const char *machine,
>       uint64_t actual;
>       QTestState *qts;
>   
> -    if (machine && !qtest_has_machine(machine)) {
> +    if (!qtest_has_machine(machine)) {

Should we defer the NULL check to qtest_has_machine_with_env()?
It uses g_str_equal() which is described as:

   Note that this function is primarily meant as a hash table
   comparison function. For a general-purpose, NULL-safe string
   comparison function, see g_strcmp0().

Better switch to g_strcmp0() in qtest_has_machine_with_env()?

>           g_test_skip("Machine is not available");
>           return;
>       }
> @@ -107,7 +107,7 @@ static const boot_order_test test_cases_pc[] = {
>   
>   static void test_pc_boot_order(void)
>   {
> -    test_boot_orders(NULL, read_boot_order_pc, test_cases_pc);
> +    test_boot_orders("pc", read_boot_order_pc, test_cases_pc);
>   }
>   
>   static uint64_t read_boot_order_pmac(QTestState *qts)

Re: [PATCH 3/8] tests/qtest/boot-order-test: Make the machine name mandatory in this test

[Thomas Huth]

On 06/09/2024 09.59, Philippe Mathieu-Daudé wrote:
> Hi Thomas,
> 
> On 5/9/24 21:14, Thomas Huth wrote:
>> Let's make sure that we always pass a machine name to the test_boot_orders()
>> function, so we can check whether the machine is available in the binary
>> and skip the test in case it is not included in the build.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   tests/qtest/boot-order-test.c | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/tests/qtest/boot-order-test.c b/tests/qtest/boot-order-test.c
>> index 8f2b6ef05a..c67b8cfe16 100644
>> --- a/tests/qtest/boot-order-test.c
>> +++ b/tests/qtest/boot-order-test.c
>> @@ -31,7 +31,7 @@ static void test_a_boot_order(const char *machine,
>>       uint64_t actual;
>>       QTestState *qts;
>> -    if (machine && !qtest_has_machine(machine)) {
>> +    if (!qtest_has_machine(machine)) {
> 
> Should we defer the NULL check to qtest_has_machine_with_env()?
> It uses g_str_equal() which is described as:
> 
>    Note that this function is primarily meant as a hash table
>    comparison function. For a general-purpose, NULL-safe string
>    comparison function, see g_strcmp0().
> 
> Better switch to g_strcmp0() in qtest_has_machine_with_env()?

What would be the intended meaning of the function when it is called with 
"NULL" ? Use the default machine? Skip the test? ... I think it is rather an 
error to call it with NULL, so it's OK if the test crashes here since it 
should never happen?

  Thomas

Re: [PATCH 3/8] tests/qtest/boot-order-test: Make the machine name mandatory in this test

[Philippe Mathieu-Daudé]

On 6/9/24 10:04, Thomas Huth wrote:
> On 06/09/2024 09.59, Philippe Mathieu-Daudé wrote:
>> Hi Thomas,
>>
>> On 5/9/24 21:14, Thomas Huth wrote:
>>> Let's make sure that we always pass a machine name to the 
>>> test_boot_orders()
>>> function, so we can check whether the machine is available in the binary
>>> and skip the test in case it is not included in the build.
>>>
>>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>>> ---
>>>   tests/qtest/boot-order-test.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/tests/qtest/boot-order-test.c 
>>> b/tests/qtest/boot-order-test.c
>>> index 8f2b6ef05a..c67b8cfe16 100644
>>> --- a/tests/qtest/boot-order-test.c
>>> +++ b/tests/qtest/boot-order-test.c
>>> @@ -31,7 +31,7 @@ static void test_a_boot_order(const char *machine,
>>>       uint64_t actual;
>>>       QTestState *qts;
>>> -    if (machine && !qtest_has_machine(machine)) {
>>> +    if (!qtest_has_machine(machine)) {
>>
>> Should we defer the NULL check to qtest_has_machine_with_env()?
>> It uses g_str_equal() which is described as:
>>
>>    Note that this function is primarily meant as a hash table
>>    comparison function. For a general-purpose, NULL-safe string
>>    comparison function, see g_strcmp0().
>>
>> Better switch to g_strcmp0() in qtest_has_machine_with_env()?
> 
> What would be the intended meaning of the function when it is called 
> with "NULL" ? Use the default machine? Skip the test? ... I think it is 
> rather an error to call it with NULL, so it's OK if the test crashes 
> here since it should never happen?

Right.

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 7/8] Revert "target/riscv: Restrict semihosting to TCG"

[Thomas Huth]

On 05/09/2024 21.53, Peter Maydell wrote:
> On Thu, 5 Sept 2024 at 20:16, Thomas Huth <thuth@redhat.com> wrote:
>>
>> This reverts commit 10425887ba54241be1ce97f8935fc320332b531c.
>>
>> Using "imply" instead of "select" is causing a build failure:
>>
>>   /usr/bin/ld: libqemu-riscv32-softmmu.a.p/target_riscv_cpu_helper.c.o: in function `riscv_cpu_do_interrupt':
>>   .../qemu/target/riscv/cpu_helper.c:1678:(.text+0x2214): undefined reference to `do_common_semihosting'
>>
>> Thus revert to fix the build.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   target/riscv/Kconfig | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/riscv/Kconfig b/target/riscv/Kconfig
>> index c332616d36..5f30df22f2 100644
>> --- a/target/riscv/Kconfig
>> +++ b/target/riscv/Kconfig
>> @@ -1,9 +1,9 @@
>>   config RISCV32
>>       bool
>> -    imply ARM_COMPATIBLE_SEMIHOSTING if TCG
>> +    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
>>       select DEVICE_TREE # needed by boot.c
>>
>>   config RISCV64
>>       bool
>> -    imply ARM_COMPATIBLE_SEMIHOSTING if TCG
>> +    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
>>       select DEVICE_TREE # needed by boot.c
> 
> This will break the intended "we don't need semihosting if
> this is a KVM-only compile", though. Can we fix the
> build problem use see with
>   "select ARM_COMPATIBLE_SEMIHOSTING if TCG"

I haven't tried, but I assume that this will produce the same linking issues 
when TCG is disabled. Maybe best if we fix it in the code, see the patch 
suggested here:

  https://lore.kernel.org/qemu-devel/20240906080928.710051-1-thuth@redhat.com/

  Thomas

Re: [PATCH 7/8] Revert "target/riscv: Restrict semihosting to TCG"

[Philippe Mathieu-Daudé]

On 5/9/24 21:53, Peter Maydell wrote:
> On Thu, 5 Sept 2024 at 20:16, Thomas Huth <thuth@redhat.com> wrote:
>>
>> This reverts commit 10425887ba54241be1ce97f8935fc320332b531c.
>>
>> Using "imply" instead of "select" is causing a build failure:

(please mention ./configure arguments besides --without-default-devices)

>>
>>   /usr/bin/ld: libqemu-riscv32-softmmu.a.p/target_riscv_cpu_helper.c.o: in function `riscv_cpu_do_interrupt':
>>   .../qemu/target/riscv/cpu_helper.c:1678:(.text+0x2214): undefined reference to `do_common_semihosting'
>>
>> Thus revert to fix the build.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   target/riscv/Kconfig | 4 ++--
>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/target/riscv/Kconfig b/target/riscv/Kconfig
>> index c332616d36..5f30df22f2 100644
>> --- a/target/riscv/Kconfig
>> +++ b/target/riscv/Kconfig
>> @@ -1,9 +1,9 @@
>>   config RISCV32
>>       bool
>> -    imply ARM_COMPATIBLE_SEMIHOSTING if TCG
>> +    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
>>       select DEVICE_TREE # needed by boot.c
>>
>>   config RISCV64
>>       bool
>> -    imply ARM_COMPATIBLE_SEMIHOSTING if TCG
>> +    select ARM_COMPATIBLE_SEMIHOSTING # for do_common_semihosting()
>>       select DEVICE_TREE # needed by boot.c
> 
> This will break the intended "we don't need semihosting if
> this is a KVM-only compile", though. Can we fix the
> build problem use see with
>   "select ARM_COMPATIBLE_SEMIHOSTING if TCG"

We had this discussion with Paolo in
https://lore.kernel.org/qemu-devel/CABgObfbvjG9bBgCwM-kL+YhjhMw1qLnQdQToCEkKW+V3trskoA@mail.gmail.com/
Not sure this is as easy as it looks...

I feel the riscv part could be fixed by a respin of:
https://lore.kernel.org/qemu-devel/20230711121453.59138-1-philmd@linaro.org/
where semihosting is restricted to TCG and isn't an issue
anymore for other accelerators such KVM.

Let me have a try.