Skip to content

test(rdc): make the fake amd-smi match amd-smi on three calls - #14980

Merged
joshnguy-amd merged 1 commit into
users/adam360x/rdc-fake-gpu-testsfrom
users/joshnguy/rdc-fake-amdsmi-review-fixes
Nov 4, 2026
Merged

joshnguy-amd merged 1 commit into
users/adam360x/rdc-fake-gpu-testsfrom
users/joshnguy/rdc-fake-amdsmi-review-fixes

Conversation

@joshnguy-amd

Copy link
Copy Markdown
Contributor

Address the review comments on #14041:

  • amdsmi_get_gpu_compute_process_gpus returns AMDSMI_STATUS_INVAL for a NULL list, as amd-smi does, instead of reporting a count. RDC's own NULL-count call then fails here as it does on real hardware.
  • amdsmi_init accepts AMDSMI_INIT_AMD_CPUS and finds no CPUs, as amd-smi does without the HSMP driver, so rdcd takes its production init path rather than the disable-CPUs retry.
  • The GPU-less daemon, harness and embedded launches set RDC_DISABLE_ROCP, so the profiler module is not loaded into the hermetic tests.

Motivation

#14041 runs rdcd and rdci in CI against a fake amd-smi. Review found three places where the fake behaves differently from real amd-smi. Each difference hides RDC behavior that this suite is meant to exercise: a NULL-count call that fails on hardware, the CPU-init fallback path taken by every rdcd start, and the profiler module loading into a hermetic test. This PR makes the fake match amd-smi on those calls so the suite tests what runs in production.

Technical Details

Verified each point against the real sources before changing the fake:

  • amd_smi.cc (amdsmi_get_gpu_compute_process_gpus) returns AMDSMI_STATUS_INVAL when dv_indices or num_devices is NULL. There is no count-only query. The fake now matches. This also means RdcSmiDiagnosticImpl.cc always takes its failure branch on real hardware, which is a separate RDC bug to file.
  • amd_smi_system.cc (populate_amd_cpus) returns AMDSMI_STATUS_SUCCESS when esmi_init() fails, so a CPU init never fails without the HSMP driver. The fake accepts AMDSMI_INIT_AMD_CPUS and reports no CPUs. rdcd's "disable CPUs and retry" fallback in RdcEmbeddedHandler.cc is no longer taken.
  • RDC_DISABLE_ROCP=yes is set in the rdcd_server fixture in conftest.py, in the daemon harness environment in test_fake_gpu.py, and in the embedded test, as test_gtest_runner.py already does.
  • Two tests in test_fake_gpu_fake.py and the README now expect INVAL for a NULL list.

This PR targets users/adam360x/rdc-fake-gpu-tests and does not merge develop. #14041 conflicts with develop in rdc-ci.yml (the rdc_test runner label and timeout), which the #14041 author needs to resolve.

Issue Tracking

GitHub issue: #14041
JIRA ID:

Test Plan

  • Compile fake_amdsmi.cc and run tests/ci/test_fake_gpu_fake.py, which drives the fake through ctypes.
  • Run test_fake_gpu.py and tests/ci in CI (rdc_test) to check that the stricter amd-smi behavior doesn't expose new rdcd/rdci failures.

Test Result

  • test_fake_gpu_fake.py: 24 passed locally against the compiled fake.
  • test_fake_gpu.py and tests/ci: not run locally, because they need a built RDC. They run in CI on this PR.

Submission Checklist

Address the review comments on #14041:

- amdsmi_get_gpu_compute_process_gpus returns AMDSMI_STATUS_INVAL for a NULL
  list, as amd-smi does, instead of reporting a count. RDC's own NULL-count
  call then fails here as it does on real hardware.
- amdsmi_init accepts AMDSMI_INIT_AMD_CPUS and finds no CPUs, as amd-smi does
  without the HSMP driver, so rdcd takes its production init path rather than
  the disable-CPUs retry.
- The GPU-less daemon, harness and embedded launches set RDC_DISABLE_ROCP, so
  the profiler module is not loaded into the hermetic tests.

Co-Authored-By: Claude <noreply@anthropic.com>
@joshnguy-amd
joshnguy-amd requested review from a team as code owners November 3, 2026 22:22
@joshnguy-amd
joshnguy-amd merged commit daf94a9 into users/adam360x/rdc-fake-gpu-tests Nov 4, 2026
100 of 105 checks passed
@joshnguy-amd
joshnguy-amd deleted the users/joshnguy/rdc-fake-amdsmi-review-fixes branch November 4, 2026 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant