From fa3b7bdefadef01ee9af593245c47ca1a33212f4 Mon Sep 17 00:00:00 2001 From: Charles Giessen Date: Fri, 12 Apr 2024 10:17:50 -0700 Subject: [PATCH] Gracefully handle error results from vkEnumeratePhysicalDevices If any driver returned non-VK_SUCCESS values (except for OOHM) when the loader calls vkEnumeratePhysicalDevices, it would give up and return immediately. This could result in perfectly functional drivers being ignored. The solution is to just skip over drivers that return non-VK_SUCCESS. --- loader/loader.c | 54 ++++++++----- loader/loader_windows.c | 8 +- tests/framework/icd/test_icd.cpp | 6 ++ tests/framework/icd/test_icd.h | 3 + tests/loader_regression_tests.cpp | 121 ++++++++++++++++++++++++++++++ 5 files changed, 170 insertions(+), 22 deletions(-) diff --git a/loader/loader.c b/loader/loader.c index bfbf515fd..93008f377 100644 --- a/loader/loader.c +++ b/loader/loader.c @@ -6160,27 +6160,47 @@ VkResult setup_loader_term_phys_devs(struct loader_instance *inst) { icd_term = inst->icd_terms; while (NULL != icd_term) { res = icd_term->dispatch.EnumeratePhysicalDevices(icd_term->instance, &icd_phys_dev_array[icd_idx].device_count, NULL); - if (VK_SUCCESS != res) { + if (VK_ERROR_OUT_OF_HOST_MEMORY == res) { loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0, - "setup_loader_term_phys_devs: Call to ICD %d's \'vkEnumeratePhysicalDevices\' failed with error 0x%08x", - icd_idx, res); + "setup_loader_term_phys_devs: Call to ICD %d's \'vkEnumeratePhysicalDevices\' failed with error code " + "VK_ERROR_OUT_OF_HOST_MEMORY", + icd_idx); goto out; - } + } else if (VK_SUCCESS == res) { + icd_phys_dev_array[icd_idx].physical_devices = + (VkPhysicalDevice *)loader_stack_alloc(icd_phys_dev_array[icd_idx].device_count * sizeof(VkPhysicalDevice)); + if (NULL == icd_phys_dev_array[icd_idx].physical_devices) { + loader_log( + inst, VULKAN_LOADER_ERROR_BIT, 0, + "setup_loader_term_phys_devs: Failed to allocate temporary ICD Physical device array for ICD %d of size %d", + icd_idx, icd_phys_dev_array[icd_idx].device_count); + res = VK_ERROR_OUT_OF_HOST_MEMORY; + goto out; + } - icd_phys_dev_array[icd_idx].physical_devices = - (VkPhysicalDevice *)loader_stack_alloc(icd_phys_dev_array[icd_idx].device_count * sizeof(VkPhysicalDevice)); - if (NULL == icd_phys_dev_array[icd_idx].physical_devices) { + res = icd_term->dispatch.EnumeratePhysicalDevices(icd_term->instance, &(icd_phys_dev_array[icd_idx].device_count), + icd_phys_dev_array[icd_idx].physical_devices); + if (VK_ERROR_OUT_OF_HOST_MEMORY == res) { + loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0, + "setup_loader_term_phys_devs: Call to ICD %d's \'vkEnumeratePhysicalDevices\' failed with error code " + "VK_ERROR_OUT_OF_HOST_MEMORY", + icd_idx); + goto out; + } + if (VK_SUCCESS != res) { + loader_log( + inst, VULKAN_LOADER_ERROR_BIT, 0, + "setup_loader_term_phys_devs: Call to ICD %d's \'vkEnumeratePhysicalDevices\' failed with error code %d", + icd_idx, res); + icd_phys_dev_array[icd_idx].device_count = 0; + icd_phys_dev_array[icd_idx].physical_devices = 0; + } + } else { loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0, - "setup_loader_term_phys_devs: Failed to allocate temporary ICD Physical device array for ICD %d of size %d", - icd_idx, icd_phys_dev_array[icd_idx].device_count); - res = VK_ERROR_OUT_OF_HOST_MEMORY; - goto out; - } - - res = icd_term->dispatch.EnumeratePhysicalDevices(icd_term->instance, &(icd_phys_dev_array[icd_idx].device_count), - icd_phys_dev_array[icd_idx].physical_devices); - if (VK_SUCCESS != res) { - goto out; + "setup_loader_term_phys_devs: Call to ICD %d's \'vkEnumeratePhysicalDevices\' failed with error code %d", + icd_idx, res); + icd_phys_dev_array[icd_idx].device_count = 0; + icd_phys_dev_array[icd_idx].physical_devices = 0; } icd_phys_dev_array[icd_idx].icd_term = icd_term; icd_phys_dev_array[icd_idx].icd_index = icd_idx; diff --git a/loader/loader_windows.c b/loader/loader_windows.c index 5fbf8d6b4..fa245cc00 100644 --- a/loader/loader_windows.c +++ b/loader/loader_windows.c @@ -802,14 +802,12 @@ VkResult enumerate_adapter_physical_devices(struct loader_instance *inst, struct VkResult res = icd_term->scanned_icd->EnumerateAdapterPhysicalDevices(icd_term->instance, luid, &count, NULL); if (res == VK_ERROR_OUT_OF_HOST_MEMORY) { return res; - } else if (res == VK_ERROR_INCOMPATIBLE_DRIVER) { + } else if (res == VK_ERROR_INCOMPATIBLE_DRIVER || res == VK_ERROR_INITIALIZATION_FAILED || 0 == count) { return VK_SUCCESS; // This driver doesn't support the adapter } else if (res != VK_SUCCESS) { loader_log(inst, VULKAN_LOADER_WARN_BIT, 0, - "Failed to convert DXGI adapter into Vulkan physical device with unexpected error code"); - return res; - } else if (0 == count) { - return VK_SUCCESS; // This driver doesn't support the adapter + "Failed to convert DXGI adapter into Vulkan physical device with unexpected error code: %d", res); + return VK_SUCCESS; } // Take a pointer to the last element of icd_phys_devs_array to simplify usage diff --git a/tests/framework/icd/test_icd.cpp b/tests/framework/icd/test_icd.cpp index a8cf1b24f..5df34ce6d 100644 --- a/tests/framework/icd/test_icd.cpp +++ b/tests/framework/icd/test_icd.cpp @@ -214,6 +214,9 @@ VKAPI_ATTR void VKAPI_CALL test_vkDestroyInstance([[maybe_unused]] VkInstance in // VK_SUCCESS,VK_INCOMPLETE VKAPI_ATTR VkResult VKAPI_CALL test_vkEnumeratePhysicalDevices([[maybe_unused]] VkInstance instance, uint32_t* pPhysicalDeviceCount, VkPhysicalDevice* pPhysicalDevices) { + if (icd.enum_physical_devices_return_code != VK_SUCCESS) { + return icd.enum_physical_devices_return_code; + } if (pPhysicalDevices == nullptr) { *pPhysicalDeviceCount = static_cast(icd.physical_devices.size()); } else { @@ -1104,6 +1107,9 @@ VKAPI_ATTR VkResult VKAPI_CALL test_vkGetCalibratedTimestampsEXT(VkDevice, uint3 VKAPI_ATTR VkResult VKAPI_CALL test_vk_icdEnumerateAdapterPhysicalDevices(VkInstance instance, LUID adapterLUID, uint32_t* pPhysicalDeviceCount, VkPhysicalDevice* pPhysicalDevices) { + if (icd.enum_adapter_physical_devices_return_code != VK_SUCCESS) { + return icd.enum_adapter_physical_devices_return_code; + } if (adapterLUID.LowPart != icd.adapterLUID.LowPart || adapterLUID.HighPart != icd.adapterLUID.HighPart) { *pPhysicalDeviceCount = 0; return VK_ERROR_INCOMPATIBLE_DRIVER; diff --git a/tests/framework/icd/test_icd.h b/tests/framework/icd/test_icd.h index 8515a8493..d9a748f7f 100644 --- a/tests/framework/icd/test_icd.h +++ b/tests/framework/icd/test_icd.h @@ -139,6 +139,9 @@ struct TestICD { VkInstanceCreateFlags passed_in_instance_create_flags{}; + BUILDER_VALUE(TestICD, VkResult, enum_physical_devices_return_code, VK_SUCCESS); + BUILDER_VALUE(TestICD, VkResult, enum_adapter_physical_devices_return_code, VK_SUCCESS); + PhysicalDevice& GetPhysDevice(VkPhysicalDevice physicalDevice) { for (auto& phys_dev : physical_devices) { if (phys_dev.vk_physical_device.handle == physicalDevice) return phys_dev; diff --git a/tests/loader_regression_tests.cpp b/tests/loader_regression_tests.cpp index 570c97262..52d59d566 100644 --- a/tests/loader_regression_tests.cpp +++ b/tests/loader_regression_tests.cpp @@ -1163,6 +1163,66 @@ TEST(EnumeratePhysicalDevices, MultipleAddRemoves) { ASSERT_GE(found_items[6], 4U); } +TEST(EnumeratePhysicalDevices, OneDriverWithWrongErrorCodes) { + FrameworkEnvironment env; + env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2)); + + InstWrapper inst{env.vulkan_functions}; + inst.create_info.set_api_version(VK_API_VERSION_1_1); + inst.CheckCreate(); + + { + env.get_test_icd().set_enum_physical_devices_return_code(VK_ERROR_INITIALIZATION_FAILED); + uint32_t returned_physical_count = 0; + EXPECT_EQ(VK_ERROR_INITIALIZATION_FAILED, + env.vulkan_functions.vkEnumeratePhysicalDevices(inst.inst, &returned_physical_count, nullptr)); + EXPECT_EQ(returned_physical_count, 0); + } + { + env.get_test_icd().set_enum_physical_devices_return_code(VK_ERROR_INCOMPATIBLE_DRIVER); + uint32_t returned_physical_count = 0; + EXPECT_EQ(VK_ERROR_INITIALIZATION_FAILED, + env.vulkan_functions.vkEnumeratePhysicalDevices(inst.inst, &returned_physical_count, nullptr)); + EXPECT_EQ(returned_physical_count, 0); + } + { + env.get_test_icd().set_enum_physical_devices_return_code(VK_ERROR_SURFACE_LOST_KHR); + uint32_t returned_physical_count = 0; + EXPECT_EQ(VK_ERROR_INITIALIZATION_FAILED, + env.vulkan_functions.vkEnumeratePhysicalDevices(inst.inst, &returned_physical_count, nullptr)); + EXPECT_EQ(returned_physical_count, 0); + } +} + +TEST(EnumeratePhysicalDevices, TwoDriversOneWithWrongErrorCodes) { + FrameworkEnvironment env; + env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2)).add_physical_device({}); + TestICD& icd1 = env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2)); + + InstWrapper inst{env.vulkan_functions}; + inst.create_info.set_api_version(VK_API_VERSION_1_1); + inst.CheckCreate(); + + { + icd1.set_enum_physical_devices_return_code(VK_ERROR_INITIALIZATION_FAILED); + uint32_t returned_physical_count = 0; + EXPECT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumeratePhysicalDevices(inst.inst, &returned_physical_count, nullptr)); + EXPECT_EQ(returned_physical_count, 1); + } + { + icd1.set_enum_physical_devices_return_code(VK_ERROR_INCOMPATIBLE_DRIVER); + uint32_t returned_physical_count = 0; + EXPECT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumeratePhysicalDevices(inst.inst, &returned_physical_count, nullptr)); + EXPECT_EQ(returned_physical_count, 1); + } + { + icd1.set_enum_physical_devices_return_code(VK_ERROR_SURFACE_LOST_KHR); + uint32_t returned_physical_count = 0; + EXPECT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumeratePhysicalDevices(inst.inst, &returned_physical_count, nullptr)); + EXPECT_EQ(returned_physical_count, 1); + } +} + TEST(CreateDevice, ExtensionNotPresent) { FrameworkEnvironment env{}; env.add_icd(TestICDDetails(TEST_ICD_PATH_VERSION_2)).add_physical_device("physical_device_0"); @@ -4363,4 +4423,65 @@ TEST(EnumerateAdapterPhysicalDevices, SameAdapterLUID_same_order) { env.vulkan_functions.vkGetPhysicalDeviceProperties2(physical_device_handles[2], &props2); EXPECT_EQ(layered_driver_properties_msft.underlyingAPI, VK_LAYERED_DRIVER_UNDERLYING_API_NONE_MSFT); } + +TEST(EnumerateAdapterPhysicalDevices, WrongErrorCodes) { + FrameworkEnvironment env; + + add_dxgi_adapter(env, "physical_device_0", LUID{10, 100}, 2); + InstWrapper inst{env.vulkan_functions}; + inst.create_info.setup_WSI().set_api_version(VK_API_VERSION_1_1); + inst.CheckCreate(); + // TestICD only fails in EnumAdapters, so shouldn't fail to query VkPhysicalDevices + { + env.get_test_icd().set_enum_adapter_physical_devices_return_code(VK_ERROR_INITIALIZATION_FAILED); + uint32_t returned_physical_count = 0; + EXPECT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumeratePhysicalDevices(inst.inst, &returned_physical_count, nullptr)); + EXPECT_EQ(returned_physical_count, 1); + } + { + env.get_test_icd().set_enum_adapter_physical_devices_return_code(VK_ERROR_INCOMPATIBLE_DRIVER); + uint32_t returned_physical_count = 0; + EXPECT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumeratePhysicalDevices(inst.inst, &returned_physical_count, nullptr)); + EXPECT_EQ(returned_physical_count, 1); + } + { + env.get_test_icd().set_enum_adapter_physical_devices_return_code(VK_ERROR_SURFACE_LOST_KHR); + uint32_t returned_physical_count = 0; + EXPECT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumeratePhysicalDevices(inst.inst, &returned_physical_count, nullptr)); + EXPECT_EQ(returned_physical_count, 1); + } + + // TestICD fails in EnumPhysDevs, should return VK_ERROR_INCOMPATIBLE_DRIVER + auto check_icds = [&env, &inst] { + env.get_test_icd().set_enum_adapter_physical_devices_return_code(VK_ERROR_INITIALIZATION_FAILED); + uint32_t returned_physical_count = 0; + EXPECT_EQ(VK_ERROR_INITIALIZATION_FAILED, + env.vulkan_functions.vkEnumeratePhysicalDevices(inst.inst, &returned_physical_count, nullptr)); + EXPECT_EQ(returned_physical_count, 0); + + env.get_test_icd().set_enum_adapter_physical_devices_return_code(VK_ERROR_INCOMPATIBLE_DRIVER); + returned_physical_count = 0; + EXPECT_EQ(VK_ERROR_INITIALIZATION_FAILED, + env.vulkan_functions.vkEnumeratePhysicalDevices(inst.inst, &returned_physical_count, nullptr)); + EXPECT_EQ(returned_physical_count, 0); + + env.get_test_icd().set_enum_adapter_physical_devices_return_code(VK_ERROR_SURFACE_LOST_KHR); + returned_physical_count = 0; + EXPECT_EQ(VK_ERROR_INITIALIZATION_FAILED, + env.vulkan_functions.vkEnumeratePhysicalDevices(inst.inst, &returned_physical_count, nullptr)); + EXPECT_EQ(returned_physical_count, 0); + }; + + // TestICD fails in EnumPhysDevs, should return VK_ERROR_INCOMPATIBLE_DRIVER + env.get_test_icd().set_enum_physical_devices_return_code(VK_ERROR_INITIALIZATION_FAILED); + check_icds(); + + // TestICD fails in EnumPhysDevs, should return VK_ERROR_INCOMPATIBLE_DRIVER + env.get_test_icd().set_enum_physical_devices_return_code(VK_ERROR_INCOMPATIBLE_DRIVER); + check_icds(); + + // TestICD fails in EnumPhysDevs, should return VK_ERROR_INCOMPATIBLE_DRIVER + env.get_test_icd().set_enum_physical_devices_return_code(VK_ERROR_SURFACE_LOST_KHR); + check_icds(); +} #endif // defined(WIN32)