Repository navigation
Conversation
…se finish Old claim (CPU reads behind an address-based build, libSceAgcDriver.md): the count record of the copy kernel HLE is one of the CPU reads that do not finish the batches recorded with a lease, so it can see guest memory from before such a build ran, and it is guarded only by its shape check. What the code does at upstream/main 50843f4: the indexed copy branch calls Graphics::FinishLeaseWork() before it checks the record (accessibility, snapshot and flush checks) and before it reads the count, source and destination offsets from it. https://github.com/boykopovar/AnyPS5/blob/50843f45d95b60e5772f4396431f9a39b8709d72/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Memory/BufferCopy.cpp#L197 https://github.com/boykopovar/AnyPS5/blob/50843f45d95b60e5772f4396431f9a39b8709d72/core/libs/prx/libSceAgcDriver/Execution/src/Driver/Memory/BufferCopy.cpp#L207-L209 The claim is removed from that entry. The other reads listed there are unchanged: Pm4::ReadDrawArguments and Pm4::ReadDrawCount are not preceded by a lease finish (Draw.cpp), and the awaitsTitle and orderReleased checks in Submission.cpp do not call one either. The entry is also shortened to under 1000 characters, the wording of those two checks only, with no change of meaning.
Collaborator
|
@MarcZX10 reviewed at The other branch of Could the entry keep that read, named as the non-indexed count record, and drop only the indexed copy record? The shortened wording of the |
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
MarcZX10
commented
Nov 2, 2026
MarcZX10
left a comment
MarcZX10
left a comment
Contributor
Author
There was a problem hiding this comment.
Reviewed — reply answered, the count record is read without a lease finish
oneandonlydean's 2026-10-30 reply shows that the 8-byte copy count record is read without FinishLeaseWork, and the code at this head confirms it, with one correction to the reply's call list.
What I ran
Live head 256fc34e23a533ef86aba3fe9e93033a990b29c3 (matches the batch file). Read the comment with gh api, the check rollup with gh pr view --json statusCheckRollup, the base doc diff with git diff 73f4f7e… 256fc34… -- docs/dev/debt/libSceAgcDriver.md, and the sources with git -C /marc/AnyPS5 show 256fc34…:<path>. Nothing was built or run.
Results
| Check | Result | Note |
|---|---|---|
| eligibility | SUCCESS | hosted |
| conventions | SUCCESS | hosted |
| intake, labels | SUCCESS | hosted |
| linux, windows, relinker, relinker-asan, write-watch, register-context | SKIPPED | hosted, doc-only change |
Findings
The reply (2026-10-30T19:32:01Z) asks to restore the count-record read to the entry. The code supports the claim.
FinishLeaseWork()is called incopyBufferonly in the indexed branch (core/libs/prx/libSceAgcDriver/Execution/src/Driver/Memory/BufferCopy.cppline 196-197).- The non-indexed branch (lines 218-229) reads the 8-byte count and period with no lease finish.
copyBufferis called fromDispatch.cppline 162 only whenindirectArguments == 0. - The base doc (73f4f7e) named "the count record of the copy kernel HLE" in the CPU-reads entry; doc line 181 at head no longer does.
Two corrections:
- The reply says the only driver calls to
FinishLeaseWorkare line 197 andIndirectDispatch.cpp.VulkanDevice.cppline 5182 also calls it, in the indirect-argument read, not on thecopyBufferpath. - If the old "guarded only by its shape check" wording returns, note that lines 222-226 check accessibility, pending writes and labels before the read. The shape check is at line 232, after it.
Sources
AnyPS5/core/libs/prx/libSceAgcDriver/Execution/src/VulkanDevice.cpp
Lines 5176 to 5182 in 256fc34
AnyPS5/docs/dev/debt/libSceAgcDriver.md
Line 181 in 256fc34
Not verified
- Whether the count record can see stale guest memory on hardware, or whether any title reads it there; the reply's trace is code reading only.
- Nothing was built or run.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Documentation correction in
docs/dev/debt/libSceAgcDriver.md, entry "CPU reads behind an address-based build" (second half of the file, audited at upstream/main50843f45d95b60e5772f4396431f9a39b8709d72).The entry said the count record of the copy kernel HLE is a CPU read that does not finish the batches recorded with a lease. The indexed copy branch of
BufferCopy.cppcallsGraphics::FinishLeaseWork()before it reads that record, so the claim is removed. The other reads listed in the entry are unchanged. The entry is also shortened to under 1000 characters.One commit,
docs(libSceAgcDriver): copy kernel count record is read after the lease finish.Overlap:
tools/pr_overlap.pylists open PRs that changedocs/dev/debt/libSceAgcDriver.md(for example #13471, #13474, #13484, #13499, #13373 and others). Each of them changes other entries. None edits the corrected entry (checked against the diff of each). #12840 shows the entry only as unchanged context. Permalinks in the commit body are pinned to the audited SHA.Tested
Docs only. Not built, no runtime tests.
Run on branch
docs/debt-D12-1(head256fc34e23a533ef86aba3fe9e93033a990b29c3, base50843f45d95b60e5772f4396431f9a39b8709d72):python3 tools/check_conventions.py --base upstream/main: "All conventions checks passed."git diff --check upstream/main: clean.python3 tools/env_reference.py --check: fails with the same single line as on a clean upstream/main run (identical output, the known baseline failure ofAPS5_PAD_SPEAKER_FALLBACK_DB).NOT RUN:
tools/pr_precheck.pytest_quality_heuristics: reported as failed for docs-only PRs (harness limitation per RULES.md, the check points at a.mdfile). Raw failure line:/usr/bin/python3: can't open file '/marc/agent-work/D12/work/precheck/head-256fc34e23a5-693d8407/tools/pr_test_quality.py': [Errno 2] No such file or directoryChecklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)