[ihhethan]

Motivation

These are four independent DXG issues discovered while validating the WSL
topology path. None of them is required for topology access, so they have been
split out of
#10034 to keep that PR
focused on the multi-consumer topology prerequisite and make both changes
easier to review.

Two of the issues produce incorrect results rather than representing only
latent risks: NumCUPerArray is over-reported on devices with more than one
shader engine, and KFDGpuID collides across identical GPUs, causing reverse
lookups for both devices to resolve to the first node.

Technical Details

NumCUPerArray used the wrong shader-array count

src/dxg/topology.cpp

NumArrays is ShaderArrayPerShaderEngine(), so the total number of shader
arrays across the GPU is:

NumShaderBanks * NumArrays

The previous calculation divided ComputeUnitCount() by NumArrays alone,
which over-reported NumCUPerArray by the shader-engine count. It produced the
correct result only when NumShaderBanks == 1, which is why single-shader-engine
devices were unaffected.

The divisor now uses the total number of shader arrays. The zero-divisor guard
has been updated accordingly, and a warning is emitted if the CU count is not
evenly divisible by the array count because the integer division would
otherwise truncate the result.

KFDGpuID was not unique across identical GPUs

src/dxg/topology.cpp

KFDGpuID was populated with the PCI device ID. The field is used both as the
predicate indicating that a node is a GPU and as the key inverted by
gpuid_to_nodeid() and get_device_id_by_gpu_id().

A PCI device ID is neither unique across identical GPUs nor guaranteed to be
nonzero. As a result, identical GPUs could receive the same KFDGpuID, causing
both reverse lookups to resolve to the first matching node.

KFDGpuID is now assigned as node_id + 1, making it unique within the
topology snapshot and nonzero for every node.

Failed DXG open left a stale file descriptor

src/dxg/openclose.cpp

On the dxcore_loader_failed path, the local fd was closed while
dxg_runtime->dxg_fd still retained the closed descriptor value.

A subsequent hsaKmtOpenKFD() could therefore observe a non-negative
dxg_runtime->dxg_fd, skip reopening /dev/dxg, and return a stale file
descriptor.

The failure path now closes the stored descriptor and resets
dxg_runtime->dxg_fd to -1.

librocdxg.pc resolved the install prefix one directory too high

librocdxg.pc.in

The pkg-config file is installed under ${CMAKE_INSTALL_LIBDIR}/pkgconfig,
which is two directory levels below the install prefix. Using
${pcfiledir}/../../.. therefore resolved one level above the actual prefix and
made the derived libdir and includedir paths incorrect.

The prefix is now derived from ${pcfiledir}/../.., matching the existing
layout used by libhsakmt.pc.in.

Issue Tracking

JIRA ID: ROCM-23912

Test Plan

Test Result

Submission Checklist

• Look over the contributing guidelines at https://github.com/ROCm/rocm-systems/blob/develop/CONTRIBUTING.md.

[therock-pr-bot]

✅ 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/src/dxg/openclose.cpp, projects/rocr-runtime/libhsakmt/src/dxg/topology.cpp; 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?

[cfreeamd]

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

Reviewed all three fixes. The openclose.cpp unwind (guarding close() on dxg_fd >= 0 and resetting it to -1) and the topology.cpp NumCUPerArray correction (dividing by NumShaderBanks * NumArrays = total arrays, not arrays-per-engine) both look correct — the CU-per-array fix addresses a real over-count. Two clarifying questions inline on the .pc prefix depth and the synthesized KFDGpuID.

[cfreeamd]

🤖 AI-assisted pre-review (for @cfreeamd) — follow-up after the KFDGpuID docs commit (0596909)

The new contract comment at the KFDGpuID = node_id + 1 assignment resolves my earlier question: it documents that this is a nonzero snapshot-local key (not the PCI device ID, not a kernel gpu_id), that DXG consumers only need uniqueness + nonzeroness, and — helpfully — that it surfaces externally via amd_core_dump.cpp as the core dump's gpu_id, so a debugger correlating that field against kernel state won't match on WSL. Good disposition. The only remaining item is the non-blocking .pc prefix-depth question (single-segment vs multiarch CMAKE_INSTALL_LIBDIR); the three fixes look correct.

[github-actions]

This pull request has been inactive for 25 days and will be marked as stale.

If you would like to keep this PR open, please:

• Add new commits
• Add a comment explaining why it should remain open

This PR will be automatically closed in 5 days if no further activity occurs.

[github-actions]

This pull request has been automatically closed due to inactivity (30 days with no updates).

If you'd like to continue working on this, feel free to reopen the PR or create a new one.