[PATCH v2 0/3] rust: miscdevice: Add additional data to MiscDeviceRegistration

[PATCH v2 0/3] rust: miscdevice: Add additional data to MiscDeviceRegistration

[Christian Schrefl]

Currently there is no good way to pass arbitrary data from the driver to
a miscdevice or to share data between individual handles to a miscdevice in rust.

This series adds additional (generic) data to the MiscDeviceRegistration
for this purpose.

The first patch adds the UnsafePinned (Previously Aliased) type.

The second patch implements the changes and fixes the build of the sample.

The third patch changes the `rust_misc_device` sample to use this to 
share the same data between multiple handles to the miscdevice.
I have tested the sample with qemu and the C userspace example
from the doc comments.

Some discussion on Zulip about the motivation and approach
(Thanks a lot to everyone helping me out with this):
Link: https://rust-for-linux.zulipchat.com/#narrow/channel/288089-General/topic/Passing.20a.20DevRes.20to.20a.20miscdev/near/494553814

Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
---
Changes in v2:
- Don't use associated_type_bounds since the MSRV does not support
    that on stable yet (Kernel test robot)
- Doc changes and add intra-doc links (Miguel)
- Use container_of macro instead of pointer cast in `fops_open` (Greg)
- Rename `Aliased` to `UnsafePinned` (Boqun)
- Make sure Data is initialized befofre `misc_register` is called
- Rework the example to use an additional shared value instead of 
    replacing the unique one
- Expanded the c code for the example to use the new ioctls
- Link to v1: https://lore.kernel.org/r/20250119-b4-rust_miscdevice_registrationdata-v1-0-edbf18dde5fc@gmail.com

---
Christian Schrefl (3):
      rust: add UnsafePinned type
      rust: miscdevice: Add additional data to MiscDeviceRegistration
      rust: miscdevice: adjust the rust_misc_device sample to use RegistrationData.

 rust/kernel/miscdevice.rs        |  75 ++++++++++++++++++-------
 rust/kernel/types.rs             |  57 +++++++++++++++++++
 samples/rust/rust_misc_device.rs | 117 ++++++++++++++++++++++++++++++++++++---
 3 files changed, 221 insertions(+), 28 deletions(-)
---
base-commit: b4b0881156fb8209bf5ef6cb63211bb0ad6e1a6b
change-id: 20250119-b4-rust_miscdevice_registrationdata-a11d88dcb284

Best regards,
-- 
Christian Schrefl <chrisi.schrefl@gmail.com>

[PATCH v2 1/3] rust: add UnsafePinned type

[Christian Schrefl]

`UnsafePinned<T>` is useful for cases where a value might be shared with C
code but not directly used by it. In particular this is added for
additional data in the `MiscDeviceRegistration` which will be shared
between `fops->open` and the containing struct.

Similar to `Opaque` but guarantees that the value is always initialized
and that the inner value is dropped when `UnsafePinned` is dropped.

This was originally proposed for the IRQ abstractions [0] and is also
useful for other where the inner data may be aliased, but is always valid
and automatic `Drop` is desired.

Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
---
 rust/kernel/types.rs | 57 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 2bbaab83b9d65da667a07e85b3c89c7fa881b53c..3c2f6ac62d161f1187b5e7ade86689eec667ff4d 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -253,6 +253,9 @@ fn drop(&mut self) {
 ///
 /// `Opaque<T>` is meant to be used with FFI objects that are never interpreted by Rust code.
 ///
+/// In cases where the contained data is only used by Rust, is not allowed to be
+/// uninitialized and automatic [`Drop`] is desired [`UnsafePinned`] should be used instead.
+///
 /// It is used to wrap structs from the C side, like for example `Opaque<bindings::mutex>`.
 /// It gets rid of all the usual assumptions that Rust has for a value:
 ///
@@ -573,3 +576,57 @@ pub enum Either<L, R> {
 /// [`NotThreadSafe`]: type@NotThreadSafe
 #[allow(non_upper_case_globals)]
 pub const NotThreadSafe: NotThreadSafe = PhantomData;
+
+/// Stores a value that may be used from multiple mutable pointers.
+///
+/// `UnsafePinned` gets rid of some of the usual assumptions that Rust has for a value:
+/// - The value is allowed to be mutated, when a `&UnsafePinned<T>` exists on the Rust side.
+/// - No uniqueness for mutable references: it is fine to have multiple `&mut UnsafePinned<T>`
+///   point to the same value.
+///
+/// To avoid the ability to use [`core::mem::swap`] this still needs to be used through a
+/// [`core::pin::Pin`] reference.
+///
+/// This is useful for cases where a value might be shared with C code
+/// but not interpreted by it or in cases where it can not always be guaranteed that the
+/// references are unique.
+///
+/// This is similar to [`Opaque<T>`] but is guaranteed to always contain valid data and will
+/// call the [`Drop`] implementation of `T` when dropped.
+#[repr(transparent)]
+pub struct UnsafePinned<T> {
+    value: UnsafeCell<T>,
+    _pin: PhantomPinned,
+}
+
+impl<T> UnsafePinned<T> {
+    /// Creates a new [`UnsafePinned`] value.
+    pub const fn new(value: T) -> Self {
+        Self {
+            value: UnsafeCell::new(value),
+            _pin: PhantomPinned,
+        }
+    }
+
+    /// Create an [`UnsafePinned`] pin-initializer from the given pin-initializer.
+    pub fn try_pin_init<E>(value: impl PinInit<T, E>) -> impl PinInit<Self, E> {
+        // SAFETY:
+        //   - In case of an error in `value` the error is returned, otherwise `slot` is fully
+        //     initialized, since `self.value` is initialized and `_pin` is a zero sized type.
+        //   - The `Pin` invariants of `self.value` are upheld, since no moving occurs.
+        unsafe { init::pin_init_from_closure(move |slot| value.__pinned_init(Self::raw_get(slot))) }
+    }
+
+    /// Returns a raw pointer to the contained data.
+    pub const fn get(&self) -> *mut T {
+        UnsafeCell::get(&self.value).cast::<T>()
+    }
+
+    /// Gets the value behind `this`.
+    ///
+    /// This function is useful to get access to the value without creating intermediate
+    /// references.
+    pub const fn raw_get(this: *const Self) -> *mut T {
+        UnsafeCell::raw_get(this.cast::<UnsafeCell<MaybeUninit<T>>>()).cast::<T>()
+    }
+}

-- 
2.48.1

[PATCH v2 2/3] rust: miscdevice: Add additional data to MiscDeviceRegistration

[Christian Schrefl]

When using the Rust miscdevice bindings, you generally embed the
`MiscDeviceRegistration` within another struct:

struct MyDriverData {
    data: SomeOtherData,
    misc: MiscDeviceRegistration<MyMiscFile>
}

In the `fops->open` callback of the miscdevice, you are given a
reference to the registration, which allows you to access its fields.
For example, as of commit 284ae0be4dca ("rust: miscdevice: Provide
accessor to pull out miscdevice::this_device") you can access the
internal `struct device`. However, there is still no way to access the
`data` field in the above example, because you only have a reference to
the registration.

Using `container_of` is also not possible to do safely. For example, if
the destructor of `MyDriverData` runs, then the destructor of `data`
would run before the miscdevice is deregistered, so using `container_of`
to access `data` from `fops->open` could result in a UAF. A similar
problem can happen on initialization if `misc` is not the last field to
be initialized.

To provide a safe way to access user-defined data stored next to the
`struct miscdevice`, make `MiscDeviceRegistration` into a container that
can store a user-provided piece of data. This way, `fops->open` can
access that data via the registration, since the data is stored inside
the registration.

The container enforces that the additional user data is initialized
before the miscdevice is registered, and that the miscdevice is
deregistered before the user data is destroyed. This ensures that access
to the userdata is safe.

For the same reasons as in commit 88441d5c6d17 ("rust: miscdevice:
access the `struct miscdevice` from fops->open()"), you cannot access
the user data in any other fops callback than open. This is because a
miscdevice can be deregistered while there are still open files.

A situation where this user data might be required is when a platform
driver acquires a resource in `probe` and wants to use this resource in
the `fops` implementation of a `MiscDevice`.

This solution is similar to the approach used by the initial downstream
Rust-for-Linux/Rust branch [0].

Link: https://github.com/Rust-for-Linux/linux/blob/rust/rust/kernel/miscdev.rs#L108 [0]
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
---
 rust/kernel/miscdevice.rs        | 75 +++++++++++++++++++++++++++++-----------
 samples/rust/rust_misc_device.rs |  4 ++-
 2 files changed, 58 insertions(+), 21 deletions(-)

diff --git a/rust/kernel/miscdevice.rs b/rust/kernel/miscdevice.rs
index e14433b2ab9d8fa391474b2ad7e3ed55c64b4d91..dea7d8d1a0366cf2243c7a3888ebfb8a90d6295c 100644
--- a/rust/kernel/miscdevice.rs
+++ b/rust/kernel/miscdevice.rs
@@ -9,7 +9,7 @@
 //! Reference: <https://www.kernel.org/doc/html/latest/driver-api/misc_devices.html>
 
 use crate::{
-    bindings,
+    bindings, container_of,
     device::Device,
     error::{to_result, Error, Result, VTABLE_DEFAULT_ERROR},
     ffi::{c_int, c_long, c_uint, c_ulong},
@@ -17,7 +17,7 @@
     prelude::*,
     seq_file::SeqFile,
     str::CStr,
-    types::{ForeignOwnable, Opaque},
+    types::{ForeignOwnable, Opaque, UnsafePinned},
 };
 use core::{marker::PhantomData, mem::MaybeUninit, pin::Pin};
 
@@ -45,32 +45,46 @@ pub const fn into_raw<T: MiscDevice>(self) -> bindings::miscdevice {
 /// # Invariants
 ///
 /// `inner` is a registered misc device.
-#[repr(transparent)]
+#[repr(C)]
 #[pin_data(PinnedDrop)]
-pub struct MiscDeviceRegistration<T> {
+pub struct MiscDeviceRegistration<T: MiscDevice> {
     #[pin]
     inner: Opaque<bindings::miscdevice>,
+    #[pin]
+    data: UnsafePinned<T::RegistrationData>,
     _t: PhantomData<T>,
 }
 
-// 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> {}
-// 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> {}
+// SAFETY:
+// - It is allowed to call `misc_deregister` on a different thread from where you called
+//   `misc_register`.
+// - Only implements `Send` if `MiscDevice::RegistrationData` is also `Send`.
+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.
+// - `MiscDevice::RegistrationData` 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 <- UnsafePinned::try_pin_init(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>()) };
 
-                // 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.
+                // 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.
+                // * `data` is Initialized before `misc_register` so no race with `fops->open()`
+                //   is possible.
                 // INVARIANT: If this returns `Ok(())`, then the `slot` will contain a registered
                 // misc device.
                 to_result(unsafe { bindings::misc_register(slot) })
@@ -93,10 +107,18 @@ pub fn device(&self) -> &Device {
         // before the underlying `struct miscdevice` is destroyed.
         unsafe { Device::as_ref((*self.as_raw()).this_device) }
     }
+
+    /// Access the additional data stored in this registration.
+    pub fn data(&self) -> &T::RegistrationData {
+        // SAFETY:
+        // * No mutable reference to the value contained by `self.data` can ever be created.
+        // * The value contained by `self.data` is valid for the entire lifetime of `&self`.
+        unsafe { &*self.data.get() }
+    }
 }
 
 #[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()) };
@@ -109,6 +131,13 @@ pub trait MiscDevice: Sized {
     /// What kind of pointer should `Self` be wrapped in.
     type Ptr: ForeignOwnable + Send + Sync;
 
+    /// The additional data carried by the [`MiscDeviceRegistration`] for this [`MiscDevice`].
+    /// If no additional data is required than the unit type `()` should be used.
+    ///
+    /// This data can be accessed in [`MiscDevice::open()`] using
+    /// [`MiscDeviceRegistration::data()`].
+    type RegistrationData: Sync;
+
     /// Called when the misc device is opened.
     ///
     /// The returned pointer will be stored as the private data for the file.
@@ -211,17 +240,23 @@ impl<T: MiscDevice> VtableHelper<T> {
     // SAFETY: The open call of a file can access the private data.
     let misc_ptr = 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>>() };
+    // This is a miscdevice, so `misc_open()` sets the private data to a pointer to the
+    // associated `struct miscdevice` before calling into this method.
+    let misc_ptr = misc_ptr.cast::<bindings::miscdevice>();
+
+    // SAFETY:
+    // * `misc_open()` ensures that the `struct miscdevice` can't be unregistered and freed
+    //   during this call to `fops_open`.
+    // * The `misc_ptr` always points to the `inner` field of a `MiscDeviceRegistration<T>`.
+    // * The `MiscDeviceRegistration<T>` is valid until the `struct miscdevice` was unregistered.
+    let registration = unsafe { &*container_of!(misc_ptr, MiscDeviceRegistration<T>, inner) };
 
     // SAFETY:
     // * This underlying file is valid for (much longer than) the duration of `T::open`.
     // * There is no active fdget_pos region on the file on this thread.
     let file = unsafe { File::from_raw_file(raw_file) };
 
-    let ptr = match T::open(file, misc) {
+    let ptr = match T::open(file, registration) {
         Ok(ptr) => ptr,
         Err(err) => return err.to_errno(),
     };
diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
index 40ad7266c2252e5c0b4e91e501ef9ada2eda3b16..779fcfd64119bdd5b4f8be740f7e8336c652b4d3 100644
--- a/samples/rust/rust_misc_device.rs
+++ b/samples/rust/rust_misc_device.rs
@@ -136,7 +136,7 @@ fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
         };
 
         try_pin_init!(Self {
-            _miscdev <- MiscDeviceRegistration::register(options),
+            _miscdev <- MiscDeviceRegistration::register(options, ()),
         })
     }
 }
@@ -156,6 +156,8 @@ struct RustMiscDevice {
 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());
 

-- 
2.48.1

[PATCH v2 3/3] rust: miscdevice: adjust the rust_misc_device sample to use RegistrationData.

[Christian Schrefl]

Add a second mutex to the RustMiscDevice, which is shared between all
instances of the device using an Arc and the RegistrationData of
MiscDeviceRegistration.

This is mostly to demonstrate the capability to share data in this way.

Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
---
 samples/rust/rust_misc_device.rs | 117 ++++++++++++++++++++++++++++++++++++---
 1 file changed, 108 insertions(+), 9 deletions(-)

diff --git a/samples/rust/rust_misc_device.rs b/samples/rust/rust_misc_device.rs
index 779fcfd64119bdd5b4f8be740f7e8336c652b4d3..050910400c6f05024625136a66ffbeec21d654e8 100644
--- a/samples/rust/rust_misc_device.rs
+++ b/samples/rust/rust_misc_device.rs
@@ -86,6 +86,62 @@
 ///     return -1;
 ///   }
 ///
+///   value++;
+///
+///   // Set shared value to something different
+///   printf("Submitting new shared value (%d)\n", value);
+///   ret = ioctl(fd, RUST_MISC_DEV_SET_SHARED_VALUE, &value);
+///   if (ret < 0) {
+///     perror("ioctl: Failed to submit new value");
+///     close(fd);
+///     return errno;
+///   }
+///
+///   // Close the device file
+///   printf("Closing /dev/rust-misc-device\n");
+///   close(fd);
+///
+///   // Open the device file again
+///   printf("Opening /dev/rust-misc-device again for reading\n");
+///   fd = open("/dev/rust-misc-device", O_RDWR);
+///   if (fd < 0) {
+///     perror("open");
+///     return errno;
+///   }
+///
+///   // Ensure new value was applied
+///   printf("Fetching new value\n");
+///   ret = ioctl(fd, RUST_MISC_DEV_GET_SHARED_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;
+///   }
+///
+///   value = 0;
+///   // Ensure non-shared value is still 0
+///   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;
+///   }
+///
 ///   // Close the device file
 ///   printf("Closing /dev/rust-misc-device\n");
 ///   close(fd);
@@ -104,7 +160,7 @@
     miscdevice::{MiscDevice, MiscDeviceOptions, MiscDeviceRegistration},
     new_mutex,
     prelude::*,
-    sync::Mutex,
+    sync::{Arc, Mutex},
     types::ARef,
     uaccess::{UserSlice, UserSliceReader, UserSliceWriter},
 };
@@ -112,6 +168,8 @@
 const RUST_MISC_DEV_HELLO: u32 = _IO('|' as u32, 0x80);
 const RUST_MISC_DEV_GET_VALUE: u32 = _IOR::<i32>('|' as u32, 0x81);
 const RUST_MISC_DEV_SET_VALUE: u32 = _IOW::<i32>('|' as u32, 0x82);
+const RUST_MISC_DEV_GET_SHARED_VALUE: u32 = _IOR::<i32>('|' as u32, 0x83);
+const RUST_MISC_DEV_SET_SHARED_VALUE: u32 = _IOW::<i32>('|' as u32, 0x84);
 
 module! {
     type: RustMiscDeviceModule,
@@ -129,14 +187,17 @@ struct RustMiscDeviceModule {
 
 impl kernel::InPlaceModule for RustMiscDeviceModule {
     fn init(_module: &'static ThisModule) -> impl PinInit<Self, Error> {
-        pr_info!("Initialising Rust Misc Device Sample\n");
+        pr_info!("Initializing Rust Misc Device Sample\n");
 
         let options = MiscDeviceOptions {
             name: c_str!("rust-misc-device"),
         };
 
         try_pin_init!(Self {
-            _miscdev <- MiscDeviceRegistration::register(options, ()),
+            _miscdev <- MiscDeviceRegistration::register(
+                options,
+                Arc::pin_init(new_mutex!(Inner { value: 0_i32 }), GFP_KERNEL)?
+            ),
         })
     }
 }
@@ -147,8 +208,9 @@ struct Inner {
 
 #[pin_data(PinnedDrop)]
 struct RustMiscDevice {
+    shared: Arc<Mutex<Inner>>,
     #[pin]
-    inner: Mutex<Inner>,
+    unique: Mutex<Inner>,
     dev: ARef<Device>,
 }
 
@@ -156,7 +218,7 @@ struct RustMiscDevice {
 impl MiscDevice for RustMiscDevice {
     type Ptr = Pin<KBox<Self>>;
 
-    type RegistrationData = ();
+    type RegistrationData = Arc<Mutex<Inner>>;
 
     fn open(_file: &File, misc: &MiscDeviceRegistration<Self>) -> Result<Pin<KBox<Self>>> {
         let dev = ARef::from(misc.device());
@@ -166,7 +228,8 @@ fn open(_file: &File, misc: &MiscDeviceRegistration<Self>) -> Result<Pin<KBox<Se
         KBox::try_pin_init(
             try_pin_init! {
                 RustMiscDevice {
-                    inner <- new_mutex!( Inner{ value: 0_i32 } ),
+                    shared: misc.data().clone(),
+                    unique <- new_mutex!(Inner { value: 0_i32 }),
                     dev: dev,
                 }
             },
@@ -182,6 +245,12 @@ fn ioctl(me: Pin<&RustMiscDevice>, _file: &File, cmd: u32, arg: usize) -> Result
         match cmd {
             RUST_MISC_DEV_GET_VALUE => me.get_value(UserSlice::new(arg, size).writer())?,
             RUST_MISC_DEV_SET_VALUE => me.set_value(UserSlice::new(arg, size).reader())?,
+            RUST_MISC_DEV_GET_SHARED_VALUE => {
+                me.get_shared_value(UserSlice::new(arg, size).writer())?
+            }
+            RUST_MISC_DEV_SET_SHARED_VALUE => {
+                me.set_shared_value(UserSlice::new(arg, size).reader())?
+            }
             RUST_MISC_DEV_HELLO => me.hello()?,
             _ => {
                 dev_err!(me.dev, "-> IOCTL not recognised: {}\n", cmd);
@@ -192,7 +261,6 @@ fn ioctl(me: Pin<&RustMiscDevice>, _file: &File, cmd: u32, arg: usize) -> Result
         Ok(0)
     }
 }
-
 #[pinned_drop]
 impl PinnedDrop for RustMiscDevice {
     fn drop(self: Pin<&mut Self>) {
@@ -203,7 +271,7 @@ fn drop(self: Pin<&mut Self>) {
 impl RustMiscDevice {
     fn set_value(&self, mut reader: UserSliceReader) -> Result<isize> {
         let new_value = reader.read::<i32>()?;
-        let mut guard = self.inner.lock();
+        let mut guard = self.unique.lock();
 
         dev_info!(
             self.dev,
@@ -216,7 +284,38 @@ fn set_value(&self, mut reader: UserSliceReader) -> Result<isize> {
     }
 
     fn get_value(&self, mut writer: UserSliceWriter) -> Result<isize> {
-        let guard = self.inner.lock();
+        let guard = self.unique.lock();
+        let value = guard.value;
+
+        // Free-up the lock and use our locally cached instance from here
+        drop(guard);
+
+        dev_info!(
+            self.dev,
+            "-> Copying data to userspace (value: {})\n",
+            &value
+        );
+
+        writer.write::<i32>(&value)?;
+        Ok(0)
+    }
+
+    fn set_shared_value(&self, mut reader: UserSliceReader) -> Result<isize> {
+        let new_value = reader.read::<i32>()?;
+        let mut guard = self.shared.lock();
+
+        dev_info!(
+            self.dev,
+            "-> Copying data from userspace (value: {})\n",
+            new_value
+        );
+
+        guard.value = new_value;
+        Ok(0)
+    }
+
+    fn get_shared_value(&self, mut writer: UserSliceWriter) -> Result<isize> {
+        let guard = self.shared.lock();
         let value = guard.value;
 
         // Free-up the lock and use our locally cached instance from here

-- 
2.48.1

Re: [PATCH v2 1/3] rust: add UnsafePinned type

[Benno Lossin]

On Fri Jan 31, 2025 at 4:08 PM CET, Christian Schrefl wrote:
> @@ -573,3 +576,57 @@ pub enum Either<L, R> {
>  /// [`NotThreadSafe`]: type@NotThreadSafe
>  #[allow(non_upper_case_globals)]
>  pub const NotThreadSafe: NotThreadSafe = PhantomData;
> +
> +/// Stores a value that may be used from multiple mutable pointers.
> +///
> +/// `UnsafePinned` gets rid of some of the usual assumptions that Rust has for a value:
> +/// - The value is allowed to be mutated, when a `&UnsafePinned<T>` exists on the Rust side.
> +/// - No uniqueness for mutable references: it is fine to have multiple `&mut UnsafePinned<T>`
> +///   point to the same value.

We have another patch series [1] in transit that changes the wording on
the `Opaque<T>` type. I think we should mirror the same wording here.

[1]: https://lore.kernel.org/rust-for-linux/20250305053438.1532397-2-dirk.behme@de.bosch.com/

> +///
> +/// To avoid the ability to use [`core::mem::swap`] this still needs to be used through a

IIRC typing out the whole path makes it also appear in the docs. I think
we should just use `mem::swap` and omit `core` of course you need to
add this to make it work:
    
    /// [`mem::swap`]: core::mem::swap

> +/// [`core::pin::Pin`] reference.

Here, I would link to `Pin`, but say "[pinned](core::pin::Pin) pointer."
instead, since eg `Pin<Box<T>>` also is okay to use.

> +///
> +/// This is useful for cases where a value might be shared with C code
> +/// but not interpreted by it or in cases where it can not always be guaranteed that the
> +/// references are unique.
> +///
> +/// This is similar to [`Opaque<T>`] but is guaranteed to always contain valid data and will
> +/// call the [`Drop`] implementation of `T` when dropped.
> +#[repr(transparent)]
> +pub struct UnsafePinned<T> {
> +    value: UnsafeCell<T>,
> +    _pin: PhantomPinned,
> +}
> +
> +impl<T> UnsafePinned<T> {
> +    /// Creates a new [`UnsafePinned`] value.
> +    pub const fn new(value: T) -> Self {
> +        Self {
> +            value: UnsafeCell::new(value),
> +            _pin: PhantomPinned,
> +        }
> +    }
> +
> +    /// Create an [`UnsafePinned`] pin-initializer from the given pin-initializer.
> +    pub fn try_pin_init<E>(value: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> +        // SAFETY:
> +        //   - In case of an error in `value` the error is returned, otherwise `slot` is fully
> +        //     initialized, since `self.value` is initialized and `_pin` is a zero sized type.
> +        //   - The `Pin` invariants of `self.value` are upheld, since no moving occurs.
> +        unsafe { init::pin_init_from_closure(move |slot| value.__pinned_init(Self::raw_get(slot))) }

Ah this is a bit suboptimal, but I guess there currently isn't a better
way to do this. I'll add it to my list of things to improve with
pin-init.

> +    }
> +
> +    /// Returns a raw pointer to the contained data.
> +    pub const fn get(&self) -> *mut T {
> +        UnsafeCell::get(&self.value).cast::<T>()
> +    }
> +
> +    /// Gets the value behind `this`.
> +    ///
> +    /// This function is useful to get access to the value without creating intermediate
> +    /// references.
> +    pub const fn raw_get(this: *const Self) -> *mut T {
> +        UnsafeCell::raw_get(this.cast::<UnsafeCell<MaybeUninit<T>>>()).cast::<T>()

Why the cast to `MaybeUninit<T>`? I think this can just be:

    UnsafeCell::raw_get(&raw const this.value)

---
Cheers,
Benno

> +    }
> +}

[PATCH v2 1/3] rust: add UnsafePinned type

[Christian Schrefl]

`UnsafePinned<T>` is useful for cases where a value might be shared with
C code but not directly used by it. In particular this is added for
storing additional data in the `MiscDeviceRegistration` which will be
shared between `fops->open` and the containing struct.

Similar to `Opaque` but guarantees that the value is always initialized
and that the inner value is dropped when `UnsafePinned` is dropped.

This was originally proposed for the IRQ abstractions [0] and is also
useful for other where the inner data may be aliased, but is always
valid and automatic `Drop` is desired.

Since then the `UnsafePinned` type was added to upstream Rust [1] by Sky
as a unstable feature, therefore this patch implements the subset of the
upstream API for the `UnsafePinned` type required for additional data in
`MiscDeviceRegistration` and in the implementation of the `Opaque` type.

Some differences to the upstream type definition are required in the
kernel implementation, because upstream type uses some compiler changes
to opt out of certain optimizations, this is documented in the
documentation and a comment on the `UnsafePinned` type.

The documentation on is based on the upstream rust documentation with
minor modifications for the kernel implementation.

Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
Link: https://github.com/rust-lang/rust/pull/137043 [1]
Suggested-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink>
Co-developed-by: Sky <sky@sky9.dev>
Signed-off-by: Sky <sky@sky9.dev>
Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
---
 init/Kconfig                       |   3 +
 rust/kernel/lib.rs                 |   1 +
 rust/kernel/types.rs               |   6 ++
 rust/kernel/types/unsafe_pinned.rs | 115 +++++++++++++++++++++++++++++++++++++
 4 files changed, 125 insertions(+)

diff --git a/init/Kconfig b/init/Kconfig
index 63f5974b9fa6ea3f5c92203cedd1f2f82aa468a1..727d85d2b59f555f1c33103bb78698551a41e7ca 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -140,6 +140,9 @@ config LD_CAN_USE_KEEP_IN_OVERLAY
 config RUSTC_HAS_COERCE_POINTEE
 	def_bool RUSTC_VERSION >= 108400
 
+config RUSTC_HAS_UNSAFE_PINNED
+	def_bool RUSTC_VERSION >= 108800
+
 config PAHOLE_VERSION
 	int
 	default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index de07aadd1ff5fe46fd89517e234b97a6590c8e93..c08f0a50f1d8db95799478caa8e85558a1fcae8d 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -17,6 +17,7 @@
 #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))]
 #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))]
 #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))]
+#![cfg_attr(CONFIG_RUSTC_HAS_UNSAFE_PINNED, feature(unsafe_pinned))]
 #![feature(inline_const)]
 #![feature(lint_reasons)]
 // Stable in Rust 1.82
diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
index 9d0471afc9648f2973235488b441eb109069adb1..705f420fdfbc4a576de1c4546578f2f04cdf615e 100644
--- a/rust/kernel/types.rs
+++ b/rust/kernel/types.rs
@@ -253,6 +253,9 @@ fn drop(&mut self) {
 ///
 /// [`Opaque<T>`] is meant to be used with FFI objects that are never interpreted by Rust code.
 ///
+/// In cases where the contained data is only used by Rust, is not allowed to be
+/// uninitialized and automatic [`Drop`] is desired [`UnsafePinned`] should be used instead.
+///
 /// It is used to wrap structs from the C side, like for example `Opaque<bindings::mutex>`.
 /// It gets rid of all the usual assumptions that Rust has for a value:
 ///
@@ -578,3 +581,6 @@ pub enum Either<L, R> {
 /// [`NotThreadSafe`]: type@NotThreadSafe
 #[allow(non_upper_case_globals)]
 pub const NotThreadSafe: NotThreadSafe = PhantomData;
+
+mod unsafe_pinned;
+pub use unsafe_pinned::UnsafePinned;
diff --git a/rust/kernel/types/unsafe_pinned.rs b/rust/kernel/types/unsafe_pinned.rs
new file mode 100644
index 0000000000000000000000000000000000000000..5a200aac30792bf71098087aee0fd9d2d51c468f
--- /dev/null
+++ b/rust/kernel/types/unsafe_pinned.rs
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: Apache-2.0 OR MIT
+
+//! The contents of this file partially come from the Rust standard library, hosted in
+//! the <https://github.com/rust-lang/rust> repository, licensed under
+//! "Apache-2.0 OR MIT" and adapted for kernel use. For copyright details,
+//! see <https://github.com/rust-lang/rust/blob/master/COPYRIGHT>.
+//!
+//! This file provides a implementation / polyfill of a subset of the upstream
+//! rust `UnsafePinned` type.
+
+use core::{cell::UnsafeCell, marker::PhantomPinned};
+use pin_init::{cast_pin_init, PinInit, Wrapper};
+
+/// This type provides a way to opt-out of typical aliasing rules;
+/// specifically, `&mut UnsafePinned<T>` is not guaranteed to be a unique pointer.
+///
+/// However, even if you define your type like `pub struct Wrapper(UnsafePinned<...>)`, it is still
+/// very risky to have an `&mut Wrapper` that aliases anything else. Many functions that work
+/// generically on `&mut T` assume that the memory that stores `T` is uniquely owned (such as
+/// `mem::swap`). In other words, while having aliasing with `&mut Wrapper` is not immediate
+/// Undefined Behavior, it is still unsound to expose such a mutable reference to code you do not
+/// control! Techniques such as pinning via [`Pin`](core::pin::Pin) are needed to ensure soundness.
+///
+/// Similar to [`UnsafeCell`], [`UnsafePinned`] will not usually show up in
+/// the public API of a library. It is an internal implementation detail of libraries that need to
+/// support aliasing mutable references.
+///
+/// Further note that this does *not* lift the requirement that shared references must be read-only!
+/// Use [`UnsafeCell`] for that.
+///
+/// This type blocks niches the same way [`UnsafeCell`] does.
+///
+/// # Kernel implementation notes
+///
+/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
+/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
+/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
+/// to future rust versions only this polyfill of this type should be used when this behavior is
+/// required.
+///
+/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
+/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
+/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).
+// As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
+// - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
+//      Required to use the `!Unpin hack`.
+// - `UnsafeCell<T>` instead of T to disallow niche optimizations,
+//     which is handled in the compiler in upstream Rust
+#[repr(transparent)]
+pub struct UnsafePinned<T: ?Sized> {
+    _ph: PhantomPinned,
+    value: UnsafeCell<T>,
+}
+
+impl<T> UnsafePinned<T> {
+    /// Constructs a new instance of [`UnsafePinned`] which will wrap the specified value.
+    ///
+    /// All access to the inner value through `&UnsafePinned<T>` or `&mut UnsafePinned<T>` or
+    /// `Pin<&mut UnsafePinned<T>>` requires `unsafe` code.
+    #[inline(always)]
+    #[must_use]
+    pub const fn new(value: T) -> Self {
+        UnsafePinned {
+            value: UnsafeCell::new(value),
+            _ph: PhantomPinned,
+        }
+    }
+}
+impl<T: ?Sized> UnsafePinned<T> {
+    /// Get read-only access to the contents of a shared `UnsafePinned`.
+    ///
+    /// Note that `&UnsafePinned<T>` is read-only if `&T` is read-only. This means that if there is
+    /// mutation of the `T`, future reads from the `*const T` returned here are UB! Use
+    /// [`UnsafeCell`] if you also need interior mutability.
+    ///
+    /// [`UnsafeCell`]: core::cell::UnsafeCell
+    ///
+    /// ```rust,no_build
+    /// use kernel::types::UnsafePinned;
+    ///
+    /// unsafe {
+    ///     let mut x = UnsafePinned::new(0);
+    ///     let ptr = x.get(); // read-only pointer, assumes immutability
+    ///     x.get_mut_unchecked().write(1);
+    ///     ptr.read(); // UB!
+    /// }
+    /// ```
+    ///
+    /// Note that the `get_mut_unchecked` function used by this example is
+    /// currently not implemented in the kernel implementation.
+    #[inline(always)]
+    #[must_use]
+    pub const fn get(&self) -> *const T {
+        self.value.get()
+    }
+
+    /// Gets a mutable pointer to the wrapped value.
+    ///
+    /// The difference from `get_mut_pinned` and `get_mut_unchecked` is that this function
+    /// accepts a raw pointer, which is useful to avoid the creation of temporary references.
+    ///
+    /// These functions mentioned here are currently not implemented in the kernel.
+    #[inline(always)]
+    #[must_use]
+    pub const fn raw_get_mut(this: *mut Self) -> *mut T {
+        this as *mut T
+    }
+}
+
+impl<T> Wrapper<T> for UnsafePinned<T> {
+    fn pin_init<E>(init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
+        // SAFETY: `UnsafePinned<T>` has a compatible layout to `T`.
+        unsafe { cast_pin_init(init) }
+    }
+}

-- 
2.49.0

Re: [PATCH v2 1/3] rust: add UnsafePinned type

[Alice Ryhl]

On Wed, Apr 30, 2025 at 10:36:11AM +0200, Christian Schrefl wrote:
> `UnsafePinned<T>` is useful for cases where a value might be shared with
> C code but not directly used by it. In particular this is added for
> storing additional data in the `MiscDeviceRegistration` which will be
> shared between `fops->open` and the containing struct.
> 
> Similar to `Opaque` but guarantees that the value is always initialized
> and that the inner value is dropped when `UnsafePinned` is dropped.
> 
> This was originally proposed for the IRQ abstractions [0] and is also
> useful for other where the inner data may be aliased, but is always
> valid and automatic `Drop` is desired.
> 
> Since then the `UnsafePinned` type was added to upstream Rust [1] by Sky
> as a unstable feature, therefore this patch implements the subset of the
> upstream API for the `UnsafePinned` type required for additional data in
> `MiscDeviceRegistration` and in the implementation of the `Opaque` type.
> 
> Some differences to the upstream type definition are required in the
> kernel implementation, because upstream type uses some compiler changes
> to opt out of certain optimizations, this is documented in the
> documentation and a comment on the `UnsafePinned` type.
> 
> The documentation on is based on the upstream rust documentation with
> minor modifications for the kernel implementation.
> 
> Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
> Link: https://github.com/rust-lang/rust/pull/137043 [1]
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink>
> Co-developed-by: Sky <sky@sky9.dev>
> Signed-off-by: Sky <sky@sky9.dev>
> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH v2 1/3] rust: add UnsafePinned type

[Alice Ryhl]

On Wed, Apr 30, 2025 at 10:36:11AM +0200, Christian Schrefl wrote:
> `UnsafePinned<T>` is useful for cases where a value might be shared with
> C code but not directly used by it. In particular this is added for
> storing additional data in the `MiscDeviceRegistration` which will be
> shared between `fops->open` and the containing struct.
> 
> Similar to `Opaque` but guarantees that the value is always initialized
> and that the inner value is dropped when `UnsafePinned` is dropped.
> 
> This was originally proposed for the IRQ abstractions [0] and is also
> useful for other where the inner data may be aliased, but is always
> valid and automatic `Drop` is desired.
> 
> Since then the `UnsafePinned` type was added to upstream Rust [1] by Sky
> as a unstable feature, therefore this patch implements the subset of the
> upstream API for the `UnsafePinned` type required for additional data in
> `MiscDeviceRegistration` and in the implementation of the `Opaque` type.
> 
> Some differences to the upstream type definition are required in the
> kernel implementation, because upstream type uses some compiler changes
> to opt out of certain optimizations, this is documented in the
> documentation and a comment on the `UnsafePinned` type.
> 
> The documentation on is based on the upstream rust documentation with
> minor modifications for the kernel implementation.
> 
> Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
> Link: https://github.com/rust-lang/rust/pull/137043 [1]
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink>
> Co-developed-by: Sky <sky@sky9.dev>
> Signed-off-by: Sky <sky@sky9.dev>
> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>

> +config RUSTC_HAS_UNSAFE_PINNED
> +	def_bool RUSTC_VERSION >= 108800

> +#![cfg_attr(CONFIG_RUSTC_HAS_UNSAFE_PINNED, feature(unsafe_pinned))]

Actually, I missed this on the first look. Where is this feature used?
You only have a re-implementation right now.

Alice

Re: [PATCH v2 1/3] rust: add UnsafePinned type

[Benno Lossin]

On Wed Apr 30, 2025 at 10:36 AM CEST, Christian Schrefl wrote:
> `UnsafePinned<T>` is useful for cases where a value might be shared with
> C code but not directly used by it. In particular this is added for
> storing additional data in the `MiscDeviceRegistration` which will be
> shared between `fops->open` and the containing struct.
>
> Similar to `Opaque` but guarantees that the value is always initialized
> and that the inner value is dropped when `UnsafePinned` is dropped.
>
> This was originally proposed for the IRQ abstractions [0] and is also
> useful for other where the inner data may be aliased, but is always
> valid and automatic `Drop` is desired.
>
> Since then the `UnsafePinned` type was added to upstream Rust [1] by Sky
> as a unstable feature, therefore this patch implements the subset of the
> upstream API for the `UnsafePinned` type required for additional data in
> `MiscDeviceRegistration` and in the implementation of the `Opaque` type.
>
> Some differences to the upstream type definition are required in the
> kernel implementation, because upstream type uses some compiler changes
> to opt out of certain optimizations, this is documented in the
> documentation and a comment on the `UnsafePinned` type.
>
> The documentation on is based on the upstream rust documentation with
> minor modifications for the kernel implementation.
>
> Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
> Link: https://github.com/rust-lang/rust/pull/137043 [1]
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink>
> Co-developed-by: Sky <sky@sky9.dev>
> Signed-off-by: Sky <sky@sky9.dev>
> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
> ---
>  init/Kconfig                       |   3 +
>  rust/kernel/lib.rs                 |   1 +
>  rust/kernel/types.rs               |   6 ++
>  rust/kernel/types/unsafe_pinned.rs | 115 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 125 insertions(+)
>
> diff --git a/init/Kconfig b/init/Kconfig
> index 63f5974b9fa6ea3f5c92203cedd1f2f82aa468a1..727d85d2b59f555f1c33103bb78698551a41e7ca 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -140,6 +140,9 @@ config LD_CAN_USE_KEEP_IN_OVERLAY
>  config RUSTC_HAS_COERCE_POINTEE
>  	def_bool RUSTC_VERSION >= 108400
>  
> +config RUSTC_HAS_UNSAFE_PINNED
> +	def_bool RUSTC_VERSION >= 108800
> +
>  config PAHOLE_VERSION
>  	int
>  	default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index de07aadd1ff5fe46fd89517e234b97a6590c8e93..c08f0a50f1d8db95799478caa8e85558a1fcae8d 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -17,6 +17,7 @@
>  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))]
>  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))]
>  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))]
> +#![cfg_attr(CONFIG_RUSTC_HAS_UNSAFE_PINNED, feature(unsafe_pinned))]
>  #![feature(inline_const)]
>  #![feature(lint_reasons)]
>  // Stable in Rust 1.82
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 9d0471afc9648f2973235488b441eb109069adb1..705f420fdfbc4a576de1c4546578f2f04cdf615e 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -253,6 +253,9 @@ fn drop(&mut self) {
>  ///
>  /// [`Opaque<T>`] is meant to be used with FFI objects that are never interpreted by Rust code.
>  ///
> +/// In cases where the contained data is only used by Rust, is not allowed to be
> +/// uninitialized and automatic [`Drop`] is desired [`UnsafePinned`] should be used instead.
> +///
>  /// It is used to wrap structs from the C side, like for example `Opaque<bindings::mutex>`.
>  /// It gets rid of all the usual assumptions that Rust has for a value:
>  ///
> @@ -578,3 +581,6 @@ pub enum Either<L, R> {
>  /// [`NotThreadSafe`]: type@NotThreadSafe
>  #[allow(non_upper_case_globals)]
>  pub const NotThreadSafe: NotThreadSafe = PhantomData;
> +
> +mod unsafe_pinned;
> +pub use unsafe_pinned::UnsafePinned;
> diff --git a/rust/kernel/types/unsafe_pinned.rs b/rust/kernel/types/unsafe_pinned.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..5a200aac30792bf71098087aee0fd9d2d51c468f
> --- /dev/null
> +++ b/rust/kernel/types/unsafe_pinned.rs
> @@ -0,0 +1,115 @@
> +// SPDX-License-Identifier: Apache-2.0 OR MIT
> +
> +//! The contents of this file partially come from the Rust standard library, hosted in
> +//! the <https://github.com/rust-lang/rust> repository, licensed under
> +//! "Apache-2.0 OR MIT" and adapted for kernel use. For copyright details,
> +//! see <https://github.com/rust-lang/rust/blob/master/COPYRIGHT>.
> +//!
> +//! This file provides a implementation / polyfill of a subset of the upstream
> +//! rust `UnsafePinned` type.
> +
> +use core::{cell::UnsafeCell, marker::PhantomPinned};
> +use pin_init::{cast_pin_init, PinInit, Wrapper};
> +
> +/// This type provides a way to opt-out of typical aliasing rules;
> +/// specifically, `&mut UnsafePinned<T>` is not guaranteed to be a unique pointer.
> +///
> +/// However, even if you define your type like `pub struct Wrapper(UnsafePinned<...>)`, it is still
> +/// very risky to have an `&mut Wrapper` that aliases anything else. Many functions that work
> +/// generically on `&mut T` assume that the memory that stores `T` is uniquely owned (such as
> +/// `mem::swap`). In other words, while having aliasing with `&mut Wrapper` is not immediate
> +/// Undefined Behavior, it is still unsound to expose such a mutable reference to code you do not
> +/// control! Techniques such as pinning via [`Pin`](core::pin::Pin) are needed to ensure soundness.
> +///
> +/// Similar to [`UnsafeCell`], [`UnsafePinned`] will not usually show up in
> +/// the public API of a library. It is an internal implementation detail of libraries that need to
> +/// support aliasing mutable references.
> +///
> +/// Further note that this does *not* lift the requirement that shared references must be read-only!
> +/// Use [`UnsafeCell`] for that.
> +///
> +/// This type blocks niches the same way [`UnsafeCell`] does.
> +///
> +/// # Kernel implementation notes
> +///
> +/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
> +/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
> +/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
> +/// to future rust versions only this polyfill of this type should be used when this behavior is
> +/// required.
> +///
> +/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
> +/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
> +/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).

I would make this last paragraph a normal comment, I don't think we
should expose it in the docs.

> +// As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
> +// - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
> +//      Required to use the `!Unpin hack`.
> +// - `UnsafeCell<T>` instead of T to disallow niche optimizations,
> +//     which is handled in the compiler in upstream Rust
> +#[repr(transparent)]
> +pub struct UnsafePinned<T: ?Sized> {
> +    _ph: PhantomPinned,
> +    value: UnsafeCell<T>,
> +}
> +
> +impl<T> UnsafePinned<T> {
> +    /// Constructs a new instance of [`UnsafePinned`] which will wrap the specified value.
> +    ///
> +    /// All access to the inner value through `&UnsafePinned<T>` or `&mut UnsafePinned<T>` or
> +    /// `Pin<&mut UnsafePinned<T>>` requires `unsafe` code.
> +    #[inline(always)]
> +    #[must_use]
> +    pub const fn new(value: T) -> Self {
> +        UnsafePinned {
> +            value: UnsafeCell::new(value),
> +            _ph: PhantomPinned,
> +        }
> +    }
> +}
> +impl<T: ?Sized> UnsafePinned<T> {
> +    /// Get read-only access to the contents of a shared `UnsafePinned`.
> +    ///
> +    /// Note that `&UnsafePinned<T>` is read-only if `&T` is read-only. This means that if there is
> +    /// mutation of the `T`, future reads from the `*const T` returned here are UB! Use
> +    /// [`UnsafeCell`] if you also need interior mutability.

I agree with copy-pasting the docs from upstream, even though our
implementation already wraps the value in `UnsafeCell`, but I think we
should include a comment at the top of this doc that mentions this
difference. So something along the lines "In order to make replacing
this type with the upstream one, we want to have as little API
divergence as possible. Thus we don't mention the implementation detail
of `UnsafeCell` and people have to use `UnsafePinned<UnsafeCell<T>>`
instead of just `UnsafePinned<T>`." feel free to modify.

If we add that, maybe we don't need the comment about needing
`UnsafeCell` in `Opaque` after all (from my other mail).

> +    ///
> +    /// [`UnsafeCell`]: core::cell::UnsafeCell
> +    ///
> +    /// ```rust,no_build
> +    /// use kernel::types::UnsafePinned;
> +    ///
> +    /// unsafe {
> +    ///     let mut x = UnsafePinned::new(0);
> +    ///     let ptr = x.get(); // read-only pointer, assumes immutability
> +    ///     x.get_mut_unchecked().write(1);
> +    ///     ptr.read(); // UB!
> +    /// }
> +    /// ```
> +    ///
> +    /// Note that the `get_mut_unchecked` function used by this example is
> +    /// currently not implemented in the kernel implementation.
> +    #[inline(always)]
> +    #[must_use]
> +    pub const fn get(&self) -> *const T {
> +        self.value.get()
> +    }
> +
> +    /// Gets a mutable pointer to the wrapped value.
> +    ///
> +    /// The difference from `get_mut_pinned` and `get_mut_unchecked` is that this function
> +    /// accepts a raw pointer, which is useful to avoid the creation of temporary references.

You did not include the `get_mut_{pinned,unchecked}` methods, so
mentioning them here in the docs might confuse people. Do we want to
have those methods?

---
Cheers,
Benno

> +    ///
> +    /// These functions mentioned here are currently not implemented in the kernel.
> +    #[inline(always)]
> +    #[must_use]
> +    pub const fn raw_get_mut(this: *mut Self) -> *mut T {
> +        this as *mut T
> +    }
> +}
> +
> +impl<T> Wrapper<T> for UnsafePinned<T> {
> +    fn pin_init<E>(init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> +        // SAFETY: `UnsafePinned<T>` has a compatible layout to `T`.
> +        unsafe { cast_pin_init(init) }
> +    }
> +}

Re: [PATCH v2 1/3] rust: add UnsafePinned type

[Christian Schrefl]

On 30.04.25 11:19 AM, Alice Ryhl wrote:
> On Wed, Apr 30, 2025 at 10:36:11AM +0200, Christian Schrefl wrote:
>> `UnsafePinned<T>` is useful for cases where a value might be shared with
>> C code but not directly used by it. In particular this is added for
>> storing additional data in the `MiscDeviceRegistration` which will be
>> shared between `fops->open` and the containing struct.
>>
>> Similar to `Opaque` but guarantees that the value is always initialized
>> and that the inner value is dropped when `UnsafePinned` is dropped.
>>
>> This was originally proposed for the IRQ abstractions [0] and is also
>> useful for other where the inner data may be aliased, but is always
>> valid and automatic `Drop` is desired.
>>
>> Since then the `UnsafePinned` type was added to upstream Rust [1] by Sky
>> as a unstable feature, therefore this patch implements the subset of the
>> upstream API for the `UnsafePinned` type required for additional data in
>> `MiscDeviceRegistration` and in the implementation of the `Opaque` type.
>>
>> Some differences to the upstream type definition are required in the
>> kernel implementation, because upstream type uses some compiler changes
>> to opt out of certain optimizations, this is documented in the
>> documentation and a comment on the `UnsafePinned` type.
>>
>> The documentation on is based on the upstream rust documentation with
>> minor modifications for the kernel implementation.
>>
>> Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
>> Link: https://github.com/rust-lang/rust/pull/137043 [1]
>> Suggested-by: Alice Ryhl <aliceryhl@google.com>
>> Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink>
>> Co-developed-by: Sky <sky@sky9.dev>
>> Signed-off-by: Sky <sky@sky9.dev>
>> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
> 
>> +config RUSTC_HAS_UNSAFE_PINNED
>> +	def_bool RUSTC_VERSION >= 108800
> 
>> +#![cfg_attr(CONFIG_RUSTC_HAS_UNSAFE_PINNED, feature(unsafe_pinned))]
> 
> Actually, I missed this on the first look. Where is this feature used?
> You only have a re-implementation right now.
Was just a leftover from the previous version, removing the feature and
config flag (to be reintroduced once we want to use the upstream version).

Cheers
Christian

Re: [PATCH v2 1/3] rust: add UnsafePinned type

[Christian Schrefl]

On 30.04.25 11:45 AM, Benno Lossin wrote:
> On Wed Apr 30, 2025 at 10:36 AM CEST, Christian Schrefl wrote:
>> `UnsafePinned<T>` is useful for cases where a value might be shared with
>> C code but not directly used by it. In particular this is added for
>> storing additional data in the `MiscDeviceRegistration` which will be
>> shared between `fops->open` and the containing struct.
>>
>> Similar to `Opaque` but guarantees that the value is always initialized
>> and that the inner value is dropped when `UnsafePinned` is dropped.
>>
>> This was originally proposed for the IRQ abstractions [0] and is also
>> useful for other where the inner data may be aliased, but is always
>> valid and automatic `Drop` is desired.
>>
>> Since then the `UnsafePinned` type was added to upstream Rust [1] by Sky
>> as a unstable feature, therefore this patch implements the subset of the
>> upstream API for the `UnsafePinned` type required for additional data in
>> `MiscDeviceRegistration` and in the implementation of the `Opaque` type.
>>
>> Some differences to the upstream type definition are required in the
>> kernel implementation, because upstream type uses some compiler changes
>> to opt out of certain optimizations, this is documented in the
>> documentation and a comment on the `UnsafePinned` type.
>>
>> The documentation on is based on the upstream rust documentation with
>> minor modifications for the kernel implementation.
>>
>> Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
>> Link: https://github.com/rust-lang/rust/pull/137043 [1]
>> Suggested-by: Alice Ryhl <aliceryhl@google.com>
>> Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink>
>> Co-developed-by: Sky <sky@sky9.dev>
>> Signed-off-by: Sky <sky@sky9.dev>
>> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
>> ---
>>  init/Kconfig                       |   3 +
>>  rust/kernel/lib.rs                 |   1 +
>>  rust/kernel/types.rs               |   6 ++
>>  rust/kernel/types/unsafe_pinned.rs | 115 +++++++++++++++++++++++++++++++++++++
>>  4 files changed, 125 insertions(+)
>>
>> diff --git a/init/Kconfig b/init/Kconfig
>> index 63f5974b9fa6ea3f5c92203cedd1f2f82aa468a1..727d85d2b59f555f1c33103bb78698551a41e7ca 100644
>> --- a/init/Kconfig
>> +++ b/init/Kconfig
>> @@ -140,6 +140,9 @@ config LD_CAN_USE_KEEP_IN_OVERLAY
>>  config RUSTC_HAS_COERCE_POINTEE
>>  	def_bool RUSTC_VERSION >= 108400
>>  
>> +config RUSTC_HAS_UNSAFE_PINNED
>> +	def_bool RUSTC_VERSION >= 108800
>> +
>>  config PAHOLE_VERSION
>>  	int
>>  	default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>> index de07aadd1ff5fe46fd89517e234b97a6590c8e93..c08f0a50f1d8db95799478caa8e85558a1fcae8d 100644
>> --- a/rust/kernel/lib.rs
>> +++ b/rust/kernel/lib.rs
>> @@ -17,6 +17,7 @@
>>  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))]
>>  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))]
>>  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))]
>> +#![cfg_attr(CONFIG_RUSTC_HAS_UNSAFE_PINNED, feature(unsafe_pinned))]
>>  #![feature(inline_const)]
>>  #![feature(lint_reasons)]
>>  // Stable in Rust 1.82
>> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
>> index 9d0471afc9648f2973235488b441eb109069adb1..705f420fdfbc4a576de1c4546578f2f04cdf615e 100644
>> --- a/rust/kernel/types.rs
>> +++ b/rust/kernel/types.rs
>> @@ -253,6 +253,9 @@ fn drop(&mut self) {
>>  ///
>>  /// [`Opaque<T>`] is meant to be used with FFI objects that are never interpreted by Rust code.
>>  ///
>> +/// In cases where the contained data is only used by Rust, is not allowed to be
>> +/// uninitialized and automatic [`Drop`] is desired [`UnsafePinned`] should be used instead.
>> +///
>>  /// It is used to wrap structs from the C side, like for example `Opaque<bindings::mutex>`.
>>  /// It gets rid of all the usual assumptions that Rust has for a value:
>>  ///
>> @@ -578,3 +581,6 @@ pub enum Either<L, R> {
>>  /// [`NotThreadSafe`]: type@NotThreadSafe
>>  #[allow(non_upper_case_globals)]
>>  pub const NotThreadSafe: NotThreadSafe = PhantomData;
>> +
>> +mod unsafe_pinned;
>> +pub use unsafe_pinned::UnsafePinned;
>> diff --git a/rust/kernel/types/unsafe_pinned.rs b/rust/kernel/types/unsafe_pinned.rs
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..5a200aac30792bf71098087aee0fd9d2d51c468f
>> --- /dev/null
>> +++ b/rust/kernel/types/unsafe_pinned.rs
>> @@ -0,0 +1,115 @@
>> +// SPDX-License-Identifier: Apache-2.0 OR MIT
>> +
>> +//! The contents of this file partially come from the Rust standard library, hosted in
>> +//! the <https://github.com/rust-lang/rust> repository, licensed under
>> +//! "Apache-2.0 OR MIT" and adapted for kernel use. For copyright details,
>> +//! see <https://github.com/rust-lang/rust/blob/master/COPYRIGHT>.
>> +//!
>> +//! This file provides a implementation / polyfill of a subset of the upstream
>> +//! rust `UnsafePinned` type.
>> +
>> +use core::{cell::UnsafeCell, marker::PhantomPinned};
>> +use pin_init::{cast_pin_init, PinInit, Wrapper};
>> +
>> +/// This type provides a way to opt-out of typical aliasing rules;
>> +/// specifically, `&mut UnsafePinned<T>` is not guaranteed to be a unique pointer.
>> +///
>> +/// However, even if you define your type like `pub struct Wrapper(UnsafePinned<...>)`, it is still
>> +/// very risky to have an `&mut Wrapper` that aliases anything else. Many functions that work
>> +/// generically on `&mut T` assume that the memory that stores `T` is uniquely owned (such as
>> +/// `mem::swap`). In other words, while having aliasing with `&mut Wrapper` is not immediate
>> +/// Undefined Behavior, it is still unsound to expose such a mutable reference to code you do not
>> +/// control! Techniques such as pinning via [`Pin`](core::pin::Pin) are needed to ensure soundness.
>> +///
>> +/// Similar to [`UnsafeCell`], [`UnsafePinned`] will not usually show up in
>> +/// the public API of a library. It is an internal implementation detail of libraries that need to
>> +/// support aliasing mutable references.
>> +///
>> +/// Further note that this does *not* lift the requirement that shared references must be read-only!
>> +/// Use [`UnsafeCell`] for that.
>> +///
>> +/// This type blocks niches the same way [`UnsafeCell`] does.
>> +///
>> +/// # Kernel implementation notes
>> +///
>> +/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
>> +/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
>> +/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
>> +/// to future rust versions only this polyfill of this type should be used when this behavior is
>> +/// required.
>> +///
>> +/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
>> +/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
>> +/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).
> 
> I would make this last paragraph a normal comment, I don't think we
> should expose it in the docs.

I added this as docs since I wanted it to be a bit more visible,
but I can replace the comment text (about `UnsafeCell`) with this paragraph
and drop it from the docs if you want.
 
>> +// As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
>> +// - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
>> +//      Required to use the `!Unpin hack`.
>> +// - `UnsafeCell<T>` instead of T to disallow niche optimizations,
>> +//     which is handled in the compiler in upstream Rust
>> +#[repr(transparent)]
>> +pub struct UnsafePinned<T: ?Sized> {
>> +    _ph: PhantomPinned,
>> +    value: UnsafeCell<T>,
>> +}
>> +
>> +impl<T> UnsafePinned<T> {
>> +    /// Constructs a new instance of [`UnsafePinned`] which will wrap the specified value.
>> +    ///
>> +    /// All access to the inner value through `&UnsafePinned<T>` or `&mut UnsafePinned<T>` or
>> +    /// `Pin<&mut UnsafePinned<T>>` requires `unsafe` code.
>> +    #[inline(always)]
>> +    #[must_use]
>> +    pub const fn new(value: T) -> Self {
>> +        UnsafePinned {
>> +            value: UnsafeCell::new(value),
>> +            _ph: PhantomPinned,
>> +        }
>> +    }
>> +}
>> +impl<T: ?Sized> UnsafePinned<T> {
>> +    /// Get read-only access to the contents of a shared `UnsafePinned`.
>> +    ///
>> +    /// Note that `&UnsafePinned<T>` is read-only if `&T` is read-only. This means that if there is
>> +    /// mutation of the `T`, future reads from the `*const T` returned here are UB! Use
>> +    /// [`UnsafeCell`] if you also need interior mutability.
> 
> I agree with copy-pasting the docs from upstream, even though our
> implementation already wraps the value in `UnsafeCell`, but I think we
> should include a comment at the top of this doc that mentions this
> difference. So something along the lines "In order to make replacing
> this type with the upstream one, we want to have as little API
> divergence as possible. Thus we don't mention the implementation detail
> of `UnsafeCell` and people have to use `UnsafePinned<UnsafeCell<T>>`
> instead of just `UnsafePinned<T>`." feel free to modify.
> 

I already wrote about this in comments (and documentation in this version)
on the `UnsafePinned` type definition.

I'm not sure where exactly we want to have this, but I think having it
at the top of the file and on the type definition is a bit redundant.

>
> If we add that, maybe we don't need the comment about needing
> `UnsafeCell` in `Opaque` after all (from my other mail).

I think I should still add a comment there.

> 
>> +    ///
>> +    /// [`UnsafeCell`]: core::cell::UnsafeCell
>> +    ///
>> +    /// ```rust,no_build
>> +    /// use kernel::types::UnsafePinned;
>> +    ///
>> +    /// unsafe {
>> +    ///     let mut x = UnsafePinned::new(0);
>> +    ///     let ptr = x.get(); // read-only pointer, assumes immutability
>> +    ///     x.get_mut_unchecked().write(1);
>> +    ///     ptr.read(); // UB!
>> +    /// }
>> +    /// ```
>> +    ///
>> +    /// Note that the `get_mut_unchecked` function used by this example is
>> +    /// currently not implemented in the kernel implementation.
>> +    #[inline(always)]
>> +    #[must_use]
>> +    pub const fn get(&self) -> *const T {
>> +        self.value.get()
>> +    }
>> +
>> +    /// Gets a mutable pointer to the wrapped value.
>> +    ///
>> +    /// The difference from `get_mut_pinned` and `get_mut_unchecked` is that this function
>> +    /// accepts a raw pointer, which is useful to avoid the creation of temporary references.
> 
> You did not include the `get_mut_{pinned,unchecked}` methods, so
> mentioning them here in the docs might confuse people. Do we want to
> have those methods?

I only included the functions that we needed for `Opaque` and my
`miscdevice' patches. I think these functions should only be added
once they have a user. That's why I wrote the next sentence in the
documents.

Should I handle this differently?

It should be a really simple patch to add these functions and I can
do that if someone needs them or I can just include them in this
patch set.

Cheers
Christian

> 
> ---
> Cheers,
> Benno
> 
>> +    ///
>> +    /// These functions mentioned here are currently not implemented in the kernel.
>> +    #[inline(always)]
>> +    #[must_use]
>> +    pub const fn raw_get_mut(this: *mut Self) -> *mut T {
>> +        this as *mut T
>> +    }
>> +}
>> +
>> +impl<T> Wrapper<T> for UnsafePinned<T> {
>> +    fn pin_init<E>(init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
>> +        // SAFETY: `UnsafePinned<T>` has a compatible layout to `T`.
>> +        unsafe { cast_pin_init(init) }
>> +    }
>> +}
> 

Re: [PATCH v2 1/3] rust: add UnsafePinned type

[Christian Schrefl]

On 30.04.25 10:36 AM, Christian Schrefl wrote:
> `UnsafePinned<T>` is useful for cases where a value might be shared with
> C code but not directly used by it. In particular this is added for
> storing additional data in the `MiscDeviceRegistration` which will be
> shared between `fops->open` and the containing struct.
> 
> Similar to `Opaque` but guarantees that the value is always initialized
> and that the inner value is dropped when `UnsafePinned` is dropped.
> 
> This was originally proposed for the IRQ abstractions [0] and is also
> useful for other where the inner data may be aliased, but is always
> valid and automatic `Drop` is desired.
> 
> Since then the `UnsafePinned` type was added to upstream Rust [1] by Sky
> as a unstable feature, therefore this patch implements the subset of the
> upstream API for the `UnsafePinned` type required for additional data in
> `MiscDeviceRegistration` and in the implementation of the `Opaque` type.
> 
> Some differences to the upstream type definition are required in the
> kernel implementation, because upstream type uses some compiler changes
> to opt out of certain optimizations, this is documented in the
> documentation and a comment on the `UnsafePinned` type.
> 
> The documentation on is based on the upstream rust documentation with
> minor modifications for the kernel implementation.
> 
> Link: https://lore.kernel.org/rust-for-linux/CAH5fLgiOASgjoYKFz6kWwzLaH07DqP2ph+3YyCDh2+gYqGpABA@mail.gmail.com [0]
> Link: https://github.com/rust-lang/rust/pull/137043 [1]
> Suggested-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Gerald Wisböck <gerald.wisboeck@feather.ink>
> Co-developed-by: Sky <sky@sky9.dev>
> Signed-off-by: Sky <sky@sky9.dev>
> Signed-off-by: Christian Schrefl <chrisi.schrefl@gmail.com>
> ---
>  init/Kconfig                       |   3 +
>  rust/kernel/lib.rs                 |   1 +
>  rust/kernel/types.rs               |   6 ++
>  rust/kernel/types/unsafe_pinned.rs | 115 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 125 insertions(+)
> 
> diff --git a/init/Kconfig b/init/Kconfig
> index 63f5974b9fa6ea3f5c92203cedd1f2f82aa468a1..727d85d2b59f555f1c33103bb78698551a41e7ca 100644
> --- a/init/Kconfig
> +++ b/init/Kconfig
> @@ -140,6 +140,9 @@ config LD_CAN_USE_KEEP_IN_OVERLAY
>  config RUSTC_HAS_COERCE_POINTEE
>  	def_bool RUSTC_VERSION >= 108400
>  
> +config RUSTC_HAS_UNSAFE_PINNED
> +	def_bool RUSTC_VERSION >= 108800
> +
>  config PAHOLE_VERSION
>  	int
>  	default $(shell,$(srctree)/scripts/pahole-version.sh $(PAHOLE))
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index de07aadd1ff5fe46fd89517e234b97a6590c8e93..c08f0a50f1d8db95799478caa8e85558a1fcae8d 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -17,6 +17,7 @@
>  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(coerce_unsized))]
>  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(dispatch_from_dyn))]
>  #![cfg_attr(not(CONFIG_RUSTC_HAS_COERCE_POINTEE), feature(unsize))]
> +#![cfg_attr(CONFIG_RUSTC_HAS_UNSAFE_PINNED, feature(unsafe_pinned))]
>  #![feature(inline_const)]
>  #![feature(lint_reasons)]
>  // Stable in Rust 1.82
> diff --git a/rust/kernel/types.rs b/rust/kernel/types.rs
> index 9d0471afc9648f2973235488b441eb109069adb1..705f420fdfbc4a576de1c4546578f2f04cdf615e 100644
> --- a/rust/kernel/types.rs
> +++ b/rust/kernel/types.rs
> @@ -253,6 +253,9 @@ fn drop(&mut self) {
>  ///
>  /// [`Opaque<T>`] is meant to be used with FFI objects that are never interpreted by Rust code.
>  ///
> +/// In cases where the contained data is only used by Rust, is not allowed to be
> +/// uninitialized and automatic [`Drop`] is desired [`UnsafePinned`] should be used instead.
> +///
>  /// It is used to wrap structs from the C side, like for example `Opaque<bindings::mutex>`.
>  /// It gets rid of all the usual assumptions that Rust has for a value:
>  ///
> @@ -578,3 +581,6 @@ pub enum Either<L, R> {
>  /// [`NotThreadSafe`]: type@NotThreadSafe
>  #[allow(non_upper_case_globals)]
>  pub const NotThreadSafe: NotThreadSafe = PhantomData;
> +
> +mod unsafe_pinned;
> +pub use unsafe_pinned::UnsafePinned;
> diff --git a/rust/kernel/types/unsafe_pinned.rs b/rust/kernel/types/unsafe_pinned.rs
> new file mode 100644
> index 0000000000000000000000000000000000000000..5a200aac30792bf71098087aee0fd9d2d51c468f
> --- /dev/null
> +++ b/rust/kernel/types/unsafe_pinned.rs
> @@ -0,0 +1,115 @@
> +// SPDX-License-Identifier: Apache-2.0 OR MIT
> +
> +//! The contents of this file partially come from the Rust standard library, hosted in
> +//! the <https://github.com/rust-lang/rust> repository, licensed under
> +//! "Apache-2.0 OR MIT" and adapted for kernel use. For copyright details,
> +//! see <https://github.com/rust-lang/rust/blob/master/COPYRIGHT>.
> +//!
> +//! This file provides a implementation / polyfill of a subset of the upstream
> +//! rust `UnsafePinned` type.
> +
> +use core::{cell::UnsafeCell, marker::PhantomPinned};
> +use pin_init::{cast_pin_init, PinInit, Wrapper};
> +
> +/// This type provides a way to opt-out of typical aliasing rules;
> +/// specifically, `&mut UnsafePinned<T>` is not guaranteed to be a unique pointer.
> +///
> +/// However, even if you define your type like `pub struct Wrapper(UnsafePinned<...>)`, it is still
> +/// very risky to have an `&mut Wrapper` that aliases anything else. Many functions that work
> +/// generically on `&mut T` assume that the memory that stores `T` is uniquely owned (such as
> +/// `mem::swap`). In other words, while having aliasing with `&mut Wrapper` is not immediate
> +/// Undefined Behavior, it is still unsound to expose such a mutable reference to code you do not
> +/// control! Techniques such as pinning via [`Pin`](core::pin::Pin) are needed to ensure soundness.
> +///
> +/// Similar to [`UnsafeCell`], [`UnsafePinned`] will not usually show up in
> +/// the public API of a library. It is an internal implementation detail of libraries that need to
> +/// support aliasing mutable references.
> +///
> +/// Further note that this does *not* lift the requirement that shared references must be read-only!
> +/// Use [`UnsafeCell`] for that.

[CC Ralf]

Ralf has replied to me on Github that this will most likely change [0]. How should this be handled?

I would fine with submitting a patch once it changes on the rust side (possibly waiting until the
feature is close to stabilization). I think it is better to only add this guarantee later as it
will be easier to remove unnecessary `UnsafeCell`s than it would be to later add them back in ever
case where they would be needed (in case rust doesn't change `UnsafePinned` to act like `UnsafeCell`).

Also see to the tracking issue [1] for the reason why `UnsafeCell` behavior is most likely required.

[0]: https://github.com/rust-lang/rust/issues/125735#issuecomment-2842926832
[1]: https://github.com/rust-lang/rust/issues/137750

Cheers
Christian

> +///
> +/// This type blocks niches the same way [`UnsafeCell`] does.
> +///
> +/// # Kernel implementation notes
> +///
> +/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
> +/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
> +/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
> +/// to future rust versions only this polyfill of this type should be used when this behavior is
> +/// required.
> +///
> +/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
> +/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
> +/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).
> +// As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
> +// - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
> +//      Required to use the `!Unpin hack`.
> +// - `UnsafeCell<T>` instead of T to disallow niche optimizations,
> +//     which is handled in the compiler in upstream Rust
> +#[repr(transparent)]
> +pub struct UnsafePinned<T: ?Sized> {
> +    _ph: PhantomPinned,
> +    value: UnsafeCell<T>,
> +}
> +
> +impl<T> UnsafePinned<T> {
> +    /// Constructs a new instance of [`UnsafePinned`] which will wrap the specified value.
> +    ///
> +    /// All access to the inner value through `&UnsafePinned<T>` or `&mut UnsafePinned<T>` or
> +    /// `Pin<&mut UnsafePinned<T>>` requires `unsafe` code.
> +    #[inline(always)]
> +    #[must_use]
> +    pub const fn new(value: T) -> Self {
> +        UnsafePinned {
> +            value: UnsafeCell::new(value),
> +            _ph: PhantomPinned,
> +        }
> +    }
> +}
> +impl<T: ?Sized> UnsafePinned<T> {
> +    /// Get read-only access to the contents of a shared `UnsafePinned`.
> +    ///
> +    /// Note that `&UnsafePinned<T>` is read-only if `&T` is read-only. This means that if there is
> +    /// mutation of the `T`, future reads from the `*const T` returned here are UB! Use
> +    /// [`UnsafeCell`] if you also need interior mutability.
> +    ///
> +    /// [`UnsafeCell`]: core::cell::UnsafeCell
> +    ///
> +    /// ```rust,no_build
> +    /// use kernel::types::UnsafePinned;
> +    ///
> +    /// unsafe {
> +    ///     let mut x = UnsafePinned::new(0);
> +    ///     let ptr = x.get(); // read-only pointer, assumes immutability
> +    ///     x.get_mut_unchecked().write(1);
> +    ///     ptr.read(); // UB!
> +    /// }
> +    /// ```
> +    ///
> +    /// Note that the `get_mut_unchecked` function used by this example is
> +    /// currently not implemented in the kernel implementation.
> +    #[inline(always)]
> +    #[must_use]
> +    pub const fn get(&self) -> *const T {
> +        self.value.get()
> +    }
> +
> +    /// Gets a mutable pointer to the wrapped value.
> +    ///
> +    /// The difference from `get_mut_pinned` and `get_mut_unchecked` is that this function
> +    /// accepts a raw pointer, which is useful to avoid the creation of temporary references.
> +    ///
> +    /// These functions mentioned here are currently not implemented in the kernel.
> +    #[inline(always)]
> +    #[must_use]
> +    pub const fn raw_get_mut(this: *mut Self) -> *mut T {
> +        this as *mut T
> +    }
> +}
> +
> +impl<T> Wrapper<T> for UnsafePinned<T> {
> +    fn pin_init<E>(init: impl PinInit<T, E>) -> impl PinInit<Self, E> {
> +        // SAFETY: `UnsafePinned<T>` has a compatible layout to `T`.
> +        unsafe { cast_pin_init(init) }
> +    }
> +}
> 

Re: [PATCH v2 1/3] rust: add UnsafePinned type

[Benno Lossin]

On Wed Apr 30, 2025 at 7:30 PM CEST, Christian Schrefl wrote:
> On 30.04.25 11:45 AM, Benno Lossin wrote:
>> On Wed Apr 30, 2025 at 10:36 AM CEST, Christian Schrefl wrote:
>>> +/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
>>> +/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
>>> +/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
>>> +/// to future rust versions only this polyfill of this type should be used when this behavior is
>>> +/// required.
>>> +///
>>> +/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
>>> +/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
>>> +/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).
>> 
>> I would make this last paragraph a normal comment, I don't think we
>> should expose it in the docs.
>
> I added this as docs since I wanted it to be a bit more visible,
> but I can replace the comment text (about `UnsafeCell`) with this paragraph
> and drop it from the docs if you want.

I think we shouldn't talk about these implementation details in the
docs.

>>> +// As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
>>> +// - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
>>> +//      Required to use the `!Unpin hack`.
>>> +// - `UnsafeCell<T>` instead of T to disallow niche optimizations,
>>> +//     which is handled in the compiler in upstream Rust
>>> +#[repr(transparent)]
>>> +pub struct UnsafePinned<T: ?Sized> {
>>> +    _ph: PhantomPinned,
>>> +    value: UnsafeCell<T>,
>>> +}
>>> +
>>> +impl<T> UnsafePinned<T> {
>>> +    /// Constructs a new instance of [`UnsafePinned`] which will wrap the specified value.
>>> +    ///
>>> +    /// All access to the inner value through `&UnsafePinned<T>` or `&mut UnsafePinned<T>` or
>>> +    /// `Pin<&mut UnsafePinned<T>>` requires `unsafe` code.
>>> +    #[inline(always)]
>>> +    #[must_use]
>>> +    pub const fn new(value: T) -> Self {
>>> +        UnsafePinned {
>>> +            value: UnsafeCell::new(value),
>>> +            _ph: PhantomPinned,
>>> +        }
>>> +    }
>>> +}
>>> +impl<T: ?Sized> UnsafePinned<T> {
>>> +    /// Get read-only access to the contents of a shared `UnsafePinned`.
>>> +    ///
>>> +    /// Note that `&UnsafePinned<T>` is read-only if `&T` is read-only. This means that if there is
>>> +    /// mutation of the `T`, future reads from the `*const T` returned here are UB! Use
>>> +    /// [`UnsafeCell`] if you also need interior mutability.
>> 
>> I agree with copy-pasting the docs from upstream, even though our
>> implementation already wraps the value in `UnsafeCell`, but I think we
>> should include a comment at the top of this doc that mentions this
>> difference. So something along the lines "In order to make replacing
>> this type with the upstream one, we want to have as little API
>> divergence as possible. Thus we don't mention the implementation detail
>> of `UnsafeCell` and people have to use `UnsafePinned<UnsafeCell<T>>`
>> instead of just `UnsafePinned<T>`." feel free to modify.
>> 
>
> I already wrote about this in comments (and documentation in this version)
> on the `UnsafePinned` type definition.
>
> I'm not sure where exactly we want to have this, but I think having it
> at the top of the file and on the type definition is a bit redundant.

Sure.

>>> +    /// Gets a mutable pointer to the wrapped value.
>>> +    ///
>>> +    /// The difference from `get_mut_pinned` and `get_mut_unchecked` is that this function
>>> +    /// accepts a raw pointer, which is useful to avoid the creation of temporary references.
>> 
>> You did not include the `get_mut_{pinned,unchecked}` methods, so
>> mentioning them here in the docs might confuse people. Do we want to
>> have those methods?
>
> I only included the functions that we needed for `Opaque` and my
> `miscdevice' patches. I think these functions should only be added
> once they have a user. That's why I wrote the next sentence in the
> documents.
>
> Should I handle this differently?
>
> It should be a really simple patch to add these functions and I can
> do that if someone needs them or I can just include them in this
> patch set.

Then I'd remove the sentence referencing the functions you don't add.

---
Cheers,
Benno

Re: [PATCH v2 1/3] rust: add UnsafePinned type

[Benno Lossin]

On Thu May 1, 2025 at 7:12 PM CEST, Christian Schrefl wrote:
> On 30.04.25 10:36 AM, Christian Schrefl wrote:
>> +/// This type provides a way to opt-out of typical aliasing rules;
>> +/// specifically, `&mut UnsafePinned<T>` is not guaranteed to be a unique pointer.
>> +///
>> +/// However, even if you define your type like `pub struct Wrapper(UnsafePinned<...>)`, it is still
>> +/// very risky to have an `&mut Wrapper` that aliases anything else. Many functions that work
>> +/// generically on `&mut T` assume that the memory that stores `T` is uniquely owned (such as
>> +/// `mem::swap`). In other words, while having aliasing with `&mut Wrapper` is not immediate
>> +/// Undefined Behavior, it is still unsound to expose such a mutable reference to code you do not
>> +/// control! Techniques such as pinning via [`Pin`](core::pin::Pin) are needed to ensure soundness.
>> +///
>> +/// Similar to [`UnsafeCell`], [`UnsafePinned`] will not usually show up in
>> +/// the public API of a library. It is an internal implementation detail of libraries that need to
>> +/// support aliasing mutable references.
>> +///
>> +/// Further note that this does *not* lift the requirement that shared references must be read-only!
>> +/// Use [`UnsafeCell`] for that.
>
> [CC Ralf]
>
> Ralf has replied to me on Github that this will most likely change [0]. How should this be handled?
>
> I would fine with submitting a patch once it changes on the rust side (possibly waiting until the
> feature is close to stabilization). I think it is better to only add this guarantee later as it
> will be easier to remove unnecessary `UnsafeCell`s than it would be to later add them back in ever
> case where they would be needed (in case rust doesn't change `UnsafePinned` to act like `UnsafeCell`).

Agreed, unless Ralf suggests a different way, we should do it like this.

---
Cheers,
Benno

> Also see to the tracking issue [1] for the reason why `UnsafeCell` behavior is most likely required.
>
> [0]: https://github.com/rust-lang/rust/issues/125735#issuecomment-2842926832
> [1]: https://github.com/rust-lang/rust/issues/137750
>
> Cheers
> Christian

Re: [PATCH v2 1/3] rust: add UnsafePinned type

[Christian Schrefl]

On 01.05.25 8:51 PM, Benno Lossin wrote:
> On Wed Apr 30, 2025 at 7:30 PM CEST, Christian Schrefl wrote:
>> On 30.04.25 11:45 AM, Benno Lossin wrote:
>>> On Wed Apr 30, 2025 at 10:36 AM CEST, Christian Schrefl wrote:
>>>> +/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
>>>> +/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
>>>> +/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
>>>> +/// to future rust versions only this polyfill of this type should be used when this behavior is
>>>> +/// required.
>>>> +///
>>>> +/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
>>>> +/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
>>>> +/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).
>>>
>>> I would make this last paragraph a normal comment, I don't think we
>>> should expose it in the docs.
>>
>> I added this as docs since I wanted it to be a bit more visible,
>> but I can replace the comment text (about `UnsafeCell`) with this paragraph
>> and drop it from the docs if you want.
> 
> I think we shouldn't talk about these implementation details in the
> docs.

Alright, what do you think of:

// As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
// - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
//   Required to use the `!Unpin hack`.
// - In order to disable niche optimizations this implementation uses `UnsafeCell` internally,
//   the upstream version however currently does not. This will most likely change in the future
//   but for now we don't expose this in the documentation, since adding the guarantee is simpler
//   than removing it. Meaning that for now the fact that `UnsafePinned` contains an `UnsafeCell`
//   must not be relied on (Other than the niche blocking).

> 
>>>> +// As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
>>>> +// - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
>>>> +//      Required to use the `!Unpin hack`.
>>>> +// - `UnsafeCell<T>` instead of T to disallow niche optimizations,
>>>> +//     which is handled in the compiler in upstream Rust
>>>> +#[repr(transparent)]
>>>> +pub struct UnsafePinned<T: ?Sized> {
>>>> +    _ph: PhantomPinned,
>>>> +    value: UnsafeCell<T>,
>>>> +}
>>>> +
>>>> +impl<T> UnsafePinned<T> {
>>>> +    /// Constructs a new instance of [`UnsafePinned`] which will wrap the specified value.
>>>> +    ///
>>>> +    /// All access to the inner value through `&UnsafePinned<T>` or `&mut UnsafePinned<T>` or
>>>> +    /// `Pin<&mut UnsafePinned<T>>` requires `unsafe` code.
>>>> +    #[inline(always)]
>>>> +    #[must_use]
>>>> +    pub const fn new(value: T) -> Self {
>>>> +        UnsafePinned {
>>>> +            value: UnsafeCell::new(value),
>>>> +            _ph: PhantomPinned,
>>>> +        }
>>>> +    }
>>>> +}
>>>> +impl<T: ?Sized> UnsafePinned<T> {
>>>> +    /// Get read-only access to the contents of a shared `UnsafePinned`.
>>>> +    ///
>>>> +    /// Note that `&UnsafePinned<T>` is read-only if `&T` is read-only. This means that if there is
>>>> +    /// mutation of the `T`, future reads from the `*const T` returned here are UB! Use
>>>> +    /// [`UnsafeCell`] if you also need interior mutability.
>>>
>>> I agree with copy-pasting the docs from upstream, even though our
>>> implementation already wraps the value in `UnsafeCell`, but I think we
>>> should include a comment at the top of this doc that mentions this
>>> difference. So something along the lines "In order to make replacing
>>> this type with the upstream one, we want to have as little API
>>> divergence as possible. Thus we don't mention the implementation detail
>>> of `UnsafeCell` and people have to use `UnsafePinned<UnsafeCell<T>>`
>>> instead of just `UnsafePinned<T>`." feel free to modify.
>>>
>>
>> I already wrote about this in comments (and documentation in this version)
>> on the `UnsafePinned` type definition.
>>
>> I'm not sure where exactly we want to have this, but I think having it
>> at the top of the file and on the type definition is a bit redundant.
> 
> Sure.

I'll add the following sentence to the end of the module documentation:

For details on the difference to the upstream implementation see the
comment on the [`UnsafePinned`] struct definition.

> 
>>>> +    /// Gets a mutable pointer to the wrapped value.
>>>> +    ///
>>>> +    /// The difference from `get_mut_pinned` and `get_mut_unchecked` is that this function
>>>> +    /// accepts a raw pointer, which is useful to avoid the creation of temporary references.
>>>
>>> You did not include the `get_mut_{pinned,unchecked}` methods, so
>>> mentioning them here in the docs might confuse people. Do we want to
>>> have those methods?
>>
>> I only included the functions that we needed for `Opaque` and my
>> `miscdevice' patches. I think these functions should only be added
>> once they have a user. That's why I wrote the next sentence in the
>> documents.
>>
>> Should I handle this differently?
>>
>> It should be a really simple patch to add these functions and I can
>> do that if someone needs them or I can just include them in this
>> patch set.
> 
> Then I'd remove the sentence referencing the functions you don't add.

Alright

Cheers
Christian

Re: [PATCH v2 1/3] rust: add UnsafePinned type

[Benno Lossin]

On Thu May 1, 2025 at 9:11 PM CEST, Christian Schrefl wrote:
> On 01.05.25 8:51 PM, Benno Lossin wrote:
>> On Wed Apr 30, 2025 at 7:30 PM CEST, Christian Schrefl wrote:
>>> On 30.04.25 11:45 AM, Benno Lossin wrote:
>>>> On Wed Apr 30, 2025 at 10:36 AM CEST, Christian Schrefl wrote:
>>>>> +/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
>>>>> +/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
>>>>> +/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
>>>>> +/// to future rust versions only this polyfill of this type should be used when this behavior is
>>>>> +/// required.
>>>>> +///
>>>>> +/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
>>>>> +/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
>>>>> +/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).
>>>>
>>>> I would make this last paragraph a normal comment, I don't think we
>>>> should expose it in the docs.
>>>
>>> I added this as docs since I wanted it to be a bit more visible,
>>> but I can replace the comment text (about `UnsafeCell`) with this paragraph
>>> and drop it from the docs if you want.
>> 
>> I think we shouldn't talk about these implementation details in the
>> docs.
>
> Alright, what do you think of:
>
> // As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`

There are two '`' after PhantomPinned.

> // - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`

s/ a / an /

I find the phrasing 'avoid needing <negative impl>' a bit weird, I'd
just say "`PhantomPinned` to ensure the struct always is `!Unpin` and
thus enables the `!Unpin` hack".

If you have a link to somewhere that explains that hack, then I'd also
put it there. I forgot if it's written down somewhere.

> //   Required to use the `!Unpin hack`.
> // - In order to disable niche optimizations this implementation uses `UnsafeCell` internally,
> //   the upstream version however currently does not. This will most likely change in the future
> //   but for now we don't expose this in the documentation, since adding the guarantee is simpler
> //   than removing it. Meaning that for now the fact that `UnsafePinned` contains an `UnsafeCell`
> //   must not be relied on (Other than the niche blocking).

---
Cheers,
Benno

Re: [PATCH v2 1/3] rust: add UnsafePinned type

[Christian Schrefl]

[cc Ralf]

On 02.05.25 12:51 AM, Benno Lossin wrote:
> On Thu May 1, 2025 at 9:11 PM CEST, Christian Schrefl wrote:
>> On 01.05.25 8:51 PM, Benno Lossin wrote:
>>> On Wed Apr 30, 2025 at 7:30 PM CEST, Christian Schrefl wrote:
>>>> On 30.04.25 11:45 AM, Benno Lossin wrote:
>>>>> On Wed Apr 30, 2025 at 10:36 AM CEST, Christian Schrefl wrote:
>>>>>> +/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
>>>>>> +/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
>>>>>> +/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
>>>>>> +/// to future rust versions only this polyfill of this type should be used when this behavior is
>>>>>> +/// required.
>>>>>> +///
>>>>>> +/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
>>>>>> +/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
>>>>>> +/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).
>>>>>
>>>>> I would make this last paragraph a normal comment, I don't think we
>>>>> should expose it in the docs.
>>>>
>>>> I added this as docs since I wanted it to be a bit more visible,
>>>> but I can replace the comment text (about `UnsafeCell`) with this paragraph
>>>> and drop it from the docs if you want.
>>>
>>> I think we shouldn't talk about these implementation details in the
>>> docs.
>>
>> Alright, what do you think of:
>>
>> // As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
> 
> There are two '`' after PhantomPinned.
> 
>> // - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
> 
> s/ a / an /
> 
> I find the phrasing 'avoid needing <negative impl>' a bit weird, I'd
> just say "`PhantomPinned` to ensure the struct always is `!Unpin` and
> thus enables the `!Unpin` hack".

Thanks I'll use that.

> 
> If you have a link to somewhere that explains that hack, then I'd also
> put it there. I forgot if it's written down somewhere.

I haven't found anything that describes the hack in detail.
From what I understand its a combination of disabling `noalias`
[0] (this PR enables it for `Unpin` types) and disabling 
`dereferencable` [1] on `&mut !Unpin` types.
Related rust issue about this [2].

Maybe Alice, Ralf or someone else form the rust side can provide
a better reference?

[0]: https://github.com/rust-lang/rust/pull/82834
[1]: https://github.com/rust-lang/rust/pull/106180
[2]: https://github.com/rust-lang/rust/issues/63818

Cheers
Christian

>> //   Required to use the `!Unpin hack`.
>> // - In order to disable niche optimizations this implementation uses `UnsafeCell` internally,
>> //   the upstream version however currently does not. This will most likely change in the future
>> //   but for now we don't expose this in the documentation, since adding the guarantee is simpler
>> //   than removing it. Meaning that for now the fact that `UnsafePinned` contains an `UnsafeCell`
>> //   must not be relied on (Other than the niche blocking).

Re: [PATCH v2 1/3] rust: add UnsafePinned type

[Alice Ryhl]

On Fri, May 02, 2025 at 02:08:13AM +0200, Christian Schrefl wrote:
> [cc Ralf]
> 
> On 02.05.25 12:51 AM, Benno Lossin wrote:
> > On Thu May 1, 2025 at 9:11 PM CEST, Christian Schrefl wrote:
> >> On 01.05.25 8:51 PM, Benno Lossin wrote:
> >>> On Wed Apr 30, 2025 at 7:30 PM CEST, Christian Schrefl wrote:
> >>>> On 30.04.25 11:45 AM, Benno Lossin wrote:
> >>>>> On Wed Apr 30, 2025 at 10:36 AM CEST, Christian Schrefl wrote:
> >>>>>> +/// This implementation works because of the "`!Unpin` hack" in rustc, which allows (some kinds of)
> >>>>>> +/// mutual aliasing of `!Unpin` types. This hack might be removed at some point, after which only
> >>>>>> +/// the `core::pin::UnsafePinned` type will allow this behavior. In order to simplify the migration
> >>>>>> +/// to future rust versions only this polyfill of this type should be used when this behavior is
> >>>>>> +/// required.
> >>>>>> +///
> >>>>>> +/// In order to disable niche optimizations this implementation uses [`UnsafeCell`] internally,
> >>>>>> +/// the upstream version however will not. So the fact that [`UnsafePinned`] contains an
> >>>>>> +/// [`UnsafeCell`] must not be relied on (Other than the niche blocking).
> >>>>>
> >>>>> I would make this last paragraph a normal comment, I don't think we
> >>>>> should expose it in the docs.
> >>>>
> >>>> I added this as docs since I wanted it to be a bit more visible,
> >>>> but I can replace the comment text (about `UnsafeCell`) with this paragraph
> >>>> and drop it from the docs if you want.
> >>>
> >>> I think we shouldn't talk about these implementation details in the
> >>> docs.
> >>
> >> Alright, what do you think of:
> >>
> >> // As opposed to the upstream Rust type this contains a `PhantomPinned`` and `UnsafeCell<T>`
> > 
> > There are two '`' after PhantomPinned.
> > 
> >> // - `PhantomPinned` to avoid needing a `impl<T> !Unpin for UnsafePinned<T>`
> > 
> > s/ a / an /
> > 
> > I find the phrasing 'avoid needing <negative impl>' a bit weird, I'd
> > just say "`PhantomPinned` to ensure the struct always is `!Unpin` and
> > thus enables the `!Unpin` hack".
> 
> Thanks I'll use that.
> 
> > 
> > If you have a link to somewhere that explains that hack, then I'd also
> > put it there. I forgot if it's written down somewhere.
> 
> I haven't found anything that describes the hack in detail.
> From what I understand its a combination of disabling `noalias`
> [0] (this PR enables it for `Unpin` types) and disabling 
> `dereferencable` [1] on `&mut !Unpin` types.
> Related rust issue about this [2].
> 
> Maybe Alice, Ralf or someone else form the rust side can provide
> a better reference?
> 
> [0]: https://github.com/rust-lang/rust/pull/82834
> [1]: https://github.com/rust-lang/rust/pull/106180
> [2]: https://github.com/rust-lang/rust/issues/63818

I wrote this a long time ago:
https://gist.github.com/Darksonn/1567538f56af1a8038ecc3c664a42462

But it doesn't really take the angle of explaining the !Unpin hack. It's
more of an early doc arguing for having an UnsafePinned type.

Alice

Re: [PATCH v2 1/3] rust: add UnsafePinned type

[Ralf Jung]

Hi all,

On 01.05.25 20:55, Benno Lossin wrote:
> On Thu May 1, 2025 at 7:12 PM CEST, Christian Schrefl wrote:
>> On 30.04.25 10:36 AM, Christian Schrefl wrote:
>>> +/// This type provides a way to opt-out of typical aliasing rules;
>>> +/// specifically, `&mut UnsafePinned<T>` is not guaranteed to be a unique pointer.
>>> +///
>>> +/// However, even if you define your type like `pub struct Wrapper(UnsafePinned<...>)`, it is still
>>> +/// very risky to have an `&mut Wrapper` that aliases anything else. Many functions that work
>>> +/// generically on `&mut T` assume that the memory that stores `T` is uniquely owned (such as
>>> +/// `mem::swap`). In other words, while having aliasing with `&mut Wrapper` is not immediate
>>> +/// Undefined Behavior, it is still unsound to expose such a mutable reference to code you do not
>>> +/// control! Techniques such as pinning via [`Pin`](core::pin::Pin) are needed to ensure soundness.
>>> +///
>>> +/// Similar to [`UnsafeCell`], [`UnsafePinned`] will not usually show up in
>>> +/// the public API of a library. It is an internal implementation detail of libraries that need to
>>> +/// support aliasing mutable references.
>>> +///
>>> +/// Further note that this does *not* lift the requirement that shared references must be read-only!
>>> +/// Use [`UnsafeCell`] for that.
>>
>> [CC Ralf]
>>
>> Ralf has replied to me on Github that this will most likely change [0]. How should this be handled?
>>
>> I would fine with submitting a patch once it changes on the rust side (possibly waiting until the
>> feature is close to stabilization). I think it is better to only add this guarantee later as it
>> will be easier to remove unnecessary `UnsafeCell`s than it would be to later add them back in ever
>> case where they would be needed (in case rust doesn't change `UnsafePinned` to act like `UnsafeCell`).
> 
> Agreed, unless Ralf suggests a different way, we should do it like this.

Sorry, I replied only on github since that was easier to do more quickly. ;)

Yes I would recommend for now to keep the `UnsafeCell` in the kernel sources, 
until the compiler actually implements the change that removes `noalias` from 
`&UnsafePinned`. And even then you should probably keep the `UnsafeCell` when 
building for older compiler versions from before that patch (or keep it until 
you drop support for those older compiler versions).

This is not a spec-only change, codegen really has to be adjusted to make 
`&UnsafePinned` properly compatible with mutable aliasing, and I see no reason 
to risk potential problems here by prematurely removing that `UnsafeCell`.

Kind regards,
Ralf

> 
> ---
> Cheers,
> Benno
> 
>> Also see to the tracking issue [1] for the reason why `UnsafeCell` behavior is most likely required.
>>
>> [0]: https://github.com/rust-lang/rust/issues/125735#issuecomment-2842926832
>> [1]: https://github.com/rust-lang/rust/issues/137750
>>
>> Cheers
>> Christian

Re: [PATCH v2 1/3] rust: add UnsafePinned type

[Ralf Jung]

Hi all,

>> If you have a link to somewhere that explains that hack, then I'd also
>> put it there. I forgot if it's written down somewhere.
> 
> I haven't found anything that describes the hack in detail.
>  From what I understand its a combination of disabling `noalias`
> [0] (this PR enables it for `Unpin` types) and disabling
> `dereferencable` [1] on `&mut !Unpin` types.
> Related rust issue about this [2].
> 
> Maybe Alice, Ralf or someone else form the rust side can provide
> a better reference?

The `!Unpick` hack simply removes `noalias` and `dereferenceable` from `&mut 
!Unpin` and `Box<!Unpin>` types (similar to how we already remove  `noalias` and 
`dereferenceable` from `&!Freeze` types). This is not a stable guarantee and 
will probably change in future version of the compiler.

Kind regards,
Ralf