gpu: nova-core: minor NovaCoreApi cleanup
Subsystems (5)
Danilo Krummrich
maintainerPATCH v2 1/2 gpu: nova-core: minor NovaCoreApi cleanup
Add a constructor for NovaCoreApi and use it when registering the auxiliary device, allowing the gpu and pdev fields to become private.
While at it, rename the lifetime from 'bound to 'api to reflect the API handle's scope; 'bound should only be used by a driver's bus device private data.
Signed-off-by Danilo Krummrich <[email protected]>
---
Changes in v2: new in this revision
--- drivers/gpu/nova-core/api.rs | 13 +++++++++---- drivers/gpu/nova-core/driver.rs | 2 +- 2 files changed, 10 insertions(+), 5 deletions(-)
drivers/gpu/nova-core/api.rs +9 -4
@@ -22,12 +22,17 @@ };/// API handle for the auxiliary bus child drivers to interact with nova-core. -pub struct NovaCoreApi <'bound> { -pub (crate ) gpu:Pin <&'boundGpu <'bound>>, -pub (crate ) pdev: &'bound pci::Device <Bound >, +pub struct NovaCoreApi <'api> { + gpu:Pin <&'apiGpu <'api>>, + pdev: &'api pci::Device <Bound >, } -impl NovaCoreApi <'_> { +impl <'api>NovaCoreApi <'api> { +/// Creates an API handle. +pub (crate )fn new (pdev: &'api pci::Device <Bound >, gpu:Pin <&'apiGpu <'api>>) ->Self { +Self { gpu, pdev } + } +/// Obtain a [`NovaCoreApi`] handle from an auxiliary device registered /// by nova-core. pub fn of (adev: &auxiliary::Device <Bound >) ->Result <NovaCoreApiHandle <'_>> {
drivers/gpu/nova-core/driver.rs +1 -1
@@ -164,7 +164,7 @@ fn probe<'bound>(// never recycles IDs. AUXILIARY_ID_COUNTER.fetch_add(1, Relaxed), crate::MODULE_NAME, - NovaCoreApi { gpu, pdev }, + NovaCoreApi::new(pdev, gpu), )? } },
base-commit: 6e557330b874b0e1fdd03e8fe4c438f36c93eefa
Signature · 1 line
2.56.0
Danilo Krummrich
maintainerPATCH v2 2/2 gpu: nova-core: use pin-init self-referential support
Express all self-referential borrows through pin-init's self-referential support [1], replacing unsafely derived references.
Cc Gary Guo <[email protected]>
Signed-off-by Danilo Krummrich <[email protected]>
---
Changes in v2:
- Make Cmdq lifetime covariant using existential lifetimes.
--- drivers/gpu/nova-core/driver.rs | 53 +++++++++------------------ drivers/gpu/nova-core/gpu.rs | 24 ++++-------- drivers/gpu/nova-core/gsp/cmdq.rs | 16 ++++++-- drivers/gpu/nova-core/gsp/cmdq/gmc.rs | 9 +++-- drivers/gpu/nova-core/gsp/cmdq/rpc.rs | 11 +++--- 5 files changed, 50 insertions(+), 63 deletions(-)
drivers/gpu/nova-core/driver.rs +18 -35
@@ -33,9 +33,11 @@#[pin_data] pub (crate )struct NovaCore <'bound> {#[allow(clippy::type_complexity)] - _reg: auxiliary::Registration <'bound,ForLt! (NovaCoreApi <'_>)>, +#[uses('gpu: invariant)] + _reg: auxiliary::Registration <'gpu,ForLt! (NovaCoreApi <'_>)>,#[pin] -pub (crate ) gpu:Gpu <'bound>, +#[uses('bar1)] +pub (crate ) gpu:Gpu <'bar>, bar: pci::Bar <'bound,BAR0_SIZE >, bar1: pci::Bar <'bound>, }
@@ -129,44 +131,25 @@ fn probe<'bound>(crate ::irq::doorbell_test::run_selftest (pdev, bar, spec.chipset)?; }, -// TODO: Use self-referential pin-init syntax once available. - gpu <- Gpu::new( - pdev, - // SAFETY: `bar` is initialized above, pinned, and outlives `gpu`. - unsafe { &*core::ptr::from_ref(bar) }, - // SAFETY: `bar1` is initialized above, pinned, and outlives `gpu`. - unsafe { &*core::ptr::from_ref(bar1) }, - ).pin_chain(|_gpu| { + gpu <- Gpu::new(pdev, bar, bar1).pin_chain(|_gpu| { #[cfg(CONFIG_NOVA_CORE_SELFTESTS)] _gpu.run_selftests(pdev); Ok(()) }), - _reg: { -// TODO: Use `&gpu` self-referential pin-init syntax once available. -// -// SAFETY: `gpu` is initialized before this expression is evaluated -// (`try_pin_init!()` initializes fields in initializer order), lives at -// a pinned stable address, and is dropped after `_reg` (struct field -// drop order). -let gpu =unsafe { -Pin ::new_unchecked (&*core::ptr::from_ref (gpu.as_ref ().get_ref ())) - }; - -// SAFETY: `NovaCore` is dropped when the device is unbound; -// i.e. `mem::forget()` is never called on it. -unsafe { - auxiliary::Registration ::new_with_lt ( - pdev.as_ref (), -c"nova-drm" , -// TODO[XARR]: Use XArray or perhaps IDA for proper ID -// allocation/recycling. For now, use a simple atomic counter that -// never recycles IDs. -AUXILIARY_ID_COUNTER .fetch_add (1 ,Relaxed ), -crate ::MODULE_NAME , -NovaCoreApi ::new (pdev, gpu), - )? - } +// SAFETY: `NovaCore` is dropped when the device is unbound; i.e. `mem::forget()` is +// never called on it. + _reg: unsafe { + auxiliary::Registration ::new_with_lt ( + pdev.as_ref (), +c"nova-drm" , +// TODO[XARR]: Use XArray or perhaps IDA for proper ID +// allocation/recycling. For now, use a simple atomic counter that +// never recycles IDs. +AUXILIARY_ID_COUNTER .fetch_add (1 ,Relaxed ), +crate ::MODULE_NAME , +NovaCoreApi ::new (pdev, gpu), + )? }, }) }
drivers/gpu/nova-core/gpu.rs +8 -16
@@ -383,7 +383,8 @@ pub(crate) struct Gpu<'gpu> {/// any in-flight run of it has finished, before the command queue that it drains and the /// falcon that it reads are freed, and before the GSP is unloaded. #[pin] - _gsp_subtree: GspSubtree<'gpu>, +#[uses('tree)] + _gsp_subtree: GspSubtree<'gsp_resources>,/// Static GPU information as provided by the GSP. pub(crate) gsp_static_info: GetGspStaticInfoReply,/// GPU memory manager owning memory management resources.
@@ -403,9 +404,6 @@ pub(crate) struct Gpu<'gpu> {/// Must be kept declared *after* `gsp_resources`, as the latter's `PinnedDrop` implementation /// requires the sysmem flush page to be in place. sysmem_flush: SysmemFlush<'gpu>, -/// Borrow of `tree` that `_gsp_subtree` holds. A field that borrows a sibling field is -/// self-referential, which `pin_init` cannot express, so the borrow is taken by hand. - tree_ref: &'gpu Tree<'gpu>,/// The GIN CPU interrupt tree and the PCI vectors that deliver it. /// /// Must be kept declared *after* `_gsp_subtree`, which holds a borrow of it.
@@ -456,10 +454,6 @@ pub(crate) fn new<'a>( tree: Tree::new(pdev, bar, spec.chipset, irq::gsp::GSP_SUBTREE.into())?, -// SAFETY: `tree` is initialized above, is pinned at a stable address, and is dropped -// after every field that uses `tree_ref` (struct field drop order). - tree_ref: unsafe { &*core::ptr::from_ref(tree.as_ref().get_ref()) }, - _: {let dma_mask = hal::gpu_hal (spec.chipset).dma_mask ();
@@ -513,19 +507,17 @@ pub(crate) fn new<'a>( }), _: { - irq::gsp::quiesce (tree_ref, &gsp_resources.gsp_falcon); + irq::gsp::quiesce (tree.get_ref (), &gsp_resources.gsp_falcon); }, -// SAFETY: the GSP falcon and the command queue are fields of `gsp_resources`, which -// is initialized above and pinned, so both references outlive the registration. The -// registration is a field of `Gpu` and is never leaked, so its `Drop` runs, and field -// drop order runs it before either is freed. +// SAFETY: The registration is owned by `Gpu`, which is dropped when the device is +// unbound, so the registration's `Drop` runs. _gsp_subtree <-unsafe {GspSubtree ::new ( pdev, - tree_ref, - &*core::ptr::from_ref (&gsp_resources.gsp_falcon), - &*core::ptr::from_ref (&gsp_resources.gsp.cmdq), + tree.get_ref (), + &gsp_resources.get_ref ().gsp_falcon, + &gsp_resources.get_ref ().gsp.cmdq, ) },
drivers/gpu/nova-core/gsp/cmdq.rs +13 -3
@@ -451,10 +451,15 @@ fn log(&self, dev: &device::Device) {/// Provides the ability to send commands and receive messages from the GSP using a shared memory /// area. #[pin_data] -pub (crate )struct Cmdq <'cmdq> { +pub (crate )struct Cmdq <'cmdq> +where +// Hide the mutex's invariant resource lifetime so `Cmdq` remains covariant. +exists <'gpu>: 'cmdq, +{/// Inner mutex-protected state. #[pin] - inner:Mutex <CmdqInner <'cmdq>>, +#[uses('gpu: invariant)] + inner:Mutex <CmdqInner <'gpu>>,/// DMA address of the command queue's shared memory region. pub (super ) dma_addr:DmaAddress , }
@@ -498,6 +503,11 @@ pub(crate) fn new( }) } +/// Runs `f` with the command queue locked. +fn with_inner_locked <R >(&self, f:impl FnOnce (&mut CmdqInner <'_>) ->R ) ->R { + self.with_inner (|inner|f (&mut inner.lock ())) + } +/// Computes the checksum for the message pointed to by `it`. /// /// A message is made of several parts, so `it` is an iterator over byte slices representing
@@ -522,7 +532,7 @@ fn calculate_checksum<T: Iterator<Item = u8>>(it: T) -> u32 {/// /// `EIO` if a message fails framing or checksum validation. pub (crate )fn drain (&self) ->Result { - self.inner.lock ().drain () + self.with_inner_locked (|inner| inner.drain ()) } }
drivers/gpu/nova-core/gsp/cmdq/gmc.rs +5 -4
@@ -302,10 +302,11 @@ pub(crate) fn send_gmc_command<C>( Error: From<C::InitError>, Error: From<<C::Reply as MessageFromGsp>::InitError>, { -let mut inner = self.inner.lock (); -let seq = inner.send_gmc_command (command)?; + self.with_inner_locked (|inner| { +let seq = inner.send_gmc_command (command)?; - inner.await_gmc_reply ::<C >(seq, on_event) + inner.await_gmc_reply ::<C >(seq, on_event) + }) }/// Sends `command` to the GSP without waiting for a reply.
@@ -323,7 +324,7 @@ pub(crate) fn send_gmc_command_no_wait<C>(&self, command: C) -> Result C: CommandToGsp<Reply = NoReply>, Error: From<C::InitError>, { - self.inner.lock ().send_gmc_command (command).map (|_| ()) + self.with_inner_locked (|inner| inner.send_gmc_command (command).map (|_| ())) } }
drivers/gpu/nova-core/gsp/cmdq/rpc.rs +6 -5
@@ -273,10 +273,11 @@ pub(crate) fn send_rpc_command<M>(&self, command: M) -> Result<M::Reply> Error: From<M::InitError>, Error: From<<M::Reply as MessageFromGsp>::InitError>, { -let mut inner = self.inner.lock (); - inner.send_rpc_command (command)?; + self.with_inner_locked (|inner| { + inner.send_rpc_command (command)?; - inner.await_rpc_msg () + inner.await_rpc_msg () + }) }/// Sends `command` to the GSP without waiting for a reply.
@@ -293,7 +294,7 @@ pub(crate) fn send_rpc_command_no_wait<M>(&self, command: M) -> Result M: CommandToGsp<Reply = NoReply>, Error: From<M::InitError>, { - self.inner.lock ().send_rpc_command (command) + self.with_inner_locked (|inner| inner.send_rpc_command (command)) }/// Waits for an unsolicited GSP event of type `M`. Events that arrive before it are logged and
@@ -314,7 +315,7 @@ pub(crate) fn await_rpc_msg<M: MessageFromGsp>(&self) -> Result<M>// This allows all error types, including `Infallible`, to be used for `M::InitError`. Error: From<M::InitError>, { - self.inner.lock ().await_rpc_msg () + self.with_inner_locked (|inner| inner.await_rpc_msg ()) } }
Signature · 1 line
2.56.0