Re: [PATCH v4 10/17] gpu: nova-core: stop re-parsing a bad GSP message

From: Alexandre Courbot

Date: Mon Sep 21 2026 - 02:33:00 EST


On Sat Sep 12, 2026 at 1:43 PM JST, John Hubbard wrote:
> A GSP message carries its length inside the checksummed region. Once the
> framing or the checksum fails, there is no trustworthy length with which
> to skip the message.
>
> Two failures left a bad message at the queue head. A framing or checksum
> failure returned without advancing the read pointer, so every later
> receive parsed the same message again. A validly framed message whose
> typed payload failed to decode returned early and did the same.
>
> Poison the queue on a framing or checksum failure: log what was
> inconsistent and fail every later receive, so the bad message is parsed
> once and recovery takes a device reset. Advance the read pointer past a
> validly framed message whether or not its payload decodes, and warn when
> the payload is shorter than the type it decodes into.
>
> Assisted-by: LLM
> Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>

The idea is nice, but the implementation is a bit too C-like and even
with this patch we are missing cases: for instance `send_command` still
operates , which is technically not an error but introduces a timeout
before the receiving end fails.

Basically the poison flag is here to prevent access to some
functionality if it is set. This is conceptually very similar to the
`Mutex` and should operate similarly, i.e. by wrapping the data it
protects into a dedicated type, and providing references to the wrapped
data only if we are not poisoned.

Here the data we want to protect is `gsp_mem` in `CmdqInner`, so it
would be declared like:

gsp_mem: Poisonable<DmaGspMem<'a>>,

And now every access to the inner `gsp_mem` is conditioned to it not
being poisoned, and we can't omit checking the flag before accessing it.

Eventually I think `Poisonable` could even be useful in the `kernel` crate.

That's quite a rework though, so I'd suggest to remove this patch from
this series and re-consider it after it is merged, as it is not really
related to IRQ handling and is more a cmdq hardening effort.