Re: [PATCH 03/13] gpu: nova-core: fsp: try to enforce exclusive access to FSP channel

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Polling queue pointers to attempt mutual exclusion between the kernel and GSP creates a Time-of-Check to Time-of-Use (TOCTOU) race condition and incorrectly identifies active processing windows as idle.

Pre-existing issues:
- [High] `send_msg` does not validate that the packet length is within the FSP EMEM channel boundaries before writing.
- [High] `QUEUE_TAIL` is written before `QUEUE_HEAD` is reset, potentially exposing an invalid queue state to the FSP.
--

commit cbbcd901d8b26aba0c2ec257efff6284b3bf260b
Author: Eliot Courtney <ecourtney@nvidia.com>

gpu: nova-core: fsp: try to enforce exclusive access to FSP channel
    
Currently, send_msg assumes that the channel to FSP is free to write
into. But, it might not be. Both the kernel driver and GSP communicate
with FSP. The way they should attempt to keep exclusive access to this
channel to FSP is by making sure they don't try to start writing if
there's pending data until the full round trip has finished.

> diff --git a/drivers/gpu/nova-core/falcon/fsp.rs b/drivers/gpu/nova-core/falcon/fsp.rs
> index 21eaa8e261ce6..cdb476894e1ad 100644
> --- a/drivers/gpu/nova-core/falcon/fsp.rs
> +++ b/drivers/gpu/nova-core/falcon/fsp.rs

[ ... ]

> @@ -133,6 +153,9 @@ pub(crate) fn send_msg(&mut self, bar: Bar0<'_>, packet: &[u8]) -> Result {
>              return Err(EINVAL);
>          }
>  
> +        // Try to make sure we have exclusive access to the FSP at this point.
> +        self.wait_until_ready(bar)?;
> +
>          self.write_emem(bar, packet)?;

[Severity: High]
Does this introduce a time-of-check to time-of-use (TOCTOU) race condition?

The kernel and the GSP run asynchronously.

This lockless software polling approach does not appear to provide strict
mutual exclusion between the two processors.

Could both evaluate the idle condition (qhead == qtail && mhead == mtail)
simultaneously in wait_until_ready() and subsequently write to the shared
FSP EMEM channel at the same time, leading to hardware message corruption?

[Severity: High]
This isn't a bug introduced by this patch, but does this write operation lack
a bounds check against the FSP EMEM channel capacity?

There does not appear to be a check ensuring that packet.len() is within
FSP_EMEM_CHANNEL_0_SIZE (1024 bytes).

If a caller submits a larger packet, could write_emem() overwrite adjacent
hardware memory and potentially corrupt secure boot data or other channel
states?

>  
>          // Update queue pointers. TAIL points at the last DWORD written.
>          let tail_offset = u32::try_from(packet.len() - 4).map_err(|_| EINVAL)?;
>          bar.write(
>              Array::at(0),
>              regs::NV_PFSP_QUEUE_TAIL::zeroed().with_address(tail_offset),
>          );
>          bar.write(
>              Array::at(0),
>              regs::NV_PFSP_QUEUE_HEAD::zeroed().with_address(0),
>          );

[Severity: High]
This is a pre-existing issue, but does writing QUEUE_TAIL before resetting
QUEUE_HEAD expose an invalid queue state to the hardware?

If QUEUE_HEAD contains a non-zero value from a previous transaction, writing
QUEUE_TAIL first could cause the hardware to briefly observe a state where
TAIL < HEAD.

Because the queue is not circular, could this trigger an immediate hardware
processing error before the kernel updates QUEUE_HEAD to 0?

>  
>          Ok(())
>      }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615-blackwell-fixes-v1-0-f2853e49ff7d@nvidia.com?part=3