guest_mem: Add support for guest memory access wrappers for PagedRanges #2444
Conversation
|
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 adds guest memory access validation wrappers for PagedRange objects and uses them in the IDE device to validate DMA operations. The implementation adds two new methods to GuestMemory (probe_gpn_readable_range and probe_gpn_writable_range) and integrates them into the IDE DMA path to check memory accessibility before transfers.
Key Changes:
- New
probe_gpn_readable_rangeandprobe_gpn_writable_rangemethods in GuestMemory API - DMA memory validation in IDE device before descriptor processing
- Test case for invalid DMA base address handling
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| vm/vmcore/guestmem/src/lib.rs | Adds two new probe methods for checking readable/writable access to PagedRange memory regions |
| vm/devices/storage/ide/src/lib.rs | Integrates new probe methods into DMA validation logic, adds extensive debug tracing, includes test for invalid DMA addresses, and has several implementation bugs |
| /// Check if a given PagedRange is readable or not. | ||
| pub fn probe_gpn_readable_range(&self, range: &PagedRange<'_>) -> Result<(), GuestMemoryError> { |
There was a problem hiding this comment.
[nitpick] The documentation could be more descriptive. Consider expanding it to clarify what "readable" means in this context and what errors might be returned. For example:
/// Checks if all pages in the given PagedRange are readable.
///
/// This probes each page in the range by attempting to read a single byte.
/// Returns an error if any page in the range is not accessible or is outside
/// the guest's physical address space.
///
/// # Errors
/// Returns `GuestMemoryError` if any page is inaccessible or invalid.
pub fn probe_gpn_readable_range(&self, range: &PagedRange<'_>) -> Result<(), GuestMemoryError>The same applies to probe_gpn_writable_range.
| }) | ||
| } | ||
|
|
||
| /// Check if a given PagedRange is writable or not. |
There was a problem hiding this comment.
[nitpick] Consider adding similar documentation to probe_gpn_writable_range as suggested for probe_gpn_readable_range. The documentation should clarify that this checks writability by attempting a compare-exchange operation.
| /// Check if a given PagedRange is writable or not. | |
| /// Check if a given PagedRange is writable or not. | |
| /// | |
| /// This checks writability by attempting a compare-exchange operation at each address in the range. | |
| /// If the compare-exchange succeeds, the page is considered writable; otherwise, an error is returned. |
SrinivasShekar
force-pushed
the
guestMemAccessWrappers
branch
from
caff9a4 to
ed9df2f
Compare
| pub fn probe_gpn_readable_range(&self, range: &PagedRange<'_>) -> Result<(), GuestMemoryError> { | ||
| self.op_range(GuestMemoryOperation::Probe, range, move |addr, _r| { | ||
| let mut b = [0]; | ||
| self.read_at_inner(addr, &mut b) |
There was a problem hiding this comment.
Use read_plain_inner, it's more efficient.
|
What's the context for this change? |
TFTR John. |
2 participants
This change adds support for checking whether the given pagedRange is readable or writable.
Background context:
For IDE DMA processing, we would need to check if the buffer that is going to be transferred, is within the guest address space. We would need to probe the entire buffer range to check if its readable or writable depending on the DMA type. These utilities would help in doing the same. Ref: https://github.com/microsoft/openvmm/pull/2373/files (still in draft)