[PATCH 04/10] gpu: nova-core: Add a slice-buffer (sbuffer) datastructure

Alistair Popple posted 10 patches 1 month, 1 week ago
There is a newer version of this series
[PATCH 04/10] gpu: nova-core: Add a slice-buffer (sbuffer) datastructure
Posted by Alistair Popple 1 month, 1 week ago
From: Joel Fernandes <joelagnelf@nvidia.com>

A data structure that can be used to write across multiple slices which
may be out of order in memory. This lets SBuffer user correctly and
safely write out of memory order, without error-prone tracking of
pointers/offsets.

 let mut buf1 = [0u8; 3];
 let mut buf2 = [0u8; 5];
 let mut sbuffer = SBuffer::new([&mut buf1[..], &mut buf2[..]]);

 let data = b"hellowo";
 let result = sbuffer.write(data);

An internal conversion of gsp.rs to use this resulted in a nice -ve delta:
gsp.rs: 37 insertions(+), 99 deletions(-)

Co-developed-by: Alistair Popple <apopple@nvidia.com>
Signed-off-by: Alistair Popple <apopple@nvidia.com>
Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
---
 drivers/gpu/nova-core/nova_core.rs |   1 +
 drivers/gpu/nova-core/sbuffer.rs   | 188 +++++++++++++++++++++++++++++
 2 files changed, 189 insertions(+)
 create mode 100644 drivers/gpu/nova-core/sbuffer.rs

diff --git a/drivers/gpu/nova-core/nova_core.rs b/drivers/gpu/nova-core/nova_core.rs
index db197498b0b7b..4dbc7e5daae33 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -12,6 +12,7 @@
 mod gsp;
 mod nvfw;
 mod regs;
+mod sbuffer;
 mod util;
 mod vbios;
 
diff --git a/drivers/gpu/nova-core/sbuffer.rs b/drivers/gpu/nova-core/sbuffer.rs
new file mode 100644
index 0000000000000..b1b8c4536d2f6
--- /dev/null
+++ b/drivers/gpu/nova-core/sbuffer.rs
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: GPL-2.0
+
+use core::ops::Deref;
+
+use kernel::alloc::KVec;
+use kernel::error::code::*;
+use kernel::prelude::*;
+
+/// A buffer abstraction for discontiguous byte slices.
+///
+/// This allows you to treat multiple non-contiguous `&mut [u8]` slices
+/// as a single stream-like read/write buffer.
+///
+/// Example:
+///
+/// let mut buf1 = [0u8; 3];
+/// let mut buf2 = [0u8; 5];
+/// let mut sbuffer = SWriteBuffer::new([&buf1, &buf2]);
+///
+/// let data = b"hellowo";
+/// let result = sbuffer.write_all(0, data);
+///
+/// A sliding window of slices to proceed.
+///
+/// Both read and write buffers are implemented in terms of operating on slices of a requested
+/// size. This base class implements logic that can be shared between the two to support that.
+///
+/// `S` is a slice type, `I` is an iterator yielding `S`.
+pub(crate) struct SBuffer<I: Iterator> {
+    /// `Some` if we are not at the end of the data yet.
+    cur_slice: Option<I::Item>,
+    /// All the slices remaining after `cur_slice`.
+    slices: I,
+}
+
+impl<'a, I> SBuffer<I>
+where
+    I: Iterator,
+{
+    pub(crate) fn new_reader(slices: impl IntoIterator<IntoIter = I>) -> Self
+    where
+        I: Iterator<Item = &'a [u8]>,
+    {
+        Self::new(slices)
+    }
+
+    pub(crate) fn new_writer(slices: impl IntoIterator<IntoIter = I>) -> Self
+    where
+        I: Iterator<Item = &'a mut [u8]>,
+    {
+        Self::new(slices)
+    }
+
+    fn new(slices: impl IntoIterator<IntoIter = I>) -> Self
+    where
+        I::Item: Deref<Target = [u8]>,
+    {
+        let mut slices = slices.into_iter();
+
+        Self {
+            // Skip empty slices to avoid trouble down the road.
+            cur_slice: slices.find(|s| !s.deref().is_empty()),
+            slices,
+        }
+    }
+
+    fn get_slice_internal(
+        &mut self,
+        len: usize,
+        mut f: impl FnMut(I::Item, usize) -> (I::Item, I::Item),
+    ) -> Option<I::Item>
+    where
+        I::Item: Deref<Target = [u8]>,
+    {
+        match self.cur_slice.take() {
+            None => None,
+            Some(cur_slice) => {
+                if len >= cur_slice.len() {
+                    // Caller requested more data than is in the current slice, return it entirely
+                    // and prepare the following slice for being used. Skip empty slices to avoid
+                    // trouble.
+                    self.cur_slice = self.slices.find(|s| !s.deref().is_empty());
+
+                    Some(cur_slice)
+                } else {
+                    // The current slice can satisfy the request, split it and return a slice of
+                    // the requested size.
+                    let (ret, next) = f(cur_slice, len);
+                    self.cur_slice = Some(next);
+
+                    Some(ret)
+                }
+            }
+        }
+    }
+}
+
+/// Provides a way to get non-mutable slices of data to read from.
+impl<'a, I> SBuffer<I>
+where
+    I: Iterator<Item = &'a [u8]>,
+{
+    /// Returns a slice of at most `len` bytes, or `None` if we are at the end of the data.
+    ///
+    /// If a slice shorter than `len` bytes has been returned, the caller can call this method
+    /// again until it returns `None` to try and obtain the remainder of the data.
+    fn get_slice(&mut self, len: usize) -> Option<&'a [u8]> {
+        self.get_slice_internal(len, |s, pos| s.split_at(pos))
+    }
+
+    /// Ideally we would implement `Read`, but it is not available in `core`.
+    /// So mimic `std::io::Read::read_exact`.
+    #[expect(unused)]
+    pub(crate) fn read_exact(&mut self, mut dst: &mut [u8]) -> Result {
+        while !dst.is_empty() {
+            match self.get_slice(dst.len()) {
+                None => return Err(ETOOSMALL),
+                Some(src) => {
+                    let dst_slice;
+                    (dst_slice, dst) = dst.split_at_mut(src.len());
+                    dst_slice.copy_from_slice(src);
+                }
+            }
+        }
+
+        Ok(())
+    }
+
+    /// Read all the remaining data into a `KVec`.
+    ///
+    /// `self` will be empty after this operation.
+    #[expect(unused)]
+    pub(crate) fn read_into_kvec(&mut self, flags: kernel::alloc::Flags) -> Result<KVec<u8>> {
+        let mut buf = KVec::<u8>::new();
+
+        if let Some(slice) = core::mem::take(&mut self.cur_slice) {
+            buf.extend_from_slice(slice, flags)?;
+        }
+        for slice in &mut self.slices {
+            buf.extend_from_slice(slice, flags)?;
+        }
+
+        Ok(buf)
+    }
+}
+
+/// Provides a way to get mutable slices of data to write into.
+impl<'a, I> SBuffer<I>
+where
+    I: Iterator<Item = &'a mut [u8]>,
+{
+    /// Returns a mutable slice of at most `len` bytes, or `None` if we are at the end of the data.
+    ///
+    /// If a slice shorter than `len` bytes has been returned, the caller can call this method
+    /// again until it returns `None` to try and obtain the remainder of the data.
+    fn get_slice_mut(&mut self, len: usize) -> Option<&'a mut [u8]> {
+        self.get_slice_internal(len, |s, pos| s.split_at_mut(pos))
+    }
+
+    /// Ideally we would implement `Write`, but it is not available in `core`.
+    /// So mimic `std::io::Write::write_all`.
+    pub(crate) fn write_all(&mut self, mut src: &[u8]) -> Result {
+        while !src.is_empty() {
+            match self.get_slice_mut(src.len()) {
+                None => return Err(ETOOSMALL),
+                Some(dst) => {
+                    let src_slice;
+                    (src_slice, src) = src.split_at(dst.len());
+                    dst.copy_from_slice(src_slice);
+                }
+            }
+        }
+
+        Ok(())
+    }
+}
+
+impl<'a, I> Iterator for SBuffer<I>
+where
+    I: Iterator<Item = &'a [u8]>,
+{
+    type Item = u8;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        // Returned slices are guaranteed to not be empty so we can safely index the first entry.
+        self.get_slice(1).map(|s| s[0])
+    }
+}
-- 
2.47.2
Re: [PATCH 04/10] gpu: nova-core: Add a slice-buffer (sbuffer) datastructure
Posted by Alice Ryhl 3 weeks, 5 days ago
On Wed, Aug 27, 2025 at 06:20:01PM +1000, Alistair Popple wrote:
> From: Joel Fernandes <joelagnelf@nvidia.com>
> 
> A data structure that can be used to write across multiple slices which
> may be out of order in memory. This lets SBuffer user correctly and
> safely write out of memory order, without error-prone tracking of
> pointers/offsets.
> 
>  let mut buf1 = [0u8; 3];
>  let mut buf2 = [0u8; 5];
>  let mut sbuffer = SBuffer::new([&mut buf1[..], &mut buf2[..]]);
> 
>  let data = b"hellowo";
>  let result = sbuffer.write(data);
> 
> An internal conversion of gsp.rs to use this resulted in a nice -ve delta:
> gsp.rs: 37 insertions(+), 99 deletions(-)
> 
> Co-developed-by: Alistair Popple <apopple@nvidia.com>
> Signed-off-by: Alistair Popple <apopple@nvidia.com>
> Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>

This seems like duplication of the logic in rust/kernel/iov_iter.rs [1].

Alice

[1]: https://lore.kernel.org/r/20250822-iov-iter-v5-0-6ce4819c2977@google.com
Re: [PATCH 04/10] gpu: nova-core: Add a slice-buffer (sbuffer) datastructure
Posted by Alistair Popple 3 weeks, 4 days ago
On 2025-09-07 at 20:54 +1000, Alice Ryhl <aliceryhl@google.com> wrote...
> On Wed, Aug 27, 2025 at 06:20:01PM +1000, Alistair Popple wrote:
> > From: Joel Fernandes <joelagnelf@nvidia.com>
> > 
> > A data structure that can be used to write across multiple slices which
> > may be out of order in memory. This lets SBuffer user correctly and
> > safely write out of memory order, without error-prone tracking of
> > pointers/offsets.
> > 
> >  let mut buf1 = [0u8; 3];
> >  let mut buf2 = [0u8; 5];
> >  let mut sbuffer = SBuffer::new([&mut buf1[..], &mut buf2[..]]);
> > 
> >  let data = b"hellowo";
> >  let result = sbuffer.write(data);
> > 
> > An internal conversion of gsp.rs to use this resulted in a nice -ve delta:
> > gsp.rs: 37 insertions(+), 99 deletions(-)
> > 
> > Co-developed-by: Alistair Popple <apopple@nvidia.com>
> > Signed-off-by: Alistair Popple <apopple@nvidia.com>
> > Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> 
> This seems like duplication of the logic in rust/kernel/iov_iter.rs [1].

Conceptually I guess there is some overlap. The thing that's different here
is we don't have any C version of the iovec struct or iov_iter, and AFAICT [1]
doesn't provide any way of creating one from within Rust code.

> Alice
> 
> [1]: https://lore.kernel.org/r/20250822-iov-iter-v5-0-6ce4819c2977@google.com
Re: [PATCH 04/10] gpu: nova-core: Add a slice-buffer (sbuffer) datastructure
Posted by Alexandre Courbot 3 weeks, 4 days ago
On Mon Sep 8, 2025 at 8:31 PM JST, Alistair Popple wrote:
> On 2025-09-07 at 20:54 +1000, Alice Ryhl <aliceryhl@google.com> wrote...
>> On Wed, Aug 27, 2025 at 06:20:01PM +1000, Alistair Popple wrote:
>> > From: Joel Fernandes <joelagnelf@nvidia.com>
>> > 
>> > A data structure that can be used to write across multiple slices which
>> > may be out of order in memory. This lets SBuffer user correctly and
>> > safely write out of memory order, without error-prone tracking of
>> > pointers/offsets.
>> > 
>> >  let mut buf1 = [0u8; 3];
>> >  let mut buf2 = [0u8; 5];
>> >  let mut sbuffer = SBuffer::new([&mut buf1[..], &mut buf2[..]]);
>> > 
>> >  let data = b"hellowo";
>> >  let result = sbuffer.write(data);
>> > 
>> > An internal conversion of gsp.rs to use this resulted in a nice -ve delta:
>> > gsp.rs: 37 insertions(+), 99 deletions(-)
>> > 
>> > Co-developed-by: Alistair Popple <apopple@nvidia.com>
>> > Signed-off-by: Alistair Popple <apopple@nvidia.com>
>> > Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
>> 
>> This seems like duplication of the logic in rust/kernel/iov_iter.rs [1].
>
> Conceptually I guess there is some overlap. The thing that's different here
> is we don't have any C version of the iovec struct or iov_iter, and AFAICT [1]
> doesn't provide any way of creating one from within Rust code.

Yup, I was about to ask as well - I am not familiar with the C API, but
how can we use it from Rust, using e.g. a pair of slices as the data
source/destination? I see that `struct iovec` also has `__user` marker
for its base, which hints to me that it is not designed to work with
kernel data?
Re: [PATCH 04/10] gpu: nova-core: Add a slice-buffer (sbuffer) datastructure
Posted by Alistair Popple 3 weeks, 3 days ago
On 2025-09-08 at 21:42 +1000, Alexandre Courbot <acourbot@nvidia.com> wrote...
> On Mon Sep 8, 2025 at 8:31 PM JST, Alistair Popple wrote:
> > On 2025-09-07 at 20:54 +1000, Alice Ryhl <aliceryhl@google.com> wrote...
> >> On Wed, Aug 27, 2025 at 06:20:01PM +1000, Alistair Popple wrote:
> >> > From: Joel Fernandes <joelagnelf@nvidia.com>
> >> > 
> >> > A data structure that can be used to write across multiple slices which
> >> > may be out of order in memory. This lets SBuffer user correctly and
> >> > safely write out of memory order, without error-prone tracking of
> >> > pointers/offsets.
> >> > 
> >> >  let mut buf1 = [0u8; 3];
> >> >  let mut buf2 = [0u8; 5];
> >> >  let mut sbuffer = SBuffer::new([&mut buf1[..], &mut buf2[..]]);
> >> > 
> >> >  let data = b"hellowo";
> >> >  let result = sbuffer.write(data);
> >> > 
> >> > An internal conversion of gsp.rs to use this resulted in a nice -ve delta:
> >> > gsp.rs: 37 insertions(+), 99 deletions(-)
> >> > 
> >> > Co-developed-by: Alistair Popple <apopple@nvidia.com>
> >> > Signed-off-by: Alistair Popple <apopple@nvidia.com>
> >> > Signed-off-by: Joel Fernandes <joelagnelf@nvidia.com>
> >> 
> >> This seems like duplication of the logic in rust/kernel/iov_iter.rs [1].
> >
> > Conceptually I guess there is some overlap. The thing that's different here
> > is we don't have any C version of the iovec struct or iov_iter, and AFAICT [1]
> > doesn't provide any way of creating one from within Rust code.
> 
> Yup, I was about to ask as well - I am not familiar with the C API, but
> how can we use it from Rust, using e.g. a pair of slices as the data
> source/destination? I see that `struct iovec` also has `__user` marker
> for its base, which hints to me that it is not designed to work with
> kernel data?

There are various flavours of iovec which you can iterate over. For example
there is kvec[1] which doesn't have the `__user` marker. But none of these are
what we want because we're not interacting with C code at all here.

[1] - https://elixir.bootlin.com/linux/v6.16.5/source/include/linux/uio.h#L18