diff --git a/kernel/comps/pci/src/bus.rs b/kernel/comps/pci/src/bus.rs index d538e8f10..b6994db3d 100644 --- a/kernel/comps/pci/src/bus.rs +++ b/kernel/comps/pci/src/bus.rs @@ -10,15 +10,17 @@ use ostd::bus::BusProbeError; use super::{PciCommonDevice, device_info::PciDeviceId}; -/// PciDevice trait. +/// A trait that represents PCI devices. pub trait PciDevice: Sync + Send + Debug { - /// Gets device id. + /// Returns the device ID. fn device_id(&self) -> PciDeviceId; } -/// PCI device driver, PCI bus will pass the device through the `probe` function when a new device is registered. +/// A trait that represents PCI device drivers. +/// +/// The PCI bus will pass the device through the `probe` function when a new device is registered. pub trait PciDriver: Sync + Send + Debug { - /// Probe an unclaimed PCI device. + /// Probes an unclaimed PCI device. /// /// If the driver matches and succeeds in initializing the unclaimed device, /// then the driver will return an claimed instance of the device, @@ -33,10 +35,11 @@ pub trait PciDriver: Sync + Send + Debug { ) -> Result, (BusProbeError, PciCommonDevice)>; } -/// The PCI bus used to register PCI devices. If a component wishes to drive a PCI device, it needs to provide the following: +/// The PCI bus used to register PCI devices. /// -/// 1. The structure that implements the PciDevice trait. -/// 2. PCI driver. +/// If a component wishes to drive a PCI device, it needs to provide the following: +/// 1. The structure that implements the [`PciDevice`] trait. +/// 2. A [`PciDriver`] instance. pub struct PciBus { common_devices: VecDeque, devices: Vec>, @@ -46,7 +49,7 @@ pub struct PciBus { impl PciBus { /// Registers a PCI driver to the PCI bus. pub fn register_driver(&mut self, driver: Arc) { - debug!("Register driver:{:#x?}", driver); + debug!("Register PCI driver: {:#x?}", driver); let length = self.common_devices.len(); for _ in (0..length).rev() { let common_device = self.common_devices.pop_front().unwrap(); @@ -71,7 +74,7 @@ impl PciBus { } pub(super) fn register_common_device(&mut self, mut common_device: PciCommonDevice) { - debug!("Find pci common devices:{:x?}", common_device); + debug!("Find PCI common device: {:#x?}", common_device); let device_id = *common_device.device_id(); for driver in self.drivers.iter() { common_device = match driver.probe(common_device) { diff --git a/kernel/comps/pci/src/capability/mod.rs b/kernel/comps/pci/src/capability/mod.rs index e02c83e93..ac224a86d 100644 --- a/kernel/comps/pci/src/capability/mod.rs +++ b/kernel/comps/pci/src/capability/mod.rs @@ -67,7 +67,7 @@ pub enum CapabilityData { } impl Capability { - /// 0xFC, the top of the capability position. + /// The top of the capability position. const CAPABILITY_TOP: u16 = 0xFC; /// Gets the capability data @@ -77,26 +77,29 @@ impl Capability { /// Gets the capabilities of one device pub(super) fn device_capabilities(dev: &mut PciCommonDevice) -> Vec { - if !dev.status().contains(Status::CAPABILITIES_LIST) { + if !dev.read_status().contains(Status::CAPABILITIES_LIST) { return Vec::new(); } - // The offset of the first capability pointer is the same for PCI general devices and PCI bridge devices. + // The offset of the first capability pointer is the same for PCI general devices and PCI + // bridge devices. const CAP_OFFSET: u16 = PciGeneralDeviceCfgOffset::CapabilitiesPointer as u16; let mut cap_ptr = (dev.location().read8(CAP_OFFSET) as u16).align_down(align_of::() as _); let mut cap_ptr_vec = Vec::new(); let mut capabilities = Vec::new(); - // read all cap_ptr so that it is easy for us to get the length. + // Read all capability pointers so that it is easy for us to get the length of each + // capability. while cap_ptr > 0 { cap_ptr_vec.push(cap_ptr); cap_ptr = (dev.location().read8(cap_ptr + 1) as u16).align_down(align_of::() as _); } cap_ptr_vec.sort(); - // Push here so that we can calculate the length of the last capability. + // Push the top position so that we can calculate the length of the last capability. cap_ptr_vec.push(Self::CAPABILITY_TOP); + let length = cap_ptr_vec.len(); for i in 0..length - 1 { let cap_ptr = cap_ptr_vec[i]; diff --git a/kernel/comps/pci/src/capability/msix.rs b/kernel/comps/pci/src/capability/msix.rs index f95b27ba3..f97832592 100644 --- a/kernel/comps/pci/src/capability/msix.rs +++ b/kernel/comps/pci/src/capability/msix.rs @@ -15,12 +15,11 @@ use crate::{ /// MSI-X capability. It will set the BAR space it uses to be hidden. #[derive(Debug)] -#[repr(C)] pub struct CapabilityMsixData { loc: PciDeviceLocation, ptr: u16, table_size: u16, - /// MSIX table entry content: + /// MSI-X table entry content: /// | Vector Control: u32 | Msg Data: u32 | Msg Upper Addr: u32 | Msg Addr: u32 | table_bar: Arc, /// Pending bits table. @@ -85,13 +84,11 @@ impl CapabilityMsixData { let table_offset = (table_info & !(0b111u32)) as usize; let table_size = (dev.location().read16(cap_ptr + 2) & 0b11_1111_1111) + 1; - // TODO: Different architecture seems to have different, so we should set different address here. + + // Set the message address and disable all MSI-X vectors. let message_address = MSIX_DEFAULT_MSG_ADDR; let message_upper_address = 0u32; - - // Set message address 0xFEE0_0000 for i in 0..table_size { - // Set message address and disable this msix entry table_bar .io_mem() .write_once((16 * i) as usize + table_offset, &message_address) @@ -106,11 +103,11 @@ impl CapabilityMsixData { .unwrap(); } - // enable MSI-X, bit15: MSI-X Enable + // Enable MSI-X (bit 15: MSI-X Enable). dev.location() .write16(cap_ptr + 2, dev.location().read16(cap_ptr + 2) | 0x8000); - // disable INTx, enable Bus master. - dev.set_command(dev.command() | Command::INTERRUPT_DISABLE | Command::BUS_MASTER); + // Disable INTx. Enable bus master. + dev.write_command(dev.read_command() | Command::INTERRUPT_DISABLE | Command::BUS_MASTER); let mut irqs = Vec::with_capacity(table_size as usize); for _ in 0..table_size { @@ -129,13 +126,15 @@ impl CapabilityMsixData { } } - /// MSI-X Table size + /// Returns the size of the MSI-X Table. pub fn table_size(&self) -> u16 { // bit 10:0 table size (self.loc.read16(self.ptr + 2) & 0b11_1111_1111) + 1 } - /// Enables an interrupt line, it will replace the old handle with the new handle. + /// Enables an interrupt line. + /// + /// If the interrupt line has already been enabled, the old [`IrqLine`] will be replaced. pub fn set_interrupt_vector(&mut self, irq: IrqLine, index: u16) { if index >= self.table_size { return; @@ -164,19 +163,21 @@ impl CapabilityMsixData { } let _old_irq = self.irqs[index as usize].replace(irq); - // Enable this msix vector + // Enable this MSI-X vector. self.table_bar .io_mem() .write_once((16 * index + 12) as usize + self.table_offset, &0_u32) .unwrap(); } - /// Gets mutable IrqLine. User can register callbacks by using this function. + /// Returns a mutable reference to the [`IrqLine`]. + /// + /// Users can register callbacks using the returned [`IrqLine`] reference. pub fn irq_mut(&mut self, index: usize) -> Option<&mut IrqLine> { self.irqs[index].as_mut() } - /// Returns true if MSI-X Enable bit is set. + /// Returns true if the MSI-X Enable bit is set. pub fn is_enabled(&self) -> bool { let msg_ctrl = self.loc.read16(self.ptr + 2); msg_ctrl & 0x8000 != 0 diff --git a/kernel/comps/pci/src/capability/vendor.rs b/kernel/comps/pci/src/capability/vendor.rs index 9bff9cbc5..adcd6868a 100644 --- a/kernel/comps/pci/src/capability/vendor.rs +++ b/kernel/comps/pci/src/capability/vendor.rs @@ -24,55 +24,57 @@ impl CapabilityVndrData { } } - /// The length of this capability + /// Returns the length of this capability. #[expect(clippy::len_without_is_empty)] pub fn len(&self) -> u16 { self.length } - /// Reads u8 from the capability. + /// Reads a `u8` from the capability. pub fn read8(&self, offset: u16) -> Result { - self.check_range(offset)?; + self.check_range(offset, size_of::() as u16)?; Ok(self.location.read8(self.cap_ptr + offset)) } - /// Writes u8 to the capability. + /// Writes a `u8` to the capability. pub fn write8(&self, offset: u16, value: u8) -> Result<()> { - self.check_range(offset)?; + self.check_range(offset, size_of::() as u16)?; self.location.write8(self.cap_ptr + offset, value); Ok(()) } - /// Reads u16 from the capability. + /// Reads a `u16` from the capability. pub fn read16(&self, offset: u16) -> Result { - self.check_range(offset)?; + self.check_range(offset, size_of::() as u16)?; Ok(self.location.read16(self.cap_ptr + offset)) } - /// Writes u16 to the capability. + /// Writes a `u16` to the capability. pub fn write16(&self, offset: u16, value: u16) -> Result<()> { - self.check_range(offset)?; + self.check_range(offset, size_of::() as u16)?; self.location.write16(self.cap_ptr + offset, value); Ok(()) } - /// Reads u32 from the capability. + /// Reads a `u32` from the capability. pub fn read32(&self, offset: u16) -> Result { - self.check_range(offset)?; + self.check_range(offset, size_of::() as u16)?; Ok(self.location.read32(self.cap_ptr + offset)) } - /// Writes u32 to the capability. + /// Writes a `u32` to the capability. pub fn write32(&self, offset: u16, value: u32) -> Result<()> { - self.check_range(offset)?; + self.check_range(offset, size_of::() as u16)?; self.location.write32(self.cap_ptr + offset, value); Ok(()) } - fn check_range(&self, offset: u16) -> Result<()> { - if self.length < offset { - return Err(Error::InvalidArgs); + fn check_range(&self, offset: u16, size: u16) -> Result<()> { + if let Some(end) = offset.checked_add(size) + && end <= self.length + { + return Ok(()); } - Ok(()) + Err(Error::InvalidArgs) } } diff --git a/kernel/comps/pci/src/cfg_space.rs b/kernel/comps/pci/src/cfg_space.rs index 0eb6f6177..43b89b484 100644 --- a/kernel/comps/pci/src/cfg_space.rs +++ b/kernel/comps/pci/src/cfg_space.rs @@ -283,7 +283,7 @@ impl Bar { } } -/// Memory BAR +/// Memory BAR. #[derive(Debug)] pub struct MemoryBar { base: u64, @@ -294,23 +294,23 @@ pub struct MemoryBar { } impl MemoryBar { - /// Memory BAR bits type + /// Returns the BAR's address length (32 bits or 64 bits). pub fn address_length(&self) -> AddrLen { self.address_length } - /// Whether this bar is prefetchable, allowing the CPU to get the data + /// Returns whether the BAR is prefetchable, i.e., whether the CPU is allowed to get the data /// in advance. pub fn prefetchable(&self) -> bool { self.prefetchable } - /// Base address + /// Returns the BAR's base address. pub fn base(&self) -> u64 { self.base } - /// Size of the memory + /// Returns the BAR's size. pub fn size(&self) -> u64 { self.size } @@ -422,7 +422,7 @@ impl MemoryBar { } } -/// Whether this BAR is 64bit address or 32bit address +/// The address length of a memory BAR (32 bits or 64 bits). #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub enum AddrLen { /// 32 bits @@ -439,12 +439,12 @@ pub struct IoBar { } impl IoBar { - /// Base port + /// Returns the BAR's base port. pub fn base(&self) -> u32 { self.base } - /// Size of the port + /// Returns the BAR's size. pub fn size(&self) -> u32 { self.size } diff --git a/kernel/comps/pci/src/common_device.rs b/kernel/comps/pci/src/common_device.rs index 96d93b308..0ad8c76b0 100644 --- a/kernel/comps/pci/src/common_device.rs +++ b/kernel/comps/pci/src/common_device.rs @@ -14,7 +14,9 @@ use crate::{ device_info::PciDeviceLocation, }; -/// PCI common device, Contains a range of information and functions common to PCI devices. +/// PCI common device. +/// +/// This type contains a range of information and functions common to PCI devices. #[derive(Debug)] pub struct PciCommonDevice { device_id: PciDeviceId, @@ -25,55 +27,55 @@ pub struct PciCommonDevice { } impl PciCommonDevice { - /// Gets the PCI device ID + /// Returns the PCI device ID. pub fn device_id(&self) -> &PciDeviceId { &self.device_id } - /// Gets the PCI device location + /// Returns the PCI device location. pub fn location(&self) -> &PciDeviceLocation { &self.location } - /// Gets the PCI Base Address Register (BAR) manager + /// Returns the PCI Base Address Register (BAR) manager. pub fn bar_manager(&self) -> &BarManager { &self.bar_manager } - /// Gets the PCI capabilities + /// Returns the PCI capabilities. pub fn capabilities(&self) -> &Vec { &self.capabilities } - /// Gets the PCI device type + /// Returns the PCI device type. pub fn device_type(&self) -> PciDeviceType { self.header_type.device_type() } - /// Checks whether the device is a multi-function device + /// Checks whether the device is a multi-function device. pub fn has_multi_funcs(&self) -> bool { self.header_type.has_multi_funcs() } - /// Gets the PCI Command - pub fn command(&self) -> Command { + /// Reads the PCI command. + pub fn read_command(&self) -> Command { Command::from_bits_truncate(self.location.read16(PciCommonCfgOffset::Command as u16)) } - /// Sets the PCI Command - pub fn set_command(&self, command: Command) { + /// Writes the PCI command. + pub fn write_command(&self, command: Command) { self.location .write16(PciCommonCfgOffset::Command as u16, command.bits()) } - /// Gets the PCI status - pub fn status(&self) -> Status { + /// Reads the PCI status. + pub fn read_status(&self) -> Status { Status::from_bits_truncate(self.location.read16(PciCommonCfgOffset::Status as u16)) } pub(super) fn new(location: PciDeviceLocation) -> Option { if location.read16(0) == 0xFFFF { - // not exists + // No device. return None; } @@ -101,8 +103,8 @@ impl PciCommonDevice { capabilities, }; - device.set_command( - device.command() | Command::MEMORY_SPACE | Command::BUS_MASTER | Command::IO_SPACE, + device.write_command( + device.read_command() | Command::MEMORY_SPACE | Command::BUS_MASTER | Command::IO_SPACE, ); device.bar_manager = BarManager::new(device.header_type.device_type(), location); device.capabilities = Capability::device_capabilities(&mut device); @@ -138,18 +140,19 @@ impl PciHeaderType { }) } - /// Gets the device type. + /// Returns the device type. pub fn device_type(self) -> PciDeviceType { self.device_type } - /// Indicates whether the device has multiple functions. + /// Returns whether the device has multiple functions. pub fn has_multi_funcs(self) -> bool { self.has_multi_funcs } } /// Represents the type of PCI device, determined by the device's header type. +/// /// Used to distinguish between general devices, PCI-to-PCI bridges, and PCI-to-Cardbus bridges. #[derive(Debug, Clone, Copy, Eq, PartialEq)] #[repr(u8)] @@ -175,7 +178,7 @@ impl PciDeviceType { } } -/// Base Address Registers manager. +/// Base Address Register (BAR) manager. #[derive(Debug)] pub struct BarManager { /// There are at most 6 BARs in PCI device. diff --git a/kernel/comps/pci/src/device_info.rs b/kernel/comps/pci/src/device_info.rs index 23736d5ac..d030fe844 100644 --- a/kernel/comps/pci/src/device_info.rs +++ b/kernel/comps/pci/src/device_info.rs @@ -4,7 +4,7 @@ use crate::cfg_space::PciCommonCfgOffset; -/// PCI device Location +/// PCI device location. #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] pub struct PciDeviceLocation { /// Bus number