diff options
author | Charles Giessen <charles@lunarg.com> | 2022-05-03 23:29:06 +0300 |
---|---|---|
committer | Charles Giessen <46324611+charles-lunarg@users.noreply.github.com> | 2022-05-05 00:40:27 +0300 |
commit | a9543c5ac3758bc5bd1baf6df79859c3fe936e88 (patch) | |
tree | 7da2b248a44109fbffc10846efe880a1ce578899 | |
parent | 461f53cdb5ba224a87144484c6b0f77eb636cb0d (diff) |
Make portability drivers not load by default
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.
-rw-r--r-- | loader/loader.c | 40 | ||||
-rw-r--r-- | loader/loader.h | 3 | ||||
-rw-r--r-- | loader/loader_common.h | 2 | ||||
-rw-r--r-- | loader/trampoline.c | 22 | ||||
-rw-r--r-- | tests/loader_regression_tests.cpp | 101 |
5 files changed, 69 insertions, 99 deletions
diff --git a/loader/loader.c b/loader/loader.c index c7cd3eced..cd32470e4 100644 --- a/loader/loader.c +++ b/loader/loader.c @@ -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; diff --git a/loader/loader.h b/loader/loader.h index 566692ff9..35765a3c2 100644 --- a/loader/loader.h +++ b/loader/loader.h @@ -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); diff --git a/loader/loader_common.h b/loader/loader_common.h index 4a6d6d182..6aa1c9917 100644 --- a/loader/loader_common.h +++ b/loader/loader_common.h @@ -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 { diff --git a/loader/trampoline.c b/loader/trampoline.c index 701df9581..c1a9bd18e 100644 --- a/loader/trampoline.c +++ b/loader/trampoline.c @@ -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); diff --git a/tests/loader_regression_tests.cpp b/tests/loader_regression_tests.cpp index dca30f480..5a4b4e682 100644 --- a/tests/loader_regression_tests.cpp +++ b/tests/loader_regression_tests.cpp @@ -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); } } |