Repository navigation
feat(amdsmi): get/set NPM balancing mode with supported-mode query - #12804
Merged
Merged
Conversation
Adds amdsmi_get_npm_balancing_mode()/amdsmi_set_npm_balancing_mode() to
toggle a node between Power Balancing and Frequency Balancing, backed by
board/npm_mode (ASCII "1"/"2"). Get always reflects the last-selected
mode regardless of NPM enablement; set is AMD-SMI-only, requires
elevation, and fails without touching sysfs when NPM is disabled on the
node. Exposed via CLI as `amd-smi node -p` (BALANCING_MODE) and
`amd-smi set --node-balancing-mode {PB,FB}`.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
CLI-facing input/output now use POWER_BALANCING/FREQUENCY_BALANCING instead of the PB/FB abbreviations; the library layer's internal PB/FB string contract is unchanged.
…-balancing-mode # Conflicts: # projects/amdsmi/amdsmi_cli/amdsmi_helpers.py # projects/amdsmi/amdsmi_cli/amdsmi_parser.py # projects/amdsmi/amdsmi_cli/subcommands/node.py # projects/amdsmi/amdsmi_cli/subcommands/set_value.py # projects/amdsmi/docs/how-to/amdsmi-cli-tool.md # projects/amdsmi/py-interface/amdsmi_wrapper.py # projects/amdsmi/rocm_smi/include/rocm_smi/rocm_smi.h # projects/amdsmi/rocm_smi/include/rocm_smi/rocm_smi_npm.h # projects/amdsmi/rocm_smi/src/rocm_smi.cc # projects/amdsmi/src/amd_smi/amd_smi.cc
- amdsmi_get_npm_balancing_mode()/amdsmi_set_npm_balancing_mode() get/set the NPM power-balancing mode (Power Balancing vs. Frequency Balancing) for a node. - Add amdsmi_get_npm_supported_balancing_modes(), decoding board/supported_mode's bitmask (bit N = enum value N of amdsmi_npm_balancing_mode_t) into the list of modes this platform/ASIC accepts, independent of NPM enablement. - Falls back from board/npm/ to flat board/ with no ASIC-specific branching. - Exposes Get/Set via the CLI (amd-smi node, amd-smi set --node-balancing-mode), alongside SUPPORTED_BALANCING_MODES Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Bill Liu <shuzhliu@amd.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: Bill Liu <shuzhliu@amd.com>
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
- Validate node_handle against the registry before dereferencing in amdsmi_get_npm_balancing_mode(), amdsmi_set_npm_balancing_mode(), and amdsmi_get_npm_supported_balancing_modes(), matching sibling NPM APIs. - Reject non-digit-leading supported_mode sysfs content before std::stoull() parsing, since it silently accepts a leading '-' and wraps to UINT64_MAX instead of failing. - Make the supported-mode gate in rsmi_dev_npm_balancing_mode_set() fail closed: only a missing sysfs file (NOT_SUPPORTED) still permits the write; any other read failure now rejects instead of silently allowing it through. - Run the balancing-mode Get calls in the CLI's node command regardless of whether amdsmi_get_npm_info() returned a dict. - Fix stale board/npm_mode and board/supported_npm_mode references in comments/log strings to match the actual sysfs paths.
bill-shuzhou-liu
requested review from
adam360x,
gabrpham,
marifamd and
oliveiradan
October 5, 2026 18:58
validate_and_set_node_balancing_mode() inferred "NPM disabled" by checking whether the requested mode was absent from the (enablement-independent) supported_mode bitmask, so platforms supporting both modes always fell through to a generic "not supported on this node" message instead of reporting that NPM itself is disabled. Pre-check NPM enablement via amdsmi_get_npm_info(), mirroring validate_and_set_node_power_limit(). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… loaded The hardware-free ASAN unit CI runner has no amdgpu driver, so amdsmi_init() returns AMDSMI_STATUS_DRIVER_NOT_LOADED and every test's ASSERT_EQ(..., AMDSMI_STATUS_SUCCESS) aborted the suite. Skip instead, mirroring the existing is_sudo_user() GTEST_SKIP_ convention.
gabrpham
requested changes
Oct 8, 2026
gabrpham
left a comment
Contributor
There was a problem hiding this comment.
Please see comments below.
…codes Split RSMI_STATUS_SETTING_UNAVAILABLE out from RSMI_STATUS_NOT_SUPPORTED so callers can tell "mode absent from supported_modes bitmask" apart from "NPM is disabled on this node". Also dedupes npm directory resolution via resolve_npm_dir(), pins explicit enum values with static_asserts, and simplifies the CLI's pre/post-check now that the status code itself disambiguates. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
adam360x
reviewed
Oct 9, 2026
Returns AMDSMI_STATUS_NOT_SUPPORTED instead of SUCCESS+INVALID when the balancing-mode sysfs file is missing or unreadable, matching the setter and supported-modes getter. Also tidies the CHANGELOG wording. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
adam360x
self-requested a review
October 9, 2026 13:47
adam360x
approved these changes
Oct 9, 2026
gabrpham
approved these changes
Oct 9, 2026
gabrpham
left a comment
Contributor
There was a problem hiding this comment.
Comments have been addressed. Approved.
WSLGPUBackend::IsActive() makes every NPM call return AMDSMI_STATUS_NOT_SUPPORTED before a test's fabricated sysfs content is ever consulted, so root-gated/content-driven NPM limit and balancing-mode tests fail on a real WSL2 CI runner instead of skipping. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
1 task done
marifamd
added a commit
that referenced
this pull request
Oct 9, 2026
…smi_asan_npm_driverless Conflict in amdsmi_npm_limit_test.cc: develop (#12804) added SKIP_IF_WSL_BACKEND_ACTIVE() after the init ASSERT_EQ that this branch replaces with REQUIRE_AMDSMI_INIT(init). Kept both. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
anujshuk-amd
pushed a commit
that referenced
this pull request
Oct 12, 2026
…12804) ## Motivation NPM (Node Power Management) balancing mode controls whether a node prioritizes Power Balancing (PB) or Frequency Balancing (FB) across its GPUs. There was previously no way to read or change this mode through amd-smi. ## Technical Details - Add `amdsmi_get_npm_balancing_mode()` / `amdsmi_set_npm_balancing_mode()` to get/set the current NPM balancing mode for a node. - Add `amdsmi_get_npm_supported_balancing_modes()`, which decodes `board/supported_mode`'s bitmask (bit N corresponds to enum value N of `amdsmi_npm_balancing_mode_t`) into the list of modes the platform/ASIC actually accepts, independent of whether NPM is currently enabled. The decode loop is generic over the enum range, so a future ASIC with a different/larger set of supported modes requires no decode-logic changes. - `amdsmi_npm_balancing_mode_t` values are explicit (`POWER_BALANCING = 1`, `FREQUENCY_BALANCING = 2`) so the bit-position-equals-enum-value invariant used by the bitmask decode is visible in the header and protected against accidental reordering. - Resolves the NPM board directory via `board/npm/` first, falling back to a flat `board/` layout (e.g. MI350X) with no ASIC-specific branching — purely driven by directory existence. - Exposes Get/Set via the CLI: `amd-smi node` (adds `SUPPORTED_BALANCING_MODES`) and `amd-smi set --node-balancing-mode`. `--node-balancing-mode` is a system-wide setting and is rejected in combination with `--gpu`/`-g`. - Set to a mode that is syntactically valid but absent from the platform's supported-mode bitmask returns `AMDSMI_STATUS_NOT_SUPPORTED` with CLI output text that names the rejected mode, distinguishable from the generic "NPM is disabled on this node" message. ## Issue Tracking JIRA ID : ROCM-31886 ## Test Plan - C++ gtest unit suite (`amdsmi_npm_balancing_mode_test.cc`), run both without root (parameter validation, read-only paths) and with root (Set-path tests gated by `REQUIRE_ROOT_ACCESS`). - Python unit suite (`test_npm_balancing_mode.py`, `test_cli_set_node_balancing_mode.py`), run with root via `unit_tests.py -k npm`. - Manual sanity test against mocked sysfs (overlayfs via the project's mock-sysfs tooling), covering: - Flat `board/` layout with no `mode`/`supported_mode` files (e.g. MI350X) → Get returns `N/A` and supported-modes query returns `AMDSMI_STATUS_NOT_SUPPORTED`, instead of erroring. - `board/npm/mode` + `board/npm/supported_mode=0x6` → Get decodes `FREQUENCY_BALANCING`, supported-modes decodes `[PB, FB]`. - CLI `amd-smi set --node-balancing-mode FREQUENCY_BALANCING` against a node whose `supported_mode` bitmask only allows `POWER_BALANCING` (`0x2`) → rejected with a message naming the specific unsupported mode, sysfs left unchanged. - CLI `amd-smi set --node-balancing-mode POWER_BALANCING` against the same node (mode present in the bitmask) → succeeds, sysfs `mode` updated accordingly. - `pre-commit run` (clang-format, ruff format, SPDX headers, C++ test naming conventions) clean on all changed files. ## Test Result - C++ gtest: all NPM balancing-mode tests passed (non-root and root runs). - Python unit tests: 35/35 passed (`-k npm`). - Manual mocked-sysfs sanity checks: all of the above scenarios behaved as expected. ## Submission Checklist - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. --------- Signed-off-by: Bill Liu <shuzhliu@amd.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
TheRock Submodule Bump ActivityNewest first
|
marifamd
added a commit
that referenced
this pull request
Oct 12, 2026
## Motivation ROCm 10.1 has branched to `release/therock-10.1`. This opens the 10.2 development cycle: it bumps the library minor version to 27.2.0 and fills the `10.2.0` changelog section. While opening it, the `10.1.0` section on `develop` was audited against the release branch, and entries that will not ship in 10.1 were moved to `10.2.0`. ## Technical Details **Version** (`include/amd_smi/amdsmi.h`, `rust-interface/src/amdsmi_wrapper.rs`): - Bump `AMDSMI_LIB_VERSION_MINOR` `1 -> 2` (27.2.0). SONAME stays `libamd_smi.so.27` (`SOVERSION` is `MAJOR` only). - Regenerate the Rust binding constant to match. The Python wrapper and Go shim carry no minor-version constant. **Changelog** (`CHANGELOG.md`): - Extend the existing `## amd_smi_lib for ROCm 10.2.0` section. - Move the `10.1.0` entries whose source PR is not on `release/therock-10.1`, and add entries for three develop-only PRs whose fixes merged without one: | Action | Source PR | Entries | |--------|-----------|---------| | Move `10.1.0` → `10.2.0` | #8624 | CLI exit-code rework: 5 Changed, 1 Removed, 2 Resolved Issues | | Move `10.1.0` → `10.2.0` | #11494 | `AMDSMI_VRAM_TYPE_HBM4` (Added) | | Move `10.1.0` → `10.2.0` | #11066 | fwupd UMA carveout (Changed) | | Move `10.1.0` → `10.2.0` | #11471 | NPM node power limit: 1 Added, 2 Changed | | Move `10.1.0` → `10.2.0` | #10678 | Version-absent `amd-smi metric` fields omitted from human output (Changed; was under Removed, but it changes output) | | Move `10.1.0` → `10.2.0` | #12144 | xGMI data counters reading as unavailable on MI450 (Resolved Issues) | | Move `10.1.0` → `10.2.0` | #12804 | NPM balancing mode get/set (Added) | | New in `10.2.0` | #11474 | Topology APIs returned uninitialized NUMA data (Resolved Issues) | | New in `10.2.0` | #11045 | `amdsmi_get_gpu_fabric_info()` rejected partial UALink sysfs data (Resolved Issues) | | New in `10.2.0` | #12999 | NULL output pointers, CPER paging after a large record, `amd-smi static --clock` traceback (3 Resolved Issues) | - Drop the `### Removed` heading left empty in `10.1.0`. - Keep the #11628 entry in `10.1.0` (cherry-picked to the release branch as #12032) and the `10.0.0` section as is (it differs from `release/therock-10.0` only by the #11046 placement fixes). **Docs** (`docs/how-to/amdsmi-cli-tool.md`): - Update the `amd-smi --help` banner and default-view header examples to 27.2.0 / ROCm 10.2.0 (stale since 27.0.0). ## Issue Tracking N/A ## Test Plan - Configure and build with `-DBUILD_TESTS=ON`; confirm CMake reports 27.2.0. - Run `pre-commit` (amdsmi config) on the changed files. - Check `10.2.0`, `10.1.0` and `10.0.0` against changelog conventions: allowed subsection headings, two-space line breaks on headline bullets, no JIRA IDs in entry text. - Diff the `10.1.0` entries and headings against `release/therock-10.1`. - No GPU run or new unit test: no runtime code changed. ## Test Result - Build: pass (`amdsmitst` links), CMake reports 27.2.0 - `pre-commit`: all hooks passed - Changelog conventions: clean for `10.2.0`, `10.1.0`, `10.0.0` - `10.1.0` vs `release/therock-10.1`: identical entries and headings --------- Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
1 task
marifamd
added a commit
that referenced
this pull request
Oct 30, 2026
## Motivation The ABI compliance check never flags a break. `check_report_content()` matches text that abi-compliance-checker 2.3 does not print, so every report reads as clean, and major mode passes with a warning even when the checker exits 1. It passed #8575 (removed `amdsmi_gpu_driver_reload`) and #11821 (grew `amdsmi_driver_info_t` from 768 to 1792 bytes, reverted in #13926). ## Technical Details **Report parser** (`tests/abi_check/abi_check.py`): - Read the counts from the report's section headings (`<h2>Removed Symbols <span class='failed'> 1 </span>`) - Fail on removed symbols, High or Medium severity problems, and any `Size of this type has been changed` row, naming the type - The checker rates struct growth Low because it assumes the library allocates the struct; amd-smi callers allocate their output structs, so growth breaks binaries built against the old header - Fail major mode when the checker exits non-zero; it used to finish "with warnings" and pass **Baseline** (`tests/abi_check/abi_check.py`): - Compare against `git merge-base <base> HEAD` instead of the base branch tip. Without this, once the check can fail, a PR that forked before an API landed on `develop` is reported as removing it **Tests and docs**: - `tests/abi_check/abi_check_test.py`: fixtures are fragments of real abi-compliance-checker 2.3 reports; adds merge-base and pass/fail tests - `.pre-commit-config.yaml`: run `abi_check_test.py` when `tests/abi_check/` changes, so the LSTT Formatting job runs it; no workflow ran it before - `tests/README.md`: describe both **Not changed**: `abi-compliance-check.yml`. The check stays non-blocking and still reports through the `MAJOR ABI BREAKAGE` / `MINOR ABI BREAKAGE` labels. The workflow runs the base branch's `abi_check.py`, so this PR's own ABI jobs still use the old parser. ## Issue Tracking Related: #11821, #13926 ## Test Plan - `abi_check_test.py` on Python 3.6, 3.10 and 3.14, against `develop`'s `abi_check.py` and this branch - `develop`'s and this branch's `abi_check.py` with abi-compliance-checker 2.3 on Ubuntu 22.04 (the CI job's tool and OS), major and minor mode, on real `amdsmi.h` pairs, each in a throwaway git repo so `git merge-base` and `git show` behave as in CI - `pre-commit run --from-ref origin/develop --to-ref HEAD` from `projects/amdsmi`, as the LSTT Formatting job runs it ## Test Result - `abi_check_test.py`: the new tests fail on `develop`'s `abi_check.py` (3 failures, 5 errors) and all 16 pass with this change on every Python version Real abi-compliance-checker 2.3 runs, exit code (major / minor): | `amdsmi.h` change | `develop` | This PR | This PR reports | |---|---|---|---| | #8575: removed `amdsmi_gpu_driver_reload` | 0 / 1 | **1** / 1 | Removed Symbols: 1 | | #11821: grew `amdsmi_driver_info_t` | 0 / 1 | **1** / 1 | `amdsmi_driver_info_t` 768 to 1792 bytes | | `release/therock-10.0` to `release/therock-10.1` | 0 / 1 | **1** / 1 | `amdsmi_enumeration_info_t` 276 to 280 bytes | | `release/therock-10.1` to `release/therock-10.2` | 0 / 1 | **1** / 1 | Medium data type problems; `amdsmi_driver_info_t`, `amdsmi_fabric_info_t` and its `fabric_info_` union grew | | #12804: added APIs | 0 / 0 | 0 / 0 | | | #11471: added APIs | 0 / 1 | 0 / 1 | | | `AMDSMI_LIB_VERSION_MINOR` bump | 0 / 1 | 0 / 1 | | | No change | 0 / 0 | 0 / 0 | | | PR forked before #12804, base branch then gained it | 0 / 1 | 0 / 0 | | Minor mode keeps its meaning (any change the checker reports with `-strict`); only its baseline changed. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
NPM (Node Power Management) balancing mode controls whether a node
prioritizes Power Balancing (PB) or Frequency Balancing (FB) across its
GPUs. There was previously no way to read or change this mode through
amd-smi.
Technical Details
amdsmi_get_npm_balancing_mode()/amdsmi_set_npm_balancing_mode()to get/set the current NPM balancing mode for a node.
amdsmi_get_npm_supported_balancing_modes(), which decodesboard/supported_mode's bitmask (bit N corresponds to enum value N ofamdsmi_npm_balancing_mode_t) into the list of modes the platform/ASICactually accepts, independent of whether NPM is currently enabled. The
decode loop is generic over the enum range, so a future ASIC with a
different/larger set of supported modes requires no decode-logic changes.
amdsmi_npm_balancing_mode_tvalues are explicit(
POWER_BALANCING = 1,FREQUENCY_BALANCING = 2) so thebit-position-equals-enum-value invariant used by the bitmask decode is
visible in the header and protected against accidental reordering.
board/npm/first, falling back toa flat
board/layout (e.g. MI350X) with no ASIC-specific branching —purely driven by directory existence.
amd-smi node(addsSUPPORTED_BALANCING_MODES) andamd-smi set --node-balancing-mode.--node-balancing-modeis a system-wide setting and is rejected incombination with
--gpu/-g.supported-mode bitmask returns
AMDSMI_STATUS_NOT_SUPPORTEDwith CLIoutput text that names the rejected mode, distinguishable from the
generic "NPM is disabled on this node" message.
Issue Tracking
JIRA ID : ROCM-31886
Test Plan
amdsmi_npm_balancing_mode_test.cc), run bothwithout root (parameter validation, read-only paths) and with root
(Set-path tests gated by
REQUIRE_ROOT_ACCESS).test_npm_balancing_mode.py,test_cli_set_node_balancing_mode.py), run with root viaunit_tests.py -k npm.mock-sysfs tooling), covering:
board/layout with nomode/supported_modefiles (e.g.MI350X) → Get returns
N/Aand supported-modes query returnsAMDSMI_STATUS_NOT_SUPPORTED, instead of erroring.board/npm/mode+board/npm/supported_mode=0x6→ Get decodesFREQUENCY_BALANCING, supported-modes decodes[PB, FB].amd-smi set --node-balancing-mode FREQUENCY_BALANCINGagainst anode whose
supported_modebitmask only allowsPOWER_BALANCING(
0x2) → rejected with a message naming the specific unsupportedmode, sysfs left unchanged.
amd-smi set --node-balancing-mode POWER_BALANCINGagainst thesame node (mode present in the bitmask) → succeeds, sysfs
modeupdated accordingly.
pre-commit run(clang-format, ruff format, SPDX headers, C++ testnaming conventions) clean on all changed files.
Test Result
-k npm).expected.
Submission Checklist