[PATCH v3 00/11] KVM: x86/xen: Don't drop event deliveries which race memslot updates

[David Woodhouse]

From: David Woodhouse <dwmw@xxxxxxxxxxxx>

Sashiko analysis and torture testing of the upcoming conversion of the
gfn_to_pfn_cache from rwlock to atomic SRCU¹ highlighted a pre-existing
problem: refreshing a cache can fail while a memslot is briefly marked
KVM_MEMSLOT_INVALID during a memslot update, causing kvm_gpc_refresh() to
return -EFAULT.

The Xen event channel and timer delivery paths silently dropped events when
they hit that window. In some cases we don't really care very much: the VMM
should really be using the KVM_XEN_ATTR_TYPE_SHARED_INFO_HVA option to more
faithfully represent the Xen semantics where the shared info page exists
whether it's mapped into the guest at any given moment or not, which doesn't
depend on memslots at all. But we can be more consistent, and more
intentional about it. So...

Return -EAGAIN from __kvm_gpc_refresh() to distinguish the transient case
from a genuinely unbacked GFN, and add a retry loop for the common case
callers which can just wait for the memslot update to complete, as well as
dealing with the corner cases which can't:

 - The Xen attribute ioctls wait, so userspace never sees the transient
   state.

 - Timer delivery leaves ->arch.xen.timer_pending set and retries from
   kvm_xen_inject_timer_irqs() on the next vCPU entry.

 - The guest's own EVTCHNOP_send hypercall runs under vcpu_run's kvm->srcu
   read section — the very reader the memslot update is waiting for — so it
   cannot wait. Leave the hypercall uncompleted without advancing RIP, so
   the guest re-executes it. This is how Xen itself restarts preempted
   hypercalls.

 - The hypercall page MSR may want to update the shared info page if
   latching a change of long_mode. Plumbing a retry like the one added for
   EVTCHNOP_send would be complex overkill, so on -EAGAIN just allow the
   wallclock change to fail (as opposed to #GP as it did before without
   even filling the hypercall page). A guest sets the hypercall page
   *early* in boot, and nothing should be manipulating memslots then
   anyway.

The waiting is done without holding xen_lock, on which a vCPU writing
the hypercall page MSR may block from within its kvm->srcu read
section — the very reader the memslot update has to wait out.

Patches 6-9 are pulled forward from the SRCU series because they stand
alone:

 - Three from Sean², extracting vcpu_info delivery into a separate
   helper and taking the shared_info and vcpu_info pages out of dirty
   tracking. Rebased: his series had first converted the GPC locks to
   read_trylock(), which this one doesn't carry as the rwlocks are all
   going away soon anyway.

 - Requesting the guest TLB flush from record_steal_time() itself, as
   a standalone cleanup in preparation for converting that to GPC.

The last two patches are selftests: a new test racing a guest
EVTCHNOP_send loop against a tight memslot delete/recreate loop, and a
fix to xen_shinfo_test's timer recovery protocol. Tested with both and
the rest of the KVM selftests; an instrumented run showed 10 of the
race test's 4000 sends hitting the update window and re-executing.

Based on kvm/next (d4b7fb647204).

v3: https://git.infradead.org/?p=users/dwmw2/linux.git;a=shortlog;h=refs/heads/gpc-eagain-v3
 - Take kvm->srcu around each kvm_gpc_check() and the copy through the
   mapping in xen_get_guest_pvclock(), which had none (also found by
   the syzbot CI on v2).
 - Drop v2's patch 3 and move the waiting out from under xen_lock
   instead. The srcu unlock/relock it added to the hypercall page MSR
   write was reachable from nested VM-exit MSR-load lists in contexts
   (KVM_SET_NESTED_STATE and friends) which hold kvm->srcu by a local
   index, corrupting the SRCU counts. With no ioctl ever waiting
   while holding xen_lock, taking it from within a kvm->srcu read
   section is safe again and no srcu games are needed. Activations
   racing a concurrent SET_ATTR in the window are userspace's own
   race; last one wins.
 - The hypercall page MSR write keeps wait=false for its shared_info
   re-init and no longer fails the WRMSR on the transient race (see
   above).
 - Re-arm through kvm_xen_start_timer() in the KVM_XEN_VCPU_GET_ATTR
   timer flush, recomputing the expiry from timer_expires. The
   hrtimer's own expiry field is stale when the timer last fired
   through the direct callback path, which doesn't program it. Don't
   re-arm an undeliverable (out of range) port, where the immediate
   -EINVAL would destroy the timer_expires state just reported.
 - New final patch: give the timer recovery poll in xen_shinfo_test a
   timeout. If the timer event is dropped while the guest is already
   blocked in SCHEDOP_poll, nothing else ever wakes it, so the test's
   documented VMM re-arm protocol could never actually run.
 - Commit message and kernel-doc fixes.

Thanks again to the Sashiko review bot, which found the first three
on v2. Less so to my own local incarnation of Sashiko, which didn't —
though it did redeem itself by finding the stale-srcu-index problem
with v2's patch 3.

v2: https://git.infradead.org/?p=users/dwmw2/linux.git;a=shortlog;h=refs/heads/gpc-eagain-v2
    https://lore.kernel.org/kvm/20261014220508.518772-1-dwmw2@xxxxxxxxxxxxx/
 - Take kvm->srcu around the kvm_gpc_check() loop and the wallclock
   write in kvm_xen_shared_info_init(), which had lost it.
 - New patch 3: don't block on xen_lock within the vCPU's kvm->srcu
   read section (three-way deadlock against a waiting ioctl and a
   memslot update). Document the rule at the lock's definition.
 - On -EINVAL (deactivated shared_info, or an invalid port), clear
   timer_pending but keep timer_expires: a halted vCPU would
   otherwise spin aborting its own block, and the event is still
   reported as pending through KVM_XEN_VCPU_GET_ATTR.
 - Drop the wait argument from kvm_xen_inject_timer_irqs(); neither
   caller may wait.
 - Extend the KVM_GET_DIRTY_LOG note in api.rst to the vcpu_info area.
 - Teach lockdep that the wait functions as a synchronize_srcu(), so
   it reports any lock held across it which a kvm->srcu read section
   can take.
 - Comment fixes; drop a leftover cosmetic hunk.

v1: https://git.infradead.org/?p=users/dwmw2/linux.git;a=shortlog;h=refs/heads/gpc-eagain
    https://lore.kernel.org/kvm/20261013204824.454208-1-dwmw2@xxxxxxxxxxxxx/

¹ https://git.infradead.org/?p=users/dwmw2/linux.git;a=shortlog;h=refs/heads/gpc-srcu-v2
² https://lore.kernel.org/kvm/20260529165114.748639-1-seanjc@xxxxxxxxxx/


David Woodhouse (8):
  KVM: pfncache: Return -EAGAIN for a lookup which hits an invalid
    memslot
  KVM: pfncache: Add a 'wait' argument to the refresh and activate
    functions
  KVM: x86/xen: Wait out memslot updates in the Xen attribute ioctls
  KVM: x86/xen: Don't lose the timer event when delivery fails
    transiently
  KVM: x86/xen: Retry event channel delivery which races a memslot
    update
  KVM: x86: Request the guest TLB flush from record_steal_time()
  KVM: selftests: Add Xen evtchn vs memslot-update race test
  KVM: selftests: Fix unreachable timer recovery in xen_shinfo_test

Sean Christopherson (3):
  KVM: x86/xen: Extract delivery of event to vCPU into a separate helper
  KVM: x86/xen: Explicitly tag "shared info" page as never being dirty
    tracked
  KVM: x86/xen: Don't dirty track "vCPU info" page

 Documentation/virt/kvm/api.rst                |   5 +-
 arch/x86/include/asm/kvm_host.h               |   6 +
 arch/x86/kvm/msrs.c                           |   2 +-
 arch/x86/kvm/x86.c                            |  28 +-
 arch/x86/kvm/xen.c                            | 442 ++++++++++++------
 include/linux/kvm_host.h                      |  39 +-
 include/linux/kvm_types.h                     |   1 +
 tools/testing/selftests/kvm/Makefile.kvm      |   1 +
 .../kvm/x86/xen_evtchn_memslot_race_test.c    | 178 +++++++
 .../selftests/kvm/x86/xen_shinfo_test.c       |  20 +-
 virt/kvm/pfncache.c                           | 142 ++++--
 11 files changed, 661 insertions(+), 203 deletions(-)
 create mode 100644 tools/testing/selftests/kvm/x86/xen_evtchn_memslot_race_test.c


base-commit: d4b7fb647204f0c81dfeae2d1a708e4d858e0c94
-- 
2.43.0