mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "John Hubbard" <jhubbard@nvidia.com>,
	"Danilo Krummrich" <dakr@kernel.org>,
	"Alice Ryhl" <aliceryhl@google.com>,
	"David Airlie" <airlied@gmail.com>,
	"Simona Vetter" <simona@ffwll.ch>,
	"Benno Lossin" <lossin@kernel.org>, "Gary Guo" <gary@garyguo.net>
Cc: "Alistair Popple" <apopple@nvidia.com>,
	"Timur Tabi" <ttabi@nvidia.com>,
	"Eliot Courtney" <ecourtney@nvidia.com>,
	"Zhi Wang" <zhiw@nvidia.com>, <nova-gpu@lists.linux.dev>,
	<dri-devel@lists.freedesktop.org>, <linux-kernel@vger.kernel.org>,
	<rust-for-linux@vger.kernel.org>,
	"Alexandre Courbot" <acourbot@nvidia.com>
Subject: Re: [PATCH v2 6/9] gpu: nova-core: gsp: cmdq: split the transport part of the receive path
Date: Mon, 28 Sep 2026 12:19:17 +0900	[thread overview]
Message-ID: <DLQMIL9HQRBR.3N6WCVYOAM5H1@nvidia.com> (raw)
In-Reply-To: <20260927-cmdq-rpc-v2-6-c3f66ae73be4@nvidia.com>

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@nvidia.com>
> ---
>  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.

  reply	other threads:[~2026-09-28  3:19 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-27 13:46 [PATCH v2 0/9] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types Alexandre Courbot
2026-09-27 13:46 ` [PATCH v2 1/9] gpu: nova-core: gsp: cmdq: validate checksum earlier on receive Alexandre Courbot
2026-09-28  4:25   ` Eliot Courtney
2026-09-28  6:24     ` Alexandre Courbot
2026-09-27 13:46 ` [PATCH v2 2/9] gpu: nova-core: gsp: introduce and use proper RpcMessageHeader type Alexandre Courbot
2026-09-28  4:43   ` Eliot Courtney
2026-09-28  6:22     ` Alexandre Courbot
2026-09-27 13:46 ` [PATCH v2 3/9] gpu: nova-core: gsp: cmdq: group the RPC-specific part of send_single_command Alexandre Courbot
2026-09-28  4:52   ` Eliot Courtney
2026-09-27 13:46 ` [PATCH v2 4/9] gpu: nova-core: gsp: cmdq: split the transport part of the send path Alexandre Courbot
2026-09-28  5:16   ` Eliot Courtney
2026-09-28  6:19     ` Alexandre Courbot
2026-09-28  6:40       ` Eliot Courtney
2026-09-28 11:35         ` Alexandre Courbot
2026-09-27 13:46 ` [PATCH v2 5/9] gpu: nova-core: gsp: cmdq: move the RPC send code into a sub-module Alexandre Courbot
2026-09-27 13:46 ` [PATCH v2 6/9] gpu: nova-core: gsp: cmdq: split the transport part of the receive path Alexandre Courbot
2026-09-28  3:19   ` Alexandre Courbot [this message]
2026-09-27 13:46 ` [PATCH v2 7/9] gpu: nova-core: gsp: cmdq: move the RPC receive code into a sub-module Alexandre Courbot
2026-09-27 13:46 ` [PATCH v2 8/9] gpu: nova-core: gsp: move the RPC commands " Alexandre Courbot
2026-09-28  5:25   ` Eliot Courtney
2026-09-28  9:20   ` Zhi Wang
2026-09-28 11:40     ` Alexandre Courbot
2026-09-28 15:11       ` Zhi Wang
2026-09-27 13:46 ` [PATCH v2 9/9] gpu: nova-core: gsp: add `rpc` to RPC message send/receive methods Alexandre Courbot
2026-09-28  5:32   ` Eliot Courtney

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=DLQMIL9HQRBR.3N6WCVYOAM5H1@nvidia.com \
    --to=acourbot@nvidia.com \
    --cc=airlied@gmail.com \
    --cc=aliceryhl@google.com \
    --cc=apopple@nvidia.com \
    --cc=dakr@kernel.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=ecourtney@nvidia.com \
    --cc=gary@garyguo.net \
    --cc=jhubbard@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lossin@kernel.org \
    --cc=nova-gpu@lists.linux.dev \
    --cc=rust-for-linux@vger.kernel.org \
    --cc=simona@ffwll.ch \
    --cc=ttabi@nvidia.com \
    --cc=zhiw@nvidia.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®