From a4df38d7c5f980597dd943dc319ea0f201b694f3 Mon Sep 17 00:00:00 2001 From: Zhang Junyu Date: Thu, 5 Mar 2026 17:00:42 +0800 Subject: [PATCH 1/6] refine: Refine virtio-mmio --- crates/vm-virtio/src/transport/mmio.rs | 208 +----------------- .../src/transport/mmio/mmio_handler.rs | 196 +++++++++++++++++ 2 files changed, 198 insertions(+), 206 deletions(-) create mode 100644 crates/vm-virtio/src/transport/mmio/mmio_handler.rs diff --git a/crates/vm-virtio/src/transport/mmio.rs b/crates/vm-virtio/src/transport/mmio.rs index 06b6334..a040aac 100644 --- a/crates/vm-virtio/src/transport/mmio.rs +++ b/crates/vm-virtio/src/transport/mmio.rs @@ -10,217 +10,13 @@ use vm_mm::allocator::MemoryContainer; use crate::device::VirtioDevice; use crate::transport::VirtioDev; -use crate::transport::mmio::handler::Handler; +use crate::transport::mmio::mmio_handler::Handler; mod control_register; +mod mmio_handler; const CONFIGURATION_SPACE_OFFSET: usize = 0x100; -mod handler { - use std::sync::Arc; - use std::sync::Mutex; - - use tracing::debug; - use tracing::error; - use tracing::warn; - use vm_core::device::mmio::MmioRange; - use vm_core::device::mmio::mmio_device::MmioHandler; - use vm_mm::allocator::MemoryContainer; - - use crate::device::VirtioDevice; - use crate::result::Result as VirtioResult; - use crate::transport::VirtioDev; - use crate::transport::control_register::ControlRegister; - use crate::transport::mmio::CONFIGURATION_SPACE_OFFSET; - use crate::transport::mmio::control_register::MmioControlRegister; - use crate::types::interrupt_status::InterruptStatus; - - pub struct Handler { - mmio_range: MmioRange, - transport: Arc>>, - } - - impl Handler - where - D: VirtioDevice, - { - pub fn new(mmio_range: MmioRange, transport: Arc>>) -> Self { - Handler { - mmio_range, - transport, - } - } - - fn read_reg(&self, transport: &VirtioDev, reg: MmioControlRegister) -> u32 { - match reg { - MmioControlRegister::MagicValue => u32::from_le_bytes(*b"virt"), - MmioControlRegister::Version => 0x2, - MmioControlRegister::DeviceId => D::DEVICE_ID, - MmioControlRegister::VendorId => u32::from_le_bytes(*b"QEMU"), - MmioControlRegister::DeviceFeatures => { - transport.read_reg(ControlRegister::DeviceFeatures) - } - MmioControlRegister::QueueSizeMax => { - transport.read_reg(ControlRegister::QueueSizeMax) - } - MmioControlRegister::QueueReady => transport.read_reg(ControlRegister::QueueReady), - MmioControlRegister::InterruptStatus => { - transport.read_reg(ControlRegister::InterruptStatus) - } - MmioControlRegister::Status => transport.read_reg(ControlRegister::Status), - MmioControlRegister::QueueReset => todo!(), - MmioControlRegister::ConfigGeneration => { - transport.read_reg(ControlRegister::ConfigGeneration) - } - _ => unreachable!("try to read a WO register: {reg:?}"), - } - } - - fn write_reg( - &self, - transport: &mut VirtioDev, - reg: MmioControlRegister, - val: u32, - ) -> VirtioResult<()> { - match reg { - MmioControlRegister::DeviceFeaturesSel => { - transport.write_reg(ControlRegister::DeviceFeaturesSel, val) - } - MmioControlRegister::DriverFeatures => { - transport.write_reg(ControlRegister::DriverFeatures, val) - } - MmioControlRegister::DriverFeaturesSel => { - transport.write_reg(ControlRegister::DriverFeaturesSel, val) - } - MmioControlRegister::QueueSel => { - transport.write_reg(ControlRegister::QueueSel, val) - } - MmioControlRegister::QueueSize => { - transport.write_reg(ControlRegister::QueueSize, val) - } - MmioControlRegister::QueueReady => { - transport.write_reg(ControlRegister::QueueReady, val) - } - MmioControlRegister::QueueNotify => { - transport.write_reg(ControlRegister::QueueNotify, val) - } - MmioControlRegister::InterruptAck => { - transport - .interrupt_status - .remove(InterruptStatus::from_bits_truncate(val)); - - if transport.interrupt_status.is_empty() { - transport.device.trigger_irq(false); - } - - Ok(()) - } - MmioControlRegister::Status => transport.write_reg(ControlRegister::Status, val), - MmioControlRegister::QueueDescLow => { - transport.write_reg(ControlRegister::QueueDescLow, val) - } - MmioControlRegister::QueueDescHigh => { - transport.write_reg(ControlRegister::QueueDescHigh, val) - } - MmioControlRegister::QueueAvailLow => { - transport.write_reg(ControlRegister::QueueAvailLow, val) - } - MmioControlRegister::QueueAvailHigh => { - transport.write_reg(ControlRegister::QueueAvailHigh, val) - } - MmioControlRegister::QueueUsedLow => { - transport.write_reg(ControlRegister::QueueUsedLow, val) - } - MmioControlRegister::QueueUsedHigh => { - transport.write_reg(ControlRegister::QueueUsedHigh, val) - } - MmioControlRegister::ShmSel => todo!(), - MmioControlRegister::QueueReset => todo!(), - _ => unreachable!("Try to write a RO register {reg:?}"), - } - } - } - - impl MmioHandler for Handler - where - C: MemoryContainer, - D: VirtioDevice, - { - fn mmio_range(&self) -> MmioRange { - self.mmio_range - } - - fn mmio_read(&self, offset: u64, len: usize, data: &mut [u8]) { - let transport = self.transport.lock().unwrap(); - - let offset: usize = offset.try_into().unwrap(); - if offset < CONFIGURATION_SPACE_OFFSET { - if let Some(reg) = MmioControlRegister::from_repr(offset as u16) { - assert_eq!(len, 4); - assert_eq!(data.len(), 4); - - let val = self.read_reg(&transport, reg); - - debug!(name = D::NAME, ?reg, len, val, "read reg from virtio-mmio"); - - data.copy_from_slice(&val.to_le_bytes()); - } else { - warn!( - offset, - len, - ?data, - "read from invalid offset of virtio-mmio device" - ); - - panic!() - } - } else if let Err(err) = - transport.read_config(offset - CONFIGURATION_SPACE_OFFSET, len, data) - { - error!(?err, "Failed to read device configuration"); - - panic!() - } - } - - fn mmio_write(&self, offset: u64, len: usize, data: &[u8]) { - let mut transport = self.transport.lock().unwrap(); - - debug!(name = D::NAME, offset, len, ?data); - - let offset: usize = offset.try_into().unwrap(); - if offset < CONFIGURATION_SPACE_OFFSET { - if let Some(reg) = MmioControlRegister::from_repr(offset as u16) { - assert_eq!(len, 4); - assert_eq!(data.len(), 4); - - self.write_reg( - &mut transport, - reg, - u32::from_le_bytes(data.try_into().unwrap()), - ) - .unwrap(); - } else { - warn!( - offset, - len, - ?data, - "write from invalid offset of virtio-mmio device" - ); - - panic!() - } - } else if let Err(err) = - transport.write_config(offset - CONFIGURATION_SPACE_OFFSET, len, data) - { - error!(?err, "Failed to write device configuration"); - - panic!() - } - } - } -} - pub struct VirtioMmioTransport { mmio_range: MmioRange, dev: Arc>>, diff --git a/crates/vm-virtio/src/transport/mmio/mmio_handler.rs b/crates/vm-virtio/src/transport/mmio/mmio_handler.rs new file mode 100644 index 0000000..5ca2ed6 --- /dev/null +++ b/crates/vm-virtio/src/transport/mmio/mmio_handler.rs @@ -0,0 +1,196 @@ +use std::sync::Arc; +use std::sync::Mutex; + +use tracing::debug; +use tracing::error; +use tracing::warn; +use vm_core::device::mmio::MmioRange; +use vm_core::device::mmio::mmio_device::MmioHandler; +use vm_mm::allocator::MemoryContainer; + +use crate::device::VirtioDevice; +use crate::result::Result as VirtioResult; +use crate::transport::VirtioDev; +use crate::transport::control_register::ControlRegister; +use crate::transport::mmio::CONFIGURATION_SPACE_OFFSET; +use crate::transport::mmio::control_register::MmioControlRegister; +use crate::types::interrupt_status::InterruptStatus; + +pub struct Handler { + mmio_range: MmioRange, + transport: Arc>>, +} + +impl Handler +where + D: VirtioDevice, +{ + pub fn new(mmio_range: MmioRange, transport: Arc>>) -> Self { + Handler { + mmio_range, + transport, + } + } + + fn read_reg(&self, transport: &VirtioDev, reg: MmioControlRegister) -> u32 { + match reg { + MmioControlRegister::MagicValue => u32::from_le_bytes(*b"virt"), + MmioControlRegister::Version => 0x2, + MmioControlRegister::DeviceId => D::DEVICE_ID, + MmioControlRegister::VendorId => u32::from_le_bytes(*b"QEMU"), + MmioControlRegister::DeviceFeatures => { + transport.read_reg(ControlRegister::DeviceFeatures) + } + MmioControlRegister::QueueSizeMax => transport.read_reg(ControlRegister::QueueSizeMax), + MmioControlRegister::QueueReady => transport.read_reg(ControlRegister::QueueReady), + MmioControlRegister::InterruptStatus => { + transport.read_reg(ControlRegister::InterruptStatus) + } + MmioControlRegister::Status => transport.read_reg(ControlRegister::Status), + MmioControlRegister::QueueReset => todo!(), + MmioControlRegister::ConfigGeneration => { + transport.read_reg(ControlRegister::ConfigGeneration) + } + _ => unreachable!("try to read a WO register: {reg:?}"), + } + } + + fn write_reg( + &self, + transport: &mut VirtioDev, + reg: MmioControlRegister, + val: u32, + ) -> VirtioResult<()> { + match reg { + MmioControlRegister::DeviceFeaturesSel => { + transport.write_reg(ControlRegister::DeviceFeaturesSel, val) + } + MmioControlRegister::DriverFeatures => { + transport.write_reg(ControlRegister::DriverFeatures, val) + } + MmioControlRegister::DriverFeaturesSel => { + transport.write_reg(ControlRegister::DriverFeaturesSel, val) + } + MmioControlRegister::QueueSel => transport.write_reg(ControlRegister::QueueSel, val), + MmioControlRegister::QueueSize => transport.write_reg(ControlRegister::QueueSize, val), + MmioControlRegister::QueueReady => { + transport.write_reg(ControlRegister::QueueReady, val) + } + MmioControlRegister::QueueNotify => { + transport.write_reg(ControlRegister::QueueNotify, val) + } + MmioControlRegister::InterruptAck => { + transport + .interrupt_status + .remove(InterruptStatus::from_bits_truncate(val)); + + if transport.interrupt_status.is_empty() { + transport.device.trigger_irq(false); + } + + Ok(()) + } + MmioControlRegister::Status => transport.write_reg(ControlRegister::Status, val), + MmioControlRegister::QueueDescLow => { + transport.write_reg(ControlRegister::QueueDescLow, val) + } + MmioControlRegister::QueueDescHigh => { + transport.write_reg(ControlRegister::QueueDescHigh, val) + } + MmioControlRegister::QueueAvailLow => { + transport.write_reg(ControlRegister::QueueAvailLow, val) + } + MmioControlRegister::QueueAvailHigh => { + transport.write_reg(ControlRegister::QueueAvailHigh, val) + } + MmioControlRegister::QueueUsedLow => { + transport.write_reg(ControlRegister::QueueUsedLow, val) + } + MmioControlRegister::QueueUsedHigh => { + transport.write_reg(ControlRegister::QueueUsedHigh, val) + } + MmioControlRegister::ShmSel => todo!(), + MmioControlRegister::QueueReset => todo!(), + _ => unreachable!("Try to write a RO register {reg:?}"), + } + } +} + +impl MmioHandler for Handler +where + C: MemoryContainer, + D: VirtioDevice, +{ + fn mmio_range(&self) -> MmioRange { + self.mmio_range + } + + fn mmio_read(&self, offset: u64, len: usize, data: &mut [u8]) { + let transport = self.transport.lock().unwrap(); + + let offset: usize = offset.try_into().unwrap(); + if offset < CONFIGURATION_SPACE_OFFSET { + if let Some(reg) = MmioControlRegister::from_repr(offset as u16) { + assert_eq!(len, 4); + assert_eq!(data.len(), 4); + + let val = self.read_reg(&transport, reg); + + debug!(name = D::NAME, ?reg, len, val, "read reg from virtio-mmio"); + + data.copy_from_slice(&val.to_le_bytes()); + } else { + warn!( + offset, + len, + ?data, + "read from invalid offset of virtio-mmio device" + ); + + panic!() + } + } else if let Err(err) = + transport.read_config(offset - CONFIGURATION_SPACE_OFFSET, len, data) + { + error!(?err, "Failed to read device configuration"); + + panic!() + } + } + + fn mmio_write(&self, offset: u64, len: usize, data: &[u8]) { + let mut transport = self.transport.lock().unwrap(); + + debug!(name = D::NAME, offset, len, ?data); + + let offset: usize = offset.try_into().unwrap(); + if offset < CONFIGURATION_SPACE_OFFSET { + if let Some(reg) = MmioControlRegister::from_repr(offset as u16) { + assert_eq!(len, 4); + assert_eq!(data.len(), 4); + + self.write_reg( + &mut transport, + reg, + u32::from_le_bytes(data.try_into().unwrap()), + ) + .unwrap(); + } else { + warn!( + offset, + len, + ?data, + "write from invalid offset of virtio-mmio device" + ); + + panic!() + } + } else if let Err(err) = + transport.write_config(offset - CONFIGURATION_SPACE_OFFSET, len, data) + { + error!(?err, "Failed to write device configuration"); + + panic!() + } + } +} From 9c9e6bc8963e07ffc61b3aacb7b57a85b421df15 Mon Sep 17 00:00:00 2001 From: Zhang Junyu Date: Thu, 5 Mar 2026 19:54:28 +0800 Subject: [PATCH 2/6] refine: Refine code --- crates/vm-virtio/src/transport/mmio.rs | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/crates/vm-virtio/src/transport/mmio.rs b/crates/vm-virtio/src/transport/mmio.rs index a040aac..53c91d3 100644 --- a/crates/vm-virtio/src/transport/mmio.rs +++ b/crates/vm-virtio/src/transport/mmio.rs @@ -30,10 +30,6 @@ where pub fn new(dev: Arc>>, mmio_range: MmioRange) -> Self { VirtioMmioTransport { mmio_range, dev } } - - fn generate_mmio_handler(&self) -> Handler { - Handler::new(self.mmio_range, self.dev.clone()) - } } impl Device for VirtioMmioTransport @@ -52,7 +48,7 @@ where D: VirtioDevice, { fn mmio_range_handlers(&self) -> Vec> { - vec![Box::new(self.generate_mmio_handler())] + vec![Box::new(Handler::new(self.mmio_range, self.dev.clone()))] } fn generate_dt(&self, fdt: &mut FdtWriter) -> Result<(), vm_fdt::Error> { @@ -62,15 +58,16 @@ where fdt.property_string("compatible", "virtio,mmio")?; fdt.property_array_u64("reg", &[self.mmio_range.start, self.mmio_range.len as u64])?; - if let Some(_irq) = dev.device.irq() { + if let Some(irq) = dev.device.irq() { #[cfg(target_arch = "aarch64")] { use vm_core::arch::aarch64::irq::GIC_SPI; use vm_core::arch::aarch64::irq::IRQ_TYPE_LEVEL_HIGH; - fdt.property_array_u32("interrupts", &[GIC_SPI, _irq, IRQ_TYPE_LEVEL_HIGH])?; + fdt.property_array_u32("interrupts", &[GIC_SPI, irq, IRQ_TYPE_LEVEL_HIGH])?; } #[cfg(not(target_arch = "aarch64"))] { + std::hint::black_box(irq); todo!() } } From 58dd176efb0b8254f64db454eca339825523ea07 Mon Sep 17 00:00:00 2001 From: Zhang Junyu Date: Thu, 5 Mar 2026 20:03:42 +0800 Subject: [PATCH 3/6] refine: Refine code --- crates/vm-virtio/src/transport/mmio.rs | 2 -- crates/vm-virtio/src/transport/mmio/mmio_handler.rs | 3 ++- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/crates/vm-virtio/src/transport/mmio.rs b/crates/vm-virtio/src/transport/mmio.rs index 53c91d3..df72729 100644 --- a/crates/vm-virtio/src/transport/mmio.rs +++ b/crates/vm-virtio/src/transport/mmio.rs @@ -15,8 +15,6 @@ use crate::transport::mmio::mmio_handler::Handler; mod control_register; mod mmio_handler; -const CONFIGURATION_SPACE_OFFSET: usize = 0x100; - pub struct VirtioMmioTransport { mmio_range: MmioRange, dev: Arc>>, diff --git a/crates/vm-virtio/src/transport/mmio/mmio_handler.rs b/crates/vm-virtio/src/transport/mmio/mmio_handler.rs index 5ca2ed6..5e5de0b 100644 --- a/crates/vm-virtio/src/transport/mmio/mmio_handler.rs +++ b/crates/vm-virtio/src/transport/mmio/mmio_handler.rs @@ -12,10 +12,11 @@ use crate::device::VirtioDevice; use crate::result::Result as VirtioResult; use crate::transport::VirtioDev; use crate::transport::control_register::ControlRegister; -use crate::transport::mmio::CONFIGURATION_SPACE_OFFSET; use crate::transport::mmio::control_register::MmioControlRegister; use crate::types::interrupt_status::InterruptStatus; +const CONFIGURATION_SPACE_OFFSET: usize = 0x100; + pub struct Handler { mmio_range: MmioRange, transport: Arc>>, From c821a21907e4df924376ea13fbba262ec4b96572 Mon Sep 17 00:00:00 2001 From: Zhang Junyu Date: Thu, 5 Mar 2026 22:46:59 +0800 Subject: [PATCH 4/6] refine: Refine virtio-mmio expect irq --- .../src/transport/mmio/control_register.rs | 1 - .../src/transport/mmio/mmio_handler.rs | 180 ++++++++++-------- 2 files changed, 97 insertions(+), 84 deletions(-) diff --git a/crates/vm-virtio/src/transport/mmio/control_register.rs b/crates/vm-virtio/src/transport/mmio/control_register.rs index b45f03e..5b95df9 100644 --- a/crates/vm-virtio/src/transport/mmio/control_register.rs +++ b/crates/vm-virtio/src/transport/mmio/control_register.rs @@ -3,7 +3,6 @@ use strum_macros::FromRepr; #[derive(Clone, Copy, Debug, FromRepr)] #[repr(u16)] pub enum MmioControlRegister { - /* Control registers */ /// Magic value ("virt") - Read Only MagicValue = 0x000, diff --git a/crates/vm-virtio/src/transport/mmio/mmio_handler.rs b/crates/vm-virtio/src/transport/mmio/mmio_handler.rs index 5e5de0b..9079553 100644 --- a/crates/vm-virtio/src/transport/mmio/mmio_handler.rs +++ b/crates/vm-virtio/src/transport/mmio/mmio_handler.rs @@ -17,102 +17,98 @@ use crate::types::interrupt_status::InterruptStatus; const CONFIGURATION_SPACE_OFFSET: usize = 0x100; +const VIRTIO_MMIO_MAGIC_VALUE: u32 = u32::from_le_bytes(*b"virt"); +const VIRTIO_MMIO_VERSION: u32 = 0x2; +const VIRTIO_MMIO_VENDOR_ID: u32 = u32::from_le_bytes(*b"QEMU"); + pub struct Handler { mmio_range: MmioRange, - transport: Arc>>, + dev: Arc>>, } impl Handler where D: VirtioDevice, { - pub fn new(mmio_range: MmioRange, transport: Arc>>) -> Self { - Handler { - mmio_range, - transport, - } + pub fn new(mmio_range: MmioRange, dev: Arc>>) -> Self { + Handler { mmio_range, dev } } - fn read_reg(&self, transport: &VirtioDev, reg: MmioControlRegister) -> u32 { + fn read_reg(&self, dev: &VirtioDev, reg: MmioControlRegister) -> u32 { match reg { - MmioControlRegister::MagicValue => u32::from_le_bytes(*b"virt"), - MmioControlRegister::Version => 0x2, + MmioControlRegister::MagicValue => VIRTIO_MMIO_MAGIC_VALUE, + MmioControlRegister::Version => VIRTIO_MMIO_VERSION, MmioControlRegister::DeviceId => D::DEVICE_ID, - MmioControlRegister::VendorId => u32::from_le_bytes(*b"QEMU"), - MmioControlRegister::DeviceFeatures => { - transport.read_reg(ControlRegister::DeviceFeatures) - } - MmioControlRegister::QueueSizeMax => transport.read_reg(ControlRegister::QueueSizeMax), - MmioControlRegister::QueueReady => transport.read_reg(ControlRegister::QueueReady), - MmioControlRegister::InterruptStatus => { - transport.read_reg(ControlRegister::InterruptStatus) - } - MmioControlRegister::Status => transport.read_reg(ControlRegister::Status), + MmioControlRegister::VendorId => VIRTIO_MMIO_VENDOR_ID, + MmioControlRegister::DeviceFeatures => dev.read_reg(ControlRegister::DeviceFeatures), + MmioControlRegister::QueueSizeMax => dev.read_reg(ControlRegister::QueueSizeMax), + MmioControlRegister::QueueReady => dev.read_reg(ControlRegister::QueueReady), + MmioControlRegister::InterruptStatus => dev.read_reg(ControlRegister::InterruptStatus), + MmioControlRegister::Status => dev.read_reg(ControlRegister::Status), MmioControlRegister::QueueReset => todo!(), MmioControlRegister::ConfigGeneration => { - transport.read_reg(ControlRegister::ConfigGeneration) + dev.read_reg(ControlRegister::ConfigGeneration) + } + _ => { + warn!(?reg, "try to read a WO register"); + 0 // ignore the error } - _ => unreachable!("try to read a WO register: {reg:?}"), } } fn write_reg( &self, - transport: &mut VirtioDev, + dev: &mut VirtioDev, reg: MmioControlRegister, val: u32, ) -> VirtioResult<()> { match reg { MmioControlRegister::DeviceFeaturesSel => { - transport.write_reg(ControlRegister::DeviceFeaturesSel, val) + dev.write_reg(ControlRegister::DeviceFeaturesSel, val) } MmioControlRegister::DriverFeatures => { - transport.write_reg(ControlRegister::DriverFeatures, val) + dev.write_reg(ControlRegister::DriverFeatures, val) } MmioControlRegister::DriverFeaturesSel => { - transport.write_reg(ControlRegister::DriverFeaturesSel, val) - } - MmioControlRegister::QueueSel => transport.write_reg(ControlRegister::QueueSel, val), - MmioControlRegister::QueueSize => transport.write_reg(ControlRegister::QueueSize, val), - MmioControlRegister::QueueReady => { - transport.write_reg(ControlRegister::QueueReady, val) - } - MmioControlRegister::QueueNotify => { - transport.write_reg(ControlRegister::QueueNotify, val) + dev.write_reg(ControlRegister::DriverFeaturesSel, val) } + MmioControlRegister::QueueSel => dev.write_reg(ControlRegister::QueueSel, val), + MmioControlRegister::QueueSize => dev.write_reg(ControlRegister::QueueSize, val), + MmioControlRegister::QueueReady => dev.write_reg(ControlRegister::QueueReady, val), + MmioControlRegister::QueueNotify => dev.write_reg(ControlRegister::QueueNotify, val), MmioControlRegister::InterruptAck => { - transport - .interrupt_status + // TODO + + dev.interrupt_status .remove(InterruptStatus::from_bits_truncate(val)); - if transport.interrupt_status.is_empty() { - transport.device.trigger_irq(false); + if dev.interrupt_status.is_empty() { + dev.device.trigger_irq(false); } Ok(()) } - MmioControlRegister::Status => transport.write_reg(ControlRegister::Status, val), - MmioControlRegister::QueueDescLow => { - transport.write_reg(ControlRegister::QueueDescLow, val) - } + MmioControlRegister::Status => dev.write_reg(ControlRegister::Status, val), + MmioControlRegister::QueueDescLow => dev.write_reg(ControlRegister::QueueDescLow, val), MmioControlRegister::QueueDescHigh => { - transport.write_reg(ControlRegister::QueueDescHigh, val) + dev.write_reg(ControlRegister::QueueDescHigh, val) } MmioControlRegister::QueueAvailLow => { - transport.write_reg(ControlRegister::QueueAvailLow, val) + dev.write_reg(ControlRegister::QueueAvailLow, val) } MmioControlRegister::QueueAvailHigh => { - transport.write_reg(ControlRegister::QueueAvailHigh, val) - } - MmioControlRegister::QueueUsedLow => { - transport.write_reg(ControlRegister::QueueUsedLow, val) + dev.write_reg(ControlRegister::QueueAvailHigh, val) } + MmioControlRegister::QueueUsedLow => dev.write_reg(ControlRegister::QueueUsedLow, val), MmioControlRegister::QueueUsedHigh => { - transport.write_reg(ControlRegister::QueueUsedHigh, val) + dev.write_reg(ControlRegister::QueueUsedHigh, val) } MmioControlRegister::ShmSel => todo!(), MmioControlRegister::QueueReset => todo!(), - _ => unreachable!("Try to write a RO register {reg:?}"), + _ => { + warn!(name = D::NAME, ?reg, "Try to write a RO register"); + Ok(()) // ignore the error + } } } } @@ -127,71 +123,89 @@ where } fn mmio_read(&self, offset: u64, len: usize, data: &mut [u8]) { - let transport = self.transport.lock().unwrap(); + let dev = self.dev.lock().unwrap(); - let offset: usize = offset.try_into().unwrap(); + let Ok(offset) = usize::try_from(offset) else { + warn!(name = D::NAME, offset, "offset too large"); + return; + }; if offset < CONFIGURATION_SPACE_OFFSET { if let Some(reg) = MmioControlRegister::from_repr(offset as u16) { - assert_eq!(len, 4); - assert_eq!(data.len(), 4); + assert_eq!(len, data.len()); // TODO: mmio_read can remove the `len` argument in the future + if data.len() == 4 { + let val = self.read_reg(&dev, reg); - let val = self.read_reg(&transport, reg); + debug!(name = D::NAME, ?reg, len, val, "virtio-mmio read"); - debug!(name = D::NAME, ?reg, len, val, "read reg from virtio-mmio"); - - data.copy_from_slice(&val.to_le_bytes()); + data.copy_from_slice(&val.to_le_bytes()); + } else { + warn!(name = D::NAME, ?reg, len, "invalid virtio-mmio access size"); + debug_assert!(false); + } } else { warn!( + name = D::NAME, offset, len, ?data, - "read from invalid offset of virtio-mmio device" + "read from invalid offset of the virtio-mmio device" ); - panic!() + debug_assert!(false) } - } else if let Err(err) = - transport.read_config(offset - CONFIGURATION_SPACE_OFFSET, len, data) - { - error!(?err, "Failed to read device configuration"); + } else if let Err(err) = dev.read_config(offset - CONFIGURATION_SPACE_OFFSET, len, data) { + error!(name = D::NAME, ?err, "Failed to read device configuration"); - panic!() + debug_assert!(false) } } fn mmio_write(&self, offset: u64, len: usize, data: &[u8]) { - let mut transport = self.transport.lock().unwrap(); + let mut dev = self.dev.lock().unwrap(); - debug!(name = D::NAME, offset, len, ?data); + debug!(name = D::NAME, offset, len, ?data, "virtio-mmio write"); - let offset: usize = offset.try_into().unwrap(); + let Ok(offset) = usize::try_from(offset) else { + warn!(name = D::NAME, offset, "offset too large"); + return; + }; if offset < CONFIGURATION_SPACE_OFFSET { if let Some(reg) = MmioControlRegister::from_repr(offset as u16) { - assert_eq!(len, 4); - assert_eq!(data.len(), 4); - - self.write_reg( - &mut transport, - reg, - u32::from_le_bytes(data.try_into().unwrap()), - ) - .unwrap(); + assert_eq!(len, data.len()); + if data.len() == 4 { + if let Err(err) = + self.write_reg(&mut dev, reg, u32::from_le_bytes(data.try_into().unwrap())) + { + error!( + name = D::NAME, + ?err, + offset, + len, + ?data, + "error while writing virtio-mmio control register" + ); + + debug_assert!(false) + } + } else { + warn!(name = D::NAME, ?reg, len, "invalid virtio-mmio access size"); + debug_assert!(false); + } } else { warn!( + name = D::NAME, offset, len, ?data, - "write from invalid offset of virtio-mmio device" + "write to invalid offset of the virtio-mmio device" ); - panic!() + debug_assert!(false) } - } else if let Err(err) = - transport.write_config(offset - CONFIGURATION_SPACE_OFFSET, len, data) - { - error!(?err, "Failed to write device configuration"); + } else if let Err(err) = dev.write_config(offset - CONFIGURATION_SPACE_OFFSET, len, data) { + error!(name = D::NAME, ?err, "Failed to write device configuration"); - panic!() + debug_assert!(false) } } } From d3d71a2ec69583a760b6f8c9264a81c9f31eb71d Mon Sep 17 00:00:00 2001 From: Zhang Junyu Date: Thu, 5 Mar 2026 22:53:09 +0800 Subject: [PATCH 5/6] refine: Remove useless handler for virtio-mmio --- crates/vm-virtio/src/transport/mmio.rs | 12 ++++++++++-- .../src/transport/mmio/mmio_handler.rs | 17 +++-------------- 2 files changed, 13 insertions(+), 16 deletions(-) diff --git a/crates/vm-virtio/src/transport/mmio.rs b/crates/vm-virtio/src/transport/mmio.rs index df72729..0fc85f8 100644 --- a/crates/vm-virtio/src/transport/mmio.rs +++ b/crates/vm-virtio/src/transport/mmio.rs @@ -10,7 +10,6 @@ use vm_mm::allocator::MemoryContainer; use crate::device::VirtioDevice; use crate::transport::VirtioDev; -use crate::transport::mmio::mmio_handler::Handler; mod control_register; mod mmio_handler; @@ -20,6 +19,15 @@ pub struct VirtioMmioTransport { dev: Arc>>, } +impl Clone for VirtioMmioTransport { + fn clone(&self) -> Self { + Self { + mmio_range: self.mmio_range.clone(), + dev: self.dev.clone(), + } + } +} + impl VirtioMmioTransport where C: MemoryContainer, @@ -46,7 +54,7 @@ where D: VirtioDevice, { fn mmio_range_handlers(&self) -> Vec> { - vec![Box::new(Handler::new(self.mmio_range, self.dev.clone()))] + vec![Box::new(self.clone())] } fn generate_dt(&self, fdt: &mut FdtWriter) -> Result<(), vm_fdt::Error> { diff --git a/crates/vm-virtio/src/transport/mmio/mmio_handler.rs b/crates/vm-virtio/src/transport/mmio/mmio_handler.rs index 9079553..b7c4eb0 100644 --- a/crates/vm-virtio/src/transport/mmio/mmio_handler.rs +++ b/crates/vm-virtio/src/transport/mmio/mmio_handler.rs @@ -1,6 +1,3 @@ -use std::sync::Arc; -use std::sync::Mutex; - use tracing::debug; use tracing::error; use tracing::warn; @@ -12,6 +9,7 @@ use crate::device::VirtioDevice; use crate::result::Result as VirtioResult; use crate::transport::VirtioDev; use crate::transport::control_register::ControlRegister; +use crate::transport::mmio::VirtioMmioTransport; use crate::transport::mmio::control_register::MmioControlRegister; use crate::types::interrupt_status::InterruptStatus; @@ -21,19 +19,10 @@ const VIRTIO_MMIO_MAGIC_VALUE: u32 = u32::from_le_bytes(*b"virt"); const VIRTIO_MMIO_VERSION: u32 = 0x2; const VIRTIO_MMIO_VENDOR_ID: u32 = u32::from_le_bytes(*b"QEMU"); -pub struct Handler { - mmio_range: MmioRange, - dev: Arc>>, -} - -impl Handler +impl VirtioMmioTransport where D: VirtioDevice, { - pub fn new(mmio_range: MmioRange, dev: Arc>>) -> Self { - Handler { mmio_range, dev } - } - fn read_reg(&self, dev: &VirtioDev, reg: MmioControlRegister) -> u32 { match reg { MmioControlRegister::MagicValue => VIRTIO_MMIO_MAGIC_VALUE, @@ -113,7 +102,7 @@ where } } -impl MmioHandler for Handler +impl MmioHandler for VirtioMmioTransport where C: MemoryContainer, D: VirtioDevice, From ea50ccafe1a02e086cbf35a40213e8887b4d28b0 Mon Sep 17 00:00:00 2001 From: Zhang Junyu Date: Thu, 5 Mar 2026 22:55:18 +0800 Subject: [PATCH 6/6] fix: Fix clippy --- crates/vm-virtio/src/transport/mmio.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/vm-virtio/src/transport/mmio.rs b/crates/vm-virtio/src/transport/mmio.rs index 0fc85f8..78e4da2 100644 --- a/crates/vm-virtio/src/transport/mmio.rs +++ b/crates/vm-virtio/src/transport/mmio.rs @@ -22,7 +22,7 @@ pub struct VirtioMmioTransport { impl Clone for VirtioMmioTransport { fn clone(&self) -> Self { Self { - mmio_range: self.mmio_range.clone(), + mmio_range: self.mmio_range, dev: self.dev.clone(), } }