On Sun May 17, 2026 at 1:01 AM BST, Danilo Krummrich wrote:
> Convert pci::Bar<SIZE> to pci::Bar<'bound, SIZE>, storing &'bound
> Device<Bound> to tie the BAR mapping lifetime to the device.
>
> iomap_region_sized() now returns Result<Bar<'bound, SIZE>> directly instead
> of impl PinInit<Devres<Bar<SIZE>>, Error>.
>
> Add Bar::into_devres() to consume the bar and register it as a
> device-managed resource, returning Devres<Bar<'static, SIZE>>. The
> lifetime is erased to 'static because Devres guarantees the bar does not
> actually outlive the device -- access is revoked on unbind.
>
> Signed-off-by: Danilo Krummrich <[email protected]>
> ---
> drivers/gpu/nova-core/driver.rs | 7 ++--
> rust/kernel/devres.rs | 2 +-
> rust/kernel/pci/io.rs | 68 +++++++++++++++++----------------
> samples/rust/rust_driver_pci.rs | 5 ++-
> 4 files changed, 44 insertions(+), 38 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> index ab0d7f2b516c..3f6d8cedf5ab 100644
> --- a/drivers/gpu/nova-core/driver.rs
> +++ b/drivers/gpu/nova-core/driver.rs
> @@ -47,7 +47,7 @@ pub(crate) struct NovaCore {
> // DMA addresses. These systems should be quite rare.
> const GPU_DMA_BITS: u32 = 47;
>
> -pub(crate) type Bar0 = pci::Bar<BAR0_SIZE>;
> +pub(crate) type Bar0 = pci::Bar<'static, BAR0_SIZE>;
>
> kernel::pci_device_table!(
> PCI_TABLE,
> @@ -94,8 +94,9 @@ fn probe<'bound>(
> // other threads of execution.
> unsafe {
> pdev.dma_set_mask_and_coherent(DmaMask::new::<GPU_DMA_BITS>())? };
>
> - let bar = Arc::pin_init(
> - pdev.iomap_region_sized::<BAR0_SIZE>(0, c"nova-core/bar0"),
> + let bar = Arc::new(
> + pdev.iomap_region_sized::<BAR0_SIZE>(0, c"nova-core/bar0")?
> + .into_devres()?,
> GFP_KERNEL,
> )?;
>
> diff --git a/rust/kernel/devres.rs b/rust/kernel/devres.rs
> index 9e5f93aed20c..ec63317665f4 100644
> --- a/rust/kernel/devres.rs
> +++ b/rust/kernel/devres.rs
> @@ -304,7 +304,7 @@ pub fn device(&self) -> &Device {
> /// pci, //
> /// };
> ///
> - /// fn from_core(dev: &pci::Device<Core>, devres: Devres<pci::Bar<0x4>>)
> -> Result {
> + /// fn from_core(dev: &pci::Device<Core>, devres: Devres<pci::Bar<'_,
> 0x4>>) -> Result {
> /// let bar = devres.access(dev.as_ref())?;
> ///
> /// let _ = bar.read32(0x0);
> diff --git a/rust/kernel/pci/io.rs b/rust/kernel/pci/io.rs
> index ae78676c927f..5668394a155b 100644
> --- a/rust/kernel/pci/io.rs
> +++ b/rust/kernel/pci/io.rs
> @@ -14,8 +14,7 @@
> Mmio,
> MmioRaw, //
> },
> - prelude::*,
> - sync::aref::ARef, //
> + prelude::*, //
> };
> use core::{
> marker::PhantomData,
> @@ -78,15 +77,15 @@ impl ConfigSpaceKind for Extended {
> /// The generic parameter `S` indicates the maximum size of the
> configuration space.
> /// Use [`Normal`] for 256-byte legacy configuration space or [`Extended`]
> for
> /// 4096-byte PCIe extended configuration space (default).
> -pub struct ConfigSpace<'a, S: ConfigSpaceKind = Extended> {
> - pub(crate) pdev: &'a Device<device::Bound>,
> +pub struct ConfigSpace<'bound, S: ConfigSpaceKind = Extended> {
> + pub(crate) pdev: &'bound Device<device::Bound>,
> _marker: PhantomData<S>,
> }
>
> /// Implements [`IoCapable`] on [`ConfigSpace`] for `$ty` using `$read_fn`
> and `$write_fn`.
> macro_rules! impl_config_space_io_capable {
> ($ty:ty, $read_fn:ident, $write_fn:ident) => {
> - impl<'a, S: ConfigSpaceKind> IoCapable<$ty> for ConfigSpace<'a, S> {
> + impl<'bound, S: ConfigSpaceKind> IoCapable<$ty> for
> ConfigSpace<'bound, S> {
> unsafe fn io_read(&self, address: usize) -> $ty {
> let mut val: $ty = 0;
>
> @@ -119,7 +118,7 @@ unsafe fn io_write(&self, value: $ty, address: usize) {
> impl_config_space_io_capable!(u16, pci_read_config_word,
> pci_write_config_word);
> impl_config_space_io_capable!(u32, pci_read_config_dword,
> pci_write_config_dword);
>
> -impl<'a, S: ConfigSpaceKind> Io for ConfigSpace<'a, S> {
> +impl<'bound, S: ConfigSpaceKind> Io for ConfigSpace<'bound, S> {
> /// Returns the base address of the I/O region. It is always 0 for
> configuration space.
> #[inline]
> fn addr(&self) -> usize {
> @@ -133,7 +132,7 @@ fn maxsize(&self) -> usize {
> }
> }
>
> -impl<'a, S: ConfigSpaceKind> IoKnownSize for ConfigSpace<'a, S> {
> +impl<'bound, S: ConfigSpaceKind> IoKnownSize for ConfigSpace<'bound, S> {
> const MIN_SIZE: usize = S::SIZE;
> }
>
> @@ -146,14 +145,14 @@ impl<'a, S: ConfigSpaceKind> IoKnownSize for
> ConfigSpace<'a, S> {
> ///
> /// `Bar` always holds an `IoRaw` instance 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: ARef<Device>,
> +pub struct Bar<'bound, const SIZE: usize = 0> {
> + pdev: &'bound Device<device::Bound>,
I think there's probably no need to name this `'bound`. From an individual
resource's perspective, there is no need to ensure that the lifetime is living
as long as a bound device.
It is perfectly fine to temporarily obtain a `Bar` and just destroy it before
device is unbound. So in some sense, this is
struct Bar<'a, const SIZE: usize> where 'bound: 'a
and this bound is implied by the `&'a Device<device::Bound>` type.
For interfaces which currently require invariance (e.g. `Driver::Data<'bound>`),
the name change is more important because the API does require a matching
lifetime (and also DevResLt).
> io: MmioRaw<SIZE>,
> num: i32,
> }
>
> -impl<const SIZE: usize> Bar<SIZE> {
> - pub(super) fn new(pdev: &Device, num: u32, name: &CStr) -> Result<Self> {
> +impl<'bound, const SIZE: usize> Bar<'bound, SIZE> {
> + pub(super) fn new(pdev: &'bound Device<device::Bound>, num: u32, name:
> &CStr) -> Result<Self> {
> let len = pdev.resource_len(num)?;
> if len == 0 {
> return Err(ENOMEM);
> @@ -196,11 +195,7 @@ pub(super) fn new(pdev: &Device, num: u32, name: &CStr)
> -> Result<Self> {
> }
> };
>
> - Ok(Bar {
> - pdev: pdev.into(),
> - io,
> - num,
> - })
> + Ok(Bar { pdev, io, num })
> }
>
> /// # Safety
> @@ -219,11 +214,24 @@ unsafe fn do_release(pdev: &Device, ioptr: usize, num:
> i32) {
>
> 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) };
> + unsafe { Self::do_release(self.pdev, self.io.addr(), self.num) };
> + }
> +
> + /// Consume the `Bar` and register it as a device-managed resource.
> + ///
> + /// The returned `Devres<Bar<'static, SIZE>>` can outlive the original
> lifetime `'bound`. Access
> + /// to the BAR is revoked when the device is unbound.
> + pub fn into_devres(self) -> Result<Devres<Bar<'static, SIZE>>> {
> + // SAFETY: Casting to `'static` is sound because `Devres` guarantees
> the `Bar` does not
> + // actually outlive the device -- access is revoked and the resource
> is released when the
> + // device is unbound.
> + let bar: Bar<'static, SIZE> = unsafe { core::mem::transmute(self) };
> + let pdev = bar.pdev;
> + Devres::new(pdev.as_ref(), bar)
One thing to note (which kinda motivates to suggest `'bound` might not be a good
name) is that while this is sound for `Bar`, this logic isn't true in general
for things that with the same shape as `Bar`.
For example, say if we have
struct MyResource<'bound> {
dev: &'bound Device<Bound>,
data: &'bound [u8],
}
then it's not okay to turn this into `DevRes<MyResource<'static>>` (or
`DevResLt<ForLt!(MyResource<'_>)>`, because the user can shorten the lifetime on
`&Device<Bound>`:
fn bad_fn<'bound>(dev: &'bound Device<Bound>) ->
DevRes<MyResource<'static>> {
let local = [0; 64];
// Basically `MyResource<'local>`.
let my_resource = MyResource {
dev,
data: &local,
};
my_resource.into_devres()
}
Now this is obviously bad because the resource now outlives `local` despite that
it shouldn't. The `'bound` lifetime becomes the the confusion factor here.
If this is just `&'a`, then you need to actually justify why the `into_devres()`
is fine, which you still can for `Bar` not for `MyResource`.
(This also means that sound and generic API of `DevResLt` would need either an
API with invariant lifetime, or with callback API. So something like
struct BoundDevice<'bound>(&'bound Device<Bound>, PhantomData<fn(&'bound
())-> &'bound ()>);
impl BoundDevice<'bound> {
fn into_devres<F: ForLt>(&self, value: F::Of<'bound>) -> DevResLt<F>;
}
Or
impl Device<Bound> {
fn new_devres<F: ForLt>(&self, f: impl for<'bound> FnOnce(&'bound
Device<Bound>) -> F::Of<'bound>) -> DevResLt<F>;
}
Either way, I think we probably want to limit `'bound` to cases where it's
actually `'bound` and not some possibly shortened lifetime.
Best,
Gary
> }
> }
>
> -impl Bar {
> +impl Bar<'_> {
> #[inline]
> pub(super) fn index_is_valid(index: u32) -> bool {
> // A `struct pci_dev` owns an array of resources with at most
> `PCI_NUM_RESOURCES` entries.
> @@ -231,13 +239,13 @@ pub(super) fn index_is_valid(index: u32) -> bool {
> }
> }
>
> -impl<const SIZE: usize> Drop for Bar<SIZE> {
> +impl<const SIZE: usize> Drop for Bar<'_, SIZE> {
> fn drop(&mut self) {
> self.release();
> }
> }
>
> -impl<const SIZE: usize> Deref for Bar<SIZE> {
> +impl<const SIZE: usize> Deref for Bar<'_, SIZE> {
> type Target = Mmio<SIZE>;
>
> fn deref(&self) -> &Self::Target {
> @@ -249,20 +257,16 @@ fn deref(&self) -> &Self::Target {
> impl Device<device::Bound> {
> /// Maps 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<'a, const SIZE: usize>(
> - &'a self,
> + pub fn iomap_region_sized<'bound, const SIZE: usize>(
> + &'bound self,
> bar: u32,
> - name: &'a CStr,
> - ) -> impl PinInit<Devres<Bar<SIZE>>, Error> + 'a {
> - Devres::new(self.as_ref(), Bar::<SIZE>::new(self, bar, name))
> + name: &CStr,
> + ) -> Result<Bar<'bound, SIZE>> {
> + Bar::new(self, bar, name)
> }
>
> /// Maps an entire PCI BAR after performing a region-request on it.
> - pub fn iomap_region<'a>(
> - &'a self,
> - bar: u32,
> - name: &'a CStr,
> - ) -> impl PinInit<Devres<Bar>, Error> + 'a {
> + pub fn iomap_region<'bound>(&'bound self, bar: u32, name: &CStr) ->
> Result<Bar<'bound>> {
> self.iomap_region_sized::<0>(bar, name)
> }
>
> @@ -282,7 +286,7 @@ pub fn cfg_size(&self) -> ConfigSpaceSize {
> }
>
> /// Return an initialized normal (256-byte) config space object.
> - pub fn config_space<'a>(&'a self) -> ConfigSpace<'a, Normal> {
> + pub fn config_space<'bound>(&'bound self) -> ConfigSpace<'bound, Normal>
> {
> ConfigSpace {
> pdev: self,
> _marker: PhantomData,
> @@ -290,7 +294,7 @@ pub fn config_space<'a>(&'a self) -> ConfigSpace<'a,
> Normal> {
> }
>
> /// Return an initialized extended (4096-byte) config space object.
> - pub fn config_space_extended<'a>(&'a self) -> Result<ConfigSpace<'a,
> Extended>> {
> + pub fn config_space_extended<'bound>(&'bound self) ->
> Result<ConfigSpace<'bound, Extended>> {
> if self.cfg_size() != ConfigSpaceSize::Extended {
> return Err(EINVAL);
> }
> diff --git a/samples/rust/rust_driver_pci.rs b/samples/rust/rust_driver_pci.rs
> index 30b9311812f3..7791363df0a0 100644
> --- a/samples/rust/rust_driver_pci.rs
> +++ b/samples/rust/rust_driver_pci.rs
> @@ -45,7 +45,7 @@ mod regs {
> pub(super) const END: usize = 0x10;
> }
>
> -type Bar0 = pci::Bar<{ regs::END }>;
> +type Bar0 = pci::Bar<'static, { regs::END }>;
>
> #[derive(Copy, Clone, Debug)]
> struct TestIndex(u8);
> @@ -161,7 +161,8 @@ fn probe<'bound>(
> pdev.set_master();
>
> Ok(try_pin_init!(Self {
> - bar <- pdev.iomap_region_sized::<{ regs::END }>(0,
> c"rust_driver_pci"),
> + bar: pdev.iomap_region_sized::<{ regs::END }>(0,
> c"rust_driver_pci")?
> + .into_devres()?,
> index: *info,
> _: {
> let bar = bar.access(pdev.as_ref())?;
