Re: [PATCH v3 3/7] gpu: nova-core: transition booter_load to TLV images

Timur Tabi <[email protected]>
Newsgroups dev.linux.lists.nova-gpu,dev.linux.lists.driver-core,org.kernel.vger.rust-for-linux
Message-ID <[email protected]>
On Mon, 2026-07-06 at 15:31 +0900, Alexandre Courbot wrote:
> 
> > +        let os_data_offset = tlv.get_u32(b"DAOF")?;
> > +        let os_data_size = tlv.get_u32(b"DASZ")?;
> > +        let os_code_offset = tlv.get_u32(b"CDOF")?;
> > +        let os_code_size = tlv.get_u32(b"CDSZ")?;
> > +        let patch_loc = tlv.get_u32(b"PLOC")?;
> > +        let fuse_version = tlv.get_u32(b"FUSE")?;
> > +        let engine_id = tlv.get_u32(b"ENID")?;
> > +        let ucode_id = tlv.get_u32(b"UCID")?;
> > +        let app0_code_offset = tlv.get_u32(b"A0CO")?;
> > +        let app0_code_size = tlv.get_u32(b"A0CS")?;
> > +        let num_sigs = tlv.get_u32(b"NSIG")?;
> > +        let sig_bytes = tlv.get_bytes(b"SIGN")?;
> > +
> > +        // Booter is always signed
> 
> The `.rst` file mentions the non-signed case though - we should
> reconcile the spec and the code one way or the other.

The .rst does not mention that Booter is signed, only that if the firmware is unsigned, then NSIG is
0 and SIGN does not exist.  And since get_bytes() now returns Error if SIGN doesn't exist, we
enforce that here.

But you're right in that it does feel a little off.  That's one of the reasons I had get_nth() in v1
-- to streamline getting the nth signature.  

What do you think about adding a get_signature(index) method that combines parsing NSIG and SIGN,
automatically calculating sig_size and returning Option<&[u8]> ?

> 
> >  
> > +        // The size of one signature
> > +        let sig_size = sig_bytes
> > +            .len()
> > +            .checked_div(num_sigs.into_safe_cast())
> > +            .ok_or(EINVAL)?;
> > +
> > +        // Extract the nth signature
> > +        let sig_chunk = sig_bytes
> > +            .chunks_exact(sig_size)
> 
> As Sashiko pointed out, this will panic if `sig_size == 0`, so we need
> to check that - and whether an unsigned Booter is valid at all.

sig_size cannot be 0 because of this:

        if !(1..=15).contains(&num_sigs) || sig_bytes.len() % num_sigs != 0 {

Neither sig_bytes.len() nor num_sigs can be 0.

But IMHO, this just reinforces the idea that I should add get_signature().
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.