From 212ccb8bd48050a3e25668249c410f5b5a526424 Mon Sep 17 00:00:00 2001 From: Bellekallu Rajkiran Date: Fri, 24 Jan 2025 10:44:43 +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 --- .../os_interface/linux/drm_memory_manager.cpp | 11 +++++++- .../source/os_interface/linux/ioctl_helper.h | 2 ++ .../os_interface/linux/xe/ioctl_helper_xe.cpp | 2 ++ .../os_interface/linux/xe/ioctl_helper_xe.h | 1 + .../common/mocks/linux/mock_ioctl_helper.h | 3 ++- .../linux/drm_memory_manager_tests.cpp | 27 +++++++++++++++++++ .../linux/xe/ioctl_helper_xe_tests.cpp | 13 ++++++--- 7 files changed, 54 insertions(+), 5 deletions(-) diff --git a/shared/source/os_interface/linux/drm_memory_manager.cpp b/shared/source/os_interface/linux/drm_memory_manager.cpp index 87b485a5cf..3483d9bc67 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 7c810a3e4d..ddee7a3818 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() { 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 96346b3752..4ed8293e3e 100644 --- a/shared/source/os_interface/linux/xe/ioctl_helper_xe.cpp +++ b/shared/source/os_interface/linux/xe/ioctl_helper_xe.cpp @@ -665,6 +665,7 @@ int IoctlHelperXe::createGemExt(const MemRegionsVec &memClassInstances, size_t a } create.placement = static_cast(memoryInstances.to_ulong()); create.cpu_caching = this->getCpuCachingMode(isCoherent, isSysMemOnly); + 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); @@ -707,6 +708,7 @@ 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); + 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); 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 06372870dd..86786ad929 100644 --- a/shared/source/os_interface/linux/xe/ioctl_helper_xe.h +++ b/shared/source/os_interface/linux/xe/ioctl_helper_xe.h @@ -136,6 +136,7 @@ class IoctlHelperXe : public IoctlHelper { int getTileIdFromGtId(int gtId) const override { return gtIdToTileId[gtId]; } + bool makeResidentBeforeLockNeeded() override { return true; } 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 6c435c958f..1a584bcea5 100644 --- a/shared/test/common/mocks/linux/mock_ioctl_helper.h +++ b/shared/test/common/mocks/linux/mock_ioctl_helper.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2023-2024 Intel Corporation + * Copyright (C) 2023-2025 Intel Corporation * * SPDX-License-Identifier: MIT * @@ -40,6 +40,7 @@ class MockIoctlHelper : public IoctlHelperPrelim20 { ADDMETHOD_CONST_NOBASE(getNumMediaDecoders, uint32_t, 0, ()); ADDMETHOD_CONST_NOBASE(getNumMediaEncoders, uint32_t, 0, ()); ADDMETHOD_NOBASE(queryDeviceParams, bool, true, (uint32_t *, uint16_t *)); + ADDMETHOD_NOBASE(makeResidentBeforeLockNeeded, bool, false, ()); int getDrmParamValue(DrmParam drmParam) const override { if (drmParam == DrmParam::memoryClassSystem) { 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 ecc81e26f5..30421457a3 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" @@ -2570,6 +2571,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 99a43977b2..e45b01d2f5 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 @@ -140,7 +140,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); @@ -171,7 +171,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 @@ -201,7 +201,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 @@ -2515,6 +2515,13 @@ TEST_F(IoctlHelperXeTest, givenXeIoctlHelperWhenIsTimestampsRefreshEnabledCalled EXPECT_TRUE(xeIoctlHelper->isTimestampsRefreshEnabled()); } +TEST_F(IoctlHelperXeTest, givenXeIoctlHelperWhenMakeResidentBeforeLockNeededIsCalledThenVerifyTrueIsReturned) { + auto executionEnvironment = std::make_unique(); + DrmMock drm{*executionEnvironment->rootDeviceEnvironments[0]}; + auto xeIoctlHelper = std::make_unique(drm); + EXPECT_TRUE(xeIoctlHelper->makeResidentBeforeLockNeeded()); +} + TEST_F(IoctlHelperXeTest, whenIoctlFailsOnQueryConfigSizeThenQueryDeviceIdAndRevisionFails) { MockExecutionEnvironment executionEnvironment{}; std::unique_ptr drm{Drm::create(std::make_unique(0, ""), *executionEnvironment.rootDeviceEnvironments[0])};