Merge pull request #1958 from JarlEvanson/pci-root-bridge

uefi: Add PciRootBridgeIo memory and I/O space access

Author: phip1611

uefi-test-runner/src/proto/pci/root_bridge.rs

@@ -12,6 +12,7 @@ use uefi::proto::scsi::pass_thru::ExtScsiPassThru;
 use uefi_raw::protocol::pci::root_bridge::PciRootBridgeIoProtocolAttributes;
 
 const RED_HAT_PCI_VENDOR_ID: u16 = 0x1AF4;
+const VIRTIO_RNG_DEVICE_ID: u16 = 0x1005;
 const MASS_STORAGE_CTRL_CLASS_CODE: u8 = 0x1;
 const SATA_CTRL_SUBCLASS_CODE: u8 = 0x6;
 
@@ -40,15 +41,20 @@ fn test_enumeration_and_address_space_access() {
                 .pci()
                 .read_one::<u32>(addr.with_register(0))
                 .unwrap();
-            let reg1 = pci_proto
+            let reg2 = pci_proto
                 .pci()
                 .read_one::<u32>(addr.with_register(2 * REG_SIZE))
                 .unwrap();
+            let reg3 = pci_proto
+                .pci()
+                .read_one::<u32>(addr.with_register(3 * REG_SIZE))
+                .unwrap();
 
             let vendor_id = (reg0 & 0xFFFF) as u16;
             let device_id = (reg0 >> 16) as u16;
-            let class_code = (reg1 >> 24) as u8;
-            let subclass_code = ((reg1 >> 16) & 0xFF) as u8;
+            let class_code = (reg2 >> 24) as u8;
+            let subclass_code = ((reg2 >> 16) & 0xFF) as u8;
+            let header_type = ((reg3 >> 16) & 0x7F) as u8;
             let device_path = pci_tree.device_path(&root_device_path, addr).unwrap();
             let device_path_str = device_path
                 .to_string16(DisplayOnly(false), AllowShortcuts(false))
@@ -66,6 +72,63 @@ fn test_enumeration_and_address_space_access() {
                 mass_storage_dev_paths.insert(device_path_str.clone());
             }
 
+            if vendor_id == RED_HAT_PCI_VENDOR_ID && device_id == VIRTIO_RNG_DEVICE_ID {
+                // This PCI device has well-defined IO port and MMIO address spaces
+                // suitable for testing their respective access APIs These address
+                // spaces are suitable because they feature idempotent reads.
+                assert_eq!(
+                    header_type, 0x00,
+                    "unexpected header type for PCI virtio rng device"
+                );
+
+                let mut bars = [0; 6];
+                pci_proto
+                    .pci()
+                    .read::<u32>(addr.with_register(4 * REG_SIZE), &mut bars)
+                    .unwrap();
+                log::info!("BARS: {bars:#x?}");
+
+                let mut bars = bars.into_iter();
+                let mut next_bar = bars.next();
+                while let Some(bar) = next_bar {
+                    next_bar = bars.next();
+
+                    let bar = decode_bar(bar, next_bar);
+                    match bar {
+                        Bar::Io(base) => {
+                            // Virtio RNG devices have a device features register that is safe to
+                            // read at the start of the I/O space.
+                            let device_features = pci_proto.io().read_one::<u32>(base).unwrap();
+                            log::info!("Device Features: {device_features:#0b}");
+                        }
+                        Bar::Mem32 {
+                            base,
+                            prefetchable: true,
+                        } => {
+                            // Reading from a prefetchable MMIO region is always non-destructive.
+                            pci_proto.memory().read_one::<u32>(u64::from(base)).unwrap();
+                        }
+                        Bar::Mem64 {
+                            base,
+                            prefetchable: true,
+                        } => {
+                            // Reading from a prefetchable MMIO region is always non-destructive.
+                            pci_proto.memory().read_one::<u32>(base).unwrap();
+                        }
+                        _ => {}
+                    }
+
+                    log::info!("BAR: {bar:x?}");
+                    if let Bar::Mem64 {
+                        base: _,
+                        prefetchable: _,
+                    } = bar
+                    {
+                        next_bar = bars.next();
+                    }
+                }
+            }
+
             let (bus, dev, fun) = (addr.bus, addr.dev, addr.fun);
             log::info!(
                 "PCI Device: [{bus:02x}, {dev:02x}, {fun:02x}]: vendor={vendor_id:04X}, device={device_id:04X}, class={class_code:02X}, subclass={subclass_code:02X} - {}",
@@ -124,3 +187,34 @@ fn get_open_protocol<P: ProtocolPointer + ?Sized>(handle: Handle) -> ScopedProto
     let open_attrs = OpenProtocolAttributes::GetProtocol;
     unsafe { uefi::boot::open_protocol(open_opts, open_attrs).unwrap() }
 }
+
+fn decode_bar(bar: u32, next_bar: Option<u32>) -> Bar {
+    if bar & 0b1 == 0b0 {
+        match (bar & 0b110) >> 1 {
+            0b00 => Bar::Mem32 {
+                base: bar & !0b1111,
+                prefetchable: bar & 0b1000 != 0,
+            },
+            0b10 => {
+                if let Some(next_bar) = next_bar {
+                    Bar::Mem64 {
+                        base: u64::from(bar & !0b1111) | (u64::from(next_bar) << 32),
+                        prefetchable: bar & 0b1000 != 0,
+                    }
+                } else {
+                    unreachable!("PCI hardware error")
+                }
+            }
+            _ => unimplemented!(),
+        }
+    } else {
+        Bar::Io(bar & !0b11)
+    }
+}
+
+#[derive(Debug)]
+enum Bar {
+    Mem32 { base: u32, prefetchable: bool },
+    Mem64 { base: u64, prefetchable: bool },
+    Io(u32),
+}

uefi/CHANGELOG.md

@@ -4,6 +4,8 @@
 - Added `proto::console::text::InputEx`.
 - Added `proto::pci::PciRootBridgeIo::{supported_attributes(), attributes(),
   set_attributes(), set_attributes_with_range()}`
+- Added `memory()` and `io()` address space access to `PciRootBridgeIo`
+  protocol.
 
 ## Changed
 - MSRV increased from 1.88 to 1.91.
@@ -13,6 +15,8 @@
   argument should pass in `&[]` instead.
 - **Breaking:** Corrected function signature of `boot::exit` to enable handling
   errors during exit.
+- **Breaking:** Renamed `PciIoAccessPci` to `PciIoAccess` and added a generic
+  parameter to handle the PCI configuration, IO port, and MMIO address spaces.
 
 ## Removed
 - **Breaking:** Removed the deprecated `table::cfg::*_GUID` constants. Use

uefi/src/proto/pci/root_bridge.rs

@@ -36,11 +36,30 @@ impl PciRootBridgeIo {
         self.0.segment_number
     }
 
-    /// Access PCI I/O operations on this root bridge.
-    pub const fn pci(&mut self) -> PciIoAccessPci<'_> {
-        PciIoAccessPci {
+    /// Access PCI controller registers in the configuration space on this root bridge.
+    pub const fn pci(&mut self) -> PciIoAccess<'_, PciConfigurationSpace> {
+        PciIoAccess {
             proto: &mut self.0,
             io_access: &mut self.0.pci,
+            _address_space: PciConfigurationSpace,
+        }
+    }
+
+    /// Access PCI controller registers in the memory space on this root bridge.
+    pub const fn memory(&mut self) -> PciIoAccess<'_, PciMemorySpace> {
+        PciIoAccess {
+            proto: &mut self.0,
+            io_access: &mut self.0.mem,
+            _address_space: PciMemorySpace,
+        }
+    }
+
+    /// Access PCI controller registers in the I/O space on this root bridge.
+    pub const fn io(&mut self) -> PciIoAccess<'_, PciIoSpace> {
+        PciIoAccess {
+            proto: &mut self.0,
+            io_access: &mut self.0.io,
+            _address_space: PciIoSpace,
         }
     }
 
@@ -117,8 +136,6 @@ impl PciRootBridgeIo {
     }
 
     // TODO: poll I/O
-    // TODO: mem I/O access
-    // TODO: io I/O access
     // TODO: map & unmap & copy memory
     // TODO: buffer management
 
@@ -154,7 +171,7 @@ impl PciRootBridgeIo {
     /// An ordered list of addresses containing all present devices below this RootBridge.
     ///
     /// # Errors
-    /// This can basically fail with all the IO errors found in [`PciIoAccessPci`] methods.
+    /// This can basically fail with all the IO errors found in [`PciIoAccess`] methods.
     #[cfg(feature = "alloc")]
     pub fn enumerate(&mut self) -> crate::Result<super::enumeration::PciTree> {
         use super::enumeration::{self, PciTree};
@@ -182,12 +199,13 @@ impl PciRootBridgeIo {
 
 /// Struct for performing PCI I/O operations on a root bridge.
 #[derive(Debug)]
-pub struct PciIoAccessPci<'a> {
+pub struct PciIoAccess<'a, S: PciIoAddressSpace> {
     proto: *mut PciRootBridgeIoProtocol,
     io_access: &'a mut PciRootBridgeIoAccess,
+    _address_space: S,
 }
 
-impl PciIoAccessPci<'_> {
+impl<S: PciIoAddressSpace> PciIoAccess<'_, S> {
     /// Reads a single value of type `U` from the specified PCI address.
     ///
     /// # Arguments
@@ -199,7 +217,7 @@ impl PciIoAccessPci<'_> {
     /// # Errors
     /// - [`Status::INVALID_PARAMETER`] The requested width is invalid for this PCI root bridge.
     /// - [`Status::OUT_OF_RESOURCES`] The read request could not be completed due to a lack of resources.
-    pub fn read_one<U: PciIoUnit>(&self, addr: PciIoAddress) -> crate::Result<U> {
+    pub fn read_one<U: PciIoUnit>(&self, addr: S::Address) -> crate::Result<U> {
         let width_mode = encode_io_mode_and_unit::<U>(super::PciIoMode::Normal);
         let mut result = U::default();
         unsafe {
@@ -223,7 +241,7 @@ impl PciIoAccessPci<'_> {
     /// # Errors
     /// - [`Status::INVALID_PARAMETER`] The requested width is invalid for this PCI root bridge.
     /// - [`Status::OUT_OF_RESOURCES`] The write request could not be completed due to a lack of resources.
-    pub fn write_one<U: PciIoUnit>(&self, addr: PciIoAddress, data: U) -> crate::Result<()> {
+    pub fn write_one<U: PciIoUnit>(&self, addr: S::Address, data: U) -> crate::Result<()> {
         let width_mode = encode_io_mode_and_unit::<U>(super::PciIoMode::Normal);
         unsafe {
             (self.io_access.write)(
@@ -246,7 +264,7 @@ impl PciIoAccessPci<'_> {
     /// # Errors
     /// - [`Status::INVALID_PARAMETER`] The requested width is invalid for this PCI root bridge.
     /// - [`Status::OUT_OF_RESOURCES`] The read operation could not be completed due to a lack of resources.
-    pub fn read<U: PciIoUnit>(&self, addr: PciIoAddress, data: &mut [U]) -> crate::Result<()> {
+    pub fn read<U: PciIoUnit>(&self, addr: S::Address, data: &mut [U]) -> crate::Result<()> {
         let width_mode = encode_io_mode_and_unit::<U>(super::PciIoMode::Normal);
         unsafe {
             (self.io_access.read)(
@@ -269,7 +287,7 @@ impl PciIoAccessPci<'_> {
     /// # Errors
     /// - [`Status::INVALID_PARAMETER`] The requested width is invalid for this PCI root bridge.
     /// - [`Status::OUT_OF_RESOURCES`] The write operation could not be completed due to a lack of resources.
-    pub fn write<U: PciIoUnit>(&self, addr: PciIoAddress, data: &[U]) -> crate::Result<()> {
+    pub fn write<U: PciIoUnit>(&self, addr: S::Address, data: &[U]) -> crate::Result<()> {
         let width_mode = encode_io_mode_and_unit::<U>(super::PciIoMode::Normal);
         unsafe {
             (self.io_access.write)(
@@ -295,7 +313,7 @@ impl PciIoAccessPci<'_> {
     /// - [`Status::OUT_OF_RESOURCES`] The operation could not be completed due to a lack of resources.
     pub fn fill_write<U: PciIoUnit>(
         &self,
-        addr: PciIoAddress,
+        addr: S::Address,
         count: usize,
         data: U,
     ) -> crate::Result<()> {
@@ -325,7 +343,7 @@ impl PciIoAccessPci<'_> {
     /// # Errors
     /// - [`Status::INVALID_PARAMETER`] The requested width is invalid for this PCI root bridge.
     /// - [`Status::OUT_OF_RESOURCES`] The read operation could not be completed due to a lack of resources.
-    pub fn fifo_read<U: PciIoUnit>(&self, addr: PciIoAddress, data: &mut [U]) -> crate::Result<()> {
+    pub fn fifo_read<U: PciIoUnit>(&self, addr: S::Address, data: &mut [U]) -> crate::Result<()> {
         let width_mode = encode_io_mode_and_unit::<U>(super::PciIoMode::Fifo);
         unsafe {
             (self.io_access.read)(
@@ -352,7 +370,7 @@ impl PciIoAccessPci<'_> {
     /// # Errors
     /// - [`Status::INVALID_PARAMETER`] The requested width is invalid for this PCI root bridge.
     /// - [`Status::OUT_OF_RESOURCES`] The write operation could not be completed due to a lack of resources.
-    pub fn fifo_write<U: PciIoUnit>(&self, addr: PciIoAddress, data: &[U]) -> crate::Result<()> {
+    pub fn fifo_write<U: PciIoUnit>(&self, addr: S::Address, data: &[U]) -> crate::Result<()> {
         let width_mode = encode_io_mode_and_unit::<U>(super::PciIoMode::Fifo);
         unsafe {
             (self.io_access.write)(
@@ -366,3 +384,41 @@ impl PciIoAccessPci<'_> {
         }
     }
 }
+
+/// Marker struct for the PCI memory space.
+#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)]
+pub struct PciMemorySpace;
+
+impl private::Sealed for PciMemorySpace {}
+impl PciIoAddressSpace for PciMemorySpace {
+    type Address = u64;
+}
+
+/// Marker struct for the PCI I/O space.
+#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)]
+pub struct PciIoSpace;
+
+impl private::Sealed for PciIoSpace {}
+impl PciIoAddressSpace for PciIoSpace {
+    type Address = u32;
+}
+
+/// Marker struct for the PCI configuration space.
+#[derive(Clone, Copy, Debug, PartialEq, Eq, PartialOrd, Ord)]
+pub struct PciConfigurationSpace;
+
+impl private::Sealed for PciConfigurationSpace {}
+impl PciIoAddressSpace for PciConfigurationSpace {
+    type Address = PciIoAddress;
+}
+
+/// Trait representing how to convert from the address type expected for the address space and the
+/// raw address space.
+pub trait PciIoAddressSpace: private::Sealed {
+    /// Specifies the type of the address space addresses.
+    type Address: Into<u64>;
+}
+
+mod private {
+    pub trait Sealed {}
+}