Introduce PciAddress type copied from the pci_types crate

Cargo.lock

@@ -956,6 +956,7 @@ name = "pcid"
 version = "0.1.0"
 dependencies = [
  "bincode",
+ "bit_field",
  "bitflags 1.3.2",
  "byteorder 1.4.3",
  "common",

pcid/Cargo.toml

@@ -14,6 +14,7 @@ path = "src/lib.rs"
 [dependencies]
 bincode = "1.2"
 bitflags = "1"
+bit_field = "0.10"
 byteorder = "1.2"
 libc = "0.2"
 log = "0.4"

pcid/src/main.rs

@@ -10,7 +10,7 @@ use log::{debug, error, info, warn, trace};
 use redox_log::{OutputBuilder, RedoxLogger};
 
 use crate::config::Config;
-use crate::pci::{CfgAccess, Pci, PciIter, PciBar, PciBus, PciClass, PciDev, PciFunc, PciHeader, PciHeaderError, PciHeaderType};
+use crate::pci::{CfgAccess, Pci, PciAddress, PciBar, PciBus, PciClass, PciDev, PciFunc, PciHeader, PciHeaderError, PciHeaderType};
 use crate::pci::cap::Capability as PciCapability;
 use crate::pci::func::{ConfigReader, ConfigWriter};
 use crate::pcie::Pcie;
@@ -327,9 +327,9 @@ fn handle_parsed_header(state: Arc<State>, config: &Config, bus_num: u8,
         if let Some(ref args) = driver.command {
             // Enable bus mastering, memory space, and I/O space
             unsafe {
-                let mut data = pci.read(bus_num, dev_num, func_num, 0x04);
+                let mut data = pci.read(PciAddress::new(0, bus_num, dev_num, func_num), 0x04);
                 data |= 7;
-                pci.write(bus_num, dev_num, func_num, 0x04, data);
+                pci.write(PciAddress::new(0, bus_num, dev_num, func_num), 0x04, data);
             }
 
             // Set IRQ line to 9 if not set
@@ -337,14 +337,14 @@ fn handle_parsed_header(state: Arc<State>, config: &Config, bus_num: u8,
             let mut interrupt_pin;
 
             unsafe {
-                let mut data = pci.read(bus_num, dev_num, func_num, 0x3C);
+                let mut data = pci.read(PciAddress::new(0, bus_num, dev_num, func_num), 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(bus_num, dev_num, func_num, 0x3C, data);
+                pci.write(PciAddress::new(0, bus_num, dev_num, func_num), 0x3C, data);
             };
 
             // Find BAR sizes
@@ -363,11 +363,25 @@ fn handle_parsed_header(state: Arc<State>, config: &Config, bus_num: u8,
 
                     let offset = 0x10 + (i as u8) * 4;
 
-                    let original = pci.read(bus_num, dev_num, func_num, offset.into());
-                    pci.write(bus_num, dev_num, func_num, offset.into(), 0xFFFFFFFF);
+                    let original = pci.read(
+                        PciAddress::new(0, bus_num, dev_num, func_num),
+                        offset.into(),
+                    );
+                    pci.write(
+                        PciAddress::new(0, bus_num, dev_num, func_num),
+                        offset.into(),
+                        0xFFFFFFFF,
+                    );
 
-                    let new = pci.read(bus_num, dev_num, func_num, offset.into());
-                    pci.write(bus_num, dev_num, func_num, offset.into(), original);
+                    let new = pci.read(
+                        PciAddress::new(0, bus_num, dev_num, func_num),
+                        offset.into(),
+                    );
+                    pci.write(
+                        PciAddress::new(0, bus_num, dev_num, func_num),
+                        offset.into(),
+                        original,
+                    );
 
                     let masked = if new & 1 == 1 {
                         new & 0xFFFFFFFC

pcid/src/pci/bus.rs

@@ -1,8 +1,8 @@
-use super::{PciDev, CfgAccess};
+use super::{CfgAccess, PciAddress, PciDev};
 
 pub struct PciBus<'pci> {
     pub pci: &'pci dyn CfgAccess,
-    pub num: u8
+    pub num: u8,
 }
 
 impl<'pci> PciBus<'pci> {
@@ -11,24 +11,23 @@ impl<'pci> PciBus<'pci> {
     }
 
     pub unsafe fn read(&self, dev: u8, func: u8, offset: u16) -> u32 {
-        self.pci.read(self.num, dev, func, offset)
+        self.pci
+            .read(PciAddress::new(0, self.num, dev, func), offset)
     }
     pub unsafe fn write(&self, dev: u8, func: u8, offset: u16, value: u32) {
-        self.pci.write(self.num, dev, func, offset, value)
+        self.pci
+            .write(PciAddress::new(0, self.num, dev, func), offset, value)
     }
 }
 
 pub struct PciBusIter<'pci> {
     bus: &'pci PciBus<'pci>,
-    num: u8
+    num: u8,
 }
 
 impl<'pci> PciBusIter<'pci> {
     pub fn new(bus: &'pci PciBus<'pci>) -> Self {
-        PciBusIter {
-            bus,
-            num: 0
-        }
+        PciBusIter { bus, num: 0 }
     }
 }
 
@@ -39,11 +38,11 @@ impl<'pci> Iterator for PciBusIter<'pci> {
             dev_num if dev_num < 32 => {
                 let dev = PciDev {
                     bus: self.bus,
-                    num: self.num
+                    num: self.num,
                 };
                 self.num += 1;
                 Some(dev)
-            },
+            }
             _ => None,
         }
     }

pcid/src/pci/mod.rs

@@ -1,6 +1,8 @@
 use std::convert::TryFrom;
+use std::fmt;
 use std::sync::{Mutex, Once};
 
+use bit_field::BitField;
 #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
 use syscall::io::{Io as _, Pio};
 
@@ -23,8 +25,69 @@ pub mod header;
 pub mod msi;
 
 pub trait CfgAccess {
-    unsafe fn read(&self, bus: u8, dev: u8, func: u8, offset: u16) -> u32;
-    unsafe fn write(&self, bus: u8, dev: u8, func: u8, offset: u16, value: u32);
+    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.
+// 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)]
+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)
+    }
 }
 
 pub struct Pci {
@@ -48,42 +111,53 @@ impl Pci {
         // make sure that pcid is not granted io port permission unless pcie memory-mapped
         // configuration space is not available.
         info!("PCI: couldn't find or access PCIe extended configuration, and thus falling back to PCI 3.0 io ports");
-        unsafe { syscall::iopl(3).expect("pcid: failed to set iopl to 3"); }
+        unsafe {
+            syscall::iopl(3).expect("pcid: failed to set iopl to 3");
+        }
     }
-    fn address(bus: u8, dev: u8, func: u8, offset: u8) -> u32 {
+    fn address(address: PciAddress, offset: u8) -> u32 {
         // TODO: Find the part of pcid that uses an unaligned offset!
         //
         // assert_eq!(offset & 0xFC, offset, "pci offset is not aligned");
         //
         let offset = offset & 0xFC;
 
-        assert_eq!(dev & 0x1F, dev, "pci device larger than 5 bits");
-        assert_eq!(func & 0x7, func, "pci func larger than 3 bits");
+        assert_eq!(
+            address.segment(),
+            0,
+            "usage of multiple segments requires PCIe extended configuration"
+        );
 
-        0x80000000 | (u32::from(bus) << 16) | (u32::from(dev) << 11) | (u32::from(func) << 8) | u32::from(offset)
+        0x80000000
+            | (u32::from(address.bus()) << 16)
+            | (u32::from(address.device()) << 11)
+            | (u32::from(address.function()) << 8)
+            | u32::from(offset)
     }
 }
 #[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
 impl CfgAccess for Pci {
-    unsafe fn read(&self, bus: u8, dev: u8, func: u8, offset: u16) -> u32 {
+    unsafe fn read(&self, address: PciAddress, offset: u16) -> u32 {
         let _guard = self.lock.lock().unwrap();
 
         self.iopl_once.call_once(Self::set_iopl);
 
-        let offset = u8::try_from(offset).expect("offset too large for PCI 3.0 configuration space");
-        let address = Self::address(bus, dev, func, offset);
+        let offset =
+            u8::try_from(offset).expect("offset too large for PCI 3.0 configuration space");
+        let address = Self::address(address, offset);
 
         Pio::<u32>::new(0xCF8).write(address);
         Pio::<u32>::new(0xCFC).read()
     }
 
-    unsafe fn write(&self, bus: u8, dev: u8, func: u8, offset: u16, value: u32) {
+    unsafe fn write(&self, address: PciAddress, offset: u16, value: u32) {
         let _guard = self.lock.lock().unwrap();
 
         self.iopl_once.call_once(Self::set_iopl);
 
-        let offset = u8::try_from(offset).expect("offset too large for PCI 3.0 configuration space");
-        let address = Self::address(bus, dev, func, offset);
+        let offset =
+            u8::try_from(offset).expect("offset too large for PCI 3.0 configuration space");
+        let address = Self::address(address, offset);
 
         Pio::<u32>::new(0xCF8).write(address);
         Pio::<u32>::new(0xCFC).write(value);
@@ -91,12 +165,12 @@ impl CfgAccess for Pci {
 }
 #[cfg(not(any(target_arch = "x86", target_arch = "x86_64")))]
 impl CfgAccess for Pci {
-    unsafe fn read(&self, bus: u8, dev: u8, func: u8, offset: u16) -> u32 {
+    unsafe fn read(&self, addr: PciAddress, offset: u16) -> u32 {
         let _guard = self.lock.lock().unwrap();
         todo!("Pci::CfgAccess::read on this architecture")
     }
 
-    unsafe fn write(&self, bus: u8, dev: u8, func: u8, offset: u16, value: u32) {
+    unsafe fn write(&self, addr: PciAddress, offset: u16, value: u32) {
         let _guard = self.lock.lock().unwrap();
         todo!("Pci::CfgAccess::write on this architecture")
     }
@@ -104,15 +178,12 @@ impl CfgAccess for Pci {
 
 pub struct PciIter<'pci> {
     pci: &'pci dyn CfgAccess,
-    num: Option<u8>
+    num: Option<u8>,
 }
 
 impl<'pci> PciIter<'pci> {
     pub fn new(pci: &'pci dyn CfgAccess) -> Self {
-        PciIter {
-            pci,
-            num: Some(0)
-        }
+        PciIter { pci, num: Some(0) }
     }
 }
 
@@ -123,11 +194,11 @@ impl<'pci> Iterator for PciIter<'pci> {
             Some(bus_num) => {
                 let bus = PciBus {
                     pci: self.pci,
-                    num: bus_num
+                    num: bus_num,
                 };
                 self.num = bus_num.checked_add(1);
                 Some(bus)
-            },
+            }
             None => None,
         }
     }

pcid/src/pcie/mod.rs

@@ -1,12 +1,12 @@
-use std::{fmt, fs, io, mem, ptr, slice};
 use std::collections::BTreeMap;
 use std::sync::{Arc, Mutex};
+use std::{fmt, fs, io, mem, ptr, slice};
 
 use syscall::PAGE_SIZE;
 
 use smallvec::SmallVec;
 
-use crate::pci::{CfgAccess, Pci, PciIter};
+use crate::pci::{CfgAccess, Pci, PciAddress, PciIter};
 
 pub const MCFG_NAME: [u8; 4] = *b"MCFG";
 
@@ -137,7 +137,7 @@ impl fmt::Debug for Mcfgs {
 pub struct Pcie {
     lock: Mutex<()>,
     mcfgs: Mcfgs,
-    maps: Mutex<BTreeMap<(u8, u8, u8), *mut u32>>,
+    maps: Mutex<BTreeMap<PciAddress, *mut u32>>,
     fallback: Arc<Pci>,
 }
 unsafe impl Send for Pcie {}
@@ -154,34 +154,55 @@ impl Pcie {
             fallback,
         })
     }
-    fn addr_offset_in_bytes(starting_bus: u8, bus: u8, dev: u8, func: u8, offset: u16) -> usize {
+    fn addr_offset_in_bytes(starting_bus: u8, address: PciAddress, offset: u16) -> usize {
         assert_eq!(offset & 0xFFFC, offset, "pcie offset not dword-aligned");
         assert_eq!(offset & 0x0FFF, offset, "pcie offset larger than 4095");
-        assert_eq!(dev & 0x1F, dev, "pcie dev number larger than 5 bits");
-        assert_eq!(func & 0x7, func, "pcie func number larger than 3 bits");
 
-        (((bus - starting_bus) as usize) << 20) | ((dev as usize) << 15) | ((func as usize) << 12) | (offset as usize)
+        assert_eq!(
+            address.segment(),
+            0,
+            "multiple segments not yet implemented"
+        );
+
+        (((address.bus() - starting_bus) as usize) << 20)
+            | ((address.device() as usize) << 15)
+            | ((address.function() as usize) << 12)
+            | (offset as usize)
     }
-    fn addr_offset_in_dwords(starting_bus: u8, bus: u8, dev: u8, func: u8, offset: u16) -> usize {
-        Self::addr_offset_in_bytes(starting_bus, bus, dev, func, offset) / mem::size_of::<u32>()
+    fn addr_offset_in_dwords(starting_bus: u8, address: PciAddress, offset: u16) -> usize {
+        Self::addr_offset_in_bytes(starting_bus, address, offset) / mem::size_of::<u32>()
     }
-    unsafe fn with_pointer<T, F: FnOnce(Option<&mut u32>) -> T>(&self, bus: u8, dev: u8, func: u8, offset: u16, f: F) -> T {
-        let (base_address_phys, starting_bus) = match self.mcfgs.at_bus(bus) {
+    unsafe fn with_pointer<T, F: FnOnce(Option<&mut u32>) -> T>(
+        &self,
+        address: PciAddress,
+        offset: u16,
+        f: F,
+    ) -> T {
+        let (base_address_phys, starting_bus) = match self.mcfgs.at_bus(address.bus()) {
             Some(t) => (t.base_addr, t.start_bus),
             None => return f(None),
         };
         let mut maps_lock = self.maps.lock().unwrap();
-        let virt_pointer = maps_lock.entry((bus, dev, func)).or_insert_with(|| {
+        let virt_pointer = maps_lock.entry(address).or_insert_with(|| {
             common::physmap(
-                base_address_phys as usize + Self::addr_offset_in_bytes(starting_bus, bus, dev, func, 0),
+                base_address_phys as usize + Self::addr_offset_in_bytes(starting_bus, address, 0),
                 PAGE_SIZE,
-                common::Prot { read: true, write: true },
+                common::Prot {
+                    read: true,
+                    write: true,
+                },
                 common::MemoryType::Uncacheable,
-            ).unwrap_or_else(|error| {
-                panic!("failed to physmap pcie configuration space for {:2x}:{:2x}.{:2x}: {:?}", bus, dev, func, error)
+            )
+            .unwrap_or_else(|error| {
+                panic!(
+                    "failed to physmap pcie configuration space for {}: {:?}",
+                    address, error
+                )
             }) as *mut u32
         });
-        f(Some(&mut *virt_pointer.offset((offset as usize / mem::size_of::<u32>()) as isize)))
+        f(Some(&mut *virt_pointer.offset(
+            (offset as usize / mem::size_of::<u32>()) as isize,
+        )))
     }
     pub fn buses<'pcie>(&'pcie self) -> PciIter<'pcie> {
         PciIter::new(self)
@@ -189,21 +210,23 @@ impl Pcie {
 }
 
 impl CfgAccess for Pcie {
-    unsafe fn read(&self, bus: u8, dev: u8, func: u8, offset: u16) -> u32 {
+    unsafe fn read(&self, address: PciAddress, offset: u16) -> u32 {
         let _guard = self.lock.lock().unwrap();
 
-        self.with_pointer(bus, dev, func, offset, |pointer| match pointer {
+        self.with_pointer(address, offset, |pointer| match pointer {
             Some(address) => ptr::read_volatile::<u32>(address),
-            None => self.fallback.read(bus, dev, func, offset),
+            None => self.fallback.read(address, offset),
         })
     }
 
-    unsafe fn write(&self, bus: u8, dev: u8, func: u8, offset: u16, value: u32) {
+    unsafe fn write(&self, address: PciAddress, offset: u16, value: u32) {
         let _guard = self.lock.lock().unwrap();
 
-        self.with_pointer(bus, dev, func, offset, |pointer| match pointer {
+        self.with_pointer(address, offset, |pointer| match pointer {
             Some(address) => ptr::write_volatile::<u32>(address, value),
-            None => { self.fallback.read(bus, dev, func, offset); }
+            None => {
+                self.fallback.write(address, offset, value);
+            }
         });
     }
 }