On Fri Oct 30, 2026 at 6:12 PM CET, Simon Song wrote: > From: Zhi Wang <[email protected]> > > 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 <[email protected]> > Signed-off-by: Zhi Wang <[email protected]> > Signed-off-by: Simon Song <[email protected]> > --- > 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
