Make portability drivers not load by default
authorCharles Giessen <charles@lunarg.com>
Tue, 3 May 2022 20:29:06 +0000 (14:29 -0600)
committerCharles Giessen <46324611+charles-lunarg@users.noreply.github.com>
Wed, 4 May 2022 21:40:27 +0000 (15:40 -0600)
Unless the portability enumeration extension is enabled and the create instance flags
contain VK_INSTANCE_CREATE_ENUMERATE_PORTABILITY_BIT_KHR, do not enumerate drivers that
contain `is_portability_driver` in their JSON Manifest. This is phase 2 of the release
for the VK_KHR_portability_enumeration extension.

An error message will be printed when no drivers were reported but there was a
portability driver which was skipped over.

loader/loader.c
loader/loader.h
loader/loader_common.h
loader/trampoline.c
tests/loader_regression_tests.cpp

index c7cd3eceddaa16c54312e31556902b4ee5c5c28c..cd32470e4e63086f6264467c6e896a89ccadda2f 100644 (file)
@@ -1372,8 +1372,7 @@ static VkResult loader_scanned_icd_init(const struct loader_instance *inst, stru
 }
 
 static VkResult loader_scanned_icd_add(const struct loader_instance *inst, struct loader_icd_tramp_list *icd_tramp_list,
-                                       const char *filename, uint32_t api_version, bool is_portability_driver,
-                                       enum loader_layer_library_status *lib_status) {
+                                       const char *filename, uint32_t api_version, enum loader_layer_library_status *lib_status) {
     loader_platform_dl_handle handle;
     PFN_vkCreateInstance fp_create_inst;
     PFN_vkEnumerateInstanceExtensionProperties fp_get_inst_ext_props;
@@ -1513,7 +1512,6 @@ static VkResult loader_scanned_icd_add(const struct loader_instance *inst, struc
     new_scanned_icd->EnumerateAdapterPhysicalDevices = fp_enum_dxgi_adapter_phys_devs;
 #endif
     new_scanned_icd->interface_version = interface_vers;
-    new_scanned_icd->portability_driver = is_portability_driver;
 
     new_scanned_icd->lib_name = (char *)loader_instance_heap_alloc(inst, strlen(filename) + 1, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE);
     if (NULL == new_scanned_icd->lib_name) {
@@ -1569,7 +1567,7 @@ void loader_preload_icds(void) {
     }
 
     memset(&scanned_icds, 0, sizeof(scanned_icds));
-    VkResult result = loader_icd_scan(NULL, &scanned_icds);
+    VkResult result = loader_icd_scan(NULL, &scanned_icds, NULL);
     if (result != VK_SUCCESS) {
         loader_scanned_icd_clear(NULL, &scanned_icds);
     }
@@ -3360,10 +3358,15 @@ void loader_destroy_icd_lib_list() {}
 // VK ICDs manifest files.
 // From these manifest files it finds the ICD libraries.
 //
+// skipped_portability_drivers is used to report whether the loader found drivers which report
+// portability but the application didn't enable the bit to enumerate them
+// Can be NULL
+//
 // \returns
 // Vulkan result
 // (on result == VK_SUCCESS) a list of icds that were discovered
-VkResult loader_icd_scan(const struct loader_instance *inst, struct loader_icd_tramp_list *icd_tramp_list) {
+VkResult loader_icd_scan(const struct loader_instance *inst, struct loader_icd_tramp_list *icd_tramp_list,
+                         bool *skipped_portability_drivers) {
     char *file_str;
     loader_api_version json_file_version = {0, 0, 0};
     struct loader_data_files manifest_files;
@@ -3545,18 +3548,19 @@ VkResult loader_icd_scan(const struct loader_instance *inst, struct loader_icd_t
                     json = NULL;
                     continue;
                 }
-                bool portability_driver = false;
+                // Skip over ICD's which contain a true "is_portability_driver" value whenever the application doesn't enable
+                // portability enumeration.
                 item = cJSON_GetObjectItem(itemICD, "is_portability_driver");
-                if (item != NULL && item->type == cJSON_True) {
-                    portability_driver = true;
-                    // TODO: skip over the driver if the is_portability_driver field is present and true but the portability
-                    // enumeration extension is present. Then emit an error if no drivers are present but a portability driver
-                    // was skipped.
+                if (item != NULL && item->type == cJSON_True && !inst->portability_enumeration_enabled) {
+                    if (skipped_portability_drivers) *skipped_portability_drivers = true;
+                    cJSON_Delete(inst, json);
+                    json = NULL;
+                    continue;
                 }
 
                 VkResult icd_add_res = VK_SUCCESS;
                 enum loader_layer_library_status lib_status;
-                icd_add_res = loader_scanned_icd_add(inst, icd_tramp_list, fullpath, vers, portability_driver, &lib_status);
+                icd_add_res = loader_scanned_icd_add(inst, icd_tramp_list, fullpath, vers, &lib_status);
                 if (VK_ERROR_OUT_OF_HOST_MEMORY == icd_add_res) {
                     res = icd_add_res;
                     goto out;
@@ -5434,16 +5438,6 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateDevice(VkPhysicalDevice physical
 
     icd_exts.list = NULL;
 
-    // Check if the driver the VkPhysicalDevice comes from is a portability driver and emit a warning if the
-    // VK_INSTANCE_CREATE_ENUMERATE_PORTABILITY_BIT_KHR bit isn't set
-    if (icd_term->scanned_icd->portability_driver && !icd_term->this_instance->portability_enumeration_enabled) {
-        loader_log(icd_term->this_instance, VULKAN_LOADER_ERROR_BIT | VULKAN_LOADER_DRIVER_BIT, 0,
-                   "vkCreateDevice: Attempting to create a VkDevice from a VkPhysicalDevice which is from a portability driver "
-                   "without the VK_INSTANCE_CREATE_ENUMERATE_PORTABILITY_BIT_KHR bit in the VkInstanceCreateInfo flags being set "
-                   "and the VK_KHR_portability_enumeration extension enabled. In future versions of the loader this "
-                   "VkPhysicalDevice will not be enumerated.");
-    }
-
     if (fpCreateDevice == NULL) {
         loader_log(icd_term->this_instance, VULKAN_LOADER_ERROR_BIT | VULKAN_LOADER_DRIVER_BIT, 0,
                    "terminator_CreateDevice: No vkCreateDevice command exposed by ICD %s", icd_term->scanned_icd->lib_name);
@@ -6464,7 +6458,7 @@ terminator_EnumerateInstanceExtensionProperties(const VkEnumerateInstanceExtensi
         loader_preload_icds();
 
         // Scan/discover all ICD libraries
-        res = loader_icd_scan(NULL, &icd_tramp_list);
+        res = loader_icd_scan(NULL, &icd_tramp_list, NULL);
         // EnumerateInstanceExtensionProperties can't return anything other than OOM or VK_ERROR_LAYER_NOT_PRESENT
         if ((VK_SUCCESS != res && icd_tramp_list.count > 0) || res == VK_ERROR_OUT_OF_HOST_MEMORY) {
             goto out;
index 566692ff9cebff57e316c158929ac602354c588d..35765a3c257a9fcf846c30a4316050ae5873aab7 100644 (file)
@@ -115,7 +115,8 @@ VkResult loader_add_layer_name_to_list(const struct loader_instance *inst, const
                                        const struct loader_layer_list *source_list, struct loader_layer_list *target_list,
                                        struct loader_layer_list *expanded_target_list);
 void loader_scanned_icd_clear(const struct loader_instance *inst, struct loader_icd_tramp_list *icd_tramp_list);
-VkResult loader_icd_scan(const struct loader_instance *inst, struct loader_icd_tramp_list *icd_tramp_list);
+VkResult loader_icd_scan(const struct loader_instance *inst, struct loader_icd_tramp_list *icd_tramp_list,
+                         bool *skipped_portability_drivers);
 void loader_scan_for_layers(struct loader_instance *inst, struct loader_layer_list *instance_layers);
 void loader_scan_for_implicit_layers(struct loader_instance *inst, struct loader_layer_list *instance_layers);
 bool loader_implicit_layer_is_enabled(const struct loader_instance *inst, const struct loader_layer_properties *prop);
index 4a6d6d182f522e8670e67abab8af5d534f8981f4..6aa1c99171e3a260ef5a9ebab4050b968f3b2457 100644 (file)
@@ -427,8 +427,6 @@ struct loader_scanned_icd {
 #if defined(VK_USE_PLATFORM_WIN32_KHR)
     PFN_vk_icdEnumerateAdapterPhysicalDevices EnumerateAdapterPhysicalDevices;
 #endif
-    // whether the device is a portability driver
-    bool portability_driver;
 };
 
 enum loader_data_files_type {
index 701df958101433d43353726267b04feeed673404..c1a9bd18e66d51857f23f3b6e3862241b9e2e775 100644 (file)
@@ -563,20 +563,24 @@ LOADER_EXPORT VKAPI_ATTR VkResult VKAPI_CALL vkCreateInstance(const VkInstanceCr
 
     // Scan/discover all ICD libraries
     memset(&ptr_instance->icd_tramp_list, 0, sizeof(ptr_instance->icd_tramp_list));
-    res = loader_icd_scan(ptr_instance, &ptr_instance->icd_tramp_list);
-    if (res == VK_SUCCESS && ptr_instance->icd_tramp_list.count == 0) {
+    bool skipped_portability_drivers = false;
+    res = loader_icd_scan(ptr_instance, &ptr_instance->icd_tramp_list, &skipped_portability_drivers);
+    if (res == VK_ERROR_OUT_OF_HOST_MEMORY) {
+        goto out;
+    } else if (ptr_instance->icd_tramp_list.count == 0) {
         // No drivers found
+        if (skipped_portability_drivers) {
+            loader_log(
+                ptr_instance, VULKAN_LOADER_ERROR_BIT | VULKAN_LOADER_DRIVER_BIT, 0,
+                "vkCreateInstance: Found drivers that contain devices which support the portability subset, but the "
+                "portability enumeration bit was not set!. Applications that wish to enumerate portability drivers must set the "
+                "VK_INSTANCE_CREATE_ENUMERATE_PORTABILITY_BIT_KHR bit in the VkInstanceCreateInfo flags and"
+                "enable the VK_KHR_portability_enumeration instance extension.");
+        }
         loader_log(ptr_instance, VULKAN_LOADER_ERROR_BIT | VULKAN_LOADER_DRIVER_BIT, 0, "vkCreateInstance: Found no drivers!");
         res = VK_ERROR_INCOMPATIBLE_DRIVER;
         goto out;
     }
-    if (res != VK_SUCCESS) {
-        if (res != VK_ERROR_OUT_OF_HOST_MEMORY && ptr_instance->icd_tramp_list.count == 0) {
-            loader_log(ptr_instance, VULKAN_LOADER_ERROR_BIT | VULKAN_LOADER_DRIVER_BIT, 0, "vkCreateInstance: Found no drivers!");
-            res = VK_ERROR_INCOMPATIBLE_DRIVER;
-        }
-        goto out;
-    }
 
     // Get extensions from all ICD's, merge so no duplicates, then validate
     res = loader_get_icd_loader_instance_extensions(ptr_instance, &ptr_instance->icd_tramp_list, &ptr_instance->ext_list);
index dca30f4800d4d9baa3cb463bd27f5da520eb65d3..5a4b4e6828983c5133b4a2fe92bed1ab59995f1c 100644 (file)
@@ -3116,10 +3116,10 @@ TEST(SortedPhysicalDevices, DeviceGroupsSortedDisabled) {
 #endif  // __linux__ || __FreeBSD__
 
 const char* portability_driver_warning =
-    "vkCreateDevice: Attempting to create a VkDevice from a VkPhysicalDevice which is from a portability driver "
-    "without the VK_INSTANCE_CREATE_ENUMERATE_PORTABILITY_BIT_KHR bit in the VkInstanceCreateInfo flags being set "
-    "and the VK_KHR_portability_enumeration extension enabled. In future versions of the loader this "
-    "VkPhysicalDevice will not be enumerated.";
+    "vkCreateInstance: Found drivers that contain devices which support the portability subset, but the "
+    "portability enumeration bit was not set!. Applications that wish to enumerate portability drivers must set the "
+    "VK_INSTANCE_CREATE_ENUMERATE_PORTABILITY_BIT_KHR bit in the VkInstanceCreateInfo flags and"
+    "enable the VK_KHR_portability_enumeration instance extension.";
 
 TEST(PortabilityICDConfiguration, PortabilityICDOnly) {
     FrameworkEnvironment env{};
@@ -3129,14 +3129,15 @@ TEST(PortabilityICDConfiguration, PortabilityICDOnly) {
     auto& driver = env.get_test_icd();
     driver.physical_devices.emplace_back("physical_device_0");
     driver.max_icd_interface_version = 1;
-    // TODO - Fix tests when portability devices are not longer enumerated by default
     {  // enable portability extension and flag
         InstWrapper inst{env.vulkan_functions};
         inst.create_info.add_extension(VK_EXT_DEBUG_UTILS_EXTENSION_NAME);
         inst.create_info.add_extension("VK_KHR_portability_enumeration");
         inst.create_info.flags = VK_INSTANCE_CREATE_ENUMERATE_PORTABILITY_BIT_KHR;
-
+        FillDebugUtilsCreateDetails(inst.create_info, env.debug_log);
         inst.CheckCreate();
+        ASSERT_FALSE(env.debug_log.find(portability_driver_warning));
+
         DebugUtilsWrapper log{inst, VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT};
         CreateDebugUtilsMessenger(log);
 
@@ -3151,46 +3152,25 @@ TEST(PortabilityICDConfiguration, PortabilityICDOnly) {
         InstWrapper inst{env.vulkan_functions};
         inst.create_info.flags = VK_INSTANCE_CREATE_ENUMERATE_PORTABILITY_BIT_KHR;
         inst.create_info.add_extension(VK_EXT_DEBUG_UTILS_EXTENSION_NAME);
-        inst.CheckCreate();
-        DebugUtilsWrapper log{inst, VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT};
-        CreateDebugUtilsMessenger(log);
-
-        auto phys_dev = inst.GetPhysDev();
-        handle_assert_has_value(phys_dev);
-
-        DeviceWrapper dev_info{inst};
-        dev_info.CheckCreate(phys_dev);
-        ASSERT_TRUE(log.find(portability_driver_warning));
+        FillDebugUtilsCreateDetails(inst.create_info, env.debug_log);
+        inst.CheckCreate(VK_ERROR_INCOMPATIBLE_DRIVER);
+        ASSERT_TRUE(env.debug_log.find(portability_driver_warning));
     }
     {  // enable portability extension but not flag - shouldn't be able to create an instance when filtering is enabled
         InstWrapper inst{env.vulkan_functions};
         inst.create_info.add_extension("VK_KHR_portability_enumeration");
         inst.create_info.add_extension(VK_EXT_DEBUG_UTILS_EXTENSION_NAME);
-        inst.CheckCreate();
-        DebugUtilsWrapper log{inst, VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT};
-        CreateDebugUtilsMessenger(log);
-
-        auto phys_dev = inst.GetPhysDev();
-        handle_assert_has_value(phys_dev);
-
-        DeviceWrapper dev_info{inst};
-        dev_info.CheckCreate(phys_dev);
-        ASSERT_TRUE(log.find(portability_driver_warning));
+        FillDebugUtilsCreateDetails(inst.create_info, env.debug_log);
+        inst.CheckCreate(VK_ERROR_INCOMPATIBLE_DRIVER);
+        ASSERT_TRUE(env.debug_log.find(portability_driver_warning));
     }
     {  // enable neither the portability extension or the flag - shouldn't be able to create an instance when filtering is enabled
         InstWrapper inst{env.vulkan_functions};
         inst.create_info.flags = 0;  // make sure its 0 - no portability
         inst.create_info.add_extension(VK_EXT_DEBUG_UTILS_EXTENSION_NAME);
-        inst.CheckCreate();
-        DebugUtilsWrapper log{inst, VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT};
-        CreateDebugUtilsMessenger(log);
-
-        auto phys_dev = inst.GetPhysDev();
-        handle_assert_has_value(phys_dev);
-
-        DeviceWrapper dev_info{inst};
-        dev_info.CheckCreate(phys_dev);
-        ASSERT_TRUE(log.find(portability_driver_warning));
+        FillDebugUtilsCreateDetails(inst.create_info, env.debug_log);
+        inst.CheckCreate(VK_ERROR_INCOMPATIBLE_DRIVER);
+        ASSERT_TRUE(env.debug_log.find(portability_driver_warning));
     }
 }
 
@@ -3208,13 +3188,15 @@ TEST(PortabilityICDConfiguration, PortabilityAndRegularICD) {
 
     driver1.physical_devices.emplace_back("portability_physical_device_1");
     driver1.max_icd_interface_version = 1;
-    // TODO - Fix tests when portability devices are not longer enumerated by default
     {  // enable portability extension and flag
         InstWrapper inst{env.vulkan_functions};
         inst.create_info.add_extension(VK_EXT_DEBUG_UTILS_EXTENSION_NAME);
         inst.create_info.add_extension("VK_KHR_portability_enumeration");
         inst.create_info.flags = VK_INSTANCE_CREATE_ENUMERATE_PORTABILITY_BIT_KHR;
+        FillDebugUtilsCreateDetails(inst.create_info, env.debug_log);
         inst.CheckCreate();
+        ASSERT_FALSE(env.debug_log.find(portability_driver_warning));
+
         DebugUtilsWrapper log{inst, VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT};
         CreateDebugUtilsMessenger(log);
 
@@ -3226,58 +3208,49 @@ TEST(PortabilityICDConfiguration, PortabilityAndRegularICD) {
         DeviceWrapper dev_info_1{inst};
         dev_info_0.CheckCreate(phys_devs[0]);
         dev_info_1.CheckCreate(phys_devs[1]);
-        ASSERT_FALSE(log.find(portability_driver_warning));
     }
     {  // enable portability extension but not flag - should only enumerate 1 physical device when filtering is enabled
         InstWrapper inst{env.vulkan_functions};
         inst.create_info.add_extension(VK_EXT_DEBUG_UTILS_EXTENSION_NAME);
         inst.create_info.add_extension("VK_KHR_portability_enumeration");
+        FillDebugUtilsCreateDetails(inst.create_info, env.debug_log);
         inst.CheckCreate();
+        ASSERT_FALSE(env.debug_log.find(portability_driver_warning));
+
         DebugUtilsWrapper log{inst, VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT};
         CreateDebugUtilsMessenger(log);
-
-        auto phys_devs = inst.GetPhysDevs(2);
-        for (const auto& phys_dev : phys_devs) {
-            handle_assert_has_value(phys_dev);
-        }
+        auto phys_dev = inst.GetPhysDev();
+        handle_assert_has_value(phys_dev);
         DeviceWrapper dev_info_0{inst};
-        DeviceWrapper dev_info_1{inst};
-        dev_info_0.CheckCreate(phys_devs[0]);
-        dev_info_1.CheckCreate(phys_devs[1]);
-        ASSERT_TRUE(log.find(portability_driver_warning));
+        dev_info_0.CheckCreate(phys_dev);
     }
     {  // enable portability flag but not extension - should only enumerate 1 physical device when filtering is enabled
         InstWrapper inst{env.vulkan_functions};
         inst.create_info.add_extension(VK_EXT_DEBUG_UTILS_EXTENSION_NAME);
         inst.create_info.flags = VK_INSTANCE_CREATE_ENUMERATE_PORTABILITY_BIT_KHR;
+        FillDebugUtilsCreateDetails(inst.create_info, env.debug_log);
         inst.CheckCreate();
+        ASSERT_FALSE(env.debug_log.find(portability_driver_warning));
+
         DebugUtilsWrapper log{inst, VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT};
         CreateDebugUtilsMessenger(log);
-
-        auto phys_devs = inst.GetPhysDevs(2);
-        for (const auto& phys_dev : phys_devs) {
-            handle_assert_has_value(phys_dev);
-        }
+        auto phys_dev = inst.GetPhysDev();
+        handle_assert_has_value(phys_dev);
         DeviceWrapper dev_info_0{inst};
-        DeviceWrapper dev_info_1{inst};
-        dev_info_0.CheckCreate(phys_devs[0]);
-        dev_info_1.CheckCreate(phys_devs[1]);
-        ASSERT_TRUE(log.find(portability_driver_warning));
+        dev_info_0.CheckCreate(phys_dev);
     }
     {  // do not enable portability extension or flag - should only enumerate 1 physical device when filtering is enabled
         InstWrapper inst{env.vulkan_functions};
         inst.create_info.add_extension(VK_EXT_DEBUG_UTILS_EXTENSION_NAME);
+        FillDebugUtilsCreateDetails(inst.create_info, env.debug_log);
         inst.CheckCreate();
+        ASSERT_FALSE(env.debug_log.find(portability_driver_warning));
+
         DebugUtilsWrapper log{inst, VK_DEBUG_UTILS_MESSAGE_SEVERITY_ERROR_BIT_EXT};
         CreateDebugUtilsMessenger(log);
-        auto phys_devs = inst.GetPhysDevs(2);
-        for (const auto& phys_dev : phys_devs) {
-            handle_assert_has_value(phys_dev);
-        }
+        auto phys_dev = inst.GetPhysDev();
+        handle_assert_has_value(phys_dev);
         DeviceWrapper dev_info_0{inst};
-        DeviceWrapper dev_info_1{inst};
-        dev_info_0.CheckCreate(phys_devs[0]);
-        dev_info_1.CheckCreate(phys_devs[1]);
-        ASSERT_TRUE(log.find(portability_driver_warning));
+        dev_info_0.CheckCreate(phys_dev);
     }
 }