Pass Drm to ioctl helper ctor

JablonskiMateusz · Compute-Runtime-Automation · commit c9e2b4bc3292 · 2022-06-29T15:47:01.000+02:00
Related-To: NEO-6999
Signed-off-by: Mateusz Jablonski &lt;mateusz.jablonski@intel.com&gt;
diff --git a/level_zero/tools/test/unit_tests/sources/metrics/linux/test_metric_ip_sampling_linux_pvc_prelim.cpp b/level_zero/tools/test/unit_tests/sources/metrics/linux/test_metric_ip_sampling_linux_pvc_prelim.cpp
@@ -28,7 +28,8 @@ namespace L0 {
 namespace ult {
 
 class IoctlHelperPrelim20Mock : public NEO::IoctlHelperPrelim20 {
-
+  public:
+    using NEO::IoctlHelperPrelim20::IoctlHelperPrelim20;
     bool getEuStallProperties(std::array<uint64_t, 12u> &properties, uint64_t dssBufferSize, uint64_t samplingRate,
                               uint64_t pollPeriod, uint64_t engineInstance, uint64_t notifyNReports) override {
         return false;
@@ -43,7 +44,7 @@ class DrmPrelimMock : public DrmMock {
                                                            &inputHwInfo->workaroundTable, &inputHwInfo->gtSystemInfo, inputHwInfo->capabilityTable);
         customHwInfo->gtSystemInfo.MaxDualSubSlicesSupported = 64;
         rootDeviceEnvironment.setHwInfo(customHwInfo.get());
-        this->ioctlHelper = std::make_unique<IoctlHelperPrelim20>();
+        this->ioctlHelper = std::make_unique<IoctlHelperPrelim20>(*this);
         if (invokeQueryEngineInfo) {
             queryEngineInfo(); // NOLINT(clang-analyzer-optin.cplusplus.VirtualCall)
         }
@@ -101,7 +102,7 @@ class DrmPrelimMock : public DrmMock {
 
     void setIoctlHelperPrelim20Mock() {
         backUpIoctlHelper = std::move(ioctlHelper);
-        ioctlHelper = static_cast<std::unique_ptr<NEO::IoctlHelper>>(std::make_unique<IoctlHelperPrelim20Mock>());
+        ioctlHelper = static_cast<std::unique_ptr<NEO::IoctlHelper>>(std::make_unique<IoctlHelperPrelim20Mock>(*this));
     }
 
     void restoreIoctlHelperPrelim20() {
diff --git a/shared/source/os_interface/linux/drm_neo.cpp b/shared/source/os_interface/linux/drm_neo.cpp
@@ -989,7 +989,7 @@ void Drm::setupIoctlHelper(const PRODUCT_FAMILY productFamily) {
         std::string prelimVersion = "";
         getPrelimVersion(prelimVersion);
         auto drmVersion = Drm::getDrmVersion(getFileDescriptor());
-        this->ioctlHelper.reset(IoctlHelper::get(productFamily, prelimVersion, drmVersion));
+        this->ioctlHelper = IoctlHelper::get(productFamily, prelimVersion, drmVersion, *this);
     }
 }
 
diff --git a/shared/source/os_interface/linux/ioctl_helper.h b/shared/source/os_interface/linux/ioctl_helper.h
@@ -25,8 +25,6 @@ class IoctlHelper;
 enum class CacheRegion : uint16_t;
 struct HardwareInfo;
 
-extern IoctlHelper *ioctlHelperFactory[IGFX_MAX_PRODUCT];
-
 struct MemoryRegion {
     MemoryClassInstance region;
     uint64_t probedSize;
@@ -65,10 +63,10 @@ using VmBindExtUserFenceT = uint8_t[56];
 
 class IoctlHelper {
   public:
+    IoctlHelper(Drm &drmArg) : drm(drmArg){};
     virtual ~IoctlHelper() {}
-    static IoctlHelper *get(const PRODUCT_FAMILY productFamily, const std::string &prelimVersion, const std::string &drmVersion);
+    static std::unique_ptr<IoctlHelper> get(const PRODUCT_FAMILY productFamily, const std::string &prelimVersion, const std::string &drmVersion, Drm &drm);
     uint32_t ioctl(Drm *drm, DrmIoctl request, void *arg);
-    virtual IoctlHelper *clone() = 0;
 
     virtual bool isVmBindAvailable(Drm *drm) = 0;
     virtual uint32_t createGemExt(Drm *drm, const MemRegionsVec &memClassInstances, size_t allocSize, uint32_t &handle, std::optional<uint32_t> vmId) = 0;
@@ -124,11 +122,14 @@ class IoctlHelper {
     void logExecBuffer(const ExecBuffer &execBuffer, std::stringstream &logger);
     int getDrmParamValueBase(DrmParam drmParam) const;
     unsigned int getIoctlRequestValueBase(DrmIoctl ioctlRequest) const;
+
+  protected:
+    Drm &drm;
 };
 
 class IoctlHelperUpstream : public IoctlHelper {
   public:
-    IoctlHelper *clone() override;
+    using IoctlHelper::IoctlHelper;
 
     bool isVmBindAvailable(Drm *drm) override;
     uint32_t createGemExt(Drm *drm, const MemRegionsVec &memClassInstances, size_t allocSize, uint32_t &handle, std::optional<uint32_t> vmId) override;
@@ -176,11 +177,10 @@ class IoctlHelperUpstream : public IoctlHelper {
 template <PRODUCT_FAMILY gfxProduct>
 class IoctlHelperImpl : public IoctlHelperUpstream {
   public:
-    static IoctlHelper *get() {
-        static IoctlHelperImpl<gfxProduct> instance;
-        return &instance;
+    using IoctlHelperUpstream::IoctlHelperUpstream;
+    static std::unique_ptr<IoctlHelper> get(Drm &drm) {
+        return std::make_unique<IoctlHelperImpl<gfxProduct>>(drm);
     }
-    IoctlHelper *clone() override;
 
     uint32_t createGemExt(Drm *drm, const MemRegionsVec &memClassInstances, size_t allocSize, uint32_t &handle, std::optional<uint32_t> vmId) override;
     std::vector<MemoryRegion> translateToMemoryRegions(const std::vector<uint8_t> &regionInfo) override;
@@ -189,7 +189,7 @@ class IoctlHelperImpl : public IoctlHelperUpstream {
 
 class IoctlHelperPrelim20 : public IoctlHelper {
   public:
-    IoctlHelper *clone() override;
+    using IoctlHelper::IoctlHelper;
 
     bool isVmBindAvailable(Drm *drm) override;
     uint32_t createGemExt(Drm *drm, const MemRegionsVec &memClassInstances, size_t allocSize, uint32_t &handle, std::optional<uint32_t> vmId) override;
diff --git a/shared/source/os_interface/linux/ioctl_helper_getter.cpp b/shared/source/os_interface/linux/ioctl_helper_getter.cpp
@@ -10,25 +10,22 @@
 #include "shared/source/os_interface/linux/drm_neo.h"
 #include "shared/source/os_interface/linux/ioctl_helper.h"
 
+#include <functional>
 #include <string>
 
 namespace NEO {
 
-IoctlHelper *ioctlHelperFactory[IGFX_MAX_PRODUCT] = {};
+std::optional<std::function<std::unique_ptr<IoctlHelper>(Drm &drm)>> ioctlHelperFactory[IGFX_MAX_PRODUCT] = {};
 
-std::map<std::string, std::shared_ptr<IoctlHelper>> ioctlHelperImpls{
-    {"", std::make_shared<IoctlHelperUpstream>()},
-    {"2.0", std::make_shared<IoctlHelperPrelim20>()}};
-
-IoctlHelper *IoctlHelper::get(const PRODUCT_FAMILY productFamily, const std::string &prelimVersion, const std::string &drmVersion) {
-    auto productSpecificIoctlHelper = ioctlHelperFactory[productFamily];
-    if (productSpecificIoctlHelper) {
-        return productSpecificIoctlHelper->clone();
+std::unique_ptr<IoctlHelper> IoctlHelper::get(const PRODUCT_FAMILY productFamily, const std::string &prelimVersion, const std::string &drmVersion, Drm &drm) {
+    auto productSpecificIoctlHelperCreator = ioctlHelperFactory[productFamily];
+    if (productSpecificIoctlHelperCreator) {
+        return productSpecificIoctlHelperCreator.value()(drm);
     }
-    if (ioctlHelperImpls.find(prelimVersion) != ioctlHelperImpls.end()) {
-        return ioctlHelperImpls[prelimVersion]->clone();
+    if (prelimVersion == "") {
+        return std::make_unique<IoctlHelperUpstream>(drm);
     }
-    return new IoctlHelperPrelim20{}; //fallback to 2.0 if no proper implementation has been found
+    return std::make_unique<IoctlHelperPrelim20>(drm);
 }
 
 } // namespace NEO
diff --git a/shared/source/os_interface/linux/ioctl_helper_getter_upstream.cpp b/shared/source/os_interface/linux/ioctl_helper_getter_upstream.cpp
@@ -8,17 +8,18 @@
 #include "shared/source/helpers/hw_info.h"
 #include "shared/source/os_interface/linux/ioctl_helper.h"
 
+#include <functional>
 #include <memory>
 
 namespace NEO {
-IoctlHelper *ioctlHelperFactory[IGFX_MAX_PRODUCT] = {};
+std::optional<std::function<std::unique_ptr<IoctlHelper>(Drm &drm)>> ioctlHelperFactory[IGFX_MAX_PRODUCT] = {};
 
-IoctlHelper *IoctlHelper::get(const PRODUCT_FAMILY productFamily, const std::string &prelimVersion, const std::string &drmVersion) {
-    auto productSpecificIoctlHelper = ioctlHelperFactory[productFamily];
-    if (productSpecificIoctlHelper) {
-        return productSpecificIoctlHelper->clone();
+std::unique_ptr<IoctlHelper> IoctlHelper::get(const PRODUCT_FAMILY productFamily, const std::string &prelimVersion, const std::string &drmVersion, Drm &drm) {
+    auto productSpecificIoctlHelperCreator = ioctlHelperFactory[productFamily];
+    if (productSpecificIoctlHelperCreator) {
+        return productSpecificIoctlHelperCreator.value()(drm);
     }
-    return new IoctlHelperUpstream{};
+    return std::make_unique<IoctlHelperUpstream>(drm);
 }
 
 } // namespace NEO
diff --git a/shared/source/os_interface/linux/ioctl_helper_prelim.cpp b/shared/source/os_interface/linux/ioctl_helper_prelim.cpp
@@ -24,10 +24,6 @@
 
 namespace NEO {
 
-IoctlHelper *IoctlHelperPrelim20::clone() {
-    return new IoctlHelperPrelim20{};
-}
-
 bool IoctlHelperPrelim20::isVmBindAvailable(Drm *drm) {
     int vmBindSupported = 0;
     GetParam getParam{};
diff --git a/shared/source/os_interface/linux/ioctl_helper_upstream.cpp b/shared/source/os_interface/linux/ioctl_helper_upstream.cpp
@@ -15,10 +15,6 @@
 
 namespace NEO {
 
-IoctlHelper *IoctlHelperUpstream::clone() {
-    return new IoctlHelperUpstream{};
-}
-
 bool IoctlHelperUpstream::isVmBindAvailable(Drm *drm) {
     return false;
 }
diff --git a/shared/source/os_interface/linux/local/dg1/enable_ioctl_helper_dg1.cpp b/shared/source/os_interface/linux/local/dg1/enable_ioctl_helper_dg1.cpp
@@ -1,19 +1,20 @@
 /*
- * Copyright (C) 2021 Intel Corporation
+ * Copyright (C) 2021-2022 Intel Corporation
  *
  * SPDX-License-Identifier: MIT
  *
  */
 
 #include "shared/source/os_interface/linux/ioctl_helper.h"
 
+#include <functional>
+
 namespace NEO {
-extern IoctlHelper *ioctlHelperFactory[IGFX_MAX_PRODUCT];
+extern std::optional<std::function<std::unique_ptr<IoctlHelper>(Drm &drm)>> ioctlHelperFactory[IGFX_MAX_PRODUCT];
 
 struct EnableProductIoctlHelperDg1 {
     EnableProductIoctlHelperDg1() {
-        IoctlHelper *pIoctlHelper = IoctlHelperImpl<IGFX_DG1>::get();
-        ioctlHelperFactory[IGFX_DG1] = pIoctlHelper;
+        ioctlHelperFactory[IGFX_DG1] = IoctlHelperImpl<IGFX_DG1>::get;
     }
 };
 
diff --git a/shared/source/os_interface/linux/local/dg1/ioctl_helper_dg1.cpp b/shared/source/os_interface/linux/local/dg1/ioctl_helper_dg1.cpp
@@ -17,11 +17,6 @@ constexpr static auto gfxProduct = IGFX_DG1;
 extern bool isQueryDrmTip(const std::vector<uint8_t> &queryInfo);
 extern std::vector<uint8_t> translateToDrmTip(const uint8_t *dataQuery);
 
-template <>
-IoctlHelper *IoctlHelperImpl<gfxProduct>::clone() {
-    return new IoctlHelperImpl<gfxProduct>{};
-}
-
 template <>
 uint32_t IoctlHelperImpl<gfxProduct>::createGemExt(Drm *drm, const MemRegionsVec &memClassInstances, size_t allocSize, uint32_t &handle, std::optional<uint32_t> vmId) {
     auto ret = IoctlHelperUpstream::createGemExt(drm, memClassInstances, allocSize, handle, vmId);
diff --git a/shared/test/common/libult/linux/drm_query_mock.h b/shared/test/common/libult/linux/drm_query_mock.h
@@ -25,7 +25,7 @@ class DrmQueryMock : public DrmMock {
         context.hwInfo = rootDeviceEnvironment.getHardwareInfo();
         callBaseIsVmBindAvailable = true;
 
-        this->ioctlHelper = std::make_unique<IoctlHelperPrelim20>();
+        this->ioctlHelper = std::make_unique<IoctlHelperPrelim20>(*this);
 
         EXPECT_TRUE(queryMemoryInfo());
         EXPECT_EQ(2u + virtualMemoryIds.size(), ioctlCallsCount);
diff --git a/shared/test/common/os_interface/linux/device_command_stream_fixture_prelim.h b/shared/test/common/os_interface/linux/device_command_stream_fixture_prelim.h
@@ -17,7 +17,7 @@ class DrmMockCustomPrelim : public DrmMockCustom {
     using Drm::memoryInfo;
 
     DrmMockCustomPrelim(RootDeviceEnvironment &rootDeviceEnvironment) : DrmMockCustom(rootDeviceEnvironment) {
-        this->ioctlHelper = std::make_unique<IoctlHelperPrelim20>();
+        this->ioctlHelper = std::make_unique<IoctlHelperPrelim20>(*this);
     }
 
     int ioctlExtra(DrmIoctl request, void *arg) override {
diff --git a/shared/test/unit_test/os_interface/linux/drm_tests.cpp b/shared/test/unit_test/os_interface/linux/drm_tests.cpp
@@ -1077,8 +1077,12 @@ TEST(DrmQueryTest, givenUapiPrelimVersionWithInvalidPathThenReturnEmptyString) {
 }
 
 TEST(DrmTest, givenInvalidUapiPrelimVersionThenFallbackToBasePrelim) {
+    auto executionEnvironment = std::make_unique<ExecutionEnvironment>();
+    executionEnvironment->prepareRootDeviceEnvironments(1);
+    DrmMock drm{*executionEnvironment->rootDeviceEnvironments[0]};
+
     const auto productFamily = defaultHwInfo->platform.eProductFamily;
-    std::unique_ptr<IoctlHelper> ioctlHelper(IoctlHelper::get(productFamily, "-1", "unk"));
+    std::unique_ptr<IoctlHelper> ioctlHelper(IoctlHelper::get(productFamily, "-1", "unk", drm));
     EXPECT_NE(nullptr, ioctlHelper.get());
 }
 
@@ -1227,6 +1231,7 @@ TEST(DrmWrapperTest, WhenGettingRevisionParamValueThenIoctlHelperIsNotNeeded) {
 
 class MockIoctlHelper : public IoctlHelperPrelim20 {
   public:
+    using IoctlHelperPrelim20::IoctlHelperPrelim20;
     unsigned int getIoctlRequestValue(DrmIoctl ioctlRequest) const override {
         return ioctlRequestValue;
     };
@@ -1240,7 +1245,11 @@ class MockIoctlHelper : public IoctlHelperPrelim20 {
 };
 
 TEST(DrmWrapperTest, whenGettingDrmParamOrIoctlRequestValueThenUseIoctlHelperWhenAvailable) {
-    MockIoctlHelper ioctlHelper{};
+    auto executionEnvironment = std::make_unique<ExecutionEnvironment>();
+    executionEnvironment->prepareRootDeviceEnvironments(1);
+    DrmMock drm{*executionEnvironment->rootDeviceEnvironments[0]};
+
+    MockIoctlHelper ioctlHelper{drm};
     EXPECT_EQ(getIoctlRequestValue(DrmIoctl::Getparam, &ioctlHelper), ioctlHelper.ioctlRequestValue);
     EXPECT_NE(getIoctlRequestValue(DrmIoctl::Getparam, nullptr), getIoctlRequestValue(DrmIoctl::Getparam, &ioctlHelper));
 
diff --git a/shared/test/unit_test/os_interface/linux/drm_with_prelim_tests.cpp b/shared/test/unit_test/os_interface/linux/drm_with_prelim_tests.cpp
@@ -46,7 +46,7 @@ class IoctlHelperPrelimFixture : public ::testing::Test {
         executionEnvironment = std::make_unique<ExecutionEnvironment>();
         executionEnvironment->prepareRootDeviceEnvironments(1);
         drm = std::make_unique<DrmPrelimMock>(*executionEnvironment->rootDeviceEnvironments[0]);
-        drm->ioctlHelper = std::make_unique<IoctlHelperPrelim20>();
+        drm->ioctlHelper = std::make_unique<IoctlHelperPrelim20>(*drm);
     }
 
     std::unique_ptr<ExecutionEnvironment> executionEnvironment;
@@ -57,7 +57,7 @@ TEST(IoctlHelperPrelimTest, whenGettingVmBindAvailabilityThenProperValueIsReturn
     auto executionEnvironment = std::make_unique<MockExecutionEnvironment>();
     DrmQueryMock drm{*executionEnvironment->rootDeviceEnvironments[0]};
 
-    IoctlHelperPrelim20 ioctlHelper{};
+    IoctlHelperPrelim20 ioctlHelper{drm};
 
     for (auto &ioctlValue : {0, EINVAL}) {
         drm.context.vmBindQueryReturn = ioctlValue;
@@ -79,7 +79,7 @@ TEST(IoctlHelperPrelimTest, whenVmBindIsCalledThenProperValueIsReturnedBasedOnIo
     auto executionEnvironment = std::make_unique<MockExecutionEnvironment>();
     DrmQueryMock drm{*executionEnvironment->rootDeviceEnvironments[0]};
 
-    IoctlHelperPrelim20 ioctlHelper{};
+    IoctlHelperPrelim20 ioctlHelper{drm};
 
     VmBindParams vmBindParams{};
 
@@ -95,7 +95,7 @@ TEST(IoctlHelperPrelimTest, whenVmUnbindIsCalledThenProperValueIsReturnedBasedOn
     auto executionEnvironment = std::make_unique<MockExecutionEnvironment>();
     DrmQueryMock drm{*executionEnvironment->rootDeviceEnvironments[0]};
 
-    IoctlHelperPrelim20 ioctlHelper{};
+    IoctlHelperPrelim20 ioctlHelper{drm};
 
     VmBindParams vmBindParams{};
 
diff --git a/shared/test/unit_test/os_interface/linux/ioctl_helper_tests_prelim.cpp b/shared/test/unit_test/os_interface/linux/ioctl_helper_tests_prelim.cpp
@@ -6,8 +6,10 @@
  */
 
 #include "shared/source/helpers/string.h"
+#include "shared/source/os_interface/linux/drm_neo.h"
 #include "shared/source/os_interface/linux/ioctl_helper.h"
 #include "shared/test/common/helpers/debug_manager_state_restore.h"
+#include "shared/test/common/mocks/mock_execution_environment.h"
 #include "shared/test/common/test_macros/test.h"
 
 #include "third_party/uapi/prelim/drm/i915_drm.h"
@@ -18,7 +20,9 @@ extern std::vector<uint8_t> getRegionInfo(const std::vector<MemoryRegion> &input
 extern std::vector<uint8_t> getEngineInfo(const std::vector<EngineCapabilities> &inputEngines);
 
 struct IoctlPrelimHelperTests : ::testing::Test {
-    IoctlHelperPrelim20 ioctlHelper{};
+    MockExecutionEnvironment executionEnvironment{};
+    std::unique_ptr<Drm> drm{Drm::create(std::make_unique<HwDeviceIdDrm>(0, ""), *executionEnvironment.rootDeviceEnvironments[0])};
+    IoctlHelperPrelim20 ioctlHelper{*drm};
 };
 
 TEST_F(IoctlPrelimHelperTests, whenGettingIoctlRequestValueThenPropertValueIsReturned) {
@@ -113,11 +117,6 @@ TEST_F(IoctlPrelimHelperTests, givenPrelimsWhenTranslateToMemoryRegionsThenRetur
     }
 }
 
-TEST_F(IoctlPrelimHelperTests, whenCloneIsCalledThenValidPtrIsReturned) {
-    std::unique_ptr<IoctlHelper> cloned(ioctlHelper.clone());
-    EXPECT_NE(nullptr, cloned);
-}
-
 TEST_F(IoctlPrelimHelperTests, givenEmptyRegionInstanceClassWhenCreatingVmControlRegionExtThenNullptrIsReturned) {
     std::optional<MemoryClassInstance> regionInstanceClass{};
 
diff --git a/shared/test/unit_test/os_interface/linux/ioctl_helper_tests_upstream.cpp b/shared/test/unit_test/os_interface/linux/ioctl_helper_tests_upstream.cpp

Original file line number	Diff line number	Diff line change
`@@ -989,7 +989,7 @@ void Drm::setupIoctlHelper(const PRODUCT_FAMILY productFamily) {`
`989`	`989`	`std::string prelimVersion = "";`
`990`	`990`	`getPrelimVersion(prelimVersion);`
`991`	`991`	`auto drmVersion = Drm::getDrmVersion(getFileDescriptor());`
`992`		`- this->ioctlHelper.reset(IoctlHelper::get(productFamily, prelimVersion, drmVersion));`
	`992`	`+ this->ioctlHelper = IoctlHelper::get(productFamily, prelimVersion, drmVersion, *this);`
`993`	`993`	`}`
`994`	`994`	`}`
`995`	`995`
Original file line number	Diff line number	Diff line change
`@@ -15,10 +15,6 @@`
`15`	`15`
`16`	`16`	`namespace NEO {`
`17`	`17`
`18`		`-IoctlHelper *IoctlHelperUpstream::clone() {`
`19`		`- return new IoctlHelperUpstream{};`
`20`		`-}`
`21`		`-`
`22`	`18`	`bool IoctlHelperUpstream::isVmBindAvailable(Drm *drm) {`
`23`	`19`	`return false;`
`24`	`20`	`}`