Skip to content

Commit bf8faa1

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 3cbb73a commit bf8faa1

File tree

5 files changed

+62
-7
lines changed

5 files changed

+62
-7
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: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -600,7 +600,7 @@ fn attach_virtio_device<T: 'static + VirtioDevice + MutEventSubscriber + Debug>(
600600
event_manager.add_subscriber(device.clone());
601601

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

src/vmm/src/persist.rs

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

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().map_err(DirtyBitmap)?;
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
}
267316
};

0 commit comments

Comments
 (0)