Skip to content

Commit aa74281

Browse files
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 e8f2dd1 commit aa74281

File tree

25 files changed

+381
-246
lines changed

25 files changed

+381
-246
lines changed

src/vmm/src/device_manager/mmio.rs

Lines changed: 6 additions & 7 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 => {
@@ -525,6 +525,7 @@ impl MMIODeviceManager {
525525
#[cfg(test)]
526526
mod tests {
527527

528+
use std::ops::Deref;
528529
use std::sync::Arc;
529530

530531
use vmm_sys_util::eventfd::EventFd;
@@ -534,6 +535,7 @@ mod tests {
534535
use crate::devices::virtio::ActivateError;
535536
use crate::devices::virtio::device::VirtioDevice;
536537
use crate::devices::virtio::queue::Queue;
538+
use crate::devices::virtio::transport::VirtioInterrupt;
537539
use crate::devices::virtio::transport::mmio::IrqTrigger;
538540
use crate::test_utils::multi_region_mem_raw;
539541
use crate::vstate::kvm::Kvm;
@@ -620,11 +622,8 @@ mod tests {
620622
&self.queue_evts
621623
}
622624

623-
fn interrupt_trigger(&self) -> Arc<IrqTrigger> {
624-
self.interrupt_trigger
625-
.as_ref()
626-
.expect("Device is not activated")
627-
.clone()
625+
fn interrupt_trigger(&self) -> &dyn VirtioInterrupt {
626+
self.interrupt_trigger.as_ref().unwrap().deref()
628627
}
629628

630629
fn ack_features_by_page(&mut self, page: u32, value: u32) {
@@ -645,7 +644,7 @@ mod tests {
645644
fn activate(
646645
&mut self,
647646
_: GuestMemoryMmap,
648-
_: Arc<IrqTrigger>,
647+
_: Arc<dyn VirtioInterrupt>,
649648
) -> Result<(), ActivateError> {
650649
Ok(())
651650
}

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

Lines changed: 26 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Copyright 2020 Amazon.com, Inc. or its affiliates. All Rights Reserved.
22
// SPDX-License-Identifier: Apache-2.0
33

4+
use std::ops::Deref;
45
use std::sync::Arc;
56
use std::time::Duration;
67

@@ -24,8 +25,9 @@ use super::{
2425
VIRTIO_BALLOON_S_SWAP_OUT,
2526
};
2627
use crate::devices::virtio::balloon::BalloonError;
28+
use crate::devices::virtio::device::ActiveState;
2729
use crate::devices::virtio::generated::virtio_config::VIRTIO_F_VERSION_1;
28-
use crate::devices::virtio::transport::mmio::{IrqTrigger, IrqType};
30+
use crate::devices::virtio::transport::{VirtioInterrupt, VirtioInterruptType};
2931
use crate::logger::IncMetric;
3032
use crate::utils::u64_to_usize;
3133
use crate::vstate::memory::{Address, ByteValued, Bytes, GuestAddress, GuestMemoryMmap};
@@ -258,7 +260,7 @@ impl Balloon {
258260

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

264266
let queue = &mut self.queues[INFLATE_INDEX];
@@ -339,7 +341,7 @@ impl Balloon {
339341
}
340342

341343
if needs_interrupt {
342-
self.signal_used_queue()?;
344+
self.signal_used_queue(INFLATE_INDEX)?;
343345
}
344346

345347
Ok(())
@@ -357,15 +359,15 @@ impl Balloon {
357359
}
358360

359361
if needs_interrupt {
360-
self.signal_used_queue()
362+
self.signal_used_queue(DEFLATE_INDEX)
361363
} else {
362364
Ok(())
363365
}
364366
}
365367

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

371373
while let Some(head) = self.queues[STATS_INDEX].pop() {
@@ -401,9 +403,12 @@ impl Balloon {
401403
Ok(())
402404
}
403405

404-
pub(crate) fn signal_used_queue(&self) -> Result<(), BalloonError> {
406+
pub(crate) fn signal_used_queue(&self, qidx: usize) -> Result<(), BalloonError> {
405407
self.interrupt_trigger()
406-
.trigger_irq(IrqType::Vring)
408+
.trigger(VirtioInterruptType::Queue(
409+
qidx.try_into()
410+
.unwrap_or_else(|_| panic!("balloon: invalid queue id: {qidx}")),
411+
))
407412
.map_err(|err| {
408413
METRICS.event_fails.inc();
409414
BalloonError::InterruptError(err)
@@ -428,7 +433,7 @@ impl Balloon {
428433
self.queues[STATS_INDEX]
429434
.add_used(index, 0)
430435
.map_err(BalloonError::Queue)?;
431-
self.signal_used_queue()
436+
self.signal_used_queue(STATS_INDEX)
432437
} else {
433438
error!("Failed to update balloon stats, missing descriptor.");
434439
Ok(())
@@ -440,7 +445,7 @@ impl Balloon {
440445
if self.is_activated() {
441446
self.config_space.num_pages = mib_to_pages(amount_mib)?;
442447
self.interrupt_trigger()
443-
.trigger_irq(IrqType::Config)
448+
.trigger(VirtioInterruptType::Config)
444449
.map_err(BalloonError::InterruptError)
445450
} else {
446451
Err(BalloonError::DeviceNotActive)
@@ -551,11 +556,12 @@ impl VirtioDevice for Balloon {
551556
&self.queue_evts
552557
}
553558

554-
fn interrupt_trigger(&self) -> Arc<IrqTrigger> {
559+
fn interrupt_trigger(&self) -> &dyn VirtioInterrupt {
555560
self.device_state
556561
.active_state()
557562
.expect("Device is not activated")
558-
.1
563+
.interrupt
564+
.deref()
559565
}
560566

561567
fn read_config(&self, offset: u64, data: &mut [u8]) {
@@ -585,14 +591,14 @@ impl VirtioDevice for Balloon {
585591
fn activate(
586592
&mut self,
587593
mem: GuestMemoryMmap,
588-
interrupt: Arc<IrqTrigger>,
594+
interrupt: Arc<dyn VirtioInterrupt>,
589595
) -> Result<(), ActivateError> {
590596
for q in self.queues.iter_mut() {
591597
q.initialize(&mem)
592598
.map_err(ActivateError::QueueMemoryError)?;
593599
}
594600

595-
self.device_state = DeviceState::Activated((mem, interrupt));
601+
self.device_state = DeviceState::Activated(ActiveState { mem, interrupt });
596602
if self.activate_evt.write(1).is_err() {
597603
METRICS.activate_fails.inc();
598604
self.device_state = DeviceState::Inactive;
@@ -1057,7 +1063,9 @@ pub(crate) mod tests {
10571063
assert!(balloon.stats_desc_index.is_some());
10581064
balloon.process_stats_timer_event().unwrap();
10591065
assert!(balloon.stats_desc_index.is_none());
1060-
assert!(balloon.interrupt_trigger().has_pending_irq(IrqType::Vring));
1066+
assert!(balloon.interrupt_trigger().has_pending_interrupt(
1067+
VirtioInterruptType::Queue(STATS_INDEX.try_into().unwrap())
1068+
));
10611069
});
10621070
}
10631071
}
@@ -1112,8 +1120,10 @@ pub(crate) mod tests {
11121120
fn test_num_pages() {
11131121
let mut balloon = Balloon::new(0, true, 0, false).unwrap();
11141122
// Switch the state to active.
1115-
balloon.device_state =
1116-
DeviceState::Activated((single_region_mem(0x1), default_interrupt()));
1123+
balloon.device_state = DeviceState::Activated(ActiveState {
1124+
mem: single_region_mem(0x1),
1125+
interrupt: default_interrupt(),
1126+
});
11171127

11181128
assert_eq!(balloon.num_pages(), 0);
11191129
assert_eq!(balloon.actual_pages(), 0);

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: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,16 @@
33

44
#![doc(hidden)]
55

6+
#[cfg(test)]
7+
use crate::devices::virtio::device::VirtioDevice;
68
use crate::devices::virtio::test_utils::VirtQueue;
79
#[cfg(test)]
810
use crate::devices::virtio::{balloon::BALLOON_NUM_QUEUES, balloon::Balloon};
911

1012
#[cfg(test)]
1113
pub fn invoke_handler_for_queue_event(b: &mut Balloon, queue_index: usize) {
1214
use crate::devices::virtio::balloon::{DEFLATE_INDEX, INFLATE_INDEX, STATS_INDEX};
13-
use crate::devices::virtio::transport::mmio::IrqType;
15+
use crate::devices::virtio::transport::VirtioInterruptType;
1416

1517
assert!(queue_index < BALLOON_NUM_QUEUES);
1618
// Trigger the queue event.
@@ -23,8 +25,11 @@ pub fn invoke_handler_for_queue_event(b: &mut Balloon, queue_index: usize) {
2325
_ => unreachable!(),
2426
};
2527
// Validate the queue operation finished successfully.
26-
let (_, interrupt) = b.device_state.active_state().unwrap();
27-
assert!(interrupt.has_pending_irq(IrqType::Vring));
28+
let interrupt = b.interrupt_trigger();
29+
assert!(
30+
interrupt
31+
.has_pending_interrupt(VirtioInterruptType::Queue(queue_index.try_into().unwrap()))
32+
);
2833
}
2934

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

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

Lines changed: 5 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use super::vhost_user::device::{VhostUserBlock, VhostUserBlockConfig};
1212
use super::virtio::device::{VirtioBlock, VirtioBlockConfig};
1313
use crate::devices::virtio::device::VirtioDevice;
1414
use crate::devices::virtio::queue::Queue;
15-
use crate::devices::virtio::transport::mmio::IrqTrigger;
15+
use crate::devices::virtio::transport::VirtioInterrupt;
1616
use crate::devices::virtio::{ActivateError, TYPE_BLOCK};
1717
use crate::rate_limiter::BucketUpdate;
1818
use crate::snapshot::Persist;
@@ -176,20 +176,10 @@ impl VirtioDevice for Block {
176176
}
177177
}
178178

179-
fn interrupt_trigger(&self) -> Arc<IrqTrigger> {
179+
fn interrupt_trigger(&self) -> &dyn VirtioInterrupt {
180180
match self {
181-
Self::Virtio(b) => {
182-
b.device_state
183-
.active_state()
184-
.expect("Device is not initialized")
185-
.1
186-
}
187-
Self::VhostUser(b) => {
188-
b.device_state
189-
.active_state()
190-
.expect("Device is not initialized")
191-
.1
192-
}
181+
Self::Virtio(b) => b.interrupt_trigger(),
182+
Self::VhostUser(b) => b.interrupt_trigger(),
193183
}
194184
}
195185

@@ -210,7 +200,7 @@ impl VirtioDevice for Block {
210200
fn activate(
211201
&mut self,
212202
mem: GuestMemoryMmap,
213-
interrupt: Arc<IrqTrigger>,
203+
interrupt: Arc<dyn VirtioInterrupt>,
214204
) -> Result<(), ActivateError> {
215205
match self {
216206
Self::Virtio(b) => b.activate(mem, interrupt),

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ use serde::{Deserialize, Serialize};
77

88
use super::vhost_user::persist::VhostUserBlockState;
99
use super::virtio::persist::VirtioBlockState;
10-
use crate::devices::virtio::transport::mmio::IrqTrigger;
10+
use crate::devices::virtio::transport::VirtioInterrupt;
1111
use crate::vstate::memory::GuestMemoryMmap;
1212

1313
/// Block device state.
@@ -21,5 +21,5 @@ pub enum BlockState {
2121
#[derive(Debug)]
2222
pub struct BlockConstructorArgs {
2323
pub mem: GuestMemoryMmap,
24-
pub interrupt: Arc<IrqTrigger>,
24+
pub interrupt: Arc<dyn VirtioInterrupt>,
2525
}

0 commit comments

Comments
 (0)