Skip to content

Commit 2b5066d

Browse files
committed
refactor: Simplify VirtioDevice trait by combining methods
Combine the `interrupt_evt` and `interrupt_status` methods into a single method `interrupt_trigger` that returns a `IrqTrigger` reference (which essentially combines the two objects originally returned by the status and evt methods). The advantage to this is that `IrqTrigger` exposes a `trigger_irq` method, which I'd like to use in the next commit. Signed-off-by: Patrick Roy <[email protected]>
1 parent dc17a23 commit 2b5066d

File tree

10 files changed

+39
-92
lines changed

10 files changed

+39
-92
lines changed

src/vmm/src/device_manager/mmio.rs

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -193,8 +193,11 @@ impl MMIODeviceManager {
193193
vm.register_ioevent(queue_evt, &io_addr, u32::try_from(i).unwrap())
194194
.map_err(MmioError::RegisterIoEvent)?;
195195
}
196-
vm.register_irqfd(locked_device.interrupt_evt(), device_info.irqs[0])
197-
.map_err(MmioError::RegisterIrqFd)?;
196+
vm.register_irqfd(
197+
&locked_device.interrupt_trigger().irq_evt,
198+
device_info.irqs[0],
199+
)
200+
.map_err(MmioError::RegisterIrqFd)?;
198201
}
199202

200203
self.register_mmio_device(
@@ -513,13 +516,13 @@ impl DeviceInfoForFDT for MMIODeviceInfo {
513516

514517
#[cfg(test)]
515518
mod tests {
516-
use std::sync::atomic::AtomicU32;
519+
517520
use std::sync::Arc;
518521

519522
use utils::eventfd::EventFd;
520523

521524
use super::*;
522-
use crate::devices::virtio::device::VirtioDevice;
525+
use crate::devices::virtio::device::{IrqTrigger, VirtioDevice};
523526
use crate::devices::virtio::queue::Queue;
524527
use crate::devices::virtio::ActivateError;
525528
use crate::utilities::test_utils::multi_region_mem;
@@ -566,7 +569,7 @@ mod tests {
566569
dummy: u32,
567570
queues: Vec<Queue>,
568571
queue_evts: [EventFd; 1],
569-
interrupt_evt: EventFd,
572+
interrupt_trigger: IrqTrigger,
570573
}
571574

572575
impl DummyDevice {
@@ -575,7 +578,7 @@ mod tests {
575578
dummy: 0,
576579
queues: QUEUE_SIZES.iter().map(|&s| Queue::new(s)).collect(),
577580
queue_evts: [EventFd::new(libc::EFD_NONBLOCK).expect("cannot create eventFD")],
578-
interrupt_evt: EventFd::new(libc::EFD_NONBLOCK).expect("cannot create eventFD"),
581+
interrupt_trigger: IrqTrigger::new().expect("cannot create eventFD"),
579582
}
580583
}
581584
}
@@ -607,12 +610,8 @@ mod tests {
607610
&self.queue_evts
608611
}
609612

610-
fn interrupt_evt(&self) -> &EventFd {
611-
&self.interrupt_evt
612-
}
613-
614-
fn interrupt_status(&self) -> Arc<AtomicU32> {
615-
Arc::new(AtomicU32::new(0))
613+
fn interrupt_trigger(&self) -> &IrqTrigger {
614+
&self.interrupt_trigger
616615
}
617616

618617
fn ack_features_by_page(&mut self, page: u32, value: u32) {

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@
22
// SPDX-License-Identifier: Apache-2.0
33

44
use std::fmt;
5-
use std::sync::atomic::AtomicU32;
6-
use std::sync::Arc;
75
use std::time::Duration;
86

97
use log::error;
@@ -584,12 +582,8 @@ impl VirtioDevice for Balloon {
584582
&self.queue_evts
585583
}
586584

587-
fn interrupt_evt(&self) -> &EventFd {
588-
&self.irq_trigger.irq_evt
589-
}
590-
591-
fn interrupt_status(&self) -> Arc<AtomicU32> {
592-
self.irq_trigger.irq_status.clone()
585+
fn interrupt_trigger(&self) -> &IrqTrigger {
586+
&self.irq_trigger
593587
}
594588

595589
fn read_config(&self, offset: u64, data: &mut [u8]) {

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

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

4-
use std::sync::atomic::AtomicU32;
5-
use std::sync::Arc;
6-
74
use event_manager::{EventOps, Events, MutEventSubscriber};
85
use utils::eventfd::EventFd;
96

107
use super::persist::{BlockConstructorArgs, BlockState};
118
use super::vhost_user::device::{VhostUserBlock, VhostUserBlockConfig};
129
use super::virtio::device::{VirtioBlock, VirtioBlockConfig};
1310
use super::BlockError;
14-
use crate::devices::virtio::device::VirtioDevice;
11+
use crate::devices::virtio::device::{IrqTrigger, VirtioDevice};
1512
use crate::devices::virtio::queue::Queue;
1613
use crate::devices::virtio::{ActivateError, TYPE_BLOCK};
1714
use crate::rate_limiter::BucketUpdate;
@@ -176,17 +173,10 @@ impl VirtioDevice for Block {
176173
}
177174
}
178175

179-
fn interrupt_evt(&self) -> &EventFd {
180-
match self {
181-
Self::Virtio(b) => &b.irq_trigger.irq_evt,
182-
Self::VhostUser(b) => &b.irq_trigger.irq_evt,
183-
}
184-
}
185-
186-
fn interrupt_status(&self) -> Arc<AtomicU32> {
176+
fn interrupt_trigger(&self) -> &IrqTrigger {
187177
match self {
188-
Self::Virtio(b) => b.irq_trigger.irq_status.clone(),
189-
Self::VhostUser(b) => b.irq_trigger.irq_status.clone(),
178+
Self::Virtio(b) => &b.irq_trigger,
179+
Self::VhostUser(b) => &b.irq_trigger,
190180
}
191181
}
192182

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
// Portions Copyright 2019 Intel Corporation. All Rights Reserved.
55
// SPDX-License-Identifier: Apache-2.0
66

7-
use std::sync::atomic::AtomicU32;
87
use std::sync::Arc;
98

109
use log::error;
@@ -311,13 +310,8 @@ impl<T: VhostUserHandleBackend + Send + 'static> VirtioDevice for VhostUserBlock
311310
&self.queue_evts
312311
}
313312

314-
fn interrupt_evt(&self) -> &EventFd {
315-
&self.irq_trigger.irq_evt
316-
}
317-
318-
/// Returns the current device interrupt status.
319-
fn interrupt_status(&self) -> Arc<AtomicU32> {
320-
self.irq_trigger.irq_status.clone()
313+
fn interrupt_trigger(&self) -> &IrqTrigger {
314+
&self.irq_trigger
321315
}
322316

323317
fn read_config(&self, offset: u64, data: &mut [u8]) {

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ use std::fs::{File, OpenOptions};
1111
use std::io::{Seek, SeekFrom, Write};
1212
use std::os::linux::fs::MetadataExt;
1313
use std::path::PathBuf;
14-
use std::sync::atomic::AtomicU32;
1514
use std::sync::Arc;
1615

1716
use block_io::FileEngine;
@@ -609,13 +608,8 @@ impl VirtioDevice for VirtioBlock {
609608
&self.queue_evts
610609
}
611610

612-
fn interrupt_evt(&self) -> &EventFd {
613-
&self.irq_trigger.irq_evt
614-
}
615-
616-
/// Returns the current device interrupt status.
617-
fn interrupt_status(&self) -> Arc<AtomicU32> {
618-
self.irq_trigger.irq_status.clone()
611+
fn interrupt_trigger(&self) -> &IrqTrigger {
612+
&self.irq_trigger
619613
}
620614

621615
fn read_config(&self, offset: u64, mut data: &mut [u8]) {

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

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -119,11 +119,12 @@ pub trait VirtioDevice: AsAny + Send {
119119
/// Returns the device queues event fds.
120120
fn queue_events(&self) -> &[EventFd];
121121

122-
/// Returns the device interrupt eventfd.
123-
fn interrupt_evt(&self) -> &EventFd;
124-
125122
/// Returns the current device interrupt status.
126-
fn interrupt_status(&self) -> Arc<AtomicU32>;
123+
fn interrupt_status(&self) -> Arc<AtomicU32> {
124+
Arc::clone(&self.interrupt_trigger().irq_status)
125+
}
126+
127+
fn interrupt_trigger(&self) -> &IrqTrigger;
127128

128129
/// The set of feature bits shifted by `page * 32`.
129130
fn avail_features_by_page(&self, page: u32) -> u32 {
@@ -266,11 +267,7 @@ pub(crate) mod tests {
266267
todo!()
267268
}
268269

269-
fn interrupt_evt(&self) -> &EventFd {
270-
todo!()
271-
}
272-
273-
fn interrupt_status(&self) -> Arc<AtomicU32> {
270+
fn interrupt_trigger(&self) -> &IrqTrigger {
274271
todo!()
275272
}
276273

src/vmm/src/devices/virtio/mmio.rs

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,7 @@ pub(crate) mod tests {
363363
use utils::u64_to_usize;
364364

365365
use super::*;
366+
use crate::devices::virtio::device::IrqTrigger;
366367
use crate::devices::virtio::ActivateError;
367368
use crate::utilities::test_utils::single_region_mem;
368369
use crate::vstate::memory::GuestMemoryMmap;
@@ -371,8 +372,7 @@ pub(crate) mod tests {
371372
pub(crate) struct DummyDevice {
372373
acked_features: u64,
373374
avail_features: u64,
374-
interrupt_evt: EventFd,
375-
interrupt_status: Arc<AtomicU32>,
375+
interrupt_trigger: IrqTrigger,
376376
queue_evts: Vec<EventFd>,
377377
queues: Vec<Queue>,
378378
device_activated: bool,
@@ -384,8 +384,7 @@ pub(crate) mod tests {
384384
DummyDevice {
385385
acked_features: 0,
386386
avail_features: 0,
387-
interrupt_evt: EventFd::new(libc::EFD_NONBLOCK).unwrap(),
388-
interrupt_status: Arc::new(AtomicU32::new(0)),
387+
interrupt_trigger: IrqTrigger::new().unwrap(),
389388
queue_evts: vec![
390389
EventFd::new(libc::EFD_NONBLOCK).unwrap(),
391390
EventFd::new(libc::EFD_NONBLOCK).unwrap(),
@@ -430,12 +429,8 @@ pub(crate) mod tests {
430429
&self.queue_evts
431430
}
432431

433-
fn interrupt_evt(&self) -> &EventFd {
434-
&self.interrupt_evt
435-
}
436-
437-
fn interrupt_status(&self) -> Arc<AtomicU32> {
438-
self.interrupt_status.clone()
432+
fn interrupt_trigger(&self) -> &IrqTrigger {
433+
&self.interrupt_trigger
439434
}
440435

441436
fn read_config(&self, offset: u64, data: &mut [u8]) {

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
use std::io::Read;
1010
use std::mem;
1111
use std::net::Ipv4Addr;
12-
use std::sync::atomic::AtomicU32;
1312
use std::sync::{Arc, Mutex};
1413

1514
use libc::EAGAIN;
@@ -823,14 +822,9 @@ impl VirtioDevice for Net {
823822
&self.queue_evts
824823
}
825824

826-
fn interrupt_evt(&self) -> &EventFd {
827-
&self.irq_trigger.irq_evt
825+
fn interrupt_trigger(&self) -> &IrqTrigger {
826+
&self.irq_trigger
828827
}
829-
830-
fn interrupt_status(&self) -> Arc<AtomicU32> {
831-
self.irq_trigger.irq_status.clone()
832-
}
833-
834828
fn read_config(&self, offset: u64, data: &mut [u8]) {
835829
if let Some(config_space_bytes) = self.config_space.as_slice().get(u64_to_usize(offset)..) {
836830
let len = config_space_bytes.len().min(data.len());

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

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -259,12 +259,8 @@ impl VirtioDevice for Entropy {
259259
&self.queue_events
260260
}
261261

262-
fn interrupt_evt(&self) -> &EventFd {
263-
&self.irq_trigger.irq_evt
264-
}
265-
266-
fn interrupt_status(&self) -> Arc<AtomicU32> {
267-
self.irq_trigger.irq_status.clone()
262+
fn interrupt_trigger(&self) -> &IrqTrigger {
263+
&self.irq_trigger
268264
}
269265

270266
fn avail_features(&self) -> u64 {

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

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
// found in the THIRD-PARTY file.
77

88
use std::fmt::Debug;
9+
910
/// This is the `VirtioDevice` implementation for our vsock device. It handles the virtio-level
1011
/// device logic: feature negociation, device configuration, and device activation.
1112
///
@@ -20,9 +21,6 @@ use std::fmt::Debug;
2021
/// - a TX queue FD;
2122
/// - an event queue FD; and
2223
/// - a backend FD.
23-
use std::sync::atomic::AtomicU32;
24-
use std::sync::Arc;
25-
2624
use log::{error, warn};
2725
use utils::byte_order;
2826
use utils::eventfd::EventFd;
@@ -290,12 +288,8 @@ where
290288
&self.queue_events
291289
}
292290

293-
fn interrupt_evt(&self) -> &EventFd {
294-
&self.irq_trigger.irq_evt
295-
}
296-
297-
fn interrupt_status(&self) -> Arc<AtomicU32> {
298-
self.irq_trigger.irq_status.clone()
291+
fn interrupt_trigger(&self) -> &IrqTrigger {
292+
&self.irq_trigger
299293
}
300294

301295
fn read_config(&self, offset: u64, data: &mut [u8]) {

0 commit comments

Comments
 (0)