btw the PCI subsystem consistently always writes the acronym all- uppercase (PCI) in commit messages and comments. If you go for a v5, might make sense to adjust that. Same applies for patch 8. P. On Thu, 2024-12-05 at 15:14 +0100, Danilo Krummrich wrote: > Implement `pci::Bar`, `pci::Device::iomap_region` and > `pci::Device::iomap_region_sized` to allow for I/O mappings of PCI > BARs. > > To ensure that a `pci::Bar`, and hence the I/O memory mapping, can't > out-live the PCI device, the `pci::Bar` type is always embedded into > a > `Devres` container, such that the `pci::Bar` is revoked once the > device > is unbound and hence the I/O mapped memory is unmapped. > > A `pci::Bar` can be requested with > (`pci::Device::iomap_region_sized`) or > without (`pci::Device::iomap_region`) a const generic representing > the > minimal requested size of the I/O mapped memory region. In case of > the > latter only runtime checked I/O reads / writes are possible. > > Co-developed-by: Philipp Stanner <pstanner@xxxxxxxxxx> > Signed-off-by: Philipp Stanner <pstanner@xxxxxxxxxx> > Signed-off-by: Danilo Krummrich <dakr@xxxxxxxxxx> > --- > rust/kernel/pci.rs | 145 > +++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 145 insertions(+) > > diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs > index 748912c1c994..0a32a5935c9c 100644 > --- a/rust/kernel/pci.rs > +++ b/rust/kernel/pci.rs > @@ -5,10 +5,14 @@ > //! C header: [`include/linux/pci.h`](srctree/include/linux/pci.h) > > use crate::{ > + alloc::flags::*, > bindings, container_of, device, > device_id::RawDeviceId, > + devres::Devres, > driver, > error::{to_result, Result}, > + io::Io, > + io::IoRaw, > str::CStr, > types::{ARef, ForeignOwnable}, > ThisModule, > @@ -239,9 +243,115 @@ pub trait Driver { > /// > /// A PCI device is based on an always reference counted > `device:Device` instance. Cloning a PCI > /// device, hence, also increments the base device' reference count. > +/// > +/// # Invariants > +/// > +/// `Device` hold a valid reference of `ARef<device::Device>` whose > underlying `struct device` is a > +/// member of a `struct pci_dev`. > #[derive(Clone)] > pub struct Device(ARef<device::Device>); > > +/// A PCI BAR to perform I/O-Operations on. > +/// > +/// # Invariants > +/// > +/// `Bar` always holds an `IoRaw` inststance that holds a valid > pointer to the start of the I/O > +/// memory mapped PCI bar and its size. > +pub struct Bar<const SIZE: usize = 0> { > + pdev: Device, > + io: IoRaw<SIZE>, > + num: i32, > +} > + > +impl<const SIZE: usize> Bar<SIZE> { > + fn new(pdev: Device, num: u32, name: &CStr) -> Result<Self> { > + let len = pdev.resource_len(num)?; > + if len == 0 { > + return Err(ENOMEM); > + } > + > + // Convert to `i32`, since that's what all the C bindings > use. > + let num = i32::try_from(num)?; > + > + // SAFETY: > + // `pdev` is valid by the invariants of `Device`. > + // `num` is checked for validity by a previous call to > `Device::resource_len`. > + // `name` is always valid. > + let ret = unsafe { > bindings::pci_request_region(pdev.as_raw(), num, name.as_char_ptr()) > }; > + if ret != 0 { > + return Err(EBUSY); > + } > + > + // SAFETY: > + // `pdev` is valid by the invariants of `Device`. > + // `num` is checked for validity by a previous call to > `Device::resource_len`. > + // `name` is always valid. > + let ioptr: usize = unsafe { > bindings::pci_iomap(pdev.as_raw(), num, 0) } as usize; > + if ioptr == 0 { > + // SAFETY: > + // `pdev` valid by the invariants of `Device`. > + // `num` is checked for validity by a previous call to > `Device::resource_len`. > + unsafe { bindings::pci_release_region(pdev.as_raw(), > num) }; > + return Err(ENOMEM); > + } > + > + let io = match IoRaw::new(ioptr, len as usize) { > + Ok(io) => io, > + Err(err) => { > + // SAFETY: > + // `pdev` is valid by the invariants of `Device`. > + // `ioptr` is guaranteed to be the start of a valid > I/O mapped memory region. > + // `num` is checked for validity by a previous call > to `Device::resource_len`. > + unsafe { Self::do_release(&pdev, ioptr, num) }; > + return Err(err); > + } > + }; > + > + Ok(Bar { pdev, io, num }) > + } > + > + /// # Safety > + /// > + /// `ioptr` must be a valid pointer to the memory mapped PCI bar > number `num`. > + unsafe fn do_release(pdev: &Device, ioptr: usize, num: i32) { > + // SAFETY: > + // `pdev` is valid by the invariants of `Device`. > + // `ioptr` is valid by the safety requirements. > + // `num` is valid by the safety requirements. > + unsafe { > + bindings::pci_iounmap(pdev.as_raw(), ioptr as _); > + bindings::pci_release_region(pdev.as_raw(), num); > + } > + } > + > + fn release(&self) { > + // SAFETY: The safety requirements are guaranteed by the > type invariant of `self.pdev`. > + unsafe { Self::do_release(&self.pdev, self.io.addr(), > self.num) }; > + } > +} > + > +impl Bar { > + fn index_is_valid(index: u32) -> bool { > + // A `struct pci_dev` owns an array of resources with at > most `PCI_NUM_RESOURCES` entries. > + index < bindings::PCI_NUM_RESOURCES > + } > +} > + > +impl<const SIZE: usize> Drop for Bar<SIZE> { > + fn drop(&mut self) { > + self.release(); > + } > +} > + > +impl<const SIZE: usize> Deref for Bar<SIZE> { > + type Target = Io<SIZE>; > + > + fn deref(&self) -> &Self::Target { > + // SAFETY: By the type invariant of `Self`, the MMIO range > in `self.io` is properly mapped. > + unsafe { Io::from_raw(&self.io) } > + } > +} > + > impl Device { > /// Create a PCI Device instance from an existing > `device::Device`. > /// > @@ -275,6 +385,41 @@ pub fn set_master(&self) { > // SAFETY: `self.as_raw` is guaranteed to be a pointer to a > valid `struct pci_dev`. > unsafe { bindings::pci_set_master(self.as_raw()) }; > } > + > + /// Returns the size of the given PCI bar resource. > + pub fn resource_len(&self, bar: u32) -> > Result<bindings::resource_size_t> { > + if !Bar::index_is_valid(bar) { > + return Err(EINVAL); > + } > + > + // SAFETY: > + // - `bar` is a valid bar number, as guaranteed by the above > call to `Bar::index_is_valid`, > + // - by its type invariant `self.as_raw` is always a valid > pointer to a `struct pci_dev`. > + Ok(unsafe { bindings::pci_resource_len(self.as_raw(), > bar.try_into()?) }) > + } > + > + /// Mapps an entire PCI-BAR after performing a region-request on > it. I/O operation bound checks > + /// can be performed on compile time for offsets (plus the > requested type size) < SIZE. > + pub fn iomap_region_sized<const SIZE: usize>( > + &self, > + bar: u32, > + name: &CStr, > + ) -> Result<Devres<Bar<SIZE>>> { > + let bar = Bar::<SIZE>::new(self.clone(), bar, name)?; > + let devres = Devres::new(self.as_ref(), bar, GFP_KERNEL)?; > + > + Ok(devres) > + } > + > + /// Mapps an entire PCI-BAR after performing a region-request on > it. > + pub fn iomap_region(&self, bar: u32, name: &CStr) -> > Result<Devres<Bar>> { > + self.iomap_region_sized::<0>(bar, name) > + } > + > + /// Returns a new `ARef` of the base `device::Device`. > + pub fn as_dev(&self) -> ARef<device::Device> { > + self.0.clone() > + } > } > > impl AsRef<device::Device> for Device {