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().)
diff --git a/drivers/gpu/nova-core/gsp/nvkv.rs
b/drivers/gpu/nova-core/gsp/nvkv.rs
index 0afd6d5c48bd..98a54bdeff52 100644
--- a/drivers/gpu/nova-core/gsp/nvkv.rs
+++ b/drivers/gpu/nova-core/gsp/nvkv.rs
@@ -20,6 +20,7 @@
num::Bounded,
prelude::*, //
};
+use zerocopy::Immutable;
mod encode;
pub(crate) use encode::*;
@@ -27,6 +28,49 @@
mod decode;
pub(crate) use decode::*;
+/// An encoded NVKV byte stream.
+///
+/// # Invariants
+///
+/// The byte length is always a multiple of `size_of::<u64>()`.
+pub(crate) struct EncodedStream(KVVec<u8>);
+
+impl EncodedStream {
+ /// Creates an empty stream.
+ fn new() -> Self {
+ Self(KVVec::new())
+ }
+
+ /// Reserves capacity for at least `additional` u64 entries.
+ fn reserve(&mut self, additional: usize) -> Result {
+ Ok(self.0.reserve(additional * size_of::<u64>(), GFP_KERNEL)?)
+ }
+
+ /// Appends a single `u64` to the stream.
+ fn push_u64(&mut self, value: u64) -> Result {
+ Ok(self.0.extend_from_slice(&value.to_ne_bytes(), GFP_KERNEL)?)
+ }
+
+ /// Appends `data` as bytes to the stream, zero-padded to a `u64` boundary.
+ fn extend_with_padding<T: IntoBytes + Immutable + ?Sized>(&mut self, data:
&T) -> Result {
+ let bytes = data.as_bytes();
+ self.0.extend_from_slice(bytes, GFP_KERNEL)?;
+ let padding = bytes.len().next_multiple_of(size_of::<u64>()) -
bytes.len();
+ // INVARIANT: The padding ensures the total length remains a multiple
of `size_of::<u64>()`.
+ Ok(self.0.extend_with(padding, 0u8, GFP_KERNEL)?)
+ }
+}
+
+impl Deref for EncodedStream {
+ type Target = [u64];
+
+ fn deref(&self) -> &[u64] {
+ let count = self.0.len() / size_of::<u64>();
+ <[u64]>::ref_from_bytes_with_elems(&self.0, count)
+ .expect("EncodedStream invariant violated: not u64-aligned")
+ }
+}
+
/// The identifier of an NVKV key.
pub(crate) type KeyId = u16;
diff --git a/drivers/gpu/nova-core/gsp/nvkv/decode.rs
b/drivers/gpu/nova-core/gsp/nvkv/decode.rs
index 9112dcf1aaca..d1271917c62e 100644
--- a/drivers/gpu/nova-core/gsp/nvkv/decode.rs
+++ b/drivers/gpu/nova-core/gsp/nvkv/decode.rs
@@ -381,9 +381,9 @@ fn take_u8s(&mut self, count: usize) -> Result<&[u8]> {
fn take_u32s(&mut self, count: usize) -> Result<&[u32]> {
let values = self.take_u64s(count.div_ceil(2))?;
- // SAFETY: `values` is 8 byte aligned and only 4 byte alignment is
required. All bit
- // patterns are valid for `u32`.
- Ok(unsafe { core::slice::from_raw_parts(values.as_ptr().cast::<u32>(),
count) })
+ let bytes = values.as_bytes();
+ <[u32]>::ref_from_bytes_with_elems(&bytes[..count * size_of::<u32>()],
count)
+ .map_err(|_| EINVAL)
}
fn take_u64s(&mut self, count: usize) -> Result<&[u64]> {
@@ -401,8 +401,11 @@ pub(crate) struct Decoder<'a> {
impl<'a> Decoder<'a> {
/// Creates a decoder for `data` that handles unknown keys per `policy`.
- pub(crate) fn new(data: &'a [u64], policy: UnknownKeyPolicy) -> Self {
- Self { data, policy }
+ pub(crate) fn new(data: &'a super::EncodedStream, policy:
UnknownKeyPolicy) -> Self {
+ Self {
+ data: &**data,
+ policy,
+ }
}
fn visit<S: Schema>(
diff --git a/drivers/gpu/nova-core/gsp/nvkv/encode.rs
b/drivers/gpu/nova-core/gsp/nvkv/encode.rs
index 31ea5788e772..4f599939d6e7 100644
--- a/drivers/gpu/nova-core/gsp/nvkv/encode.rs
+++ b/drivers/gpu/nova-core/gsp/nvkv/encode.rs
@@ -156,51 +156,26 @@ fn encode(&self, encoder: &mut Encoder) -> Result {
/// An encoder for an NVKV stream.
pub(crate) struct Encoder {
- backing: KVVec<u64>,
+ stream: super::EncodedStream,
}
impl Encoder {
/// Creates an empty encoder.
pub(crate) fn new() -> Self {
Self {
- backing: KVVec::new(),
+ stream: super::EncodedStream::new(),
}
}
- /// Appends `bytes` to the stream, padded to a multiple of 8 bytes.
- 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(())
- }
-
/// Returns the encoded data.
#[must_use = "encoded data must be consumed"]
- pub(crate) fn finish(self) -> KVVec<u64> {
- self.backing
+ pub(crate) fn finish(self) -> super::EncodedStream {
+ self.stream
}
#[inline(always)]
fn encode_op(&mut self, op: Op) -> Result {
- self.backing.push(op.into_raw(), GFP_KERNEL)?;
- Ok(())
+ self.stream.push_u64(op.into_raw())
}
/// Encodes a 32-bit value as an IMM32 pair, with the value in the op word.
@@ -213,8 +188,7 @@ pub(crate) fn encode_u32(&mut self, key: KeyId, index:
Index, value: u32) -> Res
.with_index(index)
.with_opcode(Opcode::Imm32)
.with_value(value),
- )?;
- Ok(())
+ )
}
/// Encodes a 64-bit value as a single-element SEQ64 pair.
@@ -222,7 +196,7 @@ pub(crate) fn encode_u32(&mut self, key: KeyId, index:
Index, value: u32) -> Res
pub(crate) fn encode_u64(&mut self, key: KeyId, index: Index, value: u64)
-> Result {
// TODO: Consider automatically merging sequential keys.
const KEY_COUNT: u32 = 1;
- self.backing.reserve(2, GFP_KERNEL)?;
+ self.stream.reserve(2)?;
self.encode_op(
Op::zeroed()
.with_key(key)
@@ -230,8 +204,7 @@ pub(crate) fn encode_u64(&mut self, key: KeyId, index:
Index, value: u64) -> Res
.with_opcode(Opcode::Seq64)
.with_value(KEY_COUNT),
)?;
- self.backing.push_within_capacity(value)?;
- Ok(())
+ self.stream.push_u64(value)
}
/// Encodes a byte array as an ARRAY8 pair, zero-padded to a multiple of 8
bytes.
@@ -239,7 +212,7 @@ pub(crate) fn encode_u64(&mut self, key: KeyId, index:
Index, value: u64) -> Res
pub(crate) fn encode_array8(&mut self, key: KeyId, index: Index, array:
&[u8]) -> Result {
let value_count = u32::try_from(array.len()).map_err(|_| EMSGSIZE)?;
let num_entries = array.len().div_ceil(size_of::<u64>());
- self.backing.reserve(num_entries + 1, GFP_KERNEL)?;
+ self.stream.reserve(num_entries + 1)?;
self.encode_op(
Op::zeroed()
.with_key(key)
@@ -247,8 +220,7 @@ pub(crate) fn encode_array8(&mut self, key: KeyId, index:
Index, array: &[u8]) -
.with_opcode(Opcode::Array8)
.with_value(value_count),
)?;
- self.push_bytes_with_padding(array.as_bytes())?;
- Ok(())
+ self.stream.extend_with_padding(array)
}
/// Encodes a 32-bit array as an ARRAY32 pair, zero-padded to a multiple
of 8 bytes.
@@ -256,7 +228,7 @@ pub(crate) fn encode_array8(&mut self, key: KeyId, index:
Index, array: &[u8]) -
pub(crate) fn encode_array32(&mut self, key: KeyId, index: Index, array:
&[u32]) -> Result {
let value_count = u32::try_from(array.len()).map_err(|_| EMSGSIZE)?;
let num_entries = array.len().div_ceil(2);
- self.backing.reserve(num_entries + 1, GFP_KERNEL)?;
+ self.stream.reserve(num_entries + 1)?;
self.encode_op(
Op::zeroed()
.with_key(key)
@@ -264,15 +236,14 @@ pub(crate) fn encode_array32(&mut self, key: KeyId,
index: Index, array: &[u32])
.with_opcode(Opcode::Array32)
.with_value(value_count),
)?;
- self.push_bytes_with_padding(array.as_bytes())?;
- Ok(())
+ self.stream.extend_with_padding(array)
}
/// Encodes a 64-bit array as an ARRAY64 pair.
#[inline(always)]
pub(crate) fn encode_array64(&mut self, key: KeyId, index: Index, array:
&[u64]) -> Result {
let value_count = u32::try_from(array.len()).map_err(|_| EMSGSIZE)?;
- self.backing.reserve(array.len() + 1, GFP_KERNEL)?;
+ self.stream.reserve(array.len() + 1)?;
self.encode_op(
Op::zeroed()
.with_key(key)
@@ -280,8 +251,7 @@ pub(crate) fn encode_array64(&mut self, key: KeyId, index:
Index, array: &[u64])
.with_opcode(Opcode::Array64)
.with_value(value_count),
)?;
- self.push_bytes_with_padding(array.as_bytes())?;
- Ok(())
+ self.stream.extend_with_padding(array)
}
}
