Skip to content

Commit 120c6f4

Browse files
committed
add concept of "io memory" to struct Vm
Add a special `GuestMemoryMmap` to `struct Vm` that has the semantics that if its not empty, it represents the swiotlb area in the guest. Have all virtio devices use io memory by default. This means that in case of swiotlb regions being set up, virtio devices cannot access generic memory, so if the guest decides to (incorrectly) place I/O buffers outside of swiotlb we'll get a somewhat meaningful error of "invalid guest address" (or similar), instead of random things failing further down the line in case generic memory isn't accessible by firecracker/the host kernel for some reason (e.g. if its guest_memfd backed in the future). Signed-off-by: Patrick Roy <[email protected]>
1 parent 9db2212 commit 120c6f4

File tree

2 files changed

+45
-5
lines changed

2 files changed

+45
-5
lines changed

src/vmm/src/builder.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -868,7 +868,7 @@ fn attach_virtio_device<T: 'static + VirtioDevice + MutEventSubscriber + Debug>(
868868
event_manager.add_subscriber(device.clone());
869869

870870
// The device mutex mustn't be locked here otherwise it will deadlock.
871-
let device = MmioTransport::new(vmm.guest_memory().clone(), device, is_vhost_user);
871+
let device = MmioTransport::new(vmm.vm.io_memory().clone(), device, is_vhost_user);
872872
vmm.mmio_device_manager
873873
.register_mmio_virtio_for_boot(
874874
vmm.vm.fd(),

src/vmm/src/vstate/vm.rs

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@ pub struct VmCommon {
2727
max_memslots: usize,
2828
/// The guest memory of this Vm.
2929
pub guest_memory: GuestMemoryMmap,
30+
/// The swiotlb regions of this Vm.
31+
pub io_memory: GuestMemoryMmap,
3032
}
3133

3234
/// Errors associated with the wrappers over KVM ioctls.
@@ -94,6 +96,7 @@ impl Vm {
9496
fd,
9597
max_memslots: kvm.max_nr_memslots(),
9698
guest_memory: GuestMemoryMmap::default(),
99+
io_memory: GuestMemoryMmap::default(),
97100
})
98101
}
99102

@@ -131,11 +134,41 @@ impl Vm {
131134

132135
/// Register a new memory region to this [`Vm`].
133136
pub fn register_memory_region(&mut self, region: GuestRegionMmap) -> Result<(), VmError> {
137+
let arcd_region = Arc::new(region);
138+
let new_guest_memory = self
139+
.common
140+
.guest_memory
141+
.insert_region(Arc::clone(&arcd_region))?;
142+
143+
self.register_kvm_region(arcd_region.as_ref())?;
144+
145+
self.common.guest_memory = new_guest_memory;
146+
Ok(())
147+
}
148+
149+
/// Registers a new io memory region to this [`Vm`].
150+
pub fn register_io_region(&mut self, region: GuestRegionMmap) -> Result<(), VmError> {
151+
let arcd_region = Arc::new(region);
152+
let new_io_memory = self
153+
.common
154+
.io_memory
155+
.insert_region(Arc::clone(&arcd_region))?;
156+
157+
self.register_kvm_region(arcd_region.as_ref())?;
158+
159+
self.common.io_memory = new_io_memory;
160+
Ok(())
161+
}
162+
163+
fn register_kvm_region(&mut self, region: &GuestRegionMmap) -> Result<(), VmError> {
134164
let next_slot = self
135165
.guest_memory()
136166
.num_regions()
167+
.checked_add(self.io_memory().num_regions())
168+
.ok_or(VmError::NotEnoughMemorySlots)?
137169
.try_into()
138170
.map_err(|_| VmError::NotEnoughMemorySlots)?;
171+
139172
if next_slot as usize + 1 >= self.common.max_memslots {
140173
return Err(VmError::NotEnoughMemorySlots);
141174
}
@@ -154,17 +187,13 @@ impl Vm {
154187
flags,
155188
};
156189

157-
let new_guest_memory = self.common.guest_memory.insert_region(Arc::new(region))?;
158-
159190
// SAFETY: Safe because the fd is a valid KVM file descriptor.
160191
unsafe {
161192
self.fd()
162193
.set_user_memory_region(memory_region)
163194
.map_err(VmError::SetUserMemoryRegion)?;
164195
}
165196

166-
self.common.guest_memory = new_guest_memory;
167-
168197
Ok(())
169198
}
170199

@@ -177,6 +206,17 @@ impl Vm {
177206
pub fn guest_memory(&self) -> &GuestMemoryMmap {
178207
&self.common.guest_memory
179208
}
209+
210+
/// Returns a reference to the [`GuestMemoryMmap`] that I/O devices attached to this [`Vm`]
211+
/// have access to. If no I/O regions were registered, return the same as [`Vm::guest_memory`],
212+
/// otherwise returns the [`GuestMemoryMmap`] describing a specific swiotlb region.
213+
pub fn io_memory(&self) -> &GuestMemoryMmap {
214+
if self.common.io_memory.num_regions() > 0 {
215+
&self.common.io_memory
216+
} else {
217+
&self.common.guest_memory
218+
}
219+
}
180220
}
181221

182222
#[cfg(test)]

0 commit comments

Comments
 (0)