On 21/07/2025 16:38, Alice Ryhl wrote: > When working with a bus device, many operations are only possible while > the device is still bound. The &Device<Bound> type represents a proof in > the type system that you are in a scope where the device is guaranteed > to still be bound. Since we deregister irq callbacks when unbinding a > device, if an irq callback is running, that implies that the device has > not yet been unbound. > > To allow drivers to take advantage of that, add an additional argument > to irq callbacks. > > Signed-off-by: Alice Ryhl <aliceryhl@xxxxxxxxxx> With https://lore.kernel.org/rust-for-linux/dd34e5f4-5027-4096-9f32-129c8a067d0a@xxxxxxxxxxxx/ let me add Tested-by: Dirk Behme <dirk.behme@xxxxxxxxxxxx> here as well. Thanks! Dirk > --- > This patch is a follow-up to Daniel's irq series [1] that adds a > &Device<Bound> argument to all irq callbacks. This allows you to use > operations that are only safe on a bound device inside an irq callback. > > The patch is otherwise based on top of driver-core-next. > > [1]: https://lore.kernel.org/r/20250715-topics-tyr-request_irq2-v7-0-d469c0f37c07@xxxxxxxxxxxxx > --- > rust/kernel/irq/request.rs | 88 ++++++++++++++++++++++++++-------------------- > 1 file changed, 49 insertions(+), 39 deletions(-) > > diff --git a/rust/kernel/irq/request.rs b/rust/kernel/irq/request.rs > index d070ddabd37e7806f76edefd5d2ad46524be620e..f99aff2dd479f5223c90f0d2694f57e6c864bdb5 100644 > --- a/rust/kernel/irq/request.rs > +++ b/rust/kernel/irq/request.rs > @@ -37,18 +37,18 @@ pub trait Handler: Sync { > /// All work that does not necessarily need to be executed from > /// interrupt context, should be deferred to a threaded handler. > /// See also [`ThreadedRegistration`]. > - fn handle(&self) -> IrqReturn; > + fn handle(&self, device: &Device<Bound>) -> IrqReturn; > } > > impl<T: ?Sized + Handler + Send> Handler for Arc<T> { > - fn handle(&self) -> IrqReturn { > - T::handle(self) > + fn handle(&self, device: &Device<Bound>) -> IrqReturn { > + T::handle(self, device) > } > } > > impl<T: ?Sized + Handler, A: Allocator> Handler for Box<T, A> { > - fn handle(&self) -> IrqReturn { > - T::handle(self) > + fn handle(&self, device: &Device<Bound>) -> IrqReturn { > + T::handle(self, device) > } > } > > @@ -134,7 +134,7 @@ pub fn irq(&self) -> u32 { > /// use core::sync::atomic::Ordering; > /// > /// use kernel::prelude::*; > -/// use kernel::device::Bound; > +/// use kernel::device::{Bound, Device}; > /// use kernel::irq::flags::Flags; > /// use kernel::irq::Registration; > /// use kernel::irq::IrqRequest; > @@ -156,7 +156,7 @@ pub fn irq(&self) -> u32 { > /// impl kernel::irq::request::Handler for Handler { > /// // This is executing in IRQ context in some CPU. Other CPUs can still > /// // try to access to data. > -/// fn handle(&self) -> IrqReturn { > +/// fn handle(&self, _dev: &Device<Bound>) -> IrqReturn { > /// self.0.fetch_add(1, Ordering::Relaxed); > /// > /// IrqReturn::Handled > @@ -182,8 +182,7 @@ pub fn irq(&self) -> u32 { > /// > /// # Invariants > /// > -/// * We own an irq handler using `&self.handler` as its private data. > -/// > +/// * We own an irq handler whose cookie is a pointer to `Self`. > #[pin_data] > pub struct Registration<T: Handler + 'static> { > #[pin] > @@ -211,8 +210,8 @@ pub fn new<'a>( > inner <- Devres::new( > request.dev, > try_pin_init!(RegistrationInner { > - // SAFETY: `this` is a valid pointer to the `Registration` instance > - cookie: unsafe { &raw mut (*this.as_ptr()).handler }.cast(), > + // INVARIANT: `this` is a valid pointer to the `Registration` instance > + cookie: this.as_ptr().cast::<c_void>(), > irq: { > // SAFETY: > // - The callbacks are valid for use with request_irq. > @@ -225,7 +224,7 @@ pub fn new<'a>( > Some(handle_irq_callback::<T>), > flags.into_inner(), > name.as_char_ptr(), > - (&raw mut (*this.as_ptr()).handler).cast(), > + this.as_ptr().cast::<c_void>(), > ) > })?; > request.irq > @@ -262,9 +261,13 @@ pub fn synchronize(&self, dev: &Device<Bound>) -> Result { > /// > /// This function should be only used as the callback in `request_irq`. > unsafe extern "C" fn handle_irq_callback<T: Handler>(_irq: i32, ptr: *mut c_void) -> c_uint { > - // SAFETY: `ptr` is a pointer to T set in `Registration::new` > - let handler = unsafe { &*(ptr as *const T) }; > - T::handle(handler) as c_uint > + // SAFETY: `ptr` is a pointer to `Registration<T>` set in `Registration::new` > + let registration = unsafe { &*(ptr as *const Registration<T>) }; > + // SAFETY: The irq callback is removed before the device is unbound, so the fact that the irq > + // callback is running implies that the device has not yet been unbound. > + let device = unsafe { registration.inner.device().as_bound() }; > + > + T::handle(®istration.handler, device) as c_uint > } > > /// The value that can be returned from `ThreadedHandler::handle_irq`. > @@ -288,32 +291,32 @@ pub trait ThreadedHandler: Sync { > /// limitations do apply. All work that does not necessarily need to be > /// executed from interrupt context, should be deferred to the threaded > /// handler, i.e. [`ThreadedHandler::handle_threaded`]. > - fn handle(&self) -> ThreadedIrqReturn; > + fn handle(&self, device: &Device<Bound>) -> ThreadedIrqReturn; > > /// The threaded IRQ handler. > /// > /// This is executed in process context. The kernel creates a dedicated > /// kthread for this purpose. > - fn handle_threaded(&self) -> IrqReturn; > + fn handle_threaded(&self, device: &Device<Bound>) -> IrqReturn; > } > > impl<T: ?Sized + ThreadedHandler + Send> ThreadedHandler for Arc<T> { > - fn handle(&self) -> ThreadedIrqReturn { > - T::handle(self) > + fn handle(&self, device: &Device<Bound>) -> ThreadedIrqReturn { > + T::handle(self, device) > } > > - fn handle_threaded(&self) -> IrqReturn { > - T::handle_threaded(self) > + fn handle_threaded(&self, device: &Device<Bound>) -> IrqReturn { > + T::handle_threaded(self, device) > } > } > > impl<T: ?Sized + ThreadedHandler, A: Allocator> ThreadedHandler for Box<T, A> { > - fn handle(&self) -> ThreadedIrqReturn { > - T::handle(self) > + fn handle(&self, device: &Device<Bound>) -> ThreadedIrqReturn { > + T::handle(self, device) > } > > - fn handle_threaded(&self) -> IrqReturn { > - T::handle_threaded(self) > + fn handle_threaded(&self, device: &Device<Bound>) -> IrqReturn { > + T::handle_threaded(self, device) > } > } > > @@ -334,7 +337,7 @@ fn handle_threaded(&self) -> IrqReturn { > /// use core::sync::atomic::Ordering; > /// > /// use kernel::prelude::*; > -/// use kernel::device::Bound; > +/// use kernel::device::{Bound, Device}; > /// use kernel::irq::flags::Flags; > /// use kernel::irq::ThreadedIrqReturn; > /// use kernel::irq::ThreadedRegistration; > @@ -356,7 +359,7 @@ fn handle_threaded(&self) -> IrqReturn { > /// impl kernel::irq::request::ThreadedHandler for Handler { > /// // This is executing in IRQ context in some CPU. Other CPUs can still > /// // try to access the data. > -/// fn handle(&self) -> ThreadedIrqReturn { > +/// fn handle(&self, _dev: &Device<Bound>) -> ThreadedIrqReturn { > /// self.0.fetch_add(1, Ordering::Relaxed); > /// // By returning `WakeThread`, we indicate to the system that the > /// // thread function should be called. Otherwise, return > @@ -366,7 +369,7 @@ fn handle_threaded(&self) -> IrqReturn { > /// > /// // This will run (in a separate kthread) if and only if `handle` > /// // returns `WakeThread`. > -/// fn handle_threaded(&self) -> IrqReturn { > +/// fn handle_threaded(&self, _dev: &Device<Bound>) -> IrqReturn { > /// self.0.fetch_add(1, Ordering::Relaxed); > /// IrqReturn::Handled > /// } > @@ -391,8 +394,7 @@ fn handle_threaded(&self) -> IrqReturn { > /// > /// # Invariants > /// > -/// * We own an irq handler using `&T` as its private data. > -/// > +/// * We own an irq handler whose cookie is a pointer to `Self`. > #[pin_data] > pub struct ThreadedRegistration<T: ThreadedHandler + 'static> { > #[pin] > @@ -420,8 +422,8 @@ pub fn new<'a>( > inner <- Devres::new( > request.dev, > try_pin_init!(RegistrationInner { > - // SAFETY: `this` is a valid pointer to the `ThreadedRegistration` instance. > - cookie: unsafe { &raw mut (*this.as_ptr()).handler }.cast(), > + // INVARIANT: `this` is a valid pointer to the `ThreadedRegistration` instance. > + cookie: this.as_ptr().cast::<c_void>(), > irq: { > // SAFETY: > // - The callbacks are valid for use with request_threaded_irq. > @@ -435,7 +437,7 @@ pub fn new<'a>( > Some(thread_fn_callback::<T>), > flags.into_inner() as usize, > name.as_char_ptr(), > - (&raw mut (*this.as_ptr()).handler).cast(), > + this.as_ptr().cast::<c_void>(), > ) > })?; > request.irq > @@ -475,16 +477,24 @@ pub fn synchronize(&self, dev: &Device<Bound>) -> Result { > _irq: i32, > ptr: *mut c_void, > ) -> c_uint { > - // SAFETY: `ptr` is a pointer to T set in `ThreadedRegistration::new` > - let handler = unsafe { &*(ptr as *const T) }; > - T::handle(handler) as c_uint > + // SAFETY: `ptr` is a pointer to `ThreadedRegistration<T>` set in `ThreadedRegistration::new` > + let registration = unsafe { &*(ptr as *const ThreadedRegistration<T>) }; > + // SAFETY: The irq callback is removed before the device is unbound, so the fact that the irq > + // callback is running implies that the device has not yet been unbound. > + let device = unsafe { registration.inner.device().as_bound() }; > + > + T::handle(®istration.handler, device) as c_uint > } > > /// # Safety > /// > /// This function should be only used as the callback in `request_threaded_irq`. > unsafe extern "C" fn thread_fn_callback<T: ThreadedHandler>(_irq: i32, ptr: *mut c_void) -> c_uint { > - // SAFETY: `ptr` is a pointer to T set in `ThreadedRegistration::new` > - let handler = unsafe { &*(ptr as *const T) }; > - T::handle_threaded(handler) as c_uint > + // SAFETY: `ptr` is a pointer to `ThreadedRegistration<T>` set in `ThreadedRegistration::new` > + let registration = unsafe { &*(ptr as *const ThreadedRegistration<T>) }; > + // SAFETY: The irq callback is removed before the device is unbound, so the fact that the irq > + // callback is running implies that the device has not yet been unbound. > + let device = unsafe { registration.inner.device().as_bound() }; > + > + T::handle_threaded(®istration.handler, device) as c_uint > } > > --- > base-commit: d860d29e91be18de62b0f441edee7d00f6cb4972 > change-id: 20250721-irq-bound-device-c9fdbfdd8cd9 > > Best regards,
