Skip to content

Commit 9d758f1

Browse files
committed
add concept of "swiotlb regions" 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 only get access to swiotlb regions if they exist, only falling back onto having access to all of guest memory otherwise. 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). When a VM with swiotlb regions is being snapshotted, the regions are simply concatenated to the end of the snapshot memory file. This changes the snapshot memory file format in the sense that regions in the file are not necessarily sorted by guest address anymore. Signed-off-by: Patrick Roy <[email protected]>
1 parent 119a9a3 commit 9d758f1

File tree

5 files changed

+63
-8
lines changed

5 files changed

+63
-8
lines changed

src/vmm/src/arch/aarch64/vm.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ impl ArchVm {
7070
pub fn save_state(&self, mpidrs: &[u64]) -> Result<VmState, ArchVmError> {
7171
Ok(VmState {
7272
memory: self.common.guest_memory.describe(),
73+
io_memory: self.common.swiotlb_regions.describe(),
7374
gic: self
7475
.get_irqchip()
7576
.save_device(mpidrs)
@@ -96,6 +97,8 @@ impl ArchVm {
9697
pub struct VmState {
9798
/// Guest memory state
9899
pub memory: GuestMemoryState,
100+
/// io memory state
101+
pub io_memory: GuestMemoryState,
99102
/// GIC state.
100103
pub gic: GicState,
101104
}

src/vmm/src/arch/x86_64/vm.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,7 @@ impl ArchVm {
187187

188188
Ok(VmState {
189189
memory: self.common.guest_memory.describe(),
190+
io_memory: self.common.swiotlb_regions.describe(),
190191
pitstate,
191192
clock,
192193
pic_master,
@@ -211,6 +212,8 @@ impl ArchVm {
211212
pub struct VmState {
212213
/// guest memory state
213214
pub memory: GuestMemoryState,
215+
/// io memory state
216+
pub io_memory: GuestMemoryState,
214217
pitstate: kvm_pit_state2,
215218
clock: kvm_clock_data,
216219
// TODO: rename this field to adopt inclusive language once Linux updates it, too.

src/vmm/src/builder.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -471,7 +471,7 @@ pub fn build_microvm_from_snapshot(
471471

472472
// Restore devices states.
473473
let mmio_ctor_args = MMIODevManagerConstructorArgs {
474-
mem: vmm.vm.guest_memory(),
474+
mem: vmm.vm.io_memory(),
475475
vm: vmm.vm.fd(),
476476
event_manager,
477477
resource_allocator: &mut vmm.resource_allocator,
@@ -596,7 +596,7 @@ fn attach_virtio_device<T: 'static + VirtioDevice + MutEventSubscriber + Debug>(
596596
event_manager.add_subscriber(device.clone());
597597

598598
// The device mutex mustn't be locked here otherwise it will deadlock.
599-
let device = MmioTransport::new(vmm.vm.guest_memory().clone(), device, is_vhost_user);
599+
let device = MmioTransport::new(vmm.vm.io_memory().clone(), device, is_vhost_user);
600600
vmm.mmio_device_manager
601601
.register_mmio_virtio_for_boot(
602602
vmm.vm.fd(),

src/vmm/src/persist.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ pub fn create_snapshot(
180180
.for_each_virtio_device(|_, _, _, dev| {
181181
let d = dev.lock().unwrap();
182182
if d.is_activated() {
183-
d.mark_queue_memory_dirty(vmm.vm.guest_memory())
183+
d.mark_queue_memory_dirty(vmm.vm.io_memory())
184184
} else {
185185
Ok(())
186186
}

src/vmm/src/vstate/vm.rs

Lines changed: 54 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,8 @@ pub struct VmCommon {
3434
max_memslots: usize,
3535
/// The guest memory of this Vm.
3636
pub guest_memory: GuestMemoryMmap,
37+
/// The swiotlb regions of this Vm.
38+
pub swiotlb_regions: GuestMemoryMmap,
3739
}
3840

3941
/// Errors associated with the wrappers over KVM ioctls.
@@ -101,6 +103,7 @@ impl Vm {
101103
fd,
102104
max_memslots: kvm.max_nr_memslots(),
103105
guest_memory: GuestMemoryMmap::default(),
106+
swiotlb_regions: GuestMemoryMmap::default(),
104107
})
105108
}
106109

@@ -140,6 +143,8 @@ impl Vm {
140143
let next_slot = self
141144
.guest_memory()
142145
.num_regions()
146+
.checked_add(self.swiotlb_regions().num_regions())
147+
.ok_or(VmError::NotEnoughMemorySlots)?
143148
.try_into()
144149
.map_err(|_| VmError::NotEnoughMemorySlots)?;
145150

@@ -175,19 +180,59 @@ impl Vm {
175180
Ok(())
176181
}
177182

183+
/// Registers a new io memory region to this [`Vm`].
184+
pub fn register_swiotlb_region(&mut self, region: GuestRegionMmap) -> Result<(), VmError> {
185+
let arcd_region = Arc::new(self.kvmify_region(region)?);
186+
let new_collection = self
187+
.common
188+
.swiotlb_regions
189+
.insert_region(Arc::clone(&arcd_region))?;
190+
191+
self.register_kvm_region(arcd_region.as_ref())?;
192+
193+
self.common.swiotlb_regions = new_collection;
194+
Ok(())
195+
}
196+
178197
/// Gets a reference to the kvm file descriptor owned by this VM.
179198
pub fn fd(&self) -> &VmFd {
180199
&self.common.fd
181200
}
182201

183-
/// Gets a reference to this [`Vm`]'s [`GuestMemoryMmap`] object
202+
/// Gets a reference to this [`Vm`]'s [`GuestMemoryMmap`] object, which
203+
/// contains all non-swiotlb guest memory regions.
184204
pub fn guest_memory(&self) -> &GuestMemoryMmap {
185205
&self.common.guest_memory
186206
}
187207

208+
/// Returns a reference to the [`GuestMemoryMmap`] that I/O devices attached to this [`Vm`]
209+
/// have access to. If no I/O regions were registered, return the same as [`Vm::guest_memory`],
210+
/// otherwise returns the [`GuestMemoryMmap`] describing a specific swiotlb region.
211+
pub fn io_memory(&self) -> &GuestMemoryMmap {
212+
if self.common.swiotlb_regions.num_regions() > 0 {
213+
&self.common.swiotlb_regions
214+
} else {
215+
&self.common.guest_memory
216+
}
217+
}
218+
219+
/// Gets a reference to the [`GuestMemoryMmap`] holding the swiotlb regions registered to
220+
/// this [`Vm`]. Unlike [`Vm::io_memory`], does not fall back to returning the
221+
/// [`GuestMemoryMmap`] of normal memory when no swiotlb regions were registered.
222+
pub fn swiotlb_regions(&self) -> &GuestMemoryMmap {
223+
&self.common.swiotlb_regions
224+
}
225+
226+
/// Returns an iterator over all regions, normal and swiotlb.
227+
fn all_regions(&self) -> impl Iterator<Item = &KvmRegion> {
228+
self.guest_memory()
229+
.iter()
230+
.chain(self.common.swiotlb_regions.iter())
231+
}
232+
188233
/// Resets the KVM dirty bitmap for each of the guest's memory regions.
189234
pub fn reset_dirty_bitmap(&self) {
190-
self.guest_memory().iter().for_each(|region| {
235+
self.all_regions().for_each(|region| {
191236
let _ = self
192237
.fd()
193238
.get_dirty_log(region.inner().slot, u64_to_usize(region.len()));
@@ -197,7 +242,7 @@ impl Vm {
197242
/// Retrieves the KVM dirty bitmap for each of the guest's memory regions.
198243
pub fn get_dirty_bitmap(&self) -> Result<DirtyBitmap, vmm_sys_util::errno::Error> {
199244
let mut bitmap: DirtyBitmap = HashMap::new();
200-
self.guest_memory().iter().try_for_each(|region| {
245+
self.all_regions().try_for_each(|region| {
201246
self.fd()
202247
.get_dirty_log(region.inner().slot, u64_to_usize(region.len()))
203248
.map(|bitmap_region| _ = bitmap.insert(region.inner().slot, bitmap_region))
@@ -258,10 +303,14 @@ impl Vm {
258303
match snapshot_type {
259304
SnapshotType::Diff => {
260305
let dirty_bitmap = self.get_dirty_bitmap()?;
261-
self.guest_memory().dump_dirty(&mut file, &dirty_bitmap)?;
306+
self.guest_memory()
307+
.dump_dirty(&mut file, &dirty_bitmap)
308+
.and_then(|_| self.swiotlb_regions().dump_dirty(&mut file, &dirty_bitmap))?;
262309
}
263310
SnapshotType::Full => {
264-
self.guest_memory().dump(&mut file)?;
311+
self.guest_memory()
312+
.dump(&mut file)
313+
.and_then(|_| self.swiotlb_regions().dump(&mut file))?;
265314
self.reset_dirty_bitmap();
266315
self.guest_memory().reset_dirty();
267316
}

0 commit comments

Comments
 (0)