Re: [PATCH 3/4] gpu: nova-core: gsp: cmdq: add the GMC send path

[Eliot Courtney]

On Tue Nov 3, 2026 at 9:02 AM JST, Alexandre Courbot wrote:
> From: John Hubbard <jhubbard@xxxxxxxxxx>
>
> The r000 boot protocol has the driver send GSP_INIT, the request that
> carries the driver's configuration to GSP-RM, as a GMC (GPU Management
> Controller) API command. GMC and RPC commands share the command queue
> and its sequence counter. A GMC command always fits in one element, as
> the GMC API has no continuation records.
>
> Add the gmc sub-module of the command queue, sibling of rpc, traits and
> types definitions mirroring those of rpc and adding support for GMC messages.
>
> The module has no user yet, and is built as dead code until the switch
> to r000.
>
> Assisted-by: LLM
> Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
> Co-developed-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
> ---
>  drivers/gpu/nova-core/gsp/cmdq.rs     |   4 +
>  drivers/gpu/nova-core/gsp/cmdq/gmc.rs | 194 ++++++++++++++++++++++++++++++++++
>  2 files changed, 198 insertions(+)
>
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index c8b103a0785d..629ab8d35f87 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -1,6 +1,10 @@
>  // SPDX-License-Identifier: GPL-2.0
>  // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
>  
> +// The transport header of the r570 firmware carries no message type, so nothing can produce or
> +// consume a GMC message before the switch to r000. Until then the module is built as dead code.
> +#[expect(dead_code)]
> +pub(super) mod gmc;
>  pub(super) mod rpc;
>  
>  use core::mem;
> diff --git a/drivers/gpu/nova-core/gsp/cmdq/gmc.rs b/drivers/gpu/nova-core/gsp/cmdq/gmc.rs
> new file mode 100644
> index 000000000000..b035f30479a8
> --- /dev/null
> +++ b/drivers/gpu/nova-core/gsp/cmdq/gmc.rs
> @@ -0,0 +1,194 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> +
> +//! Support for the GMC (GPU Management Controller) API message type on the GSP command queue.
> +
> +use kernel::{
> +    device,
> +    prelude::*,
> +    transmute::{
> +        AsBytes,
> +        FromBytes, //
> +    },
> +};
> +
> +use crate::{
> +    gsp::{
> +        cmdq::{
> +            GspCommand,
> +            NoReply, //
> +        },
> +        fw::{
> +            GmcApiHeader,
> +            GmcCommandId, //
> +        },
> +    },
> +    sbuffer::SBufferIter,
> +};
> +
> +use super::{
> +    Cmdq,
> +    CmdqInner,
> +    CommandElement, //
> +};

nit: importing from the same module through both crate:: and super::?

> +
> +/// Trait implemented by types representing a GMC command to send to the GSP.
> +///
> +/// The main purpose of this trait is to provide [`Cmdq`] with the information it needs to send
> +/// a given command.
> +///
> +/// [`CommandToGsp::init`] in particular is responsible for initializing the command directly
> +/// into the space reserved for it in the command queue buffer.
> +///
> +/// Some commands may be followed by a variable-length payload. For these, the
> +/// [`CommandToGsp::variable_payload_len`] and [`CommandToGsp::init_variable_payload`] need to be
> +/// defined as well.
> +pub(crate) trait CommandToGsp {
> +    /// Id identifying this command to the GSP.
> +    const COMMAND: GmcCommandId;
> +
> +    /// Type generated by [`CommandToGsp::init`], to be written into the command queue buffer right
> +    /// after the GMC API header.
> +    type Command: FromBytes + AsBytes;
> +
> +    /// Type of the reply expected from the GSP, or [`NoReply`] for commands that don't have a
> +    /// reply.
> +    type Reply;
> +
> +    /// Largest reply this command accepts, in bytes. Zero for commands that the GSP does not
> +    /// answer.
> +    const MAX_REPLY_SIZE: u32;

Same issue here as patch 2 - max reply size 0 and no reply are not the
same IIUC.

I considered the idea of putting this on the Reply type instead. It
would be simpler now, but conceptually the max reply size depends on the
content of the command, not just the reply type. And in the future we
might want to make this into a trait method not a constant. So lgtm.

[...]

> +    fn write(&self, dev: &device::Device, seq: u32, dst: &mut GspCommand<'_>) -> Result {
> +        let command = &self.0;
> +        let payload_size = command.size();
> +        // Extract the areas for the GMC API header and the command itself. The GSP message
> +        // header, the GMC API header and the command header together are guaranteed to fit
> +        // entirely into a single page, so it's ok to only look at `dst.contents.0` here.

optional nit: add const assert for checking the sizing fits in one slot
here, since it's a protocol constraint that we can check statically.

Reviewed-by: Eliot Courtney <ecourtney@xxxxxxxxxx>