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
