From 66f5af0fec0de374fbd20a9d610dc633f584d41b Mon Sep 17 00:00:00 2001 From: Igor Venevtsev Date: Mon, 3 Oct 2022 11:33:59 +0000 Subject: [PATCH] L0Debug: simple code cleanup Use dedicated readSystemRoutineIdent() instead of raw readGpuMemory() Signed-off-by: Igor Venevtsev --- .../tools/source/debug/debug_session_imp.cpp | 33 +++---- .../sources/debug/debug_session_tests.cpp | 89 ++++++++----------- .../debug/linux/test_debug_api_linux.cpp | 10 +-- .../debug/windows/test_debug_api_windows.cpp | 6 +- 4 files changed, 60 insertions(+), 78 deletions(-) diff --git a/level_zero/tools/source/debug/debug_session_imp.cpp b/level_zero/tools/source/debug/debug_session_imp.cpp index 6df3b5b13a..27d0bb4c01 100644 --- a/level_zero/tools/source/debug/debug_session_imp.cpp +++ b/level_zero/tools/source/debug/debug_session_imp.cpp @@ -459,23 +459,10 @@ bool DebugSessionImp::checkThreadIsResumed(const EuThread::ThreadId &threadID) { bool resumed = true; if (stateSaveAreaHeader->versionHeader.version.major >= 2u) { + SIP::sr_ident srMagic = {{0}}; const auto thread = allThreads[threadID].get(); - auto gpuVa = getContextStateSaveAreaGpuVa(thread->getMemoryHandle()); - if (gpuVa == 0) { - PRINT_DEBUGGER_ERROR_LOG("Failed to get Context State Save Area GPU Virtual Address\n", ""); - return resumed; - } - auto threadSlotOffset = calculateThreadSlotOffset(thread->getThreadId()); - auto srMagicOffset = threadSlotOffset + getStateSaveAreaHeader()->regHeader.sr_magic_offset; - SIP::sr_ident srMagic; - memset(srMagic.magic, 0, sizeof(SIP::sr_ident::magic)); - - auto status = readGpuMemory(thread->getMemoryHandle(), reinterpret_cast(&srMagic), sizeof(srMagic), gpuVa + srMagicOffset); - DEBUG_BREAK_IF(status != ZE_RESULT_SUCCESS); - - if (status != ZE_RESULT_SUCCESS || 0 != strcmp(srMagic.magic, "srmagic")) { - PRINT_DEBUGGER_ERROR_LOG("checkThreadIsResumed - Failed to read srMagic for thread %s\n", EuThread::toString(threadID).c_str()); + if (!readSystemRoutineIdent(thread, thread->getMemoryHandle(), srMagic)) { return resumed; } @@ -649,18 +636,25 @@ bool DebugSessionImp::readSystemRoutineIdent(EuThread *thread, uint64_t memoryHa if (gpuVa == 0) { return false; } + auto threadSlotOffset = calculateThreadSlotOffset(thread->getThreadId()); - auto srMagicOffset = threadSlotOffset + getStateSaveAreaHeader()->regHeader.sr_magic_offset; + auto srMagicOffset = threadSlotOffset + stateSaveAreaHeader->regHeader.sr_magic_offset; if (ZE_RESULT_SUCCESS != readGpuMemory(memoryHandle, reinterpret_cast(&srIdent), sizeof(srIdent), gpuVa + srMagicOffset)) { return false; } + + if (0 != strcmp(srIdent.magic, "srmagic")) { + PRINT_DEBUGGER_ERROR_LOG("readSystemRoutineIdent - Failed to read srMagic for thread %s\n", EuThread::toString(thread->getThreadId()).c_str()); + return false; + } + return true; } void DebugSessionImp::markPendingInterruptsOrAddToNewlyStoppedFromRaisedAttention(EuThread::ThreadId threadId, uint64_t memoryHandle) { - SIP::sr_ident srMagic = {}; + SIP::sr_ident srMagic = {{0}}; srMagic.count = 0; bool wasStopped = false; @@ -1115,10 +1109,9 @@ ze_result_t DebugSessionImp::registersAccessHelper(const EuThread *thread, const auto threadSlotOffset = calculateThreadSlotOffset(thread->getThreadId()); - auto srMagicOffset = threadSlotOffset + getStateSaveAreaHeader()->regHeader.sr_magic_offset; - SIP::sr_ident srMagic; - memset(srMagic.magic, 0, sizeof(SIP::sr_ident::magic)); + SIP::sr_ident srMagic = {{0}}; + auto srMagicOffset = threadSlotOffset + getStateSaveAreaHeader()->regHeader.sr_magic_offset; readGpuMemory(thread->getMemoryHandle(), reinterpret_cast(&srMagic), sizeof(srMagic), gpuVa + srMagicOffset); if (0 != strcmp(srMagic.magic, "srmagic")) { return ZE_RESULT_ERROR_UNKNOWN; diff --git a/level_zero/tools/test/unit_tests/sources/debug/debug_session_tests.cpp b/level_zero/tools/test/unit_tests/sources/debug/debug_session_tests.cpp index 8c50133cb9..019c27bc0d 100644 --- a/level_zero/tools/test/unit_tests/sources/debug/debug_session_tests.cpp +++ b/level_zero/tools/test/unit_tests/sources/debug/debug_session_tests.cpp @@ -207,8 +207,11 @@ struct MockDebugSession : public L0::DebugSessionImp { } bool readSystemRoutineIdent(EuThread *thread, uint64_t vmHandle, SIP::sr_ident &srMagic) override { - srMagic.count = threadStopped ? 1 : 0; - return readSystemRoutineIdentRetVal; + if (skipReadSystemRoutineIdent) { + srMagic.count = threadStopped ? 1 : 0; + return readSystemRoutineIdentRetVal; + } + return DebugSessionImp::readSystemRoutineIdent(thread, vmHandle, srMagic); } bool areRequestedThreadsStopped(ze_device_thread_t thread) override { @@ -316,6 +319,7 @@ struct MockDebugSession : public L0::DebugSessionImp { bool skipCheckThreadIsResumed = true; uint32_t checkThreadIsResumedCalled = 0; + bool skipReadSystemRoutineIdent = true; std::vector events; std::vector interruptedDevices; std::vector resumedDevices; @@ -758,11 +762,9 @@ TEST(DebugSessionTest, givenThreadsToResumeWhenResumeAccidentallyStoppedThreadsC class InternalMockDebugSession : public MockDebugSession { public: InternalMockDebugSession(const zet_debug_config_t &config, L0::Device *device) : MockDebugSession(config, device) {} - ze_result_t readGpuMemory(uint64_t memoryHandle, char *output, size_t size, uint64_t gpuVa) override { - SIP::sr_ident srMagic; + bool readSystemRoutineIdent(EuThread *thread, uint64_t vmHandle, SIP::sr_ident &srMagic) override { srMagic.count = counter++; - memcpy_s(output, size, reinterpret_cast(&srMagic), sizeof(srMagic)); - return readMemoryResult; + return true; } int counter = 0; @@ -992,17 +994,33 @@ TEST(DebugSessionTest, whenCallingCheckThreadIsResumedWithoutSrMagicThenThreadIs auto sessionMock = std::make_unique(config, &deviceImp); ASSERT_NE(nullptr, sessionMock); sessionMock->skipCheckThreadIsResumed = false; + sessionMock->skipReadSystemRoutineIdent = false; sessionMock->stateSaveAreaHeader = MockSipData::createStateSaveAreaHeader(2); + { + auto pStateSaveAreaHeader = reinterpret_cast(sessionMock->stateSaveAreaHeader.data()); + auto size = pStateSaveAreaHeader->versionHeader.size * 8 + + pStateSaveAreaHeader->regHeader.state_area_offset + + pStateSaveAreaHeader->regHeader.state_save_size * 16; + sessionMock->stateSaveAreaHeader.resize(size); + } + + auto stateSaveAreaHeader = reinterpret_cast(sessionMock->stateSaveAreaHeader.data()); ze_device_thread_t thread = {0, 0, 0, 0}; EuThread::ThreadId threadId(0, thread); + + auto threadSlotOffset = sessionMock->calculateThreadSlotOffset(threadId); + auto srMagicOffset = threadSlotOffset + stateSaveAreaHeader->regHeader.sr_magic_offset; + SIP::sr_ident srMagic = {{0}}; + sessionMock->writeGpuMemory(0, reinterpret_cast(&srMagic), sizeof(srMagic), reinterpret_cast(stateSaveAreaHeader) + srMagicOffset); + bool resumed = sessionMock->checkThreadIsResumed(threadId); EXPECT_TRUE(resumed); EXPECT_EQ(1u, sessionMock->checkThreadIsResumedCalled); } -TEST(DebugSessionTest, givenErrorFromReadGpuMemoryWhenCallingCheckThreadIsResumedThenThreadIsAssumedRunning) { +TEST(DebugSessionTest, givenErrorFromReadSystemRoutineIdentWhenCallingCheckThreadIsResumedThenThreadIsAssumedRunning) { zet_debug_config_t config = {}; config.pid = 0x1234; auto hwInfo = *NEO::defaultHwInfo.get(); @@ -1013,7 +1031,7 @@ TEST(DebugSessionTest, givenErrorFromReadGpuMemoryWhenCallingCheckThreadIsResume auto sessionMock = std::make_unique(config, &deviceImp); ASSERT_NE(nullptr, sessionMock); sessionMock->skipCheckThreadIsResumed = false; - sessionMock->readMemoryResult = ZE_RESULT_ERROR_UNKNOWN; + sessionMock->readSystemRoutineIdentRetVal = false; sessionMock->stateSaveAreaHeader = MockSipData::createStateSaveAreaHeader(2); ze_device_thread_t thread = {0, 0, 0, 0}; @@ -1028,11 +1046,9 @@ TEST(DebugSessionTest, givenSrMagicWithCounterLessThanlLastThreadCounterThenThre class InternalMockDebugSession : public MockDebugSession { public: InternalMockDebugSession(const zet_debug_config_t &config, L0::Device *device) : MockDebugSession(config, device) {} - ze_result_t readGpuMemory(uint64_t memoryHandle, char *output, size_t size, uint64_t gpuVa) override { - SIP::sr_ident srMagic; + bool readSystemRoutineIdent(EuThread *thread, uint64_t vmHandle, SIP::sr_ident &srMagic) override { srMagic.count = 0; - memcpy_s(output, size, reinterpret_cast(&srMagic), sizeof(srMagic)); - return readMemoryResult; + return true; } }; zet_debug_config_t config = {}; @@ -1061,11 +1077,9 @@ TEST(DebugSessionTest, givenSrMagicWithCounterEqualToPrevousThenThreadHasNotBeen class InternalMockDebugSession : public MockDebugSession { public: InternalMockDebugSession(const zet_debug_config_t &config, L0::Device *device) : MockDebugSession(config, device) {} - ze_result_t readGpuMemory(uint64_t memoryHandle, char *output, size_t size, uint64_t gpuVa) override { - SIP::sr_ident srMagic; + bool readSystemRoutineIdent(EuThread *thread, uint64_t vmHandle, SIP::sr_ident &srMagic) override { srMagic.count = 1; - memcpy_s(output, size, reinterpret_cast(&srMagic), sizeof(srMagic)); - return readMemoryResult; + return true; } }; zet_debug_config_t config = {}; @@ -1096,11 +1110,9 @@ TEST(DebugSessionTest, givenSrMagicWithCounterBiggerThanPreviousThenThreadIsResu class InternalMockDebugSession : public MockDebugSession { public: InternalMockDebugSession(const zet_debug_config_t &config, L0::Device *device) : MockDebugSession(config, device) {} - ze_result_t readGpuMemory(uint64_t memoryHandle, char *output, size_t size, uint64_t gpuVa) override { - SIP::sr_ident srMagic; + bool readSystemRoutineIdent(EuThread *thread, uint64_t vmHandle, SIP::sr_ident &srMagic) override { srMagic.count = 3; - memcpy_s(output, size, reinterpret_cast(&srMagic), sizeof(srMagic)); - return readMemoryResult; + return true; } }; zet_debug_config_t config = {}; @@ -1131,11 +1143,9 @@ TEST(DebugSessionTest, givenSrMagicWithCounterOverflowingZeroThenThreadIsResumed class InternalMockDebugSession : public MockDebugSession { public: InternalMockDebugSession(const zet_debug_config_t &config, L0::Device *device) : MockDebugSession(config, device) {} - ze_result_t readGpuMemory(uint64_t memoryHandle, char *output, size_t size, uint64_t gpuVa) override { - SIP::sr_ident srMagic; + bool readSystemRoutineIdent(EuThread *thread, uint64_t vmHandle, SIP::sr_ident &srMagic) override { srMagic.count = 0; - memcpy_s(output, size, reinterpret_cast(&srMagic), sizeof(srMagic)); - return readMemoryResult; + return true; } }; zet_debug_config_t config = {}; @@ -1339,31 +1349,13 @@ TEST(DebugSessionTest, GivenBindlessSipVersion2WhenWritingResumeFailsThenErrorIs class InternalMockDebugSession : public MockDebugSession { public: InternalMockDebugSession(const zet_debug_config_t &config, L0::Device *device) : MockDebugSession(config, device) {} - ze_result_t readGpuMemory(uint64_t memoryHandle, char *output, size_t size, uint64_t gpuVa) override { - switch (counter) { - case 0: - case 1: - MockDebugSession::readGpuMemory(memoryHandle, output, size, gpuVa); - counter++; - break; - default: - SIP::sr_ident srMagic; - if (firstEntry) { - srMagic.count = 1; - firstEntry = false; - } else { - srMagic.count = 2; - } - memcpy_s(output, size, reinterpret_cast(&srMagic), sizeof(srMagic)); - break; - } - - return readMemoryResult; + bool readSystemRoutineIdent(EuThread *thread, uint64_t vmHandle, SIP::sr_ident &srMagic) override { + srMagic.count = ++counter; + return true; } private: int counter = 0; - bool firstEntry = true; }; zet_debug_config_t config = {}; config.pid = 0x1234; @@ -1396,18 +1388,15 @@ TEST(DebugSessionTest, GivenBindlessSipVersion2WhenResumingThreadThenCheckIfThre class InternalMockDebugSession : public MockDebugSession { public: InternalMockDebugSession(const zet_debug_config_t &config, L0::Device *device) : MockDebugSession(config, device) {} - ze_result_t readGpuMemory(uint64_t memoryHandle, char *output, size_t size, uint64_t gpuVa) override { - - SIP::sr_ident srMagic{}; + bool readSystemRoutineIdent(EuThread *thread, uint64_t vmHandle, SIP::sr_ident &srMagic) override { if (counter > 5) { srMagic.count = 4; } else { srMagic.count = 1; } - memcpy_s(output, size, reinterpret_cast(&srMagic), sizeof(srMagic)); counter++; - return readMemoryResult; + return true; } int counter = 0; }; diff --git a/level_zero/tools/test/unit_tests/sources/debug/linux/test_debug_api_linux.cpp b/level_zero/tools/test/unit_tests/sources/debug/linux/test_debug_api_linux.cpp index 833419da8f..260de2bc62 100644 --- a/level_zero/tools/test/unit_tests/sources/debug/linux/test_debug_api_linux.cpp +++ b/level_zero/tools/test/unit_tests/sources/debug/linux/test_debug_api_linux.cpp @@ -6355,7 +6355,7 @@ TEST_F(DebugApiRegistersAccessTest, GivenThreadWhenReadingSystemRoutineIdentThen session->ioctlHandler.reset(ioctlHandler); EuThread thread({0, 0, 0, 0, 0}); - SIP::sr_ident srIdent = {}; + SIP::sr_ident srIdent = {{0}}; auto result = session->readSystemRoutineIdent(&thread, vmHandle, srIdent); EXPECT_TRUE(result); @@ -6420,7 +6420,7 @@ TEST_F(DebugApiRegistersAccessTest, GivenNoVmHandleWhenReadingSystemRoutineIdent EuThread thread({0, 0, 0, 0, 0}); - SIP::sr_ident srIdent = {}; + SIP::sr_ident srIdent = {{0}}; auto result = session->readSystemRoutineIdent(&thread, DebugSessionLinux::invalidHandle, srIdent); EXPECT_FALSE(result); @@ -6431,7 +6431,7 @@ TEST_F(DebugApiRegistersAccessTest, GivenNoStatSaveAreaWhenReadingSystemRoutineI session->ioctlHandler.reset(ioctlHandler); EuThread thread({0, 0, 0, 0, 0}); - SIP::sr_ident srIdent = {}; + SIP::sr_ident srIdent = {{0}}; auto result = session->readSystemRoutineIdent(&thread, 0x1234u, srIdent); EXPECT_FALSE(result); @@ -6450,7 +6450,7 @@ TEST_F(DebugApiRegistersAccessTest, GivenMemReadFailureWhenReadingSystemRoutineI EuThread thread({0, 0, 0, 0, 0}); - SIP::sr_ident srIdent = {}; + SIP::sr_ident srIdent = {{0}}; auto result = session->readSystemRoutineIdent(&thread, vmHandle, srIdent); EXPECT_FALSE(result); @@ -6469,7 +6469,7 @@ TEST_F(DebugApiRegistersAccessTest, GivenCSSANotBoundWhenReadingSystemRoutineIde EuThread thread({0, 0, 0, 0, 0}); - SIP::sr_ident srIdent = {}; + SIP::sr_ident srIdent = {{0}}; auto result = session->readSystemRoutineIdent(&thread, 0x1234u, srIdent); EXPECT_FALSE(result); diff --git a/level_zero/tools/test/unit_tests/sources/debug/windows/test_debug_api_windows.cpp b/level_zero/tools/test/unit_tests/sources/debug/windows/test_debug_api_windows.cpp index 88f35f99e1..7af1c03a8c 100644 --- a/level_zero/tools/test/unit_tests/sources/debug/windows/test_debug_api_windows.cpp +++ b/level_zero/tools/test/unit_tests/sources/debug/windows/test_debug_api_windows.cpp @@ -300,7 +300,7 @@ TEST_F(DebugApiWindowsAttentionTest, GivenThreadWhenReadingSystemRoutineIdentThe mockWddm->srcReadBuffer = session->stateSaveAreaHeader.data(); mockWddm->srcReadBufferBaseAddress = session->stateSaveAreaVA.load(); - SIP::sr_ident srIdent = {}; + SIP::sr_ident srIdent = {{0}}; auto vmHandle = *session->allContexts.begin(); auto result = session->readSystemRoutineIdent(&thread, vmHandle, srIdent); @@ -343,7 +343,7 @@ TEST_F(DebugApiWindowsAttentionTest, GivenThreadWhenReadingSystemRoutineIdentAnd mockWddm->srcReadBuffer = session->stateSaveAreaHeader.data(); mockWddm->srcReadBufferBaseAddress = session->stateSaveAreaVA.load(); - SIP::sr_ident srIdent = {}; + SIP::sr_ident srIdent = {{0}}; auto vmHandle = *session->allContexts.begin(); auto result = session->readSystemRoutineIdent(&thread, vmHandle, srIdent); @@ -369,7 +369,7 @@ TEST_F(DebugApiWindowsAttentionTest, GivenThreadWhenReadingSystemRoutineIdentAnd mockWddm->srcReadBuffer = session->stateSaveAreaHeader.data(); mockWddm->srcReadBufferBaseAddress = session->stateSaveAreaVA.load(); - SIP::sr_ident srIdent = {}; + SIP::sr_ident srIdent = {{0}}; auto vmHandle = *session->allContexts.begin(); auto result = session->readSystemRoutineIdent(&thread, vmHandle, srIdent);