* [PATCH 1/6] rust: alloc: add Vec::push_init
2026-08-17 12:56 [PATCH 0/6] gpu: nova-core: add NVKV codec Eliot Courtney
@ 2026-08-17 12:56 ` Eliot Courtney
2026-08-17 14:02 ` Gary Guo
2026-08-17 12:56 ` [PATCH 2/6] gpu: nova-core: add NVKV encoder Eliot Courtney
` (4 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Eliot Courtney @ 2026-08-17 12:56 UTC (permalink / raw)
To: Danilo Krummrich, Lorenzo Stoakes, Vlastimil Babka,
Liam R. Howlett, Uladzislau Rezki, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, rust-for-linux,
linux-kernel, nova-gpu, dri-devel, Eliot Courtney
Add `Vec::push_init` which initializes a new element in place. We can't
modify the existing `Vec::push` signature to take an `impl Init<T, E>`
without changing its Error type.
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
rust/kernel/alloc/kvec.rs | 42 +++++++++++++++++++++++++++++++++++++++++-
1 file changed, 41 insertions(+), 1 deletion(-)
diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
index c7546b9da4fa..9f6f25d7e218 100644
--- a/rust/kernel/alloc/kvec.rs
+++ b/rust/kernel/alloc/kvec.rs
@@ -52,7 +52,10 @@
}, //
};
-use pin_init::Zeroable;
+use pin_init::{
+ Init,
+ Zeroable, //
+};
mod errors;
pub use self::errors::{InsertError, PushError, RemoveError};
@@ -359,6 +362,43 @@ pub fn push(&mut self, v: T, flags: Flags) -> Result<(), AllocError> {
Ok(())
}
+ /// Appends an element to the back of the [`Vec`] instance by initializing it in place.
+ ///
+ /// # Examples
+ ///
+ /// ```
+ /// struct Element {
+ /// buf: KVec<u8>,
+ /// }
+ ///
+ /// impl Element {
+ /// fn new() -> impl Init<Self, Error> {
+ /// try_init!(Element {
+ /// buf: KVec::with_capacity(16, GFP_KERNEL)?,
+ /// }? Error)
+ /// }
+ /// }
+ ///
+ /// let mut v: KVec<Element> = KVec::new();
+ /// v.push_init(Element::new(), GFP_KERNEL)?;
+ /// assert!(v[0].buf.is_empty());
+ /// # Ok::<(), Error>(())
+ /// ```
+ pub fn push_init<E>(&mut self, init: impl Init<T, E>, flags: Flags) -> Result<(), E>
+ where
+ E: From<AllocError>,
+ {
+ self.reserve(1, flags)?;
+ // SAFETY: The call to `reserve` was successful, so there is at least one spare slot; the
+ // pointer therefore refers to allocated, aligned memory valid for a write of one `T`.
+ unsafe { init.__init(self.spare_capacity_mut().as_mut_ptr().cast::<T>())? };
+ // SAFETY: The call to `__init` returned `Ok`, so the first spare slot now holds an
+ // initialized `T`. The new length does not exceed the capacity because `reserve` ensured
+ // the capacity is greater than the length by at least one.
+ unsafe { self.inc_len(1) };
+ Ok(())
+ }
+
/// Appends an element to the back of the [`Vec`] instance without reallocating.
///
/// Fails if the vector does not have capacity for the new element.
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 1/6] rust: alloc: add Vec::push_init
2026-08-17 12:56 ` [PATCH 1/6] rust: alloc: add Vec::push_init Eliot Courtney
@ 2026-08-17 14:02 ` Gary Guo
0 siblings, 0 replies; 11+ messages in thread
From: Gary Guo @ 2026-08-17 14:02 UTC (permalink / raw)
To: Eliot Courtney, Danilo Krummrich, Lorenzo Stoakes,
Vlastimil Babka, Liam R. Howlett, Uladzislau Rezki, Miguel Ojeda,
Boqun Feng, Gary Guo, Björn Roy Baron, Benno Lossin,
Andreas Hindborg, Alice Ryhl, Trevor Gross, Daniel Almeida,
Tamir Duberstein, Alexandre Courbot, Onur Özkan,
David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, rust-for-linux,
linux-kernel, nova-gpu, dri-devel
On Mon Aug 17, 2026 at 1:56 PM BST, Eliot Courtney wrote:
> Add `Vec::push_init` which initializes a new element in place. We can't
> modify the existing `Vec::push` signature to take an `impl Init<T, E>`
> without changing its Error type.
>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
> rust/kernel/alloc/kvec.rs | 42 +++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 41 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/alloc/kvec.rs b/rust/kernel/alloc/kvec.rs
> index c7546b9da4fa..9f6f25d7e218 100644
> --- a/rust/kernel/alloc/kvec.rs
> +++ b/rust/kernel/alloc/kvec.rs
> @@ -52,7 +52,10 @@
> }, //
> };
>
> -use pin_init::Zeroable;
> +use pin_init::{
> + Init,
> + Zeroable, //
> +};
>
> mod errors;
> pub use self::errors::{InsertError, PushError, RemoveError};
> @@ -359,6 +362,43 @@ pub fn push(&mut self, v: T, flags: Flags) -> Result<(), AllocError> {
> Ok(())
> }
>
> + /// Appends an element to the back of the [`Vec`] instance by initializing it in place.
> + ///
> + /// # Examples
> + ///
> + /// ```
> + /// struct Element {
> + /// buf: KVec<u8>,
> + /// }
> + ///
> + /// impl Element {
> + /// fn new() -> impl Init<Self, Error> {
> + /// try_init!(Element {
> + /// buf: KVec::with_capacity(16, GFP_KERNEL)?,
> + /// }? Error)
> + /// }
> + /// }
> + ///
> + /// let mut v: KVec<Element> = KVec::new();
> + /// v.push_init(Element::new(), GFP_KERNEL)?;
> + /// assert!(v[0].buf.is_empty());
> + /// # Ok::<(), Error>(())
> + /// ```
> + pub fn push_init<E>(&mut self, init: impl Init<T, E>, flags: Flags) -> Result<(), E>
> + where
> + E: From<AllocError>,
> + {
> + self.reserve(1, flags)?;
> + // SAFETY: The call to `reserve` was successful, so there is at least one spare slot; the
> + // pointer therefore refers to allocated, aligned memory valid for a write of one `T`.
> + unsafe { init.__init(self.spare_capacity_mut().as_mut_ptr().cast::<T>())? };
> + // SAFETY: The call to `__init` returned `Ok`, so the first spare slot now holds an
> + // initialized `T`. The new length does not exceed the capacity because `reserve` ensured
> + // the capacity is greater than the length by at least one.
> + unsafe { self.inc_len(1) };
> + Ok(())
> + }
Thinking about this from a fresh design perspective, I wonder if we can create
something more composable by splitting the allocation and insertion, like entry
APIs do.
So
impl<T, A: Allocator> Vec<T, A> {
pub fn reserve(&mut self, additional: usize, flags: Flags) -> Result<Reservation<'_, T>, AllocError> {
...
}
}
/// Type indicating vector with reserved capacity.
pub struct<'a> Reservation<'a, T> {
}
impl<'a, T> Reservation<'a, T> {
pub fn init(&mut self, i: impl Init<T, E>) -> Result<(), E> {
...
}
}
You can imagine even pushing this further, e.g. have a type indicating just a
single reserved slot. Or perhaps have a type that is `Vec` but with fixed
capacity and cannot reallocate (something like `ArrayVec`) that the reserve
method will return.
Best,
Gary
> +
> /// Appends an element to the back of the [`Vec`] instance without reallocating.
> ///
> /// Fails if the vector does not have capacity for the new element.
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 2/6] gpu: nova-core: add NVKV encoder
2026-08-17 12:56 [PATCH 0/6] gpu: nova-core: add NVKV codec Eliot Courtney
2026-08-17 12:56 ` [PATCH 1/6] rust: alloc: add Vec::push_init Eliot Courtney
@ 2026-08-17 12:56 ` Eliot Courtney
2026-08-17 13:06 ` sashiko-bot
2026-08-17 12:56 ` [PATCH 3/6] gpu: nova-core: add NVKV decoder Eliot Courtney
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Eliot Courtney @ 2026-08-17 12:56 UTC (permalink / raw)
To: Danilo Krummrich, Lorenzo Stoakes, Vlastimil Babka,
Liam R. Howlett, Uladzislau Rezki, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, rust-for-linux,
linux-kernel, nova-gpu, dri-devel, Eliot Courtney
Add an encoder for NVKV, which is the wire format for GMCAPI. The
encoded stream is a sequence of 64-bit values. The first 64-bit value
encodes an op word which describes the function of the next N values.
Essentially, the format encodes a sequence of calls to some function
f(key, index, value), where value is a [u8], u32, u64, [u32], or a
[u64]. The key is a u16 and the index is a 12 bit integer. The
interpretation of these function calls is per GMCAPI.
Add tests for the wire encoding for each primitive.
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/gsp.rs | 1 +
drivers/gpu/nova-core/gsp/nvkv.rs | 79 ++++++++++
drivers/gpu/nova-core/gsp/nvkv/encode.rs | 245 +++++++++++++++++++++++++++++++
3 files changed, 325 insertions(+)
diff --git a/drivers/gpu/nova-core/gsp.rs b/drivers/gpu/nova-core/gsp.rs
index 13f361406a6c..84dfe07ae6ba 100644
--- a/drivers/gpu/nova-core/gsp.rs
+++ b/drivers/gpu/nova-core/gsp.rs
@@ -24,6 +24,7 @@
pub(crate) mod cmdq;
pub(crate) mod commands;
mod fw;
+mod nvkv;
mod regs;
mod sequencer;
diff --git a/drivers/gpu/nova-core/gsp/nvkv.rs b/drivers/gpu/nova-core/gsp/nvkv.rs
new file mode 100644
index 000000000000..b908f66e760d
--- /dev/null
+++ b/drivers/gpu/nova-core/gsp/nvkv.rs
@@ -0,0 +1,79 @@
+// SPDX-License-Identifier: GPL-2.0
+// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved.
+
+//! Codec for NVKV, the binary key-value format of GMCAPI.
+//!
+//! Essentially, the format encodes a sequence of calls to some function f(key, index, value),
+//! where value is a [u8], u32, u64, [u32], or a [u64]. The key is a u16 and the index is a 12 bit
+//! integer. The interpretation of these function calls is per GMCAPI. Generally speaking, the
+//! function calls will map to some struct - for example, f(GPU_NAME_STRING_KEY, 0, b"some gpu")
+//! naturally maps to storing a &str with the GPU name.
+
+#![expect(unused_imports)]
+
+use kernel::{
+ bitfield,
+ num::Bounded,
+ prelude::*, //
+};
+
+mod encode;
+pub(crate) use encode::*;
+
+/// The identifier of an NVKV key.
+pub(crate) type KeyId = u16;
+
+/// The index of an NVKV value.
+pub(crate) type Index = Bounded<u64, 12>;
+
+bitfield! {
+ /// The op word that starts each NVKV operation.
+ struct Op(u64) {
+ 15:0 key;
+ 27:16 index => Index;
+ 31:28 opcode ?=> Opcode;
+ 63:32 value;
+ }
+}
+
+/// 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.
+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)
+ }
+}
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..6da81a371651
--- /dev/null
+++ b/drivers/gpu/nova-core/gsp/nvkv/encode.rs
@@ -0,0 +1,245 @@
+// 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::{
+ Index,
+ KeyId,
+ Op,
+ Opcode, //
+};
+
+/// An encoder for an NVKV stream.
+pub(crate) struct Encoder {
+ backing: KVVec<u64>,
+}
+
+impl Encoder {
+ /// Creates an empty encoder.
+ pub(crate) fn new() -> Self {
+ Self {
+ backing: KVVec::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
+ }
+
+ #[inline(always)]
+ fn encode_op(&mut self, op: Op) -> Result {
+ self.backing.push(op.into_raw(), GFP_KERNEL)?;
+ Ok(())
+ }
+
+ /// Encodes a 32-bit value as an IMM32 pair, with the value in the op word.
+ #[inline(always)]
+ pub(crate) fn encode_u32(&mut self, key: KeyId, index: Index, value: u32) -> Result {
+ // TODO: Consider automatically merging sequential keys.
+ self.encode_op(
+ Op::zeroed()
+ .with_key(key)
+ .with_index(index)
+ .with_opcode(Opcode::Imm32)
+ .with_value(value),
+ )?;
+ Ok(())
+ }
+
+ /// Encodes a 64-bit value as a single-element SEQ64 pair.
+ #[inline(always)]
+ 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)
+ .with_index(index)
+ .with_opcode(Opcode::Seq64)
+ .with_value(KEY_COUNT),
+ )?;
+ self.backing.push_within_capacity(value)?;
+ Ok(())
+ }
+
+ /// 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)
+ .with_index(index)
+ .with_opcode(Opcode::Array8)
+ .with_value(value_count),
+ )?;
+ self.push_bytes_with_padding(array.as_bytes())?;
+ Ok(())
+ }
+
+ /// 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)
+ .with_index(index)
+ .with_opcode(Opcode::Array32)
+ .with_value(value_count),
+ )?;
+ self.push_bytes_with_padding(array.as_bytes())?;
+ Ok(())
+ }
+
+ /// 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)
+ .with_index(index)
+ .with_opcode(Opcode::Array64)
+ .with_value(value_count),
+ )?;
+ self.push_bytes_with_padding(array.as_bytes())?;
+ Ok(())
+ }
+}
+
+#[kunit_tests(nova_core_nvkv_encode)]
+mod tests {
+ use super::*;
+
+ // 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;
+ const ARRAY32_KEY: KeyId = 0x1004;
+ const ARRAY64_KEY: KeyId = 0x1005;
+
+ const U32_VALUE: u32 = 0x1111_2222;
+ const U64_VALUE: u64 = 0x3333_4444_5555_6666;
+ const ARRAY8_VALUE: &[u8] = &[0xaa, 0xbb, 0xcc];
+ const ARRAY32_VALUE: &[u32] = &[0xbbbb_cccc, 0xdddd_eeee];
+ const ARRAY64_VALUE: &[u64] = &[0x0123_4567_89ab_cdef, 0xfedc_ba98_7654_3210];
+
+ let mut encoder = Encoder::new();
+ encoder.encode_u32(U32_KEY, Index::new::<0>(), U32_VALUE)?;
+ encoder.encode_u64(U64_KEY, Index::new::<1>(), U64_VALUE)?;
+ encoder.encode_array8(ARRAY8_KEY, Index::new::<2>(), ARRAY8_VALUE)?;
+ encoder.encode_array32(ARRAY32_KEY, Index::new::<3>(), ARRAY32_VALUE)?;
+ encoder.encode_array64(ARRAY64_KEY, Index::new::<4>(), ARRAY64_VALUE)?;
+
+ let encoded = encoder.finish();
+ assert_eq!(encoded.len(), 10);
+
+ // IMM32 has its value in the op word.
+ assert_eq!(
+ encoded[0],
+ Op::zeroed()
+ .with_key(U32_KEY)
+ .with_index(Index::new::<0>())
+ .with_opcode(Opcode::Imm32)
+ .with_value(U32_VALUE)
+ .into_raw()
+ );
+
+ // The SEQ64 op word followed by the value.
+ assert_eq!(
+ encoded[1],
+ Op::zeroed()
+ .with_key(U64_KEY)
+ .with_index(Index::new::<1>())
+ .with_opcode(Opcode::Seq64)
+ .with_value(1u32)
+ .into_raw()
+ );
+ assert_eq!(encoded[2], U64_VALUE);
+
+ // The ARRAY8 op word has the byte count. The bytes follow, padded out to a whole word.
+ assert_eq!(
+ encoded[3],
+ Op::zeroed()
+ .with_key(ARRAY8_KEY)
+ .with_index(Index::new::<2>())
+ .with_opcode(Opcode::Array8)
+ .with_value(3u32)
+ .into_raw()
+ );
+ assert_eq!(
+ encoded[4],
+ u64::from_le_bytes([0xaa, 0xbb, 0xcc, 0, 0, 0, 0, 0])
+ );
+
+ // The ARRAY32 op word has the element count. The two elements follow in little endian.
+ assert_eq!(
+ encoded[5],
+ Op::zeroed()
+ .with_key(ARRAY32_KEY)
+ .with_index(Index::new::<3>())
+ .with_opcode(Opcode::Array32)
+ .with_value(2u32)
+ .into_raw()
+ );
+ assert_eq!(
+ encoded[6],
+ u64::from(ARRAY32_VALUE[1]) << 32 | u64::from(ARRAY32_VALUE[0])
+ );
+
+ // The ARRAY64 op word has the element count with the two elements after.
+ assert_eq!(
+ encoded[7],
+ Op::zeroed()
+ .with_key(ARRAY64_KEY)
+ .with_index(Index::new::<4>())
+ .with_opcode(Opcode::Array64)
+ .with_value(2u32)
+ .into_raw()
+ );
+ assert_eq!(encoded[8], ARRAY64_VALUE[0]);
+ assert_eq!(encoded[9], ARRAY64_VALUE[1]);
+
+ Ok(())
+ }
+}
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 2/6] gpu: nova-core: add NVKV encoder
2026-08-17 12:56 ` [PATCH 2/6] gpu: nova-core: add NVKV encoder Eliot Courtney
@ 2026-08-17 13:06 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-17 13:06 UTC (permalink / raw)
To: Eliot Courtney; +Cc: ojeda, Danilo Krummrich, dri-devel, Alexandre Courbot
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] NVKV array encoding relies on host memory layout, causing endianness corruption and unconditional KUnit test failures on big-endian architectures.
--
--- Patch [2]: [PATCH 2/6] gpu: nova-core: add NVKV encoder ---
commit 0223916dc52ad28f6ad72c2fdc7087b7939c7ed3
Author: Eliot Courtney <ecourtney@nvidia.com>
gpu: nova-core: add NVKV encoder
This commit adds an encoder for the NVKV wire format, which encodes calls
as a sequence of 64-bit values. It includes functions to encode single
values and arrays, and adds a test for the encoding.
> diff --git a/drivers/gpu/nova-core/gsp/nvkv/encode.rs b/drivers/gpu/nova-core/gsp/nvkv/encode.rs
> index 0000000000000..6da81a371651a 100644
> --- /dev/null
> +++ b/drivers/gpu/nova-core/gsp/nvkv/encode.rs
[ ... ]
> + fn push_bytes_with_padding(&mut self, bytes: &[u8]) -> Result {
[ ... ]
> + // 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(())
> + }
[ ... ]
> + pub(crate) fn encode_array32(&mut self, key: KeyId, index: Index, array: &[u32]) -> Result {
[ ... ]
> + self.push_bytes_with_padding(array.as_bytes())?;
> + Ok(())
> + }
[ ... ]
> + pub(crate) fn encode_array64(&mut self, key: KeyId, index: Index, array: &[u64]) -> Result {
[ ... ]
> + self.push_bytes_with_padding(array.as_bytes())?;
> + Ok(())
> + }
[Severity: High]
Will this produce an incorrect byte stream on big-endian architectures?
Because push_bytes_with_padding() in drivers/gpu/nova-core/gsp/nvkv/encode.rs
copies host memory directly into the u64 backing storage, and encode_array32()
and encode_array64() extract host-endian bytes via array.as_bytes(), the
endianness of the generated stream depends on the host.
[ ... ]
> + assert_eq!(
> + encoded[4],
> + u64::from_le_bytes([0xaa, 0xbb, 0xcc, 0, 0, 0, 0, 0])
> + );
[Severity: High]
Will this test unconditionally fail on big-endian systems?
The test in drivers/gpu/nova-core/gsp/nvkv/encode.rs explicitly asserts that
the generated u64 must match a little-endian layout, which seems to prove
the host-endian implementation above will produce unexpected results on
big-endian architectures.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-b4-nvkv-v1-0-b84db5e84b67@nvidia.com?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 3/6] gpu: nova-core: add NVKV decoder
2026-08-17 12:56 [PATCH 0/6] gpu: nova-core: add NVKV codec Eliot Courtney
2026-08-17 12:56 ` [PATCH 1/6] rust: alloc: add Vec::push_init Eliot Courtney
2026-08-17 12:56 ` [PATCH 2/6] gpu: nova-core: add NVKV encoder Eliot Courtney
@ 2026-08-17 12:56 ` Eliot Courtney
2026-08-17 13:06 ` sashiko-bot
2026-08-17 12:56 ` [PATCH 4/6] gpu: nova-core: add NVKV typed encoding Eliot Courtney
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Eliot Courtney @ 2026-08-17 12:56 UTC (permalink / raw)
To: Danilo Krummrich, Lorenzo Stoakes, Vlastimil Babka,
Liam R. Howlett, Uladzislau Rezki, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, rust-for-linux,
linux-kernel, nova-gpu, dri-devel, Eliot Courtney
Add a decoder for NVKV. This is for receiving messages from GSP for
GMCAPI calls. The NVKV format essentially encodes a sequence of function
calls f(key, index, value). This decoder reads an encoded stream and
invokes a type implementing the new `Schema` visitor trait. The
`Schema` trait can either consume the value or not, which is useful for
composing Schemas. If a (key, index, value) is not consumed, error out
depending on `UnknownKeyPolicy`. Whether ignoring unknown keys is ok or
not is per each GMCAPI call.
Add kunit tests for the decoder.
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/gsp/nvkv.rs | 3 +
drivers/gpu/nova-core/gsp/nvkv/decode.rs | 258 +++++++++++++++++++++++++++++++
2 files changed, 261 insertions(+)
diff --git a/drivers/gpu/nova-core/gsp/nvkv.rs b/drivers/gpu/nova-core/gsp/nvkv.rs
index b908f66e760d..64d8d0118452 100644
--- a/drivers/gpu/nova-core/gsp/nvkv.rs
+++ b/drivers/gpu/nova-core/gsp/nvkv.rs
@@ -20,6 +20,9 @@
mod encode;
pub(crate) use encode::*;
+mod decode;
+pub(crate) use decode::*;
+
/// 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
new file mode 100644
index 000000000000..ee8b6ab5a3a4
--- /dev/null
+++ b/drivers/gpu/nova-core/gsp/nvkv/decode.rs
@@ -0,0 +1,258 @@
+// 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 crate::gsp::nvkv::{
+ Index,
+ KeyId,
+ Op,
+ Opcode, //
+};
+use crate::num;
+
+/// A decoded NVKV value.
+#[derive(Copy, Clone)]
+pub(crate) enum DecoderValue<'a> {
+ Scalar32(u32),
+ Scalar64(u64),
+ Array8(&'a [u8]),
+ Array32(&'a [u32]),
+ Array64(&'a [u64]),
+}
+
+/// Implements `TryFrom` from the given `DecoderValue` variant to the given type.
+///
+/// `TryFrom` is used by the `Schema` implementations in this file to convert from the
+/// `DecoderValue`s into the types to store. Provide the implementations for basic types here.
+macro_rules! impl_try_from_decoder_value {
+ ($ty:ty, $variant:ident) => {
+ impl<'a> TryFrom<DecoderValue<'a>> for $ty {
+ type Error = Error;
+
+ fn try_from(value: DecoderValue<'a>) -> Result<Self> {
+ if let DecoderValue::$variant(v) = value {
+ Ok(v)
+ } else {
+ Err(EINVAL)
+ }
+ }
+ }
+ };
+}
+
+impl_try_from_decoder_value!(u32, Scalar32);
+impl_try_from_decoder_value!(u64, Scalar64);
+impl_try_from_decoder_value!(&'a [u8], Array8);
+impl_try_from_decoder_value!(&'a [u32], Array32);
+impl_try_from_decoder_value!(&'a [u64], Array64);
+
+/// A visitor that consumes decoded NVKV and produces a `Target`.
+pub(crate) trait Schema {
+ type Target;
+
+ /// Visits one decoded pair. Returns `Ok(true)` if the schema consumed it.
+ fn visit<'a>(&mut self, key: KeyId, index: Index, value: DecoderValue<'a>) -> Result<bool>;
+
+ /// Returns an initializer that makes the decoded `Target`.
+ fn finish(self) -> impl Init<Self::Target, Error>;
+}
+
+/// A read position in an NVKV stream.
+struct Cursor<'a> {
+ data: &'a [u64],
+}
+
+impl<'a> Cursor<'a> {
+ fn new(data: &'a [u64]) -> Self {
+ Self { data }
+ }
+
+ fn is_empty(&self) -> bool {
+ self.data.is_empty()
+ }
+
+ fn take_u64(&mut self) -> Result<u64> {
+ // PANIC: `take_u64s(1)` returns exactly one element on success.
+ Ok(self.take_u64s(1)?[0])
+ }
+
+ fn take_u8s(&mut self, count: usize) -> Result<&[u8]> {
+ let values = self.take_u64s(count.div_ceil(8))?;
+ values.as_bytes().get(..count).ok_or(EINVAL)
+ }
+
+ 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) })
+ }
+
+ fn take_u64s(&mut self, count: usize) -> Result<&[u64]> {
+ let (prefix, suffix) = self.data.split_at_checked(count).ok_or(EINVAL)?;
+ self.data = suffix;
+ Ok(prefix)
+ }
+}
+
+/// A decoder for an NVKV stream.
+pub(crate) struct Decoder<'a> {
+ data: &'a [u64],
+ policy: UnknownKeyPolicy,
+}
+
+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 }
+ }
+
+ fn visit<S: Schema>(
+ &self,
+ schema: &mut S,
+ key: KeyId,
+ index: Index,
+ value: DecoderValue<'_>,
+ ) -> Result {
+ let consumed = schema.visit(key, index, value)?;
+ if !consumed && self.policy == UnknownKeyPolicy::Error {
+ Err(EINVAL)
+ } else {
+ Ok(())
+ }
+ }
+
+ fn seq_key(base: KeyId, offset: usize) -> Result<KeyId> {
+ base.checked_add(KeyId::try_from(offset)?).ok_or(EINVAL)
+ }
+
+ /// Decodes every pair into `schema` and returns the result of [`Schema::finish`].
+ pub(crate) fn decode<S: Schema>(&self, mut schema: S) -> Result<impl Init<S::Target, Error>> {
+ let mut cursor = Cursor::new(self.data);
+ while !cursor.is_empty() {
+ let op: Op = cursor.take_u64()?.into();
+
+ let key = op.key().into();
+ let index = op.index();
+ let op_value: u32 = op.value().into();
+ match op.opcode()? {
+ Opcode::Imm32 => {
+ self.visit(&mut schema, key, index, DecoderValue::Scalar32(op_value))?;
+ }
+ Opcode::Seq32 => {
+ let values = cursor.take_u32s(num::u32_as_usize(op_value))?;
+ for (i, &value) in values.iter().enumerate() {
+ let key = Self::seq_key(key, i)?;
+ self.visit(&mut schema, key, index, DecoderValue::Scalar32(value))?;
+ }
+ }
+ Opcode::Seq64 => {
+ let values = cursor.take_u64s(num::u32_as_usize(op_value))?;
+ for (i, &value) in values.iter().enumerate() {
+ let key = Self::seq_key(key, i)?;
+ self.visit(&mut schema, key, index, DecoderValue::Scalar64(value))?;
+ }
+ }
+ Opcode::Array8 => {
+ let value = cursor.take_u8s(num::u32_as_usize(op_value))?;
+ self.visit(&mut schema, key, index, DecoderValue::Array8(value))?;
+ }
+ Opcode::Array32 => {
+ let value = cursor.take_u32s(num::u32_as_usize(op_value))?;
+ self.visit(&mut schema, key, index, DecoderValue::Array32(value))?;
+ }
+ Opcode::Array64 => {
+ let value = cursor.take_u64s(num::u32_as_usize(op_value))?;
+ self.visit(&mut schema, key, index, DecoderValue::Array64(value))?;
+ }
+ };
+ }
+ Ok(schema.finish())
+ }
+}
+
+/// This is defined per call.
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub(crate) enum UnknownKeyPolicy {
+ Ignore,
+ Error,
+}
+
+#[kunit_tests(nova_core_nvkv_decode)]
+mod tests {
+ use super::*;
+
+ use crate::gsp::nvkv::Encoder;
+
+ // Tests that basic decoding into a manually implemented `Schema` works correctly.
+ #[test]
+ fn decode_raw_schema() -> Result {
+ // Decodes an IMM32 pair and a SEQ64 pair (the encoder emits a u64 as a single-element
+ // SEQ64) with a hand written `Schema`. Keys and value constants chosen to distinguish e.g.
+ // saving the wrong value to the wrong location.
+ const SCALAR32_KEY: KeyId = 0x1001;
+ const SCALAR64_KEY: KeyId = 0x1002;
+ const UNKNOWN_KEY: KeyId = 0x2001;
+
+ const SCALAR32_VALUE: u32 = 0x1111_2222;
+ const SCALAR64_VALUE: u64 = 0x3333_4444_5555_6666;
+
+ // The output type of the hand written Schema. In this case, we can have it also implement
+ // `Schema` on itself rather than having a separate carrier type, since the `Schema`
+ // implementation is completely stateless.
+ #[derive(Default)]
+ struct RawSchema {
+ scalar32: u32,
+ scalar64: u64,
+ }
+
+ impl Schema for RawSchema {
+ type Target = Self;
+
+ fn visit(&mut self, key: KeyId, index: Index, value: DecoderValue<'_>) -> Result<bool> {
+ if index != Index::new::<0>() {
+ return Err(EINVAL);
+ }
+ match key {
+ SCALAR32_KEY => self.scalar32 = value.try_into()?,
+ SCALAR64_KEY => self.scalar64 = value.try_into()?,
+ _ => return Ok(false),
+ }
+ Ok(true)
+ }
+
+ fn finish(self) -> impl Init<Self::Target, Error> {
+ Ok(self)
+ }
+ }
+
+ let mut encoder = Encoder::new();
+ encoder.encode_u32(SCALAR32_KEY, Index::new::<0>(), SCALAR32_VALUE)?;
+ encoder.encode_u64(SCALAR64_KEY, Index::new::<0>(), SCALAR64_VALUE)?;
+ let serialized = encoder.finish();
+
+ let decoder = Decoder::new(&serialized, UnknownKeyPolicy::Error);
+ let decoded = KBox::try_init(decoder.decode(RawSchema::default())?, GFP_KERNEL)?;
+
+ assert_eq!(decoded.scalar32, SCALAR32_VALUE);
+ assert_eq!(decoded.scalar64, SCALAR64_VALUE);
+
+ // An unknown key should fail with under `UnknownKeyPolicy::Error` and be skipped under
+ // `UnknownKeyPolicy::Ignore`.
+ let mut encoder = Encoder::new();
+ encoder.encode_u32(UNKNOWN_KEY, Index::new::<0>(), 1)?;
+
+ let serialized = encoder.finish();
+ let decoder = Decoder::new(&serialized, UnknownKeyPolicy::Error);
+ assert!(decoder.decode(RawSchema::default()).is_err());
+
+ let decoder = Decoder::new(&serialized, UnknownKeyPolicy::Ignore);
+ let decoded = KBox::try_init(decoder.decode(RawSchema::default())?, GFP_KERNEL)?;
+ assert_eq!(decoded.scalar32, 0);
+
+ Ok(())
+ }
+}
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 3/6] gpu: nova-core: add NVKV decoder
2026-08-17 12:56 ` [PATCH 3/6] gpu: nova-core: add NVKV decoder Eliot Courtney
@ 2026-08-17 13:06 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-17 13:06 UTC (permalink / raw)
To: Eliot Courtney; +Cc: ojeda, Danilo Krummrich, Alexandre Courbot, dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] NVKV decoder uses host-endian memory pointer casts and byte casting, breaking big-endian compatibility.
--
--- Patch [3]: [PATCH 3/6] gpu: nova-core: add NVKV decoder ---
commit 462ea319dececf0891b941e125a3a7cd586a70f8
Author: Eliot Courtney <ecourtney@nvidia.com>
gpu: nova-core: add NVKV decoder
This commit adds a decoder for NVKV to process GMCAPI call messages from
the GSP. It introduces a Schema visitor trait to process the (key, index,
value) sequences from the encoded stream.
> diff --git a/drivers/gpu/nova-core/gsp/nvkv/decode.rs b/drivers/gpu/nova-core/gsp/nvkv/decode.rs
> new file mode 100644
> index 0000000000000..ee8b6ab5a3a43
> --- /dev/null
> +++ b/drivers/gpu/nova-core/gsp/nvkv/decode.rs
[ ... ]
> + fn take_u8s(&mut self, count: usize) -> Result<&[u8]> {
> + let values = self.take_u64s(count.div_ceil(8))?;
> + values.as_bytes().get(..count).ok_or(EINVAL)
[Severity: High]
Does casting a native [u64] to [u8] using as_bytes() expose the native
byte order of the host? If the GSP sends data in little-endian format, won't
this yield reversed 8-byte chunks on big-endian architectures?
> + }
> +
> + 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) })
[Severity: High]
Could this raw pointer cast from u64 to u32 cause issues on big-endian
systems? This appears to change the order of the 32-bit words compared to
the original little-endian GSP message layout.
> + }
> +
> + fn take_u64s(&mut self, count: usize) -> Result<&[u64]> {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-b4-nvkv-v1-0-b84db5e84b67@nvidia.com?part=3
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 4/6] gpu: nova-core: add NVKV typed encoding
2026-08-17 12:56 [PATCH 0/6] gpu: nova-core: add NVKV codec Eliot Courtney
` (2 preceding siblings ...)
2026-08-17 12:56 ` [PATCH 3/6] gpu: nova-core: add NVKV decoder Eliot Courtney
@ 2026-08-17 12:56 ` Eliot Courtney
2026-08-17 12:56 ` [PATCH 5/6] gpu: nova-core: add NVKV typed decoding Eliot Courtney
2026-08-17 12:56 ` [PATCH 6/6] gpu: nova-core: add NVKV GSP_INIT schemas Eliot Courtney
5 siblings, 0 replies; 11+ messages in thread
From: Eliot Courtney @ 2026-08-17 12:56 UTC (permalink / raw)
To: Danilo Krummrich, Lorenzo Stoakes, Vlastimil Babka,
Liam R. Howlett, Uladzislau Rezki, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, rust-for-linux,
linux-kernel, nova-gpu, dri-devel, Eliot Courtney
For struct-like GMCAPI messages encoding field by field manually is
noisy. Add some type machinery and a macro to automate encoding of
struct-like messages. The `Encodeable` trait can be implemented by any
type to say that it can be encoded into an NVKV `Encoder`. Add a simple
`nvkv_encode!` macro that works on structs and encodes each field in
order. Provide some base types, such as `Key` which statically
associates a NVKV key with some value, to avoid having to make a lot of
newtypes and implement `Encodeable` on them.
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/gsp/nvkv.rs | 49 +++++++++
drivers/gpu/nova-core/gsp/nvkv/encode.rs | 178 +++++++++++++++++++++++++++++++
2 files changed, 227 insertions(+)
diff --git a/drivers/gpu/nova-core/gsp/nvkv.rs b/drivers/gpu/nova-core/gsp/nvkv.rs
index 64d8d0118452..bf6500d54b21 100644
--- a/drivers/gpu/nova-core/gsp/nvkv.rs
+++ b/drivers/gpu/nova-core/gsp/nvkv.rs
@@ -10,6 +10,13 @@
//! naturally maps to storing a &str with the GPU name.
#![expect(unused_imports)]
+#![cfg_attr(not(CONFIG_KUNIT), expect(unused_macros))]
+
+use core::marker::PhantomData;
+use core::ops::{
+ Deref,
+ DerefMut, //
+};
use kernel::{
bitfield,
@@ -29,6 +36,48 @@
/// The index of an NVKV value.
pub(crate) type Index = Bounded<u64, 12>;
+/// A static association between an NVKV key `KEY_ID` and the storage of its value.
+///
+/// Use with the encoder or decoder macros `nvkv_encode!` and `nvkv_decode!` to let them know how to
+/// map the value `Key<T, KEY_ID, As>` to/from encoded data. For brevity, `As` inserts an additional
+/// conversion (`From`) to avoid having to implement [`Encodable`] for many types. For example,
+/// enums that are easily convertible to a u32 can have `As = u32` and rely on the existing encoding
+/// for u32.
+#[repr(transparent)]
+pub(crate) struct Key<T, const KEY_ID: KeyId, As = T>(pub(crate) T, PhantomData<As>);
+
+impl<T, const KEY_ID: KeyId, As> From<T> for Key<T, KEY_ID, As> {
+ fn from(value: T) -> Self {
+ Self(value, PhantomData)
+ }
+}
+
+impl<'a, T, const KEY_ID: KeyId, As, const N: usize> From<&'a [T; N]> for Key<&'a [T], KEY_ID, As> {
+ fn from(value: &'a [T; N]) -> Self {
+ Self(&value[..], PhantomData)
+ }
+}
+
+impl<T, const KEY_ID: KeyId, As> Deref for Key<T, KEY_ID, As> {
+ type Target = T;
+
+ fn deref(&self) -> &Self::Target {
+ &self.0
+ }
+}
+
+impl<T, const KEY_ID: KeyId, As> DerefMut for Key<T, KEY_ID, As> {
+ fn deref_mut(&mut self) -> &mut Self::Target {
+ &mut self.0
+ }
+}
+
+impl<T: Default, const KEY_ID: KeyId, As> Default for Key<T, KEY_ID, As> {
+ fn default() -> Self {
+ Self(T::default(), PhantomData)
+ }
+}
+
bitfield! {
/// The op word that starts each NVKV operation.
struct Op(u64) {
diff --git a/drivers/gpu/nova-core/gsp/nvkv/encode.rs b/drivers/gpu/nova-core/gsp/nvkv/encode.rs
index 6da81a371651..31ea5788e772 100644
--- a/drivers/gpu/nova-core/gsp/nvkv/encode.rs
+++ b/drivers/gpu/nova-core/gsp/nvkv/encode.rs
@@ -7,11 +7,153 @@
use super::{
Index,
+ Key,
KeyId,
Op,
Opcode, //
};
+/// A type that can encode itself into an [`Encoder`].
+pub(crate) trait Encodable {
+ /// Encodes `self` into `encoder`.
+ fn encode(&self, encoder: &mut Encoder) -> Result;
+}
+
+/// Defines a struct together with its [`Encodable`] implementation.
+///
+/// The implementation encodes each field in declaration order. Each field type must implement
+/// [`Encodable`], which is done already for types like `Key<T, KEY_ID>`.
+///
+/// # Examples
+///
+/// ```
+/// nvkv_encode! {
+/// struct Request {
+/// id: Key<u32, 0x0001>,
+/// name: Key<&'static [u8], 0x0002>,
+/// }
+/// }
+/// ```
+macro_rules! nvkv_encode {
+ (
+ $(#[$attr:meta])*
+ $vis:vis struct $name:ident {
+ $(
+ $(#[$field_attr:meta])*
+ $field_vis:vis $field:ident : $ty:ty
+ ),* $(,)?
+ }
+ ) => {
+ $(#[$attr])*
+ $vis struct $name {
+ $(
+ $(#[$field_attr])*
+ $field_vis $field: $ty,
+ )*
+ }
+
+ impl $crate::gsp::nvkv::Encodable for $name {
+ #[inline(always)]
+ fn encode(&self, encoder: &mut $crate::gsp::nvkv::Encoder) -> ::kernel::error::Result {
+ $( $crate::gsp::nvkv::Encodable::encode(&self.$field, encoder)?; )*
+ Ok(())
+ }
+ }
+ };
+}
+pub(crate) use nvkv_encode;
+
+/// A value with a specific index that encodes under the NVKV key `KEY_ID`.
+struct IndexedKey<T, const KEY_ID: KeyId> {
+ index: Index,
+ value: T,
+}
+
+impl<T, const KEY_ID: KeyId> IndexedKey<T, KEY_ID> {
+ /// Creates a key with the given index and value.
+ pub(crate) fn new(index: Index, value: T) -> Self {
+ Self { index, value }
+ }
+}
+
+impl<const KEY_ID: KeyId> Encodable for IndexedKey<u32, KEY_ID> {
+ #[inline(always)]
+ fn encode(&self, encoder: &mut Encoder) -> Result {
+ encoder.encode_u32(KEY_ID, self.index, self.value)
+ }
+}
+
+impl<const KEY_ID: KeyId> Encodable for IndexedKey<u64, KEY_ID> {
+ #[inline(always)]
+ fn encode(&self, encoder: &mut Encoder) -> Result {
+ encoder.encode_u64(KEY_ID, self.index, self.value)
+ }
+}
+
+impl<const KEY_ID: KeyId> Encodable for IndexedKey<&[u8], KEY_ID> {
+ #[inline(always)]
+ fn encode(&self, encoder: &mut Encoder) -> Result {
+ encoder.encode_array8(KEY_ID, self.index, self.value)
+ }
+}
+
+impl<const KEY_ID: KeyId> Encodable for IndexedKey<&[u32], KEY_ID> {
+ #[inline(always)]
+ fn encode(&self, encoder: &mut Encoder) -> Result {
+ encoder.encode_array32(KEY_ID, self.index, self.value)
+ }
+}
+
+impl<const KEY_ID: KeyId> Encodable for IndexedKey<&[u64], KEY_ID> {
+ #[inline(always)]
+ fn encode(&self, encoder: &mut Encoder) -> Result {
+ encoder.encode_array64(KEY_ID, self.index, self.value)
+ }
+}
+
+impl<const N: usize, const KEY_ID: KeyId> Encodable for IndexedKey<[u8; N], KEY_ID> {
+ #[inline(always)]
+ fn encode(&self, encoder: &mut Encoder) -> Result {
+ encoder.encode_array8(KEY_ID, self.index, &self.value)
+ }
+}
+
+impl<const N: usize, const KEY_ID: KeyId> Encodable for IndexedKey<[u32; N], KEY_ID> {
+ #[inline(always)]
+ fn encode(&self, encoder: &mut Encoder) -> Result {
+ encoder.encode_array32(KEY_ID, self.index, &self.value)
+ }
+}
+
+impl<const N: usize, const KEY_ID: KeyId> Encodable for IndexedKey<[u64; N], KEY_ID> {
+ #[inline(always)]
+ fn encode(&self, encoder: &mut Encoder) -> Result {
+ encoder.encode_array64(KEY_ID, self.index, &self.value)
+ }
+}
+
+impl<T, const KEY_ID: KeyId, As> Encodable for Key<T, KEY_ID, As>
+where
+ IndexedKey<As, KEY_ID>: Encodable,
+ As: From<T>,
+ T: Copy,
+{
+ #[inline(always)]
+ fn encode(&self, encoder: &mut Encoder) -> Result {
+ IndexedKey::new(Index::new::<0>(), As::from(self.0)).encode(encoder)
+ }
+}
+
+impl<T: Encodable> Encodable for Option<T> {
+ #[inline(always)]
+ fn encode(&self, encoder: &mut Encoder) -> Result {
+ if let Some(value) = self {
+ value.encode(encoder)?;
+ }
+ Ok(())
+ }
+}
+
/// An encoder for an NVKV stream.
pub(crate) struct Encoder {
backing: KVVec<u64>,
@@ -242,4 +384,40 @@ fn encode_all_value_kinds() -> Result {
Ok(())
}
+
+ // Tests that encoding via the `nvkv_encode!` macro works correctly.
+ #[test]
+ fn encode_typed_struct() -> Result {
+ const U32_KEY: KeyId = 0x0001;
+ const U64_KEY: KeyId = 0x0002;
+ const NAME_KEY: KeyId = 0x0003;
+ const FIXED_KEY: KeyId = 0x0004;
+ const OPT_KEY: KeyId = 0x0005;
+
+ nvkv_encode! {
+ struct TypedRequest {
+ a: Key<u32, { U32_KEY }>,
+ b: Key<u64, { U64_KEY }>,
+ name: Key<&'static [u8], { NAME_KEY }>,
+ fixed: Key<[u8; 4], { FIXED_KEY }>,
+ opt: Option<Key<u32, { OPT_KEY }>>,
+ }
+ }
+
+ let request = TypedRequest {
+ a: 0x89ab_cdef.into(),
+ b: 0x0123_4567_89ab_cdef.into(),
+ name: b"name\0".into(),
+ fixed: [1u8, 2, 3, 4].into(),
+ opt: None,
+ };
+
+ let mut encoder = Encoder::new();
+ request.encode(&mut encoder)?;
+ let encoded = encoder.finish();
+
+ assert_eq!(encoded.len(), 7);
+
+ Ok(())
+ }
}
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 5/6] gpu: nova-core: add NVKV typed decoding
2026-08-17 12:56 [PATCH 0/6] gpu: nova-core: add NVKV codec Eliot Courtney
` (3 preceding siblings ...)
2026-08-17 12:56 ` [PATCH 4/6] gpu: nova-core: add NVKV typed encoding Eliot Courtney
@ 2026-08-17 12:56 ` Eliot Courtney
2026-08-17 12:56 ` [PATCH 6/6] gpu: nova-core: add NVKV GSP_INIT schemas Eliot Courtney
5 siblings, 0 replies; 11+ messages in thread
From: Eliot Courtney @ 2026-08-17 12:56 UTC (permalink / raw)
To: Danilo Krummrich, Lorenzo Stoakes, Vlastimil Babka,
Liam R. Howlett, Uladzislau Rezki, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, rust-for-linux,
linux-kernel, nova-gpu, dri-devel, Eliot Courtney
Similar to the typed encoding layer, add some decoding type machinery.
Add a simple macro `nvkv_decode!` which implements `Schema` for a struct
by composing visit calls to each member. Add some common `Schema` kinds,
such as `Array` which collects an array value into a fixed maximum size
array, and `Required` which fails a decode if the value is not sent.
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/gsp/nvkv.rs | 60 ++++-
drivers/gpu/nova-core/gsp/nvkv/decode.rs | 393 +++++++++++++++++++++++++++++++
2 files changed, 452 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/nova-core/gsp/nvkv.rs b/drivers/gpu/nova-core/gsp/nvkv.rs
index bf6500d54b21..a0068847bb80 100644
--- a/drivers/gpu/nova-core/gsp/nvkv.rs
+++ b/drivers/gpu/nova-core/gsp/nvkv.rs
@@ -9,7 +9,7 @@
//! function calls will map to some struct - for example, f(GPU_NAME_STRING_KEY, 0, b"some gpu")
//! naturally maps to storing a &str with the GPU name.
-#![expect(unused_imports)]
+#![cfg_attr(not(CONFIG_KUNIT), expect(unused_imports))]
#![cfg_attr(not(CONFIG_KUNIT), expect(unused_macros))]
use core::marker::PhantomData;
@@ -78,6 +78,64 @@ fn default() -> Self {
}
}
+/// A fixed capacity vector that holds at most `N` elements.
+#[derive(Debug, Copy, Clone, PartialEq, Eq, Zeroable)]
+pub(crate) struct ArrayVec<T, const N: usize> {
+ data: [T; N],
+ len: usize,
+}
+
+impl<T, const N: usize> ArrayVec<T, N> {
+ /// Replaces the contents with a copy of `slice`.
+ ///
+ /// Fails with `EMSGSIZE` if `slice` is longer than the capacity.
+ pub(crate) fn set_from_slice(&mut self, slice: &[T]) -> Result
+ where
+ T: Copy,
+ {
+ let Some(dst) = self.data.get_mut(..slice.len()) else {
+ return Err(EMSGSIZE);
+ };
+
+ dst.copy_from_slice(slice);
+ self.len = slice.len();
+
+ Ok(())
+ }
+
+ /// Returns the initialized elements as a slice.
+ #[inline]
+ pub(crate) fn as_slice(&self) -> &[T] {
+ // PANIC: `len` is bounded by `N`.
+ &self.data[..self.len]
+ }
+}
+
+impl<T: Default + Copy, const N: usize> Default for ArrayVec<T, N> {
+ fn default() -> Self {
+ Self {
+ data: [T::default(); N],
+ len: 0,
+ }
+ }
+}
+
+impl<T, const N: usize> Deref for ArrayVec<T, N> {
+ type Target = [T];
+
+ #[inline]
+ fn deref(&self) -> &Self::Target {
+ self.as_slice()
+ }
+}
+
+/// A schema field for an array value under the NVKV key `KEY_ID`.
+#[derive(Default)]
+#[repr(transparent)]
+pub(crate) struct Array<T: Default + Copy, const N: usize, const KEY_ID: KeyId>(
+ pub(crate) ArrayVec<T, N>,
+);
+
bitfield! {
/// The op word that starts each NVKV operation.
struct Op(u64) {
diff --git a/drivers/gpu/nova-core/gsp/nvkv/decode.rs b/drivers/gpu/nova-core/gsp/nvkv/decode.rs
index ee8b6ab5a3a4..9112dcf1aaca 100644
--- a/drivers/gpu/nova-core/gsp/nvkv/decode.rs
+++ b/drivers/gpu/nova-core/gsp/nvkv/decode.rs
@@ -3,16 +3,311 @@
#![cfg_attr(not(CONFIG_KUNIT), expect(dead_code))]
+use core::marker::PhantomData;
+
use kernel::prelude::*;
use crate::gsp::nvkv::{
+ Array,
+ ArrayVec,
Index,
+ Key,
KeyId,
Op,
Opcode, //
};
use crate::num;
+/// Defines a schema struct together with its [`Schema`] implementation that decodes into `$target`.
+///
+/// Each member of the struct should implement `Schema`. For every (key, index, value) triple
+/// decoded from the NVKV stream, the generated parent `Schema` implementation will call each member
+/// in declaration order with that triple. If a member consumes that triple, it will stop there.
+/// Otherwise it will keep going until all members are tried.
+///
+/// The schema struct holds the state required by the schema implementation to do the decode. It's
+/// recommended to use one of the existing Schema kinds (`Required`, `Accumulated`, `Key`, `Array`,
+/// `Indexed`) for each member.
+///
+/// # Examples
+///
+/// ```
+/// nvkv_decode! {
+/// #[derive(Default)]
+/// struct RequestSchema => Request {
+/// id: Required<u32, 0x0001>,
+/// name: Array<u8, 64, 0x0002>,
+/// }
+/// }
+/// ```
+macro_rules! nvkv_decode {
+ (
+ $(#[$attr:meta])*
+ $vis:vis struct $name:ident => $target:ident {
+ $(
+ $(#[$field_attr:meta])*
+ $field_vis:vis $field:ident : $ty:ty
+ ),* $(,)?
+ }
+ ) => {
+ $(#[$attr])*
+ $vis struct $name {
+ $(
+ $(#[$field_attr])*
+ $field_vis $field: $ty,
+ )*
+ }
+
+ impl $crate::gsp::nvkv::Schema for $name {
+ type Target = $target;
+
+ fn visit(
+ &mut self,
+ key: $crate::gsp::nvkv::KeyId,
+ index: $crate::gsp::nvkv::Index,
+ value: $crate::gsp::nvkv::DecoderValue<'_>,
+ ) -> ::kernel::error::Result<bool> {
+ Ok(false
+ $( || $crate::gsp::nvkv::Schema::visit(&mut self.$field, key, index, value)? )*)
+ }
+
+ #[inline(always)]
+ fn finish(self) -> impl ::kernel::prelude::Init<Self::Target, ::kernel::error::Error> {
+ ::kernel::try_init!(Self::Target {
+ $( $field <- $crate::gsp::nvkv::Schema::finish(self.$field), )*
+ }? ::kernel::error::Error)
+ }
+ }
+ };
+}
+pub(crate) use nvkv_decode;
+
+impl<T: for<'a> TryFrom<DecoderValue<'a>, Error = Error> + Default, const KEY_ID: KeyId> Schema
+ for Key<T, KEY_ID>
+{
+ type Target = T;
+
+ #[inline(always)]
+ fn visit<'a>(&mut self, key: KeyId, index: Index, value: DecoderValue<'a>) -> Result<bool> {
+ if key != KEY_ID {
+ Ok(false)
+ } else if index != Index::new::<0>() {
+ // Single values being set must be at index 0.
+ Err(EINVAL)
+ } else {
+ // Overwrite and take the latest value here.
+ self.0 = value.try_into()?;
+ Ok(true)
+ }
+ }
+
+ #[inline(always)]
+ fn finish(self) -> impl Init<Self::Target, Error> {
+ Ok(self.0)
+ }
+}
+
+impl<T: for<'a> TryFrom<DecoderValue<'a>, Error = Error>, const KEY_ID: KeyId> Schema
+ for Key<Option<T>, KEY_ID>
+{
+ type Target = Option<T>;
+
+ #[inline(always)]
+ fn visit<'a>(&mut self, key: KeyId, index: Index, value: DecoderValue<'a>) -> Result<bool> {
+ if key != KEY_ID {
+ Ok(false)
+ } else if index != Index::new::<0>() {
+ // Single values being set must be at index 0.
+ Err(EINVAL)
+ } else {
+ // Overwrite and take the latest value here.
+ self.0 = Some(value.try_into()?);
+ Ok(true)
+ }
+ }
+
+ #[inline(always)]
+ fn finish(self) -> impl Init<Self::Target, Error> {
+ Ok(self.0)
+ }
+}
+
+impl<T: Default + Copy, const N: usize, const KEY_ID: KeyId> Schema for Array<T, N, KEY_ID>
+where
+ for<'a> &'a [T]: TryFrom<DecoderValue<'a>, Error = Error>,
+{
+ type Target = ArrayVec<T, N>;
+
+ fn visit<'a>(&mut self, key: KeyId, index: Index, value: DecoderValue<'a>) -> Result<bool> {
+ if key != KEY_ID {
+ return Ok(false);
+ }
+ // Require to be at index 0
+ if index != Index::new::<0>() {
+ return Err(EINVAL);
+ }
+ // Reject oversized and take the latest value.
+ self.0.set_from_slice(value.try_into()?)?;
+ Ok(true)
+ }
+
+ #[inline(always)]
+ fn finish(self) -> impl Init<Self::Target, Error> {
+ Ok(self.0)
+ }
+}
+
+/// A schema field for a key that must be present.
+///
+/// `finish` fails with `EINVAL` if no value arrived for the key.
+#[repr(transparent)]
+pub(crate) struct Required<T, const KEY_ID: KeyId>(Key<Option<T>, KEY_ID>);
+
+impl<T: for<'a> TryFrom<DecoderValue<'a>, Error = Error>, const KEY_ID: KeyId> Schema
+ for Required<T, KEY_ID>
+{
+ type Target = T;
+
+ #[inline(always)]
+ fn visit<'a>(&mut self, key: KeyId, index: Index, value: DecoderValue<'a>) -> Result<bool> {
+ self.0.visit(key, index, value)
+ }
+
+ #[inline(always)]
+ fn finish(self) -> impl Init<Self::Target, Error> {
+ (self.0).0.ok_or(EINVAL)
+ }
+}
+
+impl<T, const KEY_ID: KeyId> Default for Required<T, KEY_ID> {
+ fn default() -> Self {
+ Self(None.into())
+ }
+}
+
+/// Expects objects specified sequentially with index starting from zero.
+pub(crate) struct Accumulated<S: Schema> {
+ current_index: Index,
+ current: S,
+ current_started: bool,
+ next: S,
+ accumulated: KVVec<S::Target>,
+}
+
+impl<S: Schema + Default> Accumulated<S> {
+ /// Creates an empty accumulator.
+ pub(crate) fn new() -> Self {
+ Self {
+ current_index: Index::new::<0>(),
+ current: S::default(),
+ current_started: false,
+ next: S::default(),
+ accumulated: KVVec::new(),
+ }
+ }
+
+ fn into_vec(mut self) -> Result<KVVec<S::Target>> {
+ if self.current_started {
+ let done = core::mem::take(&mut self.current);
+ self.accumulated.push_init(done.finish(), GFP_KERNEL)?;
+ }
+ Ok(self.accumulated)
+ }
+}
+
+impl<S: Schema + Default> Schema for Accumulated<S> {
+ type Target = KVVec<S::Target>;
+
+ fn visit<'a>(&mut self, key: KeyId, index: Index, value: DecoderValue<'a>) -> Result<bool> {
+ if index != self.current_index {
+ if !self.next.visit(key, Index::new::<0>(), value)? {
+ // Unrelated key to us.
+ return Ok(false);
+ }
+
+ // Require that objects at index k have all their keys sent before the k + 1 th object
+ // can be completed. Require that objects are sent contiguously in order from index 0.
+ if !self.current_started || index != self.current_index + 1 {
+ return Err(EINVAL);
+ }
+
+ // The current value must be finished. Finish it and start working on `next`.
+ let done = core::mem::replace(&mut self.current, core::mem::take(&mut self.next));
+ self.accumulated.push_init(done.finish(), GFP_KERNEL)?;
+ self.current_started = true;
+ self.current_index = index;
+ Ok(true)
+ } else {
+ let consumed = self.current.visit(key, Index::new::<0>(), value)?;
+ self.current_started |= consumed;
+ Ok(consumed)
+ }
+ }
+
+ #[inline(always)]
+ fn finish(self) -> impl Init<Self::Target, Error> {
+ self.into_vec()
+ }
+}
+
+impl<S: Schema + Default> Default for Accumulated<S> {
+ fn default() -> Self {
+ Self::new()
+ }
+}
+
+/// A schema field that scatters indexed values into an array of `N` slots.
+#[repr(transparent)]
+pub(crate) struct Indexed<T, const N: usize, const KEY_ID: KeyId, As = T>([T; N], PhantomData<As>);
+
+/// Copies `elems`, converted to `T`, into `slots` at `start`.
+///
+/// Fails with `EINVAL` if the window does not fit in `slots`.
+fn scatter_window<T: From<As>, As: Copy>(slots: &mut [T], start: usize, elems: &[As]) -> Result {
+ let end = start.checked_add(elems.len()).ok_or(EINVAL)?;
+ // Reject indices outside of the declared array size.
+ let dst = slots.get_mut(start..end).ok_or(EINVAL)?;
+ for (d, &e) in dst.iter_mut().zip(elems) {
+ *d = T::from(e);
+ }
+ Ok(())
+}
+
+impl<T, const N: usize, const KEY_ID: KeyId, As> Schema for Indexed<T, N, KEY_ID, As>
+where
+ T: From<As>,
+ As: Copy + for<'a> TryFrom<DecoderValue<'a>, Error = Error>,
+ for<'a> &'a [As]: TryFrom<DecoderValue<'a>, Error = Error>,
+{
+ type Target = [T; N];
+
+ fn visit<'a>(&mut self, key: KeyId, index: Index, value: DecoderValue<'a>) -> Result<bool> {
+ if key != KEY_ID {
+ return Ok(false);
+ }
+ let start = index.cast::<usize>().get();
+ // Accept both scalar vs scattered array setting for flexibility.
+ match <&[As]>::try_from(value) {
+ Ok(elems) => scatter_window(&mut self.0, start, elems)?,
+ Err(_) => scatter_window(&mut self.0, start, &[As::try_from(value)?])?,
+ }
+ Ok(true)
+ }
+
+ #[inline(always)]
+ fn finish(self) -> impl Init<Self::Target, Error> {
+ Ok(self.0)
+ }
+}
+
+impl<T: Default + Copy, const N: usize, const KEY_ID: KeyId, As> Default
+ for Indexed<T, N, KEY_ID, As>
+{
+ fn default() -> Self {
+ Self([T::default(); N], PhantomData)
+ }
+}
+
/// A decoded NVKV value.
#[derive(Copy, Clone)]
pub(crate) enum DecoderValue<'a> {
@@ -255,4 +550,102 @@ fn finish(self) -> impl Init<Self::Target, Error> {
Ok(())
}
+
+ // Tests that decoding via the `nvkv_decode!` macro works correctly.
+ #[test]
+ fn decode_typed_struct() -> Result {
+ const SCALAR32_KEY: KeyId = 0x1234;
+ const SCALAR64_KEY: KeyId = 0x1235;
+ const ARRAY8_KEY: KeyId = 0x1236;
+ const ARRAY32_KEY: KeyId = 0x1237;
+ const ARRAY64_KEY: KeyId = 0x1238;
+ const OPT_PRESENT_KEY: KeyId = 0x1239;
+ const OPT_ABSENT_KEY: KeyId = 0x123a;
+ const X_KEY: KeyId = 0x0100;
+ const Y_KEY: KeyId = 0x0101;
+ const SLOT_KEY: KeyId = 0x0200;
+
+ const SCALAR32_VALUE: u32 = 0x89ab_cdef;
+ const SCALAR64_VALUE: u64 = 0x0123_4567_89ab_cdef;
+ const ARRAY8_VALUE: &[u8] = &[0x12, 0x34, 0x56];
+ const ARRAY32_VALUE: &[u32] = &[0x0123_4567, 0x89ab_cdef];
+ const ARRAY64_VALUE: &[u64] = &[0x0123_4567_89ab_cdef, 0xfedc_ba98_7654_3210];
+ const OPT_PRESENT_VALUE: u32 = 0x55;
+
+ nvkv_decode! {
+ #[derive(Default)]
+ struct PairSchema => Pair {
+ x: Required<u32, { X_KEY }>,
+ y: Required<u32, { Y_KEY }>,
+ }
+ }
+
+ struct Pair {
+ x: u32,
+ y: u32,
+ }
+
+ nvkv_decode! {
+ #[derive(Default)]
+ struct TestSchema => TestDecodeable {
+ scalar32: Required<u32, { SCALAR32_KEY }>,
+ scalar64: Required<u64, { SCALAR64_KEY }>,
+ array8: Array<u8, 64, { ARRAY8_KEY }>,
+ array32: Array<u32, 64, { ARRAY32_KEY }>,
+ array64: Array<u64, 64, { ARRAY64_KEY }>,
+ opt_present: Key<Option<u32>, { OPT_PRESENT_KEY }>,
+ opt_absent: Key<Option<u32>, { OPT_ABSENT_KEY }>,
+ pairs: Accumulated<PairSchema>,
+ slots: Indexed<u32, 4, { SLOT_KEY }>,
+ }
+ }
+
+ struct TestDecodeable {
+ scalar32: u32,
+ scalar64: u64,
+ array8: ArrayVec<u8, 64>,
+ array32: ArrayVec<u32, 64>,
+ array64: ArrayVec<u64, 64>,
+ opt_present: Option<u32>,
+ opt_absent: Option<u32>,
+ pairs: KVVec<Pair>,
+ slots: [u32; 4],
+ }
+
+ let index0 = Index::new::<0>();
+ let index1 = Index::new::<1>();
+ let mut encoder = Encoder::new();
+ encoder.encode_u32(SCALAR32_KEY, index0, SCALAR32_VALUE)?;
+ encoder.encode_u64(SCALAR64_KEY, index0, SCALAR64_VALUE)?;
+ encoder.encode_array8(ARRAY8_KEY, index0, ARRAY8_VALUE)?;
+ encoder.encode_array32(ARRAY32_KEY, index0, ARRAY32_VALUE)?;
+ encoder.encode_array64(ARRAY64_KEY, index0, ARRAY64_VALUE)?;
+ encoder.encode_u32(OPT_PRESENT_KEY, index0, OPT_PRESENT_VALUE)?;
+ encoder.encode_u32(X_KEY, index0, 1)?;
+ encoder.encode_u32(Y_KEY, index0, 2)?;
+ encoder.encode_u32(SLOT_KEY, index1, 20)?;
+ encoder.encode_u32(X_KEY, index1, 3)?;
+ encoder.encode_u32(Y_KEY, index1, 4)?;
+ encoder.encode_u32(SLOT_KEY, index0, 10)?;
+ let serialized = encoder.finish();
+
+ let decoder = Decoder::new(&serialized, UnknownKeyPolicy::Error);
+ let decoded = KBox::try_init(decoder.decode(TestSchema::default())?, GFP_KERNEL)?;
+
+ assert_eq!(decoded.scalar32, SCALAR32_VALUE);
+ assert_eq!(decoded.scalar64, SCALAR64_VALUE);
+ assert_eq!(*decoded.array8, *ARRAY8_VALUE);
+ assert_eq!(*decoded.array32, *ARRAY32_VALUE);
+ assert_eq!(*decoded.array64, *ARRAY64_VALUE);
+ assert_eq!(decoded.opt_present, Some(OPT_PRESENT_VALUE));
+ assert_eq!(decoded.opt_absent, None);
+ assert_eq!(decoded.pairs.len(), 2);
+ assert_eq!(decoded.pairs[0].x, 1);
+ assert_eq!(decoded.pairs[0].y, 2);
+ assert_eq!(decoded.pairs[1].x, 3);
+ assert_eq!(decoded.pairs[1].y, 4);
+ assert_eq!(decoded.slots, [10, 20, 0, 0]);
+
+ Ok(())
+ }
}
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH 6/6] gpu: nova-core: add NVKV GSP_INIT schemas
2026-08-17 12:56 [PATCH 0/6] gpu: nova-core: add NVKV codec Eliot Courtney
` (4 preceding siblings ...)
2026-08-17 12:56 ` [PATCH 5/6] gpu: nova-core: add NVKV typed decoding Eliot Courtney
@ 2026-08-17 12:56 ` Eliot Courtney
2026-08-17 13:06 ` sashiko-bot
5 siblings, 1 reply; 11+ messages in thread
From: Eliot Courtney @ 2026-08-17 12:56 UTC (permalink / raw)
To: Danilo Krummrich, Lorenzo Stoakes, Vlastimil Babka,
Liam R. Howlett, Uladzislau Rezki, Miguel Ojeda, Boqun Feng,
Gary Guo, Björn Roy Baron, Benno Lossin, Andreas Hindborg,
Alice Ryhl, Trevor Gross, Daniel Almeida, Tamir Duberstein,
Alexandre Courbot, Onur Özkan, David Airlie, Simona Vetter
Cc: John Hubbard, Alistair Popple, Timur Tabi, rust-for-linux,
linux-kernel, nova-gpu, dri-devel, Eliot Courtney
Add the first user of NVKV encode/decode which is the request and
response for GSP init. For now this is exercised via unit tests. Later
patches will support GMCAPI in `Cmdq` and use these messages.
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
drivers/gpu/nova-core/gsp/fw/commands.rs | 349 +++++++++++++++++++++++++++++++
drivers/gpu/nova-core/gsp/nvkv.rs | 3 -
2 files changed, 349 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/nova-core/gsp/fw/commands.rs b/drivers/gpu/nova-core/gsp/fw/commands.rs
index 6dc31d1bf5ae..4de44c2dc6aa 100644
--- a/drivers/gpu/nova-core/gsp/fw/commands.rs
+++ b/drivers/gpu/nova-core/gsp/fw/commands.rs
@@ -4,6 +4,7 @@
use core::ops::Range;
use kernel::{
+ bitfield,
device,
pci,
prelude::*,
@@ -19,6 +20,20 @@
num::IntoSafeCast, //
};
+use crate::gsp::nvkv::{
+ nvkv_decode,
+ nvkv_encode,
+ Accumulated,
+ Array,
+ ArrayVec,
+ DecoderValue,
+ Encodable,
+ Encoder,
+ Key,
+ KeyId,
+ Required, //
+};
+
use super::bindings;
/// Payload of the `GspSetSystemInfo` command.
@@ -217,3 +232,337 @@ unsafe impl AsBytes for UnloadingGuestDriver {}
// SAFETY: This struct only contains integer types for which all bit patterns
// are valid.
unsafe impl FromBytes for UnloadingGuestDriver {}
+
+/// The host CPU architecture.
+#[derive(Clone, Copy)]
+pub(crate) enum HostArch {
+ None = 0,
+ X86_64 = 1,
+ Ppc64le = 2,
+ Arm = 3,
+ Aarch64 = 4,
+ Riscv64 = 5,
+}
+
+// TODO[FPRI]: This is a temporary solution to be replaced with the corresponding derive macros once
+// they land.
+impl TryFrom<u32> for HostArch {
+ type Error = Error;
+
+ fn try_from(value: u32) -> Result<Self> {
+ match value {
+ 0 => Ok(Self::None),
+ 1 => Ok(Self::X86_64),
+ 2 => Ok(Self::Ppc64le),
+ 3 => Ok(Self::Arm),
+ 4 => Ok(Self::Aarch64),
+ 5 => Ok(Self::Riscv64),
+ _ => Err(EINVAL),
+ }
+ }
+}
+
+impl From<HostArch> for u32 {
+ fn from(value: HostArch) -> Self {
+ value as u32
+ }
+}
+
+nvkv_encode! {
+ /// A GSP registry entry.
+ struct RegKey {
+ key_name: Key<&'static [u8], { Self::REGKEY_NAME_KEY }>,
+ key_value: Key<u32, { Self::REGKEY_VALUE_U32_KEY }>,
+ }
+}
+
+impl RegKey {
+ // Define the Key IDs read/written by GSP.
+ const REGKEY_NAME_KEY: KeyId = 0x3070;
+ const REGKEY_VALUE_U32_KEY: KeyId = 0x3071;
+}
+
+impl Encodable for KVVec<RegKey> {
+ fn encode(&self, encoder: &mut Encoder) -> Result {
+ for regkey in self {
+ regkey.encode(encoder)?;
+ }
+ Ok(())
+ }
+}
+
+nvkv_encode! {
+ /// SR-IOV virtual function information.
+ struct VfInfo {
+ total_vfs: Key<u32, { Self::VF_TOTAL_VFS_KEY }>,
+ first_vf_offset: Key<u32, { Self::VF_FIRST_VF_OFFSET_KEY }>,
+ flags: Key<u64, { Self::VF_FLAGS_KEY }>,
+ first_bar0_address: Key<u64, { Self::VF_FIRST_BAR0_ADDRESS_KEY }>,
+ first_bar1_address: Key<u64, { Self::VF_FIRST_BAR1_ADDRESS_KEY }>,
+ first_bar2_address: Key<u64, { Self::VF_FIRST_BAR2_ADDRESS_KEY }>,
+ }
+}
+
+impl VfInfo {
+ // Define the Key IDs read/written by GSP.
+ const VF_TOTAL_VFS_KEY: KeyId = 0x0080;
+ const VF_FIRST_VF_OFFSET_KEY: KeyId = 0x0081;
+ const VF_FLAGS_KEY: KeyId = 0x1003;
+ const VF_FIRST_BAR0_ADDRESS_KEY: KeyId = 0x1050;
+ const VF_FIRST_BAR1_ADDRESS_KEY: KeyId = 0x1051;
+ const VF_FIRST_BAR2_ADDRESS_KEY: KeyId = 0x1052;
+}
+
+nvkv_encode! {
+ /// Payload of the `GSP_INIT` command.
+ // TODO: expect() doesn't work here due to Self:: reference, fixed in 1.97.0
+ // https://github.com/rust-lang/rust/pull/154377
+ #[cfg_attr(not(CONFIG_KUNIT), allow(dead_code))]
+ struct GspInitRequest {
+ pci_device_id: Key<u32, { Self::PCI_DEVICE_ID_KEY }>,
+ pci_sub_device_id: Key<u32, { Self::PCI_SUBDEVICE_ID_KEY }>,
+ pci_revision_id: Key<u32, { Self::PCI_REVISION_ID_KEY }>,
+ pci_config_mirror_base: Key<u32, { Self::PCI_CONFIG_MIRROR_BASE_KEY }>,
+ pci_config_mirror_size: Key<u32, { Self::PCI_CONFIG_MIRROR_SIZE_KEY }>,
+ host_arch: Key<HostArch, { Self::HOST_ARCH_KEY }, u32>,
+ bus_device_func: Key<u64, { Self::NV_DOMAIN_BUS_DEVICE_FUNC_KEY }>,
+ regkeys: KVVec<RegKey>,
+ vf_info: Option<VfInfo>,
+ }
+}
+
+impl GspInitRequest {
+ // Define the Key IDs read/written by GSP.
+ const PCI_DEVICE_ID_KEY: KeyId = 0x0001;
+ const PCI_SUBDEVICE_ID_KEY: KeyId = 0x0002;
+ const PCI_REVISION_ID_KEY: KeyId = 0x0003;
+ const PCI_CONFIG_MIRROR_BASE_KEY: KeyId = 0x0010;
+ const PCI_CONFIG_MIRROR_SIZE_KEY: KeyId = 0x0011;
+ const HOST_ARCH_KEY: KeyId = 0x0070;
+ const NV_DOMAIN_BUS_DEVICE_FUNC_KEY: KeyId = 0x1020;
+}
+
+// Decode:
+
+// Should decode with UnknownKeyPolicy::Ignore.
+nvkv_decode! {
+ /// Schema for the `GSP_INIT` response.
+ // TODO: expect() doesn't work here due to Self:: reference, fixed in 1.97.0
+ // https://github.com/rust-lang/rust/pull/154377
+ #[cfg_attr(not(CONFIG_KUNIT), allow(dead_code))]
+ #[derive(Default)]
+ struct GspInitResponseSchema => GspInitResponse {
+ gpu_name:
+ Array<u8, { GspInitResponse::MAX_GPU_NAME_LEN }, { Self::GPU_NAME_STRING_KEY }>,
+ fb_regions: Accumulated<FbRegionSchema>,
+ bar1_pde_base: Required<u64, { Self::BAR1_PDE_BASE_KEY }>,
+ vmmu_segment_size: Key<u64, { Self::VMMU_SEGMENT_SIZE_KEY }>,
+ }
+}
+
+impl GspInitResponseSchema {
+ // Define the Key IDs read/written by GSP.
+ const GPU_NAME_STRING_KEY: KeyId = 0x2000;
+ const BAR1_PDE_BASE_KEY: KeyId = 0x1020;
+ const VMMU_SEGMENT_SIZE_KEY: KeyId = 0x1050;
+}
+
+/// Payload of the `GSP_INIT` response.
+struct GspInitResponse {
+ gpu_name: ArrayVec<u8, { Self::MAX_GPU_NAME_LEN }>,
+ fb_regions: KVVec<FbRegion>,
+ bar1_pde_base: u64,
+ vmmu_segment_size: u64,
+}
+
+impl GspInitResponse {
+ const MAX_GPU_NAME_LEN: usize = 64;
+}
+
+nvkv_decode! {
+ /// Schema for one FB region of the `GSP_INIT` response.
+ #[derive(Default)]
+ struct FbRegionSchema => FbRegion {
+ base: Required<u64, { Self::BASE_KEY }>,
+ limit: Required<u64, { Self::LIMIT_KEY }>,
+ flags: Required<FbRegionFlags, { Self::FLAGS_KEY }>,
+ tag: Required<u32, { Self::TAG_KEY }>,
+ }
+}
+
+impl FbRegionSchema {
+ // Define the Key IDs read/written by GSP.
+ const BASE_KEY: KeyId = 0x1011;
+ const LIMIT_KEY: KeyId = 0x1012;
+ const FLAGS_KEY: KeyId = 0x0012;
+ const TAG_KEY: KeyId = 0x0013;
+}
+
+bitfield! {
+ /// FB region attribute flags.
+ struct FbRegionFlags(u32) {
+ 0:0 support_compressed => bool;
+ 1:1 support_iso => bool;
+ 2:2 protected => bool;
+ }
+}
+
+impl TryFrom<DecoderValue<'_>> for FbRegionFlags {
+ type Error = Error;
+
+ fn try_from(value: DecoderValue<'_>) -> Result<Self> {
+ if let DecoderValue::Scalar32(v) = value {
+ Ok(v.into())
+ } else {
+ Err(EINVAL)
+ }
+ }
+}
+
+/// One FB memory region.
+struct FbRegion {
+ base: u64,
+ limit: u64,
+ flags: FbRegionFlags,
+ tag: u32,
+}
+
+#[kunit_tests(nova_core_fw_commands)]
+mod tests {
+ use crate::gsp::nvkv::{
+ Decoder,
+ Index,
+ UnknownKeyPolicy, //
+ };
+
+ use super::*;
+
+ // Tests that `GspInitRequest` encodes correctly.
+ #[test]
+ fn gsp_init_request() -> Result {
+ let mut encoder = Encoder::new();
+
+ let mut regkeys = KVVec::new();
+ regkeys.push(
+ RegKey {
+ key_name: b"test_key\0".into(),
+ key_value: 0xdead_beef.into(),
+ },
+ GFP_KERNEL,
+ )?;
+
+ let gsp_init = GspInitRequest {
+ pci_device_id: 45.into(),
+ pci_sub_device_id: 67.into(),
+ pci_revision_id: 3.into(),
+ pci_config_mirror_base: 0x1234_5678.into(),
+ pci_config_mirror_size: 0x1000.into(),
+ host_arch: HostArch::Aarch64.into(),
+ bus_device_func: 0x0001_0203_0405_0607.into(),
+ regkeys,
+ vf_info: Some(VfInfo {
+ total_vfs: 8.into(),
+ first_vf_offset: 1.into(),
+ flags: 0x7.into(),
+ first_bar0_address: 0x1000_0000.into(),
+ first_bar1_address: 0x2000_0000.into(),
+ first_bar2_address: 0x3000_0000.into(),
+ }),
+ };
+
+ gsp_init.encode(&mut encoder)?;
+ let encoded = encoder.finish();
+ assert_eq!(encoded.len(), 22);
+
+ Ok(())
+ }
+
+ // Tests that FB region decoding fails when required keys are missing.
+ #[test]
+ fn decode_fb_region_missing_required_fails() -> Result {
+ let mut encoder = Encoder::new();
+ encoder.encode_u64(FbRegionSchema::BASE_KEY, Index::new::<0>(), 0x1000_0000)?;
+ let data = encoder.finish();
+
+ let decoder = Decoder::new(&data, UnknownKeyPolicy::Ignore);
+ let init = decoder.decode(FbRegionSchema::default())?;
+ assert!(KBox::try_init(init, GFP_KERNEL).is_err());
+
+ Ok(())
+ }
+
+ // Tests that a minimal and a full `GSP_INIT` response decode correctly.
+ #[test]
+ fn gsp_init_response() -> Result {
+ let name = b"test name\0";
+ const BAR1_PDE_BASE: u64 = 0xdead_0000;
+ const FB_REGION0_BASE: u64 = 0x1000_0000;
+ const FB_REGION0_LIMIT: u64 = 0x1fff_ffff;
+ const FB_REGION0_FLAGS: u32 = 0x7;
+ const FB_REGION0_TAG: u32 = 0;
+ const FB_REGION1_BASE: u64 = 0x2000_0000;
+ const FB_REGION1_LIMIT: u64 = 0x2fff_ffff;
+ const FB_REGION1_FLAGS: u32 = 0x3;
+ const FB_REGION1_TAG: u32 = 1;
+ const VMMU_SEGMENT_SIZE: u64 = 0x0200_0000;
+
+ type Resp = GspInitResponseSchema;
+
+ let index0 = Index::new::<0>();
+ let index1 = Index::new::<1>();
+
+ // A minimal response: only the BAR1 PDE base, so the FB region list stays empty.
+ let mut encoder = Encoder::new();
+ encoder.encode_u64(Resp::BAR1_PDE_BASE_KEY, index0, BAR1_PDE_BASE)?;
+ let data = encoder.finish();
+
+ let decoder = Decoder::new(&data, UnknownKeyPolicy::Ignore);
+ let response = KBox::try_init(decoder.decode(Resp::default())?, GFP_KERNEL)?;
+ assert_eq!(response.bar1_pde_base, BAR1_PDE_BASE);
+ assert!(response.fb_regions.is_empty());
+
+ // A full response.
+ let mut encoder = Encoder::new();
+ encoder.encode_array8(Resp::GPU_NAME_STRING_KEY, index0, name)?;
+ encoder.encode_u64(Resp::BAR1_PDE_BASE_KEY, index0, BAR1_PDE_BASE)?;
+ encoder.encode_u64(FbRegionSchema::BASE_KEY, index0, FB_REGION0_BASE)?;
+ encoder.encode_u64(FbRegionSchema::LIMIT_KEY, index0, FB_REGION0_LIMIT)?;
+ encoder.encode_u32(FbRegionSchema::FLAGS_KEY, index0, FB_REGION0_FLAGS)?;
+ encoder.encode_u32(FbRegionSchema::TAG_KEY, index0, FB_REGION0_TAG)?;
+
+ // Test that this unrelated key can safely interleave.
+ encoder.encode_u64(Resp::VMMU_SEGMENT_SIZE_KEY, index0, VMMU_SEGMENT_SIZE)?;
+
+ encoder.encode_u64(FbRegionSchema::BASE_KEY, index1, FB_REGION1_BASE)?;
+ encoder.encode_u64(FbRegionSchema::LIMIT_KEY, index1, FB_REGION1_LIMIT)?;
+ encoder.encode_u32(FbRegionSchema::FLAGS_KEY, index1, FB_REGION1_FLAGS)?;
+ encoder.encode_u32(FbRegionSchema::TAG_KEY, index1, FB_REGION1_TAG)?;
+ let data = encoder.finish();
+
+ let decoder = Decoder::new(&data, UnknownKeyPolicy::Error);
+ let response = KBox::try_init(decoder.decode(Resp::default())?, GFP_KERNEL)?;
+
+ assert_eq!(&*response.gpu_name, &name[..]);
+ assert_eq!(response.bar1_pde_base, BAR1_PDE_BASE);
+ assert_eq!(response.fb_regions.len(), 2);
+
+ let fb_region0 = &response.fb_regions[0];
+ assert_eq!(fb_region0.base, FB_REGION0_BASE);
+ assert_eq!(fb_region0.limit, FB_REGION0_LIMIT);
+ assert_eq!(fb_region0.flags.into_raw(), FB_REGION0_FLAGS);
+ assert!(fb_region0.flags.support_compressed());
+ assert!(fb_region0.flags.support_iso());
+ assert!(fb_region0.flags.protected());
+ assert_eq!(fb_region0.tag, FB_REGION0_TAG);
+
+ let fb_region1 = &response.fb_regions[1];
+ assert_eq!(fb_region1.base, FB_REGION1_BASE);
+ assert_eq!(fb_region1.limit, FB_REGION1_LIMIT);
+ assert_eq!(fb_region1.flags.into_raw(), FB_REGION1_FLAGS);
+ assert_eq!(fb_region1.tag, FB_REGION1_TAG);
+
+ assert_eq!(response.vmmu_segment_size, VMMU_SEGMENT_SIZE);
+
+ Ok(())
+ }
+}
diff --git a/drivers/gpu/nova-core/gsp/nvkv.rs b/drivers/gpu/nova-core/gsp/nvkv.rs
index a0068847bb80..0afd6d5c48bd 100644
--- a/drivers/gpu/nova-core/gsp/nvkv.rs
+++ b/drivers/gpu/nova-core/gsp/nvkv.rs
@@ -9,9 +9,6 @@
//! function calls will map to some struct - for example, f(GPU_NAME_STRING_KEY, 0, b"some gpu")
//! naturally maps to storing a &str with the GPU name.
-#![cfg_attr(not(CONFIG_KUNIT), expect(unused_imports))]
-#![cfg_attr(not(CONFIG_KUNIT), expect(unused_macros))]
-
use core::marker::PhantomData;
use core::ops::{
Deref,
--
2.55.0
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH 6/6] gpu: nova-core: add NVKV GSP_INIT schemas
2026-08-17 12:56 ` [PATCH 6/6] gpu: nova-core: add NVKV GSP_INIT schemas Eliot Courtney
@ 2026-08-17 13:06 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-08-17 13:06 UTC (permalink / raw)
To: Eliot Courtney; +Cc: Danilo Krummrich, dri-devel, Alexandre Courbot, ojeda
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] KVVec<RegKey> encoding does not increment NVKV element indices, causing all array elements to be sent with index 0.
--
--- Patch [6]: [PATCH 6/6] gpu: nova-core: add NVKV GSP_INIT schemas ---
commit 2486533ab3027e1001c092bef8bd2ab3fb8541bf
Author: Eliot Courtney <ecourtney@nvidia.com>
gpu: nova-core: add NVKV GSP_INIT schemas
This commit adds the first user of NVKV encode/decode, which is the request
and response for GSP init. It supports these messages for unit tests.
> diff --git a/drivers/gpu/nova-core/gsp/fw/commands.rs b/drivers/gpu/nova-core/gsp/fw/commands.rs
> index 6dc31d1bf5ae4..4de44c2dc6aa3 100644
> --- a/drivers/gpu/nova-core/gsp/fw/commands.rs
> +++ b/drivers/gpu/nova-core/gsp/fw/commands.rs
[ ... ]
> +impl Encodable for KVVec<RegKey> {
> + fn encode(&self, encoder: &mut Encoder) -> Result {
> + for regkey in self {
> + regkey.encode(encoder)?;
> + }
> + Ok(())
> + }
> +}
[Severity: High]
When iterating over the KVVec<RegKey> array in this custom Encodable
implementation, does this code need a mechanism to increment the NVKV
element index?
Because the underlying Key type appears to hardcode Index::new::<0>() for
all encodings (as Encodable::encode does not accept an index parameter), it
looks like all array elements will be encoded with index 0.
Will the GSP firmware overwrite earlier items with the last one or reject
the message due to the missing incrementing index when multiple registry
keys are present?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-b4-nvkv-v1-0-b84db5e84b67@nvidia.com?part=6
^ permalink raw reply [flat|nested] 11+ messages in thread