[PATCH v3 0/5] gpu: nova-core: blackwell follow-ups and fixes

[PATCH v3 0/5] gpu: nova-core: blackwell follow-ups and fixes

[Eliot Courtney]

Make FbLayout code more obvious and correct.

Currently, the frts vidmem offset is calculated based on the non-wpr
heap size and pmu reservation size, but AFAICT this is not right. The fb
layout actually looks like: | non-wpr heap | WPR2 .. FRTS | PMU reserved
| ... | VGA workspace | It's just by coincidence + generous alignment
that the values happened to match with something more like pmu reserved
size + vga workspace.

`FbLayout` is used for both pre and post FSP architectures. FbLayout
contains ranges for each region of framebuffer, but on post FSP
architectures, only the size is actually used by GSP. The offsets are
not decided by the driver. So, for post FSP architectures FbLayout
contains essentially guesses for the offsets. Instead, make separate
types so that we only store the information that's actually needed. This
includes the actual reserved size after the pmu reservation so we can
properly compute the frts offset.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
Changes in v3:
- Remove patches merged already
- Move `fb_end_reserved_size` to FspHal
- Move construction of FbSizes/FbRanges into GSP HAL.
- Link to v2: https://patch.msgid.link/20260703-blackwell-fixes-v2-0-8e3d8bc32bb9@nvidia.com

Changes in v2:
- Rebased on top of 20260629-nova-bootcontext-v4-0-5539d8469590@nvidia.com
- Dropped applied patches ("falcon: gsp: move PRIV target mask constants"
  and "fsp: move FMC firmware loading into wait_secure_boot")
- Dropped "gsp: keep FMC boot params DMA region alive during error" - 
  will finish discussion/fix on the bootcontext series from Alex
- Drop "fsp: try to enforce exclusive access to FSP channel" as not
  necessary.
- Added some extra fixes around the falcon "halted" state
- Drop "wait for FSP boot earlier" since FSP is now at Gpu level in the
  bootcontext series from Alex
- Don't reset FSP queue pointers on error
- Add r-b from Alistair
- Return EMSGSIZE not EIO for FSP queue return message too big
- Link to v1: https://patch.msgid.link/20260615-blackwell-fixes-v1-0-f2853e49ff7d@nvidia.com

---
Eliot Courtney (5):
      gpu: nova-core: correct FRTS vidmem offset calculation
      gpu: nova-core: rename heap size field
      gpu: nova-core: return non-WPR heap size as u64 from HALs
      gpu: nova-core: split FbLayout into FSP and non-FSP versions
      gpu: nova-core: pass WPR metadata ownership to FmcBootArgs

 drivers/gpu/nova-core/fb.rs            | 50 ++++++++++++++++----
 drivers/gpu/nova-core/fb/hal.rs        |  2 +-
 drivers/gpu/nova-core/fb/hal/ga100.rs  |  2 +-
 drivers/gpu/nova-core/fb/hal/ga102.rs  |  2 +-
 drivers/gpu/nova-core/fb/hal/gb100.rs  |  5 +-
 drivers/gpu/nova-core/fb/hal/gb202.rs  |  5 +-
 drivers/gpu/nova-core/fb/hal/gh100.rs  |  4 +-
 drivers/gpu/nova-core/fb/hal/tu102.rs  |  6 +--
 drivers/gpu/nova-core/fsp.rs           | 44 +++++++++++-------
 drivers/gpu/nova-core/fsp/hal.rs       |  4 ++
 drivers/gpu/nova-core/fsp/hal/gb100.rs |  6 +++
 drivers/gpu/nova-core/fsp/hal/gb202.rs |  9 +++-
 drivers/gpu/nova-core/fsp/hal/gh100.rs |  9 +++-
 drivers/gpu/nova-core/gsp/boot.rs      | 28 ++++-------
 drivers/gpu/nova-core/gsp/fw.rs        | 85 ++++++++++++++++++++++++++--------
 drivers/gpu/nova-core/gsp/hal.rs       | 12 ++---
 drivers/gpu/nova-core/gsp/hal/gh100.rs | 21 +++++----
 drivers/gpu/nova-core/gsp/hal/tu102.rs | 32 ++++++++-----
 18 files changed, 219 insertions(+), 107 deletions(-)
---
base-commit: 71d4e7233f235871b13553e504e591ace6b54373
change-id: 20260608-blackwell-fixes-30c9358c90a0

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

[PATCH v3 1/5] gpu: nova-core: correct FRTS vidmem offset calculation

[Eliot Courtney]

Currently, the frts vidmem offset is calculated based on the non-wpr
heap size and pmu reservation size, but this is not right. The layout
actually looks like this:

| non-wpr heap | WPR2 .. FRTS | PMU reserved | ... | VGA workspace |

It's just by coincidence + generous alignment that the values happened
to match. Instead, define a per-architecture reserved size at the end of
the framebuffer and use this plus the PMU reserved size to calculate the
frts vidmem offset.

Fixes: d317e4585fa3 ("gpu: nova-core: Hopper/Blackwell: add FSP Chain of Trust boot")
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/fb/hal/gb100.rs  |  1 +
 drivers/gpu/nova-core/fb/hal/gb202.rs  |  1 +
 drivers/gpu/nova-core/fsp.rs           | 27 +++++++++++++++++++--------
 drivers/gpu/nova-core/fsp/hal.rs       |  4 ++++
 drivers/gpu/nova-core/fsp/hal/gb100.rs |  6 ++++++
 drivers/gpu/nova-core/fsp/hal/gb202.rs |  9 ++++++++-
 drivers/gpu/nova-core/fsp/hal/gh100.rs |  9 ++++++++-
 7 files changed, 47 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/nova-core/fb/hal/gb100.rs b/drivers/gpu/nova-core/fb/hal/gb100.rs
index 6e0eba101ca1..51bd0c0bede6 100644
--- a/drivers/gpu/nova-core/fb/hal/gb100.rs
+++ b/drivers/gpu/nova-core/fb/hal/gb100.rs
@@ -78,6 +78,7 @@ fn write_sysmem_flush_page_gb100(bar: Bar0<'_>, addr: Bounded<u64, 52>) {
     );
 }
 
+// This PMU reservation size is r570-specific.
 pub(super) const fn pmu_reserved_size_gb100() -> u32 {
     usize_into_u32::<{ const_align_up(SZ_8M + SZ_16M + SZ_4K, Alignment::new::<SZ_128K>()).unwrap() }>(
     )
diff --git a/drivers/gpu/nova-core/fb/hal/gb202.rs b/drivers/gpu/nova-core/fb/hal/gb202.rs
index b78e0970f66d..8ec30afcf701 100644
--- a/drivers/gpu/nova-core/fb/hal/gb202.rs
+++ b/drivers/gpu/nova-core/fb/hal/gb202.rs
@@ -71,6 +71,7 @@ fn pmu_reserved_size(&self) -> u32 {
 
     fn non_wpr_heap_size(&self) -> u32 {
         // Non-WPR heap for GB20x (see Open RM: kgspGetNonWprHeapSize, GB202+).
+        // This size is r570-specific.
         u32::SZ_2M + u32::SZ_128K
     }
 
diff --git a/drivers/gpu/nova-core/fsp.rs b/drivers/gpu/nova-core/fsp.rs
index 1475485bded3..30e53df81aa2 100644
--- a/drivers/gpu/nova-core/fsp.rs
+++ b/drivers/gpu/nova-core/fsp.rs
@@ -134,20 +134,31 @@ struct FspCotMessage {
 }
 
 impl FspCotMessage {
+    /// Computes the FRTS vidmem offset for the Chain-of-Trust message. It is measured from the end
+    /// of the framebuffer.
+    fn frts_vidmem_offset(hal: &dyn hal::FspHal, fb_layout: &FbLayout) -> Result<u64> {
+        let mut offset = u64::from(hal.fb_end_reserved_size());
+
+        if fb_layout.pmu_reserved_size != 0 {
+            offset = (offset + u64::from(fb_layout.pmu_reserved_size))
+                // The 2 MiB alignment is r570-specific.
+                .align_up(Alignment::new::<SZ_2M>())
+                .ok_or(EINVAL)?;
+        }
+
+        Ok(offset)
+    }
+
     /// Returns an in-place initializer for [`FspCotMessage`].
     fn new<'a>(
         fb_layout: &FbLayout,
         fsp_fw: &'a FspFirmware,
         args: &'a FmcBootArgs<'_>,
     ) -> Result<impl Init<Self> + 'a> {
-        // frts_vidmem_offset is measured from the end of FB, so FRTS sits at
-        // (end of FB) - frts_vidmem_offset.
-        let frts_vidmem_offset = if !args.resume {
-            let frts_reserved_size = fb_layout.heap.len() + u64::from(fb_layout.pmu_reserved_size);
+        let hal = hal::fsp_hal(args.chipset).ok_or(ENOTSUPP)?;
 
-            frts_reserved_size
-                .align_up(Alignment::new::<SZ_2M>())
-                .ok_or(EINVAL)?
+        let frts_vidmem_offset = if !args.resume {
+            Self::frts_vidmem_offset(hal, fb_layout)?
         } else {
             0
         };
@@ -158,7 +169,7 @@ fn new<'a>(
             0
         };
 
-        let version = hal::fsp_hal(args.chipset).ok_or(ENOTSUPP)?.cot_version();
+        let version = hal.cot_version();
         let size = num::usize_into_u16::<{ core::mem::size_of::<NvdmPayloadCot>() }>();
 
         Ok(init!(Self {
diff --git a/drivers/gpu/nova-core/fsp/hal.rs b/drivers/gpu/nova-core/fsp/hal.rs
index b6f2624bb13d..aa2f8bda59d2 100644
--- a/drivers/gpu/nova-core/fsp/hal.rs
+++ b/drivers/gpu/nova-core/fsp/hal.rs
@@ -19,6 +19,10 @@ pub(super) trait FspHal {
 
     /// Returns the FSP Chain of Trust protocol version this chipset advertises.
     fn cot_version(&self) -> u16;
+
+    // TODO: consider moving this into the TLV firmware metadata when ready
+    /// Returns the size reserved at the end of the framebuffer, in bytes.
+    fn fb_end_reserved_size(&self) -> u32;
 }
 
 /// Returns the FSP HAL, or `None` if the architecture doesn't support FSP.
diff --git a/drivers/gpu/nova-core/fsp/hal/gb100.rs b/drivers/gpu/nova-core/fsp/hal/gb100.rs
index 42f5ecfc6400..f601b5d7bf61 100644
--- a/drivers/gpu/nova-core/fsp/hal/gb100.rs
+++ b/drivers/gpu/nova-core/fsp/hal/gb100.rs
@@ -1,6 +1,8 @@
 // SPDX-License-Identifier: GPL-2.0
 // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 
+use kernel::sizes::SizeConstants;
+
 use crate::{
     driver::Bar0,
     fsp::hal::FspHal, //
@@ -17,6 +19,10 @@ fn fsp_boot_status(&self, bar: Bar0<'_>) -> u32 {
     fn cot_version(&self) -> u16 {
         2
     }
+
+    fn fb_end_reserved_size(&self) -> u32 {
+        u32::SZ_2M + u32::SZ_128K
+    }
 }
 
 const GB100: Gb100 = Gb100;
diff --git a/drivers/gpu/nova-core/fsp/hal/gb202.rs b/drivers/gpu/nova-core/fsp/hal/gb202.rs
index 1091b169a645..b022d298195a 100644
--- a/drivers/gpu/nova-core/fsp/hal/gb202.rs
+++ b/drivers/gpu/nova-core/fsp/hal/gb202.rs
@@ -1,7 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0
 // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 
-use kernel::io::Io;
+use kernel::{
+    io::Io,
+    sizes::SizeConstants, //
+};
 
 use crate::{
     driver::Bar0,
@@ -21,6 +24,10 @@ fn fsp_boot_status(&self, bar: Bar0<'_>) -> u32 {
     fn cot_version(&self) -> u16 {
         2
     }
+
+    fn fb_end_reserved_size(&self) -> u32 {
+        u32::SZ_2M + u32::SZ_128K
+    }
 }
 
 const GB202: Gb202 = Gb202;
diff --git a/drivers/gpu/nova-core/fsp/hal/gh100.rs b/drivers/gpu/nova-core/fsp/hal/gh100.rs
index 291acaf2845a..002f7ccdca3e 100644
--- a/drivers/gpu/nova-core/fsp/hal/gh100.rs
+++ b/drivers/gpu/nova-core/fsp/hal/gh100.rs
@@ -1,7 +1,10 @@
 // SPDX-License-Identifier: GPL-2.0
 // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
 
-use kernel::io::Io;
+use kernel::{
+    io::Io,
+    sizes::SizeConstants, //
+};
 
 use crate::{
     driver::Bar0,
@@ -26,6 +29,10 @@ fn fsp_boot_status(&self, bar: Bar0<'_>) -> u32 {
     fn cot_version(&self) -> u16 {
         1
     }
+
+    fn fb_end_reserved_size(&self) -> u32 {
+        u32::SZ_2M
+    }
 }
 
 const GH100: Gh100 = Gh100;

-- 
2.55.0

[PATCH v3 2/5] gpu: nova-core: rename heap size field

[Eliot Courtney]

This field is called non_wpr_heap_size everywhere else. Unify the name
to make it more obvious which heap it is.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/fb.rs     | 10 +++++-----
 drivers/gpu/nova-core/gsp/fw.rs |  6 +++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
index 273cff752fae..1f7b6a69e4db 100644
--- a/drivers/gpu/nova-core/fb.rs
+++ b/drivers/gpu/nova-core/fb.rs
@@ -163,7 +163,7 @@ pub(crate) struct FbLayout {
     pub(crate) wpr2_heap: FbRange,
     /// WPR2 region range, starting with an instance of `GspFwWprMeta`.
     pub(crate) wpr2: FbRange,
-    pub(crate) heap: FbRange,
+    pub(crate) non_wpr_heap: FbRange,
     pub(crate) vf_partition_count: u8,
     /// PMU reserved memory size, in bytes.
     pub(crate) pmu_reserved_size: u32,
@@ -251,9 +251,9 @@ pub(crate) fn new(chipset: Chipset, bar: Bar0<'_>, gsp_fw: &GspFirmware) -> Resu
             FbRange(wpr2_addr..frts.end)
         };
 
-        let heap = {
-            let heap_size = u64::from(hal.non_wpr_heap_size());
-            FbRange(wpr2.start - heap_size..wpr2.start)
+        let non_wpr_heap = {
+            let non_wpr_heap_size = u64::from(hal.non_wpr_heap_size());
+            FbRange(wpr2.start - non_wpr_heap_size..wpr2.start)
         };
 
         Ok(Self {
@@ -264,7 +264,7 @@ pub(crate) fn new(chipset: Chipset, bar: Bar0<'_>, gsp_fw: &GspFirmware) -> Resu
             elf,
             wpr2_heap,
             wpr2,
-            heap,
+            non_wpr_heap,
             vf_partition_count: 0,
             pmu_reserved_size: hal.pmu_reserved_size(),
         })
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index 166b0c69edb7..93f2125dbbfb 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -191,9 +191,9 @@ pub(crate) fn new<'a>(
                     sizeOfSignature: u64::from_safe_cast(gsp_firmware.signatures.size()),
                 },
             },
-            gspFwRsvdStart: fb_layout.heap.start,
-            nonWprHeapOffset: fb_layout.heap.start,
-            nonWprHeapSize: fb_layout.heap.end - fb_layout.heap.start,
+            gspFwRsvdStart: fb_layout.non_wpr_heap.start,
+            nonWprHeapOffset: fb_layout.non_wpr_heap.start,
+            nonWprHeapSize: fb_layout.non_wpr_heap.end - fb_layout.non_wpr_heap.start,
             gspFwWprStart: fb_layout.wpr2.start,
             gspFwHeapOffset: fb_layout.wpr2_heap.start,
             gspFwHeapSize: fb_layout.wpr2_heap.end - fb_layout.wpr2_heap.start,

-- 
2.55.0

[PATCH v3 3/5] gpu: nova-core: return non-WPR heap size as u64 from HALs

[Eliot Courtney]

This is always immediately widened to u64, so just return it as a u64
from the beginning.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/fb.rs           | 2 +-
 drivers/gpu/nova-core/fb/hal.rs       | 2 +-
 drivers/gpu/nova-core/fb/hal/ga100.rs | 2 +-
 drivers/gpu/nova-core/fb/hal/ga102.rs | 2 +-
 drivers/gpu/nova-core/fb/hal/gb100.rs | 4 ++--
 drivers/gpu/nova-core/fb/hal/gb202.rs | 4 ++--
 drivers/gpu/nova-core/fb/hal/gh100.rs | 4 ++--
 drivers/gpu/nova-core/fb/hal/tu102.rs | 6 +++---
 8 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
index 1f7b6a69e4db..f5832e59de59 100644
--- a/drivers/gpu/nova-core/fb.rs
+++ b/drivers/gpu/nova-core/fb.rs
@@ -252,7 +252,7 @@ pub(crate) fn new(chipset: Chipset, bar: Bar0<'_>, gsp_fw: &GspFirmware) -> Resu
         };
 
         let non_wpr_heap = {
-            let non_wpr_heap_size = u64::from(hal.non_wpr_heap_size());
+            let non_wpr_heap_size = hal.non_wpr_heap_size();
             FbRange(wpr2.start - non_wpr_heap_size..wpr2.start)
         };
 
diff --git a/drivers/gpu/nova-core/fb/hal.rs b/drivers/gpu/nova-core/fb/hal.rs
index 714f0b51cd8f..99363cfb116b 100644
--- a/drivers/gpu/nova-core/fb/hal.rs
+++ b/drivers/gpu/nova-core/fb/hal.rs
@@ -37,7 +37,7 @@ pub(crate) trait FbHal {
     fn pmu_reserved_size(&self) -> u32;
 
     /// Returns the non-WPR heap size for this chipset, in bytes.
-    fn non_wpr_heap_size(&self) -> u32;
+    fn non_wpr_heap_size(&self) -> u64;
 
     /// Returns the FRTS size, in bytes.
     fn frts_size(&self) -> u64;
diff --git a/drivers/gpu/nova-core/fb/hal/ga100.rs b/drivers/gpu/nova-core/fb/hal/ga100.rs
index 3cc1caf361c7..3b99028f3828 100644
--- a/drivers/gpu/nova-core/fb/hal/ga100.rs
+++ b/drivers/gpu/nova-core/fb/hal/ga100.rs
@@ -72,7 +72,7 @@ fn pmu_reserved_size(&self) -> u32 {
         super::tu102::pmu_reserved_size_tu102()
     }
 
-    fn non_wpr_heap_size(&self) -> u32 {
+    fn non_wpr_heap_size(&self) -> u64 {
         super::tu102::non_wpr_heap_size_tu102()
     }
 
diff --git a/drivers/gpu/nova-core/fb/hal/ga102.rs b/drivers/gpu/nova-core/fb/hal/ga102.rs
index 44a2cf8a00f1..a5eeddb8a1ae 100644
--- a/drivers/gpu/nova-core/fb/hal/ga102.rs
+++ b/drivers/gpu/nova-core/fb/hal/ga102.rs
@@ -41,7 +41,7 @@ fn pmu_reserved_size(&self) -> u32 {
         super::tu102::pmu_reserved_size_tu102()
     }
 
-    fn non_wpr_heap_size(&self) -> u32 {
+    fn non_wpr_heap_size(&self) -> u64 {
         super::tu102::non_wpr_heap_size_tu102()
     }
 
diff --git a/drivers/gpu/nova-core/fb/hal/gb100.rs b/drivers/gpu/nova-core/fb/hal/gb100.rs
index 51bd0c0bede6..9a1fd76c179e 100644
--- a/drivers/gpu/nova-core/fb/hal/gb100.rs
+++ b/drivers/gpu/nova-core/fb/hal/gb100.rs
@@ -109,9 +109,9 @@ fn pmu_reserved_size(&self) -> u32 {
         pmu_reserved_size_gb100()
     }
 
-    fn non_wpr_heap_size(&self) -> u32 {
+    fn non_wpr_heap_size(&self) -> u64 {
         // Non-WPR heap for GB10x (see Open RM: kgspGetNonWprHeapSize, GB100/GB102).
-        u32::SZ_2M
+        u64::SZ_2M
     }
 
     fn frts_size(&self) -> u64 {
diff --git a/drivers/gpu/nova-core/fb/hal/gb202.rs b/drivers/gpu/nova-core/fb/hal/gb202.rs
index 8ec30afcf701..86b5b5613169 100644
--- a/drivers/gpu/nova-core/fb/hal/gb202.rs
+++ b/drivers/gpu/nova-core/fb/hal/gb202.rs
@@ -69,10 +69,10 @@ fn pmu_reserved_size(&self) -> u32 {
         super::gb100::pmu_reserved_size_gb100()
     }
 
-    fn non_wpr_heap_size(&self) -> u32 {
+    fn non_wpr_heap_size(&self) -> u64 {
         // Non-WPR heap for GB20x (see Open RM: kgspGetNonWprHeapSize, GB202+).
         // This size is r570-specific.
-        u32::SZ_2M + u32::SZ_128K
+        u64::SZ_2M + u64::SZ_128K
     }
 
     fn frts_size(&self) -> u64 {
diff --git a/drivers/gpu/nova-core/fb/hal/gh100.rs b/drivers/gpu/nova-core/fb/hal/gh100.rs
index d39fe99537ed..3cffc6ef6769 100644
--- a/drivers/gpu/nova-core/fb/hal/gh100.rs
+++ b/drivers/gpu/nova-core/fb/hal/gh100.rs
@@ -61,9 +61,9 @@ fn pmu_reserved_size(&self) -> u32 {
         super::tu102::pmu_reserved_size_tu102()
     }
 
-    fn non_wpr_heap_size(&self) -> u32 {
+    fn non_wpr_heap_size(&self) -> u64 {
         // Non-WPR heap for Hopper (see Open RM: kgspCalculateFbLayout_GH100).
-        u32::SZ_2M
+        u64::SZ_2M
     }
 
     fn frts_size(&self) -> u64 {
diff --git a/drivers/gpu/nova-core/fb/hal/tu102.rs b/drivers/gpu/nova-core/fb/hal/tu102.rs
index f629e8e9d5d5..4d30c005cfb4 100644
--- a/drivers/gpu/nova-core/fb/hal/tu102.rs
+++ b/drivers/gpu/nova-core/fb/hal/tu102.rs
@@ -44,8 +44,8 @@ pub(super) const fn pmu_reserved_size_tu102() -> u32 {
     0
 }
 
-pub(super) const fn non_wpr_heap_size_tu102() -> u32 {
-    u32::SZ_1M
+pub(super) const fn non_wpr_heap_size_tu102() -> u64 {
+    u64::SZ_1M
 }
 
 pub(super) const fn frts_size_tu102() -> u64 {
@@ -75,7 +75,7 @@ fn pmu_reserved_size(&self) -> u32 {
         pmu_reserved_size_tu102()
     }
 
-    fn non_wpr_heap_size(&self) -> u32 {
+    fn non_wpr_heap_size(&self) -> u64 {
         non_wpr_heap_size_tu102()
     }
 

-- 
2.55.0

[PATCH v3 4/5] gpu: nova-core: split FbLayout into FSP and non-FSP versions

[Eliot Courtney]

`FbLayout` is currently used for both pre and post FSP architectures. It
contains ranges for each region of framebuffer, but on post FSP
architectures, only the size is actually used by GSP. The offsets are
not decided by the driver. So, for post FSP architectures `FbLayout`
contains essentially guesses for the offsets. Instead, make separate
types so that we only store the information that's actually needed,
rather than keeping around offsets that may not be correct.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/fb.rs            | 40 ++++++++++++++--
 drivers/gpu/nova-core/fsp.rs           | 21 ++++-----
 drivers/gpu/nova-core/gsp/boot.rs      | 28 ++++-------
 drivers/gpu/nova-core/gsp/fw.rs        | 85 ++++++++++++++++++++++++++--------
 drivers/gpu/nova-core/gsp/hal.rs       | 12 ++---
 drivers/gpu/nova-core/gsp/hal/gh100.rs | 17 ++++---
 drivers/gpu/nova-core/gsp/hal/tu102.rs | 32 ++++++++-----
 7 files changed, 155 insertions(+), 80 deletions(-)

diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
index f5832e59de59..2336e97f7748 100644
--- a/drivers/gpu/nova-core/fb.rs
+++ b/drivers/gpu/nova-core/fb.rs
@@ -148,7 +148,7 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
 ///
 /// Contains ranges of GPU memory reserved for a given purpose during the GSP boot process.
 #[derive(Debug)]
-pub(crate) struct FbLayout {
+pub(crate) struct FbRanges {
     /// Range of the framebuffer. Starts at `0`.
     pub(crate) fb: FbRange,
     /// VGA workspace, small area of reserved memory at the end of the framebuffer.
@@ -163,14 +163,16 @@ pub(crate) struct FbLayout {
     pub(crate) wpr2_heap: FbRange,
     /// WPR2 region range, starting with an instance of `GspFwWprMeta`.
     pub(crate) wpr2: FbRange,
+    /// Non-WPR heap, located just below WPR2.
     pub(crate) non_wpr_heap: FbRange,
+    /// Number of VF partitions.
     pub(crate) vf_partition_count: u8,
     /// PMU reserved memory size, in bytes.
     pub(crate) pmu_reserved_size: u32,
 }
 
-impl FbLayout {
-    /// Computes the FB layout for `chipset` required to run the `gsp_fw` GSP firmware.
+impl FbRanges {
+    /// Computes concrete framebuffer ranges required on non-FSP booting architectures.
     pub(crate) fn new(chipset: Chipset, bar: Bar0<'_>, gsp_fw: &GspFirmware) -> Result<Self> {
         let hal = hal::fb_hal(chipset);
 
@@ -270,3 +272,35 @@ pub(crate) fn new(chipset: Chipset, bar: Bar0<'_>, gsp_fw: &GspFirmware) -> Resu
         })
     }
 }
+
+/// Framebuffer region sizes needed for GSP-FMC boot.
+#[derive(Debug)]
+pub(crate) struct FbSizes {
+    /// FRTS size, in bytes.
+    pub(crate) frts_size: u64,
+    /// WPR2 heap size, in bytes.
+    pub(crate) wpr2_heap_size: u64,
+    /// Non-WPR heap size, in bytes.
+    pub(crate) non_wpr_heap_size: u64,
+    /// PMU reserved memory size, in bytes.
+    pub(crate) pmu_reserved_size: u32,
+    /// Number of VF partitions.
+    pub(crate) vf_partition_count: u8,
+}
+
+impl FbSizes {
+    /// Computes the framebuffer region sizes for GSP-FMC boot.
+    pub(crate) fn new(chipset: Chipset, bar: Bar0<'_>) -> Result<Self> {
+        let hal = hal::fb_hal(chipset);
+        let fb_size = hal.vidmem_size(bar);
+
+        Ok(Self {
+            frts_size: hal.frts_size(),
+            wpr2_heap_size: gsp::LibosParams::from_chipset(chipset)
+                .wpr_heap_size(chipset, fb_size)?,
+            non_wpr_heap_size: hal.non_wpr_heap_size(),
+            pmu_reserved_size: hal.pmu_reserved_size(),
+            vf_partition_count: 0,
+        })
+    }
+}
diff --git a/drivers/gpu/nova-core/fsp.rs b/drivers/gpu/nova-core/fsp.rs
index 30e53df81aa2..48438ba21b91 100644
--- a/drivers/gpu/nova-core/fsp.rs
+++ b/drivers/gpu/nova-core/fsp.rs
@@ -31,7 +31,7 @@
         fsp::Fsp as FspEngine,
         Falcon, //
     },
-    fb::FbLayout,
+    fb::FbSizes,
     firmware::{
         fsp::{
             FmcSignatures,
@@ -136,11 +136,11 @@ struct FspCotMessage {
 impl FspCotMessage {
     /// Computes the FRTS vidmem offset for the Chain-of-Trust message. It is measured from the end
     /// of the framebuffer.
-    fn frts_vidmem_offset(hal: &dyn hal::FspHal, fb_layout: &FbLayout) -> Result<u64> {
+    fn frts_vidmem_offset(hal: &dyn hal::FspHal, fb_info: &FbSizes) -> Result<u64> {
         let mut offset = u64::from(hal.fb_end_reserved_size());
 
-        if fb_layout.pmu_reserved_size != 0 {
-            offset = (offset + u64::from(fb_layout.pmu_reserved_size))
+        if fb_info.pmu_reserved_size != 0 {
+            offset = (offset + u64::from(fb_info.pmu_reserved_size))
                 // The 2 MiB alignment is r570-specific.
                 .align_up(Alignment::new::<SZ_2M>())
                 .ok_or(EINVAL)?;
@@ -151,20 +151,20 @@ fn frts_vidmem_offset(hal: &dyn hal::FspHal, fb_layout: &FbLayout) -> Result<u64
 
     /// Returns an in-place initializer for [`FspCotMessage`].
     fn new<'a>(
-        fb_layout: &FbLayout,
+        fb_info: &FbSizes,
         fsp_fw: &'a FspFirmware,
         args: &'a FmcBootArgs<'_>,
     ) -> Result<impl Init<Self> + 'a> {
         let hal = hal::fsp_hal(args.chipset).ok_or(ENOTSUPP)?;
 
         let frts_vidmem_offset = if !args.resume {
-            Self::frts_vidmem_offset(hal, fb_layout)?
+            Self::frts_vidmem_offset(hal, fb_info)?
         } else {
             0
         };
 
         let frts_size: u32 = if !args.resume {
-            fb_layout.frts.len().try_into()?
+            fb_info.frts_size.try_into()?
         } else {
             0
         };
@@ -368,15 +368,12 @@ fn send_sync_fsp<M>(&mut self, dev: &device::Device, msg: &M) -> Result<KVec<u8>
     pub(crate) fn boot_fmc(
         &mut self,
         dev: &device::Device<device::Bound>,
-        fb_layout: &FbLayout,
+        fb_info: &FbSizes,
         args: &FmcBootArgs<'_>,
     ) -> Result {
         dev_dbg!(dev, "Starting FSP boot sequence for {}\n", args.chipset);
 
-        let msg = KBox::init(
-            FspCotMessage::new(fb_layout, &self.fsp_fw, args)?,
-            GFP_KERNEL,
-        )?;
+        let msg = KBox::init(FspCotMessage::new(fb_info, &self.fsp_fw, args)?, GFP_KERNEL)?;
 
         let _response_buf = self.send_sync_fsp(dev, &*msg)?;
 
diff --git a/drivers/gpu/nova-core/gsp/boot.rs b/drivers/gpu/nova-core/gsp/boot.rs
index 6b7d00205000..028a58f2bd40 100644
--- a/drivers/gpu/nova-core/gsp/boot.rs
+++ b/drivers/gpu/nova-core/gsp/boot.rs
@@ -3,7 +3,6 @@
 
 use kernel::{
     bits,
-    dma::Coherent,
     io::poll::read_poll_timeout,
     prelude::*,
     time::Delta,
@@ -16,15 +15,13 @@
         gsp::Gsp,
         Falcon, //
     },
-    fb::FbLayout,
     firmware::{
         gsp::GspFirmware,
         FIRMWARE_VERSION, //
     },
     gsp::{
         cmdq::Cmdq,
-        commands,
-        GspFwWprMeta, //
+        commands, //
     },
 };
 
@@ -50,23 +47,16 @@ pub(crate) fn boot(
 
         let gsp_fw = KBox::pin_init(GspFirmware::new(dev, chipset, FIRMWARE_VERSION), GFP_KERNEL)?;
 
-        let fb_layout = FbLayout::new(chipset, bar, &gsp_fw)?;
-        dev_dbg!(dev, "{:#x?}\n", fb_layout);
-
-        let wpr_meta = Coherent::init(dev, GFP_KERNEL, GspFwWprMeta::new(&gsp_fw, &fb_layout))?;
-
         // Perform the chipset-specific boot sequence, and retrieve the unload bundle.
-        let unload_bundle = hal
-            .boot(&self, &mut ctx, &fb_layout, &wpr_meta)?
-            .or_else(|| {
-                dev_warn!(dev, "The GSP won't be able to unload properly on unbind.\n");
-                dev_warn!(
-                    dev,
-                    "The GPU will need to be reset before the driver can bind again.\n"
-                );
+        let unload_bundle = hal.boot(&self, &mut ctx, &gsp_fw)?.or_else(|| {
+            dev_warn!(dev, "The GSP won't be able to unload properly on unbind.\n");
+            dev_warn!(
+                dev,
+                "The GPU will need to be reset before the driver can bind again.\n"
+            );
 
-                None
-            });
+            None
+        });
 
         let mut unload_guard =
             ScopeGuard::new_with_data((ctx, unload_bundle), |(ctx, unload_bundle)| {
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index 93f2125dbbfb..d8f88f1665e9 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -36,7 +36,10 @@
 };
 
 use crate::{
-    fb::FbLayout,
+    fb::{
+        FbRanges,
+        FbSizes, //
+    },
     firmware::gsp::GspFirmware,
     gpu::{
         Architecture,
@@ -169,10 +172,10 @@ unsafe impl FromBytes for GspFwWprMeta {}
 
 impl GspFwWprMeta {
     /// Returns an initializer for a `GspFwWprMeta` suitable for booting `gsp_firmware` using the
-    /// `fb_layout` layout.
-    pub(crate) fn new<'a>(
+    /// framebuffer ranges `ranges`.
+    pub(crate) fn from_ranges<'a>(
         gsp_firmware: &'a GspFirmware,
-        fb_layout: &'a FbLayout,
+        ranges: &'a FbRanges,
     ) -> impl Init<Self> + 'a {
         let init_inner = init!(bindings::GspFwWprMeta {
             // CAST: we want to store the bits of `GSP_FW_WPR_META_MAGIC` unmodified.
@@ -191,25 +194,67 @@ pub(crate) fn new<'a>(
                     sizeOfSignature: u64::from_safe_cast(gsp_firmware.signatures.size()),
                 },
             },
-            gspFwRsvdStart: fb_layout.non_wpr_heap.start,
-            nonWprHeapOffset: fb_layout.non_wpr_heap.start,
-            nonWprHeapSize: fb_layout.non_wpr_heap.end - fb_layout.non_wpr_heap.start,
-            gspFwWprStart: fb_layout.wpr2.start,
-            gspFwHeapOffset: fb_layout.wpr2_heap.start,
-            gspFwHeapSize: fb_layout.wpr2_heap.end - fb_layout.wpr2_heap.start,
-            gspFwOffset: fb_layout.elf.start,
-            bootBinOffset: fb_layout.boot.start,
-            frtsOffset: fb_layout.frts.start,
-            frtsSize: fb_layout.frts.end - fb_layout.frts.start,
-            gspFwWprEnd: fb_layout
+            gspFwRsvdStart: ranges.non_wpr_heap.start,
+            nonWprHeapOffset: ranges.non_wpr_heap.start,
+            nonWprHeapSize: ranges.non_wpr_heap.len(),
+            gspFwWprStart: ranges.wpr2.start,
+            gspFwHeapOffset: ranges.wpr2_heap.start,
+            gspFwHeapSize: ranges.wpr2_heap.len(),
+            gspFwOffset: ranges.elf.start,
+            bootBinOffset: ranges.boot.start,
+            frtsOffset: ranges.frts.start,
+            frtsSize: ranges.frts.len(),
+            gspFwWprEnd: ranges
                 .vga_workspace
                 .start
                 .align_down(Alignment::new::<SZ_128K>()),
-            gspFwHeapVfPartitionCount: fb_layout.vf_partition_count,
-            fbSize: fb_layout.fb.end - fb_layout.fb.start,
-            vgaWorkspaceOffset: fb_layout.vga_workspace.start,
-            vgaWorkspaceSize: fb_layout.vga_workspace.end - fb_layout.vga_workspace.start,
-            pmuReservedSize: fb_layout.pmu_reserved_size,
+            gspFwHeapVfPartitionCount: ranges.vf_partition_count,
+            fbSize: ranges.fb.len(),
+            vgaWorkspaceOffset: ranges.vga_workspace.start,
+            vgaWorkspaceSize: ranges.vga_workspace.len(),
+            pmuReservedSize: ranges.pmu_reserved_size,
+            ..Zeroable::init_zeroed()
+        });
+
+        init!(GspFwWprMeta {
+            inner <- init_inner,
+        })
+    }
+
+    /// Returns an initializer for a `GspFwWprMeta` suitable for booting `gsp_firmware` using the
+    /// framebuffer region sizes `sizes`.
+    ///
+    /// The region offsets are left at zero: the ACR ucode computes them when it sets up WPR2.
+    pub(crate) fn from_sizes<'a>(
+        gsp_firmware: &'a GspFirmware,
+        sizes: &'a FbSizes,
+    ) -> impl Init<Self> + 'a {
+        /// VGA workspace size to reserve at the end of the framebuffer, in bytes.
+        const VGA_WORKSPACE_SIZE: u64 = u64::SZ_128K;
+
+        let init_inner = init!(bindings::GspFwWprMeta {
+            // CAST: we want to store the bits of `GSP_FW_WPR_META_MAGIC` unmodified.
+            magic: bindings::GSP_FW_WPR_META_MAGIC as u64,
+            revision: u64::from(bindings::GSP_FW_WPR_META_REVISION),
+            sysmemAddrOfRadix3Elf: gsp_firmware.radix3_dma_handle(),
+            sizeOfRadix3Elf: u64::from_safe_cast(gsp_firmware.size),
+            sysmemAddrOfBootloader: gsp_firmware.bootloader.ucode.dma_handle(),
+            sizeOfBootloader: u64::from_safe_cast(gsp_firmware.bootloader.ucode.size()),
+            bootloaderCodeOffset: u64::from(gsp_firmware.bootloader.code_offset),
+            bootloaderDataOffset: u64::from(gsp_firmware.bootloader.data_offset),
+            bootloaderManifestOffset: u64::from(gsp_firmware.bootloader.manifest_offset),
+            __bindgen_anon_1: GspFwWprMetaBootResumeInfo {
+                __bindgen_anon_1: GspFwWprMetaBootInfo {
+                    sysmemAddrOfSignature: gsp_firmware.signatures.dma_handle(),
+                    sizeOfSignature: u64::from_safe_cast(gsp_firmware.signatures.size()),
+                },
+            },
+            nonWprHeapSize: sizes.non_wpr_heap_size,
+            gspFwHeapSize: sizes.wpr2_heap_size,
+            frtsSize: sizes.frts_size,
+            gspFwHeapVfPartitionCount: sizes.vf_partition_count,
+            vgaWorkspaceSize: VGA_WORKSPACE_SIZE,
+            pmuReservedSize: sizes.pmu_reserved_size,
             ..Zeroable::init_zeroed()
         });
 
diff --git a/drivers/gpu/nova-core/gsp/hal.rs b/drivers/gpu/nova-core/gsp/hal.rs
index 34b4bb82a999..11e436651a69 100644
--- a/drivers/gpu/nova-core/gsp/hal.rs
+++ b/drivers/gpu/nova-core/gsp/hal.rs
@@ -5,13 +5,9 @@
 mod gh100;
 mod tu102;
 
-use kernel::{
-    dma::Coherent,
-    prelude::*, //
-};
+use kernel::prelude::*;
 
 use crate::{
-    fb::FbLayout,
     firmware::gsp::GspFirmware,
     gpu::{
         Architecture,
@@ -19,8 +15,7 @@
     },
     gsp::{
         Gsp,
-        GspBootContext,
-        GspFwWprMeta, //
+        GspBootContext, //
     },
 };
 
@@ -44,8 +39,7 @@ fn boot(
         &self,
         gsp: &Gsp,
         ctx: &mut GspBootContext<'_, '_>,
-        fb_layout: &FbLayout,
-        wpr_meta: &Coherent<GspFwWprMeta>,
+        gsp_fw: &GspFirmware,
     ) -> Result<Option<crate::gsp::UnloadBundle>>;
 
     /// Performs HAL-specific post-GSP boot tasks.
diff --git a/drivers/gpu/nova-core/gsp/hal/gh100.rs b/drivers/gpu/nova-core/gsp/hal/gh100.rs
index 22b60f9233de..358dc2f25e26 100644
--- a/drivers/gpu/nova-core/gsp/hal/gh100.rs
+++ b/drivers/gpu/nova-core/gsp/hal/gh100.rs
@@ -16,7 +16,8 @@
         gsp::Gsp as GspEngine,
         Falcon, //
     },
-    fb::FbLayout,
+    fb::FbSizes,
+    firmware::gsp::GspFirmware,
     fsp::FmcBootArgs,
     gsp::{
         hal::{
@@ -144,19 +145,23 @@ fn boot(
         &self,
         gsp: &Gsp,
         ctx: &mut GspBootContext<'_, '_>,
-        fb_layout: &FbLayout,
-        wpr_meta: &Coherent<GspFwWprMeta>,
+        gsp_fw: &GspFirmware,
     ) -> Result<Option<crate::gsp::UnloadBundle>> {
         let dev = ctx.dev();
         let chipset = ctx.chipset;
         let gsp_falcon = ctx.gsp_falcon;
 
+        let fb_sizes = FbSizes::new(chipset, ctx.bar)?;
+        dev_dbg!(dev, "{:#x?}\n", fb_sizes);
+
+        let wpr_meta =
+            Coherent::init(dev, GFP_KERNEL, GspFwWprMeta::from_sizes(gsp_fw, &fb_sizes))?;
+        let args = FmcBootArgs::new(dev, chipset, &wpr_meta, &gsp.libos, false)?;
+
         let unload_bundle = crate::gsp::UnloadBundle(
             KBox::new(FspUnloadBundle, GFP_KERNEL)? as KBox<dyn UnloadBundle>
         );
 
-        let args = FmcBootArgs::new(dev, chipset, wpr_meta, &gsp.libos, false)?;
-
         // Wait for the GSP RISC-V core to halt in case of error. We create this guard after `args`
         // to make sure that boot args are kept alive until halt, in case they are still being
         // accessed.
@@ -167,7 +172,7 @@ fn boot(
 
         let fsp = unload_guard.1.fsp.as_mut().ok_or(ENODEV)?;
 
-        fsp.boot_fmc(dev, fb_layout, &args)?;
+        fsp.boot_fmc(dev, &fb_sizes, &args)?;
 
         // Wait for GSP-FMC to release the GSP lockdown, indicating that `args` is not accessed
         // anymore.
diff --git a/drivers/gpu/nova-core/gsp/hal/tu102.rs b/drivers/gpu/nova-core/gsp/hal/tu102.rs
index 648657e248da..7a7c7c58aadb 100644
--- a/drivers/gpu/nova-core/gsp/hal/tu102.rs
+++ b/drivers/gpu/nova-core/gsp/hal/tu102.rs
@@ -17,7 +17,7 @@
         sec2::Sec2,
         Falcon, //
     },
-    fb::FbLayout,
+    fb::FbRanges,
     firmware::{
         booter::{
             BooterFirmware,
@@ -142,7 +142,7 @@ fn run_fwsec_frts(
         falcon: &Falcon<'_, GspEngine>,
         bar: Bar0<'_>,
         bios: &Vbios,
-        fb_layout: &FbLayout,
+        fb_ranges: &FbRanges,
     ) -> Result {
         // Check that the WPR2 region does not already exist - if it does, we cannot run
         // FWSEC-FRTS until the GPU is reset.
@@ -160,8 +160,8 @@ fn run_fwsec_frts(
             falcon,
             bios,
             FwsecCommand::Frts {
-                frts_addr: fb_layout.frts.start,
-                frts_size: fb_layout.frts.len(),
+                frts_addr: fb_ranges.frts.start,
+                frts_size: fb_ranges.frts.len(),
             },
         )?;
 
@@ -200,12 +200,12 @@ fn run_fwsec_frts(
 
                 Err(EIO)
             }
-            (wpr2_lo, _) if wpr2_lo != fb_layout.frts.start => {
+            (wpr2_lo, _) if wpr2_lo != fb_ranges.frts.start => {
                 dev_err!(
                     dev,
                     "WPR2 region created at unexpected address {:#x}; expected {:#x}\n",
                     wpr2_lo,
-                    fb_layout.frts.start,
+                    fb_ranges.frts.start,
                 );
 
                 Err(EIO)
@@ -259,8 +259,7 @@ fn boot(
         &self,
         gsp: &Gsp,
         ctx: &mut GspBootContext<'_, '_>,
-        fb_layout: &FbLayout,
-        wpr_meta: &Coherent<GspFwWprMeta>,
+        gsp_fw: &GspFirmware,
     ) -> Result<Option<crate::gsp::UnloadBundle>> {
         let dev = ctx.dev();
         let bar = ctx.bar;
@@ -268,6 +267,17 @@ fn boot(
         let gsp_falcon = ctx.gsp_falcon;
         let sec2_falcon = ctx.sec2_falcon;
 
+        let fb_ranges = FbRanges::new(chipset, bar, gsp_fw)?;
+        dev_dbg!(dev, "{:#x?}\n", fb_ranges);
+
+        // Declared before the unload guard so that if Booter fails while running, SEC2 is reset
+        // by the guard before this allocation is freed.
+        let wpr_meta = Coherent::init(
+            dev,
+            GFP_KERNEL,
+            GspFwWprMeta::from_ranges(gsp_fw, &fb_ranges),
+        )?;
+
         let bios = Vbios::new(dev, bar)?;
 
         // Try and prepare the unload bundle.
@@ -287,8 +297,8 @@ fn boot(
         });
 
         // FWSEC-FRTS is not executed on chips where the FRTS region size is 0 (e.g. GA100).
-        if !fb_layout.frts.is_empty() {
-            self.run_fwsec_frts(dev, chipset, gsp_falcon, bar, &bios, fb_layout)?;
+        if !fb_ranges.frts.is_empty() {
+            self.run_fwsec_frts(dev, chipset, gsp_falcon, bar, &bios, &fb_ranges)?;
         }
 
         gsp_falcon.reset()?;
@@ -309,7 +319,7 @@ fn boot(
             FIRMWARE_VERSION,
             sec2_falcon,
         )?
-        .run(dev, sec2_falcon, wpr_meta)?;
+        .run(dev, sec2_falcon, &wpr_meta)?;
 
         Ok(unload_guard.dismiss())
     }

-- 
2.55.0

[PATCH v3 5/5] gpu: nova-core: pass WPR metadata ownership to FmcBootArgs

[Eliot Courtney]

`FmcBootArgs` logically owns this, so pass ownership to it instead of
storing a reference.

Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/fsp.rs           | 4 ++--
 drivers/gpu/nova-core/gsp/hal/gh100.rs | 6 +++---
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/nova-core/fsp.rs b/drivers/gpu/nova-core/fsp.rs
index 48438ba21b91..67de54fbd008 100644
--- a/drivers/gpu/nova-core/fsp.rs
+++ b/drivers/gpu/nova-core/fsp.rs
@@ -208,7 +208,7 @@ pub(crate) struct FmcBootArgs<'a> {
     fmc_boot_params: Coherent<GspFmcBootParams>,
     resume: bool,
     // Additional dependencies required to be kept alive for FMC boot.
-    _wpr_meta: &'a Coherent<GspFwWprMeta>,
+    _wpr_meta: Coherent<GspFwWprMeta>,
     _libos: &'a Coherent<[LibosMemoryRegionInitArgument]>,
 }
 
@@ -218,7 +218,7 @@ impl<'a> FmcBootArgs<'a> {
     pub(crate) fn new(
         dev: &device::Device<device::Bound>,
         chipset: Chipset,
-        wpr_meta: &'a Coherent<GspFwWprMeta>,
+        wpr_meta: Coherent<GspFwWprMeta>,
         libos: &'a Coherent<[LibosMemoryRegionInitArgument]>,
         resume: bool,
     ) -> Result<Self> {
diff --git a/drivers/gpu/nova-core/gsp/hal/gh100.rs b/drivers/gpu/nova-core/gsp/hal/gh100.rs
index 358dc2f25e26..f7e7da62ad9c 100644
--- a/drivers/gpu/nova-core/gsp/hal/gh100.rs
+++ b/drivers/gpu/nova-core/gsp/hal/gh100.rs
@@ -156,15 +156,15 @@ fn boot(
 
         let wpr_meta =
             Coherent::init(dev, GFP_KERNEL, GspFwWprMeta::from_sizes(gsp_fw, &fb_sizes))?;
-        let args = FmcBootArgs::new(dev, chipset, &wpr_meta, &gsp.libos, false)?;
+        let args = FmcBootArgs::new(dev, chipset, wpr_meta, &gsp.libos, false)?;
 
         let unload_bundle = crate::gsp::UnloadBundle(
             KBox::new(FspUnloadBundle, GFP_KERNEL)? as KBox<dyn UnloadBundle>
         );
 
         // Wait for the GSP RISC-V core to halt in case of error. We create this guard after `args`
-        // to make sure that boot args are kept alive until halt, in case they are still being
-        // accessed.
+        // to make sure that the boot args and the WPR metadata they own are kept alive until halt,
+        // in case they are still being accessed.
         let mut unload_guard =
             ScopeGuard::new_with_data((unload_bundle, ctx), |(unload_bundle, ctx)| {
                 let _ = unload_bundle.0.run(ctx);

-- 
2.55.0

Re: [PATCH v3 2/5] gpu: nova-core: rename heap size field

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Framebuffer layout calculations use raw subtraction on hardware-provided values, leading to integer underflow if the hardware reports an insufficient framebuffer size.
--

--- Patch [2]: [PATCH v3 2/5] gpu: nova-core: rename heap size field ---
commit 11d9ee61e9c07ef3fa88364b2849b70d424c5735
Author: Eliot Courtney <ecourtney@nvidia.com>

gpu: nova-core: rename heap size field

This commit renames the heap field to non_wpr_heap in the nova-core driver's
framebuffer layout struct, unifying the naming convention.

> diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
> index 273cff752fae7..1f7b6a69e4db8 100644
> --- a/drivers/gpu/nova-core/fb.rs
> +++ b/drivers/gpu/nova-core/fb.rs
[ ... ]
> @@ -251,9 +251,9 @@ pub(crate) fn new(chipset: Chipset, bar: Bar0<'_>, gsp_fw: &GspFirmware) -> Resu
>              FbRange(wpr2_addr..frts.end)
>          };
>  
> -        let heap = {
> -            let heap_size = u64::from(hal.non_wpr_heap_size());
> -            FbRange(wpr2.start - heap_size..wpr2.start)
> +        let non_wpr_heap = {
> +            let non_wpr_heap_size = u64::from(hal.non_wpr_heap_size());
> +            FbRange(wpr2.start - non_wpr_heap_size..wpr2.start)

[Severity: High]
This isn't a bug introduced by this patch, but could this subtraction underflow
if the hardware reports a framebuffer size smaller than the required components?

The layout calculations in FbLayout::new() sequentially subtract required
firmware region sizes from the total hardware-provided vidmem_size. For example,
an earlier calculation does fb.end - NV_PRAMIN_SIZE, which would underflow if
the reported size is less than 1MB. Similarly, the subtraction
wpr2.start - non_wpr_heap_size here will underflow if the accumulated
allocations exceed the total vidmem_size.

Could this cause a panic in debug builds, or wrap to a very large offset in
release builds, leading to memory corruption or crashes when interacting with
untrusted or malfunctioning hardware?

>          };
>  
>          Ok(Self {

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724-blackwell-fixes-v3-0-01aa6a54e3fd@nvidia.com?part=2

Re: [PATCH v3 3/5] gpu: nova-core: return non-WPR heap size as u64 from HALs

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The driver lacks bounds checking for framebuffer size and VGA workspace addresses read from untrusted device MMIO, leading to integer underflows and kernel panics.
--

--- Patch [3]: [PATCH v3 3/5] gpu: nova-core: return non-WPR heap size as u64 from HALs ---
commit 607f14f3038014abfcbe45e7fcb56d2a70d0bac2
Author: Eliot Courtney <ecourtney@nvidia.com>

gpu: nova-core: return non-WPR heap size as u64 from HALs

This commit changes the non_wpr_heap_size() HAL function to return a u64
directly, avoiding the need for a conversion when calculating the heap size.
It updates the HAL implementations for various chipsets accordingly.

> diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
> index 1f7b6a69e4db8..f5832e59de599 100644
> --- a/drivers/gpu/nova-core/fb.rs
> +++ b/drivers/gpu/nova-core/fb.rs
> @@ -252,7 +252,7 @@ pub(crate) fn new(chipset: Chipset, bar: Bar0<'_>, gsp_fw: &GspFirmware) -> Resu
>          };
>  
>          let non_wpr_heap = {
> -            let non_wpr_heap_size = u64::from(hal.non_wpr_heap_size());
> +            let non_wpr_heap_size = hal.non_wpr_heap_size();
>              FbRange(wpr2.start - non_wpr_heap_size..wpr2.start)

[Severity: High]
This is a pre-existing issue, but since we are modifying size calculations
here in FbLayout::new(), could a malicious device cause an integer underflow
earlier in this function?

If a device returns a framebuffer size smaller than NV_PRAMIN_SIZE (1MB) from
MMIO, the subtraction fb.end - NV_PRAMIN_SIZE will underflow and panic.

Additionally, the untrusted VGA workspace address read from MMIO isn't bounded
against fb.end. If the VGA workspace address is larger than the framebuffer
size, it can result in a range where start > end. When FbRange::len() is
called on this range, it will unconditionally subtract start from end, causing
another underflow panic.

Could we add upper bounds checking for the framebuffer size and VGA workspace
address against MMIO registers to prevent these crashes?

>          };

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724-blackwell-fixes-v3-0-01aa6a54e3fd@nvidia.com?part=3

Re: [PATCH v3 1/5] gpu: nova-core: correct FRTS vidmem offset calculation

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Brittle Logic / Incorrect Condition for 2MiB alignment

Pre-existing issues:
- [Critical] Architecture/Memory Layout Mismatch
--

--- Patch [1]: [PATCH v3 1/5] gpu: nova-core: correct FRTS vidmem offset calculation ---
commit 281515add8e49de6b8ca823d190ac6318796ffcf
Author: Eliot Courtney <ecourtney@nvidia.com>

gpu: nova-core: correct FRTS vidmem offset calculation

The commit updates how the FRTS vidmem offset is calculated for the FSP Chain
of Trust message. It calculates this offset using a new per-architecture
reserved size at the end of the framebuffer, plus the PMU reserved size.

> diff --git a/drivers/gpu/nova-core/fsp.rs b/drivers/gpu/nova-core/fsp.rs
> index 1475485bded32..30e53df81aa2f 100644
> --- a/drivers/gpu/nova-core/fsp.rs
> +++ b/drivers/gpu/nova-core/fsp.rs
> @@ -134,20 +134,31 @@ struct FspCotMessage {
>  }
>  
>  impl FspCotMessage {
> +    /// Computes the FRTS vidmem offset for the Chain-of-Trust message. It is measured from the end
> +    /// of the framebuffer.
> +    fn frts_vidmem_offset(hal: &dyn hal::FspHal, fb_layout: &FbLayout) -> Result<u64> {
> +        let mut offset = u64::from(hal.fb_end_reserved_size());
> +

[Severity: Critical]
This is a pre-existing issue, but by updating frts_vidmem_offset() to use
hal.fb_end_reserved_size() + pmu_reserved_size, there is now a mismatch with
how FbLayout::new() calculates the FRTS base address.

FbLayout::new() calculates frts_base by subtracting frts_size directly from
vga_workspace.start:

drivers/gpu/nova-core/fb.rs:FbLayout::new() {
    let frts = {
        ...
        let frts_size: u64 = hal.frts_size();
        let frts_base = vga_workspace.start.align_down(FRTS_DOWN_ALIGN) - frts_size;
        ...
    }
}

This completely omits the PMU reserved memory space. For architectures like
GB202, FbLayout places FRTS ~2MiB from the end of the framebuffer, while the
FSP is told it is located ~28MiB from the end.

Could this discrepancy cause a regression where the host driver loads the GSP
firmware into physical memory locations that contradict the FSP's memory
protection firewall?

> +        if fb_layout.pmu_reserved_size != 0 {
> +            offset = (offset + u64::from(fb_layout.pmu_reserved_size))
> +                // The 2 MiB alignment is r570-specific.
> +                .align_up(Alignment::new::<SZ_2M>())
> +                .ok_or(EINVAL)?;
> +        }

[Severity: Medium]
Is it brittle to gate the 2MiB alignment behind fb_layout.pmu_reserved_size != 0?

The comment notes this alignment is r570-specific. Using the presence of a PMU
reservation as a proxy for checking if the firmware is r570 (or CoT version >= 2)
could cause a regression if a future architecture or firmware requires the
alignment but lacks a PMU reservation.

Could this proxy logic fail if an architecture has no PMU reservation but uses
r570-compatible firmware?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724-blackwell-fixes-v3-0-01aa6a54e3fd@nvidia.com?part=1