Skip to contentSkip to conversation

gpu: nova-core: firmware: fwsec: use zeroed() to zero-pad buffers

PATCH Superseded Thorsten Blum 2 participants 2 messages Oct 7

Subsystems (4)
DRM drivers DRM Rust nova-core (Rust) nova-drm (Rust)

v2 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Thorsten Blum

author
#0 lore raw

PATCH v2 1/1 gpu: nova-core: firmware: fwsec: use zeroed() to zero-pad buffers

Use KVec::zeroed() to allocate the zero-padded microcode buffer and copy the bootloader code into it. Also use KVVec::zeroed() to initialize the firmware object's leading zero padding.

Signed-off-by Thorsten Blum <[email protected]>

---
Changes in v2:
- Add a // PANIC comment to copy_from_slice() (Danilo)
- v1: https://lore.kernel.org/r/[email protected]/

---
 drivers/gpu/nova-core/firmware/fwsec/bootloader.rs | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/gpu/nova-core/firmware/fwsec/bootloader.rs +4 -6
index ec4d92317a93..1aa08ae8de78 100644
@@ -137,10 +137,9 @@ pub(crate) fn new(
                 .align_up(Alignment::new::<{ falcon::MEM_BLOCK_ALIGNMENT }>())
                 .ok_or(EINVAL)?;
 
-            let mut ucode = KVec::with_capacity(aligned_code_size, GFP_KERNEL)?;
-            ucode.extend_from_slice(code, GFP_KERNEL)?;
-            ucode.resize(aligned_code_size, 0, GFP_KERNEL)?;
-
+            let mut ucode = KVec::zeroed(aligned_code_size, GFP_KERNEL)?;
+            // PANIC: ucode is sliced to code.len(); code.len() <= aligned_code_size.
+            ucode[..code.len()].copy_from_slice(code);
             ucode
         };
 

@@ -162,8 +161,7 @@ pub(crate) fn new(
                     .ok_or(EOVERFLOW)?
             };
 
-            let mut firmware_obj = KVVec::new();
-            firmware_obj.extend_with(align_padding, 0u8, GFP_KERNEL)?;
+            let mut firmware_obj = KVVec::zeroed(align_padding, GFP_KERNEL)?;
             firmware_obj.extend_from_slice(firmware.ucode.0.as_slice(), GFP_KERNEL)?;
 
             (
maintainer
#1 lore raw
Quoted 18 lines · Thorsten Blum

Use KVec::zeroed() to allocate the zero-padded microcode buffer and copy the bootloader code into it. Also use KVVec::zeroed() to initialize the firmware object's leading zero padding.

Signed-off-by Thorsten Blum <[email protected]>

---
Changes in v2:
- Add a // PANIC comment to copy_from_slice() (Danilo)
- v1: https://lore.kernel.org/r/[email protected]/

---
 drivers/gpu/nova-core/firmware/fwsec/bootloader.rs | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)
drivers/gpu/nova-core/firmware/fwsec/bootloader.rs +0 -0
index ec4d92317a93..1aa08ae8de78 100644

drivers/gpu/nova-core/firmware/fwsec/bootloader.rs pub(crate) fn new( · line 142

Quoted 10 lines
drivers/gpu/nova-core/firmware/fwsec/bootloader.rs +3 -4
@@ -137,10 +137,9 @@ pub(crate) fn new(
                 .align_up(Alignment::new::<{ falcon::MEM_BLOCK_ALIGNMENT }>())
                 .ok_or(EINVAL)?;
 
-            let mut ucode = KVec::with_capacity(aligned_code_size, GFP_KERNEL)?;
-            ucode.extend_from_slice(code, GFP_KERNEL)?;
-            ucode.resize(aligned_code_size, 0, GFP_KERNEL)?;
-
+            let mut ucode = KVec::zeroed(aligned_code_size, GFP_KERNEL)?;
+            // PANIC: ucode is sliced to code.len(); code.len() <= aligned_code_size.
+            ucode[..code.len()].copy_from_slice(code);

As I mentioned on the previous revision, I think not inserting panic sites should take priority over other considerations. Besides this code effectively writes he code area twice (not that this is a big concern).

I guess if we want to introduce a new Vec API to cover this better, it should be some kind of constructor cloning a slice - from_slice maybe?

drivers/gpu/nova-core/firmware/fwsec/bootloader.rs pub(crate) fn new( · line 164

Quoted 10 lines
drivers/gpu/nova-core/firmware/fwsec/bootloader.rs +1 -2
             ucode
         };
 

@@ -162,8 +161,7 @@ pub(crate) fn new(
                     .ok_or(EOVERFLOW)?
             };
 
-            let mut firmware_obj = KVVec::new();
-            firmware_obj.extend_with(align_padding, 0u8, GFP_KERNEL)?;
+            let mut firmware_obj = KVVec::zeroed(align_padding, GFP_KERNEL)?;

This hunk is fine though.