amd-justchen · Sep 2, 2026 · Aug 15, 2026 · Aug 15, 2026 · Aug 15, 2026 · Aug 15, 2026
@@ -121,10 +121,19 @@ jobs:
             exit 1
           fi
 
-          # Build the library
+          # Build the library, and with it the DXG unit tests: they are off by
+          # default so that a packaging build is unaffected, which leaves this
+          # job as the only place they ever get built and run.
           mkdir -p build
           cd build
-          cmake .. -G Ninja -DWIN_SDK="${win_sdk}/shared"
+          cmake .. -G Ninja -DWIN_SDK="${win_sdk}/shared" -DBUILD_ROCDXG_TESTS=ON
           ninja
 
           echo "libhsakmt build completed successfully!"
+
+      - name: Run DXG unit tests
+        shell: wsl-bash {0}
+        run: |
+          set -euo pipefail
+          cd ~/rocm-systems/projects/rocr-runtime/libhsakmt/build
+          ctest --output-on-failure
@@ -200,6 +200,16 @@ install ( TARGETS ${ROCDXG_TARGET} EXPORT ${ROCDXG_TARGET}Targets
 # separate header installation is needed here (and duplicating it breaks
 # TheRock artifact uniqueness checks — see ROCm/TheRock#7005).
 
+# Pure-logic unit tests. One compiles a header out of src/dxg on its own and
+# links nothing; the other reads the built library's dynamic symbol table.
+# Neither needs a GPU or a DXCore adapter, so `ctest` here works on a machine
+# with neither. Off by default so a packaging build is unaffected.
+option ( BUILD_ROCDXG_TESTS "Build the librocdxg unit tests" OFF )
+if ( BUILD_ROCDXG_TESTS )
+  enable_testing()
+  add_subdirectory ( tests/dxg )
+endif()
+
 # Record our usage data for clients find_package calls.
 install ( EXPORT ${ROCDXG_TARGET}Targets
   FILE ${ROCDXG_TARGET}Targets.cmake

@@ -32,8 +32,14 @@
 extern "C" {
 #endif
 
-/* Forward declaration for debug trap ioctl arguments */
+/* Forward declarations for the KFD ioctl arguments the entry points below take
+ * by pointer. linux/kfd_ioctl.h defines them and the dev package excludes it,
+ * so without these a consumer's C compiler invents a type scoped to the
+ * prototype and the pointer it passes is a different one.
+ */
 struct kfd_ioctl_dbg_trap_args;
+struct kfd_runtime_info;
+struct kfd_dbg_device_info_entry;
 
 /**
   "Opens" the HSA kernel driver for user-kernel mode communication.

@@ -61,10 +61,19 @@ HSAKMT_STATUS HSAKMTAPI hsaKmtDbgAddressWatch(
 HSAKMT_STATUS HSAKMTAPI hsaKmtCheckRuntimeDebugSupport(void) {
   CHECK_DXG_OPEN();
 
-  // Make sure the WDDMDevices are created.
-  HsaSystemProperties props;
-  hsaKmtAcquireSystemProperties(&props);
-  [](auto&&...) {}(props);
+  // The caller owns the topology snapshot the WDDMDevices belong to; this probe
+  // deliberately takes no reference of its own. Acquiring here would enumerate
+  // the adapters only to destroy them again on return, leaving the caller to
+  // build the very same list a second time.
+  //
+  // An empty device list therefore means no snapshot is held (or the machine
+  // has no adapter). Say so, rather than letting the loop below fall through
+  // and report support for devices nobody has looked at.
+  if (dxg_topology->wdevices_.empty()) {
+    pr_warn_once(
+        "DXG system properties must be acquired before checking runtime debug support\n");
+    return HSAKMT_STATUS_NOT_SUPPORTED;
+  }
 
   for (auto&& device : dxg_topology->wdevices_) {
     if (Wkmi::KmdDbgVersion version;
@@ -77,6 +86,12 @@ HSAKMT_STATUS HSAKMTAPI hsaKmtCheckRuntimeDebugSupport(void) {
 }
 
 HSAKMT_STATUS HSAKMTAPI hsaKmtRuntimeEnable(void *rDebug, bool setupTtmp) {
+  /* No acquire here either: the caller holds the snapshot across this call.
+   * ROCr's KfdDriver takes it in Init() before enabling, precisely so the
+   * device list this walks is the one BuildTopology() goes on to use. The
+   * probe reports NOT_SUPPORTED when that list is empty, so a caller that has
+   * not acquired never reaches the loop below.
+   */
   HSAKMT_STATUS result = hsaKmtCheckRuntimeDebugSupport();
 
   if (result)

@@ -255,6 +255,10 @@ uint32_t get_direct_link_cpu(uint32_t gpu_node);
 HSAKMT_STATUS topology_sysfs_get_system_props(HsaSystemProperties& props);
 HSAKMT_STATUS topology_get_node_props(HSAuint32 NodeId,
 				      HsaNodeProperties *NodeProperties);
+void topology_clear_snapshot_refs(void);
+void topology_abandon_after_fork(void);
+bool topology_snapshot_is_live(void);
+void topology_drop_snapshot_at_last_close(void);
 HSAKMT_STATUS topology_get_iolink_props(HSAuint32 NodeId,
 					HSAuint32 NumIoLinks,
 					HsaIoLinkProperties *IoLinkProperties);

@@ -501,6 +501,17 @@ static void parent_fork_handler(void) { dxg_runtime->hsakmt_mutex.unlock(); }
 static void child_fork_handler(void) {
   dxg_runtime->is_forked = true;
 
+  /* Sever the references the child inherited but never took, before any public
+   * entry point can act on them. CHECK_DXG_OPEN() already rejects calls while
+   * is_forked is set, but zeroing the counts means that even a path that
+   * bypasses it cannot decrement the parent's bookkeeping. The inherited open
+   * count is cleared with a plain store and the snapshot count with a relaxed
+   * store on a lock-free atomic, so neither allocates nor takes a lock; heavier
+   * snapshot and object teardown waits for clear_after_fork().
+   */
+  dxg_runtime->dxg_open_count = 0;
+  topology_clear_snapshot_refs();
+
   /* prepare_fork_handler() locked hsakmt_mutex right before fork() so that
    * no other thread would be mid-operation during the fork snapshot. In the
    * child only this one thread survives, and its TID differs from whichever
@@ -524,6 +535,11 @@ static void child_fork_handler(void) {
 static void clear_after_fork(void) {
   reset_suballocator();
   clear_allocation_map();
+  /* The snapshot's WDDMDevices belong to the parent's DXCore session, which
+   * the runtime reset below discards. Let go of them rather than reusing or
+   * destroying them.
+   */
+  topology_abandon_after_fork();
 
   if (dxg_runtime->dxg_fd >= 0) {
 #if defined(__linux__)
@@ -638,15 +654,22 @@ HSAKMT_STATUS HSAKMTAPI hsaKmtOpenKFD(void) {
   HsaSystemProperties sys_props;
   char *error;
 
-  std::lock_guard<std::recursive_mutex> lck(dxg_runtime->hsakmt_mutex);
-
   /* If the process has forked, the child process must re-initialize
    * it's connection to DXG. Any references tracked by dxg_open_count
-   * belong to the parent
+   * belong to the parent.
+   *
+   * This runs before the lock is taken, and must: clear_after_fork() replaces
+   * *dxg_runtime, so a lock_guard constructed on the old object would unlock
+   * freed memory when it went out of scope. Only the forking thread survives
+   * into the child and child_fork_handler() has already reinitialized the
+   * mutex, so there is nothing here to exclude. is_forked is only ever written
+   * in a single-threaded child, so reading it unlocked races with nothing.
    */
   if (is_forked_child())
     clear_after_fork();
 
+  std::lock_guard<std::recursive_mutex> lck(dxg_runtime->hsakmt_mutex);
+
   if (dxg_runtime->dxg_open_count == 0) {
     static bool atfork_installed = false;
 
@@ -680,7 +703,16 @@ HSAKMT_STATUS HSAKMTAPI hsaKmtOpenKFD(void) {
 
 
 
-    dxg_runtime->dxg_open_count = 1;
+    dxg_runtime->dxg_open_count++;
+
+    /* Only the 0->1 transition owns the suballocator. A later open comes from
+     * a second in-process consumer (on WSL, rocprofiler-sdk reading the KMT
+     * topology alongside the HSA runtime) and the first one's allocations are
+     * still live - resetting the fragment allocator here would throw away the
+     * bookkeeping that tracks them. clear_after_fork() above resets it on the
+     * one path where the inherited bookkeeping really is meaningless.
+     */
+    reset_suballocator();
 
     if (!atfork_installed) {
       /* Atfork handlers cannot be uninstalled and
@@ -699,7 +731,6 @@ HSAKMT_STATUS HSAKMTAPI hsaKmtOpenKFD(void) {
     result = HSAKMT_STATUS_KERNEL_ALREADY_OPENED;
   }
 
-  reset_suballocator();
   return result;
 dxcore_loader_failed:
 #if defined(__linux__)
@@ -716,6 +747,11 @@ HSAKMT_STATUS HSAKMTAPI hsaKmtCloseKFD(void) {
 
   if (dxg_runtime->dxg_open_count > 0) {
     if (--dxg_runtime->dxg_open_count == 0) {
+      /* Before DXCore goes, and so before the WDDMDevice objects a snapshot
+       * names become pointers into a dead session.
+       */
+      topology_drop_snapshot_at_last_close();
+
       dxg_runtime->HeapFini();
 #if defined(__linux__)
       close(dxg_runtime->dxg_fd);