[PATCH 0/4] gpu: nova-core: gsp: add support for GMC messages

[PATCH 0/4] gpu: nova-core: gsp: add support for GMC messages

[Alexandre Courbot]

This series contains patches 9 to 12 from John's r000 series v3 [1],
that add support for the GMC GSP message protocol. This code is rebased
on and adapted to the latest `drm-rust-next`, which clearly separates
the transport and message layers of the GSP command queue code. As a
result, this extracted series should be both smaller and easier to
review.

GMC is a new message type with its own header, used by the r000 firmware
for its boot protocol and newer interfaces.

No functional change is introduced by this series. The newly added code
remains dormant until the switch to r000 takes effect.

This series is based on `drm-rust-next` with [2] applied.

[1] https://lore.kernel.org/20260918010719.1176945-1-jhubbard@nvidia.com
[2] https://lore.kernel.org/all/20261102-r000-cmdq-prep-v2-0-c2c2b69bdec0@nvidia.com/

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
Changes since John's v3:
- Rebase on top of `drm-rust-next`.
- Use same message type abstractions as the RPC message type.
- Simplify comments and code where relevant.
- Make `GmcApiHeader` wrap the bindings type instead of reproducing it.

---
John Hubbard (4):
      gpu: nova-core: gsp: fw: add the r000 bindings for the GMC API header
      gpu: nova-core: gsp: add the GMC API header
      gpu: nova-core: gsp: cmdq: add the GMC send path
      gpu: nova-core: gsp: cmdq: add the GMC receive path

 drivers/gpu/nova-core/gsp/cmdq.rs                |  11 +
 drivers/gpu/nova-core/gsp/cmdq/gmc.rs            | 422 +++++++++++++++++++++++
 drivers/gpu/nova-core/gsp/cmdq/rpc.rs            |   8 +-
 drivers/gpu/nova-core/gsp/fw.rs                  | 106 ++++++
 drivers/gpu/nova-core/gsp/fw/r000_00.rs          |  29 ++
 drivers/gpu/nova-core/gsp/fw/r000_00/bindings.rs | 110 ++++++
 6 files changed, 685 insertions(+), 1 deletion(-)
---
base-commit: bde6db72e1d70d16551ca2108ae6da2c5cc9504f
change-id: 20261030-r000_rebase_on_cmdq-rpc-2990d9296a8f

Best regards,
--  
Alexandre Courbot <acourbot@nvidia.com>

[PATCH 1/4] gpu: nova-core: gsp: fw: add the r000 bindings for the GMC API header

[Alexandre Courbot]

From: John Hubbard <jhubbard@nvidia.com>

Nova-core boots the r570 GSP firmware, release 570.144. The GSP firmware
releases from r615 onward share one ABI, and their firmware files carry
no version number in their names, because each image records its version
internally. The driver calls that ABI "r000", so that its name does not
change with each release.

Add the first part of the bindings that bindgen generates from the r000
headers of OpenRM, the open-source NVIDIA kernel driver. The following
patch builds the GMC API header type on them, and the driver keeps
compiling against the r570 bindings until the patch that switches
firmware.

The headers are those of OpenRM release 615.71.09.

Assisted-by: LLM
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Reviewed-by: Timur Tabi <ttabi@nvidia.com>
[acourbot: split the bindings by the code that needs them, name the
 OpenRM release.]
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/gsp/fw.rs                  |   1 +
 drivers/gpu/nova-core/gsp/fw/r000_00.rs          |  29 ++++++
 drivers/gpu/nova-core/gsp/fw/r000_00/bindings.rs | 110 +++++++++++++++++++++++
 3 files changed, 140 insertions(+)

diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index b2ddeb27984f..213ef5bfb258 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -2,6 +2,7 @@
 // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 
 pub(crate) mod commands;
+mod r000_00;
 mod r570_144;
 
 // Alias to avoid repeating the version number with every use.
diff --git a/drivers/gpu/nova-core/gsp/fw/r000_00.rs b/drivers/gpu/nova-core/gsp/fw/r000_00.rs
new file mode 100644
index 000000000000..0c32f4ec1e27
--- /dev/null
+++ b/drivers/gpu/nova-core/gsp/fw/r000_00.rs
@@ -0,0 +1,29 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Bindings generated by `bindgen` from the r000 GSP firmware headers.
+//!
+//! Use them only through the parent module, which abstracts or re-exports the symbols that the
+//! driver needs.
+
+#![allow(
+    dead_code,
+    clippy::all,
+    clippy::undocumented_unsafe_blocks,
+    clippy::ptr_as_ptr,
+    clippy::ref_as_ptr,
+    missing_docs,
+    non_camel_case_types,
+    non_upper_case_globals,
+    non_snake_case,
+    improper_ctypes,
+    unreachable_pub,
+    unsafe_op_in_unsafe_fn
+)]
+use kernel::ffi;
+use pin_init::MaybeZeroable;
+
+include!("r000_00/bindings.rs");
+
+// SAFETY: This type has a size of zero, so its inclusion into another type should not affect their
+// ability to implement `Zeroable`.
+unsafe impl<T> kernel::prelude::Zeroable for __IncompleteArrayField<T> {}
diff --git a/drivers/gpu/nova-core/gsp/fw/r000_00/bindings.rs b/drivers/gpu/nova-core/gsp/fw/r000_00/bindings.rs
new file mode 100644
index 000000000000..552299c98169
--- /dev/null
+++ b/drivers/gpu/nova-core/gsp/fw/r000_00/bindings.rs
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#[repr(C)]
+#[derive(Default)]
+pub struct __IncompleteArrayField<T>(::core::marker::PhantomData<T>, [T; 0]);
+impl<T> __IncompleteArrayField<T> {
+    #[inline]
+    pub const fn new() -> Self {
+        __IncompleteArrayField(::core::marker::PhantomData, [])
+    }
+    #[inline]
+    pub fn as_ptr(&self) -> *const T {
+        self as *const _ as *const T
+    }
+    #[inline]
+    pub fn as_mut_ptr(&mut self) -> *mut T {
+        self as *mut _ as *mut T
+    }
+    #[inline]
+    pub unsafe fn as_slice(&self, len: usize) -> &[T] {
+        ::core::slice::from_raw_parts(self.as_ptr(), len)
+    }
+    #[inline]
+    pub unsafe fn as_mut_slice(&mut self, len: usize) -> &mut [T] {
+        ::core::slice::from_raw_parts_mut(self.as_mut_ptr(), len)
+    }
+}
+impl<T> ::core::fmt::Debug for __IncompleteArrayField<T> {
+    fn fmt(&self, fmt: &mut ::core::fmt::Formatter<'_>) -> ::core::fmt::Result {
+        fmt.write_str("__IncompleteArrayField")
+    }
+}
+pub type __u8 = ffi::c_uchar;
+pub type __u16 = ffi::c_ushort;
+pub type __u32 = ffi::c_uint;
+pub type __u64 = ffi::c_ulonglong;
+pub type u8_ = __u8;
+pub type u16_ = __u16;
+pub type u32_ = __u32;
+pub type u64_ = __u64;
+#[repr(C)]
+#[derive(MaybeZeroable)]
+pub struct GMCAPI_HEADER {
+    pub command: u32_,
+    pub size: u32_,
+    pub sequence: u64_,
+    pub __bindgen_anon_1: GMCAPI_HEADER__bindgen_ty_1,
+    pub reserved: [u32_; 5usize],
+    pub data: __IncompleteArrayField<u8_>,
+}
+#[repr(C)]
+#[derive(Copy, Clone, MaybeZeroable)]
+pub union GMCAPI_HEADER__bindgen_ty_1 {
+    pub request: GMCAPI_HEADER__bindgen_ty_1__bindgen_ty_1,
+    pub response: GMCAPI_HEADER__bindgen_ty_1__bindgen_ty_2,
+}
+#[repr(C)]
+#[derive(Debug, Default, Copy, Clone, MaybeZeroable)]
+pub struct GMCAPI_HEADER__bindgen_ty_1__bindgen_ty_1 {
+    pub max_response_size: u32_,
+}
+#[repr(C)]
+#[derive(Debug, Default, Copy, Clone, MaybeZeroable)]
+pub struct GMCAPI_HEADER__bindgen_ty_1__bindgen_ty_2 {
+    pub status: u32_,
+}
+impl Default for GMCAPI_HEADER__bindgen_ty_1 {
+    fn default() -> Self {
+        let mut s = ::core::mem::MaybeUninit::<Self>::uninit();
+        unsafe {
+            ::core::ptr::write_bytes(s.as_mut_ptr(), 0, 1);
+            s.assume_init()
+        }
+    }
+}
+impl Default for GMCAPI_HEADER {
+    fn default() -> Self {
+        let mut s = ::core::mem::MaybeUninit::<Self>::uninit();
+        unsafe {
+            ::core::ptr::write_bytes(s.as_mut_ptr(), 0, 1);
+            s.assume_init()
+        }
+    }
+}
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_RESERVED_LAST: GMCAPI_COMMANDS = 65535;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_GSP_INIT: GMCAPI_COMMANDS = 65537;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_EXEC_GENERIC_BOOTLOADER: GMCAPI_COMMANDS = 65538;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_EXEC_HS_BINARY: GMCAPI_COMMANDS = 65539;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_GSP_SUSPEND: GMCAPI_COMMANDS = 65540;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_GSP_RESUME_DONE: GMCAPI_COMMANDS = 65541;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_ADD_VGPU_TYPE: GMCAPI_COMMANDS = 131073;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_QUERY_SUPPORTED_VGPU_TYPES: GMCAPI_COMMANDS = 131074;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_QUERY_CREATABLE_VGPU_TYPES: GMCAPI_COMMANDS = 131075;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_ASSIGN_VGPU_TYPE: GMCAPI_COMMANDS = 131076;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_DEASSIGN_VGPU_TYPE: GMCAPI_COMMANDS = 131077;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_QUERY_VGPU_PROPERTIES: GMCAPI_COMMANDS = 131078;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_QUERY_ASSIGNED_VF_VGPU_TYPE: GMCAPI_COMMANDS = 131079;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_BOOTLOAD_GSP_VGPU_PLUGIN_TASK: GMCAPI_COMMANDS = 131104;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_SHUTDOWN_GSP_VGPU_PLUGIN_TASK: GMCAPI_COMMANDS = 131105;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_SHUTDOWN_GSP_VGPU_PLUGIN_TASK_COMPLETE: GMCAPI_COMMANDS =
+    131106;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_CLEANUP_GSP_VGPU_PLUGIN_RESOURCES: GMCAPI_COMMANDS = 131107;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_VGPU_PLUGIN_TRIGGERED_EVENT: GMCAPI_COMMANDS = 131108;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_VGPU_MGR_SCRUB_GUEST_FB: GMCAPI_COMMANDS = 131109;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_VGPU_MGR_ALLOC_GSP_CEUTILS: GMCAPI_COMMANDS = 131110;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_VGPU_MGR_FREE_GSP_CEUTILS: GMCAPI_COMMANDS = 131111;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_SCHED_CONTROL: GMCAPI_COMMANDS = 196609;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_MMU_FAULT_QUEUED: GMCAPI_COMMANDS = 262145;
+pub const GMCAPI_COMMANDS_GMCAPI_CMD_INVALID: GMCAPI_COMMANDS = 4294967295;
+pub type GMCAPI_COMMANDS = ffi::c_uint;

-- 
2.56.0

[PATCH 2/4] gpu: nova-core: gsp: add the GMC API header

[Alexandre Courbot]

From: John Hubbard <jhubbard@nvidia.com>

The r000 firmware carries GSP-RM's boot protocol, and its newer
interfaces in general, over the GMC (GPU Management Controller) API:
messages that follow the queue element header with a GMC API header
instead of an RPC header.

Add the GmcApiHeader type as a newtype over the GMCAPI_HEADER binding,
so we can define the GMC send and receive paths in the following
patches.

[ acourbot: wrap the bindings type instead of redefining it; make
  `status` return an `Option` to avoid returning a value whose semantics
  are ambiguous.]

Assisted-by: LLM
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Co-developed-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/gsp/fw.rs | 106 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 106 insertions(+)

diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index 213ef5bfb258..367c8b5e3443 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -37,6 +37,7 @@
 };
 
 use crate::{
+    bounded_enum,
     fb::{
         FbRanges,
         FbSizes, //
@@ -906,6 +907,111 @@ pub(crate) fn function(&self) -> Result<MsgFunction, u32> {
     }
 }
 
+bounded_enum! {
+    /// Commands of the GMC API.
+    #[derive(Debug, Clone, Copy, PartialEq, Eq)]
+    #[repr(u32)]
+    pub(crate) enum GmcCommandId with TryFrom<Bounded<u32, 24>> {
+        GspInit = r000_00::GMCAPI_COMMANDS_GMCAPI_CMD_GSP_INIT,
+        ExecGenericBootloader = r000_00::GMCAPI_COMMANDS_GMCAPI_CMD_EXEC_GENERIC_BOOTLOADER,
+        ExecHsBinary = r000_00::GMCAPI_COMMANDS_GMCAPI_CMD_EXEC_HS_BINARY,
+        GspSuspend = r000_00::GMCAPI_COMMANDS_GMCAPI_CMD_GSP_SUSPEND,
+        MmuFaultQueued = r000_00::GMCAPI_COMMANDS_GMCAPI_CMD_MMU_FAULT_QUEUED,
+    }
+}
+
+bitfield! {
+    /// The `command` word of a [`GmcApiHeader`].
+    pub(crate) struct GmcCommandWord(u32) {
+        /// Command ID.
+        23:0 id ?=> GmcCommandId;
+        /// Set by GSP-RM on the response to a request. Not set for requests and events.
+        24:24 is_response => bool;
+    }
+}
+
+/// Header of a GMC (GPU Management Controller) API message.
+///
+/// In a queue element, it follows the queue element header and precedes the payload of the
+/// command.
+#[repr(transparent)]
+#[expect(dead_code)]
+pub(crate) struct GmcApiHeader {
+    inner: r000_00::GMCAPI_HEADER,
+}
+
+#[expect(dead_code)]
+impl GmcApiHeader {
+    /// Creates the header of a request for `command`, sent with sequence number `sequence` and
+    /// followed by `payload_size` bytes of payload.
+    ///
+    /// `max_reply_size` is the largest reply the sender accepts, and zero for a request that
+    /// GSP-RM does not answer.
+    ///
+    /// # Errors
+    ///
+    /// - `EOVERFLOW` if `payload_size` does not fit its 32-bit field.
+    pub(crate) fn init(
+        command: GmcCommandId,
+        sequence: u64,
+        payload_size: usize,
+        max_reply_size: u32,
+    ) -> impl Init<Self, Error> {
+        let init_inner = try_init!(r000_00::GMCAPI_HEADER {
+            command: GmcCommandWord::zeroed().with_id(command).into(),
+            size: payload_size.try_into().map_err(|_| EOVERFLOW)?,
+            sequence,
+            __bindgen_anon_1: r000_00::GMCAPI_HEADER__bindgen_ty_1 {
+                request: r000_00::GMCAPI_HEADER__bindgen_ty_1__bindgen_ty_1 {
+                    max_response_size: max_reply_size,
+                },
+            },
+            ..Zeroable::init_zeroed()
+        });
+
+        try_init!(GmcApiHeader {
+            inner <- init_inner,
+        })
+    }
+
+    /// Returns the `command` word of the header.
+    pub(crate) fn command(&self) -> GmcCommandWord {
+        GmcCommandWord::from(self.inner.command)
+    }
+
+    /// Returns the sequence number of the message.
+    pub(crate) fn sequence(&self) -> u64 {
+        self.inner.sequence
+    }
+
+    /// Returns the size in bytes of the payload following the header.
+    pub(crate) fn payload_size(&self) -> usize {
+        num::u32_as_usize(self.inner.size)
+    }
+
+    /// Returns the `NV_STATUS` carried by a response to a command.
+    ///
+    /// Returns `Some` with the `NV_STATUS` code if the `response` flag of [`Self::command`] is set;
+    /// otherwise returns `None`.
+    pub(crate) fn status(&self) -> Option<u32> {
+        if self.command().is_response() {
+            // SAFETY: we just checked that the message is a response.
+            let status = unsafe { self.inner.__bindgen_anon_1.response.status };
+
+            Some(status)
+        } else {
+            None
+        }
+    }
+}
+
+// SAFETY: This struct only contains integer types, including inside its union, and a zero-sized
+// array. It has no padding.
+unsafe impl AsBytes for GmcApiHeader {}
+
+// SAFETY: This struct only contains integer types for which all bit patterns are valid.
+unsafe impl FromBytes for GmcApiHeader {}
+
 /// Arguments for GSP startup.
 #[repr(transparent)]
 #[derive(Zeroable)]

-- 
2.56.0

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

[Alexandre Courbot]

From: John Hubbard <jhubbard@nvidia.com>

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@nvidia.com>
Co-developed-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 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, //
+};
+
+/// 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;
+
+    /// Error type returned by [`CommandToGsp::init`].
+    type InitError;
+
+    /// In-place command initializer responsible for filling the command in the command queue
+    /// buffer.
+    fn init(&self) -> impl Init<Self::Command, Self::InitError>;
+
+    /// Size of the variable-length payload following the command structure generated by
+    /// [`CommandToGsp::init`].
+    ///
+    /// Most commands don't have a variable-length payload, so this is zero by default.
+    fn variable_payload_len(&self) -> usize {
+        0
+    }
+
+    /// Method initializing the variable-length payload.
+    ///
+    /// The command buffer is circular, which means that we may need to jump back to its beginning
+    /// while in the middle of a command. For this reason, the variable-length payload is
+    /// initialized using a [`SBufferIter`].
+    ///
+    /// This method will receive a buffer of the length returned by
+    /// [`CommandToGsp::variable_payload_len`], and must write every single byte of it. Leaving
+    /// unwritten space will lead to an error.
+    ///
+    /// Most commands don't have a variable-length payload, so this does nothing by default.
+    fn init_variable_payload(
+        &self,
+        _dst: &mut SBufferIter<core::array::IntoIter<&mut [u8], 2>>,
+    ) -> Result {
+        Ok(())
+    }
+
+    /// Total size of the command (including its variable-length payload) without the GMC API
+    /// header, i.e. the payload size advertised by that header.
+    fn size(&self) -> usize {
+        size_of::<Self::Command>() + self.variable_payload_len()
+    }
+}
+
+/// Wrapper type for sending a GMC command as a command queue element.
+struct GmcCommandElement<C>(C);
+
+impl<C> CommandElement for GmcCommandElement<C>
+where
+    C: CommandToGsp,
+    Error: From<C::InitError>,
+{
+    fn size(&self) -> usize {
+        size_of::<GmcApiHeader>() + self.0.size()
+    }
+
+    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.
+        let (header, contents) = GmcApiHeader::from_bytes_mut_prefix(dst.contents.0).ok_or(EIO)?;
+        let (cmd, payload_1) = C::Command::from_bytes_mut_prefix(contents).ok_or(EIO)?;
+        let header_init =
+            GmcApiHeader::init(C::COMMAND, u64::from(seq), payload_size, C::MAX_REPLY_SIZE);
+        // SAFETY: `header` is a valid reference, and is not touched if the initializer fails.
+        unsafe {
+            pin_init::raw_try_init(core::ptr::from_mut(header), header_init)?;
+        }
+        // SAFETY: `cmd` is a valid reference, and is not touched if the initializer fails.
+        unsafe {
+            pin_init::raw_try_init(core::ptr::from_mut(cmd), command.init())?;
+        }
+
+        // Fill the variable-length payload, which may be empty.
+        let mut sbuffer = SBufferIter::new_writer([&mut payload_1[..], &mut dst.contents.1[..]]);
+        command.init_variable_payload(&mut sbuffer)?;
+
+        if !sbuffer.is_empty() {
+            return Err(EIO);
+        }
+
+        dev_dbg!(
+            dev,
+            "GSP GMC: send: seq# {}, command={:?}, length=0x{:x}\n",
+            seq,
+            C::COMMAND,
+            payload_size,
+        );
+
+        Ok(())
+    }
+}
+
+impl Cmdq<'_> {
+    /// Sends `command` to the GSP without waiting for a reply.
+    ///
+    /// # Errors
+    ///
+    /// - `EMSGSIZE` if the command does not fit in a single command queue element.
+    /// - `ETIMEDOUT` if space does not become available within the timeout.
+    /// - `EIO` if the variable payload requested by the command has not been entirely
+    ///   written to by its [`CommandToGsp::init_variable_payload`] method.
+    ///
+    /// Error codes returned by the command initializer are propagated as-is.
+    pub(crate) fn send_gmc_command_no_wait<C>(&self, command: C) -> Result
+    where
+        C: CommandToGsp<Reply = NoReply>,
+        Error: From<C::InitError>,
+    {
+        self.inner.lock().send_gmc_command(command).map(|_| ())
+    }
+}
+
+impl CmdqInner<'_> {
+    /// Sends `command` to the GSP, and returns the sequence number of its element.
+    ///
+    /// Unlike RPC commands, GMC commands are never split: a command that does not fit in a single
+    /// element is an error.
+    ///
+    /// # Errors
+    ///
+    /// - `EMSGSIZE` if the command does not fit in a single command queue element.
+    /// - `ETIMEDOUT` if space does not become available within the timeout.
+    /// - `EIO` if the variable payload requested by the command has not been entirely
+    ///   written to by its [`CommandToGsp::init_variable_payload`] method.
+    ///
+    /// Error codes returned by the command initializer are propagated as-is.
+    fn send_gmc_command<C>(&mut self, command: C) -> Result<u32>
+    where
+        C: CommandToGsp,
+        Error: From<C::InitError>,
+    {
+        self.send_command_element(GmcCommandElement(command))
+    }
+}

-- 
2.56.0

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

[Alexandre Courbot]

From: John Hubbard <jhubbard@nvidia.com>

GSP-RM answers a GMC request with a GMC message whose command word
carries the response flag and the id of the request, and whose sequence
number is that of the request. It also raises GMC events, named by a
command id. During boot, the load-and-execute events arrive while the
driver is waiting for the GSP_INIT reply, and GSP-RM only answers once
they have been handled.

Add the receive side of the gmc sub-module, mirroring the
already-existing rpc sub-module.

The transport cannot produce GMC messages yet: the r570 queue element
header does not carry the type of the message it transports, so the Gmc
variant of GspParsedMessage is only constructed once the switch to r000
makes the transport dispatch on the NVDM type. The RPC receive path and
drain() account for it by logging unclaimed GMC messages.

Assisted-by: LLM
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
Co-developed-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs     |   7 ++
 drivers/gpu/nova-core/gsp/cmdq/gmc.rs | 230 +++++++++++++++++++++++++++++++++-
 drivers/gpu/nova-core/gsp/cmdq/rpc.rs |   8 +-
 drivers/gpu/nova-core/gsp/fw.rs       |   1 -
 4 files changed, 243 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 629ab8d35f87..a1fa7a3b4079 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -420,6 +420,12 @@ struct GspMessage<'a> {
 enum GspParsedMessage<'a> {
     /// A [RPC message](rpc::RpcMessage).
     Rpc(rpc::RpcMessage<'a>),
+    /// A [GMC message](gmc::GmcMessage).
+    //
+    // The transport header of the r570 firmware carries no message type, so this variant cannot
+    // be constructed before the switch to r000.
+    #[expect(dead_code)]
+    Gmc(gmc::GmcMessage<'a>),
 }
 
 impl GspParsedMessage<'_> {
@@ -427,6 +433,7 @@ impl GspParsedMessage<'_> {
     fn log(&self, dev: &device::Device) {
         match self {
             GspParsedMessage::Rpc(rpc) => rpc.log(dev),
+            GspParsedMessage::Gmc(gmc) => gmc.log(dev),
         }
     }
 }
diff --git a/drivers/gpu/nova-core/gsp/cmdq/gmc.rs b/drivers/gpu/nova-core/gsp/cmdq/gmc.rs
index b035f30479a8..9adba5cbcd5c 100644
--- a/drivers/gpu/nova-core/gsp/cmdq/gmc.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq/gmc.rs
@@ -29,7 +29,9 @@
 use super::{
     Cmdq,
     CmdqInner,
-    CommandElement, //
+    CommandElement,
+    GspMessage,
+    GspParsedMessage, //
 };
 
 /// Trait implemented by types representing a GMC command to send to the GSP.
@@ -150,7 +152,167 @@ fn write(&self, dev: &device::Device, seq: u32, dst: &mut GspCommand<'_>) -> Res
     }
 }
 
+/// Trait representing GMC messages received from the GSP.
+///
+/// These can be either replies to sent commands that expect a reply, or an event not bound to any
+/// command.
+pub(crate) trait MessageFromGsp: Sized {
+    /// Id of the command this is the reply to.
+    const COMMAND: GmcCommandId;
+
+    /// Error type returned by [`MessageFromGsp::read`].
+    type InitError;
+
+    /// Method reading the reply from the payload of the message and returning it.
+    fn read(
+        payload: &mut SBufferIter<core::array::IntoIter<&[u8], 2>>,
+    ) -> Result<Self, Self::InitError>;
+}
+
+/// A GMC message ready to be processed from the message queue.
+pub(crate) struct GmcMessage<'a> {
+    // Reference to the GMC API header of the message.
+    header: &'a GmcApiHeader,
+    // Slices to the payload of the message. The second slice is empty unless the message loops
+    // over the message queue.
+    contents: (&'a [u8], &'a [u8]),
+}
+
+impl<'a> GmcMessage<'a> {
+    /// Validate the GMC layer of `element` and returns its GMC API header and its contents trimmed
+    /// down to the GMC payload.
+    ///
+    /// # Errors
+    ///
+    /// - `EIO` if the element is shorter than the GMC API header, or than the payload length
+    ///   advertised by the GMC API header.
+    pub(super) fn parse(dev: &device::Device, element: GspMessage<'a>) -> Result<GmcMessage<'a>> {
+        let GspMessage {
+            contents: (slice_1, slice_2),
+            ..
+        } = element;
+
+        // The GSP message header and the GMC API header together are guaranteed to fit entirely
+        // into a single page, so it's ok to only look at `slice_1` here.
+        let (gmc_header, slice_1) = GmcApiHeader::from_bytes_prefix(slice_1).ok_or(EIO)?;
+        let payload_size = gmc_header.payload_size();
+
+        dev_dbg!(
+            dev,
+            "GSP GMC: receive: seq# {}, command={:?}, length=0x{:x}\n",
+            gmc_header.sequence(),
+            gmc_header.command(),
+            payload_size,
+        );
+
+        // Check that the driver read area is large enough for the message.
+        if slice_1.len() + slice_2.len() < payload_size {
+            return Err(EIO);
+        }
+
+        // Cut the message slices down to the actual length of the message.
+        let (slice_1, slice_2) = if slice_1.len() > payload_size {
+            // PANIC: we checked above that `slice_1` is at least as long as `payload_size`.
+            (slice_1.split_at(payload_size).0, &slice_2[0..0])
+        } else {
+            (
+                slice_1,
+                // PANIC: we checked above that `slice_1.len() + slice_2.len()` is at least as
+                // large as `payload_size`.
+                slice_2.split_at(payload_size - slice_1.len()).0,
+            )
+        };
+
+        Ok(GmcMessage {
+            header: gmc_header,
+            contents: (slice_1, slice_2),
+        })
+    }
+
+    /// Returns the GMC API header of the message.
+    pub(crate) fn header(&self) -> &'a GmcApiHeader {
+        self.header
+    }
+
+    /// Returns the payload of the message.
+    ///
+    /// The second slice is empty unless the message loops over the message queue.
+    pub(crate) fn payload(&self) -> (&'a [u8], &'a [u8]) {
+        self.contents
+    }
+
+    /// Returns the status code of this message if it is the reply to `command` sent with sequence
+    /// number `seq`, `None` otherwise.
+    fn is_reply_to(&self, command: GmcCommandId, seq: u32) -> Option<u32> {
+        if matches!(self.header.command().id(), Ok(id) if id == command)
+            && self.header.sequence() == u64::from(seq)
+        {
+            self.header.status()
+        } else {
+            None
+        }
+    }
+
+    /// Logs an event, meaning a message that no caller was waiting for.
+    ///
+    /// A reply that nobody claimed (e.g. one that arrived after its request timed out) and an
+    /// unknown command id are logged at warning level. Other events are not logged.
+    pub(super) fn log(&self, dev: &device::Device) {
+        let command = self.header.command();
+        let seq = self.header.sequence();
+
+        match command.id() {
+            Ok(id) if command.is_response() => {
+                dev_warn!(dev, "unclaimed GSP GMC reply to {:?} (seq {})\n", id, seq);
+            }
+            // Nothing to do for the remaining known command ids.
+            Ok(_) => {}
+            Err(_) => {
+                dev_warn!(dev, "unknown GSP GMC command {:?} (seq {})\n", command, seq);
+            }
+        }
+    }
+}
+
 impl Cmdq<'_> {
+    /// Sends `command` to the GSP and waits for the reply.
+    ///
+    /// GMC events that arrive before the reply are passed to `on_event`, and RPC messages are
+    /// logged and consumed. The boot sequence relies on this to handle the load-and-execute
+    /// events that GSP-RM raises before it answers `GSP_INIT`.
+    ///
+    /// The queue is locked for the entire send+receive cycle, including the calls to `on_event`,
+    /// to ensure that no other command can be interleaved. `on_event` must thus not use the queue.
+    ///
+    /// # Errors
+    ///
+    /// - `EMSGSIZE` if the command does not fit in a single command queue element.
+    /// - `ETIMEDOUT` if space does not become available to send the command, or if the reply does
+    ///   not arrive within [`Self::RECEIVE_TIMEOUT`] of the send, however many events arrive
+    ///   while waiting.
+    /// - `EIO` if the variable payload requested by the command has not been entirely
+    ///   written to by its [`GmcCommand::init_variable_payload`] method, or if the reply carries
+    ///   a status other than `NV_OK`.
+    ///
+    /// Error codes returned by the command and reply initializers, and by `on_event`, are
+    /// propagated as-is.
+    pub(crate) fn send_gmc_command<C>(
+        &self,
+        command: C,
+        on_event: impl Fn(&GmcMessage<'_>) -> Result,
+    ) -> Result<C::Reply>
+    where
+        C: CommandToGsp,
+        C::Reply: MessageFromGsp,
+        Error: From<C::InitError>,
+        Error: From<<C::Reply as MessageFromGsp>::InitError>,
+    {
+        let mut inner = self.inner.lock();
+        let seq = inner.send_gmc_command(command)?;
+
+        inner.await_gmc_reply(seq, on_event)
+    }
+
     /// Sends `command` to the GSP without waiting for a reply.
     ///
     /// # Errors
@@ -191,4 +353,70 @@ fn send_gmc_command<C>(&mut self, command: C) -> Result<u32>
     {
         self.send_command_element(GmcCommandElement(command))
     }
+
+    /// Receives the reply of type `R` to the command sent with sequence number `seq`, waiting up
+    /// to [`Cmdq::RECEIVE_TIMEOUT`] from the call.
+    ///
+    /// Any GMC event that arrives first is passed to `on_event`, and any RPC message is logged.
+    /// Neither extends the deadline.
+    ///
+    /// # Errors
+    ///
+    /// - `ETIMEDOUT` if the reply does not arrive before the deadline, however many other messages
+    ///   arrive while waiting.
+    /// - `EIO` if a message fails framing or checksum validation, or if the reply carries a status
+    ///   other than `NV_OK`.
+    ///
+    /// Error codes returned by [`MessageFromGsp::read`] and by `on_event` are propagated as-is.
+    fn await_gmc_reply<R: MessageFromGsp>(
+        &mut self,
+        seq: u32,
+        on_event: impl Fn(&GmcMessage<'_>) -> Result,
+    ) -> Result<R>
+    where
+        // This allows all error types, including `Infallible`, to be used for `R::InitError`.
+        Error: From<R::InitError>,
+    {
+        let dev = self.dev;
+
+        self.await_msg_with(|message| {
+            let gmc_message = match message {
+                GspParsedMessage::Gmc(gmc_message) => gmc_message,
+                _ => {
+                    message.log(dev);
+                    return Err(ENOMSG);
+                }
+            };
+
+            match gmc_message.is_reply_to(R::COMMAND, seq) {
+                // Message is not the reply to our command, log it.
+                None => {
+                    on_event(&gmc_message)?;
+                    return Err(ENOMSG);
+                }
+                // Reply has an error status.
+                Some(status @ 1..) => {
+                    dev_err!(
+                        dev,
+                        "GSP GMC: command {:?} failed with status {:#x}\n",
+                        R::COMMAND,
+                        status
+                    );
+
+                    return Err(EIO);
+                }
+                // Reply indicates the command has succeeded.
+                Some(0) => (),
+            }
+
+            let mut sbuffer =
+                SBufferIter::new_reader([gmc_message.contents.0, gmc_message.contents.1]);
+
+            R::read(&mut sbuffer).map_err(|e| e.into()).inspect(|_| {
+                if !sbuffer.is_empty() {
+                    dev_warn!(dev, "GSP GMC reply {:?} has unprocessed data\n", R::COMMAND);
+                }
+            })
+        })
+    }
 }
diff --git a/drivers/gpu/nova-core/gsp/cmdq/rpc.rs b/drivers/gpu/nova-core/gsp/cmdq/rpc.rs
index 36cba7c28d3e..de3734daa99e 100644
--- a/drivers/gpu/nova-core/gsp/cmdq/rpc.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq/rpc.rs
@@ -387,7 +387,13 @@ fn await_rpc_msg<M: MessageFromGsp>(&mut self) -> Result<M>
         Error: From<M::InitError>,
     {
         let f = |message: GspParsedMessage<'_>| {
-            let GspParsedMessage::Rpc(rpc_message) = message;
+            let rpc_message = match message {
+                GspParsedMessage::Rpc(rpc_message) => rpc_message,
+                _ => {
+                    message.log(self.dev);
+                    return Err(ENOMSG);
+                }
+            };
             let function = rpc_message.header.function();
             if matches!(function, Ok(f) if f == M::FUNCTION) {
                 let (cmd, contents_1) =
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index 367c8b5e3443..de61afd60403 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -935,7 +935,6 @@ pub(crate) struct GmcCommandWord(u32) {
 /// In a queue element, it follows the queue element header and precedes the payload of the
 /// command.
 #[repr(transparent)]
-#[expect(dead_code)]
 pub(crate) struct GmcApiHeader {
     inner: r000_00::GMCAPI_HEADER,
 }

-- 
2.56.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] gpu: nova-core: return type mismatch in CmdqInner::send_gmc_command()
--

commit 1e1d150084c3f6fc3d6e13ffe004caffa1b2a4a0
Author: John Hubbard <jhubbard@nvidia.com>

gpu: nova-core: gsp: cmdq: add the GMC send path

This commit adds the gmc submodule to the GSP command queue in nova-core to
support sending GPU Management Controller (GMC) API commands such as GSP_INIT.

> 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 0000000000000..b035f30479a84
> --- /dev/null
> +++ b/drivers/gpu/nova-core/gsp/cmdq/gmc.rs
> @@ -0,0 +1,194 @@
[ ... ]
> +impl CmdqInner<'_> {
> +    /// Sends `command` to the GSP, and returns the sequence number of its element.
> +    ///
> +    /// Unlike RPC commands, GMC commands are never split: a command that does not fit in a single
> +    /// element is an error.
> +    ///
> +    /// # Errors
> +    ///
> +    /// - `EMSGSIZE` if the command does not fit in a single command queue element.
> +    /// - `ETIMEDOUT` if space does not become available within the timeout.
> +    /// - `EIO` if the variable payload requested by the command has not been entirely
> +    ///   written to by its [`CommandToGsp::init_variable_payload`] method.
> +    ///
> +    /// Error codes returned by the command initializer are propagated as-is.
> +    fn send_gmc_command<C>(&mut self, command: C) -> Result<u32>
> +    where
> +        C: CommandToGsp,
> +        Error: From<C::InitError>,
> +    {
> +        self.send_command_element(GmcCommandElement(command))
> +    }
> +}

[Severity: Low]
Does this compile when CONFIG_DRM_NOVA is enabled?

In drivers/gpu/nova-core/gsp/cmdq/gmc.rs, CmdqInner::send_gmc_command() is
declared with a return type of Result<u32> and directly returns the result of
self.send_command_element().

However, in drivers/gpu/nova-core/gsp/cmdq.rs, CmdqInner::send_command_element()
returns Result (i.e. Result<(), Error>):

    fn send_command_element<E: CommandElement>(&mut self, command: E) -> Result {
        ...
        Ok(())
    }

Wouldn't this fail to build with a mismatched types error?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-r000_rebase_on_cmdq-rpc-v1-0-189f9ff3bfd2@nvidia.com?part=3

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] gpu: nova-core: gsp: type mismatch in CmdqInner::send_gmc_command()
- [Medium] gpu: nova-core: gsp: unclaimed replies passed to on_event in await_gmc_reply()
--

commit 54e27415bb52450de6eeae48a370384ffd8a7d77
Author: John Hubbard <jhubbard@nvidia.com>

gpu: nova-core: gsp: cmdq: add the GMC receive path

This commit adds the receive side of the GMC sub-module in nova-core so the
driver can parse GMC messages, handle GMC events, and wait for GMC command
replies from GSP-RM.

> diff --git a/drivers/gpu/nova-core/gsp/cmdq/gmc.rs b/drivers/gpu/nova-core/gsp/cmdq/gmc.rs
> index b035f30479a84..9adba5cbcd5c8 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq/gmc.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq/gmc.rs

[ ... ]

> @@ -150,7 +152,167 @@ fn write(&self, dev: &device::Device, seq: u32, dst: &mut GspCommand<'_>) -> Res

[ ... ]

> +    ///Logs an event, meaning a message that no caller was waiting for.
> +    ///
> +    /// A reply that nobody claimed (e.g. one that arrived after its request timed out) and an
> +    /// unknown command id are logged at warning level. Other events are not logged.
> +    pub(super) fn log(&self, dev: &device::Device) {
> +        let command = self.header.command();
> +        let seq = self.header.sequence();
> +
> +        match command.id() {
> +            Ok(id) if command.is_response() => {
> +                dev_warn!(dev, "unclaimed GSP GMC reply to {:?} (seq {})\n", id, seq);
> +            }
> +            // Nothing to do for the remaining known command ids.
> +            Ok(_) => {}
> +            Err(_) => {
> +                dev_warn!(dev, "unknown GSP GMC command {:?} (seq {})\n", command, seq);
> +            }
> +        }
> +    }
> +}
> +
>  impl Cmdq<'_> {
> +    /// Sends `command` to the GSP and waits for the reply.

[ ... ]

> +    pub(crate) fn send_gmc_command<C>(
> +        &self,
> +        command: C,
> +        on_event: impl Fn(&GmcMessage<'_>) -> Result,
> +    ) -> Result<C::Reply>
> +    where
> +        C: CommandToGsp,
> +        C::Reply: MessageFromGsp,
> +        Error: From<C::InitError>,
> +        Error: From<<C::Reply as MessageFromGsp>::InitError>,
> +    {
> +        let mut inner = self.inner.lock();
> +        let seq = inner.send_gmc_command(command)?;
> +
> +        inner.await_gmc_reply(seq, on_event)
> +    }
> +
>      /// Sends `command` to the GSP without waiting for a reply.
> @@ -191,4 +353,70 @@ fn send_gmc_command<C>(&mut self, command: C) -> Result<u32>
>      {
>          self.send_command_element(GmcCommandElement(command))
>      }

[Severity: High]
Does CmdqInner::send_gmc_command() compile and return the sequence number as
expected here?

Cmdq::send_gmc_command() calls inner.send_gmc_command(command)? to obtain
seq: u32 and pass it to inner.await_gmc_reply(seq, on_event). However,
CmdqInner::send_gmc_command() returns the result of
self.send_command_element(GmcCommandElement(command)), and in
drivers/gpu/nova-core/gsp/cmdq.rs, CmdqInner::send_command_element() has
return type Result (i.e. Result<(), Error>):

drivers/gpu/nova-core/gsp/cmdq.rs:CmdqInner::send_command_element() {
    ...
        let elem_count = dst.header.element_count();
        self.seq += 1;
        self.gsp_mem.advance_cpu_write_ptr(elem_count);
        Cmdq::notify_gsp(self.bar);

        Ok(())
    }

Because rustc type-checks generic function bodies even when unused, won't
this fail to build with a mismatched types error (expected Result<u32, Error>,
found Result<(), Error>) unless send_command_element() or send_gmc_command()
returns the sequence number of the sent element?

> +
> +    /// Receives the reply of type `R` to the command sent with sequence number `seq`, waiting up
> +    /// to [`Cmdq::RECEIVE_TIMEOUT`] from the call.

[ ... ]

> +    fn await_gmc_reply<R: MessageFromGsp>(
> +        &mut self,
> +        seq: u32,
> +        on_event: impl Fn(&GmcMessage<'_>) -> Result,
> +    ) -> Result<R>
> +    where
> +        // This allows all error types, including `Infallible`, to be used for `R::InitError`.
> +        Error: From<R::InitError>,
> +    {
> +        let dev = self.dev;
> +
> +        self.await_msg_with(|message| {
> +            let gmc_message = match message {
> +                GspParsedMessage::Gmc(gmc_message) => gmc_message,
> +                _ => {
> +                    message.log(dev);
> +                    return Err(ENOMSG);
> +                }
> +            };
> +
> +            match gmc_message.is_reply_to(R::COMMAND, seq) {
> +                // Message is not the reply to our command, log it.
> +                None => {
> +                    on_event(&gmc_message)?;
> +                    return Err(ENOMSG);
> +                }

[Severity: Medium]
When gmc_message.is_reply_to(R::COMMAND, seq) returns None in
CmdqInner::await_gmc_reply(), can an unclaimed GMC reply to an earlier
command or an unknown GMC command be passed to on_event() instead of being
logged?

In drivers/gpu/nova-core/gsp/fw.rs, GmcCommandWord::is_response() is set on
responses to requests and not set on events, and GmcMessage::log() checks
command.is_response() and command.id() to log unclaimed replies and unknown
commands at warning level.

Here, the comment says "Message is not the reply to our command, log it.",
but the None arm calls on_event(&gmc_message)? without checking whether the
message is a response or calling gmc_message.log(dev) (which is pub(super)
and not callable from outside the cmdq module). Should unclaimed responses
be logged via gmc_message.log(dev) rather than forwarded to on_event()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261103-r000_rebase_on_cmdq-rpc-v1-0-189f9ff3bfd2@nvidia.com?part=4