fix(detector): stop AMD SMI's "N/A" sentinel from reaching numeric device fields

gpustack_runtime/detector/amd.py

@@ -4,6 +4,7 @@
 import logging
 from functools import lru_cache
 from pathlib import Path
+from typing import Any
 
 from .. import envs
 from ..logging import debug_log_exception, debug_log_warning
@@ -137,7 +138,7 @@ def detect_info(self) -> Devices | None:
                 )
 
                 dev_gpu_driver_info = pyamdsmi.amdsmi_get_gpu_driver_info(dev)
-                dev_driver_ver = dev_gpu_driver_info.get("driver_version")
+                dev_driver_ver = _get_reading(dev_gpu_driver_info, "driver_version")
 
                 # The operator resolves the name from the local PCI ID database
                 # first: pci.ids knows the board -- the subsystem vendor's name
@@ -157,7 +158,7 @@ def detect_info(self) -> Devices | None:
                     ):
                         dev_name = pyamdgpu.amdgpu_get_marketing_name(dev_gpudev)
                 if not dev_name:
-                    dev_name = dev_gpu_asic_info.get("market_name")
+                    dev_name = _get_reading(dev_gpu_asic_info, "market_name", "")
 
                 dev_cc = dev_hsa_agent.compute_capability
                 if not dev_cc:
@@ -211,12 +212,12 @@ def detect_info(self) -> Devices | None:
                 # The power limit is inventory, so it stays here, while the used
                 # power the same call carries belongs to the usage query.
                 dev_power = None
-                try:
+                with contextlib.suppress(pyamdsmi.AmdSmiException):
                     dev_power_info = pyamdsmi.amdsmi_get_power_info(dev)
-                    dev_power = (
-                        dev_power_info.get("power_limit", 0) // 1000000
-                    )  # uW to W
-                except pyamdsmi.AmdSmiException:
+                    dev_power_limit = _get_reading(dev_power_info, "power_limit")
+                    if dev_power_limit is not None:
+                        dev_power = dev_power_limit // 1000000  # uW to W
+                if dev_power is None:
                     with contextlib.suppress(pyrocmsmi.ROCMSMIError):
                         dev_power = pyrocmsmi.rsmi_dev_power_cap_get(dev_idx)
 
@@ -308,8 +309,14 @@ def detect_usage(self, devices: Devices | None = None) -> Devices | None:
                 dev_temp = None
                 try:
                     dev_gpu_metrics_info = pyamdsmi.amdsmi_get_gpu_metrics_info(dev)
-                    dev_cores_util = dev_gpu_metrics_info.get("average_gfx_activity", 0)
-                    dev_temp = dev_gpu_metrics_info.get("temperature_hotspot", 0)
+                    dev_cores_util = _get_reading(
+                        dev_gpu_metrics_info,
+                        "average_gfx_activity",
+                    )
+                    dev_temp = _get_reading(
+                        dev_gpu_metrics_info,
+                        "temperature_hotspot",
+                    )
                 except pyamdsmi.AmdSmiException:
                     with contextlib.suppress(pyrocmsmi.ROCMSMIError):
                         dev_cores_util = pyrocmsmi.rsmi_dev_busy_percent_get(dev_idx)
@@ -353,15 +360,21 @@ def detect_usage(self, devices: Devices | None = None) -> Devices | None:
                             if dev_ecc_count.uncorrectable_err > 0:
                                 dev_mem_status = DeviceMemoryStatusEnum.UNHEALTHY
 
+                # A sentinel reading means the same as a failed call -- AMD SMI
+                # cannot tell the used power -- so both route to ROCm SMI.
                 dev_power_used = None
-                try:
+                with contextlib.suppress(pyamdsmi.AmdSmiException):
                     dev_power_info = pyamdsmi.amdsmi_get_power_info(dev)
-                    dev_power_used = (
-                        dev_power_info.get("current_socket_power")
-                        if dev_power_info.get("current_socket_power", "N/A") != "N/A"
-                        else dev_power_info.get("average_socket_power", 0)
+                    dev_power_used = _get_reading(
+                        dev_power_info,
+                        "current_socket_power",
                     )
-                except pyamdsmi.AmdSmiException:
+                    if dev_power_used is None:
+                        dev_power_used = _get_reading(
+                            dev_power_info,
+                            "average_socket_power",
+                        )
+                if dev_power_used is None:
                     with contextlib.suppress(pyrocmsmi.ROCMSMIError):
                         dev_power_used = pyrocmsmi.rsmi_dev_power_get(dev_idx)
 
@@ -510,6 +523,31 @@ def distance_pci_devices(bdf_a: str, bdf_b: str) -> TopologyDistanceEnum:
         return ret
 
 
+def _get_reading(dev_info: dict, key: str, default: Any = None) -> Any:
+    """
+    Read one value out of an AMD SMI answer, treating its sentinel as absent.
+
+    AMD SMI reports an unavailable reading in-band: it rewrites the 0xFFFF (or
+    max-uint) the driver answered into the string "N/A" before returning, so a
+    dict otherwise holding numbers can carry a string at any key. A VF is where
+    this surfaces, its host-side telemetry being invisible to the guest.
+
+    Args:
+        dev_info:
+            The answer AMD SMI returned.
+        key:
+            The reading to take.
+        default:
+            What an absent or unavailable reading becomes.
+
+    Returns:
+        The reading, or the default.
+
+    """
+    dev_reading = dev_info.get(key, "N/A")
+    return default if dev_reading == "N/A" else dev_reading
+
+
 def _get_pci_device_name_by_bdf(dev_bdf: str) -> str:
     """
     Get the name of a device from the local PCI ID database.

gpustack_runtime/detector/hygon.py

@@ -349,8 +349,15 @@ def detect_usage(self, devices: Devices | None = None) -> Devices | None:
             for dev_idx in range(devs_count):
                 dev_uuid = f"GPU-{pyrocmsmi.rsmi_dev_unique_id_get(dev_idx)[2:]}"
 
-                dev_cores_util = pyrocmsmi.rsmi_dev_busy_percent_get(dev_idx)
-                dev_temp = pyrocmsmi.rsmi_dev_temp_metric_get(dev_idx)
+                # Each reading is isolated on its own, mirroring the AMD path:
+                # ROCm SMI raises on one it cannot serve, and an unwrapped call
+                # would cost the whole sweep rather than the reading.
+                dev_cores_util = None
+                with contextlib.suppress(pyrocmsmi.ROCMSMIError):
+                    dev_cores_util = pyrocmsmi.rsmi_dev_busy_percent_get(dev_idx)
+                dev_temp = None
+                with contextlib.suppress(pyrocmsmi.ROCMSMIError):
+                    dev_temp = pyrocmsmi.rsmi_dev_temp_metric_get(dev_idx)
                 if dev_cores_util is None:
                     debug_log_warning(
                         logger,
@@ -376,7 +383,9 @@ def detect_usage(self, devices: Devices | None = None) -> Devices | None:
                         if dev_ecc_count.uncorrectable_err > 0:
                             dev_mem_status = DeviceMemoryStatusEnum.UNHEALTHY
 
-                dev_power_used = pyrocmsmi.rsmi_dev_power_get(dev_idx)
+                dev_power_used = None
+                with contextlib.suppress(pyrocmsmi.ROCMSMIError):
+                    dev_power_used = pyrocmsmi.rsmi_dev_power_get(dev_idx)
 
                 usages.append(
                     Device(

tests/gpustack_runtime/detector/test_amd.py

@@ -152,7 +152,14 @@ def rsmi_dev_power_cap_get(self, dev_idx: int) -> int:
 
     def rsmi_dev_power_get(self, dev_idx: int) -> int:
         self.calls.append("rsmi_dev_power_get")
-        return self.cards[dev_idx]["power_used"]
+        # A card whose fixture carries no used power stands for the one ROCm
+        # SMI cannot read either -- a VF, where the binding raises rather than
+        # answering the sentinel AMD SMI would.
+        power_used = self.cards[dev_idx]["power_used"]
+        if power_used is None:
+            msg = "power is not supported on this device"
+            raise _FakeRocmSmiError(msg)
+        return power_used
 
 
 class _FakeHSA:
@@ -527,6 +534,94 @@ def test_detect_composes_the_information_and_the_usage(amd_bindings):
     assert devices[0].power_used == 142
 
 
+# --------------------------------------------------------------------------- #
+# AMD SMI's "N/A": an unavailable reading is reported in-band, as a string     #
+# inside a dict otherwise holding numbers. A VF exposes no host-side power     #
+# telemetry, so its driver answers 0xFFFF and the library hands over "N/A".    #
+# --------------------------------------------------------------------------- #
+
+
+def test_detect_info_falls_back_when_the_sentinel_hides_the_power_limit(amd_bindings):
+    # The limit is floor-divided from uW to W, so a sentinel reaching that
+    # arithmetic raises a TypeError -- which is not an AmdSmiException, so it
+    # escapes the handler and costs the host every AMD card.
+    card = _card("0000:05:00.0", "0x00a1b2c3d4e5f600")
+    card["power"]["power_limit"] = "N/A"
+    calls = amd_bindings([card], agents=[_agent("0000:05:00.0")])
+
+    devices = AMDDetector().detect_info()
+
+    assert "rsmi_dev_power_cap_get" in calls
+    assert devices[0].power == 700
+
+
+def test_detect_info_reports_no_driver_version_when_the_sentinel_hides_it(
+    amd_bindings,
+):
+    card = _card("0000:05:00.0", "0x00a1b2c3d4e5f600")
+    card["driver_info"]["driver_version"] = "N/A"
+    amd_bindings([card], agents=[_agent("0000:05:00.0")])
+
+    devices = AMDDetector().detect_info()
+
+    assert devices[0].driver_version is None
+
+
+def test_detect_info_never_names_a_board_after_the_sentinel(amd_bindings):
+    # The ASIC market name is the last link of the name chain, so a sentinel
+    # there would otherwise be stored as the board's name.
+    card = _card("0000:05:00.0", "0x00a1b2c3d4e5f600")
+    card["asic_info"]["market_name"] = "N/A"
+    amd_bindings([card], pci_ids=None)
+
+    devices = AMDDetector().detect_info()
+
+    assert devices[0].name == ""
+
+
+def test_detect_usage_falls_back_when_the_sentinel_hides_the_socket_power(
+    amd_bindings,
+):
+    # The sentinel has to route to the ROCm SMI fallback the same way an AMD
+    # SMI failure does -- both mean "AMD SMI cannot tell us the used power".
+    card = _card("0000:05:00.0", "0x00a1b2c3d4e5f600")
+    card["power"]["current_socket_power"] = "N/A"
+    card["power"]["average_socket_power"] = "N/A"
+    calls = amd_bindings([card], agents=[_agent("0000:05:00.0")])
+
+    devices = AMDDetector().detect_usage()
+
+    assert "rsmi_dev_power_get" in calls
+    assert devices[0].power_used == 131
+
+
+def test_detect_usage_reports_no_power_when_no_binding_can_read_it(amd_bindings):
+    # The reported VF: AMD SMI answers the sentinel for both socket readings
+    # and ROCm SMI cannot read the power either, so the field is absent rather
+    # than carrying a string the consumer parses as a number.
+    card = _card("0000:05:00.0", "0x00a1b2c3d4e5f600")
+    card["power"]["current_socket_power"] = "N/A"
+    card["power"]["average_socket_power"] = "N/A"
+    card["power_used"] = None
+    amd_bindings([card], agents=[_agent("0000:05:00.0")])
+
+    devices = AMDDetector().detect_usage()
+
+    assert devices[0].power_used is None
+
+
+def test_detect_usage_reports_no_metrics_when_the_sentinel_hides_them(amd_bindings):
+    card = _card("0000:05:00.0", "0x00a1b2c3d4e5f600")
+    card["metrics"]["average_gfx_activity"] = "N/A"
+    card["metrics"]["temperature_hotspot"] = "N/A"
+    amd_bindings([card], agents=[_agent("0000:05:00.0")])
+
+    devices = AMDDetector().detect_usage()
+
+    assert devices[0].cores_utilization == 0
+    assert devices[0].temperature is None
+
+
 # --------------------------------------------------------------------------- #
 # The CDI generator, which numbers its device nodes from the appendix.        #
 # --------------------------------------------------------------------------- #

tests/gpustack_runtime/detector/test_hygon.py

@@ -244,19 +244,29 @@ def rsmi_dev_ecc_count_get(self, dev_idx: int) -> _EccCount:
 
     def rsmi_dev_busy_percent_get(self, dev_idx: int) -> int:
         self.calls.append("rsmi_dev_busy_percent_get")
-        return self.cards[dev_idx]["busy_percent"]
+        return self._reading(dev_idx, "busy_percent")
 
     def rsmi_dev_temp_metric_get(self, dev_idx: int) -> int:
         self.calls.append("rsmi_dev_temp_metric_get")
-        return self.cards[dev_idx]["temperature"]
+        return self._reading(dev_idx, "temperature")
 
     def rsmi_dev_power_cap_get(self, dev_idx: int) -> int:
         self.calls.append("rsmi_dev_power_cap_get")
         return self.cards[dev_idx]["power_cap"]
 
     def rsmi_dev_power_get(self, dev_idx: int) -> int:
         self.calls.append("rsmi_dev_power_get")
-        return self.cards[dev_idx]["power_used"]
+        return self._reading(dev_idx, "power_used")
+
+    def _reading(self, dev_idx: int, key: str) -> int:
+        # A card whose fixture carries None for a reading stands for the one
+        # ROCm SMI cannot answer: the binding checks every return code and
+        # raises, rather than handing back a sentinel the way AMD SMI does.
+        reading = self.cards[dev_idx][key]
+        if reading is None:
+            msg = f"{key} is not supported on this device"
+            raise _FakeRocmSmiError(msg)
+        return reading
 
     def rsmi_topo_get_numa_node_number(self, dev_idx: int) -> int:
         return self.cards[dev_idx]["numa"]
@@ -629,6 +639,45 @@ def test_detect_composes_the_information_and_the_usage(hygon_bindings):
     assert devices[0].power_used == 217
 
 
+def test_detect_usage_bounds_an_unreadable_power_to_its_own_card(hygon_bindings):
+    # ROCm SMI raises on a reading it cannot serve, so an unwrapped read takes
+    # the whole sweep down with it and leaves every card on the host with its
+    # information-query values.
+    hygon_bindings(
+        [
+            _card("0000:0b:00.0", "0x9f8e7d6c5b4a3921", power_used=None),
+            _card("0000:0c:00.0", "0x9f8e7d6c5b4a3922"),
+        ],
+        agents=[_agent("0000:0b:00.0"), _agent("0000:0c:00.0")],
+    )
+
+    devices = HygonDetector().detect_usage()
+
+    assert [dev.power_used for dev in devices] == [None, 217]
+    # The rest of the faulty card's usage still arrives, and the healthy card
+    # is untouched.
+    assert [dev.cores_utilization for dev in devices] == [44, 44]
+    assert [dev.temperature for dev in devices] == [51, 51]
+    assert [dev.memory_used for dev in devices] == [1024, 1024]
+
+
+def test_detect_usage_bounds_an_unreadable_utilization_to_its_own_reading(
+    hygon_bindings,
+):
+    # Each reading is isolated on its own, so an unreadable utilization does
+    # not carry off the temperature the next call would have served.
+    hygon_bindings(
+        [_card("0000:0b:00.0", "0x9f8e7d6c5b4a3921", busy_percent=None)],
+        agents=[_agent("0000:0b:00.0")],
+    )
+
+    devices = HygonDetector().detect_usage()
+
+    assert devices[0].cores_utilization == 0
+    assert devices[0].temperature == 51
+    assert devices[0].power_used == 217
+
+
 # --------------------------------------------------------------------------- #
 # The CDI generator, which numbers its device nodes from the appendix.        #
 # --------------------------------------------------------------------------- #