Skip to content

Commit 151fa1b

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 869ee78 commit 151fa1b

File tree

16 files changed

+231
-171
lines changed

16 files changed

+231
-171
lines changed

‎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: 24 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ use super::{
2525
};
2626
use crate::devices::virtio::balloon::BalloonError;
2727
use crate::devices::virtio::generated::virtio_config::VIRTIO_F_VERSION_1;
28-
use crate::devices::virtio::transport::mmio::{IrqTrigger, IrqType};
28+
use crate::devices::virtio::transport::{VirtioInterrupt, VirtioInterruptType};
2929
use crate::logger::IncMetric;
3030
use crate::utils::u64_to_usize;
3131
use crate::vstate::memory::{Address, ByteValued, Bytes, GuestAddress, GuestMemoryMmap};
@@ -339,7 +339,7 @@ impl Balloon {
339339
}
340340

341341
if needs_interrupt {
342-
self.signal_used_queue()?;
342+
self.signal_used_queue(INFLATE_INDEX)?;
343343
}
344344

345345
Ok(())
@@ -357,7 +357,7 @@ impl Balloon {
357357
}
358358

359359
if needs_interrupt {
360-
self.signal_used_queue()
360+
self.signal_used_queue(DEFLATE_INDEX)
361361
} else {
362362
Ok(())
363363
}
@@ -401,9 +401,12 @@ impl Balloon {
401401
Ok(())
402402
}
403403

404-
pub(crate) fn signal_used_queue(&self) -> Result<(), BalloonError> {
404+
pub(crate) fn signal_used_queue(&self, qidx: usize) -> Result<(), BalloonError> {
405405
self.interrupt_trigger()
406-
.trigger_irq(IrqType::Vring)
406+
.trigger(VirtioInterruptType::Queue(
407+
qidx.try_into()
408+
.unwrap_or_else(|_| panic!("balloon: invalid queue id: {qidx}")),
409+
))
407410
.map_err(|err| {
408411
METRICS.event_fails.inc();
409412
BalloonError::InterruptError(err)
@@ -428,7 +431,7 @@ impl Balloon {
428431
self.queues[STATS_INDEX]
429432
.add_used(index, 0)
430433
.map_err(BalloonError::Queue)?;
431-
self.signal_used_queue()
434+
self.signal_used_queue(STATS_INDEX)
432435
} else {
433436
error!("Failed to update balloon stats, missing descriptor.");
434437
Ok(())
@@ -440,7 +443,7 @@ impl Balloon {
440443
if self.is_activated() {
441444
self.config_space.num_pages = mib_to_pages(amount_mib)?;
442445
self.interrupt_trigger()
443-
.trigger_irq(IrqType::Config)
446+
.trigger(VirtioInterruptType::Config)
444447
.map_err(BalloonError::InterruptError)
445448
} else {
446449
Err(BalloonError::DeviceNotActive)
@@ -551,7 +554,7 @@ impl VirtioDevice for Balloon {
551554
&self.queue_evts
552555
}
553556

554-
fn interrupt_trigger(&self) -> Arc<IrqTrigger> {
557+
fn interrupt_trigger(&self) -> Arc<dyn VirtioInterrupt> {
555558
self.device_state
556559
.active_state()
557560
.expect("Device is not activated")
@@ -585,7 +588,7 @@ impl VirtioDevice for Balloon {
585588
fn activate(
586589
&mut self,
587590
mem: GuestMemoryMmap,
588-
interrupt: Arc<IrqTrigger>,
591+
interrupt: Arc<dyn VirtioInterrupt>,
589592
) -> Result<(), ActivateError> {
590593
for q in self.queues.iter_mut() {
591594
q.initialize(&mem)
@@ -621,7 +624,7 @@ pub(crate) mod tests {
621624
check_request_completion, invoke_handler_for_queue_event, set_request,
622625
};
623626
use crate::devices::virtio::queue::{VIRTQ_DESC_F_NEXT, VIRTQ_DESC_F_WRITE};
624-
use crate::devices::virtio::test_utils::{VirtQueue, default_mem};
627+
use crate::devices::virtio::test_utils::{VirtQueue, default_interrupt, default_mem};
625628
use crate::test_utils::single_region_mem;
626629
use crate::vstate::memory::GuestAddress;
627630

@@ -798,11 +801,10 @@ pub(crate) mod tests {
798801
fn test_invalid_request() {
799802
let mut balloon = Balloon::new(0, true, 0, false).unwrap();
800803
let mem = default_mem();
801-
let interrupt = Arc::new(IrqTrigger::new().unwrap());
802804
// Only initialize the inflate queue to demonstrate invalid request handling.
803805
let infq = VirtQueue::new(GuestAddress(0), &mem, 16);
804806
balloon.set_queue(INFLATE_INDEX, infq.create_queue());
805-
balloon.activate(mem.clone(), interrupt).unwrap();
807+
balloon.activate(mem.clone(), default_interrupt()).unwrap();
806808

807809
// Fill the second page with non-zero bytes.
808810
for i in 0..0x1000 {
@@ -858,10 +860,9 @@ pub(crate) mod tests {
858860
fn test_inflate() {
859861
let mut balloon = Balloon::new(0, true, 0, false).unwrap();
860862
let mem = default_mem();
861-
let interrupt = Arc::new(IrqTrigger::new().unwrap());
862863
let infq = VirtQueue::new(GuestAddress(0), &mem, 16);
863864
balloon.set_queue(INFLATE_INDEX, infq.create_queue());
864-
balloon.activate(mem.clone(), interrupt).unwrap();
865+
balloon.activate(mem.clone(), default_interrupt()).unwrap();
865866

866867
// Fill the third page with non-zero bytes.
867868
for i in 0..0x1000 {
@@ -929,10 +930,9 @@ pub(crate) mod tests {
929930
fn test_deflate() {
930931
let mut balloon = Balloon::new(0, true, 0, false).unwrap();
931932
let mem = default_mem();
932-
let interrupt = Arc::new(IrqTrigger::new().unwrap());
933933
let defq = VirtQueue::new(GuestAddress(0), &mem, 16);
934934
balloon.set_queue(DEFLATE_INDEX, defq.create_queue());
935-
balloon.activate(mem.clone(), interrupt).unwrap();
935+
balloon.activate(mem.clone(), default_interrupt()).unwrap();
936936

937937
let page_addr = 0x10;
938938

@@ -978,10 +978,9 @@ pub(crate) mod tests {
978978
fn test_stats() {
979979
let mut balloon = Balloon::new(0, true, 1, false).unwrap();
980980
let mem = default_mem();
981-
let interrupt = Arc::new(IrqTrigger::new().unwrap());
982981
let statsq = VirtQueue::new(GuestAddress(0), &mem, 16);
983982
balloon.set_queue(STATS_INDEX, statsq.create_queue());
984-
balloon.activate(mem.clone(), interrupt).unwrap();
983+
balloon.activate(mem.clone(), default_interrupt()).unwrap();
985984

986985
let page_addr = 0x100;
987986

@@ -1057,7 +1056,9 @@ pub(crate) mod tests {
10571056
assert!(balloon.stats_desc_index.is_some());
10581057
balloon.process_stats_timer_event().unwrap();
10591058
assert!(balloon.stats_desc_index.is_none());
1060-
assert!(balloon.interrupt_trigger().has_pending_irq(IrqType::Vring));
1059+
assert!(balloon.interrupt_trigger().has_pending_interrupt(
1060+
VirtioInterruptType::Queue(STATS_INDEX.try_into().unwrap())
1061+
));
10611062
});
10621063
}
10631064
}
@@ -1066,23 +1067,21 @@ pub(crate) mod tests {
10661067
fn test_process_balloon_queues() {
10671068
let mut balloon = Balloon::new(0x10, true, 0, false).unwrap();
10681069
let mem = default_mem();
1069-
let interrupt = Arc::new(IrqTrigger::new().unwrap());
10701070
let infq = VirtQueue::new(GuestAddress(0), &mem, 16);
10711071
let defq = VirtQueue::new(GuestAddress(0), &mem, 16);
10721072

10731073
balloon.set_queue(INFLATE_INDEX, infq.create_queue());
10741074
balloon.set_queue(DEFLATE_INDEX, defq.create_queue());
10751075

1076-
balloon.activate(mem, interrupt).unwrap();
1076+
balloon.activate(mem, default_interrupt()).unwrap();
10771077
balloon.process_virtio_queues()
10781078
}
10791079

10801080
#[test]
10811081
fn test_update_stats_interval() {
10821082
let mut balloon = Balloon::new(0, true, 0, false).unwrap();
10831083
let mem = default_mem();
1084-
let interrupt = Arc::new(IrqTrigger::new().unwrap());
1085-
balloon.activate(mem, interrupt).unwrap();
1084+
balloon.activate(mem, default_interrupt()).unwrap();
10861085
assert_eq!(
10871086
format!("{:?}", balloon.update_stats_polling_interval(1)),
10881087
"Err(StatisticsStateChange)"
@@ -1091,8 +1090,7 @@ pub(crate) mod tests {
10911090

10921091
let mut balloon = Balloon::new(0, true, 1, false).unwrap();
10931092
let mem = default_mem();
1094-
let interrupt = Arc::new(IrqTrigger::new().unwrap());
1095-
balloon.activate(mem, interrupt).unwrap();
1093+
balloon.activate(mem, default_interrupt()).unwrap();
10961094
assert_eq!(
10971095
format!("{:?}", balloon.update_stats_polling_interval(0)),
10981096
"Err(StatisticsStateChange)"
@@ -1113,7 +1111,7 @@ pub(crate) mod tests {
11131111
let mut balloon = Balloon::new(0, true, 0, false).unwrap();
11141112
// Switch the state to active.
11151113
balloon.device_state =
1116-
DeviceState::Activated((single_region_mem(0x1), Arc::new(IrqTrigger::new().unwrap())));
1114+
DeviceState::Activated((single_region_mem(0x1), default_interrupt()));
11171115

11181116
assert_eq!(balloon.num_pages(), 0);
11191117
assert_eq!(balloon.actual_pages(), 0);

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

Lines changed: 5 additions & 2 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.
@@ -24,7 +24,10 @@ pub fn invoke_handler_for_queue_event(b: &mut Balloon, queue_index: usize) {
2424
};
2525
// Validate the queue operation finished successfully.
2626
let (_, interrupt) = b.device_state.active_state().unwrap();
27-
assert!(interrupt.has_pending_irq(IrqType::Vring));
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) {

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

Lines changed: 3 additions & 3 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,7 +176,7 @@ impl VirtioDevice for Block {
176176
}
177177
}
178178

179-
fn interrupt_trigger(&self) -> Arc<IrqTrigger> {
179+
fn interrupt_trigger(&self) -> Arc<dyn VirtioInterrupt> {
180180
match self {
181181
Self::Virtio(b) => {
182182
b.device_state
@@ -210,7 +210,7 @@ impl VirtioDevice for Block {
210210
fn activate(
211211
&mut self,
212212
mem: GuestMemoryMmap,
213-
interrupt: Arc<IrqTrigger>,
213+
interrupt: Arc<dyn VirtioInterrupt>,
214214
) -> Result<(), ActivateError> {
215215
match self {
216216
Self::Virtio(b) => b.activate(mem, interrupt),

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ use crate::devices::virtio::generated::virtio_blk::{VIRTIO_BLK_F_FLUSH, VIRTIO_B
1919
use crate::devices::virtio::generated::virtio_config::VIRTIO_F_VERSION_1;
2020
use crate::devices::virtio::generated::virtio_ring::VIRTIO_RING_F_EVENT_IDX;
2121
use crate::devices::virtio::queue::Queue;
22-
use crate::devices::virtio::transport::mmio::{IrqTrigger, IrqType};
22+
use crate::devices::virtio::transport::{VirtioInterrupt, VirtioInterruptType};
2323
use crate::devices::virtio::vhost_user::{VhostUserHandleBackend, VhostUserHandleImpl};
2424
use crate::devices::virtio::vhost_user_metrics::{
2525
VhostUserDeviceMetrics, VhostUserMetricsPerDevice,
@@ -272,7 +272,7 @@ impl<T: VhostUserHandleBackend> VhostUserBlockImpl<T> {
272272
.map_err(VhostUserBlockError::Vhost)?;
273273
self.config_space = new_config_space;
274274
interrupt
275-
.trigger_irq(IrqType::Config)
275+
.trigger(VirtioInterruptType::Config)
276276
.map_err(VhostUserBlockError::IrqTrigger)?;
277277

278278
let delta_us = get_time_us(ClockType::Monotonic) - start_time;
@@ -311,7 +311,7 @@ impl<T: VhostUserHandleBackend + Send + 'static> VirtioDevice for VhostUserBlock
311311
&self.queue_evts
312312
}
313313

314-
fn interrupt_trigger(&self) -> Arc<IrqTrigger> {
314+
fn interrupt_trigger(&self) -> Arc<dyn VirtioInterrupt> {
315315
self.device_state
316316
.active_state()
317317
.expect("Device is not initialized")
@@ -337,7 +337,7 @@ impl<T: VhostUserHandleBackend + Send + 'static> VirtioDevice for VhostUserBlock
337337
fn activate(
338338
&mut self,
339339
mem: GuestMemoryMmap,
340-
interrupt: Arc<IrqTrigger>,
340+
interrupt: Arc<dyn VirtioInterrupt>,
341341
) -> Result<(), ActivateError> {
342342
for q in self.queues.iter_mut() {
343343
q.initialize(&mem)
@@ -353,7 +353,7 @@ impl<T: VhostUserHandleBackend + Send + 'static> VirtioDevice for VhostUserBlock
353353
self.vu_handle.setup_backend(
354354
&mem,
355355
&[(0, &self.queues[0], &self.queue_evts[0])],
356-
&interrupt,
356+
interrupt.clone(),
357357
)
358358
})
359359
.map_err(|err| {
@@ -383,7 +383,7 @@ mod tests {
383383

384384
use super::*;
385385
use crate::devices::virtio::block::virtio::device::FileEngineType;
386-
use crate::devices::virtio::test_utils::default_mem;
386+
use crate::devices::virtio::test_utils::{default_interrupt, default_mem};
387387
use crate::devices::virtio::transport::mmio::VIRTIO_MMIO_INT_CONFIG;
388388
use crate::devices::virtio::vhost_user::tests::create_mem;
389389
use crate::test_utils::create_tmp_socket;
@@ -660,8 +660,7 @@ mod tests {
660660
assert_eq!(vhost_block.config_space, vec![0x69, 0x69, 0x69]);
661661

662662
// Testing [`config_update`]
663-
vhost_block.device_state =
664-
DeviceState::Activated((default_mem(), Arc::new(IrqTrigger::new().unwrap())));
663+
vhost_block.device_state = DeviceState::Activated((default_mem(), default_interrupt()));
665664
vhost_block.config_space = vec![];
666665
vhost_block.config_update().unwrap();
667666
assert_eq!(vhost_block.config_space, vec![0x69, 0x69, 0x69]);
@@ -791,10 +790,11 @@ mod tests {
791790
file.set_len(region_size as u64).unwrap();
792791
let regions = vec![(GuestAddress(0x0), region_size)];
793792
let guest_memory = create_mem(file, &regions);
794-
let interrupt = Arc::new(IrqTrigger::new().unwrap());
795793

796794
// During actiavion of the device features, memory and queues should be set and activated.
797-
vhost_block.activate(guest_memory, interrupt).unwrap();
795+
vhost_block
796+
.activate(guest_memory, default_interrupt())
797+
.unwrap();
798798
assert!(unsafe { *vhost_block.vu_handle.vu.features_are_set.get() });
799799
assert!(unsafe { *vhost_block.vu_handle.vu.memory_is_set.get() });
800800
assert!(unsafe { *vhost_block.vu_handle.vu.vring_enabled.get() });

0 commit comments

Comments
 (0)