On Thu Sep 10, 2026 at 5:10 PM JST, Alexandre Courbot wrote: > On Thu Aug 27, 2026 at 11:12 PM JST, Eliot Courtney wrote: >> 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 <[email protected]> >> --- >> drivers/gpu/nova-core/gsp/nvkv.rs | 49 ++++++++- >> drivers/gpu/nova-core/gsp/nvkv/encode.rs | 178 >> +++++++++++++++++++++++++++++++ >> 2 files changed, 226 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/nova-core/gsp/nvkv.rs >> b/drivers/gpu/nova-core/gsp/nvkv.rs >> index cbeee7f376b6..10dcbb9e602c 100644 >> --- a/drivers/gpu/nova-core/gsp/nvkv.rs >> +++ b/drivers/gpu/nova-core/gsp/nvkv.rs >> @@ -10,8 +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::ops::Deref; >> +use core::marker::PhantomData; >> +use core::ops::{ >> + Deref, >> + DerefMut, // >> +}; >> >> use kernel::{ >> alloc::{ >> @@ -92,6 +97,48 @@ fn deref(&self) -> &Self::Target { >> /// 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>); > > Does the `T` need to be `pub(crate)`? The series builds fine with it > being private. > > Also the relationship between `Key` and `IndexedKey` is a bit unclear > with the current type layout. IIUC `Key` is basically a specialization > of `IndexedKey` with an index of 0. And yet `Key` is declared in the > root `nvkv` module while `IndexedKey` is in the `encode` submodule... > I'm also wondering whether it would make sense to make the relationship > completely explicit by making `Key` a newtype embedding a `IndexedKey` > with the invariant that the index is `0`, but not sure about that one so > your call.
Ah, I guess that's because `IndexedKey` is local to `encoder`. In this case keeping it there does indeed make sense. After complaining about the visibility of other declarations, I should have noticed that one too. :P What worries me more is the fact that no index larger than `0` is ever created. This looks more and more like a bug to me.
