diff --git a/CHANGELOG.md b/CHANGELOG.md index ebcea780da9..41b57d1c8a5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -35,6 +35,12 @@ and this project adheres to requests to `/mmds/config` to enforce MMDS to always respond plain text contents in the IMDS format regardless of the `Accept` header in requests. Users need to regenerate snapshots. +- [#5364](https://github.com/firecracker-microvm/firecracker/pull/5364): Added + PCI support in Firecracker. PCI support is optional. Users can enable it + passing the `--enable-pci` flag when launching the Firecracker process. When + Firecracker process is launched with PCI support, it will create all VirtIO + devices using a PCI VirtIO transport. If not enabled, Firecracker will use the + MMIO transport instead. ### Changed diff --git a/FAQ.md b/FAQ.md index 3ace710fc96..d5410237add 100644 --- a/FAQ.md +++ b/FAQ.md @@ -129,7 +129,7 @@ Example of a kernel valid command line that enables the serial console (which goes in the `boot_args` field of the `/boot-source` Firecracker API resource): ```console -console=ttyS0 reboot=k panic=1 pci=off nomodule +console=ttyS0 reboot=k panic=1 nomodule ``` ### How can I configure multiple Ethernet devices through the kernel command line? diff --git a/docs/getting-started.md b/docs/getting-started.md index f0b02c6e1b5..17c681da34c 100644 --- a/docs/getting-started.md +++ b/docs/getting-started.md @@ -194,9 +194,16 @@ API_SOCKET="/tmp/firecracker.socket" sudo rm -f $API_SOCKET # Run firecracker -sudo ./firecracker --api-sock "${API_SOCKET}" +sudo ./firecracker --api-sock "${API_SOCKET} --enable-pci" ``` +The `--enable-pci` flag instructs Firecracker to create all VirtIO devices using +a PCI VirtIO transport. This flag is optional. If not passed, Firecracker will +create devices using the legacy MMIO transport. We suggest that users enable the +PCI transport, as it yields higher throughput and lower latency for VirtIO +devices. For more information regarding guest kernel requirements for using PCI +look at our [kernel policy documentation](./kernel-policy.md). + In a new terminal (do not close the 1st one): ```bash @@ -240,7 +247,7 @@ sudo curl -X PUT --unix-socket "${API_SOCKET}" \ "http://localhost/logger" KERNEL="./$(ls vmlinux* | tail -1)" -KERNEL_BOOT_ARGS="console=ttyS0 reboot=k panic=1 pci=off" +KERNEL_BOOT_ARGS="console=ttyS0 reboot=k panic=1" ARCH=$(uname -m) diff --git a/docs/kernel-policy.md b/docs/kernel-policy.md index d67496d8435..20b3ed4c415 100644 --- a/docs/kernel-policy.md +++ b/docs/kernel-policy.md @@ -67,6 +67,15 @@ The configuration items that may be relevant for Firecracker are: - use CPU RNG instructions (if present) to initialize RNG. Available for >= 5.10 - ACPI support - `CONFIG_ACPI` and `CONFIG_PCI` +- PCI support: + - `CONFIG_BLK_MQ_PCI` + - `CONFIG_PCI` + - `CONFIG_PCI_MMCONFIG` + - `CONFIG_PCI_MSI` + - `CONFIG_PCIEPORTBUS` + - `CONFIG_VIRTIO_PCI` + - `CONFIG_PCI_HOST_COMMON` + - `CONFIG_PCI_HOST_GENERIC` There are also guest config options which are dependant on the platform on which Firecracker is run: @@ -138,6 +147,63 @@ following configurations: - Only legacy mechanisms - Both ACPI and legacy mechanisms +##### Booting with PCI: + +Firecracker supports booting guest microVMs with PCI support. This option is +enabled using the `--enable-pci` flag when launching the Firecracker process. +With PCI enabled, Firecracker will create all VirtIO devices using a PCI VirtIO +transport. The PCI transport typically achieves higher throughput and lower +latency for VirtIO devices. No further, per device, configuration is needed to +enable the PCI transport. + +PCI support is optional; if it is not enabled Firecracker will create VirtIO +devices using the MMIO transport. + +For Firecracker microVMs to boot properly with PCI support, use a guest kernel +built with PCI support. See the relevant Kconfig flags in our list of +[relevant Kconfig options](#guest-kernel-configuration-items): + +> [!IMPORTANT] +> +> Make sure that the kernel command line **does NOT** include the `pci=off` +> slug, which disables PCI support during boot time within the guest. When PCI +> is disabled, Firecracker will add this slug in the command line to instruct +> the guest kernel to skip useless PCI checks. For more info, look into the +> section for [Kernel command line parameters](#kernel-command-line-parameters). + +> [!NOTE] +> +> On x86_64 systems, `CONFIG_PCI` Kconfig option is needed even when booting +> microVMs without PCI support in case users want to use ACPI to boot. See +> [here](#booting-with-acpi-x86_64-only) for more info. + +## Kernel command line parameters + +By default, Firecracker will boot a guest microVM passing the following command +line parameters to the kernel: + +`reboot=k panic=1 nomodule 8250.nr_uarts=0 i8042.noaux i8042.nomux i8042.dumbkbd swiotlb=noforce`. + +- `reboot=k` shut down the guest on reboot, instead of rebooting +- `panic=1` on panic, reboot after 1 second +- `nomodule` disable loadable kernel module support +- `8250.nr_uarts=0` disable 8250 serial interface +- `i8042.noaux` do not probe the i8042 controller for an attached mouse (save + boot time) +- `i8042.nomux` do not probe i8042 for a multiplexing controller (save boot + time) +- `i8042.dumbkbd` do not attempt to control kbd state via the i8042 (save boot + time) +- `swiotlb=noforce` disable software bounce buffers (SWIOTLB) + +When running without [PCI support](#booting-with-pci), Firecracker will also +append `pci=off` to the above list. This option instructs the guest kernel to +avoid PCI probing. + +Users can provide their own command line parameters through the `boot_args` +field of the `/boot-source` +[Firecracker API](../src/firecracker/swagger/firecracker.yaml). + ## Caveats - [Snapshot compatibility across kernel versions](snapshotting/snapshot-support.md#snapshot-compatibility-across-kernel-versions) diff --git a/src/pci/src/bus.rs b/src/pci/src/bus.rs index 01c9b1f1933..12188ee7ba5 100644 --- a/src/pci/src/bus.rs +++ b/src/pci/src/bus.rs @@ -161,6 +161,9 @@ impl PciConfigIo { return 0xffff_ffff; } + // NOTE: Potential contention among vCPU threads on this lock. This should not + // be a problem currently, since we mainly access this when we are setting up devices. + // We might want to do some profiling to ensure this does not become a bottleneck. self.pci_bus .as_ref() .lock() @@ -195,6 +198,9 @@ impl PciConfigIo { return None; } + // NOTE: Potential contention among vCPU threads on this lock. This should not + // be a problem currently, since we mainly access this when we are setting up devices. + // We might want to do some profiling to ensure this does not become a bottleneck. let pci_bus = self.pci_bus.as_ref().lock().unwrap(); if let Some(d) = pci_bus.devices.get(&(device as u32)) { let mut device = d.lock().unwrap(); diff --git a/src/vmm/src/builder.rs b/src/vmm/src/builder.rs index 88d7f56cb4e..59789ef9b2e 100644 --- a/src/vmm/src/builder.rs +++ b/src/vmm/src/builder.rs @@ -648,9 +648,9 @@ pub(crate) mod tests { use super::*; use crate::device_manager::tests::default_device_manager; use crate::devices::virtio::block::CacheType; + use crate::devices::virtio::generated::virtio_ids; use crate::devices::virtio::rng::device::ENTROPY_DEV_ID; - use crate::devices::virtio::vsock::{TYPE_VSOCK, VSOCK_DEV_ID}; - use crate::devices::virtio::{TYPE_BALLOON, TYPE_BLOCK, TYPE_RNG}; + use crate::devices::virtio::vsock::VSOCK_DEV_ID; use crate::mmds::data_store::{Mmds, MmdsVersion}; use crate::mmds::ns::MmdsNetworkStack; use crate::utils::mib_to_bytes; @@ -848,7 +848,7 @@ pub(crate) mod tests { assert!( vmm.device_manager - .get_virtio_device(TYPE_VSOCK, &vsock_dev_id) + .get_virtio_device(virtio_ids::VIRTIO_ID_VSOCK, &vsock_dev_id) .is_some() ); } @@ -873,7 +873,7 @@ pub(crate) mod tests { assert!( vmm.device_manager - .get_virtio_device(TYPE_RNG, ENTROPY_DEV_ID) + .get_virtio_device(virtio_ids::VIRTIO_ID_RNG, ENTROPY_DEV_ID) .is_some() ); } @@ -907,7 +907,7 @@ pub(crate) mod tests { assert!( vmm.device_manager - .get_virtio_device(TYPE_BALLOON, BALLOON_DEV_ID) + .get_virtio_device(virtio_ids::VIRTIO_ID_BALLOON, BALLOON_DEV_ID) .is_some() ); } @@ -958,7 +958,7 @@ pub(crate) mod tests { assert!(cmdline_contains(&cmdline, "root=/dev/vda ro")); assert!( vmm.device_manager - .get_virtio_device(TYPE_BLOCK, drive_id.as_str()) + .get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, drive_id.as_str()) .is_some() ); } @@ -979,7 +979,7 @@ pub(crate) mod tests { assert!(cmdline_contains(&cmdline, "root=PARTUUID=0eaa91a0-01 rw")); assert!( vmm.device_manager - .get_virtio_device(TYPE_BLOCK, drive_id.as_str()) + .get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, drive_id.as_str()) .is_some() ); } @@ -1001,7 +1001,7 @@ pub(crate) mod tests { assert!(!cmdline_contains(&cmdline, "root=/dev/vda")); assert!( vmm.device_manager - .get_virtio_device(TYPE_BLOCK, drive_id.as_str()) + .get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, drive_id.as_str()) .is_some() ); } @@ -1038,17 +1038,17 @@ pub(crate) mod tests { assert!(cmdline_contains(&cmdline, "root=PARTUUID=0eaa91a0-01 rw")); assert!( vmm.device_manager - .get_virtio_device(TYPE_BLOCK, "root") + .get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, "root") .is_some() ); assert!( vmm.device_manager - .get_virtio_device(TYPE_BLOCK, "secondary") + .get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, "secondary") .is_some() ); assert!( vmm.device_manager - .get_virtio_device(TYPE_BLOCK, "third") + .get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, "third") .is_some() ); @@ -1077,7 +1077,7 @@ pub(crate) mod tests { assert!(cmdline_contains(&cmdline, "root=/dev/vda rw")); assert!( vmm.device_manager - .get_virtio_device(TYPE_BLOCK, drive_id.as_str()) + .get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, drive_id.as_str()) .is_some() ); } @@ -1098,7 +1098,7 @@ pub(crate) mod tests { assert!(cmdline_contains(&cmdline, "root=PARTUUID=0eaa91a0-01 ro")); assert!( vmm.device_manager - .get_virtio_device(TYPE_BLOCK, drive_id.as_str()) + .get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, drive_id.as_str()) .is_some() ); } @@ -1119,7 +1119,7 @@ pub(crate) mod tests { assert!(cmdline_contains(&cmdline, "root=/dev/vda rw")); assert!( vmm.device_manager - .get_virtio_device(TYPE_BLOCK, drive_id.as_str()) + .get_virtio_device(virtio_ids::VIRTIO_ID_BLOCK, drive_id.as_str()) .is_some() ); } diff --git a/src/vmm/src/device_manager/mmio.rs b/src/vmm/src/device_manager/mmio.rs index a87646b11cf..46accb637b0 100644 --- a/src/vmm/src/device_manager/mmio.rs +++ b/src/vmm/src/device_manager/mmio.rs @@ -468,7 +468,7 @@ pub(crate) mod tests { use crate::test_utils::multi_region_mem_raw; use crate::vstate::kvm::Kvm; use crate::vstate::memory::{GuestAddress, GuestMemoryMmap}; - use crate::{Vm, arch}; + use crate::{Vm, arch, impl_device_type}; const QUEUE_SIZES: &[u16] = &[64]; @@ -522,6 +522,8 @@ pub(crate) mod tests { } impl VirtioDevice for DummyDevice { + impl_device_type!(0); + fn avail_features(&self) -> u64 { 0 } @@ -532,10 +534,6 @@ pub(crate) mod tests { fn set_acked_features(&mut self, _: u64) {} - fn device_type(&self) -> u32 { - 0 - } - fn queues(&self) -> &[Queue] { &self.queues } diff --git a/src/vmm/src/device_manager/mod.rs b/src/vmm/src/device_manager/mod.rs index c7f6acabfe1..4f228ad3309 100644 --- a/src/vmm/src/device_manager/mod.rs +++ b/src/vmm/src/device_manager/mod.rs @@ -89,7 +89,7 @@ pub enum FindDeviceError { /// Device not found DeviceNotFound, /// Internal Device error: {0} - InternalDeviceError(String), + InternalDeviceError(anyhow::Error), } #[derive(Debug)] @@ -355,27 +355,35 @@ impl DeviceManager { } /// Run fn `f()` for the virtio device matching `virtio_type` and `id`. - pub fn with_virtio_device_with_id( + pub fn try_with_virtio_device_with_id( &self, - virtio_type: u32, id: &str, f: F, - ) -> Result<(), FindDeviceError> + ) -> Result where T: VirtioDevice + 'static + Debug, - F: FnOnce(&mut T) -> Result<(), String>, + E: std::error::Error + 'static + Send + Sync, + F: FnOnce(&mut T) -> Result, { - if let Some(device) = self.get_virtio_device(virtio_type, id) { + if let Some(device) = self.get_virtio_device(T::const_device_type(), id) { let mut dev = device.lock().expect("Poisoned lock"); f(dev .as_mut_any() .downcast_mut::() .ok_or(FindDeviceError::InvalidDeviceType)?) - .map_err(FindDeviceError::InternalDeviceError)?; + .map_err(|e| FindDeviceError::InternalDeviceError(e.into())) } else { - return Err(FindDeviceError::DeviceNotFound); + Err(FindDeviceError::DeviceNotFound) } - Ok(()) + } + + /// Run fn `f()` for the virtio device matching `virtio_type` and `id`. + pub fn with_virtio_device_with_id(&self, id: &str, f: F) -> Result + where + T: VirtioDevice + 'static + Debug, + F: FnOnce(&mut T) -> R, + { + self.try_with_virtio_device_with_id(id, |dev: &mut T| Ok::(f(dev))) } } diff --git a/src/vmm/src/device_manager/pci_mngr.rs b/src/vmm/src/device_manager/pci_mngr.rs index 578d521162b..8ebf3250581 100644 --- a/src/vmm/src/device_manager/pci_mngr.rs +++ b/src/vmm/src/device_manager/pci_mngr.rs @@ -19,6 +19,7 @@ use crate::devices::virtio::balloon::persist::{BalloonConstructorArgs, BalloonSt use crate::devices::virtio::block::device::Block; use crate::devices::virtio::block::persist::{BlockConstructorArgs, BlockState}; use crate::devices::virtio::device::VirtioDevice; +use crate::devices::virtio::generated::virtio_ids; use crate::devices::virtio::net::Net; use crate::devices::virtio::net::persist::{NetConstructorArgs, NetState}; use crate::devices::virtio::rng::Entropy; @@ -29,8 +30,7 @@ use crate::devices::virtio::transport::pci::device::{ use crate::devices::virtio::vsock::persist::{ VsockConstructorArgs, VsockState, VsockUdsConstructorArgs, }; -use crate::devices::virtio::vsock::{TYPE_VSOCK, Vsock, VsockUnixBackend}; -use crate::devices::virtio::{TYPE_BALLOON, TYPE_BLOCK, TYPE_NET, TYPE_RNG}; +use crate::devices::virtio::vsock::{Vsock, VsockUnixBackend}; use crate::resources::VmResources; use crate::snapshot::Persist; use crate::vmm_config::mmds::MmdsConfigError; @@ -285,7 +285,7 @@ impl<'a> Persist<'a> for PciDevices { let pci_device_bdf = transport_state.pci_device_bdf.into(); match locked_virtio_dev.device_type() { - TYPE_BALLOON => { + virtio_ids::VIRTIO_ID_BALLOON => { let balloon_device = locked_virtio_dev .as_any() .downcast_ref::() @@ -300,7 +300,7 @@ impl<'a> Persist<'a> for PciDevices { transport_state, }); } - TYPE_BLOCK => { + virtio_ids::VIRTIO_ID_BLOCK => { let block_dev = locked_virtio_dev .as_mut_any() .downcast_mut::() @@ -321,7 +321,7 @@ impl<'a> Persist<'a> for PciDevices { }); } } - TYPE_NET => { + virtio_ids::VIRTIO_ID_NET => { let net_dev = locked_virtio_dev .as_mut_any() .downcast_mut::() @@ -343,7 +343,7 @@ impl<'a> Persist<'a> for PciDevices { transport_state, }) } - TYPE_VSOCK => { + virtio_ids::VIRTIO_ID_VSOCK => { let vsock_dev = locked_virtio_dev .as_mut_any() // Currently, VsockUnixBackend is the only implementation of VsockBackend. @@ -374,7 +374,7 @@ impl<'a> Persist<'a> for PciDevices { transport_state, }); } - TYPE_RNG => { + virtio_ids::VIRTIO_ID_RNG => { let rng_dev = locked_virtio_dev .as_mut_any() .downcast_mut::() diff --git a/src/vmm/src/device_manager/persist.rs b/src/vmm/src/device_manager/persist.rs index 74e71f3a6bf..0a074dc9fa1 100644 --- a/src/vmm/src/device_manager/persist.rs +++ b/src/vmm/src/device_manager/persist.rs @@ -19,12 +19,14 @@ use crate::devices::acpi::vmgenid::{VMGenIDState, VMGenIdConstructorArgs, VmGenI use crate::devices::legacy::serial::SerialOut; #[cfg(target_arch = "aarch64")] use crate::devices::legacy::{RTCDevice, SerialDevice}; +use crate::devices::virtio::ActivateError; use crate::devices::virtio::balloon::persist::{BalloonConstructorArgs, BalloonState}; use crate::devices::virtio::balloon::{Balloon, BalloonError}; use crate::devices::virtio::block::BlockError; use crate::devices::virtio::block::device::Block; use crate::devices::virtio::block::persist::{BlockConstructorArgs, BlockState}; use crate::devices::virtio::device::VirtioDevice; +use crate::devices::virtio::generated::virtio_ids; use crate::devices::virtio::net::Net; use crate::devices::virtio::net::persist::{ NetConstructorArgs, NetPersistError as NetError, NetState, @@ -38,10 +40,7 @@ use crate::devices::virtio::transport::mmio::{IrqTrigger, MmioTransport}; use crate::devices::virtio::vsock::persist::{ VsockConstructorArgs, VsockState, VsockUdsConstructorArgs, }; -use crate::devices::virtio::vsock::{ - TYPE_VSOCK, Vsock, VsockError, VsockUnixBackend, VsockUnixBackendError, -}; -use crate::devices::virtio::{ActivateError, TYPE_BALLOON, TYPE_BLOCK, TYPE_NET, TYPE_RNG}; +use crate::devices::virtio::vsock::{Vsock, VsockError, VsockUnixBackend, VsockUnixBackendError}; use crate::mmds::data_store::MmdsVersion; use crate::resources::{ResourcesError, VmResources}; use crate::snapshot::Persist; @@ -244,7 +243,7 @@ impl<'a> Persist<'a> for MMIODeviceManager { let mut locked_device = mmio_transport_locked.locked_device(); match locked_device.device_type() { - TYPE_BALLOON => { + virtio_ids::VIRTIO_ID_BALLOON => { let device_state = locked_device .as_any() .downcast_ref::() @@ -258,7 +257,7 @@ impl<'a> Persist<'a> for MMIODeviceManager { }); } // Both virtio-block and vhost-user-block share same device type. - TYPE_BLOCK => { + virtio_ids::VIRTIO_ID_BLOCK => { let block = locked_device.as_mut_any().downcast_mut::().unwrap(); if block.is_vhost_user() { warn!( @@ -276,7 +275,7 @@ impl<'a> Persist<'a> for MMIODeviceManager { }); } } - TYPE_NET => { + virtio_ids::VIRTIO_ID_NET => { let net = locked_device.as_mut_any().downcast_mut::().unwrap(); if let (Some(mmds_ns), None) = (net.mmds_ns.as_ref(), states.mmds.as_ref()) { let mmds_guard = mmds_ns.mmds.lock().expect("Poisoned lock"); @@ -295,7 +294,7 @@ impl<'a> Persist<'a> for MMIODeviceManager { device_info, }); } - TYPE_VSOCK => { + virtio_ids::VIRTIO_ID_VSOCK => { let vsock = locked_device .as_mut_any() // Currently, VsockUnixBackend is the only implementation of VsockBackend. @@ -324,7 +323,7 @@ impl<'a> Persist<'a> for MMIODeviceManager { device_info, }); } - TYPE_RNG => { + virtio_ids::VIRTIO_ID_RNG => { let entropy = locked_device .as_mut_any() .downcast_mut::() diff --git a/src/vmm/src/devices/virtio/balloon/device.rs b/src/vmm/src/devices/virtio/balloon/device.rs index 4586592182c..87a82c4fa9d 100644 --- a/src/vmm/src/devices/virtio/balloon/device.rs +++ b/src/vmm/src/devices/virtio/balloon/device.rs @@ -10,9 +10,9 @@ use serde::Serialize; use timerfd::{ClockId, SetTimeFlags, TimerFd, TimerState}; use vmm_sys_util::eventfd::EventFd; +use super::super::ActivateError; use super::super::device::{DeviceState, VirtioDevice}; use super::super::queue::Queue; -use super::super::{ActivateError, TYPE_BALLOON}; use super::metrics::METRICS; use super::util::{compact_page_frame_numbers, remove_range}; use super::{ @@ -27,11 +27,13 @@ use super::{ use crate::devices::virtio::balloon::BalloonError; use crate::devices::virtio::device::ActiveState; use crate::devices::virtio::generated::virtio_config::VIRTIO_F_VERSION_1; +use crate::devices::virtio::generated::virtio_ids::VIRTIO_ID_BALLOON; use crate::devices::virtio::queue::InvalidAvailIdx; use crate::devices::virtio::transport::{VirtioInterrupt, VirtioInterruptType}; use crate::logger::IncMetric; use crate::utils::u64_to_usize; use crate::vstate::memory::{Address, ByteValued, Bytes, GuestAddress, GuestMemoryMmap}; +use crate::{impl_device_type, mem_size_mib}; const SIZE_OF_U32: usize = std::mem::size_of::(); const SIZE_OF_STAT: usize = std::mem::size_of::(); @@ -39,7 +41,9 @@ const SIZE_OF_STAT: usize = std::mem::size_of::(); fn mib_to_pages(amount_mib: u32) -> Result { amount_mib .checked_mul(MIB_TO_4K_PAGES) - .ok_or(BalloonError::TooManyPagesRequested) + .ok_or(BalloonError::TooMuchMemoryRequested( + u32::MAX / MIB_TO_4K_PAGES, + )) } fn pages_to_mib(amount_pages: u32) -> u32 { @@ -80,7 +84,7 @@ pub struct BalloonConfig { } /// BalloonStats holds statistics returned from the stats_queue. -#[derive(Clone, Default, Debug, PartialEq, Eq, Serialize)] +#[derive(Clone, Copy, Default, Debug, PartialEq, Eq, Serialize)] #[serde(deny_unknown_fields)] pub struct BalloonStats { /// The target size of the balloon, in 4K pages. @@ -449,6 +453,12 @@ impl Balloon { /// Update the target size of the balloon. pub fn update_size(&mut self, amount_mib: u32) -> Result<(), BalloonError> { if self.is_activated() { + let mem = &self.device_state.active_state().unwrap().mem; + // The balloon cannot have a target size greater than the size of + // the guest memory. + if u64::from(amount_mib) > mem_size_mib(mem) { + return Err(BalloonError::TooMuchMemoryRequested(amount_mib)); + } self.config_space.num_pages = mib_to_pages(amount_mib)?; self.interrupt_trigger() .trigger(VirtioInterruptType::Config) @@ -503,15 +513,15 @@ impl Balloon { } /// Retrieve latest stats for the balloon device. - pub fn latest_stats(&mut self) -> Option<&BalloonStats> { + pub fn latest_stats(&mut self) -> Result { if self.stats_enabled() { self.latest_stats.target_pages = self.config_space.num_pages; self.latest_stats.actual_pages = self.config_space.actual_pages; self.latest_stats.target_mib = pages_to_mib(self.latest_stats.target_pages); self.latest_stats.actual_mib = pages_to_mib(self.latest_stats.actual_pages); - Some(&self.latest_stats) + Ok(self.latest_stats) } else { - None + Err(BalloonError::StatisticsDisabled) } } @@ -534,6 +544,8 @@ impl Balloon { } impl VirtioDevice for Balloon { + impl_device_type!(VIRTIO_ID_BALLOON); + fn avail_features(&self) -> u64 { self.avail_features } @@ -546,10 +558,6 @@ impl VirtioDevice for Balloon { self.acked_features = acked_features; } - fn device_type(&self) -> u32 { - TYPE_BALLOON - } - fn queues(&self) -> &[Queue] { &self.queues } @@ -732,7 +740,7 @@ pub(crate) mod tests { for deflate_on_oom in [true, false].iter() { for stats_interval in [0, 1].iter() { let mut balloon = Balloon::new(0, *deflate_on_oom, *stats_interval, false).unwrap(); - assert_eq!(balloon.device_type(), TYPE_BALLOON); + assert_eq!(balloon.device_type(), VIRTIO_ID_BALLOON); let features: u64 = (1u64 << VIRTIO_F_VERSION_1) | (u64::from(*deflate_on_oom) << VIRTIO_BALLOON_F_DEFLATE_ON_OOM) @@ -1072,7 +1080,7 @@ pub(crate) mod tests { free_memory: Some(0x5678), ..BalloonStats::default() }; - assert_eq!(stats, &expected_stats); + assert_eq!(stats, expected_stats); // Wait for the timer to expire, although as it is non-blocking // we could just process the timer event and it would not @@ -1149,7 +1157,7 @@ pub(crate) mod tests { let mut balloon = Balloon::new(0, true, 0, false).unwrap(); // Switch the state to active. balloon.device_state = DeviceState::Activated(ActiveState { - mem: single_region_mem(0x1), + mem: single_region_mem(32 << 20), interrupt: default_interrupt(), }); diff --git a/src/vmm/src/devices/virtio/balloon/mod.rs b/src/vmm/src/devices/virtio/balloon/mod.rs index 3f3e9346545..91de01a8393 100644 --- a/src/vmm/src/devices/virtio/balloon/mod.rs +++ b/src/vmm/src/devices/virtio/balloon/mod.rs @@ -67,8 +67,6 @@ const VIRTIO_BALLOON_S_HTLB_PGFAIL: u16 = 9; /// Balloon device related errors. #[derive(Debug, thiserror::Error, displaydoc::Display)] pub enum BalloonError { - /// No balloon device found. - DeviceNotFound, /// Device not activated yet. DeviceNotActive, /// EventFd error: {0} @@ -85,8 +83,8 @@ pub enum BalloonError { StatisticsDisabled, /// Statistics cannot be enabled/disabled after activation. StatisticsStateChange, - /// Amount of pages requested cannot fit in `u32`. - TooManyPagesRequested, + /// Requested memory should be less than {0}MiB + TooMuchMemoryRequested(u32), /// Error while processing the virt queues: {0} Queue(#[from] QueueError), /// {0} diff --git a/src/vmm/src/devices/virtio/balloon/persist.rs b/src/vmm/src/devices/virtio/balloon/persist.rs index 15ae1e26b9e..0356ca08557 100644 --- a/src/vmm/src/devices/virtio/balloon/persist.rs +++ b/src/vmm/src/devices/virtio/balloon/persist.rs @@ -10,9 +10,9 @@ use serde::{Deserialize, Serialize}; use timerfd::{SetTimeFlags, TimerState}; use super::*; -use crate::devices::virtio::TYPE_BALLOON; use crate::devices::virtio::balloon::device::{BalloonStats, ConfigSpace}; use crate::devices::virtio::device::{ActiveState, DeviceState}; +use crate::devices::virtio::generated::virtio_ids::VIRTIO_ID_BALLOON; use crate::devices::virtio::persist::VirtioDeviceState; use crate::devices::virtio::queue::FIRECRACKER_MAX_QUEUE_SIZE; use crate::devices::virtio::transport::VirtioInterrupt; @@ -139,7 +139,7 @@ impl Persist<'_> for Balloon { .virtio_state .build_queues_checked( &constructor_args.mem, - TYPE_BALLOON, + VIRTIO_ID_BALLOON, num_queues, FIRECRACKER_MAX_QUEUE_SIZE, ) @@ -174,7 +174,6 @@ impl Persist<'_> for Balloon { mod tests { use super::*; - use crate::devices::virtio::TYPE_BALLOON; use crate::devices::virtio::device::VirtioDevice; use crate::devices::virtio::test_utils::{default_interrupt, default_mem}; use crate::snapshot::Snapshot; @@ -199,7 +198,7 @@ mod tests { ) .unwrap(); - assert_eq!(restored_balloon.device_type(), TYPE_BALLOON); + assert_eq!(restored_balloon.device_type(), VIRTIO_ID_BALLOON); assert!(restored_balloon.restored_from_file); assert_eq!(restored_balloon.acked_features, balloon.acked_features); diff --git a/src/vmm/src/devices/virtio/block/device.rs b/src/vmm/src/devices/virtio/block/device.rs index c1fa95f7b1c..93eb56e7760 100644 --- a/src/vmm/src/devices/virtio/block/device.rs +++ b/src/vmm/src/devices/virtio/block/device.rs @@ -11,10 +11,12 @@ use super::BlockError; use super::persist::{BlockConstructorArgs, BlockState}; use super::vhost_user::device::{VhostUserBlock, VhostUserBlockConfig}; use super::virtio::device::{VirtioBlock, VirtioBlockConfig}; +use crate::devices::virtio::ActivateError; use crate::devices::virtio::device::VirtioDevice; +use crate::devices::virtio::generated::virtio_ids::VIRTIO_ID_BLOCK; use crate::devices::virtio::queue::{InvalidAvailIdx, Queue}; use crate::devices::virtio::transport::VirtioInterrupt; -use crate::devices::virtio::{ActivateError, TYPE_BLOCK}; +use crate::impl_device_type; use crate::rate_limiter::BucketUpdate; use crate::snapshot::Persist; use crate::vmm_config::drive::BlockDeviceConfig; @@ -131,6 +133,8 @@ impl Block { } impl VirtioDevice for Block { + impl_device_type!(VIRTIO_ID_BLOCK); + fn avail_features(&self) -> u64 { match self { Self::Virtio(b) => b.avail_features, @@ -152,10 +156,6 @@ impl VirtioDevice for Block { } } - fn device_type(&self) -> u32 { - TYPE_BLOCK - } - fn queues(&self) -> &[Queue] { match self { Self::Virtio(b) => &b.queues, diff --git a/src/vmm/src/devices/virtio/block/vhost_user/device.rs b/src/vmm/src/devices/virtio/block/vhost_user/device.rs index 1d6c2aac080..dd08b8de7c8 100644 --- a/src/vmm/src/devices/virtio/block/vhost_user/device.rs +++ b/src/vmm/src/devices/virtio/block/vhost_user/device.rs @@ -14,10 +14,12 @@ use vhost::vhost_user::message::*; use vmm_sys_util::eventfd::EventFd; use super::{NUM_QUEUES, QUEUE_SIZE, VhostUserBlockError}; +use crate::devices::virtio::ActivateError; use crate::devices::virtio::block::CacheType; use crate::devices::virtio::device::{ActiveState, DeviceState, VirtioDevice}; use crate::devices::virtio::generated::virtio_blk::{VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_RO}; use crate::devices::virtio::generated::virtio_config::VIRTIO_F_VERSION_1; +use crate::devices::virtio::generated::virtio_ids::VIRTIO_ID_BLOCK; use crate::devices::virtio::generated::virtio_ring::VIRTIO_RING_F_EVENT_IDX; use crate::devices::virtio::queue::Queue; use crate::devices::virtio::transport::{VirtioInterrupt, VirtioInterruptType}; @@ -25,7 +27,7 @@ use crate::devices::virtio::vhost_user::{VhostUserHandleBackend, VhostUserHandle use crate::devices::virtio::vhost_user_metrics::{ VhostUserDeviceMetrics, VhostUserMetricsPerDevice, }; -use crate::devices::virtio::{ActivateError, TYPE_BLOCK}; +use crate::impl_device_type; use crate::logger::{IncMetric, StoreMetric, log_dev_preview_warning}; use crate::utils::u64_to_usize; use crate::vmm_config::drive::BlockDeviceConfig; @@ -286,6 +288,8 @@ impl VhostUserBlockImpl { } impl VirtioDevice for VhostUserBlockImpl { + impl_device_type!(VIRTIO_ID_BLOCK); + fn avail_features(&self) -> u64 { self.avail_features } @@ -298,10 +302,6 @@ impl VirtioDevice for VhostUserBlock self.acked_features = acked_features; } - fn device_type(&self) -> u32 { - TYPE_BLOCK - } - fn queues(&self) -> &[Queue] { &self.queues } diff --git a/src/vmm/src/devices/virtio/block/virtio/device.rs b/src/vmm/src/devices/virtio/block/virtio/device.rs index d04fd5674ea..62ebdbbf5fd 100644 --- a/src/vmm/src/devices/virtio/block/virtio/device.rs +++ b/src/vmm/src/devices/virtio/block/virtio/device.rs @@ -22,6 +22,7 @@ use vmm_sys_util::eventfd::EventFd; use super::io::async_io; use super::request::*; use super::{BLOCK_QUEUE_SIZES, SECTOR_SHIFT, SECTOR_SIZE, VirtioBlockError, io as block_io}; +use crate::devices::virtio::ActivateError; use crate::devices::virtio::block::CacheType; use crate::devices::virtio::block::virtio::metrics::{BlockDeviceMetrics, BlockMetricsPerDevice}; use crate::devices::virtio::device::{ActiveState, DeviceState, VirtioDevice}; @@ -29,10 +30,11 @@ use crate::devices::virtio::generated::virtio_blk::{ VIRTIO_BLK_F_FLUSH, VIRTIO_BLK_F_RO, VIRTIO_BLK_ID_BYTES, }; use crate::devices::virtio::generated::virtio_config::VIRTIO_F_VERSION_1; +use crate::devices::virtio::generated::virtio_ids::VIRTIO_ID_BLOCK; use crate::devices::virtio::generated::virtio_ring::VIRTIO_RING_F_EVENT_IDX; use crate::devices::virtio::queue::{InvalidAvailIdx, Queue}; use crate::devices::virtio::transport::{VirtioInterrupt, VirtioInterruptType}; -use crate::devices::virtio::{ActivateError, TYPE_BLOCK}; +use crate::impl_device_type; use crate::logger::{IncMetric, error, warn}; use crate::rate_limiter::{BucketUpdate, RateLimiter}; use crate::utils::u64_to_usize; @@ -581,6 +583,8 @@ impl VirtioBlock { } impl VirtioDevice for VirtioBlock { + impl_device_type!(VIRTIO_ID_BLOCK); + fn avail_features(&self) -> u64 { self.avail_features } @@ -593,10 +597,6 @@ impl VirtioDevice for VirtioBlock { self.acked_features = acked_features; } - fn device_type(&self) -> u32 { - TYPE_BLOCK - } - fn queues(&self) -> &[Queue] { &self.queues } @@ -790,7 +790,7 @@ mod tests { for engine in [FileEngineType::Sync, FileEngineType::Async] { let mut block = default_block(engine); - assert_eq!(block.device_type(), TYPE_BLOCK); + assert_eq!(block.device_type(), VIRTIO_ID_BLOCK); let features: u64 = (1u64 << VIRTIO_F_VERSION_1) | (1u64 << VIRTIO_RING_F_EVENT_IDX); diff --git a/src/vmm/src/devices/virtio/block/virtio/persist.rs b/src/vmm/src/devices/virtio/block/virtio/persist.rs index 1c7a1bce106..5b544a12c19 100644 --- a/src/vmm/src/devices/virtio/block/virtio/persist.rs +++ b/src/vmm/src/devices/virtio/block/virtio/persist.rs @@ -9,12 +9,12 @@ use vmm_sys_util::eventfd::EventFd; use super::device::DiskProperties; use super::*; -use crate::devices::virtio::TYPE_BLOCK; 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::{ActiveState, DeviceState}; use crate::devices::virtio::generated::virtio_blk::VIRTIO_BLK_F_RO; +use crate::devices::virtio::generated::virtio_ids::VIRTIO_ID_BLOCK; use crate::devices::virtio::persist::VirtioDeviceState; use crate::rate_limiter::RateLimiter; use crate::rate_limiter::persist::RateLimiterState; @@ -102,7 +102,7 @@ impl Persist<'_> for VirtioBlock { .virtio_state .build_queues_checked( &constructor_args.mem, - TYPE_BLOCK, + VIRTIO_ID_BLOCK, BLOCK_NUM_QUEUES, FIRECRACKER_MAX_QUEUE_SIZE, ) @@ -224,7 +224,7 @@ mod tests { .unwrap(); // Test that virtio specific fields are the same. - assert_eq!(restored_block.device_type(), TYPE_BLOCK); + assert_eq!(restored_block.device_type(), VIRTIO_ID_BLOCK); assert_eq!(restored_block.avail_features(), block.avail_features()); assert_eq!(restored_block.acked_features(), block.acked_features()); assert_eq!(restored_block.queues(), block.queues()); diff --git a/src/vmm/src/devices/virtio/device.rs b/src/vmm/src/devices/virtio/device.rs index ca3efc8cf2f..8d98b3f0d11 100644 --- a/src/vmm/src/devices/virtio/device.rs +++ b/src/vmm/src/devices/virtio/device.rs @@ -74,7 +74,14 @@ pub trait VirtioDevice: AsAny + Send { (self.acked_features() & (1 << feature)) != 0 } + /// The virtio device type (as a constant of the struct). + fn const_device_type() -> u32 + where + Self: Sized; + /// The virtio device type. + /// + /// It should be the same as returned by Self::const_device_type(). fn device_type(&self) -> u32; /// Returns the device queues. @@ -170,6 +177,20 @@ impl fmt::Debug for dyn VirtioDevice { } } +/// Utility to define both const_device_type and device_type with a u32 constant +#[macro_export] +macro_rules! impl_device_type { + ($const_type:expr) => { + fn const_device_type() -> u32 { + $const_type + } + + fn device_type(&self) -> u32 { + Self::const_device_type() + } + }; +} + #[cfg(test)] pub(crate) mod tests { use super::*; @@ -180,6 +201,8 @@ pub(crate) mod tests { } impl VirtioDevice for MockVirtioDevice { + impl_device_type!(0); + fn avail_features(&self) -> u64 { todo!() } @@ -192,10 +215,6 @@ pub(crate) mod tests { todo!() } - fn device_type(&self) -> u32 { - todo!() - } - fn queues(&self) -> &[Queue] { todo!() } diff --git a/src/vmm/src/devices/virtio/generated/mod.rs b/src/vmm/src/devices/virtio/generated/mod.rs index ab1a73dae7d..a9d1f08f88f 100644 --- a/src/vmm/src/devices/virtio/generated/mod.rs +++ b/src/vmm/src/devices/virtio/generated/mod.rs @@ -12,5 +12,6 @@ pub mod virtio_blk; pub mod virtio_config; +pub mod virtio_ids; pub mod virtio_net; pub mod virtio_ring; diff --git a/src/vmm/src/devices/virtio/generated/virtio_ids.rs b/src/vmm/src/devices/virtio/generated/virtio_ids.rs new file mode 100644 index 00000000000..d156fe7f7b3 --- /dev/null +++ b/src/vmm/src/devices/virtio/generated/virtio_ids.rs @@ -0,0 +1,57 @@ +// Copyright 2025 Amazon.com, Inc. or its affiliates. All Rights Reserved. +// SPDX-License-Identifier: Apache-2.0 + +// automatically generated by tools/bindgen.sh + +#![allow( + non_camel_case_types, + non_upper_case_globals, + dead_code, + non_snake_case, + clippy::ptr_as_ptr, + clippy::undocumented_unsafe_blocks, + missing_debug_implementations, + clippy::tests_outside_test_module, + unsafe_op_in_unsafe_fn, + clippy::redundant_static_lifetimes +)] + +pub const VIRTIO_ID_NET: u32 = 1; +pub const VIRTIO_ID_BLOCK: u32 = 2; +pub const VIRTIO_ID_CONSOLE: u32 = 3; +pub const VIRTIO_ID_RNG: u32 = 4; +pub const VIRTIO_ID_BALLOON: u32 = 5; +pub const VIRTIO_ID_IOMEM: u32 = 6; +pub const VIRTIO_ID_RPMSG: u32 = 7; +pub const VIRTIO_ID_SCSI: u32 = 8; +pub const VIRTIO_ID_9P: u32 = 9; +pub const VIRTIO_ID_MAC80211_WLAN: u32 = 10; +pub const VIRTIO_ID_RPROC_SERIAL: u32 = 11; +pub const VIRTIO_ID_CAIF: u32 = 12; +pub const VIRTIO_ID_MEMORY_BALLOON: u32 = 13; +pub const VIRTIO_ID_GPU: u32 = 16; +pub const VIRTIO_ID_CLOCK: u32 = 17; +pub const VIRTIO_ID_INPUT: u32 = 18; +pub const VIRTIO_ID_VSOCK: u32 = 19; +pub const VIRTIO_ID_CRYPTO: u32 = 20; +pub const VIRTIO_ID_SIGNAL_DIST: u32 = 21; +pub const VIRTIO_ID_PSTORE: u32 = 22; +pub const VIRTIO_ID_IOMMU: u32 = 23; +pub const VIRTIO_ID_MEM: u32 = 24; +pub const VIRTIO_ID_SOUND: u32 = 25; +pub const VIRTIO_ID_FS: u32 = 26; +pub const VIRTIO_ID_PMEM: u32 = 27; +pub const VIRTIO_ID_RPMB: u32 = 28; +pub const VIRTIO_ID_MAC80211_HWSIM: u32 = 29; +pub const VIRTIO_ID_VIDEO_ENCODER: u32 = 30; +pub const VIRTIO_ID_VIDEO_DECODER: u32 = 31; +pub const VIRTIO_ID_SCMI: u32 = 32; +pub const VIRTIO_ID_NITRO_SEC_MOD: u32 = 33; +pub const VIRTIO_ID_I2C_ADAPTER: u32 = 34; +pub const VIRTIO_ID_WATCHDOG: u32 = 35; +pub const VIRTIO_ID_CAN: u32 = 36; +pub const VIRTIO_ID_DMABUF: u32 = 37; +pub const VIRTIO_ID_PARAM_SERV: u32 = 38; +pub const VIRTIO_ID_AUDIO_POLICY: u32 = 39; +pub const VIRTIO_ID_BT: u32 = 40; +pub const VIRTIO_ID_GPIO: u32 = 41; diff --git a/src/vmm/src/devices/virtio/mod.rs b/src/vmm/src/devices/virtio/mod.rs index 0ac3b660397..1e9e3541720 100644 --- a/src/vmm/src/devices/virtio/mod.rs +++ b/src/vmm/src/devices/virtio/mod.rs @@ -46,17 +46,6 @@ mod device_status { pub const DEVICE_NEEDS_RESET: u32 = 64; } -/// Types taken from linux/virtio_ids.h. -/// Type 0 is not used by virtio. Use it as wildcard for non-virtio devices -/// Virtio net device ID. -pub const TYPE_NET: u32 = 1; -/// Virtio block device ID. -pub const TYPE_BLOCK: u32 = 2; -/// Virtio rng device ID. -pub const TYPE_RNG: u32 = 4; -/// Virtio balloon device ID. -pub const TYPE_BALLOON: u32 = 5; - /// Offset from the base MMIO address of a virtio device used by the guest to notify the device of /// queue events. pub const NOTIFY_REG_OFFSET: u32 = 0x50; diff --git a/src/vmm/src/devices/virtio/net/device.rs b/src/vmm/src/devices/virtio/net/device.rs index 0b2f3150c09..70fe5405e22 100755 --- a/src/vmm/src/devices/virtio/net/device.rs +++ b/src/vmm/src/devices/virtio/net/device.rs @@ -17,8 +17,10 @@ use log::{error, info}; use vmm_sys_util::eventfd::EventFd; use super::NET_QUEUE_MAX_SIZE; +use crate::devices::virtio::ActivateError; use crate::devices::virtio::device::{ActiveState, DeviceState, VirtioDevice}; use crate::devices::virtio::generated::virtio_config::VIRTIO_F_VERSION_1; +use crate::devices::virtio::generated::virtio_ids::VIRTIO_ID_NET; use crate::devices::virtio::generated::virtio_net::{ VIRTIO_NET_F_CSUM, VIRTIO_NET_F_GUEST_CSUM, VIRTIO_NET_F_GUEST_TSO4, VIRTIO_NET_F_GUEST_TSO6, VIRTIO_NET_F_GUEST_UFO, VIRTIO_NET_F_HOST_TSO4, VIRTIO_NET_F_HOST_TSO6, VIRTIO_NET_F_HOST_UFO, @@ -35,10 +37,10 @@ use crate::devices::virtio::net::{ }; use crate::devices::virtio::queue::{DescriptorChain, InvalidAvailIdx, Queue}; use crate::devices::virtio::transport::{VirtioInterrupt, VirtioInterruptType}; -use crate::devices::virtio::{ActivateError, TYPE_NET}; use crate::devices::{DeviceError, report_net_event_fail}; use crate::dumbo::pdu::arp::ETH_IPV4_FRAME_LEN; use crate::dumbo::pdu::ethernet::{EthernetFrame, PAYLOAD_OFFSET}; +use crate::impl_device_type; use crate::logger::{IncMetric, METRICS}; use crate::mmds::data_store::Mmds; use crate::mmds::ns::MmdsNetworkStack; @@ -960,6 +962,8 @@ impl Net { } impl VirtioDevice for Net { + impl_device_type!(VIRTIO_ID_NET); + fn avail_features(&self) -> u64 { self.avail_features } @@ -972,10 +976,6 @@ impl VirtioDevice for Net { self.acked_features = acked_features; } - fn device_type(&self) -> u32 { - TYPE_NET - } - fn queues(&self) -> &[Queue] { &self.queues } @@ -1154,7 +1154,7 @@ pub mod tests { fn test_virtio_device_type() { let mut net = default_net(); set_mac(&mut net, MacAddr::from_str("11:22:33:44:55:66").unwrap()); - assert_eq!(net.device_type(), TYPE_NET); + assert_eq!(net.device_type(), VIRTIO_ID_NET); } #[test] diff --git a/src/vmm/src/devices/virtio/net/persist.rs b/src/vmm/src/devices/virtio/net/persist.rs index 6ef8ad842ac..c988796330d 100644 --- a/src/vmm/src/devices/virtio/net/persist.rs +++ b/src/vmm/src/devices/virtio/net/persist.rs @@ -10,8 +10,8 @@ use serde::{Deserialize, Serialize}; use super::device::{Net, RxBuffers}; use super::{NET_NUM_QUEUES, NET_QUEUE_MAX_SIZE, RX_INDEX, TapError}; -use crate::devices::virtio::TYPE_NET; use crate::devices::virtio::device::{ActiveState, DeviceState}; +use crate::devices::virtio::generated::virtio_ids::VIRTIO_ID_NET; use crate::devices::virtio::persist::{PersistError as VirtioStateError, VirtioDeviceState}; use crate::devices::virtio::transport::VirtioInterrupt; use crate::mmds::data_store::Mmds; @@ -121,7 +121,7 @@ impl Persist<'_> for Net { net.queues = state.virtio_state.build_queues_checked( &constructor_args.mem, - TYPE_NET, + VIRTIO_ID_NET, NET_NUM_QUEUES, NET_QUEUE_MAX_SIZE, )?; @@ -177,7 +177,7 @@ mod tests { ) { Ok(restored_net) => { // Test that virtio specific fields are the same. - assert_eq!(restored_net.device_type(), TYPE_NET); + assert_eq!(restored_net.device_type(), VIRTIO_ID_NET); assert_eq!(restored_net.avail_features(), virtio_state.avail_features); assert_eq!(restored_net.acked_features(), virtio_state.acked_features); assert_eq!(restored_net.is_activated(), virtio_state.activated); diff --git a/src/vmm/src/devices/virtio/rng/device.rs b/src/vmm/src/devices/virtio/rng/device.rs index 2cf1c6bf5dd..6f488fbe217 100644 --- a/src/vmm/src/devices/virtio/rng/device.rs +++ b/src/vmm/src/devices/virtio/rng/device.rs @@ -13,13 +13,15 @@ use vmm_sys_util::eventfd::EventFd; use super::metrics::METRICS; use super::{RNG_NUM_QUEUES, RNG_QUEUE}; use crate::devices::DeviceError; +use crate::devices::virtio::ActivateError; use crate::devices::virtio::device::{ActiveState, DeviceState, VirtioDevice}; use crate::devices::virtio::generated::virtio_config::VIRTIO_F_VERSION_1; +use crate::devices::virtio::generated::virtio_ids::VIRTIO_ID_RNG; use crate::devices::virtio::iov_deque::IovDequeError; use crate::devices::virtio::iovec::IoVecBufferMut; use crate::devices::virtio::queue::{FIRECRACKER_MAX_QUEUE_SIZE, InvalidAvailIdx, Queue}; use crate::devices::virtio::transport::{VirtioInterrupt, VirtioInterruptType}; -use crate::devices::virtio::{ActivateError, TYPE_RNG}; +use crate::impl_device_type; use crate::logger::{IncMetric, debug, error}; use crate::rate_limiter::{RateLimiter, TokenType}; use crate::vstate::memory::GuestMemoryMmap; @@ -252,9 +254,7 @@ impl Entropy { } impl VirtioDevice for Entropy { - fn device_type(&self) -> u32 { - TYPE_RNG - } + impl_device_type!(VIRTIO_ID_RNG); fn queues(&self) -> &[Queue] { &self.queues @@ -366,7 +366,7 @@ mod tests { #[test] fn test_device_type() { let entropy_dev = default_entropy(); - assert_eq!(entropy_dev.device_type(), TYPE_RNG); + assert_eq!(entropy_dev.device_type(), VIRTIO_ID_RNG); } #[test] diff --git a/src/vmm/src/devices/virtio/rng/persist.rs b/src/vmm/src/devices/virtio/rng/persist.rs index d266e259418..6c9e7212c7b 100644 --- a/src/vmm/src/devices/virtio/rng/persist.rs +++ b/src/vmm/src/devices/virtio/rng/persist.rs @@ -7,7 +7,7 @@ use std::sync::Arc; use serde::{Deserialize, Serialize}; -use crate::devices::virtio::TYPE_RNG; +use crate::devices::virtio::generated::virtio_ids::VIRTIO_ID_RNG; use crate::devices::virtio::persist::{PersistError as VirtioStateError, VirtioDeviceState}; use crate::devices::virtio::queue::FIRECRACKER_MAX_QUEUE_SIZE; use crate::devices::virtio::rng::{Entropy, EntropyError, RNG_NUM_QUEUES}; @@ -56,7 +56,7 @@ impl Persist<'_> for Entropy { ) -> Result { let queues = state.virtio_state.build_queues_checked( &constructor_args.mem, - TYPE_RNG, + VIRTIO_ID_RNG, RNG_NUM_QUEUES, FIRECRACKER_MAX_QUEUE_SIZE, )?; @@ -94,7 +94,7 @@ mod tests { ) .unwrap(); - assert_eq!(restored.device_type(), TYPE_RNG); + assert_eq!(restored.device_type(), VIRTIO_ID_RNG); assert_eq!(restored.id(), ENTROPY_DEV_ID); assert!(!restored.is_activated()); assert!(!entropy.is_activated()); diff --git a/src/vmm/src/devices/virtio/transport/mmio.rs b/src/vmm/src/devices/virtio/transport/mmio.rs index 4964f837aca..d6cb83b87c6 100644 --- a/src/vmm/src/devices/virtio/transport/mmio.rs +++ b/src/vmm/src/devices/virtio/transport/mmio.rs @@ -476,6 +476,7 @@ pub(crate) mod tests { use super::*; use crate::devices::virtio::ActivateError; use crate::devices::virtio::device_status::DEVICE_NEEDS_RESET; + use crate::impl_device_type; use crate::test_utils::single_region_mem; use crate::utils::byte_order::{read_le_u32, write_le_u32}; use crate::utils::u64_to_usize; @@ -510,12 +511,14 @@ pub(crate) mod tests { } } - fn set_avail_features(&mut self, avail_features: u64) { + pub fn set_avail_features(&mut self, avail_features: u64) { self.avail_features = avail_features; } } impl VirtioDevice for DummyDevice { + impl_device_type!(123); + fn avail_features(&self) -> u64 { self.avail_features } @@ -528,10 +531,6 @@ pub(crate) mod tests { self.acked_features = acked_features; } - fn device_type(&self) -> u32 { - 123 - } - fn queues(&self) -> &[Queue] { &self.queues } diff --git a/src/vmm/src/devices/virtio/transport/pci/common_config.rs b/src/vmm/src/devices/virtio/transport/pci/common_config.rs index d353b04c43e..70876d7aefc 100644 --- a/src/vmm/src/devices/virtio/transport/pci/common_config.rs +++ b/src/vmm/src/devices/virtio/transport/pci/common_config.rs @@ -17,7 +17,8 @@ use vm_memory::GuestAddress; use crate::devices::virtio::device::VirtioDevice; use crate::devices::virtio::queue::Queue; use crate::devices::virtio::transport::pci::device::VIRTQ_MSI_NO_VECTOR; -use crate::logger::{debug, error, info, trace, warn}; +use crate::logger::warn; + pub const VIRTIO_PCI_COMMON_CONFIG_ID: &str = "virtio_pci_common_config"; #[derive(Debug, Clone, Serialize, Deserialize)] @@ -31,72 +32,6 @@ pub struct VirtioPciCommonConfigState { pub msix_queues: Vec, } -// The standard layout for the ring is a continuous chunk of memory which looks -// like this. We assume num is a power of 2. -// -// struct vring -// { -// // The actual descriptors (16 bytes each) -// struct vring_desc desc[num]; -// -// // A ring of available descriptor heads with free-running index. -// __virtio16 avail_flags; -// __virtio16 avail_idx; -// __virtio16 available[num]; -// __virtio16 used_event_idx; -// -// // Padding to the next align boundary. -// char pad[]; -// -// // A ring of used descriptor heads with free-running index. -// __virtio16 used_flags; -// __virtio16 used_idx; -// struct vring_used_elem used[num]; -// __virtio16 avail_event_idx; -// }; -// struct vring_desc { -// __virtio64 addr; -// __virtio32 len; -// __virtio16 flags; -// __virtio16 next; -// }; -// -// struct vring_avail { -// __virtio16 flags; -// __virtio16 idx; -// __virtio16 ring[]; -// }; -// -// // u32 is used here for ids for padding reasons. -// struct vring_used_elem { -// // Index of start of used descriptor chain. -// __virtio32 id; -// // Total length of the descriptor chain which was used (written to) -// __virtio32 len; -// }; -// -// Kernel header used for this reference: include/uapi/linux/virtio_ring.h -// Virtio Spec: https://docs.oasis-open.org/virtio/virtio/v1.2/csd01/virtio-v1.2-csd01.html -// -const VRING_DESC_ELEMENT_SIZE: usize = 16; -const VRING_AVAIL_ELEMENT_SIZE: usize = 2; -const VRING_USED_ELEMENT_SIZE: usize = 8; -#[derive(Debug)] -pub enum VringType { - Desc, - Avail, - Used, -} - -pub fn get_vring_size(t: VringType, queue_size: u16) -> u64 { - let (length_except_ring, element_size) = match t { - VringType::Desc => (0, VRING_DESC_ELEMENT_SIZE), - VringType::Avail => (6, VRING_AVAIL_ELEMENT_SIZE), - VringType::Used => (6, VRING_USED_ELEMENT_SIZE), - }; - (length_except_ring + element_size * queue_size as usize) as u64 -} - /// Contains the data for reading and writing the common configuration structure of a virtio PCI /// device. /// @@ -173,11 +108,10 @@ impl VirtioPciCommonConfig { let v = self.read_common_config_dword(offset, device); LittleEndian::write_u32(data, v); } - 8 => { - let v = self.read_common_config_qword(offset); - LittleEndian::write_u64(data, v); - } - _ => error!("invalid data length for virtio read: len {}", data.len()), + _ => warn!( + "pci: invalid data length for virtio read: len {}", + data.len() + ), } } @@ -192,40 +126,35 @@ impl VirtioPciCommonConfig { device.lock().unwrap().queues_mut(), ), 4 => self.write_common_config_dword(offset, LittleEndian::read_u32(data), device), - 8 => self.write_common_config_qword( - offset, - LittleEndian::read_u64(data), - device.lock().unwrap().queues_mut(), + _ => warn!( + "pci: invalid data length for virtio write: len {}", + data.len() ), - _ => error!("invalid data length for virtio write: len {}", data.len()), } } fn read_common_config_byte(&self, offset: u64) -> u8 { - debug!("read_common_config_byte: offset 0x{:x}", offset); // The driver is only allowed to do aligned, properly sized access. match offset { 0x14 => self.driver_status, 0x15 => self.config_generation, _ => { - warn!("invalid virtio config byte read: 0x{:x}", offset); + warn!("pci: invalid virtio config byte read: 0x{:x}", offset); 0 } } } fn write_common_config_byte(&mut self, offset: u64, value: u8) { - debug!("write_common_config_byte: offset 0x{offset:x}: {value:x}"); match offset { 0x14 => self.driver_status = value, _ => { - warn!("invalid virtio config byte write: 0x{:x}", offset); + warn!("pci: invalid virtio config byte write: 0x{:x}", offset); } } } fn read_common_config_word(&self, offset: u64, queues: &[Queue]) -> u16 { - debug!("read_common_config_word: offset 0x{:x}", offset); match offset { 0x10 => self.msix_config.load(Ordering::Acquire), 0x12 => queues.len().try_into().unwrap(), // num_queues @@ -247,14 +176,13 @@ impl VirtioPciCommonConfig { 0x1c => u16::from(self.with_queue(queues, |q| q.ready).unwrap_or(false)), 0x1e => self.queue_select, // notify_off _ => { - warn!("invalid virtio register word read: 0x{:x}", offset); + warn!("pci: invalid virtio register word read: 0x{:x}", offset); 0 } } } fn write_common_config_word(&mut self, offset: u64, value: u16, queues: &mut [Queue]) { - debug!("write_common_config_word: offset 0x{:x}", offset); match offset { 0x10 => { // Make sure that the guest doesn't select an invalid vector. We are offering @@ -289,16 +217,17 @@ impl VirtioPciCommonConfig { } } 0x1c => self.with_queue_mut(queues, |q| { - q.ready = value == 1; + if value != 0 { + q.ready = value == 1; + } }), _ => { - warn!("invalid virtio register word write: 0x{:x}", offset); + warn!("pci: invalid virtio register word write: 0x{:x}", offset); } } } fn read_common_config_dword(&self, offset: u64, device: Arc>) -> u32 { - debug!("read_common_config_dword: offset 0x{:x}", offset); match offset { 0x00 => self.device_feature_select, 0x04 => { @@ -313,8 +242,50 @@ impl VirtioPciCommonConfig { } } 0x08 => self.driver_feature_select, + 0x20 => { + let locked_device = device.lock().unwrap(); + self.with_queue(locked_device.queues(), |q| { + (q.desc_table_address.0 & 0xffff_ffff) as u32 + }) + .unwrap_or_default() + } + 0x24 => { + let locked_device = device.lock().unwrap(); + self.with_queue(locked_device.queues(), |q| { + (q.desc_table_address.0 >> 32) as u32 + }) + .unwrap_or_default() + } + 0x28 => { + let locked_device = device.lock().unwrap(); + self.with_queue(locked_device.queues(), |q| { + (q.avail_ring_address.0 & 0xffff_ffff) as u32 + }) + .unwrap_or_default() + } + 0x2c => { + let locked_device = device.lock().unwrap(); + self.with_queue(locked_device.queues(), |q| { + (q.avail_ring_address.0 >> 32) as u32 + }) + .unwrap_or_default() + } + 0x30 => { + let locked_device = device.lock().unwrap(); + self.with_queue(locked_device.queues(), |q| { + (q.used_ring_address.0 & 0xffff_ffff) as u32 + }) + .unwrap_or_default() + } + 0x34 => { + let locked_device = device.lock().unwrap(); + self.with_queue(locked_device.queues(), |q| { + (q.used_ring_address.0 >> 32) as u32 + }) + .unwrap_or_default() + } _ => { - warn!("invalid virtio register dword read: 0x{:x}", offset); + warn!("pci: invalid virtio register dword read: 0x{:x}", offset); 0 } } @@ -326,7 +297,6 @@ impl VirtioPciCommonConfig { value: u32, device: Arc>, ) { - debug!("write_common_config_dword: offset 0x{:x}", offset); fn hi(v: &mut GuestAddress, x: u32) { *v = (*v & 0xffff_ffff) | (u64::from(x) << 32) } @@ -360,28 +330,7 @@ impl VirtioPciCommonConfig { hi(&mut q.used_ring_address, value) }), _ => { - warn!("invalid virtio register dword write: 0x{:x}", offset); - } - } - } - - fn read_common_config_qword(&self, _offset: u64) -> u64 { - debug!("read_common_config_qword: offset 0x{:x}", _offset); - 0 // Assume the guest has no reason to read write-only registers. - } - - fn write_common_config_qword(&mut self, offset: u64, value: u64, queues: &mut [Queue]) { - debug!("write_common_config_qword: offset 0x{:x}", offset); - - let low = Some((value & 0xffff_ffff) as u32); - let high = Some((value >> 32) as u32); - - match offset { - 0x20 => self.with_queue_mut(queues, |q| q.desc_table_address.0 = value), - 0x28 => self.with_queue_mut(queues, |q| q.avail_ring_address.0 = value), - 0x30 => self.with_queue_mut(queues, |q| q.used_ring_address.0 = value), - _ => { - warn!("invalid virtio register qword write: 0x{:x}", offset); + warn!("pci: invalid virtio register dword write: 0x{:x}", offset); } } } @@ -402,9 +351,27 @@ impl VirtioPciCommonConfig { #[cfg(test)] mod tests { + use vm_memory::ByteValued; + use super::*; use crate::devices::virtio::transport::mmio::tests::DummyDevice; + fn default_device() -> Arc> { + Arc::new(Mutex::new(DummyDevice::new())) + } + + fn default_pci_common_config() -> VirtioPciCommonConfig { + VirtioPciCommonConfig { + driver_status: 0, + config_generation: 0, + device_feature_select: 0, + driver_feature_select: 0, + queue_select: 0, + msix_config: Arc::new(AtomicU16::new(0)), + msix_queues: Arc::new(Mutex::new(vec![0u16; 2])), + } + } + #[test] fn write_base_regs() { let mut regs = VirtioPciCommonConfig { @@ -468,4 +435,313 @@ mod tests { regs.read(0x1a, &mut read_back, dev); assert_eq!(LittleEndian::read_u16(&read_back[..2]), 0x1); } + + #[test] + fn test_device_feature() { + let mut config = default_pci_common_config(); + let mut device = default_device(); + let mut features = 0u32; + + device + .lock() + .unwrap() + .set_avail_features(0x0000_1312_0000_1110); + + config.read(0x04, features.as_mut_slice(), device.clone()); + assert_eq!(features, 0x1110); + // select second page + config.write(0x0, 1u32.as_slice(), device.clone()); + config.read(0x04, features.as_mut_slice(), device.clone()); + assert_eq!(features, 0x1312); + // Try a third page. It doesn't exist so we should get all 0s + config.write(0x0, 2u32.as_slice(), device.clone()); + config.read(0x04, features.as_mut_slice(), device.clone()); + assert_eq!(features, 0x0); + } + + #[test] + fn test_driver_feature() { + let mut config = default_pci_common_config(); + let mut device = default_device(); + device + .lock() + .unwrap() + .set_avail_features(0x0000_1312_0000_1110); + + // ACK some features of the first page + config.write(0x0c, 0x1100u32.as_slice(), device.clone()); + assert_eq!(device.lock().unwrap().acked_features(), 0x1100); + // ACK some features of the second page + config.write(0x08, 1u32.as_slice(), device.clone()); + config.write(0x0c, 0x0000_1310u32.as_slice(), device.clone()); + assert_eq!( + device.lock().unwrap().acked_features(), + 0x0000_1310_0000_1100 + ); + } + + #[test] + fn test_num_queues() { + let mut config = default_pci_common_config(); + let mut device = default_device(); + let mut num_queues = 0u16; + + config.read(0x12, num_queues.as_mut_slice(), device.clone()); + assert_eq!(num_queues, 2); + // `num_queues` is read-only + config.write(0x12, 4u16.as_slice(), device.clone()); + config.read(0x12, num_queues.as_mut_slice(), device.clone()); + assert_eq!(num_queues, 2); + } + + #[test] + fn test_device_status() { + let mut config = default_pci_common_config(); + let mut device = default_device(); + let mut status = 0u8; + + config.read(0x14, status.as_mut_slice(), device.clone()); + assert_eq!(status, 0); + config.write(0x14, 0x42u8.as_slice(), device.clone()); + config.read(0x14, status.as_mut_slice(), device.clone()); + assert_eq!(status, 0x42); + } + + #[test] + fn test_config_msix_vector() { + let mut config = default_pci_common_config(); + let device = default_device(); + let mut vector: u16 = 0; + + // Our device has 2 queues, so we should be using 3 vectors in total. + // Trying to set a vector bigger than that should fail. Observing the + // failure happens through a subsequent read that should return NO_VECTOR. + config.write(0x10, 3u16.as_slice(), device.clone()); + config.read(0x10, vector.as_mut_slice(), device.clone()); + assert_eq!(vector, VIRTQ_MSI_NO_VECTOR); + + // Any of the 3 valid values should work + for i in 0u16..3 { + config.write(0x10, i.as_slice(), device.clone()); + config.read(0x10, vector.as_mut_slice(), device.clone()); + assert_eq!(vector, i); + } + } + + #[test] + fn test_queue_size() { + let mut config = default_pci_common_config(); + let device = default_device(); + let mut len = 0u16; + let mut max_size = [0u16; 2]; + + for queue_id in 0u16..2 { + config.write(0x16, queue_id.as_slice(), device.clone()); + config.read(0x18, len.as_mut_slice(), device.clone()); + assert_eq!( + len, + device.lock().unwrap().queues()[queue_id as usize].max_size + ); + max_size[queue_id as usize] = len; + } + + config.write(0x16, 2u16.as_slice(), device.clone()); + config.read(0x18, len.as_mut_slice(), device.clone()); + assert_eq!(len, 0); + + // Setup size smaller than what is the maximum offered + for queue_id in 0u16..2 { + config.write(0x16, queue_id.as_slice(), device.clone()); + config.write( + 0x18, + (max_size[queue_id as usize] - 1).as_slice(), + device.clone(), + ); + config.read(0x18, len.as_mut_slice(), device.clone()); + assert_eq!(len, max_size[queue_id as usize] - 1); + } + } + + #[test] + fn test_queue_msix_vector() { + let mut config = default_pci_common_config(); + let device = default_device(); + let mut vector = 0u16; + + // Our device has 2 queues, so we should be using 3 vectors in total. + // Trying to set a vector bigger than that should fail. Observing the + // failure happens through a subsequent read that should return NO_VECTOR. + for queue_id in 0u16..2 { + // Select queue + config.write(0x16, queue_id.as_slice(), device.clone()); + + config.write(0x1a, 3u16.as_slice(), device.clone()); + config.read(0x1a, vector.as_mut_slice(), device.clone()); + assert_eq!(vector, VIRTQ_MSI_NO_VECTOR); + + // Any of the 3 valid values should work + for vector_id in 0u16..3 { + config.write(0x1a, vector_id.as_slice(), device.clone()); + config.read(0x1a, vector.as_mut_slice(), device.clone()); + assert_eq!(vector, vector_id); + } + } + } + + #[test] + fn test_queue_enable() { + let mut config = default_pci_common_config(); + let device = default_device(); + let mut enabled = 0u16; + + for queue_id in 0u16..2 { + config.write(0x16, queue_id.as_slice(), device.clone()); + + // Initially queue should be disabled + config.read(0x1c, enabled.as_mut_slice(), device.clone()); + assert_eq!(enabled, 0); + + // Enable queue + config.write(0x1c, 1u16.as_slice(), device.clone()); + config.read(0x1c, enabled.as_mut_slice(), device.clone()); + assert_eq!(enabled, 1); + + // According to the specification "The driver MUST NOT write a 0 to queue_enable." + config.write(0x1c, 0u16.as_slice(), device.clone()); + config.read(0x1c, enabled.as_mut_slice(), device.clone()); + assert_eq!(enabled, 1); + } + } + + #[test] + fn test_queue_notify_off() { + let mut config = default_pci_common_config(); + let device = default_device(); + let mut offset = 0u16; + + // `queue_notify_off` is an offset (index not bytes) from the notification structure + // that helps locate the address of the queue notify within the device's BAR. This is + // a field setup by the device and should be read-only for the driver + + for queue_id in 0u16..2 { + config.write(0x16, queue_id.as_slice(), device.clone()); + config.read(0x1e, offset.as_mut_slice(), device.clone()); + assert_eq!(offset, queue_id); + + // Writing to it should not have any effect + config.write(0x1e, 0x42.as_slice(), device.clone()); + config.read(0x1e, offset.as_mut_slice(), device.clone()); + assert_eq!(offset, queue_id); + } + } + + fn write_64bit_field( + config: &mut VirtioPciCommonConfig, + device: Arc>, + offset: u64, + value: u64, + ) { + let lo32 = (value & 0xffff_ffff) as u32; + let hi32 = (value >> 32) as u32; + + config.write(offset, lo32.as_slice(), device.clone()); + config.write(offset + 4, hi32.as_slice(), device.clone()); + } + + fn read_64bit_field( + config: &mut VirtioPciCommonConfig, + device: Arc>, + offset: u64, + ) -> u64 { + let mut lo32 = 0u32; + let mut hi32 = 0u32; + + config.read(offset, lo32.as_mut_slice(), device.clone()); + config.read(offset + 4, hi32.as_mut_slice(), device.clone()); + + (lo32 as u64) | ((hi32 as u64) << 32) + } + + #[test] + fn test_queue_addresses() { + let mut config = default_pci_common_config(); + let device = default_device(); + let mut reg64bit = 0; + + for queue_id in 0u16..2 { + config.write(0x16, queue_id.as_slice(), device.clone()); + + for offset in [0x20, 0x28, 0x30] { + write_64bit_field(&mut config, device.clone(), offset, 0x0000_1312_0000_1110); + assert_eq!( + read_64bit_field(&mut config, device.clone(), offset), + 0x0000_1312_0000_1110 + ); + } + } + } + + #[test] + fn test_bad_width_reads() { + let mut config = default_pci_common_config(); + let mut device = default_device(); + + // According to the VirtIO specification (section 4.1.3.1) + // + // > For device configuration access, the driver MUST use 8-bit wide accesses for 8-bit + // > wide fields, 16-bit wide and aligned accesses for 16-bit wide fields and 32-bit wide + // > and aligned accesses for 32-bit and 64-bit wide fields. For 64-bit fields, the driver + // > MAY access each of the high and low 32-bit parts of the field independently. + + // 64-bit fields + device.lock().unwrap().queues_mut()[0].desc_table_address = + GuestAddress(0x0000_1312_0000_1110); + let mut buffer = [0u8; 8]; + config.read(0x20, &mut buffer[..1], device.clone()); + assert_eq!(buffer, [0u8; 8]); + config.read(0x20, &mut buffer[..2], device.clone()); + assert_eq!(buffer, [0u8; 8]); + config.read(0x20, &mut buffer[..8], device.clone()); + assert_eq!(buffer, [0u8; 8]); + config.read(0x20, &mut buffer[..4], device.clone()); + assert_eq!(LittleEndian::read_u32(&buffer[..4]), 0x1110); + config.read(0x24, &mut buffer[..4], device.clone()); + assert_eq!(LittleEndian::read_u32(&buffer[..4]), 0x1312); + + // 32-bit fields + config.device_feature_select = 0x42; + let mut buffer = [0u8; 8]; + config.read(0, &mut buffer[..1], device.clone()); + assert_eq!(buffer, [0u8; 8]); + config.read(0, &mut buffer[..2], device.clone()); + assert_eq!(buffer, [0u8; 8]); + config.read(0, &mut buffer[..8], device.clone()); + assert_eq!(buffer, [0u8; 8]); + config.read(0, &mut buffer[..4], device.clone()); + assert_eq!(LittleEndian::read_u32(&buffer[..4]), 0x42); + + // 16-bit fields + let mut buffer = [0u8; 8]; + config.queue_select = 0x42; + config.read(0x16, &mut buffer[..1], device.clone()); + assert_eq!(buffer, [0u8; 8]); + config.read(0x16, &mut buffer[..4], device.clone()); + assert_eq!(buffer, [0u8; 8]); + config.read(0x16, &mut buffer[..8], device.clone()); + assert_eq!(buffer, [0u8; 8]); + config.read(0x16, &mut buffer[..2], device.clone()); + assert_eq!(LittleEndian::read_u16(&buffer[..2]), 0x42); + + // 8-bit fields + let mut buffer = [0u8; 8]; + config.driver_status = 0x42; + config.read(0x14, &mut buffer[..2], device.clone()); + assert_eq!(buffer, [0u8; 8]); + config.read(0x14, &mut buffer[..4], device.clone()); + assert_eq!(buffer, [0u8; 8]); + config.read(0x14, &mut buffer[..8], device.clone()); + assert_eq!(buffer, [0u8; 8]); + config.read(0x14, &mut buffer[..1], device.clone()); + assert_eq!(buffer[0], 0x42); + } } diff --git a/src/vmm/src/devices/virtio/transport/pci/device.rs b/src/vmm/src/devices/virtio/transport/pci/device.rs index 038264bb417..3d6e4aee6a8 100644 --- a/src/vmm/src/devices/virtio/transport/pci/device.rs +++ b/src/vmm/src/devices/virtio/transport/pci/device.rs @@ -16,6 +16,7 @@ use std::sync::{Arc, Barrier, Mutex}; use anyhow::anyhow; use kvm_ioctls::{IoEventAddress, NoDatamatch}; +use log::warn; use pci::{ BarReprogrammingParams, MsixCap, MsixConfig, MsixConfigState, PciBarConfiguration, PciBarRegionType, PciBdf, PciCapability, PciCapabilityId, PciClassCode, PciConfiguration, @@ -33,12 +34,12 @@ use vmm_sys_util::eventfd::EventFd; use crate::Vm; use crate::devices::virtio::device::VirtioDevice; +use crate::devices::virtio::generated::virtio_ids; use crate::devices::virtio::queue::Queue; use crate::devices::virtio::transport::pci::common_config::{ VirtioPciCommonConfig, VirtioPciCommonConfigState, }; use crate::devices::virtio::transport::{VirtioInterrupt, VirtioInterruptType}; -use crate::devices::virtio::{TYPE_BLOCK, TYPE_NET}; use crate::logger::{debug, error}; use crate::snapshot::Persist; use crate::utils::u64_to_usize; @@ -53,16 +54,6 @@ const DEVICE_DRIVER_OK: u8 = 0x04; const DEVICE_FEATURES_OK: u8 = 0x08; const DEVICE_FAILED: u8 = 0x80; -const VIRTIO_F_RING_INDIRECT_DESC: u32 = 28; -const VIRTIO_F_RING_EVENT_IDX: u32 = 29; -const VIRTIO_F_VERSION_1: u32 = 32; -const VIRTIO_F_IOMMU_PLATFORM: u32 = 33; -const VIRTIO_F_IN_ORDER: u32 = 35; -const VIRTIO_F_ORDER_PLATFORM: u32 = 36; -#[allow(dead_code)] -const VIRTIO_F_SR_IOV: u32 = 37; -const VIRTIO_F_NOTIFICATION_DATA: u32 = 38; - /// Vector value used to disable MSI for a queue. pub const VIRTQ_MSI_NO_VECTOR: u16 = 0xffff; @@ -83,14 +74,13 @@ enum PciCapabilityType { // fields cap_vndr (1 byte) and cap_next (1 byte) defined in the virtio spec. const VIRTIO_PCI_CAP_OFFSET: usize = 2; -#[allow(dead_code)] #[repr(C, packed)] -#[derive(Debug, Clone, Copy, Default)] +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] struct VirtioPciCap { cap_len: u8, // Generic PCI field: capability length cfg_type: u8, // Identifies the structure. pci_bar: u8, // Where to find it. - id: u8, // Multiple capabilities of the same type + id: u8, // Multiple capabilities of the same type. padding: [u8; 2], // Pad to full dword. offset: Le32, // Offset within bar. length: Le32, // Length of the structure, in bytes. @@ -126,7 +116,6 @@ impl VirtioPciCap { } } -#[allow(dead_code)] #[repr(C, packed)] #[derive(Clone, Copy, Default)] struct VirtioPciNotifyCap { @@ -164,49 +153,8 @@ impl VirtioPciNotifyCap { } } -#[allow(dead_code)] #[repr(C, packed)] -#[derive(Clone, Copy, Default)] -struct VirtioPciCap64 { - cap: VirtioPciCap, - offset_hi: Le32, - length_hi: Le32, -} -// SAFETY: All members are simple numbers and any value is valid. -unsafe impl ByteValued for VirtioPciCap64 {} - -impl PciCapability for VirtioPciCap64 { - fn bytes(&self) -> &[u8] { - self.as_slice() - } - - fn id(&self) -> PciCapabilityId { - PciCapabilityId::VendorSpecific - } -} - -impl VirtioPciCap64 { - pub fn new(cfg_type: PciCapabilityType, pci_bar: u8, id: u8, offset: u64, length: u64) -> Self { - VirtioPciCap64 { - cap: VirtioPciCap { - cap_len: u8::try_from(std::mem::size_of::()).unwrap() - + VIRTIO_PCI_CAP_LEN_OFFSET, - cfg_type: cfg_type as u8, - pci_bar, - id, - padding: [0; 2], - offset: Le32::from((offset & 0xffff_ffff) as u32), - length: Le32::from((length & 0xffff_ffff) as u32), - }, - offset_hi: Le32::from((offset >> 32) as u32), - length_hi: Le32::from((length >> 32) as u32), - } - } -} - -#[allow(dead_code)] -#[repr(C, packed)] -#[derive(Debug, Clone, Copy, Default)] +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] struct VirtioPciCfgCap { cap: VirtioPciCap, pci_cfg_data: [u8; 4], @@ -227,7 +175,15 @@ impl PciCapability for VirtioPciCfgCap { impl VirtioPciCfgCap { fn new() -> Self { VirtioPciCfgCap { - cap: VirtioPciCap::new(PciCapabilityType::Pci, 0, 0), + cap: VirtioPciCap { + cap_len: u8::try_from(size_of::()).unwrap() + VIRTIO_PCI_CAP_LEN_OFFSET, + cfg_type: PciCapabilityType::Pci as u8, + pci_bar: VIRTIO_BAR_INDEX, + id: 0, + padding: [0; 2], + offset: Le32::from(0), + length: Le32::from(0), + }, ..Default::default() } } @@ -239,7 +195,6 @@ struct VirtioPciCfgCapInfo { cap: VirtioPciCfgCap, } -#[allow(dead_code)] #[derive(Debug, Copy, Clone)] pub enum PciVirtioSubclass { NonTransitionalBase = 0xff, @@ -281,21 +236,10 @@ const NOTIFY_OFF_MULTIPLIER: u32 = 4; // A dword per notification address. const VIRTIO_PCI_VENDOR_ID: u16 = 0x1af4; const VIRTIO_PCI_DEVICE_ID_BASE: u16 = 0x1040; // Add to device type to get device ID. -#[derive(Debug, Clone, Serialize, Deserialize)] -pub struct QueueState { - max_size: u16, - size: u16, - ready: bool, - desc_table: u64, - avail_ring: u64, - used_ring: u64, -} - #[derive(Debug, Clone, Serialize, Deserialize)] pub struct VirtioPciDeviceState { pub pci_device_bdf: PciBdf, pub device_activated: bool, - pub interrupt_status: usize, pub cap_pci_cfg_offset: usize, pub cap_pci_cfg: Vec, pub pci_configuration_state: PciConfigurationState, @@ -337,7 +281,6 @@ pub struct VirtioPciDevice { device_activated: Arc, // PCI interrupts. - interrupt_status: Arc, virtio_interrupt: Option>, interrupt_source_group: Arc, @@ -372,11 +315,11 @@ impl VirtioPciDevice { ) -> PciConfiguration { let pci_device_id = VIRTIO_PCI_DEVICE_ID_BASE + u16::try_from(virtio_device_type).unwrap(); let (class, subclass) = match virtio_device_type { - TYPE_NET => ( + virtio_ids::VIRTIO_ID_NET => ( PciClassCode::NetworkController, &PciNetworkControllerSubclass::EthernetController as &dyn PciSubclass, ), - TYPE_BLOCK => ( + virtio_ids::VIRTIO_ID_BLOCK => ( PciClassCode::MassStorage, &PciMassStorageSubclass::MassStorage as &dyn PciSubclass, ), @@ -458,7 +401,6 @@ impl VirtioPciDevice { msix_num: msi_vectors.num_vectors(), device, device_activated: Arc::new(AtomicBool::new(false)), - interrupt_status: Arc::new(AtomicUsize::new(0)), virtio_interrupt: Some(interrupt), memory, interrupt_source_group: msi_vectors, @@ -509,7 +451,6 @@ impl VirtioPciDevice { msix_num: msi_vectors.num_vectors(), device, device_activated: Arc::new(AtomicBool::new(state.device_activated)), - interrupt_status: Arc::new(AtomicUsize::new(state.interrupt_status)), virtio_interrupt: Some(interrupt), memory: memory.clone(), interrupt_source_group: msi_vectors, @@ -627,10 +568,14 @@ impl VirtioPciDevice { .unwrap(); } } else { - let bar_offset: u32 = - // SAFETY: we know self.cap_pci_cfg_info.cap.cap.offset is 32bits long. - unsafe { std::mem::transmute(self.cap_pci_cfg_info.cap.cap.offset) }; - self.read_bar(0, bar_offset as u64, data) + let bar_offset: u32 = self.cap_pci_cfg_info.cap.cap.offset.into(); + let len = u32::from(self.cap_pci_cfg_info.cap.cap.length) as usize; + // BAR reads expect that the buffer has the exact size of the field that + // offset is pointing to. So, do some check that the `length` has a meaningful value + // and only use the part of the buffer we actually need. + if len <= 4 { + self.read_bar(0, bar_offset as u64, &mut data[..len]); + } } } @@ -648,10 +593,17 @@ impl VirtioPciDevice { right[..data_len].copy_from_slice(data); None } else { - let bar_offset: u32 = - // SAFETY: we know self.cap_pci_cfg_info.cap.cap.offset is 32bits long. - unsafe { std::mem::transmute(self.cap_pci_cfg_info.cap.cap.offset) }; - self.write_bar(0, bar_offset as u64, data) + let bar_offset: u32 = self.cap_pci_cfg_info.cap.cap.offset.into(); + let len = u32::from(self.cap_pci_cfg_info.cap.cap.length) as usize; + // BAR writes expect that the buffer has the exact size of the field that + // offset is pointing to. So, do some check that the `length` has a meaningful value + // and only use the part of the buffer we actually need. + if len <= 4 { + let len = len.min(data.len()); + self.write_bar(0, bar_offset as u64, &data[..len]) + } else { + None + } } } @@ -682,34 +634,10 @@ impl VirtioPciDevice { Ok(()) } - /// Unregister the IoEvent notification for a VirtIO device - pub fn unregister_notification_ioevent( - &self, - vm: &Vm, - ) -> std::result::Result<(), errno::Error> { - let bar_addr = self.config_bar_addr(); - for (i, queue_evt) in self - .device - .lock() - .expect("Poisoned lock") - .queue_events() - .iter() - .enumerate() - { - let notify_base = bar_addr + NOTIFICATION_BAR_OFFSET; - let io_addr = - IoEventAddress::Mmio(notify_base + i as u64 * NOTIFY_OFF_MULTIPLIER as u64); - vm.fd() - .unregister_ioevent(queue_evt, &io_addr, NoDatamatch)?; - } - Ok(()) - } - pub fn state(&self) -> VirtioPciDeviceState { VirtioPciDeviceState { pci_device_bdf: self.pci_device_bdf, device_activated: self.device_activated.load(Ordering::Acquire), - interrupt_status: self.interrupt_status.load(Ordering::Acquire), cap_pci_cfg_offset: self.cap_pci_cfg_info.offset, cap_pci_cfg: self.cap_pci_cfg_info.cap.bytes().to_vec(), pci_configuration_state: self.configuration.state(), @@ -850,8 +778,13 @@ impl PciDevice for VirtioPciDevice { { let offset = base - self.cap_pci_cfg_info.offset; let mut data = [0u8; 4]; - self.read_cap_pci_cfg(offset, &mut data); - u32::from_le_bytes(data) + let len = u32::from(self.cap_pci_cfg_info.cap.cap.length) as usize; + if len <= 4 { + self.read_cap_pci_cfg(offset, &mut data[..len]); + u32::from_le_bytes(data) + } else { + 0 + } } else { self.configuration.read_reg(reg_idx) } @@ -928,14 +861,9 @@ impl PciDevice for VirtioPciDevice { .read(o - COMMON_CONFIG_BAR_OFFSET, data, self.device.clone()) } o if (ISR_CONFIG_BAR_OFFSET..ISR_CONFIG_BAR_OFFSET + ISR_CONFIG_SIZE).contains(&o) => { - if let Some(v) = data.get_mut(0) { - // Reading this register resets it to 0. - *v = self - .interrupt_status - .swap(0, Ordering::AcqRel) - .try_into() - .unwrap(); - } + // We don't actually support legacy INT#x interrupts for VirtIO PCI devices + warn!("pci: read access to unsupported ISR status field"); + data.fill(0); } o if (DEVICE_CONFIG_BAR_OFFSET..DEVICE_CONFIG_BAR_OFFSET + DEVICE_CONFIG_SIZE) .contains(&o) => @@ -947,6 +875,7 @@ impl PciDevice for VirtioPciDevice { .contains(&o) => { // Handled with ioeventfds. + warn!("pci: unexpected read to notification BAR. Offset {o:#x}"); } o if (MSIX_TABLE_BAR_OFFSET..MSIX_TABLE_BAR_OFFSET + MSIX_TABLE_SIZE).contains(&o) => { if let Some(msix_config) = &self.msix_config { @@ -975,10 +904,8 @@ impl PciDevice for VirtioPciDevice { .write(o - COMMON_CONFIG_BAR_OFFSET, data, self.device.clone()) } o if (ISR_CONFIG_BAR_OFFSET..ISR_CONFIG_BAR_OFFSET + ISR_CONFIG_SIZE).contains(&o) => { - if let Some(v) = data.first() { - self.interrupt_status - .fetch_and(!(*v as usize), Ordering::AcqRel); - } + // We don't actually support legacy INT#x interrupts for VirtIO PCI devices + warn!("pci: access to unsupported ISR status field"); } o if (DEVICE_CONFIG_BAR_OFFSET..DEVICE_CONFIG_BAR_OFFSET + DEVICE_CONFIG_SIZE) .contains(&o) => @@ -990,7 +917,7 @@ impl PciDevice for VirtioPciDevice { .contains(&o) => { // Handled with ioeventfds. - error!("Unexpected write to notification BAR: offset = 0x{:x}", o); + warn!("pci: unexpected write to notification BAR. Offset {o:#x}"); } o if (MSIX_TABLE_BAR_OFFSET..MSIX_TABLE_BAR_OFFSET + MSIX_TABLE_SIZE).contains(&o) => { if let Some(msix_config) = &self.msix_config { @@ -1081,19 +1008,30 @@ mod tests { use event_manager::MutEventSubscriber; use linux_loader::loader::Cmdline; - use pci::{PciBdf, PciClassCode, PciDevice, PciSubclass}; + use pci::{ + MsixCap, PciBdf, PciCapability, PciCapabilityId, PciClassCode, PciDevice, PciSubclass, + }; + use vm_memory::{ByteValued, Le32}; - use super::VirtioPciDevice; - use crate::Vm; + use super::{PciCapabilityType, VirtioPciDevice}; use crate::arch::MEM_64BIT_DEVICES_START; use crate::builder::tests::default_vmm; use crate::devices::virtio::device::VirtioDevice; + use crate::devices::virtio::device_status::{ACKNOWLEDGE, DRIVER, DRIVER_OK, FEATURES_OK}; + use crate::devices::virtio::generated::virtio_config::VIRTIO_F_VERSION_1; + use crate::devices::virtio::generated::virtio_ids; use crate::devices::virtio::rng::Entropy; - use crate::devices::virtio::transport::pci::device::PciVirtioSubclass; + use crate::devices::virtio::transport::pci::device::{ + COMMON_CONFIG_BAR_OFFSET, COMMON_CONFIG_SIZE, DEVICE_CONFIG_BAR_OFFSET, DEVICE_CONFIG_SIZE, + ISR_CONFIG_BAR_OFFSET, ISR_CONFIG_SIZE, NOTIFICATION_BAR_OFFSET, NOTIFICATION_SIZE, + NOTIFY_OFF_MULTIPLIER, PciVirtioSubclass, VirtioPciCap, VirtioPciCfgCap, + VirtioPciNotifyCap, + }; use crate::rate_limiter::RateLimiter; + use crate::utils::u64_to_usize; + use crate::{Vm, Vmm}; - #[test] - fn test_pci_device_config() { + fn create_vmm_with_virtio_pci_device() -> Vmm { let mut vmm = default_vmm(); vmm.device_manager.enable_pci(&vmm.vm); let entropy = Arc::new(Mutex::new(Entropy::new(RateLimiter::default()).unwrap())); @@ -1106,13 +1044,21 @@ mod tests { false, ) .unwrap(); + vmm + } - let device = vmm - .device_manager + fn get_virtio_device(vmm: &Vmm) -> Arc> { + vmm.device_manager .pci_devices - .get_virtio_device(entropy.lock().unwrap().device_type(), "rng") - .unwrap(); + .get_virtio_device(virtio_ids::VIRTIO_ID_RNG, "rng") + .unwrap() + .clone() + } + #[test] + fn test_pci_device_config() { + let mut vmm = create_vmm_with_virtio_pci_device(); + let device = get_virtio_device(&vmm); let mut locked_virtio_pci_device = device.lock().unwrap(); // For more information for the values we are checking here look into the VirtIO spec here: @@ -1212,25 +1158,8 @@ mod tests { #[test] fn test_reading_bars() { - let mut vmm = default_vmm(); - vmm.device_manager.enable_pci(&vmm.vm); - let entropy = Arc::new(Mutex::new(Entropy::new(RateLimiter::default()).unwrap())); - vmm.device_manager - .attach_virtio_device( - &vmm.vm, - "rng".to_string(), - entropy.clone(), - &mut Cmdline::new(1024).unwrap(), - false, - ) - .unwrap(); - - let device = vmm - .device_manager - .pci_devices - .get_virtio_device(entropy.lock().unwrap().device_type(), "rng") - .unwrap(); - + let mut vmm = create_vmm_with_virtio_pci_device(); + let device = get_virtio_device(&vmm); let mut locked_virtio_pci_device = device.lock().unwrap(); // According to OSdev wiki (https://wiki.osdev.org/PCI#Configuration_Space): @@ -1283,4 +1212,460 @@ mod tests { // We create a capabilities BAR region of 0x80000 bytes assert_eq!(bar_size, 0x80000); } + + fn read_virtio_pci_cap( + device: &mut VirtioPciDevice, + offset: u32, + ) -> (PciCapabilityId, u8, VirtioPciCap) { + let word1 = device.read_config_register((offset >> 2) as usize); + let word2 = device.read_config_register((offset >> 2) as usize + 1); + let word3 = device.read_config_register((offset >> 2) as usize + 2); + let word4 = device.read_config_register((offset >> 2) as usize + 3); + + let id = PciCapabilityId::from((word1 & 0xff) as u8); + let next = ((word1 >> 8) & 0xff) as u8; + + let cap = VirtioPciCap { + cap_len: ((word1 >> 16) & 0xff) as u8, + cfg_type: ((word1 >> 24) & 0xff) as u8, + pci_bar: (word2 & 0xff) as u8, + id: ((word2 >> 8) & 0xff) as u8, + padding: [0u8; 2], + offset: Le32::from(word3), + length: Le32::from(word4), + }; + + // We only ever set a single capability of a type. It's ID is 0. + assert_eq!(cap.id, 0); + + (id, next, cap) + } + + fn read_virtio_notification_cap( + device: &mut VirtioPciDevice, + offset: u32, + ) -> (PciCapabilityId, u8, VirtioPciNotifyCap) { + let (id, next, cap) = read_virtio_pci_cap(device, offset); + let word5 = device.read_config_register((offset >> 2) as usize + 4); + + let notification_cap = VirtioPciNotifyCap { + cap, + notify_off_multiplier: Le32::from(word5), + }; + + (id, next, notification_cap) + } + + fn read_virtio_pci_config_cap( + device: &mut VirtioPciDevice, + offset: u32, + ) -> (PciCapabilityId, u8, VirtioPciCfgCap) { + let (id, next, cap) = read_virtio_pci_cap(device, offset); + let word5 = device.read_config_register((offset >> 2) as usize + 4); + + let pci_cfg_cap = VirtioPciCfgCap { + cap, + pci_cfg_data: word5.as_slice().try_into().unwrap(), + }; + + (id, next, pci_cfg_cap) + } + + fn read_msix_cap(device: &mut VirtioPciDevice, offset: u32) -> (PciCapabilityId, u8, MsixCap) { + let word1 = device.read_config_register((offset >> 2) as usize); + let table = device.read_config_register((offset >> 2) as usize + 1); + let pba = device.read_config_register((offset >> 2) as usize + 2); + + let id = PciCapabilityId::from((word1 & 0xff) as u8); + let next = ((word1 >> 8) & 0xff) as u8; + + let cap = MsixCap { + msg_ctl: (word1 & 0xffff) as u16, + table, + pba, + }; + + (id, next, cap) + } + + fn capabilities_start(device: &mut VirtioPciDevice) -> u32 { + device.read_config_register(0xd) & 0xfc + } + + #[test] + fn test_capabilities() { + let mut vmm = create_vmm_with_virtio_pci_device(); + let device = get_virtio_device(&vmm); + let mut locked_virtio_pci_device = device.lock().unwrap(); + + // VirtIO devices need to expose a set of mandatory capabilities: + // * Common configuration + // * Notifications + // * ISR status + // * PCI configuration access + // + // and, optionally, a device-specific configuration area for those devices that need it. + // + // We always expose all 5 capabilities, so check that the capabilities are present + + // Common config + let common_config_cap_offset = capabilities_start(&mut locked_virtio_pci_device); + let (id, next, cap) = + read_virtio_pci_cap(&mut locked_virtio_pci_device, common_config_cap_offset); + assert_eq!(id, PciCapabilityId::VendorSpecific); + assert_eq!(cap.cap_len as usize, size_of::() + 2); + assert_eq!(cap.cfg_type, PciCapabilityType::Common as u8); + assert_eq!(cap.pci_bar, 0); + assert_eq!(u32::from(cap.offset) as u64, COMMON_CONFIG_BAR_OFFSET); + assert_eq!(u32::from(cap.length) as u64, COMMON_CONFIG_SIZE); + assert_eq!(next as u32, common_config_cap_offset + cap.cap_len as u32); + + // ISR + let isr_cap_offset = next as u32; + let (id, next, cap) = read_virtio_pci_cap(&mut locked_virtio_pci_device, isr_cap_offset); + assert_eq!(id, PciCapabilityId::VendorSpecific); + assert_eq!(cap.cap_len as usize, size_of::() + 2); + assert_eq!(cap.cfg_type, PciCapabilityType::Isr as u8); + assert_eq!(cap.pci_bar, 0); + assert_eq!(u32::from(cap.offset) as u64, ISR_CONFIG_BAR_OFFSET); + assert_eq!(u32::from(cap.length) as u64, ISR_CONFIG_SIZE); + assert_eq!(next as u32, isr_cap_offset + cap.cap_len as u32); + + // Device config + let device_config_cap_offset = next as u32; + let (id, next, cap) = + read_virtio_pci_cap(&mut locked_virtio_pci_device, device_config_cap_offset); + assert_eq!(id, PciCapabilityId::VendorSpecific); + assert_eq!(cap.cap_len as usize, size_of::() + 2); + assert_eq!(cap.cfg_type, PciCapabilityType::Device as u8); + assert_eq!(cap.pci_bar, 0); + assert_eq!(u32::from(cap.offset) as u64, DEVICE_CONFIG_BAR_OFFSET); + assert_eq!(u32::from(cap.length) as u64, DEVICE_CONFIG_SIZE); + assert_eq!(next as u32, device_config_cap_offset + cap.cap_len as u32); + + let notification_cap_offset = next as u32; + let (id, next, cap) = + read_virtio_notification_cap(&mut locked_virtio_pci_device, notification_cap_offset); + assert_eq!(id, PciCapabilityId::VendorSpecific); + assert_eq!( + cap.cap.cap_len as usize, + size_of::() + 2 + ); + assert_eq!(cap.cap.cfg_type, PciCapabilityType::Notify as u8); + assert_eq!(cap.cap.pci_bar, 0); + assert_eq!(u32::from(cap.cap.offset) as u64, NOTIFICATION_BAR_OFFSET); + assert_eq!(u32::from(cap.cap.length) as u64, NOTIFICATION_SIZE); + assert_eq!( + next as u32, + notification_cap_offset + cap.cap.cap_len as u32 + ); + assert_eq!(u32::from(cap.notify_off_multiplier), NOTIFY_OFF_MULTIPLIER); + + let pci_config_cap_offset = next as u32; + let (id, next, cap) = + read_virtio_pci_config_cap(&mut locked_virtio_pci_device, pci_config_cap_offset); + assert_eq!(id, PciCapabilityId::VendorSpecific); + assert_eq!(cap.cap.cap_len as usize, size_of::() + 2); + assert_eq!(cap.cap.cfg_type, PciCapabilityType::Pci as u8); + assert_eq!(cap.cap.pci_bar, 0); + assert_eq!(u32::from(cap.cap.offset) as u64, 0); + assert_eq!(u32::from(cap.cap.length) as u64, 0); + assert_eq!( + locked_virtio_pci_device.cap_pci_cfg_info.offset, + pci_config_cap_offset as usize + 2 + ); + assert_eq!(locked_virtio_pci_device.cap_pci_cfg_info.cap, cap); + assert_eq!(next as u32, pci_config_cap_offset + cap.cap.cap_len as u32); + + let msix_cap_offset = next as u32; + let (id, next, cap) = read_msix_cap(&mut locked_virtio_pci_device, msix_cap_offset); + assert_eq!(id, PciCapabilityId::MsiX); + assert_eq!(next, 0); + } + + fn cap_pci_cfg_read(device: &mut VirtioPciDevice, bar_offset: u32, length: u32) -> u32 { + let pci_config_cap_offset = capabilities_start(device) as usize + + 3 * (size_of::() + 2) + + (size_of::() + 2); + + // To program the access through the PCI config capability mechanism, we need to write the + // bar offset and read length in the `VirtioPciCfgCap::cap.offset` and + // `VirtioPciCfgCap::length` fields. These are the third and fourth word respectively + // within the capability. The fifth word of the capability should contain the data + let offset_register = (pci_config_cap_offset + 8) >> 2; + let length_register = (pci_config_cap_offset + 12) >> 2; + let data_register = (pci_config_cap_offset + 16) >> 2; + + device.write_config_register(offset_register, 0, bar_offset.as_slice()); + device.write_config_register(length_register, 0, length.as_slice()); + device.read_config_register(data_register) + } + + fn cap_pci_cfg_write(device: &mut VirtioPciDevice, bar_offset: u32, length: u32, data: &[u8]) { + let pci_config_cap_offset = capabilities_start(device) as usize + + 3 * (size_of::() + 2) + + (size_of::() + 2); + + // To program the access through the PCI config capability mechanism, we need to write the + // bar offset and read length in the `VirtioPciCfgCap::cap.offset` and + // `VirtioPciCfgCap::length` fields. These are the third and fourth word respectively + // within the capability. The fifth word of the capability should contain the data + let offset_register = (pci_config_cap_offset + 8) >> 2; + let length_register = (pci_config_cap_offset + 12) >> 2; + let data_register = (pci_config_cap_offset + 16) >> 2; + + device.write_config_register(offset_register, 0, bar_offset.as_slice()); + device.write_config_register(length_register, 0, length.as_slice()); + device.write_config_register(data_register, 0, data); + } + + #[test] + fn test_pci_configuration_cap() { + let mut vmm = create_vmm_with_virtio_pci_device(); + let device = get_virtio_device(&vmm); + let mut locked_virtio_pci_device = device.lock().unwrap(); + + // Let's read the number of queues of the entropy device + // That information is located at offset 0x12 past the BAR region belonging to the common + // config capability. + let bar_offset = u32::try_from(COMMON_CONFIG_BAR_OFFSET).unwrap() + 0x12; + let len = 2u32; + let num_queues = cap_pci_cfg_read(&mut locked_virtio_pci_device, bar_offset, len); + assert_eq!(num_queues, 1); + + // Let's update the driver features and see if that takes effect + let bar_offset = u32::try_from(COMMON_CONFIG_BAR_OFFSET).unwrap() + 0x14; + let len = 1u32; + let device_status = cap_pci_cfg_read(&mut locked_virtio_pci_device, bar_offset, len); + assert_eq!(device_status, 0); + cap_pci_cfg_write( + &mut locked_virtio_pci_device, + bar_offset, + len, + 0x42u32.as_slice(), + ); + let device_status = cap_pci_cfg_read(&mut locked_virtio_pci_device, bar_offset, len); + assert_eq!(device_status, 0x42); + + // reads with out-of-bounds lengths should return 0s + assert_eq!( + cap_pci_cfg_read(&mut locked_virtio_pci_device, bar_offset, 8), + 0 + ); + // writes out-of-bounds lengths should have no effect + cap_pci_cfg_write( + &mut locked_virtio_pci_device, + bar_offset, + 8, + 0x84u32.as_slice(), + ); + assert_eq!( + cap_pci_cfg_read(&mut locked_virtio_pci_device, bar_offset, 1), + 0x42 + ); + // Make sure that we handle properly from/to a BAR where the access length doesn't match + // what we've set in the capability's length + cap_pci_cfg_write( + &mut locked_virtio_pci_device, + bar_offset, + 2, + 0x42u8.as_slice(), + ); + } + + fn isr_status_read(device: &mut VirtioPciDevice) -> u32 { + let mut data = 0u32; + device.read_bar(0, ISR_CONFIG_BAR_OFFSET, data.as_mut_slice()); + data + } + + fn isr_status_write(device: &mut VirtioPciDevice, data: u32) { + device.write_bar(0, ISR_CONFIG_BAR_OFFSET, data.as_slice()); + } + + #[test] + fn test_isr_capability() { + let mut vmm = create_vmm_with_virtio_pci_device(); + let device = get_virtio_device(&vmm); + let mut locked_virtio_pci_device = device.lock().unwrap(); + + // We don't support legacy interrupts so reads to ISR BAR should always return 0s and + // writes to it should not have any effect + assert_eq!(isr_status_read(&mut locked_virtio_pci_device), 0); + isr_status_write(&mut locked_virtio_pci_device, 0x1312); + assert_eq!(isr_status_read(&mut locked_virtio_pci_device), 0); + } + + #[test] + fn test_notification_capability() { + let mut vmm = create_vmm_with_virtio_pci_device(); + let device = get_virtio_device(&vmm); + let mut locked_virtio_pci_device = device.lock().unwrap(); + + let notification_cap_offset = (capabilities_start(&mut locked_virtio_pci_device) as usize + + 3 * (size_of::() + 2)) + .try_into() + .unwrap(); + + let (_, _, notify_cap) = + read_virtio_notification_cap(&mut locked_virtio_pci_device, notification_cap_offset); + + // We do not offer `VIRTIO_F_NOTIFICATION_DATA` so: + // * `cap.offset` MUST by 2-byte aligned + assert_eq!(u32::from(notify_cap.cap.offset) & 0x3, 0); + // * The device MUST either present notify_off_multiplier as an even power of 2, or present + // notify_off_multiplier as 0. + let multiplier = u32::from(notify_cap.notify_off_multiplier); + assert!(multiplier.is_power_of_two() && multiplier.trailing_zeros() % 2 == 0); + // * For all queues, the value cap.length presented by the device MUST satisfy: + // + // `cap.length >= queue_notify_off * notify_off_multiplier + 2` + // + // The spec allows for up to 65536 queues, but in reality the device we are using with most + // queues is vsock (3). Let's check here for 16, projecting for future devices and + // use-cases such as multiple queue pairs in network devices + assert!(u32::from(notify_cap.cap.length) >= 15 * multiplier + 2); + + // Reads and writes to the notification region of the BAR are handled by IoEvent file + // descriptors. Any such accesses should have no effects. + let data = [0x42u8; u64_to_usize(NOTIFICATION_SIZE)]; + locked_virtio_pci_device.write_bar(0, NOTIFICATION_BAR_OFFSET, &data); + let mut buffer = [0x0; u64_to_usize(NOTIFICATION_SIZE)]; + locked_virtio_pci_device.read_bar(0, NOTIFICATION_BAR_OFFSET, &mut buffer); + assert_eq!(buffer, [0u8; u64_to_usize(NOTIFICATION_SIZE)]); + } + + fn write_driver_status(device: &mut VirtioPciDevice, status: u8) { + device.write_bar(0, COMMON_CONFIG_BAR_OFFSET + 0x14, status.as_slice()); + } + + fn read_driver_status(device: &mut VirtioPciDevice) -> u8 { + let mut status = 0u8; + device.read_bar(0, COMMON_CONFIG_BAR_OFFSET + 0x14, status.as_mut_slice()); + status + } + + fn read_device_features(device: &mut VirtioPciDevice) -> u64 { + let mut features_lo = 0u32; + device.write_bar(0, COMMON_CONFIG_BAR_OFFSET, 0u32.as_slice()); + device.read_bar( + 0, + COMMON_CONFIG_BAR_OFFSET + 0x4, + features_lo.as_mut_slice(), + ); + let mut features_hi = 0u32; + device.write_bar(0, COMMON_CONFIG_BAR_OFFSET, 1u32.as_slice()); + device.read_bar( + 0, + COMMON_CONFIG_BAR_OFFSET + 0x4, + features_hi.as_mut_slice(), + ); + + features_lo as u64 | ((features_hi as u64) << 32) + } + + fn write_driver_features(device: &mut VirtioPciDevice, features: u64) { + device.write_bar(0, COMMON_CONFIG_BAR_OFFSET + 0x8, 0u32.as_slice()); + device.write_bar( + 0, + COMMON_CONFIG_BAR_OFFSET + 0xc, + ((features & 0xffff_ffff) as u32).as_slice(), + ); + device.write_bar(0, COMMON_CONFIG_BAR_OFFSET + 0x8, 1u32.as_slice()); + device.write_bar( + 0, + COMMON_CONFIG_BAR_OFFSET + 0xc, + (((features >> 32) & 0xffff_ffff) as u32).as_slice(), + ); + } + + fn setup_queues(device: &mut VirtioPciDevice) { + device.write_bar( + 0, + COMMON_CONFIG_BAR_OFFSET + 0x20, + 0x8000_0000u64.as_slice(), + ); + device.write_bar( + 0, + COMMON_CONFIG_BAR_OFFSET + 0x28, + 0x8000_1000u64.as_slice(), + ); + device.write_bar( + 0, + COMMON_CONFIG_BAR_OFFSET + 0x30, + 0x8000_2000u64.as_slice(), + ); + device.write_bar(0, COMMON_CONFIG_BAR_OFFSET + 0x1c, 1u16.as_slice()); + } + + #[test] + fn test_device_initialization() { + let mut vmm = create_vmm_with_virtio_pci_device(); + let device = get_virtio_device(&vmm); + let mut locked_virtio_pci_device = device.lock().unwrap(); + + assert!(locked_virtio_pci_device.is_driver_init()); + assert!(!locked_virtio_pci_device.is_driver_ready()); + assert!( + !locked_virtio_pci_device + .device_activated + .load(std::sync::atomic::Ordering::SeqCst) + ); + + write_driver_status( + &mut locked_virtio_pci_device, + ACKNOWLEDGE.try_into().unwrap(), + ); + write_driver_status( + &mut locked_virtio_pci_device, + (ACKNOWLEDGE | DRIVER).try_into().unwrap(), + ); + assert!(!locked_virtio_pci_device.is_driver_init()); + assert!(!locked_virtio_pci_device.is_driver_ready()); + assert!( + !locked_virtio_pci_device + .device_activated + .load(std::sync::atomic::Ordering::SeqCst) + ); + + let status = read_driver_status(&mut locked_virtio_pci_device); + assert_eq!(status as u32, ACKNOWLEDGE | DRIVER); + + // Entropy device just offers VIRTIO_F_VERSION_1 + let offered_features = read_device_features(&mut locked_virtio_pci_device); + assert_eq!(offered_features, 1 << VIRTIO_F_VERSION_1); + // ACK features + write_driver_features(&mut locked_virtio_pci_device, offered_features); + write_driver_status( + &mut locked_virtio_pci_device, + (ACKNOWLEDGE | DRIVER | FEATURES_OK).try_into().unwrap(), + ); + let status = read_driver_status(&mut locked_virtio_pci_device); + assert!((status & u8::try_from(FEATURES_OK).unwrap()) != 0); + + assert!(!locked_virtio_pci_device.is_driver_init()); + assert!(!locked_virtio_pci_device.is_driver_ready()); + assert!( + !locked_virtio_pci_device + .device_activated + .load(std::sync::atomic::Ordering::SeqCst) + ); + + setup_queues(&mut locked_virtio_pci_device); + + write_driver_status( + &mut locked_virtio_pci_device, + (ACKNOWLEDGE | DRIVER | FEATURES_OK | DRIVER_OK) + .try_into() + .unwrap(), + ); + + assert!(!locked_virtio_pci_device.is_driver_init()); + assert!(locked_virtio_pci_device.is_driver_ready()); + assert!( + locked_virtio_pci_device + .device_activated + .load(std::sync::atomic::Ordering::SeqCst) + ); + } } diff --git a/src/vmm/src/devices/virtio/vsock/device.rs b/src/vmm/src/devices/virtio/vsock/device.rs index 43c9d4cb2ba..7fe10d158ad 100644 --- a/src/vmm/src/devices/virtio/vsock/device.rs +++ b/src/vmm/src/devices/virtio/vsock/device.rs @@ -34,10 +34,12 @@ use super::{VsockBackend, defs}; use crate::devices::virtio::ActivateError; use crate::devices::virtio::device::{ActiveState, DeviceState, VirtioDevice}; use crate::devices::virtio::generated::virtio_config::{VIRTIO_F_IN_ORDER, VIRTIO_F_VERSION_1}; +use crate::devices::virtio::generated::virtio_ids::VIRTIO_ID_VSOCK; use crate::devices::virtio::queue::{InvalidAvailIdx, Queue as VirtQueue}; use crate::devices::virtio::transport::{VirtioInterrupt, VirtioInterruptType}; use crate::devices::virtio::vsock::VsockError; use crate::devices::virtio::vsock::metrics::METRICS; +use crate::impl_device_type; use crate::logger::IncMetric; use crate::utils::byte_order; use crate::vstate::memory::{Bytes, GuestMemoryMmap}; @@ -282,6 +284,8 @@ impl VirtioDevice for Vsock where B: VsockBackend + Debug + 'static, { + impl_device_type!(VIRTIO_ID_VSOCK); + fn avail_features(&self) -> u64 { self.avail_features } @@ -294,10 +298,6 @@ where self.acked_features = acked_features } - fn device_type(&self) -> u32 { - uapi::VIRTIO_ID_VSOCK - } - fn queues(&self) -> &[VirtQueue] { &self.queues } @@ -412,7 +412,7 @@ mod tests { (driver_features & 0xffff_ffff) as u32, (driver_features >> 32) as u32, ]; - assert_eq!(ctx.device.device_type(), uapi::VIRTIO_ID_VSOCK); + assert_eq!(ctx.device.device_type(), VIRTIO_ID_VSOCK); assert_eq!(ctx.device.avail_features_by_page(0), device_pages[0]); assert_eq!(ctx.device.avail_features_by_page(1), device_pages[1]); assert_eq!(ctx.device.avail_features_by_page(2), 0); diff --git a/src/vmm/src/devices/virtio/vsock/mod.rs b/src/vmm/src/devices/virtio/vsock/mod.rs index 859e198860b..cc9f7746580 100644 --- a/src/vmm/src/devices/virtio/vsock/mod.rs +++ b/src/vmm/src/devices/virtio/vsock/mod.rs @@ -26,7 +26,6 @@ use vm_memory::GuestMemoryError; use vmm_sys_util::epoll::EventSet; pub use self::defs::VSOCK_DEV_ID; -pub use self::defs::uapi::VIRTIO_ID_VSOCK as TYPE_VSOCK; pub use self::device::Vsock; use self::packet::{VsockPacketRx, VsockPacketTx}; pub use self::unix::{VsockUnixBackend, VsockUnixBackendError}; @@ -56,11 +55,6 @@ mod defs { pub const MAX_PKT_BUF_SIZE: u32 = 64 * 1024; pub mod uapi { - - /// Virtio vsock device ID. - /// Defined in `include/uapi/linux/virtio_ids.h`. - pub const VIRTIO_ID_VSOCK: u32 = 19; - /// Vsock packet operation IDs. /// Defined in `/include/uapi/linux/virtio_vsock.h`. /// diff --git a/src/vmm/src/devices/virtio/vsock/persist.rs b/src/vmm/src/devices/virtio/vsock/persist.rs index 6775707da3e..e00e1cde2f1 100644 --- a/src/vmm/src/devices/virtio/vsock/persist.rs +++ b/src/vmm/src/devices/virtio/vsock/persist.rs @@ -10,10 +10,10 @@ use serde::{Deserialize, Serialize}; use super::*; use crate::devices::virtio::device::{ActiveState, DeviceState}; +use crate::devices::virtio::generated::virtio_ids::{self, VIRTIO_ID_VSOCK}; use crate::devices::virtio::persist::VirtioDeviceState; use crate::devices::virtio::queue::FIRECRACKER_MAX_QUEUE_SIZE; use crate::devices::virtio::transport::VirtioInterrupt; -use crate::devices::virtio::vsock::TYPE_VSOCK; use crate::snapshot::Persist; use crate::vstate::memory::GuestMemoryMmap; @@ -112,7 +112,7 @@ where .virtio_state .build_queues_checked( &constructor_args.mem, - TYPE_VSOCK, + VIRTIO_ID_VSOCK, defs::VSOCK_NUM_QUEUES, FIRECRACKER_MAX_QUEUE_SIZE, ) @@ -195,7 +195,7 @@ pub(crate) mod tests { ) .unwrap(); - assert_eq!(restored_device.device_type(), uapi::VIRTIO_ID_VSOCK); + assert_eq!(restored_device.device_type(), VIRTIO_ID_VSOCK); assert_eq!(restored_device.avail_features_by_page(0), device_pages[0]); assert_eq!(restored_device.avail_features_by_page(1), device_pages[1]); assert_eq!(restored_device.avail_features_by_page(2), 0); diff --git a/src/vmm/src/lib.rs b/src/vmm/src/lib.rs index 7bb33411b7e..a01242bdeb9 100644 --- a/src/vmm/src/lib.rs +++ b/src/vmm/src/lib.rs @@ -134,12 +134,9 @@ use vstate::kvm::Kvm; use vstate::vcpu::{self, StartThreadedError, VcpuSendEventError}; use crate::cpu_config::templates::CpuConfiguration; -use crate::devices::virtio::balloon::{ - BALLOON_DEV_ID, Balloon, BalloonConfig, BalloonError, BalloonStats, -}; +use crate::devices::virtio::balloon::{BALLOON_DEV_ID, Balloon, BalloonConfig, BalloonStats}; use crate::devices::virtio::block::device::Block; use crate::devices::virtio::net::Net; -use crate::devices::virtio::{TYPE_BALLOON, TYPE_BLOCK, TYPE_NET}; use crate::logger::{METRICS, MetricsError, error, info, warn}; use crate::persist::{MicrovmState, MicrovmStateError, VmInfo}; use crate::rate_limiter::BucketUpdate; @@ -522,10 +519,8 @@ impl Vmm { path_on_host: String, ) -> Result<(), VmmError> { self.device_manager - .with_virtio_device_with_id(TYPE_BLOCK, drive_id, |block: &mut Block| { - block - .update_disk_image(path_on_host) - .map_err(|err| err.to_string()) + .try_with_virtio_device_with_id(drive_id, |block: &mut Block| { + block.update_disk_image(path_on_host) }) .map_err(VmmError::FindDeviceError) } @@ -538,10 +533,8 @@ impl Vmm { rl_ops: BucketUpdate, ) -> Result<(), VmmError> { self.device_manager - .with_virtio_device_with_id(TYPE_BLOCK, drive_id, |block: &mut Block| { - block - .update_rate_limiter(rl_bytes, rl_ops) - .map_err(|err| err.to_string()) + .try_with_virtio_device_with_id(drive_id, |block: &mut Block| { + block.update_rate_limiter(rl_bytes, rl_ops) }) .map_err(VmmError::FindDeviceError) } @@ -549,9 +542,7 @@ impl Vmm { /// Updates the rate limiter parameters for block device with `drive_id` id. pub fn update_vhost_user_block_config(&mut self, drive_id: &str) -> Result<(), VmmError> { self.device_manager - .with_virtio_device_with_id(TYPE_BLOCK, drive_id, |block: &mut Block| { - block.update_config().map_err(|err| err.to_string()) - }) + .try_with_virtio_device_with_id(drive_id, |block: &mut Block| block.update_config()) .map_err(VmmError::FindDeviceError) } @@ -565,105 +556,45 @@ impl Vmm { tx_ops: BucketUpdate, ) -> Result<(), VmmError> { self.device_manager - .with_virtio_device_with_id(TYPE_NET, net_id, |net: &mut Net| { - net.patch_rate_limiters(rx_bytes, rx_ops, tx_bytes, tx_ops); - Ok(()) + .with_virtio_device_with_id(net_id, |net: &mut Net| { + net.patch_rate_limiters(rx_bytes, rx_ops, tx_bytes, tx_ops) }) .map_err(VmmError::FindDeviceError) } /// Returns a reference to the balloon device if present. - pub fn balloon_config(&self) -> Result { - if let Some(virtio_device) = self - .device_manager - .get_virtio_device(TYPE_BALLOON, BALLOON_DEV_ID) - { - let config = virtio_device - .lock() - .expect("Poisoned lock") - .as_mut_any() - .downcast_mut::() - .unwrap() - .config(); - - Ok(config) - } else { - Err(BalloonError::DeviceNotFound) - } + pub fn balloon_config(&self) -> Result { + self.device_manager + .with_virtio_device_with_id(BALLOON_DEV_ID, |dev: &mut Balloon| dev.config()) + .map_err(VmmError::FindDeviceError) } /// Returns the latest balloon statistics if they are enabled. - pub fn latest_balloon_stats(&self) -> Result { - if let Some(virtio_device) = self - .device_manager - .get_virtio_device(TYPE_BALLOON, BALLOON_DEV_ID) - { - let latest_stats = virtio_device - .lock() - .expect("Poisoned lock") - .as_mut_any() - .downcast_mut::() - .unwrap() - .latest_stats() - .ok_or(BalloonError::StatisticsDisabled) - .cloned()?; - - Ok(latest_stats) - } else { - Err(BalloonError::DeviceNotFound) - } + pub fn latest_balloon_stats(&self) -> Result { + self.device_manager + .try_with_virtio_device_with_id(BALLOON_DEV_ID, |dev: &mut Balloon| dev.latest_stats()) + .map_err(VmmError::FindDeviceError) } /// Updates configuration for the balloon device target size. - pub fn update_balloon_config(&mut self, amount_mib: u32) -> Result<(), BalloonError> { - // The balloon cannot have a target size greater than the size of - // the guest memory. - if u64::from(amount_mib) > mem_size_mib(self.vm.guest_memory()) { - return Err(BalloonError::TooManyPagesRequested); - } - - if let Some(virtio_device) = self - .device_manager - .get_virtio_device(TYPE_BALLOON, BALLOON_DEV_ID) - { - { - virtio_device - .lock() - .expect("Poisoned lock") - .as_mut_any() - .downcast_mut::() - .unwrap() - .update_size(amount_mib)?; - - Ok(()) - } - } else { - Err(BalloonError::DeviceNotFound) - } + pub fn update_balloon_config(&mut self, amount_mib: u32) -> Result<(), VmmError> { + self.device_manager + .try_with_virtio_device_with_id(BALLOON_DEV_ID, |dev: &mut Balloon| { + dev.update_size(amount_mib) + }) + .map_err(VmmError::FindDeviceError) } /// Updates configuration for the balloon device as described in `balloon_stats_update`. pub fn update_balloon_stats_config( &mut self, stats_polling_interval_s: u16, - ) -> Result<(), BalloonError> { - if let Some(virtio_device) = self - .device_manager - .get_virtio_device(TYPE_BALLOON, BALLOON_DEV_ID) - { - { - virtio_device - .lock() - .expect("Poisoned lock") - .as_mut_any() - .downcast_mut::() - .unwrap() - .update_stats_polling_interval(stats_polling_interval_s)?; - } - Ok(()) - } else { - Err(BalloonError::DeviceNotFound) - } + ) -> Result<(), VmmError> { + self.device_manager + .try_with_virtio_device_with_id(BALLOON_DEV_ID, |dev: &mut Balloon| { + dev.update_stats_polling_interval(stats_polling_interval_s) + }) + .map_err(VmmError::FindDeviceError) } /// Signals Vmm to stop and exit. diff --git a/src/vmm/src/rpc_interface.rs b/src/vmm/src/rpc_interface.rs index d26b1ba877d..1360959a481 100644 --- a/src/vmm/src/rpc_interface.rs +++ b/src/vmm/src/rpc_interface.rs @@ -128,6 +128,8 @@ pub enum VmmAction { pub enum VmmActionError { /// Balloon config error: {0} BalloonConfig(#[from] BalloonConfigError), + /// Balloon update error: {0} + BalloonUpdate(VmmError), /// Boot source error: {0} BootSource(#[from] BootSourceConfigError), /// Create snapshot error: {0} @@ -630,14 +632,14 @@ impl RuntimeApiController { .expect("Poisoned lock") .balloon_config() .map(|state| VmmData::BalloonConfig(BalloonDeviceConfig::from(state))) - .map_err(|err| VmmActionError::BalloonConfig(BalloonConfigError::from(err))), + .map_err(VmmActionError::InternalVmm), GetBalloonStats => self .vmm .lock() .expect("Poisoned lock") .latest_balloon_stats() .map(VmmData::BalloonStats) - .map_err(|err| VmmActionError::BalloonConfig(BalloonConfigError::from(err))), + .map_err(VmmActionError::InternalVmm), GetFullVmConfig => Ok(VmmData::FullVmConfig((&self.vm_resources).into())), GetMMDS => self.get_mmds(), GetVmMachineConfig => Ok(VmmData::MachineConfiguration( @@ -661,14 +663,14 @@ impl RuntimeApiController { .expect("Poisoned lock") .update_balloon_config(balloon_update.amount_mib) .map(|_| VmmData::Empty) - .map_err(|err| VmmActionError::BalloonConfig(BalloonConfigError::from(err))), + .map_err(VmmActionError::BalloonUpdate), UpdateBalloonStatistics(balloon_stats_update) => self .vmm .lock() .expect("Poisoned lock") .update_balloon_stats_config(balloon_stats_update.stats_polling_interval_s) .map(|_| VmmData::Empty) - .map_err(|err| VmmActionError::BalloonConfig(BalloonConfigError::from(err))), + .map_err(VmmActionError::BalloonUpdate), UpdateBlockDevice(new_cfg) => self.update_block_device(new_cfg), UpdateNetworkInterface(netif_update) => self.update_net_rate_limiters(netif_update), diff --git a/src/vmm/src/vmm_config/balloon.rs b/src/vmm/src/vmm_config/balloon.rs index 6ac2fb34ecf..83d419c49db 100644 --- a/src/vmm/src/vmm_config/balloon.rs +++ b/src/vmm/src/vmm_config/balloon.rs @@ -16,18 +16,10 @@ type MutexBalloon = Arc>; pub enum BalloonConfigError { /// No balloon device found. DeviceNotFound, - /// Device is inactive, check if balloon driver is enabled in guest kernel. - DeviceNotActive, - /// Cannot enable/disable the statistics after boot. - InvalidStatsUpdate, /// Amount of pages requested is too large. TooManyPagesRequested, - /// Statistics for the balloon device are not enabled - StatsNotFound, /// Error creating the balloon device: {0} CreateFailure(crate::devices::virtio::balloon::BalloonError), - /// Error updating the balloon device configuration: {0} - UpdateFailure(std::io::Error), /// Firecracker's huge pages support is incompatible with memory ballooning. HugePages, } diff --git a/tools/bindgen.sh b/tools/bindgen.sh index 86817a0561f..4808dfceeb7 100755 --- a/tools/bindgen.sh +++ b/tools/bindgen.sh @@ -87,6 +87,11 @@ fc-bindgen \ --allowlist-type "virtio_net_hdr_v1" \ "$KERNEL_HEADERS_HOME/include/linux/virtio_net.h" >src/vmm/src/devices/virtio/generated/virtio_net.rs +info "BINDGEN virtio_ids.h" +fc-bindgen \ + --allowlist-var "VIRTIO_ID.*" \ + "$INCLUDE/linux/virtio_ids.h" >src/vmm/src/devices/virtio/generated/virtio_ids.rs + info "BINDGEN prctl.h" fc-bindgen \ --allowlist-var "PR_.*" \