Re: [PATCH v4 06/10] gpu: nova-core: gsp: cmdq: move RPC message logging to RpcMessage

[Eliot Courtney]

On Tue Oct 20, 2026 at 7:13 PM JST, Alexandre Courbot wrote:
> 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.

I took a look and it's looking really nice now, so well worth it IMO.