Use avic_is_addressable_vcpu() instead of open coding a check on the bounds
of the allocated table for the sanity checks when loading/putting AVIC
state for a vCPU. If KVM botches the allocation, then KVM will already
have performed an OOB write in avic_init_backing_page(), i.e. being super
paranoid in load()/put() doesn't provide meaningful protection in practice.
Cc: Naveen N Rao (AMD) <[email protected]>
Reviewed-by: Naveen N Rao (AMD) <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Add a helper to check if a vCPU is addressable by AVIC hardware, i.e. has
an APIC ID that fits in the physical ID table, and use the more paranoid
helper when determining if a vCPU is compatible with AVIC when initializing
the vCPU. KVM is supposed to reject vCPU creation if the vCPU's ID is
greater than or equal to max_vcpu_ids, i.e. simply checking the
architectural maximum *should* suffice. But piecing together why this is
safe is unnecessarily difficult, and there is no meaningful downside to
being extra cautious.
Suggested-by: Naveen N Rao (AMD) <[email protected]>
Reviewed-by: Naveen N Rao (AMD) <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Explicitly flush caches after intra-host migration before clearing "SEV
active" on the source VM, as doing cache maintenance afterwards creates a
tiny window where memory reclaim could return memory to the host without
performing a cache flush, e.g. as pointed out by Sashiko:
CPU1 in sev_migrate_from():
src->active = false;
CPU2 running concurrent unmap:
Since active is false, the automatic cache flush in
sev_guest_memory_reclaimed is skipped.
The host frees and reallocates the page.
CPU1 in sev_migrate_from():
sev_writeback_caches(src_kvm);
Executes a hardware cache flush (wbnoinvd), which writes the guest's old
dirty ciphertext over the new page owner's data.
Fixes: 93de2a6a4b ("KVM: SEV: Do cache maintenance on the source VM during intra-host migration")
Cc: [email protected]
Reported-by: Sashiko Bot <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Nullify have_run_cpus when freeing the mask, particularly in the error path
of __sev_guest_init(), so that KVM doesn't have to subtly use sev->active
to track whether or not the mask has been freed. As pointed out by Sashiko,
blindly freeing the mask in sev_vm_destroy() results in a double-free if
the mask is freed if __sev_guest_init() fails.
Throw the logic in a helper as nullifying the pointer is frustratingly
difficult and weird due to have_run_cpus being a single-entry array when
CPUMASK_OFFSTACK=n. Deliberately don't use CPUMASK_VAR_NULL, as it's not
directly assignable when the cpumask is on-stack, e.g. requires using a
local variable and a memcpy(), which is beyond ridiculous. Furthermore,
while clearing the on-stack bitmask is an unnecessary and arguably unwanted
side effect, KVM absolutely relies on '0' being the "null" value given that
the struct is zero-allocated.
Opportunistically add an alloc() helper to pair with free(); there are just
enough call sites to make doing so worthwhile.
Fixes: 12c1f6e03f ("KVM: SEV: Free have_run_cpus during VM destruction even if VM is no longer SEV")
Cc: [email protected]
Reported-by: Sashiko Bot <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Add a selftest to verify that KVM intercepts x2APIC MSR accesses for L1
after APICv is inhibited while L2 is active. This is a regression test for
an AVIC bug where KVM would skip updating x2APIC MSR intercepts while L2
is active, thus giving L1 access to a wide swath of L0's x2APIC surface.
Signed-off-by: Sean Christopherson <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Force a refresh of the vmcs02 MSR bitmap during nested VM-Enter if the
runtime eVMCS controls (pin, primary, secondary, etc.) are being updated.
If L1 isn't intercepting TPR writes, runs L2 with TPR virtualization, and
then runs the same L2 with TPR virtualization disabled, KVM will fail to
refresh msr_bitmap02 and leave TPR in passthrough mode even though TPR
virtualization is disabled. I.e. failure to refresh the bitmap lets L2 (or
L1 by proxy) read and write L0's TPR.
Fixes: 502d2bf5f2 ("KVM: nVMX: Implement Enlightened MSR Bitmap feature")
Cc: [email protected]
Reviewed-by: Vitaly Kuznetsov <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Use the MSR permission bitmap of the active VMCB instead of assuming that
KVM is always using vmcb02's bitmap when L2 is active, as KVM uses msrpm02
if and only if L1 wants to intercept MSR accesses, i.e. if and only if KVM
needs to merge msprm01 with msrpm12.
Don't bother tracking the virtual address of the bitmap that's being used,
as __va() is cheap on x86, and caching the virtual address would introduce
yet another source of potentially stale information.
Fixes: b2ac58f905 ("KVM/SVM: Allow direct access to MSR_IA32_SPEC_CTRL")
Cc: [email protected]
Reported-by: Stefan Teodorescu <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Don't (re)read PERF_CNTR_GLOBAL_CTL from hardware on a failed VMRUN, as the
purpose of the read is to synchronize KVM's cache with any writes done by
the guest, and the guest can't possibly have modified the MSR if it never
got a chance to run.
Signed-off-by: Sean Christopherson <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Don't mark the ASID as dirty in the VMCB when requesting a TLB flush via
control.tlb_ctl. Per "15.15.3 VMCB Clean Field" of the July 2026, Revision
3.45 version of the APM:
The following are explicitly not cached and not represented by Clean bits:
* TLB_Control
Fixes: 7e8e6eed75 ("KVM: SVM: Move asid to vcpu_svm")
Suggested-by: Yosry Ahmed <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Leave control.erap_ctl and control.clean as-is in the VMCS if VMRUN fails,
because as per AMD:
there's no explicit architectural guarantee about the behavior in the
presence of VMRUN failures. So the best thing to do would be to assume
that if VMRUN fails, the actions requested in the control fields may not
have been performed.
Cc: [email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Don't reset the VMCB's TLB control back to "do nothing" on a failed VMRUN,
as empirical testing shows that the CPU performs the requested TLB flush if
and only if VMRUN is successful, i.e. clearing TLB control on a failed
VMRUN effectively drops a TLB flush.
Explicitly track the need to flush all ASIDs on a per-CPU basis, as the
ASID reuse condition is tied to the pCPU, not to the vCPU. As a bonus,
this also obviates the need to avoid clobbering FLUSH_ALL_ASID with
TLB_CONTROL_FLUSH_ASID, e.g. in svm_flush_tlb_asid().
Deliberately don't bother saving/restoring the "old" tlb_ctl on failure,
in quotes because it's not exactly the old tlb_ctl, it's the tlb_ctl from
after pre_svm_run(), but before updating tlb_ctl for flush_all_asids. If
VMRUN fails and TLB_CONTROL_FLUSH_ALL_ASID is forced, then the next
successful run of the VMCB *may* unnecessarily flush all ASIDs, which
strictly speaking could result in noisy neighbor issues. However, the
fact that new_asid() is already guest-triggerable, because of KVM's flawed
behavior of clearing the ASID on emulated INIT, means that a guest can
already trigger a flush of all ASIDs at roughly the same rate. And once
KVM stops clobbering the ASID on emulated INIT, *or* assigns a static ASID
to each vCPU, this flaw goes away.
Fixes: 38e5e92fe8 ("KVM: SVM: Implement Flush-By-Asid feature")
Cc: [email protected]
Reported-by: Stefan Teodorescu <[email protected]>
Suggested-by: Yosry Ahmed <[email protected]>
Cc: Tom Lendacky <[email protected]>
Cc: Jim Mattson <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
When walking shadow page tables, immediately terminate the walk if the root
is a "dummy" root, i.e. a root whose top-level page table is backed by the
zero page, but otherwise doesn't exist. If memslot creation races with a
stage-2 page fault (EPT violation or #NPF) from L2, then if the stars align,
KVM will attempt to walk shadow page tables using the zero page and hit a
NULL pointer deref.
BUG: kernel NULL pointer dereference, address: 0000000000000021
#PF: supervisor read access in kernel mode
#PF: error_code(0x0000) - not-present page
PGD 0 P4D 0
Oops: Oops: 0000 [#1] SMP
CPU: 30 UID: 1000 PID: 941 Comm: qemu Not tainted 7.2.0-rc2-1b731e5ded48-next-vm #1741 PREEMPTLAZY
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
RIP: 0010:__kvm_mmu_invalidate_addr+0xea/0x210 [kvm]
Call Trace:
<TASK>
kvm_mmu_invalidate_addr+0x92/0xe0 [kvm]
__kvm_inject_emulated_page_fault+0x67/0x80 [kvm]
ept_page_fault+0x160/0x850 [kvm]
kvm_mmu_do_page_fault+0x102/0x1f0 [kvm]
kvm_mmu_page_fault+0x8e/0x6b0 [kvm]
vmx_handle_exit+0x163/0x640 [kvm_intel]
kvm_arch_vcpu_ioctl_run+0x960/0x2120 [kvm]
kvm_vcpu_ioctl+0x2c7/0x970 [kvm]
__x64_sys_ioctl+0x90/0xd0
do_syscall_64+0x67/0x5f0
entry_SYSCALL_64_after_hwframe+0x4b/0x53
RIP: 0033:0x7f606d0b53bb
Opportunistically harden the shadow walks against fully invalid roots, but
WARN, as all callers are expected/required to pre-check for a valid root.
Don't WARN in the dummy root case as the whole point of using a dummy root
is to provide a root that's valid enough to enter the guest, i.e. it should
Just Work for all flows except those that *need* to know about dummy roots.
Alternatively, KVM could allocate a dedicated page and associated shadow
page structure for the dummy root, which is very tempting as it such an
approach should be more resilient against unexpected behavior. But that
would be a much larger and thus riskier change than simply terminating
walks of dummy roots.
Fixes: 0e3223d8d0 ("KVM: x86/mmu: Use dummy root, backed by zero page, for !visible guest roots")
Reported-by: Gabriel Schneider <[email protected]>
Cc: [email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Now that KVM holds kvm->lock for the entirety of vCPU creation, from when
created_vcpus is incremented until the new vCPU is fully onlined, WARN if
the impossible happens and KVM somehow sees a discrepancy between the
number of vCPUs "created" and "onlined".
Signed-off-by: Sean Christopherson <[email protected]>
Tested-by: Jean-Christophe Guillain <[email protected]>
Tested-by: Naveen N Rao (AMD) <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Now that KVM uses kvm_get_vcpu_by_id() to check for an existing vCPU ID
before doing any meaningful work, which was made possible by holding
kvm->lock for the entirety of vCPU creation, revert the now-redundant
"early" vCPU ID tracking. The claims about the impact of kvm->vcpu_ids on
the memory footprint were a wee bit wrong: the worst case scenario isn't
256 bytes per VM, it's 256 "unsigned longs" per VM, i.e. 2048 bytes per VM.
Increasing the size of "struct kvm" by 2048 nearly doubled the total size
on many architectures, and tripped x86's KVM_SANITY_CHECK_VM_STRUCT_SIZE,
which was added to detect this *exact* scenario, where a single change
significantly increased the size of "struct kvm". I.e. attempting to build
KVM with CONFIG_DEBUG_KERNEL=n fails on x86 (the build failures got missed
because all build bots apparently test only CONFIG_DEBUG_KERNEL=y kernels,
and maintainers' test flows were similarly lacking).
This reverts commit 97d65b544f.
Fixes: 97d65b544f ("KVM: Check for duplicate vcpu_id as early as possible")
Reported-by: Jean-Christophe Guillain <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]
Reported-by: Paweł S <[email protected]>
Closes: https://lore.kernel.org/all/CABD%3DWFOS4j4hDv%2BpW-eEM9HAM2q2GY_iYdAG%[email protected]
Tested-by: Jean-Christophe Guillain <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
Tested-by: Naveen N Rao (AMD) <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Now that kvm->lock is held for the entirety of vCPU creation, check for a
conflicting vCPU ID at the begnning of vCPU creation, before the arch
precreate() hook is invoked. This will allow reverting commit 97d65b544f
("KVM: Check for duplicate vcpu_id as early as possible"). For now, keep
the redundant vcpu_ids tracking as a sanity check.
No functional change intended (absent KVM bugs, checking vcpu_ids and
walking kvm_get_vcpu_by_id() should yield the same result).
Signed-off-by: Sean Christopherson <[email protected]>
Tested-by: Jean-Christophe Guillain <[email protected]>
Tested-by: Naveen N Rao (AMD) <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
When creating a vCPU, don't drop kvm->lock to when doing the bulk of actual
vCPU creation, as allowing multiple vCPUs to be created in parallel adds
significant complexity in KVM (as evidenced by the many related bugs), and
all known VMMs fully serialize vCPU creation. Remove all manually locking
of kvm->lock from kvm_arch_vcpu_{post,}create() for obvious reasons.
For many years, "everyone" has assumed that dropping kvm->lock was done for
performance reasons optimization, e.g. to allow userspace to create all
vCPUs concurrently for latency purposes. But as above, no known VMM does
that. Looking at the history of this code, before commit 11ec280471
("KVM: Convert vm lock to a mutex"), kvm->lock was a spinlock. I.e. KVM
*had* to drop kvm->lock when doing the bulk of vCPU creation, otherwise KVM
couldn't do normal memory allocations. When kvm->lock got turned into a
mutex for unrelated reasons, no one took advantage updated of the change to
simplify vCPU creation. And 19 years later, everyone just assumed that KVM
continued to deal with the complexity for performance reasons.
Furthermore, naively parallelizing vCPU creation in userspace is likely a
net negative due to the overheads of task creation. Unless a VMM carefully
avoids the extra overhead related to parallelization, e.g. spawns each
vCPU's thread before creating the vCPU, creating vCPUs concurrently is a
net *negative* up until about ~64 vCPUs, after which the times are a wash.
The absolute speed of light _is_ faster if KVM doesn't hold kvm-lock, but
at vCPU counts of ~16 or less, it's probably in the noise when considering
total VM creation time, as the added latency is less than 1ms up until 16
or so vCPUs.
On top of all that, KVM has had a *lot* of fatal bugs (most often found by
syzkaller) related to vCPUs being created while trying to do per-VM
operations (basically, see every flow that locks all vCPUs). I.e. the
parallel vCPU creation "support" is actively harmful as the only "use case"
is for misbehaving userspace to exploit KVM bugs.
Serializing vCPU creation will allow reverting commit 97d65b544f ("KVM:
Check for duplicate vcpu_id as early as possible"), which had "minor" math
error: the worst case scenario isn't "256 bytes per VM", it's "256 unsigned
longs per VM", i.e. 2048 bytes per VM, which doubles the size of each VM
and pushes several architectures into order-1 allocations.
Signed-off-by: Sean Christopherson <[email protected]>
Tested-by: Jean-Christophe Guillain <[email protected]>
Tested-by: Naveen N Rao (AMD) <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Use kvm_is_vcpu_creation_in_progress() instead of an open-coded equivalent
during AIA initialization. Unlike similar vGIC code in arm64, the relevant
RISC-V code runs under kvm->lock, i.e. can use the standard API without
hitting lockdep false positive.
No functional change intended.
Signed-off-by: Sean Christopherson <[email protected]>
Tested-by: Jean-Christophe Guillain <[email protected]>
Tested-by: Naveen N Rao (AMD) <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Now that KVM's APIs for locking all vCPUs return -EBUSY if vCPU creation is
in-progress, drop the manual check for the same from vGIC creation, and
update the comments accordingly.
Note, while KVM arm64 guards many vGIC operations with its arch-specific
config_lock, holding kvm->lock is sufficient to guarantee a stable result
for "is vCPU creation in-progress". So, no functional change intended.
Note #2, the open coded check in vgic_init() is racy when called without
kvm->lock held, e.g. via vgic_lazy_init(). I.e. that check needs to stay
open coded to avoid triggering a lockdep assert. Whether or not the race
is "fine" is a problem for a different day.
Signed-off-by: Sean Christopherson <[email protected]>
Tested-by: Jean-Christophe Guillain <[email protected]>
Tested-by: Naveen N Rao (AMD) <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Reject locking of all vCPUs if vCPU creation is in-progress, i.e. if the
number of "created" vCPUs doesn't match the number of "onlined" vCPUs.
It's simply not possible to guarantee that KVM has truly locked all vCPUs
if one or more vCPUs are actively being created. Holding kvm->lock does
prevent in-flight vCPUs from being fully onlined, but it's infeasible for
common KVM to know whether or not that provides sufficient protection.
In practice, this is likely a minor bug fix for the ARM and RISC-V usage of
kvm_trylock_all_vcpus(), and a glorified nop for everything else. E.g.
ARM's kvm_timer_vcpu_init() can race kvm_vm_ioctl_set_counter_offset() with
respect to observing KVM_ARCH_FLAG_VM_COUNTER_OFFSET.
Opportunistically drop x86's existing manual checks on vCPU creation being
in-progress as all of x86's checks immediately precede or follow locking of
all vCPUs. Leave arm64 and RISC-V alone for the moment, as their checks
aren't as obviously redundant/equivalent.
Signed-off-by: Sean Christopherson <[email protected]>
Tested-by: Jean-Christophe Guillain <[email protected]>
Tested-by: Naveen N Rao (AMD) <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Manually perform cache maintenance on the source VM during intra-host
migration to ensure no stale data is left in CPU caches after the VM is
destroyed. Because the source VM is "converted" to a non-SEV VM, KVM's
memory reclaim flows won't trigger cache maintenance, e.g. when all guest
memory is reclaimed in response to detaching from the mmu_notifier.
Note, relying on the destination VM to do cache maintenance isn't an option
as KVM doesn't require identical guest memory configurations, i.e. the
source VM may have access to memory that the destination VM does not.
Enforcing equivalent memory configurations is infeasible, as it would
require a *deep* comparison of memslots, e.g. to verify that not only are
the memslot identical, but what the memslots point at is also identical.
Fixes: b56639318b ("KVM: SEV: Add support for SEV intra host migration")
Cc: [email protected]
Reported-by: Stefan Teodorescu <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Unconditionally free SEV's "have run CPUs" cpumask in the VM destroy path,
i.e. even for what appear to be non-SEV VMs, as an SEV VM becomes a non-SEV
VM if its state is intra-host migrated. Alternatively, the mask could be
freed in sev_migrate_from() when "converting" the source VM, but that gets
annoying because ideally KVM would nullify the mask to guard against UAF,
and nullifying the mask would need be conditioned on CPUMASK_OFFSTACK=y.
Freeing the mask during sev_migrate_from() is also not robust against other
KVM bugs, though that's kind of a moot point since any such bugs would show
up even if sev->active is never set. I.e. KVM must get that side of things
correct. But, that's not a great reason to add more code just to make
things marginally less robust.
Fixes: 6f38f8c574 ("KVM: SVM: Flush cache only on CPUs running SEV guest")
Cc: [email protected]
Reported-by: Stefan Teodorescu <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
Message-ID: <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Fill kvm_run with "internal error, emulation" in the common error handling
paths for getting nested state pages, as requiring each check to manually
fill kvm_run is error prone and requires a non-trivial amount of copy+paste.
Specifically, both SVM and VMX fail to fill kvm_run if load_pdptrs() fails,
and SVM fails to fill kvm_run if kvm_hv_verify_vp_assist() fails. If those
flows fail, the *best* case scenario is that KVM will exit to userspace
with KVM_EXIT_UNKNOWN. The worst case scenario is that KVM exits with a
stale exit_reason and confuses userspace.
Note, SVM never exits to userspace if something goes sideways when dealing
with vmcb12 assets while emulating VMRUN, i.e. lack of SVM-specific code is
not a bug.
Fixes: 0f85722341 ("KVM: nVMX: delay loading of PDPTRs to KVM_REQ_GET_NESTED_STATE_PAGES")
Fixes: 232f75d3b4 ("KVM: nSVM: call nested_svm_load_cr3 on nested state load")
Fixes: 3f4a812edf ("KVM: nSVM: hyper-v: Enable L2 TLB flush")
Cc: [email protected]
Reported-by: Jinwoo Lee <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]
Reported-by: Stefan Teodorescu <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Re-pend GET_NESTED_STATE_PAGES before exiting to userspace if getting the
nested pages fails in the KVM_RUN path. If userspace re-runs the vCPU, and
vmcs02 holds valid PFNs from the *previous* run of L2, then KVM could
re-enter L2 with stale, unpinned PFNs mapped into e.g. the vAPIC page.
Note, both SVM and VMX (as of commit 11722439fb ("KVM: nVMX: Ensure
KVM_REQ_GET_NESTED_STATE_PAGES is cleared on VM-Exit") ensure the request
is cleared on VM-Exit (including the "forced" case), i.e. there is no risk
of double-mapping due to emulated VMLAUNCH/VMRESUME/VMRUN *and* the request
trying to map the nested pages.
Fixes: 671ddc700f ("KVM: nVMX: Don't leak L1 MMIO regions to L2")
Cc: [email protected]
Reported-by: Jinwoo Lee <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]
Reported-by: Stefan Teodorescu <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Drop "support" for passing a NULL @data/@kvm_pmu param when getting guest
MSRs. KVM, the only in-tree user, unconditionally passes a non-NULL
pointer, and carrying code that suggests @data may be NULL is confusing,
e.g. incorrectly implies that there are scenarios where KVM doesn't pass
a PMU context.
Fixes: 8183a538cd ("KVM: x86/pmu: Add IA32_DS_AREA MSR emulation to support guest DS")
Signed-off-by: Sean Christopherson <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Reviewed-by: Jim Mattson <[email protected]>
Reviewed-by: Dapeng Mi <[email protected]>
Link: https://patch.msgid.link/[email protected]
When filling the list of MSRs to be loaded by KVM on VM-Enter and VM-Exit,
load the guest values for DS_AREA and (conditionally) MSR_PEBS_DATA_CFG if
and only if PEBS will be active in the guest, i.e. only if a PEBS record
may be generated while running the guest. As shown by the !pebs_ept path,
it's perfectly safe to run with the host's DS_AREA, so long as PEBS-enabled
counters are disabled via PERF_GLOBAL_CTRL.
Omitting DS_AREA and MSR_PEBS_DATA_CFG when PEBS is unused saves two MSR
writes per MSR on each VMX transition, i.e. eliminates two/four pointless
MSR writes on each VMX roundtrip when PEBS isn't being used by the guest.
Fixes: c59a1f106f ("KVM: x86/pmu: Add IA32_PEBS_ENABLE MSR emulation for extended PEBS")
Signed-off-by: Sean Christopherson <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Reviewed-by: Jim Mattson <[email protected]>
Reviewed-by: Dapeng Mi <[email protected]>
Link: https://patch.msgid.link/[email protected]
When filling the list of MSRs to be loaded by KVM on VM-Enter and VM-Exit,
*never* insert an entry for PEBS_ENABLED if the CPU properly isolates PEBS
events, in which case disabling counters via PERF_GLOBAL_CTRL is sufficient
to prevent unwanted PEBS events in the guest (or host). Because perf loads
PEBS_ENABLE with the unfiltered cpu_hw_events.pebs_enabled, i.e. with both
host and guest masks, there is no need to load different values for the
guest versus host, perf+KVM can and should simply control which counters
are enabled/disabled via PERF_GLOBAL_CTRL.
Avoiding touching PEBS_ENABLED "fixes" a bug where PEBS_ENABLED can end up
with "stuck" bits if a PEBS event is throttled between generating the list
and actually entering the guest (Intel CPUs can't arbtitrarily block NMIs).
Fixes in quotes because leaving PEBS_ENABLED as-is doesn't fix the
underlying problem of perf (via PMIs) being able to modify state after the
perf<=>KVM handoff.
But not writing PEBS_ENABLED is desirable no matter what, as stating the
obvious, leaving PEBS_ENABLED as-is avoids three MSR writes on every VMX
transition: one each on entry/exit, and one more explicit WRMSR to zero
PEBS_ENABLED before VM-Entry (KVM assumes the only reason PEBS_ENABLED is
in the load list is if the CPU lacks PEBS isolation and thus needs a
quiescent period).
Opportunistically add comments to (better) explain the rules for generating
the set of PEBS counters that will be active while the guest is running,
along with a FIXME for the suspected hack-a-fix where perf disables guest
PEBS if _any_ PEBS event is configured to count in the host (commit
854250329c ("KVM: x86/pmu: Disable guest PEBS temporarily in two rare
situations") doesn't explain the motivation, at all).
Fixes: c59a1f106f ("KVM: x86/pmu: Add IA32_PEBS_ENABLE MSR emulation for extended PEBS")
Signed-off-by: Sean Christopherson <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Reviewed-by: Dapeng Mi <[email protected]>
Link: https://patch.msgid.link/[email protected]
When reinstating PEBS counters into PERF_GLOBAL_CTRL for a KVM guest, mask
the value with perf's desired/original PERF_GLOBAL_CTRL value to ensure
KVM doesn't unintentionally set reserved bits in PERF_GLOBAL_CTRL. E.g.
if the guest's PEBS_ENABLE value had bit 63, "Enable Precise Store", set,
then using the raw guest PEBS value would propagate bit 63 to the guest's
PERF_GLOBAL_CTRL value (which thankfully would be a failed VM-Entry, not
a VMX Abort).
The only reason this bug isn't reachable is because KVM doesn't support
"Enable Precise Store" (which is probably a KVM bug?), i.e. bit 63 can't
be set in kvm_pmu->pebs_enable and thus not in arr[pebs_enable].guest. In
other words, this _should_ be a glorified NOP in the current code base.
Fixes: c59a1f106f ("KVM: x86/pmu: Add IA32_PEBS_ENABLE MSR emulation for extended PEBS")
Signed-off-by: Sean Christopherson <[email protected]>
Signed-off-by: Peter Zijlstra (Intel) <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Reviewed-by: Dapeng Mi <[email protected]>
Link: https://patch.msgid.link/[email protected]
Use kvm_test_request() instead of kvm_check_request() when querying
KVM_REQ_VM_DEAD, i.e. don't clear KVM_REQ_VM_DEAD, as the entire purpose
of KVM_REQ_VM_DEAD is to prevent the vCPU from enterring the guest ever
again, even if userspace insists on redoing KVM_RUN.
Ensuring KVM_REQ_VM_DEAD is never cleared will allow relaxing KVM's rule
that ioctls can't be invoked on dead VMs, to only disallow ioctls if the
VM is bugged, i.e. if KVM hit a KVM_BUG_ON().
Opportunistically add compile-time assertions to guard against clearing
KVM_REQ_VM_DEAD through the standard APIs.
Reviewed-by: Kai Huang <[email protected]>
Acked-by: Marc Zyngier <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Use __GLIBC__ in the hardware disable test to detect when selftests are
being built/linked against glibc and thus pthread_attr_setaffinity_np() is
(hopefully) available. As pointed out by Sashiko and Hisam, _GNU_SOURCE is
effectively a "request" macro to enable functionality, whereas __GLIBC__ is
an announcement of support and selftests' idiomatic way of guarding code
that's specific to glibc.
Fixes: 496779b549 ("KVM: selftests: Pre-set threads affinity in hardware disable test when possible")
Reported-by: Sashiko Bot <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]
Suggested-by: Hisam Mehboob <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
To reduce the runtime of the PMU counters test by an order of magnitude,
create one VM per category of test (arch events, GP PMCs, fixed PMCs),
with N vCPUs per VM, where 'N' is the number of testscases to run per
category. The vast majority of the test's runtime is spent creating VMs,
because while creating a VM only takes a few milliseconds, the total time
adds up when creating hundreds of VMs. The only reason the test creates
so many VMs is because KVM disallows changing the virtual PMU model after
a vCPU runs, so rather than create an entirely new VM just to get a "fresh"
vCPUs, simply use a different vCPU.
On an Emerald Rapids host, this reduces the runtime from 75+ seconds to
less than 6 seconds.
Opportunistically fix a typo in the comment about unavailable_masks pointed
out by Like Xu[1].
Link: https://lore.kernel.org/all/[email protected][1]
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Explicitly clear role.invalid when deriving a child shadow page's role from
its parent to harden against bugs elsewhere in KVM, as violating KVM's
invariant that invalid pages are NOT on the list of active MMU pages leads
to use-after-free due to __kvm_mmu_prepare_zap_page() using list_add()
instead of list_move() when processing an invalid shadow page, i.e. makes a
bad situation far worse.
Yell loudly if the parent is invalid, as it means KVM has missed a validity
check, i.e. KVM is attempting to map memory using an invalid/obsolete root,
but continue on as the child is otherwise still a valid shadow page.
==================================================================
BUG: KASAN: slab-use-after-free in __kvm_mmu_get_shadow_page+0x1817/0x1860 [kvm]
Write of size 8 at addr ff11000153dd1368 by task repro/853
CPU: 1 UID: 1000 PID: 853 Comm: repro Not tainted 7.2.0-rc2-3aec122bdcaf-next-vm #5 PREEMPT
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
Call Trace:
<TASK>
dump_stack_lvl+0x4b/0x70
print_report+0x153/0x49c
kasan_report+0xbc/0xf0
__kvm_mmu_get_shadow_page+0x1817/0x1860 [kvm]
mmu_alloc_root+0x141/0x320 [kvm]
kvm_mmu_load+0x612/0x20f0 [kvm]
kvm_arch_vcpu_ioctl_run+0x3dd5/0x6150 [kvm]
kvm_vcpu_ioctl+0x5e4/0x10d0 [kvm]
__x64_sys_ioctl+0x131/0x1b0
do_syscall_64+0x67/0x5f0
entry_SYSCALL_64_after_hwframe+0x4b/0x53
</TASK>
Allocated by task 853:
kasan_save_stack+0x20/0x40
kasan_save_track+0x14/0x30
__kasan_slab_alloc+0x5f/0x70
kmem_cache_alloc_noprof+0xfe/0x2e0
__kvm_mmu_topup_memory_cache+0x135/0x530 [kvm]
paging64_page_fault+0x318/0x1e30 [kvm]
kvm_mmu_do_page_fault+0x21d/0x630 [kvm]
kvm_mmu_page_fault+0x18c/0x17b0 [kvm]
kvm_arch_vcpu_ioctl_run+0x1f35/0x6150 [kvm]
kvm_vcpu_ioctl+0x5e4/0x10d0 [kvm]
__x64_sys_ioctl+0x131/0x1b0
do_syscall_64+0x67/0x5f0
entry_SYSCALL_64_after_hwframe+0x4b/0x53
Freed by task 853:
kasan_save_stack+0x20/0x40
kasan_save_track+0x14/0x30
kasan_save_free_info+0x3b/0x60
__kasan_slab_free+0x43/0x70
kmem_cache_free+0xe2/0x400
kvm_mmu_commit_zap_page.part.0+0x1e2/0x310 [kvm]
kvm_mmu_free_roots+0x283/0x560 [kvm]
kvm_arch_vcpu_ioctl_run+0x33c8/0x6150 [kvm]
kvm_vcpu_ioctl+0x5e4/0x10d0 [kvm]
__x64_sys_ioctl+0x131/0x1b0
do_syscall_64+0x67/0x5f0
entry_SYSCALL_64_after_hwframe+0x4b/0x53
Reported-by: Hyunwoo Kim <[email protected]>
Fixes: a770f6f28b ("KVM: MMU: Inherit a shadow page's guest level count from vcpu setup")
Cc: [email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
To significantly reduce the PMU counter test's runtime, without sacrificing
test coverage in the aggregate, test a random GP counter in the arch events
testcase instead of testing every possible GP counter. Testing every PMC
in every run of the test significantly increases the runtime of the test,
without providing an equivalent increase in validation coverage, as the
odds of a KVM having a bug that only affected a subset of counters and only
when testing all other counters are extremely low.
Opportunistically clean up kvm_random_u64_in_range() to eliminate
unnecessary newlines.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Add myself as a reviewer for overall KVM, as I have been helping shepherd
a decent chunk of the virt/kvm commits into the kernel over the last few
years, and I expect to continue doing so. Of the 270 non-merge commits
that have been made to virt/kvm since v6.6, I have committed 127 (~47%)
and Paolo has committed 117 (~43.3%), with all but 5 of the remaining 26
coming from non-KVM folks, e.g. from treewide changes.
Note, this intended to purely be an efficiency and quality of life
improvement, for both contributors and myself. There is no implied right
of succession or anything of the sort.
Cc: Marc Zyngier <[email protected]>
Cc: Oliver Upton <[email protected]>
Cc: Tianrui Zhao <[email protected]>
Cc: Bibo Mao <[email protected]>
Cc: Huacai Chen <[email protected]>
Cc: Anup Patel <[email protected]>
Cc: Christian Borntraeger <[email protected]>
Cc: Janosch Frank <[email protected]>
Cc: Claudio Imbrenda <[email protected]>
Acked-by: Janosch Frank <[email protected]>
Acked-by: Anup Patel <[email protected]>
Acked-by: Bibo Mao <[email protected]>
Acked-by: Paolo Bonzini <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Officially "reward" David with guest_memfd responsibilities and authority,
as he's been instrumental in pushing guest_memfd forward, and his ongoing
input and feedback, especially from an mm/ perspective, is and will be
critical to keeping us virtualization folks from going too far off the
rails.
Cc: David Hildenbrand <[email protected]>
Acked-by: David Hildenbrand (Arm) <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
To better reflect how guest_memfd has been maintained for the last few
years, and to prepare for significant upcoming growth and expansion, add a
dedicated MAINTAINERS entry for KVM's guest_memfd, with Paolo and myself
(Sean) as co-maintainers.
List both the KVM and KVM x86 git trees as authoritative repositories, as
any given guest_memfd change has a equal odds of hitting KVM x86 or not.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Extract the gmem function declarations out of kvm_mm.h and into a dedicated
header, guest_memfd.h. This will allow creating a MAINTAINERS entry for
guest_memfd without having to rely on content pattern matching.
Reviewed-by: Ackerley Tng <[email protected]>
Acked-by: David Hildenbrand (Arm) <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Add the kvm-x86 tree to all KVM x86 entries so that humans, bots, and
everything in between can more easily find KVM x86 (and some broader KVM)
patches that are targeted for the next kernel release, but haven't yet made
their way to the main KVM tree. While the KVM x86 patch flow is documented
in Documentation/process/maintainer-kvm-x86.rst, that's very much one-off
documentation; random passersbys likely don't know it exists, and it's not
suitable for any workflow that's at all automated.
Reported-by: Yosry Ahmed <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
In the invalid nVMX guest state test, extract the creation of the VM and
initial running of the vCPU to get to L2 into helpers so that the common
code can be reused to extend the test to also cover RSM.
Eliminate the unnecessary global "vm", and opportunistically free the VM
after the testcase as there's zero reason not to.
Opportunistically assert that L2 is never resumed after the I/O exit to L1,
e.g. to guard against false passes.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Use port 0x80 instead of 0x2000 to trigger an L2 => L0 VM-Exit in the nVMX
invalid guest state test, so that the test can be expanded to cover RSM in
the L2 => SMI => L2 path without having to manually encode a MOV to DX (the
immediate form of IN only supports an imm8).
Use port 0x80 as it's a well-known port and is used by other tests, e.g. in
the coalesced I/O test.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Rework .vcpu_pre_run() into a more specific .vcpu_needs_initialization() to
consolidate the SNP and TDX control flows, and to eliminate the potentially
confusing almost-collision between svm_vcpu_pre_run() and pre_sev_run(): the
former is SEV specific, but is pre-KVM_RUN, whereas the latter is pre-VMRUN.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Synthesize SHUTDOWN (for L1) if L2 requires unhandleable emulation after
loading guest state from SMRAM during RSM to prevent a misbehaving L1 (or
userspace via L1) from tripping the sanity check that KVM doesn't try to
cancel a pending nested VM-Enter. If SMRAM is modified such that RSM will
load what should be impossible state for L2, then KVM will detect that it
needs to emulate the current code stream and will abort VM-Entry to L2.
And because KVM (rightly) expects such a scenario to be impossible, KVM
WARNs and bugs the VM.
__ret && !(vcpu->kvm)->vm_bugged
WARNING: arch/x86/kvm/vmx/vmx.c:6741 at vmx_handle_exit+0x65/0x790 [kvm_intel], CPU#13: vmx_invalid_nes/2902
Modules linked in: kvm_intel kvm irqbypass [last unloaded: kvm]
CPU: 13 UID: 1000 PID: 2902 Comm: vmx_invalid_nes Tainted: G W 7.2.0-rc2 #124 PREEMPT
Tainted: [W]=WARN
Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 0.0.0 02/06/2015
RIP: 0010:vmx_handle_exit+0x65/0x790 [kvm_intel]
Call Trace:
<TASK>
kvm_arch_vcpu_ioctl_run+0xdf8/0x1d00 [kvm]
kvm_vcpu_ioctl+0x2d5/0x960 [kvm]
__x64_sys_ioctl+0x8a/0xd0
do_syscall_64+0xb7/0x570
entry_SYSCALL_64_after_hwframe+0x4b/0x53
</TASK>
Alternatively, KVM could suppress the WARN for the RSM case, but that would
still leave the vCPU in a "bad" state that KVM doesn't know how to handle
(which is also why KVM rejects attempts to do KVM_RUN when the vCPU is
loaded with invalid state). And architecturally, the Intel SDM explicitly
states that RSM leads to shutdown if the CPU detects invalid state.
Fixes: 2bb8cafea8 ("KVM: vVMX: signal failure for nested VMEntry if emulation_required")
Reported-by: Hao Zhang <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Expose VMX's check for unhandleable emulation as its own kvm_x86_ops hook,
and move the actual pre-KVM_RUN check into common x86. This will allow
sharing the core logic with KVM's RSM emulation without needed to add a
post-RSM hook, and is a step towards removing the .vcpu_pre_run() hook
entirely.
Alternatively, KVM could provide a post-RSM hook as mentioned, but pre/post
hooks tend to be unwieldy as the exact "timing" of the call often matters
greatly. E.g. in this case, the call must slot in exactly between loading
guest state from SMRAM and the hack to force the vCPU out of L2 on SHUTDOWN.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Add and use KVM wrappers for pthread_{g,s}etaffinity_np() so that selftests
don't need to manually assert that the syscalls succeeded, and so that they
don't need to manually pass in sizeof(cpu_set_t) for the size.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Rename the global constants in the hardware disable test to better
capture what they control, and open code the literal number of sleeping
tasks and iterations instead of using power-of-2 math to express values
that are 100% arbitrary.
Opportunistically tag the global semaphore with "static".
No functional change intended.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Deduplicate test code that does cancel()+join() on pthreads by adding a
collection of syscall wrappers. The tests that asynchronously cancel
pthreads, i.e. use PTHREAD_CANCEL_ASYNCHRONOUS, are particularly ugly,
especially since it's not immediately obvious why those tests assert on
the thread being CANCELED, whereas others do not.
No functional change intended.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Add and use KVM wrappers for pthread_{cancel,join}() so that selftests
don't need to manually assert that the syscalls succeeded.
Note, the vast majority tests don't actually assert success, but they all
obviously rely on the syscall to succeed.
Other than explicitly failing if a syscall fails, no functional change
intended.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Add and use a KVM wrapper for pthread_create() syscall so that selftests
don't need to manually assert that the syscall succeeded.
Note, most tests don't actually assert success, but they all obviously
rely on the syscall to succeed.
Other than explicitly failing if pthread_create() fails, no functional
change intended.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Drop all of the code that is unreachable in the hardware disable test, as
it adds a lot of noise that makes the test seem far more complicated than
it actually is.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Affine the worker threads to random CPUs in the hardware disable test, and
honor the starting CPU set in the process. Hardcoding to CPUs 0-3 is both
wrong and less interesting than running on random CPUs.
Opportunistically convert the local 'i' to an int, as the macros it tests
against are signed, and using 'i' as an iterator is conventionally declared
as an int.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Extract kvm_pick_random_cpu() out of pin_task_to_random_cpu() so that tests
can choose a random CPU without having to immediately pin a task to that
CPU.
No functional change intended.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
When pinning a task to a random CPU, return which CPU the task was pinned
to so that the caller can do things like avoid running other tasks on the
target CPU.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
When building against a libc that provides pthread_attr_setaffinity_np(),
pre-set the grandchildren threads CPU affinity in the hardware disable test
to further reduce the perceived latency of pthread_create(). On large NUMA
systems, this reduces the average runtime from ~10s to ~5s.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Affine worker threads to their target pCPU set *before* doing any work in
the hardware disable test. When running a preemptible kernel on a NUMA
system with a large number of CPUs (100+ of CPUs), waiting to set the
affinity of the grandchildren until after the grandchild is fully spun up
results in total test runtimes of over a minute, presumably due to
cross-node NUMA penalties and scheduler lock contention.
E.g. from the perspective of the caller, when waiting to affine the tasks,
pthread_create() takes ~100x longer.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Add and use a KVM wrapper for sched_setaffinity() so that selftests don't
need to manually assert that the syscall succeeded.
No functional change intended.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Double down on using the VMX preemption timer only for 64-bit kernels, and
bury the setup and runtime adjustment code, and all global variables, under
CONFIG_X86_64=y. This will allow addressing a widespread Intel erratum
without running afoul of unused-but-set-variable and __udivdi3() errors on
32-bit kernels.
No functional change intended.
Reviewed-by: Binbin Wu <[email protected]>
Reviewed-by: Chao Gao <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Link to the VFIO library only on architectures where it's actually needed,
i.e. only on architectures that support irq_test, which is currently
x86-only. The irq_test series, which pulled in VFIO support, effectively
raced with commit e65f1bf8a2 ("vfio: selftests: Extend container/iommufd
setup for passing vf_token"). I.e. when the KVM test was posted, pulling
in libvfio.mk didn't require linking to any new libraries other than VFIO
itself.
Now that libvfio.mk pulls in luuid, unconditionally linking in support for
VFIO is quite onerous, especially when cross-compiling, as it requires
installing what is effectively an unused library on four architectures.
Eventually KVM selftests will likely support irq_test and thus need to link
to VFIO on architectures other than x86, but at least then there will be
some amount of gain for the pain.
Fixes: a262fc49e0 ("KVM: selftests: Build and link selftests/vfio/lib into KVM selftests")
Cc: David Matlack <[email protected]>
Cc: Josh Hilke <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Always update x2APIC MSR intercepts for L1 when AVIC is deactivated, even
if L2 is active and KVM is using a separate MSR bitmap to run L2. If AVIC
is fully enabled prior to running L2, and is then inhibited while L2 is
active (for a VM-scoped inhibit), then KVM will run L1 with AVIC disabled,
but with x2APIC MSR intercepts disabled, i.e. will allow L1 to read most of
the host's APIC state, send arbitrary interrupts, change task priority, and
ultimately trivially DoS the host.
E.g. sending a self-IPI in L1 on HYPERV_REENLIGHTENMENT_VECTOR, 0xee, with
CONFIG_HYPERV=n in the host kernel as a "safe" PoC, yields:
Spurious interrupt (vector 0xee) on CPU#425. Acked
And hacking KVM to abuse kvm_set_posted_intr_wakeup_handler() to register a
handler and WARN on POSTED_INTR_WAKEUP_VECTOR yields:
------------[ cut here ]------------
WARNING: arch/x86/kvm/svm/svm.c:5594 at pi_wakeup_handler+0x9/0x10 [kvm_amd], CPU#156: nested_x2apic_t/316940
CPU: 156 UID: 0 PID: 316940 Comm: nested_x2apic_t Tainted: G S U
Tainted: [S]=CPU_OUT_OF_SPEC, [U]=USER
Hardware name: Google Astoria-Turin/astoria, BIOS 0.20260209.0-0 02/09/2026
RIP: 0010:pi_wakeup_handler+0x9/0x10 [kvm_amd]
Call Trace:
<IRQ>
sysvec_kvm_posted_intr_wakeup_ipi+0x64/0x80
</IRQ>
<TASK>
asm_sysvec_kvm_posted_intr_wakeup_ipi+0x1a/0x20
RIP: 0010:vcpu_run+0x1430/0x1e40 [kvm]
kvm_arch_vcpu_ioctl_run+0x2c1/0x600 [kvm]
kvm_vcpu_ioctl+0x580/0x6b0 [kvm]
__se_sys_ioctl+0x6d/0xb0
do_syscall_64+0x10a/0x480
entry_SYSCALL_64_after_hwframe+0x4b/0x53
RIP: 0033:0x46ff4b
</TASK>
---[ end trace 0000000000000000 ]---
Fixes: 091abbf578 ("KVM: x86: nSVM: optimize svm_set_x2apic_msr_interception")
Cc: [email protected]
Cc: Yosry Ahmed <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
Link: https://patch.msgid.link/[email protected]/
Signed-off-by: Paolo Bonzini <[email protected]>
Use CMPXCHG instead of clear_bit(), which currently emits a LOCK BTR since
the to-be-cleared bit isn't a compile-time constant, when aging SPTEs in
the shadow MMU to align with the approach taken by the TDP MMU, and because
using CMPXCHG is far more robust against bugs in KVM. E.g. if the SPTE is
somehow no longer an SPTE due to a KVM bug, CMPXCHG will fail gracefully,
whereas clear_bit() would potentially corrupt/clobber memory.
Clearing the Accessed bit without atomically ensuring the SPTE is still the
old SPTE is "fine", as holding the rmap's lock ensures zapping the old SPTE
can't fully complete, which in turn ensures a new, different SPTE can't be
installed. But that chain of logic isn't exactly obvious, and there's zero
reason to avoid CMPXCHG as its cost on modern hardware is within ~1-2 uops
of LOCK BTR (and may even be cheaper on some microarchitectures). Doing a
64-bit CMPXCHG on 32-bit kernels does require a more expensive CMPXCHG8B,
but 32-bit KVM is all but dead at this point.
Cc: James Houghton <[email protected]>
Reviewed-by: Kai Huang <[email protected]>
Reviewed-by: James Houghton <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Use LOCK CMPXCHG instead of LOCK AND to clear the Accessed bit when aging
SPTEs in the TDP MMU, as doing a LOCK AND can corrupt a FROZEN SPTE and
allow a third CPU to effectively overwrite the FROZEN SPTE. As pointed
out by AI of some kind, because the magic FROZEN_SPTE value is a "full"
SPTE, not a single bit, and includes the Accessed bit, clearing the
Accessed bit in a FROZEN SPTE will result in is_frozen_spte() getting a
false negative.
E.g. if CPU0 freezes an SPTE, and CPU1 clears the Accessed bit in the
frozen SPTE, then CPU2 could come along and overwrite the frozen SPTE with
a shadow-present SPTE.
Thankfully, the false negative is largely benign, because outside of TDX,
which doesn't support aging, KVM only freezes leaf SPTEs when removing an
upper level shadow page. So while KVM could clobber a frozen SPTE back to
a shadow-present SPTE, and could even use the new SPTE, the subsequent TLB
flush will make the orphaned, shadow-present SPTE unreachable. Failure to
ever zap the orphaned leaf SPTE would show up in KVM's stats, but otherwise
is benign (because KVM no longer keeps an elevated refcount for leaf SPTEs).
Opportunistically add a comment to warn future developers away from using
kvm_tdp_mmu_write_spte_atomic() and tdp_mmu_clear_spte_bits_atomic(), as
they are generally unsafe. Keep the helpers, e.g. instead of open-coding
the atomic64_fetch_and() in tdp_mmu_clear_spte_bits(), as scary warnings
usually are more effective deterrent against recidivism than removal of the
dangerous code.
Alternatively, KVM could use different bits for the magic FROZEN_SPTE value,
e.g. setting the Dirty bits (with effective IPAT and Global aliases) would
likely be "ok", as IPAT/Global are extremely unlikely to be cleared without
doing a full SPTE write, and KVM's clearing of Dirty bits shares logic with
Write-Protection, which must do a full SPTE write (via cmpxchg64() in the
TDP MMU) to ensure KVM isn't clobbering state. But there is zero reason to
carry that risk (beyond stubbornness in wanting to preserve a "cute" idea),
as the cost of LOCK CMPXCHG and LOCK AND are within 1-2 uops of each other
on modern hardware.
Fixes: b146a9b34a ("KVM: x86/mmu: Age TDP MMU SPTEs without holding mmu_lock")
Cc: [email protected]
Reviewed-by: Kai Huang <[email protected]>
Reviewed-by: James Houghton <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Now that all on-stack maps use CLASS(kvm_vcpu_map_local), i.e. now that all
maps are zero-allocated, explicitly put any existing mappings/references
when establishing a new mapping to harden against KVM bugs leaking memory,
but yell loudly as the owner of the map is still ultimately responsible for
the lifecycle of the mapping.
Suggested-by: Yosry Ahmed <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Convert the kvm_vcpu_map() usage in PPC dcbz patching to the new
CLASS(kvm_vcpu_map_local) implementation, to eliminate the very last of the
the open-coded on-stack "struct kvm_host_map" declarations. This will
allow adding hardening kvm_vcpu_map() against memory leaks (due to
clobbering the existing mapping).
No functional change intended.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Convert the kvm_vcpu_map_readonly() usage in nVMX's MSR bitmap merging to
the new CLASS(kvm_vcpu_map_local_readonly) implementation, to eliminate the
last of the open-coded on-stack "struct kvm_host_map" declarations (in x86,
PPC still has one more to convert).
No functional change intended.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Convert the kvm_vcpu_map() usage in the enter/leave SMM flows to the new
CLASS(kvm_vcpu_map_local) implementations, to eliminate the need to
manually do unmap() in error paths, and more importantly to eliminate more
of the open-coded on-stack "struct kvm_host_map" declarations.
No functional change intended.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Add CLASS() definitions for locally mapping a PFN using kvm_vcpu_map()
given a vCPU+gfn pair. In addition to eliminating the need to manually
do unmap(), e.g. in error paths, this will allow hardening KVM against
double-mapping without having to manually ensure every on-stack declaration
is zero-initialized.
Use "map local" as the primary terminology as the basic concept is more or
less the same as kmap_local(): ensure the current context has a kernel
mapping to the underlying memory.
Immediately convert the relatively straightforward nested SVM flows, and
defer converting the more involved SMM flows to a separate change.
No functional change intended.
Cc: Yosry Ahmed <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Always check and clear KVM_REQ_GET_NESTED_STATE_PAGES when emulating a
nested VM-Exit to ensure the request is cleared, even when KVM was built
with CONFIG_KVM_HYPERV=n, as KVM subtly relies on the "check" to clear
the flag and thus avoid double-mapping the vmcs12 pages, e.g. if KVM
manages to bail from VM-Enter without processing the request, and then
emulates VMLAUNCH or VMRESUME.
Fixes: b4f69df0f6 ("KVM: x86: Make Hyper-V emulation optional")
Cc: [email protected]
Reported-by: Yosry Ahmed <[email protected]>
Reviewed-by: Yosry Ahmed <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
When getting a to-be-injected IRQ, don't WARN if the IRQ disappeared and
Xen emulation is supported, as a guest could concurrently toggle its
evtchn_upcall_pending flag in shared memory and deassert the IRQ. Even
more annoyingly, userspace could disable Xen emulation for the entire VM
KVM_XEN_HVM_CONFIG. So, suppress WARNs on lost IRQs if Xen emulation is
supported to prevent false positives.
Alternatively, KVM could track if the VM has ever used Xen emulation, but
the added complexity isn't worth carrying given that the vast majority of
deployments can and should disable Xen emulation.
Fixes: bf672720e8 ("KVM: x86: check the kvm_cpu_get_interrupt result before using it")
Reported-by: Sashiko Bot <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
When getting a to-be-injected IRQ, don't WARN if the IRQ disappeared and
the VM has an in-kernel PIC, as the ExtINT handling that's routed through
KVM's virtual PIC is tracked per-VM, not per-vCPU. If another vCPU grabs
the IRQ, or deasserts the interrupt (which is level-triggered), then it's
both expected and "fine" for a
Keep the assert for split IRQCHIP VMs to help detect KVM bugs, as userspace
is responsible for routing ExtINT to the intended vCPU, i.e. once an ExtINT
is pending, it can't be cleared without holding the vCPU's mutex, and thus
false positives are impossible.
Fixes: bf672720e8 ("KVM: x86: check the kvm_cpu_get_interrupt result before using it")
Debugged-by: Alexander Potapenko <[email protected]>
Reported-by: [email protected]
Closes: https://syzkaller.appspot.com/bug?extid=dd769db18693736eee89
Closes: https://lore.kernel.org/all/[email protected]
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
When calling into arch code to make the underlying memory private, i.e. to
assign memory to the VM in SNP's RMP table, assign/convert *exactly* the
range of memory that can be mapped into the guest for the current page
fault, instead of aggressively converting/assigning the entire folio. For
SNP, the mapping size in the stage-2 page tables (Nested Page Tables, NPT)
must be at least the size of the corresponding RMP entry, e.g. assigning a
2MiB mapping in the RMP when it can only be mapped at 4KiB granualarity
will ultimate result in another page fault (#NPF for SNP) to "smash" the
RMP down to the correct mapping size.
Assigning the entire folio was necessary back when guest_memfd tracked
preparedness, which was done on a per-folio basis. At the time, it made
sense to do per-folio tracking/preparation, because tracking per-folio
meant guest_memfd didn't need to add a separate data structure to track
that information, and doing per-folio tracking only works if the entire
folio is prepared (or not).
Now that guest_memfd no longer does preparation tracking (see commit
8622ef0570 ("KVM: guest_memfd: Remove preparation tracking")), in favor
having SNP query the RMP, per-folio preparation, i.e. per-folio conversions
to private, doesn't make any sense.
*If* SNP allowed the RMP size to be greater than the NPT size, then
per-folio conversion could theoretically provide marginal value, as it
would allow KVM to assign a hugepage in the RMP even if it can only be
mapped into the NPT with a smaller page, e.g. because of memslot alignment.
The documentation of that reasoning would be something like this:
/*
* If the memory is private from KVM's perspective, and hardware tracks
* VM-assigned private memory in a dedicated data structure, i.e. not
* in the stage-2 page tables, then call into arch code to assign the
* entire folio to the guest. Assigning the entire folio, e.g. instead
* of only the memory being mapped into the guest, allows KVM to assign
* an entire hugepage of memory in the out-of-band structure even if
* KVM can only map a smaller page size into the MMU, e.g. because the
* gmem hugepage is spread across multiple memslots.
*/
But even *if* a future SNP implementation supported that behavior, the
value added would be dubious, as having a huge folio that is fully private,
but can only be mapped at a smaller granularity, would be rare. E.g. maybe
for memory at the top of lower DRAM that has holes for non-RAM assets?
So, convert/assign exactly what guest_memfd allows the caller to map to
simplify the guest_memfd code and provide a (super) minor performance
optimization for SNP. E.g. once hugepage support comes along, guest_memfd
will only need a single flow to compute "how much memory can be assigned
and at what size".
Reviewed-by: Ackerley Tng <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Tweak the guest_memfd make_private() hook to explicitly pass the number of
pages to align with the signature of the make_shared() hook, and because
the existing code is outright broken if a guest_memfd folio is comprised of
more than one page (which can't happen, yet). The SNP code *tries* to
create a corresponding huge entry, but if the RMP must use 4KiB entries for
whatever reason, KVM will only convert the first pfn, and not the entire
range of pfns that will be mapped into the guest.
Alternatively, @max_order could simply be repurposed as _the_ @order, but
that will fall apart when in-place conversion comes along, at which point
KVM will need to deal with conversions that aren't bound 1:1 to a folio.
I.e. the number of pages to convert may not be exactly a power-of-2 (and
folios don't strictly guarantee power-of-2 pages anyways).
WARN in the SNP code if the number of pages to prepare is anything other
than '1', i.e. if guest_memfd is trying to prepare/convert more than a
single 4KiB page, as sev_gmem_prepare() doesn't actually handle conversion
greater than order-0 folios.
Opportunistically swap the ordering of @pfn and @gfn params for
kvm_x86_ops.gmem_make_private() to match kvm_arch_gmem_make_private().
Fixes: b85524314a ("KVM: guest_memfd: delay kvm_gmem_prepare_folio() until the memory is passed to the guest")
Reviewed-by: Xiaoyao Li <[email protected]>
Reviewed-by: Ackerley Tng <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Rework guest_memfd's prepare() hook into a more accurate make_private(),
and rework its Kconfig from PREPARE to a more generic CONVERT. This will
allow x86 to share (pun intended) a kvm_x86_ops.gmem_make_shared() hook
between the "convert to shared" and "reclaim" flows, which are one and the
same for SNP.
No functional change intended.
Reviewed-by: Ackerley Tng <[email protected]>
Reviewed-by: Fuad Tabba <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Add helpers to check if a given page in a guest_memfd instance is PRIVATE
versus SHARED, and use the "is shared" helper instead of an open-coded
equivalent in the user pagefault handler. In addition to the immediate
usage, providing an "is private" helper will allow cleaning up the so
called prepare() code, and eventually will be heavily used once in-place
conversion support comes along.
No functional change intended.
Reviewed-by: Xiaoyao Li <[email protected]>
Reviewed-by: Ackerley Tng <[email protected]>
Reviewed-by: Fuad Tabba <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Drop guest_memfd's ratelimited printk to log "preparation" failures, as KVM
SNP already logs more precise messages in all error paths, and whether or
not failure to convert the pfn to private is "unexpected", i.e. warrants
logging, is firmly an architecture specific detail.
Reviewed-by: Ackerley Tng <[email protected]>
Reviewed-by: Xiaoyao Li <[email protected]>
Reviewed-by: Fuad Tabba <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Rename kvm_x86_ops's gmem_invalidate() hook to gmem_make_shared(), as the
hook doesn't invalidate anything, and so that KVM doesn't need to add yet
another vendor callback to support "convert to shared" once in-place
conversion comes along.
Opportunistically wrap the ops declarations with a GMEM_RECLAIM guard so
that attempting to wire up a .gmem_make_shared() hook without selecting
CONFIG_HAVE_KVM_ARCH_GMEM_RECLAIM will result in a build failure.
No functional change intended.
Reviewed-by: Xiaoyao Li <[email protected]>
Reviewed-by: Ackerley Tng <[email protected]>
Reviewed-by: Fuad Tabba <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Rename guest_memfd's invalidate() hook to reclaim() and isolate it via its
own RECLAIM Kconfig, as the hook is called when a folio is freed, which is
far too late and lacks sufficient information for KVM to actually
invalidate its usage of the memory. E.g. SNP uses the hook to convert
memory back to SHARED so that it can be safely accessed by the host, there
is no invalidation of guest mappings anywhere. Isolating the hook will
also allow pKVM on arm64 to opt-in to reclaim() without also having to
differentiate between reclaim and conversions to shared for active VMs.
Keep guest_memfd's trampoline, even though it would be trivial to wire up
.free_folio() directly to an arch callback, to avoid bleeding guest_memfd
internals into arch code (specifically, avoid referencing folios in arch
code).
Leave the kvm_x86_ops hook as-is for the moment, as "reclaim" on SNP is
the same as convert-to-shared, i.e. using a different name for the x86 hook
will allow reusing it for in-place conversion.
Reviewed-by: Xiaoyao Li <[email protected]>
Reviewed-by: Fuad Tabba <[email protected]>
Reviewed-by: Ackerley Tng <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Pass the number of pages to "invalidate", i.e. reclaim, instead of the end
pfn, as a first step towards aligning the function prototypes between the
de facto "to private" and "to shared" arch hooks. Eventually, the goal
is to end up with kvm_gmem_arch_make_{private,shared}(), and in both cases,
providing the number of pages makes the call sites slightly nicer, and also
avoids any confusion over whether the end pfn is inclusive or exclusive.
Opportunistically rename "start" to "pfn", again to align with the expected
signature of make_private() (which needs to pass a starting gfn as well, at
which point the "start" becomes noise).
No functional change intended.
Cc: Fuad Tabba <[email protected]>
Cc: Ackerley Tng <[email protected]>
Reviewed-by: Xiaoyao Li <[email protected]>
Reviewed-by: Ackerley Tng <[email protected]>
Reviewed-by: Fuad Tabba <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
When {de,en}crypting memory of an SEV or SEV-ES guest on an SNP-enabled
host via a temporary buffer, allocate a full 4KiB page for the buffer to
ensure the page containing the buffer is wholly owned by KVM, i.e. won't
be concurrently allocated and accessed by other kernel code while KVM is
using the buffer to {de,en}crypt memory. On SNP-enabled platforms, when
sending SEV/SEV-ES commands that trigger firmware writes to memory, the
to-be-written page(s) must be (temporarily) assigned to Firmware (as
required by the SNP architecture, to guard against using such commands as
gadgets to attack SNP guests). See snp_map_cmd_buf_desc() and friends.
Unfortunately, transferring ownership of a page to Firmware makes the page
inaccessible to software, and thus writes generate RMP #PF violations. If
KVM uses a sub-page allocation for its temporary buffer, some other actor
in the kernel can allocate and use the other portions of the page, and thus
trigger unexpected (and seemingly spurious) RMP #PF violations due to
software attempting to access a Firmware-owned page.
BUG: unable to handle page fault for address: ffff906ae30f0300
#PF: supervisor write access in kernel mode
#PF: error_code(0x80000003) - RMP violation
PGD 6b1b80d067 P4D 6b1b80d067 PUD 100231e2063 PMD 10055a88063 PTE 80000100630f0163
SEV-SNP: PFN 0x100630f0 unassigned, dumping non-zero entries in 2M PFN region: [0x10063000 - 0x10063200]
Oops: Oops: 0003 [#1] SMP
CPU: 70 UID: 0 PID: 10658 Comm: svw_WaiterThrea Tainted: G U W O 7.1.0-smp--c22293789940-seanjc-next #1 PREEMPTLAZY
Tainted: [U]=USER, [W]=WARN, [O]=OOT_MODULE
Hardware name: Google, Inc. Arcadia_IT_80/Arcadia_IT_80, BIOS 34.86.0-102 01/25/2026
RIP: 0010:memset+0xf/0x20
Call Trace:
<TASK>
__kvmalloc_node_noprof+0x2a4/0x710
do_getxattr+0x4e/0x130
path_getxattrat+0x125/0x1b0
do_syscall_64+0x10a/0x480
entry_SYSCALL_64_after_hwframe+0x4b/0x53
RIP: 0033:0x7f3a22cb6daa
</TASK>
Modules linked in: kvm_amd kvm irqbypass vfat fat ccp k10temp sha3 libsha3 i2c_piix4 gq(O) cdc_acm xhci_pci xhci_hcd
gsmi: Log Shutdown Reason 0x03
CR2: ffff906ae30f0300
---[ end trace 0000000000000000 ]---
RIP: 0010:memset+0xf/0x20
Kernel panic - not syncing: Fatal exception
Kernel Offset: 0x39e00000 from 0xffffffff81000000 (relocation range: 0xffffffff80000000-0xffffffffbfffffff)
gsmi: Log Shutdown Reason 0x02
Fixes: 4c735bf1bc ("KVM: SEV: Allocate only as many bytes as needed for temp crypt buffers")
Cc: [email protected]
Cc: Michael Roth <[email protected]>
Debugged-by: Michael Roth <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Check for a "stale" page fault, i.e. for an invalid and/or obsolete root,
after making MMU pages available for the shadow MMU. If reclaiming shadow
pages zaps an in-use root, i.e. marks it invalid, then KVM will attempt to
map memory into an invalid root. On its own, populating an invalid root is
"fine", but because child shadow pages inherit their parent's role, any
children created during the map/fetch will be created as invalid pages,
thus violating KVM's invariant that invalid pages are never on the list of
active MMU pages.
Note, the underlying flaw has existed since KVM first started tracking
invalid roots in 2008 (commit 2e53d63acb, "KVM: MMU: ignore zapped root
pagetables"), but the true badness only came along in 2020 (Linux 5.9)
with the invariant that invalid shadow pages can't be on the list of
active pages.
Note #2, inheriting role.invalid when creating child shadow pages is also
far from ideal; that flaw will be addressed separately.
Reported-by: Hyunwoo Kim <[email protected]>
Fixes: f95eec9bed ("KVM: x86/mmu: Don't put invalid SPs back on the list of active pages")
Cc: [email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Put all vmcs12 pages if KVM synthesizes a nested VM-Exit due to invalid
guest while emulating VMLAUNCH or VMRESUME. The invalid guest state path
doesn't use nested_vmx_vmexit() as that API is intended to be used if and
only if L2 is active, and the open coded equivalent neglects to put the
vmcs12 pages. Failure to put the vmcs12 pages leaks any pinned pages
(and/or mappings) if L1 retries VMLAUNCH/VMRESUME.
Note, the !from_vmenter scenario doesn't suffer the same problem, as
vmx_get_nested_state_pages() only gets/pins/maps the vmcs12 pages if L2 is
active, i.e. if a "full" VM-Exit is guaranteed before KVM will retry
getting vmcs12 pages.
Fixes: 96c66e87de ("KVM/nVMX: Use kvm_vcpu_map when mapping the virtual APIC page")
Fixes: 3278e04925 ("KVM/nVMX: Use kvm_vcpu_map when mapping the posted interrupt descriptor table")
Fixes: fe1911aa44 ("KVM: nVMX: Use kvm_vcpu_map() to get/pin vmcs12's APIC-access page")
Reported-by: Minh Nguyen <[email protected]>
Cc: [email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Signed-off-by: Paolo Bonzini <[email protected]>
Rework KVM's handling of per-vendor nested ops to copy the vendor's ops
into a global structure owned by common x86, i.e. treat nested ops just
like x86 and PMU ops. In addition to providing consistency across all ops
implementations, making a copy of the ops prevents changes to the vendor's
ops after KVM is initialized, i.e. guards against goofs where KVM *thinks*
it is updating nested ops, but which won't take effect now that KVM uses
static calls to invoke vendor hooks.
Ignoring the side effects of tagging {svm,vmx}_nested_ops as __initdata,
no functional change intended.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Use static calls to invoke nested virtualization ops, as many of the calls
are in relatively hot paths when L2 is active, e.g. checking for events,
and because there's no reason not use static calls these days.
Opportunistically use a RET0 static call for get_evmcs_version() instead
of manually checking for a non-NULL vendor hook.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Add a flag to explicitly track if nested virtualization is enabled, and use
it enumerate that various nested CAPs are unsupported, and to reject
enablement of said CAPs. When the nested ops hooks were moved to their
own structure, KVM's NULL-by-default behavior was deliberately dropped,
with the changelog asserting that all was well. That wasn't quite true;
there is no danger to KVM, but now KVM is over-reporting support for
KVM_CAP_NESTED_STATE and KVM_CAP_HYPERV_ENLIGHTENED_VMCS.
Fixes: 33b2217245 ("KVM: x86: move nested-related kvm_x86_ops to a separate struct")
Reviewed-by: Vitaly Kuznetsov <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Pin the steal time test's tasks to a random pCPU in the system instead of
hardcoding the test to always run on pCPU0 as a cheap way of increasing
test coverage, and to do the "right thing" if the parent task of the test
doesn't have pCPU0 in its CPU affinity mask.
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
In the steal time test, don't explicitly set the CPU affinity mask of the
worker child and instead rely on the child inheriting the affinity of the
main thread. Per the pthread_create()[1] and pthread_setaffinity_np()[2]
documentation, new threads inherit the parent's affinity mask.
Linux-specific details
The new thread inherits copies of the calling thread's capability
sets (see capabilities(7)) and CPU affinity mask (see
sched_setaffinity(2)).
Out of an abundance of caution, assert that the child did indeed inherit
the CPU affinity mask, as the test will hang indefinitely if the system is
under light load.
Dropping use of pthread_attr_setaffinity_np() allows building the steal
time test against non-glibc C libraries that don't implement that GNU
extension.
Link: https://man7.org/linux/man-pages/man3/pthread_setaffinity_np.3.html [1]
Link: https://man7.org/linux/man-pages/man3/pthread_create.3.html [1]
Cc: Hisam Mehboob <[email protected]>
Reported-by: Aqib Faruqui <[email protected]>
Closes: https://lore.kernel.org/all/[email protected]
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Wrap the .gmem_prepare() declarations with HAVE_KVM_GMEM_PREPARE so that
non-SEV code doesn't try to wire up a callback without doing the necessary
enabling.
No functional change intended.
Fixes: 3bb2531e20 ("KVM: guest_memfd: Add hook for initializing memory")
Reviewed-by: Ackerley Tng <[email protected]>
Reviewed-by: Michael Roth <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Track the guest as having a guest-provided VMSA as soon as control.vmsa_pa
is invalidated, instead of waiting to see if the guest-provided VMSA is
usable, so that KVM doesn't switch back to the original VMSA instead of
exiting to userspace (due to an invalid VMSA). By the time a vCPU tries
to load a guest-provided VMSA, KVM has already communicated "success" for
AP creation, i.e. KVM has committed to using the guest-provided VMSA.
Reviewed-by: Michael Roth <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Wire up a gmem_invalidate_range() call for SNP VMs, and use it to force
vCPUs to reload/recheck their guest-provided VMSA if the backing gmem
page is being invalidated, e.g. is being PUNCH_HOLE'd. Use the same core
logic to handle invalidations as VMX does for the APIC-access page, as the
two concepts are nearly identical: shove the physical address of a page
into the vCPU's control structure:
1. Snapshot the invalidation sequence counter
2. Grab the pfn (from guest_memfd in this case)
3. Acquire mmu_lock for read
4. Re-request reload if retry is needed, otherwise commit the change.
Note, the re-request action in #4 is necessary as KVM's retry logic is
fuzzy, i.e. can get false positives. If the guest_memfd page has been
dropped, at some point a subsequent reload will fail to get a PFN from
guest_memfd, and KVM will fail KVM_RUN. If the retry was due to a false
positive, KVM will retry until there are no relevant MMU notifier events
(and will retry in the "outer" loop, i.e. will drop locks and resched as
needed).
Note #2! Take care to invalidate the VMSA when a relevant memslot is
DELETED or MOVED, as invalidations in response to PUNCH_HOLE are predicated
on memslot bindings (KVM doesn't know what GFN range(s) to invalidate
without a binding). And more importantly, the VMSA mapping requires a
memslot, i.e. must be invalidated if its memslots disappears, regardless of
the state of the underlying guest_memfd inode.
Failure to invalidate the vCPU's control.vmsa_pa (which is checked by
pre_sev_run()) can prevent KVM from properly freeing the page as firmware
will reject the RMPUPDATE to reclaim the page with FAIL_INUSE if the vCPU
is actively running, i.e. if VMSA page is in-use. That in turn leads to an
RMP #PF on the next use, as the page will still be assigned to the SNP VM.
SEV-SNP: RMPUPDATE failed for PFN 78d198, pg_level: 1, ret: 3
SEV-SNP: PFN 0x78d198, RMP entry: [0xfff0000000144001 - 0x000000000000000f]
CPU: 3 UID: 0 PID: 31345 Comm: sev_snp_vmsa_pu Tainted: G U O
Tainted: [U]=USER, [O]=OOT_MODULE
Hardware name: Google, Inc. Arcadia_IT_80/Arcadia_IT_80, BIOS 34.86.0-102 01/25/2026
Call Trace:
<TASK>
dump_stack_lvl+0x54/0x70
rmpupdate+0x12c/0x140
rmp_make_shared+0x3b/0x60
sev_gmem_invalidate+0xe0/0x170 [kvm_amd]
delete_from_page_cache_batch+0x1d8/0x220
truncate_inode_pages_range+0x120/0x3d0
kvm_gmem_fallocate+0x19a/0x270 [kvm]
vfs_fallocate+0x1bc/0x1f0
__x64_sys_fallocate+0x48/0x70
do_syscall_64+0x10a/0x480
entry_SYSCALL_64_after_hwframe+0x4b/0x53
RIP: 0033:0x496c7e
</TASK>
------------[ cut here ]------------
SEV: Failed to update RMP entry for PFN 0x78d198 error -14
WARNING: arch/x86/kvm/svm/sev.c:5160 at sev_gmem_invalidate+0x126/0x170 [kvm_amd], CPU#3: sev_snp_vmsa_pu/31345
CPU: 3 UID: 0 PID: 31345 Comm: sev_snp_vmsa_pu Tainted: G U O
Tainted: [U]=USER, [O]=OOT_MODULE
Hardware name: Google, Inc. Arcadia_IT_80/Arcadia_IT_80, BIOS 34.86.0-102 01/25/2026
RIP: 0010:sev_gmem_invalidate+0x12b/0x170 [kvm_amd]
Call Trace:
<TASK>
delete_from_page_cache_batch+0x1d8/0x220
truncate_inode_pages_range+0x120/0x3d0
kvm_gmem_fallocate+0x19a/0x270 [kvm]
vfs_fallocate+0x1bc/0x1f0
__x64_sys_fallocate+0x48/0x70
do_syscall_64+0x10a/0x480
entry_SYSCALL_64_after_hwframe+0x4b/0x53
RIP: 0033:0x496c7e
</TASK>
irq event stamp: 20689
hardirqs last enabled at (20699): [<ffffffff8e76092c>] __console_unlock+0x5c/0x60
hardirqs last disabled at (20708): [<ffffffff8e760911>] __console_unlock+0x41/0x60
softirqs last enabled at (20722): [<ffffffff8e6cd74e>] __irq_exit_rcu+0x7e/0x140
softirqs last disabled at (20717): [<ffffffff8e6cd74e>] __irq_exit_rcu+0x7e/0x140
---[ end trace 0000000000000000 ]---
BUG: unable to handle page fault for address: ffff99a64d198000
#PF: supervisor write access in kernel mode
#PF: error_code(0x80000003) - RMP violation
PGD 13eb001067 P4D 13eb001067 PUD 78d1d1063 PMD 1184e0063 PTE 800000078d198163
SEV-SNP: PFN 0x78d198, RMP entry: [0x6030000000144001 - 0x000000000000000f]
Oops: Oops: 0003 [#1] SMP
CPU: 3 UID: 0 PID: 31407 Comm: highlanderd_hea Tainted: G U W O
Tainted: [U]=USER, [W]=WARN, [O]=OOT_MODULE
Hardware name: Google, Inc. Arcadia_IT_80/Arcadia_IT_80, BIOS 34.86.0-102 01/25/2026
RIP: 0010:prep_new_page+0x67/0x220
Call Trace:
<TASK>
get_page_from_freelist+0x1c40/0x1c70
__alloc_frozen_pages_noprof+0xca/0x1f0
alloc_pages_mpol+0x10b/0x1b0
alloc_pages_noprof+0x81/0x90
pte_alloc_one+0x1b/0xd0
do_pte_missing+0xdf/0x1020
handle_mm_fault+0x7c7/0xb20
do_user_addr_fault+0x268/0x6b0
exc_page_fault+0x67/0xa0
asm_exc_page_fault+0x26/0x30
RIP: 0033:0x4a6b1e
</TASK>
gsmi: Log Shutdown Reason 0x03
CR2: ffff99a64d198000
---[ end trace 0000000000000000 ]---
RIP: 0010:prep_new_page+0x67/0x220
Drop the pseudo-TODO comment about needing to pin the page if guest_memfd
every supports migration, as integrating with invalidations events means
KVM will Just Work if/when page migration is ever supported (assuming SNP
hardware supports migrating VMSA pages).
Note #3, invalidate() and invalidate_range() have _completely_ different
semantics; the new invalidate_range() is a true invalidation, whereas the
existing invalidate() is really a "make shared" operation. Ignore the
confusing naming and poor Kconfig bundling for the moment to minimize the
delta for LTS kernels, the mess will be cleaned up shortly.
Reported-by: Hyunwoo Kim <[email protected]>
Closes: https://lore.kernel.org/all/aimMWzAf5b3luM0b@v4bel
Fixes: e366f92ea9 ("KVM: SEV: Support SEV-SNP AP Creation NAE event")
Cc: [email protected]
Cc: Tom Lendacky <[email protected]>
Cc: Michael Roth <[email protected]>
Cc: Jörg Rödel <[email protected]>
Cc: Fuad Tabba <[email protected]>
Cc: Ackerley Tng <[email protected]>
Reviewed-by: Michael Roth <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Manually invoke the front half and back half of the "zap all fast" flow
when invalidating a memslot so that mmu_lock is acquired at function scope
in kvm_arch_flush_shadow_memslot(). This will allow putting more code
inside the critical section without having to take mmu_lock twice in quick
succession.
Opportunistically open code checking whether or not to do the fast zap, to
discourage removing the local "zap_all" in a future cleanup, i.e. to ensure
the SLOT_ZAP_ALL quirk is queried exactly once. Processing the front half
but not the back half of the fast zap (if SLOT_ZAP_ALL were disabled
concurrently) would result in KVM unnecessarily keeping invalid TDP MMU
roots until the VM is destroyed.
No functional change intended.
Cc: [email protected] # 6.12.x
Reviewed-by: Michael Roth <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Split kvm_mmu_zap_all_fast() into a "front half" and a "back half", where
the front half is everything that runs with mmu_lock held for write, and
the back half is the code that runs outside of mmu_lock. This will allow
putting more code inside kvm_arch_flush_shadow_memslot()'s critical section
without having to take mmu_lock twice in quick succession.
No functional change intended.
Cc: [email protected] # 6.12.x
Reviewed-by: Michael Roth <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Fold kvm_mmu_zap_memslot() into its sole caller so that its GFN range
structure can be used to trigger guest_memfd invalidations regardless of
whether KVM will do a partial or full zap of the MMU.
No functional change intended.
Cc: [email protected] # 6.12.x
Reviewed-by: Michael Roth <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Wrap the sole reader of disabled_quirks with READ_ONCE(), and wrap the
post-VM-creation write to disabled_quirks with WRITE_ONCE(), to ensure
checking the status of a quirk doesn't re-read disabled_quirks *if* the
caller needs such a guarantee. This will allow splitting the "fast" MMU
zap into front and back halves, without potentially skipping the back
half if SLOT_ZAP_ALL were concurrently disabled (which would be "fine" in
the current code base, but far from ideal).
Cc: [email protected] # 6.12.x
Reviewed-by: Michael Roth <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Protect writes to disabled_quirks with kvm->lock to ensure KVM doesn't
clobber state in the unlikely scenario that userspace disables disparate
quirks from multiple tasks. More importantly, this will allow wrapping
accesses with {READ,WRITE}_ONCE without "needing" to also guard the writer
with a useless and confusing READ_ONCE (since the RMW wouldn't be atomic
anyways).
Ideally, KVM would disallow disabling quirks once quirks are "live", but
that would be a potentially breaking userspace ABI change, and while all
existing quirks are fully live only after vCPUs have been created, several
MMU-related quirks, IGNORE_GUEST_PAT and SLOT_ZAP_ALL, are partially live
at all times. Because populating MMUs requires a vCPU, the guest-visible
behavior of IGNORE_GUEST_PAT and SLOT_ZAP_ALL requires a vCPU, but for KVM
itself, processing the quirk (or not) has functional impact, i.e. for all
intents and purposes, KVM can't prevent those quirks from being disabled
after they've been consumed.
Cc: [email protected] # 6.12.x
Reviewed-by: Michael Roth <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Wire up the SEV-SNP guest_memfd kvm_x86_ops hooks if and only if SEV is
actually enabled, and drop the now-unnecessary stubs. Leaving the hooks
NULL allows the static call infrastructure to elide the CALL+RET, and more
importantly, referencing the hooks if and only if SEV support is enabled
will allow conditionally definining the hooks using their corresponding
HAVE_KVM_ARCH_GMEM_XXX Kconfig.
No functional change intended.
Cc: [email protected] # 6.12.x
Reviewed-by: Ackerley Tng <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Always mark the vCPU as RUNNABLE after responding to AP_CREATE, even if the
guest-specified VMSA is unusable, e.g. isn't backed by a memslot or doesn't
have a backing guest_memfd page. If the VMSA is unusable, leaving the vCPU
in a non-running state will effectively hang the vCPU instead of reporting
an error to userspace. This will also allow retrying the VMSA load in the
future, to fix a bug where KVM doesn't honor guest_memfd invalidation
events, e.g. if AP_CREATION races with PUNCH_HOLE.
Cc: [email protected] # 6.12.x
Reviewed-by: Michael Roth <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Extract the loading/retrieval of a guest-provided VMSA to a separate helper
so that KVM can reuse the core logic when refreshing the VMSA after an MMU
invalidation from guest_memfd.
No functional change intended.
Cc: [email protected] # 6.12.x
Reviewed-by: Michael Roth <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Track the GPA of the guest-provided VMSA used after AP_CREATION events when
running SNP guests, instead of simply tracking whether or not the vCPU is
using a guest-provided VMSA. KVM needs to know the GPA of the VMSA that's
actively being used so that it can react to MMU invalidation events, i.e.
so that KVM can drop the VMSA if its backing guest_memfd page is punched
out of existence.
Opportunistically rename snp_vmsa_gpa to clarify that it tracks the pending
VMSA GPA, whereas snp_guest_vmsa_gpa now tracks the in-use VMSA GPA.
Note! Take care to track the GPA, not the GFN, as VALID_PAGE() won't
behave correctly if an invalid GFN is converted to a GPA for checking.
Note #2! Keep snp_has_guest_vmsa so that switching to a guest-provided
VMSA is sticky, even if the guest-provided VMSA becomes invalid.
No functional change intended.
Cc: [email protected] # 6.12.x
Reviewed-by: Michael Roth <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
When a VM is being destroyed, delete it from the list used to process GA
Log interrupts before vCPUs are freed, otherwise avic_ga_log_notifier()
could theoretically hit a use-after-free if a GA Log notification arrives
for a vCPU after the last reference to the VM has been put.
Note, in practice, it's likely all but impossible to trigger UAF, as all
all irqfds and thus all IRTEs are cleaned up by:
kvm_irqfd_release()
|
|-> irqfd_deactivate()
|
|-> irqfd_shutdown()
|
|-> irq_bypass_unregister_consumer()
And kvm_irqfd_release() is guaranteed to run before the last reference to
the VM is put. KVM also configures GA Log interrupts only when a vCPU is
blocking (older versions of KVM configre GA Log interrupts at all times,
but AVIC is off by default on those kernels). Hitting UAF would require
tearing down a VM shortly after a vCPU stopped blocking, and with a very,
very delayed IRQ from hardware.
Opportunistically use guard() to avoid a local "flags" variable.
Fixes: 5881f73757 ("svm: Introduce AMD IOMMU avic_ga_log_notifier")
Cc: Naveen N Rao (AMD) <[email protected]>
Cc: Xiao Wu <[email protected]>
Reviewed-by: Naveen N Rao (AMD) <[email protected]>
Link: https://patch.msgid.link/[email protected]
Signed-off-by: Sean Christopherson <[email protected]>
Sean Christopherson