Gracefully handle error results from vkEnumeratePhysicalDevices
authorCharles Giessen <charles@lunarg.com>
Fri, 12 Apr 2024 17:17:50 +0000 (10:17 -0700)
committerCharles Giessen <46324611+charles-lunarg@users.noreply.github.com>
Wed, 24 Apr 2024 21:31:21 +0000 (15:31 -0600)
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
loader/loader_windows.c
tests/framework/icd/test_icd.cpp
tests/framework/icd/test_icd.h
tests/loader_regression_tests.cpp

index bfbf515fd9204a179933de71ae9a5514f11fe505..93008f3773004bab46fff665dffe2fa21d3f9416 100644 (file)
@@ -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;
index 5fbf8d6b42ef6a820b5b773eb1ee01b54c52a5f8..fa245cc00035cb0b13436ba97c9f5b4dd805bb48 100644 (file)
@@ -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
index a8cf1b24f560cbcee7bdac8c67cd0d8f719a3f02..5df34ce6dcfce269c93f118ba4478d92cf39bf7a 100644 (file)
@@ -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<uint32_t>(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;
index 8515a849390361efd34a9db4b20073de2caea960..d9a748f7fc945a7021ca0c593982860d02550612 100644 (file)
@@ -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;
index 570c9726245a0c9601b2e85750adf23e90dac71a..52d59d566c90801d359ce58a6ec0d5cba5ee3004 100644 (file)
@@ -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)