Repository navigation
feat(amdsmi): replace UUID with CUID - #9273
Merged
Merged
Conversation
github-actions
Bot
added
documentation
✅ All Policy Checks Passed
📖 Need help? See the Policy FAQ for details on every check and how to fix failures. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a new GPU “CUID” identifier API into AMD SMI, wires it through the C++/Python surfaces, and updates CLI and test coverage to prefer CUID where available while retaining UUID compatibility.
Changes:
- Added
amdsmi_get_gpu_device_cuid()to the public C API, Python wrapper/interface, and documentation. - Updated CLI device listing to try CUID first and fall back to UUID.
- Added new functional/performance tests (Python) and a new C++ functional test for device CUID.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| projects/amdsmi/tools/update_wrapper.sh | Updates wrapper-generation flow to optionally build/mount CUID, but currently has command/flow breakages. |
| projects/amdsmi/tools/hw_compatibility_status.py | Adds CUID API into hardware compatibility smoke coverage. |
| projects/amdsmi/tests/python/functional/gpu/test_identity.py | Adds functional Python test for amdsmi_get_gpu_device_cuid. |
| projects/amdsmi/tests/python/functional/gpu/test_benchmark.py | Adds performance benchmark coverage for the CUID API. |
| projects/amdsmi/tests/amd_smi_test/main.cc | Registers the new C++ functional test for device CUID. |
| projects/amdsmi/tests/amd_smi_test/functional/gpu/identity/device_cuid_read.h | Adds C++ test class declaration for CUID read functional testing. |
| projects/amdsmi/tests/amd_smi_test/functional/gpu/identity/device_cuid_read_test.cc | Implements the C++ functional test and input-validation checks for the CUID API. |
| projects/amdsmi/src/CMakeLists.txt | Adds BUILD_CUID option and links CUID/OpenSSL when enabled. |
| projects/amdsmi/src/amd_smi/amd_smi.cc | Implements amdsmi_get_gpu_device_cuid() behind BUILD_CUID. |
| projects/amdsmi/py-interface/amdsmi_wrapper.py | Exposes the new C function to Python via ctypes bindings. |
| projects/amdsmi/py-interface/amdsmi_interface.py | Adds the high-level Python amdsmi_get_gpu_device_cuid() helper. |
| projects/amdsmi/py-interface/init.py | Re-exports the new Python CUID API. |
| projects/amdsmi/include/amd_smi/amdsmi.h | Adds AMDSMI_GPU_CUID_SIZE and the CUID API declaration + docs. |
| projects/amdsmi/example/amd_smi_drm_example.cc | Demonstrates querying and printing device CUID. |
| projects/amdsmi/docs/reference/amdsmi-py-api.md | Documents the Python CUID API and exceptions. |
| projects/amdsmi/CHANGELOG.md | Notes upcoming CUID/UUID transition and the new API. |
| projects/amdsmi/amdsmi_cli/subcommands/list_devices.py | Uses CUID first for the “uuid” output field, falls back to UUID. |
| projects/amdsmi/amdsmi_cli/amdsmi_helpers.py | Uses CUID first when building GPU choice metadata, falls back to UUID. |
Comments suppressed due to low confidence (3)
projects/amdsmi/tools/update_wrapper.sh:85
- The CUID-enabled docker path
cds into/tmp/src, but the copy step that creates/tmp/srcis commented out, so the wrapper generation will fail.
&& make -C /tmp/cuid-build install \
## cp -r /src /tmp/src \
&& cd /tmp/src \
&& rm -rf build .cache \
projects/amdsmi/tools/update_wrapper.sh:96
- In the non-CUID docker path, the bash command string starts with
&& cp ..., which is a syntax/command-chaining error (there is no prior command to chain from).
--volume "$DIR":/src:rw \
"$IMAGE_REF" bash -c "
&& cp -r /src /tmp/src \
&& cd /tmp/src \
&& rm -rf build .cache \
projects/amdsmi/tests/amd_smi_test/functional/gpu/identity/device_cuid_read_test.cc:99
- Use
AMDSMI_GPU_CUID_SIZEconsistently in the happy-path section as well (length initialization and bounds check) to match the new public constant.
len = AMDSMI_GPU_UUID_SIZE;
DISPLAY_AMDSMI_API("amdsmi_get_gpu_device_cuid", "gpu=" + std::to_string(i), VERB(STANDARD));
err = amdsmi_get_gpu_device_cuid(processor_handles_[i], &len, cuid_buf);
DISPLAY_AMDSMI_STATUS(VERB(STANDARD), __FILE__, __LINE__, err, AMDSMI_STATUS_SUCCESS);
if (err == AMDSMI_STATUS_NOT_SUPPORTED) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
marifamd
approved these changes
Jul 28, 2026
amd-josnarlo
approved these changes
Jul 28, 2026
lamikr
self-requested a review
July 28, 2026 19:09
lamikr
approved these changes
Jul 28, 2026
lamikr
left a comment
lamikr
left a comment
Collaborator
There was a problem hiding this comment.
pr-bot error in test is unrelated, I will approve.
Collaborator
|
Some comments from the codex review: 1. CUID-enabled shared-library build❌ BLOCKING: Static CUID archive is not position-independent[ On ELF platforms, linking those non-PIC objects into Required action: Link |
Collaborator
3. CLI compatibility❌ BLOCKING: CUID silently replaces the UUID field and UUID selector
When CUID is available, Required action: Preserve and label UUID and CUID as separate fields, accept either identifier for selection
|
Collaborator
4. Wrapper generation
|
marifamd
force-pushed
the
users/gabrpham/AMDSMI_uuid_cuid_replacement
branch
from
July 29, 2026 01:35
8e0318e to
83666da
Compare
Signed-off-by: gabrpham <Gabriel.Pham@amd.com>
Signed-off-by: gabrpham <Gabriel.Pham@amd.com>
Move parameter validation outside #ifdef BUILD_CUID so null/undersized arguments return AMDSMI_STATUS_INVAL regardless of build flag, fixing the all-distro TestDeviceCuidRead failure. Also fix the broken update_wrapper.sh docker commands, extract the shared CUID-or-UUID CLI fallback into a helper, add AMDSMI_GPU_CUID_SIZE to the Python interface, and correct the CHANGELOG wording. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
Address review: use AMDSMI_GPU_CUID_SIZE in the CUID test buffer and length checks instead of the UUID constant, and correct 'definitely' to 'definitively' in the CHANGELOG. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
- update_wrapper.sh: remove dead CUID_DIR lookup (wrapper is generated from the header and never needs the CUID library). - update_rust_wrapper.sh: escape $(nproc) so it runs inside the container. - rust-interface/Dockerfile: install python3 so the cmake configure step (py-interface) succeeds during rust wrapper regeneration. Verified: both wrappers regenerate byte-identical to the committed versions (python via docker, rust via docker with the fixed image). Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
- Remove the ineffective hasattr guard in amdsmi_get_gpu_device_cuid; the NOT_SUPPORTED status already surfaces through _check_res. - Extract a _query_or_na helper so get_gpu_choices and get_gpu_cuid_or_uuid share one guarded-query path instead of repeated try/except blocks. - Document AmdSmiLibraryException in the CUID docstring. Signed-off-by: Maisam Arif <Maisam.Arif@amd.com>
marifamd
force-pushed
the
users/gabrpham/AMDSMI_uuid_cuid_replacement
branch
from
July 29, 2026 03:02
83666da to
b7c65ea
Compare
marifamd
self-requested a review
July 29, 2026 05:27
marifamd
approved these changes
Jul 29, 2026
bill-shuzhou-liu
approved these changes
Jul 29, 2026
1 task done
marbre
pushed a commit
that referenced
this pull request
Aug 18, 2026
#9864) ## 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: ```cpp // projects/rccl/src/misc/amdsmi_wrap.cc @ 6ccde5d 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 #9734). - **`py-interface/amdsmi_interface.py`** — keep-both: added the `AMDSMI_GPU_CUID_SIZE` alias (context on `develop`, which has #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 - Cherry-picked #8210 → #8575 → #7844 → #9464 onto the `release/therock-10.0` tip (`6ccde5dbc27`) - Full amd_smi build (library + CLI + `amdsmitst`) - Regenerated `amdsmi_wrapper.py` and diffed against the committed file - Diffed every cherry-pick against its `develop` original - Verified the `amdsmi_fabric_info_t` flattening is ABI-preserving, by compiling both headers and comparing `sizeof`/`offsetof` - Checked every in-repo consumer of the changed/removed types for breakage ## 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 #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 #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 #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 - [x] Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md. --------- Signed-off-by: Bindhiya Kanangot Balakrishnan <Bindhiya.KanangotBalakrishnan@amd.com> Signed-off-by: Maisam Arif <Maisam.Arif@amd.com> Signed-off-by: gabrpham <Gabriel.Pham@amd.com> Signed-off-by: josnarlo <joseph.narlo@amd.com> Signed-off-by: Galantsev, Dmitrii <dmitrii.galantsev@amd.com> Co-authored-by: gabrpham <Gabriel.Pham@amd.com> Co-authored-by: Joseph Narlo <joseph.narlo@amd.com> Co-authored-by: Galantsev, Dmitrii <dmitrii.galantsev@amd.com> Co-authored-by: Mika Laitio <mika.laitio@amd.com> Co-authored-by: Nilesh M Negi <Nilesh.Negi@amd.com> Co-authored-by: Cursor <cursoragent@cursor.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
Integrate CUID in Amd Smi for general usage by users and other tools.
Technical Details
Adds CUID related APIs and prioritizes their usage over the older UUID APIs.
Issue Tracking
JIRA ID : ROCM-2207
Test Plan
Test Result