[PATCH v3 00/11] gpu: nova-core: vbios: harden various array accesses and refactor

[PATCH v3 00/11] gpu: nova-core: vbios: harden various array accesses and refactor

[Eliot Courtney]

We have some code that accesses arrays based on values from firmware.
This patch series makes a bunch of those accesses more robust. This
series only touches accesses that are not guaranteed to be safe by local
invariants - some accesses are safe due to earlier checks and I haven't
modified those.

This series also refactors and removes some code that can be simplified.
In particular, it removes `FwSecBiosBuilder`. It also adds some more
stringent checking for PCI-AT and FWSEC images so duplicate ones will
result in an error.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
Changes in v3:
- Use first PCI-AT and FWSEC images instead of erroring.
- Expand commit messages.
- Add Joel's Reviewed-by's (thanks!)
- Link to v2: https://patch.msgid.link/20260414-fix-vbios-v2-0-705d30d16bba@nvidia.com

Changes in v2:
- Add Joel's reviewed-by tags.
- Remove unnecessary code like `falcon_data_offset` from
  `FwSecBiosBuilder`
- Push offset handling into `falcon_data_ptr` (renamed)
- Simplify `setup_falcon_data`
- Add checking for spurious PCI-AT and FWSEC images.
- Remove `FwSecBiosBuilder`
- Link to v1: https://patch.msgid.link/20260410-fix-vbios-v1-0-bc6f71d153d6@nvidia.com

---
Eliot Courtney (11):
      gpu: nova-core: vbios: fix various cases of reading past `BIOS_MAX_SCAN_LEN`
      gpu: nova-core: vbios: limit `BitToken` entry reads
      gpu: nova-core: vbios: use checked ops and accesses in `FwSecBiosImage::ucode`
      gpu: nova-core: vbios: use checked access in `FwSecBiosImage::header`
      gpu: nova-core: vbios: use checked accesses in `setup_falcon_data`
      gpu: nova-core: vbios: drop unused falcon_data_offset from FwSecBiosBuilder
      gpu: nova-core: vbios: keep PmuLookupTable local in setup_falcon_data
      gpu: nova-core: vbios: compute FWSEC-relative Falcon data offset
      gpu: nova-core: vbios: simplify setup_falcon_data
      gpu: nova-core: vbios: construct `FwSecBiosImage` directly from BIOS images
      gpu: nova-core: vbios: use the first PCI-AT and FWSEC images

 drivers/gpu/nova-core/vbios.rs | 300 +++++++++++++++++------------------------
 1 file changed, 125 insertions(+), 175 deletions(-)
---
base-commit: a7a080bb4236ebe577b6776d940d1717912ff6dd
change-id: 20260409-fix-vbios-d668e9c21d23

Best regards,
--  
Eliot Courtney <ecourtney@nvidia.com>

[PATCH v3 01/11] gpu: nova-core: vbios: fix various cases of reading past `BIOS_MAX_SCAN_LEN`

[Eliot Courtney]

Fix various cases that allow reading past `BIOS_MAX_SCAN_LEN` when
scanning the VBIOS.

Fix bug where `read_more_at_offset` would unnecessarily read more data.
This happens when the window to read has some part cached and some part
not. It would read `len` bytes instead of just the uncached portion,
which could read past `BIOS_MAX_SCAN_LEN`.

Also add more checked arithmetic to catch potential overflows.
`read_bios_image_at_offset` is called with a length from the VBIOS
header, so we should be more defensive here.

Fixes: 6fda04e7f0cd ("gpu: nova-core: vbios: Add base support for VBIOS construction and iteration")
Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index ebda28e596c5..6de7e58e0da0 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -132,17 +132,14 @@ fn read_more(&mut self, len: usize) -> Result {
 
     /// Read bytes at a specific offset, filling any gap.
     fn read_more_at_offset(&mut self, offset: usize, len: usize) -> Result {
-        if offset > BIOS_MAX_SCAN_LEN {
+        let end = offset.checked_add(len).ok_or(EINVAL)?;
+
+        if end > BIOS_MAX_SCAN_LEN {
             dev_err!(self.dev, "Error: exceeded BIOS scan limit.\n");
             return Err(EINVAL);
         }
 
-        // If `offset` is beyond current data size, fill the gap first.
-        let current_len = self.data.len();
-        let gap_bytes = offset.saturating_sub(current_len);
-
-        // Now read the requested bytes at the offset.
-        self.read_more(gap_bytes + len)
+        self.read_more(end.saturating_sub(self.data.len()))
     }
 
     /// Read a BIOS image at a specific offset and create a [`BiosImage`] from it.
@@ -155,8 +152,9 @@ fn read_bios_image_at_offset(
         len: usize,
         context: &str,
     ) -> Result<BiosImage> {
+        let end = offset.checked_add(len).ok_or(EINVAL)?;
         let data_len = self.data.len();
-        if offset + len > data_len {
+        if end > data_len {
             self.read_more_at_offset(offset, len).inspect_err(|e| {
                 dev_err!(
                     self.dev,
@@ -167,7 +165,7 @@ fn read_bios_image_at_offset(
             })?;
         }
 
-        BiosImage::new(self.dev, &self.data[offset..offset + len]).inspect_err(|err| {
+        BiosImage::new(self.dev, &self.data[offset..end]).inspect_err(|err| {
             dev_err!(
                 self.dev,
                 "Failed to {} at offset {:#x}: {:?}\n",
@@ -189,7 +187,7 @@ fn next(&mut self) -> Option<Self::Item> {
             return None;
         }
 
-        if self.current_offset > BIOS_MAX_SCAN_LEN {
+        if self.current_offset >= BIOS_MAX_SCAN_LEN {
             dev_err!(self.dev, "Error: exceeded BIOS scan limit, stopping scan\n");
             return None;
         }

-- 
2.53.0

[PATCH v3 02/11] gpu: nova-core: vbios: limit `BitToken` entry reads

[Eliot Courtney]

If `header.token_size` is smaller than `BitToken`, then we currently can
read past the end of `image.base.data`. Check that the token size is at
least as big as `BitToken`.

Fixes: dc70c6ae2441 ("gpu: nova-core: vbios: Add support to look up PMU table in FWSEC")
Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 34 +++++++++++++++++-----------------
 1 file changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 6de7e58e0da0..de856000de23 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -423,31 +423,31 @@ impl BitToken {
     /// Find a BIT token entry by BIT ID in a PciAtBiosImage
     fn from_id(image: &PciAtBiosImage, token_id: u8) -> Result<Self> {
         let header = &image.bit_header;
+        let entry_size = usize::from(header.token_size);
+
+        if entry_size < size_of::<BitToken>() {
+            return Err(EINVAL);
+        }
 
         // Offset to the first token entry
         let tokens_start = image.bit_offset + usize::from(header.header_size);
 
         for i in 0..usize::from(header.token_entries) {
-            let entry_offset = tokens_start + (i * usize::from(header.token_size));
-
-            // Make sure we don't go out of bounds
-            if entry_offset + usize::from(header.token_size) > image.base.data.len() {
-                return Err(EINVAL);
-            }
+            let entry_offset = tokens_start + (i * entry_size);
+            let entry = image
+                .base
+                .data
+                .get(entry_offset..)
+                .and_then(|data| data.get(..entry_size))
+                .ok_or(EINVAL)?;
 
             // Check if this token has the requested ID
-            if image.base.data[entry_offset] == token_id {
+            if entry[0] == token_id {
                 return Ok(BitToken {
-                    id: image.base.data[entry_offset],
-                    data_version: image.base.data[entry_offset + 1],
-                    data_size: u16::from_le_bytes([
-                        image.base.data[entry_offset + 2],
-                        image.base.data[entry_offset + 3],
-                    ]),
-                    data_offset: u16::from_le_bytes([
-                        image.base.data[entry_offset + 4],
-                        image.base.data[entry_offset + 5],
-                    ]),
+                    id: entry[0],
+                    data_version: entry[1],
+                    data_size: u16::from_le_bytes([entry[2], entry[3]]),
+                    data_offset: u16::from_le_bytes([entry[4], entry[5]]),
                 });
             }
         }

-- 
2.53.0

[PATCH v3 03/11] gpu: nova-core: vbios: use checked ops and accesses in `FwSecBiosImage::ucode`

[Eliot Courtney]

Use checked arithmetic and access for extracting the microcode since the
offsets are firmware derived.

Fixes: 47c4846e4319 ("gpu: nova-core: vbios: Add support for FWSEC ucode extraction")
Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index de856000de23..632c8a90ea76 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -1029,16 +1029,21 @@ pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
 
     /// Get the ucode data as a byte slice
     pub(crate) fn ucode(&self, desc: &FalconUCodeDesc) -> Result<&[u8]> {
-        let falcon_ucode_offset = self.falcon_ucode_offset;
-
         // The ucode data follows the descriptor.
-        let ucode_data_offset = falcon_ucode_offset + desc.size();
-        let size = usize::from_safe_cast(desc.imem_load_size() + desc.dmem_load_size());
+        let data = self
+            .base
+            .data
+            .get(self.falcon_ucode_offset..)
+            .ok_or(ERANGE)?;
+        let size = usize::from_safe_cast(
+            desc.imem_load_size()
+                .checked_add(desc.dmem_load_size())
+                .ok_or(ERANGE)?,
+        );
 
         // Get the data slice, checking bounds in a single operation.
-        self.base
-            .data
-            .get(ucode_data_offset..ucode_data_offset + size)
+        data.get(desc.size()..)
+            .and_then(|data| data.get(..size))
             .ok_or(ERANGE)
             .inspect_err(|_| {
                 dev_err!(

-- 
2.53.0

[PATCH v3 04/11] gpu: nova-core: vbios: use checked access in `FwSecBiosImage::header`

[Eliot Courtney]

Use checked access in `FwSecBiosImage::header` for getting the header
version since the value is firmware derived.

Fixes: 47c4846e4319 ("gpu: nova-core: vbios: Add support for FWSEC ucode extraction")
Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 632c8a90ea76..bc752d135cbf 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -996,17 +996,14 @@ fn build(self) -> Result<FwSecBiosImage> {
 impl FwSecBiosImage {
     /// Get the FwSec header ([`FalconUCodeDesc`]).
     pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
-        // Get the falcon ucode offset that was found in setup_falcon_data.
-        let falcon_ucode_offset = self.falcon_ucode_offset;
+        let data = self
+            .base
+            .data
+            .get(self.falcon_ucode_offset..)
+            .ok_or(EINVAL)?;
 
-        // Read the first 4 bytes to get the version.
-        let hdr_bytes: [u8; 4] = self.base.data[falcon_ucode_offset..falcon_ucode_offset + 4]
-            .try_into()
-            .map_err(|_| EINVAL)?;
-        let hdr = u32::from_le_bytes(hdr_bytes);
-        let ver = (hdr & 0xff00) >> 8;
-
-        let data = self.base.data.get(falcon_ucode_offset..).ok_or(EINVAL)?;
+        // Read the version byte from the header.
+        let ver = data.get(1).copied().ok_or(EINVAL)?;
         match ver {
             2 => {
                 let v2 = FalconUCodeDescV2::from_bytes_copy_prefix(data)

-- 
2.53.0

[PATCH v3 05/11] gpu: nova-core: vbios: use checked accesses in `setup_falcon_data`

[Eliot Courtney]

Use checked arithmetic for `ucode_offset` in `setup_falcon_data`. This
prevents a malformed firmware from causing a panic.

Fixes: dc70c6ae2441 ("gpu: nova-core: vbios: Add support to look up PMU table in FWSEC")
Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index bc752d135cbf..d8633e61178b 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -955,14 +955,15 @@ fn setup_falcon_data(
             .find_entry_by_type(FALCON_UCODE_ENTRY_APPID_FWSEC_PROD)
         {
             Ok(entry) => {
-                let mut ucode_offset = usize::from_safe_cast(entry.data);
-                ucode_offset -= pci_at_image.base.data.len();
-                if ucode_offset < first_fwsec.base.data.len() {
-                    dev_err!(self.base.dev, "Falcon Ucode offset not in second Fwsec.\n");
-                    return Err(EINVAL);
-                }
-                ucode_offset -= first_fwsec.base.data.len();
-                self.falcon_ucode_offset = Some(ucode_offset);
+                self.falcon_ucode_offset = Some(
+                    usize::from_safe_cast(entry.data)
+                        .checked_sub(pci_at_image.base.data.len())
+                        .and_then(|o| o.checked_sub(first_fwsec.base.data.len()))
+                        .ok_or(EINVAL)
+                        .inspect_err(|_| {
+                            dev_err!(self.base.dev, "Falcon Ucode offset not in second Fwsec.\n");
+                        })?,
+                );
             }
             Err(e) => {
                 dev_err!(

-- 
2.53.0

[PATCH v3 06/11] gpu: nova-core: vbios: drop unused falcon_data_offset from FwSecBiosBuilder

[Eliot Courtney]

This is unused, so we can remove it.

Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index d8633e61178b..d63af95eb642 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -256,7 +256,6 @@ pub(crate) fn new(dev: &device::Device, bar0: &Bar0) -> Result<Vbios> {
                 Ok(BiosImageType::FwSec) => {
                     let fwsec = FwSecBiosBuilder {
                         base: image,
-                        falcon_data_offset: None,
                         pmu_lookup_table: None,
                         falcon_ucode_offset: None,
                     };
@@ -631,8 +630,6 @@ struct FwSecBiosBuilder {
     /// Once FwSecBiosBuilder is constructed, the `falcon_ucode_offset` will be copied into a new
     /// [`FwSecBiosImage`].
     ///
-    /// The offset of the Falcon data from the start of Fwsec image.
-    falcon_data_offset: Option<usize>,
     /// The [`PmuLookupTable`] starts at the offset of the falcon data pointer.
     pmu_lookup_table: Option<PmuLookupTable>,
     /// The offset of the Falcon ucode.
@@ -934,8 +931,6 @@ fn setup_falcon_data(
             offset -= first_fwsec.base.data.len();
         }
 
-        self.falcon_data_offset = Some(offset);
-
         if pmu_in_first_fwsec {
             self.pmu_lookup_table = Some(PmuLookupTable::new(
                 &self.base.dev,

-- 
2.53.0

[PATCH v3 07/11] gpu: nova-core: vbios: keep PmuLookupTable local in setup_falcon_data

[Eliot Courtney]

This does not need to be stored in `FwSecBiosBuilder` so we can remove
it from there, and just create and use it locally in
`setup_falcon_data`.

Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 25 ++++++-------------------
 1 file changed, 6 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index d63af95eb642..01f65d50cbb3 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -256,7 +256,6 @@ pub(crate) fn new(dev: &device::Device, bar0: &Bar0) -> Result<Vbios> {
                 Ok(BiosImageType::FwSec) => {
                     let fwsec = FwSecBiosBuilder {
                         base: image,
-                        pmu_lookup_table: None,
                         falcon_ucode_offset: None,
                     };
                     if first_fwsec_image.is_none() {
@@ -630,8 +629,6 @@ struct FwSecBiosBuilder {
     /// Once FwSecBiosBuilder is constructed, the `falcon_ucode_offset` will be copied into a new
     /// [`FwSecBiosImage`].
     ///
-    /// The [`PmuLookupTable`] starts at the offset of the falcon data pointer.
-    pmu_lookup_table: Option<PmuLookupTable>,
     /// The offset of the Falcon ucode.
     falcon_ucode_offset: Option<usize>,
 }
@@ -931,24 +928,14 @@ fn setup_falcon_data(
             offset -= first_fwsec.base.data.len();
         }
 
-        if pmu_in_first_fwsec {
-            self.pmu_lookup_table = Some(PmuLookupTable::new(
-                &self.base.dev,
-                &first_fwsec.base.data[offset..],
-            )?);
+        let pmu_lookup_data = if pmu_in_first_fwsec {
+            &first_fwsec.base.data[offset..]
         } else {
-            self.pmu_lookup_table = Some(PmuLookupTable::new(
-                &self.base.dev,
-                &self.base.data[offset..],
-            )?);
-        }
+            self.base.data.get(offset..).ok_or(EINVAL)?
+        };
+        let pmu_lookup_table = PmuLookupTable::new(&self.base.dev, pmu_lookup_data)?;
 
-        match self
-            .pmu_lookup_table
-            .as_ref()
-            .ok_or(EINVAL)?
-            .find_entry_by_type(FALCON_UCODE_ENTRY_APPID_FWSEC_PROD)
-        {
+        match pmu_lookup_table.find_entry_by_type(FALCON_UCODE_ENTRY_APPID_FWSEC_PROD) {
             Ok(entry) => {
                 self.falcon_ucode_offset = Some(
                     usize::from_safe_cast(entry.data)

-- 
2.53.0

[PATCH v3 08/11] gpu: nova-core: vbios: compute FWSEC-relative Falcon data offset

[Eliot Courtney]

Push the computation of the falcon data offset into a helper function.
The subtraction to create the offset should be checked, and by doing
this the check can be folded into the existing check in
`falcon_data_ptr`.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 48 +++++++++++++++++-------------------------
 1 file changed, 19 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 01f65d50cbb3..0c0e0402e715 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -765,33 +765,29 @@ fn get_bit_token(&self, token_id: u8) -> Result<BitToken> {
         BitToken::from_id(self, token_id)
     }
 
-    /// Find the Falcon data pointer structure in the [`PciAtBiosImage`].
+    /// Find the Falcon data offset from the start of the FWSEC region.
     ///
-    /// This is just a 4 byte structure that contains a pointer to the Falcon data in the FWSEC
-    /// image.
-    fn falcon_data_ptr(&self) -> Result<u32> {
+    /// The BIT table contains a 4-byte pointer to the Falcon data. Testing shows this pointer
+    /// treats the PCI-AT and FWSEC images as logically contiguous even when an EFI image sits in
+    /// between them, so subtract the PCI-AT image size here to convert it to a FWSEC-relative
+    /// offset.
+    fn falcon_data_offset(&self) -> Result<usize> {
         let token = self.get_bit_token(BIT_TOKEN_ID_FALCON_DATA)?;
-
-        // Make sure we don't go out of bounds
-        if usize::from(token.data_offset) + 4 > self.base.data.len() {
-            return Err(EINVAL);
-        }
-
-        // read the 4 bytes at the offset specified in the token
         let offset = usize::from(token.data_offset);
-        let bytes: [u8; 4] = self.base.data[offset..offset + 4].try_into().map_err(|_| {
-            dev_err!(self.base.dev, "Failed to convert data slice to array\n");
-            EINVAL
-        })?;
 
-        let data_ptr = u32::from_le_bytes(bytes);
+        // Read the 4-byte falcon data pointer at the offset specified in the token.
+        let data = &self.base.data;
+        let (ptr, _) = data
+            .get(offset..)
+            .and_then(u32::from_bytes_copy_prefix)
+            .ok_or(EINVAL)?;
 
-        if (usize::from_safe_cast(data_ptr)) < self.base.data.len() {
-            dev_err!(self.base.dev, "Falcon data pointer out of bounds\n");
-            return Err(EINVAL);
-        }
-
-        Ok(data_ptr)
+        usize::from_safe_cast(ptr)
+            .checked_sub(data.len())
+            .ok_or(EINVAL)
+            .inspect_err(|_| {
+                dev_err!(self.base.dev, "Falcon data pointer out of bounds\n");
+            })
     }
 }
 
@@ -908,15 +904,9 @@ fn setup_falcon_data(
         pci_at_image: &PciAtBiosImage,
         first_fwsec: &FwSecBiosBuilder,
     ) -> Result {
-        let mut offset = usize::from_safe_cast(pci_at_image.falcon_data_ptr()?);
+        let mut offset = pci_at_image.falcon_data_offset()?;
         let mut pmu_in_first_fwsec = false;
 
-        // The falcon data pointer assumes that the PciAt and FWSEC images
-        // are contiguous in memory. However, testing shows the EFI image sits in
-        // between them. So calculate the offset from the end of the PciAt image
-        // rather than the start of it. Compensate.
-        offset -= pci_at_image.base.data.len();
-
         // The offset is now from the start of the first Fwsec image, however
         // the offset points to a location in the second Fwsec image. Since
         // the fwsec images are contiguous, subtract the length of the first Fwsec

-- 
2.53.0

[PATCH v3 09/11] gpu: nova-core: vbios: simplify setup_falcon_data

[Eliot Courtney]

The code first computes `pmu_in_first_fwsec` or adjusts the offset and
then uses it in a branch just once to get the correct source for the PMU
table. This can be simplified to a single branch while also avoiding the
mutation of `offset`. Also, adjust the code after this to keep the
success case non-nested.

Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 59 +++++++++++++++++++-----------------------
 1 file changed, 26 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 0c0e0402e715..d71ff5de794f 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -904,48 +904,41 @@ fn setup_falcon_data(
         pci_at_image: &PciAtBiosImage,
         first_fwsec: &FwSecBiosBuilder,
     ) -> Result {
-        let mut offset = pci_at_image.falcon_data_offset()?;
-        let mut pmu_in_first_fwsec = false;
+        let offset = pci_at_image.falcon_data_offset()?;
 
-        // The offset is now from the start of the first Fwsec image, however
-        // the offset points to a location in the second Fwsec image. Since
-        // the fwsec images are contiguous, subtract the length of the first Fwsec
-        // image from the offset to get the offset to the start of the second
-        // Fwsec image.
-        if offset < first_fwsec.base.data.len() {
-            pmu_in_first_fwsec = true;
+        // The offset is from the start of the first FwSec image, but it
+        // may point into the second FwSec image. Treat the two FwSec images
+        // as contiguous here and subtract the first image length when the
+        // target lies in the second one.
+        let pmu_lookup_data = if offset < first_fwsec.base.data.len() {
+            first_fwsec.base.data.get(offset..)
         } else {
-            offset -= first_fwsec.base.data.len();
-        }
-
-        let pmu_lookup_data = if pmu_in_first_fwsec {
-            &first_fwsec.base.data[offset..]
-        } else {
-            self.base.data.get(offset..).ok_or(EINVAL)?
+            self.base.data.get(offset - first_fwsec.base.data.len()..)
         };
-        let pmu_lookup_table = PmuLookupTable::new(&self.base.dev, pmu_lookup_data)?;
 
-        match pmu_lookup_table.find_entry_by_type(FALCON_UCODE_ENTRY_APPID_FWSEC_PROD) {
-            Ok(entry) => {
-                self.falcon_ucode_offset = Some(
-                    usize::from_safe_cast(entry.data)
-                        .checked_sub(pci_at_image.base.data.len())
-                        .and_then(|o| o.checked_sub(first_fwsec.base.data.len()))
-                        .ok_or(EINVAL)
-                        .inspect_err(|_| {
-                            dev_err!(self.base.dev, "Falcon Ucode offset not in second Fwsec.\n");
-                        })?,
-                );
-            }
-            Err(e) => {
+        let pmu_lookup_table = pmu_lookup_data
+            .ok_or(EINVAL)
+            .and_then(|data| PmuLookupTable::new(&self.base.dev, data))?;
+
+        let entry = pmu_lookup_table
+            .find_entry_by_type(FALCON_UCODE_ENTRY_APPID_FWSEC_PROD)
+            .inspect_err(|e| {
                 dev_err!(
                     self.base.dev,
                     "PmuLookupTableEntry not found, error: {:?}\n",
                     e
                 );
-                return Err(EINVAL);
-            }
-        }
+            })?;
+
+        let falcon_ucode_offset = usize::from_safe_cast(entry.data)
+            .checked_sub(pci_at_image.base.data.len())
+            .and_then(|o| o.checked_sub(first_fwsec.base.data.len()))
+            .ok_or(EINVAL)
+            .inspect_err(|_| {
+                dev_err!(self.base.dev, "Falcon Ucode offset not in second Fwsec.\n");
+            })?;
+
+        self.falcon_ucode_offset = Some(falcon_ucode_offset);
         Ok(())
     }
 

-- 
2.53.0

[PATCH v3 10/11] gpu: nova-core: vbios: construct `FwSecBiosImage` directly from BIOS images

[Eliot Courtney]

`FwSecBiosBuilder` now only contains `falcon_ucode_offset` which just
gets passed directly into `FwSecBiosImage`. Remove `FwSecBiosBuilder`
and construct `FwSecBiosImage` directly, as a simplification.

Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 98 +++++++++++++++++-------------------------
 1 file changed, 39 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index d71ff5de794f..5cc251c73800 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -233,8 +233,8 @@ impl Vbios {
     pub(crate) fn new(dev: &device::Device, bar0: &Bar0) -> Result<Vbios> {
         // Images to extract from iteration
         let mut pci_at_image: Option<PciAtBiosImage> = None;
-        let mut first_fwsec_image: Option<FwSecBiosBuilder> = None;
-        let mut second_fwsec_image: Option<FwSecBiosBuilder> = None;
+        let mut first_fwsec_image: Option<BiosImage> = None;
+        let mut second_fwsec_image: Option<BiosImage> = None;
 
         // Parse all VBIOS images in the ROM
         for image_result in VbiosIterator::new(dev, bar0)? {
@@ -254,14 +254,10 @@ pub(crate) fn new(dev: &device::Device, bar0: &Bar0) -> Result<Vbios> {
                     pci_at_image = Some(PciAtBiosImage::try_from(image)?);
                 }
                 Ok(BiosImageType::FwSec) => {
-                    let fwsec = FwSecBiosBuilder {
-                        base: image,
-                        falcon_ucode_offset: None,
-                    };
                     if first_fwsec_image.is_none() {
-                        first_fwsec_image = Some(fwsec);
+                        first_fwsec_image = Some(image);
                     } else {
-                        second_fwsec_image = Some(fwsec);
+                        second_fwsec_image = Some(image);
                     }
                 }
                 _ => {
@@ -271,15 +267,23 @@ pub(crate) fn new(dev: &device::Device, bar0: &Bar0) -> Result<Vbios> {
         }
 
         // Using all the images, setup the falcon data pointer in Fwsec.
-        if let (Some(mut second), Some(first), Some(pci_at)) =
+        if let (Some(second), Some(first), Some(pci_at)) =
             (second_fwsec_image, first_fwsec_image, pci_at_image)
         {
-            second
-                .setup_falcon_data(&pci_at, &first)
+            let fwsec_image = FwSecBiosImage::new(pci_at, first, second)
                 .inspect_err(|e| dev_err!(dev, "Falcon data setup failed: {:?}\n", e))?;
-            Ok(Vbios {
-                fwsec_image: second.build()?,
-            })
+
+            if cfg!(debug_assertions) {
+                // Print the desc header for debugging
+                let desc = fwsec_image.header()?;
+                dev_dbg!(
+                    fwsec_image.base.dev,
+                    "PmuLookupTableEntry desc: {:#?}\n",
+                    desc
+                );
+            }
+
+            Ok(Vbios { fwsec_image })
         } else {
             dev_err!(
                 dev,
@@ -621,18 +625,6 @@ struct NbsiBiosImage {
     // NBSI-specific fields can be added here in the future.
 }
 
-struct FwSecBiosBuilder {
-    base: BiosImage,
-    /// These are temporary fields that are used during the construction of the
-    /// [`FwSecBiosBuilder`].
-    ///
-    /// Once FwSecBiosBuilder is constructed, the `falcon_ucode_offset` will be copied into a new
-    /// [`FwSecBiosImage`].
-    ///
-    /// The offset of the Falcon ucode.
-    falcon_ucode_offset: Option<usize>,
-}
-
 /// The [`FwSecBiosImage`] structure contains the PMU table and the Falcon Ucode.
 ///
 /// The PMU table contains voltage/frequency tables as well as a pointer to the Falcon Ucode.
@@ -898,33 +890,34 @@ fn find_entry_by_type(&self, entry_type: u8) -> Result<PmuLookupTableEntry> {
     }
 }
 
-impl FwSecBiosBuilder {
-    fn setup_falcon_data(
-        &mut self,
-        pci_at_image: &PciAtBiosImage,
-        first_fwsec: &FwSecBiosBuilder,
-    ) -> Result {
+impl FwSecBiosImage {
+    /// Build the final `FwSecBiosImage` from the PCI-AT and FWSEC BIOS images
+    fn new(
+        pci_at_image: PciAtBiosImage,
+        first_fwsec: BiosImage,
+        second_fwsec: BiosImage,
+    ) -> Result<FwSecBiosImage> {
         let offset = pci_at_image.falcon_data_offset()?;
 
         // The offset is from the start of the first FwSec image, but it
         // may point into the second FwSec image. Treat the two FwSec images
         // as contiguous here and subtract the first image length when the
         // target lies in the second one.
-        let pmu_lookup_data = if offset < first_fwsec.base.data.len() {
-            first_fwsec.base.data.get(offset..)
+        let pmu_lookup_data = if offset < first_fwsec.data.len() {
+            first_fwsec.data.get(offset..)
         } else {
-            self.base.data.get(offset - first_fwsec.base.data.len()..)
+            second_fwsec.data.get(offset - first_fwsec.data.len()..)
         };
 
         let pmu_lookup_table = pmu_lookup_data
             .ok_or(EINVAL)
-            .and_then(|data| PmuLookupTable::new(&self.base.dev, data))?;
+            .and_then(|data| PmuLookupTable::new(&second_fwsec.dev, data))?;
 
         let entry = pmu_lookup_table
             .find_entry_by_type(FALCON_UCODE_ENTRY_APPID_FWSEC_PROD)
             .inspect_err(|e| {
                 dev_err!(
-                    self.base.dev,
+                    second_fwsec.dev,
                     "PmuLookupTableEntry not found, error: {:?}\n",
                     e
                 );
@@ -932,34 +925,21 @@ fn setup_falcon_data(
 
         let falcon_ucode_offset = usize::from_safe_cast(entry.data)
             .checked_sub(pci_at_image.base.data.len())
-            .and_then(|o| o.checked_sub(first_fwsec.base.data.len()))
+            .and_then(|o| o.checked_sub(first_fwsec.data.len()))
             .ok_or(EINVAL)
             .inspect_err(|_| {
-                dev_err!(self.base.dev, "Falcon Ucode offset not in second Fwsec.\n");
+                dev_err!(
+                    second_fwsec.dev,
+                    "Falcon Ucode offset not in second Fwsec.\n"
+                );
             })?;
 
-        self.falcon_ucode_offset = Some(falcon_ucode_offset);
-        Ok(())
+        Ok(FwSecBiosImage {
+            base: second_fwsec,
+            falcon_ucode_offset,
+        })
     }
 
-    /// Build the final FwSecBiosImage from this builder
-    fn build(self) -> Result<FwSecBiosImage> {
-        let ret = FwSecBiosImage {
-            base: self.base,
-            falcon_ucode_offset: self.falcon_ucode_offset.ok_or(EINVAL)?,
-        };
-
-        if cfg!(debug_assertions) {
-            // Print the desc header for debugging
-            let desc = ret.header()?;
-            dev_dbg!(ret.base.dev, "PmuLookupTableEntry desc: {:#?}\n", desc);
-        }
-
-        Ok(ret)
-    }
-}
-
-impl FwSecBiosImage {
     /// Get the FwSec header ([`FalconUCodeDesc`]).
     pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
         let data = self

-- 
2.53.0

[PATCH v3 11/11] gpu: nova-core: vbios: use the first PCI-AT and FWSEC images

[Eliot Courtney]

Currently, PCI-AT takes the final image if multiple exist. For FWSEC, it
takes the first one and the last one. Align both of these to nouveau
behavior by taking the first ones.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/vbios.rs | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
index 5cc251c73800..8cfc75b1184f 100644
--- a/drivers/gpu/nova-core/vbios.rs
+++ b/drivers/gpu/nova-core/vbios.rs
@@ -251,12 +251,16 @@ pub(crate) fn new(dev: &device::Device, bar0: &Bar0) -> Result<Vbios> {
             // Convert to a specific image type
             match BiosImageType::try_from(image.pcir.code_type) {
                 Ok(BiosImageType::PciAt) => {
-                    pci_at_image = Some(PciAtBiosImage::try_from(image)?);
+                    // Silently ignore any extra PCI-AT images.
+                    if pci_at_image.is_none() {
+                        pci_at_image = Some(PciAtBiosImage::try_from(image)?);
+                    }
                 }
                 Ok(BiosImageType::FwSec) => {
+                    // Silently ignore any extra FwSec images beyond the first two.
                     if first_fwsec_image.is_none() {
                         first_fwsec_image = Some(image);
-                    } else {
+                    } else if second_fwsec_image.is_none() {
                         second_fwsec_image = Some(image);
                     }
                 }

-- 
2.53.0

Re: [PATCH v3 01/11] gpu: nova-core: vbios: fix various cases of reading past `BIOS_MAX_SCAN_LEN`

[Alexandre Courbot]

On Tue Apr 21, 2026 at 5:20 PM JST, Eliot Courtney wrote:
> Fix various cases that allow reading past `BIOS_MAX_SCAN_LEN` when
> scanning the VBIOS.
>
> Fix bug where `read_more_at_offset` would unnecessarily read more data.
> This happens when the window to read has some part cached and some part
> not. It would read `len` bytes instead of just the uncached portion,
> which could read past `BIOS_MAX_SCAN_LEN`.
>
> Also add more checked arithmetic to catch potential overflows.
> `read_bios_image_at_offset` is called with a length from the VBIOS
> header, so we should be more defensive here.

This reads like this patch is doing 3 different things, or at least two,
since the second chunk (`read_bios_image_at_offset`) does not seem
related to `BIOS_MAX_SCAN_LEN`.

The general rule is that one patch should do one thing - the trick here
will be to either update the message to describe a larger thing (and not
3 small ones), or to split the patch. Both are acceptable IMHO.

>
> Fixes: 6fda04e7f0cd ("gpu: nova-core: vbios: Add base support for VBIOS construction and iteration")
> Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  drivers/gpu/nova-core/vbios.rs | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index ebda28e596c5..6de7e58e0da0 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -132,17 +132,14 @@ fn read_more(&mut self, len: usize) -> Result {
>  
>      /// Read bytes at a specific offset, filling any gap.
>      fn read_more_at_offset(&mut self, offset: usize, len: usize) -> Result {
> -        if offset > BIOS_MAX_SCAN_LEN {
> +        let end = offset.checked_add(len).ok_or(EINVAL)?;
> +
> +        if end > BIOS_MAX_SCAN_LEN {
>              dev_err!(self.dev, "Error: exceeded BIOS scan limit.\n");
>              return Err(EINVAL);
>          }
>  
> -        // If `offset` is beyond current data size, fill the gap first.
> -        let current_len = self.data.len();
> -        let gap_bytes = offset.saturating_sub(current_len);
> -
> -        // Now read the requested bytes at the offset.
> -        self.read_more(gap_bytes + len)
> +        self.read_more(end.saturating_sub(self.data.len()))
>      }
>  
>      /// Read a BIOS image at a specific offset and create a [`BiosImage`] from it.
> @@ -155,8 +152,9 @@ fn read_bios_image_at_offset(
>          len: usize,
>          context: &str,
>      ) -> Result<BiosImage> {
> +        let end = offset.checked_add(len).ok_or(EINVAL)?;
>          let data_len = self.data.len();
> -        if offset + len > data_len {
> +        if end > data_len {

nit: `data_len` is only used on this line, so it can if `if end >
self.data.len() {`.

Otherwise these fixes look quite needed inded.

Re: [PATCH v3 02/11] gpu: nova-core: vbios: limit `BitToken` entry reads

[Alexandre Courbot]

On Tue Apr 21, 2026 at 5:20 PM JST, Eliot Courtney wrote:
> If `header.token_size` is smaller than `BitToken`, then we currently can
> read past the end of `image.base.data`. Check that the token size is at
> least as big as `BitToken`.
>
> Fixes: dc70c6ae2441 ("gpu: nova-core: vbios: Add support to look up PMU table in FWSEC")
> Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  drivers/gpu/nova-core/vbios.rs | 34 +++++++++++++++++-----------------
>  1 file changed, 17 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index 6de7e58e0da0..de856000de23 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -423,31 +423,31 @@ impl BitToken {
>      /// Find a BIT token entry by BIT ID in a PciAtBiosImage
>      fn from_id(image: &PciAtBiosImage, token_id: u8) -> Result<Self> {
>          let header = &image.bit_header;
> +        let entry_size = usize::from(header.token_size);
> +
> +        if entry_size < size_of::<BitToken>() {
> +            return Err(EINVAL);
> +        }

You can get rid of this check if you convert the code as suggested
below.

>  
>          // Offset to the first token entry
>          let tokens_start = image.bit_offset + usize::from(header.header_size);
>  
>          for i in 0..usize::from(header.token_entries) {
> -            let entry_offset = tokens_start + (i * usize::from(header.token_size));
> -
> -            // Make sure we don't go out of bounds
> -            if entry_offset + usize::from(header.token_size) > image.base.data.len() {
> -                return Err(EINVAL);
> -            }
> +            let entry_offset = tokens_start + (i * entry_size);

Should we use checked arithmetic here?

> +            let entry = image
> +                .base
> +                .data
> +                .get(entry_offset..)
> +                .and_then(|data| data.get(..entry_size))
> +                .ok_or(EINVAL)?;
>  
>              // Check if this token has the requested ID
> -            if image.base.data[entry_offset] == token_id {
> +            if entry[0] == token_id {
>                  return Ok(BitToken {
> -                    id: image.base.data[entry_offset],
> -                    data_version: image.base.data[entry_offset + 1],
> -                    data_size: u16::from_le_bytes([
> -                        image.base.data[entry_offset + 2],
> -                        image.base.data[entry_offset + 3],
> -                    ]),
> -                    data_offset: u16::from_le_bytes([
> -                        image.base.data[entry_offset + 4],
> -                        image.base.data[entry_offset + 5],
> -                    ]),
> +                    id: entry[0],
> +                    data_version: entry[1],
> +                    data_size: u16::from_le_bytes([entry[2], entry[3]]),
> +                    data_offset: u16::from_le_bytes([entry[4], entry[5]]),

A common pattern in this file (with several such sites still to fix), is
that since Nova only supports little-endian we can leverage `FromBytes`
in order to avoid all these `from_le_bytes` call. Here this would look
as follows:

    for i in 0..usize::from(header.token_entries) {
        let entry_offset = i
            .checked_mul(entry_size)
            .and_then(|off| tokens_start.checked_add(off))
            .ok_or(EINVAL)?;

        let entry = image
            .base
            .data
            .get(entry_offset..entry_offset + entry_size)
            .and_then(|data| data.get(..entry_size))
            .ok_or(EINVAL)?;

        let (token, _) = BitToken::from_bytes_copy_prefix(entry).ok_or(EINVAL)?;

        if token.id == token_id {
            return Ok(token);
        }
    }

which has several benefits:

- No error-prone `entry[index]` accesses,
- The size check on `entry_size` is done for free by
  `from_bytes_copy_prefix`, and the slice bounds cannot be wrong,
- Shorter, more readable code overall.

Unfortunately we cannot just use `from_bytes_prefix` because we don't
have any alignment guarantee, but this is still an improvement IMHO.

If you go that way and derive `FromBytes` on `BitToken`, don't forget to
also make it `#[repr(C)]`. :)

Re: [PATCH v3 03/11] gpu: nova-core: vbios: use checked ops and accesses in `FwSecBiosImage::ucode`

[Alexandre Courbot]

On Tue Apr 21, 2026 at 5:20 PM JST, Eliot Courtney wrote:
> Use checked arithmetic and access for extracting the microcode since the
> offsets are firmware derived.
>
> Fixes: 47c4846e4319 ("gpu: nova-core: vbios: Add support for FWSEC ucode extraction")
> Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  drivers/gpu/nova-core/vbios.rs | 19 ++++++++++++-------
>  1 file changed, 12 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index de856000de23..632c8a90ea76 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -1029,16 +1029,21 @@ pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
>  
>      /// Get the ucode data as a byte slice
>      pub(crate) fn ucode(&self, desc: &FalconUCodeDesc) -> Result<&[u8]> {
> -        let falcon_ucode_offset = self.falcon_ucode_offset;
> -
>          // The ucode data follows the descriptor.
> -        let ucode_data_offset = falcon_ucode_offset + desc.size();
> -        let size = usize::from_safe_cast(desc.imem_load_size() + desc.dmem_load_size());
> +        let data = self
> +            .base
> +            .data
> +            .get(self.falcon_ucode_offset..)
> +            .ok_or(ERANGE)?;
> +        let size = usize::from_safe_cast(
> +            desc.imem_load_size()
> +                .checked_add(desc.dmem_load_size())
> +                .ok_or(ERANGE)?,
> +        );
>  
>          // Get the data slice, checking bounds in a single operation.
> -        self.base
> -            .data
> -            .get(ucode_data_offset..ucode_data_offset + size)
> +        data.get(desc.size()..)
> +            .and_then(|data| data.get(..size))
>              .ok_or(ERANGE)
>              .inspect_err(|_| {
>                  dev_err!(

This looks like we are doing part of the operation, stop to compute
size, and then resume the operation. As a result the `inspect_err` only
applies to the last `get` operation.

The following would flow better IMHO:

    let size = usize::from_safe_cast(
        desc.imem_load_size()
            .checked_add(desc.dmem_load_size())
            .ok_or(ERANGE)?
    );

    self.base
        .data
        .get(self.falcon_ucode_offset..)
        .and_then(|data| data.get(desc.size()..))
        .and_then(|data| data.get(..size))
        .ok_or(ERANGE)
        .inspect_err(|_| {
            dev_err!(
                self.base.dev,
                "fwsec ucode data not contained within BIOS bounds\n"
            )
        })

Re: [PATCH v3 04/11] gpu: nova-core: vbios: use checked access in `FwSecBiosImage::header`

[Alexandre Courbot]

On Tue Apr 21, 2026 at 5:20 PM JST, Eliot Courtney wrote:
> Use checked access in `FwSecBiosImage::header` for getting the header
> version since the value is firmware derived.
>
> Fixes: 47c4846e4319 ("gpu: nova-core: vbios: Add support for FWSEC ucode extraction")
> Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  drivers/gpu/nova-core/vbios.rs | 17 +++++++----------
>  1 file changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index 632c8a90ea76..bc752d135cbf 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -996,17 +996,14 @@ fn build(self) -> Result<FwSecBiosImage> {
>  impl FwSecBiosImage {
>      /// Get the FwSec header ([`FalconUCodeDesc`]).
>      pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
> -        // Get the falcon ucode offset that was found in setup_falcon_data.
> -        let falcon_ucode_offset = self.falcon_ucode_offset;
> +        let data = self
> +            .base
> +            .data
> +            .get(self.falcon_ucode_offset..)
> +            .ok_or(EINVAL)?;
>  
> -        // Read the first 4 bytes to get the version.
> -        let hdr_bytes: [u8; 4] = self.base.data[falcon_ucode_offset..falcon_ucode_offset + 4]
> -            .try_into()
> -            .map_err(|_| EINVAL)?;
> -        let hdr = u32::from_le_bytes(hdr_bytes);
> -        let ver = (hdr & 0xff00) >> 8;
> -
> -        let data = self.base.data.get(falcon_ucode_offset..).ok_or(EINVAL)?;
> +        // Read the version byte from the header.
> +        let ver = data.get(1).copied().ok_or(EINVAL)?;

This doesn't need to be done with this patch, but once the kernel-wide
`bitfield` macro lands I hope we can use to to define a proper header
type for this and use it both here and in `firmware.rs` which does some
nasty bit masking - see for instance `FalconUCodeDescriptor::size`.

Re: [PATCH v3 09/11] gpu: nova-core: vbios: simplify setup_falcon_data

[Alexandre Courbot]

On Tue Apr 21, 2026 at 5:20 PM JST, Eliot Courtney wrote:
> The code first computes `pmu_in_first_fwsec` or adjusts the offset and
> then uses it in a branch just once to get the correct source for the PMU
> table. This can be simplified to a single branch while also avoiding the
> mutation of `offset`. Also, adjust the code after this to keep the
> success case non-nested.
>
> Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  drivers/gpu/nova-core/vbios.rs | 59 +++++++++++++++++++-----------------------
>  1 file changed, 26 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index 0c0e0402e715..d71ff5de794f 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -904,48 +904,41 @@ fn setup_falcon_data(
>          pci_at_image: &PciAtBiosImage,
>          first_fwsec: &FwSecBiosBuilder,
>      ) -> Result {
> -        let mut offset = pci_at_image.falcon_data_offset()?;
> -        let mut pmu_in_first_fwsec = false;
> +        let offset = pci_at_image.falcon_data_offset()?;
>  
> -        // The offset is now from the start of the first Fwsec image, however
> -        // the offset points to a location in the second Fwsec image. Since
> -        // the fwsec images are contiguous, subtract the length of the first Fwsec
> -        // image from the offset to get the offset to the start of the second
> -        // Fwsec image.
> -        if offset < first_fwsec.base.data.len() {
> -            pmu_in_first_fwsec = true;
> +        // The offset is from the start of the first FwSec image, but it
> +        // may point into the second FwSec image. Treat the two FwSec images
> +        // as contiguous here and subtract the first image length when the
> +        // target lies in the second one.
> +        let pmu_lookup_data = if offset < first_fwsec.base.data.len() {
> +            first_fwsec.base.data.get(offset..)
>          } else {
> -            offset -= first_fwsec.base.data.len();
> -        }
> -
> -        let pmu_lookup_data = if pmu_in_first_fwsec {
> -            &first_fwsec.base.data[offset..]
> -        } else {
> -            self.base.data.get(offset..).ok_or(EINVAL)?
> +            self.base.data.get(offset - first_fwsec.base.data.len()..)
>          };
> -        let pmu_lookup_table = PmuLookupTable::new(&self.base.dev, pmu_lookup_data)?;
>  
> -        match pmu_lookup_table.find_entry_by_type(FALCON_UCODE_ENTRY_APPID_FWSEC_PROD) {
> -            Ok(entry) => {
> -                self.falcon_ucode_offset = Some(
> -                    usize::from_safe_cast(entry.data)
> -                        .checked_sub(pci_at_image.base.data.len())
> -                        .and_then(|o| o.checked_sub(first_fwsec.base.data.len()))
> -                        .ok_or(EINVAL)
> -                        .inspect_err(|_| {
> -                            dev_err!(self.base.dev, "Falcon Ucode offset not in second Fwsec.\n");
> -                        })?,
> -                );
> -            }
> -            Err(e) => {
> +        let pmu_lookup_table = pmu_lookup_data
> +            .ok_or(EINVAL)
> +            .and_then(|data| PmuLookupTable::new(&self.base.dev, data))?;

It looks a bit weird to check the result of the previous statement in
this one. How about:

        let pmu_lookup_data = if offset < first_fwsec.base.data.len() {
            ...
        }
        .ok_or(EINVAL);

        let pmu_lookup_table = PmuLookupTable::new(&self.base.dev, pmu_lookup_data)?;

> +
> +        let entry = pmu_lookup_table
> +            .find_entry_by_type(FALCON_UCODE_ENTRY_APPID_FWSEC_PROD)

It doesn't necessarily need to be addressed in this series, but if you
feel like it I believe there is also potential to use `FromBytes` to
create `PmuLookupTableEntry` instead of building it byte-by-byte.

Re: [PATCH v3 11/11] gpu: nova-core: vbios: use the first PCI-AT and FWSEC images

[Alexandre Courbot]

On Tue Apr 21, 2026 at 5:20 PM JST, Eliot Courtney wrote:
> Currently, PCI-AT takes the final image if multiple exist. For FWSEC, it
> takes the first one and the last one. Align both of these to nouveau
> behavior by taking the first ones.
>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  drivers/gpu/nova-core/vbios.rs | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
> index 5cc251c73800..8cfc75b1184f 100644
> --- a/drivers/gpu/nova-core/vbios.rs
> +++ b/drivers/gpu/nova-core/vbios.rs
> @@ -251,12 +251,16 @@ pub(crate) fn new(dev: &device::Device, bar0: &Bar0) -> Result<Vbios> {
>              // Convert to a specific image type
>              match BiosImageType::try_from(image.pcir.code_type) {
>                  Ok(BiosImageType::PciAt) => {
> -                    pci_at_image = Some(PciAtBiosImage::try_from(image)?);
> +                    // Silently ignore any extra PCI-AT images.
> +                    if pci_at_image.is_none() {
> +                        pci_at_image = Some(PciAtBiosImage::try_from(image)?);
> +                    }
>                  }

I am getting a Clippy here:

warning: this `if` can be collapsed into the outer `match`
   --> ../drivers/gpu/nova-core/vbios.rs:338:21
    |
338 | /                     if pci_at_image.is_none() {
339 | |                         pci_at_image = Some(PciAtBiosImage::try_from(image)?);
340 | |                     }
    | |_____________________^
    |
    = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.95.0/index.html#collapsible_match
    = note: `-W clippy::collapsible-match` implied by `-W clippy::all`
    = help: to override `-W clippy::all` add `#[allow(clippy::collapsible_match)]`
help: collapse nested if block
    |
336 ~                 Ok(BiosImageType::PciAt)
337 |                     // Silently ignore any extra PCI-AT images.
338 ~                     if pci_at_image.is_none() => {
339 |                         pci_at_image = Some(PciAtBiosImage::try_from(image)?);
340 ~                     }

I have tested this series on Turing and probe completed successfully.

Re: [PATCH v3 01/11] gpu: nova-core: vbios: fix various cases of reading past `BIOS_MAX_SCAN_LEN`

[Joel Fernandes]

On 4/29/2026 9:24 AM, Alexandre Courbot wrote:
> On Tue Apr 21, 2026 at 5:20 PM JST, Eliot Courtney wrote:
>> Fix various cases that allow reading past `BIOS_MAX_SCAN_LEN` when
>> scanning the VBIOS.
>>
>> Fix bug where `read_more_at_offset` would unnecessarily read more data.
>> This happens when the window to read has some part cached and some part
>> not. It would read `len` bytes instead of just the uncached portion,
>> which could read past `BIOS_MAX_SCAN_LEN`.
>>
>> Also add more checked arithmetic to catch potential overflows.
>> `read_bios_image_at_offset` is called with a length from the VBIOS
>> header, so we should be more defensive here.
> 
> This reads like this patch is doing 3 different things, or at least two,
> since the second chunk (`read_bios_image_at_offset`) does not seem
> related to `BIOS_MAX_SCAN_LEN`.
> 
> The general rule is that one patch should do one thing - the trick here
> will be to either update the message to describe a larger thing (and not
> 3 small ones), or to split the patch. Both are acceptable IMHO.

I thought about that too but didn't say it because it seemed the other
changes were just 2-3 lines and generally same 'functional area'. However,
I agree with Alex, perhaps splitting into 1 for BIOS_MAX_SCAN_LEN and
another for checked_add makes sense.

> 
>>
>> Fixes: 6fda04e7f0cd ("gpu: nova-core: vbios: Add base support for VBIOS construction and iteration")
>> Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
Feel free to still carry my tag either way.

Thanks.

Re: [PATCH v3 11/11] gpu: nova-core: vbios: use the first PCI-AT and FWSEC images

[Gary Guo]

On Wed Apr 29, 2026 at 3:32 PM BST, Alexandre Courbot wrote:
> On Tue Apr 21, 2026 at 5:20 PM JST, Eliot Courtney wrote:
>> Currently, PCI-AT takes the final image if multiple exist. For FWSEC, it
>> takes the first one and the last one. Align both of these to nouveau
>> behavior by taking the first ones.
>>
>> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
>> ---
>>  drivers/gpu/nova-core/vbios.rs | 8 ++++++--
>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
>> index 5cc251c73800..8cfc75b1184f 100644
>> --- a/drivers/gpu/nova-core/vbios.rs
>> +++ b/drivers/gpu/nova-core/vbios.rs
>> @@ -251,12 +251,16 @@ pub(crate) fn new(dev: &device::Device, bar0: &Bar0) -> Result<Vbios> {
>>              // Convert to a specific image type
>>              match BiosImageType::try_from(image.pcir.code_type) {
>>                  Ok(BiosImageType::PciAt) => {
>> -                    pci_at_image = Some(PciAtBiosImage::try_from(image)?);
>> +                    // Silently ignore any extra PCI-AT images.
>> +                    if pci_at_image.is_none() {
>> +                        pci_at_image = Some(PciAtBiosImage::try_from(image)?);
>> +                    }
>>                  }
>
> I am getting a Clippy here:
>
> warning: this `if` can be collapsed into the outer `match`
>    --> ../drivers/gpu/nova-core/vbios.rs:338:21
>     |
> 338 | /                     if pci_at_image.is_none() {
> 339 | |                         pci_at_image = Some(PciAtBiosImage::try_from(image)?);
> 340 | |                     }
>     | |_____________________^
>     |
>     = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.95.0/index.html#collapsible_match
>     = note: `-W clippy::collapsible-match` implied by `-W clippy::all`
>     = help: to override `-W clippy::all` add `#[allow(clippy::collapsible_match)]`
> help: collapse nested if block
>     |
> 336 ~                 Ok(BiosImageType::PciAt)
> 337 |                     // Silently ignore any extra PCI-AT images.
> 338 ~                     if pci_at_image.is_none() => {
> 339 |                         pci_at_image = Some(PciAtBiosImage::try_from(image)?);
> 340 ~                     }
>
> I have tested this series on Turing and probe completed successfully.

Be aware of false positives and the suggested code changes the behaviour. See
https://lore.kernel.org/rust-for-linux/20260426144201.227108-1-ojeda@kernel.org/.

Best,
Gary

Re: [PATCH v3 01/11] gpu: nova-core: vbios: fix various cases of reading past `BIOS_MAX_SCAN_LEN`

[Eliot Courtney]

On Wed Apr 29, 2026 at 11:32 PM JST, Joel Fernandes wrote:
>
>
> On 4/29/2026 9:24 AM, Alexandre Courbot wrote:
>> On Tue Apr 21, 2026 at 5:20 PM JST, Eliot Courtney wrote:
>>> Fix various cases that allow reading past `BIOS_MAX_SCAN_LEN` when
>>> scanning the VBIOS.
>>>
>>> Fix bug where `read_more_at_offset` would unnecessarily read more data.
>>> This happens when the window to read has some part cached and some part
>>> not. It would read `len` bytes instead of just the uncached portion,
>>> which could read past `BIOS_MAX_SCAN_LEN`.
>>>
>>> Also add more checked arithmetic to catch potential overflows.
>>> `read_bios_image_at_offset` is called with a length from the VBIOS
>>> header, so we should be more defensive here.
>> 
>> This reads like this patch is doing 3 different things, or at least two,
>> since the second chunk (`read_bios_image_at_offset`) does not seem
>> related to `BIOS_MAX_SCAN_LEN`.
>> 
>> The general rule is that one patch should do one thing - the trick here
>> will be to either update the message to describe a larger thing (and not
>> 3 small ones), or to split the patch. Both are acceptable IMHO.
>
> I thought about that too but didn't say it because it seemed the other
> changes were just 2-3 lines and generally same 'functional area'. However,
> I agree with Alex, perhaps splitting into 1 for BIOS_MAX_SCAN_LEN and
> another for checked_add makes sense.

Thanks all, I have split this into three patches.

>
>> 
>>>
>>> Fixes: 6fda04e7f0cd ("gpu: nova-core: vbios: Add base support for VBIOS construction and iteration")
>>> Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
> Feel free to still carry my tag either way.

Thanks. It's a pure split with no code changes so I have carried your
reviewed by for each.

>
> Thanks.

Re: [PATCH v3 02/11] gpu: nova-core: vbios: limit `BitToken` entry reads

[Eliot Courtney]

On Wed Apr 29, 2026 at 10:35 PM JST, Alexandre Courbot wrote:
> On Tue Apr 21, 2026 at 5:20 PM JST, Eliot Courtney wrote:
>> If `header.token_size` is smaller than `BitToken`, then we currently can
>> read past the end of `image.base.data`. Check that the token size is at
>> least as big as `BitToken`.
>>
>> Fixes: dc70c6ae2441 ("gpu: nova-core: vbios: Add support to look up PMU table in FWSEC")
>> Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
>> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
>> ---
>>  drivers/gpu/nova-core/vbios.rs | 34 +++++++++++++++++-----------------
>>  1 file changed, 17 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
>> index 6de7e58e0da0..de856000de23 100644
>> --- a/drivers/gpu/nova-core/vbios.rs
>> +++ b/drivers/gpu/nova-core/vbios.rs
>> @@ -423,31 +423,31 @@ impl BitToken {
>>      /// Find a BIT token entry by BIT ID in a PciAtBiosImage
>>      fn from_id(image: &PciAtBiosImage, token_id: u8) -> Result<Self> {
>>          let header = &image.bit_header;
>> +        let entry_size = usize::from(header.token_size);
>> +
>> +        if entry_size < size_of::<BitToken>() {
>> +            return Err(EINVAL);
>> +        }
>
> You can get rid of this check if you convert the code as suggested
> below.
>
>>  
>>          // Offset to the first token entry
>>          let tokens_start = image.bit_offset + usize::from(header.header_size);
>>  
>>          for i in 0..usize::from(header.token_entries) {
>> -            let entry_offset = tokens_start + (i * usize::from(header.token_size));
>> -
>> -            // Make sure we don't go out of bounds
>> -            if entry_offset + usize::from(header.token_size) > image.base.data.len() {
>> -                return Err(EINVAL);
>> -            }
>> +            let entry_offset = tokens_start + (i * entry_size);
>
> Should we use checked arithmetic here?
>
>> +            let entry = image
>> +                .base
>> +                .data
>> +                .get(entry_offset..)
>> +                .and_then(|data| data.get(..entry_size))
>> +                .ok_or(EINVAL)?;
>>  
>>              // Check if this token has the requested ID
>> -            if image.base.data[entry_offset] == token_id {
>> +            if entry[0] == token_id {
>>                  return Ok(BitToken {
>> -                    id: image.base.data[entry_offset],
>> -                    data_version: image.base.data[entry_offset + 1],
>> -                    data_size: u16::from_le_bytes([
>> -                        image.base.data[entry_offset + 2],
>> -                        image.base.data[entry_offset + 3],
>> -                    ]),
>> -                    data_offset: u16::from_le_bytes([
>> -                        image.base.data[entry_offset + 4],
>> -                        image.base.data[entry_offset + 5],
>> -                    ]),
>> +                    id: entry[0],
>> +                    data_version: entry[1],
>> +                    data_size: u16::from_le_bytes([entry[2], entry[3]]),
>> +                    data_offset: u16::from_le_bytes([entry[4], entry[5]]),
>
> A common pattern in this file (with several such sites still to fix), is
> that since Nova only supports little-endian we can leverage `FromBytes`
> in order to avoid all these `from_le_bytes` call. Here this would look
> as follows:
>
>     for i in 0..usize::from(header.token_entries) {
>         let entry_offset = i
>             .checked_mul(entry_size)
>             .and_then(|off| tokens_start.checked_add(off))
>             .ok_or(EINVAL)?;
>
>         let entry = image
>             .base
>             .data
>             .get(entry_offset..entry_offset + entry_size)
>             .and_then(|data| data.get(..entry_size))
>             .ok_or(EINVAL)?;
>
>         let (token, _) = BitToken::from_bytes_copy_prefix(entry).ok_or(EINVAL)?;
>
>         if token.id == token_id {
>             return Ok(token);
>         }
>     }
>
> which has several benefits:
>
> - No error-prone `entry[index]` accesses,
> - The size check on `entry_size` is done for free by
>   `from_bytes_copy_prefix`, and the slice bounds cannot be wrong,
> - Shorter, more readable code overall.
>
> Unfortunately we cannot just use `from_bytes_prefix` because we don't
> have any alignment guarantee, but this is still an improvement IMHO.
>
> If you go that way and derive `FromBytes` on `BitToken`, don't forget to
> also make it `#[repr(C)]`. :)

I agree this is better, thanks!

Re: [PATCH v3 04/11] gpu: nova-core: vbios: use checked access in `FwSecBiosImage::header`

[Eliot Courtney]

On Wed Apr 29, 2026 at 10:56 PM JST, Alexandre Courbot wrote:
> On Tue Apr 21, 2026 at 5:20 PM JST, Eliot Courtney wrote:
>> Use checked access in `FwSecBiosImage::header` for getting the header
>> version since the value is firmware derived.
>>
>> Fixes: 47c4846e4319 ("gpu: nova-core: vbios: Add support for FWSEC ucode extraction")
>> Reviewed-by: Joel Fernandes <joelagnelf@nvidia.com>
>> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
>> ---
>>  drivers/gpu/nova-core/vbios.rs | 17 +++++++----------
>>  1 file changed, 7 insertions(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
>> index 632c8a90ea76..bc752d135cbf 100644
>> --- a/drivers/gpu/nova-core/vbios.rs
>> +++ b/drivers/gpu/nova-core/vbios.rs
>> @@ -996,17 +996,14 @@ fn build(self) -> Result<FwSecBiosImage> {
>>  impl FwSecBiosImage {
>>      /// Get the FwSec header ([`FalconUCodeDesc`]).
>>      pub(crate) fn header(&self) -> Result<FalconUCodeDesc> {
>> -        // Get the falcon ucode offset that was found in setup_falcon_data.
>> -        let falcon_ucode_offset = self.falcon_ucode_offset;
>> +        let data = self
>> +            .base
>> +            .data
>> +            .get(self.falcon_ucode_offset..)
>> +            .ok_or(EINVAL)?;
>>  
>> -        // Read the first 4 bytes to get the version.
>> -        let hdr_bytes: [u8; 4] = self.base.data[falcon_ucode_offset..falcon_ucode_offset + 4]
>> -            .try_into()
>> -            .map_err(|_| EINVAL)?;
>> -        let hdr = u32::from_le_bytes(hdr_bytes);
>> -        let ver = (hdr & 0xff00) >> 8;
>> -
>> -        let data = self.base.data.get(falcon_ucode_offset..).ok_or(EINVAL)?;
>> +        // Read the version byte from the header.
>> +        let ver = data.get(1).copied().ok_or(EINVAL)?;
>
> This doesn't need to be done with this patch, but once the kernel-wide
> `bitfield` macro lands I hope we can use to to define a proper header
> type for this and use it both here and in `firmware.rs` which does some
> nasty bit masking - see for instance `FalconUCodeDescriptor::size`.

Yerp that will be nice. For this specific instance, not sure if it's
worth it though since it's just get the value of one byte.

Re: [PATCH v3 11/11] gpu: nova-core: vbios: use the first PCI-AT and FWSEC images

[Eliot Courtney]

On Thu Apr 30, 2026 at 2:49 AM JST, Gary Guo wrote:
> On Wed Apr 29, 2026 at 3:32 PM BST, Alexandre Courbot wrote:
>> On Tue Apr 21, 2026 at 5:20 PM JST, Eliot Courtney wrote:
>>> Currently, PCI-AT takes the final image if multiple exist. For FWSEC, it
>>> takes the first one and the last one. Align both of these to nouveau
>>> behavior by taking the first ones.
>>>
>>> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
>>> ---
>>>  drivers/gpu/nova-core/vbios.rs | 8 ++++++--
>>>  1 file changed, 6 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/nova-core/vbios.rs b/drivers/gpu/nova-core/vbios.rs
>>> index 5cc251c73800..8cfc75b1184f 100644
>>> --- a/drivers/gpu/nova-core/vbios.rs
>>> +++ b/drivers/gpu/nova-core/vbios.rs
>>> @@ -251,12 +251,16 @@ pub(crate) fn new(dev: &device::Device, bar0: &Bar0) -> Result<Vbios> {
>>>              // Convert to a specific image type
>>>              match BiosImageType::try_from(image.pcir.code_type) {
>>>                  Ok(BiosImageType::PciAt) => {
>>> -                    pci_at_image = Some(PciAtBiosImage::try_from(image)?);
>>> +                    // Silently ignore any extra PCI-AT images.
>>> +                    if pci_at_image.is_none() {
>>> +                        pci_at_image = Some(PciAtBiosImage::try_from(image)?);
>>> +                    }
>>>                  }
>>
>> I am getting a Clippy here:
>>
>> warning: this `if` can be collapsed into the outer `match`
>>    --> ../drivers/gpu/nova-core/vbios.rs:338:21
>>     |
>> 338 | /                     if pci_at_image.is_none() {
>> 339 | |                         pci_at_image = Some(PciAtBiosImage::try_from(image)?);
>> 340 | |                     }
>>     | |_____________________^
>>     |
>>     = help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.95.0/index.html#collapsible_match
>>     = note: `-W clippy::collapsible-match` implied by `-W clippy::all`
>>     = help: to override `-W clippy::all` add `#[allow(clippy::collapsible_match)]`
>> help: collapse nested if block
>>     |
>> 336 ~                 Ok(BiosImageType::PciAt)
>> 337 |                     // Silently ignore any extra PCI-AT images.
>> 338 ~                     if pci_at_image.is_none() => {
>> 339 |                         pci_at_image = Some(PciAtBiosImage::try_from(image)?);
>> 340 ~                     }
>>
>> I have tested this series on Turing and probe completed successfully.


Thank you for testing (and reviewing) this!
>
> Be aware of false positives and the suggested code changes the behaviour. See
> https://lore.kernel.org/rust-for-linux/20260426144201.227108-1-ojeda@kernel.org/.
>
> Best,
> Gary

I have to say I am not a fan of this lint in this particular case, since
it moves the "ignore this" semantics to be match level rather than match
branch level. In this particular case it's fine since the _ branch also
silently ignores, but I have been bitten by this kind of match case
lifting making it easy for semantics to be accidentally changed before
IIRC.