From e40ab36c068171457de2ad9283444ce2923cbbab Mon Sep 17 00:00:00 2001 From: thxCode Date: Mon, 31 Aug 2026 18:53:12 +0800 Subject: [PATCH 1/3] fix(detector): normalize amd smi's "N/A" readings in the usage query - add a _get_reading helper mapping an AMD SMI reading to a value or a default, so the library's in-band "N/A" sentinel is handled in one place - route a sentinel used power to the ROCm SMI fallback, the same way a failed AMD SMI call already does, instead of reporting the string - take the sentinel as absent for the cores utilization and the temperature, letting the existing "unreadable utilization is 0" guard do its job Task 1 of amd-na-sentinel-leaks-into-numeric-fields. Signed-off-by: thxCode --- gpustack_runtime/detector/amd.py | 54 ++++++++++++++++--- tests/gpustack_runtime/detector/test_amd.py | 59 ++++++++++++++++++++- 2 files changed, 104 insertions(+), 9 deletions(-) diff --git a/gpustack_runtime/detector/amd.py b/gpustack_runtime/detector/amd.py index d8e1f8b..33b90f6 100644 --- a/gpustack_runtime/detector/amd.py +++ b/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 @@ -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. diff --git a/tests/gpustack_runtime/detector/test_amd.py b/tests/gpustack_runtime/detector/test_amd.py index 918b961..43829f4 100644 --- a/tests/gpustack_runtime/detector/test_amd.py +++ b/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,56 @@ 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_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. # # --------------------------------------------------------------------------- # From ecbc7704d108a9f5a743f095d1cd913fc8859b9b Mon Sep 17 00:00:00 2001 From: thxCode Date: Mon, 31 Aug 2026 18:55:14 +0800 Subject: [PATCH 2/3] fix(detector): normalize amd smi's "N/A" readings in the inventory query - fall back to the ROCm SMI power cap when the power limit is unreadable, instead of floor-dividing a string and raising a TypeError that no AmdSmiException handler catches, which costs the host every AMD card - report no driver version, rather than the sentinel, when AMD SMI cannot read it - stop naming a board after the sentinel, the ASIC market name being the last link of the name chain Task 2 of amd-na-sentinel-leaks-into-numeric-fields. Signed-off-by: thxCode --- gpustack_runtime/detector/amd.py | 14 ++++---- tests/gpustack_runtime/detector/test_amd.py | 38 +++++++++++++++++++++ 2 files changed, 45 insertions(+), 7 deletions(-) diff --git a/gpustack_runtime/detector/amd.py b/gpustack_runtime/detector/amd.py index 33b90f6..08551e4 100644 --- a/gpustack_runtime/detector/amd.py +++ b/gpustack_runtime/detector/amd.py @@ -138,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 @@ -158,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: @@ -212,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) diff --git a/tests/gpustack_runtime/detector/test_amd.py b/tests/gpustack_runtime/detector/test_amd.py index 43829f4..28b2c53 100644 --- a/tests/gpustack_runtime/detector/test_amd.py +++ b/tests/gpustack_runtime/detector/test_amd.py @@ -541,6 +541,44 @@ def test_detect_composes_the_information_and_the_usage(amd_bindings): # --------------------------------------------------------------------------- # +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, ): From 28e1954a5e4f5d84960a4c8977c62b07e70dee1c Mon Sep 17 00:00:00 2001 From: thxCode Date: Mon, 31 Aug 2026 19:06:50 +0800 Subject: [PATCH 3/3] fix(detector): isolate each hygon usage reading - wrap the busy percent, the temperature and the used power on their own, mirroring the AMD path: ROCm SMI raises on a reading it cannot serve, and an unwrapped call cost the whole sweep rather than the reading, leaving every card on the host with its information-query values Task 4a of amd-na-sentinel-leaks-into-numeric-fields. Signed-off-by: thxCode --- gpustack_runtime/detector/hygon.py | 15 ++++- tests/gpustack_runtime/detector/test_hygon.py | 55 ++++++++++++++++++- 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/gpustack_runtime/detector/hygon.py b/gpustack_runtime/detector/hygon.py index 68eadb5..858a8f8 100644 --- a/gpustack_runtime/detector/hygon.py +++ b/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( diff --git a/tests/gpustack_runtime/detector/test_hygon.py b/tests/gpustack_runtime/detector/test_hygon.py index f855750..13ed2fb 100644 --- a/tests/gpustack_runtime/detector/test_hygon.py +++ b/tests/gpustack_runtime/detector/test_hygon.py @@ -244,11 +244,11 @@ 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") @@ -256,7 +256,17 @@ 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"] + 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. # # --------------------------------------------------------------------------- #