[PATCH v16 06/99] qtest/arm-cpu-features: Remove TCG fallback to KVM specific tests

Alex Bennée posted 99 patches 4 years, 8 months ago
[PATCH v16 06/99] qtest/arm-cpu-features: Remove TCG fallback to KVM specific tests
Posted by Alex Bennée 4 years, 8 months ago
From: Philippe Mathieu-Daudé <philmd@redhat.com>

sve_tests_sve_off_kvm() and test_query_cpu_model_expansion_kvm()
tests are now only being run if KVM is available. Drop the TCG
fallback.

Suggested-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Message-Id: <20210505125806.1263441-7-philmd@redhat.com>
---
 tests/qtest/arm-cpu-features.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
index 66300c3bc2..b1d406542f 100644
--- a/tests/qtest/arm-cpu-features.c
+++ b/tests/qtest/arm-cpu-features.c
@@ -21,7 +21,7 @@
 #define SVE_MAX_VQ 16
 
 #define MACHINE     "-machine virt,gic-version=max -accel tcg "
-#define MACHINE_KVM "-machine virt,gic-version=max -accel kvm -accel tcg "
+#define MACHINE_KVM "-machine virt,gic-version=max -accel kvm "
 #define QUERY_HEAD  "{ 'execute': 'query-cpu-model-expansion', " \
                     "  'arguments': { 'type': 'full', "
 #define QUERY_TAIL  "}}"
-- 
2.20.1


Re: [PATCH v16 06/99] qtest/arm-cpu-features: Remove TCG fallback to KVM specific tests
Posted by Richard Henderson 4 years, 8 months ago
On 6/4/21 8:51 AM, Alex Bennée wrote:
> From: Philippe Mathieu-Daudé<philmd@redhat.com>
> 
> sve_tests_sve_off_kvm() and test_query_cpu_model_expansion_kvm()
> tests are now only being run if KVM is available. Drop the TCG
> fallback.
> 
> Suggested-by: Andrew Jones<drjones@redhat.com>
> Reviewed-by: Andrew Jones<drjones@redhat.com>
> Reviewed-by: Alex Bennée<alex.bennee@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé<philmd@redhat.com>
> Signed-off-by: Alex Bennée<alex.bennee@linaro.org>
> Message-Id:<20210505125806.1263441-7-philmd@redhat.com>
> ---
>   tests/qtest/arm-cpu-features.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH v16 06/99] qtest/arm-cpu-features: Remove TCG fallback to KVM specific tests
Posted by Thomas Huth 4 years, 8 months ago
On 04/06/2021 17.51, Alex Bennée wrote:
> From: Philippe Mathieu-Daudé <philmd@redhat.com>
> 
> sve_tests_sve_off_kvm() and test_query_cpu_model_expansion_kvm()
> tests are now only being run if KVM is available. Drop the TCG
> fallback.
> 
> Suggested-by: Andrew Jones <drjones@redhat.com>
> Reviewed-by: Andrew Jones <drjones@redhat.com>
> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20210505125806.1263441-7-philmd@redhat.com>
> ---
>   tests/qtest/arm-cpu-features.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/qtest/arm-cpu-features.c b/tests/qtest/arm-cpu-features.c
> index 66300c3bc2..b1d406542f 100644
> --- a/tests/qtest/arm-cpu-features.c
> +++ b/tests/qtest/arm-cpu-features.c
> @@ -21,7 +21,7 @@
>   #define SVE_MAX_VQ 16
>   
>   #define MACHINE     "-machine virt,gic-version=max -accel tcg "
> -#define MACHINE_KVM "-machine virt,gic-version=max -accel kvm -accel tcg "
> +#define MACHINE_KVM "-machine virt,gic-version=max -accel kvm "

Same comment as with patch 04/99: I think this is wrong. You're mixing up 
whether an accelerator has been built into the binary with the fact whether 
an accelerator is available and usable. There are plenty of cases where e.g. 
kvm is built into the binary but not usable during runtime, e.g. because:
1) The kernel does not support it
2) The current host CPU does not support it
3) There are problems with the permission to /dev/kvm
etc.

I think we either need the fallback mechanism to tcg or you need to properly 
check whether KVM is usable, too.

  Thomas


Re: [PATCH v16 06/99] qtest/arm-cpu-features: Remove TCG fallback to KVM specific tests
Posted by Philippe Mathieu-Daudé 4 years, 8 months ago
On 6/7/21 3:28 PM, Thomas Huth wrote:
> On 04/06/2021 17.51, Alex Bennée wrote:
>> From: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
>> sve_tests_sve_off_kvm() and test_query_cpu_model_expansion_kvm()
>> tests are now only being run if KVM is available. Drop the TCG
>> fallback.
>>
>> Suggested-by: Andrew Jones <drjones@redhat.com>
>> Reviewed-by: Andrew Jones <drjones@redhat.com>
>> Reviewed-by: Alex Bennée <alex.bennee@linaro.org>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> Message-Id: <20210505125806.1263441-7-philmd@redhat.com>
>> ---
>>   tests/qtest/arm-cpu-features.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/tests/qtest/arm-cpu-features.c
>> b/tests/qtest/arm-cpu-features.c
>> index 66300c3bc2..b1d406542f 100644
>> --- a/tests/qtest/arm-cpu-features.c
>> +++ b/tests/qtest/arm-cpu-features.c
>> @@ -21,7 +21,7 @@
>>   #define SVE_MAX_VQ 16
>>     #define MACHINE     "-machine virt,gic-version=max -accel tcg "
>> -#define MACHINE_KVM "-machine virt,gic-version=max -accel kvm -accel
>> tcg "
>> +#define MACHINE_KVM "-machine virt,gic-version=max -accel kvm "
> 
> Same comment as with patch 04/99: I think this is wrong. You're mixing
> up whether an accelerator has been built into the binary with the fact
> whether an accelerator is available and usable. There are plenty of
> cases where e.g. kvm is built into the binary but not usable during
> runtime, e.g. because:
> 1) The kernel does not support it
> 2) The current host CPU does not support it
> 3) There are problems with the permission to /dev/kvm
> etc.

Yes.

> I think we either need the fallback mechanism to tcg

No, this is precisely what we want to test.

> or you need to
> properly check whether KVM is usable, too.

Yes.