Extract HwDeviceId from Drm

JablonskiMateusz · Compute-Runtime-Automation · commit 018e585eb1da · 2020-02-07T12:11:40.000+01:00
Related-To: NEO-4208
Change-Id: I1678ad92cab2a369769b93da69dc46a1d515f261
Signed-off-by: Jablonski, Mateusz &lt;mateusz.jablonski@intel.com&gt;
diff --git a/core/os_interface/linux/CMakeLists.txt b/core/os_interface/linux/CMakeLists.txt
@@ -23,6 +23,8 @@ set(NEO_CORE_OS_INTERFACE_LINUX
   ${CMAKE_CURRENT_SOURCE_DIR}/drm_memory_operations_handler.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/drm_memory_operations_handler.h
   ${CMAKE_CURRENT_SOURCE_DIR}/hw_info_config.cpp
+  ${CMAKE_CURRENT_SOURCE_DIR}/hw_device_id.h
+  ${CMAKE_CURRENT_SOURCE_DIR}/hw_device_id_linux.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/linux_inc.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}${BRANCH_DIR_SUFFIX}/drm_query.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/engine_info.h
@@ -43,6 +45,7 @@ set(NEO_CORE_OS_INTERFACE_LINUX
   ${CMAKE_CURRENT_SOURCE_DIR}/os_time_linux.h
   ${CMAKE_CURRENT_SOURCE_DIR}/page_table_manager_functions.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/print.cpp
+  ${CMAKE_CURRENT_SOURCE_DIR}/sys_calls.h
 )
 
 set_property(GLOBAL PROPERTY NEO_CORE_OS_INTERFACE_LINUX ${NEO_CORE_OS_INTERFACE_LINUX})
diff --git a/core/os_interface/linux/drm_neo.cpp b/core/os_interface/linux/drm_neo.cpp
@@ -56,7 +56,7 @@ int Drm::ioctl(unsigned long request, void *arg) {
     int ret;
     SYSTEM_ENTER();
     do {
-        ret = ::ioctl(fd, request, arg);
+        ret = ::ioctl(getFileDescriptor(), request, arg);
     } while (ret == -1 && (errno == EINTR || errno == EAGAIN));
     SYSTEM_LEAVE(request);
     return ret;
diff --git a/core/os_interface/linux/drm_neo.h b/core/os_interface/linux/drm_neo.h
@@ -8,6 +8,7 @@
 #pragma once
 #include "core/helpers/basic_math.h"
 #include "core/os_interface/linux/engine_info.h"
+#include "core/os_interface/linux/hw_device_id.h"
 #include "core/os_interface/linux/memory_info.h"
 #include "core/utilities/api_intercept.h"
 
@@ -65,7 +66,7 @@ class Drm {
     bool isPreemptionSupported() const { return preemptionSupported; }
 
     MOCKABLE_VIRTUAL void checkPreemptionSupport();
-    int getFileDescriptor() const { return fd; }
+    inline int getFileDescriptor() const { return hwDeviceId->getFileDescriptor(); }
     uint32_t createDrmContext();
     void destroyDrmContext(uint32_t drmContextId);
     void setLowPriorityContextParam(uint32_t drmContextId);
@@ -99,12 +100,12 @@ class Drm {
     drm_i915_gem_context_param_sseu sseu{};
     bool preemptionSupported = false;
     bool nonPersistentContextsSupported = false;
-    int fd;
+    std::unique_ptr<HwDeviceId> hwDeviceId;
     int deviceId = 0;
     int revisionId = 0;
     GTTYPE eGtType = GTTYPE_UNDEFINED;
     RootDeviceEnvironment &rootDeviceEnvironment;
-    Drm(int fd, RootDeviceEnvironment &rootDeviceEnvironment) : fd(fd), rootDeviceEnvironment(rootDeviceEnvironment) {}
+    Drm(std::unique_ptr<HwDeviceId> hwDeviceIdIn, RootDeviceEnvironment &rootDeviceEnvironment) : hwDeviceId(std::move(hwDeviceIdIn)), rootDeviceEnvironment(rootDeviceEnvironment) {}
     std::unique_ptr<EngineInfo> engineInfo;
     std::unique_ptr<MemoryInfo> memoryInfo;
 
diff --git a/core/os_interface/linux/drm_null_device.h b/core/os_interface/linux/drm_null_device.h
@@ -38,7 +38,7 @@ class DrmNullDevice : public Drm {
         }
     }
 
-    DrmNullDevice(int fd, RootDeviceEnvironment &rootDeviceEnvironment) : Drm(fd, rootDeviceEnvironment), gpuTimestamp(0){};
+    DrmNullDevice(std::unique_ptr<HwDeviceId> hwDeviceId, RootDeviceEnvironment &rootDeviceEnvironment) : Drm(std::move(hwDeviceId), rootDeviceEnvironment), gpuTimestamp(0){};
 
   protected:
     uint64_t gpuTimestamp;
diff --git a/core/os_interface/linux/hw_device_id.h b/core/os_interface/linux/hw_device_id.h
@@ -0,0 +1,22 @@
+/*
+ * Copyright (C) 2020 Intel Corporation
+ *
+ * SPDX-License-Identifier: MIT
+ *
+ */
+
+#pragma once
+#include "core/helpers/non_copyable_or_moveable.h"
+
+namespace NEO {
+
+class HwDeviceId : NonCopyableClass {
+  public:
+    HwDeviceId(int fileDescriptorIn) : fileDescriptor(fileDescriptorIn) {}
+    ~HwDeviceId();
+    constexpr int getFileDescriptor() const { return fileDescriptor; }
+
+  protected:
+    const int fileDescriptor;
+};
+} // namespace NEO
diff --git a/core/os_interface/linux/hw_device_id_linux.cpp b/core/os_interface/linux/hw_device_id_linux.cpp
@@ -0,0 +1,17 @@
+/*
+ * Copyright (C) 2020 Intel Corporation
+ *
+ * SPDX-License-Identifier: MIT
+ *
+ */
+
+#include "core/os_interface/linux/hw_device_id.h"
+#include "core/os_interface/linux/sys_calls.h"
+
+namespace NEO {
+
+HwDeviceId::~HwDeviceId() {
+    SysCalls::close(fileDescriptor);
+}
+
+} // namespace NEO
diff --git a/core/os_interface/linux/sys_calls.h b/core/os_interface/linux/sys_calls.h
@@ -0,0 +1,14 @@
+/*
+ * Copyright (C) 2020 Intel Corporation
+ *
+ * SPDX-License-Identifier: MIT
+ *
+ */
+
+#pragma once
+
+namespace NEO {
+namespace SysCalls {
+int close(int fileDescriptor);
+}
+} // namespace NEO
diff --git a/core/os_interface/linux/sys_calls_linux.cpp b/core/os_interface/linux/sys_calls_linux.cpp
@@ -0,0 +1,18 @@
+/*
+ * Copyright (C) 2020 Intel Corporation
+ *
+ * SPDX-License-Identifier: MIT
+ *
+ */
+
+#include "core/os_interface/linux/sys_calls.h"
+
+#include <unistd.h>
+
+namespace NEO {
+namespace SysCalls {
+int close(int fileDescriptor) {
+    return ::close(fileDescriptor);
+}
+} // namespace SysCalls
+} // namespace NEO
diff --git a/runtime/dll/CMakeLists.txt b/runtime/dll/CMakeLists.txt
@@ -47,6 +47,7 @@ set(RUNTIME_SRCS_DLL_LINUX
   ${CMAKE_CURRENT_SOURCE_DIR}/linux/drm_neo_create.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/linux/options_linux.cpp
   ${CMAKE_CURRENT_SOURCE_DIR}/linux/os_interface.cpp
+  ${NEO_SOURCE_DIR}/core/os_interface/linux/sys_calls_linux.cpp
 )
 
 set(RUNTIME_SRCS_DLL_WINDOWS
diff --git a/runtime/dll/linux/drm_neo_create.cpp b/runtime/dll/linux/drm_neo_create.cpp
@@ -34,10 +34,7 @@ const DeviceDescriptor deviceDescriptorTable[] = {
 
 static std::array<Drm *, 1> drms = {{nullptr}};
 
-Drm::~Drm() {
-    close(fd);
-    fd = -1;
-}
+Drm::~Drm() = default;
 
 Drm *Drm::get(int32_t deviceOrdinal) {
     if (static_cast<uint32_t>(deviceOrdinal) >= drms.size())
@@ -138,9 +135,9 @@ Drm *Drm::create(int32_t deviceOrdinal, RootDeviceEnvironment &rootDeviceEnviron
 
     std::unique_ptr<Drm> drmObject;
     if (DebugManager.flags.EnableNullHardware.get() == true) {
-        drmObject.reset(new DrmNullDevice(fd, rootDeviceEnvironment));
+        drmObject.reset(new DrmNullDevice(std::make_unique<HwDeviceId>(fd), rootDeviceEnvironment));
     } else {
-        drmObject.reset(new Drm(fd, rootDeviceEnvironment));
+        drmObject.reset(new Drm(std::make_unique<HwDeviceId>(fd), rootDeviceEnvironment));
     }
 
     // Get HW version (I915_drm.h)
diff --git a/unit_tests/libult/CMakeLists.txt b/unit_tests/libult/CMakeLists.txt
@@ -111,9 +111,10 @@ set(IGDRCL_SRCS_LIB_ULT_ENV_WINDOWS
 )
 
 set(IGDRCL_SRCS_LIB_ULT_ENV_LINUX
+  ${NEO_SOURCE_DIR}/unit_tests/os_interface/linux/allocator_helper.cpp
   ${NEO_SOURCE_DIR}/unit_tests/os_interface/linux/create_drm_memory_manager.cpp
   ${NEO_SOURCE_DIR}/unit_tests/os_interface/linux/options.cpp
-  ${NEO_SOURCE_DIR}/unit_tests/os_interface/linux/allocator_helper.cpp
+  ${NEO_SOURCE_DIR}/unit_tests/os_interface/linux/sys_calls_linux_ult.cpp
 )
 
 if(WIN32)
diff --git a/unit_tests/linux/CMakeLists.txt b/unit_tests/linux/CMakeLists.txt
@@ -38,6 +38,7 @@ endif(NEO__LIBVA_FOUND)
 
 foreach(target_name linux_tests linux_dll_tests)
   add_executable(igdrcl_${target_name}
+    ${NEO_SOURCE_DIR}/core/os_interface/linux/sys_calls_linux.cpp
     ${IGDRCL_SRCS_${target_name}}
     $<TARGET_OBJECTS:igdrcl_libult>
     $<TARGET_OBJECTS:igdrcl_libult_cs>
diff --git a/unit_tests/mt_tests/os_interface/linux/drm_memory_manager_mt_tests.cpp b/unit_tests/mt_tests/os_interface/linux/drm_memory_manager_mt_tests.cpp
@@ -24,7 +24,7 @@ using namespace std;
 TEST(DrmMemoryManagerTest, givenDrmMemoryManagerWhenSharedAllocationIsCreatedFromMultipleThreadsThenSingleBoIsReused) {
     class MockDrm : public Drm {
       public:
-        MockDrm(int fd, RootDeviceEnvironment &rootDeviceEnvironment) : Drm(fd, rootDeviceEnvironment) {}
+        MockDrm(int fd, RootDeviceEnvironment &rootDeviceEnvironment) : Drm(std::make_unique<HwDeviceId>(fd), rootDeviceEnvironment) {}
 
         int ioctl(unsigned long request, void *arg) override {
             if (request == DRM_IOCTL_PRIME_FD_TO_HANDLE) {
@@ -71,7 +71,7 @@ TEST(DrmMemoryManagerTest, givenDrmMemoryManagerWhenSharedAllocationIsCreatedFro
 TEST(DrmMemoryManagerTest, givenMultipleThreadsWhenSharedAllocationIsCreatedThenPrimeFdToHandleDoesNotRaceWithClose) {
     class MockDrm : public Drm {
       public:
-        MockDrm(int fd, RootDeviceEnvironment &rootDeviceEnvironment) : Drm(fd, rootDeviceEnvironment) {
+        MockDrm(int fd, RootDeviceEnvironment &rootDeviceEnvironment) : Drm(std::make_unique<HwDeviceId>(fd), rootDeviceEnvironment) {
             primeFdHandle = 1;
             closeHandle = 1;
         }
diff --git a/unit_tests/os_interface/linux/device_command_stream_fixture.h b/unit_tests/os_interface/linux/device_command_stream_fixture.h
@@ -27,27 +27,28 @@
 
 using NEO::constructPlatform;
 using NEO::Drm;
+using NEO::HwDeviceId;
 using NEO::RootDeviceEnvironment;
 
 static const int mockFd = 33;
 
 class DrmMockImpl : public Drm {
   public:
-    DrmMockImpl(int fd) : Drm(fd, *constructPlatform()->peekExecutionEnvironment()->rootDeviceEnvironments[0]){};
+    DrmMockImpl(int fd) : Drm(std::make_unique<HwDeviceId>(fd), *constructPlatform()->peekExecutionEnvironment()->rootDeviceEnvironments[0]){};
     MOCK_METHOD2(ioctl, int(unsigned long request, void *arg));
 };
 
 class DrmMockSuccess : public Drm {
   public:
     DrmMockSuccess() : DrmMockSuccess(*constructPlatform()->peekExecutionEnvironment()->rootDeviceEnvironments[0]) {}
-    DrmMockSuccess(RootDeviceEnvironment &rootDeviceEnvironment) : Drm(mockFd, rootDeviceEnvironment) {}
+    DrmMockSuccess(RootDeviceEnvironment &rootDeviceEnvironment) : Drm(std::make_unique<HwDeviceId>(mockFd), rootDeviceEnvironment) {}
 
     int ioctl(unsigned long request, void *arg) override { return 0; };
 };
 
 class DrmMockFail : public Drm {
   public:
-    DrmMockFail() : Drm(mockFd, *constructPlatform()->peekExecutionEnvironment()->rootDeviceEnvironments[0]) {}
+    DrmMockFail() : Drm(std::make_unique<HwDeviceId>(mockFd), *constructPlatform()->peekExecutionEnvironment()->rootDeviceEnvironments[0]) {}
 
     int ioctl(unsigned long request, void *arg) override { return -1; };
 };
@@ -327,7 +328,7 @@ class DrmMockCustom : public Drm {
         ioctl_res_ext = &NONE;
     }
 
-    DrmMockCustom() : Drm(mockFd, *constructPlatform()->peekExecutionEnvironment()->rootDeviceEnvironments[0]) {
+    DrmMockCustom() : Drm(std::make_unique<HwDeviceId>(mockFd), *constructPlatform()->peekExecutionEnvironment()->rootDeviceEnvironments[0]) {
         reset();
         ioctl_expected.contextCreate = static_cast<int>(NEO::HwHelper::get(NEO::platformDevices[0]->platform.eRenderCoreFamily).getGpgpuEngineInstances().size());
         ioctl_expected.contextDestroy = ioctl_expected.contextCreate.load();
diff --git a/unit_tests/os_interface/linux/device_factory_tests.cpp b/unit_tests/os_interface/linux/device_factory_tests.cpp
@@ -73,7 +73,6 @@ TEST_F(DeviceFactoryLinuxTest, ReleaseDevices) {
 
     mockDeviceFactory.releaseDevices();
     EXPECT_TRUE(mockDeviceFactory.getNumDevices() == 0);
-    EXPECT_TRUE(pDrm->getFileDescriptor() == -1);
 }
 
 TEST_F(DeviceFactoryLinuxTest, givenGetDeviceCallWhenItIsDoneThenOsInterfaceIsAllocatedAndItContainDrm) {
@@ -82,5 +81,6 @@ TEST_F(DeviceFactoryLinuxTest, givenGetDeviceCallWhenItIsDoneThenOsInterfaceIsAl
     bool success = mockDeviceFactory.getDevices(numDevices, executionEnvironment);
     EXPECT_TRUE(success);
     EXPECT_NE(nullptr, executionEnvironment.rootDeviceEnvironments[0]->osInterface);
+    EXPECT_NE(nullptr, pDrm);
     EXPECT_EQ(pDrm, executionEnvironment.rootDeviceEnvironments[0]->osInterface->get()->getDrm());
 }
diff --git a/unit_tests/os_interface/linux/drm_gem_close_worker_tests.cpp b/unit_tests/os_interface/linux/drm_gem_close_worker_tests.cpp
@@ -36,7 +36,7 @@ class DrmMockForWorker : public Drm {
     std::atomic<int> gem_close_cnt;
     std::atomic<int> gem_close_expected;
     std::atomic<std::thread::id> ioctl_caller_thread_id;
-    DrmMockForWorker() : Drm(33, *platform()->peekExecutionEnvironment()->rootDeviceEnvironments[0]) {
+    DrmMockForWorker() : Drm(std::make_unique<HwDeviceId>(33), *platform()->peekExecutionEnvironment()->rootDeviceEnvironments[0]) {
     }
     int ioctl(unsigned long request, void *arg) override {
         if (_IOC_TYPE(request) == DRM_IOCTL_BASE) {
diff --git a/unit_tests/os_interface/linux/drm_mock.cpp b/unit_tests/os_interface/linux/drm_mock.cpp
@@ -1,12 +1,14 @@
 /*
- * Copyright (C) 2019 Intel Corporation
+ * Copyright (C) 2019-2020 Intel Corporation
  *
  * SPDX-License-Identifier: MIT
  *
  */
 
 #include "unit_tests/os_interface/linux/drm_mock.h"
 
+const int DrmMock::mockFd;
+
 int DrmMock::ioctl(unsigned long request, void *arg) {
     ioctlCallsCount++;
 
diff --git a/unit_tests/os_interface/linux/drm_mock.h b/unit_tests/os_interface/linux/drm_mock.h
@@ -32,7 +32,7 @@ class DrmMock : public Drm {
     using Drm::query;
     using Drm::sliceCountChangeSupported;
 
-    DrmMock(RootDeviceEnvironment &rootDeviceEnvironment) : Drm(mockFd, rootDeviceEnvironment) {
+    DrmMock(RootDeviceEnvironment &rootDeviceEnvironment) : Drm(std::make_unique<HwDeviceId>(mockFd), rootDeviceEnvironment) {
         sliceCountChangeSupported = true;
     }
     DrmMock() : DrmMock(*platform()->peekExecutionEnvironment()->rootDeviceEnvironments[0]) {}
@@ -69,7 +69,7 @@ class DrmMock : public Drm {
     }
 
     void setFileDescriptor(int fd) {
-        this->fd = fd;
+        hwDeviceId = std::make_unique<HwDeviceId>(fd);
     }
 
     void setDeviceID(int deviceId) { this->deviceId = deviceId; }
diff --git a/unit_tests/os_interface/linux/drm_neo_create.cpp b/unit_tests/os_interface/linux/drm_neo_create.cpp
@@ -45,7 +45,7 @@ void pushDrmMock(Drm *mock) { drmMockStack.push_back(mock); }
 
 void popDrmMock() { drmMockStack.pop_back(); }
 
-Drm::~Drm() { fd = -1; }
+Drm::~Drm() = default;
 
 Drm *Drm::get(int32_t deviceOrdinal) {
     // We silently skip deviceOrdinal
@@ -61,5 +61,5 @@ Drm *Drm::create(int32_t deviceOrdinal, RootDeviceEnvironment &rootDeviceEnviron
     return drmMockStack[drmMockStack.size() - 1];
 }
 
-void Drm::closeDevice(int32_t deviceOrdinal) { drmMockStack[drmMockStack.size() - 1]->fd = -1; }
+void Drm::closeDevice(int32_t deviceOrdinal) {}
 } // namespace NEO
diff --git a/unit_tests/os_interface/linux/drm_tests.cpp b/unit_tests/os_interface/linux/drm_tests.cpp
@@ -388,3 +388,19 @@ TEST(DrmTest, givenPlatformWithSupportToChangeSliceCountWhenCallSetQueueSliceCou
     EXPECT_EQ(0, drm->getQueueSliceCount(&sseu));
     EXPECT_EQ(drm->getSliceMask(newSliceCount), sseu.slice_mask);
 }
+namespace NEO {
+namespace SysCalls {
+extern uint32_t closeFuncCalled;
+extern int closeFuncArgPassed;
+} // namespace SysCalls
+} // namespace NEO
+
+TEST(HwDeviceId, whenHwDeviceIdIsDestroyedThenFileDescriptorIsClosed) {
+    SysCalls::closeFuncCalled = 0;
+    int fileDescriptor = 0x1234;
+    {
+        HwDeviceId hwDeviceId(fileDescriptor);
+    }
+    EXPECT_EQ(1u, SysCalls::closeFuncCalled);
+    EXPECT_EQ(fileDescriptor, SysCalls::closeFuncArgPassed);
+}
diff --git a/unit_tests/os_interface/linux/sys_calls_linux_ult.cpp b/unit_tests/os_interface/linux/sys_calls_linux_ult.cpp
@@ -0,0 +1,23 @@
+/*
+ * Copyright (C) 2020 Intel Corporation
+ *
+ * SPDX-License-Identifier: MIT
+ *
+ */
+
+#include "core/os_interface/linux/sys_calls.h"
+
+#include <cstdint>
+
+namespace NEO {
+namespace SysCalls {
+uint32_t closeFuncCalled = 0u;
+int closeFuncArgPassed = 0;
+
+int close(int fileDescriptor) {
+    closeFuncCalled++;
+    closeFuncArgPassed = fileDescriptor;
+    return 0;
+}
+} // namespace SysCalls
+} // namespace NEO

Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,7 @@ class DrmNullDevice : public Drm {`
`38`	`38`	`}`
`39`	`39`	`}`
`40`	`40`
`41`		`- DrmNullDevice(int fd, RootDeviceEnvironment &rootDeviceEnvironment) : Drm(fd, rootDeviceEnvironment), gpuTimestamp(0){};`
	`41`	`+ DrmNullDevice(std::unique_ptr<HwDeviceId> hwDeviceId, RootDeviceEnvironment &rootDeviceEnvironment) : Drm(std::move(hwDeviceId), rootDeviceEnvironment), gpuTimestamp(0){};`
`42`	`42`
`43`	`43`	`protected:`
`44`	`44`	`uint64_t gpuTimestamp;`
Original file line number	Diff line number	Diff line change
`@@ -47,6 +47,7 @@ set(RUNTIME_SRCS_DLL_LINUX`
`47`	`47`	`${CMAKE_CURRENT_SOURCE_DIR}/linux/drm_neo_create.cpp`
`48`	`48`	`${CMAKE_CURRENT_SOURCE_DIR}/linux/options_linux.cpp`
`49`	`49`	`${CMAKE_CURRENT_SOURCE_DIR}/linux/os_interface.cpp`
	`50`	`+ ${NEO_SOURCE_DIR}/core/os_interface/linux/sys_calls_linux.cpp`
`50`	`51`	`)`
`51`	`52`
`52`	`53`	`set(RUNTIME_SRCS_DLL_WINDOWS`