dev/pci: make PCI driver an external crate

Cargo.toml

@@ -18,6 +18,9 @@ memtables = { path = "lib/memtables" }
 vmalloc = { path = "lib/vmalloc" }
 device-api-macros = { path = "lib/device-api/macros" }
 
+# Drivers
+ygg_driver_pci = { path = "driver/bus/pci" }
+
 atomic_enum = "0.2.0"
 bitflags = "2.3.3"
 linked_list_allocator = "0.10.5"

driver/bus/pci/Cargo.toml

@@ -0,0 +1,16 @@
+[package]
+name = "ygg_driver_pci"
+version = "0.1.0"
+edition = "2021"
+
+# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
+
+[dependencies]
+yggdrasil-abi = { git = "https://git.alnyan.me/yggdrasil/yggdrasil-abi.git" }
+device-api = { path = "../../../lib/device-api", features = ["derive"] }
+kernel-util = { path = "../../../lib/kernel-util" }
+
+log = "0.4.20"
+bitflags = "2.3.3"
+
+acpi = { git = "https://github.com/alnyan/acpi.git", package = "acpi", branch = "acpi-system" }

driver/bus/pci/src/capability.rs

@@ -1,15 +1,14 @@
 //! PCI capability structures and queries
 
-use abi::error::Error;
 use device_api::interrupt::{MessageInterruptController, MsiHandler};
 use kernel_util::mem::{
     address::{FromRaw, PhysicalAddress},
     device::DeviceMemoryIoMut,
 };
-
-use crate::device::bus::pci::PciBaseAddress;
+use yggdrasil_abi::error::Error;
 
 use super::{PciCapability, PciCapabilityId, PciConfigurationSpace};
+use crate::PciBaseAddress;
 
 /// MSI-X capability query
 pub struct MsiXCapability;
@@ -61,7 +60,7 @@ impl<'s, S: PciConfigurationSpace + ?Sized + 's> MsiXData<'s, S> {
             return Err(Error::InvalidOperation);
         };
 
-        debugln!("MSI-X table address: {:#x}", base + table_offset);
+        log::debug!("MSI-X table address: {:#x}", base + table_offset);
 
         unsafe {
             DeviceMemoryIoMut::map_slice(PhysicalAddress::from_raw(base + table_offset), table_size)

driver/bus/pci/src/lib.rs

@@ -1,8 +1,12 @@
 //! PCI/PCIe bus interfaces
+#![no_std]
+
+extern crate alloc;
+
 use core::fmt;
 
-use acpi_lib::mcfg::McfgEntry;
-use alloc::{boxed::Box, rc::Rc, vec::Vec};
+use acpi::mcfg::McfgEntry;
+use alloc::{rc::Rc, vec::Vec};
 use bitflags::bitflags;
 use device_api::Device;
 use kernel_util::{
@@ -18,8 +22,6 @@ pub use space::{
     ecam::PciEcam, PciConfigSpace, PciConfigurationSpace, PciLegacyConfigurationSpace,
 };
 
-use crate::device::nvme;
-
 bitflags! {
     /// Command register of the PCI configuration space
     pub struct PciCommandRegister: u16 {
@@ -100,10 +102,15 @@ pub struct PciDeviceInfo {
     pub config_space: PciConfigSpace,
 }
 
-/// Interface for "taking" PCI devices from the bus
-pub trait FromPciBus: Sized {
-    /// Constructs an instance of a driver for the device using the information provided
-    fn from_pci_bus(info: &PciDeviceInfo) -> Result<Self, Error>;
+pub enum PciMatch {
+    Generic(fn(&PciDeviceInfo) -> bool),
+    Class(u8, Option<u8>, Option<u8>),
+}
+
+pub struct PciDriver {
+    name: &'static str,
+    check: PciMatch,
+    probe: fn(&PciDeviceInfo) -> Result<&'static dyn Device, Error>,
 }
 
 /// Used to store PCI bus devices which were enumerated by the kernel
@@ -296,28 +303,76 @@ fn setup_bus_device(device: &mut PciBusDevice) -> Result<(), Error> {
 
     let config = &device.info.config_space;
 
-    debugln!(
+    log::debug!(
         "{}: {:04x}:{:04x}",
         device.info.address,
         config.vendor_id(),
         config.device_id()
     );
 
-    // Match by class
-    match (config.class_code(), config.subclass(), config.prog_if()) {
-        (0x01, 0x08, 0x02) => {
-            let dev = Box::leak(Box::new(nvme::NvmeController::from_pci_bus(&device.info)?));
+    let class = config.class_code();
+    let subclass = config.subclass();
+    let prog_if = config.prog_if();
+
+    let drivers = PCI_DRIVERS.lock();
+    for driver in drivers.iter() {
+        if driver
+            .check
+            .check_device(&device.info, class, subclass, prog_if)
+        {
+            // TODO add the device to the bus
+            log::debug!(" -> {:?}", driver.name);
+            let device = (driver.probe)(&device.info)?;
 
             unsafe {
-                dev.init()?;
+                device.init()?;
             }
-        }
-        _ => {
-            debugln!(" -> No driver");
+        } else {
+            log::debug!(" -> No driver");
         }
     }
 
     Ok(())
 }
 
+impl PciMatch {
+    pub fn check_device(&self, info: &PciDeviceInfo, class: u8, subclass: u8, prog_if: u8) -> bool {
+        match self {
+            Self::Generic(f) => f(info),
+            &Self::Class(class_, Some(subclass_), Some(prog_if_)) => {
+                class_ == class && subclass_ == subclass && prog_if_ == prog_if
+            }
+            &Self::Class(class_, Some(subclass_), _) => class_ == class && subclass_ == subclass,
+            &Self::Class(class_, _, _) => class_ == class,
+        }
+    }
+}
+
+pub fn register_class_driver(
+    name: &'static str,
+    class: u8,
+    subclass: Option<u8>,
+    prog_if: Option<u8>,
+    probe: fn(&PciDeviceInfo) -> Result<&'static dyn Device, Error>,
+) {
+    PCI_DRIVERS.lock().push(PciDriver {
+        name,
+        check: PciMatch::Class(class, subclass, prog_if),
+        probe,
+    });
+}
+
+pub fn register_generic_driver(
+    name: &'static str,
+    check: fn(&PciDeviceInfo) -> bool,
+    probe: fn(&PciDeviceInfo) -> Result<&'static dyn Device, Error>,
+) {
+    PCI_DRIVERS.lock().push(PciDriver {
+        name,
+        check: PciMatch::Generic(check),
+        probe,
+    });
+}
+
+static PCI_DRIVERS: IrqSafeSpinlock<Vec<PciDriver>> = IrqSafeSpinlock::new(Vec::new());
 static PCI_MANAGER: IrqSafeSpinlock<PciBusManager> = IrqSafeSpinlock::new(PciBusManager::new());

driver/bus/pci/src/space/mod.rs

@@ -1,6 +1,5 @@
-use crate::device::bus::pci::PciStatusRegister;
-
 use super::{PciAddress, PciBaseAddress, PciCapability, PciCapabilityId, PciEcam};
+use crate::PciStatusRegister;
 
 pub(super) mod ecam;
 

src/arch/x86_64/mod.rs

@@ -21,6 +21,7 @@ use kernel_util::{
     util::OneTimeInit,
 };
 use yboot_proto::{v1::AvailableMemoryRegion, LoadProtocolV1};
+use ygg_driver_pci::PciBusManager;
 
 mod acpi;
 mod apic;
@@ -49,8 +50,8 @@ use crate::{
     debug::{self, LogLevel},
     device::{
         self,
-        bus::pci::PciBusManager,
         display::{console, fb_console::FramebufferConsole, linear_fb::LinearFramebuffer},
+        nvme,
         tty::CombinedTerminal,
     },
     fs::{
@@ -384,6 +385,16 @@ impl X86_64 {
         Cpu::init_local(LocalApic::new(), cpu_id as _);
 
         if cpu_id == 0 {
+            // Register the PCI drivers
+            //  TODO make this implicit init
+            ygg_driver_pci::register_class_driver(
+                "NVMe Host Controller",
+                0x01,
+                Some(0x08),
+                Some(0x02),
+                nvme::probe,
+            );
+
             match self.boot_data.get() {
                 &BootData::YBoot(data) => {
                     let start = PhysicalAddress::from_raw(data.initrd_address);

src/device/bus/mod.rs

@@ -2,5 +2,3 @@
 
 #[cfg(feature = "device-tree")]
 pub mod simple_bus;
-
-pub mod pci;

src/device/nvme/mod.rs

@@ -2,7 +2,7 @@
 use core::{mem::size_of, time::Duration};
 
 use abi::error::Error;
-use alloc::{collections::BTreeMap, vec::Vec};
+use alloc::{boxed::Box, collections::BTreeMap, vec::Vec};
 use device_api::{interrupt::MsiHandler, Device};
 use kernel_util::{
     mem::{
@@ -17,20 +17,19 @@ use tock_registers::{
     register_bitfields, register_structs,
     registers::{ReadOnly, ReadWrite, WriteOnly},
 };
+use ygg_driver_pci::{
+    capability::{MsiXCapability, MsiXEntry},
+    PciBaseAddress, PciCommandRegister, PciConfigurationSpace, PciDeviceInfo,
+};
 
 use crate::{
     arch::{Architecture, ARCHITECTURE},
-    device::{
-        bus::pci::{
-            capability::MsiXCapability, PciBaseAddress, PciCommandRegister, PciConfigurationSpace,
-        },
-        nvme::{
-            command::{
-                IdentifyActiveNamespaceIdListRequest, IdentifyControllerRequest, IoRead, IoWrite,
-            },
-            drive::NvmeDrive,
-            queue::{CompletionQueueEntry, SubmissionQueueEntry},
+    device::nvme::{
+        command::{
+            IdentifyActiveNamespaceIdListRequest, IdentifyControllerRequest, IoRead, IoWrite,
         },
+        drive::NvmeDrive,
+        queue::{CompletionQueueEntry, SubmissionQueueEntry},
     },
     task::runtime,
 };
@@ -41,8 +40,6 @@ use self::{
     queue::QueuePair,
 };
 
-use super::bus::pci::{capability::MsiXEntry, FromPciBus, PciDeviceInfo};
-
 mod command;
 mod drive;
 mod error;
@@ -358,48 +355,51 @@ impl Device for NvmeController {
     }
 }
 
-impl FromPciBus for NvmeController {
-    fn from_pci_bus(info: &PciDeviceInfo) -> Result<Self, Error> {
-        let PciBaseAddress::Memory(bar0) = info.config_space.bar(0).unwrap() else {
-            panic!();
-        };
-
-        // TODO also support MSI
-        let mut msix = info.config_space.capability::<MsiXCapability>().unwrap();
-        let mut vt = msix.vector_table()?;
-
-        for vector in vt.iter_mut() {
-            vector.set_masked(true);
-        }
-        msix.set_enabled(true);
-
-        let mut cmd = PciCommandRegister::from_bits_retain(info.config_space.command());
-        cmd &= !(PciCommandRegister::DISABLE_INTERRUPTS | PciCommandRegister::ENABLE_IO);
-        cmd |= PciCommandRegister::ENABLE_MEMORY | PciCommandRegister::BUS_MASTER;
-        info.config_space.set_command(cmd.bits());
-
-        let regs = unsafe { DeviceMemoryIo::<Regs>::map(PhysicalAddress::from_raw(bar0)) }?;
-
-        // Disable the controller
-        regs.CC.modify(CC::ENABLE::CLEAR);
-
-        let doorbell_shift = regs.CAP.read(CAP::DSTRD) as usize + 1;
-
-        Ok(Self {
-            regs: IrqSafeSpinlock::new(regs),
-            admin_q: OneTimeInit::new(),
-            ioqs: OneTimeInit::new(),
-            vector_table: IrqSafeSpinlock::new(vt),
-            drive_table: IrqSafeSpinlock::new(BTreeMap::new()),
-            controller_id: OneTimeInit::new(),
-            doorbell_shift,
-        })
-    }
-}
+// impl FromPciBus for NvmeController {
+//     fn from_pci_bus(info: &PciDeviceInfo) -> Result<Self, Error> {
+//     }
+// }
 
 static NVME_CONTROLLERS: IrqSafeSpinlock<Vec<&'static NvmeController>> =
     IrqSafeSpinlock::new(Vec::new());
 
+pub fn probe(info: &PciDeviceInfo) -> Result<&'static dyn Device, Error> {
+    let PciBaseAddress::Memory(bar0) = info.config_space.bar(0).unwrap() else {
+        panic!();
+    };
+
+    // TODO also support MSI
+    let mut msix = info.config_space.capability::<MsiXCapability>().unwrap();
+    let mut vt = msix.vector_table()?;
+
+    for vector in vt.iter_mut() {
+        vector.set_masked(true);
+    }
+    msix.set_enabled(true);
+
+    let mut cmd = PciCommandRegister::from_bits_retain(info.config_space.command());
+    cmd &= !(PciCommandRegister::DISABLE_INTERRUPTS | PciCommandRegister::ENABLE_IO);
+    cmd |= PciCommandRegister::ENABLE_MEMORY | PciCommandRegister::BUS_MASTER;
+    info.config_space.set_command(cmd.bits());
+
+    let regs = unsafe { DeviceMemoryIo::<Regs>::map(PhysicalAddress::from_raw(bar0)) }?;
+
+    // Disable the controller
+    regs.CC.modify(CC::ENABLE::CLEAR);
+
+    let doorbell_shift = regs.CAP.read(CAP::DSTRD) as usize + 1;
+
+    Ok(Box::leak(Box::new(NvmeController {
+        regs: IrqSafeSpinlock::new(regs),
+        admin_q: OneTimeInit::new(),
+        ioqs: OneTimeInit::new(),
+        vector_table: IrqSafeSpinlock::new(vt),
+        drive_table: IrqSafeSpinlock::new(BTreeMap::new()),
+        controller_id: OneTimeInit::new(),
+        doorbell_shift,
+    })))
+}
+
 pub fn register_nvme_controller(ctrl: &'static NvmeController) {
     let mut list = NVME_CONTROLLERS.lock();
     let id = list.len();