loader: apply settings device configurations to device groups

The plain vkEnumeratePhysicalDevices path applies the settings file's
device_configurations in terminator_EnumeratePhysicalDevices. Group
enumeration did not, so an application enumerating groups could reach a
VkPhysicalDevice that the settings file meant to hide, and the validation
layer would then fault on a device it had never seen enumerated.

Apply the same filter in terminator_EnumeratePhysicalDeviceGroups, reusing
loader_apply_settings_device_configurations so the two paths cannot drift
apart. If a group contains an excluded VkPhysicalDevice, drop the whole
group rather than editing it: the devices in a group are physically linked,
so a group with a member removed no longer describes the hardware it claims
to.

Also publish the post-filter group count. The copy loop already skipped
groups whose physicalDeviceCount is zero, but the function published the
pre-skip total, so enabling that path would have read past the groups
actually written.

Author: Wint3rNight

loader/loader.c

@@ -7217,7 +7217,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));
@@ -7734,6 +7734,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;
@@ -7988,11 +7992,61 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumeratePhysicalDeviceGroups(
             }
         }
 
+        // Apply the settings file's device_configurations to the groups.  If a group
+        // contains an excluded VkPhysicalDevice, drop the whole group.
+        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;
+                    memset(local_phys_dev_groups[group].group_props.physicalDevices, 0,
+                           sizeof(local_phys_dev_groups[group].group_props.physicalDevices));
+                }
+            }
+        }
+
         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;
             }
@@ -8046,6 +8100,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:
@@ -8096,7 +8154,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;
         }
 

tests/loader_settings_tests.cpp

@@ -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}};