From 0e8c3bf47302817d4c33c54b82baf432111ad026 Mon Sep 17 00:00:00 2001 From: Bellekallu Rajkiran Date: Wed, 8 Nov 2023 10:27:24 +0000 Subject: [PATCH] fix: Remove use of device sysfs paths after unbind Signed-off-by: Bellekallu Rajkiran --- .../linux/sysman_os_global_operations_imp.cpp | 13 ++++++++----- .../linux/mock_global_operations.h | 14 ++++++++------ .../linux/test_zes_global_operations.cpp | 10 ++++++++++ .../linux/os_global_operations_imp.cpp | 13 ++++++++----- .../linux/mock_global_operations.h | 14 ++++++++------ .../linux/test_zes_global_operations.cpp | 10 ++++++++++ 6 files changed, 52 insertions(+), 22 deletions(-) diff --git a/level_zero/sysman/source/api/global_operations/linux/sysman_os_global_operations_imp.cpp b/level_zero/sysman/source/api/global_operations/linux/sysman_os_global_operations_imp.cpp index 2dc1f044bb..ad6aec3ee6 100644 --- a/level_zero/sysman/source/api/global_operations/linux/sysman_os_global_operations_imp.cpp +++ b/level_zero/sysman/source/api/global_operations/linux/sysman_os_global_operations_imp.cpp @@ -35,7 +35,7 @@ const std::string LinuxGlobalOperationsImp::deviceDir("device"); const std::string LinuxGlobalOperationsImp::vendorFile("device/vendor"); const std::string LinuxGlobalOperationsImp::subsystemVendorFile("device/subsystem_vendor"); const std::string LinuxGlobalOperationsImp::driverFile("device/driver"); -const std::string LinuxGlobalOperationsImp::functionLevelReset("device/reset"); +const std::string LinuxGlobalOperationsImp::functionLevelReset("/reset"); const std::string LinuxGlobalOperationsImp::clientsDir("clients"); const std::string LinuxGlobalOperationsImp::srcVersionFile("/sys/module/i915/srcversion"); const std::string LinuxGlobalOperationsImp::agamaVersionFile("/sys/module/i915/agama_version"); @@ -301,7 +301,12 @@ ze_result_t LinuxGlobalOperationsImp::resetImpl(ze_bool_t force, zes_reset_type_ } std::string resetName; - pSysfsAccess->getRealPath(deviceDir, resetName); + result = pSysfsAccess->getRealPath(deviceDir, resetName); + if (result != ZE_RESULT_SUCCESS) { + NEO::printDebugString(NEO::DebugManager.flags.PrintDebugMessages.get(), stderr, "Error@ %s(): Failed to get reset sysfs path and returning error:0x%x \n", __FUNCTION__, result); + return result; + } + std::string flrPath = resetName + functionLevelReset; resetName = pFsAccess->getBaseName(resetName); // Unbind the device from the kernel driver. @@ -346,7 +351,6 @@ ze_result_t LinuxGlobalOperationsImp::resetImpl(ze_bool_t force, zes_reset_type_ } } - std::string resetPath = {}; switch (resetType) { case ZES_RESET_TYPE_WARM: result = pLinuxSysmanImp->osWarmReset(); @@ -355,8 +359,7 @@ ze_result_t LinuxGlobalOperationsImp::resetImpl(ze_bool_t force, zes_reset_type_ result = pLinuxSysmanImp->osColdReset(); break; case ZES_RESET_TYPE_FLR: - pSysfsAccess->getRealPath(functionLevelReset, resetPath); - result = pFsAccess->write(resetPath, "1"); + result = pFsAccess->write(flrPath, "1"); break; default: return ZE_RESULT_ERROR_INVALID_ARGUMENT; diff --git a/level_zero/sysman/test/unit_tests/sources/global_operations/linux/mock_global_operations.h b/level_zero/sysman/test/unit_tests/sources/global_operations/linux/mock_global_operations.h index 3663c17469..886dc1cead 100644 --- a/level_zero/sysman/test/unit_tests/sources/global_operations/linux/mock_global_operations.h +++ b/level_zero/sysman/test/unit_tests/sources/global_operations/linux/mock_global_operations.h @@ -31,7 +31,6 @@ const std::string subsystemVendorFile("device/subsystem_vendor"); const std::string driverFile("device/driver"); const std::string agamaVersionFile("/sys/module/i915/agama_version"); const std::string srcVersionFile("/sys/module/i915/srcversion"); -const std::string functionLevelReset("device/reset"); const std::string clientsDir("clients"); constexpr uint64_t pid1 = 1711u; constexpr uint64_t pid2 = 1722u; @@ -58,8 +57,8 @@ const std::string engine6("6"); const std::string driverVersion("5.0.0-37-generic SMP mod_unload"); const std::string srcVersion("5.0.0-37"); const std::string ueventWedgedFile("/var/lib/libze_intel_gpu/wedged_file"); -const std::string mockFunctionResetPath("/MOCK_FUNCTION_LEVEL_RESET_PATH"); const std::string mockDeviceDir("devices/pci0000:89/0000:89:02.0/0000:8a:00.0/0000:8b:01.0/0000:8c:00.0"); +const std::string mockFunctionResetPath(mockDeviceDir + "/reset"); const std::string mockDeviceName("/MOCK_DEVICE_NAME"); const std::string mockSlotPath("/sys/bus/pci/slots/1"); const std::string mockSlotPathAddress("/sys/bus/pci/slots/1/address"); @@ -105,16 +104,15 @@ struct MockGlobalOperationsSysfsAccess : public L0::Sysman::SysfsAccess { bool mockGetScannedDirPidEntriesForClientsStatus = false; bool mockReadStatus = false; bool mockGetValUnsignedLongStatus = false; + bool mockDeviceUnbound = false; ze_result_t getRealPath(const std::string file, std::string &val) override { - if (file.compare(functionLevelReset) == 0) { - val = mockFunctionResetPath; - } else if (file.compare(deviceDir) == 0) { + if (file.compare(deviceDir) == 0) { val = mockDeviceDir; } else { return ZE_RESULT_ERROR_NOT_AVAILABLE; } - return ZE_RESULT_SUCCESS; + return mockDeviceUnbound ? ZE_RESULT_ERROR_NOT_AVAILABLE : ZE_RESULT_SUCCESS; } enum class Index { @@ -345,6 +343,8 @@ struct MockGlobalOperationsSysfsAccess : public L0::Sysman::SysfsAccess { return mockUnbindDeviceError; } + mockDeviceUnbound = true; + return ZE_RESULT_SUCCESS; } @@ -353,6 +353,8 @@ struct MockGlobalOperationsSysfsAccess : public L0::Sysman::SysfsAccess { return mockBindDeviceError; } + mockDeviceUnbound = false; + return ZE_RESULT_SUCCESS; } diff --git a/level_zero/sysman/test/unit_tests/sources/global_operations/linux/test_zes_global_operations.cpp b/level_zero/sysman/test/unit_tests/sources/global_operations/linux/test_zes_global_operations.cpp index 98cff87a38..a565391dc0 100644 --- a/level_zero/sysman/test/unit_tests/sources/global_operations/linux/test_zes_global_operations.cpp +++ b/level_zero/sysman/test/unit_tests/sources/global_operations/linux/test_zes_global_operations.cpp @@ -781,6 +781,16 @@ TEST_F(SysmanGlobalOperationsFixture, GivenDeviceInUseWhenCallingResetExtWithInv EXPECT_EQ(ZE_RESULT_ERROR_INVALID_ARGUMENT, result); } +TEST_F(SysmanGlobalOperationsFixture, GivenGettingSysfsPathFailsWhenCallingResetExtThenFailureIsReturned) { + init(true); + DebugManagerStateRestore dbgRestore; + DebugManager.flags.VfBarResourceAllocationWa.set(false); + pSysfsAccess->mockDeviceUnbound = true; + zes_reset_properties_t pProperties = {.stype = ZES_STRUCTURE_TYPE_RESET_PROPERTIES, .pNext = nullptr, .force = true, .resetType = ZES_RESET_TYPE_FORCE_UINT32}; + ze_result_t result = zesDeviceResetExt(pSysmanDevice->toHandle(), &pProperties); + EXPECT_EQ(ZE_RESULT_ERROR_NOT_AVAILABLE, result); +} + TEST_F(SysmanGlobalOperationsFixture, GivenForceTrueWhenCallingResetThenSuccessIsReturned) { DebugManagerStateRestore dbgRestore; DebugManager.flags.VfBarResourceAllocationWa.set(false); diff --git a/level_zero/tools/source/sysman/global_operations/linux/os_global_operations_imp.cpp b/level_zero/tools/source/sysman/global_operations/linux/os_global_operations_imp.cpp index 6fdab74126..1e960d14ab 100644 --- a/level_zero/tools/source/sysman/global_operations/linux/os_global_operations_imp.cpp +++ b/level_zero/tools/source/sysman/global_operations/linux/os_global_operations_imp.cpp @@ -31,7 +31,7 @@ namespace L0 { const std::string LinuxGlobalOperationsImp::deviceDir("device"); const std::string LinuxGlobalOperationsImp::subsystemVendorFile("device/subsystem_vendor"); const std::string LinuxGlobalOperationsImp::driverFile("device/driver"); -const std::string LinuxGlobalOperationsImp::functionLevelReset("device/reset"); +const std::string LinuxGlobalOperationsImp::functionLevelReset("/reset"); const std::string LinuxGlobalOperationsImp::clientsDir("clients"); const std::string LinuxGlobalOperationsImp::srcVersionFile("/sys/module/i915/srcversion"); const std::string LinuxGlobalOperationsImp::agamaVersionFile("/sys/module/i915/agama_version"); @@ -223,7 +223,12 @@ ze_result_t LinuxGlobalOperationsImp::resetImpl(ze_bool_t force, zes_reset_type_ } std::string resetName; - pSysfsAccess->getRealPath(deviceDir, resetName); + result = pSysfsAccess->getRealPath(deviceDir, resetName); + if (result != ZE_RESULT_SUCCESS) { + NEO::printDebugString(NEO::DebugManager.flags.PrintDebugMessages.get(), stderr, "Error@ %s(): Failed to get reset sysfs path and returning error:0x%x \n", __FUNCTION__, result); + return result; + } + std::string flrPath = resetName + functionLevelReset; resetName = pFsAccess->getBaseName(resetName); // Unbind the device from the kernel driver. @@ -268,7 +273,6 @@ ze_result_t LinuxGlobalOperationsImp::resetImpl(ze_bool_t force, zes_reset_type_ } } - std::string resetPath = {}; switch (resetType) { case ZES_RESET_TYPE_WARM: result = pLinuxSysmanImp->osWarmReset(); @@ -277,8 +281,7 @@ ze_result_t LinuxGlobalOperationsImp::resetImpl(ze_bool_t force, zes_reset_type_ result = pLinuxSysmanImp->osColdReset(); break; case ZES_RESET_TYPE_FLR: - pSysfsAccess->getRealPath(functionLevelReset, resetPath); - result = pFsAccess->write(resetPath, "1"); + result = pFsAccess->write(flrPath, "1"); break; default: return ZE_RESULT_ERROR_INVALID_ARGUMENT; diff --git a/level_zero/tools/test/unit_tests/sources/sysman/global_operations/linux/mock_global_operations.h b/level_zero/tools/test/unit_tests/sources/sysman/global_operations/linux/mock_global_operations.h index a05d0bfeba..9b6edf3b14 100644 --- a/level_zero/tools/test/unit_tests/sources/sysman/global_operations/linux/mock_global_operations.h +++ b/level_zero/tools/test/unit_tests/sources/sysman/global_operations/linux/mock_global_operations.h @@ -23,7 +23,6 @@ const std::string subsystemVendorFile("device/subsystem_vendor"); const std::string driverFile("device/driver"); const std::string agamaVersionFile("/sys/module/i915/agama_version"); const std::string srcVersionFile("/sys/module/i915/srcversion"); -const std::string functionLevelReset("device/reset"); const std::string clientsDir("clients"); constexpr uint64_t pid1 = 1711u; constexpr uint64_t pid2 = 1722u; @@ -50,8 +49,8 @@ const std::string engine6("6"); const std::string driverVersion("5.0.0-37-generic SMP mod_unload"); const std::string srcVersion("5.0.0-37"); const std::string ueventWedgedFile("/var/lib/libze_intel_gpu/wedged_file"); -const std::string mockFunctionResetPath("/MOCK_FUNCTION_LEVEL_RESET_PATH"); const std::string mockDeviceDir("devices/pci0000:89/0000:89:02.0/0000:8a:00.0/0000:8b:01.0/0000:8c:00.0"); +const std::string mockFunctionResetPath(mockDeviceDir + "/reset"); const std::string mockDeviceName("/MOCK_DEVICE_NAME"); enum mockEnumListProcessCall { @@ -93,16 +92,15 @@ struct MockGlobalOperationsSysfsAccess : public SysfsAccess { bool mockGetScannedDirPidEntriesForClientsStatus = false; bool mockReadStatus = false; bool mockGetValUnsignedLongStatus = false; + bool mockDeviceUnbound = false; ze_result_t getRealPath(const std::string file, std::string &val) override { - if (file.compare(functionLevelReset) == 0) { - val = mockFunctionResetPath; - } else if (file.compare(deviceDir) == 0) { + if (file.compare(deviceDir) == 0) { val = mockDeviceDir; } else { return ZE_RESULT_ERROR_NOT_AVAILABLE; } - return ZE_RESULT_SUCCESS; + return mockDeviceUnbound ? ZE_RESULT_ERROR_NOT_AVAILABLE : ZE_RESULT_SUCCESS; } ze_result_t readResult = ZE_RESULT_SUCCESS; @@ -322,6 +320,8 @@ struct MockGlobalOperationsSysfsAccess : public SysfsAccess { return mockUnbindDeviceError; } + mockDeviceUnbound = true; + return ZE_RESULT_SUCCESS; } @@ -330,6 +330,8 @@ struct MockGlobalOperationsSysfsAccess : public SysfsAccess { return mockBindDeviceError; } + mockDeviceUnbound = false; + return ZE_RESULT_SUCCESS; } diff --git a/level_zero/tools/test/unit_tests/sources/sysman/global_operations/linux/test_zes_global_operations.cpp b/level_zero/tools/test/unit_tests/sources/sysman/global_operations/linux/test_zes_global_operations.cpp index 9baa9cceb2..2bab45cdb3 100644 --- a/level_zero/tools/test/unit_tests/sources/sysman/global_operations/linux/test_zes_global_operations.cpp +++ b/level_zero/tools/test/unit_tests/sources/sysman/global_operations/linux/test_zes_global_operations.cpp @@ -179,6 +179,16 @@ TEST_F(SysmanGlobalOperationsFixture, GivenDeviceInUseWhenCallingResetExtWithInv EXPECT_EQ(ZE_RESULT_ERROR_INVALID_ARGUMENT, result); } +TEST_F(SysmanGlobalOperationsFixture, GivenGettingSysfsPathFailsWhenCallingResetExtThenFailureIsReturned) { + DebugManagerStateRestore dbgRestore; + DebugManager.flags.VfBarResourceAllocationWa.set(false); + initGlobalOps(); + pSysfsAccess->mockDeviceUnbound = true; + zes_reset_properties_t pProperties = {.stype = ZES_STRUCTURE_TYPE_RESET_PROPERTIES, .pNext = nullptr, .force = true, .resetType = ZES_RESET_TYPE_FORCE_UINT32}; + ze_result_t result = zesDeviceResetExt(device, &pProperties); + EXPECT_EQ(ZE_RESULT_ERROR_NOT_AVAILABLE, result); +} + TEST_F(SysmanGlobalOperationsFixture, GivenValidDeviceHandleWhenCallingzesGlobalOperationsGetPropertiesThenVerifyValidPropertiesAreReturned) { VariableBackup mockReadLink(&NEO::SysCalls::sysCallsReadlink, [](const char *path, char *buf, size_t bufsize) -> int {