* [PATCH 0/3] x86 cpu test refactoring
@ 2024-06-05 7:25 Ani Sinha
2024-06-05 7:25 ` [PATCH v2 1/3] qtest/x86/numa-test: do not use the obsolete 'pentium' cpu Ani Sinha
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Ani Sinha @ 2024-06-05 7:25 UTC (permalink / raw)
Cc: Ani Sinha, thuth, imammedo, qemu-devel, pbonzini, lvivier
Add a new library api to check for the support of a specific cpu type.
Used the new api to check support for some older x86 cpu models before
running the tests.
CC: thuth@redhat.com
CC: imammedo@redhat.com
CC: qemu-devel@nongnu.org
CC: pbonzini@redhat.com
CC: lvivier@redhat.com
Ani Sinha (3):
qtest/x86/numa-test: do not use the obsolete 'pentium' cpu
tests/qtest/libqtest: add qtest_has_cpu() api
tests/qtest/x86: check for availability of older cpu models before
running tests
tests/qtest/libqtest.c | 84 +++++++++++
tests/qtest/libqtest.h | 8 ++
tests/qtest/numa-test.c | 3 +-
tests/qtest/test-x86-cpuid-compat.c | 214 +++++++++++++++++-----------
4 files changed, 221 insertions(+), 88 deletions(-)
--
2.42.0
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/3] qtest/x86/numa-test: do not use the obsolete 'pentium' cpu
2024-06-05 7:25 [PATCH 0/3] x86 cpu test refactoring Ani Sinha
@ 2024-06-05 7:25 ` Ani Sinha
2024-06-05 7:25 ` [PATCH 2/3] tests/qtest/libqtest: add qtest_has_cpu() api Ani Sinha
2024-06-05 7:25 ` [PATCH 3/3] tests/qtest/x86: check for availability of older cpu models before running tests Ani Sinha
2 siblings, 0 replies; 5+ messages in thread
From: Ani Sinha @ 2024-06-05 7:25 UTC (permalink / raw)
To: Thomas Huth, Laurent Vivier, Paolo Bonzini; +Cc: Ani Sinha, qemu-devel
'pentium' cpu is old and obsolete and should be avoided for running tests if
its not strictly needed. Use 'max' cpu instead for generic non-cpu specific
numa test.
CC: thuth@redhat.com
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Ani Sinha <anisinha@redhat.com>
---
tests/qtest/numa-test.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
changelog:
review tag added. No other changes.
diff --git a/tests/qtest/numa-test.c b/tests/qtest/numa-test.c
index 7aa262dbb9..f01f19592d 100644
--- a/tests/qtest/numa-test.c
+++ b/tests/qtest/numa-test.c
@@ -125,7 +125,8 @@ static void pc_numa_cpu(const void *data)
QTestState *qts;
g_autofree char *cli = NULL;
- cli = make_cli(data, "-cpu pentium -machine smp.cpus=8,smp.sockets=2,smp.cores=2,smp.threads=2 "
+ cli = make_cli(data,
+ "-cpu max -machine smp.cpus=8,smp.sockets=2,smp.cores=2,smp.threads=2 "
"-numa node,nodeid=0,memdev=ram -numa node,nodeid=1 "
"-numa cpu,node-id=1,socket-id=0 "
"-numa cpu,node-id=0,socket-id=1,core-id=0 "
--
2.42.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/3] tests/qtest/libqtest: add qtest_has_cpu() api
2024-06-05 7:25 [PATCH 0/3] x86 cpu test refactoring Ani Sinha
2024-06-05 7:25 ` [PATCH v2 1/3] qtest/x86/numa-test: do not use the obsolete 'pentium' cpu Ani Sinha
@ 2024-06-05 7:25 ` Ani Sinha
2024-06-05 7:25 ` [PATCH 3/3] tests/qtest/x86: check for availability of older cpu models before running tests Ani Sinha
2 siblings, 0 replies; 5+ messages in thread
From: Ani Sinha @ 2024-06-05 7:25 UTC (permalink / raw)
To: Thomas Huth, Laurent Vivier, Paolo Bonzini; +Cc: Ani Sinha, qemu-devel
Added a new test api qtest_has_cpu() in order to check availability of some
cpu models in the current QEMU binary. The specific architecture of the QEMU
binary is selected using the QTEST_QEMU_BINARY environment variable. This api
would be useful to run tests against some older cpu models after checking if
QEMU actually supported these models.
CC: thuth@redhat.com
Signed-off-by: Ani Sinha <anisinha@redhat.com>
---
tests/qtest/libqtest.c | 84 ++++++++++++++++++++++++++++++++++++++++++
tests/qtest/libqtest.h | 8 ++++
2 files changed, 92 insertions(+)
diff --git a/tests/qtest/libqtest.c b/tests/qtest/libqtest.c
index d8f80d335e..135a607728 100644
--- a/tests/qtest/libqtest.c
+++ b/tests/qtest/libqtest.c
@@ -37,6 +37,7 @@
#include "qapi/qmp/qjson.h"
#include "qapi/qmp/qlist.h"
#include "qapi/qmp/qstring.h"
+#include "qapi/qmp/qbool.h"
#define MAX_IRQ 256
@@ -1471,6 +1472,12 @@ struct MachInfo {
char *alias;
};
+struct CpuInfo {
+ char *name;
+ char *alias_of;
+ bool deprecated;
+};
+
static void qtest_free_machine_list(struct MachInfo *machines)
{
if (machines) {
@@ -1550,6 +1557,83 @@ static struct MachInfo *qtest_get_machines(const char *var)
return machines;
}
+static struct CpuInfo *qtest_get_cpus(void)
+{
+ static struct CpuInfo *cpus;
+ QDict *response, *minfo;
+ QList *list;
+ const QListEntry *p;
+ QObject *qobj;
+ QString *qstr;
+ QBool *qbool;
+ QTestState *qts;
+ int idx;
+
+ if (cpus) {
+ return cpus;
+ }
+
+ silence_spawn_log = !g_test_verbose();
+
+ qts = qtest_init_with_env(NULL, "-machine none");
+ response = qtest_qmp(qts, "{ 'execute': 'query-cpu-definitions' }");
+ g_assert(response);
+ list = qdict_get_qlist(response, "return");
+ g_assert(list);
+
+ cpus = g_new(struct CpuInfo, qlist_size(list) + 1);
+
+ for (p = qlist_first(list), idx = 0; p; p = qlist_next(p), idx++) {
+ minfo = qobject_to(QDict, qlist_entry_obj(p));
+ g_assert(minfo);
+
+ qobj = qdict_get(minfo, "name");
+ g_assert(qobj);
+ qstr = qobject_to(QString, qobj);
+ g_assert(qstr);
+ cpus[idx].name = g_strdup(qstring_get_str(qstr));
+
+ qobj = qdict_get(minfo, "alias_of");
+ if (qobj) { /* old machines do not report aliases */
+ qstr = qobject_to(QString, qobj);
+ g_assert(qstr);
+ cpus[idx].alias_of = g_strdup(qstring_get_str(qstr));
+ } else {
+ cpus[idx].alias_of = NULL;
+ }
+
+ qobj = qdict_get(minfo, "deprecated");
+ qbool = qobject_to(QBool, qobj);
+ g_assert(qbool);
+ cpus[idx].deprecated = qbool_get_bool(qbool);
+ }
+
+ qtest_quit(qts);
+ qobject_unref(response);
+
+ silence_spawn_log = false;
+
+ memset(&cpus[idx], 0, sizeof(struct CpuInfo)); /* Terminating entry */
+ return cpus;
+}
+
+bool qtest_has_cpu(const char *cpu)
+{
+ struct CpuInfo *cpus;
+ int i;
+
+ cpus = qtest_get_cpus();
+
+ for (i = 0; cpus[i].name != NULL; i++) {
+ if (g_str_equal(cpu, cpus[i].name) ||
+ (cpus[i].alias_of && g_str_equal(cpu, cpus[i].alias_of))) {
+ return true;
+ }
+ }
+
+ return false;
+}
+
void qtest_cb_for_every_machine(void (*cb)(const char *machine),
bool skip_old_versioned)
{
diff --git a/tests/qtest/libqtest.h b/tests/qtest/libqtest.h
index 6e3d3525bf..c94b64f960 100644
--- a/tests/qtest/libqtest.h
+++ b/tests/qtest/libqtest.h
@@ -949,6 +949,14 @@ bool qtest_has_machine(const char *machine);
*/
bool qtest_has_machine_with_env(const char *var, const char *machine);
+/**
+ * qtest_has_cpu:
+ * @cpu: The cpu to look for
+ *
+ * Returns: true if the cpu is available in the target binary.
+ */
+bool qtest_has_cpu(const char *cpu);
+
/**
* qtest_has_device:
* @device: The device to look for
--
2.42.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 3/3] tests/qtest/x86: check for availability of older cpu models before running tests
2024-06-05 7:25 [PATCH 0/3] x86 cpu test refactoring Ani Sinha
2024-06-05 7:25 ` [PATCH v2 1/3] qtest/x86/numa-test: do not use the obsolete 'pentium' cpu Ani Sinha
2024-06-05 7:25 ` [PATCH 2/3] tests/qtest/libqtest: add qtest_has_cpu() api Ani Sinha
@ 2024-06-05 7:25 ` Ani Sinha
2024-06-05 8:42 ` Daniel P. Berrangé
2 siblings, 1 reply; 5+ messages in thread
From: Ani Sinha @ 2024-06-05 7:25 UTC (permalink / raw)
To: Michael S. Tsirkin, Marcel Apfelbaum, Thomas Huth, Laurent Vivier,
Paolo Bonzini
Cc: Ani Sinha, imammedo, qemu-devel
Some older cpu models like 486, athlon, pentium, penryn, phenom, core2duo etc
may not be available in all builds. Check for their availability in qemu before
running the corresponding tests.
The order of the tests has been altered so that all tests for similar checks
under a specific cpu is placed together.
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 | 214 +++++++++++++++++-----------
1 file changed, 127 insertions(+), 87 deletions(-)
diff --git a/tests/qtest/test-x86-cpuid-compat.c b/tests/qtest/test-x86-cpuid-compat.c
index 6a39454fce..054f9eae47 100644
--- a/tests/qtest/test-x86-cpuid-compat.c
+++ b/tests/qtest/test-x86-cpuid-compat.c
@@ -209,99 +209,135 @@ static void test_plus_minus(void)
int main(int argc, char **argv)
{
+ bool has_486, has_athlon, has_conroe;
+ bool has_core2duo, has_penryn, has_pentium, has_phenom;
+
g_test_init(&argc, &argv, NULL);
- g_test_add_func("/x86/cpuid/parsing-plus-minus/subprocess",
- test_plus_minus_subprocess);
- g_test_add_func("/x86/cpuid/parsing-plus-minus", test_plus_minus);
+ has_486 = qtest_has_cpu("486");
+ has_athlon = qtest_has_cpu("athlon");
+ has_conroe = qtest_has_cpu("Conroe");
+ has_core2duo = qtest_has_cpu("core2duo");
+ has_penryn = qtest_has_cpu("Penryn");
+ has_pentium = qtest_has_cpu("pentium");
+ has_phenom = qtest_has_cpu("phenom");
+
+ if (has_pentium) {
+ g_test_add_func("/x86/cpuid/parsing-plus-minus/subprocess",
+ test_plus_minus_subprocess);
+ g_test_add_func("/x86/cpuid/parsing-plus-minus", test_plus_minus);
+ }
/* Original level values for CPU models: */
- add_cpuid_test("x86/cpuid/phenom/level",
- "-cpu phenom", "level", 5);
- add_cpuid_test("x86/cpuid/Conroe/level",
- "-cpu Conroe", "level", 10);
+ if (has_486) {
+ add_cpuid_test("x86/cpuid/486/xlevel",
+ "-cpu 486", "xlevel", 0);
+ }
+ if (has_phenom) {
+ add_cpuid_test("x86/cpuid/phenom/level",
+ "-cpu phenom", "level", 5);
+ add_cpuid_test("x86/cpuid/phenom/xlevel",
+ "-cpu phenom", "xlevel", 0x8000001A);
+ }
+ if (has_athlon) {
+ add_cpuid_test("x86/cpuid/athlon/xlevel",
+ "-cpu athlon", "xlevel", 0x80000008);
+ }
+ if (has_core2duo) {
+ add_cpuid_test("x86/cpuid/core2duo/xlevel",
+ "-cpu core2duo", "xlevel", 0x80000008);
+ }
+ if (has_conroe) {
+ add_cpuid_test("x86/cpuid/Conroe/level",
+ "-cpu Conroe", "level", 10);
+ }
add_cpuid_test("x86/cpuid/SandyBridge/level",
"-cpu SandyBridge", "level", 0xd);
- add_cpuid_test("x86/cpuid/486/xlevel",
- "-cpu 486", "xlevel", 0);
- add_cpuid_test("x86/cpuid/core2duo/xlevel",
- "-cpu core2duo", "xlevel", 0x80000008);
- add_cpuid_test("x86/cpuid/phenom/xlevel",
- "-cpu phenom", "xlevel", 0x8000001A);
- add_cpuid_test("x86/cpuid/athlon/xlevel",
- "-cpu athlon", "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);
- /* CPUID[EAX=7,ECX=0].EBX: */
- add_cpuid_test("x86/cpuid/auto-level/phenom/fsgsbase",
- "-cpu phenom,fsgsbase=on", "level", 7);
- /* CPUID[EAX=7,ECX=0].ECX: */
- add_cpuid_test("x86/cpuid/auto-level/phenom/avx512vbmi",
- "-cpu phenom,avx512vbmi=on", "level", 7);
- /* CPUID[EAX=0xd,ECX=1].EAX: */
- add_cpuid_test("x86/cpuid/auto-level/phenom/xsaveopt",
- "-cpu phenom,xsaveopt=on", "level", 0xd);
- /* CPUID[8000_0001].EDX: */
- add_cpuid_test("x86/cpuid/auto-xlevel/486/3dnow",
- "-cpu 486,3dnow=on", "xlevel", 0x80000001);
- /* CPUID[8000_0001].ECX: */
- add_cpuid_test("x86/cpuid/auto-xlevel/486/sse4a",
- "-cpu 486,sse4a=on", "xlevel", 0x80000001);
- /* CPUID[8000_0007].EDX: */
- add_cpuid_test("x86/cpuid/auto-xlevel/486/invtsc",
- "-cpu 486,invtsc=on", "xlevel", 0x80000007);
- /* CPUID[8000_000A].EDX: */
- add_cpuid_test("x86/cpuid/auto-xlevel/486/npt",
- "-cpu 486,svm=on,npt=on", "xlevel", 0x8000000A);
- /* CPUID[C000_0001].EDX: */
- add_cpuid_test("x86/cpuid/auto-xlevel2/phenom/xstore",
- "-cpu phenom,xstore=on", "xlevel2", 0xC0000001);
- /* SVM needs CPUID[0x8000000A] */
- add_cpuid_test("x86/cpuid/auto-xlevel/athlon/svm",
- "-cpu athlon,svm=on", "xlevel", 0x8000000A);
-
+ if (has_486) {
+ /* CPUID[6].EAX: */
+ add_cpuid_test("x86/cpuid/auto-level/486/arat",
+ "-cpu 486,arat=on", "level", 6);
+ /* CPUID[8000_0001].EDX: */
+ add_cpuid_test("x86/cpuid/auto-xlevel/486/3dnow",
+ "-cpu 486,3dnow=on", "xlevel", 0x80000001);
+ /* CPUID[8000_0001].ECX: */
+ add_cpuid_test("x86/cpuid/auto-xlevel/486/sse4a",
+ "-cpu 486,sse4a=on", "xlevel", 0x80000001);
+ /* CPUID[8000_0007].EDX: */
+ add_cpuid_test("x86/cpuid/auto-xlevel/486/invtsc",
+ "-cpu 486,invtsc=on", "xlevel", 0x80000007);
+ /* CPUID[8000_000A].EDX: */
+ add_cpuid_test("x86/cpuid/auto-xlevel/486/npt",
+ "-cpu 486,svm=on,npt=on", "xlevel", 0x8000000A);
+ }
+ if (has_phenom) {
+ /* CPUID[EAX=7,ECX=0].EBX: */
+ add_cpuid_test("x86/cpuid/auto-level/phenom/fsgsbase",
+ "-cpu phenom,fsgsbase=on", "level", 7);
+ /* CPUID[EAX=7,ECX=0].ECX: */
+ add_cpuid_test("x86/cpuid/auto-level/phenom/avx512vbmi",
+ "-cpu phenom,avx512vbmi=on", "level", 7);
+ /* CPUID[EAX=0xd,ECX=1].EAX: */
+ add_cpuid_test("x86/cpuid/auto-level/phenom/xsaveopt",
+ "-cpu phenom,xsaveopt=on", "level", 0xd);
+ /* CPUID[C000_0001].EDX: */
+ add_cpuid_test("x86/cpuid/auto-xlevel2/phenom/xstore",
+ "-cpu phenom,xstore=on", "xlevel2", 0xC0000001);
+ }
+ if (has_athlon) {
+ /* SVM needs CPUID[0x8000000A] */
+ add_cpuid_test("x86/cpuid/auto-xlevel/athlon/svm",
+ "-cpu athlon,svm=on", "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);
/* 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);
- add_cpuid_test("x86/cpuid/auto-level/486/fixed/2",
- "-cpu 486,level=2,arat=on,fsgsbase=on,avx512vbmi=on,xsaveopt=on",
- "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);
+ if (has_486) {
+ add_cpuid_test("x86/cpuid/auto-level/486/fixed/0xF",
+ "-cpu 486,level=0xF,arat=on,fsgsbase=on,avx512vbmi=on,xsaveopt=on",
+ "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);
+ add_cpuid_test("x86/cpuid/auto-level/486/fixed/0",
+ "-cpu 486,level=0,arat=on,fsgsbase=on,avx512vbmi=on,xsaveopt=on",
+ "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);
+ if (has_phenom) {
+ add_cpuid_test("x86/cpuid/auto-xlevel/phenom/3dnow",
+ "-cpu phenom,3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
+ "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);
- 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);
- 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);
-
- /* 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);
+ if (has_486) {
+ 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);
+ 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);
+ 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);
+
+ /* 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);
+ }
+
/* Check compatibility of old machine-types that didn't
* auto-increase level/xlevel/xlevel2: */
- if (qtest_has_machine("pc-i440fx-2.7")) {
+ if (qtest_has_machine("pc-i440fx-2.7") && has_486) {
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);
@@ -317,7 +353,7 @@ int main(int argc, char **argv)
* and the compat code that sets default level shouldn't
* disable the auto-level=7 code:
*/
- if (qtest_has_machine("pc-i440fx-2.3")) {
+ if (qtest_has_machine("pc-i440fx-2.3") && has_penryn) {
add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-2.3/off",
"-machine pc-i440fx-2.3 -cpu Penryn",
"level", 4);
@@ -325,7 +361,7 @@ int main(int argc, char **argv)
"-machine pc-i440fx-2.3 -cpu Penryn,erms=on",
"level", 7);
}
- if (qtest_has_machine("pc-i440fx-2.9")) {
+ if (qtest_has_machine("pc-i440fx-2.9") && has_conroe) {
add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-2.9/off",
"-machine pc-i440fx-2.9 -cpu Conroe",
"level", 10);
@@ -354,18 +390,22 @@ int main(int argc, char **argv)
}
/* Test feature parsing */
- add_feature_test("x86/cpuid/features/plus",
- "-cpu 486,+arat",
- 6, 0, "EAX", 2, true);
- add_feature_test("x86/cpuid/features/minus",
- "-cpu pentium,-mmx",
- 1, 0, "EDX", 23, false);
- add_feature_test("x86/cpuid/features/on",
- "-cpu 486,arat=on",
- 6, 0, "EAX", 2, true);
- add_feature_test("x86/cpuid/features/off",
- "-cpu pentium,mmx=off",
- 1, 0, "EDX", 23, false);
+ if (has_486) {
+ add_feature_test("x86/cpuid/features/plus",
+ "-cpu 486,+arat",
+ 6, 0, "EAX", 2, true);
+ add_feature_test("x86/cpuid/features/on",
+ "-cpu 486,arat=on",
+ 6, 0, "EAX", 2, true);
+ }
+ if (has_pentium) {
+ add_feature_test("x86/cpuid/features/minus",
+ "-cpu pentium,-mmx",
+ 1, 0, "EDX", 23, false);
+ add_feature_test("x86/cpuid/features/off",
+ "-cpu pentium,mmx=off",
+ 1, 0, "EDX", 23, false);
+ }
add_feature_test("x86/cpuid/features/max-plus-invtsc",
"-cpu max,+invtsc",
0x80000007, 0, "EDX", 8, true);
--
2.42.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 3/3] tests/qtest/x86: check for availability of older cpu models before running tests
2024-06-05 7:25 ` [PATCH 3/3] tests/qtest/x86: check for availability of older cpu models before running tests Ani Sinha
@ 2024-06-05 8:42 ` Daniel P. Berrangé
0 siblings, 0 replies; 5+ messages in thread
From: Daniel P. Berrangé @ 2024-06-05 8:42 UTC (permalink / raw)
To: Ani Sinha
Cc: Michael S. Tsirkin, Marcel Apfelbaum, Thomas Huth, Laurent Vivier,
Paolo Bonzini, imammedo, qemu-devel
On Wed, Jun 05, 2024 at 12:55:11PM +0530, Ani Sinha wrote:
> Some older cpu models like 486, athlon, pentium, penryn, phenom, core2duo etc
> may not be available in all builds. Check for their availability in qemu before
> running the corresponding tests.
From an upstream POV this is very much not the case - all CPUs models
are unconditionally built in to QEMU.
This change is being driven RHEL's desire to only ship a small subset
of CPU models, dropping the legacy stuff that's only interesting for
emulation use cases with ancient OS.
>
> The order of the tests has been altered so that all tests for similar checks
> under a specific cpu is placed together.
>
> 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 | 214 +++++++++++++++++-----------
> 1 file changed, 127 insertions(+), 87 deletions(-)
>
> diff --git a/tests/qtest/test-x86-cpuid-compat.c b/tests/qtest/test-x86-cpuid-compat.c
> index 6a39454fce..054f9eae47 100644
> --- a/tests/qtest/test-x86-cpuid-compat.c
> +++ b/tests/qtest/test-x86-cpuid-compat.c
> @@ -209,99 +209,135 @@ static void test_plus_minus(void)
>
> int main(int argc, char **argv)
> {
> + bool has_486, has_athlon, has_conroe;
> + bool has_core2duo, has_penryn, has_pentium, has_phenom;
> +
> g_test_init(&argc, &argv, NULL);
>
> - g_test_add_func("/x86/cpuid/parsing-plus-minus/subprocess",
> - test_plus_minus_subprocess);
> - g_test_add_func("/x86/cpuid/parsing-plus-minus", test_plus_minus);
> + has_486 = qtest_has_cpu("486");
> + has_athlon = qtest_has_cpu("athlon");
> + has_conroe = qtest_has_cpu("Conroe");
> + has_core2duo = qtest_has_cpu("core2duo");
> + has_penryn = qtest_has_cpu("Penryn");
> + has_pentium = qtest_has_cpu("pentium");
> + has_phenom = qtest_has_cpu("phenom");
> +
> + if (has_pentium) {
> + g_test_add_func("/x86/cpuid/parsing-plus-minus/subprocess",
> + test_plus_minus_subprocess);
> + g_test_add_func("/x86/cpuid/parsing-plus-minus", test_plus_minus);
> + }
>
> /* Original level values for CPU models: */
> - add_cpuid_test("x86/cpuid/phenom/level",
> - "-cpu phenom", "level", 5);
> - add_cpuid_test("x86/cpuid/Conroe/level",
> - "-cpu Conroe", "level", 10);
> + if (has_486) {
> + add_cpuid_test("x86/cpuid/486/xlevel",
> + "-cpu 486", "xlevel", 0);
> + }
I think that modifying every test like this is a very cumbersome
way of doing it, as you're needing to hardcode a particular list
of CPUs to check for, and this list is not neccessarily complete.
Instead, IMHO the add_cpuid_test() method should be changed such
that instead of taking an ARGV string as its 2nd parameter, it
has a "cpu" and a "machine" parameter, with 'machine' being passed
NULL for most of the tests.
IOW, we should be calling
add_cpuid_test("x86/cpuid/486/xlevel", NULL, "486", "xlevel", 0);
Now the 'add_cpuid_test' method itself, can check the CPU name
that it receives, and turn itself into a no-op if the CPU is
missing. This avoids adding any conditionals here, and will
work correctly no matter what CPUs are present.
> + if (has_phenom) {
> + add_cpuid_test("x86/cpuid/phenom/level",
> + "-cpu phenom", "level", 5);
> + add_cpuid_test("x86/cpuid/phenom/xlevel",
> + "-cpu phenom", "xlevel", 0x8000001A);
> + }
> + if (has_athlon) {
> + add_cpuid_test("x86/cpuid/athlon/xlevel",
> + "-cpu athlon", "xlevel", 0x80000008);
> + }
> + if (has_core2duo) {
> + add_cpuid_test("x86/cpuid/core2duo/xlevel",
> + "-cpu core2duo", "xlevel", 0x80000008);
> + }
> + if (has_conroe) {
> + add_cpuid_test("x86/cpuid/Conroe/level",
> + "-cpu Conroe", "level", 10);
> + }
> add_cpuid_test("x86/cpuid/SandyBridge/level",
> "-cpu SandyBridge", "level", 0xd);
> - add_cpuid_test("x86/cpuid/486/xlevel",
> - "-cpu 486", "xlevel", 0);
> - add_cpuid_test("x86/cpuid/core2duo/xlevel",
> - "-cpu core2duo", "xlevel", 0x80000008);
> - add_cpuid_test("x86/cpuid/phenom/xlevel",
> - "-cpu phenom", "xlevel", 0x8000001A);
> - add_cpuid_test("x86/cpuid/athlon/xlevel",
> - "-cpu athlon", "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);
> - /* CPUID[EAX=7,ECX=0].EBX: */
> - add_cpuid_test("x86/cpuid/auto-level/phenom/fsgsbase",
> - "-cpu phenom,fsgsbase=on", "level", 7);
> - /* CPUID[EAX=7,ECX=0].ECX: */
> - add_cpuid_test("x86/cpuid/auto-level/phenom/avx512vbmi",
> - "-cpu phenom,avx512vbmi=on", "level", 7);
> - /* CPUID[EAX=0xd,ECX=1].EAX: */
> - add_cpuid_test("x86/cpuid/auto-level/phenom/xsaveopt",
> - "-cpu phenom,xsaveopt=on", "level", 0xd);
> - /* CPUID[8000_0001].EDX: */
> - add_cpuid_test("x86/cpuid/auto-xlevel/486/3dnow",
> - "-cpu 486,3dnow=on", "xlevel", 0x80000001);
> - /* CPUID[8000_0001].ECX: */
> - add_cpuid_test("x86/cpuid/auto-xlevel/486/sse4a",
> - "-cpu 486,sse4a=on", "xlevel", 0x80000001);
> - /* CPUID[8000_0007].EDX: */
> - add_cpuid_test("x86/cpuid/auto-xlevel/486/invtsc",
> - "-cpu 486,invtsc=on", "xlevel", 0x80000007);
> - /* CPUID[8000_000A].EDX: */
> - add_cpuid_test("x86/cpuid/auto-xlevel/486/npt",
> - "-cpu 486,svm=on,npt=on", "xlevel", 0x8000000A);
> - /* CPUID[C000_0001].EDX: */
> - add_cpuid_test("x86/cpuid/auto-xlevel2/phenom/xstore",
> - "-cpu phenom,xstore=on", "xlevel2", 0xC0000001);
> - /* SVM needs CPUID[0x8000000A] */
> - add_cpuid_test("x86/cpuid/auto-xlevel/athlon/svm",
> - "-cpu athlon,svm=on", "xlevel", 0x8000000A);
> -
> + if (has_486) {
> + /* CPUID[6].EAX: */
> + add_cpuid_test("x86/cpuid/auto-level/486/arat",
> + "-cpu 486,arat=on", "level", 6);
> + /* CPUID[8000_0001].EDX: */
> + add_cpuid_test("x86/cpuid/auto-xlevel/486/3dnow",
> + "-cpu 486,3dnow=on", "xlevel", 0x80000001);
> + /* CPUID[8000_0001].ECX: */
> + add_cpuid_test("x86/cpuid/auto-xlevel/486/sse4a",
> + "-cpu 486,sse4a=on", "xlevel", 0x80000001);
> + /* CPUID[8000_0007].EDX: */
> + add_cpuid_test("x86/cpuid/auto-xlevel/486/invtsc",
> + "-cpu 486,invtsc=on", "xlevel", 0x80000007);
> + /* CPUID[8000_000A].EDX: */
> + add_cpuid_test("x86/cpuid/auto-xlevel/486/npt",
> + "-cpu 486,svm=on,npt=on", "xlevel", 0x8000000A);
> + }
> + if (has_phenom) {
> + /* CPUID[EAX=7,ECX=0].EBX: */
> + add_cpuid_test("x86/cpuid/auto-level/phenom/fsgsbase",
> + "-cpu phenom,fsgsbase=on", "level", 7);
> + /* CPUID[EAX=7,ECX=0].ECX: */
> + add_cpuid_test("x86/cpuid/auto-level/phenom/avx512vbmi",
> + "-cpu phenom,avx512vbmi=on", "level", 7);
> + /* CPUID[EAX=0xd,ECX=1].EAX: */
> + add_cpuid_test("x86/cpuid/auto-level/phenom/xsaveopt",
> + "-cpu phenom,xsaveopt=on", "level", 0xd);
> + /* CPUID[C000_0001].EDX: */
> + add_cpuid_test("x86/cpuid/auto-xlevel2/phenom/xstore",
> + "-cpu phenom,xstore=on", "xlevel2", 0xC0000001);
> + }
> + if (has_athlon) {
> + /* SVM needs CPUID[0x8000000A] */
> + add_cpuid_test("x86/cpuid/auto-xlevel/athlon/svm",
> + "-cpu athlon,svm=on", "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);
> /* 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);
> - add_cpuid_test("x86/cpuid/auto-level/486/fixed/2",
> - "-cpu 486,level=2,arat=on,fsgsbase=on,avx512vbmi=on,xsaveopt=on",
> - "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);
> + if (has_486) {
> + add_cpuid_test("x86/cpuid/auto-level/486/fixed/0xF",
> + "-cpu 486,level=0xF,arat=on,fsgsbase=on,avx512vbmi=on,xsaveopt=on",
> + "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);
> + add_cpuid_test("x86/cpuid/auto-level/486/fixed/0",
> + "-cpu 486,level=0,arat=on,fsgsbase=on,avx512vbmi=on,xsaveopt=on",
> + "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);
> + if (has_phenom) {
> + add_cpuid_test("x86/cpuid/auto-xlevel/phenom/3dnow",
> + "-cpu phenom,3dnow=on,sse4a=on,invtsc=on,npt=on,svm=on",
> + "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);
> - 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);
> - 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);
> -
> - /* 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);
> + if (has_486) {
> + 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);
> + 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);
> + 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);
> +
> + /* 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);
> + }
> +
>
> /* Check compatibility of old machine-types that didn't
> * auto-increase level/xlevel/xlevel2: */
> - if (qtest_has_machine("pc-i440fx-2.7")) {
> + if (qtest_has_machine("pc-i440fx-2.7") && has_486) {
> 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);
> @@ -317,7 +353,7 @@ int main(int argc, char **argv)
> * and the compat code that sets default level shouldn't
> * disable the auto-level=7 code:
> */
> - if (qtest_has_machine("pc-i440fx-2.3")) {
> + if (qtest_has_machine("pc-i440fx-2.3") && has_penryn) {
> add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-2.3/off",
> "-machine pc-i440fx-2.3 -cpu Penryn",
> "level", 4);
> @@ -325,7 +361,7 @@ int main(int argc, char **argv)
> "-machine pc-i440fx-2.3 -cpu Penryn,erms=on",
> "level", 7);
> }
> - if (qtest_has_machine("pc-i440fx-2.9")) {
> + if (qtest_has_machine("pc-i440fx-2.9") && has_conroe) {
> add_cpuid_test("x86/cpuid/auto-level7/pc-i440fx-2.9/off",
> "-machine pc-i440fx-2.9 -cpu Conroe",
> "level", 10);
> @@ -354,18 +390,22 @@ int main(int argc, char **argv)
> }
>
> /* Test feature parsing */
> - add_feature_test("x86/cpuid/features/plus",
> - "-cpu 486,+arat",
> - 6, 0, "EAX", 2, true);
> - add_feature_test("x86/cpuid/features/minus",
> - "-cpu pentium,-mmx",
> - 1, 0, "EDX", 23, false);
> - add_feature_test("x86/cpuid/features/on",
> - "-cpu 486,arat=on",
> - 6, 0, "EAX", 2, true);
> - add_feature_test("x86/cpuid/features/off",
> - "-cpu pentium,mmx=off",
> - 1, 0, "EDX", 23, false);
> + if (has_486) {
> + add_feature_test("x86/cpuid/features/plus",
> + "-cpu 486,+arat",
> + 6, 0, "EAX", 2, true);
> + add_feature_test("x86/cpuid/features/on",
> + "-cpu 486,arat=on",
> + 6, 0, "EAX", 2, true);
> + }
> + if (has_pentium) {
> + add_feature_test("x86/cpuid/features/minus",
> + "-cpu pentium,-mmx",
> + 1, 0, "EDX", 23, false);
> + add_feature_test("x86/cpuid/features/off",
> + "-cpu pentium,mmx=off",
> + 1, 0, "EDX", 23, false);
> + }
> add_feature_test("x86/cpuid/features/max-plus-invtsc",
> "-cpu max,+invtsc",
> 0x80000007, 0, "EDX", 8, true);
> --
> 2.42.0
>
>
With regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-06-05 8:44 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-06-05 7:25 [PATCH 0/3] x86 cpu test refactoring Ani Sinha
2024-06-05 7:25 ` [PATCH v2 1/3] qtest/x86/numa-test: do not use the obsolete 'pentium' cpu Ani Sinha
2024-06-05 7:25 ` [PATCH 2/3] tests/qtest/libqtest: add qtest_has_cpu() api Ani Sinha
2024-06-05 7:25 ` [PATCH 3/3] tests/qtest/x86: check for availability of older cpu models before running tests Ani Sinha
2024-06-05 8:42 ` Daniel P. Berrangé
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).