From 6a19950b614f341e24c56eb2734db5fb84a2f57d Mon Sep 17 00:00:00 2001 From: Konstantin Seurer Date: Tue, 13 Sep 2022 10:42:56 +0200 Subject: [PATCH] radv: Explicitly store the VA of accel structs Gets rid of a bit of code and fixes the RRA accel_struct_vas table if the BO is freed before vkDestroyAccelerationStructureKHR is called. Signed-off-by: Konstantin Seurer Reviewed-by: Samuel Pitoiset Part-of: --- src/amd/vulkan/layers/radv_rra_layer.c | 9 +++------ src/amd/vulkan/radv_acceleration_structure.c | 28 +++++++++++----------------- src/amd/vulkan/radv_acceleration_structure.h | 7 +------ src/amd/vulkan/radv_descriptor_set.c | 2 +- src/amd/vulkan/radv_query.c | 2 +- src/amd/vulkan/radv_rra.c | 2 +- 6 files changed, 18 insertions(+), 32 deletions(-) diff --git a/src/amd/vulkan/layers/radv_rra_layer.c b/src/amd/vulkan/layers/radv_rra_layer.c index 4f05e20..6297019 100644 --- a/src/amd/vulkan/layers/radv_rra_layer.c +++ b/src/amd/vulkan/layers/radv_rra_layer.c @@ -116,8 +116,7 @@ rra_CreateAccelerationStructureKHR(VkDevice _device, RADV_FROM_HANDLE(radv_acceleration_structure, structure, *pAccelerationStructure); simple_mtx_lock(&device->rra_trace.data_mtx); - if (_mesa_hash_table_u64_search(device->rra_trace.accel_struct_vas, - radv_accel_struct_get_va(structure)) != NULL) { + if (_mesa_hash_table_u64_search(device->rra_trace.accel_struct_vas, structure->va) != NULL) { fprintf(stderr, "radv: Memory aliasing between acceleration structures detected. RRA " "captures might not work correctly.\n"); goto end; @@ -133,8 +132,7 @@ rra_CreateAccelerationStructureKHR(VkDevice _device, RADV_FROM_HANDLE(radv_event, build_submit_event, _build_submit_event); _mesa_hash_table_insert(device->rra_trace.accel_structs, structure, build_submit_event); - _mesa_hash_table_u64_insert(device->rra_trace.accel_struct_vas, - radv_accel_struct_get_va(structure), structure); + _mesa_hash_table_u64_insert(device->rra_trace.accel_struct_vas, structure->va, structure); end: simple_mtx_unlock(&device->rra_trace.data_mtx); @@ -220,8 +218,7 @@ rra_DestroyAccelerationStructureKHR(VkDevice _device, VkAccelerationStructureKHR radv_DestroyEvent(_device, radv_event_to_handle(entry->data), NULL); _mesa_hash_table_remove(device->rra_trace.accel_structs, entry); - _mesa_hash_table_u64_remove(device->rra_trace.accel_struct_vas, - radv_accel_struct_get_va(structure)); + _mesa_hash_table_u64_remove(device->rra_trace.accel_struct_vas, structure->va); simple_mtx_unlock(&device->rra_trace.data_mtx); radv_DestroyAccelerationStructureKHR(_device, _structure, pAllocator); diff --git a/src/amd/vulkan/radv_acceleration_structure.c b/src/amd/vulkan/radv_acceleration_structure.c index 76fbe62..d6f04ca 100644 --- a/src/amd/vulkan/radv_acceleration_structure.c +++ b/src/amd/vulkan/radv_acceleration_structure.c @@ -139,6 +139,7 @@ radv_CreateAccelerationStructureKHR(VkDevice _device, accel->mem_offset = buffer->offset + pCreateInfo->offset; accel->size = pCreateInfo->size; accel->bo = buffer->bo; + accel->va = radv_buffer_get_va(accel->bo) + accel->mem_offset; *pAccelerationStructure = radv_acceleration_structure_to_handle(accel); return VK_SUCCESS; @@ -164,7 +165,7 @@ radv_GetAccelerationStructureDeviceAddressKHR( VkDevice _device, const VkAccelerationStructureDeviceAddressInfoKHR *pInfo) { RADV_FROM_HANDLE(radv_acceleration_structure, accel, pInfo->accelerationStructure); - return radv_accel_struct_get_va(accel); + return accel->va; } VKAPI_ATTR VkResult VKAPI_CALL @@ -669,7 +670,7 @@ radv_CmdBuildAccelerationStructuresKHR( pInfos[i].dstAccelerationStructure); struct leaf_args leaf_consts = { - .bvh = radv_accel_struct_get_va(accel_struct), + .bvh = accel_struct->va, .bounds = pInfos[i].scratchData.deviceAddress, .ids = pInfos[i].scratchData.deviceAddress + SCRATCH_TOTAL_BOUNDS_SIZE, .dst_offset = @@ -761,7 +762,7 @@ radv_CmdBuildAccelerationStructuresKHR( pInfos[i].dstAccelerationStructure); const struct morton_args consts = { - .bvh = radv_accel_struct_get_va(accel_struct), + .bvh = accel_struct->va, .bounds = pInfos[i].scratchData.deviceAddress, .ids = pInfos[i].scratchData.deviceAddress + SCRATCH_TOTAL_BOUNDS_SIZE, }; @@ -846,7 +847,7 @@ radv_CmdBuildAccelerationStructuresKHR( dst_node_offset = ALIGN(sizeof(struct radv_accel_struct_header), 64); const struct internal_args consts = { - .bvh = radv_accel_struct_get_va(accel_struct), + .bvh = accel_struct->va, .src_ids = pInfos[i].scratchData.deviceAddress + src_scratch_offset, .dst_ids = pInfos[i].scratchData.deviceAddress + dst_scratch_offset, .dst_offset = dst_node_offset, @@ -940,15 +941,12 @@ radv_CmdCopyAccelerationStructureKHR(VkCommandBuffer commandBuffer, &saved_state, cmd_buffer, RADV_META_SAVE_COMPUTE_PIPELINE | RADV_META_SAVE_DESCRIPTORS | RADV_META_SAVE_CONSTANTS); - uint64_t src_addr = radv_accel_struct_get_va(src); - uint64_t dst_addr = radv_accel_struct_get_va(dst); - radv_CmdBindPipeline(radv_cmd_buffer_to_handle(cmd_buffer), VK_PIPELINE_BIND_POINT_COMPUTE, cmd_buffer->device->meta_state.accel_struct_build.copy_pipeline); const struct copy_constants consts = { - .src_addr = src_addr, - .dst_addr = dst_addr, + .src_addr = src->va, + .dst_addr = dst->va, .mode = COPY_MODE_COPY, }; @@ -960,7 +958,7 @@ radv_CmdCopyAccelerationStructureKHR(VkCommandBuffer commandBuffer, radv_dst_access_flush(cmd_buffer, VK_ACCESS_2_INDIRECT_COMMAND_READ_BIT, NULL); radv_indirect_dispatch(cmd_buffer, src->bo, - src_addr + offsetof(struct radv_accel_struct_header, copy_dispatch_size)); + src->va + offsetof(struct radv_accel_struct_header, copy_dispatch_size)); radv_meta_restore(&saved_state, cmd_buffer); } @@ -1010,14 +1008,12 @@ radv_CmdCopyMemoryToAccelerationStructureKHR( &saved_state, cmd_buffer, RADV_META_SAVE_COMPUTE_PIPELINE | RADV_META_SAVE_DESCRIPTORS | RADV_META_SAVE_CONSTANTS); - uint64_t dst_addr = radv_accel_struct_get_va(dst); - radv_CmdBindPipeline(radv_cmd_buffer_to_handle(cmd_buffer), VK_PIPELINE_BIND_POINT_COMPUTE, cmd_buffer->device->meta_state.accel_struct_build.copy_pipeline); const struct copy_constants consts = { .src_addr = pInfo->src.deviceAddress, - .dst_addr = dst_addr, + .dst_addr = dst->va, .mode = COPY_MODE_DESERIALIZE, }; @@ -1041,13 +1037,11 @@ radv_CmdCopyAccelerationStructureToMemoryKHR( &saved_state, cmd_buffer, RADV_META_SAVE_COMPUTE_PIPELINE | RADV_META_SAVE_DESCRIPTORS | RADV_META_SAVE_CONSTANTS); - uint64_t src_addr = radv_accel_struct_get_va(src); - radv_CmdBindPipeline(radv_cmd_buffer_to_handle(cmd_buffer), VK_PIPELINE_BIND_POINT_COMPUTE, cmd_buffer->device->meta_state.accel_struct_build.copy_pipeline); const struct copy_constants consts = { - .src_addr = src_addr, + .src_addr = src->va, .dst_addr = pInfo->dst.deviceAddress, .mode = COPY_MODE_SERIALIZE, }; @@ -1060,7 +1054,7 @@ radv_CmdCopyAccelerationStructureToMemoryKHR( radv_dst_access_flush(cmd_buffer, VK_ACCESS_2_INDIRECT_COMMAND_READ_BIT, NULL); radv_indirect_dispatch(cmd_buffer, src->bo, - src_addr + offsetof(struct radv_accel_struct_header, copy_dispatch_size)); + src->va + offsetof(struct radv_accel_struct_header, copy_dispatch_size)); radv_meta_restore(&saved_state, cmd_buffer); /* Set the header of the serialized data. */ diff --git a/src/amd/vulkan/radv_acceleration_structure.h b/src/amd/vulkan/radv_acceleration_structure.h index 81b7f65..fde3a0c 100644 --- a/src/amd/vulkan/radv_acceleration_structure.h +++ b/src/amd/vulkan/radv_acceleration_structure.h @@ -34,15 +34,10 @@ struct radv_acceleration_structure { struct radeon_winsys_bo *bo; uint64_t mem_offset; uint64_t size; + uint64_t va; }; VK_DEFINE_NONDISP_HANDLE_CASTS(radv_acceleration_structure, base, VkAccelerationStructureKHR, VK_OBJECT_TYPE_ACCELERATION_STRUCTURE_KHR) -static inline uint64_t -radv_accel_struct_get_va(const struct radv_acceleration_structure *accel) -{ - return radv_buffer_get_va(accel->bo) + accel->mem_offset; -} - #endif diff --git a/src/amd/vulkan/radv_descriptor_set.c b/src/amd/vulkan/radv_descriptor_set.c index 26f0a2f..cb96711 100644 --- a/src/amd/vulkan/radv_descriptor_set.c +++ b/src/amd/vulkan/radv_descriptor_set.c @@ -1203,7 +1203,7 @@ static ALWAYS_INLINE void write_accel_struct(void *ptr, VkAccelerationStructureKHR _accel_struct) { RADV_FROM_HANDLE(radv_acceleration_structure, accel_struct, _accel_struct); - uint64_t va = accel_struct ? radv_accel_struct_get_va(accel_struct) : 0; + uint64_t va = accel_struct ? accel_struct->va : 0; memcpy(ptr, &va, sizeof(va)); } diff --git a/src/amd/vulkan/radv_query.c b/src/amd/vulkan/radv_query.c index b875f4e..d160bec 100644 --- a/src/amd/vulkan/radv_query.c +++ b/src/amd/vulkan/radv_query.c @@ -2088,7 +2088,7 @@ radv_CmdWriteAccelerationStructuresPropertiesKHR( for (uint32_t i = 0; i < accelerationStructureCount; ++i) { RADV_FROM_HANDLE(radv_acceleration_structure, accel_struct, pAccelerationStructures[i]); - uint64_t va = radv_accel_struct_get_va(accel_struct); + uint64_t va = accel_struct->va; switch (queryType) { case VK_QUERY_TYPE_ACCELERATION_STRUCTURE_COMPACTED_SIZE_KHR: diff --git a/src/amd/vulkan/radv_rra.c b/src/amd/vulkan/radv_rra.c index c65e36b..8eb1988 100644 --- a/src/amd/vulkan/radv_rra.c +++ b/src/amd/vulkan/radv_rra.c @@ -749,7 +749,7 @@ rra_dump_acceleration_structure(struct rra_copied_accel_struct *copied_struct, * In order to make sure BLASes can be found in the hashmap, we have * to replicate that mask here. */ - uint64_t va = radv_accel_struct_get_va(accel_struct) & 0x1FFFFFFFFFFFFFF; + uint64_t va = accel_struct->va & 0x1FFFFFFFFFFFFFF; memcpy(chunk_header.virtual_address, &va, sizeof(uint64_t)); struct rra_accel_struct_metadata rra_metadata = { -- 2.7.4