From d8749ae001d1df02e5c84bfadfc5e78b5271830a Mon Sep 17 00:00:00 2001 From: Arthur Hanson Rasmusson Date: Wed, 21 Oct 2026 23:35:51 -0400 Subject: r444 copy-snapshot ownership: key the fail4 held-copy staging snapshot by (channel, ring entry, push-content hash), not (dst,src,total) alone The wedge_ab deterministic reproducer (bad = mutated-element count) root cause: a held CE copy's source snapshot was captured for one ring entry, left behind when that entry's copy executed through a path that never consumed it (hardware-primary forwarded copy, or a post-execution re-decode whose copy was re-held), and then consumed by a LATER entry's copy that happens to share the (dst,src,total) triple - the client's cudaMemcpy released early with the PREVIOUS step's bytes in the device buffer, and the next GR launch computed on stale inputs (r441/r442/r443 evidence: step N read step N-1 content; sema-bigcopy src16 shows the fresh live source at execution time while dst16_after shows the old snapshot's bytes). Owner identity is (ch->h_object, entry_idx, svc_pass_hash); the pass hash separates a ring slot reused by a fresh push from the entry that captured the snapshot. svc_opdef_clear() additionally frees any snapshot still owned by a consumed entry, so no snapshot can outlive its entry. --- kernel/gsp_sema_service.c | 123 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 99 insertions(+), 24 deletions(-) diff --git a/kernel/gsp_sema_service.c b/kernel/gsp_sema_service.c index 931669c..b90c948 100644 --- a/kernel/gsp_sema_service.c +++ b/kernel/gsp_sema_service.c @@ -4820,7 +4820,8 @@ static int svc_fill_1d(struct nv_vgpu_device *vgpu, u64 addr, u64 value, u64 *done_out); static int svc_copy_1d(struct nv_vgpu_device *vgpu, u64 dst, u64 src, size_t total, u32 dst_phys, u32 dst_aperture, - u32 src_phys, u32 src_aperture, u64 *done_out); + u32 src_phys, u32 src_aperture, u64 *done_out, + u32 owner_ch, u32 owner_idx, u64 owner_hash); /* * r402 row-wise wrapper: a 2D (pitched) fill runs the 1D fill once per @@ -5938,12 +5939,29 @@ static void svc_copy_audit_chunk(struct nv_vgpu_device *vgpu, u64 dst_va, struct svc_copy_snap { u64 dst, src; size_t total; + /* + * Owner identity (r444): the channel, ring entry and push-content + * hash of the decode pass that captured this snapshot. A snapshot + * exists to serve ONE entry's retried copy (the held staging read); + * keying it by (dst,src,total) alone let a snapshot captured for an + * earlier entry poison a later copy with the same operand triple - + * the wedge_ab "bad = mutated-element count" class: step N's H2D + * executed from step N-1's snapshot because the re-decode of the + * completed step N-1 entry re-captured its (unchanged) source after + * its copy had already run, and step N's copy then consumed it. + * The pass hash changes whenever the guest rewrites the push, so a + * ring slot reused by a fresh push can never match a stale owner. + */ + u64 owner_hash; + u32 owner_ch; + u32 owner_idx; u8 *buf; unsigned long stamp; }; static struct svc_copy_snap svc_copy_snaps[SVC_COPY_SNAP_N]; -static u8 *svc_copy_snap_lookup(u64 dst, u64 src, size_t total) +static u8 *svc_copy_snap_lookup(u64 dst, u64 src, size_t total, + u32 owner_ch, u32 owner_idx, u64 owner_hash) { u32 i; @@ -5951,12 +5969,16 @@ static u8 *svc_copy_snap_lookup(u64 dst, u64 src, size_t total) if (svc_copy_snaps[i].buf && svc_copy_snaps[i].dst == dst && svc_copy_snaps[i].src == src && - svc_copy_snaps[i].total == total) + svc_copy_snaps[i].total == total && + svc_copy_snaps[i].owner_ch == owner_ch && + svc_copy_snaps[i].owner_idx == owner_idx && + svc_copy_snaps[i].owner_hash == owner_hash) return svc_copy_snaps[i].buf; return NULL; } -static void svc_copy_snap_free(u64 dst, u64 src, size_t total) +static void svc_copy_snap_free(u64 dst, u64 src, size_t total, + u32 owner_ch, u32 owner_idx, u64 owner_hash) { u32 i; @@ -5964,16 +5986,41 @@ static void svc_copy_snap_free(u64 dst, u64 src, size_t total) if (svc_copy_snaps[i].buf && svc_copy_snaps[i].dst == dst && svc_copy_snaps[i].src == src && - svc_copy_snaps[i].total == total) { + svc_copy_snaps[i].total == total && + svc_copy_snaps[i].owner_ch == owner_ch && + svc_copy_snaps[i].owner_idx == owner_idx && + svc_copy_snaps[i].owner_hash == owner_hash) { kfree(svc_copy_snaps[i].buf); svc_copy_snaps[i].buf = NULL; return; } } +/* + * r444: drop every snapshot still owned by one entry. A snapshot that was + * not consumed by the entry's own copy execution (the copy ran via the + * forwarded hardware path, or a post-execution re-decode re-captured the + * unchanged staging) has no future consumer once the entry's release is + * delivered - leaving it behind let a LATER entry whose copy shares the + * (dst,src,total) triple execute from the stale bytes. + */ +static void svc_copy_snap_free_owner(u32 owner_ch, u32 owner_idx) +{ + u32 i; + + for (i = 0; i < SVC_COPY_SNAP_N; i++) + if (svc_copy_snaps[i].buf && + svc_copy_snaps[i].owner_ch == owner_ch && + svc_copy_snaps[i].owner_idx == owner_idx) { + kfree(svc_copy_snaps[i].buf); + svc_copy_snaps[i].buf = NULL; + } +} + /* Capture the source of a virtual-operand copy about to be held. */ static void svc_copy_snap_capture(struct nv_vgpu_device *vgpu, u64 dst, - u64 src, size_t total) + u64 src, size_t total, u32 owner_ch, + u32 owner_idx, u64 owner_hash) { struct svc_copy_snap *s = NULL; u8 *buf; @@ -5982,7 +6029,8 @@ static void svc_copy_snap_capture(struct nv_vgpu_device *vgpu, u64 dst, if (!total || total > SVC_COPY_SNAP_MAX) return; - if (svc_copy_snap_lookup(dst, src, total)) + if (svc_copy_snap_lookup(dst, src, total, owner_ch, owner_idx, + owner_hash)) return; /* already captured */ for (i = 0; i < SVC_COPY_SNAP_N; i++) { if (!svc_copy_snaps[i].buf) { @@ -6019,10 +6067,14 @@ static void svc_copy_snap_capture(struct nv_vgpu_device *vgpu, u64 dst, s->dst = dst; s->src = src; s->total = total; + s->owner_ch = owner_ch; + s->owner_idx = owner_idx; + s->owner_hash = owner_hash; s->stamp = jiffies; s->buf = buf; - pr_info_ratelimited("vgsp-sema: copy snapshot captured dst=0x%llx src=0x%llx total=%zu first4=%08x %08x %08x %08x\n", - dst, src, total, + pr_info_ratelimited("vgsp-sema: copy snapshot captured dst=0x%llx src=0x%llx total=%zu owner=0x%x/%u/%016llx first4=%08x %08x %08x %08x\n", + dst, src, total, owner_ch, owner_idx, + owner_hash, total >= 4 ? *(const u32 *)buf : 0, total >= 8 ? *(const u32 *)(buf + 4) : 0, total >= 12 ? *(const u32 *)(buf + 8) : 0, @@ -6039,7 +6091,8 @@ static const char *svc_gr_src_probe(struct nv_vgpu_device *vgpu, u64 va, u64 *pa_out, u32 *ap_out); static int svc_copy(struct nv_vgpu_device *vgpu, u64 dst, u64 src, size_t total, u32 dst_phys, u32 dst_aperture, u32 src_phys, u32 src_aperture, - u64 *done_out, u32 rows, u32 dpitch, u32 spitch) + u64 *done_out, u32 rows, u32 dpitch, u32 spitch, + u32 owner_ch, u32 owner_idx, u64 owner_hash) { if (rows > 1 && dpitch) { u64 done = 0; @@ -6052,7 +6105,8 @@ static int svc_copy(struct nv_vgpu_device *vgpu, u64 dst, u64 src, size_t total, rc = svc_copy_1d(vgpu, dst + (u64)r * dpitch, src + (u64)r * spitch, total, dst_phys, dst_aperture, src_phys, - src_aperture, &row_done); + src_aperture, &row_done, + owner_ch, owner_idx, owner_hash); done += row_done; if (rc) break; @@ -6062,12 +6116,13 @@ static int svc_copy(struct nv_vgpu_device *vgpu, u64 dst, u64 src, size_t total, return rc; } return svc_copy_1d(vgpu, dst, src, total, dst_phys, dst_aperture, - src_phys, src_aperture, done_out); + src_phys, src_aperture, done_out, + owner_ch, owner_idx, owner_hash); } static int svc_copy_1d(struct nv_vgpu_device *vgpu, u64 dst, u64 src, size_t total, u32 dst_phys, u32 dst_aperture, u32 src_phys, u32 src_aperture, - u64 *done_out) + u64 *done_out, u32 owner_ch, u32 owner_idx, u64 owner_hash) { u8 bounce[256]; size_t o; @@ -6098,7 +6153,8 @@ static int svc_copy_1d(struct nv_vgpu_device *vgpu, u64 dst, u64 src, size_t tot svc.cur_ch, dst, src, total, dst_phys, dst_aperture, src_phys, src_aperture); } - snap = svc_copy_snap_lookup(dst_orig, src_orig, total); + snap = svc_copy_snap_lookup(dst_orig, src_orig, total, owner_ch, + owner_idx, owner_hash); if (!dst_phys) { u64 pa; @@ -6232,7 +6288,8 @@ static int svc_copy_1d(struct nv_vgpu_device *vgpu, u64 dst, u64 src, size_t tot */ if (!src_phys && src_orig >= SVC_VAS_FLOOR) svc_copy_snap_capture(vgpu, dst_orig, src_orig, - total); + total, owner_ch, owner_idx, + owner_hash); return -ENOENT; } /* @@ -6261,7 +6318,9 @@ static int svc_copy_1d(struct nv_vgpu_device *vgpu, u64 dst, u64 src, size_t tot dst_orig, dst_prov, ipa); if (!src_phys && src_orig >= SVC_VAS_FLOOR) svc_copy_snap_capture(vgpu, dst_orig, - src_orig, total); + src_orig, total, + owner_ch, owner_idx, + owner_hash); return -ENOENT; } } @@ -7036,7 +7095,8 @@ static int svc_copy_1d(struct nv_vgpu_device *vgpu, u64 dst, u64 src, size_t tot svc_ledger_store(dst_orig + po, dst + po, cap); } if (snap) - svc_copy_snap_free(dst_orig, src_orig, total); + svc_copy_snap_free(dst_orig, src_orig, total, owner_ch, + owner_idx, owner_hash); if (done_out) *done_out = (u64)o; /* o == total on the success path */ return 0; @@ -7238,7 +7298,8 @@ static struct svc_wait_hold *svc_wait_hold_get(u32 h, bool create) } static void svc_run_ops(struct nv_vgpu_device *vgpu, struct svc_memset_op *ops, - u32 n_ops, u32 ch_h, u64 *ops_done, u32 ops_base) + u32 n_ops, u32 ch_h, u64 *ops_done, u32 ops_base, + u32 entry_idx, u64 pass_hash) { u32 i; static u64 bigop_log; @@ -7404,7 +7465,8 @@ svc_wait_done: m->phys, m->aperture, m->src_phys, m->src_aperture, &ce_done, m->rows, m->dpitch, - m->spitch); + m->spitch, ch_h, entry_idx, + pass_hash); else rc = svc_fill(vgpu, m->dst, m->value, m->elem, m->count, m->phys, m->aperture, @@ -9385,7 +9447,8 @@ static void svc_decode_stream(struct nv_vgpu_device *vgpu, const u8 *buf, * REPORT_SEM comment below. */ if (allow_ops && n_ops && !gsp_sema_skip_ops) { svc_run_ops(vgpu, ops, n_ops, ch->h_object, - ops_done, ops_seq - n_ops); + ops_done, ops_seq - n_ops, + entry_idx, svc_pass_hash); svc.ring_ops++; ops_runs++; } @@ -9585,7 +9648,8 @@ static void svc_decode_stream(struct nv_vgpu_device *vgpu, const u8 *buf, */ if (allow_ops && n_ops && !gsp_sema_skip_ops) { svc_run_ops(vgpu, ops, n_ops, ch->h_object, - ops_done, ops_seq - n_ops); + ops_done, ops_seq - n_ops, + entry_idx, svc_pass_hash); svc.ring_ops++; ops_runs++; } @@ -10295,7 +10359,10 @@ static void svc_decode_stream(struct nv_vgpu_device *vgpu, const u8 *buf, svc_copy_snap_capture(vgpu, st.offset_out, st.offset_in, - st.line_length); + st.line_length, + ch->h_object, + entry_idx, + svc_pass_hash); } /* * r308: the `!replay` gate above kept this @@ -10386,7 +10453,10 @@ static void svc_decode_stream(struct nv_vgpu_device *vgpu, const u8 *buf, svc_copy_snap_capture(vgpu, lm->dst, lm->src, - lm->bytes); + lm->bytes, + ch->h_object, + entry_idx, + svc_pass_hash); if (gsp_sema_dbg && svc.snap_gate_log < 256) { svc.snap_gate_log++; @@ -10707,7 +10777,8 @@ static void svc_decode_stream(struct nv_vgpu_device *vgpu, const u8 *buf, gsp_sema_skip_ops); } svc_run_ops(vgpu, ops, n_ops, ch->h_object, - ops_done, ops_seq - n_ops); + ops_done, ops_seq - n_ops, + entry_idx, svc_pass_hash); svc.ring_ops++; ops_runs++; } @@ -15805,6 +15876,10 @@ static void svc_opdef_clear(struct svc_opdef_ent *e, u32 caller) e->n ? e->rel[0].va : 0ull, e->n ? e->rel[0].payload : 0u); } + /* r444: the entry is being consumed; no retry can need its + * staging snapshot any more (see svc_copy_snap_free_owner). */ + if (e->active) + svc_copy_snap_free_owner(e->h_object, e->entry_idx); memset(e, 0, sizeof(*e)); } -- cgit v1.2.3