Skip to content
Merged
69 changes: 59 additions & 10 deletions src/vmm/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

use std::fmt::Debug;
use std::io::{self};
use std::os::fd::AsFd;
use std::os::unix::fs::MetadataExt;
#[cfg(feature = "gdb")]
use std::sync::mpsc;
Expand Down Expand Up @@ -65,7 +66,7 @@
use crate::vmm_config::machine_config::MachineConfigError;
use crate::vmm_config::snapshot::{MemBackendConfig, MemBackendType};
use crate::vstate::kvm::Kvm;
use crate::vstate::memory::Bounce;
use crate::vstate::memory::MaybeBounce;

Check warning on line 69 in src/vmm/src/builder.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/builder.rs#L69

Added line #L69 was not covered by tests
use crate::vstate::vcpu::{Vcpu, VcpuError};
use crate::vstate::vm::{KVM_GMEM_NO_DIRECT_MAP, Vm};
use crate::{EventManager, Vmm, VmmError, device_manager};
Expand Down Expand Up @@ -235,6 +236,11 @@
false => None,
};

#[cfg(target_arch = "x86_64")]
if secret_free {
boot_cmdline.insert_str("no-kvmclock")?;

Check warning on line 241 in src/vmm/src/builder.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/builder.rs#L241

Added line #L241 was not covered by tests
}

let (mut vmm, mut vcpus) = create_vmm_and_vcpus(
instance_info,
event_manager,
Expand Down Expand Up @@ -274,7 +280,7 @@
}

let entry_point = load_kernel(
Bounce(&boot_config.kernel_file, secret_free),
MaybeBounce::new(boot_config.kernel_file.try_clone().unwrap(), secret_free),
vmm.vm.guest_memory(),
)?;
let initrd = match &boot_config.initrd_file {
Expand All @@ -286,7 +292,7 @@

Some(InitrdConfig::from_reader(
vmm.vm.guest_memory(),
Bounce(initrd_file, secret_free),
MaybeBounce::new(initrd_file.as_fd(), secret_free),

Check warning on line 295 in src/vmm/src/builder.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/builder.rs#L295

Added line #L295 was not covered by tests
u64_to_usize(size),
)?)
}
Expand Down Expand Up @@ -321,16 +327,24 @@
&mut boot_cmdline,
vm_resources.block.devices.iter(),
event_manager,
secret_free,
)?;
attach_net_devices(
&mut vmm,
&mut boot_cmdline,
vm_resources.net_builder.iter(),
event_manager,
secret_free,
)?;

if let Some(unix_vsock) = vm_resources.vsock.get() {
attach_unixsock_vsock_device(&mut vmm, &mut boot_cmdline, unix_vsock, event_manager)?;
attach_unixsock_vsock_device(
&mut vmm,
&mut boot_cmdline,
unix_vsock,
event_manager,
secret_free,
)?;

Check warning on line 347 in src/vmm/src/builder.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/builder.rs#L341-L347

Added lines #L341 - L347 were not covered by tests
}

if let Some(entropy) = vm_resources.entropy.get() {
Expand Down Expand Up @@ -707,14 +721,19 @@
device: Arc<Mutex<T>>,
cmdline: &mut LoaderKernelCmdline,
is_vhost_user: bool,
needs_bouncing: bool,
) -> Result<(), MmioError> {
event_manager.add_subscriber(device.clone());

// We have to enable swiotlb as part of the boot process, because the device objects
// themselves are created when the corresponding PUT API calls are made, and at that
// point we don't know yet whether swiotlb should be enabled or not.
if vmm.vm.has_swiotlb() {
// If swiotlb was requested via the API, enable it independently of whether
// bouncing is actually required (e.g. of whether secret hiding is enabled)

Check warning on line 733 in src/vmm/src/builder.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/builder.rs#L732-L733

Added lines #L732 - L733 were not covered by tests
device.lock().unwrap().force_swiotlb();
} else if needs_bouncing {
device.lock().unwrap().force_userspace_bounce_buffers();

Check warning on line 736 in src/vmm/src/builder.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/builder.rs#L736

Added line #L736 was not covered by tests
}

// The device mutex mustn't be locked here otherwise it will deadlock.
Expand Down Expand Up @@ -772,6 +791,7 @@
entropy_device.clone(),
cmdline,
false,
false,
)
}

Expand All @@ -780,6 +800,7 @@
cmdline: &mut LoaderKernelCmdline,
blocks: I,
event_manager: &mut EventManager,
needs_bouncing: bool,
) -> Result<(), StartMicrovmError> {
for block in blocks {
let (id, is_vhost_user) = {
Expand All @@ -804,6 +825,7 @@
block.clone(),
cmdline,
is_vhost_user,
needs_bouncing,
)?;
}
Ok(())
Expand All @@ -814,11 +836,20 @@
cmdline: &mut LoaderKernelCmdline,
net_devices: I,
event_manager: &mut EventManager,
needs_bouncing: bool,
) -> Result<(), StartMicrovmError> {
for net_device in net_devices {
let id = net_device.lock().expect("Poisoned lock").id().clone();
// The device mutex mustn't be locked here otherwise it will deadlock.
attach_virtio_device(event_manager, vmm, id, net_device.clone(), cmdline, false)?;
attach_virtio_device(
event_manager,
vmm,
id,
net_device.clone(),
cmdline,
false,
needs_bouncing,
)?;
}
Ok(())
}
Expand All @@ -828,10 +859,19 @@
cmdline: &mut LoaderKernelCmdline,
unix_vsock: &Arc<Mutex<Vsock<VsockUnixBackend>>>,
event_manager: &mut EventManager,
needs_bouncing: bool,
) -> Result<(), MmioError> {
let id = String::from(unix_vsock.lock().expect("Poisoned lock").id());
// The device mutex mustn't be locked here otherwise it will deadlock.
attach_virtio_device(event_manager, vmm, id, unix_vsock.clone(), cmdline, false)
attach_virtio_device(
event_manager,
vmm,
id,
unix_vsock.clone(),
cmdline,
false,
needs_bouncing,
)
}

fn attach_balloon_device(
Expand All @@ -842,7 +882,15 @@
) -> Result<(), MmioError> {
let id = String::from(balloon.lock().expect("Poisoned lock").id());
// The device mutex mustn't be locked here otherwise it will deadlock.
attach_virtio_device(event_manager, vmm, id, balloon.clone(), cmdline, false)
attach_virtio_device(
event_manager,
vmm,
id,
balloon.clone(),
cmdline,
false,
false,
)
}

// Adds `O_NONBLOCK` to the stdout flags.
Expand Down Expand Up @@ -1018,6 +1066,7 @@
cmdline,
block_dev_configs.devices.iter(),
event_manager,
false,
)
.unwrap();
block_files
Expand All @@ -1032,7 +1081,7 @@
let mut net_builder = NetBuilder::new();
net_builder.build(net_config).unwrap();

let res = attach_net_devices(vmm, cmdline, net_builder.iter(), event_manager);
let res = attach_net_devices(vmm, cmdline, net_builder.iter(), event_manager, false);
res.unwrap();
}

Expand All @@ -1053,7 +1102,7 @@
Arc::new(Mutex::new(mmds)),
);

attach_net_devices(vmm, cmdline, net_builder.iter(), event_manager).unwrap();
attach_net_devices(vmm, cmdline, net_builder.iter(), event_manager, false).unwrap();
}

pub(crate) fn insert_vsock_device(
Expand All @@ -1066,7 +1115,7 @@
let vsock = VsockBuilder::create_unixsock_vsock(vsock_config).unwrap();
let vsock = Arc::new(Mutex::new(vsock));

attach_unixsock_vsock_device(vmm, cmdline, &vsock, event_manager).unwrap();
attach_unixsock_vsock_device(vmm, cmdline, &vsock, event_manager, false).unwrap();

assert!(
vmm.mmio_device_manager
Expand Down
8 changes: 8 additions & 0 deletions src/vmm/src/device_manager/mmio.rs
Original file line number Diff line number Diff line change
Expand Up @@ -604,6 +604,14 @@ mod tests {
unimplemented!()
}

fn force_userspace_bounce_buffers(&mut self) {
todo!()
}

fn userspace_bounce_buffers(&self) -> bool {
todo!()
}

fn device_type(&self) -> u32 {
0
}
Expand Down
8 changes: 8 additions & 0 deletions src/vmm/src/devices/virtio/balloon/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -563,6 +563,14 @@
self.avail_features |= 1 << VIRTIO_F_ACCESS_PLATFORM;
}

fn force_userspace_bounce_buffers(&mut self) {
// balloon device doesn't have a need for bounce buffers
}

Check warning on line 568 in src/vmm/src/devices/virtio/balloon/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/balloon/device.rs#L566-L568

Added lines #L566 - L568 were not covered by tests

fn userspace_bounce_buffers(&self) -> bool {
false
}

fn device_type(&self) -> u32 {
TYPE_BALLOON
}
Expand Down
14 changes: 14 additions & 0 deletions src/vmm/src/devices/virtio/block/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,20 @@
}
}

fn force_userspace_bounce_buffers(&mut self) {
match self {
Block::Virtio(b) => b.force_userspace_bounce_buffers(),
Block::VhostUser(b) => b.force_userspace_bounce_buffers(),

Check warning on line 161 in src/vmm/src/devices/virtio/block/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/block/device.rs#L158-L161

Added lines #L158 - L161 were not covered by tests
}
}

Check warning on line 163 in src/vmm/src/devices/virtio/block/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/block/device.rs#L163

Added line #L163 was not covered by tests

fn userspace_bounce_buffers(&self) -> bool {
match self {
Block::Virtio(b) => b.userspace_bounce_buffers(),
Block::VhostUser(b) => b.userspace_bounce_buffers(),

Check warning on line 168 in src/vmm/src/devices/virtio/block/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/block/device.rs#L165-L168

Added lines #L165 - L168 were not covered by tests
}
}

Check warning on line 170 in src/vmm/src/devices/virtio/block/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/block/device.rs#L170

Added line #L170 was not covered by tests

fn device_type(&self) -> u32 {
TYPE_BLOCK
}
Expand Down
9 changes: 9 additions & 0 deletions src/vmm/src/devices/virtio/block/vhost_user/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,15 @@
self.avail_features |= 1 << VIRTIO_F_ACCESS_PLATFORM;
}

fn force_userspace_bounce_buffers(&mut self) {
// Nothing Firecracker can do about this, the backend would need to do the bouncing
panic!("vhost-user-blk is incompatible with userspace bounce buffers")

Check warning on line 305 in src/vmm/src/devices/virtio/block/vhost_user/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/block/vhost_user/device.rs#L303-L305

Added lines #L303 - L305 were not covered by tests
}

fn userspace_bounce_buffers(&self) -> bool {
false
}

Check warning on line 310 in src/vmm/src/devices/virtio/block/vhost_user/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/block/vhost_user/device.rs#L308-L310

Added lines #L308 - L310 were not covered by tests

fn device_type(&self) -> u32 {
TYPE_BLOCK
}
Expand Down
16 changes: 16 additions & 0 deletions src/vmm/src/devices/virtio/block/virtio/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,6 +584,22 @@
self.avail_features |= 1 << VIRTIO_F_ACCESS_PLATFORM;
}

fn force_userspace_bounce_buffers(&mut self) {
match self.disk.file_engine {

Check warning on line 588 in src/vmm/src/devices/virtio/block/virtio/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/block/virtio/device.rs#L587-L588

Added lines #L587 - L588 were not covered by tests
FileEngine::Async(_) => {
panic!("async engine is incompatible with userspace bounce buffers")

Check warning on line 590 in src/vmm/src/devices/virtio/block/virtio/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/block/virtio/device.rs#L590

Added line #L590 was not covered by tests
}
FileEngine::Sync(ref mut engine) => engine.start_bouncing(),
}
}

Check warning on line 594 in src/vmm/src/devices/virtio/block/virtio/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/block/virtio/device.rs#L592-L594

Added lines #L592 - L594 were not covered by tests

fn userspace_bounce_buffers(&self) -> bool {
match self.disk.file_engine {
FileEngine::Async(_) => false,

Check warning on line 598 in src/vmm/src/devices/virtio/block/virtio/device.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/block/virtio/device.rs#L598

Added line #L598 was not covered by tests
FileEngine::Sync(ref engine) => engine.is_bouncing(),
}
}

fn device_type(&self) -> u32 {
TYPE_BLOCK
}
Expand Down
29 changes: 22 additions & 7 deletions src/vmm/src/devices/virtio/block/virtio/io/sync_io.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

use vm_memory::{GuestMemoryError, ReadVolatile, WriteVolatile};

use crate::vstate::memory::{GuestAddress, GuestMemory, GuestMemoryMmap};
use crate::vstate::memory::{GuestAddress, GuestMemory, GuestMemoryMmap, MaybeBounce};

#[derive(Debug, thiserror::Error, displaydoc::Display)]
pub enum SyncIoError {
Expand All @@ -22,25 +22,40 @@

#[derive(Debug)]
pub struct SyncFileEngine {
file: File,
// 65536 is the largest buffer a linux guest will give us, empirically. Determined by
// having `MaybeBounce` logging scenarios where the fixed size bounce buffer isn't sufficient.
// Note that even if this assumption ever changes, the worse that'll happen is that we do
// multiple roundtrips between guest memory and the bounce buffer, as MaybeBounce would
// just chop larger reads/writes into chunks of 65k.
file: MaybeBounce<File, { u16::MAX as usize + 1 }>,
}

// SAFETY: `File` is send and ultimately a POD.
unsafe impl Send for SyncFileEngine {}

impl SyncFileEngine {
pub fn from_file(file: File) -> SyncFileEngine {
SyncFileEngine { file }
SyncFileEngine {
file: MaybeBounce::new_persistent(file, false),
}
}

#[cfg(test)]
pub fn file(&self) -> &File {
&self.file
&self.file.target
}

pub fn start_bouncing(&mut self) {
self.file.activate()
}

Check warning on line 50 in src/vmm/src/devices/virtio/block/virtio/io/sync_io.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/block/virtio/io/sync_io.rs#L48-L50

Added lines #L48 - L50 were not covered by tests

pub fn is_bouncing(&self) -> bool {
self.file.is_activated()
}

/// Update the backing file of the engine
pub fn update_file(&mut self, file: File) {
self.file = file
self.file.target = file
}

pub fn read(
Expand Down Expand Up @@ -77,8 +92,8 @@

pub fn flush(&mut self) -> Result<(), SyncIoError> {
// flush() first to force any cached data out of rust buffers.
self.file.flush().map_err(SyncIoError::Flush)?;
self.file.target.flush().map_err(SyncIoError::Flush)?;
// Sync data out to physical media on host.
self.file.sync_all().map_err(SyncIoError::SyncAll)
self.file.target.sync_all().map_err(SyncIoError::SyncAll)
}
}
12 changes: 9 additions & 3 deletions src/vmm/src/devices/virtio/block/virtio/persist.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
use crate::devices::virtio::block::persist::BlockConstructorArgs;
use crate::devices::virtio::block::virtio::device::FileEngineType;
use crate::devices::virtio::block::virtio::metrics::BlockMetricsPerDevice;
use crate::devices::virtio::device::{DeviceState, IrqTrigger};
use crate::devices::virtio::device::{DeviceState, IrqTrigger, VirtioDevice};
use crate::devices::virtio::generated::virtio_blk::VIRTIO_BLK_F_RO;
use crate::devices::virtio::persist::VirtioDeviceState;
use crate::rate_limiter::RateLimiter;
Expand Down Expand Up @@ -127,7 +127,7 @@
capacity: disk_properties.nsectors.to_le(),
};

Ok(VirtioBlock {
let mut dev = VirtioBlock {
avail_features,
acked_features,
config_space,
Expand All @@ -148,7 +148,13 @@
rate_limiter,
is_io_engine_throttled: false,
metrics: BlockMetricsPerDevice::alloc(state.id.clone()),
})
};

if state.virtio_state.bounce_in_userspace {
dev.force_userspace_bounce_buffers()

Check warning on line 154 in src/vmm/src/devices/virtio/block/virtio/persist.rs

View check run for this annotation

Codecov / codecov/patch

src/vmm/src/devices/virtio/block/virtio/persist.rs#L154

Added line #L154 was not covered by tests
}

Ok(dev)
}
}

Expand Down
Loading