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.)
diff --git a/drivers/gpu/nova-core/gsp/nvkv.rs
b/drivers/gpu/nova-core/gsp/nvkv.rs
index 0afd6d5c48bd..564a9a93f7cd 100644
--- a/drivers/gpu/nova-core/gsp/nvkv.rs
+++ b/drivers/gpu/nova-core/gsp/nvkv.rs
@@ -27,6 +27,43 @@
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())
+ }
+
+ /// 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 `bytes` to the stream, zero-padded to a `u64` boundary.
+ fn extend_with_padding(&mut self, bytes: &[u8]) -> Result {
+ 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..88f2a03a593f 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,6 @@ 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.encode_op(
Op::zeroed()
.with_key(key)
@@ -230,16 +203,13 @@ 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.
#[inline(always)]
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.encode_op(
Op::zeroed()
.with_key(key)
@@ -247,16 +217,13 @@ 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.as_bytes())
}
/// Encodes a 32-bit array as an ARRAY32 pair, zero-padded to a multiple
of 8 bytes.
#[inline(always)]
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.encode_op(
Op::zeroed()
.with_key(key)
@@ -264,15 +231,13 @@ 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.as_bytes())
}
/// 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.encode_op(
Op::zeroed()
.with_key(key)
@@ -280,8 +245,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.as_bytes())
}
}
@@ -292,8 +256,6 @@ mod tests {
// Tests that each kind of value is encoded to NVKV wire format properly.
#[test]
fn encode_all_value_kinds() -> Result {
- // All keys, indexes, and values are distinct but arbitrary values to
make it easier for the
- // test to catch bugs in the encoded output.
const U32_KEY: KeyId = 0x1001;
const U64_KEY: KeyId = 0x1002;
const ARRAY8_KEY: KeyId = 0x1003;
