On Thu Aug 27, 2026 at 11:12 PM JST, Eliot Courtney wrote:
<...>
> +/// An encoded NVKV byte stream.
> +///
> +/// # Invariants
> +///
> +/// The byte length is always a multiple of `size_of::<u64>()`.
> +pub(crate) struct EncodedStream(Vec<u8, StreamAllocator>);
> +
> +impl EncodedStream {
> + /// Creates an empty stream.
> + fn new() -> Self {
> + // INVARIANT: An empty stream's byte length is 0, a multiple of
> `size_of::<u64>()`.
> + Self(Vec::new())
> + }
> +
> + /// Appends a single `u64` to the stream.
> + fn push_u64(&mut self, value: u64) -> Result {
> + // INVARIANT: Appending `size_of::<u64>()` bytes keeps the byte
> length a multiple of
> + // `size_of::<u64>()`.
> + 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();
> + let padded = bytes.len().next_multiple_of(size_of::<u64>());
> + // Reserve so that a failed allocation can't leave the invariant
> violated.
> + self.0.reserve(padded, GFP_KERNEL)?;
> + self.0.extend_from_slice(bytes, GFP_KERNEL)?;
> + // INVARIANT: The padding ensures the total length remains a
> multiple of
> + // `size_of::<u64>()`.
> + Ok(self.0.extend_with(padded - bytes.len(), 0u8, GFP_KERNEL)?)
> + }
> +}
> +
> +// The Deref to &[u64] relies on this alignment guarantee.
nit: `&[u64]`.
<...>
> +/// Describes the format of the following NVKV operation.
> +#[derive(Debug, Copy, Clone, PartialEq, Eq)]
> +#[repr(u8)]
> +enum Opcode {
> + /// A 32-bit value in the op word.
> + Imm32 = 0,
> + /// 32-bit values for consecutive keys, starting at the op word's key.
> + Seq32 = 1,
> + /// 64-bit values for consecutive keys, starting at the op word's key.
> + Seq64 = 2,
> + /// An array of bytes.
> + Array8 = 3,
> + /// An array of 32-bit elements.
> + Array32 = 4,
> + /// An array of 64-bit elements.
> + Array64 = 5,
> +}
> +
> +// TODO[FPRI]: This is a temporary solution to be replaced with the
> corresponding derive macros once
> +// they land.
Actually, can't you use the nova-core local `bounded_enum!` macro to
define `OpCode`? This would generate the implementations below automatically.
> +impl TryFrom<Bounded<u64, 4>> for Opcode {
> + type Error = Error;
> +
> + fn try_from(value: Bounded<u64, 4>) -> Result<Self> {
> + match value.get() {
> + 0 => Ok(Self::Imm32),
> + 1 => Ok(Self::Seq32),
> + 2 => Ok(Self::Seq64),
> + 3 => Ok(Self::Array8),
> + 4 => Ok(Self::Array32),
> + 5 => Ok(Self::Array64),
> + _ => Err(EINVAL),
> + }
> + }
> +}
> +
> +impl From<Opcode> for Bounded<u64, 4> {
> + fn from(value: Opcode) -> Self {
> + Bounded::from_expr(value as u64)
> + }
> +}
Sashiko has a point that `from` requires `#[inline(always)]`. Ideally,
we prefer to avoid using `from_expr` when we can, which in this case we
can by doing an exhaustive enumeration. Which is exactly what the
`bounded_enum` does, so leveraging it also solves this issue. :)
> diff --git a/drivers/gpu/nova-core/gsp/nvkv/encode.rs
> b/drivers/gpu/nova-core/gsp/nvkv/encode.rs
> new file mode 100644
> index 000000000000..6c1a9cbd90e8
> --- /dev/null
> +++ b/drivers/gpu/nova-core/gsp/nvkv/encode.rs
> @@ -0,0 +1,210 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION &
> AFFILIATES. All rights reserved.
> +
> +#![cfg_attr(not(CONFIG_KUNIT), expect(dead_code))]
> +
> +use kernel::prelude::*;
> +
> +use super::{
> + EncodedStream,
> + Index,
> + KeyId,
> + Op,
> + Opcode, //
> +};
> +
> +/// An encoder for an NVKV stream.
> +pub(crate) struct Encoder {
> + stream: EncodedStream,
> +}
> +
> +impl Encoder {
> + /// Creates an empty encoder.
> + pub(crate) fn new() -> Self {
> + Self {
> + stream: EncodedStream::new(),
> + }
> + }
> +
> + /// Returns the encoded data.
> + #[must_use = "encoded stream must be consumed"]
> + pub(crate) fn finish(self) -> EncodedStream {
> + self.stream
> + }
> +
> + #[inline(always)]
Unless not inlining implies a build error (as is the case for
`Bounded::from_expr`), I think the convention is to stick to `#[inline]`
for these.
Same applies for other methods.
