| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-Id: <DDD39JHAI4ED.2BGLWYNIRA987@kernel.org>
Date: Wed, 08 Oct 2025 18:41:40 +0200
From: "Danilo Krummrich" <dakr@...nel.org>
To: "Alistair Popple" <apopple@...dia.com>
Cc: <rust-for-linux@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
<acourbot@...dia.com>, "Miguel Ojeda" <ojeda@...nel.org>, "Alex Gaynor"
<alex.gaynor@...il.com>, "Boqun Feng" <boqun.feng@...il.com>, "Gary Guo"
<gary@...yguo.net>, Björn Roy Baron
<bjorn3_gh@...tonmail.com>, "Benno Lossin" <lossin@...nel.org>, "Andreas
Hindborg" <a.hindborg@...nel.org>, "Alice Ryhl" <aliceryhl@...gle.com>,
"Trevor Gross" <tmgross@...ch.edu>, "David Airlie" <airlied@...il.com>,
"Simona Vetter" <simona@...ll.ch>, "Maarten Lankhorst"
<maarten.lankhorst@...ux.intel.com>, "Maxime Ripard" <mripard@...nel.org>,
"Thomas Zimmermann" <tzimmermann@...e.de>, "John Hubbard"
<jhubbard@...dia.com>, "Joel Fernandes" <joelagnelf@...dia.com>, "Timur
Tabi" <ttabi@...dia.com>, <linux-kernel@...r.kernel.org>,
<nouveau@...ts.freedesktop.org>, "Lyude Paul" <lyude@...hat.com>
Subject: Re: [PATCH v4 04/13] gpu: nova-core: Add a slice-buffer (sbuffer)
datastructure
On Wed Oct 8, 2025 at 2:12 AM CEST, Alistair Popple wrote:
> diff --git a/drivers/gpu/nova-core/sbuffer.rs b/drivers/gpu/nova-core/sbuffer.rs
> new file mode 100644
> index 000000000000..e82f9d97ad21
> --- /dev/null
> +++ b/drivers/gpu/nova-core/sbuffer.rs
> @@ -0,0 +1,191 @@
> +// 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";
Not that it matters, but "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,
> +}
Does it make sense to split SBuffer into itself and a separate SBufferIter that
keeps a reference to the SBuffer? If not, I'd rename it to SBufferIter to make
it obvious to the user that it is an iterator type.
> +impl<'a, I> SBuffer<I>
> +where
> + I: Iterator,
> +{
> + #[expect(unused)]
> + pub(crate) fn new_reader(slices: impl IntoIterator<IntoIter = I>) -> Self
> + where
> + I: Iterator<Item = &'a [u8]>,
> + {
> + Self::new(slices)
> + }
> +
> + #[expect(unused)]
> + pub(crate) fn new_writer(slices: impl IntoIterator<IntoIter = I>) -> Self
> + where
> + I: Iterator<Item = &'a mut [u8]>,
> + {
> + Self::new(slices)
> + }
Please add some documentation for the constructors.
> + 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> 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),
ETOOSMALL is an NFS error code (it should also never be exposed to userspace). I
suggest to implement a custom error type instead and make it resolve to ENOSPC
or probably just EINVAL instead.
> + 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`.
> + #[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 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.50.1
Powered by blists - more mailing lists