From 38ea4370c1282b3e508ccd24b51074f88c1068fc Mon Sep 17 00:00:00 2001 From: David Yat Sin Date: Thu, 8 May 2025 00:33:51 +0000 Subject: [PATCH] rocr: Fix doorbell ring When compiling with -O0, some compilers generate a xchg instruction for the __atomic_store(...) built-in. Using xchg on MMIO memory is undefined-behavior and may be ignored on certain CPUs. [ROCm/ROCR-Runtime commit: f011a9506d6be30318ef663bff9212c9d6ed756f] --- .../core/runtime/amd_aql_queue.cpp | 6 +++-- .../core/runtime/amd_blit_kernel.cpp | 3 ++- .../runtime/hsa-runtime/inc/hsa.h | 27 +++++++++++-------- 3 files changed, 22 insertions(+), 14 deletions(-) diff --git a/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_aql_queue.cpp b/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_aql_queue.cpp index 65a323e200..d8cce1081f 100644 --- a/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_aql_queue.cpp +++ b/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_aql_queue.cpp @@ -3,7 +3,7 @@ // The University of Illinois/NCSA // Open Source License (NCSA) // -// Copyright (c) 2014-2024, Advanced Micro Devices, Inc. All rights reserved. +// Copyright (c) 2014-2025, Advanced Micro Devices, Inc. All rights reserved. // // Developed by: // @@ -478,7 +478,9 @@ void AqlQueue::StoreRelaxed(hsa_signal_value_t value) { HSAKMT_CALL(hsaKmtQueueRingDoorbell(queue_id_)); } else { // Hardware doorbell supports AQL semantics. - atomic::Store(signal_.hardware_doorbell_ptr, uint64_t(value), std::memory_order_release); + _mm_sfence(); + *(signal_.hardware_doorbell_ptr) = uint64_t(value); + /* signal_ is allocated as uncached so we do not need read-back to flush WC */ } return; } diff --git a/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_blit_kernel.cpp b/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_blit_kernel.cpp index c92ac2f710..ed4364eab0 100644 --- a/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_blit_kernel.cpp +++ b/projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_blit_kernel.cpp @@ -894,7 +894,8 @@ void BlitKernel::PopulateQueue(uint64_t index, uint64_t code_handle, void* args, // Ensure the packet body is written as header may get reordered when writing over PCIE _mm_sfence(); } - queue_buffer[index & queue_bitmask_].header = kDispatchPacketHeader; + __atomic_store_n(&(queue_buffer[index & queue_bitmask_].full_header), + kDispatchPacketHeader | packet.setup << 16, __ATOMIC_RELEASE); LogPrint(HSA_AMD_LOG_FLAG_BLIT_KERNEL_PKTS, "HWq=%p, id=%d, Dispatch Header = " diff --git a/projects/rocr-runtime/runtime/hsa-runtime/inc/hsa.h b/projects/rocr-runtime/runtime/hsa-runtime/inc/hsa.h index ed793b8a24..00753e992e 100644 --- a/projects/rocr-runtime/runtime/hsa-runtime/inc/hsa.h +++ b/projects/rocr-runtime/runtime/hsa-runtime/inc/hsa.h @@ -2957,18 +2957,23 @@ typedef enum { * @brief AQL kernel dispatch packet */ typedef struct hsa_kernel_dispatch_packet_s { - /** - * Packet header. Used to configure multiple packet parameters such as the - * packet type. The parameters are described by ::hsa_packet_header_t. - */ - uint16_t header; + union { + struct { + /** + * Packet header. Used to configure multiple packet parameters such as the + * packet type. The parameters are described by ::hsa_packet_header_t. + */ + uint16_t header; - /** - * Dispatch setup parameters. Used to configure kernel dispatch parameters - * such as the number of dimensions in the grid. The parameters are described - * by ::hsa_kernel_dispatch_packet_setup_t. - */ - uint16_t setup; + /** + * Dispatch setup parameters. Used to configure kernel dispatch parameters + * such as the number of dimensions in the grid. The parameters are described + * by ::hsa_kernel_dispatch_packet_setup_t. + */ + uint16_t setup; + }; + uint32_t full_header; + }; /** * X dimension of work-group, in work-items. Must be greater than 0.