* [PATCH 01/17] KVM: pfncache: Use atomic SRCU for readers instead of a rwlock
2026-09-20 20:49 [PATCH 00/17] KVM: Use atomic SRCU for gfn-to-pfn cache, reinstate guest mode for x86 nesting David Woodhouse
@ 2026-09-20 20:49 ` David Woodhouse
2026-09-20 21:55 ` sashiko-bot
2026-09-21 1:42 ` Hillf Danton
2026-09-20 20:49 ` [PATCH 02/17] KVM: x86/xen: Extract delivery of event to vCPU into a separate helper David Woodhouse
` (15 subsequent siblings)
16 siblings, 2 replies; 27+ messages in thread
From: David Woodhouse @ 2026-09-20 20:49 UTC (permalink / raw)
To: kvm
Cc: linux-kernel, rcu, linux-rt-devel, linux-kselftest,
Paolo Bonzini, Sean Christopherson, Paul Durrant,
Vitaly Kuznetsov, Fred Griffoul, Paul E. McKenney, Kunwu Chan,
Kunwu Chan, Zqiang, Boqun Feng, Neeraj Upadhyay, Joel Fernandes,
Lai Jiangshan, Josh Triplett, Mathieu Desnoyers,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Jason Gunthorpe, Michal Hocko, Nikita Kalyazin, Keir Fraser,
David Matlack, nh-open-source, David Woodhouse,
syzbot+208f7f3e5f59c11aeb90, syzbot+919877893c9d28162dc2
From: David Woodhouse <dwmw@amazon.co.uk>
Replace the per-cache rwlock with a dedicated per-VM atomic SRCU
domain (kvm->gpc_srcu) for the read side. Readers now run under
srcu_read_lock_atomic() alone — preemption disabled and non-blocking
enforced, which every GPC read-side section already satisfied as a
former rwlock critical section. It works in any context
(including hardirq and sched-out paths) and never fails or spins,
eliminating the
read_trylock() contortions in the atomic paths, the double-lock dance
with the lockdep subclass hack in the runstate update, and the
PREEMPT_RT problems inherent to taking an rwlock in those contexts.
In particular, kvm_xen_set_evtchn_fast() is called from hardirq
context (timer callback, kvm_arch_set_irq_inatomic()), where taking
gpc->lock is a sleeping-lock-in-atomic-context bug on PREEMPT_RT.
The mutators' reader drains use synchronize_srcu_atomic(), which
spins for the bounded readers instead of sleeping: no path waiting
out GPC readers ever schedules, satisfying even the strictest
reading of the OOM-reaper non-blocking requirement in the
invalidation walk. The srcu_struct is initialized with
init_srcu_struct_atomic() so the domain is declared atomic up front
rather than latched at the first reader.
SRCU rather than plain RCU because the write side must wait for
readers: with a per-VM srcu_struct, that wait covers only this VM's
GPC readers, not a grace period across every pCPU in the system —
which matters on large hosts running many small guests, where a
vCPU's refresh must not stall on unrelated load. The read-side cost
is a this_cpu_inc() and a barrier.
The invariant is that a cache's fields (pfn, khva, uhva, gpa, memslot)
are only
ever mutated after clearing GPC_VALID and waiting for a full grace
period, so any reader which observed GPC_VALID (with an acquire load,
paired with the release store publishing the fields) has stable values
for its entire SRCU read-side critical section — including writes
through khva, which are guaranteed to land before the backing page can
be unmapped, exactly as with a TLB shootdown.
The mmu_notifier invalidation path clears GPC_VALID on any overlapping
cache and then does synchronize_srcu_expedited() before returning, so the
primary MMU cannot proceed to zap the page tables until all readers of
the stale mapping have drained. This wait happens even on unblockable
(OOM reaper) ranges: the actual constraint on those is not "no
sleeping" but "no blocking on anything which may itself depend on
memory allocation to make progress", and GPC readers never allocate,
never take mmap_lock, and never sleep. (The one allocation in the
SRCU grace-period machinery itself, on the one-time transition of an
srcu_struct to its node tree, is addressed separately.) The previous
patch removed the over-broad non_block_start() debug annotation which
would have splatted on any voluntary schedule regardless.
Dropping gpc->lock means the refresh can no longer rely on holding it
to make its final invalidation check and its publish atomic with
respect to the mmu_notifier walk, and can no longer treat "GPC_VALID
is already clear" as "no readers remain". Three state bits therefore
live in a single atomic word:
- GPC_BECOMING_VALID is set before the uHVA is resolved and before
any lock is dropped. The invalidation walk keys on it as well as on
GPC_VALID, so a cache whose refresh is in flight is *not* skipped:
its uHVA is already known, and clearing the bit vetoes the pending
publish. The announce is ordered after the uHVA store with
smp_mb__before_atomic() (atomic_or() alone is unordered), pairing
with the walk's acquire load of the state word: a walker which
observes the bit observes the uHVA the refresh is resolving, so the
veto is aimed at the right cache. The walk reads uhva with
READ_ONCE() (and the refresh writes it with WRITE_ONCE()); the
refresh only mutates uhva while the state word is zero, so a walker
which sees either bit sees a current value.
- The publish is a cmpxchg from GPC_BECOMING_VALID to GPC_VALID. If
an invalidation of this uHVA landed at any point since the bit was
set — including in the window after the last retry check — the bit
is gone, the cmpxchg fails, and the refresh goes around again.
Without this, an invalidation which starts and completes entirely
within that window is missed by both mechanisms: the walk skips the
cache because it is not valid (and so does not even wait for a
grace period), and the refresh never re-checks. On a vetoed
publish the speculatively stored pfn/khva are restored to the
previously published values, and the retry loop's own bookkeeping
(the mapping-reuse comparison and the unmap of a failed attempt)
works exclusively from locals: cache fields never carry a doomed
attempt's mapping, which the loop could otherwise free while the
cache still referenced it.
- GPC_INVALIDATING is set in the same atomic operation that clears
GPC_VALID, and means "a grace period for the previously published
mapping is owed and not known to be complete". A refresh or
deactivation entering while the invalidator's synchronize_srcu()
is still in flight would otherwise find GPC_VALID already clear,
skip its own wait, and mutate reader-visible fields under readers
which sampled GPC_VALID before the clear. The mutators consume the
state word with atomic_xchg() and wait whenever either bit was
set; their wait necessarily begins after the clear which set the
marker, so it covers every reader the original clearer is still
waiting for. The invalidator deliberately never clears the marker:
with overlapping invalidate/republish cycles, an earlier
invalidation completing its wait cannot know that a later one is
not still draining the same cache.
As with the rwlock version, a refresh which resolves to the same uHVA,
in the same memslot, for the same gPA does not need to invalidate the
cache at all: the gPA => uHVA translation has not changed, and
GPC_VALID already asserts that the uHVA => PFN mapping is good. Such a
refresh updates only the memslot generation and returns, leaving
concurrent readers undisturbed and skipping the grace period entirely.
This matters because the mmu_notifier invalidates caches on any host
memory management activity (page migration, NUMA balancing, KSM), and
the affected pages usually come straight back at the same uHVA; making
each of those cost a full grace period in the reader's refresh path
would add seconds to a guest boot.
Refreshes are serialized by the existing refresh_lock mutex, and the
gpc_invalidate_seq mechanism continues to catch invalidations which
race with the (lockless) HVA->PFN lookup.
Fixes: 14243b387137 ("KVM: x86/xen: Add KVM_IRQ_ROUTING_XEN_EVTCHN and event channel delivery")
Reported-by: syzbot+208f7f3e5f59c11aeb90@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=208f7f3e5f59c11aeb90
Reported-by: syzbot+919877893c9d28162dc2@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=919877893c9d28162dc2
Tested-by: syzbot+919877893c9d28162dc2@syzkaller.appspotmail.com
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
arch/x86/kvm/x86.c | 10 +-
arch/x86/kvm/xen.c | 111 ++++------
include/linux/kvm_host.h | 25 ++-
include/linux/kvm_types.h | 37 +++-
include/linux/srcu.h | 7 +
virt/kvm/kvm_main.c | 9 +
virt/kvm/pfncache.c | 424 +++++++++++++++++++++++++++++---------
7 files changed, 439 insertions(+), 184 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 79468ddfe473..ccca707d768f 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -1719,18 +1719,18 @@ static void kvm_setup_guest_pvclock(struct pvclock_vcpu_time_info *ref_hv_clock,
{
struct pvclock_vcpu_time_info *guest_hv_clock;
struct pvclock_vcpu_time_info hv_clock;
- unsigned long flags;
+ int idx;
memcpy(&hv_clock, ref_hv_clock, sizeof(hv_clock));
- read_lock_irqsave(&gpc->lock, flags);
+ idx = srcu_read_lock_atomic(&vcpu->kvm->gpc_srcu);
while (!kvm_gpc_check(gpc, offset + sizeof(*guest_hv_clock))) {
- read_unlock_irqrestore(&gpc->lock, flags);
+ srcu_read_unlock_atomic(&vcpu->kvm->gpc_srcu, idx);
if (kvm_gpc_refresh(gpc, offset + sizeof(*guest_hv_clock)))
return;
- read_lock_irqsave(&gpc->lock, flags);
+ idx = srcu_read_lock_atomic(&vcpu->kvm->gpc_srcu);
}
guest_hv_clock = (void *)(gpc->khva + offset);
@@ -1755,7 +1755,7 @@ static void kvm_setup_guest_pvclock(struct pvclock_vcpu_time_info *ref_hv_clock,
guest_hv_clock->version = ++hv_clock.version;
kvm_gpc_mark_dirty_in_slot(gpc);
- read_unlock_irqrestore(&gpc->lock, flags);
+ srcu_read_unlock_atomic(&vcpu->kvm->gpc_srcu, idx);
trace_kvm_pvclock_update(vcpu->vcpu_id, &hv_clock);
}
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 6aa00f746cba..ea907f5fa5f8 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -45,16 +45,17 @@ static int kvm_xen_shared_info_init(struct kvm *kvm)
u64 wall_nsec;
int ret = 0;
int idx = srcu_read_lock(&kvm->srcu);
+ int gpc_idx;
- read_lock_irq(&gpc->lock);
+ gpc_idx = srcu_read_lock_atomic(&kvm->gpc_srcu);
while (!kvm_gpc_check(gpc, PAGE_SIZE)) {
- read_unlock_irq(&gpc->lock);
+ srcu_read_unlock_atomic(&kvm->gpc_srcu, gpc_idx);
ret = kvm_gpc_refresh(gpc, PAGE_SIZE);
if (ret)
goto out;
- read_lock_irq(&gpc->lock);
+ gpc_idx = srcu_read_lock_atomic(&kvm->gpc_srcu);
}
/*
@@ -97,7 +98,7 @@ static int kvm_xen_shared_info_init(struct kvm *kvm)
smp_wmb();
wc->version = wc_version + 1;
- read_unlock_irq(&gpc->lock);
+ srcu_read_unlock_atomic(&kvm->gpc_srcu, gpc_idx);
out:
srcu_read_unlock(&kvm->srcu, idx);
@@ -154,22 +155,21 @@ static int xen_get_guest_pvclock(struct kvm_vcpu *vcpu,
struct gfn_to_pfn_cache *gpc,
unsigned int offset)
{
- unsigned long flags;
- int r;
+ int idx, r;
- read_lock_irqsave(&gpc->lock, flags);
+ idx = srcu_read_lock_atomic(&vcpu->kvm->gpc_srcu);
while (!kvm_gpc_check(gpc, offset + sizeof(*hv_clock))) {
- read_unlock_irqrestore(&gpc->lock, flags);
+ srcu_read_unlock_atomic(&vcpu->kvm->gpc_srcu, idx);
r = kvm_gpc_refresh(gpc, offset + sizeof(*hv_clock));
if (r)
return r;
- read_lock_irqsave(&gpc->lock, flags);
+ idx = srcu_read_lock_atomic(&vcpu->kvm->gpc_srcu);
}
memcpy(hv_clock, gpc->khva + offset, sizeof(*hv_clock));
- read_unlock_irqrestore(&gpc->lock, flags);
+ srcu_read_unlock_atomic(&vcpu->kvm->gpc_srcu, idx);
/*
* Sanity check TSC shift+multiplier to verify the guest's view of time
@@ -324,12 +324,12 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
struct gfn_to_pfn_cache *gpc2 = &vx->runstate2_cache;
size_t user_len, user_len1, user_len2;
struct vcpu_runstate_info rs;
- unsigned long flags;
size_t times_ofs;
uint8_t *update_bit = NULL;
uint64_t entry_time;
uint64_t *rs_times;
int *rs_state;
+ int idx;
/*
* The only difference between 32-bit and 64-bit versions of the
@@ -416,20 +416,12 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
retry:
/*
- * Attempt to obtain the GPC lock on *both* (if there are two)
- * gfn_to_pfn caches that cover the region.
+ * Check *both* (if there are two) gfn_to_pfn caches that cover
+ * the region, under a single SRCU read-side critical section.
*/
- if (atomic) {
- local_irq_save(flags);
- if (!read_trylock(&gpc1->lock)) {
- local_irq_restore(flags);
- return;
- }
- } else {
- read_lock_irqsave(&gpc1->lock, flags);
- }
+ idx = srcu_read_lock_atomic(&v->kvm->gpc_srcu);
while (!kvm_gpc_check(gpc1, user_len1)) {
- read_unlock_irqrestore(&gpc1->lock, flags);
+ srcu_read_unlock_atomic(&v->kvm->gpc_srcu, idx);
/* When invoked from kvm_sched_out() we cannot sleep */
if (atomic)
@@ -438,7 +430,7 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
if (kvm_gpc_refresh(gpc1, user_len1))
return;
- read_lock_irqsave(&gpc1->lock, flags);
+ idx = srcu_read_lock_atomic(&v->kvm->gpc_srcu);
}
if (likely(!user_len2)) {
@@ -458,24 +450,11 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
} else {
/*
* The guest's runstate_info is split across two pages and we
- * need to hold and validate both GPCs simultaneously. We can
- * declare a lock ordering GPC1 > GPC2 because nothing else
- * takes them more than one at a time. Set a subclass on the
- * gpc1 lock to make lockdep shut up about it.
+ * need to validate both GPCs simultaneously. They are both
+ * covered by the single SRCU read-side critical section above.
*/
- lock_set_subclass(&gpc1->lock.dep_map, 1, _THIS_IP_);
- if (atomic) {
- if (!read_trylock(&gpc2->lock)) {
- read_unlock_irqrestore(&gpc1->lock, flags);
- return;
- }
- } else {
- read_lock(&gpc2->lock);
- }
-
if (!kvm_gpc_check(gpc2, user_len2)) {
- read_unlock(&gpc2->lock);
- read_unlock_irqrestore(&gpc1->lock, flags);
+ srcu_read_unlock_atomic(&v->kvm->gpc_srcu, idx);
/* When invoked from kvm_sched_out() we cannot sleep */
if (atomic)
@@ -574,13 +553,11 @@ static void kvm_xen_update_runstate_guest(struct kvm_vcpu *v, bool atomic)
smp_wmb();
}
- if (user_len2) {
+ if (user_len2)
kvm_gpc_mark_dirty_in_slot(gpc2);
- read_unlock(&gpc2->lock);
- }
kvm_gpc_mark_dirty_in_slot(gpc1);
- read_unlock_irqrestore(&gpc1->lock, flags);
+ srcu_read_unlock_atomic(&v->kvm->gpc_srcu, idx);
}
void kvm_xen_update_runstate(struct kvm_vcpu *v, int state)
@@ -655,7 +632,7 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
{
unsigned long evtchn_pending_sel = READ_ONCE(v->arch.xen.evtchn_pending_sel);
struct gfn_to_pfn_cache *gpc = &v->arch.xen.vcpu_info_cache;
- unsigned long flags;
+ int idx;
if (!evtchn_pending_sel)
return;
@@ -665,14 +642,14 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
* does anyway. Page it in and retry the instruction. We're just a
* little more honest about it.
*/
- read_lock_irqsave(&gpc->lock, flags);
+ idx = srcu_read_lock_atomic(&v->kvm->gpc_srcu);
while (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) {
- read_unlock_irqrestore(&gpc->lock, flags);
+ srcu_read_unlock_atomic(&v->kvm->gpc_srcu, idx);
if (kvm_gpc_refresh(gpc, sizeof(struct vcpu_info)))
return;
- read_lock_irqsave(&gpc->lock, flags);
+ idx = srcu_read_lock_atomic(&v->kvm->gpc_srcu);
}
/* Now gpc->khva is a valid kernel address for the vcpu_info */
@@ -706,7 +683,7 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
}
kvm_gpc_mark_dirty_in_slot(gpc);
- read_unlock_irqrestore(&gpc->lock, flags);
+ srcu_read_unlock_atomic(&v->kvm->gpc_srcu, idx);
/* For the per-vCPU lapic vector, deliver it as MSI. */
if (v->arch.xen.upcall_vector)
@@ -716,8 +693,8 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
int __kvm_xen_has_interrupt(struct kvm_vcpu *v)
{
struct gfn_to_pfn_cache *gpc = &v->arch.xen.vcpu_info_cache;
- unsigned long flags;
u8 rc = 0;
+ int idx;
/*
* If the global upcall vector (HVMIRQ_callback_vector) is set and
@@ -742,9 +719,9 @@ int __kvm_xen_has_interrupt(struct kvm_vcpu *v)
*/
guard(srcu)(&v->kvm->srcu);
- read_lock_irqsave(&gpc->lock, flags);
+ idx = srcu_read_lock_atomic(&v->kvm->gpc_srcu);
while (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) {
- read_unlock_irqrestore(&gpc->lock, flags);
+ srcu_read_unlock_atomic(&v->kvm->gpc_srcu, idx);
/*
* This function gets called from kvm_vcpu_block() after setting the
@@ -764,11 +741,11 @@ int __kvm_xen_has_interrupt(struct kvm_vcpu *v)
*/
return 0;
}
- read_lock_irqsave(&gpc->lock, flags);
+ idx = srcu_read_lock_atomic(&v->kvm->gpc_srcu);
}
rc = ((struct vcpu_info *)gpc->khva)->evtchn_upcall_pending;
- read_unlock_irqrestore(&gpc->lock, flags);
+ srcu_read_unlock_atomic(&v->kvm->gpc_srcu, idx);
return rc;
}
@@ -1480,14 +1457,13 @@ static bool wait_pending_event(struct kvm_vcpu *vcpu, bool has_64bit_shinfo,
struct kvm *kvm = vcpu->kvm;
struct gfn_to_pfn_cache *gpc = &kvm->arch.xen.shinfo_cache;
unsigned long *pending_bits;
- unsigned long flags;
bool ret = true;
- int idx, i;
+ int idx, gpc_idx, i;
idx = srcu_read_lock(&kvm->srcu);
- read_lock_irqsave(&gpc->lock, flags);
+ gpc_idx = srcu_read_lock_atomic(&kvm->gpc_srcu);
if (!kvm_gpc_check(gpc, PAGE_SIZE))
- goto out_rcu;
+ goto out_gpc_srcu;
ret = false;
if (has_64bit_shinfo) {
@@ -1505,8 +1481,8 @@ static bool wait_pending_event(struct kvm_vcpu *vcpu, bool has_64bit_shinfo,
}
}
- out_rcu:
- read_unlock_irqrestore(&gpc->lock, flags);
+ out_gpc_srcu:
+ srcu_read_unlock_atomic(&kvm->gpc_srcu, gpc_idx);
srcu_read_unlock(&kvm->srcu, idx);
return ret;
@@ -1847,10 +1823,9 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
bool has_64bit_shinfo = kvm_xen_has_64bit_shinfo(kvm);
unsigned long *pending_bits, *mask_bits, vi_pending_sel_ofs;
struct kvm_vcpu *vcpu;
- unsigned long flags;
int port_word_bit;
bool kick_vcpu = false;
- int vcpu_idx, idx, rc;
+ int vcpu_idx, idx, gpc_idx, rc;
vcpu_idx = READ_ONCE(xe->vcpu_idx);
if (vcpu_idx >= 0)
@@ -1869,9 +1844,9 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
idx = srcu_read_lock(&kvm->srcu);
- read_lock_irqsave(&gpc->lock, flags);
+ gpc_idx = srcu_read_lock_atomic(&kvm->gpc_srcu);
if (!kvm_gpc_check(gpc, PAGE_SIZE))
- goto out_rcu;
+ goto out_gpc_srcu;
if (has_64bit_shinfo) {
struct shared_info *shinfo = gpc->khva;
@@ -1909,10 +1884,8 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
rc = 1; /* Delivered to the bitmap in shared_info. */
/* Now switch to the vCPU's vcpu_info to set the index and pending_sel */
- read_unlock_irqrestore(&gpc->lock, flags);
gpc = &vcpu->arch.xen.vcpu_info_cache;
- read_lock_irqsave(&gpc->lock, flags);
if (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) {
/*
* Could not access the vcpu_info. Set the bit in-kernel
@@ -1920,7 +1893,7 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
*/
if (!test_and_set_bit(port_word_bit, &vcpu->arch.xen.evtchn_pending_sel))
kick_vcpu = true;
- goto out_rcu;
+ goto out_gpc_srcu;
}
/*
@@ -1955,8 +1928,8 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
}
}
- out_rcu:
- read_unlock_irqrestore(&gpc->lock, flags);
+ out_gpc_srcu:
+ srcu_read_unlock_atomic(&kvm->gpc_srcu, gpc_idx);
srcu_read_unlock(&kvm->srcu, idx);
if (kick_vcpu) {
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 3dd04605f2e5..9da947eb08ed 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -806,6 +806,15 @@ struct kvm {
/* For management / invalidation of gfn_to_pfn_caches */
spinlock_t gpc_lock;
struct list_head gpc_list;
+ /*
+ * Protects the contents of active gfn_to_pfn_caches. Readers hold it
+ * across kvm_gpc_check() and any subsequent access to gpc->khva; the
+ * invalidation and refresh paths clear gpc->valid and then wait for a
+ * grace period before mutating anything a reader might be looking at.
+ * A dedicated domain (rather than kvm->srcu) keeps those waits from
+ * being lengthened by unrelated memslot readers.
+ */
+ struct srcu_struct gpc_srcu;
/*
* created_vcpus is protected by kvm->lock, and is incremented
@@ -1510,12 +1519,9 @@ int kvm_gpc_activate_hva(struct gfn_to_pfn_cache *gpc, unsigned long hva, unsign
* @return: %true if the cache is still valid and the address matches.
* %false if the cache is not valid.
*
- * Callers outside IN_GUEST_MODE context should hold a read lock on @gpc->lock
- * while calling this function, and then continue to hold the lock until the
- * access is complete.
- *
- * Callers in IN_GUEST_MODE may do so without locking, although they should
- * still hold a read lock on kvm->scru for the memslot checks.
+ * Callers must hold a read lock on @gpc->kvm->gpc_srcu across this function
+ * and any subsequent access to the target page, and must hold a read lock on
+ * kvm->srcu for the memslot checks.
*/
bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len);
@@ -1532,8 +1538,8 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len);
* This will attempt to refresh a gfn_to_pfn_cache. Note that a successful
* return from this function does not mean the page can be immediately
* accessed because it may have raced with an invalidation. Callers must
- * still lock and check the cache status, as this function does not return
- * with the lock still held to permit access.
+ * still check the cache status under a kvm->gpc_srcu read lock, via
+ * kvm_gpc_check(), before accessing the target page.
*/
int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, unsigned long len);
@@ -1974,7 +1980,8 @@ static inline bool kvm_is_gpa_in_memslot(struct kvm *kvm, gpa_t gpa)
static inline void kvm_gpc_mark_dirty_in_slot(struct gfn_to_pfn_cache *gpc)
{
- lockdep_assert_held(&gpc->lock);
+ RCU_LOCKDEP_WARN(!srcu_read_lock_held(&gpc->kvm->gpc_srcu),
+ "kvm_gpc_mark_dirty_in_slot() without kvm->gpc_srcu read lock");
if (!gpc->memslot)
return;
diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
index a568d8e6f4e8..31f3eb0dd2aa 100644
--- a/include/linux/kvm_types.h
+++ b/include/linux/kvm_types.h
@@ -88,14 +88,47 @@ struct gfn_to_pfn_cache {
struct kvm_memory_slot *memslot;
struct kvm *kvm;
struct list_head list;
- rwlock_t lock;
struct mutex refresh_lock;
void *khva;
kvm_pfn_t pfn;
bool active;
- bool valid;
+ /*
+ * GPC_VALID, GPC_BECOMING_VALID and GPC_INVALIDATING live in a
+ * single atomic word so that a refresh can publish
+ * (BECOMING_VALID => VALID) with a single conditional RMW, an
+ * invalidation can veto that publish by clearing both of those bits
+ * (setting INVALIDATING in the same operation if VALID was set, to
+ * record that its grace period is still owed), and a mutator can
+ * consume the whole state with one xchg to learn whether it must
+ * wait for readers before touching anything they see. See the
+ * comments in pfncache.c.
+ *
+ * 'active' deliberately stays separate: the invalidation and
+ * mutation paths operate on the word without having to care about
+ * racing with deactivation, which is ordered by refresh_lock
+ * instead.
+ */
+ atomic_t state;
};
+/* The cache is valid: pfn and khva may be used by readers. */
+#define GPC_VALID 0x1
+/*
+ * A refresh is in flight and intends to publish. Set before the uhva is
+ * resolved, so an invalidation of that uhva can veto the publish even
+ * though the cache is not yet valid.
+ */
+#define GPC_BECOMING_VALID 0x2
+/*
+ * GPC_VALID has been cleared but the grace period for readers of the
+ * previously published mapping has not yet completed. Nothing may mutate
+ * reader-visible fields (pfn, khva, uhva, gpa, memslot) until whoever
+ * next needs to do so has waited it out. Set by whichever party clears
+ * GPC_VALID; cleared after a synchronize_srcu() which began after the
+ * VALID clear.
+ */
+#define GPC_INVALIDATING 0x4
+
#ifdef KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE
/*
* Memory caches are used to preallocate memory ahead of various MMU flows,
diff --git a/include/linux/srcu.h b/include/linux/srcu.h
index 1a8a465a5650..f7a6bbcabc4e 100644
--- a/include/linux/srcu.h
+++ b/include/linux/srcu.h
@@ -706,6 +706,13 @@ DEFINE_LOCK_GUARD_1(srcu, struct srcu_struct,
DECLARE_LOCK_GUARD_1_ATTRS(srcu, __acquires_shared(_T), __releases_shared(*(struct srcu_struct **)_T))
#define class_srcu_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(srcu, _T)
+DEFINE_LOCK_GUARD_1(srcu_atomic, struct srcu_struct,
+ _T->idx = srcu_read_lock_atomic(_T->lock),
+ srcu_read_unlock_atomic(_T->lock, _T->idx),
+ int idx)
+DECLARE_LOCK_GUARD_1_ATTRS(srcu_atomic, __acquires_shared(_T), __releases_shared(*(struct srcu_struct **)_T))
+#define class_srcu_atomic_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(srcu_atomic, _T)
+
DEFINE_LOCK_GUARD_1(srcu_fast, struct srcu_struct,
_T->scp = srcu_read_lock_fast(_T->lock),
srcu_read_unlock_fast(_T->lock, _T->scp),
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 108d42c5c1d6..bf0c21969726 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -1152,6 +1152,8 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
goto out_err_no_srcu;
if (init_srcu_struct(&kvm->irq_srcu))
goto out_err_no_irq_srcu;
+ if (init_srcu_struct_atomic(&kvm->gpc_srcu))
+ goto out_err_no_gpc_srcu;
r = kvm_init_irq_routing(kvm);
if (r)
@@ -1232,6 +1234,8 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
kfree(kvm_get_bus_for_destruction(kvm, i));
kvm_free_irq_routing(kvm);
out_err_no_irq_routing:
+ cleanup_srcu_struct(&kvm->gpc_srcu);
+out_err_no_gpc_srcu:
cleanup_srcu_struct(&kvm->irq_srcu);
out_err_no_irq_srcu:
cleanup_srcu_struct(&kvm->srcu);
@@ -1308,6 +1312,11 @@ static void kvm_destroy_vm(struct kvm *kvm)
kvm_free_memslots(kvm, &kvm->__memslots[i][0]);
kvm_free_memslots(kvm, &kvm->__memslots[i][1]);
}
+ /*
+ * All gfn_to_pfn_caches have been deactivated by kvm_arch_destroy_vm()
+ * above, so there can be no readers or pending invalidations left.
+ */
+ cleanup_srcu_struct(&kvm->gpc_srcu);
cleanup_srcu_struct(&kvm->irq_srcu);
srcu_barrier(&kvm->srcu);
cleanup_srcu_struct(&kvm->srcu);
diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index 3659686b97c2..067da05a87ac 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -26,35 +26,115 @@ void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm, unsigned long start,
unsigned long end)
{
struct gfn_to_pfn_cache *gpc;
+ bool cleared = false;
spin_lock(&kvm->gpc_lock);
list_for_each_entry(gpc, &kvm->gpc_list, list) {
- read_lock_irq(&gpc->lock);
+ /*
+ * A cache covers only a single page, keyed by its uhva, so
+ * there is no length to consider.
+ *
+ * The acquire pairs with the release-publish in
+ * hva_to_pfn_retry() (for GPC_VALID) and with the
+ * smp_mb__before_atomic() before the GPC_BECOMING_VALID
+ * announce (for that bit): whichever is seen, the uhva read
+ * below is guaranteed to see the value stored before it. A
+ * stale uhva paired with a fresh state bit could otherwise
+ * cause a cache whose new uhva is in the invalidated range
+ * to be skipped.
+ *
+ * uhva is read once, with READ_ONCE(), and only after the
+ * state (the acquire provides that ordering): it is written
+ * (with WRITE_ONCE()) by __kvm_gpc_refresh() without holding
+ * gpc_lock. Note the refresh only mutates uhva while the
+ * state word is 0 — after its xchg, before its announce —
+ * so this walk cannot observe a half-updated cache: if
+ * either state bit is visible, the uhva is current.
+ *
+ * Keying on GPC_BECOMING_VALID as well as GPC_VALID means a
+ * cache whose refresh is in flight is *not* skipped: its
+ * uhva is already known at that point, and clearing the bit
+ * vetoes the pending publish. Testing only GPC_VALID would
+ * leave the window between the refresh's last invalidation
+ * check and its publish unprotected. (For an invalidation
+ * which begins before that check, this veto is belt and
+ * braces: the mn_active_invalidate_count / gpc_invalidate_seq
+ * check makes the refresh retry anyway. The window after
+ * that check has only the veto.)
+ */
+ unsigned long gpc_uhva;
+ int old, new;
- /* Only a single page so no need to care about length */
- if (gpc->valid && !is_error_noslot_pfn(gpc->pfn) &&
- gpc->uhva >= start && gpc->uhva < end) {
- read_unlock_irq(&gpc->lock);
+ old = atomic_read_acquire(&gpc->state);
+ if (!(old & (GPC_VALID | GPC_BECOMING_VALID)))
+ continue;
+ gpc_uhva = READ_ONCE(gpc->uhva);
+ if (gpc_uhva >= start && gpc_uhva < end) {
/*
- * There is a small window here where the cache could
- * be modified, and invalidation would no longer be
- * necessary. Hence check again whether invalidation
- * is still necessary once the write lock has been
- * acquired.
+ * Clear VALID and BECOMING_VALID; if VALID was set,
+ * also set INVALIDATING so that anyone else who wants
+ * to mutate reader-visible fields before our grace
+ * period below completes knows to wait for one of
+ * their own. (A cache which was merely becoming
+ * valid has no readers: the veto alone suffices.)
+ *
+ * 'old' is seeded from the read above; if the state
+ * changed since (e.g. the refresh published), the
+ * cmpxchg fails, reloads it, and goes around.
*/
-
- write_lock_irq(&gpc->lock);
- if (gpc->valid && !is_error_noslot_pfn(gpc->pfn) &&
- gpc->uhva >= start && gpc->uhva < end)
- gpc->valid = false;
- write_unlock_irq(&gpc->lock);
- continue;
+ do {
+ new = old & ~(GPC_VALID | GPC_BECOMING_VALID);
+ if (old & GPC_VALID)
+ new |= GPC_INVALIDATING;
+ } while (!atomic_try_cmpxchg(&gpc->state, &old, new));
+
+ if (old & GPC_VALID)
+ cleared = true;
}
-
- read_unlock_irq(&gpc->lock);
}
spin_unlock(&kvm->gpc_lock);
+
+ /*
+ * Readers may still be using the old mapping, having sampled
+ * GPC_VALID before it was cleared. Wait for them all to drain
+ * before the caller proceeds to zap the page tables; like a TLB
+ * shootdown, this guarantees no access via the stale mapping
+ * once the invalidation completes.
+ *
+ * This wait is safe even on unblockable ranges (the OOM reaper):
+ * the constraint there is not "no sleeping" but "no blocking on
+ * anything which may itself depend on memory allocation to make
+ * progress" (see the reasoning in commit 312364f3534c and its
+ * discussion). A GPC read-side critical section has no such
+ * dependency: readers never allocate, never take mmap_lock, and
+ * never sleep.
+ *
+ * The expedited variant: this stalls progress of the primary MMU
+ * operation which triggered the invalidation, and the readers being
+ * drained are bounded, short, non-sleeping sections. Expedited SRCU
+ * does not IPI; it only polls the (per-VM) domain more aggressively.
+ *
+ * (The one allocation in the SRCU grace-period machinery itself —
+ * the srcu_node tree on the one-time upgrade of a contended
+ * srcu_struct — is dealt with separately, by having srcu_gp_end()
+ * allocate with GFP_NOWAIT and fall back to a preallocated spare,
+ * so the grace period never blocks in reclaim either.)
+ */
+ if (cleared)
+ synchronize_srcu_expedited(&kvm->gpc_srcu);
+
+ /*
+ * Note the GPC_INVALIDATING markers set above are deliberately NOT
+ * cleared here. Clearing them safely requires knowing that no
+ * *other* invalidation's grace period is still covering the same
+ * cache (consider a republish-and-reinvalidate cycle completing
+ * entirely within this synchronize_srcu()). Only a party which is
+ * about to mutate the cache — and which therefore performs its own
+ * wait, necessarily beginning after the VALID clear which set the
+ * marker — can meaningfully clear it. Readers ignore the bit; if no
+ * mutation ever follows, it sits there harmlessly.
+ */
}
static bool kvm_gpc_is_valid_len(gpa_t gpa, unsigned long uhva,
@@ -74,6 +154,22 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len)
{
struct kvm_memslots *slots = kvm_memslots(gpc->kvm);
+ RCU_LOCKDEP_WARN(!srcu_read_lock_held(&gpc->kvm->gpc_srcu),
+ "kvm_gpc_check() without kvm->gpc_srcu read lock");
+
+ /*
+ * Check valid *first*. The acquire pairs with the release-publish
+ * in hva_to_pfn_retry(), so every field read below — and any use
+ * of gpc->khva by the caller — is guaranteed to be from the
+ * published generation, not a stale value reordered from before
+ * the publish. The fields are then stable for the remainder of
+ * the RCU read-side critical section, because every mutator
+ * clears valid and waits a full grace period before changing
+ * anything.
+ */
+ if (!(atomic_read_acquire(&gpc->state) & GPC_VALID))
+ return false;
+
if (!gpc->active)
return false;
@@ -81,7 +177,8 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len)
* If the page was cached from a memslot, make sure the memslots have
* not been re-configured.
*/
- if (!kvm_is_error_gpa(gpc->gpa) && gpc->generation != slots->generation)
+ if (!kvm_is_error_gpa(gpc->gpa) &&
+ READ_ONCE(gpc->generation) != slots->generation)
return false;
if (kvm_is_error_hva(gpc->uhva))
@@ -90,9 +187,6 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len)
if (!kvm_gpc_is_valid_len(gpc->gpa, gpc->uhva, len))
return false;
- if (!gpc->valid)
- return false;
-
return true;
}
@@ -134,8 +228,8 @@ static inline bool mmu_notifier_retry_cache(struct kvm *kvm, unsigned long gpc_s
* is elevated.
*
* Note, it does not matter that mn_active_invalidate_count
- * is not protected by gpc->lock. It is guaranteed to
- * be elevated before the mmu_notifier acquires gpc->lock, and
+ * is not protected by any lock the refresher holds. It is
+ * guaranteed to be elevated before the mmu_notifier walk, and
* isn't dropped until after gpc_invalidate_seq is updated.
*/
if (kvm->mn_active_invalidate_count)
@@ -156,10 +250,13 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc)
{
/* Note, the new page offset may be different than the old! */
void *old_khva = (void *)PAGE_ALIGN_DOWN((uintptr_t)gpc->khva);
+ void *old_khva_exact = gpc->khva;
+ kvm_pfn_t old_pfn = gpc->pfn;
kvm_pfn_t new_pfn = KVM_PFN_ERR_FAULT;
void *new_khva = NULL;
unsigned long gpc_seq;
struct page *page;
+ int state;
struct kvm_follow_pfn kfp = {
.slot = gpc->memslot,
@@ -171,21 +268,32 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc)
lockdep_assert_held(&gpc->refresh_lock);
- lockdep_assert_held_write(&gpc->lock);
-
- /*
- * Invalidate the cache prior to dropping gpc->lock, the gpa=>uhva
- * assets have already been updated and so a concurrent check() from a
- * different task may not fail the gpa/uhva/generation checks.
- */
- gpc->valid = false;
+ WARN_ON_ONCE(atomic_read(&gpc->state) & GPC_VALID);
do {
+ /*
+ * Announce the intent to publish before the uhva is resolved
+ * and before any lock is dropped. From here on an
+ * invalidation of this uhva will clear the bit, and the
+ * cmpxchg below will then fail and send us around again.
+ * This is what makes the check-and-publish atomic with
+ * respect to the mmu_notifier walk without holding a lock
+ * across it.
+ *
+ * The barrier orders the caller's gpc->uhva store before the
+ * bit becoming visible (atomic_or() alone is unordered, and
+ * x86's lock-or being a full barrier must not be relied on
+ * in generic code). It pairs with the atomic_read_acquire()
+ * in gfn_to_pfn_cache_invalidate_start(): a walker which
+ * observes the bit observes the uhva this refresh is
+ * resolving, and so aims its veto correctly.
+ */
+ smp_mb__before_atomic();
+ atomic_or(GPC_BECOMING_VALID, &gpc->state);
+
gpc_seq = gpc->kvm->gpc_invalidate_seq;
smp_rmb();
- write_unlock_irq(&gpc->lock);
-
/*
* If the previous iteration "failed" due to an mmu_notifier
* event, release the pfn and unmap the kernel virtual address
@@ -213,9 +321,15 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc)
/*
* Obtain a new kernel mapping if KVM itself will access the
* pfn. Note, kmap() and memremap() can both sleep, so this
- * too must be done outside of gpc->lock!
+ * can sleep, which is fine: this path holds no spinning locks.
*/
- if (new_pfn == gpc->pfn)
+ /*
+ * Compare against the *published* pfn captured at entry, via
+ * a local: gpc->pfn must not be consulted mid-loop. (A
+ * previous iteration's vetoed publish attempt must not be
+ * able to leak its doomed pfn into this comparison.)
+ */
+ if (new_pfn == old_pfn)
new_khva = old_khva;
else
new_khva = gpc_map(new_pfn);
@@ -225,18 +339,39 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc)
goto out_error;
}
- write_lock_irq(&gpc->lock);
+ if (mmu_notifier_retry_cache(gpc->kvm, gpc_seq))
+ continue;
+
+ gpc->pfn = new_pfn;
+ gpc->khva = new_khva + offset_in_page(gpc->uhva);
/*
- * Other tasks must wait for _this_ refresh to complete before
- * attempting to refresh.
+ * Publish: BECOMING_VALID => VALID, but only if the bit is
+ * still set. If an invalidation of this uhva landed at any
+ * point since it was set — including in the window after the
+ * retry check just above — the bit is gone, the cmpxchg fails
+ * and we go around again. The release ordering pairs with the
+ * atomic_read_acquire() in kvm_gpc_check() so the pfn/khva
+ * stores are visible before GPC_VALID is. (The stores must
+ * precede the VALID publish; a reader which observes VALID
+ * must observe the fields it covers.)
*/
- WARN_ON_ONCE(gpc->valid);
- } while (mmu_notifier_retry_cache(gpc->kvm, gpc_seq));
+ state = GPC_BECOMING_VALID;
+ if (atomic_try_cmpxchg_release(&gpc->state, &state, GPC_VALID))
+ break;
- gpc->valid = true;
- gpc->pfn = new_pfn;
- gpc->khva = new_khva + offset_in_page(gpc->uhva);
+ /*
+ * Vetoed: an invalidation cleared GPC_BECOMING_VALID after
+ * the retry check above. No reader ever saw the values just
+ * stored (VALID was never set for them), but the loop's own
+ * cleanup and aliasing logic must not see them either —
+ * restore the published values. This runs under refresh_lock
+ * with the cache unpublished, so it is single-writer plain
+ * stores.
+ */
+ gpc->pfn = old_pfn;
+ gpc->khva = old_khva_exact;
+ } while (1);
/*
* Put the reference to the _new_ page. The page is now tracked by the
@@ -248,8 +383,19 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_cache *gpc)
return 0;
out_error:
- write_lock_irq(&gpc->lock);
-
+ /*
+ * Withdraw the announcement: no publish is coming. Leaving the bit
+ * set would be harmless — an invalidation walk would merely veto a
+ * publish which was never going to happen, and the next refresh or
+ * deactivation starts by xchg'ing the whole state word to zero —
+ * but clearing it here keeps the invariant that GPC_BECOMING_VALID
+ * means a refresh is between announce and publish. Only VALID can
+ * have been set here by anyone else, and only by our own publish
+ * (which didn't happen), and INVALIDATING only by whoever clears
+ * VALID; the sole concurrent writer is an invalidation walk also
+ * clearing this bit, so a plain andnot cannot lose an update.
+ */
+ atomic_andnot(GPC_BECOMING_VALID, &gpc->state);
return -EFAULT;
}
@@ -259,7 +405,7 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
bool unmap_old = false;
unsigned long old_uhva;
kvm_pfn_t old_pfn;
- bool hva_change = false;
+ bool must_drain;
void *old_khva;
int ret;
@@ -269,13 +415,94 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
lockdep_assert_held(&gpc->refresh_lock);
- write_lock_irq(&gpc->lock);
+ if (!gpc->active)
+ return -EINVAL;
- if (!gpc->active) {
- ret = -EINVAL;
- goto out_unlock;
+ /*
+ * Resolve the target uHVA (and memslot, for a GPA-based cache) before
+ * deciding whether anything needs to be invalidated at all.
+ *
+ * If the cache is still valid and this refresh resolves to exactly the
+ * same uHVA, in the same memslot, for the same GPA, then nothing which
+ * a reader can observe is changing: the gPA => uHVA translation is
+ * unchanged, and GPC_VALID being set is precisely the assertion that
+ * the second stage (uHVA => PFN, and the kernel mapping of it) is
+ * still good. No mapping is retired, so there is nothing for a grace
+ * period to wait for. At most the memslot generation needs updating,
+ * which no reader consults except via kvm_gpc_check() itself.
+ *
+ * All of the following must hold to take this shortcut:
+ *
+ * - GPC_VALID: the PFN and its kernel mapping are still good.
+ * - the resolved uHVA is unchanged. kvm_gpc_check() validates
+ * 'offset + len <= PAGE_SIZE' from gpc->uhva/gpa on the reader's
+ * behalf, and the reader then accesses gpc->khva for 'len' bytes
+ * with no recheck; moving the offset under a live reader would let
+ * it run off the end of the page.
+ * - the memslot is unchanged. kvm_gpc_mark_dirty_in_slot() uses
+ * gpc->memslot, so replacing it under a reader could mark the wrong
+ * slot dirty and thus lose a dirty page for live migration.
+ * - the gPA is unchanged, as it provides the gfn for dirty tracking.
+ */
+ if (atomic_read(&gpc->state) & GPC_VALID) {
+ struct kvm_memory_slot *new_slot = gpc->memslot;
+ unsigned long new_uhva = KVM_HVA_ERR_BAD;
+ u64 new_generation = gpc->generation;
+
+ if (kvm_is_error_gpa(gpa)) {
+ new_uhva = uhva;
+ } else {
+ struct kvm_memslots *slots = kvm_memslots(gpc->kvm);
+ gfn_t gfn = gpa_to_gfn(gpa);
+
+ new_generation = slots->generation;
+ new_slot = __gfn_to_memslot(slots, gfn);
+ if (new_slot)
+ new_uhva = gfn_to_hva_memslot(new_slot, gfn) +
+ offset_in_page(gpa);
+ }
+
+ if (!kvm_is_error_hva(new_uhva) && new_uhva == gpc->uhva &&
+ new_slot == gpc->memslot && gpa == gpc->gpa) {
+ /*
+ * Nothing to invalidate. A concurrent reader may be
+ * using the cache right now and can safely continue
+ * to do so; only the memslot generation, which is
+ * read solely by kvm_gpc_check(), may need updating.
+ */
+ if (new_generation != gpc->generation)
+ WRITE_ONCE(gpc->generation, new_generation);
+
+ return 0;
+ }
}
+ /*
+ * Take the cache invalid and wait for all current readers to
+ * drain before mutating anything they might be looking at. Once
+ * the grace period has elapsed, this task (serialized by
+ * refresh_lock) owns all the cache fields exclusively: readers
+ * check valid (with an acquire load) inside their SRCU read-side
+ * critical sections and back off. This mirrors what a TLB
+ * shootdown does for the hardware page tables.
+ *
+ * The clear and the test are one operation on purpose, and the wait
+ * triggers on GPC_INVALIDATING as well as GPC_VALID: an mmu_notifier
+ * invalidation which cleared GPC_VALID a moment ago is not
+ * necessarily finished — its grace period may still be running, with
+ * readers which sampled GPC_VALID before the clear still
+ * dereferencing gpc->khva. GPC_INVALIDATING is set by whoever clears
+ * GPC_VALID precisely so that this path knows those readers may
+ * exist. Our own grace-period wait below necessarily begins after
+ * that clear, so waiting here covers them; only then may
+ * reader-visible fields (uhva, gpa, memslot, and later pfn/khva) be
+ * mutated or the old mapping torn down.
+ */
+ must_drain = atomic_xchg(&gpc->state, 0) &
+ (GPC_VALID | GPC_INVALIDATING);
+ if (must_drain)
+ synchronize_srcu_expedited(&gpc->kvm->gpc_srcu);
+
old_pfn = gpc->pfn;
old_khva = (void *)PAGE_ALIGN_DOWN((uintptr_t)gpc->khva);
old_uhva = PAGE_ALIGN_DOWN(gpc->uhva);
@@ -285,10 +512,14 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
gpc->gpa = INVALID_GPA;
gpc->memslot = NULL;
- gpc->uhva = PAGE_ALIGN_DOWN(uhva);
-
- if (gpc->uhva != old_uhva)
- hva_change = true;
+ /*
+ * uhva stores are WRITE_ONCE(): the invalidation walks read
+ * it without holding refresh_lock. This mutation window is
+ * safe because the state word is 0 here (cleared by the
+ * xchg above, announced again only in hva_to_pfn_retry()
+ * after these stores), so no walk will match this cache.
+ */
+ WRITE_ONCE(gpc->uhva, PAGE_ALIGN_DOWN(uhva));
} else {
struct kvm_memslots *slots = kvm_memslots(gpc->kvm);
@@ -301,43 +532,33 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
gpc->gpa = gpa;
gpc->generation = slots->generation;
gpc->memslot = __gfn_to_memslot(slots, gfn);
- gpc->uhva = gfn_to_hva_memslot(gpc->memslot, gfn);
+ WRITE_ONCE(gpc->uhva, gfn_to_hva_memslot(gpc->memslot, gfn));
if (kvm_is_error_hva(gpc->uhva)) {
ret = -EFAULT;
goto out;
}
- /*
- * Even if the GPA and/or the memslot generation changed, the
- * HVA may still be the same.
- */
- if (gpc->uhva != old_uhva)
- hva_change = true;
+
} else {
- gpc->uhva = old_uhva;
+ WRITE_ONCE(gpc->uhva, old_uhva);
}
}
/* Note: the offset must be correct before calling hva_to_pfn_retry() */
- gpc->uhva += page_offset;
+ WRITE_ONCE(gpc->uhva, gpc->uhva + page_offset);
/*
- * If the userspace HVA changed or the PFN was already invalid,
- * drop the lock and do the HVA to PFN lookup again.
+ * Always redo the HVA to PFN lookup: an invalidation of our uhva
+ * may have raced with (or followed) the valid-clearing above, in
+ * which case the notifier walk skipped this cache (valid was
+ * already false) and the old pfn may already be stale. The
+ * gpc_invalidate_seq check in hva_to_pfn_retry() is what detects
+ * that race, so the lookup path is the only safe way to publish.
+ * If the mapping is in fact unchanged, hva_to_pfn_retry() reuses
+ * the existing kernel mapping for the same pfn.
*/
- if (!gpc->valid || hva_change) {
- ret = hva_to_pfn_retry(gpc);
- } else {
- /*
- * If the HVA→PFN mapping was already valid, don't unmap it.
- * But do update gpc->khva because the offset within the page
- * may have changed.
- */
- gpc->khva = old_khva + page_offset;
- ret = 0;
- goto out_unlock;
- }
+ ret = hva_to_pfn_retry(gpc);
out:
/*
@@ -346,17 +567,12 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
* valid, leave it as is.
*/
if (ret) {
- gpc->valid = false;
+ WARN_ON_ONCE(atomic_read(&gpc->state) & GPC_VALID);
gpc->pfn = KVM_PFN_ERR_FAULT;
gpc->khva = NULL;
}
- /* Detect a pfn change before dropping the lock! */
unmap_old = (old_pfn != gpc->pfn);
-
-out_unlock:
- write_unlock_irq(&gpc->lock);
-
if (unmap_old)
gpc_unmap(old_pfn, old_khva);
@@ -384,14 +600,14 @@ int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, unsigned long len)
void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm)
{
- rwlock_init(&gpc->lock);
mutex_init(&gpc->refresh_lock);
gpc->kvm = kvm;
gpc->pfn = KVM_PFN_ERR_FAULT;
gpc->gpa = INVALID_GPA;
gpc->uhva = KVM_HVA_ERR_BAD;
- gpc->active = gpc->valid = false;
+ gpc->active = false;
+ atomic_set(&gpc->state, 0);
}
static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long uhva,
@@ -405,7 +621,7 @@ static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned
guard(mutex)(&gpc->refresh_lock);
if (!gpc->active) {
- if (KVM_BUG_ON(gpc->valid, kvm))
+ if (KVM_BUG_ON(atomic_read(&gpc->state) & GPC_VALID, kvm))
return -EIO;
spin_lock(&kvm->gpc_lock);
@@ -415,11 +631,11 @@ static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned
/*
* Activate the cache after adding it to the list, a concurrent
* refresh must not establish a mapping until the cache is
- * reachable by mmu_notifier events.
+ * reachable by mmu_notifier events. (Refreshes are serialized
+ * by refresh_lock, which we hold; the store ordering matters
+ * only against the notifier walk, which holds gpc_lock.)
*/
- write_lock_irq(&gpc->lock);
- gpc->active = true;
- write_unlock_irq(&gpc->lock);
+ WRITE_ONCE(gpc->active, true);
}
return __kvm_gpc_refresh(gpc, gpa, uhva);
}
@@ -454,13 +670,24 @@ void kvm_gpc_deactivate(struct gfn_to_pfn_cache *gpc)
if (gpc->active) {
/*
- * Deactivate the cache before removing it from the list, KVM
- * must stall mmu_notifier events until all users go away, i.e.
- * until gpc->lock is dropped and refresh is guaranteed to fail.
+ * Mark the cache inactive and invalid, and wait for all
+ * current readers to drain, before tearing down the mapping
+ * they may have been using. Refreshes are excluded by
+ * refresh_lock, which we hold.
+ */
+ WRITE_ONCE(gpc->active, false);
+
+ /*
+ * As in __kvm_gpc_refresh(): clear and test in one
+ * operation, waiting on GPC_INVALIDATING as well as
+ * GPC_VALID. An invalidation which cleared GPC_VALID first
+ * may still be inside its own synchronize_srcu() with
+ * readers outstanding — it left GPC_INVALIDATING set to say
+ * so — and the unmap below must not run under those readers.
*/
- write_lock_irq(&gpc->lock);
- gpc->active = false;
- gpc->valid = false;
+ if (atomic_xchg(&gpc->state, 0) &
+ (GPC_VALID | GPC_INVALIDATING))
+ synchronize_srcu_atomic(&kvm->gpc_srcu);
/*
* Leave the GPA => uHVA cache intact, it's protected by the
@@ -473,7 +700,6 @@ void kvm_gpc_deactivate(struct gfn_to_pfn_cache *gpc)
old_pfn = gpc->pfn;
gpc->pfn = KVM_PFN_ERR_FAULT;
- write_unlock_irq(&gpc->lock);
spin_lock(&kvm->gpc_lock);
list_del(&gpc->list);
--
2.55.0
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 01/17] KVM: pfncache: Use atomic SRCU for readers instead of a rwlock
2026-09-20 20:49 ` [PATCH 01/17] KVM: pfncache: Use atomic SRCU for readers instead of a rwlock David Woodhouse
@ 2026-09-20 21:55 ` sashiko-bot
2026-09-21 1:42 ` Hillf Danton
1 sibling, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-09-20 21:55 UTC (permalink / raw)
To: David Woodhouse; +Cc: kvm, linux-rt-devel, linux-kernel
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Critical] kvm: concurrent MMU notifiers skip SRCU wait causing Use-After-Free
- [High] kvm: mmu_notifier_invalidate_range_start unconditionally sleeps, breaking OOM reaper
- [Low] kvm: compilation failure due to fabricated atomic SRCU API
--
commit 47eed0ef4a708a1117920edb2f9b64f3a7b2a8d4
Author: David Woodhouse <dwmw@amazon.co.uk>
KVM: pfncache: Use atomic SRCU for readers instead of a rwlock
This commit replaces the per-cache rwlock with a dedicated per-VM atomic
SRCU domain (kvm->gpc_srcu) for the read side in the KVM pfncache. Readers
now run under an atomic SRCU read lock, allowing them to execute in hardirq
and sched-out paths without spinning or failing.
Reported-by: syzbot+208f7f3e5f59c11aeb90@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=208f7f3e5f59c11aeb90
Reported-by: syzbot+919877893c9d28162dc2@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=919877893c9d28162dc2
> diff --git a/include/linux/srcu.h b/include/linux/srcu.h
> --- a/include/linux/srcu.h
> +++ b/include/linux/srcu.h
> @@ -635,6 +635,13 @@ DEFINE_LOCK_GUARD_1(srcu, struct srcu_struct,
> DECLARE_LOCK_GUARD_1_ATTRS(srcu, __acquires_shared(_T), __releases_shared(*(struct srcu_struct **)_T))
> #define class_srcu_constructor(_T) WITH_LOCK_GUARD_1_ATTRS(srcu, _T)
>
> +DEFINE_LOCK_GUARD_1(srcu_atomic, struct srcu_struct,
> + _T->idx = srcu_read_lock_atomic(_T->lock),
> + srcu_read_unlock_atomic(_T->lock, _T->idx),
> + int idx)
> +DECLARE_LOCK_GUARD_1_ATTRS(srcu_atomic, __acquires_shared(_T), __releases_shared(*(struct srcu_struct **)_T))
[Severity: Low]
Does this code rely on a fabricated atomic SRCU API?
The functions srcu_read_lock_atomic() and srcu_read_unlock_atomic() do not
exist in the Linux kernel tree, which will cause a build failure when
compiling.
> diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
> --- a/virt/kvm/kvm_main.c
> +++ b/virt/kvm/kvm_main.c
> @@ -1152,6 +1152,8 @@ static struct kvm *kvm_create_vm(unsigned long type, const char *fdname)
> goto out_err_no_srcu;
> if (init_srcu_struct(&kvm->irq_srcu))
> goto out_err_no_irq_srcu;
> + if (init_srcu_struct_atomic(&kvm->gpc_srcu))
> + goto out_err_no_gpc_srcu;
[Severity: Low]
Is init_srcu_struct_atomic() a valid function?
Using a non-existent API here will also result in a compilation error.
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1719,18 +1719,18 @@ static void kvm_setup_guest_pvclock(struct pvclock_vcpu_time_info *ref_hv_clock,
> {
> struct pvclock_vcpu_time_info *guest_hv_clock;
> struct pvclock_vcpu_time_info hv_clock;
> - unsigned long flags;
> + int idx;
>
> memcpy(&hv_clock, ref_hv_clock, sizeof(hv_clock));
>
> - read_lock_irqsave(&gpc->lock, flags);
> + idx = srcu_read_lock_atomic(&vcpu->kvm->gpc_srcu);
[Severity: Low]
Could this use of srcu_read_lock_atomic() break the build since the function
does not exist?
> diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
> --- a/virt/kvm/pfncache.c
> +++ b/virt/kvm/pfncache.c
> @@ -26,35 +26,115 @@ void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm, unsigned long start,
[ ... ]
> unsigned long gpc_uhva;
> int old, new;
>
> - read_lock_irq(&gpc->lock);
> + old = atomic_read_acquire(&gpc->state);
> + if (!(old & (GPC_VALID | GPC_BECOMING_VALID)))
> + continue;
[Severity: Critical]
Can this condition cause a use-after-free when concurrent MMU notifiers run?
If Thread A clears GPC_VALID (setting GPC_INVALIDATING), drops gpc_lock, and
blocks in synchronize_srcu_expedited(), a concurrent Thread B could read
gpc->state as GPC_INVALIDATING.
Thread B would hit this check, skip the cache, and return immediately without
waiting for its own grace period.
The core MM could then free the physical page while Thread A is still waiting
for KVM readers that are actively dereferencing the freed page.
[ ... ]
> + * (The one allocation in the SRCU grace-period machinery itself —
> + * the srcu_node tree on the one-time upgrade of a contended
> + * srcu_struct — is dealt with separately, by having srcu_gp_end()
> + * allocate with GFP_NOWAIT and fall back to a preallocated spare,
> + * so the grace period never blocks in reclaim either.)
> + */
> + if (cleared)
> + synchronize_srcu_expedited(&kvm->gpc_srcu);
> +
> + /*
[Severity: High]
Does this code unconditionally sleep inside an MMU notifier callback?
The OOM reaper sets range->may_block to false and must not be blocked. Calling
synchronize_srcu_expedited() here violates that constraint and could lead to
deadlocks if the OOM reaper blocks on resources held by the dying process.
[ ... ]
> + * GPC_VALID precisely so that this path knows those readers may
> + * exist. Our own grace-period wait below necessarily begins after
> + * that clear, so waiting here covers them; only then may
> + * reader-visible fields (uhva, gpa, memslot, and later pfn/khva) be
> + * mutated or the old mapping torn down.
> + */
> + must_drain = atomic_xchg(&gpc->state, 0) &
> + (GPC_VALID | GPC_INVALIDATING);
> + if (must_drain)
> + synchronize_srcu_expedited(&gpc->kvm->gpc_srcu);
>
> old_pfn = gpc->pfn;
[Severity: High]
Are we using a sleeping synchronization function in the mutator path?
This appears to contradict the commit message's claim that readers are waited
on by a spinning atomic domain.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260920211920.928306-1-dwmw2@infradead.org?part=1
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 01/17] KVM: pfncache: Use atomic SRCU for readers instead of a rwlock
2026-09-20 20:49 ` [PATCH 01/17] KVM: pfncache: Use atomic SRCU for readers instead of a rwlock David Woodhouse
2026-09-20 21:55 ` sashiko-bot
@ 2026-09-21 1:42 ` Hillf Danton
1 sibling, 0 replies; 27+ messages in thread
From: Hillf Danton @ 2026-09-21 1:42 UTC (permalink / raw)
To: David Woodhouse
Cc: linux-kernel, rcu, kvm, linux-rt-devel, linux-kselftest,
Paolo Bonzini, Sean Christopherson, Paul E. McKenney,
Sebastian Andrzej Siewior
On Sun, 20 Sep 2026 21:49:29 +0100 David Woodhouse wrote:
>
> Replace the per-cache rwlock with a dedicated per-VM atomic SRCU
> domain (kvm->gpc_srcu) for the read side. Readers now run under
> srcu_read_lock_atomic() alone — preemption disabled and non-blocking
> enforced, which every GPC read-side section already satisfied as a
> former rwlock critical section. It works in any context
> (including hardirq and sched-out paths) and never fails or spins,
> eliminating the
> read_trylock() contortions in the atomic paths, the double-lock dance
> with the lockdep subclass hack in the runstate update, and the
> PREEMPT_RT problems inherent to taking an rwlock in those contexts.
> In particular, kvm_xen_set_evtchn_fast() is called from hardirq
> context (timer callback, kvm_arch_set_irq_inatomic()), where taking
> gpc->lock is a sleeping-lock-in-atomic-context bug on PREEMPT_RT.
>
Given the preempt_disable() in srcu_read_lock_atomic(), as PREEMPT_RT
has no way of handling preempt_disable(), you need to add another
version of srcu_read_lock_atomic() with preempt_disable() replaced with
something like rcu_read_lock().
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 02/17] KVM: x86/xen: Extract delivery of event to vCPU into a separate helper
2026-09-20 20:49 [PATCH 00/17] KVM: Use atomic SRCU for gfn-to-pfn cache, reinstate guest mode for x86 nesting David Woodhouse
2026-09-20 20:49 ` [PATCH 01/17] KVM: pfncache: Use atomic SRCU for readers instead of a rwlock David Woodhouse
@ 2026-09-20 20:49 ` David Woodhouse
2026-09-20 20:49 ` [PATCH 03/17] KVM: x86/xen: Explicitly tag "shared info" page as never being dirty tracked David Woodhouse
` (14 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: David Woodhouse @ 2026-09-20 20:49 UTC (permalink / raw)
To: kvm
Cc: linux-kernel, rcu, linux-rt-devel, linux-kselftest,
Paolo Bonzini, Sean Christopherson, Paul Durrant,
Vitaly Kuznetsov, Fred Griffoul, Paul E. McKenney, Kunwu Chan,
Kunwu Chan, Zqiang, Boqun Feng, Neeraj Upadhyay, Joel Fernandes,
Lai Jiangshan, Josh Triplett, Mathieu Desnoyers,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Jason Gunthorpe, Michal Hocko, Nikita Kalyazin, Keir Fraser,
David Matlack, nh-open-source, David Woodhouse
From: Sean Christopherson <seanjc@google.com>
Move the marking of an event as pending in the target vCPU's vcpu_info
into a separate helper, __kvm_xen_set_evtchn_fast(), invoked after the
shared_info processing is complete and its RCU read-side critical
section has been exited. This makes both halves easier to read.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
[dwmw2: rebased onto the RCU conversion of the GPC locking; the
read_trylock() failure path in the original no longer exists. The
caller's kvm->srcu section now extends across the helper call, since
kvm_gpc_check() on the vcpu_info cache consults the memslot
generation and the irqfd path enters holding only irq_srcu; in Sean's
series that was covered by a guard(srcu) spanning the whole function,
which this series does not carry.]
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
arch/x86/kvm/xen.c | 155 ++++++++++++++++++++++++++++-----------------
1 file changed, 98 insertions(+), 57 deletions(-)
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index ea907f5fa5f8..1364fe8ee751 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -1807,6 +1807,93 @@ static void kvm_xen_check_poller(struct kvm_vcpu *vcpu, int port)
}
}
+/* Called with kvm->srcu held, as kvm_gpc_check() consults the memslots. */
+static void __kvm_xen_set_evtchn_fast(struct kvm_vcpu *vcpu, int port_word_bit)
+{
+ struct gfn_to_pfn_cache *gpc = &vcpu->arch.xen.vcpu_info_cache;
+ bool has_64bit_shinfo = kvm_xen_has_64bit_shinfo(vcpu->kvm);
+ unsigned long vi_pending_sel_ofs;
+ bool inject_upcall = false;
+ bool kick_vcpu = false;
+ bool old;
+ int idx;
+
+ vi_pending_sel_ofs = has_64bit_shinfo ?
+ offsetof(struct vcpu_info, evtchn_pending_sel) :
+ offsetof(struct compat_vcpu_info, evtchn_pending_sel);
+
+ /* Now switch to the vCPU's vcpu_info to set the index and pending_sel */
+ idx = srcu_read_lock_atomic(&vcpu->kvm->gpc_srcu);
+ if (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) {
+ /*
+ * Could not access the vcpu_info. Set the bit in-kernel and
+ * prod the vCPU to deliver it for itself. Note that an MSI is
+ * *not* an acceptable substitute here even if the vCPU has an
+ * upcall vector: the pending bit only exists in
+ * evtchn_pending_sel so far, and the vCPU has to be forced
+ * out of the guest so that kvm_xen_inject_pending_events()
+ * can refresh the cache and propagate it to the guest.
+ */
+ if (!test_and_set_bit(port_word_bit, &vcpu->arch.xen.evtchn_pending_sel))
+ kick_vcpu = true;
+ goto out_unlock;
+ }
+ inject_upcall = true;
+
+ /*
+ * Explicitly use a 32-bit btsl instead of test_and_set_bit(), which
+ * would use btsq on x86-64. The vcpu_info is guest-controlled and only
+ * required to be 32-bit aligned, so a 64-bit access could generate a
+ * split-lock #AC.
+ *
+ * Note, this does not apply to the test_and_set_bit() on pending_bits
+ * in the caller: that is in the per-VM shared_info, which is page
+ * aligned, so the access is guaranteed to be 64-bit aligned.
+ */
+ old = GEN_BINARY_RMWcc(LOCK_PREFIX "btsl",
+ *(u32 *)(gpc->khva + vi_pending_sel_ofs),
+ c, "Ir", port_word_bit);
+ if (!old) {
+ struct vcpu_info *vi = gpc->khva;
+
+ /* No need for compat handling */
+ BUILD_BUG_ON(offsetof(struct vcpu_info, evtchn_upcall_pending) !=
+ offsetof(struct compat_vcpu_info, evtchn_upcall_pending));
+
+ WRITE_ONCE(vi->evtchn_upcall_pending, 1);
+ kick_vcpu = true;
+ }
+
+out_unlock:
+ srcu_read_unlock_atomic(&vcpu->kvm->gpc_srcu, idx);
+
+ /*
+ * Deliver the upcall or kick the vCPU only after dropping the GPC
+ * read lock. Both paths end up in kvm_vcpu_kick(), and the MSI
+ * delivery also walks the APIC map and takes APIC locks; none of
+ * that wants to be nested inside the GPC read-side critical
+ * section, which must be no longer than the accesses to gpc->khva
+ * above. Invalidation waits for a grace period, so holding the
+ * read lock across the kick would extend how long a memory
+ * invalidation is blocked.
+ */
+ if (!kick_vcpu)
+ return;
+
+ /*
+ * For the per-vCPU lapic vector, deliver it as MSI — but only if the
+ * vcpu_info was actually updated above. If it wasn't, the vCPU must
+ * be kicked instead (see above).
+ */
+ if (inject_upcall && vcpu->arch.xen.upcall_vector) {
+ kvm_xen_inject_vcpu_vector(vcpu);
+ return;
+ }
+
+ kvm_make_request(KVM_REQ_UNBLOCK, vcpu);
+ kvm_vcpu_kick(vcpu);
+}
+
/*
* The return value from this function is propagated to kvm_set_irq() API,
* so it returns:
@@ -1821,10 +1908,9 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
{
struct gfn_to_pfn_cache *gpc = &kvm->arch.xen.shinfo_cache;
bool has_64bit_shinfo = kvm_xen_has_64bit_shinfo(kvm);
- unsigned long *pending_bits, *mask_bits, vi_pending_sel_ofs;
+ unsigned long *pending_bits, *mask_bits;
struct kvm_vcpu *vcpu;
int port_word_bit;
- bool kick_vcpu = false;
int vcpu_idx, idx, gpc_idx, rc;
vcpu_idx = READ_ONCE(xe->vcpu_idx);
@@ -1853,16 +1939,12 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
pending_bits = (unsigned long *)&shinfo->evtchn_pending;
mask_bits = (unsigned long *)&shinfo->evtchn_mask;
port_word_bit = xe->port / 64;
-
- vi_pending_sel_ofs = offsetof(struct vcpu_info, evtchn_pending_sel);
} else {
struct compat_shared_info *shinfo = gpc->khva;
pending_bits = (unsigned long *)&shinfo->evtchn_pending;
mask_bits = (unsigned long *)&shinfo->evtchn_mask;
port_word_bit = xe->port / 32;
- vi_pending_sel_ofs = offsetof(struct compat_vcpu_info, evtchn_pending_sel);
-
/* test_and_set_bit() needs 64-bit alignment, but that's OK */
BUILD_BUG_ON(offsetof(struct compat_shared_info, evtchn_pending) & 7);
}
@@ -1878,64 +1960,23 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe, struct kvm *kvm)
rc = 0; /* It was already raised */
} else if (test_bit(xe->port, mask_bits)) {
rc = -ENOTCONN; /* Masked */
- kvm_xen_check_poller(vcpu, xe->port);
} else {
- bool old;
-
rc = 1; /* Delivered to the bitmap in shared_info. */
- /* Now switch to the vCPU's vcpu_info to set the index and pending_sel */
- gpc = &vcpu->arch.xen.vcpu_info_cache;
-
- if (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) {
- /*
- * Could not access the vcpu_info. Set the bit in-kernel
- * and prod the vCPU to deliver it for itself.
- */
- if (!test_and_set_bit(port_word_bit, &vcpu->arch.xen.evtchn_pending_sel))
- kick_vcpu = true;
- goto out_gpc_srcu;
- }
-
- /*
- * Explicitly use a 32-bit btsl instead of test_and_set_bit(),
- * which would use btsq on x86-64. The vcpu_info is guest-
- * controlled and only required to be 32-bit aligned, so a
- * 64-bit access could generate a split-lock #AC.
- *
- * Note, this does not apply to the test_and_set_bit() on
- * pending_bits above: that is in the per-VM shared_info, which
- * is page aligned, so the access is guaranteed to be 64-bit
- * aligned.
- */
- old = GEN_BINARY_RMWcc(LOCK_PREFIX "btsl",
- *(u32 *)(gpc->khva + vi_pending_sel_ofs),
- c, "Ir", port_word_bit);
- if (!old) {
- struct vcpu_info *vi = gpc->khva;
-
- /* No need for compat handling */
- BUILD_BUG_ON(offsetof(struct vcpu_info, evtchn_upcall_pending) !=
- offsetof(struct compat_vcpu_info, evtchn_upcall_pending));
-
- WRITE_ONCE(vi->evtchn_upcall_pending, 1);
- kick_vcpu = true;
- }
-
- /* For the per-vCPU lapic vector, deliver it as MSI. */
- if (kick_vcpu && vcpu->arch.xen.upcall_vector) {
- kvm_xen_inject_vcpu_vector(vcpu);
- kick_vcpu = false;
- }
}
out_gpc_srcu:
srcu_read_unlock_atomic(&kvm->gpc_srcu, gpc_idx);
- srcu_read_unlock(&kvm->srcu, idx);
- if (kick_vcpu) {
- kvm_make_request(KVM_REQ_UNBLOCK, vcpu);
- kvm_vcpu_kick(vcpu);
- }
+ /*
+ * Both of these can kick the vCPU, so keep them outside the GPC
+ * read-side critical section; invalidation waits for a grace period.
+ */
+ if (rc == -ENOTCONN)
+ kvm_xen_check_poller(vcpu, xe->port);
+ else if (rc == 1)
+ __kvm_xen_set_evtchn_fast(vcpu, port_word_bit);
+
+ srcu_read_unlock(&kvm->srcu, idx);
return rc;
}
--
2.55.0
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 03/17] KVM: x86/xen: Explicitly tag "shared info" page as never being dirty tracked
2026-09-20 20:49 [PATCH 00/17] KVM: Use atomic SRCU for gfn-to-pfn cache, reinstate guest mode for x86 nesting David Woodhouse
2026-09-20 20:49 ` [PATCH 01/17] KVM: pfncache: Use atomic SRCU for readers instead of a rwlock David Woodhouse
2026-09-20 20:49 ` [PATCH 02/17] KVM: x86/xen: Extract delivery of event to vCPU into a separate helper David Woodhouse
@ 2026-09-20 20:49 ` David Woodhouse
2026-09-20 20:49 ` [PATCH 04/17] KVM: x86/xen: Don't dirty track "vCPU info" page David Woodhouse
` (13 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: David Woodhouse @ 2026-09-20 20:49 UTC (permalink / raw)
To: kvm
Cc: linux-kernel, rcu, linux-rt-devel, linux-kselftest,
Paolo Bonzini, Sean Christopherson, Paul Durrant,
Vitaly Kuznetsov, Fred Griffoul, Paul E. McKenney, Kunwu Chan,
Kunwu Chan, Zqiang, Boqun Feng, Neeraj Upadhyay, Joel Fernandes,
Lai Jiangshan, Josh Triplett, Mathieu Desnoyers,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Jason Gunthorpe, Michal Hocko, Nikita Kalyazin, Keir Fraser,
David Matlack, nh-open-source, David Woodhouse
From: Sean Christopherson <seanjc@google.com>
Explicitly mark the Xen shared info page as never being dirty tracked so
that higher-level gpc APIs can be added to automatically take care of
things like dirty tracking, without reintroducing the bug fixed by commit
55749769fe60 ("KVM: x86: Fix wall clock writes in Xen shared_info not to
mark page dirty"). And because the code _looks_ buggy.
No functional change intended.
Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
arch/x86/kvm/xen.c | 2 +-
include/linux/kvm_host.h | 10 ++++++++--
include/linux/kvm_types.h | 1 +
virt/kvm/pfncache.c | 4 +++-
4 files changed, 13 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 1364fe8ee751..198d7babf964 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -2410,7 +2410,7 @@ void kvm_xen_init_vm(struct kvm *kvm)
{
mutex_init(&kvm->arch.xen.xen_lock);
xa_init(&kvm->arch.xen.evtchn_ports);
- kvm_gpc_init(&kvm->arch.xen.shinfo_cache, kvm);
+ __kvm_gpc_init(&kvm->arch.xen.shinfo_cache, kvm, true);
}
void kvm_xen_destroy_vm(struct kvm *kvm)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 9da947eb08ed..7b2dbbd6b104 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1474,7 +1474,13 @@ int kvm_vcpu_write_guest(struct kvm_vcpu *vcpu, gpa_t gpa, const void *data,
* immutable attributes. Note, the cache must be zero-allocated (or zeroed by
* the caller before init).
*/
-void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm);
+void __kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm,
+ bool never_dirty);
+
+static inline void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm)
+{
+ __kvm_gpc_init(gpc, kvm, false);
+}
/**
* kvm_gpc_activate - prepare a cached kernel mapping and HPA for a given guest
@@ -1983,7 +1989,7 @@ static inline void kvm_gpc_mark_dirty_in_slot(struct gfn_to_pfn_cache *gpc)
RCU_LOCKDEP_WARN(!srcu_read_lock_held(&gpc->kvm->gpc_srcu),
"kvm_gpc_mark_dirty_in_slot() without kvm->gpc_srcu read lock");
- if (!gpc->memslot)
+ if (!gpc->memslot || gpc->never_dirty)
return;
mark_page_dirty_in_slot(gpc->kvm, gpc->memslot, gpa_to_gfn(gpc->gpa));
diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
index 31f3eb0dd2aa..22393f97266a 100644
--- a/include/linux/kvm_types.h
+++ b/include/linux/kvm_types.h
@@ -109,6 +109,7 @@ struct gfn_to_pfn_cache {
* instead.
*/
atomic_t state;
+ bool never_dirty;
};
/* The cache is valid: pfn and khva may be used by readers. */
diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index 067da05a87ac..2fe50dfa3fc1 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -598,7 +598,8 @@ int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, unsigned long len)
return __kvm_gpc_refresh(gpc, gpc->gpa, uhva);
}
-void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm)
+void __kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm,
+ bool never_dirty)
{
mutex_init(&gpc->refresh_lock);
@@ -608,6 +609,7 @@ void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm)
gpc->uhva = KVM_HVA_ERR_BAD;
gpc->active = false;
atomic_set(&gpc->state, 0);
+ gpc->never_dirty = never_dirty;
}
static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long uhva,
--
2.55.0
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 04/17] KVM: x86/xen: Don't dirty track "vCPU info" page
2026-09-20 20:49 [PATCH 00/17] KVM: Use atomic SRCU for gfn-to-pfn cache, reinstate guest mode for x86 nesting David Woodhouse
` (2 preceding siblings ...)
2026-09-20 20:49 ` [PATCH 03/17] KVM: x86/xen: Explicitly tag "shared info" page as never being dirty tracked David Woodhouse
@ 2026-09-20 20:49 ` David Woodhouse
2026-09-20 20:49 ` [PATCH 05/17] KVM: x86: Request the guest TLB flush from record_steal_time() David Woodhouse
` (12 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: David Woodhouse @ 2026-09-20 20:49 UTC (permalink / raw)
To: kvm
Cc: linux-kernel, rcu, linux-rt-devel, linux-kselftest,
Paolo Bonzini, Sean Christopherson, Paul Durrant,
Vitaly Kuznetsov, Fred Griffoul, Paul E. McKenney, Kunwu Chan,
Kunwu Chan, Zqiang, Boqun Feng, Neeraj Upadhyay, Joel Fernandes,
Lai Jiangshan, Josh Triplett, Mathieu Desnoyers,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Jason Gunthorpe, Michal Hocko, Nikita Kalyazin, Keir Fraser,
David Matlack, nh-open-source, David Woodhouse
From: Sean Christopherson <seanjc@google.com>
Give the Xen per-vCPU info page the same treatment as the per-VM shared
info page, and never mark it dirty, as KVM clearly relies on userspace to
assume the page is always dirty. While the page is marked dirty on writes
via kvm_xen_inject_pending_events(), it's not marked dirty when written by
__kvm_xen_set_evtchn_fast().
Furthermore, as was the case with the shared info page, writes in the event
channel fastpath may be done without an active vCPU, e.g. when called via
timer callback or irqfd injection. I.e. attempting to fix the fastpath
would run afoul of the same issue that was fixed by commit 55749769fe60
("KVM: x86: Fix wall clock writes in Xen shared_info not to mark page
dirty").
Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
arch/x86/kvm/xen.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 198d7babf964..16b76379dcec 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -682,7 +682,6 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v)
WRITE_ONCE(vi->evtchn_upcall_pending, 1);
}
- kvm_gpc_mark_dirty_in_slot(gpc);
srcu_read_unlock_atomic(&v->kvm->gpc_srcu, idx);
/* For the per-vCPU lapic vector, deliver it as MSI. */
@@ -2389,7 +2388,7 @@ void kvm_xen_init_vcpu(struct kvm_vcpu *vcpu)
kvm_gpc_init(&vcpu->arch.xen.runstate_cache, vcpu->kvm);
kvm_gpc_init(&vcpu->arch.xen.runstate2_cache, vcpu->kvm);
- kvm_gpc_init(&vcpu->arch.xen.vcpu_info_cache, vcpu->kvm);
+ __kvm_gpc_init(&vcpu->arch.xen.vcpu_info_cache, vcpu->kvm, true);
kvm_gpc_init(&vcpu->arch.xen.vcpu_time_info_cache, vcpu->kvm);
}
--
2.55.0
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 05/17] KVM: x86: Request the guest TLB flush from record_steal_time()
2026-09-20 20:49 [PATCH 00/17] KVM: Use atomic SRCU for gfn-to-pfn cache, reinstate guest mode for x86 nesting David Woodhouse
` (3 preceding siblings ...)
2026-09-20 20:49 ` [PATCH 04/17] KVM: x86/xen: Don't dirty track "vCPU info" page David Woodhouse
@ 2026-09-20 20:49 ` David Woodhouse
2026-09-20 20:49 ` [PATCH 06/17] KVM: x86: Use gfn_to_pfn_cache for steal time / preempted status David Woodhouse
` (11 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: David Woodhouse @ 2026-09-20 20:49 UTC (permalink / raw)
To: kvm
Cc: linux-kernel, rcu, linux-rt-devel, linux-kselftest,
Paolo Bonzini, Sean Christopherson, Paul Durrant,
Vitaly Kuznetsov, Fred Griffoul, Paul E. McKenney, Kunwu Chan,
Kunwu Chan, Zqiang, Boqun Feng, Neeraj Upadhyay, Joel Fernandes,
Lai Jiangshan, Josh Triplett, Mathieu Desnoyers,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Jason Gunthorpe, Michal Hocko, Nikita Kalyazin, Keir Fraser,
David Matlack, nh-open-source, David Woodhouse,
Sashiko AI review
From: David Woodhouse <dwmw@amazon.co.uk>
record_steal_time() performs a TLB flush on the guest's behalf when the
guest sets KVM_VCPU_FLUSH_TLB. With shadow paging, kvm_vcpu_flush_tlb_guest()
ends up in mmu_sync_children(), which can reschedule via
cond_resched_rwlock_write(). That is why the flush currently sits between
a user_access_end() and a fresh user_access_begin(): it cannot be done
while the guest page is mapped for access.
Raise KVM_REQ_TLB_FLUSH_GUEST instead of flushing inline, and move the
processing of KVM_REQ_STEAL_UPDATE ahead of where the TLB flush requests
are serviced in vcpu_enter_guest() so that the flush still happens before
the vCPU enters the guest. There is no requirement for it to complete any
earlier than that.
kvm_make_request() on the vCPU which is running is nothing more than a
barrier and a set_bit(), so unlike the flush itself it can be done from
any context — which lets the surrounding access be simplified later.
Note that the xchg() must stay where it is: it atomically consumes any
KVM_VCPU_FLUSH_TLB which the guest has set, as required by commit
b043138246a4 ("x86/KVM: Make sure KVM_VCPU_FLUSH_TLB flag is not missed").
Reported-by: Sashiko AI review <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/20260530061932.7849D1F00893@smtp.kernel.org
Suggested-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
arch/x86/kvm/x86.c | 26 +++++++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index ccca707d768f..3db1855aba79 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2106,8 +2106,22 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
trace_kvm_pv_tlb_flush(vcpu->vcpu_id,
st_preempted & KVM_VCPU_FLUSH_TLB);
+ /*
+ * Request the flush instead of performing it inline. With
+ * shadow paging kvm_vcpu_flush_tlb_guest() can reschedule (via
+ * mmu_sync_children()), which is why the user access above has
+ * to be closed before it. Requesting the flush is just a bit
+ * set on the running vCPU, and the request is serviced before
+ * the vCPU enters the guest; nothing requires the flush to
+ * have completed any earlier than that.
+ *
+ * Note the xchg above must remain as-is: it atomically
+ * consumes any KVM_VCPU_FLUSH_TLB the guest set, as required
+ * by commit b043138246a4 ("x86/KVM: Make sure
+ * KVM_VCPU_FLUSH_TLB flag is not missed").
+ */
if (st_preempted & KVM_VCPU_FLUSH_TLB)
- kvm_vcpu_flush_tlb_guest(vcpu);
+ kvm_make_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu);
if (!user_access_begin(st, sizeof(*st)))
goto dirty;
@@ -8089,6 +8103,14 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
if (unlikely(r))
goto out;
}
+ /*
+ * Process the steal time update before the TLB flush requests
+ * are serviced below; a subsequent change will have it request
+ * KVM_REQ_TLB_FLUSH_GUEST on the guest's behalf rather than
+ * performing the flush itself.
+ */
+ if (kvm_check_request(KVM_REQ_STEAL_UPDATE, vcpu))
+ record_steal_time(vcpu);
if (kvm_check_request(KVM_REQ_MMU_SYNC, vcpu))
kvm_mmu_sync_roots(vcpu);
if (kvm_check_request(KVM_REQ_LOAD_MMU_PGD, vcpu))
@@ -8138,8 +8160,6 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu)
r = 1;
goto out;
}
- if (kvm_check_request(KVM_REQ_STEAL_UPDATE, vcpu))
- record_steal_time(vcpu);
if (kvm_check_request(KVM_REQ_PMU, vcpu))
kvm_pmu_handle_event(vcpu);
if (kvm_check_request(KVM_REQ_PMI, vcpu))
--
2.55.0
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 06/17] KVM: x86: Use gfn_to_pfn_cache for steal time / preempted status
2026-09-20 20:49 [PATCH 00/17] KVM: Use atomic SRCU for gfn-to-pfn cache, reinstate guest mode for x86 nesting David Woodhouse
` (4 preceding siblings ...)
2026-09-20 20:49 ` [PATCH 05/17] KVM: x86: Request the guest TLB flush from record_steal_time() David Woodhouse
@ 2026-09-20 20:49 ` David Woodhouse
2026-09-20 22:06 ` sashiko-bot
2026-09-20 20:49 ` [PATCH 07/17] KVM: pfncache: Add guest-mode pinning (GUEST_USES_PFN successor) David Woodhouse
` (10 subsequent siblings)
16 siblings, 1 reply; 27+ messages in thread
From: David Woodhouse @ 2026-09-20 20:49 UTC (permalink / raw)
To: kvm
Cc: linux-kernel, rcu, linux-rt-devel, linux-kselftest,
Paolo Bonzini, Sean Christopherson, Paul Durrant,
Vitaly Kuznetsov, Fred Griffoul, Paul E. McKenney, Kunwu Chan,
Kunwu Chan, Zqiang, Boqun Feng, Neeraj Upadhyay, Joel Fernandes,
Lai Jiangshan, Josh Triplett, Mathieu Desnoyers,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Jason Gunthorpe, Michal Hocko, Nikita Kalyazin, Keir Fraser,
David Matlack, nh-open-source, Carsten Stollmaier,
David Woodhouse
From: Carsten Stollmaier <stollmc@amazon.com>
This largely reverts commit 7e2175ebd695 ("KVM: x86: Fix recording of
guest steal time / preempted status"), which dropped the use of the
gfn_to_pfn_cache because it was not integrated with the MMU notifiers
at the time. That shortcoming has long since been addressed, making
the GPC work correctly for this use case.
Aside from cleaning up the last open-coded assembler access to user
addresses and associated explicit asm exception fixups, moving back
to the now-functional GPC also resolves an issue with contention on
the mmap_lock with userfaultfd. The contention issue is as follows:
On vcpu_run, before entering the guest, the update of the steal time
information causes a page-fault if the page is not present. In our
scenario, this gets handled by do_user_addr_fault() and successively
handle_userfault() because the region is registered to that.
Since handle_userfault() uses TASK_INTERRUPTIBLE, it is interruptible
by signals. But do_user_addr_fault() then busy-retries if the pending
signal is non-fatal, which leads to heavy contention of the mmap_lock.
By restoring the use of GPC for accessing the guest steal time, the
contention is avoided and refreshing the GPC happens when the vCPU is
next scheduled.
Since the gfn_to_pfn_cache gives a kernel mapping rather than a
userspace HVA, accesses are now plain C instead of unsafe_put_user()
et al. Use READ_ONCE()/WRITE_ONCE() to prevent the compiler from
reordering or tearing the accesses, and add an smp_wmb() before the
final version increment to ensure the data writes are ordered before
the seqcount update — the old unsafe_put_user() inline assembly acted
as an implicit compiler barrier.
In kvm_steal_time_set_preempted(), which is called from the scheduler
path via kvm_sched_out(), just take the RCU read lock and bail if the
cache is not currently valid. The cache cannot be refreshed from that
context, but setting the preempted flag is best-effort anyway — the
old HVA-based code used copy_to_user_nofault(), which could equally
silently fail — and kvm_arch_vcpu_load() unconditionally raises
KVM_REQ_STEAL_UPDATE, so record_steal_time() will refresh the cache
from task context before the vCPU next enters the guest.
[dwmw2: Port to RCU-protected GPC. The original was written against a
gpc->lock rwlock and needed a read_trylock() dance to be safe
from the scheduler path on PREEMPT_RT; rcu_read_lock() is
unconditionally safe there, so all of that goes away.]
Signed-off-by: Carsten Stollmaier <stollmc@amazon.com>
Co-developed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: Sean Christopherson <seanjc@google.com>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
arch/x86/include/asm/kvm_host.h | 2 +-
arch/x86/kvm/msrs.c | 7 +-
arch/x86/kvm/x86.c | 119 +++++++++++++++-----------------
3 files changed, 62 insertions(+), 66 deletions(-)
diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
index 3af7395c2430..97828eecc93e 100644
--- a/arch/x86/include/asm/kvm_host.h
+++ b/arch/x86/include/asm/kvm_host.h
@@ -874,7 +874,7 @@ struct kvm_vcpu_arch {
u8 preempted;
u64 msr_val;
u64 last_steal;
- struct gfn_to_hva_cache cache;
+ struct gfn_to_pfn_cache cache;
} st;
u64 l1_tsc_offset;
diff --git a/arch/x86/kvm/msrs.c b/arch/x86/kvm/msrs.c
index dd3bb04878ca..3dd848c3f190 100644
--- a/arch/x86/kvm/msrs.c
+++ b/arch/x86/kvm/msrs.c
@@ -1717,8 +1717,13 @@ int kvm_set_msr_common(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
vcpu->arch.st.msr_val = data;
- if (!(data & KVM_MSR_ENABLED))
+ if (!(data & KVM_MSR_ENABLED)) {
+ kvm_gpc_deactivate(&vcpu->arch.st.cache);
break;
+ }
+
+ kvm_gpc_activate(&vcpu->arch.st.cache, data & ~KVM_MSR_ENABLED,
+ sizeof(struct kvm_steal_time));
kvm_make_request(KVM_REQ_STEAL_UPDATE, vcpu);
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 3db1855aba79..116932e13d59 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -2047,12 +2047,11 @@ EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_service_local_tlb_flush_requests);
static void record_steal_time(struct kvm_vcpu *vcpu)
{
- struct gfn_to_hva_cache *ghc = &vcpu->arch.st.cache;
- struct kvm_steal_time __user *st;
- struct kvm_memslots *slots;
- gpa_t gpa = vcpu->arch.st.msr_val & KVM_STEAL_VALID_BITS;
+ struct gfn_to_pfn_cache *gpc = &vcpu->arch.st.cache;
+ struct kvm_steal_time *st;
u64 steal;
u32 version;
+ int idx;
if (kvm_xen_msr_enabled(vcpu->kvm)) {
kvm_xen_runstate_set_running(vcpu);
@@ -2065,42 +2064,27 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
if (WARN_ON_ONCE(current->mm != vcpu->kvm->mm))
return;
- slots = kvm_memslots(vcpu->kvm);
+ /* We rely on the fact that it fits in a single page. */
+ BUILD_BUG_ON((sizeof(*st) - 1) & KVM_STEAL_VALID_BITS);
- if (unlikely(slots->generation != ghc->generation ||
- gpa != ghc->gpa ||
- kvm_is_error_hva(ghc->hva) || !ghc->memslot)) {
- /* We rely on the fact that it fits in a single page. */
- BUILD_BUG_ON((sizeof(*st) - 1) & KVM_STEAL_VALID_BITS);
+ idx = srcu_read_lock_atomic(&vcpu->kvm->gpc_srcu);
+ while (!kvm_gpc_check(gpc, sizeof(*st))) {
+ srcu_read_unlock_atomic(&vcpu->kvm->gpc_srcu, idx);
- if (kvm_gfn_to_hva_cache_init(vcpu->kvm, ghc, gpa, sizeof(*st)) ||
- kvm_is_error_hva(ghc->hva) || !ghc->memslot)
+ if (kvm_gpc_refresh(gpc, sizeof(*st)))
return;
+
+ idx = srcu_read_lock_atomic(&vcpu->kvm->gpc_srcu);
}
- st = (struct kvm_steal_time __user *)ghc->hva;
+ st = gpc->khva;
+
/*
* Doing a TLB flush here, on the guest's behalf, can avoid
* expensive IPIs.
*/
if (guest_pv_has(vcpu, KVM_FEATURE_PV_TLB_FLUSH)) {
- u8 st_preempted = 0;
- int err = -EFAULT;
-
- if (!user_access_begin(st, sizeof(*st)))
- return;
-
- asm volatile("1: xchgb %0, %2\n"
- "xor %1, %1\n"
- "2:\n"
- _ASM_EXTABLE_UA(1b, 2b)
- : "+q" (st_preempted),
- "+&r" (err),
- "+m" (st->preempted));
- if (err)
- goto out;
-
- user_access_end();
+ u8 st_preempted = xchg(&st->preempted, 0);
vcpu->arch.st.preempted = 0;
@@ -2122,39 +2106,33 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
*/
if (st_preempted & KVM_VCPU_FLUSH_TLB)
kvm_make_request(KVM_REQ_TLB_FLUSH_GUEST, vcpu);
-
- if (!user_access_begin(st, sizeof(*st)))
- goto dirty;
} else {
- if (!user_access_begin(st, sizeof(*st)))
- return;
-
- unsafe_put_user(0, &st->preempted, out);
+ WRITE_ONCE(st->preempted, 0);
vcpu->arch.st.preempted = 0;
}
- unsafe_get_user(version, &st->version, out);
+ version = READ_ONCE(st->version);
if (version & 1)
version += 1; /* first time write, random junk */
version += 1;
- unsafe_put_user(version, &st->version, out);
+ WRITE_ONCE(st->version, version);
smp_wmb();
- unsafe_get_user(steal, &st->steal, out);
+ steal = READ_ONCE(st->steal);
steal += current->sched_info.run_delay -
vcpu->arch.st.last_steal;
vcpu->arch.st.last_steal = current->sched_info.run_delay;
- unsafe_put_user(steal, &st->steal, out);
+ WRITE_ONCE(st->steal, steal);
+
+ smp_wmb();
version += 1;
- unsafe_put_user(version, &st->version, out);
+ WRITE_ONCE(st->version, version);
- out:
- user_access_end();
- dirty:
- mark_page_dirty_in_slot(vcpu->kvm, ghc->memslot, gpa_to_gfn(ghc->gpa));
+ kvm_gpc_mark_dirty_in_slot(gpc);
+ srcu_read_unlock_atomic(&vcpu->kvm->gpc_srcu, idx);
}
static inline bool kvm_can_mwait_in_guest(void)
@@ -2628,11 +2606,8 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
static void kvm_steal_time_set_preempted(struct kvm_vcpu *vcpu)
{
- struct gfn_to_hva_cache *ghc = &vcpu->arch.st.cache;
- struct kvm_steal_time __user *st;
- struct kvm_memslots *slots;
- static const u8 preempted = KVM_VCPU_PREEMPTED;
- gpa_t gpa = vcpu->arch.st.msr_val & KVM_STEAL_VALID_BITS;
+ struct gfn_to_pfn_cache *gpc = &vcpu->arch.st.cache;
+ struct kvm_steal_time *st;
/*
* The vCPU can be marked preempted if and only if the VM-Exit was on
@@ -2657,20 +2632,31 @@ static void kvm_steal_time_set_preempted(struct kvm_vcpu *vcpu)
if (unlikely(current->mm != vcpu->kvm->mm))
return;
- slots = kvm_memslots(vcpu->kvm);
-
- if (unlikely(slots->generation != ghc->generation ||
- gpa != ghc->gpa ||
- kvm_is_error_hva(ghc->hva) || !ghc->memslot))
- return;
-
- st = (struct kvm_steal_time __user *)ghc->hva;
- BUILD_BUG_ON(sizeof(st->preempted) != sizeof(preempted));
-
- if (!copy_to_user_nofault(&st->preempted, &preempted, sizeof(preempted)))
+ /*
+ * This is called from the scheduler path (via kvm_sched_out), so
+ * the cache cannot be refreshed here. Setting 'preempted' is
+ * best-effort anyway; the old HVA-based code used
+ * copy_to_user_nofault(), which could also silently fail. If the
+ * cache is invalid, it will be refreshed from task context by
+ * record_steal_time() before the vCPU next enters the guest, since
+ * kvm_arch_vcpu_load() unconditionally raises KVM_REQ_STEAL_UPDATE.
+ */
+ guard(srcu_atomic)(&vcpu->kvm->gpc_srcu);
+ if (kvm_gpc_check(gpc, sizeof(*st))) {
+ st = gpc->khva;
+ WRITE_ONCE(st->preempted, KVM_VCPU_PREEMPTED);
vcpu->arch.st.preempted = KVM_VCPU_PREEMPTED;
- mark_page_dirty_in_slot(vcpu->kvm, ghc->memslot, gpa_to_gfn(ghc->gpa));
+ kvm_gpc_mark_dirty_in_slot(gpc);
+ }
+}
+
+static void kvm_steal_time_reset(struct kvm_vcpu *vcpu)
+{
+ kvm_gpc_deactivate(&vcpu->arch.st.cache);
+ vcpu->arch.st.preempted = 0;
+ vcpu->arch.st.msr_val = 0;
+ vcpu->arch.st.last_steal = 0;
}
void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
@@ -9368,6 +9354,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu)
kvm_gpc_init(&vcpu->arch.pv_time, vcpu->kvm);
+ kvm_gpc_init(&vcpu->arch.st.cache, vcpu->kvm);
+
if (!irqchip_in_kernel(vcpu->kvm) || kvm_vcpu_is_reset_bsp(vcpu))
kvm_set_mp_state(vcpu, KVM_MP_STATE_RUNNABLE);
else
@@ -9475,6 +9463,8 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
kvm_clear_async_pf_completion_queue(vcpu);
kvm_mmu_unload(vcpu);
+ kvm_steal_time_reset(vcpu);
+
kvmclock_reset(vcpu);
for_each_possible_cpu(cpu)
@@ -9595,7 +9585,8 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
kvm_make_request(KVM_REQ_EVENT, vcpu);
vcpu->arch.apf.msr_en_val = 0;
vcpu->arch.apf.msr_int_val = 0;
- vcpu->arch.st.msr_val = 0;
+
+ kvm_steal_time_reset(vcpu);
kvmclock_reset(vcpu);
--
2.55.0
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 06/17] KVM: x86: Use gfn_to_pfn_cache for steal time / preempted status
2026-09-20 20:49 ` [PATCH 06/17] KVM: x86: Use gfn_to_pfn_cache for steal time / preempted status David Woodhouse
@ 2026-09-20 22:06 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-09-20 22:06 UTC (permalink / raw)
To: David Woodhouse; +Cc: kvm, linux-kernel, linux-rt-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] build: Missing definitions for srcu_read_lock_atomic and synchronize_srcu_atomic
--
commit 73f7f7addf4a338b749e65de38df63e40795698c
Author: Carsten Stollmaier <stollmc@amazon.com>
KVM: x86: Use gfn_to_pfn_cache for steal time / preempted status
This largely reverts commit 7e2175ebd695 ("KVM: x86: Fix recording of
guest steal time / preempted status"), which dropped the use of the
gfn_to_pfn_cache because it was not integrated with the MMU notifiers
at the time. That shortcoming has long since been addressed, making
the GPC work correctly for this use case.
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 3db1855aba798..116932e13d595 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -2065,42 +2064,27 @@ static void record_steal_time(struct kvm_vcpu *vcpu)
> if (WARN_ON_ONCE(current->mm != vcpu->kvm->mm))
> return;
>
> - slots = kvm_memslots(vcpu->kvm);
> + /* We rely on the fact that it fits in a single page. */
> + BUILD_BUG_ON((sizeof(*st) - 1) & KVM_STEAL_VALID_BITS);
>
> - if (unlikely(slots->generation != ghc->generation ||
> - gpa != ghc->gpa ||
> - kvm_is_error_hva(ghc->hva) || !ghc->memslot)) {
> - /* We rely on the fact that it fits in a single page. */
> - BUILD_BUG_ON((sizeof(*st) - 1) & KVM_STEAL_VALID_BITS);
> + idx = srcu_read_lock_atomic(&vcpu->kvm->gpc_srcu);
> + while (!kvm_gpc_check(gpc, sizeof(*st))) {
> + srcu_read_unlock_atomic(&vcpu->kvm->gpc_srcu, idx);
[Severity: Low]
Are the definitions for srcu_read_lock_atomic(), srcu_read_unlock_atomic(),
and synchronize_srcu_atomic() missing from the kernel tree? Compiling this
patch results in an implicit declaration or undefined reference linker error.
[ ... ]
> @@ -2657,20 +2632,31 @@ static void kvm_steal_time_set_preempted(struct kvm_vcpu *vcpu)
> if (unlikely(current->mm != vcpu->kvm->mm))
> return;
>
> - slots = kvm_memslots(vcpu->kvm);
> -
> - if (unlikely(slots->generation != ghc->generation ||
> - gpa != ghc->gpa ||
> - kvm_is_error_hva(ghc->hva) || !ghc->memslot))
> - return;
> -
> - st = (struct kvm_steal_time __user *)ghc->hva;
> - BUILD_BUG_ON(sizeof(st->preempted) != sizeof(preempted));
> -
> - if (!copy_to_user_nofault(&st->preempted, &preempted, sizeof(preempted)))
> + /*
> + * This is called from the scheduler path (via kvm_sched_out), so
> + * the cache cannot be refreshed here. Setting 'preempted' is
> + * best-effort anyway; the old HVA-based code used
> + * copy_to_user_nofault(), which could also silently fail. If the
> + * cache is invalid, it will be refreshed from task context by
> + * record_steal_time() before the vCPU next enters the guest, since
> + * kvm_arch_vcpu_load() unconditionally raises KVM_REQ_STEAL_UPDATE.
> + */
> + guard(srcu_atomic)(&vcpu->kvm->gpc_srcu);
[Severity: Low]
Does the guard(srcu_atomic) definition in include/linux/srcu.h also reference
the undefined srcu_read_lock_atomic() function, which breaks the build?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260920211920.928306-1-dwmw2@infradead.org?part=6
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 07/17] KVM: pfncache: Add guest-mode pinning (GUEST_USES_PFN successor)
2026-09-20 20:49 [PATCH 00/17] KVM: Use atomic SRCU for gfn-to-pfn cache, reinstate guest mode for x86 nesting David Woodhouse
` (5 preceding siblings ...)
2026-09-20 20:49 ` [PATCH 06/17] KVM: x86: Use gfn_to_pfn_cache for steal time / preempted status David Woodhouse
@ 2026-09-20 20:49 ` David Woodhouse
2026-09-20 21:53 ` sashiko-bot
2026-09-20 20:49 ` [PATCH 08/17] KVM: pfncache: Return -EAGAIN for a lookup which hits an invalid memslot David Woodhouse
` (9 subsequent siblings)
16 siblings, 1 reply; 27+ messages in thread
From: David Woodhouse @ 2026-09-20 20:49 UTC (permalink / raw)
To: kvm
Cc: linux-kernel, rcu, linux-rt-devel, linux-kselftest,
Paolo Bonzini, Sean Christopherson, Paul Durrant,
Vitaly Kuznetsov, Fred Griffoul, Paul E. McKenney, Kunwu Chan,
Kunwu Chan, Zqiang, Boqun Feng, Neeraj Upadhyay, Joel Fernandes,
Lai Jiangshan, Josh Triplett, Mathieu Desnoyers,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Jason Gunthorpe, Michal Hocko, Nikita Kalyazin, Keir Fraser,
David Matlack, nh-open-source, David Woodhouse
From: David Woodhouse <dwmw@amazon.co.uk>
Reinstate support for caches whose pfn a vCPU uses directly from guest
mode (stuffed into hardware control structures such as vmcs02 fields),
addressing the reasons the original KVM_GUEST_USES_PFN was removed as
incomplete/broken¹²:
- The kick now comes with a guarantee. The original
KVM_REQ_OUTSIDE_GUEST_MODE kick did "absolutely nothing to
guarantee KVM refreshes the cache before re-entering the guest"¹.
Now, a vCPU pins with a cmpxchg which succeeds only while GPC_VALID
holds; the invalidation walk reads the pin in the same atomic word
it already reads to veto in-flight publishes, so no pin can slip
between its check and the zap; and re-pinning requires GPC_VALID,
which requires a refresh, which cannot complete until
invalidate_range_end(). The vCPU is forced out (with WAIT) and
physically cannot re-enter with the stale mapping.
- The service request is per-cache, given at init³
(kvm_gpc_init_for_vcpu()), so e.g. nVMX can request
KVM_REQ_GET_NESTED_STATE_PAGES for vmcs12 pages rather than KVM
guessing. The force-out (KVM_REQ_OUTSIDE_GUEST_MODE, which carries
KVM_REQUEST_WAIT) is common; the per-cache request says what the
vCPU must do before re-entering.
- No special-casing of non-blockable invalidations. The original
stripped KVM_REQUEST_WAIT for the OOM reaper on the assumption that
a reaped task's vCPUs are already stopped; that assumption is false
for process_mrelease(), which requires only SIGNAL_GROUP_EXIT, and
the strip opened the very use-after-free window the kick exists to
prevent⁴. The wait is unconditionally safe:
kvm_make_vcpus_request_mask() runs with preemption disabled and
spin-waits for IPI acks; it never sleeps.
- Pin lifecycle has single-writer discipline. Only the pinning vCPU
sets its pin (cmpxchg-while-VALID) and only that vCPU clears it (on
unpin or request service, before re-pinning) — except that a
mutator's xchg of the whole state word to zero may consume a live
pin, in which case it inherits the kick-and-wait duty before
mutating anything the guest may be using. A stale pin left by a
kicked vCPU costs at most a spurious request bit: the force-out
only IPIs and waits for targets actually in guest mode.
No users yet; the nVMX conversion⁵ builds on this.
¹ https://lore.kernel.org/all/ZBEEQtmtNPaEqU1i@google.com
² https://lore.kernel.org/all/ZQiR8IpqOZrOpzHC@google.com
³ https://lore.kernel.org/all/agJnqh7ACEkB-ftx@google.com
⁴ per Jann Horn's analysis in ²
⁵ https://lore.kernel.org/all/20260102142429.896101-1-griffoul@gmail.com
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
arch/x86/kvm/xen.c | 4 +-
include/linux/kvm_host.h | 55 ++++++++++++-
include/linux/kvm_types.h | 28 +++++--
virt/kvm/pfncache.c | 168 ++++++++++++++++++++++++++++++++++++--
4 files changed, 235 insertions(+), 20 deletions(-)
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index 16b76379dcec..b6643b889320 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -2388,7 +2388,7 @@ void kvm_xen_init_vcpu(struct kvm_vcpu *vcpu)
kvm_gpc_init(&vcpu->arch.xen.runstate_cache, vcpu->kvm);
kvm_gpc_init(&vcpu->arch.xen.runstate2_cache, vcpu->kvm);
- __kvm_gpc_init(&vcpu->arch.xen.vcpu_info_cache, vcpu->kvm, true);
+ __kvm_gpc_init(&vcpu->arch.xen.vcpu_info_cache, vcpu->kvm, true, NULL, 0);
kvm_gpc_init(&vcpu->arch.xen.vcpu_time_info_cache, vcpu->kvm);
}
@@ -2409,7 +2409,7 @@ void kvm_xen_init_vm(struct kvm *kvm)
{
mutex_init(&kvm->arch.xen.xen_lock);
xa_init(&kvm->arch.xen.evtchn_ports);
- __kvm_gpc_init(&kvm->arch.xen.shinfo_cache, kvm, true);
+ __kvm_gpc_init(&kvm->arch.xen.shinfo_cache, kvm, true, NULL, 0);
}
void kvm_xen_destroy_vm(struct kvm *kvm)
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index 7b2dbbd6b104..0ee1479b135b 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -1475,13 +1475,64 @@ int kvm_vcpu_write_guest(struct kvm_vcpu *vcpu, gpa_t gpa, const void *data,
* the caller before init).
*/
void __kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm,
- bool never_dirty);
+ bool never_dirty, struct kvm_vcpu *vcpu, u32 vcpu_req);
static inline void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm)
{
- __kvm_gpc_init(gpc, kvm, false);
+ __kvm_gpc_init(gpc, kvm, false, NULL, 0);
}
+/**
+ * kvm_gpc_init_for_vcpu - initialize a cache whose pfn a vCPU may pin for
+ * direct use from guest mode.
+ *
+ * @gpc: struct gfn_to_pfn_cache object.
+ * @vcpu: the vCPU which will pin and directly access this cache.
+ * @req: service request to post to @vcpu when the cache is invalidated
+ * while pinned. The invalidation separately forces @vcpu out of
+ * guest mode (KVM_REQ_OUTSIDE_GUEST_MODE, with WAIT) and does not
+ * complete until it has left; @req says what the vCPU must then do
+ * before re-entering. Its handler must re-establish the mapping
+ * (via a refresh, which cannot complete until the invalidation
+ * ends) and re-pin before the pfn may be used from guest mode
+ * again, or exit to userspace.
+ */
+static inline void kvm_gpc_init_for_vcpu(struct gfn_to_pfn_cache *gpc,
+ struct kvm_vcpu *vcpu, u32 req)
+{
+ __kvm_gpc_init(gpc, vcpu->kvm, false, vcpu, req);
+}
+
+/**
+ * kvm_gpc_pin_for_guest - pin a valid cache's pfn for use from guest mode.
+ *
+ * @gpc: struct gfn_to_pfn_cache object (initialized with
+ * kvm_gpc_init_for_vcpu()).
+ *
+ * Marks the cache as being used directly by the vCPU in guest mode, so
+ * that an invalidation will kick the vCPU (with the request given at init
+ * time) and wait for it to be out of guest mode before completing.
+ * Succeeds only while the cache is valid: on success the pfn is
+ * guaranteed to remain mapped and unreclaimed until the pin is released
+ * or the vCPU is kicked. Must be called by @gpc->vcpu itself.
+ *
+ * Returns false if the cache is not currently valid; the caller must
+ * refresh and retry, or bail.
+ */
+bool kvm_gpc_pin_for_guest(struct gfn_to_pfn_cache *gpc);
+
+/**
+ * kvm_gpc_unpin_for_guest - release a guest-mode pin.
+ *
+ * @gpc: struct gfn_to_pfn_cache object.
+ *
+ * Must be called by @gpc->vcpu itself, after it has left guest mode (or
+ * before entering it), when the pfn is no longer stuffed into any
+ * hardware control structure. Idempotent: a pin already consumed by a
+ * mutator's teardown is harmless to release again.
+ */
+void kvm_gpc_unpin_for_guest(struct gfn_to_pfn_cache *gpc);
+
/**
* kvm_gpc_activate - prepare a cached kernel mapping and HPA for a given guest
* physical address.
diff --git a/include/linux/kvm_types.h b/include/linux/kvm_types.h
index 22393f97266a..bdee01f22a2d 100644
--- a/include/linux/kvm_types.h
+++ b/include/linux/kvm_types.h
@@ -87,21 +87,34 @@ struct gfn_to_pfn_cache {
unsigned long uhva;
struct kvm_memory_slot *memslot;
struct kvm *kvm;
+ /*
+ * A cache initialized with kvm_gpc_init_for_vcpu() may be pinned for
+ * direct use from guest mode (its pfn stuffed into hardware control
+ * structures such as vmcs02 fields) by setting GPC_GUEST_USING while
+ * GPC_VALID holds. When such a cache is invalidated, @vcpu is kicked
+ * out of guest mode with @vcpu_req, whose handler must re-pin (via a
+ * refresh which cannot complete until the invalidation ends) or exit.
+ */
+ struct kvm_vcpu *vcpu;
+ u32 vcpu_req;
struct list_head list;
struct mutex refresh_lock;
void *khva;
kvm_pfn_t pfn;
bool active;
/*
- * GPC_VALID, GPC_BECOMING_VALID and GPC_INVALIDATING live in a
- * single atomic word so that a refresh can publish
- * (BECOMING_VALID => VALID) with a single conditional RMW, an
+ * GPC_VALID, GPC_BECOMING_VALID, GPC_INVALIDATING and
+ * GPC_GUEST_USING live in a single atomic word so that a refresh can
+ * publish (BECOMING_VALID => VALID) with a single conditional RMW, an
* invalidation can veto that publish by clearing both of those bits
* (setting INVALIDATING in the same operation if VALID was set, to
- * record that its grace period is still owed), and a mutator can
- * consume the whole state with one xchg to learn whether it must
- * wait for readers before touching anything they see. See the
- * comments in pfncache.c.
+ * record that its grace period is still owed) and observe in the same
+ * read whether a vCPU has the pfn pinned for guest-mode use
+ * (GUEST_USING, set by the vCPU with a cmpxchg only while VALID
+ * holds, cleared only by that vCPU or by a mutator's xchg), and a
+ * mutator can consume the whole state with one xchg to learn whether
+ * it must wait for readers and/or kick the pinning vCPU before
+ * touching anything they see. See the comments in pfncache.c.
*
* 'active' deliberately stays separate: the invalidation and
* mutation paths operate on the word without having to care about
@@ -129,6 +142,7 @@ struct gfn_to_pfn_cache {
* VALID clear.
*/
#define GPC_INVALIDATING 0x4
+#define GPC_GUEST_USING 0x8
#ifdef KVM_ARCH_NR_OBJS_PER_MEMORY_CACHE
/*
diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index 2fe50dfa3fc1..e15d9a475fed 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -25,7 +25,9 @@
void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm, unsigned long start,
unsigned long end)
{
+ DECLARE_BITMAP(vcpu_bitmap, KVM_MAX_VCPUS);
struct gfn_to_pfn_cache *gpc;
+ bool evict_vcpus = false;
bool cleared = false;
spin_lock(&kvm->gpc_lock);
@@ -91,10 +93,60 @@ void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm, unsigned long start,
if (old & GPC_VALID)
cleared = true;
+
+ /*
+ * If a vCPU has the pfn pinned for direct use from
+ * guest mode, it must be forced out of guest mode
+ * before the invalidation may complete. The pin bit
+ * was set with a cmpxchg only while GPC_VALID held,
+ * so this same read of 'old' cannot miss it. It is
+ * NOT cleared here: only the vCPU itself (on request
+ * service, before re-pinning) or a mutator's xchg
+ * (which inherits the kick duty) may clear it. A
+ * stale pin costs at most a spurious request bit:
+ * kvm_make_vcpus_request_mask() only IPIs and waits
+ * for targets actually in guest mode.
+ */
+ if (old & GPC_GUEST_USING) {
+ if (!evict_vcpus) {
+ evict_vcpus = true;
+ bitmap_zero(vcpu_bitmap, KVM_MAX_VCPUS);
+ }
+ __set_bit(gpc->vcpu->vcpu_idx, vcpu_bitmap);
+ /*
+ * Set this cache's own service request (what
+ * the vCPU must do before re-entering); the
+ * force-out below is common to all of them.
+ */
+ __kvm_make_request(gpc->vcpu_req, gpc->vcpu);
+ }
}
}
spin_unlock(&kvm->gpc_lock);
+ /*
+ * Force pinned vCPUs out of guest mode before waiting for the
+ * kernel readers below and before the caller zaps the page tables.
+ * The request includes KVM_REQUEST_WAIT (enforced at init), so this
+ * does not return until the vCPUs have left guest mode; they cannot
+ * re-enter with the stale mapping, because re-pinning requires
+ * GPC_VALID, which requires a refresh, which cannot complete until
+ * invalidate_range_end().
+ *
+ * This is safe on unblockable (OOM reaper) ranges too, with no
+ * special-casing: kvm_make_vcpus_request_mask() runs with
+ * preemption disabled and spin-waits for IPI acks; it never sleeps.
+ * Do not be tempted to skip the wait for !blockable ranges on the
+ * assumption that a reaped task's vCPUs are already stopped: that
+ * is false for process_mrelease(), which only requires
+ * SIGNAL_GROUP_EXIT, and a vCPU still in guest mode after this
+ * walk returns is exactly the use-after-free this kick exists to
+ * prevent.
+ */
+ if (evict_vcpus)
+ kvm_make_vcpus_request_mask(kvm, KVM_REQ_OUTSIDE_GUEST_MODE,
+ vcpu_bitmap);
+
/*
* Readers may still be using the old mapping, having sampled
* GPC_VALID before it was cleared. Wait for them all to drain
@@ -122,7 +174,7 @@ void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm, unsigned long start,
* so the grace period never blocks in reclaim either.)
*/
if (cleared)
- synchronize_srcu_expedited(&kvm->gpc_srcu);
+ synchronize_srcu_atomic(&kvm->gpc_srcu);
/*
* Note the GPC_INVALIDATING markers set above are deliberately NOT
@@ -190,6 +242,100 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len)
return true;
}
+/*
+ * A mutator's xchg of the state word to zero may have consumed a live
+ * GPC_GUEST_USING pin. Clearing that bit obliges the clearer to do what
+ * the invalidation walk would have done: post the cache's service
+ * request and force the vCPU out of guest mode, waiting until it has
+ * left, before mutating or tearing down what the guest may be using.
+ *
+ * The service request is posted unconditionally: consuming a pin means
+ * whatever physical address the vCPU had latched (e.g. into a hardware
+ * control structure) is stale, and only the request handler makes that
+ * good. That holds even when the pinning vCPU is the caller itself —
+ * a vCPU refreshing its own pinned cache may re-map at a different pfn,
+ * and without the request nothing would rewrite the stale address
+ * before the next guest entry. Only the waited force-out is skipped in
+ * that case: a vCPU running a refresh is by definition not in guest
+ * mode. (The vCPU's own unpin is the one silent path, in
+ * kvm_gpc_unpin_for_guest(), because unpinning asserts the address is
+ * no longer latched anywhere.)
+ */
+static void gpc_kick_pinned_vcpu(struct gfn_to_pfn_cache *gpc, int old_state)
+{
+ DECLARE_BITMAP(vcpu_bitmap, KVM_MAX_VCPUS);
+
+ if (!(old_state & GPC_GUEST_USING))
+ return;
+ if (WARN_ON_ONCE(!gpc->vcpu))
+ return;
+
+ __kvm_make_request(gpc->vcpu_req, gpc->vcpu);
+
+ if (kvm_get_running_vcpu() == gpc->vcpu)
+ return;
+
+ /*
+ * As in the invalidation walk: force the vCPU out of guest mode and
+ * wait until it has left (KVM_REQ_OUTSIDE_GUEST_MODE carries
+ * KVM_REQUEST_WAIT). A plain kvm_vcpu_kick() would not wait.
+ */
+ bitmap_zero(vcpu_bitmap, KVM_MAX_VCPUS);
+ __set_bit(gpc->vcpu->vcpu_idx, vcpu_bitmap);
+ kvm_make_vcpus_request_mask(gpc->kvm, KVM_REQ_OUTSIDE_GUEST_MODE,
+ vcpu_bitmap);
+}
+
+bool kvm_gpc_pin_for_guest(struct gfn_to_pfn_cache *gpc)
+{
+ int old, new;
+
+ WARN_ON_ONCE(!gpc->vcpu);
+ WARN_ON_ONCE(gpc->vcpu && kvm_get_running_vcpu() != gpc->vcpu);
+
+ /*
+ * Set GPC_GUEST_USING only while GPC_VALID holds: the pin then
+ * cannot race with an invalidation, because the invalidation walk
+ * reads the whole state word in one atomic operation — either it
+ * sees VALID (and this pin, if set) and kicks; or the publish which
+ * set VALID has not happened and the walk's veto of
+ * GPC_BECOMING_VALID prevents it. The acquire pairs with the
+ * release-publish so the pinned pfn/khva are the published ones.
+ */
+ old = atomic_read_acquire(&gpc->state);
+ do {
+ if (!(old & GPC_VALID))
+ return false;
+ new = old | GPC_GUEST_USING;
+ } while (!atomic_try_cmpxchg(&gpc->state, &old, new));
+
+ return true;
+}
+EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gpc_pin_for_guest);
+
+void kvm_gpc_unpin_for_guest(struct gfn_to_pfn_cache *gpc)
+{
+ /*
+ * Only the pinning vCPU itself may unpin — except on teardown
+ * paths (vCPU destruction), which run with no running-vCPU
+ * context but with the vCPU necessarily out of guest mode.
+ */
+ WARN_ON_ONCE(gpc->vcpu && kvm_get_running_vcpu() &&
+ kvm_get_running_vcpu() != gpc->vcpu);
+
+ /*
+ * The vCPU's last access to the pfn must be visible before the pin
+ * is seen clear by an invalidator deciding it need not kick
+ * (atomic_andnot() alone is unordered). In practice the VM-exit
+ * which preceded this call already serialized the guest's accesses,
+ * but the API should not depend on its callers' exit paths. A pin
+ * already consumed by a mutator's xchg makes this a harmless no-op.
+ */
+ smp_mb__before_atomic();
+ atomic_andnot(GPC_GUEST_USING, &gpc->state);
+}
+EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gpc_unpin_for_guest);
+
static void *gpc_map(kvm_pfn_t pfn)
{
if (pfn_valid(pfn))
@@ -405,7 +551,7 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
bool unmap_old = false;
unsigned long old_uhva;
kvm_pfn_t old_pfn;
- bool must_drain;
+ int old_state;
void *old_khva;
int ret;
@@ -498,10 +644,10 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
* reader-visible fields (uhva, gpa, memslot, and later pfn/khva) be
* mutated or the old mapping torn down.
*/
- must_drain = atomic_xchg(&gpc->state, 0) &
- (GPC_VALID | GPC_INVALIDATING);
- if (must_drain)
- synchronize_srcu_expedited(&gpc->kvm->gpc_srcu);
+ old_state = atomic_xchg(&gpc->state, 0);
+ gpc_kick_pinned_vcpu(gpc, old_state);
+ if (old_state & (GPC_VALID | GPC_INVALIDATING))
+ synchronize_srcu_atomic(&gpc->kvm->gpc_srcu);
old_pfn = gpc->pfn;
old_khva = (void *)PAGE_ALIGN_DOWN((uintptr_t)gpc->khva);
@@ -599,7 +745,7 @@ int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, unsigned long len)
}
void __kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm,
- bool never_dirty)
+ bool never_dirty, struct kvm_vcpu *vcpu, u32 vcpu_req)
{
mutex_init(&gpc->refresh_lock);
@@ -610,6 +756,8 @@ void __kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm,
gpc->active = false;
atomic_set(&gpc->state, 0);
gpc->never_dirty = never_dirty;
+ gpc->vcpu = vcpu;
+ gpc->vcpu_req = vcpu_req;
}
static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long uhva,
@@ -667,6 +815,7 @@ void kvm_gpc_deactivate(struct gfn_to_pfn_cache *gpc)
struct kvm *kvm = gpc->kvm;
kvm_pfn_t old_pfn;
void *old_khva;
+ int old_state;
guard(mutex)(&gpc->refresh_lock);
@@ -687,8 +836,9 @@ void kvm_gpc_deactivate(struct gfn_to_pfn_cache *gpc)
* readers outstanding — it left GPC_INVALIDATING set to say
* so — and the unmap below must not run under those readers.
*/
- if (atomic_xchg(&gpc->state, 0) &
- (GPC_VALID | GPC_INVALIDATING))
+ old_state = atomic_xchg(&gpc->state, 0);
+ gpc_kick_pinned_vcpu(gpc, old_state);
+ if (old_state & (GPC_VALID | GPC_INVALIDATING))
synchronize_srcu_atomic(&kvm->gpc_srcu);
/*
--
2.55.0
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 07/17] KVM: pfncache: Add guest-mode pinning (GUEST_USES_PFN successor)
2026-09-20 20:49 ` [PATCH 07/17] KVM: pfncache: Add guest-mode pinning (GUEST_USES_PFN successor) David Woodhouse
@ 2026-09-20 21:53 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-09-20 21:53 UTC (permalink / raw)
To: David Woodhouse; +Cc: kvm, linux-rt-devel, linux-kernel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] kvm: uaf in pfncache due to bypassed vcpu kick on concurrent invalidation
- [Low] kvm: build failure due to non-existent synchronize_srcu_atomic API
--
commit f94174d271f1e35cea5e589d0bf75a4374e505a9
Author: David Woodhouse <dwmw@amazon.co.uk>
KVM: pfncache: Add guest-mode pinning (GUEST_USES_PFN successor)
This commit reinstates support for pfn caches used directly from guest
mode. It attempts to provide guarantees that the vCPU is kicked out
of guest mode and waits when the cache is invalidated, ensuring that
a stale mapping is not used.
Link: https://lore.kernel.org/all/ZBEEQtmtNPaEqU1i@google.com
Link: https://lore.kernel.org/all/ZQiR8IpqOZrOpzHC@google.com
Link: https://lore.kernel.org/all/agJnqh7ACEkB-ftx@google.com
Link: https://lore.kernel.org/all/20260102142429.896101-1-griffoul@gmail.com
> diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
> index 2fe50dfa3fc16..e15d9a475fed7 100644
> --- a/virt/kvm/pfncache.c
> +++ b/virt/kvm/pfncache.c
[ ... ]
> @@ -91,10 +93,60 @@ void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm, unsigned long start,
>
> if (old & GPC_VALID)
> cleared = true;
> +
> + /*
> + * If a vCPU has the pfn pinned for direct use from
> + * guest mode, it must be forced out of guest mode
> + * before the invalidation may complete. The pin bit
> + * was set with a cmpxchg only while GPC_VALID held,
> + * so this same read of 'old' cannot miss it. It is
> + * NOT cleared here: only the vCPU itself (on request
> + * service, before re-pinning) or a mutator's xchg
> + * (which inherits the kick duty) may clear it. A
> + * stale pin costs at most a spurious request bit:
> + * kvm_make_vcpus_request_mask() only IPIs and waits
> + * for targets actually in guest mode.
> + */
> + if (old & GPC_GUEST_USING) {
[Severity: Critical]
Does the early skip condition in this loop fail to check for GPC_GUEST_USING?
Looking at the existing context just before this hunk in
gfn_to_pfn_cache_invalidate_start():
old = atomic_read_acquire(&gpc->state);
if (!(old & (GPC_VALID | GPC_BECOMING_VALID)))
continue;
If Thread A clears GPC_VALID (but leaves GPC_GUEST_USING set), and
concurrently Thread B runs mmu_notifier_invalidate_range_start(), it seems
Thread B would evaluate the above condition as true, skipping the cache
entirely and ignoring the pin.
Could this allow core MM to free the physical page while the vCPU is still
running in guest mode and using the freed page in its hardware control
structures, leading to a use-after-free?
[ ... ]
> @@ -122,7 +174,7 @@ void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm, unsigned long start,
> * so the grace period never blocks in reclaim either.)
> */
> if (cleared)
> - synchronize_srcu_expedited(&kvm->gpc_srcu);
> + synchronize_srcu_atomic(&kvm->gpc_srcu);
[Severity: Low]
Is synchronize_srcu_atomic() a defined API in the kernel? It looks like
this patch replaces synchronize_srcu_expedited() with this function, but
it does not appear to exist, which could cause build failures.
[ ... ]
> @@ -498,10 +644,10 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
> * reader-visible fields (uhva, gpa, memslot, and later pfn/khva) be
> * mutated or the old mapping torn down.
> */
> - must_drain = atomic_xchg(&gpc->state, 0) &
> - (GPC_VALID | GPC_INVALIDATING);
> - if (must_drain)
> - synchronize_srcu_expedited(&gpc->kvm->gpc_srcu);
> + old_state = atomic_xchg(&gpc->state, 0);
> + gpc_kick_pinned_vcpu(gpc, old_state);
[Severity: Critical]
Does clearing the state to 0 here prematurely hide the GPC_GUEST_USING pin
from concurrent MMU notifiers?
It appears this completely wipes the pin before waiting for the vCPU. If a
concurrent invalidation occurs before the vCPU exits, it might bypass the
vCPU kick because the pin was erased, potentially exposing the same
use-after-free window.
> + if (old_state & (GPC_VALID | GPC_INVALIDATING))
> + synchronize_srcu_atomic(&gpc->kvm->gpc_srcu);
[Severity: Low]
As noted above, synchronize_srcu_atomic() does not appear to exist.
[ ... ]
> @@ -687,8 +836,9 @@ void kvm_gpc_deactivate(struct gfn_to_pfn_cache *gpc)
> * readers outstanding — it left GPC_INVALIDATING set to say
> * so — and the unmap below must not run under those readers.
> */
> - if (atomic_xchg(&gpc->state, 0) &
> - (GPC_VALID | GPC_INVALIDATING))
> + old_state = atomic_xchg(&gpc->state, 0);
> + gpc_kick_pinned_vcpu(gpc, old_state);
[Severity: Critical]
Similarly, does this zeroing of the state in kvm_gpc_deactivate() hide the
GPC_GUEST_USING pin from concurrent invalidators before the vCPU actually
exits?
> + if (old_state & (GPC_VALID | GPC_INVALIDATING))
> synchronize_srcu_atomic(&kvm->gpc_srcu);
[Severity: Low]
This also uses synchronize_srcu_atomic() which does not seem to be defined.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260920211920.928306-1-dwmw2@infradead.org?part=7
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 08/17] KVM: pfncache: Return -EAGAIN for a lookup which hits an invalid memslot
2026-09-20 20:49 [PATCH 00/17] KVM: Use atomic SRCU for gfn-to-pfn cache, reinstate guest mode for x86 nesting David Woodhouse
` (6 preceding siblings ...)
2026-09-20 20:49 ` [PATCH 07/17] KVM: pfncache: Add guest-mode pinning (GUEST_USES_PFN successor) David Woodhouse
@ 2026-09-20 20:49 ` David Woodhouse
2026-09-20 21:53 ` sashiko-bot
2026-09-20 20:49 ` [PATCH 09/17] KVM: x86: Post KVM_REQ_GET_NESTED_STATE_PAGES on memslot updates David Woodhouse
` (8 subsequent siblings)
16 siblings, 1 reply; 27+ messages in thread
From: David Woodhouse @ 2026-09-20 20:49 UTC (permalink / raw)
To: kvm
Cc: linux-kernel, rcu, linux-rt-devel, linux-kselftest,
Paolo Bonzini, Sean Christopherson, Paul Durrant,
Vitaly Kuznetsov, Fred Griffoul, Paul E. McKenney, Kunwu Chan,
Kunwu Chan, Zqiang, Boqun Feng, Neeraj Upadhyay, Joel Fernandes,
Lai Jiangshan, Josh Triplett, Mathieu Desnoyers,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Jason Gunthorpe, Michal Hocko, Nikita Kalyazin, Keir Fraser,
David Matlack, nh-open-source, David Woodhouse
From: David Woodhouse <dwmw@amazon.co.uk>
A memslot being deleted or moved is present in the active array but
flagged KVM_MEMSLOT_INVALID — a transient state which exists only
within a single KVM_SET_USER_MEMORY_REGION call, resolved (or
reverted) before that ioctl returns and announced by a further
memslot generation bump. gfn_to_hva_memslot() collapses that case
and a genuinely unbacked gfn into one error, and the pfncache
refresh reported both as -EFAULT.
Distinguish them: return -EAGAIN when the slot exists but is
invalid, so that callers may retry — mirroring what RET_PF_RETRY
does for ordinary guest faults on an invalid slot — rather than
treat the gfn as unbacked.
For most consumers no change is needed: check-at-use sites (vcpu_info
event injection, steal_time, pvclock) skip the update and self-heal
on their next invocation, and treating -EAGAIN as any other failure
is no worse than before. Event channel delivery is the exception:
abandoning it on a transient refresh failure drops the event. All
three producers — userspace ioctl, guest evtchn_send hypercall, and
eventfd/irqfd (whose inatomic fast path already punts to a workqueue
on -EWOULDBLOCK) — converge on the kvm_xen_set_evtchn() slow path, so
teach its retry loop to keep going on -EAGAIN. A guest send racing a
memslot update now waits out the update (with cond_resched()) instead
of bouncing the hypercall out to userspace.
Where a userspace ioctl performs the resolution (e.g. setting a
vcpu_info address), a concurrent memslot update in the VMM's own
other thread now yields -EAGAIN to userspace instead of -EFAULT:
truthful, actionable, and only visible to a VMM racing against
itself.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
arch/x86/kvm/xen.c | 12 +++++++++++-
virt/kvm/pfncache.c | 17 ++++++++++++++++-
2 files changed, 27 insertions(+), 2 deletions(-)
diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
index b6643b889320..0b49eb67d06c 100644
--- a/arch/x86/kvm/xen.c
+++ b/arch/x86/kvm/xen.c
@@ -2026,7 +2026,17 @@ static int kvm_xen_set_evtchn(struct kvm_xen_evtchn *xe, struct kvm *kvm)
idx = srcu_read_lock(&kvm->srcu);
rc = kvm_gpc_refresh(gpc, PAGE_SIZE);
srcu_read_unlock(&kvm->srcu, idx);
- } while(!rc);
+
+ /*
+ * A refresh which raced with a concurrent memslot update
+ * (-EAGAIN: the slot exists but is mid-DELETE/MOVE) will
+ * succeed once the update completes or reverts; giving up
+ * instead would drop the event. Only a genuinely unbacked
+ * gfn (-EFAULT etc.) is fatal.
+ */
+ if (rc == -EAGAIN)
+ cond_resched();
+ } while (!rc || rc == -EAGAIN);
if (mm_borrowed)
kthread_unuse_mm(kvm->mm);
diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index e15d9a475fed..29397ac02668 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -681,7 +681,22 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned l
WRITE_ONCE(gpc->uhva, gfn_to_hva_memslot(gpc->memslot, gfn));
if (kvm_is_error_hva(gpc->uhva)) {
- ret = -EFAULT;
+ /*
+ * A slot being deleted or moved is present
+ * but flagged invalid; the lookup failure is
+ * transient, resolved when the update
+ * completes (or reverts) and announced by a
+ * new memslot generation. Distinguish that
+ * from a genuinely unbacked gfn so that the
+ * caller may retry rather than report an
+ * error, as RET_PF_RETRY does for ordinary
+ * guest faults.
+ */
+ if (gpc->memslot &&
+ (gpc->memslot->flags & KVM_MEMSLOT_INVALID))
+ ret = -EAGAIN;
+ else
+ ret = -EFAULT;
goto out;
}
--
2.55.0
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 08/17] KVM: pfncache: Return -EAGAIN for a lookup which hits an invalid memslot
2026-09-20 20:49 ` [PATCH 08/17] KVM: pfncache: Return -EAGAIN for a lookup which hits an invalid memslot David Woodhouse
@ 2026-09-20 21:53 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-09-20 21:53 UTC (permalink / raw)
To: David Woodhouse; +Cc: linux-rt-devel, kvm, linux-kernel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] kvm: ABBA deadlock in kvm_xen_set_evtchn() against memslot updates
--
commit d08948c3f01d34b213b2379e1b128830ccb5f086
Author: David Woodhouse <dwmw@amazon.co.uk>
KVM: pfncache: Return -EAGAIN for a lookup which hits an invalid memslot
This commit modifies the pfncache lookup to return -EAGAIN when a memslot
is marked KVM_MEMSLOT_INVALID. It also updates kvm_xen_set_evtchn() to
indefinitely retry the refresh on -EAGAIN instead of failing.
> diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c
> index b6643b889320d..0b49eb67d06c0 100644
> --- a/arch/x86/kvm/xen.c
> +++ b/arch/x86/kvm/xen.c
> @@ -2026,7 +2026,17 @@ static int kvm_xen_set_evtchn(struct kvm_xen_evtchn *xe, struct kvm *kvm)
> idx = srcu_read_lock(&kvm->srcu);
> rc = kvm_gpc_refresh(gpc, PAGE_SIZE);
> srcu_read_unlock(&kvm->srcu, idx);
> - } while(!rc);
> +
> + /*
> + * A refresh which raced with a concurrent memslot update
> + * (-EAGAIN: the slot exists but is mid-DELETE/MOVE) will
> + * succeed once the update completes or reverts; giving up
> + * instead would drop the event. Only a genuinely unbacked
> + * gfn (-EFAULT etc.) is fatal.
> + */
> + if (rc == -EAGAIN)
> + cond_resched();
> + } while (!rc || rc == -EAGAIN);
[Severity: High]
Does this create an ABBA deadlock against memslot updates when called from
the vcpu_run() path?
A guest VM issuing an EVTCHNOP_send hypercall operates inside vcpu_run(),
which holds the outer kvm->srcu read lock. If this races with a VMM memslot
update (e.g., KVM_SET_USER_MEMORY_REGION), kvm_xen_set_evtchn() will encounter
-EAGAIN and loop here waiting for the update to complete.
However, cond_resched() does not drop the outer kvm->srcu lock. The thread
performing the memslot update is blocked in synchronize_srcu(&kvm->srcu)
waiting for all existing readers (including this vCPU thread) to drop their
locks.
This results in an unbreakable deadlock where the vCPU thread waits for the
memslot update, while the update waits for the vCPU thread to drop kvm->srcu.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260920211920.928306-1-dwmw2@infradead.org?part=8
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 09/17] KVM: x86: Post KVM_REQ_GET_NESTED_STATE_PAGES on memslot updates
2026-09-20 20:49 [PATCH 00/17] KVM: Use atomic SRCU for gfn-to-pfn cache, reinstate guest mode for x86 nesting David Woodhouse
` (7 preceding siblings ...)
2026-09-20 20:49 ` [PATCH 08/17] KVM: pfncache: Return -EAGAIN for a lookup which hits an invalid memslot David Woodhouse
@ 2026-09-20 20:49 ` David Woodhouse
2026-09-20 20:49 ` [PATCH 10/17] KVM: nVMX: Implement cache for L1 MSR bitmap David Woodhouse
` (7 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: David Woodhouse @ 2026-09-20 20:49 UTC (permalink / raw)
To: kvm
Cc: linux-kernel, rcu, linux-rt-devel, linux-kselftest,
Paolo Bonzini, Sean Christopherson, Paul Durrant,
Vitaly Kuznetsov, Fred Griffoul, Paul E. McKenney, Kunwu Chan,
Kunwu Chan, Zqiang, Boqun Feng, Neeraj Upadhyay, Joel Fernandes,
Lai Jiangshan, Josh Triplett, Mathieu Desnoyers,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Jason Gunthorpe, Michal Hocko, Nikita Kalyazin, Keir Fraser,
David Matlack, nh-open-source, David Woodhouse
From: David Woodhouse <dwmw@amazon.co.uk>
The physical addresses which nested page setup latches into hardware
control structures (vmcs02's APIC-access, virtual-APIC and posted
interrupt descriptor addresses) are derived from gPA→uHVA translations
which a memslot update can change. Software readers of the underlying
gfn_to_pfn_caches catch this lazily, via the memslot generation check
in kvm_gpc_check() at their next use — but the CPU's use of a latched
address from guest mode is continuous and checks nothing. A vCPU
running L2 across a memslot move would keep using the old translation
until something forced it to re-resolve; L0 exits which re-enter L2
without a nested VM-exit never re-run nested page setup.
(This is a staleness, not a lifetime, problem: freeing the underlying
page is the mmu_notifier's business and that path kicks pinned vCPUs
synchronously. The replaced kvm_host_map code had the same staleness
with no remedy at all.)
The alternative, checking each cache's memslot generation in the
VM-entry path after vcpu->mode is set, is strictly worse: the check
would run on every nested VM-entry forever, in a context which cannot
refresh (IRQs off), so its only possible action on a mismatch would be
to post KVM_REQ_GET_NESTED_STATE_PAGES and bail for the refresh to
happen outside. Posting that same request from the memslot update
itself — the single point where the generation actually changes, and
a slow path by definition — is the same mechanism minus the per-entry
cost. The request bit is also the artifact that survives racing with
a concurrent VM-entry: a bare kick landing before vcpu->mode is set
would be lost, and a vCPU which resolved its pages against the old
memslots but has not yet entered guest mode is invisible to any
is_guest_mode() filter, so the request is posted unconditionally to
every vCPU.
Accordingly, downgrade the WARN in svm_get_nested_state_pages(): a
spurious request outside guest mode is now expected, and a no-op.
(vmx_get_nested_state_pages already tolerates it.)
The memslot-move mode of the vmx_apic_update_test selftest exercises
this path.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
arch/x86/kvm/svm/nested.c | 7 ++++++-
arch/x86/kvm/x86.c | 26 ++++++++++++++++++++------
2 files changed, 26 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 73f37b050d0a..acc423b13445 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -2106,7 +2106,12 @@ static int svm_set_nested_state(struct kvm_vcpu *vcpu,
static bool svm_get_nested_state_pages(struct kvm_vcpu *vcpu)
{
- if (WARN_ON(!is_guest_mode(vcpu)))
+ /*
+ * Memslot updates post this request to every vCPU (to make any
+ * vCPU which has guest pages latched re-resolve them against the
+ * new memslots), so it can arrive with nothing to do.
+ */
+ if (!is_guest_mode(vcpu))
return true;
if (is_pae_paging(vcpu)) {
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 116932e13d59..07d1cfb051f5 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -10172,18 +10172,32 @@ static int kvm_alloc_memslot_metadata(struct kvm *kvm,
void kvm_arch_memslots_updated(struct kvm *kvm, u64 gen)
{
- struct kvm_vcpu *vcpu;
- unsigned long i;
-
/*
* memslots->generation has been incremented.
* mmio generation may have reached its maximum value.
*/
kvm_mmu_invalidate_mmio_sptes(kvm, gen);
- /* Force re-initialization of steal_time cache */
- kvm_for_each_vcpu(i, vcpu, kvm)
- kvm_vcpu_kick(vcpu);
+ /*
+ * Force re-initialization of the steal_time cache, and of any
+ * nested-state pages whose physical addresses a vCPU has latched
+ * in hardware control structures (e.g. vmcs02) from a
+ * gfn_to_pfn_cache. Software readers of such caches catch the
+ * generation bump lazily, via kvm_gpc_check() at their next use;
+ * the CPU's use from guest mode is continuous and checks nothing,
+ * so the vCPU must be told to re-resolve and re-latch before it
+ * next enters the guest. The request is the artifact that
+ * survives racing with a concurrent VM-entry (a bare kick landing
+ * before vcpu->mode is set would be lost); its handler re-runs
+ * nested page setup, whose gPA lookups then see the new
+ * generation and refresh.
+ *
+ * The wake/kick this performs on every vCPU is also what forces
+ * re-initialization of the steal_time cache: its check-at-use
+ * sites likewise only see the new generation once the vCPU goes
+ * around its run loop.
+ */
+ kvm_make_all_cpus_request(kvm, KVM_REQ_GET_NESTED_STATE_PAGES);
}
int kvm_arch_prepare_memory_region(struct kvm *kvm,
--
2.55.0
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 10/17] KVM: nVMX: Implement cache for L1 MSR bitmap
2026-09-20 20:49 [PATCH 00/17] KVM: Use atomic SRCU for gfn-to-pfn cache, reinstate guest mode for x86 nesting David Woodhouse
` (8 preceding siblings ...)
2026-09-20 20:49 ` [PATCH 09/17] KVM: x86: Post KVM_REQ_GET_NESTED_STATE_PAGES on memslot updates David Woodhouse
@ 2026-09-20 20:49 ` David Woodhouse
2026-09-20 21:56 ` sashiko-bot
2026-09-20 20:49 ` [PATCH 11/17] KVM: nVMX: Use pinned pfncache for L1 APIC virtualization pages David Woodhouse
` (6 subsequent siblings)
16 siblings, 1 reply; 27+ messages in thread
From: David Woodhouse @ 2026-09-20 20:49 UTC (permalink / raw)
To: kvm
Cc: linux-kernel, rcu, linux-rt-devel, linux-kselftest,
Paolo Bonzini, Sean Christopherson, Paul Durrant,
Vitaly Kuznetsov, Fred Griffoul, Paul E. McKenney, Kunwu Chan,
Kunwu Chan, Zqiang, Boqun Feng, Neeraj Upadhyay, Joel Fernandes,
Lai Jiangshan, Josh Triplett, Mathieu Desnoyers,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Jason Gunthorpe, Michal Hocko, Nikita Kalyazin, Keir Fraser,
David Matlack, nh-open-source, David Woodhouse
From: Fred Griffoul <fgriffo@amazon.co.uk>
Introduce a gfn_to_pfn_cache to optimize L1 MSR bitmap access by
replacing map/unmap operations. This optimization reduces overhead
during L2 VM-entry where nested_vmx_prepare_msr_bitmap() merges L1's
MSR intercepts with L0's requirements.
The current implementation maps and unmaps the page on every bitmap
merge, which is a significant performance impact with unmanaged guest
memory (where the map is a memremap/memunmap cycle).
The cache is initialized when entering VMX operation and deactivated
when VMX operation ends. Readers run under kvm->gpc_srcu, following
the check/activate/retry pattern; KVM only ever reads the L1 bitmap,
so the cache is marked never-dirty.
This exports the core pfncache API to kvm-intel.ko, which is its
first modular user.
[dwmw2: Port from the rwlock-protected pfncache to the SRCU reader
protocol, mark the cache never-dirty, rebase over the
kvm_vcpu_map_local_readonly CLASS() conversion.]
Signed-off-by: Fred Griffoul <fgriffo@amazon.co.uk>
Co-developed-by: David Woodhouse <dwmw@amazon.co.uk>
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
arch/x86/kvm/vmx/nested.c | 47 +++++++++++++++++++++++++++++++++++----
arch/x86/kvm/vmx/vmx.h | 2 ++
virt/kvm/pfncache.c | 5 +++++
3 files changed, 50 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 151873407abd..5fe7e5d1f72d 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -320,6 +320,38 @@ static void vmx_switch_vmcs(struct kvm_vcpu *vcpu, struct loaded_vmcs *vmcs)
kvm_reset_dirty_registers(vcpu);
}
+/*
+ * Map a single page of L1 guest memory at @gpa and enter an SRCU read-side
+ * critical section protecting the mapping. Returns the SRCU index (>= 0) to
+ * pass to nested_gpc_unlock(), or a negative error code. While locked, the
+ * cache's khva and pfn are guaranteed to remain valid: an invalidation
+ * cannot complete until the reader drains.
+ */
+static int nested_gpc_lock(struct gfn_to_pfn_cache *gpc, gpa_t gpa)
+{
+ int idx, err;
+
+ if (!PAGE_ALIGNED(gpa))
+ return -EINVAL;
+retry:
+ idx = srcu_read_lock_atomic(&gpc->kvm->gpc_srcu);
+ if (!kvm_gpc_check(gpc, PAGE_SIZE) || gpc->gpa != gpa) {
+ srcu_read_unlock_atomic(&gpc->kvm->gpc_srcu, idx);
+ err = kvm_gpc_activate(gpc, gpa, PAGE_SIZE);
+ if (err)
+ return err;
+
+ goto retry;
+ }
+
+ return idx;
+}
+
+static void nested_gpc_unlock(struct gfn_to_pfn_cache *gpc, int idx)
+{
+ srcu_read_unlock_atomic(&gpc->kvm->gpc_srcu, idx);
+}
+
static void nested_put_vmcs12_pages(struct kvm_vcpu *vcpu)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
@@ -370,6 +402,8 @@ static void free_nested(struct kvm_vcpu *vcpu)
kfree(vmx->nested.cached_shadow_vmcs12);
vmx->nested.cached_shadow_vmcs12 = NULL;
+ kvm_gpc_deactivate(&vmx->nested.msr_bitmap_cache);
+
nested_put_vmcs12_pages(vcpu);
kvm_mmu_free_roots(vcpu->kvm, &vcpu->arch.guest_mmu, KVM_MMU_ROOTS_ALL);
@@ -734,7 +768,7 @@ static inline bool nested_vmx_prepare_msr_bitmap(struct kvm_vcpu *vcpu,
struct vmcs12 *vmcs12)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
- int msr;
+ int msr, idx;
unsigned long *msr_bitmap_l1;
unsigned long *msr_bitmap_l0 = vmx->nested.vmcs02.msr_bitmap;
@@ -759,11 +793,11 @@ static inline bool nested_vmx_prepare_msr_bitmap(struct kvm_vcpu *vcpu,
return true;
}
- CLASS(kvm_vcpu_map_local_readonly, m)(vcpu, gpa_to_gfn(vmcs12->msr_bitmap));
- if (m.ret)
+ idx = nested_gpc_lock(&vmx->nested.msr_bitmap_cache, vmcs12->msr_bitmap);
+ if (idx < 0)
return false;
- msr_bitmap_l1 = (unsigned long *)m.map.hva;
+ msr_bitmap_l1 = (unsigned long *)vmx->nested.msr_bitmap_cache.khva;
/*
* To keep the control flow simple, pay eight 8-byte writes (sixteen
@@ -843,6 +877,8 @@ static inline bool nested_vmx_prepare_msr_bitmap(struct kvm_vcpu *vcpu,
nested_vmx_merge_pmu_msr_bitmaps(vcpu, msr_bitmap_l1, msr_bitmap_l0);
+ nested_gpc_unlock(&vmx->nested.msr_bitmap_cache, idx);
+
vmx->nested.force_msr_bitmap_recalc = false;
return true;
@@ -5443,6 +5479,9 @@ static int enter_vmx_operation(struct kvm_vcpu *vcpu)
vmx->nested.vpid02 = allocate_vpid();
+ /* KVM only ever reads the L1 MSR bitmap, so never mark it dirty. */
+ __kvm_gpc_init(&vmx->nested.msr_bitmap_cache, vcpu->kvm, true, NULL, 0);
+
/*
* Clear last_vpid to ensure that the VPID is flushed on the first
* nested VM-Enter. Otherwise, stale TLB entries from a previous life of
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index dc8517f15bc4..d5aa5aa83c95 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -146,6 +146,8 @@ struct nested_vmx {
* Guest pages referred to in the vmcs02 with host-physical
* pointers, so we must keep them pinned while L2 runs.
*/
+ struct gfn_to_pfn_cache msr_bitmap_cache;
+
struct kvm_host_map apic_access_page_map;
struct kvm_host_map virtual_apic_map;
struct kvm_host_map pi_desc_map;
diff --git a/virt/kvm/pfncache.c b/virt/kvm/pfncache.c
index 29397ac02668..c634058a73bf 100644
--- a/virt/kvm/pfncache.c
+++ b/virt/kvm/pfncache.c
@@ -241,6 +241,7 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigned long len)
return true;
}
+EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gpc_check);
/*
* A mutator's xchg of the state word to zero may have consumed a live
@@ -758,6 +759,7 @@ int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, unsigned long len)
return __kvm_gpc_refresh(gpc, gpc->gpa, uhva);
}
+EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gpc_refresh);
void __kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm,
bool never_dirty, struct kvm_vcpu *vcpu, u32 vcpu_req)
@@ -774,6 +776,7 @@ void __kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm,
gpc->vcpu = vcpu;
gpc->vcpu_req = vcpu_req;
}
+EXPORT_SYMBOL_FOR_KVM_INTERNAL(__kvm_gpc_init);
static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long uhva,
unsigned long len)
@@ -816,6 +819,7 @@ int kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, unsigned long len)
return __kvm_gpc_activate(gpc, gpa, KVM_HVA_ERR_BAD, len);
}
+EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gpc_activate);
int kvm_gpc_activate_hva(struct gfn_to_pfn_cache *gpc, unsigned long uhva, unsigned long len)
{
@@ -875,3 +879,4 @@ void kvm_gpc_deactivate(struct gfn_to_pfn_cache *gpc)
gpc_unmap(old_pfn, old_khva);
}
}
+EXPORT_SYMBOL_FOR_KVM_INTERNAL(kvm_gpc_deactivate);
--
2.55.0
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 10/17] KVM: nVMX: Implement cache for L1 MSR bitmap
2026-09-20 20:49 ` [PATCH 10/17] KVM: nVMX: Implement cache for L1 MSR bitmap David Woodhouse
@ 2026-09-20 21:56 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-09-20 21:56 UTC (permalink / raw)
To: David Woodhouse; +Cc: linux-rt-devel, linux-kernel, kvm
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] kvm: nVMX: __kvm_gpc_init() re-initializes mutex on every VMXON
- [Medium] kvm: nVMX: gfn_to_pfn_cache breaks read-only L1 MSR bitmaps
--
commit 6f0690b4a89f3e0c76a94240889c416f2d71a8ec
Author: Fred Griffoul <fgriffo@amazon.co.uk>
KVM: nVMX: Implement cache for L1 MSR bitmap
This commit introduces a gfn_to_pfn_cache to optimize L1 MSR bitmap accesses
during L2 VM-entry, replacing individual map and unmap operations. The cache
is initialized on VMXON and deactivated when VMX operation ends.
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 151873407abd3..5fe7e5d1f72d9 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
[ ... ]
> @@ -759,11 +793,11 @@ static inline bool nested_vmx_prepare_msr_bitmap(struct kvm_vcpu *vcpu,
> return true;
> }
>
> - CLASS(kvm_vcpu_map_local_readonly, m)(vcpu, gpa_to_gfn(vmcs12->msr_bitmap));
> - if (m.ret)
> + idx = nested_gpc_lock(&vmx->nested.msr_bitmap_cache, vmcs12->msr_bitmap);
> + if (idx < 0)
[Severity: Medium]
Does this change break support for read-only L1 MSR bitmaps?
The previous code used kvm_vcpu_map_local_readonly(), which maps guest pages
without requesting write access. By switching to nested_gpc_lock(), the
underlying page lookup hardcodes FOLL_WRITE.
If the L1 hypervisor provides a read-only MSR bitmap, GPC activation will
fail here, causing KVM to silently fall back to software emulation for all
L2 MSR accesses.
> return false;
>
> - msr_bitmap_l1 = (unsigned long *)m.map.hva;
> + msr_bitmap_l1 = (unsigned long *)vmx->nested.msr_bitmap_cache.khva;
>
> /*
> * To keep the control flow simple, pay eight 8-byte writes (sixteen
[ ... ]
> @@ -5443,6 +5479,9 @@ static int enter_vmx_operation(struct kvm_vcpu *vcpu)
>
> vmx->nested.vpid02 = allocate_vpid();
>
> + /* KVM only ever reads the L1 MSR bitmap, so never mark it dirty. */
> + __kvm_gpc_init(&vmx->nested.msr_bitmap_cache, vcpu->kvm, true, NULL, 0);
[Severity: High]
Does this re-initialize an active mutex on consecutive VMX transitions?
enter_vmx_operation() is called when the L1 guest executes VMXON, and
__kvm_gpc_init() calls mutex_init(&gpc->refresh_lock). When the guest
executes VMXOFF, free_nested() deactivates the cache but does not destroy
the mutex.
A subsequent VMXON will re-initialize the already active mutex, which
triggers lockdep/debugobjects warnings and can cause a host panic if
panic_on_warn is set.
> +
> /*
> * Clear last_vpid to ensure that the VPID is flushed on the first
> * nested VM-Enter. Otherwise, stale TLB entries from a previous life of
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260920211920.928306-1-dwmw2@infradead.org?part=10
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 11/17] KVM: nVMX: Use pinned pfncache for L1 APIC virtualization pages
2026-09-20 20:49 [PATCH 00/17] KVM: Use atomic SRCU for gfn-to-pfn cache, reinstate guest mode for x86 nesting David Woodhouse
` (9 preceding siblings ...)
2026-09-20 20:49 ` [PATCH 10/17] KVM: nVMX: Implement cache for L1 MSR bitmap David Woodhouse
@ 2026-09-20 20:49 ` David Woodhouse
2026-09-20 21:57 ` sashiko-bot
2026-09-20 20:49 ` [PATCH 12/17] KVM: selftests: Add nested VMX APIC cache invalidation test David Woodhouse
` (5 subsequent siblings)
16 siblings, 1 reply; 27+ messages in thread
From: David Woodhouse @ 2026-09-20 20:49 UTC (permalink / raw)
To: kvm
Cc: linux-kernel, rcu, linux-rt-devel, linux-kselftest,
Paolo Bonzini, Sean Christopherson, Paul Durrant,
Vitaly Kuznetsov, Fred Griffoul, Paul E. McKenney, Kunwu Chan,
Kunwu Chan, Zqiang, Boqun Feng, Neeraj Upadhyay, Joel Fernandes,
Lai Jiangshan, Josh Triplett, Mathieu Desnoyers,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Jason Gunthorpe, Michal Hocko, Nikita Kalyazin, Keir Fraser,
David Matlack, nh-open-source, David Woodhouse
From: Fred Griffoul <fgriffo@amazon.co.uk>
Replace kvm_host_map usage with gfn_to_pfn_cache for the L1 pages whose
physical addresses are handed to the CPU in vmcs02 for direct use while
running L2: the APIC-access page, the virtual-APIC page, and the posted
interrupt descriptor. This eliminates the map/unmap (with unmanaged
memory: memremap/memunmap) cycle on every L2 VM-entry/exit.
The caches are initialized with kvm_gpc_init_for_vcpu(), naming
KVM_REQ_GET_NESTED_STATE_PAGES as the service request, and their pfns
are pinned for guest use (GPC_GUEST_USING) when written into vmcs02 by
nested_get_vmcs12_pages(). If an MMU notifier invalidation hits a
pinned cache, the pfncache core posts that request, forces the vCPU
out of guest mode, and waits for it to leave; the request handler
re-runs nested_get_vmcs12_pages(), whose refresh cannot complete until
the invalidation ends, and rewrites the vmcs02 fields before the next
entry. The pins are dropped at nested VM-exit (including the
vmentry_fail_vmexit path, which does not pass through
__nested_vmx_vmexit()) and the caches deactivated when VMX operation
ends.
Note that no validity check is added to the guest-entry path: the
request substitutes for it. The invariant is that a stale physical
address latched in vmcs02 always implies a pending request, which the
existing kvm_request_pending() check catches after vcpu->mode is set
to IN_GUEST_MODE. It is maintained from both directions:
- Every path which consumes a pin (the notifier walk, or a
refresh/deactivate from any context, including the owning vCPU
refreshing its own cache from a host-side reader) posts the
service request.
- The request handler cannot lose it: vcpu_enter_guest() consumes
the request bit before calling in, so ANY failure of
vmx_get_nested_state_pages() — including early exits which touch
no cache at all, such as a failed load_pdptrs() — re-posts the
request before exiting to userspace. A persistently failing
configuration bounces to userspace on every KVM_RUN rather than
ever entering L2 with a stale address. (The old kvm_host_map code
could tolerate losing this race because the map held a page
reference; the pfncache deliberately holds none.)
This is what closes the historical KVM_GUEST_USES_PFN gaps¹: the kick
is guaranteed to be acted upon before re-entry, without polling cache
validity on every entry.
Host-side (khva) readers of the virtual-APIC and PI descriptor pages
do not need the pin, which exists only for the benefit of the
physical addresses latched in vmcs02.
vmx_complete_nested_posted_interrupt() runs in a sleepable context
and uses the SRCU-protected check/refresh protocol.
vmx_has_nested_events() is reachable from the kvm_vcpu_block() loop
under set_current_state(TASK_INTERRUPTIBLE), where a refresh (mutex,
GUP, synchronize_srcu) must not sleep: it instead uses a non-sleeping
try-lock and reports "event pending" when a cache has been
invalidated. The resulting bounce through vcpu_run services the
pending request in a sleepable context and re-evaluates events for
real; a spurious wakeup is safe, whereas returning false could miss
a wakeup (the service request is posted with KVM_REQUEST_NO_WAKEUP).
To keep an unbackable page from turning that conservative answer into
a wakeup livelock, a failed activation deactivates the cache rather
than leaving it active-but-invalid.
Dirty marking for these pages, which the CPU writes via the vmcs02
physical addresses and thus bypasses EPT-based dirty tracking, moves
from the unmap path to gfn-based marking from the vmcs12 fields on
every L2 exit (__vmx_handle_exit() already does this on every exit
from L2, so unmap-time marking was redundant); the one software
writer, __kvm_apic_update_irr() from
vmx_complete_nested_posted_interrupt(), marks at the write site.
¹ https://lore.kernel.org/all/ZBEEQtmtNPaEqU1i@google.com
Signed-off-by: Fred Griffoul <fgriffo@amazon.co.uk>
Co-developed-by: David Woodhouse <dwmw@amazon.co.uk>
Failure to resolve a page distinguishes three cases. At entry time
(VMLAUNCH/VMRESUME, or resume from KVM_SET_NESTED_STATE, tracked in
nested.resume_pending) an unbackable page is a misconfiguration:
report KVM_EXIT_INTERNAL_ERROR as ever (vmx_apic_access_test asserts
this). On lazy revalidation — a pending KVM_REQ_GET_NESTED_STATE_PAGES
consumed while L2 runs — a slot mid-update (-EAGAIN from the
pfncache) re-posts the request and continues; the pending request
prevents VM entry, so the vCPU retries until the memslot update
completes. And a genuinely unbacked gfn on lazy revalidation means
userspace yanked the backing from under a running L2: clear
SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES in vmcs02 instead of killing
the VM, so L2's accesses take ordinary EPT violations and are
handled as MMIO, exactly as a non-nested guest touching the same
unbacked gpa would be — and as upstream KVM (which never re-resolves)
effectively behaves. The control is recomputed from vmcs12 on the
next nested entry, and a later memslot update which re-backs the gfn
re-latches the page, so the degradation heals itself
(vmx_apic_update_test's move-memslot phase exercises this).
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
arch/x86/kvm/vmx/nested.c | 345 +++++++++++++++++++++++++++++++++-----
arch/x86/kvm/vmx/vmx.c | 11 +-
arch/x86/kvm/vmx/vmx.h | 14 +-
3 files changed, 319 insertions(+), 51 deletions(-)
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 5fe7e5d1f72d..105ff6cbfe58 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -352,14 +352,107 @@ static void nested_gpc_unlock(struct gfn_to_pfn_cache *gpc, int idx)
srcu_read_unlock_atomic(&gpc->kvm->gpc_srcu, idx);
}
+/*
+ * Map a page of L1 memory, pin it for direct use by the guest (i.e. by
+ * the CPU while running the L2 guest, via a physical address in vmcs02),
+ * and return the host physical address to write into vmcs02. The pin
+ * guarantees that if the mapping is invalidated, this vCPU is forced out
+ * of guest mode and KVM_REQ_GET_NESTED_STATE_PAGES is posted so that the
+ * address is re-established (or the vCPU exits to userspace) before the
+ * next entry to guest mode.
+ */
+static int nested_gpc_hpa(struct gfn_to_pfn_cache *gpc, gpa_t gpa, hpa_t *hpa)
+{
+ int idx;
+
+ do {
+ idx = nested_gpc_lock(gpc, gpa);
+ if (idx < 0)
+ return idx;
+
+ /*
+ * The pin can fail only if an invalidation cleared the
+ * valid bit after nested_gpc_lock() checked it; go back
+ * around to refresh (which will not complete until the
+ * invalidation is over) and try again.
+ */
+ if (kvm_gpc_pin_for_guest(gpc))
+ break;
+
+ nested_gpc_unlock(gpc, idx);
+ } while (1);
+
+ *hpa = pfn_to_hpa(gpc->pfn);
+ nested_gpc_unlock(gpc, idx);
+ return 0;
+}
+
+/*
+ * Try to lock an already-configured cache for host-side (khva) access,
+ * without sleeping. Returns -ENOENT if the cache is not active (e.g.
+ * posted interrupts not configured for this L2), -EWOULDBLOCK if it is
+ * active but currently invalidated (only a sleeping refresh would make
+ * it usable). Does not pin; the mapping is stable only until
+ * nested_gpc_unlock().
+ */
+static int nested_gpc_try_lock_if_active(struct gfn_to_pfn_cache *gpc)
+{
+ int idx;
+
+ /*
+ * Check for an inactive (or never-initialized: gpc->kvm is NULL
+ * before VMXON) cache before dereferencing gpc->kvm. Deactivation
+ * only happens on this vCPU (under vcpu->mutex), so the check
+ * cannot race with the lock which follows it.
+ */
+ if (!gpc->active)
+ return -ENOENT;
+
+ idx = srcu_read_lock_atomic(&gpc->kvm->gpc_srcu);
+ if (!gpc->active) {
+ srcu_read_unlock_atomic(&gpc->kvm->gpc_srcu, idx);
+ return -ENOENT;
+ }
+
+ if (!kvm_gpc_check(gpc, PAGE_SIZE)) {
+ srcu_read_unlock_atomic(&gpc->kvm->gpc_srcu, idx);
+ return -EWOULDBLOCK;
+ }
+
+ return idx;
+}
+
+/*
+ * As above, but refresh the cache if it has been invalidated. May sleep;
+ * callers must be in a sleepable context (in particular, NOT under
+ * set_current_state() in a block loop, and not in a VM-exit fastpath).
+ */
+static int nested_gpc_lock_if_active(struct gfn_to_pfn_cache *gpc)
+{
+ int idx, err;
+
+ while ((idx = nested_gpc_try_lock_if_active(gpc)) == -EWOULDBLOCK) {
+ err = kvm_gpc_refresh(gpc, PAGE_SIZE);
+ if (err)
+ return err;
+ }
+
+ return idx;
+}
+
+static struct pi_desc *nested_pi_desc(struct vcpu_vmx *vmx)
+{
+ return (struct pi_desc *)((u8 *)vmx->nested.pi_desc_cache.khva +
+ vmx->nested.pi_desc_offset);
+}
+
static void nested_put_vmcs12_pages(struct kvm_vcpu *vcpu)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
- kvm_vcpu_unmap(vcpu, &vmx->nested.apic_access_page_map);
- kvm_vcpu_unmap(vcpu, &vmx->nested.virtual_apic_map);
- kvm_vcpu_unmap(vcpu, &vmx->nested.pi_desc_map);
- vmx->nested.pi_desc = NULL;
+ kvm_gpc_unpin_for_guest(&vmx->nested.apic_access_page_cache);
+ kvm_gpc_unpin_for_guest(&vmx->nested.virtual_apic_cache);
+ kvm_gpc_unpin_for_guest(&vmx->nested.pi_desc_cache);
}
/*
@@ -378,6 +471,7 @@ static void free_nested(struct kvm_vcpu *vcpu)
return;
kvm_clear_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu);
+ vmx->nested.resume_pending = false;
vmx->nested.vmxon = false;
vmx->nested.smm.vmxon = false;
@@ -402,10 +496,13 @@ static void free_nested(struct kvm_vcpu *vcpu)
kfree(vmx->nested.cached_shadow_vmcs12);
vmx->nested.cached_shadow_vmcs12 = NULL;
- kvm_gpc_deactivate(&vmx->nested.msr_bitmap_cache);
-
nested_put_vmcs12_pages(vcpu);
+ kvm_gpc_deactivate(&vmx->nested.pi_desc_cache);
+ kvm_gpc_deactivate(&vmx->nested.virtual_apic_cache);
+ kvm_gpc_deactivate(&vmx->nested.apic_access_page_cache);
+ kvm_gpc_deactivate(&vmx->nested.msr_bitmap_cache);
+
kvm_mmu_free_roots(vcpu->kvm, &vcpu->arch.guest_mmu, KVM_MMU_ROOTS_ALL);
nested_release_evmcs(vcpu);
@@ -3475,11 +3572,11 @@ static bool nested_get_evmcs_page(struct kvm_vcpu *vcpu)
}
#endif
-static bool nested_get_vmcs12_pages(struct kvm_vcpu *vcpu)
+static bool nested_get_vmcs12_pages(struct kvm_vcpu *vcpu, bool lazy)
{
struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
struct vcpu_vmx *vmx = to_vmx(vcpu);
- struct kvm_host_map *map;
+ hpa_t hpa;
if (!vcpu->arch.pdptrs_from_userspace &&
!nested_cpu_has_ept(vmcs12) && is_pae_paging(vcpu)) {
@@ -3494,26 +3591,74 @@ static bool nested_get_vmcs12_pages(struct kvm_vcpu *vcpu)
if (nested_cpu_has2(vmcs12, SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)) {
- map = &vmx->nested.apic_access_page_map;
-
- if (!kvm_vcpu_map(vcpu, gpa_to_gfn(vmcs12->apic_access_addr), map)) {
- vmcs_write64(APIC_ACCESS_ADDR, pfn_to_hpa(map->pfn));
- } else {
+ int err = nested_gpc_hpa(&vmx->nested.apic_access_page_cache,
+ vmcs12->apic_access_addr, &hpa);
+ if (!err) {
+ vmcs_write64(APIC_ACCESS_ADDR, hpa);
+ } else if (err == -EAGAIN) {
+ kvm_gpc_unpin_for_guest(&vmx->nested.apic_access_page_cache);
+ kvm_gpc_deactivate(&vmx->nested.apic_access_page_cache);
+ goto retry;
+ } else if (!lazy) {
+ /*
+ * Entry-time resolution (VMLAUNCH/VMRESUME, or resume
+ * from KVM_SET_NESTED_STATE): an unbackable
+ * APIC-access page is a misconfiguration to report,
+ * not a transient to ride out.
+ */
pr_debug_ratelimited("%s: no backing for APIC-access address in vmcs12\n",
__func__);
vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
vcpu->run->internal.suberror =
KVM_INTERNAL_ERROR_EMULATION;
vcpu->run->internal.ndata = 0;
+ kvm_gpc_unpin_for_guest(&vmx->nested.apic_access_page_cache);
+ kvm_gpc_deactivate(&vmx->nested.apic_access_page_cache);
return false;
+ } else {
+ /*
+ * Lazy revalidation (a memslot update or invalidation
+ * while L2 runs) found the gfn unbacked: userspace
+ * yanked the backing from under a running L2. Turn
+ * off the APIC-access match for vmcs02 instead of
+ * killing the VM; L2's accesses to the page then take
+ * ordinary EPT violations on an unbacked gfn and are
+ * handled (or reported) as MMIO, exactly as a
+ * non-nested guest touching the same gpa would be.
+ * prepare_vmcs02_early() recomputes the control from
+ * vmcs12 on the next nested entry, and a later
+ * memslot update which re-backs the gfn re-posts the
+ * request and re-latches the page, so the
+ * degradation heals itself.
+ */
+ kvm_gpc_unpin_for_guest(&vmx->nested.apic_access_page_cache);
+ kvm_gpc_deactivate(&vmx->nested.apic_access_page_cache);
+ secondary_exec_controls_clearbit(vmx,
+ SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES);
}
}
if (nested_cpu_has(vmcs12, CPU_BASED_TPR_SHADOW)) {
- map = &vmx->nested.virtual_apic_map;
+ int vapic_err = nested_gpc_hpa(&vmx->nested.virtual_apic_cache,
+ vmcs12->virtual_apic_page_addr, &hpa);
- if (!kvm_vcpu_map(vcpu, gpa_to_gfn(vmcs12->virtual_apic_page_addr), map)) {
- vmcs_write64(VIRTUAL_APIC_PAGE_ADDR, pfn_to_hpa(map->pfn));
+ /*
+ * On failure, deactivate the cache: an active-but-invalid
+ * cache would read as "might have an event" to the
+ * non-sleeping check in vmx_has_nested_events(), causing
+ * spurious wakeups for as long as the page remains
+ * unbackable.
+ */
+ if (vapic_err) {
+ kvm_gpc_unpin_for_guest(&vmx->nested.virtual_apic_cache);
+ kvm_gpc_deactivate(&vmx->nested.virtual_apic_cache);
+ }
+
+ if (vapic_err == -EAGAIN)
+ goto retry;
+
+ if (!vapic_err) {
+ vmcs_write64(VIRTUAL_APIC_PAGE_ADDR, hpa);
} else if (nested_cpu_has(vmcs12, CPU_BASED_CR8_LOAD_EXITING) &&
nested_cpu_has(vmcs12, CPU_BASED_CR8_STORE_EXITING) &&
!nested_cpu_has2(vmcs12, SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)) {
@@ -3536,14 +3681,13 @@ static bool nested_get_vmcs12_pages(struct kvm_vcpu *vcpu)
}
if (nested_cpu_has_posted_intr(vmcs12)) {
- map = &vmx->nested.pi_desc_map;
-
- if (!kvm_vcpu_map(vcpu, gpa_to_gfn(vmcs12->posted_intr_desc_addr), map)) {
- vmx->nested.pi_desc =
- (struct pi_desc *)(((void *)map->hva) +
- offset_in_page(vmcs12->posted_intr_desc_addr));
+ int pi_err = nested_gpc_hpa(&vmx->nested.pi_desc_cache,
+ vmcs12->posted_intr_desc_addr & PAGE_MASK, &hpa);
+ if (!pi_err) {
+ vmx->nested.pi_desc_offset =
+ offset_in_page(vmcs12->posted_intr_desc_addr);
vmcs_write64(POSTED_INTR_DESC_ADDR,
- pfn_to_hpa(map->pfn) + offset_in_page(vmcs12->posted_intr_desc_addr));
+ hpa + offset_in_page(vmcs12->posted_intr_desc_addr));
} else {
/*
* Defer the KVM_INTERNAL_EXIT until KVM tries to
@@ -3551,7 +3695,10 @@ static bool nested_get_vmcs12_pages(struct kvm_vcpu *vcpu)
* descriptor. (Note that KVM may do this when it
* should not, per the architectural specification.)
*/
- vmx->nested.pi_desc = NULL;
+ kvm_gpc_unpin_for_guest(&vmx->nested.pi_desc_cache);
+ kvm_gpc_deactivate(&vmx->nested.pi_desc_cache);
+ if (pi_err == -EAGAIN)
+ goto retry;
pin_controls_clearbit(vmx, PIN_BASED_POSTED_INTR);
}
}
@@ -3561,9 +3708,27 @@ static bool nested_get_vmcs12_pages(struct kvm_vcpu *vcpu)
exec_controls_clearbit(vmx, CPU_BASED_USE_MSR_BITMAPS);
return true;
+
+retry:
+ /*
+ * The gfn's memslot is being moved or deleted: present but
+ * flagged invalid, so the lookup failure is transient and the
+ * gfn must not (yet) be treated as unbacked. Re-post the
+ * request and report success: the pending request prevents
+ * any VM entry (vcpu_enter_guest() bails and reprocesses
+ * requests), so L2 cannot run with the partially-updated
+ * vmcs02, and the vCPU retries here until the memslot update
+ * completes or reverts. This mirrors RET_PF_RETRY for
+ * ordinary guest faults on an invalid slot; as there, a
+ * userspace which never completes the update leaves the vCPU
+ * retrying indefinitely, with signals and request processing
+ * still serviced.
+ */
+ kvm_make_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu);
+ return true;
}
-static bool vmx_get_nested_state_pages(struct kvm_vcpu *vcpu)
+static bool __vmx_get_nested_state_pages(struct kvm_vcpu *vcpu)
{
#ifdef CONFIG_KVM_HYPERV
/*
@@ -3584,9 +3749,39 @@ static bool vmx_get_nested_state_pages(struct kvm_vcpu *vcpu)
}
#endif
- if (is_guest_mode(vcpu) && !nested_get_vmcs12_pages(vcpu))
+ if (is_guest_mode(vcpu) &&
+ !nested_get_vmcs12_pages(vcpu, !to_vmx(vcpu)->nested.resume_pending))
return false;
+ /*
+ * Success consumes the resume classification (a -EAGAIN retry
+ * "succeeds" too: the resume did all it could against a memslot
+ * mid-update, and the re-posted request's consumption is a fresh
+ * revalidation like any other).
+ */
+ to_vmx(vcpu)->nested.resume_pending = false;
+ return true;
+}
+
+static bool vmx_get_nested_state_pages(struct kvm_vcpu *vcpu)
+{
+ if (!__vmx_get_nested_state_pages(vcpu)) {
+ /*
+ * The caller (vcpu_enter_guest) consumed
+ * KVM_REQ_GET_NESTED_STATE_PAGES before calling in, and a
+ * failure here may leave a previously latched — and, after
+ * an invalidation, stale and unreferenced — physical
+ * address in vmcs02. Re-post the request so that no path
+ * can re-enter L2 without retrying this function: the
+ * invariant is that a stale latched address always implies
+ * a pending request. If the failure persists, the vCPU
+ * bounces to userspace on every KVM_RUN rather than ever
+ * entering L2, which is the invariant doing its job.
+ */
+ kvm_make_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu);
+ return false;
+ }
+
return true;
}
@@ -3715,7 +3910,7 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
prepare_vmcs02_early(vmx, &vmx->vmcs01, vmcs12);
if (from_vmentry) {
- if (unlikely(!nested_get_vmcs12_pages(vcpu))) {
+ if (unlikely(!nested_get_vmcs12_pages(vcpu, false))) {
vmx_switch_vmcs(vcpu, &vmx->vmcs01);
return NVMX_VMENTRY_KVM_INTERNAL_ERROR;
}
@@ -3753,6 +3948,7 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
* to nested_get_vmcs12_pages before the next VM-entry. The MSRs
* have already been set at vmentry time and should not be reset.
*/
+ vmx->nested.resume_pending = true;
kvm_make_request(KVM_REQ_GET_NESTED_STATE_PAGES, vcpu);
}
@@ -3809,6 +4005,14 @@ enum nvmx_vmentry_status nested_vmx_enter_non_root_mode(struct kvm_vcpu *vcpu,
vmentry_fail_vmexit:
vmx_switch_vmcs(vcpu, &vmx->vmcs01);
+ /*
+ * Release any guest-mode pins taken by nested_get_vmcs12_pages()
+ * during this failed entry: this path returns to L1 without going
+ * through __nested_vmx_vmexit(), which is where they are normally
+ * dropped. (Idempotent if a later vmexit drops them again.)
+ */
+ nested_put_vmcs12_pages(vcpu);
+
if (!from_vmentry)
return NVMX_VMENTRY_VMEXIT;
@@ -4065,6 +4269,8 @@ static void vmcs12_save_pending_event(struct kvm_vcpu *vcpu,
static int vmx_complete_nested_posted_interrupt(struct kvm_vcpu *vcpu)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
+ int pi_idx, vapic_idx;
+ struct pi_desc *pi_desc;
int max_irr;
void *vapic_page;
u16 status;
@@ -4072,22 +4278,32 @@ static int vmx_complete_nested_posted_interrupt(struct kvm_vcpu *vcpu)
if (!vmx->nested.pi_pending)
return 0;
- if (!vmx->nested.pi_desc)
+ pi_idx = nested_gpc_lock_if_active(&vmx->nested.pi_desc_cache);
+ if (pi_idx < 0)
goto mmio_needed;
+ pi_desc = nested_pi_desc(vmx);
vmx->nested.pi_pending = false;
- if (!pi_test_and_clear_on(vmx->nested.pi_desc))
+ if (!pi_test_and_clear_on(pi_desc)) {
+ nested_gpc_unlock(&vmx->nested.pi_desc_cache, pi_idx);
return 0;
+ }
- max_irr = pi_find_highest_vector(vmx->nested.pi_desc);
+ max_irr = pi_find_highest_vector(pi_desc);
if (max_irr > 0) {
- vapic_page = vmx->nested.virtual_apic_map.hva;
- if (!vapic_page)
+ vapic_idx = nested_gpc_lock_if_active(&vmx->nested.virtual_apic_cache);
+ if (vapic_idx < 0) {
+ nested_gpc_unlock(&vmx->nested.pi_desc_cache, pi_idx);
goto mmio_needed;
+ }
+ vapic_page = vmx->nested.virtual_apic_cache.khva;
+
+ __kvm_apic_update_irr(pi_desc->pir, vapic_page, &max_irr);
+
+ kvm_gpc_mark_dirty_in_slot(&vmx->nested.virtual_apic_cache);
+ nested_gpc_unlock(&vmx->nested.virtual_apic_cache, vapic_idx);
- __kvm_apic_update_irr(vmx->nested.pi_desc->pir,
- vapic_page, &max_irr);
status = vmcs_read16(GUEST_INTR_STATUS);
if ((u8)max_irr > ((u8)status & 0xff)) {
status &= ~0xff;
@@ -4096,8 +4312,8 @@ static int vmx_complete_nested_posted_interrupt(struct kvm_vcpu *vcpu)
}
}
- kvm_vcpu_map_mark_dirty(vcpu, &vmx->nested.virtual_apic_map);
- kvm_vcpu_map_mark_dirty(vcpu, &vmx->nested.pi_desc_map);
+ kvm_gpc_mark_dirty_in_slot(&vmx->nested.pi_desc_cache);
+ nested_gpc_unlock(&vmx->nested.pi_desc_cache, pi_idx);
return 0;
mmio_needed:
@@ -4216,8 +4432,9 @@ static bool nested_vmx_preemption_timer_pending(struct kvm_vcpu *vcpu)
static bool vmx_has_nested_events(struct kvm_vcpu *vcpu, bool for_injection)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
- void *vapic = vmx->nested.virtual_apic_map.hva;
- int max_irr, vppr;
+ int max_irr, vppr, idx;
+ struct pi_desc *pi_desc;
+ void *vapic;
if (nested_vmx_preemption_timer_pending(vcpu) ||
vmx->nested.mtf_pending)
@@ -4236,20 +4453,48 @@ static bool vmx_has_nested_events(struct kvm_vcpu *vcpu, bool for_injection)
__vmx_interrupt_blocked(vcpu))
return false;
- if (!vapic)
+ /*
+ * This is called from non-sleeping contexts (notably the
+ * kvm_vcpu_block() loop, under set_current_state()), so it must
+ * not refresh an invalidated cache. Claim a pending event
+ * instead: the bounce through vcpu_run services the pending
+ * KVM_REQ_GET_NESTED_STATE_PAGES (posted by whatever invalidated
+ * the cache) in a sleepable context, re-establishing the cache,
+ * and re-evaluates events for real. A spurious wakeup is safe;
+ * sleeping here is not, and returning false could miss a wakeup.
+ */
+ idx = nested_gpc_try_lock_if_active(&vmx->nested.virtual_apic_cache);
+ if (idx == -EWOULDBLOCK)
+ return true;
+ if (idx < 0)
return false;
+ vapic = vmx->nested.virtual_apic_cache.khva;
vppr = *((u32 *)(vapic + APIC_PROCPRI));
+ nested_gpc_unlock(&vmx->nested.virtual_apic_cache, idx);
+
max_irr = vmx_get_rvi();
if ((max_irr & 0xf0) > (vppr & 0xf0))
return true;
- if (vmx->nested.pi_pending && vmx->nested.pi_desc &&
- pi_test_on(vmx->nested.pi_desc)) {
- max_irr = pi_find_highest_vector(vmx->nested.pi_desc);
- if (max_irr > 0 && (max_irr & 0xf0) > (vppr & 0xf0))
+ if (vmx->nested.pi_pending) {
+ idx = nested_gpc_try_lock_if_active(&vmx->nested.pi_desc_cache);
+ if (idx == -EWOULDBLOCK)
return true;
+ if (idx < 0)
+ return false;
+
+ pi_desc = nested_pi_desc(vmx);
+ if (pi_test_on(pi_desc)) {
+ max_irr = pi_find_highest_vector(pi_desc);
+ if (max_irr > 0 && (max_irr & 0xf0) > (vppr & 0xf0)) {
+ nested_gpc_unlock(&vmx->nested.pi_desc_cache, idx);
+ return true;
+ }
+ }
+
+ nested_gpc_unlock(&vmx->nested.pi_desc_cache, idx);
}
return false;
@@ -5482,6 +5727,20 @@ static int enter_vmx_operation(struct kvm_vcpu *vcpu)
/* KVM only ever reads the L1 MSR bitmap, so never mark it dirty. */
__kvm_gpc_init(&vmx->nested.msr_bitmap_cache, vcpu->kvm, true, NULL, 0);
+ /*
+ * The pfns of these three caches are handed to the CPU in vmcs02
+ * fields, for direct use while running the L2 guest. Pinning them
+ * for guest use makes an invalidation force this vCPU out of
+ * guest mode and post KVM_REQ_GET_NESTED_STATE_PAGES, whose
+ * handler re-establishes them before the next entry.
+ */
+ kvm_gpc_init_for_vcpu(&vmx->nested.apic_access_page_cache, vcpu,
+ KVM_REQ_GET_NESTED_STATE_PAGES);
+ kvm_gpc_init_for_vcpu(&vmx->nested.virtual_apic_cache, vcpu,
+ KVM_REQ_GET_NESTED_STATE_PAGES);
+ kvm_gpc_init_for_vcpu(&vmx->nested.pi_desc_cache, vcpu,
+ KVM_REQ_GET_NESTED_STATE_PAGES);
+
/*
* Clear last_vpid to ensure that the VPID is flushed on the first
* nested VM-Enter. Otherwise, stale TLB entries from a previous life of
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 612ab07d4100..25b0d114f2bf 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -6467,11 +6467,14 @@ static void vmx_flush_pml_buffer(struct kvm_vcpu *vcpu)
static void nested_vmx_mark_all_vmcs12_pages_dirty(struct kvm_vcpu *vcpu)
{
- struct vcpu_vmx *vmx = to_vmx(vcpu);
+ struct vmcs12 *vmcs12 = get_vmcs12(vcpu);
- kvm_vcpu_map_mark_dirty(vcpu, &vmx->nested.apic_access_page_map);
- kvm_vcpu_map_mark_dirty(vcpu, &vmx->nested.virtual_apic_map);
- kvm_vcpu_map_mark_dirty(vcpu, &vmx->nested.pi_desc_map);
+ if (nested_cpu_has2(vmcs12, SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES))
+ kvm_vcpu_mark_page_dirty(vcpu, gpa_to_gfn(vmcs12->apic_access_addr));
+ if (nested_cpu_has(vmcs12, CPU_BASED_TPR_SHADOW))
+ kvm_vcpu_mark_page_dirty(vcpu, gpa_to_gfn(vmcs12->virtual_apic_page_addr));
+ if (nested_cpu_has_posted_intr(vmcs12))
+ kvm_vcpu_mark_page_dirty(vcpu, gpa_to_gfn(vmcs12->posted_intr_desc_addr));
}
static void vmx_dump_sel(char *name, uint32_t sel)
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index d5aa5aa83c95..f88e0031be4e 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -73,6 +73,12 @@ struct pt_desc {
struct nested_vmx {
/* Has the level1 guest done vmxon? */
bool vmxon;
+ /*
+ * The pending KVM_REQ_GET_NESTED_STATE_PAGES was posted by
+ * KVM_SET_NESTED_STATE: an unbackable page is then a userspace
+ * error to report, not a transient state to ride out.
+ */
+ bool resume_pending;
gpa_t vmxon_ptr;
bool pml_full;
@@ -148,11 +154,11 @@ struct nested_vmx {
*/
struct gfn_to_pfn_cache msr_bitmap_cache;
- struct kvm_host_map apic_access_page_map;
- struct kvm_host_map virtual_apic_map;
- struct kvm_host_map pi_desc_map;
+ struct gfn_to_pfn_cache apic_access_page_cache;
+ struct gfn_to_pfn_cache virtual_apic_cache;
+ struct gfn_to_pfn_cache pi_desc_cache;
- struct pi_desc *pi_desc;
+ u64 pi_desc_offset;
bool pi_pending;
u16 posted_intr_nv;
--
2.55.0
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 11/17] KVM: nVMX: Use pinned pfncache for L1 APIC virtualization pages
2026-09-20 20:49 ` [PATCH 11/17] KVM: nVMX: Use pinned pfncache for L1 APIC virtualization pages David Woodhouse
@ 2026-09-20 21:57 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-09-20 21:57 UTC (permalink / raw)
To: David Woodhouse; +Cc: linux-rt-devel, linux-kernel, kvm
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] kvm: nVMX: Guest-triggered KVM_EXIT_INTERNAL_ERROR via unaligned APIC-access address
- [Medium] kvm: lockdep: AB-BA circular dependency between gpc->refresh_lock and kvm->gpc_srcu
--
commit daeb1861c76d9216ee8842648d7c50ed244624b3
Author: Fred Griffoul <fgriffo@amazon.co.uk>
Date: Sun Sep 20 20:49:39 2026 +0000
KVM: nVMX: Use pinned pfncache for L1 APIC virtualization pages
Replace kvm_host_map usage with gfn_to_pfn_cache for the L1 pages whose
physical addresses are handed to the CPU in vmcs02 for direct use while
running L2. This eliminates the map/unmap cycle on every L2 VM-entry/exit.
> diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
> index 5fe7e5d1f72d9..105ff6cbfe583 100644
> --- a/arch/x86/kvm/vmx/nested.c
> +++ b/arch/x86/kvm/vmx/nested.c
[ ... ]
> @@ -3494,26 +3591,74 @@ static bool nested_get_vmcs12_pages(struct kvm_vcpu *vcpu)
>
>
> if (nested_cpu_has2(vmcs12, SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES)) {
> - map = &vmx->nested.apic_access_page_map;
> -
> - if (!kvm_vcpu_map(vcpu, gpa_to_gfn(vmcs12->apic_access_addr), map)) {
> - vmcs_write64(APIC_ACCESS_ADDR, pfn_to_hpa(map->pfn));
> - } else {
> + int err = nested_gpc_hpa(&vmx->nested.apic_access_page_cache,
> + vmcs12->apic_access_addr, &hpa);
> + if (!err) {
> + vmcs_write64(APIC_ACCESS_ADDR, hpa);
> + } else if (err == -EAGAIN) {
> + kvm_gpc_unpin_for_guest(&vmx->nested.apic_access_page_cache);
> + kvm_gpc_deactivate(&vmx->nested.apic_access_page_cache);
> + goto retry;
> + } else if (!lazy) {
> + /*
> + * Entry-time resolution (VMLAUNCH/VMRESUME, or resume
> + * from KVM_SET_NESTED_STATE): an unbackable
> + * APIC-access page is a misconfiguration to report,
> + * not a transient to ride out.
> + */
> pr_debug_ratelimited("%s: no backing for APIC-access address in vmcs12\n",
> __func__);
> vcpu->run->exit_reason = KVM_EXIT_INTERNAL_ERROR;
[Severity: High]
Does this allow an untrusted L1 guest to crash the VM by triggering a
KVM_EXIT_INTERNAL_ERROR?
The L1 guest can write a non-page-aligned address to the APIC_ACCESS_ADDR field
in vmcs12 and execute VMLAUNCH. When nested_get_vmcs12_pages() calls
nested_gpc_hpa() with this raw, unaligned address, nested_gpc_lock() will return
-EINVAL due to its strict !PAGE_ALIGNED(gpa) check.
Because this non-lazy path treats -EINVAL as an unbackable page and aborts the
entry with KVM_EXIT_INTERNAL_ERROR, the host userspace might kill the entire VM.
Should this be handled as a graceful architectural VM-Entry failure instead?
> vcpu->run->internal.suberror =
> KVM_INTERNAL_ERROR_EMULATION;
[ ... ]
> @@ -4065,6 +4269,8 @@ static void vmcs12_save_pending_event(struct kvm_vcpu *vcpu,
> static int vmx_complete_nested_posted_interrupt(struct kvm_vcpu *vcpu)
> {
> struct vcpu_vmx *vmx = to_vmx(vcpu);
> + int pi_idx, vapic_idx;
> + struct pi_desc *pi_desc;
> int max_irr;
> void *vapic_page;
> u16 status;
> @@ -4072,22 +4278,32 @@ static int vmx_complete_nested_posted_interrupt(struct kvm_vcpu *vcpu)
> if (!vmx->nested.pi_pending)
> return 0;
>
> - if (!vmx->nested.pi_desc)
> + pi_idx = nested_gpc_lock_if_active(&vmx->nested.pi_desc_cache);
> + if (pi_idx < 0)
> goto mmio_needed;
>
> + pi_desc = nested_pi_desc(vmx);
> vmx->nested.pi_pending = false;
>
> - if (!pi_test_and_clear_on(vmx->nested.pi_desc))
> + if (!pi_test_and_clear_on(pi_desc)) {
> + nested_gpc_unlock(&vmx->nested.pi_desc_cache, pi_idx);
> return 0;
> + }
>
> - max_irr = pi_find_highest_vector(vmx->nested.pi_desc);
> + max_irr = pi_find_highest_vector(pi_desc);
> if (max_irr > 0) {
> - vapic_page = vmx->nested.virtual_apic_map.hva;
> - if (!vapic_page)
> + vapic_idx = nested_gpc_lock_if_active(&vmx->nested.virtual_apic_cache);
[Severity: Medium]
Will this nested lock cause an AB-BA circular locking dependency warning from
lockdep?
Here in vmx_complete_nested_posted_interrupt(), nested_gpc_lock_if_active() is
first called for pi_desc_cache, acquiring the SRCU read lock for kvm->gpc_srcu.
Then, while still holding this SRCU read lock, it attempts to lock the
virtual_apic_cache. If a refresh is needed, kvm_gpc_refresh() is called, which
acquires the gpc->refresh_lock mutex.
Since all gpc->refresh_lock instances share the same lock class, lockdep will
observe gpc_srcu -> refresh_lock here.
However, in kvm_gpc_deactivate() in virt/kvm/pfncache.c:
guard(mutex)(&gpc->refresh_lock);
if (gpc->active) {
...
if (old_state & (GPC_VALID | GPC_INVALIDATING))
synchronize_srcu_atomic(&kvm->gpc_srcu);
The refresh_lock is held while synchronize_srcu_atomic() waits, establishing the
reverse dependency: refresh_lock -> gpc_srcu. This circular dependency can
disable lockdep and pollute kernel logs.
> + if (vapic_idx < 0) {
> + nested_gpc_unlock(&vmx->nested.pi_desc_cache, pi_idx);
> goto mmio_needed;
> + }
> + vapic_page = vmx->nested.virtual_apic_cache.khva;
> +
> + __kvm_apic_update_irr(pi_desc->pir, vapic_page, &max_irr);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260920211920.928306-1-dwmw2@infradead.org?part=11
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 12/17] KVM: selftests: Add nested VMX APIC cache invalidation test
2026-09-20 20:49 [PATCH 00/17] KVM: Use atomic SRCU for gfn-to-pfn cache, reinstate guest mode for x86 nesting David Woodhouse
` (10 preceding siblings ...)
2026-09-20 20:49 ` [PATCH 11/17] KVM: nVMX: Use pinned pfncache for L1 APIC virtualization pages David Woodhouse
@ 2026-09-20 20:49 ` David Woodhouse
2026-09-20 21:51 ` sashiko-bot
2026-09-20 20:49 ` [PATCH 13/17] KVM: x86: Move nested GPC lock helpers to x86.h as kvm_gpc_lock_page() David Woodhouse
` (4 subsequent siblings)
16 siblings, 1 reply; 27+ messages in thread
From: David Woodhouse @ 2026-09-20 20:49 UTC (permalink / raw)
To: kvm
Cc: linux-kernel, rcu, linux-rt-devel, linux-kselftest,
Paolo Bonzini, Sean Christopherson, Paul Durrant,
Vitaly Kuznetsov, Fred Griffoul, Paul E. McKenney, Kunwu Chan,
Kunwu Chan, Zqiang, Boqun Feng, Neeraj Upadhyay, Joel Fernandes,
Lai Jiangshan, Josh Triplett, Mathieu Desnoyers,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Jason Gunthorpe, Michal Hocko, Nikita Kalyazin, Keir Fraser,
David Matlack, nh-open-source, David Woodhouse
From: Fred Griffoul <fgriffo@amazon.co.uk>
Introduce selftest to verify nested VMX APIC virtualization page cache
invalidation and refresh mechanisms for pfncache implementation.
The test exercises the nested VMX APIC cache invalidation path through:
- L2 guest setup: creates a nested environment where L2 accesses the
APIC access page that is cached by KVM using pfncache.
- Cache invalidation triggers: a separate update thread periodically
invalidates the cached pages using either:
- madvise(MADV_DONTNEED) to trigger MMU notifications.
- vm_mem_region_move() to trigger memslot changes.
The test validates that:
- L2 can successfully access APIC page before and after invalidation.
- KVM properly handles cache refresh without guest-visible errors.
- Both MMU notification and memslot change invalidation paths work
correctly.
Signed-off-by: Fred Griffoul <fgriffo@amazon.co.uk>
[dwmw2: Adapt to selftest API drift since the v4 posting: prepare_vmcs()
no longer takes an L2 stack (4c396f19de5d), vm_vaddr_t is now
gva_t, and the prepare_eptp()/nested_map*() EPT helpers were
replaced by vm_enable_tdp()/tdp_map()/
tdp_identity_map_default_memslots().]
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
tools/testing/selftests/kvm/Makefile.kvm | 1 +
.../selftests/kvm/x86/vmx_apic_update_test.c | 299 ++++++++++++++++++
2 files changed, 300 insertions(+)
create mode 100644 tools/testing/selftests/kvm/x86/vmx_apic_update_test.c
diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index 96bab7002d39..b30fa994d5e5 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -155,6 +155,7 @@ TEST_GEN_PROGS_x86 += x86/max_vcpuid_cap_test
TEST_GEN_PROGS_x86 += x86/triple_fault_event_test
TEST_GEN_PROGS_x86 += x86/recalc_apic_map_test
TEST_GEN_PROGS_x86 += x86/aperfmperf_test
+TEST_GEN_PROGS_x86 += x86/vmx_apic_update_test
TEST_GEN_PROGS_x86 += access_tracking_perf_test
TEST_GEN_PROGS_x86 += coalesced_io_test
TEST_GEN_PROGS_x86 += dirty_log_perf_test
diff --git a/tools/testing/selftests/kvm/x86/vmx_apic_update_test.c b/tools/testing/selftests/kvm/x86/vmx_apic_update_test.c
new file mode 100644
index 000000000000..8dd15c92fa90
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86/vmx_apic_update_test.c
@@ -0,0 +1,299 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * vmx_apic_update_test
+ *
+ * Copyright (C) 2025, Amazon.com, Inc. or its affiliates. All Rights Reserved.
+ *
+ * Test L2 guest APIC access page writes with concurrent MMU
+ * notification and memslot move updates.
+ */
+#include <pthread.h>
+#include "test_util.h"
+#include "kvm_util.h"
+#include "processor.h"
+#include "vmx.h"
+
+#define VAPIC_GPA 0xc0000000
+#define VAPIC_SLOT 1
+
+
+#define L2_DELAY (100)
+
+static void l2_guest_code(void)
+{
+ uint32_t *vapic_addr = (uint32_t *) (VAPIC_GPA + 0x80);
+
+ /* Unroll the loop to avoid any compiler side effect */
+
+ WRITE_ONCE(*vapic_addr, 1 << 0);
+ udelay(msecs_to_usecs(L2_DELAY));
+
+ WRITE_ONCE(*vapic_addr, 1 << 1);
+ udelay(msecs_to_usecs(L2_DELAY));
+
+ WRITE_ONCE(*vapic_addr, 1 << 2);
+ udelay(msecs_to_usecs(L2_DELAY));
+
+ WRITE_ONCE(*vapic_addr, 1 << 3);
+ udelay(msecs_to_usecs(L2_DELAY));
+
+ WRITE_ONCE(*vapic_addr, 1 << 4);
+ udelay(msecs_to_usecs(L2_DELAY));
+
+ WRITE_ONCE(*vapic_addr, 1 << 5);
+ udelay(msecs_to_usecs(L2_DELAY));
+
+ WRITE_ONCE(*vapic_addr, 1 << 6);
+ udelay(msecs_to_usecs(L2_DELAY));
+
+ WRITE_ONCE(*vapic_addr, 0);
+ udelay(msecs_to_usecs(L2_DELAY));
+
+ /* Exit to L1 */
+ vmcall();
+}
+
+static void l1_guest_code(struct vmx_pages *vmx_pages)
+{
+ uint32_t control, exit_reason;
+
+ GUEST_ASSERT(prepare_for_vmx_operation(vmx_pages));
+ GUEST_ASSERT(load_vmcs(vmx_pages));
+ prepare_vmcs(vmx_pages, l2_guest_code);
+
+ /* Enable APIC access */
+ control = vmreadz(CPU_BASED_VM_EXEC_CONTROL);
+ control |= CPU_BASED_ACTIVATE_SECONDARY_CONTROLS;
+ vmwrite(CPU_BASED_VM_EXEC_CONTROL, control);
+ control = vmreadz(SECONDARY_VM_EXEC_CONTROL);
+ control |= SECONDARY_EXEC_VIRTUALIZE_APIC_ACCESSES;
+ vmwrite(SECONDARY_VM_EXEC_CONTROL, control);
+ vmwrite(APIC_ACCESS_ADDR, VAPIC_GPA);
+
+ GUEST_SYNC1(0);
+ GUEST_ASSERT(!vmlaunch());
+again:
+ exit_reason = vmreadz(VM_EXIT_REASON);
+ if (exit_reason == EXIT_REASON_APIC_ACCESS) {
+ uint64_t guest_rip = vmreadz(GUEST_RIP);
+ uint64_t instr_len = vmreadz(VM_EXIT_INSTRUCTION_LEN);
+
+ vmwrite(GUEST_RIP, guest_rip + instr_len);
+ GUEST_ASSERT(!vmresume());
+ goto again;
+ }
+
+ GUEST_SYNC1(exit_reason);
+ GUEST_ASSERT(exit_reason == EXIT_REASON_VMCALL);
+ GUEST_DONE();
+}
+
+static const char *progname;
+static int update_period_ms = L2_DELAY / 4;
+
+struct update_control {
+ pthread_mutex_t mutex;
+ pthread_cond_t start_cond;
+ struct kvm_vm *vm;
+ bool running;
+ bool started;
+ int updates;
+};
+
+static void wait_for_start_signal(struct update_control *ctrl)
+{
+ pthread_mutex_lock(&ctrl->mutex);
+ while (!ctrl->started)
+ pthread_cond_wait(&ctrl->start_cond, &ctrl->mutex);
+
+ pthread_mutex_unlock(&ctrl->mutex);
+ printf("%s: starting update\n", progname);
+}
+
+static bool is_running(struct update_control *ctrl)
+{
+ return READ_ONCE(ctrl->running);
+}
+
+static void set_running(struct update_control *ctrl, bool running)
+{
+ WRITE_ONCE(ctrl->running, running);
+}
+
+static void signal_thread_start(struct update_control *ctrl)
+{
+ pthread_mutex_lock(&ctrl->mutex);
+ if (!ctrl->started) {
+ ctrl->started = true;
+ pthread_cond_signal(&ctrl->start_cond);
+ }
+ pthread_mutex_unlock(&ctrl->mutex);
+}
+
+static void *update_madvise(void *arg)
+{
+ struct update_control *ctrl = arg;
+ void *hva;
+
+ wait_for_start_signal(ctrl);
+
+ hva = addr_gpa2hva(ctrl->vm, VAPIC_GPA);
+ memset(hva, 0x45, ctrl->vm->page_size);
+
+ while (is_running(ctrl)) {
+ usleep(update_period_ms * 1000);
+ madvise(hva, ctrl->vm->page_size, MADV_DONTNEED);
+ ctrl->updates++;
+ }
+
+ return NULL;
+}
+
+static void *update_move_memslot(void *arg)
+{
+ struct update_control *ctrl = arg;
+ uint64_t gpa = VAPIC_GPA;
+
+ wait_for_start_signal(ctrl);
+
+ while (is_running(ctrl)) {
+ usleep(update_period_ms * 1000);
+ gpa += 0x10000;
+ vm_mem_region_move(ctrl->vm, VAPIC_SLOT, gpa);
+ ctrl->updates++;
+ }
+
+ return NULL;
+}
+
+static void run(void * (*update)(void *), const char *name)
+{
+ struct kvm_vm *vm;
+ struct kvm_vcpu *vcpu;
+ struct update_control ctrl;
+ struct ucall uc;
+ gva_t vmx_pages_gva;
+ pthread_t update_thread;
+ bool done = false;
+
+ vm = vm_create_with_one_vcpu(&vcpu, l1_guest_code);
+
+ /* Set up nested EPT */
+ vm_enable_tdp(vm);
+
+ /* Allocate VMX pages */
+ vcpu_alloc_vmx(vm, &vmx_pages_gva);
+
+ /* Allocate memory and create VAPIC memslot */
+ vm_userspace_mem_region_add(vm, VM_MEM_SRC_ANONYMOUS, VAPIC_GPA,
+ VAPIC_SLOT, 1, 0);
+
+ /* Allocate guest page table */
+ virt_map(vm, VAPIC_GPA, VAPIC_GPA, 1);
+
+ /* Identity-map everything, including the VAPIC page, in the EPT */
+ tdp_identity_map_default_memslots(vm);
+ tdp_map(vm, VAPIC_GPA, VAPIC_GPA, vm->page_size);
+
+ vcpu_args_set(vcpu, 1, vmx_pages_gva);
+
+ pthread_mutex_init(&ctrl.mutex, NULL);
+ pthread_cond_init(&ctrl.start_cond, NULL);
+ ctrl.vm = vm;
+ ctrl.running = true;
+ ctrl.started = false;
+ ctrl.updates = 0;
+
+ pthread_create(&update_thread, NULL, update, &ctrl);
+
+ printf("%s: running %s (tsc_khz %lu)\n", progname, name, guest_tsc_khz);
+
+ while (!done) {
+ vcpu_run(vcpu);
+
+ switch (vcpu->run->exit_reason) {
+ case KVM_EXIT_IO:
+ switch (get_ucall(vcpu, &uc)) {
+ case UCALL_SYNC:
+ printf("%s: sync(%ld)\n", progname, uc.args[0]);
+ if (uc.args[0] == 0)
+ signal_thread_start(&ctrl);
+ break;
+ case UCALL_ABORT:
+ REPORT_GUEST_ASSERT(uc);
+ /* NOT REACHED */
+ case UCALL_DONE:
+ done = true;
+ break;
+ default:
+ TEST_ASSERT(false, "Unknown ucall %lu", uc.cmd);
+ }
+ break;
+ case KVM_EXIT_MMIO:
+ /* Handle APIC MMIO access after memslot move */
+ printf
+ ("%s: APIC MMIO access at 0x%llx (memslot move effect)\n",
+ progname, vcpu->run->mmio.phys_addr);
+ break;
+ default:
+ TEST_FAIL("%s: Unexpected exit reason: %d (flags 0x%x)",
+ progname,
+ vcpu->run->exit_reason, vcpu->run->flags);
+ }
+ }
+
+ set_running(&ctrl, false);
+ if (!ctrl.started)
+ signal_thread_start(&ctrl);
+ pthread_join(update_thread, NULL);
+ printf("%s: completed with %d updates\n", progname, ctrl.updates);
+
+ pthread_mutex_destroy(&ctrl.mutex);
+ pthread_cond_destroy(&ctrl.start_cond);
+ kvm_vm_free(vm);
+}
+
+int main(int argc, char *argv[])
+{
+ int opt_madvise = 0;
+ int opt_memslot_move = 0;
+
+ TEST_REQUIRE(kvm_cpu_has(X86_FEATURE_VMX));
+ TEST_REQUIRE(kvm_cpu_has_ept());
+
+ if (argc == 1) {
+ opt_madvise = 1;
+ opt_memslot_move = 1;
+ } else {
+ int opt;
+
+ while ((opt = getopt(argc, argv, "amp:")) != -1) {
+ switch (opt) {
+ case 'a':
+ opt_madvise = 1;
+ break;
+ case 'm':
+ opt_memslot_move = 1;
+ break;
+ case 'p':
+ update_period_ms = atoi(optarg);
+ break;
+ default:
+ exit(1);
+ }
+ }
+ }
+
+ TEST_ASSERT(opt_madvise
+ || opt_memslot_move, "No update test configured");
+
+ progname = argv[0];
+
+ if (opt_madvise)
+ run(update_madvise, "madvise");
+
+ if (opt_memslot_move)
+ run(update_move_memslot, "move memslot");
+
+ return 0;
+}
--
2.55.0
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 12/17] KVM: selftests: Add nested VMX APIC cache invalidation test
2026-09-20 20:49 ` [PATCH 12/17] KVM: selftests: Add nested VMX APIC cache invalidation test David Woodhouse
@ 2026-09-20 21:51 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-09-20 21:51 UTC (permalink / raw)
To: David Woodhouse; +Cc: linux-kernel, kvm, linux-rt-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] kvm: selftests: test randomly fails on KVM_EXIT_INTERNAL_ERROR
--
commit 9c12532066400c0055054c635c5df9f04407d1a2
Author: Fred Griffoul <fgriffo@amazon.co.uk>
KVM: selftests: Add nested VMX APIC cache invalidation test
Introduce selftest to verify nested VMX APIC virtualization page cache
invalidation and refresh mechanisms for pfncache implementation.
> diff --git a/tools/testing/selftests/kvm/x86/vmx_apic_update_test.c b/tools/testing/selftests/kvm/x86/vmx_apic_update_test.c
> new file mode 100644
> index 0000000000000..8dd15c92fa906
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86/vmx_apic_update_test.c
[ ... ]
> +static void l1_guest_code(struct vmx_pages *vmx_pages)
> +{
[ ... ]
> +again:
> + exit_reason = vmreadz(VM_EXIT_REASON);
> + if (exit_reason == EXIT_REASON_APIC_ACCESS) {
> + uint64_t guest_rip = vmreadz(GUEST_RIP);
> + uint64_t instr_len = vmreadz(VM_EXIT_INSTRUCTION_LEN);
> +
> + vmwrite(GUEST_RIP, guest_rip + instr_len);
> + GUEST_ASSERT(!vmresume());
[Severity: Medium]
If the VAPIC_SLOT memslot is moved asynchronously and leaves the VAPIC_GPA
unbacked just prior to this VMRESUME call, will KVM treat the unbacked
APIC access page at VM-entry as a misconfiguration and exit to userspace
with KVM_EXIT_INTERNAL_ERROR?
> + goto again;
> + }
[ ... ]
> +static void *update_move_memslot(void *arg)
> +{
> + struct update_control *ctrl = arg;
> + uint64_t gpa = VAPIC_GPA;
> +
> + wait_for_start_signal(ctrl);
> +
> + while (is_running(ctrl)) {
> + usleep(update_period_ms * 1000);
> + gpa += 0x10000;
> + vm_mem_region_move(ctrl->vm, VAPIC_SLOT, gpa);
[Severity: Medium]
Does this background memslot move leave the VAPIC_GPA unbacked precisely
when L1 is about to execute VMRESUME?
> + ctrl->updates++;
> + }
> +
> + return NULL;
> +}
[ ... ]
> +static void run(void * (*update)(void *), const char *name)
> +{
[ ... ]
> + while (!done) {
> + vcpu_run(vcpu);
> +
> + switch (vcpu->run->exit_reason) {
[ ... ]
> + default:
> + TEST_FAIL("%s: Unexpected exit reason: %d (flags 0x%x)",
> + progname,
> + vcpu->run->exit_reason, vcpu->run->flags);
[Severity: Medium]
Does this unconditionally fail the test if it encounters the resulting
KVM_EXIT_INTERNAL_ERROR?
If update_move_memslot() occasionally leaves the VAPIC_GPA unbacked
precisely while L1 handles an EXIT_REASON_APIC_ACCESS, the resulting
KVM_EXIT_INTERNAL_ERROR will cause this test to fail randomly in
CI environments.
> + }
> + }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260920211920.928306-1-dwmw2@infradead.org?part=12
^ permalink raw reply [flat|nested] 27+ messages in thread
* [PATCH 13/17] KVM: x86: Move nested GPC lock helpers to x86.h as kvm_gpc_lock_page()
2026-09-20 20:49 [PATCH 00/17] KVM: Use atomic SRCU for gfn-to-pfn cache, reinstate guest mode for x86 nesting David Woodhouse
` (11 preceding siblings ...)
2026-09-20 20:49 ` [PATCH 12/17] KVM: selftests: Add nested VMX APIC cache invalidation test David Woodhouse
@ 2026-09-20 20:49 ` David Woodhouse
2026-09-20 20:49 ` [PATCH 14/17] KVM: nSVM: Use a gfn_to_pfn_cache for the vmcb12 page David Woodhouse
` (3 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: David Woodhouse @ 2026-09-20 20:49 UTC (permalink / raw)
To: kvm
Cc: linux-kernel, rcu, linux-rt-devel, linux-kselftest,
Paolo Bonzini, Sean Christopherson, Paul Durrant,
Vitaly Kuznetsov, Fred Griffoul, Paul E. McKenney, Kunwu Chan,
Kunwu Chan, Zqiang, Boqun Feng, Neeraj Upadhyay, Joel Fernandes,
Lai Jiangshan, Josh Triplett, Mathieu Desnoyers,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Jason Gunthorpe, Michal Hocko, Nikita Kalyazin, Keir Fraser,
David Matlack, nh-open-source, David Woodhouse
From: David Woodhouse <dwmw@amazon.co.uk>
Move the SRCU lock/activate/retry helpers for accessing a page of L1
guest memory through a gfn_to_pfn_cache from vmx/nested.c to x86.h, so
that nested SVM can use them too.
There is nothing nesting-specific about these helpers: they lock a
gfn_to_pfn_cache as a mapping of a given whole guest page, activating
or re-keying it as required. Name them kvm_gpc_lock_page() and
kvm_gpc_unlock() accordingly; they are candidates for hoisting into
the generic GPC API in kvm_host.h when a non-x86 user appears (the
Hyper-V VP assist and enlightened VMCS pages are likely candidates).
No functional change.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
arch/x86/kvm/vmx/nested.c | 60 +++++++++------------------------------
arch/x86/kvm/x86.h | 33 +++++++++++++++++++++
2 files changed, 47 insertions(+), 46 deletions(-)
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 105ff6cbfe58..b9f12313c960 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -320,38 +320,6 @@ static void vmx_switch_vmcs(struct kvm_vcpu *vcpu, struct loaded_vmcs *vmcs)
kvm_reset_dirty_registers(vcpu);
}
-/*
- * Map a single page of L1 guest memory at @gpa and enter an SRCU read-side
- * critical section protecting the mapping. Returns the SRCU index (>= 0) to
- * pass to nested_gpc_unlock(), or a negative error code. While locked, the
- * cache's khva and pfn are guaranteed to remain valid: an invalidation
- * cannot complete until the reader drains.
- */
-static int nested_gpc_lock(struct gfn_to_pfn_cache *gpc, gpa_t gpa)
-{
- int idx, err;
-
- if (!PAGE_ALIGNED(gpa))
- return -EINVAL;
-retry:
- idx = srcu_read_lock_atomic(&gpc->kvm->gpc_srcu);
- if (!kvm_gpc_check(gpc, PAGE_SIZE) || gpc->gpa != gpa) {
- srcu_read_unlock_atomic(&gpc->kvm->gpc_srcu, idx);
- err = kvm_gpc_activate(gpc, gpa, PAGE_SIZE);
- if (err)
- return err;
-
- goto retry;
- }
-
- return idx;
-}
-
-static void nested_gpc_unlock(struct gfn_to_pfn_cache *gpc, int idx)
-{
- srcu_read_unlock_atomic(&gpc->kvm->gpc_srcu, idx);
-}
-
/*
* Map a page of L1 memory, pin it for direct use by the guest (i.e. by
* the CPU while running the L2 guest, via a physical address in vmcs02),
@@ -366,24 +334,24 @@ static int nested_gpc_hpa(struct gfn_to_pfn_cache *gpc, gpa_t gpa, hpa_t *hpa)
int idx;
do {
- idx = nested_gpc_lock(gpc, gpa);
+ idx = kvm_gpc_lock_page(gpc, gpa);
if (idx < 0)
return idx;
/*
* The pin can fail only if an invalidation cleared the
- * valid bit after nested_gpc_lock() checked it; go back
+ * valid bit after kvm_gpc_lock_page() checked it; go back
* around to refresh (which will not complete until the
* invalidation is over) and try again.
*/
if (kvm_gpc_pin_for_guest(gpc))
break;
- nested_gpc_unlock(gpc, idx);
+ kvm_gpc_unlock(gpc, idx);
} while (1);
*hpa = pfn_to_hpa(gpc->pfn);
- nested_gpc_unlock(gpc, idx);
+ kvm_gpc_unlock(gpc, idx);
return 0;
}
@@ -393,7 +361,7 @@ static int nested_gpc_hpa(struct gfn_to_pfn_cache *gpc, gpa_t gpa, hpa_t *hpa)
* posted interrupts not configured for this L2), -EWOULDBLOCK if it is
* active but currently invalidated (only a sleeping refresh would make
* it usable). Does not pin; the mapping is stable only until
- * nested_gpc_unlock().
+ * kvm_gpc_unlock().
*/
static int nested_gpc_try_lock_if_active(struct gfn_to_pfn_cache *gpc)
{
@@ -890,7 +858,7 @@ static inline bool nested_vmx_prepare_msr_bitmap(struct kvm_vcpu *vcpu,
return true;
}
- idx = nested_gpc_lock(&vmx->nested.msr_bitmap_cache, vmcs12->msr_bitmap);
+ idx = kvm_gpc_lock_page(&vmx->nested.msr_bitmap_cache, vmcs12->msr_bitmap);
if (idx < 0)
return false;
@@ -974,7 +942,7 @@ static inline bool nested_vmx_prepare_msr_bitmap(struct kvm_vcpu *vcpu,
nested_vmx_merge_pmu_msr_bitmaps(vcpu, msr_bitmap_l1, msr_bitmap_l0);
- nested_gpc_unlock(&vmx->nested.msr_bitmap_cache, idx);
+ kvm_gpc_unlock(&vmx->nested.msr_bitmap_cache, idx);
vmx->nested.force_msr_bitmap_recalc = false;
@@ -4286,7 +4254,7 @@ static int vmx_complete_nested_posted_interrupt(struct kvm_vcpu *vcpu)
vmx->nested.pi_pending = false;
if (!pi_test_and_clear_on(pi_desc)) {
- nested_gpc_unlock(&vmx->nested.pi_desc_cache, pi_idx);
+ kvm_gpc_unlock(&vmx->nested.pi_desc_cache, pi_idx);
return 0;
}
@@ -4294,7 +4262,7 @@ static int vmx_complete_nested_posted_interrupt(struct kvm_vcpu *vcpu)
if (max_irr > 0) {
vapic_idx = nested_gpc_lock_if_active(&vmx->nested.virtual_apic_cache);
if (vapic_idx < 0) {
- nested_gpc_unlock(&vmx->nested.pi_desc_cache, pi_idx);
+ kvm_gpc_unlock(&vmx->nested.pi_desc_cache, pi_idx);
goto mmio_needed;
}
vapic_page = vmx->nested.virtual_apic_cache.khva;
@@ -4302,7 +4270,7 @@ static int vmx_complete_nested_posted_interrupt(struct kvm_vcpu *vcpu)
__kvm_apic_update_irr(pi_desc->pir, vapic_page, &max_irr);
kvm_gpc_mark_dirty_in_slot(&vmx->nested.virtual_apic_cache);
- nested_gpc_unlock(&vmx->nested.virtual_apic_cache, vapic_idx);
+ kvm_gpc_unlock(&vmx->nested.virtual_apic_cache, vapic_idx);
status = vmcs_read16(GUEST_INTR_STATUS);
if ((u8)max_irr > ((u8)status & 0xff)) {
@@ -4313,7 +4281,7 @@ static int vmx_complete_nested_posted_interrupt(struct kvm_vcpu *vcpu)
}
kvm_gpc_mark_dirty_in_slot(&vmx->nested.pi_desc_cache);
- nested_gpc_unlock(&vmx->nested.pi_desc_cache, pi_idx);
+ kvm_gpc_unlock(&vmx->nested.pi_desc_cache, pi_idx);
return 0;
mmio_needed:
@@ -4472,7 +4440,7 @@ static bool vmx_has_nested_events(struct kvm_vcpu *vcpu, bool for_injection)
vapic = vmx->nested.virtual_apic_cache.khva;
vppr = *((u32 *)(vapic + APIC_PROCPRI));
- nested_gpc_unlock(&vmx->nested.virtual_apic_cache, idx);
+ kvm_gpc_unlock(&vmx->nested.virtual_apic_cache, idx);
max_irr = vmx_get_rvi();
if ((max_irr & 0xf0) > (vppr & 0xf0))
@@ -4489,12 +4457,12 @@ static bool vmx_has_nested_events(struct kvm_vcpu *vcpu, bool for_injection)
if (pi_test_on(pi_desc)) {
max_irr = pi_find_highest_vector(pi_desc);
if (max_irr > 0 && (max_irr & 0xf0) > (vppr & 0xf0)) {
- nested_gpc_unlock(&vmx->nested.pi_desc_cache, idx);
+ kvm_gpc_unlock(&vmx->nested.pi_desc_cache, idx);
return true;
}
}
- nested_gpc_unlock(&vmx->nested.pi_desc_cache, idx);
+ kvm_gpc_unlock(&vmx->nested.pi_desc_cache, idx);
}
return false;
diff --git a/arch/x86/kvm/x86.h b/arch/x86/kvm/x86.h
index 0f5919b092e4..e3d9889a245d 100644
--- a/arch/x86/kvm/x86.h
+++ b/arch/x86/kvm/x86.h
@@ -14,6 +14,39 @@
#define KVM_MAX_MCE_BANKS 32
+/*
+ * Map a single page of L1 guest memory at @gpa into a gfn_to_pfn_cache
+ * and enter an SRCU read-side critical section protecting the mapping.
+ * Returns the SRCU index (>= 0) to pass to kvm_gpc_unlock(), or a
+ * negative error code. While locked, the cache's khva and pfn are
+ * guaranteed to remain valid: an invalidation cannot complete until the
+ * reader drains.
+ */
+static inline int kvm_gpc_lock_page(struct gfn_to_pfn_cache *gpc, gpa_t gpa)
+{
+ int idx, err;
+
+ if (!PAGE_ALIGNED(gpa))
+ return -EINVAL;
+retry:
+ idx = srcu_read_lock_atomic(&gpc->kvm->gpc_srcu);
+ if (!kvm_gpc_check(gpc, PAGE_SIZE) || gpc->gpa != gpa) {
+ srcu_read_unlock_atomic(&gpc->kvm->gpc_srcu, idx);
+ err = kvm_gpc_activate(gpc, gpa, PAGE_SIZE);
+ if (err)
+ return err;
+
+ goto retry;
+ }
+
+ return idx;
+}
+
+static inline void kvm_gpc_unlock(struct gfn_to_pfn_cache *gpc, int idx)
+{
+ srcu_read_unlock_atomic(&gpc->kvm->gpc_srcu, idx);
+}
+
int kvm_x86_vendor_init(struct kvm_x86_init_ops *ops);
void kvm_x86_vendor_exit(void);
--
2.55.0
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 14/17] KVM: nSVM: Use a gfn_to_pfn_cache for the vmcb12 page
2026-09-20 20:49 [PATCH 00/17] KVM: Use atomic SRCU for gfn-to-pfn cache, reinstate guest mode for x86 nesting David Woodhouse
` (12 preceding siblings ...)
2026-09-20 20:49 ` [PATCH 13/17] KVM: x86: Move nested GPC lock helpers to x86.h as kvm_gpc_lock_page() David Woodhouse
@ 2026-09-20 20:49 ` David Woodhouse
2026-09-20 20:49 ` [PATCH 15/17] KVM: nSVM: Cache L1's MSR permissions map pages David Woodhouse
` (2 subsequent siblings)
16 siblings, 0 replies; 27+ messages in thread
From: David Woodhouse @ 2026-09-20 20:49 UTC (permalink / raw)
To: kvm
Cc: linux-kernel, rcu, linux-rt-devel, linux-kselftest,
Paolo Bonzini, Sean Christopherson, Paul Durrant,
Vitaly Kuznetsov, Fred Griffoul, Paul E. McKenney, Kunwu Chan,
Kunwu Chan, Zqiang, Boqun Feng, Neeraj Upadhyay, Joel Fernandes,
Lai Jiangshan, Josh Triplett, Mathieu Desnoyers,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Jason Gunthorpe, Michal Hocko, Nikita Kalyazin, Keir Fraser,
David Matlack, nh-open-source, David Woodhouse
From: David Woodhouse <dwmw@amazon.co.uk>
Replace the transient map/unmap of the vmcb12 page on every nested
VMRUN (control/save copy-in) and nested VM-exit (state copy-back) with
a gfn_to_pfn_cache keyed on vmcb12_gpa. With unmanaged guest memory
the transient map is a memremap/memunmap cycle on every L1<->L2
transition; the cache persists the kernel mapping for as long as the
gPA and its translation are unchanged.
No pinning is needed, unlike the nVMX APIC pages: SVM never hands an
L1-owned physical address to the CPU in vmcb02 (the merged MSRPM and
the IOPM are kernel-owned pages), so the vmcb12 page is only ever
accessed by KVM itself, under the SRCU read lock, with the usual
check/refresh protocol.
Dirty marking moves from unconditional-at-unmap to the actual write
paths: the SVM_EXIT_ERR reflection in the VMRUN consistency-check
failure path, and the copy-back at nested VM-exit.
The SMM enter/leave and VMLOAD/VMSAVE emulation paths keep the
transient map: they are cold, and VMLOAD/VMSAVE takes an arbitrary
gPA from RAX which would thrash a single-slot cache.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
arch/x86/kvm/svm/nested.c | 26 +++++++++++++++++++-------
arch/x86/kvm/svm/svm.h | 3 +++
2 files changed, 22 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index acc423b13445..a8ee0a03d9f7 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1088,12 +1088,13 @@ static int nested_svm_copy_vmcb12_to_cache(struct kvm_vcpu *vcpu, u64 vmcb12_gpa
struct vcpu_svm *svm = to_svm(vcpu);
struct vmcb *vmcb12;
int r = 0;
+ int idx;
- CLASS(kvm_vcpu_map_local, m)(vcpu, gpa_to_gfn(vmcb12_gpa));
- if (m.ret)
+ idx = kvm_gpc_lock_page(&svm->nested.vmcb12_cache, vmcb12_gpa);
+ if (idx < 0)
return -EFAULT;
- vmcb12 = m.map.hva;
+ vmcb12 = svm->nested.vmcb12_cache.khva;
nested_copy_vmcb_control_to_cache(svm, &vmcb12->control);
nested_copy_vmcb_save_to_cache(svm, &vmcb12->save);
@@ -1104,9 +1105,11 @@ static int nested_svm_copy_vmcb12_to_cache(struct kvm_vcpu *vcpu, u64 vmcb12_gpa
vmcb12->control.event_inj = 0;
vmcb12->control.event_inj_err = 0;
svm_set_gif(svm, false);
+ kvm_gpc_mark_dirty_in_slot(&svm->nested.vmcb12_cache);
r = -EINVAL;
}
+ kvm_gpc_unlock(&svm->nested.vmcb12_cache, idx);
return r;
}
@@ -1251,12 +1254,14 @@ static int nested_svm_vmexit_update_vmcb12(struct kvm_vcpu *vcpu)
struct vcpu_svm *svm = to_svm(vcpu);
struct vmcb *vmcb02 = svm->nested.vmcb02.ptr;
struct vmcb *vmcb12;
+ int idx;
- CLASS(kvm_vcpu_map_local, m)(vcpu, gpa_to_gfn(svm->nested.vmcb12_gpa));
- if (m.ret)
- return m.ret;
+ idx = kvm_gpc_lock_page(&svm->nested.vmcb12_cache,
+ svm->nested.vmcb12_gpa);
+ if (idx < 0)
+ return idx;
- vmcb12 = m.map.hva;
+ vmcb12 = svm->nested.vmcb12_cache.khva;
vmcb12->save.es = vmcb02->save.es;
vmcb12->save.cs = vmcb02->save.cs;
@@ -1311,6 +1316,9 @@ static int nested_svm_vmexit_update_vmcb12(struct kvm_vcpu *vcpu)
vmcb12->control.exit_int_info_err,
KVM_ISA_SVM);
+ kvm_gpc_mark_dirty_in_slot(&svm->nested.vmcb12_cache);
+ kvm_gpc_unlock(&svm->nested.vmcb12_cache, idx);
+
return 0;
}
@@ -1494,6 +1502,8 @@ int svm_allocate_nested(struct vcpu_svm *svm)
if (!svm->nested.msrpm)
goto err_free_vmcb02;
+ kvm_gpc_init(&svm->nested.vmcb12_cache, svm->vcpu.kvm);
+
svm->nested.initialized = true;
return 0;
@@ -1513,6 +1523,8 @@ void svm_free_nested(struct vcpu_svm *svm)
svm_vcpu_free_msrpm(svm->nested.msrpm);
svm->nested.msrpm = NULL;
+ kvm_gpc_deactivate(&svm->nested.vmcb12_cache);
+
__free_page(virt_to_page(svm->nested.vmcb02.ptr));
svm->nested.vmcb02.ptr = NULL;
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index e958943b8162..cd0755919062 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -218,6 +218,9 @@ struct svm_nested_state {
u64 last_vmcb12_gpa;
u64 last_bus_lock_rip;
+ /* Cached mapping of the vmcb12 page, keyed on vmcb12_gpa */
+ struct gfn_to_pfn_cache vmcb12_cache;
+
/*
* The MSR permissions map used for vmcb02, which is the merge result
* of vmcb01 and vmcb12
--
2.55.0
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 15/17] KVM: nSVM: Cache L1's MSR permissions map pages
2026-09-20 20:49 [PATCH 00/17] KVM: Use atomic SRCU for gfn-to-pfn cache, reinstate guest mode for x86 nesting David Woodhouse
` (13 preceding siblings ...)
2026-09-20 20:49 ` [PATCH 14/17] KVM: nSVM: Use a gfn_to_pfn_cache for the vmcb12 page David Woodhouse
@ 2026-09-20 20:49 ` David Woodhouse
2026-09-20 20:49 ` [PATCH 16/17] KVM: nSVM: Cache L1's IO " David Woodhouse
2026-09-20 20:49 ` [PATCH 17/17] KVM: selftests: Add nested transition benchmark David Woodhouse
16 siblings, 0 replies; 27+ messages in thread
From: David Woodhouse @ 2026-09-20 20:49 UTC (permalink / raw)
To: kvm
Cc: linux-kernel, rcu, linux-rt-devel, linux-kselftest,
Paolo Bonzini, Sean Christopherson, Paul Durrant,
Vitaly Kuznetsov, Fred Griffoul, Paul E. McKenney, Kunwu Chan,
Kunwu Chan, Zqiang, Boqun Feng, Neeraj Upadhyay, Joel Fernandes,
Lai Jiangshan, Josh Triplett, Mathieu Desnoyers,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Jason Gunthorpe, Michal Hocko, Nikita Kalyazin, Keir Fraser,
David Matlack, nh-open-source, David Woodhouse
From: David Woodhouse <dwmw@amazon.co.uk>
Reads of L1's MSRPM go through kvm_vcpu_read_guest() on two paths:
the vmcb02 MSRPM merge on nested VMRUN (~10 reads per entry), and the
per-intercept check on every trapped L2 MSR access. With unmanaged
guest memory each read is a memremap/memunmap cycle.
Cache the two pages of L1's MSRPM in a pair of gfn_to_pfn_caches,
keyed on msrpm_base_pa (re-keying automatically when L1 switches to
a different L2 with a different bitmap). Reads preserve the existing
failure semantics: an unreadable bitmap means NESTED_EXIT_DONE
(reflect to L1) on the intercept path and a failed VMRUN on the merge
path.
An 8-byte merge read is naturally aligned and a one-byte intercept
read cannot cross a page, so each access hits exactly one cached page;
a WARN in the accessor enforces that invariant.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
arch/x86/kvm/svm/nested.c | 46 +++++++++++++++++++++++++++++++++------
arch/x86/kvm/svm/svm.h | 7 ++++++
2 files changed, 46 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index a8ee0a03d9f7..07609e4802f9 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -292,6 +292,34 @@ int __init nested_svm_init_msrpm_merge_offsets(void)
* is optimized in that it only merges the parts where KVM MSR permission bitmap
* may contain zero bits.
*/
+/*
+ * Read from L1's MSR permissions map through the per-page caches.
+ * @offset is a byte offset into the (two-page) MSRPM; the read must not
+ * cross a page boundary. msrpm_base_pa was validated page-aligned and
+ * in-bounds by nested_svm_check_controls().
+ */
+static int nested_svm_read_l1_msrpm(struct vcpu_svm *svm, unsigned long offset,
+ void *val, unsigned int len)
+{
+ gpa_t base = svm->nested.ctl.msrpm_base_pa;
+ struct gfn_to_pfn_cache *gpc;
+ int idx;
+
+ if (WARN_ON_ONCE(offset >= MSRPM_SIZE ||
+ offset_in_page(offset) + len > PAGE_SIZE))
+ return -EINVAL;
+
+ gpc = &svm->nested.msrpm12_cache[offset >> PAGE_SHIFT];
+
+ idx = kvm_gpc_lock_page(gpc, base + (offset & PAGE_MASK));
+ if (idx < 0)
+ return idx;
+
+ memcpy(val, gpc->khva + offset_in_page(offset), len);
+ kvm_gpc_unlock(gpc, idx);
+ return 0;
+}
+
static bool nested_svm_merge_msrpm(struct kvm_vcpu *vcpu)
{
struct vcpu_svm *svm = to_svm(vcpu);
@@ -324,11 +352,9 @@ static bool nested_svm_merge_msrpm(struct kvm_vcpu *vcpu)
for (i = 0; i < nested_svm_nr_msrpm_merge_offsets; i++) {
const int p = nested_svm_msrpm_merge_offsets[i];
nsvm_msrpm_merge_t l1_val;
- gpa_t gpa;
- gpa = svm->nested.ctl.msrpm_base_pa + (p * sizeof(l1_val));
-
- if (kvm_vcpu_read_guest(vcpu, gpa, &l1_val, sizeof(l1_val)))
+ if (nested_svm_read_l1_msrpm(svm, p * sizeof(l1_val),
+ &l1_val, sizeof(l1_val)))
return false;
msrpm02[p] = msrpm01[p] | l1_val;
@@ -1488,6 +1514,7 @@ static void nested_svm_triple_fault(struct kvm_vcpu *vcpu)
int svm_allocate_nested(struct vcpu_svm *svm)
{
struct page *vmcb02_page;
+ int i;
if (svm->nested.initialized)
return 0;
@@ -1503,6 +1530,8 @@ int svm_allocate_nested(struct vcpu_svm *svm)
goto err_free_vmcb02;
kvm_gpc_init(&svm->nested.vmcb12_cache, svm->vcpu.kvm);
+ for (i = 0; i < ARRAY_SIZE(svm->nested.msrpm12_cache); i++)
+ kvm_gpc_init(&svm->nested.msrpm12_cache[i], svm->vcpu.kvm);
svm->nested.initialized = true;
return 0;
@@ -1514,6 +1543,8 @@ int svm_allocate_nested(struct vcpu_svm *svm)
void svm_free_nested(struct vcpu_svm *svm)
{
+ int i;
+
if (!svm->nested.initialized)
return;
@@ -1524,6 +1555,8 @@ void svm_free_nested(struct vcpu_svm *svm)
svm->nested.msrpm = NULL;
kvm_gpc_deactivate(&svm->nested.vmcb12_cache);
+ for (i = 0; i < ARRAY_SIZE(svm->nested.msrpm12_cache); i++)
+ kvm_gpc_deactivate(&svm->nested.msrpm12_cache[i]);
__free_page(virt_to_page(svm->nested.vmcb02.ptr));
svm->nested.vmcb02.ptr = NULL;
@@ -1575,7 +1608,6 @@ void svm_leave_nested(struct kvm_vcpu *vcpu)
static int nested_svm_exit_handled_msr(struct vcpu_svm *svm)
{
- gpa_t base = svm->nested.ctl.msrpm_base_pa;
int write, bit_nr;
u8 value, mask;
u32 msr;
@@ -1590,8 +1622,8 @@ static int nested_svm_exit_handled_msr(struct vcpu_svm *svm)
if (bit_nr < 0)
return NESTED_EXIT_DONE;
- if (kvm_vcpu_read_guest(&svm->vcpu, base + bit_nr / BITS_PER_BYTE,
- &value, sizeof(value)))
+ if (nested_svm_read_l1_msrpm(svm, bit_nr / BITS_PER_BYTE,
+ &value, sizeof(value)))
return NESTED_EXIT_DONE;
mask = BIT(write) << (bit_nr & (BITS_PER_BYTE - 1));
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index cd0755919062..ac2f9731bf5d 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -221,6 +221,13 @@ struct svm_nested_state {
/* Cached mapping of the vmcb12 page, keyed on vmcb12_gpa */
struct gfn_to_pfn_cache vmcb12_cache;
+ /*
+ * Cached mappings of the two pages of L1's MSR permissions map,
+ * keyed on ctl.msrpm_base_pa, for the vmcb02 merge and the
+ * per-intercept checks.
+ */
+ struct gfn_to_pfn_cache msrpm12_cache[MSRPM_SIZE / PAGE_SIZE];
+
/*
* The MSR permissions map used for vmcb02, which is the merge result
* of vmcb01 and vmcb12
--
2.55.0
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 16/17] KVM: nSVM: Cache L1's IO permissions map pages
2026-09-20 20:49 [PATCH 00/17] KVM: Use atomic SRCU for gfn-to-pfn cache, reinstate guest mode for x86 nesting David Woodhouse
` (14 preceding siblings ...)
2026-09-20 20:49 ` [PATCH 15/17] KVM: nSVM: Cache L1's MSR permissions map pages David Woodhouse
@ 2026-09-20 20:49 ` David Woodhouse
2026-09-20 20:49 ` [PATCH 17/17] KVM: selftests: Add nested transition benchmark David Woodhouse
16 siblings, 0 replies; 27+ messages in thread
From: David Woodhouse @ 2026-09-20 20:49 UTC (permalink / raw)
To: kvm
Cc: linux-kernel, rcu, linux-rt-devel, linux-kselftest,
Paolo Bonzini, Sean Christopherson, Paul Durrant,
Vitaly Kuznetsov, Fred Griffoul, Paul E. McKenney, Kunwu Chan,
Kunwu Chan, Zqiang, Boqun Feng, Neeraj Upadhyay, Joel Fernandes,
Lai Jiangshan, Josh Triplett, Mathieu Desnoyers,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Jason Gunthorpe, Michal Hocko, Nikita Kalyazin, Keir Fraser,
David Matlack, nh-open-source, David Woodhouse
From: David Woodhouse <dwmw@amazon.co.uk>
The per-intercept IOPM check on every trapped L2 IO instruction reads
L1's IO permissions map with kvm_vcpu_read_guest() — a
memremap/memunmap cycle per access with unmanaged guest memory. Cache
the three IOPM pages in gfn_to_pfn_caches keyed on iopm_base_pa, as
for the MSR permissions map.
Unlike the MSRPM accesses, an IOPM read is not naturally aligned: a
two-byte read for a port range straddling a page boundary (e.g. ports
0x7ff8-0x7fff at bytes 0xfff-0x1000) spans two pages, so read byte by
byte, each from its own page's cache.
Failure semantics are preserved: an unreadable IOPM reflects the
intercept to L1 (NESTED_EXIT_DONE).
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
arch/x86/kvm/svm/nested.c | 33 +++++++++++++++++++++++++++++----
arch/x86/kvm/svm/svm.h | 6 ++++++
2 files changed, 35 insertions(+), 4 deletions(-)
diff --git a/arch/x86/kvm/svm/nested.c b/arch/x86/kvm/svm/nested.c
index 07609e4802f9..0fa3183a509c 100644
--- a/arch/x86/kvm/svm/nested.c
+++ b/arch/x86/kvm/svm/nested.c
@@ -1532,6 +1532,8 @@ int svm_allocate_nested(struct vcpu_svm *svm)
kvm_gpc_init(&svm->nested.vmcb12_cache, svm->vcpu.kvm);
for (i = 0; i < ARRAY_SIZE(svm->nested.msrpm12_cache); i++)
kvm_gpc_init(&svm->nested.msrpm12_cache[i], svm->vcpu.kvm);
+ for (i = 0; i < ARRAY_SIZE(svm->nested.iopm12_cache); i++)
+ kvm_gpc_init(&svm->nested.iopm12_cache[i], svm->vcpu.kvm);
svm->nested.initialized = true;
return 0;
@@ -1557,6 +1559,8 @@ void svm_free_nested(struct vcpu_svm *svm)
kvm_gpc_deactivate(&svm->nested.vmcb12_cache);
for (i = 0; i < ARRAY_SIZE(svm->nested.msrpm12_cache); i++)
kvm_gpc_deactivate(&svm->nested.msrpm12_cache[i]);
+ for (i = 0; i < ARRAY_SIZE(svm->nested.iopm12_cache); i++)
+ kvm_gpc_deactivate(&svm->nested.iopm12_cache[i]);
__free_page(virt_to_page(svm->nested.vmcb02.ptr));
svm->nested.vmcb02.ptr = NULL;
@@ -1635,7 +1639,7 @@ static int nested_svm_intercept_ioio(struct vcpu_svm *svm)
unsigned port, size, iopm_len;
u16 val, mask;
u8 start_bit;
- u64 gpa;
+ int i;
if (!(vmcb12_is_intercept(&svm->nested.ctl, INTERCEPT_IOIO_PROT)))
return NESTED_EXIT_HOST;
@@ -1643,14 +1647,35 @@ static int nested_svm_intercept_ioio(struct vcpu_svm *svm)
port = svm->vmcb->control.exit_info_1 >> 16;
size = (svm->vmcb->control.exit_info_1 & SVM_IOIO_SIZE_MASK) >>
SVM_IOIO_SIZE_SHIFT;
- gpa = svm->nested.ctl.iopm_base_pa + (port / 8);
start_bit = port % 8;
iopm_len = (start_bit + size > 8) ? 2 : 1;
mask = (0xf >> (4 - size)) << start_bit;
val = 0;
- if (kvm_vcpu_read_guest(&svm->vcpu, gpa, &val, iopm_len))
- return NESTED_EXIT_DONE;
+ /*
+ * Read byte by byte: a two-byte read at the end of a page (e.g.
+ * ports 0x7ff8-0x7fff spanning bytes 0xfff-0x1000) crosses into
+ * the next page of the IOPM, i.e. the next cache.
+ */
+ for (i = 0; i < iopm_len; i++) {
+ unsigned long offset = port / 8 + i;
+ struct gfn_to_pfn_cache *gpc;
+ gpa_t gpa;
+ int idx;
+
+ if (WARN_ON_ONCE(offset >= IOPM_SIZE))
+ return NESTED_EXIT_DONE;
+
+ gpc = &svm->nested.iopm12_cache[offset >> PAGE_SHIFT];
+ gpa = svm->nested.ctl.iopm_base_pa + (offset & PAGE_MASK);
+
+ idx = kvm_gpc_lock_page(gpc, gpa);
+ if (idx < 0)
+ return NESTED_EXIT_DONE;
+
+ val |= *(u8 *)(gpc->khva + offset_in_page(offset)) << (i * 8);
+ kvm_gpc_unlock(gpc, idx);
+ }
return (val & mask) ? NESTED_EXIT_DONE : NESTED_EXIT_HOST;
}
diff --git a/arch/x86/kvm/svm/svm.h b/arch/x86/kvm/svm/svm.h
index ac2f9731bf5d..8ccd5a2d942e 100644
--- a/arch/x86/kvm/svm/svm.h
+++ b/arch/x86/kvm/svm/svm.h
@@ -228,6 +228,12 @@ struct svm_nested_state {
*/
struct gfn_to_pfn_cache msrpm12_cache[MSRPM_SIZE / PAGE_SIZE];
+ /*
+ * Cached mappings of the three pages of L1's IO permissions map,
+ * keyed on ctl.iopm_base_pa, for the per-intercept checks.
+ */
+ struct gfn_to_pfn_cache iopm12_cache[IOPM_SIZE / PAGE_SIZE];
+
/*
* The MSR permissions map used for vmcb02, which is the merge result
* of vmcb01 and vmcb12
--
2.55.0
^ permalink raw reply [flat|nested] 27+ messages in thread* [PATCH 17/17] KVM: selftests: Add nested transition benchmark
2026-09-20 20:49 [PATCH 00/17] KVM: Use atomic SRCU for gfn-to-pfn cache, reinstate guest mode for x86 nesting David Woodhouse
` (15 preceding siblings ...)
2026-09-20 20:49 ` [PATCH 16/17] KVM: nSVM: Cache L1's IO " David Woodhouse
@ 2026-09-20 20:49 ` David Woodhouse
2026-09-20 21:52 ` sashiko-bot
16 siblings, 1 reply; 27+ messages in thread
From: David Woodhouse @ 2026-09-20 20:49 UTC (permalink / raw)
To: kvm
Cc: linux-kernel, rcu, linux-rt-devel, linux-kselftest,
Paolo Bonzini, Sean Christopherson, Paul Durrant,
Vitaly Kuznetsov, Fred Griffoul, Paul E. McKenney, Kunwu Chan,
Kunwu Chan, Zqiang, Boqun Feng, Neeraj Upadhyay, Joel Fernandes,
Lai Jiangshan, Josh Triplett, Mathieu Desnoyers,
Sebastian Andrzej Siewior, Clark Williams, Steven Rostedt,
Jason Gunthorpe, Michal Hocko, Nikita Kalyazin, Keir Fraser,
David Matlack, nh-open-source, David Woodhouse
From: David Woodhouse <dwmw@amazon.co.uk>
Measure L1<->L2 transition costs from L1's viewpoint, in TSC cycles:
a reflected-exit phase (cpuid in L2, forwarded to L1: the full
emulated vmexit->vmentry round trip, exercising the vmcb12/vmcs12
access paths), and on SVM an MSR-intercept phase (rdmsr in L2 with
INTERCEPT_MSR_PROT: exercises the per-intercept lookup in L1's MSR
permissions map). Reports min/avg/p50/p99/max over 100k iterations.
Works on both VMX and SVM.
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
Assisted-by: Claude:claude-mythos-5
---
tools/testing/selftests/kvm/Makefile.kvm | 1 +
.../kvm/x86/nested_transition_bench.c | 204 ++++++++++++++++++
2 files changed, 205 insertions(+)
create mode 100644 tools/testing/selftests/kvm/x86/nested_transition_bench.c
diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index b30fa994d5e5..2c5cd619f747 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -156,6 +156,7 @@ TEST_GEN_PROGS_x86 += x86/triple_fault_event_test
TEST_GEN_PROGS_x86 += x86/recalc_apic_map_test
TEST_GEN_PROGS_x86 += x86/aperfmperf_test
TEST_GEN_PROGS_x86 += x86/vmx_apic_update_test
+TEST_GEN_PROGS_x86 += x86/nested_transition_bench
TEST_GEN_PROGS_x86 += access_tracking_perf_test
TEST_GEN_PROGS_x86 += coalesced_io_test
TEST_GEN_PROGS_x86 += dirty_log_perf_test
diff --git a/tools/testing/selftests/kvm/x86/nested_transition_bench.c b/tools/testing/selftests/kvm/x86/nested_transition_bench.c
new file mode 100644
index 000000000000..12fc1ac610e9
--- /dev/null
+++ b/tools/testing/selftests/kvm/x86/nested_transition_bench.c
@@ -0,0 +1,204 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * nested_transition_bench
+ *
+ * Measure nested virtualization transition costs:
+ * 1. L2 exit reflected to L1 (cpuid): full emulated vmexit->vmentry
+ * round trip, exercising the vmcb12/vmcs12 access paths.
+ * 2. L2 MSR access with MSR intercepts enabled: exercises the
+ * per-intercept L1 permissions-bitmap lookup on SVM, and the
+ * bitmap merge path when force_msr_bitmap_recalc is set.
+ *
+ * Reports TSC cycles per iteration (min/avg/p99) from L1's viewpoint.
+ */
+#include <stdio.h>
+#include <stdlib.h>
+
+#include "test_util.h"
+#include "kvm_util.h"
+#include "processor.h"
+#include "vmx.h"
+#include "svm_util.h"
+
+/*
+ * Keep the iteration count modest: every timed iteration includes a
+ * ucall round trip to userspace to report its sample, which on a
+ * debug (KASAN) kernel costs milliseconds. 10k samples still give a
+ * stable p99.
+ */
+#define N_WARMUP 1000
+#define N_ITERS 10000
+
+
+static uint64_t results[N_ITERS];
+
+/* L2: exit to L1 in a tight loop. */
+static void l2_guest_code(void)
+{
+ u32 eax, ebx, ecx, edx;
+
+ for (;;)
+ cpuid(0, &eax, &ebx, &ecx, &edx);
+}
+
+static void l2_guest_code_msr(void)
+{
+ for (;;)
+ rdmsr(MSR_IA32_TSC_DEADLINE);
+}
+
+/* SVM L1: run L2, timing VMRUN -> #VMEXIT round trips. */
+static void l1_svm_code(struct svm_test_data *svm)
+{
+ struct vmcb *vmcb = svm->vmcb;
+ uint64_t t0, t1;
+ int i;
+
+ generic_svm_setup(svm, l2_guest_code);
+ /*
+ * Unlike VMX, where CPUID unconditionally exits, SVM only
+ * intercepts CPUID if asked; without it L0 emulates L2's cpuid
+ * and resumes L2 directly, and run_guest() never returns.
+ */
+ vmcb->control.intercept |= BIT_ULL(INTERCEPT_CPUID) |
+ BIT_ULL(INTERCEPT_MSR_PROT);
+
+ /*
+ * Set the read-intercept bit for MSR_IA32_TSC_DEADLINE (0x6e0,
+ * MSRPM range 0: bit 2*msr, i.e. byte 0x1b8 bit 0) in L1's MSRPM,
+ * so that L2's rdmsr in phase 2 reflects to L1. L0's per-exit
+ * lookup in this bitmap is the path being measured.
+ */
+ ((u8 *)svm->msr)[(MSR_IA32_TSC_DEADLINE & 0x1fff) * 2 / 8] |=
+ BIT((MSR_IA32_TSC_DEADLINE & 0x1fff) * 2 % 8);
+
+ for (i = 0; i < N_WARMUP; i++) {
+ run_guest(vmcb, svm->vmcb_gpa);
+ GUEST_ASSERT(vmcb->control.exit_code == SVM_EXIT_CPUID);
+ vmcb->save.rip += 2; /* cpuid */
+ }
+
+ for (i = 0; i < N_ITERS; i++) {
+ t0 = rdtsc();
+ run_guest(vmcb, svm->vmcb_gpa);
+ t1 = rdtsc();
+ GUEST_ASSERT(vmcb->control.exit_code == SVM_EXIT_CPUID);
+ vmcb->save.rip += 2;
+ GUEST_SYNC1(t1 - t0);
+ }
+
+ /* Phase 2: MSR intercept path */
+ vmcb->save.rip = (u64)l2_guest_code_msr;
+ for (i = 0; i < N_ITERS; i++) {
+ t0 = rdtsc();
+ run_guest(vmcb, svm->vmcb_gpa);
+ t1 = rdtsc();
+ GUEST_ASSERT(vmcb->control.exit_code == SVM_EXIT_MSR);
+ vmcb->save.rip += 2; /* rdmsr */
+ GUEST_SYNC1(t1 - t0);
+ }
+
+ GUEST_DONE();
+}
+
+/* VMX L1: run L2, timing VMRESUME -> vmexit round trips. */
+static void l1_vmx_code(struct vmx_pages *vmx)
+{
+ uint64_t t0, t1;
+ int i;
+
+ GUEST_ASSERT(prepare_for_vmx_operation(vmx));
+ GUEST_ASSERT(load_vmcs(vmx));
+ prepare_vmcs(vmx, l2_guest_code);
+
+ GUEST_ASSERT(!vmlaunch());
+ /* first exit is here; loop resumes */
+ for (i = 0; i < N_WARMUP; i++) {
+ GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_CPUID);
+ vmwrite(GUEST_RIP, vmreadz(GUEST_RIP) +
+ vmreadz(VM_EXIT_INSTRUCTION_LEN));
+ GUEST_ASSERT(!vmresume());
+ }
+
+ for (i = 0; i < N_ITERS; i++) {
+ GUEST_ASSERT(vmreadz(VM_EXIT_REASON) == EXIT_REASON_CPUID);
+ vmwrite(GUEST_RIP, vmreadz(GUEST_RIP) +
+ vmreadz(VM_EXIT_INSTRUCTION_LEN));
+ t0 = rdtsc();
+ GUEST_ASSERT(!vmresume());
+ t1 = rdtsc();
+ GUEST_SYNC1(t1 - t0);
+ }
+
+ GUEST_DONE();
+}
+
+static int cmp_u64(const void *a, const void *b)
+{
+ uint64_t x = *(const uint64_t *)a, y = *(const uint64_t *)b;
+
+ return x < y ? -1 : x > y;
+}
+
+static void report(const char *phase, int n)
+{
+ uint64_t sum = 0;
+ int i;
+
+ qsort(results, n, sizeof(results[0]), cmp_u64);
+ for (i = 0; i < n; i++)
+ sum += results[i];
+
+ printf("%-24s n=%d min=%" PRIu64 " avg=%" PRIu64 " p50=%" PRIu64
+ " p99=%" PRIu64 " max=%" PRIu64 " cycles\n",
+ phase, n, results[0], sum / n, results[n / 2],
+ results[(int)(n * 0.99)], results[n - 1]);
+}
+
+int main(int argc, char *argv[])
+{
+ struct kvm_vcpu *vcpu;
+ struct kvm_vm *vm;
+ struct ucall uc;
+ gva_t nested_gva = 0;
+ bool is_svm = kvm_cpu_has(X86_FEATURE_SVM);
+ int n = 0, phase = 0;
+ bool done = false;
+
+ TEST_REQUIRE(is_svm || kvm_cpu_has(X86_FEATURE_VMX));
+
+ vm = vm_create_with_one_vcpu(&vcpu, is_svm ? (void *)l1_svm_code
+ : (void *)l1_vmx_code);
+ if (is_svm)
+ vcpu_alloc_svm(vm, &nested_gva);
+ else
+ vcpu_alloc_vmx(vm, &nested_gva);
+ vcpu_args_set(vcpu, 1, nested_gva);
+
+ while (!done) {
+ vcpu_run(vcpu);
+ switch (get_ucall(vcpu, &uc)) {
+ case UCALL_SYNC:
+ results[n++] = uc.args[0];
+ if (n == N_ITERS) {
+ report(phase == 0 ? "reflected-exit" :
+ "msr-intercept", n);
+ n = 0;
+ phase++;
+ }
+ break;
+ case UCALL_ABORT:
+ REPORT_GUEST_ASSERT(uc);
+ case UCALL_DONE:
+ if (n)
+ report("partial", n);
+ done = true;
+ break;
+ default:
+ TEST_FAIL("Unknown ucall %lu", uc.cmd);
+ }
+ }
+
+ kvm_vm_free(vm);
+ return 0;
+}
--
2.55.0
^ permalink raw reply [flat|nested] 27+ messages in thread* Re: [PATCH 17/17] KVM: selftests: Add nested transition benchmark
2026-09-20 20:49 ` [PATCH 17/17] KVM: selftests: Add nested transition benchmark David Woodhouse
@ 2026-09-20 21:52 ` sashiko-bot
0 siblings, 0 replies; 27+ messages in thread
From: sashiko-bot @ 2026-09-20 21:52 UTC (permalink / raw)
To: David Woodhouse; +Cc: linux-rt-devel, linux-kernel, kvm
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] kvm: selftests: mismatch between commit message (100k) and code (10k) iterations
--
commit fe906ec5562df5e882aa5dfe67e35c886ebd82f2
Author: David Woodhouse <dwmw@amazon.co.uk>
KVM: selftests: Add nested transition benchmark
This commit adds a benchmark to measure L1 to L2 nested virtualization
transition costs. It tests the reflected-exit phase and an MSR-intercept
phase on SVM, reporting performance metrics from L1's perspective.
> Measure L1<->L2 transition costs from L1's viewpoint, in TSC cycles:
> a reflected-exit phase (cpuid in L2, forwarded to L1: the full
> emulated vmexit->vmentry round trip, exercising the vmcb12/vmcs12
> access paths), and on SVM an MSR-intercept phase (rdmsr in L2 with
> INTERCEPT_MSR_PROT: exercises the per-intercept lookup in L1's MSR
> permissions map). Reports min/avg/p50/p99/max over 100k iterations.
[Severity: Medium]
Is the iteration count in the commit message accurate? The message states
the benchmark reports over 100k iterations, but the code defines N_ITERS
as 10000 and explicitly notes that 10k samples is sufficient:
> diff --git a/tools/testing/selftests/kvm/x86/nested_transition_bench.c b/tools/testing/selftests/kvm/x86/nested_transition_bench.c
> --- /dev/null
> +++ b/tools/testing/selftests/kvm/x86/nested_transition_bench.c
[ ... ]
> +/*
> + * Keep the iteration count modest: every timed iteration includes a
> + * ucall round trip to userspace to report its sample, which on a
> + * debug (KASAN) kernel costs milliseconds. 10k samples still give a
> + * stable p99.
> + */
> +#define N_WARMUP 1000
> +#define N_ITERS 10000
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260920211920.928306-1-dwmw2@infradead.org?part=17
^ permalink raw reply [flat|nested] 27+ messages in thread