From e5df9a1d74a6558246051cbb5f024616b8f52cca Mon Sep 17 00:00:00 2001 From: Lionel Landwerlin Date: Sun, 13 Nov 2022 00:24:11 +0200 Subject: [PATCH] vulkan/overlay: deal with unknown pNext structures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit To implement some of the features of the layer, we need to enable some of the feature bits at device/command_buffer creation. To do so, we need to edit some of the structures coming from the application. Most of those are const so we need to clone them before edition. This change disables some of the layer features if we run into a situation where one of the structure we need to clone is unknown such that we can't make a copy of it (since we don't know its size). Signed-off-by: Lionel Landwerlin Closes: https://gitlab.freedesktop.org/mesa/mesa/-/issues/7677 Cc: mesa-stable Reviewed-by: José Roberto de Souza Part-of: (cherry picked from commit b30a75a195fea9013fc912b84cd776aaa76f4692) --- .pick_status.json | 2 +- src/vulkan/overlay-layer/overlay.cpp | 135 ++++++++++++++++----------- 2 files changed, 81 insertions(+), 56 deletions(-) diff --git a/.pick_status.json b/.pick_status.json index 0d713e4472a..27b08828197 100644 --- a/.pick_status.json +++ b/.pick_status.json @@ -562,7 +562,7 @@ "description": "vulkan/overlay: deal with unknown pNext structures", "nominated": true, "nomination_type": 0, - "resolution": 0, + "resolution": 1, "main_sha": null, "because_sha": null }, diff --git a/src/vulkan/overlay-layer/overlay.cpp b/src/vulkan/overlay-layer/overlay.cpp index 105b0239031..3d02ade4903 100644 --- a/src/vulkan/overlay-layer/overlay.cpp +++ b/src/vulkan/overlay-layer/overlay.cpp @@ -89,6 +89,8 @@ struct device_data { struct queue_data **queues; uint32_t n_queues; + bool pipeline_statistics_enabled; + /* For a single frame */ struct frame_stat frame_stats; }; @@ -290,6 +292,16 @@ static VkLayerDeviceCreateInfo *get_device_chain_info(const VkDeviceCreateInfo * return NULL; } +static void +free_chain(struct VkBaseOutStructure *chain) +{ + while (chain) { + void *node = chain; + chain = chain->pNext; + free(node); + } +} + static struct VkBaseOutStructure * clone_chain(const struct VkBaseInStructure *chain) { @@ -297,6 +309,11 @@ clone_chain(const struct VkBaseInStructure *chain) vk_foreach_struct_const(item, chain) { size_t item_size = vk_structure_type_size(item); + if (item_size == 0) { + free_chain(head); + return NULL; + } + struct VkBaseOutStructure *new_item = (struct VkBaseOutStructure *)malloc(item_size);; @@ -312,16 +329,6 @@ clone_chain(const struct VkBaseInStructure *chain) return head; } -static void -free_chain(struct VkBaseOutStructure *chain) -{ - while (chain) { - void *node = chain; - chain = chain->pNext; - free(node); - } -} - /**/ static struct instance_data *new_instance_data(VkInstance instance) @@ -2203,34 +2210,44 @@ static VkResult overlay_BeginCommandBuffer( * we have the right inheritance. */ if (cmd_buffer_data->level == VK_COMMAND_BUFFER_LEVEL_SECONDARY) { - VkCommandBufferBeginInfo *begin_info = (VkCommandBufferBeginInfo *) - clone_chain((const struct VkBaseInStructure *)pBeginInfo); - VkCommandBufferInheritanceInfo *parent_inhe_info = (VkCommandBufferInheritanceInfo *) - vk_find_struct(begin_info, COMMAND_BUFFER_INHERITANCE_INFO); - VkCommandBufferInheritanceInfo inhe_info = { - VK_STRUCTURE_TYPE_COMMAND_BUFFER_INHERITANCE_INFO, - NULL, - VK_NULL_HANDLE, - 0, - VK_NULL_HANDLE, - VK_FALSE, - 0, - overlay_query_flags, - }; + VkCommandBufferBeginInfo begin_info = *pBeginInfo; - if (parent_inhe_info) - parent_inhe_info->pipelineStatistics = overlay_query_flags; - else { - inhe_info.pNext = begin_info->pNext; - begin_info->pNext = &inhe_info; + struct VkBaseOutStructure *new_pnext = + clone_chain((const struct VkBaseInStructure *)pBeginInfo->pNext); + VkCommandBufferInheritanceInfo inhe_info; + + /* If there was no pNext chain given or we managed to copy it, we can + * add our stuff in there. + * + * Otherwise, keep the old pointer. We failed to copy the pNext chain, + * meaning there is an unknown extension somewhere in there. + */ + if (new_pnext || pBeginInfo->pNext == NULL) { + begin_info.pNext = new_pnext; + + VkCommandBufferInheritanceInfo *parent_inhe_info = (VkCommandBufferInheritanceInfo *) + vk_find_struct(new_pnext, COMMAND_BUFFER_INHERITANCE_INFO); + inhe_info = (VkCommandBufferInheritanceInfo) { + VK_STRUCTURE_TYPE_COMMAND_BUFFER_INHERITANCE_INFO, + NULL, + VK_NULL_HANDLE, + 0, + VK_NULL_HANDLE, + VK_FALSE, + 0, + overlay_query_flags, + }; + + if (parent_inhe_info) + parent_inhe_info->pipelineStatistics = overlay_query_flags; + else + __vk_append_struct(&begin_info, &inhe_info); } - VkResult result = device_data->vtable.BeginCommandBuffer(commandBuffer, pBeginInfo); + VkResult result = device_data->vtable.BeginCommandBuffer( + commandBuffer, &begin_info); - if (!parent_inhe_info) - begin_info->pNext = inhe_info.pNext; - - free_chain((struct VkBaseOutStructure *)begin_info); + free_chain(new_pnext); return result; } @@ -2334,7 +2351,7 @@ static VkResult overlay_AllocateCommandBuffers( VkQueryPool pipeline_query_pool = VK_NULL_HANDLE; VkQueryPool timestamp_query_pool = VK_NULL_HANDLE; - if (device_data->instance->pipeline_statistics_enabled && + if (device_data->pipeline_statistics_enabled && pAllocateInfo->level == VK_COMMAND_BUFFER_LEVEL_PRIMARY) { VkQueryPoolCreateInfo pool_info = { VK_STRUCTURE_TYPE_QUERY_POOL_CREATE_INFO, @@ -2517,29 +2534,33 @@ static VkResult overlay_CreateDevice( VkPhysicalDeviceFeatures device_features = {}; VkPhysicalDeviceFeatures *device_features_ptr = NULL; - VkDeviceCreateInfo *device_info = (VkDeviceCreateInfo *) - clone_chain((const struct VkBaseInStructure *)pCreateInfo); + VkDeviceCreateInfo create_info = *pCreateInfo; - VkPhysicalDeviceFeatures2 *device_features2 = (VkPhysicalDeviceFeatures2 *) - vk_find_struct(device_info, PHYSICAL_DEVICE_FEATURES_2); - if (device_features2) { - /* Can't use device_info->pEnabledFeatures when VkPhysicalDeviceFeatures2 is present */ - device_features_ptr = &device_features2->features; - } else { - if (device_info->pEnabledFeatures) - device_features = *(device_info->pEnabledFeatures); - device_features_ptr = &device_features; - device_info->pEnabledFeatures = &device_features; + struct VkBaseOutStructure *new_pnext = + clone_chain((const struct VkBaseInStructure *) pCreateInfo->pNext); + if (new_pnext != NULL) { + create_info.pNext = new_pnext; + + VkPhysicalDeviceFeatures2 *device_features2 = (VkPhysicalDeviceFeatures2 *) + vk_find_struct(new_pnext, PHYSICAL_DEVICE_FEATURES_2); + if (device_features2) { + /* Can't use device_info->pEnabledFeatures when VkPhysicalDeviceFeatures2 is present */ + device_features_ptr = &device_features2->features; + } else { + if (create_info.pEnabledFeatures) + device_features = *(create_info.pEnabledFeatures); + device_features_ptr = &device_features; + create_info.pEnabledFeatures = &device_features; + } + + if (instance_data->pipeline_statistics_enabled) { + device_features_ptr->inheritedQueries = true; + device_features_ptr->pipelineStatisticsQuery = true; + } } - if (instance_data->pipeline_statistics_enabled) { - device_features_ptr->inheritedQueries = true; - device_features_ptr->pipelineStatisticsQuery = true; - } - - - VkResult result = fpCreateDevice(physicalDevice, device_info, pAllocator, pDevice); - free_chain((struct VkBaseOutStructure *)device_info); + VkResult result = fpCreateDevice(physicalDevice, &create_info, pAllocator, pDevice); + free_chain(new_pnext); if (result != VK_SUCCESS) return result; struct device_data *device_data = new_device_data(*pDevice, instance_data); @@ -2556,6 +2577,10 @@ static VkResult overlay_CreateDevice( device_map_queues(device_data, pCreateInfo); + device_data->pipeline_statistics_enabled = + new_pnext != NULL && + instance_data->pipeline_statistics_enabled; + return result; }