From 145f03c294e1b19d7b43718d4a7a426e34deae62 Mon Sep 17 00:00:00 2001 From: Mateusz Jablonski Date: Fri, 24 Oct 2025 09:53:03 +0000 Subject: [PATCH] refactor: encapsulate gmm page table manager handle wrap destruction logic within unique ptr set csr handle when creating page table manager Related-To: NEO-11080 Signed-off-by: Mateusz Jablonski --- .../windows/cl_wddm_memory_manager_tests.cpp | 2 +- shared/source/gmm_helper/page_table_mngr.cpp | 6 +++--- shared/source/gmm_helper/page_table_mngr.h | 14 +++++++------- .../source/gmm_helper/page_table_mngr_impl.cpp | 8 +------- .../linux/page_table_manager_functions.cpp | 14 ++++++++------ .../windows/page_table_manager_functions.cpp | 17 ++++++++++------- .../windows/wddm_device_command_stream.inl | 7 +++---- .../common/mocks/mock_gmm_page_table_mngr.cpp | 10 +++------- .../common/mocks/mock_gmm_page_table_mngr.h | 11 ++++------- .../windows/device_command_stream_tests.cpp | 5 ++--- .../windows/wddm_memory_manager_tests.cpp | 2 +- 11 files changed, 43 insertions(+), 53 deletions(-) diff --git a/opencl/test/unit_test/os_interface/windows/cl_wddm_memory_manager_tests.cpp b/opencl/test/unit_test/os_interface/windows/cl_wddm_memory_manager_tests.cpp index 290e18e35c..9dcbce7021 100644 --- a/opencl/test/unit_test/os_interface/windows/cl_wddm_memory_manager_tests.cpp +++ b/opencl/test/unit_test/os_interface/windows/cl_wddm_memory_manager_tests.cpp @@ -53,7 +53,7 @@ void ClWddmMemoryManagerFixture::setUp() { for (auto engine : memoryManager->getRegisteredEngines(rootDeviceIndex)) { if (engine.getEngineUsage() == EngineUsage::regular) { - engine.commandStreamReceiver->pageTableManager.reset(GmmPageTableMngr::create(nullptr, 0, &dummyTTCallbacks)); + engine.commandStreamReceiver->pageTableManager.reset(GmmPageTableMngr::create(nullptr, 0, &dummyTTCallbacks, nullptr)); } } } diff --git a/shared/source/gmm_helper/page_table_mngr.cpp b/shared/source/gmm_helper/page_table_mngr.cpp index f507567b66..cdcb2b53b9 100644 --- a/shared/source/gmm_helper/page_table_mngr.cpp +++ b/shared/source/gmm_helper/page_table_mngr.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2020-2021 Intel Corporation + * Copyright (C) 2020-2025 Intel Corporation * * SPDX-License-Identifier: MIT * @@ -8,8 +8,8 @@ #include "shared/source/gmm_helper/page_table_mngr.h" namespace NEO { -GmmPageTableMngr *GmmPageTableMngr::create(GmmClientContext *clientContext, unsigned int translationTableFlags, GMM_TRANSLATIONTABLE_CALLBACKS *translationTableCb) { - return new GmmPageTableMngr(clientContext, translationTableFlags, translationTableCb); +GmmPageTableMngr *GmmPageTableMngr::create(GmmClientContext *clientContext, unsigned int translationTableFlags, GMM_TRANSLATIONTABLE_CALLBACKS *translationTableCb, void *aubCsrHandle) { + return new GmmPageTableMngr(clientContext, translationTableFlags, translationTableCb, aubCsrHandle); } } // namespace NEO diff --git a/shared/source/gmm_helper/page_table_mngr.h b/shared/source/gmm_helper/page_table_mngr.h index a93717a7fb..026800f0ae 100644 --- a/shared/source/gmm_helper/page_table_mngr.h +++ b/shared/source/gmm_helper/page_table_mngr.h @@ -11,17 +11,18 @@ #include "External/Common/GmmPageTableMgr.h" +#include +#include + namespace NEO { class Gmm; class GmmClientContext; class LinearStream; class GmmPageTableMngr : NonCopyableAndNonMovableClass { public: - MOCKABLE_VIRTUAL ~GmmPageTableMngr(); + MOCKABLE_VIRTUAL ~GmmPageTableMngr() = default; - static GmmPageTableMngr *create(GmmClientContext *clientContext, unsigned int translationTableFlags, GMM_TRANSLATIONTABLE_CALLBACKS *translationTableCb); - - MOCKABLE_VIRTUAL void setCsrHandle(void *csrHandle); + static GmmPageTableMngr *create(GmmClientContext *clientContext, unsigned int translationTableFlags, GMM_TRANSLATIONTABLE_CALLBACKS *translationTableCb, void *aubCsrHandle); bool updateAuxTable(uint64_t gpuVa, Gmm *gmm, bool map); bool initPageTableManagerRegisters(void *csrHandle); @@ -37,9 +38,8 @@ class GmmPageTableMngr : NonCopyableAndNonMovableClass { return pageTableManager->InitContextAuxTableRegister(initialBBHandle, engineType); } - GmmPageTableMngr(GmmClientContext *clientContext, unsigned int translationTableFlags, GMM_TRANSLATIONTABLE_CALLBACKS *translationTableCb); - GMM_CLIENT_CONTEXT *clientContext = nullptr; - GMM_PAGETABLE_MGR *pageTableManager = nullptr; + GmmPageTableMngr(GmmClientContext *clientContext, unsigned int translationTableFlags, GMM_TRANSLATIONTABLE_CALLBACKS *translationTableCb, void *aubCsrHandle); + std::unique_ptr> pageTableManager; }; static_assert(NonCopyableAndNonMovable); diff --git a/shared/source/gmm_helper/page_table_mngr_impl.cpp b/shared/source/gmm_helper/page_table_mngr_impl.cpp index 4a848e4ed4..429054df7d 100644 --- a/shared/source/gmm_helper/page_table_mngr_impl.cpp +++ b/shared/source/gmm_helper/page_table_mngr_impl.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2018-2021 Intel Corporation + * Copyright (C) 2018-2025 Intel Corporation * * SPDX-License-Identifier: MIT * @@ -12,12 +12,6 @@ #include "shared/source/gmm_helper/resource_info.h" namespace NEO { -GmmPageTableMngr::~GmmPageTableMngr() { - if (clientContext) { - clientContext->DestroyPageTblMgrObject(pageTableManager); - } -} - bool GmmPageTableMngr::updateAuxTable(uint64_t gpuVa, Gmm *gmm, bool map) { GMM_DDI_UPDATEAUXTABLE ddiUpdateAuxTable = {}; ddiUpdateAuxTable.BaseGpuVA = gpuVa; diff --git a/shared/source/os_interface/linux/page_table_manager_functions.cpp b/shared/source/os_interface/linux/page_table_manager_functions.cpp index cda937527c..dc048fb838 100644 --- a/shared/source/os_interface/linux/page_table_manager_functions.cpp +++ b/shared/source/os_interface/linux/page_table_manager_functions.cpp @@ -1,20 +1,22 @@ /* - * Copyright (C) 2019-2022 Intel Corporation + * Copyright (C) 2019-2025 Intel Corporation * * SPDX-License-Identifier: MIT * */ #include "shared/source/gmm_helper/client_context/gmm_client_context.h" -#include "shared/source/gmm_helper/gmm_helper.h" #include "shared/source/gmm_helper/page_table_mngr.h" #include "shared/source/helpers/debug_helpers.h" namespace NEO { -GmmPageTableMngr::GmmPageTableMngr(GmmClientContext *gmmClientContext, unsigned int translationTableFlags, GMM_TRANSLATIONTABLE_CALLBACKS *translationTableCb) : clientContext(gmmClientContext->getHandle()) { - pageTableManager = clientContext->CreatePageTblMgrObject(translationTableFlags); +GmmPageTableMngr::GmmPageTableMngr(GmmClientContext *gmmClientContext, unsigned int translationTableFlags, GMM_TRANSLATIONTABLE_CALLBACKS *translationTableCb, void *aubCsrHandle) { + auto clientContext = gmmClientContext->getHandle(); + auto deleter = [=](GMM_PAGETABLE_MGR *pageTableManager) { + clientContext->DestroyPageTblMgrObject(pageTableManager); + }; + + pageTableManager = {clientContext->CreatePageTblMgrObject(translationTableFlags), deleter}; DEBUG_BREAK_IF(pageTableManager == nullptr); } - -void GmmPageTableMngr::setCsrHandle(void *csrHandle) {} } // namespace NEO diff --git a/shared/source/os_interface/windows/page_table_manager_functions.cpp b/shared/source/os_interface/windows/page_table_manager_functions.cpp index aaf862bde9..d4a46b3f7a 100644 --- a/shared/source/os_interface/windows/page_table_manager_functions.cpp +++ b/shared/source/os_interface/windows/page_table_manager_functions.cpp @@ -1,20 +1,23 @@ /* - * Copyright (C) 2019-2021 Intel Corporation + * Copyright (C) 2019-2025 Intel Corporation * * SPDX-License-Identifier: MIT * */ #include "shared/source/gmm_helper/client_context/gmm_client_context.h" -#include "shared/source/gmm_helper/gmm_helper.h" #include "shared/source/gmm_helper/page_table_mngr.h" namespace NEO { -GmmPageTableMngr::GmmPageTableMngr(GmmClientContext *gmmClientContext, unsigned int translationTableFlags, GMM_TRANSLATIONTABLE_CALLBACKS *translationTableCb) : clientContext(gmmClientContext->getHandle()) { - pageTableManager = clientContext->CreatePageTblMgrObject(translationTableCb, translationTableFlags); -} +GmmPageTableMngr::GmmPageTableMngr(GmmClientContext *gmmClientContext, unsigned int translationTableFlags, GMM_TRANSLATIONTABLE_CALLBACKS *translationTableCb, void *aubCsrHandle) { + auto clientContext = gmmClientContext->getHandle(); + auto deleter = [=](GMM_PAGETABLE_MGR *pageTableManager) { + clientContext->DestroyPageTblMgrObject(pageTableManager); + }; -void GmmPageTableMngr::setCsrHandle(void *csrHandle) { - pageTableManager->GmmSetCsrHandle(csrHandle); + pageTableManager = {clientContext->CreatePageTblMgrObject(translationTableCb, translationTableFlags), deleter}; + if (aubCsrHandle) { + pageTableManager->GmmSetCsrHandle(aubCsrHandle); + } } } // namespace NEO diff --git a/shared/source/os_interface/windows/wddm_device_command_stream.inl b/shared/source/os_interface/windows/wddm_device_command_stream.inl index 4eb298e3f3..a2da4b1c54 100644 --- a/shared/source/os_interface/windows/wddm_device_command_stream.inl +++ b/shared/source/os_interface/windows/wddm_device_command_stream.inl @@ -177,10 +177,9 @@ GmmPageTableMngr *WddmCommandStreamReceiver::createPageTableManager() ttCallbacks.pfWriteL3Adr = writeL3AddressFuncFactory[hwInfo->platform.eRenderCoreFamily]; - GmmPageTableMngr *gmmPageTableMngr = GmmPageTableMngr::create(rootDeviceEnvironment->getGmmClientContext(), TT_TYPE::AUXTT, &ttCallbacks); - if (this->wddm->needsNotifyAubCaptureCallback()) { - gmmPageTableMngr->setCsrHandle(this); - } + void *aubCsrHandle = this->wddm->needsNotifyAubCaptureCallback() ? this : nullptr; + + GmmPageTableMngr *gmmPageTableMngr = GmmPageTableMngr::create(rootDeviceEnvironment->getGmmClientContext(), TT_TYPE::AUXTT, &ttCallbacks, aubCsrHandle); this->pageTableManager.reset(gmmPageTableMngr); return gmmPageTableMngr; } diff --git a/shared/test/common/mocks/mock_gmm_page_table_mngr.cpp b/shared/test/common/mocks/mock_gmm_page_table_mngr.cpp index 4d9a5cc62c..31e6047131 100644 --- a/shared/test/common/mocks/mock_gmm_page_table_mngr.cpp +++ b/shared/test/common/mocks/mock_gmm_page_table_mngr.cpp @@ -1,5 +1,5 @@ /* - * Copyright (C) 2018-2022 Intel Corporation + * Copyright (C) 2018-2025 Intel Corporation * * SPDX-License-Identifier: MIT * @@ -9,12 +9,8 @@ namespace NEO { -GmmPageTableMngr *GmmPageTableMngr::create(GmmClientContext *clientContext, unsigned int translationTableFlags, GMM_TRANSLATIONTABLE_CALLBACKS *translationTableCb) { - auto pageTableMngr = new MockGmmPageTableMngr(translationTableFlags, translationTableCb); +GmmPageTableMngr *GmmPageTableMngr::create(GmmClientContext *clientContext, unsigned int translationTableFlags, GMM_TRANSLATIONTABLE_CALLBACKS *translationTableCb, void *aubCsrHandle) { + auto pageTableMngr = new MockGmmPageTableMngr(translationTableFlags, translationTableCb, aubCsrHandle); return pageTableMngr; } -void MockGmmPageTableMngr::setCsrHandle(void *csrHandle) { - passedCsrHandle = csrHandle; - setCsrHanleCalled++; -} } // namespace NEO diff --git a/shared/test/common/mocks/mock_gmm_page_table_mngr.h b/shared/test/common/mocks/mock_gmm_page_table_mngr.h index 5d906e20e6..3016721e44 100644 --- a/shared/test/common/mocks/mock_gmm_page_table_mngr.h +++ b/shared/test/common/mocks/mock_gmm_page_table_mngr.h @@ -1,5 +1,5 @@ /* - * Copyright (C) 2018-2022 Intel Corporation + * Copyright (C) 2018-2025 Intel Corporation * * SPDX-License-Identifier: MIT * @@ -17,8 +17,8 @@ class MockGmmPageTableMngr : public GmmPageTableMngr { initContextAuxTableRegisterParamsPassed.clear(); }; - MockGmmPageTableMngr(unsigned int translationTableFlags, GMM_TRANSLATIONTABLE_CALLBACKS *translationTableCb) - : translationTableFlags(translationTableFlags) { + MockGmmPageTableMngr(unsigned int translationTableFlags, GMM_TRANSLATIONTABLE_CALLBACKS *translationTableCb, void *aubCsrHandle) + : passedAubCsrHandle(aubCsrHandle), translationTableFlags(translationTableFlags) { if (translationTableCb) { this->translationTableCb = *translationTableCb; } @@ -53,10 +53,7 @@ class MockGmmPageTableMngr : public GmmPageTableMngr { uint32_t updateAuxTableCalled = 0u; GMM_STATUS updateAuxTableResult = GMM_STATUS::GMM_SUCCESS; - void setCsrHandle(void *csrHandle) override; - - uint32_t setCsrHanleCalled = 0; - void *passedCsrHandle = nullptr; + void *passedAubCsrHandle = nullptr; unsigned int translationTableFlags = 0; GMM_TRANSLATIONTABLE_CALLBACKS translationTableCb = {}; diff --git a/shared/test/unit_test/os_interface/windows/device_command_stream_tests.cpp b/shared/test/unit_test/os_interface/windows/device_command_stream_tests.cpp index f11381895c..6b8cb00979 100644 --- a/shared/test/unit_test/os_interface/windows/device_command_stream_tests.cpp +++ b/shared/test/unit_test/os_interface/windows/device_command_stream_tests.cpp @@ -893,10 +893,9 @@ HWTEST_P(WddmCsrCompressionParameterizedTest, givenEnabledCompressionWhenInitial auto mockMngr = reinterpret_cast(mockWddmCsr.pageTableManager.get()); if (aubCaptureMode) { - EXPECT_EQ(1u, mockMngr->setCsrHanleCalled); - EXPECT_EQ(&mockWddmCsr, mockMngr->passedCsrHandle); + EXPECT_EQ(&mockWddmCsr, mockMngr->passedAubCsrHandle); } else { - EXPECT_EQ(0u, mockMngr->setCsrHanleCalled); + EXPECT_EQ(nullptr, mockMngr->passedAubCsrHandle); } GMM_TRANSLATIONTABLE_CALLBACKS expectedTTCallbacks = {}; diff --git a/shared/test/unit_test/os_interface/windows/wddm_memory_manager_tests.cpp b/shared/test/unit_test/os_interface/windows/wddm_memory_manager_tests.cpp index b8506545bc..ae8b560e43 100644 --- a/shared/test/unit_test/os_interface/windows/wddm_memory_manager_tests.cpp +++ b/shared/test/unit_test/os_interface/windows/wddm_memory_manager_tests.cpp @@ -2878,7 +2878,7 @@ class WddmMemoryManagerTest : public ::Test { for (auto engine : memoryManager->getRegisteredEngines(rootDeviceIndex)) { if (engine.getEngineUsage() == EngineUsage::regular) { - engine.commandStreamReceiver->pageTableManager.reset(GmmPageTableMngr::create(nullptr, 0, &dummyTTCallbacks)); + engine.commandStreamReceiver->pageTableManager.reset(GmmPageTableMngr::create(nullptr, 0, &dummyTTCallbacks, nullptr)); } } }