Skip to content

Commit 6b8dd03

Browse files
bchaliosbchalios
committed
refactor: use VirtioInterrupt in VirtIO devices
VirtIO devices assume they're operating under an MMIO transport and as a consequence they use IrqTrigger as interrupts. Switch that to using VirtioInterrupt for all VirtIO device objects. Only assume a VirtioInterrupt is an IrqTrigger in MMIO specific code. Signed-off-by: Babis Chalios <[email protected]>
1 parent 23f5a10 commit 6b8dd03

29 files changed

+410
-310
lines changed

‎resources/seccomp/aarch64-unknown-linux-musl.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1017,6 +1017,10 @@
10171017
{
10181018
"syscall": "restart_syscall",
10191019
"comment": "automatically issued by the kernel when specific timing-related syscalls (e.g. nanosleep) get interrupted by SIGSTOP"
1020+
},
1021+
{
1022+
"syscall": "dup",
1023+
"comment": "Used to get a copy of the EventFd that is backing a VirtioInterrupt object"
10201024
}
10211025
]
10221026
}

‎resources/seccomp/x86_64-unknown-linux-musl.json‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1149,6 +1149,10 @@
11491149
{
11501150
"syscall": "restart_syscall",
11511151
"comment": "automatically issued by the kernel when specific timing-related syscalls (e.g. nanosleep) get interrupted by SIGSTOP"
1152+
},
1153+
{
1154+
"syscall": "dup",
1155+
"comment": "Used to get a copy of the EventFd that is backing a VirtioInterrupt object"
11521156
}
11531157
]
11541158
}

‎src/vmm/src/device_manager/mmio.rs‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,7 @@ impl MMIODeviceManager {
505505
.unwrap();
506506
if vsock.is_activated() {
507507
info!("kick vsock {id}.");
508-
vsock.signal_used_queue().unwrap();
508+
vsock.signal_used_queue(0).unwrap();
509509
}
510510
}
511511
TYPE_RNG => {
@@ -534,6 +534,7 @@ mod tests {
534534
use crate::devices::virtio::ActivateError;
535535
use crate::devices::virtio::device::VirtioDevice;
536536
use crate::devices::virtio::queue::Queue;
537+
use crate::devices::virtio::transport::VirtioInterrupt;
537538
use crate::devices::virtio::transport::mmio::IrqTrigger;
538539
use crate::test_utils::multi_region_mem_raw;
539540
use crate::vstate::kvm::Kvm;
@@ -620,7 +621,7 @@ mod tests {
620621
&self.queue_evts
621622
}
622623

623-
fn interrupt_trigger(&self) -> Arc<IrqTrigger> {
624+
fn interrupt_trigger(&self) -> Arc<dyn VirtioInterrupt> {
624625
self.interrupt_trigger
625626
.as_ref()
626627
.expect("Device is not activated")
@@ -645,7 +646,7 @@ mod tests {
645646
fn activate(
646647
&mut self,
647648
_: GuestMemoryMmap,
648-
_: Arc<IrqTrigger>,
649+
_: Arc<dyn VirtioInterrupt>,
649650
) -> Result<(), ActivateError> {
650651
Ok(())
651652
}

‎src/vmm/src/devices/virtio/balloon/device.rs‎

Lines changed: 33 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,9 @@ use super::{
2424
VIRTIO_BALLOON_S_SWAP_OUT,
2525
};
2626
use crate::devices::virtio::balloon::BalloonError;
27+
use crate::devices::virtio::device::ActiveState;
2728
use crate::devices::virtio::generated::virtio_config::VIRTIO_F_VERSION_1;
28-
use crate::devices::virtio::transport::mmio::{IrqTrigger, IrqType};
29+
use crate::devices::virtio::transport::{VirtioInterrupt, VirtioInterruptType};
2930
use crate::logger::IncMetric;
3031
use crate::utils::u64_to_usize;
3132
use crate::vstate::memory::{Address, ByteValued, Bytes, GuestAddress, GuestMemoryMmap};
@@ -258,7 +259,7 @@ impl Balloon {
258259

259260
pub(crate) fn process_inflate(&mut self) -> Result<(), BalloonError> {
260261
// This is safe since we checked in the event handler that the device is activated.
261-
let (mem, _) = self.device_state.active_state().unwrap();
262+
let mem = &self.device_state.active_state().unwrap().mem;
262263
METRICS.inflate_count.inc();
263264

264265
let queue = &mut self.queues[INFLATE_INDEX];
@@ -339,7 +340,7 @@ impl Balloon {
339340
}
340341

341342
if needs_interrupt {
342-
self.signal_used_queue()?;
343+
self.signal_used_queue(INFLATE_INDEX)?;
343344
}
344345

345346
Ok(())
@@ -357,15 +358,15 @@ impl Balloon {
357358
}
358359

359360
if needs_interrupt {
360-
self.signal_used_queue()
361+
self.signal_used_queue(DEFLATE_INDEX)
361362
} else {
362363
Ok(())
363364
}
364365
}
365366

366367
pub(crate) fn process_stats_queue(&mut self) -> Result<(), BalloonError> {
367368
// This is safe since we checked in the event handler that the device is activated.
368-
let (mem, _) = self.device_state.active_state().unwrap();
369+
let mem = &self.device_state.active_state().unwrap().mem;
369370
METRICS.stats_updates_count.inc();
370371

371372
while let Some(head) = self.queues[STATS_INDEX].pop() {
@@ -401,9 +402,12 @@ impl Balloon {
401402
Ok(())
402403
}
403404

404-
pub(crate) fn signal_used_queue(&self) -> Result<(), BalloonError> {
405+
pub(crate) fn signal_used_queue(&self, qidx: usize) -> Result<(), BalloonError> {
405406
self.interrupt_trigger()
406-
.trigger_irq(IrqType::Vring)
407+
.trigger(VirtioInterruptType::Queue(
408+
qidx.try_into()
409+
.unwrap_or_else(|_| panic!("balloon: invalid queue id: {qidx}")),
410+
))
407411
.map_err(|err| {
408412
METRICS.event_fails.inc();
409413
BalloonError::InterruptError(err)
@@ -428,7 +432,7 @@ impl Balloon {
428432
self.queues[STATS_INDEX]
429433
.add_used(index, 0)
430434
.map_err(BalloonError::Queue)?;
431-
self.signal_used_queue()
435+
self.signal_used_queue(STATS_INDEX)
432436
} else {
433437
error!("Failed to update balloon stats, missing descriptor.");
434438
Ok(())
@@ -440,7 +444,7 @@ impl Balloon {
440444
if self.is_activated() {
441445
self.config_space.num_pages = mib_to_pages(amount_mib)?;
442446
self.interrupt_trigger()
443-
.trigger_irq(IrqType::Config)
447+
.trigger(VirtioInterruptType::Config)
444448
.map_err(BalloonError::InterruptError)
445449
} else {
446450
Err(BalloonError::DeviceNotActive)
@@ -551,11 +555,12 @@ impl VirtioDevice for Balloon {
551555
&self.queue_evts
552556
}
553557

554-
fn interrupt_trigger(&self) -> Arc<IrqTrigger> {
558+
fn interrupt_trigger(&self) -> Arc<dyn VirtioInterrupt> {
555559
self.device_state
556560
.active_state()
557561
.expect("Device is not activated")
558-
.1
562+
.interrupt
563+
.clone()
559564
}
560565

561566
fn read_config(&self, offset: u64, data: &mut [u8]) {
@@ -585,14 +590,14 @@ impl VirtioDevice for Balloon {
585590
fn activate(
586591
&mut self,
587592
mem: GuestMemoryMmap,
588-
interrupt: Arc<IrqTrigger>,
593+
interrupt: Arc<dyn VirtioInterrupt>,
589594
) -> Result<(), ActivateError> {
590595
for q in self.queues.iter_mut() {
591596
q.initialize(&mem)
592597
.map_err(ActivateError::QueueMemoryError)?;
593598
}
594599

595-
self.device_state = DeviceState::Activated((mem, interrupt));
600+
self.device_state = DeviceState::Activated(ActiveState { mem, interrupt });
596601
if self.activate_evt.write(1).is_err() {
597602
METRICS.activate_fails.inc();
598603
self.device_state = DeviceState::Inactive;
@@ -621,7 +626,7 @@ pub(crate) mod tests {
621626
check_request_completion, invoke_handler_for_queue_event, set_request,
622627
};
623628
use crate::devices::virtio::queue::{VIRTQ_DESC_F_NEXT, VIRTQ_DESC_F_WRITE};
624-
use crate::devices::virtio::test_utils::{VirtQueue, default_mem};
629+
use crate::devices::virtio::test_utils::{VirtQueue, default_interrupt, default_mem};
625630
use crate::test_utils::single_region_mem;
626631
use crate::vstate::memory::GuestAddress;
627632

@@ -798,11 +803,10 @@ pub(crate) mod tests {
798803
fn test_invalid_request() {
799804
let mut balloon = Balloon::new(0, true, 0, false).unwrap();
800805
let mem = default_mem();
801-
let interrupt = Arc::new(IrqTrigger::new().unwrap());
802806
// Only initialize the inflate queue to demonstrate invalid request handling.
803807
let infq = VirtQueue::new(GuestAddress(0), &mem, 16);
804808
balloon.set_queue(INFLATE_INDEX, infq.create_queue());
805-
balloon.activate(mem.clone(), interrupt).unwrap();
809+
balloon.activate(mem.clone(), default_interrupt()).unwrap();
806810

807811
// Fill the second page with non-zero bytes.
808812
for i in 0..0x1000 {
@@ -858,10 +862,9 @@ pub(crate) mod tests {
858862
fn test_inflate() {
859863
let mut balloon = Balloon::new(0, true, 0, false).unwrap();
860864
let mem = default_mem();
861-
let interrupt = Arc::new(IrqTrigger::new().unwrap());
862865
let infq = VirtQueue::new(GuestAddress(0), &mem, 16);
863866
balloon.set_queue(INFLATE_INDEX, infq.create_queue());
864-
balloon.activate(mem.clone(), interrupt).unwrap();
867+
balloon.activate(mem.clone(), default_interrupt()).unwrap();
865868

866869
// Fill the third page with non-zero bytes.
867870
for i in 0..0x1000 {
@@ -929,10 +932,9 @@ pub(crate) mod tests {
929932
fn test_deflate() {
930933
let mut balloon = Balloon::new(0, true, 0, false).unwrap();
931934
let mem = default_mem();
932-
let interrupt = Arc::new(IrqTrigger::new().unwrap());
933935
let defq = VirtQueue::new(GuestAddress(0), &mem, 16);
934936
balloon.set_queue(DEFLATE_INDEX, defq.create_queue());
935-
balloon.activate(mem.clone(), interrupt).unwrap();
937+
balloon.activate(mem.clone(), default_interrupt()).unwrap();
936938

937939
let page_addr = 0x10;
938940

@@ -978,10 +980,9 @@ pub(crate) mod tests {
978980
fn test_stats() {
979981
let mut balloon = Balloon::new(0, true, 1, false).unwrap();
980982
let mem = default_mem();
981-
let interrupt = Arc::new(IrqTrigger::new().unwrap());
982983
let statsq = VirtQueue::new(GuestAddress(0), &mem, 16);
983984
balloon.set_queue(STATS_INDEX, statsq.create_queue());
984-
balloon.activate(mem.clone(), interrupt).unwrap();
985+
balloon.activate(mem.clone(), default_interrupt()).unwrap();
985986

986987
let page_addr = 0x100;
987988

@@ -1057,7 +1058,9 @@ pub(crate) mod tests {
10571058
assert!(balloon.stats_desc_index.is_some());
10581059
balloon.process_stats_timer_event().unwrap();
10591060
assert!(balloon.stats_desc_index.is_none());
1060-
assert!(balloon.interrupt_trigger().has_pending_irq(IrqType::Vring));
1061+
assert!(balloon.interrupt_trigger().has_pending_interrupt(
1062+
VirtioInterruptType::Queue(STATS_INDEX.try_into().unwrap())
1063+
));
10611064
});
10621065
}
10631066
}
@@ -1066,23 +1069,21 @@ pub(crate) mod tests {
10661069
fn test_process_balloon_queues() {
10671070
let mut balloon = Balloon::new(0x10, true, 0, false).unwrap();
10681071
let mem = default_mem();
1069-
let interrupt = Arc::new(IrqTrigger::new().unwrap());
10701072
let infq = VirtQueue::new(GuestAddress(0), &mem, 16);
10711073
let defq = VirtQueue::new(GuestAddress(0), &mem, 16);
10721074

10731075
balloon.set_queue(INFLATE_INDEX, infq.create_queue());
10741076
balloon.set_queue(DEFLATE_INDEX, defq.create_queue());
10751077

1076-
balloon.activate(mem, interrupt).unwrap();
1078+
balloon.activate(mem, default_interrupt()).unwrap();
10771079
balloon.process_virtio_queues()
10781080
}
10791081

10801082
#[test]
10811083
fn test_update_stats_interval() {
10821084
let mut balloon = Balloon::new(0, true, 0, false).unwrap();
10831085
let mem = default_mem();
1084-
let interrupt = Arc::new(IrqTrigger::new().unwrap());
1085-
balloon.activate(mem, interrupt).unwrap();
1086+
balloon.activate(mem, default_interrupt()).unwrap();
10861087
assert_eq!(
10871088
format!("{:?}", balloon.update_stats_polling_interval(1)),
10881089
"Err(StatisticsStateChange)"
@@ -1091,8 +1092,7 @@ pub(crate) mod tests {
10911092

10921093
let mut balloon = Balloon::new(0, true, 1, false).unwrap();
10931094
let mem = default_mem();
1094-
let interrupt = Arc::new(IrqTrigger::new().unwrap());
1095-
balloon.activate(mem, interrupt).unwrap();
1095+
balloon.activate(mem, default_interrupt()).unwrap();
10961096
assert_eq!(
10971097
format!("{:?}", balloon.update_stats_polling_interval(0)),
10981098
"Err(StatisticsStateChange)"
@@ -1112,8 +1112,10 @@ pub(crate) mod tests {
11121112
fn test_num_pages() {
11131113
let mut balloon = Balloon::new(0, true, 0, false).unwrap();
11141114
// Switch the state to active.
1115-
balloon.device_state =
1116-
DeviceState::Activated((single_region_mem(0x1), Arc::new(IrqTrigger::new().unwrap())));
1115+
balloon.device_state = DeviceState::Activated(ActiveState {
1116+
mem: single_region_mem(0x1),
1117+
interrupt: default_interrupt(),
1118+
});
11171119

11181120
assert_eq!(balloon.num_pages(), 0);
11191121
assert_eq!(balloon.actual_pages(), 0);

‎src/vmm/src/devices/virtio/balloon/event_handler.rs‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -136,16 +136,15 @@ pub mod tests {
136136

137137
use super::*;
138138
use crate::devices::virtio::balloon::test_utils::set_request;
139-
use crate::devices::virtio::test_utils::{VirtQueue, default_mem};
140-
use crate::devices::virtio::transport::mmio::IrqTrigger;
139+
use crate::devices::virtio::test_utils::{VirtQueue, default_interrupt, default_mem};
141140
use crate::vstate::memory::GuestAddress;
142141

143142
#[test]
144143
fn test_event_handler() {
145144
let mut event_manager = EventManager::new().unwrap();
146145
let mut balloon = Balloon::new(0, true, 10, false).unwrap();
147146
let mem = default_mem();
148-
let interrupt = Arc::new(IrqTrigger::new().unwrap());
147+
let interrupt = default_interrupt();
149148
let infq = VirtQueue::new(GuestAddress(0), &mem, 16);
150149
balloon.set_queue(INFLATE_INDEX, infq.create_queue());
151150

‎src/vmm/src/devices/virtio/balloon/persist.rs‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,10 +12,10 @@ use timerfd::{SetTimeFlags, TimerState};
1212
use super::*;
1313
use crate::devices::virtio::TYPE_BALLOON;
1414
use crate::devices::virtio::balloon::device::{BalloonStats, ConfigSpace};
15-
use crate::devices::virtio::device::DeviceState;
15+
use crate::devices::virtio::device::{ActiveState, DeviceState};
1616
use crate::devices::virtio::persist::VirtioDeviceState;
1717
use crate::devices::virtio::queue::FIRECRACKER_MAX_QUEUE_SIZE;
18-
use crate::devices::virtio::transport::mmio::IrqTrigger;
18+
use crate::devices::virtio::transport::VirtioInterrupt;
1919
use crate::snapshot::Persist;
2020
use crate::vstate::memory::GuestMemoryMmap;
2121

@@ -96,7 +96,7 @@ pub struct BalloonConstructorArgs {
9696
/// Pointer to guest memory.
9797
pub mem: GuestMemoryMmap,
9898
/// Interrupt used from the device.
99-
pub interrupt: Arc<IrqTrigger>,
99+
pub interrupt: Arc<dyn VirtioInterrupt>,
100100
pub restored_from_file: bool,
101101
}
102102

@@ -155,8 +155,10 @@ impl Persist<'_> for Balloon {
155155
};
156156

157157
if state.virtio_state.activated {
158-
balloon.device_state =
159-
DeviceState::Activated((constructor_args.mem, constructor_args.interrupt));
158+
balloon.device_state = DeviceState::Activated(ActiveState {
159+
mem: constructor_args.mem,
160+
interrupt: constructor_args.interrupt,
161+
});
160162

161163
if balloon.stats_enabled() {
162164
// Restore the stats descriptor.

‎src/vmm/src/devices/virtio/balloon/test_utils.rs‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use crate::devices::virtio::{balloon::BALLOON_NUM_QUEUES, balloon::Balloon};
1010
#[cfg(test)]
1111
pub fn invoke_handler_for_queue_event(b: &mut Balloon, queue_index: usize) {
1212
use crate::devices::virtio::balloon::{DEFLATE_INDEX, INFLATE_INDEX, STATS_INDEX};
13-
use crate::devices::virtio::transport::mmio::IrqType;
13+
use crate::devices::virtio::transport::VirtioInterruptType;
1414

1515
assert!(queue_index < BALLOON_NUM_QUEUES);
1616
// Trigger the queue event.
@@ -23,8 +23,11 @@ pub fn invoke_handler_for_queue_event(b: &mut Balloon, queue_index: usize) {
2323
_ => unreachable!(),
2424
};
2525
// Validate the queue operation finished successfully.
26-
let (_, interrupt) = b.device_state.active_state().unwrap();
27-
assert!(interrupt.has_pending_irq(IrqType::Vring));
26+
let interrupt = b.interrupt_trigger();
27+
assert!(
28+
interrupt
29+
.has_pending_interrupt(VirtioInterruptType::Queue(queue_index.try_into().unwrap()))
30+
);
2831
}
2932

3033
pub fn set_request(queue: &VirtQueue, idx: u16, addr: u64, len: u32, flags: u16) {

0 commit comments

Comments
 (0)