Skip to content

refactor(amdsmi): backport unified ABI changes to release/therock-10.0 - #9864

Merged
marbre merged 5 commits into
release/therock-10.0from
users/marifamd/therock-10.0-abi-release
Aug 18, 2026
Merged

marbre merged 5 commits into
release/therock-10.0from
users/marifamd/therock-10.0-abi-release

Conversation

@marifamd

@marifamd marifamd commented Aug 7, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Bring the unified ABI changes onto the release/therock-10.0 release branch. These already-merged develop commits change the amd_smi ABI (64-bit metrics counters, driver-reload removal, unified struct sync) and must land together so the 10.0 header, wrapper and interface stay consistent with each other — and so 10.0 does not ship an ABI that differs from develop.

The two earlier attempts are closed: #9852 stacked these changes on the Python packaging series, and #9734 was that packaging series. With #9734 closed, the packaging rework is not coming to 10.0, so this PR applies the ABI changes to the raw release branch.

Technical Details

Five commits on the release/therock-10.0 tip (6ccde5dbc27). Four are cherry-picks of merged develop commits (-x provenance preserved); one is a changelog retarget.

# Commit Cherry-picked from Notes
1 15432e923f fix(amdsmi): Bump GPU metrics accumulator counters (#8210) aeb9da985b byte-identical
2 7ff1686439 refactor(amdsmi): remove driver reload API and related functions (#8575) 706209f4f5 byte-identical
3 23ec5edc37 refactor(amdsmi): Sync with unified with known abi changes (#7844) 923767a953 3 conflicts, resolved below
4 9030ac095c docs(amdsmi): set changelog version to 10.0 — retargets the changelog heading from 7.15.0 to 10.0
5 94cec8fda6 fix(rccl): correct UALoE fabric detection and diagnostics on the amdsmi path (#9464) 0f674c16e8 byte-identical — required, see below

Why the RCCL commit is part of this PR

Commit 5 is not incidental: #7844 does not build without it. #7844 flattens amdsmi_fabric_info_t, deleting amdsmi_fabric_info_ver_t and moving the version to a top-level fabric_version. RCCL on the release branch hardcodes the old nested spelling:

// projects/rccl/src/misc/amdsmi_wrap.cc @ 6ccde5dbc27
fabricInfo.fabric_info.version                   // deleted by #7844
fabricInfo.fabric_info.fabric_version.v1         // deleted by #7844

Both identifiers cease to exist after commit 3, so RCCL fails to compile against the new header. #9464 replaces those accesses with a amdSmiFabricInfoIsFlat SFINAE trait that compiles against both the flat and nested layouts, and fixes the UALoE detection bugs described in that PR. It is a large diff (8 files, +1140/−29, including new RCCL tests), so it is called out explicitly here.

These five commits must land as a unit. Splitting the PR, or reverting the RCCL commit on its own, breaks the RCCL build.

Conflicts resolved in #7844

Three conflicts, all because the release branch lacks changes #7844 was written on top of:

  • py-interface/CMakeLists.txt — kept the release version. #7844's only hunk here reworks the packaging site-packages install block, which does not exist on this branch (it came from the packaging series in the now-closed Cherry-pick AMDSMI commits into ROCm 10.0 #9734).
  • py-interface/amdsmi_interface.py — keep-both: added the AMDSMI_GPU_CUID_SIZE alias (context on develop, which has feat(amdsmi): replace UUID with CUID #9273) alongside the release branch's _AMDSMI_STRING_LENGTH constants.
  • CHANGELOG.md — dropped the packaging-rework entry, which describes work not on this branch. #7844's ABI entries merged cleanly.

Issue Tracking

JIRA ID: AILITOOLS-270, ROCM-1553, ROCM-26658
JIRA ID: AIDREL-13
JIRA ID: AIDREL-19

Test Plan

Test Result

  • Build: clean (library + CLI + tests, 0 errors)
  • Wrapper: byte-identical after regeneration (0-line diff)
  • Cherry-pick fidelity: commits 1, 2 and 5 diff byte-identically against their develop originals (modulo hunk offsets); commit 3 differs only in the three conflicts documented above
  • Fabric ABI is preserved: sizeof(amdsmi_fabric_info_t) = 320 before and after, with every offset unchanged — bdf=0, version =8, v1=12, v1.accel_state=252, reserved=256
  • Removed symbols: amdsmi_gpu_driver_reload has no remaining references outside the CHANGELOG; none of the 16 removed/renamed symbols has a stale consumer anywhere under projects/
  • In-repo consumers:
    • projects/rdc reads amdsmi_gpu_metrics_t and the widened accumulators and is not modified here — a -fsyntax-only A/B of RdcMetricFetcherImpl.cc against the old and new headers produces the identical error set (pre-existing, ESMI-gated), so nothing regresses. The fields only widen, and they feed a uint64_t-valued map.
    • projects/rocshmem keeps a private dlopen mirror of amdsmi_fabric_info_t; it does not include amd_smi/amdsmi.h, and the layout is unchanged, so it is unaffected.
    • projects/rocprofiler-systems uses amdsmi_gpu_metrics_t and builds green in this PR's TheRock CI.
    • projects/rocm-smi-lib has its own independent rocm_smi.h; unaffected.

Known issues inherited from develop

Faithful cherry-picks, so these come along unchanged; they are present on develop today and are not introduced by this backport. Listed so the release branch owners know what is shipping. Fixes belong on develop first.

  • gpu_metrics accumulators lose their "unset" sentinel (from fix(amdsmi): [ROCM-1553] Bump GPU metrics accumulator counters #8210). The counters widened to 64-bit but the amdgpu table still reports 32, so the 0xFFFFFFFF sentinel zero-extends and stops matching the 64-bit sentinel consumers check. amdsmi_get_gpu_metrics_info() reports gfx_activity_acc/mem_activity_acc as a literal 4294967295 instead of N/A on hardware that does not report them (reproduced on gfx1030). Unchanged on parts that do report real values.
  • tests/python/cli/test_set.py fails instead of skipping (from refactor(amdsmi): Sync with unified with known abi changes #7844). cmds_need_permission is assigned in setUpClass after the _initialized early-return, so TestSet raises AttributeError; and the filter that replaced the old blanket skip covers only 3 of the privileged set subcommands, so an unprivileged run also fails on --perf-level, --profile, --perf-determinism, --clk-level and others.
  • amd-smi set --json and amd-smi static --partition --json disagree on one key (from refactor(amdsmi): Sync with unified with known abi changes #7844). set now emits accelerator_partition_mem_alloc_mode while static and the --compute-partition-mem-alloc-mode flag keep compute_partition_mem_alloc_mode.
  • Dead constants: the keep-both resolution leaves _AMDSMI_STRING_LENGTH and _AMDSMI_MAX_STRING_LENGTH with zero references (every call site moved to the public AMDSMI_MAX_STRING_LENGTH).

Submission Checklist

marifamd and others added 4 commits August 7, 2026 11:45
Widen five amdsmi_gpu_metrics_t accumulator counter fields from uint32_t
to uint64_t to match the amdgpu pmfw MetricsTable_t definition and prevent
overflow:
 - gfx_activity_acc
 - mem_activity_acc
 - pcie_nak_sent_count_acc
 - pcie_nak_rcvd_count_acc
 - pcie_lc_perf_other_end_recovery

This is a breaking ABI change: the struct layout and field offsets shift,
so callers reading these fields must be recompiled. The change cascades
through the ROCm SMI headers (rocm_smi.h, rocm_smi_dyn_gpu_metrics.h) and
the C++ implementation, and the ctypes and Rust wrappers are regenerated
to match. A 7.15.0 CHANGELOG entry documents the widening.

---------

Signed-off-by: Bindhiya Kanangot Balakrishnan <Bindhiya.KanangotBalakrishnan@amd.com>
Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
(cherry picked from commit aeb9da9)
A static code scan flagged an insecure pattern in the driver-reload path.
The functionality was already removed from the amd-smi CLI, so remove the
API entirely rather than leave a vulnerable, unused entry point.

- include/amd_smi/amdsmi.h: drop the amdsmi_gpu_driver_reload() declaration
  and its tagDriverControl doxygen group.
- src/amd_smi/amd_smi.cc: delete the implementation.
- rocm_smi: remove the underlying driver-reload plumbing
  (rocm_smi.cc, rocm_smi_device.cc, rocm_smi_utils.cc and their headers).
- py-interface: remove the Python binding and __init__ export; regenerate
  amdsmi_wrapper.py and the Rust wrapper to match.
- Callers now reload the driver out of band with
  `sudo modprobe -r amdgpu && sudo modprobe amdgpu` to apply memory
  partition changes; examples, docs/conceptual/partition.md, and
  hw_compatibility_status.py are updated accordingly.
- tests: rework memorypartition_read_write_test.cc and test_partition.py
  for the removed reload step and add
  test_memory_partition_lifecycle.py.
- CHANGELOG: add the 7.15.0 Removed entry.

Signed-off-by: gabrpham <Gabriel.Pham@amd.com>
Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
(cherry picked from commit 706209f)
The fabric (UALoE/IFoE) telemetry, fabric-info, and NIC firmware APIs
were introduced in the in-progress ROCm 7.14.0 release but their
declarations had drifted from the "unified" ABI that downstream
consumers build against. The public surface carried inconsistencies that
would force source churn on every caller and leave dead symbols in the
header: a string-returning telemetry helper that couldn't report
failure, a misspelled fabric-type enumerator (UALLINK), an intermediate
versioning struct that added an extra access level, a NIC firmware
typedef whose name didn't reflect that it describes a single entry, a
duplicate telemetry-category enumerator, and a cluster of unreferenced
max-count macros. Because the feature is still unreleased, this is the
last clean opportunity to reconcile the headers with the agreed ABI
before the symbols are frozen, avoiding a future breaking change once
external code depends on them.

- `amdsmi_fabric_telem_id_to_string()` signature changed from `const
char* f(uint64_t)` to `amdsmi_status_t f(uint64_t, const char**)`, so
the function can surface lookup failures via a status code and return
the name through an out-parameter. Callers (the GTest, the example, and
the Python interface) were updated to pass a `const char**` and check
the status.
- Flattened `amdsmi_fabric_info_t` by removing the intermediate
`amdsmi_fabric_info_ver_t` type. Its payload union is now the
`fabric_info` member of `amdsmi_fabric_info_t`, and its version field is
exposed directly as the top-level `fabric_version`. Field access
simplifies from `fabric_info.fabric_version.v1.<field>` to
`fabric_info.v1.<field>`, and `fabric_info.version` becomes
`fabric_version`. The change is ABI-preserving: field offsets and
overall struct size are unchanged, and the Python
`amdsmi_get_gpu_fabric_info()` dictionary keys are unchanged.
- Typedef rename: `amdsmi_nic_fw_t` → `amdsmi_nic_fw_entry_t` (it models
one firmware entry within `amdsmi_nic_fw_info_t::fw[]`), with the
`AMDSmiAINICDevice` member updated to match.
- Enumerator rename: `AMDSMI_FABRIC_TYPE_UALLINK` →
`AMDSMI_FABRIC_TYPE_UALINK`, including the `UALoeLinkTypeMap` entry.
- Removed symbols: the duplicate
`AMDSMI_FABRIC_TELEMETRY_CATEGORY_UNKNOWN` enumerator (identical
`0xFFFFFFFF` value to `..._INVALID`), and eight unreferenced public
macros (`AMDSMI_MAX_VF_COUNT`, `AMDSMI_MAX_DRIVER_NUM`,
`AMDSMI_DFC_FW_NUMBER_OF_ENTRIES`, `AMDSMI_MAX_WHITE_LIST_ELEMENTS`,
`AMDSMI_MAX_BLACK_LIST_ELEMENTS`, `AMDSMI_MAX_TA_WHITE_LIST_ELEMENTS`,
`AMDSMI_MAX_ERR_RECORDS`, `AMDSMI_MAX_PROFILE_COUNT`).
- Cascade: the ctypes wrapper (`amdsmi_wrapper.py`) and Rust bindings
(`amdsmi_wrapper.rs`) were regenerated; `amdsmi_interface.py` was
updated by hand; the CHANGELOG records the Changed/Removed entries.

JIRA ID : AILITOOLS-270

C++ functional tests
Python CLI test

All tests passed as expected

---------

Signed-off-by: josnarlo <joseph.narlo@amd.com>
Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
Signed-off-by: Galantsev, Dmitrii <dmitrii.galantsev@amd.com>
Co-authored-by: Maisam Arif <Maisam.Arif@amd.com>
Co-authored-by: Galantsev, Dmitrii <dmitrii.galantsev@amd.com>
Co-authored-by: Mika Laitio <mika.laitio@amd.com>
(cherry picked from commit 923767a)
Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
@marifamd
marifamd requested review from a team August 7, 2026 18:13
@marifamd
marifamd requested review from a team as code owners August 7, 2026 18:13
@github-actions github-actions Bot added documentation Improvements or additions to documentation project: amdsmi project: rccl labels Aug 7, 2026
@marifamd

Copy link
Copy Markdown
Contributor Author

RCCL failures are due to not having 0f674c1 (#9464). This change is scheduled for cherry-pick too and is needed in tandem due to the RCCL testing

…mi path (#9464)

## Motivation

`RCCL_USE_AMD_SMI_LIB=1` silently disabled UALoE fabric on hardware
where it is present and `ready`, while the default sysfs path worked. At
the same time, the default path printed a warning telling users to set
that very variable to "enable fabric support" — advice that would have
made things worse.

These two together sent readers (and agents) chasing a configuration
problem that did not exist.

## Technical Details

**Version check (`src/misc/amdsmi_wrap.cc`)**
`amdsmi_get_gpu_fabric_info()` returns `SUCCESS` with a correct payload
but never writes `fabric_info.version`, which stays at the sentinel
`amdsmi.h` documents for fields it cannot source from sysfs. Strict
equality against `AMDSMI_FABRIC_INFO_CURRENT_VERSION` could therefore
never pass against `libamd_smi.so.26`, so every device was dropped with
a warning. A probe showed `version = 0xFFFFFFFF` alongside `fabric_type
= UALOE`, `accel_state = READY` and an `accel_id`/`vpod` matching sysfs;
pre-setting version is overwritten, so it is not an input field. The
sentinel now means "unreported", and v1 is assumed, while a version the
library actually claims is still rejected.

**False warning (`src/misc/amdsmi_wrap.cc`)**
The "RCCL_USE_AMD_SMI_LIB not set, but HIP_FABRIC_API is defined"
warning predates sysfs fabric discovery via `ARSMI_get_fabric_info()`,
which populates `amdsmiFabricDevices` in the same way. With the env var
defaulting to 0 and `HIP_FABRIC_API` auto-detected on modern ROCm, it
printed on every rank of every job, and it conflated `HIP_FABRIC_API` (a
`hipMemFabricHandle_t` build flag) with which library does discovery.
Replaced by an `INFO` naming the active path.

**Struct layout compatibility (`src/include/amdsmi_wrap.h`)**
amd-smi 27.0 flattened `amdsmi_fabric_info_t`: the version moved from
`fabric_info.version` to a top-level `fabric_version`, and the payload
from `fabric_info.fabric_version.v1` to `fabric_info.v1`. Field offsets
did not move (version at byte 8, v1 at 12), so this is a source-level
rename only. A template-based shape detector (`amdSmiFabricInfoIsFlat`)
with `if constexpr` accessor functions handles either layout at compile
time. `static_assert` guards pin the struct sizes (244/320 bytes) and
`reserved` offset (256) to catch future ABI drift.

**Telemetry ABI (`src/misc/amdsmi_wrap.cc`)**
`amdsmi_fabric_telem_id_to_string` changed from returning the name to
writing it through an out-parameter in 27.0. The two are not
call-compatible and we reach it through `dlopen`, so the choice follows
the version the loaded runtime reports. An unknown version assumes the
out-parameter form: calling a 27+ implementation the old way would leave
it writing through an uninitialized pointer, whereas calling an older
one the new way merely leaves the name unwritten.

**Minor**
amd-smi's version string was printed as a fourth dotted component,
rendering as `version 26.5.0.26.5.0`. Now it will be printed as `version
26.5.0 (build 26.5.0)`.

## JIRA ID

AICOMRCCL-1656

Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Maisam Arif <Maisam.Arif@amd.com>
Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>

@astyrrian1 astyrrian1 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please specify a Jira ID that aligns to a qualified ticket.

@marifamd

Copy link
Copy Markdown
Contributor Author

Please specify a Jira ID that aligns to a qualified ticket.

AIDREL-13 & AIDREL-19

@marifamd
marifamd requested a review from astyrrian1 August 17, 2026 19:44

@astyrrian1 astyrrian1 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please address question from marius

@marbre

marbre commented Aug 18, 2026

Copy link
Copy Markdown
Member

Please specify a Jira ID that aligns to a qualified ticket.

AIDREL-13 & AIDREL-19

Every relevant information belongs into the PR description to propagate into the commit message, not into a comment.

@marbre
marbre merged commit 4c86383 into release/therock-10.0 Aug 18, 2026
67 of 76 checks passed
@marbre
marbre deleted the users/marifamd/therock-10.0-abi-release branch August 18, 2026 19:49
marbre added a commit to ROCm/TheRock that referenced this pull request Aug 19, 2026
Bumps rocm-systems to include cherry-picks

* ROCm/rocm-systems#9864 - JIRA ID AIRDEL-13, AIRDEL-19
* ROCm/rocm-systems#10315 - JIRA ID AIRDEL-24
marbre added a commit to ROCm/TheRock that referenced this pull request Aug 19, 2026
Bumps rocm-systems to include cherry-picks

* ROCm/rocm-systems#9864 - JIRA ID AIRDEL-13,
AIRDEL-19
* ROCm/rocm-systems#10315 - JIRA ID AIRDEL-24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants