[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <bc22ff85-4caa-410e-b8b0-ea35975aad65@linux.intel.com>
Date: Wed, 19 Mar 2025 09:20:36 +0800
From: Binbin Wu <binbin.wu@...ux.intel.com>
To: Paolo Bonzini <pbonzini@...hat.com>
Cc: linux-kernel@...r.kernel.org, kvm@...r.kernel.org, xiaoyao.li@...el.com,
seanjc@...gle.com, yan.y.zhao@...el.com
Subject: Re: [PATCH v3 1/6] KVM: x86: do not allow re-enabling quirks
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@...hat.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@...ux.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@...ux.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;
}
--
2.46.0
Powered by blists - more mailing lists