Re: [PATCH] rust: iommu: add device lifetime to IoPageTable

[Alice Ryhl <aliceryhl@google.com>]

On Thu, Jul 02, 2026 at 04:47:39PM -0700, Deborah Brouwer wrote:
> Currently, using a raw IoPageTable is unsafe because the returned
> IoPageTable is not tied to the device driver binding lifetime.
> 
> Since device drivers now receive a lifetime parameter <'bound>
> representing the interval during which a device driver is bound to its bus
> device, add this lifetime parameter to IoPageTable. This ensures that
> the returned IoPageTable cannot outlive the bus device binding.
> 
> Also temporarily remove the option to create a page table as a device
> resource since currently Devres is not compatible with resources that have
> a lifetime parameter. This option can be restored once the lifetime-aware
> wrapper for devres is available.
> 
> Suggested-by: Boris Brezillon <boris.brezillon@collabora.com>
> Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
> ---
>  rust/kernel/iommu/pgtable.rs | 32 ++++++++++----------------------
>  1 file changed, 10 insertions(+), 22 deletions(-)
> 
> diff --git a/rust/kernel/iommu/pgtable.rs b/rust/kernel/iommu/pgtable.rs
> index c88e38fd938a..d4a16283b5a7 100644
> --- a/rust/kernel/iommu/pgtable.rs
> +++ b/rust/kernel/iommu/pgtable.rs
> @@ -16,7 +16,6 @@
>          Bound,
>          Device, //
>      },
> -    devres::Devres,
>      error::to_result,
>      io::PhysAddr,
>      prelude::*, //
> @@ -59,15 +58,16 @@ pub struct Config {
>  /// # Invariants
>  ///
>  /// The pointer references a valid io page table.
> -pub struct IoPageTable<F: IoPageTableFmt> {
> +pub struct IoPageTable<'bound, F: IoPageTableFmt> {
>      ptr: NonNull<bindings::io_pgtable_ops>,
> +    _dev: PhantomData<&'bound Device<Bound>>,
>      _marker: PhantomData<F>,
>  }
>  
>  // SAFETY: `struct io_pgtable_ops` is not restricted to a single thread.
> -unsafe impl<F: IoPageTableFmt> Send for IoPageTable<F> {}
> +unsafe impl<F: IoPageTableFmt> Send for IoPageTable<'_, F> {}
>  // SAFETY: `struct io_pgtable_ops` may be accessed concurrently.
> -unsafe impl<F: IoPageTableFmt> Sync for IoPageTable<F> {}
> +unsafe impl<F: IoPageTableFmt> Sync for IoPageTable<'_, F> {}
>  
>  /// The format used by this page table.
>  pub trait IoPageTableFmt: 'static {
> @@ -75,25 +75,12 @@ pub trait IoPageTableFmt: 'static {
>      const FORMAT: io_pgtable_fmt;
>  }
>  
> -impl<F: IoPageTableFmt> IoPageTable<F> {
> -    /// Create a new `IoPageTable` as a device resource.
> -    #[inline]
> -    pub fn new(
> -        dev: &Device<Bound>,
> -        config: Config,
> -    ) -> impl PinInit<Devres<IoPageTable<F>>, Error> + '_ {
> -        // SAFETY: Devres ensures that the value is dropped during device unbind.
> -        Devres::new(dev, unsafe { Self::new_raw(dev, config) })
> -    }
> +impl<'bound, F: IoPageTableFmt> IoPageTable<'bound, F> {
> +    // TODO: Create a new `IoPageTable` as a device resource when DevresLt is available.
>  
>      /// Create a new `IoPageTable`.
> -    ///
> -    /// # Safety
> -    ///
> -    /// If successful, then the returned `IoPageTable` must be dropped before the device is
> -    /// unbound.
>      #[inline]
> -    pub unsafe fn new_raw(dev: &Device<Bound>, config: Config) -> Result<IoPageTable<F>> {
> +    pub fn new_raw(dev: &'bound Device<Bound>, config: Config) -> Result<IoPageTable<'bound, F>> {

The name new_raw() indicates that this is something you only use in rare
cases when you are doing weird stuff. But with this change, I no longer
think that's the case.

I would suggest renaming this method to just `new()`.

As for creating a device resoure version with DevresLt, I think that can
be the secondary constructor and be called `new_devres()` or similar.

Otherwise LGTM.

Alice