Skip to content

Commit

Permalink
Gracefully handle error results from vkEnumeratePhysicalDevices
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@charles-lunarg
charles-lunarg committed Apr 24, 2024
1 parent e94cd2e commit fa3b7bd
Show file tree
Hide file tree
Showing 5 changed files with 170 additions and 22 deletions.
54 changes: 37 additions & 17 deletions loader/loader.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 3 additions & 5 deletions loader/loader_windows.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 6 additions & 0 deletions tests/framework/icd/test_icd.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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;
Expand Down
3 changes: 3 additions & 0 deletions tests/framework/icd/test_icd.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
121 changes: 121 additions & 0 deletions tests/loader_regression_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -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)

0 comments on commit fa3b7bd

Please sign in to comment.