linux-kernel - Re: [PATCH V10 11/15] rust: cpufreq: Add initial abstractions for cpufreq framework

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: <20250416093720.5nigxsirbvyiumcv@vireshk-i7>
Date: Wed, 16 Apr 2025 15:07:20 +0530
From: Viresh Kumar <viresh.kumar@...aro.org>
To: Danilo Krummrich <dakr@...nel.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>,
	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@...nel.org>,
	Alice Ryhl <aliceryhl@...gle.com>, Trevor Gross <tmgross@...ch.edu>,
	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>,
	Alex Bennée <alex.bennee@...aro.org>,
	Joakim Bech <joakim.bech@...aro.org>, Rob Herring <robh@...nel.org>,
	Yury Norov <yury.norov@...il.com>, Burak Emir <bqe@...gle.com>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Russell King <linux@...linux.org.uk>, linux-clk@...r.kernel.org,
	Michael Turquette <mturquette@...libre.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH V10 11/15] rust: cpufreq: Add initial abstractions for
 cpufreq framework

On 16-04-25, 11:14, Danilo Krummrich wrote:
> On Wed, Apr 16, 2025 at 12:09:28PM +0530, Viresh Kumar wrote:
> > +#[allow(dead_code)]
> 
> Why is this needed?

Looks like leftover from a previous version. Same for the other one.

I have also made a change to the cpufreq driver now to remove
`dead_code`, hope that is fine:

diff --git a/drivers/cpufreq/rcpufreq_dt.rs b/drivers/cpufreq/rcpufreq_dt.rs
index 81ede13909b7..0d1612ff5677 100644
--- a/drivers/cpufreq/rcpufreq_dt.rs
+++ b/drivers/cpufreq/rcpufreq_dt.rs
@@ -43,12 +43,9 @@ fn find_supply_names(dev: &Device, cpu: u32) -> Option<KVec<CString>> {
 struct CPUFreqDTDevice {
     opp_table: opp::Table,
     freq_table: opp::FreqTable,
-    #[allow(dead_code)]
-    mask: CpumaskVar,
-    #[allow(dead_code)]
-    token: Option<opp::ConfigToken>,
-    #[allow(dead_code)]
-    clk: Clk,
+    _mask: CpumaskVar,
+    _token: Option<opp::ConfigToken>,
+    _clk: Clk,
 }

> > +    pub unsafe fn data(&self, index: usize) -> u32 {
> > +        // SAFETY: By the type invariant, the pointer stored in `self` is valid and `index` is
> > +        // guaranteed to be valid by the safety requirements of the function.
> > +        unsafe { (*self.as_raw().add(index)).driver_data }
> > +    }
> 
> Those three functions above look like they're supposed to be used directly by
> drivers, but are unsafe. :(
> 
> It looks like the reason for them being unsafe is that with only the pointer to
> the struct cpufreq_frequency_table array we don't know the length of the array.

Yes.

> However, a Table instance seems to come from TableBox, which *does* know the
> length of the KVec<bindings::cpufreq_frequency_table>. Why can't we just preserve the
> length and provide a safe API?

The Table is also created from a raw pointer, when it is received from
the C callbacks. Also the Table can be created from the OPP table,
where again we receive a raw pointer from the C code.

I tried to do this differently earlier and finalized on current
version after some discussions on the list:

https://lore.kernel.org/all/2025011327-cubbyhole-idealness-d4cc@gregkh/

-- 
viresh