[PATCH 0/3] radix3 and ELF cleanup, pre-r000

[PATCH 0/3] radix3 and ELF cleanup, pre-r000

[John Hubbard]

I split these three cleanup patches out of the r000 series [1] in order
to shrink it slightly. r000 will still build on top of these, but
because I have some patch re-ordering and cleanup to do there, let's get
this easier small set posted and hopefully merged in the meantime.

This is based on drm-rust-next at commit d669686f8170 ("gpu: nova-core:
mm: Add BAR1 memory management self-tests"). No other dependencies.

In order to boot the GSP, nova-core loads the GSP firmware image into
system memory, and the GPU copies it into the framebuffer, the GPU's own
memory. The GPU finds the GSP firmware image's pages via "radix3", a
three-level page table of 4 KiB pages that the driver builds. Today the
table exists as fields within the GSP firmware wrapper, which is fine
because there is only one radix3 consumer.

The larger r000 series, however, introduces a second radix3 caller: the
ucodes firmware image. That is because the GSP has been changed, so that
it automatically fetches the subset of ucodes that it requires, thus
reducing the size of the GSP image. This is a helpful change for the
firmware situation overall, but it does require nova-core to load and
provide ucodes, in much the same way that it was loading and providing
the GSP firmware image.

So the first patch splits out radix3 so that it can be called by the two
separate code paths. The other two patches fix problems I found in the
same code.

* Patch 1 moves the radix3 table into a type of its own that maps any
  buffer, and makes the GSP firmware wrapper hold one. No functional
  change.

* Patch 2 allocates each level of the table as a whole number of 4 KiB
  pages and zeroes the whole level. The booter, the firmware stage that
  copies the image on Turing, Ampere and Ada GPUs, reads each level a
  whole page at a time, so today it reads stale kernel memory from the
  unused rest of a level's last page.

* Patch 3 renames the field for the framebuffer range that holds the GSP
  firmware image, from elf to fw_image. After this,

      $ cd drivers/gpu/nova-core
      $ grep -iw ELF

  comes up empty. :)

[1] https://lore.kernel.org/all/20260822015448.238214-1-jhubbard@nvidia.com/

John Hubbard (3):
  gpu: nova-core: extract radix3 page table into its own module
  gpu: nova-core: zero-pad radix3 page table levels to page boundary
  gpu: nova-core: rename the FbRanges elf field to fw_image

 drivers/gpu/nova-core/fb.rs              |  18 +--
 drivers/gpu/nova-core/firmware.rs        |   1 +
 drivers/gpu/nova-core/firmware/gsp.rs    | 107 ++---------------
 drivers/gpu/nova-core/firmware/radix3.rs | 140 +++++++++++++++++++++++
 drivers/gpu/nova-core/gsp/fw.rs          |   6 +-
 5 files changed, 164 insertions(+), 108 deletions(-)
 create mode 100644 drivers/gpu/nova-core/firmware/radix3.rs


base-commit: d669686f8170c234edf12212fac9180ea18b1448
-- 
2.55.0

[PATCH 1/3] gpu: nova-core: extract radix3 page table into its own module

[John Hubbard]

In order to boot the GSP, nova-core loads the GSP firmware image into
system memory, and the GPU copies it into the framebuffer, the GPU's own
memory. The GPU finds the GSP firmware image's pages via "radix3", a
three-level page table of 4 KiB pages that the driver builds. Today the
table exists as fields within the GSP firmware wrapper, which is fine
because there is only one radix3 consumer.

The upcoming r000 series[1], however, introduces a second radix3 caller:
the ucodes firmware image. That is because the GSP has been changed, so
that it automatically fetches the subset of ucodes that it requires,
thus reducing the size of the GSP image. This is a helpful change for
the firmware situation overall, but it will require nova-core to load
and provide ucodes, in much the same way that it was loading and
providing the GSP firmware image.

Therefore, split out radix3 into its own module.

[1] https://lore.kernel.org/all/20260822015448.238214-1-jhubbard@nvidia.com/

Assisted-by: LLM
Reviewed-by: Timur Tabi <ttabi@nvidia.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 drivers/gpu/nova-core/fb.rs              |   2 +-
 drivers/gpu/nova-core/firmware.rs        |   1 +
 drivers/gpu/nova-core/firmware/gsp.rs    | 107 ++-----------------
 drivers/gpu/nova-core/firmware/radix3.rs | 126 +++++++++++++++++++++++
 drivers/gpu/nova-core/gsp/fw.rs          |   4 +-
 5 files changed, 141 insertions(+), 99 deletions(-)
 create mode 100644 drivers/gpu/nova-core/firmware/radix3.rs

diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
index b3a6ab8b57a6..b18dc5447a73 100644
--- a/drivers/gpu/nova-core/fb.rs
+++ b/drivers/gpu/nova-core/fb.rs
@@ -236,7 +236,7 @@ pub(crate) fn new(
 
         let elf = {
             const ELF_DOWN_ALIGN: Alignment = Alignment::new::<SZ_64K>();
-            let elf_size = u64::from_safe_cast(gsp_fw.size);
+            let elf_size = u64::from_safe_cast(gsp_fw.size());
             let elf_addr = (boot.start - elf_size).align_down(ELF_DOWN_ALIGN);
 
             FbRange(elf_addr..elf_addr + elf_size)
diff --git a/drivers/gpu/nova-core/firmware.rs b/drivers/gpu/nova-core/firmware.rs
index c16fee6e2b2a..d8f6509a35d1 100644
--- a/drivers/gpu/nova-core/firmware.rs
+++ b/drivers/gpu/nova-core/firmware.rs
@@ -26,6 +26,7 @@
 pub(crate) mod fwsec;
 pub(crate) mod gsp;
 pub(crate) mod gsp_fmc;
+pub(crate) mod radix3;
 pub(crate) mod riscv;
 pub(crate) mod tlv;
 
diff --git a/drivers/gpu/nova-core/firmware/gsp.rs b/drivers/gpu/nova-core/firmware/gsp.rs
index 22d1f9329c9f..341a8b19aa38 100644
--- a/drivers/gpu/nova-core/firmware/gsp.rs
+++ b/drivers/gpu/nova-core/firmware/gsp.rs
@@ -4,21 +4,16 @@
     device,
     dma::{
         Coherent,
-        CoherentBox,
-        DataDirection,
         DmaAddress, //
     },
     firmware,
     prelude::*,
-    scatterlist::{
-        Owned,
-        SGTable, //
-    },
     str::CString,
 };
 
 use crate::{
     firmware::{
+        radix3::Radix3,
         riscv::RiscvFirmware, //
         tlv::{
             request_tlv, //
@@ -26,38 +21,15 @@
         },
     },
     gpu::Chipset,
-    gsp::GSP_PAGE_SIZE,
     num::FromSafeCast,
 };
 
-/// GSP firmware with 3-level radix page tables for the GSP bootloader.
-///
-/// The bootloader expects firmware to be mapped starting at address 0 in GSP's virtual address
-/// space:
-///
-/// ```text
-/// Level 0:  1 page, 1 entry         -> points to first level 1 page
-/// Level 1:  Multiple pages/entries  -> each entry points to a level 2 page
-/// Level 2:  Multiple pages/entries  -> each entry points to a firmware page
-/// ```
-///
-/// Each page is 4KB, each entry is 8 bytes (64-bit DMA address).
-/// Also known as "Radix3" firmware.
+/// The GSP firmware image, its signatures, and the GSP bootloader.
 #[pin_data]
 pub(crate) struct GspFirmware<'a> {
-    /// The GSP firmware inside a [`VVec`], device-mapped via a SG table.
-    #[pin]
-    fw: SGTable<Owned<VVec<u8>>>,
-    /// Level 2 page table whose entries contain DMA addresses of firmware pages.
+    /// The firmware image and the radix3 table that maps it.
     #[pin]
-    level2: SGTable<Owned<VVec<u8>>>,
-    /// Level 1 page table whose entries contain DMA addresses of level 2 pages.
-    #[pin]
-    level1: SGTable<Owned<VVec<u8>>>,
-    /// Level 0 page table (single 4KB page) with one entry: DMA address of first level 1 page.
-    level0: Coherent<'a, [u64]>,
-    /// Size in bytes of the firmware contained in [`Self::fw`].
-    pub(crate) size: usize,
+    radix3: Radix3<'a>,
     /// Device-mapped GSP signatures matching the GPU's [`Chipset`].
     pub(crate) signatures: Coherent<'a, [u8]>,
     /// GSP bootloader, verifies the GSP firmware before loading and running it.
@@ -87,49 +59,7 @@ pub(crate) fn new(
             let signatures = Coherent::from_slice(dev, tlv.get_bytes(b"SIGN")?, GFP_KERNEL)?;
 
             Ok(try_pin_init!(Self {
-                fw <- SGTable::new(dev, fw_vvec, DataDirection::ToDevice, GFP_KERNEL),
-                level2 <- {
-                    // Allocate the level 2 page table, map the firmware onto it, and map it into
-                    // the device address space.
-                    VVec::<u8>::with_capacity(
-                        fw.iter().count() * core::mem::size_of::<u64>(),
-                        GFP_KERNEL,
-                    )
-                    .map_err(|_| ENOMEM)
-                    .and_then(|level2| map_into_lvl(&fw, level2))
-                    .map(|level2| SGTable::new(dev, level2, DataDirection::ToDevice, GFP_KERNEL))?
-                },
-                level1 <- {
-                    // Allocate the level 1 page table, map the level 2 page table onto it, and map
-                    // it into the device address space.
-                    VVec::<u8>::with_capacity(
-                        level2.iter().count() * core::mem::size_of::<u64>(),
-                        GFP_KERNEL,
-                    )
-                    .map_err(|_| ENOMEM)
-                    .and_then(|level1| map_into_lvl(&level2, level1))
-                    .map(|level1| SGTable::new(dev, level1, DataDirection::ToDevice, GFP_KERNEL))?
-                },
-                level0: {
-                    // Allocate the level 0 page table as a device-visible DMA object, and map the
-                    // level 1 page table onto it.
-
-                    // Fill level 1 page entry.
-                    let level1_entry = level1.iter().next().ok_or(EINVAL)?;
-                    let level1_entry_addr = level1_entry.dma_address();
-
-                    // Create level 0 page table data and fill its first entry with the level 1
-                    // table.
-                    let mut level0 = CoherentBox::<'_, [u64]>::zeroed_slice(
-                        dev,
-                        GSP_PAGE_SIZE / size_of::<u64>(),
-                        GFP_KERNEL
-                    )?;
-                    level0[0] = level1_entry_addr.to_le();
-
-                    level0.into()
-                },
-                size,
+                radix3 <- Radix3::new(dev, fw_vvec),
                 signatures,
                 bootloader: {
                     let bl = request_tlv(dev, chipset, "gsp_bootloader")?;
@@ -140,28 +70,13 @@ pub(crate) fn new(
         })
     }
 
-    /// Returns the DMA address of the radix3 level 0 page table.
-    pub(crate) fn radix3_dma_address(&self) -> DmaAddress {
-        self.level0.dma_address()
+    /// Returns the size of the firmware image, in bytes.
+    pub(crate) fn size(&self) -> usize {
+        self.radix3.size()
     }
-}
 
-/// Build a page table from a scatter-gather list.
-///
-/// Takes each DMA-mapped region from `sg_table` and writes page table entries
-/// for all 4KB pages within that region. For example, a 16KB SG entry becomes
-/// 4 consecutive page table entries.
-fn map_into_lvl(sg_table: &SGTable<Owned<VVec<u8>>>, mut dst: VVec<u8>) -> Result<VVec<u8>> {
-    for sg_entry in sg_table.iter() {
-        // Number of pages we need to map.
-        let num_pages = usize::from_safe_cast(sg_entry.dma_len()).div_ceil(GSP_PAGE_SIZE);
-
-        for i in 0..num_pages {
-            let entry = sg_entry.dma_address()
-                + (u64::from_safe_cast(i) * u64::from_safe_cast(GSP_PAGE_SIZE));
-            dst.extend_from_slice(&entry.to_le_bytes(), GFP_KERNEL)?;
-        }
+    /// Returns the DMA address of the radix3 table that maps the firmware image.
+    pub(crate) fn radix3_dma_address(&self) -> DmaAddress {
+        self.radix3.dma_address()
     }
-
-    Ok(dst)
 }
diff --git a/drivers/gpu/nova-core/firmware/radix3.rs b/drivers/gpu/nova-core/firmware/radix3.rs
new file mode 100644
index 000000000000..6b8251ed871d
--- /dev/null
+++ b/drivers/gpu/nova-core/firmware/radix3.rs
@@ -0,0 +1,126 @@
+// SPDX-License-Identifier: GPL-2.0
+// SPDX-FileCopyrightText: Copyright (c) 2025-2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+
+//! The radix3 page table, through which firmware running on the GPU reads a buffer in system
+//! memory. LibOS, the operating system of the GSP, defines the format: three levels of
+//! [`GSP_PAGE_SIZE`] pages, each entry the little-endian DMA address of one page.
+//!
+//! ```text
+//! Level 0:  one page, one entry  ->  the first level 1 page
+//! Level 1:  pages of entries     ->  each entry a level 2 page
+//! Level 2:  pages of entries     ->  each entry a page of the buffer
+//! ```
+
+use core::mem::size_of;
+
+use kernel::{
+    device,
+    dma::{
+        Coherent,
+        CoherentBox,
+        DataDirection,
+        DmaAddress, //
+    },
+    prelude::*,
+    scatterlist::{
+        Owned,
+        SGTable, //
+    },
+};
+
+use crate::{
+    gsp::GSP_PAGE_SIZE,
+    num::FromSafeCast, //
+};
+
+/// A radix3 page table and the data it maps.
+#[pin_data]
+pub(crate) struct Radix3<'a> {
+    /// The mapped data.
+    #[pin]
+    data: SGTable<Owned<VVec<u8>>>,
+    /// Level 2: one entry per page of `data`.
+    #[pin]
+    level2: SGTable<Owned<VVec<u8>>>,
+    /// Level 1: one entry per page of `level2`.
+    #[pin]
+    level1: SGTable<Owned<VVec<u8>>>,
+    /// Level 0: one page, whose single entry is the DMA address of the first `level1` page.
+    level0: Coherent<'a, [u64]>,
+    /// Length of `data`, in bytes.
+    size: usize,
+}
+
+impl<'a> Radix3<'a> {
+    /// Builds a radix3 page table over `data`, mapped for `dev` to read. May sleep.
+    pub(crate) fn new(
+        dev: &'a device::Device<device::Bound>,
+        data: VVec<u8>,
+    ) -> impl PinInit<Self, Error> + 'a {
+        let size = data.len();
+
+        pin_init::pin_init_scope(move || {
+            Ok(try_pin_init!(Self {
+                data <- SGTable::new(dev, data, DataDirection::ToDevice, GFP_KERNEL),
+                level2 <- {
+                    VVec::<u8>::with_capacity(
+                        data.iter().count() * core::mem::size_of::<u64>(),
+                        GFP_KERNEL,
+                    )
+                    .map_err(|_| ENOMEM)
+                    .and_then(|level2| map_into_lvl(&data, level2))
+                    .map(|level2| SGTable::new(dev, level2, DataDirection::ToDevice, GFP_KERNEL))?
+                },
+                level1 <- {
+                    VVec::<u8>::with_capacity(
+                        level2.iter().count() * core::mem::size_of::<u64>(),
+                        GFP_KERNEL,
+                    )
+                    .map_err(|_| ENOMEM)
+                    .and_then(|level1| map_into_lvl(&level2, level1))
+                    .map(|level1| SGTable::new(dev, level1, DataDirection::ToDevice, GFP_KERNEL))?
+                },
+                level0: {
+                    let level1_entry = level1.iter().next().ok_or(EINVAL)?;
+                    let level1_entry_addr = level1_entry.dma_address();
+
+                    let mut level0 = CoherentBox::<'_, [u64]>::zeroed_slice(
+                        dev,
+                        GSP_PAGE_SIZE / size_of::<u64>(),
+                        GFP_KERNEL,
+                    )?;
+                    level0[0] = level1_entry_addr.to_le();
+
+                    level0.into()
+                },
+                size,
+            }))
+        })
+    }
+
+    /// Returns the DMA address of the level 0 page, which is the address of the table.
+    pub(crate) fn dma_address(&self) -> DmaAddress {
+        self.level0.dma_address()
+    }
+
+    /// Returns the length of the mapped data, in bytes.
+    pub(crate) fn size(&self) -> usize {
+        self.size
+    }
+}
+
+/// Appends one level of the table to `dst`: one entry per [`GSP_PAGE_SIZE`] page of each
+/// DMA-mapped region of `sg_table`, in region order.
+fn map_into_lvl(sg_table: &SGTable<Owned<VVec<u8>>>, mut dst: VVec<u8>) -> Result<VVec<u8>> {
+    for sg_entry in sg_table.iter() {
+        let num_pages = usize::from_safe_cast(sg_entry.dma_len()).div_ceil(GSP_PAGE_SIZE);
+
+        for i in 0..num_pages {
+            let entry = sg_entry.dma_address()
+                + (u64::from_safe_cast(i) * u64::from_safe_cast(GSP_PAGE_SIZE));
+            dst.extend_from_slice(&entry.to_le_bytes(), GFP_KERNEL)?;
+        }
+    }
+
+    Ok(dst)
+}
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index 8778c4bf79c0..ae98da42410b 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -187,7 +187,7 @@ pub(crate) fn from_ranges<'a>(
             magic: bindings::GSP_FW_WPR_META_MAGIC as u64,
             revision: u64::from(bindings::GSP_FW_WPR_META_REVISION),
             sysmemAddrOfRadix3Elf: gsp_firmware.radix3_dma_address(),
-            sizeOfRadix3Elf: u64::from_safe_cast(gsp_firmware.size),
+            sizeOfRadix3Elf: u64::from_safe_cast(gsp_firmware.size()),
             sysmemAddrOfBootloader: gsp_firmware.bootloader.ucode.dma_address(),
             sizeOfBootloader: u64::from_safe_cast(gsp_firmware.bootloader.ucode.size()),
             bootloaderCodeOffset: u64::from(gsp_firmware.bootloader.code_offset),
@@ -242,7 +242,7 @@ pub(crate) fn from_sizes<'a>(
             magic: bindings::GSP_FW_WPR_META_MAGIC as u64,
             revision: u64::from(bindings::GSP_FW_WPR_META_REVISION),
             sysmemAddrOfRadix3Elf: gsp_firmware.radix3_dma_address(),
-            sizeOfRadix3Elf: u64::from_safe_cast(gsp_firmware.size),
+            sizeOfRadix3Elf: u64::from_safe_cast(gsp_firmware.size()),
             sysmemAddrOfBootloader: gsp_firmware.bootloader.ucode.dma_address(),
             sizeOfBootloader: u64::from_safe_cast(gsp_firmware.bootloader.ucode.size()),
             bootloaderCodeOffset: u64::from(gsp_firmware.bootloader.code_offset),
-- 
2.55.0

Re: [PATCH 1/3] gpu: nova-core: extract radix3 page table into its own module

[Gary Guo]

On Sun Sep 13, 2026 at 8:54 PM BST, John Hubbard wrote:
> In order to boot the GSP, nova-core loads the GSP firmware image into
> system memory, and the GPU copies it into the framebuffer, the GPU's own
> memory. The GPU finds the GSP firmware image's pages via "radix3", a
> three-level page table of 4 KiB pages that the driver builds. Today the
> table exists as fields within the GSP firmware wrapper, which is fine
> because there is only one radix3 consumer.
>
> The upcoming r000 series[1], however, introduces a second radix3 caller:
> the ucodes firmware image. That is because the GSP has been changed, so
> that it automatically fetches the subset of ucodes that it requires,
> thus reducing the size of the GSP image. This is a helpful change for
> the firmware situation overall, but it will require nova-core to load
> and provide ucodes, in much the same way that it was loading and
> providing the GSP firmware image.
>
> Therefore, split out radix3 into its own module.

I find the name itself confusing if I don't read the docs. The radix in "radix3"
is actually 512 (given 4 KiB page and 64-bit entry) and definitely not 3.

Perhaps name this "radixlv3" or something similar instead? Or perhaps call it
GspPageTable?

Best,
Gary

>
> [1] https://lore.kernel.org/all/20260822015448.238214-1-jhubbard@nvidia.com/
>
> Assisted-by: LLM
> Reviewed-by: Timur Tabi <ttabi@nvidia.com>
> Signed-off-by: John Hubbard <jhubbard@nvidia.com>

Re: [PATCH 1/3] gpu: nova-core: extract radix3 page table into its own module

[John Hubbard]

On 9/13/26 1:36 PM, Gary Guo wrote:
> On Sun Sep 13, 2026 at 8:54 PM BST, John Hubbard wrote:
>> In order to boot the GSP, nova-core loads the GSP firmware image into
>> system memory, and the GPU copies it into the framebuffer, the GPU's own
>> memory. The GPU finds the GSP firmware image's pages via "radix3", a
>> three-level page table of 4 KiB pages that the driver builds. Today the
>> table exists as fields within the GSP firmware wrapper, which is fine
>> because there is only one radix3 consumer.
>>
>> The upcoming r000 series[1], however, introduces a second radix3 caller:
>> the ucodes firmware image. That is because the GSP has been changed, so
>> that it automatically fetches the subset of ucodes that it requires,
>> thus reducing the size of the GSP image. This is a helpful change for
>> the firmware situation overall, but it will require nova-core to load
>> and provide ucodes, in much the same way that it was loading and
>> providing the GSP firmware image.
>>
>> Therefore, split out radix3 into its own module.
> 
> I find the name itself confusing if I don't read the docs. The radix in "radix3"
> is actually 512 (given 4 KiB page and 64-bit entry) and definitely not 3.
> 
> Perhaps name this "radixlv3" or something similar instead? Or perhaps call it
> GspPageTable?
Right, it's all about the number of levels, not the radix value, that's
true. Open RM loves "radix3" so much that I've become tone-deaf to the
name. :)

three_level_radix works, but is silly long for this.

Do we really care enough to insist on this, though? I'd be inclined
to ask to just leave it alone.

thanks,
-- 
John Hubbard

Re: [PATCH 1/3] gpu: nova-core: extract radix3 page table into its own module

[Alexandre Courbot]

On Mon Sep 14, 2026 at 5:56 AM JST, John Hubbard wrote:
> On 9/13/26 1:36 PM, Gary Guo wrote:
>> On Sun Sep 13, 2026 at 8:54 PM BST, John Hubbard wrote:
>>> In order to boot the GSP, nova-core loads the GSP firmware image into
>>> system memory, and the GPU copies it into the framebuffer, the GPU's own
>>> memory. The GPU finds the GSP firmware image's pages via "radix3", a
>>> three-level page table of 4 KiB pages that the driver builds. Today the
>>> table exists as fields within the GSP firmware wrapper, which is fine
>>> because there is only one radix3 consumer.
>>>
>>> The upcoming r000 series[1], however, introduces a second radix3 caller:
>>> the ucodes firmware image. That is because the GSP has been changed, so
>>> that it automatically fetches the subset of ucodes that it requires,
>>> thus reducing the size of the GSP image. This is a helpful change for
>>> the firmware situation overall, but it will require nova-core to load
>>> and provide ucodes, in much the same way that it was loading and
>>> providing the GSP firmware image.
>>>
>>> Therefore, split out radix3 into its own module.
>> 
>> I find the name itself confusing if I don't read the docs. The radix in "radix3"
>> is actually 512 (given 4 KiB page and 64-bit entry) and definitely not 3.
>> 
>> Perhaps name this "radixlv3" or something similar instead? Or perhaps call it
>> GspPageTable?
> Right, it's all about the number of levels, not the radix value, that's
> true. Open RM loves "radix3" so much that I've become tone-deaf to the
> name. :)
>
> three_level_radix works, but is silly long for this.
>
> Do we really care enough to insist on this, though? I'd be inclined
> to ask to just leave it alone.

As long as we agree on where the "3" comes from, I agree that keeping
the name aligned with OpenRM sounds preferable.

[PATCH 2/3] gpu: nova-core: zero-pad radix3 page table levels to page boundary

[John Hubbard]

On Turing through Ada, the booter, a firmware stage that runs on the
GPU's SEC2 falcon, copies the GSP firmware image from system memory into
the framebuffer. It walks the radix3 table to find each page of the
image, and it reads each level of the table by DMA, a whole 4 KiB page
at a time. The last page of a level is only partly filled with entries,
so the bytes past the last entry reach the booter too.

Nova-core allocated each level at the size of its entries and wrote only
the entries, so the rest of the level's last page held whatever the
allocator left there, and the booter read that stale kernel memory.

Allocate each level zeroed and sized to a whole number of 4 KiB pages,
as Open RM does, and write the entries into it.

Assisted-by: LLM
Reviewed-by: Timur Tabi <ttabi@nvidia.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 drivers/gpu/nova-core/firmware/radix3.rs | 50 +++++++++++++++---------
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/nova-core/firmware/radix3.rs b/drivers/gpu/nova-core/firmware/radix3.rs
index 6b8251ed871d..228ed54e6dad 100644
--- a/drivers/gpu/nova-core/firmware/radix3.rs
+++ b/drivers/gpu/nova-core/firmware/radix3.rs
@@ -63,22 +63,12 @@ pub(crate) fn new(
             Ok(try_pin_init!(Self {
                 data <- SGTable::new(dev, data, DataDirection::ToDevice, GFP_KERNEL),
                 level2 <- {
-                    VVec::<u8>::with_capacity(
-                        data.iter().count() * core::mem::size_of::<u64>(),
-                        GFP_KERNEL,
-                    )
-                    .map_err(|_| ENOMEM)
-                    .and_then(|level2| map_into_lvl(&data, level2))
-                    .map(|level2| SGTable::new(dev, level2, DataDirection::ToDevice, GFP_KERNEL))?
+                    build_lvl(&data)
+                        .map(|l2| SGTable::new(dev, l2, DataDirection::ToDevice, GFP_KERNEL))?
                 },
                 level1 <- {
-                    VVec::<u8>::with_capacity(
-                        level2.iter().count() * core::mem::size_of::<u64>(),
-                        GFP_KERNEL,
-                    )
-                    .map_err(|_| ENOMEM)
-                    .and_then(|level1| map_into_lvl(&level2, level1))
-                    .map(|level1| SGTable::new(dev, level1, DataDirection::ToDevice, GFP_KERNEL))?
+                    build_lvl(&level2)
+                        .map(|l1| SGTable::new(dev, l1, DataDirection::ToDevice, GFP_KERNEL))?
                 },
                 level0: {
                     let level1_entry = level1.iter().next().ok_or(EINVAL)?;
@@ -109,16 +99,40 @@ pub(crate) fn size(&self) -> usize {
     }
 }
 
-/// Appends one level of the table to `dst`: one entry per [`GSP_PAGE_SIZE`] page of each
-/// DMA-mapped region of `sg_table`, in region order.
-fn map_into_lvl(sg_table: &SGTable<Owned<VVec<u8>>>, mut dst: VVec<u8>) -> Result<VVec<u8>> {
+/// Returns the size of the level that maps `sg_table`: one `u64` entry per [`GSP_PAGE_SIZE`]
+/// page of each DMA-mapped region, rounded up to whole pages.
+fn lvl_size(sg_table: &SGTable<Owned<VVec<u8>>>) -> usize {
+    let entries: usize = sg_table
+        .iter()
+        .map(|sg_entry| usize::from_safe_cast(sg_entry.dma_len()).div_ceil(GSP_PAGE_SIZE))
+        .sum();
+
+    (entries * size_of::<u64>()).next_multiple_of(GSP_PAGE_SIZE)
+}
+
+/// Builds one level of the table over `sg_table`: one entry per [`GSP_PAGE_SIZE`] page of each
+/// DMA-mapped region, in region order. The level is a whole number of pages and every byte past
+/// the last entry is zero, because the booter reads a level a whole page at a time.
+///
+/// # Errors
+///
+/// - `ENOMEM` if the level cannot be allocated.
+/// - `EINVAL` if `sg_table` yields more entries than [`lvl_size`] sized the level for.
+fn build_lvl(sg_table: &SGTable<Owned<VVec<u8>>>) -> Result<VVec<u8>> {
+    let mut dst = VVec::<u8>::zeroed(lvl_size(sg_table), GFP_KERNEL).map_err(|_| ENOMEM)?;
+    let mut entries = dst.chunks_exact_mut(size_of::<u64>());
+
     for sg_entry in sg_table.iter() {
         let num_pages = usize::from_safe_cast(sg_entry.dma_len()).div_ceil(GSP_PAGE_SIZE);
 
         for i in 0..num_pages {
             let entry = sg_entry.dma_address()
                 + (u64::from_safe_cast(i) * u64::from_safe_cast(GSP_PAGE_SIZE));
-            dst.extend_from_slice(&entry.to_le_bytes(), GFP_KERNEL)?;
+
+            entries
+                .next()
+                .ok_or(EINVAL)?
+                .copy_from_slice(&entry.to_le_bytes());
         }
     }
 
-- 
2.55.0

[PATCH 3/3] gpu: nova-core: rename the FbRanges elf field to fw_image

[John Hubbard]

A TLV firmware file carries the GSP firmware image as a section
extracted from the original ELF, and the section is not itself an ELF.

Before the driver moved to TLV firmware files, it parsed that ELF
itself, and it named the framebuffer range that holds the image for the
ELF. The range kept the name through the move.

Rename the field for the framebuffer range that holds the GSP firmware
image, from elf to fw_image. After this, the following:

    $ cd drivers/gpu/nova-core
    $ grep -iw ELF

comes up empty. :)

Assisted-by: LLM
Reviewed-by: Timur Tabi <ttabi@nvidia.com>
Signed-off-by: John Hubbard <jhubbard@nvidia.com>
---
 drivers/gpu/nova-core/fb.rs     | 18 +++++++++---------
 drivers/gpu/nova-core/gsp/fw.rs |  2 +-
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/nova-core/fb.rs b/drivers/gpu/nova-core/fb.rs
index b18dc5447a73..b0c0b8fe6008 100644
--- a/drivers/gpu/nova-core/fb.rs
+++ b/drivers/gpu/nova-core/fb.rs
@@ -159,7 +159,7 @@ pub(crate) struct FbRanges {
     /// Memory area containing the GSP bootloader image.
     pub(crate) boot: FbRange,
     /// Memory area containing the GSP firmware image.
-    pub(crate) elf: FbRange,
+    pub(crate) fw_image: FbRange,
     /// WPR2 heap.
     pub(crate) wpr2_heap: FbRange,
     /// WPR2 region range, starting with an instance of `GspFwWprMeta`.
@@ -234,25 +234,25 @@ pub(crate) fn new(
             FbRange(bootloader_base..bootloader_base + bootloader_size)
         };
 
-        let elf = {
-            const ELF_DOWN_ALIGN: Alignment = Alignment::new::<SZ_64K>();
-            let elf_size = u64::from_safe_cast(gsp_fw.size());
-            let elf_addr = (boot.start - elf_size).align_down(ELF_DOWN_ALIGN);
+        let fw_image = {
+            const FW_IMAGE_DOWN_ALIGN: Alignment = Alignment::new::<SZ_64K>();
+            let fw_image_size = u64::from_safe_cast(gsp_fw.size());
+            let fw_image_addr = (boot.start - fw_image_size).align_down(FW_IMAGE_DOWN_ALIGN);
 
-            FbRange(elf_addr..elf_addr + elf_size)
+            FbRange(fw_image_addr..fw_image_addr + fw_image_size)
         };
 
         let (vf_partition_count, wpr2_heap_size) = wpr2_heap_params(chipset, vgpu_state, fb.end)?;
 
         let wpr2_heap = {
             const WPR2_HEAP_DOWN_ALIGN: Alignment = Alignment::new::<SZ_1M>();
-            let wpr2_heap_addr = elf
+            let wpr2_heap_addr = fw_image
                 .start
                 .checked_sub(wpr2_heap_size)
                 .ok_or(EOVERFLOW)?
                 .align_down(WPR2_HEAP_DOWN_ALIGN);
 
-            FbRange(wpr2_heap_addr..(elf.start).align_down(WPR2_HEAP_DOWN_ALIGN))
+            FbRange(wpr2_heap_addr..(fw_image.start).align_down(WPR2_HEAP_DOWN_ALIGN))
         };
 
         let wpr2 = {
@@ -273,7 +273,7 @@ pub(crate) fn new(
             vga_workspace,
             frts,
             boot,
-            elf,
+            fw_image,
             wpr2_heap,
             wpr2,
             non_wpr_heap,
diff --git a/drivers/gpu/nova-core/gsp/fw.rs b/drivers/gpu/nova-core/gsp/fw.rs
index ae98da42410b..918a7ae809eb 100644
--- a/drivers/gpu/nova-core/gsp/fw.rs
+++ b/drivers/gpu/nova-core/gsp/fw.rs
@@ -205,7 +205,7 @@ pub(crate) fn from_ranges<'a>(
             gspFwWprStart: ranges.wpr2.start,
             gspFwHeapOffset: ranges.wpr2_heap.start,
             gspFwHeapSize: ranges.wpr2_heap.len(),
-            gspFwOffset: ranges.elf.start,
+            gspFwOffset: ranges.fw_image.start,
             bootBinOffset: ranges.boot.start,
             frtsOffset: ranges.frts.start,
             frtsSize: ranges.frts.len(),
-- 
2.55.0