Skip to content

loader: apply settings device configurations to device groups #2011

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Wint3rNight wants to merge 1 commit into
base: main
Choose a base branch
from
+216 −3
Open

loader: apply settings device configurations to device groups #2011

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 61 additions & 3 deletions loader/loader.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7207,7 +7207,7 @@ typedef struct physical_device_configuration_details {
VkResult loader_apply_settings_device_configurations(struct loader_instance *inst, uint32_t *pPhysicalDeviceCount,
VkPhysicalDevice *pPhysicalDevices) {
loader_log(inst, VULKAN_LOADER_INFO_BIT, 0,
"Reordering the output of vkEnumeratePhysicalDevices to match the loader settings device configurations list");
"Selecting and ordering VkPhysicalDevices to match the loader settings device configurations list");

physical_device_configuration_details *pd_details =
loader_stack_alloc(inst->phys_dev_count_term * sizeof(physical_device_configuration_details));
Expand Down Expand Up @@ -7724,6 +7724,10 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumeratePhysicalDeviceGroups(
VkResult res = VK_SUCCESS;
struct loader_icd_term *icd_term;
uint32_t total_count = 0;
// The number of groups actually written into new_phys_dev_groups. This is
// less than total_count when groups are skipped, which happens when the
// settings file hides a device that a group contains.
uint32_t new_group_count = 0;
uint32_t cur_icd_group_count = 0;
VkPhysicalDeviceGroupProperties **new_phys_dev_groups = NULL;
struct loader_physical_device_group_term *local_phys_dev_groups = NULL;
Expand Down Expand Up @@ -7978,11 +7982,61 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumeratePhysicalDeviceGroups(
}
}

// Apply the settings file's device_configurations to the groups. The plain
// vkEnumeratePhysicalDevices path does this in terminator_EnumeratePhysicalDevices;
// without the same treatment here an application can reach a VkPhysicalDevice
// through a group that the settings file meant to hide.
if (inst->settings.settings_active && inst->settings.device_configurations_active && NULL != inst->phys_devs_term) {
uint32_t visible_count = inst->phys_dev_count_term;
VkPhysicalDevice *visible_phys_devs = loader_stack_alloc(visible_count * sizeof(VkPhysicalDevice));
if (NULL == visible_phys_devs) {
res = VK_ERROR_OUT_OF_HOST_MEMORY;
goto out;
}

// Reuse the same matching the non-group path uses, so the two can't drift apart.
VkResult settings_res = loader_apply_settings_device_configurations(inst, &visible_count, visible_phys_devs);
if (VK_SUCCESS != settings_res) {
res = settings_res;
goto out;
}

for (uint32_t group = 0; group < total_count; group++) {
bool group_fully_visible = true;
for (uint32_t group_gpu = 0; group_gpu < local_phys_dev_groups[group].group_props.physicalDeviceCount;
group_gpu++) {
bool found = false;
for (uint32_t vis = 0; vis < visible_count; vis++) {
if (local_phys_dev_groups[group].group_props.physicalDevices[group_gpu] == visible_phys_devs[vis]) {
found = true;
break;
}
}
if (!found) {
group_fully_visible = false;
break;
}
}

if (!group_fully_visible) {
// Drop the whole group rather than removing the hidden device from it.
// The devices in a group are physically linked, so a group that has had
// a member removed no longer describes the hardware it claims to.
loader_log(inst, VULKAN_LOADER_INFO_BIT, 0,
"terminator_EnumeratePhysicalDeviceGroups: Physical device group %d contains a VkPhysicalDevice "
"which the settings file device configurations exclude, so the group was not reported.",
group);
local_phys_dev_groups[group].group_props.physicalDeviceCount = 0;
}
}
}

uint32_t idx = 0;

// Copy or create everything to fill the new array of physical device groups
for (uint32_t group = 0; group < total_count; group++) {
// Skip groups which have been included through sorting
// Skip groups which have been included through sorting, and groups the
// settings file excluded above.
if (local_phys_dev_groups[group].group_props.physicalDeviceCount == 0) {
continue;
}
Expand Down Expand Up @@ -8036,6 +8090,10 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumeratePhysicalDeviceGroups(

++idx;
}

// Only idx entries were written; the rest of new_phys_dev_groups is still NULL
// from the calloc above, so the count must reflect what was actually filled in.
new_group_count = idx;
}

out:
Expand Down Expand Up @@ -8086,7 +8144,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumeratePhysicalDeviceGroups(
}

// Swap in the new physical device group list
inst->phys_dev_group_count_term = total_count;
inst->phys_dev_group_count_term = new_group_count;
inst->phys_dev_groups_term = new_phys_dev_groups;
}

Expand Down
155 changes: 155 additions & 0 deletions tests/loader_settings_tests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3203,6 +3203,161 @@ TEST(SettingsFile, InvalidAdditionalDriversField) {
EXPECT_TRUE(string_eq(active_layer_props.at(0).layerName, layer_name));
}

// Reproduces #1915: the settings file's device_configurations list restricts
// what vkEnumeratePhysicalDevices reports, but vkEnumeratePhysicalDeviceGroups
// ignores it entirely, so an application can still reach a hidden device.
//
// Singleton groups only -- one group per physical device. There is no
// ambiguity in that case: a device the settings file hides must not be
// reachable through any group.
TEST(SettingsFile, DeviceConfigurationAppliesToPhysicalDeviceGroups) {
FrameworkEnvironment env{};
std::vector<VulkanUUID> uuids{2, VulkanUUID{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}};

// Mix up the uuid's so that they are all unique
int count = 1;
for (auto& uuid : uuids) {
std::rotate(uuid.begin(), uuid.begin() + count, uuid.end());
count++;
}

auto& icd = env.add_icd(TEST_ICD_PATH_VERSION_2).set_icd_api_version(VK_API_VERSION_1_1);
icd.add_physical_device(
PhysicalDevice().set_deviceName("visible").set_api_version(VK_API_VERSION_1_1).set_deviceUUID(uuids[0]));
icd.add_physical_device(PhysicalDevice().set_deviceName("hidden").set_api_version(VK_API_VERSION_1_1).set_deviceUUID(uuids[1]));

icd.physical_device_groups.emplace_back(0);
icd.physical_device_groups.emplace_back(1);

// The settings file lists only the first device, so the second is hidden.
env.loader_settings.set_file_format_version({1, 0, 0}).add_app_specific_setting(AppSpecificSettings{});
env.loader_settings.app_specific_settings.at(0).add_device_configuration(
LoaderSettingsDeviceConfiguration{}.set_deviceUUID(uuids[0]));
env.update_loader_settings(env.loader_settings);

InstWrapper inst{env.vulkan_functions};
inst.CheckCreate();

// The non-group path already honours the settings file.
auto pds = inst.GetPhysDevs();
ASSERT_EQ(pds.size(), 1U);

// The group path must agree. The count query is an upper bound -- the
// non-group path estimates its count the same way -- so what matters is
// that the groups actually written out exclude the hidden device.
uint32_t group_count = 0;
ASSERT_EQ(VK_SUCCESS, inst->vkEnumeratePhysicalDeviceGroups(inst, &group_count, nullptr));
ASSERT_GE(group_count, 1U);

std::vector<VkPhysicalDeviceGroupProperties> groups{
group_count, VkPhysicalDeviceGroupProperties{VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_GROUP_PROPERTIES}};
ASSERT_EQ(VK_SUCCESS, inst->vkEnumeratePhysicalDeviceGroups(inst, &group_count, groups.data()));
ASSERT_EQ(group_count, 1U);

// ...and the one device reachable through it is the visible one.
ASSERT_EQ(groups[0].physicalDeviceCount, 1U);
VkPhysicalDeviceProperties props{};
inst->vkGetPhysicalDeviceProperties(groups[0].physicalDevices[0], &props);
ASSERT_TRUE(string_eq(props.deviceName, "visible"));
}

// A group holding both a visible and a hidden device is dropped whole rather
// than having the hidden device removed from it. The devices in a group are
// physically linked, so a group missing a member misdescribes the hardware.
TEST(SettingsFile, DeviceConfigurationDropsPartiallyHiddenPhysicalDeviceGroup) {
FrameworkEnvironment env{};
std::vector<VulkanUUID> uuids{2, VulkanUUID{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}};

int count = 1;
for (auto& uuid : uuids) {
std::rotate(uuid.begin(), uuid.begin() + count, uuid.end());
count++;
}

auto& icd = env.add_icd(TEST_ICD_PATH_VERSION_2).set_icd_api_version(VK_API_VERSION_1_1);
icd.add_physical_device(
PhysicalDevice().set_deviceName("visible").set_api_version(VK_API_VERSION_1_1).set_deviceUUID(uuids[0]));
icd.add_physical_device(PhysicalDevice().set_deviceName("hidden").set_api_version(VK_API_VERSION_1_1).set_deviceUUID(uuids[1]));

// A single group containing both devices.
icd.physical_device_groups.push_back(PhysicalDeviceGroup({0, 1}));

env.loader_settings.set_file_format_version({1, 0, 0}).add_app_specific_setting(AppSpecificSettings{});
env.loader_settings.app_specific_settings.at(0).add_device_configuration(
LoaderSettingsDeviceConfiguration{}.set_deviceUUID(uuids[0]));
env.update_loader_settings(env.loader_settings);

InstWrapper inst{env.vulkan_functions};
inst.CheckCreate();

auto pds = inst.GetPhysDevs();
ASSERT_EQ(pds.size(), 1U);

uint32_t group_count = 0;
ASSERT_EQ(VK_SUCCESS, inst->vkEnumeratePhysicalDeviceGroups(inst, &group_count, nullptr));
std::vector<VkPhysicalDeviceGroupProperties> groups{
group_count, VkPhysicalDeviceGroupProperties{VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_GROUP_PROPERTIES}};
ASSERT_EQ(VK_SUCCESS, inst->vkEnumeratePhysicalDeviceGroups(inst, &group_count, groups.data()));
ASSERT_EQ(group_count, 0U);
}

// When no device matches the settings file, the group path fails the same way
// the plain path does rather than quietly reporting every group.
TEST(SettingsFile, DeviceConfigurationMatchingNothingFailsPhysicalDeviceGroups) {
FrameworkEnvironment env{};
std::vector<VulkanUUID> uuids{2, VulkanUUID{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}};

int count = 1;
for (auto& uuid : uuids) {
std::rotate(uuid.begin(), uuid.begin() + count, uuid.end());
count++;
}

auto& icd = env.add_icd(TEST_ICD_PATH_VERSION_2).set_icd_api_version(VK_API_VERSION_1_1);
icd.add_physical_device(PhysicalDevice().set_deviceName("only").set_api_version(VK_API_VERSION_1_1).set_deviceUUID(uuids[0]));
icd.physical_device_groups.emplace_back(0);

// The settings file names a device that does not exist.
env.loader_settings.set_file_format_version({1, 0, 0}).add_app_specific_setting(AppSpecificSettings{});
env.loader_settings.app_specific_settings.at(0).add_device_configuration(
LoaderSettingsDeviceConfiguration{}.set_deviceUUID(uuids[1]));
env.update_loader_settings(env.loader_settings);

InstWrapper inst{env.vulkan_functions};
inst.CheckCreate();

inst.GetPhysDev(VK_ERROR_INITIALIZATION_FAILED);

uint32_t group_count = 0;
ASSERT_EQ(VK_SUCCESS, inst->vkEnumeratePhysicalDeviceGroups(inst, &group_count, nullptr));
std::vector<VkPhysicalDeviceGroupProperties> groups{
group_count, VkPhysicalDeviceGroupProperties{VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_GROUP_PROPERTIES}};
ASSERT_EQ(VK_ERROR_INITIALIZATION_FAILED, inst->vkEnumeratePhysicalDeviceGroups(inst, &group_count, groups.data()));
}

// Without a settings file the groups are reported exactly as the driver gives
// them, so the filtering above must not disturb the normal path.
TEST(SettingsFile, NoDeviceConfigurationLeavesPhysicalDeviceGroupsAlone) {
FrameworkEnvironment env{};

auto& icd = env.add_icd(TEST_ICD_PATH_VERSION_2).set_icd_api_version(VK_API_VERSION_1_1);
icd.add_physical_device(PhysicalDevice().set_deviceName("first").set_api_version(VK_API_VERSION_1_1));
icd.add_physical_device(PhysicalDevice().set_deviceName("second").set_api_version(VK_API_VERSION_1_1));

icd.physical_device_groups.emplace_back(0);
icd.physical_device_groups.emplace_back(1);

InstWrapper inst{env.vulkan_functions};
inst.CheckCreate();

uint32_t group_count = 0;
ASSERT_EQ(VK_SUCCESS, inst->vkEnumeratePhysicalDeviceGroups(inst, &group_count, nullptr));
std::vector<VkPhysicalDeviceGroupProperties> groups{
group_count, VkPhysicalDeviceGroupProperties{VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_GROUP_PROPERTIES}};
ASSERT_EQ(VK_SUCCESS, inst->vkEnumeratePhysicalDeviceGroups(inst, &group_count, groups.data()));
ASSERT_EQ(group_count, 2U);
}

TEST(SettingsFile, DriverConfigurationsInSpecifiedOrder) {
FrameworkEnvironment env{};
std::vector<VulkanUUID> uuids{10, VulkanUUID{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15}};
Expand Down