[PATCH] x86/altp2m: pause the vCPU before taking the altp2m list lock

[lluisbessasanz <lluisbessasanz@xxxxxxxxx>]

From: Lluís Bessa <evilbootloader@xxxxxxxxx>

p2m_switch_vcpu_altp2m_by_id() takes the altp2m list lock and then
updates the vCPU's EPTP. For a vCPU other than current,
vmx_vcpu_update_eptp() -> vmx_vmcs_enter() pauses it and waits in
vcpu_sleep_sync() until it is off its pCPU, with the lock still held.

A vCPU on its way off its pCPU may need the p2m lock: context_switch()
updates its runstate area, which for an area registered by guest virtual
address (VCPUOP_register_runstate_memory_area, as Linux does) is a copy
to guest memory and needs a p2m lookup. A vCPU handling a log-dirty
write fault holds the host p2m lock in p2m_change_type_one() and waits
for the altp2m list lock in p2m_altp2m_propagate_change(). That closes
a cycle:

  dom0 vCPU, vm_event_resume() -> p2m_altp2m_check():
    holds the altp2m list lock, waits for vCPU X to be descheduled
  vCPU X, context_switch() -> update_runstate_area():
    waits for the host p2m lock
  vCPU Y, hvm_hap_nested_page_fault() -> p2m_change_type_one():
    holds the host p2m lock, waits for the altp2m list lock

A vm_event consumer switching views in its responses
(VM_EVENT_FLAG_ALTERNATE_P2M) hits it with a Linux HVM guest while QEMU
tracks dirty VRAM for a VNC client. The dom0 vCPUs involved spin in Xen
and stop handling interrupts, and dom0 hangs.

Pause the vCPU before taking the lock, like
p2m_switch_domain_altp2m_by_id() does with domain_pause_except_self().
The pause in vmx_vmcs_enter() then finds the vCPU already descheduled.
Only vm_event_resume() switches another vCPU's view, the other callers
switch current and are unaffected.

Signed-off-by: Lluís Bessa <evilbootloader@xxxxxxxxx>
Co-Authored-By: Claude Opus 5.5 <noreply@xxxxxxxxxxxxx>
---

Notes:
    Seen on Xen 4.20.1 (debug=n), Intel family 6 model 0xb7, PV dom0 (Linux
    6.8, 6 vCPUs pinned), Xen command line:
      dom0_mem=6144M,max:6144M dom0_max_vcpus=6 dom0_vcpus_pin=1 force-ept=1
      ept=ad=0 hap_1gb=0 hap_2mb=0 altp2m=1 hpet=legacy-replacement smt=0
      sched=credit conring_size=4M
    Guest: HVM, hap=1, altp2m=2 (external), 3 vCPUs, 8 GiB, vga="stdvga",
    vnc=1, QEMU 9.1.0. Ubuntu 22.04 with its stock 5.15 kernel, no drivers
    installed. The trace of CPU12 below shows Xen copying the runstate into
    guest memory, so the guest kernel registered a runstate area by itself.
    
    vm_event consumer: a multithreaded DRAKVUF build we are developing,
    which handles the vm_events of different vCPUs in parallel threads. It
    traces syscalls with altp2m breakpoints, every breakpoint response sets
    VM_EVENT_FLAG_ALTERNATE_P2M (fast single step). The same build with
    single-threaded event handling hangs as well, so one dom0 thread
    answering events is enough. Upstream DRAKVUF wasn't tested, it sends
    the same kind of responses.
    
    With a VNC client attached and the guest screen changing, dom0 hangs
    within seconds in most runs, with multithreaded and single-threaded
    event handling: soft lockups in dom0, disk and network I/O stop. Not
    seen without a VNC client, nor with a Windows guest without Xen PV
    drivers.
    
    Debug keys 'q' and 'd' during the hang (trimmed). d1 has pause_count=1
    (QEMU in hap_track_dirty_vram()), but its vCPUs are still running:
    
      VCPU0: CPU12 [has=T] pause_count=2
      VCPU1: CPU14 [has=T] pause_count=0
      VCPU2: CPU16 [has=T] pause_count=0
    
    CPU2 (d0v1, DRAKVUF answering a breakpoint; our build running with
    single-threaded event handling):
      vcpu_sleep_sync+0x32/0x60
      vmx_vmcs_try_enter.part.0+0x3e/0xe0
      vmx_vcpu_update_eptp+0x43/0xb0
      p2m_switch_vcpu_altp2m_by_id+0x14a/0x1d0
      vm_event_resume.part.0+0x245/0x270
      evtchn_send+0x28a/0x2e0
      do_event_channel_op+0x1cc/0x1130
    
    CPU12 (d1v0, being descheduled):
      queue_read_lock_slowpath+0x22/0x60
      p2m_get_page_from_gfn+0x305/0x360
      hvm_translate_get_page+0x19c/0x1e0
      __hvm_copy+0xa9/0x460
      copy_to_user_hvm+0x71/0x90
      update_runstate_area+0x200/0x3a0
      _update_runstate_area+0x9/0x40
      context_switch+0x63/0x940
      schedule+0x1c4/0x260
      __do_softirq+0x59/0xa0
      vmx_asm_do_vmentry+0x2b/0x30
    
    CPU14 (d1v1, log-dirty write fault on VRAM):
      _rspin_lock+0x42/0x60
      p2m_altp2m_propagate_change+0x62/0x2d0
      ept_set_entry+0x71c/0x7c0
      p2m_set_entry+0x8e/0x150
      p2m_change_type_one+0xb5/0x270
      hvm_hap_nested_page_fault+0x3f6/0x6b0
      vmx_vmexit_handler+0xe7f/0x17f0
    
    Also waiting: CPU8 (d0v4, QEMU) in vcpu_sleep_sync() from _domain_pause()
    in hap_track_dirty_vram(), CPU16 (d1v2) for the p2m write lock in
    p2m_get_gfn_type_access(), CPU10 (d0v5, netback) for the p2m read lock in
    gnttab_copy().
    
    Tested on 4.20.1 with this patch: 9 runs of 15 s with a VNC client
    attached, 3 with single-threaded and 6 with multithreaded event
    handling, 390k to 818k traced syscalls each, no hang. A run with a
    Windows guest works as before. Compile-tested on staging.

 xen/arch/x86/mm/altp2m.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/xen/arch/x86/mm/altp2m.c b/xen/arch/x86/mm/altp2m.c
index 0bc9b9a..8c3450c 100644
--- a/xen/arch/x86/mm/altp2m.c
+++ b/xen/arch/x86/mm/altp2m.c
@@ -220,6 +220,17 @@ bool p2m_switch_vcpu_altp2m_by_id(struct vcpu *v, unsigned 
int idx)
     if ( idx >= d->nr_altp2m )
         return rc;
 
+    /*
+     * Updating another vCPU's EPTP (vmx_vmcs_enter()) waits until that vCPU
+     * is off its pCPU. Wait for it before taking the altp2m list lock: the
+     * vCPU may need the p2m lock to get off its pCPU (to update its runstate
+     * area in guest memory), while a vCPU handling a log-dirty fault holds the
+     * p2m lock and waits for the altp2m list lock in
+     * p2m_altp2m_propagate_change().
+     */
+    if ( v != current )
+        vcpu_pause(v);
+
     altp2m_list_lock(d);
 
     if ( d->arch.altp2m_eptp[idx] != mfn_x(INVALID_MFN) )
@@ -230,6 +241,10 @@ bool p2m_switch_vcpu_altp2m_by_id(struct vcpu *v, unsigned 
int idx)
     }
 
     altp2m_list_unlock(d);
+
+    if ( v != current )
+        vcpu_unpause(v);
+
     return rc;
 }
 
-- 
2.56.0