Nouveau - Oct 2025 - [PATCH v5 06/14] gpu: nova-core: Add GSP command queue bindings

On Mon Oct 13, 2025 at 3:20 PM JST, Alistair Popple
wrote:
> Add bindings and accessors used for the GSP command queue.
>
> Signed-off-by: Alistair Popple <apopple at nvidia.com>
There are lots of unused warnings when building on this commit. I know
we don't like big patches, but since this is semantically related to the
next one and the two touch different files anyway, how about merging
them? IMHO this is preferable to adding lots of `#[expect(unused)]` that
we are going to remove right after.

If you have been told to split in a previous version, let's just add a
the `#[expect(unused)]`  where needed.
>
> ---
>
> Changes for v5:
>  - Derive the Zeroable trait for structs and enums
>
> Changes for v4:
>  - Don't panic the kernel if trying to initialise a large (> 4GB)
>    message header.
>  - Use `init!` to provide safe and complete initialisers.
>  - Take an enum type instead of a u32 for the function.
>
> Changes for v3:
>  - New for v3
> ---
>  drivers/gpu/nova-core/gsp/fw.rs               | 275 +++++++++
>  .../gpu/nova-core/gsp/fw/r570_144/bindings.rs | 541 ++++++++++++++++++
>  2 files changed, 816 insertions(+)
>
> diff --git a/drivers/gpu/nova-core/gsp/fw.rs
b/drivers/gpu/nova-core/gsp/fw.rs
> index 1cc992ca492c..a2ce570ddfaf 100644
> --- a/drivers/gpu/nova-core/gsp/fw.rs
> +++ b/drivers/gpu/nova-core/gsp/fw.rs
> @@ -5,6 +5,7 @@
>  // Alias to avoid repeating the version number with every use.
>  use r570_144 as bindings;
>  
> +use core::fmt;
>  use core::ops::Range;
>  
>  use kernel::dma::CoherentAllocation;
> @@ -16,6 +17,7 @@
>  use crate::firmware::gsp::GspFirmware;
>  use crate::gpu::Chipset;
>  use crate::gsp::FbLayout;
> +use crate::gsp::GSP_PAGE_SIZE;
>  
>  /// Dummy type to group methods related to heap parameters for running the
GSP firmware.
>  pub(crate) struct GspFwHeapParams(());
> @@ -158,6 +160,120 @@ pub(crate) fn new(gsp_firmware: &GspFirmware,
fb_layout: &FbLayout) -> Self {
>      }
>  }
>  
> +#[derive(PartialEq)]
> +pub(crate) enum MsgFunction {
> +    // Common function codes
> +    Nop = bindings::NV_VGPU_MSG_FUNCTION_NOP as isize,
> +    SetGuestSystemInfo =
bindings::NV_VGPU_MSG_FUNCTION_SET_GUEST_SYSTEM_INFO as isize,
> +    AllocRoot = bindings::NV_VGPU_MSG_FUNCTION_ALLOC_ROOT as isize,
> +    AllocDevice = bindings::NV_VGPU_MSG_FUNCTION_ALLOC_DEVICE as isize,
> +    AllocMemory = bindings::NV_VGPU_MSG_FUNCTION_ALLOC_MEMORY as isize,
> +    AllocCtxDma = bindings::NV_VGPU_MSG_FUNCTION_ALLOC_CTX_DMA as isize,
> +    AllocChannelDma = bindings::NV_VGPU_MSG_FUNCTION_ALLOC_CHANNEL_DMA as
isize,
> +    MapMemory = bindings::NV_VGPU_MSG_FUNCTION_MAP_MEMORY as isize,
> +    BindCtxDma = bindings::NV_VGPU_MSG_FUNCTION_BIND_CTX_DMA as isize,
> +    AllocObject = bindings::NV_VGPU_MSG_FUNCTION_ALLOC_OBJECT as isize,
> +    Free = bindings::NV_VGPU_MSG_FUNCTION_FREE as isize,
> +    Log = bindings::NV_VGPU_MSG_FUNCTION_LOG as isize,
> +    GetGspStaticInfo = bindings::NV_VGPU_MSG_FUNCTION_GET_GSP_STATIC_INFO
as isize,
> +    SetRegistry = bindings::NV_VGPU_MSG_FUNCTION_SET_REGISTRY as isize,
> +    GspSetSystemInfo = bindings::NV_VGPU_MSG_FUNCTION_GSP_SET_SYSTEM_INFO
as isize,
> +    GspInitPostObjGpu =
bindings::NV_VGPU_MSG_FUNCTION_GSP_INIT_POST_OBJGPU as isize,
> +    GspRmControl = bindings::NV_VGPU_MSG_FUNCTION_GSP_RM_CONTROL as isize,
> +    GetStaticInfo = bindings::NV_VGPU_MSG_FUNCTION_GET_STATIC_INFO as
isize,
> +
> +    // Event codes
> +    GspInitDone = bindings::NV_VGPU_MSG_EVENT_GSP_INIT_DONE as isize,
> +    GspRunCpuSequencer = bindings::NV_VGPU_MSG_EVENT_GSP_RUN_CPU_SEQUENCER
as isize,
> +    PostEvent = bindings::NV_VGPU_MSG_EVENT_POST_EVENT as isize,
> +    RcTriggered = bindings::NV_VGPU_MSG_EVENT_RC_TRIGGERED as isize,
> +    MmuFaultQueued = bindings::NV_VGPU_MSG_EVENT_MMU_FAULT_QUEUED as
isize,
> +    OsErrorLog = bindings::NV_VGPU_MSG_EVENT_OS_ERROR_LOG as isize,
> +    GspPostNoCat = bindings::NV_VGPU_MSG_EVENT_GSP_POST_NOCAT_RECORD as
isize,
> +    GspLockdownNotice = bindings::NV_VGPU_MSG_EVENT_GSP_LOCKDOWN_NOTICE as
isize,
> +    UcodeLibOsPrint = bindings::NV_VGPU_MSG_EVENT_UCODE_LIBOS_PRINT as
isize,
> +}
Can we split functions and events into distinct types? Their bindings
even have different prefixes.

Also, rather than casting these all as `isize`, I'd suggest making the
type `#[repr(u32)]`. We don't need negative values here.
> +
> +impl fmt::Display for MsgFunction {
> +    fn fmt(&self, f: &mut fmt::Formatter<'_>) ->
fmt::Result {
> +        match self {
> +            // Common function codes
> +            MsgFunction::Nop => write!(f, "NOP"),
> +            MsgFunction::SetGuestSystemInfo => write!(f,
"SET_GUEST_SYSTEM_INFO"),
> +            MsgFunction::AllocRoot => write!(f,
"ALLOC_ROOT"),
> +            MsgFunction::AllocDevice => write!(f,
"ALLOC_DEVICE"),
> +            MsgFunction::AllocMemory => write!(f,
"ALLOC_MEMORY"),
> +            MsgFunction::AllocCtxDma => write!(f,
"ALLOC_CTX_DMA"),
> +            MsgFunction::AllocChannelDma => write!(f,
"ALLOC_CHANNEL_DMA"),
> +            MsgFunction::MapMemory => write!(f,
"MAP_MEMORY"),
> +            MsgFunction::BindCtxDma => write!(f,
"BIND_CTX_DMA"),
> +            MsgFunction::AllocObject => write!(f,
"ALLOC_OBJECT"),
> +            MsgFunction::Free => write!(f, "FREE"),
> +            MsgFunction::Log => write!(f, "LOG"),
> +            MsgFunction::GetGspStaticInfo => write!(f,
"GET_GSP_STATIC_INFO"),
> +            MsgFunction::SetRegistry => write!(f,
"SET_REGISTRY"),
> +            MsgFunction::GspSetSystemInfo => write!(f,
"GSP_SET_SYSTEM_INFO"),
> +            MsgFunction::GspInitPostObjGpu => write!(f,
"GSP_INIT_POST_OBJGPU"),
> +            MsgFunction::GspRmControl => write!(f,
"GSP_RM_CONTROL"),
> +            MsgFunction::GetStaticInfo => write!(f,
"GET_STATIC_INFO"),
> +
> +            // Event codes
> +            MsgFunction::GspInitDone => write!(f,
"INIT_DONE"),
> +            MsgFunction::GspRunCpuSequencer => write!(f,
"RUN_CPU_SEQUENCER"),
> +            MsgFunction::PostEvent => write!(f,
"POST_EVENT"),
> +            MsgFunction::RcTriggered => write!(f,
"RC_TRIGGERED"),
> +            MsgFunction::MmuFaultQueued => write!(f,
"MMU_FAULT_QUEUED"),
> +            MsgFunction::OsErrorLog => write!(f,
"OS_ERROR_LOG"),
> +            MsgFunction::GspPostNoCat => write!(f, "NOCAT"),
> +            MsgFunction::GspLockdownNotice => write!(f,
"LOCKDOWN_NOTICE"),
> +            MsgFunction::UcodeLibOsPrint => write!(f,
"LIBOS_PRINT"),
> +        }
> +    }
> +}
> +
> +impl TryFrom<u32> for MsgFunction {
> +    type Error = kernel::error::Error;
> +
> +    fn try_from(value: u32) -> Result<MsgFunction> {
> +        match value {
> +            bindings::NV_VGPU_MSG_FUNCTION_NOP => Ok(MsgFunction::Nop),
> +            bindings::NV_VGPU_MSG_FUNCTION_SET_GUEST_SYSTEM_INFO => {
> +                Ok(MsgFunction::SetGuestSystemInfo)
> +            }
> +            bindings::NV_VGPU_MSG_FUNCTION_ALLOC_ROOT =>
Ok(MsgFunction::AllocRoot),
> +            bindings::NV_VGPU_MSG_FUNCTION_ALLOC_DEVICE =>
Ok(MsgFunction::AllocDevice),
> +            bindings::NV_VGPU_MSG_FUNCTION_ALLOC_MEMORY =>
Ok(MsgFunction::AllocMemory),
> +            bindings::NV_VGPU_MSG_FUNCTION_ALLOC_CTX_DMA =>
Ok(MsgFunction::AllocCtxDma),
> +            bindings::NV_VGPU_MSG_FUNCTION_ALLOC_CHANNEL_DMA =>
Ok(MsgFunction::AllocChannelDma),
> +            bindings::NV_VGPU_MSG_FUNCTION_MAP_MEMORY =>
Ok(MsgFunction::MapMemory),
> +            bindings::NV_VGPU_MSG_FUNCTION_BIND_CTX_DMA =>
Ok(MsgFunction::BindCtxDma),
> +            bindings::NV_VGPU_MSG_FUNCTION_ALLOC_OBJECT =>
Ok(MsgFunction::AllocObject),
> +            bindings::NV_VGPU_MSG_FUNCTION_FREE =>
Ok(MsgFunction::Free),
> +            bindings::NV_VGPU_MSG_FUNCTION_LOG => Ok(MsgFunction::Log),
> +            bindings::NV_VGPU_MSG_FUNCTION_GET_GSP_STATIC_INFO =>
Ok(MsgFunction::GetGspStaticInfo),
> +            bindings::NV_VGPU_MSG_FUNCTION_SET_REGISTRY =>
Ok(MsgFunction::SetRegistry),
> +            bindings::NV_VGPU_MSG_FUNCTION_GSP_SET_SYSTEM_INFO =>
Ok(MsgFunction::GspSetSystemInfo),
> +            bindings::NV_VGPU_MSG_FUNCTION_GSP_INIT_POST_OBJGPU => {
> +                Ok(MsgFunction::GspInitPostObjGpu)
> +            }
> +            bindings::NV_VGPU_MSG_FUNCTION_GSP_RM_CONTROL =>
Ok(MsgFunction::GspRmControl),
> +            bindings::NV_VGPU_MSG_FUNCTION_GET_STATIC_INFO =>
Ok(MsgFunction::GetStaticInfo),
> +            bindings::NV_VGPU_MSG_EVENT_GSP_INIT_DONE =>
Ok(MsgFunction::GspInitDone),
> +            bindings::NV_VGPU_MSG_EVENT_GSP_RUN_CPU_SEQUENCER => {
> +                Ok(MsgFunction::GspRunCpuSequencer)
> +            }
> +            bindings::NV_VGPU_MSG_EVENT_POST_EVENT =>
Ok(MsgFunction::PostEvent),
> +            bindings::NV_VGPU_MSG_EVENT_RC_TRIGGERED =>
Ok(MsgFunction::RcTriggered),
> +            bindings::NV_VGPU_MSG_EVENT_MMU_FAULT_QUEUED =>
Ok(MsgFunction::MmuFaultQueued),
> +            bindings::NV_VGPU_MSG_EVENT_OS_ERROR_LOG =>
Ok(MsgFunction::OsErrorLog),
> +            bindings::NV_VGPU_MSG_EVENT_GSP_POST_NOCAT_RECORD =>
Ok(MsgFunction::GspPostNoCat),
> +            bindings::NV_VGPU_MSG_EVENT_GSP_LOCKDOWN_NOTICE =>
Ok(MsgFunction::GspLockdownNotice),
> +            bindings::NV_VGPU_MSG_EVENT_UCODE_LIBOS_PRINT =>
Ok(MsgFunction::UcodeLibOsPrint),
> +            _ => Err(EINVAL),
> +        }
> +    }
> +}
This is another place where the `TryFrom` macro will shine. :)
> +
>  /// Struct containing the arguments required to pass a memory buffer to
the GSP
>  /// for use during initialisation.
>  ///
> @@ -208,3 +324,162 @@ fn id8(name: &str) -> u64 {
>          }))
>      }
>  }
> +
> +#[repr(transparent)]
> +pub(crate) struct MsgqTxHeader(bindings::msgqTxHeader);
Short documentation please (`MsgqRxHeader` provides a template).
> +
> +impl MsgqTxHeader {
> +    pub(crate) fn new(msgq_size: u32, rx_hdr_offset: u32, msg_count: u32)
-> Self {
> +        Self(bindings::msgqTxHeader {
> +            version: 0,
> +            size: msgq_size,
> +            msgSize: GSP_PAGE_SIZE as u32,
> +            msgCount: msg_count,
> +            writePtr: 0,
> +            flags: 1,
> +            rxHdrOff: rx_hdr_offset,
> +            entryOff: GSP_PAGE_SIZE as u32,
> +        })
> +    }
> +
> +    pub(crate) fn write_ptr(&self) -> u32 {
> +        let ptr = (&self.0.writePtr) as *const u32;
> +
> +        // SAFETY: This is part of a CoherentAllocation and implements the
> +        // equivalent as what the dma_read! macro would and is therefore
safe
> +        // for the same reasons.
> +        unsafe { ptr.read_volatile() }
> +    }
> +
> +    pub(crate) fn set_write_ptr(&mut self, val: u32) {
> +        let ptr = (&mut self.0.writePtr) as *mut u32;
> +
> +        // SAFETY: This is part of a CoherentAllocation and implements the
> +        // equivalent as what the dma_write! macro would and is therefore
safe
> +        // for the same reasons.
> +        unsafe { ptr.write_volatile(val) }
> +    }
> +}
These methods should also be succintly documented imho.
> +
> +// SAFETY: Padding is explicit and will not contain uninitialized data.
> +unsafe impl AsBytes for MsgqTxHeader {}
> +
> +/// RX header for setting up a message queue with the GSP.
> +#[repr(transparent)]
> +pub(crate) struct MsgqRxHeader(bindings::msgqRxHeader);
> +
> +impl MsgqRxHeader {
> +    pub(crate) fn new() -> Self {
> +        Self(Default::default())
> +    }
> +
> +    pub(crate) fn read_ptr(&self) -> u32 {
> +        let ptr = (&self.0.readPtr) as *const u32;
> +
> +        // SAFETY: This is part of a CoherentAllocation and implements the
> +        // equivalent as what the dma_read! macro would and is therefore
safe
> +        // for the same reasons.
> +        unsafe { ptr.read_volatile() }
> +    }
> +
> +    pub(crate) fn set_read_ptr(&mut self, val: u32) {
> +        let ptr = (&mut self.0.readPtr) as *mut u32;
> +
> +        // SAFETY: This is part of a CoherentAllocation and implements the
> +        // equivalent as what the dma_write! macro would and is therefore
safe
> +        // for the same reasons.
> +        unsafe { ptr.write_volatile(val) }
> +    }
> +}
Documentation here as well.
> +
> +// SAFETY: Padding is explicit and will not contain uninitialized data.
> +unsafe impl AsBytes for MsgqRxHeader {}
> +
> +impl bindings::rpc_message_header_v {
> +    pub(crate) fn init(cmd_size: u32, function: MsgFunction) -> impl
Init<Self, Error> {
This method can be kept private.
> +        type RpcMessageHeader = bindings::rpc_message_header_v;
> +        try_init!(RpcMessageHeader {
> +            // TODO: magic number
> +            header_version: 0x03000000,
Let's resolve this TODO by declaring a constant or even better,
adding the relevant bindings symbol if there is one.
> +            signature: bindings::NV_VGPU_MSG_SIGNATURE_VALID,
> +            function: function as u32,
This `as` is ok once we set `MsgFunction` to have a `u32`
representation, but...
> +            length: (size_of::<Self>() as u32)
... at the risk of sounding too pedantic, we probably want to do the
arithmetic on the `usize`, and then do a `try_into` u32 to remove this
use of `as` here:

    length: size_of::<Self>()
        .checked_add(cmd_size)
        .ok_or(EOVERFLOW)
        .and_then(|v| v.try_into().map_err(|_| EINVAL))?,

This also requires changing the `cmd_size` to `usize`, which is actually
better as that's also the type used by the caller anyway, so this
removes another conversion.
> +                .checked_add(cmd_size)
> +                .ok_or(EOVERFLOW)?,
> +            rpc_result: 0xffffffff,
> +            rpc_result_private: 0xffffffff,
> +            ..Zeroable::init_zeroed()
> +        })
> +    }
> +}
> +
> +// SAFETY: We can't derive the Zeroable trait for this binding because
the
> +// procedural macro doesn't support the syntax used by bindgen to
create the
> +// __IncompleteArrayField types. So instead we implement it here, which is
safe
> +// because these are explicitly padded structures only containing types
for
> +// which any bit pattern, including all zeros, is valid.
> +unsafe impl Zeroable for bindings::rpc_message_header_v {}
> +
> +#[repr(transparent)]
> +pub(crate) struct GspMsgElement {
> +    inner: bindings::GSP_MSG_QUEUE_ELEMENT,
> +}
Small documentation needed.
> +
> +impl GspMsgElement {
> +    #[allow(non_snake_case)]
> +    pub(crate) fn init(
> +        sequence: u32,
> +        cmd_size: usize,
> +        function: MsgFunction,
> +    ) -> impl Init<Self, Error> {
> +        type RpcMessageHeader = bindings::rpc_message_header_v;
> +        type InnerGspMsgElement = bindings::GSP_MSG_QUEUE_ELEMENT;
> +        let init_inner = try_init!(InnerGspMsgElement {
> +            seqNum: sequence,
> +            elemCount: size_of::<Self>()
> +                .checked_add(cmd_size)
> +                .ok_or(EOVERFLOW)?
> +                .div_ceil(GSP_PAGE_SIZE) as u32,
Let's chain a call to `try_into` here as well.
> +            rpc <- RpcMessageHeader::init(cmd_size as u32, function),
... and thanks to the change suggested 3 blocks above, this `as` is no
longer needed.
> +            ..Zeroable::init_zeroed()
> +        });
> +
> +        try_init!(GspMsgElement {
> +            inner <- init_inner,
> +        })
> +    }
> +
> +    pub(crate) fn set_checksum(&mut self, checksum: u32) {
> +        self.inner.checkSum = checksum;
> +    }
> +
> +    // Return the total length of the message, noting that rpc.length
includes
> +    // the length of the GspRpcHeader but not the message header.
> +    pub(crate) fn length(&self) -> u32 {
> +        size_of::<Self>() as u32 -
size_of::<bindings::rpc_message_header_v>() as u32
> +            + self.inner.rpc.length
> +    }
We can save ourselves a bunch of `as` conversions in the following patch
if we make this method just return `usize`. The only one needed will be
for `self.inner.rpc.length` to `usize`, we really ought to have
non-fallible converters for things like this that obviously cannot fail.
> +
> +    pub(crate) fn sequence(&self) -> u32 {
> +        self.inner.rpc.sequence
> +    }
> +
> +    pub(crate) fn function_number(&self) -> u32 {
> +        self.inner.rpc.function
> +    }
> +
> +    pub(crate) fn function(&self) -> Result<MsgFunction> {
> +        self.inner.rpc.function.try_into()
> +    }
> +
> +    pub(crate) fn element_count(&self) -> u32 {
> +        self.inner.elemCount
> +    }
> +}
Documentation needed for these too.

On Thu, Oct 16, 2025 at 8:24?AM Alexandre Courbot <acourbot at nvidia.com>
wrote:
>
> There are lots of unused warnings when building on this commit. I know
> we don't like big patches, but since this is semantically related to
the
> next one and the two touch different files anyway, how about merging
> them? IMHO this is preferable to adding lots of `#[expect(unused)]` that
> we are going to remove right after.
>
> If you have been told to split in a previous version, let's just add a
> the `#[expect(unused)]`  where needed.
A possible middle-ground is to add the `expect` in a "more global"
position, e.g. at the top, so that it is a single line change. I think
that is fine as long as it is removed right after.

Thanks!

Cheers,
Miguel

On 2025-10-17 at 06:22 +1100, Miguel Ojeda <miguel.ojeda.sandonis at
gmail.com> wrote...
> On Thu, Oct 16, 2025 at 8:24?AM Alexandre Courbot <acourbot at
nvidia.com> wrote:
> >
> > There are lots of unused warnings when building on this commit. I know
> > we don't like big patches, but since this is semantically related
to the
> > next one and the two touch different files anyway, how about merging
> > them? IMHO this is preferable to adding lots of `#[expect(unused)]`
that
> > we are going to remove right after.
> >
> > If you have been told to split in a previous version, let's just
add a
> > the `#[expect(unused)]`  where needed.
No, I just split it here because it seemed like a nice self-contained thing.
That and I'm conditioned to linux-mm patches where 20 changes is often
considered a big patch :-)

So I will probably just squash this as suggested.
> 
> A possible middle-ground is to add the `expect` in a "more
global"
> position, e.g. at the top, so that it is a single line change. I think
> that is fine as long as it is removed right after.
That could work, thanks.
> Thanks!
> 
> Cheers,
> Miguel