Re: [PATCH v12 15/22] gpu: nova-core: Hopper/Blackwell: add FSP message infrastructure

[Eliot Courtney]

On Wed Jun 3, 2026 at 10:34 AM JST, Alexandre Courbot wrote:
> On Tue Jun 2, 2026 at 9:21 PM JST, Eliot Courtney wrote:
>> On Tue Jun 2, 2026 at 12:21 PM JST, John Hubbard wrote:
>>> FSP communication uses a pair of non-circular queues in the FSP
>>> falcon's EMEM, one for messages from the driver to FSP and one for
>>> replies, with the driver polling for response data. Add the queue
>>> registers and the low-level helpers used by the higher-level FSP
>>> message layer.
>>>
>>> Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
>>> ---
>>>  drivers/gpu/nova-core/falcon/fsp.rs | 61 ++++++++++++++++++++++++++++-
>>>  drivers/gpu/nova-core/regs.rs       | 21 ++++++++++
>>>  2 files changed, 80 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/nova-core/falcon/fsp.rs b/drivers/gpu/nova-core/falcon/fsp.rs
>>> index 6b057d958115..0ec1c55213bc 100644
>>> --- a/drivers/gpu/nova-core/falcon/fsp.rs
>>> +++ b/drivers/gpu/nova-core/falcon/fsp.rs
>>> @@ -112,7 +112,6 @@ impl Falcon<Fsp> {
>>>      ///
>>>      /// `data` is interpreted as little-endian 32-bit words. Returns `EINVAL`
>>>      /// if `offset` or the `data` length is not 4-byte aligned.
>>> -    #[expect(dead_code)]
>>>      fn write_emem(&mut self, bar: &Bar0, offset: u32, data: &[u8]) -> Result {
>>>          if offset % 4 != 0 || data.len() % 4 != 0 {
>>>              return Err(EINVAL);
>>> @@ -131,7 +130,6 @@ fn write_emem(&mut self, bar: &Bar0, offset: u32, data: &[u8]) -> Result {
>>>      ///
>>>      /// `data` is stored as little-endian 32-bit words. Returns `EINVAL` if
>>>      /// `offset` or the `data` length is not 4-byte aligned.
>>> -    #[expect(dead_code)]
>>>      fn read_emem(&mut self, bar: &Bar0, offset: u32, data: &mut [u8]) -> Result {
>>>          if offset % 4 != 0 || data.len() % 4 != 0 {
>>>              return Err(EINVAL);
>>> @@ -145,4 +143,63 @@ fn read_emem(&mut self, bar: &Bar0, offset: u32, data: &mut [u8]) -> Result {
>>>  
>>>          Ok(())
>>>      }
>>> +
>>> +    /// Poll FSP for incoming data.
>>> +    ///
>>> +    /// Returns the size of available data in bytes, or 0 if no data is available.
>>> +    ///
>>> +    /// The FSP message queue is not circular. Pointers are reset to 0 after each
>>> +    /// message exchange, so `tail >= head` is always true when data is present.
>>> +    #[expect(dead_code)]
>>> +    pub(crate) fn poll_msgq(&self, bar: &Bar0) -> u32 {
>>> +        let head = bar.read(regs::NV_PFSP_MSGQ_HEAD).address();
>>> +        let tail = bar.read(regs::NV_PFSP_MSGQ_TAIL).address();
>>> +
>>> +        if head == tail {
>>> +            return 0;
>>> +        }
>>> +
>>> +        // TAIL points at last DWORD written, so add 4 to get total size
>>> +        tail.saturating_sub(head) + 4
>>> +    }
>>
>> In a later patch, `send_sync_fsp` polls this then calls `recv_msg`. But,
>> structurally it's possible to pass in any size to `recv_msg` and read
>> more than we are supposed to. What about having `recv_msg` do the
>> polling to get the size and return a KVec with the read out data,
>> instead of `send_sync_fsp`? `poll_msgq` could stay private and we can
>> make it public later if we need to.
>
> The issue I see with returning a `KVec` is that it imposes a dynamic
> allocation for every message. Granted, this is what the current code
> does, but now that we have this `&mut self` logic in place that
> guarantees exclusive access, we can also turn the receiving `KVec` into
> a member of `Fsp` and keep passing it as a mut reference to avoid that.

I don't have a strong opinion here, but is having a dynamic allocation
for every message an issue here? AFAICT, this is called once during
boot. But by having Falcon<Fsp> decide the allocation we make it
structurally impossible to provide a wrongly sized output buffer, and
remove the need for the caller to separately poll, even though all it
wants to do now is wait for the next message whatever size it is. What
do we gain by delegating the polling and allocation to the caller?

Anyway I don't really mind, I am just trying improve my understanding
of the conventions for how much we try to avoid allocation in the
kernel.