From cb7b0c8d9fc02536e96e282790bab78f4c148602 Mon Sep 17 00:00:00 2001 From: Tony Gutierrez Date: Tue, 17 Jun 2025 12:48:55 -0700 Subject: [PATCH] rocr: Remove driver usage from filter device Slightly refactor the RvdFilter so it doesn't need to call into the driver. --- .../hsa-runtime/core/inc/amd_filter_device.h | 7 ++- .../core/runtime/amd_filter_device.cpp | 13 +----- .../hsa-runtime/core/runtime/amd_topology.cpp | 45 ++++++++++++------- 3 files changed, 36 insertions(+), 29 deletions(-) diff --git a/runtime/hsa-runtime/core/inc/amd_filter_device.h b/runtime/hsa-runtime/core/inc/amd_filter_device.h index 01ad14e327..a6e1026672 100644 --- a/runtime/hsa-runtime/core/inc/amd_filter_device.h +++ b/runtime/hsa-runtime/core/inc/amd_filter_device.h @@ -44,13 +44,16 @@ #define HSA_RUNTIME_CORE_INC_AMD_FILTER_DEVICE_H_ #include +#include #include #include #include #include #include -#include "hsakmt/hsakmt.h" +// Forward declaration of the HsaNodeProperties. +struct _HsaNodeProperties; +using HsaNodeProperties = _HsaNodeProperties; namespace rocr { namespace AMD { @@ -153,7 +156,7 @@ class RvdFilter { /// /// @param numNodes Number of ROCm devices present on system, includes /// both Cpu and Gpu's devices - void BuildDeviceUuidList(uint32_t numNodes); + void BuildDeviceUuidList(const std::vector& node_props); /// @brief Build the list of Gpu devices that will be enumerated to user /// diff --git a/runtime/hsa-runtime/core/runtime/amd_filter_device.cpp b/runtime/hsa-runtime/core/runtime/amd_filter_device.cpp index eaf21d2137..dba38c7617 100644 --- a/runtime/hsa-runtime/core/runtime/amd_filter_device.cpp +++ b/runtime/hsa-runtime/core/runtime/amd_filter_device.cpp @@ -52,8 +52,6 @@ #include #include -#include "hsakmt/hsakmt.h" - #include "core/util/utils.h" #include "core/inc/runtime.h" #include "core/inc/amd_cpu_agent.h" @@ -90,15 +88,8 @@ void RvdFilter::BuildRvdTokenList() { } } -void RvdFilter::BuildDeviceUuidList(uint32_t numNodes) { - HSAKMT_STATUS status; - HsaNodeProperties props = {0}; - for (HSAuint32 idx = 0; idx < numNodes; idx++) { - // Query for node properties and ignore Cpu devices - status = HSAKMT_CALL(hsaKmtGetNodeProperties(idx, &props)); - if (status != HSAKMT_STATUS_SUCCESS) { - continue; - } +void RvdFilter::BuildDeviceUuidList(const std::vector& node_props) { + for (const auto& props : node_props) { if (props.NumFComputeCores == 0) { continue; } diff --git a/runtime/hsa-runtime/core/runtime/amd_topology.cpp b/runtime/hsa-runtime/core/runtime/amd_topology.cpp index ac85ca42cc..55b75591e6 100644 --- a/runtime/hsa-runtime/core/runtime/amd_topology.cpp +++ b/runtime/hsa-runtime/core/runtime/amd_topology.cpp @@ -286,6 +286,7 @@ void SurfaceGpuList(std::vector& gpu_list, bool xnack_mode, bool enable bool BuildTopology() { auto rt = core::Runtime::runtime_singleton_; std::unordered_map driver_sys_props; + std::unordered_map> driver_node_props; size_t link_count = 0; /// @todo Currently we can filter out GPU devices using the /// ROCR_VISIBLE_DEVICES environment variable. Eventually this @@ -298,30 +299,45 @@ bool BuildTopology() { std::vector gpu_disabled; bool filter = RvdFilter::FilterDevices(); - // Get the system properties (i.e., node count) from each driver - // then update the runtime's link count before traversing each + // Get the system properties from each driver, populate the node properties list + // for each driver, then update the runtime's link count before traversing each // driver's individual nodes. for (const auto& driver : rt->AgentDrivers()) { - driver->GetSystemProperties(driver_sys_props[driver->kernel_driver_type_]); + auto &sys_props = driver_sys_props[driver->kernel_driver_type_]; + auto &node_props_vec = driver_node_props[driver->kernel_driver_type_]; + if (driver->GetSystemProperties(sys_props) != HSA_STATUS_SUCCESS) + return false; - if (!driver_sys_props[driver->kernel_driver_type_].NumNodes) continue; + const auto num_nodes = sys_props.NumNodes; - link_count += driver_sys_props[driver->kernel_driver_type_].NumNodes; + if (!num_nodes) { + continue; + } + + link_count += num_nodes; + node_props_vec.resize(num_nodes); + uint32_t node_id = 0; + + for (auto& node_props : node_props_vec) { + if (driver->GetNodeProperties(node_props, node_id) != HSA_STATUS_SUCCESS) { + return false; + } + ++node_id; + } } rt->SetLinkCount(link_count); // Traverse each driver's nodes and discover their agents. - for (const auto& driver : core::Runtime::runtime_singleton_->AgentDrivers()) { - if (driver_sys_props.find(driver->kernel_driver_type_) == driver_sys_props.end()) return false; - - const HsaSystemProperties& sys_props = driver_sys_props[driver->kernel_driver_type_]; + for (const auto& driver : rt->AgentDrivers()) { + auto& node_props_vec = driver_node_props[driver->kernel_driver_type_]; + /// @todo: Add support for AIEs. // Query if env ROCR_VISIBLE_DEVICES is defined. If defined // determine number and order of GPU devices to be surfaced. if (filter && driver->kernel_driver_type_ == core::DriverType::KFD) { rvdFilter.BuildRvdTokenList(); - rvdFilter.BuildDeviceUuidList(sys_props.NumNodes); + rvdFilter.BuildDeviceUuidList(node_props_vec); visibleCnt = rvdFilter.BuildUsrDeviceList(); for (int32_t idx = 0; idx < visibleCnt; idx++) { gpu_usr_list.push_back(invalidIdx); @@ -330,12 +346,8 @@ bool BuildTopology() { // Discover agents on every node in the platform. int32_t kfdIdx = 0; - for (HSAuint32 node_id = 0; node_id < sys_props.NumNodes; node_id++) { - HsaNodeProperties node_props = {0}; - if (driver->GetNodeProperties(node_props, node_id) != HSA_STATUS_SUCCESS) { - return false; - } - + uint32_t node_id = 0; + for (auto& node_props : node_props_vec) { if (node_props.NumCPUCores) { // Node has CPU cores so instantiate a CPU agent. DiscoverCpu(node_id, node_props); @@ -368,6 +380,7 @@ bool BuildTopology() { // possible to access links of nodes that are // not visible RegisterLinkInfo(driver, node_id, node_props.NumIOLinks); + ++node_id; } }