Re: [PATCH 07/27] gpu: nova-core: add optional ucodes firmware loading

[John Hubbard]

On 8/19/26 11:41 AM, Timur Tabi wrote:
> On Tue, 2026-08-18 at 20:52 -0700, John Hubbard wrote:
>> +    /// Loads the payload described by this TLV into owned memory.
>> +    ///
>> +    /// The generic firmware TLV representation is either an inline `BLOB`, or a `FILE`
>> basename
>> +    /// together with its `SIZE`. Mixing the representations or specifying only half of the
>> file
>> +    /// representation is invalid. Referenced files are constrained to the metadata file's
>> +    /// directory.
>> +    pub(crate) fn load_blob_or_file(
>> +        &self,
>> +        dev: &device::Device,
>> +        chipset: gpu::Chipset,
>> +    ) -> Result<VVec<u8>> {
>> +        let has_blob = self.contains(b"BLOB");
>> +        let has_file = self.contains(b"FILE");
>> +        let has_size = self.contains(b"SIZE");
> 
> The problem with this approach is that it allocates a buffer for the BLOB and memcpys the data,
> even though that's not necessary if you expect the TLV to have a BLOB tag.  It's highly unlikely
> that the driver will ever not know in advance whether a given TLV has a BLOB or a FILE tag.  
> 
> We should not encourage the driver to use this function for all images.  Instead, I would like
> to see a comment that stipulates that this function should be used in only two cases:
> 
> 1) We will want to keep the image data after the TLV goes out of scope, so we want the BLOB
> copied into a VVec anyway.
> 
> 2) We expect the TLV to have a FILE tag, but obviously it shouldn't fail if it doesn't.
> 

OK, let me go tighten this up accordingly, yes.

thanks,
-- 
John Hubbard