linux-kernel - Re: [PATCH v2 03/10] rust: implement `IdArray`, `IdTable` and `RawDeviceId`

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2024062031-untracked-opt-3ff1@gregkh>
Date: Thu, 20 Jun 2024 16:31:32 +0200
From: Greg KH <gregkh@...uxfoundation.org>
To: Danilo Krummrich <dakr@...hat.com>
Cc: rafael@...nel.org, bhelgaas@...gle.com, ojeda@...nel.org,
	alex.gaynor@...il.com, wedsonaf@...il.com, boqun.feng@...il.com,
	gary@...yguo.net, bjorn3_gh@...tonmail.com, benno.lossin@...ton.me,
	a.hindborg@...sung.com, aliceryhl@...gle.com, airlied@...il.com,
	fujita.tomonori@...il.com, lina@...hilina.net, pstanner@...hat.com,
	ajanulgu@...hat.com, lyude@...hat.com, robh@...nel.org,
	daniel.almeida@...labora.com, rust-for-linux@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-pci@...r.kernel.org
Subject: Re: [PATCH v2 03/10] rust: implement `IdArray`, `IdTable` and
 `RawDeviceId`

On Wed, Jun 19, 2024 at 01:39:49AM +0200, Danilo Krummrich wrote:
> From: Wedson Almeida Filho <wedsonaf@...il.com>
> 
> 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.

Let's not.

For now, let's stick with the C arrays please.  That way it's simple,
makes it easy to understand, and you can put the array in a .c file and
access it that way.  That way also all of our existing infrastructure
will work properly (I don't know how you deal with the modinfo sections
here...)

> 
> 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: Asahi Lina <lina@...hilina.net>
> Signed-off-by: Asahi Lina <lina@...hilina.net>
> Signed-off-by: Wedson Almeida Filho <wedsonaf@...il.com>
> Co-developed-by: Danilo Krummrich <dakr@...hat.com>
> Signed-off-by: Danilo Krummrich <dakr@...hat.com>
> ---
>  rust/kernel/device_id.rs | 336 +++++++++++++++++++++++++++++++++++++++
>  rust/kernel/lib.rs       |   2 +
>  2 files changed, 338 insertions(+)
>  create mode 100644 rust/kernel/device_id.rs
> 
> diff --git a/rust/kernel/device_id.rs b/rust/kernel/device_id.rs
> new file mode 100644
> index 000000000000..c490300f29bb
> --- /dev/null
> +++ b/rust/kernel/device_id.rs

While I see the temptation to attempt to create a generic class for ids,
this is all VERY bus specific.  Please just stick with a simple C array
for now, trying to mess with generic array types of arbitrary structures
and values and fields is a fun exercise in rust macros, but ick, let's
worry about that much later.

Try to do something "simple" if you really must, like just a pci device
id list, and if that works, then try USB ids.  And then, if you think
they look comon, THEN think about maybe combining them into something
that looks crazy like this.

But not now, no, please, keep it simple.  Stick to C arrays.

greg k-h