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 1/9] 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 From cc015eab13b5c3d9f8ee73910911c4d9d6edc9c9 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Sun, 21 Jan 2024 21:25:50 +0100 Subject: [PATCH 2/9] Move PciHeader out of the pcid_interface crate --- pcid/src/main.rs | 4 +++- pcid/src/pci/mod.rs | 2 -- pcid/src/{pci/header.rs => pci_header.rs} | 12 +++++------- 3 files changed, 8 insertions(+), 10 deletions(-) rename pcid/src/{pci/header.rs => pci_header.rs} (98%) diff --git a/pcid/src/main.rs b/pcid/src/main.rs index 74c3f80688..13d16a2835 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -11,14 +11,16 @@ use redox_log::{OutputBuilder, RedoxLogger}; use crate::cfg_access::Pcie; use crate::config::Config; -use crate::pci::{CfgAccess, PciAddress, PciBar, PciClass, PciFunc, PciHeader, PciHeaderError, PciHeaderType}; +use crate::pci::{CfgAccess, PciAddress, PciBar, PciClass, PciFunc}; use crate::pci::cap::Capability as PciCapability; use crate::pci::func::{ConfigReader, ConfigWriter}; +use crate::pci_header::{PciHeader, PciHeaderError, PciHeaderType}; mod cfg_access; mod config; mod driver_interface; mod pci; +mod pci_header; #[derive(StructOpt)] #[structopt(about)] diff --git a/pcid/src/pci/mod.rs b/pcid/src/pci/mod.rs index 264a3f35dd..8f5e661ab7 100644 --- a/pcid/src/pci/mod.rs +++ b/pcid/src/pci/mod.rs @@ -6,14 +6,12 @@ use serde::{Deserialize, Serialize}; pub use self::bar::PciBar; pub use self::class::PciClass; pub use self::func::PciFunc; -pub use self::header::{PciHeader, PciHeaderError, PciHeaderType}; pub use self::id::FullDeviceId; mod bar; pub mod cap; mod class; pub mod func; -pub mod header; mod id; pub mod msi; diff --git a/pcid/src/pci/header.rs b/pcid/src/pci_header.rs similarity index 98% rename from pcid/src/pci/header.rs rename to pcid/src/pci_header.rs index 00de49ed30..cb3c59405f 100644 --- a/pcid/src/pci/header.rs +++ b/pcid/src/pci_header.rs @@ -2,10 +2,8 @@ use bitflags::bitflags; use byteorder::{ByteOrder, LittleEndian}; use serde::{Deserialize, Serialize}; -use super::bar::PciBar; -use super::class::PciClass; -use super::func::ConfigReader; -use super::id::FullDeviceId; +use crate::pci::{FullDeviceId, PciBar, PciClass}; +use crate::pci::func::ConfigReader; #[derive(Debug, PartialEq)] pub enum PciHeaderError { @@ -38,6 +36,7 @@ pub struct SharedPciHeader { header_type: PciHeaderType, } +// FIXME move out of pcid_interface #[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] pub enum PciHeader { General { @@ -297,9 +296,8 @@ impl<'a> ConfigReader for &'a [u8] { #[cfg(test)] mod test { use super::{PciHeaderError, PciHeader, PciHeaderType}; - use super::super::func::ConfigReader; - use super::super::class::PciClass; - use super::super::bar::PciBar; + use crate::pci::func::ConfigReader; + use crate::pci::{PciBar, PciClass}; const IGB_DEV_BYTES: [u8; 256] = [ 0x86, 0x80, 0x33, 0x15, 0x07, 0x04, 0x10, 0x00, 0x03, 0x00, 0x00, 0x02, 0x10, 0x00, 0x00, 0x00, From d56881de88847c3e326a34fefe9c10229dd36516 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 22 Jan 2024 11:00:17 +0100 Subject: [PATCH 3/9] Use CfgAccess instead of ConfigReader in PciHeader::from_reader This will enable getting BAR sizes directly inside this function in the future. --- pcid/src/main.rs | 3 +- pcid/src/pci_header.rs | 121 +++++++++++++++++++++++------------------ 2 files changed, 69 insertions(+), 55 deletions(-) diff --git a/pcid/src/main.rs b/pcid/src/main.rs index 13d16a2835..793505276b 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -524,8 +524,7 @@ fn main(args: Args) { 'dev: for dev_num in 0..32 { for func_num in 0..8 { let func_addr = PciAddress::new(0, bus_num, dev_num, func_num); - let func = PciFunc { pci, addr: func_addr }; - match PciHeader::from_reader(func) { + match PciHeader::from_reader(pci, func_addr) { Ok(header) => { handle_parsed_header(Arc::clone(&state), &config, func_addr, header); if let PciHeader::PciToPci { secondary_bus_num, .. } = header { diff --git a/pcid/src/pci_header.rs b/pcid/src/pci_header.rs index cb3c59405f..f904e1e930 100644 --- a/pcid/src/pci_header.rs +++ b/pcid/src/pci_header.rs @@ -2,8 +2,7 @@ use bitflags::bitflags; use byteorder::{ByteOrder, LittleEndian}; use serde::{Deserialize, Serialize}; -use crate::pci::{FullDeviceId, PciBar, PciClass}; -use crate::pci::func::ConfigReader; +use crate::pci::{CfgAccess, FullDeviceId, PciAddress, PciBar, PciClass}; #[derive(Debug, PartialEq)] pub enum PciHeaderError { @@ -89,10 +88,19 @@ impl PciHeader { /// Parse the bytes found in the Configuration Space of the PCI device into /// a more usable PciHeader. - pub fn from_reader(reader: T) -> Result { - if unsafe { reader.read_u32(0) } != 0xffffffff { + pub fn from_reader( + cfg_access: &dyn CfgAccess, + addr: PciAddress, + ) -> Result { + if unsafe { cfg_access.read(addr, 0) } != 0xffffffff { // Read the initial 16 bytes and set variables used by all header types. - let bytes = unsafe { reader.read_range(0, 16) }; + let bytes = unsafe { + let mut ret = Vec::with_capacity(16); + for offset in (0..16).step_by(4) { + ret.extend(cfg_access.read(addr, offset).to_le_bytes()); + } + ret + }; let vendor_id = LittleEndian::read_u16(&bytes[0..2]); let device_id = LittleEndian::read_u16(&bytes[2..4]); let command = LittleEndian::read_u16(&bytes[4..6]); @@ -118,7 +126,13 @@ impl PciHeader { match header_type & PciHeaderType::HEADER_TYPE { PciHeaderType::GENERAL => { - let bytes = unsafe { reader.read_range(16, 48) }; + let bytes = unsafe { + let mut ret = Vec::with_capacity(48); + for offset in (16..64).step_by(4) { + ret.extend(cfg_access.read(addr, offset).to_le_bytes()); + } + ret + }; let mut bars = [PciBar::None; 6]; Self::get_bars(&bytes, &mut bars); let subsystem_vendor_id = LittleEndian::read_u16(&bytes[28..30]); @@ -137,7 +151,13 @@ impl PciHeader { }) } PciHeaderType::PCITOPCI => { - let bytes = unsafe { reader.read_range(16, 48) }; + let bytes = unsafe { + let mut ret = Vec::with_capacity(48); + for offset in (16..64).step_by(4) { + ret.extend(cfg_access.read(addr, offset).to_le_bytes()); + } + ret + }; let mut bars = [PciBar::None; 2]; Self::get_bars(&bytes, &mut bars); let secondary_bus_num = bytes[9]; @@ -284,20 +304,30 @@ impl PciHeader { } } -#[cfg(test)] -impl<'a> ConfigReader for &'a [u8] { - unsafe fn read_u32(&self, offset: u16) -> u32 { - let offset = offset as usize; - assert!(offset < self.len()); - LittleEndian::read_u32(&self[offset..offset + 4]) - } -} - #[cfg(test)] mod test { - use super::{PciHeaderError, PciHeader, PciHeaderType}; - use crate::pci::func::ConfigReader; - use crate::pci::{PciBar, PciClass}; + use std::convert::TryInto; + + use super::{PciHeader, PciHeaderError, PciHeaderType}; + use crate::pci::{CfgAccess, PciAddress, PciBar, PciClass}; + + struct TestCfgAccess<'a> { + addr: PciAddress, + bytes: &'a [u8], + } + + impl CfgAccess for TestCfgAccess<'_> { + unsafe fn read(&self, addr: PciAddress, offset: u16) -> u32 { + assert_eq!(addr, self.addr); + let offset = offset as usize; + assert!(offset < self.bytes.len()); + u32::from_le_bytes(self.bytes[offset..offset + 4].try_into().unwrap()) + } + + unsafe fn write(&self, _addr: PciAddress, _offset: u16, _value: u32) { + unreachable!("should not write during tests"); + } + } const IGB_DEV_BYTES: [u8; 256] = [ 0x86, 0x80, 0x33, 0x15, 0x07, 0x04, 0x10, 0x00, 0x03, 0x00, 0x00, 0x02, 0x10, 0x00, 0x00, 0x00, @@ -320,7 +350,14 @@ mod test { #[test] fn tset_parse_igb_dev() { - let header = PciHeader::from_reader(&IGB_DEV_BYTES[..]).unwrap(); + let header = PciHeader::from_reader( + &TestCfgAccess { + addr: PciAddress::new(0, 2, 4, 0), + bytes: &IGB_DEV_BYTES, + }, + PciAddress::new(0, 2, 4, 0), + ) + .unwrap(); assert_eq!(header.header_type(), PciHeaderType::GENERAL); assert_eq!(header.device_id(), 0x1533); assert_eq!(header.vendor_id(), 0x8086); @@ -340,38 +377,16 @@ mod test { #[test] fn test_parse_nonexistent() { - let bytes = [ - 0xff, 0xff, 0xff, 0xff, - 0xff, 0xff, 0xff, 0xff - ]; - assert_eq!(PciHeader::from_reader(&bytes[..]), Err(PciHeaderError::NoDevice)); + let bytes = &[0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff]; + assert_eq!( + PciHeader::from_reader( + &TestCfgAccess { + addr: PciAddress::new(0, 2, 4, 0), + bytes, + }, + PciAddress::new(0, 2, 4, 0), + ), + Err(PciHeaderError::NoDevice) + ); } - - #[test] - fn test_read_range() { - let res = unsafe { (&IGB_DEV_BYTES[..]).read_range(0, 4) }; - assert_eq!(res, &[0x86, 0x80, 0x33, 0x15][..]); - - let res = unsafe { (&IGB_DEV_BYTES[..]).read_range(16, 32) }; - let expected = [ - 0x00, 0x00, 0x50, 0xf7, 0x00, 0x00, 0x00, 0x00, - 0x01, 0xb0, 0x00, 0x00, 0x00, 0x00, 0x58, 0xf7, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0xd9, 0x15, 0x33, 0x15 - ]; - assert_eq!(res, expected); - } - - macro_rules! read_range_should_panic { - ($name:ident, $len:expr) => { - #[test] - #[should_panic(expected = "invalid range length")] - fn $name() { - let _ = unsafe { (&IGB_DEV_BYTES[..]).read_range(0, $len) }; - } - } - } - - read_range_should_panic!(test_short_len, 2); - read_range_should_panic!(test_not_mod_4_len, 7); } From 0b611dca044b0d41bbaf691f4cdd532e77fe1ddd Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 22 Jan 2024 11:25:43 +0100 Subject: [PATCH 4/9] Start using the pci_types crate --- Cargo.lock | 23 +++++++--- pcid/Cargo.toml | 1 + pcid/src/cfg_access/fallback.rs | 15 +++++-- pcid/src/cfg_access/mod.rs | 8 +++- pcid/src/driver_interface/mod.rs | 20 +++++++++ pcid/src/main.rs | 7 ++-- pcid/src/pci/func.rs | 5 +-- pcid/src/pci/mod.rs | 72 +------------------------------- pcid/src/pci_header.rs | 16 +++++-- 9 files changed, 74 insertions(+), 93 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 5364d127f1..47e84816d9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -196,9 +196,9 @@ checksum = "bef38d45163c2f1dde094a7dfd33ccf595c92905c8f8f4fdc18d06fb1037718a" [[package]] name = "bitflags" -version = "2.4.0" +version = "2.4.2" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b4682ae6287fcf752ecaabbfcc7b6f9b72aa33933dc23a554d853aea8eea8635" +checksum = "ed570934406eb16438a4e976b1b4500774099c13b8cb96eec99f620f05090ddf" [[package]] name = "bitvec" @@ -527,7 +527,7 @@ version = "3.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "8283e7331b8c93b9756e0cfdbcfb90312852f953c6faf9bf741e684cc3b6ad69" dependencies = [ - "bitflags 2.4.0", + "bitflags 2.4.2", "crc", "log", "uuid", @@ -698,7 +698,7 @@ version = "0.0.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "85c833ca1e66078851dba29046874e38f08b2c883700aa29a03ddd3b23814ee8" dependencies = [ - "bitflags 2.4.0", + "bitflags 2.4.2", "libc", "redox_syscall 0.4.1", ] @@ -956,6 +956,16 @@ dependencies = [ "winapi", ] +[[package]] +name = "pci_types" +version = "0.6.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "831e5bebf010674bc2e8070b892948120d4c453c71f37387e1ffea5636620dbe" +dependencies = [ + "bit_field", + "bitflags 2.4.2", +] + [[package]] name = "pcid" version = "0.1.0" @@ -968,6 +978,7 @@ dependencies = [ "libc", "log", "paw", + "pci_types", "plain", "redox-log", "redox_syscall 0.4.1", @@ -1151,7 +1162,7 @@ version = "0.2.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "475e252d7add4825405d2248d530d33e22364ac5477eab816b56efbeec1e2712" dependencies = [ - "bitflags 2.4.0", + "bitflags 2.4.2", "libredox", "redox_syscall 0.4.1", ] @@ -1727,7 +1738,7 @@ dependencies = [ name = "virtio-core" version = "0.1.0" dependencies = [ - "bitflags 2.4.0", + "bitflags 2.4.2", "common", "crossbeam-queue", "futures", diff --git a/pcid/Cargo.toml b/pcid/Cargo.toml index 6c276cc52b..794002fb18 100644 --- a/pcid/Cargo.toml +++ b/pcid/Cargo.toml @@ -19,6 +19,7 @@ byteorder = "1.2" libc = "0.2" log = "0.4" paw = "1.0" +pci_types = "0.6.1" plain = "0.2" redox-log = "0.1" redox_syscall = "0.4" diff --git a/pcid/src/cfg_access/fallback.rs b/pcid/src/cfg_access/fallback.rs index 90023961f7..65682e171f 100644 --- a/pcid/src/cfg_access/fallback.rs +++ b/pcid/src/cfg_access/fallback.rs @@ -6,8 +6,7 @@ use std::sync::Mutex; use syscall::io::{Io as _, Pio}; use log::info; - -use crate::pci::{CfgAccess, PciAddress}; +use pci_types::{ConfigRegionAccess, PciAddress}; pub(crate) struct Pci { lock: Mutex<()>, @@ -58,7 +57,11 @@ impl Pci { } } #[cfg(any(target_arch = "x86", target_arch = "x86_64"))] -impl CfgAccess for Pci { +impl ConfigRegionAccess for Pci { + fn function_exists(&self, _address: PciAddress) -> bool { + todo!(); + } + unsafe fn read(&self, address: PciAddress, offset: u16) -> u32 { let _guard = self.lock.lock().unwrap(); @@ -86,7 +89,11 @@ impl CfgAccess for Pci { } } #[cfg(not(any(target_arch = "x86", target_arch = "x86_64")))] -impl CfgAccess for Pci { +impl ConfigRegionAccess for Pci { + fn function_exists(&self, _address: PciAddress) -> bool { + todo!(); + } + unsafe fn read(&self, addr: PciAddress, offset: u16) -> u32 { let _guard = self.lock.lock().unwrap(); todo!("Pci::CfgAccess::read on this architecture") diff --git a/pcid/src/cfg_access/mod.rs b/pcid/src/cfg_access/mod.rs index 0925b21d03..fd7f6e706a 100644 --- a/pcid/src/cfg_access/mod.rs +++ b/pcid/src/cfg_access/mod.rs @@ -2,8 +2,8 @@ use std::sync::Mutex; use std::{fmt, fs, io, mem, ptr, slice}; use log::info; +use pci_types::{ConfigRegionAccess, PciAddress}; -use crate::pci::{CfgAccess, PciAddress}; use fallback::Pci; mod fallback; @@ -221,7 +221,11 @@ impl Pcie { } } -impl CfgAccess for Pcie { +impl ConfigRegionAccess for Pcie { + fn function_exists(&self, _address: PciAddress) -> bool { + todo!(); + } + unsafe fn read(&self, address: PciAddress, offset: u16) -> u32 { let _guard = self.lock.lock().unwrap(); diff --git a/pcid/src/driver_interface/mod.rs b/pcid/src/driver_interface/mod.rs index 4f7fd64703..4c01b88282 100644 --- a/pcid/src/driver_interface/mod.rs +++ b/pcid/src/driver_interface/mod.rs @@ -26,9 +26,29 @@ pub enum LegacyInterruptPin { IntD = 4, } +#[derive(Serialize, Deserialize)] +#[serde(remote = "PciAddress")] +struct PciAddressDef { + #[serde(getter = "PciAddress::segment")] + segment: u16, + #[serde(getter = "PciAddress::bus")] + bus: u8, + #[serde(getter = "PciAddress::device")] + device: u8, + #[serde(getter = "PciAddress::function")] + function: u8, +} + +impl From for PciAddress { + fn from(value: PciAddressDef) -> Self { + PciAddress::new(value.segment, value.bus, value.device, value.function) + } +} + #[derive(Clone, Copy, Debug, Serialize, Deserialize)] pub struct PciFunction { /// Address of the PCI function. + #[serde(with = "PciAddressDef")] pub addr: PciAddress, /// PCI Base Address Registers diff --git a/pcid/src/main.rs b/pcid/src/main.rs index 793505276b..bd61bdf4c9 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -5,13 +5,14 @@ use std::process::Command; use std::thread; use std::sync::{Arc, Mutex}; +use pci_types::{ConfigRegionAccess, PciAddress}; use structopt::StructOpt; use log::{debug, error, info, warn, trace}; use redox_log::{OutputBuilder, RedoxLogger}; use crate::cfg_access::Pcie; use crate::config::Config; -use crate::pci::{CfgAccess, PciAddress, PciBar, PciClass, PciFunc}; +use crate::pci::{PciBar, PciClass, PciFunc}; use crate::pci::cap::Capability as PciCapability; use crate::pci::func::{ConfigReader, ConfigWriter}; use crate::pci_header::{PciHeader, PciHeaderError, PciHeaderType}; @@ -40,7 +41,7 @@ pub struct DriverHandler { state: Arc, } -fn with_pci_func_raw T>(pci: &dyn CfgAccess, addr: PciAddress, function: F) -> T { +fn with_pci_func_raw T>(pci: &dyn ConfigRegionAccess, addr: PciAddress, function: F) -> T { let func = PciFunc { pci, addr, @@ -204,7 +205,7 @@ pub struct State { pcie: Pcie, } impl State { - fn preferred_cfg_access(&self) -> &dyn CfgAccess { + fn preferred_cfg_access(&self) -> &dyn ConfigRegionAccess { &self.pcie } } diff --git a/pcid/src/pci/func.rs b/pcid/src/pci/func.rs index d6c7e93b70..7bdabff43d 100644 --- a/pcid/src/pci/func.rs +++ b/pcid/src/pci/func.rs @@ -1,6 +1,5 @@ use byteorder::{ByteOrder, LittleEndian}; - -use super::{CfgAccess, PciAddress}; +use pci_types::{ConfigRegionAccess, PciAddress}; pub trait ConfigReader { unsafe fn read_range(&self, offset: u16, len: u16) -> Vec { @@ -33,7 +32,7 @@ pub trait ConfigWriter { } pub struct PciFunc<'pci> { - pub pci: &'pci dyn CfgAccess, + pub pci: &'pci dyn ConfigRegionAccess, pub addr: PciAddress, } diff --git a/pcid/src/pci/mod.rs b/pcid/src/pci/mod.rs index 8f5e661ab7..f0eb8b558a 100644 --- a/pcid/src/pci/mod.rs +++ b/pcid/src/pci/mod.rs @@ -1,12 +1,8 @@ -use std::fmt; - -use bit_field::BitField; -use serde::{Deserialize, Serialize}; - pub use self::bar::PciBar; pub use self::class::PciClass; pub use self::func::PciFunc; pub use self::id::FullDeviceId; +pub use pci_types::PciAddress; mod bar; pub mod cap; @@ -14,69 +10,3 @@ mod class; pub mod func; mod id; pub mod msi; - -pub trait CfgAccess { - unsafe fn read(&self, addr: PciAddress, offset: u16) -> u32; - unsafe fn write(&self, addr: PciAddress, offset: u16, value: u32); -} - -// Copied from the pci_types crate, version 0.6.1. It has been modified to add serde support. -// FIXME If we start using it in the future use the upstream version instead. -/// The address of a PCIe function. -/// -/// PCIe supports 65536 segments, each with 256 buses, each with 32 slots, each with 8 possible functions. We pack this into a `u32`: -/// -/// ```ignore -/// 32 16 8 3 0 -/// +-------------------------------+---------------+---------+------+ -/// | segment | bus | device | func | -/// +-------------------------------+---------------+---------+------+ -/// ``` -#[derive(Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Default, Serialize, Deserialize)] -pub struct PciAddress(u32); - -impl PciAddress { - pub fn new(segment: u16, bus: u8, device: u8, function: u8) -> PciAddress { - let mut result = 0; - result.set_bits(0..3, function as u32); - result.set_bits(3..8, device as u32); - result.set_bits(8..16, bus as u32); - result.set_bits(16..32, segment as u32); - PciAddress(result) - } - - pub fn segment(&self) -> u16 { - self.0.get_bits(16..32) as u16 - } - - pub fn bus(&self) -> u8 { - self.0.get_bits(8..16) as u8 - } - - pub fn device(&self) -> u8 { - self.0.get_bits(3..8) as u8 - } - - pub fn function(&self) -> u8 { - self.0.get_bits(0..3) as u8 - } -} - -impl fmt::Display for PciAddress { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!( - f, - "{:02x}-{:02x}:{:02x}.{}", - self.segment(), - self.bus(), - self.device(), - self.function() - ) - } -} - -impl fmt::Debug for PciAddress { - fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", self) - } -} diff --git a/pcid/src/pci_header.rs b/pcid/src/pci_header.rs index f904e1e930..183b5254a4 100644 --- a/pcid/src/pci_header.rs +++ b/pcid/src/pci_header.rs @@ -1,8 +1,9 @@ use bitflags::bitflags; use byteorder::{ByteOrder, LittleEndian}; +use pci_types::{ConfigRegionAccess, PciAddress}; use serde::{Deserialize, Serialize}; -use crate::pci::{CfgAccess, FullDeviceId, PciAddress, PciBar, PciClass}; +use crate::pci::{FullDeviceId, PciBar, PciClass}; #[derive(Debug, PartialEq)] pub enum PciHeaderError { @@ -89,7 +90,7 @@ impl PciHeader { /// Parse the bytes found in the Configuration Space of the PCI device into /// a more usable PciHeader. pub fn from_reader( - cfg_access: &dyn CfgAccess, + cfg_access: &dyn ConfigRegionAccess, addr: PciAddress, ) -> Result { if unsafe { cfg_access.read(addr, 0) } != 0xffffffff { @@ -308,15 +309,21 @@ impl PciHeader { mod test { use std::convert::TryInto; + use pci_types::{ConfigRegionAccess, PciAddress}; + use super::{PciHeader, PciHeaderError, PciHeaderType}; - use crate::pci::{CfgAccess, PciAddress, PciBar, PciClass}; + use crate::pci::{PciBar, PciClass}; struct TestCfgAccess<'a> { addr: PciAddress, bytes: &'a [u8], } - impl CfgAccess for TestCfgAccess<'_> { + impl ConfigRegionAccess for TestCfgAccess<'_> { + fn function_exists(&self, _address: PciAddress) -> bool { + unreachable!(); + } + unsafe fn read(&self, addr: PciAddress, offset: u16) -> u32 { assert_eq!(addr, self.addr); let offset = offset as usize; @@ -329,6 +336,7 @@ mod test { } } + #[rustfmt::skip] const IGB_DEV_BYTES: [u8; 256] = [ 0x86, 0x80, 0x33, 0x15, 0x07, 0x04, 0x10, 0x00, 0x03, 0x00, 0x00, 0x02, 0x10, 0x00, 0x00, 0x00, 0x00, 0x00, 0x50, 0xf7, 0x00, 0x00, 0x00, 0x00, 0x01, 0xb0, 0x00, 0x00, 0x00, 0x00, 0x58, 0xf7, From d1b6009e3d86af3ad63269c1abb7c602b24d9831 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 22 Jan 2024 15:25:08 +0100 Subject: [PATCH 5/9] Start converting PciHeader::from_reader to pci_types --- pcid/src/main.rs | 41 ++++------ pcid/src/pci_header.rs | 166 ++++++++++++++++++++--------------------- 2 files changed, 95 insertions(+), 112 deletions(-) diff --git a/pcid/src/main.rs b/pcid/src/main.rs index bd61bdf4c9..6250e8ec8c 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -74,7 +74,7 @@ impl DriverHandler { None => return PcidClientResponse::Error(PcidServerResponseError::NonexistentFeature(feature)), }; unsafe { - with_pci_func_raw(self.state.preferred_cfg_access(), self.addr, |func| { + with_pci_func_raw(&self.state.pcie, self.addr, |func| { capability.set_enabled(true); capability.write_message_control(func, offset); }); @@ -87,7 +87,7 @@ impl DriverHandler { None => return PcidClientResponse::Error(PcidServerResponseError::NonexistentFeature(feature)), }; unsafe { - with_pci_func_raw(self.state.preferred_cfg_access(), self.addr, |func| { + with_pci_func_raw(&self.state.pcie, self.addr, |func| { capability.set_msix_enabled(true); capability.write_a(func, offset); }); @@ -147,7 +147,7 @@ impl DriverHandler { info.set_mask_bits(mask_bits); } unsafe { - with_pci_func_raw(self.state.preferred_cfg_access(), self.addr, |func| { + with_pci_func_raw(&self.state.pcie, self.addr, |func| { info.write_all(func, offset); }); } @@ -159,7 +159,7 @@ impl DriverHandler { if let Some(mask) = function_mask { info.set_function_mask(mask); unsafe { - with_pci_func_raw(self.state.preferred_cfg_access(), self.addr, |func| { + with_pci_func_raw(&self.state.pcie, self.addr, |func| { info.write_a(func, offset); }); } @@ -171,7 +171,7 @@ impl DriverHandler { } PcidClientRequest::ReadConfig(offset) => { let value = unsafe { - with_pci_func_raw(self.state.preferred_cfg_access(), self.addr, |func| { + with_pci_func_raw(&self.state.pcie, self.addr, |func| { func.read_u32(offset) }) }; @@ -179,7 +179,7 @@ impl DriverHandler { }, PcidClientRequest::WriteConfig(offset, value) => { unsafe { - with_pci_func_raw(self.state.preferred_cfg_access(), self.addr, |func| { + with_pci_func_raw(&self.state.pcie, self.addr, |func| { func.write_u32(offset, value); }); } @@ -204,15 +204,8 @@ pub struct State { threads: Mutex>>, pcie: Pcie, } -impl State { - fn preferred_cfg_access(&self) -> &dyn ConfigRegionAccess { - &self.pcie - } -} fn handle_parsed_header(state: Arc, config: &Config, addr: PciAddress, header: PciHeader) { - let pci = state.preferred_cfg_access(); - let raw_class: u8 = header.class().into(); let mut string = format!("PCI {} {:>04X}:{:>04X} {:>02X}.{:>02X}.{:>02X}.{:>02X} {:?}", addr, header.vendor_id(), header.device_id(), raw_class, @@ -273,9 +266,9 @@ fn handle_parsed_header(state: Arc, config: &Config, addr: PciAddress, he // Enable bus mastering, memory space, and I/O space unsafe { - let mut data = pci.read(addr, 0x04); + let mut data = state.pcie.read(addr, 0x04); data |= 7; - pci.write(addr, 0x04, data); + state.pcie.write(addr, 0x04, data); } // Set IRQ line to 9 if not set @@ -283,14 +276,14 @@ fn handle_parsed_header(state: Arc, config: &Config, addr: PciAddress, he let interrupt_pin; unsafe { - let mut data = pci.read(addr, 0x3C); + let mut data = state.pcie.read(addr, 0x3C); irq = (data & 0xFF) as u8; interrupt_pin = ((data & 0x0000_FF00) >> 8) as u8; if irq == 0xFF { irq = 9; } data = (data & 0xFFFFFF00) | irq as u32; - pci.write(addr, 0x3C, data); + state.pcie.write(addr, 0x3C, data); }; // Find BAR sizes @@ -309,11 +302,11 @@ fn handle_parsed_header(state: Arc, config: &Config, addr: PciAddress, he let offset = 0x10 + (i as u8) * 4; - let original = pci.read(addr, offset.into()); - pci.write(addr, offset.into(), 0xFFFFFFFF); + let original = state.pcie.read(addr, offset.into()); + state.pcie.write(addr, offset.into(), 0xFFFFFFFF); - let new = pci.read(addr, offset.into()); - pci.write(addr, offset.into(), original); + let new = state.pcie.read(addr, offset.into()); + state.pcie.write(addr, offset.into(), original); let masked = if new & 1 == 1 { new & 0xFFFFFFFC @@ -332,7 +325,7 @@ fn handle_parsed_header(state: Arc, config: &Config, addr: PciAddress, he let capabilities = if header.status() & (1 << 4) != 0 { let func = PciFunc { - pci: state.preferred_cfg_access(), + pci: &state.pcie, addr }; crate::pci::cap::CapabilitiesIter { inner: crate::pci::cap::CapabilityOffsetsIter::new(header.cap_pointer(), &func) }.collect::>() @@ -509,8 +502,6 @@ fn main(args: Args) { threads: Mutex::new(Vec::new()), }); - let pci = state.preferred_cfg_access(); - info!("PCI SG-BS:DV.F VEND:DEVI CL.SC.IN.RV"); // FIXME Use full ACPI for enumerating the host bridges. MCFG only describes the first @@ -525,7 +516,7 @@ fn main(args: Args) { 'dev: for dev_num in 0..32 { for func_num in 0..8 { let func_addr = PciAddress::new(0, bus_num, dev_num, func_num); - match PciHeader::from_reader(pci, func_addr) { + match PciHeader::from_reader(&state.pcie, func_addr) { Ok(header) => { handle_parsed_header(Arc::clone(&state), &config, func_addr, header); if let PciHeader::PciToPci { secondary_bus_num, .. } = header { diff --git a/pcid/src/pci_header.rs b/pcid/src/pci_header.rs index 183b5254a4..94fac26dc6 100644 --- a/pcid/src/pci_header.rs +++ b/pcid/src/pci_header.rs @@ -1,6 +1,6 @@ use bitflags::bitflags; use byteorder::{ByteOrder, LittleEndian}; -use pci_types::{ConfigRegionAccess, PciAddress}; +use pci_types::{ConfigRegionAccess, PciAddress, PciHeader as TyPciHeader}; use serde::{Deserialize, Serialize}; use crate::pci::{FullDeviceId, PciBar, PciClass}; @@ -90,96 +90,88 @@ impl PciHeader { /// Parse the bytes found in the Configuration Space of the PCI device into /// a more usable PciHeader. pub fn from_reader( - cfg_access: &dyn ConfigRegionAccess, + access: &impl ConfigRegionAccess, addr: PciAddress, ) -> Result { - if unsafe { cfg_access.read(addr, 0) } != 0xffffffff { - // Read the initial 16 bytes and set variables used by all header types. - let bytes = unsafe { - let mut ret = Vec::with_capacity(16); - for offset in (0..16).step_by(4) { - ret.extend(cfg_access.read(addr, offset).to_le_bytes()); - } - ret - }; - let vendor_id = LittleEndian::read_u16(&bytes[0..2]); - let device_id = LittleEndian::read_u16(&bytes[2..4]); - let command = LittleEndian::read_u16(&bytes[4..6]); - let status = LittleEndian::read_u16(&bytes[6..8]); - let revision = bytes[8]; - let interface = bytes[9]; - let subclass = bytes[10]; - let class = bytes[11]; - let header_type = PciHeaderType::from_bits_truncate(bytes[14]); - let shared = SharedPciHeader { - full_device_id: FullDeviceId { - vendor_id, - device_id, - class, - subclass, - interface, - revision, - }, - command, - status, - header_type, - }; + if unsafe { access.read(addr, 0) } == 0xffffffff { + return Err(PciHeaderError::NoDevice); + } - match header_type & PciHeaderType::HEADER_TYPE { - PciHeaderType::GENERAL => { - let bytes = unsafe { - let mut ret = Vec::with_capacity(48); - for offset in (16..64).step_by(4) { - ret.extend(cfg_access.read(addr, offset).to_le_bytes()); - } - ret - }; - let mut bars = [PciBar::None; 6]; - Self::get_bars(&bytes, &mut bars); - let subsystem_vendor_id = LittleEndian::read_u16(&bytes[28..30]); - let subsystem_id = LittleEndian::read_u16(&bytes[30..32]); - let cap_pointer = bytes[36]; - let interrupt_line = bytes[44]; - let interrupt_pin = bytes[45]; - Ok(PciHeader::General { - shared, - bars, - subsystem_vendor_id, - subsystem_id, - cap_pointer, - interrupt_line, - interrupt_pin, - }) - } - PciHeaderType::PCITOPCI => { - let bytes = unsafe { - let mut ret = Vec::with_capacity(48); - for offset in (16..64).step_by(4) { - ret.extend(cfg_access.read(addr, offset).to_le_bytes()); - } - ret - }; - let mut bars = [PciBar::None; 2]; - Self::get_bars(&bytes, &mut bars); - let secondary_bus_num = bytes[9]; - let cap_pointer = bytes[36]; - let interrupt_line = bytes[44]; - let interrupt_pin = bytes[45]; - let bridge_control = LittleEndian::read_u16(&bytes[46..48]); - Ok(PciHeader::PciToPci { - shared, - bars, - secondary_bus_num, - cap_pointer, - interrupt_line, - interrupt_pin, - bridge_control, - }) - } - id => Err(PciHeaderError::UnknownHeaderType(id.bits())), + let header = TyPciHeader::new(addr); + let (vendor_id, device_id) = header.id(access); + let command_and_status = unsafe { access.read(addr, 4) }; + let command = (command_and_status & 0xffff) as u16; + let status = (command_and_status >> 16) as u16; + let (revision, class, subclass, interface) = header.revision_and_class(access); + let header_type = PciHeaderType::from_bits_truncate( + ((unsafe { access.read(addr, 12) } >> 24) & 0xff) as u8, + ); + let shared = SharedPciHeader { + full_device_id: FullDeviceId { + vendor_id, + device_id, + class, + subclass, + interface, + revision, + }, + command, + status, + header_type, + }; + + match header_type & PciHeaderType::HEADER_TYPE { + PciHeaderType::GENERAL => { + let bytes = unsafe { + let mut ret = Vec::with_capacity(48); + for offset in (16..64).step_by(4) { + ret.extend(access.read(addr, offset).to_le_bytes()); + } + ret + }; + let mut bars = [PciBar::None; 6]; + Self::get_bars(&bytes, &mut bars); + let subsystem_vendor_id = LittleEndian::read_u16(&bytes[28..30]); + let subsystem_id = LittleEndian::read_u16(&bytes[30..32]); + let cap_pointer = bytes[36]; + let interrupt_line = bytes[44]; + let interrupt_pin = bytes[45]; + Ok(PciHeader::General { + shared, + bars, + subsystem_vendor_id, + subsystem_id, + cap_pointer, + interrupt_line, + interrupt_pin, + }) } - } else { - Err(PciHeaderError::NoDevice) + PciHeaderType::PCITOPCI => { + let bytes = unsafe { + let mut ret = Vec::with_capacity(48); + for offset in (16..64).step_by(4) { + ret.extend(access.read(addr, offset).to_le_bytes()); + } + ret + }; + let mut bars = [PciBar::None; 2]; + Self::get_bars(&bytes, &mut bars); + let secondary_bus_num = bytes[9]; + let cap_pointer = bytes[36]; + let interrupt_line = bytes[44]; + let interrupt_pin = bytes[45]; + let bridge_control = LittleEndian::read_u16(&bytes[46..48]); + Ok(PciHeader::PciToPci { + shared, + bars, + secondary_bus_num, + cap_pointer, + interrupt_line, + interrupt_pin, + bridge_control, + }) + } + id => Err(PciHeaderError::UnknownHeaderType(id.bits())), } } From a5d0c7c3544df7f5c206d13f8756e9f5a9d06e35 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 22 Jan 2024 15:54:18 +0100 Subject: [PATCH 6/9] Use pci_types for reading BARs This simplifies a lot of code and adds support for 64bit BARs. --- ahcid/src/main.rs | 7 +- e1000d/src/main.rs | 5 +- ihdad/src/main.rs | 5 +- ixgbed/src/main.rs | 4 +- nvmed/src/main.rs | 18 ++--- pcid/src/driver_interface/mod.rs | 4 +- pcid/src/main.rs | 45 +---------- pcid/src/pci/bar.rs | 38 +++------- pcid/src/pci/msi.rs | 4 +- pcid/src/pci_header.rs | 124 ++++++++++++++----------------- rtl8139d/src/main.rs | 35 ++++----- rtl8168d/src/main.rs | 37 ++++----- vboxd/src/main.rs | 2 +- virtio-core/src/arch/x86_64.rs | 9 +-- virtio-core/src/probe.rs | 2 +- xhcid/src/main.rs | 12 ++- 16 files changed, 129 insertions(+), 222 deletions(-) diff --git a/ahcid/src/main.rs b/ahcid/src/main.rs index bd8e6af790..3d661c321f 100644 --- a/ahcid/src/main.rs +++ b/ahcid/src/main.rs @@ -9,7 +9,7 @@ use std::io::{ErrorKind, Read, Write}; use std::os::unix::io::{FromRawFd, RawFd}; use std::usize; -use pcid_interface::{PciBar, PcidServerHandle}; +use pcid_interface::PcidServerHandle; use syscall::error::{Error, ENODEV}; use syscall::data::{Event, Packet}; use syscall::flag::EVENT_READ; @@ -81,8 +81,7 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { let mut name = pci_config.func.name(); name.push_str("_ahci"); - let bar = pci_config.func.bars[5].expect_mem(); - let bar_size = pci_config.func.bar_sizes[5]; + let (bar, bar_size) = pci_config.func.bars[5].expect_mem(); let irq = pci_config.func.legacy_interrupt_line; @@ -93,7 +92,7 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { let address = unsafe { common::physmap( bar, - bar_size as usize, + bar_size, common::Prot { read: true, write: true }, common::MemoryType::Uncacheable, ).expect("ahcid: failed to map address") diff --git a/e1000d/src/main.rs b/e1000d/src/main.rs index 58a8a9f4ed..dffeb43f4d 100644 --- a/e1000d/src/main.rs +++ b/e1000d/src/main.rs @@ -10,7 +10,7 @@ use std::process; use std::sync::Arc; use event::EventQueue; -use pcid_interface::{PciBar, PcidServerHandle}; +use pcid_interface::PcidServerHandle; use syscall::{EventFlags, Packet, SchemeBlockMut}; pub mod device; @@ -68,8 +68,7 @@ fn main() { let mut name = pci_config.func.name(); name.push_str("_e1000"); - let bar = pci_config.func.bars[0].expect_mem(); - let bar_size = pci_config.func.bar_sizes[0] as usize; + let (bar, bar_size) = pci_config.func.bars[0].expect_mem(); let irq = pci_config.func.legacy_interrupt_line; diff --git a/ihdad/src/main.rs b/ihdad/src/main.rs index a4ff9a98f0..860446ad5c 100755 --- a/ihdad/src/main.rs +++ b/ihdad/src/main.rs @@ -160,13 +160,12 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { let mut name = pci_config.func.name(); name.push_str("_ihda"); - let bar_ptr = pci_config.func.bars[0].expect_mem(); - let bar_size = pci_config.func.bar_sizes[0]; + let (bar_ptr, bar_size) = pci_config.func.bars[0].expect_mem(); log::info!(" + IHDA {} on: {:#X} size: {}", name, bar_ptr, bar_size); let address = unsafe { - common::physmap(bar_ptr, bar_size as usize, common::Prot::RW, common::MemoryType::Uncacheable) + common::physmap(bar_ptr, bar_size, 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 a0b3d46c39..4666b98142 100644 --- a/ixgbed/src/main.rs +++ b/ixgbed/src/main.rs @@ -13,7 +13,7 @@ use std::sync::Arc; use std::thread; use event::EventQueue; -use pcid_interface::{PciBar, PcidServerHandle}; +use pcid_interface::PcidServerHandle; use std::time::Duration; use syscall::{EventFlags, Packet, SchemeBlockMut}; @@ -76,7 +76,7 @@ fn main() { let mut name = pci_config.func.name(); name.push_str("_ixgbe"); - let bar = pci_config.func.bars[0].expect_mem(); + 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 07933e1659..09ced15aff 100644 --- a/nvmed/src/main.rs +++ b/nvmed/src/main.rs @@ -9,7 +9,7 @@ use std::ptr::NonNull; use std::sync::{Arc, Mutex}; use std::{slice, usize}; -use pcid_interface::{PciBar, PciFeature, PciFeatureInfo, PciFunction, PcidServerHandle}; +use pcid_interface::{PciFeature, PciFeatureInfo, PciFunction, PcidServerHandle}; use syscall::{ Event, Mmio, Packet, Result, SchemeBlockMut, PAGE_SIZE, @@ -93,10 +93,9 @@ fn get_int_method( match &mut *bar_guard { &mut Some(ref bar) => Ok(bar.ptr), bar_to_set @ &mut None => { - let bar = function.bars[bir].expect_mem(); - let bar_size = function.bar_sizes[bir]; + let (bar, bar_size) = function.bars[bir].expect_mem(); - let bar = Bar::allocate(bar as usize, bar_size as usize)?; + let bar = Bar::allocate(bar, bar_size)?; *bar_to_set = Some(bar); Ok(bar_to_set.as_ref().unwrap().ptr) } @@ -293,8 +292,7 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { let _logger_ref = setup_logging(&scheme_name); - let bar = pci_config.func.bars[0].expect_mem(); - let bar_size = pci_config.func.bar_sizes[0]; + let (bar, bar_size) = pci_config.func.bars[0].expect_mem(); let irq = pci_config.func.legacy_interrupt_line; log::debug!("NVME PCI CONFIG: {:?}", pci_config); @@ -303,16 +301,16 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { let address = unsafe { common::physmap( - bar as usize, - bar_size as usize, + bar, + bar_size, common::Prot { read: true, write: true }, common::MemoryType::Uncacheable, ) .expect("nvmed: failed to map address") } as usize; *allocated_bars.0[0].lock().unwrap() = Some(Bar { - physical: bar as usize, - bar_size: bar_size as usize, + physical: bar, + bar_size, ptr: NonNull::new(address as *mut u8).expect("Physmapping BAR gave nullptr"), }); diff --git a/pcid/src/driver_interface/mod.rs b/pcid/src/driver_interface/mod.rs index 4c01b88282..36c002a15f 100644 --- a/pcid/src/driver_interface/mod.rs +++ b/pcid/src/driver_interface/mod.rs @@ -54,9 +54,6 @@ pub struct PciFunction { /// PCI Base Address Registers pub bars: [PciBar; 6], - /// BAR sizes - pub bar_sizes: [u32; 6], - /// Legacy IRQ line: It's the responsibility of pcid to make sure that it be mapped in either /// the I/O APIC or the 8259 PIC, so that the subdriver can map the interrupt vector directly. /// The vector to map is always this field, plus 32. @@ -282,6 +279,7 @@ impl PcidServerHandle { } } + // FIXME turn into struct with bool fields pub fn fetch_all_features(&mut self) -> Result> { self.send(&PcidClientRequest::RequestFeatures)?; match self.recv()? { diff --git a/pcid/src/main.rs b/pcid/src/main.rs index 6250e8ec8c..de3203345d 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -244,11 +244,12 @@ fn handle_parsed_header(state: Arc, config: &Config, addr: PciAddress, he _ => () } - for (i, bar) in header.bars().iter().enumerate() { + let bars = header.bars(&state.pcie); + for (i, bar) in bars.iter().enumerate() { match bar { PciBar::None => {}, - PciBar::Memory32(addr) => string.push_str(&format!(" {i}={addr:08X}")), - PciBar::Memory64(addr) => string.push_str(&format!(" {i}={addr:016X}")), + PciBar::Memory32{addr,..} => string.push_str(&format!(" {i}={addr:08X}")), + PciBar::Memory64{addr,..} => string.push_str(&format!(" {i}={addr:016X}")), PciBar::Port(port) => string.push_str(&format!(" {i}=P{port:04X}")), } } @@ -286,43 +287,6 @@ fn handle_parsed_header(state: Arc, config: &Config, addr: PciAddress, he state.pcie.write(addr, 0x3C, data); }; - // Find BAR sizes - //TODO: support 64-bit BAR sizes? - let mut bars = [PciBar::None; 6]; - let mut bar_sizes = [0; 6]; - unsafe { - let count = match header.header_type() { - PciHeaderType::GENERAL => 6, - PciHeaderType::PCITOPCI => 2, - _ => 0, - }; - - for i in 0..count { - bars[i] = header.get_bar(i); - - let offset = 0x10 + (i as u8) * 4; - - let original = state.pcie.read(addr, offset.into()); - state.pcie.write(addr, offset.into(), 0xFFFFFFFF); - - let new = state.pcie.read(addr, offset.into()); - state.pcie.write(addr, offset.into(), original); - - let masked = if new & 1 == 1 { - new & 0xFFFFFFFC - } else { - new & 0xFFFFFFF0 - }; - - let size = (!masked).wrapping_add(1); - bar_sizes[i] = if size <= 1 { - 0 - } else { - size - }; - } - } - let capabilities = if header.status() & (1 << 4) != 0 { let func = PciFunc { pci: &state.pcie, @@ -351,7 +315,6 @@ fn handle_parsed_header(state: Arc, config: &Config, addr: PciAddress, he let func = driver_interface::PciFunction { bars, - bar_sizes, addr, legacy_interrupt_line: irq, legacy_interrupt_pin, diff --git a/pcid/src/pci/bar.rs b/pcid/src/pci/bar.rs index f0d3a14911..4ee887d91f 100644 --- a/pcid/src/pci/bar.rs +++ b/pcid/src/pci/bar.rs @@ -5,8 +5,8 @@ use serde::{Deserialize, Serialize}; #[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] pub enum PciBar { None, - Memory32(u32), - Memory64(u64), + Memory32 { addr: u32, size: u32 }, + Memory64 { addr: u64, size: u64 }, Port(u16), } @@ -21,40 +21,24 @@ impl PciBar { pub fn expect_port(&self) -> u16 { match *self { PciBar::Port(port) => port, - PciBar::Memory32(_) | PciBar::Memory64(_) => { + PciBar::Memory32 { .. } | PciBar::Memory64 { .. } => { panic!("expected port BAR, found memory BAR"); } PciBar::None => panic!("expected BAR to exist"), } } - pub fn expect_mem(&self) -> usize { + pub fn expect_mem(&self) -> (usize, usize) { match *self { - PciBar::Memory32(ptr) => ptr as usize, - PciBar::Memory64(ptr) => ptr - .try_into() - .expect("conversion from 64bit BAR to usize failed"), + PciBar::Memory32 { addr, size } => (addr as usize, size as usize), + PciBar::Memory64 { addr, size } => ( + addr.try_into() + .expect("conversion from 64bit BAR to usize failed"), + size.try_into() + .expect("conversion from 64bit BAR size to usize failed"), + ), PciBar::Port(_) => panic!("expected memory BAR, found port BAR"), PciBar::None => panic!("expected BAR to exist"), } } } - -impl From for PciBar { - fn from(bar: u32) -> Self { - if bar & 0xFFFFFFFC == 0 { - PciBar::None - } else if bar & 1 == 0 { - match (bar >> 1) & 3 { - 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 b4d3c67ca1..ba37e30127 100644 --- a/pcid/src/pci/msi.rs +++ b/pcid/src/pci/msi.rs @@ -285,7 +285,7 @@ impl MsixCapability { if self.table_bir() > 5 { panic!("MSI-X Table BIR contained a reserved enum value: {}", self.table_bir()); } - bars[usize::from(self.table_bir())].expect_mem() + self.table_offset() as usize + bars[usize::from(self.table_bir())].expect_mem().0 + 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 @@ -295,7 +295,7 @@ impl MsixCapability { if self.pba_bir() > 5 { panic!("MSI-X PBA BIR contained a reserved enum value: {}", self.pba_bir()); } - bars[usize::from(self.pba_bir())].expect_mem() + self.pba_offset() as usize + bars[usize::from(self.pba_bir())].expect_mem().0 + 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/pcid/src/pci_header.rs b/pcid/src/pci_header.rs index 94fac26dc6..b9918fdba7 100644 --- a/pcid/src/pci_header.rs +++ b/pcid/src/pci_header.rs @@ -1,6 +1,10 @@ +use std::convert::TryInto; + use bitflags::bitflags; use byteorder::{ByteOrder, LittleEndian}; -use pci_types::{ConfigRegionAccess, PciAddress, PciHeader as TyPciHeader}; +use pci_types::{ + Bar as TyBar, ConfigRegionAccess, EndpointHeader, PciAddress, PciHeader as TyPciHeader, +}; use serde::{Deserialize, Serialize}; use crate::pci::{FullDeviceId, PciBar, PciClass}; @@ -28,20 +32,20 @@ bitflags! { } } -#[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Copy, Debug, PartialEq)] pub struct SharedPciHeader { full_device_id: FullDeviceId, command: u16, status: u16, header_type: PciHeaderType, + addr: PciAddress, } // FIXME move out of pcid_interface -#[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Copy, Debug, PartialEq)] pub enum PciHeader { General { shared: SharedPciHeader, - bars: [PciBar; 6], subsystem_vendor_id: u16, subsystem_id: u16, cap_pointer: u8, @@ -50,7 +54,6 @@ pub enum PciHeader { }, PciToPci { shared: SharedPciHeader, - bars: [PciBar; 2], secondary_bus_num: u8, cap_pointer: u8, interrupt_line: u8, @@ -60,33 +63,6 @@ pub enum PciHeader { } impl PciHeader { - fn get_bars(bytes: &[u8], bars: &mut [PciBar]) { - let mut i = 0; - while i < bars.len() { - let offset = i * 4; - let bar_bytes = match bytes.get(offset..offset + 4) { - Some(some) => some, - None => continue, - }; - - match PciBar::from(LittleEndian::read_u32(bar_bytes)) { - PciBar::Memory64(mut addr) => { - let high_bytes = match bytes.get(offset + 4..offset + 8) { - Some(some) => some, - None => continue, - }; - addr |= (LittleEndian::read_u32(high_bytes) as u64) << 32; - bars[i] = PciBar::Memory64(addr); - i += 2; - } - bar => { - bars[i] = bar; - i += 1; - } - } - } - } - /// Parse the bytes found in the Configuration Space of the PCI device into /// a more usable PciHeader. pub fn from_reader( @@ -118,10 +94,12 @@ impl PciHeader { command, status, header_type, + addr, }; match header_type & PciHeaderType::HEADER_TYPE { PciHeaderType::GENERAL => { + let endpoint_header = EndpointHeader::from_header(header, access).unwrap(); let bytes = unsafe { let mut ret = Vec::with_capacity(48); for offset in (16..64).step_by(4) { @@ -129,16 +107,11 @@ impl PciHeader { } ret }; - let mut bars = [PciBar::None; 6]; - Self::get_bars(&bytes, &mut bars); - let subsystem_vendor_id = LittleEndian::read_u16(&bytes[28..30]); - let subsystem_id = LittleEndian::read_u16(&bytes[30..32]); + let (subsystem_id, subsystem_vendor_id) = endpoint_header.subsystem(access); let cap_pointer = bytes[36]; - let interrupt_line = bytes[44]; - let interrupt_pin = bytes[45]; + let (interrupt_pin, interrupt_line) = endpoint_header.interrupt(access); Ok(PciHeader::General { shared, - bars, subsystem_vendor_id, subsystem_id, cap_pointer, @@ -154,8 +127,6 @@ impl PciHeader { } ret }; - let mut bars = [PciBar::None; 2]; - Self::get_bars(&bytes, &mut bars); let secondary_bus_num = bytes[9]; let cap_pointer = bytes[36]; let interrupt_line = bytes[44]; @@ -163,7 +134,6 @@ impl PciHeader { let bridge_control = LittleEndian::read_u16(&bytes[46..48]); Ok(PciHeader::PciToPci { shared, - bars, secondary_bus_num, cap_pointer, interrupt_line, @@ -242,29 +212,52 @@ impl PciHeader { } /// Return the Headers BARs. - pub fn bars(&self) -> &[PciBar] { - match self { - &PciHeader::General { ref bars, .. } => bars, - &PciHeader::PciToPci { ref bars, .. } => bars, - } - } + // FIXME use pci_types::Bar instead + pub fn bars(&self, access: &impl ConfigRegionAccess) -> [PciBar; 6] { + let endpoint_header = match *self { + PciHeader::General { + shared: SharedPciHeader { addr, .. }, + .. + } => EndpointHeader::from_header(TyPciHeader::new(addr), access).unwrap(), + PciHeader::PciToPci { .. } => unreachable!(), + }; - /// Return the BAR at the given index. - /// - /// # Panics - /// This function panics if the requested BAR index is beyond the length of the header - /// types BAR array. - pub fn get_bar(&self, idx: usize) -> PciBar { - match self { - &PciHeader::General { bars, .. } => { - assert!(idx < 6, "the general PCI device only has 6 BARs"); - bars[idx] + let mut bars = [PciBar::None; 6]; + let mut skip = false; + for i in 0..6 { + if skip { + skip = false; + continue; } - &PciHeader::PciToPci { bars, .. } => { - assert!(idx < 2, "the general PCI device only has 2 BARs"); - bars[idx] + match endpoint_header.bar(i, access) { + Some(TyBar::Io { port }) => { + bars[i as usize] = PciBar::Port(port.try_into().unwrap()) + } + Some(TyBar::Memory32 { + address, + size, + prefetchable: _, + }) => { + bars[i as usize] = PciBar::Memory32 { + addr: address, + size, + } + } + Some(TyBar::Memory64 { + address, + size, + prefetchable: _, + }) => { + bars[i as usize] = PciBar::Memory64 { + addr: address, + size, + }; + skip = true; // Each 64bit memory BAR occupies two slots + } + None => bars[i as usize] = PciBar::None, } } + bars } /// Return the Interrupt Line field. @@ -304,7 +297,7 @@ mod test { use pci_types::{ConfigRegionAccess, PciAddress}; use super::{PciHeader, PciHeaderError, PciHeaderType}; - use crate::pci::{PciBar, PciClass}; + use crate::pci::PciClass; struct TestCfgAccess<'a> { addr: PciAddress, @@ -365,13 +358,6 @@ mod test { assert_eq!(header.interface(), 0); assert_eq!(header.class(), PciClass::Network); assert_eq!(header.subclass(), 0); - assert_eq!(header.bars().len(), 6); - assert_eq!(header.get_bar(0), PciBar::Memory32(0xf7500000)); - assert_eq!(header.get_bar(1), PciBar::None); - assert_eq!(header.get_bar(2), PciBar::Port(0xb000)); - assert_eq!(header.get_bar(3), PciBar::Memory32(0xf7580000)); - assert_eq!(header.get_bar(4), PciBar::None); - assert_eq!(header.get_bar(5), PciBar::None); assert_eq!(header.interrupt_line(), 10); } diff --git a/rtl8139d/src/main.rs b/rtl8139d/src/main.rs index 62925a1ec2..dc3616a08a 100644 --- a/rtl8139d/src/main.rs +++ b/rtl8139d/src/main.rs @@ -171,24 +171,23 @@ 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_ptr = pci_config.func.bars[bir].expect_mem() as u64; - let bar_size = pci_config.func.bar_sizes[bir] as u64; + let (bar_ptr, bar_size) = pci_config.func.bars[bir].expect_mem(); 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, common::Prot::RW, common::MemoryType::Uncacheable) .expect("rtl8139d: failed to map address") as usize }; - if !(bar_ptr..bar_ptr + bar_size).contains(&(table_base as u64 + table_min_length as u64)) { + if !(bar_ptr as u64..bar_ptr as u64 + bar_size as u64).contains(&(table_base as u64 + table_min_length as u64)) { panic!("Table {:#x}{:#x} outside of BAR {:#x}:{:#x}", table_base, table_base + table_min_length as usize, bar_ptr, bar_ptr + bar_size); } - if !(bar_ptr..bar_ptr + bar_size).contains(&(pba_base as u64 + pba_min_length as u64)) { + if !(bar_ptr as u64..bar_ptr as u64 + bar_size as u64).contains(&(pba_base as u64 + pba_min_length as u64)) { panic!("PBA {:#x}{:#x} outside of BAR {:#x}:{:#X}", pba_base, pba_base + pba_min_length as usize, bar_ptr, bar_ptr + bar_size); } - let virt_table_base = ((table_base - bar_ptr as usize) + address) as *mut MsixTableEntry; - let virt_pba_base = ((pba_base - bar_ptr as usize) + address) as *mut u64; + let virt_table_base = ((table_base - bar_ptr) + address) as *mut MsixTableEntry; + let virt_pba_base = ((pba_base - bar_ptr) + address) as *mut u64; let mut info = MsixInfo { virt_table_base: NonNull::new(virt_table_base).unwrap(), @@ -299,20 +298,14 @@ fn find_bar(pci_config: &SubdriverArguments) -> Option<(usize, usize)> { // RTL8139 uses BAR2, RTL8169 uses BAR1, search in that order for &barnum in &[2, 1] { match pci_config.func.bars[barnum] { - pcid_interface::PciBar::Memory32(ptr) => match ptr { - 0 => log::warn!("BAR {} is mapped to address 0", barnum), - _ => return Some(( - ptr.try_into().unwrap(), - pci_config.func.bar_sizes[barnum].try_into().unwrap() - )), - }, - pcid_interface::PciBar::Memory64(ptr) => match ptr { - 0 => log::warn!("BAR {} is mapped to address 0", barnum), - _ => return Some(( - ptr.try_into().unwrap(), - pci_config.func.bar_sizes[barnum].try_into().unwrap() - )), - }, + pcid_interface::PciBar::Memory32 { addr, size } => return Some(( + addr.try_into().unwrap(), + size.try_into().unwrap() + )), + pcid_interface::PciBar::Memory64 { addr, size } => return Some(( + addr.try_into().unwrap(), + size.try_into().unwrap() + )), other => log::warn!("BAR {} is {:?} instead of memory BAR", barnum, other), } } diff --git a/rtl8168d/src/main.rs b/rtl8168d/src/main.rs index 284b84c3e4..2592100b2c 100644 --- a/rtl8168d/src/main.rs +++ b/rtl8168d/src/main.rs @@ -169,26 +169,23 @@ 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_size = pci_config.func.bar_sizes[bir] as u64; - - let bar_ptr = bar.expect_mem() as u64; + let (bar_ptr, bar_size) = pci_config.func.bars[bir].expect_mem(); 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, common::Prot::RW, common::MemoryType::Uncacheable) .expect("rtl8168d: failed to map address") as usize }; - if !(bar_ptr..bar_ptr + bar_size).contains(&(table_base as u64 + table_min_length as u64)) { + if !(bar_ptr as u64..bar_ptr as u64 + bar_size as u64).contains(&(table_base as u64 + table_min_length as u64)) { panic!("Table {:#x}{:#x} outside of BAR {:#x}:{:#x}", table_base, table_base + table_min_length as usize, bar_ptr, bar_ptr + bar_size); } - if !(bar_ptr..bar_ptr + bar_size).contains(&(pba_base as u64 + pba_min_length as u64)) { + if !(bar_ptr as u64..bar_ptr as u64 + bar_size as u64).contains(&(pba_base as u64 + pba_min_length as u64)) { panic!("PBA {:#x}{:#x} outside of BAR {:#x}:{:#X}", pba_base, pba_base + pba_min_length as usize, bar_ptr, bar_ptr + bar_size); } - let virt_table_base = ((table_base - bar_ptr as usize) + address) as *mut MsixTableEntry; - let virt_pba_base = ((pba_base - bar_ptr as usize) + address) as *mut u64; + let virt_table_base = ((table_base - bar_ptr) + address) as *mut MsixTableEntry; + let virt_pba_base = ((pba_base - bar_ptr) + address) as *mut u64; let mut info = MsixInfo { virt_table_base: NonNull::new(virt_table_base).unwrap(), @@ -299,20 +296,14 @@ fn find_bar(pci_config: &SubdriverArguments) -> Option<(usize, usize)> { // RTL8168 uses BAR2, RTL8169 uses BAR1, search in that order for &barnum in &[2, 1] { match pci_config.func.bars[barnum] { - pcid_interface::PciBar::Memory32(ptr) => match ptr { - 0 => log::warn!("BAR {} is mapped to address 0", barnum), - _ => return Some(( - ptr.try_into().unwrap(), - pci_config.func.bar_sizes[barnum].try_into().unwrap() - )), - }, - pcid_interface::PciBar::Memory64(ptr) => match ptr { - 0 => log::warn!("BAR {} is mapped to address 0", barnum), - _ => return Some(( - ptr.try_into().unwrap(), - pci_config.func.bar_sizes[barnum].try_into().unwrap() - )), - }, + pcid_interface::PciBar::Memory32 { addr, size } => return Some(( + addr.try_into().unwrap(), + size.try_into().unwrap() + )), + pcid_interface::PciBar::Memory64 { addr, size } => return Some(( + addr.try_into().unwrap(), + size.try_into().unwrap() + )), other => log::warn!("BAR {} is {:?} instead of memory BAR", barnum, other), } } diff --git a/vboxd/src/main.rs b/vboxd/src/main.rs index 0602649c5c..77f43eb10d 100644 --- a/vboxd/src/main.rs +++ b/vboxd/src/main.rs @@ -196,7 +196,7 @@ fn main() { let bar0 = pci_config.func.bars[0].expect_port(); - let bar1 = pci_config.func.bars[1].expect_mem(); + 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_64.rs b/virtio-core/src/arch/x86_64.rs index 2dd3cd27d3..5bccd00c2a 100644 --- a/virtio-core/src/arch/x86_64.rs +++ b/virtio-core/src/arch/x86_64.rs @@ -27,13 +27,12 @@ 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_ptr = pci_config.func.bars[bir].expect_mem() as u64; - let bar_size = pci_config.func.bar_sizes[bir] as u64; + let (bar_ptr, bar_size) = pci_config.func.bars[bir].expect_mem(); let address = unsafe { common::physmap( - bar_ptr as usize, - bar_size as usize, + bar_ptr, + bar_size, common::Prot::RW, common::MemoryType::Uncacheable, )? as usize @@ -41,7 +40,7 @@ pub fn enable_msix(pcid_handle: &mut PcidServerHandle) -> Result { // Ensure that the table and PBA are be within the BAR. { - let bar_range = bar_ptr..bar_ptr + bar_size; + let bar_range = bar_ptr as u64..bar_ptr as u64 + bar_size as u64; assert!(bar_range.contains(&(table_base as u64 + table_min_length as u64))); assert!(bar_range.contains(&(pba_base as u64 + pba_min_length as u64))); } diff --git a/virtio-core/src/probe.rs b/virtio-core/src/probe.rs index 0ad300a673..4f7d48719d 100644 --- a/virtio-core/src/probe.rs +++ b/virtio-core/src/probe.rs @@ -90,7 +90,7 @@ pub fn probe_device(pcid_handle: &mut PcidServerHandle) -> Result _ => continue, } - let addr = pci_config.func.bars[capability.bar as usize].expect_mem(); + 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 497fb01a16..9c8dbf542e 100644 --- a/xhcid/src/main.rs +++ b/xhcid/src/main.rs @@ -85,8 +85,7 @@ 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_ptr = pci_config.func.bars[0].expect_mem() as u64; - let bar_size = pci_config.func.bar_sizes[0] as u64; + let (bar_ptr, bar_size) = pci_config.func.bars[0].expect_mem(); let irq = pci_config.func.legacy_interrupt_line; let all_pci_features = pcid_handle.fetch_all_features().expect("xhcid: failed to fetch pci features"); @@ -146,11 +145,11 @@ fn get_int_method(pcid_handle: &mut PcidServerHandle, address: usize) -> (Option let pba_base = capability.pba_base_pointer(pci_config.func.bars); - if !(bar_ptr..bar_ptr + bar_size).contains(&(table_base as u64 + table_min_length as u64)) { + if !(bar_ptr as u64..bar_ptr as u64 + bar_size as u64).contains(&(table_base as u64 + table_min_length as u64)) { panic!("Table {:#x}{:#x} outside of BAR {:#x}:{:#x}", table_base, table_base + table_min_length as usize, bar_ptr, bar_ptr + bar_size); } - if !(bar_ptr..bar_ptr + bar_size).contains(&(pba_base as u64 + pba_min_length as u64)) { + if !(bar_ptr as u64..bar_ptr as u64 + bar_size as u64).contains(&(pba_base as u64 + pba_min_length as u64)) { panic!("PBA {:#x}{:#x} outside of BAR {:#x}:{:#X}", pba_base, pba_base + pba_min_length as usize, bar_ptr, bar_ptr + bar_size); } @@ -235,12 +234,11 @@ fn daemon(daemon: redox_daemon::Daemon) -> ! { let _logger_ref = setup_logging(&name); log::debug!("XHCI PCI CONFIG: {:?}", pci_config); - let bar_ptr = pci_config.func.bars[0].expect_mem(); - let bar_size = pci_config.func.bar_sizes[0]; + let (bar_ptr, bar_size) = pci_config.func.bars[0].expect_mem(); let irq = pci_config.func.legacy_interrupt_line; 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, common::Prot::RW, common::MemoryType::Uncacheable) .expect("xhcid: failed to map address") as usize }; From d2431d41404c2afddcd069a11536454ba22192bb Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 22 Jan 2024 16:32:18 +0100 Subject: [PATCH 7/9] Make all parsing in PciHeader::from_reader use pci_types --- pcid/src/pci_header.rs | 68 +++++++----------------------------------- 1 file changed, 10 insertions(+), 58 deletions(-) diff --git a/pcid/src/pci_header.rs b/pcid/src/pci_header.rs index b9918fdba7..de5d02c004 100644 --- a/pcid/src/pci_header.rs +++ b/pcid/src/pci_header.rs @@ -1,9 +1,9 @@ use std::convert::TryInto; use bitflags::bitflags; -use byteorder::{ByteOrder, LittleEndian}; use pci_types::{ Bar as TyBar, ConfigRegionAccess, EndpointHeader, PciAddress, PciHeader as TyPciHeader, + PciPciBridgeHeader, }; use serde::{Deserialize, Serialize}; @@ -41,7 +41,6 @@ pub struct SharedPciHeader { addr: PciAddress, } -// FIXME move out of pcid_interface #[derive(Clone, Copy, Debug, PartialEq)] pub enum PciHeader { General { @@ -49,16 +48,11 @@ pub enum PciHeader { subsystem_vendor_id: u16, subsystem_id: u16, cap_pointer: u8, - interrupt_line: u8, - interrupt_pin: u8, }, PciToPci { shared: SharedPciHeader, secondary_bus_num: u8, cap_pointer: u8, - interrupt_line: u8, - interrupt_pin: u8, - bridge_control: u16, }, } @@ -100,65 +94,29 @@ impl PciHeader { match header_type & PciHeaderType::HEADER_TYPE { PciHeaderType::GENERAL => { let endpoint_header = EndpointHeader::from_header(header, access).unwrap(); - let bytes = unsafe { - let mut ret = Vec::with_capacity(48); - for offset in (16..64).step_by(4) { - ret.extend(access.read(addr, offset).to_le_bytes()); - } - ret - }; let (subsystem_id, subsystem_vendor_id) = endpoint_header.subsystem(access); - let cap_pointer = bytes[36]; - let (interrupt_pin, interrupt_line) = endpoint_header.interrupt(access); + let cap_pointer = (unsafe { access.read(addr, 0x34) } & 0xff) as u8; Ok(PciHeader::General { shared, subsystem_vendor_id, subsystem_id, cap_pointer, - interrupt_line, - interrupt_pin, }) } PciHeaderType::PCITOPCI => { - let bytes = unsafe { - let mut ret = Vec::with_capacity(48); - for offset in (16..64).step_by(4) { - ret.extend(access.read(addr, offset).to_le_bytes()); - } - ret - }; - let secondary_bus_num = bytes[9]; - let cap_pointer = bytes[36]; - let interrupt_line = bytes[44]; - let interrupt_pin = bytes[45]; - let bridge_control = LittleEndian::read_u16(&bytes[46..48]); + let bridge_header = PciPciBridgeHeader::from_header(header, access).unwrap(); + let secondary_bus_num = bridge_header.secondary_bus_number(access); + let cap_pointer = (unsafe { access.read(addr, 0x34) } & 0xff) as u8; Ok(PciHeader::PciToPci { shared, secondary_bus_num, cap_pointer, - interrupt_line, - interrupt_pin, - bridge_control, }) } id => Err(PciHeaderError::UnknownHeaderType(id.bits())), } } - /// Return the Header Type. - pub fn header_type(&self) -> PciHeaderType { - match self { - &PciHeader::General { - shared: SharedPciHeader { header_type, .. }, - .. - } - | &PciHeader::PciToPci { - shared: SharedPciHeader { header_type, .. }, - .. - } => header_type, - } - } - /// Return all identifying information of the PCI function. pub fn full_device_id(&self) -> &FullDeviceId { match self { @@ -260,14 +218,6 @@ impl PciHeader { bars } - /// Return the Interrupt Line field. - pub fn interrupt_line(&self) -> u8 { - match self { - &PciHeader::General { interrupt_line, .. } - | &PciHeader::PciToPci { interrupt_line, .. } => interrupt_line, - } - } - pub fn status(&self) -> u16 { match self { &PciHeader::General { @@ -296,7 +246,7 @@ mod test { use pci_types::{ConfigRegionAccess, PciAddress}; - use super::{PciHeader, PciHeaderError, PciHeaderType}; + use super::{PciHeader, PciHeaderError}; use crate::pci::PciClass; struct TestCfgAccess<'a> { @@ -351,14 +301,16 @@ mod test { PciAddress::new(0, 2, 4, 0), ) .unwrap(); - assert_eq!(header.header_type(), PciHeaderType::GENERAL); + match header { + PciHeader::General { .. } => {} + _ => panic!("wrong header type"), + } assert_eq!(header.device_id(), 0x1533); assert_eq!(header.vendor_id(), 0x8086); assert_eq!(header.revision(), 3); assert_eq!(header.interface(), 0); assert_eq!(header.class(), PciClass::Network); assert_eq!(header.subclass(), 0); - assert_eq!(header.interrupt_line(), 10); } #[test] From fb0dcb384bb789081ca823170a636370510096f1 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 22 Jan 2024 16:48:52 +0100 Subject: [PATCH 8/9] Use pci_types in a couple more places in PciHeader::from_reader --- pcid/src/main.rs | 4 ++-- pcid/src/pci_header.rs | 39 ++++++++------------------------------- 2 files changed, 10 insertions(+), 33 deletions(-) diff --git a/pcid/src/main.rs b/pcid/src/main.rs index de3203345d..1c2c1d14e7 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -15,7 +15,7 @@ use crate::config::Config; use crate::pci::{PciBar, PciClass, PciFunc}; use crate::pci::cap::Capability as PciCapability; use crate::pci::func::{ConfigReader, ConfigWriter}; -use crate::pci_header::{PciHeader, PciHeaderError, PciHeaderType}; +use crate::pci_header::{PciHeader, PciHeaderError}; mod cfg_access; mod config; @@ -493,7 +493,7 @@ fn main(args: Args) { } }, Err(PciHeaderError::UnknownHeaderType(id)) => { - warn!("pcid: unknown header type: {}", id); + warn!("pcid: unknown header type: {id:?}"); } } } diff --git a/pcid/src/pci_header.rs b/pcid/src/pci_header.rs index de5d02c004..605743d5a9 100644 --- a/pcid/src/pci_header.rs +++ b/pcid/src/pci_header.rs @@ -1,35 +1,16 @@ use std::convert::TryInto; -use bitflags::bitflags; use pci_types::{ - Bar as TyBar, ConfigRegionAccess, EndpointHeader, PciAddress, PciHeader as TyPciHeader, - PciPciBridgeHeader, + Bar as TyBar, ConfigRegionAccess, EndpointHeader, HeaderType, PciAddress, + PciHeader as TyPciHeader, PciPciBridgeHeader, }; -use serde::{Deserialize, Serialize}; use crate::pci::{FullDeviceId, PciBar, PciClass}; #[derive(Debug, PartialEq)] pub enum PciHeaderError { NoDevice, - UnknownHeaderType(u8), -} - -bitflags! { - /// Flags found in the status register of a PCI device - #[derive(Serialize, Deserialize)] - pub struct PciHeaderType: u8 { - /// A general PCI device (Type 0x01). - const GENERAL = 0b00000000; - /// A PCI-to-PCI bridge device (Type 0x01). - const PCITOPCI = 0b00000001; - /// A PCI-to-PCI bridge device (Type 0x02). - const CARDBUSBRIDGE = 0b00000010; - /// A multifunction device. - const MULTIFUNCTION = 0b01000000; - /// Mask used for fetching the header type. - const HEADER_TYPE = 0b00000011; - } + UnknownHeaderType(HeaderType), } #[derive(Clone, Copy, Debug, PartialEq)] @@ -37,7 +18,6 @@ pub struct SharedPciHeader { full_device_id: FullDeviceId, command: u16, status: u16, - header_type: PciHeaderType, addr: PciAddress, } @@ -73,9 +53,7 @@ impl PciHeader { let command = (command_and_status & 0xffff) as u16; let status = (command_and_status >> 16) as u16; let (revision, class, subclass, interface) = header.revision_and_class(access); - let header_type = PciHeaderType::from_bits_truncate( - ((unsafe { access.read(addr, 12) } >> 24) & 0xff) as u8, - ); + let header_type = header.header_type(access); let shared = SharedPciHeader { full_device_id: FullDeviceId { vendor_id, @@ -87,12 +65,11 @@ impl PciHeader { }, command, status, - header_type, addr, }; - match header_type & PciHeaderType::HEADER_TYPE { - PciHeaderType::GENERAL => { + match header_type { + HeaderType::Endpoint => { let endpoint_header = EndpointHeader::from_header(header, access).unwrap(); let (subsystem_id, subsystem_vendor_id) = endpoint_header.subsystem(access); let cap_pointer = (unsafe { access.read(addr, 0x34) } & 0xff) as u8; @@ -103,7 +80,7 @@ impl PciHeader { cap_pointer, }) } - PciHeaderType::PCITOPCI => { + HeaderType::PciPciBridge => { let bridge_header = PciPciBridgeHeader::from_header(header, access).unwrap(); let secondary_bus_num = bridge_header.secondary_bus_number(access); let cap_pointer = (unsafe { access.read(addr, 0x34) } & 0xff) as u8; @@ -113,7 +90,7 @@ impl PciHeader { cap_pointer, }) } - id => Err(PciHeaderError::UnknownHeaderType(id.bits())), + ty => Err(PciHeaderError::UnknownHeaderType(ty)), } } From 18529f6cee6ed02f4d4378b1e0395ab940d722a5 Mon Sep 17 00:00:00 2001 From: bjorn3 <17426603+bjorn3@users.noreply.github.com> Date: Mon, 22 Jan 2024 16:57:53 +0100 Subject: [PATCH 9/9] Remove PciClass in favor of pci_types::DeviceType --- pcid/src/main.rs | 48 +++++++++---------------- pcid/src/pci/class.rs | 79 ------------------------------------------ pcid/src/pci/mod.rs | 2 -- pcid/src/pci_header.rs | 13 ++++--- 4 files changed, 25 insertions(+), 117 deletions(-) delete mode 100644 pcid/src/pci/class.rs diff --git a/pcid/src/main.rs b/pcid/src/main.rs index 1c2c1d14e7..5789d21263 100644 --- a/pcid/src/main.rs +++ b/pcid/src/main.rs @@ -5,6 +5,7 @@ use std::process::Command; use std::thread; use std::sync::{Arc, Mutex}; +use pci_types::device_type::DeviceType; use pci_types::{ConfigRegionAccess, PciAddress}; use structopt::StructOpt; use log::{debug, error, info, warn, trace}; @@ -12,7 +13,7 @@ use redox_log::{OutputBuilder, RedoxLogger}; use crate::cfg_access::Pcie; use crate::config::Config; -use crate::pci::{PciBar, PciClass, PciFunc}; +use crate::pci::{PciBar, PciFunc}; use crate::pci::cap::Capability as PciCapability; use crate::pci::func::{ConfigReader, ConfigWriter}; use crate::pci_header::{PciHeader, PciHeaderError}; @@ -210,38 +211,23 @@ fn handle_parsed_header(state: Arc, config: &Config, addr: PciAddress, he let mut string = format!("PCI {} {:>04X}:{:>04X} {:>02X}.{:>02X}.{:>02X}.{:>02X} {:?}", addr, header.vendor_id(), header.device_id(), raw_class, header.subclass(), header.interface(), header.revision(), header.class()); - match header.class() { - PciClass::Legacy if header.subclass() == 1 => string.push_str(" VGA CTL"), - PciClass::Storage => match header.subclass() { - 0x01 => { - string.push_str(" IDE"); - }, - 0x06 => if header.interface() == 0 { - string.push_str(" SATA VND"); - } else if header.interface() == 1 { - string.push_str(" SATA AHCI"); - }, - _ => () + let device_type = DeviceType::from((header.class(), header.subclass())); + match device_type { + DeviceType::LegacyVgaCompatible => string.push_str(" VGA CTL"), + DeviceType::IdeController => string.push_str(" IDE"), + DeviceType::SataController => match header.interface() { + 0 => string.push_str(" SATA VND"), + 1 => string.push_str(" SATA AHCI"), + _ => (), }, - PciClass::SerialBus => match header.subclass() { - 0x03 => match header.interface() { - 0x00 => { - string.push_str(" UHCI"); - }, - 0x10 => { - string.push_str(" OHCI"); - }, - 0x20 => { - string.push_str(" EHCI"); - }, - 0x30 => { - string.push_str(" XHCI"); - }, - _ => () - }, - _ => () + DeviceType::UsbController => match header.interface() { + 0x00 => string.push_str(" UHCI"), + 0x10 => string.push_str(" OHCI"), + 0x20 => string.push_str(" EHCI"), + 0x30 => string.push_str(" XHCI"), + _ => (), }, - _ => () + _ => (), } let bars = header.bars(&state.pcie); diff --git a/pcid/src/pci/class.rs b/pcid/src/pci/class.rs deleted file mode 100644 index 042c354df0..0000000000 --- a/pcid/src/pci/class.rs +++ /dev/null @@ -1,79 +0,0 @@ -use serde::{Deserialize, Serialize}; - -#[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] -pub enum PciClass { - Legacy, - Storage, - Network, - Display, - Multimedia, - Memory, - Bridge, - SimpleComms, - Peripheral, - Input, - Docking, - Processor, - SerialBus, - Wireless, - IntelligentIo, - SatelliteComms, - Cryptography, - SignalProc, - Reserved(u8), - Unknown -} - -impl From for PciClass { - fn from(class: u8) -> PciClass { - match class { - 0x00 => PciClass::Legacy, - 0x01 => PciClass::Storage, - 0x02 => PciClass::Network, - 0x03 => PciClass::Display, - 0x04 => PciClass::Multimedia, - 0x05 => PciClass::Memory, - 0x06 => PciClass::Bridge, - 0x07 => PciClass::SimpleComms, - 0x08 => PciClass::Peripheral, - 0x09 => PciClass::Input, - 0x0A => PciClass::Docking, - 0x0B => PciClass::Processor, - 0x0C => PciClass::SerialBus, - 0x0D => PciClass::Wireless, - 0x0E => PciClass::IntelligentIo, - 0x0F => PciClass::SatelliteComms, - 0x10 => PciClass::Cryptography, - 0x11 => PciClass::SignalProc, - 0xFF => PciClass::Unknown, - reserved => PciClass::Reserved(reserved) - } - } -} - -impl Into for PciClass { - fn into(self) -> u8 { - match self { - PciClass::Legacy => 0x00, - PciClass::Storage => 0x01, - PciClass::Network => 0x02, - PciClass::Display => 0x03, - PciClass::Multimedia => 0x04, - PciClass::Memory => 0x05, - PciClass::Bridge => 0x06, - PciClass::SimpleComms => 0x07, - PciClass::Peripheral => 0x08, - PciClass::Input => 0x09, - PciClass::Docking => 0x0A, - PciClass::Processor => 0x0B, - PciClass::SerialBus => 0x0C, - PciClass::Wireless => 0x0D, - PciClass::IntelligentIo => 0x0E, - PciClass::SatelliteComms => 0x0F, - PciClass::Cryptography => 0x10, - PciClass::SignalProc => 0x11, - PciClass::Unknown => 0xFF, - PciClass::Reserved(reserved) => reserved - } - } -} diff --git a/pcid/src/pci/mod.rs b/pcid/src/pci/mod.rs index f0eb8b558a..b8080263f5 100644 --- a/pcid/src/pci/mod.rs +++ b/pcid/src/pci/mod.rs @@ -1,12 +1,10 @@ pub use self::bar::PciBar; -pub use self::class::PciClass; pub use self::func::PciFunc; pub use self::id::FullDeviceId; pub use pci_types::PciAddress; mod bar; pub mod cap; -mod class; pub mod func; mod id; pub mod msi; diff --git a/pcid/src/pci_header.rs b/pcid/src/pci_header.rs index 605743d5a9..9905af380e 100644 --- a/pcid/src/pci_header.rs +++ b/pcid/src/pci_header.rs @@ -5,7 +5,7 @@ use pci_types::{ PciHeader as TyPciHeader, PciPciBridgeHeader, }; -use crate::pci::{FullDeviceId, PciBar, PciClass}; +use crate::pci::{FullDeviceId, PciBar}; #[derive(Debug, PartialEq)] pub enum PciHeaderError { @@ -142,8 +142,8 @@ impl PciHeader { } /// Return the Class field. - pub fn class(&self) -> PciClass { - PciClass::from(self.full_device_id().class) + pub fn class(&self) -> u8 { + self.full_device_id().class } /// Return the Headers BARs. @@ -221,10 +221,10 @@ impl PciHeader { mod test { use std::convert::TryInto; + use pci_types::device_type::DeviceType; use pci_types::{ConfigRegionAccess, PciAddress}; use super::{PciHeader, PciHeaderError}; - use crate::pci::PciClass; struct TestCfgAccess<'a> { addr: PciAddress, @@ -286,7 +286,10 @@ mod test { assert_eq!(header.vendor_id(), 0x8086); assert_eq!(header.revision(), 3); assert_eq!(header.interface(), 0); - assert_eq!(header.class(), PciClass::Network); + assert_eq!( + DeviceType::from((header.class(), header.subclass())), + DeviceType::EthernetController + ); assert_eq!(header.subclass(), 0); }