* [PATCH 1/3] KVM: SVM: Check EFER.SVME before updating MSR_VM_CR on guest writes
@ 2026-10-29 20:28 Jim Mattson
2026-10-29 20:28 ` [PATCH 2/3] KVM: selftests: Rename svm_nested_efer_test to svm_msrs_test Jim Mattson
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Jim Mattson @ 2026-10-29 20:28 UTC (permalink / raw)
To: Sean Christopherson, Paolo Bonzini
Cc: Shuah Khan, kvm, linux-kselftest, linux-kernel, Yosry Ahmed,
Jim Mattson
Per the AMD APM, setting VM_CR.SVMDIS while EFER.SVME is set raises a #GP.
In svm_set_vm_cr(), check whether EFER.SVME is set and the value being
written sets SVMDIS *before* modifying svm->nested.vm_cr_msr. (Host-
initiated writes are allowed to set any valid MSR_VM_CR bits.)
Fixes: 4a810181c8bc ("KVM: SVM: Implement emulation of vm_cr msr")
Signed-off-by: Jim Mattson <jmattson@google.com>
---
arch/x86/kvm/svm/svm.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
index 136069dfcd44..3a0a507c94b2 100644
--- a/arch/x86/kvm/svm/svm.c
+++ b/arch/x86/kvm/svm/svm.c
@@ -3015,14 +3015,24 @@ static int svm_complete_emulated_msr(struct kvm_vcpu *vcpu, int err)
return 1;
}
-static int svm_set_vm_cr(struct kvm_vcpu *vcpu, u64 data)
+static int svm_set_vm_cr(struct kvm_vcpu *vcpu, struct msr_data *msr)
{
struct vcpu_svm *svm = to_svm(vcpu);
- int svm_dis, chg_mask;
+ u64 data = msr->data;
+ int chg_mask;
if (data & ~SVM_VM_CR_VALID_MASK)
return 1;
+ if (msr->host_initiated) {
+ svm->nested.vm_cr_msr = data;
+ return 0;
+ }
+
+ /* check for svm_disable while efer.svme is set */
+ if ((data & SVM_VM_CR_SVM_DIS_MASK) && (vcpu->arch.efer & EFER_SVME))
+ return 1;
+
chg_mask = SVM_VM_CR_VALID_MASK;
if (svm->nested.vm_cr_msr & SVM_VM_CR_SVM_DIS_MASK)
@@ -3031,12 +3041,6 @@ static int svm_set_vm_cr(struct kvm_vcpu *vcpu, u64 data)
svm->nested.vm_cr_msr &= ~chg_mask;
svm->nested.vm_cr_msr |= (data & chg_mask);
- svm_dis = svm->nested.vm_cr_msr & SVM_VM_CR_SVM_DIS_MASK;
-
- /* check for svm_disable while efer.svme is set */
- if (svm_dis && (vcpu->arch.efer & EFER_SVME))
- return 1;
-
return 0;
}
@@ -3262,7 +3266,7 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr)
svm->nested.hsave_msr = data & PAGE_MASK;
break;
case MSR_VM_CR:
- return svm_set_vm_cr(vcpu, data);
+ return svm_set_vm_cr(vcpu, msr);
case MSR_VM_IGNNE:
kvm_pr_unimpl_wrmsr(vcpu, ecx, data);
break;
--
2.56.0.317.g454f8302e1-goog
^ permalink raw reply related [flat|nested] 8+ messages in thread* [PATCH 2/3] KVM: selftests: Rename svm_nested_efer_test to svm_msrs_test 2026-10-29 20:28 [PATCH 1/3] KVM: SVM: Check EFER.SVME before updating MSR_VM_CR on guest writes Jim Mattson @ 2026-10-29 20:28 ` Jim Mattson 2026-10-29 20:28 ` [PATCH 3/3] KVM: selftests: Add MSR_VM_CR tests " Jim Mattson 2026-10-29 21:19 ` [PATCH 1/3] KVM: SVM: Check EFER.SVME before updating MSR_VM_CR on guest writes Sean Christopherson 2 siblings, 0 replies; 8+ messages in thread From: Jim Mattson @ 2026-10-29 20:28 UTC (permalink / raw) To: Sean Christopherson, Paolo Bonzini Cc: Shuah Khan, kvm, linux-kselftest, linux-kernel, Yosry Ahmed, Jim Mattson Rename svm_nested_efer_test.c to svm_msrs_test.c (matching vmx_msrs_test.c), in preparation for testing other SVM MSRs. No functional change intended. Signed-off-by: Jim Mattson <jmattson@google.com> --- tools/testing/selftests/kvm/Makefile.kvm | 2 +- .../kvm/x86/{svm_nested_efer_test.c => svm_msrs_test.c} | 0 2 files changed, 1 insertion(+), 1 deletion(-) rename tools/testing/selftests/kvm/x86/{svm_nested_efer_test.c => svm_msrs_test.c} (100%) diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm index 888a7530a280..d1ddce584efe 100644 --- a/tools/testing/selftests/kvm/Makefile.kvm +++ b/tools/testing/selftests/kvm/Makefile.kvm @@ -122,7 +122,7 @@ TEST_GEN_PROGS_x86 += x86/state_test TEST_GEN_PROGS_x86 += x86/vmx_preemption_timer_test TEST_GEN_PROGS_x86 += x86/svm_vmcall_test TEST_GEN_PROGS_x86 += x86/svm_int_ctl_test -TEST_GEN_PROGS_x86 += x86/svm_nested_efer_test +TEST_GEN_PROGS_x86 += x86/svm_msrs_test TEST_GEN_PROGS_x86 += x86/svm_nested_shutdown_test TEST_GEN_PROGS_x86 += x86/svm_nested_soft_inject_test TEST_GEN_PROGS_x86 += x86/svm_nested_vmcb12_gpa diff --git a/tools/testing/selftests/kvm/x86/svm_nested_efer_test.c b/tools/testing/selftests/kvm/x86/svm_msrs_test.c similarity index 100% rename from tools/testing/selftests/kvm/x86/svm_nested_efer_test.c rename to tools/testing/selftests/kvm/x86/svm_msrs_test.c -- 2.56.0.317.g454f8302e1-goog ^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCH 3/3] KVM: selftests: Add MSR_VM_CR tests to svm_msrs_test 2026-10-29 20:28 [PATCH 1/3] KVM: SVM: Check EFER.SVME before updating MSR_VM_CR on guest writes Jim Mattson 2026-10-29 20:28 ` [PATCH 2/3] KVM: selftests: Rename svm_nested_efer_test to svm_msrs_test Jim Mattson @ 2026-10-29 20:28 ` Jim Mattson 2026-10-29 21:19 ` [PATCH 1/3] KVM: SVM: Check EFER.SVME before updating MSR_VM_CR on guest writes Sean Christopherson 2 siblings, 0 replies; 8+ messages in thread From: Jim Mattson @ 2026-10-29 20:28 UTC (permalink / raw) To: Sean Christopherson, Paolo Bonzini Cc: Shuah Khan, kvm, linux-kselftest, linux-kernel, Yosry Ahmed, Jim Mattson Add sub-tests to svm_msrs_test to verify KVM's emulation of guest and host-initiated writes to MSR_VM_CR: - Writing reserved bits 63:5 faults with #GP(0) (or fails KVM_SET_MSRS) without modifying MSR_VM_CR. - Guest WRMSR setting SVMDIS while EFER.SVME=1 faults with #GP(0) without modifying MSR_VM_CR. - Guest WRMSR setting SVMDIS while EFER.SVME=0 succeeds, after which guest writes to LOCK and SVMDIS are silently ignored while bits 2:0 remain writable. - Host-initiated writes via KVM_SET_MSRS can set any valid MSR_VM_CR bit regardless of EFER.SVME and can clear SVMDIS and LOCK. Signed-off-by: Jim Mattson <jmattson@google.com> --- .../testing/selftests/kvm/x86/svm_msrs_test.c | 167 ++++++++++++++++-- 1 file changed, 153 insertions(+), 14 deletions(-) diff --git a/tools/testing/selftests/kvm/x86/svm_msrs_test.c b/tools/testing/selftests/kvm/x86/svm_msrs_test.c index fa0d76a9c390..e1f386ee0e7e 100644 --- a/tools/testing/selftests/kvm/x86/svm_msrs_test.c +++ b/tools/testing/selftests/kvm/x86/svm_msrs_test.c @@ -1,6 +1,6 @@ // SPDX-License-Identifier: GPL-2.0-only /* - * Tests for KVM's handling of EFER bits whose behavior is tied to nested SVM. + * Tests for KVM's handling of SVM MSRs (EFER and VM_CR). * * Copyright (C) 2026, Google LLC. */ @@ -33,6 +33,74 @@ static void l1_clear_efer_svme(struct svm_test_data *svm) GUEST_ASSERT(0); } +static void guest_wrmsr_vm_cr(u64 val, u64 expected_val, bool expect_gp) +{ + u8 expected_vec = expect_gp ? GP_VECTOR : 0; + u64 actual_val; + u8 vec; + + vec = wrmsr_safe(MSR_VM_CR, val); + __GUEST_ASSERT(vec == expected_vec, + "Wanted %s on WRMSR(MSR_VM_CR, 0x%lx), got %s", + ex_str(expected_vec), val, ex_str(vec)); + + actual_val = rdmsr(MSR_VM_CR); + __GUEST_ASSERT(actual_val == expected_val, + "Wanted MSR_VM_CR == 0x%lx after WRMSR(0x%lx), got 0x%lx", + expected_val, val, actual_val); +} + +static void guest_vm_cr(void) +{ + const u64 svme_valid_mask = SVM_VM_CR_VALID_MASK & ~SVM_VM_CR_SVM_DIS_MASK; + const u64 unlocked_mask = SVM_VM_CR_VALID_MASK & + ~(SVM_VM_CR_SVM_LOCK_MASK | SVM_VM_CR_SVM_DIS_MASK); + u64 efer, val, expected = 0; + bool valid; + int bit; + + GUEST_ASSERT_EQ(rdmsr(MSR_VM_CR), 0); + + efer = rdmsr(MSR_EFER); + wrmsr(MSR_EFER, efer | EFER_SVME); + + /* + * With EFER.SVME=1, setting non-SVMDIS bits in MSR_VM_CR is allowed, + * whereas setting SVMDIS or reserved bits must fault with #GP(0) + * without modifying MSR_VM_CR. + */ + guest_wrmsr_vm_cr(SVM_VM_CR_VALID_MASK, 0, true); + + for (bit = 0; bit < BITS_PER_LONG; bit++) { + val = BIT_ULL(bit); + valid = val & svme_valid_mask; + if (valid) + expected = val; + + guest_wrmsr_vm_cr(val, expected, !valid); + } + + /* + * Verify that prior faulting attempts to set SVMDIS while EFER.SVME=1 + * did not latch SVMDIS and cause subsequent valid writes to fault. + */ + guest_wrmsr_vm_cr(0, 0, false); + + /* + * With EFER.SVME=0, setting SVMDIS succeeds. Once set, guest writes + * to LOCK and SVMDIS are silently ignored while bits 2:0 remain + * writable. + */ + wrmsr(MSR_EFER, efer & ~EFER_SVME); + + guest_wrmsr_vm_cr(SVM_VM_CR_SVM_DIS_MASK, SVM_VM_CR_SVM_DIS_MASK, false); + guest_wrmsr_vm_cr(svme_valid_mask, + SVM_VM_CR_SVM_DIS_MASK | unlocked_mask, false); + guest_wrmsr_vm_cr(0, SVM_VM_CR_SVM_DIS_MASK, false); + + GUEST_DONE(); +} + static void l2_lmsle(void) { GUEST_ASSERT(rdmsr(MSR_EFER) & EFER_LMSLE); @@ -144,17 +212,10 @@ static void test_clear_efer_svme(void) ksft_test_result_pass("L2 clearing EFER.SVME shuts down L1\n"); } -static void test_lmsle(bool lmsle_mbz) +static void run_vcpu_to_done(struct kvm_vcpu *vcpu) { - struct kvm_vcpu *vcpu; - struct kvm_vm *vm; struct ucall uc; - vcpu = create_l1_vcpu(&vm, l1_lmsle); - - vcpu_set_or_clear_cpuid_feature(vcpu, X86_FEATURE_EFER_LMSLE_MBZ, - lmsle_mbz); - vcpu_run(vcpu); TEST_ASSERT_KVM_EXIT_REASON(vcpu, KVM_EXIT_IO); @@ -166,6 +227,79 @@ static void test_lmsle(bool lmsle_mbz) default: TEST_FAIL("Unexpected ucall: %lu", uc.cmd); } +} + +static void test_vm_cr(void) +{ + struct kvm_vcpu *vcpu; + struct kvm_vm *vm; + + vm = vm_create_with_one_vcpu(&vcpu, guest_vm_cr); + + run_vcpu_to_done(vcpu); + + kvm_vm_free(vm); + ksft_test_result_pass("Guest writes to MSR_VM_CR\n"); +} + +static void test_host_initiated_vm_cr(void) +{ + u64 efer, val, actual, expected = 0; + struct kvm_vcpu *vcpu; + struct kvm_vm *vm; + int bit, rc; + bool valid; + + vm = vm_create_with_one_vcpu(&vcpu, NULL); + + /* + * Host-initiated writes must be able to set SVMDIS even when + * EFER.SVME=1 (e.g. when restoring MSR_VM_CR before MSR_EFER), and + * must be able to modify and clear SVMDIS and LOCK after SVMDIS has + * been set. + */ + TEST_ASSERT_EQ(vcpu_get_msr(vcpu, MSR_VM_CR), 0); + + efer = vcpu_get_msr(vcpu, MSR_EFER); + vcpu_set_msr(vcpu, MSR_EFER, efer | EFER_SVME); + + for (bit = 0; bit < BITS_PER_LONG; bit++) { + val = BIT_ULL(bit); + valid = val & SVM_VM_CR_VALID_MASK; + + rc = _vcpu_set_msr(vcpu, MSR_VM_CR, val); + TEST_ASSERT(valid ? rc == 1 : !rc, + "Expected KVM_SET_MSRS(MSR_VM_CR, 0x%lx) to %s, got rc=%d", + val, valid ? "succeed" : "fail", rc); + if (valid) + expected = val; + actual = vcpu_get_msr(vcpu, MSR_VM_CR); + TEST_ASSERT(actual == expected, + "Wanted MSR_VM_CR == 0x%lx after KVM_SET_MSRS(0x%lx), got 0x%lx", + expected, val, actual); + } + + vcpu_set_msr(vcpu, MSR_VM_CR, SVM_VM_CR_VALID_MASK); + TEST_ASSERT_EQ(vcpu_get_msr(vcpu, MSR_VM_CR), SVM_VM_CR_VALID_MASK); + + vcpu_set_msr(vcpu, MSR_VM_CR, 0); + TEST_ASSERT_EQ(vcpu_get_msr(vcpu, MSR_VM_CR), 0); + + kvm_vm_free(vm); + ksft_test_result_pass("Host-initiated writes to MSR_VM_CR\n"); +} + +static void test_lmsle(bool lmsle_mbz) +{ + struct kvm_vcpu *vcpu; + struct kvm_vm *vm; + + vcpu = create_l1_vcpu(&vm, l1_lmsle); + + vcpu_set_or_clear_cpuid_feature(vcpu, X86_FEATURE_EFER_LMSLE_MBZ, + lmsle_mbz); + + run_vcpu_to_done(vcpu); kvm_vm_free(vm); ksft_test_result_pass("Guest EFER_LMSLE_MBZ=%d\n", lmsle_mbz); @@ -225,15 +359,16 @@ int main(int argc, char *argv[]) bool has_nested_svm, has_lmsle; ksft_print_header(); - ksft_set_plan(6); + ksft_set_plan(8); test_enumeration(); /* - * The sub-tests below need to actually run a nested guest, and the - * EFER.LMSLE sub-tests additionally need KVM to allow EFER.LMSLE. - * It's KVM's view of the world, not raw CPUID, that dictates whether - * EFER.LMSLE is allowed, i.e. whether the defeature is emulated. + * The sub-tests below need to actually run a nested guest or set + * EFER.SVME, and the EFER.LMSLE sub-tests additionally need KVM to + * allow EFER.LMSLE. It's KVM's view of the world, not raw CPUID, that + * dictates whether EFER.LMSLE is allowed, i.e. whether the defeature is + * emulated. */ has_nested_svm = kvm_cpu_has(X86_FEATURE_SVM); has_lmsle = has_nested_svm && @@ -246,9 +381,13 @@ int main(int argc, char *argv[]) if (has_nested_svm) { test_clear_efer_svme(); + test_vm_cr(); + test_host_initiated_vm_cr(); test_lmsle(true); } else { ksft_test_result_skip("L2 clearing EFER.SVME shuts down L1\n"); + ksft_test_result_skip("Guest writes to MSR_VM_CR\n"); + ksft_test_result_skip("Host-initiated writes to MSR_VM_CR\n"); ksft_test_result_skip("Guest EFER_LMSLE_MBZ=1\n"); } -- 2.56.0.317.g454f8302e1-goog ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] KVM: SVM: Check EFER.SVME before updating MSR_VM_CR on guest writes 2026-10-29 20:28 [PATCH 1/3] KVM: SVM: Check EFER.SVME before updating MSR_VM_CR on guest writes Jim Mattson 2026-10-29 20:28 ` [PATCH 2/3] KVM: selftests: Rename svm_nested_efer_test to svm_msrs_test Jim Mattson 2026-10-29 20:28 ` [PATCH 3/3] KVM: selftests: Add MSR_VM_CR tests " Jim Mattson @ 2026-10-29 21:19 ` Sean Christopherson 2026-10-29 21:46 ` Jim Mattson 2 siblings, 1 reply; 8+ messages in thread From: Sean Christopherson @ 2026-10-29 21:19 UTC (permalink / raw) To: Jim Mattson Cc: Paolo Bonzini, Shuah Khan, kvm, linux-kselftest, linux-kernel, Yosry Ahmed On Thu, Oct 29, 2026, Jim Mattson wrote: > (Host- initiated writes are allowed to set any valid MSR_VM_CR bits.) Eww. Why? If we allow this, then we'll end up with the mess where the guest can't write back the current value (if userspace is being weird). I get that we want to avoid ordering issues, but there's *already* a required order here, unless userspace ignores the failure from svm_set_vm_cr() and continues on. KVM doesn't advertise the MSR as something to be saved/restored, and none of QEMU, CHV, CrosVM, or our VMM appear to save/restore it. So why make trouble for ourselves? > Fixes: 4a810181c8bc ("KVM: SVM: Implement emulation of vm_cr msr") > Signed-off-by: Jim Mattson <jmattson@google.com> > --- > arch/x86/kvm/svm/svm.c | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) > > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > index 136069dfcd44..3a0a507c94b2 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -3015,14 +3015,24 @@ static int svm_complete_emulated_msr(struct kvm_vcpu *vcpu, int err) > return 1; > } > > -static int svm_set_vm_cr(struct kvm_vcpu *vcpu, u64 data) > +static int svm_set_vm_cr(struct kvm_vcpu *vcpu, struct msr_data *msr) > { > struct vcpu_svm *svm = to_svm(vcpu); > - int svm_dis, chg_mask; > + u64 data = msr->data; > + int chg_mask; > > if (data & ~SVM_VM_CR_VALID_MASK) > return 1; > > + if (msr->host_initiated) { > + svm->nested.vm_cr_msr = data; > + return 0; > + } > + > + /* check for svm_disable while efer.svme is set */ > + if ((data & SVM_VM_CR_SVM_DIS_MASK) && (vcpu->arch.efer & EFER_SVME)) > + return 1; > + > chg_mask = SVM_VM_CR_VALID_MASK; > > if (svm->nested.vm_cr_msr & SVM_VM_CR_SVM_DIS_MASK) > @@ -3031,12 +3041,6 @@ static int svm_set_vm_cr(struct kvm_vcpu *vcpu, u64 data) > svm->nested.vm_cr_msr &= ~chg_mask; > svm->nested.vm_cr_msr |= (data & chg_mask); > > - svm_dis = svm->nested.vm_cr_msr & SVM_VM_CR_SVM_DIS_MASK; > - > - /* check for svm_disable while efer.svme is set */ > - if (svm_dis && (vcpu->arch.efer & EFER_SVME)) > - return 1; > - > return 0; > } > > @@ -3262,7 +3266,7 @@ static int svm_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr) > svm->nested.hsave_msr = data & PAGE_MASK; > break; > case MSR_VM_CR: > - return svm_set_vm_cr(vcpu, data); > + return svm_set_vm_cr(vcpu, msr); > case MSR_VM_IGNNE: > kvm_pr_unimpl_wrmsr(vcpu, ecx, data); > break; > -- > 2.56.0.317.g454f8302e1-goog > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] KVM: SVM: Check EFER.SVME before updating MSR_VM_CR on guest writes 2026-10-29 21:19 ` [PATCH 1/3] KVM: SVM: Check EFER.SVME before updating MSR_VM_CR on guest writes Sean Christopherson @ 2026-10-29 21:46 ` Jim Mattson 2026-10-29 21:52 ` Sean Christopherson 0 siblings, 1 reply; 8+ messages in thread From: Jim Mattson @ 2026-10-29 21:46 UTC (permalink / raw) To: Sean Christopherson Cc: Paolo Bonzini, Shuah Khan, kvm, linux-kselftest, linux-kernel, Yosry Ahmed On Thu, Oct 29, 2026 at 2:19 PM Sean Christopherson <seanjc@google.com> wrote: > > On Thu, Oct 29, 2026, Jim Mattson wrote: > > (Host- initiated writes are allowed to set any valid MSR_VM_CR bits.) > > Eww. Why? If we allow this, then we'll end up with the mess where the guest > can't write back the current value (if userspace is being weird). I get that > we want to avoid ordering issues, but there's *already* a required order here, > unless userspace ignores the failure from svm_set_vm_cr() and continues on. > > KVM doesn't advertise the MSR as something to be saved/restored, and none of > QEMU, CHV, CrosVM, or our VMM appear to save/restore it. So why make trouble > for ourselves? OMG! I didn't look any farther than the obviously broken svm_set_vm_cr(). It looks like svm->nested.vm_cr_msr controls nothing other than itself. It's just 5 bits of storage with some weird locking rules, and it resets to 0 on suspend/resume. What is the point of such half-baked support? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] KVM: SVM: Check EFER.SVME before updating MSR_VM_CR on guest writes 2026-10-29 21:46 ` Jim Mattson @ 2026-10-29 21:52 ` Sean Christopherson 2026-10-29 22:04 ` Jim Mattson 0 siblings, 1 reply; 8+ messages in thread From: Sean Christopherson @ 2026-10-29 21:52 UTC (permalink / raw) To: Jim Mattson Cc: Paolo Bonzini, Shuah Khan, kvm, linux-kselftest, linux-kernel, Yosry Ahmed On Thu, Oct 29, 2026, Jim Mattson wrote: > On Thu, Oct 29, 2026 at 2:19 PM Sean Christopherson <seanjc@google.com> wrote: > > > > On Thu, Oct 29, 2026, Jim Mattson wrote: > > > (Host- initiated writes are allowed to set any valid MSR_VM_CR bits.) > > > > Eww. Why? If we allow this, then we'll end up with the mess where the guest > > can't write back the current value (if userspace is being weird). I get that > > we want to avoid ordering issues, but there's *already* a required order here, > > unless userspace ignores the failure from svm_set_vm_cr() and continues on. > > > > KVM doesn't advertise the MSR as something to be saved/restored, and none of > > QEMU, CHV, CrosVM, or our VMM appear to save/restore it. So why make trouble > > for ourselves? > > OMG! I didn't look any farther than the obviously broken svm_set_vm_cr(). > > It looks like svm->nested.vm_cr_msr controls nothing other than itself. LOL, nice. > It's just 5 bits of storage with some weird locking rules, and it > resets to 0 on suspend/resume. > > What is the point of such half-baked support? More than likely, so that older kernels didn't crash when writing an MSR KVM didn't emulate. :-( ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] KVM: SVM: Check EFER.SVME before updating MSR_VM_CR on guest writes 2026-10-29 21:52 ` Sean Christopherson @ 2026-10-29 22:04 ` Jim Mattson 2026-10-30 0:00 ` Jim Mattson 0 siblings, 1 reply; 8+ messages in thread From: Jim Mattson @ 2026-10-29 22:04 UTC (permalink / raw) To: Sean Christopherson Cc: Paolo Bonzini, Shuah Khan, kvm, linux-kselftest, linux-kernel, Yosry Ahmed On Thu, Oct 29, 2026 at 2:52 PM Sean Christopherson <seanjc@google.com> wrote: > > On Thu, Oct 29, 2026, Jim Mattson wrote: > > On Thu, Oct 29, 2026 at 2:19 PM Sean Christopherson <seanjc@google.com> wrote: > > > > > > On Thu, Oct 29, 2026, Jim Mattson wrote: > > > > (Host- initiated writes are allowed to set any valid MSR_VM_CR bits.) > > > > > > Eww. Why? If we allow this, then we'll end up with the mess where the guest > > > can't write back the current value (if userspace is being weird). I get that > > > we want to avoid ordering issues, but there's *already* a required order here, > > > unless userspace ignores the failure from svm_set_vm_cr() and continues on. > > > > > > KVM doesn't advertise the MSR as something to be saved/restored, and none of > > > QEMU, CHV, CrosVM, or our VMM appear to save/restore it. So why make trouble > > > for ourselves? > > > > OMG! I didn't look any farther than the obviously broken svm_set_vm_cr(). > > > > It looks like svm->nested.vm_cr_msr controls nothing other than itself. > > LOL, nice. > > > It's just 5 bits of storage with some weird locking rules, and it > > resets to 0 on suspend/resume. > > > > What is the point of such half-baked support? > > More than likely, so that older kernels didn't crash when writing an MSR KVM > didn't emulate. :-( You're not going to let me get rid of it, are you? ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH 1/3] KVM: SVM: Check EFER.SVME before updating MSR_VM_CR on guest writes 2026-10-29 22:04 ` Jim Mattson @ 2026-10-30 0:00 ` Jim Mattson 0 siblings, 0 replies; 8+ messages in thread From: Jim Mattson @ 2026-10-30 0:00 UTC (permalink / raw) To: Sean Christopherson Cc: Paolo Bonzini, Shuah Khan, kvm, linux-kselftest, linux-kernel, Yosry Ahmed On Thu, Oct 29, 2026 at 3:04 PM Jim Mattson <jmattson@google.com> wrote: > > On Thu, Oct 29, 2026 at 2:52 PM Sean Christopherson <seanjc@google.com> wrote: > > > > On Thu, Oct 29, 2026, Jim Mattson wrote: > > > On Thu, Oct 29, 2026 at 2:19 PM Sean Christopherson <seanjc@google.com> wrote: > > > > > > > > On Thu, Oct 29, 2026, Jim Mattson wrote: > > > > > (Host- initiated writes are allowed to set any valid MSR_VM_CR bits.) > > > > > > > > Eww. Why? If we allow this, then we'll end up with the mess where the guest > > > > can't write back the current value (if userspace is being weird). I get that > > > > we want to avoid ordering issues, but there's *already* a required order here, > > > > unless userspace ignores the failure from svm_set_vm_cr() and continues on. > > > > > > > > KVM doesn't advertise the MSR as something to be saved/restored, and none of > > > > QEMU, CHV, CrosVM, or our VMM appear to save/restore it. So why make trouble > > > > for ourselves? > > > > > > OMG! I didn't look any farther than the obviously broken svm_set_vm_cr(). > > > > > > It looks like svm->nested.vm_cr_msr controls nothing other than itself. > > > > LOL, nice. > > > > > It's just 5 bits of storage with some weird locking rules, and it > > > resets to 0 on suspend/resume. > > > > > > What is the point of such half-baked support? > > > > More than likely, so that older kernels didn't crash when writing an MSR KVM > > didn't emulate. :-( > > You're not going to let me get rid of it, are you? LOL! Even the 'LOCK' check is wrong: if (svm->nested.vm_cr_msr & SVM_VM_CR_SVM_DIS_MASK) chg_mask &= ~(SVM_VM_CR_SVM_LOCK_MASK | SVM_VM_CR_SVM_DIS_MASK); The condition should be checking SVM_VM_CR_SVM_LOCK_MASK. Well, at least it can be used as a sentinel you can poll to see if your VM has live-migrated. :) ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-10-30 0:00 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-29 20:28 [PATCH 1/3] KVM: SVM: Check EFER.SVME before updating MSR_VM_CR on guest writes Jim Mattson 2026-10-29 20:28 ` [PATCH 2/3] KVM: selftests: Rename svm_nested_efer_test to svm_msrs_test Jim Mattson 2026-10-29 20:28 ` [PATCH 3/3] KVM: selftests: Add MSR_VM_CR tests " Jim Mattson 2026-10-29 21:19 ` [PATCH 1/3] KVM: SVM: Check EFER.SVME before updating MSR_VM_CR on guest writes Sean Christopherson 2026-10-29 21:46 ` Jim Mattson 2026-10-29 21:52 ` Sean Christopherson 2026-10-29 22:04 ` Jim Mattson 2026-10-30 0:00 ` Jim Mattson
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox