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

[Zhi Wang <zhiw@nvidia.com>]

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