Skip to content

refactor(amdsmi): backport unified ABI changes to therock-10.0 - #9852

Open
marifamd wants to merge 7 commits into
users/marbre/therock-10.0-AIRDEL-17from
users/marifamd/therock-10.0-abi-sync
Open

refactor(amdsmi): backport unified ABI changes to therock-10.0#9852
marifamd wants to merge 7 commits into
users/marbre/therock-10.0-AIRDEL-17from
users/marifamd/therock-10.0-abi-sync

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 line, stacked on the Python packaging series (PR #9734). These three 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 wrapper/interface stay consistent.

Technical Details

Cherry-picks (chronological, -x provenance preserved) of three merged develop PRs onto the #9734 packaging base (36175f976db):

GPU metrics (#8210, aeb9da985bd):

  • Widen GPU metrics accumulator counters 32 to 64 bit (struct layout change)

Driver reload (#8575, 706209f4f54):

  • Remove the driver-reload API and related functions (drops exported symbols across header/impl/wrapper/interface)

Unified ABI sync (#7844, 923767a9533):

  • Sync with the unified known-ABI changes across header, wrapper, interface, CLI, examples, tests
  • One trivial keep-both conflict resolved in py-interface/amdsmi_interface.py (AMDSMI_GPU_CUID_SIZE alias alongside the base's _AMDSMI_STRING_LENGTH constants)

py-interface/amdsmi_wrapper.py regenerates byte-identical from the post-cherry-pick header (verified via the wrapper Docker image).

Issue Tracking

JIRA ID: AILITOOLS-270, ROCM-1553, ROCM-26658

Test Plan

Test Result

Submission Checklist

marifamd and others added 6 commits August 5, 2026 22:28
Relands the manylinux wheel CI job (#7913) and the Python packaging rework
(#7914) after they were reverted in #9339, together with the follow-up
amd-smi CLI shebang fixes so the rework and its corrections land as one
change.

Packaging rework (reintroduced):
- Install the amdsmi module into the system python's site-packages via
  install(DIRECTORY) instead of pip + a .pth file; drop the pip plumbing and
  python3-pip/setuptools/wheel deps from the DEBIAN/RPM scriptlets.
- Replace the ROCm-path-walking resolver in amdsmi_wrapper.py (and
  tools/generator.py, regenerated in lockstep) with a flat loader:
  AMDSMI_LIB_OVERRIDE, a bundled libamd_smi_python.so, then the system
  libamd_smi.so; a _MissingLibrary sentinel keeps import tolerant.
- Add BUILD_PYTHON_WHEEL (default OFF): build a SONAME-isolated
  libamd_smi_python.so and a wheel that disables the loader's system fallback.
- Stage the module under share/amd_smi for ROCM_PATH consumers and TheRock.
- Add test_abi_compat.py and the manylinux wheel CI job.

CLI shebang fixes:
- Use a portable "#!/usr/bin/env python3" shebang for the amd-smi CLI so it
  runs on relocatable installs with no /usr/bin/python3 (TheRock containers)
  and on wheel runners where /usr/bin/python3 lacks the amdsmi module.
- Stop pinning the CLI shebang to a fixed interpreter: remove pin_shebang.cmake
  and the AMDSMI_CLI_SHEBANG_INTERP derivation. The pinned absolute path became
  a hard RPM "Requires:" (e.g. /usr/libexec/platform-python3.6) that RHEL10 and
  SLES16 could not satisfy; the RPM now depends only on python(abi).
- Resolve the env-form shebang in the install-verify harness instead of running
  "/usr/bin/env -c".

Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
…7912)

Add tests/run_amdsmi_pkg_conflict_test.py, which asserts that two AMD SMI
packages built with different SOVERSIONs do not co-own the same
libamd_smi.so.<major>, so installing one over the other cannot silently
break the other's loader.

Wire the ABI-compat output into the build harness: summarize_results in
run_amdsmi_build.py now scans abi_compat_output.txt alongside the existing
integration/unit/perf test outputs so an ABI regression surfaces in the CI
summary.

Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
…rework (#9033)

## Description

Correctness and coverage follow-ups to the AMD SMI Python packaging
rework (relanded in #9340). This branch is the packaging-hardening layer
on top of the SONAME conflict test + build harness (#7912).

Rebased onto `develop` after #7912 and #9340 merged; the now-upstream
shebang and self-locate-loader commits were dropped, and the surviving
relocatable-loader change was reconciled with develop's version (keeps
develop's `len(parents) > 3` guard and adds a `try/except OSError`
fall-through so a present-but-unloadable relocatable library does not
shadow the system SONAME).

### Loader / wrapper
- Fall through to the system library when the relocatable TheRock
library exists but fails to load (missing deps).
- Dedupe the wrapper `_libraries` init (the ctypesgen preamble already
initializes it).
- Read and write the staged wrapper as UTF-8 in
`disable_system_fallback.py` so the wheel flag flip is deterministic
regardless of build locale; make the staging idempotent so a rebuild
that reuses the flipped copy succeeds.

### Packaging correctness
- Stop the RPM from co-owning system Python directories.
- Restrict package `preun`/`prerm` cleanup to a full erase (not
upgrades).
- Quote the install prefix when writing `ld.so.conf.d` on RPM.
- Pin permissions on the installed Python module; make the operational
log group-writable (not world-writable) and stop the logger from
restoring world-writable permissions.
- Only strip release libraries when a strip tool is available; fail the
build when the system `python3` cannot be detected.

### Wheel build
- Build one wheel per platform (not per interpreter), tag it
`manylinux_2_28`, and verify the bundled `.so`.
- Use a clean PyPI version and expose the commit as metadata.
- Reject regenerating the wrapper against the wheel library; skip the
system site-packages install for wheel builds.

### Tests
- Loader-contract tests in `test_abi_compat.py`: preferred bundled
`.so`, wheel refuses system fallback when the bundle is missing, the
relocatable OSError fall-through, and an ambiguous-wrapper hard failure.
Assert the idempotent no-op in the fallback double-run guard.
- Guard against drift between the two installed module copies; install
the dual-copy runner so its guard tests run in CI; run the wheel
install-location check without a GPU.

### Verification
- `cmake -DBUILD_TESTS=ON` + `make`: 0 errors, `libamd_smi.so.27` +
`amdsmitst` built.
- `test_abi_compat.py` (10) and `test_dual_copy_guard.py` (4): all pass.

## JIRA ID

ROCM-3941

---------

Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
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)
@marifamd
marifamd requested review from a team as code owners August 7, 2026 16:43
@github-actions github-actions Bot added documentation Improvements or additions to documentation project: amdsmi project: rccl labels Aug 7, 2026
Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
@marifamd
marifamd force-pushed the users/marbre/therock-10.0-AIRDEL-17 branch from 36175f9 to 91aea1d Compare August 8, 2026 17:51
@marifamd
marifamd requested a review from a team as a code owner August 8, 2026 17:51
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.

4 participants