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

[PATCH 0/4] 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 ("sync") and commands that don't
("async"). For sync commands 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 sync 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.
SyncCommand and AsyncCommand, 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 send a sync or async
   command.
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>
---
Eliot Courtney (4):
      gpu: nova-core: gsp: fix stale doc comments on command queue methods
      gpu: nova-core: gsp: add sync and async command queue API to `Cmdq`
      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      | 240 ++++++++++++++++++++++-----------
 drivers/gpu/nova-core/gsp/commands.rs  |  23 ++--
 drivers/gpu/nova-core/gsp/sequencer.rs |   2 +-
 5 files changed, 182 insertions(+), 101 deletions(-)
---
base-commit: 4f938c7d3b25d87b356af4106c2682caf8c835a2
change-id: 20260225-cmdq-locking-d32928a2c2cf
prerequisite-message-id: <20260129-nova-core-cmdq1-v3-0-2ede85493a27@nvidia.com>
prerequisite-patch-id: bd2da0d580038dbab7ef06c0508d157873796ff4
prerequisite-patch-id: f834078971a3a46babe131251353184ffd7ecc38
prerequisite-patch-id: db5049ec7fe4348eed9e54dd5a2c07c697a54c81
prerequisite-patch-id: bafdd38cb9aba355a796232e07d5bbcd6fc59ba5
prerequisite-patch-id: 72a8d4626458707ae4ed632c629ffb6728cee032
prerequisite-message-id: <20260219-cmdq-continuation-v2-0-2e8b7615536f@nvidia.com>
prerequisite-patch-id: 20003099ca19d81b6255c5e95148f6584c108a29
prerequisite-patch-id: d0f59ef489346e978a222755f9fb42dfe7af19e5
prerequisite-patch-id: 8844970d0e387488c70979a73732579ba174b46c
prerequisite-patch-id: e138a94ed48fa8d50d5ed1eb36524f98923ed478
prerequisite-patch-id: 4599a5e90d6665fa3acb7d4045de5d378ac28b4d
prerequisite-patch-id: 7975f5bcaf28a99202d86d755fc256cd25be9cc4
prerequisite-patch-id: ee694b609c76628508d12fbe9b8b5a8a9354bfc4
prerequisite-patch-id: 1bb2f05c6a409a39421586e8148f82fa62ce6875
prerequisite-patch-id: a932945cb83fb631ebff3ac76ffff4319cacaabe

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

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

[Eliot Courtney]

Fix some inaccuracies / old doc comments.

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 436d1ff20705..cfae5b35adec 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -528,6 +528,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.
@@ -710,22 +711,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 1/4] gpu: nova-core: gsp: fix stale doc comments on command queue methods

[Zhi Wang]

On Wed, 25 Feb 2026 22:41:48 +0900
Eliot Courtney <ecourtney@nvidia.com> wrote:

Reviewed-by: Zhi Wang <zhiw@nvidia.com>

> Fix some inaccuracies / old doc comments.
> 
> 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 436d1ff20705..cfae5b35adec
> 100644 --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -528,6 +528,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. @@ -710,22 +711,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`.
> 

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

[Claude Code Review Bot]

Patch Review

Simple doc comment cleanup. This is a straightforward improvement.

The changes are accurate:
- Adding `EMSGSIZE` to the error docs for `send_command` (presumably added by the prerequisite split series).
- Replacing the outdated description of `receive_msg` with accurate behavior: it now returns `ERANGE` (not `EAGAIN`) for non-matching function codes, and the read pointer is always advanced.
- Changing `EINVAL` wording from "unrecognized" to "not recognized" and adding `ERANGE` for recognized-but-non-matching.
- Adding note about `MessageFromGsp::read` error propagation.

No issues. **Reviewed-by worthy.**

---

---
Generated by Claude Code Patch Reviewer

[PATCH 2/4] gpu: nova-core: gsp: add sync and async command queue API to `Cmdq`

[Eliot Courtney]

Add sync and async command queue API and the type infrastructure to know
what reply is expected from each `CommandToGsp`.

Use a marker type `NoReply` which does not implement `MessageFromGsp` to
mark async commands which don't expect a response.

This prepares for adding locking to the queue.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/boot.rs     |  5 ++--
 drivers/gpu/nova-core/gsp/cmdq.rs     | 54 ++++++++++++++++++++++++++++++++++-
 drivers/gpu/nova-core/gsp/commands.rs | 19 ++++++------
 3 files changed, 65 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index be427fe26a58..1cb21da855b9 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_async_command(bar, commands::SetSystemInfo::new(pdev))?;
+        self.cmdq
+            .send_async_command(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 cfae5b35adec..e1ca1bb9e07d 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -48,6 +48,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
@@ -66,6 +70,9 @@ 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 async commands.
+    type Reply;
+
     /// Error type returned by [`CommandToGsp::init`].
     type InitError;
 
@@ -604,7 +611,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<M>(&mut self, bar: &Bar0, command: M) -> Result
     where
         M: CommandToGsp,
         Error: From<M::InitError>,
@@ -626,6 +633,51 @@ pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
         Ok(())
     }
 
+    /// 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_sync_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(bar, command)?;
+
+        loop {
+            match self.receive_msg::<M::Reply>(Delta::from_secs(10)) {
+                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_async_command<M>(&mut self, bar: &Bar0, command: M) -> Result
+    where
+        M: CommandToGsp<Reply = NoReply>,
+        Error: From<M::InitError>,
+    {
+        self.send_command(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/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index 1683ebb4c685..b42e32dcc55c 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -26,7 +26,8 @@
             command_size,
             Cmdq,
             CommandToGsp,
-            MessageFromGsp, //
+            MessageFromGsp,
+            NoReply, //
         },
         fw::{
             commands::*,
@@ -53,6 +54,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> {
@@ -104,6 +106,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> {
@@ -183,6 +186,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> {
@@ -236,15 +240,7 @@ 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>(Delta::from_secs(5)) {
-            Ok(info) => return Ok(info),
-            Err(ERANGE) => continue,
-            Err(e) => return Err(e),
-        }
-    }
+    cmdq.send_sync_command(bar, GetGspStaticInfo)
 }
 
 /// The `ContinuationRecord` command.
@@ -262,6 +258,7 @@ pub(crate) 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> {
@@ -354,6 +351,7 @@ pub(crate) enum SplitCommand<'a, C: CommandToGsp> {
 impl<'a, C: CommandToGsp> CommandToGsp for SplitCommand<'a, 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> {
@@ -410,6 +408,7 @@ fn new(len: usize) -> Result<Self> {
     impl CommandToGsp for TestPayload {
         const FUNCTION: MsgFunction = MsgFunction::Nop;
         type Command = ();
+        type Reply = NoReply;
         type InitError = Infallible;
 
         fn init(&self) -> impl Init<Self::Command, Self::InitError> {

-- 
2.53.0

Re: [PATCH 2/4] gpu: nova-core: gsp: add sync and async command queue API to `Cmdq`

[Zhi Wang]

On Wed, 25 Feb 2026 22:41:49 +0900
Eliot Courtney <ecourtney@nvidia.com> wrote:

> Add sync and async command queue API and the type infrastructure to
> know what reply is expected from each `CommandToGsp`.
> 

snip

> +        loop {
> +            match self.receive_msg::<M::Reply>(Delta::from_secs(10))

It turns out here the timeout is changed to 10s from 5s which was used
in other places. Any problem you encountered during the debugging?

Z.

> {
> +                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_async_command<M>(&mut self, bar: &Bar0,
> command: M) -> Result
> +    where
> +        M: CommandToGsp<Reply = NoReply>,
> +        Error: From<M::InitError>,
> +    {
> +        self.send_command(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/commands.rs
> b/drivers/gpu/nova-core/gsp/commands.rs index
> 1683ebb4c685..b42e32dcc55c 100644 ---
> a/drivers/gpu/nova-core/gsp/commands.rs +++
> b/drivers/gpu/nova-core/gsp/commands.rs @@ -26,7 +26,8 @@
> command_size, Cmdq,
>              CommandToGsp,
> -            MessageFromGsp, //
> +            MessageFromGsp,
> +            NoReply, //
>          },
>          fw::{
>              commands::*,
> @@ -53,6 +54,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> {
> @@ -104,6 +106,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> {
> @@ -183,6 +186,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> {
> @@ -236,15 +240,7 @@ 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>(Delta::from_secs(5)) {
> -            Ok(info) => return Ok(info),
> -            Err(ERANGE) => continue,
> -            Err(e) => return Err(e),
> -        }
> -    }
> +    cmdq.send_sync_command(bar, GetGspStaticInfo)
>  }
>  
>  /// The `ContinuationRecord` command.
> @@ -262,6 +258,7 @@ pub(crate) 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> {
> @@ -354,6 +351,7 @@ pub(crate) enum SplitCommand<'a, C: CommandToGsp>
> { impl<'a, C: CommandToGsp> CommandToGsp for SplitCommand<'a, 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> {
> @@ -410,6 +408,7 @@ fn new(len: usize) -> Result<Self> {
>      impl CommandToGsp for TestPayload {
>          const FUNCTION: MsgFunction = MsgFunction::Nop;
>          type Command = ();
> +        type Reply = NoReply;
>          type InitError = Infallible;
>  
>          fn init(&self) -> impl Init<Self::Command, Self::InitError> {
> 

Re: [PATCH 2/4] gpu: nova-core: gsp: add sync and async command queue API to `Cmdq`

[Eliot Courtney]

On Thu Feb 26, 2026 at 4:42 AM JST, Zhi Wang wrote:
> On Wed, 25 Feb 2026 22:41:49 +0900
> Eliot Courtney <ecourtney@nvidia.com> wrote:
>
>> Add sync and async command queue API and the type infrastructure to
>> know what reply is expected from each `CommandToGsp`.
>> 
>
> snip
>
>> +        loop {
>> +            match self.receive_msg::<M::Reply>(Delta::from_secs(10))
>
> It turns out here the timeout is changed to 10s from 5s which was used
> in other places. Any problem you encountered during the debugging?

No issues, but it /looked/ to me like the particular values didn't
really matter so it would be fine and simpler to have a uniform 10
second timeout for now. Please LMK if you know this is wrong and the
5 second value is important and specifically chosen.

I think it's also fine to include the timeout as a parameter and would
also fit the convention with receive_msg.

Thanks for the review~

Claude review: gpu: nova-core: gsp: add sync and async command queue API to `Cmdq`

[Claude Code Review Bot]

Patch Review

This is the core type infrastructure patch. Key observations:

**The `NoReply` marker type is well-designed:**
```rust
pub(crate) struct NoReply;
```
Not implementing `MessageFromGsp` means `send_sync_command` can't be called with async commands — the compiler enforces it via `M::Reply: MessageFromGsp`.

**Associated type on `CommandToGsp`:**
```rust
type Reply;
```
Clean addition. The cover letter discusses the alternative of separate `SyncCommand`/`AsyncCommand` traits, and I agree the associated type approach is more composable.

**`send_sync_command` implementation:**
```rust
pub(crate) fn send_sync_command<M>(&mut self, bar: &Bar0, command: M) -> Result<M::Reply>
{
    self.send_command(bar, command)?;

    loop {
        match self.receive_msg::<M::Reply>(Delta::from_secs(10)) {
            Ok(reply) => break Ok(reply),
            Err(ERANGE) => continue,
            Err(e) => break Err(e),
        }
    }
}
```
This correctly extracts the existing pattern from `get_gsp_info`. Note the timeout change from 5s to 10s as mentioned above — this is fine but worth acknowledging.

**`send_async_command` is a thin wrapper:**
```rust
pub(crate) fn send_async_command<M>(&mut self, bar: &Bar0, command: M) -> Result
where
    M: CommandToGsp<Reply = NoReply>,
{
    self.send_command(bar, command)
}
```
The `Reply = NoReply` constraint is the enforcement mechanism. Clean.

**Making `send_command` private (`fn` instead of `pub(crate) fn`):**
```rust
-    pub(crate) fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
+    fn send_command<M>(&mut self, bar: &Bar0, command: M) -> Result
```
Good — callers should now go through the sync/async API.

**`SplitCommand` propagates `Reply`:**
```rust
+    type Reply = C::Reply;
```
Correct — a split command should have the same reply type as the underlying command.

**Caller updates in boot.rs:**
```rust
-            .send_command(bar, commands::SetSystemInfo::new(pdev))?;
+            .send_async_command(bar, commands::SetSystemInfo::new(pdev))?;
```
Correct — these pre-boot commands are fire-and-forget.

**Refactoring of `get_gsp_info`:**
```rust
-    cmdq.send_command(bar, GetGspStaticInfo)?;
-    loop {
-        match cmdq.receive_msg::<GetGspStaticInfoReply>(Delta::from_secs(5)) {
-            Ok(info) => return Ok(info),
-            Err(ERANGE) => continue,
-            Err(e) => return Err(e),
-        }
-    }
+    cmdq.send_sync_command(bar, GetGspStaticInfo)
```
Nice simplification.

No issues. **Reviewed-by worthy.**

---

---
Generated by Claude Code Patch Reviewer

[PATCH 3/4] 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`.

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 e1ca1bb9e07d..44c3e960c965 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -474,6 +474,7 @@ pub(crate) fn command_size<M>(command: &M) -> usize
 ///
 /// 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>,
@@ -501,13 +502,11 @@ impl Cmdq {
     pub(crate) const NUM_PTES: usize = size_of::<GspMem>() >> GSP_PAGE_SHIFT;
 
     /// 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

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

[Zhi Wang]

On Wed, 25 Feb 2026 22:41:50 +0900
Eliot Courtney <ecourtney@nvidia.com> wrote:

Looks good to me.

Reviewed-by: Zhi Wang <zhiw@nvidia.com>

> Make `Cmdq` a pinned type. This is needed to use Mutex, which is
> needed to add locking to `Cmdq`.
> 
> 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 e1ca1bb9e07d..44c3e960c965
> 100644 --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -474,6 +474,7 @@ pub(crate) fn command_size<M>(command: &M) ->
> usize ///
>  /// 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>,
> @@ -501,13 +502,11 @@ impl Cmdq {
>      pub(crate) const NUM_PTES: usize = size_of::<GspMem>() >>
> GSP_PAGE_SHIFT; 
>      /// 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,
>          })
>      }
>  
> 

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

[Claude Code Review Bot]

Patch Review

Mechanical change to prepare for `Mutex` in patch 4.

**`#[pin_data]` annotation added:**
```rust
+#[pin_data]
 pub(crate) struct Cmdq {
```

**Constructor returns `PinInit` instead of `Result<Cmdq>`:**
```rust
-    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,
         })
```

**Parent struct uses pin-init syntax:**
```rust
-                cmdq: Cmdq::new(dev)?,
+                cmdq <- Cmdq::new(dev),
```

**Reference update for `GspArgumentsCached::new`:**
```rust
-                    dma_write!(rmargs[0].inner = fw::GspArgumentsCached::new(cmdq))?;
+                    dma_write!(rmargs[0].inner = fw::GspArgumentsCached::new(&cmdq))?;
```
Correct — after pin-init, `cmdq` is accessed by reference via the pin projection, so the explicit `&` is needed.

Straightforward and correct. **Reviewed-by worthy.**

---

---
Generated by Claude Code Patch Reviewer

[PATCH 4/4] 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_sync_command` to
make sure that it doesn't get the reply of some other command that could
have been sent just beforehand.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/boot.rs      |   8 +-
 drivers/gpu/nova-core/gsp/cmdq.rs      | 260 ++++++++++++++++++---------------
 drivers/gpu/nova-core/gsp/commands.rs  |   4 +-
 drivers/gpu/nova-core/gsp/sequencer.rs |   2 +-
 4 files changed, 152 insertions(+), 122 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 1cb21da855b9..cb583f57666a 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,
@@ -232,13 +232,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.as_ref(), "GPU name: {}\n", name),
             Err(e) => dev_warn!(pdev.as_ref(), "GPU name unavailable: {:?}\n", e),
diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 44c3e960c965..faf1e9d5072b 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -16,8 +16,12 @@
     },
     dma_write,
     io::poll::read_poll_timeout,
+    new_mutex,
     prelude::*,
-    sync::aref::ARef,
+    sync::{
+        aref::ARef,
+        Mutex, //
+    },
     time::Delta,
     transmute::{
         AsBytes,
@@ -54,8 +58,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.
@@ -470,66 +474,15 @@ pub(crate) fn command_size<M>(command: &M) -> usize
     size_of::<M::Command>() + command.variable_payload_len()
 }
 
-/// 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 {
-    /// Device this command queue belongs to.
-    dev: ARef<device::Device>,
+/// Inner mutex protected state of [`Cmdq`].
+struct CmdqInner {
     /// Current command sequence number.
     seq: u32,
     /// Memory area shared with the GSP for communicating commands and messages.
     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;
-
-    /// 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);
-    }
-
+impl CmdqInner {
     /// Sends `command` to the GSP, without splitting it.
     ///
     /// # Errors
@@ -540,7 +493,7 @@ fn notify_gsp(bar: &Bar0) {
     ///   written to by its [`CommandToGsp::init_variable_payload`] method.
     ///
     /// Error codes returned by the command initializers are propagated as-is.
-    fn send_single_command<M>(&mut self, bar: &Bar0, command: M) -> Result
+    fn send_single_command<M>(&mut self, dev: &device::Device, bar: &Bar0, command: M) -> Result
     where
         M: CommandToGsp,
         // This allows all error types, including `Infallible`, to be used for `M::InitError`.
@@ -583,7 +536,7 @@ fn send_single_command<M>(&mut self, bar: &Bar0, command: M) -> Result
             ])));
 
         dev_dbg!(
-            &self.dev,
+            dev,
             "GSP RPC: send: seq# {}, function={}, length=0x{:x}\n",
             self.seq,
             M::FUNCTION,
@@ -610,73 +563,27 @@ 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<M>(&mut self, bar: &Bar0, command: M) -> Result
+    fn send_command<M>(&mut self, dev: &device::Device, bar: &Bar0, command: M) -> Result
     where
         M: CommandToGsp,
         Error: From<M::InitError>,
     {
         let mut state = SplitState::new(&command)?;
-
-        self.send_single_command(bar, state.command(command))?;
+        self.send_single_command(dev, bar, state.command(command))?;
 
         while let Some(continuation) = state.next_continuation_record() {
             dev_dbg!(
-                &self.dev,
+                dev,
                 "GSP RPC: send continuation: size=0x{:x}\n",
                 command_size(&continuation),
             );
             // Turbofish needed because the compiler cannot infer M here.
-            self.send_single_command::<ContinuationRecord<'_>>(bar, continuation)?;
+            self.send_single_command::<ContinuationRecord<'_>>(dev, bar, continuation)?;
         }
 
         Ok(())
     }
 
-    /// 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_sync_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(bar, command)?;
-
-        loop {
-            match self.receive_msg::<M::Reply>(Delta::from_secs(10)) {
-                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_async_command<M>(&mut self, bar: &Bar0, command: M) -> Result
-    where
-        M: CommandToGsp<Reply = NoReply>,
-        Error: From<M::InitError>,
-    {
-        self.send_command(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
@@ -695,7 +602,7 @@ pub(crate) fn send_async_command<M>(&mut self, bar: &Bar0, command: M) -> Result
     ///   message queue.
     ///
     /// Error codes returned by the message constructor are propagated as-is.
-    fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
+    fn wait_for_msg(&self, dev: &device::Device, timeout: Delta) -> Result<GspMessage<'_>> {
         // Wait for a message to arrive from the GSP.
         let (slice_1, slice_2) = read_poll_timeout(
             || Ok(self.gsp_mem.driver_read_area()),
@@ -712,7 +619,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,
+            dev,
             "GSP RPC: receive: seq# {}, function={:?}, length=0x{:x}\n",
             header.sequence(),
             header.function(),
@@ -747,7 +654,7 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
         ])) != 0
         {
             dev_err!(
-                self.dev,
+                dev,
                 "GSP RPC: receive: Call {} - bad checksum\n",
                 header.sequence()
             );
@@ -776,12 +683,12 @@ 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, dev: &device::Device, timeout: Delta) -> Result<M>
     where
         // This allows all error types, including `Infallible`, to be used for `M::InitError`.
         Error: From<M::InitError>,
     {
-        let message = self.wait_for_msg(timeout)?;
+        let message = self.wait_for_msg(dev, timeout)?;
         let function = message.header.function().map_err(|_| EINVAL)?;
 
         // Extract the message. Store the result as we want to advance the read pointer even in
@@ -802,9 +709,132 @@ 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 {
+    /// Device this command queue belongs to.
+    dev: ARef<device::Device>,
+    /// 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;
+
+    /// 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 {
+                gsp_mem: DmaGspMem::new(dev)?,
+                seq: 0,
+                }),
+            dev: dev.into(),
+        })
+    }
+
+    /// 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_sync_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(&self.dev, bar, command)?;
+
+        loop {
+            match inner.receive_msg::<M::Reply>(&self.dev, Delta::from_secs(10)) {
+                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_async_command<M>(&self, bar: &Bar0, command: M) -> Result
+    where
+        M: CommandToGsp<Reply = NoReply>,
+        Error: From<M::InitError>,
+    {
+        self.inner.lock().send_command(&self.dev, 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(&self.dev, 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 b42e32dcc55c..7ceff2e6bd63 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -170,7 +170,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>(Delta::from_secs(10)) {
             Ok(_) => break Ok(()),
@@ -239,7 +239,7 @@ 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_sync_command(bar, GetGspStaticInfo)
 }
 
diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs b/drivers/gpu/nova-core/gsp/sequencer.rs
index 2fa2a0792cec..bc94b8567c6a 100644
--- a/drivers/gpu/nova-core/gsp/sequencer.rs
+++ b/drivers/gpu/nova-core/gsp/sequencer.rs
@@ -366,7 +366,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>(Delta::from_secs(10)) {
                 Ok(seq_info) => break seq_info,

-- 
2.53.0

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

[Zhi Wang]

On Wed, 25 Feb 2026 22:41:51 +0900
Eliot Courtney <ecourtney@nvidia.com> wrote:

Looks good to me.

Reviewed-by: Zhi Wang <zhiw@nvidia.com>

> 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_sync_command` to
> make sure that it doesn't get the reply of some other command that
> could have been sent just beforehand.
> 
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  drivers/gpu/nova-core/gsp/boot.rs      |   8 +-
>  drivers/gpu/nova-core/gsp/cmdq.rs      | 260
> ++++++++++++++++++---------------
> drivers/gpu/nova-core/gsp/commands.rs  |   4 +-
> drivers/gpu/nova-core/gsp/sequencer.rs |   2 +- 4 files changed, 152
> insertions(+), 122 deletions(-)
> 
> diff --git a/drivers/gpu/nova-core/gsp/boot.rs
> b/drivers/gpu/nova-core/gsp/boot.rs index 1cb21da855b9..cb583f57666a
> 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,
> @@ -232,13 +232,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.as_ref(), "GPU name: {}\n",
> name), Err(e) => dev_warn!(pdev.as_ref(), "GPU name unavailable:
> {:?}\n", e), diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs
> b/drivers/gpu/nova-core/gsp/cmdq.rs index 44c3e960c965..faf1e9d5072b
> 100644 --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -16,8 +16,12 @@
>      },
>      dma_write,
>      io::poll::read_poll_timeout,
> +    new_mutex,
>      prelude::*,
> -    sync::aref::ARef,
> +    sync::{
> +        aref::ARef,
> +        Mutex, //
> +    },
>      time::Delta,
>      transmute::{
>          AsBytes,
> @@ -54,8 +58,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. @@ -470,66 +474,15 @@ pub(crate) fn
> command_size<M>(command: &M) -> usize size_of::<M::Command>() +
> command.variable_payload_len() }
>  
> -/// 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 {
> -    /// Device this command queue belongs to.
> -    dev: ARef<device::Device>,
> +/// Inner mutex protected state of [`Cmdq`].
> +struct CmdqInner {
>      /// Current command sequence number.
>      seq: u32,
>      /// Memory area shared with the GSP for communicating commands
> and messages. 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; -
> -    /// 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);
> -    }
> -
> +impl CmdqInner {
>      /// Sends `command` to the GSP, without splitting it.
>      ///
>      /// # Errors
> @@ -540,7 +493,7 @@ fn notify_gsp(bar: &Bar0) {
>      ///   written to by its [`CommandToGsp::init_variable_payload`]
> method. ///
>      /// Error codes returned by the command initializers are
> propagated as-is.
> -    fn send_single_command<M>(&mut self, bar: &Bar0, command: M) ->
> Result
> +    fn send_single_command<M>(&mut self, dev: &device::Device, bar:
> &Bar0, command: M) -> Result where
>          M: CommandToGsp,
>          // This allows all error types, including `Infallible`, to
> be used for `M::InitError`. @@ -583,7 +536,7 @@ fn
> send_single_command<M>(&mut self, bar: &Bar0, command: M) -> Result
> ]))); 
>          dev_dbg!(
> -            &self.dev,
> +            dev,
>              "GSP RPC: send: seq# {}, function={}, length=0x{:x}\n",
>              self.seq,
>              M::FUNCTION,
> @@ -610,73 +563,27 @@ 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<M>(&mut self, bar: &Bar0, command: M) -> Result
> +    fn send_command<M>(&mut self, dev: &device::Device, bar: &Bar0,
> command: M) -> Result where
>          M: CommandToGsp,
>          Error: From<M::InitError>,
>      {
>          let mut state = SplitState::new(&command)?;
> -
> -        self.send_single_command(bar, state.command(command))?;
> +        self.send_single_command(dev, bar, state.command(command))?;
>  
>          while let Some(continuation) =
> state.next_continuation_record() { dev_dbg!(
> -                &self.dev,
> +                dev,
>                  "GSP RPC: send continuation: size=0x{:x}\n",
>                  command_size(&continuation),
>              );
>              // Turbofish needed because the compiler cannot infer M
> here.
> -            self.send_single_command::<ContinuationRecord<'_>>(bar,
> continuation)?;
> +            self.send_single_command::<ContinuationRecord<'_>>(dev,
> bar, continuation)?; }
>  
>          Ok(())
>      }
>  
> -    /// 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_sync_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(bar, command)?;
> -
> -        loop {
> -            match self.receive_msg::<M::Reply>(Delta::from_secs(10))
> {
> -                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_async_command<M>(&mut self, bar: &Bar0,
> command: M) -> Result
> -    where
> -        M: CommandToGsp<Reply = NoReply>,
> -        Error: From<M::InitError>,
> -    {
> -        self.send_command(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 @@ -695,7 +602,7 @@ pub(crate) fn
> send_async_command<M>(&mut self, bar: &Bar0, command: M) -> Result
> ///   message queue. ///
>      /// Error codes returned by the message constructor are
> propagated as-is.
> -    fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>>
> {
> +    fn wait_for_msg(&self, dev: &device::Device, timeout: Delta) ->
> Result<GspMessage<'_>> { // Wait for a message to arrive from the GSP.
>          let (slice_1, slice_2) = read_poll_timeout(
>              || Ok(self.gsp_mem.driver_read_area()),
> @@ -712,7 +619,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,
> +            dev,
>              "GSP RPC: receive: seq# {}, function={:?},
> length=0x{:x}\n", header.sequence(),
>              header.function(),
> @@ -747,7 +654,7 @@ fn wait_for_msg(&self, timeout: Delta) ->
> Result<GspMessage<'_>> { ])) != 0
>          {
>              dev_err!(
> -                self.dev,
> +                dev,
>                  "GSP RPC: receive: Call {} - bad checksum\n",
>                  header.sequence()
>              );
> @@ -776,12 +683,12 @@ 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, dev:
> &device::Device, timeout: Delta) -> Result<M> where
>          // This allows all error types, including `Infallible`, to
> be used for `M::InitError`. Error: From<M::InitError>,
>      {
> -        let message = self.wait_for_msg(timeout)?;
> +        let message = self.wait_for_msg(dev, timeout)?;
>          let function = message.header.function().map_err(|_|
> EINVAL)?; 
>          // Extract the message. Store the result as we want to
> advance the read pointer even in @@ -802,9 +709,132 @@ 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 {
> +    /// Device this command queue belongs to.
> +    dev: ARef<device::Device>,
> +    /// 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; +
> +    /// 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 {
> +                gsp_mem: DmaGspMem::new(dev)?,
> +                seq: 0,
> +                }),
> +            dev: dev.into(),
> +        })
> +    }
> +
> +    /// 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_sync_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(&self.dev, bar, command)?;
> +
> +        loop {
> +            match inner.receive_msg::<M::Reply>(&self.dev,
> Delta::from_secs(10)) {
> +                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_async_command<M>(&self, bar: &Bar0, command:
> M) -> Result
> +    where
> +        M: CommandToGsp<Reply = NoReply>,
> +        Error: From<M::InitError>,
> +    {
> +        self.inner.lock().send_command(&self.dev, 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(&self.dev, 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
> b42e32dcc55c..7ceff2e6bd63 100644 ---
> a/drivers/gpu/nova-core/gsp/commands.rs +++
> b/drivers/gpu/nova-core/gsp/commands.rs @@ -170,7 +170,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>(Delta::from_secs(10)) {
>              Ok(_) => break Ok(()),
> @@ -239,7 +239,7 @@ 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_sync_command(bar, GetGspStaticInfo) }
>  
> diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs
> b/drivers/gpu/nova-core/gsp/sequencer.rs index
> 2fa2a0792cec..bc94b8567c6a 100644 ---
> a/drivers/gpu/nova-core/gsp/sequencer.rs +++
> b/drivers/gpu/nova-core/gsp/sequencer.rs @@ -366,7 +366,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>(Delta::from_secs(10)) { Ok(seq_info)
> => break seq_info,
> 

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

[Claude Code Review Bot]

Patch Review

This is the main patch. It splits `Cmdq` into an outer struct (with `dev` and the `Mutex`) and `CmdqInner` (with `seq` and `gsp_mem`).

**The split is clean:**
```rust
struct CmdqInner {
    seq: u32,
    gsp_mem: DmaGspMem,
}

pub(crate) struct Cmdq {
    dev: ARef<device::Device>,
    #[pin]
    inner: Mutex<CmdqInner>,
}
```

**Methods that need `&mut` go to `CmdqInner`, stateless helpers stay on `Cmdq`.** The `dev` reference is passed as a parameter to `CmdqInner` methods. This is a reasonable pattern to avoid putting `dev` inside the mutex.

**`send_sync_command` holds the lock across send+receive:**
```rust
pub(crate) fn send_sync_command<M>(&self, bar: &Bar0, command: M) -> Result<M::Reply> {
    let mut inner = self.inner.lock();
    inner.send_command(&self.dev, bar, command)?;
    loop {
        match inner.receive_msg::<M::Reply>(&self.dev, Delta::from_secs(10)) {
            Ok(reply) => break Ok(reply),
            Err(ERANGE) => continue,
            Err(e) => break Err(e),
        }
    }
}
```
This is the correct approach for reply integrity. As noted above, it does block other queue access while waiting. The `read_poll_timeout` inside `wait_for_msg` will sleep (via `Delta::from_millis(1)` polling interval), so at least the mutex won't be hot-spinning — the kernel `Mutex` is a sleeping lock. This is important and correct.

**`send_async_command` only holds the lock for the send:**
```rust
pub(crate) fn send_async_command<M>(&self, bar: &Bar0, command: M) -> Result {
    self.inner.lock().send_command(&self.dev, bar, command)
}
```
Good — drops the lock immediately after sending.

**`receive_msg` on outer Cmdq:**
```rust
pub(crate) fn receive_msg<M: MessageFromGsp>(&self, timeout: Delta) -> Result<M> {
    self.inner.lock().receive_msg(&self.dev, timeout)
}
```
Used by `wait_gsp_init_done` and the sequencer. The lock is held during the poll timeout, same concern as above. For the init-done case, this is a single-threaded initialization path, so it's fine.

**`dma_handle()` concern:**
```rust
pub(crate) fn dma_handle(&self) -> DmaAddress {
    self.inner.lock().gsp_mem.0.dma_handle()
}
```
As noted in the overall review, this takes the lock to read an immutable value. Minor issue — could be optimized by storing the DMA handle (or the entire `DmaGspMem`) outside the mutex, but this is not a correctness problem and can be addressed later.

**All callers updated from `&mut self` / `&mut Cmdq` to `&self` / `&Cmdq`:**
```rust
-        mut self: Pin<&mut Self>,
+        self: Pin<&mut Self>,
```
```rust
-pub(crate) fn wait_gsp_init_done(cmdq: &mut Cmdq) -> Result {
+pub(crate) fn wait_gsp_init_done(cmdq: &Cmdq) -> Result {
```
```rust
-    pub(crate) fn run(cmdq: &mut Cmdq, ...) -> Result {
+    pub(crate) fn run(cmdq: &Cmdq, ...) -> Result {
```
All consistent and correct.

**Minor formatting nit:** In the `new_mutex!` invocation, the closing `})` has inconsistent indentation:
```rust
        try_pin_init!(Self {
            inner <- new_mutex!(CmdqInner {
                gsp_mem: DmaGspMem::new(dev)?,
                seq: 0,
                }),
            dev: dev.into(),
        })
```
The `})` closing `new_mutex!` should probably be at the same level as `inner`, i.e.:
```rust
            inner <- new_mutex!(CmdqInner {
                gsp_mem: DmaGspMem::new(dev)?,
                seq: 0,
            }),
```

This is purely cosmetic.

Overall a solid patch. **Reviewed-by worthy** with the optional consideration of moving `dma_handle` out of the locked state.

---
Generated by Claude Code Patch Reviewer

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: 10
Reviewed: 2026-02-27T13:18:03.810448

---

This is a well-structured 4-patch series that adds mutex locking to the GSP command queue (`Cmdq`) in the nova-core Rust GPU driver. The series has clear prerequisites (John's cmdq split/continuation series and possibly others) which explains why it doesn't apply to drm-next.

The approach is sound: distinguish sync from async commands at the type level using an associated `Reply` type, make `Cmdq` a pinned type (required for kernel `Mutex`), then protect the mutable state with a `Mutex` so the API takes `&self` instead of `&mut self`. This enables concurrent access from different threads (e.g., during driver unbind).

**Key design decisions that look correct:**
- Holding the mutex across both send and receive in `send_sync_command` prevents reply mismatches between concurrent sync commands.
- Using `NoReply` as a marker type with the `Reply = NoReply` constraint on `send_async_command` provides compile-time enforcement.
- Moving `calculate_checksum` and `notify_gsp` (which don't need `&mut self`) to the outer `Cmdq` struct, while keeping actual state-mutating methods on `CmdqInner`, is a clean split.

**Concerns:**

1. **`dma_handle()` takes the lock**: After patch 4, `dma_handle()` acquires the mutex just to read the DMA address, which is immutable after construction. This is wasteful and could be a problem if the DMA address is needed while the lock is held elsewhere. The DMA handle could be stored as a separate field on `Cmdq` (outside the mutex), or `DmaGspMem` could be split so the handle is accessible without locking.

2. **`send_sync_command` busy-loops on `ERANGE` while holding the mutex**: If unexpected messages arrive (non-matching function codes), the loop silently consumes them while blocking all other command queue access. This means async commands cannot be sent while a sync command is waiting for its reply. The cover letter acknowledges this trade-off and says GSP is expected to be fast, which is acceptable for now, but should be documented as a known limitation.

3. **Timeout accumulation**: The `receive_msg` call inside the `send_sync_command` loop uses a 10-second timeout per attempt. Since `ERANGE` causes a `continue`, the effective timeout can be `N * 10` seconds if many non-matching messages arrive. This differs from the original `get_gsp_info` which used 5-second timeouts — the series changes this to 10 seconds.

Overall the series is clean and well-structured. The concerns above are mostly future considerations rather than blockers.

---

---
Generated by Claude Code Patch Reviewer

[PATCH v2 0/4] 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 ("sync") and commands that don't
("async"). For sync commands 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 sync 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.
SyncCommand and AsyncCommand, 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 send a sync or async
   command.
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 v2:
- Rebase on drm-rust-next
- Link to v1: https://lore.kernel.org/r/20260225-cmdq-locking-v1-0-bbf6b4156706@nvidia.com

---
Eliot Courtney (4):
      gpu: nova-core: gsp: fix stale doc comments on command queue methods
      gpu: nova-core: gsp: add sync and async command queue API to `Cmdq`
      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      | 246 +++++++++++++++++++++------------
 drivers/gpu/nova-core/gsp/commands.rs  |  23 ++-
 drivers/gpu/nova-core/gsp/sequencer.rs |   2 +-
 5 files changed, 183 insertions(+), 106 deletions(-)
---
base-commit: 4a49fe23e357b48845e31fe9c28a802c05458198
change-id: 20260225-cmdq-locking-d32928a2c2cf
prerequisite-message-id: <20260226-cmdq-continuation-v3-0-572ab9916766@nvidia.com>
prerequisite-patch-id: fd45bc5b8eda5e2b54a052dddb1a1c363107f0a7
prerequisite-patch-id: d0f59ef489346e978a222755f9fb42dfe7af19e5
prerequisite-patch-id: 8844970d0e387488c70979a73732579ba174b46c
prerequisite-patch-id: e138a94ed48fa8d50d5ed1eb36524f98923ed478
prerequisite-patch-id: 4599a5e90d6665fa3acb7d4045de5d378ac28b4d
prerequisite-patch-id: 30ed64c398e541d6efbcb2e46ed9a9e6cf953f4f
prerequisite-patch-id: 9a965e9f29c8680c0b554e656ff8e9a1bfc67280
prerequisite-patch-id: d8cccc3dfb070f304805fc7e0f24121809b4b300
prerequisite-patch-id: c0a73dfd1fb630ab02486f0180b90f8fe850b4dc

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

[PATCH v2 4/4] 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_sync_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>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/boot.rs      |   8 +-
 drivers/gpu/nova-core/gsp/cmdq.rs      | 266 ++++++++++++++++++---------------
 drivers/gpu/nova-core/gsp/commands.rs  |   4 +-
 drivers/gpu/nova-core/gsp/sequencer.rs |   2 +-
 4 files changed, 153 insertions(+), 127 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 55899eba75db..d12ad1bd2cd8 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 6bb1decd2af5..5010587c96f9 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -16,8 +16,12 @@
     },
     dma_write,
     io::poll::read_poll_timeout,
+    new_mutex,
     prelude::*,
-    sync::aref::ARef,
+    sync::{
+        aref::ARef,
+        Mutex, //
+    },
     time::Delta,
     transmute::{
         AsBytes,
@@ -54,8 +58,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.
@@ -470,66 +474,15 @@ pub(crate) fn command_size<M>(command: &M) -> usize
     size_of::<M::Command>() + command.variable_payload_len()
 }
 
-/// 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 {
-    /// Device this command queue belongs to.
-    dev: ARef<device::Device>,
+/// Inner mutex protected state of [`Cmdq`].
+struct CmdqInner {
     /// Current command sequence number.
     seq: u32,
     /// Memory area shared with the GSP for communicating commands and messages.
     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;
-
-    /// 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);
-    }
-
+impl CmdqInner {
     /// Sends `command` to the GSP, without splitting it.
     ///
     /// # Errors
@@ -540,7 +493,7 @@ fn notify_gsp(bar: &Bar0) {
     ///   written to by its [`CommandToGsp::init_variable_payload`] method.
     ///
     /// Error codes returned by the command initializers are propagated as-is.
-    fn send_single_command<M>(&mut self, bar: &Bar0, command: M) -> Result
+    fn send_single_command<M>(&mut self, dev: &device::Device, bar: &Bar0, command: M) -> Result
     where
         M: CommandToGsp,
         // This allows all error types, including `Infallible`, to be used for `M::InitError`.
@@ -583,7 +536,7 @@ fn send_single_command<M>(&mut self, bar: &Bar0, command: M) -> Result
             ])));
 
         dev_dbg!(
-            &self.dev,
+            dev,
             "GSP RPC: send: seq# {}, function={:?}, length=0x{:x}\n",
             self.seq,
             M::FUNCTION,
@@ -610,73 +563,27 @@ 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<M>(&mut self, bar: &Bar0, command: M) -> Result
+    fn send_command<M>(&mut self, dev: &device::Device, bar: &Bar0, command: M) -> Result
     where
         M: CommandToGsp,
         Error: From<M::InitError>,
     {
         let mut state = SplitState::new(&command)?;
-
-        self.send_single_command(bar, state.command(command))?;
+        self.send_single_command(dev, bar, state.command(command))?;
 
         while let Some(continuation) = state.next_continuation_record() {
             dev_dbg!(
-                &self.dev,
+                dev,
                 "GSP RPC: send continuation: size=0x{:x}\n",
                 command_size(&continuation),
             );
             // Turbofish needed because the compiler cannot infer M here.
-            self.send_single_command::<ContinuationRecord<'_>>(bar, continuation)?;
+            self.send_single_command::<ContinuationRecord<'_>>(dev, bar, continuation)?;
         }
 
         Ok(())
     }
 
-    /// 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_sync_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(bar, command)?;
-
-        loop {
-            match self.receive_msg::<M::Reply>(Delta::from_secs(10)) {
-                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_async_command<M>(&mut self, bar: &Bar0, command: M) -> Result
-    where
-        M: CommandToGsp<Reply = NoReply>,
-        Error: From<M::InitError>,
-    {
-        self.send_command(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
@@ -695,7 +602,7 @@ pub(crate) fn send_async_command<M>(&mut self, bar: &Bar0, command: M) -> Result
     ///   message queue.
     ///
     /// Error codes returned by the message constructor are propagated as-is.
-    fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
+    fn wait_for_msg(&self, dev: &device::Device, timeout: Delta) -> Result<GspMessage<'_>> {
         // Wait for a message to arrive from the GSP.
         let (slice_1, slice_2) = read_poll_timeout(
             || Ok(self.gsp_mem.driver_read_area()),
@@ -712,7 +619,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,
+            dev,
             "GSP RPC: receive: seq# {}, function={:?}, length=0x{:x}\n",
             header.sequence(),
             header.function(),
@@ -747,7 +654,7 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
         ])) != 0
         {
             dev_err!(
-                self.dev,
+                dev,
                 "GSP RPC: receive: Call {} - bad checksum\n",
                 header.sequence()
             );
@@ -776,12 +683,12 @@ 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, dev: &device::Device, timeout: Delta) -> Result<M>
     where
         // This allows all error types, including `Infallible`, to be used for `M::InitError`.
         Error: From<M::InitError>,
     {
-        let message = self.wait_for_msg(timeout)?;
+        let message = self.wait_for_msg(dev, timeout)?;
         let function = message.header.function().map_err(|_| EINVAL)?;
 
         // Extract the message. Store the result as we want to advance the read pointer even in
@@ -794,11 +701,7 @@ pub(crate) fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Resul
                 .map_err(|e| e.into())
                 .inspect(|_| {
                     if !sbuffer.is_empty() {
-                        dev_warn!(
-                            &self.dev,
-                            "GSP message {:?} has unprocessed data\n",
-                            function
-                        );
+                        dev_warn!(dev, "GSP message {:?} has unprocessed data\n", function);
                     }
                 })
         } else {
@@ -812,9 +715,132 @@ 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 {
+    /// Device this command queue belongs to.
+    dev: ARef<device::Device>,
+    /// 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;
+
+    /// 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 {
+                gsp_mem: DmaGspMem::new(dev)?,
+                seq: 0,
+                }),
+            dev: dev.into(),
+        })
+    }
+
+    /// 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_sync_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(&self.dev, bar, command)?;
+
+        loop {
+            match inner.receive_msg::<M::Reply>(&self.dev, Delta::from_secs(10)) {
+                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_async_command<M>(&self, bar: &Bar0, command: M) -> Result
+    where
+        M: CommandToGsp<Reply = NoReply>,
+        Error: From<M::InitError>,
+    {
+        self.inner.lock().send_command(&self.dev, 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(&self.dev, 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 47ca31611927..4740cda0b51c 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -170,7 +170,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>(Delta::from_secs(10)) {
             Ok(_) => break Ok(()),
@@ -239,7 +239,7 @@ 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_sync_command(bar, GetGspStaticInfo)
 }
 
diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs b/drivers/gpu/nova-core/gsp/sequencer.rs
index 0cfbedc47fcf..f99f4fe652ba 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>(Delta::from_secs(10)) {
                 Ok(seq_info) => break seq_info,

-- 
2.53.0

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

[Claude Code Review Bot]

Patch Review

**Status: Looks good, a few observations**

This is the main patch, splitting `Cmdq` into an outer struct (holding `dev` and the `Mutex`) and `CmdqInner` (holding mutable state: `seq` and `gsp_mem`).

The split is well-motivated:
```rust
+struct CmdqInner {
+    seq: u32,
+    gsp_mem: DmaGspMem,
+}
+
+pub(crate) struct Cmdq {
+    dev: ARef<device::Device>,
+    #[pin]
+    inner: Mutex<CmdqInner>,
+}
```

Moving `dev` outside the mutex is correct — it's `ARef` (immutable, reference-counted) and doesn't need mutex protection.

The `send_sync_command` correctly holds the lock across send+receive:
```rust
+    pub(crate) fn send_sync_command<M>(&self, bar: &Bar0, command: M) -> Result<M::Reply>
+    ...
+    {
+        let mut inner = self.inner.lock();
+        inner.send_command(&self.dev, bar, command)?;
+        loop {
+            match inner.receive_msg::<M::Reply>(&self.dev, Delta::from_secs(10)) {
```
This ensures no interleaving of commands and their replies. Methods now take `&self` instead of `&mut self`, which is the whole point.

Moving `calculate_checksum` and `notify_gsp` to `Cmdq` (rather than `CmdqInner`) makes sense since they don't use any mutable state — they're effectively static methods.

**Observations:**

1. **`dma_handle()` takes the lock unnecessarily:**
```rust
+    pub(crate) fn dma_handle(&self) -> DmaAddress {
+        self.inner.lock().gsp_mem.0.dma_handle()
+    }
```
The DMA handle is set once during allocation and never changes. This could instead store the DMA handle in the outer `Cmdq` struct at construction time, avoiding the lock. However, this is a minor performance point and correctness is not affected.

2. **`receive_msg` is exposed as `pub(crate)` on `Cmdq`:**
```rust
+    pub(crate) fn receive_msg<M: MessageFromGsp>(&self, timeout: Delta) -> Result<M>
+    ...
+    {
+        self.inner.lock().receive_msg(&self.dev, timeout)
+    }
```
This is used by `wait_gsp_init_done` and `GspSequencer::run` which receive messages without a preceding send. The lock is acquired and released per call, which is correct for these use cases. However, if someone were to call this in a loop (as `wait_gsp_init_done` does), each iteration re-acquires the lock — this is fine for correctness and allows other operations to interleave.

3. **Thread passing `dev` through method parameters:** The `CmdqInner` methods now take `dev: &device::Device` as a parameter instead of accessing `self.dev`, since `dev` lives outside the inner struct. This is a clean pattern that avoids the need for the inner struct to hold a reference back to the outer struct.

4. **Formatting nit** in the constructor:
```rust
+        try_pin_init!(Self {
+            inner <- new_mutex!(CmdqInner {
+                gsp_mem: DmaGspMem::new(dev)?,
+                seq: 0,
+                }),
+            dev: dev.into(),
+        })
```
The closing `})` for `new_mutex!` has unusual indentation — the `)` is at 16 spaces indent while the opening `new_mutex!` is at 24. This is a very minor style nit.

5. The `boot()` signature change from `mut self: Pin<&mut Self>` to `self: Pin<&mut Self>` reflects that `Cmdq` methods no longer require `&mut self`:
```rust
-        mut self: Pin<&mut Self>,
+        self: Pin<&mut Self>,
```

Overall the locking design is correct and well-documented. Already has Reviewed-by from Zhi Wang.

---
Generated by Claude Code Patch Reviewer

[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 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

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

[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 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      | 165 ++++++++++++++++++++-------------
 drivers/gpu/nova-core/gsp/commands.rs  |   4 +-
 drivers/gpu/nova-core/gsp/sequencer.rs |   2 +-
 4 files changed, 105 insertions(+), 74 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 90179256a929..47406d494523 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,
@@ -477,12 +481,9 @@ struct GspMessage<'a> {
 /// area.
 #[pin_data]
 pub(crate) struct Cmdq {
-    /// Device this command queue belongs to.
-    dev: ARef<device::Device>,
-    /// Current command sequence number.
-    seq: u32,
-    /// Memory area shared with the GSP for communicating commands and messages.
-    gsp_mem: DmaGspMem,
+    /// Inner mutex-protected state.
+    #[pin]
+    inner: Mutex<CmdqInner>,
 }
 
 impl Cmdq {
@@ -502,18 +503,17 @@ impl Cmdq {
     /// Number of page table entries for the GSP shared region.
     pub(crate) const NUM_PTES: usize = size_of::<GspMem>() >> GSP_PAGE_SHIFT;
 
-    /// 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(5);
 
     /// 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,
+            inner <- new_mutex!(CmdqInner {
+                dev: dev.into(),
+                gsp_mem: DmaGspMem::new(dev)?,
+                seq: 0,
+            }),
         })
     }
 
@@ -537,6 +537,87 @@ fn notify_gsp(bar: &Bar0) {
             .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.inner.lock().gsp_mem.0.dma_handle()
+    }
+}
+
+/// Inner mutex protected state of [`Cmdq`].
+struct CmdqInner {
+    /// Device this command queue belongs to.
+    dev: ARef<device::Device>,
+    /// Current command sequence number.
+    seq: u32,
+    /// Memory area shared with the GSP for communicating commands and messages.
+    gsp_mem: DmaGspMem,
+}
+
+impl CmdqInner {
+    /// Timeout for waiting for space on the command queue.
+    const ALLOCATE_TIMEOUT: Delta = Delta::from_secs(1);
+
     /// Sends `command` to the GSP, without splitting it.
     ///
     /// # Errors
@@ -617,7 +698,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 +718,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 +753,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 +788,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 +817,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 +853,4 @@ pub(crate) fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Resul
 
         result
     }
-
-    /// 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()
-    }
 }
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

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

[Claude Code Review Bot]

Patch Review

This is the main event. Key design decisions:

1. **All mutable state moved to `CmdqInner`** wrapped in `Mutex<CmdqInner>`:
```rust
pub(crate) struct Cmdq {
+    #[pin]
+    inner: Mutex<CmdqInner>,
}
```

2. **`Cmdq` methods now take `&self`** instead of `&mut self`, enabling shared access from multiple threads.

3. **The mutex is held across send+receive in `send_command`**:
```rust
+        let mut inner = self.inner.lock();
+        inner.send_command(bar, command)?;
+        loop {
+            match inner.receive_msg::<M::Reply>(Self::RECEIVE_TIMEOUT) {
```

This is correct for ensuring reply ordering -- no other command can slip in between send and receive.

4. **`dma_handle()` now acquires the mutex**:
```rust
+    pub(crate) fn dma_handle(&self) -> DmaAddress {
+        self.inner.lock().gsp_mem.0.dma_handle()
+    }
```

This is called during init from `MessageQueueInitArguments::new()`. Since `dma_handle()` returns an immutable property (the DMA address doesn't change after allocation), acquiring the mutex here is unnecessary overhead but harmless. If this becomes a hot path in the future, it could be worth storing the DMA handle separately outside the mutex, but for now this is fine.

5. **`boot()` signature change** drops `mut` from `Pin<&mut Self>`:
```rust
-        mut self: Pin<&mut Self>,
+        self: Pin<&mut Self>,
```

This is correct since `cmdq` methods no longer need `&mut self`.

6. **Potential concern -- holding mutex during `receive_msg` timeout**: The `receive_msg` call inside `send_command` can block for up to `RECEIVE_TIMEOUT` (5 seconds) while holding the mutex lock. During this time, no other thread can send commands or receive messages. The cover letter acknowledges this: "For now this should be ok, and we expect GSP to be fast anyway." This is acceptable for the current use case but worth noting for future scaling.

7. **The `receive_msg` public wrapper also acquires the lock**:
```rust
+    pub(crate) fn receive_msg<M: MessageFromGsp>(&self, timeout: Delta) -> Result<M>
+    ...
+        self.inner.lock().receive_msg(timeout)
```

This is used by `wait_gsp_init_done` and `GspSequencer::run`, which call `receive_msg` in loops. Each iteration will acquire and release the lock, which is fine -- it allows other threads to interleave between iterations.

The `ALLOCATE_TIMEOUT` constant correctly moved from `Cmdq` to `CmdqInner` since it's only used by `CmdqInner::send_single_command`.

The `dev_dbg!`/`dev_err!` changes from `self.dev` to `&self.dev` are because `dev` is now `ARef<device::Device>` inside `CmdqInner` and the macro needs a reference.

No blocking issues. The series is clean and well-decomposed.

---
Generated by Claude Code Patch Reviewer