From 3bef9f886ad3e0c99a6afa7850f22232bfae9a9c Mon Sep 17 00:00:00 2001 From: Bellekallu Rajkiran Date: Mon, 3 Feb 2025 10:27:00 +0000 Subject: [PATCH] fix: Crash on over memory allocation - Add defer backing flag to gem create ioctl - Make memory resident before lock Related-To: NEO-13403 Signed-off-by: Bellekallu Rajkiran --- .../debug_settings/debug_variables_base.inl | 1 + .../os_interface/linux/drm_memory_manager.cpp | 11 +++++++- .../source/os_interface/linux/ioctl_helper.h | 2 ++ .../os_interface/linux/xe/ioctl_helper_xe.cpp | 16 +++++++++++ .../os_interface/linux/xe/ioctl_helper_xe.h | 1 + .../common/mocks/linux/mock_ioctl_helper.h | 1 + shared/test/common/test_files/igdrcl.config | 1 + .../linux/drm_memory_manager_tests.cpp | 27 +++++++++++++++++++ .../linux/xe/ioctl_helper_xe_tests.cpp | 22 ++++++++++++--- 9 files changed, 78 insertions(+), 4 deletions(-) diff --git a/shared/source/debug_settings/debug_variables_base.inl b/shared/source/debug_settings/debug_variables_base.inl index a171ed1ded..8525e3cb25 100644 --- a/shared/source/debug_settings/debug_variables_base.inl +++ b/shared/source/debug_settings/debug_variables_base.inl @@ -485,6 +485,7 @@ DECLARE_DEBUG_VARIABLE(bool, ForceSamplerLowFilteringPrecision, false, "Force Lo DECLARE_DEBUG_VARIABLE(bool, EnablePrivateBO, false, "Enable PRELIM_I915_GEM_CREATE_EXT_VM_PRIVATE extension creating VM_PRIVATE BOs") DECLARE_DEBUG_VARIABLE(bool, EnableAIL, true, "Enables AIL") DECLARE_DEBUG_VARIABLE(bool, EnableReservingInSvmRange, true, "Enables reserving virtual memory in the SVM range") +DECLARE_DEBUG_VARIABLE(bool, EnableDeferBacking, false, "Enables defer backing on xe kmd") DECLARE_DEBUG_VARIABLE(bool, DisableProgrammableMetricsSupport, false, "Disable Programmable Metrics support") DECLARE_DEBUG_VARIABLE(int64_t, VmBindWaitUserFenceTimeout, -1, "-1: default, >0: time in ns for wait function timeout") DECLARE_DEBUG_VARIABLE(int32_t, ForceRunAloneContext, -1, "Control creation of run-alone HW context, -1:default, 0:disable, 1:enable") diff --git a/shared/source/os_interface/linux/drm_memory_manager.cpp b/shared/source/os_interface/linux/drm_memory_manager.cpp index 8d9e2d5e6a..ac1aed04b0 100644 --- a/shared/source/os_interface/linux/drm_memory_manager.cpp +++ b/shared/source/os_interface/linux/drm_memory_manager.cpp @@ -1518,8 +1518,17 @@ void *DrmMemoryManager::lockResourceImpl(GraphicsAllocation &graphicsAllocation) return cpuPtr; } - auto bo = static_cast(graphicsAllocation).getBO(); + auto rootDeviceIndex = graphicsAllocation.getRootDeviceIndex(); + auto ioctlHelper = this->getDrm(rootDeviceIndex).getIoctlHelper(); + if (ioctlHelper->makeResidentBeforeLockNeeded()) { + auto memoryOperationsInterface = static_cast(executionEnvironment.rootDeviceEnvironments[rootDeviceIndex]->memoryOperationsInterface.get()); + auto graphicsAllocationPtr = &graphicsAllocation; + [[maybe_unused]] auto ret = memoryOperationsInterface->makeResidentWithinOsContext(getDefaultOsContext(rootDeviceIndex), ArrayRef(&graphicsAllocationPtr, 1), false) == MemoryOperationsStatus::success; + DEBUG_BREAK_IF(!ret); + } + + auto bo = static_cast(graphicsAllocation).getBO(); if (graphicsAllocation.getAllocationType() == AllocationType::writeCombined) { auto addr = lockBufferObject(bo); auto alignedAddr = alignUp(addr, MemoryConstants::pageSize64k); diff --git a/shared/source/os_interface/linux/ioctl_helper.h b/shared/source/os_interface/linux/ioctl_helper.h index 75c73c057a..8cfa7fb4ff 100644 --- a/shared/source/os_interface/linux/ioctl_helper.h +++ b/shared/source/os_interface/linux/ioctl_helper.h @@ -236,6 +236,8 @@ class IoctlHelper { virtual bool isTimestampsRefreshEnabled() { return false; } + virtual bool makeResidentBeforeLockNeeded() const { return false; } + protected: Drm &drm; ExternalCtx *externalCtx = nullptr; diff --git a/shared/source/os_interface/linux/xe/ioctl_helper_xe.cpp b/shared/source/os_interface/linux/xe/ioctl_helper_xe.cpp index e042b6c0e5..f801f67fb3 100644 --- a/shared/source/os_interface/linux/xe/ioctl_helper_xe.cpp +++ b/shared/source/os_interface/linux/xe/ioctl_helper_xe.cpp @@ -678,6 +678,10 @@ int IoctlHelperXe::createGemExt(const MemRegionsVec &memClassInstances, size_t a create.placement = static_cast(memoryInstances.to_ulong()); create.cpu_caching = this->getCpuCachingMode(isCoherent, isSysMemOnly); + if (debugManager.flags.EnableDeferBacking.get()) { + create.flags |= DRM_XE_GEM_CREATE_FLAG_DEFER_BACKING; + } + printDebugString(debugManager.flags.PrintBOCreateDestroyResult.get(), stdout, "Performing DRM_IOCTL_XE_GEM_CREATE with {vmid=0x%x size=0x%lx flags=0x%x placement=0x%x caching=%hu }", create.vm_id, create.size, create.flags, create.placement, create.cpu_caching); @@ -720,6 +724,10 @@ uint32_t IoctlHelperXe::createGem(uint64_t size, uint32_t memoryBanks, std::opti create.placement = static_cast(memoryInstances.to_ulong()); create.cpu_caching = this->getCpuCachingMode(isCoherent, isSysMemOnly); + if (debugManager.flags.EnableDeferBacking.get()) { + create.flags |= DRM_XE_GEM_CREATE_FLAG_DEFER_BACKING; + } + printDebugString(debugManager.flags.PrintBOCreateDestroyResult.get(), stdout, "Performing DRM_IOCTL_XE_GEM_CREATE with {vmid=0x%x size=0x%lx flags=0x%x placement=0x%x caching=%hu }", create.vm_id, create.size, create.flags, create.placement, create.cpu_caching); @@ -1566,6 +1574,14 @@ bool IoctlHelperXe::isImmediateVmBindRequired() const { return true; } +bool IoctlHelperXe::makeResidentBeforeLockNeeded() const { + auto makeResidentBeforeLockNeeded = false; + if (debugManager.flags.EnableDeferBacking.get()) { + makeResidentBeforeLockNeeded = true; + } + return makeResidentBeforeLockNeeded; +} + void IoctlHelperXe::insertEngineToContextParams(ContextParamEngines<> &contextParamEngines, uint32_t engineId, const EngineClassInstance *engineClassInstance, uint32_t tileId, bool hasVirtualEngines) { auto engines = reinterpret_cast(contextParamEngines.enginesData); if (engineClassInstance) { diff --git a/shared/source/os_interface/linux/xe/ioctl_helper_xe.h b/shared/source/os_interface/linux/xe/ioctl_helper_xe.h index 5744d0a309..4e86f27bf0 100644 --- a/shared/source/os_interface/linux/xe/ioctl_helper_xe.h +++ b/shared/source/os_interface/linux/xe/ioctl_helper_xe.h @@ -133,6 +133,7 @@ class IoctlHelperXe : public IoctlHelper { int getTileIdFromGtId(int gtId) const override { return gtIdToTileId[gtId]; } + bool makeResidentBeforeLockNeeded() const override; protected: static constexpr uint32_t maxContextSetProperties = 4; diff --git a/shared/test/common/mocks/linux/mock_ioctl_helper.h b/shared/test/common/mocks/linux/mock_ioctl_helper.h index 020eaa6a14..7da9cfe51c 100644 --- a/shared/test/common/mocks/linux/mock_ioctl_helper.h +++ b/shared/test/common/mocks/linux/mock_ioctl_helper.h @@ -31,6 +31,7 @@ class MockIoctlHelper : public IoctlHelperPrelim20 { ADDMETHOD_CONST_NOBASE(isImmediateVmBindRequired, bool, false, ()); ADDMETHOD_CONST_NOBASE(getIoctlRequestValue, unsigned int, 1234u, (DrmIoctl)); ADDMETHOD_CONST_NOBASE(isWaitBeforeBindRequired, bool, false, (bool)); + ADDMETHOD_CONST_NOBASE(makeResidentBeforeLockNeeded, bool, false, ()); ADDMETHOD_NOBASE(vmBind, int, 0, (const VmBindParams &)); ADDMETHOD_NOBASE(vmUnbind, int, 0, (const VmBindParams &)); diff --git a/shared/test/common/test_files/igdrcl.config b/shared/test/common/test_files/igdrcl.config index 1cb304244a..c441cc4c80 100644 --- a/shared/test/common/test_files/igdrcl.config +++ b/shared/test/common/test_files/igdrcl.config @@ -663,4 +663,5 @@ DirectSubmissionRelaxedOrderingCounterHeuristicTreshold = -1 ClearStandaloneInOrderTimestampAllocation = -1 PipelinedEuThreadArbitration = -1 ExperimentalUSMAllocationReuseCleaner = -1 +EnableDeferBacking = 0 # Please don't edit below this line diff --git a/shared/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp b/shared/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp index d7b5792d77..2101a16a5d 100644 --- a/shared/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp +++ b/shared/test/unit_test/os_interface/linux/drm_memory_manager_tests.cpp @@ -15,6 +15,7 @@ #include "shared/source/indirect_heap/indirect_heap.h" #include "shared/source/memory_manager/memory_banks.h" #include "shared/source/os_interface/linux/drm_memory_operations_handler.h" +#include "shared/source/os_interface/linux/drm_memory_operations_handler_bind.h" #include "shared/source/os_interface/linux/i915.h" #include "shared/source/os_interface/linux/os_context_linux.h" #include "shared/test/common/helpers/engine_descriptor_helper.h" @@ -2571,6 +2572,32 @@ TEST_F(DrmMemoryManagerTest, givenDrmMemoryManagerWhenLockUnlockIsCalledThenRetu memoryManager->freeGraphicsMemory(allocation); } +TEST_F(DrmMemoryManagerWithLocalMemoryTest, givenDrmMemoryManagerAndResidentNeededbeforeLockWhenLockIsCalledThenverifyAllocationIsResident) { + mock->ioctlExpected.gemWait = 1; + mock->ioctlExpected.gemClose = 1; + mock->ioctlExpected.gemMmapOffset = 1; + mock->ioctlExpected.gemCreateExt = 1; + + auto mockIoctlHelper = new MockIoctlHelper(*mock); + mockIoctlHelper->makeResidentBeforeLockNeededResult = true; + + auto &drm = static_cast(memoryManager->getDrm(rootDeviceIndex)); + drm.ioctlHelper.reset(mockIoctlHelper); + + executionEnvironment->rootDeviceEnvironments[rootDeviceIndex]->memoryOperationsInterface.reset(new DrmMemoryOperationsHandlerBind(*executionEnvironment->rootDeviceEnvironments[rootDeviceIndex].get(), 0)); + auto allocation = memoryManager->allocateGraphicsMemoryWithProperties(MockAllocationProperties{rootDeviceIndex, true, MemoryConstants::pageSize, AllocationType::buffer}); + ASSERT_NE(nullptr, allocation); + + auto ptr = memoryManager->lockResource(allocation); + EXPECT_NE(nullptr, ptr); + + auto osContext = device->getDefaultEngine().osContext; + EXPECT_TRUE(allocation->isAlwaysResident(osContext->getContextId())); + + memoryManager->unlockResource(allocation); + memoryManager->freeGraphicsMemory(allocation); +} + TEST_F(DrmMemoryManagerTest, givenDrmMemoryManagerWhenLockUnlockIsCalledOnAllocationWithCpuPtrThenReturnCpuPtrAndSetCpuDomain) { mock->ioctlExpected.gemUserptr = 1; mock->ioctlExpected.gemSetDomain = 1; diff --git a/shared/test/unit_test/os_interface/linux/xe/ioctl_helper_xe_tests.cpp b/shared/test/unit_test/os_interface/linux/xe/ioctl_helper_xe_tests.cpp index b2d42828de..77950f0082 100644 --- a/shared/test/unit_test/os_interface/linux/xe/ioctl_helper_xe_tests.cpp +++ b/shared/test/unit_test/os_interface/linux/xe/ioctl_helper_xe_tests.cpp @@ -117,16 +117,21 @@ TEST_F(IoctlHelperXeGemCreateExtTests, givenIoctlHelperXeWhenCallingGemCreateExt } TEST_F(IoctlHelperXeGemCreateExtTests, givenIoctlHelperXeWhenCallingGemCreateExtWithOnlySystemRegionAndCoherencyThenWriteBackCPUCachingIsUsed) { + DebugManagerStateRestore restorer; + debugManager.flags.EnableDeferBacking.set(1); + MemRegionsVec memRegions = {systemMemory}; bool isCoherent = true; EXPECT_NE(0, xeIoctlHelper->createGemExt(memRegions, allocSize, handle, patIndex, std::nullopt, pairHandle, isChunked, numOfChunks, std::nullopt, std::nullopt, isCoherent)); EXPECT_EQ(DRM_XE_GEM_CPU_CACHING_WB, drm->createParamsCpuCaching); + EXPECT_EQ(static_cast(DRM_XE_GEM_CREATE_FLAG_DEFER_BACKING), (drm->createParamsFlags & DRM_XE_GEM_CREATE_FLAG_DEFER_BACKING)); } TEST_F(IoctlHelperXeTest, givenIoctlHelperXeWhenCallGemCreateAndNoLocalMemoryThenProperValuesSet) { DebugManagerStateRestore restorer; debugManager.flags.EnableLocalMemory.set(0); + debugManager.flags.EnableDeferBacking.set(1); auto executionEnvironment = std::make_unique(); auto drm = DrmMockXe::create(*executionEnvironment->rootDeviceEnvironments[0]); auto xeIoctlHelper = static_cast(drm->getIoctlHelper()); @@ -145,7 +150,7 @@ TEST_F(IoctlHelperXeTest, givenIoctlHelperXeWhenCallGemCreateAndNoLocalMemoryThe EXPECT_TRUE(xeIoctlHelper->bindInfo.empty()); EXPECT_EQ(size, drm->createParamsSize); - EXPECT_EQ(0u, drm->createParamsFlags); + EXPECT_EQ(static_cast(DRM_XE_GEM_CREATE_FLAG_DEFER_BACKING), (drm->createParamsFlags & DRM_XE_GEM_CREATE_FLAG_DEFER_BACKING)); EXPECT_EQ(DRM_XE_GEM_CPU_CACHING_WC, drm->createParamsCpuCaching); EXPECT_EQ(1u, drm->createParamsPlacement); @@ -157,6 +162,7 @@ TEST_F(IoctlHelperXeTest, givenIoctlHelperXeWhenCallGemCreateAndNoLocalMemoryThe TEST_F(IoctlHelperXeTest, givenIoctlHelperXeWhenCallGemCreateWhenMemoryBanksZeroThenProperValuesSet) { DebugManagerStateRestore restorer; debugManager.flags.EnableLocalMemory.set(0); + debugManager.flags.EnableDeferBacking.set(1); auto executionEnvironment = std::make_unique(); auto drm = DrmMockXe::create(*executionEnvironment->rootDeviceEnvironments[0]); auto xeIoctlHelper = static_cast(drm->getIoctlHelper()); @@ -176,7 +182,7 @@ TEST_F(IoctlHelperXeTest, givenIoctlHelperXeWhenCallGemCreateWhenMemoryBanksZero EXPECT_EQ(size, drm->createParamsSize); EXPECT_EQ(DRM_XE_GEM_CPU_CACHING_WC, drm->createParamsCpuCaching); - EXPECT_EQ(0u, drm->createParamsFlags); + EXPECT_EQ(static_cast(DRM_XE_GEM_CREATE_FLAG_DEFER_BACKING), (drm->createParamsFlags & DRM_XE_GEM_CREATE_FLAG_DEFER_BACKING)); EXPECT_EQ(1u, drm->createParamsPlacement); // dummy mock handle @@ -187,6 +193,7 @@ TEST_F(IoctlHelperXeTest, givenIoctlHelperXeWhenCallGemCreateWhenMemoryBanksZero TEST_F(IoctlHelperXeTest, givenIoctlHelperXeWhenCallGemCreateAndLocalMemoryThenProperValuesSet) { DebugManagerStateRestore restorer; debugManager.flags.EnableLocalMemory.set(1); + debugManager.flags.EnableDeferBacking.set(1); auto executionEnvironment = std::make_unique(); auto drm = DrmMockXe::create(*executionEnvironment->rootDeviceEnvironments[0]); auto xeIoctlHelper = static_cast(drm->getIoctlHelper()); @@ -206,7 +213,7 @@ TEST_F(IoctlHelperXeTest, givenIoctlHelperXeWhenCallGemCreateAndLocalMemoryThenP EXPECT_EQ(size, drm->createParamsSize); EXPECT_EQ(DRM_XE_GEM_CPU_CACHING_WC, drm->createParamsCpuCaching); - EXPECT_EQ(0u, drm->createParamsFlags); + EXPECT_EQ(static_cast(DRM_XE_GEM_CREATE_FLAG_DEFER_BACKING), (drm->createParamsFlags & DRM_XE_GEM_CREATE_FLAG_DEFER_BACKING)); EXPECT_EQ(6u, drm->createParamsPlacement); // dummy mock handle @@ -2677,3 +2684,12 @@ TEST_F(IoctlHelperXeTest, whenQueryDeviceIdAndRevisionAndSharedSystemUsmSupportD EXPECT_TRUE(IoctlHelperXe::queryDeviceIdAndRevision(*drm)); EXPECT_FALSE(drm->isSharedSystemAllocEnabled()); } + +TEST_F(IoctlHelperXeTest, givenXeIoctlHelperAndDeferBackingFlagSetToTrueWhenMakeResidentBeforeLockNeededIsCalledThenVerifyTrueIsReturned) { + DebugManagerStateRestore restorer; + debugManager.flags.EnableDeferBacking.set(1); + auto executionEnvironment = std::make_unique(); + DrmMock drm{*executionEnvironment->rootDeviceEnvironments[0]}; + auto xeIoctlHelper = std::make_unique(drm); + EXPECT_TRUE(xeIoctlHelper->makeResidentBeforeLockNeeded()); +}