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. */
@@ -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 */
-- 
2.35.3

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.

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 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!