rust: device: Higher-Ranked Lifetime Types for device drivers
Subsystems (20)
Danilo Krummrich
maintainerv5 cover rust: device: Higher-Ranked Lifetime Types for device drivers
Currently, Rust device drivers access device resources such as PCI BAR mappings and I/O memory regions through Devres<T>.
Devres::access() provides zero-overhead access by taking a &Device<Bound> reference as proof that the device is still bound. Since a &Device<Bound> is available in almost all contexts by design, Devres is mostly a type-system level proof that the resource is valid, but it can also be used from scopes without this guarantee through its try_access() accessor.
This works well in general, but has a few limitations:
- Every access to a device resource goes through Devres::access(), which despite zero cost, adds boilerplate to every access site.
- Destructors do not receive a &Device<Bound>, so they must use try_access(), which can fail. In practice the access succeeds if teardown ordering is correct, but the type system can't express this, forcing drivers to handle a failure path that should never be taken.
- Sharing a resource across components (e.g. passing a BAR to a sub-component) requires Arc<Devres<T>>.
- Device references must be stored as ARef<Device> rather than plain &Device borrows.
These limitations stem from the driver's bus device private data being 'static -- the driver struct cannot borrow from the device reference it receives in probe(), even though it structurally cannot outlive the device binding.
This series introduces Higher-Ranked Lifetime Types (HRT) for Rust device drivers. An HRT is a type that is generic over a lifetime -- it does not have a fixed lifetime, but can be instantiated with any lifetime chosen by the caller.
Bus driver traits use a Generic Associated Type (GAT) type Data<'bound> to introduce the lifetime on the private data, rather than parameterizing the Driver trait itself. This avoids a driver trait global lifetime and avoids the need for ForLt for bus device private data, making the bus implementations much simpler. ForLt is only needed for auxiliary registration data, where the lifetime is not introduced by a trait callback but must be threaded through Registration.
With HRT, driver structs carry a lifetime parameter tied to the device binding scope -- the interval of a bus device being bound to a driver. Device resources like pci::Bar<'bound> and IoMem<'bound> are handed out with this lifetime, so the compiler enforces at build time that they do not escape the binding scope.
Before:
struct MyDriver {
pdev: ARef<pci::Device>,
bar: Devres<pci::Bar<BAR_SIZE>>,
}
let io = self.bar.access(dev)?;
io.read32(OFFSET);After:
struct MyDriver<'bound> {
pdev: &'bound pci::Device,
bar: pci::Bar<'bound, BAR_SIZE>,
}
self.bar.read32(OFFSET);Lifetime-parameterized device resources can be put into a Devres at any point via Bar::into_devres() / IoMem::into_devres(), providing the exact same semantics as before. This is useful for resources shared across subsystem boundaries where revocation is needed.
This also synergizes with the upcoming self-referential initialization support in pin-init, which allows one field of the driver struct to borrow another during initialization without unsafe code.
The same pattern is applied to auxiliary device registration data as a first example beyond bus device private data. Registration<F: ForLt> can hold lifetime-parameterized data tied to the parent driver's binding scope. Since the auxiliary bus guarantees that the parent remains bound while the auxiliary device is registered, the registration data can safely borrow the parent's device resources.
More generally, binding resource lifetimes to a registration scope applies to every registration that is scoped to a driver binding -- auxiliary devices, class devices, IRQ handlers, workqueues.
A follow-up series extends this to class device registrations, starting with DRM, so that class device callbacks (IOCTLs, etc.) can safely access device resources through the separate registration data bound to the registration's lifetime without Devres indirection.
Thanks to Gary for coming up with the ForLt implementation; thanks to Alice for the early discussions around lifetime-parameterized private data that helped shape the direction of this work.
Changes in v5:
- Fix Bar name parameter to &'static CStr
- ForLt: handle macro_rules! invocations in covariance/WF checks by conservatively assuming they may contain lifetimes; fix typos in commit message and doc comments
- Update stale SAFETY comments from T::Data to T::Data<'_> in bus driver callbacks
- Fix USB commit message: unbind() -> disconnect()
- Clarify Bar commit message on why Devres is no longer needed
- Drop REF patches (nova-core, tyr); will be revisited and sent separately to be routed through drm-rust
Changes in v4:
- Use 'bound only for lifetimes that represent the full duration of a device being bound to a driver
- Make Core and CoreInternal lifetime-parameterized
- Split auxiliary::Registration constructor into unsafe fn new_with_lt (borrowed data) and safe fn new ('static data) to close mem::forget() soundness hole
- Use TypeId::of::<F>() instead of TypeId::of::<F::Of<'static>>() in auxiliary registration_data()
- Handle Type::Macro in ForLt! covariance and WF checks; proc macros cannot expand macro_rules invocations, so use WithLt trait projection for lifetime substitution
- Fix missing Send bound in type Data: Send
Changes in v3:
- Rework all bus "make Driver trait lifetime-parameterized" patches to use a GAT (type Data<'bound>); in the context of adding a 'bound lifetime, this avoids a driver trait global lifetime and avoids ForLt for bus device private data making the bus implementations much simpler
Changes in v2:
- Add 'a bound to ForLt::Of<'a> and WithLt::Of, making the lifetime bound inherent to the trait; remove all F::Of<'static>: 'static where clauses
- Drop "rust: devres: add ForLt support to Devres"; Devres itself stays unchanged -- ForLt-aware access is introduced later through DevresLt in a separate series
- Use 'bound instead of 'a; add patches to consistently use 'bound for pre-existing 'a
Danilo Krummrich (21):
rust: pci: use 'static lifetime for PCI BAR resource names
rust: driver: decouple driver private data from driver type
rust: driver core: drop drvdata before devres release
rust: pci: implement Sync for Device<Bound>
rust: platform: implement Sync for Device<Bound>
rust: auxiliary: implement Sync for Device<Bound>
rust: usb: implement Sync for Device<Bound>
rust: device: implement Sync for Device<Bound>
rust: device: make Core and CoreInternal lifetime-parameterized
rust: pci: make Driver trait lifetime-parameterized
rust: platform: make Driver trait lifetime-parameterized
rust: auxiliary: make Driver trait lifetime-parameterized
rust: usb: make Driver trait lifetime-parameterized
rust: i2c: make Driver trait lifetime-parameterized
rust: driver: update module documentation for GAT-based Data type
rust: pci: make Bar lifetime-parameterized
rust: io: make IoMem and ExclusiveIoMem lifetime-parameterized
samples: rust: rust_driver_pci: use HRT lifetime for Bar
gpu: nova-core: separate driver type from driver data
rust: auxiliary: generalize Registration over ForLt
samples: rust: rust_driver_auxiliary: showcase lifetime-bound
registration data
Gary Guo (3):
rust: alloc: remove 'static bound on ForeignOwnable
rust: driver: move 'static bounds to constructor
rust: types: add ForLt trait for higher-ranked lifetime support
drivers/base/dd.c | 2 +- drivers/cpufreq/rcpufreq_dt.rs | 9 +- drivers/gpu/drm/nova/driver.rs | 6 +- drivers/gpu/drm/tyr/driver.rs | 13 +- drivers/gpu/nova-core/driver.rs | 30 ++-- drivers/gpu/nova-core/gpu.rs | 2 +- drivers/gpu/nova-core/nova_core.rs | 2 +- drivers/pwm/pwm_th1520.rs | 13 +- include/linux/device/driver.h | 4 +- rust/Makefile | 1 + rust/kernel/alloc/kbox.rs | 24 ++- rust/kernel/auxiliary.rs | 142 ++++++++++----- rust/kernel/cpufreq.rs | 9 +- rust/kernel/device.rs | 61 +++++-- rust/kernel/devres.rs | 2 +- rust/kernel/dma.rs | 2 +- rust/kernel/driver.rs | 41 +++-- rust/kernel/i2c.rs | 61 ++++--- rust/kernel/io/mem.rs | 121 +++++++------ rust/kernel/pci.rs | 51 +++--- rust/kernel/pci/id.rs | 2 +- rust/kernel/pci/io.rs | 54 +++--- rust/kernel/platform.rs | 52 +++--- rust/kernel/types.rs | 12 +- rust/kernel/types/for_lt.rs | 117 ++++++++++++ rust/kernel/usb.rs | 57 +++--- rust/macros/for_lt.rs | 248 ++++++++++++++++++++++++++ rust/macros/lib.rs | 12 ++ samples/rust/rust_debugfs.rs | 11 +- samples/rust/rust_dma.rs | 6 +- samples/rust/rust_driver_auxiliary.rs | 79 +++++--- samples/rust/rust_driver_i2c.rs | 13 +- samples/rust/rust_driver_pci.rs | 90 +++++----- samples/rust/rust_driver_platform.rs | 9 +- samples/rust/rust_driver_usb.rs | 15 +- samples/rust/rust_i2c_client.rs | 14 +- samples/rust/rust_soc.rs | 9 +- 37 files changed, 998 insertions(+), 398 deletions(-) create mode 100644 rust/kernel/types/for_lt.rs create mode 100644 rust/macros/for_lt.rs
base-commit: 95ade775c4ab9b9b3d7cfa2d45283e93fbfa4e7a
Signature · 1 line
2.54.0
Danilo Krummrich
maintainerPATCH v5 1/24 rust: pci: use 'static lifetime for PCI BAR resource names
pci_request_region() stores the name pointer directly in struct resource; use &'static CStr to ensure the pointer remains valid even if the Bar is leaked.
--- rust/kernel/pci/io.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
rust/kernel/pci/io.rs +3 -3
@@ -153,7 +153,7 @@ pub struct Bar<const SIZE: usize = 0> { }impl <const SIZE :usize >Bar <SIZE > { -pub (super )fn new (pdev: &Device , num:u32 , name: &CStr ) ->Result <Self > { +pub (super )fn new (pdev: &Device , num:u32 , name: &'staticCStr ) ->Result <Self > {let len = pdev.resource_len (num)?;if len ==0 {return Err (ENOMEM );
@@ -252,7 +252,7 @@ impl Device<device::Bound> {pub fn iomap_region_sized <'a,const SIZE :usize >( &'a self, bar:u32 , - name: &'aCStr , + name: &'staticCStr , ) ->impl PinInit <Devres <Bar <SIZE >>,Error > + 'a {Devres ::new (self.as_ref (),Bar ::<SIZE >::new (self, bar, name)) }
@@ -261,7 +261,7 @@ pub fn iomap_region_sized<'a, const SIZE: usize>(pub fn iomap_region <'a>( &'a self, bar:u32 , - name: &'aCStr , + name: &'staticCStr , ) ->impl PinInit <Devres <Bar >,Error > + 'a { self.iomap_region_sized ::<0 >(bar, name) }
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 01/24] rust: pci: use 'static lifetime for PCI BAR resource names
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Devres::new lacks a 'static bound on T, allowing safe Rust code to register objects with non-static lifetimes to the C devres subsystem and leak the Devres wrapper, leading to UAF on device unbind. --
commit 1ea8f39dcd612a476e90616d2d99987a3bfdbf27
Author: Danilo Krummrich <[email protected]>
rust: pci: use 'static lifetime for PCI BAR resource names
This commit updates the lifetime of the name parameter in Bar::new and iomap_region functions to a static CStr. This ensures the pointer remains valid even if the Bar is leaked, preventing potential use-after-free issues when pci_request_region stores the name pointer directly.
Quoted 4 lines
rust/kernel/pci/io.rs +0
-0
[ ... ]
rust/kernel/pci/io.rs impl Device<device::Bound> { · line 254
Quoted 8 lines
rust/kernel/pci/io.rs +1
-1@@ -249,10 +249,10 @@ impl Device<device::Bound> {pub fn iomap_region_sized <'a,const SIZE :usize >( &'a self, bar:u32 , - name: &'aCStr , + name: &'staticCStr , ) ->impl PinInit <Devres <Bar <SIZE >>,Error > + 'a {Devres ::new (self.as_ref (),Bar ::<SIZE >::new (self, bar, name))
[Severity: High]
This isn't a bug introduced by this patch, but does Devres::new lack a static bound on T?
Since Devres::new registers a C callback via devres_node_add() and can intentionally leak a reference into the C devres subsystem (which is safe in Rust via core::mem::forget), a user could pass an object with a non-static lifetime.
The C devres subsystem will eventually drop the object upon device unbind, which might happen long after any stack references have become invalid.
Should Devres or its constructor enforce a static bound on T to prevent this potential use-after-free?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1
Re: [PATCH v5 01/24] rust: pci: use 'static lifetime for PCI BAR resource names
In reply to Danilo KrummrichQuoted 10 lines · Danilo Krummrich
pci_request_region()stores the name pointer directly in struct resource; use &'static CStr to ensure the pointer remains valid even if the Bar is leaked.Reported-by Sashiko <[email protected]>Fixes 3c2e31d717ac ("rust: pci: move I/O infrastructure to separate file")Signed-off-by Danilo Krummrich <[email protected]>---
Re: [PATCH v5 01/24] rust: pci: use 'static lifetime for PCI BAR resource names
In reply to Danilo KrummrichQuoted 9 lines · Danilo Krummrich
pci_request_region()stores the name pointer directly in struct resource; use &'static CStr to ensure the pointer remains valid even if the Bar is leaked.Reported-by Sashiko <[email protected]>Fixes 3c2e31d717ac ("rust: pci: move I/O infrastructure to separate file")Signed-off-by Danilo Krummrich <[email protected]>
Re: [PATCH v5 01/24] rust: pci: use 'static lifetime for PCI BAR resource names
In reply to Danilo KrummrichQuoted 9 lines · Danilo Krummrich
pci_request_region()stores the name pointer directly in struct resource; use &'static CStr to ensure the pointer remains valid even if the Bar is leaked.Reported-by Sashiko <[email protected]>Fixes 3c2e31d717ac ("rust: pci: move I/O infrastructure to separate file")Signed-off-by Danilo Krummrich <[email protected]>
Quoted 3 lines
--- rust/kernel/pci/io.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
Danilo Krummrich
maintainerPATCH v5 2/24 rust: alloc: remove `'static` bound on `ForeignOwnable`
In reply to Danilo KrummrichFrom: Gary Guo <[email protected]>
The 'static bound is currently necessary because there's no restriction on the lifetime of the GAT. Add a Self: 'a bound to restrict possible lifetimes on Borrowed and BorrowedMut, and lift the 'static requirement.
--- rust/kernel/alloc/kbox.rs | 24 ++++++++++++++++++------ rust/kernel/types.rs | 8 ++++++-- 2 files changed, 24 insertions(+), 8 deletions(-)
rust/kernel/alloc/kbox.rs +18 -6
@@ -477,7 +477,7 @@ fn try_init<E>(init: impl Init<T, E>, flags: Flags) -> Result<Self, E>// SAFETY: The pointer returned by `into_foreign` comes from a well aligned // pointer to `T` allocated by `A`. -unsafe impl <T : 'static,A >ForeignOwnable for Box <T ,A > +unsafe impl <T ,A >ForeignOwnable for Box <T ,A >where A :Allocator , {
@@ -487,8 +487,14 @@ unsafe impl<T: 'static, A> ForeignOwnable for Box<T, A> core::mem::align_of ::<T >() }; -type Borrowed <'a> = &'aT ; -type BorrowedMut <'a> = &'amut T ; +type Borrowed <'a> + = &'aT +where +Self : 'a; +type BorrowedMut <'a> + = &'amut T +where +Self : 'a;fn into_foreign (self) -> *mut c_void {Box ::into_raw (self).cast ()
@@ -516,13 +522,19 @@ unsafe fn borrow_mut<'a>(ptr: *mut c_void) -> &'a mut T {// SAFETY: The pointer returned by `into_foreign` comes from a well aligned // pointer to `T` allocated by `A`. -unsafe impl <T : 'static,A >ForeignOwnable for Pin <Box <T ,A >> +unsafe impl <T ,A >ForeignOwnable for Pin <Box <T ,A >>where A :Allocator , {const FOREIGN_ALIGN :usize = <Box <T ,A >as ForeignOwnable >::FOREIGN_ALIGN ; -type Borrowed <'a> =Pin <&'aT >; -type BorrowedMut <'a> =Pin <&'amut T >; +type Borrowed <'a> + =Pin <&'aT > +where +Self : 'a; +type BorrowedMut <'a> + =Pin <&'amut T > +where +Self : 'a;fn into_foreign (self) -> *mut c_void {// SAFETY: We are still treating the box as pinned.
rust/kernel/types.rs +6 -2
@@ -27,10 +27,14 @@ pub unsafe trait ForeignOwnable: Sized {const FOREIGN_ALIGN :usize ;/// Type used to immutably borrow a value that is currently foreign-owned. -type Borrowed <'a>; +type Borrowed <'a> +where +Self : 'a;/// Type used to mutably borrow a value that is currently foreign-owned. -type BorrowedMut <'a>; +type BorrowedMut <'a> +where +Self : 'a;/// Converts a Rust-owned object to a foreign-owned one. ///
Signature · 1 line
2.54.0
Re: [PATCH v5 02/24] rust: alloc: remove `'static` bound on `ForeignOwnable`
Quoted 8 lines · Danilo Krummrich
From: Gary Guo <[email protected]>
The
'staticbound is currently necessary because there's no restriction on the lifetime of the GAT. Add aSelf: 'abound to restrict possible lifetimes onBorrowedandBorrowedMut, and lift the'staticrequirement.
Quoted 4 lines
Reviewed-by Alexandre Courbot <[email protected]>Reviewed-by Greg Kroah-Hartman <[email protected]>Signed-off-by Gary Guo <[email protected]>Signed-off-by Danilo Krummrich <[email protected]>
Cheers,
Miguel
Danilo Krummrich
maintainerPATCH v5 3/24 rust: driver: move 'static bounds to constructor
In reply to Danilo KrummrichFrom: Gary Guo <[email protected]>
With the ForeignOwnable lifetime change, the 'static bound is no longer necessary on the drvdata methods or bus adapter impls. Move it to the Registration constructor instead.
--- rust/kernel/auxiliary.rs | 6 +++--- rust/kernel/device.rs | 8 ++++---- rust/kernel/driver.rs | 7 +++++-- rust/kernel/i2c.rs | 8 ++++---- rust/kernel/pci.rs | 6 +++--- rust/kernel/platform.rs | 8 ++++---- rust/kernel/usb.rs | 6 +++--- 7 files changed, 26 insertions(+), 23 deletions(-)
rust/kernel/auxiliary.rs +3 -3
@@ -44,7 +44,7 @@// - `T` is the type of the driver's device private data. // - `struct auxiliary_driver` embeds a `struct device_driver`. // - `DEVICE_DRIVER_OFFSET` is the correct byte offset to the embedded `struct device_driver`. -unsafe impl <T :Driver + 'static> driver::DriverLayout for Adapter <T > { +unsafe impl <T :Driver > driver::DriverLayout for Adapter <T > {type DriverType = bindings::auxiliary_driver ;type DriverData =T ;const DEVICE_DRIVER_OFFSET :usize = core::mem::offset_of!(Self ::DriverType , driver);
@@ -52,7 +52,7 @@ unsafe impl<T: Driver + 'static> driver::DriverLayout for Adapter<T> {// SAFETY: A call to `unregister` for a given instance of `DriverType` is guaranteed to be valid if // a preceding call to `register` has been successful. -unsafe impl <T :Driver + 'static> driver::RegistrationOps for Adapter <T > { +unsafe impl <T :Driver > driver::RegistrationOps for Adapter <T > {unsafe fn register ( adrv: &Opaque <Self ::DriverType >, name: &'static CStr,
@@ -78,7 +78,7 @@ unsafe fn unregister(adrv: &Opaque<Self::DriverType>) { } } -impl <T :Driver + 'static>Adapter <T > { +impl <T :Driver >Adapter <T > {extern "C" fn probe_callback ( adev: *mut bindings::auxiliary_device , id: *const bindings::auxiliary_device_id,
rust/kernel/device.rs +4 -4
@@ -203,7 +203,7 @@ pub unsafe fn as_bound(&self) -> &Device<Bound> {impl Device <CoreInternal > {/// Store a pointer to the bound driver's private data. -pub fn set_drvdata <T : 'static>(&self, data:impl PinInit <T ,Error >) ->Result { +pub fn set_drvdata <T >(&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`.
@@ -218,7 +218,7 @@ pub fn set_drvdata<T: 'static>(&self, data: impl PinInit<T, Error>) -> Result {/// /// - The type `T` must match the type of the `ForeignOwnable` previously stored by /// [`Device::set_drvdata`]. -pub (crate )unsafe fn drvdata_obtain <T : 'static>(&self) ->Option <Pin <KBox <T >>> { +pub (crate )unsafe fn drvdata_obtain <T >(&self) ->Option <Pin <KBox <T >>> {// SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`. let ptr =unsafe { bindings::dev_get_drvdata (self.as_raw ()) };
@@ -244,7 +244,7 @@ pub(crate) unsafe fn drvdata_obtain<T: 'static>(&self) -> Option<Pin<KBox<T>>> {/// device is fully unbound. /// - The type `T` must match the type of the `ForeignOwnable` previously stored by /// [`Device::set_drvdata`]. -pub unsafe fn drvdata_borrow <T : 'static>(&self) ->Pin <&T > { +pub unsafe fn drvdata_borrow <T >(&self) ->Pin <&T > {// SAFETY: `drvdata_unchecked()` has the exact same safety requirements as the ones // required by this method. unsafe { self.drvdata_unchecked () }
@@ -260,7 +260,7 @@ impl Device<Bound> {/// the device is fully unbound. /// - The type `T` must match the type of the `ForeignOwnable` previously stored by /// [`Device::set_drvdata`]. -unsafe fn drvdata_unchecked <T : 'static>(&self) ->Pin <&T > { +unsafe fn drvdata_unchecked <T >(&self) ->Pin <&T > {// SAFETY: By the type invariants, `self.as_raw()` is a valid pointer to a `struct device`. let ptr =unsafe { bindings::dev_get_drvdata (self.as_raw ()) };
rust/kernel/driver.rs +5 -2
@@ -181,7 +181,7 @@ unsafe impl<T: RegistrationOps> Sync for Registration<T> {}// any thread, so `Registration` is `Send`. unsafe impl <T :RegistrationOps >Send for Registration <T > {} -impl <T :RegistrationOps + 'static>Registration <T > { +impl <T :RegistrationOps >Registration <T > {extern "C" fn post_unbind_callback (dev: *mut bindings::device ) {// SAFETY: The driver core only ever calls the post unbind callback with a valid pointer to // a `struct device`.
@@ -215,7 +215,10 @@ fn callbacks_attach(drv: &Opaque<T::DriverType>) { } /// Creates a new instance of the registration object. - pub fn new(name: &'static CStr, module: &'static ThisModule) -> impl PinInit<Self, Error> { + pub fn new(name: &'static CStr, module: &'static ThisModule) -> impl PinInit<Self, Error> + where + T: 'static, + { try_pin_init!(Self { reg <- Opaque::try_ffi_init(|ptr: *mut T::DriverType| { // SAFETY: `try_ffi_init` guarantees that `ptr` is valid for write.
rust/kernel/i2c.rs +4 -4
@@ -96,7 +96,7 @@ macro_rules! i2c_device_table {// - `T` is the type of the driver's device private data. // - `struct i2c_driver` embeds a `struct device_driver`. // - `DEVICE_DRIVER_OFFSET` is the correct byte offset to the embedded `struct device_driver`. -unsafe impl <T :Driver + 'static> driver::DriverLayout for Adapter <T > { +unsafe impl <T :Driver > driver::DriverLayout for Adapter <T > {type DriverType = bindings::i2c_driver ;type DriverData =T ;const DEVICE_DRIVER_OFFSET :usize = core::mem::offset_of!(Self ::DriverType , driver);
@@ -104,7 +104,7 @@ unsafe impl<T: Driver + 'static> driver::DriverLayout for Adapter<T> {// SAFETY: A call to `unregister` for a given instance of `DriverType` is guaranteed to be valid if // a preceding call to `register` has been successful. -unsafe impl <T :Driver + 'static> driver::RegistrationOps for Adapter <T > { +unsafe impl <T :Driver > driver::RegistrationOps for Adapter <T > {unsafe fn register ( idrv: &Opaque <Self ::DriverType >, name: &'static CStr,
@@ -151,7 +151,7 @@ unsafe fn unregister(idrv: &Opaque<Self::DriverType>) { } } -impl <T :Driver + 'static>Adapter <T > { +impl <T :Driver >Adapter <T > {extern "C" fn probe_callback (idev: *mut bindings::i2c_client ) -> kernel::ffi::c_int {// SAFETY: The I2C bus only ever calls the probe callback with a valid pointer to a // `struct i2c_client`.
@@ -222,7 +222,7 @@ fn i2c_id_info(dev: &I2cClient) -> Option<&'static <Self as driver::Adapter>::Id } } -impl <T :Driver + 'static> driver::Adapter for Adapter <T > { +impl <T :Driver > driver::Adapter for Adapter <T > {type IdInfo =T ::IdInfo ;fn of_id_table () ->Option <of::IdTable <Self ::IdInfo >> {
rust/kernel/pci.rs +3 -3
@@ -62,7 +62,7 @@// - `T` is the type of the driver's device private data. // - `struct pci_driver` embeds a `struct device_driver`. // - `DEVICE_DRIVER_OFFSET` is the correct byte offset to the embedded `struct device_driver`. -unsafe impl <T :Driver + 'static> driver::DriverLayout for Adapter <T > { +unsafe impl <T :Driver > driver::DriverLayout for Adapter <T > {type DriverType = bindings::pci_driver ;type DriverData =T ;const DEVICE_DRIVER_OFFSET :usize = core::mem::offset_of!(Self ::DriverType , driver);
@@ -70,7 +70,7 @@ unsafe impl<T: Driver + 'static> driver::DriverLayout for Adapter<T> {// SAFETY: A call to `unregister` for a given instance of `DriverType` is guaranteed to be valid if // a preceding call to `register` has been successful. -unsafe impl <T :Driver + 'static> driver::RegistrationOps for Adapter <T > { +unsafe impl <T :Driver > driver::RegistrationOps for Adapter <T > {unsafe fn register ( pdrv: &Opaque <Self ::DriverType >, name: &'static CStr,
@@ -96,7 +96,7 @@ unsafe fn unregister(pdrv: &Opaque<Self::DriverType>) { } } -impl <T :Driver + 'static>Adapter <T > { +impl <T :Driver >Adapter <T > {extern "C" fn probe_callback ( pdev: *mut bindings::pci_dev , id: *const bindings::pci_device_id,
rust/kernel/platform.rs +4 -4
@@ -48,7 +48,7 @@// - `T` is the type of the driver's device private data. // - `struct platform_driver` embeds a `struct device_driver`. // - `DEVICE_DRIVER_OFFSET` is the correct byte offset to the embedded `struct device_driver`. -unsafe impl <T :Driver + 'static> driver::DriverLayout for Adapter <T > { +unsafe impl <T :Driver > driver::DriverLayout for Adapter <T > {type DriverType = bindings::platform_driver ;type DriverData =T ;const DEVICE_DRIVER_OFFSET :usize = core::mem::offset_of!(Self ::DriverType , driver);
@@ -56,7 +56,7 @@ unsafe impl<T: Driver + 'static> driver::DriverLayout for Adapter<T> {// SAFETY: A call to `unregister` for a given instance of `DriverType` is guaranteed to be valid if // a preceding call to `register` has been successful. -unsafe impl <T :Driver + 'static> driver::RegistrationOps for Adapter <T > { +unsafe impl <T :Driver > driver::RegistrationOps for Adapter <T > {unsafe fn register ( pdrv: &Opaque <Self ::DriverType >, name: &'static CStr,
@@ -91,7 +91,7 @@ unsafe fn unregister(pdrv: &Opaque<Self::DriverType>) { } } -impl <T :Driver + 'static>Adapter <T > { +impl <T :Driver >Adapter <T > {extern "C" fn probe_callback (pdev: *mut bindings::platform_device ) -> kernel::ffi::c_int {// SAFETY: The platform bus only ever calls the probe callback with a valid pointer to a // `struct platform_device`.
@@ -124,7 +124,7 @@ extern "C" fn remove_callback(pdev: *mut bindings::platform_device) { } } -impl <T :Driver + 'static> driver::Adapter for Adapter <T > { +impl <T :Driver > driver::Adapter for Adapter <T > {type IdInfo =T ::IdInfo ;fn of_id_table () ->Option <of::IdTable <Self ::IdInfo >> {
rust/kernel/usb.rs +3 -3
@@ -39,7 +39,7 @@// - `T` is the type of the driver's device private data. // - `struct usb_driver` embeds a `struct device_driver`. // - `DEVICE_DRIVER_OFFSET` is the correct byte offset to the embedded `struct device_driver`. -unsafe impl <T :Driver + 'static> driver::DriverLayout for Adapter <T > { +unsafe impl <T :Driver > driver::DriverLayout for Adapter <T > {type DriverType = bindings::usb_driver ;type DriverData =T ;const DEVICE_DRIVER_OFFSET :usize = core::mem::offset_of!(Self ::DriverType , driver);
@@ -47,7 +47,7 @@ unsafe impl<T: Driver + 'static> driver::DriverLayout for Adapter<T> {// SAFETY: A call to `unregister` for a given instance of `DriverType` is guaranteed to be valid if // a preceding call to `register` has been successful. -unsafe impl <T :Driver + 'static> driver::RegistrationOps for Adapter <T > { +unsafe impl <T :Driver > driver::RegistrationOps for Adapter <T > {unsafe fn register ( udrv: &Opaque <Self ::DriverType >, name: &'static CStr,
@@ -73,7 +73,7 @@ unsafe fn unregister(udrv: &Opaque<Self::DriverType>) { } } -impl <T :Driver + 'static>Adapter <T > { +impl <T :Driver >Adapter <T > {extern "C" fn probe_callback ( intf: *mut bindings::usb_interface , id: *const bindings::usb_device_id,
Signature · 1 line
2.54.0
Danilo Krummrich
maintainerPATCH v5 4/24 rust: driver: decouple driver private data from driver type
In reply to Danilo KrummrichAdd a type Data<'bound> associated type to all bus driver traits, decoupling the driver's bus device private data type from the driver struct itself.
In the context of adding a 'bound lifetime, making this an associated type has the advantage that it allows us to avoid a driver trait global lifetime and it avoids the need for ForLt for bus device private data; both of which make the subsequent implementation by buses much simpler.
All existing drivers and doc examples set type Data = Self to preserve the current behavior.
--- drivers/cpufreq/rcpufreq_dt.rs | 1 + drivers/gpu/drm/nova/driver.rs | 1 + drivers/gpu/drm/tyr/driver.rs | 1 + drivers/gpu/nova-core/driver.rs | 1 + drivers/pwm/pwm_th1520.rs | 1 + rust/kernel/auxiliary.rs | 18 +++++++++------ rust/kernel/cpufreq.rs | 1 + rust/kernel/driver.rs | 24 +++++++++++--------- rust/kernel/i2c.rs | 32 +++++++++++++++------------ rust/kernel/io/mem.rs | 2 ++ rust/kernel/pci.rs | 21 +++++++++++------- rust/kernel/platform.rs | 20 ++++++++++------- rust/kernel/usb.rs | 20 ++++++++++------- samples/rust/rust_debugfs.rs | 1 + samples/rust/rust_dma.rs | 1 + samples/rust/rust_driver_auxiliary.rs | 2 ++ samples/rust/rust_driver_i2c.rs | 1 + samples/rust/rust_driver_pci.rs | 1 + samples/rust/rust_driver_platform.rs | 1 + samples/rust/rust_driver_usb.rs | 1 + samples/rust/rust_i2c_client.rs | 1 + samples/rust/rust_soc.rs | 1 + 22 files changed, 98 insertions(+), 55 deletions(-)
drivers/cpufreq/rcpufreq_dt.rs +1 -0
@@ -201,6 +201,7 @@ fn register_em(policy: &mut cpufreq::Policy) {impl platform::Driver for CPUFreqDTDriver {type IdInfo = (); +type Data =Self ;const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =Some (&OF_TABLE ); fn probe(
drivers/gpu/drm/nova/driver.rs +1 -0
@@ -51,6 +51,7 @@ pub(crate) struct NovaData {impl auxiliary::Driver for NovaDriver {type IdInfo = (); +type Data =Self ;const ID_TABLE : auxiliary::IdTable <Self ::IdInfo > = &AUX_TABLE ;fn probe (adev: &auxiliary::Device <Core >, _info: &Self ::IdInfo ) ->impl PinInit <Self ,Error > {
drivers/gpu/drm/tyr/driver.rs +1 -0
@@ -91,6 +91,7 @@ fn issue_soft_reset(dev: &Device<Bound>, iomem: &Devres<IoMem>) -> Result {impl platform::Driver for TyrPlatformDriverData {type IdInfo = (); +type Data =Self ;const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =Some (&OF_TABLE ); fn probe(
drivers/gpu/nova-core/driver.rs +1 -0
@@ -74,6 +74,7 @@ pub(crate) struct NovaCore {impl pci::Driver for NovaCore {type IdInfo = (); +type Data =Self ;const ID_TABLE : pci::IdTable <Self ::IdInfo > = &PCI_TABLE ;fn probe (pdev: &pci::Device <Core >, _info: &Self ::IdInfo ) ->impl PinInit <Self ,Error > {
drivers/pwm/pwm_th1520.rs +1 -0
@@ -316,6 +316,7 @@ fn drop(self: Pin<&mut Self>) {impl platform::Driver for Th1520PwmPlatformDriver {type IdInfo = (); +type Data =Self ;const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =Some (&OF_TABLE ); fn probe(
rust/kernel/auxiliary.rs +11 -7
@@ -41,12 +41,12 @@// SAFETY: // - `bindings::auxiliary_driver` is a C type declared as `repr(C)`. -// - `T` is the type of the driver's device private data. +// - `T::Data` is the type of the driver's device private data. // - `struct auxiliary_driver` embeds a `struct device_driver`. // - `DEVICE_DRIVER_OFFSET` is the correct byte offset to the embedded `struct device_driver`. unsafe impl <T :Driver > driver::DriverLayout for Adapter <T > {type DriverType = bindings::auxiliary_driver ; -type DriverData =T ; +type DriverData <'bound> =T ::Data ;const DEVICE_DRIVER_OFFSET :usize = core::mem::offset_of!(Self ::DriverType , driver); }
@@ -111,8 +111,8 @@ extern "C" fn remove_callback(adev: *mut bindings::auxiliary_device) {// SAFETY: `remove_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called -// and stored a `Pin<KBox<T>>`. -let data =unsafe { adev.as_ref ().drvdata_borrow ::<T >() }; +// and stored a `Pin<KBox<T::Data>>`. +let data =unsafe { adev.as_ref ().drvdata_borrow ::<T ::Data >() };T ::unbind (adev, data); }
@@ -202,13 +202,17 @@ pub trait Driver {/// type IdInfo: 'static = (); type IdInfo : 'static; +/// The type of the driver's bus device private data. +type Data :Send ; +/// The table of device ids supported by the driver. const ID_TABLE :IdTable <Self ::IdInfo >;/// Auxiliary driver probe. /// /// Called when an auxiliary device is matches a corresponding driver. -fn probe (dev: &Device <device::Core >, id_info: &Self ::IdInfo ) ->impl PinInit <Self ,Error >; +fn probe (dev: &Device <device::Core >, id_info: &Self ::IdInfo ) + ->impl PinInit <Self ::Data ,Error >;/// Auxiliary driver unbind. ///
@@ -219,8 +223,8 @@ pub trait Driver {/// `&Device<Core>` or `&Device<Bound>` reference. For instance, drivers may try to perform I/O /// operations to gracefully tear down the device. /// -/// Otherwise, release operations for driver resources should be performed in `Self::drop`. -fn unbind (dev: &Device <device::Core >, this:Pin <&Self >) { +/// Otherwise, release operations for driver resources should be performed in `Drop`. +fn unbind (dev: &Device <device::Core >, this:Pin <&Self ::Data >) {let _ = (dev, this); } }
rust/kernel/cpufreq.rs +1 -0
@@ -888,6 +888,7 @@ fn register_em(_policy: &mut Policy) {/// /// impl platform::Driver for SampleDriver { /// type IdInfo = (); +/// type Data = Self; /// const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None; /// /// fn probe(
rust/kernel/driver.rs +14 -10
@@ -13,10 +13,13 @@//! The main driver interface is defined by a bus specific driver trait. For instance: //! //! ```ignore -//! pub trait Driver: Send { +//! pub trait Driver { //! /// The type holding information about each device ID supported by the driver. //! type IdInfo: 'static; //! +//! /// The type of the driver's bus device private data. +//! type Data: Send; +//! //! /// The table of OF device ids supported by the driver. //! const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None; //!
@@ -24,10 +27,11 @@//! const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = None; //! //! /// Driver probe. -//! fn probe(dev: &Device<device::Core>, id_info: &Self::IdInfo) -> impl PinInit<Self, Error>; +//! fn probe(dev: &Device<device::Core>, id_info: &Self::IdInfo) +//! -> impl PinInit<Self::Data, Error>; //! //! /// Driver unbind (optional). -//! fn unbind(dev: &Device<device::Core>, this: Pin<&Self>) { +//! fn unbind(dev: &Device<device::Core>, this: Pin<&Self::Data>) { //! let _ = (dev, this); //! } //! }
@@ -42,9 +46,9 @@ )]#" )] //! -//! The `probe()` callback should return a `impl PinInit<Self, Error>`, i.e. the driver's private -//! data. The bus abstraction should store the pointer in the corresponding bus device. The generic -//! [`Device`] infrastructure provides common helpers for this purpose on its +//! The `probe()` callback should return a `impl PinInit<Self::Data, Error>`, i.e. the driver's +//! private data. The bus abstraction should store the pointer in the corresponding bus device. The +//! generic [`Device`] infrastructure provides common helpers for this purpose on its //! [`Device<CoreInternal>`] implementation. //! //! All driver callbacks should provide a reference to the driver's private data. Once the driver
@@ -118,8 +122,8 @@ pub unsafe trait DriverLayout {/// The specific driver type embedding a `struct device_driver`. type DriverType :Default ; -/// The type of the driver's device private data. -type DriverData ; +/// The type of the driver's bus device private data. +type DriverData <'bound>;/// Byte offset of the embedded `struct device_driver` within `DriverType`. ///
@@ -193,8 +197,8 @@ extern "C" fn post_unbind_callback(dev: *mut bindings::device) {// driver's device private data. // // SAFETY: By the safety requirements of the `Driver` trait, `T::DriverData` is the -// driver's device private data type. -drop (unsafe { dev.drvdata_obtain ::<T ::DriverData >() }); +// driver's bus device private data type. +drop (unsafe { dev.drvdata_obtain ::<T ::DriverData <'_>>() }); }/// Attach generic `struct device_driver` callbacks.
rust/kernel/i2c.rs +18 -14
@@ -93,12 +93,12 @@ macro_rules! i2c_device_table {// SAFETY: // - `bindings::i2c_driver` is a C type declared as `repr(C)`. -// - `T` is the type of the driver's device private data. +// - `T::Data` is the type of the driver's device private data. // - `struct i2c_driver` embeds a `struct device_driver`. // - `DEVICE_DRIVER_OFFSET` is the correct byte offset to the embedded `struct device_driver`. unsafe impl <T :Driver > driver::DriverLayout for Adapter <T > {type DriverType = bindings::i2c_driver ; -type DriverData =T ; +type DriverData <'bound> =T ::Data ;const DEVICE_DRIVER_OFFSET :usize = core::mem::offset_of!(Self ::DriverType , driver); }
@@ -176,8 +176,8 @@ extern "C" fn remove_callback(idev: *mut bindings::i2c_client) {// SAFETY: `remove_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `I2cClient::set_drvdata()` has been called -// and stored a `Pin<KBox<T>>`. -let data =unsafe { idev.as_ref ().drvdata_borrow ::<T >() }; +// and stored a `Pin<KBox<T::Data>>`. +let data =unsafe { idev.as_ref ().drvdata_borrow ::<T ::Data >() };T ::unbind (idev, data); }
@@ -188,8 +188,8 @@ extern "C" fn shutdown_callback(idev: *mut bindings::i2c_client) {// SAFETY: `shutdown_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called -// and stored a `Pin<KBox<T>>`. -let data =unsafe { idev.as_ref ().drvdata_borrow ::<T >() }; +// and stored a `Pin<KBox<T::Data>>`. +let data =unsafe { idev.as_ref ().drvdata_borrow ::<T ::Data >() };T ::shutdown (idev, data); }
@@ -294,6 +294,7 @@ macro_rules! module_i2c_driver {/// /// impl i2c::Driver for MyDriver { /// type IdInfo = (); +/// type Data = Self; /// const I2C_ID_TABLE: Option<i2c::IdTable<Self::IdInfo>> = Some(&I2C_TABLE); /// const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE); /// const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE);
@@ -301,15 +302,15 @@ macro_rules! module_i2c_driver {/// fn probe( /// _idev: &i2c::I2cClient<Core>, /// _id_info: Option<&Self::IdInfo>, -/// ) -> impl PinInit<Self, Error> { +/// ) -> impl PinInit<Self::Data, Error> { /// Err(ENODEV) /// } /// -/// fn shutdown(_idev: &i2c::I2cClient<Core>, this: Pin<&Self>) { +/// fn shutdown(_idev: &i2c::I2cClient<Core>, this: Pin<&Self::Data>) { /// } /// } ///``` -pub trait Driver :Send { +pub trait Driver {/// The type holding information about each device id supported by the driver. // TODO: Use `associated_type_defaults` once stabilized: //
@@ -318,6 +319,9 @@ pub trait Driver: Send {// ``` type IdInfo : 'static; +/// The type of the driver's bus device private data. +type Data :Send ; +/// The table of device ids supported by the driver. const I2C_ID_TABLE :Option <IdTable <Self ::IdInfo >> =None ;
@@ -334,7 +338,7 @@ pub trait Driver: Send {fn probe ( dev: &I2cClient <device::Core >, id_info:Option <&Self ::IdInfo >, - ) ->impl PinInit <Self ,Error >; + ) ->impl PinInit <Self ::Data ,Error >;/// I2C driver shutdown. ///
@@ -346,8 +350,8 @@ fn probe(/// /// This callback is distinct from final resource cleanup, as the driver instance remains valid /// after it returns. Any deallocation or teardown of driver-owned resources should instead be -/// handled in `Self::drop`. -fn shutdown (dev: &I2cClient <device::Core >, this:Pin <&Self >) { +/// handled in `Drop`. +fn shutdown (dev: &I2cClient <device::Core >, this:Pin <&Self ::Data >) {let _ = (dev, this); }
@@ -360,8 +364,8 @@ fn shutdown(dev: &I2cClient<device::Core>, this: Pin<&Self>) {/// `&Device<Core>` or `&Device<Bound>` reference. For instance, drivers may try to perform I/O /// operations to gracefully tear down the device. /// -/// Otherwise, release operations for driver resources should be performed in `Self::drop`. -fn unbind (dev: &I2cClient <device::Core >, this:Pin <&Self >) { +/// Otherwise, release operations for driver resources should be performed in `Drop`. +fn unbind (dev: &I2cClient <device::Core >, this:Pin <&Self ::Data >) {let _ = (dev, this); } }
rust/kernel/io/mem.rs +2 -0
@@ -62,6 +62,7 @@ pub(crate) unsafe fn new(device: &'a Device<Bound>, resource: &'a Resource) -> S/// /// impl platform::Driver for SampleDriver { /// # type IdInfo = (); +/// # type Data = Self; /// /// fn probe( /// pdev: &platform::Device<Core>,
@@ -126,6 +127,7 @@ pub fn iomap_exclusive_sized<const SIZE: usize>(/// /// impl platform::Driver for SampleDriver { /// # type IdInfo = (); +/// # type Data = Self; /// /// fn probe( /// pdev: &platform::Device<Core>,
rust/kernel/pci.rs +13 -8
@@ -59,12 +59,12 @@// SAFETY: // - `bindings::pci_driver` is a C type declared as `repr(C)`. -// - `T` is the type of the driver's device private data. +// - `T::Data` is the type of the driver's device private data. // - `struct pci_driver` embeds a `struct device_driver`. // - `DEVICE_DRIVER_OFFSET` is the correct byte offset to the embedded `struct device_driver`. unsafe impl <T :Driver > driver::DriverLayout for Adapter <T > {type DriverType = bindings::pci_driver ; -type DriverData =T ; +type DriverData <'bound> =T ::Data ;const DEVICE_DRIVER_OFFSET :usize = core::mem::offset_of!(Self ::DriverType , driver); }
@@ -129,8 +129,8 @@ extern "C" fn remove_callback(pdev: *mut bindings::pci_dev) {// SAFETY: `remove_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called -// and stored a `Pin<KBox<T>>`. -let data =unsafe { pdev.as_ref ().drvdata_borrow ::<T >() }; +// and stored a `Pin<KBox<T::Data>>`. +let data =unsafe { pdev.as_ref ().drvdata_borrow ::<T ::Data >() };T ::unbind (pdev, data); }
@@ -279,6 +279,7 @@ macro_rules! pci_device_table {/// /// impl pci::Driver for MyDriver { /// type IdInfo = (); +/// type Data = Self; /// const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE; /// /// fn probe(
@@ -291,7 +292,7 @@ macro_rules! pci_device_table {///``` /// Drivers must implement this trait in order to get a PCI driver registered. Please refer to the /// `Adapter` documentation for an example. -pub trait Driver :Send { +pub trait Driver {/// The type holding information about each device id supported by the driver. // TODO: Use `associated_type_defaults` once stabilized: //
@@ -300,6 +301,9 @@ pub trait Driver: Send {// ``` type IdInfo : 'static; +/// The type of the driver's bus device private data. +type Data :Send ; +/// The table of device ids supported by the driver. const ID_TABLE :IdTable <Self ::IdInfo >;
@@ -307,7 +311,8 @@ pub trait Driver: Send {/// /// Called when a new pci device is added or discovered. Implementers should /// attempt to initialize the device here. -fn probe (dev: &Device <device::Core >, id_info: &Self ::IdInfo ) ->impl PinInit <Self ,Error >; +fn probe (dev: &Device <device::Core >, id_info: &Self ::IdInfo ) + ->impl PinInit <Self ::Data ,Error >;/// PCI driver unbind. ///
@@ -318,8 +323,8 @@ pub trait Driver: Send {/// `&Device<Core>` or `&Device<Bound>` reference. For instance, drivers may try to perform I/O /// operations to gracefully tear down the device. /// -/// Otherwise, release operations for driver resources should be performed in `Self::drop`. -fn unbind (dev: &Device <device::Core >, this:Pin <&Self >) { +/// Otherwise, release operations for driver resources should be performed in `Drop`. +fn unbind (dev: &Device <device::Core >, this:Pin <&Self ::Data >) {let _ = (dev, this); } }
rust/kernel/platform.rs +12 -8
@@ -45,12 +45,12 @@// SAFETY: // - `bindings::platform_driver` is a C type declared as `repr(C)`. -// - `T` is the type of the driver's device private data. +// - `T::Data` is the type of the driver's device private data. // - `struct platform_driver` embeds a `struct device_driver`. // - `DEVICE_DRIVER_OFFSET` is the correct byte offset to the embedded `struct device_driver`. unsafe impl <T :Driver > driver::DriverLayout for Adapter <T > {type DriverType = bindings::platform_driver ; -type DriverData =T ; +type DriverData <'bound> =T ::Data ;const DEVICE_DRIVER_OFFSET :usize = core::mem::offset_of!(Self ::DriverType , driver); }
@@ -117,8 +117,8 @@ extern "C" fn remove_callback(pdev: *mut bindings::platform_device) {// SAFETY: `remove_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called -// and stored a `Pin<KBox<T>>`. -let data =unsafe { pdev.as_ref ().drvdata_borrow ::<T >() }; +// and stored a `Pin<KBox<T::Data>>`. +let data =unsafe { pdev.as_ref ().drvdata_borrow ::<T ::Data >() };T ::unbind (pdev, data); }
@@ -192,6 +192,7 @@ macro_rules! module_platform_driver {/// /// impl platform::Driver for MyDriver { /// type IdInfo = (); +/// type Data = Self; /// const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE); /// const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE); ///
@@ -203,7 +204,7 @@ macro_rules! module_platform_driver {/// } /// } ///``` -pub trait Driver :Send { +pub trait Driver {/// The type holding driver private data about each device id supported by the driver. // TODO: Use associated_type_defaults once stabilized: //
@@ -212,6 +213,9 @@ pub trait Driver: Send {// ``` type IdInfo : 'static; +/// The type of the driver's bus device private data. +type Data :Send ; +/// The table of OF device ids supported by the driver. const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =None ;
@@ -225,7 +229,7 @@ pub trait Driver: Send {fn probe ( dev: &Device <device::Core >, id_info:Option <&Self ::IdInfo >, - ) ->impl PinInit <Self ,Error >; + ) ->impl PinInit <Self ::Data ,Error >;/// Platform driver unbind. ///
@@ -236,8 +240,8 @@ fn probe(/// `&Device<Core>` or `&Device<Bound>` reference. For instance, drivers may try to perform I/O /// operations to gracefully tear down the device. /// -/// Otherwise, release operations for driver resources should be performed in `Self::drop`. -fn unbind (dev: &Device <device::Core >, this:Pin <&Self >) { +/// Otherwise, release operations for driver resources should be performed in `Drop`. +fn unbind (dev: &Device <device::Core >, this:Pin <&Self ::Data >) {let _ = (dev, this); } }
rust/kernel/usb.rs +12 -8
@@ -36,12 +36,12 @@// SAFETY: // - `bindings::usb_driver` is a C type declared as `repr(C)`. -// - `T` is the type of the driver's device private data. +// - `T::Data` is the type of the driver's device private data. // - `struct usb_driver` embeds a `struct device_driver`. // - `DEVICE_DRIVER_OFFSET` is the correct byte offset to the embedded `struct device_driver`. unsafe impl <T :Driver > driver::DriverLayout for Adapter <T > {type DriverType = bindings::usb_driver ; -type DriverData =T ; +type DriverData <'bound> =T ::Data ;const DEVICE_DRIVER_OFFSET :usize = core::mem::offset_of!(Self ::DriverType , driver); }
@@ -109,8 +109,8 @@ extern "C" fn disconnect_callback(intf: *mut bindings::usb_interface) {// SAFETY: `disconnect_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called -// and stored a `Pin<KBox<T>>`. -let data =unsafe { dev.drvdata_borrow ::<T >() }; +// and stored a `Pin<KBox<T::Data>>`. +let data =unsafe { dev.drvdata_borrow ::<T ::Data >() };T ::disconnect (intf, data); }
@@ -287,23 +287,27 @@ macro_rules! usb_device_table {/// /// impl usb::Driver for MyDriver { /// type IdInfo = (); +/// type Data = Self; /// const ID_TABLE: usb::IdTable<Self::IdInfo> = &USB_TABLE; /// /// fn probe( /// _interface: &usb::Interface<Core>, /// _id: &usb::DeviceId, /// _info: &Self::IdInfo, -/// ) -> impl PinInit<Self, Error> { +/// ) -> impl PinInit<Self::Data, Error> { /// Err(ENODEV) /// } /// -/// fn disconnect(_interface: &usb::Interface<Core>, _data: Pin<&Self>) {} +/// fn disconnect(_interface: &usb::Interface<Core>, _data: Pin<&Self::Data>) {} /// } ///``` pub trait Driver {/// The type holding information about each one of the device ids supported by the driver. type IdInfo : 'static; +/// The type of the driver's bus device private data. +type Data :Send ; +/// The table of device ids supported by the driver. const ID_TABLE :IdTable <Self ::IdInfo >;
@@ -315,12 +319,12 @@ fn probe( interface: &Interface<device::Core>, id: &DeviceId, id_info: &Self::IdInfo, - ) -> impl PinInit<Self, Error>; + ) -> impl PinInit<Self::Data, Error>;/// USB driver disconnect. /// /// Called when the USB interface is about to be unbound from this driver. -fn disconnect (interface: &Interface <device::Core >, data:Pin <&Self >); +fn disconnect (interface: &Interface <device::Core >, data:Pin <&Self ::Data >); }/// A USB interface.
samples/rust/rust_debugfs.rs +1 -0
@@ -117,6 +117,7 @@ fn from_str(s: &str) -> Result<Self> {impl platform::Driver for RustDebugFs {type IdInfo = (); +type Data =Self ;const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =None ;const ACPI_ID_TABLE :Option <acpi::IdTable <Self ::IdInfo >> =Some (&ACPI_TABLE );
samples/rust/rust_dma.rs +1 -0
@@ -58,6 +58,7 @@ unsafe impl kernel::transmute::FromBytes for MyStruct {}impl pci::Driver for DmaSampleDriver {type IdInfo = (); +type Data =Self ;const ID_TABLE : pci::IdTable <Self ::IdInfo > = &PCI_TABLE ;fn probe (pdev: &pci::Device <Core >, _info: &Self ::IdInfo ) ->impl PinInit <Self ,Error > {
samples/rust/rust_driver_auxiliary.rs +2 -0
@@ -31,6 +31,7 @@impl auxiliary::Driver for AuxiliaryDriver {type IdInfo = (); +type Data =Self ;const ID_TABLE : auxiliary::IdTable <Self ::IdInfo > = &AUX_TABLE ;
@@ -65,6 +66,7 @@ struct ParentDriver {impl pci::Driver for ParentDriver {type IdInfo = (); +type Data =Self ;const ID_TABLE : pci::IdTable <Self ::IdInfo > = &PCI_TABLE ;
samples/rust/rust_driver_i2c.rs +1 -0
@@ -35,6 +35,7 @@impl i2c::Driver for SampleDriver {type IdInfo =u32 ; +type Data =Self ;const ACPI_ID_TABLE :Option <acpi::IdTable <Self ::IdInfo >> =Some (&ACPI_TABLE );const I2C_ID_TABLE :Option <i2c::IdTable <Self ::IdInfo >> =Some (&I2C_TABLE );
samples/rust/rust_driver_pci.rs +1 -0
@@ -140,6 +140,7 @@ fn config_space(pdev: &pci::Device<Bound>) {impl pci::Driver for SampleDriver {type IdInfo =TestIndex ; +type Data =Self ;const ID_TABLE : pci::IdTable <Self ::IdInfo > = &PCI_TABLE ;
samples/rust/rust_driver_platform.rs +1 -0
@@ -101,6 +101,7 @@ struct SampleDriver {impl platform::Driver for SampleDriver {type IdInfo =Info ; +type Data =Self ;const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =Some (&OF_TABLE );const ACPI_ID_TABLE :Option <acpi::IdTable <Self ::IdInfo >> =Some (&ACPI_TABLE );
samples/rust/rust_driver_usb.rs +1 -0
@@ -26,6 +26,7 @@ struct SampleDriver {impl usb::Driver for SampleDriver {type IdInfo = (); +type Data =Self ;const ID_TABLE : usb::IdTable <Self ::IdInfo > = &USB_TABLE ; fn probe(
samples/rust/rust_i2c_client.rs +1 -0
@@ -106,6 +106,7 @@ struct SampleDriver {impl platform::Driver for SampleDriver {type IdInfo = (); +type Data =Self ;const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =Some (&OF_TABLE );const ACPI_ID_TABLE :Option <acpi::IdTable <Self ::IdInfo >> =Some (&ACPI_TABLE );
samples/rust/rust_soc.rs +1 -0
@@ -37,6 +37,7 @@ struct SampleSocDriver {impl platform::Driver for SampleSocDriver {type IdInfo = (); +type Data =Self ;const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =Some (&OF_TABLE );const ACPI_ID_TABLE :Option <acpi::IdTable <Self ::IdInfo >> =Some (&ACPI_TABLE );
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 04/24] rust: driver: decouple driver private data from driver type
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] Mismatch between commit message and implementation regarding Data<'bound> associated type…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Mismatch between commit message and implementation regarding Data<'bound> associated type
- [Low] Incomplete updates to sample drivers and doc comments to reflect the new probe return type --
commit 0e6d93fa6eec846c359654d90bf3423215c84f3d
Author: Danilo Krummrich <[email protected]>
rust: driver: decouple driver private data from driver type
This commit adds a Data associated type to all bus driver traits, decoupling the driver's bus device private data type from the driver struct itself. It also updates existing drivers and documentation examples to set Data = Self to preserve existing behavior.
Quoted 8 lines
Add a type Data<'bound> associated type to all bus driver traits, decoupling the driver's bus device private data type from the driver struct itself.
In the context of adding a 'bound lifetime, making this an associated type has the advantage that it allows us to avoid a driver trait global lifetime and it avoids the need for ForLt for bus device private data; both of which make the subsequent implementation by buses much simpler.
[Severity: Medium]
Is this commit message accurate for the current implementation?
The commit message explicitly claims to add a Data<'bound> associated type to all bus driver traits to avoid the need for ForLt. However, looking at the code changes in this patch, the user-facing bus traits only add "type Data: Send;" without any lifetime parameter.
Could this be an outdated description from a previous iteration of the design?
samples/rust/rust_dma.rs unsafe impl kernel::transmute::FromBytes for MyStruct {} · line 64
Quoted 12 lines
samples/rust/rust_dma.rs +1
-0@@ -58,6 +58,7 @@ unsafe impl kernel::transmute::FromBytes for MyStruct {}impl pci::Driver for DmaSampleDriver {type IdInfo = (); +type Data =Self ;const ID_TABLE : pci::IdTable <Self ::IdInfo > = &PCI_TABLE ;fn probe (pdev: &pci::Device <Core >, _info: &Self ::IdInfo ) ->impl PinInit <Self ,Error > {
[Severity: Low]
Should the return type in these sample drivers and documentation examples be updated to use impl PinInit<Self::Data, Error> instead of Self?
While this compiles correctly because the examples define type Data = Self, it doesn't fully demonstrate the newly decoupled API. If someone uses this code as a template and changes the Data type to a distinct structure, they will likely encounter a confusing type mismatch error. There are several other examples in this patch (like rust_driver_platform.rs and io/mem.rs) that also still return impl PinInit<Self, Error>.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4
Danilo Krummrich
maintainerPATCH v5 5/24 rust: driver core: drop drvdata before devres release
In reply to Danilo KrummrichMove the post_unbind_rust callback before devres_release_all() in device_unbind_cleanup().
With drvdata() removed, the driver's bus device private data is only accessible by the owning driver itself. It is hence safe to drop the driver's bus device private data before devres actions are released.
This reordering is the key enabler for Higher-Ranked Lifetime Types (HRT) in Rust device drivers -- it allows driver structs to hold direct references to devres-managed resources, because the bus device private data (and with it all such references) is guaranteed to be dropped while the underlying devres resources are still alive.
Without this change, devres resources would be freed first, leaving the driver's bus device private data with dangling references during its destructor.
--- drivers/base/dd.c | 2 +- include/linux/device/driver.h | 4 ++-- rust/kernel/driver.rs | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-)
drivers/base/dd.c +1 -1
@@ -595,9 +595,9 @@ static DEVICE_ATTR_RW(state_synced);static void device_unbind_cleanup (struct device *dev) { -devres_release_all (dev);if (dev->driver->p_cb.post_unbind_rust) dev->driver->p_cb.post_unbind_rust (dev); +devres_release_all (dev);arch_teardown_dma_ops (dev);kfree (dev->dma_range_map); dev->dma_range_map =NULL ;
include/linux/device/driver.h +2 -2
@@ -123,8 +123,8 @@ struct device_driver {struct driver_private *p;struct {/* -* Called after remove() and after all devres entries have been -* processed. This is a Rust only callback. +* Called after remove() but before devres entries are released. +* This is a Rust only callback. */ void (*post_unbind_rust)(struct device *dev); } p_cb;
rust/kernel/driver.rs +2 -2
@@ -193,8 +193,8 @@ extern "C" fn post_unbind_callback(dev: *mut bindings::device) {// INVARIANT: `dev` is valid for the duration of the `post_unbind_callback()`. let dev =unsafe { &*dev.cast ::<device::Device <device::CoreInternal >>() }; -// `remove()` and all devres callbacks have been completed at this point, hence drop the -// driver's device private data. +// `remove()` has been completed at this point; devres resources are still valid and will +// be released after the driver's bus device private data is dropped. // // SAFETY: By the safety requirements of the `Driver` trait, `T::DriverData` is the // driver's bus device private data type.
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 05/24] rust: driver core: drop drvdata before devres release
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Critical] Use-After-Free vulnerability in devres cleanup sequence due to premature Rust driver data…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Use-After-Free vulnerability in devres cleanup sequence due to premature Rust driver data destruction. --
commit 7750f9ee9a8388dc1f41404481618c4bf5ae2148
Author: Danilo Krummrich <[email protected]>
rust: driver core: drop drvdata before devres release
This commit moves the post_unbind_rust callback before devres_release_all() in device_unbind_cleanup(). This allows Rust driver structs to hold direct references to devres-managed resources without dangling references.
drivers/base/dd.c static DEVICE_ATTR_RW(state_synced); · line 600
Quoted 11 lines
drivers/base/dd.c +1
-1@@ -595,9 +595,9 @@ static DEVICE_ATTR_RW(state_synced);static void device_unbind_cleanup (struct device *dev) { -devres_release_all (dev);if (dev->driver->p_cb.post_unbind_rust) dev->driver->p_cb.post_unbind_rust (dev); +devres_release_all (dev);
[Severity: Critical]
Could moving the Rust post_unbind callback before devres_release_all() trigger a use-after-free vulnerability?
If the driver data is dropped during post_unbind_rust(), but devres-managed actions (such as shared interrupts or timer expirations) are still active, can asynchronous events dereference the freed driver memory before unregistration completes in devres_release_all()?
Quoted 2 lines
drivers/base/dd.c +0
-0arch_teardown_dma_ops (dev);kfree (dev->dma_range_map);
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=5
Danilo Krummrich
maintainerPATCH v5 6/24 rust: pci: implement Sync for Device<Bound>
In reply to Danilo KrummrichImplement Sync for Device<Bound> in addition to Device<Normal>.
Device<Bound> uses the same underlying struct pci_dev as Device<Normal>; Bound is a zero-sized type-state marker that does not affect thread safety.
This is needed for drivers to store &'bound pci::Device<Bound> in their private data while remaining Send.
--- rust/kernel/pci.rs | 4 ++++ 1 file changed, 4 insertions(+)
rust/kernel/pci.rs +4 -0
@@ -528,3 +528,7 @@ unsafe impl Send for Device {}// SAFETY: `Device` can be shared among threads because all methods of `Device` // (i.e. `Device<Normal>) are thread safe. unsafe impl Sync for Device {} + +// SAFETY: Same as `Device<Normal>` -- the underlying `struct pci_dev` is the same; +// `Bound` is a zero-sized type-state marker that does not affect thread safety. +unsafe impl Sync for Device <device::Bound > {}
Signature · 1 line
2.54.0
Danilo Krummrich
maintainerPATCH v5 7/24 rust: platform: implement Sync for Device<Bound>
In reply to Danilo KrummrichImplement Sync for Device<Bound> in addition to Device<Normal>.
Device<Bound> uses the same underlying struct platform_device as Device<Normal>; Bound is a zero-sized type-state marker that does not affect thread safety.
This is needed for drivers to store &'bound platform::Device<Bound> in their private data while remaining Send.
--- rust/kernel/platform.rs | 4 ++++ 1 file changed, 4 insertions(+)
rust/kernel/platform.rs +4 -0
@@ -565,3 +565,7 @@ unsafe impl Send for Device {}// SAFETY: `Device` can be shared among threads because all methods of `Device` // (i.e. `Device<Normal>) are thread safe. unsafe impl Sync for Device {} + +// SAFETY: Same as `Device<Normal>` -- the underlying `struct platform_device` is the same; +// `Bound` is a zero-sized type-state marker that does not affect thread safety. +unsafe impl Sync for Device <device::Bound > {}
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 07/24] rust: platform: implement Sync for Device<Bound>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] The patch manually implements `Sync` specifically for `platform::Device<device::Bound>` instead…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch manually implements Sync specifically for platform::Device<device::Bound> instead of generically implementing Send and Sync for all device contexts, leading to multiple gaps in thread-safety trait coverage. --
commit 1e7bb03e034e1d473f033686b8475722ba9f181f
Author: Danilo Krummrich <[email protected]>
rust: platform: implement Sync for Device<Bound>
This commit implements the Sync trait for Device<Bound> by relying on the fact that Bound is a zero-sized type-state marker. This allows drivers to safely store &'bound platform::Device<Bound> inside their private data while remaining Send.
Quoted 4 lines
rust/kernel/platform.rs +0
-0@@ -565,3 +565,7 @@ unsafe impl Send for Device {}
[ ... ]
rust/kernel/platform.rs unsafe impl Send for Device {} · line 571
Quoted 5 lines
unsafe impl Sync for Device {}
rust/kernel/platform.rs +4
-0+ +// SAFETY: Same as `Device<Normal>` -- the underlying `struct platform_device` is the same; +// `Bound` is a zero-sized type-state marker that does not affect thread safety. +unsafe impl Sync for Device <device::Bound > {}
[Severity: Medium]
Could this be implemented generically for all device contexts, such as unsafe impl<Ctx: device::DeviceContext> Sync for Device<Ctx> {}?
By adding traits piecemeal, this creates a situation where Device<Bound> is Sync but not Send. This prevents valid use cases like moving a &mut Device<Bound> across threads.
Additionally, Device<Core> remains without Send or Sync implementations. This restricts sharing &Device<Core> with scoped threads inside probe(), which could otherwise be useful for concurrent initialization patterns.
Since the underlying C struct is identical and Ctx is a zero-sized marker, a generic implementation for Send and Sync would cover all current and future contexts while removing the need for duplicate unsafe blocks across different bus modules.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7
Danilo Krummrich
maintainerPATCH v5 8/24 rust: auxiliary: implement Sync for Device<Bound>
In reply to Danilo KrummrichImplement Sync for Device<Bound> in addition to Device<Normal>.
Device<Bound> uses the same underlying struct auxiliary_device as Device<Normal>; Bound is a zero-sized type-state marker that does not affect thread safety.
This is needed for drivers to store &'bound auxiliary::Device<Bound> in their private data while remaining Send.
--- rust/kernel/auxiliary.rs | 4 ++++ 1 file changed, 4 insertions(+)
rust/kernel/auxiliary.rs +4 -0
@@ -369,6 +369,10 @@ unsafe impl Send for Device {}// (i.e. `Device<Normal>) are thread safe. unsafe impl Sync for Device {} +// SAFETY: Same as `Device<Normal>` -- the underlying `struct auxiliary_device` is the same; +// `Bound` is a zero-sized type-state marker that does not affect thread safety. +unsafe impl Sync for Device <device::Bound > {} +/// Wrapper that stores a [`TypeId`] alongside the registration data for runtime type checking. #[repr( C )] #[pin_data]
Signature · 1 line
2.54.0
Danilo Krummrich
maintainerPATCH v5 9/24 rust: usb: implement Sync for Device<Bound>
In reply to Danilo KrummrichImplement Sync for Device<Bound> in addition to Device<Normal>.
Device<Bound> uses the same underlying struct usb_device as Device<Normal>; Bound is a zero-sized type-state marker that does not affect thread safety.
This is needed for drivers to store &'bound usb::Device<Bound> in their private data while remaining Send.
--- rust/kernel/usb.rs | 4 ++++ 1 file changed, 4 insertions(+)
rust/kernel/usb.rs +4 -0
@@ -468,6 +468,10 @@ unsafe impl Send for Device {}// allow any mutation through a shared reference. unsafe impl Sync for Device {} +// SAFETY: Same as `Device<Normal>` -- the underlying `struct usb_device` is the same; +// `Bound` is a zero-sized type-state marker that does not affect thread safety. +unsafe impl Sync for Device <device::Bound > {} +/// Declares a kernel module that exposes a single USB driver. /// /// # Examples
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 09/24] rust: usb: implement Sync for Device<Bound>
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] The implementation of `Sync` for `usb::Device<Bound>` is unusable and conceptually incorrect…
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The implementation of Sync for usb::Device<Bound> is unusable and conceptually incorrect because Device is private and cannot be obtained.
- [Medium] The targeted implementation of Sync for Device<Bound> propagates an anti-pattern by failing to implement thread safety traits generically over all device contexts. --
commit 6f765816705f98d508d584483f9224253a79d788
Author: Danilo Krummrich <[email protected]>
rust: usb: implement Sync for Device<Bound>
This commit implements the Sync trait for Device<Bound> to allow drivers to store a bound USB device in their private data.
rust/kernel/usb.rs unsafe impl Send for Device {} · line 473
Quoted 11 lines
rust/kernel/usb.rs +3
-0@@ -468,6 +468,10 @@ unsafe impl Send for Device {}// allow any mutation through a shared reference. unsafe impl Sync for Device {} +// SAFETY: Same as `Device<Normal>` -- the underlying `struct usb_device` is the same; +// `Bound` is a zero-sized type-state marker that does not affect thread safety. +unsafe impl Sync for Device <device::Bound > {}
[Severity: Medium]
The commit message states this is needed for drivers to store &'bound usb::Device<Bound> in their private data. However, since struct Device is defined without the pub visibility modifier in rust/kernel/usb.rs, how can external drivers name this type in their structs?
Additionally, there does not seem to be an API to obtain a Device<Bound>. Calling as_ref() on an Interface explicitly returns &Device (which defaults to &Device<device::Normal>). Since USB drivers bind to struct usb_interface rather than struct usb_device, shouldn't drivers be storing &'bound usb::Interface<Bound> instead?
[Severity: Medium]
Would it be better to implement these traits generically across all contexts, such as using unsafe impl<Ctx> Sync for Device<Ctx>?
Implementing this piecemeal leaves Device<Bound> without a Send implementation, whereas Device<Normal> has one. It also entirely omits both Send and Sync for usb::Interface<Bound>, which is the structure drivers actually bind to.
Since the Ctx type-state marker is a zero-sized PhantomData that does not affect the thread-safety semantics of the underlying C structures, a generic implementation might prevent downstream drivers from breaking when requiring Send or Sync bounds.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=9
Danilo Krummrich
maintainerPATCH v5 10/24 rust: device: implement Sync for Device<Bound>
In reply to Danilo KrummrichImplement Sync for Device<Bound> in addition to Device<Normal>.
Device<Bound> uses the same underlying struct device as Device<Normal>; Bound is a zero-sized type-state marker that does not affect thread safety.
This is needed for types that hold &'bound Device<Bound>, such as io::mem::IoMem, to be Send.
--- rust/kernel/device.rs | 4 ++++ 1 file changed, 4 insertions(+)
rust/kernel/device.rs +4 -0
@@ -467,6 +467,10 @@ unsafe impl Send for Device {}// synchronization in `struct device`. unsafe impl Sync for Device {} +// SAFETY: Same as `Device<Normal>` -- the underlying `struct device` is the same; `Bound` is a +// zero-sized type-state marker that does not affect thread safety. +unsafe impl Sync for Device <Bound > {} +/// Marker trait for the context or scope of a bus specific device. /// /// [`DeviceContext`] is a marker trait for types representing the context of a bus specific
Signature · 1 line
2.54.0
Danilo Krummrich
maintainerPATCH v5 11/24 rust: device: make Core and CoreInternal lifetime-parameterized
In reply to Danilo KrummrichDevice<Core> references in probe callbacks are scoped to the callback, not the full binding duration. Add a lifetime parameter to Core and CoreInternal to accurately represent this in the type system.
--- drivers/cpufreq/rcpufreq_dt.rs | 2 +- drivers/gpu/drm/nova/driver.rs | 5 ++- drivers/gpu/drm/tyr/driver.rs | 2 +- drivers/gpu/nova-core/driver.rs | 4 +-- drivers/gpu/nova-core/gpu.rs | 2 +- drivers/pwm/pwm_th1520.rs | 2 +- rust/kernel/auxiliary.rs | 12 ++++--- rust/kernel/cpufreq.rs | 2 +- rust/kernel/device.rs | 49 +++++++++++++++++++++------ rust/kernel/devres.rs | 2 +- rust/kernel/dma.rs | 2 +- rust/kernel/driver.rs | 6 ++-- rust/kernel/i2c.rs | 16 ++++----- rust/kernel/io/mem.rs | 4 +-- rust/kernel/pci.rs | 20 ++++++----- rust/kernel/pci/id.rs | 2 +- rust/kernel/platform.rs | 12 +++---- rust/kernel/usb.rs | 16 ++++----- samples/rust/rust_debugfs.rs | 4 +-- samples/rust/rust_dma.rs | 2 +- samples/rust/rust_driver_auxiliary.rs | 7 ++-- samples/rust/rust_driver_i2c.rs | 6 ++-- samples/rust/rust_driver_pci.rs | 4 +-- samples/rust/rust_driver_platform.rs | 2 +- samples/rust/rust_driver_usb.rs | 8 ++--- samples/rust/rust_i2c_client.rs | 4 +-- samples/rust/rust_soc.rs | 2 +- 27 files changed, 118 insertions(+), 81 deletions(-)
drivers/cpufreq/rcpufreq_dt.rs +1 -1
@@ -205,7 +205,7 @@ impl platform::Driver for CPUFreqDTDriver {const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =Some (&OF_TABLE );fn probe ( - pdev: &platform::Device <Core >, + pdev: &platform::Device <Core <'_>>, _id_info:Option <&Self ::IdInfo >, ) ->impl PinInit <Self ,Error > { cpufreq::Registration ::<CPUFreqDTDriver >::new_foreign_owned (pdev.as_ref ())?;
drivers/gpu/drm/nova/driver.rs +4 -1
@@ -54,7 +54,10 @@ impl auxiliary::Driver for NovaDriver {type Data =Self ;const ID_TABLE : auxiliary::IdTable <Self ::IdInfo > = &AUX_TABLE ; -fn probe (adev: &auxiliary::Device <Core >, _info: &Self ::IdInfo ) ->impl PinInit <Self ,Error > { +fn probe ( + adev: &auxiliary::Device <Core <'_>>, + _info: &Self ::IdInfo , + ) ->impl PinInit <Self ,Error > {let data =try_pin_init! (NovaData { adev: adev.into () });let drm = drm::Device ::<Self >::new (adev.as_ref (), data)?;
drivers/gpu/drm/tyr/driver.rs +1 -1
@@ -95,7 +95,7 @@ impl platform::Driver for TyrPlatformDriverData {const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =Some (&OF_TABLE );fn probe ( - pdev: &platform::Device <Core >, + pdev: &platform::Device <Core <'_>>, _info:Option <&Self ::IdInfo >, ) ->impl PinInit <Self ,Error > {let core_clk =Clk ::get (pdev.as_ref (),Some (c"core" ))?;
drivers/gpu/nova-core/driver.rs +2 -2
@@ -77,7 +77,7 @@ impl pci::Driver for NovaCore {type Data =Self ;const ID_TABLE : pci::IdTable <Self ::IdInfo > = &PCI_TABLE ; -fn probe (pdev: &pci::Device <Core >, _info: &Self ::IdInfo ) ->impl PinInit <Self ,Error > { +fn probe (pdev: &pci::Device <Core <'_>>, _info: &Self ::IdInfo ) ->impl PinInit <Self ,Error > { pin_init::pin_init_scope (move || {dev_dbg! (pdev,"Probe Nova Core GPU driver.\n" );
@@ -109,7 +109,7 @@ fn probe(pdev: &pci::Device<Core>, _info: &Self::IdInfo) -> impl PinInit<Self, E }) } -fn unbind (pdev: &pci::Device <Core >, this:Pin <&Self >) { +fn unbind (pdev: &pci::Device <Core <'_>>, this:Pin <&Self >) { this.gpu.unbind (pdev.as_ref ()); } }
drivers/gpu/nova-core/gpu.rs +1 -1
@@ -278,7 +278,7 @@ pub(crate) fn new<'a>(/// Called when the corresponding [`Device`](device::Device) is unbound. /// /// Note: This method must only be called from `Driver::unbind`. -pub (crate )fn unbind (&self, dev: &device::Device <device::Core >) { +pub (crate )fn unbind (&self, dev: &device::Device <device::Core <'_>>) { kernel::warn_on!(self .bar .access (dev)
drivers/pwm/pwm_th1520.rs +1 -1
@@ -320,7 +320,7 @@ impl platform::Driver for Th1520PwmPlatformDriver {const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =Some (&OF_TABLE );fn probe ( - pdev: &platform::Device <Core >, + pdev: &platform::Device <Core <'_>>, _id_info:Option <&Self ::IdInfo >, ) ->impl PinInit <Self ,Error > {let dev = pdev.as_ref ();
rust/kernel/auxiliary.rs +7 -5
@@ -87,7 +87,7 @@ extern "C" fn probe_callback(// `struct auxiliary_device`. // // INVARIANT: `adev` is valid for the duration of `probe_callback()`. -let adev =unsafe { &*adev.cast ::<Device <device::CoreInternal >>() }; +let adev =unsafe { &*adev.cast ::<Device <device::CoreInternal <'_>>>() };// SAFETY: `DeviceId` is a `#[repr(transparent)`] wrapper of `struct auxiliary_device_id` // and does not add additional invariants, so it's safe to transmute.
@@ -107,7 +107,7 @@ extern "C" fn remove_callback(adev: *mut bindings::auxiliary_device) {// `struct auxiliary_device`. // // INVARIANT: `adev` is valid for the duration of `remove_callback()`. -let adev =unsafe { &*adev.cast ::<Device <device::CoreInternal >>() }; +let adev =unsafe { &*adev.cast ::<Device <device::CoreInternal <'_>>>() };// SAFETY: `remove_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
@@ -211,8 +211,10 @@ pub trait Driver {/// Auxiliary driver probe. /// /// Called when an auxiliary device is matches a corresponding driver. -fn probe (dev: &Device <device::Core >, id_info: &Self ::IdInfo ) - ->impl PinInit <Self ::Data ,Error >; +fn probe ( + dev: &Device <device::Core <'_>>, + id_info: &Self ::IdInfo , + ) ->impl PinInit <Self ::Data ,Error >;/// Auxiliary driver unbind. ///
@@ -224,7 +226,7 @@ fn probe(dev: &Device<device::Core>, id_info: &Self::IdInfo)/// operations to gracefully tear down the device. /// /// Otherwise, release operations for driver resources should be performed in `Drop`. -fn unbind (dev: &Device <device::Core >, this:Pin <&Self ::Data >) { +fn unbind (dev: &Device <device::Core <'_>>, this:Pin <&Self ::Data >) {let _ = (dev, this); } }
rust/kernel/cpufreq.rs +1 -1
@@ -892,7 +892,7 @@ fn register_em(_policy: &mut Policy) {/// const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None; /// /// fn probe( -/// pdev: &platform::Device<Core>, +/// pdev: &platform::Device<Core<'_>>, /// _id_info: Option<&Self::IdInfo>, /// ) -> impl PinInit<Self, Error> { /// cpufreq::Registration::<SampleDriver>::new_foreign_owned(pdev.as_ref())?;
rust/kernel/device.rs +38 -11
107 lines not on this page · raw message
rust/kernel/devres.rs +1 -1
@@ -304,7 +304,7 @@ pub fn device(&self) -> &Device {/// pci, // /// }; /// -/// fn from_core(dev: &pci::Device<Core>, devres: Devres<pci::Bar<0x4>>) -> Result { +/// fn from_core(dev: &pci::Device<Core<'_>>, devres: Devres<pci::Bar<0x4>>) -> Result { /// let bar = devres.access(dev.as_ref())?; /// /// let _ = bar.read32(0x0);
rust/kernel/dma.rs +1 -1
@@ -47,7 +47,7 @@/// where the underlying bus is DMA capable, such as: #[cfg_attr( CONFIG_PCI , doc = "* [`pci::Device`](kernel::pci::Device)" )] /// * [`platform::Device`](::kernel::platform::Device) -pub trait Device :AsRef <device::Device <Core >> { +pub trait Device <'a>:AsRef <device::Device <Core <'a>>> {/// Set up the device's DMA streaming addressing capabilities. /// /// This method is usually called once from `probe()` as soon as the device capabilities are
rust/kernel/driver.rs +3 -3
@@ -27,11 +27,11 @@//! const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = None; //! //! /// Driver probe. -//! fn probe(dev: &Device<device::Core>, id_info: &Self::IdInfo) +//! fn probe(dev: &Device<device::Core<'_>>, id_info: &Self::IdInfo) //! -> impl PinInit<Self::Data, Error>; //! //! /// Driver unbind (optional). -//! fn unbind(dev: &Device<device::Core>, this: Pin<&Self::Data>) { +//! fn unbind(dev: &Device<device::Core<'_>>, this: Pin<&Self::Data>) { //! let _ = (dev, this); //! } //! }
@@ -191,7 +191,7 @@ extern "C" fn post_unbind_callback(dev: *mut bindings::device) {// a `struct device`. // // INVARIANT: `dev` is valid for the duration of the `post_unbind_callback()`. -let dev =unsafe { &*dev.cast ::<device::Device <device::CoreInternal >>() }; +let dev =unsafe { &*dev.cast ::<device::Device <device::CoreInternal <'_>>>() };// `remove()` has been completed at this point; devres resources are still valid and will // be released after the driver's bus device private data is dropped.
rust/kernel/i2c.rs +8 -8
@@ -157,7 +157,7 @@ extern "C" fn probe_callback(idev: *mut bindings::i2c_client) -> kernel::ffi::c_// `struct i2c_client`. // // INVARIANT: `idev` is valid for the duration of `probe_callback()`. -let idev =unsafe { &*idev.cast ::<I2cClient <device::CoreInternal >>() }; +let idev =unsafe { &*idev.cast ::<I2cClient <device::CoreInternal <'_>>>() };let info =Self ::i2c_id_info (idev).or_else (|| <Self as driver::Adapter >::id_info (idev.as_ref ()));
@@ -172,7 +172,7 @@ extern "C" fn probe_callback(idev: *mut bindings::i2c_client) -> kernel::ffi::c_extern "C" fn remove_callback (idev: *mut bindings::i2c_client ) {// SAFETY: `idev` is a valid pointer to a `struct i2c_client`. -let idev =unsafe { &*idev.cast ::<I2cClient <device::CoreInternal >>() }; +let idev =unsafe { &*idev.cast ::<I2cClient <device::CoreInternal <'_>>>() };// SAFETY: `remove_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `I2cClient::set_drvdata()` has been called
@@ -184,7 +184,7 @@ extern "C" fn remove_callback(idev: *mut bindings::i2c_client) {extern "C" fn shutdown_callback (idev: *mut bindings::i2c_client ) {// SAFETY: `shutdown_callback` is only ever called for a valid `idev` -let idev =unsafe { &*idev.cast ::<I2cClient <device::CoreInternal >>() }; +let idev =unsafe { &*idev.cast ::<I2cClient <device::CoreInternal <'_>>>() };// SAFETY: `shutdown_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
@@ -300,13 +300,13 @@ macro_rules! module_i2c_driver {/// const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE); /// /// fn probe( -/// _idev: &i2c::I2cClient<Core>, +/// _idev: &i2c::I2cClient<Core<'_>>, /// _id_info: Option<&Self::IdInfo>, /// ) -> impl PinInit<Self::Data, Error> { /// Err(ENODEV) /// } /// -/// fn shutdown(_idev: &i2c::I2cClient<Core>, this: Pin<&Self::Data>) { +/// fn shutdown(_idev: &i2c::I2cClient<Core<'_>>, this: Pin<&Self::Data>) { /// } /// } ///```
@@ -336,7 +336,7 @@ pub trait Driver {/// Called when a new i2c client is added or discovered. /// Implementers should attempt to initialize the client here. fn probe ( - dev: &I2cClient <device::Core >, + dev: &I2cClient <device::Core <'_>>, id_info:Option <&Self ::IdInfo >, ) ->impl PinInit <Self ::Data ,Error >;
@@ -351,7 +351,7 @@ fn probe(/// This callback is distinct from final resource cleanup, as the driver instance remains valid /// after it returns. Any deallocation or teardown of driver-owned resources should instead be /// handled in `Drop`. -fn shutdown (dev: &I2cClient <device::Core >, this:Pin <&Self ::Data >) { +fn shutdown (dev: &I2cClient <device::Core <'_>>, this:Pin <&Self ::Data >) {let _ = (dev, this); }
@@ -365,7 +365,7 @@ fn shutdown(dev: &I2cClient<device::Core>, this: Pin<&Self::Data>) {/// operations to gracefully tear down the device. /// /// Otherwise, release operations for driver resources should be performed in `Drop`. -fn unbind (dev: &I2cClient <device::Core >, this:Pin <&Self ::Data >) { +fn unbind (dev: &I2cClient <device::Core <'_>>, this:Pin <&Self ::Data >) {let _ = (dev, this); } }
rust/kernel/io/mem.rs +2 -2
@@ -65,7 +65,7 @@ pub(crate) unsafe fn new(device: &'a Device<Bound>, resource: &'a Resource) -> S/// # type Data = Self; /// /// fn probe( -/// pdev: &platform::Device<Core>, +/// pdev: &platform::Device<Core<'_>>, /// info: Option<&Self::IdInfo>, /// ) -> impl PinInit<Self, Error> { /// let offset = 0; // Some offset.
@@ -130,7 +130,7 @@ pub fn iomap_exclusive_sized<const SIZE: usize>(/// # type Data = Self; /// /// fn probe( -/// pdev: &platform::Device<Core>, +/// pdev: &platform::Device<Core<'_>>, /// info: Option<&Self::IdInfo>, /// ) -> impl PinInit<Self, Error> { /// let offset = 0; // Some offset.
rust/kernel/pci.rs +11 -9
@@ -105,7 +105,7 @@ extern "C" fn probe_callback(// `struct pci_dev`. // // INVARIANT: `pdev` is valid for the duration of `probe_callback()`. -let pdev =unsafe { &*pdev.cast ::<Device <device::CoreInternal >>() }; +let pdev =unsafe { &*pdev.cast ::<Device <device::CoreInternal <'_>>>() };// SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct pci_device_id` and // does not add additional invariants, so it's safe to transmute.
@@ -125,7 +125,7 @@ extern "C" fn remove_callback(pdev: *mut bindings::pci_dev) {// `struct pci_dev`. // // INVARIANT: `pdev` is valid for the duration of `remove_callback()`. -let pdev =unsafe { &*pdev.cast ::<Device <device::CoreInternal >>() }; +let pdev =unsafe { &*pdev.cast ::<Device <device::CoreInternal <'_>>>() };// SAFETY: `remove_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
@@ -283,7 +283,7 @@ macro_rules! pci_device_table {/// const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE; /// /// fn probe( -/// _pdev: &pci::Device<Core>, +/// _pdev: &pci::Device<Core<'_>>, /// _id_info: &Self::IdInfo, /// ) -> impl PinInit<Self, Error> { /// Err(ENODEV)
@@ -311,8 +311,10 @@ pub trait Driver {/// /// Called when a new pci device is added or discovered. Implementers should /// attempt to initialize the device here. -fn probe (dev: &Device <device::Core >, id_info: &Self ::IdInfo ) - ->impl PinInit <Self ::Data ,Error >; +fn probe ( + dev: &Device <device::Core <'_>>, + id_info: &Self ::IdInfo , + ) ->impl PinInit <Self ::Data ,Error >;/// PCI driver unbind. ///
@@ -324,7 +326,7 @@ fn probe(dev: &Device<device::Core>, id_info: &Self::IdInfo)/// operations to gracefully tear down the device. /// /// Otherwise, release operations for driver resources should be performed in `Drop`. -fn unbind (dev: &Device <device::Core >, this:Pin <&Self ::Data >) { +fn unbind (dev: &Device <device::Core <'_>>, this:Pin <&Self ::Data >) {let _ = (dev, this); } }
@@ -359,7 +361,7 @@ impl Device {/// /// ``` /// # use kernel::{device::Core, pci::{self, Vendor}, prelude::*}; -/// fn log_device_info(pdev: &pci::Device<Core>) -> Result { +/// fn log_device_info(pdev: &pci::Device<Core<'_>>) -> Result { /// // Get an instance of `Vendor`. /// let vendor = pdev.vendor_id(); /// dev_info!(
@@ -450,7 +452,7 @@ pub fn pci_class(&self) -> Class { } } -impl Device <device::Core > { +impl <'a>Device <device::Core <'a>> {/// Enable memory resources for this device. pub fn enable_device_mem (&self) ->Result {// SAFETY: `self.as_raw` is guaranteed to be a pointer to a valid `struct pci_dev`.
@@ -476,7 +478,7 @@ unsafe impl<Ctx: device::DeviceContext> device::AsBusDevice<Ctx> for Device<Ctx> kernel::impl_device_context_deref!(unsafe {Device }); kernel::impl_device_context_into_aref!(Device ); -impl crate ::dma::Device for Device <device::Core > {} +impl <'a>crate ::dma::Device <'a>for Device <device::Core <'a>> {}// SAFETY: Instances of `Device` are always reference-counted. unsafe impl crate ::sync::aref::AlwaysRefCounted for Device {
rust/kernel/pci/id.rs +1 -1
@@ -19,7 +19,7 @@/// /// ``` /// # use kernel::{device::Core, pci::{self, Class}, prelude::*}; -/// fn probe_device(pdev: &pci::Device<Core>) -> Result { +/// fn probe_device(pdev: &pci::Device<Core<'_>>) -> Result { /// let pci_class = pdev.pci_class(); /// dev_info!( /// pdev,
rust/kernel/platform.rs +6 -6
@@ -97,7 +97,7 @@ extern "C" fn probe_callback(pdev: *mut bindings::platform_device) -> kernel::ff// `struct platform_device`. // // INVARIANT: `pdev` is valid for the duration of `probe_callback()`. -let pdev =unsafe { &*pdev.cast ::<Device <device::CoreInternal >>() }; +let pdev =unsafe { &*pdev.cast ::<Device <device::CoreInternal <'_>>>() };let info = <Self as driver::Adapter >::id_info (pdev.as_ref ());from_result (|| {
@@ -113,7 +113,7 @@ extern "C" fn remove_callback(pdev: *mut bindings::platform_device) {// `struct platform_device`. // // INVARIANT: `pdev` is valid for the duration of `remove_callback()`. -let pdev =unsafe { &*pdev.cast ::<Device <device::CoreInternal >>() }; +let pdev =unsafe { &*pdev.cast ::<Device <device::CoreInternal <'_>>>() };// SAFETY: `remove_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
@@ -197,7 +197,7 @@ macro_rules! module_platform_driver {/// const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE); /// /// fn probe( -/// _pdev: &platform::Device<Core>, +/// _pdev: &platform::Device<Core<'_>>, /// _id_info: Option<&Self::IdInfo>, /// ) -> impl PinInit<Self, Error> { /// Err(ENODEV)
@@ -227,7 +227,7 @@ pub trait Driver {/// Called when a new platform device is added or discovered. /// Implementers should attempt to initialize the device here. fn probe ( - dev: &Device <device::Core >, + dev: &Device <device::Core <'_>>, id_info:Option <&Self ::IdInfo >, ) ->impl PinInit <Self ::Data ,Error >;
@@ -241,7 +241,7 @@ fn probe(/// operations to gracefully tear down the device. /// /// Otherwise, release operations for driver resources should be performed in `Drop`. -fn unbind (dev: &Device <device::Core >, this:Pin <&Self ::Data >) { +fn unbind (dev: &Device <device::Core <'_>>, this:Pin <&Self ::Data >) {let _ = (dev, this); } }
@@ -513,7 +513,7 @@ pub fn optional_irq_by_name(&self, name: &CStr) -> Result<IrqRequest<'_>> { kernel::impl_device_context_deref!(unsafe {Device }); kernel::impl_device_context_into_aref!(Device ); -impl crate ::dma::Device for Device <device::Core > {} +impl <'a>crate ::dma::Device <'a>for Device <device::Core <'a>> {}// SAFETY: Instances of `Device` are always reference-counted. unsafe impl crate ::sync::aref::AlwaysRefCounted for Device {
rust/kernel/usb.rs +8 -8
@@ -82,7 +82,7 @@ extern "C" fn probe_callback(// `struct usb_interface` and `struct usb_device_id`. // // INVARIANT: `intf` is valid for the duration of `probe_callback()`. -let intf =unsafe { &*intf.cast ::<Interface <device::CoreInternal >>() }; +let intf =unsafe { &*intf.cast ::<Interface <device::CoreInternal <'_>>>() };from_result (|| {// SAFETY: `DeviceId` is a `#[repr(transparent)]` wrapper of `struct usb_device_id` and
@@ -92,7 +92,7 @@ extern "C" fn probe_callback(let info =T ::ID_TABLE .info (id.index ());let data =T ::probe (intf, id, info); -let dev: &device::Device <device::CoreInternal > = intf.as_ref (); +let dev: &device::Device <device::CoreInternal <'_>> = intf.as_ref (); dev.set_drvdata (data)?;Ok (0 ) })
@@ -103,9 +103,9 @@ extern "C" fn disconnect_callback(intf: *mut bindings::usb_interface) {// `struct usb_interface`. // // INVARIANT: `intf` is valid for the duration of `disconnect_callback()`. -let intf =unsafe { &*intf.cast ::<Interface <device::CoreInternal >>() }; +let intf =unsafe { &*intf.cast ::<Interface <device::CoreInternal <'_>>>() }; -let dev: &device::Device <device::CoreInternal > = intf.as_ref (); +let dev: &device::Device <device::CoreInternal <'_>> = intf.as_ref ();// SAFETY: `disconnect_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called
@@ -291,14 +291,14 @@ macro_rules! usb_device_table {/// const ID_TABLE: usb::IdTable<Self::IdInfo> = &USB_TABLE; /// /// fn probe( -/// _interface: &usb::Interface<Core>, +/// _interface: &usb::Interface<Core<'_>>, /// _id: &usb::DeviceId, /// _info: &Self::IdInfo, /// ) -> impl PinInit<Self::Data, Error> { /// Err(ENODEV) /// } /// -/// fn disconnect(_interface: &usb::Interface<Core>, _data: Pin<&Self::Data>) {} +/// fn disconnect(_interface: &usb::Interface<Core<'_>>, _data: Pin<&Self::Data>) {} /// } ///``` pub trait Driver {
@@ -316,7 +316,7 @@ pub trait Driver {/// Called when a new USB interface is bound to this driver. /// Implementers should attempt to initialize the interface here. fn probe ( - interface: &Interface <device::Core >, + interface: &Interface <device::Core <'_>>, id: &DeviceId , id_info: &Self ::IdInfo , ) ->impl PinInit <Self ::Data ,Error >;
@@ -324,7 +324,7 @@ fn probe(/// USB driver disconnect. /// /// Called when the USB interface is about to be unbound from this driver. -fn disconnect (interface: &Interface <device::Core >, data:Pin <&Self ::Data >); +fn disconnect (interface: &Interface <device::Core <'_>>, data:Pin <&Self ::Data >); }/// A USB interface.
samples/rust/rust_debugfs.rs +2 -2
@@ -122,7 +122,7 @@ impl platform::Driver for RustDebugFs {const ACPI_ID_TABLE :Option <acpi::IdTable <Self ::IdInfo >> =Some (&ACPI_TABLE );fn probe ( - pdev: &platform::Device <Core >, + pdev: &platform::Device <Core <'_>>, _info:Option <&Self ::IdInfo >, ) ->impl PinInit <Self ,Error > {RustDebugFs ::new (pdev).pin_chain (|this| {
@@ -147,7 +147,7 @@ fn build_inner(dir: &Dir) -> impl PinInit<File<Mutex<Inner>>> + '_ { dir.read_write_file (c"pair" ,new_mutex! (Inner { x:3 , y:10 })) } -fn new (pdev: &platform::Device <Core >) ->impl PinInit <Self ,Error > + '_ { +fn new <'a>(pdev: &'a platform::Device <Core <'_>>) ->impl PinInit <Self ,Error > + 'a {let debugfs =Dir ::new (c"sample_debugfs" );let dev = pdev.as_ref ();
samples/rust/rust_dma.rs +1 -1
@@ -61,7 +61,7 @@ impl pci::Driver for DmaSampleDriver {type Data =Self ;const ID_TABLE : pci::IdTable <Self ::IdInfo > = &PCI_TABLE ; -fn probe (pdev: &pci::Device <Core >, _info: &Self ::IdInfo ) ->impl PinInit <Self ,Error > { +fn probe (pdev: &pci::Device <Core <'_>>, _info: &Self ::IdInfo ) ->impl PinInit <Self ,Error > { pin_init::pin_init_scope (move || {dev_info! (pdev,"Probe DMA test driver.\n" );
samples/rust/rust_driver_auxiliary.rs +5 -2
@@ -35,7 +35,10 @@ impl auxiliary::Driver for AuxiliaryDriver {const ID_TABLE : auxiliary::IdTable <Self ::IdInfo > = &AUX_TABLE ; -fn probe (adev: &auxiliary::Device <Core >, _info: &Self ::IdInfo ) ->impl PinInit <Self ,Error > { +fn probe ( + adev: &auxiliary::Device <Core <'_>>, + _info: &Self ::IdInfo , + ) ->impl PinInit <Self ,Error > {dev_info! ( adev,"Probing auxiliary driver for auxiliary device with id={}\n" ,
@@ -70,7 +73,7 @@ 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> { + fn probe(pdev: &pci::Device<Core<'_>>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> { Ok(Self { _reg0: auxiliary::Registration::new( pdev.as_ref(),
samples/rust/rust_driver_i2c.rs +3 -3
@@ -42,7 +42,7 @@ impl i2c::Driver for SampleDriver {const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =Some (&OF_TABLE );fn probe ( - idev: &i2c::I2cClient <Core >, + idev: &i2c::I2cClient <Core <'_>>, info:Option <&Self ::IdInfo >, ) ->impl PinInit <Self ,Error > {let dev = idev.as_ref ();
@@ -56,11 +56,11 @@ fn probe(Ok (Self ) } -fn shutdown (idev: &i2c::I2cClient <Core >, _this:Pin <&Self >) { +fn shutdown (idev: &i2c::I2cClient <Core <'_>>, _this:Pin <&Self >) {dev_info! (idev.as_ref (),"Shutdown Rust I2C driver sample.\n" ); } -fn unbind (idev: &i2c::I2cClient <Core >, _this:Pin <&Self >) { +fn unbind (idev: &i2c::I2cClient <Core <'_>>, _this:Pin <&Self >) {dev_info! (idev.as_ref (),"Unbind Rust I2C driver sample.\n" ); } }
samples/rust/rust_driver_pci.rs +2 -2
@@ -144,7 +144,7 @@ impl pci::Driver for SampleDriver { const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE; - fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Error> { + fn probe(pdev: &pci::Device<Core<'_>>, info: &Self::IdInfo) -> impl PinInit<Self, Error> { pin_init::pin_init_scope(move || { let vendor = pdev.vendor_id(); dev_dbg!(
@@ -175,7 +175,7 @@ fn probe(pdev: &pci::Device<Core>, info: &Self::IdInfo) -> impl PinInit<Self, Er }) } -fn unbind (pdev: &pci::Device <Core >, this:Pin <&Self >) { +fn unbind (pdev: &pci::Device <Core <'_>>, this:Pin <&Self >) {if let Ok (bar) = this.bar.access (pdev.as_ref ()) {// Reset pci-testdev by writing a new test index. bar.write_reg (regs::TEST ::zeroed ().with_index (this.index));
samples/rust/rust_driver_platform.rs +1 -1
@@ -106,7 +106,7 @@ impl platform::Driver for SampleDriver {const ACPI_ID_TABLE :Option <acpi::IdTable <Self ::IdInfo >> =Some (&ACPI_TABLE );fn probe ( - pdev: &platform::Device <Core >, + pdev: &platform::Device <Core <'_>>, info:Option <&Self ::IdInfo >, ) ->impl PinInit <Self ,Error > {let dev = pdev.as_ref ();
samples/rust/rust_driver_usb.rs +4 -4
@@ -30,18 +30,18 @@ impl usb::Driver for SampleDriver {const ID_TABLE : usb::IdTable <Self ::IdInfo > = &USB_TABLE ;fn probe ( - intf: &usb::Interface <Core >, + intf: &usb::Interface <Core <'_>>, _id: &usb::DeviceId , _info: &Self ::IdInfo , ) ->impl PinInit <Self ,Error > { -let dev: &device::Device <Core > = intf.as_ref (); +let dev: &device::Device <Core <'_>> = intf.as_ref ();dev_info! (dev,"Rust USB driver sample probed\n" );Ok (Self { _intf: intf.into () }) } -fn disconnect (intf: &usb::Interface <Core >, _data:Pin <&Self >) { -let dev: &device::Device <Core > = intf.as_ref (); +fn disconnect (intf: &usb::Interface <Core <'_>>, _data:Pin <&Self >) { +let dev: &device::Device <Core <'_>> = intf.as_ref ();dev_info! (dev,"Rust USB driver sample disconnected\n" ); } }
samples/rust/rust_i2c_client.rs +2 -2
@@ -111,7 +111,7 @@ impl platform::Driver for SampleDriver {const ACPI_ID_TABLE :Option <acpi::IdTable <Self ::IdInfo >> =Some (&ACPI_TABLE );fn probe ( - pdev: &platform::Device <device::Core >, + pdev: &platform::Device <device::Core <'_>>, _info:Option <&Self ::IdInfo >, ) ->impl PinInit <Self ,Error > {dev_info! (
@@ -130,7 +130,7 @@ fn probe( }) } -fn unbind (pdev: &platform::Device <device::Core >, _this:Pin <&Self >) { +fn unbind (pdev: &platform::Device <device::Core <'_>>, _this:Pin <&Self >) {dev_info! ( pdev.as_ref (),"Unbind Rust I2C Client registration sample.\n"
samples/rust/rust_soc.rs +1 -1
@@ -42,7 +42,7 @@ impl platform::Driver for SampleSocDriver {const ACPI_ID_TABLE :Option <acpi::IdTable <Self ::IdInfo >> =Some (&ACPI_TABLE );fn probe ( - pdev: &platform::Device <Core >, + pdev: &platform::Device <Core <'_>>, _info:Option <&Self ::IdInfo >, ) ->impl PinInit <Self ,Error > {dev_dbg! (pdev,"Probe Rust SoC driver sample.\n" );
Signature · 1 line
2.54.0
Re: [PATCH v5 11/24] rust: device: make Core and CoreInternal lifetime-parameterized
Quoted 9 lines · Danilo Krummrich
Device<Core> references in probe callbacks are scoped to the callback, not the full binding duration. Add a lifetime parameter to Core and CoreInternal to accurately represent this in the type system.
Suggested-by Gary Guo <[email protected]>Reviewed-by Greg Kroah-Hartman <[email protected]>Reviewed-by Alexandre Courbot <[email protected]>Reviewed-by Eliot Courtney <[email protected]>Signed-off-by Danilo Krummrich <[email protected]>
Quoted 29 lines
--- drivers/cpufreq/rcpufreq_dt.rs | 2 +- drivers/gpu/drm/nova/driver.rs | 5 ++- drivers/gpu/drm/tyr/driver.rs | 2 +- drivers/gpu/nova-core/driver.rs | 4 +-- drivers/gpu/nova-core/gpu.rs | 2 +- drivers/pwm/pwm_th1520.rs | 2 +- rust/kernel/auxiliary.rs | 12 ++++--- rust/kernel/cpufreq.rs | 2 +- rust/kernel/device.rs | 49 +++++++++++++++++++++------ rust/kernel/devres.rs | 2 +- rust/kernel/dma.rs | 2 +- rust/kernel/driver.rs | 6 ++-- rust/kernel/i2c.rs | 16 ++++----- rust/kernel/io/mem.rs | 4 +-- rust/kernel/pci.rs | 20 ++++++----- rust/kernel/pci/id.rs | 2 +- rust/kernel/platform.rs | 12 +++---- rust/kernel/usb.rs | 16 ++++----- samples/rust/rust_debugfs.rs | 4 +-- samples/rust/rust_dma.rs | 2 +- samples/rust/rust_driver_auxiliary.rs | 7 ++-- samples/rust/rust_driver_i2c.rs | 6 ++-- samples/rust/rust_driver_pci.rs | 4 +-- samples/rust/rust_driver_platform.rs | 2 +- samples/rust/rust_driver_usb.rs | 8 ++--- samples/rust/rust_i2c_client.rs | 4 +-- samples/rust/rust_soc.rs | 2 +- 27 files changed, 118 insertions(+), 81 deletions(-)
Danilo Krummrich
maintainerPATCH v5 12/24 rust: pci: make Driver trait lifetime-parameterized
In reply to Danilo KrummrichAdd a 'bound lifetime to the associated Data, changing type Data to type Data<'bound>.
This allows the driver's bus device private data to capture the device / driver bound lifetime; device resources can be stored directly by reference rather than requiring Devres.
The probe() and unbind() callbacks thus gain a 'bound lifetime parameter on the methods themselves; avoiding a global lifetime on the trait impl.
Existing drivers set type Data<'bound> = Self, preserving the current behavior.
--- drivers/gpu/nova-core/driver.rs | 9 ++++++--- rust/kernel/pci.rs | 28 +++++++++++++-------------- samples/rust/rust_dma.rs | 7 +++++-- samples/rust/rust_driver_auxiliary.rs | 7 +++++-- samples/rust/rust_driver_pci.rs | 7 +++++-- 5 files changed, 35 insertions(+), 23 deletions(-)
drivers/gpu/nova-core/driver.rs +6 -3
@@ -74,10 +74,13 @@ pub(crate) struct NovaCore {impl pci::Driver for NovaCore {type IdInfo = (); -type Data =Self ; +type Data <'bound> =Self ;const ID_TABLE : pci::IdTable <Self ::IdInfo > = &PCI_TABLE ; -fn probe (pdev: &pci::Device <Core <'_>>, _info: &Self ::IdInfo ) ->impl PinInit <Self ,Error > { +fn probe <'bound>( + pdev: &'bound pci::Device <Core <'_>>, + _info: &'boundSelf ::IdInfo , + ) ->impl PinInit <Self ,Error > + 'bound { pin_init::pin_init_scope (move || {dev_dbg! (pdev,"Probe Nova Core GPU driver.\n" );
@@ -109,7 +112,7 @@ fn probe(pdev: &pci::Device<Core<'_>>, _info: &Self::IdInfo) -> impl PinInit<Sel }) } -fn unbind (pdev: &pci::Device <Core <'_>>, this:Pin <&Self >) { +fn unbind <'bound>(pdev: &'bound pci::Device <Core <'_>>, this:Pin <&Self >) { this.gpu.unbind (pdev.as_ref ()); } }
rust/kernel/pci.rs +14 -14
@@ -64,7 +64,7 @@// - `DEVICE_DRIVER_OFFSET` is the correct byte offset to the embedded `struct device_driver`. unsafe impl <T :Driver > driver::DriverLayout for Adapter <T > {type DriverType = bindings::pci_driver ; -type DriverData <'bound> =T ::Data ; +type DriverData <'bound> =T ::Data <'bound>;const DEVICE_DRIVER_OFFSET :usize = core::mem::offset_of!(Self ::DriverType , driver); }
@@ -129,8 +129,8 @@ extern "C" fn remove_callback(pdev: *mut bindings::pci_dev) {// SAFETY: `remove_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called -// and stored a `Pin<KBox<T::Data>>`. -let data =unsafe { pdev.as_ref ().drvdata_borrow ::<T ::Data >() }; +// and stored a `Pin<KBox<T::Data<'_>>>`. +let data =unsafe { pdev.as_ref ().drvdata_borrow ::<T ::Data <'_>>() };T ::unbind (pdev, data); }
@@ -279,13 +279,13 @@ macro_rules! pci_device_table {/// /// impl pci::Driver for MyDriver { /// type IdInfo = (); -/// type Data = Self; +/// type Data<'bound> = Self; /// const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE; /// -/// fn probe( -/// _pdev: &pci::Device<Core<'_>>, -/// _id_info: &Self::IdInfo, -/// ) -> impl PinInit<Self, Error> { +/// fn probe<'bound>( +/// _pdev: &'bound pci::Device<Core<'_>>, +/// _id_info: &'bound Self::IdInfo, +/// ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound { /// Err(ENODEV) /// } /// }
@@ -302,7 +302,7 @@ pub trait Driver {type IdInfo : 'static;/// The type of the driver's bus device private data. -type Data :Send ; +type Data <'bound>:Send + 'bound;/// The table of device ids supported by the driver. const ID_TABLE :IdTable <Self ::IdInfo >;
@@ -311,10 +311,10 @@ pub trait Driver {/// /// Called when a new pci device is added or discovered. Implementers should /// attempt to initialize the device here. -fn probe ( - dev: &Device <device::Core <'_>>, - id_info: &Self ::IdInfo , - ) ->impl PinInit <Self ::Data ,Error >; +fn probe <'bound>( + dev: &'boundDevice <device::Core <'_>>, + id_info: &'boundSelf ::IdInfo , + ) ->impl PinInit <Self ::Data <'bound>,Error > + 'bound;/// PCI driver unbind. ///
@@ -326,7 +326,7 @@ fn probe(/// operations to gracefully tear down the device. /// /// Otherwise, release operations for driver resources should be performed in `Drop`. -fn unbind (dev: &Device <device::Core <'_>>, this:Pin <&Self ::Data >) { +fn unbind <'bound>(dev: &'boundDevice <device::Core <'_>>, this:Pin <&Self ::Data <'bound>>) {let _ = (dev, this); } }
samples/rust/rust_dma.rs +5 -2
@@ -58,10 +58,13 @@ unsafe impl kernel::transmute::FromBytes for MyStruct {}impl pci::Driver for DmaSampleDriver {type IdInfo = (); -type Data =Self ; +type Data <'bound> =Self ;const ID_TABLE : pci::IdTable <Self ::IdInfo > = &PCI_TABLE ; -fn probe (pdev: &pci::Device <Core <'_>>, _info: &Self ::IdInfo ) ->impl PinInit <Self ,Error > { +fn probe <'bound>( + pdev: &'bound pci::Device <Core <'_>>, + _info: &'boundSelf ::IdInfo , + ) ->impl PinInit <Self ,Error > + 'bound { pin_init::pin_init_scope (move || {dev_info! (pdev,"Probe DMA test driver.\n" );
samples/rust/rust_driver_auxiliary.rs +5 -2
@@ -69,11 +69,14 @@ struct ParentDriver { impl pci::Driver for ParentDriver { type IdInfo = (); - type Data = Self; + type Data<'bound> = Self; const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE; - fn probe(pdev: &pci::Device<Core<'_>>, _info: &Self::IdInfo) -> impl PinInit<Self, Error> { + fn probe<'bound>( + pdev: &'bound pci::Device<Core<'_>>, + _info: &'bound Self::IdInfo, + ) -> impl PinInit<Self, Error> + 'bound { Ok(Self { _reg0: auxiliary::Registration::new( pdev.as_ref(),
samples/rust/rust_driver_pci.rs +5 -2
@@ -140,11 +140,14 @@ fn config_space(pdev: &pci::Device<Bound>) { impl pci::Driver for SampleDriver { type IdInfo = TestIndex; - type Data = Self; + type Data<'bound> = Self; const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE; - fn probe(pdev: &pci::Device<Core<'_>>, info: &Self::IdInfo) -> impl PinInit<Self, Error> { + fn probe<'bound>( + pdev: &'bound pci::Device<Core<'_>>, + info: &'bound Self::IdInfo, + ) -> impl PinInit<Self, Error> + 'bound { pin_init::pin_init_scope(move || { let vendor = pdev.vendor_id(); dev_dbg!(
Signature · 1 line
2.54.0
Re: [PATCH v5 12/24] rust: pci: make Driver trait lifetime-parameterized
Quoted 10 lines · Danilo Krummrich
Add a 'bound lifetime to the associated Data, changing type Data to type Data<'bound>.
This allows the driver's bus device private data to capture the device / driver bound lifetime; device resources can be stored directly by reference rather than requiring Devres.
The
probe()andunbind()callbacks thus gain a 'bound lifetime parameter on the methods themselves; avoiding a global lifetime on the trait impl.
Quoted 6 lines
Existing drivers set type Data<'bound> = Self, preserving the current behavior.
Reviewed-by Alexandre Courbot <[email protected]>Reviewed-by Greg Kroah-Hartman <[email protected]>Signed-off-by Danilo Krummrich <[email protected]>
Quoted 7 lines
--- drivers/gpu/nova-core/driver.rs | 9 ++++++--- rust/kernel/pci.rs | 28 +++++++++++++-------------- samples/rust/rust_dma.rs | 7 +++++-- samples/rust/rust_driver_auxiliary.rs | 7 +++++-- samples/rust/rust_driver_pci.rs | 7 +++++-- 5 files changed, 35 insertions(+), 23 deletions(-)
Danilo Krummrich
maintainerPATCH v5 13/24 rust: platform: make Driver trait lifetime-parameterized
In reply to Danilo KrummrichAdd a 'bound lifetime to the associated Data, changing type Data to type Data<'bound>.
This allows the driver's bus device private data to capture the device / driver bound lifetime; device resources can be stored directly by reference rather than requiring Devres.
The probe() and unbind() callbacks thus gain a 'bound lifetime parameter on the methods themselves; avoiding a global lifetime on the trait impl.
Existing drivers set type Data<'bound> = Self, preserving the current behavior.
--- drivers/cpufreq/rcpufreq_dt.rs | 10 +++++----- drivers/gpu/drm/tyr/driver.rs | 10 +++++----- drivers/pwm/pwm_th1520.rs | 10 +++++----- rust/kernel/cpufreq.rs | 10 +++++----- rust/kernel/io/mem.rs | 20 ++++++++++---------- rust/kernel/platform.rs | 28 ++++++++++++++-------------- samples/rust/rust_debugfs.rs | 10 +++++----- samples/rust/rust_driver_platform.rs | 10 +++++----- samples/rust/rust_i2c_client.rs | 15 +++++++++------ samples/rust/rust_soc.rs | 10 +++++----- 10 files changed, 68 insertions(+), 65 deletions(-)
drivers/cpufreq/rcpufreq_dt.rs +5 -5
@@ -201,13 +201,13 @@ fn register_em(policy: &mut cpufreq::Policy) {impl platform::Driver for CPUFreqDTDriver {type IdInfo = (); -type Data =Self ; +type Data <'bound> =Self ;const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =Some (&OF_TABLE ); -fn probe ( - pdev: &platform::Device <Core <'_>>, - _id_info:Option <&Self ::IdInfo >, - ) ->impl PinInit <Self ,Error > { +fn probe <'bound>( + pdev: &'bound platform::Device <Core <'_>>, + _id_info:Option <&'boundSelf ::IdInfo >, + ) ->impl PinInit <Self ,Error > + 'bound { cpufreq::Registration ::<CPUFreqDTDriver >::new_foreign_owned (pdev.as_ref ())?;Ok (Self {}) }
drivers/gpu/drm/tyr/driver.rs +5 -5
@@ -91,13 +91,13 @@ fn issue_soft_reset(dev: &Device<Bound>, iomem: &Devres<IoMem>) -> Result {impl platform::Driver for TyrPlatformDriverData {type IdInfo = (); -type Data =Self ; +type Data <'bound> =Self ;const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =Some (&OF_TABLE ); -fn probe ( - pdev: &platform::Device <Core <'_>>, - _info:Option <&Self ::IdInfo >, - ) ->impl PinInit <Self ,Error > { +fn probe <'bound>( + pdev: &'bound platform::Device <Core <'_>>, + _info:Option <&'boundSelf ::IdInfo >, + ) ->impl PinInit <Self ,Error > + 'bound {let core_clk =Clk ::get (pdev.as_ref (),Some (c"core" ))?;let stacks_clk =OptionalClk ::get (pdev.as_ref (),Some (c"stacks" ))?;let coregroup_clk =OptionalClk ::get (pdev.as_ref (),Some (c"coregroup" ))?;
drivers/pwm/pwm_th1520.rs +5 -5
@@ -316,13 +316,13 @@ fn drop(self: Pin<&mut Self>) {impl platform::Driver for Th1520PwmPlatformDriver {type IdInfo = (); -type Data =Self ; +type Data <'bound> =Self ;const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =Some (&OF_TABLE ); -fn probe ( - pdev: &platform::Device <Core <'_>>, - _id_info:Option <&Self ::IdInfo >, - ) ->impl PinInit <Self ,Error > { +fn probe <'bound>( + pdev: &'bound platform::Device <Core <'_>>, + _id_info:Option <&'boundSelf ::IdInfo >, + ) ->impl PinInit <Self ,Error > + 'bound {let dev = pdev.as_ref ();let request = pdev.io_request_by_index (0 ).ok_or (ENODEV )?;
rust/kernel/cpufreq.rs +5 -5
@@ -888,13 +888,13 @@ fn register_em(_policy: &mut Policy) {/// /// impl platform::Driver for SampleDriver { /// type IdInfo = (); -/// type Data = Self; +/// type Data<'bound> = Self; /// const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None; /// -/// fn probe( -/// pdev: &platform::Device<Core<'_>>, -/// _id_info: Option<&Self::IdInfo>, -/// ) -> impl PinInit<Self, Error> { +/// fn probe<'bound>( +/// pdev: &'bound platform::Device<Core<'_>>, +/// _id_info: Option<&'bound Self::IdInfo>, +/// ) -> impl PinInit<Self, Error> + 'bound { /// cpufreq::Registration::<SampleDriver>::new_foreign_owned(pdev.as_ref())?; /// Ok(Self {}) /// }
rust/kernel/io/mem.rs +10 -10
@@ -62,12 +62,12 @@ pub(crate) unsafe fn new(device: &'a Device<Bound>, resource: &'a Resource) -> S/// /// impl platform::Driver for SampleDriver { /// # type IdInfo = (); -/// # type Data = Self; +/// # type Data<'bound> = Self; /// -/// fn probe( -/// pdev: &platform::Device<Core<'_>>, -/// info: Option<&Self::IdInfo>, -/// ) -> impl PinInit<Self, Error> { +/// fn probe<'bound>( +/// pdev: &'bound platform::Device<Core<'_>>, +/// info: Option<&'bound Self::IdInfo>, +/// ) -> impl PinInit<Self, Error> + 'bound { /// let offset = 0; // Some offset. /// /// // If the size is known at compile time, use [`Self::iomap_sized`].
@@ -127,12 +127,12 @@ pub fn iomap_exclusive_sized<const SIZE: usize>(/// /// impl platform::Driver for SampleDriver { /// # type IdInfo = (); -/// # type Data = Self; +/// # type Data<'bound> = Self; /// -/// fn probe( -/// pdev: &platform::Device<Core<'_>>, -/// info: Option<&Self::IdInfo>, -/// ) -> impl PinInit<Self, Error> { +/// fn probe<'bound>( +/// pdev: &'bound platform::Device<Core<'_>>, +/// info: Option<&'bound Self::IdInfo>, +/// ) -> impl PinInit<Self, Error> + 'bound { /// let offset = 0; // Some offset. /// /// // Unlike [`Self::iomap_sized`], here the size of the memory region
rust/kernel/platform.rs +14 -14
@@ -50,7 +50,7 @@// - `DEVICE_DRIVER_OFFSET` is the correct byte offset to the embedded `struct device_driver`. unsafe impl <T :Driver > driver::DriverLayout for Adapter <T > {type DriverType = bindings::platform_driver ; -type DriverData <'bound> =T ::Data ; +type DriverData <'bound> =T ::Data <'bound>;const DEVICE_DRIVER_OFFSET :usize = core::mem::offset_of!(Self ::DriverType , driver); }
@@ -117,8 +117,8 @@ extern "C" fn remove_callback(pdev: *mut bindings::platform_device) {// SAFETY: `remove_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called -// and stored a `Pin<KBox<T::Data>>`. -let data =unsafe { pdev.as_ref ().drvdata_borrow ::<T ::Data >() }; +// and stored a `Pin<KBox<T::Data<'_>>>`. +let data =unsafe { pdev.as_ref ().drvdata_borrow ::<T ::Data <'_>>() };T ::unbind (pdev, data); }
@@ -192,14 +192,14 @@ macro_rules! module_platform_driver {/// /// impl platform::Driver for MyDriver { /// type IdInfo = (); -/// type Data = Self; +/// type Data<'bound> = Self; /// const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE); /// const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE); /// -/// fn probe( -/// _pdev: &platform::Device<Core<'_>>, -/// _id_info: Option<&Self::IdInfo>, -/// ) -> impl PinInit<Self, Error> { +/// fn probe<'bound>( +/// _pdev: &'bound platform::Device<Core<'_>>, +/// _id_info: Option<&'bound Self::IdInfo>, +/// ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound { /// Err(ENODEV) /// } /// }
@@ -214,7 +214,7 @@ pub trait Driver {type IdInfo : 'static;/// The type of the driver's bus device private data. -type Data :Send ; +type Data <'bound>:Send + 'bound;/// The table of OF device ids supported by the driver. const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =None ;
@@ -226,10 +226,10 @@ pub trait Driver {/// /// Called when a new platform device is added or discovered. /// Implementers should attempt to initialize the device here. -fn probe ( - dev: &Device <device::Core <'_>>, - id_info:Option <&Self ::IdInfo >, - ) ->impl PinInit <Self ::Data ,Error >; +fn probe <'bound>( + dev: &'boundDevice <device::Core <'_>>, + id_info:Option <&'boundSelf ::IdInfo >, + ) ->impl PinInit <Self ::Data <'bound>,Error > + 'bound;/// Platform driver unbind. ///
@@ -241,7 +241,7 @@ fn probe(/// operations to gracefully tear down the device. /// /// Otherwise, release operations for driver resources should be performed in `Drop`. -fn unbind (dev: &Device <device::Core <'_>>, this:Pin <&Self ::Data >) { +fn unbind <'bound>(dev: &'boundDevice <device::Core <'_>>, this:Pin <&Self ::Data <'bound>>) {let _ = (dev, this); } }
samples/rust/rust_debugfs.rs +5 -5
@@ -117,14 +117,14 @@ fn from_str(s: &str) -> Result<Self> {impl platform::Driver for RustDebugFs {type IdInfo = (); -type Data =Self ; +type Data <'bound> =Self ;const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =None ;const ACPI_ID_TABLE :Option <acpi::IdTable <Self ::IdInfo >> =Some (&ACPI_TABLE ); -fn probe ( - pdev: &platform::Device <Core <'_>>, - _info:Option <&Self ::IdInfo >, - ) ->impl PinInit <Self ,Error > { +fn probe <'bound>( + pdev: &'bound platform::Device <Core <'_>>, + _info:Option <&'boundSelf ::IdInfo >, + ) ->impl PinInit <Self ,Error > + 'bound {RustDebugFs ::new (pdev).pin_chain (|this| { this.counter.store (91 ,Relaxed ); {
samples/rust/rust_driver_platform.rs +5 -5
@@ -101,14 +101,14 @@ struct SampleDriver {impl platform::Driver for SampleDriver {type IdInfo =Info ; -type Data =Self ; +type Data <'bound> =Self ;const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =Some (&OF_TABLE );const ACPI_ID_TABLE :Option <acpi::IdTable <Self ::IdInfo >> =Some (&ACPI_TABLE ); -fn probe ( - pdev: &platform::Device <Core <'_>>, - info:Option <&Self ::IdInfo >, - ) ->impl PinInit <Self ,Error > { +fn probe <'bound>( + pdev: &'bound platform::Device <Core <'_>>, + info:Option <&'boundSelf ::IdInfo >, + ) ->impl PinInit <Self ,Error > + 'bound {let dev = pdev.as_ref ();dev_dbg! (dev,"Probe Rust Platform driver sample.\n" );
samples/rust/rust_i2c_client.rs +9 -6
@@ -106,14 +106,14 @@ struct SampleDriver {impl platform::Driver for SampleDriver {type IdInfo = (); -type Data =Self ; +type Data <'bound> =Self ;const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =Some (&OF_TABLE );const ACPI_ID_TABLE :Option <acpi::IdTable <Self ::IdInfo >> =Some (&ACPI_TABLE ); -fn probe ( - pdev: &platform::Device <device::Core <'_>>, - _info:Option <&Self ::IdInfo >, - ) ->impl PinInit <Self ,Error > { +fn probe <'bound>( + pdev: &'bound platform::Device <device::Core <'_>>, + _info:Option <&'boundSelf ::IdInfo >, + ) ->impl PinInit <Self ,Error > + 'bound {dev_info! ( pdev.as_ref (),"Probe Rust I2C Client registration sample.\n"
@@ -130,7 +130,10 @@ fn probe( }) } -fn unbind (pdev: &platform::Device <device::Core <'_>>, _this:Pin <&Self >) { +fn unbind <'bound>( + pdev: &'bound platform::Device <device::Core <'_>>, + _this:Pin <&Self ::Data <'bound>>, + ) {dev_info! ( pdev.as_ref (),"Unbind Rust I2C Client registration sample.\n"
samples/rust/rust_soc.rs +5 -5
@@ -37,14 +37,14 @@ struct SampleSocDriver {impl platform::Driver for SampleSocDriver {type IdInfo = (); -type Data =Self ; +type Data <'bound> =Self ;const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =Some (&OF_TABLE );const ACPI_ID_TABLE :Option <acpi::IdTable <Self ::IdInfo >> =Some (&ACPI_TABLE ); -fn probe ( - pdev: &platform::Device <Core <'_>>, - _info:Option <&Self ::IdInfo >, - ) ->impl PinInit <Self ,Error > { +fn probe <'bound>( + pdev: &'bound platform::Device <Core <'_>>, + _info:Option <&'boundSelf ::IdInfo >, + ) ->impl PinInit <Self ,Error > + 'bound {dev_dbg! (pdev,"Probe Rust SoC driver sample.\n" );let pdev = pdev.into ();
Signature · 1 line
2.54.0
Re: [PATCH v5 13/24] rust: platform: make Driver trait lifetime-parameterized
Quoted 10 lines · Danilo Krummrich
Add a 'bound lifetime to the associated Data, changing type Data to type Data<'bound>.
This allows the driver's bus device private data to capture the device / driver bound lifetime; device resources can be stored directly by reference rather than requiring Devres.
The
probe()andunbind()callbacks thus gain a 'bound lifetime parameter on the methods themselves; avoiding a global lifetime on the trait impl.
Quoted 7 lines
Existing drivers set type Data<'bound> = Self, preserving the current behavior.
Acked-by Uwe Kleine-König <[email protected]>Reviewed-by Alexandre Courbot <[email protected]>Reviewed-by Greg Kroah-Hartman <[email protected]>Signed-off-by Danilo Krummrich <[email protected]>
Quoted 12 lines
--- drivers/cpufreq/rcpufreq_dt.rs | 10 +++++----- drivers/gpu/drm/tyr/driver.rs | 10 +++++----- drivers/pwm/pwm_th1520.rs | 10 +++++----- rust/kernel/cpufreq.rs | 10 +++++----- rust/kernel/io/mem.rs | 20 ++++++++++---------- rust/kernel/platform.rs | 28 ++++++++++++++-------------- samples/rust/rust_debugfs.rs | 10 +++++----- samples/rust/rust_driver_platform.rs | 10 +++++----- samples/rust/rust_i2c_client.rs | 15 +++++++++------ samples/rust/rust_soc.rs | 10 +++++----- 10 files changed, 68 insertions(+), 65 deletions(-)
Danilo Krummrich
maintainerPATCH v5 14/24 rust: auxiliary: make Driver trait lifetime-parameterized
In reply to Danilo KrummrichAdd a 'bound lifetime to the associated Data, changing type Data to type Data<'bound>.
This allows the driver's bus device private data to capture the device / driver bound lifetime; device resources can be stored directly by reference rather than requiring Devres.
The probe() and unbind() callbacks thus gain a 'bound lifetime parameter on the methods themselves; avoiding a global lifetime on the trait impl.
Existing drivers set type Data<'bound> = Self, preserving the current behavior.
--- drivers/gpu/drm/nova/driver.rs | 10 +++++----- rust/kernel/auxiliary.rs | 18 +++++++++--------- samples/rust/rust_driver_auxiliary.rs | 10 +++++----- 3 files changed, 19 insertions(+), 19 deletions(-)
drivers/gpu/drm/nova/driver.rs +5 -5
@@ -51,13 +51,13 @@ pub(crate) struct NovaData {impl auxiliary::Driver for NovaDriver {type IdInfo = (); -type Data =Self ; +type Data <'bound> =Self ;const ID_TABLE : auxiliary::IdTable <Self ::IdInfo > = &AUX_TABLE ; -fn probe ( - adev: &auxiliary::Device <Core <'_>>, - _info: &Self ::IdInfo , - ) ->impl PinInit <Self ,Error > { +fn probe <'bound>( + adev: &'bound auxiliary::Device <Core <'_>>, + _info: &'boundSelf ::IdInfo , + ) ->impl PinInit <Self ,Error > + 'bound {let data =try_pin_init! (NovaData { adev: adev.into () });let drm = drm::Device ::<Self >::new (adev.as_ref (), data)?;
rust/kernel/auxiliary.rs +9 -9
@@ -46,7 +46,7 @@// - `DEVICE_DRIVER_OFFSET` is the correct byte offset to the embedded `struct device_driver`. unsafe impl <T :Driver > driver::DriverLayout for Adapter <T > {type DriverType = bindings::auxiliary_driver ; -type DriverData <'bound> =T ::Data ; +type DriverData <'bound> =T ::Data <'bound>;const DEVICE_DRIVER_OFFSET :usize = core::mem::offset_of!(Self ::DriverType , driver); }
@@ -111,8 +111,8 @@ extern "C" fn remove_callback(adev: *mut bindings::auxiliary_device) {// SAFETY: `remove_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called -// and stored a `Pin<KBox<T::Data>>`. -let data =unsafe { adev.as_ref ().drvdata_borrow ::<T ::Data >() }; +// and stored a `Pin<KBox<T::Data<'_>>>`. +let data =unsafe { adev.as_ref ().drvdata_borrow ::<T ::Data <'_>>() };T ::unbind (adev, data); }
@@ -203,7 +203,7 @@ pub trait Driver {type IdInfo : 'static;/// The type of the driver's bus device private data. -type Data :Send ; +type Data <'bound>:Send + 'bound;/// The table of device ids supported by the driver. const ID_TABLE :IdTable <Self ::IdInfo >;
@@ -211,10 +211,10 @@ pub trait Driver {/// Auxiliary driver probe. /// /// Called when an auxiliary device is matches a corresponding driver. -fn probe ( - dev: &Device <device::Core <'_>>, - id_info: &Self ::IdInfo , - ) ->impl PinInit <Self ::Data ,Error >; +fn probe <'bound>( + dev: &'boundDevice <device::Core <'_>>, + id_info: &'boundSelf ::IdInfo , + ) ->impl PinInit <Self ::Data <'bound>,Error > + 'bound;/// Auxiliary driver unbind. ///
@@ -226,7 +226,7 @@ fn probe(/// operations to gracefully tear down the device. /// /// Otherwise, release operations for driver resources should be performed in `Drop`. -fn unbind (dev: &Device <device::Core <'_>>, this:Pin <&Self ::Data >) { +fn unbind <'bound>(dev: &'boundDevice <device::Core <'_>>, this:Pin <&Self ::Data <'bound>>) {let _ = (dev, this); } }
samples/rust/rust_driver_auxiliary.rs +5 -5
@@ -31,14 +31,14 @@impl auxiliary::Driver for AuxiliaryDriver {type IdInfo = (); -type Data =Self ; +type Data <'bound> =Self ;const ID_TABLE : auxiliary::IdTable <Self ::IdInfo > = &AUX_TABLE ; -fn probe ( - adev: &auxiliary::Device <Core <'_>>, - _info: &Self ::IdInfo , - ) ->impl PinInit <Self ,Error > { +fn probe <'bound>( + adev: &'bound auxiliary::Device <Core <'_>>, + _info: &'boundSelf ::IdInfo , + ) ->impl PinInit <Self ,Error > + 'bound {dev_info! ( adev,"Probing auxiliary driver for auxiliary device with id={}\n" ,
Signature · 1 line
2.54.0
Re: [PATCH v5 14/24] rust: auxiliary: make Driver trait lifetime-parameterized
Quoted 10 lines · Danilo Krummrich
Add a 'bound lifetime to the associated Data, changing type Data to type Data<'bound>.
This allows the driver's bus device private data to capture the device / driver bound lifetime; device resources can be stored directly by reference rather than requiring Devres.
The
probe()andunbind()callbacks thus gain a 'bound lifetime parameter on the methods themselves; avoiding a global lifetime on the trait impl.
Quoted 6 lines
Existing drivers set type Data<'bound> = Self, preserving the current behavior.
Reviewed-by Alexandre Courbot <[email protected]>Reviewed-by Greg Kroah-Hartman <[email protected]>Signed-off-by Danilo Krummrich <[email protected]>
Quoted 5 lines
--- drivers/gpu/drm/nova/driver.rs | 10 +++++----- rust/kernel/auxiliary.rs | 18 +++++++++--------- samples/rust/rust_driver_auxiliary.rs | 10 +++++----- 3 files changed, 19 insertions(+), 19 deletions(-)
Danilo Krummrich
maintainerPATCH v5 15/24 rust: usb: make Driver trait lifetime-parameterized
In reply to Danilo KrummrichAdd a 'bound lifetime to the associated Data, changing type Data to type Data<'bound>.
This allows the driver's bus device private data to capture the device / driver bound lifetime; device resources can be stored directly by reference rather than requiring Devres.
The probe() and disconnect() callbacks thus gain a 'bound lifetime parameter on the methods themselves; avoiding a global lifetime on the trait impl.
Existing drivers set type Data<'bound> = Self, preserving the current behavior.
--- rust/kernel/usb.rs | 37 ++++++++++++++++++++------------- samples/rust/rust_driver_usb.rs | 12 +++++------ 2 files changed, 28 insertions(+), 21 deletions(-)
rust/kernel/usb.rs +22 -15
@@ -41,7 +41,7 @@// - `DEVICE_DRIVER_OFFSET` is the correct byte offset to the embedded `struct device_driver`. unsafe impl <T :Driver > driver::DriverLayout for Adapter <T > {type DriverType = bindings::usb_driver ; -type DriverData <'bound> =T ::Data ; +type DriverData <'bound> =T ::Data <'bound>;const DEVICE_DRIVER_OFFSET :usize = core::mem::offset_of!(Self ::DriverType , driver); }
@@ -109,8 +109,8 @@ extern "C" fn disconnect_callback(intf: *mut bindings::usb_interface) {// SAFETY: `disconnect_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called -// and stored a `Pin<KBox<T::Data>>`. -let data =unsafe { dev.drvdata_borrow ::<T ::Data >() }; +// and stored a `Pin<KBox<T::Data<'_>>>`. +let data =unsafe { dev.drvdata_borrow ::<T ::Data <'_>>() };T ::disconnect (intf, data); }
@@ -287,18 +287,22 @@ macro_rules! usb_device_table {/// /// impl usb::Driver for MyDriver { /// type IdInfo = (); -/// type Data = Self; +/// type Data<'bound> = Self; /// const ID_TABLE: usb::IdTable<Self::IdInfo> = &USB_TABLE; /// -/// fn probe( -/// _interface: &usb::Interface<Core<'_>>, +/// fn probe<'bound>( +/// _interface: &'bound usb::Interface<Core<'_>>, /// _id: &usb::DeviceId, -/// _info: &Self::IdInfo, -/// ) -> impl PinInit<Self::Data, Error> { +/// _info: &'bound Self::IdInfo, +/// ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound { /// Err(ENODEV) /// } /// -/// fn disconnect(_interface: &usb::Interface<Core<'_>>, _data: Pin<&Self::Data>) {} +/// fn disconnect<'bound>( +/// _interface: &'bound usb::Interface<Core<'_>>, +/// _data: Pin<&Self::Data<'bound>>, +/// ) { +/// } /// } ///``` pub trait Driver {
@@ -306,7 +310,7 @@ pub trait Driver {type IdInfo : 'static;/// The type of the driver's bus device private data. -type Data :Send ; +type Data <'bound>:Send + 'bound;/// The table of device ids supported by the driver. const ID_TABLE :IdTable <Self ::IdInfo >;
@@ -315,16 +319,19 @@ pub trait Driver {/// /// Called when a new USB interface is bound to this driver. /// Implementers should attempt to initialize the interface here. -fn probe ( - interface: &Interface <device::Core <'_>>, +fn probe <'bound>( + interface: &'boundInterface <device::Core <'_>>, id: &DeviceId , - id_info: &Self ::IdInfo , - ) ->impl PinInit <Self ::Data ,Error >; + id_info: &'boundSelf ::IdInfo , + ) ->impl PinInit <Self ::Data <'bound>,Error > + 'bound;/// USB driver disconnect. /// /// Called when the USB interface is about to be unbound from this driver. -fn disconnect (interface: &Interface <device::Core <'_>>, data:Pin <&Self ::Data >); +fn disconnect <'bound>( + interface: &'boundInterface <device::Core <'_>>, + data:Pin <&Self ::Data <'bound>>, + ); }/// A USB interface.
samples/rust/rust_driver_usb.rs +6 -6
@@ -26,21 +26,21 @@ struct SampleDriver {impl usb::Driver for SampleDriver {type IdInfo = (); -type Data =Self ; +type Data <'bound> =Self ;const ID_TABLE : usb::IdTable <Self ::IdInfo > = &USB_TABLE ; -fn probe ( - intf: &usb::Interface <Core <'_>>, +fn probe <'bound>( + intf: &'bound usb::Interface <Core <'_>>, _id: &usb::DeviceId , - _info: &Self ::IdInfo , - ) ->impl PinInit <Self ,Error > { + _info: &'boundSelf ::IdInfo , + ) ->impl PinInit <Self ,Error > + 'bound {let dev: &device::Device <Core <'_>> = intf.as_ref ();dev_info! (dev,"Rust USB driver sample probed\n" );Ok (Self { _intf: intf.into () }) } -fn disconnect (intf: &usb::Interface <Core <'_>>, _data:Pin <&Self >) { +fn disconnect <'bound>(intf: &'bound usb::Interface <Core <'_>>, _data:Pin <&Self >) {let dev: &device::Device <Core <'_>> = intf.as_ref ();dev_info! (dev,"Rust USB driver sample disconnected\n" ); }
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 15/24] rust: usb: make Driver trait lifetime-parameterized
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Lifetime transmute in `drvdata_borrow` and `drvdata_obtain` violates `ForeignOwnable` safety…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Lifetime transmute in drvdata_borrow and drvdata_obtain violates ForeignOwnable safety invariants. --
commit 7eabedf806aa3828a851211d08e32adafe52e9bb
Author: Danilo Krummrich <[email protected]>
rust: usb: make Driver trait lifetime-parameterized
This commit adds a 'bound lifetime to the associated Data type in the USB Driver trait. This allows driver data to capture the device bound lifetime and store device resources directly by reference without requiring Devres.
Quoted 4 lines
rust/kernel/usb.rs +0
-0
[ ... ]
rust/kernel/usb.rs extern "C" fn disconnect_callback(intf: *mut bindings::usb_interface) { · line 113
Quoted 8 lines
rust/kernel/usb.rs +2
-2@@ -109,8 +109,8 @@ extern "C" fn disconnect_callback(intf: *mut bindings::usb_interface) {// SAFETY: disconnect_callback is only ever called after a successful call to // probe_callback, hence it's guaranteed that Device::set_drvdata() has been called -// and stored a Pin<KBox<T::Data>>. -let data =unsafe { dev.drvdata_borrow ::<T ::Data >() }; +// and stored a Pin<KBox<T::Data<'_>>>. +let data =unsafe { dev.drvdata_borrow ::<T ::Data <'_>>() };
[Severity: High]
Does this lifetime transmute in drvdata_borrow violate the ForeignOwnable safety invariants?
The safety documentation for drvdata_borrow and drvdata_obtain requires that the type T must match the exact type of the ForeignOwnable previously stored. Because the probe callback stores the data with its local lifetime and the disconnect callback retrieves it with a different local lifetime, T::Data<'probe> and T::Data<'_> represent distinct types in Rust. Passing a different type to these unsafe functions violates their documented preconditions.
Additionally, since the Data<'bound> associated type does not guarantee covariance, could a driver implement an invariant type (for example, using Cell<&'bound Device>)?
While an immediate use-after-free might be hard to trigger because the C pointer remains valid, bypassing the borrow checker and casting lifetimes via raw pointers without covariance bounds appears to be structurally unsound.
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=15
Re: [PATCH v5 15/24] rust: usb: make Driver trait lifetime-parameterized
In reply to Danilo KrummrichQuoted 11 lines · Danilo Krummrich
Add a 'bound lifetime to the associated Data, changing type Data to type Data<'bound>.
This allows the driver's bus device private data to capture the device / driver bound lifetime; device resources can be stored directly by reference rather than requiring Devres.
The
probe()anddisconnect()callbacks thus gain a 'bound lifetime parameter on the methods themselves; avoiding a global lifetime on the trait impl.
Quoted 7 lines
Existing drivers set type Data<'bound> = Self, preserving the current behavior.
Reviewed-by Alexandre Courbot <[email protected]>Reviewed-by Greg Kroah-Hartman <[email protected]>Reviewed-by Eliot Courtney <[email protected]>Signed-off-by Danilo Krummrich <[email protected]>
Quoted 4 lines
--- rust/kernel/usb.rs | 37 ++++++++++++++++++++------------- samples/rust/rust_driver_usb.rs | 12 +++++------ 2 files changed, 28 insertions(+), 21 deletions(-)
Re: [PATCH v5 15/24] rust: usb: make Driver trait lifetime-parameterized
In reply to Danilo KrummrichQuoted 150 lines · Danilo Krummrich
Add a 'bound lifetime to the associated Data, changing type Data to type Data<'bound>.
This allows the driver's bus device private data to capture the device / driver bound lifetime; device resources can be stored directly by reference rather than requiring Devres.
The
probe()anddisconnect()callbacks thus gain a 'bound lifetime parameter on the methods themselves; avoiding a global lifetime on the trait impl.Existing drivers set type Data<'bound> = Self, preserving the current behavior.
Reviewed-by Alexandre Courbot <[email protected]>Reviewed-by Greg Kroah-Hartman <[email protected]>Reviewed-by Eliot Courtney <[email protected]>Signed-off-by Danilo Krummrich <[email protected]>---
rust/kernel/usb.rs| 37 ++++++++++++++++++++-------------samples/rust/rust_driver_usb.rs| 12 +++++------2 files changed, 28 insertions(+), 21 deletions(-)rust/kernel/usb.rs +22
-15@@ -41,7 +41,7 @@// - `DEVICE_DRIVER_OFFSET` is the correct byte offset to the embedded `struct device_driver`. unsafe impl <T :Driver > driver::DriverLayout for Adapter <T > {type DriverType = bindings::usb_driver ; -type DriverData <'bound> =T ::Data ; +type DriverData <'bound> =T ::Data <'bound>;const DEVICE_DRIVER_OFFSET :usize = core::mem::offset_of!(Self ::DriverType , driver); }@@ -109,8 +109,8 @@ extern "C" fn disconnect_callback(intf: *mut bindings::usb_interface) {// SAFETY: `disconnect_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called -// and stored a `Pin<KBox<T::Data>>`. -let data =unsafe { dev.drvdata_borrow ::<T ::Data >() }; +// and stored a `Pin<KBox<T::Data<'_>>>`. +let data =unsafe { dev.drvdata_borrow ::<T ::Data <'_>>() };T ::disconnect (intf, data); }@@ -287,18 +287,22 @@ macro_rules! usb_device_table {/// /// impl usb::Driver for MyDriver { /// type IdInfo = (); -/// type Data = Self; +/// type Data<'bound> = Self; /// const ID_TABLE: usb::IdTable<Self::IdInfo> = &USB_TABLE; /// -/// fn probe( -/// _interface: &usb::Interface<Core<'_>>, +/// fn probe<'bound>( +/// _interface: &'bound usb::Interface<Core<'_>>, /// _id: &usb::DeviceId, -/// _info: &Self::IdInfo, -/// ) -> impl PinInit<Self::Data, Error> { +/// _info: &'bound Self::IdInfo, +/// ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound { /// Err(ENODEV) /// } /// -/// fn disconnect(_interface: &usb::Interface<Core<'_>>, _data: Pin<&Self::Data>) {} +/// fn disconnect<'bound>( +/// _interface: &'bound usb::Interface<Core<'_>>, +/// _data: Pin<&Self::Data<'bound>>, +/// ) { +/// } /// } ///``` pub trait Driver {@@ -306,7 +310,7 @@ pub trait Driver {type IdInfo : 'static;/// The type of the driver's bus device private data. -type Data :Send ; +type Data <'bound>:Send + 'bound;/// The table of device ids supported by the driver. const ID_TABLE :IdTable <Self ::IdInfo >;@@ -315,16 +319,19 @@ pub trait Driver {/// /// Called when a new USB interface is bound to this driver. /// Implementers should attempt to initialize the interface here. -fn probe ( - interface: &Interface <device::Core <'_>>, +fn probe <'bound>( + interface: &'boundInterface <device::Core <'_>>, id: &DeviceId , - id_info: &Self ::IdInfo , - ) ->impl PinInit <Self ::Data ,Error >; + id_info: &'boundSelf ::IdInfo , + ) ->impl PinInit <Self ::Data <'bound>,Error > + 'bound;/// USB driver disconnect. /// /// Called when the USB interface is about to be unbound from this driver. -fn disconnect (interface: &Interface <device::Core <'_>>, data:Pin <&Self ::Data >); +fn disconnect <'bound>( + interface: &'boundInterface <device::Core <'_>>, + data:Pin <&Self ::Data <'bound>>, + ); }/// A USB interface. samples/rust/rust_driver_usb.rs +6
-6@@ -26,21 +26,21 @@ struct SampleDriver {impl usb::Driver for SampleDriver {type IdInfo = (); -type Data =Self ; +type Data <'bound> =Self ;const ID_TABLE : usb::IdTable <Self ::IdInfo > = &USB_TABLE ; -fn probe ( - intf: &usb::Interface <Core <'_>>, +fn probe <'bound>( + intf: &'bound usb::Interface <Core <'_>>, _id: &usb::DeviceId , - _info: &Self ::IdInfo , - ) ->impl PinInit <Self ,Error > { + _info: &'boundSelf ::IdInfo , + ) ->impl PinInit <Self ,Error > + 'bound {let dev: &device::Device <Core <'_>> = intf.as_ref ();dev_info! (dev,"Rust USB driver sample probed\n" );Ok (Self { _intf: intf.into () }) } -fn disconnect (intf: &usb::Interface <Core <'_>>, _data:Pin <&Self >) { +fn disconnect <'bound>(intf: &'bound usb::Interface <Core <'_>>, _data:Pin <&Self >) {let dev: &device::Device <Core <'_>> = intf.as_ref ();dev_info! (dev,"Rust USB driver sample disconnected\n" ); }Signature · 1 line
2.54.0
Danilo Krummrich
maintainerPATCH v5 16/24 rust: i2c: make Driver trait lifetime-parameterized
In reply to Danilo KrummrichAdd a 'bound lifetime to the associated Data, changing type Data to type Data<'bound>.
This allows the driver's bus device private data to capture the device / driver bound lifetime; device resources can be stored directly by reference rather than requiring Devres.
The probe() and unbind() callbacks thus gain a 'bound lifetime parameter on the methods themselves; avoiding a global lifetime on the trait impl.
Existing drivers set type Data<'bound> = Self, preserving the current behavior.
--- rust/kernel/i2c.rs | 39 ++++++++++++++++++--------------- samples/rust/rust_driver_i2c.rs | 14 ++++++------ 2 files changed, 28 insertions(+), 25 deletions(-)
rust/kernel/i2c.rs +21 -18
@@ -98,7 +98,7 @@ macro_rules! i2c_device_table {// - `DEVICE_DRIVER_OFFSET` is the correct byte offset to the embedded `struct device_driver`. unsafe impl <T :Driver > driver::DriverLayout for Adapter <T > {type DriverType = bindings::i2c_driver ; -type DriverData <'bound> =T ::Data ; +type DriverData <'bound> =T ::Data <'bound>;const DEVICE_DRIVER_OFFSET :usize = core::mem::offset_of!(Self ::DriverType , driver); }
@@ -176,8 +176,8 @@ extern "C" fn remove_callback(idev: *mut bindings::i2c_client) {// SAFETY: `remove_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `I2cClient::set_drvdata()` has been called -// and stored a `Pin<KBox<T::Data>>`. -let data =unsafe { idev.as_ref ().drvdata_borrow ::<T ::Data >() }; +// and stored a `Pin<KBox<T::Data<'_>>>`. +let data =unsafe { idev.as_ref ().drvdata_borrow ::<T ::Data <'_>>() };T ::unbind (idev, data); }
@@ -188,8 +188,8 @@ extern "C" fn shutdown_callback(idev: *mut bindings::i2c_client) {// SAFETY: `shutdown_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `Device::set_drvdata()` has been called -// and stored a `Pin<KBox<T::Data>>`. -let data =unsafe { idev.as_ref ().drvdata_borrow ::<T ::Data >() }; +// and stored a `Pin<KBox<T::Data<'_>>>`. +let data =unsafe { idev.as_ref ().drvdata_borrow ::<T ::Data <'_>>() };T ::shutdown (idev, data); }
@@ -294,19 +294,22 @@ macro_rules! module_i2c_driver {/// /// impl i2c::Driver for MyDriver { /// type IdInfo = (); -/// type Data = Self; +/// type Data<'bound> = Self; /// const I2C_ID_TABLE: Option<i2c::IdTable<Self::IdInfo>> = Some(&I2C_TABLE); /// const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = Some(&OF_TABLE); /// const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = Some(&ACPI_TABLE); /// -/// fn probe( -/// _idev: &i2c::I2cClient<Core<'_>>, -/// _id_info: Option<&Self::IdInfo>, -/// ) -> impl PinInit<Self::Data, Error> { +/// fn probe<'bound>( +/// _idev: &'bound i2c::I2cClient<Core<'_>>, +/// _id_info: Option<&'bound Self::IdInfo>, +/// ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound { /// Err(ENODEV) /// } /// -/// fn shutdown(_idev: &i2c::I2cClient<Core<'_>>, this: Pin<&Self::Data>) { +/// fn shutdown<'bound>( +/// _idev: &'bound i2c::I2cClient<Core<'_>>, +/// this: Pin<&Self::Data<'bound>>, +/// ) { /// } /// } ///```
@@ -320,7 +323,7 @@ pub trait Driver {type IdInfo : 'static;/// The type of the driver's bus device private data. -type Data :Send ; +type Data <'bound>:Send + 'bound;/// The table of device ids supported by the driver. const I2C_ID_TABLE :Option <IdTable <Self ::IdInfo >> =None ;
@@ -335,10 +338,10 @@ pub trait Driver {/// /// Called when a new i2c client is added or discovered. /// Implementers should attempt to initialize the client here. -fn probe ( - dev: &I2cClient <device::Core <'_>>, - id_info:Option <&Self ::IdInfo >, - ) ->impl PinInit <Self ::Data ,Error >; +fn probe <'bound>( + dev: &'boundI2cClient <device::Core <'_>>, + id_info:Option <&'boundSelf ::IdInfo >, + ) ->impl PinInit <Self ::Data <'bound>,Error > + 'bound;/// I2C driver shutdown. ///
@@ -351,7 +354,7 @@ fn probe(/// This callback is distinct from final resource cleanup, as the driver instance remains valid /// after it returns. Any deallocation or teardown of driver-owned resources should instead be /// handled in `Drop`. -fn shutdown (dev: &I2cClient <device::Core <'_>>, this:Pin <&Self ::Data >) { +fn shutdown <'bound>(dev: &'boundI2cClient <device::Core <'_>>, this:Pin <&Self ::Data <'bound>>) {let _ = (dev, this); }
@@ -365,7 +368,7 @@ fn shutdown(dev: &I2cClient<device::Core<'_>>, this: Pin<&Self::Data>) {/// operations to gracefully tear down the device. /// /// Otherwise, release operations for driver resources should be performed in `Drop`. -fn unbind (dev: &I2cClient <device::Core <'_>>, this:Pin <&Self ::Data >) { +fn unbind <'bound>(dev: &'boundI2cClient <device::Core <'_>>, this:Pin <&Self ::Data <'bound>>) {let _ = (dev, this); } }
samples/rust/rust_driver_i2c.rs +7 -7
@@ -35,16 +35,16 @@impl i2c::Driver for SampleDriver {type IdInfo =u32 ; -type Data =Self ; +type Data <'bound> =Self ;const ACPI_ID_TABLE :Option <acpi::IdTable <Self ::IdInfo >> =Some (&ACPI_TABLE );const I2C_ID_TABLE :Option <i2c::IdTable <Self ::IdInfo >> =Some (&I2C_TABLE );const OF_ID_TABLE :Option <of::IdTable <Self ::IdInfo >> =Some (&OF_TABLE ); -fn probe ( - idev: &i2c::I2cClient <Core <'_>>, - info:Option <&Self ::IdInfo >, - ) ->impl PinInit <Self ,Error > { +fn probe <'bound>( + idev: &'bound i2c::I2cClient <Core <'_>>, + info:Option <&'boundSelf ::IdInfo >, + ) ->impl PinInit <Self ,Error > + 'bound {let dev = idev.as_ref ();dev_info! (dev,"Probe Rust I2C driver sample.\n" );
@@ -56,11 +56,11 @@ fn probe(Ok (Self ) } -fn shutdown (idev: &i2c::I2cClient <Core <'_>>, _this:Pin <&Self >) { +fn shutdown <'bound>(idev: &'bound i2c::I2cClient <Core <'_>>, _this:Pin <&Self >) {dev_info! (idev.as_ref (),"Shutdown Rust I2C driver sample.\n" ); } -fn unbind (idev: &i2c::I2cClient <Core <'_>>, _this:Pin <&Self >) { +fn unbind <'bound>(idev: &'bound i2c::I2cClient <Core <'_>>, _this:Pin <&Self >) {dev_info! (idev.as_ref (),"Unbind Rust I2C driver sample.\n" ); } }
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 16/24] rust: i2c: make Driver trait lifetime-parameterized
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Adding a `'bound` lifetime parameter to the driver's `Data` associated type causes unsound…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Adding a 'bound lifetime parameter to the driver's Data associated type causes unsound lifetime transmutation during driver lifecycle callbacks, leading to potential use-after-free vulnerabilities. --
commit 9a3d76f725cb591e4d7bb2fd07852884f495a892
Author: Danilo Krummrich <[email protected]>
rust: i2c: make Driver trait lifetime-parameterized
This commit adds a 'bound lifetime to the associated Data type in the I2C Driver trait. This allows the bus device private data to store references to device resources directly instead of requiring Devres.
Quoted 4 lines
rust/kernel/i2c.rs +0
-0
[ ... ]
rust/kernel/i2c.rs extern "C" fn remove_callback(idev: *mut bindings::i2c_client) { · line 180
Quoted 8 lines
rust/kernel/i2c.rs +2
-2@@ -176,8 +176,8 @@ extern "C" fn remove_callback(idev: *mut bindings::i2c_client) {// SAFETY: `remove_callback` is only ever called after a successful call to // `probe_callback`, hence it's guaranteed that `I2cClient::set_drvdata()` has been called -// and stored a `Pin<KBox<T::Data>>`. -let data =unsafe { idev.as_ref ().drvdata_borrow ::<T ::Data >() }; +// and stored a `Pin<KBox<T::Data<'_>>>`. +let data =unsafe { idev.as_ref ().drvdata_borrow ::<T ::Data <'_>>() };
[Severity: High]
Does this type inference violate the safety contract of drvdata_borrow()?
During probe(), the data is stored with the lifetime of the probe call's reference to idev, creating a T::Data<'probe>. However, here in remove_callback(), drvdata_borrow::<T::Data<'_>>() infers a new lifetime based on the local remove_callback() reference, resulting in T::Data<'remove>.
Since drvdata_borrow() requires the types to match exactly, changing the inferred lifetime seems to cause an unsound transmutation. If T::Data<'bound> is invariant over the lifetime (for example, if it contains a Mutex holding an Option<&'bound I2cClient>), could safe Rust driver code stash the short lived 'remove reference inside the Data struct during unbind?
When remove_callback() returns, the 'remove reference would be invalidated. If the driver data is subsequently dropped later on, could the Drop implementation read the stashed, now-dangling reference and cause a use-after-free?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=16
Re: [PATCH v5 16/24] rust: i2c: make Driver trait lifetime-parameterized
In reply to Danilo KrummrichQuoted 10 lines · Danilo Krummrich
Add a 'bound lifetime to the associated Data, changing type Data to type Data<'bound>.
This allows the driver's bus device private data to capture the device / driver bound lifetime; device resources can be stored directly by reference rather than requiring Devres.
The
probe()andunbind()callbacks thus gain a 'bound lifetime parameter on the methods themselves; avoiding a global lifetime on the trait impl.
Quoted 7 lines
Existing drivers set type Data<'bound> = Self, preserving the current behavior.
Acked-by Igor Korotin <[email protected]>Reviewed-by Alexandre Courbot <[email protected]>Reviewed-by Greg Kroah-Hartman <[email protected]>Signed-off-by Danilo Krummrich <[email protected]>
Quoted 4 lines
--- rust/kernel/i2c.rs | 39 ++++++++++++++++++--------------- samples/rust/rust_driver_i2c.rs | 14 ++++++------ 2 files changed, 28 insertions(+), 25 deletions(-)
Danilo Krummrich
maintainerPATCH v5 17/24 rust: driver: update module documentation for GAT-based Data type
In reply to Danilo KrummrichNow that all bus driver traits use type Data<'bound>: 'bound, update the illustrative driver trait in the module documentation to reflect the GAT pattern and lifetime-parameterized callbacks.
--- rust/kernel/driver.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)
rust/kernel/driver.rs +13 -7
@@ -18,7 +18,7 @@//! type IdInfo: 'static; //! //! /// The type of the driver's bus device private data. -//! type Data: Send; +//! type Data<'bound>: Send + 'bound; //! //! /// The table of OF device ids supported by the driver. //! const OF_ID_TABLE: Option<of::IdTable<Self::IdInfo>> = None;
@@ -27,11 +27,16 @@//! const ACPI_ID_TABLE: Option<acpi::IdTable<Self::IdInfo>> = None; //! //! /// Driver probe. -//! fn probe(dev: &Device<device::Core<'_>>, id_info: &Self::IdInfo) -//! -> impl PinInit<Self::Data, Error>; +//! fn probe<'bound>( +//! dev: &'bound Device<device::Core<'_>>, +//! id_info: &'bound Self::IdInfo, +//! ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound; //! //! /// Driver unbind (optional). -//! fn unbind(dev: &Device<device::Core<'_>>, this: Pin<&Self::Data>) { +//! fn unbind<'bound>( +//! dev: &'bound Device<device::Core<'_>>, +//! this: Pin<&Self::Data<'bound>>, +//! ) { //! let _ = (dev, this); //! } //! }
@@ -46,9 +51,10 @@ )]#" )] //! -//! The `probe()` callback should return a `impl PinInit<Self::Data, Error>`, i.e. the driver's -//! private data. The bus abstraction should store the pointer in the corresponding bus device. The -//! generic [`Device`] infrastructure provides common helpers for this purpose on its +//! The `probe()` callback should return a +//! `impl PinInit<Self::Data<'bound>, Error>`, i.e. the driver's private data. The bus +//! abstraction should store the pointer in the corresponding bus device. The generic +//! [`Device`] infrastructure provides common helpers for this purpose on its //! [`Device<CoreInternal>`] implementation. //! //! All driver callbacks should provide a reference to the driver's private data. Once the driver
Signature · 1 line
2.54.0
Re: [PATCH v5 17/24] rust: driver: update module documentation for GAT-based Data type
Quoted 7 lines · Danilo Krummrich
Now that all bus driver traits use type Data<'bound>: 'bound, update the illustrative driver trait in the module documentation to reflect the GAT pattern and lifetime-parameterized callbacks.
Reviewed-by Alexandre Courbot <[email protected]>Reviewed-by Greg Kroah-Hartman <[email protected]>Signed-off-by Danilo Krummrich <[email protected]>
Quoted 3 lines
--- rust/kernel/driver.rs | 20 +++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-)
Danilo Krummrich
maintainerPATCH v5 18/24 rust: pci: make Bar lifetime-parameterized
In reply to Danilo KrummrichConvert pci::Bar<SIZE> to pci::Bar<'a, SIZE>, storing &'a Device<Bound> to tie the BAR mapping lifetime to the device.
iomap_region_sized() now returns Result<Bar<'a, SIZE>> directly instead of impl PinInit<Devres<Bar<SIZE>>, Error>.
Since the lifetime ties the mapping to the device's bound state, callers no longer need Devres for the common case where the Bar lives in the driver's private data.
Add Bar::into_devres() to consume the bar and register it as a device-managed resource, returning Devres<Bar<'static, SIZE>>. The lifetime is erased to 'static because Devres guarantees the bar does not actually outlive the device -- access is revoked on unbind.
--- drivers/gpu/nova-core/driver.rs | 7 +++-- rust/kernel/devres.rs | 2 +- rust/kernel/pci/io.rs | 52 +++++++++++++++++++-------------- samples/rust/rust_driver_pci.rs | 5 ++-- 4 files changed, 38 insertions(+), 28 deletions(-)
drivers/gpu/nova-core/driver.rs +4 -3
@@ -45,7 +45,7 @@ pub(crate) struct NovaCore {// DMA addresses. These systems should be quite rare. const GPU_DMA_BITS :u32 =47 ; -pub (crate )type Bar0 = pci::Bar <BAR0_SIZE >; +pub (crate )type Bar0 = pci::Bar <'static,BAR0_SIZE >; kernel::pci_device_table!(PCI_TABLE ,
@@ -92,8 +92,9 @@ fn probe<'bound>(// other threads of execution. unsafe { pdev.dma_set_mask_and_coherent (DmaMask ::new ::<GPU_DMA_BITS >())? }; -let bar =Arc ::pin_init ( - pdev.iomap_region_sized ::<BAR0_SIZE >(0 ,c"nova-core/bar0" ), +let bar =Arc ::new ( + pdev.iomap_region_sized ::<BAR0_SIZE >(0 ,c"nova-core/bar0" )? + .into_devres ()?,GFP_KERNEL , )?;
rust/kernel/devres.rs +1 -1
@@ -304,7 +304,7 @@ pub fn device(&self) -> &Device {/// pci, // /// }; /// -/// fn from_core(dev: &pci::Device<Core<'_>>, devres: Devres<pci::Bar<0x4>>) -> Result { +/// fn from_core(dev: &pci::Device<Core<'_>>, devres: Devres<pci::Bar<'_, 0x4>>) -> Result { /// let bar = devres.access(dev.as_ref())?; /// /// let _ = bar.read32(0x0);
rust/kernel/pci/io.rs +30 -22
103 lines not on this page · raw message
samples/rust/rust_driver_pci.rs +3 -2
@@ -45,7 +45,7 @@ mod regs {pub (super )const END :usize =0x10 ; } -type Bar0 = pci::Bar <{ regs::END }>; +type Bar0 = pci::Bar <'static, { regs::END }>;#[derive( Copy , Clone , Debug )] struct TestIndex (u8 );
@@ -161,7 +161,8 @@ fn probe<'bound>( pdev.set_master (); Ok(try_pin_init!(Self { - bar <- pdev.iomap_region_sized::<{ regs::END }>(0, c"rust_driver_pci"), + bar: pdev.iomap_region_sized::<{ regs::END }>(0, c"rust_driver_pci")? + .into_devres()?, index: *info, _: { let bar = bar.access(pdev.as_ref())?;
Signature · 1 line
2.54.0
Re: [PATCH v5 18/24] rust: pci: make Bar lifetime-parameterized
Quoted 15 lines · Danilo Krummrich
Convert pci::Bar<SIZE> to pci::Bar<'a, SIZE>, storing &'a Device<Bound> to tie the BAR mapping lifetime to the device.
iomap_region_sized()now returns Result<Bar<'a, SIZE>> directly instead of impl PinInit<Devres<Bar<SIZE>>, Error>.Since the lifetime ties the mapping to the device's bound state, callers no longer need Devres for the common case where the Bar lives in the driver's private data.
Add
Bar::into_devres()to consume the bar and register it as a device-managed resource, returning Devres<Bar<'static, SIZE>>. The lifetime is erased to 'static because Devres guarantees the bar does not actually outlive the device -- access is revoked on unbind.
Quoted 4 lines
Reviewed-by Eliot Courtney <[email protected]>Reviewed-by Greg Kroah-Hartman <[email protected]>Reviewed-by Alexandre Courbot <[email protected]>Signed-off-by Danilo Krummrich <[email protected]>
Quoted 6 lines
--- drivers/gpu/nova-core/driver.rs | 7 +++-- rust/kernel/devres.rs | 2 +- rust/kernel/pci/io.rs | 52 +++++++++++++++++++-------------- samples/rust/rust_driver_pci.rs | 5 ++-- 4 files changed, 38 insertions(+), 28 deletions(-)
Danilo Krummrich
maintainerPATCH v5 19/24 rust: io: make IoMem and ExclusiveIoMem lifetime-parameterized
In reply to Danilo KrummrichAdd a lifetime parameter to IoMem<'a, SIZE> and ExclusiveIoMem<'a, SIZE>, storing a &'a Device<Bound> reference to tie the mapping to the device's lifetime.
This mirrors the pci::Bar<'a, SIZE> design and enables drivers to hold I/O memory mappings directly in their HRT private data, tied to the device lifetime.
IoRequest::iomap_* methods now return the mapping directly instead of wrapping it in Devres. Callers that need device-managed revocation can call the new into_devres() method.
--- drivers/gpu/drm/tyr/driver.rs | 4 +- drivers/pwm/pwm_th1520.rs | 4 +- rust/kernel/io/mem.rs | 103 +++++++++++++++++----------------- 3 files changed, 56 insertions(+), 55 deletions(-)
drivers/gpu/drm/tyr/driver.rs +2 -2
@@ -37,7 +37,7 @@ regs, // }; -pub (crate )type IoMem = kernel::io::mem::IoMem <SZ_2M >; +pub (crate )type IoMem = kernel::io::mem::IoMem <'static,SZ_2M >;pub (crate )struct TyrDrmDriver ;
@@ -110,7 +110,7 @@ fn probe<'bound>(let sram_regulator =Regulator ::<regulator::Enabled >::get (pdev.as_ref (),c"sram" )?;let request = pdev.io_request_by_index (0 ).ok_or (ENODEV )?; -let iomem =Arc ::pin_init (request.iomap_sized ::<SZ_2M >(),GFP_KERNEL )?; +let iomem =Arc ::new (request.iomap_sized ::<SZ_2M >()?.into_devres ()?,GFP_KERNEL )?;issue_soft_reset (pdev.as_ref (), &iomem)?; gpu::l2_power_on (pdev.as_ref (), &iomem)?;
drivers/pwm/pwm_th1520.rs +2 -2
@@ -92,7 +92,7 @@ struct Th1520WfHw {#[pin_data( PinnedDrop )] struct Th1520PwmDriverData {#[pin] - iomem: devres::Devres <IoMem <TH1520_PWM_REG_SIZE >>, + iomem: devres::Devres <IoMem <'static,TH1520_PWM_REG_SIZE >>, clk:Clk , }
@@ -352,7 +352,7 @@ fn probe<'bound>( dev, TH1520_MAX_PWM_NUM,try_pin_init! (Th1520PwmDriverData { - iomem <- request.iomap_sized ::<TH1520_PWM_REG_SIZE >(), + iomem <- request.iomap_sized ::<TH1520_PWM_REG_SIZE >()?.into_devres (), clk <- clk, }), )?;
rust/kernel/io/mem.rs +52 -51
195 lines not on this page · raw message
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 19/24] rust: io: make IoMem and ExclusiveIoMem lifetime-parameterized
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues:
- [High] Incorrect syntax and type mismatch in try_pin_init! macro leads to a compilation failure in drivers/pwm/pwm_th1520.rs.
Pre-existing issues:
- [Low] The struct construction for ExclusiveIoMem is missing the mandated // INVARIANT: comment. --
commit 67b598dd02fe735e45135c07185ac5128b22e8d0
Author: Danilo Krummrich <[email protected]>
rust: io: make IoMem and ExclusiveIoMem lifetime-parameterized
This commit adds a lifetime parameter to IoMem and ExclusiveIoMem, tying the mapping to the device's lifetime. It mirrors the pci::Bar design and enables drivers to hold I/O memory mappings directly in their HRT private data.
Quoted 4 lines
drivers/pwm/pwm_th1520.rs +0
-0
[ ... ]
drivers/pwm/pwm_th1520.rs fn probe<'bound>( · line 355
Quoted 6 lines
drivers/pwm/pwm_th1520.rs +1
-1@@ -352,7 +352,7 @@ fn probe<'bound>( dev, TH1520_MAX_PWM_NUM,try_pin_init! (Th1520PwmDriverData { - iomem <- request.iomap_sized ::<TH1520_PWM_REG_SIZE >(), + iomem <- request.iomap_sized ::<TH1520_PWM_REG_SIZE >()?.into_devres (),
[Severity: High]
Does this assignment cause a type mismatch in the try_pin_init! macro?
The into_devres() method returns a Result, but the <- operator expects an expression that implements the PinInit trait. Could this be missing a trailing ? operator to unwrap the Result before the pin initialization?
Quoted 8 lines
drivers/pwm/pwm_th1520.rs +0
-0clk <- clk, }), )?;rust/kernel/io/mem.rs +0
-0
[ ... ]
rust/kernel/io/mem.rs fn ioremap(resource: &Resource) -> Result<Self> { · line 193
Quoted 9 lines
rust/kernel/io/mem.rs +2
-2@@ -196,26 +188,29 @@ fn ioremap(resource: &Resource) -> Result<Self> { ) .ok_or(EBUSY)?; -let iomem =IoMem ::ioremap (resource)?; +let iomem =IoMem ::ioremap (dev, resource)?; -let iomem =ExclusiveIoMem { +Ok (ExclusiveIoMem {
[Severity: Low]
This isn't a bug introduced by this patch, but ExclusiveIoMem has an # Invariants documentation section. According to the Rust subsystem coding guidelines, shouldn't its construction include an // INVARIANT: comment explaining why the invariants are satisfied?
Quoted 7 lines
rust/kernel/io/mem.rs +1
-3iomem, _region: region, - }; - -Ok (iomem) + }) }
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=19
Re: [PATCH v5 19/24] rust: io: make IoMem and ExclusiveIoMem lifetime-parameterized
In reply to Danilo KrummrichQuoted 12 lines · Danilo Krummrich
Add a lifetime parameter to IoMem<'a, SIZE> and ExclusiveIoMem<'a, SIZE>, storing a &'a Device<Bound> reference to tie the mapping to the device's lifetime.
This mirrors the pci::Bar<'a, SIZE> design and enables drivers to hold I/O memory mappings directly in their HRT private data, tied to the device lifetime.
IoRequest::iomap_* methods now return the mapping directly instead of wrapping it in Devres. Callers that need device-managed revocation can call the new
into_devres()method.
Quoted 5 lines
Acked-by Uwe Kleine-König <[email protected]>Reviewed-by Eliot Courtney <[email protected]>Reviewed-by Greg Kroah-Hartman <[email protected]>Reviewed-by Alexandre Courbot <[email protected]>Signed-off-by Danilo Krummrich <[email protected]>
Quoted 5 lines
--- drivers/gpu/drm/tyr/driver.rs | 4 +- drivers/pwm/pwm_th1520.rs | 4 +- rust/kernel/io/mem.rs | 103 +++++++++++++++++----------------- 3 files changed, 56 insertions(+), 55 deletions(-)
Danilo Krummrich
maintainerPATCH v5 20/24 samples: rust: rust_driver_pci: use HRT lifetime for Bar
In reply to Danilo KrummrichConvert the sample driver to SampleDriver<'bound>, taking advantage of the lifetime-parameterized Driver trait.
The driver struct holds &'bound pci::Device directly instead of ARef<pci::Device>, and pci::Bar<'bound> directly instead of Devres<pci::Bar>. This removes PinnedDrop, pin_init_scope, and runtime revocation checks on BAR access.
--- samples/rust/rust_driver_pci.rs | 89 +++++++++++++++------------------ 1 file changed, 40 insertions(+), 49 deletions(-)
samples/rust/rust_driver_pci.rs +40 -49
@@ -9,7 +9,6 @@ Bound, Core, // }, - devres::Devres, io::{ register, register::Array,
@@ -17,8 +16,7 @@ }, num::Bounded, pci, - prelude::*, - sync::aref::ARef, // + prelude::*, // };mod regs {
@@ -45,7 +43,7 @@ mod regs {pub (super )const END :usize =0x10 ; } -type Bar0 = pci::Bar <'static, { regs::END }>; +type Bar0 <'bound> = pci::Bar <'bound, { regs::END }>;#[derive( Copy , Clone , Debug )] struct TestIndex (u8 );
@@ -66,14 +64,14 @@ impl TestIndex {const NO_EVENTFD :Self =Self (0 ); } -#[pin_data( PinnedDrop )] -struct SampleDriver { - pdev:ARef <pci::Device >, -#[pin] - bar:Devres <Bar0 >, +struct SampleDriverData <'bound> { + pdev: &'bound pci::Device , + bar:Bar0 <'bound>, index:TestIndex , } +struct SampleDriver ; + kernel::pci_device_table!(PCI_TABLE ,MODULE_PCI_TABLE ,
@@ -84,8 +82,8 @@ struct SampleDriver { )] ); -impl SampleDriver { -fn testdev (index: &TestIndex , bar: &Bar0 ) ->Result <u32 > { +impl SampleDriverData <'_> { +fn testdev (index: &TestIndex , bar: &Bar0 <'_>) ->Result <u32 > {// Select the test. bar.write_reg (regs::TEST ::zeroed ().with_index (*index));
@@ -140,56 +138,49 @@ fn config_space(pdev: &pci::Device<Bound>) {impl pci::Driver for SampleDriver {type IdInfo =TestIndex ; -type Data <'bound> =Self ; +type Data <'bound> =SampleDriverData <'bound>;const ID_TABLE : pci::IdTable <Self ::IdInfo > = &PCI_TABLE ;fn probe <'bound>( pdev: &'bound pci::Device <Core <'_>>, info: &'boundSelf ::IdInfo , - ) ->impl PinInit <Self ,Error > + 'bound { - pin_init::pin_init_scope (move || { -let vendor = pdev.vendor_id (); -dev_dbg! ( - pdev, -"Probe Rust PCI driver sample (PCI ID: {}, 0x{:x}).\n" , - vendor, - pdev.device_id () - ); - - pdev.enable_device_mem ()?; - pdev.set_master (); - -Ok (try_pin_init! (Self { - bar: pdev.iomap_region_sized ::<{ regs::END }>(0 ,c"rust_driver_pci" )? - .into_devres ()?, - index: *info, - _: { -let bar = bar.access (pdev.as_ref ())?; - -dev_info! ( - pdev, -"pci-testdev data-match count: {}\n" , -Self ::testdev (info, bar)? - ); -Self ::config_space (pdev); - }, - pdev: pdev.into (), - })) + ) ->impl PinInit <Self ::Data <'bound>,Error > + 'bound { +let vendor = pdev.vendor_id (); +dev_dbg! ( + pdev, +"Probe Rust PCI driver sample (PCI ID: {}, 0x{:x}).\n" , + vendor, + pdev.device_id () + ); + + pdev.enable_device_mem ()?; + pdev.set_master (); + +let bar = pdev.iomap_region_sized ::<{ regs::END }>(0 ,c"rust_driver_pci" )?; + +dev_info! ( + pdev, +"pci-testdev data-match count: {}\n" , +SampleDriverData ::testdev (info, &bar)? + ); +SampleDriverData ::config_space (pdev); + +Ok (SampleDriverData { + pdev, + bar, + index: *info, }) } -fn unbind (pdev: &pci::Device <Core <'_>>, this:Pin <&Self >) { -if let Ok (bar) = this.bar.access (pdev.as_ref ()) { -// Reset pci-testdev by writing a new test index. - bar.write_reg (regs::TEST ::zeroed ().with_index (this.index)); - } +fn unbind <'bound>(_pdev: &'bound pci::Device <Core <'_>>, this:Pin <&Self ::Data <'bound>>) { + this.bar + .write_reg (regs::TEST ::zeroed ().with_index (this.index)); } } -#[pinned_drop] -impl PinnedDrop for SampleDriver { -fn drop (self:Pin <&mut Self >) { +impl Drop for SampleDriverData <'_> { +fn drop (&mut self) {dev_dbg! (self.pdev,"Remove Rust PCI driver sample.\n" ); } }
Signature · 1 line
2.54.0
Re: [PATCH v5 20/24] samples: rust: rust_driver_pci: use HRT lifetime for Bar
Quoted 7 lines · Danilo Krummrich
Convert the sample driver to SampleDriver<'bound>, taking advantage of the lifetime-parameterized Driver trait.
The driver struct holds &'bound
pci::Devicedirectly instead of ARef<pci::Device>, and pci::Bar<'bound> directly instead of Devres<pci::Bar>. This removes PinnedDrop,pin_init_scope, and runtime revocation checks on BAR access.
It looks like the original code doesn't need pin_init to begin with, so it's not like the lifetime actually helps here, but it's still a good cleanup.
Quoted 8 lines
Reviewed-by Eliot Courtney <[email protected]>Reviewed-by Greg Kroah-Hartman <[email protected]>Reviewed-by Alexandre Courbot <[email protected]>Signed-off-by Danilo Krummrich <[email protected]>--- samples/rust/rust_driver_pci.rs | 89 +++++++++++++++------------------ 1 file changed, 40 insertions(+), 49 deletions(-)
Danilo Krummrich
maintainerPATCH v5 21/24 gpu: nova-core: separate driver type from driver data
In reply to Danilo KrummrichIntroduce NovaCoreDriver as the driver type implementing pci::Driver, keeping NovaCore as the per-device data type. This prepares for making NovaCore lifetime-parameterized once auxiliary::Registration requires a lifetime for the binding scope.
--- drivers/gpu/nova-core/driver.rs | 14 ++++++++------ drivers/gpu/nova-core/nova_core.rs | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-)
drivers/gpu/nova-core/driver.rs +8 -6
@@ -35,6 +35,8 @@ pub(crate) struct NovaCore { _reg: Devres<auxiliary::Registration<()>>, } +pub (crate )struct NovaCoreDriver ; +const BAR0_SIZE :usize =SZ_16M ;// For now we only support Ampere which can use up to 47-bit DMA addresses.
@@ -50,7 +52,7 @@ pub(crate) struct NovaCore { kernel::pci_device_table!( PCI_TABLE, MODULE_PCI_TABLE, - <NovaCore as pci::Driver>::IdInfo, + <NovaCoreDriver as pci::Driver>::IdInfo, [ // Modern NVIDIA GPUs will show up as either VGA or 3D controllers. (
@@ -72,15 +74,15 @@ pub(crate) struct NovaCore { ] ); -impl pci::Driver for NovaCore { +impl pci::Driver for NovaCoreDriver {type IdInfo = (); -type Data <'bound> =Self ; +type Data <'bound> =NovaCore ;const ID_TABLE : pci::IdTable <Self ::IdInfo > = &PCI_TABLE ;fn probe <'bound>( pdev: &'bound pci::Device <Core <'_>>, _info: &'boundSelf ::IdInfo , - ) ->impl PinInit <Self ,Error > + 'bound { + ) ->impl PinInit <NovaCore ,Error > + 'bound { pin_init::pin_init_scope (move || {dev_dbg! (pdev,"Probe Nova Core GPU driver.\n" );
@@ -98,7 +100,7 @@ fn probe<'bound>( GFP_KERNEL, )?; - Ok(try_pin_init!(Self { + Ok(try_pin_init!(NovaCore { gpu <- Gpu::new(pdev, bar.clone(), bar.access(pdev.as_ref())?), _reg: auxiliary::Registration::new( pdev.as_ref(),
@@ -113,7 +115,7 @@ fn probe<'bound>( }) } -fn unbind <'bound>(pdev: &'bound pci::Device <Core <'_>>, this:Pin <&Self >) { +fn unbind <'bound>(pdev: &'bound pci::Device <Core <'_>>, this:Pin <&NovaCore >) { this.gpu.unbind (pdev.as_ref ()); } }
drivers/gpu/nova-core/nova_core.rs +1 -1
@@ -47,7 +47,7 @@ struct NovaCoreModule {// Fields are dropped in declaration order, so `_driver` is dropped first, // then `_debugfs_guard` clears `DEBUGFS_ROOT`. #[pin] - _driver: Registration<pci::Adapter<driver::NovaCore>>, + _driver: Registration<pci::Adapter<driver::NovaCoreDriver>>, _debugfs_guard: DebugfsRootGuard, }
Signature · 1 line
2.54.0
Re: [PATCH v5 21/24] gpu: nova-core: separate driver type from driver data
Quoted 9 lines · Danilo Krummrich
Introduce NovaCoreDriver as the driver type implementing
pci::Driver, keeping NovaCore as the per-device data type. This prepares for making NovaCore lifetime-parameterized onceauxiliary::Registrationrequires a lifetime for the binding scope.Reviewed-by Greg Kroah-Hartman <[email protected]>Reviewed-by Alexandre Courbot <[email protected]>Reviewed-by Eliot Courtney <[email protected]>Signed-off-by Danilo Krummrich <[email protected]>
Quoted 4 lines
--- drivers/gpu/nova-core/driver.rs | 14 ++++++++------ drivers/gpu/nova-core/nova_core.rs | 2 +- 2 files changed, 9 insertions(+), 7 deletions(-)
Danilo Krummrich
maintainerPATCH v5 22/24 rust: types: add `ForLt` trait for higher-ranked lifetime support
In reply to Danilo KrummrichFrom: Gary Guo <[email protected]>
There are a few cases, e.g. when dealing with data referencing each other, one might want to write code that is generic over lifetimes. For example, if you want to take a function that takes &'a Foo and gives Bar<'a>, you can write:
f: impl for<'a> FnOnce(&'a Foo) -> Bar<'a>,
However, it becomes tricky when you want that function to not have a fixed Bar, but have it be generic again. In this case, one needs something that is generic over types that are themselves generic over lifetimes.
ForLt provides such support. It provides a trait ForLt which describes a type generic over a lifetime. One may use ForLt::Of<'a> to get an instance of a type for a specific lifetime.
For the case of cross referencing, one would almost always want the lifetime to be covariant. Therefore this is also made a requirement for the ForLt trait, so functions with ForLt trait bound can assume covariance.
A macro ForLt!() is provided to be able to obtain a type that implements ForLt. For example, ForLt!(for<'a> Bar<'a>) would yield a type that <TheType as ForLt>::Of<'a> is Bar<'a>. This also works with lifetime elision, e.g. ForLt!(Bar<'_>) or for types without lifetime at all, e.g. ForLt!(u32).
The API design draws inspiration from the higher-kinded-types [1] crate, however a different design decision has been taken (e.g. covariance requirement) and the implementation is independent.
License headers use "Apache-2.0 OR MIT" because I anticipate this to be used in pin-init crate too which is licensed as such.
[ Handle macro_rules! invocations in the ForLt! proc macro's covariance
and WF checks. Since proc macros cannot expand macro_rules!, add a visit_macro() implementation to conservatively assume macro invocations may contain lifetimes, forcing them through the compiler-assisted covariance proof.
While at it, fix a few typos in the documentation and in the commit message. - Danilo ]
--- rust/Makefile | 1 + rust/kernel/types.rs | 4 + rust/kernel/types/for_lt.rs | 117 +++++++++++++++++ rust/macros/for_lt.rs | 248 ++++++++++++++++++++++++++++++++++++ rust/macros/lib.rs | 12 ++ 5 files changed, 382 insertions(+) create mode 100644 rust/kernel/types/for_lt.rs create mode 100644 rust/macros/for_lt.rs
rust/Makefile +1 -0
@@ -110,6 +110,7 @@ syn-cfgs := \ feature="parsing" \ feature="printing" \ feature="proc-macro" \ + feature="visit" \ feature="visit-mut" syn-flags := \
rust/kernel/types.rs +4 -0
@@ -11,6 +11,10 @@ };use pin_init::{PinInit ,Wrapper ,Zeroable }; +#[doc(hidden)] +pub mod for_lt; +pub use for_lt::ForLt ; +/// Used to transfer ownership to and from foreign (non-Rust) languages. /// /// Ownership is transferred from Rust to a foreign language by calling [`Self::into_foreign`] and
rust/kernel/types/for_lt.rs new file +117 -0
117 lines not on this page · raw message
rust/macros/for_lt.rs new file +248 -0
248 lines not on this page · raw message
rust/macros/lib.rs +12 -0
@@ -17,6 +17,7 @@mod concat_idents;mod export;mod fmt; +mod for_lt;mod helpers;mod kunit;mod module;
@@ -489,3 +490,14 @@ pub fn kunit_tests(attr: TokenStream, input: TokenStream) -> TokenStream { .unwrap_or_else(|e| e.into_compile_error()) .into () } + +/// Obtain a type that implements `ForLt` for the given higher-ranked type. +/// +/// Please refer to the documentation of [`ForLt`] trait. +/// +/// [`ForLt`]: trait.ForLt.html +#[proc_macro] +#[allow(non_snake_case)] // The macro shares the name with the trait. +pub fn ForLt (input:TokenStream ) ->TokenStream { + for_lt::for_lt (parse_macro_input! (input)).into () +}
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 22/24] rust: types: add `ForLt` trait for higher-ranked lifetime support
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider: - [High] Safe code can bypass the `ForLt!` macro's covariance checks and cause Undefined Behavior.…
Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Safe code can bypass the ForLt! macro's covariance checks and cause Undefined Behavior.
- [Medium] ForLt! macro fails to replace lifetimes inside macro invocations, causing E0261 undeclared lifetime errors.
- [Low] Incorrect use of #[inline(always)] on cast_ref.
- [Low] The well-formedness workaround #ty_static strictly bounds captured generic types to 'static.
- [Low] The rustdoc compile_fail test is ignored.
--
commit 7d554c95fb79799acb08213b598bd1324b690e69
Author: Gary Guo <[email protected]>
rust: types: add ForLt trait for higher-ranked lifetime support
This commit introduces the ForLt trait and ForLt! macro to describe types generic over a covariant lifetime. The macro includes logic to prove covariance and ensure well-formedness.
Quoted 4 lines
rust/kernel/types/for_lt.rs new file +0
-0
[ ... ]
rust/kernel/types/for_lt.rs line 59
Quoted 10 lines
rust/kernel/types/for_lt.rs new file +10
-0+/// The macro will attempt to prove that the type is indeed covariant over the lifetime supplied. +/// When it cannot be syntactically proven, it will emit checks to ask the Rust compiler to prove +/// it. +/// ```ignore,compile_fail +/// # use kernel::types::ForLt; +/// # fn expect_lt<F: ForLt>() {} +/// # expect_lt::< +/// ForLt!(fn(&u32)) // Contravariant, will fail compilation. +/// # >(); +/// ```
[Severity: Low]
Is this test intentionally skipped? The rustdoc 'ignore' attribute supersedes 'compile_fail', meaning the test is skipped entirely rather than asserted for compilation failure. Can this be changed to just 'compile_fail' so the covariance check is mechanically verified?
[ ... ]
rust/kernel/types/for_lt.rs line 91
Quoted 11 lines
rust/kernel/types/for_lt.rs new file +11
-0+pub unsafe trait ForLt { +/// The type parameterized by the lifetime. +type Of <'a>: 'a; + +/// Cast a reference to a shorter lifetime. +#[inline(always)] +fn cast_ref <'r, 'short: 'r, 'long: 'short>(long: &'rSelf ::Of <'long>) -> &'rSelf ::Of <'short> { +// SAFETY: This is sound as this trait guarantees covariance. +unsafe { core::mem::transmute (long) } + } +}
[Severity: Low]
This isn't a bug, but does this violate the inline annotation guidelines? According to the rust subsystem rules, #[inline(always)] should only be used for functions utilizing build_assert!() that depend on function parameters. For small forwarding functions in abstractions, #[inline] should be used instead.
[ ... ]
Quoted 14 lines
rust/kernel/types/for_lt.rs new file +14
-0+/// This is intended to be an "unsafe-to-refer-to" type. +/// +/// Must only be used by the `ForLt!` macro. +/// +/// `T` is the magic `dyn for<'a> WithLt<'a, TypeThatUse<'a>>` generated by macro. +/// +/// `WF` is a type that the macro can use to assert some specific type is well-formed. +/// +/// `N` is to provide the macro a place to emit arbitrary items, in case it needs to prove +/// additional properties. +#[doc(hidden)] +pub struct UnsafeForLtImpl <T : ?Sized ,WF ,const N :usize >(PhantomData <(WF ,T )>); + +// This is a helper trait for implementation `ForLt` to be able to use HRTB.
rust/kernel/types/for_lt.rs line 117
Quoted 10 lines
rust/kernel/types/for_lt.rs new file +10
-0+#[doc(hidden)] +pub trait WithLt <'a> { +type Of : 'a; +} + +// SAFETY: In `ForLt!` macro, a covariance proof is generated when naming `UnsafeForLtImpl` +// and it will fail to evaluate if the type is not covariant. +unsafe impl <T : ?Sized +for <'a>WithLt <'a>,WF >ForLt for UnsafeForLtImpl <T ,WF ,0 > { +type Of <'a> = <T as WithLt <'a>>::Of ; +}
[Severity: High]
Can safe code bypass the covariance checks and cause undefined behavior here? Since UnsafeForLtImpl and WithLt are public, safe code can interact with them directly without going through the ForLt! macro.
If a user explicitly names a trait object type like:
UnsafeForLtImpl<dyn for<'a> WithLt<'a, Of = fn(&'a i32)>, (), 0>
The blanket unsafe impl ForLt accepts this contravariant type. Safe code could then call the safe cast_ref method on this type, which performs an unsafe transmute assuming covariance. This would allow converting a long-lived reference requirement into a short-lived one.
Does this need additional protection to ensure safe code cannot instantiate it directly?
Quoted 5 lines
rust/macros/for_lt.rs new file +0
-0
[ ... ]
Quoted 5 lines
rust/macros/for_lt.rs new file +5
-0+fn replace_lifetime (&self, src: &Lifetime , dst: &Lifetime ) ->Type { +struct LifetimeReplacer <'a>(&'aLifetime , &'aLifetime ); + +impl VisitMut for LifetimeReplacer <'_> { +fn visit_lifetime_mut (&mut self, lifetime: &mut Lifetime ) {
rust/macros/for_lt.rs line 99
Quoted 10 lines
rust/macros/for_lt.rs new file +10
-0+if lifetime.ident == self.0 .ident { + *lifetime = self.1 .clone (); + } + } + } + +let mut ret = self.clone (); +LifetimeReplacer (src, dst).visit_type_mut (&mut ret); + ret + }
[Severity: Medium]
Does this miss replacing lifetimes inside macro invocations? The has_lifetime() method was updated with a visit_macro implementation to correctly identify lifetimes inside macros, but replace_lifetime() uses VisitMut which does not traverse opaque TokenStreams in macros.
If explicit lifetimes inside a macro invocation are not replaced with 'static, will the generated type retain the original lifetime binder but be emitted outside the scope, causing an E0261 undeclared lifetime error?
[ ... ]
rust/macros/for_lt.rs line 235
Quoted 12 lines
rust/macros/for_lt.rs new file +12
-0+// Make sure that the type is wellformed when substituting lifetime with `'static`. +// +// Currently the Rust compiler doesn't check this, see the above ProveWf documentation. +// +// We prefer to use this way of proving WF-ness as it can work when generics are involved. +let ty_static = ty.replace_lifetime ( + &lifetime, + &Lifetime { + apostrophe:Span ::mixed_site (), + ident:format_ident! ("static" ), + }, + );
[Severity: Low]
Does this well-formedness workaround strictly bound captured generic types to 'static? By substituting the lifetime with 'static to generate the type here, any generic type parameters inside the macro (like ForLt!(&T)) will be forced to satisfy a 'static bound. Does this limit the abstraction and prevent it from being used with generic parameters bounded by shorter lifetimes?
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=22
Re: [PATCH v5 22/24] rust: types: add `ForLt` trait for higher-ranked lifetime support
In reply to Danilo KrummrichQuoted 47 lines · Danilo Krummrich
From: Gary Guo <[email protected]>
There are a few cases, e.g. when dealing with data referencing each other, one might want to write code that is generic over lifetimes. For example, if you want to take a function that takes
&'a Fooand givesBar<'a>, you can write:f: impl for<'a> FnOnce(&'a Foo) -> Bar<'a>,However, it becomes tricky when you want that function to not have a fixed
Bar, but have it be generic again. In this case, one needs something that is generic over types that are themselves generic over lifetimes.
ForLtprovides such support. It provides a traitForLtwhich describes a type generic over a lifetime. One may useForLt::Of<'a>to get an instance of a type for a specific lifetime.For the case of cross referencing, one would almost always want the lifetime to be covariant. Therefore this is also made a requirement for the
ForLttrait, so functions withForLttrait bound can assume covariance.A macro
ForLt!()is provided to be able to obtain a type that implementsForLt. For example,ForLt!(for<'a> Bar<'a>)would yield a type that<TheType as ForLt>::Of<'a>isBar<'a>. This also works with lifetime elision, e.g.ForLt!(Bar<'_>)or for types without lifetime at all, e.g.ForLt!(u32).The API design draws inspiration from the higher-kinded-types [1] crate, however a different design decision has been taken (e.g. covariance requirement) and the implementation is independent.
License headers use "Apache-2.0 OR MIT" because I anticipate this to be used in pin-init crate too which is licensed as such.
Link https://docs.rs/higher-kinded-types/ [1]Reviewed-by Greg Kroah-Hartman <[email protected]>Reviewed-by Eliot Courtney <[email protected]>Signed-off-by Gary Guo <[email protected]>[ Handle
macro_rules! invocations in the ForLt! proc macro's covariance
and WF checks. Since proc macros cannot expandmacro_rules!, add avisit_macro()implementation to conservatively assume macro invocations may contain lifetimes, forcing them through the compiler-assisted covariance proof.
Quoted 3 lines
While at it, fix a few typos in the documentation and in the commit message. - Danilo ]
Signed-off-by Danilo Krummrich <[email protected]>
For everything but the proc macro,
Re: [PATCH v5 22/24] rust: types: add `ForLt` trait for higher-ranked lifetime support
In reply to Danilo Krummrichrust/kernel/types/for_lt.rs line 12
Quoted 5 lines · Danilo Krummrich
rust/kernel/types/for_lt.rs new file +4
-0+/// Representation of types generic over a lifetime. +/// +/// The type must be covariant over the generic lifetime, i.e. the lifetime parameter +/// can be soundly shortened.
I wonder if we could keep some of the commit message around here -- it was a well-written one!
Review
Quoted 2 lines
(quoted diff) +2
-0+/// The full syntax is +/// ```
Nit: we are not fully consistent treewide about this, but please add a newline before (and after) examples/code blocks.
Quoted 1 line
+/// ForLt!(TypeThatUse<'_>) // Equivalent to
ForLt!(for<'a> TypeThatUse<'a>)
Nit: please end comments with a period (most in the patch are already OK).
Quoted 1 line
Linked to:
https://github.com/Rust-for-Linux/linux/issues/355
and tagged the issue with our label upstream.
Quoted 1 line
+ // Currently the Rust compiler doesn't check this, see the above ProveWf documentation.
Nit: ProveWf
Quoted 1 line
+/// Obtain a type that implements
ForLtfor the given higher-ranked type.
Nit: [ForLt] like below.
Quoted 1 line
+/// Please refer to the documentation of [
ForLt] trait.
"of the"?
Quoted 1 line
+#[allow(non_snake_case)] // The macro shares the name with the trait.
We normally put comments on top of attributes, but if somehow that messes up the formatting or similar, please ignore it.
Thanks!
Cheers,
Miguel
Danilo Krummrich
maintainerPATCH v5 23/24 rust: auxiliary: generalize Registration over ForLt
In reply to Danilo KrummrichGeneralize Registration<T> to Registration<F: ForLt> and Device::registration_data<F: ForLt>() to return Pin<&F::Of<'_>>.
The stored 'static lifetime is shortened to the borrow lifetime of &self via ForLt::cast_ref; ForLt's covariance guarantee makes this sound.
--- drivers/gpu/nova-core/driver.rs | 13 ++-- rust/kernel/auxiliary.rs | 108 +++++++++++++++++++------- samples/rust/rust_driver_auxiliary.rs | 19 +++-- 3 files changed, 96 insertions(+), 44 deletions(-)
drivers/gpu/nova-core/driver.rs +7 -6
@@ -3,7 +3,6 @@use kernel::{ auxiliary, device::Core , - devres::Devres , dma::Device , dma::DmaMask , pci,
@@ -21,6 +20,7 @@ }, Arc, }, + types::ForLt, };use crate ::gpu::Gpu ;
@@ -29,10 +29,11 @@static AUXILIARY_ID_COUNTER :Atomic <u32 > =Atomic ::new (0 );#[pin_data] -pub (crate )struct NovaCore { +pub (crate )struct NovaCore <'bound> {#[pin] pub (crate ) gpu:Gpu , - _reg:Devres <auxiliary::Registration <()>>, +#[allow(clippy::type_complexity)] + _reg: auxiliary::Registration <'bound,ForLt! (())>, }pub (crate )struct NovaCoreDriver ;
@@ -76,13 +77,13 @@ pub(crate) struct NovaCore {impl pci::Driver for NovaCoreDriver {type IdInfo = (); -type Data <'bound> =NovaCore ; +type Data <'bound> =NovaCore <'bound>;const ID_TABLE : pci::IdTable <Self ::IdInfo > = &PCI_TABLE ;fn probe <'bound>( pdev: &'bound pci::Device <Core <'_>>, _info: &'boundSelf ::IdInfo , - ) ->impl PinInit <NovaCore ,Error > + 'bound { + ) ->impl PinInit <Self ::Data <'bound>,Error > + 'bound { pin_init::pin_init_scope (move || {dev_dbg! (pdev,"Probe Nova Core GPU driver.\n" );
@@ -115,7 +116,7 @@ fn probe<'bound>( }) } -fn unbind <'bound>(pdev: &'bound pci::Device <Core <'_>>, this:Pin <&NovaCore >) { +fn unbind <'bound>(pdev: &'bound pci::Device <Core <'_>>, this:Pin <&Self ::Data <'bound>>) { this.gpu.unbind (pdev.as_ref ()); } }
rust/kernel/auxiliary.rs +78 -30
203 lines not on this page · raw message
samples/rust/rust_driver_auxiliary.rs +11 -8
@@ -10,10 +10,10 @@ Bound, Core, // }, - devres::Devres, driver, pci, prelude::*, + types::ForLt, InPlaceModule, // };
@@ -55,9 +55,12 @@ struct Data { index: u32, } -struct ParentDriver { - _reg0:Devres <auxiliary::Registration <Data >>, - _reg1:Devres <auxiliary::Registration <Data >>, +struct ParentDriver ; + +#[allow(clippy::type_complexity)] +struct ParentData <'bound> { + _reg0: auxiliary::Registration <'bound,ForLt! (Data )>, + _reg1: auxiliary::Registration <'bound,ForLt! (Data )>, } kernel::pci_device_table!(
@@ -69,15 +72,15 @@ struct ParentDriver { impl pci::Driver for ParentDriver { type IdInfo = (); - type Data<'bound> = Self; + type Data<'bound> = ParentData<'bound>; const ID_TABLE: pci::IdTable<Self::IdInfo> = &PCI_TABLE; fn probe<'bound>( pdev: &'bound pci::Device<Core<'_>>, _info: &'bound Self::IdInfo, - ) -> impl PinInit<Self, Error> + 'bound { - Ok(Self { + ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound { + Ok(ParentData { _reg0: auxiliary::Registration::new( pdev.as_ref(), AUXILIARY_NAME,
@@ -101,7 +104,7 @@ fn connect(adev: &auxiliary::Device<Bound>) -> Result {let dev = adev.parent ();let pdev: &pci::Device <Bound > = dev.try_into ()?; -let data = adev.registration_data ::<Data >()?; +let data = adev.registration_data ::<ForLt! (Data )>()?;dev_info! ( dev,
Signature · 1 line
2.54.0
Gary Guo
reviewerRe: [PATCH v5 23/24] rust: auxiliary: generalize Registration over ForLt
Quoted 128 lines · Danilo Krummrich
Generalize Registration<T> to Registration<F: ForLt> and Device::registration_data<F: ForLt>() to return Pin<&F::Of<'_>>.
The stored 'static lifetime is shortened to the borrow lifetime of &self via
ForLt::cast_ref; ForLt's covariance guarantee makes this sound.Reviewed-by Greg Kroah-Hartman <[email protected]>Reviewed-by Alexandre Courbot <[email protected]>Reviewed-by Eliot Courtney <[email protected]>Signed-off-by Danilo Krummrich <[email protected]>--- drivers/gpu/nova-core/driver.rs | 13 ++-- rust/kernel/auxiliary.rs | 108 +++++++++++++++++++------- samples/rust/rust_driver_auxiliary.rs | 19 +++-- 3 files changed, 96 insertions(+), 44 deletions(-)drivers/gpu/nova-core/driver.rs +7
-6@@ -3,7 +3,6 @@use kernel::{ auxiliary, device::Core , - devres::Devres , dma::Device , dma::DmaMask , pci,@@ -21,6 +20,7 @@ }, Arc, }, + types::ForLt, };use crate ::gpu::Gpu ;@@ -29,10 +29,11 @@static AUXILIARY_ID_COUNTER :Atomic <u32 > =Atomic ::new (0 );#[pin_data] -pub (crate )struct NovaCore { +pub (crate )struct NovaCore <'bound> {#[pin] pub (crate ) gpu:Gpu , - _reg:Devres <auxiliary::Registration <()>>, +#[allow(clippy::type_complexity)] + _reg: auxiliary::Registration <'bound,ForLt! (())>, }pub (crate )struct NovaCoreDriver ;@@ -76,13 +77,13 @@ pub(crate) struct NovaCore {impl pci::Driver for NovaCoreDriver {type IdInfo = (); -type Data <'bound> =NovaCore ; +type Data <'bound> =NovaCore <'bound>;const ID_TABLE : pci::IdTable <Self ::IdInfo > = &PCI_TABLE ;fn probe <'bound>( pdev: &'bound pci::Device <Core <'_>>, _info: &'boundSelf ::IdInfo , - ) ->impl PinInit <NovaCore ,Error > + 'bound { + ) ->impl PinInit <Self ::Data <'bound>,Error > + 'bound { pin_init::pin_init_scope (move || {dev_dbg! (pdev,"Probe Nova Core GPU driver.\n" );@@ -115,7 +116,7 @@ fn probe<'bound>( }) } -fn unbind <'bound>(pdev: &'bound pci::Device <Core <'_>>, this:Pin <&NovaCore >) { +fn unbind <'bound>(pdev: &'bound pci::Device <Core <'_>>, this:Pin <&Self ::Data <'bound>>) { this.gpu.unbind (pdev.as_ref ()); } }rust/kernel/auxiliary.rs +10
-6@@ -12,7 +12,7 @@ RawDeviceId, RawDeviceIdIndex, // }, - devres::Devres, + driver, error::{ from_result,@@ -20,6 +20,7 @@ }, prelude::*, types::{ +ForLt ,ForeignOwnable ,Opaque ,// },@@ -271,12 +272,16 @@ pub fn parent(&self) -> &device::Device<device::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 +/// `F` is the [`ForLt`](trait@ForLt) encoding of the data type. The returned +/// reference has its lifetime shortened from `'static` to `&self`'s borrow lifetime via +/// [`ForLt::cast_ref`]. +/// +/// Returns [`EINVAL`] if `F` 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 >> { +pub fn registration_data <F :ForLt + 'static>(&self) ->Result <Pin <&F ::Of <'_>>> {// 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 () {@@ -289,18 +294,23 @@ pub fn registration_data<T: 'static>(&self) -> Result<Pin<&T>> {// 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`. +// at the start of the allocation is valid regardless of `F`. let type_id =unsafe { ptr.cast ::<TypeId >().read () }; -if type_id !=TypeId ::of ::<T >() { +if type_id !=TypeId ::of ::<F >() {return Err (EINVAL ); } -// SAFETY: The `TypeId` check above confirms that the stored type is `T`; `ptr` remains
rust/kernel/auxiliary.rs pub fn registration_data<T: 'static>(&self) -> Result<Pin<&T>> { · line 309
Quoted 10 lines
rust/kernel/auxiliary.rs +5
-3-// valid until `Registration::drop()` calls `from_foreign()`. -let wrapper =unsafe {Pin ::<KBox <RegistrationData <T >>>::borrow (ptr) }; +// SAFETY: The `TypeId` check above confirms that the stored type matches +// `F::Of<'static>`; `ptr` remains valid until `Registration::drop()` calls +// `from_foreign()`. +let wrapper =unsafe {Pin ::<KBox <RegistrationData <F ::Of <'static>>>>::borrow (ptr) };// SAFETY: `data` is a structurally pinned field of `RegistrationData`. -Ok (unsafe { wrapper.map_unchecked (|w| &w.data) }) +let pinned:Pin <&F ::Of <'static>> =unsafe { wrapper.map_unchecked (|w| &w.data) };
I think '_ is sufficient here?
Quoted 19 lines
rust/kernel/auxiliary.rs +7
-3+ +// SAFETY: The data was pinned when stored; `cast_ref` only shortens +// the lifetime, so the pinning guarantee is preserved. +Ok (unsafe {Pin ::new_unchecked (F ::cast_ref (pinned.get_ref ())) }) } }@@ -389,43 +399,61 @@ struct RegistrationData<T> {/// 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()`]. +/// The type parameter `F` is a [`ForLt`](trait@ForLt) encoding of the registration +/// data type. For non-lifetime-parameterized types, use [`ForLt!(T)`](macro@ForLt). +/// The data can be accessed by the auxiliary driver through [`Device::registration_data()`]. /// /// # Invariants
rust/kernel/auxiliary.rs struct RegistrationData<T> { · line 413
Quoted 10 lines
rust/kernel/auxiliary.rs +3
-3/// /// `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> { +/// valid `Pin<KBox<RegistrationData<F::Of<'static>>>>`. +pub struct Registration <'a,F :ForLt + 'static> { adev:NonNull <bindings::auxiliary_device >, - _data:PhantomData <T >, + #[allow(clippy::type_complexity)]
Is this acutally needed?
rust/kernel/auxiliary.rs struct RegistrationData<T> { · line 414
Quoted 1 line
rust/kernel/auxiliary.rs +1
-0+ _phantom: PhantomData<(fn(&'a ()) -> &'a (), F)>,
Could you update this to PhantomData<F::Of<'a>> please? It matches better what's is actually being stored.
'a also does not actually need to be invariant here if F::Of is covariant (which currently ForLt guarantees). (Although, lifetimes will still be invariant as it's used in assoc type by changing to F::Of<'a>). Variance of 'a is not what matters here; the covariance of F::Of is load-bearing part for the registration_data method, and the !Leak requirement is the load-bearing part for UAF protection.
Feel free to add my R-b with the change.
Best,
Gary
Quoted 98 lines
rust/kernel/auxiliary.rs +48
-11} -impl <T :Send +Sync + 'static>Registration <T > { +impl <'a,F :ForLt >Registration <'a,F > +where +for <'b>F ::Of <'b>:Send +Sync , +{/// 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 >, +/// +/// # Safety +/// +/// The caller must not `mem::forget()` the returned [`Registration`] or otherwise prevent its +/// [`Drop`] implementation from running, since the registration data may contain borrowed +/// references that become invalid after `'a` ends. +/// +/// If the registration data is `'static`, use the safe [`Registration::new()`] instead. +pub unsafe fn new_with_lt <E >( + parent: &'a device::Device <device::Bound >, name: &CStr , id:u32 , modname: &CStr , - data:impl PinInit <T ,E >, - ) ->Result <Devres <Self >> + data:impl PinInit <F ::Of <'a>,E >, + ) ->Result <Self >where Error :From <E >, {let data =KBox ::pin_init ::<Error >(try_pin_init! (RegistrationData { - type_id:TypeId ::of ::<T >(), + type_id:TypeId ::of ::<F >(), data <- data, }),GFP_KERNEL , )?; +// SAFETY: `'a` is invariant (via `Registration`'s `PhantomData`). Lifetimes do not +// affect layout, so RegistrationData<F::Of<'a>> and RegistrationData<F::Of<'static>> +// have identical representation. +let data:Pin <KBox <RegistrationData <F ::Of <'static>>>> = +unsafe { core::mem::transmute (data) }; +let boxed:KBox <Opaque <bindings::auxiliary_device >> =KBox ::zeroed (GFP_KERNEL )?;let adev = boxed.get ();@@ -455,7 +483,9 @@ pub fn new<E>( if ret != 0 { // SAFETY: `registration_data` was set above via `into_foreign()`. drop(unsafe { -Pin ::<KBox <RegistrationData <T >>>::from_foreign ((*adev).registration_data_rust) + Pin::<KBox<RegistrationData<F::Of<'static>>>>::from_foreign( + (*adev).registration_data_rust, + )If it's un }); // SAFETY: `adev` is guaranteed to be a valid pointer to a@@ -467,18 +497,36 @@ pub fn new<E>(// INVARIANT: The device will remain registered until `auxiliary_device_delete()` is // called, which happens in `Self::drop()`. -let reg =Self { +Ok (Self {// SAFETY: `adev` is guaranteed to be non-null, since the `KBox` was allocated // successfully. adev:unsafe {NonNull ::new_unchecked (adev) }, - _data:PhantomData , - }; + _phantom:PhantomData , + }) + } -Devres ::new ::<core::convert::Infallible >(parent, reg) +/// Create and register a new auxiliary device with `'static` registration data. +/// +/// Safe variant of [`Registration::new_with_lt()`] for registration data that does not contain +/// borrowed references. +pub fn new <E >( + parent: &'a device::Device <device::Bound >, + name: &CStr , + id:u32 , + modname: &CStr , + data:impl PinInit <F ::Of <'a>,E >, + ) ->Result <Self > +where +F ::Of <'a>: 'static, +Error :From <E >, + { +// SAFETY: `F::Of<'a>: 'static` guarantees the data contains no borrowed references, +// so forgetting the `Registration` cannot cause use-after-free. +unsafe {Self ::new_with_lt (parent, name, id, modname, data) } } }
Danilo Krummrich
maintainerPATCH v5 24/24 samples: rust: rust_driver_auxiliary: showcase lifetime-bound registration data
In reply to Danilo KrummrichMake the Data struct lifetime-parameterized, storing a reference to the parent pci::Device<Bound>. This demonstrates that registration data can hold device resources tied to the parent driver's lifetime.
In connect(), retrieve the parent PCI device from the registration data rather than casting through adev.parent().
--- samples/rust/rust_driver_auxiliary.rs | 58 ++++++++++++++++----------- 1 file changed, 35 insertions(+), 23 deletions(-)
samples/rust/rust_driver_auxiliary.rs +35 -23
@@ -51,16 +51,17 @@ fn probe<'bound>( } } -struct Data { +struct Data <'bound> { index:u32 , + parent: &'bound pci::Device <Bound >, }struct ParentDriver ;#[allow(clippy::type_complexity)] struct ParentData <'bound> { - _reg0: auxiliary::Registration <'bound,ForLt! (Data )>, - _reg1: auxiliary::Registration <'bound,ForLt! (Data )>, + _reg0: auxiliary::Registration <'bound,ForLt! (Data <'_>)>, + _reg1: auxiliary::Registration <'bound,ForLt! (Data <'_>)>, } kernel::pci_device_table!(
@@ -81,33 +82,44 @@ fn probe<'bound>( _info: &'bound Self::IdInfo, ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound {Ok (ParentData { - _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 }, - )?, +// SAFETY: `ParentData` is the driver's private data, which is dropped when the +// device is unbound; i.e. `mem::forget()` is never called on it. + _reg0:unsafe { + auxiliary::Registration ::new_with_lt ( + pdev.as_ref (), +AUXILIARY_NAME , +0 , +MODULE_NAME , +Data { + index:0 , + parent: pdev, + }, + )? + }, +// SAFETY: See `_reg0` above. + _reg1:unsafe { + auxiliary::Registration ::new_with_lt ( + pdev.as_ref (), +AUXILIARY_NAME , +1 , +MODULE_NAME , +Data { + index:1 , + parent: pdev, + }, + )? + }, }) } }impl ParentDriver {fn connect (adev: &auxiliary::Device <Bound >) ->Result { -let dev = adev.parent (); -let pdev: &pci::Device <Bound > = dev.try_into ()?; - -let data = adev.registration_data ::<ForLt! (Data )>()?; +let data = adev.registration_data ::<ForLt! (Data <'_>)>()?; +let pdev = data.parent;dev_info! ( - dev, + pdev,"Connect auxiliary {} with parent: VendorID={}, DeviceID={:#x}\n" , adev.id (), pdev.vendor_id (),
@@ -115,7 +127,7 @@ fn connect(adev: &auxiliary::Device<Bound>) -> Result { );dev_info! ( - dev, + pdev,"Connected to auxiliary device with index {}.\n" , data.index );
Signature · 1 line
2.54.0
[email protected]
AI reviewRe: [PATCH v5 24/24] samples: rust: rust_driver_auxiliary: showcase lifetime-bound registration data
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] Struct field initialization causes a compilation error due to mismatched types (missing Deref…
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] Struct field initialization causes a compilation error due to mismatched types (missing Deref coercion). --
commit 7dde6b480c3cf8bc2ec8ee3a89c129ed25216a30
Author: Danilo Krummrich <[email protected]>
Subject: samples: rust: rust_driver_auxiliary: showcase lifetime-bound registration data
This commit makes the Data struct lifetime-parameterized by storing a reference to the parent pci::Device. It also updates the connect method to retrieve the parent PCI device directly from the registration data.
Quoted 4 lines
samples/rust/rust_driver_auxiliary.rs +0
-0
[ ... ]
Quoted 5 lines
samples/rust/rust_driver_auxiliary.rs +0
-1@@ -81,33 +82,44 @@ fn probe<'bound>( _info: &'bound Self::IdInfo, ) -> impl PinInit<Self::Data<'bound>, Error> + 'bound { Ok(ParentData { - _reg0: auxiliary::Registration::new(
[ ... ]
samples/rust/rust_driver_auxiliary.rs fn probe<'bound>( · line 95
Quoted 9 lines
samples/rust/rust_driver_auxiliary.rs +9
-0+ _reg0: unsafe { + auxiliary::Registration ::new_with_lt ( + pdev.as_ref (), +AUXILIARY_NAME , +0 , +MODULE_NAME , +Data { + index:0 , + parent: pdev,
[Severity: Low]
Will this compile cleanly without an explicit dereference?
In Rust, struct field initializations do not trigger implicit Deref coercions in the same way that function arguments do. Since the parent field expects a &pci::Device<Bound> but pdev is being provided directly, does this cause a mismatched types error during the build?
It might be necessary to explicitly dereference it here, perhaps using &**pdev or a similar construct, to ensure the types match. This also applies to the _reg1 initialization below.
Quoted 3 lines
samples/rust/rust_driver_auxiliary.rs +3
-0+ }, + )? + },
[ ... ]
Signature · 1 line
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=24
Quoted 1 line · Danilo Krummrich
[PATCH v5 00/24] rust: device: Higher-Ranked Lifetime Types for device drivers
Applied, thanks!
Branch: topic/dd-lifetimes
Tree: git://git.kernel.org/pub/scm/linux/kernel/git/driver-core/driver-core.git
[1/24] rust: pci: use 'static lifetime for PCI BAR resource names
commit: e566a9e17f37
[2/24] rust: alloc: remove `'static` bound on `ForeignOwnable`
commit: e9df918d61e0
[3/24] rust: driver: move 'static bounds to constructor
commit: c8a43666bade
[4/24] rust: driver: decouple driver private data from driver type
commit: 7fdffdda630e
[5/24] rust: driver core: drop drvdata before devres release
commit: be31fcf5af75
[6/24] rust: pci: implement Sync for Device<Bound>
commit: 0b9a29c3a4e2
[7/24] rust: platform: implement Sync for Device<Bound>
commit: a89111c00b68
[8/24] rust: auxiliary: implement Sync for Device<Bound>
commit: 5bbcefe8db74
[9/24] rust: usb: implement Sync for Device<Bound>
commit: 3bb1655192ae
[10/24] rust: device: implement Sync for Device<Bound>
commit: de12e48a1be3
[11/24] rust: device: make Core and CoreInternal lifetime-parameterized
commit: 24799831d631
[12/24] rust: pci: make Driver trait lifetime-parameterized
commit: 16c2b8fdab7c
[13/24] rust: platform: make Driver trait lifetime-parameterized
commit: 81fdc7881443
[14/24] rust: auxiliary: make Driver trait lifetime-parameterized
commit: 46f651d88662
[15/24] rust: usb: make Driver trait lifetime-parameterized
commit: a3f09f8e47c4
[16/24] rust: i2c: make Driver trait lifetime-parameterized
commit: 71e6b6a80b51
[17/24] rust: driver: update module documentation for GAT-based Data type
commit: d31a349a7fd8
[18/24] rust: pci: make Bar lifetime-parameterized
commit: 8ea0b6d5bef5
[19/24] rust: io: make IoMem and ExclusiveIoMem lifetime-parameterized
commit: 89f55d04c602
[20/24] samples: rust: rust_driver_pci: use HRT lifetime for Bar
commit: e397d405c4c6
[21/24] gpu: nova-core: separate driver type from driver data
commit: bb1cf43f2fa8[22/24] rust: types: add ForLt trait for higher-ranked lifetime support
commit: e189bdb687a5
[ Handle macro_rules! invocations in the ForLt! proc macro's covariance
and WF checks. Since proc macros cannot expand macro_rules!, add a visit_macro() implementation to conservatively assume macro invocations may contain lifetimes, forcing them through the compiler-assisted covariance proof.
Fix a few typos in the documentation and in the commit message, add empty lines before samples, add missing periods and consistently use markdown.
- Danilo ]
[23/24] rust: auxiliary: generalize Registration over ForLt
commit: 4555291ddae9
[ Use PhantomData<F::Of<'a>> instead of
PhantomData<(fn(&'a ()) -> &'a (), F)>], which also gets us rid of #[allow(clippy::type_complexity)]. - Danilo ]
[24/24] samples: rust: rust_driver_auxiliary: showcase lifetime-bound registration data
commit: d18f3646184f
The patches are in the topic/dd-lifetimes topic branch and will be merged into the corresponding target branch soon.