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

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.