Re: [PATCH v2 6/9] gpu: nova-core: gsp: cmdq: split the transport part of the receive path

[Alexandre Courbot]

On Sun Sep 27, 2026 at 10:46 PM JST, Alexandre Courbot wrote:
> `wait_for_msg` mixes two layers: the transport layer which polls the
> queue, extracts the element header and validates the checksum, and the
> RPC layer which reads the RPC header and trims the payload slices to the
> length advertised by the RPC header.
>
> Move the transport layer into `wait_for_element`, and introduce
> `consume_element`, a transport-level method which runs a closure on the
> next element before advancing the CPU read pointer past it, and
> `parse_rpc_message`, which validates the RPC layer. This sets things up
> for moving the RPC code into its own module, leaving the transport
>   agnostic of the message type.
>
> Signed-off-by: Alexandre Courbot <acourbot@xxxxxxxxxx>
> ---
>  drivers/gpu/nova-core/gsp/cmdq.rs | 91 ++++++++++++++++++++++++++++-----------
>  1 file changed, 65 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
> index 640afe2e29cb..169ef0865339 100644
> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
> @@ -411,7 +411,7 @@ struct GspCommand<'a> {
>  
>  /// A message ready to be processed from the message queue.
>  ///
> -/// This is the type returned by [`CmdqInner::wait_for_msg`].
> +/// This is the type returned by [`CmdqInner::wait_for_element`].
>  struct GspMessage<'a> {
>      // Reference to the header of the message.
>      header: &'a GspMsgElement,
> @@ -566,7 +566,7 @@ fn send_command_element(
>          Ok(())
>      }
>  
> -    /// Wait for a message to become available on the message queue.
> +    /// Wait for the next element to become available on the message queue.
>      ///
>      /// This works purely at the transport layer and does not interpret or validate the message
>      /// beyond the advertised length in its [`GspMsgElement`].
> @@ -584,7 +584,7 @@ fn send_command_element(
>      ///   message queue.
>      ///
>      /// Error codes returned by the message constructor are propagated as-is.
> -    fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
> +    fn wait_for_element(&self, timeout: Delta) -> Result<GspMessage<'_>> {
>          // Wait for a message to arrive from the GSP.
>          let (slice_1, slice_2) = read_poll_timeout(
>              || Ok(self.gsp_mem.driver_read_area()),
> @@ -606,18 +606,65 @@ fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
>              return Err(EIO);
>          }
>  
> +        Ok(GspMessage {
> +            header,
> +            contents: (slice_1, slice_2),
> +        })
> +    }
> +
> +    /// Wait for the next element on the message queue, pass it to `process_element`, and advances
> +    /// the read pointer past it.
> +    ///
> +    /// The read pointer advances regardless of whether `process_element` succeeds or not.
> +    ///
> +    /// # Errors
> +    ///
> +    /// Errors from [`Self::wait_for_element`] and from `process_element` are propagated as-is.
> +    fn consume_element<R>(
> +        &mut self,
> +        timeout: Delta,
> +        process_element: impl FnOnce(GspMessage<'_>) -> Result<R>,
> +    ) -> Result<R> {
> +        let (elem_count, result) = {
> +            let message = self.wait_for_element(timeout)?;
> +
> +            (
> +                u32::try_from(message.header.length().div_ceil(GSP_PAGE_SIZE))?,
> +                process_element(message),
> +            )
> +        };
> +
> +        self.gsp_mem.advance_cpu_read_ptr(elem_count);
> +
> +        result
> +    }
> +
> +    /// Validate the RPC layer of `element` and trim its contents down to the RPC payload.
> +    ///
> +    /// # Errors
> +    ///
> +    /// - `EIO` if the element is shorter than the payload length advertised by the RPC header.
> +    fn parse_rpc_message<'a>(
> +        dev: &device::Device,
> +        element: GspMessage<'a>,
> +    ) -> Result<GspMessage<'a>> {

Since this trims the payload, we should return a different type
(`RpcMessage`?) with a field referring to the RPC header, otherwise we
risk getting confused as to which headers are in the payload or not.

Not re-sending just for that, but wanted to flag this for v3.