gpu: nova: adopt driver lifetime infrastructure
Subsystems (5)
Danilo Krummrich
maintainercover gpu: nova: adopt driver lifetime infrastructure
Adopt the driver core lifetime infrastructure for nova.
Use the lifetime-bound pci::Bar directly in NovaCore, eliminating the Arc<Devres<Bar0>> indirection. This lets SysmemFlush borrow the Bar and implement Drop for automatic cleanup.
Replace ARef<Device> with plain borrows in SysmemFlush and the GSP sequencer, where the structs are already lifetime-parameterized.
Separate the driver type from the driver data to allow the private data to be lifetime-parameterized via the Data GAT.
This patch series is based on [1] and drm-rust-next.
[1] https://lore.kernel.org/driver-core/[email protected]/
Danilo Krummrich (5):
gpu: nova-core: use lifetime for Bar
gpu: nova-core: unregister sysmem flush page from Drop
gpu: nova-core: replace ARef<Device> with &'bound Device in
SysmemFlush
gpu: nova-core: gsp: replace ARef<Device> with &'a Device in sequencer
gpu: nova: separate driver type from driver data
drivers/gpu/drm/nova/driver.rs | 12 ++++---- drivers/gpu/nova-core/driver.rs | 32 +++++++++------------- drivers/gpu/nova-core/fb.rs | 31 ++++++++++----------- drivers/gpu/nova-core/gpu.rs | 38 ++++++++------------------ drivers/gpu/nova-core/gsp/boot.rs | 2 +- drivers/gpu/nova-core/gsp/sequencer.rs | 11 ++++---- 6 files changed, 52 insertions(+), 74 deletions(-)
Signature · 1 line
2.54.0
Danilo Krummrich
maintainerPATCH 1/5 gpu: nova-core: use lifetime for Bar
Take advantage of the lifetime-parameterized pci::Bar<'bound> to hold the BAR mapping directly in NovaCore<'bound>, and pass a borrowed reference to Gpu<'bound>.
This eliminates the Arc<Devres<Bar0>> indirection, removes runtime revocation checks for BAR access, and simplifies Gpu::unbind().
--- drivers/gpu/nova-core/driver.rs | 32 +++++++++++++++----------------- drivers/gpu/nova-core/gpu.rs | 33 +++++++++++++-------------------- 2 files changed, 28 insertions(+), 37 deletions(-)
drivers/gpu/nova-core/driver.rs +15 -17
@@ -13,12 +13,9 @@ }, prelude::*, sizes::SZ_16M, - sync::{ - atomic::{ - Atomic, - Relaxed, // - }, - Arc, + sync::atomic::{ + Atomic, + Relaxed, // }, types::ForLt, };
@@ -31,7 +28,8 @@#[pin_data] pub (crate )struct NovaCore <'bound> {#[pin] -pub (crate ) gpu:Gpu , +pub (crate ) gpu:Gpu <'bound>, + bar: pci::Bar <'bound,BAR0_SIZE >,#[allow(clippy::type_complexity)] _reg: auxiliary::Registration <'bound,ForLt! (())>, }
@@ -48,7 +46,7 @@ pub(crate) struct NovaCore<'bound> {// DMA addresses. These systems should be quite rare. const GPU_DMA_BITS :u32 =47 ; -pub (crate )type Bar0 = pci::Bar <'static,BAR0_SIZE >; +pub (crate )type Bar0 = kernel::io::Mmio <BAR0_SIZE >; kernel::pci_device_table!(PCI_TABLE ,
@@ -95,14 +93,14 @@ fn probe<'bound>(// other threads of execution. unsafe { pdev.dma_set_mask_and_coherent (DmaMask ::new ::<GPU_DMA_BITS >())? }; -let bar =Arc ::new ( - pdev.iomap_region_sized ::<BAR0_SIZE >(0 ,c"nova-core/bar0" )? - .into_devres ()?, -GFP_KERNEL , - )?; - Ok(try_pin_init!(NovaCore { - gpu <- Gpu::new(pdev, bar.clone(), bar.access(pdev.as_ref())?), + bar: pdev.iomap_region_sized::<BAR0_SIZE>(0, c"nova-core/bar0")?, + // TODO: Use `&bar` self-referential pin-init syntax once available. + // + // SAFETY: `bar` is initialized before this expression is evaluated + // (`try_pin_init!()` initializes fields in declaration order), lives at a pinned + // stable address, and is dropped after `gpu` (struct field drop order). + gpu <- Gpu::new(pdev, unsafe { &*core::ptr::from_ref(bar) }), _reg: auxiliary::Registration::new( pdev.as_ref(), c"nova-drm",
@@ -116,7 +114,7 @@ fn probe<'bound>( }) } -fn unbind <'bound>(pdev: &'bound pci::Device <Core <'_>>, this:Pin <&Self ::Data <'bound>>) { - this.gpu.unbind (pdev.as_ref ()); +fn unbind <'bound>(_pdev: &'bound pci::Device <Core <'_>>, this:Pin <&Self ::Data <'bound>>) { + this.gpu.unbind (); } }
drivers/gpu/nova-core/gpu.rs +13 -20
@@ -2,13 +2,11 @@use kernel::{ device, - devres::Devres , fmt, io::Io , num::Bounded , pci, - prelude::*, - sync::Arc ,// + prelude::*,// };use crate ::{
@@ -246,10 +244,10 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {/// Structure holding the resources required to operate the GPU. #[pin_data] -pub (crate )struct Gpu { +pub (crate )struct Gpu <'gpu> { spec:Spec , -/// MMIO mapping of PCI BAR 0 - bar:Arc <Devres <Bar0 >>, +/// MMIO mapping of PCI BAR 0. + bar: &'gpuBar0 ,/// System memory page required for flushing all pending GPU-side memory writes done through /// PCIE into system memory, via sysmembar (A GPU-initiated HW memory-barrier operation). sysmem_flush:SysmemFlush ,
@@ -262,12 +260,11 @@ pub(crate) struct Gpu { gsp: Gsp, } -impl Gpu { - pub(crate) fn new<'a>( - pdev: &'a pci::Device<device::Bound>, - devres_bar: Arc<Devres<Bar0>>, - bar: &'a Bar0, - ) -> impl PinInit<Self, Error> + 'a { +impl<'gpu> Gpu<'gpu> { + pub(crate) fn new( + pdev: &'gpu pci::Device<device::Bound>, + bar: &'gpu Bar0, + ) -> impl PinInit<Self, Error> + 'gpu { try_pin_init!(Self { spec: Spec::new(pdev.as_ref(), bar).inspect(|spec| { dev_info!(pdev,"NVIDIA ({})\n", spec);
@@ -279,6 +276,8 @@ pub(crate) fn new<'a>( .inspect_err(|_| dev_err!(pdev, "GFW boot did not complete\n"))?; }, + bar, + sysmem_flush: SysmemFlush::register(pdev.as_ref(), bar, spec.chipset)?, gsp_falcon: Falcon::new(
@@ -292,19 +291,13 @@ pub(crate) fn new<'a>( gsp <- Gsp::new(pdev), _: { gsp.boot(pdev, bar, spec.chipset, gsp_falcon, sec2_falcon)? }, - - bar: devres_bar, }) }/// Called when the corresponding [`Device`](device::Device) is unbound. /// /// Note: This method must only be called from `Driver::unbind`. -pub (crate )fn unbind (&self, dev: &device::Device <device::Core <'_>>) { - kernel::warn_on!(self - .bar - .access (dev) - .inspect (|bar| self.sysmem_flush.unregister (bar)) - .is_err ()); +pub (crate )fn unbind (&self) { + self.sysmem_flush.unregister (self.bar); } }
Signature · 1 line
2.54.0
Re: [PATCH 1/5] gpu: nova-core: use lifetime for Bar
Quoted 9 lines · Danilo Krummrich
Take advantage of the lifetime-parameterized pci::Bar<'bound> to hold the BAR mapping directly in NovaCore<'bound>, and pass a borrowed reference to Gpu<'bound>.
This eliminates the Arc<Devres<Bar0>> indirection, removes runtime revocation checks for BAR access, and simplifies
Gpu::unbind().Signed-off-by Danilo Krummrich <[email protected]>---
Danilo Krummrich
maintainerPATCH 2/5 gpu: nova-core: unregister sysmem flush page from Drop
In reply to Danilo KrummrichNow that SysmemFlush can borrow the Bar via HRT lifetime, store a &'bound Bar0 reference and implement Drop to automatically unregister the sysmem flush page. This removes the need for manual unregister() calls and the Gpu::unbind() method.
--- drivers/gpu/nova-core/driver.rs | 4 ---- drivers/gpu/nova-core/fb.rs | 22 ++++++++++------------ drivers/gpu/nova-core/gpu.rs | 9 +-------- 3 files changed, 11 insertions(+), 24 deletions(-)
drivers/gpu/nova-core/driver.rs +0 -4
@@ -113,8 +113,4 @@ fn probe<'bound>( })) }) } - -fn unbind <'bound>(_pdev: &'bound pci::Device <Core <'_>>, this:Pin <&Self ::Data <'bound>>) { - this.gpu.unbind (); - } }
drivers/gpu/nova-core/fb.rs +10 -12
@@ -43,21 +43,20 @@/// Because of this, the sysmem flush memory page must be registered as early as possible during /// driver initialization, and before any falcon is reset. /// -/// Users are responsible for manually calling [`Self::unregister`] before dropping this object, -/// otherwise the GPU might still use it even after it has been freed. -pub (crate )struct SysmemFlush { +pub (crate )struct SysmemFlush <'sys> {/// Chipset we are operating on. chipset:Chipset , device:ARef <device::Device >, + bar: &'sysBar0 ,/// Keep the page alive as long as we need it. page:CoherentHandle , } -impl SysmemFlush { +impl <'sys>SysmemFlush <'sys> {/// Allocate a memory page and register it as the sysmem flush page. pub (crate )fn register ( dev: &device::Device <device::Bound >, - bar: &Bar0 , + bar: &'sysBar0 , chipset:Chipset , ) ->Result <Self > {let page =CoherentHandle ::alloc (dev, kernel::page::PAGE_SIZE ,GFP_KERNEL )?;
@@ -67,19 +66,18 @@ pub(crate) fn register( Ok(Self { chipset, device: dev.into(), + bar, page, }) } +} -/// Unregister the managed sysmem flush page. -/// -/// In order to gracefully tear down the GPU, users must make sure to call this method before -/// dropping the object. - pub(crate) fn unregister(&self, bar: &Bar0) { +impl Drop for SysmemFlush<'_> { + fn drop(&mut self) { let hal = hal::fb_hal(self.chipset); - if hal.read_sysmem_flush_page(bar) == self.page.dma_handle() { - let _ = hal.write_sysmem_flush_page(bar, 0).inspect_err(|e| { + if hal.read_sysmem_flush_page(self.bar) == self.page.dma_handle() { + let _ = hal.write_sysmem_flush_page(self.bar, 0).inspect_err(|e| { dev_warn!( &self.device, "failed to unregister sysmem flush page: {:?}\n",
drivers/gpu/nova-core/gpu.rs +1 -8
@@ -250,7 +250,7 @@ pub(crate) struct Gpu<'gpu> { bar: &'gpu Bar0, /// System memory page required for flushing all pending GPU-side memory writes done through /// PCIE into system memory, via sysmembar (A GPU-initiated HW memory-barrier operation). - sysmem_flush: SysmemFlush, + sysmem_flush: SysmemFlush<'gpu>, /// GSP falcon instance, used for GSP boot up and cleanup. gsp_falcon: Falcon<GspFalcon>,/// SEC2 falcon instance, used for GSP boot up and cleanup.
@@ -293,11 +293,4 @@ pub(crate) fn new( _: { gsp.boot(pdev, bar, spec.chipset, gsp_falcon, sec2_falcon)? }, }) } - -/// Called when the corresponding [`Device`](device::Device) is unbound. -/// -/// Note: This method must only be called from `Driver::unbind`. -pub (crate )fn unbind (&self) { - self.sysmem_flush.unregister (self.bar); - } }
Signature · 1 line
2.54.0
Re: [PATCH 2/5] gpu: nova-core: unregister sysmem flush page from Drop
Quoted 10 lines · Danilo Krummrich
Now that SysmemFlush can borrow the Bar via HRT lifetime, store a &'bound Bar0 reference and implement Drop to automatically unregister the sysmem flush page. This removes the need for manual
unregister()calls and theGpu::unbind()method.Reported-by Eliot Courtney <[email protected]>Fixes 6554ad65b589 ("gpu: nova-core: register sysmem flush page")Signed-off-by Danilo Krummrich <[email protected]>---
Danilo Krummrich
maintainerPATCH 3/5 gpu: nova-core: replace ARef<Device> with &'bound Device in SysmemFlush
In reply to Danilo KrummrichNow that SysmemFlush is lifetime-parameterized, the ARef<Device> is unnecessary -- a plain &'bound Device reference suffices.
--- drivers/gpu/nova-core/fb.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-)
drivers/gpu/nova-core/fb.rs +4 -5
@@ -15,8 +15,7 @@ Alignable, Alignment, // }, - sizes::*, - sync::aref::ARef, // + sizes::*, // };use crate ::{
@@ -46,7 +45,7 @@pub (crate )struct SysmemFlush <'sys> {/// Chipset we are operating on. chipset:Chipset , - device:ARef <device::Device >, + device: &'sys device::Device , bar: &'sysBar0 ,/// Keep the page alive as long as we need it. page:CoherentHandle ,
@@ -55,7 +54,7 @@ pub(crate) struct SysmemFlush<'sys> {impl <'sys>SysmemFlush <'sys> {/// Allocate a memory page and register it as the sysmem flush page. pub (crate )fn register ( - dev: &device::Device <device::Bound >, + dev: &'sys device::Device <device::Bound >, bar: &'sysBar0 , chipset:Chipset , ) ->Result <Self > {
@@ -65,7 +64,7 @@ pub(crate) fn register(Ok (Self { chipset, - device: dev.into (), + device: dev, bar, page, })
Signature · 1 line
2.54.0
Re: [PATCH 3/5] gpu: nova-core: replace ARef<Device> with &'bound Device in SysmemFlush
Quoted 5 lines · Danilo Krummrich
Now that SysmemFlush is lifetime-parameterized, the ARef<Device> is unnecessary -- a plain &'bound Device reference suffices.
Signed-off-by Danilo Krummrich <[email protected]>---
Danilo Krummrich
maintainerPATCH 4/5 gpu: nova-core: gsp: replace ARef<Device> with &'a Device in sequencer
In reply to Danilo KrummrichGspSequencer, GspSeqIter, and GspSequencerParams are already lifetime-parameterized; the ARef<Device> is unnecessary -- a plain &'a Device reference suffices.
--- drivers/gpu/nova-core/gsp/boot.rs | 2 +- drivers/gpu/nova-core/gsp/sequencer.rs | 11 +++++------ 2 files changed, 6 insertions(+), 7 deletions(-)
drivers/gpu/nova-core/gsp/boot.rs +1 -1
@@ -234,7 +234,7 @@ pub(crate) fn boot( libos_dma_handle: libos_handle, gsp_falcon, sec2_falcon, - dev: pdev.as_ref().into(), + dev, bar, };GspSequencer ::run (&self.cmdq, seq_params)?;
drivers/gpu/nova-core/gsp/sequencer.rs +5 -6
@@ -11,7 +11,6 @@ Io, // }, prelude::*, - sync::aref::ARef, time::{ delay::fsleep, Delta, //
@@ -142,7 +141,7 @@ pub(crate) struct GspSequencer<'a> {/// Bootloader application version. bootloader_app_version: u32,/// Device for logging. - dev: ARef<device::Device>, + dev: &'a device::Device, }impl fw::RegWritePayload {
@@ -281,7 +280,7 @@ pub(crate) struct GspSeqIter<'a> {/// Number of commands processed so far. cmds_processed: u32,/// Device for logging. - dev: ARef<device::Device>, + dev: &'a device::Device, }impl <'a>Iterator for GspSeqIter <'a> {
@@ -309,7 +308,7 @@ fn next(&mut self) -> Option<Self::Item> { self.cmd_data.len () - offset }; buffer[..copy_len].copy_from_slice (&self.cmd_data[offset..offset + copy_len]); -let cmd_result =GspSeqCmd ::new (&buffer, &self.dev); +let cmd_result =GspSeqCmd ::new (&buffer, self.dev); cmd_result.map_or_else ( |_err| {
@@ -334,7 +333,7 @@ fn iter(&self) -> GspSeqIter<'_> { current_offset: 0, total_cmds: self.seq_info.cmd_index, cmds_processed: 0, - dev: self.dev.clone(), + dev: self.dev, } } }
@@ -350,7 +349,7 @@ pub(crate) struct GspSequencerParams<'a> {/// SEC2 falcon for core operations. pub(crate) sec2_falcon: &'a Falcon<Sec2>,/// Device for logging. - pub(crate) dev: ARef<device::Device>, + pub(crate) dev: &'a device::Device,/// BAR0 for register access. pub(crate) bar: &'a Bar0, }
Signature · 1 line
2.54.0
Re: [PATCH 4/5] gpu: nova-core: gsp: replace ARef<Device> with &'a Device in sequencer
Quoted 6 lines · Danilo Krummrich
GspSequencer, GspSeqIter, and GspSequencerParams are already lifetime-parameterized; the ARef<Device> is unnecessary -- a plain &'a Device reference suffices.
Signed-off-by Danilo Krummrich <[email protected]>---
Danilo Krummrich
maintainerPATCH 5/5 gpu: nova: separate driver type from driver data
In reply to Danilo KrummrichSplit NovaDriver into a unit struct for trait implementations and a separate Nova struct for the private driver data.
--- drivers/gpu/drm/nova/driver.rs | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
drivers/gpu/drm/nova/driver.rs +7 -5
@@ -15,9 +15,11 @@use crate ::file::File ;use crate ::gem::NovaObject ; -pub (crate )struct NovaDriver { +pub (crate )struct NovaDriver ; + +pub (crate )struct Nova {#[expect(unused)] - drm:ARef <drm::Device <Self >>, + drm:ARef <drm::Device <NovaDriver >>, }/// Convienence type alias for the DRM device type for this driver
@@ -51,19 +53,19 @@ pub(crate) struct NovaData {impl auxiliary::Driver for NovaDriver {type IdInfo = (); -type Data <'bound> =Self ; +type Data <'bound> =Nova ;const ID_TABLE : auxiliary::IdTable <Self ::IdInfo > = &AUX_TABLE ;fn probe <'bound>( adev: &'bound auxiliary::Device <Core <'_>>, _info: &'boundSelf ::IdInfo , - ) ->impl PinInit <Self ,Error > + 'bound { + ) ->impl PinInit <Self ::Data <'bound>,Error > + 'bound {let data =try_pin_init! (NovaData { adev: adev.into () });let drm = drm::Device ::<Self >::new (adev.as_ref (), data)?; drm::Registration ::new_foreign_owned (&drm, adev.as_ref (),0 )?; -Ok (Self { drm }) +Ok (Nova { drm }) } }
Signature · 1 line
2.54.0
Re: [PATCH 5/5] gpu: nova: separate driver type from driver data
Quoted 5 lines · Danilo Krummrich
Split NovaDriver into a unit struct for trait implementations and a separate Nova struct for the private driver data.
Signed-off-by Danilo Krummrich <[email protected]>---
Quoted 9 lines · Danilo Krummrich
Adopt the driver core lifetime infrastructure for nova.
Use the lifetime-bound
pci::Bardirectly in NovaCore, eliminating the Arc<Devres<Bar0>> indirection. This lets SysmemFlush borrow the Bar and implement Drop for automatic cleanup.Replace ARef<Device> with plain borrows in SysmemFlush and the GSP sequencer, where the structs are already lifetime-parameterized.
Quoted 6 lines
Separate the driver type from the driver data to allow the private data to be lifetime-parameterized via the Data GAT.
This patch series is based on [1] and drm-rust-next.
The series,
Quoted 1 line · Danilo Krummrich
[PATCH 0/5] gpu: nova: adopt driver lifetime infrastructure
Applied, thanks!
Branch: drm-rust-next
Tree: https://gitlab.freedesktop.org/drm/rust/kernel.git
[1/5] gpu: nova-core: use lifetime for Bar
commit: 88b70f5a05af
[2/5] gpu: nova-core: unregister sysmem flush page from Drop
commit: d8143c9e988b
[3/5] gpu: nova-core: replace ARef<Device> with &'bound Device in SysmemFlush
commit: 17b6544ec141
[4/5] gpu: nova-core: gsp: replace ARef<Device> with &'a Device in sequencer
commit: 859805f070bb
[5/5] gpu: nova: separate driver type from driver data
commit: dc395c2831b5The patches will appear in the next linux-next integration (typically within 24 hours on weekdays).
The patches are queued up for the upcoming merge window for the next major kernel release.