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

["Gary Guo" <gary@garyguo.net>]

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
> +    }
> +}