[PATCH v3 9/9] gpu: nova-core: gsp: add tests for SplitState

Eliot Courtney posted 9 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH v3 9/9] gpu: nova-core: gsp: add tests for SplitState
Posted by Eliot Courtney 1 month, 1 week ago
Add tests for SplitState. They cover boundary conditions at the split
points to make sure the right number of continuation records are made.
They also check that the data concatenated is correct.

Tested-by: Zhi Wang <zhiw@nvidia.com>
Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
---
 drivers/gpu/nova-core/gsp/commands.rs | 114 ++++++++++++++++++++++++++++++++++
 1 file changed, 114 insertions(+)

diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
index 6ffd0b9cbf05..74f875755e53 100644
--- a/drivers/gpu/nova-core/gsp/commands.rs
+++ b/drivers/gpu/nova-core/gsp/commands.rs
@@ -384,3 +384,117 @@ fn init_variable_payload(
         }
     }
 }
+
+#[kunit_tests(nova_core_gsp_commands)]
+mod tests {
+    use super::*;
+
+    struct TestPayload {
+        data: KVVec<u8>,
+    }
+
+    impl TestPayload {
+        fn generate_pattern(len: usize) -> Result<KVVec<u8>> {
+            let mut data = KVVec::with_capacity(len, GFP_KERNEL)?;
+            for i in 0..len {
+                data.push(i as u8, GFP_KERNEL)?;
+            }
+            Ok(data)
+        }
+
+        fn new(len: usize) -> Result<Self> {
+            Ok(Self {
+                data: Self::generate_pattern(len)?,
+            })
+        }
+    }
+
+    impl CommandToGsp for TestPayload {
+        const FUNCTION: MsgFunction = MsgFunction::Nop;
+        type Command = ();
+        type InitError = Infallible;
+
+        fn init(&self) -> impl Init<Self::Command, Self::InitError> {
+            <()>::init_zeroed()
+        }
+
+        fn variable_payload_len(&self) -> usize {
+            self.data.len()
+        }
+
+        fn init_variable_payload(
+            &self,
+            dst: &mut SBufferIter<core::array::IntoIter<&mut [u8], 2>>,
+        ) -> Result {
+            dst.write_all(self.data.as_slice())
+        }
+    }
+
+    const MAX_CMD_SIZE: usize = SplitState::<TestPayload>::MAX_CMD_SIZE;
+
+    fn read_payload(cmd: &impl CommandToGsp) -> Result<KVVec<u8>> {
+        let len = cmd.variable_payload_len();
+        let mut buf = KVVec::from_elem(0u8, len, GFP_KERNEL)?;
+        let mut sbuf = SBufferIter::new_writer([buf.as_mut_slice(), &mut []]);
+        cmd.init_variable_payload(&mut sbuf)?;
+        drop(sbuf);
+        Ok(buf)
+    }
+
+    struct SplitTest {
+        payload_size: usize,
+        num_continuations: usize,
+    }
+
+    fn check_split(t: SplitTest) -> Result {
+        let payload = TestPayload::new(t.payload_size)?;
+        let mut state = SplitState::new(&payload)?;
+
+        let mut buf = read_payload(&state.command(payload))?;
+        assert!(buf.len() <= MAX_CMD_SIZE);
+
+        let mut num_continuations = 0;
+        while let Some(cont) = state.next_continuation_record() {
+            let payload = read_payload(&cont)?;
+            assert!(payload.len() <= MAX_CMD_SIZE);
+            buf.extend_from_slice(&payload, GFP_KERNEL)?;
+            num_continuations += 1;
+        }
+
+        assert_eq!(num_continuations, t.num_continuations);
+        assert_eq!(
+            buf.as_slice(),
+            TestPayload::generate_pattern(t.payload_size)?.as_slice()
+        );
+        Ok(())
+    }
+
+    #[test]
+    fn split_command() -> Result {
+        check_split(SplitTest {
+            payload_size: 0,
+            num_continuations: 0,
+        })?;
+        check_split(SplitTest {
+            payload_size: MAX_CMD_SIZE,
+            num_continuations: 0,
+        })?;
+        check_split(SplitTest {
+            payload_size: MAX_CMD_SIZE + 1,
+            num_continuations: 1,
+        })?;
+        check_split(SplitTest {
+            payload_size: MAX_CMD_SIZE * 2,
+            num_continuations: 1,
+        })?;
+        check_split(SplitTest {
+            payload_size: MAX_CMD_SIZE * 2 + 1,
+            num_continuations: 2,
+        })?;
+        check_split(SplitTest {
+            payload_size: MAX_CMD_SIZE * 3 + MAX_CMD_SIZE / 2,
+            num_continuations: 3,
+        })?;
+        Ok(())
+    }
+}

-- 
2.53.0
Re: [PATCH v3 9/9] gpu: nova-core: gsp: add tests for SplitState
Posted by Alexandre Courbot 1 month, 1 week ago
On Thu Feb 26, 2026 at 8:45 PM JST, Eliot Courtney wrote:
> Add tests for SplitState. They cover boundary conditions at the split
> points to make sure the right number of continuation records are made.
> They also check that the data concatenated is correct.
>
> Tested-by: Zhi Wang <zhiw@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>

Also I'd call this patch "add tests for continuation records". That's
another reason to move the continuation record stuff to its own module,
that way these tests can be contained to it and named accordingly
instead of being mixed with potentially future command-related tests.
Re: [PATCH v3 9/9] gpu: nova-core: gsp: add tests for SplitState
Posted by Alexandre Courbot 1 month, 1 week ago
On Thu Feb 26, 2026 at 8:45 PM JST, Eliot Courtney wrote:
> Add tests for SplitState. They cover boundary conditions at the split
> points to make sure the right number of continuation records are made.
> They also check that the data concatenated is correct.
>
> Tested-by: Zhi Wang <zhiw@nvidia.com>
> Signed-off-by: Eliot Courtney <ecourtney@nvidia.com>
> ---
>  drivers/gpu/nova-core/gsp/commands.rs | 114 ++++++++++++++++++++++++++++++++++
>  1 file changed, 114 insertions(+)
>
> diff --git a/drivers/gpu/nova-core/gsp/commands.rs b/drivers/gpu/nova-core/gsp/commands.rs
> index 6ffd0b9cbf05..74f875755e53 100644
> --- a/drivers/gpu/nova-core/gsp/commands.rs
> +++ b/drivers/gpu/nova-core/gsp/commands.rs
> @@ -384,3 +384,117 @@ fn init_variable_payload(
>          }
>      }
>  }
> +
> +#[kunit_tests(nova_core_gsp_commands)]
> +mod tests {
> +    use super::*;
> +
> +    struct TestPayload {
> +        data: KVVec<u8>,
> +    }
> +
> +    impl TestPayload {
> +        fn generate_pattern(len: usize) -> Result<KVVec<u8>> {
> +            let mut data = KVVec::with_capacity(len, GFP_KERNEL)?;
> +            for i in 0..len {
> +                data.push(i as u8, GFP_KERNEL)?;
> +            }
> +            Ok(data)
> +        }
> +
> +        fn new(len: usize) -> Result<Self> {
> +            Ok(Self {
> +                data: Self::generate_pattern(len)?,
> +            })
> +        }
> +    }
> +
> +    impl CommandToGsp for TestPayload {
> +        const FUNCTION: MsgFunction = MsgFunction::Nop;
> +        type Command = ();

Since we are testing the size of the data written on the command queue,
can you make the command itself larger than 0 bytes? Otherwise there is
a potential for missing errors.

I'm saying that because it actually happened to me, I used
`MAX_CMD_SIZE` instead of `MAX_CMD_SIZE - size_of::<C::Command>()`
somewhere, and despite it being broken the tests were passing.
Re: [PATCH v3 9/9] gpu: nova-core: gsp: add tests for SplitState
Posted by Eliot Courtney 1 month ago
On Fri Feb 27, 2026 at 8:32 PM JST, Alexandre Courbot wrote:
>> +#[kunit_tests(nova_core_gsp_commands)]
>> +mod tests {
>> +    use super::*;
>> +
>> +    struct TestPayload {
>> +        data: KVVec<u8>,
>> +    }
>> +
>> +    impl TestPayload {
>> +        fn generate_pattern(len: usize) -> Result<KVVec<u8>> {
>> +            let mut data = KVVec::with_capacity(len, GFP_KERNEL)?;
>> +            for i in 0..len {
>> +                data.push(i as u8, GFP_KERNEL)?;
>> +            }
>> +            Ok(data)
>> +        }
>> +
>> +        fn new(len: usize) -> Result<Self> {
>> +            Ok(Self {
>> +                data: Self::generate_pattern(len)?,
>> +            })
>> +        }
>> +    }
>> +
>> +    impl CommandToGsp for TestPayload {
>> +        const FUNCTION: MsgFunction = MsgFunction::Nop;
>> +        type Command = ();
>
> Since we are testing the size of the data written on the command queue,
> can you make the command itself larger than 0 bytes? Otherwise there is
> a potential for missing errors.
>
> I'm saying that because it actually happened to me, I used
> `MAX_CMD_SIZE` instead of `MAX_CMD_SIZE - size_of::<C::Command>()`
> somewhere, and despite it being broken the tests were passing.

Yeah good idea, particularly since you ran into it too. Added.