On Sat Aug 30, 2025 at 10:32 PM JST, Danilo Krummrich wrote: > Now that we have pci::Device::unbind() we can unregister the sysmem > flush page with a direct access the I/O resource, i.e. without RCU read > side critical section. > > Signed-off-by: Danilo Krummrich <[email protected]> > --- > drivers/gpu/nova-core/driver.rs | 4 ++++ > drivers/gpu/nova-core/gpu.rs | 20 ++++++++++---------- > 2 files changed, 14 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs > index 274989ea1fb4..02514e1e2529 100644 > --- a/drivers/gpu/nova-core/driver.rs > +++ b/drivers/gpu/nova-core/driver.rs > @@ -54,4 +54,8 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> > Result<Pin<KBox<Self > > Ok(this) > } > + > + fn unbind(pdev: &pci::Device<Core>, this: Pin<&Self>) { > + this.gpu.unbind(pdev); > + } > } > diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs > index 8caecaf7dfb4..2db9afdc6087 100644 > --- a/drivers/gpu/nova-core/gpu.rs > +++ b/drivers/gpu/nova-core/gpu.rs > @@ -163,7 +163,7 @@ fn new(bar: &Bar0) -> Result<Spec> { > } > > /// Structure holding the resources required to operate the GPU. > -#[pin_data(PinnedDrop)] > +#[pin_data] > pub(crate) struct Gpu { > spec: Spec, > /// MMIO mapping of PCI BAR 0 > @@ -174,15 +174,6 @@ pub(crate) struct Gpu { > sysmem_flush: SysmemFlush, > } > > -#[pinned_drop] > -impl PinnedDrop for Gpu { > - fn drop(self: Pin<&mut Self>) { > - // Unregister the sysmem flush page before we release it. > - self.bar > - .try_access_with(|b| self.sysmem_flush.unregister(b)); > - } > -} > - > impl Gpu { > /// Helper function to load and run the FWSEC-FRTS firmware and confirm > that it has properly > /// created the WPR2 region. > @@ -309,4 +300,13 @@ pub(crate) fn new( > sysmem_flush, > })) > } > + > + pub(crate) fn unbind(&self, pdev: &pci::Device<device::Bound>) { > + // Unregister the sysmem flush page before we release it. > + kernel::warn_on!(self.bar.access(pdev.as_ref()).map_or(true, |bar| { > + self.sysmem_flush.unregister(bar); > + > + false > + })); > + }
Maybe I'm overtly cautious, but this method can be called from a large part of the driver, leaving the Gpu device in a half-unbound state. The `PinnedDrop` approach had the benefit of not allowing this. One way to solve the problem would be to make this method `pub(in crate::driver)`, so other modules cannot call it.
