vmm: probe soft-dirty support with real pagemap round-trip

[aichitudou123]

The original probe_soft_dirty_support() simply wrote "4" to /proc/self/clear_refs to determine whether the kernel supports the soft-dirty mechanism. However, on kernels without CONFIG_MEM_SOFT_DIRTY (ARM64, x86 with the option disabled), clear_refs_write() silently no-ops the write and returns success, causing the probe to incorrectly return true. This led three soft-dirty-dependent unit tests to enter the test body on unsupported kernels and panic because bit 55 is always 0.

Replace the probe with a real pagemap round-trip:

1. mmap one page of anonymous memory, then write one byte to trigger VM_SOFTDIRTY to set bit 55 in the PTE.
2. Read /proc/self/pagemap to verify bit 55 is actually set.
   • If bit 55 is not 1, treat the kernel as unsupported and return false directly (skip clear_soft_dirty to avoid unnecessary mmap_lock write-lock PTE traversal on incapable machines).
   • If bit 55 is 1, call clear_soft_dirty once to arm the tracker and return true.

[cubesandboxbot]

Swap edge case: The probe relies on get_soft_dirty_pages which treats swapped pages (bit 62) as dirty regardless of bit 55. If the freshly-written scratch page is swapped out between the write and the pagemap read (extremely unlikely — ~ms window, startup, single page), the probe would return a false positive.

Consider guarding the probe result against this: if get_soft_dirty_pages returns true only because swapped=true (bit 62) on a page you know you just wrote, the result is ambiguous. Adding an inline note or a dedicated is_soft_dirty_supported(host_addr) helper that checks present && (entry & SOFT_DIRTY_BIT) != 0 would eliminate this theoretical false positive entirely.

[cubesandboxbot]

Review: vmm — probe soft-dirty support with real pagemap round-trip

AI-generated review. No human approval has been obtained.

---

Overview

This PR replaces the unreliable /proc/self/clear_refs write-based probe for CONFIG_MEM_SOFT_DIRTY support with a real pagemap round-trip: allocate a scratch anonymous page, write one byte, then read /proc/self/pagemap bit 55 to confirm the kernel actually tracks the bit. Only on success does the probe call clear_soft_dirty() to arm the tracker.

Files changed: 1 file (hypervisor/vmm/src/soft_dirty.rs)
Additions: 92, Deletions: 14

---

What the old code got wrong

The old probe_soft_dirty_support() wrote "4" to /proc/self/clear_refs and assumed Ok(()) meant soft-dirty was supported. On kernels built without CONFIG_MEM_SOFT_DIRTY (ARM64, x86 with the option disabled), the kernel's clear_refs_write() silently no-ops the write and returns success, so the probe incorrectly returns true. Three soft-dirty-dependent tests then panic because bit 55 is always 0.

This is a real, well-diagnosed bug. The fix is correct and necessary.

---

Implementation assessment

Approach — sound

The round-trip is the right way to probe:

1. mmap anonymous page → VMA gets VM_SOFTDIRTY (if config enabled)
2. First write → minor fault installs PTE → VM_SOFTDIRTY propagates into PTE bit 55
3. Read pagemap → bit 55 = 1 means support, 0 means no support

On kernels without CONFIG_MEM_SOFT_DIRTY, bit 55 is never set by the page table walker and stays 0, so the probe correctly returns false.

Error handling — adequate

All fallible operations are handled gracefully with debug!() logging and return false:

Failure mode	Behavior
mmap fails (OOM, rlimit)	false, logged
get_host_address fails	false, logged
Write to scratch page fails	false, logged
get_soft_dirty_pages returns Err	false (via unwrap_or(false))
clear_soft_dirty() fails after successful probe	false, logged

All paths are covered; no panic is introduced.

Safety

The function remains entirely safe. The GuestMemoryMmap::<()>::from_ranges pattern is already used in this file's own tests (lines 564, 707 in the base), so it is a pre-existing and established usage.

Dependency on Bytes trait

The import of Bytes (use vm_memory::{Bytes, ...}) is required for scratch.write(&[0xabu8], GuestAddress(0)). Without it, the Bytes::write method would not be in scope. This is correct.

Arm side-effect

The old probe always called clear_soft_dirty(), which both tested support and armed the tracker. The new probe only arms on confirmed support. This is actually better — on unsupported kernels no unnecessary PTE walk occurs — and the caller is documented to handle the false fallback path.

---

Issues found

1. Minor: Stale doc comment on clear_soft_dirty

The probe's own doc comment is thorough and accurate. However, the doc comment on get_soft_dirty_pages (line 186) says:

A page counts as soft-dirty if either:

• It is present and bit 55 is set, or
• It is in swap (bit 62)

The probe calls get_soft_dirty_pages to check if the scratch page is dirty. On a freshly-written scratch page that somehow gets swapped out before the pagemap read (theoretically possible under extreme memory pressure), the probe would see swapped=true and treat it as dirty — a false positive. In practice this is vanishingly unlikely (millisecond window, single page, startup time), but a dedicated swap-page path in the probe that ignores swapped pages would be more precise than relying on the coincidence.

Suggested fix: Make the probe ignore swapped pages when using get_soft_dirty_pages for detection, or add an inline comment acknowledging this edge case.

2. Nit: Trailing blank line at end of file

The diff adds one trailing blank line after the closing } of mod tests (@@ -769,3 +846,4 @@). This is cosmetic — rustfmt and most editors strip trailing whitespace, so it will likely be removed on the next format pass.

3. Nit: Inconsistent return style

The function uses explicit return false; for most early-exit paths but relies on the expression true at the end. Not a bug, but slightly inconsistent with the surrounding code style (the old implementation used match { Ok(()) => true, ... } which was uniform). Consider using true without return for the tail position (already done) but keeping all error paths as return false (already done too — they all use return false). The only inconsistency is that clear_soft_dirty().map(|_| true).unwrap_or(false) would have been more consistent than the if let Err(e) = ... { debug!(); return false; }; true pattern. Minor.

---

Conclusion

The change is correct, well-motivated, and the implementation is sound. The core fix (detecting soft-dirty support via an actual pagemap read instead of trusting clear_refs success) resolves a real bug on ARM64 and stripped-down x86 kernels. All error paths are handled gracefully. The existing test patterns (GuestMemoryMmap::<()>::from_ranges, CLEAR_REFS_LOCK) are reused appropriately.

The issues found are minor (theoretical swap edge case and cosmetic nits) and do not block the change. I would recommend accepting this PR after optionally addressing issue #1 with a comment or guard.

[lisongqian]

Please fix the error in the code format check