From 81d6f94ceb511ce7e4bbd42d0ee2df2552de4bc0 Mon Sep 17 00:00:00 2001 From: Dean Galvin Date: Thu, 1 Oct 2026 16:09:16 +1300 Subject: [PATCH] fix(agc): patch the memory offsets of misaligned guest buffers in the shader data buffer A guest buffer view off minStorageBufferOffsetAlignment is bound from the aligned offset below it, and the difference goes into the shader's memory byte offsets (3dec6ced), but only when those live in the push constants: a shader whose shader data is a buffer failed with "a guest buffer off the storage buffer offset alignment in a shader whose shader data is a buffer is not implemented". The adjustment now goes into the shader data buffer's byte at memoryOffsetDword * 4 + element, and a template's data refresh writes the dispatch's words with the same patches applied. --- .../Graphics/include/ShaderResources.hpp | 9 +++++ .../Graphics/src/ShaderResources.cpp | 35 +++++++++++++++-- .../prx/libSceAgcDriver/tests/Graphics.cpp | 39 ++++++++++++++++++- 3 files changed, 79 insertions(+), 4 deletions(-) diff --git a/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp b/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp index eaf43678a..727c4b3ec 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/include/ShaderResources.hpp @@ -242,7 +242,15 @@ private: std::vector dataWords; std::uint32_t adjustment = 0; std::int32_t pushByte = -1; + std::int64_t dataAllocation = -1; + std::uint32_t dataByte = 0; }; + struct DataPatch { + std::size_t allocation; + std::uint32_t byte; + std::uint32_t adjustment; + }; + void writeDataWords(VkCommandBuffer commands, std::size_t allocation, std::span words) const; struct Binding { VkDescriptorSetLayoutBinding layout; @@ -406,6 +414,7 @@ private: std::uint64_t dataWordsHash = 14695981039346656037ull; void rehashDataWords(); std::vector> pushPatches; + std::vector dataPatches; BuildTiming timing; // Build state carried from stage A to stage B: the bindings in plan order, the image bindings // still to look up (index into `bindings`; the DescriptorBinding lives in the compiled shader), diff --git a/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp b/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp index c4b2930f9..7c33fddf1 100644 --- a/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp +++ b/core/libs/prx/libSceAgcDriver/Graphics/src/ShaderResources.cpp @@ -816,6 +816,8 @@ void ShaderResources::buildPrepare(std::span shaders, cons Require(shader.program != nullptr, "missing compiled shader"); const VkShaderStageFlags flags = VulkanStage(shader.stage); std::uint64_t stageDescriptors = 0; + std::vector offsetsInData; + std::int64_t shaderData = -1; for (const auto& binding : shader.program->bindings) { Require(binding.descriptorSet == 0, "unexpected descriptor set: every shader resource must use descriptor set zero"); Require(occupied.insert(binding.binding).second, "duplicate shader binding"); @@ -847,6 +849,9 @@ void ShaderResources::buildPrepare(std::span shaders, cons const auto position = shader.program->memoryOffsetDword * 4u + element; Require(position < push.size(), "guest buffer offset lies outside the shader's push constants"); allocations[index].pushByte = static_cast(shader.pushConstantOffset + position); + } else { + allocations[index].dataByte = shader.program->memoryOffsetDword * 4u + element; + offsetsInData.push_back(index); } item.allocations.push_back(index); } @@ -857,9 +862,11 @@ void ShaderResources::buildPrepare(std::span shaders, cons Require(binding.count == 1, "shader data and flattened SRT descriptors must not be arrays"); Require(!binding.guestDescriptor.empty(), "empty shader data descriptor"); item.allocations.push_back(addDataBuffer(binding.guestDescriptor)); + if (binding.role == ShaderRecompiler::DescriptorRole::ShaderData) shaderData = static_cast(item.allocations.back()); } bindings.push_back(std::move(item)); } + for (const auto index : offsetsInData) allocations[index].dataAllocation = shaderData; } Require(storageBuffers <= context.limits.maxDescriptorSetStorageBuffers, "pipeline descriptors exceed device limits"); timing.bindingsMs = phase(BuildPhase::Bindings); @@ -988,8 +995,15 @@ void ShaderResources::buildComplete() { } for (const auto& allocation : allocations) { if (allocation.adjustment == 0) continue; - Require(allocation.pushByte >= 0, "a guest buffer off the storage buffer offset alignment in a shader whose shader data is a buffer is not implemented"); - pushPatches.emplace_back(static_cast(allocation.pushByte), allocation.adjustment); + if (allocation.pushByte >= 0) { + pushPatches.emplace_back(static_cast(allocation.pushByte), allocation.adjustment); + continue; + } + Require(allocation.dataAllocation >= 0, "a guest buffer off the storage buffer offset alignment in a shader without shader data is not implemented"); + auto& data = allocations[static_cast(allocation.dataAllocation)]; + Require(data.buffer != nullptr && allocation.dataByte < data.size, "guest buffer offset lies outside the shader's data buffer"); + data.buffer->Bytes()[allocation.dataByte] = static_cast(allocation.adjustment); + dataPatches.push_back({static_cast(allocation.dataAllocation), allocation.dataByte, allocation.adjustment}); } timing.descriptorsMs += phase(BuildPhase::Descriptors); noteReusable(); @@ -2188,7 +2202,7 @@ bool ShaderResources::RefreshData(VkCommandBuffer commands, const CompiledShader Require(allocation.buffer != nullptr && !allocation.guest && allocation.size == size && size <= MaxRefreshBytes, "template data buffer cannot take the dispatch's words"); if (allocation.dataWords.size() == binding.guestDescriptor.size() && std::equal(allocation.dataWords.begin(), allocation.dataWords.end(), binding.guestDescriptor.begin())) continue; if (recorder != nullptr && timing == Recorder::NoTiming) timing = recorder->BeginGpuTiming(Recorder::CommandClass::TemplateDataRefresh); - context.Resolved(&DeviceFunctions::cmdUpdateBuffer, "vkCmdUpdateBuffer")(commands, allocation.buffer->Handle(), 0, size, binding.guestDescriptor.data()); + writeDataWords(commands, bindings[index].allocations.front(), binding.guestDescriptor); allocation.dataWords.assign(binding.guestDescriptor.begin(), binding.guestDescriptor.end()); refreshedBytes += size; recorded = true; @@ -2198,6 +2212,21 @@ bool ShaderResources::RefreshData(VkCommandBuffer commands, const CompiledShader return recorded; } +void ShaderResources::writeDataWords(VkCommandBuffer commands, std::size_t allocation, std::span words) const { + const auto& buffer = *allocations[allocation].buffer; + const auto size = words.size() * sizeof(std::uint32_t); + if (std::none_of(dataPatches.begin(), dataPatches.end(), [&](const DataPatch& patch) { return patch.allocation == allocation; })) { + context.Resolved(&DeviceFunctions::cmdUpdateBuffer, "vkCmdUpdateBuffer")(commands, buffer.Handle(), 0, size, words.data()); + return; + } + std::vector patched(words.begin(), words.end()); + auto* bytes = reinterpret_cast(patched.data()); + for (const auto& patch : dataPatches) { + if (patch.allocation == allocation && patch.byte < size) bytes[patch.byte] = static_cast(patch.adjustment); + } + context.Resolved(&DeviceFunctions::cmdUpdateBuffer, "vkCmdUpdateBuffer")(commands, buffer.Handle(), 0, size, patched.data()); +} + void ShaderResources::PrecollectSurfaces() const { for (const auto& range : describedRanges) GuestMemory::CollectWrites(range.address, static_cast(range.bytes)); } diff --git a/core/libs/prx/libSceAgcDriver/tests/Graphics.cpp b/core/libs/prx/libSceAgcDriver/tests/Graphics.cpp index e6fb5904c..c987c55fc 100644 --- a/core/libs/prx/libSceAgcDriver/tests/Graphics.cpp +++ b/core/libs/prx/libSceAgcDriver/tests/Graphics.cpp @@ -566,6 +566,12 @@ VKAPI_ATTR void VKAPI_CALL mockCmdDispatch(VkCommandBuffer, std::uint32_t x, std mock.lastDispatchGroups = {x, y, z}; } +VKAPI_ATTR void VKAPI_CALL mockCmdUpdateBuffer(VkCommandBuffer, VkBuffer buffer, VkDeviceSize offset, VkDeviceSize size, const void* data) { + auto& memory = mock.memories.at(mock.bufferMemory.at(buffer)); + Require(offset + size <= memory.size(), "a buffer update exceeds its buffer"); + std::memcpy(memory.data() + offset, data, static_cast(size)); +} + PFN_vkVoidFunction VKAPI_CALL mockProc(VkDevice, const char* name) { static const std::map table{ {"vkGetBufferDeviceAddressKHR", reinterpret_cast(mockGetBufferDeviceAddress)}, @@ -592,7 +598,8 @@ PFN_vkVoidFunction VKAPI_CALL mockProc(VkDevice, const char* name) { {"vkDestroyPipeline", reinterpret_cast(mockDestroyPipeline)}, {"vkCmdBindPipeline", reinterpret_cast(mockCmdBindPipeline)}, {"vkCmdPushConstants", reinterpret_cast(mockCmdPushConstants)}, - {"vkCmdDispatch", reinterpret_cast(mockCmdDispatch)} + {"vkCmdDispatch", reinterpret_cast(mockCmdDispatch)}, + {"vkCmdUpdateBuffer", reinterpret_cast(mockCmdUpdateBuffer)} }; const auto it = table.find(name); return it == table.end() ? nullptr : it->second; @@ -849,6 +856,35 @@ void resourceTests() { } } +void misalignedShaderDataTests() { + mock = MockVulkan{}; + auto context = mockContext(); + context.limits.minStorageBufferOffsetAlignment = 16; + const auto commands = reinterpret_cast(std::uintptr_t{1}); + alignas(64) std::array guest{1, 2, 3, 4, 5, 6, 7, 8}; + { + ShaderRecompiler::RecompileResult compute; + compute.bindings.push_back(makeBinding(Role::GuestBuffers, 0, 2, join(vsharp(guest.data(), 32), vsharp(guest.data() + 1, 8)))); + compute.bindings.push_back(makeBinding(Role::ShaderData, 1, 1, {0x11, 0x22, 0})); + compute.memoryOffsetDword = 2; + auto live = compute; + live.bindings[1].guestDescriptor = {0x33, 0x44, 0}; + const AgcDriver::Graphics::CompiledShader shader{ShaderRecompiler::ShaderStage::Compute, &compute, 0}; + const AgcDriver::Graphics::CompiledShader liveShader{ShaderRecompiler::ShaderStage::Compute, &live, 0}; + AgcDriver::Graphics::ShaderResources resources(context, shader); + const auto& views = findWrite(0); + Require(views.buffers.size() == 2 && views.buffers[0].range == 32 && views.buffers[1].range == 12 && views.buffers[1].offset == views.buffers[0].offset, "the misaligned view is not bound from the aligned offset below it"); + const auto data = findWrite(1).buffers.at(0).buffer; + const std::array patched{0x11, 0x22, 0x400}; + Require(sameBytes(bufferBytes(data), patched.data(), sizeof(patched)), "the shader data buffer does not hold the misaligned view's offset"); + Require(resources.RefreshData(commands, liveShader), "a refresh with different words recorded nothing"); + const std::array refreshed{0x33, 0x44, 0x400}; + Require(sameBytes(bufferBytes(data), refreshed.data(), sizeof(refreshed)), "a data refresh dropped the misaligned view's offset"); + Require(!resources.DataWordsDiffer(liveShader) && resources.DataWordsHash() == AgcDriver::Graphics::ShaderResources::DataWordsHash(liveShader), "the refreshed template's words are not the dispatch's"); + } + Require(mock.live == 0, "misaligned shader data resources leaked Vulkan objects"); +} + struct ModuleShape { bool fragment = false; bool push = false; @@ -1322,6 +1358,7 @@ int main() { InitialContextTests(); pushConstantTests(); resourceTests(); + misalignedShaderDataTests(); debugBranchTests(); validationTests(); rectListTests();