[PATCH 0/3] rust: miscdevice: Extending the abstraction

[PATCH 0/3] rust: miscdevice: Extending the abstraction

[ManeraKai]

Hi,

The following patches are fairly simple, as this is my first time:

1. Introduced a new general abstraction called `FileOperations`.
2. Implemented `read` and `write` for misc.
3. Updated the sample file for misc.

ManeraKai (3):
  rust: miscdevice: Moved `MiscDevice` to a more general abstraction
  rust: miscdevice: Implemented `read` and `write`
  samples: rust: Updated the example using the Rust MiscDevice
    abstraction

 rust/kernel/fs.rs                 |   1 +
 rust/kernel/fs/file_operations.rs | 109 +++++++++++
 rust/kernel/miscdevice.rs         | 178 +++++++++---------
 samples/rust/rust_misc_device.rs  | 289 ++++++++++++++++++++----------
 4 files changed, 391 insertions(+), 186 deletions(-)
 create mode 100644 rust/kernel/fs/file_operations.rs

-- 
2.43.0

[PATCH 1/3] rust: miscdevice: Moved `MiscDevice` to a more general abstraction

[ManeraKai]

This new general abstraction is called `FileOperations`.

`struct file_operations` is not only meant for misc. Its methods are
accessible from any other driver type. This change, however, doesn't
generalize the safe wrapping for all driver types, but rather just the
method declarations. The actual safe wrappings are left for every driver
type to implement. This may make each implementation simpler. For
example, misc can choose not to implement the safe wrapping for `lock`,
`sendfile`, or `sendpage`, since they have no use in misc drivers.

Signed-off-by: ManeraKai <manerakai@protonmail.com>
---
 rust/kernel/fs.rs                 |  1 +
 rust/kernel/fs/file_operations.rs | 91 +++++++++++++++++++++++++++++++
 rust/kernel/miscdevice.rs         | 86 ++---------------------------
 samples/rust/rust_misc_device.rs  |  6 +-
 4 files changed, 101 insertions(+), 83 deletions(-)
 create mode 100644 rust/kernel/fs/file_operations.rs

diff --git a/rust/kernel/fs.rs b/rust/kernel/fs.rs
index 0121b38c59e6..94519b41086b 100644
--- a/rust/kernel/fs.rs
+++ b/rust/kernel/fs.rs
@@ -5,4 +5,5 @@
 //! C headers: [`include/linux/fs.h`](srctree/include/linux/fs.h)
 
 pub mod file;
+pub mod file_operations;
 pub use self::file::{File, LocalFile};
diff --git a/rust/kernel/fs/file_operations.rs b/rust/kernel/fs/file_operations.rs
new file mode 100644
index 000000000000..aa60cd46a012
--- /dev/null
+++ b/rust/kernel/fs/file_operations.rs
@@ -0,0 +1,91 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Wrapper for struct file_operations.
+//!
+//! C headers: [`include/linux/fs.h`](srctree/include/linux/fs.h).
+
+use macros::vtable;
+
+#[cfg(CONFIG_COMPAT)]
+use crate::fs::File;
+use crate::{
+    build_error,
+    error::{Result, VTABLE_DEFAULT_ERROR},
+    miscdevice::MiscDeviceRegistration,
+    mm::virt::VmaNew,
+    seq_file::SeqFile,
+    types::ForeignOwnable,
+};
+
+/// Trait implemented by the private data of an open misc device.
+#[vtable]
+pub trait FileOperations: Sized {
+    /// What kind of pointer should `Self` be wrapped in.
+    type Ptr: ForeignOwnable + Send + Sync;
+
+    /// Called when the misc device is opened.
+    ///
+    /// The returned pointer will be stored as the private data for the file.
+    fn open(_file: &File, _misc: &MiscDeviceRegistration<Self>) -> Result<Self::Ptr>;
+
+    /// Called when the misc device is released.
+    fn release(device: Self::Ptr, _file: &File) {
+        drop(device);
+    }
+
+    /// Handle for mmap.
+    ///
+    /// This function is invoked when a user space process invokes the `mmap` system call on
+    /// `file`. The function is a callback that is part of the VMA initializer. The kernel will do
+    /// initial setup of the VMA before calling this function. The function can then interact with
+    /// the VMA initialization by calling methods of `vma`. If the function does not return an
+    /// error, the kernel will complete initialization of the VMA according to the properties of
+    /// `vma`.
+    fn mmap(
+        _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
+        _file: &File,
+        _vma: &VmaNew,
+    ) -> Result {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Handler for ioctls.
+    ///
+    /// The `cmd` argument is usually manipulated using the utilities in [`kernel::ioctl`].
+    ///
+    /// [`kernel::ioctl`]: mod@crate::ioctl
+    fn ioctl(
+        _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
+        _file: &File,
+        _cmd: u32,
+        _arg: usize,
+    ) -> Result<isize> {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Handler for ioctls.
+    ///
+    /// Used for 32-bit userspace on 64-bit platforms.
+    ///
+    /// This method is optional and only needs to be provided if the ioctl relies on structures
+    /// that have different layout on 32-bit and 64-bit userspace. If no implementation is
+    /// provided, then `compat_ptr_ioctl` will be used instead.
+    #[cfg(CONFIG_COMPAT)]
+    fn compat_ioctl(
+        _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
+        _file: &File,
+        _cmd: u32,
+        _arg: usize,
+    ) -> Result<isize> {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Show info for this fd.
+    fn show_fdinfo(
+        _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
+        _m: &SeqFile,
+        _file: &File,
+    ) {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+}
diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index 6373fe183b27..578f33383ce6 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -11,9 +11,9 @@
 use crate::{
     bindings,
     device::Device,
-    error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR},
+    error::{to_result, Error},
     ffi::{c_int, c_long, c_uint, c_ulong},
-    fs::File,
+    fs::{file_operations::FileOperations, File},
     mm::virt::VmaNew,
     prelude::*,
     seq_file::SeqFile,
@@ -30,7 +30,7 @@ pub struct MiscDeviceOptions {
 
 impl MiscDeviceOptions {
     /// Create a raw `struct miscdev` ready for registration.
-    pub const fn into_raw<T: MiscDevice>(self) -> bindings::miscdevice {
+    pub const fn into_raw<T: FileOperations>(self) -> bindings::miscdevice {
         // SAFETY: All zeros is valid for this C type.
         let mut result: bindings::miscdevice = unsafe { MaybeUninit::zeroed().assume_init() };
         result.minor = bindings::MISC_DYNAMIC_MINOR as ffi::c_int;
@@ -66,7 +66,7 @@ unsafe impl<T> Send for MiscDeviceRegistration<T> {}
 // parallel.
 unsafe impl<T> Sync for MiscDeviceRegistration<T> {}
 
-impl<T: MiscDevice> MiscDeviceRegistration<T> {
+impl<T: FileOperations> MiscDeviceRegistration<T> {
     /// Register a misc device.
     pub fn register(opts: MiscDeviceOptions) -> impl PinInit<Self, Error> {
         try_pin_init!(Self {
@@ -108,84 +108,10 @@ fn drop(self: Pin<&mut Self>) {
         unsafe { bindings::misc_deregister(self.inner.get()) };
     }
 }
-
-/// Trait implemented by the private data of an open misc device.
-#[vtable]
-pub trait MiscDevice: Sized {
-    /// What kind of pointer should `Self` be wrapped in.
-    type Ptr: ForeignOwnable + Send + Sync;
-
-    /// Called when the misc device is opened.
-    ///
-    /// The returned pointer will be stored as the private data for the file.
-    fn open(_file: &File, _misc: &MiscDeviceRegistration<Self>) -> Result<Self::Ptr>;
-
-    /// Called when the misc device is released.
-    fn release(device: Self::Ptr, _file: &File) {
-        drop(device);
-    }
-
-    /// Handle for mmap.
-    ///
-    /// This function is invoked when a user space process invokes the `mmap` system call on
-    /// `file`. The function is a callback that is part of the VMA initializer. The kernel will do
-    /// initial setup of the VMA before calling this function. The function can then interact with
-    /// the VMA initialization by calling methods of `vma`. If the function does not return an
-    /// error, the kernel will complete initialization of the VMA according to the properties of
-    /// `vma`.
-    fn mmap(
-        _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
-        _file: &File,
-        _vma: &VmaNew,
-    ) -> Result {
-        build_error!(VTABLE_DEFAULT_ERROR)
-    }
-
-    /// Handler for ioctls.
-    ///
-    /// The `cmd` argument is usually manipulated using the utilities in [`kernel::ioctl`].
-    ///
-    /// [`kernel::ioctl`]: mod@crate::ioctl
-    fn ioctl(
-        _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
-        _file: &File,
-        _cmd: u32,
-        _arg: usize,
-    ) -> Result<isize> {
-        build_error!(VTABLE_DEFAULT_ERROR)
-    }
-
-    /// Handler for ioctls.
-    ///
-    /// Used for 32-bit userspace on 64-bit platforms.
-    ///
-    /// This method is optional and only needs to be provided if the ioctl relies on structures
-    /// that have different layout on 32-bit and 64-bit userspace. If no implementation is
-    /// provided, then `compat_ptr_ioctl` will be used instead.
-    #[cfg(CONFIG_COMPAT)]
-    fn compat_ioctl(
-        _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
-        _file: &File,
-        _cmd: u32,
-        _arg: usize,
-    ) -> Result<isize> {
-        build_error!(VTABLE_DEFAULT_ERROR)
-    }
-
-    /// Show info for this fd.
-    fn show_fdinfo(
-        _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
-        _m: &SeqFile,
-        _file: &File,
-    ) {
-        build_error!(VTABLE_DEFAULT_ERROR)
-    }
-}
-
 /// A vtable for the file operations of a Rust miscdevice.
-struct MiscdeviceVTable<T: MiscDevice>(PhantomData<T>);
+struct MiscdeviceVTable<T: FileOperations>(PhantomData<T>);
 
-impl<T: MiscDevice> MiscdeviceVTable<T> {
+impl<T: FileOperations> MiscdeviceVTable<T> {
     /// # Safety
     ///
     /// `file` and `inode` must be the file and inode for a file that is undergoing initialization.
diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
index e7ab77448f75..d052294cebb8 100644
--- a/samples/rust/rust_misc_device.rs
+++ b/samples/rust/rust_misc_device.rs
@@ -100,9 +100,9 @@
 use kernel::{
     c_str,
     device::Device,
-    fs::File,
+    fs::{file_operations::FileOperations, File},
     ioctl::{_IO, _IOC_SIZE, _IOR, _IOW},
-    miscdevice::{MiscDevice, MiscDeviceOptions, MiscDeviceRegistration},
+    miscdevice::{MiscDeviceOptions, MiscDeviceRegistration},
     new_mutex,
     prelude::*,
     sync::Mutex,
@@ -154,7 +154,7 @@ struct RustMiscDevice {
 }
 
 #[vtable]
-impl MiscDevice for RustMiscDevice {
+impl FileOperations for RustMiscDevice {
     type Ptr = Pin<KBox<Self>>;
 
     fn open(_file: &File, misc: &MiscDeviceRegistration<Self>) -> Result<Pin<KBox<Self>>> {
-- 
2.43.0

[PATCH 2/3] rust: miscdevice: Implemented `read` and `write`

[ManeraKai]

Added the general declaration in `FileOperations`. And implemented the
safe wrapping for misc.

Note: Renamed some raw pointer variables to `raw_<name>`. I'm not sure
this way of naming is good or not. I would like your opinion.

Signed-off-by: ManeraKai <manerakai@protonmail.com>
---
 rust/kernel/fs/file_operations.rs |  20 +++++-
 rust/kernel/miscdevice.rs         | 108 +++++++++++++++++++++++-------
 2 files changed, 104 insertions(+), 24 deletions(-)

diff --git a/rust/kernel/fs/file_operations.rs b/rust/kernel/fs/file_operations.rs
index aa60cd46a012..b21a2bae4803 100644
--- a/rust/kernel/fs/file_operations.rs
+++ b/rust/kernel/fs/file_operations.rs
@@ -14,7 +14,7 @@
     miscdevice::MiscDeviceRegistration,
     mm::virt::VmaNew,
     seq_file::SeqFile,
-    types::ForeignOwnable,
+    types::ForeignOwnable, uaccess::{UserSliceReader, UserSliceWriter},
 };
 
 /// Trait implemented by the private data of an open misc device.
@@ -23,6 +23,24 @@ pub trait FileOperations: Sized {
     /// What kind of pointer should `Self` be wrapped in.
     type Ptr: ForeignOwnable + Send + Sync;
 
+    /// Handler for read.
+    fn read(
+        _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
+        _buf: UserSliceWriter,
+        _offset: &mut i64,
+    ) -> Result<i64> {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
+    /// Handler for write.
+    fn write(
+        _device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
+        mut _buf: UserSliceReader,
+        _offset: &mut i64,
+    ) -> Result<i64> {
+        build_error!(VTABLE_DEFAULT_ERROR)
+    }
+
     /// Called when the misc device is opened.
     ///
     /// The returned pointer will be stored as the private data for the file.
diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index 578f33383ce6..f4b6388a3742 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -8,6 +8,8 @@
 //!
 //! Reference: <https://www.kernel.org/doc/html/latest/driver-api/misc_devices.html>
 
+use bindings::loff_t;
+
 use crate::{
     bindings,
     device::Device,
@@ -17,7 +19,7 @@
     mm::virt::VmaNew,
     prelude::*,
     seq_file::SeqFile,
-    types::{ForeignOwnable, Opaque},
+    types::{ForeignOwnable, Opaque}, uaccess::UserSlice,
 };
 use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin};
 
@@ -112,6 +114,54 @@ fn drop(self: Pin<&mut Self>) {
 struct MiscdeviceVTable<T: FileOperations>(PhantomData<T>);
 
 impl<T: FileOperations> MiscdeviceVTable<T> {
+    unsafe extern "C" fn read(
+        raw_file: *mut bindings::file,
+        raw_buf: *mut ffi::c_char,
+        size: usize,
+        raw_offset: *mut loff_t,
+    ) -> c_long {
+        // SAFETY: The read call of a file can access file and its private_data.
+        let raw_device = unsafe { (*raw_file).private_data };
+
+        // SAFETY: The read call of a file can borrow the private_data of the file.
+        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(raw_device) };
+
+        let user_slice = UserSlice::new(UserPtr::from_ptr(raw_buf as *mut c_void), size);
+        let user_slice_writer = user_slice.writer();
+
+        // SAFETY: The read call of a file can access and modify the offset pointer value.
+        let offset = unsafe { &mut *raw_offset };
+
+        match T::read(device, user_slice_writer, offset) {
+            Ok(ret) => ret as c_long,
+            Err(err) => err.to_errno() as c_long,
+        }
+    }
+
+    unsafe extern "C" fn write(
+        raw_file: *mut bindings::file,
+        raw_buf: *const ffi::c_char,
+        size: usize,
+        raw_offset: *mut loff_t,
+    ) -> c_long {
+        // SAFETY: The read call of a file can access file and its private_data.
+        let raw_device = unsafe { (*raw_file).private_data };
+
+        // SAFETY: The read call of a file can borrow the private_data of the file.
+        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(raw_device) };
+
+        let user_slice = UserSlice::new(UserPtr::from_ptr(raw_buf as *mut c_void), size);
+        let user_slice_reader = user_slice.reader();
+
+        // SAFETY: The read call of a file can access and modify the offset pointer value.
+        let offset = unsafe { &mut *raw_offset };
+
+        match T::write(device, user_slice_reader, offset) {
+            Ok(ret) => ret as c_long,
+            Err(err) => err.to_errno() as c_long,
+        }
+    }
+
     /// # Safety
     ///
     /// `file` and `inode` must be the file and inode for a file that is undergoing initialization.
@@ -124,13 +174,13 @@ impl<T: FileOperations> MiscdeviceVTable<T> {
         }
 
         // SAFETY: The open call of a file can access the private data.
-        let misc_ptr = unsafe { (*raw_file).private_data };
+        let raw_misc = unsafe { (*raw_file).private_data };
 
         // SAFETY: This is a miscdevice, so `misc_open()` set the private data to a pointer to the
         // associated `struct miscdevice` before calling into this method. Furthermore,
         // `misc_open()` ensures that the miscdevice can't be unregistered and freed during this
         // call to `fops_open`.
-        let misc = unsafe { &*misc_ptr.cast::<MiscDeviceRegistration<T>>() };
+        let misc = unsafe { &*raw_misc.cast::<MiscDeviceRegistration<T>>() };
 
         // SAFETY:
         // * This underlying file is valid for (much longer than) the duration of `T::open`.
@@ -157,16 +207,19 @@ impl<T: FileOperations> MiscdeviceVTable<T> {
     ///
     /// `file` and `inode` must be the file and inode for a file that is being released. The file
     /// must be associated with a `MiscDeviceRegistration<T>`.
-    unsafe extern "C" fn release(_inode: *mut bindings::inode, file: *mut bindings::file) -> c_int {
+    unsafe extern "C" fn release(
+        _inode: *mut bindings::inode,
+        raw_file: *mut bindings::file,
+    ) -> c_int {
         // SAFETY: The release call of a file owns the private data.
-        let private = unsafe { (*file).private_data };
+        let raw_device = unsafe { (*raw_file).private_data };
         // SAFETY: The release call of a file owns the private data.
-        let ptr = unsafe { <T::Ptr as ForeignOwnable>::from_foreign(private) };
+        let device = unsafe { <T::Ptr as ForeignOwnable>::from_foreign(raw_device) };
 
         // SAFETY:
         // * The file is valid for the duration of this call.
         // * There is no active fdget_pos region on the file on this thread.
-        T::release(ptr, unsafe { File::from_raw_file(file) });
+        T::release(device, unsafe { File::from_raw_file(raw_file) });
 
         0
     }
@@ -176,21 +229,21 @@ impl<T: FileOperations> MiscdeviceVTable<T> {
     /// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
     /// `vma` must be a vma that is currently being mmap'ed with this file.
     unsafe extern "C" fn mmap(
-        file: *mut bindings::file,
+        raw_file: *mut bindings::file,
         vma: *mut bindings::vm_area_struct,
     ) -> c_int {
         // SAFETY: The mmap call of a file can access the private data.
-        let private = unsafe { (*file).private_data };
+        let raw_device = unsafe { (*raw_file).private_data };
         // SAFETY: This is a Rust Miscdevice, so we call `into_foreign` in `open` and
         // `from_foreign` in `release`, and `fops_mmap` is guaranteed to be called between those
         // two operations.
-        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private.cast()) };
+        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(raw_device) };
         // SAFETY: The caller provides a vma that is undergoing initial VMA setup.
         let area = unsafe { VmaNew::from_raw(vma) };
         // SAFETY:
         // * The file is valid for the duration of this call.
         // * There is no active fdget_pos region on the file on this thread.
-        let file = unsafe { File::from_raw_file(file) };
+        let file = unsafe { File::from_raw_file(raw_file) };
 
         match T::mmap(device, file, area) {
             Ok(()) => 0,
@@ -201,16 +254,16 @@ impl<T: FileOperations> MiscdeviceVTable<T> {
     /// # Safety
     ///
     /// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
-    unsafe extern "C" fn ioctl(file: *mut bindings::file, cmd: c_uint, arg: c_ulong) -> c_long {
+    unsafe extern "C" fn ioctl(raw_file: *mut bindings::file, cmd: c_uint, arg: c_ulong) -> c_long {
         // SAFETY: The ioctl call of a file can access the private data.
-        let private = unsafe { (*file).private_data };
+        let raw_device = unsafe { (*raw_file).private_data };
         // SAFETY: Ioctl calls can borrow the private data of the file.
-        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
+        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(raw_device) };
 
         // SAFETY:
         // * The file is valid for the duration of this call.
         // * There is no active fdget_pos region on the file on this thread.
-        let file = unsafe { File::from_raw_file(file) };
+        let file = unsafe { File::from_raw_file(raw_file) };
 
         match T::ioctl(device, file, cmd, arg) {
             Ok(ret) => ret as c_long,
@@ -223,19 +276,19 @@ impl<T: FileOperations> MiscdeviceVTable<T> {
     /// `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
     #[cfg(CONFIG_COMPAT)]
     unsafe extern "C" fn compat_ioctl(
-        file: *mut bindings::file,
+        raw_file: *mut bindings::file,
         cmd: c_uint,
         arg: c_ulong,
     ) -> c_long {
         // SAFETY: The compat ioctl call of a file can access the private data.
-        let private = unsafe { (*file).private_data };
+        let raw_device = unsafe { (*raw_file).private_data };
         // SAFETY: Ioctl calls can borrow the private data of the file.
-        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
+        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(raw_device) };
 
         // SAFETY:
         // * The file is valid for the duration of this call.
         // * There is no active fdget_pos region on the file on this thread.
-        let file = unsafe { File::from_raw_file(file) };
+        let file = unsafe { File::from_raw_file(raw_file) };
 
         match T::compat_ioctl(device, file, cmd, arg) {
             Ok(ret) => ret as c_long,
@@ -247,15 +300,18 @@ impl<T: FileOperations> MiscdeviceVTable<T> {
     ///
     /// - `file` must be a valid file that is associated with a `MiscDeviceRegistration<T>`.
     /// - `seq_file` must be a valid `struct seq_file` that we can write to.
-    unsafe extern "C" fn show_fdinfo(seq_file: *mut bindings::seq_file, file: *mut bindings::file) {
+    unsafe extern "C" fn show_fdinfo(
+        seq_file: *mut bindings::seq_file,
+        raw_file: *mut bindings::file,
+    ) {
         // SAFETY: The release call of a file owns the private data.
-        let private = unsafe { (*file).private_data };
+        let raw_device = unsafe { (*raw_file).private_data };
         // SAFETY: Ioctl calls can borrow the private data of the file.
-        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(private) };
+        let device = unsafe { <T::Ptr as ForeignOwnable>::borrow(raw_device) };
         // SAFETY:
         // * The file is valid for the duration of this call.
         // * There is no active fdget_pos region on the file on this thread.
-        let file = unsafe { File::from_raw_file(file) };
+        let file = unsafe { File::from_raw_file(raw_file) };
         // SAFETY: The caller ensures that the pointer is valid and exclusive for the duration in
         // which this method is called.
         let m = unsafe { SeqFile::from_raw(seq_file) };
@@ -264,6 +320,12 @@ impl<T: FileOperations> MiscdeviceVTable<T> {
     }
 
     const VTABLE: bindings::file_operations = bindings::file_operations {
+        read: if T::HAS_READ { Some(Self::read) } else { None },
+        write: if T::HAS_WRITE {
+            Some(Self::write)
+        } else {
+            None
+        },
         open: Some(Self::open),
         release: Some(Self::release),
         mmap: if T::HAS_MMAP { Some(Self::mmap) } else { None },
-- 
2.43.0

[PATCH 3/3] samples: rust: Updated the example using the Rust MiscDevice abstraction

[ManeraKai]

This sample driver will now:
- use the new general `FileOperations` abstraction.
- have `read` and `write` methods, that use a persistent kernel buffer to
store data.

Signed-off-by: ManeraKai <manerakai@protonmail.com>
---
 samples/rust/rust_misc_device.rs | 283 +++++++++++++++++++++----------
 1 file changed, 195 insertions(+), 88 deletions(-)

diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
index d052294cebb8..c8e90eb9b9ad 100644
--- a/samples/rust/rust_misc_device.rs
+++ b/samples/rust/rust_misc_device.rs
@@ -7,92 +7,139 @@
 //! Below is an example userspace C program that exercises this sample's functionality.
 //!
 //! ```c
-//! #include <stdio.h>
-//! #include <stdlib.h>
-//! #include <errno.h>
-//! #include <fcntl.h>
-//! #include <unistd.h>
-//! #include <sys/ioctl.h>
-//!
-//! #define RUST_MISC_DEV_FAIL _IO('|', 0)
-//! #define RUST_MISC_DEV_HELLO _IO('|', 0x80)
-//! #define RUST_MISC_DEV_GET_VALUE _IOR('|', 0x81, int)
-//! #define RUST_MISC_DEV_SET_VALUE _IOW('|', 0x82, int)
-//!
-//! int main() {
-//!   int value, new_value;
-//!   int fd, ret;
-//!
-//!   // Open the device file
-//!   printf("Opening /dev/rust-misc-device for reading and writing\n");
-//!   fd = open("/dev/rust-misc-device", O_RDWR);
-//!   if (fd < 0) {
-//!     perror("open");
-//!     return errno;
-//!   }
-//!
-//!   // Make call into driver to say "hello"
-//!   printf("Calling Hello\n");
-//!   ret = ioctl(fd, RUST_MISC_DEV_HELLO, NULL);
-//!   if (ret < 0) {
-//!     perror("ioctl: Failed to call into Hello");
-//!     close(fd);
-//!     return errno;
-//!   }
-//!
-//!   // Get initial value
-//!   printf("Fetching initial value\n");
-//!   ret = ioctl(fd, RUST_MISC_DEV_GET_VALUE, &value);
-//!   if (ret < 0) {
-//!     perror("ioctl: Failed to fetch the initial value");
-//!     close(fd);
-//!     return errno;
-//!   }
-//!
-//!   value++;
-//!
-//!   // Set value to something different
-//!   printf("Submitting new value (%d)\n", value);
-//!   ret = ioctl(fd, RUST_MISC_DEV_SET_VALUE, &value);
-//!   if (ret < 0) {
-//!     perror("ioctl: Failed to submit new value");
-//!     close(fd);
-//!     return errno;
-//!   }
-//!
-//!   // Ensure new value was applied
-//!   printf("Fetching new value\n");
-//!   ret = ioctl(fd, RUST_MISC_DEV_GET_VALUE, &new_value);
-//!   if (ret < 0) {
-//!     perror("ioctl: Failed to fetch the new value");
-//!     close(fd);
-//!     return errno;
-//!   }
-//!
-//!   if (value != new_value) {
-//!     printf("Failed: Committed and retrieved values are different (%d - %d)\n", value, new_value);
-//!     close(fd);
-//!     return -1;
-//!   }
-//!
-//!   // Call the unsuccessful ioctl
-//!   printf("Attempting to call in to an non-existent IOCTL\n");
-//!   ret = ioctl(fd, RUST_MISC_DEV_FAIL, NULL);
-//!   if (ret < 0) {
-//!     perror("ioctl: Succeeded to fail - this was expected");
-//!   } else {
-//!     printf("ioctl: Failed to fail\n");
-//!     close(fd);
-//!     return -1;
-//!   }
-//!
-//!   // Close the device file
-//!   printf("Closing /dev/rust-misc-device\n");
-//!   close(fd);
-//!
-//!   printf("Success\n");
-//!   return 0;
-//! }
+//!#include <errno.h>
+//!#include <fcntl.h>
+//!#include <stdint.h>
+//!#include <stdio.h>
+//!#include <stdlib.h>
+//!#include <sys/ioctl.h>
+//!#include <unistd.h>
+//!
+//!#define RUST_MISC_DEV_FAIL _IO('|', 0)
+//!#define RUST_MISC_DEV_HELLO _IO('|', 0x80)
+//!#define RUST_MISC_DEV_GET_VALUE _IOR('|', 0x81, int)
+//!#define RUST_MISC_DEV_SET_VALUE _IOW('|', 0x82, int)
+//!
+//!int main() {
+//!  int value, new_value;
+//!  int fd, ret;
+//!
+//!  // Open the device file
+//!  printf("Opening /dev/rust-misc-device for reading and writing\n");
+//!  fd = open("/dev/rust-misc-device", O_RDWR);
+//!  if (fd < 0) {
+//!    perror("open");
+//!    return errno;
+//!  }
+//!
+//!  // Make call into driver to say "hello"
+//!  printf("Calling Hello\n");
+//!  ret = ioctl(fd, RUST_MISC_DEV_HELLO, NULL);
+//!  if (ret < 0) {
+//!    perror("ioctl: Failed to call into Hello");
+//!    close(fd);
+//!    return errno;
+//!  }
+//!
+//!  // Get initial value
+//!  printf("Fetching initial value\n");
+//!  ret = ioctl(fd, RUST_MISC_DEV_GET_VALUE, &value);
+//!  if (ret < 0) {
+//!    perror("ioctl: Failed to fetch the initial value");
+//!    close(fd);
+//!    return errno;
+//!  }
+//!
+//!  value++;
+//!
+//!  // Set value to something different
+//!  printf("Submitting new value (%d)\n", value);
+//!  ret = ioctl(fd, RUST_MISC_DEV_SET_VALUE, &value);
+//!  if (ret < 0) {
+//!    perror("ioctl: Failed to submit new value");
+//!    close(fd);
+//!    return errno;
+//!  }
+//!
+//!  // Ensure new value was applied
+//!  printf("Fetching new value\n");
+//!  ret = ioctl(fd, RUST_MISC_DEV_GET_VALUE, &new_value);
+//!  if (ret < 0) {
+//!    perror("ioctl: Failed to fetch the new value");
+//!    close(fd);
+//!    return errno;
+//!  }
+//!
+//!  if (value != new_value) {
+//!    printf("Failed: Committed and retrieved values are different (%d -%d)\n",
+//!           value, new_value);
+//!    close(fd);
+//!    return -1;
+//!  }
+//!
+//!  // Call the unsuccessful ioctl
+//!  printf("Attempting to call in to an non-existent IOCTL\n");
+//!  ret = ioctl(fd, RUST_MISC_DEV_FAIL, NULL);
+//!  if (ret < 0) {
+//!    perror("ioctl: Succeeded to fail - this was expected");
+//!  } else {
+//!    printf("ioctl: Failed to fail\n");
+//!    close(fd);
+//!    return -1;
+//!  }
+//!
+//!  ssize_t bytes_written;
+//!
+//!  // Write
+//!  char str1[] = {'H', 'e', 'l', 'l', 'o', '\0'};
+//!  printf("Attempting to write to the file\n");
+//!  bytes_written = write(fd, str1, sizeof(str1) * sizeof(uint8_t));
+//!  if (bytes_written < 0) {
+//!    perror("Error writing to file");
+//!    close(fd);
+//!    return 1;
+//!  }
+//!
+//!  // Write with a custom offset
+//!  printf("Attempting to write to the file with a custom offset\n");
+//!  char str2[] = {'i', ',', ' ', 'w', 'o', 'r', 'l', 'd', '!', '\0'};
+//!  // bytes_written = write(fd, str2, sizeof(str2) * sizeof(uint8_t));
+//!  bytes_written = pwrite(fd, str2, sizeof(str2), 1);
+//!  if (bytes_written < 0) {
+//!    perror("Error writing to file 2");
+//!    close(fd);
+//!    return 1;
+//!  }
+//!
+//!  // Read
+//!  printf("Attempting to read from the file\n");
+//!  char buffer1[20];
+//!  ssize_t readCount1 = pread(fd, buffer1, sizeof(buffer1), 0);
+//!  if (readCount1 < 0) {
+//!    perror("Error reading from file\n");
+//!    close(fd);
+//!    return errno;
+//!  }
+//!  printf("Data: \"%s\n\"", buffer1);
+//!
+//!  // Read with a custom offset
+//!  printf("Attempting to read from the file with a custom offset\n");
+//!  char buffer2[20];
+//!  ssize_t readCount2 = pread(fd, buffer2, sizeof(buffer2), 4);
+//!  if (readCount2 < 0) {
+//!    perror("Error reading from file\n");
+//!    close(fd);
+//!    return errno;
+//!  }
+//!  printf("Data: \"%s\n\"", buffer2);
+//!
+//!  // Close the device file
+//!  printf("Closing /dev/rust-misc-device\n");
+//!  close(fd);
+//!
+//!  printf("Success\n");
+//!  return 0;
+//!}
 //! ```
 
 use core::pin::Pin;
@@ -101,12 +148,13 @@
     c_str,
     device::Device,
     fs::{file_operations::FileOperations, File},
+    global_lock,
     ioctl::{_IO, _IOC_SIZE, _IOR, _IOW},
     miscdevice::{MiscDeviceOptions, MiscDeviceRegistration},
     new_mutex,
     prelude::*,
     sync::Mutex,
-    types::ARef,
+    types::{ARef, ForeignOwnable},
     uaccess::{UserSlice, UserSliceReader, UserSliceWriter},
 };
 
@@ -128,6 +176,11 @@ struct RustMiscDeviceModule {
     _miscdev: MiscDeviceRegistration<RustMiscDevice>,
 }
 
+global_lock! {
+    // SAFETY: Initialized in module initializer before first use.
+    unsafe(uninit) static DATA: Mutex<KVec<u8>> = KVec::new();
+}
+
 impl kernel::InPlaceModule for RustMiscDeviceModule {
     fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
         pr_info!("Initialising Rust Misc Device Sample\n");
@@ -136,6 +189,9 @@ fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
             name: c_str!("rust-misc-device"),
         };
 
+        // SAFETY: Called exactly once.
+        unsafe { DATA.init() };
+
         try_pin_init!(Self {
             _miscdev <- MiscDeviceRegistration::register(options),
         })
@@ -185,13 +241,64 @@ fn ioctl(me: Pin<&RustMiscDevice>, _file: &File, cmd: u32, arg: usize) -> Result
             RUST_MISC_DEV_SET_VALUE => me.set_value(UserSlice::new(arg, size).reader())?,
             RUST_MISC_DEV_HELLO => me.hello()?,
             _ => {
-                dev_err!(me.dev, "-> IOCTL not recognised: {}\n", cmd);
+                dev_warn!(me.dev, "-> IOCTL not recognised: {}\n", cmd);
                 return Err(ENOTTY);
             }
         };
 
         Ok(0)
     }
+
+    fn read(
+        device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
+        mut buf: UserSliceWriter,
+        offset: &mut i64,
+    ) -> Result<i64> {
+        dev_info!(device.dev, "read() called\n");
+
+        let data = DATA.lock();
+
+        if *offset >= data.len() as i64 {
+            return Ok(0);
+        }
+
+        let slice = &data[(*offset as usize)..];
+
+        dev_info!(device.dev, "Writing to user: {:?}\n", slice);
+
+        buf.write_slice(slice)?;
+        *offset += slice.len() as i64;
+
+        Ok(data.len() as i64)
+    }
+
+    fn write(
+        device: <Self::Ptr as ForeignOwnable>::Borrowed<'_>,
+        buf: UserSliceReader,
+        offset: &mut i64,
+    ) -> Result<i64> {
+        dev_info!(device.dev, "write() called\n");
+
+        let mut data = DATA.lock();
+
+        let mut tmp = KVec::with_capacity(buf.len(), GFP_KERNEL)?;
+        buf.read_all(&mut tmp, GFP_KERNEL)?;
+
+        for (i, val) in tmp.iter().enumerate() {
+            let idx = *offset as usize + i;
+            if idx < data.len() {
+                data[idx] = *val;
+            } else {
+                data.push(*val, GFP_KERNEL)?;
+            }
+        }
+
+        *offset += tmp.len() as i64;
+
+        dev_info!(device.dev, "Reading from user: {:?}\n", tmp);
+
+        Ok(*offset)
+    }
 }
 
 #[pinned_drop]
-- 
2.43.0

Re: [PATCH 2/3] rust: miscdevice: Implemented `read` and `write`

[gregkh]

On Thu, Sep 18, 2025 at 02:45:36PM +0000, ManeraKai wrote:
> Added the general declaration in `FileOperations`. And implemented the
> safe wrapping for misc.

While read/write is "traditional", perhaps instead you should just
export read_iter and write_iter as that is the proper way forward for
drivers to be using.

thanks,

greg k-h

Re: [PATCH 1/3] rust: miscdevice: Moved `MiscDevice` to a more general abstraction

[gregkh]

On Thu, Sep 18, 2025 at 02:45:28PM +0000, ManeraKai wrote:
> This new general abstraction is called `FileOperations`.
> 
> `struct file_operations` is not only meant for misc. Its methods are
> accessible from any other driver type. This change, however, doesn't
> generalize the safe wrapping for all driver types, but rather just the
> method declarations. The actual safe wrappings are left for every driver
> type to implement. This may make each implementation simpler. For
> example, misc can choose not to implement the safe wrapping for `lock`,
> `sendfile`, or `sendpage`, since they have no use in misc drivers.

This has come up many times, but we really are not "ready" for a generic
file operations export.  Let's keep this just for misc for now, until we
have another use for it, as the interaction with the vfs is tricky and
subtle and full of corner cases (see the debugfs bindings for lots of
examples here.)

So for a misc device, let's just stick with what we have for now.

> Signed-off-by: ManeraKai <manerakai@protonmail.com>

Nit, we need a full name please.

thanks,

greg k-h

Re: [PATCH 3/3] samples: rust: Updated the example using the Rust MiscDevice abstraction

[gregkh]

On Thu, Sep 18, 2025 at 02:45:44PM +0000, ManeraKai wrote:
> This sample driver will now:
> - use the new general `FileOperations` abstraction.
> - have `read` and `write` methods, that use a persistent kernel buffer to
> store data.
> 
> Signed-off-by: ManeraKai <manerakai@protonmail.com>
> ---
>  samples/rust/rust_misc_device.rs | 283 +++++++++++++++++++++----------
>  1 file changed, 195 insertions(+), 88 deletions(-)
> 
> diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
> index d052294cebb8..c8e90eb9b9ad 100644
> --- a/samples/rust/rust_misc_device.rs
> +++ b/samples/rust/rust_misc_device.rs
> @@ -7,92 +7,139 @@
>  //! Below is an example userspace C program that exercises this sample's functionality.
>  //!
>  //! ```c
> -//! #include <stdio.h>

You seem to have dropped the leading ' ' character, which makes this
diff almost impossible to review :(

Please fix that up and only change the modified files, not all of them,
in the example.

thanks,

greg k-h

Re: [PATCH 2/3] rust: miscdevice: Implemented `read` and `write`

[gregkh]

On Thu, Sep 18, 2025 at 04:59:01PM +0200, gregkh@linuxfoundation.org wrote:
> On Thu, Sep 18, 2025 at 02:45:36PM +0000, ManeraKai wrote:
> > Added the general declaration in `FileOperations`. And implemented the
> > safe wrapping for misc.
> 
> While read/write is "traditional", perhaps instead you should just
> export read_iter and write_iter as that is the proper way forward for
> drivers to be using.

Wait, maybe not.  The _iter stuff is "hard" for misc devices, and I
don't think that any of the current ones use that, so nevermind, a
simple read/write should be all that we need here.

What type of real misc driver do you want to write in rust that you need
a read/write callback and not just an ioctl one?

thanks,

greg k-h

Re: [PATCH 2/3] rust: miscdevice: Implemented `read` and `write`

[Alice Ryhl]

On Thu, Sep 18, 2025 at 4:45 PM ManeraKai <manerakai@protonmail.com> wrote:
>
> Added the general declaration in `FileOperations`. And implemented the
> safe wrapping for misc.
>
> Note: Renamed some raw pointer variables to `raw_<name>`. I'm not sure
> this way of naming is good or not. I would like your opinion.
>
> Signed-off-by: ManeraKai <manerakai@protonmail.com>

We already merged read_iter / write_iter functions for miscdevice this cycle.

Alice

Re: [PATCH 2/3] rust: miscdevice: Implemented `read` and `write`

[ManeraKai]

 > What type of real misc driver do you want to write in rust that you
 > need a read/write callback and not just an ioctl one?

I don't have one in mind. I was reading your book and experimenting with 
char drivers. However, I found out that you prefer new developers to use 
misc instead:
https://lore.kernel.org/rust-for-linux/2024101256-amplifier-joylessly-6ca9@gregkh/

So, I wanted to safe-wrap misc for new Rust developers (me included). 
And go from there to write real drivers.

Re: [PATCH 2/3] rust: miscdevice: Implemented `read` and `write`

[ManeraKai]

 > We already merged read_iter / write_iter functions for miscdevice this
 > cycle.

I couldn't find it. Can you send me a link please?

Re: [PATCH 2/3] rust: miscdevice: Implemented `read` and `write`

[Alice Ryhl]

On Thu, Sep 18, 2025 at 7:22 PM ManeraKai <manerakai@protonmail.com> wrote:
>
>  > We already merged read_iter / write_iter functions for miscdevice this
>  > cycle.
>
> I couldn't find it. Can you send me a link please?

https://lore.kernel.org/rust-for-linux/20250822-iov-iter-v5-0-6ce4819c2977@google.com/