| Message ID | 2f7a1331ad513b94fb47c05bf1d0f5c3fa803858.1744366572.git.viresh.kumar@linaro.org (mailing list archive) |
|---|---|
| State | Under Review |
| Headers | show |
| Series | Rust abstractions for clk, cpumask, cpufreq, OPP | expand |
On Fri, Apr 11, 2025 at 04:25:14PM +0530, Viresh Kumar wrote: > +pub struct Registration<T: Driver> { > + drv: KBox<UnsafeCell<bindings::cpufreq_driver>>, > + _p: 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. > +unsafe impl<T: Driver> Sync for Registration<T> {} > + > +#[allow(clippy::non_send_fields_in_send_ty)] > +// SAFETY: Registration with and unregistration from the cpufreq subsystem can happen from any > +// thread. Additionally, `T::Data` (which is dropped during unregistration) is `Send`, so it is > +// okay to move `Registration` to different threads. > +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(name: &'static CStr, data: T::Data, flags: u16, boost: bool) -> Result<Self> { Do you really need the private data? It seems to be used by only very few drivers in C either. If yes, I think you have to split this up into a CpufreqDriver structure and a separate Registration. Otherwise you won't ever get access to the private data again, after Registration::new_foreign_owned(). Note that we typically want Registration::new_foreign_owned(), because it implies the guaranteed "fence" for where we can safely consider a device to be bound. If we'd give out a Registration instance instead, the driver can arbitrarily extend its lifetime across the remove() boundary. If no, it seems to me that you can even avoid allocating a struct cpufreq_driver dynamically and make it const instead. The fields name, boost_enabled and flags could be required consts in your cpufreq::Driver trait.
On 11-04-25, 13:58, Danilo Krummrich wrote: > On Fri, Apr 11, 2025 at 04:25:14PM +0530, Viresh Kumar wrote: > > +impl<T: Driver> Registration<T> { > > + /// Registers a CPU frequency driver with the cpufreq core. > > + pub fn new(name: &'static CStr, data: T::Data, flags: u16, boost: bool) -> Result<Self> { > > Do you really need the private data? It seems to be used by only very few > drivers in C either. Yes, only a few of them are setting it to a value other than `pdev`. Maybe we can avoid it for the time being and come back to this when a driver really wants it. > If no, it seems to me that you can even avoid allocating a struct cpufreq_driver > dynamically and make it const instead. I am not sure if I understood your suggestion. The Registration::new() method still updates the instance of cpufreq_driver before passing it to the C cpufreq core. I have tried to fix the other issues though.
On Mon, Apr 14, 2025 at 02:17:06PM +0530, Viresh Kumar wrote: > On 11-04-25, 13:58, Danilo Krummrich wrote: > > On Fri, Apr 11, 2025 at 04:25:14PM +0530, Viresh Kumar wrote: > > > If no, it seems to me that you can even avoid allocating a struct cpufreq_driver > > dynamically and make it const instead. > > I am not sure if I understood your suggestion. The Registration::new() > method still updates the instance of cpufreq_driver before passing it > to the C cpufreq core. See comment in the diff below. > diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs > index 4194b9558413..9b275d4d3eb6 100644 > --- a/rust/kernel/cpufreq.rs > +++ b/rust/kernel/cpufreq.rs > @@ -685,13 +685,14 @@ fn drop(&mut self) { > /// Reference: <https://docs.kernel.org/cpu-freq/cpu-drivers.html> > #[vtable] > pub trait Driver { > - /// Driver specific data. > - /// > - /// Corresponds to the data retrieved via the kernel's `cpufreq_get_driver_data` function. > - /// > - /// Require `Data` to implement `ForeignOwnable`. We guarantee to never move the underlying > - /// wrapped data structure. > - type Data: ForeignOwnable; > + /// Driver's name. > + const NAME: &'static CStr; > + > + /// Driver's flags. > + const FLAGS: u16; > + > + /// Boost support. > + const BOOST_ENABLED: bool; > > /// Policy specific data. > /// > @@ -804,8 +805,8 @@ fn register_em(_policy: &mut Policy) { > /// > /// ``` > /// use kernel::{ > -/// c_str, > /// cpu, cpufreq, > +/// c_str, > /// device::Device, > /// macros::vtable, > /// sync::Arc, > @@ -817,7 +818,10 @@ fn register_em(_policy: &mut Policy) { > /// > /// #[vtable] > /// impl cpufreq::Driver for FooDriver { > -/// type Data = (); > +/// const NAME: &'static CStr = c_str!("cpufreq-foo"); > +/// const FLAGS: u16 = cpufreq::flags::NEED_INITIAL_FREQ_CHECK | cpufreq::flags::IS_COOLING_DEV; > +/// const BOOST_ENABLED: bool = true; > +/// > /// type PData = Arc<FooDevice>; > /// > /// fn init(policy: &mut cpufreq::Policy) -> Result<Self::PData> { > @@ -848,13 +852,7 @@ fn register_em(_policy: &mut Policy) { > /// } > /// > /// fn foo_probe(dev: &Device) { > -/// cpufreq::Registration::<FooDriver>::new_foreign_owned( > -/// dev, > -/// c_str!("cpufreq-foo"), > -/// (), > -/// cpufreq::flags::NEED_INITIAL_FREQ_CHECK | cpufreq::flags::IS_COOLING_DEV, > -/// true, > -/// ).unwrap(); > +/// cpufreq::Registration::<FooDriver>::new_foreign_owned(dev).unwrap(); > /// } > /// ``` > pub struct Registration<T: Driver> { You could define Registration<T: Driver> as pub struct Registration<T: Driver>( NonNull<bindings::cpufreq_driver>, PhantomData<T> ); and subsequently... > @@ -868,13 +866,12 @@ unsafe impl<T: Driver> Sync for Registration<T> {} > > #[allow(clippy::non_send_fields_in_send_ty)] > // SAFETY: Registration with and unregistration from the cpufreq subsystem can happen from any > -// thread. Additionally, `T::Data` (which is dropped during unregistration) is `Send`, so it is > -// okay to move `Registration` to different threads. > +// thread. > unsafe impl<T: Driver> Send for Registration<T> {} > > impl<T: Driver> Registration<T> { ...add a new const of type bindings::cpufreq_driver, i.e. const VTABLE: bindings::cpufreq_driver = bindings::cpufreq_driver { name: Self::copy_name(T::NAME), boost_enabled: T::BOOST_ENABLED, flags: T::FLAGS, [...] } const fn copy_name(name: &'static CStr) -> [kernel::ffi::c_char; CPUFREQ_NAME_LEN] { let src name.as_bytes_with_nul(); let mut dst = [0; CPUFREQ_NAME_LEN]; build_assert!(name.len() <= dst.len()); let mut i = 0; while i < dst.len() { dst[i] = src[i]; i += 1; } dst } You should then be able to store a pointer of Self::VTABLE in your Registration and and hence avoid dynamic allocation of struct cpufreq_driver. > /// Registers a CPU frequency driver with the cpufreq core. > - pub fn new(name: &'static CStr, data: T::Data, flags: u16, boost: bool) -> Result<Self> { > + 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)] > @@ -886,18 +883,18 @@ pub fn new(name: &'static CStr, data: T::Data, flags: u16, boost: bool) -> Resul > let drv_ref = drv.get_mut(); > > // Account for the trailing null byte. > - let len = name.len() + 1; > + let len = T::NAME.len() + 1; > if len > drv_ref.name.len() { > return Err(EINVAL); > }; > > - // SAFETY: `name` is a valid `CStr`, and we are copying it to an array of equal or larger > - // size. > - let name = unsafe { &*(name.as_bytes_with_nul() as *const [u8]) }; > + // 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 = boost; > - drv_ref.flags = flags; > + drv_ref.boost_enabled = T::BOOST_ENABLED; > + drv_ref.flags = T::FLAGS; > > // Initialize mandatory callbacks. > drv_ref.init = Some(Self::init_callback); > @@ -995,10 +992,6 @@ pub fn new(name: &'static CStr, data: T::Data, flags: u16, boost: bool) -> Resul > None > }; > > - // Set driver data before registering the driver, as the cpufreq core calls few callbacks > - // before `cpufreq_register_driver` returns. > - Self::set_data(drv_ref, data)?; > - > // 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) })?; > > @@ -1012,53 +1005,10 @@ pub fn new(name: &'static CStr, data: T::Data, flags: u16, boost: bool) -> Resul > /// > /// Instead the [`Registration`] is owned by [`Devres`] and will be revoked / dropped, once the > /// device is detached. > - pub fn new_foreign_owned( > - dev: &Device, > - name: &'static CStr, > - data: T::Data, > - flags: u16, > - boost: bool, > - ) -> Result<()> { > - Devres::new_foreign_owned(dev, Self::new(name, data, flags, boost)?, GFP_KERNEL)?; > + pub fn new_foreign_owned(dev: &Device) -> Result<()> { > + Devres::new_foreign_owned(dev, Self::new()?, GFP_KERNEL)?; > Ok(()) > } > - > - // Sets the `Data` for the CPU frequency driver. > - fn set_data(drv: &mut bindings::cpufreq_driver, data: T::Data) -> Result<()> { > - if drv.driver_data.is_null() { > - // Transfer the ownership of the data to the C code. > - drv.driver_data = <T::Data as ForeignOwnable>::into_foreign(data) as _; > - Ok(()) > - } else { > - Err(EBUSY) > - } > - } > - > - /// Returns borrowed `Data` previously set for the CPU frequency driver. > - pub fn data(&mut self) -> Option<<T::Data as ForeignOwnable>::Borrowed<'static>> { > - let drv = self.drv.get_mut(); > - > - if drv.driver_data.is_null() { > - None > - } else { > - // SAFETY: The data is earlier set by us from `set_data`. > - Some(unsafe { <T::Data as ForeignOwnable>::borrow(drv.driver_data) }) > - } > - } > - > - // Clears and returns the `Data` for the CPU frequency driver. > - fn clear_data(&mut self) -> Option<T::Data> { > - let drv = self.drv.get_mut(); > - > - if drv.driver_data.is_null() { > - None > - } else { > - // SAFETY: The data is earlier set by us from `set_data`. > - let data = Some(unsafe { <T::Data as ForeignOwnable>::from_foreign(drv.driver_data) }); > - drv.driver_data = ptr::null_mut(); > - data > - } > - } > } > > // CPU frequency driver callbacks. > @@ -1313,8 +1263,5 @@ fn drop(&mut self) { > > // SAFETY: The driver was earlier registered from `new`. > unsafe { bindings::cpufreq_unregister_driver(drv) }; > - > - // Free data > - drop(self.clear_data()); > } > }
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.
On Mon, Apr 14, 2025 at 04:22:12PM +0530, Viresh Kumar wrote: > 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. The diff below looks good! One nit below. > 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 _; Hm, I didn't think of that, maybe it's better to store a *const directly instead of a NonNull, given that VTABLE can't be altered. Tamir is trying to clean up 'as' casts in [1]. So, I'd write this as let drv: *const bindings::cpufreq_driver = &Self::VTABLE; [1] https://lore.kernel.org/rust-for-linux/20250409-ptr-as-ptr-v8-0-3738061534ef@gmail.com/ > > // 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()) }; > } > }
diff --git a/rust/kernel/cpufreq.rs b/rust/kernel/cpufreq.rs index 3e9ded655d46..4194b9558413 100644 --- a/rust/kernel/cpufreq.rs +++ b/rust/kernel/cpufreq.rs @@ -13,7 +13,8 @@ clk::{Clk, Hertz}, cpumask, device::Device, - error::{code::*, from_err_ptr, to_result, Result, VTABLE_DEFAULT_ERROR}, + devres::Devres, + error::{code::*, from_err_ptr, from_result, to_result, Result, VTABLE_DEFAULT_ERROR}, ffi::c_ulong, prelude::*, types::ForeignOwnable, @@ -21,9 +22,11 @@ }; use core::{ + cell::UnsafeCell, + marker::PhantomData, ops::{Deref, DerefMut}, pin::Pin, - ptr::self, + ptr, }; use macros::vtable; @@ -792,3 +795,526 @@ fn register_em(_policy: &mut Policy) { build_error!(VTABLE_DEFAULT_ERROR) } } + +/// CPU frequency driver Registration. +/// +/// ## Examples +/// +/// The following example demonstrates how to register a cpufreq driver. +/// +/// ``` +/// use kernel::{ +/// c_str, +/// cpu, cpufreq, +/// device::Device, +/// macros::vtable, +/// sync::Arc, +/// }; +/// struct FooDevice; +/// +/// #[derive(Default)] +/// struct FooDriver; +/// +/// #[vtable] +/// impl cpufreq::Driver for FooDriver { +/// type Data = (); +/// type PData = Arc<FooDevice>; +/// +/// fn init(policy: &mut cpufreq::Policy) -> Result<Self::PData> { +/// // Initialize here +/// Ok(Arc::new(FooDevice, GFP_KERNEL)?) +/// } +/// +/// fn exit(_policy: &mut cpufreq::Policy, _data: Option<Self::PData>) -> Result<()> { +/// Ok(()) +/// } +/// +/// fn suspend(policy: &mut cpufreq::Policy) -> Result<()> { +/// policy.generic_suspend() +/// } +/// +/// fn verify(data: &mut cpufreq::PolicyData) -> Result<()> { +/// data.generic_verify() +/// } +/// +/// fn target_index(policy: &mut cpufreq::Policy, index: u32) -> Result<()> { +/// // Update CPU frequency +/// Ok(()) +/// } +/// +/// fn get(policy: &mut cpufreq::Policy) -> Result<u32> { +/// policy.generic_get() +/// } +/// } +/// +/// fn foo_probe(dev: &Device) { +/// cpufreq::Registration::<FooDriver>::new_foreign_owned( +/// dev, +/// c_str!("cpufreq-foo"), +/// (), +/// cpufreq::flags::NEED_INITIAL_FREQ_CHECK | cpufreq::flags::IS_COOLING_DEV, +/// true, +/// ).unwrap(); +/// } +/// ``` +pub struct Registration<T: Driver> { + drv: KBox<UnsafeCell<bindings::cpufreq_driver>>, + _p: 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. +unsafe impl<T: Driver> Sync for Registration<T> {} + +#[allow(clippy::non_send_fields_in_send_ty)] +// SAFETY: Registration with and unregistration from the cpufreq subsystem can happen from any +// thread. Additionally, `T::Data` (which is dropped during unregistration) is `Send`, so it is +// okay to move `Registration` to different threads. +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(name: &'static CStr, data: T::Data, flags: u16, boost: bool) -> 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 = name.len() + 1; + if len > drv_ref.name.len() { + return Err(EINVAL); + }; + + // SAFETY: `name` is a valid `CStr`, and we are copying it to an array of equal or larger + // size. + let name = unsafe { &*(name.as_bytes_with_nul() as *const [u8]) }; + drv_ref.name[..len].copy_from_slice(name); + + drv_ref.boost_enabled = boost; + drv_ref.flags = flags; + + // Initialize mandatory callbacks. + drv_ref.init = Some(Self::init_callback); + drv_ref.verify = Some(Self::verify_callback); + + // Initialize optional callbacks based on the traits of `T`. + drv_ref.setpolicy = if T::HAS_SETPOLICY { + Some(Self::setpolicy_callback) + } else { + None + }; + drv_ref.target = if T::HAS_TARGET { + Some(Self::target_callback) + } else { + None + }; + drv_ref.target_index = if T::HAS_TARGET_INDEX { + Some(Self::target_index_callback) + } else { + None + }; + drv_ref.fast_switch = if T::HAS_FAST_SWITCH { + Some(Self::fast_switch_callback) + } else { + None + }; + drv_ref.adjust_perf = if T::HAS_ADJUST_PERF { + Some(Self::adjust_perf_callback) + } else { + None + }; + drv_ref.get_intermediate = if T::HAS_GET_INTERMEDIATE { + Some(Self::get_intermediate_callback) + } else { + None + }; + drv_ref.target_intermediate = if T::HAS_TARGET_INTERMEDIATE { + Some(Self::target_intermediate_callback) + } else { + None + }; + drv_ref.get = if T::HAS_GET { + Some(Self::get_callback) + } else { + None + }; + drv_ref.update_limits = if T::HAS_UPDATE_LIMITS { + Some(Self::update_limits_callback) + } else { + None + }; + drv_ref.bios_limit = if T::HAS_BIOS_LIMIT { + Some(Self::bios_limit_callback) + } else { + None + }; + drv_ref.online = if T::HAS_ONLINE { + Some(Self::online_callback) + } else { + None + }; + drv_ref.offline = if T::HAS_OFFLINE { + Some(Self::offline_callback) + } else { + None + }; + drv_ref.exit = if T::HAS_EXIT { + Some(Self::exit_callback) + } else { + None + }; + drv_ref.suspend = if T::HAS_SUSPEND { + Some(Self::suspend_callback) + } else { + None + }; + drv_ref.resume = if T::HAS_RESUME { + Some(Self::resume_callback) + } else { + None + }; + drv_ref.ready = if T::HAS_READY { + Some(Self::ready_callback) + } else { + None + }; + drv_ref.set_boost = if T::HAS_SET_BOOST { + Some(Self::set_boost_callback) + } else { + None + }; + drv_ref.register_em = if T::HAS_REGISTER_EM { + Some(Self::register_em_callback) + } else { + None + }; + + // Set driver data before registering the driver, as the cpufreq core calls few callbacks + // before `cpufreq_register_driver` returns. + Self::set_data(drv_ref, data)?; + + // 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) })?; + + Ok(Self { + drv, + _p: PhantomData, + }) + } + + /// Same as [`Registration::new`], but does not return a [`Registration`] instance. + /// + /// Instead the [`Registration`] is owned by [`Devres`] and will be revoked / dropped, once the + /// device is detached. + pub fn new_foreign_owned( + dev: &Device, + name: &'static CStr, + data: T::Data, + flags: u16, + boost: bool, + ) -> Result<()> { + Devres::new_foreign_owned(dev, Self::new(name, data, flags, boost)?, GFP_KERNEL)?; + Ok(()) + } + + // Sets the `Data` for the CPU frequency driver. + fn set_data(drv: &mut bindings::cpufreq_driver, data: T::Data) -> Result<()> { + if drv.driver_data.is_null() { + // Transfer the ownership of the data to the C code. + drv.driver_data = <T::Data as ForeignOwnable>::into_foreign(data) as _; + Ok(()) + } else { + Err(EBUSY) + } + } + + /// Returns borrowed `Data` previously set for the CPU frequency driver. + pub fn data(&mut self) -> Option<<T::Data as ForeignOwnable>::Borrowed<'static>> { + let drv = self.drv.get_mut(); + + if drv.driver_data.is_null() { + None + } else { + // SAFETY: The data is earlier set by us from `set_data`. + Some(unsafe { <T::Data as ForeignOwnable>::borrow(drv.driver_data) }) + } + } + + // Clears and returns the `Data` for the CPU frequency driver. + fn clear_data(&mut self) -> Option<T::Data> { + let drv = self.drv.get_mut(); + + if drv.driver_data.is_null() { + None + } else { + // SAFETY: The data is earlier set by us from `set_data`. + let data = Some(unsafe { <T::Data as ForeignOwnable>::from_foreign(drv.driver_data) }); + drv.driver_data = ptr::null_mut(); + data + } + } +} + +// CPU frequency driver callbacks. +impl<T: Driver> Registration<T> { + // Driver's `init` callback. + // + // SAFETY: Called from C. Inputs must be valid pointers. + extern "C" fn init_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int { + from_result(|| { + // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the + // lifetime of `policy`. + let policy = unsafe { Policy::from_raw_mut(ptr) }; + + let data = T::init(policy)?; + policy.set_data(data)?; + Ok(0) + }) + } + + // Driver's `exit` callback. + // + // SAFETY: Called from C. Inputs must be valid pointers. + extern "C" fn exit_callback(ptr: *mut bindings::cpufreq_policy) { + // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the + // lifetime of `policy`. + let policy = unsafe { Policy::from_raw_mut(ptr) }; + + let data = policy.clear_data(); + let _ = T::exit(policy, data); + } + + // Driver's `online` callback. + // + // SAFETY: Called from C. Inputs must be valid pointers. + extern "C" fn online_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int { + from_result(|| { + // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the + // lifetime of `policy`. + let policy = unsafe { Policy::from_raw_mut(ptr) }; + T::online(policy).map(|()| 0) + }) + } + + // Driver's `offline` callback. + // + // SAFETY: Called from C. Inputs must be valid pointers. + extern "C" fn offline_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int { + from_result(|| { + // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the + // lifetime of `policy`. + let policy = unsafe { Policy::from_raw_mut(ptr) }; + T::offline(policy).map(|()| 0) + }) + } + + // Driver's `suspend` callback. + // + // SAFETY: Called from C. Inputs must be valid pointers. + extern "C" fn suspend_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int { + from_result(|| { + // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the + // lifetime of `policy`. + let policy = unsafe { Policy::from_raw_mut(ptr) }; + T::suspend(policy).map(|()| 0) + }) + } + + // Driver's `resume` callback. + // + // SAFETY: Called from C. Inputs must be valid pointers. + extern "C" fn resume_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int { + from_result(|| { + // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the + // lifetime of `policy`. + let policy = unsafe { Policy::from_raw_mut(ptr) }; + T::resume(policy).map(|()| 0) + }) + } + + // Driver's `ready` callback. + // + // SAFETY: Called from C. Inputs must be valid pointers. + extern "C" fn ready_callback(ptr: *mut bindings::cpufreq_policy) { + // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the + // lifetime of `policy`. + let policy = unsafe { Policy::from_raw_mut(ptr) }; + T::ready(policy); + } + + // Driver's `verify` callback. + // + // SAFETY: Called from C. Inputs must be valid pointers. + extern "C" fn verify_callback(ptr: *mut bindings::cpufreq_policy_data) -> kernel::ffi::c_int { + from_result(|| { + // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the + // lifetime of `policy`. + let data = unsafe { PolicyData::from_raw_mut(ptr) }; + T::verify(data).map(|()| 0) + }) + } + + // Driver's `setpolicy` callback. + // + // SAFETY: Called from C. Inputs must be valid pointers. + extern "C" fn setpolicy_callback(ptr: *mut bindings::cpufreq_policy) -> kernel::ffi::c_int { + from_result(|| { + // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the + // lifetime of `policy`. + let policy = unsafe { Policy::from_raw_mut(ptr) }; + T::setpolicy(policy).map(|()| 0) + }) + } + + // Driver's `target` callback. + // + // SAFETY: Called from C. Inputs must be valid pointers. + extern "C" fn target_callback( + ptr: *mut bindings::cpufreq_policy, + target_freq: u32, + relation: u32, + ) -> kernel::ffi::c_int { + from_result(|| { + // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the + // lifetime of `policy`. + let policy = unsafe { Policy::from_raw_mut(ptr) }; + T::target(policy, target_freq, Relation::new(relation)?).map(|()| 0) + }) + } + + // Driver's `target_index` callback. + // + // SAFETY: Called from C. Inputs must be valid pointers. + extern "C" fn target_index_callback( + ptr: *mut bindings::cpufreq_policy, + index: u32, + ) -> kernel::ffi::c_int { + from_result(|| { + // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the + // lifetime of `policy`. + let policy = unsafe { Policy::from_raw_mut(ptr) }; + T::target_index(policy, index).map(|()| 0) + }) + } + + // Driver's `fast_switch` callback. + // + // SAFETY: Called from C. Inputs must be valid pointers. + extern "C" fn fast_switch_callback( + ptr: *mut bindings::cpufreq_policy, + target_freq: u32, + ) -> kernel::ffi::c_uint { + // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the + // lifetime of `policy`. + let policy = unsafe { Policy::from_raw_mut(ptr) }; + T::fast_switch(policy, target_freq) + } + + // Driver's `adjust_perf` callback. + extern "C" fn adjust_perf_callback( + cpu: u32, + min_perf: usize, + target_perf: usize, + capacity: usize, + ) { + if let Ok(mut policy) = PolicyCpu::from_cpu(cpu) { + T::adjust_perf(&mut policy, min_perf, target_perf, capacity); + } + } + + // Driver's `get_intermediate` callback. + // + // SAFETY: Called from C. Inputs must be valid pointers. + extern "C" fn get_intermediate_callback( + ptr: *mut bindings::cpufreq_policy, + index: u32, + ) -> kernel::ffi::c_uint { + // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the + // lifetime of `policy`. + let policy = unsafe { Policy::from_raw_mut(ptr) }; + T::get_intermediate(policy, index) + } + + // Driver's `target_intermediate` callback. + // + // SAFETY: Called from C. Inputs must be valid pointers. + extern "C" fn target_intermediate_callback( + ptr: *mut bindings::cpufreq_policy, + index: u32, + ) -> kernel::ffi::c_int { + from_result(|| { + // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the + // lifetime of `policy`. + let policy = unsafe { Policy::from_raw_mut(ptr) }; + T::target_intermediate(policy, index).map(|()| 0) + }) + } + + // Driver's `get` callback. + extern "C" fn get_callback(cpu: u32) -> kernel::ffi::c_uint { + PolicyCpu::from_cpu(cpu).map_or(0, |mut policy| T::get(&mut policy).map_or(0, |f| f)) + } + + // Driver's `update_limit` callback. + extern "C" fn update_limits_callback(cpu: u32) { + if let Ok(mut policy) = PolicyCpu::from_cpu(cpu) { + T::update_limits(&mut policy); + } + } + + // Driver's `bios_limit` callback. + // + // SAFETY: Called from C. Inputs must be valid pointers. + extern "C" fn bios_limit_callback(cpu: i32, limit: *mut u32) -> kernel::ffi::c_int { + from_result(|| { + let mut policy = PolicyCpu::from_cpu(cpu as u32)?; + + // SAFETY: `limit` is guaranteed by the C code to be valid. + T::bios_limit(&mut policy, &mut (unsafe { *limit })).map(|()| 0) + }) + } + + // Driver's `set_boost` callback. + // + // SAFETY: Called from C. Inputs must be valid pointers. + extern "C" fn set_boost_callback( + ptr: *mut bindings::cpufreq_policy, + state: i32, + ) -> kernel::ffi::c_int { + from_result(|| { + // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the + // lifetime of `policy`. + let policy = unsafe { Policy::from_raw_mut(ptr) }; + T::set_boost(policy, state).map(|()| 0) + }) + } + + // Driver's `register_em` callback. + // + // SAFETY: Called from C. Inputs must be valid pointers. + extern "C" fn register_em_callback(ptr: *mut bindings::cpufreq_policy) { + // SAFETY: The `ptr` is guaranteed to be valid by the contract with the C code for the + // lifetime of `policy`. + let policy = unsafe { Policy::from_raw_mut(ptr) }; + T::register_em(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) }; + + // Free data + drop(self.clear_data()); + } +}
Extend the cpufreq abstractions to support driver registration from Rust. Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org> --- rust/kernel/cpufreq.rs | 530 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 528 insertions(+), 2 deletions(-)