Re: [PATCH 05/27] gpu: nova-core: zero-pad radix3 page table levels to page boundary

On 8/19/26 10:41 AM, Timur Tabi wrote:
> On Tue, 2026-08-18 at 20:51 -0700, John Hubbard wrote:
>> diff --git a/drivers/gpu/nova-core/firmware/radix3.rs b/drivers/gpu/nova-
>> core/firmware/radix3.rs
>> index b60611c7bea0..f14ad4e82d3d 100644
>> --- a/drivers/gpu/nova-core/firmware/radix3.rs
>> +++ b/drivers/gpu/nova-core/firmware/radix3.rs
>> @@ -67,22 +67,18 @@ pub(crate) fn new<'a>(
>>              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))?
>> +                    let level2 = VVec::<u8>::with_capacity(lvl_size(&data), GFP_KERNEL)
>> +                        .map_err(|_| ENOMEM)
>> +                        .and_then(|level2| map_into_lvl(&data, level2))?;
>> +
>> +                    SGTable::new(dev, level2, DataDirection::ToDevice, GFP_KERNEL)
> 
> Could you use the new Vec::zeroed() to get a buffer that's already all zeroed-out?

Yes. The incremental diff below does that, and is passing my runtime tests:

<blueforge> linux-github (nova-core-run-on-r615-or-later-v2)$ git d -- drivers/gpu/nova-core/firmware/radix3.rs
diff --git a/drivers/gpu/nova-core/firmware/radix3.rs b/drivers/gpu/nova-core/firmware/radix3.rs
index f14ad4e82d3d..b0630fd96c01 100644
--- a/drivers/gpu/nova-core/firmware/radix3.rs
+++ b/drivers/gpu/nova-core/firmware/radix3.rs
@@ -67,16 +67,12 @@ pub(crate) fn new<'a>(
             Ok(try_pin_init!(Self {
                 data <- SGTable::new(dev, data, DataDirection::ToDevice, GFP_KERNEL),
                 level2 <- {
-                    let level2 = VVec::<u8>::with_capacity(lvl_size(&data), GFP_KERNEL)
-                        .map_err(|_| ENOMEM)
-                        .and_then(|level2| map_into_lvl(&data, level2))?;
+                    let level2 = build_lvl(&data)?;
 
                     SGTable::new(dev, level2, DataDirection::ToDevice, GFP_KERNEL)
                 },
                 level1 <- {
-                    let level1 = VVec::<u8>::with_capacity(lvl_size(&level2), GFP_KERNEL)
-                        .map_err(|_| ENOMEM)
-                        .and_then(|level1| map_into_lvl(&level2, level1))?;
+                    let level1 = build_lvl(&level2)?;
 
                     SGTable::new(dev, level1, DataDirection::ToDevice, GFP_KERNEL)
                 },
@@ -110,7 +106,7 @@ pub(crate) fn size(&self) -> usize {
 }
 
 /// Returns the size, in bytes, of the page table level that maps `sg_table`: one `u64` entry per
-/// 4KB page it spans, rounded up to the page boundary that [`map_into_lvl`] pads to.
+/// 4KB page it spans, rounded up to a whole number of `GSP_PAGE_SIZE` pages.
 fn lvl_size(sg_table: &SGTable<Owned<VVec<u8>>>) -> usize {
     let entries: usize = sg_table
         .iter()
@@ -120,26 +116,34 @@ fn lvl_size(sg_table: &SGTable<Owned<VVec<u8>>>) -> usize {
     (entries * size_of::<u64>()).next_multiple_of(GSP_PAGE_SIZE)
 }
 
-/// Build a page table from a scatter-gather list.
+/// Builds a page table level 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>> {
+///
+/// The returned buffer spans a whole number of `GSP_PAGE_SIZE` pages, and every byte past the
+/// last entry is zero. The booter DMAs each level a whole page at a time.
+///
+/// Returns `ENOMEM` if the level cannot be allocated, and `EINVAL` if `sg_table` spans more
+/// pages than [`lvl_size`] accounted 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());
         }
     }
 
-    // The last page of a level is only partly filled, and the booter DMAs each level a
-    // whole page at a time, so no entry past the last valid one may hold a stale address.
-    let padded = dst.len().next_multiple_of(GSP_PAGE_SIZE);
-    dst.resize(padded, 0, GFP_KERNEL)?;
-
     Ok(dst)
 }



thanks,
-- 
John Hubbard