linux-kernel - Re: [PATCH V5 8/8] cpufreq: Add Rust based cpufreq-dt driver

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: <CAL_Jsq+SxdPyb3qQyce7u8Ur=WCd1p+pQxJ+yJrTyS2xk3BF0w@mail.gmail.com>
Date: Wed, 31 Jul 2024 15:14:23 -0600
From: Rob Herring <robh@...nel.org>
To: Viresh Kumar <viresh.kumar@...aro.org>
Cc: "Rafael J. Wysocki" <rafael@...nel.org>, Miguel Ojeda <miguel.ojeda.sandonis@...il.com>, 
	Danilo Krummrich <dakr@...hat.com>, Miguel Ojeda <ojeda@...nel.org>, Alex Gaynor <alex.gaynor@...il.com>, 
	Wedson Almeida Filho <wedsonaf@...il.com>, Boqun Feng <boqun.feng@...il.com>, Gary Guo <gary@...yguo.net>, 
	Björn Roy Baron <bjorn3_gh@...tonmail.com>, 
	Benno Lossin <benno.lossin@...ton.me>, Andreas Hindborg <a.hindborg@...sung.com>, 
	Alice Ryhl <aliceryhl@...gle.com>, linux-pm@...r.kernel.org, 
	Vincent Guittot <vincent.guittot@...aro.org>, Stephen Boyd <sboyd@...nel.org>, 
	Nishanth Menon <nm@...com>, rust-for-linux@...r.kernel.org, 
	Manos Pitsidianakis <manos.pitsidianakis@...aro.org>, Erik Schilling <erik.schilling@...aro.org>, 
	Alex Bennée <alex.bennee@...aro.org>, 
	Joakim Bech <joakim.bech@...aro.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH V5 8/8] cpufreq: Add Rust based cpufreq-dt driver

On Tue, Jul 30, 2024 at 4:27 AM Viresh Kumar <viresh.kumar@...aro.org> wrote:
>
> This commit adds a Rust based cpufreq-dt driver, which covers most of
> the functionality of the existing C based driver. Only a handful of
> things are left, like fetching platform data from cpufreq-dt-platdev.c.
>
> This is tested with the help of QEMU for now and switching of
> frequencies work as expected.
>
> Signed-off-by: Viresh Kumar <viresh.kumar@...aro.org>
> ---
>  drivers/cpufreq/Kconfig        |  12 ++
>  drivers/cpufreq/Makefile       |   1 +
>  drivers/cpufreq/rcpufreq_dt.rs | 221 +++++++++++++++++++++++++++++++++
>  3 files changed, 234 insertions(+)
>  create mode 100644 drivers/cpufreq/rcpufreq_dt.rs
>
> diff --git a/drivers/cpufreq/Kconfig b/drivers/cpufreq/Kconfig
> index 94e55c40970a..eb9359bd3c5c 100644
> --- a/drivers/cpufreq/Kconfig
> +++ b/drivers/cpufreq/Kconfig
> @@ -217,6 +217,18 @@ config CPUFREQ_DT
>
>           If in doubt, say N.
>
> +config CPUFREQ_DT_RUST
> +       tristate "Rust based Generic DT based cpufreq driver"
> +       depends on HAVE_CLK && OF && RUST
> +       select CPUFREQ_DT_PLATDEV
> +       select PM_OPP
> +       help
> +         This adds a Rust based generic DT based cpufreq driver for frequency
> +         management.  It supports both uniprocessor (UP) and symmetric
> +         multiprocessor (SMP) systems.
> +
> +         If in doubt, say N.
> +
>  config CPUFREQ_DT_PLATDEV
>         tristate "Generic DT based cpufreq platdev driver"
>         depends on OF
> diff --git a/drivers/cpufreq/Makefile b/drivers/cpufreq/Makefile
> index 8d141c71b016..4981d908b803 100644
> --- a/drivers/cpufreq/Makefile
> +++ b/drivers/cpufreq/Makefile
> @@ -15,6 +15,7 @@ obj-$(CONFIG_CPU_FREQ_GOV_COMMON)             += cpufreq_governor.o
>  obj-$(CONFIG_CPU_FREQ_GOV_ATTR_SET)    += cpufreq_governor_attr_set.o
>
>  obj-$(CONFIG_CPUFREQ_DT)               += cpufreq-dt.o
> +obj-$(CONFIG_CPUFREQ_DT_RUST)          += rcpufreq_dt.o
>  obj-$(CONFIG_CPUFREQ_DT_PLATDEV)       += cpufreq-dt-platdev.o
>
>  # Traces
> diff --git a/drivers/cpufreq/rcpufreq_dt.rs b/drivers/cpufreq/rcpufreq_dt.rs
> new file mode 100644
> index 000000000000..9204d92d3eec
> --- /dev/null
> +++ b/drivers/cpufreq/rcpufreq_dt.rs
> @@ -0,0 +1,221 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Rust based implementation of the cpufreq-dt driver.
> +
> +use core::format_args;
> +
> +use kernel::{
> +    b_str, c_str, clk, cpufreq, cpumask::Cpumask, define_of_id_table, device::Device,
> +    error::code::*, fmt, macros::vtable, module_platform_driver, of, opp, platform, prelude::*,
> +    str::CString, sync::Arc,
> +};
> +
> +// Finds exact supply name from the OF node.
> +fn find_supply_name_exact(np: &of::DeviceNode, name: &str) -> Option<CString> {
> +    let name_cstr = CString::try_from_fmt(fmt!("{}-supply", name)).ok()?;
> +
> +    np.find_property(&name_cstr).ok()?;

We don't want anything in Rust based on of_find_property() which this
is. That function assumes a device node and its properties are never
freed which is no longer a valid assumption (since OF_DYNAMIC and then
overlays). There's some work starting to address that, and my plan is
using of_find_property() on dynamic nodes will start warning. The
of_property_* API mostly avoids that issue (string types are an issue)
.

Also, it's probably the device property API we want to build Rust
bindings on top of rather than DT and ACPI. OTOH, the device property
API may be missing some features needed with OPP bindings.

Rob