From 5192816de7760c6c359559943cbba9dc5d4df686 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 21 Jan 2024 21:12:24 +0100 Subject: [PATCH] Introduce expect_port and expect_mem helpers --- ac97d/src/main.rs | 11 ++----- ahcid/src/main.rs | 6 +--- e1000d/src/main.rs | 6 +--- ided/src/main.rs | 5 +--- ihdad/src/main.rs | 15 ++-------- ixgbed/src/main.rs | 6 +--- nvmed/src/main.rs | 24 ++-------------- pcid/src/pci/bar.rs | 37 ++++++++++++++++++------ pcid/src/pci/msi.rs | 36 ++--------------------- rtl8139d/src/main.rs | 14 +-------- rtl8168d/src/main.rs | 12 +------- vboxd/src/main.rs | 11 ++----- virtio-core/src/arch/x86.rs | 44 ++++++++++++++-------------- virtio-core/src/arch/x86_64.rs | 52 ++++++++++++++-------------------- virtio-core/src/probe.rs | 8 +----- xhcid/src/main.rs | 28 ++---------------- 16 files changed, 91 insertions(+), 224 deletions(-) diff --git a/ac97d/src/main.rs b/ac97d/src/main.rs index a1e6760070..04a5f3f7be 100644 --- a/ac97d/src/main.rs +++ b/ac97d/src/main.rs @@ -73,15 +73,8 @@ fn main() { let mut name = pci_config.func.name(); name.push_str("_ac97"); - let bar0 = match pci_config.func.bars[0] { - PciBar::Port(port) => port, - _ => unreachable!(), - }; - - let bar1 = match pci_config.func.bars[1] { - PciBar::Port(port) => port, - _ => unreachable!(), - }; + let bar0 = pci_config.func.bars[0].expect_port(); + let bar1 = pci_config.func.bars[1].expect_port(); let irq = pci_config.func.legacy_interrupt_line; diff --git a/ahcid/src/main.rs b/ahcid/src/main.rs index f41f397e73..bd8e6af790 100644 --- a/ahcid/src/main.rs +++ b/ahcid/src/main.rs @@ -81,11 +81,7 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { let mut name = pci_config.func.name(); name.push_str("_ahci"); - let bar = match pci_config.func.bars[5] { - PciBar::Memory32(addr) => addr as usize, - PciBar::Memory64(addr) => addr as usize, - PciBar::None | PciBar::Port(_) => unreachable!(), - }; + let bar = pci_config.func.bars[5].expect_mem(); let bar_size = pci_config.func.bar_sizes[5]; let irq = pci_config.func.legacy_interrupt_line; diff --git a/e1000d/src/main.rs b/e1000d/src/main.rs index 328c7d0d3c..58a8a9f4ed 100644 --- a/e1000d/src/main.rs +++ b/e1000d/src/main.rs @@ -68,11 +68,7 @@ fn main() { let mut name = pci_config.func.name(); name.push_str("_e1000"); - let bar = match pci_config.func.bars[0] { - PciBar::Memory32(addr) => addr as usize, - PciBar::Memory64(addr) => addr as usize, - PciBar::None | PciBar::Port(_) => unreachable!(), - }; + let bar = pci_config.func.bars[0].expect_mem(); let bar_size = pci_config.func.bar_sizes[0] as usize; let irq = pci_config.func.legacy_interrupt_line; diff --git a/ided/src/main.rs b/ided/src/main.rs index 603a20baab..bf176e4986 100644 --- a/ided/src/main.rs +++ b/ided/src/main.rs @@ -86,10 +86,7 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { info!("IDE PCI CONFIG: {:?}", pci_config); - let busmaster_base = match pci_config.func.bars[4] { - PciBar::Port(port) => port, - other => panic!("TODO: IDE busmaster BAR {:#x?}", other), - }; + let busmaster_base = pci_config.func.bars[4].expect_port(); let (primary, primary_irq) = if pci_config.func.full_device_id.interface & 1 != 0 { panic!("TODO: IDE primary channel is PCI native"); } else { diff --git a/ihdad/src/main.rs b/ihdad/src/main.rs index 16ab7ea4a9..a4ff9a98f0 100755 --- a/ihdad/src/main.rs +++ b/ihdad/src/main.rs @@ -160,24 +160,13 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { let mut name = pci_config.func.name(); name.push_str("_ihda"); - let bar = pci_config.func.bars[0]; + let bar_ptr = pci_config.func.bars[0].expect_mem(); let bar_size = pci_config.func.bar_sizes[0]; - let bar_ptr = match bar { - pcid_interface::PciBar::Memory32(ptr) => match ptr { - 0 => panic!("BAR 0 is mapped to address 0"), - _ => ptr as u64, - }, - pcid_interface::PciBar::Memory64(ptr) => match ptr { - 0 => panic!("BAR 0 is mapped to address 0"), - _ => ptr, - }, - other => panic!("Expected memory bar, found {:?}", other), - }; log::info!(" + IHDA {} on: {:#X} size: {}", name, bar_ptr, bar_size); let address = unsafe { - common::physmap(bar_ptr as usize, bar_size as usize, common::Prot::RW, common::MemoryType::Uncacheable) + common::physmap(bar_ptr, bar_size as usize, common::Prot::RW, common::MemoryType::Uncacheable) .expect("ihdad: failed to map address") as usize }; diff --git a/ixgbed/src/main.rs b/ixgbed/src/main.rs index b60cfb9982..a0b3d46c39 100644 --- a/ixgbed/src/main.rs +++ b/ixgbed/src/main.rs @@ -76,11 +76,7 @@ fn main() { let mut name = pci_config.func.name(); name.push_str("_ixgbe"); - let bar = match pci_config.func.bars[0] { - PciBar::Memory32(addr) => addr as usize, - PciBar::Memory64(addr) => addr as usize, - PciBar::None | PciBar::Port(_) => unreachable!(), - }; + let bar = pci_config.func.bars[0].expect_mem(); let irq = pci_config.func.legacy_interrupt_line; diff --git a/nvmed/src/main.rs b/nvmed/src/main.rs index 2fed31c18c..07933e1659 100644 --- a/nvmed/src/main.rs +++ b/nvmed/src/main.rs @@ -93,17 +93,7 @@ fn get_int_method( match &mut *bar_guard { &mut Some(ref bar) => Ok(bar.ptr), bar_to_set @ &mut None => { - let bar = match function.bars[bir] { - PciBar::Memory32(addr) => match addr { - 0 => panic!("BAR {} is mapped to address 0", bir), - _ => addr as u64, - }, - PciBar::Memory64(addr) => match addr { - 0 => panic!("BAR {} is mapped to address 0", bir), - _ => addr, - }, - other => panic!("Expected memory BAR, found {:?}", other), - }; + let bar = function.bars[bir].expect_mem(); let bar_size = function.bar_sizes[bir]; let bar = Bar::allocate(bar as usize, bar_size as usize)?; @@ -303,17 +293,7 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { let _logger_ref = setup_logging(&scheme_name); - let bar = match pci_config.func.bars[0] { - PciBar::Memory32(mem) => match mem { - 0 => panic!("BAR 0 is mapped to address 0"), - _ => mem as u64, - }, - PciBar::Memory64(mem) => match mem { - 0 => panic!("BAR 0 is mapped to address 0"), - _ => mem, - }, - other => panic!("received a non-memory BAR ({:?})", other), - }; + let bar = pci_config.func.bars[0].expect_mem(); let bar_size = pci_config.func.bar_sizes[0]; let irq = pci_config.func.legacy_interrupt_line; diff --git a/pcid/src/pci/bar.rs b/pcid/src/pci/bar.rs index e51c0b31d2..f0d3a14911 100644 --- a/pcid/src/pci/bar.rs +++ b/pcid/src/pci/bar.rs @@ -1,11 +1,13 @@ -use serde::{Serialize, Deserialize}; +use std::convert::TryInto; + +use serde::{Deserialize, Serialize}; #[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] pub enum PciBar { None, Memory32(u32), Memory64(u64), - Port(u16) + Port(u16), } impl PciBar { @@ -15,6 +17,27 @@ impl PciBar { _ => false, } } + + pub fn expect_port(&self) -> u16 { + match *self { + PciBar::Port(port) => port, + PciBar::Memory32(_) | PciBar::Memory64(_) => { + panic!("expected port BAR, found memory BAR"); + } + PciBar::None => panic!("expected BAR to exist"), + } + } + + pub fn expect_mem(&self) -> usize { + match *self { + PciBar::Memory32(ptr) => ptr as usize, + PciBar::Memory64(ptr) => ptr + .try_into() + .expect("conversion from 64bit BAR to usize failed"), + PciBar::Port(_) => panic!("expected memory BAR, found port BAR"), + PciBar::None => panic!("expected BAR to exist"), + } + } } impl From for PciBar { @@ -23,16 +46,12 @@ impl From for PciBar { PciBar::None } else if bar & 1 == 0 { match (bar >> 1) & 3 { - 0 => { - PciBar::Memory32(bar & 0xFFFFFFF0) - }, - 2 => { - PciBar::Memory64((bar & 0xFFFFFFF0) as u64) - }, + 0 => PciBar::Memory32(bar & 0xFFFFFFF0), + 2 => PciBar::Memory64((bar & 0xFFFFFFF0) as u64), other => { log::warn!("unsupported PCI memory type {}", other); PciBar::None - }, + } } } else { PciBar::Port((bar & 0xFFFC) as u16) diff --git a/pcid/src/pci/msi.rs b/pcid/src/pci/msi.rs index fc8a25f4a3..b4d3c67ca1 100644 --- a/pcid/src/pci/msi.rs +++ b/pcid/src/pci/msi.rs @@ -219,13 +219,9 @@ impl MsixCapability { self.a &= 0x0000_FFFF; self.a |= u32::from(message_control) << 16; } - /// Returns the MSI-X table size, subtracted by one. - pub const fn table_size_raw(&self) -> u16 { - self.message_control() & Self::MC_TABLE_SIZE_MASK - } /// Returns the MSI-X table size. pub const fn table_size(&self) -> u16 { - self.table_size_raw() + 1 + (self.message_control() & Self::MC_TABLE_SIZE_MASK) + 1 } /// Returns the MSI-X enabled bit, which enables MSI-X if the MSI enable bit is also set in the /// MSI capability structure. @@ -289,20 +285,7 @@ impl MsixCapability { if self.table_bir() > 5 { panic!("MSI-X Table BIR contained a reserved enum value: {}", self.table_bir()); } - let base = bars[usize::from(self.table_bir())]; - - //TODO: ensure type conversions are safe - match base { - PciBar::Memory32(ptr) => { - ptr as usize + self.table_offset() as usize - }, - PciBar::Memory64(ptr) => { - ptr as usize + self.table_offset() as usize - }, - _ => { - panic!("MSI-X Table BIR referenced a non-memory BAR: {:?}", base); - } - } + bars[usize::from(self.table_bir())].expect_mem() + self.table_offset() as usize } pub fn table_pointer(&self, bars: [PciBar; 6], k: u16) -> usize { self.table_base_pointer(bars) + k as usize * 16 @@ -312,20 +295,7 @@ impl MsixCapability { if self.pba_bir() > 5 { panic!("MSI-X PBA BIR contained a reserved enum value: {}", self.pba_bir()); } - let base = bars[usize::from(self.pba_bir())]; - - //TODO: ensure type conversions are safe - match base { - PciBar::Memory32(ptr) => { - ptr as usize + self.pba_offset() as usize - }, - PciBar::Memory64(ptr) => { - ptr as usize + self.pba_offset() as usize - }, - _ => { - panic!("MSI-X PBA BIR referenced a non-memory BAR: {:?}", base); - } - } + bars[usize::from(self.pba_bir())].expect_mem() + self.pba_offset() as usize } pub fn pba_pointer_dword(&self, bars: [PciBar; 6], k: u16) -> usize { self.pba_base_pointer(bars) + (k as usize / 32) * 4 diff --git a/rtl8139d/src/main.rs b/rtl8139d/src/main.rs index fd26b06812..62925a1ec2 100644 --- a/rtl8139d/src/main.rs +++ b/rtl8139d/src/main.rs @@ -171,21 +171,9 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> Option { let pba_base = capability.pba_base_pointer(pci_config.func.bars); let bir = capability.table_bir() as usize; - let bar = pci_config.func.bars[bir]; + let bar_ptr = pci_config.func.bars[bir].expect_mem() as u64; let bar_size = pci_config.func.bar_sizes[bir] as u64; - let bar_ptr = match bar { - pcid_interface::PciBar::Memory32(ptr) => match ptr { - 0 => panic!("BAR {} is mapped to address 0", bir), - _ => ptr as u64, - }, - pcid_interface::PciBar::Memory64(ptr) => match ptr { - 0 => panic!("BAR {} is mapped to address 0", bir), - _ => ptr, - }, - other => panic!("Expected memory bar, found {:?}", other), - }; - let address = unsafe { common::physmap(bar_ptr as usize, bar_size as usize, common::Prot::RW, common::MemoryType::Uncacheable) .expect("rtl8139d: failed to map address") as usize diff --git a/rtl8168d/src/main.rs b/rtl8168d/src/main.rs index 53415a2d0f..284b84c3e4 100644 --- a/rtl8168d/src/main.rs +++ b/rtl8168d/src/main.rs @@ -172,17 +172,7 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle) -> Option { let bar = pci_config.func.bars[bir]; let bar_size = pci_config.func.bar_sizes[bir] as u64; - let bar_ptr = match bar { - pcid_interface::PciBar::Memory32(ptr) => match ptr { - 0 => panic!("BAR {} is mapped to address 0", bir), - _ => ptr as u64, - }, - pcid_interface::PciBar::Memory64(ptr) => match ptr { - 0 => panic!("BAR {} is mapped to address 0", bir), - _ => ptr, - }, - other => panic!("Expected memory bar, found {:?}", other), - }; + let bar_ptr = bar.expect_mem() as u64; let address = unsafe { common::physmap(bar_ptr as usize, bar_size as usize, common::Prot::RW, common::MemoryType::Uncacheable) diff --git a/vboxd/src/main.rs b/vboxd/src/main.rs index 555b5eb675..0602649c5c 100644 --- a/vboxd/src/main.rs +++ b/vboxd/src/main.rs @@ -194,16 +194,9 @@ fn main() { let mut name = pci_config.func.name(); name.push_str("_vbox"); - let bar0 = match pci_config.func.bars[0] { - PciBar::Port(port) => port, - _ => unreachable!(), - }; + let bar0 = pci_config.func.bars[0].expect_port(); - let bar1 = match pci_config.func.bars[1] { - PciBar::Memory32(addr) => addr as usize, - PciBar::Memory64(addr) => addr as usize, - PciBar::None | PciBar::Port(_) => unreachable!(), - }; + let bar1 = pci_config.func.bars[1].expect_mem(); let irq = pci_config.func.legacy_interrupt_line; diff --git a/virtio-core/src/arch/x86.rs b/virtio-core/src/arch/x86.rs index 56b33d8fd8..45d67b9a1c 100644 --- a/virtio-core/src/arch/x86.rs +++ b/virtio-core/src/arch/x86.rs @@ -11,33 +11,31 @@ pub fn probe_legacy_port_transport( pci_header: &PciHeader, pcid_handle: &mut PcidServerHandle, ) -> Result { - if let PciBar::Port(port) = pci_header.get_bar(0) { - unsafe { syscall::iopl(3).expect("virtio: failed to set I/O privilege level") }; - log::warn!("virtio: using legacy transport"); + let port = pci_header.get_bar(0).expect_port(); - let transport = LegacyTransport::new(port); + unsafe { syscall::iopl(3).expect("virtio: failed to set I/O privilege level") }; + log::warn!("virtio: using legacy transport"); - // Setup interrupts. - let all_pci_features = pcid_handle.fetch_all_features()?; - let has_msix = all_pci_features - .iter() - .any(|(feature, _)| feature.is_msix()); + let transport = LegacyTransport::new(port); - // According to the virtio specification, the device REQUIRED to support MSI-X. - assert!(has_msix, "virtio: device does not support MSI-X"); - let irq_handle = enable_msix(pcid_handle)?; + // Setup interrupts. + let all_pci_features = pcid_handle.fetch_all_features()?; + let has_msix = all_pci_features + .iter() + .any(|(feature, _)| feature.is_msix()); - let device = Device { - transport, - irq_handle, - device_space: core::ptr::null_mut(), - }; + // According to the virtio specification, the device REQUIRED to support MSI-X. + assert!(has_msix, "virtio: device does not support MSI-X"); + let irq_handle = enable_msix(pcid_handle)?; - device.transport.reset(); - reinit(&device)?; + let device = Device { + transport, + irq_handle, + device_space: core::ptr::null_mut(), + }; - Ok(device) - } else { - unreachable!("virtio: legacy transport with non-port IO?") - } + device.transport.reset(); + reinit(&device)?; + + Ok(device) } diff --git a/virtio-core/src/arch/x86_64.rs b/virtio-core/src/arch/x86_64.rs index 502bfcdafa..2dd3cd27d3 100644 --- a/virtio-core/src/arch/x86_64.rs +++ b/virtio-core/src/arch/x86_64.rs @@ -27,15 +27,9 @@ pub fn enable_msix(pcid_handle: &mut PcidServerHandle) -> Result { let pba_base = capability.pba_base_pointer(pci_config.func.bars); let bir = capability.table_bir() as usize; - let bar = pci_config.func.bars[bir]; + let bar_ptr = pci_config.func.bars[bir].expect_mem() as u64; let bar_size = pci_config.func.bar_sizes[bir] as u64; - let bar_ptr = match bar { - PciBar::Memory32(ptr) => ptr.into(), - PciBar::Memory64(ptr) => ptr, - _ => unreachable!(), - }; - let address = unsafe { common::physmap( bar_ptr as usize, @@ -99,33 +93,31 @@ pub fn probe_legacy_port_transport( pci_config: &SubdriverArguments, pcid_handle: &mut PcidServerHandle, ) -> Result { - if let PciBar::Port(port) = pci_config.func.bars[0] { - unsafe { syscall::iopl(3).expect("virtio: failed to set I/O privilege level") }; - log::warn!("virtio: using legacy transport"); + let port = pci_config.func.bars[0].expect_port(); - let transport = LegacyTransport::new(port); + unsafe { syscall::iopl(3).expect("virtio: failed to set I/O privilege level") }; + log::warn!("virtio: using legacy transport"); - // Setup interrupts. - let all_pci_features = pcid_handle.fetch_all_features()?; - let has_msix = all_pci_features - .iter() - .any(|(feature, _)| feature.is_msix()); + let transport = LegacyTransport::new(port); - // According to the virtio specification, the device REQUIRED to support MSI-X. - assert!(has_msix, "virtio: device does not support MSI-X"); - let irq_handle = enable_msix(pcid_handle)?; + // Setup interrupts. + let all_pci_features = pcid_handle.fetch_all_features()?; + let has_msix = all_pci_features + .iter() + .any(|(feature, _)| feature.is_msix()); - let device = Device { - transport, - irq_handle, - device_space: core::ptr::null_mut(), - }; + // According to the virtio specification, the device REQUIRED to support MSI-X. + assert!(has_msix, "virtio: device does not support MSI-X"); + let irq_handle = enable_msix(pcid_handle)?; - device.transport.reset(); - reinit(&device)?; + let device = Device { + transport, + irq_handle, + device_space: core::ptr::null_mut(), + }; - Ok(device) - } else { - unreachable!("virtio: legacy transport with non-port IO?") - } + device.transport.reset(); + reinit(&device)?; + + Ok(device) } diff --git a/virtio-core/src/probe.rs b/virtio-core/src/probe.rs index 292bf429a6..0ad300a673 100644 --- a/virtio-core/src/probe.rs +++ b/virtio-core/src/probe.rs @@ -90,13 +90,7 @@ pub fn probe_device(pcid_handle: &mut PcidServerHandle) -> Result _ => continue, } - let bar = pci_config.func.bars[capability.bar as usize]; - let addr = match bar { - PciBar::Memory32(addr) => addr as usize, - PciBar::Memory64(addr) => addr as usize, - - _ => unreachable!("virtio: unsupported bar type: {bar:?}"), - }; + let addr = pci_config.func.bars[capability.bar as usize].expect_mem(); let address = unsafe { let addr = addr + capability.offset as usize; diff --git a/xhcid/src/main.rs b/xhcid/src/main.rs index 6e15e8b0da..497fb01a16 100644 --- a/xhcid/src/main.rs +++ b/xhcid/src/main.rs @@ -85,22 +85,10 @@ fn setup_logging(name: &str) -> Option<&'static RedoxLogger> { fn get_int_method(pcid_handle: &mut PcidServerHandle, address: usize) -> (Option, InterruptMethod) { let pci_config = pcid_handle.fetch_config().expect("xhcid: failed to fetch config"); - let bar = pci_config.func.bars[0]; + let bar_ptr = pci_config.func.bars[0].expect_mem() as u64; let bar_size = pci_config.func.bar_sizes[0] as u64; let irq = pci_config.func.legacy_interrupt_line; - let bar_ptr = match bar { - pcid_interface::PciBar::Memory32(ptr) => match ptr { - 0 => panic!("BAR 0 is mapped to address 0"), - _ => ptr as u64, - }, - pcid_interface::PciBar::Memory64(ptr) => match ptr { - 0 => panic!("BAR 0 is mapped to address 0"), - _ => ptr, - }, - other => panic!("Expected memory bar, found {:?}", other), - }; - let all_pci_features = pcid_handle.fetch_all_features().expect("xhcid: failed to fetch pci features"); log::debug!("XHCI PCI FEATURES: {:?}", all_pci_features); @@ -247,22 +235,10 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { let _logger_ref = setup_logging(&name); log::debug!("XHCI PCI CONFIG: {:?}", pci_config); - let bar = pci_config.func.bars[0]; + let bar_ptr = pci_config.func.bars[0].expect_mem(); let bar_size = pci_config.func.bar_sizes[0]; let irq = pci_config.func.legacy_interrupt_line; - let bar_ptr = match bar { - pcid_interface::PciBar::Memory32(ptr) => match ptr { - 0 => panic!("BAR 0 is mapped to address 0"), - _ => ptr as u64, - }, - pcid_interface::PciBar::Memory64(ptr) => match ptr { - 0 => panic!("BAR 0 is mapped to address 0"), - _ => ptr, - }, - other => panic!("Expected memory bar, found {:?}", other), - }; - let address = unsafe { common::physmap(bar_ptr as usize, bar_size as usize, common::Prot::RW, common::MemoryType::Uncacheable) .expect("xhcid: failed to map address") as usize