From 9cce1183cd9c6b3230918b413d4f6a093e81d08c Mon Sep 17 00:00:00 2001 From: Compute-Runtime-Validation Date: Wed, 13 Mar 2024 13:53:42 +0100 Subject: [PATCH] Revert "feature: use prelim reset_stats for detailed statisics" This reverts commit 835dc8b594ea84b7af784eaaad8af40f1697b1ef. Signed-off-by: Compute-Runtime-Validation --- shared/source/os_interface/linux/drm_neo.cpp | 2 +- .../source/os_interface/linux/i915_prelim.h | 2 +- .../source/os_interface/linux/ioctl_helper.h | 11 ----- .../linux/ioctl_helper_prelim.cpp | 27 +---------- .../linux/ioctl_helper_upstream.cpp | 4 -- .../os_interface/linux/xe/ioctl_helper_xe.cpp | 4 -- .../os_interface/linux/xe/ioctl_helper_xe.h | 1 - .../linux/ioctl_helper_tests_prelim.cpp | 45 ++----------------- .../linux/ioctl_helper_tests_upstream.cpp | 12 ----- .../linux/xe/ioctl_helper_xe_tests.cpp | 14 ------ 10 files changed, 7 insertions(+), 115 deletions(-) diff --git a/shared/source/os_interface/linux/drm_neo.cpp b/shared/source/os_interface/linux/drm_neo.cpp index 928414f8af..5301af55db 100644 --- a/shared/source/os_interface/linux/drm_neo.cpp +++ b/shared/source/os_interface/linux/drm_neo.cpp @@ -245,7 +245,7 @@ bool Drm::isGpuHangDetected(OsContext &osContext) { ResetStats resetStats{}; resetStats.contextId = drmContextId; - const auto retVal{ioctlHelper->getResetStats(resetStats, nullptr, nullptr)}; + const auto retVal{ioctlHelper->ioctl(DrmIoctl::getResetStats, &resetStats)}; UNRECOVERABLE_IF(retVal != 0); if (resetStats.batchActive > 0 || resetStats.batchPending > 0) { diff --git a/shared/source/os_interface/linux/i915_prelim.h b/shared/source/os_interface/linux/i915_prelim.h index c72d20377a..ddf40a589a 100644 --- a/shared/source/os_interface/linux/i915_prelim.h +++ b/shared/source/os_interface/linux/i915_prelim.h @@ -36,6 +36,7 @@ using NEO::PrelimI915::drm_i915_query; using NEO::PrelimI915::drm_i915_query_engine_info; using NEO::PrelimI915::drm_i915_query_memory_regions; using NEO::PrelimI915::drm_i915_reg_read; +using NEO::PrelimI915::drm_i915_reset_stats; using NEO::PrelimI915::drm_prime_handle; using NEO::PrelimI915::i915_context_engines_load_balance; using NEO::PrelimI915::i915_context_param_engines; @@ -78,7 +79,6 @@ using NEO::PrelimI915::prelim_drm_i915_gem_vm_region_ext; using NEO::PrelimI915::prelim_drm_i915_gem_wait_user_fence; using NEO::PrelimI915::prelim_drm_i915_query_distance_info; using NEO::PrelimI915::prelim_drm_i915_query_hw_ip_version; -using NEO::PrelimI915::prelim_drm_i915_reset_stats; using NEO::PrelimI915::prelim_drm_i915_uuid_control; using NEO::PrelimI915::prelim_drm_i915_vm_bind_ext_set_pat; using NEO::PrelimI915::prelim_drm_i915_vm_bind_ext_user_fence; diff --git a/shared/source/os_interface/linux/ioctl_helper.h b/shared/source/os_interface/linux/ioctl_helper.h index cb187c4063..a25821f70f 100644 --- a/shared/source/os_interface/linux/ioctl_helper.h +++ b/shared/source/os_interface/linux/ioctl_helper.h @@ -72,14 +72,6 @@ struct UuidRegisterResult { uint32_t handle; }; -struct ResetStatsFault { - uint64_t addr; - uint16_t type; - uint16_t level; - uint16_t access; - uint16_t flags; -}; - using MemRegionsVec = StackVec; using VmBindExtSetPatT = uint8_t[40]; using VmBindExtUserFenceT = uint8_t[56]; @@ -131,7 +123,6 @@ class IoctlHelper { virtual uint32_t getVmAdviseAtomicAttribute() = 0; virtual int vmBind(const VmBindParams &vmBindParams) = 0; virtual int vmUnbind(const VmBindParams &vmBindParams) = 0; - virtual int getResetStats(ResetStats &resetStats, uint32_t *status, ResetStatsFault *resetStatsFault) = 0; virtual bool getEuStallProperties(std::array &properties, uint64_t dssBufferSize, uint64_t samplingRate, uint64_t pollPeriod, uint64_t engineInstance, uint64_t notifyNReports) = 0; virtual uint32_t getEuStallFdParameter() = 0; @@ -265,7 +256,6 @@ class IoctlHelperUpstream : public IoctlHelperI915 { uint32_t getVmAdviseAtomicAttribute() override; int vmBind(const VmBindParams &vmBindParams) override; int vmUnbind(const VmBindParams &vmBindParams) override; - int getResetStats(ResetStats &resetStats, uint32_t *status, ResetStatsFault *resetStatsFault) override; bool getEuStallProperties(std::array &properties, uint64_t dssBufferSize, uint64_t samplingRate, uint64_t pollPeriod, uint64_t engineInstance, uint64_t notifyNReports) override; uint32_t getEuStallFdParameter() override; @@ -341,7 +331,6 @@ class IoctlHelperPrelim20 : public IoctlHelperI915 { uint32_t getVmAdviseAtomicAttribute() override; int vmBind(const VmBindParams &vmBindParams) override; int vmUnbind(const VmBindParams &vmBindParams) override; - int getResetStats(ResetStats &resetStats, uint32_t *status, ResetStatsFault *resetStatsFault) override; bool getEuStallProperties(std::array &properties, uint64_t dssBufferSize, uint64_t samplingRate, uint64_t pollPeriod, uint64_t engineInstance, uint64_t notifyNReports) override; uint32_t getEuStallFdParameter() override; diff --git a/shared/source/os_interface/linux/ioctl_helper_prelim.cpp b/shared/source/os_interface/linux/ioctl_helper_prelim.cpp index 8954ad6783..886ea3efdf 100644 --- a/shared/source/os_interface/linux/ioctl_helper_prelim.cpp +++ b/shared/source/os_interface/linux/ioctl_helper_prelim.cpp @@ -725,27 +725,6 @@ int IoctlHelperPrelim20::vmUnbind(const VmBindParams &vmBindParams) { return IoctlHelper::ioctl(DrmIoctl::gemVmUnbind, &prelimVmBind); } -int IoctlHelperPrelim20::getResetStats(ResetStats &resetStats, uint32_t *status, ResetStatsFault *resetStatsFault) { - prelim_drm_i915_reset_stats prelimResetStats{}; - prelimResetStats.ctx_id = resetStats.contextId; - prelimResetStats.flags = resetStats.flags; - - const auto retVal = ioctl(DrmIoctl::getResetStats, &prelimResetStats); - - resetStats.resetCount = prelimResetStats.reset_count; - resetStats.batchActive = prelimResetStats.batch_active; - resetStats.batchPending = prelimResetStats.batch_pending; - if (status) { - *status = prelimResetStats.status; - } - if (resetStatsFault) { - auto fault = reinterpret_cast(&(prelimResetStats.fault)); - *resetStatsFault = *fault; - } - - return retVal; -} - UuidRegisterResult IoctlHelperPrelim20::registerUuid(const std::string &uuid, uint32_t uuidClass, uint64_t ptr, uint64_t size) { prelim_drm_i915_uuid_control uuidControl = {}; memcpy_s(uuidControl.uuid, sizeof(uuidControl.uuid), uuid.c_str(), uuid.size()); @@ -824,7 +803,7 @@ unsigned int IoctlHelperPrelim20::getIoctlRequestValue(DrmIoctl ioctlRequest) co case DrmIoctl::gemCacheReserve: return PRELIM_DRM_IOCTL_I915_GEM_CACHE_RESERVE; case DrmIoctl::getResetStats: - return PRELIM_DRM_IOCTL_I915_GET_RESET_STATS; + return DRM_IOCTL_I915_GET_RESET_STATS; default: return IoctlHelperI915::getIoctlRequestValue(ioctlRequest); } @@ -883,8 +862,6 @@ std::string IoctlHelperPrelim20::getIoctlString(DrmIoctl ioctlRequest) const { return "PRELIM_DRM_IOCTL_I915_GEM_CLOS_FREE"; case DrmIoctl::gemCacheReserve: return "PRELIM_DRM_IOCTL_I915_GEM_CACHE_RESERVE"; - case DrmIoctl::getResetStats: - return "PRELIM_DRM_IOCTL_I915_GET_RESET_STATS"; default: return IoctlHelperI915::getIoctlString(ioctlRequest); } @@ -1128,5 +1105,5 @@ int IoctlHelperPrelim20::getEuDebugSysFsEnable() { static_assert(sizeof(MemoryClassInstance) == sizeof(prelim_drm_i915_gem_memory_class_instance)); static_assert(offsetof(MemoryClassInstance, memoryClass) == offsetof(prelim_drm_i915_gem_memory_class_instance, memory_class)); static_assert(offsetof(MemoryClassInstance, memoryInstance) == offsetof(prelim_drm_i915_gem_memory_class_instance, memory_instance)); -static_assert(sizeof(ResetStatsFault) == sizeof(prelim_drm_i915_reset_stats::fault)); + } // namespace NEO diff --git a/shared/source/os_interface/linux/ioctl_helper_upstream.cpp b/shared/source/os_interface/linux/ioctl_helper_upstream.cpp index 56ae71bf3b..1aa990dd9b 100644 --- a/shared/source/os_interface/linux/ioctl_helper_upstream.cpp +++ b/shared/source/os_interface/linux/ioctl_helper_upstream.cpp @@ -247,10 +247,6 @@ int IoctlHelperUpstream::vmUnbind(const VmBindParams &vmBindParams) { return 0; } -int IoctlHelperUpstream::getResetStats(ResetStats &resetStats, uint32_t *status, ResetStatsFault *resetStatsFault) { - return ioctl(DrmIoctl::getResetStats, &resetStats); -} - UuidRegisterResult IoctlHelperUpstream::registerUuid(const std::string &uuid, uint32_t uuidClass, uint64_t ptr, uint64_t size) { return {0, 0}; } diff --git a/shared/source/os_interface/linux/xe/ioctl_helper_xe.cpp b/shared/source/os_interface/linux/xe/ioctl_helper_xe.cpp index 8d3813aa55..29a392958f 100644 --- a/shared/source/os_interface/linux/xe/ioctl_helper_xe.cpp +++ b/shared/source/os_interface/linux/xe/ioctl_helper_xe.cpp @@ -856,10 +856,6 @@ int IoctlHelperXe::vmUnbind(const VmBindParams &vmBindParams) { return xeVmBind(vmBindParams, false); } -int IoctlHelperXe::getResetStats(ResetStats &resetStats, uint32_t *status, ResetStatsFault *resetStatsFault) { - return ioctl(DrmIoctl::getResetStats, &resetStats); -} - UuidRegisterResult IoctlHelperXe::registerUuid(const std::string &uuid, uint32_t uuidClass, uint64_t ptr, uint64_t size) { xeLog(" -> IoctlHelperXe::%s\n", __FUNCTION__); return {}; diff --git a/shared/source/os_interface/linux/xe/ioctl_helper_xe.h b/shared/source/os_interface/linux/xe/ioctl_helper_xe.h index a79b046435..8d54df4ddd 100644 --- a/shared/source/os_interface/linux/xe/ioctl_helper_xe.h +++ b/shared/source/os_interface/linux/xe/ioctl_helper_xe.h @@ -93,7 +93,6 @@ class IoctlHelperXe : public IoctlHelper { uint32_t getVmAdviseAtomicAttribute() override; int vmBind(const VmBindParams &vmBindParams) override; int vmUnbind(const VmBindParams &vmBindParams) override; - int getResetStats(ResetStats &resetStats, uint32_t *status, ResetStatsFault *resetStatsFault) override; bool getEuStallProperties(std::array &properties, uint64_t dssBufferSize, uint64_t samplingRate, uint64_t pollPeriod, uint64_t engineInstance, uint64_t notifyNReports) override; uint32_t getEuStallFdParameter() override; 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 index ca0726946c..6eee015c81 100644 --- 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 @@ -63,7 +63,7 @@ TEST_F(IoctlPrelimHelperTests, whenGettingIoctlRequestValueThenPropertValueIsRet EXPECT_EQ(ioctlHelper.getIoctlRequestValue(DrmIoctl::gemContextCreateExt), static_cast(DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT)); EXPECT_EQ(ioctlHelper.getIoctlRequestValue(DrmIoctl::gemContextDestroy), static_cast(DRM_IOCTL_I915_GEM_CONTEXT_DESTROY)); EXPECT_EQ(ioctlHelper.getIoctlRequestValue(DrmIoctl::regRead), static_cast(DRM_IOCTL_I915_REG_READ)); - EXPECT_EQ(ioctlHelper.getIoctlRequestValue(DrmIoctl::getResetStats), static_cast(PRELIM_DRM_IOCTL_I915_GET_RESET_STATS)); + EXPECT_EQ(ioctlHelper.getIoctlRequestValue(DrmIoctl::getResetStats), static_cast(DRM_IOCTL_I915_GET_RESET_STATS)); EXPECT_EQ(ioctlHelper.getIoctlRequestValue(DrmIoctl::gemContextGetparam), static_cast(DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM)); EXPECT_EQ(ioctlHelper.getIoctlRequestValue(DrmIoctl::gemContextSetparam), static_cast(DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM)); EXPECT_EQ(ioctlHelper.getIoctlRequestValue(DrmIoctl::query), static_cast(DRM_IOCTL_I915_QUERY)); @@ -115,7 +115,7 @@ TEST_F(IoctlPrelimHelperTests, whenGettingIoctlRequestStringThenProperStringIsRe EXPECT_STREQ(ioctlHelper.getIoctlString(DrmIoctl::gemContextCreateExt).c_str(), "DRM_IOCTL_I915_GEM_CONTEXT_CREATE_EXT"); EXPECT_STREQ(ioctlHelper.getIoctlString(DrmIoctl::gemContextDestroy).c_str(), "DRM_IOCTL_I915_GEM_CONTEXT_DESTROY"); EXPECT_STREQ(ioctlHelper.getIoctlString(DrmIoctl::regRead).c_str(), "DRM_IOCTL_I915_REG_READ"); - EXPECT_STREQ(ioctlHelper.getIoctlString(DrmIoctl::getResetStats).c_str(), "PRELIM_DRM_IOCTL_I915_GET_RESET_STATS"); + EXPECT_STREQ(ioctlHelper.getIoctlString(DrmIoctl::getResetStats).c_str(), "DRM_IOCTL_I915_GET_RESET_STATS"); EXPECT_STREQ(ioctlHelper.getIoctlString(DrmIoctl::gemContextGetparam).c_str(), "DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM"); EXPECT_STREQ(ioctlHelper.getIoctlString(DrmIoctl::gemContextSetparam).c_str(), "DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM"); EXPECT_STREQ(ioctlHelper.getIoctlString(DrmIoctl::query).c_str(), "DRM_IOCTL_I915_QUERY"); @@ -389,17 +389,6 @@ struct MockIoctlHelperPrelim20 : IoctlHelperPrelim20 { return *overrideGemCreateExtReturnValue; } } - if (request == DrmIoctl::getResetStats) { - if (overrideResetStats.has_value()) { - auto resetStats = reinterpret_cast(arg); - *resetStats = std::get<0>(overrideResetStats.value()); - auto resetStatsPrelim = reinterpret_cast(arg); - resetStatsPrelim->status = std::get<1>(overrideResetStats.value()); - auto fault = std::get<2>(overrideResetStats.value()); - resetStatsPrelim->fault = {fault.addr, fault.type, fault.level, fault.access, fault.flags}; - return 0; - } - } return IoctlHelperPrelim20::ioctl(request, arg); } bool checkWhetherGemCreateExtContainsMemPolicy(void *arg) { @@ -427,7 +416,6 @@ struct MockIoctlHelperPrelim20 : IoctlHelperPrelim20 { uint32_t lastPolicyMode = 0; uint32_t lastPolicyFlags = 0; std::vector lastPolicyNodeMask{}; - std::optional> overrideResetStats{}; }; TEST(IoctlPrelimHelperCreateGemExtTests, givenPrelimWhenCreateGemExtWithMemPolicyThenMemPolicyExtensionsIsAdded) { @@ -822,35 +810,8 @@ TEST_F(IoctlPrelimHelperTests, givenInitializeGetGpuTimeFunctionNotCalledWhenSet EXPECT_EQ(false, ret); } -TEST_F(IoctlPrelimHelperTests, givenPrelimWhenGetFdFromVmExportIsCalledThenFalseIsReturned) { +TEST_F(IoctlPrelimHelperTests, givenUpstreamWhenGetFdFromVmExportIsCalledThenFalseIsReturned) { uint32_t vmId = 0, flags = 0; int32_t fd = 0; EXPECT_FALSE(ioctlHelper.getFdFromVmExport(vmId, flags, &fd)); } - -TEST_F(IoctlPrelimHelperTests, whenGetResetStatsIsCalledThenCorrectValueIsReturned) { - auto executionEnvironment = std::make_unique(); - auto drm = std::make_unique(*executionEnvironment->rootDeviceEnvironments[0]); - MockIoctlHelperPrelim20 mockIoctlHelper{*drm}; - - uint32_t status = 1; - ResetStatsFault fault{}; - fault.flags = 1; - - ResetStats resetStats{}; - resetStats.contextId = 0; - mockIoctlHelper.overrideResetStats = {resetStats, status, fault}; - - status = 0; - fault.flags = 0; - - EXPECT_EQ(0, mockIoctlHelper.getResetStats(resetStats, nullptr, nullptr)); - - EXPECT_EQ(0, mockIoctlHelper.getResetStats(resetStats, &status, nullptr)); - EXPECT_EQ(1u, status); - - status = 0; - EXPECT_EQ(0, mockIoctlHelper.getResetStats(resetStats, &status, &fault)); - EXPECT_EQ(1u, status); - EXPECT_EQ(1, fault.flags); -} 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 index 478754280d..b26b3349f2 100644 --- 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 @@ -816,15 +816,3 @@ TEST(IoctlHelperTestsUpstream, givenUpstreamWhenGetFdFromVmExportIsCalledThenFal int32_t fd = 0; EXPECT_FALSE(ioctlHelper.getFdFromVmExport(vmId, flags, &fd)); } - -TEST(IoctlHelperTestsUpstream, whenCallingGetResetStatsThenSuccessIsReturned) { - auto executionEnvironment = std::make_unique(); - auto drm = std::make_unique(*executionEnvironment->rootDeviceEnvironments[0]); - IoctlHelperUpstream ioctlHelper{*drm}; - - ResetStats resetStats{}; - resetStats.contextId = 0; - drm->resetStatsToReturn.push_back(resetStats); - - EXPECT_EQ(0, ioctlHelper.getResetStats(resetStats, nullptr, nullptr)); -} diff --git a/shared/test/unit_test/os_interface/linux/xe/ioctl_helper_xe_tests.cpp b/shared/test/unit_test/os_interface/linux/xe/ioctl_helper_xe_tests.cpp index 4250b7a631..8e43db2412 100644 --- a/shared/test/unit_test/os_interface/linux/xe/ioctl_helper_xe_tests.cpp +++ b/shared/test/unit_test/os_interface/linux/xe/ioctl_helper_xe_tests.cpp @@ -1722,17 +1722,3 @@ TEST(IoctlHelperXeTest, whenCallingVmBindThenPatIndexIsSet) { EXPECT_EQ(drm.vmBindInputs[0].bind.pat_index, expectedPatIndex); } - -TEST(IoctlHelperXeTest, whenCallingGetResetStatsThenSuccessIsReturned) { - auto executionEnvironment = std::make_unique(); - DrmMockXe drm{*executionEnvironment->rootDeviceEnvironments[0]}; - - auto xeIoctlHelper = std::make_unique(drm); - drm.memoryInfo.reset(xeIoctlHelper->createMemoryInfo().release()); - ASSERT_NE(nullptr, xeIoctlHelper); - - ResetStats resetStats{}; - resetStats.contextId = 0; - - EXPECT_EQ(0, xeIoctlHelper->getResetStats(resetStats, nullptr, nullptr)); -}