Re: [PATCH v3 5/9] gpu: nova-core: gsp: cmdq: split RPC parsing part of the receive path

[Alexandre Courbot]

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@xxxxxxxxxx>
>> ---
>>  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@xxxxxxxxxx/

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.