From 3e59a2f10820a5ee48e2fa286ebea12eb44ac85b Mon Sep 17 00:00:00 2001 From: Filip Hazubski Date: Wed, 21 May 2025 17:51:30 +0000 Subject: [PATCH] fix: Correct IoctlHelperXe logic to query PF support Move logic in order to respect relevant checks before trying to query support. Signed-off-by: Filip Hazubski --- .../os_interface/linux/xe/ioctl_helper_xe.cpp | 40 +++++++------- .../os_interface/linux/xe/ioctl_helper_xe.h | 12 ----- .../linux/xe/mock_ioctl_helper_xe.h | 1 - .../linux/xe/ioctl_helper_xe_tests.cpp | 53 ++++--------------- 4 files changed, 28 insertions(+), 78 deletions(-) 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 e68aa20c14..df0e59cee8 100644 --- a/shared/source/os_interface/linux/xe/ioctl_helper_xe.cpp +++ b/shared/source/os_interface/linux/xe/ioctl_helper_xe.cpp @@ -265,7 +265,6 @@ bool IoctlHelperXe::initialize() { assignValue(tileIdToMediaGtId, gt.tile_id, gt.gt_id); } } - querySupportedFeatures(); return true; } @@ -1017,8 +1016,25 @@ int IoctlHelperXe::queryDistances(std::vector &queryItems, std::vecto } bool IoctlHelperXe::isPageFaultSupported() { - xeLog(" -> IoctlHelperXe::%s %d\n", __FUNCTION__, false); - return false; + auto checkVmCreateFlagsSupport = [&](uint32_t flags) -> bool { + struct drm_xe_vm_create vmCreate = {}; + vmCreate.flags = flags; + + auto ret = IoctlHelper::ioctl(DrmIoctl::gemVmCreate, &vmCreate); + if (ret == 0) { + struct drm_xe_vm_destroy vmDestroy = {}; + vmDestroy.vm_id = vmCreate.vm_id; + ret = IoctlHelper::ioctl(DrmIoctl::gemVmDestroy, &vmDestroy); + DEBUG_BREAK_IF(ret != 0); + return true; + } + return false; + }; + bool pageFaultSupport = checkVmCreateFlagsSupport(DRM_XE_VM_CREATE_FLAG_LR_MODE | DRM_XE_VM_CREATE_FLAG_FAULT_MODE); + + xeLog(" -> IoctlHelperXe::%s %d\n", __FUNCTION__, pageFaultSupport); + + return pageFaultSupport; } uint32_t IoctlHelperXe::getEuStallFdParameter() { @@ -1864,22 +1880,4 @@ std::string IoctlHelperXe::getIoctlString(DrmIoctl ioctlRequest) const { } } -void IoctlHelperXe::querySupportedFeatures() { - auto checkVmCreateFlagsSupport = [&](uint32_t flags) -> bool { - struct drm_xe_vm_create vmCreate = {}; - vmCreate.flags = flags; - - auto ret = IoctlHelper::ioctl(DrmIoctl::gemVmCreate, &vmCreate); - if (ret == 0) { - struct drm_xe_vm_destroy vmDestroy = {}; - vmDestroy.vm_id = vmCreate.vm_id; - ret = IoctlHelper::ioctl(DrmIoctl::gemVmDestroy, &vmDestroy); - DEBUG_BREAK_IF(ret != 0); - return true; - } - return false; - }; - supportedFeatures.flags.pageFault = checkVmCreateFlagsSupport(DRM_XE_VM_CREATE_FLAG_LR_MODE | DRM_XE_VM_CREATE_FLAG_FAULT_MODE); -}; - } // namespace NEO 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 aca4d12e4f..d06c9fb1d3 100644 --- a/shared/source/os_interface/linux/xe/ioctl_helper_xe.h +++ b/shared/source/os_interface/linux/xe/ioctl_helper_xe.h @@ -223,18 +223,6 @@ class IoctlHelperXe : public IoctlHelper { uint32_t drmContextId; }; - struct SupportedFeatures { - union { - struct { - uint32_t pageFault : 1; - uint32_t reserved : 31; - } flags; - uint32_t allFlags = 0; - }; - } supportedFeatures{}; - static_assert(sizeof(SupportedFeatures::flags) == sizeof(SupportedFeatures::allFlags), ""); - - void querySupportedFeatures(); std::unique_ptr euDebugInterface; }; diff --git a/shared/test/common/os_interface/linux/xe/mock_ioctl_helper_xe.h b/shared/test/common/os_interface/linux/xe/mock_ioctl_helper_xe.h index 09925b1b52..2ae0174e12 100644 --- a/shared/test/common/os_interface/linux/xe/mock_ioctl_helper_xe.h +++ b/shared/test/common/os_interface/linux/xe/mock_ioctl_helper_xe.h @@ -24,7 +24,6 @@ struct MockIoctlHelperXe : IoctlHelperXe { using IoctlHelperXe::maxExecQueuePriority; using IoctlHelperXe::queryGtListData; using IoctlHelperXe::setContextProperties; - using IoctlHelperXe::supportedFeatures; using IoctlHelperXe::tileIdToGtId; using IoctlHelperXe::updateBindInfo; using IoctlHelperXe::UserFenceExtension; 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 21ff0d6507..2387c19a4f 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 @@ -414,8 +414,6 @@ TEST_F(IoctlHelperXeTest, givenIoctlHelperXeWhenCallingAnyMethodThenDummyValueIs EXPECT_FALSE(xeIoctlHelper->completionFenceExtensionSupported(false)); - EXPECT_EQ(false, xeIoctlHelper->isPageFaultSupported()); - EXPECT_EQ(nullptr, xeIoctlHelper->createVmControlExtRegion({})); GemContextCreateExt gcc; @@ -2391,68 +2389,35 @@ TEST_F(IoctlHelperXeTest, givenImmediateAndReadOnlyBindFlagsSupportedWhenGetting } } -struct DrmMockXeVmBind : public DrmMockXe { +struct DrmMockXePageFault : public DrmMockXe { static auto create(RootDeviceEnvironment &rootDeviceEnvironment) { - auto drm = std::unique_ptr(new DrmMockXeVmBind{rootDeviceEnvironment}); + auto drm = std::unique_ptr(new DrmMockXePageFault{rootDeviceEnvironment}); drm->initInstance(); - return drm; } int ioctl(DrmIoctl request, void *arg) override { - switch (request) { - case DrmIoctl::gemVmCreate: { - auto vmCreate = static_cast(arg); - if (deviceIsInFaultMode && - ((vmCreate->flags & DRM_XE_VM_CREATE_FLAG_LR_MODE) == 0 || - (vmCreate->flags & DRM_XE_VM_CREATE_FLAG_FAULT_MODE) == 0)) { - return -EINVAL; - } - - if ((vmCreate->flags & DRM_XE_VM_CREATE_FLAG_FAULT_MODE) == DRM_XE_VM_CREATE_FLAG_FAULT_MODE && - (vmCreate->flags & DRM_XE_VM_CREATE_FLAG_LR_MODE) == DRM_XE_VM_CREATE_FLAG_LR_MODE && - (!supportsRecoverablePageFault)) { - return -EINVAL; - } + if (supportsRecoverablePageFault) { return 0; - } break; - - default: - return DrmMockXe::ioctl(request, arg); } + return -EINVAL; }; bool supportsRecoverablePageFault = true; - bool deviceIsInFaultMode = false; - protected: // Don't call directly, use the create() function - DrmMockXeVmBind(RootDeviceEnvironment &rootDeviceEnvironment) : DrmMockXe(rootDeviceEnvironment) {} + DrmMockXePageFault(RootDeviceEnvironment &rootDeviceEnvironment) : DrmMockXe(rootDeviceEnvironment) {} }; -TEST_F(IoctlHelperXeTest, whenInitializeIoctlHelperThenQueryPageFaultFlagsSupport) { - +TEST_F(IoctlHelperXeTest, givenPageFaultSupportIsSetWhenCallingIsPageFaultSupportedThenProperValueIsReturned) { auto executionEnvironment = std::make_unique(); - auto drm = DrmMockXeVmBind::create(*executionEnvironment->rootDeviceEnvironments[0]); + auto drm = DrmMockXePageFault::create(*executionEnvironment->rootDeviceEnvironments[0]); for (const auto &recoverablePageFault : ::testing::Bool()) { drm->supportsRecoverablePageFault = recoverablePageFault; auto xeIoctlHelper = std::make_unique(*drm); xeIoctlHelper->initialize(); - EXPECT_EQ(xeIoctlHelper->supportedFeatures.flags.pageFault, recoverablePageFault); - } -} - -TEST_F(IoctlHelperXeTest, givenDeviceInFaultModeWhenInitializeIoctlHelperThenQueryFeaturesIsSuccessful) { - auto executionEnvironment = std::make_unique(); - auto drm = DrmMockXeVmBind::create(*executionEnvironment->rootDeviceEnvironments[0]); - drm->deviceIsInFaultMode = true; - - for (const auto &recoverablePageFault : ::testing::Bool()) { - drm->supportsRecoverablePageFault = recoverablePageFault; - auto xeIoctlHelper = std::make_unique(*drm); - xeIoctlHelper->initialize(); - EXPECT_EQ(xeIoctlHelper->supportedFeatures.flags.pageFault, recoverablePageFault); + EXPECT_EQ(xeIoctlHelper->isPageFaultSupported(), recoverablePageFault); } } @@ -2982,4 +2947,4 @@ TEST_F(IoctlHelperXeTest, whenInitializeIoctlHelperAndLowLatencyAvailableButDebu xeQueryConfig->info[DRM_XE_QUERY_CONFIG_FLAGS] = DRM_XE_QUERY_CONFIG_FLAG_HAS_LOW_LATENCY; xeIoctlHelper->initialize(); EXPECT_FALSE(static_cast(xeIoctlHelper)->isLowLatencyHintAvailable); -} \ No newline at end of file +}