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

[Joel Fernandes]

On 12/1/2025 6:39 PM, Timur Tabi wrote:
>  
> +
> +    /// See nvkm_falcon_pio_wr - takes a byte array instead of a 
> FalconFirmware
> +    fn pio_wr_bytes(
> +        &self,
> +        bar: &Bar0,
> +        img: &[u8],
> +        mem_base: u16,
> +        target_mem: FalconMem,
> +        port: u8,
> +        tag: u16
> +    ) {
> +        let port = usize::from(port);
> +
> +        match target_mem {
> +            FalconMem::ImemSecure | FalconMem::ImemNonSecure => {
> +                regs::NV_PFALCON_FALCON_IMEMC::default()
> +                    .set_secure(target_mem == FalconMem::ImemSecure)
> +                    .set_aincw(true)
> +                    .set_offs(mem_base)
> +                    .write(bar, &E::ID, port);
> +
> +                let mut tag = tag;
> +                for block in img.chunks(256) {
> +                    regs::NV_PFALCON_FALCON_IMEMT::default()
> +                        .set_tag(tag)
> +                        .write(bar, &E::ID, port);
> +                    for word in block.chunks(4) {
> +                        let w = u32::from_le_bytes(word.try_into().unwrap());

If img.size is not a multiple of 4 bytes, this can panic right?

Even if it is unlikely, unwrap() is quite frowned up due to possibility of
panic. I'd recommend something like the following since the function cannot
return an error:

                        let w = if let Ok(bytes) = word.try_into() {
                            u32::from_le_bytes(bytes)
                        } else {
                            // can print a warning here too if needed.
                            let mut buf = [0u8; 4];
                            buf[..word.len()].copy_from_slice(word);
                            u32::from_le_bytes(buf)
                        };

Btw, I wish we could encode the slice length constraint in the slice type itself
(i.e., the slice length ought to be a certain multiple). But I think there's no
way to do that without introducing a new type.

Thanks.