qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/4] tests/qtest: Introduce qtest_get_base_arch() and qtest_get_arch_bits()
@ 2023-10-10  7:49 Philippe Mathieu-Daudé
  2023-10-10  7:49 ` [PATCH 1/4] tests/libqtest: Introduce qtest_get_arch_bits() Philippe Mathieu-Daudé
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-10  7:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Paolo Bonzini, qemu-ppc, Alex Bennée, qemu-arm,
	qemu-block, Philippe Mathieu-Daudé

When unifying some 32/64 architectures as a single binary
(for example i386 & x86_64 -> x86), some qtests fail because
we lose some information (the arch bits).

This series introduce the tuple qtest_get_base_arch() /
qtest_get_arch_bits() to allow the tests to keep passing.

Eventually we should remove qtest_get_arch() entirely
and rename qtest_get_base_arch() -> qtest_get_arch().

So far these changes are sufficient to keep me progressing,
so posting up to this point.

Philippe Mathieu-Daudé (4):
  tests/libqtest: Introduce qtest_get_arch_bits()
  tests/qtest: Use qtest_get_arch_bits()
  tests/libqtest: Introduce qtest_get_base_arch()
  tests/qtest: Use qtest_get_base_arch()

 tests/qtest/libqtest.h            | 15 ++++++++++
 tests/qtest/ahci-test.c           |  5 ++--
 tests/qtest/am53c974-test.c       |  4 +--
 tests/qtest/arm-cpu-features.c    | 49 +++++++++++++++----------------
 tests/qtest/bios-tables-test.c    | 22 +++++++-------
 tests/qtest/boot-sector.c         |  6 ++--
 tests/qtest/device-plug-test.c    |  6 ++--
 tests/qtest/drive_del-test.c      | 16 ++++------
 tests/qtest/erst-test.c           |  4 +--
 tests/qtest/fuzz-e1000e-test.c    |  4 +--
 tests/qtest/ivshmem-test.c        |  6 ++--
 tests/qtest/libqos/qos_external.c |  6 ++--
 tests/qtest/libqtest.c            | 49 +++++++++++++++++++++++++++++++
 tests/qtest/lpc-ich9-test.c       |  4 +--
 tests/qtest/m48t59-test.c         |  6 ++--
 tests/qtest/pxe-test.c            |  6 ++--
 tests/qtest/qos-test.c            |  6 ++--
 tests/qtest/readconfig-test.c     |  6 +---
 tests/qtest/rtas-test.c           |  4 +--
 tests/qtest/ufs-test.c            |  4 +--
 tests/qtest/usb-hcd-uhci-test.c   |  6 ++--
 tests/qtest/virtio-net-test.c     |  3 +-
 tests/qtest/virtio-rng-test.c     |  4 +--
 23 files changed, 138 insertions(+), 103 deletions(-)

-- 
2.41.0



^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/4] tests/libqtest: Introduce qtest_get_arch_bits()
  2023-10-10  7:49 [PATCH 0/4] tests/qtest: Introduce qtest_get_base_arch() and qtest_get_arch_bits() Philippe Mathieu-Daudé
@ 2023-10-10  7:49 ` Philippe Mathieu-Daudé
  2023-10-10  9:46   ` Thomas Huth
  2023-10-10  7:49 ` [PATCH 2/4] tests/qtest: Use qtest_get_arch_bits() Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-10  7:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Paolo Bonzini, qemu-ppc, Alex Bennée, qemu-arm,
	qemu-block, Philippe Mathieu-Daudé, Laurent Vivier

Add a method to return the architecture bits (currently 8/32/64).

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/libqtest.h |  8 ++++++++
 tests/qtest/libqtest.c | 21 +++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index e53e350e3a..1e1b42241d 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -654,6 +654,14 @@ bool qtest_big_endian(QTestState *s);
  */
 const char *qtest_get_arch(void);
 
+/**
+ * qtest_get_arch_bits:
+ *
+ * Returns: The architecture bits (a.k.a. word size) for the QEMU executable
+ * under test.
+ */
+unsigned qtest_get_arch_bits(void);
+
 /**
  * qtest_has_accel:
  * @accel_name: Accelerator name to check for.
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index b1eba71ffe..a643a6309c 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -925,6 +925,27 @@ const char *qtest_get_arch(void)
     return end + 1;
 }
 
+unsigned qtest_get_arch_bits(void)
+{
+    static const char *const arch64[] = {
+        "aarch64", "hppa", "x86_64", "loongarch64", "mips64",
+        "mips64el", "ppc64", "riscv64", "s390x", "sparc64",
+    };
+    const char *arch = qtest_get_arch();
+
+    if (!strcmp(arch, "avr")) {
+        return 8;
+    }
+
+    for (unsigned i = 0; i < ARRAY_SIZE(arch64); i++) {
+        if (!strcmp(arch, arch64[i])) {
+            return 64;
+        }
+    }
+
+    return 32;
+}
+
 bool qtest_has_accel(const char *accel_name)
 {
     if (g_str_equal(accel_name, "tcg")) {
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/4] tests/qtest: Use qtest_get_arch_bits()
  2023-10-10  7:49 [PATCH 0/4] tests/qtest: Introduce qtest_get_base_arch() and qtest_get_arch_bits() Philippe Mathieu-Daudé
  2023-10-10  7:49 ` [PATCH 1/4] tests/libqtest: Introduce qtest_get_arch_bits() Philippe Mathieu-Daudé
@ 2023-10-10  7:49 ` Philippe Mathieu-Daudé
  2023-10-10  9:49   ` Thomas Huth
  2023-10-10  7:49 ` [PATCH 3/4] tests/libqtest: Introduce qtest_get_base_arch() Philippe Mathieu-Daudé
  2023-10-10  7:49 ` [PATCH 4/4] tests/qtest: Use qtest_get_base_arch() Philippe Mathieu-Daudé
  3 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-10  7:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Paolo Bonzini, qemu-ppc, Alex Bennée, qemu-arm,
	qemu-block, Philippe Mathieu-Daudé, Peter Maydell,
	Laurent Vivier, Michael S. Tsirkin, Igor Mammedov, Ani Sinha

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/arm-cpu-features.c | 49 ++++++++++++++++------------------
 tests/qtest/bios-tables-test.c | 16 +++++------
 2 files changed, 30 insertions(+), 35 deletions(-)

diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index a8a4c668ad..b6c1b430c1 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -462,7 +462,7 @@ static void test_query_cpu_model_expansion(const void *data)
     assert_has_not_feature(qts, "max", "kvm-no-adjvtime");
     assert_has_not_feature(qts, "max", "kvm-steal-time");
 
-    if (g_str_equal(qtest_get_arch(), "aarch64")) {
+    if (qtest_get_arch_bits() == 64) {
         assert_has_feature_enabled(qts, "max", "aarch64");
         assert_has_feature_enabled(qts, "max", "sve");
         assert_has_feature_enabled(qts, "max", "sve128");
@@ -507,7 +507,7 @@ static void test_query_cpu_model_expansion_kvm(const void *data)
     assert_set_feature(qts, "host", "kvm-no-adjvtime", true);
     assert_set_feature(qts, "host", "kvm-no-adjvtime", false);
 
-    if (g_str_equal(qtest_get_arch(), "aarch64")) {
+    if (qtest_get_arch_bits() == 64) {
         bool kvm_supports_steal_time;
         bool kvm_supports_sve;
         char max_name[8], name[8];
@@ -636,34 +636,31 @@ int main(int argc, char **argv)
                             NULL, test_query_cpu_model_expansion);
     }
 
-    if (!g_str_equal(qtest_get_arch(), "aarch64")) {
-        goto out;
-    }
-
-    /*
-     * For now we only run KVM specific tests with AArch64 QEMU in
-     * order avoid attempting to run an AArch32 QEMU with KVM on
-     * AArch64 hosts. That won't work and isn't easy to detect.
-     */
-    if (qtest_has_accel("kvm")) {
+    if (qtest_get_arch_bits() == 64) {
         /*
-         * This tests target the 'host' CPU type, so register it only if
-         * KVM is available.
+         * For now we only run KVM specific tests with AArch64 QEMU in
+         * order avoid attempting to run an AArch32 QEMU with KVM on
+         * AArch64 hosts. That won't work and isn't easy to detect.
          */
-        qtest_add_data_func("/arm/kvm/query-cpu-model-expansion",
-                            NULL, test_query_cpu_model_expansion_kvm);
+        if (qtest_has_accel("kvm")) {
+            /*
+             * This tests target the 'host' CPU type, so register it only if
+             * KVM is available.
+             */
+            qtest_add_data_func("/arm/kvm/query-cpu-model-expansion",
+                                NULL, test_query_cpu_model_expansion_kvm);
 
-        qtest_add_data_func("/arm/kvm/query-cpu-model-expansion/sve-off",
-                            NULL, sve_tests_sve_off_kvm);
+            qtest_add_data_func("/arm/kvm/query-cpu-model-expansion/sve-off",
+                                NULL, sve_tests_sve_off_kvm);
+        }
+
+        if (qtest_has_accel("tcg")) {
+            qtest_add_data_func("/arm/max/query-cpu-model-expansion/sve-max-vq-8",
+                                NULL, sve_tests_sve_max_vq_8);
+            qtest_add_data_func("/arm/max/query-cpu-model-expansion/sve-off",
+                                NULL, sve_tests_sve_off);
+        }
     }
 
-    if (qtest_has_accel("tcg")) {
-        qtest_add_data_func("/arm/max/query-cpu-model-expansion/sve-max-vq-8",
-                            NULL, sve_tests_sve_max_vq_8);
-        qtest_add_data_func("/arm/max/query-cpu-model-expansion/sve-off",
-                            NULL, sve_tests_sve_off);
-    }
-
-out:
     return g_test_run();
 }
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index f8e03dfd46..7e708d78b3 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -2089,7 +2089,7 @@ int main(int argc, char *argv[])
             qtest_add_func("acpi/piix4/nohpet", test_acpi_piix4_tcg_nohpet);
 
             /* i386 does not support memory hotplug */
-            if (strcmp(arch, "i386")) {
+            if (qtest_get_arch_bits() == 64) {
                 qtest_add_func("acpi/piix4/memhp", test_acpi_piix4_tcg_memhp);
                 qtest_add_func("acpi/piix4/dimmpxm",
                                test_acpi_piix4_tcg_dimm_pxm);
@@ -2127,7 +2127,7 @@ int main(int argc, char *argv[])
                            test_acpi_q35_tcg_acpi_hmat_noinitiator);
 
             /* i386 does not support memory hotplug */
-            if (strcmp(arch, "i386")) {
+            if (qtest_get_arch_bits() == 64) {
                 qtest_add_func("acpi/q35/memhp", test_acpi_q35_tcg_memhp);
                 qtest_add_func("acpi/q35/dimmpxm", test_acpi_q35_tcg_dimm_pxm);
                 qtest_add_func("acpi/q35/acpihmat",
@@ -2164,15 +2164,13 @@ int main(int argc, char *argv[])
                            test_acpi_microvm_ioapic2_tcg);
             qtest_add_func("acpi/microvm/oem-fields",
                            test_acpi_microvm_oem_fields);
-            if (has_tcg) {
-                if (strcmp(arch, "x86_64") == 0) {
-                    qtest_add_func("acpi/microvm/pcie",
-                                   test_acpi_microvm_pcie_tcg);
+            if (has_tcg && qtest_get_arch_bits() == 64) {
+                qtest_add_func("acpi/microvm/pcie",
+                               test_acpi_microvm_pcie_tcg);
 #ifdef CONFIG_POSIX
-                    qtest_add_func("acpi/microvm/acpierst",
-                                   test_acpi_microvm_acpi_erst);
+                qtest_add_func("acpi/microvm/acpierst",
+                               test_acpi_microvm_acpi_erst);
 #endif
-                }
             }
         }
     } else if (strcmp(arch, "aarch64") == 0) {
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/4] tests/libqtest: Introduce qtest_get_base_arch()
  2023-10-10  7:49 [PATCH 0/4] tests/qtest: Introduce qtest_get_base_arch() and qtest_get_arch_bits() Philippe Mathieu-Daudé
  2023-10-10  7:49 ` [PATCH 1/4] tests/libqtest: Introduce qtest_get_arch_bits() Philippe Mathieu-Daudé
  2023-10-10  7:49 ` [PATCH 2/4] tests/qtest: Use qtest_get_arch_bits() Philippe Mathieu-Daudé
@ 2023-10-10  7:49 ` Philippe Mathieu-Daudé
  2023-10-10  8:42   ` Philippe Mathieu-Daudé
  2023-10-10  7:49 ` [PATCH 4/4] tests/qtest: Use qtest_get_base_arch() Philippe Mathieu-Daudé
  3 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-10  7:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Paolo Bonzini, qemu-ppc, Alex Bennée, qemu-arm,
	qemu-block, Philippe Mathieu-Daudé, Laurent Vivier

While qtest_get_arch() returns the target architecture name,
such "i386" or "x86_64", qtest_get_base_arch() return the
"base" (or real underlying) architecture, in this example
that is "x86".

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/libqtest.h |  7 +++++++
 tests/qtest/libqtest.c | 28 ++++++++++++++++++++++++++++
 2 files changed, 35 insertions(+)

diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index 1e1b42241d..54071e74ec 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -654,6 +654,13 @@ bool qtest_big_endian(QTestState *s);
  */
 const char *qtest_get_arch(void);
 
+/**
+ * qtest_get_base_arch:
+ *
+ * Returns: The base architecture for the QEMU executable under test.
+ */
+const char *qtest_get_base_arch(void);
+
 /**
  * qtest_get_arch_bits:
  *
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index a643a6309c..51cc92af21 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -925,6 +925,34 @@ const char *qtest_get_arch(void)
     return end + 1;
 }
 
+const char *qtest_get_base_arch(void)
+{
+    static const struct {
+        const char *const arch;
+        const char *const base;
+    } basearch[] = {
+        { "aarch64", "arm" },
+        { "i386", "x86" },
+        { "loongarch64", "loongarch" },
+        { "mipsel", "mips" },
+        { "mips64", "mips" },
+        { "mips64el", "mips" },
+        { "ppc64", "ppc" },
+        { "riscv32", "riscv" },
+        { "riscv64", "riscv" },
+        { "sparc64", "sparc" },
+        { "x86_64", "x86" },
+    };
+    const char *arch = qtest_get_arch();
+
+    for (unsigned i = 0; i < ARRAY_SIZE(basearch); i++) {
+        if (!strcmp(arch, basearch[i].arch)) {
+            return basearch[i].base;
+        }
+    }
+    g_assert_not_reached();
+}
+
 unsigned qtest_get_arch_bits(void)
 {
     static const char *const arch64[] = {
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/4] tests/qtest: Use qtest_get_base_arch()
  2023-10-10  7:49 [PATCH 0/4] tests/qtest: Introduce qtest_get_base_arch() and qtest_get_arch_bits() Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2023-10-10  7:49 ` [PATCH 3/4] tests/libqtest: Introduce qtest_get_base_arch() Philippe Mathieu-Daudé
@ 2023-10-10  7:49 ` Philippe Mathieu-Daudé
  2023-10-10  9:38   ` Akihiko Odaki
  2023-10-10  9:58   ` Thomas Huth
  3 siblings, 2 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-10  7:49 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Paolo Bonzini, qemu-ppc, Alex Bennée, qemu-arm,
	qemu-block, Philippe Mathieu-Daudé, John Snow,
	Laurent Vivier, Fam Zheng, Michael S. Tsirkin, Igor Mammedov,
	Ani Sinha, Alexander Bulekov, Bandan Das, Stefan Hajnoczi,
	Darren Kenny, Qiuhao Li, Dmitry Fleytman, Akihiko Odaki,
	Nicholas Piggin, Daniel Henrique Barboza, Cédric Le Goater,
	David Gibson, Harsh Prateek Bora, Jeuk Kim, Gerd Hoffmann,
	Jason Wang, Amit Shah

Additionally use qtest_get_arch_bits() when relevant.

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
---
 tests/qtest/ahci-test.c           |  5 ++---
 tests/qtest/am53c974-test.c       |  4 +---
 tests/qtest/bios-tables-test.c    |  6 +++---
 tests/qtest/boot-sector.c         |  6 +++---
 tests/qtest/device-plug-test.c    |  6 ++----
 tests/qtest/drive_del-test.c      | 16 ++++++----------
 tests/qtest/erst-test.c           |  4 +---
 tests/qtest/fuzz-e1000e-test.c    |  4 +---
 tests/qtest/ivshmem-test.c        |  6 +++---
 tests/qtest/libqos/qos_external.c |  6 ++++--
 tests/qtest/lpc-ich9-test.c       |  4 +---
 tests/qtest/m48t59-test.c         |  6 ++----
 tests/qtest/pxe-test.c            |  6 +++---
 tests/qtest/qos-test.c            |  6 ++++--
 tests/qtest/readconfig-test.c     |  6 +-----
 tests/qtest/rtas-test.c           |  4 +---
 tests/qtest/ufs-test.c            |  4 +---
 tests/qtest/usb-hcd-uhci-test.c   |  6 +++---
 tests/qtest/virtio-net-test.c     |  3 +--
 tests/qtest/virtio-rng-test.c     |  4 +---
 20 files changed, 44 insertions(+), 68 deletions(-)

diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
index eea8b5f77b..93d1e14896 100644
--- a/tests/qtest/ahci-test.c
+++ b/tests/qtest/ahci-test.c
@@ -1835,7 +1835,7 @@ static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
 
 int main(int argc, char **argv)
 {
-    const char *arch, *base;
+    const char *base;
     int ret;
     int fd;
     int c;
@@ -1867,8 +1867,7 @@ int main(int argc, char **argv)
     }
 
     /* Check architecture */
-    arch = qtest_get_arch();
-    if (strcmp(arch, "i386") && strcmp(arch, "x86_64")) {
+    if (strcmp(qtest_get_base_arch(), "x86")) {
         g_test_message("Skipping test for non-x86");
         return 0;
     }
diff --git a/tests/qtest/am53c974-test.c b/tests/qtest/am53c974-test.c
index ed3ac7db20..dc41182a38 100644
--- a/tests/qtest/am53c974-test.c
+++ b/tests/qtest/am53c974-test.c
@@ -253,11 +253,9 @@ static void test_reset_before_transfer_ok(void)
 
 int main(int argc, char **argv)
 {
-    const char *arch = qtest_get_arch();
-
     g_test_init(&argc, &argv, NULL);
 
-    if (strcmp(arch, "i386") == 0) {
+    if (!strcmp(qtest_get_base_arch(), "x86") && qtest_get_arch_bits() == 32) {
         qtest_add_func("am53c974/test_cmdfifo_underflow_ok",
                        test_cmdfifo_underflow_ok);
         qtest_add_func("am53c974/test_cmdfifo_underflow2_ok",
diff --git a/tests/qtest/bios-tables-test.c b/tests/qtest/bios-tables-test.c
index 7e708d78b3..573e7dfcd9 100644
--- a/tests/qtest/bios-tables-test.c
+++ b/tests/qtest/bios-tables-test.c
@@ -2044,7 +2044,7 @@ static void test_acpi_virt_oem_fields(void)
 
 int main(int argc, char *argv[])
 {
-    const char *arch = qtest_get_arch();
+    const char *arch = qtest_get_base_arch();
     bool has_kvm, has_tcg;
     char *v_env = getenv("V");
     int ret;
@@ -2063,7 +2063,7 @@ int main(int argc, char *argv[])
         return 0;
     }
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(arch, "x86") == 0) {
         ret = boot_sector_init(disk);
         if (ret) {
             return ret;
@@ -2173,7 +2173,7 @@ int main(int argc, char *argv[])
 #endif
             }
         }
-    } else if (strcmp(arch, "aarch64") == 0) {
+    } else if (strcmp(arch, "arm") == 0 && qtest_get_arch_bits() == 64) {
         if (has_tcg && qtest_has_device("virtio-blk-pci")) {
             qtest_add_func("acpi/virt", test_acpi_virt_tcg);
             qtest_add_func("acpi/virt/acpihmatvirt",
diff --git a/tests/qtest/boot-sector.c b/tests/qtest/boot-sector.c
index 679ee17e2a..f13e9a12ff 100644
--- a/tests/qtest/boot-sector.c
+++ b/tests/qtest/boot-sector.c
@@ -90,7 +90,7 @@ int boot_sector_init(char *fname)
     int fd, ret;
     size_t len;
     char *boot_code;
-    const char *arch = qtest_get_arch();
+    const char *arch = qtest_get_base_arch();
 
     fd = mkstemp(fname);
     if (fd < 0) {
@@ -98,12 +98,12 @@ int boot_sector_init(char *fname)
         return 1;
     }
 
-    if (g_str_equal(arch, "i386") || g_str_equal(arch, "x86_64")) {
+    if (g_str_equal(arch, "x86")) {
         /* Q35 requires a minimum 0x7e000 bytes disk (bug or feature?) */
         len = MAX(0x7e000, sizeof(x86_boot_sector));
         boot_code = g_malloc0(len);
         memcpy(boot_code, x86_boot_sector, sizeof(x86_boot_sector));
-    } else if (g_str_equal(arch, "ppc64")) {
+    } else if (g_str_equal(arch, "ppc") && qtest_get_arch_bits() == 64) {
         /* For Open Firmware based system, use a Forth script */
         boot_code = g_strdup_printf("\\ Bootscript\n%x %x c! %x %x c!\n",
                                     LOW(SIGNATURE), SIGNATURE_ADDR,
diff --git a/tests/qtest/device-plug-test.c b/tests/qtest/device-plug-test.c
index c6f33153eb..59153e3493 100644
--- a/tests/qtest/device-plug-test.c
+++ b/tests/qtest/device-plug-test.c
@@ -65,7 +65,6 @@ static void process_device_remove(QTestState *qtest, const char *id)
 static void test_pci_unplug_request(void)
 {
     QTestState *qtest;
-    const char *arch = qtest_get_arch();
     const char *machine_addition = "";
 
     if (!qtest_has_device("virtio-mouse-pci")) {
@@ -73,7 +72,7 @@ static void test_pci_unplug_request(void)
         return;
     }
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         machine_addition = "-machine pc";
     }
 
@@ -107,7 +106,6 @@ static void test_q35_pci_unplug_request(void)
 static void test_pci_unplug_json_request(void)
 {
     QTestState *qtest;
-    const char *arch = qtest_get_arch();
     const char *machine_addition = "";
 
     if (!qtest_has_device("virtio-mouse-pci")) {
@@ -115,7 +113,7 @@ static void test_pci_unplug_json_request(void)
         return;
     }
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         machine_addition = "-machine pc";
     }
 
diff --git a/tests/qtest/drive_del-test.c b/tests/qtest/drive_del-test.c
index 8a6f3ac963..ccc42acce9 100644
--- a/tests/qtest/drive_del-test.c
+++ b/tests/qtest/drive_del-test.c
@@ -125,9 +125,9 @@ static void drive_del(QTestState *qts)
  */
 static const char *qvirtio_get_dev_type(void)
 {
-    const char *arch = qtest_get_arch();
+    const char *arch = qtest_get_base_arch();
 
-    if (g_str_equal(arch, "arm") || g_str_equal(arch, "aarch64")) {
+    if (g_str_equal(arch, "arm")) {
         return "device";  /* for virtio-mmio */
     } else if (g_str_equal(arch, "s390x")) {
         return "ccw";
@@ -249,7 +249,6 @@ static void test_drive_del_device_del(void)
 static void test_cli_device_del(void)
 {
     QTestState *qts;
-    const char *arch = qtest_get_arch();
     const char *machine_addition = "";
 
     if (!has_device_builtin("virtio-blk")) {
@@ -257,7 +256,7 @@ static void test_cli_device_del(void)
         return;
     }
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         machine_addition = "-machine pc";
     }
 
@@ -323,7 +322,6 @@ static void test_empty_device_del(void)
 static void test_device_add_and_del(void)
 {
     QTestState *qts;
-    const char *arch = qtest_get_arch();
     const char *machine_addition = "";
 
     if (!has_device_builtin("virtio-blk")) {
@@ -331,7 +329,7 @@ static void test_device_add_and_del(void)
         return;
     }
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         machine_addition = "-machine pc";
     }
 
@@ -394,7 +392,6 @@ static void test_device_add_and_del_q35(void)
 static void test_drive_add_device_add_and_del(void)
 {
     QTestState *qts;
-    const char *arch = qtest_get_arch();
     const char *machine_addition = "";
 
     if (!has_device_builtin("virtio-blk")) {
@@ -402,7 +399,7 @@ static void test_drive_add_device_add_and_del(void)
         return;
     }
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         machine_addition = "-machine pc";
     }
 
@@ -447,7 +444,6 @@ static void test_drive_add_device_add_and_del_q35(void)
 static void test_blockdev_add_device_add_and_del(void)
 {
     QTestState *qts;
-    const char *arch = qtest_get_arch();
     const char *machine_addition = "";
 
     if (!has_device_builtin("virtio-blk")) {
@@ -455,7 +451,7 @@ static void test_blockdev_add_device_add_and_del(void)
         return;
     }
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         machine_addition = "-machine pc";
     }
 
diff --git a/tests/qtest/erst-test.c b/tests/qtest/erst-test.c
index c45bee7f05..8d3665b82c 100644
--- a/tests/qtest/erst-test.c
+++ b/tests/qtest/erst-test.c
@@ -95,9 +95,7 @@ static void cleanup_vm(ERSTState *s)
 
 static void setup_vm_cmd(ERSTState *s, const char *cmd)
 {
-    const char *arch = qtest_get_arch();
-
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         s->qs = qtest_pc_boot("%s", cmd);
     } else {
         g_printerr("erst-test tests are only available on x86\n");
diff --git a/tests/qtest/fuzz-e1000e-test.c b/tests/qtest/fuzz-e1000e-test.c
index 5052883fb6..a8d3596f2c 100644
--- a/tests/qtest/fuzz-e1000e-test.c
+++ b/tests/qtest/fuzz-e1000e-test.c
@@ -40,11 +40,9 @@ static void test_lp1879531_eth_get_rss_ex_dst_addr(void)
 
 int main(int argc, char **argv)
 {
-    const char *arch = qtest_get_arch();
-
     g_test_init(&argc, &argv, NULL);
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         qtest_add_func("fuzz/test_lp1879531_eth_get_rss_ex_dst_addr",
                        test_lp1879531_eth_get_rss_ex_dst_addr);
     }
diff --git a/tests/qtest/ivshmem-test.c b/tests/qtest/ivshmem-test.c
index 9bf8e78df6..32024d9243 100644
--- a/tests/qtest/ivshmem-test.c
+++ b/tests/qtest/ivshmem-test.c
@@ -397,9 +397,9 @@ static void test_ivshmem_hotplug_q35(void)
 static void test_ivshmem_hotplug(void)
 {
     QTestState *qts;
-    const char *arch = qtest_get_arch();
+    const char *arch = qtest_get_base_arch();
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(arch, "x86") == 0) {
         qts = qtest_init("-object memory-backend-ram,size=1M,id=mb1"
                          " -machine pc");
     } else {
@@ -409,7 +409,7 @@ static void test_ivshmem_hotplug(void)
     qtest_qmp_device_add(qts, "ivshmem-plain", "iv1",
                          "{'addr': %s, 'memdev': 'mb1'}",
                          stringify(PCI_SLOT_HP));
-    if (strcmp(arch, "ppc64") != 0) {
+    if (strcmp(arch, "ppc") != 0 && qtest_get_arch_bits() == 64) {
         qpci_unplug_acpi_device_test(qts, "iv1", PCI_SLOT_HP);
     }
 
diff --git a/tests/qtest/libqos/qos_external.c b/tests/qtest/libqos/qos_external.c
index c6bb8bff09..f01bb17628 100644
--- a/tests/qtest/libqos/qos_external.c
+++ b/tests/qtest/libqos/qos_external.c
@@ -31,10 +31,12 @@
 
 static void machine_apply_to_node(const char *name)
 {
-    char *machine_name = g_strconcat(qtest_get_arch(), "/", name, NULL);
+    g_autofree char *machine_name = g_strdup_printf("%s/%u/%s",
+                                                    qtest_get_base_arch(),
+                                                    qtest_get_arch_bits(),
+                                                    name);
 
     qos_graph_node_set_availability(machine_name, true);
-    g_free(machine_name);
 }
 
 void machines_apply_to_node(MachineInfoList *mach_info)
diff --git a/tests/qtest/lpc-ich9-test.c b/tests/qtest/lpc-ich9-test.c
index 8ac95b89f7..9f27591b74 100644
--- a/tests/qtest/lpc-ich9-test.c
+++ b/tests/qtest/lpc-ich9-test.c
@@ -28,11 +28,9 @@ static void test_lp1878642_pci_bus_get_irq_level_assert(void)
 
 int main(int argc, char **argv)
 {
-    const char *arch = qtest_get_arch();
-
     g_test_init(&argc, &argv, NULL);
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         qtest_add_func("ich9/test_lp1878642_pci_bus_get_irq_level_assert",
                        test_lp1878642_pci_bus_get_irq_level_assert);
     }
diff --git a/tests/qtest/m48t59-test.c b/tests/qtest/m48t59-test.c
index b9cd209165..3579effdf4 100644
--- a/tests/qtest/m48t59-test.c
+++ b/tests/qtest/m48t59-test.c
@@ -239,15 +239,13 @@ static void fuzz_registers(void)
 
 static void base_setup(void)
 {
-    const char *arch = qtest_get_arch();
-
-    if (g_str_equal(arch, "sparc")) {
+    if (g_str_equal(qtest_get_arch(), "sparc")) {
         /* Note: For sparc64, we'd need to map-in the PCI bridge memory first */
         base = 0x71200000;
         base_year = 1968;
         base_machine = "SS-5";
         use_mmio = true;
-    } else if (g_str_equal(arch, "ppc") || g_str_equal(arch, "ppc64")) {
+    } else if (g_str_equal(qtest_get_base_arch(), "ppc")) {
         base = 0xF0000000;
         base_year = 1968;
         base_machine = "ref405ep";
diff --git a/tests/qtest/pxe-test.c b/tests/qtest/pxe-test.c
index e4b48225a5..5112bceb76 100644
--- a/tests/qtest/pxe-test.c
+++ b/tests/qtest/pxe-test.c
@@ -129,7 +129,7 @@ static void test_batch(const testdef_t *tests, bool ipv6)
 int main(int argc, char *argv[])
 {
     int ret;
-    const char *arch = qtest_get_arch();
+    const char *arch = qtest_get_base_arch();
 
     g_test_init(&argc, &argv, NULL);
 
@@ -143,12 +143,12 @@ int main(int argc, char *argv[])
         return ret;
 
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(arch, "x86") == 0) {
         test_batch(x86_tests, false);
         if (g_test_slow()) {
             test_batch(x86_tests_slow, false);
         }
-    } else if (strcmp(arch, "ppc64") == 0) {
+    } else if (strcmp(arch, "ppc") == 0 && qtest_get_arch_bits() == 64) {
         test_batch(ppc64_tests, g_test_slow());
         if (g_test_slow()) {
             test_batch(ppc64_tests_slow, true);
diff --git a/tests/qtest/qos-test.c b/tests/qtest/qos-test.c
index 5da4091ec3..5271582f6a 100644
--- a/tests/qtest/qos-test.c
+++ b/tests/qtest/qos-test.c
@@ -293,8 +293,10 @@ static void walk_path(QOSGraphNode *orig_path, int len)
     path_vec[0] = g_string_free(cmd_line, false);
 
     if (path->u.test.subprocess) {
-        gchar *subprocess_path = g_strdup_printf("/%s/%s/subprocess",
-                                                 qtest_get_arch(), path_str);
+        gchar *subprocess_path = g_strdup_printf("/%s/%u/%s/subprocess",
+                                                 qtest_get_base_arch(),
+                                                 qtest_get_arch_bits(),
+                                                 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);
     } else {
diff --git a/tests/qtest/readconfig-test.c b/tests/qtest/readconfig-test.c
index 760f974e63..7837997c6f 100644
--- a/tests/qtest/readconfig-test.c
+++ b/tests/qtest/readconfig-test.c
@@ -382,13 +382,9 @@ static void test_docs_q35_virtio_serial(void)
 
 int main(int argc, char *argv[])
 {
-    const char *arch;
     g_test_init(&argc, &argv, NULL);
 
-    arch = qtest_get_arch();
-
-    if (g_str_equal(arch, "i386") ||
-        g_str_equal(arch, "x86_64")) {
+    if (g_str_equal(qtest_get_base_arch(), "x86")) {
         qtest_add_func("readconfig/x86/memdev", test_x86_memdev);
         if (qtest_has_device("ich9-usb-ehci1") &&
             qtest_has_device("ich9-usb-uhci1")) {
diff --git a/tests/qtest/rtas-test.c b/tests/qtest/rtas-test.c
index 1ba42b37d2..26c8e8e926 100644
--- a/tests/qtest/rtas-test.c
+++ b/tests/qtest/rtas-test.c
@@ -36,11 +36,9 @@ static void test_rtas_get_time_of_day_vof(void)
 
 int main(int argc, char *argv[])
 {
-    const char *arch = qtest_get_arch();
-
     g_test_init(&argc, &argv, NULL);
 
-    if (strcmp(arch, "ppc64")) {
+    if (strcmp(qtest_get_base_arch(), "ppc") && qtest_get_arch_bits() != 64) {
         g_printerr("RTAS requires qemu-system-ppc64\n");
         exit(EXIT_FAILURE);
     }
diff --git a/tests/qtest/ufs-test.c b/tests/qtest/ufs-test.c
index ed3dbca154..5a5b10b520 100644
--- a/tests/qtest/ufs-test.c
+++ b/tests/qtest/ufs-test.c
@@ -552,7 +552,6 @@ static void *ufs_blk_test_setup(GString *cmd_line, void *arg)
 
 static void ufs_register_nodes(void)
 {
-    const char *arch;
     QOSGraphEdgeOptions edge_opts = {
         .before_cmd_line = "-blockdev null-co,node-name=drv0,read-zeroes=on",
         .after_cmd_line = "-device ufs-lu,bus=ufs0,drive=drv0,lun=0",
@@ -575,8 +574,7 @@ static void ufs_register_nodes(void)
      * Check architecture
      * TODO: Enable ufs io tests for ppc64
      */
-    arch = qtest_get_arch();
-    if (!strcmp(arch, "ppc64")) {
+    if (!strcmp(qtest_get_base_arch(), "ppc") && qtest_get_arch_bits() == 64) {
         g_test_message("Skipping ufs io tests for ppc64");
         return;
     }
diff --git a/tests/qtest/usb-hcd-uhci-test.c b/tests/qtest/usb-hcd-uhci-test.c
index 4446555f08..144a5541f0 100644
--- a/tests/qtest/usb-hcd-uhci-test.c
+++ b/tests/qtest/usb-hcd-uhci-test.c
@@ -53,7 +53,7 @@ static void test_usb_storage_hotplug(void)
 
 int main(int argc, char **argv)
 {
-    const char *arch = qtest_get_arch();
+    const char *arch = qtest_get_base_arch();
     const char *cmd = "-device piix3-usb-uhci,id=uhci,addr=1d.0"
                       " -drive id=drive0,if=none,file=null-co://,"
                       "file.read-zeroes=on,format=raw"
@@ -73,9 +73,9 @@ int main(int argc, char **argv)
         qtest_add_func("/uhci/pci/hotplug/usb-storage", test_usb_storage_hotplug);
     }
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(arch, "x86") == 0) {
         qs = qtest_pc_boot("%s", cmd);
-    } else if (strcmp(arch, "ppc64") == 0) {
+    } else if (strcmp(arch, "ppc") == 0 && qtest_get_arch_bits() == 64) {
         qs = qtest_spapr_boot("%s", cmd);
     } else {
         g_printerr("usb-hcd-uhci-test tests are only "
diff --git a/tests/qtest/virtio-net-test.c b/tests/qtest/virtio-net-test.c
index fab5dd8b05..c37fe36af6 100644
--- a/tests/qtest/virtio-net-test.c
+++ b/tests/qtest/virtio-net-test.c
@@ -169,7 +169,6 @@ static void hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
 {
     QVirtioPCIDevice *dev = obj;
     QTestState *qts = dev->pdev->bus->qts;
-    const char *arch = qtest_get_arch();
 
     if (dev->pdev->bus->not_hotpluggable) {
         g_test_skip("pci bus does not support hotplug");
@@ -179,7 +178,7 @@ static void hotplug(void *obj, void *data, QGuestAllocator *t_alloc)
     qtest_qmp_device_add(qts, "virtio-net-pci", "net1",
                          "{'addr': %s}", stringify(PCI_SLOT_HP));
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         qpci_unplug_acpi_device_test(qts, "net1", PCI_SLOT_HP);
     }
 }
diff --git a/tests/qtest/virtio-rng-test.c b/tests/qtest/virtio-rng-test.c
index 64e131cd8c..173f42ff7b 100644
--- a/tests/qtest/virtio-rng-test.c
+++ b/tests/qtest/virtio-rng-test.c
@@ -25,12 +25,10 @@ static void rng_hotplug(void *obj, void *data, QGuestAllocator *alloc)
         return;
     }
 
-    const char *arch = qtest_get_arch();
-
     qtest_qmp_device_add(qts, "virtio-rng-pci", "rng1",
                          "{'addr': %s}", stringify(PCI_SLOT_HP));
 
-    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+    if (strcmp(qtest_get_base_arch(), "x86") == 0) {
         qpci_unplug_acpi_device_test(qts, "rng1", PCI_SLOT_HP);
     }
 }
-- 
2.41.0



^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/4] tests/libqtest: Introduce qtest_get_base_arch()
  2023-10-10  7:49 ` [PATCH 3/4] tests/libqtest: Introduce qtest_get_base_arch() Philippe Mathieu-Daudé
@ 2023-10-10  8:42   ` Philippe Mathieu-Daudé
  2023-10-10  9:50     ` Thomas Huth
  0 siblings, 1 reply; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-10  8:42 UTC (permalink / raw)
  To: qemu-devel
  Cc: Thomas Huth, Paolo Bonzini, qemu-ppc, Alex Bennée, qemu-arm,
	qemu-block, Laurent Vivier

On 10/10/23 09:49, Philippe Mathieu-Daudé wrote:
> While qtest_get_arch() returns the target architecture name,
> such "i386" or "x86_64", qtest_get_base_arch() return the
> "base" (or real underlying) architecture, in this example
> that is "x86".
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/qtest/libqtest.h |  7 +++++++
>   tests/qtest/libqtest.c | 28 ++++++++++++++++++++++++++++
>   2 files changed, 35 insertions(+)
> 
> diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
> index 1e1b42241d..54071e74ec 100644
> --- a/tests/qtest/libqtest.h
> +++ b/tests/qtest/libqtest.h
> @@ -654,6 +654,13 @@ bool qtest_big_endian(QTestState *s);
>    */
>   const char *qtest_get_arch(void);
>   
> +/**
> + * qtest_get_base_arch:
> + *
> + * Returns: The base architecture for the QEMU executable under test.
> + */
> +const char *qtest_get_base_arch(void);
> +
>   /**
>    * qtest_get_arch_bits:
>    *
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index a643a6309c..51cc92af21 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -925,6 +925,34 @@ const char *qtest_get_arch(void)
>       return end + 1;
>   }
>   
> +const char *qtest_get_base_arch(void)
> +{
> +    static const struct {
> +        const char *const arch;
> +        const char *const base;
> +    } basearch[] = {
> +        { "aarch64", "arm" },
> +        { "i386", "x86" },
> +        { "loongarch64", "loongarch" },
> +        { "mipsel", "mips" },
> +        { "mips64", "mips" },
> +        { "mips64el", "mips" },
> +        { "ppc64", "ppc" },
> +        { "riscv32", "riscv" },
> +        { "riscv64", "riscv" },
> +        { "sparc64", "sparc" },
> +        { "x86_64", "x86" },
> +    };
> +    const char *arch = qtest_get_arch();
> +
> +    for (unsigned i = 0; i < ARRAY_SIZE(basearch); i++) {
> +        if (!strcmp(arch, basearch[i].arch)) {
> +            return basearch[i].base;
> +        }
> +    }
> +    g_assert_not_reached();

Sorry, I forgot to commit this change:

-- >8 --
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -950,7 +950,8 @@ const char *qtest_get_base_arch(void)
              return basearch[i].base;
          }
      }
-    g_assert_not_reached();
+
+    return arch;
  }

---

> +}
> +
>   unsigned qtest_get_arch_bits(void)
>   {
>       static const char *const arch64[] = {



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/4] tests/qtest: Use qtest_get_base_arch()
  2023-10-10  7:49 ` [PATCH 4/4] tests/qtest: Use qtest_get_base_arch() Philippe Mathieu-Daudé
@ 2023-10-10  9:38   ` Akihiko Odaki
  2023-10-10  9:58   ` Thomas Huth
  1 sibling, 0 replies; 14+ messages in thread
From: Akihiko Odaki @ 2023-10-10  9:38 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Thomas Huth, Paolo Bonzini, qemu-ppc, Alex Bennée, qemu-arm,
	qemu-block, John Snow, Laurent Vivier, Fam Zheng,
	Michael S. Tsirkin, Igor Mammedov, Ani Sinha, Alexander Bulekov,
	Bandan Das, Stefan Hajnoczi, Darren Kenny, Qiuhao Li,
	Dmitry Fleytman, Nicholas Piggin, Daniel Henrique Barboza,
	Cédric Le Goater, David Gibson, Harsh Prateek Bora, Jeuk Kim,
	Gerd Hoffmann, Jason Wang, Amit Shah

On 2023/10/10 16:49, Philippe Mathieu-Daudé wrote:
> Additionally use qtest_get_arch_bits() when relevant.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Reviewed-by: Akihiko Odaki <akihiko.odaki@daynix.com>


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] tests/libqtest: Introduce qtest_get_arch_bits()
  2023-10-10  7:49 ` [PATCH 1/4] tests/libqtest: Introduce qtest_get_arch_bits() Philippe Mathieu-Daudé
@ 2023-10-10  9:46   ` Thomas Huth
  2023-10-10  9:48     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Huth @ 2023-10-10  9:46 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Paolo Bonzini, qemu-ppc, Alex Bennée, qemu-arm, qemu-block,
	Laurent Vivier

On 10/10/2023 09.49, Philippe Mathieu-Daudé wrote:
> Add a method to return the architecture bits (currently 8/32/64).
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/qtest/libqtest.h |  8 ++++++++
>   tests/qtest/libqtest.c | 21 +++++++++++++++++++++
>   2 files changed, 29 insertions(+)
> 
> diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
> index e53e350e3a..1e1b42241d 100644
> --- a/tests/qtest/libqtest.h
> +++ b/tests/qtest/libqtest.h
> @@ -654,6 +654,14 @@ bool qtest_big_endian(QTestState *s);
>    */
>   const char *qtest_get_arch(void);
>   
> +/**
> + * qtest_get_arch_bits:
> + *
> + * Returns: The architecture bits (a.k.a. word size) for the QEMU executable
> + * under test.
> + */
> +unsigned qtest_get_arch_bits(void);
> +
>   /**
>    * qtest_has_accel:
>    * @accel_name: Accelerator name to check for.
> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
> index b1eba71ffe..a643a6309c 100644
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -925,6 +925,27 @@ const char *qtest_get_arch(void)
>       return end + 1;
>   }
>   
> +unsigned qtest_get_arch_bits(void)
> +{
> +    static const char *const arch64[] = {
> +        "aarch64", "hppa", "x86_64", "loongarch64", "mips64",
> +        "mips64el", "ppc64", "riscv64", "s390x", "sparc64",
> +    };
> +    const char *arch = qtest_get_arch();
> +
> +    if (!strcmp(arch, "avr")) {

Just a matter of taste, but I prefer g_str_equal(), that's easier to read.

> +        return 8;
> +    }
> +
> +    for (unsigned i = 0; i < ARRAY_SIZE(arch64); i++) {
> +        if (!strcmp(arch, arch64[i])) {
> +            return 64;
> +        }
> +    }
> +
> +    return 32;
> +}

Since this function might get called multiple times, would it make sense to 
cache the value? I.e.:

   static const unsigned bits;

   if (!bits) {
       ... do all the magic to find out the right bits ...
   }

   return bits;

?

  Thomas



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/4] tests/libqtest: Introduce qtest_get_arch_bits()
  2023-10-10  9:46   ` Thomas Huth
@ 2023-10-10  9:48     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-10  9:48 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel
  Cc: Paolo Bonzini, qemu-ppc, Alex Bennée, qemu-arm, qemu-block,
	Laurent Vivier

On 10/10/23 11:46, Thomas Huth wrote:
> On 10/10/2023 09.49, Philippe Mathieu-Daudé wrote:
>> Add a method to return the architecture bits (currently 8/32/64).
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   tests/qtest/libqtest.h |  8 ++++++++
>>   tests/qtest/libqtest.c | 21 +++++++++++++++++++++
>>   2 files changed, 29 insertions(+)

>> +unsigned qtest_get_arch_bits(void)
>> +{
>> +    static const char *const arch64[] = {
>> +        "aarch64", "hppa", "x86_64", "loongarch64", "mips64",
>> +        "mips64el", "ppc64", "riscv64", "s390x", "sparc64",
>> +    };
>> +    const char *arch = qtest_get_arch();
>> +
>> +    if (!strcmp(arch, "avr")) {
> 
> Just a matter of taste, but I prefer g_str_equal(), that's easier to read.
> 
>> +        return 8;
>> +    }
>> +
>> +    for (unsigned i = 0; i < ARRAY_SIZE(arch64); i++) {
>> +        if (!strcmp(arch, arch64[i])) {
>> +            return 64;
>> +        }
>> +    }
>> +
>> +    return 32;
>> +}
> 
> Since this function might get called multiple times, would it make sense 
> to cache the value? I.e.:
> 
>    static const unsigned bits;
> 
>    if (!bits) {
>        ... do all the magic to find out the right bits ...
>    }
> 
>    return bits;
> 
> ?

Fine by me, I'll do as suggested in v2.



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/4] tests/qtest: Use qtest_get_arch_bits()
  2023-10-10  7:49 ` [PATCH 2/4] tests/qtest: Use qtest_get_arch_bits() Philippe Mathieu-Daudé
@ 2023-10-10  9:49   ` Thomas Huth
  2023-10-10 10:03     ` Ani Sinha
  0 siblings, 1 reply; 14+ messages in thread
From: Thomas Huth @ 2023-10-10  9:49 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Paolo Bonzini, qemu-ppc, Alex Bennée, qemu-arm, qemu-block,
	Peter Maydell, Laurent Vivier, Michael S. Tsirkin, Igor Mammedov,
	Ani Sinha

On 10/10/2023 09.49, Philippe Mathieu-Daudé wrote:

Some short patch description, please! Why is this necessary/useful?
(I think I know, but other might not, and it is also important for the history)

> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/qtest/arm-cpu-features.c | 49 ++++++++++++++++------------------
>   tests/qtest/bios-tables-test.c | 16 +++++------
>   2 files changed, 30 insertions(+), 35 deletions(-)

  Thomas



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/4] tests/libqtest: Introduce qtest_get_base_arch()
  2023-10-10  8:42   ` Philippe Mathieu-Daudé
@ 2023-10-10  9:50     ` Thomas Huth
  0 siblings, 0 replies; 14+ messages in thread
From: Thomas Huth @ 2023-10-10  9:50 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Paolo Bonzini, qemu-ppc, Alex Bennée, qemu-arm, qemu-block,
	Laurent Vivier

On 10/10/2023 10.42, Philippe Mathieu-Daudé wrote:
> On 10/10/23 09:49, Philippe Mathieu-Daudé wrote:
>> While qtest_get_arch() returns the target architecture name,
>> such "i386" or "x86_64", qtest_get_base_arch() return the
>> "base" (or real underlying) architecture, in this example
>> that is "x86".
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   tests/qtest/libqtest.h |  7 +++++++
>>   tests/qtest/libqtest.c | 28 ++++++++++++++++++++++++++++
>>   2 files changed, 35 insertions(+)
>>
>> diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
>> index 1e1b42241d..54071e74ec 100644
>> --- a/tests/qtest/libqtest.h
>> +++ b/tests/qtest/libqtest.h
>> @@ -654,6 +654,13 @@ bool qtest_big_endian(QTestState *s);
>>    */
>>   const char *qtest_get_arch(void);
>> +/**
>> + * qtest_get_base_arch:
>> + *
>> + * Returns: The base architecture for the QEMU executable under test.
>> + */
>> +const char *qtest_get_base_arch(void);
>> +
>>   /**
>>    * qtest_get_arch_bits:
>>    *
>> diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
>> index a643a6309c..51cc92af21 100644
>> --- a/tests/qtest/libqtest.c
>> +++ b/tests/qtest/libqtest.c
>> @@ -925,6 +925,34 @@ const char *qtest_get_arch(void)
>>       return end + 1;
>>   }
>> +const char *qtest_get_base_arch(void)
>> +{
>> +    static const struct {
>> +        const char *const arch;
>> +        const char *const base;
>> +    } basearch[] = {
>> +        { "aarch64", "arm" },
>> +        { "i386", "x86" },
>> +        { "loongarch64", "loongarch" },
>> +        { "mipsel", "mips" },
>> +        { "mips64", "mips" },
>> +        { "mips64el", "mips" },
>> +        { "ppc64", "ppc" },
>> +        { "riscv32", "riscv" },
>> +        { "riscv64", "riscv" },
>> +        { "sparc64", "sparc" },
>> +        { "x86_64", "x86" },
>> +    };
>> +    const char *arch = qtest_get_arch();
>> +
>> +    for (unsigned i = 0; i < ARRAY_SIZE(basearch); i++) {
>> +        if (!strcmp(arch, basearch[i].arch)) {
>> +            return basearch[i].base;
>> +        }
>> +    }
>> +    g_assert_not_reached();
> 
> Sorry, I forgot to commit this change:
> 
> -- >8 --
> --- a/tests/qtest/libqtest.c
> +++ b/tests/qtest/libqtest.c
> @@ -950,7 +950,8 @@ const char *qtest_get_base_arch(void)
>               return basearch[i].base;
>           }
>       }
> -    g_assert_not_reached();
> +
> +    return arch;
>   }

I'd maybe also do a caching here, as suggested in the first patch.

  Thomas




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/4] tests/qtest: Use qtest_get_base_arch()
  2023-10-10  7:49 ` [PATCH 4/4] tests/qtest: Use qtest_get_base_arch() Philippe Mathieu-Daudé
  2023-10-10  9:38   ` Akihiko Odaki
@ 2023-10-10  9:58   ` Thomas Huth
  2023-10-10 10:37     ` Philippe Mathieu-Daudé
  1 sibling, 1 reply; 14+ messages in thread
From: Thomas Huth @ 2023-10-10  9:58 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Paolo Bonzini, qemu-ppc, Alex Bennée, qemu-arm, qemu-block,
	John Snow, Laurent Vivier, Fam Zheng, Michael S. Tsirkin,
	Igor Mammedov, Ani Sinha, Alexander Bulekov, Bandan Das,
	Stefan Hajnoczi, Darren Kenny, Qiuhao Li, Dmitry Fleytman,
	Akihiko Odaki, Nicholas Piggin, Daniel Henrique Barboza,
	Cédric Le Goater, David Gibson, Harsh Prateek Bora, Jeuk Kim,
	Gerd Hoffmann, Jason Wang, Amit Shah

On 10/10/2023 09.49, Philippe Mathieu-Daudé wrote:
> Additionally use qtest_get_arch_bits() when relevant.
> 
> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> ---
>   tests/qtest/ahci-test.c           |  5 ++---
>   tests/qtest/am53c974-test.c       |  4 +---
>   tests/qtest/bios-tables-test.c    |  6 +++---
>   tests/qtest/boot-sector.c         |  6 +++---
>   tests/qtest/device-plug-test.c    |  6 ++----
>   tests/qtest/drive_del-test.c      | 16 ++++++----------
>   tests/qtest/erst-test.c           |  4 +---
>   tests/qtest/fuzz-e1000e-test.c    |  4 +---
>   tests/qtest/ivshmem-test.c        |  6 +++---
>   tests/qtest/libqos/qos_external.c |  6 ++++--
>   tests/qtest/lpc-ich9-test.c       |  4 +---
>   tests/qtest/m48t59-test.c         |  6 ++----
>   tests/qtest/pxe-test.c            |  6 +++---
>   tests/qtest/qos-test.c            |  6 ++++--
>   tests/qtest/readconfig-test.c     |  6 +-----
>   tests/qtest/rtas-test.c           |  4 +---
>   tests/qtest/ufs-test.c            |  4 +---
>   tests/qtest/usb-hcd-uhci-test.c   |  6 +++---
>   tests/qtest/virtio-net-test.c     |  3 +--
>   tests/qtest/virtio-rng-test.c     |  4 +---
>   20 files changed, 44 insertions(+), 68 deletions(-)
> 
> diff --git a/tests/qtest/ahci-test.c b/tests/qtest/ahci-test.c
> index eea8b5f77b..93d1e14896 100644
> --- a/tests/qtest/ahci-test.c
> +++ b/tests/qtest/ahci-test.c
> @@ -1835,7 +1835,7 @@ static void create_ahci_io_test(enum IOMode type, enum AddrMode addr,
>   
>   int main(int argc, char **argv)
>   {
> -    const char *arch, *base;
> +    const char *base;
>       int ret;
>       int fd;
>       int c;
> @@ -1867,8 +1867,7 @@ int main(int argc, char **argv)
>       }
>   
>       /* Check architecture */
> -    arch = qtest_get_arch();
> -    if (strcmp(arch, "i386") && strcmp(arch, "x86_64")) {
> +    if (strcmp(qtest_get_base_arch(), "x86")) {
>           g_test_message("Skipping test for non-x86");
>           return 0;
>       }

While this change makes sense (unifying two checks into one) ...

> diff --git a/tests/qtest/am53c974-test.c b/tests/qtest/am53c974-test.c
> index ed3ac7db20..dc41182a38 100644
> --- a/tests/qtest/am53c974-test.c
> +++ b/tests/qtest/am53c974-test.c
> @@ -253,11 +253,9 @@ static void test_reset_before_transfer_ok(void)
>   
>   int main(int argc, char **argv)
>   {
> -    const char *arch = qtest_get_arch();
> -
>       g_test_init(&argc, &argv, NULL);
>   
> -    if (strcmp(arch, "i386") == 0) {
> +    if (!strcmp(qtest_get_base_arch(), "x86") && qtest_get_arch_bits() == 32) {

... this change looks more cumbersome now (doing two checks now instead of 
one), at least at the current point in time. Do you urgently need this for 
your refactoring? If not, I'd maybe postpone such changes that make the 
checks more compilcated to a later point in time.

  Thomas




^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/4] tests/qtest: Use qtest_get_arch_bits()
  2023-10-10  9:49   ` Thomas Huth
@ 2023-10-10 10:03     ` Ani Sinha
  0 siblings, 0 replies; 14+ messages in thread
From: Ani Sinha @ 2023-10-10 10:03 UTC (permalink / raw)
  To: Thomas Huth
  Cc: Philippe Mathieu-Daudé, qemu-devel, Paolo Bonzini, qemu-ppc,
	Alex Bennée, qemu-arm, qemu-block, Peter Maydell,
	Laurent Vivier, Michael S. Tsirkin, Igor Mammedov



> On 10-Oct-2023, at 3:19 PM, Thomas Huth <thuth@redhat.com> wrote:
> 
> On 10/10/2023 09.49, Philippe Mathieu-Daudé wrote:
> 
> Some short patch description, please! Why is this necessary/useful?
> (I think I know, but other might not, and it is also important for the history)

Other than this,
Reviewed-by: Ani Sinha <anisinha@redhat.com>

> 
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>  tests/qtest/arm-cpu-features.c | 49 ++++++++++++++++------------------
>>  tests/qtest/bios-tables-test.c | 16 +++++------
>>  2 files changed, 30 insertions(+), 35 deletions(-)
> 
> Thomas
> 



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 4/4] tests/qtest: Use qtest_get_base_arch()
  2023-10-10  9:58   ` Thomas Huth
@ 2023-10-10 10:37     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 14+ messages in thread
From: Philippe Mathieu-Daudé @ 2023-10-10 10:37 UTC (permalink / raw)
  To: Thomas Huth, qemu-devel
  Cc: Paolo Bonzini, qemu-ppc, Alex Bennée, qemu-arm, qemu-block,
	John Snow, Laurent Vivier, Fam Zheng, Michael S. Tsirkin,
	Igor Mammedov, Ani Sinha, Alexander Bulekov, Bandan Das,
	Stefan Hajnoczi, Darren Kenny, Qiuhao Li, Dmitry Fleytman,
	Akihiko Odaki, Nicholas Piggin, Daniel Henrique Barboza,
	Cédric Le Goater, David Gibson, Harsh Prateek Bora, Jeuk Kim,
	Gerd Hoffmann, Jason Wang, Amit Shah

On 10/10/23 11:58, Thomas Huth wrote:
> On 10/10/2023 09.49, Philippe Mathieu-Daudé wrote:
>> Additionally use qtest_get_arch_bits() when relevant.
>>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
>> ---
>>   tests/qtest/ahci-test.c           |  5 ++---
>>   tests/qtest/am53c974-test.c       |  4 +---
>>   tests/qtest/bios-tables-test.c    |  6 +++---
>>   tests/qtest/boot-sector.c         |  6 +++---
>>   tests/qtest/device-plug-test.c    |  6 ++----
>>   tests/qtest/drive_del-test.c      | 16 ++++++----------


>> @@ -1867,8 +1867,7 @@ int main(int argc, char **argv)
>>       }
>>       /* Check architecture */
>> -    arch = qtest_get_arch();
>> -    if (strcmp(arch, "i386") && strcmp(arch, "x86_64")) {
>> +    if (strcmp(qtest_get_base_arch(), "x86")) {
>>           g_test_message("Skipping test for non-x86");
>>           return 0;
>>       }
> 
> While this change makes sense (unifying two checks into one) ...
> 
>> diff --git a/tests/qtest/am53c974-test.c b/tests/qtest/am53c974-test.c
>> index ed3ac7db20..dc41182a38 100644
>> --- a/tests/qtest/am53c974-test.c
>> +++ b/tests/qtest/am53c974-test.c
>> @@ -253,11 +253,9 @@ static void test_reset_before_transfer_ok(void)
>>   int main(int argc, char **argv)
>>   {
>> -    const char *arch = qtest_get_arch();
>> -
>>       g_test_init(&argc, &argv, NULL);
>> -    if (strcmp(arch, "i386") == 0) {
>> +    if (!strcmp(qtest_get_base_arch(), "x86") && 
>> qtest_get_arch_bits() == 32) {
> 
> ... this change looks more cumbersome now (doing two checks now instead 
> of one), at least at the current point in time. Do you urgently need 
> this for your refactoring? If not, I'd maybe postpone such changes that 
> make the checks more compilcated to a later point in time.

I wanted to replace qtest_get_arch() completely, but not that simple :(



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2023-10-10 10:38 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-10-10  7:49 [PATCH 0/4] tests/qtest: Introduce qtest_get_base_arch() and qtest_get_arch_bits() Philippe Mathieu-Daudé
2023-10-10  7:49 ` [PATCH 1/4] tests/libqtest: Introduce qtest_get_arch_bits() Philippe Mathieu-Daudé
2023-10-10  9:46   ` Thomas Huth
2023-10-10  9:48     ` Philippe Mathieu-Daudé
2023-10-10  7:49 ` [PATCH 2/4] tests/qtest: Use qtest_get_arch_bits() Philippe Mathieu-Daudé
2023-10-10  9:49   ` Thomas Huth
2023-10-10 10:03     ` Ani Sinha
2023-10-10  7:49 ` [PATCH 3/4] tests/libqtest: Introduce qtest_get_base_arch() Philippe Mathieu-Daudé
2023-10-10  8:42   ` Philippe Mathieu-Daudé
2023-10-10  9:50     ` Thomas Huth
2023-10-10  7:49 ` [PATCH 4/4] tests/qtest: Use qtest_get_base_arch() Philippe Mathieu-Daudé
2023-10-10  9:38   ` Akihiko Odaki
2023-10-10  9:58   ` Thomas Huth
2023-10-10 10:37     ` Philippe Mathieu-Daudé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).