| Message ID | 20250304060647.2903469-2-pbonzini@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | KVM: x86: Introduce quirk KVM_X86_QUIRK_IGNORE_GUEST_PAT | expand |
LGTM. On Tue, Mar 04, 2025 at 01:06:42AM -0500, 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@redhat.com> > --- > 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: { > -- > 2.43.5 > >
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@redhat.com> > --- > 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@linux.intel.com> 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@linux.intel.com> --- .../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; }
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: {
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@redhat.com> --- arch/x86/kvm/x86.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)