From cb152ba821484b59e7095bfc978f16bf5a6c35d1 Mon Sep 17 00:00:00 2001 From: Mateusz Hoppe Date: Mon, 30 Jun 2025 18:35:32 +0000 Subject: [PATCH] fix: copy sip binary to allocation based on blitRequired query - readOnly ISA allocations must be written through CPU pointer - command buffer allocation in singleAddressSpaceSbaTracking mode cannot be readonly - it is written by SBA tracking commands - this change is fixing ZET_ENABLE_PROGRAM_DEBUGGING mode 2 Related-To: GSD-10359 Signed-off-by: Mateusz Hoppe --- shared/source/built_ins/built_ins.cpp | 6 +++- .../command_stream_receiver.cpp | 2 +- shared/source/debugger/debugger.h | 1 + shared/source/debugger/debugger_l0.h | 2 +- .../memory_manager/allocation_properties.h | 3 +- .../memory_manager/graphics_allocation.cpp | 6 ++-- .../memory_manager/graphics_allocation.h | 3 +- .../source/memory_manager/memory_manager.cpp | 14 ++++++-- shared/source/memory_manager/memory_manager.h | 2 +- shared/test/common/mocks/mock_debugger.h | 6 +++- shared/test/unit_test/built_ins/sip_tests.cpp | 34 +++++++++++++++++-- ...nager_allocate_in_preferred_pool_tests.cpp | 32 +++++++++++++++++ 12 files changed, 96 insertions(+), 15 deletions(-) diff --git a/shared/source/built_ins/built_ins.cpp b/shared/source/built_ins/built_ins.cpp index 86506a3605..47637c7fe9 100644 --- a/shared/source/built_ins/built_ins.cpp +++ b/shared/source/built_ins/built_ins.cpp @@ -114,9 +114,13 @@ const SipKernel &BuiltIns::getSipKernel(Device &device, OsContext *context) { binary[bindlessSip.getPidOffset()] = static_cast((context->getOfflineDumpContextId(deviceIndex) >> 32) & 0xFFFFFFFF); } + auto &rootDeviceEnvironment = device.getRootDeviceEnvironment(); + auto &productHelper = device.getProductHelper(); + DeviceBitfield copyBitfield{}; copyBitfield.set(deviceIndex); - copySuccess = MemoryTransferHelper::transferMemoryToAllocationBanks(device, sipAllocation, 0, binary.get(), bindlessSip.getBinary().size(), copyBitfield); + copySuccess = MemoryTransferHelper::transferMemoryToAllocationBanks(productHelper.isBlitCopyRequiredForLocalMemory(rootDeviceEnvironment, *sipAllocation), + device, sipAllocation, 0, binary.get(), bindlessSip.getBinary().size(), copyBitfield); DEBUG_BREAK_IF(!copySuccess); } } diff --git a/shared/source/command_stream/command_stream_receiver.cpp b/shared/source/command_stream/command_stream_receiver.cpp index 902b8d5143..8729954fdd 100644 --- a/shared/source/command_stream/command_stream_receiver.cpp +++ b/shared/source/command_stream/command_stream_receiver.cpp @@ -868,7 +868,7 @@ bool CommandStreamReceiver::createWorkPartitionAllocation(const Device &device) *copySrc = {logicalId++, deviceIndex}; DeviceBitfield copyBitfield{}; copyBitfield.set(deviceIndex); - auto copySuccess = MemoryTransferHelper::transferMemoryToAllocationBanks(device, workPartitionAllocation, 0, copySrc.get(), 2 * sizeof(uint32_t), copyBitfield); + auto copySuccess = MemoryTransferHelper::transferMemoryToAllocationBanks(true, device, workPartitionAllocation, 0, copySrc.get(), 2 * sizeof(uint32_t), copyBitfield); if (!copySuccess) { return false; diff --git a/shared/source/debugger/debugger.h b/shared/source/debugger/debugger.h index 14f094cba6..0a1cf115ce 100644 --- a/shared/source/debugger/debugger.h +++ b/shared/source/debugger/debugger.h @@ -32,6 +32,7 @@ class Debugger { virtual ~Debugger() = default; virtual void captureStateBaseAddress(NEO::LinearStream &cmdStream, SbaAddresses sba, bool useFirstLevelBB) = 0; virtual size_t getSbaTrackingCommandsSize(size_t trackedAddressCount) = 0; + virtual bool getSingleAddressSpaceSbaTracking() const = 0; void *getDebugSurfaceReservedSurfaceState(IndirectHeap &ssh); diff --git a/shared/source/debugger/debugger_l0.h b/shared/source/debugger/debugger_l0.h index 5f355b6097..e0fd72b5e3 100644 --- a/shared/source/debugger/debugger_l0.h +++ b/shared/source/debugger/debugger_l0.h @@ -111,7 +111,7 @@ class DebuggerL0 : public NEO::Debugger, NEO::NonCopyableAndNonMovableClass { void setSingleAddressSpaceSbaTracking(bool value) { singleAddressSpaceSbaTracking = value; } - bool getSingleAddressSpaceSbaTracking() { return singleAddressSpaceSbaTracking; } + bool getSingleAddressSpaceSbaTracking() const override { return singleAddressSpaceSbaTracking; } struct CommandQueueNotification { uint32_t subDeviceIndex = 0; diff --git a/shared/source/memory_manager/allocation_properties.h b/shared/source/memory_manager/allocation_properties.h index 76afd48e02..00274d590e 100644 --- a/shared/source/memory_manager/allocation_properties.h +++ b/shared/source/memory_manager/allocation_properties.h @@ -111,7 +111,8 @@ struct AllocationData { uint32_t use32BitFrontWindow : 1; uint32_t isUSMDeviceMemory : 1; uint32_t zeroMemory : 1; - uint32_t reserved : 16; + uint32_t cantBeReadOnly : 1; + uint32_t reserved : 15; } flags; uint32_t allFlags = 0; }; diff --git a/shared/source/memory_manager/graphics_allocation.cpp b/shared/source/memory_manager/graphics_allocation.cpp index 4b0ffc04b3..8713d7a981 100644 --- a/shared/source/memory_manager/graphics_allocation.cpp +++ b/shared/source/memory_manager/graphics_allocation.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2018-2024 Intel Corporation + * Copyright (C) 2018-2025 Intel Corporation * * SPDX-License-Identifier: MIT * @@ -158,9 +158,9 @@ bool GraphicsAllocation::hasAllocationReadOnlyType() { return false; } -void GraphicsAllocation::checkAllocationTypeReadOnlyRestrictions(const AllocationProperties &properties) { +void GraphicsAllocation::checkAllocationTypeReadOnlyRestrictions(const AllocationData &allocData) { if (getAllocationType() == AllocationType::commandBuffer && - (properties.flags.cantBeReadOnly | properties.flags.multiOsContextCapable)) { + (allocData.flags.cantBeReadOnly | allocData.flags.multiOsContextCapable)) { setAsCantBeReadOnly(true); return; } diff --git a/shared/source/memory_manager/graphics_allocation.h b/shared/source/memory_manager/graphics_allocation.h index 46256031bd..59f99184d8 100644 --- a/shared/source/memory_manager/graphics_allocation.h +++ b/shared/source/memory_manager/graphics_allocation.h @@ -39,6 +39,7 @@ class CommandStreamReceiver; class GraphicsAllocation; class ProductHelper; +struct AllocationData; struct AllocationProperties; struct AubInfo { @@ -338,7 +339,7 @@ class GraphicsAllocation : public IDNode, NEO::NonCopyableAn void setShareableHostMemory(bool shareableHostMemory) { this->shareableHostMemory = shareableHostMemory; } bool isShareableHostMemory() const { return shareableHostMemory; } MOCKABLE_VIRTUAL bool hasAllocationReadOnlyType(); - MOCKABLE_VIRTUAL void checkAllocationTypeReadOnlyRestrictions(const AllocationProperties &properties); + MOCKABLE_VIRTUAL void checkAllocationTypeReadOnlyRestrictions(const AllocationData &allocData); OsHandleStorage fragmentsStorage; StorageInfo storageInfo = {}; diff --git a/shared/source/memory_manager/memory_manager.cpp b/shared/source/memory_manager/memory_manager.cpp index 63ae336daa..0f2f4252ab 100644 --- a/shared/source/memory_manager/memory_manager.cpp +++ b/shared/source/memory_manager/memory_manager.cpp @@ -654,8 +654,13 @@ bool MemoryManager::getAllocationData(AllocationData &allocationData, const Allo allocationData.flags.preferCompressed = properties.flags.preferCompressed; allocationData.flags.preferCompressed |= CompressionSelector::preferCompressedAllocation(properties); allocationData.flags.multiOsContextCapable = properties.flags.multiOsContextCapable; + allocationData.flags.cantBeReadOnly = properties.flags.cantBeReadOnly; allocationData.usmInitialPlacement = properties.usmInitialPlacement; + if (properties.allocationType == AllocationType::commandBuffer && rootDeviceEnvironment.debugger.get() && rootDeviceEnvironment.debugger->getSingleAddressSpaceSbaTracking()) { + allocationData.flags.cantBeReadOnly = true; + } + if (GraphicsAllocation::isDebugSurfaceAllocationType(properties.allocationType) || GraphicsAllocation::isConstantOrGlobalSurfaceAllocationType(properties.allocationType)) { allocationData.flags.zeroMemory = 1; @@ -786,7 +791,7 @@ GraphicsAllocation *MemoryManager::allocateGraphicsMemoryInPreferredPool(const A if (!allocation) { return nullptr; } - allocation->checkAllocationTypeReadOnlyRestrictions(properties); + allocation->checkAllocationTypeReadOnlyRestrictions(allocationData); auto &rootDeviceEnvironment = *executionEnvironment.rootDeviceEnvironments[properties.rootDeviceIndex]; auto &productHelper = rootDeviceEnvironment.getProductHelper(); @@ -1271,10 +1276,13 @@ bool MemoryTransferHelper::transferMemoryToAllocation(bool useBlitter, const Dev } return device.getMemoryManager()->copyMemoryToAllocation(dstAllocation, dstOffset, srcMemory, srcSize); } -bool MemoryTransferHelper::transferMemoryToAllocationBanks(const Device &device, GraphicsAllocation *dstAllocation, size_t dstOffset, const void *srcMemory, +bool MemoryTransferHelper::transferMemoryToAllocationBanks(bool useBlitter, const Device &device, GraphicsAllocation *dstAllocation, size_t dstOffset, const void *srcMemory, size_t srcSize, DeviceBitfield dstMemoryBanks) { - auto blitSuccess = BlitHelper::blitMemoryToAllocationBanks(device, dstAllocation, dstOffset, srcMemory, {srcSize, 1, 1}, dstMemoryBanks) == BlitOperationResult::success; + auto blitSuccess = false; + if (useBlitter) { + blitSuccess = BlitHelper::blitMemoryToAllocationBanks(device, dstAllocation, dstOffset, srcMemory, {srcSize, 1, 1}, dstMemoryBanks) == BlitOperationResult::success; + } if (!blitSuccess) { return device.getMemoryManager()->copyMemoryToAllocationBanks(dstAllocation, dstOffset, srcMemory, srcSize, dstMemoryBanks); } diff --git a/shared/source/memory_manager/memory_manager.h b/shared/source/memory_manager/memory_manager.h index bfa56ab36f..aaec47f279 100644 --- a/shared/source/memory_manager/memory_manager.h +++ b/shared/source/memory_manager/memory_manager.h @@ -95,7 +95,7 @@ constexpr size_t paddingBufferSize = 2 * MemoryConstants::megaByte; namespace MemoryTransferHelper { bool transferMemoryToAllocation(bool useBlitter, const Device &device, GraphicsAllocation *dstAllocation, size_t dstOffset, const void *srcMemory, size_t srcSize); -bool transferMemoryToAllocationBanks(const Device &device, GraphicsAllocation *dstAllocation, size_t dstOffset, const void *srcMemory, +bool transferMemoryToAllocationBanks(bool useBlitter, const Device &device, GraphicsAllocation *dstAllocation, size_t dstOffset, const void *srcMemory, size_t srcSize, DeviceBitfield dstMemoryBanks); } // namespace MemoryTransferHelper diff --git a/shared/test/common/mocks/mock_debugger.h b/shared/test/common/mocks/mock_debugger.h index 03b7cbbafc..f1a14ff34d 100644 --- a/shared/test/common/mocks/mock_debugger.h +++ b/shared/test/common/mocks/mock_debugger.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2021-2022 Intel Corporation + * Copyright (C) 2021-2025 Intel Corporation * * SPDX-License-Identifier: MIT * @@ -19,5 +19,9 @@ class MockDebugger : public Debugger { size_t getSbaTrackingCommandsSize(size_t trackedAddressCount) override { return 0; } + bool getSingleAddressSpaceSbaTracking() const override { + return singleAddressSpaceSbaTracking; + } + bool singleAddressSpaceSbaTracking = false; }; } // namespace NEO diff --git a/shared/test/unit_test/built_ins/sip_tests.cpp b/shared/test/unit_test/built_ins/sip_tests.cpp index e20b6b5c99..4c64baf281 100644 --- a/shared/test/unit_test/built_ins/sip_tests.cpp +++ b/shared/test/unit_test/built_ins/sip_tests.cpp @@ -639,8 +639,6 @@ TEST(DebugBindlessSip, givenOfflineDebuggingModeWhenGettingSipForContextThenCorr auto csr = mockDevice->createCommandStreamReceiver(); csr->setupContext(*osContext); - EXPECT_NE(nullptr, mockDevice); - auto &sipKernel = NEO::SipKernel::getSipKernel(*mockDevice, &csr->getOsContext()); EXPECT_NE(nullptr, &sipKernel); @@ -652,6 +650,38 @@ TEST(DebugBindlessSip, givenOfflineDebuggingModeWhenGettingSipForContextThenCorr EXPECT_FALSE(contextSip->getStateSaveAreaHeader().empty()); } +TEST(DebugSip, givenOfflineDebuggingModeWhenGettingSipForContextThenMemoryTransferGoesThroughCpuPointer) { + auto executionEnvironment = MockDevice::prepareExecutionEnvironment(defaultHwInfo.get(), 0u); + executionEnvironment->setDebuggingMode(DebuggingMode::offline); + VariableBackup mockSipBackup(&MockSipData::useMockSip, false); + auto builtIns = new NEO::MockBuiltins(); + builtIns->callBaseGetSipKernel = true; + MockRootDeviceEnvironment::resetBuiltins(executionEnvironment->rootDeviceEnvironments[0].get(), builtIns); + auto mockDevice = std::unique_ptr(MockDevice::createWithExecutionEnvironment(defaultHwInfo.get(), executionEnvironment, 0u)); + + auto memoryManager = static_cast(mockDevice->getMemoryManager()); + + const uint32_t contextId = 2u; + std::unique_ptr osContext(OsContext::create(executionEnvironment->rootDeviceEnvironments[0]->osInterface.get(), + mockDevice->getRootDeviceIndex(), contextId, + EngineDescriptorHelper::getDefaultDescriptor({aub_stream::ENGINE_BCS, EngineUsage::regular}, PreemptionMode::ThreadGroup, mockDevice->getDeviceBitfield()))); + osContext->setDefaultContext(true); + + auto csr = mockDevice->createCommandStreamReceiver(); + csr->setupContext(*osContext); + + std::vector sipBinary; + std::vector stateSaveAreaHeader; + mockDevice->getCompilerInterface()->getSipKernelBinary(*mockDevice, SipKernelType::dbgBindless, sipBinary, stateSaveAreaHeader); + + memoryManager->copyMemoryToAllocationBanksCalled = 0; + auto &sipKernel = NEO::SipKernel::getSipKernel(*mockDevice, &csr->getOsContext()); + ASSERT_NE(nullptr, &sipKernel); + EXPECT_EQ(1u, memoryManager->copyMemoryToAllocationBanksCalled); + + EXPECT_EQ(0, memcmp(sipBinary.data(), sipKernel.getSipAllocation()->getUnderlyingBuffer(), sipBinary.size())); +} + TEST(DebugBindlessSip, givenTwoContextsWhenBindlessDebugSipIsRequestedThenEachSipKernelIsAssignedToADifferentContextId) { auto executionEnvironment = MockDevice::prepareExecutionEnvironment(defaultHwInfo.get(), 0u); auto builtIns = new NEO::MockBuiltins(); diff --git a/shared/test/unit_test/memory_manager/memory_manager_allocate_in_preferred_pool_tests.cpp b/shared/test/unit_test/memory_manager/memory_manager_allocate_in_preferred_pool_tests.cpp index e1c6bb3de7..5e1801bccd 100644 --- a/shared/test/unit_test/memory_manager/memory_manager_allocate_in_preferred_pool_tests.cpp +++ b/shared/test/unit_test/memory_manager/memory_manager_allocate_in_preferred_pool_tests.cpp @@ -13,6 +13,7 @@ #include "shared/test/common/helpers/engine_descriptor_helper.h" #include "shared/test/common/helpers/unit_test_helper.h" #include "shared/test/common/mocks/mock_allocation_properties.h" +#include "shared/test/common/mocks/mock_debugger.h" #include "shared/test/common/mocks/mock_execution_environment.h" #include "shared/test/common/mocks/mock_gmm.h" #include "shared/test/common/mocks/mock_graphics_allocation.h" @@ -1149,6 +1150,37 @@ TEST(MemoryManagerTest, givenMemoryManagerWhenAllocationIsCommandBufferAndMultiC EXPECT_EQ(mockGa.setAsReadOnlyCalled, 0u); } +TEST(MemoryManagerTest, givenSingleAddressSpaceSbaTrackingWhenAllocationIsCommandBufferAndMultiContextCapableIsFalseThenAllocationIsNotSetAsReadOnly) { + MockExecutionEnvironment executionEnvironment(defaultHwInfo.get()); + executionEnvironment.setDebuggingMode(NEO::DebuggingMode::offline); + auto debugger = new MockDebugger; + debugger->singleAddressSpaceSbaTracking = true; + executionEnvironment.rootDeviceEnvironments[0]->debugger.reset(debugger); + + auto mockProductHelper = std::make_unique(); + mockProductHelper->isBlitCopyRequiredForLocalMemoryResult = false; + mockProductHelper->supportReadOnlyAllocationsResult = true; + std::unique_ptr productHelper = std::move(mockProductHelper); + std::swap(executionEnvironment.rootDeviceEnvironments[0]->productHelper, productHelper); + MockMemoryManager memoryManager(false, true, executionEnvironment); + MockGraphicsAllocation mockGa; + mockGa.setAllocationType(AllocationType::commandBuffer); + + mockGa.hasAllocationReadOnlyTypeResult = true; + + memoryManager.mockGa = &mockGa; + memoryManager.returnMockGAFromDevicePool = true; + + AllocationProperties properties(mockRootDeviceIndex, MemoryConstants::pageSize, AllocationType::commandBuffer, mockDeviceBitfield); + properties.flags.cantBeReadOnly = false; + properties.flags.multiOsContextCapable = false; + + auto allocation = memoryManager.allocateGraphicsMemoryInPreferredPool(properties, + nullptr); + EXPECT_EQ(allocation, &mockGa); + EXPECT_EQ(mockGa.setAsReadOnlyCalled, 0u); +} + TEST(MemoryManagerTest, givenMemoryManagerWhenAllocationTypeAndPlatrormSupportReadOnlyAllocationBliterAndAllocationTypeOtherThanCmdBufferTransferNotRequiredThenAllocationIsSetAsReadOnly) { MockExecutionEnvironment executionEnvironment(defaultHwInfo.get()); auto mockProductHelper = std::make_unique();