Convert libos (LibosMemoryRegionInitArgument) and rmargs
(GspArgumentsPadded) to use CoherentBox / Coherent::init() and simplify
the initialization. This also avoids separate initialization on the
stack.
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
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 72f173726f87..f0a50bdc4c00 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,
+ CoherentBox,
DmaAddress, //
},
- dma_write,
pci,
prelude::*,
transmute::AsBytes, //
@@ -106,7 +107,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.
@@ -117,7 +118,7 @@ pub(crate) struct Gsp {
#[pin]
pub(crate) cmdq: Cmdq,
/// RM arguments.
- rmargs: CoherentAllocation<GspArgumentsPadded>,
+ rmargs: Coherent<GspArgumentsPadded>,
}
impl Gsp {
@@ -126,34 +127,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 = CoherentBox::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 4e3bfc6c4c47..0d8daf6a80b7 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,
@@ -648,7 +649,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,
+}
// SAFETY: Padding is explicit and does not contain uninitialized data.
unsafe impl AsBytes for LibosMemoryRegionInitArgument {}
@@ -658,10 +661,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>()];
@@ -673,7 +676,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()),
@@ -683,7 +687,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,
})
}
}
@@ -862,15 +870,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> + '_ {
+ #[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,
})
}
}
@@ -882,11 +898,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 {}
@@ -895,18 +921,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()
})
}
}
--
2.53.0
On Fri Mar 20, 2026 at 7:45 PM GMT, Danilo Krummrich wrote:
> Convert libos (LibosMemoryRegionInitArgument) and rmargs
> (GspArgumentsPadded) to use CoherentBox / Coherent::init() and simplify
> the initialization. This also avoids separate initialization on the
> stack.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
> 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 72f173726f87..f0a50bdc4c00 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,
> + CoherentBox,
> DmaAddress, //
> },
> - dma_write,
> pci,
> prelude::*,
> transmute::AsBytes, //
> @@ -106,7 +107,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.
> @@ -117,7 +118,7 @@ pub(crate) struct Gsp {
> #[pin]
> pub(crate) cmdq: Cmdq,
> /// RM arguments.
> - rmargs: CoherentAllocation<GspArgumentsPadded>,
> + rmargs: Coherent<GspArgumentsPadded>,
> }
>
> impl Gsp {
> @@ -126,34 +127,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 = CoherentBox::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 4e3bfc6c4c47..0d8daf6a80b7 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,
> @@ -648,7 +649,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,
> +}
>
> // SAFETY: Padding is explicit and does not contain uninitialized data.
> unsafe impl AsBytes for LibosMemoryRegionInitArgument {}
> @@ -658,10 +661,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>()];
> @@ -673,7 +676,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()),
> @@ -683,7 +687,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,
> })
> }
> }
> @@ -862,15 +870,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> + '_ {
> + #[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,
> })
> }
> }
> @@ -882,11 +898,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 {}
>
> @@ -895,18 +921,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;
Why is this typedef while the others new type? Because this one is not
`pub(crate)`?
Anyway, just a question.
Reviewed-by: Gary Guo <gary@garyguo.net>
>
> 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()
> })
> }
> }
© 2016 - 2026 Red Hat, Inc.