NVIDIA GPU driver infrastructure
 help / color / mirror / Atom feed
* [PATCH v5 00/14] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types
@ 2026-10-20 10:08 Alexandre Courbot
  2026-10-20 10:08 ` [PATCH v5 01/14] gpu: nova-core: gsp: cmdq: remove stale comment Alexandre Courbot
                   ` (13 more replies)
  0 siblings, 14 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-10-20 10:08 UTC (permalink / raw)
  To: John Hubbard, Danilo Krummrich, Alice Ryhl, David Airlie,
	Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, linux-kernel, rust-for-linux, Alexandre Courbot

The upcoming r000 firmware support [1] introduces a new type of command
named GMC alongside the existing and already supported RPC command type.

Since RPC was the only command type supported so far, it was embedded
into the lowest command queue level: the message header actually
includes the RPC header and the code considers them as one.

This makes the r000/GMC support difficult to land without a refactor,
with the result of transport layer code being duplicated between the two
message types, and no clear separation between the RPC and GMC code
which are mixed together in the command queue. Merging the code that way
would introduce quite some technical debt.

Thus, this series prepares a proper landing ground for a new message
type by extracting all the RPC layer code and moving it into its own
sub-modules, making the `cmdq` module completely agnostic of the type of
message it transports.

With the transport and message layers properly separated, support for
GMC can be added into sibling modules of `rpc`, and the transport layer
can be converted once when doing the switch to r000.

This moves quite a bit of code around, but most of it is moved verbatim.
No significant functional change is intended.

For the r000 series, this essentially means the following:

- Its patch 7 making `allocate_command` generic can be dropped as the
  transport layer makes no assumption about the message type.
- Patches adding GMC support will do it in dedicated `gmc` sub-modules,
  following the model set by RPC. These `gmc` sub-modules will start as
  `dead_code` as they are being built.
- The "switch to r000" patch will essentially operate on the transport
  layer level, i.e. `cmdq.rs`.
- There should be no need for extra post-switch RPC fixing, as the
  transport layer code will already be shared with GMC.

Overall, this should make the r000 series much easier to review and
eventually land, while not creating new technical debt.

For this revision, the main rework is processing received messages using
closures rather than a trait. This solution ended up being more flexible
than the trait as it allows us to factor some code out of `drain`.

This series is based on `drm-rust-next`.

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

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
Changes in v5:
- Drop `MessageElement` and instead use a closure to process messages in
  `consume_element`. This reduces the behavioral change of `await_msg`
  and harmonizes it with the behavior of `drain` to the point where the
  latter can also use `consume_element`. Eliot's R-b has
  been dropped on that patch as the code has changed significantly.
- Avoid unneeded temporary result variable. (Eliot)
- Rename `await_msg` to `await_rpc_msg`. (Eliot)
- Add patch splitting looping behavior of `await_msg` from its RPC
  message handling.
- Add patch factoring out `drain` code.
- Add two doccomment fixes.
- Link to v4: https://patch.msgid.link/20261009-cmdq-rpc-v4-0-c9ab8de1d3f2@nvidia.com

Changes in v4:
- Rebase on top of latest drm-rust-next, including the interrupt series.
- Move RPC parsing code to `RpcMessage`. (Eliot)
- Do not mention the command queue slots in the public documentation. (Eliot)
- Use different names for the length methods of the message and RPC
  headers. (Eliot)
- Fix bug where the RPC header length was used instead of the message
  one to compute the number of pages to advance by. (Eliot)
- Add a patch moving the message logging code to `RpcMessage`.
- Make the `rpc` module only visible from `gsp`. (Eliot)
- Link to v3: https://patch.msgid.link/20260930-cmdq-rpc-v3-0-91613f06520b@nvidia.com

Changes in v3:
- Use dedicated `RpcMessage` type to distinguish parsed RPC message from
  the transport-level `GspMessage`.
- Introduce traits for sending and receiving elements on the command
  queue (thanks Eliot!).
- Split unsafe blocks, drop unused impls, and other fixes on patch 2.
  (thanks Eliot!)
- Squash the two move commits into a single one.
- Link to v2: https://patch.msgid.link/20260927-cmdq-rpc-v2-0-c3f66ae73be4@nvidia.com

Changes in v2:
- Fix checksum on receive incorrectly using the whole slots allocated
  to the message instead of its actual length. (Sashiko)
- Drop patch 1 to avoid having to validate the `elemCount` header
  member against edge cases. (Sashiko)
- Fix incorrect layer separation in receive path, where the CPU read
  pointer was advanced in the message layer.
- Move `log_event` into a method of `RpcMessage`.
- Make the `rpc` module only visible to `gsp` and its submodules.
- Fix doclinks pointing to old function names. (Sashiko)
- Link to v1: https://patch.msgid.link/20260927-cmdq-rpc-v1-0-822db5af910e@nvidia.com

---
Alexandre Courbot (14):
      gpu: nova-core: gsp: cmdq: remove stale comment
      gpu: nova-core: gsp: cmdq: simplify framing error description
      gpu: nova-core: gsp: cmdq: validate checksum earlier on receive
      gpu: nova-core: gsp: introduce and use proper RpcMessageHeader type
      gpu: nova-core: gsp: cmdq: group the RPC-specific part of send_single_command
      gpu: nova-core: gsp: cmdq: split the transport part of the send path
      gpu: nova-core: gsp: cmdq: split RPC parsing part of the receive path
      gpu: nova-core: gsp: cmdq: move RPC message logging to RpcMessage
      gpu: nova-core: gsp: cmdq: split the transport part of the receive path
      gpu: nova-core: gsp: cmdq: extract await_msg_with from await_msg
      gpu: nova-core: gsp: cmdq: use consume_element to implement drain
      gpu: nova-core: gsp: cmdq: move the RPC code into a sub-module
      gpu: nova-core: gsp: move the RPC commands into a sub-module
      gpu: nova-core: gsp: add `rpc` to RPC message send/receive methods

 Documentation/gpu/nova/core/interrupts.rst         |   7 +-
 drivers/gpu/nova-core/api.rs                       |   2 +-
 drivers/gpu/nova-core/gpu.rs                       |   2 +-
 drivers/gpu/nova-core/gsp.rs                       |   4 +-
 drivers/gpu/nova-core/gsp/boot.rs                  |  12 +-
 drivers/gpu/nova-core/gsp/cmdq.rs                  | 461 ++++-----------------
 drivers/gpu/nova-core/gsp/cmdq/rpc.rs              | 415 +++++++++++++++++++
 .../nova-core/gsp/cmdq/{ => rpc}/continuation.rs   |   0
 drivers/gpu/nova-core/gsp/commands.rs              | 329 +--------------
 drivers/gpu/nova-core/gsp/commands/rpc.rs          | 333 +++++++++++++++
 drivers/gpu/nova-core/gsp/fw.rs                    | 123 +++---
 drivers/gpu/nova-core/gsp/sequencer.rs             |   6 +-
 12 files changed, 927 insertions(+), 767 deletions(-)
---
base-commit: 3497a403264b094ebd61b8b755165d56e7d9dc37
change-id: 20260927-cmdq-rpc-ed5f850b207e

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


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v5 01/14] gpu: nova-core: gsp: cmdq: remove stale comment
  2026-10-20 10:08 [PATCH v5 00/14] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types Alexandre Courbot
@ 2026-10-20 10:08 ` Alexandre Courbot
  2026-10-20 11:43   ` Eliot Courtney
  2026-10-20 10:08 ` [PATCH v5 02/14] gpu: nova-core: gsp: cmdq: simplify framing error description Alexandre Courbot
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 20+ messages in thread
From: Alexandre Courbot @ 2026-10-20 10:08 UTC (permalink / raw)
  To: John Hubbard, Danilo Krummrich, Alice Ryhl, David Airlie,
	Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, linux-kernel, rust-for-linux, Alexandre Courbot

The message constructor is no longer run by `wait_for_msg`.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 1b2f346a237f..accfc1d20d28 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -777,8 +777,6 @@ fn send_command<M>(&mut self, command: M) -> Result
     /// - `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.
-    ///
-    /// Error codes returned by the message constructor are propagated as-is.
     fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
         // Wait for a message to arrive from the GSP.
         let (slice_1, slice_2) = read_poll_timeout(

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 02/14] gpu: nova-core: gsp: cmdq: simplify framing error description
  2026-10-20 10:08 [PATCH v5 00/14] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types Alexandre Courbot
  2026-10-20 10:08 ` [PATCH v5 01/14] gpu: nova-core: gsp: cmdq: remove stale comment Alexandre Courbot
@ 2026-10-20 10:08 ` Alexandre Courbot
  2026-10-20 11:43   ` Eliot Courtney
  2026-10-20 10:08 ` [PATCH v5 03/14] gpu: nova-core: gsp: cmdq: validate checksum earlier on receive Alexandre Courbot
                   ` (11 subsequent siblings)
  13 siblings, 1 reply; 20+ messages in thread
From: Alexandre Courbot @ 2026-10-20 10:08 UTC (permalink / raw)
  To: John Hubbard, Danilo Krummrich, Alice Ryhl, David Airlie,
	Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, linux-kernel, rust-for-linux, Alexandre Courbot

Use simpler and more precise language to explain the conditions in which
`EIO` is returned.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index accfc1d20d28..1bd15fc0aef2 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -775,8 +775,7 @@ fn send_command<M>(&mut self, command: M) -> Result
     /// # 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.
+    /// - `EIO` if a message fails framing or checksum validation.
     fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
         // Wait for a message to arrive from the GSP.
         let (slice_1, slice_2) = read_poll_timeout(
@@ -849,8 +848,7 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
     /// # 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.
+    /// - `EIO` if a message fails framing or checksum validation.
     /// - `ENOMSG` if the message was not the awaited reply.
     ///
     /// Error codes returned by [`MessageFromGsp::read`] are propagated as-is.
@@ -902,7 +900,7 @@ fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M>
     ///
     /// - `ETIMEDOUT` if no message of type `M` arrives before the deadline, however many other
     ///   messages arrive while waiting.
-    /// - `EIO` if a message fails framing or checksum validation (see [`Self::wait_for_msg`]).
+    /// - `EIO` if a message fails framing or checksum validation.
     ///
     /// Error codes returned by [`MessageFromGsp::read`] are propagated as-is.
     fn await_msg<M: MessageFromGsp>(&mut self) -> Result<M>

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 03/14] gpu: nova-core: gsp: cmdq: validate checksum earlier on receive
  2026-10-20 10:08 [PATCH v5 00/14] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types Alexandre Courbot
  2026-10-20 10:08 ` [PATCH v5 01/14] gpu: nova-core: gsp: cmdq: remove stale comment Alexandre Courbot
  2026-10-20 10:08 ` [PATCH v5 02/14] gpu: nova-core: gsp: cmdq: simplify framing error description Alexandre Courbot
@ 2026-10-20 10:08 ` Alexandre Courbot
  2026-10-20 10:08 ` [PATCH v5 04/14] gpu: nova-core: gsp: introduce and use proper RpcMessageHeader type Alexandre Courbot
                   ` (10 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-10-20 10:08 UTC (permalink / raw)
  To: John Hubbard, Danilo Krummrich, Alice Ryhl, David Airlie,
	Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, linux-kernel, rust-for-linux, Alexandre Courbot

The checksum validation error message of `wait_for_msg` prints the
message's sequence number before validating the checksum.

But the checksum is part of the transport layer, and it not validating
indicates a corruption that could very well be in the RPC message
header, meaning the value of this field cannot be trusted.

Furthermore, the transport layer is not supposed to know the kind of
message it transports, and it accessing the RPC header is a layering
violation.

Thus, move the checksum validation before we start looking at the
message header, and drop that information from the error message.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 1bd15fc0aef2..0c18a586576c 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -789,6 +789,15 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
         // Extract the `GspMsgElement`.
         let (header, slice_1) = GspMsgElement::from_bytes_prefix(slice_1).ok_or(EIO)?;
 
+        // Validate checksum after truncating the message to its exact length.
+        if Cmdq::calculate_checksum(
+            SBufferIter::new_reader([header.as_bytes(), slice_1, slice_2]).take(header.length()),
+        ) != 0
+        {
+            dev_err!(&self.dev, "GSP receive: bad checksum\n");
+            return Err(EIO);
+        }
+
         dev_dbg!(
             &self.dev,
             "GSP RPC: receive: seq# {}, function={:?}, length=0x{:x}\n",
@@ -817,21 +826,6 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
             )
         };
 
-        // Validate checksum.
-        if Cmdq::calculate_checksum(SBufferIter::new_reader([
-            header.as_bytes(),
-            slice_1,
-            slice_2,
-        ])) != 0
-        {
-            dev_err!(
-                &self.dev,
-                "GSP RPC: receive: Call {} - bad checksum\n",
-                header.sequence()
-            );
-            return Err(EIO);
-        }
-
         Ok(GspMessage {
             header,
             contents: (slice_1, slice_2),

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 04/14] gpu: nova-core: gsp: introduce and use proper RpcMessageHeader type
  2026-10-20 10:08 [PATCH v5 00/14] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types Alexandre Courbot
                   ` (2 preceding siblings ...)
  2026-10-20 10:08 ` [PATCH v5 03/14] gpu: nova-core: gsp: cmdq: validate checksum earlier on receive Alexandre Courbot
@ 2026-10-20 10:08 ` Alexandre Courbot
  2026-10-20 10:08 ` [PATCH v5 05/14] gpu: nova-core: gsp: cmdq: group the RPC-specific part of send_single_command Alexandre Courbot
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-10-20 10:08 UTC (permalink / raw)
  To: John Hubbard, Danilo Krummrich, Alice Ryhl, David Airlie,
	Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, linux-kernel, rust-for-linux, Alexandre Courbot

So far, the GSP command queue transport and message layer code were
intertwined, a design issue that goes as deep as the types themselves:
the generated bindings for `GspMsgElement` even include the RPC header
at its end.

This makes it difficult to introduce the new GMC message type; thus this
patch works around these limitations to make the RPC message header more
explicit and allow it to be eventually handled by a different layer.

The `RpcMessageHeader` wrapping type is introduced following the same
model as `GspMsgElement`, and can be obtained from the latter. The
methods of `GspMsgElement` that actually query the RPC header are moved
to `RpcMessageHeader`.

Regarding initialization, `GspMsgElement` leaves the RPC header zeroed,
and the command queue code is now responsible for initializing it in a
separate call.

The only functional change is that the RPC debug messages now display
the size of the RPC payload instead of the whole message including its
headers, as they are technically part of the message layer. This metric
is arguably more useful as the headers have successfully been parsed by
the time we can print these messages.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs |  50 ++++++++++------
 drivers/gpu/nova-core/gsp/fw.rs   | 123 ++++++++++++++++++++++----------------
 2 files changed, 106 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 0c18a586576c..01c83a79da14 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -54,6 +54,7 @@
             MsgFunction,
             MsgqRxHeader,
             MsgqTxHeader,
+            RpcMessageHeader,
             GSP_MSG_QUEUE_ELEMENT_SIZE_MAX, //
         },
         PteArray,
@@ -688,11 +689,22 @@ fn send_single_command<M>(&mut self, command: M) -> Result
         let (cmd, payload_1) = M::Command::from_bytes_mut_prefix(dst.contents.0).ok_or(EIO)?;
 
         // Fill the header and command in-place.
-        let msg_element = GspMsgElement::init(self.seq, size_in_bytes, M::FUNCTION);
-        // SAFETY: `msg_header` and `cmd` are valid references, and not touched if the initializer
-        // fails.
+        let msg_element_init = GspMsgElement::init(self.seq, size_in_bytes);
+        let rpc_header_init = RpcMessageHeader::init(size_in_bytes, M::FUNCTION);
+        // SAFETY: `dst.header` is a valid reference, and is not touched if the initializer fails.
+        unsafe {
+            pin_init::raw_try_init(core::ptr::from_mut(dst.header), msg_element_init)?;
+        }
+        // SAFETY: `dst.header.rpc_header_mut()` is a valid reference, and is not touched if the
+        // initializer fails.
+        unsafe {
+            pin_init::raw_try_init(
+                core::ptr::from_mut(dst.header.rpc_header_mut()),
+                rpc_header_init,
+            )?;
+        }
+        // SAFETY: `cmd` is a valid reference, and is not touched if the initializer fails.
         unsafe {
-            pin_init::raw_try_init(core::ptr::from_mut(dst.header), msg_element)?;
             pin_init::raw_try_init(core::ptr::from_mut(cmd), command.init())?;
         }
 
@@ -718,7 +730,7 @@ fn send_single_command<M>(&mut self, command: M) -> Result
             "GSP RPC: send: seq# {}, function={:?}, length=0x{:x}\n",
             self.seq,
             M::FUNCTION,
-            dst.header.length(),
+            size_in_bytes,
         );
 
         // All set - update the write pointer and inform the GSP of the new command.
@@ -791,23 +803,25 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
 
         // Validate checksum after truncating the message to its exact length.
         if Cmdq::calculate_checksum(
-            SBufferIter::new_reader([header.as_bytes(), slice_1, slice_2]).take(header.length()),
+            SBufferIter::new_reader([header.as_bytes(), slice_1, slice_2])
+                .take(header.msg_length()),
         ) != 0
         {
             dev_err!(&self.dev, "GSP receive: bad checksum\n");
             return Err(EIO);
         }
 
+        let rpc_header = header.rpc_header();
+        let payload_length = rpc_header.rpc_length();
+
         dev_dbg!(
             &self.dev,
             "GSP RPC: receive: seq# {}, function={:?}, length=0x{:x}\n",
-            header.sequence(),
-            header.function(),
-            header.length(),
+            rpc_header.sequence(),
+            rpc_header.function(),
+            payload_length,
         );
 
-        let payload_length = header.payload_length();
-
         // Check that the driver read area is large enough for the message.
         if slice_1.len() + slice_2.len() < payload_length {
             return Err(EIO);
@@ -852,8 +866,9 @@ fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M>
         Error: From<M::InitError>,
     {
         let message = self.wait_for_msg(timeout)?;
-        let function = message.header.function();
-        let seq = message.header.sequence();
+        let rpc_header = message.header.rpc_header();
+        let function = rpc_header.function();
+        let seq = rpc_header.sequence();
 
         // An early return here would leave the read pointer on this message.
         let result = if matches!(function, Ok(f) if f == M::FUNCTION) {
@@ -879,7 +894,7 @@ fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M>
 
         // Advance the read pointer past this message.
         self.gsp_mem.advance_cpu_read_ptr(u32::try_from(
-            message.header.length().div_ceil(GSP_PAGE_SIZE),
+            message.header.msg_length().div_ceil(GSP_PAGE_SIZE),
         )?);
 
         result
@@ -958,12 +973,13 @@ fn drain(&mut self) -> Result {
             let msg = self.wait_for_msg(Delta::ZERO)?;
 
             let pages =
-                u32::try_from(msg.header.length().div_ceil(GSP_PAGE_SIZE)).map_err(|_| {
+                u32::try_from(msg.header.msg_length().div_ceil(GSP_PAGE_SIZE)).map_err(|_| {
                     dev_err!(&self.dev, "GSP drain: message length overflow\n");
                     EIO
                 })?;
-            let function = msg.header.function();
-            let seq = msg.header.sequence();
+            let rpc_header = msg.header.rpc_header();
+            let function = rpc_header.function();
+            let seq = rpc_header.sequence();
 
             self.gsp_mem.advance_cpu_read_ptr(pages);
             self.log_event(function, seq);
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index 918a7ae809eb..54274913a68f 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -781,25 +781,6 @@ fn new() -> Self {
     }
 }
 
-impl bindings::rpc_message_header_v {
-    fn init(cmd_size: usize, function: MsgFunction) -> impl Init<Self, Error> {
-        type RpcMessageHeader = bindings::rpc_message_header_v;
-
-        try_init!(RpcMessageHeader {
-            header_version: MsgHeaderVersion::new().into(),
-            signature: bindings::NV_VGPU_MSG_SIGNATURE_VALID,
-            function: function.into(),
-            length: size_of::<Self>()
-                .checked_add(cmd_size)
-                .ok_or(EOVERFLOW)
-                .and_then(|v| v.try_into().map_err(|_| EINVAL))?,
-            rpc_result: 0xffffffff,
-            rpc_result_private: 0xffffffff,
-            ..Zeroable::init_zeroed()
-        })
-    }
-}
-
 /// GSP Message Element.
 ///
 /// This is essentially a message header expected to be followed by the message data.
@@ -811,17 +792,14 @@ pub(crate) struct GspMsgElement {
 impl GspMsgElement {
     /// Creates a new message element.
     ///
+    /// The RPC header is left initialized to zero and must be initialized separately using e.g.
+    /// [`Self::rpc_header_mut`].
+    ///
     /// # Arguments
     ///
     /// * `sequence` - Sequence number of the message.
     /// * `cmd_size` - Size of the command (not including the message element), in bytes.
-    /// * `function` - Function of the message.
-    pub(crate) fn init(
-        sequence: u32,
-        cmd_size: usize,
-        function: MsgFunction,
-    ) -> impl Init<Self, Error> {
-        type RpcMessageHeader = bindings::rpc_message_header_v;
+    pub(crate) fn init(sequence: u32, cmd_size: usize) -> impl Init<Self, Error> {
         type InnerGspMsgElement = bindings::GSP_MSG_QUEUE_ELEMENT;
         let init_inner = try_init!(InnerGspMsgElement {
             seqNum: sequence,
@@ -831,7 +809,6 @@ pub(crate) fn init(
                 .div_ceil(GSP_PAGE_SIZE)
                 .try_into()
                 .map_err(|_| EOVERFLOW)?,
-            rpc <- RpcMessageHeader::init(cmd_size, function),
             ..Zeroable::init_zeroed()
         });
 
@@ -848,34 +825,27 @@ pub(crate) fn set_checksum(&mut self, checksum: u32) {
         self.inner.checkSum = checksum;
     }
 
-    /// Returns the length of the message's payload.
-    pub(crate) fn payload_length(&self) -> usize {
-        // `rpc.length` includes the length of the RPC message header.
-        num::u32_as_usize(self.inner.rpc.length)
-            .saturating_sub(size_of::<bindings::rpc_message_header_v>())
+    /// Returns a reference to the RPC header within the message element.
+    pub(crate) fn rpc_header(&self) -> &RpcMessageHeader {
+        // SAFETY: `RpcMessageHeader` is a transparent wrapper for the type of `inner.rpc`.
+        unsafe { core::mem::transmute(&self.inner.rpc) }
+    }
+
+    /// Returns a mutable reference to the RPC header within the message element.
+    pub(crate) fn rpc_header_mut(&mut self) -> &mut RpcMessageHeader {
+        // SAFETY: `RpcMessageHeader` is a transparent wrapper for the type of `inner.rpc`.
+        unsafe { core::mem::transmute(&mut self.inner.rpc) }
     }
 
     /// Returns the total length of the message, message and RPC headers included.
-    pub(crate) fn length(&self) -> usize {
-        size_of::<Self>() + self.payload_length()
+    pub(crate) fn msg_length(&self) -> usize {
+        // Note: this is transport-layer code accessing message-layer data, which is only ok here
+        // because RPC is the only kind of message we handle, and its header can (with a stretch) be
+        // considered part of the transport header.
+        size_of::<Self>() + self.rpc_header().rpc_length()
     }
 
-    // Returns the sequence number of the message.
-    pub(crate) fn sequence(&self) -> u32 {
-        self.inner.rpc.sequence
-    }
-
-    // Returns the function of the message, if it is valid, or the invalid function number as an
-    // error.
-    pub(crate) fn function(&self) -> Result<MsgFunction, u32> {
-        self.inner
-            .rpc
-            .function
-            .try_into()
-            .map_err(|_| self.inner.rpc.function)
-    }
-
-    // Returns the number of elements (i.e. memory pages) used by this message.
+    /// Returns the number of elements (i.e. memory pages) used by this message.
     pub(crate) fn element_count(&self) -> u32 {
         self.inner.elemCount
     }
@@ -888,6 +858,59 @@ unsafe impl AsBytes for GspMsgElement {}
 // are valid.
 unsafe impl FromBytes for GspMsgElement {}
 
+/// RPC Message Header.
+///
+/// The header of a RPC message, wrapped inside a [`GspMsgElement`].
+#[repr(transparent)]
+pub(crate) struct RpcMessageHeader {
+    inner: bindings::rpc_message_header_v,
+}
+
+impl RpcMessageHeader {
+    /// Creates a new RPC header.
+    ///
+    /// `cmd_size` is the size in bytes of the payload. `function` is the RPC function of the
+    /// message.
+    pub(crate) fn init(cmd_size: usize, function: MsgFunction) -> impl Init<Self, Error> {
+        let init_inner = try_init!(bindings::rpc_message_header_v {
+            header_version: MsgHeaderVersion::new().into(),
+            signature: bindings::NV_VGPU_MSG_SIGNATURE_VALID,
+            function: function.into(),
+            length: size_of::<Self>()
+                .checked_add(cmd_size)
+                .ok_or(EOVERFLOW)
+                .and_then(|v| v.try_into().map_err(|_| EINVAL))?,
+            rpc_result: 0xffffffff,
+            rpc_result_private: 0xffffffff,
+            ..Zeroable::init_zeroed()
+        });
+
+        try_init!(RpcMessageHeader {
+            inner <- init_inner,
+        })
+    }
+
+    /// Returns the length of the RPC's payload, not including the message and RPC headers.
+    pub(crate) fn rpc_length(&self) -> usize {
+        // `length` includes the length of the RPC message header.
+        num::u32_as_usize(self.inner.length).saturating_sub(size_of::<Self>())
+    }
+
+    /// Returns the sequence number of the message.
+    pub(crate) fn sequence(&self) -> u32 {
+        self.inner.sequence
+    }
+
+    /// Returns the function of the message, if it is valid, or the invalid function number as an
+    /// error.
+    pub(crate) fn function(&self) -> Result<MsgFunction, u32> {
+        self.inner
+            .function
+            .try_into()
+            .map_err(|_| self.inner.function)
+    }
+}
+
 /// Arguments for GSP startup.
 #[repr(transparent)]
 #[derive(Zeroable)]

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 05/14] gpu: nova-core: gsp: cmdq: group the RPC-specific part of send_single_command
  2026-10-20 10:08 [PATCH v5 00/14] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types Alexandre Courbot
                   ` (3 preceding siblings ...)
  2026-10-20 10:08 ` [PATCH v5 04/14] gpu: nova-core: gsp: introduce and use proper RpcMessageHeader type Alexandre Courbot
@ 2026-10-20 10:08 ` Alexandre Courbot
  2026-10-20 10:08 ` [PATCH v5 06/14] gpu: nova-core: gsp: cmdq: split the transport part of the send path Alexandre Courbot
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-10-20 10:08 UTC (permalink / raw)
  To: John Hubbard, Danilo Krummrich, Alice Ryhl, David Airlie,
	Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, linux-kernel, rust-for-linux, Alexandre Courbot

`send_single_command` handles both the transport and message layers of
the command, intertwining the logic of the two.

Reorder the code so the message layer logic is within the same
contiguous block of code, so it can easily be moved.

No functional change intended.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 30 +++++++++++++++---------------
 1 file changed, 15 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 01c83a79da14..88d81a4e37ef 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -683,18 +683,18 @@ fn send_single_command<M>(&mut self, command: M) -> Result
             .gsp_mem
             .allocate_command(size_in_bytes, Self::ALLOCATE_TIMEOUT)?;
 
-        // Extract area for the command itself. The GSP message header and the command header
-        // together are guaranteed to fit entirely into a single page, so it's ok to only look
-        // at `dst.contents.0` here.
-        let (cmd, payload_1) = M::Command::from_bytes_mut_prefix(dst.contents.0).ok_or(EIO)?;
-
-        // Fill the header and command in-place.
+        // Fill the header.
         let msg_element_init = GspMsgElement::init(self.seq, size_in_bytes);
-        let rpc_header_init = RpcMessageHeader::init(size_in_bytes, M::FUNCTION);
         // SAFETY: `dst.header` is a valid reference, and is not touched if the initializer fails.
         unsafe {
             pin_init::raw_try_init(core::ptr::from_mut(dst.header), msg_element_init)?;
         }
+
+        // Extract area for the command itself. The GSP message header and the command header
+        // together are guaranteed to fit entirely into a single page, so it's ok to only look
+        // at `dst.contents.0` here.
+        let (cmd, payload_1) = M::Command::from_bytes_mut_prefix(dst.contents.0).ok_or(EIO)?;
+        let rpc_header_init = RpcMessageHeader::init(size_in_bytes, M::FUNCTION);
         // SAFETY: `dst.header.rpc_header_mut()` is a valid reference, and is not touched if the
         // initializer fails.
         unsafe {
@@ -717,14 +717,6 @@ fn send_single_command<M>(&mut self, command: M) -> Result
         }
         drop(sbuffer);
 
-        // Compute checksum now that the whole message is ready.
-        dst.header
-            .set_checksum(Cmdq::calculate_checksum(SBufferIter::new_reader([
-                dst.header.as_bytes(),
-                dst.contents.0,
-                dst.contents.1,
-            ])));
-
         dev_dbg!(
             &self.dev,
             "GSP RPC: send: seq# {}, function={:?}, length=0x{:x}\n",
@@ -733,6 +725,14 @@ fn send_single_command<M>(&mut self, command: M) -> Result
             size_in_bytes,
         );
 
+        // Compute checksum now that the whole message is ready.
+        dst.header
+            .set_checksum(Cmdq::calculate_checksum(SBufferIter::new_reader([
+                dst.header.as_bytes(),
+                dst.contents.0,
+                dst.contents.1,
+            ])));
+
         // All set - update the write pointer and inform the GSP of the new command.
         let elem_count = dst.header.element_count();
         self.seq += 1;

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 06/14] gpu: nova-core: gsp: cmdq: split the transport part of the send path
  2026-10-20 10:08 [PATCH v5 00/14] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types Alexandre Courbot
                   ` (4 preceding siblings ...)
  2026-10-20 10:08 ` [PATCH v5 05/14] gpu: nova-core: gsp: cmdq: group the RPC-specific part of send_single_command Alexandre Courbot
@ 2026-10-20 10:08 ` Alexandre Courbot
  2026-10-20 10:08 ` [PATCH v5 07/14] gpu: nova-core: gsp: cmdq: split RPC parsing part of the receive path Alexandre Courbot
                   ` (7 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-10-20 10:08 UTC (permalink / raw)
  To: John Hubbard, Danilo Krummrich, Alice Ryhl, David Airlie,
	Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, linux-kernel, rust-for-linux, Alexandre Courbot

Introduce the `CommandElement` trait and `RpcCommandElement` wrapper
type to define how messages (independently of their type) are sent
through the transport layer. `send_single_command` becomes
`send_command_element`, which allocates the queue slots, writes the
element header, and delegates the writing of the message itself to the
implementation of `CommandElement` before computing the checksum,
advancing the write pointer and ringing the doorbell.

The RPC part of `send_single_command` (writing the RPC header and the
command payload) is now part of the `CommandElement` implementation.
This sets things up for moving the RPC code into its own sub-module,
leaving the transport agnostic of the message type.

No functional change intended.

Suggested-by: Eliot Courtney <ecourtney@nvidia.com>
Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 133 +++++++++++++++++++++++---------------
 1 file changed, 80 insertions(+), 53 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 88d81a4e37ef..ef59f541f7e6 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -39,10 +39,7 @@
     },
 };
 
-use continuation::{
-    ContinuationRecord,
-    SplitState, //
-};
+use continuation::SplitState;
 
 use pin_init::pin_init_scope;
 
@@ -71,6 +68,19 @@
 /// reply type are sent using [`Cmdq::send_command_no_wait`].
 pub(crate) struct NoReply;
 
+/// Trait implemented by types that can be sent as a single command queue element.
+///
+/// The command queue allocates `size()` bytes after the `GspMsgElement` header and calls `write()`
+/// to fill them.
+trait CommandElement {
+    /// Size in bytes of the element, not including the `GspMsgElement` header.
+    fn size(&self) -> usize;
+
+    /// Writes the contents of the command into `dst`. `dev` is the queue's device (to be used for
+    /// logging), `seq` is the sequence number of the element.
+    fn write(&self, dev: &device::Device, seq: u32, dst: &mut GspCommand<'_>) -> Result;
+}
+
 /// Trait implemented by types representing a command to send to the GSP.
 ///
 /// The main purpose of this trait is to provide [`Cmdq`] with the information it needs to send
@@ -133,6 +143,62 @@ fn size(&self) -> usize {
     }
 }
 
+/// Wrapper type for sending a RPC command as a command queue element.
+///
+/// [`CommandElement`] cannot be directly implemented for all [`CommandToGsp`] with a blanket
+/// implementation as it would conflict with other future command types.
+struct RpcCommandElement<M>(M);
+
+impl<M> CommandElement for RpcCommandElement<M>
+where
+    M: CommandToGsp,
+    Error: From<M::InitError>,
+{
+    fn size(&self) -> usize {
+        self.0.size()
+    }
+
+    fn write(&self, dev: &device::Device, seq: u32, dst: &mut GspCommand<'_>) -> Result {
+        let command = &self.0;
+        let size_in_bytes = command.size();
+        // Extract area for the command itself. The GSP message header and the command header
+        // together are guaranteed to fit entirely into a single page, so it's ok to only look
+        // at `dst.contents.0` here.
+        let (cmd, payload_1) = M::Command::from_bytes_mut_prefix(dst.contents.0).ok_or(EIO)?;
+        let rpc_header_init = RpcMessageHeader::init(size_in_bytes, M::FUNCTION);
+        // SAFETY: `dst.header.rpc_header_mut()` is a valid reference, and is not touched if the
+        // initializer fails.
+        unsafe {
+            pin_init::raw_try_init(
+                core::ptr::from_mut(dst.header.rpc_header_mut()),
+                rpc_header_init,
+            )?;
+        }
+        // SAFETY: `cmd` is a valid reference, and is not touched if the initializer fails.
+        unsafe {
+            pin_init::raw_try_init(core::ptr::from_mut(cmd), command.init())?;
+        }
+
+        // Fill the variable-length payload, which may be empty.
+        let mut sbuffer = SBufferIter::new_writer([&mut payload_1[..], &mut dst.contents.1[..]]);
+        command.init_variable_payload(&mut sbuffer)?;
+
+        if !sbuffer.is_empty() {
+            return Err(EIO);
+        }
+
+        dev_dbg!(
+            dev,
+            "GSP RPC: send: seq# {}, function={:?}, length=0x{:x}\n",
+            seq,
+            M::FUNCTION,
+            size_in_bytes,
+        );
+
+        Ok(())
+    }
+}
+
 /// Trait representing messages received from the GSP.
 ///
 /// A reply that [`Cmdq::send_command`] waits for, or an event that [`Cmdq::await_msg`] waits for.
@@ -662,24 +728,18 @@ 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.
+    /// Allocate enough space on the command queue to store `command`, initialize it with
+    /// [`CommandElement::write`], and send the command to the GSP.
     ///
     /// # 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.
     ///
-    /// Error codes returned by the command initializers are propagated as-is.
-    fn send_single_command<M>(&mut self, command: M) -> Result
-    where
-        M: CommandToGsp,
-        // This allows all error types, including `Infallible`, to be used for `M::InitError`.
-        Error: From<M::InitError>,
-    {
+    /// Error codes returned by [`CommandElement::write`] are returned as-is.
+    fn send_command_element<E: CommandElement>(&mut self, command: E) -> Result {
         let size_in_bytes = command.size();
-        let dst = self
+        let mut dst = self
             .gsp_mem
             .allocate_command(size_in_bytes, Self::ALLOCATE_TIMEOUT)?;
 
@@ -690,40 +750,8 @@ fn send_single_command<M>(&mut self, command: M) -> Result
             pin_init::raw_try_init(core::ptr::from_mut(dst.header), msg_element_init)?;
         }
 
-        // Extract area for the command itself. The GSP message header and the command header
-        // together are guaranteed to fit entirely into a single page, so it's ok to only look
-        // at `dst.contents.0` here.
-        let (cmd, payload_1) = M::Command::from_bytes_mut_prefix(dst.contents.0).ok_or(EIO)?;
-        let rpc_header_init = RpcMessageHeader::init(size_in_bytes, M::FUNCTION);
-        // SAFETY: `dst.header.rpc_header_mut()` is a valid reference, and is not touched if the
-        // initializer fails.
-        unsafe {
-            pin_init::raw_try_init(
-                core::ptr::from_mut(dst.header.rpc_header_mut()),
-                rpc_header_init,
-            )?;
-        }
-        // SAFETY: `cmd` is a valid reference, and is not touched if the initializer fails.
-        unsafe {
-            pin_init::raw_try_init(core::ptr::from_mut(cmd), command.init())?;
-        }
-
-        // Fill the variable-length payload, which may be empty.
-        let mut sbuffer = SBufferIter::new_writer([&mut payload_1[..], &mut dst.contents.1[..]]);
-        command.init_variable_payload(&mut sbuffer)?;
-
-        if !sbuffer.is_empty() {
-            return Err(EIO);
-        }
-        drop(sbuffer);
-
-        dev_dbg!(
-            &self.dev,
-            "GSP RPC: send: seq# {}, function={:?}, length=0x{:x}\n",
-            self.seq,
-            M::FUNCTION,
-            size_in_bytes,
-        );
+        // Write the message payload.
+        command.write(self.dev, self.seq, &mut dst)?;
 
         // Compute checksum now that the whole message is ready.
         dst.header
@@ -759,13 +787,12 @@ fn send_command<M>(&mut self, command: M) -> Result
         Error: From<M::InitError>,
     {
         match SplitState::new(command)? {
-            SplitState::Single(command) => self.send_single_command(command),
+            SplitState::Single(command) => self.send_command_element(RpcCommandElement(command)),
             SplitState::Split(command, mut continuations) => {
-                self.send_single_command(command)?;
+                self.send_command_element(RpcCommandElement(command))?;
 
                 while let Some(continuation) = continuations.next() {
-                    // Turbofish needed because the compiler cannot infer M here.
-                    self.send_single_command::<ContinuationRecord<'_>>(continuation)?;
+                    self.send_command_element(RpcCommandElement(continuation))?;
                 }
 
                 Ok(())

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 07/14] gpu: nova-core: gsp: cmdq: split RPC parsing part of the receive path
  2026-10-20 10:08 [PATCH v5 00/14] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types Alexandre Courbot
                   ` (5 preceding siblings ...)
  2026-10-20 10:08 ` [PATCH v5 06/14] gpu: nova-core: gsp: cmdq: split the transport part of the send path Alexandre Courbot
@ 2026-10-20 10:08 ` Alexandre Courbot
  2026-10-20 10:08 ` [PATCH v5 08/14] gpu: nova-core: gsp: cmdq: move RPC message logging to RpcMessage Alexandre Courbot
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-10-20 10:08 UTC (permalink / raw)
  To: John Hubbard, Danilo Krummrich, Alice Ryhl, David Airlie,
	Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, linux-kernel, rust-for-linux, Alexandre Courbot

The RPC parsing should not be done in the same method as the transport
header parsing. Split it out and introduce a `RpcMessage` type to
indicate messages that have been successfully parsed as RPC. This makes
the code easier to move around into its new destination.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 112 ++++++++++++++++++++++++--------------
 1 file changed, 70 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index ef59f541f7e6..18861400c3be 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -554,11 +554,69 @@ struct GspCommand<'a> {
 struct GspMessage<'a> {
     // Reference to the header of the message.
     header: &'a GspMsgElement,
-    // Slices to the contents of the message. The second slice is zero unless the message loops
+    // Slices to the contents of the message. The second slice is empty unless the message loops
     // over the message queue.
     contents: (&'a [u8], &'a [u8]),
 }
 
+/// A RPC message ready to be processed from the message queue.
+struct RpcMessage<'a> {
+    // Reference to the RPC header of the message.
+    header: &'a RpcMessageHeader,
+    // Slices to the contents of the message. The second slice is empty unless the message loops
+    // over the message queue.
+    contents: (&'a [u8], &'a [u8]),
+}
+
+impl<'a> RpcMessage<'a> {
+    /// Validate the RPC layer of `element` and returns its RPC header and its contents trimmed down
+    /// to the RPC payload.
+    ///
+    /// # Errors
+    ///
+    /// - `EIO` if the element is shorter than the payload length advertised by the RPC header.
+    fn parse(dev: &device::Device, element: GspMessage<'a>) -> Result<RpcMessage<'a>> {
+        let GspMessage {
+            header,
+            contents: (slice_1, slice_2),
+        } = element;
+
+        let rpc_header = header.rpc_header();
+        let payload_length = rpc_header.rpc_length();
+
+        dev_dbg!(
+            dev,
+            "GSP RPC: receive: seq# {}, function={:?}, length=0x{:x}\n",
+            rpc_header.sequence(),
+            rpc_header.function(),
+            payload_length,
+        );
+
+        // Check that the driver read area is large enough for the message.
+        if slice_1.len() + slice_2.len() < payload_length {
+            return Err(EIO);
+        }
+
+        // Cut the message slices down to the actual length of the message.
+        let (slice_1, slice_2) = if slice_1.len() > payload_length {
+            // PANIC: we checked above that `slice_1` is at least as long as `payload_length`.
+            (slice_1.split_at(payload_length).0, &slice_2[0..0])
+        } else {
+            (
+                slice_1,
+                // PANIC: we checked above that `slice_1.len() + slice_2.len()` is at least as
+                // large as `payload_length`.
+                slice_2.split_at(payload_length - slice_1.len()).0,
+            )
+        };
+
+        Ok(RpcMessage {
+            header: rpc_header,
+            contents: (slice_1, slice_2),
+        })
+    }
+}
+
 /// GSP command queue.
 ///
 /// Provides the ability to send commands and receive messages from the GSP using a shared memory
@@ -838,35 +896,6 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
             return Err(EIO);
         }
 
-        let rpc_header = header.rpc_header();
-        let payload_length = rpc_header.rpc_length();
-
-        dev_dbg!(
-            &self.dev,
-            "GSP RPC: receive: seq# {}, function={:?}, length=0x{:x}\n",
-            rpc_header.sequence(),
-            rpc_header.function(),
-            payload_length,
-        );
-
-        // Check that the driver read area is large enough for the message.
-        if slice_1.len() + slice_2.len() < payload_length {
-            return Err(EIO);
-        }
-
-        // Cut the message slices down to the actual length of the message.
-        let (slice_1, slice_2) = if slice_1.len() > payload_length {
-            // PANIC: we checked above that `slice_1` is at least as long as `payload_length`.
-            (slice_1.split_at(payload_length).0, &slice_2[0..0])
-        } else {
-            (
-                slice_1,
-                // PANIC: we checked above that `slice_1.len() + slice_2.len()` is at least as
-                // large as `payload_length`.
-                slice_2.split_at(payload_length - slice_1.len()).0,
-            )
-        };
-
         Ok(GspMessage {
             header,
             contents: (slice_1, slice_2),
@@ -893,14 +922,15 @@ fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M>
         Error: From<M::InitError>,
     {
         let message = self.wait_for_msg(timeout)?;
-        let rpc_header = message.header.rpc_header();
-        let function = rpc_header.function();
-        let seq = rpc_header.sequence();
+        let elem_count = u32::try_from(message.header.msg_length().div_ceil(GSP_PAGE_SIZE))?;
+        let rpc_message = RpcMessage::parse(self.dev, message)?;
+        let function = rpc_message.header.function();
 
         // An early return here would leave the read pointer on this message.
         let result = if matches!(function, Ok(f) if f == M::FUNCTION) {
-            let (cmd, contents_1) = M::Message::from_bytes_prefix(message.contents.0).ok_or(EIO)?;
-            let mut sbuffer = SBufferIter::new_reader([contents_1, message.contents.1]);
+            let (cmd, contents_1) =
+                M::Message::from_bytes_prefix(rpc_message.contents.0).ok_or(EIO)?;
+            let mut sbuffer = SBufferIter::new_reader([contents_1, rpc_message.contents.1]);
 
             M::read(cmd, &mut sbuffer)
                 .map_err(|e| e.into())
@@ -914,15 +944,13 @@ fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M>
                     }
                 })
         } else {
-            self.log_event(function, seq);
+            self.log_event(function, rpc_message.header.sequence());
 
             Err(ENOMSG)
         };
 
         // Advance the read pointer past this message.
-        self.gsp_mem.advance_cpu_read_ptr(u32::try_from(
-            message.header.msg_length().div_ceil(GSP_PAGE_SIZE),
-        )?);
+        self.gsp_mem.advance_cpu_read_ptr(elem_count);
 
         result
     }
@@ -1004,9 +1032,9 @@ fn drain(&mut self) -> Result {
                     dev_err!(&self.dev, "GSP drain: message length overflow\n");
                     EIO
                 })?;
-            let rpc_header = msg.header.rpc_header();
-            let function = rpc_header.function();
-            let seq = rpc_header.sequence();
+            let rpc_msg = RpcMessage::parse(self.dev, msg)?;
+            let function = rpc_msg.header.function();
+            let seq = rpc_msg.header.sequence();
 
             self.gsp_mem.advance_cpu_read_ptr(pages);
             self.log_event(function, seq);

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 08/14] gpu: nova-core: gsp: cmdq: move RPC message logging to RpcMessage
  2026-10-20 10:08 [PATCH v5 00/14] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types Alexandre Courbot
                   ` (6 preceding siblings ...)
  2026-10-20 10:08 ` [PATCH v5 07/14] gpu: nova-core: gsp: cmdq: split RPC parsing part of the receive path Alexandre Courbot
@ 2026-10-20 10:08 ` Alexandre Courbot
  2026-10-20 10:08 ` [PATCH v5 09/14] gpu: nova-core: gsp: cmdq: split the transport part of the receive path Alexandre Courbot
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-10-20 10:08 UTC (permalink / raw)
  To: John Hubbard, Danilo Krummrich, Alice Ryhl, David Airlie,
	Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, linux-kernel, rust-for-linux, Alexandre Courbot

The `RpcMessage` type is the right container for the `log_event` method,
since it contains all the data we passed to it. Move it there so callers
don't need to extract the required data themselves.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 66 ++++++++++++++++++---------------------
 1 file changed, 31 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 18861400c3be..4efc96eba4dd 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -615,6 +615,34 @@ fn parse(dev: &device::Device, element: GspMessage<'a>) -> Result<RpcMessage<'a>
             contents: (slice_1, slice_2),
         })
     }
+
+    /// Logs an event, meaning a message that no caller was waiting for.
+    ///
+    /// An OS error or robust-channel record is logged at error level and an unknown function code
+    /// at warning level. Other events are not logged.
+    fn log(&self, dev: &device::Device) {
+        let function = self.header.function();
+        let seq = self.header.sequence();
+
+        match function {
+            Ok(MsgFunction::OsErrorLog) => {
+                dev_err!(dev, "GSP reported an OS error (seq {})\n", seq);
+            }
+            Ok(MsgFunction::RcTriggered) => {
+                dev_err!(dev, "GSP triggered robust-channel recovery (seq {})\n", seq);
+            }
+            // Nothing to do for the remaining known function codes.
+            Ok(_) => {}
+            Err(raw) => {
+                dev_warn!(
+                    dev,
+                    "unknown GSP message function {:#x} (seq {})\n",
+                    raw,
+                    seq
+                );
+            }
+        }
+    }
 }
 
 /// GSP command queue.
@@ -944,7 +972,7 @@ fn receive_msg<M: MessageFromGsp>(&mut self, timeout: Delta) -> Result<M>
                     }
                 })
         } else {
-            self.log_event(function, rpc_message.header.sequence());
+            rpc_message.log(self.dev);
 
             Err(ENOMSG)
         };
@@ -986,36 +1014,6 @@ fn await_msg<M: MessageFromGsp>(&mut self) -> Result<M>
         }
     }
 
-    /// Logs an event, meaning a message that no caller was waiting for.
-    ///
-    /// An OS error or robust-channel record is logged at error level and an unknown function code
-    /// at warning level. Every other event is recorded only by the receive trace in
-    /// [`Self::wait_for_msg`].
-    fn log_event(&self, function: Result<MsgFunction, u32>, seq: u32) {
-        match function {
-            Ok(MsgFunction::OsErrorLog) => {
-                dev_err!(&self.dev, "GSP reported an OS error (seq {})\n", seq);
-            }
-            Ok(MsgFunction::RcTriggered) => {
-                dev_err!(
-                    &self.dev,
-                    "GSP triggered robust-channel recovery (seq {})\n",
-                    seq
-                );
-            }
-            // Nothing to do for the remaining known function codes.
-            Ok(_) => {}
-            Err(raw) => {
-                dev_warn!(
-                    &self.dev,
-                    "unknown GSP message function {:#x} (seq {})\n",
-                    raw,
-                    seq
-                );
-            }
-        }
-    }
-
     /// Logs and consumes every message the queue holds.
     ///
     /// # Errors
@@ -1032,12 +1030,10 @@ fn drain(&mut self) -> Result {
                     dev_err!(&self.dev, "GSP drain: message length overflow\n");
                     EIO
                 })?;
-            let rpc_msg = RpcMessage::parse(self.dev, msg)?;
-            let function = rpc_msg.header.function();
-            let seq = rpc_msg.header.sequence();
+
+            RpcMessage::parse(self.dev, msg)?.log(self.dev);
 
             self.gsp_mem.advance_cpu_read_ptr(pages);
-            self.log_event(function, seq);
         }
 
         Ok(())

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 09/14] gpu: nova-core: gsp: cmdq: split the transport part of the receive path
  2026-10-20 10:08 [PATCH v5 00/14] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types Alexandre Courbot
                   ` (7 preceding siblings ...)
  2026-10-20 10:08 ` [PATCH v5 08/14] gpu: nova-core: gsp: cmdq: move RPC message logging to RpcMessage Alexandre Courbot
@ 2026-10-20 10:08 ` Alexandre Courbot
  2026-10-20 10:08 ` [PATCH v5 10/14] gpu: nova-core: gsp: cmdq: extract await_msg_with from await_msg Alexandre Courbot
                   ` (4 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-10-20 10:08 UTC (permalink / raw)
  To: John Hubbard, Danilo Krummrich, Alice Ryhl, David Airlie,
	Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, linux-kernel, rust-for-linux, Alexandre Courbot

`receive_msg` currently handles both the transport and RPC layers.
Retire it and split its logic between the new `consume_element`, a
transport-level method which passes the parsed message contents to a
closure before advancing the CPU read pointer past it, and the
already-existing `await_msg`, which calls `consume_element` with a
closure matching the RPC message.

This introduces a small behavioral change: a matched RPC message that is
too short to contain the message component itself no longer stalls the
queue. This behavior is justified because it fixes an inconsistency:
unmatched RPC messages were always skipped, whereas matched RPC messages
could block the queue if they were too short. While a misconstructed RPC
message is a problem, it is not a fatal queue error as long as the
transport framing and checksum are valid.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 Documentation/gpu/nova/core/interrupts.rst |  7 +--
 drivers/gpu/nova-core/gsp/cmdq.rs          | 93 +++++++++++++++++-------------
 2 files changed, 56 insertions(+), 44 deletions(-)

diff --git a/Documentation/gpu/nova/core/interrupts.rst b/Documentation/gpu/nova/core/interrupts.rst
index e42286915711..13a087b960c0 100644
--- a/Documentation/gpu/nova/core/interrupts.rst
+++ b/Documentation/gpu/nova/core/interrupts.rst
@@ -583,13 +583,12 @@ the command's sequence number on every reply. On r570 the reply to
 ``UnloadingGuestDriver`` carries sequence 0.
 
 The read pointer advances past every message that passes framing and checksum
-validation, whether it matched or was an event. A matched message that is too
-short to decode is the exception, and it stays at the queue head.
+validation, whether it matched or was an event.
 
 A message's length is inside the region that the checksum covers, so once the
 framing or the checksum fails there is no trustworthy length with which to skip
-the message. Such a message also stays at the queue head, and every later
-receive fails on it with ``EIO`` until the device is reset.
+the message. Such a message stays at the queue head, and every later receive
+fails on it with ``EIO`` until the device is reset.
 
 The polling path and the IRQ thread both read the queue under the command-queue
 mutex. Replies and events share one queue and one read pointer, so one lock is
diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 4efc96eba4dd..58761a5f609f 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -645,6 +645,12 @@ fn log(&self, dev: &device::Device) {
     }
 }
 
+/// A message whose protocol layer has been parsed and validated.
+enum GspParsedMessage<'a> {
+    /// A [RPC message](RpcMessage).
+    Rpc(RpcMessage<'a>),
+}
+
 /// GSP command queue.
 ///
 /// Provides the ability to send commands and receive messages from the GSP using a shared memory
@@ -930,57 +936,39 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
         })
     }
 
-    /// Receive a message from the GSP.
+    /// Wait for the next element on the message queue, pass it to `f`, advance the read pointer
+    /// past the message, and returns the result of `f`.
     ///
-    /// A message whose function code is `M::FUNCTION` is decoded and returned. Any other message
-    /// is logged as an event.
-    ///
-    /// The read pointer is always advanced past the message, regardless of whether it matched.
+    /// The read pointer advances regardless of whether `f` succeeds or not.
     ///
     /// # Errors
     ///
     /// - `ETIMEDOUT` if `timeout` has elapsed before any message becomes available.
-    /// - `EIO` if a message fails framing or checksum validation.
-    /// - `ENOMSG` if the message was not the awaited reply.
+    /// - `EIO` if a message fails framing or checksum validation. In this case, the read pointer
+    ///   does not advance and the queue is stalled. This is a fatal error.
     ///
-    /// Error codes returned by [`MessageFromGsp::read`] are propagated as-is.
-    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>,
-    {
+    /// If an error is returned, then `f` has not been called.
+    fn consume_element<R>(
+        &mut self,
+        timeout: Delta,
+        f: impl FnOnce(GspParsedMessage<'_>) -> R,
+    ) -> Result<R> {
         let message = self.wait_for_msg(timeout)?;
-        let elem_count = u32::try_from(message.header.msg_length().div_ceil(GSP_PAGE_SIZE))?;
-        let rpc_message = RpcMessage::parse(self.dev, message)?;
-        let function = rpc_message.header.function();
+        let elem_count = u32::try_from(message.header.msg_length().div_ceil(GSP_PAGE_SIZE))
+            .map_err(|_| {
+                dev_err!(&self.dev, "GSP receive: message length overflow\n");
+                EIO
+            })?;
 
-        // An early return here would leave the read pointer on this message.
-        let result = if matches!(function, Ok(f) if f == M::FUNCTION) {
-            let (cmd, contents_1) =
-                M::Message::from_bytes_prefix(rpc_message.contents.0).ok_or(EIO)?;
-            let mut sbuffer = SBufferIter::new_reader([contents_1, rpc_message.contents.1]);
+        // RPC is the only type of message supported for now.
+        let message = GspParsedMessage::Rpc(RpcMessage::parse(self.dev, message)?);
 
-            M::read(cmd, &mut sbuffer)
-                .map_err(|e| e.into())
-                .inspect(|_| {
-                    if !sbuffer.is_empty() {
-                        dev_warn!(
-                            &self.dev,
-                            "GSP message {:?} has unprocessed data\n",
-                            M::FUNCTION
-                        );
-                    }
-                })
-        } else {
-            rpc_message.log(self.dev);
-
-            Err(ENOMSG)
-        };
+        let res = f(message);
 
         // Advance the read pointer past this message.
         self.gsp_mem.advance_cpu_read_ptr(elem_count);
 
-        result
+        Ok(res)
     }
 
     /// Receives a message of type `M`, waiting up to [`Cmdq::RECEIVE_TIMEOUT`] from the call.
@@ -1000,14 +988,39 @@ fn await_msg<M: MessageFromGsp>(&mut self) -> Result<M>
         // This allows all error types, including `Infallible`, to be used for `M::InitError`.
         Error: From<M::InitError>,
     {
+        let f = |message: GspParsedMessage<'_>| {
+            let GspParsedMessage::Rpc(rpc_message) = message;
+            let function = rpc_message.header.function();
+            if matches!(function, Ok(f) if f == M::FUNCTION) {
+                let (cmd, contents_1) =
+                    M::Message::from_bytes_prefix(rpc_message.contents.0).ok_or(EIO)?;
+                let mut sbuffer = SBufferIter::new_reader([contents_1, rpc_message.contents.1]);
+
+                M::read(cmd, &mut sbuffer)
+                    .map_err(|e| e.into())
+                    .inspect(|_| {
+                        if !sbuffer.is_empty() {
+                            dev_warn!(
+                                self.dev,
+                                "GSP message {:?} has unprocessed data\n",
+                                M::FUNCTION
+                            );
+                        }
+                    })
+            } else {
+                rpc_message.log(self.dev);
+
+                Err(ENOMSG)
+            }
+        };
         let deadline = Instant::<Monotonic>::now() + Cmdq::RECEIVE_TIMEOUT;
         loop {
             let remaining = deadline - Instant::<Monotonic>::now();
             if remaining.is_negative() {
                 break Err(ETIMEDOUT);
             }
-            match self.receive_msg::<M>(remaining) {
-                Ok(msg) => break Ok(msg),
+            match self.consume_element(remaining, f)? {
+                Ok(m) => break Ok(m),
                 Err(ENOMSG) => continue,
                 Err(e) => break Err(e),
             }

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 10/14] gpu: nova-core: gsp: cmdq: extract await_msg_with from await_msg
  2026-10-20 10:08 [PATCH v5 00/14] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types Alexandre Courbot
                   ` (8 preceding siblings ...)
  2026-10-20 10:08 ` [PATCH v5 09/14] gpu: nova-core: gsp: cmdq: split the transport part of the receive path Alexandre Courbot
@ 2026-10-20 10:08 ` Alexandre Courbot
  2026-10-20 10:08 ` [PATCH v5 11/14] gpu: nova-core: gsp: cmdq: use consume_element to implement drain Alexandre Courbot
                   ` (3 subsequent siblings)
  13 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-10-20 10:08 UTC (permalink / raw)
  To: John Hubbard, Danilo Krummrich, Alice Ryhl, David Airlie,
	Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, linux-kernel, rust-for-linux, Alexandre Courbot

The logic of `await_msg` is also usable for other kinds of messages that
are not RPC; it will be used to wait for GMC messages as well. Extract
that logic into a new `await_msg_with` method that uses a closure to
decide whether a given message is the one we wait for, and use it in
`await_msg`.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 44 ++++++++++++++++++++++++++++-----------
 1 file changed, 32 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 58761a5f609f..7db3d28e8902 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -971,6 +971,36 @@ fn consume_element<R>(
         Ok(res)
     }
 
+    /// Consumes messages from the queue until a message that satisfies `f` is encountered, and
+    /// returns the result of `f`.
+    ///
+    /// The message is waited for up to [`Cmdq::RECEIVE_TIMEOUT`] from the call.
+    ///
+    /// `f` is expected to return `Ok` with the result to return to the caller, and `Err(ENOMSG)` to
+    /// signal that the received message was not the one expected (such messages are dropped). Any
+    /// other error returned by `f` is propagated to the caller.
+    ///
+    /// # Errors
+    ///
+    /// - `ETIMEDOUT` if no message satisfying `f` arrives before the deadline, however many other
+    ///   messages arrive while waiting.
+    /// - `EIO` if a message fails framing or checksum validation.
+    /// - Any error returned by `f` with the exception of `ENOMSG`.
+    fn await_msg_with<R>(&mut self, f: impl Fn(GspParsedMessage<'_>) -> Result<R>) -> Result<R> {
+        let deadline = Instant::<Monotonic>::now() + Cmdq::RECEIVE_TIMEOUT;
+        loop {
+            let remaining = deadline - Instant::<Monotonic>::now();
+            if remaining.is_negative() {
+                break Err(ETIMEDOUT);
+            }
+            match self.consume_element(remaining, &f)? {
+                Ok(m) => break Ok(m),
+                Err(ENOMSG) => continue,
+                Err(e) => break Err(e),
+            }
+        }
+    }
+
     /// Receives a message of type `M`, waiting up to [`Cmdq::RECEIVE_TIMEOUT`] from the call.
     ///
     /// Any other message that arrives first is logged as an event and does not extend the
@@ -1013,18 +1043,8 @@ fn await_msg<M: MessageFromGsp>(&mut self) -> Result<M>
                 Err(ENOMSG)
             }
         };
-        let deadline = Instant::<Monotonic>::now() + Cmdq::RECEIVE_TIMEOUT;
-        loop {
-            let remaining = deadline - Instant::<Monotonic>::now();
-            if remaining.is_negative() {
-                break Err(ETIMEDOUT);
-            }
-            match self.consume_element(remaining, f)? {
-                Ok(m) => break Ok(m),
-                Err(ENOMSG) => continue,
-                Err(e) => break Err(e),
-            }
-        }
+
+        self.await_msg_with(f)
     }
 
     /// Logs and consumes every message the queue holds.

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 11/14] gpu: nova-core: gsp: cmdq: use consume_element to implement drain
  2026-10-20 10:08 [PATCH v5 00/14] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types Alexandre Courbot
                   ` (9 preceding siblings ...)
  2026-10-20 10:08 ` [PATCH v5 10/14] gpu: nova-core: gsp: cmdq: extract await_msg_with from await_msg Alexandre Courbot
@ 2026-10-20 10:08 ` Alexandre Courbot
  2026-10-20 10:18   ` sashiko-bot
  2026-10-20 10:08 ` [PATCH v5 12/14] gpu: nova-core: gsp: cmdq: move the RPC code into a sub-module Alexandre Courbot
                   ` (2 subsequent siblings)
  13 siblings, 1 reply; 20+ messages in thread
From: Alexandre Courbot @ 2026-10-20 10:08 UTC (permalink / raw)
  To: John Hubbard, Danilo Krummrich, Alice Ryhl, David Airlie,
	Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, linux-kernel, rust-for-linux, Alexandre Courbot

`consume_element` can be used to factor out the code of `drain` that
advanced the read pointer.

No functional change intended.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 7db3d28e8902..54e87fd7158c 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -651,6 +651,15 @@ enum GspParsedMessage<'a> {
     Rpc(RpcMessage<'a>),
 }
 
+impl GspParsedMessage<'_> {
+    /// Log the message on `dev`.
+    fn log(&self, dev: &device::Device) {
+        match self {
+            GspParsedMessage::Rpc(rpc) => rpc.log(dev),
+        }
+    }
+}
+
 /// GSP command queue.
 ///
 /// Provides the ability to send commands and receive messages from the GSP using a shared memory
@@ -1054,21 +1063,14 @@ fn await_msg<M: MessageFromGsp>(&mut self) -> Result<M>
     /// `EIO` if a message fails framing or checksum validation, or if a message's page count
     /// overflows a `u32`.
     fn drain(&mut self) -> Result {
-        while !self.gsp_mem.driver_read_area().0.is_empty() {
-            // A message is available, so this returns without waiting.
-            let msg = self.wait_for_msg(Delta::ZERO)?;
-
-            let pages =
-                u32::try_from(msg.header.msg_length().div_ceil(GSP_PAGE_SIZE)).map_err(|_| {
-                    dev_err!(&self.dev, "GSP drain: message length overflow\n");
-                    EIO
-                })?;
-
-            RpcMessage::parse(self.dev, msg)?.log(self.dev);
-
-            self.gsp_mem.advance_cpu_read_ptr(pages);
+        loop {
+            match self.consume_element(Delta::ZERO, |message| message.log(self.dev)) {
+                Ok(()) => (),
+                // No more message pending on the queue, drain is complete.
+                Err(ETIMEDOUT) => return Ok(()),
+                // Other errors are fatal, and stall the queue.
+                Err(e) => return Err(e),
+            }
         }
-
-        Ok(())
     }
 }

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 12/14] gpu: nova-core: gsp: cmdq: move the RPC code into a sub-module
  2026-10-20 10:08 [PATCH v5 00/14] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types Alexandre Courbot
                   ` (10 preceding siblings ...)
  2026-10-20 10:08 ` [PATCH v5 11/14] gpu: nova-core: gsp: cmdq: use consume_element to implement drain Alexandre Courbot
@ 2026-10-20 10:08 ` Alexandre Courbot
  2026-10-20 10:15   ` sashiko-bot
  2026-10-20 10:08 ` [PATCH v5 13/14] gpu: nova-core: gsp: move the RPC commands " Alexandre Courbot
  2026-10-20 10:08 ` [PATCH v5 14/14] gpu: nova-core: gsp: add `rpc` to RPC message send/receive methods Alexandre Courbot
  13 siblings, 1 reply; 20+ messages in thread
From: Alexandre Courbot @ 2026-10-20 10:08 UTC (permalink / raw)
  To: John Hubbard, Danilo Krummrich, Alice Ryhl, David Airlie,
	Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, linux-kernel, rust-for-linux, Alexandre Courbot

Move the types and code related to sending and receiving RPC commands
and messages into the `rpc` sub-module. The methods are still declared
as members of `CmdqInner` and `Cmdq`; only they are now in their own
sub-module.

This is a pure move commit, with no functional change intended.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/cmdq.rs                  | 387 +------------------
 drivers/gpu/nova-core/gsp/cmdq/rpc.rs              | 415 +++++++++++++++++++++
 .../nova-core/gsp/cmdq/{ => rpc}/continuation.rs   |   0
 drivers/gpu/nova-core/gsp/commands.rs              |   6 +-
 drivers/gpu/nova-core/gsp/sequencer.rs             |   4 +-
 5 files changed, 427 insertions(+), 385 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index 54e87fd7158c..c5a2f08cda94 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -1,6 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0
+// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 
-mod continuation;
+pub(super) mod rpc;
 
 use core::mem;
 
@@ -28,8 +29,8 @@
         },
         Mutex, //
     },
+    time::Delta,
     time::{
-        Delta,
         Instant,
         Monotonic, //
     },
@@ -39,8 +40,6 @@
     },
 };
 
-use continuation::SplitState;
-
 use pin_init::pin_init_scope;
 
 use crate::{
@@ -48,10 +47,8 @@
     gsp::{
         fw::{
             GspMsgElement,
-            MsgFunction,
             MsgqRxHeader,
             MsgqTxHeader,
-            RpcMessageHeader,
             GSP_MSG_QUEUE_ELEMENT_SIZE_MAX, //
         },
         PteArray,
@@ -81,149 +78,6 @@ trait CommandElement {
     fn write(&self, dev: &device::Device, seq: u32, dst: &mut GspCommand<'_>) -> Result;
 }
 
-/// Trait implemented by types representing a command to send to the GSP.
-///
-/// The main purpose of this trait is to provide [`Cmdq`] with the information it needs to send
-/// a given command.
-///
-/// [`CommandToGsp::init`] in particular is responsible for initializing the command directly
-/// into the space reserved for it in the command queue buffer.
-///
-/// Some commands may be followed by a variable-length payload. For these, the
-/// [`CommandToGsp::variable_payload_len`] and [`CommandToGsp::init_variable_payload`] need to be
-/// defined as well.
-pub(crate) trait CommandToGsp {
-    /// Function identifying this command to the GSP.
-    const FUNCTION: MsgFunction;
-
-    /// Type generated by [`CommandToGsp::init`], to be written into the command queue buffer.
-    type Command: FromBytes + AsBytes;
-
-    /// Type of the reply expected from the GSP, or [`NoReply`] for commands that don't
-    /// have a reply.
-    type Reply;
-
-    /// Error type returned by [`CommandToGsp::init`].
-    type InitError;
-
-    /// In-place command initializer responsible for filling the command in the command queue
-    /// buffer.
-    fn init(&self) -> impl Init<Self::Command, Self::InitError>;
-
-    /// Size of the variable-length payload following the command structure generated by
-    /// [`CommandToGsp::init`].
-    ///
-    /// Most commands don't have a variable-length payload, so this is zero by default.
-    fn variable_payload_len(&self) -> usize {
-        0
-    }
-
-    /// Method initializing the variable-length payload.
-    ///
-    /// The command buffer is circular, which means that we may need to jump back to its beginning
-    /// while in the middle of a command. For this reason, the variable-length payload is
-    /// initialized using a [`SBufferIter`].
-    ///
-    /// This method will receive a buffer of the length returned by
-    /// [`CommandToGsp::variable_payload_len`], and must write every single byte of it. Leaving
-    /// unwritten space will lead to an error.
-    ///
-    /// Most commands don't have a variable-length payload, so this does nothing by default.
-    fn init_variable_payload(
-        &self,
-        _dst: &mut SBufferIter<core::array::IntoIter<&mut [u8], 2>>,
-    ) -> Result {
-        Ok(())
-    }
-
-    /// Total size of the command (including its variable-length payload) without the
-    /// [`GspMsgElement`] header.
-    fn size(&self) -> usize {
-        size_of::<Self::Command>() + self.variable_payload_len()
-    }
-}
-
-/// Wrapper type for sending a RPC command as a command queue element.
-///
-/// [`CommandElement`] cannot be directly implemented for all [`CommandToGsp`] with a blanket
-/// implementation as it would conflict with other future command types.
-struct RpcCommandElement<M>(M);
-
-impl<M> CommandElement for RpcCommandElement<M>
-where
-    M: CommandToGsp,
-    Error: From<M::InitError>,
-{
-    fn size(&self) -> usize {
-        self.0.size()
-    }
-
-    fn write(&self, dev: &device::Device, seq: u32, dst: &mut GspCommand<'_>) -> Result {
-        let command = &self.0;
-        let size_in_bytes = command.size();
-        // Extract area for the command itself. The GSP message header and the command header
-        // together are guaranteed to fit entirely into a single page, so it's ok to only look
-        // at `dst.contents.0` here.
-        let (cmd, payload_1) = M::Command::from_bytes_mut_prefix(dst.contents.0).ok_or(EIO)?;
-        let rpc_header_init = RpcMessageHeader::init(size_in_bytes, M::FUNCTION);
-        // SAFETY: `dst.header.rpc_header_mut()` is a valid reference, and is not touched if the
-        // initializer fails.
-        unsafe {
-            pin_init::raw_try_init(
-                core::ptr::from_mut(dst.header.rpc_header_mut()),
-                rpc_header_init,
-            )?;
-        }
-        // SAFETY: `cmd` is a valid reference, and is not touched if the initializer fails.
-        unsafe {
-            pin_init::raw_try_init(core::ptr::from_mut(cmd), command.init())?;
-        }
-
-        // Fill the variable-length payload, which may be empty.
-        let mut sbuffer = SBufferIter::new_writer([&mut payload_1[..], &mut dst.contents.1[..]]);
-        command.init_variable_payload(&mut sbuffer)?;
-
-        if !sbuffer.is_empty() {
-            return Err(EIO);
-        }
-
-        dev_dbg!(
-            dev,
-            "GSP RPC: send: seq# {}, function={:?}, length=0x{:x}\n",
-            seq,
-            M::FUNCTION,
-            size_in_bytes,
-        );
-
-        Ok(())
-    }
-}
-
-/// Trait representing messages received from the GSP.
-///
-/// A reply that [`Cmdq::send_command`] waits for, or an event that [`Cmdq::await_msg`] waits for.
-/// The receiver matches a message's function code against [`Self::FUNCTION`] and decodes the
-/// message with [`Self::read`].
-pub(crate) trait MessageFromGsp: Sized {
-    /// Function identifying this message from the GSP.
-    const FUNCTION: MsgFunction;
-
-    /// Error type returned by [`MessageFromGsp::read`].
-    type InitError;
-
-    /// Type containing the raw message to be read from the message queue.
-    type Message: FromBytes;
-
-    /// Method reading the message from the message queue and returning it.
-    ///
-    /// From a `Self::Message` and a [`SBufferIter`], constructs an instance of `Self` and returns
-    /// it.
-    fn read(
-        msg: &Self::Message,
-        sbuffer: &mut SBufferIter<core::array::IntoIter<&[u8], 2>>,
-    ) -> Result<Self, Self::InitError>;
-}
-
 /// Number of GSP pages making the [`Msgq`].
 pub(crate) const MSGQ_NUM_PAGES: u32 = 0x3f;
 
@@ -559,96 +413,10 @@ struct GspMessage<'a> {
     contents: (&'a [u8], &'a [u8]),
 }
 
-/// A RPC message ready to be processed from the message queue.
-struct RpcMessage<'a> {
-    // Reference to the RPC header of the message.
-    header: &'a RpcMessageHeader,
-    // Slices to the contents of the message. The second slice is empty unless the message loops
-    // over the message queue.
-    contents: (&'a [u8], &'a [u8]),
-}
-
-impl<'a> RpcMessage<'a> {
-    /// Validate the RPC layer of `element` and returns its RPC header and its contents trimmed down
-    /// to the RPC payload.
-    ///
-    /// # Errors
-    ///
-    /// - `EIO` if the element is shorter than the payload length advertised by the RPC header.
-    fn parse(dev: &device::Device, element: GspMessage<'a>) -> Result<RpcMessage<'a>> {
-        let GspMessage {
-            header,
-            contents: (slice_1, slice_2),
-        } = element;
-
-        let rpc_header = header.rpc_header();
-        let payload_length = rpc_header.rpc_length();
-
-        dev_dbg!(
-            dev,
-            "GSP RPC: receive: seq# {}, function={:?}, length=0x{:x}\n",
-            rpc_header.sequence(),
-            rpc_header.function(),
-            payload_length,
-        );
-
-        // Check that the driver read area is large enough for the message.
-        if slice_1.len() + slice_2.len() < payload_length {
-            return Err(EIO);
-        }
-
-        // Cut the message slices down to the actual length of the message.
-        let (slice_1, slice_2) = if slice_1.len() > payload_length {
-            // PANIC: we checked above that `slice_1` is at least as long as `payload_length`.
-            (slice_1.split_at(payload_length).0, &slice_2[0..0])
-        } else {
-            (
-                slice_1,
-                // PANIC: we checked above that `slice_1.len() + slice_2.len()` is at least as
-                // large as `payload_length`.
-                slice_2.split_at(payload_length - slice_1.len()).0,
-            )
-        };
-
-        Ok(RpcMessage {
-            header: rpc_header,
-            contents: (slice_1, slice_2),
-        })
-    }
-
-    /// Logs an event, meaning a message that no caller was waiting for.
-    ///
-    /// An OS error or robust-channel record is logged at error level and an unknown function code
-    /// at warning level. Other events are not logged.
-    fn log(&self, dev: &device::Device) {
-        let function = self.header.function();
-        let seq = self.header.sequence();
-
-        match function {
-            Ok(MsgFunction::OsErrorLog) => {
-                dev_err!(dev, "GSP reported an OS error (seq {})\n", seq);
-            }
-            Ok(MsgFunction::RcTriggered) => {
-                dev_err!(dev, "GSP triggered robust-channel recovery (seq {})\n", seq);
-            }
-            // Nothing to do for the remaining known function codes.
-            Ok(_) => {}
-            Err(raw) => {
-                dev_warn!(
-                    dev,
-                    "unknown GSP message function {:#x} (seq {})\n",
-                    raw,
-                    seq
-                );
-            }
-        }
-    }
-}
-
 /// A message whose protocol layer has been parsed and validated.
 enum GspParsedMessage<'a> {
-    /// A [RPC message](RpcMessage).
-    Rpc(RpcMessage<'a>),
+    /// A [RPC message](rpc::RpcMessage).
+    Rpc(rpc::RpcMessage<'a>),
 }
 
 impl GspParsedMessage<'_> {
@@ -731,73 +499,6 @@ fn notify_gsp(bar: Bar0<'_>) {
         bar.write_reg(regs::NV_PGSP_QUEUE_HEAD::zeroed().with_address(0u32));
     }
 
-    /// Sends `command` to the GSP and waits for the reply.
-    ///
-    /// Events that arrive before the reply are logged and consumed.
-    ///
-    /// The queue is locked for the entire send+receive cycle to ensure that no other command can
-    /// be interleaved.
-    ///
-    /// # Errors
-    ///
-    /// - `ETIMEDOUT` if space does not become available to send the command, or if the reply does
-    ///   not arrive within [`Self::RECEIVE_TIMEOUT`] of the send, however many events arrive
-    ///   while waiting.
-    /// - `EIO` if the variable payload requested by the command has not been entirely
-    ///   written to by its [`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, 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(command)?;
-
-        inner.await_msg()
-    }
-
-    /// 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, command: M) -> Result
-    where
-        M: CommandToGsp<Reply = NoReply>,
-        Error: From<M::InitError>,
-    {
-        self.inner.lock().send_command(command)
-    }
-
-    /// Waits for an unsolicited GSP event of type `M`. Events that arrive before it are logged and
-    /// consumed.
-    ///
-    /// The queue mutex is held for the whole wait, up to [`Self::RECEIVE_TIMEOUT`], so no other
-    /// caller can send a command or consume an event meanwhile.
-    ///
-    /// # Errors
-    ///
-    /// - `ETIMEDOUT` if the event does not arrive within [`Self::RECEIVE_TIMEOUT`] of the call,
-    ///   however many other events arrive while waiting.
-    /// - `EIO` if a message fails framing or checksum validation.
-    ///
-    /// Error codes returned by [`MessageFromGsp::read`] are propagated as-is.
-    pub(crate) fn await_msg<M: MessageFromGsp>(&self) -> Result<M>
-    where
-        // This allows all error types, including `Infallible`, to be used for `M::InitError`.
-        Error: From<M::InitError>,
-    {
-        self.inner.lock().await_msg()
-    }
-
     /// Logs and consumes every message the GSP has already posted, and returns without waiting for
     /// more.
     ///
@@ -871,36 +572,6 @@ fn send_command_element<E: CommandElement>(&mut self, command: E) -> Result {
         Ok(())
     }
 
-    /// Sends `command` to the GSP.
-    ///
-    /// The command may be split into multiple messages if it is large.
-    ///
-    /// # 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.
-    fn send_command<M>(&mut self, command: M) -> Result
-    where
-        M: CommandToGsp,
-        Error: From<M::InitError>,
-    {
-        match SplitState::new(command)? {
-            SplitState::Single(command) => self.send_command_element(RpcCommandElement(command)),
-            SplitState::Split(command, mut continuations) => {
-                self.send_command_element(RpcCommandElement(command))?;
-
-                while let Some(continuation) = continuations.next() {
-                    self.send_command_element(RpcCommandElement(continuation))?;
-                }
-
-                Ok(())
-            }
-        }
-    }
-
     /// 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
@@ -970,7 +641,7 @@ fn consume_element<R>(
             })?;
 
         // RPC is the only type of message supported for now.
-        let message = GspParsedMessage::Rpc(RpcMessage::parse(self.dev, message)?);
+        let message = GspParsedMessage::Rpc(rpc::RpcMessage::parse(self.dev, message)?);
 
         let res = f(message);
 
@@ -1010,52 +681,6 @@ fn await_msg_with<R>(&mut self, f: impl Fn(GspParsedMessage<'_>) -> Result<R>) -
         }
     }
 
-    /// Receives a message of type `M`, waiting up to [`Cmdq::RECEIVE_TIMEOUT`] from the call.
-    ///
-    /// Any other message that arrives first is logged as an event and does not extend the
-    /// deadline.
-    ///
-    /// # Errors
-    ///
-    /// - `ETIMEDOUT` if no message of type `M` arrives before the deadline, however many other
-    ///   messages arrive while waiting.
-    /// - `EIO` if a message fails framing or checksum validation.
-    ///
-    /// Error codes returned by [`MessageFromGsp::read`] are propagated as-is.
-    fn await_msg<M: MessageFromGsp>(&mut self) -> Result<M>
-    where
-        // This allows all error types, including `Infallible`, to be used for `M::InitError`.
-        Error: From<M::InitError>,
-    {
-        let f = |message: GspParsedMessage<'_>| {
-            let GspParsedMessage::Rpc(rpc_message) = message;
-            let function = rpc_message.header.function();
-            if matches!(function, Ok(f) if f == M::FUNCTION) {
-                let (cmd, contents_1) =
-                    M::Message::from_bytes_prefix(rpc_message.contents.0).ok_or(EIO)?;
-                let mut sbuffer = SBufferIter::new_reader([contents_1, rpc_message.contents.1]);
-
-                M::read(cmd, &mut sbuffer)
-                    .map_err(|e| e.into())
-                    .inspect(|_| {
-                        if !sbuffer.is_empty() {
-                            dev_warn!(
-                                self.dev,
-                                "GSP message {:?} has unprocessed data\n",
-                                M::FUNCTION
-                            );
-                        }
-                    })
-            } else {
-                rpc_message.log(self.dev);
-
-                Err(ENOMSG)
-            }
-        };
-
-        self.await_msg_with(f)
-    }
-
     /// Logs and consumes every message the queue holds.
     ///
     /// # Errors
diff --git a/drivers/gpu/nova-core/gsp/cmdq/rpc.rs b/drivers/gpu/nova-core/gsp/cmdq/rpc.rs
new file mode 100644
index 000000000000..13baf4398632
--- /dev/null
+++ b/drivers/gpu/nova-core/gsp/cmdq/rpc.rs
@@ -0,0 +1,415 @@
+// SPDX-License-Identifier: GPL-2.0
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+
+//! Support for the RPC message type on the GSP command queue.
+
+mod continuation;
+
+use kernel::{
+    device,
+    prelude::*,
+    transmute::{
+        AsBytes,
+        FromBytes, //
+    },
+};
+
+use continuation::SplitState;
+
+use crate::{
+    gsp::{
+        cmdq::{
+            GspCommand,
+            NoReply, //
+        },
+        fw::{
+            MsgFunction,
+            RpcMessageHeader, //
+        },
+    },
+    sbuffer::SBufferIter,
+};
+
+use super::{
+    Cmdq,
+    CmdqInner,
+    CommandElement,
+    GspMessage,
+    GspParsedMessage, //
+};
+
+/// Trait implemented by types representing a command to send to the GSP.
+///
+/// The main purpose of this trait is to provide [`Cmdq`] with the information it needs to send
+/// a given command.
+///
+/// [`CommandToGsp::init`] in particular is responsible for initializing the command directly
+/// into the space reserved for it in the command queue buffer.
+///
+/// Some commands may be followed by a variable-length payload. For these, the
+/// [`CommandToGsp::variable_payload_len`] and [`CommandToGsp::init_variable_payload`] need to be
+/// defined as well.
+pub(crate) trait CommandToGsp {
+    /// Function identifying this command to the GSP.
+    const FUNCTION: MsgFunction;
+
+    /// Type generated by [`CommandToGsp::init`], to be written into the command queue buffer.
+    type Command: FromBytes + AsBytes;
+
+    /// Type of the reply expected from the GSP, or [`NoReply`] for commands that don't
+    /// have a reply.
+    type Reply;
+
+    /// Error type returned by [`CommandToGsp::init`].
+    type InitError;
+
+    /// In-place command initializer responsible for filling the command in the command queue
+    /// buffer.
+    fn init(&self) -> impl Init<Self::Command, Self::InitError>;
+
+    /// Size of the variable-length payload following the command structure generated by
+    /// [`CommandToGsp::init`].
+    ///
+    /// Most commands don't have a variable-length payload, so this is zero by default.
+    fn variable_payload_len(&self) -> usize {
+        0
+    }
+
+    /// Method initializing the variable-length payload.
+    ///
+    /// The command buffer is circular, which means that we may need to jump back to its beginning
+    /// while in the middle of a command. For this reason, the variable-length payload is
+    /// initialized using a [`SBufferIter`].
+    ///
+    /// This method will receive a buffer of the length returned by
+    /// [`CommandToGsp::variable_payload_len`], and must write every single byte of it. Leaving
+    /// unwritten space will lead to an error.
+    ///
+    /// Most commands don't have a variable-length payload, so this does nothing by default.
+    fn init_variable_payload(
+        &self,
+        _dst: &mut SBufferIter<core::array::IntoIter<&mut [u8], 2>>,
+    ) -> Result {
+        Ok(())
+    }
+
+    /// Total size of the command (including its variable-length payload) without the
+    /// [`GspMsgElement`] header.
+    fn size(&self) -> usize {
+        size_of::<Self::Command>() + self.variable_payload_len()
+    }
+}
+
+/// Wrapper type for sending a RPC command as a command queue element.
+///
+/// [`CommandElement`] cannot be directly implemented for all [`CommandToGsp`] with a blanket
+/// implementation as it would conflict with other future command types.
+struct RpcCommandElement<M>(M);
+
+impl<M> CommandElement for RpcCommandElement<M>
+where
+    M: CommandToGsp,
+    Error: From<M::InitError>,
+{
+    fn size(&self) -> usize {
+        self.0.size()
+    }
+
+    fn write(&self, dev: &device::Device, seq: u32, dst: &mut GspCommand<'_>) -> Result {
+        let command = &self.0;
+        let size_in_bytes = command.size();
+        // Extract area for the command itself. The GSP message header and the command header
+        // together are guaranteed to fit entirely into a single page, so it's ok to only look
+        // at `dst.contents.0` here.
+        let (cmd, payload_1) = M::Command::from_bytes_mut_prefix(dst.contents.0).ok_or(EIO)?;
+        let rpc_header_init = RpcMessageHeader::init(size_in_bytes, M::FUNCTION);
+        // SAFETY: `dst.header.rpc_header_mut()` is a valid reference, and is not touched if the
+        // initializer fails.
+        unsafe {
+            pin_init::raw_try_init(
+                core::ptr::from_mut(dst.header.rpc_header_mut()),
+                rpc_header_init,
+            )?;
+        }
+        // SAFETY: `cmd` is a valid reference, and is not touched if the initializer fails.
+        unsafe {
+            pin_init::raw_try_init(core::ptr::from_mut(cmd), command.init())?;
+        }
+
+        // Fill the variable-length payload, which may be empty.
+        let mut sbuffer = SBufferIter::new_writer([&mut payload_1[..], &mut dst.contents.1[..]]);
+        command.init_variable_payload(&mut sbuffer)?;
+
+        if !sbuffer.is_empty() {
+            return Err(EIO);
+        }
+
+        dev_dbg!(
+            dev,
+            "GSP RPC: send: seq# {}, function={:?}, length=0x{:x}\n",
+            seq,
+            M::FUNCTION,
+            size_in_bytes,
+        );
+
+        Ok(())
+    }
+}
+
+/// Trait representing messages received from the GSP.
+///
+/// A reply that [`Cmdq::send_command`] waits for, or an event that [`Cmdq::await_msg`] waits for.
+/// The receiver matches a message's function code against [`Self::FUNCTION`] and decodes the
+/// message with [`Self::read`].
+pub(crate) trait MessageFromGsp: Sized {
+    /// Function identifying this message from the GSP.
+    const FUNCTION: MsgFunction;
+
+    /// Error type returned by [`MessageFromGsp::read`].
+    type InitError;
+
+    /// Type containing the raw message to be read from the message queue.
+    type Message: FromBytes;
+
+    /// Method reading the message from the message queue and returning it.
+    ///
+    /// From a `Self::Message` and a [`SBufferIter`], constructs an instance of `Self` and returns
+    /// it.
+    fn read(
+        msg: &Self::Message,
+        sbuffer: &mut SBufferIter<core::array::IntoIter<&[u8], 2>>,
+    ) -> Result<Self, Self::InitError>;
+}
+
+/// A RPC message ready to be processed from the message queue.
+pub(super) struct RpcMessage<'a> {
+    // Reference to the RPC header of the message.
+    header: &'a RpcMessageHeader,
+    // Slices to the contents of the message. The second slice is empty unless the message loops
+    // over the message queue.
+    contents: (&'a [u8], &'a [u8]),
+}
+
+impl<'a> RpcMessage<'a> {
+    /// Validate the RPC layer of `element` and returns its RPC header and its contents trimmed down
+    /// to the RPC payload.
+    ///
+    /// # Errors
+    ///
+    /// - `EIO` if the element is shorter than the payload length advertised by the RPC header.
+    pub(super) fn parse(dev: &device::Device, element: GspMessage<'a>) -> Result<RpcMessage<'a>> {
+        let GspMessage {
+            header,
+            contents: (slice_1, slice_2),
+        } = element;
+
+        let rpc_header = header.rpc_header();
+        let payload_length = rpc_header.rpc_length();
+
+        dev_dbg!(
+            dev,
+            "GSP RPC: receive: seq# {}, function={:?}, length=0x{:x}\n",
+            rpc_header.sequence(),
+            rpc_header.function(),
+            payload_length,
+        );
+
+        // Check that the driver read area is large enough for the message.
+        if slice_1.len() + slice_2.len() < payload_length {
+            return Err(EIO);
+        }
+
+        // Cut the message slices down to the actual length of the message.
+        let (slice_1, slice_2) = if slice_1.len() > payload_length {
+            // PANIC: we checked above that `slice_1` is at least as long as `payload_length`.
+            (slice_1.split_at(payload_length).0, &slice_2[0..0])
+        } else {
+            (
+                slice_1,
+                // PANIC: we checked above that `slice_1.len() + slice_2.len()` is at least as
+                // large as `payload_length`.
+                slice_2.split_at(payload_length - slice_1.len()).0,
+            )
+        };
+
+        Ok(RpcMessage {
+            header: rpc_header,
+            contents: (slice_1, slice_2),
+        })
+    }
+
+    /// Logs an event, meaning a message that no caller was waiting for.
+    ///
+    /// An OS error or robust-channel record is logged at error level and an unknown function code
+    /// at warning level. Other events are not logged.
+    pub(super) fn log(&self, dev: &device::Device) {
+        let function = self.header.function();
+        let seq = self.header.sequence();
+
+        match function {
+            Ok(MsgFunction::OsErrorLog) => {
+                dev_err!(dev, "GSP reported an OS error (seq {})\n", seq);
+            }
+            Ok(MsgFunction::RcTriggered) => {
+                dev_err!(dev, "GSP triggered robust-channel recovery (seq {})\n", seq);
+            }
+            // Nothing to do for the remaining known function codes.
+            Ok(_) => {}
+            Err(raw) => {
+                dev_warn!(
+                    dev,
+                    "unknown GSP message function {:#x} (seq {})\n",
+                    raw,
+                    seq
+                );
+            }
+        }
+    }
+}
+
+impl Cmdq<'_> {
+    /// Sends `command` to the GSP and waits for the reply.
+    ///
+    /// Events that arrive before the reply are logged and consumed.
+    ///
+    /// The queue is locked for the entire send+receive cycle to ensure that no other command can
+    /// be interleaved.
+    ///
+    /// # Errors
+    ///
+    /// - `ETIMEDOUT` if space does not become available to send the command, or if the reply does
+    ///   not arrive within [`Self::RECEIVE_TIMEOUT`] of the send, however many events arrive
+    ///   while waiting.
+    /// - `EIO` if the variable payload requested by the command has not been entirely
+    ///   written to by its [`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, 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(command)?;
+
+        inner.await_msg()
+    }
+
+    /// 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, command: M) -> Result
+    where
+        M: CommandToGsp<Reply = NoReply>,
+        Error: From<M::InitError>,
+    {
+        self.inner.lock().send_command(command)
+    }
+
+    /// Waits for an unsolicited GSP event of type `M`. Events that arrive before it are logged and
+    /// consumed.
+    ///
+    /// The queue mutex is held for the whole wait, up to [`Self::RECEIVE_TIMEOUT`], so no other
+    /// caller can send a command or consume an event meanwhile.
+    ///
+    /// # Errors
+    ///
+    /// - `ETIMEDOUT` if the event does not arrive within [`Self::RECEIVE_TIMEOUT`] of the call,
+    ///   however many other events arrive while waiting.
+    /// - `EIO` if a message fails framing or checksum validation.
+    ///
+    /// Error codes returned by [`MessageFromGsp::read`] are propagated as-is.
+    pub(crate) fn await_msg<M: MessageFromGsp>(&self) -> Result<M>
+    where
+        // This allows all error types, including `Infallible`, to be used for `M::InitError`.
+        Error: From<M::InitError>,
+    {
+        self.inner.lock().await_msg()
+    }
+}
+
+impl CmdqInner<'_> {
+    /// Sends `command` to the GSP.
+    ///
+    /// The command may be split into multiple messages if it is large.
+    ///
+    /// # 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.
+    fn send_command<M>(&mut self, command: M) -> Result
+    where
+        M: CommandToGsp,
+        Error: From<M::InitError>,
+    {
+        match SplitState::new(command)? {
+            SplitState::Single(command) => self.send_command_element(RpcCommandElement(command)),
+            SplitState::Split(command, mut continuations) => {
+                self.send_command_element(RpcCommandElement(command))?;
+
+                while let Some(continuation) = continuations.next() {
+                    self.send_command_element(RpcCommandElement(continuation))?;
+                }
+
+                Ok(())
+            }
+        }
+    }
+
+    /// Receives a message of type `M`, waiting up to [`Cmdq::RECEIVE_TIMEOUT`] from the call.
+    ///
+    /// Any other message that arrives first is logged as an event and does not extend the
+    /// deadline.
+    ///
+    /// # Errors
+    ///
+    /// - `ETIMEDOUT` if no message of type `M` arrives before the deadline, however many other
+    ///   messages arrive while waiting.
+    /// - `EIO` if a message fails framing or checksum validation.
+    ///
+    /// Error codes returned by [`MessageFromGsp::read`] are propagated as-is.
+    fn await_msg<M: MessageFromGsp>(&mut self) -> Result<M>
+    where
+        // This allows all error types, including `Infallible`, to be used for `M::InitError`.
+        Error: From<M::InitError>,
+    {
+        let f = |message: GspParsedMessage<'_>| {
+            let GspParsedMessage::Rpc(rpc_message) = message;
+            let function = rpc_message.header.function();
+            if matches!(function, Ok(f) if f == M::FUNCTION) {
+                let (cmd, contents_1) =
+                    M::Message::from_bytes_prefix(rpc_message.contents.0).ok_or(EIO)?;
+                let mut sbuffer = SBufferIter::new_reader([contents_1, rpc_message.contents.1]);
+
+                M::read(cmd, &mut sbuffer)
+                    .map_err(|e| e.into())
+                    .inspect(|_| {
+                        if !sbuffer.is_empty() {
+                            dev_warn!(
+                                self.dev,
+                                "GSP message {:?} has unprocessed data\n",
+                                M::FUNCTION
+                            );
+                        }
+                    })
+            } else {
+                rpc_message.log(self.dev);
+
+                Err(ENOMSG)
+            }
+        };
+
+        self.await_msg_with(f)
+    }
+}
diff --git a/drivers/gpu/nova-core/gsp/cmdq/continuation.rs b/drivers/gpu/nova-core/gsp/cmdq/rpc/continuation.rs
similarity index 100%
rename from drivers/gpu/nova-core/gsp/cmdq/continuation.rs
rename to drivers/gpu/nova-core/gsp/cmdq/rpc/continuation.rs
diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index cdbb13674f08..ceac0b0f9423 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -23,9 +23,11 @@
     gpu::Chipset,
     gsp::{
         cmdq::{
+            rpc::{
+                CommandToGsp,
+                MessageFromGsp, //
+            },
             Cmdq,
-            CommandToGsp,
-            MessageFromGsp,
             NoReply, //
         },
         fw::{
diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs b/drivers/gpu/nova-core/gsp/sequencer.rs
index 250adc9fe74f..9f5ab08def2c 100644
--- a/drivers/gpu/nova-core/gsp/sequencer.rs
+++ b/drivers/gpu/nova-core/gsp/sequencer.rs
@@ -28,8 +28,8 @@
     },
     gsp::{
         cmdq::{
-            Cmdq,
-            MessageFromGsp, //
+            rpc::MessageFromGsp,
+            Cmdq, //
         },
         fw,
         GspBootContext,

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 13/14] gpu: nova-core: gsp: move the RPC commands into a sub-module
  2026-10-20 10:08 [PATCH v5 00/14] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types Alexandre Courbot
                   ` (11 preceding siblings ...)
  2026-10-20 10:08 ` [PATCH v5 12/14] gpu: nova-core: gsp: cmdq: move the RPC code into a sub-module Alexandre Courbot
@ 2026-10-20 10:08 ` Alexandre Courbot
  2026-10-20 10:08 ` [PATCH v5 14/14] gpu: nova-core: gsp: add `rpc` to RPC message send/receive methods Alexandre Courbot
  13 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-10-20 10:08 UTC (permalink / raw)
  To: John Hubbard, Danilo Krummrich, Alice Ryhl, David Airlie,
	Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, linux-kernel, rust-for-linux, Alexandre Courbot

Move the types and code related to RPC commands into the
`rpc` sub-module, and update their users to reference them from their
new location.

This is a pure move commit, with no functional change intended.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/api.rs              |   2 +-
 drivers/gpu/nova-core/gpu.rs              |   2 +-
 drivers/gpu/nova-core/gsp.rs              |   4 +-
 drivers/gpu/nova-core/gsp/boot.rs         |  12 +-
 drivers/gpu/nova-core/gsp/commands.rs     | 331 +----------------------------
 drivers/gpu/nova-core/gsp/commands/rpc.rs | 333 ++++++++++++++++++++++++++++++
 6 files changed, 344 insertions(+), 340 deletions(-)

diff --git a/drivers/gpu/nova-core/api.rs b/drivers/gpu/nova-core/api.rs
index 29207c4bb743..eec69aace3b2 100644
--- a/drivers/gpu/nova-core/api.rs
+++ b/drivers/gpu/nova-core/api.rs
@@ -18,7 +18,7 @@
 use crate::{
     driver,
     gpu::Gpu,
-    gsp::commands::GetGspStaticInfoReply, //
+    gsp::commands::rpc::GetGspStaticInfoReply, //
 };
 
 /// API handle for the auxiliary bus child drivers to interact with nova-core.
diff --git a/drivers/gpu/nova-core/gpu.rs b/drivers/gpu/nova-core/gpu.rs
index 4e59072cab50..ac5305d7ff5e 100644
--- a/drivers/gpu/nova-core/gpu.rs
+++ b/drivers/gpu/nova-core/gpu.rs
@@ -35,7 +35,7 @@
     gsp::{
         self,
         cmdq::Cmdq,
-        commands::GetGspStaticInfoReply,
+        commands::rpc::GetGspStaticInfoReply,
         Gsp,
         GspBootContext, //
     },
diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index dda58095f40b..75a3ba7d50f4 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -220,8 +220,8 @@ pub(crate) fn new(
     }
 
     /// Query the GSP for the static GPU information.
-    pub(crate) fn get_static_info(&self) -> Result<commands::GetGspStaticInfoReply> {
-        self.cmdq.send_command(commands::GetGspStaticInfo)
+    pub(crate) fn get_static_info(&self) -> Result<commands::rpc::GetGspStaticInfoReply> {
+        self.cmdq.send_command(commands::rpc::GetGspStaticInfo)
     }
 }
 
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 4fb1b69ac9d5..8e446ad1eac2 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -43,9 +43,9 @@ pub(crate) fn boot(
         let gsp_fw = KBox::pin_init(GspFirmware::new(dev, chipset), GFP_KERNEL)?;
 
         self.cmdq
-            .send_command_no_wait(commands::SetSystemInfo::new(pdev, chipset))?;
+            .send_command_no_wait(commands::rpc::SetSystemInfo::new(pdev, chipset))?;
         self.cmdq
-            .send_command_no_wait(commands::SetRegistry::new(ctx.vgpu.state())?)?;
+            .send_command_no_wait(commands::rpc::SetRegistry::new(ctx.vgpu.state())?)?;
 
         // Perform the chipset-specific boot sequence, and retrieve the unload bundle.
         let unload_bundle = hal.boot(&self, &mut ctx, &gsp_fw)?.or_else(|| {
@@ -79,7 +79,7 @@ pub(crate) fn boot(
         hal.post_boot(&self, ctx, &gsp_fw)?;
 
         // Wait until GSP is fully initialized.
-        commands::wait_gsp_init_done(&self.cmdq)?;
+        commands::rpc::wait_gsp_init_done(&self.cmdq)?;
 
         Ok(unload_guard.dismiss().1)
     }
@@ -88,10 +88,10 @@ pub(crate) fn boot(
     fn shutdown_gsp(
         cmdq: &Cmdq<'_>,
         gsp_falcon: &Falcon<'_, Gsp>,
-        mode: commands::PowerStateLevel,
+        mode: commands::rpc::PowerStateLevel,
     ) -> Result {
         // Command to shut the GSP down.
-        cmdq.send_command(commands::UnloadingGuestDriver::new(mode))?;
+        cmdq.send_command(commands::rpc::UnloadingGuestDriver::new(mode))?;
 
         // Wait until GSP signals it is suspended.
         const LIBOS_INTERRUPT_PROCESSOR_SUSPENDED: u32 = bits::bit_u32(31);
@@ -118,7 +118,7 @@ pub(crate) fn unload(
         let mut res = Self::shutdown_gsp(
             &self.cmdq,
             ctx.gsp_falcon,
-            commands::PowerStateLevel::Level0,
+            commands::rpc::PowerStateLevel::Level0,
         )
         .inspect_err(|e| dev_err!(dev, "GSP shutdown failed: {:?}\n", e));
 
diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index ceac0b0f9423..5f1c944678be 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -1,333 +1,4 @@
 // SPDX-License-Identifier: GPL-2.0
 // SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 
-use core::{
-    array,
-    convert::Infallible,
-    ffi::FromBytesUntilNulError,
-    ops::Range,
-    str::Utf8Error, //
-};
-
-use kernel::{
-    device,
-    pci,
-    prelude::*,
-    transmute::{
-        AsBytes,
-        FromBytes, //
-    }, //
-};
-
-use crate::{
-    gpu::Chipset,
-    gsp::{
-        cmdq::{
-            rpc::{
-                CommandToGsp,
-                MessageFromGsp, //
-            },
-            Cmdq,
-            NoReply, //
-        },
-        fw::{
-            self,
-            MsgFunction, //
-        },
-    },
-    sbuffer::SBufferIter,
-    vgpu::VgpuState, //
-};
-
-/// The `GspSetSystemInfo` command.
-pub(crate) struct SetSystemInfo<'a> {
-    pdev: &'a pci::Device<device::Bound>,
-    chipset: Chipset,
-}
-
-impl<'a> SetSystemInfo<'a> {
-    /// Creates a new `GspSetSystemInfo` command using the parameters of `pdev`.
-    pub(crate) fn new(pdev: &'a pci::Device<device::Bound>, chipset: Chipset) -> Self {
-        Self { pdev, chipset }
-    }
-}
-
-impl<'a> CommandToGsp for SetSystemInfo<'a> {
-    const FUNCTION: MsgFunction = MsgFunction::GspSetSystemInfo;
-    type Command = fw::commands::GspSetSystemInfo;
-    type Reply = NoReply;
-    type InitError = Error;
-
-    fn init(&self) -> impl Init<Self::Command, Self::InitError> {
-        Self::Command::init(self.pdev, self.chipset)
-    }
-}
-
-struct RegistryEntry {
-    key: &'static str,
-    value: u32,
-}
-
-/// The `SetRegistry` command.
-pub(crate) struct SetRegistry {
-    entries: KVec<RegistryEntry>,
-}
-
-impl SetRegistry {
-    /// Creates a new `SetRegistry` command, using a set of hardcoded entries.
-    pub(crate) fn new(vgpu_state: VgpuState) -> Result<Self> {
-        let mut entries = KVec::new();
-
-        // RMSecBusResetEnable - enables PCI secondary bus reset
-        entries.push(
-            RegistryEntry {
-                key: "RMSecBusResetEnable",
-                value: 1,
-            },
-            GFP_KERNEL,
-        )?;
-
-        // RMForcePcieConfigSave - forces GSP-RM to preserve PCI configuration registers on
-        // any PCI reset.
-        entries.push(
-            RegistryEntry {
-                key: "RMForcePcieConfigSave",
-                value: 1,
-            },
-            GFP_KERNEL,
-        )?;
-
-        // RMDevidCheckIgnore - allows GSP-RM to boot even if the PCI dev ID is not found
-        // in the internal product name database.
-        entries.push(
-            RegistryEntry {
-                key: "RMDevidCheckIgnore",
-                value: 1,
-            },
-            GFP_KERNEL,
-        )?;
-
-        if matches!(vgpu_state, VgpuState::Enabled { .. }) {
-            // RMSetSriovMode - required when vGPU is enabled.
-            entries.push(
-                RegistryEntry {
-                    key: "RMSetSriovMode",
-                    value: 1,
-                },
-                GFP_KERNEL,
-            )?;
-        }
-
-        Ok(Self { entries })
-    }
-}
-
-impl CommandToGsp for SetRegistry {
-    const FUNCTION: MsgFunction = MsgFunction::SetRegistry;
-    type Command = fw::commands::PackedRegistryTable;
-    type Reply = NoReply;
-    type InitError = Infallible;
-
-    fn init(&self) -> impl Init<Self::Command, Self::InitError> {
-        Self::Command::init(self.entries.len() as u32, self.size() as u32)
-    }
-
-    fn variable_payload_len(&self) -> usize {
-        let mut key_size = 0;
-        for entry in self.entries.iter() {
-            key_size += entry.key.len() + 1; // +1 for NULL terminator
-        }
-        self.entries.len() * size_of::<fw::commands::PackedRegistryEntry>() + key_size
-    }
-
-    fn init_variable_payload(
-        &self,
-        dst: &mut SBufferIter<core::array::IntoIter<&mut [u8], 2>>,
-    ) -> Result {
-        let string_data_start_offset = size_of::<Self::Command>()
-            + self.entries.len() * size_of::<fw::commands::PackedRegistryEntry>();
-
-        // Array for string data.
-        let mut string_data = KVec::new();
-
-        for entry in self.entries.iter() {
-            dst.write_all(
-                fw::commands::PackedRegistryEntry::new(
-                    (string_data_start_offset + string_data.len()) as u32,
-                    entry.value,
-                )
-                .as_bytes(),
-            )?;
-
-            let key_bytes = entry.key.as_bytes();
-            string_data.extend_from_slice(key_bytes, GFP_KERNEL)?;
-            string_data.push(0, GFP_KERNEL)?;
-        }
-
-        dst.write_all(string_data.as_slice())
-    }
-}
-
-/// Message type for GSP initialization done notification.
-struct GspInitDone;
-
-// SAFETY: `GspInitDone` is a zero-sized type with no bytes, therefore it
-// trivially has no uninitialized bytes.
-unsafe impl FromBytes for GspInitDone {}
-
-impl MessageFromGsp for GspInitDone {
-    const FUNCTION: MsgFunction = MsgFunction::GspInitDone;
-    type InitError = Infallible;
-    type Message = ();
-
-    fn read(
-        _msg: &Self::Message,
-        _sbuffer: &mut SBufferIter<array::IntoIter<&[u8], 2>>,
-    ) -> Result<Self, Self::InitError> {
-        Ok(GspInitDone)
-    }
-}
-
-/// Waits for GSP initialization to complete.
-pub(crate) fn wait_gsp_init_done(cmdq: &Cmdq<'_>) -> Result {
-    cmdq.await_msg::<GspInitDone>().map(|_| ())
-}
-
-/// The `GetGspStaticInfo` command.
-pub(crate) struct GetGspStaticInfo;
-
-impl CommandToGsp for GetGspStaticInfo {
-    const FUNCTION: MsgFunction = MsgFunction::GetGspStaticInfo;
-    type Command = fw::commands::GspStaticConfigInfo;
-    type Reply = GetGspStaticInfoReply;
-    type InitError = Infallible;
-
-    fn init(&self) -> impl Init<Self::Command, Self::InitError> {
-        Self::Command::init_zeroed()
-    }
-}
-
-/// The reply from the GSP to the [`GetGspStaticInfo`] command.
-pub struct GetGspStaticInfoReply {
-    gpu_name: [u8; 64],
-    gpu_short_name: [u8; 64],
-    /// The 16-byte SHA-1 based GPU identifier (GID) reported by GSP-RM.
-    pub gpu_gid: [u8; 16],
-    /// BAR1 Page Directory Entry base address.
-    pub(crate) bar1_pde_base: u64,
-    /// Usable FB (VRAM) regions for driver memory allocation.
-    pub(crate) usable_fb_regions: KVec<Range<u64>>,
-    /// Exclusive end of the FB physical address space.
-    pub(crate) total_fb_end: u64,
-}
-
-impl MessageFromGsp for GetGspStaticInfoReply {
-    const FUNCTION: MsgFunction = MsgFunction::GetGspStaticInfo;
-    type Message = fw::commands::GspStaticConfigInfo;
-    type InitError = Error;
-
-    fn read(
-        msg: &Self::Message,
-        _sbuffer: &mut SBufferIter<array::IntoIter<&[u8], 2>>,
-    ) -> Result<Self, Self::InitError> {
-        let mut usable_fb_regions = KVec::new();
-        for region in msg.usable_fb_regions() {
-            usable_fb_regions.push(region, GFP_KERNEL)?;
-        }
-        let total_fb_end = msg.total_fb_end().ok_or(EINVAL)?;
-
-        Ok(GetGspStaticInfoReply {
-            gpu_name: msg.gpu_name_str(),
-            gpu_short_name: msg.gpu_short_name_str(),
-            gpu_gid: msg.gpu_gid(),
-            bar1_pde_base: msg.bar1_pde_base(),
-            usable_fb_regions,
-            total_fb_end,
-        })
-    }
-}
-
-/// Error type for [`GetGspStaticInfoReply::gpu_name`].
-#[derive(Debug)]
-pub enum GpuNameError {
-    /// The GPU name string does not contain a null terminator.
-    NoNullTerminator(FromBytesUntilNulError),
-
-    /// The GPU name string contains invalid UTF-8.
-    InvalidUtf8(Utf8Error),
-}
-
-impl GetGspStaticInfoReply {
-    /// Returns the name of the GPU as a string.
-    ///
-    /// Returns an error if the string given by the GSP does not contain a null terminator or
-    /// contains invalid UTF-8.
-    pub fn gpu_name(&self) -> Result<&str, GpuNameError> {
-        CStr::from_bytes_until_nul(&self.gpu_name)
-            .map_err(GpuNameError::NoNullTerminator)?
-            .to_str()
-            .map_err(GpuNameError::InvalidUtf8)
-    }
-
-    /// Returns the short name of the GPU as a string.
-    ///
-    /// Returns an error if the string given by the GSP does not contain a null terminator or
-    /// contains invalid UTF-8.
-    pub fn gpu_short_name(&self) -> core::result::Result<&str, GpuNameError> {
-        CStr::from_bytes_until_nul(&self.gpu_short_name)
-            .map_err(GpuNameError::NoNullTerminator)?
-            .to_str()
-            .map_err(GpuNameError::InvalidUtf8)
-    }
-
-    /// Returns the total usable VRAM size in bytes, i.e. the summed lengths of all usable FB
-    /// regions.
-    pub fn vram_size(&self) -> u64 {
-        self.usable_fb_regions.iter().fold(0, |size, region| {
-            size.saturating_add(region.end - region.start)
-        })
-    }
-}
-
-pub(crate) use fw::commands::PowerStateLevel;
-
-/// The `UnloadingGuestDriver` command, used to shut down the GSP.
-///
-/// Only used within the `gsp` module.
-pub(super) struct UnloadingGuestDriver {
-    level: PowerStateLevel,
-}
-
-impl UnloadingGuestDriver {
-    /// Creates a new `UnloadingGuestDriver` command for the given [`PowerStateLevel`].
-    pub(super) fn new(level: PowerStateLevel) -> Self {
-        Self { level }
-    }
-}
-
-impl CommandToGsp for UnloadingGuestDriver {
-    const FUNCTION: MsgFunction = MsgFunction::UnloadingGuestDriver;
-    type Command = fw::commands::UnloadingGuestDriver;
-    type Reply = UnloadingGuestDriverReply;
-    type InitError = Infallible;
-
-    fn init(&self) -> impl Init<Self::Command, Self::InitError> {
-        fw::commands::UnloadingGuestDriver::new(self.level)
-    }
-}
-
-/// The reply from the GSP to the [`UnloadingGuestDriver`] command.
-pub(super) struct UnloadingGuestDriverReply;
-
-impl MessageFromGsp for UnloadingGuestDriverReply {
-    const FUNCTION: MsgFunction = MsgFunction::UnloadingGuestDriver;
-    type InitError = Infallible;
-    type Message = ();
-
-    fn read(
-        _msg: &Self::Message,
-        _sbuffer: &mut SBufferIter<array::IntoIter<&[u8], 2>>,
-    ) -> Result<Self, Self::InitError> {
-        Ok(UnloadingGuestDriverReply)
-    }
-}
+pub(crate) mod rpc;
diff --git a/drivers/gpu/nova-core/gsp/commands/rpc.rs b/drivers/gpu/nova-core/gsp/commands/rpc.rs
new file mode 100644
index 000000000000..54c40f29e367
--- /dev/null
+++ b/drivers/gpu/nova-core/gsp/commands/rpc.rs
@@ -0,0 +1,333 @@
+// SPDX-License-Identifier: GPL-2.0
+// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+
+use core::{
+    array,
+    convert::Infallible,
+    ffi::FromBytesUntilNulError,
+    ops::Range,
+    str::Utf8Error, //
+};
+
+use kernel::{
+    device,
+    pci,
+    prelude::*,
+    transmute::{
+        AsBytes,
+        FromBytes, //
+    }, //
+};
+
+use crate::{
+    gpu::Chipset,
+    gsp::{
+        cmdq::{
+            rpc::{
+                CommandToGsp,
+                MessageFromGsp, //
+            },
+            Cmdq,
+            NoReply, //
+        },
+        fw::{
+            self,
+            MsgFunction, //
+        },
+    },
+    sbuffer::SBufferIter,
+    vgpu::VgpuState, //
+};
+
+/// The `GspSetSystemInfo` command.
+pub(crate) struct SetSystemInfo<'a> {
+    pdev: &'a pci::Device<device::Bound>,
+    chipset: Chipset,
+}
+
+impl<'a> SetSystemInfo<'a> {
+    /// Creates a new `GspSetSystemInfo` command using the parameters of `pdev`.
+    pub(crate) fn new(pdev: &'a pci::Device<device::Bound>, chipset: Chipset) -> Self {
+        Self { pdev, chipset }
+    }
+}
+
+impl<'a> CommandToGsp for SetSystemInfo<'a> {
+    const FUNCTION: MsgFunction = MsgFunction::GspSetSystemInfo;
+    type Command = fw::commands::GspSetSystemInfo;
+    type Reply = NoReply;
+    type InitError = Error;
+
+    fn init(&self) -> impl Init<Self::Command, Self::InitError> {
+        Self::Command::init(self.pdev, self.chipset)
+    }
+}
+
+struct RegistryEntry {
+    key: &'static str,
+    value: u32,
+}
+
+/// The `SetRegistry` command.
+pub(crate) struct SetRegistry {
+    entries: KVec<RegistryEntry>,
+}
+
+impl SetRegistry {
+    /// Creates a new `SetRegistry` command, using a set of hardcoded entries.
+    pub(crate) fn new(vgpu_state: VgpuState) -> Result<Self> {
+        let mut entries = KVec::new();
+
+        // RMSecBusResetEnable - enables PCI secondary bus reset
+        entries.push(
+            RegistryEntry {
+                key: "RMSecBusResetEnable",
+                value: 1,
+            },
+            GFP_KERNEL,
+        )?;
+
+        // RMForcePcieConfigSave - forces GSP-RM to preserve PCI configuration registers on
+        // any PCI reset.
+        entries.push(
+            RegistryEntry {
+                key: "RMForcePcieConfigSave",
+                value: 1,
+            },
+            GFP_KERNEL,
+        )?;
+
+        // RMDevidCheckIgnore - allows GSP-RM to boot even if the PCI dev ID is not found
+        // in the internal product name database.
+        entries.push(
+            RegistryEntry {
+                key: "RMDevidCheckIgnore",
+                value: 1,
+            },
+            GFP_KERNEL,
+        )?;
+
+        if matches!(vgpu_state, VgpuState::Enabled { .. }) {
+            // RMSetSriovMode - required when vGPU is enabled.
+            entries.push(
+                RegistryEntry {
+                    key: "RMSetSriovMode",
+                    value: 1,
+                },
+                GFP_KERNEL,
+            )?;
+        }
+
+        Ok(Self { entries })
+    }
+}
+
+impl CommandToGsp for SetRegistry {
+    const FUNCTION: MsgFunction = MsgFunction::SetRegistry;
+    type Command = fw::commands::PackedRegistryTable;
+    type Reply = NoReply;
+    type InitError = Infallible;
+
+    fn init(&self) -> impl Init<Self::Command, Self::InitError> {
+        Self::Command::init(self.entries.len() as u32, self.size() as u32)
+    }
+
+    fn variable_payload_len(&self) -> usize {
+        let mut key_size = 0;
+        for entry in self.entries.iter() {
+            key_size += entry.key.len() + 1; // +1 for NULL terminator
+        }
+        self.entries.len() * size_of::<fw::commands::PackedRegistryEntry>() + key_size
+    }
+
+    fn init_variable_payload(
+        &self,
+        dst: &mut SBufferIter<core::array::IntoIter<&mut [u8], 2>>,
+    ) -> Result {
+        let string_data_start_offset = size_of::<Self::Command>()
+            + self.entries.len() * size_of::<fw::commands::PackedRegistryEntry>();
+
+        // Array for string data.
+        let mut string_data = KVec::new();
+
+        for entry in self.entries.iter() {
+            dst.write_all(
+                fw::commands::PackedRegistryEntry::new(
+                    (string_data_start_offset + string_data.len()) as u32,
+                    entry.value,
+                )
+                .as_bytes(),
+            )?;
+
+            let key_bytes = entry.key.as_bytes();
+            string_data.extend_from_slice(key_bytes, GFP_KERNEL)?;
+            string_data.push(0, GFP_KERNEL)?;
+        }
+
+        dst.write_all(string_data.as_slice())
+    }
+}
+
+/// Message type for GSP initialization done notification.
+struct GspInitDone;
+
+// SAFETY: `GspInitDone` is a zero-sized type with no bytes, therefore it
+// trivially has no uninitialized bytes.
+unsafe impl FromBytes for GspInitDone {}
+
+impl MessageFromGsp for GspInitDone {
+    const FUNCTION: MsgFunction = MsgFunction::GspInitDone;
+    type InitError = Infallible;
+    type Message = ();
+
+    fn read(
+        _msg: &Self::Message,
+        _sbuffer: &mut SBufferIter<array::IntoIter<&[u8], 2>>,
+    ) -> Result<Self, Self::InitError> {
+        Ok(GspInitDone)
+    }
+}
+
+/// Waits for GSP initialization to complete.
+pub(crate) fn wait_gsp_init_done(cmdq: &Cmdq<'_>) -> Result {
+    cmdq.await_msg::<GspInitDone>().map(|_| ())
+}
+
+/// The `GetGspStaticInfo` command.
+pub(crate) struct GetGspStaticInfo;
+
+impl CommandToGsp for GetGspStaticInfo {
+    const FUNCTION: MsgFunction = MsgFunction::GetGspStaticInfo;
+    type Command = fw::commands::GspStaticConfigInfo;
+    type Reply = GetGspStaticInfoReply;
+    type InitError = Infallible;
+
+    fn init(&self) -> impl Init<Self::Command, Self::InitError> {
+        Self::Command::init_zeroed()
+    }
+}
+
+/// The reply from the GSP to the [`GetGspStaticInfo`] command.
+pub struct GetGspStaticInfoReply {
+    gpu_name: [u8; 64],
+    gpu_short_name: [u8; 64],
+    /// The 16-byte SHA-1 based GPU identifier (GID) reported by GSP-RM.
+    pub gpu_gid: [u8; 16],
+    /// BAR1 Page Directory Entry base address.
+    pub(crate) bar1_pde_base: u64,
+    /// Usable FB (VRAM) regions for driver memory allocation.
+    pub(crate) usable_fb_regions: KVec<Range<u64>>,
+    /// Exclusive end of the FB physical address space.
+    pub(crate) total_fb_end: u64,
+}
+
+impl MessageFromGsp for GetGspStaticInfoReply {
+    const FUNCTION: MsgFunction = MsgFunction::GetGspStaticInfo;
+    type Message = fw::commands::GspStaticConfigInfo;
+    type InitError = Error;
+
+    fn read(
+        msg: &Self::Message,
+        _sbuffer: &mut SBufferIter<array::IntoIter<&[u8], 2>>,
+    ) -> Result<Self, Self::InitError> {
+        let mut usable_fb_regions = KVec::new();
+        for region in msg.usable_fb_regions() {
+            usable_fb_regions.push(region, GFP_KERNEL)?;
+        }
+        let total_fb_end = msg.total_fb_end().ok_or(EINVAL)?;
+
+        Ok(GetGspStaticInfoReply {
+            gpu_name: msg.gpu_name_str(),
+            gpu_short_name: msg.gpu_short_name_str(),
+            gpu_gid: msg.gpu_gid(),
+            bar1_pde_base: msg.bar1_pde_base(),
+            usable_fb_regions,
+            total_fb_end,
+        })
+    }
+}
+
+/// Error type for [`GetGspStaticInfoReply::gpu_name`].
+#[derive(Debug)]
+pub enum GpuNameError {
+    /// The GPU name string does not contain a null terminator.
+    NoNullTerminator(FromBytesUntilNulError),
+
+    /// The GPU name string contains invalid UTF-8.
+    InvalidUtf8(Utf8Error),
+}
+
+impl GetGspStaticInfoReply {
+    /// Returns the name of the GPU as a string.
+    ///
+    /// Returns an error if the string given by the GSP does not contain a null terminator or
+    /// contains invalid UTF-8.
+    pub fn gpu_name(&self) -> Result<&str, GpuNameError> {
+        CStr::from_bytes_until_nul(&self.gpu_name)
+            .map_err(GpuNameError::NoNullTerminator)?
+            .to_str()
+            .map_err(GpuNameError::InvalidUtf8)
+    }
+
+    /// Returns the short name of the GPU as a string.
+    ///
+    /// Returns an error if the string given by the GSP does not contain a null terminator or
+    /// contains invalid UTF-8.
+    pub fn gpu_short_name(&self) -> core::result::Result<&str, GpuNameError> {
+        CStr::from_bytes_until_nul(&self.gpu_short_name)
+            .map_err(GpuNameError::NoNullTerminator)?
+            .to_str()
+            .map_err(GpuNameError::InvalidUtf8)
+    }
+
+    /// Returns the total usable VRAM size in bytes, i.e. the summed lengths of all usable FB
+    /// regions.
+    pub fn vram_size(&self) -> u64 {
+        self.usable_fb_regions.iter().fold(0, |size, region| {
+            size.saturating_add(region.end - region.start)
+        })
+    }
+}
+
+pub(crate) use fw::commands::PowerStateLevel;
+
+/// The `UnloadingGuestDriver` command, used to shut down the GSP.
+///
+/// Only used within the `gsp` module.
+pub(crate) struct UnloadingGuestDriver {
+    level: PowerStateLevel,
+}
+
+impl UnloadingGuestDriver {
+    /// Creates a new `UnloadingGuestDriver` command for the given [`PowerStateLevel`].
+    pub(crate) fn new(level: PowerStateLevel) -> Self {
+        Self { level }
+    }
+}
+
+impl CommandToGsp for UnloadingGuestDriver {
+    const FUNCTION: MsgFunction = MsgFunction::UnloadingGuestDriver;
+    type Command = fw::commands::UnloadingGuestDriver;
+    type Reply = UnloadingGuestDriverReply;
+    type InitError = Infallible;
+
+    fn init(&self) -> impl Init<Self::Command, Self::InitError> {
+        fw::commands::UnloadingGuestDriver::new(self.level)
+    }
+}
+
+/// The reply from the GSP to the [`UnloadingGuestDriver`] command.
+pub(crate) struct UnloadingGuestDriverReply;
+
+impl MessageFromGsp for UnloadingGuestDriverReply {
+    const FUNCTION: MsgFunction = MsgFunction::UnloadingGuestDriver;
+    type InitError = Infallible;
+    type Message = ();
+
+    fn read(
+        _msg: &Self::Message,
+        _sbuffer: &mut SBufferIter<array::IntoIter<&[u8], 2>>,
+    ) -> Result<Self, Self::InitError> {
+        Ok(UnloadingGuestDriverReply)
+    }
+}

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v5 14/14] gpu: nova-core: gsp: add `rpc` to RPC message send/receive methods
  2026-10-20 10:08 [PATCH v5 00/14] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types Alexandre Courbot
                   ` (12 preceding siblings ...)
  2026-10-20 10:08 ` [PATCH v5 13/14] gpu: nova-core: gsp: move the RPC commands " Alexandre Courbot
@ 2026-10-20 10:08 ` Alexandre Courbot
  13 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-10-20 10:08 UTC (permalink / raw)
  To: John Hubbard, Danilo Krummrich, Alice Ryhl, David Airlie,
	Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, linux-kernel, rust-for-linux, Alexandre Courbot

Since we are going to introduce a second type of message, add `rpc` to
the names of the relevant command queue methods so we can distinguish
between the two.

No functional change intended.

Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp.rs              |  2 +-
 drivers/gpu/nova-core/gsp/boot.rs         |  6 +++---
 drivers/gpu/nova-core/gsp/cmdq.rs         |  2 +-
 drivers/gpu/nova-core/gsp/cmdq/rpc.rs     | 24 ++++++++++++------------
 drivers/gpu/nova-core/gsp/commands/rpc.rs |  2 +-
 drivers/gpu/nova-core/gsp/sequencer.rs    |  2 +-
 6 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index 75a3ba7d50f4..ed7eef21e8f9 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -221,7 +221,7 @@ pub(crate) fn new(
 
     /// Query the GSP for the static GPU information.
     pub(crate) fn get_static_info(&self) -> Result<commands::rpc::GetGspStaticInfoReply> {
-        self.cmdq.send_command(commands::rpc::GetGspStaticInfo)
+        self.cmdq.send_rpc_command(commands::rpc::GetGspStaticInfo)
     }
 }
 
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 8e446ad1eac2..0d0b07fafd4f 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -43,9 +43,9 @@ pub(crate) fn boot(
         let gsp_fw = KBox::pin_init(GspFirmware::new(dev, chipset), GFP_KERNEL)?;
 
         self.cmdq
-            .send_command_no_wait(commands::rpc::SetSystemInfo::new(pdev, chipset))?;
+            .send_rpc_command_no_wait(commands::rpc::SetSystemInfo::new(pdev, chipset))?;
         self.cmdq
-            .send_command_no_wait(commands::rpc::SetRegistry::new(ctx.vgpu.state())?)?;
+            .send_rpc_command_no_wait(commands::rpc::SetRegistry::new(ctx.vgpu.state())?)?;
 
         // Perform the chipset-specific boot sequence, and retrieve the unload bundle.
         let unload_bundle = hal.boot(&self, &mut ctx, &gsp_fw)?.or_else(|| {
@@ -91,7 +91,7 @@ fn shutdown_gsp(
         mode: commands::rpc::PowerStateLevel,
     ) -> Result {
         // Command to shut the GSP down.
-        cmdq.send_command(commands::rpc::UnloadingGuestDriver::new(mode))?;
+        cmdq.send_rpc_command(commands::rpc::UnloadingGuestDriver::new(mode))?;
 
         // Wait until GSP signals it is suspended.
         const LIBOS_INTERRUPT_PROCESSOR_SUSPENDED: u32 = bits::bit_u32(31);
diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
index c5a2f08cda94..48f3116e8599 100644
--- a/drivers/gpu/nova-core/gsp/cmdq.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq.rs
@@ -62,7 +62,7 @@
 use super::regs;
 
 /// Marker type representing the absence of a reply for a command. Commands using this as their
-/// reply type are sent using [`Cmdq::send_command_no_wait`].
+/// reply type are sent using [`Cmdq::send_rpc_command_no_wait`].
 pub(crate) struct NoReply;
 
 /// Trait implemented by types that can be sent as a single command queue element.
diff --git a/drivers/gpu/nova-core/gsp/cmdq/rpc.rs b/drivers/gpu/nova-core/gsp/cmdq/rpc.rs
index 13baf4398632..2c52ec78871b 100644
--- a/drivers/gpu/nova-core/gsp/cmdq/rpc.rs
+++ b/drivers/gpu/nova-core/gsp/cmdq/rpc.rs
@@ -158,9 +158,9 @@ fn write(&self, dev: &device::Device, seq: u32, dst: &mut GspCommand<'_>) -> Res
 
 /// Trait representing messages received from the GSP.
 ///
-/// A reply that [`Cmdq::send_command`] waits for, or an event that [`Cmdq::await_msg`] waits for.
-/// The receiver matches a message's function code against [`Self::FUNCTION`] and decodes the
-/// message with [`Self::read`].
+/// A reply that [`Cmdq::send_rpc_command`] waits for, or an event that [`Cmdq::await_rpc_msg`]
+/// waits for. The receiver matches a message's function code against [`Self::FUNCTION`] and decodes
+/// the message with [`Self::read`].
 pub(crate) trait MessageFromGsp: Sized {
     /// Function identifying this message from the GSP.
     const FUNCTION: MsgFunction;
@@ -284,7 +284,7 @@ impl Cmdq<'_> {
     ///   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, command: M) -> Result<M::Reply>
+    pub(crate) fn send_rpc_command<M>(&self, command: M) -> Result<M::Reply>
     where
         M: CommandToGsp,
         M::Reply: MessageFromGsp,
@@ -292,9 +292,9 @@ pub(crate) fn send_command<M>(&self, command: M) -> Result<M::Reply>
         Error: From<<M::Reply as MessageFromGsp>::InitError>,
     {
         let mut inner = self.inner.lock();
-        inner.send_command(command)?;
+        inner.send_rpc_command(command)?;
 
-        inner.await_msg()
+        inner.await_rpc_msg()
     }
 
     /// Sends `command` to the GSP without waiting for a reply.
@@ -306,12 +306,12 @@ pub(crate) fn send_command<M>(&self, command: M) -> Result<M::Reply>
     ///   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, command: M) -> Result
+    pub(crate) fn send_rpc_command_no_wait<M>(&self, command: M) -> Result
     where
         M: CommandToGsp<Reply = NoReply>,
         Error: From<M::InitError>,
     {
-        self.inner.lock().send_command(command)
+        self.inner.lock().send_rpc_command(command)
     }
 
     /// Waits for an unsolicited GSP event of type `M`. Events that arrive before it are logged and
@@ -327,12 +327,12 @@ pub(crate) fn send_command_no_wait<M>(&self, command: M) -> Result
     /// - `EIO` if a message fails framing or checksum validation.
     ///
     /// Error codes returned by [`MessageFromGsp::read`] are propagated as-is.
-    pub(crate) fn await_msg<M: MessageFromGsp>(&self) -> Result<M>
+    pub(crate) fn await_rpc_msg<M: MessageFromGsp>(&self) -> Result<M>
     where
         // This allows all error types, including `Infallible`, to be used for `M::InitError`.
         Error: From<M::InitError>,
     {
-        self.inner.lock().await_msg()
+        self.inner.lock().await_rpc_msg()
     }
 }
 
@@ -348,7 +348,7 @@ impl CmdqInner<'_> {
     ///   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, command: M) -> Result
+    fn send_rpc_command<M>(&mut self, command: M) -> Result
     where
         M: CommandToGsp,
         Error: From<M::InitError>,
@@ -379,7 +379,7 @@ fn send_command<M>(&mut self, command: M) -> Result
     /// - `EIO` if a message fails framing or checksum validation.
     ///
     /// Error codes returned by [`MessageFromGsp::read`] are propagated as-is.
-    fn await_msg<M: MessageFromGsp>(&mut self) -> Result<M>
+    fn await_rpc_msg<M: MessageFromGsp>(&mut self) -> Result<M>
     where
         // This allows all error types, including `Infallible`, to be used for `M::InitError`.
         Error: From<M::InitError>,
diff --git a/drivers/gpu/nova-core/gsp/commands/rpc.rs b/drivers/gpu/nova-core/gsp/commands/rpc.rs
index 54c40f29e367..156357dd48df 100644
--- a/drivers/gpu/nova-core/gsp/commands/rpc.rs
+++ b/drivers/gpu/nova-core/gsp/commands/rpc.rs
@@ -190,7 +190,7 @@ fn read(
 
 /// Waits for GSP initialization to complete.
 pub(crate) fn wait_gsp_init_done(cmdq: &Cmdq<'_>) -> Result {
-    cmdq.await_msg::<GspInitDone>().map(|_| ())
+    cmdq.await_rpc_msg::<GspInitDone>().map(|_| ())
 }
 
 /// The `GetGspStaticInfo` command.
diff --git a/drivers/gpu/nova-core/gsp/sequencer.rs b/drivers/gpu/nova-core/gsp/sequencer.rs
index 9f5ab08def2c..23b09fb875e9 100644
--- a/drivers/gpu/nova-core/gsp/sequencer.rs
+++ b/drivers/gpu/nova-core/gsp/sequencer.rs
@@ -343,7 +343,7 @@ pub(crate) fn run(
         libos: &'a Coherent<'a, [LibosMemoryRegionInitArgument]>,
         bootloader_app_version: u32,
     ) -> Result {
-        let seq_info = cmdq.await_msg::<GspSequence>()?;
+        let seq_info = cmdq.await_rpc_msg::<GspSequence>()?;
 
         let sequencer = GspSequencer {
             bar: ctx.bar,

-- 
2.56.0


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 12/14] gpu: nova-core: gsp: cmdq: move the RPC code into a sub-module
  2026-10-20 10:08 ` [PATCH v5 12/14] gpu: nova-core: gsp: cmdq: move the RPC code into a sub-module Alexandre Courbot
@ 2026-10-20 10:15   ` sashiko-bot
  0 siblings, 0 replies; 20+ messages in thread
From: sashiko-bot @ 2026-10-20 10:15 UTC (permalink / raw)
  To: Alexandre Courbot, John Hubbard, Danilo Krummrich, Alice Ryhl,
	David Airlie, Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, ojeda

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] gpu: nova-core: split kernel::time import block in cmdq.rs
--

commit 33237ce05e59a6a327898e33f27b96ad56cf0c7f
Author: Alexandre Courbot <acourbot@nvidia.com>

gpu: nova-core: gsp: cmdq: move the RPC code into a sub-module

This commit moves the RPC command and message types and methods from
drivers/gpu/nova-core/gsp/cmdq.rs into a new rpc sub-module.

> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index 54e87fd7158cd..c5a2f08cda94a 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs

[ ... ]

> @@ -28,8 +29,8 @@
>          },
>          Mutex, //
>      },
> +    time::Delta,
>      time::{
> -        Delta,
>          Instant,
>          Monotonic, //
>      },

[Severity: Low]
This isn't a bug, but in the use kernel declaration in
drivers/gpu/nova-core/gsp/cmdq.rs, Delta was moved out of the time::{ ... }
group into a separate time::Delta entry above time::{ Instant, Monotonic }.
Should Delta remain grouped inside the single time::{ ... } block to follow
the kernel vertical import style?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261020-cmdq-rpc-v5-0-dceeb8f360ec@nvidia.com?part=12

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 11/14] gpu: nova-core: gsp: cmdq: use consume_element to implement drain
  2026-10-20 10:08 ` [PATCH v5 11/14] gpu: nova-core: gsp: cmdq: use consume_element to implement drain Alexandre Courbot
@ 2026-10-20 10:18   ` sashiko-bot
  2026-10-20 14:27     ` Alexandre Courbot
  0 siblings, 1 reply; 20+ messages in thread
From: sashiko-bot @ 2026-10-20 10:18 UTC (permalink / raw)
  To: Alexandre Courbot, John Hubbard, Danilo Krummrich, Alice Ryhl,
	David Airlie, Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, ojeda

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] gpu: nova-core: unintended 1ms sleep in CmdqInner::drain() on empty queue
--

commit c3321749b8f493965661eed66b6e6bf889cebd66
Author: Alexandre Courbot <acourbot@nvidia.com>

gpu: nova-core: gsp: cmdq: use consume_element to implement drain

This commit refactors CmdqInner::drain() to use consume_element() and
adds a log() helper method to GspParsedMessage.

> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index 7db3d28e8902c..54e87fd7158cd 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs

[ ... ]

> @@ -1054,21 +1063,14 @@ fn await_msg<M: MessageFromGsp>(&mut self) -> Result<M>
>      /// `EIO` if a message fails framing or checksum validation, or if a message's page count
>      /// overflows a `u32`.
>      fn drain(&mut self) -> Result {
> -        while !self.gsp_mem.driver_read_area().0.is_empty() {
> -            // A message is available, so this returns without waiting.
> -            let msg = self.wait_for_msg(Delta::ZERO)?;
> -
> -            let pages =
> -                u32::try_from(msg.header.msg_length().div_ceil(GSP_PAGE_SIZE)).map_err(|_| {
> -                    dev_err!(&self.dev, "GSP drain: message length overflow\n");
> -                    EIO
> -                })?;
> -
> -            RpcMessage::parse(self.dev, msg)?.log(self.dev);
> -
> -            self.gsp_mem.advance_cpu_read_ptr(pages);
> +        loop {
> +            match self.consume_element(Delta::ZERO, |message| message.log(self.dev)) {
> +                Ok(()) => (),
> +                // No more message pending on the queue, drain is complete.
> +                Err(ETIMEDOUT) => return Ok(()),
> +                // Other errors are fatal, and stall the queue.
> +                Err(e) => return Err(e),
> +            }
>          }
> -
> -        Ok(())
>      }
>  }

[Severity: Medium]
Can removing the !self.gsp_mem.driver_read_area().0.is_empty() check
cause CmdqInner::drain() to sleep for 1 ms when the queue is empty?

Previously, CmdqInner::drain() only called wait_for_msg(Delta::ZERO)
after confirming a message was available in driver_read_area(). With the
new loop, consume_element(Delta::ZERO, ...) is called until the queue is
empty, which invokes wait_for_msg(Delta::ZERO):

drivers/gpu/nova-core/gsp/cmdq.rs:CmdqInner::wait_for_msg()
        let (slice_1, slice_2) = read_poll_timeout(
            || Ok(self.gsp_mem.driver_read_area()),
            |driver_area| !driver_area.0.is_empty(),
            Delta::from_millis(1),
            timeout,
        )

In rust/kernel/io/poll.rs:read_poll_timeout(), when the condition is
false, the timeout check uses a strict greater-than comparison:

        if start.elapsed() > timeout_delta {
            // Unlike the C version, we immediately return.
            // We have just called `op()` so we don't need to call it again.
            return Err(ETIMEDOUT);
        }

        if !sleep_delta.is_zero() {
            fsleep(sleep_delta);
        }

Because op() only reads two u32 indices from coherent memory in RAM, if
the monotonic clock has not advanced by at least 1 ns between
Instant::now() and start.elapsed() (for instance on architectures with
coarser timer resolution or under virtualization), start.elapsed() is
Delta::ZERO. Since Delta::ZERO > Delta::ZERO is false,
read_poll_timeout() falls through to fsleep(Delta::from_millis(1)) while
holding the Cmdq mutex (in paths like GspInterrupt::handle_threaded() ->
Cmdq::drain() and Gpu::new() -> Cmdq::drain()).

Would it make sense to keep the while
!self.gsp_mem.driver_read_area().0.is_empty() loop condition around
self.consume_element(Delta::ZERO, |message| message.log(self.dev))?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261020-cmdq-rpc-v5-0-dceeb8f360ec@nvidia.com?part=11

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 01/14] gpu: nova-core: gsp: cmdq: remove stale comment
  2026-10-20 10:08 ` [PATCH v5 01/14] gpu: nova-core: gsp: cmdq: remove stale comment Alexandre Courbot
@ 2026-10-20 11:43   ` Eliot Courtney
  0 siblings, 0 replies; 20+ messages in thread
From: Eliot Courtney @ 2026-10-20 11:43 UTC (permalink / raw)
  To: Alexandre Courbot, John Hubbard, Danilo Krummrich, Alice Ryhl,
	David Airlie, Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, linux-kernel, rust-for-linux, dri-devel

On Tue Oct 20, 2026 at 7:08 PM JST, Alexandre Courbot wrote:
> The message constructor is no longer run by `wait_for_msg`.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---

Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 02/14] gpu: nova-core: gsp: cmdq: simplify framing error description
  2026-10-20 10:08 ` [PATCH v5 02/14] gpu: nova-core: gsp: cmdq: simplify framing error description Alexandre Courbot
@ 2026-10-20 11:43   ` Eliot Courtney
  0 siblings, 0 replies; 20+ messages in thread
From: Eliot Courtney @ 2026-10-20 11:43 UTC (permalink / raw)
  To: Alexandre Courbot, John Hubbard, Danilo Krummrich, Alice Ryhl,
	David Airlie, Simona Vetter, Benno Lossin, Gary Guo
  Cc: Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, linux-kernel, rust-for-linux, dri-devel

On Tue Oct 20, 2026 at 7:08 PM JST, Alexandre Courbot wrote:
> Use simpler and more precise language to explain the conditions in which
> `EIO` is returned.
>
> Signed-off-by: Alexandre Courbot <acourbot@nvidia.com>
> ---

Reviewed-by: Eliot Courtney <ecourtney@nvidia.com>

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v5 11/14] gpu: nova-core: gsp: cmdq: use consume_element to implement drain
  2026-10-20 10:18   ` sashiko-bot
@ 2026-10-20 14:27     ` Alexandre Courbot
  0 siblings, 0 replies; 20+ messages in thread
From: Alexandre Courbot @ 2026-10-20 14:27 UTC (permalink / raw)
  To: sashiko-bot
  Cc: sashiko-reviews, John Hubbard, Danilo Krummrich, Alice Ryhl,
	David Airlie, Simona Vetter, Benno Lossin, Gary Guo,
	Alistair Popple, Timur Tabi, Eliot Courtney, Zhi Wang, nova-gpu,
	dri-devel, ojeda

On Tue Oct 20, 2026 at 7:18 PM JST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] gpu: nova-core: unintended 1ms sleep in CmdqInner::drain() on empty queue
> --
>
> commit c3321749b8f493965661eed66b6e6bf889cebd66
> Author: Alexandre Courbot <acourbot@nvidia.com>
>
> gpu: nova-core: gsp: cmdq: use consume_element to implement drain
>
> This commit refactors CmdqInner::drain() to use consume_element() and
> adds a log() helper method to GspParsedMessage.
>
>> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
>> index 7db3d28e8902c..54e87fd7158cd 100644
>> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
>> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
>
> [ ... ]
>
>> @@ -1054,21 +1063,14 @@ fn await_msg<M: MessageFromGsp>(&mut self) -> Result<M>
>>      /// `EIO` if a message fails framing or checksum validation, or if a message's page count
>>      /// overflows a `u32`.
>>      fn drain(&mut self) -> Result {
>> -        while !self.gsp_mem.driver_read_area().0.is_empty() {
>> -            // A message is available, so this returns without waiting.
>> -            let msg = self.wait_for_msg(Delta::ZERO)?;
>> -
>> -            let pages =
>> -                u32::try_from(msg.header.msg_length().div_ceil(GSP_PAGE_SIZE)).map_err(|_| {
>> -                    dev_err!(&self.dev, "GSP drain: message length overflow\n");
>> -                    EIO
>> -                })?;
>> -
>> -            RpcMessage::parse(self.dev, msg)?.log(self.dev);
>> -
>> -            self.gsp_mem.advance_cpu_read_ptr(pages);
>> +        loop {
>> +            match self.consume_element(Delta::ZERO, |message| message.log(self.dev)) {
>> +                Ok(()) => (),
>> +                // No more message pending on the queue, drain is complete.
>> +                Err(ETIMEDOUT) => return Ok(()),
>> +                // Other errors are fatal, and stall the queue.
>> +                Err(e) => return Err(e),
>> +            }
>>          }
>> -
>> -        Ok(())
>>      }
>>  }
>
> [Severity: Medium]
> Can removing the !self.gsp_mem.driver_read_area().0.is_empty() check
> cause CmdqInner::drain() to sleep for 1 ms when the queue is empty?
>
> Previously, CmdqInner::drain() only called wait_for_msg(Delta::ZERO)
> after confirming a message was available in driver_read_area(). With the
> new loop, consume_element(Delta::ZERO, ...) is called until the queue is
> empty, which invokes wait_for_msg(Delta::ZERO):
>
> drivers/gpu/nova-core/gsp/cmdq.rs:CmdqInner::wait_for_msg()
>         let (slice_1, slice_2) = read_poll_timeout(
>             || Ok(self.gsp_mem.driver_read_area()),
>             |driver_area| !driver_area.0.is_empty(),
>             Delta::from_millis(1),
>             timeout,
>         )
>
> In rust/kernel/io/poll.rs:read_poll_timeout(), when the condition is
> false, the timeout check uses a strict greater-than comparison:
>
>         if start.elapsed() > timeout_delta {
>             // Unlike the C version, we immediately return.
>             // We have just called `op()` so we don't need to call it again.
>             return Err(ETIMEDOUT);
>         }
>
>         if !sleep_delta.is_zero() {
>             fsleep(sleep_delta);
>         }
>
> Because op() only reads two u32 indices from coherent memory in RAM, if
> the monotonic clock has not advanced by at least 1 ns between
> Instant::now() and start.elapsed() (for instance on architectures with
> coarser timer resolution or under virtualization), start.elapsed() is
> Delta::ZERO. Since Delta::ZERO > Delta::ZERO is false,
> read_poll_timeout() falls through to fsleep(Delta::from_millis(1)) while
> holding the Cmdq mutex (in paths like GspInterrupt::handle_threaded() ->
> Cmdq::drain() and Gpu::new() -> Cmdq::drain()).
>
> Would it make sense to keep the while
> !self.gsp_mem.driver_read_area().0.is_empty() loop condition around
> self.consume_element(Delta::ZERO, |message| message.log(self.dev))?

Mmm, that's an interesting one, in that I am not sure there are any
concrete implications but I'd like to fix it anyway.

I was thinking that maybe we should add a special-case to
`read_poll_timeout` so it returns unconditionally after checking the
condition once if `timeout_delta == Delta::ZERO`, but doing so would
make the `sleep_delta` argument moot so I guess that's not adequate (and
the current behavior is technically correct). So I think I'll
special-case nova-core's `wait_for_msg` instead.

^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2026-10-20 14:27 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-20 10:08 [PATCH v5 00/14] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types Alexandre Courbot
2026-10-20 10:08 ` [PATCH v5 01/14] gpu: nova-core: gsp: cmdq: remove stale comment Alexandre Courbot
2026-10-20 11:43   ` Eliot Courtney
2026-10-20 10:08 ` [PATCH v5 02/14] gpu: nova-core: gsp: cmdq: simplify framing error description Alexandre Courbot
2026-10-20 11:43   ` Eliot Courtney
2026-10-20 10:08 ` [PATCH v5 03/14] gpu: nova-core: gsp: cmdq: validate checksum earlier on receive Alexandre Courbot
2026-10-20 10:08 ` [PATCH v5 04/14] gpu: nova-core: gsp: introduce and use proper RpcMessageHeader type Alexandre Courbot
2026-10-20 10:08 ` [PATCH v5 05/14] gpu: nova-core: gsp: cmdq: group the RPC-specific part of send_single_command Alexandre Courbot
2026-10-20 10:08 ` [PATCH v5 06/14] gpu: nova-core: gsp: cmdq: split the transport part of the send path Alexandre Courbot
2026-10-20 10:08 ` [PATCH v5 07/14] gpu: nova-core: gsp: cmdq: split RPC parsing part of the receive path Alexandre Courbot
2026-10-20 10:08 ` [PATCH v5 08/14] gpu: nova-core: gsp: cmdq: move RPC message logging to RpcMessage Alexandre Courbot
2026-10-20 10:08 ` [PATCH v5 09/14] gpu: nova-core: gsp: cmdq: split the transport part of the receive path Alexandre Courbot
2026-10-20 10:08 ` [PATCH v5 10/14] gpu: nova-core: gsp: cmdq: extract await_msg_with from await_msg Alexandre Courbot
2026-10-20 10:08 ` [PATCH v5 11/14] gpu: nova-core: gsp: cmdq: use consume_element to implement drain Alexandre Courbot
2026-10-20 10:18   ` sashiko-bot
2026-10-20 14:27     ` Alexandre Courbot
2026-10-20 10:08 ` [PATCH v5 12/14] gpu: nova-core: gsp: cmdq: move the RPC code into a sub-module Alexandre Courbot
2026-10-20 10:15   ` sashiko-bot
2026-10-20 10:08 ` [PATCH v5 13/14] gpu: nova-core: gsp: move the RPC commands " Alexandre Courbot
2026-10-20 10:08 ` [PATCH v5 14/14] gpu: nova-core: gsp: add `rpc` to RPC message send/receive methods Alexandre Courbot

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox