Add types and traits for interfacing the C moduleparam API.
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
rust/kernel/lib.rs | 1 +
rust/kernel/module_param.rs | 191 ++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 192 insertions(+)
diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 6b4774b2b1c3..2b439ea06185 100644
--- a/rust/kernel/lib.rs
+++ b/rust/kernel/lib.rs
@@ -87,6 +87,7 @@
pub mod list;
pub mod miscdevice;
pub mod mm;
+pub mod module_param;
#[cfg(CONFIG_NET)]
pub mod net;
pub mod of;
diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
new file mode 100644
index 000000000000..ca4be7e45ff7
--- /dev/null
+++ b/rust/kernel/module_param.rs
@@ -0,0 +1,191 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Support for module parameters.
+//!
+//! C header: [`include/linux/moduleparam.h`](srctree/include/linux/moduleparam.h)
+
+use crate::prelude::*;
+use crate::str::BStr;
+use bindings;
+use kernel::sync::once_lock::OnceLock;
+
+/// Newtype to make `bindings::kernel_param` [`Sync`].
+#[repr(transparent)]
+#[doc(hidden)]
+pub struct RacyKernelParam(bindings::kernel_param);
+
+impl RacyKernelParam {
+ #[doc(hidden)]
+ pub const fn new(val: bindings::kernel_param) -> Self {
+ Self(val)
+ }
+}
+
+// SAFETY: C kernel handles serializing access to this type. We never access it
+// from Rust module.
+unsafe impl Sync for RacyKernelParam {}
+
+/// Types that can be used for module parameters.
+// NOTE: This trait is `Copy` because drop could produce unsoundness during teardown.
+pub trait ModuleParam: Sized + Copy {
+ /// The [`ModuleParam`] will be used by the kernel module through this type.
+ ///
+ /// This may differ from `Self` if, for example, `Self` needs to track
+ /// ownership without exposing it or allocate extra space for other possible
+ /// parameter values.
+ // This is required to support string parameters in the future.
+ type Value: ?Sized;
+
+ /// Parse a parameter argument into the parameter value.
+ fn try_from_param_arg(arg: &BStr) -> Result<Self>;
+}
+
+/// Set the module parameter from a string.
+///
+/// Used to set the parameter value at kernel initialization, when loading
+/// the module or when set through `sysfs`.
+///
+/// See `struct kernel_param_ops.set`.
+///
+/// # Safety
+///
+/// - If `val` is non-null then it must point to a valid null-terminated string that must be valid
+/// for reads for the duration of the call.
+/// - `param` must be a pointer to a `bindings::kernel_param` initialized by the rust module macro.
+/// The pointee must be valid for reads for the duration of the call.
+///
+/// # Note
+///
+/// - The safety requirements are satisfied by C API contract when this function is invoked by the
+/// module subsystem C code.
+/// - Currently, we only support read-only parameters that are not readable from `sysfs`. Thus, this
+/// function is only called at kernel initialization time, or at module load time, and we have
+/// exclusive access to the parameter for the duration of the function.
+///
+/// [`module!`]: macros::module
+unsafe extern "C" fn set_param<T>(val: *const c_char, param: *const bindings::kernel_param) -> c_int
+where
+ T: ModuleParam,
+{
+ // NOTE: If we start supporting arguments without values, val _is_ allowed
+ // to be null here.
+ if val.is_null() {
+ // TODO: Use pr_warn_once available.
+ crate::pr_warn!("Null pointer passed to `module_param::set_param`");
+ return EINVAL.to_errno();
+ }
+
+ // SAFETY: By function safety requirement, val is non-null, null-terminated
+ // and valid for reads for the duration of this function.
+ let arg = unsafe { CStr::from_char_ptr(val) };
+
+ crate::error::from_result(|| {
+ let new_value = T::try_from_param_arg(arg)?;
+
+ // SAFETY: By function safety requirements, this access is safe.
+ let container = unsafe { &*((*param).__bindgen_anon_1.arg as *mut OnceLock<T>) };
+
+ container
+ .populate(new_value)
+ .then_some(0)
+ .ok_or(kernel::error::code::EEXIST)
+ })
+}
+
+macro_rules! impl_int_module_param {
+ ($ty:ident) => {
+ impl ModuleParam for $ty {
+ type Value = $ty;
+
+ fn try_from_param_arg(arg: &BStr) -> Result<Self> {
+ <$ty as crate::str::parse_int::ParseInt>::from_str(arg)
+ }
+ }
+ };
+}
+
+impl_int_module_param!(i8);
+impl_int_module_param!(u8);
+impl_int_module_param!(i16);
+impl_int_module_param!(u16);
+impl_int_module_param!(i32);
+impl_int_module_param!(u32);
+impl_int_module_param!(i64);
+impl_int_module_param!(u64);
+impl_int_module_param!(isize);
+impl_int_module_param!(usize);
+
+/// A wrapper for kernel parameters.
+///
+/// This type is instantiated by the [`module!`] macro when module parameters are
+/// defined. You should never need to instantiate this type directly.
+///
+/// Note: This type is `pub` because it is used by module crates to access
+/// parameter values.
+pub struct ModuleParamAccess<T> {
+ value: OnceLock<T>,
+ default: T,
+}
+
+// SAFETY: We only create shared references to the contents of this container,
+// so if `T` is `Sync`, so is `ModuleParamAccess`.
+unsafe impl<T: Sync> Sync for ModuleParamAccess<T> {}
+
+impl<T> ModuleParamAccess<T> {
+ #[doc(hidden)]
+ pub const fn new(default: T) -> Self {
+ Self {
+ value: OnceLock::new(),
+ default,
+ }
+ }
+
+ /// Get a shared reference to the parameter value.
+ // Note: When sysfs access to parameters are enabled, we have to pass in a
+ // held lock guard here.
+ pub fn get(&self) -> &T {
+ self.value.as_ref().unwrap_or(&self.default)
+ }
+
+ /// Get a mutable pointer to `self`.
+ ///
+ /// NOTE: In most cases it is not safe deref the returned pointer.
+ pub const fn as_void_ptr(&self) -> *mut c_void {
+ (self as *const Self).cast_mut().cast()
+ }
+}
+
+#[doc(hidden)]
+/// Generate a static [`kernel_param_ops`](srctree/include/linux/moduleparam.h) struct.
+///
+/// # Examples
+///
+/// ```ignore
+/// make_param_ops!(
+/// /// Documentation for new param ops.
+/// PARAM_OPS_MYTYPE, // Name for the static.
+/// MyType // A type which implements [`ModuleParam`].
+/// );
+/// ```
+macro_rules! make_param_ops {
+ ($ops:ident, $ty:ty) => {
+ #[doc(hidden)]
+ pub static $ops: $crate::bindings::kernel_param_ops = $crate::bindings::kernel_param_ops {
+ flags: 0,
+ set: Some(set_param::<$ty>),
+ get: None,
+ free: None,
+ };
+ };
+}
+
+make_param_ops!(PARAM_OPS_I8, i8);
+make_param_ops!(PARAM_OPS_U8, u8);
+make_param_ops!(PARAM_OPS_I16, i16);
+make_param_ops!(PARAM_OPS_U16, u16);
+make_param_ops!(PARAM_OPS_I32, i32);
+make_param_ops!(PARAM_OPS_U32, u32);
+make_param_ops!(PARAM_OPS_I64, i64);
+make_param_ops!(PARAM_OPS_U64, u64);
+make_param_ops!(PARAM_OPS_ISIZE, isize);
+make_param_ops!(PARAM_OPS_USIZE, usize);
--
2.47.2On 7/2/25 3:18 PM, Andreas Hindborg wrote:
> + /// Get a shared reference to the parameter value.
> + // Note: When sysfs access to parameters are enabled, we have to pass in a
> + // held lock guard here.
> + pub fn get(&self) -> &T {
> + self.value.as_ref().unwrap_or(&self.default)
> + }
I think you forgot to rename this.
"Danilo Krummrich" <dakr@kernel.org> writes:
> On 7/2/25 3:18 PM, Andreas Hindborg wrote:
>> + /// Get a shared reference to the parameter value.
>> + // Note: When sysfs access to parameters are enabled, we have to pass in a
>> + // held lock guard here.
>> + pub fn get(&self) -> &T {
>> + self.value.as_ref().unwrap_or(&self.default)
>> + }
>
> I think you forgot to rename this.
Yes, thanks for being persistent on this :)
Best regards,
Andreas Hindborg
Andreas Hindborg <a.hindborg@kernel.org> writes:
> "Danilo Krummrich" <dakr@kernel.org> writes:
>
>> On 7/2/25 3:18 PM, Andreas Hindborg wrote:
>>> + /// Get a shared reference to the parameter value.
>>> + // Note: When sysfs access to parameters are enabled, we have to pass in a
>>> + // held lock guard here.
>>> + pub fn get(&self) -> &T {
>>> + self.value.as_ref().unwrap_or(&self.default)
>>> + }
>>
>> I think you forgot to rename this.
>
> Yes, thanks for being persistent on this :)
Actually, there is a discussion on whether to keep the API similar to
`std::sync::OnceLock` [1] but also whether to rename this to something
other than `OnceLock` [2]. Depending on how that resolves, it might make
sense to stay with `get` or rename to something else.
Best regards,
Andreas Hindborg
[1] https://lore.kernel.org/all/35e1fef4-b715-4827-a498-bdde9b58b51c@penguintechs.org
[2] https://lore.kernel.org/all/CAH5fLggY2Ei14nVJzLBEoR1Rut1GKU4SZX=+14tuRH1aSuQVTA@mail.gmail.com
On Fri Jul 4, 2025 at 9:37 AM CEST, Andreas Hindborg wrote:
> Andreas Hindborg <a.hindborg@kernel.org> writes:
>
>> "Danilo Krummrich" <dakr@kernel.org> writes:
>>
>>> On 7/2/25 3:18 PM, Andreas Hindborg wrote:
>>>> + /// Get a shared reference to the parameter value.
>>>> + // Note: When sysfs access to parameters are enabled, we have to pass in a
>>>> + // held lock guard here.
>>>> + pub fn get(&self) -> &T {
>>>> + self.value.as_ref().unwrap_or(&self.default)
>>>> + }
>>>
>>> I think you forgot to rename this.
>>
>> Yes, thanks for being persistent on this :)
>
> Actually, there is a discussion on whether to keep the API similar to
> `std::sync::OnceLock` [1] but also whether to rename this to something
> other than `OnceLock` [2]. Depending on how that resolves, it might make
> sense to stay with `get` or rename to something else.
But this is for the `ModuleParamAccess`, right? There I think it makes
sense to choose `access` or `value`.
---
Cheers,
Benno
> Best regards,
> Andreas Hindborg
>
>
> [1] https://lore.kernel.org/all/35e1fef4-b715-4827-a498-bdde9b58b51c@penguintechs.org
> [2] https://lore.kernel.org/all/CAH5fLggY2Ei14nVJzLBEoR1Rut1GKU4SZX=+14tuRH1aSuQVTA@mail.gmail.com
"Benno Lossin" <lossin@kernel.org> writes:
> On Fri Jul 4, 2025 at 9:37 AM CEST, Andreas Hindborg wrote:
>> Andreas Hindborg <a.hindborg@kernel.org> writes:
>>
>>> "Danilo Krummrich" <dakr@kernel.org> writes:
>>>
>>>> On 7/2/25 3:18 PM, Andreas Hindborg wrote:
>>>>> + /// Get a shared reference to the parameter value.
>>>>> + // Note: When sysfs access to parameters are enabled, we have to pass in a
>>>>> + // held lock guard here.
>>>>> + pub fn get(&self) -> &T {
>>>>> + self.value.as_ref().unwrap_or(&self.default)
>>>>> + }
>>>>
>>>> I think you forgot to rename this.
>>>
>>> Yes, thanks for being persistent on this :)
>>
>> Actually, there is a discussion on whether to keep the API similar to
>> `std::sync::OnceLock` [1] but also whether to rename this to something
>> other than `OnceLock` [2]. Depending on how that resolves, it might make
>> sense to stay with `get` or rename to something else.
>
> But this is for the `ModuleParamAccess`, right? There I think it makes
> sense to choose `access` or `value`.
Right, sorry. My context window was too small here.
Best regards,
Andreas Hindborg
On Wed Jul 2, 2025 at 3:18 PM CEST, Andreas Hindborg wrote:
> Add types and traits for interfacing the C moduleparam API.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
I have some nits below, but overall
Reviewed-by: Benno Lossin <lossin@kernel.org>
> ---
> rust/kernel/lib.rs | 1 +
> rust/kernel/module_param.rs | 191 ++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 192 insertions(+)
I really like how the `OnceLock` usage turned out here! Thanks for the
quick impl!
>
> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
> index 6b4774b2b1c3..2b439ea06185 100644
> --- a/rust/kernel/lib.rs
> +++ b/rust/kernel/lib.rs
> @@ -87,6 +87,7 @@
> pub mod list;
> pub mod miscdevice;
> pub mod mm;
> +pub mod module_param;
> #[cfg(CONFIG_NET)]
> pub mod net;
> pub mod of;
> diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
> new file mode 100644
> index 000000000000..ca4be7e45ff7
> --- /dev/null
> +++ b/rust/kernel/module_param.rs
> @@ -0,0 +1,191 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! Support for module parameters.
> +//!
> +//! C header: [`include/linux/moduleparam.h`](srctree/include/linux/moduleparam.h)
> +
> +use crate::prelude::*;
> +use crate::str::BStr;
> +use bindings;
> +use kernel::sync::once_lock::OnceLock;
> +
> +/// Newtype to make `bindings::kernel_param` [`Sync`].
> +#[repr(transparent)]
> +#[doc(hidden)]
> +pub struct RacyKernelParam(bindings::kernel_param);
Can you remind me why this is called `Racy`? Maybe add the explainer in
a comment? (and if it's named racy, why is it okay?)
If it doesn't have a real reason, maybe it should be called
`KernelParam`?
> +
> +impl RacyKernelParam {
> + #[doc(hidden)]
> + pub const fn new(val: bindings::kernel_param) -> Self {
> + Self(val)
> + }
> +}
> +
> +// SAFETY: C kernel handles serializing access to this type. We never access it
> +// from Rust module.
> +unsafe impl Sync for RacyKernelParam {}
> +
> +/// Types that can be used for module parameters.
> +// NOTE: This trait is `Copy` because drop could produce unsoundness during teardown.
> +pub trait ModuleParam: Sized + Copy {
> + /// The [`ModuleParam`] will be used by the kernel module through this type.
> + ///
> + /// This may differ from `Self` if, for example, `Self` needs to track
> + /// ownership without exposing it or allocate extra space for other possible
> + /// parameter values.
> + // This is required to support string parameters in the future.
> + type Value: ?Sized;
This isn't used anywhere in the patchset and AFAIK the kernel is moving
away from module params, so I'm not so sure if we're going to have
strings as params.
Or do you already have those patches ready/plan to use strings? If not,
then I think we should just remove this type and when we actually need
them add it.
> +
> + /// Parse a parameter argument into the parameter value.
> + fn try_from_param_arg(arg: &BStr) -> Result<Self>;
> +}
> +
> +impl<T> ModuleParamAccess<T> {
> + #[doc(hidden)]
> + pub const fn new(default: T) -> Self {
> + Self {
> + value: OnceLock::new(),
> + default,
> + }
> + }
> +
> + /// Get a shared reference to the parameter value.
> + // Note: When sysfs access to parameters are enabled, we have to pass in a
> + // held lock guard here.
> + pub fn get(&self) -> &T {
> + self.value.as_ref().unwrap_or(&self.default)
> + }
> +
> + /// Get a mutable pointer to `self`.
> + ///
> + /// NOTE: In most cases it is not safe deref the returned pointer.
> + pub const fn as_void_ptr(&self) -> *mut c_void {
> + (self as *const Self).cast_mut().cast()
There is `core::ptr::from_ref` that we should use instead of the `as`
cast.
---
Cheers,
Benno
> + }
> +}
> +
> +#[doc(hidden)]
> +/// Generate a static [`kernel_param_ops`](srctree/include/linux/moduleparam.h) struct.
> +///
> +/// # Examples
> +///
> +/// ```ignore
> +/// make_param_ops!(
> +/// /// Documentation for new param ops.
> +/// PARAM_OPS_MYTYPE, // Name for the static.
> +/// MyType // A type which implements [`ModuleParam`].
> +/// );
> +/// ```
> +macro_rules! make_param_ops {
> + ($ops:ident, $ty:ty) => {
> + #[doc(hidden)]
> + pub static $ops: $crate::bindings::kernel_param_ops = $crate::bindings::kernel_param_ops {
> + flags: 0,
> + set: Some(set_param::<$ty>),
> + get: None,
> + free: None,
> + };
> + };
> +}
> +
> +make_param_ops!(PARAM_OPS_I8, i8);
> +make_param_ops!(PARAM_OPS_U8, u8);
> +make_param_ops!(PARAM_OPS_I16, i16);
> +make_param_ops!(PARAM_OPS_U16, u16);
> +make_param_ops!(PARAM_OPS_I32, i32);
> +make_param_ops!(PARAM_OPS_U32, u32);
> +make_param_ops!(PARAM_OPS_I64, i64);
> +make_param_ops!(PARAM_OPS_U64, u64);
> +make_param_ops!(PARAM_OPS_ISIZE, isize);
> +make_param_ops!(PARAM_OPS_USIZE, usize);
"Benno Lossin" <lossin@kernel.org> writes:
> On Wed Jul 2, 2025 at 3:18 PM CEST, Andreas Hindborg wrote:
>> Add types and traits for interfacing the C moduleparam API.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> I have some nits below, but overall
>
> Reviewed-by: Benno Lossin <lossin@kernel.org>
>
>> ---
>> rust/kernel/lib.rs | 1 +
>> rust/kernel/module_param.rs | 191 ++++++++++++++++++++++++++++++++++++++++++++
>> 2 files changed, 192 insertions(+)
>
> I really like how the `OnceLock` usage turned out here! Thanks for the
> quick impl!
>
>>
>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>> index 6b4774b2b1c3..2b439ea06185 100644
>> --- a/rust/kernel/lib.rs
>> +++ b/rust/kernel/lib.rs
>> @@ -87,6 +87,7 @@
>> pub mod list;
>> pub mod miscdevice;
>> pub mod mm;
>> +pub mod module_param;
>> #[cfg(CONFIG_NET)]
>> pub mod net;
>> pub mod of;
>> diff --git a/rust/kernel/module_param.rs b/rust/kernel/module_param.rs
>> new file mode 100644
>> index 000000000000..ca4be7e45ff7
>> --- /dev/null
>> +++ b/rust/kernel/module_param.rs
>> @@ -0,0 +1,191 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Support for module parameters.
>> +//!
>> +//! C header: [`include/linux/moduleparam.h`](srctree/include/linux/moduleparam.h)
>> +
>> +use crate::prelude::*;
>> +use crate::str::BStr;
>> +use bindings;
>> +use kernel::sync::once_lock::OnceLock;
>> +
>> +/// Newtype to make `bindings::kernel_param` [`Sync`].
>> +#[repr(transparent)]
>> +#[doc(hidden)]
>> +pub struct RacyKernelParam(bindings::kernel_param);
>
> Can you remind me why this is called `Racy`? Maybe add the explainer in
> a comment? (and if it's named racy, why is it okay?)
>
> If it doesn't have a real reason, maybe it should be called
> `KernelParam`?
It is an inherited name from way back. The type exists to allow a static
`bindings::kernel_param`, as this C type is not `Sync`.
I agree, it should just be `KernelParam`.
>
>> +
>> +impl RacyKernelParam {
>> + #[doc(hidden)]
>> + pub const fn new(val: bindings::kernel_param) -> Self {
>> + Self(val)
>> + }
>> +}
>> +
>> +// SAFETY: C kernel handles serializing access to this type. We never access it
>> +// from Rust module.
>> +unsafe impl Sync for RacyKernelParam {}
>> +
>> +/// Types that can be used for module parameters.
>> +// NOTE: This trait is `Copy` because drop could produce unsoundness during teardown.
>> +pub trait ModuleParam: Sized + Copy {
>> + /// The [`ModuleParam`] will be used by the kernel module through this type.
>> + ///
>> + /// This may differ from `Self` if, for example, `Self` needs to track
>> + /// ownership without exposing it or allocate extra space for other possible
>> + /// parameter values.
>> + // This is required to support string parameters in the future.
>> + type Value: ?Sized;
>
> This isn't used anywhere in the patchset and AFAIK the kernel is moving
> away from module params, so I'm not so sure if we're going to have
> strings as params.
The kernel dropping module parameters depends on who you ask. But
regardless, we should probably remove it for now.
>
> Or do you already have those patches ready/plan to use strings? If not,
> then I think we should just remove this type and when we actually need
> them add it.
They are in the old rust branch and they need some work. I do not have a
user for them, which is why I am not including them in this series.
>
>> +
>> + /// Parse a parameter argument into the parameter value.
>> + fn try_from_param_arg(arg: &BStr) -> Result<Self>;
>> +}
>> +
>
>> +impl<T> ModuleParamAccess<T> {
>> + #[doc(hidden)]
>> + pub const fn new(default: T) -> Self {
>> + Self {
>> + value: OnceLock::new(),
>> + default,
>> + }
>> + }
>> +
>> + /// Get a shared reference to the parameter value.
>> + // Note: When sysfs access to parameters are enabled, we have to pass in a
>> + // held lock guard here.
>> + pub fn get(&self) -> &T {
>> + self.value.as_ref().unwrap_or(&self.default)
>> + }
>> +
>> + /// Get a mutable pointer to `self`.
>> + ///
>> + /// NOTE: In most cases it is not safe deref the returned pointer.
>> + pub const fn as_void_ptr(&self) -> *mut c_void {
>> + (self as *const Self).cast_mut().cast()
>
> There is `core::ptr::from_ref` that we should use instead of the `as`
> cast.
Cool 👍
Best regards,
Andreas Hindborg
On Fri, Jul 4, 2025 at 1:45 PM Andreas Hindborg <a.hindborg@kernel.org> wrote: > > It is an inherited name from way back. Yeah, that name comes from one of the very first PRs. I think we may have discussed its name in one of the early calls or maybe I just came up with the name. Either way, it is almost 5 years old so I would suggest taking a look at naming etc. with fresh eyes. Cheers, Miguel
© 2016 - 2025 Red Hat, Inc.