Re: [PATCH v4 06/10] gpu: nova-core: gsp: cmdq: move RPC message logging to RpcMessage
From: Alexandre Courbot
Date: Tue Oct 20 2026 - 06:17:56 EST
On Thu Oct 15, 2026 at 10:38 PM JST, Alexandre Courbot wrote:
> On Wed Oct 14, 2026 at 10:35 AM JST, Eliot Courtney wrote:
>> On Fri Oct 9, 2026 at 8:54 PM JST, Alexandre Courbot wrote:
>>> 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@xxxxxxxxxx>
>>> ---
>> [...]
>>> - let function = rpc_msg.header.function();
>>> - let seq = rpc_msg.header.sequence();
>>> +
>>> + RpcMessage::parse(self.dev, msg)?.log(self.dev);
>>
>> nit: in core/interrupts.rst we say:
>>
>> ``` A drain fails when a message's framing or checksum is bad (see
>> "Draining the GSP-to-CPU queue"). The message stays at the queue head,
>> so every later drain would fail the same way, and the IRQ thread
>> disables vector 155 and logs the failure, which leaves the queue
>> unserviced until the device is reset. ```
>>
>> So `parse()?` returning early on failure here is intentional, but I
>> reckon it is worth a comment, especially since the behaviour is
>> different to the semantics for consuming elements regularly (still
>> consumes and progresses for failed elements). Especially since you
>> wouldn't expect this behaviour to differ for a method which explicitly
>> "consumes every message the queue holds".
>
> Sure, I'll add a comment.
The inconsistency in dropping behavior between `drain` and `await_msg`
really bothered me, so I've spent more time than I should have reworking
that to harmonize the two. As a result v5 has changed a bit more than I
wanted, but hopefully it's for the better.