hyperlight-dev / hyperlight-dev/hyperlight
vmem: make AnotherSpace a follow-through, not a terminator, in the walker
- Langage dominant
- Rust
- Étoiles
- 4.7k
- Forks
- 208
- Merge moyen
- 1 j 7 h
- PR mergées (30 j)
- 47
Description
Follow-up to review feedback on #1385. The i686 walker currently treats "we already saw this sub-tree via another root" as a terminator: it emits `AnotherSpace` and stops descending. That works for the snapshot-rebuild consumer (which wants the reference so it can install a link) but not for the `virt_to_phys` consumer, which should follow through to the aliased target and return the leaf mapping in the owning space. Today `virt_to_phys` on an aliased PT page gets back nothing, which is the wrong answer for anyone asking "what does this VA resolve to".
## Proposed changes
**Rename `AnotherSpace` → `Aliasing`, or tighten the docs to make the "this alias is a reference to where the real mapping lives" invariant explicit.** The current name invites reading it as a terminator. Reviewer comment: https://github.com/hyperlight-dev/hyperlight/pull/1385#discussion_r3132245307.
**Factor the walker so the aliasing behaviour lives in one place.** Right now `walk_va_spaces` and `virt_to_phys` each have their own descent logic; the natural shape is one "walk with alias-following" function whose callers (`walk_va_spaces` and `virt_to_phys`) plug in emit callbacks — `walk_va_spaces` emits references for aliases, `virt_to_phys` follows the alias and emits leaves from the owning space. Reviewer comment: https://github.com/hyperlight-dev/hyperlight/pull/1385#discussion_r3132251136.
**Propagate "no valid entry at the referenced spot" through to the target.** With follow-through semantics, if the alias target has no valid entry at the referenced depth, the correct answer for the current space is "no valid entry here either" — i.e. the appropriate level table entry should be made invalid, not silently left pointing at an empty target. Reviewer comment: https://github.com/hyperlight-dev/hyperlight/pull/1385#discussion_r3132258032.
## Notes
This is a semantic change to how aliases compose with reads, not just a rename. Once the walker has a single "walk with alias-following" function, `virt_to_phys` becomes a special case of it (the "emit leaf" callback flattens resolved leaves into plain `Mapping`s) and `walk_va_spaces` becomes another special case (the "emit reference" callback produces `SpaceAwareMapping::AnotherSpace` entries without descending). The invariants surfaced by #1409 (per-arch `SpaceReferenceMapping`, "address in region" fields) pair naturally with this change — if that one lands first, the renaming here can ride on the same invariant set.
## Acceptance
- `AnotherSpace` renamed (or re-documented) to make follow-through semantics obvious.
- One shared descent function; `walk_va_spaces` and `virt_to_phys` both expressed in terms of it.
- `virt_to_phys` DTRT on aliased PT pages (resolves to leaves in the owning space).
- "No valid entry at the alias target" resolves to "no valid entry here" at the reference site.
- Existing snapshot compaction path continues to work; the new `virt_to_phys` semantics do not silently change rebuild behaviour.
## Links
- PR #1385 threads: [r3132245307](https://github.com/hyperlight-dev/hyperlight/pull/1385#discussion_r3132245307), [r3132251136](https://github.com/hyperlight-dev/hyperlight/pull/1385#discussion_r3132251136), [r3132258032](https://github.com/hyperlight-dev/hyperlight/pull/1385#discussion_r3132258032).
- Related: #1409 (`SpaceReferenceMapping` per-arch + `space_aware_map` cleanup). Likely best landed after #1409 so the invariants for `SpaceReferenceMapping` are already settled.
Guide de contribution
Ouvrir le guide de contribution
Évaluation
Cette issue n'a pas encore été évaluée.