From nobody Tue Aug 25 15:23:29 2026 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 502D525B0A4; Tue, 11 Aug 2026 13:22:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.50.34 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786454579; cv=none; b=EoDsY6Q30zYeuuf+hJz+sIGX6FfSuA9rTb6Orof/ZathMhcY8belTl4U06+AJQc66uKz4lu2SaNsSBrs7qtVGhqUjp1XCbD7HexaMmtXE9C3LTBikGpOPmmN/M72TyeQZCmhO3ttorvKxSVVJztSXxLxFCb6+jbtQcCH3DBn0L0= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786454579; c=relaxed/simple; bh=Zbbl65jsc0Rozb625eLnOJA6u2FHHxLI1vUcMFlB6wM=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=bm4/deCnVdB7itWMQXDudYq2x5wYbUZ93WeqotNZ58BpUSRZ6OjmgTYnFLMmSSUxwoWX+CC737hH+JsL4tiMbpaZQa3fsPZEdB1pXGtkXhlTZuTPgZ05cJtAgcoNWCTWpj3mwggtOSLBzSK9S+5wcMjFfOJ0fs46YFvNuiwTp9w= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=casper.srs.infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=U6qKGE1m; arc=none smtp.client-ip=90.155.50.34 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=casper.srs.infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="U6qKGE1m" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:MIME-Version:References:In-Reply-To:Message-ID:Date:Subject:Cc: To:From:Reply-To:Content-ID:Content-Description; bh=ABcFmVdcPBMLSSU+PCVDF9mBvSQNBpyhK2aPZDBr9Tg=; b=U6qKGE1mn/sgyPvbNdH5eayYML zYgYbUw802Rop7JeROoQPmt3F+x5o+8gzWBEz5qYxR7QRNrKNFuwW3WuUGMwWA45WDjewz3iUmYWX pYba5AxlJZya25NwoPcCSmERGF6Rz3OMl7UDieStfCEJHFCpV6ns7H+VVOCNK4TN70QubX56Nd7K1 oi4p5amSlsKHWHvu7D3INph987e/M874L6IrIe5GnRZ269qmBLJf7sv5SfKNWnGBaW9Uk0QhOqenJ GBxUNHxMi7HM1M1ylU2GmPY43SecbJlqpbOU6Te+M6Q2+dyvGQUEzTvjMcmlQO/gA5YMS1l6N7KcC r3fXvfUw==; Received: from [2001:8b0:10b:1::425] (helo=i7.infradead.org) by casper.infradead.org with esmtpsa (Exim 4.99.1 #2 (Red Hat Linux)) id 1wtmRC-00000000TZ0-24VH; Tue, 11 Aug 2026 13:22:39 +0000 Received: from dwoodhou by i7.infradead.org with local (Exim 4.99.4 #2 (Red Hat Linux)) id 1wtmRC-00000000Qle-13EQ; Tue, 11 Aug 2026 14:22:38 +0100 From: David Woodhouse To: seanjc@google.com, pbonzini@redhat.com Cc: dwmw2@infradead.org, paul@xen.org, joao.m.martins@oracle.com, boris.ostrovsky@oracle.com, ankur.a.arora@oracle.com, stollmc@amazon.com, paulmck@kernel.org, boqun.feng@gmail.com, urezki@gmail.com, akpm@linux-foundation.org, mhocko@suse.com, bigeasy@linutronix.de, syzbot+919877893c9d28162dc2@syzkaller.appspotmail.com, syzbot+208f7f3e5f59c11aeb90@syzkaller.appspotmail.com, syzkaller-bugs@googlegroups.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v4 1/6] KVM: pfncache: Use SRCU for readers instead of a rwlock Date: Tue, 11 Aug 2026 13:49:51 +0100 Message-ID: <20260811132237.102400-2-dwmw2@infradead.org> X-Mailer: git-send-email 2.55.0 In-Reply-To: <20260811132237.102400-1-dwmw2@infradead.org> References: <20260811132237.102400-1-dwmw2@infradead.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Sender: David Woodhouse X-SRS-Rewrite: SMTP reverse-path rewritten from by casper.infradead.org. See http://www.infradead.org/rpr.html From: David Woodhouse Replace the per-cache rwlock with a dedicated per-VM SRCU domain (kvm->gpc_srcu) for the read side. Readers now run under srcu_read_lock() alone, which works in any context (including hardirq and sched-out paths) and never fails or spins =E2=80=94 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. 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 =E2=80=94 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 =E2=80=94 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() 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 =E2=80=94 including in the window after the last retry check =E2=80= =94 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 =3D> uHVA translation has not changed, and GPC_VALID already asserts that the uHVA =3D> 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 even= t channel delivery") Reported-by: syzbot+208f7f3e5f59c11aeb90@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=3D208f7f3e5f59c11aeb90 Reported-by: syzbot+919877893c9d28162dc2@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=3D919877893c9d28162dc2 Tested-by: syzbot+919877893c9d28162dc2@syzkaller.appspotmail.com Signed-off-by: David Woodhouse 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 +++- virt/kvm/kvm_main.c | 9 + virt/kvm/pfncache.c | 423 +++++++++++++++++++++++++++++--------- 6 files changed, 431 insertions(+), 184 deletions(-) diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index d94b59140c45..75e5f894153f 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; =20 memcpy(&hv_clock, ref_hv_clock, sizeof(hv_clock)); =20 - read_lock_irqsave(&gpc->lock, flags); + idx =3D srcu_read_lock(&vcpu->kvm->gpc_srcu); while (!kvm_gpc_check(gpc, offset + sizeof(*guest_hv_clock))) { - read_unlock_irqrestore(&gpc->lock, flags); + srcu_read_unlock(&vcpu->kvm->gpc_srcu, idx); =20 if (kvm_gpc_refresh(gpc, offset + sizeof(*guest_hv_clock))) return; =20 - read_lock_irqsave(&gpc->lock, flags); + idx =3D srcu_read_lock(&vcpu->kvm->gpc_srcu); } =20 guest_hv_clock =3D (void *)(gpc->khva + offset); @@ -1755,7 +1755,7 @@ static void kvm_setup_guest_pvclock(struct pvclock_vc= pu_time_info *ref_hv_clock, guest_hv_clock->version =3D ++hv_clock.version; =20 kvm_gpc_mark_dirty_in_slot(gpc); - read_unlock_irqrestore(&gpc->lock, flags); + srcu_read_unlock(&vcpu->kvm->gpc_srcu, idx); =20 trace_kvm_pvclock_update(vcpu->vcpu_id, &hv_clock); } diff --git a/arch/x86/kvm/xen.c b/arch/x86/kvm/xen.c index ff55ff290afb..179efcfb40e3 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 =3D 0; int idx =3D srcu_read_lock(&kvm->srcu); + int gpc_idx; =20 - read_lock_irq(&gpc->lock); + gpc_idx =3D srcu_read_lock(&kvm->gpc_srcu); while (!kvm_gpc_check(gpc, PAGE_SIZE)) { - read_unlock_irq(&gpc->lock); + srcu_read_unlock(&kvm->gpc_srcu, gpc_idx); =20 ret =3D kvm_gpc_refresh(gpc, PAGE_SIZE); if (ret) goto out; =20 - read_lock_irq(&gpc->lock); + gpc_idx =3D srcu_read_lock(&kvm->gpc_srcu); } =20 /* @@ -97,7 +98,7 @@ static int kvm_xen_shared_info_init(struct kvm *kvm) smp_wmb(); =20 wc->version =3D wc_version + 1; - read_unlock_irq(&gpc->lock); + srcu_read_unlock(&kvm->gpc_srcu, gpc_idx); =20 out: srcu_read_unlock(&kvm->srcu, idx); @@ -154,22 +155,21 @@ static int xen_get_guest_pvclock(struct kvm_vcpu *vcp= u, struct gfn_to_pfn_cache *gpc, unsigned int offset) { - unsigned long flags; - int r; + int idx, r; =20 - read_lock_irqsave(&gpc->lock, flags); + idx =3D srcu_read_lock(&vcpu->kvm->gpc_srcu); while (!kvm_gpc_check(gpc, offset + sizeof(*hv_clock))) { - read_unlock_irqrestore(&gpc->lock, flags); + srcu_read_unlock(&vcpu->kvm->gpc_srcu, idx); =20 r =3D kvm_gpc_refresh(gpc, offset + sizeof(*hv_clock)); if (r) return r; =20 - read_lock_irqsave(&gpc->lock, flags); + idx =3D srcu_read_lock(&vcpu->kvm->gpc_srcu); } =20 memcpy(hv_clock, gpc->khva + offset, sizeof(*hv_clock)); - read_unlock_irqrestore(&gpc->lock, flags); + srcu_read_unlock(&vcpu->kvm->gpc_srcu, idx); =20 /* * 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 =3D &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 =3D NULL; uint64_t entry_time; uint64_t *rs_times; int *rs_state; + int idx; =20 /* * 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) =20 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 =3D srcu_read_lock(&v->kvm->gpc_srcu); while (!kvm_gpc_check(gpc1, user_len1)) { - read_unlock_irqrestore(&gpc1->lock, flags); + srcu_read_unlock(&v->kvm->gpc_srcu, idx); =20 /* When invoked from kvm_sched_out() we cannot sleep */ if (atomic) @@ -438,7 +430,7 @@ static void kvm_xen_update_runstate_guest(struct kvm_vc= pu *v, bool atomic) if (kvm_gpc_refresh(gpc1, user_len1)) return; =20 - read_lock_irqsave(&gpc1->lock, flags); + idx =3D srcu_read_lock(&v->kvm->gpc_srcu); } =20 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(&v->kvm->gpc_srcu, idx); =20 /* 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(); } =20 - if (user_len2) { + if (user_len2) kvm_gpc_mark_dirty_in_slot(gpc2); - read_unlock(&gpc2->lock); - } =20 kvm_gpc_mark_dirty_in_slot(gpc1); - read_unlock_irqrestore(&gpc1->lock, flags); + srcu_read_unlock(&v->kvm->gpc_srcu, idx); } =20 void kvm_xen_update_runstate(struct kvm_vcpu *v, int state) @@ -639,7 +616,7 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v) { unsigned long evtchn_pending_sel =3D READ_ONCE(v->arch.xen.evtchn_pending= _sel); struct gfn_to_pfn_cache *gpc =3D &v->arch.xen.vcpu_info_cache; - unsigned long flags; + int idx; =20 if (!evtchn_pending_sel) return; @@ -649,14 +626,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 =3D srcu_read_lock(&v->kvm->gpc_srcu); while (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) { - read_unlock_irqrestore(&gpc->lock, flags); + srcu_read_unlock(&v->kvm->gpc_srcu, idx); =20 if (kvm_gpc_refresh(gpc, sizeof(struct vcpu_info))) return; =20 - read_lock_irqsave(&gpc->lock, flags); + idx =3D srcu_read_lock(&v->kvm->gpc_srcu); } =20 /* Now gpc->khva is a valid kernel address for the vcpu_info */ @@ -690,7 +667,7 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v) } =20 kvm_gpc_mark_dirty_in_slot(gpc); - read_unlock_irqrestore(&gpc->lock, flags); + srcu_read_unlock(&v->kvm->gpc_srcu, idx); =20 /* For the per-vCPU lapic vector, deliver it as MSI. */ if (v->arch.xen.upcall_vector) @@ -700,8 +677,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 =3D &v->arch.xen.vcpu_info_cache; - unsigned long flags; u8 rc =3D 0; + int idx; =20 /* * If the global upcall vector (HVMIRQ_callback_vector) is set and @@ -726,9 +703,9 @@ int __kvm_xen_has_interrupt(struct kvm_vcpu *v) */ guard(srcu)(&v->kvm->srcu); =20 - read_lock_irqsave(&gpc->lock, flags); + idx =3D srcu_read_lock(&v->kvm->gpc_srcu); while (!kvm_gpc_check(gpc, sizeof(struct vcpu_info))) { - read_unlock_irqrestore(&gpc->lock, flags); + srcu_read_unlock(&v->kvm->gpc_srcu, idx); =20 /* * This function gets called from kvm_vcpu_block() after setting the @@ -748,11 +725,11 @@ int __kvm_xen_has_interrupt(struct kvm_vcpu *v) */ return 0; } - read_lock_irqsave(&gpc->lock, flags); + idx =3D srcu_read_lock(&v->kvm->gpc_srcu); } =20 rc =3D ((struct vcpu_info *)gpc->khva)->evtchn_upcall_pending; - read_unlock_irqrestore(&gpc->lock, flags); + srcu_read_unlock(&v->kvm->gpc_srcu, idx); return rc; } =20 @@ -1464,14 +1441,13 @@ static bool wait_pending_event(struct kvm_vcpu *vcp= u, bool has_64bit_shinfo, struct kvm *kvm =3D vcpu->kvm; struct gfn_to_pfn_cache *gpc =3D &kvm->arch.xen.shinfo_cache; unsigned long *pending_bits; - unsigned long flags; bool ret =3D true; - int idx, i; + int idx, gpc_idx, i; =20 idx =3D srcu_read_lock(&kvm->srcu); - read_lock_irqsave(&gpc->lock, flags); + gpc_idx =3D srcu_read_lock(&kvm->gpc_srcu); if (!kvm_gpc_check(gpc, PAGE_SIZE)) - goto out_rcu; + goto out_gpc_srcu; =20 ret =3D false; if (has_64bit_shinfo) { @@ -1489,8 +1465,8 @@ static bool wait_pending_event(struct kvm_vcpu *vcpu,= bool has_64bit_shinfo, } } =20 - out_rcu: - read_unlock_irqrestore(&gpc->lock, flags); + out_gpc_srcu: + srcu_read_unlock(&kvm->gpc_srcu, gpc_idx); srcu_read_unlock(&kvm->srcu, idx); =20 return ret; @@ -1831,10 +1807,9 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *x= e, struct kvm *kvm) bool has_64bit_shinfo =3D 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 =3D false; - int vcpu_idx, idx, rc; + int vcpu_idx, idx, gpc_idx, rc; =20 vcpu_idx =3D READ_ONCE(xe->vcpu_idx); if (vcpu_idx >=3D 0) @@ -1853,9 +1828,9 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe= , struct kvm *kvm) =20 idx =3D srcu_read_lock(&kvm->srcu); =20 - read_lock_irqsave(&gpc->lock, flags); + gpc_idx =3D srcu_read_lock(&kvm->gpc_srcu); if (!kvm_gpc_check(gpc, PAGE_SIZE)) - goto out_rcu; + goto out_gpc_srcu; =20 if (has_64bit_shinfo) { struct shared_info *shinfo =3D gpc->khva; @@ -1893,10 +1868,8 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *x= e, struct kvm *kvm) =20 rc =3D 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 =3D &vcpu->arch.xen.vcpu_info_cache; =20 - 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 @@ -1904,7 +1877,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 =3D true; - goto out_rcu; + goto out_gpc_srcu; } =20 /* @@ -1939,8 +1912,8 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *xe= , struct kvm *kvm) } } =20 - out_rcu: - read_unlock_irqrestore(&gpc->lock, flags); + out_gpc_srcu: + srcu_read_unlock(&kvm->gpc_srcu, gpc_idx); srcu_read_unlock(&kvm->srcu, idx); =20 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; =20 /* * created_vcpus is protected by kvm->lock, and is incremented @@ -1510,12 +1519,9 @@ int kvm_gpc_activate_hva(struct gfn_to_pfn_cache *gp= c, 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 t= he - * 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 functi= on + * 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); =20 @@ -1532,8 +1538,8 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsi= gned 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); =20 @@ -1974,7 +1980,8 @@ static inline bool kvm_is_gpa_in_memslot(struct kvm *= kvm, gpa_t gpa) =20 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"); =20 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 =3D> 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; }; =20 +/* 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/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c index 41c88a8ade95..072deea2f33c 100644 --- a/virt/kvm/kvm_main.c +++ b/virt/kvm/kvm_main.c @@ -1151,6 +1151,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(&kvm->gpc_srcu)) + goto out_err_no_gpc_srcu; =20 r =3D kvm_init_irq_routing(kvm); if (r) @@ -1231,6 +1233,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); @@ -1307,6 +1311,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..a6a7cfd13246 100644 --- a/virt/kvm/pfncache.c +++ b/virt/kvm/pfncache.c @@ -26,35 +26,114 @@ void gfn_to_pfn_cache_invalidate_start(struct kvm *kvm= , unsigned long start, unsigned long end) { struct gfn_to_pfn_cache *gpc; + bool cleared =3D false; =20 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 =E2=80=94 after its xchg, before its announce =E2=80= =94 + * 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; =20 - /* Only a single page so no need to care about length */ - if (gpc->valid && !is_error_noslot_pfn(gpc->pfn) && - gpc->uhva >=3D start && gpc->uhva < end) { - read_unlock_irq(&gpc->lock); + old =3D atomic_read_acquire(&gpc->state); + if (!(old & (GPC_VALID | GPC_BECOMING_VALID))) + continue; =20 + gpc_uhva =3D READ_ONCE(gpc->uhva); + if (gpc_uhva >=3D 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 >=3D start && gpc->uhva < end) - gpc->valid =3D false; - write_unlock_irq(&gpc->lock); - continue; + do { + new =3D old & ~(GPC_VALID | GPC_BECOMING_VALID); + if (old & GPC_VALID) + new |=3D GPC_INVALIDATING; + } while (!atomic_try_cmpxchg(&gpc->state, &old, new)); + + if (old & GPC_VALID) + cleared =3D 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. + * + * XXX: unlike synchronize_rcu(), synchronize_srcu() can complete + * only once srcu_gp_end() has run, and on the one-time + * SRCU_SIZE_ALLOC =3D> SRCU_SIZE_BIG transition that path performs a + * GFP_KERNEL allocation (init_srcu_struct_nodes()), which may enter + * direct reclaim. That would make the OOM reaper wait on reclaim. + * The transition is pre-done at init when nr_cpu_ids >=3D big_cpu_lim + * (128), but is contention-triggered below that, so this is not yet + * safe on smaller hosts without an SRCU-side change (a per-struct + * "size at init" flag, or GFP_NOWAIT in the upgrade). + */ + if (cleared) + synchronize_srcu(&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 =E2=80=94 and which therefore performs its o= wn + * wait, necessarily beginning after the VALID clear which set the + * marker =E2=80=94 can meaningfully clear it. Readers ignore the bit; if= no + * mutation ever follows, it sits there harmlessly. + */ } =20 static bool kvm_gpc_is_valid_len(gpa_t gpa, unsigned long uhva, @@ -74,6 +153,22 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsign= ed long len) { struct kvm_memslots *slots =3D kvm_memslots(gpc->kvm); =20 + 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 =E2=80=94 and any use + * of gpc->khva by the caller =E2=80=94 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; =20 @@ -81,7 +176,8 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigne= d 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 !=3D slots->generation) + if (!kvm_is_error_gpa(gpc->gpa) && + READ_ONCE(gpc->generation) !=3D slots->generation) return false; =20 if (kvm_is_error_hva(gpc->uhva)) @@ -90,9 +186,6 @@ bool kvm_gpc_check(struct gfn_to_pfn_cache *gpc, unsigne= d long len) if (!kvm_gpc_is_valid_len(gpc->gpa, gpc->uhva, len)) return false; =20 - if (!gpc->valid) - return false; - return true; } =20 @@ -134,8 +227,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 +249,13 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_c= ache *gpc) { /* Note, the new page offset may be different than the old! */ void *old_khva =3D (void *)PAGE_ALIGN_DOWN((uintptr_t)gpc->khva); + void *old_khva_exact =3D gpc->khva; + kvm_pfn_t old_pfn =3D gpc->pfn; kvm_pfn_t new_pfn =3D KVM_PFN_ERR_FAULT; void *new_khva =3D NULL; unsigned long gpc_seq; struct page *page; + int state; =20 struct kvm_follow_pfn kfp =3D { .slot =3D gpc->memslot, @@ -171,21 +267,32 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_c= ache *gpc) =20 lockdep_assert_held(&gpc->refresh_lock); =20 - lockdep_assert_held_write(&gpc->lock); - - /* - * Invalidate the cache prior to dropping gpc->lock, the gpa=3D>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 =3D false; + WARN_ON_ONCE(atomic_read(&gpc->state) & GPC_VALID); =20 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 =3D gpc->kvm->gpc_invalidate_seq; smp_rmb(); =20 - 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 +320,15 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_ca= che *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 =3D=3D 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 =3D=3D old_pfn) new_khva =3D old_khva; else new_khva =3D gpc_map(new_pfn); @@ -225,18 +338,39 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_c= ache *gpc) goto out_error; } =20 - write_lock_irq(&gpc->lock); + if (mmu_notifier_retry_cache(gpc->kvm, gpc_seq)) + continue; + + gpc->pfn =3D new_pfn; + gpc->khva =3D new_khva + offset_in_page(gpc->uhva); =20 /* - * Other tasks must wait for _this_ refresh to complete before - * attempting to refresh. + * Publish: BECOMING_VALID =3D> VALID, but only if the bit is + * still set. If an invalidation of this uhva landed at any + * point since it was set =E2=80=94 including in the window after the + * retry check just above =E2=80=94 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 =3D GPC_BECOMING_VALID; + if (atomic_try_cmpxchg_release(&gpc->state, &state, GPC_VALID)) + break; =20 - gpc->valid =3D true; - gpc->pfn =3D new_pfn; - gpc->khva =3D 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 =E2=80=94 + * restore the published values. This runs under refresh_lock + * with the cache unpublished, so it is single-writer plain + * stores. + */ + gpc->pfn =3D old_pfn; + gpc->khva =3D old_khva_exact; + } while (1); =20 /* * Put the reference to the _new_ page. The page is now tracked by the @@ -248,8 +382,19 @@ static kvm_pfn_t hva_to_pfn_retry(struct gfn_to_pfn_ca= che *gpc) return 0; =20 out_error: - write_lock_irq(&gpc->lock); - + /* + * Withdraw the announcement: no publish is coming. Leaving the bit + * set would be harmless =E2=80=94 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 =E2=80=94 + * 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; } =20 @@ -259,7 +404,7 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache *g= pc, gpa_t gpa, unsigned l bool unmap_old =3D false; unsigned long old_uhva; kvm_pfn_t old_pfn; - bool hva_change =3D false; + bool must_drain; void *old_khva; int ret; =20 @@ -269,13 +414,94 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache = *gpc, gpa_t gpa, unsigned l =20 lockdep_assert_held(&gpc->refresh_lock); =20 - write_lock_irq(&gpc->lock); + if (!gpc->active) + return -EINVAL; =20 - if (!gpc->active) { - ret =3D -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 =3D> uHVA translation is + * unchanged, and GPC_VALID being set is precisely the assertion that + * the second stage (uHVA =3D> 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 <=3D 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 =3D gpc->memslot; + unsigned long new_uhva =3D KVM_HVA_ERR_BAD; + u64 new_generation =3D gpc->generation; + + if (kvm_is_error_gpa(gpa)) { + new_uhva =3D uhva; + } else { + struct kvm_memslots *slots =3D kvm_memslots(gpc->kvm); + gfn_t gfn =3D gpa_to_gfn(gpa); + + new_generation =3D slots->generation; + new_slot =3D __gfn_to_memslot(slots, gfn); + if (new_slot) + new_uhva =3D gfn_to_hva_memslot(new_slot, gfn) + + offset_in_page(gpa); + } + + if (!kvm_is_error_hva(new_uhva) && new_uhva =3D=3D gpc->uhva && + new_slot =3D=3D gpc->memslot && gpa =3D=3D 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 !=3D gpc->generation) + WRITE_ONCE(gpc->generation, new_generation); + + return 0; + } } =20 + /* + * 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 =E2=80=94 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 synchronize_srcu() 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 =3D atomic_xchg(&gpc->state, 0) & + (GPC_VALID | GPC_INVALIDATING); + if (must_drain) + synchronize_srcu(&gpc->kvm->gpc_srcu); + old_pfn =3D gpc->pfn; old_khva =3D (void *)PAGE_ALIGN_DOWN((uintptr_t)gpc->khva); old_uhva =3D PAGE_ALIGN_DOWN(gpc->uhva); @@ -285,10 +511,14 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache = *gpc, gpa_t gpa, unsigned l =20 gpc->gpa =3D INVALID_GPA; gpc->memslot =3D NULL; - gpc->uhva =3D PAGE_ALIGN_DOWN(uhva); - - if (gpc->uhva !=3D old_uhva) - hva_change =3D 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 =3D kvm_memslots(gpc->kvm); =20 @@ -301,43 +531,33 @@ static int __kvm_gpc_refresh(struct gfn_to_pfn_cache = *gpc, gpa_t gpa, unsigned l gpc->gpa =3D gpa; gpc->generation =3D slots->generation; gpc->memslot =3D __gfn_to_memslot(slots, gfn); - gpc->uhva =3D gfn_to_hva_memslot(gpc->memslot, gfn); + WRITE_ONCE(gpc->uhva, gfn_to_hva_memslot(gpc->memslot, gfn)); =20 if (kvm_is_error_hva(gpc->uhva)) { ret =3D -EFAULT; goto out; } =20 - /* - * Even if the GPA and/or the memslot generation changed, the - * HVA may still be the same. - */ - if (gpc->uhva !=3D old_uhva) - hva_change =3D true; + } else { - gpc->uhva =3D old_uhva; + WRITE_ONCE(gpc->uhva, old_uhva); } } =20 /* Note: the offset must be correct before calling hva_to_pfn_retry() */ - gpc->uhva +=3D page_offset; + WRITE_ONCE(gpc->uhva, gpc->uhva + page_offset); =20 /* - * 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 =3D hva_to_pfn_retry(gpc); - } else { - /* - * If the HVA=E2=86=92PFN mapping was already valid, don't unmap it. - * But do update gpc->khva because the offset within the page - * may have changed. - */ - gpc->khva =3D old_khva + page_offset; - ret =3D 0; - goto out_unlock; - } + ret =3D hva_to_pfn_retry(gpc); =20 out: /* @@ -346,17 +566,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 =3D false; + WARN_ON_ONCE(atomic_read(&gpc->state) & GPC_VALID); gpc->pfn =3D KVM_PFN_ERR_FAULT; gpc->khva =3D NULL; } =20 - /* Detect a pfn change before dropping the lock! */ unmap_old =3D (old_pfn !=3D gpc->pfn); - -out_unlock: - write_unlock_irq(&gpc->lock); - if (unmap_old) gpc_unmap(old_pfn, old_khva); =20 @@ -384,14 +599,14 @@ int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, uns= igned long len) =20 void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct kvm *kvm) { - rwlock_init(&gpc->lock); mutex_init(&gpc->refresh_lock); =20 gpc->kvm =3D kvm; gpc->pfn =3D KVM_PFN_ERR_FAULT; gpc->gpa =3D INVALID_GPA; gpc->uhva =3D KVM_HVA_ERR_BAD; - gpc->active =3D gpc->valid =3D false; + gpc->active =3D false; + atomic_set(&gpc->state, 0); } =20 static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, uns= igned long uhva, @@ -405,7 +620,7 @@ static int __kvm_gpc_activate(struct gfn_to_pfn_cache *= gpc, gpa_t gpa, unsigned guard(mutex)(&gpc->refresh_lock); =20 if (!gpc->active) { - if (KVM_BUG_ON(gpc->valid, kvm)) + if (KVM_BUG_ON(atomic_read(&gpc->state) & GPC_VALID, kvm)) return -EIO; =20 spin_lock(&kvm->gpc_lock); @@ -415,11 +630,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 =3D true; - write_unlock_irq(&gpc->lock); + WRITE_ONCE(gpc->active, true); } return __kvm_gpc_refresh(gpc, gpa, uhva); } @@ -454,13 +669,24 @@ void kvm_gpc_deactivate(struct gfn_to_pfn_cache *gpc) =20 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 =E2=80=94 it left GPC_INVALIDATING set to say + * so =E2=80=94 and the unmap below must not run under those readers. */ - write_lock_irq(&gpc->lock); - gpc->active =3D false; - gpc->valid =3D false; + if (atomic_xchg(&gpc->state, 0) & + (GPC_VALID | GPC_INVALIDATING)) + synchronize_srcu(&kvm->gpc_srcu); =20 /* * Leave the GPA =3D> uHVA cache intact, it's protected by the @@ -473,7 +699,6 @@ void kvm_gpc_deactivate(struct gfn_to_pfn_cache *gpc) =20 old_pfn =3D gpc->pfn; gpc->pfn =3D KVM_PFN_ERR_FAULT; - write_unlock_irq(&gpc->lock); =20 spin_lock(&kvm->gpc_lock); list_del(&gpc->list); base-commit: 7b01d807b50a88c5bfc448286ac37e04e508b9c5 prerequisite-patch-id: 1712dde336c201759f33405c92204f91002b6d13 --=20 2.55.0 From nobody Tue Aug 25 15:23:29 2026 Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6FB1732143D; Tue, 11 Aug 2026 13:22:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.92.199 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786454575; cv=none; b=L2Z3xXRGQ/xL/JkFr2XYLuJm6VLm86Gv521JcySpXnOTO5l4EwCRK58BUpvR2pkbv6UJxsrsYvMbiUTfOesoNhUUBMEMaOlXSTpZC7iNPXgBXtpba3nTansYMF3tCUwZhZEtPcvG6B/c+AHqrnqZcOa0lMVs+969rseODa/pv1s= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786454575; c=relaxed/simple; bh=PoBwmxUowFVvctphMHiU1502xvxhZ9LDsOwk5YVbsOw=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=asGofc3O9P/SKEKJstADP/TdGISfllslIWv8PYE6V6aL1jOvJaa1SeEhplCc+vjY03I0vG52eSA4aXtrg56nzlJXelVXyb9LsQJvUJHKn1la/2SVRMueiOBfspORjOxXfICbWAF3K2IfRuPS3bHAsvX1Dt+bJTv0KROhlrYbvRE= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=desiato.srs.infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=Ub3kgyQP; arc=none smtp.client-ip=90.155.92.199 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=desiato.srs.infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="Ub3kgyQP" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding: Content-Type:MIME-Version:References:In-Reply-To:Message-ID:Date:Subject:Cc: To:From:Reply-To:Content-ID:Content-Description; bh=V29VnP0TGuLRIu0HX9T6++rQY3jn2ihSNDPJPUPtEnY=; b=Ub3kgyQPjHN45fTjoI47aKToqW q4VMXQgx/XX4ysFnER+W6lDQujxtMo7q35lNhuWlSDoHcbtpU5KWVjUZZ1h2aob7JFwDyMFpVkUVy RKigYL1x9cB/MDJVnSMKlFUACmbqubI0bUVbC44MbYuemuZKE7TBZfNkD791WVbcpMDeoRLhk/aq/ 56jLagxQDneOBdPw1nojY/oPcGQEzlDDlLLZL6AZG0/ToBUNO5cV2JmIATBZwcvzT63kkAwgoUOTw xWjiXGZcfLrkFmJ3DFtW0TcBp9rieoPFYEUPfn8lWfi8vRkj7GCWH9arJ5Tmw5TX2W7S70VDHVLMd etnXOMSQ==; Received: from [2001:8b0:10b:1::425] (helo=i7.infradead.org) by desiato.infradead.org with esmtpsa (Exim 4.99.2 #2 (Red Hat Linux)) id 1wtmRF-0000000EbSb-0uys; Tue, 11 Aug 2026 13:22:41 +0000 Received: from dwoodhou by i7.infradead.org with local (Exim 4.99.4 #2 (Red Hat Linux)) id 1wtmRC-00000000Qlh-1JH8; Tue, 11 Aug 2026 14:22:38 +0100 From: David Woodhouse To: seanjc@google.com, pbonzini@redhat.com Cc: dwmw2@infradead.org, paul@xen.org, joao.m.martins@oracle.com, boris.ostrovsky@oracle.com, ankur.a.arora@oracle.com, stollmc@amazon.com, paulmck@kernel.org, boqun.feng@gmail.com, urezki@gmail.com, akpm@linux-foundation.org, mhocko@suse.com, bigeasy@linutronix.de, syzbot+919877893c9d28162dc2@syzkaller.appspotmail.com, syzbot+208f7f3e5f59c11aeb90@syzkaller.appspotmail.com, syzkaller-bugs@googlegroups.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v4 2/6] KVM: x86/xen: Extract delivery of event to vCPU into a separate helper Date: Tue, 11 Aug 2026 13:49:52 +0100 Message-ID: <20260811132237.102400-3-dwmw2@infradead.org> X-Mailer: git-send-email 2.55.0 In-Reply-To: <20260811132237.102400-1-dwmw2@infradead.org> References: <20260811132237.102400-1-dwmw2@infradead.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Sender: David Woodhouse X-SRS-Rewrite: SMTP reverse-path rewritten from by desiato.infradead.org. See http://www.infradead.org/rpr.html From: Sean Christopherson 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 [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 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 179efcfb40e3..40cb56621e25 100644 --- a/arch/x86/kvm/xen.c +++ b/arch/x86/kvm/xen.c @@ -1791,6 +1791,93 @@ static void kvm_xen_check_poller(struct kvm_vcpu *vc= pu, int port) } } =20 +/* 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 =3D &vcpu->arch.xen.vcpu_info_cache; + bool has_64bit_shinfo =3D kvm_xen_has_64bit_shinfo(vcpu->kvm); + unsigned long vi_pending_sel_ofs; + bool inject_upcall =3D false; + bool kick_vcpu =3D false; + bool old; + int idx; + + vi_pending_sel_ofs =3D 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 =3D srcu_read_lock(&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 =3D true; + goto out_unlock; + } + inject_upcall =3D 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 =3D GEN_BINARY_RMWcc(LOCK_PREFIX "btsl", + *(u32 *)(gpc->khva + vi_pending_sel_ofs), + c, "Ir", port_word_bit); + if (!old) { + struct vcpu_info *vi =3D gpc->khva; + + /* No need for compat handling */ + BUILD_BUG_ON(offsetof(struct vcpu_info, evtchn_upcall_pending) !=3D + offsetof(struct compat_vcpu_info, evtchn_upcall_pending)); + + WRITE_ONCE(vi->evtchn_upcall_pending, 1); + kick_vcpu =3D true; + } + +out_unlock: + srcu_read_unlock(&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 =E2=80=94 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: @@ -1805,10 +1892,9 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *x= e, struct kvm *kvm) { struct gfn_to_pfn_cache *gpc =3D &kvm->arch.xen.shinfo_cache; bool has_64bit_shinfo =3D 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 =3D false; int vcpu_idx, idx, gpc_idx, rc; =20 vcpu_idx =3D READ_ONCE(xe->vcpu_idx); @@ -1837,16 +1923,12 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *= xe, struct kvm *kvm) pending_bits =3D (unsigned long *)&shinfo->evtchn_pending; mask_bits =3D (unsigned long *)&shinfo->evtchn_mask; port_word_bit =3D xe->port / 64; - - vi_pending_sel_ofs =3D offsetof(struct vcpu_info, evtchn_pending_sel); } else { struct compat_shared_info *shinfo =3D gpc->khva; pending_bits =3D (unsigned long *)&shinfo->evtchn_pending; mask_bits =3D (unsigned long *)&shinfo->evtchn_mask; port_word_bit =3D xe->port / 32; =20 - vi_pending_sel_ofs =3D 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); } @@ -1862,64 +1944,23 @@ int kvm_xen_set_evtchn_fast(struct kvm_xen_evtchn *= xe, struct kvm *kvm) rc =3D 0; /* It was already raised */ } else if (test_bit(xe->port, mask_bits)) { rc =3D -ENOTCONN; /* Masked */ - kvm_xen_check_poller(vcpu, xe->port); } else { - bool old; - rc =3D 1; /* Delivered to the bitmap in shared_info. */ - /* Now switch to the vCPU's vcpu_info to set the index and pending_sel */ - gpc =3D &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 =3D 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 =3D GEN_BINARY_RMWcc(LOCK_PREFIX "btsl", - *(u32 *)(gpc->khva + vi_pending_sel_ofs), - c, "Ir", port_word_bit); - if (!old) { - struct vcpu_info *vi =3D gpc->khva; - - /* No need for compat handling */ - BUILD_BUG_ON(offsetof(struct vcpu_info, evtchn_upcall_pending) !=3D - offsetof(struct compat_vcpu_info, evtchn_upcall_pending)); - - WRITE_ONCE(vi->evtchn_upcall_pending, 1); - kick_vcpu =3D 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 =3D false; - } } =20 out_gpc_srcu: srcu_read_unlock(&kvm->gpc_srcu, gpc_idx); - srcu_read_unlock(&kvm->srcu, idx); =20 - 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 =3D=3D -ENOTCONN) + kvm_xen_check_poller(vcpu, xe->port); + else if (rc =3D=3D 1) + __kvm_xen_set_evtchn_fast(vcpu, port_word_bit); + + srcu_read_unlock(&kvm->srcu, idx); =20 return rc; } --=20 2.55.0 From nobody Tue Aug 25 15:23:29 2026 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 34F65188CC9; Tue, 11 Aug 2026 13:22:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.50.34 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786454575; cv=none; b=DUmpAxqUJurRY8C7c/yx3yz6p2NO8UT5/jCaNbjQQvZYeKyKJOPYcM9Wupm4kuwoIdbvz2Qj7KuLXZzcDyRuANfiq0LYpiHybrw5TazK9E3+ol1T5IwTh0k9EY0yk/jTpkMTX1C+CAoaxwhaHbXxxz2zQneppRRWZ8Ii7EaxT9w= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786454575; c=relaxed/simple; bh=JAxpfyOzRShUM3aetn4jtcB9opZ/qGfCoc2Fd/FMQlA=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=PokV4KBS2kGAfwoIEZvLEuxTXv4TK+hSpk7xfDTZNgk6buYx9wSjoPtz1Sf/rP4I+EyAR1pFFM+fisbED14esDqUUm73X3N6kUmFBUq+2FYmezovh9in/T9PvlGbFjKWj+gIUXgqXN21GhilBZIhdoLeh+z4l19emdQYGCwsH+M= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=casper.srs.infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=vzrwpBUL; arc=none smtp.client-ip=90.155.50.34 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=casper.srs.infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="vzrwpBUL" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=ScdNk4Mc4txgD05pFXbeLZfLevYP2xjCgGErsx2bQ1k=; b=vzrwpBULc60auXHrbsm3YfTQDZ JKoGspBrS3g/RpwKIP/W/sh+aj7O6m+TftfLoET1nkLNIsZKnZ2nwRBbYM9QGodKhDM2zGi7I3zUI +qami8ZuVkxTWTgy0YtuzYTL1P8QzDuzkF8lsi7C1FPUNU0AJ9X0m//P13Lih5Z9aHIWYGGZznHSz w+NNgwPai+m9RKebHT4FiGQtZdqG6eAdDZhxGZvvYPKSC9tmfgKjaXesq/LVtI9EiCCLCOCAOqPjC v8v3lL6w+MdTbjcxC1ICuin+K6FUaM+EpL/YKkC9/tR8LDDMBa4brQMZqQfRsuYRFiw5Cn97YVU/h HF1ZoV8g==; Received: from [2001:8b0:10b:1::425] (helo=i7.infradead.org) by casper.infradead.org with esmtpsa (Exim 4.99.1 #2 (Red Hat Linux)) id 1wtmRC-00000000TZ2-2SCx; Tue, 11 Aug 2026 13:22:39 +0000 Received: from dwoodhou by i7.infradead.org with local (Exim 4.99.4 #2 (Red Hat Linux)) id 1wtmRC-00000000Qlm-1eEG; Tue, 11 Aug 2026 14:22:38 +0100 From: David Woodhouse To: seanjc@google.com, pbonzini@redhat.com Cc: dwmw2@infradead.org, paul@xen.org, joao.m.martins@oracle.com, boris.ostrovsky@oracle.com, ankur.a.arora@oracle.com, stollmc@amazon.com, paulmck@kernel.org, boqun.feng@gmail.com, urezki@gmail.com, akpm@linux-foundation.org, mhocko@suse.com, bigeasy@linutronix.de, syzbot+919877893c9d28162dc2@syzkaller.appspotmail.com, syzbot+208f7f3e5f59c11aeb90@syzkaller.appspotmail.com, syzkaller-bugs@googlegroups.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v4 3/6] KVM: x86/xen: Explicitly tag "shared info" page as never being dirty tracked Date: Tue, 11 Aug 2026 13:49:53 +0100 Message-ID: <20260811132237.102400-4-dwmw2@infradead.org> X-Mailer: git-send-email 2.55.0 In-Reply-To: <20260811132237.102400-1-dwmw2@infradead.org> References: <20260811132237.102400-1-dwmw2@infradead.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: David Woodhouse X-SRS-Rewrite: SMTP reverse-path rewritten from by casper.infradead.org. See http://www.infradead.org/rpr.html Content-Type: text/plain; charset="utf-8" From: Sean Christopherson 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 Signed-off-by: David Woodhouse 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 40cb56621e25..fab215a522be 100644 --- a/arch/x86/kvm/xen.c +++ b/arch/x86/kvm/xen.c @@ -2396,7 +2396,7 @@ void kvm_xen_init_vm(struct kvm *kvm) { mutex_init(&kvm->arch.xen.xen_lock); idr_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); } =20 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 zeroe= d 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); +} =20 /** * 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"); =20 - if (!gpc->memslot) + if (!gpc->memslot || gpc->never_dirty) return; =20 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; }; =20 /* 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 a6a7cfd13246..97958af667fb 100644 --- a/virt/kvm/pfncache.c +++ b/virt/kvm/pfncache.c @@ -597,7 +597,8 @@ int kvm_gpc_refresh(struct gfn_to_pfn_cache *gpc, unsig= ned long len) return __kvm_gpc_refresh(gpc, gpc->gpa, uhva); } =20 -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); =20 @@ -607,6 +608,7 @@ void kvm_gpc_init(struct gfn_to_pfn_cache *gpc, struct = kvm *kvm) gpc->uhva =3D KVM_HVA_ERR_BAD; gpc->active =3D false; atomic_set(&gpc->state, 0); + gpc->never_dirty =3D never_dirty; } =20 static int __kvm_gpc_activate(struct gfn_to_pfn_cache *gpc, gpa_t gpa, uns= igned long uhva, --=20 2.55.0 From nobody Tue Aug 25 15:23:29 2026 Received: from desiato.infradead.org (desiato.infradead.org [90.155.92.199]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 6FB97322A1F; Tue, 11 Aug 2026 13:22:51 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.92.199 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786454575; cv=none; b=WZoFBRki7Etxi7PMHJnw3d2PLmG16BGhN6ERKF9fbnhbNsnX/LLTvt9JydtOsvAK3WlVjap/tGEzHOspzc2JfCe38uVM1HX35EwKb3BnHEdxABsSh23ktLXUeJ+8ePRMQKHBM2UwDoBld/0xJbANMjo7joLzG0UmdjLj3oRT16k= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786454575; c=relaxed/simple; bh=V4aDRjWyUwTroHQeRzDrQcTmf0XJT8FUZsPZw6WqftM=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version; b=DLRP9NeC62OhvRSLf+f0wJCoukEvminTmZSzX5dSxE7ZqA4ygvf9JpcvcdE1L4BO0pY5wK3CHkZ/2VnKWgBCo1eGWMPlzuNMnvat/zR7ML5Lwz0AARbfSwTr3A8oESDKXYULQ0rrhZv1adlWueASUTEj+7VzvwEx2YSpqUUMkHM= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=desiato.srs.infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=BSOINfUC; arc=none smtp.client-ip=90.155.92.199 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=desiato.srs.infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="BSOINfUC" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=desiato.20200630; h=Sender:Content-Transfer-Encoding: MIME-Version:References:In-Reply-To:Message-ID:Date:Subject:Cc:To:From: Reply-To:Content-Type:Content-ID:Content-Description; bh=LwRSUKXXSKjG5ftUYDBBrK8NlvPko6QM5e1CCzNNyAo=; b=BSOINfUCZkUGuxmqAtgtFfFWGc D5LHAIz8GqehtE3bQ8s14hcUcUn8q0cILlrzqrretFj7yp3GkmSusNHBCHLnR0IqNBH/5rxbageEf YKsmjudX05erlQgk44ZhnvSqueHZQouZZEfxyywIaJ5fXIS8ma5PUO5/pUZHxo3cIn8Yh+nqoEsLf 2SNzgGlgasMwN7QSOCOwKK1pbrUaITnJxf03La1v6rGLwhnCW2cBkDnaVGV2VLZl7YiTkKW+z7KXF VrnlAVfme/BpB9P4n5vzA8o0I2mk+YGnB1TDxt1f3gMMXaRd2+w6fxIhz0D6OXIdpKH/4wW9qdELG S3fvDkVA==; Received: from [2001:8b0:10b:1::425] (helo=i7.infradead.org) by desiato.infradead.org with esmtpsa (Exim 4.99.2 #2 (Red Hat Linux)) id 1wtmRF-0000000EbSc-0uYh; Tue, 11 Aug 2026 13:22:41 +0000 Received: from dwoodhou by i7.infradead.org with local (Exim 4.99.4 #2 (Red Hat Linux)) id 1wtmRC-00000000Qlq-1oN1; Tue, 11 Aug 2026 14:22:38 +0100 From: David Woodhouse To: seanjc@google.com, pbonzini@redhat.com Cc: dwmw2@infradead.org, paul@xen.org, joao.m.martins@oracle.com, boris.ostrovsky@oracle.com, ankur.a.arora@oracle.com, stollmc@amazon.com, paulmck@kernel.org, boqun.feng@gmail.com, urezki@gmail.com, akpm@linux-foundation.org, mhocko@suse.com, bigeasy@linutronix.de, syzbot+919877893c9d28162dc2@syzkaller.appspotmail.com, syzbot+208f7f3e5f59c11aeb90@syzkaller.appspotmail.com, syzkaller-bugs@googlegroups.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v4 4/6] KVM: x86/xen: Don't dirty track "vCPU info" page Date: Tue, 11 Aug 2026 13:49:54 +0100 Message-ID: <20260811132237.102400-5-dwmw2@infradead.org> X-Mailer: git-send-email 2.55.0 In-Reply-To: <20260811132237.102400-1-dwmw2@infradead.org> References: <20260811132237.102400-1-dwmw2@infradead.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Sender: David Woodhouse X-SRS-Rewrite: SMTP reverse-path rewritten from by desiato.infradead.org. See http://www.infradead.org/rpr.html Content-Type: text/plain; charset="utf-8" From: Sean Christopherson 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 Signed-off-by: David Woodhouse 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 fab215a522be..5618e2ccf4c4 100644 --- a/arch/x86/kvm/xen.c +++ b/arch/x86/kvm/xen.c @@ -666,7 +666,6 @@ void kvm_xen_inject_pending_events(struct kvm_vcpu *v) WRITE_ONCE(vi->evtchn_upcall_pending, 1); } =20 - kvm_gpc_mark_dirty_in_slot(gpc); srcu_read_unlock(&v->kvm->gpc_srcu, idx); =20 /* For the per-vCPU lapic vector, deliver it as MSI. */ @@ -2375,7 +2374,7 @@ void kvm_xen_init_vcpu(struct kvm_vcpu *vcpu) =20 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); } =20 --=20 2.55.0 From nobody Tue Aug 25 15:23:29 2026 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id CDB662FE07D; Tue, 11 Aug 2026 13:22:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.50.34 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786454576; cv=none; b=LGhYMtUpOUL6uf0p8TGcwJAD/azUoSwyyKLTE31h40cPWsPn9VdjNI/H0+t49gVPj/7Utrf4bKIMHXFH+Djic3XDmF2pQR0vLtggn7eBiFY/cfhFL0fTq6EySEuYq2tCgtMyd41GWbpJ74Vj9xUJi5g0IF+jkP5b0lgLcefERrI= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786454576; c=relaxed/simple; bh=t8lK9Snhc9yquoPRTRoV93Fn4Ux8130cQotiqUI7WSE=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=Z/ob/AYqOGDDc2AY5c57TP/nj5wheKkNGSHfL98waHx/J2NTWVdZFL6USy7hP/5rL1UG48sWKiWfTGG4hzBcRetvIu8IyAIQKFykWUx1pYhfU+WFLLufcDBFGjS+10dSAn78ZnN2qdGrrTlVG1lpJOkhGE1I7/RCWplL0IhQB7U= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=casper.srs.infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=TXUVYBPu; arc=none smtp.client-ip=90.155.50.34 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=casper.srs.infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="TXUVYBPu" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:MIME-Version:References:In-Reply-To:Message-ID:Date:Subject:Cc: To:From:Reply-To:Content-ID:Content-Description; bh=38VDlC3T9U7k5c8RZkKKjsBg4drLylSLp9dK+J84LzQ=; b=TXUVYBPuhtawpFgnGcR2yMasLr VSAED2oudrQ4i5pCQsu8QveShjkrlc9+KfZofbqxVR2YlUDScRQT8pi7BZsVnB0xZFbn+ZBRwsZKq orbVM/eNfEq9xYXq98l+P32mZOkAyTJGeYwlKMjXRAMeBbDgql4frml9QBAeE+IX6+UyKBZTQVL0g OfoVbYaXUNHgiyVCZ9GbDqzf+MmgyufviRLznnPkKGh4h24joZOGlIobHg1Zs9ggK3yPVHJRtkMze L2A15dM3pm9RKKF+pa4W4Q2rOO6yD7xbg3e82ctr5VH2UpOjVas0oKJX+rCMN5+s7ze3S/oBngMeG NwdEymXQ==; Received: from [2001:8b0:10b:1::425] (helo=i7.infradead.org) by casper.infradead.org with esmtpsa (Exim 4.99.1 #2 (Red Hat Linux)) id 1wtmRC-00000000TZ4-2wxa; Tue, 11 Aug 2026 13:22:39 +0000 Received: from dwoodhou by i7.infradead.org with local (Exim 4.99.4 #2 (Red Hat Linux)) id 1wtmRC-00000000Qlu-23nS; Tue, 11 Aug 2026 14:22:38 +0100 From: David Woodhouse To: seanjc@google.com, pbonzini@redhat.com Cc: dwmw2@infradead.org, paul@xen.org, joao.m.martins@oracle.com, boris.ostrovsky@oracle.com, ankur.a.arora@oracle.com, stollmc@amazon.com, paulmck@kernel.org, boqun.feng@gmail.com, urezki@gmail.com, akpm@linux-foundation.org, mhocko@suse.com, bigeasy@linutronix.de, syzbot+919877893c9d28162dc2@syzkaller.appspotmail.com, syzbot+208f7f3e5f59c11aeb90@syzkaller.appspotmail.com, syzkaller-bugs@googlegroups.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v4 5/6] KVM: x86: Request the guest TLB flush from record_steal_time() Date: Tue, 11 Aug 2026 13:49:55 +0100 Message-ID: <20260811132237.102400-6-dwmw2@infradead.org> X-Mailer: git-send-email 2.55.0 In-Reply-To: <20260811132237.102400-1-dwmw2@infradead.org> References: <20260811132237.102400-1-dwmw2@infradead.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Sender: David Woodhouse X-SRS-Rewrite: SMTP reverse-path rewritten from by casper.infradead.org. See http://www.infradead.org/rpr.html From: David Woodhouse 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 =E2=80=94 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 Closes: https://lore.kernel.org/all/20260530061932.7849D1F00893@smtp.kernel= .org Suggested-by: Sean Christopherson Signed-off-by: David Woodhouse 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 75e5f894153f..d2e60d1146e0 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) =20 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); =20 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 =3D 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)) --=20 2.55.0 From nobody Tue Aug 25 15:23:29 2026 Received: from casper.infradead.org (casper.infradead.org [90.155.50.34]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 015DF311977; Tue, 11 Aug 2026 13:22:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=90.155.50.34 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786454576; cv=none; b=l2NeqSvKwyI8whmJ1XqFM0tf25GcAvjH94ltJN/uDYISgOYXMm1uQqxo3wvGMB29FV14e8csJ5HQjeSxk2q87LjcS5DtFeT5TKNRtej8q8le8FJYejh4J8uKCRco0fmc8OCJM4xeaed5Y0PZIg7hqMzULPfSLhzTiIvpWMYR18w= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1786454576; c=relaxed/simple; bh=fq6CY8sB2LogKrKscGrbqFHAiA+DZ8bXdzlJuE5HAWo=; h=From:To:Cc:Subject:Date:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=b/2EqFN+p1EE1YyYRhwljVA1vhoidwoKTzi7Jf+eMZNYG0KzeA9e4gJF2x7p19QEbpzdHx5Tn4ALc3QtNmnvaJrlA4WTdwvhoS4DqSHszc97Nd8LVwu8IcLtd4rS1qjfEx7fMWUi78m07FSUbwjVvK0WnVPkgTLdzU/NDYBS59A= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org; spf=none smtp.mailfrom=casper.srs.infradead.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b=pKp08oBZ; arc=none smtp.client-ip=90.155.50.34 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=infradead.org Authentication-Results: smtp.subspace.kernel.org; spf=none smtp.mailfrom=casper.srs.infradead.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=infradead.org header.i=@infradead.org header.b="pKp08oBZ" DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=casper.20170209; h=Sender:Content-Transfer-Encoding: Content-Type:MIME-Version:References:In-Reply-To:Message-ID:Date:Subject:Cc: To:From:Reply-To:Content-ID:Content-Description; bh=USjcj653Zr/NH8yJpm5nWfRcAU4X9NdS2j89SsPVCCU=; b=pKp08oBZPXCjniTyySuFCKu5nJ RLrYa6iTLM6B+9D1Rk4FOw4EFynTnDPae9QXDlzN9RJhSmExEAstqEq1CsGvLD8b3w/QxYjK+3CS+ 3FAiqNyQuzET+UzLF9iLpwZsmHTfAHiyBrcx6zcrH0HYR+2RbdbWk//ksvfUaxtzNP1ler0+9cJ6l 0OS8cBAcwQviCFuMmgyew0eYa3Jbq63B5xRrup0BbT3huEMvxMf4X8vUuwh6Ad9nsfknY1SoD22LL F5c613VwQKS2CyqKQZI9rhi+jL8X6kq73q//lKQDOz87giznauBgir5OtmBbY8b4Y+7BsBCquXXnm sTavY6vQ==; Received: from [2001:8b0:10b:1::425] (helo=i7.infradead.org) by casper.infradead.org with esmtpsa (Exim 4.99.1 #2 (Red Hat Linux)) id 1wtmRC-00000000TZ5-37Uf; Tue, 11 Aug 2026 13:22:39 +0000 Received: from dwoodhou by i7.infradead.org with local (Exim 4.99.4 #2 (Red Hat Linux)) id 1wtmRC-00000000Qly-2Jo8; Tue, 11 Aug 2026 14:22:38 +0100 From: David Woodhouse To: seanjc@google.com, pbonzini@redhat.com Cc: dwmw2@infradead.org, paul@xen.org, joao.m.martins@oracle.com, boris.ostrovsky@oracle.com, ankur.a.arora@oracle.com, stollmc@amazon.com, paulmck@kernel.org, boqun.feng@gmail.com, urezki@gmail.com, akpm@linux-foundation.org, mhocko@suse.com, bigeasy@linutronix.de, syzbot+919877893c9d28162dc2@syzkaller.appspotmail.com, syzbot+208f7f3e5f59c11aeb90@syzkaller.appspotmail.com, syzkaller-bugs@googlegroups.com, kvm@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v4 6/6] KVM: x86: Use gfn_to_pfn_cache for steal time / preempted status Date: Tue, 11 Aug 2026 13:49:56 +0100 Message-ID: <20260811132237.102400-7-dwmw2@infradead.org> X-Mailer: git-send-email 2.55.0 In-Reply-To: <20260811132237.102400-1-dwmw2@infradead.org> References: <20260811132237.102400-1-dwmw2@infradead.org> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Sender: David Woodhouse X-SRS-Rewrite: SMTP reverse-path rewritten from by casper.infradead.org. See http://www.infradead.org/rpr.html From: Carsten Stollmaier 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 =E2=80=94 the old unsafe_put_user() inline assembly act= ed 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 =E2=80=94 the old HVA-based code used copy_to_user_nofault(), which could equally silently fail =E2=80=94 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 Co-developed-by: David Woodhouse Signed-off-by: David Woodhouse Signed-off-by: Sean Christopherson Signed-off-by: David Woodhouse 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_hos= t.h index 283847619ff8..93514cb1f2f7 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -873,7 +873,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; =20 u64 l1_tsc_offset; diff --git a/arch/x86/kvm/msrs.c b/arch/x86/kvm/msrs.c index 66fa7140d65d..37d79d84e2f8 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) =20 vcpu->arch.st.msr_val =3D data; =20 - 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)); =20 kvm_make_request(KVM_REQ_STEAL_UPDATE, vcpu); =20 diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c index d2e60d1146e0..ac961f4c91da 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_tl= b_flush_requests); =20 static void record_steal_time(struct kvm_vcpu *vcpu) { - struct gfn_to_hva_cache *ghc =3D &vcpu->arch.st.cache; - struct kvm_steal_time __user *st; - struct kvm_memslots *slots; - gpa_t gpa =3D vcpu->arch.st.msr_val & KVM_STEAL_VALID_BITS; + struct gfn_to_pfn_cache *gpc =3D &vcpu->arch.st.cache; + struct kvm_steal_time *st; u64 steal; u32 version; + int idx; =20 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 !=3D vcpu->kvm->mm)) return; =20 - slots =3D 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); =20 - if (unlikely(slots->generation !=3D ghc->generation || - gpa !=3D 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 =3D srcu_read_lock(&vcpu->kvm->gpc_srcu); + while (!kvm_gpc_check(gpc, sizeof(*st))) { + srcu_read_unlock(&vcpu->kvm->gpc_srcu, idx); =20 - 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 =3D srcu_read_lock(&vcpu->kvm->gpc_srcu); } =20 - st =3D (struct kvm_steal_time __user *)ghc->hva; + st =3D 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 =3D 0; - int err =3D -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 =3D xchg(&st->preempted, 0); =20 vcpu->arch.st.preempted =3D 0; =20 @@ -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 =3D 0; } =20 - unsafe_get_user(version, &st->version, out); + version =3D READ_ONCE(st->version); if (version & 1) version +=3D 1; /* first time write, random junk */ =20 version +=3D 1; - unsafe_put_user(version, &st->version, out); + WRITE_ONCE(st->version, version); =20 smp_wmb(); =20 - unsafe_get_user(steal, &st->steal, out); + steal =3D READ_ONCE(st->steal); steal +=3D current->sched_info.run_delay - vcpu->arch.st.last_steal; vcpu->arch.st.last_steal =3D current->sched_info.run_delay; - unsafe_put_user(steal, &st->steal, out); + WRITE_ONCE(st->steal, steal); + + smp_wmb(); =20 version +=3D 1; - unsafe_put_user(version, &st->version, out); + WRITE_ONCE(st->version, version); =20 - 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(&vcpu->kvm->gpc_srcu, idx); } =20 static inline bool kvm_can_mwait_in_guest(void) @@ -2628,11 +2606,8 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int c= pu) =20 static void kvm_steal_time_set_preempted(struct kvm_vcpu *vcpu) { - struct gfn_to_hva_cache *ghc =3D &vcpu->arch.st.cache; - struct kvm_steal_time __user *st; - struct kvm_memslots *slots; - static const u8 preempted =3D KVM_VCPU_PREEMPTED; - gpa_t gpa =3D vcpu->arch.st.msr_val & KVM_STEAL_VALID_BITS; + struct gfn_to_pfn_cache *gpc =3D &vcpu->arch.st.cache; + struct kvm_steal_time *st; =20 /* * 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 !=3D vcpu->kvm->mm)) return; =20 - slots =3D kvm_memslots(vcpu->kvm); - - if (unlikely(slots->generation !=3D ghc->generation || - gpa !=3D ghc->gpa || - kvm_is_error_hva(ghc->hva) || !ghc->memslot)) - return; - - st =3D (struct kvm_steal_time __user *)ghc->hva; - BUILD_BUG_ON(sizeof(st->preempted) !=3D 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)(&vcpu->kvm->gpc_srcu); + if (kvm_gpc_check(gpc, sizeof(*st))) { + st =3D gpc->khva; + WRITE_ONCE(st->preempted, KVM_VCPU_PREEMPTED); vcpu->arch.st.preempted =3D KVM_VCPU_PREEMPTED; =20 - 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 =3D 0; + vcpu->arch.st.msr_val =3D 0; + vcpu->arch.st.last_steal =3D 0; } =20 void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu) @@ -9368,6 +9354,8 @@ int kvm_arch_vcpu_create(struct kvm_vcpu *vcpu) =20 kvm_gpc_init(&vcpu->arch.pv_time, vcpu->kvm); =20 + 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); =20 + kvm_steal_time_reset(vcpu); + kvmclock_reset(vcpu); =20 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 =3D 0; vcpu->arch.apf.msr_int_val =3D 0; - vcpu->arch.st.msr_val =3D 0; + + kvm_steal_time_reset(vcpu); =20 kvmclock_reset(vcpu); =20 --=20 2.55.0