From b033fddce10d5469a640d1070f353f2c9669e199 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Wed, 24 Dec 2025 16:22:32 +0100 Subject: [PATCH 1/5] fix(virtio-pci): remove shared memory structures ShMem and ShMemCfg We currently don't do anything with these regions anyway and the code would need some rewriting. --- src/drivers/virtio/transport/pci.rs | 118 ++-------------------------- 1 file changed, 6 insertions(+), 112 deletions(-) diff --git a/src/drivers/virtio/transport/pci.rs b/src/drivers/virtio/transport/pci.rs index 123e279856..b8a7e1aebb 100644 --- a/src/drivers/virtio/transport/pci.rs +++ b/src/drivers/virtio/transport/pci.rs @@ -182,7 +182,6 @@ pub struct UniCapsColl { pub(crate) com_cfg: ComCfg, pub(crate) notif_cfg: NotifCfg, pub(crate) isr_cfg: IsrStatus, - pub(crate) sh_mem_cfg_list: Vec, pub(crate) dev_cfg_list: Vec, } /// Wraps a [`CommonCfg`] in order to preserve @@ -532,106 +531,6 @@ impl IsrStatus { } } -/// Shared memory configuration structure of Virtio PCI devices. -/// See Virtio specification v1.1. - 4.1.4.7 -/// -/// Each shared memory region is defined via a single shared -/// memory structure. Each region is identified by an id indicated -/// via the capability.id field of PciCapRaw. -/// -/// The shared memory region is defined via a PciCap64 structure. -/// See Virtio specification v.1.1 - 4.1.4 for structure. -/// -// Only used for capabilities that require offsets or lengths -// larger than 4GB. -// #[repr(C)] -// struct PciCap64 { -// pci_cap: PciCap, -// offset_hi: u32, -// length_hi: u32 -pub struct ShMemCfg { - mem_addr: u64, - length: u64, - sh_mem: ShMem, - /// Shared memory regions are identified via an ID - /// See Virtio specification v1.1. - 4.1.4.7 - id: u8, -} - -impl ShMemCfg { - fn new(cap: &PciCap) -> Option { - if cap.bar.length < cap.len() + cap.offset() { - error!( - "Shared memory config of with id {} of device {:x}, does not fit into memory specified by bar {:x}!", - cap.cap.id, cap.dev_id, cap.bar.index - ); - return None; - } - - let offset = cap.cap.offset.to_ne(); - let length = cap.cap.length.to_ne(); - - let virt_addr_raw = cap.bar.mem_addr + offset; - let raw_ptr = ptr::with_exposed_provenance_mut::(virt_addr_raw.try_into().unwrap()); - - // Zero initialize shared memory area - unsafe { - for i in 0..usize::try_from(length).unwrap() { - *(raw_ptr.add(i)) = 0; - } - }; - - // Currently in place in order to ensure a safe cast below - // "len: cap.bar.length as usize" - // In order to remove this assert a safe conversion from - // kernel PciBar struct into usize must be made - assert!(mem::size_of::() == 8); - - Some(ShMemCfg { - mem_addr: virt_addr_raw, - length: cap.len(), - sh_mem: ShMem { - ptr: raw_ptr, - len: cap.bar.length as usize, - }, - id: cap.cap.id, - }) - } -} - -/// Defines a shared memory locate at location ptr with a length of len. -/// The shared memories Drop implementation does not dealloc the memory -/// behind the pointer but sets it to zero, to prevent leakage of data. -struct ShMem { - ptr: *mut u8, - len: usize, -} - -impl core::ops::Deref for ShMem { - type Target = [u8]; - - fn deref(&self) -> &[u8] { - unsafe { core::slice::from_raw_parts(self.ptr, self.len) } - } -} - -impl core::ops::DerefMut for ShMem { - fn deref_mut(&mut self) -> &mut [u8] { - unsafe { core::slice::from_raw_parts_mut(self.ptr, self.len) } - } -} - -// Upon drop the shared memory region is "deleted" with zeros. -impl Drop for ShMem { - fn drop(&mut self) { - for i in 0..self.len { - unsafe { - *(self.ptr.add(i)) = 0; - } - } - } -} - /// PciBar stores the virtual memory address and associated length of memory space /// a PCI device's physical memory indicated by the device's BAR has been mapped to. // @@ -708,7 +607,6 @@ pub(crate) fn map_caps(device: &PciDevice) -> Result) -> Result match ShMemCfg::new(&pci_cap) { - Some(sh_mem) => sh_mem_cfg_list.push(sh_mem), - None => { - let cap_id = pci_cap.cap.id; - error!( - "Shared Memory config capability with id {cap_id} of device {device_id:x} could not be used!" - ); - } - }, + CapCfgType::SharedMemory => { + let cap_id = pci_cap.cap.id; + error!( + "Shared Memory config capability with id {cap_id} of device {device_id:x} could not be used!" + ); + } CapCfgType::Device => dev_cfg_list.push(pci_cap), // PCI's configuration space is allowed to hold other structures, which are not virtio specific and are therefore ignored @@ -764,7 +659,6 @@ pub(crate) fn map_caps(device: &PciDevice) -> Result Date: Wed, 24 Dec 2025 17:02:19 +0100 Subject: [PATCH 2/5] fix(virtio/transport): remove unused ComCfg::dev_status --- src/drivers/virtio/transport/mmio.rs | 5 ----- src/drivers/virtio/transport/pci.rs | 5 ----- 2 files changed, 10 deletions(-) diff --git a/src/drivers/virtio/transport/mmio.rs b/src/drivers/virtio/transport/mmio.rs index 64a62cd1a4..d4ccf1a00e 100644 --- a/src/drivers/virtio/transport/mmio.rs +++ b/src/drivers/virtio/transport/mmio.rs @@ -136,11 +136,6 @@ impl ComCfg { ptr.queue_ready().read() } - /// Returns the device status field. - pub fn dev_status(&self) -> u8 { - self.com_cfg.as_ptr().status().read().bits() - } - /// Resets the device status field to zero. pub fn reset_dev(&mut self) { self.com_cfg diff --git a/src/drivers/virtio/transport/pci.rs b/src/drivers/virtio/transport/pci.rs index b8a7e1aebb..51d30fe75e 100644 --- a/src/drivers/virtio/transport/pci.rs +++ b/src/drivers/virtio/transport/pci.rs @@ -292,11 +292,6 @@ impl ComCfg { self.com_cfg.as_ptr() } - /// Returns the device status field. - pub fn dev_status(&self) -> u8 { - self.com_cfg.as_ptr().device_status().read().bits() - } - /// Resets the device status field to zero. pub fn reset_dev(&mut self) { self.com_cfg From 5b162d1c44ff6a71209b9cefd675ea4d957d22b3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Thu, 25 Dec 2025 00:03:36 +0100 Subject: [PATCH 3/5] fix(virtio-mmio): remove unused ComCfg::get_queue_ready --- src/drivers/virtio/transport/mmio.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/drivers/virtio/transport/mmio.rs b/src/drivers/virtio/transport/mmio.rs index d4ccf1a00e..ba4a5ea8e1 100644 --- a/src/drivers/virtio/transport/mmio.rs +++ b/src/drivers/virtio/transport/mmio.rs @@ -130,12 +130,6 @@ impl ComCfg { ptr.queue_num_max().read().to_ne() } - pub fn get_queue_ready(&mut self, sel: u16) -> bool { - let ptr = self.com_cfg.as_mut_ptr(); - ptr.queue_sel().write(sel.into()); - ptr.queue_ready().read() - } - /// Resets the device status field to zero. pub fn reset_dev(&mut self) { self.com_cfg From 51e956198d74531e45cb3ba35541feeda17a9c72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Wed, 24 Dec 2025 19:13:54 +0100 Subject: [PATCH 4/5] fix(virtio-pci): gate NotifCfg::length --- src/drivers/virtio/transport/pci.rs | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/drivers/virtio/transport/pci.rs b/src/drivers/virtio/transport/pci.rs index 51d30fe75e..8d98203839 100644 --- a/src/drivers/virtio/transport/pci.rs +++ b/src/drivers/virtio/transport/pci.rs @@ -415,6 +415,7 @@ pub struct NotifCfg { base_addr: u64, notify_off_multiplier: u32, /// defines the maximum size of the notification space, starting from base_addr. + #[cfg(debug_assertions)] length: u64, } @@ -443,12 +444,21 @@ impl NotifCfg { Some(NotifCfg { base_addr, notify_off_multiplier, + #[cfg(debug_assertions)] length: cap.len(), }) } pub fn notification_location(&self, vq_cfg_handler: &mut VqCfgHandler<'_>) -> *mut le32 { let addend = u32::from(vq_cfg_handler.notif_off()) * self.notify_off_multiplier; + + // TODO: This should be + // cap.length >= queue_notify_off * notify_off_multiplier + 4 + // if VIRTIO_F_NOTIFICATION_DATA has been negotiated. + // Knowing this here requires a larger refactoring. + #[cfg(debug_assertions)] + assert!(self.length >= u64::from(addend + 2)); + let addr = self.base_addr + u64::from(addend); ptr::with_exposed_provenance_mut(addr.try_into().unwrap()) } From 3e8c15e03862eceaaec6ba8b377b3dc24a6b65b0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Kr=C3=B6ning?= Date: Mon, 29 Dec 2025 23:17:15 +0100 Subject: [PATCH 5/5] fix(mmio): print virtio-console info --- src/drivers/console/mmio.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/drivers/console/mmio.rs b/src/drivers/console/mmio.rs index 1643c9d3fc..7bef82ea95 100644 --- a/src/drivers/console/mmio.rs +++ b/src/drivers/console/mmio.rs @@ -55,6 +55,7 @@ impl VirtioConsoleDriver { ) -> Result { let mut drv = VirtioConsoleDriver::new(dev_id, registers, irq)?; drv.init_dev().map_err(VirtioError::ConsoleDriver)?; + drv.com_cfg.print_information(); Ok(drv) } }