Re: [PATCH v2 07/31] gpu: nova-core: distinguish async GSP RPC traffic in debug logs

[Alexandre Courbot]

On Sat Aug 22, 2026 at 10:54 AM JST, John Hubbard wrote:
> The receive path logged every message at the transport layer and did not
> distinguish events from command replies. Async sends shared the RPC
> sequence with waited-for commands, and GSP-initiated events arrived
> with that field unset.
>
> Print a distinct debug line for async send, sync send, event, and
> command reply. Number async sends and events with driver-local counters,
> which wrap rather than trap, since they only number log lines.
>
> IS_ASYNC selects only the send log line. Every command still carries the
> next RPC sequence on the wire.
>
> Assisted-by: Cursor:claude-opus-5
> Reviewed-by: Timur Tabi <ttabi@xxxxxxxxxx>
> Reviewed-by: Zhi Wang <zhiw@xxxxxxxxxx>
> Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
> ---
>  drivers/gpu/nova-core/gsp/cmdq.rs             | 111 ++++++++++++++----
>  .../gpu/nova-core/gsp/cmdq/continuation.rs    |   1 +
>  drivers/gpu/nova-core/gsp/commands.rs         |   2 +
>  drivers/gpu/nova-core/gsp/fw.rs               |  19 +++
>  4 files changed, 107 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index ac3e6642031a..1eeef2120b6e 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -87,6 +87,12 @@ pub(crate) trait CommandToGsp {
>      /// Function identifying this command to the GSP.
>      const FUNCTION: MsgFunction;
>  
> +    /// Classifies the send-side debug log as an async send.
> +    ///
> +    /// Default is `false`. Set this only on commands that are sent without waiting for a reply. A
> +    /// [`NoReply`] continuation of a waited-for command stays `false`.
> +    const IS_ASYNC: bool = false;
> +
>      /// Type generated by [`CommandToGsp::init`], to be written into the command queue buffer.
>      type Command: FromBytes + AsBytes;
>  
> @@ -528,6 +534,8 @@ pub(crate) fn new(dev: &device::Device<device::Bound>) -> impl PinInit<Self, Err
>                      gsp_mem,
>                      elem_seq: 0,
>                      rpc_seq: 0,
> +                    tx_async_seq: 0,
> +                    rx_event_seq: 0,
>                      poisoned: Cell::new(false),
>                  }),
>              }))
> @@ -672,6 +680,12 @@ struct CmdqInner {
>      /// [`CmdqInner::receive_msg`] match that reply to the awaiting command. Advances once per
>      /// logical command.
>      rpc_seq: u32,
> +    /// Debug-log sequence for async sends. Those commands do not wait for a reply, so this
> +    /// counter is the number printed in the send log.
> +    tx_async_seq: u32,
> +    /// Debug-log sequence for GSP-initiated events. The GSP leaves the RPC sequence unset on
> +    /// those messages, so the driver numbers them itself.
> +    rx_event_seq: u32,

What do these counters give us concretely? I don't understand how they
are useful for debugging since we already have sequence numbers for the
commands.

>      /// Set once a message with corrupt framing or a bad checksum is seen. Such a message has an
>      /// untrusted length, so the queue cannot be advanced past it, and every later receive fails
>      /// until the queue is torn down and reset.
> @@ -738,13 +752,24 @@ fn send_single_command<M>(&mut self, bar: Bar0<'_>, command: M, rpc_seq: u32) ->
>                  dst.contents.1,
>              ])));
>  
> -        dev_dbg!(
> -            &self.dev,
> -            "GSP RPC: send: seq# {}, function={:?}, length=0x{:x}\n",
> -            rpc_seq,
> -            M::FUNCTION,
> -            dst.header.length(),
> -        );
> +        if M::IS_ASYNC {
> +            dev_dbg!(
> +                &self.dev,
> +                "GSP RPC: async send: seq# {}, function={:?}, length=0x{:x}\n",
> +                self.tx_async_seq,
> +                M::FUNCTION,
> +                dst.header.length(),
> +            );
> +            self.tx_async_seq = self.tx_async_seq.wrapping_add(1);
> +        } else {
> +            dev_dbg!(
> +                &self.dev,
> +                "GSP RPC: send: seq# {}, function={:?}, length=0x{:x}\n",
> +                rpc_seq,
> +                M::FUNCTION,
> +                dst.header.length(),
> +            );
> +        }

These two `dev_dbg!` statements are almost identical, can we
differentiate the relevant part with a custom string and `fmt!`?

<...>
> diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
> index 61fe93db9e7e..d5575c036eb9 100644
> --- a/drivers/gpu/nova-core/gsp/commands.rs
> +++ b/drivers/gpu/nova-core/gsp/commands.rs
> @@ -52,6 +52,7 @@ pub(crate) fn new(pdev: &'a pci::Device<device::Bound>, chipset: Chipset) -> Sel
>  
>  impl<'a> CommandToGsp for SetSystemInfo<'a> {
>      const FUNCTION: MsgFunction = MsgFunction::GspSetSystemInfo;
> +    const IS_ASYNC: bool = true;
>      type Command = fw::commands::GspSetSystemInfo;
>      type Reply = NoReply;
>      type InitError = Error;
> @@ -122,6 +123,7 @@ pub(crate) fn new(vgpu_state: VgpuState) -> Result<Self> {
>  
>  impl CommandToGsp for SetRegistry {
>      const FUNCTION: MsgFunction = MsgFunction::SetRegistry;
> +    const IS_ASYNC: bool = true;
>      type Command = fw::commands::PackedRegistryTable;
>      type Reply = NoReply;
>      type InitError = Infallible;

Async commands are effectively removed by the end of this series, so I'd
question the need to add this distinction here. This looks to me like
this patch should come *after* the switch to r000 to make sure that all
of its premises still hold.

So unless there is a strong need for it I'd wager it is best to drop
this patch for now as it looks more like a drive-by feature than a
preliminary need for r000 support.

> diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
> index d3678f16750a..857d53c77297 100644
> --- a/drivers/gpu/nova-core/gsp/fw.rs
> +++ b/drivers/gpu/nova-core/gsp/fw.rs
> @@ -359,6 +359,25 @@ fn try_from(value: u32) -> Result<MsgFunction> {
>      }
>  }
>  
> +impl MsgFunction {
> +    /// Returns true if this is a GSP-initiated async event (`NV_VGPU_MSG_EVENT_*`), as opposed to
> +    /// a command response (`NV_VGPU_MSG_FUNCTION_*`).
> +    pub(crate) fn is_event(&self) -> bool {
> +        matches!(
> +            self,
> +            Self::GspInitDone
> +                | Self::GspRunCpuSequencer
> +                | Self::PostEvent
> +                | Self::RcTriggered
> +                | Self::MmuFaultQueued
> +                | Self::OsErrorLog
> +                | Self::GspPostNoCat
> +                | Self::GspLockdownNotice
> +                | Self::UcodeLibOsPrint //
> +        )
> +    }
> +}

There is a `NV_VGPU_MSG_EVENT_FIRST_EVENT` value in the bindings that
can be used to classify events without resorting to an exhaustive and
error-prone list.