On 1/2/26 8:59 PM, Timur Tabi wrote:
> From: Alexandre Courbot <[email protected]>
>
> On Turing and GA100 (i.e. the versions that use Libos v2), GSP-RM insists
> that the 'size' parameter of the LibosMemoryRegionInitArgument struct be
> aligned to 4KB. The logging buffers are already aligned to that size, so
> only the GSP_ARGUMENTS_CACHED struct needs to be adjusted. Make that
> adjustment by adding padding to the end of the struct.
>
> Signed-off-by: Alexandre Courbot <[email protected]>
> Signed-off-by: Timur Tabi <[email protected]>
> ---
> drivers/gpu/nova-core/firmware/fwsec.rs | 2 +-
> drivers/gpu/nova-core/gsp.rs | 8 ++++----
> drivers/gpu/nova-core/gsp/fw.rs | 14 +++++++++++++-
> 3 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/firmware/fwsec.rs
> b/drivers/gpu/nova-core/firmware/fwsec.rs
> index 1c1dcdacf5f5..5d2323b07241 100644
> --- a/drivers/gpu/nova-core/firmware/fwsec.rs
> +++ b/drivers/gpu/nova-core/firmware/fwsec.rs
> @@ -433,7 +433,7 @@ pub(crate) fn new(
> };
>
> Ok(FwsecFirmware {
> - desc: desc,
> + desc,
> ucode: ucode_signed,
> })
> }
> diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
> index 766fd9905358..bcf6ce18a4a1 100644
> --- a/drivers/gpu/nova-core/gsp.rs
> +++ b/drivers/gpu/nova-core/gsp.rs
> @@ -27,7 +27,7 @@
> use crate::{
> gsp::cmdq::Cmdq,
> gsp::fw::{
> - GspArgumentsCached,
> + GspArgumentsAligned,
> LibosMemoryRegionInitArgument, //
> },
> num,
> @@ -114,7 +114,7 @@ pub(crate) struct Gsp {
> /// Command queue.
> pub(crate) cmdq: Cmdq,
> /// RM arguments.
> - rmargs: CoherentAllocation<GspArgumentsCached>,
> + rmargs: CoherentAllocation<GspArgumentsAligned>,
> }
>
> impl Gsp {
> @@ -133,7 +133,7 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) ->
> impl PinInit<Self, Error
> logintr: LogBuffer::new(dev)?,
> logrm: LogBuffer::new(dev)?,
> cmdq: Cmdq::new(dev)?,
> - rmargs:
> CoherentAllocation::<GspArgumentsCached>::alloc_coherent(
> + rmargs:
> CoherentAllocation::<GspArgumentsAligned>::alloc_coherent(
> dev,
> 1,
> GFP_KERNEL | __GFP_ZERO,
> @@ -149,7 +149,7 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) ->
> impl PinInit<Self, Error
> libos[1] =
> LibosMemoryRegionInitArgument::new("LOGINTR", &logintr.0)
> )?;
> dma_write!(libos[2] =
> LibosMemoryRegionInitArgument::new("LOGRM", &logrm.0))?;
> - dma_write!(rmargs[0] =
> fw::GspArgumentsCached::new(cmdq))?;
> + dma_write!(rmargs[0].inner =
> fw::GspArgumentsCached::new(cmdq))?;
> dma_write!(libos[3] =
> LibosMemoryRegionInitArgument::new("RMARGS", rmargs))?;
> },
> }))
> diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
> index abffd6beec65..15ca9c183ae1 100644
> --- a/drivers/gpu/nova-core/gsp/fw.rs
> +++ b/drivers/gpu/nova-core/gsp/fw.rs
> @@ -906,9 +906,21 @@ pub(crate) fn new(cmdq: &Cmdq) -> Self {
> // SAFETY: Padding is explicit and will not contain uninitialized data.
> unsafe impl AsBytes for GspArgumentsCached {}
>
> +/// On Turing and GA100, the entries in the `LibosMemoryRegionInitArgument`
> +/// must all be a multiple of GSP_PAGE_SIZE in size, so add padding to force
> it
> +/// to that size.
> +#[repr(C)]
> +pub(crate) struct GspArgumentsAligned {
> + pub(crate) inner: GspArgumentsCached,
> + _padding: [u8; GSP_PAGE_SIZE -
> core::mem::size_of::<bindings::GSP_ARGUMENTS_CACHED>()],
> +}
> +
> +// SAFETY: Padding is explicit and will not contain uninitialized data.
Sort of a pre-existing problem, but "Padding is explicit" seems meaningless
here. And if we're going to write anything here, maybe we should say *why*
we know that the padding can't or won't contain uninitialized data?
Otherwise it all looks good.
> +unsafe impl AsBytes for GspArgumentsAligned {}
> +
> // SAFETY: This struct only contains integer types for which all bit patterns
> // are valid.
> -unsafe impl FromBytes for GspArgumentsCached {}
> +unsafe impl FromBytes for GspArgumentsAligned {}
>
> /// Init arguments for the message queue.
> #[repr(transparent)]
thanks,
--
John Hubbard
