guestmem, scsi_buffers: clarify PagedRange and RequestBuffers documentation#2845
guestmem, scsi_buffers: clarify PagedRange and RequestBuffers documentation#2845jstarks merged 3 commits intomicrosoft:mainfrom
Conversation
…tation Improve the documentation on PagedRange and RequestBuffers to make their memory model constraints explicit: - PagedRange: add module-level and struct-level docs with ASCII diagrams explaining that interior pages are always fully covered. Only the first and last pages may be partial. This means multiple guest memory regions with arbitrary GPAs cannot always be combined into a single PagedRange. - RequestBuffers: explain the single-range limitation and its implications for scatter-gather IO (e.g., virtio descriptor chains). Document what is_aligned() actually checks and when bounce buffering is needed. - BounceBuffer: explain the read vs write copy direction and page-alignment guarantee. These constraints are particularly relevant for virtio device implementations, where the guest controls descriptor shapes and a descriptor chain may split at arbitrary (non-sector-aligned) byte boundaries.
|
This PR modifies files containing For more on why we check whole files, instead of just diffs, check out the Rustonomicon |
There was a problem hiding this comment.
Pull request overview
This PR improves developer-facing documentation for guest memory buffer abstractions used across the storage/virtio stacks, clarifying the memory model constraints of PagedRange and the implications for RequestBuffers/bounce buffering.
Changes:
- Add module- and type-level docs to
guestmem::rangesdescribing thePagedRangememory model and its “interior pages must be fully covered” constraint. - Expand
scsi_bufferscrate docs to explainRequestBufferssingle-range limitations, alignment checking semantics, and when bounce buffering is required. - Clarify
BounceBufferbehavior and intended copy direction for read vs write paths.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| vm/vmcore/guestmem/src/ranges.rs | Adds detailed documentation and diagrams describing PagedRange’s memory model and constraints. |
| vm/devices/storage/scsi_buffers/src/lib.rs | Expands crate/type/method docs around request buffers, alignment requirements, and bounce buffering. |
chris-oo
left a comment
chris-oo
left a comment
There was a problem hiding this comment.
LGTM but fix the typo flagged?
mattkur
left a comment
mattkur
left a comment
There was a problem hiding this comment.
Agree with the copilot comments. (I also wonder what led you to feel the need to be quite so ... forceful ... in documenting the internal contiguity guarantee :), but I agree this helps clarify the semantics here quite a lot).
|
Claude kept getting confused about the contract. If Claude is confused, probably everyone else is, too. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Improve the documentation on PagedRange and RequestBuffers to make their memory model constraints explicit:
PagedRange: add module-level and struct-level docs with ASCII diagrams explaining that interior pages are always fully covered. Only the first and last pages may be partial. This means multiple guest memory regions with arbitrary GPAs cannot always be combined into a single PagedRange.
RequestBuffers: explain the single-range limitation and its implications for scatter-gather IO (e.g., virtio descriptor chains). Document what is_aligned() actually checks and when bounce buffering is needed.
BounceBuffer: explain the read vs write copy direction and page-alignment guarantee.
These constraints are particularly relevant for virtio device implementations, where the guest controls descriptor shapes and a descriptor chain may split at arbitrary (non-sector-aligned) byte boundaries.