From abb91a7daeddbc3f3730cd573715a9149cd92b68 Mon Sep 17 00:00:00 2001 From: Raiyan Latif Date: Mon, 6 Dec 2021 18:12:47 +0000 Subject: [PATCH] Return error in canAccessPeer when P2P appendMemoryCopy fails Signed-off-by: Raiyan Latif --- level_zero/core/source/cmdlist/cmdlist_hw.inl | 5 +- level_zero/core/source/device/device_imp.cpp | 14 ++-- .../core/source/driver/driver_handle_imp.cpp | 5 ++ .../sources/cmdlist/test_cmdlist_2.cpp | 41 +++++++++- .../unit_tests/sources/device/test_device.cpp | 34 +++++++++ .../unit_tests/sources/memory/test_memory.cpp | 75 ++++++++++++++++++- .../os_interface/linux/drm_memory_manager.cpp | 1 - 7 files changed, 164 insertions(+), 11 deletions(-) diff --git a/level_zero/core/source/cmdlist/cmdlist_hw.inl b/level_zero/core/source/cmdlist/cmdlist_hw.inl index 33879598a1..f2f1551653 100644 --- a/level_zero/core/source/cmdlist/cmdlist_hw.inl +++ b/level_zero/core/source/cmdlist/cmdlist_hw.inl @@ -1090,6 +1090,10 @@ ze_result_t CommandListCoreFamily::appendMemoryCopy(void *dstptr, auto dstAllocationStruct = getAlignedAllocation(this->device, dstptr, size, false); auto srcAllocationStruct = getAlignedAllocation(this->device, srcptr, size, true); + if (dstAllocationStruct.alloc == nullptr || srcAllocationStruct.alloc == nullptr) { + return ZE_RESULT_ERROR_UNKNOWN; + } + if (size >= 4ull * MemoryConstants::gigaByte) { isStateless = true; } @@ -1767,7 +1771,6 @@ inline AlignedAllocationData CommandListCoreFamily::getAlignedAll uint64_t offset = sourcePtr - pbase; alloc = driverHandle->getPeerAllocation(device, allocData, const_cast(buffer), &alignedPtr); - UNRECOVERABLE_IF(alloc == nullptr); alignedPtr += offset; } else { alignedPtr = sourcePtr; diff --git a/level_zero/core/source/device/device_imp.cpp b/level_zero/core/source/device/device_imp.cpp index bad84ec6ea..0ea402c922 100644 --- a/level_zero/core/source/device/device_imp.cpp +++ b/level_zero/core/source/device/device_imp.cpp @@ -127,15 +127,17 @@ ze_result_t DeviceImp::canAccessPeer(ze_device_handle_t hPeerDevice, ze_bool_t * contextImp->allocDeviceMem(this->toHandle(), &deviceDesc, 8, 1, &memory); contextImp->allocDeviceMem(hPeerDevice, &peerDeviceDesc, 8, 1, &peerMemory); - L0::CommandList::fromHandle(commandList)->appendMemoryCopy(peerMemory, memory, 8, nullptr, 0, nullptr); + auto ret = L0::CommandList::fromHandle(commandList)->appendMemoryCopy(peerMemory, memory, 8, nullptr, 0, nullptr); L0::CommandList::fromHandle(commandList)->close(); - auto ret = L0::CommandQueue::fromHandle(commandQueue)->executeCommandLists(1, &commandList, nullptr, true); if (ret == ZE_RESULT_SUCCESS) { - this->crossAccessEnabledDevices[peerRootDeviceIndex] = true; - pPeerDevice->crossAccessEnabledDevices[this->getNEODevice()->getRootDeviceIndex()] = true; - L0::CommandQueue::fromHandle(commandQueue)->synchronize(std::numeric_limits::max()); - *value = true; + ret = L0::CommandQueue::fromHandle(commandQueue)->executeCommandLists(1, &commandList, nullptr, true); + if (ret == ZE_RESULT_SUCCESS) { + this->crossAccessEnabledDevices[peerRootDeviceIndex] = true; + pPeerDevice->crossAccessEnabledDevices[this->getNEODevice()->getRootDeviceIndex()] = true; + L0::CommandQueue::fromHandle(commandQueue)->synchronize(std::numeric_limits::max()); + *value = true; + } } contextImp->freeMem(peerMemory); diff --git a/level_zero/core/source/driver/driver_handle_imp.cpp b/level_zero/core/source/driver/driver_handle_imp.cpp index c001a98607..bbec314c93 100644 --- a/level_zero/core/source/driver/driver_handle_imp.cpp +++ b/level_zero/core/source/driver/driver_handle_imp.cpp @@ -469,7 +469,12 @@ NEO::GraphicsAllocation *DriverHandleImp::getPeerAllocation(Device *device, UNRECOVERABLE_IF(alloc == nullptr); uint64_t handle = alloc->peekInternalHandle(this->getMemoryManager()); ze_ipc_memory_flags_t flags = {}; + peerPtr = this->importFdHandle(device, flags, handle, &alloc); + if (peerPtr == nullptr) { + return nullptr; + } + peerAllocData = this->getSvmAllocsManager()->getSVMAlloc(peerPtr); deviceImp->peerAllocations.allocations.insert(std::make_pair(ptr, *peerAllocData)); } diff --git a/level_zero/core/test/unit_tests/sources/cmdlist/test_cmdlist_2.cpp b/level_zero/core/test/unit_tests/sources/cmdlist/test_cmdlist_2.cpp index 78030cf025..17ffab523c 100644 --- a/level_zero/core/test/unit_tests/sources/cmdlist/test_cmdlist_2.cpp +++ b/level_zero/core/test/unit_tests/sources/cmdlist/test_cmdlist_2.cpp @@ -26,8 +26,13 @@ class MockCommandListHw : public WhiteBox<::L0::CommandListCoreFamily>(), failOnFirstCopy(failOnFirst) {} AlignedAllocationData getAlignedAllocation(L0::Device *device, const void *buffer, uint64_t bufferSize, bool allowHostCopy) override { - return {0, 0, nullptr, true}; + getAlignedAllocationCalledTimes++; + if (buffer) { + return {0, 0, &alignedAlloc, true}; + } + return {0, 0, nullptr, false}; } + ze_result_t appendMemoryCopyKernelWithGA(void *dstPtr, NEO::GraphicsAllocation *dstPtrAlloc, uint64_t dstOffset, @@ -123,14 +128,48 @@ class MockCommandListHw : public WhiteBox<::L0::CommandListCoreFamily appendImageRegionCopySize = {0, 0, 0}; Vec3 appendImageRegionSrcOrigin = {9, 9, 9}; Vec3 appendImageRegionDstOrigin = {9, 9, 9}; bool failOnFirstCopy = false; + uint8_t mockAlignedAllocData[2 * MemoryConstants::pageSize]{}; + void *alignedDataPtr = alignUp(mockAlignedAllocData, MemoryConstants::pageSize); + NEO::MockGraphicsAllocation alignedAlloc{alignedDataPtr, reinterpret_cast(alignedDataPtr), MemoryConstants::pageSize}; }; using Platforms = IsAtLeastProduct; +HWTEST2_F(CommandListCreate, givenCommandListWhenMemoryCopyCalledWithNullDstPtrThenAppendMemoryCopyWithappendMemoryCopyReturnsError, Platforms) { + MockCommandListHw cmdList; + cmdList.initialize(device, NEO::EngineGroupType::RenderCompute, 0u); + void *srcPtr = reinterpret_cast(0x1234); + void *dstPtr = nullptr; + ze_result_t ret = cmdList.appendMemoryCopy(dstPtr, srcPtr, 0x1001, nullptr, 0, nullptr); + EXPECT_GT(cmdList.getAlignedAllocationCalledTimes, 0u); + EXPECT_EQ(ret, ZE_RESULT_ERROR_UNKNOWN); +} + +HWTEST2_F(CommandListCreate, givenCommandListWhenMemoryCopyCalledWithNullSrcPtrThenAppendMemoryCopyWithappendMemoryCopyReturnsError, Platforms) { + MockCommandListHw cmdList; + cmdList.initialize(device, NEO::EngineGroupType::RenderCompute, 0u); + void *srcPtr = nullptr; + void *dstPtr = reinterpret_cast(0x2345); + ze_result_t ret = cmdList.appendMemoryCopy(dstPtr, srcPtr, 0x1001, nullptr, 0, nullptr); + EXPECT_GT(cmdList.getAlignedAllocationCalledTimes, 0u); + EXPECT_EQ(ret, ZE_RESULT_ERROR_UNKNOWN); +} + +HWTEST2_F(CommandListCreate, givenCommandListWhenMemoryCopyCalledWithNullSrcPtrAndDstPtrThenAppendMemoryCopyWithappendMemoryCopyReturnsError, Platforms) { + MockCommandListHw cmdList; + cmdList.initialize(device, NEO::EngineGroupType::RenderCompute, 0u); + void *srcPtr = nullptr; + void *dstPtr = nullptr; + ze_result_t ret = cmdList.appendMemoryCopy(dstPtr, srcPtr, 0x1001, nullptr, 0, nullptr); + EXPECT_GT(cmdList.getAlignedAllocationCalledTimes, 0u); + EXPECT_EQ(ret, ZE_RESULT_ERROR_UNKNOWN); +} + HWTEST2_F(CommandListCreate, givenCommandListWhenMemoryCopyCalledThenAppendMemoryCopyWithappendMemoryCopyKernelWithGACalled, Platforms) { MockCommandListHw cmdList; cmdList.initialize(device, NEO::EngineGroupType::RenderCompute, 0u); diff --git a/level_zero/core/test/unit_tests/sources/device/test_device.cpp b/level_zero/core/test/unit_tests/sources/device/test_device.cpp index e720411878..ce3887c776 100644 --- a/level_zero/core/test/unit_tests/sources/device/test_device.cpp +++ b/level_zero/core/test/unit_tests/sources/device/test_device.cpp @@ -1589,6 +1589,40 @@ TEST_F(MultipleDevicesTest, givenCanAccessPeerCalledTwiceThenCanAccessPeerReturn EXPECT_TRUE(canAccess); } +TEST_F(MultipleDevicesTest, givenDeviceFailsAppendMemoryCopyThenCanAccessPeerReturnsFalse) { + struct MockDeviceFail : public Mock { + MockDeviceFail(L0::Device *device) : Mock(device->getNEODevice(), static_cast(device->getExecEnvironment())) { + this->driverHandle = device->getDriverHandle(); + this->commandList.appendMemoryCopyResult = ZE_RESULT_ERROR_UNKNOWN; + } + + ze_result_t createCommandQueue(const ze_command_queue_desc_t *desc, + ze_command_queue_handle_t *commandQueue) override { + *commandQueue = &this->commandQueue; + return ZE_RESULT_SUCCESS; + } + + ze_result_t createCommandList(const ze_command_list_desc_t *desc, + ze_command_list_handle_t *commandList) override { + *commandList = &this->commandList; + return ZE_RESULT_SUCCESS; + } + + MockCommandList commandList; + Mock commandQueue; + }; + + MockDeviceFail *device0 = new MockDeviceFail(driverHandle->devices[0]); + L0::Device *device1 = driverHandle->devices[1]; + + ze_bool_t canAccess = false; + ze_result_t res = device0->canAccessPeer(device1->toHandle(), &canAccess); + EXPECT_GT(device0->commandList.appendMemoryCopyCalled, 0u); + EXPECT_EQ(ZE_RESULT_SUCCESS, res); + EXPECT_FALSE(canAccess); + delete device0; +} + TEST_F(MultipleDevicesTest, givenDeviceFailsExecuteCommandListThenCanAccessPeerReturnsFalse) { struct MockDeviceFail : public Mock { struct MockCommandQueueImp : public Mock { diff --git a/level_zero/core/test/unit_tests/sources/memory/test_memory.cpp b/level_zero/core/test/unit_tests/sources/memory/test_memory.cpp index 83849e3599..47599c68c1 100644 --- a/level_zero/core/test/unit_tests/sources/memory/test_memory.cpp +++ b/level_zero/core/test/unit_tests/sources/memory/test_memory.cpp @@ -768,6 +768,7 @@ TEST_F(ContextMemoryTests, givenMultipleSubDevicesWhenAllocatingThenUseCorrectGl struct DriverHandleFailGetFdMock : public L0::DriverHandleImp { void *importFdHandle(ze_device_handle_t hDevice, ze_ipc_memory_flags_t flags, uint64_t handle, NEO::GraphicsAllocation **pAloc) override { + importFdHandleCalledTimes++; if (mockFd == allocationMap.second) { return allocationMap.first; } @@ -776,6 +777,7 @@ struct DriverHandleFailGetFdMock : public L0::DriverHandleImp { const int mockFd = 57; std::pair allocationMap; + uint32_t importFdHandleCalledTimes = 0; }; struct ContextFailFdMock : public L0::ContextImp { @@ -1650,6 +1652,74 @@ struct MemoryOpenIpcHandleTest : public ::testing::Test { std::unique_ptr context; }; +struct MultipleDevicePeerAllocationFailTest : public ::testing::Test { + void SetUp() override { + NEO::MockCompilerEnableGuard mock(true); + + std::vector> devices; + NEO::ExecutionEnvironment *executionEnvironment = new NEO::ExecutionEnvironment(); + executionEnvironment->prepareRootDeviceEnvironments(numRootDevices); + for (auto i = 0u; i < executionEnvironment->rootDeviceEnvironments.size(); i++) { + executionEnvironment->rootDeviceEnvironments[i]->setHwInfo(NEO::defaultHwInfo.get()); + } + + deviceFactory = std::make_unique(numRootDevices, 0, *executionEnvironment); + + for (auto i = 0u; i < executionEnvironment->rootDeviceEnvironments.size(); i++) { + devices.push_back(std::unique_ptr(deviceFactory->rootDevices[i])); + } + driverHandle = std::make_unique(); + driverHandle->initialize(std::move(devices)); + driverHandle->setMemoryManager(driverHandle->getMemoryManager()); + + context = std::make_unique(driverHandle.get()); + EXPECT_NE(context, nullptr); + for (auto i = 0u; i < numRootDevices; i++) { + auto device = driverHandle->devices[i]; + context->getDevices().insert(std::make_pair(device->toHandle(), device)); + auto neoDevice = device->getNEODevice(); + context->rootDeviceIndices.insert(neoDevice->getRootDeviceIndex()); + context->deviceBitfields.insert({neoDevice->getRootDeviceIndex(), neoDevice->getDeviceBitfield()}); + } + } + + DebugManagerStateRestore restorer; + std::unique_ptr driverHandle; + std::unique_ptr deviceFactory; + std::unique_ptr context; + + const uint32_t numRootDevices = 2u; +}; + +TEST_F(MultipleDevicePeerAllocationFailTest, + givenImportFdHandleFailedThenPeerAllocationReturnsNullptr) { + DebugManager.flags.EnableCrossDeviceAccess.set(true); + L0::Device *device0 = driverHandle->devices[0]; + L0::Device *device1 = driverHandle->devices[1]; + + size_t size = 1024; + size_t alignment = 1u; + void *ptr = nullptr; + ze_device_mem_alloc_desc_t deviceDesc = {}; + ze_result_t result = context->allocDeviceMem(device0->toHandle(), + &deviceDesc, + size, alignment, &ptr); + EXPECT_EQ(ZE_RESULT_SUCCESS, result); + EXPECT_NE(nullptr, ptr); + + uintptr_t peerGpuAddress = 0u; + auto allocData = context->getDriverHandle()->getSvmAllocsManager()->getSVMAlloc(ptr); + EXPECT_NE(allocData, nullptr); + + DriverHandleFailGetFdMock *driverHandleFailGetFdMock = static_cast(context->getDriverHandle()); + auto peerAlloc = driverHandle->getPeerAllocation(device1, allocData, ptr, &peerGpuAddress); + EXPECT_GT(driverHandleFailGetFdMock->importFdHandleCalledTimes, 0u); + EXPECT_EQ(peerAlloc, nullptr); + + result = context->freeMem(ptr); + ASSERT_EQ(result, ZE_RESULT_SUCCESS); +} + struct MultipleDevicePeerAllocationTest : public ::testing::Test { void createModuleFromBinary(L0::Device *device, ModuleType type = ModuleType::User) { std::string testFile; @@ -1871,7 +1941,7 @@ HWTEST2_F(MultipleDevicePeerAllocationTest, } HWTEST2_F(MultipleDevicePeerAllocationTest, - givenDeviceAllocationPassedToGetAllignedAllocationWithoutSettingEnableCrossDeviceAccessThenExceptionIsThrown, + givenDeviceAllocationPassedToGetAllignedAllocationWithoutSettingEnableCrossDeviceAccessThenPeerAllocNotFoundReturnsTrue, Platforms) { DebugManager.flags.EnableCrossDeviceAccess.set(false); L0::Device *device0 = driverHandle->devices[0]; @@ -1890,7 +1960,8 @@ HWTEST2_F(MultipleDevicePeerAllocationTest, auto commandList = std::make_unique<::L0::ult::CommandListCoreFamily>(); commandList->initialize(device1, NEO::EngineGroupType::RenderCompute, 0u); - EXPECT_THROW(commandList->getAlignedAllocation(device1, ptr, size, false), std::exception); + AlignedAllocationData outData = commandList->getAlignedAllocation(device1, ptr, size, false); + EXPECT_EQ(nullptr, outData.alloc); result = context->freeMem(ptr); ASSERT_EQ(result, ZE_RESULT_SUCCESS); diff --git a/shared/source/os_interface/linux/drm_memory_manager.cpp b/shared/source/os_interface/linux/drm_memory_manager.cpp index ebd2288e70..f3f65a0441 100644 --- a/shared/source/os_interface/linux/drm_memory_manager.cpp +++ b/shared/source/os_interface/linux/drm_memory_manager.cpp @@ -634,7 +634,6 @@ GraphicsAllocation *DrmMemoryManager::createGraphicsAllocationFromSharedHandle(o if (ret != 0) { [[maybe_unused]] int err = errno; PRINT_DEBUG_STRING(DebugManager.flags.PrintDebugMessages.get(), stderr, "ioctl(PRIME_FD_TO_HANDLE) failed with %d. errno=%d(%s)\n", ret, err, strerror(err)); - DEBUG_BREAK_IF(ret != 0); return nullptr; }