Re: [PATCH v3 19/27] rust: pci: make Bar lifetime-parameterized

["Gary Guo" <gary@garyguo.net>]

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 <dakr@kernel.org>
> ---
>  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())?;