Re: [PATCH 1/9] tests/qtest/arm-cpu-features: Remove Cortex-A15 check

[Andrew Jones]

On Fri, Feb 05, 2021 at 04:15:45PM +0100, Philippe Mathieu-Daudé wrote:
> Hi Drew,
> 
> On 2/5/21 3:59 PM, Andrew Jones wrote:
> > On Fri, Feb 05, 2021 at 03:43:37PM +0100, Philippe Mathieu-Daudé wrote:
> >> Support for ARMv7 has been dropped in commit 82bf7ae84ce
> >> ("target/arm: Remove KVM support for 32-bit Arm hosts"),
> >> no need to check for Cortex A15 host cpu anymore.
> >>
> >> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >> ---
> >>  tests/qtest/arm-cpu-features.c | 4 ----
> >>  1 file changed, 4 deletions(-)
> >>
> >> diff --git a/tests/qtest/arm-cpu-features.c 
> >> b/tests/qtest/arm-cpu-features.c
> >> index 8252b85bb85..c59c3cb002b 100644
> >> --- a/tests/qtest/arm-cpu-features.c
> >> +++ b/tests/qtest/arm-cpu-features.c
> >> @@ -515,10 +515,6 @@ static void test_query_cpu_model_expansion_kvm(const 
> >> void *data)
> >>          QDict *resp;
> >>          char *error;
> >>  
> >> -        assert_error(qts, "cortex-a15",
> >> -            "We cannot guarantee the CPU type 'cortex-a15' works "
> >> -            "with KVM on this host", NULL);
> >> -
> > 
> > This isn't testing anything regarding 32-bit KVM host support. It's
> > testing that an error is returned when a given cpu type that can't
> > be known to work with KVM is used. We know that the cortex-a15 can't
> > be known to work. If we were to use a 64-bit cpu type here then there's
> > a chance that it would work, failing the test that an error be returned.
> 
> This was my first understanding, but then why does it fail?
> 
> PASS 1 qtest-aarch64/arm-cpu-features /aarch64/arm/query-cpu-model-expansion
> **
> ERROR:../../tests/qtest/arm-cpu-features.c:543:test_query_cpu_model_expansion_kvm:
> assertion failed: (g_str_equal(_error, "We cannot guarantee the CPU type
> 'cortex-a15' works " "with KVM on this host"))
> ERROR qtest-aarch64/arm-cpu-features - Bail out!
> ERROR:../../tests/qtest/arm-cpu-features.c:543:test_query_cpu_model_expansion_kvm:
> assertion failed: (g_str_equal(_error, "We cannot guarantee the CPU type
> 'cortex-a15' works " "with KVM on this host"))
> make: *** [Makefile.mtest:905: run-test-111] Error 1
> 
> FWIW when tracing (cavium thunderX1 host, dmesg reports 0x431f0a11):
> kvm_vcpu_ioctl cpu_index 0, type 0x4020aeae, arg 0xffff9b7f9b18

Hmm... I don't know. It works for me

$ QTEST_QEMU_BINARY=./qemu-system-aarch64 ./tests/qtest/arm-cpu-features
/aarch64/arm/query-cpu-model-expansion: OK
/aarch64/arm/kvm/query-cpu-model-expansion: OK
/aarch64/arm/kvm/query-cpu-model-expansion/sve-off: OK
/aarch64/arm/max/query-cpu-model-expansion/sve-max-vq-8: OK
/aarch64/arm/max/query-cpu-model-expansion/sve-off: OK

$ lscpu
Architecture:        aarch64
Byte Order:          Little Endian
CPU(s):              48
On-line CPU(s) list: 0-47
Thread(s) per core:  1
Core(s) per cluster: 16
Socket(s):           -
Cluster(s):          3
NUMA node(s):        1
Vendor ID:           Cavium
Model:               1
Model name:          ThunderX 88XX
Stepping:            0x1
BogoMIPS:            200.00
NUMA node0 CPU(s):   0-47
Flags:               fp asimd evtstrm aes pmull sha1 sha2 crc32 cpuid

> 
> > 
> >>          assert_has_feature_enabled(qts, "host", "aarch64");
> >>  
> >>          /* Enabling and disabling pmu should always work. */
> >> -- 
> >> 2.26.2
> >>
> >>
> > 
> > This file could use a cleanup patch regarding the dropping of 32-bit KVM
> > support though. At least the comment in main(), "For now we only run KVM
> > specific tests..." could be reworded. It was written that way when we
> > planned to try testing on 32-bit KVM too eventually, but we never did,
> > and now we'll never need to.
> > 
> > Thanks,
> > drew
> > 
> > 
>