Skip to content

Commit 7f27f14

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 a03e570 commit 7f27f14

File tree

2 files changed

+44
-5
lines changed

2 files changed

+44
-5
lines changed

src/vmm/src/builder.rs

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

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

src/vmm/src/vstate/vm/mod.rs

Lines changed: 43 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ struct VmCommon {
3333
fd: VmFd,
3434
max_memslots: usize,
3535
guest_memory: GuestMemoryMmap,
36+
io_memory: GuestMemoryMmap,
3637
}
3738

3839
/// Errors associated with the wrappers over KVM ioctls.
@@ -99,6 +100,7 @@ impl Vm {
99100
fd,
100101
max_memslots: kvm.max_nr_memslots(),
101102
guest_memory: GuestMemoryMmap::default(),
103+
io_memory: GuestMemoryMmap::default(),
102104
})
103105
}
104106

@@ -136,11 +138,41 @@ impl Vm {
136138

137139
/// Register a new memory region to this [`Vm`].
138140
pub fn register_memory_region(&mut self, region: GuestRegionMmap) -> Result<(), VmError> {
141+
let arcd_region = Arc::new(region);
142+
let new_guest_memory = self
143+
.common
144+
.guest_memory
145+
.insert_region(Arc::clone(&arcd_region))?;
146+
147+
self.register_kvm_region(arcd_region.as_ref())?;
148+
149+
self.common.guest_memory = new_guest_memory;
150+
Ok(())
151+
}
152+
153+
/// Registers a new io memory region to this [`Vm`].
154+
pub fn register_io_region(&mut self, region: GuestRegionMmap) -> Result<(), VmError> {
155+
let arcd_region = Arc::new(region);
156+
let new_io_memory = self
157+
.common
158+
.io_memory
159+
.insert_region(Arc::clone(&arcd_region))?;
160+
161+
self.register_kvm_region(arcd_region.as_ref())?;
162+
163+
self.common.io_memory = new_io_memory;
164+
Ok(())
165+
}
166+
167+
fn register_kvm_region(&mut self, region: &GuestRegionMmap) -> Result<(), VmError> {
139168
let next_slot = self
140169
.guest_memory()
141170
.num_regions()
171+
.checked_add(self.io_memory().num_regions())
172+
.ok_or(VmError::NotEnoughMemorySlots)?
142173
.try_into()
143174
.map_err(|_| VmError::NotEnoughMemorySlots)?;
175+
144176
if next_slot as usize + 1 >= self.common.max_memslots {
145177
return Err(VmError::NotEnoughMemorySlots);
146178
}
@@ -159,17 +191,13 @@ impl Vm {
159191
flags,
160192
};
161193

162-
let new_guest_memory = self.common.guest_memory.insert_region(Arc::new(region))?;
163-
164194
// SAFETY: Safe because the fd is a valid KVM file descriptor.
165195
unsafe {
166196
self.fd()
167197
.set_user_memory_region(memory_region)
168198
.map_err(VmError::SetUserMemoryRegion)?;
169199
}
170200

171-
self.common.guest_memory = new_guest_memory;
172-
173201
Ok(())
174202
}
175203

@@ -182,6 +210,17 @@ impl Vm {
182210
pub fn guest_memory(&self) -> &GuestMemoryMmap {
183211
&self.common.guest_memory
184212
}
213+
214+
/// Returns a reference to the [`GuestMemoryMmap`] that I/O devices attached to this [`Vm`]
215+
/// have access to. If no I/O regions were registered, return the same as [`Vm::guest_memory`],
216+
/// otherwise returns the [`GuestMemoryMmap`] describing a specific swiotlb region.
217+
pub fn io_memory(&self) -> &GuestMemoryMmap {
218+
if self.common.io_memory.num_regions() > 0 {
219+
&self.common.io_memory
220+
} else {
221+
&self.common.guest_memory
222+
}
223+
}
185224
}
186225

187226
#[cfg(test)]

0 commit comments

Comments
 (0)