Re: [PATCH v2 8/8] gpu: nova-core: add NVKV GSP_INIT schemas

[Alexandre Courbot]

On Mon Sep 14, 2026 at 2:42 PM JST, Eliot Courtney wrote:
> On Mon Sep 14, 2026 at 1:11 PM JST, Alexandre Courbot wrote:
>> On Thu Aug 27, 2026 at 11:12 PM JST, Eliot Courtney wrote:
>> <...>
>>> +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(())
>>
>> Maybe this is just me misunderstanding, but how are the keys
>> sequentially sent here? Because I don't see any mention of an index, and
>> `Key::encode` hardcodes `Index::new::<0>()`, so how are these supposed
>> to be decoded into an array? The `gsp_init_request` test below only adds
>> one key to its `regkeys`, can we add at least another one to see what
>> happens and verify that the received content decodes as expected on top
>> of checking its length?
>
> You are not misunderstanding, it's just a bit odd. This is because
> sequential regkeys are all sent using an index of 0, according to the
> protocol. I can add a second regkey into the test to demonstrate this.
> We currently don't and won't soon have a need to decode this kind of
> repeated index 0 encoding scheme. We have `Accumualted` now, but that
> relies on the index changing to know when the previous value has been
> completely sent.
>
> To test that the content decodes we'd need to add either a test only
> Schema for it, or add a schema that isn't (and won't be soon be) used.
> Alternatively, we can test against the encoded byte content directly.
> Which do you prefer?

Whichever you think is adequate. :) As long as we test things our
current code allows us to express.

Although since the NVKV protocol is specified, I think we'll want to
confirm the fitness of our implementation for the entirety of it, even
the parts we are not yet using in practice. In that case I suppose it is
acceptable to have a Schema that is only used in the tests for the sake
of completeness (even if that means having a temporary
`#[expect(dead_code]`, as long as the reason is documented).