[RattataKing]

Motivation

Rocjitsu may reserve the virtual-address range for a simulated VRAM allocation using a PROT_NONE CPU mapping.
In local mode, if a VMID page-table lookup misses, rocjitsu may incorrectly cast the GPU virtual address to a CPU pointer, and causing a seg fault.

Technical Details

• Create permanent host backing for each VRAM allocation.
• Resolve GPU VA to their owned backing in the local model.
• Prevent inaccessible VRAM allocations from falling back to CPU pointers.
• Preserve backing data across CPU and GPU map/unmap operations.
• Share the permanent backing during IPC export and import.
• Reject spanning munmap() ranges that overlap event or doorbell pages.

Issue Tracking

GitHub issue: #9933

Test Plan

Run CI and RocBLAS SGEMM test from the test corpus.

Test Result

Passed

Submission Checklist

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

[therock-pr-bot]

✅ All Checks Passed — Ready for Review

Check	Status	Details
📝 PR Description	✅ Pass	—
⛔ Forbidden Files	✅ Pass	—
🧪 Unit Test	✅ Pass	—
🔎 pre-commit	✅ Pass	—
🚫 Draft PR	🔜 To Be Enabled	—
🚩 Feature Flag	🔜 To Be Enabled	—
📊 Code Coverage	🔜 To Be Enabled	—
🤖 therock-pr-bot	✅ Pass	—

🎉 All checks passed! This PR is ready for review.

📖 Need help? See the Policy FAQ for details on every check and how to fix failures.

🙋 Wish to Override Policy?

[therock-pr-bot]

🎉 All checks passed! This PR is ready for review.

[atgutier]

Here are some agent reviews that I think are relevant:

MAJOR

[MAJOR, merge-blocking] lib/rocjitsu/src/rocjitsu/vm/amdgpu/gpu_memory.h:320 — Local GPU-only memory
now performs a failing syscall for every access

Reasoning: Process-scoped page-table misses no longer use local identity passthrough. Combined with
simulated_kfd.cpp:374, which registers the local process PID, every such load or store first calls
process_vm_readv() or process_vm_writev().

For the PROT_NONE GPUVA reservations this PR intends to support, those calls predictably fail with
EFAULT. The implementation then emits an unconditional, mutex-protected warning at
gpu_memory.h:351/366 before finally reaching sparse memory. Hot kernels therefore incur a failed
kernel syscall and warning construction for every unresolved memory transaction.

Evidence:

• gpu_memory.h:320-334 removes process-VMID raw passthrough.

• gpu_memory.h:343-370 probes every miss with process_vm_*.

• simulated_kfd.cpp:374 activates those probes for local mode.

• The baseline that produced merge base bec9696 passed gfx942 hip_matmul_matvec.m256_n1_k1024
  in 1.55 seconds.

• At PR head 6abb4a8, that exact workload timed out at 15 seconds, then timed out again during
  an isolated 30-second retry. The head’s test (release) check consequently failed.

• The new test at tests/simulated_kfd_test.cpp:97 only reads and writes an accessible stack value.
  Both syscalls succeed, so it never exercises the intended PROT_NONE → sparse memory path or its
  hot-loop cost.

Suggested fix: represent GPU-only/sparse reservation ranges explicitly and send those addresses
directly to sparse memory. Host/userptr mappings should continue to install normal page-table
entries. If generic client probing remains necessary, cache the page/range classification and
invalidate it during map, unmap, and process teardown. Expected EFAULT probes must not generate an
unconditional warning per access.

Add a regression covering a real PROT_NONE reservation with repeated accesses, then rerun both the
target gfx1201 rocBLAS case and the failing gfx942 matvec case.

Confidence: high.

MAJOR (confirm before merge) 10:16:18 [12/1899]

Pointer-returning callers lost the local-mode identity fallback with no client-memory replacement —
gpu_memory.h:86 (resolve_host_ptr) → command_processor.cpp:1793 (resolve_sdma_ptr),
completion_tracker.cpp (fire_queue_idle_signal), find_host_range

The deleted trailing branch was the only thing making an unmapped-but-host-valid GPUVA
dereferenceable via the pointer APIs in local mode. The value accessors have a client-memory
fallback; the pointer path does not — it now returns nullptr where it used to return an identity
pointer. I confirmed resolve_sdma_ptr still assumes the removed mapping — its own doc says "In local
mode, the GPU VA IS the host VA (identity mapping), so we cast directly" — and on nullptr it
returns nullptr, which every SDMA consumer treats as "not ready" and re-arms via
stop_and_retry_current_packet(). If any SDMA operand or host-allocated signal VA is
host-valid-but-not-KFD-mapped, the queue now stalls forever instead of completing;
fire_queue_idle_signal similarly stops firing (unlike fire_signal, it has no fallback).

This ranges from harmless to a hang depending on whether local-mode SDMA operands and signals are
always KFD-mapped (PT hit). hipMalloc'd copy buffers likely are; host-allocated signals and userptr
are the risk.
Fix: author to confirm those VAs are always mapped in local mode, or extend the client-memory
fallback to the pointer path (mirror the value accessors — copy the operand in/out via
process_vm_readv/writev when resolve_host_ptr is null and vmid>0), or PROT-probe the identity
pointer before returning it.

MINOR

New test doesn't lock in the fix — simulated_kfd_test.cpp:97-119
The test reads/writes a committed host stack variable through the process VMID. Under the OLD code
that address (well under 0x800000000000) hit the deleted identity passthrough and also returned the
value — so the test passes on both old and new code and catches no regression. It also never
exercises the actual crash scenario (a PROT_NONE reservation).
Fix: add a case that mmaps a PROT_NONE region and asserts read32/write32 through the process VMID
doesn't crash and returns benign backing (that's the regression the old code failed). Ideally on the
gfx1201 config from the issue, not gfx950.

NIT

• set_process_client_pid and resolve_host_ptr lack the Doxygen @brief/@details the surrounding
  methods have.

[therock-pr-bot]

Pre-commit check failed

⛔ pre-commit failed

Please run locally:

• python -m pip install pre-commit
• pre-commit install
• pre-commit run --all-files --show-diff-on-failure

This repo uses .pre-commit-config.yaml.

[kuhar]

A few comments

[atgutier]

Agent's found more blockers/majors:

BLOCKER

[BLOCKER]

lib/rocjitsu/src/rocjitsu/vm/amdgpu/gpu_memory.h:99-102,393-405 — daemon
return a foreign-process address as a local pointer

Reasoning: resolve_client_identity_page() uses process_vm_readv() to confirm that an address is
readable in the client process, then returns that same numeric address for direct dereference in the
daemon. Successful cross-process reading does not mean the daemon has the address mapped
identically. This can segfault or access an unrelated daemon mapping.

Evidence:

• Remote clients are registered through open_process(client_pid) at simulated_kfd.cpp:410-449.

• The remote probe occurs at gpu_memory.h:399-404, followed by the unsafe pointer construction at
  line 405.

• Consumers require a genuinely local pointer and immediately dereference it:

  • command_processor.cpp:1928-1936 uses atomic_ref.
  • command_processor.cpp:1987-1992 performs an atomic load.
  • command_processor.cpp:2024-2044 uses memcpy and atomics.
  • completion_tracker.cpp:88-132,170-238 reads and writes through returned pointers.

• The new test at simulated_kfd_test.cpp:100-121 covers only same-process local mode.

Suggested fix: Restrict identity-pointer fallback to a VMID explicitly known to share the
simulator’s address space, such as client_pid == getpid() plus local mode. For daemon clients,
return only page-table-backed daemon pointers; otherwise use process_vm_readv/writev copy operations
or return nullptr. Add a separate-process daemon regression test.

Confidence: high.

MAJOR

[MAJOR] lib/rocjitsu/src/rocjitsu/kmd/linux/kfd_process.h:251-275,337-364 — sparse reservations
expand into one hash entry per page

Reasoning: The first classification after any allocation change rebuilds an unordered_map entry for
every page of every allocation. That defeats sparse backing for the large PROT_NONE GPUVA
reservations this PR is intended to support.

Evidence:

• contains_sparse_reservation() initiates the rebuild at lines 259-262.

• rebuild_sparse_reservation_index_locked() walks every page and creates a hash node at lines 343-
  355.

• Allocation size is accepted directly at simulated_kfd.cpp:1293-1299.

• The simulator advertises 64 GiB at simulated_kfd.cpp:1215. A reservation of that size creates
  16,777,216 hash entries, potentially consuming hundreds of MiB and holding alloc_mutex_ for a
  prolonged rebuild.

• Every allocation mutation invalidates the index, forcing another full rebuild on the next query.

• The new regression test exercises only one 4 KiB page at simulated_kfd_test.cpp:139-165.

Suggested fix: Represent sparse and host-backed coverage as intervals/endpoints with coverage
counts, or scan the normally small allocation set on demand. Host-backed overlap precedence can be
preserved without per-page storage. Add a large logical-reservation test demonstrating that
auxiliary storage remains proportional to allocation ranges, not page count.

Confidence: high.

[atgutier]

Thanks for iterating on this. Had some gcc compilation errors and found a few other issues.

• ## BLOCKER

[BLOCKER] tests/simulated_kfd_test.cpp:86-101,176 — the new daemon regression test does not compile under GCC with -Werror

Reasoning: EXPECT_EQ expands to control flow, so using it as the unbraced body of if (done_fd >= 0) triggers -Wdangling-else. GCC’s fortified libc also rejects the ignored write() and read() return values
despite the (void) casts. The test target therefore cannot build.

Evidence:

• ChildProcessGuard::finish() fails at line 98 with -Werror=dangling-else.
• Lines 91 and 176 fail with -Werror=unused-result in GCC ASan/UBSan.
• test (asan-ubsan-gcc) failed during compilation.
• Both TheRock Linux package builds failed on the same source file; their “custom container implementation failed” annotation is secondary, not infrastructure noise.

Suggested fix: add braces around the conditional containing EXPECT_EQ, and store/check the read() and write() results, including EINTR handling where appropriate.

Confidence: high.

MAJOR

[MAJOR] lib/rocjitsu/src/rocjitsu/kmd/linux/simulated_kfd.cpp:435-449 — documented local open_process(0) misses the PID required by the new fallback

Reasoning: rj_vm_device_open(vm, 0, ...) is explicitly documented as the local-mode API and forwards zero to open_process(). That function only records and publishes a PID when client_pid > 0.

At this head, GpuMemory::translate() no longer identity-maps process VMIDs. Both the new identity-pointer path and the process_vm_* copy paths reject a zero PID. Consequently, a valid local host pointer not
present in that process’s page table resolves to nullptr, while scalar/block accesses incorrectly use sparse backing instead of host memory.

Evidence:

• rj_vm.h:241-249 documents zero for local mode.
• rj_vm.cpp:320-330 forwards zero directly.
• simulated_kfd.cpp:435-449 leaves both KfdProcess::client_pid_ and GpuMemory::VmidEntry::client_pid at zero.
• gpu_memory.h:349-363,406-453 removes the old process-VMID identity fallback and requires a positive PID.
• The added local test uses driver.open(), whose separate path correctly registers getpid(), so it misses the public rj_vm_device_open(..., 0, ...) path.

Suggested fix: normalize zero to getpid() when open_process() is used in non-daemon mode, and publish that effective PID to both objects. Alternatively, make local device-open adopt the bootstrap local
process with balanced references. Add an API-level test covering host read, write, and resolve_host_ptr() through the returned process ID.

Confidence: high.

[MAJOR] lib/rocjitsu/src/rocjitsu/vm/amdgpu/gpu_memory.h:139-169,246-329 — sparse backing is not isolated by VMID

Reasoning: the new classifier intentionally routes valid PROT_NONE GPU allocations directly to inherited SparseMemory, but those fallback calls pass only the GPU virtual address. The base storage also keys
pages solely by address. Independent VMIDs using the same GPUVA therefore read and write the same bytes.

Sparse pages are not retired when an allocation or process is removed, so the backing also persists across process/allocation lifetimes.

Evidence:

• gpu_memory.h:372-403 makes classified sparse reservations skip client-memory probing.
• The fallback calls at gpu_memory.h:139-169,246-329 discard vmid.
• lib/simdojo/include/simdojo/components/sparse_memory.h:208-234 indexes pages only by virtual page address.
• Non-daemon open_process(0) calls create distinct KfdProcess objects.
• Every KfdProcess starts default allocation at 0x1000000000.
• Free and close remove allocation/classifier state but never remove corresponding sparse pages.

The flat fallback predates this PR for failed client probes, but this change materially exposes it by making that storage the intended successful backing for registered local GPU-only allocations.

Suggested fix: give sparse allocations explicit process/VMID-scoped backing and reclaim it during allocation/process teardown, while keeping VMID-zero simulator memory separate. Add two simultaneous process
IDs with identical sparse GPUVAs and verify that their values remain isolated.

Confidence: high.

[kuhar]

Original anchor: emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/amdgpu/gpu_memory.h:156

We should update GpuMemoryTest.RegisteredVmidPassthroughMissRespectsUserSpaceLimit for the new registered-VMID contract before landing. At 8632816, resolve_host_ptr(0x4000, 7) returns nullptr at this path while the test still expects the identity address 0x4000; the focused CTest fails, and the same sole failure appeared in the default, GCC 13, Clang ASan/UBSan, and Clang TSan trees (18/19 cases passed in each). If registered process VMIDs are intentionally no longer raw passthrough, please update that expectation while retaining explicit VMID-0 and same-process client-PID coverage.

Original anchor: emulation/rocjitsu/lib/rocjitsu/src/rocjitsu/vm/amdgpu/gpu_memory.h:237

We should avoid calling the sparse classifier while atomic_rmw holds the page-table shared lock. The classifier enters KfdProcess::contains_sparse_reservation and takes alloc_mutex_, while map/free paths hold alloc_mutex_ before map_pages/unmap_pages take page_table_mutex_; that creates an AB-BA lock inversion. A deterministic reproducer at 8632816 hung under a 2-second timeout. Please classify before taking the page-table lock, as the scalar paths do, or use a lock-free snapshot, and add a concurrent sparse-atomic versus map/free regression.

[kuhar]

A few comments

[kuhar]

A few comments

[kuhar]

We should preserve GPU-authoritative backing on the remote mmap path. The local path stops importing client pages after MAP_MEMORY, but RemoteDriver::mmap still copies every resident MAP_FIXED source page into the memfd. At this head, a repro with 0x22222222 in the backing and stale 0x11111111 in the reservation maps as 0x11111111, overwriting the simulated GPU result. Please propagate the first-import/authority state to RemoteDriver, or make the daemon own the import before the backing becomes authoritative, and add the equivalent daemon lifecycle regression.

[kuhar]

We should publish the page-rounded backing range here. create_lazy_vram_backing() and CPU mmap cover backing_size, but this vector covers only alloc.size; for a non-page-multiple VRAM allocation, the tail is classified as unmanaged and local with_host_ptr() identity-dereferences the client reservation. At this head, a 4097-byte PROT_NONE allocation accepts an in-range write and then faults with SIGSEGV on a tail-page write; with a writable reservation, the tail write lands in client memory instead of the backing. Page-granular overlap checks already reserve the whole backing page, so using backing_size is safe. Please update the overlap assertion to the same units and add a non-page-multiple regression.

[kuhar]

This would be safer if close preserved caller-owned MAP_FIXED reservations. The permanent-backing mmap path records those mappings in cpu_mappings_, and close unmaps every interval there even though GpuAllocation::host_ptr_owned documents that the driver must not unmap thunk-owned pages. A current-head repro maps VRAM over a PROT_NONE reservation, closes KFD, and then mincore returns ENOMEM because the VA became a hole. Please track ownership for these intervals and reclaim only driver-created mappings; a close-then-mincore regression would lock in the contract.

[kuhar]

The overlap check should not reject byte-disjoint userptr registrations that share one host page. alloc_memory_ioctl() page-rounds every requested size before assign_gpu_va_locked(), so two 64-byte USERPTR buffers 128 bytes apart collide. A current-head repro returns 0 for the first registration and -EINVAL for the second. ROCr can issue separate small host registrations from the same page, so please scope the new page-exclusivity rule to allocation kinds that require exclusive GPU backing, while retaining overlap rejection for permanent-backing VRAM, and add coverage for both cases.

[kuhar]

We should reject invalid imported ranges before they enter allocations_ and route requested IPC VAs through the same assignment check. import_dmabuf_ioctl() accepts a zero-length memfd and records {gpu_va=0,size=0}; the next auto-VA allocation then underflows alloc.size - 1 in this scan and returns -EINVAL. The same direct-insertion pattern lets scratch and IPC imports bypass overlap validation, and IPC still advances next_gpu_va_ manually. Please centralize range validation for these insertion paths, reject size zero at import, and add a zero-size dmabuf followed by auto-VA regression plus an overlapping IPC-import case.

[kuhar]

Can we cache backing-range resolution with the same generation-keyed mechanism used for PTEs? Permanent-backing VRAM has no PTE, so every load and store currently pays a std::function call, weak_ptr::lock(), shared lock, and binary search. A current-head RelWithDebInfo microbenchmark measured repeated read32 at 54.4 ns through this resolver versus 32.4 ns through the cached page-table path, a 1.68x difference. Please cache the resolved range with explicit lifetime invalidation, or otherwise avoid this per-access synchronization on the simulator hot path.

[atgutier]

A few more comments:

[MAJOR] lib/rocjitsu/src/rocjitsu/kmd/linux/kfd_process.h:415 - Publish
the page-owned backing extent, not the requested byte size

Reasoning: The new permanent-backing design creates a page-rounded memfd
and stores that extent in alloc.backing_size, but
publish_backing_ranges_locked() publishes only alloc.size. For a PAGE_SIZE

• 1 VRAM allocation, most of the second page is consequently classified as
  unmanaged.

When an access reaches that portion of the page, resolve_backing() returns
nullopt at lines 321–323. GpuMemory::with_host_ptr() interprets nullopt as
an unmanaged address and may use identity-pointer or client-memory
fallback at gpu_memory.h:611–617. In local mode this can dereference a
synthetic GPU VA and crash. In daemon mode it can access unrelated client
or sparse memory. This violates the central invariant introduced by this
PR: an address owned by a managed allocation must never fall through as
unmanaged.

The daemon has the same extent mismatch. remote_driver.cpp:721–722
registers alloc_args->size, while find_memfd_for_addr() requires the
entire mapping to fit that range at lines 175–180. A page-rounded
anonymous MAP_FIXED can therefore miss the shared memfd and replace the
GPUVM range with anonymous memory.

Evidence:

• simulated_kfd.cpp:1570–1574 computes the page-aligned allocation size.

• simulated_kfd.cpp:1597–1604 creates the rounded backing and records
  backing_size.

• Allocation collision checks are already page-granular.

• kfd_process.h:415 nevertheless publishes alloc.size.

• No added test uses a non-page-aligned VRAM allocation.

Suggested fix:

• Retain separate logical and page-owned sizes.

• Publish backing_size through backing_ranges_ for permanent allocations.

• Register the memfd’s actual size in the daemon’s alloc_ranges_.

• If accesses beyond the logical byte size should fault or be dropped,
  represent that as a managed fault—not an unmanaged miss.

• Add local and daemon tests for sizes 1 and PAGE_SIZE + 1. Verify that
  accesses throughout the final page never select identity, client-memory,
  or sparse fallback.

[MINOR] lib/rocjitsu/src/rocjitsu/kmd/linux/events.cpp:65 — Compare
normalized munmap intervals for event and doorbell pages

Reasoning: dispatch_munmap() correctly rounds a positive length to
complete pages at simulated_kfd.cpp:1330–1337, but the special-page paths
compare the original byte length:

• Doorbells require gs.doorbell_page_size == length at
  simulated_kfd.cpp:1347.

• Event pages require size == page_size at events.cpp:65.

Therefore, munmap(page, 1) is rejected with EPERM, although Linux treats
it as unmapping exactly that one page. Ordinary permanent mappings already
normalize this case. The PR’s own permanent-mapping test expects
munmap(mapping, 1) to work, but the event and doorbell tests cover only
full-page release and spanning rejection.

Suggested fix:

• Compare the page-aligned start and rounded end against the recorded
  special-page interval.

• Pass the normalized length to event-page classification.

• Preserve rejection for requests whose normalized interval truly
  partially overlaps or spans beyond the protected page.

• Add munmap(page, 1) and munmap(page, PAGE_SIZE - 1) tests for both event
  and doorbell mappings.

Confidence: High.

[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.