refactor(amdsmi): backport unified ABI changes to therock-10.0 - #9852
Open
marifamd wants to merge 7 commits into
Open
refactor(amdsmi): backport unified ABI changes to therock-10.0#9852marifamd wants to merge 7 commits into
marifamd wants to merge 7 commits into
Conversation
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)
Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
1 task
marifamd
force-pushed
the
users/marbre/therock-10.0-AIRDEL-17
branch
from
August 8, 2026 17:51
36175f9 to
91aea1d
Compare
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
Bring the unified ABI changes onto the
release/therock-10.0line, stacked on the Python packaging series (PR #9734). These three already-mergeddevelopcommits 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,
-xprovenance preserved) of three mergeddevelopPRs onto the #9734 packaging base (36175f976db):GPU metrics (#8210,
aeb9da985bd):Driver reload (#8575,
706209f4f54):Unified ABI sync (#7844,
923767a9533):py-interface/amdsmi_interface.py(AMDSMI_GPU_CUID_SIZEalias alongside the base's_AMDSMI_STRING_LENGTHconstants)py-interface/amdsmi_wrapper.pyregenerates 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
36175f976db)cmake -DBUILD_WRAPPER=ON+make)amdsmi_wrapper.pyand diffed against the committed fileinclude/amd_smi/amdsmi.hTest Result
[100%]library + CLI)Submission Checklist