| Message ID | 20250414131934.28418-5-dakr@kernel.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Auxiliary bus Rust abstractions | expand |
On Mon, Apr 14, 2025 at 03:18:07PM +0200, Danilo Krummrich wrote: > Implement the `auxiliary::Registration` type, which provides an API to > create and register new auxiliary devices in the system. > > Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Danilo Krummrich <dakr@kernel.org> > --- > rust/kernel/auxiliary.rs | 88 +++++++++++++++++++++++++++++++++++++++- > 1 file changed, 87 insertions(+), 1 deletion(-) > > diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs > index 75423737032a..b40d663b42eb 100644 > --- a/rust/kernel/auxiliary.rs > +++ b/rust/kernel/auxiliary.rs > @@ -5,7 +5,7 @@ > //! C header: [`include/linux/auxiliary_bus.h`](srctree/include/linux/auxiliary_bus.h) > > use crate::{ > - bindings, device, > + bindings, container_of, device, > device_id::RawDeviceId, > driver, > error::{to_result, Result}, > @@ -230,6 +230,18 @@ pub fn parent(&self) -> Option<&device::Device> { > } > } > > +impl Device { > + extern "C" fn release(dev: *mut bindings::device) { > + // SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device` > + // embedded in `struct auxiliary_device`. > + let adev = unsafe { container_of!(dev, bindings::auxiliary_device, dev) }.cast_mut(); FYI, Tamir's patch makes this cast_mut() unnecessary. > + // SAFETY: `adev` points to the memory that has been allocated in `Registration::new`, via > + // `KBox::new(Opaque::<bindings::auxiliary_device>::zeroed(), GFP_KERNEL)`. > + let _ = unsafe { KBox::<Opaque<bindings::auxiliary_device>>::from_raw(adev.cast()) }; > + } > +} > + > // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic > // argument. > kernel::impl_device_context_deref!(unsafe { Device }); > @@ -272,3 +284,77 @@ 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 {} > + > +/// The registration of an auxiliary device. > +/// > +/// This type represents the registration of a [`struct auxiliary_device`]. When an instance of this > +/// type is dropped, its respective auxiliary device will be unregistered from the system. > +/// > +/// # Invariants > +/// > +/// `self.0` always holds a valid pointer to an initialized and registered > +/// [`struct auxiliary_device`]. > +pub struct Registration(NonNull<bindings::auxiliary_device>); > + > +impl Registration { > + /// Create and register a new auxiliary device. > + pub fn new(parent: &device::Device, name: &CStr, id: u32, modname: &CStr) -> Result<Self> { > + let boxed = KBox::new(Opaque::<bindings::auxiliary_device>::zeroed(), GFP_KERNEL)?; > + let adev = boxed.get(); > + > + // SAFETY: It's safe to set the fields of `struct auxiliary_device` on initialization. > + unsafe { > + (*adev).dev.parent = parent.as_raw(); > + (*adev).dev.release = Some(Device::release); > + (*adev).name = name.as_char_ptr(); > + (*adev).id = id; > + } > + > + // SAFETY: `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, > + // which has not been initialized yet. > + unsafe { bindings::auxiliary_device_init(adev) }; > + > + // Now that `adev` is initialized, leak the `Box`; the corresponding memory will be freed > + // by `Device::release` when the last reference to the `struct auxiliary_device` is dropped. > + let _ = KBox::into_raw(boxed); > + > + // SAFETY: > + // - `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, which has > + // been initialialized, > + // - `modname.as_char_ptr()` is a NULL terminated string. > + let ret = unsafe { bindings::__auxiliary_device_add(adev, modname.as_char_ptr()) }; > + if ret != 0 { > + // SAFETY: `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, > + // which has been initialialized. > + unsafe { bindings::auxiliary_device_uninit(adev) }; Does this error-path actually free the box? Alice > + return Err(Error::from_errno(ret)); > + } > + > + // SAFETY: `adev` is guaranteed to be non-null, since the `KBox` was allocated successfully. > + // > + // INVARIANT: The device will remain registered until `auxiliary_device_delete()` is called, > + // which happens in `Self::drop()`. > + Ok(Self(unsafe { NonNull::new_unchecked(adev) })) > + } > +} > + > +impl Drop for Registration { > + fn drop(&mut self) { > + // SAFETY: By the type invariant of `Self`, `self.0.as_ptr()` is a valid registered > + // `struct auxiliary_device`. > + unsafe { bindings::auxiliary_device_delete(self.0.as_ptr()) }; > + > + // This drops the reference we acquired through `auxiliary_device_init()`. > + // > + // SAFETY: By the type invariant of `Self`, `self.0.as_ptr()` is a valid registered > + // `struct auxiliary_device`. > + unsafe { bindings::auxiliary_device_uninit(self.0.as_ptr()) }; > + } > +} > + > +// SAFETY: A `Registration` of a `struct auxiliary_device` can be released from any thread. > +unsafe impl Send for Registration {} > + > +// SAFETY: `Registration` does not expose any methods or fields that need synchronization. > +unsafe impl Sync for Registration {} > -- > 2.49.0 >
On Tue, Apr 15, 2025 at 12:11:16PM +0000, Alice Ryhl wrote: > On Mon, Apr 14, 2025 at 03:18:07PM +0200, Danilo Krummrich wrote: > > +impl Registration { > > + /// Create and register a new auxiliary device. > > + pub fn new(parent: &device::Device, name: &CStr, id: u32, modname: &CStr) -> Result<Self> { > > + let boxed = KBox::new(Opaque::<bindings::auxiliary_device>::zeroed(), GFP_KERNEL)?; > > + let adev = boxed.get(); > > + > > + // SAFETY: It's safe to set the fields of `struct auxiliary_device` on initialization. > > + unsafe { > > + (*adev).dev.parent = parent.as_raw(); > > + (*adev).dev.release = Some(Device::release); > > + (*adev).name = name.as_char_ptr(); > > + (*adev).id = id; > > + } > > + > > + // SAFETY: `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, > > + // which has not been initialized yet. > > + unsafe { bindings::auxiliary_device_init(adev) }; > > + > > + // Now that `adev` is initialized, leak the `Box`; the corresponding memory will be freed > > + // by `Device::release` when the last reference to the `struct auxiliary_device` is dropped. > > + let _ = KBox::into_raw(boxed); > > + > > + // SAFETY: > > + // - `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, which has > > + // been initialialized, > > + // - `modname.as_char_ptr()` is a NULL terminated string. > > + let ret = unsafe { bindings::__auxiliary_device_add(adev, modname.as_char_ptr()) }; > > + if ret != 0 { > > + // SAFETY: `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, > > + // which has been initialialized. > > + unsafe { bindings::auxiliary_device_uninit(adev) }; > > Does this error-path actually free the box? Yes, auxiliary_device_uninit() calls put_device() on the underlying struct device, hence the release() callback is called at this point, which frees the Box.
On Tue, Apr 15, 2025 at 02:50:02PM +0200, Danilo Krummrich wrote: > On Tue, Apr 15, 2025 at 12:11:16PM +0000, Alice Ryhl wrote: > > On Mon, Apr 14, 2025 at 03:18:07PM +0200, Danilo Krummrich wrote: > > > +impl Registration { > > > + /// Create and register a new auxiliary device. > > > + pub fn new(parent: &device::Device, name: &CStr, id: u32, modname: &CStr) -> Result<Self> { > > > + let boxed = KBox::new(Opaque::<bindings::auxiliary_device>::zeroed(), GFP_KERNEL)?; > > > + let adev = boxed.get(); > > > + > > > + // SAFETY: It's safe to set the fields of `struct auxiliary_device` on initialization. > > > + unsafe { > > > + (*adev).dev.parent = parent.as_raw(); > > > + (*adev).dev.release = Some(Device::release); > > > + (*adev).name = name.as_char_ptr(); > > > + (*adev).id = id; > > > + } > > > + > > > + // SAFETY: `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, > > > + // which has not been initialized yet. > > > + unsafe { bindings::auxiliary_device_init(adev) }; > > > + > > > + // Now that `adev` is initialized, leak the `Box`; the corresponding memory will be freed > > > + // by `Device::release` when the last reference to the `struct auxiliary_device` is dropped. > > > + let _ = KBox::into_raw(boxed); > > > + > > > + // SAFETY: > > > + // - `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, which has > > > + // been initialialized, > > > + // - `modname.as_char_ptr()` is a NULL terminated string. > > > + let ret = unsafe { bindings::__auxiliary_device_add(adev, modname.as_char_ptr()) }; > > > + if ret != 0 { > > > + // SAFETY: `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, > > > + // which has been initialialized. > > > + unsafe { bindings::auxiliary_device_uninit(adev) }; > > > > Does this error-path actually free the box? > > Yes, auxiliary_device_uninit() calls put_device() on the underlying struct > device, hence the release() callback is called at this point, which frees the > Box. In that case: Reviewed-by: Alice Ryhl <aliceryhl@google.com>
diff --git a/rust/kernel/auxiliary.rs b/rust/kernel/auxiliary.rs index 75423737032a..b40d663b42eb 100644 --- a/rust/kernel/auxiliary.rs +++ b/rust/kernel/auxiliary.rs @@ -5,7 +5,7 @@ //! C header: [`include/linux/auxiliary_bus.h`](srctree/include/linux/auxiliary_bus.h) use crate::{ - bindings, device, + bindings, container_of, device, device_id::RawDeviceId, driver, error::{to_result, Result}, @@ -230,6 +230,18 @@ pub fn parent(&self) -> Option<&device::Device> { } } +impl Device { + extern "C" fn release(dev: *mut bindings::device) { + // SAFETY: By the type invariant `self.0.as_raw` is a pointer to the `struct device` + // embedded in `struct auxiliary_device`. + let adev = unsafe { container_of!(dev, bindings::auxiliary_device, dev) }.cast_mut(); + + // SAFETY: `adev` points to the memory that has been allocated in `Registration::new`, via + // `KBox::new(Opaque::<bindings::auxiliary_device>::zeroed(), GFP_KERNEL)`. + let _ = unsafe { KBox::<Opaque<bindings::auxiliary_device>>::from_raw(adev.cast()) }; + } +} + // SAFETY: `Device` is a transparent wrapper of a type that doesn't depend on `Device`'s generic // argument. kernel::impl_device_context_deref!(unsafe { Device }); @@ -272,3 +284,77 @@ 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 {} + +/// The registration of an auxiliary device. +/// +/// This type represents the registration of a [`struct auxiliary_device`]. When an instance of this +/// type is dropped, its respective auxiliary device will be unregistered from the system. +/// +/// # Invariants +/// +/// `self.0` always holds a valid pointer to an initialized and registered +/// [`struct auxiliary_device`]. +pub struct Registration(NonNull<bindings::auxiliary_device>); + +impl Registration { + /// Create and register a new auxiliary device. + pub fn new(parent: &device::Device, name: &CStr, id: u32, modname: &CStr) -> Result<Self> { + let boxed = KBox::new(Opaque::<bindings::auxiliary_device>::zeroed(), GFP_KERNEL)?; + let adev = boxed.get(); + + // SAFETY: It's safe to set the fields of `struct auxiliary_device` on initialization. + unsafe { + (*adev).dev.parent = parent.as_raw(); + (*adev).dev.release = Some(Device::release); + (*adev).name = name.as_char_ptr(); + (*adev).id = id; + } + + // SAFETY: `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, + // which has not been initialized yet. + unsafe { bindings::auxiliary_device_init(adev) }; + + // Now that `adev` is initialized, leak the `Box`; the corresponding memory will be freed + // by `Device::release` when the last reference to the `struct auxiliary_device` is dropped. + let _ = KBox::into_raw(boxed); + + // SAFETY: + // - `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, which has + // been initialialized, + // - `modname.as_char_ptr()` is a NULL terminated string. + let ret = unsafe { bindings::__auxiliary_device_add(adev, modname.as_char_ptr()) }; + if ret != 0 { + // SAFETY: `adev` is guaranteed to be a valid pointer to a `struct auxiliary_device`, + // which has been initialialized. + unsafe { bindings::auxiliary_device_uninit(adev) }; + + return Err(Error::from_errno(ret)); + } + + // SAFETY: `adev` is guaranteed to be non-null, since the `KBox` was allocated successfully. + // + // INVARIANT: The device will remain registered until `auxiliary_device_delete()` is called, + // which happens in `Self::drop()`. + Ok(Self(unsafe { NonNull::new_unchecked(adev) })) + } +} + +impl Drop for Registration { + fn drop(&mut self) { + // SAFETY: By the type invariant of `Self`, `self.0.as_ptr()` is a valid registered + // `struct auxiliary_device`. + unsafe { bindings::auxiliary_device_delete(self.0.as_ptr()) }; + + // This drops the reference we acquired through `auxiliary_device_init()`. + // + // SAFETY: By the type invariant of `Self`, `self.0.as_ptr()` is a valid registered + // `struct auxiliary_device`. + unsafe { bindings::auxiliary_device_uninit(self.0.as_ptr()) }; + } +} + +// SAFETY: A `Registration` of a `struct auxiliary_device` can be released from any thread. +unsafe impl Send for Registration {} + +// SAFETY: `Registration` does not expose any methods or fields that need synchronization. +unsafe impl Sync for Registration {}