Re: [PATCH v5 3/8] gpu: nova-core: add TLV parser for firmware files

["Alexandre Courbot" <acourbot@nvidia.com>]

On Sat Jul 11, 2026 at 8:04 AM JST, Timur Tabi wrote:
> TLV (type, length, value) files are the new image format used by Nova
> to encapsulate firmware images and their metadata.  Unlike the firmware
> files for previous versions of the firmware, TLV filenames are not
> versioned, and they have a .tlv suffix.
>
> Add function request_tlv() to load TLV firmware images.
>
> Add the Tlv struct and supporting types for parsing TLV (type, length,
> value) firmware images. TLV files begin with a 4-byte magic header,
> which must be "NVFW" for Nvidia firmware files.  This is followed by a
> sequence of blocks each containing a 4-byte ASCII tag, a 4-byte
> little-endian length, and a payload padded to a 4-byte boundary.
>
> Tlv::new() validates the entire image up front, so that the iterator can
> subsequently yield blocks without fallible parsing.
>
> Also add accessor methods for the various encoded types that will be used
> by the driver.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  Documentation/gpu/nova/core/tlv.rst   | 187 ++++++++++++++++++

We should add a link to this new document in
`Documentation/gpu/nova/index.rst`.

>  drivers/gpu/nova-core/firmware.rs     |   1 +
>  drivers/gpu/nova-core/firmware/tlv.rs | 274 ++++++++++++++++++++++++++
>  3 files changed, 462 insertions(+)
>  create mode 100644 Documentation/gpu/nova/core/tlv.rst
>  create mode 100644 drivers/gpu/nova-core/firmware/tlv.rs
>
> diff --git a/Documentation/gpu/nova/core/tlv.rst b/Documentation/gpu/nova/core/tlv.rst
> new file mode 100644
> index 000000000000..9e8c08727977
> --- /dev/null
> +++ b/Documentation/gpu/nova/core/tlv.rst
> @@ -0,0 +1,187 @@
> +.. SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +==================================
> +TLV Tags in Nova Firmware Images
> +==================================
> +
> +Nova firmware images use a Type-Length-Value (TLV) format to encapsulate
> +firmware components and metadata. The TLV file begins with a 4-byte "magic"
> +header that contains the string "NVFW".  Following the header is a sequence of
> +TLV blocks.
> +
> +Each block consists of a 4-byte tag of ASCII characters, a 4-byte length
> +encoded as a little-endian unsigned integer, and a sequence of bytes, the size
> +of which is equal to the length rounded up to the next multiple of 4.
> +
> +The driver code that reads the TLV and uses its contents is called the parser.
> +It is the responsibility of the parser to handle missing or malformed tags,
> +lengths, and values in the TLV.
> +
> +::
> +
> +    +------+------+------+------+
> +    |  'N' |  'V' |  'F' |  'W' |  Magic header
> +    +------+------+------+------+
> +    |  Tag (4 bytes, ASCII)     |  TLV block 0
> +    +---------------------------+
> +    |  Length (4 bytes, LE)     |
> +    +---------------------------+
> +    |                           |
> +    |  Value (length bytes,     |
> +    |  padded to 4-byte align)  |
> +    |                           |
> +    +---------------------------+
> +    |  Tag (4 bytes, ASCII)     |  TLV block 1
> +    +---------------------------+
> +    |  Length (4 bytes, LE)     |
> +    +---------------------------+
> +    |                           |
> +    |  Value (length bytes,     |
> +    |  padded to 4-byte align)  |
> +    |                           |
> +    +---------------------------+
> +    |         ...               |  More TLV blocks
> +    +---------------------------+
> +
> +Tags and Length
> +===============
> +TLV tags are always four-character words, with all letters being upper case.
> +Duplicate tags are not allowed.
> +
> +Lengths of zero are allowed and indicate that the tag is a boolean.  That is,

Let's mention "Payloads of length zero" to be precise what length this
is about.

> +presence of the tag indicates ``True`` and absence indicates ``False``.
> +
> +A TLV file may contain additional tags not described in this document.
> +
> +Values
> +======
> +Values are one of three types.  The type is not encoded in the format; rather,
> +the parser expects a given tag to have a value of a given type.
> +
> +1) Integers, encoded in 32-bit or 64-bit little-endian format.
> +2) Strings, encoded as-is and required to be ASCII only and without a null terminator.
> +3) An array of bytes, for binary data.

I guess booleans adds a fourth value type?

> +
> +Common Tags
> +===========
> +These tags are shared across firmware types and carry the same meaning
> +wherever they appear.  Unlike the firmware-specific tags below, a common tag
> +is reserved: its meaning is fixed and may never be redefined for a particular
> +firmware type.
> +
> +``VERS`` (string)
> +    Human-readable firmware version string, indicates the version of
> +    the firmware.  Present in all TLV files.
> +
> +A TLV image must contain either a single ``BLOB`` tag (firmware embedded
> +inline) or a ``SIZE``/``FILE`` pair (firmware stored in a separate file).
> +
> +``BLOB`` (bytes)
> +    If the firmware microcode binary is stored in the TLV, this tag contains
> +    the actual firmware image bytes.
> +
> +``FILE`` (string)
> +    If the firmware binary is stored as a separate file, this tag contains the
> +    name of that file, which is required to be in the same directory as the TLV,
> +    so no paths are allowed in the filename.  This tag is always paired with
> +    ``SIZE``, so as to allow the driver to pre-allocate the buffer before
> +    loading the file.
> +
> +``SIZE`` (u32)
> +    Total size in bytes of the firmware image to be loaded from the companion
> +    file named by ``FILE``.  This tag is mandatory if ``FILE`` exists, so the
> +    size of the firmware image must be known when the TLV is created.  If the
> +    firmware image is updated and its size changes, then the TLV must be
> +    updated with it.
> +
> +GSP Firmware Tags
> +=================
> +``SIGN`` (bytes)
> +    Cryptographic signature for the GSP firmware.
> +
> +``BLID`` (bytes)
> +    The build ID, extracted from the ".note.gnu.build-id" section.

Should this be a string by any chance?

> +
> +Booter Firmware Tags
> +====================
> +``DAOF`` (u32) - ``os_data_offset``
> +    OS data section offset within the firmware image (absolute byte offset).
> +    Maps to the DMEM load source.
> +
> +``DASZ`` (u32) - ``os_data_size``
> +    OS data section size in bytes.
> +
> +``CDOF`` (u32) - ``os_code_offset``
> +    OS code section offset within the firmware image (absolute byte offset).
> +    Maps to the non-secure IMEM load source.
> +
> +``CDSZ`` (u32) - ``os_code_size``
> +    OS code section size in bytes.
> +
> +``PLOC`` (u32) - ``patch_loc``
> +    Signature patch location -- byte offset within the firmware image where the
> +    selected signature should be written.
> +
> +``FUSE`` (u32) - ``fuse_version``
> +    Fuse version of the firmware, used with the hardware fuse register to
> +    select the correct signature index.
> +
> +``ENID`` (u32) - ``engine_id``
> +    Engine ID mask identifying the falcon engine this firmware targets.
> +
> +``UCID`` (u32) - ``ucode_id``
> +    Microcode ID used together with the engine ID to query hardware signature
> +    fuse registers.
> +
> +``A0CO`` (u32) - ``app0_code_offset``
> +    App0 code offset -- start of the secure code region within the firmware
> +    image. Used as the IMEM secure section source.
> +
> +``A0CS`` (u32) - ``app0_code_size``
> +    App0 code size in bytes.
> +
> +``NSIG`` (u32) - ``num_sigs``
> +    Number of signatures included in the ``SIGN`` tag.  A value of 0 indicates
> +    unsigned firmware and that there is no ``SIGN`` tag.

In patch 4 `booter.rs` says "Booter is always signed", which contradicts
this definition. We had a non-signed path in the original code; why not
preserve it? The current firmware files do not make use of it, but the
point of specifying things here is to make the code future-proof in case
we introduce or enable unsigned firmwares in the future; so let's make
the code religiously follow the spec.

> +
> +``SIGN`` (bytes)
> +    Concatenated array of firmware signatures. The size of each signature is
> +    the total length of the ``SIGN`` value divided by ``NSIG``. The correct
> +    signature is selected using the fuse-version-derived index.
> +
> +Generic Bootloader Tags
> +=======================
> +``CDSZ`` (u32) - ``code_size``
> +    Size in bytes of the bootloader code to copy from the ``BLOB`` tag and
> +    PIO-load into falcon IMEM.
> +
> +``STRT`` (u32) - ``start_tag``
> +    Start tag identifying the IMEM block where execution begins.  The falcon
> +    boot address is derived as ``start_tag << 8``.
> +
> +GSP Bootloader Tags
> +===================
> +``CDOF`` (u32) - ``code_offset``
> +    Offset within the firmware image at which the code section starts.
> +
> +``DAOF`` (u32) - ``data_offset``
> +    Offset within the firmware image at which the data section starts.
> +
> +``MFOF`` (u32) - ``manifest_offset``
> +    Offset within the firmware image at which the manifest starts.
> +
> +``APPV`` (u32) - ``app_version``
> +    Application version of the firmware.
> +
> +FMC Firmware Tags
> +=================
> +``HASH`` (bytes)
> +    SHA-384 hash of the FMC firmware, exactly 48 bytes long.
> +
> +``PKEY`` (bytes)
> +    Public key used to verify the FMC firmware. At most 384 bytes (RSA-3072),
> +    but may be shorter.
> +
> +``SIGN`` (bytes)
> +    Signature of the FMC firmware. At most 384 bytes (RSA-3072), but may
> +    be shorter.
> \ No newline at end of file
> diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
> index a94820a3b335..c0cd06579643 100644
> --- a/drivers/gpu/nova-core/firmware.rs
> +++ b/drivers/gpu/nova-core/firmware.rs
> @@ -32,6 +32,7 @@
>  pub(crate) mod fwsec;
>  pub(crate) mod gsp;
>  pub(crate) mod riscv;
> +pub(crate) mod tlv;
>  
>  pub(crate) const FIRMWARE_VERSION: &str = "570.144";
>  
> diff --git a/drivers/gpu/nova-core/firmware/tlv.rs b/drivers/gpu/nova-core/firmware/tlv.rs
> new file mode 100644
> index 000000000000..5dfd2dd814f8
> --- /dev/null
> +++ b/drivers/gpu/nova-core/firmware/tlv.rs
> @@ -0,0 +1,274 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
> +
> +use kernel::{
> +    device,
> +    firmware,
> +    prelude::*,
> +    str::CString, //
> +};
> +
> +use crate::{
> +    gpu,
> +    num::*, //
> +};
> +
> +/// Requests the GPU firmware TLV `name` suitable for `chipset`.
> +#[expect(dead_code)]
> +pub(crate) fn request_tlv(
> +    dev: &device::Device,
> +    chipset: gpu::Chipset,
> +    name: &str,
> +) -> Result<firmware::Firmware> {
> +    let chip_name = chipset.name();
> +
> +    dev_dbg!(
> +        dev,
> +        "loading firmware image {}/gsp/{}.tlv\n",

This is missing `nvidia` in the path. To avoid issue I would suggest
building the `CString` earlier and using it both here and in the
`try_from_fmt` statement below.

<...>
> +    /// Return a slice of bytes.  Returns ENODATA if the value is empty.
> +    pub(crate) fn get_bytes(&self, tag: &[u8; 4]) -> Result<&'a [u8]> {
> +        let tlv = self.find(tag)?;
> +
> +        // Treat empty value as an error, to avoid trying to parse nothing.
> +        if tlv.value.is_empty() {
> +            return Err(ENODATA);
> +        }
> +
> +        Ok(tlv.value)
> +    }
> +
> +    // Return a little-endian u32.

Doccomment should use `///`.

> +    pub(crate) fn get_u32(&self, tag: &[u8; 4]) -> Result<u32> {
> +        let tlv = self.find(tag)?;
> +
> +        tlv.value
> +            .try_into()
> +            .ok()
> +            .map(u32::from_le_bytes)
> +            .ok_or(EINVAL)
> +    }
> +
> +    /// Return a string value.
> +    pub(crate) fn get_string(&self, tag: &[u8; 4]) -> Result<&'a str> {
> +        let tlv = self.find(tag)?;
> +
> +        let bytes = tlv.value;
> +
> +        // To make sure the value actually is a string, ensure it's all ASCII.
> +        if !bytes.is_ascii() {
> +            return Err(EINVAL);
> +        }

The spec also says that NULL characters are invalid; we should test for
`|| bytes.contains(&0)` as well here.

> +
> +        core::str::from_utf8(bytes).map_err(|_| EINVAL)
> +    }
> +
> +    /// Obtain the nth signature from a SIGN tag.  If `index` is None,
> +    /// then return the last signature.
> +    pub(crate) fn get_signature(&self, index: Option<usize>) -> Result<&'a [u8]> {
> +        let num_sigs: usize = match self.get_u32(b"NSIG")? {
> +            0 => return Err(EINVAL),
> +            n => n.into_safe_cast(),
> +        };
> +
> +        let sig_bytes = self.get_bytes(b"SIGN")?;
> +
> +        // Ensure that sig_bytes can be divided evenly into chunks.
> +        if sig_bytes.len() % num_sigs != 0 {
> +            return Err(EINVAL);
> +        }
> +
> +        // num_sigs cannot be 0, and sig_bytes cannot be empty, so this cannot panic.
> +        let sig_size = sig_bytes.len() / num_sigs;
> +
> +        let index: usize = match index {
> +            None => num_sigs - 1,
> +            Some(index) => index,
> +        };

This can be a one-liner:

  let index = index.unwrap_or(num_sigs - 1);