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

Eliot Courtney posted 4 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH 2/4] gpu: nova-core: gsp: add sync and async command queue API to `Cmdq`
Posted by Eliot Courtney 1 month, 1 week ago
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`
Posted by Zhi Wang 1 month, 1 week ago
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`
Posted by Eliot Courtney 1 month, 1 week ago
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~