[PATCH 0/8] Transition Nova Core to TLV firmware images

[PATCH 0/8] Transition Nova Core to TLV firmware images

[Timur Tabi]

This patch set transitions nova-core to use the new "TLV" firmware image
files, instead of the ones that Nouveau uses.

The current r570.144 images are a mix of binary headers and ELF files that
are cumbersome to parse in Rust.  There's a significant amount of code
that just reads in a struct, extracts some offset, and uses it to find
another struct, only to have nova-core use just a few fields.

The new format uses a sequence of tag/length/value fields that can be
iterated over.  The script that generates the TLV files,
extract-firmware-nova.py, does the extra work to find the specific metadata
needed by Nova and packages each one separately.

The TLV versions of r570.144 can be found here:

	https://github.com/ttabi/linux-firmware-nova

along with instructions on how to install them.  We are not planning on
submitting these images to linux-firmware.  Rather, if this patchset
is accepted upstream, I expect the small handful of people who are
actually working on Nova to grab and install these images, which needs
to be done only once.

There are still opportunities for improvement.  For example, I would like
to get rid of more GPU-specific code, especially the GA100 quirks.

Timur Tabi (8):
  rust: firmware: add request_into_buf()
  gpu: nova-core: add request_tlv to load TLV images
  gpu: nova-core: add TLV parser for firmware files
  gpu: nova-core: transition booter_load to TLV images
  gpu: nova-core: transition gsp to TLV images
  gpu: nova-core: transition gen_bootloader to TLV images
  gpu: nova-core: transition fsp to TLV images
  gpu: nova-core: update firmware module info for TLV images

 Documentation/gpu/nova/core/tlv.rst           | 172 +++++++
 drivers/gpu/nova-core/firmware.rs             | 451 ++++++++----------
 drivers/gpu/nova-core/firmware/booter.rs      | 328 +++----------
 drivers/gpu/nova-core/firmware/fsp.rs         |  84 ++--
 .../nova-core/firmware/fwsec/bootloader.rs    |  72 +--
 drivers/gpu/nova-core/firmware/gsp.rs         |  56 +--
 drivers/gpu/nova-core/firmware/riscv.rs       |  71 +--
 drivers/gpu/nova-core/gsp/boot.rs             |   7 +-
 drivers/gpu/nova-core/gsp/hal/gh100.rs        |   7 +-
 drivers/gpu/nova-core/gsp/hal/tu102.rs        |  15 +-
 rust/kernel/firmware.rs                       |  45 ++
 11 files changed, 575 insertions(+), 733 deletions(-)
 create mode 100644 Documentation/gpu/nova/core/tlv.rst


base-commit: 9eaff547805f8556992a9474465001c3e128b7bd
prerequisite-patch-id: 0a9ea098bc3576171da9f6293065fdb4db552466
-- 
2.54.0

[PATCH 1/8] rust: firmware: add request_into_buf()

[Timur Tabi]

Add request_into_buf(), a Rust wrapper around the
request_firmware_into_buf() function. This variant loads the firmware
image directly into a caller-provided buffer rather than a
kernel-allocated one.

Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 rust/kernel/firmware.rs | 45 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
index 71168d8004e2..d2ffaacc2d43 100644
--- a/rust/kernel/firmware.rs
+++ b/rust/kernel/firmware.rs
@@ -120,6 +120,51 @@ fn drop(&mut self) {
     }
 }
 
+/// Load firmware directly into the caller-provided `buf`.
+///
+/// On success the firmware image has been copied into `buf`; the number of bytes
+/// written is returned. The caller accesses the data through `buf` itself.
+/// See also `bindings::request_firmware_into_buf`.
+///
+/// This is intentionally a stand-alone function rather than a `Firmware` constructor. For
+/// the `into_buf` path, the firmware data lives in the caller's `buf`, not in a
+/// kernel-owned buffer, so returning a `Firmware` would expose `Firmware::data()` as an
+/// alias of `buf`. Since `buf` is `&mut [u8]` and the returned handle does not borrow it,
+/// the caller could mutate `buf` while reads via `data()` are outstanding (and
+/// `release_firmware()` does not free `buf` anyway). Releasing the bookkeeping here and
+/// returning only the size leaves `buf` as the single owner and accessor of the data.
+pub fn request_into_buf(name: &CStr, dev: &Device, buf: &mut [u8]) -> Result<usize> {
+    let mut fw: *mut bindings::firmware = core::ptr::null_mut();
+    let pfw: *mut *mut bindings::firmware = &mut fw;
+    let pfw: *mut *const bindings::firmware = pfw.cast();
+
+    // SAFETY: `pfw` is a valid pointer to a NULL initialized `bindings::firmware` pointer.
+    // `name` and `dev` are valid as by their type invariants. `buf` is a valid writable
+    // buffer of `buf.len()` bytes.
+    let ret = unsafe {
+        bindings::request_firmware_into_buf(
+            pfw,
+            name.as_char_ptr(),
+            dev.as_raw(),
+            buf.as_mut_ptr().cast(),
+            buf.len(),
+        )
+    };
+    if ret != 0 {
+        return Err(Error::from_errno(ret));
+    }
+
+    // SAFETY: `fw` is a valid pointer returned by `request_firmware_into_buf`.
+    let size = unsafe { (*fw).size };
+
+    // The firmware bytes are now in `buf`, which the caller owns, so we don't need
+    // the kernel to hang on to it any more.
+    // SAFETY: `fw` is a valid pointer returned by `request_firmware_into_buf`.
+    unsafe { bindings::release_firmware(fw) };
+
+    Ok(size)
+}
+
 // SAFETY: `Firmware` only holds a pointer to a C `struct firmware`, which is safe to be used from
 // any thread.
 unsafe impl Send for Firmware {}
-- 
2.54.0

[PATCH 2/8] gpu: nova-core: add request_tlv to load TLV images

[Timur Tabi]

Add function request_tlv() to load TLV firmware images.

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.

Also add some #[allow(unused)] statements to two identifiers that will
soon be removed, to keep the patchset clean.

Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 drivers/gpu/nova-core/firmware.rs | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
index 279fbacd0b8e..2749c196416d 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -33,9 +33,11 @@
 pub(crate) mod gsp;
 pub(crate) mod riscv;
 
+#[allow(unused)]
 pub(crate) const FIRMWARE_VERSION: &str = "570.144";
 
 /// Requests the GPU firmware `name` suitable for `chipset`, with version `ver`.
+#[allow(unused)]
 fn request_firmware(
     dev: &device::Device,
     chipset: gpu::Chipset,
@@ -48,6 +50,21 @@ fn request_firmware(
         .and_then(|path| firmware::Firmware::request(&path, dev))
 }
 
+/// Requests the GPU firmware TLV `name` suitable for `chipset`.
+#[allow(unused)]
+fn request_tlv(
+    dev: &device::Device,
+    chipset: gpu::Chipset,
+    name: &str,
+) -> Result<firmware::Firmware> {
+    let chip_name = chipset.name();
+
+    dev_info!(dev, "loading firmware image {}.tlv\n", name);
+
+    CString::try_from_fmt(fmt!("nvidia/{chip_name}/gsp/{name}.tlv"))
+        .and_then(|path| firmware::Firmware::request(&path, dev))
+}
+
 /// Structure used to describe some firmwares, notably FWSEC-FRTS.
 #[repr(C)]
 #[derive(Debug, Clone)]
-- 
2.54.0

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

[Timur Tabi]

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 | 172 ++++++++++++++++++++
 drivers/gpu/nova-core/firmware.rs   | 239 +++++++++++++++++++++++++++-
 2 files changed, 410 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/gpu/nova/core/tlv.rst

diff --git a/Documentation/gpu/nova/core/tlv.rst b/Documentation/gpu/nova/core/tlv.rst
new file mode 100644
index 000000000000..8223605daa6b
--- /dev/null
+++ b/Documentation/gpu/nova/core/tlv.rst
@@ -0,0 +1,172 @@
+.. 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 encode as a little-endian non-negative integer, and a
+sequence of bytes, the size of which is equal to the length rounded
+up to the next multiple of 4.
+
+::
+
+    +------+------+------+------+
+    |  '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
+    +---------------------------+
+
+Values
+======
+Values are one of three types:
+
+1) Integers, encoded in 32-bit or 64-bit little-endian format.
+2) Strings, encoded as-is and expected to be ASCII only, without a null terminator.
+3) An array of bytes.
+
+
+Common Tags
+===========
+These tags appear across multiple firmware types.
+
+``VERS`` (string)
+    Human-readable firmware version string, indicates the version of
+    the firmware.  Present in all TLV files.
+
+A TLV image contains either a ``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, assumed to be in the same directory as the TLV.
+    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``.
+
+GSP Firmware Tags
+=================
+Used when loading the GSP (GPU System Processor) firmware
+(``gsp.bin``).
+
+``SIGN`` (bytes)
+    Cryptographic signatures for the GSP firmware, DMA-mapped
+    for the GSP bootloader to verify before execution.
+
+Booter Firmware Tags
+====================
+Used by the Booter firmware (``booter_load.bin``,
+``booter_unload.bin``).
+
+``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 ``SIGS`` tag. A value
+    of 0 indicates unsigned firmware.
+
+``SIGS`` (bytes)
+    Concatenated array of firmware signatures. The size of
+    each signature is the total length of the ``SIGS`` value
+    divided by ``NSIG``. The correct signature is selected
+    using the fuse-version-derived index.
+
+Generic Bootloader Tags (FWSEC)
+===============================
+Used by the generic bootloader (``gen_bootloader.bin``) that
+loads the FWSEC firmware via DMA on Turing GPUs. Also includes
+the common ``VERS`` and ``BLOB`` 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``.
+
+RISC-V Firmware Tags (GSP Bootloader)
+======================================
+Used by RISC-V-based firmwares such as the GSP bootloader
+(``gsp_bootloader.bin``). Also includes the common ``VERS``
+and ``BLOB`` 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.
diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
index 2749c196416d..888a426b7b41 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -11,7 +11,7 @@
     device,
     firmware,
     prelude::*,
-    str::CString,
+    str::{CStr, CString},
     transmute::FromBytes, //
 };
 
@@ -684,3 +684,240 @@ pub(super) fn elf_section<'a>(elf: &'a [u8], name: &str) -> Option<&'a [u8]> {
         }
     }
 }
+
+pub(crate) struct TlvBlock<'a> {
+    pub(crate) tag: &'a str,
+    pub(crate) value: &'a [u8],
+}
+
+/// On-wire TLV block header: 4-byte ASCII tag + little-endian payload length (bytes, excluding
+/// padding to a 4-byte boundary).
+struct TlvBlockHeader<'a> {
+    tag: &'a str,
+    length: usize,
+}
+
+impl<'a> TlvBlockHeader<'a> {
+    const SIZE: usize = size_of::<[u8; 4]>() + size_of::<u32>();
+
+    /// Parses the first [`Self::SIZE`] bytes of `hdr` (caller may pass a longer slice).
+    fn parse(hdr: &'a [u8]) -> Option<Self> {
+        let hdr = hdr.get(..Self::SIZE)?;
+        let tag_bytes = hdr.get(..4)?;
+        let tag = core::str::from_utf8(tag_bytes).ok()?;
+        if !tag.is_ascii() {
+            return None;
+        }
+        let len_arr = <[u8; 4]>::try_from(hdr.get(4..Self::SIZE)?).ok()?;
+        let length = u32::from_le_bytes(len_arr) as usize;
+        Some(Self { tag, length })
+    }
+}
+
+/// Sequential scan over [`Tlv`]. Unlike [`Tlv`], this type carries a cursor (`pos`) into
+/// the parent blob.  It is not interchangeable with a fresh [`Tlv`] view of the same
+/// bytes.
+///
+/// # Invariants
+///
+/// `pos` is a byte offset into `tlv.data` that always lies on a block boundary (in the sense
+/// of the [`Tlv`] invariant): it is either the start of a well-formed block, or equal to
+/// `tlv.data.len()` (end of iteration).
+struct TlvIter<'tlv, 'a> {
+    tlv: &'tlv Tlv<'a>,
+    pos: usize,
+}
+
+impl<'tlv, 'a> Iterator for TlvIter<'tlv, 'a> {
+    type Item = TlvBlock<'a>;
+
+    /// Returns the block starting at `self.pos` and advances the cursor past it, or [`None`]
+    /// once the cursor reaches the end of the data.
+    ///
+    /// The body relies on a number of `unsafe` operations, because [`Iterator::next`] can
+    /// only return [`Option`], not [`Result`], so there is no way to report a parse error
+    /// from here.  That is why the entire TLV stream is validated up front in the
+    /// constructor.  By this type's invariants, `self.pos` is always on a block boundary
+    /// into already-validated data, so each block is known to be well-formed, and
+    /// therefore the unchecked operations cannot fail.
+    fn next(&mut self) -> Option<Self::Item> {
+        if self.pos >= self.tlv.data.len() {
+            return None;
+        }
+
+        let tail = &self.tlv.data[self.pos..];
+
+        // SAFETY: `self.pos` is on a block boundary (`TlvIter` invariant) and the check
+        // above gives `self.pos < data.len()`, so by the `Tlv` invariant a complete
+        // well-formed block starts at `tail`.
+        let hdr = unsafe { tail.get_unchecked(..TlvBlockHeader::SIZE) };
+
+        // SAFETY: same header bytes as validated in `Tlv::new` for this offset.
+        let header = unsafe { TlvBlockHeader::parse(hdr).unwrap_unchecked() };
+
+        let stored_size = header.length.next_multiple_of(4);
+        let advance = TlvBlockHeader::SIZE + stored_size;
+        let payload_end = TlvBlockHeader::SIZE + header.length;
+
+        // SAFETY: `advance` and `payload_end` are exactly the stored and logical payload extents
+        // `Tlv::new` accepted for this block.
+        let value = unsafe {
+            let block = tail.get_unchecked(..advance);
+            block.get_unchecked(TlvBlockHeader::SIZE..payload_end)
+        };
+
+        // INVARIANT: by the `Tlv` invariant the block at `self.pos` occupies exactly `advance`
+        // bytes, so `self.pos + advance` is the next block boundary (or `data.len()`).
+        self.pos += advance;
+
+        Some(TlvBlock {
+            tag: header.tag,
+            value,
+        })
+    }
+}
+
+/// The payload of a validated TLV (type, length, value) firmware image.
+///
+/// TLV firmware images start with a 4-byte "NVFW" magic header, followed by a sequence of
+/// blocks. Each block has a 4-byte type tag, a 4-byte length field, and a data payload
+/// whose stored size is the length rounded up to the nearest multiple of 4.
+///
+/// [`Self::new`] checks the magic header and walks every block: tags must be ASCII,
+/// lengths and padding must fit without overflow, and the byte stream after `NVFW` must
+/// be exactly partitionable into blocks (no trailing partial header or slack). After
+/// that, [`TlvIter`] only signals end-of-stream via [`None`], not parse failure.
+///
+/// # Invariants
+///
+/// `data` is a validated TLV payload (the bytes *after* the `NVFW` magic): it is the exact
+/// concatenation of zero or more well-formed blocks, with no trailing partial header or slack.
+/// Consequently, any offset `o` into `data` that is a block boundary and satisfies
+/// `o < data.len()` is the start of a complete block whose header parses and whose stored
+/// extent (`TlvBlockHeader::SIZE + header.length.next_multiple_of(4)` bytes) lies within
+/// `data`. `data.len()` is itself a boundary.
+#[allow(dead_code)]
+pub(crate) struct Tlv<'a> {
+    data: &'a [u8],
+}
+
+#[allow(dead_code)]
+impl<'a> Tlv<'a> {
+    const MAGIC: &'static [u8; 4] = b"NVFW";
+
+    /// Parses `data` as a TLV firmware image, returning [`EINVAL`] if the image is malformed.
+    pub(crate) fn new(data: &'a [u8]) -> Result<Self> {
+        // Verify that the magic bytes exist and are the correct value
+        let magic_len = Self::MAGIC.len();
+        if data
+            .get(..magic_len)
+            .is_none_or(|magic| magic != Self::MAGIC)
+        {
+            return Err(EINVAL);
+        }
+
+        // The payload is the contiguous sequence of TLV blocks after the magic.
+        let payload = data.get(magic_len..).ok_or(EINVAL)?;
+
+        let mut pos = 0usize;
+        while pos < payload.len() {
+            // Get the next TLV block.
+            let Some(rest) = payload.get(pos..) else {
+                return Err(EINVAL);
+            };
+            // Validate and extract the header (type, length).
+            let Some(header) = rest
+                .get(..TlvBlockHeader::SIZE)
+                .and_then(TlvBlockHeader::parse)
+            else {
+                return Err(EINVAL);
+            };
+            // The `length` field of a TLV block contains the actual byte length of the
+            // value, but each TLV block is aligned to a 4-byte boundary.
+            let Some(stored_size) = header.length.checked_next_multiple_of(4) else {
+                return Err(EINVAL);
+            };
+            let end = pos
+                .checked_add(TlvBlockHeader::SIZE)
+                .and_then(|p| p.checked_add(stored_size))
+                .ok_or(EINVAL)?;
+            if end > payload.len() {
+                return Err(EINVAL);
+            }
+            pos = end;
+        }
+
+        // INVARIANT: the loop above walked `payload` from offset 0 and only stopped once `pos`
+        // reached `payload.len()` exactly, rejecting any block whose header failed to parse or
+        // whose stored extent overran. So `payload` is an exact concatenation of well-formed
+        // blocks with no partial header or trailing slack.
+        Ok(Self { data: payload })
+    }
+
+    fn iter(&self) -> TlvIter<'_, 'a> {
+        // INVARIANT: 0 is a block boundary, either the start of the first block,
+        // or `data.len()` when `data` is empty.
+        TlvIter { tlv: self, pos: 0 }
+    }
+
+    pub(crate) fn len(&self, tag: &str) -> Result<usize> {
+        let tlv = self.iter().find(|b| b.tag == tag).ok_or(EINVAL)?;
+
+        Ok(tlv.value.len())
+    }
+
+    pub(crate) fn get_bytes(&self, tag: &str) -> Result<&'a [u8]> {
+        let tlv = self.iter().find(|b| b.tag == tag).ok_or(EINVAL)?;
+
+        Ok(tlv.value)
+    }
+
+    pub(crate) fn get_u32(&self, tag: &str) -> Result<u32> {
+        let tlv = self.iter().find(|b| b.tag == tag).ok_or(EINVAL)?;
+
+        tlv.value
+            .try_into()
+            .ok()
+            .map(u32::from_le_bytes)
+            .ok_or(EINVAL)
+    }
+
+    pub(crate) fn get_string(&self, tag: &str) -> Result<&'a str> {
+        let tlv = self.iter().find(|b| b.tag == tag).ok_or(EINVAL)?;
+
+        // Handle the possibility that the value is null-terminated.
+        let bytes = match CStr::from_bytes_until_nul(tlv.value) {
+            Ok(cstr) => cstr.to_bytes(),
+            Err(_) => tlv.value,
+        };
+
+        // But do require it to be all ASCII.
+        if !bytes.is_ascii() {
+            return Err(EINVAL);
+        }
+
+        core::str::from_utf8(bytes).map_err(|_| EINVAL)
+    }
+
+    /// Returns the `n`-th fixed-size chunk from the value of the TLV entry matching
+    /// `tag`.
+    ///
+    /// The value is treated as an array of contiguous, equally-sized chunks, each
+    /// `sig_size` bytes long. `sig_size` is the size of one signature.  The returned
+    /// slice is the `n`-th chunk.
+    ///
+    /// Returns [`EINVAL`] if no entry matches `tag`, if computing the chunk's offset
+    /// overflows, or if the value is too short to contain the requested chunk.
+    pub(crate) fn get_nth_chunk(&self, tag: &str, sig_size: usize, n: usize) -> Result<&'a [u8]> {
+        let sigs = self
+            .iter()
+            .find(|b| b.tag == tag)
+            .map(|b| b.value)
+            .ok_or(EINVAL)?;
+
+        let start = sig_size.checked_mul(n).ok_or(EINVAL)?;
+        let end = start.checked_add(sig_size).ok_or(EINVAL)?;
+
+        sigs.get(start..end).ok_or(EINVAL)
+    }
+}
-- 
2.54.0

[PATCH 4/8] gpu: nova-core: transition booter_load to TLV images

[Timur Tabi]

Switch the booter firmware loader from the legacy binary format to the
TLV format.  This change requires the new TLV versions of the r570.144
firmware images.

The new TLV format has all of the metadata needed by Nova encoded as
separate tags, eliminating the need to parse legacy firmware headers
such as HsHeaderV2 and HsSignatureParams.  All of the structs and
code for parsing those headers is therefore deleted.

Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 drivers/gpu/nova-core/firmware.rs        |  15 +-
 drivers/gpu/nova-core/firmware/booter.rs | 328 ++++-------------------
 drivers/gpu/nova-core/gsp/hal/tu102.rs   |  15 +-
 3 files changed, 63 insertions(+), 295 deletions(-)

diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
index 888a426b7b41..f632db8283f0 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -21,10 +21,7 @@
         FalconFirmware, //
     },
     gpu,
-    num::{
-        FromSafeCast,
-        IntoSafeCast, //
-    },
+    num::IntoSafeCast,
 };
 
 pub(crate) mod booter;
@@ -411,16 +408,6 @@ fn new(fw: &'a firmware::Firmware) -> Result<Self> {
             .map(|hdr| Self { hdr, fw })
             .ok_or(EINVAL)
     }
-
-    /// Returns the data payload of the firmware, or `None` if the data range is out of bounds of
-    /// the firmware image.
-    fn data(&self) -> Option<&[u8]> {
-        let fw_start = usize::from_safe_cast(self.hdr.data_offset);
-        let fw_size = usize::from_safe_cast(self.hdr.data_size);
-        let fw_end = fw_start.checked_add(fw_size)?;
-
-        self.fw.get(fw_start..fw_end)
-    }
 }
 
 pub(crate) struct ModInfoBuilder<const N: usize>(firmware::ModInfoBuilder<N>);
diff --git a/drivers/gpu/nova-core/firmware/booter.rs b/drivers/gpu/nova-core/firmware/booter.rs
index d9313ac361af..c748aa94cd0e 100644
--- a/drivers/gpu/nova-core/firmware/booter.rs
+++ b/drivers/gpu/nova-core/firmware/booter.rs
@@ -10,8 +10,7 @@
 use kernel::{
     device,
     dma::Coherent,
-    prelude::*,
-    transmute::FromBytes, //
+    prelude::*, //
 };
 
 use crate::{
@@ -25,224 +24,16 @@
         FalconFirmware, //
     },
     firmware::{
-        BinFirmware,
         FirmwareObject,
         FirmwareSignature,
         Signed,
+        Tlv,
         Unsigned, //
     },
     gpu::Chipset,
-    num::{
-        FromSafeCast,
-        IntoSafeCast, //
-    },
+    num::IntoSafeCast,
 };
 
-/// Local convenience function to return a copy of `S` by reinterpreting the bytes starting at
-/// `offset` in `slice`.
-fn frombytes_at<S: FromBytes + Sized>(slice: &[u8], offset: usize) -> Result<S> {
-    let end = offset.checked_add(size_of::<S>()).ok_or(EINVAL)?;
-    slice
-        .get(offset..end)
-        .and_then(S::from_bytes_copy)
-        .ok_or(EINVAL)
-}
-
-/// Heavy-Secured firmware header.
-///
-/// Such firmwares have an application-specific payload that needs to be patched with a given
-/// signature.
-#[repr(C)]
-#[derive(Debug, Clone)]
-struct HsHeaderV2 {
-    /// Offset to the start of the signatures.
-    sig_prod_offset: u32,
-    /// Size in bytes of the signatures.
-    sig_prod_size: u32,
-    /// Offset to a `u32` containing the location at which to patch the signature in the microcode
-    /// image.
-    patch_loc_offset: u32,
-    /// Offset to a `u32` containing the index of the signature to patch.
-    patch_sig_offset: u32,
-    /// Start offset to the signature metadata.
-    meta_data_offset: u32,
-    /// Size in bytes of the signature metadata.
-    meta_data_size: u32,
-    /// Offset to a `u32` containing the number of signatures in the signatures section.
-    num_sig_offset: u32,
-    /// Offset of the application-specific header.
-    header_offset: u32,
-    /// Size in bytes of the application-specific header.
-    header_size: u32,
-}
-
-// SAFETY: all bit patterns are valid for this type, and it doesn't use interior mutability.
-unsafe impl FromBytes for HsHeaderV2 {}
-
-/// Heavy-Secured Firmware image container.
-///
-/// This provides convenient access to the fields of [`HsHeaderV2`] that are actually indices to
-/// read from in the firmware data.
-struct HsFirmwareV2<'a> {
-    hdr: HsHeaderV2,
-    fw: &'a [u8],
-}
-
-impl<'a> HsFirmwareV2<'a> {
-    /// Interprets the header of `bin_fw` as a [`HsHeaderV2`] and returns an instance of
-    /// `HsFirmwareV2` for further parsing.
-    ///
-    /// Fails if the header pointed at by `bin_fw` is not within the bounds of the firmware image.
-    fn new(bin_fw: &BinFirmware<'a>) -> Result<Self> {
-        frombytes_at::<HsHeaderV2>(bin_fw.fw, bin_fw.hdr.header_offset.into_safe_cast())
-            .map(|hdr| Self { hdr, fw: bin_fw.fw })
-    }
-
-    /// Returns the location at which the signatures should be patched in the microcode image.
-    ///
-    /// Fails if the offset of the patch location is outside the bounds of the firmware
-    /// image.
-    fn patch_location(&self) -> Result<u32> {
-        frombytes_at::<u32>(self.fw, self.hdr.patch_loc_offset.into_safe_cast())
-    }
-
-    /// Returns an iterator to the signatures of the firmware. The iterator can be empty if the
-    /// firmware is unsigned.
-    ///
-    /// Fails if the pointed signatures are outside the bounds of the firmware image.
-    fn signatures_iter(&'a self) -> Result<impl Iterator<Item = BooterSignature<'a>>> {
-        let num_sig = frombytes_at::<u32>(self.fw, self.hdr.num_sig_offset.into_safe_cast())?;
-        let iter = match self.hdr.sig_prod_size.checked_div(num_sig) {
-            // If there are no signatures, return an iterator that will yield zero elements.
-            None => (&[] as &[u8]).chunks_exact(1),
-            Some(sig_size) => {
-                let patch_sig =
-                    frombytes_at::<u32>(self.fw, self.hdr.patch_sig_offset.into_safe_cast())?;
-
-                let signatures_start = self
-                    .hdr
-                    .sig_prod_offset
-                    .checked_add(patch_sig)
-                    .map(usize::from_safe_cast)
-                    .ok_or(EINVAL)?;
-
-                let signatures_end = signatures_start
-                    .checked_add(usize::from_safe_cast(self.hdr.sig_prod_size))
-                    .ok_or(EINVAL)?;
-
-                self.fw
-                    // Get signatures range.
-                    .get(signatures_start..signatures_end)
-                    .ok_or(EINVAL)?
-                    .chunks_exact(sig_size.into_safe_cast())
-            }
-        };
-
-        // Map the byte slices into signatures.
-        Ok(iter.map(BooterSignature))
-    }
-}
-
-/// Signature parameters, as defined in the firmware.
-#[repr(C)]
-struct HsSignatureParams {
-    /// Fuse version to use.
-    fuse_ver: u32,
-    /// Mask of engine IDs this firmware applies to.
-    engine_id_mask: u32,
-    /// ID of the microcode.
-    ucode_id: u32,
-}
-
-// SAFETY: all bit patterns are valid for this type, and it doesn't use interior mutability.
-unsafe impl FromBytes for HsSignatureParams {}
-
-impl HsSignatureParams {
-    /// Returns the signature parameters contained in `hs_fw`.
-    ///
-    /// Fails if the meta data parameter of `hs_fw` is outside the bounds of the firmware image, or
-    /// if its size doesn't match that of [`HsSignatureParams`].
-    fn new(hs_fw: &HsFirmwareV2<'_>) -> Result<Self> {
-        let start = usize::from_safe_cast(hs_fw.hdr.meta_data_offset);
-        let end = start
-            .checked_add(hs_fw.hdr.meta_data_size.into_safe_cast())
-            .ok_or(EINVAL)?;
-
-        hs_fw
-            .fw
-            .get(start..end)
-            .and_then(Self::from_bytes_copy)
-            .ok_or(EINVAL)
-    }
-}
-
-/// Header for code and data load offsets.
-#[repr(C)]
-#[derive(Debug, Clone)]
-struct HsLoadHeaderV2 {
-    // Offset at which the code starts.
-    os_code_offset: u32,
-    // Total size of the code, for all apps.
-    os_code_size: u32,
-    // Offset at which the data starts.
-    os_data_offset: u32,
-    // Size of the data.
-    os_data_size: u32,
-    // Number of apps following this header. Each app is described by a [`HsLoadHeaderV2App`].
-    num_apps: u32,
-}
-
-// SAFETY: all bit patterns are valid for this type, and it doesn't use interior mutability.
-unsafe impl FromBytes for HsLoadHeaderV2 {}
-
-impl HsLoadHeaderV2 {
-    /// Returns the load header contained in `hs_fw`.
-    ///
-    /// Fails if the header pointed at by `hs_fw` is not within the bounds of the firmware image.
-    fn new(hs_fw: &HsFirmwareV2<'_>) -> Result<Self> {
-        frombytes_at::<Self>(hs_fw.fw, hs_fw.hdr.header_offset.into_safe_cast())
-    }
-}
-
-/// Header for app code loader.
-#[repr(C)]
-#[derive(Debug, Clone)]
-struct HsLoadHeaderV2App {
-    /// Offset at which to load the app code.
-    offset: u32,
-    /// Length in bytes of the app code.
-    len: u32,
-}
-
-// SAFETY: all bit patterns are valid for this type, and it doesn't use interior mutability.
-unsafe impl FromBytes for HsLoadHeaderV2App {}
-
-impl HsLoadHeaderV2App {
-    /// Returns the [`HsLoadHeaderV2App`] for app `idx` of `hs_fw`.
-    ///
-    /// Fails if `idx` is larger than the number of apps declared in `hs_fw`, or if the header is
-    /// not within the bounds of the firmware image.
-    fn new(hs_fw: &HsFirmwareV2<'_>, idx: u32) -> Result<Self> {
-        let load_hdr = HsLoadHeaderV2::new(hs_fw)?;
-        if idx >= load_hdr.num_apps {
-            Err(EINVAL)
-        } else {
-            frombytes_at::<Self>(
-                hs_fw.fw,
-                usize::from_safe_cast(hs_fw.hdr.header_offset)
-                    // Skip the load header...
-                    .checked_add(size_of::<HsLoadHeaderV2>())
-                    // ... and jump to app header `idx`.
-                    .and_then(|offset| {
-                        offset
-                            .checked_add(usize::from_safe_cast(idx).checked_mul(size_of::<Self>())?)
-                    })
-                    .ok_or(EINVAL)?,
-            )
-        }
-    }
-}
-
 /// Signature for Booter firmware. Their size is encoded into the header and not known a compile
 /// time, so we just wrap a byte slices on which we can implement [`FirmwareSignature`].
 struct BooterSignature<'a>(&'a [u8]);
@@ -292,7 +83,6 @@ pub(crate) fn new(
         dev: &device::Device<device::Bound>,
         kind: BooterKind,
         chipset: Chipset,
-        ver: &str,
         falcon: &Falcon<<Self as FalconFirmware>::Target>,
         bar: Bar0<'_>,
     ) -> Result<Self> {
@@ -300,68 +90,64 @@ pub(crate) fn new(
             BooterKind::Loader => "booter_load",
             BooterKind::Unloader => "booter_unload",
         };
-        let fw = super::request_firmware(dev, chipset, fw_name, ver)?;
-        let bin_fw = BinFirmware::new(&fw)?;
+        let fw = super::request_tlv(dev, chipset, fw_name)?;
+        let tlv = Tlv::new(fw.data())?;
+        dev_info!(dev, "loaded booter firmware v{}\n", tlv.get_string("VERS")?);
+
+        let os_data_offset = tlv.get_u32("DAOF")?;
+        let os_data_size = tlv.get_u32("DASZ")?;
+        let os_code_offset = tlv.get_u32("CDOF")?;
+        let os_code_size = tlv.get_u32("CDSZ")?;
+        let patch_loc = tlv.get_u32("PLOC")?;
+        let fuse_version = tlv.get_u32("FUSE")?;
+        let engine_id = tlv.get_u32("ENID")?;
+        let ucode_id = tlv.get_u32("UCID")?;
+        let app0_code_offset = tlv.get_u32("A0CO")?;
+        let app0_code_size = tlv.get_u32("A0CS")?;
+        let num_sigs = tlv.get_u32("NSIG")?;
 
-        // The binary firmware embeds a Heavy-Secured firmware.
-        let hs_fw = HsFirmwareV2::new(&bin_fw)?;
-
-        // The Heavy-Secured firmware embeds a firmware load descriptor.
-        let load_hdr = HsLoadHeaderV2::new(&hs_fw)?;
-
-        // Offset in `ucode` where to patch the signature.
-        let patch_loc = hs_fw.patch_location()?;
-
-        let sig_params = HsSignatureParams::new(&hs_fw)?;
         let brom_params = FalconBromParams {
-            // `load_hdr.os_data_offset` is an absolute index, but `pkc_data_offset` is from the
+            // `os_data_offset` is an absolute index, but `pkc_data_offset` is from the
             // signature patch location.
-            pkc_data_offset: patch_loc
-                .checked_sub(load_hdr.os_data_offset)
-                .ok_or(EINVAL)?,
-            engine_id_mask: u16::try_from(sig_params.engine_id_mask).map_err(|_| EINVAL)?,
-            ucode_id: u8::try_from(sig_params.ucode_id).map_err(|_| EINVAL)?,
+            pkc_data_offset: patch_loc.checked_sub(os_data_offset).ok_or(EINVAL)?,
+            engine_id_mask: u16::try_from(engine_id).map_err(|_| EINVAL)?,
+            ucode_id: u8::try_from(ucode_id).map_err(|_| EINVAL)?,
         };
-        let app0 = HsLoadHeaderV2App::new(&hs_fw, 0)?;
 
-        // Object containing the firmware microcode to be signature-patched.
-        let ucode = bin_fw
-            .data()
-            .ok_or(EINVAL)
+        let ucode = tlv
+            .get_bytes("BLOB")
             .and_then(FirmwareObject::<Self, _>::new_booter)?;
 
-        let ucode_signed = {
-            let mut signatures = hs_fw.signatures_iter()?.peekable();
-
-            if signatures.peek().is_none() {
-                // If there are no signatures, then the firmware is unsigned.
-                ucode.no_patch_signature()
-            } else {
-                // Obtain the version from the fuse register, and extract the corresponding
-                // signature.
-                let reg_fuse_version = falcon.signature_reg_fuse_version(
-                    bar,
-                    brom_params.engine_id_mask,
-                    brom_params.ucode_id,
-                )?;
+        let ucode_signed = if num_sigs == 0 {
+            // If there are no signatures, then the firmware is unsigned.
+            ucode.no_patch_signature()
+        } else {
+            // Obtain the version from the fuse register, and extract the corresponding
+            // signature.
+            let reg_fuse_version = falcon.signature_reg_fuse_version(
+                bar,
+                brom_params.engine_id_mask,
+                brom_params.ucode_id,
+            )?;
+
+            const FUSE_VERSION_USE_LAST_SIG: u32 = 0;
 
+            let index = match reg_fuse_version {
                 // `0` means the last signature should be used.
-                const FUSE_VERSION_USE_LAST_SIG: u32 = 0;
-                let signature = match reg_fuse_version {
-                    FUSE_VERSION_USE_LAST_SIG => signatures.last(),
-                    // Otherwise hardware fuse version needs to be subtracted to obtain the index.
-                    reg_fuse_version => {
-                        let Some(idx) = sig_params.fuse_ver.checked_sub(reg_fuse_version) else {
-                            dev_err!(dev, "invalid fuse version for Booter firmware\n");
-                            return Err(EINVAL);
-                        };
-                        signatures.nth(idx.into_safe_cast())
-                    }
-                }
+                FUSE_VERSION_USE_LAST_SIG => num_sigs - 1,
+                // Otherwise, hardware fuse version needs to be subtracted to obtain the index.
+                _ => fuse_version.checked_sub(reg_fuse_version).ok_or(EINVAL)?,
+            };
+
+            // The size of one signature
+            let sig_size = tlv
+                .len("SIGS")?
+                .checked_div(num_sigs.into_safe_cast())
                 .ok_or(EINVAL)?;
 
-                ucode.patch_signature(&signature, patch_loc.into_safe_cast())?
-            }
+            let signature = BooterSignature(tlv.get_nth_chunk("SIGS", sig_size, index as usize)?);
+
+            ucode.patch_signature(&signature, patch_loc.into_safe_cast())?
         };
 
         // There are two versions of Booter, one for Turing/GA100, and another for
@@ -370,11 +156,11 @@ pub(crate) fn new(
         // don't indicate the versions.  The only way to differentiate is by the Chipset.
         let (imem_sec_dst_start, imem_ns_load_target) = if chipset <= Chipset::GA100 {
             (
-                app0.offset,
+                app0_code_offset,
                 Some(FalconDmaLoadTarget {
                     src_start: 0,
-                    dst_start: load_hdr.os_code_offset,
-                    len: load_hdr.os_code_size,
+                    dst_start: os_code_offset,
+                    len: os_code_size,
                 }),
             )
         } else {
@@ -383,15 +169,15 @@ pub(crate) fn new(
 
         Ok(Self {
             imem_sec_load_target: FalconDmaLoadTarget {
-                src_start: app0.offset,
+                src_start: app0_code_offset,
                 dst_start: imem_sec_dst_start,
-                len: app0.len,
+                len: app0_code_size,
             },
             imem_ns_load_target,
             dmem_load_target: FalconDmaLoadTarget {
-                src_start: load_hdr.os_data_offset,
+                src_start: os_data_offset,
                 dst_start: 0,
-                len: load_hdr.os_data_size,
+                len: os_data_size,
             },
             brom_params,
             ucode: ucode_signed,
diff --git a/drivers/gpu/nova-core/gsp/hal/tu102.rs b/drivers/gpu/nova-core/gsp/hal/tu102.rs
index eb7166148cc9..bd103cf99662 100644
--- a/drivers/gpu/nova-core/gsp/hal/tu102.rs
+++ b/drivers/gpu/nova-core/gsp/hal/tu102.rs
@@ -27,8 +27,7 @@
             FwsecCommand,
             FwsecFirmware, //
         },
-        gsp::GspFirmware,
-        FIRMWARE_VERSION, //
+        gsp::GspFirmware, //
     },
     gpu::Chipset,
     gsp::{
@@ -113,7 +112,6 @@ fn build(
                     dev,
                     BooterKind::Unloader,
                     chipset,
-                    FIRMWARE_VERSION,
                     sec2_falcon,
                     bar,
                 )?,
@@ -319,15 +317,12 @@ fn boot<'a>(
             "Using SEC2 to load and run the booter_load firmware...\n"
         );
 
-        BooterFirmware::new(
+        BooterFirmware::new(dev, BooterKind::Loader, chipset, sec2_falcon, bar)?.run(
             dev,
-            BooterKind::Loader,
-            chipset,
-            FIRMWARE_VERSION,
-            sec2_falcon,
             bar,
-        )?
-        .run(dev, bar, sec2_falcon, wpr_meta)?;
+            sec2_falcon,
+            wpr_meta,
+        )?;
 
         Ok(unload_guard)
     }
-- 
2.54.0

[PATCH 5/8] gpu: nova-core: transition gsp to TLV images

[Timur Tabi]

Switch the GSP firmware loader from the legacy binary format to the
TLV format.  This change requires the new TLV versions of the r570.144
firmware images.

Unlike the other TLV firmware images, gsp.tlv contains a pointer to
the actual GSP firmware file instead of its contents.  This allows
each small gsp.tlv file to contain the distinct metadata for each GPU
while still allowing the very large gsp.bin to be shared by all
GPUs.

One key piece of metadata is the signature.  The legacy GSP firmware
image is an ELF file that contains multiple sections that needed
to be parsed, and the driver needed to determine which section is
relevant for the GPU.  Instead, gsp.tlv contains the pre-processed
metadata, so all the driver needs to do is to extract it.

Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 drivers/gpu/nova-core/firmware.rs       | 22 --------
 drivers/gpu/nova-core/firmware/gsp.rs   | 56 ++++++-------------
 drivers/gpu/nova-core/firmware/riscv.rs | 71 +++++--------------------
 drivers/gpu/nova-core/gsp/boot.rs       |  7 +--
 4 files changed, 30 insertions(+), 126 deletions(-)

diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
index f632db8283f0..8ef7f084f834 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -387,28 +387,6 @@ struct BinHdr {
 // SAFETY: all bit patterns are valid for this type, and it doesn't use interior mutability.
 unsafe impl FromBytes for BinHdr {}
 
-// A firmware blob starting with a `BinHdr`.
-struct BinFirmware<'a> {
-    hdr: BinHdr,
-    fw: &'a [u8],
-}
-
-impl<'a> BinFirmware<'a> {
-    /// Interpret `fw` as a firmware image starting with a [`BinHdr`], and returns the
-    /// corresponding [`BinFirmware`] that can be used to extract its payload.
-    fn new(fw: &'a firmware::Firmware) -> Result<Self> {
-        const BIN_MAGIC: u32 = 0x10de;
-        let fw = fw.data();
-
-        fw.get(0..size_of::<BinHdr>())
-            // Extract header.
-            .and_then(BinHdr::from_bytes_copy)
-            // Validate header.
-            .filter(|hdr| hdr.bin_magic == BIN_MAGIC)
-            .map(|hdr| Self { hdr, fw })
-            .ok_or(EINVAL)
-    }
-}
 
 pub(crate) struct ModInfoBuilder<const N: usize>(firmware::ModInfoBuilder<N>);
 
diff --git a/drivers/gpu/nova-core/firmware/gsp.rs b/drivers/gpu/nova-core/firmware/gsp.rs
index 99a302bae567..382005be8516 100644
--- a/drivers/gpu/nova-core/firmware/gsp.rs
+++ b/drivers/gpu/nova-core/firmware/gsp.rs
@@ -8,6 +8,7 @@
         DataDirection,
         DmaAddress, //
     },
+    firmware,
     prelude::*,
     scatterlist::{
         Owned,
@@ -17,13 +18,11 @@
 
 use crate::{
     firmware::{
-        elf,
         riscv::RiscvFirmware, //
+        CString,
+        Tlv,
     },
-    gpu::{
-        Architecture,
-        Chipset, //
-    },
+    gpu::Chipset,
     gsp::GSP_PAGE_SIZE,
     num::FromSafeCast,
 };
@@ -63,43 +62,26 @@ pub(crate) struct GspFirmware {
 }
 
 impl GspFirmware {
-    fn find_gsp_sigs_section(chipset: Chipset) -> &'static str {
-        match chipset.arch() {
-            Architecture::Turing if matches!(chipset, Chipset::TU116 | Chipset::TU117) => {
-                ".fwsignature_tu11x"
-            }
-            Architecture::Turing => ".fwsignature_tu10x",
-            Architecture::Ampere if chipset == Chipset::GA100 => ".fwsignature_ga100",
-            Architecture::Ampere => ".fwsignature_ga10x",
-            Architecture::Ada => ".fwsignature_ad10x",
-            Architecture::Hopper => ".fwsignature_gh10x",
-            Architecture::BlackwellGB10x => ".fwsignature_gb10x",
-            Architecture::BlackwellGB20x => ".fwsignature_gb20x",
-        }
-    }
-
     /// Loads the GSP firmware binaries, map them into `dev`'s address-space, and creates the page
     /// tables expected by the GSP bootloader to load it.
     pub(crate) fn new<'a>(
         dev: &'a device::Device<device::Bound>,
         chipset: Chipset,
-        ver: &'a str,
     ) -> impl PinInit<Self, Error> + 'a {
         pin_init::pin_init_scope(move || {
-            let firmware = super::request_firmware(dev, chipset, "gsp", ver)?;
+            let firmware = super::request_tlv(dev, chipset, "gsp")?;
+            let tlv = Tlv::new(firmware.data())?;
+            dev_info!(dev, "loaded gsp firmware v{}\n", tlv.get_string("VERS")?);
 
-            let fw_section = elf::elf_section(firmware.data(), ".fwimage").ok_or(EINVAL)?;
+            let size = usize::from_safe_cast(tlv.get_u32("SIZE")?);
+            let mut fw_vvec = VVec::from_elem(0u8, size, GFP_KERNEL).map_err(|_| ENOMEM)?;
 
-            let size = fw_section.len();
+            let chip_name = chipset.name();
+            let file = tlv.get_string("FILE")?;
+            let filename = CString::try_from_fmt(fmt!("nvidia/{chip_name}/gsp/{file}"))?;
+            firmware::request_into_buf(&filename, dev, fw_vvec.as_mut_slice())?;
 
-            // Move the firmware into a vmalloc'd vector and map it into the device address
-            // space.
-            let fw_vvec = VVec::with_capacity(fw_section.len(), GFP_KERNEL)
-                .and_then(|mut v| {
-                    v.extend_from_slice(fw_section, GFP_KERNEL)?;
-                    Ok(v)
-                })
-                .map_err(|_| ENOMEM)?;
+            let signatures = Coherent::from_slice(dev, tlv.get_bytes("SIGN")?, GFP_KERNEL)?;
 
             Ok(try_pin_init!(Self {
                 fw <- SGTable::new(dev, fw_vvec, DataDirection::ToDevice, GFP_KERNEL),
@@ -145,15 +127,9 @@ pub(crate) fn new<'a>(
                     level0.into()
                 },
                 size,
-                signatures: {
-                    let sigs_section = Self::find_gsp_sigs_section(chipset);
-
-                    elf::elf_section(firmware.data(), sigs_section)
-                        .ok_or(EINVAL)
-                        .and_then(|data| Coherent::from_slice(dev, data, GFP_KERNEL))?
-                },
+                signatures,
                 bootloader: {
-                    let bl = super::request_firmware(dev, chipset, "bootloader", ver)?;
+                    let bl = super::request_tlv(dev, chipset, "gsp_bootloader")?;
 
                     RiscvFirmware::new(dev, &bl)?
                 },
diff --git a/drivers/gpu/nova-core/firmware/riscv.rs b/drivers/gpu/nova-core/firmware/riscv.rs
index 2afa7f36404e..52087b6bc0ed 100644
--- a/drivers/gpu/nova-core/firmware/riscv.rs
+++ b/drivers/gpu/nova-core/firmware/riscv.rs
@@ -7,53 +7,10 @@
     device,
     dma::Coherent,
     firmware::Firmware,
-    prelude::*,
-    transmute::FromBytes, //
+    prelude::*, //
 };
 
-use crate::{
-    firmware::BinFirmware,
-    num::FromSafeCast, //
-};
-
-/// Descriptor for microcode running on a RISC-V core.
-#[repr(C)]
-#[derive(Debug)]
-struct RmRiscvUCodeDesc {
-    version: u32,
-    bootloader_offset: u32,
-    bootloader_size: u32,
-    bootloader_param_offset: u32,
-    bootloader_param_size: u32,
-    riscv_elf_offset: u32,
-    riscv_elf_size: u32,
-    app_version: u32,
-    manifest_offset: u32,
-    manifest_size: u32,
-    monitor_data_offset: u32,
-    monitor_data_size: u32,
-    monitor_code_offset: u32,
-    monitor_code_size: u32,
-}
-
-// SAFETY: all bit patterns are valid for this type, and it doesn't use interior mutability.
-unsafe impl FromBytes for RmRiscvUCodeDesc {}
-
-impl RmRiscvUCodeDesc {
-    /// Interprets the header of `bin_fw` as a [`RmRiscvUCodeDesc`] and returns it.
-    ///
-    /// Fails if the header pointed at by `bin_fw` is not within the bounds of the firmware image.
-    fn new(bin_fw: &BinFirmware<'_>) -> Result<Self> {
-        let offset = usize::from_safe_cast(bin_fw.hdr.header_offset);
-        let end = offset.checked_add(size_of::<Self>()).ok_or(EINVAL)?;
-
-        bin_fw
-            .fw
-            .get(offset..end)
-            .and_then(Self::from_bytes_copy)
-            .ok_or(EINVAL)
-    }
-}
+use crate::firmware::Tlv;
 
 /// A parsed firmware for a RISC-V core, ready to be loaded and run.
 pub(crate) struct RiscvFirmware {
@@ -70,26 +27,22 @@ pub(crate) struct RiscvFirmware {
 }
 
 impl RiscvFirmware {
-    /// Parses the RISC-V firmware image contained in `fw`.
     pub(crate) fn new(dev: &device::Device<device::Bound>, fw: &Firmware) -> Result<Self> {
-        let bin_fw = BinFirmware::new(fw)?;
-
-        let riscv_desc = RmRiscvUCodeDesc::new(&bin_fw)?;
+        let tlv = Tlv::new(fw.data())?;
 
-        let ucode = {
-            let start = usize::from_safe_cast(bin_fw.hdr.data_offset);
-            let len = usize::from_safe_cast(bin_fw.hdr.data_size);
-            let end = start.checked_add(len).ok_or(EINVAL)?;
+        let code_offset = tlv.get_u32("CDOF")?;
+        let data_offset = tlv.get_u32("DAOF")?;
+        let manifest_offset = tlv.get_u32("MFOF")?;
+        let app_version = tlv.get_u32("APPV")?;
 
-            Coherent::from_slice(dev, fw.data().get(start..end).ok_or(EINVAL)?, GFP_KERNEL)?
-        };
+        let ucode = Coherent::from_slice(dev, tlv.get_bytes("BLOB")?, GFP_KERNEL)?;
 
         Ok(Self {
             ucode,
-            code_offset: riscv_desc.monitor_code_offset,
-            data_offset: riscv_desc.monitor_data_offset,
-            manifest_offset: riscv_desc.manifest_offset,
-            app_version: riscv_desc.app_version,
+            code_offset,
+            data_offset,
+            manifest_offset,
+            app_version,
         })
     }
 }
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 8afb62d689cb..84fb426a7113 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -20,10 +20,7 @@
         Falcon, //
     },
     fb::FbLayout,
-    firmware::{
-        gsp::GspFirmware,
-        FIRMWARE_VERSION, //
-    },
+    firmware::gsp::GspFirmware,
     gpu::Chipset,
     gsp::{
         cmdq::Cmdq,
@@ -112,7 +109,7 @@ pub(crate) fn boot(
         let dev = pdev.as_ref();
         let hal = super::hal::gsp_hal(chipset);
 
-        let gsp_fw = KBox::pin_init(GspFirmware::new(dev, chipset, FIRMWARE_VERSION), GFP_KERNEL)?;
+        let gsp_fw = KBox::pin_init(GspFirmware::new(dev, chipset), GFP_KERNEL)?;
 
         let fb_layout = FbLayout::new(chipset, bar, &gsp_fw)?;
         dev_dbg!(dev, "{:#x?}\n", fb_layout);
-- 
2.54.0

[PATCH 6/8] gpu: nova-core: transition gen_bootloader to TLV images

[Timur Tabi]

Switch the generic bootloader firmware loader from the legacy binary
format to the TLV format.  This change requires the new TLV versions
of the r570.144 firmware images.

Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 drivers/gpu/nova-core/firmware.rs             | 22 ------
 .../nova-core/firmware/fwsec/bootloader.rs    | 72 +++++--------------
 2 files changed, 16 insertions(+), 78 deletions(-)

diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
index 8ef7f084f834..fbc9e0cd0021 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -366,28 +366,6 @@ fn no_patch_signature(self) -> FirmwareObject<F, Signed> {
     }
 }
 
-/// Header common to most firmware files.
-#[repr(C)]
-#[derive(Debug, Clone)]
-struct BinHdr {
-    /// Magic number, must be `0x10de`.
-    bin_magic: u32,
-    /// Version of the header.
-    bin_ver: u32,
-    /// Size in bytes of the binary (to be ignored).
-    bin_size: u32,
-    /// Offset of the start of the application-specific header.
-    header_offset: u32,
-    /// Offset of the start of the data payload.
-    data_offset: u32,
-    /// Size in bytes of the data payload.
-    data_size: u32,
-}
-
-// SAFETY: all bit patterns are valid for this type, and it doesn't use interior mutability.
-unsafe impl FromBytes for BinHdr {}
-
-
 pub(crate) struct ModInfoBuilder<const N: usize>(firmware::ModInfoBuilder<N>);
 
 impl<const N: usize> ModInfoBuilder<N> {
diff --git a/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs b/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs
index 039920dc340b..ef80ff19127e 100644
--- a/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs
+++ b/drivers/gpu/nova-core/firmware/fwsec/bootloader.rs
@@ -23,10 +23,7 @@
         Alignment, //
     },
     sizes,
-    transmute::{
-        AsBytes,
-        FromBytes, //
-    },
+    transmute::AsBytes,
 };
 
 use crate::{
@@ -46,38 +43,14 @@
     },
     firmware::{
         fwsec::FwsecFirmware,
-        request_firmware,
-        BinHdr,
-        FIRMWARE_VERSION, //
+        request_tlv,
+        Tlv, //
     },
     gpu::Chipset,
     num::FromSafeCast,
     regs,
 };
 
-/// Descriptor used by RM to figure out the requirements of the boot loader.
-///
-/// Most of its fields appear to be legacy and carry incorrect values, so they are left unused.
-#[repr(C)]
-#[derive(Debug, Clone)]
-struct BootloaderDesc {
-    /// Starting tag of bootloader.
-    start_tag: u32,
-    /// DMEM load offset - unused here as we always load at offset `0`.
-    _dmem_load_off: u32,
-    /// Offset of code section in the image. Unused as there is only one section in the bootloader
-    /// binary.
-    _code_off: u32,
-    /// Size of code section in the image.
-    code_size: u32,
-    /// Offset of data section in the image. Unused as we build the data section ourselves.
-    _data_off: u32,
-    /// Size of data section in the image. Unused as we build the data section ourselves.
-    _data_size: u32,
-}
-// SAFETY: any byte sequence is valid for this struct.
-unsafe impl FromBytes for BootloaderDesc {}
-
 /// Structure used by the boot-loader to load the rest of the code.
 ///
 /// This has to be filled by the GPU driver and copied into DMEM at offset
@@ -150,38 +123,23 @@ pub(crate) fn new(
         dev: &Device<device::Bound>,
         chipset: Chipset,
     ) -> Result<Self> {
-        let fw = request_firmware(dev, chipset, "gen_bootloader", FIRMWARE_VERSION)?;
-        let hdr = fw
-            .data()
-            .get(0..size_of::<BinHdr>())
-            .and_then(BinHdr::from_bytes_copy)
-            .ok_or(EINVAL)?;
-
-        let desc = {
-            let desc_offset = usize::from_safe_cast(hdr.header_offset);
-
-            fw.data()
-                .get(desc_offset..)
-                .and_then(BootloaderDesc::from_bytes_copy_prefix)
-                .ok_or(EINVAL)?
-                .0
-        };
+        let fw = request_tlv(dev, chipset, "gen_bootloader")?;
+        let tlv = Tlv::new(fw.data())?;
+        dev_info!(
+            dev,
+            "loaded generic bootloader firmware v{}\n",
+            tlv.get_string("VERS")?
+        );
 
         let ucode = {
-            let ucode_start = usize::from_safe_cast(hdr.data_offset);
-            let code_size = usize::from_safe_cast(desc.code_size);
-            // Align to falcon block size (256 bytes).
+            let blob = tlv.get_bytes("BLOB")?;
+            let code_size = usize::from_safe_cast(tlv.get_u32("CDSZ")?);
             let aligned_code_size = code_size
                 .align_up(Alignment::new::<{ falcon::MEM_BLOCK_ALIGNMENT }>())
                 .ok_or(EINVAL)?;
 
             let mut ucode = KVec::with_capacity(aligned_code_size, GFP_KERNEL)?;
-            ucode.extend_from_slice(
-                fw.data()
-                    .get(ucode_start..ucode_start + code_size)
-                    .ok_or(EINVAL)?,
-                GFP_KERNEL,
-            )?;
+            ucode.extend_from_slice(blob.get(..code_size).ok_or(EINVAL)?, GFP_KERNEL)?;
             ucode.resize(aligned_code_size, 0, GFP_KERNEL)?;
 
             ucode
@@ -262,13 +220,15 @@ pub(crate) fn new(
             .checked_sub(ucode.len())
             .ok_or(EOVERFLOW)?;
 
+        let start_tag = u16::try_from(tlv.get_u32("STRT")?)?;
+
         Ok(Self {
             _firmware_dma: firmware_dma,
             ucode,
             dmem_desc,
             brom_params: firmware.brom_params(),
             imem_dst_start: u16::try_from(imem_dst_start)?,
-            start_tag: u16::try_from(desc.start_tag)?,
+            start_tag,
         })
     }
 
-- 
2.54.0

[PATCH 7/8] gpu: nova-core: transition fsp to TLV images

[Timur Tabi]

Switch the FSP firmware loaders from the legacy ELF32 format to the new
TLV format.  This change requires the new TLV versions of the r570.144
firmware images.

Because we are no longer loading ELF images, we can also delete the ELF
parser.

Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 drivers/gpu/nova-core/firmware.rs      | 207 -------------------------
 drivers/gpu/nova-core/firmware/fsp.rs  |  84 +++++-----
 drivers/gpu/nova-core/gsp/hal/gh100.rs |   7 +-
 3 files changed, 39 insertions(+), 259 deletions(-)

diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
index fbc9e0cd0021..354945cafda2 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -421,213 +421,6 @@ pub(crate) const fn create(
     }
 }
 
-/// Ad-hoc and temporary module to extract sections from ELF images.
-///
-/// Some firmware images are currently packaged as ELF files, where sections names are used as keys
-/// to specific and related bits of data. Future firmware versions are scheduled to move away from
-/// that scheme before nova-core becomes stable, which means this module will eventually be
-/// removed.
-mod elf {
-    use core::mem::size_of;
-
-    use kernel::{
-        bindings,
-        str::CStr,
-        transmute::FromBytes, //
-    };
-
-    /// Trait to abstract over ELF header differences.
-    trait ElfHeader: FromBytes {
-        fn shnum(&self) -> u16;
-        fn shoff(&self) -> u64;
-        fn shstrndx(&self) -> u16;
-    }
-
-    /// Trait to abstract over ELF section-header differences.
-    trait ElfSectionHeader: FromBytes {
-        fn name(&self) -> u32;
-        fn offset(&self) -> u64;
-        fn size(&self) -> u64;
-    }
-
-    /// Trait describing a matching ELF header and section-header format.
-    trait ElfFormat {
-        type Header: ElfHeader;
-        type SectionHeader: ElfSectionHeader;
-    }
-
-    /// Newtype to provide a [`FromBytes`] implementation.
-    #[repr(transparent)]
-    struct Elf64Hdr(bindings::elf64_hdr);
-    // SAFETY: all bit patterns are valid for this type, and it doesn't use interior mutability.
-    unsafe impl FromBytes for Elf64Hdr {}
-
-    impl ElfHeader for Elf64Hdr {
-        fn shnum(&self) -> u16 {
-            self.0.e_shnum
-        }
-
-        fn shoff(&self) -> u64 {
-            self.0.e_shoff
-        }
-
-        fn shstrndx(&self) -> u16 {
-            self.0.e_shstrndx
-        }
-    }
-
-    #[repr(transparent)]
-    struct Elf64SHdr(bindings::elf64_shdr);
-    // SAFETY: all bit patterns are valid for this type, and it doesn't use interior mutability.
-    unsafe impl FromBytes for Elf64SHdr {}
-
-    impl ElfSectionHeader for Elf64SHdr {
-        fn name(&self) -> u32 {
-            self.0.sh_name
-        }
-
-        fn offset(&self) -> u64 {
-            self.0.sh_offset
-        }
-
-        fn size(&self) -> u64 {
-            self.0.sh_size
-        }
-    }
-
-    struct Elf64Format;
-
-    impl ElfFormat for Elf64Format {
-        type Header = Elf64Hdr;
-        type SectionHeader = Elf64SHdr;
-    }
-
-    /// Newtype to provide [`FromBytes`] and [`ElfHeader`] implementations for ELF32.
-    #[repr(transparent)]
-    struct Elf32Hdr(bindings::elf32_hdr);
-    // SAFETY: all bit patterns are valid for this type, and it doesn't use interior mutability.
-    unsafe impl FromBytes for Elf32Hdr {}
-
-    impl ElfHeader for Elf32Hdr {
-        fn shnum(&self) -> u16 {
-            self.0.e_shnum
-        }
-
-        fn shoff(&self) -> u64 {
-            u64::from(self.0.e_shoff)
-        }
-
-        fn shstrndx(&self) -> u16 {
-            self.0.e_shstrndx
-        }
-    }
-
-    /// Newtype to provide [`FromBytes`] and [`ElfSectionHeader`] implementations for ELF32.
-    #[repr(transparent)]
-    struct Elf32SHdr(bindings::elf32_shdr);
-    // SAFETY: all bit patterns are valid for this type, and it doesn't use interior mutability.
-    unsafe impl FromBytes for Elf32SHdr {}
-
-    impl ElfSectionHeader for Elf32SHdr {
-        fn name(&self) -> u32 {
-            self.0.sh_name
-        }
-
-        fn offset(&self) -> u64 {
-            u64::from(self.0.sh_offset)
-        }
-
-        fn size(&self) -> u64 {
-            u64::from(self.0.sh_size)
-        }
-    }
-
-    struct Elf32Format;
-
-    impl ElfFormat for Elf32Format {
-        type Header = Elf32Hdr;
-        type SectionHeader = Elf32SHdr;
-    }
-
-    /// Returns a NULL-terminated string from the ELF image at `offset`.
-    fn elf_str(elf: &[u8], offset: u64) -> Option<&str> {
-        let idx = usize::try_from(offset).ok()?;
-        let bytes = elf.get(idx..)?;
-        CStr::from_bytes_until_nul(bytes).ok()?.to_str().ok()
-    }
-
-    fn elf_section_generic<'a, F>(elf: &'a [u8], name: &str) -> Option<&'a [u8]>
-    where
-        F: ElfFormat,
-    {
-        let hdr = F::Header::from_bytes(elf.get(0..size_of::<F::Header>())?)?;
-
-        let shdr_num = usize::from(hdr.shnum());
-        let shdr_start = usize::try_from(hdr.shoff()).ok()?;
-        let shdr_end = shdr_num
-            .checked_mul(size_of::<F::SectionHeader>())
-            .and_then(|v| v.checked_add(shdr_start))?;
-
-        // Get all the section headers as an iterator over byte chunks.
-        let shdr_bytes = elf.get(shdr_start..shdr_end)?;
-        let mut shdr_iter = shdr_bytes.chunks_exact(size_of::<F::SectionHeader>());
-
-        // Get the strings table.
-        let strhdr = shdr_iter
-            .clone()
-            .nth(usize::from(hdr.shstrndx()))
-            .and_then(F::SectionHeader::from_bytes)?;
-
-        // Find the section which name matches `name` and return it.
-        shdr_iter.find_map(|sh_bytes| {
-            let sh = F::SectionHeader::from_bytes(sh_bytes)?;
-            let name_offset = strhdr.offset().checked_add(u64::from(sh.name()))?;
-            let section_name = elf_str(elf, name_offset)?;
-
-            if section_name != name {
-                return None;
-            }
-
-            let start = usize::try_from(sh.offset()).ok()?;
-            let end = usize::try_from(sh.size())
-                .ok()
-                .and_then(|sz| start.checked_add(sz))?;
-
-            elf.get(start..end)
-        })
-    }
-
-    /// Extract the section with name `name` from the ELF64 image `elf`.
-    fn elf64_section<'a>(elf: &'a [u8], name: &str) -> Option<&'a [u8]> {
-        elf_section_generic::<Elf64Format>(elf, name)
-    }
-
-    /// Extract the section with name `name` from the ELF32 image `elf`.
-    fn elf32_section<'a>(elf: &'a [u8], name: &str) -> Option<&'a [u8]> {
-        elf_section_generic::<Elf32Format>(elf, name)
-    }
-
-    /// Automatically detects ELF32 vs ELF64 based on the ELF header.
-    pub(super) fn elf_section<'a>(elf: &'a [u8], name: &str) -> Option<&'a [u8]> {
-        // ELF identification: a 4-byte magic followed by a class byte (32- vs 64-bit).
-        const ELFMAG: &[u8] = b"\x7fELF";
-        const SELFMAG: usize = ELFMAG.len();
-        const EI_CLASS: usize = 4;
-        const ELFCLASS32: u8 = 1;
-        const ELFCLASS64: u8 = 2;
-
-        if elf.get(0..SELFMAG) != Some(ELFMAG) {
-            return None;
-        }
-
-        match *elf.get(EI_CLASS)? {
-            ELFCLASS32 => elf32_section(elf, name),
-            ELFCLASS64 => elf64_section(elf, name),
-            _ => None,
-        }
-    }
-}
-
 pub(crate) struct TlvBlock<'a> {
     pub(crate) tag: &'a str,
     pub(crate) value: &'a [u8],
diff --git a/drivers/gpu/nova-core/firmware/fsp.rs b/drivers/gpu/nova-core/firmware/fsp.rs
index 6eaf1c684b9d..2c212b195ddb 100644
--- a/drivers/gpu/nova-core/firmware/fsp.rs
+++ b/drivers/gpu/nova-core/firmware/fsp.rs
@@ -6,12 +6,11 @@
 use kernel::{
     device,
     dma::Coherent,
-    firmware::Firmware,
     prelude::*, //
 };
 
 use crate::{
-    firmware::elf,
+    firmware::Tlv,
     gpu::Chipset, //
 };
 
@@ -19,11 +18,11 @@
 const FSP_HASH_SIZE: usize = 48;
 /// Maximum size of the FSP public key (RSA-3072), in bytes.
 ///
-/// The FMC ELF `publickey` section may be shorter, so the remaining bytes are zero-padded.
+/// The FMC `PKEY` tag may be shorter, so the remaining bytes are zero-padded.
 const FSP_PKEY_SIZE: usize = 384;
 /// Maximum size of the FSP signature (RSA-3072), in bytes.
 ///
-/// The FMC ELF `signature` section may be shorter, so the remaining bytes are zero-padded.
+/// The FMC `SIGS` tag may be shorter, so the remaining bytes are zero-padded.
 const FSP_SIG_SIZE: usize = 384;
 
 /// Structure to hold FMC signatures.
@@ -38,64 +37,34 @@ pub(crate) struct FmcSignatures {
 }
 
 pub(crate) struct FspFirmware {
-    /// FMC firmware image data (only the "image" ELF section).
+    /// FMC firmware image data
     pub(crate) fmc_image: Coherent<[u8]>,
     /// FMC firmware signatures.
     pub(crate) fmc_sigs: KBox<FmcSignatures>,
 }
 
 impl FspFirmware {
-    pub(crate) fn new(
-        dev: &device::Device<device::Bound>,
-        chipset: Chipset,
-        ver: &str,
-    ) -> Result<Self> {
-        let fw = super::request_firmware(dev, chipset, "fmc", ver)?;
+    pub(crate) fn new(dev: &device::Device<device::Bound>, chipset: Chipset) -> Result<Self> {
+        let fw = super::request_tlv(dev, chipset, "fmc")?;
+        let tlv = Tlv::new(fw.data())?;
 
-        // FSP expects only the "image" section, not the entire ELF file.
-        let fmc_image_data = elf::elf_section(fw.data(), "image").ok_or_else(|| {
-            dev_err!(dev, "FMC ELF file missing 'image' section\n");
-            EINVAL
-        })?;
+        let fmc_image_data = tlv.get_bytes("BLOB")?;
         let fmc_image = Coherent::from_slice(dev, fmc_image_data, GFP_KERNEL)?;
 
         Ok(Self {
             fmc_image,
-            fmc_sigs: Self::extract_fmc_signatures(&fw, dev)?,
+            fmc_sigs: Self::extract_fmc_signatures(&tlv, dev)?,
         })
     }
 
     /// Extract FMC firmware signatures for Chain of Trust verification.
     ///
-    /// Extracts real cryptographic signatures from FMC ELF32 firmware sections.
+    /// Extracts real cryptographic signatures from FMC TLV firmware tags.
     /// Returns signatures in a heap-allocated structure to prevent stack overflow.
-    fn extract_fmc_signatures(
-        fmc_fw: &Firmware,
-        dev: &device::Device,
-    ) -> Result<KBox<FmcSignatures>> {
-        let get_section = |name: &str, max_len: usize| {
-            elf::elf_section(fmc_fw.data(), name)
-                .ok_or(EINVAL)
-                .inspect_err(|_| dev_err!(dev, "FMC firmware missing '{}' section\n", name))
-                .and_then(|section| {
-                    if section.len() > max_len {
-                        dev_err!(
-                            dev,
-                            "FMC {} section size {} > maximum {}\n",
-                            name,
-                            section.len(),
-                            max_len
-                        );
-                        Err(EINVAL)
-                    } else {
-                        Ok(section)
-                    }
-                })
-        };
-
-        let hash_section = get_section("hash", FSP_HASH_SIZE)?;
-        let pkey_section = get_section("publickey", FSP_PKEY_SIZE)?;
-        let sig_section = get_section("signature", FSP_SIG_SIZE)?;
+    fn extract_fmc_signatures(tlv: &Tlv<'_>, dev: &device::Device) -> Result<KBox<FmcSignatures>> {
+        let hash_section = tlv.get_bytes("HASH")?;
+        let pkey_section = tlv.get_bytes("PKEY")?;
+        let sig_section = tlv.get_bytes("SIGS")?;
 
         // The hash section is a SHA-384 output: it must be exactly FSP_HASH_SIZE bytes.
         if hash_section.len() != FSP_HASH_SIZE {
@@ -108,15 +77,36 @@ fn extract_fmc_signatures(
             return Err(EINVAL);
         }
 
+        // The key and signature sections are zero-padded to a fixed maximum, so they may be
+        // shorter, but must not exceed the destination buffers.
+        if pkey_section.len() > FSP_PKEY_SIZE {
+            dev_err!(
+                dev,
+                "FMC public key section size {} > maximum {}\n",
+                pkey_section.len(),
+                FSP_PKEY_SIZE
+            );
+            return Err(EINVAL);
+        }
+        if sig_section.len() > FSP_SIG_SIZE {
+            dev_err!(
+                dev,
+                "FMC signature section size {} > maximum {}\n",
+                sig_section.len(),
+                FSP_SIG_SIZE
+            );
+            return Err(EINVAL);
+        }
+
         // Initialize the signatures in place to avoid building the large `FmcSignatures` on the
         // stack, then fill each section from the firmware.
         let signatures = KBox::init(
             pin_init::init_zeroed::<FmcSignatures>().chain(|sigs| {
                 // PANIC: src and dst lengths are both FSP_HASH_SIZE (verified above).
                 sigs.hash384.copy_from_slice(hash_section);
-                // PANIC: dst is sliced to src.len(); src.len() <= FSP_PKEY_SIZE per `get_section`.
+                // PANIC: dst is sliced to src.len(); src.len() <= FSP_PKEY_SIZE (verified above).
                 sigs.public_key[..pkey_section.len()].copy_from_slice(pkey_section);
-                // PANIC: dst is sliced to src.len(); src.len() <= FSP_SIG_SIZE per `get_section`.
+                // PANIC: dst is sliced to src.len(); src.len() <= FSP_SIG_SIZE (verified above).
                 sigs.signature[..sig_section.len()].copy_from_slice(sig_section);
                 Ok(())
             }),
diff --git a/drivers/gpu/nova-core/gsp/hal/gh100.rs b/drivers/gpu/nova-core/gsp/hal/gh100.rs
index 98f5ce197d13..bb4e191bf1b2 100644
--- a/drivers/gpu/nova-core/gsp/hal/gh100.rs
+++ b/drivers/gpu/nova-core/gsp/hal/gh100.rs
@@ -18,10 +18,7 @@
         Falcon, //
     },
     fb::FbLayout,
-    firmware::{
-        fsp::FspFirmware,
-        FIRMWARE_VERSION, //
-    },
+    firmware::fsp::FspFirmware,
     fsp::{
         FmcBootArgs,
         Fsp, //
@@ -160,7 +157,7 @@ fn boot<'a>(
         gsp_falcon: &'a Falcon<GspEngine>,
         sec2_falcon: &'a Falcon<Sec2>,
     ) -> Result<BootUnloadGuard<'a>> {
-        let fsp_fw = FspFirmware::new(dev, chipset, FIRMWARE_VERSION)?;
+        let fsp_fw = FspFirmware::new(dev, chipset)?;
 
         let unload_bundle = crate::gsp::UnloadBundle(
             KBox::new(FspUnloadBundle, GFP_KERNEL)? as KBox<dyn UnloadBundle>
-- 
2.54.0

[PATCH 8/8] gpu: nova-core: update firmware module info for TLV images

[Timur Tabi]

Now that nova-core loads the TLV firmware images, update the firmware
module info to specify those files.

Also remove FIRMWARE_VERSION and request_firmware() as they are no
longer used.

Signed-off-by: Timur Tabi <ttabi@nvidia.com>
---
 drivers/gpu/nova-core/firmware.rs | 35 +++++++------------------------
 1 file changed, 8 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
index 354945cafda2..1149ef0730e9 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -30,23 +30,6 @@
 pub(crate) mod gsp;
 pub(crate) mod riscv;
 
-#[allow(unused)]
-pub(crate) const FIRMWARE_VERSION: &str = "570.144";
-
-/// Requests the GPU firmware `name` suitable for `chipset`, with version `ver`.
-#[allow(unused)]
-fn request_firmware(
-    dev: &device::Device,
-    chipset: gpu::Chipset,
-    name: &str,
-    ver: &str,
-) -> Result<firmware::Firmware> {
-    let chip_name = chipset.name();
-
-    CString::try_from_fmt(fmt!("nvidia/{chip_name}/gsp/{name}-{ver}.bin"))
-        .and_then(|path| firmware::Firmware::request(&path, dev))
-}
-
 /// Requests the GPU firmware TLV `name` suitable for `chipset`.
 #[allow(unused)]
 fn request_tlv(
@@ -376,10 +359,7 @@ const fn make_entry_file(self, chipset: &str, fw: &str) -> Self {
                 .push("nvidia/")
                 .push(chipset)
                 .push("/gsp/")
-                .push(fw)
-                .push("-")
-                .push(FIRMWARE_VERSION)
-                .push(".bin"),
+                .push(fw),
         )
     }
 
@@ -387,20 +367,21 @@ const fn make_entry_chipset(self, chipset: gpu::Chipset) -> Self {
         let name = chipset.name();
 
         let this = self
-            .make_entry_file(name, "bootloader")
-            .make_entry_file(name, "gsp");
+            .make_entry_file(name, "booter_load.tlv")
+            .make_entry_file(name, "gsp.tlv")
+            .make_entry_file(name, "gsp.bin");
 
         // FSP-based chipsets (Hopper, Blackwell and later) boot the GSP via the FMC image loaded by
         // FSP. Older chipsets use the SEC2 booter instead.
         let this = if chipset.uses_fsp() {
-            this.make_entry_file(name, "fmc")
+            this.make_entry_file(name, "fmc.tlv")
         } else {
-            this.make_entry_file(name, "booter_load")
-                .make_entry_file(name, "booter_unload")
+            this.make_entry_file(name, "booter_load.tlv")
+                .make_entry_file(name, "booter_unload.tlv")
         };
 
         if chipset.needs_fwsec_bootloader() {
-            this.make_entry_file(name, "gen_bootloader")
+            this.make_entry_file(name, "gen_bootloader.tlv")
         } else {
             this
         }
-- 
2.54.0

Re: [PATCH 1/8] rust: firmware: add request_into_buf()

[Gary Guo]

On Wed Jun 10, 2026 at 6:49 PM BST, Timur Tabi wrote:
> Add request_into_buf(), a Rust wrapper around the
> request_firmware_into_buf() function. This variant loads the firmware
> image directly into a caller-provided buffer rather than a
> kernel-allocated one.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  rust/kernel/firmware.rs | 45 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 45 insertions(+)
>
> diff --git a/rust/kernel/firmware.rs b/rust/kernel/firmware.rs
> index 71168d8004e2..d2ffaacc2d43 100644
> --- a/rust/kernel/firmware.rs
> +++ b/rust/kernel/firmware.rs
> @@ -120,6 +120,51 @@ fn drop(&mut self) {
>      }
>  }
>  
> +/// Load firmware directly into the caller-provided `buf`.
> +///
> +/// On success the firmware image has been copied into `buf`; the number of bytes
> +/// written is returned. The caller accesses the data through `buf` itself.
> +/// See also `bindings::request_firmware_into_buf`.
> +///
> +/// This is intentionally a stand-alone function rather than a `Firmware` constructor. For
> +/// the `into_buf` path, the firmware data lives in the caller's `buf`, not in a
> +/// kernel-owned buffer, so returning a `Firmware` would expose `Firmware::data()` as an
> +/// alias of `buf`. Since `buf` is `&mut [u8]` and the returned handle does not borrow it,
> +/// the caller could mutate `buf` while reads via `data()` are outstanding (and
> +/// `release_firmware()` does not free `buf` anyway). Releasing the bookkeeping here and
> +/// returning only the size leaves `buf` as the single owner and accessor of the data.
> +pub fn request_into_buf(name: &CStr, dev: &Device, buf: &mut [u8]) -> Result<usize> {

It looks like this API design is very prone to misuse. Patch 5 of this series
for example does not check for the returned size.

Also, I think people might just write `buf[..size]` and then it'll panic because
`size` can be larger.

Best,
Gary

> +    let mut fw: *mut bindings::firmware = core::ptr::null_mut();
> +    let pfw: *mut *mut bindings::firmware = &mut fw;
> +    let pfw: *mut *const bindings::firmware = pfw.cast();
> +
> +    // SAFETY: `pfw` is a valid pointer to a NULL initialized `bindings::firmware` pointer.
> +    // `name` and `dev` are valid as by their type invariants. `buf` is a valid writable
> +    // buffer of `buf.len()` bytes.
> +    let ret = unsafe {
> +        bindings::request_firmware_into_buf(
> +            pfw,
> +            name.as_char_ptr(),
> +            dev.as_raw(),
> +            buf.as_mut_ptr().cast(),
> +            buf.len(),
> +        )
> +    };
> +    if ret != 0 {
> +        return Err(Error::from_errno(ret));
> +    }
> +
> +    // SAFETY: `fw` is a valid pointer returned by `request_firmware_into_buf`.
> +    let size = unsafe { (*fw).size };
> +
> +    // The firmware bytes are now in `buf`, which the caller owns, so we don't need
> +    // the kernel to hang on to it any more.
> +    // SAFETY: `fw` is a valid pointer returned by `request_firmware_into_buf`.
> +    unsafe { bindings::release_firmware(fw) };
> +
> +    Ok(size)
> +}
> +
>  // SAFETY: `Firmware` only holds a pointer to a C `struct firmware`, which is safe to be used from
>  // any thread.
>  unsafe impl Send for Firmware {}

Re: [PATCH 0/8] Transition Nova Core to TLV firmware images

[John Hubbard]

On 6/10/26 10:49 AM, Timur Tabi wrote:
> This patch set transitions nova-core to use the new "TLV" firmware image
> files, instead of the ones that Nouveau uses.
> 
> The current r570.144 images are a mix of binary headers and ELF files that
> are cumbersome to parse in Rust.  There's a significant amount of code
> that just reads in a struct, extracts some offset, and uses it to find
> another struct, only to have nova-core use just a few fields.
> 
> The new format uses a sequence of tag/length/value fields that can be
> iterated over.  The script that generates the TLV files,
> extract-firmware-nova.py, does the extra work to find the specific metadata
> needed by Nova and packages each one separately.
> 
> The TLV versions of r570.144 can be found here:
> 
> 	https://github.com/ttabi/linux-firmware-nova

Hi Timur,

A quick heads up on this, it works for Ampere and Blackwell, but
it is failing on TU117:

<redforge> ktest $ ./tests/nova_basic_tests.py 
PCIe Probe: PASS: Chipset info: all 3 GPUs OK
  ok   0000:c1:00.0: NVIDIA (Chipset: TU117, Architecture: Turing, Revision: a.1)
  ok   0000:c2:00.0: NVIDIA (Chipset: GB202, Architecture: BlackwellGB20x, Revision: a.1)
  ok   0000:01:00.0: NVIDIA (Chipset: GA104, Architecture: Ampere, Revision: a.1)

Get GPU Name: FAIL: GPU name: 2 of 3 GPUs OK, 1 failed
  FAIL 0000:c1:00.0: probe with driver nova-core failed with error -2
  ok   0000:c2:00.0: NVIDIA RTX PRO 6000 Blackwell Max-Q Workstation Edition
  ok   0000:01:00.0: NVIDIA RTX A4000

dmesg:
nova-core 0000:c1:00.0: Direct firmware load for nvidia/tu117/gsp/gsp_bootloader.tlv failed with error -2
nova-core 0000:c1:00.0: probe with driver nova-core failed with error -2

<redforge> firmware $ ll /lib/firmware/nvidia/tu117/gsp/gsp_bootloader.tlv
/bin/ls: cannot access '/lib/firmware/nvidia/tu117/gsp/gsp_bootloader.tlv': No such file or directory

thanks,
-- 
John Hubbard

> 
> along with instructions on how to install them.  We are not planning on
> submitting these images to linux-firmware.  Rather, if this patchset
> is accepted upstream, I expect the small handful of people who are
> actually working on Nova to grab and install these images, which needs
> to be done only once.
> 
> There are still opportunities for improvement.  For example, I would like
> to get rid of more GPU-specific code, especially the GA100 quirks.
> 
> Timur Tabi (8):
>   rust: firmware: add request_into_buf()
>   gpu: nova-core: add request_tlv to load TLV images
>   gpu: nova-core: add TLV parser for firmware files
>   gpu: nova-core: transition booter_load to TLV images
>   gpu: nova-core: transition gsp to TLV images
>   gpu: nova-core: transition gen_bootloader to TLV images
>   gpu: nova-core: transition fsp to TLV images
>   gpu: nova-core: update firmware module info for TLV images
> 
>  Documentation/gpu/nova/core/tlv.rst           | 172 +++++++
>  drivers/gpu/nova-core/firmware.rs             | 451 ++++++++----------
>  drivers/gpu/nova-core/firmware/booter.rs      | 328 +++----------
>  drivers/gpu/nova-core/firmware/fsp.rs         |  84 ++--
>  .../nova-core/firmware/fwsec/bootloader.rs    |  72 +--
>  drivers/gpu/nova-core/firmware/gsp.rs         |  56 +--
>  drivers/gpu/nova-core/firmware/riscv.rs       |  71 +--
>  drivers/gpu/nova-core/gsp/boot.rs             |   7 +-
>  drivers/gpu/nova-core/gsp/hal/gh100.rs        |   7 +-
>  drivers/gpu/nova-core/gsp/hal/tu102.rs        |  15 +-
>  rust/kernel/firmware.rs                       |  45 ++
>  11 files changed, 575 insertions(+), 733 deletions(-)
>  create mode 100644 Documentation/gpu/nova/core/tlv.rst
> 
> 
> base-commit: 9eaff547805f8556992a9474465001c3e128b7bd
> prerequisite-patch-id: 0a9ea098bc3576171da9f6293065fdb4db552466

Re: [PATCH 0/8] Transition Nova Core to TLV firmware images

[Timur Tabi]

On Wed, 2026-06-10 at 11:16 -0700, John Hubbard wrote:
> Get GPU Name: FAIL: GPU name: 2 of 3 GPUs OK, 1 failed
>   FAIL 0000:c1:00.0: probe with driver nova-core failed with error -2
>   ok   0000:c2:00.0: NVIDIA RTX PRO 6000 Blackwell Max-Q Workstation Edition
>   ok   0000:01:00.0: NVIDIA RTX A4000
> 
> dmesg:
> nova-core 0000:c1:00.0: Direct firmware load for nvidia/tu117/gsp/gsp_bootloader.tlv failed with
> error -2
> nova-core 0000:c1:00.0: probe with driver nova-core failed with error -2
> 
> <redforge> firmware $ ll /lib/firmware/nvidia/tu117/gsp/gsp_bootloader.tlv
> /bin/ls: cannot access '/lib/firmware/nvidia/tu117/gsp/gsp_bootloader.tlv': No such file or
> directory

Yeah, I just noticed yesterday that the symlinks() command in my script is broken.  It needs a
symlink for gsp_bootloader.tlv.

I'll fix it soon and force-push an update to the git repo.

Re: [PATCH 1/8] rust: firmware: add request_into_buf()

[Timur Tabi]

On Wed, 2026-06-10 at 19:03 +0100, Gary Guo wrote:
> > +pub fn request_into_buf(name: &CStr, dev: &Device, buf: &mut [u8]) -> Result<usize> {
> 
> It looks like this API design is very prone to misuse. Patch 5 of this series
> for example does not check for the returned size.

I was debating changing it to use request_partial_firmware_into_buf, which would "encourage" users
to verify the return value.

> Also, I think people might just write `buf[..size]` and then it'll panic because
> `size` can be larger.

Why would it panic if the buffer is larger than the size of the file?  The kernel API requires you
to know the size of the file before using request_firmware_into_buf(). If the file on disk is larger
than the buffer, the kernel returns -EFBIG.  If the file on this is smaller, then it is loader
completely and the returned Result<usize> is the number of bytes actually loaded from disk.

This has always been a problem for Nouveau because the GSP binary is 90MB in size, and
request_firmware() allocates a contiguous block of memory for it, only for us to memcpy it to a
vmalloc'd buffer.

So now, thanks to TLVs, we can finally use request_firmware_into_buf() for gsp.bin.

Re: [PATCH 0/8] Transition Nova Core to TLV firmware images

[Timur Tabi]

On Wed, 2026-06-10 at 13:19 -0500, Timur Tabi wrote:
> Yeah, I just noticed yesterday that the symlinks() command in my script is broken.  It needs a
> symlink for gsp_bootloader.tlv.
> 
> I'll fix it soon and force-push an update to the git repo.

Ok, it should be fixed now:

# ls -l /lib/firmware/nvidia/tu117/gsp/
total 108
-rw-rw-r-- 1 ttabi admin 59064 Jun 10 18:48 booter_load.tlv
-rw-rw-r-- 1 ttabi admin 39096 Jun 10 18:48 booter_unload.tlv
lrwxrwxrwx 1 ttabi admin    34 Jun 10 18:48 gen_bootloader.tlv -> ../../tu102/gsp/gen_bootloader.tlv
lrwxrwxrwx 1 ttabi admin    23 Jun 10 18:48 gsp.bin -> ../../tu102/gsp/gsp.bin
-rw-rw-r-- 1 ttabi admin  4152 Jun 10 18:48 gsp.tlv
lrwxrwxrwx 1 ttabi admin    34 Jun 10 18:48 gsp_bootloader.tlv -> ../../tu102/gsp/gsp_bootloader.tlv

Since I did force-push, you will need to this to actually pull it:

git reset HEAD^
git pull

For some reason, `git pull -f` doesn't overwrite local history.

Alternatively, you can just do this:

cd /lib/firmware/nvidia/tu116/gsp/
ln -s ../../tu102/gsp/gsp_bootloader.tlv gsp_bootloader.tlv

This should fix it for tu116 and tu117.

Re: [PATCH 1/8] rust: firmware: add request_into_buf()

[Gary Guo]

On Wed Jun 10, 2026 at 7:24 PM BST, Timur Tabi wrote:
> On Wed, 2026-06-10 at 19:03 +0100, Gary Guo wrote:
>> > +pub fn request_into_buf(name: &CStr, dev: &Device, buf: &mut [u8]) -> Result<usize> {
>> 
>> It looks like this API design is very prone to misuse. Patch 5 of this series
>> for example does not check for the returned size.
>
> I was debating changing it to use request_partial_firmware_into_buf, which would "encourage" users
> to verify the return value.
>
>> Also, I think people might just write `buf[..size]` and then it'll panic because
>> `size` can be larger.
>
> Why would it panic if the buffer is larger than the size of the file?  The kernel API requires you
> to know the size of the file before using request_firmware_into_buf(). If the file on disk is larger
> than the buffer, the kernel returns -EFBIG.  If the file on this is smaller, then it is loader
> completely and the returned Result<usize> is the number of bytes actually loaded from disk.

Right, this is not the partial version of the API, so this would indeed not
happen.

BTW, this thread should have been cc'ed to the RfL list, as the abstraction is
changed.

Best,
Gary

>
> This has always been a problem for Nouveau because the GSP binary is 90MB in size, and
> request_firmware() allocates a contiguous block of memory for it, only for us to memcpy it to a
> vmalloc'd buffer.
>
> So now, thanks to TLVs, we can finally use request_firmware_into_buf() for gsp.bin.

Re: [PATCH 1/8] rust: firmware: add request_into_buf()

[Timur Tabi]

On Wed, 2026-06-10 at 21:26 +0100, Gary Guo wrote:

> > Why would it panic if the buffer is larger than the size of the file?  The kernel API requires
> you
> > to know the size of the file before using request_firmware_into_buf(). If the file on disk is
> larger
> > than the buffer, the kernel returns -EFBIG.  If the file on this is smaller, then it is loader
> > completely and the returned Result<usize> is the number of bytes actually loaded from disk.
> 
> Right, this is not the partial version of the API, so this would indeed not
> happen.

So you don't want me to switch to request_partial_firmware_into_buf?

> BTW, this thread should have been cc'ed to the RfL list, as the abstraction is
> changed.

Oops.  I will do that with v2.

Re: [PATCH 0/8] Transition Nova Core to TLV firmware images

[John Hubbard]

On 6/10/26 11:59 AM, Timur Tabi wrote:
> On Wed, 2026-06-10 at 13:19 -0500, Timur Tabi wrote:
>> Yeah, I just noticed yesterday that the symlinks() command in my script is broken.  It needs a
>> symlink for gsp_bootloader.tlv.
>>
>> I'll fix it soon and force-push an update to the git repo.
> 
> Ok, it should be fixed now:

Yes, all better now:

<redforge> ktest $ ./tests/nova_basic_tests.py 
PCIe Probe: PASS: Chipset info: all 3 GPUs OK
  ok   0000:c1:00.0: NVIDIA (Chipset: TU117, Architecture: Turing, Revision: a.1)
  ok   0000:c2:00.0: NVIDIA (Chipset: GB202, Architecture: BlackwellGB20x, Revision: a.1)
  ok   0000:01:00.0: NVIDIA (Chipset: GA104, Architecture: Ampere, Revision: a.1)

Get GPU Name: PASS: GPU name: all 3 GPUs OK
  ok   0000:c1:00.0: NVIDIA T400 4GB
  ok   0000:c2:00.0: NVIDIA RTX PRO 6000 Blackwell Max-Q Workstation Edition
  ok   0000:01:00.0: NVIDIA RTX A4000


But a follow-up: do you have a quick "undo" command for this? If
not, that's OK, because it's only needed between now and when this
series gets merged, but at the moment I have to bounce back and
forth between reviewing this, and other things that do *not* yet
use TLV.

Just a minor convenience, nothing to waste any real time on of course.

> 
> # ls -l /lib/firmware/nvidia/tu117/gsp/
> total 108
> -rw-rw-r-- 1 ttabi admin 59064 Jun 10 18:48 booter_load.tlv
> -rw-rw-r-- 1 ttabi admin 39096 Jun 10 18:48 booter_unload.tlv
> lrwxrwxrwx 1 ttabi admin    34 Jun 10 18:48 gen_bootloader.tlv -> ../../tu102/gsp/gen_bootloader.tlv
> lrwxrwxrwx 1 ttabi admin    23 Jun 10 18:48 gsp.bin -> ../../tu102/gsp/gsp.bin
> -rw-rw-r-- 1 ttabi admin  4152 Jun 10 18:48 gsp.tlv
> lrwxrwxrwx 1 ttabi admin    34 Jun 10 18:48 gsp_bootloader.tlv -> ../../tu102/gsp/gsp_bootloader.tlv
> 
> Since I did force-push, you will need to this to actually pull it:
> 
> git reset HEAD^
> git pull
> 
> For some reason, `git pull -f` doesn't overwrite local history.

Eh? "git pull -r" works here, even for force-pushed histories, as it
usually does, no need to invent any new steps:

<blueforge> linux-firmware-nova (main)$ git pull -r
remote: Enumerating objects: 3, done.
remote: Total 3 (delta 0), reused 0 (delta 0), pack-reused 3 (from 1)
Unpacking objects: 100% (3/3), 66.03 MiB | 19.29 MiB/s, done.
From https://github.com/ttabi/linux-firmware-nova
 + c2931d3...60f69ef main       -> origin/main  (forced update)
Successfully rebased and updated refs/heads/main.

<blueforge> linux-firmware-nova (main)$ g
60f69eff16b7 nvidia: add v570.144 TLV tarball <Timur Tabi> (Wed, 10 Jun 2026)
1d3cbf7723eb Add README.md LICENCE.nvidia <Timur Tabi> (Fri, 15 May 2026)

And with that, it is all working.

thanks,
-- 
John Hubbard

Re: [PATCH 0/8] Transition Nova Core to TLV firmware images

[Timur Tabi]

On Wed, 2026-06-10 at 14:21 -0700, John Hubbard wrote:
> On 6/10/26 11:59 AM, Timur Tabi wrote:
> > On Wed, 2026-06-10 at 13:19 -0500, Timur Tabi wrote:
> > > Yeah, I just noticed yesterday that the symlinks() command in my script is broken.  It needs a
> > > symlink for gsp_bootloader.tlv.
> > > 
> > > I'll fix it soon and force-push an update to the git repo.
> > 
> > Ok, it should be fixed now:
> 
> Yes, all better now:
> 
> <redforge> ktest $ ./tests/nova_basic_tests.py 
> PCIe Probe: PASS: Chipset info: all 3 GPUs OK
>   ok   0000:c1:00.0: NVIDIA (Chipset: TU117, Architecture: Turing, Revision: a.1)
>   ok   0000:c2:00.0: NVIDIA (Chipset: GB202, Architecture: BlackwellGB20x, Revision: a.1)
>   ok   0000:01:00.0: NVIDIA (Chipset: GA104, Architecture: Ampere, Revision: a.1)
> 
> Get GPU Name: PASS: GPU name: all 3 GPUs OK
>   ok   0000:c1:00.0: NVIDIA T400 4GB
>   ok   0000:c2:00.0: NVIDIA RTX PRO 6000 Blackwell Max-Q Workstation Edition
>   ok   0000:01:00.0: NVIDIA RTX A4000
> 
> 
> But a follow-up: do you have a quick "undo" command for this? If
> not, that's OK, because it's only needed between now and when this
> series gets merged, but at the moment I have to bounce back and
> forth between reviewing this, and other things that do *not* yet
> use TLV.

I'll see if I can add a clean.sh script, but in the meantime, Gemini has a few ideas:
https://gemini.google.com/share/a4b48287389d

> Just a minor convenience, nothing to waste any real time on of course.

It's bitten me also, so it's a good idea.

> > 
> > Since I did force-push, you will need to this to actually pull it:
> > 
> > git reset HEAD^
> > git pull
> > 
> > For some reason, `git pull -f` doesn't overwrite local history.
> 
> Eh? "git pull -r" works here, even for force-pushed histories, as it
> usually does, no need to invent any new steps:

Learn something new every day, thanks.

Re: [PATCH 1/8] rust: firmware: add request_into_buf()

[Danilo Krummrich]

On Wed Jun 10, 2026 at 10:26 PM CEST, Gary Guo wrote:
> BTW, this thread should have been cc'ed to the RfL list, as the abstraction is
> changed.

FIRMWARE LOADER uses the driver-core list, so please Cc:
driver-core@lists.linux.dev and the other FW loader maintainers.

Re: [PATCH 2/8] gpu: nova-core: add request_tlv to load TLV images

[Danilo Krummrich]

On Wed Jun 10, 2026 at 7:49 PM CEST, Timur Tabi wrote:
> Add function request_tlv() to load TLV firmware images.
>
> 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.
>
> Also add some #[allow(unused)] statements to two identifiers that will
> soon be removed, to keep the patchset clean.
>
> Signed-off-by: Timur Tabi <ttabi@nvidia.com>
> ---
>  drivers/gpu/nova-core/firmware.rs | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
> index 279fbacd0b8e..2749c196416d 100644
> --- a/drivers/gpu/nova-core/firmware.rs
> +++ b/drivers/gpu/nova-core/firmware.rs
> @@ -33,9 +33,11 @@
>  pub(crate) mod gsp;
>  pub(crate) mod riscv;
>  
> +#[allow(unused)]

This shouldn't be needed, it can just be removed when once it becomes unused,
no?

>  pub(crate) const FIRMWARE_VERSION: &str = "570.144";
>  
>  /// Requests the GPU firmware `name` suitable for `chipset`, with version `ver`.
> +#[allow(unused)]
>  fn request_firmware(
>      dev: &device::Device,
>      chipset: gpu::Chipset,
> @@ -48,6 +50,21 @@ fn request_firmware(
>          .and_then(|path| firmware::Firmware::request(&path, dev))
>  }
>  
> +/// Requests the GPU firmware TLV `name` suitable for `chipset`.
> +#[allow(unused)]

Please use expect instead.

> +fn request_tlv(
> +    dev: &device::Device,
> +    chipset: gpu::Chipset,
> +    name: &str,
> +) -> Result<firmware::Firmware> {
> +    let chip_name = chipset.name();
> +
> +    dev_info!(dev, "loading firmware image {}.tlv\n", name);

Please use dev_dbg!() instead, this should not be printed under normal
operation.

> +    CString::try_from_fmt(fmt!("nvidia/{chip_name}/gsp/{name}.tlv"))
> +        .and_then(|path| firmware::Firmware::request(&path, dev))
> +}
> +
>  /// Structure used to describe some firmwares, notably FWSEC-FRTS.
>  #[repr(C)]
>  #[derive(Debug, Clone)]
> -- 
> 2.54.0

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

[Danilo Krummrich]

On Wed Jun 10, 2026 at 7:49 PM CEST, Timur Tabi wrote:
> diff --git a/Documentation/gpu/nova/core/tlv.rst b/Documentation/gpu/nova/core/tlv.rst
> new file mode 100644
> index 000000000000..8223605daa6b
> --- /dev/null
> +++ b/Documentation/gpu/nova/core/tlv.rst
> @@ -0,0 +1,172 @@
> +.. SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +
> +==================================
> +TLV Tags in Nova Firmware Images
> +==================================

Thanks for adding this!

> diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
> index 2749c196416d..888a426b7b41 100644
> --- a/drivers/gpu/nova-core/firmware.rs
> +++ b/drivers/gpu/nova-core/firmware.rs
> @@ -11,7 +11,7 @@
>      device,
>      firmware,
>      prelude::*,
> -    str::CString,
> +    str::{CStr, CString},

NIT: format

>      transmute::FromBytes, //
>  };
>  
> @@ -684,3 +684,240 @@ pub(super) fn elf_section<'a>(elf: &'a [u8], name: &str) -> Option<&'a [u8]> {
>          }
>      }
>  }
> +
> +pub(crate) struct TlvBlock<'a> {
> +    pub(crate) tag: &'a str,
> +    pub(crate) value: &'a [u8],
> +}
> +
> +/// On-wire TLV block header: 4-byte ASCII tag + little-endian payload length (bytes, excluding
> +/// padding to a 4-byte boundary).
> +struct TlvBlockHeader<'a> {
> +    tag: &'a str,
> +    length: usize,
> +}
> +
> +impl<'a> TlvBlockHeader<'a> {
> +    const SIZE: usize = size_of::<[u8; 4]>() + size_of::<u32>();
> +
> +    /// Parses the first [`Self::SIZE`] bytes of `hdr` (caller may pass a longer slice).
> +    fn parse(hdr: &'a [u8]) -> Option<Self> {
> +        let hdr = hdr.get(..Self::SIZE)?;
> +        let tag_bytes = hdr.get(..4)?;
> +        let tag = core::str::from_utf8(tag_bytes).ok()?;
> +        if !tag.is_ascii() {
> +            return None;
> +        }
> +        let len_arr = <[u8; 4]>::try_from(hdr.get(4..Self::SIZE)?).ok()?;
> +        let length = u32::from_le_bytes(len_arr) as usize;

Here and in a couple other cases, nova-core depends on !CPU_BIG_ENDIAN, so that
shouldn't be necessary.

I'm not concerned to keep it for documentation purposes, but it may be a bit
inconsistent with the rest of the driver.

> +        Some(Self { tag, length })
> +    }
> +}
> +
> +/// Sequential scan over [`Tlv`]. Unlike [`Tlv`], this type carries a cursor (`pos`) into
> +/// the parent blob.  It is not interchangeable with a fresh [`Tlv`] view of the same
> +/// bytes.
> +///
> +/// # Invariants
> +///
> +/// `pos` is a byte offset into `tlv.data` that always lies on a block boundary (in the sense
> +/// of the [`Tlv`] invariant): it is either the start of a well-formed block, or equal to
> +/// `tlv.data.len()` (end of iteration).
> +struct TlvIter<'tlv, 'a> {
> +    tlv: &'tlv Tlv<'a>,
> +    pos: usize,
> +}
> +
> +impl<'tlv, 'a> Iterator for TlvIter<'tlv, 'a> {
> +    type Item = TlvBlock<'a>;
> +
> +    /// Returns the block starting at `self.pos` and advances the cursor past it, or [`None`]
> +    /// once the cursor reaches the end of the data.
> +    ///
> +    /// The body relies on a number of `unsafe` operations, because [`Iterator::next`] can
> +    /// only return [`Option`], not [`Result`], so there is no way to report a parse error
> +    /// from here.  That is why the entire TLV stream is validated up front in the
> +    /// constructor.  By this type's invariants, `self.pos` is always on a block boundary
> +    /// into already-validated data, so each block is known to be well-formed, and
> +    /// therefore the unchecked operations cannot fail.
> +    fn next(&mut self) -> Option<Self::Item> {
> +        if self.pos >= self.tlv.data.len() {
> +            return None;
> +        }
> +
> +        let tail = &self.tlv.data[self.pos..];
> +
> +        // SAFETY: `self.pos` is on a block boundary (`TlvIter` invariant) and the check
> +        // above gives `self.pos < data.len()`, so by the `Tlv` invariant a complete
> +        // well-formed block starts at `tail`.
> +        let hdr = unsafe { tail.get_unchecked(..TlvBlockHeader::SIZE) };
> +
> +        // SAFETY: same header bytes as validated in `Tlv::new` for this offset.
> +        let header = unsafe { TlvBlockHeader::parse(hdr).unwrap_unchecked() };
> +
> +        let stored_size = header.length.next_multiple_of(4);
> +        let advance = TlvBlockHeader::SIZE + stored_size;
> +        let payload_end = TlvBlockHeader::SIZE + header.length;
> +
> +        // SAFETY: `advance` and `payload_end` are exactly the stored and logical payload extents
> +        // `Tlv::new` accepted for this block.
> +        let value = unsafe {
> +            let block = tail.get_unchecked(..advance);
> +            block.get_unchecked(TlvBlockHeader::SIZE..payload_end)
> +        };

Given that this method returns an Option anyways, this could just use the
corresponding safe accessors, e.g.

	let hdr = tail.get(..TlvBlockHeader::SIZE)?;

Alternatively, we can just use panicking accessors with a '// PANIC' comment,
but again, since the function already returns an Option, this isn't a hot path,
and the panic comments aren't obviously trivial, it might not be worth it in
this case.

That said, I'm fine with either safe or panicking, but I don't like the unsafe
accessors.

> +
> +        // INVARIANT: by the `Tlv` invariant the block at `self.pos` occupies exactly `advance`
> +        // bytes, so `self.pos + advance` is the next block boundary (or `data.len()`).
> +        self.pos += advance;
> +
> +        Some(TlvBlock {
> +            tag: header.tag,
> +            value,
> +        })
> +    }
> +}
> +
> +/// The payload of a validated TLV (type, length, value) firmware image.
> +///
> +/// TLV firmware images start with a 4-byte "NVFW" magic header, followed by a sequence of
> +/// blocks. Each block has a 4-byte type tag, a 4-byte length field, and a data payload
> +/// whose stored size is the length rounded up to the nearest multiple of 4.
> +///
> +/// [`Self::new`] checks the magic header and walks every block: tags must be ASCII,
> +/// lengths and padding must fit without overflow, and the byte stream after `NVFW` must
> +/// be exactly partitionable into blocks (no trailing partial header or slack). After
> +/// that, [`TlvIter`] only signals end-of-stream via [`None`], not parse failure.
> +///
> +/// # Invariants
> +///
> +/// `data` is a validated TLV payload (the bytes *after* the `NVFW` magic): it is the exact
> +/// concatenation of zero or more well-formed blocks, with no trailing partial header or slack.
> +/// Consequently, any offset `o` into `data` that is a block boundary and satisfies
> +/// `o < data.len()` is the start of a complete block whose header parses and whose stored
> +/// extent (`TlvBlockHeader::SIZE + header.length.next_multiple_of(4)` bytes) lies within
> +/// `data`. `data.len()` is itself a boundary.
> +#[allow(dead_code)]
> +pub(crate) struct Tlv<'a> {
> +    data: &'a [u8],
> +}
> +
> +#[allow(dead_code)]
> +impl<'a> Tlv<'a> {
> +    const MAGIC: &'static [u8; 4] = b"NVFW";
> +
> +    /// Parses `data` as a TLV firmware image, returning [`EINVAL`] if the image is malformed.
> +    pub(crate) fn new(data: &'a [u8]) -> Result<Self> {
> +        // Verify that the magic bytes exist and are the correct value
> +        let magic_len = Self::MAGIC.len();
> +        if data
> +            .get(..magic_len)
> +            .is_none_or(|magic| magic != Self::MAGIC)
> +        {
> +            return Err(EINVAL);
> +        }
> +
> +        // The payload is the contiguous sequence of TLV blocks after the magic.
> +        let payload = data.get(magic_len..).ok_or(EINVAL)?;
> +
> +        let mut pos = 0usize;
> +        while pos < payload.len() {
> +            // Get the next TLV block.
> +            let Some(rest) = payload.get(pos..) else {

This check doesn't seem to be needed because the loop constraints it directly
above, so I'd use let rest = &payload[pos..] instead in this case.

> +                return Err(EINVAL);
> +            };
> +            // Validate and extract the header (type, length).
> +            let Some(header) = rest
> +                .get(..TlvBlockHeader::SIZE)
> +                .and_then(TlvBlockHeader::parse)
> +            else {
> +                return Err(EINVAL);
> +            };
> +            // The `length` field of a TLV block contains the actual byte length of the
> +            // value, but each TLV block is aligned to a 4-byte boundary.
> +            let Some(stored_size) = header.length.checked_next_multiple_of(4) else {
> +                return Err(EINVAL);
> +            };
> +            let end = pos
> +                .checked_add(TlvBlockHeader::SIZE)
> +                .and_then(|p| p.checked_add(stored_size))
> +                .ok_or(EINVAL)?;
> +            if end > payload.len() {
> +                return Err(EINVAL);
> +            }
> +            pos = end;
> +        }
> +
> +        // INVARIANT: the loop above walked `payload` from offset 0 and only stopped once `pos`
> +        // reached `payload.len()` exactly, rejecting any block whose header failed to parse or
> +        // whose stored extent overran. So `payload` is an exact concatenation of well-formed
> +        // blocks with no partial header or trailing slack.
> +        Ok(Self { data: payload })
> +    }
> +
> +    fn iter(&self) -> TlvIter<'_, 'a> {
> +        // INVARIANT: 0 is a block boundary, either the start of the first block,
> +        // or `data.len()` when `data` is empty.
> +        TlvIter { tlv: self, pos: 0 }
> +    }
> +
> +    pub(crate) fn len(&self, tag: &str) -> Result<usize> {
> +        let tlv = self.iter().find(|b| b.tag == tag).ok_or(EINVAL)?;
> +
> +        Ok(tlv.value.len())
> +    }
> +
> +    pub(crate) fn get_bytes(&self, tag: &str) -> Result<&'a [u8]> {
> +        let tlv = self.iter().find(|b| b.tag == tag).ok_or(EINVAL)?;
> +
> +        Ok(tlv.value)
> +    }
> +
> +    pub(crate) fn get_u32(&self, tag: &str) -> Result<u32> {
> +        let tlv = self.iter().find(|b| b.tag == tag).ok_or(EINVAL)?;
> +
> +        tlv.value
> +            .try_into()
> +            .ok()
> +            .map(u32::from_le_bytes)
> +            .ok_or(EINVAL)
> +    }
> +
> +    pub(crate) fn get_string(&self, tag: &str) -> Result<&'a str> {
> +        let tlv = self.iter().find(|b| b.tag == tag).ok_or(EINVAL)?;
> +
> +        // Handle the possibility that the value is null-terminated.
> +        let bytes = match CStr::from_bytes_until_nul(tlv.value) {
> +            Ok(cstr) => cstr.to_bytes(),
> +            Err(_) => tlv.value,
> +        };
> +
> +        // But do require it to be all ASCII.
> +        if !bytes.is_ascii() {
> +            return Err(EINVAL);
> +        }
> +
> +        core::str::from_utf8(bytes).map_err(|_| EINVAL)
> +    }

Would it make sense to provide those accessors on TlvBlock, so we don't have to
create a fresh TlvIter and scan from the start every time?

> +
> +    /// Returns the `n`-th fixed-size chunk from the value of the TLV entry matching
> +    /// `tag`.
> +    ///
> +    /// The value is treated as an array of contiguous, equally-sized chunks, each
> +    /// `sig_size` bytes long. `sig_size` is the size of one signature.  The returned
> +    /// slice is the `n`-th chunk.
> +    ///
> +    /// Returns [`EINVAL`] if no entry matches `tag`, if computing the chunk's offset
> +    /// overflows, or if the value is too short to contain the requested chunk.
> +    pub(crate) fn get_nth_chunk(&self, tag: &str, sig_size: usize, n: usize) -> Result<&'a [u8]> {
> +        let sigs = self
> +            .iter()
> +            .find(|b| b.tag == tag)
> +            .map(|b| b.value)
> +            .ok_or(EINVAL)?;
> +
> +        let start = sig_size.checked_mul(n).ok_or(EINVAL)?;
> +        let end = start.checked_add(sig_size).ok_or(EINVAL)?;
> +
> +        sigs.get(start..end).ok_or(EINVAL)
> +    }
> +}
> -- 
> 2.54.0