On Tue Mar 3, 2026 at 4:22 PM GMT, Danilo Krummrich wrote: > Convert libos (LibosMemoryRegionInitArgument) and rmargs > (GspArgumentsPadded) to use CoherentInit / Coherent::init() and simplify > the initialization. This also avoids separate initialization on the > stack. > > Signed-off-by: Danilo Krummrich <[email protected]> > --- > drivers/gpu/nova-core/gsp.rs | 47 +++++++++++-------------- > drivers/gpu/nova-core/gsp/fw.rs | 62 +++++++++++++++++++++++---------- > 2 files changed, 65 insertions(+), 44 deletions(-) > > diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs > index 25cd48514c77..cb7f6b4dc0f8 100644 > --- a/drivers/gpu/nova-core/gsp.rs > +++ b/drivers/gpu/nova-core/gsp.rs > @@ -5,10 +5,11 @@ > use kernel::{ > device, > dma::{ > + Coherent, > CoherentAllocation, > + CoherentInit, > DmaAddress, // > }, > - dma_write, > pci, > prelude::*, > transmute::AsBytes, // > @@ -104,7 +105,7 @@ fn new(dev: &device::Device<device::Bound>) -> > Result<Self> { > #[pin_data] > pub(crate) struct Gsp { > /// Libos arguments. > - pub(crate) libos: CoherentAllocation<LibosMemoryRegionInitArgument>, > + pub(crate) libos: Coherent<[LibosMemoryRegionInitArgument]>, > /// Init log buffer. > loginit: LogBuffer, > /// Interrupts log buffer. > @@ -114,7 +115,7 @@ pub(crate) struct Gsp { > /// Command queue. > pub(crate) cmdq: Cmdq, > /// RM arguments. > - rmargs: CoherentAllocation<GspArgumentsPadded>, > + rmargs: Coherent<GspArgumentsPadded>, > } > > impl Gsp { > @@ -123,34 +124,28 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> > impl PinInit<Self, Error > pin_init::pin_init_scope(move || { > let dev = pdev.as_ref(); > > + // Initialise the logging structures. The OpenRM equivalents are > in: > + // _kgspInitLibosLoggingStructures (allocates memory for buffers) > + // kgspSetupLibosInitArgs_IMPL (creates pLibosInitArgs[] array) > Ok(try_pin_init!(Self { > - libos: > CoherentAllocation::<LibosMemoryRegionInitArgument>::alloc_coherent( > - dev, > - GSP_PAGE_SIZE / > size_of::<LibosMemoryRegionInitArgument>(), > - GFP_KERNEL | __GFP_ZERO, > - )?, > loginit: LogBuffer::new(dev)?, > logintr: LogBuffer::new(dev)?, > logrm: LogBuffer::new(dev)?, > cmdq: Cmdq::new(dev)?, > - rmargs: > CoherentAllocation::<GspArgumentsPadded>::alloc_coherent( > - dev, > - 1, > - GFP_KERNEL | __GFP_ZERO, > - )?, > - _: { > - // Initialise the logging structures. The OpenRM > equivalents are in: > - // _kgspInitLibosLoggingStructures (allocates memory for > buffers) > - // kgspSetupLibosInitArgs_IMPL (creates pLibosInitArgs[] > array) > - dma_write!( > - libos, [0]?, > LibosMemoryRegionInitArgument::new("LOGINIT", &loginit.0) > - ); > - dma_write!( > - libos, [1]?, > LibosMemoryRegionInitArgument::new("LOGINTR", &logintr.0) > - ); > - dma_write!(libos, [2]?, > LibosMemoryRegionInitArgument::new("LOGRM", &logrm.0)); > - dma_write!(rmargs, [0]?.inner, > fw::GspArgumentsCached::new(cmdq)); > - dma_write!(libos, [3]?, > LibosMemoryRegionInitArgument::new("RMARGS", rmargs)); > + rmargs: Coherent::init(dev, GFP_KERNEL, > GspArgumentsPadded::new(cmdq))?, > + libos: { > + let mut libos = CoherentInit::zeroed_slice( > + dev, > + GSP_PAGE_SIZE / > size_of::<LibosMemoryRegionInitArgument>(), > + GFP_KERNEL, > + )?; > + > + libos.init_at(0, > LibosMemoryRegionInitArgument::new("LOGINIT", &loginit.0))?; > + libos.init_at(1, > LibosMemoryRegionInitArgument::new("LOGINTR", &logintr.0))?; > + libos.init_at(2, > LibosMemoryRegionInitArgument::new("LOGRM", &logrm.0))?; > + libos.init_at(3, > LibosMemoryRegionInitArgument::new("RMARGS", rmargs))?; > + > + libos.into() > }, > })) > }) > diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs > index 751d5447214d..59cb03a9b238 100644 > --- a/drivers/gpu/nova-core/gsp/fw.rs > +++ b/drivers/gpu/nova-core/gsp/fw.rs > @@ -9,11 +9,12 @@ > use core::ops::Range; > > use kernel::{ > - dma::CoherentAllocation, > + dma::Coherent, > prelude::*, > ptr::{ > Alignable, > - Alignment, // > + Alignment, > + KnownSize, // > }, > sizes::{ > SZ_128K, > @@ -568,7 +569,9 @@ unsafe impl AsBytes for RunCpuSequencer {} > /// The memory allocated for the arguments must remain until the GSP sends > the > /// init_done RPC. > #[repr(transparent)] > -pub(crate) struct > LibosMemoryRegionInitArgument(bindings::LibosMemoryRegionInitArgument); > +pub(crate) struct LibosMemoryRegionInitArgument { > + inner: bindings::LibosMemoryRegionInitArgument, > +}
FYI tuple struct construction is being worked at now: https://github.com/Rust-for-Linux/pin-init/pull/113 > > // SAFETY: Padding is explicit and does not contain uninitialized data. > unsafe impl AsBytes for LibosMemoryRegionInitArgument {} > @@ -578,10 +581,10 @@ unsafe impl AsBytes for LibosMemoryRegionInitArgument {} > unsafe impl FromBytes for LibosMemoryRegionInitArgument {} > > impl LibosMemoryRegionInitArgument { > - pub(crate) fn new<A: AsBytes + FromBytes>( > + pub(crate) fn new<'a, A: AsBytes + FromBytes + KnownSize + ?Sized>( > name: &'static str, > - obj: &CoherentAllocation<A>, > - ) -> Self { > + obj: &'a Coherent<A>, > + ) -> impl Init<Self> + 'a { > /// Generates the `ID8` identifier required for some GSP objects. > fn id8(name: &str) -> u64 { > let mut bytes = [0u8; core::mem::size_of::<u64>()]; > @@ -593,7 +596,8 @@ fn id8(name: &str) -> u64 { > u64::from_ne_bytes(bytes) > } > > - Self(bindings::LibosMemoryRegionInitArgument { > + #[allow(non_snake_case)] > + let init_inner = init!(bindings::LibosMemoryRegionInitArgument { > id8: id8(name), > pa: obj.dma_handle(), > size: num::usize_as_u64(obj.size()), > @@ -603,7 +607,11 @@ fn id8(name: &str) -> u64 { > loc: num::u32_into_u8::< > { > bindings::LibosMemoryRegionLoc_LIBOS_MEMORY_REGION_LOC_SYSMEM }, > >(), > - ..Default::default() > + ..Zeroable::init_zeroed() > + }); > + > + init!(LibosMemoryRegionInitArgument { > + inner <- init_inner, > }) > } > } > @@ -814,15 +822,23 @@ unsafe impl FromBytes for GspMsgElement {} > > /// Arguments for GSP startup. > #[repr(transparent)] > -pub(crate) struct GspArgumentsCached(bindings::GSP_ARGUMENTS_CACHED); > +#[derive(Zeroable)] > +pub(crate) struct GspArgumentsCached { > + inner: bindings::GSP_ARGUMENTS_CACHED, > +} > > impl GspArgumentsCached { > /// Creates the arguments for starting the GSP up using `cmdq` as its > command queue. > - pub(crate) fn new(cmdq: &Cmdq) -> Self { > - Self(bindings::GSP_ARGUMENTS_CACHED { > - messageQueueInitArguments: > MessageQueueInitArguments::new(cmdq).0, > + pub(crate) fn new(cmdq: &Cmdq) -> impl Init<Self> + '_ { This struct is not that big, so I think we can just keep the by-value constructor. Best, Gary > + #[allow(non_snake_case)] > + let init_inner = init!(bindings::GSP_ARGUMENTS_CACHED { > + messageQueueInitArguments <- > MessageQueueInitArguments::new(cmdq), > bDmemStack: 1, > - ..Default::default() > + ..Zeroable::init_zeroed() > + }); > + > + init!(GspArgumentsCached { > + inner <- init_inner, > }) > } > } > @@ -834,11 +850,21 @@ unsafe impl AsBytes for GspArgumentsCached {} > /// must all be a multiple of GSP_PAGE_SIZE in size, so add padding to force > it > /// to that size. > #[repr(C)] > +#[derive(Zeroable)] > pub(crate) struct GspArgumentsPadded { > pub(crate) inner: GspArgumentsCached, > _padding: [u8; GSP_PAGE_SIZE - > core::mem::size_of::<bindings::GSP_ARGUMENTS_CACHED>()], > } > > +impl GspArgumentsPadded { > + pub(crate) fn new(cmdq: &Cmdq) -> impl Init<Self> + '_ { > + init!(GspArgumentsPadded { > + inner <- GspArgumentsCached::new(cmdq), > + ..Zeroable::init_zeroed() > + }) > + } > +} > + > // SAFETY: Padding is explicit and will not contain uninitialized data. > unsafe impl AsBytes for GspArgumentsPadded {} > > @@ -847,18 +873,18 @@ unsafe impl AsBytes for GspArgumentsPadded {} > unsafe impl FromBytes for GspArgumentsPadded {} > > /// Init arguments for the message queue. > -#[repr(transparent)] > -struct MessageQueueInitArguments(bindings::MESSAGE_QUEUE_INIT_ARGUMENTS); > +type MessageQueueInitArguments = bindings::MESSAGE_QUEUE_INIT_ARGUMENTS; > > impl MessageQueueInitArguments { > /// Creates a new init arguments structure for `cmdq`. > - fn new(cmdq: &Cmdq) -> Self { > - Self(bindings::MESSAGE_QUEUE_INIT_ARGUMENTS { > + #[allow(non_snake_case)] > + fn new(cmdq: &Cmdq) -> impl Init<Self> + '_ { > + init!(MessageQueueInitArguments { > sharedMemPhysAddr: cmdq.dma_handle(), > pageTableEntryCount: num::usize_into_u32::<{ Cmdq::NUM_PTES }>(), > cmdQueueOffset: num::usize_as_u64(Cmdq::CMDQ_OFFSET), > statQueueOffset: num::usize_as_u64(Cmdq::STATQ_OFFSET), > - ..Default::default() > + ..Zeroable::init_zeroed() > }) > } > }
