[PATCH v3 0/5] gpu: nova-core: gsp: add locking to Cmdq

[PATCH v3 0/5] gpu: nova-core: gsp: add locking to Cmdq

[Eliot Courtney]

Add locking to Cmdq. This is required e.g. for unloading the driver,
which needs to send the UnloadingGuestDriver via the command queue
on unbind which may be on a different thread.

We have commands that need a reply and commands that don't. For
commands with a reply we want to make sure that they don't get
the reply of a different command back. The approach this patch series
takes is by making those commands block until they get a response. For
now this should be ok, and we expect GSP to be fast anyway.

To do this, we need to know which commands expect a reply and which
don't. John's existing series[1] adds IS_ASYNC which solves part of the
problem, but we need to know a bit more. So instead, add an
associated type called Reply which tells us what the reply is.

An alternative would be to define traits inheriting CommandToGsp, e.g.
CommandWithReply and CommandWithoutReply, instead of using the
associated type. I implemented the associated type version because it
feels more compositional rather than inherity so seemed a bit better
to me. But both of these approaches work and are fine, IMO.

In summary, this patch series has three steps:
1. Add the type infrastructure to know what replies are expected for a
   command and update each caller to explicitly wait for the reply or
   not.
2. Make Cmdq pinned so we can use Mutex
3. Add a Mutex to protect Cmdq by moving the relevant state to an
   inner struct.

[1]: https://lore.kernel.org/all/20260211000451.192109-1-jhubbard@nvidia.com/

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
Changes in v3:
- Rename send_sync_command/send_async_command to
  send_command/send_command_no_wait.
- Move `dev` field into `CmdqInner` to avoid passing it through method
  parameters.
- Add `RECEIVE_TIMEOUT` constant for the 10s receive timeout.
- Link to v2: https://lore.kernel.org/r/20260226-cmdq-locking-v2-0-c7e16a6d5885@nvidia.com

Changes in v2:
- Rebase on drm-rust-next
- Link to v1: https://lore.kernel.org/r/20260225-cmdq-locking-v1-0-bbf6b4156706@nvidia.com

---
Eliot Courtney (5):
      gpu: nova-core: gsp: fix stale doc comments on command queue methods
      gpu: nova-core: gsp: add `RECEIVE_TIMEOUT` constant for command queue
      gpu: nova-core: gsp: add reply/no-reply info to `CommandToGsp`
      gpu: nova-core: gsp: make `Cmdq` a pinned type
      gpu: nova-core: gsp: add mutex locking to Cmdq

 drivers/gpu/nova-core/gsp.rs                   |   5 +-
 drivers/gpu/nova-core/gsp/boot.rs              |  13 +-
 drivers/gpu/nova-core/gsp/cmdq.rs              | 225 +++++++++++++++++--------
 drivers/gpu/nova-core/gsp/cmdq/continuation.rs |   5 +-
 drivers/gpu/nova-core/gsp/commands.rs          |  23 +--
 drivers/gpu/nova-core/gsp/sequencer.rs         |   4 +-
 6 files changed, 180 insertions(+), 95 deletions(-)
---
base-commit: 4a49fe23e357b48845e31fe9c28a802c05458198
change-id: 20260225-cmdq-locking-d32928a2c2cf
prerequisite-message-id: <20260304-cmdq-continuation-v5-0-3f19d759ed93@nvidia.com>
prerequisite-patch-id: fd45bc5b8eda5e2b54a052dddb1a1c363107f0a7
prerequisite-patch-id: 06fe65f900206c44b5ba52286ca4ce1ca42b55d5
prerequisite-patch-id: 8844970d0e387488c70979a73732579ba174b46c
prerequisite-patch-id: e138a94ed48fa8d50d5ed1eb36524f98923ed478
prerequisite-patch-id: dccf2b12b176947e89b44baafda9c5a0aa0a93bc
prerequisite-patch-id: 30ed64c398e541d6efbcb2e46ed9a9e6cf953f4f
prerequisite-patch-id: ba1c8da0cbdb4682b879633a94a172d1b2b6bc8e
prerequisite-patch-id: 081d4a4198a0bf09f3480cb8baf296db585decce
prerequisite-patch-id: 56c8c25e7362178cd019c8f03954a6bcdb72b1b5

Best regards,
-- 
Eliot Courtney <ecourtney@nvidia.com>

[PATCH v3 1/5] gpu: nova-core: gsp: fix stale doc comments on command queue methods

[Eliot Courtney]

Fix some inaccuracies / old doc comments.

Reviewed-by: Zhi Wang <zhiw@nvidia.com>
Tested-by: Zhi Wang <zhiw@nvidia.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 492e9489e808..4829830b6921 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -531,6 +531,7 @@ fn notify_gsp(bar: &Bar0) {
     ///
     /// # Errors
     ///
+    /// - `EMSGSIZE` if the command exceeds the maximum queue element size.
     /// - `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.
@@ -711,22 +712,20 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
 
     /// Receive a message from the GSP.
     ///
-    /// `init` is a closure tasked with processing the message. It receives a reference to the
-    /// message in the message queue, and a [`SBufferIter`] pointing to its variable-length
-    /// payload, if any.
+    /// The expected message type is specified using the `M` generic parameter. If the pending
+    /// message has a different function code, `ERANGE` is returned and the message is consumed.
     ///
-    /// The expected message is specified using the `M` generic parameter. If the pending message
-    /// is different, `EAGAIN` is returned and the unexpected message is dropped.
-    ///
-    /// This design is by no means final, but it is simple and will let us go through GSP
-    /// initialization.
+    /// The read pointer is always advanced past the message, regardless of whether it matched.
     ///
     /// # Errors
     ///
     /// - `ETIMEDOUT` if `timeout` has elapsed before any message becomes available.
     /// - `EIO` if there was some inconsistency (e.g. message shorter than advertised) on the
     ///   message queue.
-    /// - `EINVAL` if the function of the message was unrecognized.
+    /// - `EINVAL` if the function code of the message was not recognized.
+    /// - `ERANGE` if the message had a recognized but non-matching function code.
+    ///
+    /// Error codes returned by [`MessageFromGsp::read`] are propagated as-is.
     pub(crate) fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M>
     where
         // This allows all error types, including `Infallible`, to be used for `M::InitError`.

-- 
2.53.0

Re: [PATCH v3 1/5] gpu: nova-core: gsp: fix stale doc comments on command queue methods

[Gary Guo]

On Wed Mar 4, 2026 at 2:46 AM GMT, Eliot Courtney wrote:
> Fix some inaccuracies / old doc comments.
>
> Reviewed-by: Zhi Wang <zhiw@nvidia.com>
> Tested-by: Zhi Wang <zhiw@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  drivers/gpu/nova-core/gsp/cmdq.rs | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index 492e9489e808..4829830b6921 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -531,6 +531,7 @@ fn notify_gsp(bar: &Bar0) {
>      ///
>      /// # Errors
>      ///
> +    /// - `EMSGSIZE` if the command exceeds the maximum queue element size.
>      /// - `ETIMEDOUT` if space does not become available within the timeout.
>      /// - `EIO` if the variable payload requested by the command has not been entirely

What's the benefit of enumerating all the error codes like this? Unless all the
mentioned error code here is supposed to be handled, then it doesn't gain much
for them to be mentioned, no?

For the errors that do need special handling, we probably want to use enums to
force their handling.

Best,
Gary


>      ///   written to by its [`CommandToGsp::init_variable_payload`] method.
> @@ -711,22 +712,20 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
>  
>      /// Receive a message from the GSP.
>      ///
> -    /// `init` is a closure tasked with processing the message. It receives a reference to the
> -    /// message in the message queue, and a [`SBufferIter`] pointing to its variable-length
> -    /// payload, if any.
> +    /// The expected message type is specified using the `M` generic parameter. If the pending
> +    /// message has a different function code, `ERANGE` is returned and the message is consumed.
>      ///
> -    /// The expected message is specified using the `M` generic parameter. If the pending message
> -    /// is different, `EAGAIN` is returned and the unexpected message is dropped.
> -    ///
> -    /// This design is by no means final, but it is simple and will let us go through GSP
> -    /// initialization.
> +    /// The read pointer is always advanced past the message, regardless of whether it matched.
>      ///
>      /// # Errors
>      ///
>      /// - `ETIMEDOUT` if `timeout` has elapsed before any message becomes available.
>      /// - `EIO` if there was some inconsistency (e.g. message shorter than advertised) on the
>      ///   message queue.
> -    /// - `EINVAL` if the function of the message was unrecognized.
> +    /// - `EINVAL` if the function code of the message was not recognized.
> +    /// - `ERANGE` if the message had a recognized but non-matching function code.
> +    ///
> +    /// Error codes returned by [`MessageFromGsp::read`] are propagated as-is.
>      pub(crate) fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M>
>      where
>          // This allows all error types, including `Infallible`, to be used for `M::InitError`.

Re: [PATCH v3 1/5] gpu: nova-core: gsp: fix stale doc comments on command queue methods

[Alexandre Courbot]

On Wed Mar 4, 2026 at 8:25 PM JST, Gary Guo wrote:
> On Wed Mar 4, 2026 at 2:46 AM GMT, Eliot Courtney wrote:
>> Fix some inaccuracies / old doc comments.
>>
>> Reviewed-by: Zhi Wang <zhiw@nvidia.com>
>> Tested-by: Zhi Wang <zhiw@nvidia.com>
>> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
>> ---
>>  drivers/gpu/nova-core/gsp/cmdq.rs | 17 ++++++++---------
>>  1 file changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
>> index 492e9489e808..4829830b6921 100644
>> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
>> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
>> @@ -531,6 +531,7 @@ fn notify_gsp(bar: &Bar0) {
>>      ///
>>      /// # Errors
>>      ///
>> +    /// - `EMSGSIZE` if the command exceeds the maximum queue element size.
>>      /// - `ETIMEDOUT` if space does not become available within the timeout.
>>      /// - `EIO` if the variable payload requested by the command has not been entirely
>
> What's the benefit of enumerating all the error codes like this? Unless all the
> mentioned error code here is supposed to be handled, then it doesn't gain much
> for them to be mentioned, no?
>
> For the errors that do need special handling, we probably want to use enums to
> force their handling.

Agreed about using enums, that's a change we will want to do driver-wide
at some point. Preferably once we have an equivalent of `thiserror` to
make the transition easier.

Meanwhile I think it is expected to document the returned error codes, as
(1) they provide an easy way to lookup the reason for runtime errors instead of
grepping the code, and (2) they can eventually be converted into the
doccomments of the enum error types once we switch to them.

Claude review: gpu: nova-core: gsp: fix stale doc comments on command queue methods

[Claude Code Review Bot]

Patch Review

Straightforward doc cleanup. The changes accurately reflect the current behavior:

- Adding `EMSGSIZE` to the error list for `send_command_internal` (line 166)
- Updating `receive_msg` docs to clarify that `ERANGE` is returned for recognized-but-non-matching function codes, replacing the old `EAGAIN` reference (lines 177-196)
- Removing the "not final" caveat which no longer applies

No issues. Clean and correct.

---
Generated by Claude Code Patch Reviewer

[PATCH v3 2/5] gpu: nova-core: gsp: add `RECEIVE_TIMEOUT` constant for command queue

[Eliot Courtney]

Remove magic numbers and add a default timeout for callers to use.

Tested-by: Zhi Wang <zhiw@nvidia.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs      | 3 +++
 drivers/gpu/nova-core/gsp/commands.rs  | 5 ++---
 drivers/gpu/nova-core/gsp/sequencer.rs | 2 +-
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 4829830b6921..0192c85ddd75 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -496,6 +496,9 @@ impl Cmdq {
     /// Timeout for waiting for space on the command queue.
     const ALLOCATE_TIMEOUT: Delta = Delta::from_secs(1);
 
+    /// Default timeout for receiving a message from the GSP.
+    pub(super) const RECEIVE_TIMEOUT: Delta = Delta::from_secs(10);
+
     /// Creates a new command queue for `dev`.
     pub(crate) fn new(dev: &device::Device<device::Bound>) -> Result<Cmdq> {
         let gsp_mem = DmaGspMem::new(dev)?;
diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index 8f270eca33be..88df117ba575 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -11,7 +11,6 @@
     device,
     pci,
     prelude::*,
-    time::Delta,
     transmute::{
         AsBytes,
         FromBytes, //
@@ -165,7 +164,7 @@ fn read(
 /// Waits for GSP initialization to complete.
 pub(crate) fn wait_gsp_init_done(cmdq: &mut Cmdq) -> Result {
     loop {
-        match cmdq.receive_msg::<GspInitDone>(Delta::from_secs(10)) {
+        match cmdq.receive_msg::<GspInitDone>(Cmdq::RECEIVE_TIMEOUT) {
             Ok(_) => break Ok(()),
             Err(ERANGE) => continue,
             Err(e) => break Err(e),
@@ -235,7 +234,7 @@ pub(crate) fn get_gsp_info(cmdq: &mut Cmdq, bar: &Bar0) -> Result<GetGspStaticIn
     cmdq.send_command(bar, GetGspStaticInfo)?;
 
     loop {
-        match cmdq.receive_msg::<GetGspStaticInfoReply>(Delta::from_secs(5)) {
+        match cmdq.receive_msg::<GetGspStaticInfoReply>(Cmdq::RECEIVE_TIMEOUT) {
             Ok(info) => return Ok(info),
             Err(ERANGE) => continue,
             Err(e) => return Err(e),
diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs b/drivers/gpu/nova-core/gsp/sequencer.rs
index 0cfbedc47fcf..ce2b3bb05d22 100644
--- a/drivers/gpu/nova-core/gsp/sequencer.rs
+++ b/drivers/gpu/nova-core/gsp/sequencer.rs
@@ -358,7 +358,7 @@ pub(crate) struct GspSequencerParams<'a> {
 impl<'a> GspSequencer<'a> {
     pub(crate) fn run(cmdq: &mut Cmdq, params: GspSequencerParams<'a>) -> Result {
         let seq_info = loop {
-            match cmdq.receive_msg::<GspSequence>(Delta::from_secs(10)) {
+            match cmdq.receive_msg::<GspSequence>(Cmdq::RECEIVE_TIMEOUT) {
                 Ok(seq_info) => break seq_info,
                 Err(ERANGE) => continue,
                 Err(e) => return Err(e),

-- 
2.53.0

Re: [PATCH v3 2/5] gpu: nova-core: gsp: add `RECEIVE_TIMEOUT` constant for command queue

[Gary Guo]

On Wed Mar 4, 2026 at 2:46 AM GMT, Eliot Courtney wrote:
> Remove magic numbers and add a default timeout for callers to use.
> 
> Tested-by: Zhi Wang <zhiw@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  drivers/gpu/nova-core/gsp/cmdq.rs      | 3 +++
>  drivers/gpu/nova-core/gsp/commands.rs  | 5 ++---
>  drivers/gpu/nova-core/gsp/sequencer.rs | 2 +-
>  3 files changed, 6 insertions(+), 4 deletions(-)

Re: [PATCH v3 2/5] gpu: nova-core: gsp: add `RECEIVE_TIMEOUT` constant for command queue

[Alexandre Courbot]

On Wed Mar 4, 2026 at 11:46 AM JST, Eliot Courtney wrote:
> Remove magic numbers and add a default timeout for callers to use.
>
> Tested-by: Zhi Wang <zhiw@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  drivers/gpu/nova-core/gsp/cmdq.rs      | 3 +++
>  drivers/gpu/nova-core/gsp/commands.rs  | 5 ++---
>  drivers/gpu/nova-core/gsp/sequencer.rs | 2 +-
>  3 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index 4829830b6921..0192c85ddd75 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -496,6 +496,9 @@ impl Cmdq {
>      /// Timeout for waiting for space on the command queue.
>      const ALLOCATE_TIMEOUT: Delta = Delta::from_secs(1);
>  
> +    /// Default timeout for receiving a message from the GSP.
> +    pub(super) const RECEIVE_TIMEOUT: Delta = Delta::from_secs(10);

Agree with the idea, but let's settle on 5 seconds - GSP replies are
supposed to be fast and there should be no good reason for any command
to take longer than that (and potentially hang the whole system for the
same time).

Claude review: gpu: nova-core: gsp: add `RECEIVE_TIMEOUT` constant for command queue

[Claude Code Review Bot]

Patch Review

Extracts the magic `Delta::from_secs(10)` and `Delta::from_secs(5)` values into a single `RECEIVE_TIMEOUT` constant.

One note: the `get_gsp_info` caller previously used a 5-second timeout (`Delta::from_secs(5)`) while the others used 10 seconds:

```rust
-        match cmdq.receive_msg::<GetGspStaticInfoReply>(Delta::from_secs(5)) {
+        match cmdq.receive_msg::<GetGspStaticInfoReply>(Cmdq::RECEIVE_TIMEOUT) {
```

This silently increases the timeout for `get_gsp_info` from 5s to 10s. This is almost certainly fine (a 5s timeout was probably just picked arbitrarily, and 10s is more conservative), but it should be mentioned in the commit message since it's a behavioral change, not just a mechanical refactor.

The visibility `pub(super)` is appropriate.

---
Generated by Claude Code Patch Reviewer

[PATCH v3 3/5] gpu: nova-core: gsp: add reply/no-reply info to `CommandToGsp`

[Eliot Courtney]

Add type infrastructure to know what reply is expected from each
`CommandToGsp`. Uses a marker type `NoReply` which does not implement
`MessageFromGsp` to mark commands which don't expect a response.

Update `send_command` to wait for a reply and add `send_command_no_wait`
which sends a command that has no reply, without blocking.

This prepares for adding locking to the queue.

Tested-by: Zhi Wang <zhiw@nvidia.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/boot.rs              |  5 ++-
 drivers/gpu/nova-core/gsp/cmdq.rs              | 55 +++++++++++++++++++++++++-
 drivers/gpu/nova-core/gsp/cmdq/continuation.rs |  5 ++-
 drivers/gpu/nova-core/gsp/commands.rs          | 16 +++-----
 4 files changed, 67 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index c56029f444cb..991eb5957e3d 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -160,8 +160,9 @@ pub(crate) fn boot(
         dma_write!(wpr_meta[0] = GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
 
         self.cmdq
-            .send_command(bar, commands::SetSystemInfo::new(pdev))?;
-        self.cmdq.send_command(bar, commands::SetRegistry::new())?;
+            .send_command_no_wait(bar, commands::SetSystemInfo::new(pdev))?;
+        self.cmdq
+            .send_command_no_wait(bar, commands::SetRegistry::new())?;
 
         gsp_falcon.reset(bar)?;
         let libos_handle = self.libos.dma_handle();
diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 0192c85ddd75..7750f5792b21 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -51,6 +51,10 @@
     sbuffer::SBufferIter, //
 };
 
+/// Marker type representing the absence of a reply for a command. This does not implement
+/// `MessageFromGsp`.
+pub(crate) struct NoReply;
+
 /// Trait implemented by types representing a command to send to the GSP.
 ///
 /// The main purpose of this trait is to provide [`Cmdq::send_command`] with the information it
@@ -69,6 +73,10 @@ pub(crate) trait CommandToGsp {
     /// Type generated by [`CommandToGsp::init`], to be written into the command queue buffer.
     type Command: FromBytes + AsBytes;
 
+    /// Type of the reply expected from the GSP, or [`NoReply`] for commands that don't
+    /// have a reply.
+    type Reply;
+
     /// Error type returned by [`CommandToGsp::init`].
     type InitError;
 
@@ -610,7 +618,7 @@ fn send_single_command<M>(&mut self, bar: &Bar0, command: M) -> Result
     ///   written to by its [`CommandToGsp::init_variable_payload`] method.
     ///
     /// Error codes returned by the command initializers are propagated as-is.
-    pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
+    fn send_command_internal<M>(&mut self, bar: &Bar0, command: M) -> Result
     where
         M: CommandToGsp,
         Error: From<M::InitError>,
@@ -630,6 +638,51 @@ pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
         }
     }
 
+    /// Sends `command` to the GSP and waits for the reply.
+    ///
+    /// # Errors
+    ///
+    /// - `ETIMEDOUT` if space does not become available to send the command, or if the reply is
+    ///   not received 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 and reply initializers are propagated as-is.
+    pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result<M::Reply>
+    where
+        M: CommandToGsp,
+        M::Reply: MessageFromGsp,
+        Error: From<M::InitError>,
+        Error: From<<M::Reply as MessageFromGsp>::InitError>,
+    {
+        self.send_command_internal(bar, command)?;
+
+        loop {
+            match self.receive_msg::<M::Reply>(Self::RECEIVE_TIMEOUT) {
+                Ok(reply) => break Ok(reply),
+                Err(ERANGE) => continue,
+                Err(e) => break Err(e),
+            }
+        }
+    }
+
+    /// Sends `command` to the GSP without waiting for a reply.
+    ///
+    /// # Errors
+    ///
+    /// - `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 initializers are propagated as-is.
+    pub(crate) fn send_command_no_wait<M>(&mut self, bar: &Bar0, command: M) -> Result
+    where
+        M: CommandToGsp<Reply = NoReply>,
+        Error: From<M::InitError>,
+    {
+        self.send_command_internal(bar, command)
+    }
+
     /// Wait for a message to become available on the message queue.
     ///
     /// This works purely at the transport layer and does not interpret or validate the message
diff --git a/drivers/gpu/nova-core/gsp/cmdq/continuation.rs b/drivers/gpu/nova-core/gsp/cmdq/continuation.rs
index 637942917237..8a6bb8fa7e60 100644
--- a/drivers/gpu/nova-core/gsp/cmdq/continuation.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq/continuation.rs
@@ -6,7 +6,7 @@
 
 use kernel::prelude::*;
 
-use super::CommandToGsp;
+use super::{CommandToGsp, NoReply};
 
 use crate::{
     gsp::fw::{
@@ -63,6 +63,7 @@ fn new(data: &'a [u8]) -> Self {
 impl<'a> CommandToGsp for ContinuationRecord<'a> {
     const FUNCTION: MsgFunction = MsgFunction::ContinuationRecord;
     type Command = ();
+    type Reply = NoReply;
     type InitError = Infallible;
 
     fn init(&self) -> impl Init<Self::Command, Self::InitError> {
@@ -144,6 +145,7 @@ fn new(command: C, payload: KVVec<u8>) -> Self {
 impl<C: CommandToGsp> CommandToGsp for SplitCommand<C> {
     const FUNCTION: MsgFunction = C::FUNCTION;
     type Command = C::Command;
+    type Reply = C::Reply;
     type InitError = C::InitError;
 
     fn init(&self) -> impl Init<Self::Command, Self::InitError> {
@@ -206,6 +208,7 @@ fn new(len: usize) -> Result<Self> {
     impl CommandToGsp for TestPayload {
         const FUNCTION: MsgFunction = MsgFunction::Nop;
         type Command = TestHeader;
+        type Reply = NoReply;
         type InitError = Infallible;
 
         fn init(&self) -> impl Init<Self::Command, Self::InitError> {
diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index 88df117ba575..77054c92fcc2 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -23,7 +23,8 @@
         cmdq::{
             Cmdq,
             CommandToGsp,
-            MessageFromGsp, //
+            MessageFromGsp,
+            NoReply, //
         },
         fw::{
             commands::*,
@@ -48,6 +49,7 @@ pub(crate) fn new(pdev: &'a pci::Device<device::Bound>) -> Self {
 impl<'a> CommandToGsp for SetSystemInfo<'a> {
     const FUNCTION: MsgFunction = MsgFunction::GspSetSystemInfo;
     type Command = GspSetSystemInfo;
+    type Reply = NoReply;
     type InitError = Error;
 
     fn init(&self) -> impl Init<Self::Command, Self::InitError> {
@@ -99,6 +101,7 @@ pub(crate) fn new() -> Self {
 impl CommandToGsp for SetRegistry {
     const FUNCTION: MsgFunction = MsgFunction::SetRegistry;
     type Command = PackedRegistryTable;
+    type Reply = NoReply;
     type InitError = Infallible;
 
     fn init(&self) -> impl Init<Self::Command, Self::InitError> {
@@ -178,6 +181,7 @@ pub(crate) fn wait_gsp_init_done(cmdq: &mut Cmdq) -> Result {
 impl CommandToGsp for GetGspStaticInfo {
     const FUNCTION: MsgFunction = MsgFunction::GetGspStaticInfo;
     type Command = GspStaticConfigInfo;
+    type Reply = GetGspStaticInfoReply;
     type InitError = Infallible;
 
     fn init(&self) -> impl Init<Self::Command, Self::InitError> {
@@ -231,13 +235,5 @@ pub(crate) fn gpu_name(&self) -> core::result::Result<&str, GpuNameError> {
 
 /// Send the [`GetGspInfo`] command and awaits for its reply.
 pub(crate) fn get_gsp_info(cmdq: &mut Cmdq, bar: &Bar0) -> Result<GetGspStaticInfoReply> {
-    cmdq.send_command(bar, GetGspStaticInfo)?;
-
-    loop {
-        match cmdq.receive_msg::<GetGspStaticInfoReply>(Cmdq::RECEIVE_TIMEOUT) {
-            Ok(info) => return Ok(info),
-            Err(ERANGE) => continue,
-            Err(e) => return Err(e),
-        }
-    }
+    cmdq.send_command(bar, GetGspStaticInfo)
 }

-- 
2.53.0

Re: [PATCH v3 3/5] gpu: nova-core: gsp: add reply/no-reply info to `CommandToGsp`

[Gary Guo]

On Wed Mar 4, 2026 at 2:46 AM GMT, Eliot Courtney wrote:
> Add type infrastructure to know what reply is expected from each
> `CommandToGsp`. Uses a marker type `NoReply` which does not implement
> `MessageFromGsp` to mark commands which don't expect a response.
> 
> Update `send_command` to wait for a reply and add `send_command_no_wait`
> which sends a command that has no reply, without blocking.
> 
> This prepares for adding locking to the queue.
> 
> Tested-by: Zhi Wang <zhiw@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>

Reviewed-by: Gary Guo <gary@garyguo.net>

> ---
>  drivers/gpu/nova-core/gsp/boot.rs              |  5 ++-
>  drivers/gpu/nova-core/gsp/cmdq.rs              | 55 +++++++++++++++++++++++++-
>  drivers/gpu/nova-core/gsp/cmdq/continuation.rs |  5 ++-
>  drivers/gpu/nova-core/gsp/commands.rs          | 16 +++-----
>  4 files changed, 67 insertions(+), 14 deletions(-)

Re: [PATCH v3 3/5] gpu: nova-core: gsp: add reply/no-reply info to `CommandToGsp`

[Alexandre Courbot]

On Wed Mar 4, 2026 at 11:46 AM JST, Eliot Courtney wrote:
> Add type infrastructure to know what reply is expected from each
> `CommandToGsp`. Uses a marker type `NoReply` which does not implement
> `MessageFromGsp` to mark commands which don't expect a response.
>
> Update `send_command` to wait for a reply and add `send_command_no_wait`
> which sends a command that has no reply, without blocking.
>
> This prepares for adding locking to the queue.
>
> Tested-by: Zhi Wang <zhiw@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  drivers/gpu/nova-core/gsp/boot.rs              |  5 ++-
>  drivers/gpu/nova-core/gsp/cmdq.rs              | 55 +++++++++++++++++++++++++-
>  drivers/gpu/nova-core/gsp/cmdq/continuation.rs |  5 ++-
>  drivers/gpu/nova-core/gsp/commands.rs          | 16 +++-----
>  4 files changed, 67 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
> index c56029f444cb..991eb5957e3d 100644
> --- a/drivers/gpu/nova-core/gsp/boot.rs
> +++ b/drivers/gpu/nova-core/gsp/boot.rs
> @@ -160,8 +160,9 @@ pub(crate) fn boot(
>          dma_write!(wpr_meta[0] = GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
>  
>          self.cmdq
> -            .send_command(bar, commands::SetSystemInfo::new(pdev))?;
> -        self.cmdq.send_command(bar, commands::SetRegistry::new())?;
> +            .send_command_no_wait(bar, commands::SetSystemInfo::new(pdev))?;
> +        self.cmdq
> +            .send_command_no_wait(bar, commands::SetRegistry::new())?;
>  
>          gsp_falcon.reset(bar)?;
>          let libos_handle = self.libos.dma_handle();
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index 0192c85ddd75..7750f5792b21 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -51,6 +51,10 @@
>      sbuffer::SBufferIter, //
>  };
>  
> +/// Marker type representing the absence of a reply for a command. This does not implement
> +/// `MessageFromGsp`.

This is giving either too much or not enough implementation detail. :)

Without knowing why `MessageFromGsp` is not implemented, this can be
confusing to users who will wonder why we give them this information.
I'd remove that sentence and instead say something like "commands using
this as their reply type are sent using `send_command_no_wait`" or
something like that.

> +pub(crate) struct NoReply;
> +
>  /// Trait implemented by types representing a command to send to the GSP.
>  ///
>  /// The main purpose of this trait is to provide [`Cmdq::send_command`] with the information it
> @@ -69,6 +73,10 @@ pub(crate) trait CommandToGsp {
>      /// Type generated by [`CommandToGsp::init`], to be written into the command queue buffer.
>      type Command: FromBytes + AsBytes;
>  
> +    /// Type of the reply expected from the GSP, or [`NoReply`] for commands that don't
> +    /// have a reply.
> +    type Reply;
> +
>      /// Error type returned by [`CommandToGsp::init`].
>      type InitError;
>  
> @@ -610,7 +618,7 @@ fn send_single_command<M>(&mut self, bar: &Bar0, command: M) -> Result
>      ///   written to by its [`CommandToGsp::init_variable_payload`] method.
>      ///
>      /// Error codes returned by the command initializers are propagated as-is.
> -    pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
> +    fn send_command_internal<M>(&mut self, bar: &Bar0, command: M) -> Result
>      where
>          M: CommandToGsp,
>          Error: From<M::InitError>,
> @@ -630,6 +638,51 @@ pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
>          }
>      }
>  
> +    /// Sends `command` to the GSP and waits for the reply.
> +    ///
> +    /// # Errors
> +    ///
> +    /// - `ETIMEDOUT` if space does not become available to send the command, or if the reply is
> +    ///   not received 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 and reply initializers are propagated as-is.
> +    pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result<M::Reply>
> +    where
> +        M: CommandToGsp,
> +        M::Reply: MessageFromGsp,
> +        Error: From<M::InitError>,
> +        Error: From<<M::Reply as MessageFromGsp>::InitError>,
> +    {
> +        self.send_command_internal(bar, command)?;
> +
> +        loop {
> +            match self.receive_msg::<M::Reply>(Self::RECEIVE_TIMEOUT) {
> +                Ok(reply) => break Ok(reply),
> +                Err(ERANGE) => continue,
> +                Err(e) => break Err(e),
> +            }
> +        }

There is an opportunity for factorize some more code here.

Notice how the other callers of `receive_msg` (`wait_gsp_init_done` and
`GspSequencer::run`) both use the same kind of loop, down to the same
error handling. We could move that loop logic here and do it in a single
place.

In the future, we will probably want to add handlers for
unexpected messages from the GSP and it will be easier if we receive all
messages from a single place.

This can be a separate patch from this one, but I think it makes sense
to have that in this series.

I expect the last patch to change a bit as a consequence of that - maybe
we will need a `receive_msg_loop` or something in `CmdqInner`.

> +    }
> +
> +    /// Sends `command` to the GSP without waiting for a reply.
> +    ///
> +    /// # Errors
> +    ///
> +    /// - `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 initializers are propagated as-is.
> +    pub(crate) fn send_command_no_wait<M>(&mut self, bar: &Bar0, command: M) -> Result
> +    where
> +        M: CommandToGsp<Reply = NoReply>,
> +        Error: From<M::InitError>,
> +    {
> +        self.send_command_internal(bar, command)
> +    }
> +
>      /// Wait for a message to become available on the message queue.
>      ///
>      /// This works purely at the transport layer and does not interpret or validate the message
> diff --git a/drivers/gpu/nova-core/gsp/cmdq/continuation.rs b/drivers/gpu/nova-core/gsp/cmdq/continuation.rs
> index 637942917237..8a6bb8fa7e60 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq/continuation.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq/continuation.rs
> @@ -6,7 +6,7 @@
>  
>  use kernel::prelude::*;
>  
> -use super::CommandToGsp;
> +use super::{CommandToGsp, NoReply};

Nit: let's follow the formatting convention for imports.

Re: [PATCH v3 3/5] gpu: nova-core: gsp: add reply/no-reply info to `CommandToGsp`

[Eliot Courtney]

On Wed Mar 4, 2026 at 8:56 PM JST, Alexandre Courbot wrote:
>> +    /// Sends `command` to the GSP and waits for the reply.
>> +    ///
>> +    /// # Errors
>> +    ///
>> +    /// - `ETIMEDOUT` if space does not become available to send the command, or if the reply is
>> +    ///   not received 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 and reply initializers are propagated as-is.
>> +    pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result<M::Reply>
>> +    where
>> +        M: CommandToGsp,
>> +        M::Reply: MessageFromGsp,
>> +        Error: From<M::InitError>,
>> +        Error: From<<M::Reply as MessageFromGsp>::InitError>,
>> +    {
>> +        self.send_command_internal(bar, command)?;
>> +
>> +        loop {
>> +            match self.receive_msg::<M::Reply>(Self::RECEIVE_TIMEOUT) {
>> +                Ok(reply) => break Ok(reply),
>> +                Err(ERANGE) => continue,
>> +                Err(e) => break Err(e),
>> +            }
>> +        }
>
> There is an opportunity for factorize some more code here.
>
> Notice how the other callers of `receive_msg` (`wait_gsp_init_done` and
> `GspSequencer::run`) both use the same kind of loop, down to the same
> error handling. We could move that loop logic here and do it in a single
> place.
>
> In the future, we will probably want to add handlers for
> unexpected messages from the GSP and it will be easier if we receive all
> messages from a single place.
>
> This can be a separate patch from this one, but I think it makes sense
> to have that in this series.
>
> I expect the last patch to change a bit as a consequence of that - maybe
> we will need a `receive_msg_loop` or something in `CmdqInner`.

I agree we should migrate all callers and make Cmdq responsible for
draining / handling spontaneous messages from the GSP, but I was
planning on doing it in a separate patch series until now. I can put it
into this one though if you want though no worries.

Re: [PATCH v3 3/5] gpu: nova-core: gsp: add reply/no-reply info to `CommandToGsp`

[Alexandre Courbot]

On Thu Mar 5, 2026 at 10:34 AM JST, Eliot Courtney wrote:
> On Wed Mar 4, 2026 at 8:56 PM JST, Alexandre Courbot wrote:
>>> +    /// Sends `command` to the GSP and waits for the reply.
>>> +    ///
>>> +    /// # Errors
>>> +    ///
>>> +    /// - `ETIMEDOUT` if space does not become available to send the command, or if the reply is
>>> +    ///   not received 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 and reply initializers are propagated as-is.
>>> +    pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result<M::Reply>
>>> +    where
>>> +        M: CommandToGsp,
>>> +        M::Reply: MessageFromGsp,
>>> +        Error: From<M::InitError>,
>>> +        Error: From<<M::Reply as MessageFromGsp>::InitError>,
>>> +    {
>>> +        self.send_command_internal(bar, command)?;
>>> +
>>> +        loop {
>>> +            match self.receive_msg::<M::Reply>(Self::RECEIVE_TIMEOUT) {
>>> +                Ok(reply) => break Ok(reply),
>>> +                Err(ERANGE) => continue,
>>> +                Err(e) => break Err(e),
>>> +            }
>>> +        }
>>
>> There is an opportunity for factorize some more code here.
>>
>> Notice how the other callers of `receive_msg` (`wait_gsp_init_done` and
>> `GspSequencer::run`) both use the same kind of loop, down to the same
>> error handling. We could move that loop logic here and do it in a single
>> place.
>>
>> In the future, we will probably want to add handlers for
>> unexpected messages from the GSP and it will be easier if we receive all
>> messages from a single place.
>>
>> This can be a separate patch from this one, but I think it makes sense
>> to have that in this series.
>>
>> I expect the last patch to change a bit as a consequence of that - maybe
>> we will need a `receive_msg_loop` or something in `CmdqInner`.
>
> I agree we should migrate all callers and make Cmdq responsible for
> draining / handling spontaneous messages from the GSP, but I was
> planning on doing it in a separate patch series until now. I can put it
> into this one though if you want though no worries.

If it ends up being convulated, let's do that afterwards but since it
looks like a quick and easy win I thought it would make sense to have it
here. Your call though.

Re: [PATCH v3 3/5] gpu: nova-core: gsp: add reply/no-reply info to `CommandToGsp`

[Alexandre Courbot]

On Wed Mar 4, 2026 at 11:46 AM JST, Eliot Courtney wrote:
<snip>
>  /// Send the [`GetGspInfo`] command and awaits for its reply.
>  pub(crate) fn get_gsp_info(cmdq: &mut Cmdq, bar: &Bar0) -> Result<GetGspStaticInfoReply> {
> -    cmdq.send_command(bar, GetGspStaticInfo)?;
> -
> -    loop {
> -        match cmdq.receive_msg::<GetGspStaticInfoReply>(Cmdq::RECEIVE_TIMEOUT) {
> -            Ok(info) => return Ok(info),
> -            Err(ERANGE) => continue,
> -            Err(e) => return Err(e),
> -        }
> -    }
> +    cmdq.send_command(bar, GetGspStaticInfo)

Also noticed something cool while rebasing the unload series on top of
this one: all these command helpers become one-liners and are not really
needed anymore! Here we can just make `GetGspStaticInfo` public and call
`cmdq.send_command` on it directly in `boot`. It removes a layer of
black magic and makes it easier to understand what is going on.

Re: [PATCH v3 3/5] gpu: nova-core: gsp: add reply/no-reply info to `CommandToGsp`

[Eliot Courtney]

On Wed Mar 4, 2026 at 11:17 PM JST, Alexandre Courbot wrote:
> On Wed Mar 4, 2026 at 11:46 AM JST, Eliot Courtney wrote:
> <snip>
>>  /// Send the [`GetGspInfo`] command and awaits for its reply.
>>  pub(crate) fn get_gsp_info(cmdq: &mut Cmdq, bar: &Bar0) -> Result<GetGspStaticInfoReply> {
>> -    cmdq.send_command(bar, GetGspStaticInfo)?;
>> -
>> -    loop {
>> -        match cmdq.receive_msg::<GetGspStaticInfoReply>(Cmdq::RECEIVE_TIMEOUT) {
>> -            Ok(info) => return Ok(info),
>> -            Err(ERANGE) => continue,
>> -            Err(e) => return Err(e),
>> -        }
>> -    }
>> +    cmdq.send_command(bar, GetGspStaticInfo)
>
> Also noticed something cool while rebasing the unload series on top of
> this one: all these command helpers become one-liners and are not really
> needed anymore! Here we can just make `GetGspStaticInfo` public and call
> `cmdq.send_command` on it directly in `boot`. It removes a layer of
> black magic and makes it easier to understand what is going on.

I think this is ok, but we will still need some helpers (e.g. for RM
control RPCs) that mimic the structure we have here. But there are a
bunch of simple RPCs like this that can just be called without the
helper. Do you think it's better to do this and have mixed helper vs
direct call, or just go all helpers?

Re: [PATCH v3 3/5] gpu: nova-core: gsp: add reply/no-reply info to `CommandToGsp`

[Alexandre Courbot]

On Thu Mar 5, 2026 at 10:29 AM JST, Eliot Courtney wrote:
> On Wed Mar 4, 2026 at 11:17 PM JST, Alexandre Courbot wrote:
>> On Wed Mar 4, 2026 at 11:46 AM JST, Eliot Courtney wrote:
>> <snip>
>>>  /// Send the [`GetGspInfo`] command and awaits for its reply.
>>>  pub(crate) fn get_gsp_info(cmdq: &mut Cmdq, bar: &Bar0) -> Result<GetGspStaticInfoReply> {
>>> -    cmdq.send_command(bar, GetGspStaticInfo)?;
>>> -
>>> -    loop {
>>> -        match cmdq.receive_msg::<GetGspStaticInfoReply>(Cmdq::RECEIVE_TIMEOUT) {
>>> -            Ok(info) => return Ok(info),
>>> -            Err(ERANGE) => continue,
>>> -            Err(e) => return Err(e),
>>> -        }
>>> -    }
>>> +    cmdq.send_command(bar, GetGspStaticInfo)
>>
>> Also noticed something cool while rebasing the unload series on top of
>> this one: all these command helpers become one-liners and are not really
>> needed anymore! Here we can just make `GetGspStaticInfo` public and call
>> `cmdq.send_command` on it directly in `boot`. It removes a layer of
>> black magic and makes it easier to understand what is going on.
>
> I think this is ok, but we will still need some helpers (e.g. for RM
> control RPCs) that mimic the structure we have here. But there are a
> bunch of simple RPCs like this that can just be called without the
> helper. Do you think it's better to do this and have mixed helper vs
> direct call, or just go all helpers?

Mmm ok, let's keep the current helpers and reconsider once we have RM
controls and more visibility.

Claude review: gpu: nova-core: gsp: add reply/no-reply info to `CommandToGsp`

[Claude Code Review Bot]

Patch Review

This is the core design patch. Adds `type Reply` to the `CommandToGsp` trait and a `NoReply` marker type:

```rust
pub(crate) struct NoReply;
```

The approach is sound. Commands that don't expect a reply use `type Reply = NoReply`, and the `send_command_no_wait` function enforces this at the type level with:

```rust
M: CommandToGsp<Reply = NoReply>,
```

The `send_command` method now returns `Result<M::Reply>` and includes the receive loop:

```rust
pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result<M::Reply>
where
    M: CommandToGsp,
    M::Reply: MessageFromGsp,
    ...
{
    self.send_command_internal(bar, command)?;
    loop {
        match self.receive_msg::<M::Reply>(Self::RECEIVE_TIMEOUT) {
            Ok(reply) => break Ok(reply),
            Err(ERANGE) => continue,
            Err(e) => break Err(e),
        }
    }
}
```

**Concern**: The `Err(ERANGE) => continue` loop has no bound. If the GSP keeps sending non-matching messages, this loops forever. Each iteration does have a `RECEIVE_TIMEOUT` on the individual receive, so it won't literally spin forever — eventually `receive_msg` will return `ETIMEDOUT` if no messages arrive. But if a stream of wrong-function-code messages keeps arriving, the loop will consume them all indefinitely without ever timing out. In practice this is unlikely, but a maximum iteration count or an overall deadline would be safer. This is a pre-existing pattern from the old code, so not a regression — but worth noting as a future improvement since this loop is now in a more prominent, reusable position.

The `SplitCommand` correctly propagates `type Reply = C::Reply` (line 951), which is correct since the split command should have the same reply type as the original.

The update to `get_gsp_info` is a nice simplification, collapsing the function to a single line.

The `ContinuationRecord` and test `TestPayload` correctly use `type Reply = NoReply` since continuation records don't receive individual replies.

---
Generated by Claude Code Patch Reviewer

[PATCH v3 4/5] gpu: nova-core: gsp: make `Cmdq` a pinned type

[Eliot Courtney]

Make `Cmdq` a pinned type. This is needed to use Mutex, which is needed
to add locking to `Cmdq`.

Reviewed-by: Zhi Wang <zhiw@nvidia.com>
Tested-by: Zhi Wang <zhiw@nvidia.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp.rs      | 5 +++--
 drivers/gpu/nova-core/gsp/cmdq.rs | 9 ++++-----
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index 174feaca0a6b..a6f3918c20b1 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -112,6 +112,7 @@ pub(crate) struct Gsp {
     /// RM log buffer.
     logrm: LogBuffer,
     /// Command queue.
+    #[pin]
     pub(crate) cmdq: Cmdq,
     /// RM arguments.
     rmargs: CoherentAllocation<GspArgumentsPadded>,
@@ -132,7 +133,7 @@ pub(crate) fn new(pdev: &pci::Device<device::Bound>) -> impl PinInit<Self, Error
                 loginit: LogBuffer::new(dev)?,
                 logintr: LogBuffer::new(dev)?,
                 logrm: LogBuffer::new(dev)?,
-                cmdq: Cmdq::new(dev)?,
+                cmdq <- Cmdq::new(dev),
                 rmargs: CoherentAllocation::<GspArgumentsPadded>::alloc_coherent(
                     dev,
                     1,
@@ -149,7 +150,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].inner = 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/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 7750f5792b21..de84a298909f 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -475,6 +475,7 @@ struct GspMessage<'a> {
 ///
 /// Provides the ability to send commands and receive messages from the GSP using a shared memory
 /// area.
+#[pin_data]
 pub(crate) struct Cmdq {
     /// Device this command queue belongs to.
     dev: ARef<device::Device>,
@@ -508,13 +509,11 @@ impl Cmdq {
     pub(super) const RECEIVE_TIMEOUT: Delta = Delta::from_secs(10);
 
     /// Creates a new command queue for `dev`.
-    pub(crate) fn new(dev: &device::Device<device::Bound>) -> Result<Cmdq> {
-        let gsp_mem = DmaGspMem::new(dev)?;
-
-        Ok(Cmdq {
+    pub(crate) fn new(dev: &device::Device<device::Bound>) -> impl PinInit<Self, Error> + '_ {
+        try_pin_init!(Self {
+            gsp_mem: DmaGspMem::new(dev)?,
             dev: dev.into(),
             seq: 0,
-            gsp_mem,
         })
     }
 

-- 
2.53.0

Claude review: gpu: nova-core: gsp: make `Cmdq` a pinned type

[Claude Code Review Bot]

Patch Review

Mechanical change to make `Cmdq` a pinned type, required for the `Mutex` in patch 5.

The `#[pin_data]` annotation and `#[pin]` on the `cmdq` field are correct. The `new()` method changes from returning `Result<Cmdq>` to `impl PinInit<Self, Error>`, and construction uses `try_pin_init!` with `<-` for pin-init.

The caller change from `cmdq: Cmdq::new(dev)?` to `cmdq <- Cmdq::new(dev)` is the correct pin-init syntax.

The change from `fw::GspArgumentsCached::new(cmdq)` to `fw::GspArgumentsCached::new(&cmdq)` (line 619) is needed because `cmdq` is now behind a pin reference rather than being moved.

No issues.

---
Generated by Claude Code Patch Reviewer

[PATCH v3 5/5] gpu: nova-core: gsp: add mutex locking to Cmdq

[Eliot Courtney]

Wrap `Cmdq`'s mutable state in a new struct `CmdqInner` and wrap that in
a Mutex. This lets `Cmdq` methods take &self instead of &mut self, which
lets required commands be sent e.g. while unloading the driver.

The mutex is held over both send and receive in `send_command` to make
sure that it doesn't get the reply of some other command that could have
been sent just beforehand.

Reviewed-by: Zhi Wang <zhiw@nvidia.com>
Tested-by: Zhi Wang <zhiw@nvidia.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/boot.rs      |   8 +-
 drivers/gpu/nova-core/gsp/cmdq.rs      | 247 +++++++++++++++++++--------------
 drivers/gpu/nova-core/gsp/commands.rs  |   4 +-
 drivers/gpu/nova-core/gsp/sequencer.rs |   2 +-
 4 files changed, 146 insertions(+), 115 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 991eb5957e3d..bc53e667cd9e 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -128,7 +128,7 @@ fn run_fwsec_frts(
     ///
     /// Upon return, the GSP is up and running, and its runtime object given as return value.
     pub(crate) fn boot(
-        mut self: Pin<&mut Self>,
+        self: Pin<&mut Self>,
         pdev: &pci::Device<device::Bound>,
         bar: &Bar0,
         chipset: Chipset,
@@ -214,13 +214,13 @@ pub(crate) fn boot(
             dev: pdev.as_ref().into(),
             bar,
         };
-        GspSequencer::run(&mut self.cmdq, seq_params)?;
+        GspSequencer::run(&self.cmdq, seq_params)?;
 
         // Wait until GSP is fully initialized.
-        commands::wait_gsp_init_done(&mut self.cmdq)?;
+        commands::wait_gsp_init_done(&self.cmdq)?;
 
         // Obtain and display basic GPU information.
-        let info = commands::get_gsp_info(&mut self.cmdq, bar)?;
+        let info = commands::get_gsp_info(&self.cmdq, bar)?;
         match info.gpu_name() {
             Ok(name) => dev_info!(pdev, "GPU name: {}\n", name),
             Err(e) => dev_warn!(pdev, "GPU name unavailable: {:?}\n", e),
diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index de84a298909f..94cb2aa6568d 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -18,8 +18,12 @@
     },
     dma_write,
     io::poll::read_poll_timeout,
+    new_mutex,
     prelude::*,
-    sync::aref::ARef,
+    sync::{
+        aref::ARef,
+        Mutex, //
+    },
     time::Delta,
     transmute::{
         AsBytes,
@@ -57,8 +61,8 @@
 
 /// Trait implemented by types representing a command to send to the GSP.
 ///
-/// The main purpose of this trait is to provide [`Cmdq::send_command`] with the information it
-/// needs to send a given command.
+/// 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.
@@ -471,12 +475,8 @@ struct GspMessage<'a> {
     contents: (&'a [u8], &'a [u8]),
 }
 
-/// GSP command queue.
-///
-/// Provides the ability to send commands and receive messages from the GSP using a shared memory
-/// area.
-#[pin_data]
-pub(crate) struct Cmdq {
+/// Inner mutex protected state of [`Cmdq`].
+struct CmdqInner {
     /// Device this command queue belongs to.
     dev: ARef<device::Device>,
     /// Current command sequence number.
@@ -485,58 +485,10 @@ pub(crate) struct Cmdq {
     gsp_mem: DmaGspMem,
 }
 
-impl Cmdq {
-    /// Offset of the data after the PTEs.
-    const POST_PTE_OFFSET: usize = core::mem::offset_of!(GspMem, cpuq);
-
-    /// Offset of command queue ring buffer.
-    pub(crate) const CMDQ_OFFSET: usize = core::mem::offset_of!(GspMem, cpuq)
-        + core::mem::offset_of!(Msgq, msgq)
-        - Self::POST_PTE_OFFSET;
-
-    /// Offset of message queue ring buffer.
-    pub(crate) const STATQ_OFFSET: usize = core::mem::offset_of!(GspMem, gspq)
-        + core::mem::offset_of!(Msgq, msgq)
-        - Self::POST_PTE_OFFSET;
-
-    /// Number of page table entries for the GSP shared region.
-    pub(crate) const NUM_PTES: usize = size_of::<GspMem>() >> GSP_PAGE_SHIFT;
-
+impl CmdqInner {
     /// Timeout for waiting for space on the command queue.
     const ALLOCATE_TIMEOUT: Delta = Delta::from_secs(1);
 
-    /// Default timeout for receiving a message from the GSP.
-    pub(super) const RECEIVE_TIMEOUT: Delta = Delta::from_secs(10);
-
-    /// Creates a new command queue for `dev`.
-    pub(crate) fn new(dev: &device::Device<device::Bound>) -> impl PinInit<Self, Error> + '_ {
-        try_pin_init!(Self {
-            gsp_mem: DmaGspMem::new(dev)?,
-            dev: dev.into(),
-            seq: 0,
-        })
-    }
-
-    /// Computes the checksum for the message pointed to by `it`.
-    ///
-    /// A message is made of several parts, so `it` is an iterator over byte slices representing
-    /// these parts.
-    fn calculate_checksum<T: Iterator<Item = u8>>(it: T) -> u32 {
-        let sum64 = it
-            .enumerate()
-            .map(|(idx, byte)| (((idx % 8) * 8) as u32, byte))
-            .fold(0, |acc, (rol, byte)| acc ^ u64::from(byte).rotate_left(rol));
-
-        ((sum64 >> 32) as u32) ^ (sum64 as u32)
-    }
-
-    /// Notifies the GSP that we have updated the command queue pointers.
-    fn notify_gsp(bar: &Bar0) {
-        regs::NV_PGSP_QUEUE_HEAD::default()
-            .set_address(0)
-            .write(bar);
-    }
-
     /// Sends `command` to the GSP, without splitting it.
     ///
     /// # Errors
@@ -617,7 +569,7 @@ fn send_single_command<M>(&mut self, bar: &Bar0, command: M) -> Result
     ///   written to by its [`CommandToGsp::init_variable_payload`] method.
     ///
     /// Error codes returned by the command initializers are propagated as-is.
-    fn send_command_internal<M>(&mut self, bar: &Bar0, command: M) -> Result
+    fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
     where
         M: CommandToGsp,
         Error: From<M::InitError>,
@@ -637,51 +589,6 @@ fn send_command_internal<M>(&mut self, bar: &Bar0, command: M) -> Result
         }
     }
 
-    /// Sends `command` to the GSP and waits for the reply.
-    ///
-    /// # Errors
-    ///
-    /// - `ETIMEDOUT` if space does not become available to send the command, or if the reply is
-    ///   not received 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 and reply initializers are propagated as-is.
-    pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result<M::Reply>
-    where
-        M: CommandToGsp,
-        M::Reply: MessageFromGsp,
-        Error: From<M::InitError>,
-        Error: From<<M::Reply as MessageFromGsp>::InitError>,
-    {
-        self.send_command_internal(bar, command)?;
-
-        loop {
-            match self.receive_msg::<M::Reply>(Self::RECEIVE_TIMEOUT) {
-                Ok(reply) => break Ok(reply),
-                Err(ERANGE) => continue,
-                Err(e) => break Err(e),
-            }
-        }
-    }
-
-    /// Sends `command` to the GSP without waiting for a reply.
-    ///
-    /// # Errors
-    ///
-    /// - `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 initializers are propagated as-is.
-    pub(crate) fn send_command_no_wait<M>(&mut self, bar: &Bar0, command: M) -> Result
-    where
-        M: CommandToGsp<Reply = NoReply>,
-        Error: From<M::InitError>,
-    {
-        self.send_command_internal(bar, command)
-    }
-
     /// Wait for a message to become available on the message queue.
     ///
     /// This works purely at the transport layer and does not interpret or validate the message
@@ -717,7 +624,7 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
         let (header, slice_1) = GspMsgElement::from_bytes_prefix(slice_1).ok_or(EIO)?;
 
         dev_dbg!(
-            self.dev,
+            &self.dev,
             "GSP RPC: receive: seq# {}, function={:?}, length=0x{:x}\n",
             header.sequence(),
             header.function(),
@@ -752,7 +659,7 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
         ])) != 0
         {
             dev_err!(
-                self.dev,
+                &self.dev,
                 "GSP RPC: receive: Call {} - bad checksum\n",
                 header.sequence()
             );
@@ -781,7 +688,7 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
     /// - `ERANGE` if the message had a recognized but non-matching function code.
     ///
     /// Error codes returned by [`MessageFromGsp::read`] are propagated as-is.
-    pub(crate) fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M>
+    fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M>
     where
         // This allows all error types, including `Infallible`, to be used for `M::InitError`.
         Error: From<M::InitError>,
@@ -817,9 +724,133 @@ pub(crate) fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Resul
 
         result
     }
+}
+
+/// GSP command queue.
+///
+/// Provides the ability to send commands and receive messages from the GSP using a shared memory
+/// area.
+#[pin_data]
+pub(crate) struct Cmdq {
+    /// Inner mutex-protected state.
+    #[pin]
+    inner: Mutex<CmdqInner>,
+}
+
+impl Cmdq {
+    /// Offset of the data after the PTEs.
+    const POST_PTE_OFFSET: usize = core::mem::offset_of!(GspMem, cpuq);
+
+    /// Offset of command queue ring buffer.
+    pub(crate) const CMDQ_OFFSET: usize = core::mem::offset_of!(GspMem, cpuq)
+        + core::mem::offset_of!(Msgq, msgq)
+        - Self::POST_PTE_OFFSET;
+
+    /// Offset of message queue ring buffer.
+    pub(crate) const STATQ_OFFSET: usize = core::mem::offset_of!(GspMem, gspq)
+        + core::mem::offset_of!(Msgq, msgq)
+        - Self::POST_PTE_OFFSET;
+
+    /// Number of page table entries for the GSP shared region.
+    pub(crate) const NUM_PTES: usize = size_of::<GspMem>() >> GSP_PAGE_SHIFT;
+
+    /// Default timeout for receiving a message from the GSP.
+    pub(super) const RECEIVE_TIMEOUT: Delta = Delta::from_secs(10);
+
+    /// Creates a new command queue for `dev`.
+    pub(crate) fn new(dev: &device::Device<device::Bound>) -> impl PinInit<Self, Error> + '_ {
+        try_pin_init!(Self {
+            inner <- new_mutex!(CmdqInner {
+                dev: dev.into(),
+                gsp_mem: DmaGspMem::new(dev)?,
+                seq: 0,
+            }),
+        })
+    }
+
+    /// Computes the checksum for the message pointed to by `it`.
+    ///
+    /// A message is made of several parts, so `it` is an iterator over byte slices representing
+    /// these parts.
+    fn calculate_checksum<T: Iterator<Item = u8>>(it: T) -> u32 {
+        let sum64 = it
+            .enumerate()
+            .map(|(idx, byte)| (((idx % 8) * 8) as u32, byte))
+            .fold(0, |acc, (rol, byte)| acc ^ u64::from(byte).rotate_left(rol));
+
+        ((sum64 >> 32) as u32) ^ (sum64 as u32)
+    }
+
+    /// Notifies the GSP that we have updated the command queue pointers.
+    fn notify_gsp(bar: &Bar0) {
+        regs::NV_PGSP_QUEUE_HEAD::default()
+            .set_address(0)
+            .write(bar);
+    }
+
+    /// Sends `command` to the GSP and waits for the reply.
+    ///
+    /// The mutex is held for the entire send+receive cycle to ensure that no other command can
+    /// be interleaved. Messages with non-matching function codes are silently consumed until the
+    /// expected reply arrives.
+    ///
+    /// # Errors
+    ///
+    /// - `ETIMEDOUT` if space does not become available to send the command, or if the reply is
+    ///   not received 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 and reply initializers are propagated as-is.
+    pub(crate) fn send_command<M>(&self, bar: &Bar0, command: M) -> Result<M::Reply>
+    where
+        M: CommandToGsp,
+        M::Reply: MessageFromGsp,
+        Error: From<M::InitError>,
+        Error: From<<M::Reply as MessageFromGsp>::InitError>,
+    {
+        let mut inner = self.inner.lock();
+        inner.send_command(bar, command)?;
+
+        loop {
+            match inner.receive_msg::<M::Reply>(Self::RECEIVE_TIMEOUT) {
+                Ok(reply) => break Ok(reply),
+                Err(ERANGE) => continue,
+                Err(e) => break Err(e),
+            }
+        }
+    }
+
+    /// Sends `command` to the GSP without waiting for a reply.
+    ///
+    /// # Errors
+    ///
+    /// - `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 initializers are propagated as-is.
+    pub(crate) fn send_command_no_wait<M>(&self, bar: &Bar0, command: M) -> Result
+    where
+        M: CommandToGsp<Reply = NoReply>,
+        Error: From<M::InitError>,
+    {
+        self.inner.lock().send_command(bar, command)
+    }
+
+    /// Receive a message from the GSP.
+    ///
+    /// See [`CmdqInner::receive_msg`] for details.
+    pub(crate) fn receive_msg<M: MessageFromGsp>(&self, timeout: Delta) -> Result<M>
+    where
+        // This allows all error types, including `Infallible`, to be used for `M::InitError`.
+        Error: From<M::InitError>,
+    {
+        self.inner.lock().receive_msg(timeout)
+    }
 
     /// Returns the DMA handle of the command queue's shared memory region.
     pub(crate) fn dma_handle(&self) -> DmaAddress {
-        self.gsp_mem.0.dma_handle()
+        self.inner.lock().gsp_mem.0.dma_handle()
     }
 }
diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index 77054c92fcc2..c89c7b57a751 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -165,7 +165,7 @@ fn read(
 }
 
 /// Waits for GSP initialization to complete.
-pub(crate) fn wait_gsp_init_done(cmdq: &mut Cmdq) -> Result {
+pub(crate) fn wait_gsp_init_done(cmdq: &Cmdq) -> Result {
     loop {
         match cmdq.receive_msg::<GspInitDone>(Cmdq::RECEIVE_TIMEOUT) {
             Ok(_) => break Ok(()),
@@ -234,6 +234,6 @@ pub(crate) fn gpu_name(&self) -> core::result::Result<&str, GpuNameError> {
 }
 
 /// Send the [`GetGspInfo`] command and awaits for its reply.
-pub(crate) fn get_gsp_info(cmdq: &mut Cmdq, bar: &Bar0) -> Result<GetGspStaticInfoReply> {
+pub(crate) fn get_gsp_info(cmdq: &Cmdq, bar: &Bar0) -> Result<GetGspStaticInfoReply> {
     cmdq.send_command(bar, GetGspStaticInfo)
 }
diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs b/drivers/gpu/nova-core/gsp/sequencer.rs
index ce2b3bb05d22..474e4c8021db 100644
--- a/drivers/gpu/nova-core/gsp/sequencer.rs
+++ b/drivers/gpu/nova-core/gsp/sequencer.rs
@@ -356,7 +356,7 @@ pub(crate) struct GspSequencerParams<'a> {
 }
 
 impl<'a> GspSequencer<'a> {
-    pub(crate) fn run(cmdq: &mut Cmdq, params: GspSequencerParams<'a>) -> Result {
+    pub(crate) fn run(cmdq: &Cmdq, params: GspSequencerParams<'a>) -> Result {
         let seq_info = loop {
             match cmdq.receive_msg::<GspSequence>(Cmdq::RECEIVE_TIMEOUT) {
                 Ok(seq_info) => break seq_info,

-- 
2.53.0

Re: [PATCH v3 5/5] gpu: nova-core: gsp: add mutex locking to Cmdq

[Alexandre Courbot]

On Wed Mar 4, 2026 at 11:46 AM JST, Eliot Courtney wrote:
> Wrap `Cmdq`'s mutable state in a new struct `CmdqInner` and wrap that in
> a Mutex. This lets `Cmdq` methods take &self instead of &mut self, which
> lets required commands be sent e.g. while unloading the driver.
>
> The mutex is held over both send and receive in `send_command` to make
> sure that it doesn't get the reply of some other command that could have
> been sent just beforehand.
>
> Reviewed-by: Zhi Wang <zhiw@nvidia.com>
> Tested-by: Zhi Wang <zhiw@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  drivers/gpu/nova-core/gsp/boot.rs      |   8 +-
>  drivers/gpu/nova-core/gsp/cmdq.rs      | 247 +++++++++++++++++++--------------
>  drivers/gpu/nova-core/gsp/commands.rs  |   4 +-
>  drivers/gpu/nova-core/gsp/sequencer.rs |   2 +-
>  4 files changed, 146 insertions(+), 115 deletions(-)

Most of the diff is code moving around. I've been able to reduce it to
+107 -76 by moving `CmdqInner` *after* `Cmdq` - this makes this patch
easier to review imho.

Re: [PATCH v3 5/5] gpu: nova-core: gsp: add mutex locking to Cmdq

[Eliot Courtney]

On Wed Mar 4, 2026 at 8:57 PM JST, Alexandre Courbot wrote:
> On Wed Mar 4, 2026 at 11:46 AM JST, Eliot Courtney wrote:
>> Wrap `Cmdq`'s mutable state in a new struct `CmdqInner` and wrap that in
>> a Mutex. This lets `Cmdq` methods take &self instead of &mut self, which
>> lets required commands be sent e.g. while unloading the driver.
>>
>> The mutex is held over both send and receive in `send_command` to make
>> sure that it doesn't get the reply of some other command that could have
>> been sent just beforehand.
>>
>> Reviewed-by: Zhi Wang <zhiw@nvidia.com>
>> Tested-by: Zhi Wang <zhiw@nvidia.com>
>> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
>> ---
>>  drivers/gpu/nova-core/gsp/boot.rs      |   8 +-
>>  drivers/gpu/nova-core/gsp/cmdq.rs      | 247 +++++++++++++++++++--------------
>>  drivers/gpu/nova-core/gsp/commands.rs  |   4 +-
>>  drivers/gpu/nova-core/gsp/sequencer.rs |   2 +-
>>  4 files changed, 146 insertions(+), 115 deletions(-)
>
> Most of the diff is code moving around. I've been able to reduce it to
> +107 -76 by moving `CmdqInner` *after* `Cmdq` - this makes this patch
> easier to review imho.

Thanks, will move this then since I think the ordering here is probably
not too important to readability.

Re: [PATCH v3 5/5] gpu: nova-core: gsp: add mutex locking to Cmdq

[Alexandre Courbot]

On Thu Mar 5, 2026 at 10:36 AM JST, Eliot Courtney wrote:
> On Wed Mar 4, 2026 at 8:57 PM JST, Alexandre Courbot wrote:
>> On Wed Mar 4, 2026 at 11:46 AM JST, Eliot Courtney wrote:
>>> Wrap `Cmdq`'s mutable state in a new struct `CmdqInner` and wrap that in
>>> a Mutex. This lets `Cmdq` methods take &self instead of &mut self, which
>>> lets required commands be sent e.g. while unloading the driver.
>>>
>>> The mutex is held over both send and receive in `send_command` to make
>>> sure that it doesn't get the reply of some other command that could have
>>> been sent just beforehand.
>>>
>>> Reviewed-by: Zhi Wang <zhiw@nvidia.com>
>>> Tested-by: Zhi Wang <zhiw@nvidia.com>
>>> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
>>> ---
>>>  drivers/gpu/nova-core/gsp/boot.rs      |   8 +-
>>>  drivers/gpu/nova-core/gsp/cmdq.rs      | 247 +++++++++++++++++++--------------
>>>  drivers/gpu/nova-core/gsp/commands.rs  |   4 +-
>>>  drivers/gpu/nova-core/gsp/sequencer.rs |   2 +-
>>>  4 files changed, 146 insertions(+), 115 deletions(-)
>>
>> Most of the diff is code moving around. I've been able to reduce it to
>> +107 -76 by moving `CmdqInner` *after* `Cmdq` - this makes this patch
>> easier to review imho.
>
> Thanks, will move this then since I think the ordering here is probably
> not too important to readability.

Yes, actually one could even argue in favor of having the public struct
before its inner private counterpart for readability purposes.

Re: [PATCH v3 5/5] gpu: nova-core: gsp: add mutex locking to Cmdq

[Alexandre Courbot]

On Wed Mar 4, 2026 at 11:46 AM JST, Eliot Courtney wrote:
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index de84a298909f..94cb2aa6568d 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -18,8 +18,12 @@
>      },
>      dma_write,
>      io::poll::read_poll_timeout,
> +    new_mutex,
>      prelude::*,
> -    sync::aref::ARef,
> +    sync::{
> +        aref::ARef,
> +        Mutex, //
> +    },
>      time::Delta,
>      transmute::{
>          AsBytes,
> @@ -57,8 +61,8 @@
>  
>  /// Trait implemented by types representing a command to send to the GSP.
>  ///
> -/// The main purpose of this trait is to provide [`Cmdq::send_command`] with the information it
> -/// needs to send a given command.
> +/// The main purpose of this trait is to provide [`Cmdq`] with the information it needs to send
> +/// a given command.

This looks unrelated - should this chunk be merged with the reply/no_reply patch?

Claude review: gpu: nova-core: gsp: add mutex locking to Cmdq

[Claude Code Review Bot]

Patch Review

The core locking patch. The key design decisions:

1. **Inner/Outer split**: `CmdqInner` holds `dev`, `seq`, and `gsp_mem` behind a `Mutex`. `Cmdq` holds `Mutex<CmdqInner>` plus constants.

2. **Static methods moved to outer**: `calculate_checksum` and `notify_gsp` are pure functions / don't access mutable state, so they stay on `Cmdq`. The various offset/size constants also stay on `Cmdq`. This is correct since they don't need mutex protection.

3. **Mutex held across send+receive**: For `send_command`, the mutex is held for both send and receive to prevent reply mismatches. This is documented:

```rust
/// The mutex is held for the entire send+receive cycle to ensure that no other command can
/// be interleaved.
```

This is the right approach — releasing the lock between send and receive would allow another thread to send a command and steal the reply.

4. **`&self` instead of `&mut self`**: All public methods now take `&self`, with interior mutability via the mutex. This is the whole point of the series.

**Concern 1**: `dma_handle()` locks the mutex just to read `gsp_mem`:

```rust
pub(crate) fn dma_handle(&self) -> DmaAddress {
    self.inner.lock().gsp_mem.0.dma_handle()
}
```

The DMA handle is determined at allocation time and never changes. Locking the mutex here adds unnecessary contention. Consider keeping `gsp_mem` (or at least the DMA handle) outside the mutex in `Cmdq` directly. The `gsp_mem` field could be in the outer struct since it's initialized once and never mutated — only `seq` truly needs mutex protection (and the queue read/write pointers accessed through `gsp_mem`). However, since `gsp_mem` provides access to the shared memory that is read/written during send/receive, keeping it inside the mutex is arguably safer. The DMA handle accessor is likely only called during init, so the performance concern is negligible. This is a minor nit.

**Concern 2**: The `boot()` method signature change from `mut self: Pin<&mut Self>` to `self: Pin<&mut Self>` (line 1414-1415) is correct since `cmdq` methods no longer need `&mut self`.

**Concern 3**: `wait_gsp_init_done` and the sequencer's `receive_msg` calls now lock/unlock the mutex on each iteration of their retry loops. For example:

```rust
pub(crate) fn wait_gsp_init_done(cmdq: &Cmdq) -> Result {
    loop {
        match cmdq.receive_msg::<GspInitDone>(Cmdq::RECEIVE_TIMEOUT) {
            ...
            Err(ERANGE) => continue,
        }
    }
}
```

Each call to `cmdq.receive_msg` locks the mutex, receives one message, then unlocks. On `ERANGE`, it loops and re-locks. This means another thread could interleave between iterations and consume the message meant for this caller. This is a potential issue for `wait_gsp_init_done` and `GspSequencer::run`, which use the standalone `receive_msg` pattern (not the `send_command` which holds the lock). However, during boot these are likely single-threaded, so this is not an immediate problem. The cover letter acknowledges this is for the "init done" waiting during boot where there's no concurrent access. Still, this is a design point to be aware of as usage expands.

Overall, this is a clean and well-thought-out series. The type-level distinction between reply/no-reply commands is elegant and the locking strategy is sound for the current use case.

---
Generated by Claude Code Patch Reviewer

Re: [PATCH v3 0/5] gpu: nova-core: gsp: add locking to Cmdq

[Alexandre Courbot]

On Wed Mar 4, 2026 at 11:46 AM JST, Eliot Courtney wrote:
> Add locking to Cmdq. This is required e.g. for unloading the driver,
> which needs to send the UnloadingGuestDriver via the command queue
> on unbind which may be on a different thread.
>
> We have commands that need a reply and commands that don't. For
> commands with a reply we want to make sure that they don't get
> the reply of a different command back. The approach this patch series
> takes is by making those commands block until they get a response. For
> now this should be ok, and we expect GSP to be fast anyway.
>
> To do this, we need to know which commands expect a reply and which
> don't. John's existing series[1] adds IS_ASYNC which solves part of the
> problem, but we need to know a bit more. So instead, add an
> associated type called Reply which tells us what the reply is.
>
> An alternative would be to define traits inheriting CommandToGsp, e.g.
> CommandWithReply and CommandWithoutReply, instead of using the
> associated type. I implemented the associated type version because it
> feels more compositional rather than inherity so seemed a bit better
> to me. But both of these approaches work and are fine, IMO.

The associated type seems to work just fine. I was wondering whether we
could mess with it by using other types that do not implement
`MessageFromGsp`, but in that case we cannot call any of the
`send_command` methods so this approach looks good to me.

Claude review: gpu: nova-core: gsp: add locking to Cmdq

[Claude Code Review Bot]

Overall Series Review

Subject: gpu: nova-core: gsp: add locking to Cmdq
Author: Eliot Courtney <ecourtney@nvidia.com>
Patches: 22
Reviewed: 2026-03-05T13:52:59.195127

---

This is a well-structured 5-patch series adding mutex locking to the nova-core GSP command queue (`Cmdq`). The motivation is clear: commands need to be sent from different threads (e.g. driver unbind), so the queue needs synchronization. The approach of distinguishing reply/no-reply commands via an associated type is clean and leverages Rust's type system well. The series is logically ordered: doc fixes → constant extraction → type infrastructure → pin requirement → mutex.

The series is generally solid. The design choice to hold the mutex across both send+receive for commands expecting a reply is correct and necessary to prevent reply mismatches. The code is well documented.

I have a few concerns, mainly around the `send_command` reply loop potentially blocking the mutex indefinitely, and the `dma_handle` accessor locking the mutex just to read an immutable field.

---
Generated by Claude Code Patch Reviewer

[PATCH v4 0/5] gpu: nova-core: gsp: add locking to Cmdq

[Eliot Courtney]

Add locking to Cmdq. This is required e.g. for unloading the driver,
which needs to send the UnloadingGuestDriver via the command queue
on unbind which may be on a different thread.

We have commands that need a reply and commands that don't. For
commands with a reply we want to make sure that they don't get
the reply of a different command back. The approach this patch series
takes is by making those commands block until they get a response. For
now this should be ok, and we expect GSP to be fast anyway.

To do this, we need to know which commands expect a reply and which
don't. John's existing series[1] adds IS_ASYNC which solves part of the
problem, but we need to know a bit more. So instead, add an
associated type called Reply which tells us what the reply is.

An alternative would be to define traits inheriting CommandToGsp, e.g.
CommandWithReply and CommandWithoutReply, instead of using the
associated type. I implemented the associated type version because it
feels more compositional rather than inherity so seemed a bit better
to me. But both of these approaches work and are fine, IMO.

In summary, this patch series has three steps:
1. Add the type infrastructure to know what replies are expected for a
   command and update each caller to explicitly wait for the reply or
   not.
2. Make Cmdq pinned so we can use Mutex
3. Add a Mutex to protect Cmdq by moving the relevant state to an
   inner struct.

[1]: https://lore.kernel.org/all/20260211000451.192109-1-jhubbard@nvidia.com/

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
Changes in v4:
- Change RECEIVE_TIMEOUT from 10s to 5s
- Update NoReply doc comment
- Move CommandToGsp doc comment update
- Fix import formatting in continuation.rs
- Move CmdqInner after Cmdq to reduce diff size
- Link to v3: https://lore.kernel.org/r/20260304-cmdq-locking-v3-0-a6314b708850@nvidia.com

Changes in v3:
- Rename send_sync_command/send_async_command to
  send_command/send_command_no_wait.
- Move `dev` field into `CmdqInner` to avoid passing it through method
  parameters.
- Add `RECEIVE_TIMEOUT` constant for the 10s receive timeout.
- Link to v2: https://lore.kernel.org/r/20260226-cmdq-locking-v2-0-c7e16a6d5885@nvidia.com

Changes in v2:
- Rebase on drm-rust-next
- Link to v1: https://lore.kernel.org/r/20260225-cmdq-locking-v1-0-bbf6b4156706@nvidia.com

---
Eliot Courtney (5):
      gpu: nova-core: gsp: fix stale doc comments on command queue methods
      gpu: nova-core: gsp: add `RECEIVE_TIMEOUT` constant for command queue
      gpu: nova-core: gsp: add reply/no-reply info to `CommandToGsp`
      gpu: nova-core: gsp: make `Cmdq` a pinned type
      gpu: nova-core: gsp: add mutex locking to Cmdq

 drivers/gpu/nova-core/gsp.rs                   |   5 +-
 drivers/gpu/nova-core/gsp/boot.rs              |  13 +-
 drivers/gpu/nova-core/gsp/cmdq.rs              | 157 +++++++++++++++++++------
 drivers/gpu/nova-core/gsp/cmdq/continuation.rs |   8 +-
 drivers/gpu/nova-core/gsp/commands.rs          |  23 ++--
 drivers/gpu/nova-core/gsp/sequencer.rs         |   4 +-
 6 files changed, 149 insertions(+), 61 deletions(-)
---
base-commit: 4a49fe23e357b48845e31fe9c28a802c05458198
change-id: 20260225-cmdq-locking-d32928a2c2cf
prerequisite-message-id: <20260304-cmdq-continuation-v5-0-3f19d759ed93@nvidia.com>
prerequisite-patch-id: fd45bc5b8eda5e2b54a052dddb1a1c363107f0a7
prerequisite-patch-id: 06fe65f900206c44b5ba52286ca4ce1ca42b55d5
prerequisite-patch-id: 8844970d0e387488c70979a73732579ba174b46c
prerequisite-patch-id: e138a94ed48fa8d50d5ed1eb36524f98923ed478
prerequisite-patch-id: dccf2b12b176947e89b44baafda9c5a0aa0a93bc
prerequisite-patch-id: 30ed64c398e541d6efbcb2e46ed9a9e6cf953f4f
prerequisite-patch-id: ba1c8da0cbdb4682b879633a94a172d1b2b6bc8e
prerequisite-patch-id: 081d4a4198a0bf09f3480cb8baf296db585decce
prerequisite-patch-id: 56c8c25e7362178cd019c8f03954a6bcdb72b1b5

Best regards,
-- 
Eliot Courtney <ecourtney@nvidia.com>

[PATCH v4 3/5] gpu: nova-core: gsp: add reply/no-reply info to `CommandToGsp`

[Eliot Courtney]

Add type infrastructure to know what reply is expected from each
`CommandToGsp`. Uses a marker type `NoReply` which does not implement
`MessageFromGsp` to mark commands which don't expect a response.

Update `send_command` to wait for a reply and add `send_command_no_wait`
which sends a command that has no reply, without blocking.

This prepares for adding locking to the queue.

Tested-by: Zhi Wang <zhiw@nvidia.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/boot.rs              |  5 ++-
 drivers/gpu/nova-core/gsp/cmdq.rs              | 59 ++++++++++++++++++++++++--
 drivers/gpu/nova-core/gsp/cmdq/continuation.rs |  8 +++-
 drivers/gpu/nova-core/gsp/commands.rs          | 16 +++----
 4 files changed, 72 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index c56029f444cb..991eb5957e3d 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -160,8 +160,9 @@ pub(crate) fn boot(
         dma_write!(wpr_meta[0] = GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
 
         self.cmdq
-            .send_command(bar, commands::SetSystemInfo::new(pdev))?;
-        self.cmdq.send_command(bar, commands::SetRegistry::new())?;
+            .send_command_no_wait(bar, commands::SetSystemInfo::new(pdev))?;
+        self.cmdq
+            .send_command_no_wait(bar, commands::SetRegistry::new())?;
 
         gsp_falcon.reset(bar)?;
         let libos_handle = self.libos.dma_handle();
diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 08ac3067f1b5..b631daae5642 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -51,10 +51,14 @@
     sbuffer::SBufferIter, //
 };
 
+/// Marker type representing the absence of a reply for a command. Commands using this as their
+/// reply type are sent using [`Cmdq::send_command_no_wait`].
+pub(crate) struct NoReply;
+
 /// Trait implemented by types representing a command to send to the GSP.
 ///
-/// The main purpose of this trait is to provide [`Cmdq::send_command`] with the information it
-/// needs to send a given command.
+/// 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.
@@ -69,6 +73,10 @@ pub(crate) trait CommandToGsp {
     /// Type generated by [`CommandToGsp::init`], to be written into the command queue buffer.
     type Command: FromBytes + AsBytes;
 
+    /// Type of the reply expected from the GSP, or [`NoReply`] for commands that don't
+    /// have a reply.
+    type Reply;
+
     /// Error type returned by [`CommandToGsp::init`].
     type InitError;
 
@@ -610,7 +618,7 @@ fn send_single_command<M>(&mut self, bar: &Bar0, command: M) -> Result
     ///   written to by its [`CommandToGsp::init_variable_payload`] method.
     ///
     /// Error codes returned by the command initializers are propagated as-is.
-    pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
+    fn send_command_internal<M>(&mut self, bar: &Bar0, command: M) -> Result
     where
         M: CommandToGsp,
         Error: From<M::InitError>,
@@ -630,6 +638,51 @@ pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
         }
     }
 
+    /// Sends `command` to the GSP and waits for the reply.
+    ///
+    /// # Errors
+    ///
+    /// - `ETIMEDOUT` if space does not become available to send the command, or if the reply is
+    ///   not received 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 and reply initializers are propagated as-is.
+    pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result<M::Reply>
+    where
+        M: CommandToGsp,
+        M::Reply: MessageFromGsp,
+        Error: From<M::InitError>,
+        Error: From<<M::Reply as MessageFromGsp>::InitError>,
+    {
+        self.send_command_internal(bar, command)?;
+
+        loop {
+            match self.receive_msg::<M::Reply>(Self::RECEIVE_TIMEOUT) {
+                Ok(reply) => break Ok(reply),
+                Err(ERANGE) => continue,
+                Err(e) => break Err(e),
+            }
+        }
+    }
+
+    /// Sends `command` to the GSP without waiting for a reply.
+    ///
+    /// # Errors
+    ///
+    /// - `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 initializers are propagated as-is.
+    pub(crate) fn send_command_no_wait<M>(&mut self, bar: &Bar0, command: M) -> Result
+    where
+        M: CommandToGsp<Reply = NoReply>,
+        Error: From<M::InitError>,
+    {
+        self.send_command_internal(bar, command)
+    }
+
     /// Wait for a message to become available on the message queue.
     ///
     /// This works purely at the transport layer and does not interpret or validate the message
diff --git a/drivers/gpu/nova-core/gsp/cmdq/continuation.rs b/drivers/gpu/nova-core/gsp/cmdq/continuation.rs
index 2aa17caac2e0..05e904f18097 100644
--- a/drivers/gpu/nova-core/gsp/cmdq/continuation.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq/continuation.rs
@@ -6,7 +6,10 @@
 
 use kernel::prelude::*;
 
-use super::CommandToGsp;
+use super::{
+    CommandToGsp,
+    NoReply, //
+};
 
 use crate::{
     gsp::fw::{
@@ -63,6 +66,7 @@ fn new(data: &'a [u8]) -> Self {
 impl<'a> CommandToGsp for ContinuationRecord<'a> {
     const FUNCTION: MsgFunction = MsgFunction::ContinuationRecord;
     type Command = ();
+    type Reply = NoReply;
     type InitError = Infallible;
 
     fn init(&self) -> impl Init<Self::Command, Self::InitError> {
@@ -144,6 +148,7 @@ fn new(command: C, payload: KVVec<u8>) -> Self {
 impl<C: CommandToGsp> CommandToGsp for SplitCommand<C> {
     const FUNCTION: MsgFunction = C::FUNCTION;
     type Command = C::Command;
+    type Reply = C::Reply;
     type InitError = C::InitError;
 
     fn init(&self) -> impl Init<Self::Command, Self::InitError> {
@@ -206,6 +211,7 @@ fn new(len: usize) -> Result<Self> {
     impl CommandToGsp for TestPayload {
         const FUNCTION: MsgFunction = MsgFunction::Nop;
         type Command = TestHeader;
+        type Reply = NoReply;
         type InitError = Infallible;
 
         fn init(&self) -> impl Init<Self::Command, Self::InitError> {
diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index 88df117ba575..77054c92fcc2 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -23,7 +23,8 @@
         cmdq::{
             Cmdq,
             CommandToGsp,
-            MessageFromGsp, //
+            MessageFromGsp,
+            NoReply, //
         },
         fw::{
             commands::*,
@@ -48,6 +49,7 @@ pub(crate) fn new(pdev: &'a pci::Device<device::Bound>) -> Self {
 impl<'a> CommandToGsp for SetSystemInfo<'a> {
     const FUNCTION: MsgFunction = MsgFunction::GspSetSystemInfo;
     type Command = GspSetSystemInfo;
+    type Reply = NoReply;
     type InitError = Error;
 
     fn init(&self) -> impl Init<Self::Command, Self::InitError> {
@@ -99,6 +101,7 @@ pub(crate) fn new() -> Self {
 impl CommandToGsp for SetRegistry {
     const FUNCTION: MsgFunction = MsgFunction::SetRegistry;
     type Command = PackedRegistryTable;
+    type Reply = NoReply;
     type InitError = Infallible;
 
     fn init(&self) -> impl Init<Self::Command, Self::InitError> {
@@ -178,6 +181,7 @@ pub(crate) fn wait_gsp_init_done(cmdq: &mut Cmdq) -> Result {
 impl CommandToGsp for GetGspStaticInfo {
     const FUNCTION: MsgFunction = MsgFunction::GetGspStaticInfo;
     type Command = GspStaticConfigInfo;
+    type Reply = GetGspStaticInfoReply;
     type InitError = Infallible;
 
     fn init(&self) -> impl Init<Self::Command, Self::InitError> {
@@ -231,13 +235,5 @@ pub(crate) fn gpu_name(&self) -> core::result::Result<&str, GpuNameError> {
 
 /// Send the [`GetGspInfo`] command and awaits for its reply.
 pub(crate) fn get_gsp_info(cmdq: &mut Cmdq, bar: &Bar0) -> Result<GetGspStaticInfoReply> {
-    cmdq.send_command(bar, GetGspStaticInfo)?;
-
-    loop {
-        match cmdq.receive_msg::<GetGspStaticInfoReply>(Cmdq::RECEIVE_TIMEOUT) {
-            Ok(info) => return Ok(info),
-            Err(ERANGE) => continue,
-            Err(e) => return Err(e),
-        }
-    }
+    cmdq.send_command(bar, GetGspStaticInfo)
 }

-- 
2.53.0

Claude review: gpu: nova-core: gsp: add reply/no-reply info to `CommandToGsp`

[Claude Code Review Bot]

Patch Review

This is the key design patch. It introduces:

1. A `NoReply` marker type for commands with no expected response
2. An associated `type Reply` on `CommandToGsp`
3. Splits the public API into `send_command` (waits for reply) and `send_command_no_wait` (fire-and-forget)

The type-level enforcement is well done:

```rust
+    pub(crate) fn send_command_no_wait<M>(&mut self, bar: &Bar0, command: M) -> Result
+    where
+        M: CommandToGsp<Reply = NoReply>,
```

This constrains `send_command_no_wait` to only accept commands whose `Reply` is `NoReply`, which prevents accidentally forgetting to wait for a reply.

The `send_command` method absorbs the receive loop pattern that was previously duplicated in `get_gsp_info`:

```rust
+        loop {
+            match self.receive_msg::<M::Reply>(Self::RECEIVE_TIMEOUT) {
+                Ok(reply) => break Ok(reply),
+                Err(ERANGE) => continue,
+                Err(e) => break Err(e),
+            }
+        }
```

This simplifies `get_gsp_info` to a single line, which is nice. The `SplitCommand` wrapper correctly propagates `type Reply = C::Reply`, and `ContinuationRecord` correctly uses `NoReply`.

One observation: the `wait_gsp_init_done` function still has its own manual receive loop in `commands.rs` since `GspInitDone` isn't triggered by a specific command -- it's an unsolicited notification. This is correct; it cannot use `send_command`.

The `// ` trailing comment on the import formatting (e.g., `NoReply, //`) is the `rustfmt` skip trick to force vertical formatting -- standard practice in the kernel Rust code.

No issues.

---
Generated by Claude Code Patch Reviewer