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

[Philippe Mathieu-Daudé]

On 2/5/21 4:33 PM, Andrew Jones wrote:
> 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

Thanks, that helped.

I ran my tests including the "Restrict v7A TCG cpus to TCG accel"
patch which removes the A15 in KVM-only build:
https://lists.gnu.org/archive/html/qemu-devel/2021-01/msg08051.html
 So when TCG is disabled,

So I get:

{ "execute": "query-cpu-model-expansion", 'arguments': { 'type': 'full',
'model': { 'name': 'cortex-a15' }}}
{
    "error": {
        "class": "GenericError",
        "desc": "The CPU type 'cortex-a15' is not a recognized ARM CPU type"
    }
}

which fails the g_str_equal().

BTW is there some easy way to dump QMP traffic on stdio?

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