[rocprofiler-compute] Improve amdsmi interface (#2245)

* Improve amdsmi interface

* Fix issue where max mem clock was being set as max gfx clock

* Handle the case when all device handles might not be usable due to
  devices being hidden by ROCR and HIP environment variables

* Fix get gpu vram size to return str in KB

* Improve testing of amdsmi interface functions

projects/rocprofiler-compute/CHANGELOG.md

@@ -41,6 +41,9 @@ Full documentation for ROCm Compute Profiler is available at [https://rocm.docs.
 
 * Corrected peak VALU Roofline profiling and analysis by removing `FP8` VALU and `BF16` VALU benchmarking.
 
+* Fixed issue where detected max memory clock from amd-smi interface was using max gfx clock
+  * Fixed issue where values detected from amd-smi were wrong when some GPU devices were hidden using ROCR or HIP environment variables
+
 ### Removed
 
 * Removed "VL1 Lat" metric for AMD Instinct MI300 series GPUs, due to MI300 series not supporting TCP_TCP_LATENCY_sum counter.

projects/rocprofiler-compute/src/utils/amdsmi_interface.py

@@ -76,111 +76,131 @@ def amdsmi_ctx() -> Iterator[None]:
 
 
 # Ignore undefined name amdsmi since it's dynamically imported
-def get_device_handle() -> "amdsmi.ProcessorHandle | None":  # noqa: F821
-    """Get the first AMD device handle."""
+def get_device_handles() -> "list[amdsmi.ProcessorHandle]":  # noqa: F821
+    """
+    Get all AMD device handles.
+    We query all handles since some handles cannot be
+    used as they are hidden by ROCR or HIP environment variables.
+    """
     amdsmi = import_amdsmi_module()
     try:
         devices = amdsmi.amdsmi_get_processor_handles()
-        if len(devices) == 0:
-            console_warning("No AMD GPU detected!")
-            return None
+        if not devices:
+            console_warning("No AMD device(s) detected!")
+            return []
         console_debug(f"Found {len(devices)} AMD device(s).")
-        return devices[0]
+        return devices
     except Exception as e:
-        console_warning(f"Error getting device handle: {e}")
-        return None
+        console_warning(f"Error getting device handles: {e}")
+        return []
 
 
 def get_mem_max_clock() -> float:
     """Get the maximum memory clock of the device."""
     amdsmi = import_amdsmi_module()
-    try:
-        return amdsmi.amdsmi_get_clock_info(
-            get_device_handle(), amdsmi.AmdSmiClkType.GFX
-        )["max_clk"]
-    except Exception as e:
-        console_warning(f"Error getting memory clocks: {e}")
-        return 0.0
+    error = None
+    for device in get_device_handles():
+        try:
+            return amdsmi.amdsmi_get_clock_info(device, amdsmi.AmdSmiClkType.MEM)[
+                "max_clk"
+            ]
+        except Exception as e:
+            error = e
+    console_warning(f"Error getting max memory clock: {error}")
+    return 0.0
 
 
-def get_gpu_model() -> str:
-    """Get the GPU model name."""
+def get_gpu_model() -> tuple[str, str, str]:
+    """Get GPU model related names."""
     amdsmi = import_amdsmi_module()
-    try:
-        gpu_model_info = (
-            # board -> product_name
-            amdsmi.amdsmi_get_gpu_board_info(get_device_handle())["product_name"],
-            # asic -> market_name
-            amdsmi.amdsmi_get_gpu_asic_info(get_device_handle())["market_name"],
-            # vbios -> name
-            amdsmi.amdsmi_get_gpu_vbios_info(get_device_handle())["name"],
-        )
-        console_debug(f"gpu model info: {str(gpu_model_info)}")
-        return gpu_model_info
-    except Exception as e:
-        console_warning(f"Error getting gpu model info: {e}")
-        return "N/A"
+    error = None
+    for device in get_device_handles():
+        try:
+            gpu_model_info = (
+                amdsmi.amdsmi_get_gpu_board_info(device)["product_name"],
+                amdsmi.amdsmi_get_gpu_asic_info(device)["market_name"],
+                amdsmi.amdsmi_get_gpu_vbios_info(device)["name"],
+            )
+            console_debug(f"gpu model info: {str(gpu_model_info)}")
+            return gpu_model_info
+        except Exception as e:
+            error = e
+    console_warning(f"Error getting gpu model info: {error}")
+    return ("N/A", "N/A", "N/A")
 
 
 def get_gpu_vbios_part_number() -> str:
     """Get the GPU VBIOS part number."""
     amdsmi = import_amdsmi_module()
-    try:
-        vbios_part_number = amdsmi.amdsmi_get_gpu_vbios_info(get_device_handle())[
-            "part_number"
-        ]
-        console_debug(f"GPU VBIOS Part Number: {vbios_part_number}")
-        return vbios_part_number
-    except Exception as e:
-        console_warning(f"Error getting GPU VBIOS part number: {e}")
-        return "N/A"
+    error = None
+    for device in get_device_handles():
+        try:
+            vbios_part_number = amdsmi.amdsmi_get_gpu_vbios_info(device)["part_number"]
+            console_debug(f"GPU VBIOS Part Number: {vbios_part_number}")
+            return vbios_part_number
+        except Exception as e:
+            error = e
+    console_warning(f"Error getting GPU VBIOS part number: {error}")
+    return "N/A"
 
 
 def get_gpu_compute_partition() -> str:
     """Get the GPU compute partition."""
     amdsmi = import_amdsmi_module()
-    try:
-        compute_partition = amdsmi.amdsmi_get_gpu_compute_partition(get_device_handle())
-        console_debug(f"GPU Compute Partition: {compute_partition}")
-        return compute_partition
-    except Exception as e:
-        console_warning(f"Error getting GPU compute partition: {e}")
-        return "N/A"
+    error = None
+    for device in get_device_handles():
+        try:
+            compute_partition = amdsmi.amdsmi_get_gpu_compute_partition(device)
+            console_debug(f"GPU Compute Partition: {compute_partition}")
+            return compute_partition
+        except Exception as e:
+            error = e
+    console_warning(f"Error getting GPU compute partition: {error}")
+    return "N/A"
 
 
 def get_gpu_memory_partition() -> str:
     """Get the GPU memory partition."""
     amdsmi = import_amdsmi_module()
-    try:
-        memory_partition = amdsmi.amdsmi_get_gpu_memory_partition(get_device_handle())
-        console_debug(f"GPU Memory Partition: {memory_partition}")
-        return memory_partition
-    except Exception as e:
-        console_warning(f"Error getting GPU memory partition: {e}")
-        return "N/A"
+    error = None
+    for device in get_device_handles():
+        try:
+            memory_partition = amdsmi.amdsmi_get_gpu_memory_partition(device)
+            console_debug(f"GPU Memory Partition: {memory_partition}")
+            return memory_partition
+        except Exception as e:
+            error = e
+    console_warning(f"Error getting GPU memory partition: {error}")
+    return "N/A"
 
 
 def get_amdgpu_driver_version() -> str:
     """Get the AMDGPU driver version."""
     amdsmi = import_amdsmi_module()
-    try:
-        driver_info = amdsmi.amdsmi_get_gpu_driver_info(get_device_handle())
-        driver_version = driver_info["driver_version"]
-        console_debug(f"AMDGPU Driver Version: {driver_version}")
-        return driver_version
-    except Exception as e:
-        console_warning(f"Error getting AMDGPU driver version: {e}")
-        return "N/A"
+    error = None
+    for device in get_device_handles():
+        try:
+            driver_info = amdsmi.amdsmi_get_gpu_driver_info(device)
+            driver_version = driver_info["driver_version"]
+            console_debug(f"AMDGPU Driver Version: {driver_version}")
+            return driver_version
+        except Exception as e:
+            error = e
+    console_warning(f"Error getting AMDGPU driver version: {error}")
+    return "N/A"
 
 
-def get_gpu_vram_size() -> int:
-    """Get the GPU VRAM size in MB."""
+def get_gpu_vram_size() -> str:
+    """Get the GPU VRAM size in KB."""
     amdsmi = import_amdsmi_module()
-    try:
-        vram_info = amdsmi.amdsmi_get_gpu_vram_info(get_device_handle())
-        vram_size = str(int(vram_info["vram_size"]) * 1024)  # MB -> KB
-        console_debug(f"GPU VRAM Size: {vram_size} MB")
-        return vram_size
-    except Exception as e:
-        console_warning(f"Error getting GPU VRAM size: {e}")
-        return 0
+    error = None
+    for device in get_device_handles():
+        try:
+            vram_info = amdsmi.amdsmi_get_gpu_vram_info(device)
+            vram_size = str(int(vram_info["vram_size"]) * 1024)  # MB -> KB
+            console_debug(f"GPU VRAM Size: {vram_size} KB")
+            return vram_size
+        except Exception as e:
+            error = e
+    console_warning(f"Error getting GPU VRAM size: {error}")
+    return "0"

projects/rocprofiler-compute/tests/test_utils.py

@@ -7719,21 +7719,22 @@ def test_amdsmi_ctx():
             amdsmi_shutdown_mock.assert_called_once()
 
 
-def test_amdsmi_get_device_handle():
-    from utils.amdsmi_interface import get_device_handle, import_amdsmi_module
+def test_amdsmi_get_device_handles():
+    from utils.amdsmi_interface import get_device_handles, import_amdsmi_module
 
     _ = import_amdsmi_module()
 
     with mock.patch("amdsmi.amdsmi_get_processor_handles") as device_handles_mock:
         device_handles_mock.return_value = [12345]
-        get_device_handle()
+        handles = get_device_handles()
+        assert handles[0] == 12345
         device_handles_mock.assert_called_once()
 
     with mock.patch(
         "amdsmi.amdsmi_get_processor_handles", side_effect=Exception("Mock exception")
     ) as device_handles_mock:
-        handle = get_device_handle()
-        assert handle is None
+        handle = get_device_handles()
+        assert len(handle) == 0
 
 
 def test_amdsmi_get_mem_max_clock():
@@ -7741,12 +7742,18 @@ def test_amdsmi_get_mem_max_clock():
 
     _ = import_amdsmi_module()
 
-    with mock.patch("amdsmi.amdsmi_get_processor_handles") as device_handles_mock:
-        device_handles_mock.return_value = [12345]
+    with mock.patch("utils.amdsmi_interface.get_device_handles") as device_handles_mock:
+        device_handles_mock.return_value = [0, 4567]
         with mock.patch("amdsmi.amdsmi_get_clock_info") as mem_max_clock_mock:
-            mem_max_clock_mock.return_value = {"max_clk": 100}
+
+            def side_effect(handle, *args, **kwargs):
+                if handle == 0:
+                    raise Exception("Invalid handle: 0")
+                return {"max_clk": 100}
+
+            mem_max_clock_mock.side_effect = side_effect
             clk = get_mem_max_clock()
-            mem_max_clock_mock.assert_called_once()
+            assert mem_max_clock_mock.call_count == 2
             assert clk == 100
 
 
@@ -7755,7 +7762,7 @@ def test_amdsmi_get_gpu_model():
 
     _ = import_amdsmi_module()
 
-    with mock.patch("amdsmi.amdsmi_get_processor_handles") as device_handles_mock:
+    with mock.patch("utils.amdsmi_interface.get_device_handles") as device_handles_mock:
         device_handles_mock.return_value = [12345]
         with mock.patch("amdsmi.amdsmi_get_gpu_board_info") as device_name_mock:
             with mock.patch("amdsmi.amdsmi_get_gpu_asic_info") as asic_name_mock:
@@ -7771,7 +7778,7 @@ def test_amdsmi_get_gpu_model():
             "amdsmi.amdsmi_get_gpu_board_info", side_effect=Exception("Mock exception")
         ):
             model = get_gpu_model()
-            assert model == "N/A"
+            assert model == ("N/A", "N/A", "N/A")
 
 
 def test_amdsmi_get_gpu_vbios_part_number():
@@ -7779,7 +7786,7 @@ def test_amdsmi_get_gpu_vbios_part_number():
 
     _ = import_amdsmi_module()
 
-    with mock.patch("amdsmi.amdsmi_get_processor_handles") as device_handles_mock:
+    with mock.patch("utils.amdsmi_interface.get_device_handles") as device_handles_mock:
         device_handles_mock.return_value = [12345]
         with mock.patch("amdsmi.amdsmi_get_gpu_vbios_info") as vbios_part_number_mock:
             vbios_part_number_mock.return_value = {
@@ -7801,7 +7808,7 @@ def test_amdsmi_get_gpu_compute_partition():
 
     _ = import_amdsmi_module()
 
-    with mock.patch("amdsmi.amdsmi_get_processor_handles") as device_handles_mock:
+    with mock.patch("utils.amdsmi_interface.get_device_handles") as device_handles_mock:
         device_handles_mock.return_value = [12345]
         with mock.patch(
             "amdsmi.amdsmi_get_gpu_compute_partition"
@@ -7824,7 +7831,7 @@ def test_amdsmi_get_gpu_memory_partition():
 
     _ = import_amdsmi_module()
 
-    with mock.patch("amdsmi.amdsmi_get_processor_handles") as device_handles_mock:
+    with mock.patch("utils.amdsmi_interface.get_device_handles") as device_handles_mock:
         device_handles_mock.return_value = [12345]
         with mock.patch(
             "amdsmi.amdsmi_get_gpu_memory_partition"