qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: Ani Sinha <anisinha@redhat.com>
To: "Michael S. Tsirkin" <mst@redhat.com>,
	Marcel Apfelbaum <marcel.apfelbaum@gmail.com>,
	Thomas Huth <thuth@redhat.com>,
	Laurent Vivier <lvivier@redhat.com>,
	Paolo Bonzini <pbonzini@redhat.com>
Cc: Ani Sinha <anisinha@redhat.com>,
	imammedo@redhat.com, qemu-devel@nongnu.org
Subject: [PATCH v3 3/3] tests/qtest/x86: check for availability of older cpu models before running tests
Date: Thu,  6 Jun 2024 10:14:19 +0530	[thread overview]
Message-ID: <20240606044419.8806-4-anisinha@redhat.com> (raw)
In-Reply-To: <20240606044419.8806-1-anisinha@redhat.com>

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

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

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

CC: thuth@redhat.com
CC: imammedo@redhat.com
Signed-off-by: Ani Sinha <anisinha@redhat.com>
---
 tests/qtest/test-x86-cpuid-compat.c | 170 ++++++++++++++++++----------
 1 file changed, 108 insertions(+), 62 deletions(-)

changelog:
v2: reworked as per suggestion from danpb.
v3: reworked as_feature_test() same way as add_cpuid_test()

diff --git a/tests/qtest/test-x86-cpuid-compat.c b/tests/qtest/test-x86-cpuid-compat.c
index 6a39454fce..d5307f39e7 100644
--- a/tests/qtest/test-x86-cpuid-compat.c
+++ b/tests/qtest/test-x86-cpuid-compat.c
@@ -67,10 +67,29 @@ static void test_cpuid_prop(const void *data)
     g_free(path);
 }
 
-static void add_cpuid_test(const char *name, const char *cmdline,
+static void add_cpuid_test(const char *name, const char *cpu,
+                           const char *cpufeat, const char *machine,
                            const char *property, int64_t expected_value)
 {
     CpuidTestArgs *args = g_new0(CpuidTestArgs, 1);
+    char *cmdline;
+    char *save;
+
+    if (!qtest_has_cpu(cpu)) {
+        return;
+    }
+    cmdline = g_strdup_printf("-cpu %s", cpu);
+
+    if (cpufeat) {
+        save = cmdline;
+        cmdline = g_strdup_printf("%s,%s", cmdline, cpufeat);
+        g_free(save);
+    }
+    if (machine) {
+        save = cmdline;
+        cmdline = g_strdup_printf("-machine %s %s", machine, cmdline);
+        g_free(save);
+    }
     args->cmdline = cmdline;
     args->property = property;
     args->expected_value = expected_value;
@@ -149,12 +168,24 @@ static void test_feature_flag(const void *data)
  * either "feature-words" or "filtered-features", when running QEMU
  * using cmdline
  */
-static FeatureTestArgs *add_feature_test(const char *name, const char *cmdline,
-                                         uint32_t eax, uint32_t ecx,
-                                         const char *reg, int bitnr,
-                                         bool expected_value)
+static void add_feature_test(const char *name, const char *cpu,
+                             const char *cpufeat, uint32_t eax,
+                             uint32_t ecx, const char *reg,
+                             int bitnr, bool expected_value)
 {
     FeatureTestArgs *args = g_new0(FeatureTestArgs, 1);
+    char *cmdline;
+
+    if (!qtest_has_cpu(cpu)) {
+        return;
+    }
+
+    if (cpufeat) {
+        cmdline = g_strdup_printf("-cpu %s,%s", cpu, cpufeat);
+    } else {
+        cmdline = g_strdup_printf("-cpu %s", cpu);
+    }
+
     args->cmdline = cmdline;
     args->in_eax = eax;
     args->in_ecx = ecx;
@@ -162,13 +193,17 @@ static FeatureTestArgs *add_feature_test(const char *name, const char *cmdline,
     args->bitnr = bitnr;
     args->expected_value = expected_value;
     qtest_add_data_func(name, args, test_feature_flag);
-    return args;
+    return;
 }
 
 static void test_plus_minus_subprocess(void)
 {
     char *path;
 
+    if (!qtest_has_cpu("pentium")) {
+        return;
+    }
+
     /* Rules:
      * 1)"-foo" overrides "+foo"
      * 2) "[+-]foo" overrides "foo=..."
@@ -198,6 +233,10 @@ static void test_plus_minus_subprocess(void)
 
 static void test_plus_minus(void)
 {
+    if (!qtest_has_cpu("pentium")) {
+        return;
+    }
+
     g_test_trap_subprocess("/x86/cpuid/parsing-plus-minus/subprocess", 0, 0);
     g_test_trap_assert_passed();
     g_test_trap_assert_stderr("*Ambiguous CPU model string. "
@@ -217,99 +256,105 @@ int main(int argc, char **argv)
 
     /* Original level values for CPU models: */
     add_cpuid_test("x86/cpuid/phenom/level",
-                   "-cpu phenom", "level", 5);
+                   "phenom", NULL, NULL, "level", 5);
     add_cpuid_test("x86/cpuid/Conroe/level",
-                   "-cpu Conroe", "level", 10);
+                   "Conroe", NULL, NULL, "level", 10);
     add_cpuid_test("x86/cpuid/SandyBridge/level",
-                   "-cpu SandyBridge", "level", 0xd);
+                   "SandyBridge", NULL, NULL, "level", 0xd);
     add_cpuid_test("x86/cpuid/486/xlevel",
-                   "-cpu 486", "xlevel", 0);
+                   "486", NULL, NULL, "xlevel", 0);
     add_cpuid_test("x86/cpuid/core2duo/xlevel",
-                   "-cpu core2duo", "xlevel", 0x80000008);
+                   "core2duo", NULL, NULL, "xlevel", 0x80000008);
     add_cpuid_test("x86/cpuid/phenom/xlevel",
-                   "-cpu phenom", "xlevel", 0x8000001A);
+                   "phenom", NULL, NULL, "xlevel", 0x8000001A);
     add_cpuid_test("x86/cpuid/athlon/xlevel",
-                   "-cpu athlon", "xlevel", 0x80000008);
+                   "athlon", NULL, NULL, "xlevel", 0x80000008);
 
     /* If level is not large enough, it should increase automatically: */
     /* CPUID[6].EAX: */
-    add_cpuid_test("x86/cpuid/auto-level/phenom/arat",
-                   "-cpu 486,arat=on", "level", 6);
+    add_cpuid_test("x86/cpuid/auto-level/486/arat",
+                   "486", "arat=on", NULL, "level", 6);
     /* CPUID[EAX=7,ECX=0].EBX: */
     add_cpuid_test("x86/cpuid/auto-level/phenom/fsgsbase",
-                   "-cpu phenom,fsgsbase=on", "level", 7);
+                   "phenom", "fsgsbase=on", NULL, "level", 7);
     /* CPUID[EAX=7,ECX=0].ECX: */
     add_cpuid_test("x86/cpuid/auto-level/phenom/avx512vbmi",
-                   "-cpu phenom,avx512vbmi=on", "level", 7);
+                   "phenom", "avx512vbmi=on", NULL, "level", 7);
     /* CPUID[EAX=0xd,ECX=1].EAX: */
     add_cpuid_test("x86/cpuid/auto-level/phenom/xsaveopt",
-                   "-cpu phenom,xsaveopt=on", "level", 0xd);
+                   "phenom", "xsaveopt=on", NULL, "level", 0xd);
     /* CPUID[8000_0001].EDX: */
     add_cpuid_test("x86/cpuid/auto-xlevel/486/3dnow",
-                   "-cpu 486,3dnow=on", "xlevel", 0x80000001);
+                   "486", "3dnow=on", NULL, "xlevel", 0x80000001);
     /* CPUID[8000_0001].ECX: */
     add_cpuid_test("x86/cpuid/auto-xlevel/486/sse4a",
-                   "-cpu 486,sse4a=on", "xlevel", 0x80000001);
+                   "486", "sse4a=on", NULL, "xlevel", 0x80000001);
     /* CPUID[8000_0007].EDX: */
     add_cpuid_test("x86/cpuid/auto-xlevel/486/invtsc",
-                   "-cpu 486,invtsc=on", "xlevel", 0x80000007);
+                   "486", "invtsc=on", NULL, "xlevel", 0x80000007);
     /* CPUID[8000_000A].EDX: */
     add_cpuid_test("x86/cpuid/auto-xlevel/486/npt",
-                   "-cpu 486,svm=on,npt=on", "xlevel", 0x8000000A);
+                   "486", "svm=on,npt=on", NULL, "xlevel", 0x8000000A);
     /* CPUID[C000_0001].EDX: */
     add_cpuid_test("x86/cpuid/auto-xlevel2/phenom/xstore",
-                   "-cpu phenom,xstore=on", "xlevel2", 0xC0000001);
+                   "phenom", "xstore=on", NULL, "xlevel2", 0xC0000001);
     /* SVM needs CPUID[0x8000000A] */
     add_cpuid_test("x86/cpuid/auto-xlevel/athlon/svm",
-                   "-cpu athlon,svm=on", "xlevel", 0x8000000A);
+                   "athlon", "svm=on", NULL, "xlevel", 0x8000000A);
 
 
     /* If level is already large enough, it shouldn't change: */
     add_cpuid_test("x86/cpuid/auto-level/SandyBridge/multiple",
-                   "-cpu SandyBridge,arat=on,fsgsbase=on,avx512vbmi=on",
-                   "level", 0xd);
+                   "SandyBridge", "arat=on,fsgsbase=on,avx512vbmi=on",
+                   NULL, "level", 0xd);
     /* If level is explicitly set, it shouldn't change: */
     add_cpuid_test("x86/cpuid/auto-level/486/fixed/0xF",
-                   "-cpu 486,level=0xF,arat=on,fsgsbase=on,avx512vbmi=on,xsaveopt=on",
-                   "level", 0xF);
+                   "486",
+                   "level=0xF,arat=on,fsgsbase=on,avx512vbmi=on,xsaveopt=on",
+                   NULL, "level", 0xF);
     add_cpuid_test("x86/cpuid/auto-level/486/fixed/2",
-                   "-cpu 486,level=2,arat=on,fsgsbase=on,avx512vbmi=on,xsaveopt=on",
-                   "level", 2);
+                   "486",
+                   "level=2,arat=on,fsgsbase=on,avx512vbmi=on,xsaveopt=on",
+                   NULL, "level", 2);
     add_cpuid_test("x86/cpuid/auto-level/486/fixed/0",
-                   "-cpu 486,level=0,arat=on,fsgsbase=on,avx512vbmi=on,xsaveopt=on",
-                   "level", 0);
+                   "486",
+                   "level=0,arat=on,fsgsbase=on,avx512vbmi=on,xsaveopt=on",
+                   NULL, "level", 0);
 
     /* if xlevel is already large enough, it shouldn't change: */
     add_cpuid_test("x86/cpuid/auto-xlevel/phenom/3dnow",
-                   "-cpu phenom,3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
-                   "xlevel", 0x8000001A);
+                   "phenom", "3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
+                   NULL, "xlevel", 0x8000001A);
     /* If xlevel is explicitly set, it shouldn't change: */
     add_cpuid_test("x86/cpuid/auto-xlevel/486/fixed/80000002",
-                   "-cpu 486,xlevel=0x80000002,3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
-                   "xlevel", 0x80000002);
+                   "486",
+                   "xlevel=0x80000002,3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
+                   NULL, "xlevel", 0x80000002);
     add_cpuid_test("x86/cpuid/auto-xlevel/486/fixed/8000001A",
-                   "-cpu 486,xlevel=0x8000001A,3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
-                   "xlevel", 0x8000001A);
+                   "486",
+                   "xlevel=0x8000001A,3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
+                   NULL, "xlevel", 0x8000001A);
     add_cpuid_test("x86/cpuid/auto-xlevel/phenom/fixed/0",
-                   "-cpu 486,xlevel=0,3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
-                   "xlevel", 0);
+                   "486",
+                   "xlevel=0,3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
+                   NULL, "xlevel", 0);
 
     /* if xlevel2 is already large enough, it shouldn't change: */
     add_cpuid_test("x86/cpuid/auto-xlevel2/486/fixed",
-                   "-cpu 486,xlevel2=0xC0000002,xstore=on",
-                   "xlevel2", 0xC0000002);
+                   "486", "xlevel2=0xC0000002,xstore=on",
+                   NULL, "xlevel2", 0xC0000002);
 
     /* Check compatibility of old machine-types that didn't
      * auto-increase level/xlevel/xlevel2: */
     if (qtest_has_machine("pc-i440fx-2.7")) {
         add_cpuid_test("x86/cpuid/auto-level/pc-2.7",
-                       "-machine pc-i440fx-2.7 -cpu 486,arat=on,avx512vbmi=on,xsaveopt=on",
-                       "level", 1);
+                       "486", "arat=on,avx512vbmi=on,xsaveopt=on",
+                       "pc-i440fx-2.7", "level", 1);
         add_cpuid_test("x86/cpuid/auto-xlevel/pc-2.7",
-                       "-machine pc-i440fx-2.7 -cpu 486,3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
-                       "xlevel", 0);
+                       "486", "3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
+                       "pc-i440fx-2.7", "xlevel", 0);
         add_cpuid_test("x86/cpuid/auto-xlevel2/pc-2.7",
-                       "-machine pc-i440fx-2.7 -cpu 486,xstore=on",
+                       "486", "xstore=on", "pc-i440fx-2.7",
                        "xlevel2", 0);
     }
     /*
@@ -319,18 +364,18 @@ int main(int argc, char **argv)
      */
     if (qtest_has_machine("pc-i440fx-2.3")) {
         add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-2.3/off",
-                       "-machine pc-i440fx-2.3 -cpu Penryn",
+                       "Penryn", NULL, "pc-i440fx-2.3",
                        "level", 4);
         add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-2.3/on",
-                       "-machine pc-i440fx-2.3 -cpu Penryn,erms=on",
+                       "Penryn", "erms=on", "pc-i440fx-2.3",
                        "level", 7);
     }
     if (qtest_has_machine("pc-i440fx-2.9")) {
         add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-2.9/off",
-                       "-machine pc-i440fx-2.9 -cpu Conroe",
+                       "Conroe", NULL, "pc-i440fx-2.9",
                        "level", 10);
         add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-2.9/on",
-                       "-machine pc-i440fx-2.9 -cpu Conroe,erms=on",
+                       "Conroe", "erms=on", "pc-i440fx-2.9",
                        "level", 10);
     }
 
@@ -341,42 +386,43 @@ int main(int argc, char **argv)
      */
     if (qtest_has_machine("pc-i440fx-2.3")) {
         add_cpuid_test("x86/cpuid/xlevel-compat/pc-i440fx-2.3",
-                       "-machine pc-i440fx-2.3 -cpu SandyBridge",
+                       "SandyBridge", NULL, "pc-i440fx-2.3",
                        "xlevel", 0x8000000a);
     }
     if (qtest_has_machine("pc-i440fx-2.4")) {
         add_cpuid_test("x86/cpuid/xlevel-compat/pc-i440fx-2.4/npt-off",
-                       "-machine pc-i440fx-2.4 -cpu SandyBridge,",
+                       "SandyBridge", NULL, "pc-i440fx-2.4",
                        "xlevel", 0x80000008);
         add_cpuid_test("x86/cpuid/xlevel-compat/pc-i440fx-2.4/npt-on",
-                       "-machine pc-i440fx-2.4 -cpu SandyBridge,svm=on,npt=on",
+                       "SandyBridge", "svm=on,npt=on", "pc-i440fx-2.4",
                        "xlevel", 0x80000008);
     }
 
     /* Test feature parsing */
     add_feature_test("x86/cpuid/features/plus",
-                     "-cpu 486,+arat",
+                     "486", "+arat",
                      6, 0, "EAX", 2, true);
     add_feature_test("x86/cpuid/features/minus",
-                     "-cpu pentium,-mmx",
+                     "pentium", "-mmx",
                      1, 0, "EDX", 23, false);
     add_feature_test("x86/cpuid/features/on",
-                     "-cpu 486,arat=on",
+                     "486", "arat=on",
                      6, 0, "EAX", 2, true);
     add_feature_test("x86/cpuid/features/off",
-                     "-cpu pentium,mmx=off",
+                     "pentium", "mmx=off",
                      1, 0, "EDX", 23, false);
+
     add_feature_test("x86/cpuid/features/max-plus-invtsc",
-                     "-cpu max,+invtsc",
+                     "max" , "+invtsc",
                      0x80000007, 0, "EDX", 8, true);
     add_feature_test("x86/cpuid/features/max-invtsc-on",
-                     "-cpu max,invtsc=on",
+                     "max", "invtsc=on",
                      0x80000007, 0, "EDX", 8, true);
     add_feature_test("x86/cpuid/features/max-minus-mmx",
-                     "-cpu max,-mmx",
+                     "max", "-mmx",
                      1, 0, "EDX", 23, false);
     add_feature_test("x86/cpuid/features/max-invtsc-on,mmx=off",
-                     "-cpu max,mmx=off",
+                     "max", "mmx=off",
                      1, 0, "EDX", 23, false);
 
     return g_test_run();
-- 
2.42.0



  parent reply	other threads:[~2024-06-06  4:45 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-06-06  4:44 [PATCH v3 0/3] x86 cpu test refactoring Ani Sinha
2024-06-06  4:44 ` [PATCH v3 1/3] qtest/x86/numa-test: do not use the obsolete 'pentium' cpu Ani Sinha
2024-06-06  4:44 ` [PATCH 2/3] tests/qtest/libqtest: add qtest_has_cpu() api Ani Sinha
2024-06-10  9:02   ` Daniel P. Berrangé
2024-06-06  4:44 ` Ani Sinha [this message]
2024-06-10  9:04   ` [PATCH v3 3/3] tests/qtest/x86: check for availability of older cpu models before running tests Daniel P. Berrangé

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20240606044419.8806-4-anisinha@redhat.com \
    --to=anisinha@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=lvivier@redhat.com \
    --cc=marcel.apfelbaum@gmail.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=thuth@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).