diff --git a/projects/amdsmi/amdsmi_cli/amdsmi_commands.py b/projects/amdsmi/amdsmi_cli/amdsmi_commands.py index 866019ff6a7..d180e74c48a 100644 --- a/projects/amdsmi/amdsmi_cli/amdsmi_commands.py +++ b/projects/amdsmi/amdsmi_cli/amdsmi_commands.py @@ -9939,10 +9939,7 @@ def reset( self.logger.clear_multiple_devices_output() return if args.power_cap: - final_output = { - "ppt0": "N/A", - "ppt1": "N/A", - } + final_output = {"ppt0": "N/A", "ppt1": "N/A"} power_limit_types = {} for power_type in amdsmi_interface.AmdSmiPowerCapType: # Strip 'AMDSMI_POWER_CAP_TYPE_' prefix and convert to lowercase diff --git a/projects/amdsmi/example/amd_smi_drm_example.cc b/projects/amdsmi/example/amd_smi_drm_example.cc index 5efe4cbd63f..57436bf6b70 100644 --- a/projects/amdsmi/example/amd_smi_drm_example.cc +++ b/projects/amdsmi/example/amd_smi_drm_example.cc @@ -302,7 +302,7 @@ int main() { // Allocate the memory for the sockets std::vector sockets(socket_count); // Get the sockets of the system - ret = amdsmi_get_socket_handles(&socket_count, &sockets[0]); + ret = amdsmi_get_socket_handles(&socket_count, sockets.data()); CHK_AMDSMI_RET(ret) std::cout << "Total Socket: " << socket_count << std::endl; @@ -338,7 +338,7 @@ int main() { // Allocate the memory for the device handlers on the socket std::vector processor_handles(device_count); // Get all devices of the socket - ret = amdsmi_get_processor_handles(sockets[i], &device_count, &processor_handles[0]); + ret = amdsmi_get_processor_handles(sockets[i], &device_count, processor_handles.data()); PRINT_AMDSMI_RET(ret) std::cout << "\t**Processor Count: " << device_count << std::endl; @@ -434,7 +434,7 @@ int main() { // Allocate the memory for the device handlers on the socket std::vector processor_handles(device_count); // Get all devices of the socket - ret = amdsmi_get_processor_handles(sockets[i], &device_count, &processor_handles[0]); + ret = amdsmi_get_processor_handles(sockets[i], &device_count, processor_handles.data()); PRINT_AMDSMI_RET(ret) std::cout << "\t**Processor Count: " << device_count << std::endl; @@ -525,7 +525,7 @@ int main() { // Allocate the memory for the device handlers on the socket std::vector processor_handles(device_count); // Get all devices of the socket - ret = amdsmi_get_processor_handles(sockets[i], &device_count, &processor_handles[0]); + ret = amdsmi_get_processor_handles(sockets[i], &device_count, processor_handles.data()); PRINT_AMDSMI_RET(ret) std::cout << "\t**Processor Count: " << device_count << std::endl; @@ -649,7 +649,7 @@ int main() { // Allocate the memory for the device handlers on the socket std::vector processor_handles(device_count); // Get all devices of the socket - ret = amdsmi_get_processor_handles(sockets[i], &device_count, &processor_handles[0]); + ret = amdsmi_get_processor_handles(sockets[i], &device_count, processor_handles.data()); PRINT_AMDSMI_RET(ret) std::cout << "\t**Processor Count: " << device_count << std::endl; @@ -744,7 +744,7 @@ int main() { // Allocate the memory for the device handlers on the socket std::vector processor_handles(device_count); // Get all devices of the socket - ret = amdsmi_get_processor_handles(sockets[i], &device_count, &processor_handles[0]); + ret = amdsmi_get_processor_handles(sockets[i], &device_count, processor_handles.data()); PRINT_AMDSMI_RET(ret) std::cout << "\t**Processor Count: " << device_count << std::endl; @@ -819,7 +819,7 @@ int main() { // Allocate the memory for the device handlers on the socket std::vector processor_handles(device_count); // Get all devices of the socket - ret = amdsmi_get_processor_handles(sockets[i], &device_count, &processor_handles[0]); + ret = amdsmi_get_processor_handles(sockets[i], &device_count, processor_handles.data()); CHK_AMDSMI_RET(ret) std::cout << "Processor Count: " << device_count << std::endl; diff --git a/projects/amdsmi/example/amd_smi_nic.cc b/projects/amdsmi/example/amd_smi_nic.cc index 43836ba1f9c..04d198701f0 100644 --- a/projects/amdsmi/example/amd_smi_nic.cc +++ b/projects/amdsmi/example/amd_smi_nic.cc @@ -281,7 +281,7 @@ std::optional> get_nics() { uint32_t soc_count = 10; std::vector sockets(soc_count); // Get the sockets of the system - amdsmi_status_t status = amdsmi_get_socket_handles(&soc_count, &sockets[0]); + amdsmi_status_t status = amdsmi_get_socket_handles(&soc_count, sockets.data()); if (status != AMDSMI_STATUS_SUCCESS) { return std::nullopt; } diff --git a/projects/amdsmi/example/amd_smi_nodrm_example.cc b/projects/amdsmi/example/amd_smi_nodrm_example.cc index ccf5c1f5ed6..7f9de3bc942 100644 --- a/projects/amdsmi/example/amd_smi_nodrm_example.cc +++ b/projects/amdsmi/example/amd_smi_nodrm_example.cc @@ -64,7 +64,7 @@ int main() { // Allocate the memory for the sockets std::vector sockets(socket_count); // Get the sockets of the system - ret = amdsmi_get_socket_handles(&socket_count, &sockets[0]); + ret = amdsmi_get_socket_handles(&socket_count, sockets.data()); CHK_AMDSMI_RET(ret) std::cout << "Total Socket: " << socket_count << std::endl; @@ -85,7 +85,7 @@ int main() { // Allocate the memory for the device handlers on the socket std::vector processor_handles(device_count); // Get all devices of the socket - ret = amdsmi_get_processor_handles(sockets[i], &device_count, &processor_handles[0]); + ret = amdsmi_get_processor_handles(sockets[i], &device_count, processor_handles.data()); CHK_AMDSMI_RET(ret) // For each device of the socket, get name and temperature. diff --git a/projects/amdsmi/example/amdsmi_esmi_intg_example.cc b/projects/amdsmi/example/amdsmi_esmi_intg_example.cc index dbb48e459d7..d13a02e21ef 100644 --- a/projects/amdsmi/example/amdsmi_esmi_intg_example.cc +++ b/projects/amdsmi/example/amdsmi_esmi_intg_example.cc @@ -72,7 +72,7 @@ int main([[maybe_unused]] int argc, [[maybe_unused]] char** argv) { vector sockets(socket_count); // Get the sockets of the system - ret = amdsmi_get_socket_handles(&socket_count, &sockets[0]); + ret = amdsmi_get_socket_handles(&socket_count, sockets.data()); CHK_AMDSMI_RET(ret) cout << "Total Socket: " << socket_count << endl; @@ -92,7 +92,8 @@ int main([[maybe_unused]] int argc, [[maybe_unused]] char** argv) { vector plist(cpu_count); // Get the cpus for each socket - ret = amdsmi_get_processor_handles_by_type(sockets[i], processor_type, &plist[0], &cpu_count); + ret = + amdsmi_get_processor_handles_by_type(sockets[i], processor_type, plist.data(), &cpu_count); CHK_AMDSMI_RET(ret) // Set processor type as AMDSMI_PROCESSOR_TYPE_AMD_CPU_CORE @@ -104,7 +105,7 @@ int main([[maybe_unused]] int argc, [[maybe_unused]] char** argv) { vector core_list(core_count); // Get the cpu cores for each socket - ret = amdsmi_get_processor_handles_by_type(sockets[i], processor_type, &core_list[0], + ret = amdsmi_get_processor_handles_by_type(sockets[i], processor_type, core_list.data(), &core_count); CHK_AMDSMI_RET(ret) diff --git a/projects/amdsmi/py-interface/amdsmi_interface.py b/projects/amdsmi/py-interface/amdsmi_interface.py index bea09a7bf0e..a590cdaa93f 100644 --- a/projects/amdsmi/py-interface/amdsmi_interface.py +++ b/projects/amdsmi/py-interface/amdsmi_interface.py @@ -41,8 +41,8 @@ class MaxUIntegerTypes(IntEnum): UINT64_T = 0xFFFFFFFFFFFFFFFF -NO_OF_32BITS = sys.getsizeof(ctypes.c_uint32) * 8 -NO_OF_64BITS = sys.getsizeof(ctypes.c_uint64) * 8 +NO_OF_32BITS = ctypes.sizeof(ctypes.c_uint32) * 8 +NO_OF_64BITS = ctypes.sizeof(ctypes.c_uint64) * 8 KILO = math.pow(10, 3) AMDSMI_MAX_UTIL = 0xFFFFFFFF AMDSMI_MAX_PPT_LIMIT = 0xFFFFFFFF @@ -474,7 +474,7 @@ class AmdSmiEvtNotificationType(IntEnum): GPU_POST_RESET = amdsmi_wrapper.AMDSMI_EVT_NOTIF_GPU_POST_RESET MIGRATE_START = amdsmi_wrapper.AMDSMI_EVT_NOTIF_MIGRATE_START MIGRATE_END = amdsmi_wrapper.AMDSMI_EVT_NOTIF_MIGRATE_END - PAGE_FAULT_START = amdsmi_wrapper.AMDSMI_EVT_NOTIF_PAGE_FAULT_END + PAGE_FAULT_START = amdsmi_wrapper.AMDSMI_EVT_NOTIF_PAGE_FAULT_START PAGE_FAULT_END = amdsmi_wrapper.AMDSMI_EVT_NOTIF_PAGE_FAULT_END QUEUE_EVICTION = amdsmi_wrapper.AMDSMI_EVT_NOTIF_QUEUE_EVICTION QUEUE_RESTORE = amdsmi_wrapper.AMDSMI_EVT_NOTIF_QUEUE_RESTORE @@ -5148,7 +5148,7 @@ def amdsmi_set_gpu_clk_limit( clk_type_conversion = amdsmi_wrapper.AMDSMI_CLK_TYPE_DF else: raise AmdSmiParameterException(f"Unsupported clock type: {clk_type}", str) - + if limit_type.lower() == "min": limit_type_conversion = amdsmi_wrapper.CLK_LIMIT_MIN elif limit_type.lower() == "max": diff --git a/projects/amdsmi/rocm_smi/src/rocm_smi.cc b/projects/amdsmi/rocm_smi/src/rocm_smi.cc index 9ac5b927927..3f4f1a38d7e 100644 --- a/projects/amdsmi/rocm_smi/src/rocm_smi.cc +++ b/projects/amdsmi/rocm_smi/src/rocm_smi.cc @@ -7052,17 +7052,32 @@ rsmi_status_t rsmi_event_notification_init(uint32_t dv_ind) { int ret = ioctl(smi.kfd_notif_evt_fh(), AMDKFD_IOC_SMI_EVENTS, &args); if (ret < 0) { - return amd::smi::ErrnoToRsmiStatus(errno); + rsmi_status_t err = amd::smi::ErrnoToRsmiStatus(errno); + if (smi.kfd_notif_evt_fh_refcnt_dec() == 0) { + close(smi.kfd_notif_evt_fh()); + smi.set_kfd_notif_evt_fh(-1); + } + return err; } if (args.anon_fd < 1) { + if (smi.kfd_notif_evt_fh_refcnt_dec() == 0) { + close(smi.kfd_notif_evt_fh()); + smi.set_kfd_notif_evt_fh(-1); + } return RSMI_STATUS_NO_DATA; } dev->set_evt_notif_anon_fd(args.anon_fd); FILE* anon_file_ptr = fdopen(static_cast(args.anon_fd), "r"); if (anon_file_ptr == nullptr) { + rsmi_status_t err = amd::smi::ErrnoToRsmiStatus(errno); close(dev->evt_notif_anon_fd()); - return amd::smi::ErrnoToRsmiStatus(errno); + dev->set_evt_notif_anon_fd(-1); + if (smi.kfd_notif_evt_fh_refcnt_dec() == 0) { + close(smi.kfd_notif_evt_fh()); + smi.set_kfd_notif_evt_fh(-1); + } + return err; } dev->set_evt_notif_anon_file_ptr(anon_file_ptr); @@ -7177,7 +7192,7 @@ rsmi_status_t rsmi_event_notification_get(int timeout_ms, uint32_t* num_elem, char task_name[MAX_EVENT_NOTIFICATION_MSG_SIZE]; memset(task_name, '\0', MAX_EVENT_NOTIFICATION_MSG_SIZE); - sscanf(message, "%x:%s\n", &pid, task_name); + sscanf(message, "%x:%255s\n", &pid, task_name); std::stringstream final_message; final_message << "PID: " << std::to_string(pid).c_str() << " task name: " << task_name; @@ -7201,7 +7216,7 @@ rsmi_status_t rsmi_event_notification_get(int timeout_ms, uint32_t* num_elem, char reset_cause[MAX_EVENT_NOTIFICATION_MSG_SIZE]; memset(reset_cause, '\0', MAX_EVENT_NOTIFICATION_MSG_SIZE); - sscanf(message, "%x %[^\n]\n", &reset_seq_num, reset_cause); + sscanf(message, "%x %255[^\n]\n", &reset_seq_num, reset_cause); std::stringstream final_message; final_message << "reset sequence number: " << std::to_string(reset_seq_num).c_str() << " reset cause: " << reset_cause; @@ -7213,7 +7228,7 @@ rsmi_status_t rsmi_event_notification_get(int timeout_ms, uint32_t* num_elem, uint32_t reset_seq_num; char tmp[MAX_EVENT_NOTIFICATION_MSG_SIZE]; - sscanf(message, "%x %[^\n]\n", &reset_seq_num, tmp); + sscanf(message, "%x %255[^\n]\n", &reset_seq_num, tmp); std::stringstream final_message; final_message << "reset sequence number: " << std::to_string(reset_seq_num).c_str(); @@ -7356,7 +7371,7 @@ rsmi_status_t rsmi_event_notification_get(int timeout_ms, uint32_t* num_elem, case RSMI_EVT_NOTIF_EVENT_PROCESS_END: { uint32_t pid; char task[MAX_EVENT_NOTIFICATION_MSG_SIZE]; - int rc = sscanf(message, "%x %s", &pid, task); + int rc = sscanf(message, "%x %255s", &pid, task); std::stringstream msg; if (rc == 2) { msg << "PID: " << pid << " task: " << task; diff --git a/projects/amdsmi/rocm_smi/src/rocm_smi_binary_parser.cc b/projects/amdsmi/rocm_smi/src/rocm_smi_binary_parser.cc index 073bd6adf17..cde6149adf1 100644 --- a/projects/amdsmi/rocm_smi/src/rocm_smi_binary_parser.cc +++ b/projects/amdsmi/rocm_smi/src/rocm_smi_binary_parser.cc @@ -113,6 +113,7 @@ int present_pmmetrics(const char* fname, rsmi_name_value_t** kv, uint32_t* kvnum buf1 = reinterpret_cast(calloc(1, 65536)); if (!buf1) { + fclose(infile); return -1; } @@ -127,6 +128,8 @@ int present_pmmetrics(const char* fname, rsmi_name_value_t** kv, uint32_t* kvnum break; default: fprintf(stderr, "Metrics version %d not supported\n", pmmetrics_version); + fclose(infile); + free(buf1); return -1; } r = parse_pmmetric_table(buf1, table, len, kv, kvnum); diff --git a/projects/amdsmi/rocm_smi/src/rocm_smi_device.cc b/projects/amdsmi/rocm_smi/src/rocm_smi_device.cc index eb76a3051d5..e96a3acdee9 100644 --- a/projects/amdsmi/rocm_smi/src/rocm_smi_device.cc +++ b/projects/amdsmi/rocm_smi/src/rocm_smi_device.cc @@ -1778,7 +1778,7 @@ rsmi_status_t Device::isRestartInProgress(bool* isRestartInProgress, bool* isAMD if ((success == true) && (!out.empty())) { isSystemAMDGPUModuleLive = containsString(out, "live"); } - if (*isAMDGPUModuleLive) { + if (isSystemAMDGPUModuleLive) { deviceRestartInProgress = false; } *isRestartInProgress = deviceRestartInProgress; diff --git a/projects/amdsmi/rocm_smi/src/rocm_smi_kfd.cc b/projects/amdsmi/rocm_smi/src/rocm_smi_kfd.cc index 7d6374f6d5d..9ae918fbcdb 100644 --- a/projects/amdsmi/rocm_smi/src/rocm_smi_kfd.cc +++ b/projects/amdsmi/rocm_smi/src/rocm_smi_kfd.cc @@ -432,7 +432,7 @@ static int ReadKFDGpuId(uint32_t kfd_node_id, uint64_t* gpu_id) { return ENXIO; } - *gpu_id = static_cast(std::stoi(gpu_id_str)); + *gpu_id = std::stoull(gpu_id_str); return 0; } @@ -668,7 +668,7 @@ int GetProcessGPUs(uint32_t pid, std::unordered_set* gpu_set) { uint64_t val; try { - val = static_cast(std::stoi(tmp)); + val = std::stoull(tmp); } catch (...) { std::cerr << "Error; read invalid data: " << tmp << " from " << q_gpu_id_str << std::endl; closedir(queues_dir_hd); diff --git a/projects/amdsmi/rocm_smi/src/rocm_smi_monitor.cc b/projects/amdsmi/rocm_smi/src/rocm_smi_monitor.cc index c0925b06d04..47ac99e8aa1 100644 --- a/projects/amdsmi/rocm_smi/src/rocm_smi_monitor.cc +++ b/projects/amdsmi/rocm_smi/src/rocm_smi_monitor.cc @@ -458,6 +458,7 @@ static int get_supported_sensors(std::string dir_path, std::string fn_reg_ex, std::cout << "Regular expression error:" << std::endl; std::cout << e.what() << std::endl; std::cout << "Regex error code: " << e.code() << std::endl; + closedir(hwmon_dir); return -3; } return 0; diff --git a/projects/amdsmi/rocm_smi/src/rocm_smi_utils.cc b/projects/amdsmi/rocm_smi/src/rocm_smi_utils.cc index ebf33a1ff8b..a917afca5ca 100644 --- a/projects/amdsmi/rocm_smi/src/rocm_smi_utils.cc +++ b/projects/amdsmi/rocm_smi/src/rocm_smi_utils.cc @@ -623,7 +623,7 @@ std::pair executeCommand(std::string command, bool stdOut) { } // any return code other than 0, is a failed execution - if (pclose(pipe) != 0) { + if (pipe && pclose(pipe) != 0) { successfulRun = false; } @@ -681,7 +681,7 @@ rsmi_status_t storeTmpFile(uint32_t dv_ind, std::string parameterName, std::stri } // template for our file std::string fullTempFilePath = "/tmp/" + fullFileName + ".XXXXXX"; - char* fileName = &fullTempFilePath[0]; + char* fileName = fullTempFilePath.data(); int fd = mkstemp(fileName); if (fd == -1) { return RSMI_STATUS_FILE_ERROR; @@ -1101,8 +1101,7 @@ const char* my_fname(void) { dladdr(reinterpret_cast(my_fname), &dl_info); return (dl_info.dli_fname); #else - std::string emptyRet = ""; - return emptyRet.c_str(); + return ""; #endif } diff --git a/projects/amdsmi/src/amd_smi/amd_smi.cc b/projects/amdsmi/src/amd_smi/amd_smi.cc index 9631817053f..f07b08adf60 100644 --- a/projects/amdsmi/src/amd_smi/amd_smi.cc +++ b/projects/amdsmi/src/amd_smi/amd_smi.cc @@ -2761,7 +2761,7 @@ amdsmi_status_t amdsmi_get_gpu_event_notification(int timeout_ms, uint32_t* num_ // Get the rsmi data std::vector r_data(*num_elem); - rsmi_status_t r = rsmi_event_notification_get(timeout_ms, num_elem, &r_data[0]); + rsmi_status_t r = rsmi_event_notification_get(timeout_ms, num_elem, r_data.data()); if (r != RSMI_STATUS_SUCCESS) { return amd::smi::rsmi_to_amdsmi_status(r); } @@ -5611,7 +5611,7 @@ amdsmi_status_t amdsmi_get_processor_handle_from_bdf(amdsmi_bdf_t bdf, std::vector sockets(socket_count); - status = amdsmi_get_socket_handles(&socket_count, &sockets[0]); + status = amdsmi_get_socket_handles(&socket_count, sockets.data()); if (status != AMDSMI_STATUS_SUCCESS) { return status; } @@ -5634,7 +5634,7 @@ amdsmi_status_t amdsmi_get_processor_handle_from_bdf(amdsmi_bdf_t bdf, // Allocate the memory for the device handlers on the socket std::vector processor_handles(processor_count); // Get all processors of the socket - status = amdsmi_get_processor_handles(sockets[i], &processor_count, &processor_handles[0]); + status = amdsmi_get_processor_handles(sockets[i], &processor_count, processor_handles.data()); if (status != AMDSMI_STATUS_SUCCESS) { return status; } @@ -5716,8 +5716,8 @@ amdsmi_status_t amdsmi_get_link_topology_nearest(amdsmi_processor_handle process return api_status; } - amdsmi_socket_handle socket_list[socket_counter]; - if (auto api_status = amdsmi_get_socket_handles(&socket_counter, &socket_list[0]); + std::vector socket_list(socket_counter); + if (auto api_status = amdsmi_get_socket_handles(&socket_counter, socket_list.data()); (api_status != amdsmi_status_t::AMDSMI_STATUS_SUCCESS)) { return api_status; } @@ -7290,7 +7290,7 @@ amdsmi_status_t amdsmi_get_cpu_handles(uint32_t* cpu_count, // Allocate the memory for the sockets std::vector sockets(soc_count); // Get the sockets of the system - status = amdsmi_get_socket_handles(&soc_count, &sockets[0]); + status = amdsmi_get_socket_handles(&soc_count, sockets.data()); if (status != AMDSMI_STATUS_SUCCESS) return status; for (index = 0; index < soc_count; index++) { @@ -7303,7 +7303,7 @@ amdsmi_status_t amdsmi_get_cpu_handles(uint32_t* cpu_count, // Allocate the memory for the cpus std::vector plist(cpu_per_soc); // Get the cpus for each socket - status = amdsmi_get_processor_handles_by_type(sockets[index], processor_type, &plist[0], + status = amdsmi_get_processor_handles_by_type(sockets[index], processor_type, plist.data(), &cpu_per_soc); if (status != AMDSMI_STATUS_SUCCESS) return status; cpu_handles.insert(cpu_handles.end(), plist.begin(), plist.end()); @@ -7342,7 +7342,7 @@ amdsmi_status_t amdsmi_get_cpucore_handles(uint32_t* cores_count, // Allocate the memory for the sockets std::vector sockets(soc_count); // Get the sockets of the system - status = amdsmi_get_socket_handles(&soc_count, &sockets[0]); + status = amdsmi_get_socket_handles(&soc_count, sockets.data()); if (status != AMDSMI_STATUS_SUCCESS) return status; for (index = 0; index < soc_count; index++) { @@ -7354,7 +7354,7 @@ amdsmi_status_t amdsmi_get_cpucore_handles(uint32_t* cores_count, // Allocate the memory for the cores std::vector plist(cores_per_soc); // Get the coress for each socket - status = amdsmi_get_processor_handles_by_type(sockets[index], processor_type, &plist[0], + status = amdsmi_get_processor_handles_by_type(sockets[index], processor_type, plist.data(), &cores_per_soc); if (status != AMDSMI_STATUS_SUCCESS) { return status; diff --git a/projects/amdsmi/src/amd_smi/amd_smi_cper.cc b/projects/amdsmi/src/amd_smi/amd_smi_cper.cc index eaaf1463bdd..0409ad785d1 100644 --- a/projects/amdsmi/src/amd_smi/amd_smi_cper.cc +++ b/projects/amdsmi/src/amd_smi/amd_smi_cper.cc @@ -20,11 +20,10 @@ * THE SOFTWARE. */ -#include #include -#include #include +#include #include #include @@ -101,14 +100,15 @@ static auto amdsmi_read_cper_file(const std::string& filepath) -> CperFileCtx { ctx.file_size = file_stats.st_size; ctx.buffer = std::make_unique(ctx.file_size); - int file = open(filepath.c_str(), O_RDONLY); - if (file == -1) { - ss << __PRETTY_FUNCTION__ << "\n:" << __LINE__ << "[CPER] failed to open file: " << filepath - << ", errno:()" << errno << "): " << strerror(errno); + + std::ifstream file(filepath, std::ios::binary); + if (!file) { + ss << __PRETTY_FUNCTION__ << "\n:" << __LINE__ << "[CPER] failed to open file: " << filepath; LOG_ERROR(ss); return ctx; } - long bytes_read = read(file, ctx.buffer.get(), ctx.file_size); + file.read(ctx.buffer.get(), ctx.file_size); + long bytes_read = file.gcount(); if (bytes_read <= 0) { ss << __PRETTY_FUNCTION__ << "\n:" << __LINE__ << "[CPER] failed to read complete file, read only " << bytes_read << " of " @@ -116,7 +116,6 @@ static auto amdsmi_read_cper_file(const std::string& filepath) -> CperFileCtx { LOG_ERROR(ss); return ctx; } - close(file); ctx.status = AMDSMI_STATUS_SUCCESS; ctx.file_size = bytes_read; diff --git a/projects/amdsmi/src/amd_smi/amd_smi_system.cc b/projects/amdsmi/src/amd_smi/amd_smi_system.cc index 7e891aadf9c..04aa407c103 100644 --- a/projects/amdsmi/src/amd_smi/amd_smi_system.cc +++ b/projects/amdsmi/src/amd_smi/amd_smi_system.cc @@ -27,6 +27,7 @@ #include #include #include +#include #include #include #include @@ -238,7 +239,7 @@ amdsmi_status_t AMDSmiSystem::get_sys_num_of_cpu_sockets(uint32_t* sock_num) { std::vector AMDSmiSystem::get_cpu_sockets_from_numa_node(int32_t numa_node) { std::vector sockets; if (numa_node < 0) { - sockets[0] = std::numeric_limits::max(); + sockets.push_back(std::numeric_limits::max()); return sockets; } std::ifstream node_info("/sys/devices/system/node/node" + std::to_string(numa_node) + "/cpulist"); @@ -551,13 +552,13 @@ amdsmi_status_t AMDSmiSystem::populate_brcm_nic_devices() { std::string nicPath; if ((no_drm_nic_.get_device_path_by_index(i, &nicPath)) != AMDSMI_STATUS_SUCCESS) continue; std::string driverPath = nicPath + "/driver"; - std::string command = "readlink " + driverPath; - std::string getData; - if (smi_brcm_execute_cmd_get_data(command, &getData) != AMDSMI_STATUS_SUCCESS) continue; - if (getData.find("bnxt_en") == std::string::npos) continue; + std::error_code ec; + auto target = std::filesystem::read_symlink(driverPath, ec); + if (ec) continue; + if (target.string().find("bnxt_en") == std::string::npos) continue; socket->add_processor(device.get()); - nic_processors_.insert(deviceget()); + nic_processors_.insert(device.get()); device.release(); } #endif // BRCM_NIC diff --git a/projects/amdsmi/src/amd_smi/amd_smi_utils.cc b/projects/amdsmi/src/amd_smi/amd_smi_utils.cc index 00cf3fa43cb..47a6237353b 100644 --- a/projects/amdsmi/src/amd_smi/amd_smi_utils.cc +++ b/projects/amdsmi/src/amd_smi/amd_smi_utils.cc @@ -362,7 +362,7 @@ amdsmi_status_t smi_amdgpu_get_ranges(amd::smi::AMDSmiGPUDevice* device, amdsmi_ current_clk_type = PARSING_FCLK; continue; } - if (sscanf(line.c_str(), "%u: %d%s", &dpm_level, &freq, str) <= 2) { + if (sscanf(line.c_str(), "%u: %d%9s", &dpm_level, &freq, str) <= 2) { // skip lines that don't conform to the format continue; } @@ -398,7 +398,7 @@ amdsmi_status_t smi_amdgpu_get_ranges(amd::smi::AMDSmiGPUDevice* device, amdsmi_ char firstChar = line[0]; if (firstChar == 'S') { - if (sscanf(line.c_str(), "%c: %d%s", &single_char, &sleep_freq, str) <= 2) { + if (sscanf(line.c_str(), "%c: %d%9s", &single_char, &sleep_freq, str) <= 2) { ranges.close(); return AMDSMI_STATUS_NO_DATA; } @@ -422,7 +422,8 @@ amdsmi_status_t smi_amdgpu_get_ranges(amd::smi::AMDSmiGPUDevice* device, amdsmi_ current_freq = freq; } - // not * was detected so check for the min max if not sclk, mclk, or fclk, which are user defined + // not * was detected so check for the min max if not sclk, mclk, or fclk, which are user + // defined if (!sclk && !mclk && !fclk) { max = freq > max ? freq : max; min = freq < min ? freq : min; @@ -493,7 +494,7 @@ amdsmi_status_t smi_amdgpu_get_bad_page_info(amd::smi::AMDSmiGPUDevice* device, } if (badPagesVec.size() == 0) { - num_pages = 0; + *num_pages = 0; return AMDSMI_STATUS_SUCCESS; } // Remove any *trailing* empty (whitespace) lines @@ -594,10 +595,10 @@ amdsmi_status_t smi_amdgpu_get_ecc_error_count(amd::smi::AMDSmiGPUDevice* device std::string line; getline(f, line); - sscanf(line.c_str(), "%s%ld", str, &(err_cnt->uncorrectable_count)); + sscanf(line.c_str(), "%9s%ld", str, &(err_cnt->uncorrectable_count)); getline(f, line); - sscanf(line.c_str(), "%s%ld", str, &(err_cnt->correctable_count)); + sscanf(line.c_str(), "%9s%ld", str, &(err_cnt->correctable_count)); f.close(); @@ -861,7 +862,9 @@ amdsmi_status_t smi_brcm_execute_cmd_get_data(const std::string& command, std::s char buffer[128]; // Open a pipe to execute the command - std::shared_ptr pipe(popen(command.c_str(), "r"), pclose); + std::shared_ptr pipe(popen(command.c_str(), "r"), [](FILE* f) { + if (f) pclose(f); + }); if (!pipe) { return AMDSMI_STATUS_API_FAILED; } @@ -893,7 +896,7 @@ amdsmi_status_t smi_amdgpu_get_device_index(amdsmi_processor_handle processor_ha } // allocate memory sockets.resize(socket_count); - ret = amdsmi_get_socket_handles(&socket_count, &sockets[0]); + ret = amdsmi_get_socket_handles(&socket_count, sockets.data()); if (ret != AMDSMI_STATUS_SUCCESS) { return ret; } @@ -913,7 +916,7 @@ amdsmi_status_t smi_amdgpu_get_device_index(amdsmi_processor_handle processor_ha // Allocate the memory for the device handlers on the socket std::vector processor_handles(device_count); // Get all devices of the socket - ret = amdsmi_get_processor_handles(sockets[i], &device_count, &processor_handles[0]); + ret = amdsmi_get_processor_handles(sockets[i], &device_count, processor_handles.data()); ss << __PRETTY_FUNCTION__ << " | Processor Count: " << device_count << "\n"; LOG_DEBUG(ss); @@ -953,7 +956,7 @@ amdsmi_status_t smi_amdgpu_get_device_count(uint32_t* total_num_devices) { } // allocate memory sockets.resize(socket_count); - ret = amdsmi_get_socket_handles(&socket_count, &sockets[0]); + ret = amdsmi_get_socket_handles(&socket_count, sockets.data()); if (ret != AMDSMI_STATUS_SUCCESS) { return ret; } @@ -973,7 +976,7 @@ amdsmi_status_t smi_amdgpu_get_device_count(uint32_t* total_num_devices) { // Allocate the memory for the device handlers on the socket std::vector processor_handles(processor_count); // Get all devices of the socket - ret = amdsmi_get_processor_handles(sockets[i], &processor_count, &processor_handles[0]); + ret = amdsmi_get_processor_handles(sockets[i], &processor_count, processor_handles.data()); ss << __PRETTY_FUNCTION__ << " | Processor Count: " << processor_count << "\n"; LOG_DEBUG(ss); @@ -1023,7 +1026,7 @@ amdsmi_status_t smi_amdgpu_get_processor_handle_by_index( } // allocate memory sockets.resize(socket_count); - ret = amdsmi_get_socket_handles(&socket_count, &sockets[0]); + ret = amdsmi_get_socket_handles(&socket_count, sockets.data()); if (ret != AMDSMI_STATUS_SUCCESS) { return ret; } @@ -1043,7 +1046,7 @@ amdsmi_status_t smi_amdgpu_get_processor_handle_by_index( // Allocate the memory for the device handlers on the socket std::vector processor_handles(device_count); // Get all devices of the socket - ret = amdsmi_get_processor_handles(sockets[i], &device_count, &processor_handles[0]); + ret = amdsmi_get_processor_handles(sockets[i], &device_count, processor_handles.data()); ss << __PRETTY_FUNCTION__ << " | Processor Count: " << device_count << "\n"; LOG_DEBUG(ss); diff --git a/projects/amdsmi/src/amd_smi/fdinfo.cc b/projects/amdsmi/src/amd_smi/fdinfo.cc index 2544cb90b72..856ae42857a 100644 --- a/projects/amdsmi/src/amd_smi/fdinfo.cc +++ b/projects/amdsmi/src/amd_smi/fdinfo.cc @@ -158,7 +158,7 @@ amdsmi_status_t gpuvsmi_get_pid_info(const amdsmi_bdf_t& bdf, long int pid, char fd_bdf_str[13]; /* Only check against fdinfo files that contain a bdf */ - if (sscanf(bdfline.c_str(), "drm-pdev: %s", &fd_bdf_str[0]) != 1) continue; + if (sscanf(bdfline.c_str(), "drm-pdev: %12s", &fd_bdf_str[0]) != 1) continue; /* Populate amdsmi_proc_info_t struct only if the bdf in * the fdinfo file matches the passed bdf */ diff --git a/projects/amdsmi/src/nic/ai-nic/amdsmi_unified/src/smi_nic.cpp b/projects/amdsmi/src/nic/ai-nic/amdsmi_unified/src/smi_nic.cpp index a5f6f84163e..755e946715d 100644 --- a/projects/amdsmi/src/nic/ai-nic/amdsmi_unified/src/smi_nic.cpp +++ b/projects/amdsmi/src/nic/ai-nic/amdsmi_unified/src/smi_nic.cpp @@ -519,21 +519,25 @@ std::optional SmiNic::perm_address() const { } const std::string& port_iface = ports_[0].interface(); - struct ethtool_perm_addr permaddr; - permaddr.cmd = ETHTOOL_GPERMADDR; - permaddr.size = 6; - - int ret = smi_ethtool_ioctl(port_iface, &permaddr); + constexpr size_t addr_len = 6; + // ethtool_perm_addr has a flexible array member data[], so we must + // allocate extra space beyond the struct header for the address bytes. + std::vector buf(sizeof(struct ethtool_perm_addr) + addr_len, 0); + auto* permaddr = reinterpret_cast(buf.data()); + permaddr->cmd = ETHTOOL_GPERMADDR; + permaddr->size = addr_len; + + int ret = smi_ethtool_ioctl(port_iface, permaddr); if (ret != 0) { return std::nullopt; } - if (permaddr.size == 6) { + if (permaddr->size == addr_len) { std::stringstream ss; ss << std::hex << std::setfill('0'); - for (int i = 0; i < 6; i++) { + for (size_t i = 0; i < addr_len; i++) { if (i > 0) ss << ":"; - ss << std::setw(2) << static_cast(permaddr.data[i]); + ss << std::setw(2) << static_cast(permaddr->data[i]); } return ss.str(); } diff --git a/projects/amdsmi/src/nic/brcm-nic/amd_smi_lspci_commands.cc b/projects/amdsmi/src/nic/brcm-nic/amd_smi_lspci_commands.cc index 045ee6f521c..2b646e2a9d1 100644 --- a/projects/amdsmi/src/nic/brcm-nic/amd_smi_lspci_commands.cc +++ b/projects/amdsmi/src/nic/brcm-nic/amd_smi_lspci_commands.cc @@ -34,29 +34,42 @@ #include "amd_smi/impl/amd_smi_utils.h" +static bool is_valid_bdf(const std::string& bdf) { + // Validate BDF format: DDDD:BB:DD.F or BB:DD.F + static const std::regex bdf_re("^([0-9a-fA-F]{4}:)?[0-9a-fA-F]{2}:[0-9a-fA-F]{2}\\.[0-7]$"); + return std::regex_match(bdf, bdf_re); +} + amdsmi_status_t get_lspci_device_data(std::string bdf_str, std::string search_key, std::string& version) { + if (!is_valid_bdf(bdf_str)) { + std::ostringstream ss; + ss << __PRETTY_FUNCTION__ << " | Invalid BDF: " << bdf_str; + LOG_ERROR(ss); + return AMDSMI_STATUS_INVAL; + } + std::string lspci_data; - std::string command = "lspci -s " + bdf_str + " -vv | grep -i '" + search_key + "'"; + // Only pass the validated BDF to the shell; filter by search_key in C++ + std::string command = "lspci -s " + bdf_str + " -vv"; if (smi_brcm_execute_cmd_get_data(command, &lspci_data) != AMDSMI_STATUS_SUCCESS) { std::ostringstream ss; ss << __PRETTY_FUNCTION__ << " | " - << "Failed to execute command: lspci -s " << bdf_str << " -vv | grep -i " << search_key - << "."; + << "Failed to execute command: lspci -s " << bdf_str << " -vv"; LOG_ERROR(ss); return AMDSMI_STATUS_NOT_SUPPORTED; } - int pos = lspci_data.find(search_key); + auto pos = lspci_data.find(search_key); if (pos != std::string::npos) { - version = lspci_data.erase(0, lspci_data.find(search_key) + search_key.length()); - if (!version.empty() && version[version.length() - 1] == '\n') { - version.erase(version.length() - 1); - } - } else + auto value_start = pos + search_key.length(); + auto line_end = lspci_data.find('\n', value_start); + version = lspci_data.substr(value_start, line_end - value_start); + } else { version = "N/A"; + } return AMDSMI_STATUS_SUCCESS; } @@ -87,7 +100,7 @@ amdsmi_status_t get_lspci_root_switch(amdsmi_bdf_t device_bdf, amdsmi_bdf_t* swi while (std::getline(lines, line)) { if (line.find("LSI PCIe Switch management endpoint") != std::string::npos) { // get Bus - bus_pos = line.rfind(']----'); + bus_pos = line.rfind("]----"); if (bus_pos == std::string::npos) { // Check if the Bus position is not found, then continue to the next line continue; diff --git a/projects/amdsmi/tests/amd_smi_test/functional/computepartition_read_write.cc b/projects/amdsmi/tests/amd_smi_test/functional/computepartition_read_write.cc index d886f36e73f..3663353c524 100644 --- a/projects/amdsmi/tests/amd_smi_test/functional/computepartition_read_write.cc +++ b/projects/amdsmi/tests/amd_smi_test/functional/computepartition_read_write.cc @@ -202,7 +202,7 @@ static void checkPartitionIdChanges(amdsmi_processor_handle* const processor_han } // Allocate the memory for the device handlers on the socket std::vector curr_processor_handles(current_num_devices); - getProcessorHandles(&curr_processor_handles[0], current_num_devices); + getProcessorHandles(curr_processor_handles.data(), current_num_devices); if (current_partition == "SPX" || current_partition == "N/A") { max_loop = MAX_SPX_PARTITIONS; diff --git a/projects/amdsmi/tests/amd_smi_test/test_base.cc b/projects/amdsmi/tests/amd_smi_test/test_base.cc index c63b8eda13d..4b09675053c 100644 --- a/projects/amdsmi/tests/amd_smi_test/test_base.cc +++ b/projects/amdsmi/tests/amd_smi_test/test_base.cc @@ -128,7 +128,7 @@ void TestBase::SetUp(uint64_t init_flags) { // allocate memory sockets_.resize(socket_count_); - err = amdsmi_get_socket_handles(&socket_count_, &sockets_[0]); + err = amdsmi_get_socket_handles(&socket_count_, sockets_.data()); if (err != AMDSMI_STATUS_SUCCESS) { setup_failed_ = true; } @@ -146,7 +146,7 @@ void TestBase::SetUp(uint64_t init_flags) { ASSERT_EQ(err, AMDSMI_STATUS_SUCCESS); std::vector processor_handles(device_count); - err = amdsmi_get_processor_handles(sockets_[i], &device_count, &processor_handles[0]); + err = amdsmi_get_processor_handles(sockets_[i], &device_count, processor_handles.data()); if (err != AMDSMI_STATUS_SUCCESS) { setup_failed_ = true; }