layers: Update DeviceQueueCreate checks
- remove some potential false-positives from QF check
- update error db with existing check
- use error db unique codes on existing checks
- move check that need state to core_validation
- deal with VK_QUEUE_FAMILY_IGNORED case
- improve error messages texts
- make messages return appropriate object
- move code that looks displaced to appropriate places
- add locks
diff --git a/layers/parameter_validation.cpp b/layers/parameter_validation.cpp
index d45d7d6..c665390 100644
--- a/layers/parameter_validation.cpp
+++ b/layers/parameter_validation.cpp
@@ -29,6 +29,7 @@
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <inttypes.h>
#include <iostream>
#include <string>
@@ -36,6 +37,7 @@
#include <unordered_map>
#include <unordered_set>
#include <vector>
+#include <mutex>
#include "vk_loader_platform.h"
#include "vulkan/vk_layer.h"
@@ -77,11 +79,15 @@
VkPhysicalDeviceLimits device_limits = {};
VkPhysicalDeviceFeatures physical_device_features = {};
VkPhysicalDevice physical_device = VK_NULL_HANDLE;
+ VkDevice device = VK_NULL_HANDLE;
DeviceExtensions enables;
VkLayerDispatchTable dispatch_table = {};
};
+// TODO : This can be much smarter, using separate locks for separate global data
+static std::mutex global_lock;
+
static uint32_t loader_layer_if_version = CURRENT_LOADER_LAYER_INTERFACE_VERSION;
static std::unordered_map<void *, layer_data *> layer_data_map;
static std::unordered_map<void *, instance_layer_data *> instance_layer_data_map;
@@ -1224,25 +1230,24 @@
return skip;
}
-static bool validate_queue_family_index(layer_data *device_data, const char *function_name, const char *parameter_name,
- uint32_t index) {
- assert(device_data != nullptr);
- debug_report_data *report_data = device_data->report_data;
+static bool ValidateDeviceQueueFamily(layer_data *device_data, uint32_t queue_family, const char *cmd_name,
+ const char *parameter_name, int32_t error_code, bool optional = false,
+ const char *vu_note = nullptr) {
bool skip = false;
- if (index == VK_QUEUE_FAMILY_IGNORED) {
- skip |= log_msg(report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__, 1,
- LayerName, "%s: %s cannot be VK_QUEUE_FAMILY_IGNORED.", function_name, parameter_name);
- } else {
- const auto &queue_data = device_data->queueFamilyIndexMap.find(index);
- if (queue_data == device_data->queueFamilyIndexMap.end()) {
- skip |= log_msg(report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__, 1,
- LayerName,
- "%s: %s (%d) must be one of the indices specified when the device was created, via "
- "the VkDeviceQueueCreateInfo structure.",
- function_name, parameter_name, index);
- return false;
- }
+ if (!vu_note) vu_note = validation_error_map[error_code];
+
+ if (!optional && queue_family == VK_QUEUE_FAMILY_IGNORED) {
+ skip |= log_msg(device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT,
+ reinterpret_cast<uint64_t>(device_data->device), __LINE__, error_code, LayerName,
+ "%s: %s is VK_QUEUE_FAMILY_IGNORED, but it is required to provide a valid queue family index value. %s",
+ cmd_name, parameter_name, vu_note);
+ } else if (device_data->queueFamilyIndexMap.find(queue_family) == device_data->queueFamilyIndexMap.end()) {
+ skip |= log_msg(device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT,
+ reinterpret_cast<uint64_t>(device_data->device), __LINE__, error_code, LayerName,
+ "%s: %s (=%" PRIu32 ") is not one of the queue families given via VkDeviceQueueCreateInfo structures when "
+ "the device was created. %s",
+ cmd_name, parameter_name, queue_family, vu_note);
}
return skip;
@@ -1497,55 +1502,104 @@
}
}
-static void validateDeviceCreateInfo(VkPhysicalDevice physicalDevice, const VkDeviceCreateInfo *pCreateInfo,
- const std::vector<VkQueueFamilyProperties> properties) {
- std::unordered_set<uint32_t> set;
+static bool ValidateDeviceCreateInfo(instance_layer_data *instance_data, VkPhysicalDevice physicalDevice,
+ const VkDeviceCreateInfo *pCreateInfo) {
+ bool skip = false;
- auto my_data = GetLayerDataPtr(get_dispatch_key(physicalDevice), instance_layer_data_map);
+ if ((pCreateInfo->enabledLayerCount > 0) && (pCreateInfo->ppEnabledLayerNames != NULL)) {
+ for (size_t i = 0; i < pCreateInfo->enabledLayerCount; i++) {
+ skip |= validate_string(instance_data->report_data, "vkCreateDevice", "pCreateInfo->ppEnabledLayerNames",
+ pCreateInfo->ppEnabledLayerNames[i]);
+ }
+ }
- if ((pCreateInfo != nullptr) && (pCreateInfo->pQueueCreateInfos != nullptr)) {
+ if ((pCreateInfo->enabledExtensionCount > 0) && (pCreateInfo->ppEnabledExtensionNames != NULL)) {
+ for (size_t i = 0; i < pCreateInfo->enabledExtensionCount; i++) {
+ skip |= validate_string(instance_data->report_data, "vkCreateDevice", "pCreateInfo->ppEnabledExtensionNames",
+ pCreateInfo->ppEnabledExtensionNames[i]);
+ }
+ }
+
+ if (pCreateInfo->pNext != NULL && pCreateInfo->pEnabledFeatures) {
+ // Check for get_physical_device_properties2 struct
+ struct std_header {
+ VkStructureType sType;
+ const void *pNext;
+ };
+ std_header *cur_pnext = (std_header *)pCreateInfo->pNext;
+ while (cur_pnext) {
+ if (VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2_KHR == cur_pnext->sType) {
+ // Cannot include VkPhysicalDeviceFeatures2KHR and have non-null pEnabledFeatures
+ skip |= log_msg(instance_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT,
+ 0, __LINE__, INVALID_USAGE, LayerName,
+ "VkDeviceCreateInfo->pNext includes a VkPhysicalDeviceFeatures2KHR struct when "
+ "pCreateInfo->pEnabledFeatures is non-NULL.");
+ break;
+ }
+ cur_pnext = (std_header *)cur_pnext->pNext;
+ }
+ }
+ if (pCreateInfo->pNext != NULL && pCreateInfo->pEnabledFeatures) {
+ // Check for get_physical_device_properties2 struct
+ struct std_header {
+ VkStructureType sType;
+ const void *pNext;
+ };
+ std_header *cur_pnext = (std_header *)pCreateInfo->pNext;
+ while (cur_pnext) {
+ if (VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2_KHR == cur_pnext->sType) {
+ // Cannot include VkPhysicalDeviceFeatures2KHR and have non-null pEnabledFeatures
+ skip |= log_msg(instance_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT,
+ 0, __LINE__, INVALID_USAGE, LayerName,
+ "VkDeviceCreateInfo->pNext includes a VkPhysicalDeviceFeatures2KHR struct when "
+ "pCreateInfo->pEnabledFeatures is non-NULL.");
+ break;
+ }
+ cur_pnext = (std_header *)cur_pnext->pNext;
+ }
+ }
+
+ // Validate pCreateInfo->pQueueCreateInfos
+ if (pCreateInfo->pQueueCreateInfos) {
+ std::unordered_set<uint32_t> set;
+
for (uint32_t i = 0; i < pCreateInfo->queueCreateInfoCount; ++i) {
- if (set.count(pCreateInfo->pQueueCreateInfos[i].queueFamilyIndex)) {
- log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__,
- VALIDATION_ERROR_00035, LayerName,
- "VkDeviceCreateInfo parameter, uint32_t pQueueCreateInfos[%d]->queueFamilyIndex, is not unique within this "
- "structure. %s",
- i, validation_error_map[VALIDATION_ERROR_00035]);
+ const uint32_t requested_queue_family = pCreateInfo->pQueueCreateInfos[i].queueFamilyIndex;
+ if (requested_queue_family == VK_QUEUE_FAMILY_IGNORED) {
+ skip |= log_msg(instance_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT,
+ VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, reinterpret_cast<uint64_t>(physicalDevice),
+ __LINE__, VALIDATION_ERROR_00054, LayerName,
+ "vkCreateDevice: pCreateInfo->pQueueCreateInfos[%" PRIu32 "].queueFamilyIndex is "
+ "VK_QUEUE_FAMILY_IGNORED, but it is required to provide a valid queue family index value. %s",
+ i, validation_error_map[VALIDATION_ERROR_00054]);
+ } else if (set.count(requested_queue_family)) {
+ skip |= log_msg(instance_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT,
+ VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, reinterpret_cast<uint64_t>(physicalDevice),
+ __LINE__, VALIDATION_ERROR_00035, LayerName,
+ "vkCreateDevice: pCreateInfo->pQueueCreateInfos[%" PRIu32 "].queueFamilyIndex (=%" PRIu32 ") is "
+ "not unique within pCreateInfo->pQueueCreateInfos array. %s",
+ i, requested_queue_family, validation_error_map[VALIDATION_ERROR_00035]);
} else {
- set.insert(pCreateInfo->pQueueCreateInfos[i].queueFamilyIndex);
+ set.insert(requested_queue_family);
}
if (pCreateInfo->pQueueCreateInfos[i].pQueuePriorities != nullptr) {
for (uint32_t j = 0; j < pCreateInfo->pQueueCreateInfos[i].queueCount; ++j) {
- if ((pCreateInfo->pQueueCreateInfos[i].pQueuePriorities[j] < 0.f) ||
- (pCreateInfo->pQueueCreateInfos[i].pQueuePriorities[j] > 1.f)) {
- log_msg(my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0,
- __LINE__, INVALID_USAGE, LayerName,
- "VkDeviceCreateInfo parameter, uint32_t pQueueCreateInfos[%d]->pQueuePriorities[%d], must be "
- "between 0 and 1. Actual value is %f",
- i, j, pCreateInfo->pQueueCreateInfos[i].pQueuePriorities[j]);
+ const float queue_priority = pCreateInfo->pQueueCreateInfos[i].pQueuePriorities[j];
+ if (!(queue_priority >= 0.f) || !(queue_priority <= 1.f)) {
+ skip |= log_msg(instance_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT,
+ VK_DEBUG_REPORT_OBJECT_TYPE_PHYSICAL_DEVICE_EXT, reinterpret_cast<uint64_t>(physicalDevice),
+ __LINE__, VALIDATION_ERROR_02056, LayerName,
+ "vkCreateDevice: pCreateInfo->pQueueCreateInfos[%" PRIu32 "].pQueuePriorities[%" PRIu32
+ "] (=%f) is not between 0 and 1 (inclusive). %s",
+ i, j, queue_priority, validation_error_map[VALIDATION_ERROR_02056]);
}
}
}
-
- if (pCreateInfo->pQueueCreateInfos[i].queueFamilyIndex >= properties.size()) {
- log_msg(
- my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__,
- INVALID_USAGE, LayerName,
- "VkDeviceCreateInfo parameter, uint32_t pQueueCreateInfos[%d]->queueFamilyIndex cannot be more than the number "
- "of queue families.",
- i);
- } else if (pCreateInfo->pQueueCreateInfos[i].queueCount >
- properties[pCreateInfo->pQueueCreateInfos[i].queueFamilyIndex].queueCount) {
- log_msg(
- my_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__,
- INVALID_USAGE, LayerName,
- "VkDeviceCreateInfo parameter, uint32_t pQueueCreateInfos[%d]->queueCount cannot be more than the number of "
- "queues for the given family index.",
- i);
- }
}
}
+
+ return skip;
}
void storeCreateDeviceData(VkDevice device, const VkDeviceCreateInfo *pCreateInfo) {
@@ -1570,62 +1624,11 @@
bool skip = false;
auto my_instance_data = GetLayerDataPtr(get_dispatch_key(physicalDevice), instance_layer_data_map);
assert(my_instance_data != nullptr);
+ std::unique_lock<std::mutex> lock(global_lock);
skip |= parameter_validation_vkCreateDevice(my_instance_data->report_data, pCreateInfo, pAllocator, pDevice);
- if (pCreateInfo != NULL) {
- if ((pCreateInfo->enabledLayerCount > 0) && (pCreateInfo->ppEnabledLayerNames != NULL)) {
- for (size_t i = 0; i < pCreateInfo->enabledLayerCount; i++) {
- skip |= validate_string(my_instance_data->report_data, "vkCreateDevice", "pCreateInfo->ppEnabledLayerNames",
- pCreateInfo->ppEnabledLayerNames[i]);
- }
- }
-
- if ((pCreateInfo->enabledExtensionCount > 0) && (pCreateInfo->ppEnabledExtensionNames != NULL)) {
- for (size_t i = 0; i < pCreateInfo->enabledExtensionCount; i++) {
- skip |= validate_string(my_instance_data->report_data, "vkCreateDevice", "pCreateInfo->ppEnabledExtensionNames",
- pCreateInfo->ppEnabledExtensionNames[i]);
- }
- }
- if (pCreateInfo->pNext != NULL && pCreateInfo->pEnabledFeatures) {
- // Check for get_physical_device_properties2 struct
- struct std_header {
- VkStructureType sType;
- const void *pNext;
- };
- std_header *cur_pnext = (std_header *)pCreateInfo->pNext;
- while (cur_pnext) {
- if (VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2_KHR == cur_pnext->sType) {
- // Cannot include VkPhysicalDeviceFeatures2KHR and have non-null pEnabledFeatures
- skip |= log_msg(my_instance_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT,
- VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__, INVALID_USAGE, LayerName,
- "VkDeviceCreateInfo->pNext includes a VkPhysicalDeviceFeatures2KHR struct when "
- "pCreateInfo->pEnabledFeatures is non-NULL.");
- break;
- }
- cur_pnext = (std_header *)cur_pnext->pNext;
- }
- }
- if (pCreateInfo->pNext != NULL && pCreateInfo->pEnabledFeatures) {
- // Check for get_physical_device_properties2 struct
- struct std_header {
- VkStructureType sType;
- const void *pNext;
- };
- std_header *cur_pnext = (std_header *)pCreateInfo->pNext;
- while (cur_pnext) {
- if (VK_STRUCTURE_TYPE_PHYSICAL_DEVICE_FEATURES_2_KHR == cur_pnext->sType) {
- // Cannot include VkPhysicalDeviceFeatures2KHR and have non-null pEnabledFeatures
- skip |= log_msg(my_instance_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT,
- VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__, INVALID_USAGE, LayerName,
- "VkDeviceCreateInfo->pNext includes a VkPhysicalDeviceFeatures2KHR struct when "
- "pCreateInfo->pEnabledFeatures is non-NULL.");
- break;
- }
- cur_pnext = (std_header *)cur_pnext->pNext;
- }
- }
- }
+ if (pCreateInfo != NULL) skip |= ValidateDeviceCreateInfo(my_instance_data, physicalDevice, pCreateInfo);
if (!skip) {
VkLayerDeviceCreateInfo *chain_info = get_chain_info(pCreateInfo, VK_LAYER_LINK_INFO);
@@ -1642,10 +1645,14 @@
// Advance the link info for the next element on the chain
chain_info->u.pLayerInfo = chain_info->u.pLayerInfo->pNext;
+ lock.unlock();
+
result = fpCreateDevice(physicalDevice, pCreateInfo, pAllocator, pDevice);
+ lock.lock();
validate_result(my_instance_data->report_data, "vkCreateDevice", {}, result);
+
if (result == VK_SUCCESS) {
layer_data *my_device_data = GetLayerDataPtr(get_dispatch_key(*pDevice), layer_data_map);
assert(my_device_data != nullptr);
@@ -1655,12 +1662,6 @@
my_device_data->enables.InitFromDeviceCreateInfo(pCreateInfo);
- uint32_t count;
- my_instance_data->dispatch_table.GetPhysicalDeviceQueueFamilyProperties(physicalDevice, &count, nullptr);
- std::vector<VkQueueFamilyProperties> properties(count);
- my_instance_data->dispatch_table.GetPhysicalDeviceQueueFamilyProperties(physicalDevice, &count, &properties[0]);
-
- validateDeviceCreateInfo(physicalDevice, pCreateInfo, properties);
storeCreateDeviceData(*pDevice, pCreateInfo);
// Query and save physical device limits for this device
@@ -1668,6 +1669,7 @@
my_instance_data->dispatch_table.GetPhysicalDeviceProperties(physicalDevice, &device_properties);
memcpy(&my_device_data->device_limits, &device_properties.limits, sizeof(VkPhysicalDeviceLimits));
my_device_data->physical_device = physicalDevice;
+ my_device_data->device = *pDevice;
// Save app-enabled features in this device's layer_data structure
if (pCreateInfo->pEnabledFeatures) {
@@ -1702,34 +1704,38 @@
}
static bool PreGetDeviceQueue(VkDevice device, uint32_t queueFamilyIndex, uint32_t queueIndex) {
+ bool skip = false;
layer_data *my_device_data = GetLayerDataPtr(get_dispatch_key(device), layer_data_map);
assert(my_device_data != nullptr);
- validate_queue_family_index(my_device_data, "vkGetDeviceQueue", "queueFamilyIndex", queueFamilyIndex);
+ skip |=
+ ValidateDeviceQueueFamily(my_device_data, queueFamilyIndex, "vkGetDeviceQueue", "queueFamilyIndex", VALIDATION_ERROR_00060);
const auto &queue_data = my_device_data->queueFamilyIndexMap.find(queueFamilyIndex);
- if (queue_data->second <= queueIndex) {
- log_msg(
- my_device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_UNKNOWN_EXT, 0, __LINE__,
- VALIDATION_ERROR_00061, LayerName,
- "vkGetDeviceQueue() parameter, uint32_t queueIndex %d, must be less than the number of queues given when the device "
- "was created. %s",
- queueIndex, validation_error_map[VALIDATION_ERROR_00061]);
- return false;
+ if (queue_data != my_device_data->queueFamilyIndexMap.end() && queue_data->second <= queueIndex) {
+ skip |= log_msg(my_device_data->report_data, VK_DEBUG_REPORT_ERROR_BIT_EXT, VK_DEBUG_REPORT_OBJECT_TYPE_DEVICE_EXT,
+ reinterpret_cast<uint64_t>(device), __LINE__, VALIDATION_ERROR_00061, LayerName,
+ "vkGetDeviceQueue: queueIndex (=%" PRIu32 ") is not less than the number of queues requested from "
+ "queueFamilyIndex (=%" PRIu32 ") when the device was created (i.e. is not less than %" PRIu32 "). %s",
+ queueIndex, queueFamilyIndex, queue_data->second, validation_error_map[VALIDATION_ERROR_00061]);
}
- return true;
+
+ return skip;
}
VKAPI_ATTR void VKAPI_CALL GetDeviceQueue(VkDevice device, uint32_t queueFamilyIndex, uint32_t queueIndex, VkQueue *pQueue) {
bool skip = false;
layer_data *my_data = GetLayerDataPtr(get_dispatch_key(device), layer_data_map);
assert(my_data != NULL);
+ std::unique_lock<std::mutex> lock(global_lock);
skip |= parameter_validation_vkGetDeviceQueue(my_data->report_data, queueFamilyIndex, queueIndex, pQueue);
if (!skip) {
PreGetDeviceQueue(device, queueFamilyIndex, queueIndex);
+ lock.unlock();
+
my_data->dispatch_table.GetDeviceQueue(device, queueFamilyIndex, queueIndex, pQueue);
}
}
@@ -3963,8 +3969,8 @@
layer_data *my_data = GetLayerDataPtr(get_dispatch_key(device), layer_data_map);
assert(my_data != NULL);
- skip |=
- validate_queue_family_index(my_data, "vkCreateCommandPool", "pCreateInfo->queueFamilyIndex", pCreateInfo->queueFamilyIndex);
+ skip |= ValidateDeviceQueueFamily(my_data, pCreateInfo->queueFamilyIndex, "vkCreateCommandPool",
+ "pCreateInfo->queueFamilyIndex", VALIDATION_ERROR_00068);
skip |= parameter_validation_vkCreateCommandPool(my_data->report_data, pCreateInfo, pAllocator, pCommandPool);