public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/3] rust: auxiliary: replace drvdata() with registration data
@ 2026-05-05 15:23 Danilo Krummrich
  2026-05-05 15:23 ` [PATCH v2 1/3] rust: alloc: add Box::zeroed() Danilo Krummrich
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-05-05 15:23 UTC (permalink / raw)
  To: gregkh, rafael, acourbot, aliceryhl, david.m.ertman, ira.weiny,
	leon, ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross
  Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux,
	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] https://lore.kernel.org/driver-core/20260427221155.2144848-1-dakr@kernel.org/

Changes in v2:
  - Introduce Box::zeroed()
  - Add Sync bound to Registration<T> impl block
  - Use KBox::zeroed() instead of KBox::new(Opaque::zeroed(), ...)
  - Use drop() instead of let _ = { ... }
  - Fix invariant doc reference

Danilo Krummrich (3):
  rust: alloc: add Box::zeroed()
  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/alloc/kbox.rs             |  22 +++
 rust/kernel/auxiliary.rs              | 208 ++++++++++++++++++--------
 rust/kernel/device.rs                 |  60 --------
 samples/rust/rust_driver_auxiliary.rs |  40 +++--
 7 files changed, 202 insertions(+), 158 deletions(-)


base-commit: 30c878ed169983190f77940594f8ba8948debe6b
-- 
2.54.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH v2 1/3] rust: alloc: add Box::zeroed()
  2026-05-05 15:23 [PATCH v2 0/3] rust: auxiliary: replace drvdata() with registration data Danilo Krummrich
@ 2026-05-05 15:23 ` Danilo Krummrich
  2026-05-05 15:48   ` Alice Ryhl
  2026-05-05 15:23 ` [PATCH v2 2/3] rust: auxiliary: add registration data to auxiliary devices Danilo Krummrich
  2026-05-05 15:23 ` [PATCH v2 3/3] rust: driver core: remove drvdata() and driver_type Danilo Krummrich
  2 siblings, 1 reply; 8+ messages in thread
From: Danilo Krummrich @ 2026-05-05 15:23 UTC (permalink / raw)
  To: gregkh, rafael, acourbot, aliceryhl, david.m.ertman, ira.weiny,
	leon, ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross
  Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux,
	Danilo Krummrich

Add Box::zeroed() for T: Zeroable types.

This allocates with __GFP_ZERO directly, letting the underlying
allocator deal with zeroing out the memory compared to
Box::new(T::zeroed(), flags).

Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 rust/kernel/alloc/kbox.rs | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
index bd6da02c7ab8..c824ed6e1523 100644
--- a/rust/kernel/alloc/kbox.rs
+++ b/rust/kernel/alloc/kbox.rs
@@ -19,6 +19,7 @@
 use crate::fmt;
 use crate::init::InPlaceInit;
 use crate::page::AsPageIter;
+use crate::prelude::*;
 use crate::types::ForeignOwnable;
 use pin_init::{InPlaceWrite, Init, PinInit, ZeroableOption};
 
@@ -256,6 +257,27 @@ pub fn new_uninit(flags: Flags) -> Result<Box<MaybeUninit<T>, A>, AllocError> {
         Ok(Box(ptr.cast(), PhantomData))
     }
 
+    /// Creates a new zero-initialized `Box<T, A>`.
+    ///
+    /// New memory is allocated with `A` and the [`__GFP_ZERO`] flag. The allocation may fail, in
+    /// which case an error is returned. For ZSTs no memory is allocated.
+    ///
+    /// # Examples
+    ///
+    /// ```
+    /// let b = KBox::<[u8; 128]>::zeroed(GFP_KERNEL)?;
+    /// assert_eq!(*b, [0; 128]);
+    /// # Ok::<(), Error>(())
+    /// ```
+    pub fn zeroed(flags: Flags) -> Result<Self, AllocError>
+    where
+        T: Zeroable,
+    {
+        // SAFETY: `__GFP_ZERO` guarantees the memory is zeroed; `T: Zeroable` guarantees that
+        // all-zeroes is a valid bit pattern for `T`.
+        Ok(unsafe { Self::new_uninit(flags | __GFP_ZERO)?.assume_init() })
+    }
+
     /// Constructs a new `Pin<Box<T, A>>`. If `T` does not implement [`Unpin`], then `x` will be
     /// pinned in memory and can't be moved.
     #[inline]
-- 
2.54.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 2/3] rust: auxiliary: add registration data to auxiliary devices
  2026-05-05 15:23 [PATCH v2 0/3] rust: auxiliary: replace drvdata() with registration data Danilo Krummrich
  2026-05-05 15:23 ` [PATCH v2 1/3] rust: alloc: add Box::zeroed() Danilo Krummrich
@ 2026-05-05 15:23 ` Danilo Krummrich
  2026-05-05 15:23 ` [PATCH v2 3/3] rust: driver core: remove drvdata() and driver_type Danilo Krummrich
  2 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-05-05 15:23 UTC (permalink / raw)
  To: gregkh, rafael, acourbot, aliceryhl, david.m.ertman, ira.weiny,
	leon, ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross
  Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux,
	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..19aec94aa95b 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`] whose `registration_data_rust` field points to a
+/// valid `Pin<KBox<RegistrationData<T>>>`.
+pub struct Registration<T: 'static> {
+    adev: NonNull<bindings::auxiliary_device>,
+    _data: PhantomData<T>,
+}
+
+impl<T: Send + Sync + '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<Opaque<bindings::auxiliary_device>> = KBox::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()`.
+            drop(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 + Sync> 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> 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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH v2 3/3] rust: driver core: remove drvdata() and driver_type
  2026-05-05 15:23 [PATCH v2 0/3] rust: auxiliary: replace drvdata() with registration data Danilo Krummrich
  2026-05-05 15:23 ` [PATCH v2 1/3] rust: alloc: add Box::zeroed() Danilo Krummrich
  2026-05-05 15:23 ` [PATCH v2 2/3] rust: auxiliary: add registration data to auxiliary devices Danilo Krummrich
@ 2026-05-05 15:23 ` Danilo Krummrich
  2 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-05-05 15:23 UTC (permalink / raw)
  To: gregkh, rafael, acourbot, aliceryhl, david.m.ertman, ira.weiny,
	leon, ojeda, boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross
  Cc: driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux,
	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.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
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 0ed1e278b957..483b99b4fa3d 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


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/3] rust: alloc: add Box::zeroed()
  2026-05-05 15:23 ` [PATCH v2 1/3] rust: alloc: add Box::zeroed() Danilo Krummrich
@ 2026-05-05 15:48   ` Alice Ryhl
  2026-05-05 20:04     ` Danilo Krummrich
  0 siblings, 1 reply; 8+ messages in thread
From: Alice Ryhl @ 2026-05-05 15:48 UTC (permalink / raw)
  To: Danilo Krummrich
  Cc: gregkh, rafael, acourbot, david.m.ertman, ira.weiny, leon, ojeda,
	boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross, driver-core,
	linux-kernel, nova-gpu, dri-devel, rust-for-linux

On Tue, May 05, 2026 at 05:23:07PM +0200, Danilo Krummrich wrote:
> Add Box::zeroed() for T: Zeroable types.
> 
> This allocates with __GFP_ZERO directly, letting the underlying
> allocator deal with zeroing out the memory compared to
> Box::new(T::zeroed(), flags).
> 
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

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

>  rust/kernel/alloc/kbox.rs | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/rust/kernel/alloc/kbox.rs b/rust/kernel/alloc/kbox.rs
> index bd6da02c7ab8..c824ed6e1523 100644
> --- a/rust/kernel/alloc/kbox.rs
> +++ b/rust/kernel/alloc/kbox.rs
> @@ -19,6 +19,7 @@
>  use crate::fmt;
>  use crate::init::InPlaceInit;
>  use crate::page::AsPageIter;
> +use crate::prelude::*;
>  use crate::types::ForeignOwnable;
>  use pin_init::{InPlaceWrite, Init, PinInit, ZeroableOption};

I suspect some of these imports can be removed when you add the prelude.

> @@ -256,6 +257,27 @@ pub fn new_uninit(flags: Flags) -> Result<Box<MaybeUninit<T>, A>, AllocError> {
>          Ok(Box(ptr.cast(), PhantomData))
>      }
>  
> +    /// Creates a new zero-initialized `Box<T, A>`.
> +    ///
> +    /// New memory is allocated with `A` and the [`__GFP_ZERO`] flag. The allocation may fail, in
> +    /// which case an error is returned. For ZSTs no memory is allocated.
> +    ///
> +    /// # Examples
> +    ///
> +    /// ```
> +    /// let b = KBox::<[u8; 128]>::zeroed(GFP_KERNEL)?;
> +    /// assert_eq!(*b, [0; 128]);
> +    /// # Ok::<(), Error>(())
> +    /// ```
> +    pub fn zeroed(flags: Flags) -> Result<Self, AllocError>

#[inline]?

Alice

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/3] rust: alloc: add Box::zeroed()
  2026-05-05 15:48   ` Alice Ryhl
@ 2026-05-05 20:04     ` Danilo Krummrich
  2026-05-05 20:06       ` Gary Guo
  0 siblings, 1 reply; 8+ messages in thread
From: Danilo Krummrich @ 2026-05-05 20:04 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: gregkh, rafael, acourbot, david.m.ertman, ira.weiny, leon, ojeda,
	boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross, driver-core,
	linux-kernel, nova-gpu, dri-devel, rust-for-linux

On Tue May 5, 2026 at 5:48 PM CEST, Alice Ryhl wrote:
> I suspect some of these imports can be removed when you add the prelude.

Yes, but I want to convert the imports separately, so keeping the change
minimal.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/3] rust: alloc: add Box::zeroed()
  2026-05-05 20:04     ` Danilo Krummrich
@ 2026-05-05 20:06       ` Gary Guo
  2026-05-05 20:10         ` Danilo Krummrich
  0 siblings, 1 reply; 8+ messages in thread
From: Gary Guo @ 2026-05-05 20:06 UTC (permalink / raw)
  To: Danilo Krummrich, Alice Ryhl
  Cc: gregkh, rafael, acourbot, david.m.ertman, ira.weiny, leon, ojeda,
	boqun, gary, bjorn3_gh, lossin, a.hindborg, tmgross, driver-core,
	linux-kernel, nova-gpu, dri-devel, rust-for-linux

On Tue May 5, 2026 at 9:04 PM BST, Danilo Krummrich wrote:
> On Tue May 5, 2026 at 5:48 PM CEST, Alice Ryhl wrote:
>> I suspect some of these imports can be removed when you add the prelude.
>
> Yes, but I want to convert the imports separately, so keeping the change
> minimal.

So far we've been asking people to convert import styles whenever imports are
being touched. What's the reason for doing that in this specific case?

Best,
Gary

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH v2 1/3] rust: alloc: add Box::zeroed()
  2026-05-05 20:06       ` Gary Guo
@ 2026-05-05 20:10         ` Danilo Krummrich
  0 siblings, 0 replies; 8+ messages in thread
From: Danilo Krummrich @ 2026-05-05 20:10 UTC (permalink / raw)
  To: Gary Guo
  Cc: Alice Ryhl, gregkh, rafael, acourbot, david.m.ertman, ira.weiny,
	leon, ojeda, boqun, bjorn3_gh, lossin, a.hindborg, tmgross,
	driver-core, linux-kernel, nova-gpu, dri-devel, rust-for-linux

On Tue May 5, 2026 at 10:06 PM CEST, Gary Guo wrote:
> On Tue May 5, 2026 at 9:04 PM BST, Danilo Krummrich wrote:
>> On Tue May 5, 2026 at 5:48 PM CEST, Alice Ryhl wrote:
>>> I suspect some of these imports can be removed when you add the prelude.
>>
>> Yes, but I want to convert the imports separately, so keeping the change
>> minimal.
>
> So far we've been asking people to convert import styles whenever imports are
> being touched. What's the reason for doing that in this specific case?

I will take this through the driver-core tree as a dependency; the import update
for the alloc module will go through the alloc tree. A minimal change avoids
unnecessary conflicts.

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-05-05 20:11 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-05 15:23 [PATCH v2 0/3] rust: auxiliary: replace drvdata() with registration data Danilo Krummrich
2026-05-05 15:23 ` [PATCH v2 1/3] rust: alloc: add Box::zeroed() Danilo Krummrich
2026-05-05 15:48   ` Alice Ryhl
2026-05-05 20:04     ` Danilo Krummrich
2026-05-05 20:06       ` Gary Guo
2026-05-05 20:10         ` Danilo Krummrich
2026-05-05 15:23 ` [PATCH v2 2/3] rust: auxiliary: add registration data to auxiliary devices Danilo Krummrich
2026-05-05 15:23 ` [PATCH v2 3/3] rust: driver core: remove drvdata() and driver_type Danilo Krummrich

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox