[PATCH v2 3/4] rust: miscdevice: Provide additional abstractions for iov_iter and kiocb structures

Alice Ryhl posted 4 patches 3 months ago
There is a newer version of this series
[PATCH v2 3/4] rust: miscdevice: Provide additional abstractions for iov_iter and kiocb structures
Posted by Alice Ryhl 3 months ago
These will be used for the read_iter() and write_iter() callbacks, which
are now the preferred back-ends for when a user operates on a char device
with read() and write() respectively.

Co-developed-by: Lee Jones <lee@kernel.org>
Signed-off-by: Lee Jones <lee@kernel.org>
Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/miscdevice.rs | 97 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 96 insertions(+), 1 deletion(-)

diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index 22f291211636f66efca6b33b675833236332719e..a49954c9b0d14117645be8139db792f1fd22589d 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -14,13 +14,14 @@
     error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR},
     ffi::{c_int, c_long, c_uint, c_ulong},
     fs::File,
+    iov::{IovIterDest, IovIterSource},
     mm::virt::VmaNew,
     prelude::*,
     seq_file::SeqFile,
     str::CStr,
     types::{ForeignOwnable, Opaque},
 };
-use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin};
+use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin, ptr::NonNull};
 
 /// Options for creating a misc device.
 #[derive(Copy, Clone)]
@@ -136,6 +137,16 @@ fn mmap(
         build_error!(VTABLE_DEFAULT_ERROR)
     }
 
+    /// Read from this miscdevice.
+    fn read_iter(_kiocb: Kiocb<'_, Self::Ptr>, _iov: &mut IovIterDest<'_>) -> Result<usize> {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Write to this miscdevice.
+    fn write_iter(_kiocb: Kiocb<'_, Self::Ptr>, _iov: &mut IovIterSource<'_>) -> Result<usize> {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
     /// Handler for ioctls.
     ///
     /// The `cmd` argument is usually manipulated using the utilities in [`kernel::ioctl`].
@@ -177,6 +188,36 @@ fn show_fdinfo(
     }
 }
 
+/// Wrapper for the kernel's `struct kiocb`.
+///
+/// The type `T` represents the private data of the file.
+pub struct Kiocb<'a, T> {
+    inner: NonNull<bindings::kiocb>,
+    _phantom: PhantomData<&'a T>,
+}
+
+impl<'a, T: ForeignOwnable> Kiocb<'a, T> {
+    /// Get the Rust data that represents this misc device.
+    pub fn device(&self) -> <T as ForeignOwnable>::Borrowed<'a> {
+        // SAFETY: The `kiocb` lets us access the private data.
+        let private = unsafe { (*(*self.inner.as_ptr()).ki_filp).private_data };
+        // SAFETY: The kiocb has shared access to the private data.
+        unsafe { <T as ForeignOwnable>::borrow(private) }
+    }
+
+    /// Gets the current value of `ki_pos`.
+    pub fn ki_pos(&self) -> i64 {
+        // SAFETY: The `kiocb` can access `ki_pos`.
+        unsafe { (*self.inner.as_ptr()).ki_pos }
+    }
+
+    /// Gets a mutable reference to the `ki_pos` field.
+    pub fn ki_pos_mut(&mut self) -> &mut i64 {
+        // SAFETY: The `kiocb` can access `ki_pos`.
+        unsafe { &mut (*self.inner.as_ptr()).ki_pos }
+    }
+}
+
 /// A vtable for the file operations of a Rust miscdevice.
 struct MiscdeviceVTable<T: MiscDevice>(PhantomData<T>);
 
@@ -240,6 +281,50 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {
         0
     }
 
+    /// # Safety
+    ///
+    /// `kiocb` must be correspond to a valid file that is associated with a
+    /// `MiscDeviceRegistration<T>`. `iter` must be a valid `struct iov_iter` for writing.
+    unsafe extern "C" fn read_iter(
+        kiocb: *mut bindings::kiocb,
+        iter: *mut bindings::iov_iter,
+    ) -> isize {
+        let kiocb = Kiocb {
+            // SAFETY: The read_iter call of a file is given a kiocb for that file.
+            inner: unsafe { NonNull::new_unchecked(kiocb) },
+            _phantom: PhantomData,
+        };
+        // SAFETY: This is a valid `struct iov_iter` for writing.
+        let iov = unsafe { IovIterDest::from_raw(iter) };
+
+        match T::read_iter(kiocb, iov) {
+            Ok(res) => res as isize,
+            Err(err) => err.to_errno() as isize,
+        }
+    }
+
+    /// # Safety
+    ///
+    /// `kiocb` must be correspond to a valid file that is associated with a
+    /// `MiscDeviceRegistration<T>`. `iter` must be a valid `struct iov_iter` for writing.
+    unsafe extern "C" fn write_iter(
+        kiocb: *mut bindings::kiocb,
+        iter: *mut bindings::iov_iter,
+    ) -> isize {
+        let kiocb = Kiocb {
+            // SAFETY: The read_iter call of a file is given a kiocb for that file.
+            inner: unsafe { NonNull::new_unchecked(kiocb) },
+            _phantom: PhantomData,
+        };
+        // SAFETY: This is a valid `struct iov_iter` for reading.
+        let iov = unsafe { IovIterSource::from_raw(iter) };
+
+        match T::write_iter(kiocb, iov) {
+            Ok(res) => res as isize,
+            Err(err) => err.to_errno() as isize,
+        }
+    }
+
     /// # Safety
     ///
     /// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
@@ -336,6 +421,16 @@ impl<T: MiscDevice> MiscdeviceVTable<T> {
         open: Some(Self::open),
         release: Some(Self::release),
         mmap: if T::HAS_MMAP { Some(Self::mmap) } else { None },
+        read_iter: if T::HAS_READ_ITER {
+            Some(Self::read_iter)
+        } else {
+            None
+        },
+        write_iter: if T::HAS_WRITE_ITER {
+            Some(Self::write_iter)
+        } else {
+            None
+        },
         unlocked_ioctl: if T::HAS_IOCTL {
             Some(Self::ioctl)
         } else {

-- 
2.50.0.727.gbf7dc18ff4-goog
Re: [PATCH v2 3/4] rust: miscdevice: Provide additional abstractions for iov_iter and kiocb structures
Posted by Andreas Hindborg 3 months ago
"Alice Ryhl" <aliceryhl@google.com> writes:

> These will be used for the read_iter() and write_iter() callbacks, which
> are now the preferred back-ends for when a user operates on a char device
> with read() and write() respectively.
>
> Co-developed-by: Lee Jones <lee@kernel.org>
> Signed-off-by: Lee Jones <lee@kernel.org>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/miscdevice.rs | 97 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 96 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
> index 22f291211636f66efca6b33b675833236332719e..a49954c9b0d14117645be8139db792f1fd22589d 100644
> --- a/rust/kernel/miscdevice.rs
> +++ b/rust/kernel/miscdevice.rs
> @@ -14,13 +14,14 @@
>      error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR},
>      ffi::{c_int, c_long, c_uint, c_ulong},
>      fs::File,
> +    iov::{IovIterDest, IovIterSource},
>      mm::virt::VmaNew,
>      prelude::*,
>      seq_file::SeqFile,
>      str::CStr,
>      types::{ForeignOwnable, Opaque},
>  };
> -use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin};
> +use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin, ptr::NonNull};
>
>  /// Options for creating a misc device.
>  #[derive(Copy, Clone)]
> @@ -136,6 +137,16 @@ fn mmap(
>          build_error!(VTABLE_DEFAULT_ERROR)
>      }
>
> +    /// Read from this miscdevice.
> +    fn read_iter(_kiocb: Kiocb<'_, Self::Ptr>, _iov: &mut IovIterDest<'_>) -> Result<usize> {
> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }
> +
> +    /// Write to this miscdevice.
> +    fn write_iter(_kiocb: Kiocb<'_, Self::Ptr>, _iov: &mut IovIterSource<'_>) -> Result<usize> {
> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }
> +
>      /// Handler for ioctls.
>      ///
>      /// The `cmd` argument is usually manipulated using the utilities in [`kernel::ioctl`].
> @@ -177,6 +188,36 @@ fn show_fdinfo(
>      }
>  }
>
> +/// Wrapper for the kernel's `struct kiocb`.
> +///
> +/// The type `T` represents the private data of the file.
> +pub struct Kiocb<'a, T> {
> +    inner: NonNull<bindings::kiocb>,
> +    _phantom: PhantomData<&'a T>,
> +}

Also, `kiocb` is not miscdevice specific. It should probably not live here.


Best regards,
Andreas Hindborg
Re: [PATCH v2 3/4] rust: miscdevice: Provide additional abstractions for iov_iter and kiocb structures
Posted by Alice Ryhl 3 months ago
On Tue, Jul 08, 2025 at 04:53:23PM +0200, Andreas Hindborg wrote:
> "Alice Ryhl" <aliceryhl@google.com> writes:
> 
> > These will be used for the read_iter() and write_iter() callbacks, which
> > are now the preferred back-ends for when a user operates on a char device
> > with read() and write() respectively.
> >
> > Co-developed-by: Lee Jones <lee@kernel.org>
> > Signed-off-by: Lee Jones <lee@kernel.org>
> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> > ---
> >  rust/kernel/miscdevice.rs | 97 ++++++++++++++++++++++++++++++++++++++++++++++-
> >  1 file changed, 96 insertions(+), 1 deletion(-)
> >
> > diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
> > index 22f291211636f66efca6b33b675833236332719e..a49954c9b0d14117645be8139db792f1fd22589d 100644
> > --- a/rust/kernel/miscdevice.rs
> > +++ b/rust/kernel/miscdevice.rs
> > @@ -14,13 +14,14 @@
> >      error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR},
> >      ffi::{c_int, c_long, c_uint, c_ulong},
> >      fs::File,
> > +    iov::{IovIterDest, IovIterSource},
> >      mm::virt::VmaNew,
> >      prelude::*,
> >      seq_file::SeqFile,
> >      str::CStr,
> >      types::{ForeignOwnable, Opaque},
> >  };
> > -use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin};
> > +use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin, ptr::NonNull};
> >
> >  /// Options for creating a misc device.
> >  #[derive(Copy, Clone)]
> > @@ -136,6 +137,16 @@ fn mmap(
> >          build_error!(VTABLE_DEFAULT_ERROR)
> >      }
> >
> > +    /// Read from this miscdevice.
> > +    fn read_iter(_kiocb: Kiocb<'_, Self::Ptr>, _iov: &mut IovIterDest<'_>) -> Result<usize> {
> > +        build_error!(VTABLE_DEFAULT_ERROR)
> > +    }
> > +
> > +    /// Write to this miscdevice.
> > +    fn write_iter(_kiocb: Kiocb<'_, Self::Ptr>, _iov: &mut IovIterSource<'_>) -> Result<usize> {
> > +        build_error!(VTABLE_DEFAULT_ERROR)
> > +    }
> > +
> >      /// Handler for ioctls.
> >      ///
> >      /// The `cmd` argument is usually manipulated using the utilities in [`kernel::ioctl`].
> > @@ -177,6 +188,36 @@ fn show_fdinfo(
> >      }
> >  }
> >
> > +/// Wrapper for the kernel's `struct kiocb`.
> > +///
> > +/// The type `T` represents the private data of the file.
> > +pub struct Kiocb<'a, T> {
> > +    inner: NonNull<bindings::kiocb>,
> > +    _phantom: PhantomData<&'a T>,
> > +}
> 
> Also, `kiocb` is not miscdevice specific. It should probably not live here.

I can place it in rust/kernel/fs.rs, but this is an instance of the more
general fact that miscdevice defines many things from `file_operations`
that we should probably generalize in the future.

Alice
Re: [PATCH v2 3/4] rust: miscdevice: Provide additional abstractions for iov_iter and kiocb structures
Posted by Andreas Hindborg 3 months ago
"Alice Ryhl" <aliceryhl@google.com> writes:

> On Tue, Jul 08, 2025 at 04:53:23PM +0200, Andreas Hindborg wrote:
>> "Alice Ryhl" <aliceryhl@google.com> writes:
>>
>> > These will be used for the read_iter() and write_iter() callbacks, which
>> > are now the preferred back-ends for when a user operates on a char device
>> > with read() and write() respectively.
>> >
>> > Co-developed-by: Lee Jones <lee@kernel.org>
>> > Signed-off-by: Lee Jones <lee@kernel.org>
>> > Signed-off-by: Alice Ryhl <aliceryhl@google.com>
>> > ---
>> >  rust/kernel/miscdevice.rs | 97 ++++++++++++++++++++++++++++++++++++++++++++++-
>> >  1 file changed, 96 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
>> > index 22f291211636f66efca6b33b675833236332719e..a49954c9b0d14117645be8139db792f1fd22589d 100644
>> > --- a/rust/kernel/miscdevice.rs
>> > +++ b/rust/kernel/miscdevice.rs
>> > @@ -14,13 +14,14 @@
>> >      error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR},
>> >      ffi::{c_int, c_long, c_uint, c_ulong},
>> >      fs::File,
>> > +    iov::{IovIterDest, IovIterSource},
>> >      mm::virt::VmaNew,
>> >      prelude::*,
>> >      seq_file::SeqFile,
>> >      str::CStr,
>> >      types::{ForeignOwnable, Opaque},
>> >  };
>> > -use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin};
>> > +use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin, ptr::NonNull};
>> >
>> >  /// Options for creating a misc device.
>> >  #[derive(Copy, Clone)]
>> > @@ -136,6 +137,16 @@ fn mmap(
>> >          build_error!(VTABLE_DEFAULT_ERROR)
>> >      }
>> >
>> > +    /// Read from this miscdevice.
>> > +    fn read_iter(_kiocb: Kiocb<'_, Self::Ptr>, _iov: &mut IovIterDest<'_>) -> Result<usize> {
>> > +        build_error!(VTABLE_DEFAULT_ERROR)
>> > +    }
>> > +
>> > +    /// Write to this miscdevice.
>> > +    fn write_iter(_kiocb: Kiocb<'_, Self::Ptr>, _iov: &mut IovIterSource<'_>) -> Result<usize> {
>> > +        build_error!(VTABLE_DEFAULT_ERROR)
>> > +    }
>> > +
>> >      /// Handler for ioctls.
>> >      ///
>> >      /// The `cmd` argument is usually manipulated using the utilities in [`kernel::ioctl`].
>> > @@ -177,6 +188,36 @@ fn show_fdinfo(
>> >      }
>> >  }
>> >
>> > +/// Wrapper for the kernel's `struct kiocb`.
>> > +///
>> > +/// The type `T` represents the private data of the file.
>> > +pub struct Kiocb<'a, T> {
>> > +    inner: NonNull<bindings::kiocb>,
>> > +    _phantom: PhantomData<&'a T>,
>> > +}
>>
>> Also, `kiocb` is not miscdevice specific. It should probably not live here.
>
> I can place it in rust/kernel/fs.rs, but this is an instance of the more
> general fact that miscdevice defines many things from `file_operations`
> that we should probably generalize in the future.

I support moving `Kiocb` to `fs` now, rather than later.


Best regards,
Andreas Hindborg
Re: [PATCH v2 3/4] rust: miscdevice: Provide additional abstractions for iov_iter and kiocb structures
Posted by Andreas Hindborg 3 months ago
"Alice Ryhl" <aliceryhl@google.com> writes:

> These will be used for the read_iter() and write_iter() callbacks, which
> are now the preferred back-ends for when a user operates on a char device
> with read() and write() respectively.
>
> Co-developed-by: Lee Jones <lee@kernel.org>
> Signed-off-by: Lee Jones <lee@kernel.org>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/miscdevice.rs | 97 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 96 insertions(+), 1 deletion(-)
>
> diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
> index 22f291211636f66efca6b33b675833236332719e..a49954c9b0d14117645be8139db792f1fd22589d 100644
> --- a/rust/kernel/miscdevice.rs
> +++ b/rust/kernel/miscdevice.rs
> @@ -14,13 +14,14 @@
>      error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR},
>      ffi::{c_int, c_long, c_uint, c_ulong},
>      fs::File,
> +    iov::{IovIterDest, IovIterSource},
>      mm::virt::VmaNew,
>      prelude::*,
>      seq_file::SeqFile,
>      str::CStr,
>      types::{ForeignOwnable, Opaque},
>  };
> -use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin};
> +use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin, ptr::NonNull};
>
>  /// Options for creating a misc device.
>  #[derive(Copy, Clone)]
> @@ -136,6 +137,16 @@ fn mmap(
>          build_error!(VTABLE_DEFAULT_ERROR)
>      }
>
> +    /// Read from this miscdevice.
> +    fn read_iter(_kiocb: Kiocb<'_, Self::Ptr>, _iov: &mut IovIterDest<'_>) -> Result<usize> {
> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }
> +
> +    /// Write to this miscdevice.
> +    fn write_iter(_kiocb: Kiocb<'_, Self::Ptr>, _iov: &mut IovIterSource<'_>) -> Result<usize> {
> +        build_error!(VTABLE_DEFAULT_ERROR)
> +    }
> +
>      /// Handler for ioctls.
>      ///
>      /// The `cmd` argument is usually manipulated using the utilities in [`kernel::ioctl`].
> @@ -177,6 +188,36 @@ fn show_fdinfo(
>      }
>  }
>
> +/// Wrapper for the kernel's `struct kiocb`.
> +///
> +/// The type `T` represents the private data of the file.


Could you give more context? Please describe the purpose for the type
and intended use. Perhaps give an example that can be compile tested.


Best regards,
Andreas Hindborg
Re: [PATCH v2 3/4] rust: miscdevice: Provide additional abstractions for iov_iter and kiocb structures
Posted by Alice Ryhl 3 months ago
On Tue, Jul 08, 2025 at 04:51:11PM +0200, Andreas Hindborg wrote:
> "Alice Ryhl" <aliceryhl@google.com> writes:
> > +/// Wrapper for the kernel's `struct kiocb`.
> > +///
> > +/// The type `T` represents the private data of the file.
> 
> Could you give more context? Please describe the purpose for the type
> and intended use. Perhaps give an example that can be compile tested.

Right now, it's basically a `(&mut i64, &T)` tuple providing access to
the file position and private data. That's it.

Alice
Re: [PATCH v2 3/4] rust: miscdevice: Provide additional abstractions for iov_iter and kiocb structures
Posted by Andreas Hindborg 3 months ago
"Alice Ryhl" <aliceryhl@google.com> writes:

> On Tue, Jul 08, 2025 at 04:51:11PM +0200, Andreas Hindborg wrote:
>> "Alice Ryhl" <aliceryhl@google.com> writes:
>> > +/// Wrapper for the kernel's `struct kiocb`.
>> > +///
>> > +/// The type `T` represents the private data of the file.
>>
>> Could you give more context? Please describe the purpose for the type
>> and intended use. Perhaps give an example that can be compile tested.
>
> Right now, it's basically a `(&mut i64, &T)` tuple providing access to
> the file position and private data. That's it.

Great! Let's get something along those lines in the docs.


Best regards,
Andreas Hindborg