From 9aa4275fda6342962902f90de09e25bd0afee7b3 Mon Sep 17 00:00:00 2001 From: "Spruit, Neil R" Date: Wed, 1 Mar 2023 20:17:15 +0000 Subject: [PATCH] Check for valid stype before reading Device Properties pNext Related-To: LOCI-3884 - Added check for valid device properties stype to remove the feature specific debug vars that enabled/disabled reading of the pNext. - Requires applications to properly set the device properties stype in order for the pNext to be read for extensions. Signed-off-by: Spruit, Neil R --- level_zero/core/source/device/device_imp.cpp | 12 ++++---- .../sources/device/device_drm/test_device.cpp | 10 ++----- .../device/device_drm_or_wddm/test_device.cpp | 10 ------- .../device/device_wddm/test_device.cpp | 2 -- .../sources/device/test_l0_device.cpp | 29 ++----------------- .../xe_hpc_core/pvc/test_device_pvc.cpp | 6 ---- .../debug_settings/debug_variables_base.inl | 3 -- shared/test/common/test_files/igdrcl.config | 3 -- 8 files changed, 12 insertions(+), 63 deletions(-) diff --git a/level_zero/core/source/device/device_imp.cpp b/level_zero/core/source/device/device_imp.cpp index 059113c456..2aab1e2a2b 100644 --- a/level_zero/core/source/device/device_imp.cpp +++ b/level_zero/core/source/device/device_imp.cpp @@ -837,19 +837,19 @@ ze_result_t DeviceImp::getProperties(ze_device_properties_t *pDeviceProperties) memcpy_s(pDeviceProperties->name, ZE_MAX_DEVICE_NAME, name.c_str(), name.length() + 1); ze_base_properties_t *extendedProperties = reinterpret_cast(pDeviceProperties->pNext); - bool anySupportedExt = NEO::DebugManager.flags.EnableL0ReadLUIDExtension.get() || NEO::DebugManager.flags.EnableL0EuCount.get() || - NEO::DebugManager.flags.EnableL0DeviceIpVersion.get(); - if (anySupportedExt) { + bool isDevicePropertiesStypeValid = (pDeviceProperties->stype == ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES) ? true : false; + + if (isDevicePropertiesStypeValid) { while (extendedProperties) { - if (extendedProperties->stype == ZE_STRUCTURE_TYPE_DEVICE_LUID_EXT_PROPERTIES && NEO::DebugManager.flags.EnableL0ReadLUIDExtension.get()) { + if (extendedProperties->stype == ZE_STRUCTURE_TYPE_DEVICE_LUID_EXT_PROPERTIES) { ze_device_luid_ext_properties_t *deviceLuidProperties = reinterpret_cast(extendedProperties); deviceLuidProperties->nodeMask = queryDeviceNodeMask(); ze_result_t result = queryDeviceLuid(deviceLuidProperties); if (result != ZE_RESULT_SUCCESS) { return result; } - } else if (extendedProperties->stype == ZE_STRUCTURE_TYPE_EU_COUNT_EXT && NEO::DebugManager.flags.EnableL0EuCount.get()) { + } else if (extendedProperties->stype == ZE_STRUCTURE_TYPE_EU_COUNT_EXT) { ze_eu_count_ext_t *zeEuCountDesc = reinterpret_cast(extendedProperties); uint32_t numTotalEUs = hardwareInfo.gtSystemInfo.MaxEuPerSubSlice * hardwareInfo.gtSystemInfo.SubSliceCount * hardwareInfo.gtSystemInfo.SliceCount; @@ -857,7 +857,7 @@ ze_result_t DeviceImp::getProperties(ze_device_properties_t *pDeviceProperties) numTotalEUs *= neoDevice->getNumGenericSubDevices(); } zeEuCountDesc->numTotalEUs = numTotalEUs; - } else if (extendedProperties->stype == ZE_STRUCTURE_TYPE_DEVICE_IP_VERSION_EXT && NEO::DebugManager.flags.EnableL0DeviceIpVersion.get()) { + } else if (extendedProperties->stype == ZE_STRUCTURE_TYPE_DEVICE_IP_VERSION_EXT) { ze_device_ip_version_ext_t *zeDeviceIpVersion = reinterpret_cast(extendedProperties); NEO::Device *activeDevice = getActiveDevice(); auto &productHelper = activeDevice->getProductHelper(); diff --git a/level_zero/core/test/unit_tests/sources/device/device_drm/test_device.cpp b/level_zero/core/test/unit_tests/sources/device/device_drm/test_device.cpp index 9073938082..ea5699f3dc 100644 --- a/level_zero/core/test/unit_tests/sources/device/device_drm/test_device.cpp +++ b/level_zero/core/test/unit_tests/sources/device/device_drm/test_device.cpp @@ -24,7 +24,6 @@ namespace ult { using LuidDeviceTest = Test; TEST_F(LuidDeviceTest, givenLuidDevicePropertiesStructureAndDRMDriverTypeThenUnsupportedReturned) { - DebugManager.flags.EnableL0ReadLUIDExtension.set(true); neoDevice->executionEnvironment->rootDeviceEnvironments[0]->osInterface.reset(new NEO::OSInterface()); neoDevice->executionEnvironment->rootDeviceEnvironments[0]->osInterface->setDriverModel(std::make_unique()); ze_device_properties_t deviceProperties = {ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES}; @@ -32,16 +31,13 @@ TEST_F(LuidDeviceTest, givenLuidDevicePropertiesStructureAndDRMDriverTypeThenUns deviceProperties.pNext = &deviceLuidProperties; ze_result_t result = device->getProperties(&deviceProperties); EXPECT_EQ(result, ZE_RESULT_ERROR_UNSUPPORTED_FEATURE); - DebugManager.flags.EnableL0ReadLUIDExtension.set(false); } -TEST_F(LuidDeviceTest, givenLuidDevicePropertiesStructureAndDebugVariableFalseThenSuccessReturned) { - DebugManager.flags.EnableL0ReadLUIDExtension.set(false); +TEST_F(LuidDeviceTest, givenLuidDevicePropertiesStructureAndStypeIsIncorrectThenpNextIsNotDereferenced) { neoDevice->executionEnvironment->rootDeviceEnvironments[0]->osInterface.reset(new NEO::OSInterface()); neoDevice->executionEnvironment->rootDeviceEnvironments[0]->osInterface->setDriverModel(std::make_unique()); - ze_device_properties_t deviceProperties = {ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES}; - ze_device_luid_ext_properties_t deviceLuidProperties = {ZE_STRUCTURE_TYPE_DEVICE_LUID_EXT_PROPERTIES}; - deviceProperties.pNext = &deviceLuidProperties; + ze_device_properties_t deviceProperties = {}; + deviceProperties.pNext = (void *)0x12345; ze_result_t result = device->getProperties(&deviceProperties); EXPECT_EQ(result, ZE_RESULT_SUCCESS); } diff --git a/level_zero/core/test/unit_tests/sources/device/device_drm_or_wddm/test_device.cpp b/level_zero/core/test/unit_tests/sources/device/device_drm_or_wddm/test_device.cpp index 47776c13c5..452443cc6c 100644 --- a/level_zero/core/test/unit_tests/sources/device/device_drm_or_wddm/test_device.cpp +++ b/level_zero/core/test/unit_tests/sources/device/device_drm_or_wddm/test_device.cpp @@ -77,17 +77,14 @@ class MockOsContextWin : public OsContextWin { using LuidDeviceTest = Test; TEST_F(LuidDeviceTest, givenOsContextWinAndGetDeviceNodeMaskThenNodeMaskIsAtLeast1) { - DebugManager.flags.EnableL0ReadLUIDExtension.set(true); auto luidMock = new MockDriverModelWDDMLUID(*neoDevice->executionEnvironment->rootDeviceEnvironments[0]); auto defaultEngine = defaultHwInfo->capabilityTable.defaultEngineType; OsContextWin osContext(*luidMock, 0, 0u, EngineDescriptorHelper::getDefaultDescriptor({defaultEngine, EngineUsage::Regular})); EXPECT_GE(osContext.getDeviceNodeMask(), 1u); delete luidMock; - DebugManager.flags.EnableL0ReadLUIDExtension.set(false); } TEST_F(LuidDeviceTest, givenOsContextWinAndGetLUIDArrayThenLUIDisValid) { - DebugManager.flags.EnableL0ReadLUIDExtension.set(true); auto luidMock = new MockDriverModelWDDMLUID(*neoDevice->executionEnvironment->rootDeviceEnvironments[0]); auto defaultEngine = defaultHwInfo->capabilityTable.defaultEngineType; OsContextWin osContext(*luidMock, 0, 0u, EngineDescriptorHelper::getDefaultDescriptor({defaultEngine, EngineUsage::Regular})); @@ -98,11 +95,9 @@ TEST_F(LuidDeviceTest, givenOsContextWinAndGetLUIDArrayThenLUIDisValid) { memcpy_s(&luid, sizeof(uint64_t), luidData.data(), sizeof(uint8_t) * luidData.size()); EXPECT_NE(luid, (uint64_t)0); delete luidMock; - DebugManager.flags.EnableL0ReadLUIDExtension.set(false); } TEST_F(LuidDeviceTest, givenLuidDevicePropertiesStructureAndWDDMDriverTypeThenSuccessReturned) { - DebugManager.flags.EnableL0ReadLUIDExtension.set(true); auto defaultEngine = defaultHwInfo->capabilityTable.defaultEngineType; auto luidMock = new MockDriverModelWDDMLUID(*neoDevice->executionEnvironment->rootDeviceEnvironments[0]); neoDevice->executionEnvironment->rootDeviceEnvironments[0]->osInterface.reset(new NEO::OSInterface()); @@ -125,11 +120,9 @@ TEST_F(LuidDeviceTest, givenLuidDevicePropertiesStructureAndWDDMDriverTypeThenSu EXPECT_EQ(lowLUID, (uint32_t)adapterLuid.LowPart); EXPECT_EQ(highLUID, (uint32_t)adapterLuid.HighPart); EXPECT_NE(deviceLuidProperties.nodeMask, (uint32_t)0); - DebugManager.flags.EnableL0ReadLUIDExtension.set(false); } TEST_F(LuidDeviceTest, givenLuidDevicePropertiesStructureAndDRMDriverTypeThenUnsupportedReturned) { - DebugManager.flags.EnableL0ReadLUIDExtension.set(true); neoDevice->executionEnvironment->rootDeviceEnvironments[0]->osInterface.reset(new NEO::OSInterface()); neoDevice->executionEnvironment->rootDeviceEnvironments[0]->osInterface->setDriverModel(std::make_unique()); ze_device_properties_t deviceProperties = {ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES}; @@ -137,18 +130,15 @@ TEST_F(LuidDeviceTest, givenLuidDevicePropertiesStructureAndDRMDriverTypeThenUns deviceProperties.pNext = &deviceLuidProperties; ze_result_t result = device->getProperties(&deviceProperties); EXPECT_EQ(result, ZE_RESULT_ERROR_UNSUPPORTED_FEATURE); - DebugManager.flags.EnableL0ReadLUIDExtension.set(false); } TEST_F(LuidDeviceTest, givenLuidDevicePropertiesStructureAndAndNoOsInterfaceThenUninitReturned) { - DebugManager.flags.EnableL0ReadLUIDExtension.set(true); neoDevice->executionEnvironment->rootDeviceEnvironments[0]->osInterface.reset(nullptr); ze_device_properties_t deviceProperties = {ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES}; ze_device_luid_ext_properties_t deviceLuidProperties = {ZE_STRUCTURE_TYPE_DEVICE_LUID_EXT_PROPERTIES}; deviceProperties.pNext = &deviceLuidProperties; ze_result_t result = device->getProperties(&deviceProperties); EXPECT_EQ(result, ZE_RESULT_ERROR_UNINITIALIZED); - DebugManager.flags.EnableL0ReadLUIDExtension.set(false); } } // namespace ult diff --git a/level_zero/core/test/unit_tests/sources/device/device_wddm/test_device.cpp b/level_zero/core/test/unit_tests/sources/device/device_wddm/test_device.cpp index f8e2ee682d..149b74b18a 100644 --- a/level_zero/core/test/unit_tests/sources/device/device_wddm/test_device.cpp +++ b/level_zero/core/test/unit_tests/sources/device/device_wddm/test_device.cpp @@ -37,7 +37,6 @@ struct LuidDeviceTest : public ::testing::Test { }; TEST_F(LuidDeviceTest, givenLuidDevicePropertiesStructureThenLuidAndNodeMaskSetForWDDMDriverType) { - DebugManager.flags.EnableL0ReadLUIDExtension.set(true); NEO::MockDevice *neoDevice = nullptr; std::unique_ptr> driverHandle; L0::Device *device = nullptr; @@ -81,7 +80,6 @@ TEST_F(LuidDeviceTest, givenLuidDevicePropertiesStructureThenLuidAndNodeMaskSetF EXPECT_EQ(lowLUID, (uint32_t)adapterLuid.LowPart); EXPECT_EQ(highLUID, (uint32_t)adapterLuid.HighPart); EXPECT_NE(deviceLuidProperties.nodeMask, (uint32_t)0); - DebugManager.flags.EnableL0ReadLUIDExtension.set(false); } } // namespace ult diff --git a/level_zero/core/test/unit_tests/sources/device/test_l0_device.cpp b/level_zero/core/test/unit_tests/sources/device/test_l0_device.cpp index 904c266fdb..20af0733c0 100644 --- a/level_zero/core/test/unit_tests/sources/device/test_l0_device.cpp +++ b/level_zero/core/test/unit_tests/sources/device/test_l0_device.cpp @@ -1161,8 +1161,6 @@ TEST_F(DeviceTest, givenDevicePropertiesStructureWhenDebugVariableOverrideDevice } TEST_F(DeviceTest, WhenRequestingZeEuCountThenExpectedEUsAreReturned) { - DebugManagerStateRestore restore; - DebugManager.flags.EnableL0EuCount.set(true); ze_device_properties_t deviceProperties = {ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES}; ze_eu_count_ext_t zeEuCountDesc = {ZE_STRUCTURE_TYPE_EU_COUNT_EXT}; deviceProperties.pNext = &zeEuCountDesc; @@ -1182,10 +1180,8 @@ TEST_F(DeviceTest, WhenRequestingZeEuCountThenExpectedEUsAreReturned) { EXPECT_EQ(expectedEUs, zeEuCountDesc.numTotalEUs); } -TEST_F(DeviceTest, WhenRequestingZeEuCountWithoutDebugKeyThenNoEusAreReturned) { - DebugManagerStateRestore restore; - DebugManager.flags.EnableL0EuCount.set(false); - ze_device_properties_t deviceProperties = {ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES}; +TEST_F(DeviceTest, WhenRequestingZeEuCountWithoutStypeCorrectThenNoEusAreReturned) { + ze_device_properties_t deviceProperties = {}; ze_eu_count_ext_t zeEuCountDesc = {ZE_STRUCTURE_TYPE_EU_COUNT_EXT}; zeEuCountDesc.numTotalEUs = std::numeric_limits::max(); deviceProperties.pNext = &zeEuCountDesc; @@ -1242,10 +1238,7 @@ TEST_F(DeviceTest, GivenDebugApiUsedSetWhenGettingDevicePropertiesThenSubslicesP EXPECT_EQ(16u, deviceProperties.numSubslicesPerSlice); } -TEST_F(DeviceTest, givenDeviceIpVersionWhenGetDevicePropertiesAndDebugFlagEnabledThenCorrectResultIsReturned) { - DebugManagerStateRestore restore; - DebugManager.flags.EnableL0DeviceIpVersion.set(true); - +TEST_F(DeviceTest, givenDeviceIpVersionWhenGetDevicePropertiesThenCorrectResultIsReturned) { ze_device_properties_t deviceProperties = {ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES}; ze_device_ip_version_ext_t zeDeviceIpVersion = {ZE_STRUCTURE_TYPE_DEVICE_IP_VERSION_EXT}; zeDeviceIpVersion.ipVersion = std::numeric_limits::max(); @@ -1259,20 +1252,6 @@ TEST_F(DeviceTest, givenDeviceIpVersionWhenGetDevicePropertiesAndDebugFlagEnable EXPECT_EQ(expectedIpVersion, zeDeviceIpVersion.ipVersion); } -TEST_F(DeviceTest, givenDeviceIpVersionWhenGetDevicePropertiesAndDebugIpDeviceFlagDisabledButSomeOtherExtIsEnabledThenCorrectResultIsReturned) { - DebugManagerStateRestore restore; - DebugManager.flags.EnableL0DeviceIpVersion.set(false); - DebugManager.flags.EnableL0EuCount.set(true); - - ze_device_properties_t deviceProperties = {ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES}; - ze_device_ip_version_ext_t zeDeviceIpVersion = {ZE_STRUCTURE_TYPE_DEVICE_IP_VERSION_EXT}; - zeDeviceIpVersion.ipVersion = std::numeric_limits::max(); - deviceProperties.pNext = &zeDeviceIpVersion; - - device->getProperties(&deviceProperties); - EXPECT_EQ(std::numeric_limits::max(), zeDeviceIpVersion.ipVersion); -} - TEST_F(DeviceTest, givenCallToDevicePropertiesThenMaximumMemoryToBeAllocatedIsCorrectlyReturned) { ze_device_properties_t deviceProperties = {ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES}; deviceProperties.maxMemAllocSize = 0; @@ -2147,8 +2126,6 @@ TEST_F(MultipleDevicesDisabledImplicitScalingTest, whenCallingGetMemoryPropertie } TEST_F(MultipleDevicesEnabledImplicitScalingTest, WhenRequestingZeEuCountThenExpectedEUsAreReturned) { - DebugManagerStateRestore restore; - DebugManager.flags.EnableL0EuCount.set(true); ze_device_properties_t deviceProperties = {ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES}; ze_eu_count_ext_t zeEuCountDesc = {ZE_STRUCTURE_TYPE_EU_COUNT_EXT}; deviceProperties.pNext = &zeEuCountDesc; diff --git a/level_zero/core/test/unit_tests/xe_hpc_core/pvc/test_device_pvc.cpp b/level_zero/core/test/unit_tests/xe_hpc_core/pvc/test_device_pvc.cpp index c0da1c5310..a070a18d98 100644 --- a/level_zero/core/test/unit_tests/xe_hpc_core/pvc/test_device_pvc.cpp +++ b/level_zero/core/test/unit_tests/xe_hpc_core/pvc/test_device_pvc.cpp @@ -63,9 +63,6 @@ PVCTEST_F(DeviceTestPvc, GivenPvcWhenGettingPhysicalEuSimdWidthThenReturn16) { } PVCTEST_F(DeviceTestPvc, givenPvcXlDeviceIdAndRevIdWhenGetDeviceIpVersion) { - DebugManagerStateRestore restore; - DebugManager.flags.EnableL0DeviceIpVersion.set(true); - ze_device_properties_t deviceProperties = {ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES}; ze_device_ip_version_ext_t zeDeviceIpVersion = {ZE_STRUCTURE_TYPE_DEVICE_IP_VERSION_EXT}; zeDeviceIpVersion.ipVersion = std::numeric_limits::max(); @@ -87,9 +84,6 @@ PVCTEST_F(DeviceTestPvc, givenPvcXlDeviceIdAndRevIdWhenGetDeviceIpVersion) { } PVCTEST_F(DeviceTestPvc, givenPvcXtDeviceIdAndRevIdWhenGetDeviceIpVersion) { - DebugManagerStateRestore restore; - DebugManager.flags.EnableL0DeviceIpVersion.set(true); - ze_device_properties_t deviceProperties = {ZE_STRUCTURE_TYPE_DEVICE_PROPERTIES}; ze_device_ip_version_ext_t zeDeviceIpVersion = {ZE_STRUCTURE_TYPE_DEVICE_IP_VERSION_EXT}; zeDeviceIpVersion.ipVersion = std::numeric_limits::max(); diff --git a/shared/source/debug_settings/debug_variables_base.inl b/shared/source/debug_settings/debug_variables_base.inl index 1043f8c2fc..1cf55d945c 100644 --- a/shared/source/debug_settings/debug_variables_base.inl +++ b/shared/source/debug_settings/debug_variables_base.inl @@ -345,9 +345,6 @@ DECLARE_DEBUG_VARIABLE(int32_t, DirectSubmissionRelaxedOrderingMinNumberOfClient DECLARE_DEBUG_VARIABLE(bool, DirectSubmissionPrintBuffers, false, "Print address of submitted command buffers") /*FEATURE FLAGS*/ -DECLARE_DEBUG_VARIABLE(bool, EnableL0ReadLUIDExtension, false, "Enables Support for L0 Extension for reading the LUID from WDDM.") -DECLARE_DEBUG_VARIABLE(bool, EnableL0EuCount, false, "Enables Support for L0 Extension for querying total nubmer of EUs.") -DECLARE_DEBUG_VARIABLE(bool, EnableL0DeviceIpVersion, false, "Enables support for L0 extension for querying device IP version") DECLARE_DEBUG_VARIABLE(bool, USMEvictAfterMigration, false, "Evict USM allocation after implicit migration to GPU") DECLARE_DEBUG_VARIABLE(bool, EnableNV12, true, "Enables NV12 extension") DECLARE_DEBUG_VARIABLE(bool, EnablePackedYuv, true, "Enables cl_packed_yuv extension") diff --git a/shared/test/common/test_files/igdrcl.config b/shared/test/common/test_files/igdrcl.config index ad4fbc518c..aca77a87b9 100644 --- a/shared/test/common/test_files/igdrcl.config +++ b/shared/test/common/test_files/igdrcl.config @@ -108,9 +108,6 @@ DirectSubmissionDisableCacheFlush = -1 DirectSubmissionDisableMonitorFence = -1 DirectSubmissionPrintBuffers = 0 DirectSubmissionMaxRingBuffers = -1 -EnableL0DeviceIpVersion = 0 -EnableL0ReadLUIDExtension = 0 -EnableL0EuCount = 0 USMEvictAfterMigration = 0 EnableDirectSubmissionController = -1 DirectSubmissionControllerTimeout = -1