[rocr-runtime] Make the DXG topology snapshot safe for multiple consumers by ihhethan · Pull Request #10034 · ROCm/rocm-systems

ihhethan · 2026-08-12T00:55:34Z

Motivation

librocdxg already exposes the WSL GPU topology through the standard KMT entry points:

hsaKmtOpenKFD()
hsaKmtAcquireSystemProperties()
hsaKmtGetNodeProperties()
hsaKmtReleaseSystemProperties()
hsaKmtCloseKFD()

This PR does not introduce a new topology interface. It fixes the lifetime and
ownership issues that arise when multiple components in the same process use
the existing DXG topology path.

ROCr is already one consumer, AMD SMI added a WSL GPU backend in #9725, and
#7016 adds rocprofiler-sdk as another. AMD SMI, for example, acquires the
topology in WSLGPUBackend::TryPopulate() and holds it until Shutdown().

The issues addressed here are:

the DXG topology snapshot was process-global and unreferenced, so one
consumer could release topology still in use by another;
hsaKmtOpenKFD() reset the fragment allocator on every open, including when
allocations from another consumer were still live;
hsaKmtCheckRuntimeDebugSupport() acquired a topology snapshot without
releasing it.

This PR was originally broader. At the reviewers' request, changes not required
for topology access have been split into two separate PRs so this one can be
reviewed as the topology prerequisite alone:

independent DXG fixes:
NumCUPerArray, KFDGpuID, failed-open descriptor cleanup, and the
librocdxg.pc prefix;
failed-initialization unwind:
KfdLifecycle, Runtime::Load() rollback, thunk API validation, and DTIF
ownership.

Technical Details

Topology snapshot reference counting

A new src/dxg/snapshot_refcount.hpp isolates the reference-count transitions
so they can be tested without a GPU, DXCore adapter, or open thunk.

hsaKmtAcquireSystemProperties() now increments the reference count when
reusing an existing snapshot and initializes the count when publishing a newly
built snapshot.

hsaKmtReleaseSystemProperties() destroys the snapshot only when the final
reference is released. An unmatched release is rejected rather than tearing
down topology still owned by another consumer.

Failed topology construction is fully unwound so a later acquire cannot reuse
partially initialized state.

The final hsaKmtCloseKFD() also releases any remaining snapshot before DXCore
is destroyed, because the snapshot contains WDDMDevice objects owned by that
session.

`hsaKmtOpenKFD()` open count and allocator lifetime

Both the first-open and already-open paths maintain dxg_open_count, so it
tracks the number of live consumers rather than saturating at one.

The already-open increment is pre-existing code and appears only as unchanged
context between two diff hunks, which can make the change look one-sided when
reviewing the patch.

The functional change is the placement of reset_suballocator(). It now runs
only on the first open. Previously, a second consumer could reset allocator
bookkeeping while allocations from the first consumer were still live.

Fork handling

child_fork_handler() clears the inherited open count and snapshot references
using plain stores only, keeping the atfork child handler minimal.

clear_after_fork() later abandons the parent's WDDMDevice pointers without
running their destructors, since doing so would issue D3DKMT calls using handles
created by the parent process. The last-close path follows the same ownership
rule in a forked child.

hsaKmtReleaseSystemProperties() now verifies that DXG is open before releasing
a reference, preventing a child from releasing topology it never acquired.

hsaKmtOpenKFD() also calls clear_after_fork() before taking the runtime lock.
Because clear_after_fork() may replace the runtime object, taking the lock
first could leave the lock_guard referring to freed storage.

Debug support query

hsaKmtCheckRuntimeDebugSupport() previously called
hsaKmtAcquireSystemProperties() without releasing the acquired reference.

It now takes no snapshot reference of its own and returns NOT_SUPPORTED when
the device list is empty, leaving topology ownership to the caller.

This change must land together with the explicit acquisition in
KfdDriver::Init(): the debug probe was where ROCr previously acquired its
snapshot reference as a side effect. Removing that behavior without adding the
explicit acquisition would leave ROCr holding no reference at all.

ROCr: accepting an already-open DXG thunk

KfdDriver::Open() now treats HSAKMT_STATUS_KERNEL_ALREADY_OPENED as success
when thunkLoader()->IsDXG().

This is valid for DXG because librocdxg reference-counts opens, so the
already-open path has acquired this consumer's handle reference. The native
thunk retains the existing behavior.

KfdDriver::Init() now explicitly acquires the driver's long-lived topology
reference before runtime enablement instead of relying on the former side
effect of the debug-support query. Failure paths release exactly what they
acquired.

Relationship to #7016

#7016 is the rocprofiler-sdk side of the WSL profiling work. It obtains topology
through the same five existing KMT entry points and requires no new topology
API.

The dependency on this PR is the snapshot lifetime mechanism.

On develop, hsaKmtReleaseSystemProperties() destroys the single global
snapshot unconditionally. With multiple consumers, that means one consumer can
release topology still held by ROCr or AMD SMI.

The normal rocprofv3 path in #7016 is safe through initialization ordering:
its open/acquire/release/close sequence completes from the library constructor
before main(), and therefore before the application's hsa_init() or
amdsmi_init().

Late attach has no equivalent ordering guarantee. For that reason, #7016
temporarily disables WSL enumeration when loaded through rocprof-attach.
That restriction can be removed once this PR is available.

There is no CI, submodule, or build-time mechanism enforcing merge order
between #7016 and this PR. They may merge in either order.

The only file currently modified by both is
libhsakmt/CMakeLists_wsl.txt: this PR adds the BUILD_ROCDXG_TESTS wiring,
while #7016 adds PM4 frame-ring test wiring. The changes merge cleanly today,
as verified with git merge-tree.

JIRA ID

JIRA ID: ROCM-23912

Test Plan

Test Result

Submission Checklist

Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests.

therock-pr-bot · 2026-08-12T00:56:27Z

✅ All Policy Checks Passed

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	⚠️ Warning	Error: Source/code files changed without an accompanying unit test. Expected: add at least one test file named like `test_<name>.py` / `test_<name>.cpp` (or `<name>_test.`). Current:* code file(s) changed: `projects/rocr-runtime/libhsakmt/include/hsakmt/hsakmt.h`, `projects/rocr-runtime/libhsakmt/src/dxg/debug.cpp`, `projects/rocr-runtime/libhsakmt/src/dxg/librocdxg.h`, `projects/rocr-runtime/libhsakmt/src/dxg/openclose.cpp`, `projects/rocr-runtime/libhsakmt/src/dxg/topology.cpp` (+3 more); no test file found
🚫 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?

… engine NumArrays is the number of shader arrays in one shader engine, so the whole GPU has NumShaderBanks * NumArrays of them. Dividing the compute unit count by NumArrays alone over-reports NumCUPerArray by the shader engine count: a 128 CU part with 4 shader engines and 2 arrays each was reported as 64 CU per array instead of 16. Divide by the total number of arrays instead, which is the relation KFD reports on bare metal. Fold the existing zero check into the same guard so a zero shader engine count cannot divide by zero either, and warn when the CU count is not a multiple of the array count, since the result is truncated there.

KFDGpuID was filled in from the PCI device id. The rest of topology.cpp uses it both as the "this node is a GPU" predicate and as the key that gpuid_to_nodeid() and get_device_id_by_gpu_id() invert, and a PCI device id satisfies neither: two identical GPUs in one machine share one, so both nodes claim the same id and every reverse lookup resolves to the first of them. Use the node ordinal instead. It is unique by construction and stable for the life of the snapshot, which is the only scope these lookups span, and it is never handed to the KMD so it does not have to match any driver-side id. Bias it by one so a GPU can never land on the reserved zero.

ROCr and the DXG thunk copy HsaNodeProperties across their interface without a size parameter, so a disagreement between the two builds corrupts the heap instead of failing the load, and nothing detects it. Add DxgAbiCheck, which takes the sizes the caller was built with and answers HSAKMT_STATUS_DRIVER_MISMATCH when one differs. ROCr resolves it with dlsym(), so a thunk built before the handshake existed reads as an older thunk rather than as a failure. Install hsakmt.h and hsakmttypes.h alongside the library. This is the only configuration that builds librocdxg and it is mutually exclusive with the libhsakmt branch that installs include/hsakmt wholesale, so without a rule here the dev package ships a pkg-config file naming an empty include directory. Add tests that compile both headers out of a staged copy of exactly that list, and that read the built library for the export ROCr will look up, then run them in the WSL job.

cfreeamd

🤖 AI-assisted pre-review (for @cfreeamd)

Reviewed projects/rocr-runtime/ only. The lifecycle core is solid: SnapshotRefcount is balanced/ABA-free, KfdLifecycle teardown is correctly ordered and idempotent, and the Runtime::Load/AMD::Load scope guards leave no leak or double-free window — all well covered by the new standalone unit tests. Two correctness concerns and a wiring question are inline; the license-header and typo notes are nits.

hsaKmtReleaseSystemProperties() tore the snapshot down unconditionally, so with two consumers in one process the first release freed the HsaNodeProperties the second was still reading. ROCr and rocprofiler-sdk are exactly that pair. Count the outstanding hsaKmtAcquireSystemProperties() references and drop the snapshot only when the last one goes. Reject an unmatched release rather than tearing down a snapshot somebody still owns, and unwind a snapshot that failed to build so a retry starts from the state the failed call found rather than on top of live device objects. Drop whatever is still standing at the last hsaKmtCloseKFD(): the snapshot names WDDMDevice objects belonging to the dxcore session that close is about to end, and a later reopen would hand those pointers back out. A fork child disowns its inherited references instead, because destroying them would issue D3DKMT calls against the parent's handles.

librocdxg.pc derived its prefix by walking three directories up from ${pcfiledir}, which is one too many: the file is installed to <prefix>/lib/pkgconfig, so three levels up lands above the prefix entirely. Every path the file emits was wrong by one level, and the comment explaining the choice described borrowing libhsakmt's headers, which this configuration does not build. Walk up two instead, so includedir resolves to the directory the header install rule writes hsakmt/ into, and say in the comment what actually populates it. Nothing else does: libhsakmt is not built in this configuration, so this package ships those headers or nobody does.

LoadThunkApiTable() returned void. Binding stops at the first entry point the shared thunk does not export, so everything after it in the table keeps whatever its member happened to hold and Load() carries on regardless. The first call through one of those slots is a fault the runtime has no way to attribute, and an installed thunk older than the runtime loading it is an ordinary configuration rather than a corrupt one. Report whether the table was fully bound and abandon the load when it was not. Give every pointer in the table a nullptr initializer as well, so a slot binding never reached holds a null that faults naming the function called rather than an indeterminate value that jumps somewhere arbitrary.

DestroyThunkInstance() looked DtifDestroy up and called it whenever it resolved, with no regard for whether an instance had ever been created. The rollback for a failed load runs it too, so a load that failed before CreateThunkInstance() destroyed an instance it never took. The loader also accepted a thunk exporting DtifCreate but not DtifDestroy, handing over an instance nothing in the process can give back, and only found that out during its own unwind with the library about to be closed. Resolve both halves before calling either and refuse a thunk that does not export both, the rule ImportExternalSemaphore() already applies to the other optional pair in the table. Record the destroy function as the instance is taken and release through that, clearing it before the call so a failed destroy cannot be retried into a second release. Owning nothing is then plain success, which is what the rollback path needs.

Acquire() keeps the singleton after a failed Load() and only drops the reference count, so Unload() never runs and everything Load() took before the failure stays. The next hsa_init() finds the drivers still registered and the agents still published, and because both are published by appending, it doubles them and re-runs Init() on a driver this process has already torn down. agents_by_gpuid_ likewise held pointers to agents DestroyAgents() had freed. Unwind on the failure path with scope guards that give back the topology, the loader, the extensions and the thunk loader in the reverse of the order Load() acquires them, sharing that teardown with Unload() so the two cannot drift. Every step tolerates the half-built state a failed load leaves behind and is safe to run more than once. Put the KFD open and the topology snapshot behind KfdLifecycle, which owns them as a pair and gives them back together, and hold the thunk loader by unique_ptr so a second attempt cannot overwrite the first and leak the shared thunk it keeps open.

cfreeamd

🤖 AI-assisted pre-review (for @cfreeamd) — follow-up after 2026-08-15 author updates

Thank you for the extensive rework. Several prior findings are now resolved:

[Important] #2 (abi.cpp compile) — resolved: new CI workflow + build-system-provided include paths confirm compilation succeeds.
[Question] #3 (DxgAbiCheck not wired) — resolved: DxgAbiCheck is now declared in hsakmt.h, exported in librocdxg.ver, and per the test comments in installed_header_smoke.cpp ('Filled the way ThunkLoader::CheckThunkAbi() fills it'), CheckThunkAbi() now calls it.
[Nit] #5 ("Snashot" typo) — resolved: comment replaced entirely.

[Important] #1 (device-handle refcount) — clarification needed. See inline comment on openclose.cpp.

ihhethan · 2026-08-25T14:14:49Z

🤖 AI-assisted pre-review (for @cfreeamd) — follow-up after author update

The refactor(libhsakmt): drop the DxgAbiCheck structure-size handshake commit resolves the two prior abi.cpp findings: DxgAbiCheck is gone entirely and the installed-header ABI concern is now handled the right way via forward declarations for kfd_runtime_info / kfd_dbg_device_info_entry in hsakmt.h, backed by the new installed_header_smoke.{c,cpp} standalone-compile tests. This also matches @fcui-amd's point that the monorepo already guarantees ABI match at build time.

One prior [Important] item still looks open: the dxg_open_count refcount on the KERNEL_ALREADY_OPENED path (see the unanswered inline follow-up at openclose.cpp). dxg_open_count++ is still only inside the if (dxg_open_count == 0) branch, so the count can only ever go 0->1; the ALREADY_OPENED else branch takes no reference. But KfdDriver::Open()'s comment still asserts that else branch "takes the handle reference on our behalf … so the matching Close() in ShutDown() drops exactly that one." In the two-consumer WSL case (rocprofiler-sdk opens first -> count=1; ROCr opens -> ALREADY_OPENED, count stays 1; ROCr ShutDown -> Close -> --count 1->0), hsaKmtCloseKFD() runs topology_drop_snapshot_at_last_close() + HeapFini() + close(dxg_fd) and tears down the DXCore session under the still-live rocprofiler-sdk consumer. The separate SnapshotRefcount doesn't cover this, since the DXCore/fd teardown is gated on dxg_open_count, not the snapshot refcount. Fix is either to increment dxg_open_count on the ALREADY_OPENED path (make it a true refcount) or to correct the driver comment + Open()/ShutDown() so they don't assume a reference was taken.

This is already handled at the current tip. The KERNEL_ALREADY_OPENED branch increments dxg_open_count at openclose.cpp:729 before returning, so the two-consumer sequence is 0→1→2, then ROCr shutdown drops it to 1 without running last-close teardown. The second consumer increment predates this PR and appears as an unchanged context line, which may be why it was missed in the diff. The substantive change in this PR was moving reset_suballocator() into the 0→1 branch.

cfreeamd

🤖 AI-assisted pre-review (for @cfreeamd) — follow-up after author reply

Thanks for the detailed reply. You're right, and I've withdrawn the prior [Important] dxg_open_count finding. I fetched the full openclose.cpp at the current tip and confirmed the KERNEL_ALREADY_OPENED else branch does increment the count:

} else {
  dxg_runtime->dxg_open_count++;              // openclose.cpp:729
  result = HSAKMT_STATUS_KERNEL_ALREADY_OPENED;
}

That line lives in the gap between the two diff hunks (hunk 1 ends at new line ~717, hunk 2 starts at new line 730), so it never appeared in the changeset context — which is why it read as one-sided across the earlier passes. With both open paths incrementing and hsaKmtCloseKFD() gating last-close teardown on --dxg_open_count == 0, the refcount is balanced: in the two-consumer WSL case (rocprofiler-sdk 0->1, ROCr 1->2 ALREADY_OPENED, ROCr shutdown 2->1) the DXCore session correctly stays alive under the live consumer. No remaining blockers from my side — prior findings #2/#3 are moot after the DxgAbiCheck removal, and the ABI concern is handled cleanly via the forward declarations + installed_header_smoke tests.

Flora Cui noted on review that the hsakmt public headers are already installed by rocr-runtime, and asked whether a second copy shipped from the WSL build could put rocprofiler-sdk and rocr-runtime on mismatched headers now that both live in one repo. Both points land on the same defect, and develop already said so: the comment this rule displaced explains that no separate header installation belongs here and that duplicating it breaks TheRock's artifact uniqueness check (ROCm/TheRock#7005). include/hsakmt is installed by the else() branch of if (WIN_SDK) in libhsakmt/CMakeLists.txt, which the native package runs, and in a full ROCm install both copies land in the same prefix. Restore develop's comment verbatim so the rule is not re-added, and drop the installed_header_smoke targets with it. They only checked that the installed header set compiles standalone, so with no install rule they have no subject. dxg-snapshot-refcount and dxg-dynamic-exports still build and pass. Keep the hsakmt.h forward declarations. They are not test scaffolding: src/extsem.c includes only <hsakmt/hsakmt.h>, and without them GCC scopes struct kfd_runtime_info and struct kfd_dbg_device_info_entry to the prototypes that name them. linux/kfd_ioctl.h defines both and is excluded from the dev package, so these declarations are what make those prototypes well formed for any C consumer.

ihhethan · 2026-08-26T18:16:57Z

🤖 AI-assisted pre-review (for @cfreeamd) — follow-up after author reply

Thanks for the detailed reply. You're right, and I've withdrawn the prior [Important] dxg_open_count finding. I fetched the full openclose.cpp at the current tip and confirmed the KERNEL_ALREADY_OPENED else branch does increment the count:
} else {
  dxg_runtime->dxg_open_count++;              // openclose.cpp:729
  result = HSAKMT_STATUS_KERNEL_ALREADY_OPENED;
}
That line lives in the gap between the two diff hunks (hunk 1 ends at new line ~717, hunk 2 starts at new line 730), so it never appeared in the changeset context — which is why it read as one-sided across the earlier passes. With both open paths incrementing and hsaKmtCloseKFD() gating last-close teardown on --dxg_open_count == 0, the refcount is balanced: in the two-consumer WSL case (rocprofiler-sdk 0->1, ROCr 1->2 ALREADY_OPENED, ROCr shutdown 2->1) the DXCore session correctly stays alive under the live consumer. No remaining blockers from my side — prior findings #2/#3 are moot after the DxgAbiCheck removal, and the ABI concern is handled cleanly via the forward declarations + installed_header_smoke tests.

Thanks, that diagnosis was helpful. The missing increment was in the unchanged context between the two hunks, so I should have made that clearer in the description.

Teardown previously stopped at the first failing step, so a failed topology- snapshot release could leave the DXCore session open. Complete every owned cleanup step while retaining the first error. Name the missing topology acquire when the debug-support query cannot inspect devices. Narrow the atfork comment because the pre-existing handler also writes a bitfield and reconstructs a mutex, so it does not follow the plain-store-only discipline previously claimed.

cfreeamd

🤖 AI-assisted pre-review (for @cfreeamd) — follow-up after 2026-08-27 commit

Reviewed the new commit fix(rocr-runtime): finish DXG cleanup after teardown errors (2026-08-27T22:45:01Z). This commit completes the teardown correctness work:

topology_drop_snapshot_at_last_close() ensures the snapshot cannot outlive the DXCore session that backs it; called before HeapFini() in hsaKmtCloseKFD() so WDDMDevice teardown still has a live session.
Lock in hsaKmtOpenKFD() moved after clear_after_fork(): correct because clear_after_fork() replaces *dxg_runtime, so a lock_guard on the old object would unlock freed memory. Fork path is single-threaded so the unlocked check is safe.
reset_suballocator() moved inside the 0→1 branch only: second consumer's ALREADY_OPENED open no longer resets the first consumer's live allocations.
AcquireTopologySnapshot() in KfdDriver is idempotent (topology_snapshot_acquired_ guard), so repeated GetSystemProperties() calls don't accumulate unbalanced thunk snapshot references.
ShutDown() runs disable→release→close regardless of intermediate failures, reporting the first error instead of short-circuiting on it.
Init() uses named scope guards (snapshot_guard, runtime_guard) for correct unwind on early failure; dismissed in order on success.
Fork child cleanup: child_fork_handler() now zeroes dxg_open_count and clears snapshot refs before any entry point can act on them; clear_after_fork() calls topology_abandon_after_fork() to discard parent WDDMDevice references without deleting them.

All paths verified against the diff. No new concerns. Verdict remains approve.

The class wrapped a plain uint32_t behind five one-line methods and a three-way enum, which is more type than the job needs. Replace it with a std::atomic<uint32_t> member on _topology_props and inline the two things the class actually carried: the assignment rather than an increment when a snapshot is first published, and the zero check that stops an unmatched release from wrapping the count to UINT32_MAX and stranding the snapshot for the life of the process. Note that fetch_sub returns the value from before the subtraction, so the last-reference test reads == 1. Every access uses memory_order_relaxed, because hsakmt_mutex is what actually orders them: the teardown a final release triggers frees g_system, clears g_props and deletes the WDDMDevice objects, so that path holds the lock however the counter is typed. The member comment records this, since the guard and the decrement are now two operations rather than one call and the atomic on its own no longer makes the field safe to touch unlocked. Removing the class removes its unit test, and with it the last consumer of tests/unit_test_harness.h, so both go as well. The harness belongs with the lifecycle tests that are the first to actually need it. The fork-child handler comment in openclose.cpp is corrected to match: the snapshot count is now a relaxed store on a lock-free atomic rather than a plain store, which is still free of allocation and locking. topology.hpp is written back as uniformly CRLF. The commit that added snapshot_refcount.hpp had given its new lines bare LF endings, the only such lines in an otherwise CRLF file.

cfreeamd

🤖 AI-assisted pre-review (for @cfreeamd) — follow-up after 2026-08-31 refactor

refactor(libhsakmt): replace SnapshotRefcount with std::atomic (2026-08-31) assessed.

The std::atomic<uint32_t> replacement is correct. All prior SnapshotRefcount invariants are preserved:

Unbalanced-release check (count == 0 → pr_err + early return before fetch_sub): preserved.
Last-reference teardown (fetch_sub(1, relaxed) == 1 → topology_drop_snapshot()): correct.
Fork-child zeroing (topology_clear_snapshot_refs() → relaxed store): async-signal-safe.

Relaxed ordering at all call sites is appropriate — hsakmt_mutex provides the required acquire/release semantics for the surrounding data structures; the atomic prevents UB on the counter variable itself.

The lock_guard move (constructed after is_forked_child() check instead of before) is a genuine correctness fix: clear_after_fork() replaces *dxg_runtime, so a pre-fork lock_guard bound to the old object would call unlock() on freed memory when it went out of scope.

Prior cfreeamd APPROVED (2026-08-28) is confirmed valid for the current head.

jbonnell-amd

Approved for the .github/workflows changes

amd-justchen · 2026-09-02T23:32:13Z

Will merge given code owner approval.
The TheRock CI failures are only known infra issues, unrelated to the rocr-runtime DXG topology snapshot change:

Windows (gfx1151) / Build Windows Packages: the therock-amd-mesa third-party sub-project could not find its lua dependency on the runner (lua54 found: NO) and aborted its preliminary CMake check, a toolchain / provisioning failure before any PR code was compiled
Linux gfx94X-dcgpu / Test rocgdb-gpu (xfail): expected-fail lane, advisory only
Linux MI455 Test / Test hip-tests: shared container-infra failure seen across several unrelated PRs, not caused by this diff
Linux MI455 Test / Test rocrtst: shared runner / harness infra failure across unrelated PRs

systems-assistant · 2026-09-03T17:50:36Z

TheRock Submodule Bump Activity

Newest first

2026-09-03 — Included in TheRock via ROCm/TheRock#7862.

projects/rocr-runtime/tests/unit_test_harness.h has no consumer left. 565fcfa re-added it on this branch - #10034 had deleted it once the refcount test stopped using it - on the stated grounds that tests/dxg/kfd_lifecycle_test.cpp still needed it. 59f85ba deleted that test when it folded KfdLifecycle into KfdDriver, which left the header as the only thing this branch still adds under tests/. A repo-wide grep for unit_test_harness now matches nothing at all, not even an include: the header uses #pragma once, so its own text does not contain the name. No CMakeLists mentions it and no add_subdirectory() pulls in projects/rocr-runtime/tests, so nothing built or installed it. Deleting it here rather than leaving it to #10034 cannot cause a merge conflict, because #10034 already merged as 80763b5 and its removal of this header is an ancestor of this branch. origin/develop has no projects/rocr-runtime/tests directory, so this restores develop's state instead of duplicating another PR's deletion. Leaving the file would mean this branch resurrects a header develop had already retired. KfdDriver::ShutDown()'s record() lambda is deliberately left alone. Its stage parameter is genuinely unused once NDEBUG reduces debug_print to do {} while (false), but hsa-runtime64 is compiled with -Wall and without -Wextra, -Wunused-parameter or -Werror (core/util/utils.h, cmake_modules/hsa_common.cmake, runtime/hsa-runtime/CMakeLists.txt), and -Wunused-parameter is not implied by -Wall, so no build configuration defined in this tree diagnoses it. Adding a suppression would be dead weight. Nothing here was compiled or run - this machine has no compiler or CMake, and a deleted header and an unchanged lambda are not testable without one. Every claim above comes from reading the CMake flag chain and grepping the tree.

…7016) ## Motivation Enable `rocprofv3` counter collection and profiling on WSL2 for gfx11-class GPUs. On WSL, the complete KMT topology is read up front through `librocdxg`, using the same thunk interface as the HSA runtime. An agent is published only after all required topology information has been collected, so the published record remains stable for its lifetime. As a result, `rocprofiler_query_available_agents()` returns consistent agent information regardless of when it is called, including before the HSA runtime has initialized. This is required by `rocprofv3-avail` and counter metric lookup. Native Linux continues to use the existing `gnulinux` path. `select_platform()` (`agent.cpp:355`) checks `gnulinux::is_available()` first, which succeeds whenever the KFD sysfs nodes directory is present, so the WSL path is not selected on a normal Linux system. There are two ungated changes that also affect native Linux. Both are fixes and are called out under **Native-Linux impact** below. This work builds on [[#6399](#6399)], which introduced the platform dispatch structure used here. `platform_dispatch.cpp` and its six tests came from that PR (`d2f0b27ad5`) and are therefore listed as pre-existing under **Tests**. This PR adds WSL as another implementation behind the existing `select_platform()` interface. ## Dependency on [#10034](#10034) [#10034](#10034) contains the libhsakmt and ROCr prerequisites for this work. There is no CI gate, submodule pin, or build-time dependency that enforces merge order, so the two PRs can technically merge in either order. Their changes to `core/runtime/amd_topology.cpp` also do not overlap. However, two parts of this PR depend on #10034 before they are fully functional. ### 1. WSL enumeration is disabled in a stock build until the required hsakmt headers are installed WSL enumeration is controlled by the CMake probe `rocprofiler_sdk_dxg_topology_supported()` (`cmake/Modules/rocprofiler-sdk-utilities.cmake:120`). The probe requires the hsakmt headers used by the build to provide: * `HsaStructureSizes` * `HsaNodeProperties::LuidLowPart` * `HsaNodeProperties::LuidHighPart` * `OverrideEngineId` Released ROCm headers, including 7.2.4, do not provide all of these. When the probe fails, `agent_unsupported.cpp` is built instead of the WSL enumerator, `platform::wsl::is_available()` returns false, and `enumerate()` returns an empty result. The copies of `hsakmttypes.h` in this repository already contain the required definitions (lines 359-360 and 1578), but they are not currently installed. #10034 adds the WSL header installation rules in `libhsakmt/CMakeLists_wsl.txt` through `ROCDXG_PUBLIC_HEADERS`. Until that lands, a stock build of this PR will not enable WSL enumeration. ### 2. The DXG topology snapshot is not reference-counted on current `develop` Current `develop` unconditionally calls `topology_drop_snapshot()` from `hsaKmtReleaseSystemProperties()` (`libhsakmt/src/dxg/topology.cpp:417`). This frees the global topology snapshot and destroys its `WDDMDevice` objects even if another consumer still has the topology acquired. #10034 adds `src/dxg/snapshot_refcount.hpp` and makes snapshot destruction conditional on the reference count. The normal `rocprofv3` path is not affected because the SDK topology read happens from `ROCPROFILER_LIBRARY_CTOR` and completes before `main()`, and therefore before the application's `hsa_init()`. `rocprof-attach` is different because it injects into an already-running process. In that case there is no ordering guarantee between the SDK and the existing HSA runtime consumer, so late attach can fail until the reference-counting change from #10034 is present. ## Technical Details ### Agent enumeration (WSL) * `platform/wsl/dxg_topology.cpp` and `dxg_thunk.hpp` read KMT topology through `librocdxg` using: * `hsaKmtOpenKFD()` * `hsaKmtAcquireSystemProperties()` * `hsaKmtGetNodeProperties()` * `hsaKmtReleaseSystemProperties()` * `hsaKmtCloseKFD()` The symbols are resolved with `dlsym`. `DxgThunk::complete()` verifies that all five are available before any are called. * `HsaNodeProperties` has no size parameter, so the destination buffer includes `kNodePropsSlack = 128` bytes of additional space. * `platform/wsl/agent.cpp` matches DXG adapters to HSA nodes by LUID, with device ID as a fallback. Enumeration is allowed to be partial: an unmatched adapter is skipped with a diagnostic while successfully matched GPUs are still published. * Memory banks, caches, and IO links are not currently published by the WSL path. * If the thunk reports no GPU node, no GPU agent is published. A warning is emitted and the application continues. * `agent_mapping.{hpp,cpp}` adds a WSL-specific mapping policy. Native Linux keeps the existing strict behavior (`agent.cpp:755`), while WSL logs mismatches, keeps successfully paired GPUs, and continues. ### Counters * `counters/controller.cpp` emits a one-time warning that WSL counters are collected through the DXG vendor-packet path rather than the KFD profiler ioctl. * `WSLKMT_VENDOR_PACKET=1` is now set from `wsl::enumerate()` instead of `is_available()`. This matters when using `ROCPROFILER_FORCE_PLATFORM=wsl`, where the old placement was bypassed. Without the environment variable, the DXG path drops aqlprofile's vendor-specific PM4 IB and counter values are returned as zero. * `basic_counters.xml` and `derived_counters.xml` each gain an empty `<gfx1150 base="gfx11">` section to keep the installed metric files consistent. These XML files are not used by the runtime metric loader. Runtime metrics come from `config.yaml` (`counters/metrics.cpp:348`), which already supports gfx1150 and is unchanged by this PR. ### KFD * `kfd::init()` now probes for a usable KFD device. If `/dev/kfd` is unavailable or cannot be opened, initialization continues with a warning instead of aborting in `kfd_device_fd()`. ## Native-Linux impact Two changes are not WSL-gated and also apply to native Linux: * `aql/helpers.cpp:51`: `get_query_info()` returns an empty ID query for metrics that do not have a hardware block. Constant and topology metrics such as `simd_count` have an empty block name. Passing that empty name to `aqlprofile_get_pmc_info()` can dereference a null pointer and abort. * `kfd/kfd.cpp:1586`: the KFD availability check is unconditional. A system without a usable `/dev/kfd`, for example because the driver is not loaded or access is denied, now receives a warning and continues instead of aborting. Other rocprofiler services are unaffected. All other WSL-specific behavior remains isolated from native Linux. The DXG implementation requires `/dev/dxg` and `libdxcore.so`, and the PM4 command-buffer bounds change is limited to libhsakmt's DXG/WDDM submission path. ## Tests * **44 new tests** * 14 `wsl_dxg_topology` tests * 16 `wsl_dxg_thunk` tests * 10 `agent_mapping` tests * 4 `parse_gfx_target_version` tests The first two groups are in new executables. The mapping and target-version tests are added to the existing `rocprofiler-sdk-lib-tests`. None of these tests requires a GPU, HSA runtime, or real DXG thunk. The DXG tests use a fake thunk and fake dynamic loader. The 30 DXG-specific tests use the same CMake capability probe as the WSL enumerator, so they are only built when the required hsakmt interface is available. In a stock configuration, 14 of the 44 new tests are built. * `platform-dispatch-tests` and its six tests are pre-existing from #6399. * `rocprofv3-avail` * Three tests are moved out of the PC-sampling gate and are now enabled on Navi. * The L2 hit counter is selected by GPU family instead of being hardcoded. These tests have not previously run in Navi CI, so they may expose existing Navi-specific issues. * `hsa_barrier` * Skip when no GPU agent is available instead of dereferencing an empty agent map. * `agent` * `agent_visibility_multigpu` performs its `substr(1)` calls after the no-GPU skip, avoiding `std::out_of_range` when no agents are present. * `grid_max_size` tolerates the difference between rocprofiler's hardcoded `INT32_MAX` value and the value reported by the installed ROCr runtime. ## JIRA ID JIRA ID: ROCM-23912 ## Test Plan ## Test Result ## Submission Checklist - [ ] Look over the contributing guidelines at https://github.com/ROCm/ROCm/blob/develop/CONTRIBUTING.md#pull-requests. --------- Co-authored-by: Mika Laitio <mika.laitio@amd.com>

ihhethan requested review from a team, atgutier, cfreeamd, chrispaquot, dayatsin-amd, fcui-amd, gandryey, kentrussell, shwetagkhatri and yuliashi-amd as code owners August 12, 2026 00:55

github-actions Bot added project: rocr-runtime github actions Pull requests that update GitHub Actions code labels Aug 12, 2026

systems-assistant Bot added the organization: ROCm label Aug 12, 2026

fcui-amd reviewed Aug 12, 2026

View reviewed changes

Comment thread projects/rocr-runtime/libhsakmt/include/hsakmt/hsakmt_dxg.h Resolved

fcui-amd reviewed Aug 12, 2026

View reviewed changes

Comment thread projects/rocr-runtime/libhsakmt/src/dxg/debug.cpp Resolved

ihhethan added 3 commits August 15, 2026 01:10

ihhethan force-pushed the users/ihhuang/rocr-dxg-topology-lifecycle branch from 22c3bdd to 7609f04 Compare August 15, 2026 15:11

cfreeamd reviewed Aug 15, 2026

View reviewed changes

ihhethan force-pushed the users/ihhuang/rocr-dxg-topology-lifecycle branch from 7609f04 to a993bb5 Compare August 15, 2026 23:57

ihhethan added 5 commits August 16, 2026 09:31

ihhethan force-pushed the users/ihhuang/rocr-dxg-topology-lifecycle branch from a993bb5 to ed34ecd Compare August 17, 2026 02:38

cfreeamd reviewed Aug 17, 2026

View reviewed changes

Comment thread projects/rocr-runtime/libhsakmt/src/dxg/openclose.cpp Resolved

ihhethan requested a review from bgopesh August 17, 2026 14:22

cfreeamd reviewed Aug 25, 2026

View reviewed changes

refactor(rocr): narrow DXG topology support to shared ownership

2584e2c

This was referenced Aug 25, 2026

fix(libhsakmt): correct independent DXG state and metadata bugs #10738

Open

fix(rocr): unwind failed HSA initialization safely #10739

Open

fcui-amd reviewed Aug 26, 2026

View reviewed changes

Comment thread projects/rocr-runtime/libhsakmt/CMakeLists_wsl.txt Resolved

fcui-amd reviewed Aug 26, 2026

View reviewed changes

Comment thread projects/rocr-runtime/libhsakmt/tests/dxg/CMakeLists.txt Resolved

ihhethan changed the title ~~[rocr-runtime] Add WSL DXG topology entry points and fix thunk loader lifecycle~~ [rocr-runtime] Make the DXG topology snapshot safe for multiple consumers Aug 26, 2026

cfreeamd reviewed Aug 27, 2026

View reviewed changes

cfreeamd approved these changes Aug 28, 2026

View reviewed changes

bgopesh approved these changes Aug 31, 2026

View reviewed changes

gandryey requested changes Aug 31, 2026

View reviewed changes

Comment thread projects/rocr-runtime/libhsakmt/src/dxg/topology.hpp Resolved

ihhethan force-pushed the users/ihhuang/rocr-dxg-topology-lifecycle branch from ce517b6 to 186e0b7 Compare August 31, 2026 16:10

cfreeamd reviewed Aug 31, 2026

View reviewed changes

ihhethan requested a review from gandryey August 31, 2026 16:44

gandryey approved these changes Sep 2, 2026

View reviewed changes

ihhethan requested a review from jbonnell-amd September 2, 2026 17:14

jbonnell-amd approved these changes Sep 2, 2026

View reviewed changes

amd-justchen merged commit 80763b5 into develop Sep 2, 2026
54 of 71 checks passed

amd-justchen deleted the users/ihhuang/rocr-dxg-topology-lifecycle branch September 2, 2026 23:32

Conversation

ihhethan commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Technical Details

Topology snapshot reference counting

hsaKmtOpenKFD() open count and allocator lifetime

Fork handling

Debug support query

ROCr: accepting an already-open DXG thunk

Relationship to #7016

JIRA ID

Test Plan

Test Result

Submission Checklist

Uh oh!

therock-pr-bot Bot commented Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ All Policy Checks Passed

Uh oh!

Uh oh!

Uh oh!

cfreeamd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cfreeamd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ihhethan commented Aug 25, 2026

Uh oh!

cfreeamd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ihhethan commented Aug 26, 2026

Uh oh!

cfreeamd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cfreeamd left a comment

Choose a reason for hiding this comment

Uh oh!

jbonnell-amd left a comment

Choose a reason for hiding this comment

Uh oh!

amd-justchen commented Sep 2, 2026

Uh oh!

Uh oh!

systems-assistant Bot commented Sep 3, 2026

TheRock Submodule Bump Activity

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

ihhethan commented Aug 12, 2026 •

edited

Loading

`hsaKmtOpenKFD()` open count and allocator lifetime

therock-pr-bot Bot commented Aug 12, 2026 •

edited

Loading