Skip to content

fix(agc): give copied guest buffer regions transfer-source usage - #7961

Merged
oneandonlydean merged 1 commit into
boykopovar:mainfrom
milescoviello:fix/copied-region-transfer-src
Oct 22, 2026
Merged

oneandonlydean merged 1 commit into
boykopovar:mainfrom
milescoviello:fix/copied-region-transfer-src

Conversation

@milescoviello

@milescoviello milescoviello commented Oct 19, 2026 •

Copy link
Copy Markdown
Contributor

What

A guest buffer region that is copied rather than imported gets a buffer with storage usage, plus device-address usage when it is addressable, and nothing else. Host imports, sparse imports and chunk imports also carry VK_BUFFER_USAGE_TRANSFER_SRC_BIT, so a buffer view of imported guest memory can be read back with vkCmdCopyBuffer, but a view of a copied region can't. agc_sparse_run_import_copied does exactly that readback. The Khronos validation layer reports VUID-vkCmdCopyBuffer-srcBuffer-00118 for it, which the sweep of main's ^agc_ tests under the layer turned up.

  • GuestBufferMemory::copyRegion now adds transfer-source usage to the copied region's buffer.
  • The sparse buffer hole tests seed the buffer pool with a buffer of the usage a copied region asks for, then check that the upload reuses it. Their seed now has transfer-source usage too, or the pool wouldn't hand it back.

Tested

Ubuntu 24.04, GCC 13.3, NVIDIA RTX 4070 Ti (595.99.02).

  • Khronos validation, as CONTRIBUTING.md asks: VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation ANYPS5_NO_SHADER_CACHE=1 ANYPS5_REQUIRE_VULKAN=1 ctest -V, ^agc_sparse_run_import, through VK_LAYER_PATH. Neither log reports the layer missing.
    • Ubuntu's 1.3.275 layer: main 6762694 reports VUID-vkCmdCopyBuffer-srcBuffer-00118 ×3; this change reports none.
    • The LunarG SDK's 1.4.363 layer, which Windows reviewers run: main reports 00118 ×2 over ^agc_sparse_run_import|sparse_buffer_holes; this change reports none.
  • The 44 tests matching ^agc_driver_recorder|sparse|import|guest_buffer|buffer_memory|bda|mirror|^agc_driver_graphics$ pass, as on main 6762694. Before the test seeds were updated, agc_driver_sparse_buffer_holes and agc_sparse_buffer_holes failed on the upload did not reuse the primed buffer, which shows the pool reuse they check is keyed on usage.
  • Merges with main b106f6f without conflicts.
  • @DavidAmsellem, on AMD (RX 7900 XT, RADV NAVI31, Mesa 25.2.8, Linux Mint 22.3, kernel 7.0, GCC 13.3), with 054cf42 merged onto main c614599: under Ubuntu's 1.3.275 validation layer, ^agc_sparse_run_import, agc_sparse_run_import_copied, agc_driver_sparse_buffer_holes and agc_driver_graphics report VUID-vkCmdCopyBuffer-srcBuffer-00118 ×1 on main and none with this PR, and pass 4 of 4 on both. The full suite gives 1735 of 1736 with and without this PR, with the same one failure (agc_driver_guest_memory_lifetime, agc_driver_guest_memory_lifetime fails on RADV since the host-import cut larger-side keep (16473dbbe, #6499 family) #7784) and no JUnit entry added, skipped or removed.
  • @xXPBSXx, on Windows 10 (RTX 4080 SUPER, WinLibs GCC 15.2.0, Release) at 054cf42: agc_driver_sparse_buffer_holes passes 3 of 3. With only Graphics/src/GuestBufferMemory.cpp from the merge base, the same test fails 3 of 3 (the upload did not reuse the primed buffer at the expected extent).
  • @DavidSamir, on Windows 11 (AMD Radeon 680M, driver 32.0.21030.2001, WinLibs GCC 15.2.0 r7, Release), ran all 875 agc_* tests under the 1.4.363 layer on main 27fabb6 and with this PR merged into it. On main, agc_sparse_run_import_copied logs VUID-vkCmdCopyBuffer-srcBuffer-00118 for copyRegion's buffer. With this PR, that error is gone and the validation output is otherwise identical. The same 6 tests fail on both sides, none of them related to this change.
  • @xXPBSXx, on lavapipe (WSL2 Ubuntu 24.04, GCC 13.3, Release) under Ubuntu's 1.3.275 layer, ran all 483 built agc_driver_*_tests on main 950388b: 00118 is reported once, only from agc_sparse_run_import_copied. With this change that test passes with no 00118. One run.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Fixes: none
  • Unblocks: none
  • GPUs tested: NVIDIA GeForce RTX 4070 Ti, driver 595.99.02, Ubuntu 24.04; AMD Radeon RX 7900 XT, RADV (Mesa 25.2.8), Linux Mint 22.3 (@DavidAmsellem); NVIDIA GeForce RTX 4080 SUPER, Windows 10 (@xXPBSXx); AMD Radeon 680M, Windows 11 (@DavidSamir); lavapipe (llvmpipe), WSL2 Ubuntu 24.04 (@xXPBSXx)
  • AI-assisted: yes (Claude Code)

🤖 Generated with Claude Code

A guest buffer region that is copied rather than imported gets a buffer
with storage (and, when addressable, device-address) usage only. Host
imports, sparse imports and chunk imports carry VK_BUFFER_USAGE_TRANSFER_SRC_BIT,
so a buffer view of imported guest memory can be read back with
vkCmdCopyBuffer, but not one of a copied region. agc_sparse_run_import_copied
does that readback, and the Khronos validation layer reports
VUID-vkCmdCopyBuffer-srcBuffer-00118 for it.

Copied regions now carry transfer-source usage too. The sparse buffer
hole tests seed the buffer pool with the usage a copied region asks for,
so their seed buffer gets transfer-source usage as well.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 19, 2026 •

Copy link
Copy Markdown

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.

Pull request Shared files not compared
PR 1191 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 2434 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 4448 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp
PR 4449 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 5021 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp
PR 5372 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp
PR 6146 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 6197 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp
PR 6292 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 6412 core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp
PR 6481 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 6548 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 6615 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 6918 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp
PR 7201 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
core/libs/prx/libSceAgcDriver/tests/BdaResources.cpp
PR 7457 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 7622 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 7642 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 8527 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 8672 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp
PR 9012 core/libs/prx/libSceAgcDriver/Graphics/src/GuestBufferMemory.cpp

@DavidAmsellem

Copy link
Copy Markdown
Contributor

First AMD run of this PR, on Linux with RADV. Head 054cf4220 merged onto main c6145997e (clean merge), built in full and compared with the same main, on the same machine.

AMD Radeon RX 7900 XT (RADV NAVI31, gfx1100), Mesa 25.2.8, Linux Mint 22.3 (Ubuntu 24.04 base), kernel 7.0.0-38, GCC 13.3, Release.

With the Khronos validation layer (Ubuntu's 1.3.275, ANYPS5_REQUIRE_VULKAN=1 ANYPS5_NO_SHADER_CACHE=1) over agc_sparse_run_import, agc_sparse_run_import_copied, agc_driver_sparse_buffer_holes and agc_driver_graphics:

main main + this PR
VUID-vkCmdCopyBuffer-srcBuffer-00118 1 0
tests passing 4 of 4 4 of 4

Full suite (ANYPS5_REQUIRE_VULKAN=1 ctest -j8) at 07a5d3d16: 1735/1736 with this PR, against 1735/1736 on main. The one failure is the same on both, so this PR adds none: agc_driver_guest_memory_lifetime (#7784). Comparing the JUnit output with main, no entry is added, skipped or removed.

GPUs tested: AMD Radeon RX 7900 XT (RADV NAVI31, gfx1100), Mesa 25.2.8, Linux Mint 22.3 / kernel 7.0.0-38

AI-assisted: yes

@milescoviello

Copy link
Copy Markdown
Contributor Author

Thanks @DavidAmsellem, that's the first AMD coverage for this one. I added your run to the description: the VUID goes from 1 to 0 under RADV, and the full suite has no new failure or skip.

AI-assisted: yes (Claude Code)

@xXPBSXx

xXPBSXx commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Independent Windows run of this PR with a baseline (data only).

Environment: Windows 10 Pro 10.0.19045, Ryzen 7 5800X3D, NVIDIA GeForce RTX 4080 SUPER (the log shows Selected GPU name=NVIDIA GeForce RTX 4080 SUPER vendor=0x10de device=0x2702 subgroup=32, picked with ANYPS5_GPU=4080), WinLibs GCC 15.2.0 (MinGW-w64 UCRT), Release. PR head 054cf42202; merge-tree against main is clean. I built agc_driver_sparse_buffer_holes_tests only.

Results (3 runs each):

  • PR head: agc_driver_sparse_buffer_holes passes 3/3.
  • Baseline: I kept the PR's tests/execution/SparseBufferHoles.cpp and replaced only Graphics/src/GuestBufferMemory.cpp with its version from the merge base 6762694735, then rebuilt. The test fails 3/3 with AGC graphics: sparse buffer holes GPU: readable interior: the upload did not reuse the primed buffer at the expected extent. I restored the PR's GuestBufferMemory.cpp afterwards.

Not run: other GPUs (no AMD card for this PR), other platforms, the other tests, the full repository CTest, and any title or console run.

AI-assisted (Claude Code).

@milescoviello

Copy link
Copy Markdown
Contributor Author

Thanks @xXPBSXx. The fail on the merge base's GuestBufferMemory.cpp with the PR's test is the check I didn't have for Windows. I added your run to the Tested section and the GPU line.

AI-assisted: yes (Claude Code)

@DavidSamir

Copy link
Copy Markdown
Contributor

Independent run with the Khronos validation layer on AMD. It confirms the layer-reported error this PR removes, with no other change across the AGC tests.

Setup: Windows 11 Pro 26200, AMD Radeon 680M (RDNA 2 iGPU), AMD driver 32.0.21030.2001, WinLibs GCC 15.2.0 r7, Release, full build. VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation (SDK 1.4.363), ANYPS5_REQUIRE_VULKAN=1, ANYPS5_NO_SHADER_CACHE=1. main is 27fabb6b7; the PR is this head merged into that same commit. ctest -R '^agc_' -j 4 -V (875 tests) on both.

  • main: agc_sparse_run_import_copied logs VUID-vkCmdCopyBuffer-srcBuffer-00118: srcBuffer ... was created with VK_BUFFER_USAGE_2_STORAGE_BUFFER_BIT|VK_BUFFER_USAGE_2_SHADER_DEVICE_ADDRESS_BIT but requires VK_BUFFER_USAGE_2_TRANSFER_SRC_BIT. The buffer is copyRegion's.
  • This PR: that error is gone. agc_sparse_run_import_copied, agc_driver_sparse_buffer_holes and agc_sparse_buffer_holes pass.
  • Across all 875 agc_* tests, the validation errors are identical on both sides except for the one removed srcBuffer-00118. The same 6 tests fail on both (agc_reimport_bench, agc_driver_tessellation, agc_driver_sample_order and two of its variants, and agc_driver_image_array_window_past_layer_limit, which doesn't build on Windows). These failures are known on main on this device and unrelated to this change.
  • The layer loaded on both sides (no was not found but was requested by env var VK_INSTANCE_LAYERS).

For reference, the GPU-copy path for the same Region::buffer (stagingBuffer(..., gpuCopyUsage(addressable)) and the Buffer made with gpuCopyUsage(addressable)) already carries TRANSFER_SRC|TRANSFER_DST, so this change brings copyRegion in line with it.

AI-assisted: yes

@milescoviello

Copy link
Copy Markdown
Contributor Author

Thanks @DavidSamir, that's the first AMD run of this PR under the layer, and the first across the whole agc_* suite. It shows exactly the one srcBuffer-00118 removed and nothing else changed. I added it to the Tested section and the GPU line.

About the unrelated failures: agc_driver_sample_order and two of its variants fail on main on my RTX 4070 Ti as well. There the cause is in the tests: they bind a multisampled depth surface's own view as a sampled image, and a surface sampled through its shadow image has no SAMPLED usage (VUID-VkWriteDescriptorSet-descriptorType-00337). #9056 makes them read a sampled copy instead, and then they pass here. If those are the variants failing on the 680M, #9056 may fix them there too.

AI-assisted: yes (Claude Code)

@xXPBSXx

xXPBSXx commented Oct 21, 2026

Copy link
Copy Markdown
Contributor

Validation-layer data point for this PR (data only; follow-up to my earlier RTX note).

Setup: Linux (WSL2 Ubuntu 24.04, GCC 13.3, Release), llvmpipe, vulkan-validationlayers 1.3.275.0-1 from the Ubuntu archive (older than the SDK a driver ships with), VK_INSTANCE_LAYERS=VK_LAYER_KHRONOS_validation. main 950388bfd; this PR's three-line change (taken as git diff upstream/main...refs/pr/7961) applied on top of it.

Before (main): across all 483 built agc_driver_*_tests the layer reports VUID-vkCmdCopyBuffer-srcBuffer-00118 once, from agc_sparse_run_import_copied: "srcBuffer ... was created with VK_BUFFER_USAGE_2_STORAGE_BUFFER_BIT_KHR|VK_BUFFER_USAGE_2_SHADER_DEVICE_ADDRESS_BIT_KHR but requires VK_BUFFER_USAGE_TRANSFER_SRC_BIT".

After (this PR): agc_sparse_run_import_copied run again with the layer: 0 reports of that VUID, test passes.

One run each, on llvmpipe only. I did not run the other tests with the change under the layer.

Not run: other GPUs for the layer run, other platforms.

AI-assisted (Claude Code).

@milescoviello

Copy link
Copy Markdown
Contributor Author

Thanks @xXPBSXx. That confirms agc_sparse_run_import_copied is the only test in the 483 that hits 00118 on main. I added the run to the Tested section and lavapipe to the GPU line.

AI-assisted: yes (Claude Code)

@Scramblrrrr

Copy link
Copy Markdown
Contributor

Windows/NVIDIA reproduction on current main d6eafa5 AGC code (our only additional initial change was SystemService parameter100): agc_sparse_run_import_copied passes its output assertion but emits00118 under official Khronos1.4.363. The log shows the copied source buffer has STORAGE_BUFFER|SHADER_DEVICE_ADDRESS and lacks TRANSFER_SRC. We found your existing fix while checking this; no duplicate implementation or issue opened.

I have merged published054cf42202 cleanly into the next 007 WIP alongside #9565 atc4162b2156. It is source-only/UNBUILT: our pinned current-main 007 baseline is still compiling changed pipelines, so no concurrent builds/native tests. I will run the sparse copied/imported and buffer-hole controls with the same layer before any runtime change. No Windows fix-pass or FPS claim yet. AI-assisted: Codex.

@oneandonlydean
oneandonlydean merged commit 7e2ba73 into boykopovar:main Oct 22, 2026
19 checks passed
@oneandonlydean

Copy link
Copy Markdown
Collaborator

@milescoviello, merged, thanks. Full CI on 054cf42 was green on Linux and Windows, and agc_sparse_run_import_copied and agc_driver_sparse_buffer_holes pass on Linux. Thanks also to @DavidAmsellem, @xXPBSXx, @DavidSamir and @Scramblrrrr for the runs on RADV, NVIDIA and AMD under the validation layer.

@milescoviello

Copy link
Copy Markdown
Contributor Author

@oneandonlydean thanks for merging. @Scramblrrrr thanks for the Windows/NVIDIA reproduction under the 1.4.363 layer: the copied source buffer lacking TRANSFER_SRC on current main is exactly the case this fixes.

AI-assisted: yes (Claude Code)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

agc libSceAgc*

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants