From 9b5bbf555a399c35fad64b1b2913fd7f74cfae00 Mon Sep 17 00:00:00 2001 From: "Galantsev, Dmitrii" Date: Tue, 17 Jun 2025 16:47:25 -0500 Subject: [PATCH] DRM - Remove FD usage Change-Id: I77dfa778ccd0d39a03289c2e11cf10357566ff16 Signed-off-by: Galantsev, Dmitrii --- include/amd_smi/impl/amd_smi_drm.h | 3 +- include/amd_smi/impl/amd_smi_gpu_device.h | 6 +- include/amd_smi/impl/amd_smi_utils.h | 42 --------- include/amd_smi/impl/scoped_fd.h | 52 +++++++++++ src/CMakeLists.txt | 4 + src/amd_smi/amd_smi.cc | 28 +++--- src/amd_smi/amd_smi_drm.cc | 82 ++---------------- src/amd_smi/amd_smi_gpu_device.cc | 84 +----------------- src/amd_smi/amd_smi_utils.cc | 86 +------------------ src/amd_smi/scoped_fd.cc | 44 ++++++++++ .../functional/computepartition_read_write.cc | 14 --- .../functional/memorypartition_read_write.cc | 1 - tests/amd_smi_test/main.cc | 1 - 13 files changed, 128 insertions(+), 319 deletions(-) create mode 100644 include/amd_smi/impl/scoped_fd.h create mode 100644 src/amd_smi/scoped_fd.cc diff --git a/include/amd_smi/impl/amd_smi_drm.h b/include/amd_smi/impl/amd_smi_drm.h index 0675ee6604..d7fbfd219f 100644 --- a/include/amd_smi/impl/amd_smi_drm.h +++ b/include/amd_smi/impl/amd_smi_drm.h @@ -34,6 +34,7 @@ #include "amd_smi/impl/amd_smi_lib_loader.h" #include "amd_smi/impl/amdgpu_drm.h" #include "amd_smi/impl/xf86drm.h" +#include "amd_smi/impl/scoped_fd.h" namespace amd { namespace smi { @@ -42,7 +43,6 @@ class AMDSmiDrm { public: amdsmi_status_t init(); amdsmi_status_t cleanup(); - amdsmi_status_t get_drm_fd_by_index(uint32_t gpu_index, uint32_t *fd_info) const; amdsmi_status_t get_bdf_by_index(uint32_t gpu_index, amdsmi_bdf_t *bdf_info) const; amdsmi_status_t get_drm_path_by_index(uint32_t gpu_index, std::string *drm_path) const; std::vector get_bdfs(); @@ -54,7 +54,6 @@ class AMDSmiDrm { // when file is not found, the empty string will be returned std::string find_file_in_folder(const std::string& folder, const std::string& regex); - std::vector drm_fds_; // drm file descriptor by gpu_index std::vector drm_paths_; // drm path (renderD128 for example) std::vector drm_bdfs_; // bdf uint32_t vendor_id; diff --git a/include/amd_smi/impl/amd_smi_gpu_device.h b/include/amd_smi/impl/amd_smi_gpu_device.h index ff62c1e194..ed1258b97a 100644 --- a/include/amd_smi/impl/amd_smi_gpu_device.h +++ b/include/amd_smi/impl/amd_smi_gpu_device.h @@ -46,8 +46,8 @@ enum class ComputeProcessListType_t : ComputeProcessListClassType_t class AMDSmiGPUDevice: public AMDSmiProcessor { public: - AMDSmiGPUDevice(uint32_t gpu_id, uint32_t fd, std::string path, amdsmi_bdf_t bdf, AMDSmiDrm& drm): - AMDSmiProcessor(AMDSMI_PROCESSOR_TYPE_AMD_GPU), gpu_id_(gpu_id), fd_(fd), path_(path), bdf_(bdf), drm_(drm) {} + AMDSmiGPUDevice(uint32_t gpu_id, std::string path, amdsmi_bdf_t bdf, AMDSmiDrm& drm): + AMDSmiProcessor(AMDSMI_PROCESSOR_TYPE_AMD_GPU), gpu_id_(gpu_id), path_(path), bdf_(bdf), drm_(drm) {} AMDSmiGPUDevice(uint32_t gpu_id, AMDSmiDrm& drm): AMDSmiProcessor(AMDSMI_PROCESSOR_TYPE_AMD_GPU), gpu_id_(gpu_id), drm_(drm) { @@ -59,7 +59,6 @@ class AMDSmiGPUDevice: public AMDSmiProcessor { amdsmi_status_t get_drm_data(); pthread_mutex_t* get_mutex(); uint32_t get_gpu_id() const; - uint32_t get_gpu_fd() const; uint32_t get_card_id(); // -e feature + we can get card_id for our internal functions uint32_t get_drm_render_minor(); // -e feature + we can get card_id for our internal functions uint64_t get_kfd_gpu_id(); // Used to decode vram usage for KFD processes @@ -76,7 +75,6 @@ class AMDSmiGPUDevice: public AMDSmiProcessor { private: uint32_t gpu_id_; - uint32_t fd_; std::string path_; amdsmi_bdf_t bdf_; uint32_t vendor_id_; diff --git a/include/amd_smi/impl/amd_smi_utils.h b/include/amd_smi/impl/amd_smi_utils.h index 818ba642ec..0b209b1fef 100644 --- a/include/amd_smi/impl/amd_smi_utils.h +++ b/include/amd_smi/impl/amd_smi_utils.h @@ -60,20 +60,6 @@ std::string smi_amdgpu_get_status_string(amdsmi_status_t ret, bool fullStatus); amdsmi_status_t smi_clear_char_and_reinitialize(char buffer[], uint32_t len, std::string newString); -/** - * @brief Wait for user input, a debugging function to pause the program - * - * @details This function will wait for user input before continuing. - * It is useful for debugging purposes to allow the user to inspect the state of the program - * before it continues. The function will print a message to the console and then wait for - * the user to press Enter. Once Enter is pressed, the function will return and the program - * will continue executing. - * - * @note This function is intended for debugging purposes only and should not be used in production code. - * It will block the program execution and cause it to wait indefinitely for user input. - */ -void amdsmi_wait_for_user_input(void); - /** * @brief Get the device index given the processor handle. * @@ -188,31 +174,3 @@ void fill_2d_array(A& arr, T value) { } #endif // AMD_SMI_INCLUDE_AMD_SMI_UTILS_H_ - -#ifndef SCOPED_FD_H -#define SCOPED_FD_H - -class ScopedFD { -private: - int fd_; - std::string path_; - -public: - ScopedFD(const std::string& path, int flags); - ~ScopedFD(); - - // Non-copyable - ScopedFD(const ScopedFD&) = delete; - ScopedFD& operator=(const ScopedFD&) = delete; - - // Movable - ScopedFD(ScopedFD&& other) noexcept; - ScopedFD& operator=(ScopedFD&& other) noexcept; - - int get() const; - bool valid() const; - operator int() const; // Allows direct use as int - int operator*() const; // Allows *fd usage like your current code -}; - -#endif // SCOPED_FD_H diff --git a/include/amd_smi/impl/scoped_fd.h b/include/amd_smi/impl/scoped_fd.h new file mode 100644 index 0000000000..1d188925f2 --- /dev/null +++ b/include/amd_smi/impl/scoped_fd.h @@ -0,0 +1,52 @@ +/* + * Copyright (c) Advanced Micro Devices, Inc. All rights reserved. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +#ifndef AMD_SMI_INCLUDE_IMPL_SCOPED_FD_H_ +#define AMD_SMI_INCLUDE_IMPL_SCOPED_FD_H_ + +#include + +#include + +class ScopedFD { + private: + int fd_; + std::string path_; + + public: + ScopedFD(const std::string& path, int flags); + ~ScopedFD(); + + // Non-copyable + ScopedFD(const ScopedFD&) = delete; + ScopedFD& operator=(const ScopedFD&) = delete; + + // Movable + ScopedFD(ScopedFD&& other) noexcept; + ScopedFD& operator=(ScopedFD&& other) noexcept; + + int get() const; + bool valid() const; + operator int() const; // Allows direct use as int +}; + +#endif // AMD_SMI_INCLUDE_IMPL_SCOPED_FD_H_ diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 2f22b7d854..2b60b23247 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -25,6 +25,7 @@ set(SRC_LIST "${SRC_DIR}/amd_smi_system.cc" "${SRC_DIR}/amd_smi_utils.cc" "${SRC_DIR}/amd_smi_uuid.cc" + "${SRC_DIR}/scoped_fd.cc" "${SRC_DIR}/fdinfo.cc" "${CMN_SRC_LIST}") set(INC_LIST @@ -37,6 +38,9 @@ set(INC_LIST "${INC_DIR}/impl/amd_smi_lib_loader.h" "${INC_DIR}/impl/amd_smi_socket.h" "${INC_DIR}/impl/amd_smi_system.h" + "${INC_DIR}/impl/amd_smi_utils.h" + "${INC_DIR}/impl/amd_smi_uuid.h" + "${INC_DIR}/impl/scoped_fd.h" "${PROJECT_SOURCE_DIR}/rocm_smi/include/rocm_smi/rocm_smi.h" "${PROJECT_SOURCE_DIR}/rocm_smi/include/rocm_smi/rocm_smi_utils.h") diff --git a/src/amd_smi/amd_smi.cc b/src/amd_smi/amd_smi.cc index 7dc92fffda..979243da73 100644 --- a/src/amd_smi/amd_smi.cc +++ b/src/amd_smi/amd_smi.cc @@ -905,7 +905,7 @@ amdsmi_status_t amdsmi_get_gpu_vram_usage(amdsmi_processor_handle processor_hand std::string path = "/dev/dri/" + render_name; ScopedFD drm_fd(path.c_str(), O_RDWR | O_CLOEXEC); - if (!drm_fd) { + if (!drm_fd.valid()) { ss << __PRETTY_FUNCTION__ << " | Failed to open " << path << ": " << strerror(errno) << "; Returning: " << smi_amdgpu_get_status_string(AMDSMI_STATUS_FILE_ERROR, false); @@ -962,7 +962,7 @@ amdsmi_status_t amdsmi_get_gpu_vram_usage(amdsmi_processor_handle processor_hand request.return_pointer = reinterpret_cast(&vram_used); request.return_size = sizeof(vram_used); request.query = AMDGPU_INFO_VRAM_USAGE; - auto drm_write = drmCommandWrite(*drm_fd, DRM_AMDGPU_INFO, &request, + auto drm_write = drmCommandWrite(drm_fd, DRM_AMDGPU_INFO, &request, sizeof(struct drm_amdgpu_info)); if (drm_write != 0) { libdrm.unload(); @@ -1662,7 +1662,7 @@ amdsmi_get_gpu_asic_info(amdsmi_processor_handle processor_handle, amdsmi_asic_i } std::string path = "/dev/dri/" + render_name; ScopedFD drm_fd(path.c_str(), O_RDWR | O_CLOEXEC); - if (!drm_fd) { + if (!drm_fd.valid()) { ss << __PRETTY_FUNCTION__ << " | Failed to open " << path << ": " << strerror(errno) << "; Returning: " << smi_amdgpu_get_status_string(AMDSMI_STATUS_FILE_ERROR, false); @@ -1706,7 +1706,7 @@ amdsmi_get_gpu_asic_info(amdsmi_processor_handle processor_handle, amdsmi_asic_i request.return_pointer = reinterpret_cast(&dev_info); request.return_size = sizeof(struct drm_amdgpu_info_device); request.query = AMDGPU_INFO_DEV_INFO; - auto drm_write = drmCommandWrite(*drm_fd, DRM_AMDGPU_INFO, &request, + auto drm_write = drmCommandWrite(drm_fd, DRM_AMDGPU_INFO, &request, sizeof(struct drm_amdgpu_info)); if (drm_write != 0) { libdrm.unload(); @@ -1885,7 +1885,7 @@ amdsmi_status_t amdsmi_get_gpu_vram_info( } ScopedFD drm_fd(path.c_str(), O_RDWR | O_CLOEXEC); - if (!drm_fd) { + if (!drm_fd.valid()) { ss << __PRETTY_FUNCTION__ << " | Failed to open " << path << ": " << strerror(errno) << "; Returning: " << smi_amdgpu_get_status_string(AMDSMI_STATUS_FILE_ERROR, false); @@ -1936,7 +1936,7 @@ amdsmi_status_t amdsmi_get_gpu_vram_info( request.return_pointer = reinterpret_cast(&dev_info); request.return_size = sizeof(struct drm_amdgpu_info_device); request.query = AMDGPU_INFO_DEV_INFO; - auto drm_write = drmCommandWrite(*drm_fd, DRM_AMDGPU_INFO, &request, + auto drm_write = drmCommandWrite(drm_fd, DRM_AMDGPU_INFO, &request, sizeof(struct drm_amdgpu_info)); if (drm_write != 0) { libdrm.unload(); @@ -3743,7 +3743,7 @@ amdsmi_get_gpu_vbios_info(amdsmi_processor_handle processor_handle, amdsmi_vbios } ScopedFD drm_fd(path.c_str(), O_RDWR | O_CLOEXEC); - if (!drm_fd) { + if (!drm_fd.valid()) { ss << __PRETTY_FUNCTION__ << " | Failed to open " << path << ": " << strerror(errno) << "; Returning: " << smi_amdgpu_get_status_string(AMDSMI_STATUS_FILE_ERROR, false); @@ -3794,7 +3794,7 @@ amdsmi_get_gpu_vbios_info(amdsmi_processor_handle processor_handle, amdsmi_vbios request.return_size = sizeof(drm_amdgpu_info_vbios); request.query = AMDGPU_INFO_VBIOS; request.vbios_info.type = AMDGPU_INFO_VBIOS_INFO; - auto drm_write = drmCommandWrite(*drm_fd, DRM_AMDGPU_INFO, &request, + auto drm_write = drmCommandWrite(drm_fd, DRM_AMDGPU_INFO, &request, sizeof(struct drm_amdgpu_info)); if (drm_write == 0) { @@ -4306,7 +4306,7 @@ amdsmi_status_t amdsmi_get_gpu_driver_info(amdsmi_processor_handle processor_han } ScopedFD drm_fd(path.c_str(), O_RDWR | O_CLOEXEC); - if (!drm_fd) { + if (!drm_fd.valid()) { ss << __PRETTY_FUNCTION__ << " | Failed to open " << path << ": " << strerror(errno) << "; Returning: " << smi_amdgpu_get_status_string(AMDSMI_STATUS_FILE_ERROR, false); @@ -4353,7 +4353,7 @@ amdsmi_status_t amdsmi_get_gpu_driver_info(amdsmi_processor_handle processor_han // Get the driver date std::string driver_date; - auto version = drm_get_version(*drm_fd); + auto version = drm_get_version(drm_fd); if (version == nullptr) { libdrm.unload(); ss << __PRETTY_FUNCTION__ @@ -4782,7 +4782,7 @@ amdsmi_get_gpu_virtualization_mode(amdsmi_processor_handle processor_handle, } ScopedFD drm_fd(path.c_str(), O_RDWR | O_CLOEXEC); - if (!drm_fd) { + if (!drm_fd.valid()) { ss << __PRETTY_FUNCTION__ << " | Failed to open " << path << ": " << strerror(errno) << "; Returning: " << smi_amdgpu_get_status_string(AMDSMI_STATUS_FILE_ERROR, false); @@ -4832,7 +4832,7 @@ amdsmi_get_gpu_virtualization_mode(amdsmi_processor_handle processor_handle, } // get drm version. If it's older than 3.62.0, then say not supported and exit. - auto drm_version = drm_get_version(*drm_fd); + auto drm_version = drm_get_version(drm_fd); // minimum version that supports getting of virtualization mode int major_version = 3; int minor_version = 62; @@ -4886,10 +4886,10 @@ amdsmi_get_gpu_virtualization_mode(amdsmi_processor_handle processor_handle, request.return_pointer = reinterpret_cast(&dev_info); request.return_size = sizeof(struct drm_amdgpu_info_device); request.query = AMDGPU_INFO_DEV_INFO; - auto drm_write = drmCommandWrite(*drm_fd, DRM_AMDGPU_INFO, &request, + auto drm_write = drmCommandWrite(drm_fd, DRM_AMDGPU_INFO, &request, sizeof(struct drm_amdgpu_info)); ss << __PRETTY_FUNCTION__ - << " | drm_fd: " << std::dec << *drm_fd << "\n" + << " | drm_fd: " << std::dec << drm_fd << "\n" << " | path: " << path << "\n" << " | drmCommandWrite: " << drm_write << "\n" << " | drmCommandWrite returned: " << strerror(errno) << "\n" diff --git a/src/amd_smi/amd_smi_drm.cc b/src/amd_smi/amd_smi_drm.cc index 40078a5777..764c2017be 100644 --- a/src/amd_smi/amd_smi_drm.cc +++ b/src/amd_smi/amd_smi_drm.cc @@ -27,9 +27,9 @@ #include #include #include -#include "amd_smi/impl/amd_smi_utils.h" #include "amd_smi/impl/amd_smi_drm.h" #include "amd_smi/impl/amd_smi_common.h" +#include "impl/scoped_fd.h" #include "rocm_smi/rocm_smi.h" #include "rocm_smi/rocm_smi_main.h" #include "rocm_smi/rocm_smi_utils.h" @@ -59,8 +59,6 @@ std::string AMDSmiDrm::find_file_in_folder(const std::string& folder, } amdsmi_status_t AMDSmiDrm::init() { - std::ostringstream ss; - amdsmi_status_t status = lib_loader_.load("libdrm.so.2"); if (status != AMDSMI_STATUS_SUCCESS) { return status; @@ -121,7 +119,6 @@ amdsmi_status_t AMDSmiDrm::init() { bool has_valid_fds = false; for (uint32_t i=0; i < devices.size(); i++) { auto rocm_smi_device = devices[i]; - std::string render_file_name; drmDevicePtr device; const std::string regex("renderD([0-9]+)"); @@ -134,70 +131,27 @@ amdsmi_status_t AMDSmiDrm::init() { ScopedFD fd(name.c_str(), O_RDWR | O_CLOEXEC); amdsmi_bdf_t bdf; - if (*fd >= 0) { - auto version = drm_get_version(*fd); - if (drm_get_device(*fd, &device) != 0) { + if (fd.valid()) { + auto version = drm_get_version(fd); + if (drm_get_device(fd, &device) != 0) { drm_free_device(&device); } - ss << __PRETTY_FUNCTION__ << " | " - << " render file name: " << name << "\n" - << "; fd: " << std::dec << *fd << "\n" - << "; drm version->name: " << version->name << "\n" - << "; drm version->date: " << version->date << "\n" - << "; drm version_major.version_minor.version_patchlevel: " - << std::dec << version->version_major << "." - << version->version_minor << "." - << version->version_patchlevel << "\n" - << "; device->deviceinfo.pci->vendor_id: 0x" - << std::hex << std::setfill('0') << std::setw(4) - << static_cast(device->deviceinfo.pci->vendor_id) << "\n" - << "; device->deviceinfo.pci->device_id: 0x" - << std::hex << std::setfill('0') << std::setw(4) - << static_cast(device->deviceinfo.pci->device_id) << "\n" - << "; device->deviceinfo.pci->revision_id: 0x" - << std::hex << std::setfill('0') << std::setw(4) - << static_cast(device->deviceinfo.pci->revision_id) << "\n" - << "; device->deviceinfo.pci->subdevice_id: 0x" - << std::hex << std::setfill('0') << std::setw(4) - << static_cast(device->deviceinfo.pci->subdevice_id) << "\n"; - LOG_INFO(ss); drm_free_version(version); + has_valid_fds = true; } - drm_fds_.push_back(*fd); drm_paths_.push_back(render_name); // even if fail, still add to prevent mismatch the index - if (*fd < 0) { + if (!has_valid_fds) { drm_bdfs_.push_back(bdf); drm_free_device(&device); continue; } - has_valid_fds = true; - std::ostringstream ss; uint64_t bdf_rocm = 0; rsmi_dev_pci_id_get(i, &bdf_rocm); vendor_id = device->deviceinfo.pci->vendor_id; - std::ostringstream bdf_sstream; - bdf_sstream << std::hex << std::setfill('0') << std::setw(4) - << ((bdf_rocm >> 32) & 0xFFFFFFFF) << ":" // DOMAIN - << std::hex << std::setfill('0') << std::setw(2) - << ((bdf_rocm >> 8) & 0xFF) << ":" // BUS - << std::hex << std::setfill('0') << std::setw(2) - << ((bdf_rocm >> 3) & 0x1F) << "." // DEVICE - << std::hex << std::setfill('0') << +(bdf_rocm & 0x7); // FUNCTION - bdf_sstream << "\n[Option 1] Partition ID ((pci_id >> 28) & 0xf): " << std::dec - << static_cast((bdf_rocm >> 28) & 0xf); - bdf_sstream << "\n[Option 2] Partition ID (pci_id & 0x7): " << std::dec - << static_cast(bdf_rocm & 0x7); - ss << __PRETTY_FUNCTION__ << " | " - << "bdf_rocm | Received bdf: " - << "\nWhole BDF: " << amd::smi::print_unsigned_hex_and_int(bdf_rocm) - << "\nBDF = " - << bdf_sstream.str() << "\n" - << "; Vendor ID: 0x" << std::hex << std::setfill('0') << std::setw(4) << vendor_id; - LOG_INFO(ss); bdf.domain_number = static_cast(((bdf_rocm >> 32) & 0xFFFFFFFF)); bdf.bus_number = static_cast(((bdf_rocm >> 8) & 0xFF)); @@ -217,41 +171,17 @@ amdsmi_status_t AMDSmiDrm::init() { } amdsmi_status_t AMDSmiDrm::cleanup() { - for (unsigned int i=0; i < drm_fds_.size(); i++) { - close(drm_fds_[i]); - } - - if (!drm_fds_.empty()) {drm_fds_.clear();} if (!drm_paths_.empty()) {drm_paths_.clear();} if (!drm_bdfs_.empty()) {drm_bdfs_.clear();} lib_loader_.unload(); return AMDSMI_STATUS_SUCCESS; } -amdsmi_status_t AMDSmiDrm::get_drm_fd_by_index(uint32_t gpu_index, uint32_t *fd_info) const { - if (gpu_index + 1 > drm_fds_.size()) return AMDSMI_STATUS_NOT_SUPPORTED; - if (drm_fds_[gpu_index] < 0 ) return AMDSMI_STATUS_NOT_SUPPORTED; - *fd_info = drm_fds_[gpu_index]; - return AMDSMI_STATUS_SUCCESS; -} - amdsmi_status_t AMDSmiDrm::get_bdf_by_index(uint32_t gpu_index, amdsmi_bdf_t *bdf_info) const { - std::ostringstream ss; if (gpu_index + 1 > drm_bdfs_.size()) { - ss << __PRETTY_FUNCTION__ << " | gpu_index = " << gpu_index - << "; \nReturning = AMDSMI_STATUS_NOT_SUPPORTED"; - LOG_INFO(ss); - // std::cout << ss.str() << std::endl; return AMDSMI_STATUS_NOT_SUPPORTED; } *bdf_info = drm_bdfs_[gpu_index]; - ss << __PRETTY_FUNCTION__ << " | gpu_index = " << gpu_index - << "; \nreceived bdf: Domain = " << bdf_info->domain_number - << "; \nBus# = " << bdf_info->bus_number - << "; \nDevice# = " << bdf_info->device_number - << "; \nFunction# = " << bdf_info->function_number - << "\nReturning = AMDSMI_STATUS_SUCCESS"; - LOG_INFO(ss); return AMDSMI_STATUS_SUCCESS; } diff --git a/src/amd_smi/amd_smi_gpu_device.cc b/src/amd_smi/amd_smi_gpu_device.cc index 637c5fb845..f7bedf8775 100644 --- a/src/amd_smi/amd_smi_gpu_device.cc +++ b/src/amd_smi/amd_smi_gpu_device.cc @@ -20,16 +20,12 @@ * THE SOFTWARE. */ -#include -#include #include #include #include #include #include "amd_smi/impl/amd_smi_gpu_device.h" -#include "amd_smi/impl/amd_smi_common.h" -#include "amd_smi/impl/amd_smi_utils.h" #include "amd_smi/impl/fdinfo.h" #include "rocm_smi/rocm_smi_kfd.h" #include "rocm_smi/rocm_smi_utils.h" @@ -43,7 +39,6 @@ uint32_t AMDSmiGPUDevice::get_gpu_id() const { } uint32_t AMDSmiGPUDevice::get_card_id() { - std::ostringstream ss; // Should never return not_supported, but just in case rsmi_status_t ret = rsmi_status_t::RSMI_STATUS_NOT_SUPPORTED; uint32_t gpu_index = this->get_gpu_id(); @@ -55,24 +50,10 @@ uint32_t AMDSmiGPUDevice::get_card_id() { this->card_index_ = identifiers.card_index; } - ss << __PRETTY_FUNCTION__ - << " | rsmi_dev_identifiers_get status: " << getRSMIStatusString(ret, false) << "\n" - << " | gpu_id_: " << gpu_id_ << "\n" - << " | identifiers.card_index: " << identifiers.card_index << "\n" - << " | identifiers.drm_render_minor: " << identifiers.drm_render_minor << "\n" - << " | identifiers.bdfid: " << std::hex << "0x" << identifiers.bdfid << "\n" - << " | identifiers.kfd_gpu_id: " << std::dec << identifiers.kfd_gpu_id << "\n" - << " | identifiers.partition_id: " << identifiers.partition_id << "\n" - << " | identifiers.smi_device_id: " << identifiers.smi_device_id << "\n" - << " | returning card_index_: " - << this->card_index_ << std::endl; - // std::cout << ss.str(); - LOG_DEBUG(ss); return this->card_index_; } uint32_t AMDSmiGPUDevice::get_drm_render_minor() { - std::ostringstream ss; // Should never return not_supported, but just in case rsmi_status_t ret = rsmi_status_t::RSMI_STATUS_NOT_SUPPORTED; uint32_t gpu_index = this->get_gpu_id(); @@ -84,24 +65,10 @@ uint32_t AMDSmiGPUDevice::get_drm_render_minor() { this->drm_render_minor_ = identifiers.drm_render_minor; } - ss << __PRETTY_FUNCTION__ - << " | rsmi_dev_identifiers_get status: " << getRSMIStatusString(ret, false) << "\n" - << " | gpu_id_: " << gpu_id_ << "\n" - << " | identifiers.card_index: " << identifiers.card_index << "\n" - << " | identifiers.drm_render_minor: " << identifiers.drm_render_minor << "\n" - << " | identifiers.bdfid: " << std::hex << "0x" << identifiers.bdfid << "\n" - << " | identifiers.kfd_gpu_id: " << std::dec << identifiers.kfd_gpu_id << "\n" - << " | identifiers.partition_id: " << identifiers.partition_id << "\n" - << " | identifiers.smi_device_id: " << identifiers.smi_device_id << "\n" - << " | returning drm_render_minor_: " - << this->drm_render_minor_ << std::endl; - // std::cout << ss.str(); - LOG_DEBUG(ss); return this->drm_render_minor_; } uint64_t AMDSmiGPUDevice::get_kfd_gpu_id() { - std::ostringstream ss; // Should never return not_supported, but just in case rsmi_status_t ret = rsmi_status_t::RSMI_STATUS_NOT_SUPPORTED; uint32_t gpu_index = this->get_gpu_id(); @@ -113,26 +80,9 @@ uint64_t AMDSmiGPUDevice::get_kfd_gpu_id() { this->kfd_gpu_id_ = identifiers.kfd_gpu_id; } - ss << __PRETTY_FUNCTION__ - << " | rsmi_dev_identifiers_get status: " << getRSMIStatusString(ret, false) << "\n" - << " | gpu_id_: " << gpu_id_ << "\n" - << " | identifiers.card_index: " << identifiers.card_index << "\n" - << " | identifiers.drm_render_minor: " << identifiers.drm_render_minor << "\n" - << " | identifiers.bdfid: " << std::hex << "0x" << identifiers.bdfid << "\n" - << " | identifiers.kfd_gpu_id: " << std::dec << identifiers.kfd_gpu_id << "\n" - << " | identifiers.partition_id: " << identifiers.partition_id << "\n" - << " | identifiers.smi_device_id: " << identifiers.smi_device_id << "\n" - << " | returning kfd_gpu_id_: " - << this->kfd_gpu_id_ << std::endl; - // std::cout << ss.str(); - LOG_DEBUG(ss); return this->kfd_gpu_id_; } -uint32_t AMDSmiGPUDevice::get_gpu_fd() const { - return fd_; -} - std::string& AMDSmiGPUDevice::get_gpu_path() { return path_; } @@ -147,38 +97,14 @@ uint32_t AMDSmiGPUDevice::get_vendor_id() { amdsmi_status_t AMDSmiGPUDevice::get_drm_data() { amdsmi_status_t ret; - uint32_t fd = 0; std::string path; amdsmi_bdf_t bdf; - std::ostringstream ss; - ret = drm_.get_drm_fd_by_index(gpu_id_, &fd); - ss << __PRETTY_FUNCTION__ << " | gpu_id_: " << gpu_id_ - << "; fd: " << fd - << "; drm_.get_drm_fd_by_index(gpu_id_, &fd): " - << smi_amdgpu_get_status_string(ret, false) << std::endl; - // std::cout << ss.str(); - LOG_DEBUG(ss); - if (ret != AMDSMI_STATUS_SUCCESS) return AMDSMI_STATUS_NOT_SUPPORTED; ret = drm_.get_drm_path_by_index(gpu_id_, &path); - ss << __PRETTY_FUNCTION__ << " | gpu_id_: " << gpu_id_ - << "; path: " << path - << "; drm_.get_drm_fd_by_index(gpu_id_, &path): " - << smi_amdgpu_get_status_string(ret, false) << std::endl; - // std::cout << ss.str(); - LOG_DEBUG(ss); if (ret != AMDSMI_STATUS_SUCCESS) return AMDSMI_STATUS_NOT_SUPPORTED; ret = drm_.get_bdf_by_index(gpu_id_, &bdf); - ss << __PRETTY_FUNCTION__ << " | gpu_id_: " << gpu_id_ - << "; domain: " << bdf.domain_number - << "; bus: " << bdf.bus_number - << "; device: " << bdf.device_number - << "; drm_.get_drm_fd_by_index(gpu_id_, &bdf): " - << smi_amdgpu_get_status_string(ret, false) << std::endl; - // std::cout << ss.str(); - LOG_DEBUG(ss); if (ret != AMDSMI_STATUS_SUCCESS) return AMDSMI_STATUS_NOT_SUPPORTED; - bdf_ = bdf, path_ = path, fd_ = fd; + bdf_ = bdf, path_ = path; vendor_id_ = drm_.get_vendor_id(); return AMDSMI_STATUS_SUCCESS; @@ -280,15 +206,7 @@ int32_t AMDSmiGPUDevice::get_compute_process_list_impl(GPUComputeProcessList_t& } } kfd_file.close(); - } else { - std::ostringstream ss; - ss << __PRETTY_FUNCTION__ << " | Failed to open KFD file: " << kfd_path; - LOG_DEBUG(ss); } - } else { - std::ostringstream ss; - ss << __PRETTY_FUNCTION__ << " | KFD file not accessible: " << kfd_path; - LOG_DEBUG(ss); } return status_code; diff --git a/src/amd_smi/amd_smi_utils.cc b/src/amd_smi/amd_smi_utils.cc index e0fe8ec733..44013cebe9 100644 --- a/src/amd_smi/amd_smi_utils.cc +++ b/src/amd_smi/amd_smi_utils.cc @@ -672,10 +672,10 @@ amdsmi_status_t smi_amdgpu_get_market_name_from_dev_id(amd::smi::AMDSmiGPUDevice } ScopedFD fd(path.c_str(), O_RDWR | O_CLOEXEC); - if (!fd) { + if (!fd.valid()) { ss << __PRETTY_FUNCTION__ << " | Render Name: " << render_name << "; path: " << path << "; fd: " - << (fd < 0 ? "less than 0" : std::to_string(*fd)) << "\n" + << (fd < 0 ? "less than 0" : std::to_string(fd)) << "\n" << "; Returning: " << smi_amdgpu_get_status_string(AMDSMI_STATUS_FILE_ERROR, false) << "\n"; LOG_INFO(ss); @@ -715,7 +715,7 @@ amdsmi_status_t smi_amdgpu_get_market_name_from_dev_id(amd::smi::AMDSmiGPUDevice amdgpu_device_handle device_handle = nullptr; uint32_t major_version, minor_version; - int ret = amdgpu_device_initialize(*fd, &major_version, &minor_version, &device_handle); + int ret = amdgpu_device_initialize(fd, &major_version, &minor_version, &device_handle); if (ret != 0) { amdgpu_device_deinitialize(device_handle); libdrm_amdgpu_.unload(); @@ -737,19 +737,13 @@ amdsmi_status_t smi_amdgpu_get_market_name_from_dev_id(amd::smi::AMDSmiGPUDevice market_name[AMDSMI_MAX_STRING_LENGTH - 1] = '\0'; amdgpu_device_deinitialize(device_handle); libdrm_amdgpu_.unload(); - ss << __PRETTY_FUNCTION__ << " | path: " << path << "\n" - << " | fd: "<< std::dec << *fd << "\n" - << " | Marketing Name: " << market_name << "\n" - << " | Returning: " - << smi_amdgpu_get_status_string(AMDSMI_STATUS_SUCCESS, false) << "\n"; - LOG_INFO(ss); return AMDSMI_STATUS_SUCCESS; } amdgpu_device_deinitialize(device_handle); libdrm_amdgpu_.unload(); ss << __PRETTY_FUNCTION__ << " | path: " << path << "\n" - << " | fd: "<< std::dec << *fd << "\n" + << " | fd: "<< std::dec << fd << "\n" << " | Marketing Name: " << market_name << "\n" << " | Returning: " << smi_amdgpu_get_status_string(AMDSMI_STATUS_DRM_ERROR, false) << "\n"; @@ -1001,75 +995,3 @@ struct CperFileCtx { std::unique_ptr buffer; long file_size = 0; }; - - -void amdsmi_wait_for_user_input(void) { - for (;;) { - std::cout << "\n\t**Press any key to continue**" << std::endl; - int input = std::cin.get(); - if (input == EOF) { - std::cout << "EOF detected. Exiting." << std::endl; - return; - } - char input_char = static_cast(input); - std::cout << "User entered: " << input_char << std::endl; - if (input_char == '\n') { - return; - } - } -} - -ScopedFD::ScopedFD(const std::string& path, int flags) - : fd_(open(path.c_str(), flags)), path_(path) { - std::ostringstream ss; - if (fd_ >= 0) { - ss << __PRETTY_FUNCTION__ << " | Opened FD: " << fd_ - << " for path: " << path_; - LOG_INFO(ss); - } else { - ss << __PRETTY_FUNCTION__ << " | Failed to open file: " << path_ - << " | Error: " << strerror(errno); - LOG_ERROR(ss); - } -} - -ScopedFD::~ScopedFD() { - if (fd_ >= 0) { - std::ostringstream ss; - ss << __PRETTY_FUNCTION__ << " | Closing FD: " << fd_ - << " for path: " << path_; - LOG_INFO(ss); - close(fd_); - } -} - -ScopedFD::ScopedFD(ScopedFD&& other) noexcept - : fd_(other.fd_), path_(std::move(other.path_)) { - other.fd_ = -1; -} - -ScopedFD& ScopedFD::operator=(ScopedFD&& other) noexcept { - if (this != &other) { - if (fd_ >= 0) close(fd_); - fd_ = other.fd_; - path_ = std::move(other.path_); - other.fd_ = -1; - } - return *this; -} - -int ScopedFD::get() const { - return fd_; -} - -bool ScopedFD::valid() const { - return fd_ >= 0; -} - -ScopedFD::operator int() const { - return fd_; -} - -int ScopedFD::operator*() const { - return fd_; -} diff --git a/src/amd_smi/scoped_fd.cc b/src/amd_smi/scoped_fd.cc new file mode 100644 index 0000000000..27b71b7d21 --- /dev/null +++ b/src/amd_smi/scoped_fd.cc @@ -0,0 +1,44 @@ +#include "amd_smi/impl/scoped_fd.h" + +#include + +#include +#include + +#include "rocm_smi/rocm_smi_logger.h" + +ScopedFD::ScopedFD(const std::string& path, int flags) + : fd_(open(path.c_str(), flags)), path_(path) { + std::ostringstream ss; + if (fd_ < 0) { + ss << __PRETTY_FUNCTION__ << " | Failed to open file: " << path_ + << " | Error: " << strerror(errno); + LOG_ERROR(ss); + } +} + +ScopedFD::~ScopedFD() { + if (fd_ >= 0) { + close(fd_); + } +} + +ScopedFD::ScopedFD(ScopedFD&& other) noexcept : fd_(other.fd_), path_(std::move(other.path_)) { + other.fd_ = -1; +} + +ScopedFD& ScopedFD::operator=(ScopedFD&& other) noexcept { + if (this != &other) { + if (fd_ >= 0) close(fd_); + fd_ = other.fd_; + path_ = std::move(other.path_); + other.fd_ = -1; + } + return *this; +} + +int ScopedFD::get() const { return fd_; } + +bool ScopedFD::valid() const { return fd_ >= 0; } + +ScopedFD::operator int() const { return fd_; } diff --git a/tests/amd_smi_test/functional/computepartition_read_write.cc b/tests/amd_smi_test/functional/computepartition_read_write.cc index 28dceaf538..6cb6b4d2ad 100755 --- a/tests/amd_smi_test/functional/computepartition_read_write.cc +++ b/tests/amd_smi_test/functional/computepartition_read_write.cc @@ -273,7 +273,6 @@ static void checkPartitionIdChanges(amdsmi_processor_handle* const processor_han "\"sudo rmmod amdgpu && sudo rmmod ast && sudo modprobe amdgpu\")." "\n\tCPX may not enumerate properly.\n"; } - // amdsmi_wait_for_user_input(); // watch for any errors break; } amdsmi_kfd_info_t kfd_info; @@ -433,7 +432,6 @@ void TestComputePartitionReadWrite::Run(void) { << computePartitionString(updatePartition) << " ===============" << std::endl; } - // amdsmi_wait_for_user_input(); // watch for any errors auto ret_set = amdsmi_set_gpu_compute_partition(processor_handles_[dv_ind], updatePartition); IF_VERB(STANDARD) { @@ -464,7 +462,6 @@ void TestComputePartitionReadWrite::Run(void) { << "\n\t Device might be in a static partition mode. " << "With inability to change partition modes." << std::endl; - // amdsmi_wait_for_user_input(); // watch for any errors break; } @@ -493,7 +490,6 @@ void TestComputePartitionReadWrite::Run(void) { static_cast( mapStringToSMIComputePartitionTypes.at( std::string(orig_char_computePartition))); - // amdsmi_wait_for_user_input(); // watch for any errors on going back to original partition auto ret_set = amdsmi_set_gpu_compute_partition(processor_handles_[dv_ind], updatePartition); EXPECT_TRUE(ret_set == AMDSMI_STATUS_SETTING_UNAVAILABLE || ret_set== AMDSMI_STATUS_NO_PERM @@ -526,7 +522,6 @@ void TestComputePartitionReadWrite::Run(void) { << " ==========================================================\n"; std::cout << "\t**======================================================================\n"; } - // amdsmi_wait_for_user_input(); // watch for any errors PrintDeviceHeader(processor_handles_[dv_ind]); amdsmi_accelerator_partition_profile_t profile = {}; uint32_t partition_id[8] = {0, 0, 0, 0, 0, 0, 0, 0}; @@ -650,7 +645,6 @@ void TestComputePartitionReadWrite::Run(void) { AcceleratorProfileConfig original_profile_config = {}; original_profile_config = getAvailableProfileConfigs(dv_ind, profile, profile_config, isVerbose); - // amdsmi_wait_for_user_input(); // watch for any errors IF_VERB(STANDARD) { std::cout << "\t**=========================================================\n"; @@ -772,7 +766,6 @@ void TestComputePartitionReadWrite::Run(void) { << profile_config.profiles[config].profile_index << ")" << " ===============" << std::endl; } - // amdsmi_wait_for_user_input(); // watch for any errors auto ret_set = amdsmi_set_gpu_accelerator_partition_profile( processor_handles_[dv_ind], @@ -799,7 +792,6 @@ void TestComputePartitionReadWrite::Run(void) { << "\n\t Device might be in a static partition mode. " << "With inability to change partition modes." << std::endl; - // amdsmi_wait_for_user_input(); // watch for any errors break; } if (ret_set == AMDSMI_STATUS_NOT_SUPPORTED) { @@ -883,7 +875,6 @@ void TestComputePartitionReadWrite::Run(void) { std::cout << "\t**" << "amdsmi_get_gpu_accelerator_partition_profile: " << "Not supported on this machine, skipping remaining tests." << std::endl; } - // amdsmi_wait_for_user_input(); // watch for any errors break; } @@ -923,7 +914,6 @@ void TestComputePartitionReadWrite::Run(void) { std::cout << "\t**Test #3: Check fluctuating # of devices & partition IDs ==============\n"; std::cout << "\t**======================================================================\n"; } - // amdsmi_wait_for_user_input(); // watch for any errors on going back to original partition // ---------------------------------------------------------// // TEST 3: Check fluctuating # of devices & partition IDs // @@ -942,7 +932,6 @@ void TestComputePartitionReadWrite::Run(void) { std::cout << "\t**======================================================================\n"; } // Leaving for debug purposes - // amdsmi_wait_for_user_input(); // watch for any errors on going back to original partition uint32_t device_index = 0; amdsmi_processor_handle p_handle = {}; uint32_t current_num_devices = 0; @@ -1025,7 +1014,6 @@ void TestComputePartitionReadWrite::Run(void) { << "\n\t Device might be in a static partition mode. " << "With inability to change partition modes." << std::endl; - // amdsmi_wait_for_user_input(); // watch for any errors break; } @@ -1052,7 +1040,6 @@ void TestComputePartitionReadWrite::Run(void) { EXPECT_NE(updatePartition, mapStringToSMIComputePartitionTypes.at( std::string(current_char_computePartition))); } - // amdsmi_wait_for_user_input(); // watch for any errors on going back to original partition } uint32_t device_index3 = 0; @@ -1068,7 +1055,6 @@ void TestComputePartitionReadWrite::Run(void) { std::cout << "\t**ABOUT TO GO BACK TO ORIGINAL PARTITION (" << orig_char_computePartition << ")\n"; } - // amdsmi_wait_for_user_input(); // watch for any errors on going back to original partition auto ret_set = amdsmi_set_gpu_compute_partition(p_handle3, updatePartition); checkPartitionIdChanges(processor_handles_, dv_ind, std::string(orig_char_computePartition), isVerbose, true); diff --git a/tests/amd_smi_test/functional/memorypartition_read_write.cc b/tests/amd_smi_test/functional/memorypartition_read_write.cc index fbd76f3e2c..60d568a790 100755 --- a/tests/amd_smi_test/functional/memorypartition_read_write.cc +++ b/tests/amd_smi_test/functional/memorypartition_read_write.cc @@ -253,7 +253,6 @@ void TestMemoryPartitionReadWrite::Run(void) { AcceleratorProfileConfig original_profile_config = getAvailableProfileConfigs(dv_ind, profile, profile_config, isVerbose); orig_dev_config[dv_ind] = original_profile_config; - // amdsmi_wait_for_user_input(); // watch for any errors IF_VERB(STANDARD) { std::cout << "\t**=========================================================\n"; diff --git a/tests/amd_smi_test/main.cc b/tests/amd_smi_test/main.cc index 5cab9dfa20..56685f832b 100644 --- a/tests/amd_smi_test/main.cc +++ b/tests/amd_smi_test/main.cc @@ -116,7 +116,6 @@ static void RunGenericTest(TestBase *test) { // RunGenericTest(&); // } TEST(amdsmitstReadOnly, TestVersionRead) { - // amdsmi_wait_for_user_input(); TestVersionRead tst; RunGenericTest(&tst); }