Devices: clean up interrupt sending code

src/devices/Cargo.toml

@@ -13,6 +13,7 @@ efi = ["blk", "net"]
 gpu = ["rutabaga_gfx", "thiserror", "zerocopy", "zerocopy-derive"]
 snd = ["pw", "thiserror"]
 virgl_resource_map2 = []
+test_utils = []
 
 [dependencies]
 bitflags = "1.2.0"

src/devices/src/legacy/irqchip.rs

@@ -94,3 +94,43 @@ pub trait IrqChipT: BusDevice + GICDevice {
         interrupt_evt: Option<&EventFd>,
     ) -> Result<(), DeviceError>;
 }
+
+#[cfg(any(test, feature = "test_utils"))]
+pub mod test_utils {
+    use super::*;
+
+    #[derive(Clone, Default, Debug)]
+    pub struct DummyIrqChip {}
+
+    impl DummyIrqChip {
+        pub fn new() -> Self {
+            Default::default()
+        }
+    }
+
+    impl Into<IrqChip> for DummyIrqChip {
+        fn into(self) -> IrqChip {
+            Arc::new(Mutex::new(IrqChipDevice::new(
+                Box::new(DummyIrqChip::new()),
+            )))
+        }
+    }
+
+    impl BusDevice for DummyIrqChip {}
+
+    impl IrqChipT for DummyIrqChip {
+        fn get_mmio_addr(&self) -> u64 {
+            0
+        }
+        fn get_mmio_size(&self) -> u64 {
+            0
+        }
+        fn set_irq(
+            &self,
+            _irq_line: Option<u32>,
+            _interrupt_evt: Option<&EventFd>,
+        ) -> Result<(), DeviceError> {
+            Ok(())
+        }
+    }
+}

src/devices/src/legacy/mod.rs

@@ -43,6 +43,8 @@ pub use self::i8042::Error as I8042DeviceError;
 pub use self::i8042::I8042Device;
 #[cfg(all(target_os = "linux", target_arch = "x86_64"))]
 pub use self::ioapic::IoApic;
+#[cfg(any(test, feature = "test_utils"))]
+pub use self::irqchip::test_utils::DummyIrqChip;
 pub use self::irqchip::{IrqChip, IrqChipDevice, IrqChipT};
 #[cfg(all(target_os = "linux", target_arch = "aarch64"))]
 pub use self::kvmgicv3::KvmGicV3;

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

@@ -1,20 +1,15 @@
 use std::cmp;
 use std::convert::TryInto;
 use std::io::Write;
-use std::result;
-use std::sync::atomic::{AtomicUsize, Ordering};
-use std::sync::Arc;
 
 use utils::eventfd::EventFd;
 use vm_memory::{ByteValued, GuestMemory, GuestMemoryMmap};
 
 use super::super::{
     ActivateError, ActivateResult, BalloonError, DeviceState, Queue as VirtQueue, VirtioDevice,
-    VIRTIO_MMIO_INT_VRING,
 };
 use super::{defs, defs::uapi};
-use crate::legacy::IrqChip;
-use crate::Error as DeviceError;
+use crate::virtio::InterruptTransport;
 
 // Inflate queue.
 pub(crate) const IFQ_INDEX: usize = 0;
@@ -54,13 +49,9 @@ pub struct Balloon {
     pub(crate) queue_events: Vec<EventFd>,
     pub(crate) avail_features: u64,
     pub(crate) acked_features: u64,
-    pub(crate) interrupt_status: Arc<AtomicUsize>,
-    pub(crate) interrupt_evt: EventFd,
     pub(crate) activate_evt: EventFd,
     pub(crate) device_state: DeviceState,
     config: VirtioBalloonConfig,
-    intc: Option<IrqChip>,
-    irq_line: Option<u32>,
 }
 
 impl Balloon {
@@ -78,15 +69,10 @@ impl Balloon {
             queue_events,
             avail_features: AVAIL_FEATURES,
             acked_features: 0,
-            interrupt_status: Arc::new(AtomicUsize::new(0)),
-            interrupt_evt: EventFd::new(utils::eventfd::EFD_NONBLOCK)
-                .map_err(BalloonError::EventFd)?,
             activate_evt: EventFd::new(utils::eventfd::EFD_NONBLOCK)
                 .map_err(BalloonError::EventFd)?,
             device_state: DeviceState::Inactive,
             config,
-            intc: None,
-            irq_line: None,
         })
     }
 
@@ -102,26 +88,10 @@ impl Balloon {
         defs::BALLOON_DEV_ID
     }
 
-    pub fn set_intc(&mut self, intc: IrqChip) {
-        self.intc = Some(intc);
-    }
-
-    pub fn signal_used_queue(&self) -> result::Result<(), DeviceError> {
-        debug!("balloon: raising IRQ");
-        self.interrupt_status
-            .fetch_or(VIRTIO_MMIO_INT_VRING as usize, Ordering::SeqCst);
-        if let Some(intc) = &self.intc {
-            intc.lock()
-                .unwrap()
-                .set_irq(self.irq_line, Some(&self.interrupt_evt))?;
-        }
-        Ok(())
-    }
-
     pub fn process_frq(&mut self) -> bool {
         debug!("balloon: process_frq()");
         let mem = match self.device_state {
-            DeviceState::Activated(ref mem) => mem,
+            DeviceState::Activated(ref mem, _) => mem,
             // This should never happen, it's been already validated in the event handler.
             DeviceState::Inactive => unreachable!(),
         };
@@ -172,6 +142,10 @@ impl VirtioDevice for Balloon {
         uapi::VIRTIO_ID_BALLOON
     }
 
+    fn device_name(&self) -> &str {
+        "balloon"
+    }
+
     fn queues(&self) -> &[VirtQueue] {
         &self.queues
     }
@@ -184,19 +158,6 @@ impl VirtioDevice for Balloon {
         &self.queue_events
     }
 
-    fn interrupt_evt(&self) -> &EventFd {
-        &self.interrupt_evt
-    }
-
-    fn interrupt_status(&self) -> Arc<AtomicUsize> {
-        self.interrupt_status.clone()
-    }
-
-    fn set_irq_line(&mut self, irq: u32) {
-        debug!("SET_IRQ_LINE (BALLOON)={}", irq);
-        self.irq_line = Some(irq);
-    }
-
     fn read_config(&self, offset: u64, mut data: &mut [u8]) {
         let config_slice = self.config.as_slice();
         let config_len = config_slice.len() as u64;
@@ -219,7 +180,7 @@ impl VirtioDevice for Balloon {
         );
     }
 
-    fn activate(&mut self, mem: GuestMemoryMmap) -> ActivateResult {
+    fn activate(&mut self, mem: GuestMemoryMmap, interrupt: InterruptTransport) -> ActivateResult {
         if self.queues.len() != defs::NUM_QUEUES {
             error!(
                 "Cannot perform activate. Expected {} queue(s), got {}",
@@ -234,15 +195,12 @@ impl VirtioDevice for Balloon {
             return Err(ActivateError::BadActivate);
         }
 
-        self.device_state = DeviceState::Activated(mem);
+        self.device_state = DeviceState::Activated(mem, interrupt);
 
         Ok(())
     }
 
     fn is_activated(&self) -> bool {
-        match self.device_state {
-            DeviceState::Inactive => false,
-            DeviceState::Activated(_) => true,
-        }
+        self.device_state.is_activated()
     }
 }

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

@@ -81,9 +81,7 @@ impl Balloon {
                 e
             );
         } else if self.process_frq() {
-            if let Err(e) = self.signal_used_queue() {
-                warn!("Failed to signal queue: {e:?}");
-            }
+            self.device_state.signal_used_queue();
         }
     }
 

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

@@ -15,7 +15,6 @@ use std::os::linux::fs::MetadataExt;
 use std::os::macos::fs::MetadataExt;
 use std::path::PathBuf;
 use std::result;
-use std::sync::atomic::AtomicUsize;
 use std::sync::Arc;
 use std::thread::JoinHandle;
 
@@ -35,8 +34,7 @@ use super::{
     Error, QUEUE_SIZES, SECTOR_SHIFT, SECTOR_SIZE,
 };
 
-use crate::legacy::IrqChip;
-use crate::virtio::{block::ImageType, ActivateError};
+use crate::virtio::{block::ImageType, ActivateError, InterruptTransport};
 
 /// Configuration options for disk caching.
 #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
@@ -191,18 +189,12 @@ pub struct Block {
 
     // Transport related fields.
     pub(crate) queues: Vec<Queue>,
-    pub(crate) interrupt_status: Arc<AtomicUsize>,
-    pub(crate) interrupt_evt: EventFd,
     pub(crate) queue_evts: [EventFd; 1],
     pub(crate) device_state: DeviceState,
 
     // Implementation specific fields.
     pub(crate) id: String,
     pub(crate) partuuid: Option<String>,
-
-    // Interrupt specific fields.
-    intc: Option<IrqChip>,
-    irq_line: Option<u32>,
 }
 
 impl Block {
@@ -271,22 +263,14 @@ impl Block {
             disk_image_id,
             avail_features,
             acked_features: 0u64,
-            interrupt_status: Arc::new(AtomicUsize::new(0)),
-            interrupt_evt: EventFd::new(EFD_NONBLOCK)?,
             queue_evts,
             queues,
             device_state: DeviceState::Inactive,
-            intc: None,
-            irq_line: None,
             worker_thread: None,
             worker_stopfd: EventFd::new(EFD_NONBLOCK)?,
         })
     }
 
-    pub fn set_intc(&mut self, intc: IrqChip) {
-        self.intc = Some(intc);
-    }
-
     /// Provides the ID of this block device.
     pub fn id(&self) -> &String {
         &self.id
@@ -308,6 +292,10 @@ impl VirtioDevice for Block {
         TYPE_BLOCK
     }
 
+    fn device_name(&self) -> &str {
+        "block"
+    }
+
     fn queues(&self) -> &[Queue] {
         &self.queues
     }
@@ -320,20 +308,6 @@ impl VirtioDevice for Block {
         &self.queue_evts
     }
 
-    fn interrupt_evt(&self) -> &EventFd {
-        &self.interrupt_evt
-    }
-
-    /// Returns the current device interrupt status.
-    fn interrupt_status(&self) -> Arc<AtomicUsize> {
-        self.interrupt_status.clone()
-    }
-
-    fn set_irq_line(&mut self, irq: u32) {
-        debug!("SET_IRQ_LINE (BLOCK)={}", irq);
-        self.irq_line = Some(irq);
-    }
-
     fn avail_features(&self) -> u64 {
         self.avail_features
     }
@@ -365,13 +339,10 @@ impl VirtioDevice for Block {
     }
 
     fn is_activated(&self) -> bool {
-        match self.device_state {
-            DeviceState::Inactive => false,
-            DeviceState::Activated(_) => true,
-        }
+        self.device_state.is_activated()
     }
 
-    fn activate(&mut self, mem: GuestMemoryMmap) -> ActivateResult {
+    fn activate(&mut self, mem: GuestMemoryMmap, interrupt: InterruptTransport) -> ActivateResult {
         if self.worker_thread.is_some() {
             panic!("virtio_blk: worker thread already exists");
         }
@@ -392,17 +363,14 @@ impl VirtioDevice for Block {
         let worker = BlockWorker::new(
             self.queues[0].clone(),
             self.queue_evts[0].try_clone().unwrap(),
-            self.interrupt_status.clone(),
-            self.interrupt_evt.try_clone().unwrap(),
-            self.intc.clone(),
-            self.irq_line,
+            interrupt.clone(),
             mem.clone(),
             disk,
             self.worker_stopfd.try_clone().unwrap(),
         );
         self.worker_thread = Some(worker.run());
 
-        self.device_state = DeviceState::Activated(mem);
+        self.device_state = DeviceState::Activated(mem, interrupt);
         Ok(())
     }