| Message ID | 20230113140419.4013-22-farosas@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | target/arm: Allow CONFIG_TCG=n builds | expand |
On 13/01/2023 15.04, Fabiano Rosas wrote: > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > tests/qtest/arm-cpu-features.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c > index 4be1415823..9a052e41fc 100644 > --- a/tests/qtest/arm-cpu-features.c > +++ b/tests/qtest/arm-cpu-features.c > @@ -217,6 +217,15 @@ static void assert_bad_props(QTestState *qts, const char *cpu_type) > qobject_unref(resp); > } > > +static bool tcg_disabled(void) > +{ > +#ifndef CONFIG_TCG > + return true; > +#else > + return false; > +#endif > +} > + > static uint64_t resp_get_sve_vls(QDict *resp) > { > QDict *props; > @@ -338,6 +347,11 @@ static void sve_tests_sve_max_vq_8(const void *data) > { > QTestState *qts; > > + if (tcg_disabled()) { > + g_test_skip("TCG support disabled in this build"); > + return; > + } Could you please use qtest_has_accel("tcg") instead? ... that's what we use in other spots in the qtests for checking for valid accelerators already. Thanks, Thomas
Thomas Huth <thuth@redhat.com> writes: > On 13/01/2023 15.04, Fabiano Rosas wrote: >> Signed-off-by: Fabiano Rosas <farosas@suse.de> >> --- >> tests/qtest/arm-cpu-features.c | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c >> index 4be1415823..9a052e41fc 100644 >> --- a/tests/qtest/arm-cpu-features.c >> +++ b/tests/qtest/arm-cpu-features.c >> @@ -217,6 +217,15 @@ static void assert_bad_props(QTestState *qts, const char *cpu_type) >> qobject_unref(resp); >> } >> >> +static bool tcg_disabled(void) >> +{ >> +#ifndef CONFIG_TCG >> + return true; >> +#else >> + return false; >> +#endif >> +} >> + >> static uint64_t resp_get_sve_vls(QDict *resp) >> { >> QDict *props; >> @@ -338,6 +347,11 @@ static void sve_tests_sve_max_vq_8(const void *data) >> { >> QTestState *qts; >> >> + if (tcg_disabled()) { >> + g_test_skip("TCG support disabled in this build"); >> + return; >> + } > > Could you please use qtest_has_accel("tcg") instead? ... that's what we use > in other spots in the qtests for checking for valid accelerators already. Ah, that's much better, thanks!
On 1/13/23 06:04, Fabiano Rosas wrote: > @@ -373,6 +387,11 @@ static void sve_tests_sve_off(const void *data) > { > QTestState *qts; > > + if (tcg_disabled()) { > + g_test_skip("TCG support is disabled in this build"); > + return; > + } > + > qts = qtest_init(MACHINE "-cpu max,sve=off"); > > /* SVE is off, so the map should be empty. */ This should work with kvm as well. It should always be ok to turn off an unsupported feature. r~
Richard Henderson <richard.henderson@linaro.org> writes: > On 1/13/23 06:04, Fabiano Rosas wrote: >> @@ -373,6 +387,11 @@ static void sve_tests_sve_off(const void *data) >> { >> QTestState *qts; >> >> + if (tcg_disabled()) { >> + g_test_skip("TCG support is disabled in this build"); >> + return; >> + } >> + >> qts = qtest_init(MACHINE "-cpu max,sve=off"); >> >> /* SVE is off, so the map should be empty. */ > > This should work with kvm as well. > It should always be ok to turn off an unsupported feature. This test forces -accel tcg. There's another one that uses -accel kvm: sve_tests_sve_off_kvm
On Fri, Jan 13 2023, Fabiano Rosas <farosas@suse.de> wrote: > Signed-off-by: Fabiano Rosas <farosas@suse.de> > --- > tests/qtest/arm-cpu-features.c | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c > index 4be1415823..9a052e41fc 100644 > --- a/tests/qtest/arm-cpu-features.c > +++ b/tests/qtest/arm-cpu-features.c > @@ -217,6 +217,15 @@ static void assert_bad_props(QTestState *qts, const char *cpu_type) > qobject_unref(resp); > } > > +static bool tcg_disabled(void) > +{ > +#ifndef CONFIG_TCG > + return true; > +#else > + return false; > +#endif > +} > + > static uint64_t resp_get_sve_vls(QDict *resp) > { > QDict *props; > @@ -338,6 +347,11 @@ static void sve_tests_sve_max_vq_8(const void *data) > { > QTestState *qts; > > + if (tcg_disabled()) { > + g_test_skip("TCG support disabled in this build"); > + return; > + } > + > qts = qtest_init(MACHINE "-cpu max,sve-max-vq=8"); > > assert_sve_vls(qts, "max", BIT_ULL(8) - 1, NULL); > @@ -373,6 +387,11 @@ static void sve_tests_sve_off(const void *data) > { > QTestState *qts; > > + if (tcg_disabled()) { > + g_test_skip("TCG support is disabled in this build"); > + return; > + } > + > qts = qtest_init(MACHINE "-cpu max,sve=off"); > > /* SVE is off, so the map should be empty. */ I'm wondering whether the invocation of tcg and kvm test cases should be reorganized a bit. Currently, we have test cases that use MACHINE (tcg), and test cases that use MACHINE_KVM (kvm with a fallback to tcg). MACHINE_KVM is used either for aarch64 && kvm (test_query_cpu_model_expansion_kvm, which tests behaviour specific to kvm), or for aarch64 (sve_tests_sve_off_kvm, which tests behaviour that is the same for both kvm and tcg, and therefore tests tcg twice if kvm is not available.) So, should we - drop "-accel tcg" from MACHINE_KVM, - call sve_tests_sve_off_kvm only if kvm is available, and - call the functions you skip here conditionally on tcg being available instead? (not sure whether not calling should be preferred to skipping in general) > @@ -429,6 +448,11 @@ static void test_query_cpu_model_expansion(const void *data) > { > QTestState *qts; > > + if (tcg_disabled()) { > + g_test_skip("TCG support disabled in this build"); > + return; > + } > + > qts = qtest_init(MACHINE "-cpu max"); > > /* Test common query-cpu-model-expansion input validation */
Cornelia Huck <cohuck@redhat.com> writes: > On Fri, Jan 13 2023, Fabiano Rosas <farosas@suse.de> wrote: > >> Signed-off-by: Fabiano Rosas <farosas@suse.de> >> --- >> tests/qtest/arm-cpu-features.c | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c >> index 4be1415823..9a052e41fc 100644 >> --- a/tests/qtest/arm-cpu-features.c >> +++ b/tests/qtest/arm-cpu-features.c >> @@ -217,6 +217,15 @@ static void assert_bad_props(QTestState *qts, const char *cpu_type) >> qobject_unref(resp); >> } >> >> +static bool tcg_disabled(void) >> +{ >> +#ifndef CONFIG_TCG >> + return true; >> +#else >> + return false; >> +#endif >> +} >> + >> static uint64_t resp_get_sve_vls(QDict *resp) >> { >> QDict *props; >> @@ -338,6 +347,11 @@ static void sve_tests_sve_max_vq_8(const void *data) >> { >> QTestState *qts; >> >> + if (tcg_disabled()) { >> + g_test_skip("TCG support disabled in this build"); >> + return; >> + } >> + >> qts = qtest_init(MACHINE "-cpu max,sve-max-vq=8"); >> >> assert_sve_vls(qts, "max", BIT_ULL(8) - 1, NULL); >> @@ -373,6 +387,11 @@ static void sve_tests_sve_off(const void *data) >> { >> QTestState *qts; >> >> + if (tcg_disabled()) { >> + g_test_skip("TCG support is disabled in this build"); >> + return; >> + } >> + >> qts = qtest_init(MACHINE "-cpu max,sve=off"); >> >> /* SVE is off, so the map should be empty. */ > > I'm wondering whether the invocation of tcg and kvm test cases should be > reorganized a bit. Currently, we have test cases that use MACHINE (tcg), > and test cases that use MACHINE_KVM (kvm with a fallback to > tcg). MACHINE_KVM is used either for aarch64 && kvm > (test_query_cpu_model_expansion_kvm, which tests behaviour specific to > kvm), or for aarch64 (sve_tests_sve_off_kvm, which tests behaviour that > is the same for both kvm and tcg, and therefore tests tcg twice if kvm > is not available.) > > So, should we > - drop "-accel tcg" from MACHINE_KVM, > - call sve_tests_sve_off_kvm only if kvm is available, and > - call the functions you skip here conditionally on tcg being available > instead? (not sure whether not calling should be preferred to skipping > in general) I did the last two already for the next version. Good point on dropping "-accel tcg", I'll do that as well.
diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c index 4be1415823..9a052e41fc 100644 --- a/tests/qtest/arm-cpu-features.c +++ b/tests/qtest/arm-cpu-features.c @@ -217,6 +217,15 @@ static void assert_bad_props(QTestState *qts, const char *cpu_type) qobject_unref(resp); } +static bool tcg_disabled(void) +{ +#ifndef CONFIG_TCG + return true; +#else + return false; +#endif +} + static uint64_t resp_get_sve_vls(QDict *resp) { QDict *props; @@ -338,6 +347,11 @@ static void sve_tests_sve_max_vq_8(const void *data) { QTestState *qts; + if (tcg_disabled()) { + g_test_skip("TCG support disabled in this build"); + return; + } + qts = qtest_init(MACHINE "-cpu max,sve-max-vq=8"); assert_sve_vls(qts, "max", BIT_ULL(8) - 1, NULL); @@ -373,6 +387,11 @@ static void sve_tests_sve_off(const void *data) { QTestState *qts; + if (tcg_disabled()) { + g_test_skip("TCG support is disabled in this build"); + return; + } + qts = qtest_init(MACHINE "-cpu max,sve=off"); /* SVE is off, so the map should be empty. */ @@ -429,6 +448,11 @@ static void test_query_cpu_model_expansion(const void *data) { QTestState *qts; + if (tcg_disabled()) { + g_test_skip("TCG support disabled in this build"); + return; + } + qts = qtest_init(MACHINE "-cpu max"); /* Test common query-cpu-model-expansion input validation */
Signed-off-by: Fabiano Rosas <farosas@suse.de> --- tests/qtest/arm-cpu-features.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)