From 8ed52616ad658cde9942bcadd97a6b8c435050e5 Mon Sep 17 00:00:00 2001 From: "Kanangot Balakrishnan, Bindhiya" Date: Mon, 2 Jun 2025 14:01:06 -0500 Subject: [PATCH] [SWDEV-519061] xgmi command output shows zero for all xgmi acc read/write data in the first column (#392) The xgmi read and write accumulated data from gpu metric index is based on sysfs xgmi_port_num file. Mapped these two to display read and write wrt src_gpu Vs dst_gpu. --------- Signed-off-by: Bindhiya Kanangot Balakrishnan --- amdsmi_cli/amdsmi_commands.py | 81 +++++++++------------ py-interface/amdsmi_interface.py | 8 +- rocm_smi/include/rocm_smi/rocm_smi.h | 20 +++++ rocm_smi/include/rocm_smi/rocm_smi_device.h | 1 + rocm_smi/src/rocm_smi.cc | 56 ++++++++++++++ rocm_smi/src/rocm_smi_device.cc | 6 ++ src/amd_smi/amd_smi.cc | 46 +++++++++++- 7 files changed, 161 insertions(+), 57 deletions(-) diff --git a/amdsmi_cli/amdsmi_commands.py b/amdsmi_cli/amdsmi_commands.py index f6cb699a92..dbe5a5b5e1 100644 --- a/amdsmi_cli/amdsmi_commands.py +++ b/amdsmi_cli/amdsmi_commands.py @@ -5948,7 +5948,6 @@ class AMDSMICommands(): xgmi_dict['link_metrics']['max_bandwidth'] = max_bandwidth # Populate link metrics - link_num = 0 for dest_gpu in args.gpu: partition_id = -1 try: @@ -5965,57 +5964,45 @@ class AMDSMICommands(): dest_link_dict = { "gpu" : dest_gpu_id, "bdf" : dest_gpu_bdf, - "read" : "N/A", - "write" : "N/A" + "read" : 0, + "write" : 0, } - # Don't make a call to check link status for the same gpu - if dest_gpu_bdf == src_gpu_bdf: + found = False + for link in xgmi_metrics_info['links']: + if link['bdf'] == dest_gpu_bdf: + # Accumulate read/write if multiple links have the same bdf + dest_link_dict['read'] += link['read'] + dest_link_dict['write'] += link['write'] + found = True + if not found: dest_link_dict['read'] = "N/A" dest_link_dict['write'] = "N/A" - xgmi_dict['link_metrics']['links'].append(dest_link_dict) - continue + else: + data_unit = 'KB' + if self.logger.is_human_readable_format(): + dest_link_dict['read'] = self.helpers.convert_bytes_to_readable(dest_link_dict['read'] * 1024, True) + dest_link_dict['write'] = self.helpers.convert_bytes_to_readable(dest_link_dict['write'] * 1024, True) + elif self.logger.is_json_format(): + dest_link_dict['read'] = {"value" : dest_link_dict['read'], + "unit" : data_unit} + dest_link_dict['write'] = {"value" : dest_link_dict['write'], + "unit" : data_unit} - try: - # Get the read write relative to the source gpu - read = xgmi_metrics_info['links'][link_num]['read'] - write = xgmi_metrics_info['links'][link_num]['write'] - link_num += 1 - except (KeyError, amdsmi_exception.AmdSmiLibraryException) as e: - read = "N/A" - write = "N/A" - logging.debug("Failed to get read data for %s to %s | %s", - self.helpers.get_gpu_id_from_device_handle(src_gpu), - self.helpers.get_gpu_id_from_device_handle(dest_gpu), - e.get_error_info()) - - data_unit = 'KB' - if self.logger.is_human_readable_format(): - dest_link_dict['read'] = self.helpers.convert_bytes_to_readable(read * 1024, True) - dest_link_dict['write'] = self.helpers.convert_bytes_to_readable(write * 1024, True) - elif self.logger.is_json_format(): - dest_link_dict['read'] = {"value" : read, - "unit" : data_unit} - dest_link_dict['write'] = {"value" : write, - "unit" : data_unit} - elif self.logger.is_csv_format(): - dest_link_dict['read'] = read - dest_link_dict['write'] = write - - try: - link_type = amdsmi_interface.amdsmi_topo_get_link_type(src_gpu, dest_gpu)['type'] - if xgmi_dict['link_metrics']['link_type'] != "XGMI" and isinstance(link_type, int): - if link_type == amdsmi_interface.amdsmi_wrapper.AMDSMI_LINK_TYPE_INTERNAL: - xgmi_dict['link_metrics']['link_type'] = "UNKNOWN" - elif link_type == amdsmi_interface.amdsmi_wrapper.AMDSMI_LINK_TYPE_PCIE: - xgmi_dict['link_metrics']['link_type'] = "PCIE" - elif link_type == amdsmi_interface.amdsmi_wrapper.AMDSMI_LINK_TYPE_XGMI: - xgmi_dict['link_metrics']['link_type'] = "XGMI" - except amdsmi_exception.AmdSmiLibraryException as e: - logging.debug("Failed to get link type for %s to %s | %s", - self.helpers.get_gpu_id_from_device_handle(src_gpu), - self.helpers.get_gpu_id_from_device_handle(dest_gpu), - e.get_error_info()) + try: + link_type = amdsmi_interface.amdsmi_topo_get_link_type(src_gpu, dest_gpu)['type'] + if xgmi_dict['link_metrics']['link_type'] != "XGMI" and isinstance(link_type, int): + if link_type == amdsmi_interface.amdsmi_wrapper.AMDSMI_LINK_TYPE_INTERNAL: + xgmi_dict['link_metrics']['link_type'] = "UNKNOWN" + elif link_type == amdsmi_interface.amdsmi_wrapper.AMDSMI_LINK_TYPE_PCIE: + xgmi_dict['link_metrics']['link_type'] = "PCIE" + elif link_type == amdsmi_interface.amdsmi_wrapper.AMDSMI_LINK_TYPE_XGMI: + xgmi_dict['link_metrics']['link_type'] = "XGMI" + except amdsmi_exception.AmdSmiLibraryException as e: + logging.debug("Failed to get link type for %s to %s | %s", + self.helpers.get_gpu_id_from_device_handle(src_gpu), + self.helpers.get_gpu_id_from_device_handle(dest_gpu), + e.get_error_info()) xgmi_dict['link_metrics']['links'].append(dest_link_dict) diff --git a/py-interface/amdsmi_interface.py b/py-interface/amdsmi_interface.py index 3728ad73b4..2230df59dc 100644 --- a/py-interface/amdsmi_interface.py +++ b/py-interface/amdsmi_interface.py @@ -3112,22 +3112,18 @@ def amdsmi_get_link_metrics(processor_handle: amdsmi_wrapper.amdsmi_processor_ha ) ) - bdf = amdsmi_wrapper.amdsmi_bdf_t() - # TODO: Dummy BDF - to be replaced with destination BDF from xgmi_port_num when available - bdf.struct_amdsmi_bdf_t = amdsmi_wrapper.struct_amdsmi_bdf_t(0xFFFF, 0xFF, 0xFF, 0xF) - links = [] for i in range(AMDSMI_MAX_NUM_XGMI_LINKS): link = link_metrics.links[i] links.append({ - "bdf": _format_bdf(bdf), + "bdf": _format_bdf(link.bdf), "link_type": link.link_type, "read": link.read, "write": link.write, }) return { - "num_links": AMDSMI_MAX_NUM_XGMI_LINKS, + "num_links": link_metrics.num_links, "bit_rate": link_metrics.bit_rate, "max_bandwidth": link_metrics.max_bandwidth, "links": links diff --git a/rocm_smi/include/rocm_smi/rocm_smi.h b/rocm_smi/include/rocm_smi/rocm_smi.h index 7c6dc4f9ad..e272067288 100644 --- a/rocm_smi/include/rocm_smi/rocm_smi.h +++ b/rocm_smi/include/rocm_smi/rocm_smi.h @@ -1994,6 +1994,26 @@ rsmi_status_t rsmi_dev_unique_id_get(uint32_t dv_ind, uint64_t *id); */ rsmi_status_t rsmi_dev_xgmi_physical_id_get(uint32_t dv_ind, uint16_t *id); +/** + * @brief Get the XGMI src_gpu to dest_gpu link_num based mapping for the device + * + * @details Given a device index @p dv_ind, a pointer to a uint16_t to get the + * count of XGMI links, and a pointer to a uint16_t array to which the dest_gpu's + * order will be written + * + * @param[in] dv_ind a device index + * + * @param[inout] count a pointer to uint16_t to which the count of XGMI links + * will be written + * + * @param[inout] xgmi_dst_gpu_order a pointer to uint16_t array to which the + * dest_gpu's order will be written + * + * @retval ::RSMI_STATUS_SUCCESS is returned upon successful call. + * + */ +rsmi_status_t rsmi_dev_xgmi_port_num_get(uint32_t dv_ind, uint32_t *count, uint16_t *link_to_dst_gpu); + /** * @brief Get the GUID, also known as the GPU device id, * associated with the provided device index indicated by KFD. diff --git a/rocm_smi/include/rocm_smi/rocm_smi_device.h b/rocm_smi/include/rocm_smi/rocm_smi_device.h index f408938419..0e7b4f9c15 100644 --- a/rocm_smi/include/rocm_smi/rocm_smi_device.h +++ b/rocm_smi/include/rocm_smi/rocm_smi_device.h @@ -89,6 +89,7 @@ enum DevInfoTypes { kDevMemOverDriveLevel, kDevDevID, kDevXGMIPhysicalID, + kDevXGMIPortNum, kDevDevRevID, kDevDevProdName, kDevDevProdNum, diff --git a/rocm_smi/src/rocm_smi.cc b/rocm_smi/src/rocm_smi.cc index d24adc60b0..5121df33d3 100644 --- a/rocm_smi/src/rocm_smi.cc +++ b/rocm_smi/src/rocm_smi.cc @@ -274,6 +274,25 @@ static rsmi_status_t get_dev_value_str(amd::smi::DevInfoTypes type, return amd::smi::ErrnoToRsmiStatus(ret); } +static rsmi_status_t read_dev_port_map_file(amd::smi::DevInfoTypes type, uint32_t dv_ind, std::string *val_str) { + assert(val_str != nullptr); + if (val_str == nullptr) { + return RSMI_STATUS_INVALID_ARGS; + } + GET_DEV_FROM_INDX + std::string file_path = dev->get_sys_file_path_by_type(type); + if (file_path.empty()) { + return RSMI_STATUS_FILE_ERROR; + } + std::ifstream infile(file_path); + if (!infile) { + return RSMI_STATUS_FILE_ERROR; + } + std::ostringstream ss; + ss << infile.rdbuf(); + *val_str = ss.str(); + return RSMI_STATUS_SUCCESS; +} static rsmi_status_t get_dev_value_int(amd::smi::DevInfoTypes type, uint32_t dv_ind, uint64_t *val_int) { assert(val_int != nullptr); @@ -974,6 +993,43 @@ rsmi_dev_xgmi_physical_id_get(uint32_t dv_ind, uint16_t *id) { return ret; } +rsmi_status_t +rsmi_dev_xgmi_port_num_get(uint32_t dv_ind, uint32_t *count, uint16_t *link_to_dst_node) { + std::ostringstream ss; + rsmi_status_t ret; + ss << __PRETTY_FUNCTION__ << "| ======= start ======="; + LOG_TRACE(ss); + CHK_SUPPORT_NAME_ONLY(link_to_dst_node) + *link_to_dst_node = std::numeric_limits::max(); + + std::string s; + ret = read_dev_port_map_file(amd::smi::kDevXGMIPortNum, dv_ind, &s); + + std::istringstream iss(s); + std::string line; + *count = 0; + while (std::getline(iss, line)) { + if (line.empty()) continue; + std::istringstream f(line); + std::string src_token, arrow, dst_token; + if (f >> src_token >> arrow >> dst_token) { + size_t src_colon = src_token.find(':'); + size_t dst_colon = dst_token.find(':'); + if (src_colon != std::string::npos && dst_colon != std::string::npos) { + uint16_t src_link = static_cast(std::stoi(src_token.substr(src_colon + 1))); + uint16_t dst_node = static_cast(std::stoi(dst_token.substr(0, dst_colon))); + link_to_dst_node[src_link] = dst_node; + (*count)++; + } + } + } + + ss << __PRETTY_FUNCTION__ << " | ======= end =======" + << ", reporting " << amd::smi::getRSMIStatusString(ret); + LOG_TRACE(ss); + return ret; +} + rsmi_status_t rsmi_dev_revision_get(uint32_t dv_ind, uint16_t *revision) { std::ostringstream outss; diff --git a/rocm_smi/src/rocm_smi_device.cc b/rocm_smi/src/rocm_smi_device.cc index eeec891c99..d9b88f5324 100644 --- a/rocm_smi/src/rocm_smi_device.cc +++ b/rocm_smi/src/rocm_smi_device.cc @@ -69,6 +69,7 @@ static const char *kDevDevProdNameFName = "product_name"; static const char *kDevDevProdNumFName = "product_number"; static const char *kDevDevIDFName = "device"; static const char* kDevXGMIPhysicalIDFName = "xgmi_physical_id"; +static const char *kDevXGMIPortNumFName = "xgmi_port_num"; static const char *kDevDevRevIDFName = "revision"; static const char *kDevVendorIDFName = "vendor"; static const char *kDevBoardInfoFName = "board_info"; @@ -249,6 +250,7 @@ static const std::map kDevAttribNameMap = { {kDevDevProdNum, kDevDevProdNumFName}, {kDevDevID, kDevDevIDFName}, {kDevXGMIPhysicalID, kDevXGMIPhysicalIDFName}, + {kDevXGMIPortNum, kDevXGMIPortNumFName}, {kDevDevRevID, kDevDevRevIDFName}, {kDevVendorID, kDevVendorIDFName}, {kDevPCieVendorID, kDevPCieVendorIDFName}, @@ -420,6 +422,7 @@ Device::devInfoTypesStrings = { {kDevMemOverDriveLevel, "kDevMemOverDriveLevel"}, {kDevDevID, "kDevDevID"}, {kDevXGMIPhysicalID, "kDevXGMIPhysicalID"}, + {kDevXGMIPortNum, "kDevXGMIPortNum"}, {kDevDevRevID, "kDevDevRevID"}, {kDevDevProdName, "kDevDevProdName"}, {kDevBoardInfo, "kDevBoardInfo"}, @@ -521,6 +524,7 @@ static const std::map kDevFuncDependsMap = { {"rsmi_dev_vram_vendor_get", {{kDevVramVendorFName}, {}}}, {"rsmi_dev_id_get", {{kDevDevIDFName}, {}}}, {"rsmi_dev_xgmi_physical_id_get", {{kDevXGMIPhysicalIDFName}, {}}}, + {"rsmi_dev_xgmi_port_num_get", {{kDevXGMIPortNumFName}, {}}}, {"rsmi_dev_revision_get", {{kDevDevRevIDFName}, {}}}, {"rsmi_dev_vendor_id_get", {{kDevVendorIDFName}, {}}}, {"rsmi_dev_name_get", {{kDevVendorIDFName, @@ -1198,6 +1202,7 @@ int Device::readDevInfo(DevInfoTypes type, uint64_t *val) { case kDevPCieVendorID: case kDevErrCntFeatures: case kDevXGMIPhysicalID: + case kDevXGMIPortNum: case kDevErrRASSchema: case kDevErrTableVersion: ret = readDevInfoStr(type, &tempStr); @@ -1380,6 +1385,7 @@ int Device::readDevInfo(DevInfoTypes type, std::string *val) { case kDevMemoryPartition: case kDevNumaNode: case kDevXGMIPhysicalID: + case kDevXGMIPortNum: case kDevAvailableMemoryPartition: case kDevProcessIsolation: case kDevSupportedXcpConfigs: diff --git a/src/amd_smi/amd_smi.cc b/src/amd_smi/amd_smi.cc index 7a39ba497b..6eeac910c9 100644 --- a/src/amd_smi/amd_smi.cc +++ b/src/amd_smi/amd_smi.cc @@ -1747,9 +1747,6 @@ amdsmi_get_gpu_xgmi_link_status(amdsmi_processor_handle processor_handle, uint32_t dev_num = 0; rsmi_num_monitor_devices(&dev_num); link_status->total_links = AMDSMI_MAX_NUM_XGMI_LINKS; - if (dev_num <= link_status->total_links) { - link_status->total_links = dev_num; - } // get the status values from the metric info for (unsigned int i = 0; i < link_status->total_links; i++) { if (metric_info.xgmi_link_status[i] == std::numeric_limits::max()) { @@ -2132,12 +2129,53 @@ amdsmi_status_t amdsmi_get_link_metrics(amdsmi_processor_handle processor_handle (metric_info.xgmi_link_width != std::numeric_limits::max())) link_metrics->max_bandwidth = metric_info.xgmi_link_speed * metric_info.xgmi_link_width; + uint16_t link_to_dst_node[AMDSMI_MAX_NUM_XGMI_LINKS]; + std::fill_n(link_to_dst_node, AMDSMI_MAX_NUM_XGMI_LINKS, std::numeric_limits::max()); + status = rsmi_wrapper(rsmi_dev_xgmi_port_num_get, processor_handle, 0, + &link_metrics->num_links, link_to_dst_node); + for (unsigned int i = 0; i < AMDSMI_MAX_NUM_XGMI_LINKS; i++) { + memset(&link_metrics->links[i].bdf, 0xFF, sizeof(amdsmi_bdf_t)); + if (link_to_dst_node[i] != std::numeric_limits::max()) { + uint32_t node_id = link_to_dst_node[i]; + std::string node_symlink = "node" + std::to_string(node_id); + std::string sysfs_base = "/sys/bus/pci/devices/"; + DIR *dir = opendir(sysfs_base.c_str()); + if (dir) { + struct dirent *entry; + while ((entry = readdir(dir)) != nullptr) { + if (entry->d_type != DT_DIR && entry->d_type != DT_LNK) + continue; + std::string bdf = entry->d_name; + if (bdf == "." || bdf == "..") continue; + std::string symlink_path = sysfs_base + bdf + "/xgmi_hive_info/" + node_symlink; + char buf[PATH_MAX] = {0}; + ssize_t len = readlink(symlink_path.c_str(), buf, sizeof(buf)-1); + if (len > 0) { + buf[len] = '\0'; + std::string target(buf); + size_t last_slash = target.find_last_of('/'); + std::string bdf_str = (last_slash != std::string::npos) ? target.substr(last_slash + 1) : target; + // Parse BDF string: "dddd:bb:dd.f" + unsigned domain = 0, bus = 0, device = 0, function = 0; + if (sscanf(bdf_str.c_str(), "%4x:%2x:%2x.%1x", &domain, &bus, &device, &function) == 4) { + amdsmi_bdf_t dst_bdf = {}; + dst_bdf.domain_number = static_cast(domain); + dst_bdf.bus_number = static_cast(bus); + dst_bdf.device_number = static_cast(device); + dst_bdf.function_number = static_cast(function); + link_metrics->links[i].bdf = dst_bdf; + } + break; // Found, stop searching + } + } + closedir(dir); + } + } link_metrics->links[i].read = metric_info.xgmi_read_data_acc[i]; link_metrics->links[i].write = metric_info.xgmi_write_data_acc[i]; link_metrics->links[i].link_type = AMDSMI_LINK_TYPE_XGMI; } - return AMDSMI_STATUS_SUCCESS; }