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

Gary Guo
2026-01-26T17:48:14Z

On Thu Jan 22, 2026 at 8:42 PM GMT, Zhi Wang wrote:
> 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
> + }
> +}