Skip to content

Commit 0499add

Browse files
committed
Merge tag 'kvm-x86-fixes-6.19-rc1' of https://github.com/kvm-x86/linux into HEAD
KVM fixes for 6.19-rc1 - Add a missing "break" to fix param parsing in the rseq selftest. - Apply runtime updates to the _current_ CPUID when userspace is setting CPUID, e.g. as part of vCPU hotplug, to fix a false positive and to avoid dropping the pending update. - Disallow toggling KVM_MEM_GUEST_MEMFD on an existing memslot, as it's not supported by KVM and leads to a use-after-free due to KVM failing to unbind the memslot from the previously-associated guest_memfd instance. - Harden against similar KVM_MEM_GUEST_MEMFD goofs, and prepare for supporting flags-only changes on KVM_MEM_GUEST_MEMFD memlslots, e.g. for dirty logging. - Set exit_code[63:32] to -1 (all 0xffs) when synthesizing a nested SVM_EXIT_ERR (a.k.a. VMEXIT_INVALID) #VMEXIT, as VMEXIT_INVALID is defined as -1ull (a 64-bit value). - Update SVI when activating APICv to fix a bug where a post-activation EOI for an in-service IRQ would effective be lost due to SVI being stale. - Immediately refresh APICv controls (if necessary) on a nested VM-Exit instead of deferring the update via KVM_REQ_APICV_UPDATE, as the request is effectively ignored because KVM thinks the vCPU already has the correct APICv settings.
2 parents ea1013c + 2976313 commit 0499add

File tree

10 files changed

+58
-18
lines changed

10 files changed

+58
-18
lines changed

arch/x86/kvm/cpuid.c

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -509,11 +509,18 @@ static int kvm_set_cpuid(struct kvm_vcpu *vcpu, struct kvm_cpuid_entry2 *e2,
509509
u32 vcpu_caps[NR_KVM_CPU_CAPS];
510510
int r;
511511

512+
/*
513+
* Apply pending runtime CPUID updates to the current CPUID entries to
514+
* avoid false positives due to mismatches on KVM-owned feature flags.
515+
*/
516+
if (vcpu->arch.cpuid_dynamic_bits_dirty)
517+
kvm_update_cpuid_runtime(vcpu);
518+
512519
/*
513520
* Swap the existing (old) entries with the incoming (new) entries in
514521
* order to massage the new entries, e.g. to account for dynamic bits
515-
* that KVM controls, without clobbering the current guest CPUID, which
516-
* KVM needs to preserve in order to unwind on failure.
522+
* that KVM controls, without losing the current guest CPUID, which KVM
523+
* needs to preserve in order to unwind on failure.
517524
*
518525
* Similarly, save the vCPU's current cpu_caps so that the capabilities
519526
* can be updated alongside the CPUID entries when performing runtime

arch/x86/kvm/svm/nested.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -985,7 +985,7 @@ int nested_svm_vmrun(struct kvm_vcpu *vcpu)
985985
if (!nested_vmcb_check_save(vcpu) ||
986986
!nested_vmcb_check_controls(vcpu)) {
987987
vmcb12->control.exit_code = SVM_EXIT_ERR;
988-
vmcb12->control.exit_code_hi = 0;
988+
vmcb12->control.exit_code_hi = -1u;
989989
vmcb12->control.exit_info_1 = 0;
990990
vmcb12->control.exit_info_2 = 0;
991991
goto out;
@@ -1018,7 +1018,7 @@ int nested_svm_vmrun(struct kvm_vcpu *vcpu)
10181018
svm->soft_int_injected = false;
10191019

10201020
svm->vmcb->control.exit_code = SVM_EXIT_ERR;
1021-
svm->vmcb->control.exit_code_hi = 0;
1021+
svm->vmcb->control.exit_code_hi = -1u;
10221022
svm->vmcb->control.exit_info_1 = 0;
10231023
svm->vmcb->control.exit_info_2 = 0;
10241024

arch/x86/kvm/svm/svm.c

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2443,6 +2443,7 @@ static bool check_selective_cr0_intercepted(struct kvm_vcpu *vcpu,
24432443

24442444
if (cr0 ^ val) {
24452445
svm->vmcb->control.exit_code = SVM_EXIT_CR0_SEL_WRITE;
2446+
svm->vmcb->control.exit_code_hi = 0;
24462447
ret = (nested_svm_exit_handled(svm) == NESTED_EXIT_DONE);
24472448
}
24482449

@@ -4617,6 +4618,7 @@ static int svm_check_intercept(struct kvm_vcpu *vcpu,
46174618
if (static_cpu_has(X86_FEATURE_NRIPS))
46184619
vmcb->control.next_rip = info->next_rip;
46194620
vmcb->control.exit_code = icpt_info.exit_code;
4621+
vmcb->control.exit_code_hi = 0;
46204622
vmexit = nested_svm_exit_handled(svm);
46214623

46224624
ret = (vmexit == NESTED_EXIT_DONE) ? X86EMUL_INTERCEPTED

arch/x86/kvm/svm/svm.h

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -761,9 +761,10 @@ int nested_svm_vmexit(struct vcpu_svm *svm);
761761

762762
static inline int nested_svm_simple_vmexit(struct vcpu_svm *svm, u32 exit_code)
763763
{
764-
svm->vmcb->control.exit_code = exit_code;
765-
svm->vmcb->control.exit_info_1 = 0;
766-
svm->vmcb->control.exit_info_2 = 0;
764+
svm->vmcb->control.exit_code = exit_code;
765+
svm->vmcb->control.exit_code_hi = 0;
766+
svm->vmcb->control.exit_info_1 = 0;
767+
svm->vmcb->control.exit_info_2 = 0;
767768
return nested_svm_vmexit(svm);
768769
}
769770

arch/x86/kvm/vmx/nested.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
#include "trace.h"
2020
#include "vmx.h"
2121
#include "smm.h"
22+
#include "x86_ops.h"
2223

2324
static bool __read_mostly enable_shadow_vmcs = 1;
2425
module_param_named(enable_shadow_vmcs, enable_shadow_vmcs, bool, S_IRUGO);
@@ -5165,7 +5166,7 @@ void __nested_vmx_vmexit(struct kvm_vcpu *vcpu, u32 vm_exit_reason,
51655166

51665167
if (vmx->nested.update_vmcs01_apicv_status) {
51675168
vmx->nested.update_vmcs01_apicv_status = false;
5168-
kvm_make_request(KVM_REQ_APICV_UPDATE, vcpu);
5169+
vmx_refresh_apicv_exec_ctrl(vcpu);
51695170
}
51705171

51715172
if (vmx->nested.update_vmcs01_hwapic_isr) {

arch/x86/kvm/vmx/vmx.c

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6937,15 +6937,6 @@ void vmx_hwapic_isr_update(struct kvm_vcpu *vcpu, int max_isr)
69376937
* VM-Exit, otherwise L1 with run with a stale SVI.
69386938
*/
69396939
if (is_guest_mode(vcpu)) {
6940-
/*
6941-
* KVM is supposed to forward intercepted L2 EOIs to L1 if VID
6942-
* is enabled in vmcs12; as above, the EOIs affect L2's vAPIC.
6943-
* Note, userspace can stuff state while L2 is active; assert
6944-
* that VID is disabled if and only if the vCPU is in KVM_RUN
6945-
* to avoid false positives if userspace is setting APIC state.
6946-
*/
6947-
WARN_ON_ONCE(vcpu->wants_to_run &&
6948-
nested_cpu_has_vid(get_vmcs12(vcpu)));
69496940
to_vmx(vcpu)->nested.update_vmcs01_hwapic_isr = true;
69506941
return;
69516942
}

arch/x86/kvm/x86.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10886,9 +10886,16 @@ void __kvm_vcpu_update_apicv(struct kvm_vcpu *vcpu)
1088610886
* pending. At the same time, KVM_REQ_EVENT may not be set as APICv was
1088710887
* still active when the interrupt got accepted. Make sure
1088810888
* kvm_check_and_inject_events() is called to check for that.
10889+
*
10890+
* Update SVI when APICv gets enabled, otherwise SVI won't reflect the
10891+
* highest bit in vISR and the next accelerated EOI in the guest won't
10892+
* be virtualized correctly (the CPU uses SVI to determine which vISR
10893+
* vector to clear).
1088910894
*/
1089010895
if (!apic->apicv_active)
1089110896
kvm_make_request(KVM_REQ_EVENT, vcpu);
10897+
else
10898+
kvm_apic_update_hwapic_isr(vcpu);
1089210899

1089310900
out:
1089410901
preempt_enable();

tools/testing/selftests/kvm/rseq_test.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ int main(int argc, char *argv[])
215215
switch (opt) {
216216
case 'u':
217217
skip_sanity_check = true;
218+
break;
218219
case 'l':
219220
latency = atoi_paranoid(optarg);
220221
break;

tools/testing/selftests/kvm/x86/cpuid_test.c

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,13 +155,28 @@ struct kvm_cpuid2 *vcpu_alloc_cpuid(struct kvm_vm *vm, vm_vaddr_t *p_gva, struct
155155
static void set_cpuid_after_run(struct kvm_vcpu *vcpu)
156156
{
157157
struct kvm_cpuid_entry2 *ent;
158+
struct kvm_sregs sregs;
158159
int rc;
159160
u32 eax, ebx, x;
160161

161162
/* Setting unmodified CPUID is allowed */
162163
rc = __vcpu_set_cpuid(vcpu);
163164
TEST_ASSERT(!rc, "Setting unmodified CPUID after KVM_RUN failed: %d", rc);
164165

166+
/*
167+
* Toggle CR4 bits that affect dynamic CPUID feature flags to verify
168+
* setting unmodified CPUID succeeds with runtime CPUID updates.
169+
*/
170+
vcpu_sregs_get(vcpu, &sregs);
171+
if (kvm_cpu_has(X86_FEATURE_XSAVE))
172+
sregs.cr4 ^= X86_CR4_OSXSAVE;
173+
if (kvm_cpu_has(X86_FEATURE_PKU))
174+
sregs.cr4 ^= X86_CR4_PKE;
175+
vcpu_sregs_set(vcpu, &sregs);
176+
177+
rc = __vcpu_set_cpuid(vcpu);
178+
TEST_ASSERT(!rc, "Setting unmodified CPUID after KVM_RUN failed: %d", rc);
179+
165180
/* Changing CPU features is forbidden */
166181
ent = vcpu_get_cpuid_entry(vcpu, 0x7);
167182
ebx = ent->ebx;

virt/kvm/kvm_main.c

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1749,6 +1749,12 @@ static void kvm_commit_memory_region(struct kvm *kvm,
17491749
kvm_free_memslot(kvm, old);
17501750
break;
17511751
case KVM_MR_MOVE:
1752+
/*
1753+
* Moving a guest_memfd memslot isn't supported, and will never
1754+
* be supported.
1755+
*/
1756+
WARN_ON_ONCE(old->flags & KVM_MEM_GUEST_MEMFD);
1757+
fallthrough;
17521758
case KVM_MR_FLAGS_ONLY:
17531759
/*
17541760
* Free the dirty bitmap as needed; the below check encompasses
@@ -1757,6 +1763,15 @@ static void kvm_commit_memory_region(struct kvm *kvm,
17571763
if (old->dirty_bitmap && !new->dirty_bitmap)
17581764
kvm_destroy_dirty_bitmap(old);
17591765

1766+
/*
1767+
* Unbind the guest_memfd instance as needed; the @new slot has
1768+
* already created its own binding. TODO: Drop the WARN when
1769+
* dirty logging guest_memfd memslots is supported. Until then,
1770+
* flags-only changes on guest_memfd slots should be impossible.
1771+
*/
1772+
if (WARN_ON_ONCE(old->flags & KVM_MEM_GUEST_MEMFD))
1773+
kvm_gmem_unbind(old);
1774+
17601775
/*
17611776
* The final quirk. Free the detached, old slot, but only its
17621777
* memory, not any metadata. Metadata, including arch specific
@@ -2086,7 +2101,7 @@ static int kvm_set_memory_region(struct kvm *kvm,
20862101
return -EINVAL;
20872102
if ((mem->userspace_addr != old->userspace_addr) ||
20882103
(npages != old->npages) ||
2089-
((mem->flags ^ old->flags) & KVM_MEM_READONLY))
2104+
((mem->flags ^ old->flags) & (KVM_MEM_READONLY | KVM_MEM_GUEST_MEMFD)))
20902105
return -EINVAL;
20912106

20922107
if (base_gfn != old->base_gfn)

0 commit comments

Comments
 (0)