Skip to content

fix(rocr): Return hipErrorNotReady when fabric export not ready#8293

Open
dayatsin-amd wants to merge 6 commits into
developfrom
users/dayatsin/fix-fabric-handle-support-check
Open

fix(rocr): Return hipErrorNotReady when fabric export not ready#8293
dayatsin-amd wants to merge 6 commits into
developfrom
users/dayatsin/fix-fabric-handle-support-check

Conversation

@dayatsin-amd

@dayatsin-amd dayatsin-amd commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Revert libhsakmt topology gating of FabricHandleSupported on ualink/accel_state; advertise fabric handle support when UALink sysfs is present.
  • Add ROCr accelerator readiness check (ualink/accel_state is ready or active) before exporting fabric handles, returning HSA_STATUS_ERROR_RESOURCE_NOT_READY.
  • Propagate the not-ready status through HIP as hipErrorNotReady and skip Unit_hipMemExportFabricHandleToStdout_Positive_Basic when the accelerator is not ready.
  • Add rocrtstFunc.VirtMemory_FabricExport_Readiness_Test to validate export behavior against sysfs accel_state on each GPU.
  • Address review feedback: bound sysfs reads, static_cast for GpuAgent, atomic readiness cache, and document FabricHandleSupported semantics.

This moves the readiness gate from capability reporting at topology time to export time, so tests can distinguish unsupported hardware from supported-but-not-ready fabric.

JIRA ID

JIRA ID : ROCM-27354

Test plan

  • Built debug libhsa-runtime64, libamdhip64, and VirtualMemoryManagementTest on b8-3 (gfx1260, accel_state=unconfigured)
  • Unit_hipMemExportFabricHandleToStdout_Positive_Basic SKIPs with hipErrorNotReady / "Accelerator not ready for fabric handle export." on b8-3
  • rocrtstFunc.VirtMemory_FabricExport_Readiness_Test PASSED on b8-3 (accel_state=unconfigured, export returns HSA_STATUS_ERROR_RESOURCE_NOT_READY)
  • Verified on AFM-provisioned f05-1 (gfx1250, accel_state=ready) that Unit_hipMemExportFabricHandleToStdout_Positive_Basic runs and fabric export succeeds
  • rocrtstFunc.VirtMemory_FabricExport_Readiness_Test PASSED on f05-1 (accel_state=ready, export and import succeed on all GPUs)

@therock-pr-bot

therock-pr-bot Bot commented Jul 9, 2026

Copy link
Copy Markdown

✅ All Policy Checks Passed

Check Status Details
🌿 Branch Name ✅ Pass
📝 PR Title/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.

@therock-pr-bot

This comment was marked as resolved.

@dayatsin-amd dayatsin-amd changed the title fix(rocr): Return hipErrorNotReady when fabric export accelerator is not ready rework(rocr): Return hipErrorNotReady when fabric export accelerator is not ready Jul 9, 2026
@dayatsin-amd dayatsin-amd changed the title rework(rocr): Return hipErrorNotReady when fabric export accelerator is not ready refactor(rocr): Return hipErrorNotReady when fabric export accelerator is not ready Jul 9, 2026

@cfreeamd cfreeamd 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.

🤖 AI-assisted pre-review (for @cfreeamd) — hsa_ext_amd.h (new status code), amd_gpu_agent.h declaration and constructor initialization, and the VirtIO/XDNA stubs look correct. Raised issues in amd_kfd_driver.cpp, runtime.cpp, amd_gpu_agent.cpp, and topology.c.

Comment thread projects/rocr-runtime/runtime/hsa-runtime/core/driver/kfd/amd_kfd_driver.cpp Outdated
Comment thread projects/rocr-runtime/runtime/hsa-runtime/core/runtime/runtime.cpp Outdated
Comment thread projects/rocr-runtime/runtime/hsa-runtime/core/runtime/amd_gpu_agent.cpp Outdated
Comment thread projects/rocr-runtime/libhsakmt/src/topology.c
Comment thread projects/rocr-runtime/runtime/hsa-runtime/core/inc/amd_gpu_agent.h Outdated
@dayatsin-amd dayatsin-amd changed the title refactor(rocr): Return hipErrorNotReady when fabric export accelerator is not ready fix(rocr): Return hipErrorNotReady when fabric export not ready Jul 9, 2026
@dayatsin-amd

Copy link
Copy Markdown
Contributor Author

Addressed review feedback in e3f2acc:

  • amd_kfd_driver.cpp: bound fscanf to %9s and treat read failure as error
  • runtime.cpp: use static_cast<AMD::GpuAgent*> after device-type check
  • amd_gpu_agent: std::atomic<bool> for accelerator_ready_ with relaxed load/store
  • topology.c: document that FabricHandleSupported is hardware presence; readiness is enforced at export time
  • amd_gpu_agent.h: remove extra blank line
  • PR title: shortened to satisfy therock-pr-bot (≤80 chars)

Re-tested after rebuild/install to /home/dayatsin/rocm:

System accel_state rocrtst HIP test
b8-3 unconfigured PASSED (RESOURCE_NOT_READY) SKIPPED (hipErrorNotReady)
f05-1 ready PASSED (export+import) PASSED

@dayatsin-amd dayatsin-amd requested a review from cfreeamd July 9, 2026 01:48
@dayatsin-amd dayatsin-amd force-pushed the users/dayatsin/fix-fabric-handle-support-check branch from e3f2acc to 5055e7f Compare July 9, 2026 02:35
dayatsin-amd and others added 2 commits July 11, 2026 09:15
Add a rocrtst that gates on VMM and fabric-handle support, reads
ualink accel_state from sysfs, and verifies export succeeds when the
accelerator is ready or active and returns RESOURCE_NOT_READY otherwise.

Co-authored-by: Cursor <cursoragent@cursor.com>
Bound sysfs accel_state reads, use static_cast for GpuAgent dispatch,
make accelerator_ready_ atomic, document FabricHandleSupported semantics,
and remove an extra blank line in amd_gpu_agent.h.

Co-authored-by: Cursor <cursoragent@cursor.com>
@dayatsin-amd dayatsin-amd force-pushed the users/dayatsin/fix-fabric-handle-support-check branch from 789ea27 to b0b4035 Compare July 11, 2026 13:15
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.

3 participants