"Gary Guo" <gary@xxxxxxxxxxx> writes:
> On Tue, 18 Feb 2025 14:00:48 +0100
> Andreas Hindborg <a.hindborg@xxxxxxxxxx> wrote:
>
>> This patch includes changes required for Rust kernel modules to utilize
>> module parameters. This code implements read only support for integer
>> types without `sysfs` support.
>>
>> Signed-off-by: Andreas Hindborg <a.hindborg@xxxxxxxxxx>
>> Acked-by: Petr Pavlu <petr.pavlu@xxxxxxxx> # from modules perspective
>> ---
>> rust/kernel/lib.rs | 1 +
>> rust/kernel/module_param.rs | 226 +++++++++++++++++++++++++++++++++++++++++++
>> rust/macros/helpers.rs | 25 +++++
>> rust/macros/lib.rs | 31 ++++++
>> rust/macros/module.rs | 191 ++++++++++++++++++++++++++++++++----
>> samples/rust/rust_minimal.rs | 10 ++
>> 6 files changed, 466 insertions(+), 18 deletions(-)
>>
>> diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
>> index 496ed32b0911a..aec04df2bac9f 100644
>> --- a/rust/kernel/lib.rs
>> +++ b/rust/kernel/lib.rs
>> @@ -57,6 +57,7 @@
>> pub mod kunit;
>> pub mod list;
>> pub mod miscdevice;
>> +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 0000000000000..0047126c917f4
>> --- /dev/null
>> +++ b/rust/kernel/module_param.rs
>> @@ -0,0 +1,226 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +
>> +//! Types for module parameters.
>> +//!
>> +//! C header: [`include/linux/moduleparam.h`](srctree/include/linux/moduleparam.h)
>> +
>> +use crate::prelude::*;
>> +use crate::str::BStr;
>> +
>> +/// Newtype to make `bindings::kernel_param` [`Sync`].
>> +#[repr(transparent)]
>> +#[doc(hidden)]
>> +pub struct RacyKernelParam(pub ::kernel::bindings::kernel_param);
>> +
>> +// SAFETY: C kernel handles serializing access to this type. We never access
>> +// from Rust module.
>> +unsafe impl Sync for RacyKernelParam {}
>
> I wonder if we should have a custom impl of `SyncUnsafeCell` for this
> kind of usage (so that when it is stabilized by Rust, we can just switc
> over).
We discussed this before, I don't recall what we decided. At any rate,
it's orthogonal. We can patch this if we add `SyncUnsafeCell`.
>
>> +
>> +/// Types that can be used for module parameters.
>> +///
>> +/// Note that displaying the type in `sysfs` will fail if
>> +/// [`Display`](core::fmt::Display) implementation would write more than
>> +/// [`PAGE_SIZE`] - 1 bytes.
>> +///
>> +/// [`PAGE_SIZE`]: `bindings::PAGE_SIZE`
>> +pub trait ModuleParam: Sized {
>> + /// 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.
>> + ///
>> + /// `Err(_)` should be returned when parsing of the argument fails.
>> + ///
>> + /// Parameters passed at boot time will be set before [`kmalloc`] is
>> + /// available (even if the module is loaded at a later time). However, in
>> + /// this case, the argument buffer will be valid for the entire lifetime of
>> + /// the kernel. So implementations of this method which need to allocate
>> + /// should first check that the allocator is available (with
>> + /// [`crate::bindings::slab_is_available`]) and when it is not available
>> + /// provide an alternative implementation which doesn't allocate. In cases
>> + /// where the allocator is not available it is safe to save references to
>> + /// `arg` in `Self`, but in other cases a copy should be made.
>> + ///
>> + /// [`kmalloc`]: srctree/include/linux/slab.h
>> + fn try_from_param_arg(arg: &'static [u8]) -> 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`.
>> +///
>> +/// `param.arg` is a pointer to `*mut T` as set up by the [`module!`]
>> +/// macro.
>> +///
>> +/// See `struct kernel_param_ops.set`.
>> +///
>> +/// # Safety
>> +///
>> +/// If `val` is non-null then it must point to a valid null-terminated
>> +/// string. The `arg` field of `param` must be an instance of `T`.
>> +///
>> +/// # Invariants
>> +///
>> +/// 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 kernel::ffi::c_char,
>> + param: *const crate::bindings::kernel_param,
>> +) -> core::ffi::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 crate::error::code::EINVAL.to_errno();
>
> This is already in prelude, so you can just use `EINVAL` directly.
Thanks.
>
>> + }
>> +
>> + // SAFETY: By function safety requirement, val is non-null and
>> + // null-terminated. By C API contract, `val` is live and valid for reads
>> + // for the duration of this function.
>> + let arg = unsafe { CStr::from_char_ptr(val).as_bytes() };
>> +
>> + crate::error::from_result(|| {
>> + let new_value = T::try_from_param_arg(arg)?;
>> +
>> + // SAFETY: `param` is guaranteed to be valid by C API contract
>> + // and `arg` is guaranteed to point to an instance of `T`.
>> + let old_value = unsafe { (*param).__bindgen_anon_1.arg as *mut T };
>> +
>> + // SAFETY: `old_value` is valid for writes, as we have exclusive
>> + // access. `old_value` is pointing to an initialized static, and
>> + // so it is properly initialized.
>> + unsafe { core::ptr::replace(old_value, new_value) };
>> + Ok(0)
>> + })
>> +}
>> +
>> +/// Drop the parameter.
>> +///
>> +/// Called when unloading a module.
>> +///
>> +/// # Safety
>> +///
>> +/// The `arg` field of `param` must be an initialized instance of `T`.
>> +unsafe extern "C" fn free<T>(arg: *mut core::ffi::c_void)
>> +where
>> + T: ModuleParam,
>> +{
>> + // SAFETY: By function safety requirement, `arg` is an initialized
>> + // instance of `T`. By C API contract, `arg` will not be used after
>> + // this function returns.
>> + unsafe { core::ptr::drop_in_place(arg as *mut T) };
>> +}
>> +
>> +macro_rules! impl_int_module_param {
>> + ($ty:ident) => {
>> + impl ModuleParam for $ty {
>> + type Value = $ty;
>> +
>> + fn try_from_param_arg(arg: &'static [u8]) -> Result<Self> {
>> + let bstr = BStr::from_bytes(arg);
>
> Why isn't `arg` BStr in the first place?
I'll change it.
Best regards,
Andreas Hindborg
