Repository navigation
mm: let XENMEM_domain_copy_range report a result per frame - #53
Merged
Merged
Conversation
A whole-guest read has to find out which frames a physmap actually holds, and that is the one thing this op could not tell it. It refuses a frame that is not ordinary RAM and fails the whole call, which is right for a write -- a caller that asked for a frame to hold something must not be told it does when it does not -- but a reader is asking a different question. A physmap's holes are what it is trying to discover: which frames are absent is half of what a snapshot records, and an absent frame is not a failure, it is the answer. Failing the call at the first one leaves the caller bisecting a batch to find out where the hole was, one hypercall per hole. XENMEM_DOMAIN_COPY_report_errs writes each frame's result to a new `errs` array and carries on past the frames it could not copy, exactly as XENMEM_add_to_physmap_batch does. Nothing is silently skipped -- the caller is told which frames it did not get and why, per frame instead of one at a time -- and a hole costs the array slot it was always going to occupy rather than a round trip. It is opt-in, and a null `errs` with the flag set is rejected rather than treated as "report nowhere", because a caller that asked to be told and was not would read a skipped frame as a copied one. Reading also already accepts every RAM type, p2m_ram_shared included: the reference a read takes is the one a read-only mapping would take, against dom_cow rather than the domain, so reading a copy-on-write frame leaves the sharing intact. That is what lets a whole-guest read walk a VM fork at all -- p2m_add_foreign() refuses a shared frame outright, so the mapping route cannot read one without unsharing it, which would hand the fork a private copy of its parent's entire memory to save a snapshot of it. Said out loud in the header next to the write rule, because the asymmetry is deliberate. A fault on the caller's own buffer stays fatal and is not reported per frame: it says the call was made wrong rather than anything about the frame. Builds for x86_64 with -Werror, and the public header still compiles in a translation unit defining neither __XEN__ nor __XEN_TOOLS__. The struct grows to 40 bytes with `errs` at offset 32, pinned by static assertion against the caller-side binding. Signed-off-by: Alex Zenla <alex@edera.dev>
A whole-guest read spends most of its walk on holes. A 512 MiB PVH guest's physmap reaches X86_HVM_END_SPECIAL_REGION, so finding the 131,094 frames it holds means walking 1,044,480 -- and for 87% of them the only work that matters is recording that there was nothing there. That recording was a copy_to_guest_offset() of four bytes per frame, and on a PVH dom0 a four-byte copy_to_guest() is a full walk of the caller's page tables to translate one linear address. It was the greater part of what finding a hole cost: measured against a capture of such a guest, the walk burned 1.2s of hypervisor time across eight parallel readers to move 145 MiB of memory. So buffer the results and write them back COPY_RANGE_ERRS_BATCH at a time. The buffer is flushed when it fills, before the call hands itself to a continuation -- a resumed call starts a fresh buffer at the frame it stopped at, so anything still held would have nothing to write it -- and once the walk is done. 128 entries keeps it to half a kilobyte of stack while turning a million writes into eight thousand. An error exit leaves whatever the buffer still held unwritten, which is deliberate rather than an oversight: a failed call reports nothing per frame, and a caller reading `errs` after one would be reading a walk that did not finish. The one in-tree caller propagates the error without looking at the array. No functional change: the array a completed call leaves behind is byte for byte the one it left before. Builds for x86_64 with -Werror. Signed-off-by: Alex Zenla <alex@edera.dev>
azenla
requested review from
alexandermerritt,
bleggett,
kaniini and
tycho
as code owners
October 17, 2026 22:30
kaniini
approved these changes
Oct 17, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
XENMEM_domain_copy_rangefails the whole call at the first frame it cannot copy. That is right for a write, but a whole-guest read is trying to find out which frames of a physmap exist, so it hits a hole almost at once and has no way to carry on.This adds
XENMEM_DOMAIN_COPY_report_errs. With it set the op writes each frame's result to a newerrsarray and continues past the frames it could not copy, the wayXENMEM_add_to_physmap_batchdoes. The results are written in blocks rather than onecopy_to_guestper frame. It is opt-in, and a nullerrswith the flag set is rejected. A hypervisor without the flag rejects the request, which is how protect'sXenCall::supports_domain_copy_errsdetects it.Without it, protect captures a zone for
protect zone exportthrough foreign mappings of the whole physmap. On a PVH dom0 that costs a map and an unmap per frame: a 512 MiB PVH zone, whose physmap spans 1,044,480 frames, takes about 3.5 s to export, with about 2.1 s of that spent reading through the mappings (captured domain ... through foreign mappings in 4.24s ... read 2.14s). The mapping route also cannot read a memory-shared domain, so it refuses to export either side of a fork.Booted on a PVH dom0 with these two commits (together with the fork-completion change in #52):
protect zone exportof the same 512 MiB PVH zone takes 154 to 168 ms, down from 3.5 to 3.75 s. The daemon's capture line changes fromcaptured domain ... through foreign mappings in 4.24s ... read 2.14stocaptured domain ... by copy hypercall in 146 ms ... read 307 ms across 8 readers. The exported file is the same size (137.6 MB).The build passes with
-Werror. These are the two commits from the viommu branch, taken ontoedera/4.22unchanged.Checklist