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

Zhi Wang
2026-01-27T19:59:40Z

On Mon, 26 Jan 2026 17:48:09 +0000
"Gary Guo" <gary@garyguo.net> wrote:

> On Thu Jan 22, 2026 at 8:42 PM GMT, Zhi Wang wrote:

Many thanks for the comments. Will integrate them in the next re-spin.

Z.

> > Introduce 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.
> >
> > Cc: Danilo Krummrich <dakr@kernel.org>
> > Cc: Jason Gunthorpe <jgg@nvidia.com>
> > Signed-off-by: Zhi Wang <zhiw@nvidia.com>
> > ---
> > drivers/fwctl/Kconfig | 12 +
> > include/uapi/fwctl/fwctl.h | 1 +
> > rust/bindings/bindings_helper.h | 1 +
> > rust/helpers/fwctl.c | 17 ++
> > rust/helpers/helpers.c | 3 +-
> > rust/kernel/fwctl.rs | 456 ++++++++++++++++++++++++++++++++
> > rust/kernel/lib.rs | 2 +
> > 7 files changed, 491 insertions(+), 1 deletion(-)
> > create mode 100644 rust/helpers/fwctl.c
> > create mode 100644 rust/kernel/fwctl.rs
> >
> > diff --git a/drivers/fwctl/Kconfig b/drivers/fwctl/Kconfig
> > index b5583b12a011..d8538249f3ae 100644
> > --- a/drivers/fwctl/Kconfig
> > +++ b/drivers/fwctl/Kconfig
> > @@ -8,6 +8,18 @@ menuconfig FWCTL
> > manipulating device FLASH, debugging, and other activities
> > that don't fit neatly into an existing subsystem.
> >
> > +config RUST_FWCTL_ABSTRACTIONS
> > + bool "Rust fwctl abstractions"
> > + depends on RUST
> > + select FWCTL
> > + 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.
> > +
> > if FWCTL
> > config FWCTL_MLX5
> > tristate "mlx5 ConnectX control fwctl driver"
> > diff --git a/include/uapi/fwctl/fwctl.h b/include/uapi/fwctl/fwctl.h
> > index 716ac0eee42d..eea1020ad180 100644
> > --- a/include/uapi/fwctl/fwctl.h
> > +++ b/include/uapi/fwctl/fwctl.h
> > @@ -45,6 +45,7 @@ enum fwctl_device_type {
> > FWCTL_DEVICE_TYPE_MLX5 = 1,
> > FWCTL_DEVICE_TYPE_CXL = 2,
> > FWCTL_DEVICE_TYPE_PDS = 4,
> > + FWCTL_DEVICE_TYPE_RUST_FWCTL_TEST = 8,
>
> This is UAPI, adding new device type just for testing is not ideal.
>
> > };
> >
> > /**
> > diff --git a/rust/bindings/bindings_helper.h
> > b/rust/bindings/bindings_helper.h index 9fdf76ca630e..2c50d5bab0cf
> > 100644 --- a/rust/bindings/bindings_helper.h
> > +++ b/rust/bindings/bindings_helper.h
> > @@ -56,6 +56,7 @@
> > #include <linux/fdtable.h>
> > #include <linux/file.h>
> > #include <linux/firmware.h>
> > +#include <linux/fwctl.h>
> > #include <linux/interrupt.h>
> > #include <linux/fs.h>
> > #include <linux/i2c.h>
> > diff --git a/rust/helpers/fwctl.c b/rust/helpers/fwctl.c
> > new file mode 100644
> > index 000000000000..bb4a028e7afb
> > --- /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)
> > +
> > +struct fwctl_device *rust_helper_fwctl_get(struct fwctl_device
> > *fwctl)
>
> Helpers need to have __rust_helper.
>
> > +{
> > + return fwctl_get(fwctl);
> > +}
> > +
> > +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 79c72762ad9c..19a505473bef 100644
> > --- a/rust/helpers/helpers.c
> > +++ b/rust/helpers/helpers.c
> > @@ -27,8 +27,9 @@
> > #include "dma.c"
> > #include "drm.c"
> > #include "err.c"
> > -#include "irq.c"
> > #include "fs.c"
> > +#include "fwctl.c"
> > +#include "irq.c"
> > #include "io.c"
> > #include "jump_label.c"
> > #include "kunit.c"
> > diff --git a/rust/kernel/fwctl.rs b/rust/kernel/fwctl.rs
> > new file mode 100644
> > index 000000000000..4065c948784d
> > --- /dev/null
> > +++ b/rust/kernel/fwctl.rs
> > @@ -0,0 +1,456 @@
> > +// SPDX-License-Identifier: GPL-2.0-only
> > +
> > +//! Abstractions for the fwctl.
> > +//!
> > +//! This module provides bindings for working with fwctl devices in
> > kernel modules. +//!
> > +//! C header: [`include/linux/fwctl.h`]
> > +
> > +use crate::{
> > + bindings,
> > + container_of,
> > + device,
> > + devres::Devres,
> > + prelude::*,
> > + types::{
> > + ARef,
> > + Opaque, //
> > + }, //
> > +};
> > +use core::{
> > + marker::PhantomData,
> > + ptr::NonNull,
> > + slice, //
> > +};
> > +
> > +/// Represents a fwctl device type.
> > +///
> > +/// This enum corresponds to the C `enum fwctl_device_type` and is
> > used to identify +/// the specific firmware control interface
> > implemented by a device. +#[repr(u32)]
> > +#[derive(Copy, Clone, Debug, Eq, PartialEq)]
> > +pub enum DeviceType {
> > + /// Error/invalid device type.
> > + Error = bindings::fwctl_device_type_FWCTL_DEVICE_TYPE_ERROR,
>
> Does this need to be present? I took a look at the C side, this isn't
> used at all. It'll be a bug if `Operations` impl uses this value as
> their `DEVICE_TYPE`.
>
> Best,
> Gary
>
> > + /// MLX5 device type.
> > + Mlx5 = bindings::fwctl_device_type_FWCTL_DEVICE_TYPE_MLX5,
> > + /// CXL device type.
> > + Cxl = bindings::fwctl_device_type_FWCTL_DEVICE_TYPE_CXL,
> > + /// PDS device type.
> > + Pds = bindings::fwctl_device_type_FWCTL_DEVICE_TYPE_PDS,
> > + /// Rust fwctl test device type.
> > + RustFwctlTest =
> > bindings::fwctl_device_type_FWCTL_DEVICE_TYPE_RUST_FWCTL_TEST, +}
> > +
> > +impl From<DeviceType> for u32 {
> > + fn from(device_type: DeviceType) -> Self {
> > + device_type as u32
> > + }
> > +}
>