Add test code to ensure features are enabled/disabled correctly in the
command-line. The test case use the "feature-words" and
"filtered-features" properties to check if the features were
enabled/disabled correctly.
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
Changes v1 -> v2:
* Remove explicit "accel=" option to avoid triggering warnings
on "make check"
* Use qdict_get_*() helpers to make code shorter
* Rename input_eax, input_ecx to in_eax, in_ecx to make
lines fit in the coding style width limit
* v1 was submitted as part of the series:
Subject: [PATCH 0/4] x86: Support "-cpu feature=force"
* Coding style: split lines
* Style changes on code comments
---
tests/test-x86-cpuid-compat.c | 111 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 111 insertions(+)
diff --git a/tests/test-x86-cpuid-compat.c b/tests/test-x86-cpuid-compat.c
index 79a2e69a28..6c71e46391 100644
--- a/tests/test-x86-cpuid-compat.c
+++ b/tests/test-x86-cpuid-compat.c
@@ -1,6 +1,7 @@
#include "qemu/osdep.h"
#include "qemu-common.h"
#include "qapi/qmp/qlist.h"
+#include "qapi/qmp/qstring.h"
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qint.h"
#include "qapi/qmp/qbool.h"
@@ -78,6 +79,90 @@ static void add_cpuid_test(const char *name, const char *cmdline,
qtest_add_data_func(name, args, test_cpuid_prop);
}
+
+/* Parameters to a add_feature_test() test case */
+typedef struct FeatureTestArgs {
+ /* cmdline to start QEMU */
+ const char *cmdline;
+ /*
+ * cpuid-input-eax and cpuid-input-ecx values to look for,
+ * in "feature-words" and "filtered-features" properties.
+ */
+ uint32_t in_eax, in_ecx;
+ /* The register name to look for, in the X86CPUFeatureWordInfo array */
+ const char *reg;
+ /* The bit to check in X86CPUFeatureWordInfo.features */
+ int bitnr;
+ /* The expected value for the bit in (X86CPUFeatureWordInfo.features) */
+ bool expected_value;
+} FeatureTestArgs;
+
+/* Get the value for a feature word in a X86CPUFeatureWordInfo list */
+static uint32_t get_feature_word(QList *features, uint32_t eax, uint32_t ecx,
+ const char *reg)
+{
+ const QListEntry *e;
+
+ for (e = qlist_first(features); e; e = qlist_next(e)) {
+ QDict *w = qobject_to_qdict(qlist_entry_obj(e));
+ const char *rreg = qdict_get_str(w, "cpuid-register");
+ uint32_t reax = qdict_get_int(w, "cpuid-input-eax");
+ bool has_ecx = qdict_haskey(w, "cpuid-input-ecx");
+ uint32_t recx = 0;
+
+ if (has_ecx) {
+ recx = qdict_get_int(w, "cpuid-input-ecx");
+ }
+ if (eax == reax && (!has_ecx || ecx == recx) && !strcmp(rreg, reg)) {
+ return qint_get_int(qobject_to_qint(qdict_get(w, "features")));
+ }
+ }
+ return 0;
+}
+
+static void test_feature_flag(const void *data)
+{
+ const FeatureTestArgs *args = data;
+ char *path;
+ QList *present, *filtered;
+ uint32_t value;
+
+ qtest_start(args->cmdline);
+ path = get_cpu0_qom_path();
+ present = qobject_to_qlist(qom_get(path, "feature-words"));
+ filtered = qobject_to_qlist(qom_get(path, "filtered-features"));
+ value = get_feature_word(present, args->in_eax, args->in_ecx, args->reg);
+ value |= get_feature_word(filtered, args->in_eax, args->in_ecx, args->reg);
+ qtest_end();
+
+ g_assert(!!(value & (1U << args->bitnr)) == args->expected_value);
+
+ QDECREF(present);
+ QDECREF(filtered);
+ g_free(path);
+}
+
+/*
+ * Add test case to ensure that a given feature flag is set in
+ * 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)
+{
+ FeatureTestArgs *args = g_new0(FeatureTestArgs, 1);
+ args->cmdline = cmdline;
+ args->in_eax = eax;
+ args->in_ecx = ecx;
+ args->reg = reg;
+ args->bitnr = bitnr;
+ args->expected_value = expected_value;
+ qtest_add_data_func(name, args, test_feature_flag);
+ return args;
+}
+
#ifdef CONFIG_HAS_GLIB_SUBPROCESS_TESTS
static void test_plus_minus_subprocess(void)
{
@@ -229,5 +314,31 @@ int main(int argc, char **argv)
"-machine pc-i440fx-2.7 -cpu 486,+xstore",
"xlevel2", 0);
+ /* 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);
+ add_feature_test("x86/cpuid/features/max-plus-invtsc",
+ "-cpu max,+invtsc",
+ 0x80000007, 0, "EDX", 8, true);
+ add_feature_test("x86/cpuid/features/max-invtsc-on",
+ "-cpu max,invtsc=on",
+ 0x80000007, 0, "EDX", 8, true);
+ add_feature_test("x86/cpuid/features/max-minus-mmx",
+ "-cpu max,-mmx",
+ 1, 0, "EDX", 23, false);
+ add_feature_test("x86/cpuid/features/max-invtsc-on,mmx=off",
+ "-cpu max,mmx=off",
+ 1, 0, "EDX", 23, false);
+
return g_test_run();
}
--
2.11.0.259.g40922b1
On Mon, 8 May 2017 15:32:05 -0300
Eduardo Habkost <ehabkost@redhat.com> wrote:
> Add test code to ensure features are enabled/disabled correctly in the
> command-line. The test case use the "feature-words" and
> "filtered-features" properties to check if the features were
> enabled/disabled correctly.
>
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
> Changes v1 -> v2:
> * Remove explicit "accel=" option to avoid triggering warnings
> on "make check"
> * Use qdict_get_*() helpers to make code shorter
> * Rename input_eax, input_ecx to in_eax, in_ecx to make
> lines fit in the coding style width limit
> * v1 was submitted as part of the series:
> Subject: [PATCH 0/4] x86: Support "-cpu feature=force"
> * Coding style: split lines
> * Style changes on code comments
> ---
> tests/test-x86-cpuid-compat.c | 111 ++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 111 insertions(+)
>
> diff --git a/tests/test-x86-cpuid-compat.c b/tests/test-x86-cpuid-compat.c
[...]
> + add_feature_test("x86/cpuid/features/max-plus-invtsc",
> + "-cpu max,+invtsc",
> + 0x80000007, 0, "EDX", 8, true);
> + add_feature_test("x86/cpuid/features/max-invtsc-on",
> + "-cpu max,invtsc=on",
> + 0x80000007, 0, "EDX", 8, true);
> + add_feature_test("x86/cpuid/features/max-minus-mmx",
> + "-cpu max,-mmx",
> + 1, 0, "EDX", 23, false);
> + add_feature_test("x86/cpuid/features/max-invtsc-on,mmx=off",
> + "-cpu max,mmx=off",
> + 1, 0, "EDX", 23, false);
Why do you add 'max' variants in addition to 486/pentium?
> return g_test_run();
> }
On Thu, May 11, 2017 at 04:00:00PM +0200, Igor Mammedov wrote:
> On Mon, 8 May 2017 15:32:05 -0300
> Eduardo Habkost <ehabkost@redhat.com> wrote:
>
> > Add test code to ensure features are enabled/disabled correctly in the
> > command-line. The test case use the "feature-words" and
> > "filtered-features" properties to check if the features were
> > enabled/disabled correctly.
> >
> > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> > ---
> > Changes v1 -> v2:
> > * Remove explicit "accel=" option to avoid triggering warnings
> > on "make check"
> > * Use qdict_get_*() helpers to make code shorter
> > * Rename input_eax, input_ecx to in_eax, in_ecx to make
> > lines fit in the coding style width limit
> > * v1 was submitted as part of the series:
> > Subject: [PATCH 0/4] x86: Support "-cpu feature=force"
> > * Coding style: split lines
> > * Style changes on code comments
> > ---
> > tests/test-x86-cpuid-compat.c | 111 ++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 111 insertions(+)
> >
> > diff --git a/tests/test-x86-cpuid-compat.c b/tests/test-x86-cpuid-compat.c
> [...]
> > + add_feature_test("x86/cpuid/features/max-plus-invtsc",
> > + "-cpu max,+invtsc",
> > + 0x80000007, 0, "EDX", 8, true);
> > + add_feature_test("x86/cpuid/features/max-invtsc-on",
> > + "-cpu max,invtsc=on",
> > + 0x80000007, 0, "EDX", 8, true);
> > + add_feature_test("x86/cpuid/features/max-minus-mmx",
> > + "-cpu max,-mmx",
> > + 1, 0, "EDX", 23, false);
> > + add_feature_test("x86/cpuid/features/max-invtsc-on,mmx=off",
> > + "-cpu max,mmx=off",
> > + 1, 0, "EDX", 23, false);
> Why do you add 'max' variants in addition to 486/pentium?
Because we already had bugs in the host/max logic in the past.
See commit d4a606b38b5d4b3689b86cc1575908e82179ecfb for example.
Let's enumerate each test case and the reason for them:
+ add_feature_test("x86/cpuid/features/plus",
+ "-cpu 486,+arat",
+ 6, 0, "EAX", 2, true);
Here I need a CPU model with level < 6. 486. pentium, pentium2,
pentium3 would work, and I chose 486. Conroe and newer won't
work, because they already have level=10.
+ add_feature_test("x86/cpuid/features/minus",
+ "-cpu pentium,-mmx",
+ 1, 0, "EDX", 23, false);
Here I need a CPU model that has at least one feature I can
disable. I decided to test "-mmx", and use the oldest CPU model
that had MMX enabled (to increase the likelihood it is runnable
on the host).
+ 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);
Same as above, but testing feat=on|off syntax.
+ add_feature_test("x86/cpuid/features/max-plus-invtsc",
+ "-cpu max,+invtsc",
+ 0x80000007, 0, "EDX", 8, true);
+ add_feature_test("x86/cpuid/features/max-invtsc-on",
+ "-cpu max,invtsc=on",
+ 0x80000007, 0, "EDX", 8, true);
This is checking for the bugs fixed by commit
46c032f3afcc05a0123914609f1003906ba63fda and commit
d4a606b38b5d4b3689b86cc1575908e82179ecfb.
+ add_feature_test("x86/cpuid/features/max-minus-mmx",
+ "-cpu max,-mmx",
+ 1, 0, "EDX", 23, false);
+ add_feature_test("x86/cpuid/features/max-invtsc-on,mmx=off",
+ "-cpu max,mmx=off",
+ 1, 0, "EDX", 23, false);
This is checking the bug fixed by commit
d4a606b38b5d4b3689b86cc1575908e82179ecfb, but when disabling
features.
--
Eduardo
On Mon, 8 May 2017 15:32:05 -0300 Eduardo Habkost <ehabkost@redhat.com> wrote: > Add test code to ensure features are enabled/disabled correctly in the > command-line. The test case use the "feature-words" and > "filtered-features" properties to check if the features were > enabled/disabled correctly. > > Signed-off-by: Eduardo Habkost <ehabkost@redhat.com> Reviewed-by: Igor Mammedov <imammedo@redhat.com> [...]
© 2016 - 2024 Red Hat, Inc.