On Tue Sep 8, 2026 at 12:07 AM JST, Alexandre Courbot wrote:
> 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.
Thanks, particularly on noticing the bounded_enum!
