diff --git a/alioth/src/hv/hv_test.rs b/alioth/src/hv/hv_test.rs index 46dc990e..3140ae96 100644 --- a/alioth/src/hv/hv_test.rs +++ b/alioth/src/hv/hv_test.rs @@ -85,7 +85,7 @@ impl IrqFd for TestIrqFd { impl AsFd for TestIrqFd { fn as_fd(&self) -> BorrowedFd<'_> { - unreachable!() + unsafe { BorrowedFd::borrow_raw(0) } } } diff --git a/alioth/src/virtio/pci.rs b/alioth/src/virtio/pci.rs index f83d6b3d..648ac110 100644 --- a/alioth/src/virtio/pci.rs +++ b/alioth/src/virtio/pci.rs @@ -462,7 +462,15 @@ where if let Some(q) = self.queues.get(q_sel) && !q.enabled.load(Ordering::Acquire) { - q.size.store(val as u16, Ordering::Release); + if val.is_power_of_two() + || VirtioFeature(self.reg.get_driver_feature()) + .contains(VirtioFeature::RING_PACKED) + && val > 0 + { + q.size.store(val as u16, Ordering::Release); + } else { + log::warn!("{}: queue {q_sel}: invalid queue size: {val}", self.name); + } } } VirtioCommonCfg::LAYOUT_QUEUE_MSIX_VECTOR => { diff --git a/alioth/src/virtio/pci_test.rs b/alioth/src/virtio/pci_test.rs index ea3eec238..84aa4799 100644 --- a/alioth/src/virtio/pci_test.rs +++ b/alioth/src/virtio/pci_test.rs @@ -12,15 +12,17 @@ // See the License for the specific language governing permissions and // limitations under the License. +use std::mem::size_of; use std::sync::Arc; -use std::sync::atomic::{AtomicU16, AtomicU64, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicU16, AtomicU64, Ordering}; use assert_matches::assert_matches; use parking_lot::RwLock; +use rstest::rstest; -use crate::hv::tests::TestMsiSender; +use crate::hv::tests::{TestIrqFd, TestMsiSender}; use crate::mem::emulated::{Action, Mmio}; -use crate::pci::cap::MsixTableMmio; +use crate::pci::cap::{MsixTableEntry, MsixTableMmio, MsixTableMmioEntry}; use crate::sync::notifier::Notifier; use crate::virtio::dev::{Register, WakeEvent}; use crate::virtio::pci::{ @@ -67,138 +69,349 @@ fn create_test_mmio(queues: Arc<[QueueReg]>) -> (TestMmio, TestWakeReceiver) { (mmio, event_rx) } -#[test] -fn test_virtio_pci_queue_registers() { +#[rstest] +#[case(VirtioCommonCfg::OFFSET_QUEUE_DESC_LO, 0x5566_7788)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_DESC_HI, 0x1122_3344)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_DRIVER_LO, 0xeeff_0011)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_DRIVER_HI, 0xaabb_ccdd)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_DEVICE_LO, 0x89ab_cdef)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_DEVICE_HI, 0x0123_4567)] +fn test_queue_address_reads(#[case] offset: usize, #[case] expected: u64) { let queues = Arc::new([QueueReg { desc: AtomicU64::new(0x1122_3344_5566_7788), driver: AtomicU64::new(0xaabb_ccdd_eeff_0011), device: AtomicU64::new(0x0123_4567_89ab_cdef), ..Default::default() }]); - let (mmio, event_rx) = create_test_mmio(queues.clone()); + let (mmio, _) = create_test_mmio(queues); + assert_matches!(mmio.read(offset as u64, 4), Ok(val) if val == expected); +} + +#[rstest] +// QUEUE_DESC_LO: must be 16-byte aligned +#[case(false, VirtioCommonCfg::OFFSET_QUEUE_DESC_LO, 0x1008, 0x1000_0000)] +#[case(false, VirtioCommonCfg::OFFSET_QUEUE_DESC_LO, 0x1010, 0x1010)] +// QUEUE_DEVICE_LO: must be 4-byte aligned +#[case(false, VirtioCommonCfg::OFFSET_QUEUE_DEVICE_LO, 0x3002, 0x3000_0000)] +#[case(false, VirtioCommonCfg::OFFSET_QUEUE_DEVICE_LO, 0x3004, 0x3004)] +// split queue: QUEUE_DRIVER_LO: must be 2-byte aligned +#[case(false, VirtioCommonCfg::OFFSET_QUEUE_DRIVER_LO, 0x2001, 0x2000_0000)] +#[case(false, VirtioCommonCfg::OFFSET_QUEUE_DRIVER_LO, 0x2002, 0x2002)] +// packed queue: QUEUE_DRIVER_LO: must be 4-byte aligned +#[case(true, VirtioCommonCfg::OFFSET_QUEUE_DRIVER_LO, 0x4002, 0x2000_0000)] +#[case(true, VirtioCommonCfg::OFFSET_QUEUE_DRIVER_LO, 0x4004, 0x4004)] +fn test_queue_alignment( + #[case] packed: bool, + #[case] offset: usize, + #[case] input: u64, + #[case] expected: u64, +) { + let queues = Arc::new([QueueReg { + desc: AtomicU64::new(0x1000_0000), + driver: AtomicU64::new(0x2000_0000), + device: AtomicU64::new(0x3000_0000), + ..Default::default() + }]); + let (mmio, _event_rx) = create_test_mmio(queues); + if packed { + assert_matches!( + mmio.write(VirtioCommonCfg::OFFSET_DRIVER_FEATURE_SELECT as u64, 4, 1), + Ok(Action::None) + ); + assert_matches!( + mmio.write( + VirtioCommonCfg::OFFSET_DRIVER_FEATURE as u64, + 4, + VirtioFeature::RING_PACKED.bits() as u64 >> 32 + ), + Ok(Action::None) + ); + } + assert_matches!(mmio.write(offset as u64, 4, input), Ok(Action::None)); + assert_matches!(mmio.read(offset as u64, 4), Ok(val) if val == expected); +} + +#[rstest] +#[case(VirtioCommonCfg::OFFSET_QUEUE_SIZE, 2, 128)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_DESC_HI, 4, 0x1122_3344)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_DRIVER_HI, 4, 0x5566_7788)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_DEVICE_HI, 4, 0x99aa_bbcc)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_ENABLE, 2, 1)] +fn test_queue_registers_write_read(#[case] offset: usize, #[case] size: u8, #[case] val: u64) { + let queues = Arc::new([QueueReg::default()]); + let (mmio, _) = create_test_mmio(queues); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_QUEUE_DESC_LO.0 as u64, 4), - Ok(0x5566_7788) + mmio.write(VirtioCommonCfg::OFFSET_QUEUE_SELECT as u64, 2, 0), + Ok(Action::None) ); + assert_matches!(mmio.write(offset as u64, size, val), Ok(Action::None)); + assert_matches!(mmio.read(offset as u64, size), Ok(v) if v == val); +} + +#[rstest] +#[case(false, 64, 64)] +#[case(false, 80, 128)] +#[case(true, 80, 80)] +#[case(true, 0, 128)] +fn test_queue_size_write_read(#[case] packed: bool, #[case] input: u64, #[case] expected: u64) { + let queues = Arc::new([QueueReg { + size: AtomicU16::new(128), + ..Default::default() + }]); + let (mmio, _event_rx) = create_test_mmio(queues); + if packed { + assert_matches!( + mmio.write(VirtioCommonCfg::OFFSET_DRIVER_FEATURE_SELECT as u64, 4, 1), + Ok(Action::None) + ); + assert_matches!( + mmio.write( + VirtioCommonCfg::OFFSET_DRIVER_FEATURE as u64, + 4, + VirtioFeature::RING_PACKED.bits() as u64 >> 32 + ), + Ok(Action::None) + ); + } assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_QUEUE_DESC_HI.0 as u64, 4), - Ok(0x1122_3344) + mmio.write(VirtioCommonCfg::OFFSET_QUEUE_SIZE as u64, 2, input), + Ok(Action::None) ); + assert_matches!(mmio.read(VirtioCommonCfg::OFFSET_QUEUE_SIZE as u64, 2), Ok(val) if val == expected); +} + +#[test] +fn test_queue_size_locked_when_enabled() { + let queues = Arc::new([QueueReg::default()]); + let (mmio, _) = create_test_mmio(queues); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_QUEUE_DRIVER_LO.0 as u64, 4), - Ok(0xeeff_0011) + mmio.write(VirtioCommonCfg::OFFSET_QUEUE_SELECT as u64, 2, 0), + Ok(Action::None) ); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_QUEUE_DRIVER_HI.0 as u64, 4), - Ok(0xaabb_ccdd) + mmio.write(VirtioCommonCfg::OFFSET_QUEUE_ENABLE as u64, 2, 1), + Ok(Action::None) ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_QUEUE_DEVICE_LO.0 as u64, 4), - Ok(0x89ab_cdef) + mmio.write(VirtioCommonCfg::OFFSET_QUEUE_SIZE as u64, 2, 0xffff), + Ok(Action::None) ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_QUEUE_DEVICE_HI.0 as u64, 4), - Ok(0x0123_4567) + assert_ne!( + mmio.read(VirtioCommonCfg::OFFSET_QUEUE_SIZE as u64, 2) + .unwrap(), + 0xffff ); +} + +#[rstest] +#[case(VirtioCommonCfg::OFFSET_QUEUE_SIZE, 2, 64)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_DESC_LO, 4, 0x1000)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_DESC_HI, 4, 0x1000)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_DRIVER_LO, 4, 0x2000)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_DRIVER_HI, 4, 0x2000)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_DEVICE_LO, 4, 0x3000)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_DEVICE_HI, 4, 0x3000)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_ENABLE, 2, 1)] +fn test_out_of_bounds_queue_writes(#[case] offset: usize, #[case] size: u8, #[case] val: u64) { + let queues = Arc::new([QueueReg::default()]); + let (mmio, _) = create_test_mmio(queues); - // Feature select 32-bit round trip assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_FEATURE_SELECT.0 as u64, - 4, - 0xdead_beef - ), + mmio.write(VirtioCommonCfg::OFFSET_QUEUE_SELECT as u64, 2, 99), Ok(Action::None) ); + assert_matches!(mmio.write(offset as u64, size, val), Ok(Action::None)); +} + +#[rstest] +// Queue size +#[case(0, VirtioCommonCfg::OFFSET_QUEUE_SIZE, 2, 128)] +#[case(1, VirtioCommonCfg::OFFSET_QUEUE_SIZE, 2, 256)] +#[case(99, VirtioCommonCfg::OFFSET_QUEUE_SIZE, 2, 0)] +// Queue enable +#[case(0, VirtioCommonCfg::OFFSET_QUEUE_ENABLE, 2, 1)] +#[case(1, VirtioCommonCfg::OFFSET_QUEUE_ENABLE, 2, 0)] +#[case(99, VirtioCommonCfg::OFFSET_QUEUE_ENABLE, 2, 0)] +// Queue MSI-X vector +#[case(0, VirtioCommonCfg::OFFSET_QUEUE_MSIX_VECTOR, 2, VIRTIO_MSI_NO_VECTOR as u64)] +#[case(99, VirtioCommonCfg::OFFSET_QUEUE_MSIX_VECTOR, 2, VIRTIO_MSI_NO_VECTOR as u64)] +// Queue notify offset: valid index vs capped/out-of-bounds +#[case(0, VirtioCommonCfg::OFFSET_QUEUE_NOTIFY_OFF, 2, 0)] +#[case(1, VirtioCommonCfg::OFFSET_QUEUE_NOTIFY_OFF, 2, 1)] +#[case(5, VirtioCommonCfg::OFFSET_QUEUE_NOTIFY_OFF, 2, 2)] +// Out-of-bounds queue reads for descriptor / driver / device areas +#[case(99, VirtioCommonCfg::OFFSET_QUEUE_DESC_LO, 4, 0)] +#[case(99, VirtioCommonCfg::OFFSET_QUEUE_DESC_HI, 4, 0)] +#[case(99, VirtioCommonCfg::OFFSET_QUEUE_DRIVER_LO, 4, 0)] +#[case(99, VirtioCommonCfg::OFFSET_QUEUE_DRIVER_HI, 4, 0)] +#[case(99, VirtioCommonCfg::OFFSET_QUEUE_DEVICE_LO, 4, 0)] +#[case(99, VirtioCommonCfg::OFFSET_QUEUE_DEVICE_HI, 4, 0)] +fn test_common_cfg_queue_reads( + #[case] q_sel: u64, + #[case] offset: usize, + #[case] size: u8, + #[case] expected: u64, +) { + let queues = Arc::new([ + QueueReg { + size: AtomicU16::new(128), + enabled: AtomicBool::new(true), + ..Default::default() + }, + QueueReg { + size: AtomicU16::new(256), + ..Default::default() + }, + ]); + let (mmio, _) = create_test_mmio(queues); + assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_FEATURE_SELECT.0 as u64, 4), - Ok(0xdead_beef) + mmio.write(VirtioCommonCfg::OFFSET_QUEUE_SELECT as u64, 2, q_sel), + Ok(Action::None) ); + assert_matches!(mmio.read(VirtioCommonCfg::OFFSET_QUEUE_SELECT as u64, 2), Ok(val) if val == q_sel); + assert_matches!(mmio.read(offset as u64, size), Ok(val) if val == expected); +} + +#[rstest] +#[case(VirtioCommonCfg::OFFSET_NUM_QUEUES, 2, 2)] +#[case(VirtioCommonCfg::OFFSET_CONFIG_GENERATION, 1, 0)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_NOTIFY_DATA, 2, 0)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_RESET, 2, 0)] +fn test_common_cfg_misc_reads(#[case] offset: usize, #[case] size: u8, #[case] expected: u64) { + let queues = Arc::new([QueueReg::default(), QueueReg::default()]); + let (mmio, _) = create_test_mmio(queues); + assert_matches!(mmio.read(offset as u64, size), Ok(val) if val == expected); +} + +#[rstest] +#[case(0, 0x1111_2222)] +#[case(1, 0x3333_4444)] +#[case(10, 0)] +fn test_device_feature_reads(#[case] sel: u64, #[case] expected_feature: u64) { + let queues = Arc::new([QueueReg::default()]); + let (mut mmio, _) = create_test_mmio(queues); + mmio.reg.device_feature = [0x1111_2222, 0x3333_4444, 0, 0]; assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_QUEUE_NOTIFY_DATA.0 as u64, 2), - Ok(0) + mmio.write(VirtioCommonCfg::OFFSET_DEVICE_FEATURE_SELECT as u64, 4, sel), + Ok(Action::None) ); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_QUEUE_RESET.0 as u64, 2), - Ok(0) + mmio.read(VirtioCommonCfg::OFFSET_DEVICE_FEATURE_SELECT as u64, 4), + Ok(s) if s == sel ); assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_QUEUE_RESET.0 as u64, 2, 1), - Ok(Action::None) + mmio.read(VirtioCommonCfg::OFFSET_DEVICE_FEATURE as u64, 4), + Ok(f) if f == expected_feature ); +} + +#[rstest] +// Bank 0: only offered device features are accepted +#[case(0, 0xffff_ffff, 0x1234_5678)] +// Bank 1: only offered device features are accepted +#[case(1, 0xffff_ffff, 0x0000_0005)] +// Bank 2 (no device features offered): writes are masked to 0 +#[case(2, 0xffff_ffff, 0)] +// Out-of-bounds bank selection does not store and does not panic +#[case(10, 0x1234, 0)] +fn test_driver_features(#[case] bank: u64, #[case] write_val: u64, #[case] expected: u64) { + let queues = Arc::new([QueueReg::default()]); + let (mut mmio, _event_rx) = create_test_mmio(queues); + mmio.reg.device_feature = [0x1234_5678, 0x0000_0005, 0, 0]; - // Queue size cannot be modified while queue is enabled assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_QUEUE_SELECT.0 as u64, 2, 0), + mmio.write( + VirtioCommonCfg::OFFSET_DRIVER_FEATURE_SELECT as u64, + 4, + bank + ), Ok(Action::None) ); assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_QUEUE_ENABLE.0 as u64, 2, 1), - Ok(Action::None) + mmio.read(VirtioCommonCfg::OFFSET_DRIVER_FEATURE_SELECT as u64, 4,), + Ok(val) if val == bank ); assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_QUEUE_SIZE.0 as u64, 2, 0xffff), + mmio.write(VirtioCommonCfg::OFFSET_DRIVER_FEATURE as u64, 4, write_val), Ok(Action::None) ); - assert_ne!( - mmio.read(VirtioCommonCfg::LAYOUT_QUEUE_SIZE.0 as u64, 2) - .unwrap(), - 0xffff + assert_matches!( + mmio.read(VirtioCommonCfg::OFFSET_DRIVER_FEATURE as u64, 4), + Ok(val) if val == expected ); +} + +#[test] +fn test_driver_features_locked_after_features_ok() { + let queues = Arc::new([QueueReg::default()]); + let (mut mmio, _event_rx) = create_test_mmio(queues); + mmio.reg.device_feature = [0x1234_5678, 0x0000_0005, 0, 0]; - // Valid queue notify (to valid queue offset) wakes up the queue assert_matches!( - mmio.write(VirtioPciRegister::OFFSET_QUEUE_NOTIFY as u64, 2, 0), + mmio.write(VirtioCommonCfg::OFFSET_DRIVER_FEATURE_SELECT as u64, 4, 0), Ok(Action::None) ); - assert_matches!(event_rx.try_recv(), Ok(WakeEvent::Notify { q_index: 0 })); - - // Queue notify offset for out of bounds queue returns the reserved slot (queues.len()) assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_QUEUE_SELECT.0 as u64, 2, 0xffff), + mmio.write( + VirtioCommonCfg::OFFSET_DRIVER_FEATURE as u64, + 4, + 0xffff_ffff + ), Ok(Action::None) ); + + // Set status to ACK | DRIVER | FEATURES_OK + let features_ok = DevStatus::ACK | DevStatus::DRIVER | DevStatus::FEATURES_OK; assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_QUEUE_NOTIFY_OFF.0 as u64, 2), - Ok(1) + mmio.write( + VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, + 1, + features_ok.bits() as u64 + ), + Ok(Action::None) ); - // Queue notify to reserved invalid slot does not wake up any queue - let invalid_notify_offset = - VirtioPciRegister::OFFSET_QUEUE_NOTIFY + size_of::() * queues.len(); + // Feature writes after FEATURES_OK must be ignored assert_matches!( - mmio.write(invalid_notify_offset as u64, 2, 0), + mmio.write(VirtioCommonCfg::OFFSET_DRIVER_FEATURE_SELECT as u64, 4, 0), Ok(Action::None) ); - assert!(event_rx.is_empty()); + assert_matches!( + mmio.write(VirtioCommonCfg::OFFSET_DRIVER_FEATURE as u64, 4, 0), + Ok(Action::None) + ); + assert_matches!( + mmio.read(VirtioCommonCfg::OFFSET_DRIVER_FEATURE as u64, 4), + Ok(0x1234_5678) + ); } #[test] -fn test_virtio_pci_device_status_valid_transitions() { +fn test_device_status_valid_transitions() { let queues = Arc::new([QueueReg::default()]); let (mmio, event_rx) = create_test_mmio(queues.clone()); // Initially status is 0 (empty) assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), + mmio.read(VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1), Ok(0) ); // Transition 0 -> ACK assert_matches!( mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, + VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1, DevStatus::ACK.bits() as u64 ), Ok(Action::None) ); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), + mmio.read(VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1), Ok(status) if status == DevStatus::ACK.bits() as u64 ); assert!(event_rx.is_empty()); @@ -207,38 +420,38 @@ fn test_virtio_pci_device_status_valid_transitions() { let ack_driver = DevStatus::ACK | DevStatus::DRIVER; assert_matches!( mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, + VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1, ack_driver.bits() as u64 ), Ok(Action::None) ); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), + mmio.read(VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1), Ok(status) if status == ack_driver.bits() as u64 ); assert!(event_rx.is_empty()); // Set driver features assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_DRIVER_FEATURE_SELECT.0 as u64, 4, 0), + mmio.write(VirtioCommonCfg::OFFSET_DRIVER_FEATURE_SELECT as u64, 4, 0), Ok(Action::None) ); assert_matches!( mmio.write( - VirtioCommonCfg::LAYOUT_DRIVER_FEATURE.0 as u64, + VirtioCommonCfg::OFFSET_DRIVER_FEATURE as u64, 4, 0x1234_5678 ), Ok(Action::None) ); assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_DRIVER_FEATURE_SELECT.0 as u64, 4, 1), + mmio.write(VirtioCommonCfg::OFFSET_DRIVER_FEATURE_SELECT as u64, 4, 1), Ok(Action::None) ); assert_matches!( mmio.write( - VirtioCommonCfg::LAYOUT_DRIVER_FEATURE.0 as u64, + VirtioCommonCfg::OFFSET_DRIVER_FEATURE as u64, 4, 0x9abc_def0 ), @@ -249,14 +462,14 @@ fn test_virtio_pci_device_status_valid_transitions() { let ack_driver_features = ack_driver | DevStatus::FEATURES_OK; assert_matches!( mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, + VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1, ack_driver_features.bits() as u64 ), Ok(Action::None) ); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), + mmio.read(VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1), Ok(status) if status == ack_driver_features.bits() as u64 ); assert!(event_rx.is_empty()); @@ -265,14 +478,14 @@ fn test_virtio_pci_device_status_valid_transitions() { let all_ok = ack_driver_features | DevStatus::DRIVER_OK; assert_matches!( mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, + VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1, all_ok.bits() as u64 ), Ok(Action::None) ); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), + mmio.read(VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1), Ok(status) if status == all_ok.bits() as u64 ); assert_matches!( @@ -286,14 +499,14 @@ fn test_virtio_pci_device_status_valid_transitions() { // Rewriting same status is idempotent and does not send duplicate WakeEvent::Start assert_matches!( mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, + VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1, all_ok.bits() as u64 ), Ok(Action::None) ); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), + mmio.read(VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1), Ok(status) if status == all_ok.bits() as u64 ); assert!(event_rx.is_empty()); @@ -302,14 +515,14 @@ fn test_virtio_pci_device_status_valid_transitions() { let failed = all_ok | DevStatus::FAILED; assert_matches!( mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, + VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1, failed.bits() as u64 ), Ok(Action::None) ); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), + mmio.read(VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1), Ok(status) if status == failed.bits() as u64 ); assert!(event_rx.is_empty()); @@ -318,400 +531,208 @@ fn test_virtio_pci_device_status_valid_transitions() { let needs_reset = failed | DevStatus::NEEDS_RESET; assert_matches!( mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, + VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1, needs_reset.bits() as u64 ), Ok(Action::None) ); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), + mmio.read(VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1), Ok(status) if status == needs_reset.bits() as u64 ); assert!(event_rx.is_empty()); // Enable queue and set MSI-X config vector assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_QUEUE_SELECT.0 as u64, 2, 0), + mmio.write(VirtioCommonCfg::OFFSET_QUEUE_SELECT as u64, 2, 0), Ok(Action::None) ); assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_QUEUE_ENABLE.0 as u64, 2, 1), + mmio.write(VirtioCommonCfg::OFFSET_QUEUE_ENABLE as u64, 2, 1), Ok(Action::None) ); assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_CONFIG_MSIX_VECTOR.0 as u64, 2, 0), + mmio.write(VirtioCommonCfg::OFFSET_CONFIG_MSIX_VECTOR as u64, 2, 0), Ok(Action::None) ); assert!(queues[0].enabled.load(Ordering::Acquire)); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_CONFIG_MSIX_VECTOR.0 as u64, 2), + mmio.read(VirtioCommonCfg::OFFSET_CONFIG_MSIX_VECTOR as u64, 2), Ok(0) ); // Reset device: write status = 0 assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1, 0), + mmio.write(VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1, 0), Ok(Action::None) ); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), + mmio.read(VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1), Ok(0) ); assert_matches!(event_rx.try_recv(), Ok(WakeEvent::Reset)); assert!(event_rx.is_empty()); assert!(!queues[0].enabled.load(Ordering::Acquire)); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_CONFIG_MSIX_VECTOR.0 as u64, 2), + mmio.read(VirtioCommonCfg::OFFSET_CONFIG_MSIX_VECTOR as u64, 2), Ok(vector) if vector == VIRTIO_MSI_NO_VECTOR as u64 ); } -#[test] -fn test_virtio_pci_device_status_invalid_transitions() { +#[rstest] +// Invalid transitions from empty state (skipping ACK or invalid combinations) +#[case(DevStatus::empty(), DevStatus::DRIVER.bits() as u64)] +#[case(DevStatus::empty(), DevStatus::FEATURES_OK.bits() as u64)] +#[case(DevStatus::empty(), DevStatus::DRIVER_OK.bits() as u64)] +#[case(DevStatus::empty(), (DevStatus::ACK | DevStatus::FEATURES_OK).bits() as u64)] +#[case(DevStatus::empty(), (DevStatus::ACK | DevStatus::DRIVER | DevStatus::DRIVER_OK).bits() as u64)] +// Unknown status bits from empty state are ignored +#[case(DevStatus::empty(), 0x10)] +#[case(DevStatus::empty(), 0x20)] +#[case(DevStatus::empty(), 0x30)] +#[case(DevStatus::empty(), 0xff)] +// Unknown status bits while in ACK state are ignored +#[case(DevStatus::ACK, DevStatus::ACK.bits() as u64 | 0x10)] +// Invalid transition: skipping DRIVER +#[case(DevStatus::ACK, (DevStatus::ACK | DevStatus::FEATURES_OK).bits() as u64)] +#[case(DevStatus::ACK, (DevStatus::ACK | DevStatus::DRIVER_OK).bits() as u64)] +// Invalid transition: clearing DRIVER bit without resetting to 0 +#[case(DevStatus::ACK | DevStatus::DRIVER, DevStatus::ACK.bits() as u64)] +// Invalid transition: setting DRIVER_OK without retaining ACK | DRIVER +#[case(DevStatus::ACK | DevStatus::DRIVER, DevStatus::DRIVER_OK.bits() as u64)] +// Invalid transition: skipping FEATURES_OK +#[case(DevStatus::ACK | DevStatus::DRIVER, (DevStatus::ACK | DevStatus::DRIVER | DevStatus::DRIVER_OK).bits() as u64)] +// Invalid transition: clearing bits from DRIVER_OK without resetting to 0 +#[case(DevStatus::ACK | DevStatus::DRIVER | DevStatus::FEATURES_OK | DevStatus::DRIVER_OK, (DevStatus::ACK | DevStatus::DRIVER | DevStatus::FEATURES_OK).bits() as u64)] +#[case(DevStatus::ACK | DevStatus::DRIVER | DevStatus::FEATURES_OK | DevStatus::DRIVER_OK, (DevStatus::ACK | DevStatus::DRIVER | DevStatus::DRIVER_OK).bits() as u64)] +#[case(DevStatus::ACK | DevStatus::DRIVER | DevStatus::FEATURES_OK | DevStatus::DRIVER_OK, (DevStatus::ACK | DevStatus::FEATURES_OK | DevStatus::DRIVER_OK).bits() as u64)] +// Invalid transition: clearing FAILED flag without resetting to 0 +#[case( + DevStatus::ACK | DevStatus::DRIVER | DevStatus::FEATURES_OK | DevStatus::DRIVER_OK | DevStatus::FAILED, + (DevStatus::ACK | DevStatus::DRIVER | DevStatus::FEATURES_OK | DevStatus::DRIVER_OK).bits() as u64 +)] +fn test_device_status_invalid_transitions(#[case] initial: DevStatus, #[case] invalid_write: u64) { let queues = Arc::new([QueueReg::default()]); let (mmio, event_rx) = create_test_mmio(queues); - // Invalid transitions from empty state (skipping ACK or invalid combinations) - for invalid in [ - DevStatus::DRIVER, - DevStatus::FEATURES_OK, - DevStatus::DRIVER_OK, - DevStatus::ACK | DevStatus::FEATURES_OK, - DevStatus::ACK | DevStatus::DRIVER | DevStatus::DRIVER_OK, - ] { + if !initial.is_empty() { assert_matches!( mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, + VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1, - invalid.bits() as u64 + initial.bits() as u64 ), Ok(Action::None) ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), - Ok(0) - ); - assert!(event_rx.is_empty()); - } - - // Unknown status bits from empty state are ignored - for unknown in [0x10u64, 0x20, 0x30, 0xff] { - assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1, unknown), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), - Ok(0) - ); - assert!(event_rx.is_empty()); + while event_rx.try_recv().is_ok() {} } - // Advance to ACK - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, - 1, - DevStatus::ACK.bits() as u64 - ), - Ok(Action::None) - ); - - // Unknown status bits while in ACK state are ignored - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, - 1, - DevStatus::ACK.bits() as u64 | 0x10 - ), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), - Ok(status) if status == DevStatus::ACK.bits() as u64 - ); - - // Invalid transition: skipping DRIVER (writing ACK | FEATURES_OK) - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, - 1, - (DevStatus::ACK | DevStatus::FEATURES_OK).bits() as u64 - ), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), - Ok(status) if status == DevStatus::ACK.bits() as u64 - ); - - // Invalid transition: skipping DRIVER (writing ACK | DRIVER_OK) - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, - 1, - (DevStatus::ACK | DevStatus::DRIVER_OK).bits() as u64 - ), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), - Ok(status) if status == DevStatus::ACK.bits() as u64 - ); - - // Advance to ACK | DRIVER - let ack_driver = DevStatus::ACK | DevStatus::DRIVER; assert_matches!( mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, + VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1, - ack_driver.bits() as u64 + invalid_write ), Ok(Action::None) ); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), - Ok(status) if status == ack_driver.bits() as u64 - ); - - // Invalid transition: clearing DRIVER bit (writing ACK only) without resetting to 0 - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, - 1, - DevStatus::ACK.bits() as u64 - ), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), - Ok(status) if status == ack_driver.bits() as u64 - ); - assert!(event_rx.is_empty()); - - // Invalid transition: setting DRIVER_OK without retaining ACK | DRIVER - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, - 1, - DevStatus::DRIVER_OK.bits() as u64 - ), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), - Ok(status) if status == ack_driver.bits() as u64 - ); - assert!(event_rx.is_empty()); - - // Invalid transition: skipping FEATURES_OK (writing ACK | DRIVER | DRIVER_OK) - let skipping_features_ok = ack_driver | DevStatus::DRIVER_OK; - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, - 1, - skipping_features_ok.bits() as u64 - ), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), - Ok(status) if status == ack_driver.bits() as u64 - ); - assert!(event_rx.is_empty()); - - // Advance to DRIVER_OK - let all_ok = ack_driver | DevStatus::FEATURES_OK | DevStatus::DRIVER_OK; - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, - 1, - all_ok.bits() as u64 - ), - Ok(Action::None) - ); - assert_matches!(event_rx.try_recv(), Ok(WakeEvent::Start { .. })); - - // Invalid transition: clearing DRIVER_OK without resetting to 0 - let without_driver_ok = ack_driver | DevStatus::FEATURES_OK; - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, - 1, - without_driver_ok.bits() as u64 - ), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), - Ok(status) if status == all_ok.bits() as u64 - ); - assert!(event_rx.is_empty()); - - // Invalid transition: clearing FEATURES_OK without resetting to 0 - let without_features_ok = ack_driver | DevStatus::DRIVER_OK; - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, - 1, - without_features_ok.bits() as u64 - ), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), - Ok(status) if status == all_ok.bits() as u64 - ); - assert!(event_rx.is_empty()); - - // Invalid transition: clearing DRIVER without resetting to 0 - let without_driver = DevStatus::ACK | DevStatus::FEATURES_OK | DevStatus::DRIVER_OK; - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, - 1, - without_driver.bits() as u64 - ), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), - Ok(status) if status == all_ok.bits() as u64 - ); - assert!(event_rx.is_empty()); - - // Add FAILED flag - let failed = all_ok | DevStatus::FAILED; - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, - 1, - failed.bits() as u64 - ), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), - Ok(status) if status == failed.bits() as u64 - ); - - // Invalid transition: clearing FAILED flag without resetting to 0 - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, - 1, - all_ok.bits() as u64 - ), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), - Ok(status) if status == failed.bits() as u64 + mmio.read(VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1), + Ok(status) if status == initial.bits() as u64 ); assert!(event_rx.is_empty()); } -#[test] -fn test_virtio_pci_device_status_multistep_transition() { +#[rstest] +// Multi-step transition: 0 -> ACK | DRIVER +#[case( + DevStatus::empty(), + DevStatus::ACK | DevStatus::DRIVER, + false +)] +// Multi-step transition: 0 -> ACK | DRIVER | FEATURES_OK | DRIVER_OK +#[case( + DevStatus::empty(), + DevStatus::ACK | DevStatus::DRIVER | DevStatus::FEATURES_OK | DevStatus::DRIVER_OK, + true +)] +// Multi-step transition: ACK | DRIVER -> ACK | DRIVER | FEATURES_OK | DRIVER_OK +#[case( + DevStatus::ACK | DevStatus::DRIVER, + DevStatus::ACK | DevStatus::DRIVER | DevStatus::FEATURES_OK | DevStatus::DRIVER_OK, + true +)] +fn test_device_status_multistep_transition( + #[case] initial: DevStatus, + #[case] target: DevStatus, + #[case] expect_start: bool, +) { let queues = Arc::new([QueueReg::default()]); let (mmio, event_rx) = create_test_mmio(queues); // Set driver features assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_DRIVER_FEATURE_SELECT.0 as u64, 4, 0), + mmio.write(VirtioCommonCfg::OFFSET_DRIVER_FEATURE_SELECT as u64, 4, 0), Ok(Action::None) ); assert_matches!( mmio.write( - VirtioCommonCfg::LAYOUT_DRIVER_FEATURE.0 as u64, + VirtioCommonCfg::OFFSET_DRIVER_FEATURE as u64, 4, 0x1234_5678 ), Ok(Action::None) ); assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_DRIVER_FEATURE_SELECT.0 as u64, 4, 1), + mmio.write(VirtioCommonCfg::OFFSET_DRIVER_FEATURE_SELECT as u64, 4, 1), Ok(Action::None) ); assert_matches!( mmio.write( - VirtioCommonCfg::LAYOUT_DRIVER_FEATURE.0 as u64, + VirtioCommonCfg::OFFSET_DRIVER_FEATURE as u64, 4, 0x9abc_def0 ), Ok(Action::None) ); - // Multi-step transition: 0 -> ACK | DRIVER | FEATURES_OK | DRIVER_OK in a single write - let all_ok = DevStatus::ACK | DevStatus::DRIVER | DevStatus::FEATURES_OK | DevStatus::DRIVER_OK; - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, - 1, - all_ok.bits() as u64 - ), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), - Ok(status) if status == all_ok.bits() as u64 - ); - assert_matches!( - event_rx.try_recv(), - Ok(WakeEvent::Start { param }) => { - assert_eq!(param.feature, ((0x9abc_def0u128) << 32) | 0x1234_5678); - } - ); - assert!(event_rx.is_empty()); - - // Reset to 0 - assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1, 0), - Ok(Action::None) - ); - assert_matches!(event_rx.try_recv(), Ok(WakeEvent::Reset)); + if !initial.is_empty() { + assert_matches!( + mmio.write( + VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, + 1, + initial.bits() as u64 + ), + Ok(Action::None) + ); + } - // Multi-step transition: 0 -> ACK | DRIVER in a single write - let ack_driver = DevStatus::ACK | DevStatus::DRIVER; assert_matches!( mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, + VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1, - ack_driver.bits() as u64 + target.bits() as u64 ), Ok(Action::None) ); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), - Ok(status) if status == ack_driver.bits() as u64 + mmio.read(VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1), + Ok(status) if status == target.bits() as u64 ); - assert!(event_rx.is_empty()); - // Multi-step transition: ACK | DRIVER -> ACK | DRIVER | FEATURES_OK | DRIVER_OK in a single write - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, - 1, - all_ok.bits() as u64 - ), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), - Ok(status) if status == all_ok.bits() as u64 - ); - assert_matches!( - event_rx.try_recv(), - Ok(WakeEvent::Start { param }) => { - assert_eq!(param.feature, ((0x9abc_def0u128) << 32) | 0x1234_5678); - } - ); + if expect_start { + assert_matches!( + event_rx.try_recv(), + Ok(WakeEvent::Start { param }) => { + assert_eq!(param.feature, ((0x9abc_def0u128) << 32) | 0x1234_5678); + } + ); + } assert!(event_rx.is_empty()); } #[test] -fn test_virtio_pci_device_status_reset_without_driver_ok() { +fn test_device_status_reset_without_driver_ok() { let queues = Arc::new([QueueReg::default()]); let (mmio, event_rx) = create_test_mmio(queues.clone()); @@ -719,7 +740,7 @@ fn test_virtio_pci_device_status_reset_without_driver_ok() { let ack_driver = DevStatus::ACK | DevStatus::DRIVER; assert_matches!( mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, + VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1, ack_driver.bits() as u64 ), @@ -728,30 +749,30 @@ fn test_virtio_pci_device_status_reset_without_driver_ok() { // Enable queue and set MSI-X config vector before reset assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_QUEUE_SELECT.0 as u64, 2, 0), + mmio.write(VirtioCommonCfg::OFFSET_QUEUE_SELECT as u64, 2, 0), Ok(Action::None) ); assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_QUEUE_ENABLE.0 as u64, 2, 1), + mmio.write(VirtioCommonCfg::OFFSET_QUEUE_ENABLE as u64, 2, 1), Ok(Action::None) ); assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_CONFIG_MSIX_VECTOR.0 as u64, 2, 0), + mmio.write(VirtioCommonCfg::OFFSET_CONFIG_MSIX_VECTOR as u64, 2, 0), Ok(Action::None) ); assert!(queues[0].enabled.load(Ordering::Acquire)); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_CONFIG_MSIX_VECTOR.0 as u64, 2), + mmio.read(VirtioCommonCfg::OFFSET_CONFIG_MSIX_VECTOR as u64, 2), Ok(0) ); // Reset status to 0 before DRIVER_OK is set assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1, 0), + mmio.write(VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1, 0), Ok(Action::None) ); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, 1), + mmio.read(VirtioCommonCfg::OFFSET_DEVICE_STATUS as u64, 1), Ok(0) ); // No WakeEvent::Reset since DRIVER_OK was not set @@ -759,221 +780,139 @@ fn test_virtio_pci_device_status_reset_without_driver_ok() { // self.reset() should still disable queues and reset MSI-X vectors assert!(!queues[0].enabled.load(Ordering::Acquire)); assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_CONFIG_MSIX_VECTOR.0 as u64, 2), + mmio.read(VirtioCommonCfg::OFFSET_CONFIG_MSIX_VECTOR as u64, 2), Ok(vector) if vector == VIRTIO_MSI_NO_VECTOR as u64 ); } -#[test] -fn test_virtio_pci_driver_features() { +#[rstest] +#[case(VirtioCommonCfg::OFFSET_CONFIG_MSIX_VECTOR, None)] +#[case(VirtioCommonCfg::OFFSET_QUEUE_MSIX_VECTOR, Some(0))] +fn test_msix_vector_configuration(#[case] offset: usize, #[case] queue_sel: Option) { let queues = Arc::new([QueueReg::default()]); - let (mut mmio, _event_rx) = create_test_mmio(queues); - mmio.reg.device_feature = [0x1234_5678, 0x0000_0005, 0, 0]; + let (mmio, _) = create_test_mmio(queues); - // Bank 0: only offered device features are accepted - assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_DRIVER_FEATURE_SELECT.0 as u64, 4, 0), - Ok(Action::None) - ); - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DRIVER_FEATURE.0 as u64, - 4, - 0xffff_ffff - ), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DRIVER_FEATURE.0 as u64, 4), - Ok(0x1234_5678) - ); + // Initialize MSI-X table with 2 Entry slots + *mmio.irq_sender.msix_table.entries.write() = vec![ + MsixTableMmioEntry::Entry(MsixTableEntry::default()), + MsixTableMmioEntry::Entry(MsixTableEntry::default()), + ] + .into_boxed_slice(); - // Bank 1: only offered device features are accepted - assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_DRIVER_FEATURE_SELECT.0 as u64, 4, 1), - Ok(Action::None) - ); - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DRIVER_FEATURE.0 as u64, - 4, - 0xffff_ffff - ), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DRIVER_FEATURE.0 as u64, 4), - Ok(0x0000_0005) - ); + if let Some(q) = queue_sel { + assert_matches!( + mmio.write(VirtioCommonCfg::OFFSET_QUEUE_SELECT as u64, 2, q as u64), + Ok(Action::None) + ); + } - // Bank 2 (no device features offered): writes are masked to 0 - assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_DRIVER_FEATURE_SELECT.0 as u64, 4, 2), - Ok(Action::None) - ); - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DRIVER_FEATURE.0 as u64, - 4, - 0xffff_ffff - ), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DRIVER_FEATURE.0 as u64, 4), - Ok(0) - ); + // Configure MSI-X vector from VIRTIO_MSI_NO_VECTOR -> 0 -> 1 + assert_matches!(mmio.write(offset as u64, 2, 0), Ok(Action::None)); + assert_matches!(mmio.read(offset as u64, 2), Ok(0)); + assert_matches!(mmio.write(offset as u64, 2, 1), Ok(Action::None)); + assert_matches!(mmio.read(offset as u64, 2), Ok(1)); - // Out-of-bounds bank selection does not store and does not panic - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DRIVER_FEATURE_SELECT.0 as u64, - 4, - 10 - ), - Ok(Action::None) - ); - assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_DRIVER_FEATURE.0 as u64, 4, 0x1234), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DRIVER_FEATURE.0 as u64, 4), - Ok(0) - ); + // Assign vector 1 to an IrqFd + mmio.irq_sender.msix_table.entries.write()[1] = MsixTableMmioEntry::IrqFd(TestIrqFd::default()); - // Set status to ACK | DRIVER | FEATURES_OK - let features_ok = DevStatus::ACK | DevStatus::DRIVER | DevStatus::FEATURES_OK; - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DEVICE_STATUS.0 as u64, - 1, - features_ok.bits() as u64 - ), - Ok(Action::None) - ); + // Attempt to change vector from 1 to 0 should be rejected + assert_matches!(mmio.write(offset as u64, 2, 0), Ok(Action::None)); + assert_matches!(mmio.read(offset as u64, 2), Ok(1)); +} - // Feature writes after FEATURES_OK must be ignored - assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_DRIVER_FEATURE_SELECT.0 as u64, 4, 0), - Ok(Action::None) - ); - assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_DRIVER_FEATURE.0 as u64, 4, 0), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_DRIVER_FEATURE.0 as u64, 4), - Ok(0x1234_5678) - ); +#[rstest] +// Valid queue notify (to valid queue offset) wakes up queue 0 +#[case(VirtioPciRegister::OFFSET_QUEUE_NOTIFY, true)] +// Queue notify to reserved invalid slot (at queues.len()) does not wake up any queue +#[case(VirtioPciRegister::OFFSET_QUEUE_NOTIFY + size_of::(), false)] +fn test_queue_notify(#[case] offset: usize, #[case] expect_wake: bool) { + let queues = Arc::new([QueueReg::default()]); + let (mmio, event_rx) = create_test_mmio(queues); + + assert_matches!(mmio.write(offset as u64, 2, 0), Ok(Action::None)); + if expect_wake { + assert_matches!(event_rx.try_recv(), Ok(WakeEvent::Notify { q_index: 0 })); + } + assert!(event_rx.is_empty()); } #[test] -fn test_virtio_pci_queue_alignment() { - let queues = Arc::new([QueueReg { - desc: AtomicU64::new(0x1000_0000), - driver: AtomicU64::new(0x2000_0000), - device: AtomicU64::new(0x3000_0000), - ..Default::default() - }]); - let (mmio, _event_rx) = create_test_mmio(queues); +fn test_notify_with_ioeventfds() { + let queues = Arc::new([QueueReg::default()]); + let (event_tx, event_rx) = flume::unbounded(); + let notifier = Arc::new(Notifier::new().unwrap()); + let msi_sender = TestMsiSender::default(); + let msix_table = Arc::new(MsixTableMmio { + entries: RwLock::new(vec![].into_boxed_slice()), + }); + let irq_sender = Arc::new(PciIrqSender { + msix_vector: VirtioPciMsixVector { + config: AtomicU16::new(VIRTIO_MSI_NO_VECTOR), + queues: vec![AtomicU16::new(VIRTIO_MSI_NO_VECTOR)], + }, + msix_table, + msi_sender, + }); + let mmio = VirtioPciRegisterMmio { + name: "test-virtio-pci-ioeventfd".into(), + reg: Register { + device_feature: [u32::MAX; 4], + ..Default::default() + }, + queues, + irq_sender, + ioeventfds: Some(Arc::new([FakeIoeventFd])), + event_tx, + notifier, + }; - // Select queue 0 assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_QUEUE_SELECT.0 as u64, 2, 0), + mmio.write(VirtioPciRegister::OFFSET_QUEUE_NOTIFY as u64, 2, 0), Ok(Action::None) ); + assert_matches!(event_rx.try_recv(), Ok(WakeEvent::Notify { q_index: 0 })); +} - // LAYOUT_QUEUE_DESC_LO: must be 16-byte aligned - // Unaligned write (e.g. offset 8) should be ignored - assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_QUEUE_DESC_LO.0 as u64, 4, 0x1008), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_QUEUE_DESC_LO.0 as u64, 4), - Ok(0x1000_0000) - ); - // Aligned write (16-byte aligned) should succeed - assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_QUEUE_DESC_LO.0 as u64, 4, 0x1010), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_QUEUE_DESC_LO.0 as u64, 4), - Ok(0x1010) - ); +#[test] +fn test_wake_up_dev_channel_error() { + let queues = Arc::new([QueueReg::default()]); + let (mmio, event_rx) = create_test_mmio(queues); - // LAYOUT_QUEUE_DEVICE_LO: must be 4-byte aligned - // Unaligned write (e.g. 2) should be ignored - assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_QUEUE_DEVICE_LO.0 as u64, 4, 0x3002), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_QUEUE_DEVICE_LO.0 as u64, 4), - Ok(0x3000_0000) - ); - // Aligned write (4-byte aligned) should succeed - assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_QUEUE_DEVICE_LO.0 as u64, 4, 0x3004), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_QUEUE_DEVICE_LO.0 as u64, 4), - Ok(0x3004) - ); + // Drop receiver so send will fail + drop(event_rx); - // LAYOUT_QUEUE_DRIVER_LO without RING_PACKED (split queue): - // 2-byte aligned is allowed, unaligned 1-byte is rejected - assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_QUEUE_DRIVER_LO.0 as u64, 4, 0x2001), - Ok(Action::None) - ); + // Trigger notify event assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_QUEUE_DRIVER_LO.0 as u64, 4), - Ok(0x2000_0000) - ); - assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_QUEUE_DRIVER_LO.0 as u64, 4, 0x2002), + mmio.write(VirtioPciRegister::OFFSET_QUEUE_NOTIFY as u64, 2, 0), Ok(Action::None) ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_QUEUE_DRIVER_LO.0 as u64, 4), - Ok(0x2002) - ); +} - // Enable RING_PACKED in driver feature - assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_DRIVER_FEATURE_SELECT.0 as u64, 4, 1), - Ok(Action::None) - ); - assert_matches!( - mmio.write( - VirtioCommonCfg::LAYOUT_DRIVER_FEATURE.0 as u64, - 4, - (VirtioFeature::RING_PACKED.bits() >> 32) as u64 - ), - Ok(Action::None) - ); +#[rstest] +// Invalid register write with offset < OFFSET_QUEUE_NOTIFY +#[case(0x3c, 4, 0x1234)] +// Invalid register write with offset >= OFFSET_QUEUE_NOTIFY +#[case(0xdead_beef, 4, 0x1234)] +fn test_invalid_register_writes(#[case] offset: u64, #[case] size: u8, #[case] val: u64) { + let queues = Arc::new([QueueReg::default()]); + let (mmio, _) = create_test_mmio(queues); + assert_matches!(mmio.write(offset, size, val), Ok(Action::None)); +} - // LAYOUT_QUEUE_DRIVER_LO with RING_PACKED: - // 2-byte aligned (not 4-byte aligned) must now be rejected - assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_QUEUE_DRIVER_LO.0 as u64, 4, 0x4002), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_QUEUE_DRIVER_LO.0 as u64, 4), - Ok(0x2002) - ); - // 4-byte aligned is accepted - assert_matches!( - mmio.write(VirtioCommonCfg::LAYOUT_QUEUE_DRIVER_LO.0 as u64, 4, 0x4004), - Ok(Action::None) - ); - assert_matches!( - mmio.read(VirtioCommonCfg::LAYOUT_QUEUE_DRIVER_LO.0 as u64, 4), - Ok(0x4004) - ); +#[rstest] +#[case(0x1234, 4)] +#[case(0, 3)] +fn test_invalid_register_reads(#[case] offset: u64, #[case] size: u8) { + let queues = Arc::new([QueueReg::default()]); + let (mmio, _) = create_test_mmio(queues); + assert_matches!(mmio.read(offset, size), Ok(0)); +} + +#[rstest] +#[case(1, (size_of::() + size_of::() * 2) as u64)] +#[case(3, (size_of::() + size_of::() * 4) as u64)] +fn test_mmio_size(#[case] num_queues: usize, #[case] expected_size: u64) { + let queues = (0..num_queues).map(|_| QueueReg::default()).collect(); + let (mmio, _) = create_test_mmio(queues); + assert_eq!(mmio.size(), expected_size); } diff --git a/alioth/src/virtio/queue/packed.rs b/alioth/src/virtio/queue/packed.rs index f5e3e8b8..63fd1a7d 100644 --- a/alioth/src/virtio/queue/packed.rs +++ b/alioth/src/virtio/queue/packed.rs @@ -20,8 +20,8 @@ use zerocopy::{FromBytes, Immutable, IntoBytes}; use crate::consts; use crate::mem::mapped::Ram; -use crate::virtio::Result; use crate::virtio::queue::{DescChain, DescFlag, QueueReg, VirtQueue}; +use crate::virtio::{Result, error}; #[repr(C, align(16))] #[derive(Debug, Clone, Default, FromBytes, Immutable, IntoBytes)] @@ -99,6 +99,9 @@ impl<'m> PackedQueue<'m> { return Ok(None); } let size = reg.size.load(Ordering::Acquire); + if size == 0 { + return error::InvalidQueueSize { size }.fail(); + } let desc = reg.desc.load(Ordering::Acquire); let notification: *mut DescEvent = ram.get_ptr(reg.device.load(Ordering::Acquire))?; Ok(Some(PackedQueue { diff --git a/alioth/src/virtio/queue/packed_test.rs b/alioth/src/virtio/queue/packed_test.rs index a74e8180..885fbc8c 100644 --- a/alioth/src/virtio/queue/packed_test.rs +++ b/alioth/src/virtio/queue/packed_test.rs @@ -18,6 +18,7 @@ use std::sync::atomic::Ordering; use assert_matches::assert_matches; use rstest::rstest; +use crate::virtio::Error; use crate::virtio::queue::packed::{DescEvent, EventFlag, PackedQueue, WrappedIndex}; use crate::virtio::queue::tests::{GuestQueue, UsedDesc, VirtQueueGuest}; use crate::virtio::queue::{DescFlag, VirtQueue}; @@ -128,6 +129,18 @@ fn disabled_queue() { assert_matches!(split_queue, Ok(None)); } +#[test] +fn invalid_queue_size() { + let ram_bus = fixture_ram_bus(); + let queues = fixture_queues(1); + let ram = ram_bus.lock_layout(); + let reg = &queues[0]; + + reg.size.store(0, Ordering::Relaxed); + let split_queue = PackedQueue::new(reg, &ram, false); + assert_matches!(split_queue, Err(Error::InvalidQueueSize { size: 0, .. })); +} + #[test] fn enabled_queue() { let ram_bus = fixture_ram_bus(); diff --git a/alioth/src/virtio/queue/split.rs b/alioth/src/virtio/queue/split.rs index 5c2ab1d3..3772c9da 100644 --- a/alioth/src/virtio/queue/split.rs +++ b/alioth/src/virtio/queue/split.rs @@ -125,6 +125,9 @@ impl<'m> SplitQueue<'m> { return Ok(None); } let size = reg.size.load(Ordering::Acquire) as u64; + if size == 0 || !size.is_power_of_two() { + return error::InvalidQueueSize { size: size as u16 }.fail(); + } let mut avail_event = None; let mut used_event = None; let used = reg.device.load(Ordering::Acquire); diff --git a/alioth/src/virtio/queue/split_test.rs b/alioth/src/virtio/queue/split_test.rs index 51fcfd9d..7c5db689 100644 --- a/alioth/src/virtio/queue/split_test.rs +++ b/alioth/src/virtio/queue/split_test.rs @@ -17,6 +17,7 @@ use std::sync::atomic::Ordering; use assert_matches::assert_matches; +use crate::virtio::Error; use crate::virtio::queue::VirtQueue; use crate::virtio::queue::split::{Desc, DescFlag, SplitQueue}; use crate::virtio::queue::tests::{GuestQueue, UsedDesc, VirtQueueGuest}; @@ -80,6 +81,22 @@ fn disabled_queue() { assert_matches!(split_queue, Ok(None)); } +#[test] +fn invalid_queue_size() { + let ram_bus = fixture_ram_bus(); + let queues = fixture_queues(1); + let ram = ram_bus.lock_layout(); + let reg = &queues[0]; + + reg.size.store(0, Ordering::Relaxed); + let split_queue = SplitQueue::new(reg, &ram, false); + assert_matches!(split_queue, Err(Error::InvalidQueueSize { size: 0, .. })); + + reg.size.store(3, Ordering::Relaxed); + let split_queue = SplitQueue::new(reg, &ram, false); + assert_matches!(split_queue, Err(Error::InvalidQueueSize { size: 3, .. })); +} + #[test] fn enabled_queue() { let ram_bus = fixture_ram_bus(); diff --git a/alioth/src/virtio/virtio.rs b/alioth/src/virtio/virtio.rs index b307f788..bec52587 100644 --- a/alioth/src/virtio/virtio.rs +++ b/alioth/src/virtio/virtio.rs @@ -69,6 +69,8 @@ pub enum Error { InvalidDescriptor { id: u16 }, #[snafu(display("Invalid queue index {index}"))] InvalidQueueIndex { index: u16 }, + #[snafu(display("Invalid queue size {size}"))] + InvalidQueueSize { size: u16 }, #[snafu(display("Invalid msix vector {vector}"))] InvalidMsixVector { vector: u16 }, #[snafu(display("Invalid virtq buffer"))]