Re: [RFC 1/2] rust: introduce abstractions for fwctl

Jason Gunthorpe
2025-10-30T16:22:12Z

On Thu, Oct 30, 2025 at 04:03:12PM +0000, Zhi Wang wrote:
> +impl<T: FwCtlOps> Registration<T> {
> + /// Allocate and register a new fwctl device under the given parent device.
> + pub fn new(parent: &device::Device) -> Result<Self> {
> + let ops = &FwCtlVTable::<T>::VTABLE as *const _ as *mut _;
> +
> + // SAFETY: `_fwctl_alloc_device()` allocates a new `fwctl_device`
> + // and initializes its embedded `struct device`.
> + let dev = unsafe {
> + bindings::_fwctl_alloc_device(
> + parent.as_raw(),
> + ops,
> + core::mem::size_of::<bindings::fwctl_device>(),
> + )
> + };
> +
> + let dev = NonNull::new(dev).ok_or(ENOMEM)?;
> +
> + // SAFETY: `fwctl_register()` expects a valid device from `_fwctl_alloc_device()`.
> + let ret = unsafe { bindings::fwctl_register(dev.as_ptr()) };

This is a Bound device, not just any device.

> + if ret != 0 {
> + // SAFETY: If registration fails, release the allocated fwctl_device().
> + unsafe {
> + bindings::put_device(core::ptr::addr_of_mut!((*dev.as_ptr()).dev));

?? Don't open code fwctl_put() - it should be called directly?

> + }
> + return Err(Error::from_errno(ret));
> + }
> +
> + Ok(Self {
> + fwctl_dev: dev,
> + _marker: PhantomData,
> + })
> + }
> +
> + fn as_raw(&self) -> *mut bindings::fwctl_device {
> + self.fwctl_dev.as_ptr()
> + }
> +}
> +
> +impl<T: FwCtlOps> Drop for Registration<T> {
> + fn drop(&mut self) {
> + // SAFETY: `fwctl_unregister()` expects a valid device from `_fwctl_alloc_device()`.

Incomplete safety statement, the device passed to fwctl_alloc_device must
still be bound prior to calling fwctl_unregister

> + unsafe {
> + bindings::fwctl_unregister(self.as_raw());
> + bindings::put_device(core::ptr::addr_of_mut!((*self.as_raw()).dev));

There for Drop can only do fwctl_put() since otherwise there is no way
to guarantee a Bound device.

unregister has to happen before remove() completes, Danilo had some
approach to this I think he told me?

Jason