Skip to content

fix(vmm): map full reservation in one segment on Windows - #737

Merged
Kaden-Schutt merged 2 commits into
warpfront:betafrom
harrytyp:fix/vmm-windows-single-segment-map
Sep 10, 2026
Merged

fix(vmm): map full reservation in one segment on Windows#737
Kaden-Schutt merged 2 commits into
warpfront:betafrom
harrytyp:fix/vmm-windows-single-segment-map

Conversation

@harrytyp

@harrytyp harrytyp commented Sep 9, 2026

Copy link
Copy Markdown

Problem

hipMemCreate/hipMemMap on Windows/ROCm 7.2 (gfx1100) maps a second, later segment onto the same physical pages as the first. The repo's own vmm_arena_smoke example panics at its boundary-growth assert (line 68) after the 2nd map_next — segment 1 aliases segment 2's pages — while single-segment maps pass.

The KV cache grows in ~1900 small map_next steps, so on Windows every growth corrupts all prior KV. This surfaces as token soup ("!!!!!!!!") from every model (qwen3.8:27b, qwen3.5:0.8b, lfm2.5:1.2b) in hipfire serve. Full repro and discussion: #655, #644.

Fix

Under #[cfg(windows)], alloc_vmm_tensor maps the full reservation in one map_next instead of growing in small segments. Linux behavior is byte-identical to before (the cfg block does not exist there).

Trade-off: costs full VRAM up front (no on-demand commit) on Windows only. This is a workaround for what looks like a ROCm-Windows VMM runtime bug — both mappings report distinct virtual addresses via hipMemGetAddressRange, yet share physical pages.

Validation (gfx1100, RX 7900 XTX, Windows 11, ROCm 7.2)

HW-gate routes

  • scripts/serve_harness.py: generation/state-lifecycle changed? No (dispatch/VMM allocation only)
  • scripts/redline_daemon_harness.py: kernel/dispatch change? Yes — dispatch-layer VMM allocation path

hipMemCreate/hipMemMap on Windows/ROCm 7.2 (gfx1100) maps a second,
later segment onto the SAME physical pages as the first: the repo's
vmm_arena_smoke example panics at its boundary-growth assert after the
2nd map_next (segment 1 aliases segment 2's pages), while single-segment
maps are correct. The KV cache grows in ~1900 small map_next steps, so
every growth corrupts all prior KV on Windows -> token soup from every
model (qwen3.8:27b, qwen3.5:0.8b).

Map the full reservation in one map_next on Windows (cfg(windows)
override of initial_mapped_bytes). Costs full VRAM up front instead of
on-demand commit, but is correct; Linux behavior unchanged.

Signed-off-by: kolja <kolja@local>
@HUSRCF

HUSRCF commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Thanks for putting this together and, especially, for validating it on the affected Windows/gfx1100 machine. The single-mapping workaround is consistent with the vmm_arena_smoke evidence and looks like the right safety policy for the affected runtime.

One small implementation detail may be worth adjusting before merge. The current Windows override uses:

#[cfg(windows)]
let initial_mapped_bytes = byte_size;

VmmArena::reserve rounds the requested byte size up to the HIP VMM allocation granularity, while VmmArena::map_next requires its size to be an exact multiple of that granularity. Therefore, a caller whose logical byte_size is not already aligned could still fail on Windows even though the reservation itself is valid.

Would it be safer to map the actual rounded reservation instead?

#[cfg(windows)]
let initial_mapped_bytes = arena.reserved_bytes();

This preserves the intended policy—one physical mapping covering the entire Windows reservation—while also keeping alloc_vmm_tensor correct for shapes whose logical byte size is not VMM-granularity aligned. The existing Qwen KV layout may happen to be aligned, but this API is shared by other VMM tensor callers as well.

beta's map_next validates that the map size is a non-zero multiple of the
allocation granularity (64 KiB), while master rounded silently. Map the
full reservation rounded up to the granularity, keeping the single-segment
Windows path correct on both branches.

Signed-off-by: kolja <kolja@local>
@Kaden-Schutt

Copy link
Copy Markdown
Collaborator

Prepared integration/pr737-windows-fullmap at d827d03, based on your full PR history, with two fixups: use arena.reserved_bytes() directly, and migrate VMM smoke/tests to the Windows full-map contract (plus a 4097-byte roundtrip regression). Independent review caught the old smoke still expecting partial mapping/growth. Linux gfx1201 4097-byte GPU regression passes; this does NOT validate the Windows branch. Before beta landing, please run on Windows ROCm 7.2: cargo run -p rdna-compute --example vmm_tensor_smoke; cargo test -p rdna-compute --lib vmm_ -- --nocapture --test-threads=1; then your real serve/KV corruption reproduction and applicable capture/parity route. Please attach exact tested SHA and decoded output. Candidate: https://github.com/warpfront/hipfire/tree/integration/pr737-windows-fullmap . No Windows execution host is available in this session, so holding rather than claiming proof.

@Kaden-Schutt

Copy link
Copy Markdown
Collaborator

Landed corrected #737 into beta0db445ecb with original PR commits, reserved_bytes alignment fix, migrated full-map smoke/tests and generated crate-map refresh. Maintainer explicitly waived the remaining Windows-host validation requirement. Fresh merged-tree 4097-byte GPU roundtrip regression passed on Linux/gfx1201/HIP7.15; Windows execution is NOT claimed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants