diff --git a/drivers/android/process.rs b/drivers/android/process.rs index ce3ab176b8acec..3c8eec74312f7d 100644 --- a/drivers/android/process.rs +++ b/drivers/android/process.rs @@ -5,7 +5,7 @@ use kernel::{ bindings, c_types, cred::Credential, file::File, - file_operations::{FileOpener, FileOperations, IoctlCommand, IoctlHandler, PollTable}, + file_operations::{FileOperations, IoctlCommand, IoctlHandler, PollTable}, io_buffer::{IoBufferReader, IoBufferWriter}, linked_list::List, pages::Pages, @@ -806,17 +806,16 @@ impl IoctlHandler for Process { } } -impl FileOpener> for Process { - fn open(ctx: &Ref, file: &File) -> Result { - Self::new(ctx.clone(), file.cred().clone()) - } -} - impl FileOperations for Process { type Wrapper = Ref; + type OpenData = Ref; kernel::declare_file_operations!(ioctl, compat_ioctl, mmap, poll); + fn open(ctx: &Ref, file: &File) -> Result { + Self::new(ctx.clone(), file.cred().clone()) + } + fn release(obj: Self::Wrapper, _file: &File) { // Mark this process as dead. We'll do the same for the threads later. obj.inner.lock().is_dead = true; diff --git a/drivers/android/rust_binder.rs b/drivers/android/rust_binder.rs index 2a9636e94d94b4..9152790318feab 100644 --- a/drivers/android/rust_binder.rs +++ b/drivers/android/rust_binder.rs @@ -102,13 +102,13 @@ const fn ptr_align(value: usize) -> usize { unsafe impl Sync for BinderModule {} struct BinderModule { - _reg: Pin>>>, + _reg: Pin>>, } impl KernelModule for BinderModule { fn init(name: &'static CStr, _module: &'static kernel::ThisModule) -> Result { let ctx = Context::new()?; - let reg = Registration::new_pinned::(name, None, ctx)?; + let reg = Registration::new_pinned(name, None, ctx)?; Ok(Self { _reg: reg }) } } diff --git a/drivers/char/hw_random/bcm2835_rng_rust.rs b/drivers/char/hw_random/bcm2835_rng_rust.rs index 2d7b81ed67d24f..0f1461789631ed 100644 --- a/drivers/char/hw_random/bcm2835_rng_rust.rs +++ b/drivers/char/hw_random/bcm2835_rng_rust.rs @@ -18,12 +18,15 @@ module! { license: b"GPL v2", } -#[derive(Default)] struct RngDevice; impl FileOperations for RngDevice { kernel::declare_file_operations!(read); + fn open(_open_data: &(), _file: &File) -> Result { + Ok(Box::try_new(RngDevice)?) + } + fn read(_: &Self, _: &File, data: &mut impl IoBufferWriter, offset: u64) -> Result { // Succeed if the caller doesn't provide a buffer or if not at the start. if data.is_empty() || offset != 0 { @@ -38,12 +41,11 @@ impl FileOperations for RngDevice { struct RngDriver; impl PlatformDriver for RngDriver { - type DrvData = Pin>>; + type DrvData = Pin>>; fn probe(device_id: i32) -> Result { pr_info!("probing discovered hwrng with id {}\n", device_id); - let drv_data = - miscdev::Registration::new_pinned::(c_str!("rust_hwrng"), None, ())?; + let drv_data = miscdev::Registration::new_pinned(c_str!("rust_hwrng"), None, ())?; Ok(drv_data) } diff --git a/rust/kernel/chrdev.rs b/rust/kernel/chrdev.rs index 20e93ec05def3c..6dd74424218327 100644 --- a/rust/kernel/chrdev.rs +++ b/rust/kernel/chrdev.rs @@ -134,7 +134,9 @@ impl Registration<{ N }> { /// Registers a character device. /// /// You may call this once per device type, up to `N` times. - pub fn register>(self: Pin<&mut Self>) -> Result { + pub fn register>( + self: Pin<&mut Self>, + ) -> Result { // SAFETY: We must ensure that we never move out of `this`. let this = unsafe { self.get_unchecked_mut() }; if this.inner.is_none() { @@ -177,13 +179,8 @@ impl Registration<{ N }> { } } -impl file_operations::FileOpenAdapter for Registration<{ N }> { - type Arg = (); - - unsafe fn convert( - _inode: *mut bindings::inode, - _file: *mut bindings::file, - ) -> *const Self::Arg { +impl file_operations::FileOpenAdapter<()> for Registration<{ N }> { + unsafe fn convert(_inode: *mut bindings::inode, _file: *mut bindings::file) -> *const () { // TODO: Update the SAFETY comment on the call to `FileOperationsVTable::build` above once // this is updated to retrieve state. &() diff --git a/rust/kernel/file_operations.rs b/rust/kernel/file_operations.rs index 3d96a32a3764b1..edab1f7fa05414 100644 --- a/rust/kernel/file_operations.rs +++ b/rust/kernel/file_operations.rs @@ -86,7 +86,7 @@ pub enum SeekFrom { /// /// The returned value of `A::convert` must be a valid non-null pointer and /// `T:open` must return a valid non-null pointer on an `Ok` result. -unsafe extern "C" fn open_callback>( +unsafe extern "C" fn open_callback, T: FileOperations>( inode: *mut bindings::inode, file: *mut bindings::file, ) -> c_types::c_int { @@ -312,7 +312,7 @@ unsafe extern "C" fn poll_callback( pub(crate) struct FileOperationsVtable(marker::PhantomData, marker::PhantomData); -impl> FileOperationsVtable { +impl, T: FileOperations> FileOperationsVtable { const VTABLE: bindings::file_operations = bindings::file_operations { open: Some(open_callback::), release: Some(release_callback::), @@ -568,10 +568,7 @@ impl IoctlCommand { /// Trait for extracting file open arguments from kernel data structures. /// /// This is meant to be implemented by registration managers. -pub trait FileOpenAdapter { - /// The type of argument this adapter extracts. - type Arg; - +pub trait FileOpenAdapter { /// Converts untyped data stored in [`struct inode`] and [`struct file`] (when [`struct /// file_operations::open`] is called) into the given type. For example, for `miscdev` /// devices, a pointer to the registered [`struct miscdev`] is stored in [`struct @@ -582,27 +579,7 @@ pub trait FileOpenAdapter { /// This function must be called only when [`struct file_operations::open`] is being called for /// a file that was registered by the implementer. The returned pointer must be valid and /// not-null. - unsafe fn convert(_inode: *mut bindings::inode, _file: *mut bindings::file) - -> *const Self::Arg; -} - -/// Trait for implementers of kernel files. -/// -/// In addition to the methods in [`FileOperations`], implementers must also provide -/// [`FileOpener::open`] with a customised argument. This allows a single implementation of -/// [`FileOperations`] to be used for different types of registrations, for example, `miscdev` and -/// `chrdev`. -pub trait FileOpener: FileOperations { - /// Creates a new instance of this file. - /// - /// Corresponds to the `open` function pointer in `struct file_operations`. - fn open(context: &T, file: &File) -> Result; -} - -impl> + Default> FileOpener<()> for T { - fn open(_: &(), _file: &File) -> Result { - Ok(Box::try_new(T::default())?) - } + unsafe fn convert(_inode: *mut bindings::inode, _file: *mut bindings::file) -> *const T; } /// Corresponds to the kernel's `struct file_operations`. @@ -619,6 +596,14 @@ pub trait FileOperations: Send + Sync + Sized + 'static { /// The pointer type that will be used to hold ourselves. type Wrapper: PointerWrapper = Box; + /// The type of the context data passed to [`FileOperations::open`]. + type OpenData: Sync = (); + + /// Creates a new instance of this file. + /// + /// Corresponds to the `open` function pointer in `struct file_operations`. + fn open(context: &Self::OpenData, file: &File) -> Result; + /// Cleans up after the last reference to the file goes away. /// /// Note that the object is moved, so it will be freed automatically unless the implementation diff --git a/rust/kernel/miscdev.rs b/rust/kernel/miscdev.rs index f38ec203969d06..ccbd4f4cfe28e1 100644 --- a/rust/kernel/miscdev.rs +++ b/rust/kernel/miscdev.rs @@ -8,10 +8,10 @@ use crate::bindings; use crate::error::{Error, Result}; -use crate::file_operations::{FileOpenAdapter, FileOpener, FileOperationsVtable}; +use crate::file_operations::{FileOpenAdapter, FileOperations, FileOperationsVtable}; use crate::{str::CStr, KernelModule, ThisModule}; use alloc::boxed::Box; -use core::marker::{PhantomData, PhantomPinned}; +use core::marker::PhantomPinned; use core::{mem::MaybeUninit, pin::Pin}; /// A registration of a miscellaneous device. @@ -19,17 +19,17 @@ use core::{mem::MaybeUninit, pin::Pin}; /// # Invariants /// /// `Context` is always initialised when `registered` is `true`, and not initialised otherwise. -pub struct Registration { +pub struct Registration { registered: bool, mdev: bindings::miscdevice, _pin: PhantomPinned, /// Context initialised on construction and made available to all file instances on - /// [`FileOpener::open`]. - open_data: MaybeUninit, + /// [`FileOperations::open`]. + open_data: MaybeUninit, } -impl Registration { +impl Registration { /// Creates a new [`Registration`] but does not register it yet. /// /// It is allowed to move. @@ -46,13 +46,13 @@ impl Registration { /// Registers a miscellaneous device. /// /// Returns a pinned heap-allocated representation of the registration. - pub fn new_pinned>( + pub fn new_pinned( name: &'static CStr, minor: Option, - open_data: T, + open_data: T::OpenData, ) -> Result>> { let mut r = Pin::from(Box::try_new(Self::new())?); - r.as_mut().register::(name, minor, open_data)?; + r.as_mut().register(name, minor, open_data)?; Ok(r) } @@ -60,11 +60,11 @@ impl Registration { /// /// It must be pinned because the memory block that represents the registration is /// self-referential. If a minor is not given, the kernel allocates a new one if possible. - pub fn register>( + pub fn register( self: Pin<&mut Self>, name: &'static CStr, minor: Option, - open_data: T, + open_data: T::OpenData, ) -> Result { // SAFETY: We must ensure that we never move out of `this`. let this = unsafe { self.get_unchecked_mut() }; @@ -74,7 +74,7 @@ impl Registration { } // SAFETY: The adapter is compatible with `misc_register`. - this.mdev.fops = unsafe { FileOperationsVtable::::build() }; + this.mdev.fops = unsafe { FileOperationsVtable::::build() }; this.mdev.name = name.as_char_ptr(); this.mdev.minor = minor.unwrap_or(bindings::MISC_DYNAMIC_MINOR as i32); @@ -98,16 +98,17 @@ impl Registration { } } -impl Default for Registration { +impl Default for Registration { fn default() -> Self { Self::new() } } -impl FileOpenAdapter for Registration { - type Arg = T; - - unsafe fn convert(_inode: *mut bindings::inode, file: *mut bindings::file) -> *const Self::Arg { +impl FileOpenAdapter for Registration { + unsafe fn convert( + _inode: *mut bindings::inode, + file: *mut bindings::file, + ) -> *const T::OpenData { // SAFETY: the caller must guarantee that `file` is valid. let reg = crate::container_of!(unsafe { (*file).private_data }, Self, mdev); @@ -120,14 +121,13 @@ impl FileOpenAdapter for Registration { // SAFETY: The only method is `register()`, which requires a (pinned) mutable `Registration`, so it // is safe to pass `&Registration` to multiple threads because it offers no interior mutability. -unsafe impl Sync for Registration {} +unsafe impl Sync for Registration {} // SAFETY: All functions work from any thread. So as long as the `Registration::open_data` is -// `Send`, so is `Registration`. `T` needs to be `Sync` because it's a requirement of -// `Registration`. -unsafe impl Send for Registration {} +// `Send`, so is `Registration`. +unsafe impl Send for Registration where T::OpenData: Send {} -impl Drop for Registration { +impl Drop for Registration { /// Removes the registration from the kernel if it has completed successfully before. fn drop(&mut self) { if self.registered { @@ -142,17 +142,15 @@ impl Drop for Registration { } } -/// Kernel module that exposes a single miscdev device implemented by `F`. -pub struct Module> { - _dev: Pin>, - _p: PhantomData, +/// Kernel module that exposes a single miscdev device implemented by `T`. +pub struct Module> { + _dev: Pin>>, } -impl> KernelModule for Module { +impl> KernelModule for Module { fn init(name: &'static CStr, _module: &'static ThisModule) -> Result { Ok(Self { - _p: PhantomData, - _dev: Registration::new_pinned::(name, None, ())?, + _dev: Registration::new_pinned(name, None, ())?, }) } } diff --git a/samples/rust/rust_chrdev.rs b/samples/rust/rust_chrdev.rs index 5a37c59019d7b2..8d7628b018f35c 100644 --- a/samples/rust/rust_chrdev.rs +++ b/samples/rust/rust_chrdev.rs @@ -6,7 +6,7 @@ #![feature(allocator_api, global_asm)] use kernel::prelude::*; -use kernel::{chrdev, file_operations::FileOperations}; +use kernel::{chrdev, file, file_operations::FileOperations}; module! { type: RustChrdev, @@ -16,11 +16,14 @@ module! { license: b"GPL v2", } -#[derive(Default)] struct RustFile; impl FileOperations for RustFile { kernel::declare_file_operations!(); + + fn open(_shared: &(), _file: &file::File) -> Result { + Ok(Box::try_new(RustFile)?) + } } struct RustChrdev { diff --git a/samples/rust/rust_miscdev.rs b/samples/rust/rust_miscdev.rs index 65dd400275243d..1064a4caeb4f9e 100644 --- a/samples/rust/rust_miscdev.rs +++ b/samples/rust/rust_miscdev.rs @@ -8,7 +8,7 @@ use kernel::prelude::*; use kernel::{ file::File, - file_operations::{FileOpener, FileOperations}, + file_operations::FileOperations, io_buffer::{IoBufferReader, IoBufferWriter}, miscdev, sync::{CondVar, Mutex, Ref, RefBorrow, UniqueRef}, @@ -55,18 +55,16 @@ impl SharedState { } struct Token; - -impl FileOpener> for Token { - fn open(shared: &Ref, _file: &File) -> Result { - Ok(shared.clone()) - } -} - impl FileOperations for Token { type Wrapper = Ref; + type OpenData = Ref; kernel::declare_file_operations!(read, write); + fn open(shared: &Ref, _file: &File) -> Result { + Ok(shared.clone()) + } + fn read( shared: RefBorrow<'_, SharedState>, _: &File, @@ -127,7 +125,7 @@ impl FileOperations for Token { } struct RustMiscdev { - _dev: Pin>>>, + _dev: Pin>>, } impl KernelModule for RustMiscdev { @@ -137,7 +135,7 @@ impl KernelModule for RustMiscdev { let state = SharedState::try_new()?; Ok(RustMiscdev { - _dev: miscdev::Registration::new_pinned::(name, None, state)?, + _dev: miscdev::Registration::new_pinned(name, None, state)?, }) } } diff --git a/samples/rust/rust_random.rs b/samples/rust/rust_random.rs index ef28c5f84f7169..ed9c7eb6c99914 100644 --- a/samples/rust/rust_random.rs +++ b/samples/rust/rust_random.rs @@ -15,12 +15,15 @@ use kernel::{ prelude::*, }; -#[derive(Default)] struct RandomFile; impl FileOperations for RandomFile { kernel::declare_file_operations!(read, write, read_iter, write_iter); + fn open(_open_data: &(), _file: &File) -> Result { + Ok(Box::try_new(RandomFile)?) + } + fn read(_this: &Self, file: &File, buf: &mut impl IoBufferWriter, _: u64) -> Result { let total_len = buf.len(); let mut chunkbuf = [0; 256]; diff --git a/samples/rust/rust_semaphore.rs b/samples/rust/rust_semaphore.rs index 28f7a34d2942ed..f7fa05ae4fb16b 100644 --- a/samples/rust/rust_semaphore.rs +++ b/samples/rust/rust_semaphore.rs @@ -20,7 +20,7 @@ use core::sync::atomic::{AtomicU64, Ordering}; use kernel::{ condvar_init, declare_file_operations, file::File, - file_operations::{FileOpener, FileOperations, IoctlCommand, IoctlHandler}, + file_operations::{FileOperations, IoctlCommand, IoctlHandler}, io_buffer::{IoBufferReader, IoBufferWriter}, miscdev::Registration, mutex_init, @@ -65,17 +65,17 @@ impl FileState { } } -impl FileOpener> for FileState { +impl FileOperations for FileState { + type OpenData = Ref; + + declare_file_operations!(read, write, ioctl); + fn open(shared: &Ref, _file: &File) -> Result> { Ok(Box::try_new(Self { read_count: AtomicU64::new(0), shared: shared.clone(), })?) } -} - -impl FileOperations for FileState { - declare_file_operations!(read, write, ioctl); fn read(this: &Self, _: &File, data: &mut impl IoBufferWriter, offset: u64) -> Result { if data.is_empty() || offset > 0 { @@ -106,7 +106,7 @@ impl FileOperations for FileState { } struct RustSemaphore { - _dev: Pin>>>, + _dev: Pin>>, } impl KernelModule for RustSemaphore { @@ -135,7 +135,7 @@ impl KernelModule for RustSemaphore { mutex_init!(pinned, "Semaphore::inner"); Ok(Self { - _dev: Registration::new_pinned::(name, None, sema.into())?, + _dev: Registration::new_pinned(name, None, sema.into())?, }) } }