Nouveau - Dec 2025 - [PATCH v2 12/13] gpu: nova-core: add PIO support for loading firmware images

On 12/2/25 3:20 PM, Joel Fernandes wrote:
> On 12/2/2025 5:51 PM, Timur Tabi wrote:
>> On Tue, 2025-12-02 at 16:23 -0500, Joel Fernandes wrote:
...
>>> If img.size is not a multiple of 4 bytes, this can panic right?
Rust for Linux avoids .unwrap() for similar reasons that we prefer WARN*()
over BUG*() these days, on the C side: avoid killing the machine if at
all possible. Because it changes a routine bug into a harder-to-work-with
bug.

...
>> Wouldn't it be a run-time constraint anyway? With the exception of
the BootloaderDmemDescV2 write,
>> all of the calls to pio_wr_bytes() have lengths only known at runtime.
> 
> I am not sure but I think rust code is expected to not panic and handle
> situations gracefully even in the face of runtime constraints being
violated,
> you could argue that the image length being violated is UB but I don't
think
> that'd be enough to justify the unwrap(). But perhaps someone from the
rust core
Agreed. This situation should return an -EINVAL Result, approximately.

In fact, I just finished looking through my Hopper/Blackwell PIO code, which
also needs 4-byte alignment, and concluded that returning -EINVAL for misaligned
data seems to be the appropriate way to handle things.

> team can chime in about that because I also have that question. Can a
"FW image
> corruption" type of scenarios be considered something that safe rust
code not
> need to worry about since it falls under the UB umbrella (similar to memory
> corruption)?
> 
I'm not the core Rust team, but I will chime in anyway: misaligned or
corrupted firmware should not *directly* cause a panic. We should detect
and error out.

thanks,
-- 
John Hubbard

On Tue, 2025-12-02 at 15:40 -0800, John Hubbard wrote:
> In fact, I just finished looking through my Hopper/Blackwell PIO code,
which
> also needs 4-byte alignment, and concluded that returning -EINVAL for
misaligned
> data seems to be the appropriate way to handle things.
I've added this for v3:

        // Rejecting misaligned images here allows us to avoid checking
        // inside the loops.
        if img.len() % 4 != 0 {
            return Err(EINVAL);
        }

And I manually create the &[u8; 4] now:

            for word in block.chunks_exact(4) {
                let w = [word[0], word[1], word[2], word[3]];
                regs::NV_PFALCON_FALCON_IMEMD::default()
                    .set_data(u32::from_le_bytes(w))
                    .write(bar, &E::ID, port);

word[3] will always exist because of chunks_exact(4).