Re: [PATCH 2/6] gpu: nova-core: add NVKV encoder

On Thu Aug 20, 2026 at 1:47 AM JST, Danilo Krummrich wrote:
> On Wed Aug 19, 2026 at 6:32 PM CEST, Danilo Krummrich wrote:
>> On Mon Aug 17, 2026 at 2:56 PM CEST, Eliot Courtney wrote:
>>> +    fn push_bytes_with_padding(&mut self, bytes: &[u8]) -> Result {
>>> +        let num_entries = bytes.len().div_ceil(size_of::<u64>());
>>> +        self.backing.reserve(num_entries, GFP_KERNEL)?;
>>> +
>>> +        let spare = self.backing.spare_capacity_mut();
>>> +        let dst = spare.as_mut_ptr().cast::<u8>();
>>> +
>>> +        // SAFETY: At least `bytes.len()` bytes of space are guaranteed since `num_entries`
>>> +        // worth of space was just reserved.
>>> +        unsafe { core::ptr::copy_nonoverlapping(bytes.as_ptr(), dst, bytes.len()) };
>>> +
>>> +        let padding = num_entries * size_of::<u64>() - bytes.len();
>>> +        if padding > 0 {
>>> +            // SAFETY: At least `num_entries * size_of::<u64>()` bytes of space are guaranteed.
>>> +            unsafe { core::ptr::write_bytes(dst.add(bytes.len()), 0, padding) };
>>> +        }
>>> +
>>> +        // SAFETY: These bytes were just initialized and every bit pattern is valid for `u64`.
>>> +        unsafe { self.backing.inc_len(num_entries) };
>>> +
>>> +        Ok(())
>>> +    }
>>
>> Ick! That's a lot of unsafe code. I think we can avoid this by using KVVec<u8>
>> instead of KVVec<u64>, ideally in a new type that upholds the padding invariant.
>>
>> Here's a diff of what I came up with; note that it also gets us rid of the
>> unsafe in take_u32s() in the decoder by using zerocopy.
>>
>> (Technically it would also be possible to make Cursor operate on a byte stream
>> and let zerocopy to the rest, as all the take methods are fallible already. But
>> I think the invariant on EncodedStream makes sense.)
>
> Actually, I forgot to add the optimization you made back in, here's the proper
> diff:
>
> (Also used T: IntoBytes as argument for extend_with_padding().)

I sent the respin of this series using your EncodedStream idea but I
think I should add you as Co-developed-by. If that makes sense I will
add it plus your Signed-off-by (I think that's the right process?).