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"hello";
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>
Reviewed-by: Lyude Paul <lyude@redhat.com>
---
Changes for v5:
- Typos
- s/ETOOSMALL/EINVAL/
- Add documentation
- Fix up examples
Changes for v3:
- Addressed minor review comment from Lyude
---
drivers/gpu/nova-core/nova_core.rs | 1 +
drivers/gpu/nova-core/sbuffer.rs | 218 +++++++++++++++++++++++++++++
2 files changed, 219 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 fffcaee2249f..a6feeba6254c 100644
--- a/drivers/gpu/nova-core/nova_core.rs
+++ b/drivers/gpu/nova-core/nova_core.rs
@@ -11,6 +11,7 @@
mod gpu;
mod gsp;
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 000000000000..d9c412a68bd8
--- /dev/null
+++ b/drivers/gpu/nova-core/sbuffer.rs
@@ -0,0 +1,218 @@
+// 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
+/// of the same length as a single stream-like read/write buffer.
+///
+/// # Example:
+///
+/// ```
+/// let mut buf1 = [0u8; 5];
+/// let mut buf2 = [0u8; 5];
+/// let mut sbuffer = SBufferIter::new_writer([&buf1, &buf2]);
+///
+/// let data = b"hello";
+/// let result = sbuffer.write_all(data);
+/// ```
+///
+/// A sliding window of slices to process.
+///
+/// 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 SBufferIter<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> SBufferIter<I>
+where
+ I: Iterator,
+{
+ /// Creates a reader buffer for a discontiguous set of byte slices.
+ ///
+ /// # Example:
+ ///
+ /// ```
+ /// let buf1: [u8; 5] = [0, 1, 2, 3, 4];
+ /// let buf2: [u8; 5] = [5, 6, 7, 8, 9];
+ /// let sbuffer = SBufferIter::new_reader([&buf1[..], &buf2[..]]);
+ /// let sum: u8 = sbuffer.sum();
+ /// assert_eq!(sum, 45);
+ /// ```
+ #[expect(unused)]
+ pub(crate) fn new_reader(slices: impl IntoIterator<IntoIter = I>) -> Self
+ where
+ I: Iterator<Item = &'a [u8]>,
+ {
+ Self::new(slices)
+ }
+
+ /// Creates a writeable buffer for a discontiguous set of byte slices.
+ ///
+ /// # Example:
+ ///
+ /// ```
+ /// let mut buf1 = [0u8; 5];
+ /// let mut buf2 = [0u8; 5];
+ /// let mut sbuffer = SBufferIter::new_writer([&mut buf1[..], &mut buf2[..]]);
+ /// sbuffer.write_all(&[0u8, 1, 2, 3, 4, 5, 6, 7, 8, 9][..])?;
+ /// drop(sbuffer);
+ /// assert_eq!(buf1, [0, 1, 2, 3, 4]);
+ /// assert_eq!(buf2, [5, 6, 7, 8, 9]);
+ ///
+ /// ```
+ #[expect(unused)]
+ 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.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> SBufferIter<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(EINVAL),
+ 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> SBufferIter<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`.
+ #[expect(unused)]
+ 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 SBufferIter<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.50.1
On Mon Oct 13, 2025 at 3:20 PM JST, 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"hello";
> 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>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
>
> ---
>
> Changes for v5:
> - Typos
> - s/ETOOSMALL/EINVAL/
> - Add documentation
> - Fix up examples
>
> Changes for v3:
> - Addressed minor review comment from Lyude
> ---
> drivers/gpu/nova-core/nova_core.rs | 1 +
> drivers/gpu/nova-core/sbuffer.rs | 218 +++++++++++++++++++++++++++++
> 2 files changed, 219 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 fffcaee2249f..a6feeba6254c 100644
> --- a/drivers/gpu/nova-core/nova_core.rs
> +++ b/drivers/gpu/nova-core/nova_core.rs
> @@ -11,6 +11,7 @@
> mod gpu;
> mod gsp;
> 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 000000000000..d9c412a68bd8
> --- /dev/null
> +++ b/drivers/gpu/nova-core/sbuffer.rs
> @@ -0,0 +1,218 @@
> +// 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
> +/// of the same length as a single stream-like read/write buffer.
> +///
> +/// # Example:
> +///
> +/// ```
> +/// let mut buf1 = [0u8; 5];
> +/// let mut buf2 = [0u8; 5];
> +/// let mut sbuffer = SBufferIter::new_writer([&buf1, &buf2]);
> +///
> +/// let data = b"hello";
> +/// let result = sbuffer.write_all(data);
> +/// ```
This example doesn't build - there are several things wrong with it. It
is also missing statements to confirm and show the expected result. Here
is a fixed and slightly improved version:
/// let mut buf1 = [0u8; 5];
/// let mut buf2 = [0u8; 5];
/// let mut sbuffer = SBufferIter::new_writer([&mut buf1[..], &mut buf2[..]]);
///
/// let data = b"hi world!";
/// sbuffer.write_all(data)?;
/// drop(sbuffer);
///
/// assert_eq!(buf1, *b"hi wo");
/// assert_eq!(buf2, *b"rld!\0");
///
/// # Ok::<(), Error>(())
> +///
> +/// A sliding window of slices to process.
> +///
> +/// 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`.
Why is there another doccomment after the example section? It looks like
this should be merged with the first doccomment before the example?
There is also no `S` generic parameter.
> +pub(crate) struct SBufferIter<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> SBufferIter<I>
> +where
> + I: Iterator,
> +{
> + /// Creates a reader buffer for a discontiguous set of byte slices.
> + ///
> + /// # Example:
> + ///
> + /// ```
> + /// let buf1: [u8; 5] = [0, 1, 2, 3, 4];
> + /// let buf2: [u8; 5] = [5, 6, 7, 8, 9];
> + /// let sbuffer = SBufferIter::new_reader([&buf1[..], &buf2[..]]);
> + /// let sum: u8 = sbuffer.sum();
> + /// assert_eq!(sum, 45);
> + /// ```
> + #[expect(unused)]
> + pub(crate) fn new_reader(slices: impl IntoIterator<IntoIter = I>) -> Self
> + where
> + I: Iterator<Item = &'a [u8]>,
> + {
> + Self::new(slices)
> + }
> +
> + /// Creates a writeable buffer for a discontiguous set of byte slices.
> + ///
> + /// # Example:
> + ///
> + /// ```
> + /// let mut buf1 = [0u8; 5];
> + /// let mut buf2 = [0u8; 5];
> + /// let mut sbuffer = SBufferIter::new_writer([&mut buf1[..], &mut buf2[..]]);
> + /// sbuffer.write_all(&[0u8, 1, 2, 3, 4, 5, 6, 7, 8, 9][..])?;
> + /// drop(sbuffer);
> + /// assert_eq!(buf1, [0, 1, 2, 3, 4]);
> + /// assert_eq!(buf2, [5, 6, 7, 8, 9]);
> + ///
> + /// ```
> + #[expect(unused)]
> + 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.
I guess "Skip empty slices" is enough as it is part of the algorithm. :)
> + 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>
Let's document this a bit as its purpose is not immediately clear. We
can take the documentation from the `get_slice` methods, with a short
explanation that the closure is supposed to split the slice received as
first argument at the position given as the second.
> + 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.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> SBufferIter<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`.
That's a useful side-comment, but we also need small sentence describing
the method, e.g. "Fill `dst` with the next bytes from this
`SBufferIter`, or return `EINVAL` if there isn't enough data available."
> + #[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(EINVAL),
> + 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>> {
nit: `flush_into_kvec` is probably a more descriptive name, as `read` is
already used for the other method.
> + 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> SBufferIter<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`.
Same comment as `read_all`.
On Thu, Oct 16, 2025 at 8:23 AM Alexandre Courbot <acourbot@nvidia.com> wrote:
>
> On Mon Oct 13, 2025 at 3:20 PM JST, Alistair Popple wrote:
> >
> > +/// # Example:
> > +///
> > +/// ```
> > +/// let mut buf1 = [0u8; 5];
> > +/// let mut buf2 = [0u8; 5];
> > +/// let mut sbuffer = SBufferIter::new_writer([&buf1, &buf2]);
> > +///
> > +/// let data = b"hello";
> > +/// let result = sbuffer.write_all(data);
> > +/// ```
>
> This example doesn't build - there are several things wrong with it. It
> is also missing statements to confirm and show the expected result. Here
> is a fixed and slightly improved version:
Yeah, I mentioned this one in a previous version -- the section header
is also still wrong too.
Alistair, please check the link I gave:
https://docs.kernel.org/rust/coding-guidelines.html#code-documentation
or other code in the `kernel` crate for examples on how it is usually done.
It is not critical today, of course, but the further it is from what
will be needed in a few months, the harder it will become to start
building the docs and running the examples as KUnit tests.
Thanks!
Cheers,
Miguel
On 2025-10-17 at 06:18 +1100, Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote... > On Thu, Oct 16, 2025 at 8:23 AM Alexandre Courbot <acourbot@nvidia.com> wrote: > > > > On Mon Oct 13, 2025 at 3:20 PM JST, Alistair Popple wrote: > > > > > > +/// # Example: > > > +/// > > > +/// ``` > > > +/// let mut buf1 = [0u8; 5]; > > > +/// let mut buf2 = [0u8; 5]; > > > +/// let mut sbuffer = SBufferIter::new_writer([&buf1, &buf2]); > > > +/// > > > +/// let data = b"hello"; > > > +/// let result = sbuffer.write_all(data); > > > +/// ``` > > > > This example doesn't build - there are several things wrong with it. It > > is also missing statements to confirm and show the expected result. Here > > is a fixed and slightly improved version: Argh, you're right. I cut and pasted then edited the wrong thing from my test build. How are you building these? The `rustdoc` target seems to ignore Nova (or I'm doing something wrong). > Yeah, I mentioned this one in a previous version -- the section header > is also still wrong too. > > Alistair, please check the link I gave: Will do. I thought Joel had addressed your comments in the fix patch I pulled in from him (he wrote most of this originally) but I can see the `/// # Example:` heading is wrong. > https://docs.kernel.org/rust/coding-guidelines.html#code-documentation > > or other code in the `kernel` crate for examples on how it is usually done. > > It is not critical today, of course, but the further it is from what > will be needed in a few months, the harder it will become to start > building the docs and running the examples as KUnit tests. No, I think it *is* critical :-) Much easier just to get this right from the beginning than deal with heaps of errors/warnings later. It's just my fingers that are still getting used to the subtle differences between C kernel code and Rust kernel code, so thanks for the guidance. - Alistair > > Thanks! > > Cheers, > Miguel
On Fri Oct 17, 2025 at 1:45 PM JST, Alistair Popple wrote: > On 2025-10-17 at 06:18 +1100, Miguel Ojeda <miguel.ojeda.sandonis@gmail.com> wrote... >> On Thu, Oct 16, 2025 at 8:23 AM Alexandre Courbot <acourbot@nvidia.com> wrote: >> > >> > On Mon Oct 13, 2025 at 3:20 PM JST, Alistair Popple wrote: >> > > >> > > +/// # Example: >> > > +/// >> > > +/// ``` >> > > +/// let mut buf1 = [0u8; 5]; >> > > +/// let mut buf2 = [0u8; 5]; >> > > +/// let mut sbuffer = SBufferIter::new_writer([&buf1, &buf2]); >> > > +/// >> > > +/// let data = b"hello"; >> > > +/// let result = sbuffer.write_all(data); >> > > +/// ``` >> > >> > This example doesn't build - there are several things wrong with it. It >> > is also missing statements to confirm and show the expected result. Here >> > is a fixed and slightly improved version: > > Argh, you're right. I cut and pasted then edited the wrong thing from my test > build. How are you building these? The `rustdoc` target seems to ignore Nova (or > I'm doing something wrong). Indeed, rustdoc doesn't consider anything outside of the kernel crate for now, although this is scheduled to change so we need to be ready for it. For now, I just copy/paste the test code into some function elsewhere and try to build. ^_^;
© 2016 - 2025 Red Hat, Inc.