From 3688aac03c05b96b58ba5bf901d4fbb4d0fa8002 Mon Sep 17 00:00:00 2001 From: Arthur Hanson Rasmusson Date: Fri, 9 Oct 2026 02:09:40 -0400 Subject: vgsp-sema: synchronize the session flush with channel registration; stop sleeping in the VA-space walker (ISSUE-0002) Two host-side defects from the r9 capture: 1. The session-end flush was executed at the top of svc_rebuild without checking whether the next session had already started registering: in r9 it ran between the second client's first and later channel registrations and wiped its freshly learned state mid-setup. The flush now runs on svc_thread only (the sole writer of the discovery state), at the top of its loop; a first channel of a new session waits on svc_sess_flush_done before registering, so the wipe can never land after that session's state exists and the new session still starts from a clean slate. Scans abort on the pending flag so the wait is bounded, and channel_del wakes svc_thread so the flush runs promptly instead of waiting out a pass. 2. svc_walk_vaspaces held svc.vas_lock (spinlock) across svc_gmmu_walk, whose guest reads go through vfio_dma_rw (mutex, may sleep) - the 'BUG: scheduling while atomic' in the r9 service log. The walker now snapshots the published VA spaces under the lock and walks unlocked. --- kernel/gsp_sema_service.c | 88 ++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 72 insertions(+), 16 deletions(-) (limited to 'kernel/gsp_sema_service.c') diff --git a/kernel/gsp_sema_service.c b/kernel/gsp_sema_service.c index 369f854..9e3b14c 100644 --- a/kernel/gsp_sema_service.c +++ b/kernel/gsp_sema_service.c @@ -407,6 +407,7 @@ static u32 svc_ring_n; */ static u8 svc_sess_flush; static u8 svc_sess_active; /* a channel of a session has registered (until flush) */ +static struct completion svc_sess_flush_done; /* a pending flush has executed */ static unsigned long svc_ring_done; /* CE ranks whose ring is pinned (session state) */ static u8 svc_pb_passes; static u64 svc_pb_last; @@ -863,6 +864,26 @@ void gsp_sema_service_channel_add(const struct sema_channel_desc *desc) if (!desc->engine_type) WRITE_ONCE(svc_gr_content_rearm, true); + /* + * ISSUE-0002: a session-end flush may still be pending on svc_thread. + * It must not execute after this session's state exists (r9: the + * deferred flush ran between the first and second channel + * registrations of the next client and wiped its fresh state), and + * this session must not inherit the previous client's discovery + * state either: wait for the flush to complete before registering. + * add/del are serialized on this RPC thread, so no re-arm is + * possible here. + */ + if (READ_ONCE(svc_sess_flush)) { + reinit_completion(&svc_sess_flush_done); + if (READ_ONCE(svc_sess_flush)) { + if (svc.task) + wake_up_process(svc.task); + if (!wait_for_completion_timeout(&svc_sess_flush_done, + msecs_to_jiffies(5000))) + pr_warn_ratelimited("sema-svc: session flush did not complete; registering anyway\n"); + } + } spin_lock_irqsave(&svc.chan_lock, flags); for (i = 0; i < SVC_MAX_CHANNELS; i++) { if (svc.chan[i].valid && svc.chan[i].h_object == desc->h_object) { @@ -1005,6 +1026,10 @@ void gsp_sema_service_channel_del(u32 h_object) } spin_unlock_irqrestore(&svc.chan_lock, flags); + /* ISSUE-0002: let svc_thread execute a requested flush promptly. */ + if (svc_sess_flush && svc.task) + wake_up_process(svc.task); + /* * P6b wave 1.5: a deleted channel's held releases must not wait out * the timeout; mark the hold aborted so the pump applies them (or @@ -1320,30 +1345,42 @@ static int svc_walk_vaspaces(struct nv_vgpu_device *vgpu, u64 va, u64 *addr, int *ap) { unsigned long flags; - int i, rc = -ENOENT; + struct svc_vaspace local[SVC_MAX_VASPACES]; + int i, n = 0, rc = -ENOENT; bool trace = svc.walks_ok + svc.walks_miss < 24; + /* + * Snapshot the published VA spaces under the lock, then walk + * unlocked: svc_gmmu_walk reads guest memory through VFIO + * (vfio_dma_rw takes a mutex and may sleep), so it must never run + * with svc.vas_lock held (the "scheduling while atomic" BUG in the + * r9 service log). + */ spin_lock_irqsave(&svc.vas_lock, flags); for (i = 0; i < SVC_MAX_VASPACES; i++) { + if (svc.vas[i].valid) + local[n++] = svc.vas[i]; + } + spin_unlock_irqrestore(&svc.vas_lock, flags); + + for (i = 0; i < n; i++) { int alt; - if (!svc.vas[i].valid) - continue; if (va >> SVC_MMU_VA_BITS) break; if (trace) pr_info("sema-walk[%d]: va=0x%llx pdb=0x%llx entries=%u\n", - i, va, svc.vas[i].pdb, svc.vas[i].entries); + i, va, local[i].pdb, local[i].entries); /* * Try the aperture the driver named first, then the other one: * the aperture encoding has changed between driver generations * and a wrong guess only costs a rejected walk. */ - alt = svc.vas[i].ap == SVC_APERTURE_FB ? SVC_APERTURE_SYS - : SVC_APERTURE_FB; - if (!svc_gmmu_walk(vgpu, svc.vas[i].pdb, svc.vas[i].entries, va, - svc.vas[i].ap, addr, ap, trace) || - !svc_gmmu_walk(vgpu, svc.vas[i].pdb, svc.vas[i].entries, va, + alt = local[i].ap == SVC_APERTURE_FB ? SVC_APERTURE_SYS + : SVC_APERTURE_FB; + if (!svc_gmmu_walk(vgpu, local[i].pdb, local[i].entries, va, + local[i].ap, addr, ap, trace) || + !svc_gmmu_walk(vgpu, local[i].pdb, local[i].entries, va, alt, addr, ap, trace)) { svc.walks_ok++; rc = 0; @@ -1352,7 +1389,6 @@ static int svc_walk_vaspaces(struct nv_vgpu_device *vgpu, u64 va, } if (rc) svc.walks_miss++; - spin_unlock_irqrestore(&svc.vas_lock, flags); return rc; } @@ -2704,6 +2740,8 @@ static void svc_scan_pass(struct nv_vgpu_device *vgpu, u8 *buf, for (gpa = 0; gpa < SEMA_PA_MAX; gpa += SEMA_PAGE_SIZE) { u32 off; + if (READ_ONCE(svc_sess_flush)) + break; /* ISSUE-0002: a flush is waiting on this scan */ if (svc_read(vgpu, gpa, buf, SEMA_PAGE_SIZE)) continue; @@ -2939,13 +2977,15 @@ static void svc_rebuild(struct nv_vgpu_device *vgpu) u32 old_n; /* - * ISSUE-0002: executed only between client sessions (the last - * channel has closed), so no live session loses translations. + * ISSUE-0002: the session-end flush is executed by svc_thread at the + * top of its loop, the only writer of the discovery state; a channel + * registration waits for it (channel_add). It must never run from + * here: a scan pass racing the wipe could repopulate stale pairs + * after it. A flush requested while this rebuild scans (channel_del + * runs on the RPC thread) aborts the scan below. */ - if (svc_sess_flush) { - svc_sess_flush = 0; - svc_session_flush(); - } + if (READ_ONCE(svc_sess_flush)) + return; if (!buf) { buf = kmalloc(SEMA_PAGE_SIZE, GFP_KERNEL); @@ -6841,6 +6881,8 @@ static void svc_pb_discover(struct nv_vgpu_device *vgpu) for (gpa = 0; gpa < SEMA_PA_MAX; gpa += SVC_SCAN_CHUNK) { u32 o; + if (READ_ONCE(svc_sess_flush)) + break; /* ISSUE-0002: a flush is waiting on this scan */ if (svc_read(vgpu, gpa, buf, SVC_SCAN_CHUNK)) continue; for (o = 0; o < SVC_SCAN_CHUNK; o += SEMA_PAGE_SIZE) @@ -13516,6 +13558,19 @@ static int svc_thread(void *arg) while (!kthread_should_stop() && !svc.stop) { bool pumped = false; + /* + * ISSUE-0002: a session-end flush runs here, on the only + * writer of the discovery state, before any pass of this + * iteration; the next session's first channel waits for this + * completion before it registers (channel_add), so the flush + * can never execute after that session's state exists. + */ + if (READ_ONCE(svc_sess_flush)) { + svc_sess_flush = 0; + svc_session_flush(); + complete_all(&svc_sess_flush_done); + } + vgpu = NULL; mutex_lock(&nv_vgpu_driver.lock); { @@ -13705,6 +13760,7 @@ int gsp_sema_service_start(void) return 0; svc.stop = false; init_completion(&svc.stopped); + init_completion(&svc_sess_flush_done); spin_lock_init(&svc.chan_lock); spin_lock_init(&svc.vas_lock); /* -- cgit v1.2.3