Re: [PATCH v5 5/9] rust: pci: drop driver data before remove returns

["Danilo Krummrich" <dakr@kernel.org>]

On Fri Oct 30, 2026 at 6:12 PM CET, Simon Song wrote:
> From: Zhi Wang <zhiw@nvidia.com>
>
> Drop PCI private data before the remove callback returns, so its
> destructors run within PCI core's removal context.
>
> Call unbind while private data remains installed, then take and release
> the data. Taking it clears drvdata, so post_unbind does not release it
> again. All borrows must end before the data is removed.

This should explain the reason this is needed:

VF drivers will store a vfio::pci::Registration, which calls
vfio_pci_core_register_device() when created and
vfio_pci_core_unregister_device() when destroyed.

Now, vfio_pci_core_unregister_device() expects to be called from PCI's bus
remove() callback and hence can't be deferred beyond remove() (which also makes
it incompatible with devres, i.e a devm_vfio_pci_core_register_device() helper
wouldn't work either).

> Suggested-by: Danilo Krummrich <dakr@kernel.org>
> Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> Signed-off-by: Simon Song <xinmengs@nvidia.com>
> ---
>  rust/kernel/device.rs | 4 ++--
>  rust/kernel/pci.rs    | 6 ++++++
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
> index 2291d85b684..1f6bb58396e 100644
> --- a/rust/kernel/device.rs
> +++ b/rust/kernel/device.rs
> @@ -243,8 +243,8 @@ impl<Ctx: InternalBoundContext> Device<Ctx> {
>      ///
>      /// # Safety
>      ///
> -    /// - Must only be called after a preceding call to [`Device::set_drvdata`] and before the
> -    ///   device is fully unbound.
> +    /// - The data stored by [`Device::set_drvdata`] must still be installed and remain valid
> +    ///   for the returned borrow.

This is what the original comment should have expressed better regardless of
this change. Can you please make this a separate commit?

>      /// - The type `T` must match the type of the `ForeignOwnable` previously stored by
>      ///   [`Device::set_drvdata`].
>      pub unsafe fn drvdata_borrow<T>(&self) -> Pin<&T> {
> diff --git a/rust/kernel/pci.rs b/rust/kernel/pci.rs
> index 9e1d02588af..8092f56ee2b 100644
> --- a/rust/kernel/pci.rs
> +++ b/rust/kernel/pci.rs
> @@ -150,6 +150,12 @@ extern "C" fn remove_callback(pdev: *mut bindings::pci_dev) {
>          let data = unsafe { pdev.as_ref().drvdata_borrow::<T::Data<'_>>() };
>  
>          T::unbind(pdev, data);
> +
> +        // SAFETY: The driver's unbind callback has returned, and no callbacks retain a borrow.
> +        let data = unsafe { pdev.as_ref().drvdata_obtain::<T::Data<'_>>() };
> +
> +        // Drop private data before returning to PCI core, while its removal context is valid.

Please add a brief note similar to the above rationale, as I consider this a
workaround (or special case) for VFIO-PCI.

In general, objects stored in the driver's bus device private data should *not*
make assumptions about implementation details of the underlying bus and
therefore require to be dropped from the underlying bus's remove() callback.

Any point between after the driver's remove() callback and before
devres_release_all() in device_unbind_cleanup() is perfectly sufficient.

> +        drop(data);
>      }
>  }
>  
> -- 
> 2.43.7