| 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
| ||
|
Message-ID: <20250414105212.glxkrto4ybvxgx3y@vireshk-i7>
Date: Mon, 14 Apr 2025 16:22:12 +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>,
Erik Schilling <erik.schilling@...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 V9 15/17] rust: cpufreq: Extend abstractions for driver
registration
On 14-04-25, 11:39, Danilo Krummrich wrote:
> const VTABLE: bindings::cpufreq_driver = bindings::cpufreq_driver {
> name: Self::copy_name(T::NAME),
> boost_enabled: T::BOOST_ENABLED,
> flags: T::FLAGS,
> [...]
> }
Ahh, thanks for this.
--
viresh
diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs
index 9b275d4d3eb6..a6e660d46304 100644
--- a/rust/kernel/cpufreq.rs
+++ b/rust/kernel/cpufreq.rs
@@ -9,28 +9,32 @@
//! Reference: <https://docs.kernel.org/admin-guide/pm/cpufreq.html>
use crate::{
+ alloc::AllocError,
bindings,
clk::{Clk, Hertz},
cpumask,
device::Device,
devres::Devres,
error::{code::*, from_err_ptr, from_result, to_result, Result, VTABLE_DEFAULT_ERROR},
- ffi::c_ulong,
+ ffi::{c_char, c_ulong},
prelude::*,
types::ForeignOwnable,
types::Opaque,
};
use core::{
- cell::UnsafeCell,
marker::PhantomData,
+ mem::MaybeUninit,
ops::{Deref, DerefMut},
pin::Pin,
- ptr,
+ ptr::{self, NonNull},
};
use macros::vtable;
+// Maximum length of CPU frequency driver's name.
+const CPUFREQ_NAME_LEN: usize = bindings::CPUFREQ_NAME_LEN as usize;
+
/// Default transition latency value in nanoseconds.
pub const ETERNAL_LATENCY_NS: u32 = bindings::CPUFREQ_ETERNAL as u32;
@@ -855,10 +859,8 @@ fn register_em(_policy: &mut Policy) {
/// cpufreq::Registration::<FooDriver>::new_foreign_owned(dev).unwrap();
/// }
/// ```
-pub struct Registration<T: Driver> {
- drv: KBox<UnsafeCell<bindings::cpufreq_driver>>,
- _p: PhantomData<T>,
-}
+#[repr(transparent)]
+pub struct Registration<T: Driver>(NonNull<bindings::cpufreq_driver>, PhantomData<T>);
// SAFETY: `Registration` doesn't offer any methods or access to fields when shared between threads
// or CPUs, so it is safe to share it.
@@ -870,135 +872,136 @@ unsafe impl<T: Driver> Sync for Registration<T> {}
unsafe impl<T: Driver> Send for Registration<T> {}
impl<T: Driver> Registration<T> {
- /// Registers a CPU frequency driver with the cpufreq core.
- pub fn new() -> Result<Self> {
- // Required due to Rust 1.82's stricter handling of `unsafe` in mutable statics. The
- // `unsafe` blocks aren't required anymore with later versions.
- #![allow(unused_unsafe)]
-
- let mut drv = KBox::new(
- UnsafeCell::new(bindings::cpufreq_driver::default()),
- GFP_KERNEL,
- )?;
- let drv_ref = drv.get_mut();
-
- // Account for the trailing null byte.
- let len = T::NAME.len() + 1;
- if len > drv_ref.name.len() {
- return Err(EINVAL);
- };
-
- // SAFETY: `T::NAME` is a valid `CStr`, and we are copying it to an array of equal or
- // larger size.
- let name = unsafe { &*(T::NAME.as_bytes_with_nul() as *const [u8]) };
- drv_ref.name[..len].copy_from_slice(name);
-
- drv_ref.boost_enabled = T::BOOST_ENABLED;
- drv_ref.flags = T::FLAGS;
+ const VTABLE: bindings::cpufreq_driver = bindings::cpufreq_driver {
+ name: Self::copy_name(T::NAME),
+ boost_enabled: T::BOOST_ENABLED,
+ flags: T::FLAGS,
// Initialize mandatory callbacks.
- drv_ref.init = Some(Self::init_callback);
- drv_ref.verify = Some(Self::verify_callback);
+ init: Some(Self::init_callback),
+ verify: Some(Self::verify_callback),
// Initialize optional callbacks based on the traits of `T`.
- drv_ref.setpolicy = if T::HAS_SETPOLICY {
+ setpolicy: if T::HAS_SETPOLICY {
Some(Self::setpolicy_callback)
} else {
None
- };
- drv_ref.target = if T::HAS_TARGET {
+ },
+ target: if T::HAS_TARGET {
Some(Self::target_callback)
} else {
None
- };
- drv_ref.target_index = if T::HAS_TARGET_INDEX {
+ },
+ target_index: if T::HAS_TARGET_INDEX {
Some(Self::target_index_callback)
} else {
None
- };
- drv_ref.fast_switch = if T::HAS_FAST_SWITCH {
+ },
+ fast_switch: if T::HAS_FAST_SWITCH {
Some(Self::fast_switch_callback)
} else {
None
- };
- drv_ref.adjust_perf = if T::HAS_ADJUST_PERF {
+ },
+ adjust_perf: if T::HAS_ADJUST_PERF {
Some(Self::adjust_perf_callback)
} else {
None
- };
- drv_ref.get_intermediate = if T::HAS_GET_INTERMEDIATE {
+ },
+ get_intermediate: if T::HAS_GET_INTERMEDIATE {
Some(Self::get_intermediate_callback)
} else {
None
- };
- drv_ref.target_intermediate = if T::HAS_TARGET_INTERMEDIATE {
+ },
+ target_intermediate: if T::HAS_TARGET_INTERMEDIATE {
Some(Self::target_intermediate_callback)
} else {
None
- };
- drv_ref.get = if T::HAS_GET {
+ },
+ get: if T::HAS_GET {
Some(Self::get_callback)
} else {
None
- };
- drv_ref.update_limits = if T::HAS_UPDATE_LIMITS {
+ },
+ update_limits: if T::HAS_UPDATE_LIMITS {
Some(Self::update_limits_callback)
} else {
None
- };
- drv_ref.bios_limit = if T::HAS_BIOS_LIMIT {
+ },
+ bios_limit: if T::HAS_BIOS_LIMIT {
Some(Self::bios_limit_callback)
} else {
None
- };
- drv_ref.online = if T::HAS_ONLINE {
+ },
+ online: if T::HAS_ONLINE {
Some(Self::online_callback)
} else {
None
- };
- drv_ref.offline = if T::HAS_OFFLINE {
+ },
+ offline: if T::HAS_OFFLINE {
Some(Self::offline_callback)
} else {
None
- };
- drv_ref.exit = if T::HAS_EXIT {
+ },
+ exit: if T::HAS_EXIT {
Some(Self::exit_callback)
} else {
None
- };
- drv_ref.suspend = if T::HAS_SUSPEND {
+ },
+ suspend: if T::HAS_SUSPEND {
Some(Self::suspend_callback)
} else {
None
- };
- drv_ref.resume = if T::HAS_RESUME {
+ },
+ resume: if T::HAS_RESUME {
Some(Self::resume_callback)
} else {
None
- };
- drv_ref.ready = if T::HAS_READY {
+ },
+ ready: if T::HAS_READY {
Some(Self::ready_callback)
} else {
None
- };
- drv_ref.set_boost = if T::HAS_SET_BOOST {
+ },
+ set_boost: if T::HAS_SET_BOOST {
Some(Self::set_boost_callback)
} else {
None
- };
- drv_ref.register_em = if T::HAS_REGISTER_EM {
+ },
+ register_em: if T::HAS_REGISTER_EM {
Some(Self::register_em_callback)
} else {
None
- };
+ },
+ // SAFETY: All zeros is a valid value for `bindings::cpufreq_driver`.
+ ..unsafe { MaybeUninit::zeroed().assume_init() }
+ };
+
+ const fn copy_name(name: &'static CStr) -> [c_char; CPUFREQ_NAME_LEN] {
+ let src = name.as_bytes_with_nul();
+ let mut dst = [0; CPUFREQ_NAME_LEN];
+
+ build_assert!(src.len() <= CPUFREQ_NAME_LEN);
+
+ let mut i = 0;
+ while i < src.len() {
+ dst[i] = src[i];
+ i += 1;
+ }
+
+ dst
+ }
+
+ /// Registers a CPU frequency driver with the cpufreq core.
+ pub fn new() -> Result<Self> {
+ let drv = &Self::VTABLE as *const _ as *mut _;
// SAFETY: It is safe to register the driver with the cpufreq core in the kernel C code.
- to_result(unsafe { bindings::cpufreq_register_driver(drv_ref) })?;
+ to_result(unsafe { bindings::cpufreq_register_driver(drv) })?;
- Ok(Self {
- drv,
- _p: PhantomData,
- })
+ Ok(Self(
+ NonNull::new(drv.cast()).ok_or(AllocError)?,
+ PhantomData,
+ ))
}
/// Same as [`Registration::new`], but does not return a [`Registration`] instance.
@@ -1259,9 +1262,7 @@ extern "C" fn register_em_callback(ptr: *mut bindings::cpufreq_policy) {
impl<T: Driver> Drop for Registration<T> {
// Removes the `Registration` from the kernel, if it has initialized successfully earlier.
fn drop(&mut self) {
- let drv = self.drv.get_mut();
-
// SAFETY: The driver was earlier registered from `new`.
- unsafe { bindings::cpufreq_unregister_driver(drv) };
+ unsafe { bindings::cpufreq_unregister_driver(self.0.as_ptr()) };
}
}
Powered by blists - more mailing lists