Re: [PATCH v3 7/6] qtest/arm-cpu-features: Use generic qtest_has_accel() to check for KVM

[Philippe Mathieu-Daudé]

Hi Drew,

On 4/15/21 3:23 PM, Andrew Jones wrote:
> On Thu, Apr 15, 2021 at 02:57:37PM +0200, Philippe Mathieu-Daudé wrote:
>> Use the recently added generic qtest_has_accel() method to
>> check if KVM is available.
>>
>> Suggested-by: Claudio Fontana <cfontana@suse.de>
>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>> ---
>>  tests/qtest/arm-cpu-features.c | 25 +------------------------
>>  1 file changed, 1 insertion(+), 24 deletions(-)

>>  static QDict *do_query_no_props(QTestState *qts, const char *cpu_type)
>>  {
>>      return qtest_qmp(qts, QUERY_HEAD "'model': { 'name': %s }"
>> @@ -493,14 +478,6 @@ static void test_query_cpu_model_expansion_kvm(const 
>> void *data)
>>  
>>      qts = qtest_init(MACHINE_KVM "-cpu max");
>>  
>> -    /*
>> -     * These tests target the 'host' CPU type, so KVM must be enabled.
>> -     */
>> -    if (!kvm_enabled(qts)) {
>> -        qtest_quit(qts);
>> -        return;
>> -    }
>> -
>>      /* Enabling and disabling kvm-no-adjvtime should always work. */
>>      assert_has_feature_disabled(qts, "host", "kvm-no-adjvtime");
>>      assert_set_feature(qts, "host", "kvm-no-adjvtime", true);
>> @@ -624,7 +601,7 @@ int main(int argc, char **argv)
>>       * order avoid attempting to run an AArch32 QEMU with KVM on
>>       * AArch64 hosts. That won't work and isn't easy to detect.
>>       */
>> -    if (g_str_equal(qtest_get_arch(), "aarch64")) {
>> +    if (g_str_equal(qtest_get_arch(), "aarch64") && qtest_has_accel("kvm")) 
>> {
> 
> With this new guard in main(), we should be able to drop the tcg fallback
> in MACHINE_KVM. But, to do that, we also need to guard the call to
> sve_tests_sve_off_kvm().

OK, I'll have a look.

>>          qtest_add_data_func("/arm/kvm/query-cpu-model-expansion",
>>                              NULL, test_query_cpu_model_expansion_kvm);
>>      }
>> -- 
>> 2.26.3
>>
> 
> And, shouldn't we also guard all the other tests, which require tcg, with
> qtest_has_accel("tcg")?

Now that we have this qtest_has_accel() method, there is certainly room
for improvements / simplifications. This doesn't have to block this
patch set and could be done on top. (Beside helping Claudio's effort,
my main selfish motivation was to stop compile pointless objects and
reduce minutes wasted on CI).

Thanks,

Phil.