Re: [PATCH v2] target/arm: Check supported KVM features globally (not per vCPU)

[Peter Maydell]

On Fri, 19 Jun 2020 at 12:41, Andrew Jones <drjones@redhat.com> wrote:
> Can you also post the attached patch with this one (a two patch series)?

This would be easier to review if you'd just posted it as
a patch with a Based-on: and a note that it needed to be
applied after the bugfix patch. Anyway:

+    /* 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);
+    assert_set_feature(qts, "host", "kvm-no-adjvtime", false);

     if (g_str_equal(qtest_get_arch(), "aarch64")) {
         bool kvm_supports_sve;
@@ -475,7 +497,11 @@ static void
test_query_cpu_model_expansion_kvm(const void *data)
         char *error;

         assert_has_feature_enabled(qts, "host", "aarch64");
+
+        /* Enabling and disabling pmu should always work. */
         assert_has_feature_enabled(qts, "host", "pmu");
+        assert_set_feature(qts, "host", "pmu", true);
+        assert_set_feature(qts, "host", "pmu", false);

It seems a bit odd that we do the same "set true, then
set false" sequence whether the feature is enabled or not.
Shouldn't the second one be "set false, then set true" ?

thanks
-- PMM