[PATCH 0/2] rust: auxiliary: replace drvdata() with registration data

[PATCH 0/2] rust: auxiliary: replace drvdata() with registration data

[Danilo Krummrich]

When drvdata() was introduced in commit 6f61a2637abe ("rust: device: introduce
Device::drvdata()"), its commit message already noted that a direct accessor to
the driver's bus device private data is not commonly required -- bus callbacks
provide access through &self, and other entry points (IRQs, workqueues, IOCTLs,
etc.) carry their own private data.

The sole motivation for drvdata() was inter-driver interaction, e.g. a parent
driver deriving its bus device private data from the child driver via the
auxiliary bus.

However, drvdata() exposes the driver's bus device private data beyond the
driver's own scope. This creates ordering constraints -- drvdata may not be set
yet when the first caller of drvdata() can appear -- and forces the driver's bus
device private data to outlive all registrations that access it; a requirement
that causes unnecessary complications.

Private data should be private to the entity that issues it; bus device private
data belongs to bus callbacks, class device private data to class callbacks, IRQ
private data to the IRQ handler, etc.

This series replaces drvdata() with a dedicated registration_data pointer on
struct auxiliary_device. The parent stores its private data explicitly during
registration; the data is private to the registration and lives as long as the
Registration object.

On teardown, Registration::drop() first triggers auxiliary_device_delete()
(unbinding the child), then frees the registration data. Ordering constraints
are structural -- the child's lifecycle is scoped to the registration by
construction, not by convention.

With no remaining use case for drvdata(), drvdata(), match_type_id(),
set_type_id() and struct driver_type are removed.

This is a prerequisite for [1], which builds on the removal of drvdata() to
enable Higher-Ranked Lifetime Types (HRT) for Rust device drivers.

[1] Posted as a reply to this series.

Danilo Krummrich (2):
  rust: auxiliary: add registration data to auxiliary devices
  rust: driver core: remove drvdata() and driver_type

 drivers/base/base.h                   |  16 --
 drivers/gpu/nova-core/driver.rs       |  10 +-
 include/linux/auxiliary_bus.h         |   4 +
 rust/kernel/auxiliary.rs              | 208 ++++++++++++++++++--------
 rust/kernel/device.rs                 |  60 --------
 samples/rust/rust_driver_auxiliary.rs |  40 +++--
 6 files changed, 180 insertions(+), 158 deletions(-)


base-commit: a7cc262a11354ab104b8e55c21200d099d141bc7
-- 
2.54.0

[PATCH 1/2] rust: auxiliary: add registration data to auxiliary devices

[Danilo Krummrich]

Add a registration_data pointer to struct auxiliary_device, allowing the
registering (parent) driver to attach private data to the device at
registration time and retrieve it later when called back by the
auxiliary (child) driver.

By tying the data to the device's registration, Rust drivers can bind
the lifetime of device resources to it, since the auxiliary bus
guarantees that the parent driver remains bound while the auxiliary
device is bound.

On the Rust side, Registration<T> takes ownership of the data via
ForeignOwnable. A TypeId is stored alongside the data for runtime type
checking, making Device::registration_data<T>() a safe method.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/gpu/nova-core/driver.rs       |  10 +-
 include/linux/auxiliary_bus.h         |   4 +
 rust/kernel/auxiliary.rs              | 208 ++++++++++++++++++--------
 samples/rust/rust_driver_auxiliary.rs |  40 +++--
 4 files changed, 180 insertions(+), 82 deletions(-)

diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
index 84b0e1703150..8fe484d357f6 100644
--- a/drivers/gpu/nova-core/driver.rs
+++ b/drivers/gpu/nova-core/driver.rs
@@ -32,8 +32,7 @@
 pub(crate) struct NovaCore {
     #[pin]
     pub(crate) gpu: Gpu,
-    #[pin]
-    _reg: Devres<auxiliary::Registration>,
+    _reg: Devres<auxiliary::Registration<()>>,
 }
 
 const BAR0_SIZE: usize = SZ_16M;
@@ -96,14 +95,15 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, E
 
             Ok(try_pin_init!(Self {
                 gpu <- Gpu::new(pdev, bar.clone(), bar.access(pdev.as_ref())?),
-                _reg <- auxiliary::Registration::new(
+                _reg: auxiliary::Registration::new(
                     pdev.as_ref(),
                     c"nova-drm",
                     // TODO[XARR]: Use XArray or perhaps IDA for proper ID allocation/recycling. For
                     // now, use a simple atomic counter that never recycles IDs.
                     AUXILIARY_ID_COUNTER.fetch_add(1, Relaxed),
-                    crate::MODULE_NAME
-                ),
+                    crate::MODULE_NAME,
+                    (),
+                )?,
             }))
         })
     }
diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
index bc09b55e3682..4e1ad8ccbcdd 100644
--- a/include/linux/auxiliary_bus.h
+++ b/include/linux/auxiliary_bus.h
@@ -62,6 +62,9 @@
  * @sysfs.irqs: irqs xarray contains irq indices which are used by the device,
  * @sysfs.lock: Synchronize irq sysfs creation,
  * @sysfs.irq_dir_exists: whether "irqs" directory exists,
+ * @registration_data_rust: private data owned by the registering (parent)
+ *                          driver; valid for as long as the device is
+ *                          registered with the driver core,
  *
  * An auxiliary_device represents a part of its parent device's functionality.
  * It is given a name that, combined with the registering drivers
@@ -148,6 +151,7 @@ struct auxiliary_device {
 		struct mutex lock; /* Synchronize irq sysfs creation */
 		bool irq_dir_exists;
 	} sysfs;
+	void *registration_data_rust;
 };
 
 /**
diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
index 93c0db1f6655..467befea8e44 100644
--- a/rust/kernel/auxiliary.rs
+++ b/rust/kernel/auxiliary.rs
@@ -19,12 +19,17 @@
         to_result, //
     },
     prelude::*,
-    types::Opaque,
+    types::{
+        ForeignOwnable,
+        Opaque, //
+    },
     ThisModule, //
 };
 use core::{
+    any::TypeId,
     marker::PhantomData,
     mem::offset_of,
+    pin::Pin,
     ptr::{
         addr_of_mut,
         NonNull, //
@@ -257,6 +262,40 @@ pub fn parent(&self) -> &device::Device<device::Bound> {
         // SAFETY: A bound auxiliary device always has a bound parent device.
         unsafe { parent.as_bound() }
     }
+
+    /// Returns a pinned reference to the registration data set by the registering (parent) driver.
+    ///
+    /// Returns [`EINVAL`] if `T` does not match the type used by the parent driver when calling
+    /// [`Registration::new()`].
+    ///
+    /// Returns [`ENOENT`] if no registration data has been set, e.g. when the device was
+    /// registered by a C driver.
+    pub fn registration_data<T: 'static>(&self) -> Result<Pin<&T>> {
+        // SAFETY: By the type invariant, `self.as_raw()` is a valid `struct auxiliary_device`.
+        let ptr = unsafe { (*self.as_raw()).registration_data_rust };
+        if ptr.is_null() {
+            dev_warn!(
+                self.as_ref(),
+                "No registration data set; parent is not a Rust driver.\n"
+            );
+            return Err(ENOENT);
+        }
+
+        // SAFETY: `ptr` is non-null and was set via `into_foreign()` in `Registration::new()`;
+        // `RegistrationData` is `#[repr(C)]` with `type_id` at offset 0, so reading a `TypeId`
+        // at the start of the allocation is valid regardless of `T`.
+        let type_id = unsafe { ptr.cast::<TypeId>().read() };
+        if type_id != TypeId::of::<T>() {
+            return Err(EINVAL);
+        }
+
+        // SAFETY: The `TypeId` check above confirms that the stored type is `T`; `ptr` remains
+        // valid until `Registration::drop()` calls `from_foreign()`.
+        let wrapper = unsafe { Pin::<KBox<RegistrationData<T>>>::borrow(ptr) };
+
+        // SAFETY: `data` is a structurally pinned field of `RegistrationData`.
+        Ok(unsafe { wrapper.map_unchecked(|w| &w.data) })
+    }
 }
 
 impl Device {
@@ -326,87 +365,132 @@ unsafe impl Send for Device {}
 // (i.e. `Device<Normal>) are thread safe.
 unsafe impl Sync for Device {}
 
+/// Wrapper that stores a [`TypeId`] alongside the registration data for runtime type checking.
+#[repr(C)]
+#[pin_data]
+struct RegistrationData<T> {
+    type_id: TypeId,
+    #[pin]
+    data: T,
+}
+
 /// The registration of an auxiliary device.
 ///
 /// This type represents the registration of a [`struct auxiliary_device`]. When its parent device
 /// is unbound, the corresponding auxiliary device will be unregistered from the system.
 ///
+/// The type parameter `T` is the type of the registration data owned by the registering (parent)
+/// driver. It can be accessed by the auxiliary driver through
+/// [`Device::registration_data()`].
+///
 /// # Invariants
 ///
-/// `self.0` always holds a valid pointer to an initialized and registered
-/// [`struct auxiliary_device`].
-pub struct Registration(NonNull<bindings::auxiliary_device>);
-
-impl Registration {
-    /// Create and register a new auxiliary device.
-    pub fn new<'a>(
-        parent: &'a device::Device<device::Bound>,
-        name: &'a CStr,
+/// `self.adev` always holds a valid pointer to an initialized and registered
+/// [`struct auxiliary_device`], and `registration_data` points to a valid
+/// `Pin<KBox<RegistrationData<T>>>`.
+pub struct Registration<T: 'static> {
+    adev: NonNull<bindings::auxiliary_device>,
+    _data: PhantomData<T>,
+}
+
+impl<T: Send + 'static> Registration<T> {
+    /// Create and register a new auxiliary device with the given registration data.
+    ///
+    /// The `data` is owned by the registration and can be accessed through the auxiliary device
+    /// via [`Device::registration_data()`].
+    pub fn new<E>(
+        parent: &device::Device<device::Bound>,
+        name: &CStr,
         id: u32,
-        modname: &'a CStr,
-    ) -> impl PinInit<Devres<Self>, Error> + 'a {
-        pin_init::pin_init_scope(move || {
-            let boxed = KBox::new(Opaque::<bindings::auxiliary_device>::zeroed(), GFP_KERNEL)?;
-            let adev = boxed.get();
-
-            // SAFETY: It's safe to set the fields of `struct auxiliary_device` on initialization.
-            unsafe {
-                (*adev).dev.parent = parent.as_raw();
-                (*adev).dev.release = Some(Device::release);
-                (*adev).name = name.as_char_ptr();
-                (*adev).id = id;
-            }
-
-            // SAFETY: `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`,
-            // which has not been initialized yet.
-            unsafe { bindings::auxiliary_device_init(adev) };
-
-            // Now that `adev` is initialized, leak the `Box`; the corresponding memory will be
-            // freed by `Device::release` when the last reference to the `struct auxiliary_device`
-            // is dropped.
-            let _ = KBox::into_raw(boxed);
-
-            // SAFETY:
-            // - `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, which
-            //   has been initialized,
-            // - `modname.as_char_ptr()` is a NULL terminated string.
-            let ret = unsafe { bindings::__auxiliary_device_add(adev, modname.as_char_ptr()) };
-            if ret != 0 {
-                // SAFETY: `adev` is guaranteed to be a valid pointer to a
-                // `struct auxiliary_device`, which has been initialized.
-                unsafe { bindings::auxiliary_device_uninit(adev) };
-
-                return Err(Error::from_errno(ret));
-            }
-
-            // INVARIANT: The device will remain registered until `auxiliary_device_delete()` is
-            // called, which happens in `Self::drop()`.
-            Ok(Devres::new(
-                parent,
-                // SAFETY: `adev` is guaranteed to be non-null, since the `KBox` was allocated
-                // successfully.
-                Self(unsafe { NonNull::new_unchecked(adev) }),
-            ))
-        })
+        modname: &CStr,
+        data: impl PinInit<T, E>,
+    ) -> Result<Devres<Self>>
+    where
+        Error: From<E>,
+    {
+        let data = KBox::pin_init::<Error>(
+            try_pin_init!(RegistrationData {
+                type_id: TypeId::of::<T>(),
+                data <- data,
+            }),
+            GFP_KERNEL,
+        )?;
+
+        let boxed = KBox::new(Opaque::<bindings::auxiliary_device>::zeroed(), GFP_KERNEL)?;
+        let adev = boxed.get();
+
+        // SAFETY: It's safe to set the fields of `struct auxiliary_device` on initialization.
+        unsafe {
+            (*adev).dev.parent = parent.as_raw();
+            (*adev).dev.release = Some(Device::release);
+            (*adev).name = name.as_char_ptr();
+            (*adev).id = id;
+            (*adev).registration_data_rust = data.into_foreign();
+        }
+
+        // SAFETY: `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`,
+        // which has not been initialized yet.
+        unsafe { bindings::auxiliary_device_init(adev) };
+
+        // Now that `adev` is initialized, leak the `Box`; the corresponding memory will be
+        // freed by `Device::release` when the last reference to the `struct auxiliary_device`
+        // is dropped.
+        let _ = KBox::into_raw(boxed);
+
+        // SAFETY:
+        // - `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, which
+        //   has been initialized,
+        // - `modname.as_char_ptr()` is a NULL terminated string.
+        let ret = unsafe { bindings::__auxiliary_device_add(adev, modname.as_char_ptr()) };
+        if ret != 0 {
+            // SAFETY: `registration_data` was set above via `into_foreign()`.
+            let _ = unsafe {
+                Pin::<KBox<RegistrationData<T>>>::from_foreign((*adev).registration_data_rust)
+            };
+
+            // SAFETY: `adev` is guaranteed to be a valid pointer to a
+            // `struct auxiliary_device`, which has been initialized.
+            unsafe { bindings::auxiliary_device_uninit(adev) };
+
+            return Err(Error::from_errno(ret));
+        }
+
+        // INVARIANT: The device will remain registered until `auxiliary_device_delete()` is
+        // called, which happens in `Self::drop()`.
+        let reg = Self {
+            // SAFETY: `adev` is guaranteed to be non-null, since the `KBox` was allocated
+            // successfully.
+            adev: unsafe { NonNull::new_unchecked(adev) },
+            _data: PhantomData,
+        };
+
+        Devres::new::<core::convert::Infallible>(parent, reg)
     }
 }
 
-impl Drop for Registration {
+impl<T: 'static> Drop for Registration<T> {
     fn drop(&mut self) {
-        // SAFETY: By the type invariant of `Self`, `self.0.as_ptr()` is a valid registered
+        // SAFETY: By the type invariant of `Self`, `self.adev.as_ptr()` is a valid registered
         // `struct auxiliary_device`.
-        unsafe { bindings::auxiliary_device_delete(self.0.as_ptr()) };
+        unsafe { bindings::auxiliary_device_delete(self.adev.as_ptr()) };
+
+        // SAFETY: `registration_data` was set in `new()` via `into_foreign()`.
+        drop(unsafe {
+            Pin::<KBox<RegistrationData<T>>>::from_foreign(
+                (*self.adev.as_ptr()).registration_data_rust,
+            )
+        });
 
         // This drops the reference we acquired through `auxiliary_device_init()`.
         //
-        // SAFETY: By the type invariant of `Self`, `self.0.as_ptr()` is a valid registered
+        // SAFETY: By the type invariant of `Self`, `self.adev.as_ptr()` is a valid registered
         // `struct auxiliary_device`.
-        unsafe { bindings::auxiliary_device_uninit(self.0.as_ptr()) };
+        unsafe { bindings::auxiliary_device_uninit(self.adev.as_ptr()) };
     }
 }
 
 // SAFETY: A `Registration` of a `struct auxiliary_device` can be released from any thread.
-unsafe impl Send for Registration {}
+unsafe impl<T: Send> Send for Registration<T> {}
 
 // SAFETY: `Registration` does not expose any methods or fields that need synchronization.
-unsafe impl Sync for Registration {}
+unsafe impl<T: Send> Sync for Registration<T> {}
diff --git a/samples/rust/rust_driver_auxiliary.rs b/samples/rust/rust_driver_auxiliary.rs
index 5c5a5105a3ff..319ef734c02b 100644
--- a/samples/rust/rust_driver_auxiliary.rs
+++ b/samples/rust/rust_driver_auxiliary.rs
@@ -17,8 +17,6 @@
     InPlaceModule, //
 };
 
-use core::any::TypeId;
-
 const MODULE_NAME: &CStr = <LocalModule as kernel::ModuleMetadata>::NAME;
 const AUXILIARY_NAME: &CStr = c"auxiliary";
 
@@ -49,13 +47,13 @@ fn probe(adev: &auxiliary::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<S
     }
 }
 
-#[pin_data]
+struct Data {
+    index: u32,
+}
+
 struct ParentDriver {
-    private: TypeId,
-    #[pin]
-    _reg0: Devres<auxiliary::Registration>,
-    #[pin]
-    _reg1: Devres<auxiliary::Registration>,
+    _reg0: Devres<auxiliary::Registration<Data>>,
+    _reg1: Devres<auxiliary::Registration<Data>>,
 }
 
 kernel::pci_device_table!(
@@ -71,10 +69,21 @@ impl pci::Driver for ParentDriver {
     const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE;
 
     fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> {
-        try_pin_init!(Self {
-            private: TypeId::of::<Self>(),
-            _reg0 <- auxiliary::Registration::new(pdev.as_ref(), AUXILIARY_NAME, 0, MODULE_NAME),
-            _reg1 <- auxiliary::Registration::new(pdev.as_ref(), AUXILIARY_NAME, 1, MODULE_NAME),
+        Ok(Self {
+            _reg0: auxiliary::Registration::new(
+                pdev.as_ref(),
+                AUXILIARY_NAME,
+                0,
+                MODULE_NAME,
+                Data { index: 0 },
+            )?,
+            _reg1: auxiliary::Registration::new(
+                pdev.as_ref(),
+                AUXILIARY_NAME,
+                1,
+                MODULE_NAME,
+                Data { index: 1 },
+            )?,
         })
     }
 }
@@ -83,7 +92,8 @@ impl ParentDriver {
     fn connect(adev: &auxiliary::Device<Bound>) -> Result {
         let dev = adev.parent();
         let pdev: &pci::Device<Bound> = dev.try_into()?;
-        let drvdata = dev.drvdata::<Self>()?;
+
+        let data = adev.registration_data::<Data>()?;
 
         dev_info!(
             dev,
@@ -95,8 +105,8 @@ fn connect(adev: &auxiliary::Device<Bound>) -> Result {
 
         dev_info!(
             dev,
-            "We have access to the private data of {:?}.\n",
-            drvdata.private
+            "Connected to auxiliary device with index {}.\n",
+            data.index
         );
 
         Ok(())
-- 
2.54.0

[PATCH 2/2] rust: driver core: remove drvdata() and driver_type

[Danilo Krummrich]

When drvdata() was introduced in commit 6f61a2637abe ("rust: device:
introduce Device::drvdata()"), its commit message already noted that a
direct accessor to the driver's bus device private data is not commonly
required -- bus callbacks provide access through &self, and other entry
points (IRQs, workqueues, IOCTLs, etc.) carry their own private data.

The sole motivation for drvdata() was inter-driver interaction -- an
auxiliary driver deriving the parent's bus device private data from the
parent device.

However, drvdata() exposes the driver's bus device private data beyond
the driver's own scope. This creates ordering constraints; for instance
drvdata may not be set yet when the first caller of drvdata() can
appear. It also forces the driver's bus device private data to outlive
all registrations that access it, which causes unnecessary
complications.

Private data should be private to the entity that issues it, i.e. bus
device private data belongs to bus callbacks, class device private data
to class callbacks, IRQ private data to the IRQ handler, etc.

With registration-private data now available through the auxiliary bus,
there is no remaining user of drvdata(), thus remove it.

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/base/base.h   | 16 ------------
 rust/kernel/device.rs | 60 -------------------------------------------
 2 files changed, 76 deletions(-)

diff --git a/drivers/base/base.h b/drivers/base/base.h
index 30b416588617..a19f4cda2c83 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -86,18 +86,6 @@ struct driver_private {
 };
 #define to_driver(obj) container_of(obj, struct driver_private, kobj)
 
-#ifdef CONFIG_RUST
-/**
- * struct driver_type - Representation of a Rust driver type.
- */
-struct driver_type {
-	/**
-	 * @id: Representation of core::any::TypeId.
-	 */
-	u8 id[16];
-} __packed;
-#endif
-
 /**
  * struct device_private - structure to hold the private to the driver core
  *			   portions of the device structure.
@@ -115,7 +103,6 @@ struct driver_type {
  *			   dev_err_probe() for later retrieval via debugfs
  * @device: pointer back to the struct device that this structure is
  *	    associated with.
- * @driver_type: The type of the bound Rust driver.
  * @dead: This device is currently either in the process of or has been
  *	  removed from the system. Any asynchronous events scheduled for this
  *	  device should exit without taking any action.
@@ -132,9 +119,6 @@ struct device_private {
 	const struct device_driver *async_driver;
 	char *deferred_probe_reason;
 	struct device *device;
-#ifdef CONFIG_RUST
-	struct driver_type driver_type;
-#endif
 	u8 dead:1;
 };
 #define to_device_private_parent(obj)	\
diff --git a/rust/kernel/device.rs b/rust/kernel/device.rs
index 6d5396a43ebe..fd50399aadea 100644
--- a/rust/kernel/device.rs
+++ b/rust/kernel/device.rs
@@ -15,16 +15,12 @@
     }, //
 };
 use core::{
-    any::TypeId,
     marker::PhantomData,
     ptr, //
 };
 
 pub mod property;
 
-// Assert that we can `read()` / `write()` a `TypeId` instance from / into `struct driver_type`.
-static_assert!(core::mem::size_of::<bindings::driver_type>() >= core::mem::size_of::<TypeId>());
-
 /// The core representation of a device in the kernel's driver model.
 ///
 /// This structure represents the Rust abstraction for a C `struct device`. A [`Device`] can either
@@ -206,29 +202,12 @@ pub unsafe fn as_bound(&self) -> &Device<Bound> {
 }
 
 impl Device<CoreInternal> {
-    fn set_type_id<T: 'static>(&self) {
-        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
-        let private = unsafe { (*self.as_raw()).p };
-
-        // SAFETY: For a bound device (implied by the `CoreInternal` device context), `private` is
-        // guaranteed to be a valid pointer to a `struct device_private`.
-        let driver_type = unsafe { &raw mut (*private).driver_type };
-
-        // SAFETY: `driver_type` is valid for (unaligned) writes of a `TypeId`.
-        unsafe {
-            driver_type
-                .cast::<TypeId>()
-                .write_unaligned(TypeId::of::<T>())
-        };
-    }
-
     /// Store a pointer to the bound driver's private data.
     pub fn set_drvdata<T: 'static>(&self, data: impl PinInit<T, Error>) -> Result {
         let data = KBox::pin_init(data, GFP_KERNEL)?;
 
         // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
         unsafe { bindings::dev_set_drvdata(self.as_raw(), data.into_foreign().cast()) };
-        self.set_type_id::<T>();
 
         Ok(())
     }
@@ -292,45 +271,6 @@ unsafe fn drvdata_unchecked<T: 'static>(&self) -> Pin<&T> {
         //   in `into_foreign()`.
         unsafe { Pin::<KBox<T>>::borrow(ptr.cast()) }
     }
-
-    fn match_type_id<T: 'static>(&self) -> Result {
-        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
-        let private = unsafe { (*self.as_raw()).p };
-
-        // SAFETY: For a bound device, `private` is guaranteed to be a valid pointer to a
-        // `struct device_private`.
-        let driver_type = unsafe { &raw mut (*private).driver_type };
-
-        // SAFETY:
-        // - `driver_type` is valid for (unaligned) reads of a `TypeId`.
-        // - A bound device guarantees that `driver_type` contains a valid `TypeId` value.
-        let type_id = unsafe { driver_type.cast::<TypeId>().read_unaligned() };
-
-        if type_id != TypeId::of::<T>() {
-            return Err(EINVAL);
-        }
-
-        Ok(())
-    }
-
-    /// Access a driver's private data.
-    ///
-    /// Returns a pinned reference to the driver's private data or [`EINVAL`] if it doesn't match
-    /// the asserted type `T`.
-    pub fn drvdata<T: 'static>(&self) -> Result<Pin<&T>> {
-        // SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`.
-        if unsafe { bindings::dev_get_drvdata(self.as_raw()) }.is_null() {
-            return Err(ENOENT);
-        }
-
-        self.match_type_id::<T>()?;
-
-        // SAFETY:
-        // - The above check of `dev_get_drvdata()` guarantees that we are called after
-        //   `set_drvdata()`.
-        // - We've just checked that the type of the driver's private data is in fact `T`.
-        Ok(unsafe { self.drvdata_unchecked() })
-    }
 }
 
 impl<Ctx: DeviceContext> Device<Ctx> {
-- 
2.54.0

Re: [PATCH 0/2] rust: auxiliary: replace drvdata() with registration data

[Danilo Krummrich]

On Tue Apr 28, 2026 at 12:09 AM CEST, Danilo Krummrich wrote:
> When drvdata() was introduced in commit 6f61a2637abe ("rust: device: introduce
> Device::drvdata()"), its commit message already noted that a direct accessor to
> the driver's bus device private data is not commonly required -- bus callbacks
> provide access through &self, and other entry points (IRQs, workqueues, IOCTLs,
> etc.) carry their own private data.
>
> The sole motivation for drvdata() was inter-driver interaction, e.g. a parent
> driver deriving its bus device private data from the child driver via the
> auxiliary bus.
>
> However, drvdata() exposes the driver's bus device private data beyond the
> driver's own scope. This creates ordering constraints -- drvdata may not be set
> yet when the first caller of drvdata() can appear -- and forces the driver's bus
> device private data to outlive all registrations that access it; a requirement
> that causes unnecessary complications.
>
> Private data should be private to the entity that issues it; bus device private
> data belongs to bus callbacks, class device private data to class callbacks, IRQ
> private data to the IRQ handler, etc.
>
> This series replaces drvdata() with a dedicated registration_data pointer on
> struct auxiliary_device. The parent stores its private data explicitly during
> registration; the data is private to the registration and lives as long as the
> Registration object.
>
> On teardown, Registration::drop() first triggers auxiliary_device_delete()
> (unbinding the child), then frees the registration data. Ordering constraints
> are structural -- the child's lifecycle is scoped to the registration by
> construction, not by convention.
>
> With no remaining use case for drvdata(), drvdata(), match_type_id(),
> set_type_id() and struct driver_type are removed.
>
> This is a prerequisite for [1], which builds on the removal of drvdata() to
> enable Higher-Ranked Lifetime Types (HRT) for Rust device drivers.
>
> [1] Posted as a reply to this series.

https://lore.kernel.org/driver-core/20260427221155.2144848-1-dakr@kernel.org/

>
> Danilo Krummrich (2):
>   rust: auxiliary: add registration data to auxiliary devices
>   rust: driver core: remove drvdata() and driver_type
>
>  drivers/base/base.h                   |  16 --
>  drivers/gpu/nova-core/driver.rs       |  10 +-
>  include/linux/auxiliary_bus.h         |   4 +
>  rust/kernel/auxiliary.rs              | 208 ++++++++++++++++++--------
>  rust/kernel/device.rs                 |  60 --------
>  samples/rust/rust_driver_auxiliary.rs |  40 +++--
>  6 files changed, 180 insertions(+), 158 deletions(-)
>
>
> base-commit: a7cc262a11354ab104b8e55c21200d099d141bc7
> -- 
> 2.54.0

Re: [PATCH 1/2] rust: auxiliary: add registration data to auxiliary devices

[Danilo Krummrich]

On Tue Apr 28, 2026 at 12:09 AM CEST, Danilo Krummrich wrote:
> +impl<T: Send + 'static> Registration<T> {

+ Sync

Re: [PATCH 1/2] rust: auxiliary: add registration data to auxiliary devices

[Alexandre Courbot]

On Tue Apr 28, 2026 at 7:09 AM JST, Danilo Krummrich wrote:
> Add a registration_data pointer to struct auxiliary_device, allowing the
> registering (parent) driver to attach private data to the device at
> registration time and retrieve it later when called back by the
> auxiliary (child) driver.
>
> By tying the data to the device's registration, Rust drivers can bind
> the lifetime of device resources to it, since the auxiliary bus
> guarantees that the parent driver remains bound while the auxiliary
> device is bound.
>
> On the Rust side, Registration<T> takes ownership of the data via
> ForeignOwnable. A TypeId is stored alongside the data for runtime type
> checking, making Device::registration_data<T>() a safe method.
>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

This is indeed much, much cleaner!

> ---
>  drivers/gpu/nova-core/driver.rs       |  10 +-
>  include/linux/auxiliary_bus.h         |   4 +
>  rust/kernel/auxiliary.rs              | 208 ++++++++++++++++++--------
>  samples/rust/rust_driver_auxiliary.rs |  40 +++--
>  4 files changed, 180 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> index 84b0e1703150..8fe484d357f6 100644
> --- a/drivers/gpu/nova-core/driver.rs
> +++ b/drivers/gpu/nova-core/driver.rs
> @@ -32,8 +32,7 @@
>  pub(crate) struct NovaCore {
>      #[pin]
>      pub(crate) gpu: Gpu,
> -    #[pin]
> -    _reg: Devres<auxiliary::Registration>,
> +    _reg: Devres<auxiliary::Registration<()>>,
>  }
>  
>  const BAR0_SIZE: usize = SZ_16M;
> @@ -96,14 +95,15 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, E
>  
>              Ok(try_pin_init!(Self {
>                  gpu <- Gpu::new(pdev, bar.clone(), bar.access(pdev.as_ref())?),
> -                _reg <- auxiliary::Registration::new(
> +                _reg: auxiliary::Registration::new(
>                      pdev.as_ref(),
>                      c"nova-drm",
>                      // TODO[XARR]: Use XArray or perhaps IDA for proper ID allocation/recycling. For
>                      // now, use a simple atomic counter that never recycles IDs.
>                      AUXILIARY_ID_COUNTER.fetch_add(1, Relaxed),
> -                    crate::MODULE_NAME
> -                ),
> +                    crate::MODULE_NAME,
> +                    (),
> +                )?,
>              }))
>          })
>      }
> diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
> index bc09b55e3682..4e1ad8ccbcdd 100644
> --- a/include/linux/auxiliary_bus.h
> +++ b/include/linux/auxiliary_bus.h
> @@ -62,6 +62,9 @@
>   * @sysfs.irqs: irqs xarray contains irq indices which are used by the device,
>   * @sysfs.lock: Synchronize irq sysfs creation,
>   * @sysfs.irq_dir_exists: whether "irqs" directory exists,
> + * @registration_data_rust: private data owned by the registering (parent)
> + *                          driver; valid for as long as the device is
> + *                          registered with the driver core,
>   *
>   * An auxiliary_device represents a part of its parent device's functionality.
>   * It is given a name that, combined with the registering drivers
> @@ -148,6 +151,7 @@ struct auxiliary_device {
>  		struct mutex lock; /* Synchronize irq sysfs creation */
>  		bool irq_dir_exists;
>  	} sysfs;
> +	void *registration_data_rust;

I'm wondering whether we could avoid introducing a Rust-only member
here, either by just allowing the aux device private data to be used in
C as well (which might be as simple as a rename, a couple helpers and a
bit more documentation), or using a wrapper type specifically for Rust
drivers:

struct rust_auxiliary_device {
	struct auxiliary_device auxdev;
	void *registration_data;
};

Although I am not sure what the implications would be for e.g. a Rust
auxiliary device spawned by a C driver? Is that even doable with the
current code anyway?

>  };
>  
>  /**
> diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
> index 93c0db1f6655..467befea8e44 100644
> --- a/rust/kernel/auxiliary.rs
> +++ b/rust/kernel/auxiliary.rs
> @@ -19,12 +19,17 @@
>          to_result, //
>      },
>      prelude::*,
> -    types::Opaque,
> +    types::{
> +        ForeignOwnable,
> +        Opaque, //
> +    },
>      ThisModule, //
>  };
>  use core::{
> +    any::TypeId,
>      marker::PhantomData,
>      mem::offset_of,
> +    pin::Pin,
>      ptr::{
>          addr_of_mut,
>          NonNull, //
> @@ -257,6 +262,40 @@ pub fn parent(&self) -> &device::Device<device::Bound> {
>          // SAFETY: A bound auxiliary device always has a bound parent device.
>          unsafe { parent.as_bound() }
>      }
> +
> +    /// Returns a pinned reference to the registration data set by the registering (parent) driver.
> +    ///
> +    /// Returns [`EINVAL`] if `T` does not match the type used by the parent driver when calling
> +    /// [`Registration::new()`].
> +    ///
> +    /// Returns [`ENOENT`] if no registration data has been set, e.g. when the device was
> +    /// registered by a C driver.
> +    pub fn registration_data<T: 'static>(&self) -> Result<Pin<&T>> {
> +        // SAFETY: By the type invariant, `self.as_raw()` is a valid `struct auxiliary_device`.
> +        let ptr = unsafe { (*self.as_raw()).registration_data_rust };
> +        if ptr.is_null() {
> +            dev_warn!(
> +                self.as_ref(),
> +                "No registration data set; parent is not a Rust driver.\n"
> +            );
> +            return Err(ENOENT);
> +        }
> +
> +        // SAFETY: `ptr` is non-null and was set via `into_foreign()` in `Registration::new()`;
> +        // `RegistrationData` is `#[repr(C)]` with `type_id` at offset 0, so reading a `TypeId`
> +        // at the start of the allocation is valid regardless of `T`.
> +        let type_id = unsafe { ptr.cast::<TypeId>().read() };
> +        if type_id != TypeId::of::<T>() {
> +            return Err(EINVAL);
> +        }
> +
> +        // SAFETY: The `TypeId` check above confirms that the stored type is `T`; `ptr` remains
> +        // valid until `Registration::drop()` calls `from_foreign()`.
> +        let wrapper = unsafe { Pin::<KBox<RegistrationData<T>>>::borrow(ptr) };
> +
> +        // SAFETY: `data` is a structurally pinned field of `RegistrationData`.
> +        Ok(unsafe { wrapper.map_unchecked(|w| &w.data) })
> +    }
>  }
>  
>  impl Device {
> @@ -326,87 +365,132 @@ unsafe impl Send for Device {}
>  // (i.e. `Device<Normal>) are thread safe.
>  unsafe impl Sync for Device {}
>  
> +/// Wrapper that stores a [`TypeId`] alongside the registration data for runtime type checking.
> +#[repr(C)]
> +#[pin_data]
> +struct RegistrationData<T> {
> +    type_id: TypeId,
> +    #[pin]
> +    data: T,
> +}
> +
>  /// The registration of an auxiliary device.
>  ///
>  /// This type represents the registration of a [`struct auxiliary_device`]. When its parent device
>  /// is unbound, the corresponding auxiliary device will be unregistered from the system.
>  ///
> +/// The type parameter `T` is the type of the registration data owned by the registering (parent)
> +/// driver. It can be accessed by the auxiliary driver through
> +/// [`Device::registration_data()`].
> +///
>  /// # Invariants
>  ///
> -/// `self.0` always holds a valid pointer to an initialized and registered
> -/// [`struct auxiliary_device`].
> -pub struct Registration(NonNull<bindings::auxiliary_device>);
> -
> -impl Registration {
> -    /// Create and register a new auxiliary device.
> -    pub fn new<'a>(
> -        parent: &'a device::Device<device::Bound>,
> -        name: &'a CStr,
> +/// `self.adev` always holds a valid pointer to an initialized and registered
> +/// [`struct auxiliary_device`], and `registration_data` points to a valid

There is no `registration_data` member in this struct, it this referring
to something else?

Re: [PATCH 1/2] rust: auxiliary: add registration data to auxiliary devices

[Danilo Krummrich]

On Wed Apr 29, 2026 at 1:21 PM CEST, Alexandre Courbot wrote:
> I'm wondering whether we could avoid introducing a Rust-only member
> here, either by just allowing the aux device private data to be used in
> C as well (which might be as simple as a rename, a couple helpers and a
> bit more documentation),

This is intentional; if this pointer would be shared we loose the guarantee that
the stored pointer is either NULL or of the following form.

	#[repr(C)]
	#[pin_data]
	struct RegistrationData<T> {
	    type_id: TypeId,
	    #[pin]
	    data: T,
	}

This is important, since otherwise we can't check the TypeId independent from T.
Of course, we could store the TypeId in a separate allocation and use this
instead, but then we'd also end up with a Rust specific pointer.

> or using a wrapper type specifically for Rust
> drivers:
>
> struct rust_auxiliary_device {
> 	struct auxiliary_device auxdev;
> 	void *registration_data;
> };
>
> Although I am not sure what the implications would be for e.g. a Rust
> auxiliary device spawned by a C driver? Is that even doable with the
> current code anyway?

You gave the answer yourself with this. :) If we'd "subclass", there'd be no way
to distinguish whether the struct auxiliary_device * passed by bus callbacks
stems from a C or from a Rust registration, i.e. we'd not know whether the
"upcast" is valid or not.

Re: [PATCH 1/2] rust: auxiliary: add registration data to auxiliary devices

[Alice Ryhl]

On Tue, Apr 28, 2026 at 12:09:40AM +0200, Danilo Krummrich wrote:
> Add a registration_data pointer to struct auxiliary_device, allowing the
> registering (parent) driver to attach private data to the device at
> registration time and retrieve it later when called back by the
> auxiliary (child) driver.
> 
> By tying the data to the device's registration, Rust drivers can bind
> the lifetime of device resources to it, since the auxiliary bus
> guarantees that the parent driver remains bound while the auxiliary
> device is bound.
> 
> On the Rust side, Registration<T> takes ownership of the data via
> ForeignOwnable. A TypeId is stored alongside the data for runtime type
> checking, making Device::registration_data<T>() a safe method.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

So overall I think this patch makes sense. A few comments below.

> diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
> index bc09b55e3682..4e1ad8ccbcdd 100644
> --- a/include/linux/auxiliary_bus.h
> +++ b/include/linux/auxiliary_bus.h
> @@ -62,6 +62,9 @@
>   * @sysfs.irqs: irqs xarray contains irq indices which are used by the device,
>   * @sysfs.lock: Synchronize irq sysfs creation,
>   * @sysfs.irq_dir_exists: whether "irqs" directory exists,
> + * @registration_data_rust: private data owned by the registering (parent)
> + *                          driver; valid for as long as the device is
> + *                          registered with the driver core,
>   *
>   * An auxiliary_device represents a part of its parent device's functionality.
>   * It is given a name that, combined with the registering drivers
> @@ -148,6 +151,7 @@ struct auxiliary_device {
>  		struct mutex lock; /* Synchronize irq sysfs creation */
>  		bool irq_dir_exists;
>  	} sysfs;
> +	void *registration_data_rust;

Is this really Rust-specific? Would you not want C drivers with the same
pattern to do the same thing?

> +        // SAFETY: `ptr` is non-null and was set via `into_foreign()` in `Registration::new()`;
> +        // `RegistrationData` is `#[repr(C)]` with `type_id` at offset 0, so reading a `TypeId`
> +        // at the start of the allocation is valid regardless of `T`.
> +        let type_id = unsafe { ptr.cast::<TypeId>().read() };
> +        if type_id != TypeId::of::<T>() {
> +            return Err(EINVAL);
> +        }

Right, okay, so if you put C stuff there, we need the layout to be
compatible with Rust type ids.

Still, we could have Rust expose a couple methods to allow C code to use
the same field with a null type id.

But I guess this is all future work.

> +        let data = KBox::pin_init::<Error>(
> +            try_pin_init!(RegistrationData {
> +                type_id: TypeId::of::<T>(),
> +                data <- data,
> +            }),
> +            GFP_KERNEL,
> +        )?;
> +
> +        let boxed = KBox::new(Opaque::<bindings::auxiliary_device>::zeroed(), GFP_KERNEL)?;

Use __GFP_ZERO here instead?

> +        // SAFETY:
> +        // - `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, which
> +        //   has been initialized,
> +        // - `modname.as_char_ptr()` is a NULL terminated string.
> +        let ret = unsafe { bindings::__auxiliary_device_add(adev, modname.as_char_ptr()) };
> +        if ret != 0 {
> +            // SAFETY: `registration_data` was set above via `into_foreign()`.
> +            let _ = unsafe {
> +                Pin::<KBox<RegistrationData<T>>>::from_foreign((*adev).registration_data_rust)
> +            };

Nit: Please use `drop(unsafe { ... })` to explicitly drop.

Alice

Re: [PATCH 2/2] rust: driver core: remove drvdata() and driver_type

[Alice Ryhl]

On Tue, Apr 28, 2026 at 12:09:41AM +0200, Danilo Krummrich wrote:
> When drvdata() was introduced in commit 6f61a2637abe ("rust: device:
> introduce Device::drvdata()"), its commit message already noted that a
> direct accessor to the driver's bus device private data is not commonly
> required -- bus callbacks provide access through &self, and other entry
> points (IRQs, workqueues, IOCTLs, etc.) carry their own private data.
> 
> The sole motivation for drvdata() was inter-driver interaction -- an
> auxiliary driver deriving the parent's bus device private data from the
> parent device.
> 
> However, drvdata() exposes the driver's bus device private data beyond
> the driver's own scope. This creates ordering constraints; for instance
> drvdata may not be set yet when the first caller of drvdata() can
> appear. It also forces the driver's bus device private data to outlive
> all registrations that access it, which causes unnecessary
> complications.
> 
> Private data should be private to the entity that issues it, i.e. bus
> device private data belongs to bus callbacks, class device private data
> to class callbacks, IRQ private data to the IRQ handler, etc.
> 
> With registration-private data now available through the auxiliary bus,
> there is no remaining user of drvdata(), thus remove it.
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

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

Re: [PATCH 1/2] rust: auxiliary: add registration data to auxiliary devices

[Danilo Krummrich]

On Thu Apr 30, 2026 at 10:59 AM CEST, Alice Ryhl wrote:
> Is this really Rust-specific? Would you not want C drivers with the same
> pattern to do the same thing?

Unlike in C, it has the effect that it allows us to let the compiler ensure that
the initilization order in probe() is correct.

The only advantage I see in C is that it provides a dedicated place for drivers
to differentiate specific private data for when a driver registers multiple
auxiliary devices per instance.

>> +        // SAFETY: `ptr` is non-null and was set via `into_foreign()` in `Registration::new()`;
>> +        // `RegistrationData` is `#[repr(C)]` with `type_id` at offset 0, so reading a `TypeId`
>> +        // at the start of the allocation is valid regardless of `T`.
>> +        let type_id = unsafe { ptr.cast::<TypeId>().read() };
>> +        if type_id != TypeId::of::<T>() {
>> +            return Err(EINVAL);
>> +        }
>
> Right, okay, so if you put C stuff there, we need the layout to be
> compatible with Rust type ids.

Correct.

> Still, we could have Rust expose a couple methods to allow C code to use
> the same field with a null type id.

I think there are multiple options, e.g. two registration data pointers, one
common pointer and a boolean, one common pointer, plus a type id pointer, etc.

> But I guess this is all future work.

Yes, I rather do that once we actually need it.

>> +        let data = KBox::pin_init::<Error>(
>> +            try_pin_init!(RegistrationData {
>> +                type_id: TypeId::of::<T>(),
>> +                data <- data,
>> +            }),
>> +            GFP_KERNEL,
>> +        )?;
>> +
>> +        let boxed = KBox::new(Opaque::<bindings::auxiliary_device>::zeroed(), GFP_KERNEL)?;
>
> Use __GFP_ZERO here instead?

This is just moving existing code, but I guess we could add something like:

	pub fn zeroed(flags: Flags) -> Result<Self, AllocError>
	where
	    T: Zeroable,
	{
	    // SAFETY: `__GFP_ZERO` guarantees the memory is zeroed, and `T: Zeroable` guarantees
	    // that all-zeroes is a valid bit pattern for `T`.
	    Ok(unsafe { Self::new_uninit(flags | __GFP_ZERO)?.assume_init() })
	}

>> +        // SAFETY:
>> +        // - `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, which
>> +        //   has been initialized,
>> +        // - `modname.as_char_ptr()` is a NULL terminated string.
>> +        let ret = unsafe { bindings::__auxiliary_device_add(adev, modname.as_char_ptr()) };
>> +        if ret != 0 {
>> +            // SAFETY: `registration_data` was set above via `into_foreign()`.
>> +            let _ = unsafe {
>> +                Pin::<KBox<RegistrationData<T>>>::from_foreign((*adev).registration_data_rust)
>> +            };
>
> Nit: Please use `drop(unsafe { ... })` to explicitly drop.

Same here, just moving existing code, but I think it's fine to change while at
it.

Re: [PATCH 1/2] rust: auxiliary: add registration data to auxiliary devices

[Gary Guo]

On Thu Apr 30, 2026 at 9:59 AM BST, Alice Ryhl wrote:
> On Tue, Apr 28, 2026 at 12:09:40AM +0200, Danilo Krummrich wrote:
>> Add a registration_data pointer to struct auxiliary_device, allowing the
>> registering (parent) driver to attach private data to the device at
>> registration time and retrieve it later when called back by the
>> auxiliary (child) driver.
>> 
>> By tying the data to the device's registration, Rust drivers can bind
>> the lifetime of device resources to it, since the auxiliary bus
>> guarantees that the parent driver remains bound while the auxiliary
>> device is bound.
>> 
>> On the Rust side, Registration<T> takes ownership of the data via
>> ForeignOwnable. A TypeId is stored alongside the data for runtime type
>> checking, making Device::registration_data<T>() a safe method.
>> 
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>
> So overall I think this patch makes sense. A few comments below.
>
>> diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
>> index bc09b55e3682..4e1ad8ccbcdd 100644
>> --- a/include/linux/auxiliary_bus.h
>> +++ b/include/linux/auxiliary_bus.h
>> @@ -62,6 +62,9 @@
>>   * @sysfs.irqs: irqs xarray contains irq indices which are used by the device,
>>   * @sysfs.lock: Synchronize irq sysfs creation,
>>   * @sysfs.irq_dir_exists: whether "irqs" directory exists,
>> + * @registration_data_rust: private data owned by the registering (parent)
>> + *                          driver; valid for as long as the device is
>> + *                          registered with the driver core,
>>   *
>>   * An auxiliary_device represents a part of its parent device's functionality.
>>   * It is given a name that, combined with the registering drivers
>> @@ -148,6 +151,7 @@ struct auxiliary_device {
>>  		struct mutex lock; /* Synchronize irq sysfs creation */
>>  		bool irq_dir_exists;
>>  	} sysfs;
>> +	void *registration_data_rust;
>
> Is this really Rust-specific? Would you not want C drivers with the same
> pattern to do the same thing?
>
>> +        // SAFETY: `ptr` is non-null and was set via `into_foreign()` in `Registration::new()`;
>> +        // `RegistrationData` is `#[repr(C)]` with `type_id` at offset 0, so reading a `TypeId`
>> +        // at the start of the allocation is valid regardless of `T`.
>> +        let type_id = unsafe { ptr.cast::<TypeId>().read() };
>> +        if type_id != TypeId::of::<T>() {
>> +            return Err(EINVAL);
>> +        }
>
> Right, okay, so if you put C stuff there, we need the layout to be
> compatible with Rust type ids.
>
> Still, we could have Rust expose a couple methods to allow C code to use
> the same field with a null type id.
>
> But I guess this is all future work.
>
>> +        let data = KBox::pin_init::<Error>(
>> +            try_pin_init!(RegistrationData {
>> +                type_id: TypeId::of::<T>(),
>> +                data <- data,
>> +            }),
>> +            GFP_KERNEL,
>> +        )?;
>> +
>> +        let boxed = KBox::new(Opaque::<bindings::auxiliary_device>::zeroed(), GFP_KERNEL)?;
>
> Use __GFP_ZERO here instead?

On thing I wanted to implement in pin-init is a way to hint whether a type
prefers uninit or zero-init before initializing. Something like

    pub unsafe trait PinInit<T: ?Sized, E = Infallible>: Sized {
        const PREFER_ZERO_INIT: bool = false;

        unsafe fn __pinned_init(self, slot: *mut T) -> Result<(), E>;

        // Extra safety requirement that `slot` is zero-filled.
        unsafe fn __pinned_init_from_zero(self, slot: *mut T) -> Result<(), E> {
            self.__pinned_init(slot)
        }
    }

Then the `pin_init` macro, if it detects that `..Zeroable::zeroed()` is used,
can generate a impl where `PREFER_ZERO_INIT` is set to `true`, `__pinned_init`
that does a memset + `__pinned_init_from_zero`.

Then we can have `KBox::pin_init` checks `PREFER_ZERO_INIT`. If true, it adds a
`__GFP_ZERO` flag and invoke the `__pinned_init_from_zero` method, other wise it
uses the `__pinned_init` method without the __GFP_ZERO flag.

I've been tempting to this for a while now, but I prioritize landing the
self-referential feature before this one.

Best,
Gary

Re: [PATCH 1/2] rust: auxiliary: add registration data to auxiliary devices

[Gary Guo]

On Mon Apr 27, 2026 at 11:09 PM BST, Danilo Krummrich wrote:
> Add a registration_data pointer to struct auxiliary_device, allowing the
> registering (parent) driver to attach private data to the device at
> registration time and retrieve it later when called back by the
> auxiliary (child) driver.
>
> By tying the data to the device's registration, Rust drivers can bind
> the lifetime of device resources to it, since the auxiliary bus
> guarantees that the parent driver remains bound while the auxiliary
> device is bound.
>
> On the Rust side, Registration<T> takes ownership of the data via
> ForeignOwnable. A TypeId is stored alongside the data for runtime type
> checking, making Device::registration_data<T>() a safe method.

I wonder if we could require auxillary drivers to specify a type, and then the
abstraction would check if the type matches; if it matches, it create a
`&Device<Core, T>` and `probe`, otherwise it skips over the driver completely.

This gets rid of the failure path.

>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
>  drivers/gpu/nova-core/driver.rs       |  10 +-
>  include/linux/auxiliary_bus.h         |   4 +
>  rust/kernel/auxiliary.rs              | 208 ++++++++++++++++++--------
>  samples/rust/rust_driver_auxiliary.rs |  40 +++--
>  4 files changed, 180 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/gpu/nova-core/driver.rs b/drivers/gpu/nova-core/driver.rs
> index 84b0e1703150..8fe484d357f6 100644
> --- a/drivers/gpu/nova-core/driver.rs
> +++ b/drivers/gpu/nova-core/driver.rs
> @@ -32,8 +32,7 @@
>  pub(crate) struct NovaCore {
>      #[pin]
>      pub(crate) gpu: Gpu,
> -    #[pin]
> -    _reg: Devres<auxiliary::Registration>,
> +    _reg: Devres<auxiliary::Registration<()>>,
>  }
>  
>  const BAR0_SIZE: usize = SZ_16M;
> @@ -96,14 +95,15 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, E
>  
>              Ok(try_pin_init!(Self {
>                  gpu <- Gpu::new(pdev, bar.clone(), bar.access(pdev.as_ref())?),
> -                _reg <- auxiliary::Registration::new(
> +                _reg: auxiliary::Registration::new(
>                      pdev.as_ref(),
>                      c"nova-drm",
>                      // TODO[XARR]: Use XArray or perhaps IDA for proper ID allocation/recycling. For
>                      // now, use a simple atomic counter that never recycles IDs.
>                      AUXILIARY_ID_COUNTER.fetch_add(1, Relaxed),
> -                    crate::MODULE_NAME
> -                ),
> +                    crate::MODULE_NAME,
> +                    (),
> +                )?,
>              }))
>          })
>      }
> diff --git a/include/linux/auxiliary_bus.h b/include/linux/auxiliary_bus.h
> index bc09b55e3682..4e1ad8ccbcdd 100644
> --- a/include/linux/auxiliary_bus.h
> +++ b/include/linux/auxiliary_bus.h
> @@ -62,6 +62,9 @@
>   * @sysfs.irqs: irqs xarray contains irq indices which are used by the device,
>   * @sysfs.lock: Synchronize irq sysfs creation,
>   * @sysfs.irq_dir_exists: whether "irqs" directory exists,
> + * @registration_data_rust: private data owned by the registering (parent)
> + *                          driver; valid for as long as the device is
> + *                          registered with the driver core,
>   *
>   * An auxiliary_device represents a part of its parent device's functionality.
>   * It is given a name that, combined with the registering drivers
> @@ -148,6 +151,7 @@ struct auxiliary_device {
>  		struct mutex lock; /* Synchronize irq sysfs creation */
>  		bool irq_dir_exists;
>  	} sysfs;
> +	void *registration_data_rust;
>  };
>  
>  /**
> diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs
> index 93c0db1f6655..467befea8e44 100644
> --- a/rust/kernel/auxiliary.rs
> +++ b/rust/kernel/auxiliary.rs
> @@ -19,12 +19,17 @@
>          to_result, //
>      },
>      prelude::*,
> -    types::Opaque,
> +    types::{
> +        ForeignOwnable,
> +        Opaque, //
> +    },
>      ThisModule, //
>  };
>  use core::{
> +    any::TypeId,
>      marker::PhantomData,
>      mem::offset_of,
> +    pin::Pin,
>      ptr::{
>          addr_of_mut,
>          NonNull, //
> @@ -257,6 +262,40 @@ pub fn parent(&self) -> &device::Device<device::Bound> {
>          // SAFETY: A bound auxiliary device always has a bound parent device.
>          unsafe { parent.as_bound() }
>      }
> +
> +    /// Returns a pinned reference to the registration data set by the registering (parent) driver.
> +    ///
> +    /// Returns [`EINVAL`] if `T` does not match the type used by the parent driver when calling
> +    /// [`Registration::new()`].
> +    ///
> +    /// Returns [`ENOENT`] if no registration data has been set, e.g. when the device was
> +    /// registered by a C driver.
> +    pub fn registration_data<T: 'static>(&self) -> Result<Pin<&T>> {

Any reason that this is not just `Result<&T>`?

> +        // SAFETY: By the type invariant, `self.as_raw()` is a valid `struct auxiliary_device`.
> +        let ptr = unsafe { (*self.as_raw()).registration_data_rust };
> +        if ptr.is_null() {

I think we could treat C-registered aux devices as if their data is `()`.

So you could do

    if TypeId::of::<T>() != TypeId::of::<()> {
        return Err(EINVAL);
    }
    return Ok(Pin::static_ref(&()));

here.

Best,
Gary

> +            dev_warn!(
> +                self.as_ref(),
> +                "No registration data set; parent is not a Rust driver.\n"
> +            );
> +            return Err(ENOENT);
> +        }
> +
> +        // SAFETY: `ptr` is non-null and was set via `into_foreign()` in `Registration::new()`;
> +        // `RegistrationData` is `#[repr(C)]` with `type_id` at offset 0, so reading a `TypeId`
> +        // at the start of the allocation is valid regardless of `T`.
> +        let type_id = unsafe { ptr.cast::<TypeId>().read() };
> +        if type_id != TypeId::of::<T>() {
> +            return Err(EINVAL);
> +        }
> +
> +        // SAFETY: The `TypeId` check above confirms that the stored type is `T`; `ptr` remains
> +        // valid until `Registration::drop()` calls `from_foreign()`.
> +        let wrapper = unsafe { Pin::<KBox<RegistrationData<T>>>::borrow(ptr) };
> +
> +        // SAFETY: `data` is a structurally pinned field of `RegistrationData`.
> +        Ok(unsafe { wrapper.map_unchecked(|w| &w.data) })
> +    }
>  }
>  

Re: [PATCH 1/2] rust: auxiliary: add registration data to auxiliary devices

[Danilo Krummrich]

On Thu Apr 30, 2026 at 5:08 PM CEST, Gary Guo wrote:
> I wonder if we could require auxillary drivers to specify a type, and then the
> abstraction would check if the type matches; if it matches, it create a
> `&Device<Core, T>` and `probe`, otherwise it skips over the driver completely.

The choice not to type auxiliary::Device over the registration data is
intentional.

The registration data is not intended for the child driver to use, but for the
parent driver to retrieve when the child passes its device back to the parent.

By typing the Device over the parent's registration data we force the parent
driver to expose its registration data to the child driver.

>> +    /// Returns a pinned reference to the registration data set by the registering (parent) driver.
>> +    ///
>> +    /// Returns [`EINVAL`] if `T` does not match the type used by the parent driver when calling
>> +    /// [`Registration::new()`].
>> +    ///
>> +    /// Returns [`ENOENT`] if no registration data has been set, e.g. when the device was
>> +    /// registered by a C driver.
>> +    pub fn registration_data<T: 'static>(&self) -> Result<Pin<&T>> {
>
> Any reason that this is not just `Result<&T>`?

Mainly consistency with other APIs, but I'm not really opinionated about it.