Re: [PATCH v7 03/16] rust: implement `IdArray`, `IdTable` and `RawDeviceId`

[Gary Guo <gary@garyguo.net>]

On Thu, 19 Dec 2024 18:04:05 +0100
Danilo Krummrich <dakr@kernel.org> wrote:

> Most subsystems use some kind of ID to match devices and drivers. Hence,
> we have to provide Rust drivers an abstraction to register an ID table
> for the driver to match.
> 
> Generally, those IDs are subsystem specific and hence need to be
> implemented by the corresponding subsystem. However, the `IdArray`,
> `IdTable` and `RawDeviceId` types provide a generalized implementation
> that makes the life of subsystems easier to do so.
> 
> Co-developed-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@gmail.com>
> Co-developed-by: Gary Guo <gary@garyguo.net>
> Signed-off-by: Gary Guo <gary@garyguo.net>
> Co-developed-by: Fabien Parent <fabien.parent@linaro.org>
> Signed-off-by: Fabien Parent <fabien.parent@linaro.org>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Thank you for converting my prototype to a working patch. There's a nit below.

> ---
>  MAINTAINERS              |   1 +
>  rust/kernel/device_id.rs | 165 +++++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs       |   6 ++
>  3 files changed, 172 insertions(+)
>  create mode 100644 rust/kernel/device_id.rs
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2ad58ed40079..3cfb68650347 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7033,6 +7033,7 @@ F:	include/linux/kobj*
>  F:	include/linux/property.h
>  F:	lib/kobj*
>  F:	rust/kernel/device.rs
> +F:	rust/kernel/device_id.rs
>  F:	rust/kernel/driver.rs
>  
>  DRIVERS FOR OMAP ADAPTIVE VOLTAGE SCALING (AVS)
> diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
> new file mode 100644
> index 000000000000..e5859217a579
> --- /dev/null
> +++ b/rust/kernel/device_id.rs
> 
> <snip>
>
> +
> +impl<T: RawDeviceId, const N: usize> RawIdArray<T, N> {
> +    #[doc(hidden)]
> +    pub const fn size(&self) -> usize {
> +        core::mem::size_of::<Self>()
> +    }
> +}
> +

This is not necessary, see below.

> <snip>
>
> +
> +/// Create device table alias for modpost.
> +#[macro_export]
> +macro_rules! module_device_table {
> +    ($table_type: literal, $module_table_name:ident, $table_name:ident) => {
> +        #[rustfmt::skip]
> +        #[export_name =
> +            concat!("__mod_device_table__", $table_type,
> +                    "__", module_path!(),
> +                    "_", line!(),
> +                    "_", stringify!($table_name))
> +        ]
> +        static $module_table_name: [core::mem::MaybeUninit<u8>; $table_name.raw_ids().size()] =
> +            unsafe { core::mem::transmute_copy($table_name.raw_ids()) };

const_size_of_val will be stable in Rust 1.85, so we can start using the
feature and 

static $module_table_name: [core::mem::MaybeUninit<u8>; core::mem::size_of_val($table_name.raw_ids())] =
    unsafe { core::mem::transmute_copy($table_name.raw_ids()) };

should work.

> +    };
> +}
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 7818407f9aac..66149ac5c0c9 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -18,6 +18,11 @@
>  #![feature(inline_const)]
>  #![feature(lint_reasons)]
>  #![feature(unsize)]
> +// Stable in Rust 1.83
> +#![feature(const_maybe_uninit_as_mut_ptr)]
> +#![feature(const_mut_refs)]
> +#![feature(const_ptr_write)]
> +#![feature(const_refs_to_cell)]
>  
>  // Ensure conditional compilation based on the kernel configuration works;
>  // otherwise we may silently break things like initcall handling.
> @@ -35,6 +40,7 @@
>  mod build_assert;
>  pub mod cred;
>  pub mod device;
> +pub mod device_id;
>  pub mod driver;
>  pub mod error;
>  #[cfg(CONFIG_RUST_FW_LOADER_ABSTRACTIONS)]