Rust for Linux List
 help / color / mirror / Atom feed
From: "Alexandre Courbot" <acourbot@nvidia.com>
To: "Eliot Courtney" <ecourtney@nvidia.com>
Cc: "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>,
	"Alistair Popple" <apopple@nvidia.com>,
	"Timur Tabi" <ttabi@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>,
	"dri-devel" <dri-devel-bounces@lists.freedesktop.org>
Subject: Re: [PATCH v3 5/9] gpu: nova-core: gsp: cmdq: split RPC parsing part of the receive path
Date: Fri, 09 Oct 2026 15:36:38 +0900	[thread overview]
Message-ID: <DM03LOKOYPFQ.3UXED0I92H8Y6@nvidia.com> (raw)
In-Reply-To: <DLT7CLF1AJVA.3BNA0XWZOZB99@nvidia.com>

On Thu Oct 1, 2026 at 1:04 PM JST, Eliot Courtney wrote:
> On Wed Sep 30, 2026 at 11:55 PM JST, Alexandre Courbot wrote:
>> 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>
>> ---
>>  drivers/gpu/nova-core/gsp/cmdq.rs | 37 +++++++++++++++++++++++++++++++++----
>>  1 file changed, 33 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/gsp/cmdq.rs b/drivers/gpu/nova-core/gsp/cmdq.rs
>> index d8a7716fc500..b8a9e02b76fe 100644
>> --- a/drivers/gpu/nova-core/gsp/cmdq.rs
>> +++ b/drivers/gpu/nova-core/gsp/cmdq.rs
>> @@ -553,6 +553,17 @@ struct GspMessage<'a> {
>>      contents: (&'a [u8], &'a [u8]),
>>  }
>>  
>> +/// A RPC message ready to be processed from the message queue.
>> +///
>> +/// This is the type returned by [`CmdqInner::parse_rpc_message`].
>> +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 zero unless the message loops
>> +    // over the message queue.
>> +    contents: (&'a [u8], &'a [u8]),
>> +}
>> +
>>  /// GSP command queue.
>>  ///
>>  /// Provides the ability to send commands and receive messages from the GSP using a shared memory
>> @@ -794,7 +805,7 @@ fn send_command<M>(&mut self, command: M) -> Result
>>      ///   message queue.
>>      ///
>>      /// Error codes returned by the message constructor are propagated as-is.
>> -    fn wait_for_msg(&self, timeout: Delta) -> Result<GspMessage<'_>> {
>> +    fn wait_for_msg(&self, timeout: Delta) -> Result<RpcMessage<'_>> {
>
> This gets changed back to `GspMessage` return value in the next patch,
> and also introduces a bug [1]. What about just keeping return
> `GspMessage` here? You can move RpcMessage to the next patch.
>
> [1]: https://lore.kernel.org/all/DLT4O89RYP29.UKGBMIJD8ZSC@nvidia.com/

Indeed, we could organize the change a bit more smoothly. I've tried to
reorder and hopefully the next revision will flow better.

And yes, it is undeniable that having both length methods called
`length` is confusing and has caused this bug, so I've given them
different names.

  reply	other threads:[~2026-10-09  6:36 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-09-30 14:55 [PATCH v3 0/9] gpu: nova-core: gsp: prepare the command queue for r000 dual-message types Alexandre Courbot
2026-09-30 14:55 ` [PATCH v3 1/9] gpu: nova-core: gsp: cmdq: validate checksum earlier on receive Alexandre Courbot
2026-09-30 14:55 ` [PATCH v3 2/9] gpu: nova-core: gsp: introduce and use proper RpcMessageHeader type Alexandre Courbot
2026-09-30 14:55 ` [PATCH v3 3/9] gpu: nova-core: gsp: cmdq: group the RPC-specific part of send_single_command Alexandre Courbot
2026-09-30 14:55 ` [PATCH v3 4/9] gpu: nova-core: gsp: cmdq: split the transport part of the send path Alexandre Courbot
2026-10-01  2:21   ` Eliot Courtney
2026-10-09  2:27     ` Alexandre Courbot
2026-09-30 14:55 ` [PATCH v3 5/9] gpu: nova-core: gsp: cmdq: split RPC parsing part of the receive path Alexandre Courbot
2026-10-01  4:04   ` Eliot Courtney
2026-10-09  6:36     ` Alexandre Courbot [this message]
2026-09-30 14:55 ` [PATCH v3 6/9] gpu: nova-core: gsp: cmdq: split the transport " Alexandre Courbot
2026-10-01  4:48   ` Eliot Courtney
2026-09-30 14:55 ` [PATCH v3 7/9] gpu: nova-core: gsp: cmdq: move the RPC code into a sub-module Alexandre Courbot
2026-10-01  5:12   ` Eliot Courtney
2026-10-09  2:16     ` Alexandre Courbot
2026-10-09  2:19     ` Alexandre Courbot
2026-10-09  6:46       ` Alexandre Courbot
2026-09-30 14:55 ` [PATCH v3 8/9] gpu: nova-core: gsp: move the RPC commands " Alexandre Courbot
2026-09-30 14:55 ` [PATCH v3 9/9] gpu: nova-core: gsp: add `rpc` to RPC message send/receive methods Alexandre Courbot

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=DM03LOKOYPFQ.3UXED0I92H8Y6@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-bounces@lists.freedesktop.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