Skip to content

test(amdsmi): cover malformed FCLK tables - #14036

Open
koushikbillakanti-amd wants to merge 8 commits into
developfrom
users/kbillaka/ROCM-33079
Open

koushikbillakanti-amd wants to merge 8 commits into
developfrom
users/kbillaka/ROCM-33079

Conversation

@koushikbillakanti-amd

@koushikbillakanti-amd koushikbillakanti-amd commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Motivation

Cover existing FCLK error reporting on mixed GPUs without hiding malformed driver data. The empty Raphael FCLK table still needs a driver fix.

Technical Details

  • Use shared sysfs/device fixtures to test valid and newline-only FCLK tables through the real public APIs.
  • Route clock-range reads through the existing private sysfs-root seam; the production path is unchanged.
  • Drop the parser return-value changes and unrelated reset/sweep changes. Newline-only data remains IO; zero-byte and missing-node behavior are unchanged.
  • Keep memory-partition coverage, --gpu all, and saved clock-limit restoration. No user-facing change remains, so the earlier changelog entries are removed.

Issue Tracking

JIRA ID: ROCM-33079

Separate driver ticket: pending required Targeted ASIC selection; the November 2 logs are attached to the original ticket.

Test Plan

BUILD and ASAN_BUILD below point to configured Debug builds (BUILD_TESTS=ON; the latter also has ADDRESS_SANITIZER=ON).

  • cmake --build "$BUILD" --target amdsmitst amd_smi amdsmi_cli amdsmi_cli_subcommands -j4 — passed.
  • "$BUILD/tests/amd_smi_test/amdsmitst" --gtest_filter='*Unit*' — passed; also passed with sudo -n.
  • ASAN_OPTIONS=detect_leaks=1:strict_string_checks=1:detect_stack_use_after_return=1:check_initialization_order=1 LSAN_OPTIONS="suppressions=$PWD/tests/amd_smi_test/lsan.supp:print_suppressions=1" "$ASAN_BUILD/tests/amd_smi_test/amdsmitst" --gtest_filter='*Unit*' — passed; privileged unit cases also passed.
  • sudo -n env PYTHONDONTWRITEBYTECODE=1 LD_LIBRARY_PATH="$BUILD/src" AMDSMI_PATH="$BUILD/py-interface/python_package" python3 tests/python/unit_tests.py -v -k cli — passed.
  • Pinned project pre-commit hooks on changed files and git diff --check — passed.

Test Result

C++: 313 passed / 25 skipped without root; 328 passed / 10 skipped with root, including under ASAN with leak checks. Python: 360 passed. Clock/RAS fixtures also passed 10 shuffled repetitions.

The hardware set sweep and driver reload were not run; pending memory-partition cancellation is not claimed. Previous-head OS CI stopped at DRIVER_NOT_LOADED; GPU ASAN produced no test results. New-head CI must still complete. The ASAN build retains one existing E-SMI discarded-const warning.

Submission Checklist

@therock-pr-bot

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
📝 PR Description ✅ Pass —
⛔ Forbidden Files ✅ Pass —
🧪 Unit Test ✅ Pass —
🚫 Draft PR 🔜 To Be Enabled —
🚩 Feature Flag 🔜 To Be Enabled —
📊 Code Coverage 🔜 To Be Enabled —

🎉 All policy checks passed!

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

@koushikbillakanti-amd
koushikbillakanti-amd marked this pull request as ready for review October 22, 2026 18:02
@koushikbillakanti-amd
koushikbillakanti-amd requested a review from a team October 22, 2026 18:02
Signed-off-by: Koushik Billakanti <koushik.billakanti@amd.com>
@koushikbillakanti-amd koushikbillakanti-amd changed the title fix(amdsmi): grade set sweep per GPU and report empty DPM tables fix(amdsmi): report empty DPM tables and reset FCLK cap Oct 22, 2026
@marifamd
marifamd marked this pull request as draft October 26, 2026 17:23
Signed-off-by: Koushik Billakanti <koushik.billakanti@amd.com>
@github-actions github-actions Bot added the documentation Improvements or additions to documentation label Oct 27, 2026
@koushikbillakanti-amd koushikbillakanti-amd changed the title fix(amdsmi): report empty DPM tables and reset FCLK cap fix(amdsmi): harden mixed-GPU set sweeps and clock resets Oct 27, 2026
Signed-off-by: Koushik Billakanti <koushik.billakanti@amd.com>
Signed-off-by: Koushik Billakanti <koushik.billakanti@amd.com>
Signed-off-by: Koushik Billakanti <koushik.billakanti@amd.com>
@koushikbillakanti-amd
koushikbillakanti-amd marked this pull request as ready for review October 27, 2026 12:36
Signed-off-by: Koushik Billakanti <koushik.billakanti@amd.com>

@gabrpham gabrpham left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please see comments below.

@koushikbillakanti-amd
koushikbillakanti-amd requested review from a team and marifamd October 30, 2026 04:46

@charis-poag-amd charis-poag-amd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

ROCM-33079 - states driver has empty pp_dpm_fclk file. Which is a driver issue.

Lets close this PR and re-assign to them (after providing evidence). Otherwise, if there are other issues we are fixing - please outline in the Jira ticket.

Signed-off-by: Koushik Billakanti <koushik.billakanti@amd.com>
@gabrpham

gabrpham commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

ROCM-33079 - states driver has empty pp_dpm_fclk file. Which is a driver issue.

Lets close this PR and re-assign to them (after providing evidence). Otherwise, if there are other issues we are fixing - please outline in the Jira ticket.

The PR still addresses other issues in ROCM-33079 which were the main issues the ticket wanted addressed in the first place. I would think we want this PR to go through but a new ticket raised for the pp_dpm_fclk empty file issue.

@charis-poag-amd charis-poag-amd left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for digging into this, Koushik. I left inline comments; the summary is below.

What ROCM-33079 shows

  • The rc=12 IO on --clk-limit FCLK MAX comes from the Raphael iGPU's pp_dpm_fclk containing only a newline. The parser already returns AMDSMI_STATUS_IO for that on develop, so nothing in AMD SMI needs to change for it. It is a driver bug, as Maisam noted on the ticket, and the fix belongs there.
  • MIXED_DEVICE_ERRORS (204) on the other FCLK commands is the CLI combining the MI300X's and the iGPU's different error codes.

Requests

  1. Please remove the changes that aren't needed for this ticket and aren't related to pp_dpm:
    • the reset --clocks FCLK cap restore, with its docs and tests
    • the readDevInfoMultiLineStr read-failure check
    • the assert(false) removal in rocm_smi.cc
    • the sweep rework
    • the --memory-partition exclusion
  2. Please don't make AMD SMI succeed on bad driver data. Blank lines in pp_dpm_* shouldn't be skipped, so a newline-only table stays IO.
  3. Please keep the sweep coverage: --memory-partition, --gpu all, and the clk-limit restore. The ticket asks to confirm the memory-partition restore works, not to remove it.
  4. If you keep any library change, say exactly which inputs change behaviour (for example, a zero-byte file going from SUCCESS to IO). Update the CHANGELOG to match.
  5. For tests, use the shared fixtures in test_fixture_utils.h (FakeSysfsTree, ScopedRocmSmiDevice) instead of FakeDpmDevice.
  6. Please file and link a driver ticket for the empty pp_dpm_fclk. Until it's fixed, mixed hosts will keep failing the FCLK commands, and the sweep shouldn't be changed to hide that.

Comment thread projects/amdsmi/tests/python/README.md Resolved
Signed-off-by: Koushik Billakanti <koushik.billakanti@amd.com>
@koushikbillakanti-amd koushikbillakanti-amd changed the title fix(amdsmi): harden mixed-GPU set sweeps and clock resets test(amdsmi): cover malformed FCLK tables Nov 3, 2026

@gabrpham gabrpham left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Comments have been addressed. Approved.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation organization: ROCm project: amdsmi

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants