Re: [PATCH v12 14/22] gpu: nova-core: Hopper/Blackwell: add FSP falcon EMEM operations

[Alexandre Courbot]

On Tue Jun 2, 2026 at 8:42 PM JST, Eliot Courtney wrote:
> On Tue Jun 2, 2026 at 12:21 PM JST, John Hubbard wrote:
>> Add external memory (EMEM) read/write operations to the GPU's FSP falcon
>> engine. These operations use Falcon PIO (Programmed I/O) to communicate
>> with the FSP through indirect memory access.
>>
>> Signed-off-by: John Hubbard <jhubbard@xxxxxxxxxx>
>> ---
>
>
>> +impl Falcon<Fsp> {
>> +    /// Writes `data` to FSP external memory at byte `offset`.
>> +    ///
>> +    /// `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);
>> +        }
>> +
>> +        let mut emem = Emem::new(bar);
>> +        emem.begin_write(offset as usize)?;
>> +        for chunk in data.chunks_exact(4) {
>> +            emem.write_next(u32::from_le_bytes([chunk[0], chunk[1], chunk[2], chunk[3]]));
>> +        }
>> +
>> +        Ok(())
>> +    }
>> +
>> +    /// Reads FSP external memory at byte `offset` into `data`.
>> +    ///
>> +    /// `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);
>> +        }
>> +
>> +        let mut emem = Emem::new(bar);
>> +        emem.begin_read(offset as usize)?;
>> +        for chunk in data.chunks_exact_mut(4) {
>> +            chunk.copy_from_slice(&emem.read_next().to_le_bytes());
>> +        }
>> +
>> +        Ok(())
>> +    }
>> +}
>
> Both `write_emem` and `read_emem` are only ever called with `offset` as
> zero. I checked openrm, and it looks like there aren't ever writes or
> reads that don't start at zero. So we could simplify the code by
> removing `offset` and starting from zero if we will never use a non-zero
> offset (given we have auto-increment). This also lets us remove
> `EMEM_MAX_SIZE` and some `Result`s.

IIUC the way FSP communication works is that EMEM is (in theory) divided
into 8 channels. Only one message or reply is ever processed at a given
time (i.e. this is not a ring buffer), so the only values valid for
`offset` are that start address of each channels.

So while we might need to have some sort of pseudo-offset in the future,
a freeform one is definitely not adequate for EMEM and for the time
being we are only working with channel 0 anyway. Thus I agree that it
makes sense to remove it altogether for now.