[PATCH] rust: miscdevice: add registration data to MiscDevice

Alice Ryhl posted 1 patch 1 month, 3 weeks ago
rust/kernel/miscdevice.rs        | 42 ++++++++++++++++++++++++++++++----------
samples/rust/rust_misc_device.rs |  3 ++-
2 files changed, 34 insertions(+), 11 deletions(-)
[PATCH] rust: miscdevice: add registration data to MiscDevice
Posted by Alice Ryhl 1 month, 3 weeks ago
Currently when fds for a miscdevice are opened, the only "global" data
they are given access to is the MiscDeviceRegistration value. However,
this value doesn't let you store any user-provided data, so there is no
way for different fds from the same miscdevice to interact with each
other. Thus, let the user specify a type to be stored in the
MiscDeviceRegistration in which the user can store whichever data they
would like.

The intended user of this patch is Rust course material. Miscdevice is a
nice and relatively easy to use API for someone's first driver, and
being able to persist data from fd to fd allows the student to interact
with their driver using 'cat' and 'echo', even though each call opens a
new fd.

Signed-off-by: Alice Ryhl <aliceryhl@google.com>
---
 rust/kernel/miscdevice.rs        | 42 ++++++++++++++++++++++++++++++----------
 samples/rust/rust_misc_device.rs |  3 ++-
 2 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index 3abef1b8543d..c4910918cd15 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -31,7 +31,10 @@
         Opaque, //
     },
 };
-use core::marker::PhantomData;
+use core::{
+    marker::PhantomData,
+    ops::Deref, //
+};
 
 /// Options for creating a misc device.
 #[derive(Copy, Clone)]
@@ -62,25 +65,30 @@ pub const fn into_raw<T: MiscDevice>(self) -> bindings::miscdevice {
 /// - Deregistration occurs exactly once in [`Drop`] via `misc_deregister()`.
 /// - `inner` wraps a valid, pinned `miscdevice` created using
 ///   [`MiscDeviceOptions::into_raw`].
-#[repr(transparent)]
+#[repr(C)]
 #[pin_data(PinnedDrop)]
-pub struct MiscDeviceRegistration<T> {
+pub struct MiscDeviceRegistration<T: MiscDevice> {
     #[pin]
     inner: Opaque<bindings::miscdevice>,
-    _t: PhantomData<T>,
+    #[pin]
+    data: T::RegistrationData,
 }
 
 // SAFETY: It is allowed to call `misc_deregister` on a different thread from where you called
 // `misc_register`.
-unsafe impl<T> Send for MiscDeviceRegistration<T> {}
+unsafe impl<T: MiscDevice> Send for MiscDeviceRegistration<T> where T::RegistrationData: Send {}
 // SAFETY: All `&self` methods on this type are written to ensure that it is safe to call them in
-// parallel.
-unsafe impl<T> Sync for MiscDeviceRegistration<T> {}
+// parallel. The `RegistrationData` type is always `Sync`.
+unsafe impl<T: MiscDevice> Sync for MiscDeviceRegistration<T> {}
 
 impl<T: MiscDevice> MiscDeviceRegistration<T> {
     /// Register a misc device.
-    pub fn register(opts: MiscDeviceOptions) -> impl PinInit<Self, Error> {
+    pub fn register(
+        opts: MiscDeviceOptions,
+        data: impl PinInit<T::RegistrationData, Error>,
+    ) -> impl PinInit<Self, Error> {
         try_pin_init!(Self {
+            data <- data,
             inner <- Opaque::try_ffi_init(move |slot: *mut bindings::miscdevice| {
                 // SAFETY: The initializer can write to the provided `slot`.
                 unsafe { slot.write(opts.into_raw::<T>()) };
@@ -88,11 +96,14 @@ pub fn register(opts: MiscDeviceOptions) -> impl PinInit<Self, Error> {
                 // SAFETY: We just wrote the misc device options to the slot. The miscdevice will
                 // get unregistered before `slot` is deallocated because the memory is pinned and
                 // the destructor of this type deallocates the memory.
+                //
+                // The `data` field is `Sync + 'static`, so it's okay for the `open` callback to
+                // access it until the destructor is invoked.
+                //
                 // INVARIANT: If this returns `Ok(())`, then the `slot` will contain a registered
                 // misc device.
                 to_result(unsafe { bindings::misc_register(slot) })
             }),
-            _t: PhantomData,
         })
     }
 
@@ -112,8 +123,16 @@ pub fn device(&self) -> &Device {
     }
 }
 
+impl<T: MiscDevice> Deref for MiscDeviceRegistration<T> {
+    type Target = T::RegistrationData;
+    #[inline]
+    fn deref(&self) -> &T::RegistrationData {
+        &self.data
+    }
+}
+
 #[pinned_drop]
-impl<T> PinnedDrop for MiscDeviceRegistration<T> {
+impl<T: MiscDevice> PinnedDrop for MiscDeviceRegistration<T> {
     fn drop(self: Pin<&mut Self>) {
         // SAFETY: We know that the device is registered by the type invariants.
         unsafe { bindings::misc_deregister(self.inner.get()) };
@@ -126,6 +145,9 @@ pub trait MiscDevice: Sized {
     /// What kind of pointer should `Self` be wrapped in.
     type Ptr: ForeignOwnable + Send + Sync;
 
+    /// The registration data shared between all open files for this character device.
+    type RegistrationData: Sync + 'static;
+
     /// Called when the misc device is opened.
     ///
     /// The returned pointer will be stored as the private data for the file.
diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
index 41e26c825060..0bde071743ef 100644
--- a/samples/rust/rust_misc_device.rs
+++ b/samples/rust/rust_misc_device.rs
@@ -156,7 +156,7 @@ fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
         };
 
         try_pin_init!(Self {
-            _miscdev <- MiscDeviceRegistration::register(options),
+            _miscdev <- MiscDeviceRegistration::register(options, Ok(())),
         })
     }
 }
@@ -176,6 +176,7 @@ struct RustMiscDevice {
 #[vtable]
 impl MiscDevice for RustMiscDevice {
     type Ptr = Pin<KBox<Self>>;
+    type RegistrationData = ();
 
     fn open(_file: &File, misc: &MiscDeviceRegistration<Self>) -> Result<Pin<KBox<Self>>> {
         let dev = ARef::from(misc.device());

---
base-commit: 220190f97da558e67cd01c62f1b84fe77b267a5a
change-id: 20260807-miscdevice-data-71c727e8b2e5

Best regards,
-- 
Alice Ryhl <aliceryhl@google.com>
Re: [PATCH] rust: miscdevice: add registration data to MiscDevice
Posted by Gary Guo 1 month, 3 weeks ago
On Fri Aug 7, 2026 at 3:22 PM BST, Alice Ryhl wrote:
> Currently when fds for a miscdevice are opened, the only "global" data
> they are given access to is the MiscDeviceRegistration value. However,
> this value doesn't let you store any user-provided data, so there is no
> way for different fds from the same miscdevice to interact with each
> other. Thus, let the user specify a type to be stored in the
> MiscDeviceRegistration in which the user can store whichever data they
> would like.
>
> The intended user of this patch is Rust course material. Miscdevice is a
> nice and relatively easy to use API for someone's first driver, and
> being able to persist data from fd to fd allows the student to interact
> with their driver using 'cat' and 'echo', even though each call opens a
> new fd.
>
> Signed-off-by: Alice Ryhl <aliceryhl@google.com>
> ---
>  rust/kernel/miscdevice.rs        | 42 ++++++++++++++++++++++++++++++----------
>  samples/rust/rust_misc_device.rs |  3 ++-
>  2 files changed, 34 insertions(+), 11 deletions(-)
>
> diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
> index 3abef1b8543d..c4910918cd15 100644
> --- a/rust/kernel/miscdevice.rs
> +++ b/rust/kernel/miscdevice.rs
> @@ -31,7 +31,10 @@
>          Opaque, //
>      },
>  };
> -use core::marker::PhantomData;
> +use core::{
> +    marker::PhantomData,
> +    ops::Deref, //
> +};
>  
>  /// Options for creating a misc device.
>  #[derive(Copy, Clone)]
> @@ -62,25 +65,30 @@ pub const fn into_raw<T: MiscDevice>(self) -> bindings::miscdevice {
>  /// - Deregistration occurs exactly once in [`Drop`] via `misc_deregister()`.
>  /// - `inner` wraps a valid, pinned `miscdevice` created using
>  ///   [`MiscDeviceOptions::into_raw`].
> -#[repr(transparent)]
> +#[repr(C)]
>  #[pin_data(PinnedDrop)]
> -pub struct MiscDeviceRegistration<T> {
> +pub struct MiscDeviceRegistration<T: MiscDevice> {
>      #[pin]
>      inner: Opaque<bindings::miscdevice>,
> -    _t: PhantomData<T>,
> +    #[pin]
> +    data: T::RegistrationData,
>  }
>  
>  // SAFETY: It is allowed to call `misc_deregister` on a different thread from where you called
>  // `misc_register`.
> -unsafe impl<T> Send for MiscDeviceRegistration<T> {}
> +unsafe impl<T: MiscDevice> Send for MiscDeviceRegistration<T> where T::RegistrationData: Send {}
>  // SAFETY: All `&self` methods on this type are written to ensure that it is safe to call them in
> -// parallel.
> -unsafe impl<T> Sync for MiscDeviceRegistration<T> {}
> +// parallel. The `RegistrationData` type is always `Sync`.
> +unsafe impl<T: MiscDevice> Sync for MiscDeviceRegistration<T> {}
>  
>  impl<T: MiscDevice> MiscDeviceRegistration<T> {
>      /// Register a misc device.
> -    pub fn register(opts: MiscDeviceOptions) -> impl PinInit<Self, Error> {
> +    pub fn register(
> +        opts: MiscDeviceOptions,
> +        data: impl PinInit<T::RegistrationData, Error>,

We can just skip the `RegistrationData` and use `T`?

Best,
Gary


> +    ) -> impl PinInit<Self, Error> {
>          try_pin_init!(Self {
> +            data <- data,
>              inner <- Opaque::try_ffi_init(move |slot: *mut bindings::miscdevice| {
>                  // SAFETY: The initializer can write to the provided `slot`.
>                  unsafe { slot.write(opts.into_raw::<T>()) };
> @@ -88,11 +96,14 @@ pub fn register(opts: MiscDeviceOptions) -> impl PinInit<Self, Error> {
>                  // SAFETY: We just wrote the misc device options to the slot. The miscdevice will
>                  // get unregistered before `slot` is deallocated because the memory is pinned and
>                  // the destructor of this type deallocates the memory.
> +                //
> +                // The `data` field is `Sync + 'static`, so it's okay for the `open` callback to
> +                // access it until the destructor is invoked.
> +                //
>                  // INVARIANT: If this returns `Ok(())`, then the `slot` will contain a registered
>                  // misc device.
>                  to_result(unsafe { bindings::misc_register(slot) })
>              }),
> -            _t: PhantomData,
>          })
>      }
>  
> @@ -112,8 +123,16 @@ pub fn device(&self) -> &Device {
>      }
>  }
>  
> +impl<T: MiscDevice> Deref for MiscDeviceRegistration<T> {
> +    type Target = T::RegistrationData;
> +    #[inline]
> +    fn deref(&self) -> &T::RegistrationData {
> +        &self.data
> +    }
> +}
> +
>  #[pinned_drop]
> -impl<T> PinnedDrop for MiscDeviceRegistration<T> {
> +impl<T: MiscDevice> PinnedDrop for MiscDeviceRegistration<T> {
>      fn drop(self: Pin<&mut Self>) {
>          // SAFETY: We know that the device is registered by the type invariants.
>          unsafe { bindings::misc_deregister(self.inner.get()) };
> @@ -126,6 +145,9 @@ pub trait MiscDevice: Sized {
>      /// What kind of pointer should `Self` be wrapped in.
>      type Ptr: ForeignOwnable + Send + Sync;
>  
> +    /// The registration data shared between all open files for this character device.
> +    type RegistrationData: Sync + 'static;
> +
>      /// Called when the misc device is opened.
>      ///
>      /// The returned pointer will be stored as the private data for the file.
> diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
> index 41e26c825060..0bde071743ef 100644
> --- a/samples/rust/rust_misc_device.rs
> +++ b/samples/rust/rust_misc_device.rs
> @@ -156,7 +156,7 @@ fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
>          };
>  
>          try_pin_init!(Self {
> -            _miscdev <- MiscDeviceRegistration::register(options),
> +            _miscdev <- MiscDeviceRegistration::register(options, Ok(())),
>          })
>      }
>  }
> @@ -176,6 +176,7 @@ struct RustMiscDevice {
>  #[vtable]
>  impl MiscDevice for RustMiscDevice {
>      type Ptr = Pin<KBox<Self>>;
> +    type RegistrationData = ();
>  
>      fn open(_file: &File, misc: &MiscDeviceRegistration<Self>) -> Result<Pin<KBox<Self>>> {
>          let dev = ARef::from(misc.device());
>
> ---
> base-commit: 220190f97da558e67cd01c62f1b84fe77b267a5a
> change-id: 20260807-miscdevice-data-71c727e8b2e5
>
> Best regards,
Re: [PATCH] rust: miscdevice: add registration data to MiscDevice
Posted by Alice Ryhl 1 month, 3 weeks ago
On Fri, Aug 7, 2026 at 4:43 PM Gary Guo <gary@garyguo.net> wrote:
>
> On Fri Aug 7, 2026 at 3:22 PM BST, Alice Ryhl wrote:
> >  impl<T: MiscDevice> MiscDeviceRegistration<T> {
> >      /// Register a misc device.
> > -    pub fn register(opts: MiscDeviceOptions) -> impl PinInit<Self, Error> {
> > +    pub fn register(
> > +        opts: MiscDeviceOptions,
> > +        data: impl PinInit<T::RegistrationData, Error>,
>
> We can just skip the `RegistrationData` and use `T`?

`T` is the type of the data stored per-fd. We do not separate types
for those two concepts.

Alice
Re: [PATCH] rust: miscdevice: add registration data to MiscDevice
Posted by Gary Guo 1 month, 3 weeks ago
On Fri Aug 7, 2026 at 4:44 PM BST, Alice Ryhl wrote:
> On Fri, Aug 7, 2026 at 4:43 PM Gary Guo <gary@garyguo.net> wrote:
>>
>> On Fri Aug 7, 2026 at 3:22 PM BST, Alice Ryhl wrote:
>> >  impl<T: MiscDevice> MiscDeviceRegistration<T> {
>> >      /// Register a misc device.
>> > -    pub fn register(opts: MiscDeviceOptions) -> impl PinInit<Self, Error> {
>> > +    pub fn register(
>> > +        opts: MiscDeviceOptions,
>> > +        data: impl PinInit<T::RegistrationData, Error>,
>>
>> We can just skip the `RegistrationData` and use `T`?
>
> `T` is the type of the data stored per-fd. We do not separate types
> for those two concepts.

This is getting backwards though, and won't work when we need to introduce
lifetimes.

Per-fd data needs to be able to reference whatever lifetime is available on
per-reg data, so it should be assoc type of the per-reg data type. The way
around you current have causes `T::RegistrationData` to be able to reference
lifetimes on `T` which is not okay and would only work with `'static` data.

Best,
Gary

>
> Alice
Re: [PATCH] rust: miscdevice: add registration data to MiscDevice
Posted by Alice Ryhl 1 month, 3 weeks ago
On Fri, Aug 07, 2026 at 04:48:53PM +0100, Gary Guo wrote:
> On Fri Aug 7, 2026 at 4:44 PM BST, Alice Ryhl wrote:
> > On Fri, Aug 7, 2026 at 4:43 PM Gary Guo <gary@garyguo.net> wrote:
> >>
> >> On Fri Aug 7, 2026 at 3:22 PM BST, Alice Ryhl wrote:
> >> >  impl<T: MiscDevice> MiscDeviceRegistration<T> {
> >> >      /// Register a misc device.
> >> > -    pub fn register(opts: MiscDeviceOptions) -> impl PinInit<Self, Error> {
> >> > +    pub fn register(
> >> > +        opts: MiscDeviceOptions,
> >> > +        data: impl PinInit<T::RegistrationData, Error>,
> >>
> >> We can just skip the `RegistrationData` and use `T`?
> >
> > `T` is the type of the data stored per-fd. We do not separate types
> > for those two concepts.
> 
> This is getting backwards though, and won't work when we need to introduce
> lifetimes.
> 
> Per-fd data needs to be able to reference whatever lifetime is available on
> per-reg data, so it should be assoc type of the per-reg data type. The way
> around you current have causes `T::RegistrationData` to be able to reference
> lifetimes on `T` which is not okay and would only work with `'static` data.

Unfortunately the fds can outlive the registration, so we can't have
such lifetimes to begin with.

I know that miscdevice isn't currently built to work with the new device
model. We should fix that, but it is out of scope for this patch. Until
then, I think the current solution in this patch is sufficient.

Alice
Re: [PATCH] rust: miscdevice: add registration data to MiscDevice
Posted by Gary Guo 1 month, 3 weeks ago
On Sat Aug 8, 2026 at 2:53 PM BST, Alice Ryhl wrote:
> On Fri, Aug 07, 2026 at 04:48:53PM +0100, Gary Guo wrote:
>> On Fri Aug 7, 2026 at 4:44 PM BST, Alice Ryhl wrote:
>> > On Fri, Aug 7, 2026 at 4:43 PM Gary Guo <gary@garyguo.net> wrote:
>> >>
>> >> On Fri Aug 7, 2026 at 3:22 PM BST, Alice Ryhl wrote:
>> >> >  impl<T: MiscDevice> MiscDeviceRegistration<T> {
>> >> >      /// Register a misc device.
>> >> > -    pub fn register(opts: MiscDeviceOptions) -> impl PinInit<Self, Error> {
>> >> > +    pub fn register(
>> >> > +        opts: MiscDeviceOptions,
>> >> > +        data: impl PinInit<T::RegistrationData, Error>,
>> >>
>> >> We can just skip the `RegistrationData` and use `T`?
>> >
>> > `T` is the type of the data stored per-fd. We do not separate types
>> > for those two concepts.
>> 
>> This is getting backwards though, and won't work when we need to introduce
>> lifetimes.
>> 
>> Per-fd data needs to be able to reference whatever lifetime is available on
>> per-reg data, so it should be assoc type of the per-reg data type. The way
>> around you current have causes `T::RegistrationData` to be able to reference
>> lifetimes on `T` which is not okay and would only work with `'static` data.
>
> Unfortunately the fds can outlive the registration, so we can't have
> such lifetimes to begin with.
>
> I know that miscdevice isn't currently built to work with the new device
> model. We should fix that, but it is out of scope for this patch. Until
> then, I think the current solution in this patch is sufficient.

For course material we should try to teach the new device model. Yes without a
proper revocation mechanism the fd data cannot have lifetimes, but we can
workaround that by having `type Data: 'static` for now.

Best,
Gary