Re: [PATCH v3 1/6] KVM: x86: do not allow re-enabling quirks

[Binbin Wu]

On 3/4/2025 2:06 PM, Paolo Bonzini wrote:
> Allowing arbitrary re-enabling of quirks puts a limit on what the
> quirks themselves can do, since you cannot assume that the quirk
> prevents a particular state.  More important, it also prevents
> KVM from disabling a quirk at VM creation time, because userspace
> can always go back and re-enable that.
>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
>   arch/x86/kvm/x86.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 856ceeb4fb35..35d03fcdb8e9 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -6525,7 +6525,7 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
>   			break;
>   		fallthrough;
>   	case KVM_CAP_DISABLE_QUIRKS:
> -		kvm->arch.disabled_quirks = cap->args[0];
> +		kvm->arch.disabled_quirks |= cap->args[0];
>   		r = 0;
>   		break;
>   	case KVM_CAP_SPLIT_IRQCHIP: {
This  change requires changes in KVM selftests for monitor_mwait_test.

I cooked a patch to pass the test case.

From 29b22d0a5cb14b418d289d78e2e290f7e0fc1749 Mon Sep 17 00:00:00 2001
From: Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>
Date: Tue, 18 Mar 2025 17:31:51 +0800
Subject: [PATCH] KVM: selftests: Test monitor/mwait cases in separate VMs

Test different cases of disabling quirk combinations for monitor/mwait in
separate VMs after KVM does not allow re-enabling quirks.

Signed-off-by: Binbin Wu <binbin.wu@xxxxxxxxxxxxxxx>
---
 .../selftests/kvm/x86/monitor_mwait_test.c    | 44 ++++++++++++-------
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/tools/testing/selftests/kvm/x86/monitor_mwait_test.c b/tools/testing/selftests/kvm/x86/monitor_mwait_test.c
index 2b550eff35f1..b583e0523575 100644
--- a/tools/testing/selftests/kvm/x86/monitor_mwait_test.c
+++ b/tools/testing/selftests/kvm/x86/monitor_mwait_test.c
@@ -16,6 +16,8 @@ enum monitor_mwait_testcases {
        MWAIT_DISABLED = BIT(2),
 };

+static int testcase;
+
 /*
  * If both MWAIT and its quirk are disabled, MONITOR/MWAIT should #UD, in all
  * other scenarios KVM should emulate them as nops.
@@ -35,7 +37,7 @@ do {                                                                  \
                               testcase, vector);                       \
 } while (0)

-static void guest_monitor_wait(int testcase)
+static void guest_monitor_wait(void)
 {
        u8 vector;

@@ -54,31 +56,22 @@ static void guest_monitor_wait(int testcase)

 static void guest_code(void)
 {
-       guest_monitor_wait(MWAIT_DISABLED);
-
-       guest_monitor_wait(MWAIT_QUIRK_DISABLED | MWAIT_DISABLED);
-
-       guest_monitor_wait(MISC_ENABLES_QUIRK_DISABLED | MWAIT_DISABLED);
-       guest_monitor_wait(MISC_ENABLES_QUIRK_DISABLED);
-
-       guest_monitor_wait(MISC_ENABLES_QUIRK_DISABLED | MWAIT_QUIRK_DISABLED | MWAIT_DISABLED);
-       guest_monitor_wait(MISC_ENABLES_QUIRK_DISABLED | MWAIT_QUIRK_DISABLED);
-
+       guest_monitor_wait();
        GUEST_DONE();
 }

-int main(int argc, char *argv[])
+static void vm_test_case(int test_case)
 {
        uint64_t disabled_quirks;
        struct kvm_vcpu *vcpu;
        struct kvm_vm *vm;
        struct ucall uc;
-       int testcase;
-
-       TEST_REQUIRE(this_cpu_has(X86_FEATURE_MWAIT));
-       TEST_REQUIRE(kvm_has_cap(KVM_CAP_DISABLE_QUIRKS2));

        vm = vm_create_with_one_vcpu(&vcpu, guest_code);
+
+       testcase = test_case;
+       sync_global_to_guest(vm, testcase);
+
        vcpu_clear_cpuid_feature(vcpu, X86_FEATURE_MWAIT);

        while (1) {
@@ -87,7 +80,7 @@ int main(int argc, char *argv[])

                switch (get_ucall(vcpu, &uc)) {
                case UCALL_SYNC:
-                       testcase = uc.args[1];
+                       TEST_ASSERT_EQ(testcase, uc.args[1]);
                        break;
                case UCALL_ABORT:
                        REPORT_GUEST_ASSERT(uc);
@@ -125,5 +118,22 @@ int main(int argc, char *argv[])

 done:
        kvm_vm_free(vm);
+}
+
+int main(int argc, char *argv[])
+{
+       TEST_REQUIRE(this_cpu_has(X86_FEATURE_MWAIT));
+       TEST_REQUIRE(kvm_has_cap(KVM_CAP_DISABLE_QUIRKS2));
+
+       vm_test_case(MWAIT_DISABLED);
+
+       vm_test_case(MWAIT_QUIRK_DISABLED | MWAIT_DISABLED);
+
+       vm_test_case(MISC_ENABLES_QUIRK_DISABLED | MWAIT_DISABLED);
+       vm_test_case(MISC_ENABLES_QUIRK_DISABLED);
+
+       vm_test_case(MISC_ENABLES_QUIRK_DISABLED | MWAIT_QUIRK_DISABLED | MWAIT_DISABLED);
+       vm_test_case(MISC_ENABLES_QUIRK_DISABLED | MWAIT_QUIRK_DISABLED);
+
        return 0;
 }
--
2.46.0