Re: [PATCH v8 1/1] rust: introduce abstractions for fwctl
"Gary Guo" <[email protected]>
| Newsgroups | org.kernel.vger.rust-for-linux,org.kernel.vger.linux-kernel |
|---|---|
| Message-ID | <[email protected]> |
On Thu Aug 13, 2026 at 4:23 PM BST, Zhi Wang wrote: > Introduce safe Rust wrappers around struct fwctl_device and > struct fwctl_uctx. This lets Rust drivers register fwctl devices and > implement firmware RPC callbacks through a typed trait interface. > > The abstraction keeps lifetime and reference-count handling inside the > wrapper, exposes pinned per-FD user contexts to drivers, and validates the > layout assumptions required by the C fwctl allocation model. Allocation > sizes are padded so the kmalloc-backed C allocations also satisfy Rust > alignment requirements. > > Registration owns driver private data with a lifetime tied to the bound > parent device and verifies the parent identity before registration. > Callbacks access that data through a higher-ranked closure, preventing its > erased lifetime from escaping, while Device remains only the refcounted > fwctl object. This avoids requiring Rust drop glue from the fwctl_device > release path after unregister or module teardown. > > RPC callbacks receive typed scope information, a mutable request/response > buffer, and the userspace output-buffer size. Response pointer conversion, > length validation, and raw output-length handling remain inside the > abstraction. > > Add the Rust sources to the FWCTL MAINTAINERS entry and add myself as the > maintainer for the Rust abstractions. > > Co-developed-by: Danilo Krummrich <[email protected]> > Signed-off-by: Danilo Krummrich <[email protected]> > Link: https://lore.kernel.org/r/[email protected] > Signed-off-by: Zhi Wang <[email protected]> > --- > MAINTAINERS | 3 + > drivers/fwctl/Kconfig | 12 + > rust/bindings/bindings_helper.h | 1 + > rust/helpers/fwctl.c | 17 + > rust/helpers/helpers.c | 3 +- > rust/kernel/fwctl.rs | 593 ++++++++++++++++++++++++++++++++ > rust/kernel/lib.rs | 2 + > 7 files changed, 630 insertions(+), 1 deletion(-) > create mode 100644 rust/helpers/fwctl.c > create mode 100644 rust/kernel/fwctl.rs > > diff --git a/MAINTAINERS b/MAINTAINERS > index 5114e6db7307..bf487b8b8e9a 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -10737,12 +10737,15 @@ FWCTL SUBSYSTEM > M: Dave Jiang <[email protected]> > M: Jason Gunthorpe <[email protected]> > M: Saeed Mahameed <[email protected]> > +M: Zhi Wang <[email protected]> (RUST) > R: Jonathan Cameron <[email protected]> > S: Maintained > F: Documentation/userspace-api/fwctl/ > F: drivers/fwctl/ > F: include/linux/fwctl.h > F: include/uapi/fwctl/ > +F: rust/helpers/fwctl.c > +F: rust/kernel/fwctl.rs > > FWCTL BNXT DRIVER > M: Pavan Chebbi <[email protected]> > diff --git a/drivers/fwctl/Kconfig b/drivers/fwctl/Kconfig > index d1b1925bdaec..cac38cf79f30 100644 > --- a/drivers/fwctl/Kconfig > +++ b/drivers/fwctl/Kconfig > @@ -9,6 +9,18 @@ menuconfig FWCTL > fit neatly into an existing subsystem. > > if FWCTL > + > +config RUST_FWCTL_ABSTRACTIONS > + bool "Rust fwctl abstractions" > + depends on RUST && FWCTL=y > + help > + This enables the Rust abstractions for the fwctl device firmware > + access framework. It provides safe wrappers around struct fwctl_device > + and struct fwctl_uctx, allowing Rust drivers to register fwctl devices > + and implement their control and RPC logic in safe Rust. > + > + If unsure, say N. > + > config FWCTL_BNXT > tristate "bnxt control fwctl driver" > depends on BNXT > diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h > index 1124785e210b..3d0511e4ab4f 100644 > --- a/rust/bindings/bindings_helper.h > +++ b/rust/bindings/bindings_helper.h > @@ -60,6 +60,7 @@ > #include <linux/fdtable.h> > #include <linux/file.h> > #include <linux/firmware.h> > +#include <linux/fwctl.h> > #include <linux/fs.h> > #include <linux/i2c.h> > #include <linux/interrupt.h> > diff --git a/rust/helpers/fwctl.c b/rust/helpers/fwctl.c > new file mode 100644 > index 000000000000..c7eecd4336a7 > --- /dev/null > +++ b/rust/helpers/fwctl.c > @@ -0,0 +1,17 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +#include <linux/fwctl.h> > + > +#if IS_ENABLED(CONFIG_RUST_FWCTL_ABSTRACTIONS) > + > +__rust_helper struct fwctl_device *rust_helper_fwctl_get(struct fwctl_device *fwctl) > +{ > + return fwctl_get(fwctl); > +} > + > +__rust_helper void rust_helper_fwctl_put(struct fwctl_device *fwctl) > +{ > + fwctl_put(fwctl); > +} > + > +#endif > diff --git a/rust/helpers/helpers.c b/rust/helpers/helpers.c > index 998e31052e66..b7d9512da9a6 100644 > --- a/rust/helpers/helpers.c > +++ b/rust/helpers/helpers.c > @@ -62,10 +62,11 @@ > #include "drm.c" > #include "drm_gpuvm.c" > #include "err.c" > -#include "irq.c" > #include "fs.c" > +#include "fwctl.c" > #include "gpu.c" > #include "io.c" > +#include "irq.c" > #include "jump_label.c" > #include "kunit.c" > #include "list.c" > diff --git a/rust/kernel/fwctl.rs b/rust/kernel/fwctl.rs > new file mode 100644 > index 000000000000..e6a8513a47d0 > --- /dev/null > +++ b/rust/kernel/fwctl.rs > @@ -0,0 +1,593 @@ > +// SPDX-License-Identifier: GPL-2.0-only > + > +//! Abstractions for the fwctl subsystem. > +//! > +//! C header: `include/linux/fwctl.h` > + > +use crate::{ > + bindings, > + container_of, > + device, > + prelude::*, > + sync::aref::{ > + ARef, > + AlwaysRefCounted, // > + }, > + types::Opaque, // > +}; > +use core::{ > + alloc::Layout, > + cell::UnsafeCell, > + marker::PhantomData, > + ptr::NonNull, > + slice, // > +}; > + > +/// Returns a kmalloc-compatible allocation size for `T`. > +const fn kmalloc_aligned_size<T>() -> usize { > + Layout::new::<T>().pad_to_align().size() What's this function doing? This is just identical to `size_of::<T>()`. The layout from a specific type is already padded to its alignment. You only need to call `pad_to_align()`, say, if you are computing a layout or increasing the alignment of layout. Best, Gary > +} > + > +/// Represents a fwctl device type. > +/// > +/// Corresponds to the C `enum fwctl_device_type`. All non-error UAPI values are represented so > +/// Rust drivers can select a device type without passing an untyped integer, while > +/// `FWCTL_DEVICE_TYPE_ERROR` remains unrepresentable. > +#[repr(u32)] > +#[derive(Copy, Clone, Debug, Eq, PartialEq)] > +pub enum DeviceType { > + /// Mellanox ConnectX (mlx5) device. > + Mlx5 = bindings::fwctl_device_type_FWCTL_DEVICE_TYPE_MLX5, > + /// CXL (Compute Express Link) device. > + Cxl = bindings::fwctl_device_type_FWCTL_DEVICE_TYPE_CXL, > + /// AMD/Pensando PDS device. > + Pds = bindings::fwctl_device_type_FWCTL_DEVICE_TYPE_PDS, > + /// Broadcom NetXtreme (bnxt) device. > + Bnxt = bindings::fwctl_device_type_FWCTL_DEVICE_TYPE_BNXT, > +}