Skip to content

Refactor VirtIO devices, preparing for a PCI transport layer #5160

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Apr 30, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions src/vmm/src/builder.rs
Original file line number Diff line number Diff line change
@@ -42,9 +42,9 @@ use crate::devices::legacy::{EventFdTrigger, SerialEventsWrapper, SerialWrapper}
use crate::devices::virtio::balloon::Balloon;
use crate::devices::virtio::block::device::Block;
use crate::devices::virtio::device::VirtioDevice;
use crate::devices::virtio::mmio::MmioTransport;
use crate::devices::virtio::net::Net;
use crate::devices::virtio::rng::Entropy;
use crate::devices::virtio::transport::mmio::{IrqTrigger, MmioTransport};
use crate::devices::virtio::vsock::{Vsock, VsockUnixBackend};
#[cfg(feature = "gdb")]
use crate::gdb;
@@ -597,8 +597,14 @@ fn attach_virtio_device<T: 'static + VirtioDevice + MutEventSubscriber + Debug>(
) -> Result<(), MmioError> {
event_manager.add_subscriber(device.clone());

let interrupt = Arc::new(IrqTrigger::new());
// The device mutex mustn't be locked here otherwise it will deadlock.
let device = MmioTransport::new(vmm.vm.guest_memory().clone(), device, is_vhost_user);
let device = MmioTransport::new(
vmm.vm.guest_memory().clone(),
interrupt,
device,
is_vhost_user,
);
vmm.mmio_device_manager
.register_mmio_virtio_for_boot(
vmm.vm.fd(),
32 changes: 21 additions & 11 deletions src/vmm/src/device_manager/mmio.rs
Original file line number Diff line number Diff line change
@@ -30,9 +30,9 @@
use crate::devices::virtio::balloon::Balloon;
use crate::devices::virtio::block::device::Block;
use crate::devices::virtio::device::VirtioDevice;
use crate::devices::virtio::mmio::MmioTransport;
use crate::devices::virtio::net::Net;
use crate::devices::virtio::rng::Entropy;
use crate::devices::virtio::transport::mmio::MmioTransport;
use crate::devices::virtio::vsock::{TYPE_VSOCK, Vsock, VsockUnixBackend};
use crate::devices::virtio::{TYPE_BALLOON, TYPE_BLOCK, TYPE_NET, TYPE_RNG};
#[cfg(target_arch = "x86_64")]
@@ -53,6 +53,8 @@
InvalidDeviceType,
/// {0}
InternalDeviceError(String),
/// Could not create IRQ for MMIO device: {0}
CreateIrq(#[from] std::io::Error),
/// Invalid MMIO IRQ configuration.
InvalidIrqConfig,
/// Failed to register IO event: {0}
@@ -205,7 +207,7 @@
vm.register_ioevent(queue_evt, &io_addr, u32::try_from(i).unwrap())
.map_err(MmioError::RegisterIoEvent)?;
}
vm.register_irqfd(&locked_device.interrupt_trigger().irq_evt, irq.get())
vm.register_irqfd(&mmio_device.interrupt.irq_evt, irq.get())
.map_err(MmioError::RegisterIrqFd)?;
}

@@ -223,7 +225,7 @@
device_info: &MMIODeviceInfo,
) -> Result<(), MmioError> {
// as per doc, [virtio_mmio.]device=<size>@<baseaddr>:<irq> needs to be appended
// to kernel command line for virtio mmio devices to get recongnized
// to kernel command line for virtio mmio devices to get recognized
// the size parameter has to be transformed to KiB, so dividing hexadecimal value in
// bytes to 1024; further, the '{}' formatting rust construct will automatically
// transform it to decimal
@@ -503,7 +505,7 @@
.unwrap();
if vsock.is_activated() {
info!("kick vsock {id}.");
vsock.signal_used_queue().unwrap();
vsock.signal_used_queue(0).unwrap();

Check warning on line 508 in src/vmm/src/device_manager/mmio.rs

Codecov / codecov/patch

src/vmm/src/device_manager/mmio.rs#L508

Added line #L508 was not covered by tests
}
}
TYPE_RNG => {
@@ -523,15 +525,18 @@
#[cfg(test)]
mod tests {

use std::ops::Deref;
use std::sync::Arc;

use vmm_sys_util::eventfd::EventFd;

use super::*;
use crate::Vm;
use crate::devices::virtio::ActivateError;
use crate::devices::virtio::device::{IrqTrigger, VirtioDevice};
use crate::devices::virtio::device::VirtioDevice;
use crate::devices::virtio::queue::Queue;
use crate::devices::virtio::transport::VirtioInterrupt;
use crate::devices::virtio::transport::mmio::IrqTrigger;
use crate::test_utils::multi_region_mem_raw;
use crate::vstate::kvm::Kvm;
use crate::vstate::memory::{GuestAddress, GuestMemoryMmap};
@@ -548,7 +553,8 @@
cmdline: &mut kernel_cmdline::Cmdline,
dev_id: &str,
) -> Result<u64, MmioError> {
let mmio_device = MmioTransport::new(guest_mem, device, false);
let interrupt = Arc::new(IrqTrigger::new());
let mmio_device = MmioTransport::new(guest_mem, interrupt, device, false);
let device_info = self.register_mmio_virtio_for_boot(
vm,
resource_allocator,
@@ -575,7 +581,7 @@
dummy: u32,
queues: Vec<Queue>,
queue_evts: [EventFd; 1],
interrupt_trigger: IrqTrigger,
interrupt_trigger: Option<Arc<IrqTrigger>>,
}

impl DummyDevice {
@@ -584,7 +590,7 @@
dummy: 0,
queues: QUEUE_SIZES.iter().map(|&s| Queue::new(s)).collect(),
queue_evts: [EventFd::new(libc::EFD_NONBLOCK).expect("cannot create eventFD")],
interrupt_trigger: IrqTrigger::new().expect("cannot create eventFD"),
interrupt_trigger: None,
}
}
}
@@ -616,8 +622,8 @@
&self.queue_evts
}

fn interrupt_trigger(&self) -> &IrqTrigger {
&self.interrupt_trigger
fn interrupt_trigger(&self) -> &dyn VirtioInterrupt {
self.interrupt_trigger.as_ref().unwrap().deref()
}

fn ack_features_by_page(&mut self, page: u32, value: u32) {
@@ -635,7 +641,11 @@
let _ = data;
}

fn activate(&mut self, _: GuestMemoryMmap) -> Result<(), ActivateError> {
fn activate(
&mut self,
_: GuestMemoryMmap,
_: Arc<dyn VirtioInterrupt>,
) -> Result<(), ActivateError> {
Ok(())
}

24 changes: 21 additions & 3 deletions src/vmm/src/device_manager/persist.rs
Original file line number Diff line number Diff line change
@@ -25,7 +25,6 @@ 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::mmio::MmioTransport;
use crate::devices::virtio::net::Net;
use crate::devices::virtio::net::persist::{
NetConstructorArgs, NetPersistError as NetError, NetState,
@@ -35,6 +34,7 @@ use crate::devices::virtio::rng::Entropy;
use crate::devices::virtio::rng::persist::{
EntropyConstructorArgs, EntropyPersistError as EntropyError, EntropyState,
};
use crate::devices::virtio::transport::mmio::{IrqTrigger, MmioTransport};
use crate::devices::virtio::vsock::persist::{
VsockConstructorArgs, VsockState, VsockUdsConstructorArgs,
};
@@ -473,11 +473,13 @@ impl<'a> Persist<'a> for MMIODeviceManager {
as_subscriber: Arc<Mutex<dyn MutEventSubscriber>>,
id: &String,
state: &MmioTransportState,
interrupt: Arc<IrqTrigger>,
device_info: &MMIODeviceInfo,
event_manager: &mut EventManager|
-> Result<(), Self::Error> {
let restore_args = MmioTransportConstructorArgs {
mem: mem.clone(),
interrupt,
device,
is_vhost_user,
};
@@ -512,9 +514,11 @@ impl<'a> Persist<'a> for MMIODeviceManager {
};

if let Some(balloon_state) = &state.balloon_device {
let interrupt = Arc::new(IrqTrigger::new());
let device = Arc::new(Mutex::new(Balloon::restore(
BalloonConstructorArgs {
mem: mem.clone(),
interrupt: interrupt.clone(),
restored_from_file: constructor_args.restored_from_file,
},
&balloon_state.device_state,
@@ -530,14 +534,19 @@ impl<'a> Persist<'a> for MMIODeviceManager {
device,
&balloon_state.device_id,
&balloon_state.transport_state,
interrupt,
&balloon_state.device_info,
constructor_args.event_manager,
)?;
}

for block_state in &state.block_devices {
let interrupt = Arc::new(IrqTrigger::new());
let device = Arc::new(Mutex::new(Block::restore(
BlockConstructorArgs { mem: mem.clone() },
BlockConstructorArgs {
mem: mem.clone(),
interrupt: interrupt.clone(),
},
&block_state.device_state,
)?));

@@ -551,6 +560,7 @@ impl<'a> Persist<'a> for MMIODeviceManager {
device,
&block_state.device_id,
&block_state.transport_state,
interrupt,
&block_state.device_info,
constructor_args.event_manager,
)?;
@@ -573,9 +583,11 @@ impl<'a> Persist<'a> for MMIODeviceManager {
}

for net_state in &state.net_devices {
let interrupt = Arc::new(IrqTrigger::new());
let device = Arc::new(Mutex::new(Net::restore(
NetConstructorArgs {
mem: mem.clone(),
interrupt: interrupt.clone(),
mmds: constructor_args
.vm_resources
.mmds
@@ -596,6 +608,7 @@ impl<'a> Persist<'a> for MMIODeviceManager {
device,
&net_state.device_id,
&net_state.transport_state,
interrupt,
&net_state.device_info,
constructor_args.event_manager,
)?;
@@ -606,9 +619,11 @@ impl<'a> Persist<'a> for MMIODeviceManager {
cid: vsock_state.device_state.frontend.cid,
};
let backend = VsockUnixBackend::restore(ctor_args, &vsock_state.device_state.backend)?;
let interrupt = Arc::new(IrqTrigger::new());
let device = Arc::new(Mutex::new(Vsock::restore(
VsockConstructorArgs {
mem: mem.clone(),
interrupt: interrupt.clone(),
backend,
},
&vsock_state.device_state.frontend,
@@ -624,13 +639,15 @@ impl<'a> Persist<'a> for MMIODeviceManager {
device,
&vsock_state.device_id,
&vsock_state.transport_state,
interrupt,
&vsock_state.device_info,
constructor_args.event_manager,
)?;
}

if let Some(entropy_state) = &state.entropy_device {
let ctor_args = EntropyConstructorArgs::new(mem.clone());
let interrupt = Arc::new(IrqTrigger::new());
let ctor_args = EntropyConstructorArgs::new(mem.clone(), interrupt.clone());

let device = Arc::new(Mutex::new(Entropy::restore(
ctor_args,
@@ -647,6 +664,7 @@ impl<'a> Persist<'a> for MMIODeviceManager {
device,
&entropy_state.device_id,
&entropy_state.transport_state,
interrupt,
&entropy_state.device_info,
constructor_args.event_manager,
)?;
2 changes: 1 addition & 1 deletion src/vmm/src/devices/bus.rs
Original file line number Diff line number Diff line change
@@ -56,7 +56,7 @@ use event_manager::{EventOps, Events, MutEventSubscriber};
use super::legacy::RTCDevice;
use super::legacy::{I8042Device, SerialDevice};
use super::pseudo::BootTimer;
use super::virtio::mmio::MmioTransport;
use super::virtio::transport::mmio::MmioTransport;

#[derive(Debug)]
pub enum BusDevice {
Loading