rust-for-linux.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support
@ 2025-02-27 14:38 Andreas Hindborg
  2025-02-27 14:38 ` [PATCH v8 1/7] rust: str: implement `PartialEq` for `BStr` Andreas Hindborg
                   ` (8 more replies)
  0 siblings, 9 replies; 19+ messages in thread
From: Andreas Hindborg @ 2025-02-27 14:38 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier, Luis Chamberlain
  Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
	linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
	linux-modules, Andreas Hindborg

Extend the `module!` macro with support module parameters. Also add some string
to integer parsing functions and updates `BStr` with a method to strip a string
prefix.

Based on code by Adam Bratschi-Kaye lifted from the original `rust` branch [1].

Link: https://github.com/Rust-for-Linux/linux/tree/bc22545f38d74473cfef3e9fd65432733435b79f [1]
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
Changes in v8:
- Change print statement in sample to better communicate parameter name.
- Use imperative mode in commit messages.
- Remove prefix path from `EINVAL`.
- Change `try_from_param_arg` to accept `&BStr` rather than `&[u8]`.
- Parse integers without 128 bit integer types.
- Seal trait `FromStrRadix`.
- Strengthen safety requirement of `set_param`.
- Remove comment about Display and `PAGE_SIZE`.
- Add note describing why `ModuleParamAccess` is pub.
- Typo and grammar fixes for documentation.
- Update MAINTAINERS with rust module files.
- Link to v7: https://lore.kernel.org/r/20250218-module-params-v3-v7-0-5e1afabcac1b@kernel.org

Changes in v7:
- Remove dependency on `pr_warn_once` patches, replace with TODO.
- Rework `ParseInt::from_str` to avoid allocating.
- Add a comment explaining how we parse "0".
- Change trait bound on `Index` impl for `BStr` to match std library approach.
- Link to v6: https://lore.kernel.org/r/20250211-module-params-v3-v6-0-24b297ddc43d@kernel.org

Changes in v6:
- Fix a bug that prevented parsing of negative default values for
  parameters in the `module!` macro.
- Fix a bug that prevented parsing zero in `strip_radix`. Also add a
  test case for this.
- Add `AsRef<BStr>` for `[u8]` and `BStr`.
- Use `impl AsRef<BStr>` as type of prefix in `BStr::strip_prefix`.
- Link to v5: https://lore.kernel.org/r/20250204-module-params-v3-v5-0-bf5ec2041625@kernel.org

Changes in v5:
- Fix a typo in a safety comment in `set_param`.
- Use a match statement in `parse_int::strip_radix`.
- Add an implementation of `Index` for `BStr`.
- Fix a logic inversion bug where parameters would not be parsed.
- Use `kernel::ffi::c_char` in `set_param` rather than the one in `core`.
- Use `kernel::c_str!` rather than `c"..."` literal in module macro.
- Rebase on v6.14-rc1.
- Link to v4: https://lore.kernel.org/r/20250109-module-params-v3-v4-0-c208bcfbe11f@kernel.org

Changes in v4:
- Add module maintainers to Cc list (sorry)
- Add a few missing [`doc_links`]
- Add panic section to `expect_string_field`
- Fix a typo in safety requirement of `module_params::free`
- Change `assert!` to `pr_warn_once!` in `module_params::set_param`
- Remove `module_params::get_param` and install null pointer instead
- Remove use of the unstable feature `sync_unsafe_cell`
- Link to v3: https://lore.kernel.org/r/20241213-module-params-v3-v3-0-485a015ac2cf@kernel.org

Changes in v3:
- use `SyncUnsafeCell` rather than `static mut` and simplify parameter access
- remove `Display` bound from `ModuleParam`
- automatically generate documentation for `PARAM_OPS_.*`
- remove `as *const _ as *mut_` phrasing
- inline parameter name in struct instantiation in  `emit_params`
- move `RacyKernelParam` out of macro template
- use C string literals rather than byte string literals with explicit null
- template out `__{name}_{param_name}` in `emit_param`
- indent template in `emit_params`
- use let-else expression in `emit_params` to get rid of an indentation level
- document `expect_string_field`
- move invication of `impl_int_module_param` to be closer to macro def
- move attributes after docs in `make_param_ops`
- rename `impl_module_param` to impl_int_module_param`
- use `ty` instead of `ident` in `impl_parse_int`
- use `BStr` instead of `&str` for string manipulation
- move string parsing functions to seperate patch and add examples, fix bugs
- degrade comment about future support from doc comment to regular comment
- remove std lib path from `Sized` marker
- update documentation for `trait ModuleParam`
- Link to v2: https://lore.kernel.org/all/20240819133345.3438739-1-nmi@metaspace.dk/

Changes in v2:
- Remove support for params without values (`NOARG_ALLOWED`).
- Improve documentation for `try_from_param_arg`.
- Use prelude import.
- Refactor `try_from_param_arg` to return `Result`.
- Refactor `ParseInt::from_str` to return `Result`.
- Move C callable functions out of `ModuleParam` trait.
- Rename literal string field parser to `expect_string_field`.
- Move parameter parsing from generation to parsing stage.
- Use absolute type paths in macro code.
- Inline `kparam`and `read_func` values.
- Resolve TODO regarding alignment attributes.
- Remove unnecessary unsafe blocks in macro code.
- Improve error message for unrecognized parameter types.
- Do not use `self` receiver when reading parameter value.
- Add parameter documentation to `module!` macro.
- Use empty `enum` for parameter type.
- Use `addr_of_mut` to get address of parameter value variable.
- Enabled building of docs for for `module_param` module.
- Link to v1: https://lore.kernel.org/rust-for-linux/20240705111455.142790-1-nmi@metaspace.dk/

---
Andreas Hindborg (7):
      rust: str: implement `PartialEq` for `BStr`
      rust: str: implement `Index` for `BStr`
      rust: str: implement `AsRef<BStr>` for `[u8]` and `BStr`
      rust: str: implement `strip_prefix` for `BStr`
      rust: str: add radix prefixed integer parsing functions
      rust: add parameter support to the `module!` macro
      modules: add rust modules files to MAINTAINERS

 MAINTAINERS                  |   2 +
 rust/kernel/lib.rs           |   1 +
 rust/kernel/module_param.rs  | 221 +++++++++++++++++++++++++++++++++++++++++++
 rust/kernel/str.rs           | 200 +++++++++++++++++++++++++++++++++++++++
 rust/macros/helpers.rs       |  25 +++++
 rust/macros/lib.rs           |  31 ++++++
 rust/macros/module.rs        | 191 +++++++++++++++++++++++++++++++++----
 samples/rust/rust_minimal.rs |  10 ++
 8 files changed, 663 insertions(+), 18 deletions(-)
---
base-commit: 379487e17ca406b47392e7ab6cf35d1c3bacb371
change-id: 20241211-module-params-v3-ae7e5c8d8b5a

Best regards,
-- 
Andreas Hindborg <a.hindborg@kernel.org>



^ permalink raw reply	[flat|nested] 19+ messages in thread

* [PATCH v8 1/7] rust: str: implement `PartialEq` for `BStr`
  2025-02-27 14:38 [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support Andreas Hindborg
@ 2025-02-27 14:38 ` Andreas Hindborg
  2025-03-04 11:07   ` Fiona Behrens
  2025-02-27 14:38 ` [PATCH v8 2/7] rust: str: implement `Index` " Andreas Hindborg
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Andreas Hindborg @ 2025-02-27 14:38 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier, Luis Chamberlain
  Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
	linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
	linux-modules, Andreas Hindborg

Implement `PartialEq` for `BStr` by comparing underlying byte slices.

Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/str.rs | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 28e2201604d6..002dcddf7c76 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -108,6 +108,12 @@ fn deref(&self) -> &Self::Target {
     }
 }
 
+impl PartialEq for BStr {
+    fn eq(&self, other: &Self) -> bool {
+        self.deref().eq(other.deref())
+    }
+}
+
 /// Creates a new [`BStr`] from a string literal.
 ///
 /// `b_str!` converts the supplied string literal to byte string, so non-ASCII

-- 
2.47.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v8 2/7] rust: str: implement `Index` for `BStr`
  2025-02-27 14:38 [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support Andreas Hindborg
  2025-02-27 14:38 ` [PATCH v8 1/7] rust: str: implement `PartialEq` for `BStr` Andreas Hindborg
@ 2025-02-27 14:38 ` Andreas Hindborg
  2025-02-28  9:10   ` Alice Ryhl
  2025-02-27 14:38 ` [PATCH v8 3/7] rust: str: implement `AsRef<BStr>` for `[u8]` and `BStr` Andreas Hindborg
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Andreas Hindborg @ 2025-02-27 14:38 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier, Luis Chamberlain
  Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
	linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
	linux-modules, Andreas Hindborg

The `Index` implementation on `BStr` was lost when we switched `BStr` from
a type alias of `[u8]` to a newtype. Add back `Index` by implementing
`Index` for `BStr` when `Index` would be implemented for `[u8]`.

Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Fiona Behrens <me@kloenk.dev>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/str.rs | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index 002dcddf7c76..ba6b1a5c4f99 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -114,6 +114,17 @@ fn eq(&self, other: &Self) -> bool {
     }
 }
 
+impl<Idx> Index<Idx> for BStr
+where
+    [u8]: Index<Idx, Output = [u8]>,
+{
+    type Output = Self;
+
+    fn index(&self, index: Idx) -> &Self::Output {
+        BStr::from_bytes(&self.0[index])
+    }
+}
+
 /// Creates a new [`BStr`] from a string literal.
 ///
 /// `b_str!` converts the supplied string literal to byte string, so non-ASCII

-- 
2.47.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v8 3/7] rust: str: implement `AsRef<BStr>` for `[u8]` and `BStr`
  2025-02-27 14:38 [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support Andreas Hindborg
  2025-02-27 14:38 ` [PATCH v8 1/7] rust: str: implement `PartialEq` for `BStr` Andreas Hindborg
  2025-02-27 14:38 ` [PATCH v8 2/7] rust: str: implement `Index` " Andreas Hindborg
@ 2025-02-27 14:38 ` Andreas Hindborg
  2025-03-04 11:09   ` Fiona Behrens
  2025-02-27 14:38 ` [PATCH v8 4/7] rust: str: implement `strip_prefix` for `BStr` Andreas Hindborg
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Andreas Hindborg @ 2025-02-27 14:38 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier, Luis Chamberlain
  Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
	linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
	linux-modules, Andreas Hindborg

Implement `AsRef<BStr>` for `[u8]` and `BStr` so these can be used
interchangeably for operations on `BStr`.

Reviewed-by: Gary Guo <gary@garyguo.net>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/str.rs | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index ba6b1a5c4f99..c6bd2c69543d 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -125,6 +125,18 @@ fn index(&self, index: Idx) -> &Self::Output {
     }
 }
 
+impl AsRef<BStr> for [u8] {
+    fn as_ref(&self) -> &BStr {
+        BStr::from_bytes(self)
+    }
+}
+
+impl AsRef<BStr> for BStr {
+    fn as_ref(&self) -> &BStr {
+        self
+    }
+}
+
 /// Creates a new [`BStr`] from a string literal.
 ///
 /// `b_str!` converts the supplied string literal to byte string, so non-ASCII

-- 
2.47.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v8 4/7] rust: str: implement `strip_prefix` for `BStr`
  2025-02-27 14:38 [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support Andreas Hindborg
                   ` (2 preceding siblings ...)
  2025-02-27 14:38 ` [PATCH v8 3/7] rust: str: implement `AsRef<BStr>` for `[u8]` and `BStr` Andreas Hindborg
@ 2025-02-27 14:38 ` Andreas Hindborg
  2025-02-27 14:38 ` [PATCH v8 5/7] rust: str: add radix prefixed integer parsing functions Andreas Hindborg
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Andreas Hindborg @ 2025-02-27 14:38 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier, Luis Chamberlain
  Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
	linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
	linux-modules, Andreas Hindborg

Implement `strip_prefix` for `BStr` by deferring to `slice::strip_prefix`
on the underlying `&[u8]`.

Reviewed-by: Gary Guo <gary@garyguo.net>
Reviewed-by: Alice Ryhl <aliceryhl@google.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---

It is also possible to get this method by implementing
`core::slice::SlicePattern` for `BStr`. `SlicePattern` is unstable, so this
seems more reasonable.
---
 rust/kernel/str.rs | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index c6bd2c69543d..db272d2198fc 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -31,6 +31,22 @@ pub const fn from_bytes(bytes: &[u8]) -> &Self {
         // SAFETY: `BStr` is transparent to `[u8]`.
         unsafe { &*(bytes as *const [u8] as *const BStr) }
     }
+
+    /// Strip a prefix from `self`. Delegates to [`slice::strip_prefix`].
+    ///
+    /// # Example
+    /// ```
+    /// use kernel::b_str;
+    /// assert_eq!(Some(b_str!("bar")), b_str!("foobar").strip_prefix(b_str!("foo")));
+    /// assert_eq!(None, b_str!("foobar").strip_prefix(b_str!("bar")));
+    /// assert_eq!(Some(b_str!("foobar")), b_str!("foobar").strip_prefix(b_str!("")));
+    /// assert_eq!(Some(b_str!("")), b_str!("foobar").strip_prefix(b_str!("foobar")));
+    /// ```
+    pub fn strip_prefix(&self, pattern: impl AsRef<Self>) -> Option<&BStr> {
+        self.deref()
+            .strip_prefix(pattern.as_ref().deref())
+            .map(Self::from_bytes)
+    }
 }
 
 impl fmt::Display for BStr {

-- 
2.47.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v8 5/7] rust: str: add radix prefixed integer parsing functions
  2025-02-27 14:38 [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support Andreas Hindborg
                   ` (3 preceding siblings ...)
  2025-02-27 14:38 ` [PATCH v8 4/7] rust: str: implement `strip_prefix` for `BStr` Andreas Hindborg
@ 2025-02-27 14:38 ` Andreas Hindborg
  2025-03-20 20:21   ` Miguel Ojeda
  2025-02-27 14:38 ` [PATCH v8 6/7] rust: add parameter support to the `module!` macro Andreas Hindborg
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 19+ messages in thread
From: Andreas Hindborg @ 2025-02-27 14:38 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier, Luis Chamberlain
  Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
	linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
	linux-modules, Andreas Hindborg

Add the trait `ParseInt` for parsing string representations of integers
where the string representations are optionally prefixed by a radix
specifier. Implement the trait for the primitive integer types.

Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/str.rs | 155 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 155 insertions(+)

diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index db272d2198fc..a708cf219348 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -945,3 +945,158 @@ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
 macro_rules! fmt {
     ($($f:tt)*) => ( core::format_args!($($f)*) )
 }
+
+pub mod parse_int {
+    //! Integer parsing functions for parsing signed and unsigned integers
+    //! potentially prefixed with `0x`, `0o`, or `0b`.
+
+    use crate::prelude::*;
+    use crate::str::BStr;
+    use core::ops::Deref;
+
+    // Make `FromStrRadix` a public type with a private name. This seals
+    // `ParseInt`, that is, prevents downstream users from implementing the
+    // trait.
+    mod private {
+        use crate::str::BStr;
+
+        /// Trait that allows parsing a [`&BStr`] to an integer with a radix.
+        ///
+        /// [`&BStr`]: kernel::str::BStr
+        // This is required because the `from_str_radix` function on the primitive
+        // integer types is not part of any trait.
+        pub trait FromStrRadix: Sized {
+            /// Parse `src` to `Self` using radix `radix`.
+            fn from_str_radix(src: &BStr, radix: u32) -> Result<Self, crate::error::Error>;
+
+            /// Return the absolute value of Self::MIN.
+            fn abs_min() -> u64;
+
+            /// Perform bitwise 2's complement on `self`.
+            ///
+            /// Note: This function does not make sense for unsigned integers.
+            fn complement(self) -> Self;
+        }
+    }
+
+    /// Extract the radix from an integer literal optionally prefixed with
+    /// one of `0x`, `0X`, `0o`, `0O`, `0b`, `0B`, `0`.
+    fn strip_radix(src: &BStr) -> (u32, &BStr) {
+        match src.deref() {
+            [b'0', b'x' | b'X', rest @ ..] => (16, rest.as_ref()),
+            [b'0', b'o' | b'O', rest @ ..] => (8, rest.as_ref()),
+            [b'0', b'b' | b'B', rest @ ..] => (2, rest.as_ref()),
+            // NOTE: We are including the leading zero to be able to parse
+            // literal 0 here. If we removed it as a radix prefix, we would not
+            // be able to parse `0`.
+            [b'0', ..] => (8, src),
+            _ => (10, src),
+        }
+    }
+
+    /// Trait for parsing string representations of integers.
+    ///
+    /// Strings beginning with `0x`, `0o`, or `0b` are parsed as hex, octal, or
+    /// binary respectively. Strings beginning with `0` otherwise are parsed as
+    /// octal. Anything else is parsed as decimal. A leading `+` or `-` is also
+    /// permitted. Any string parsed by [`kstrtol()`] or [`kstrtoul()`] will be
+    /// successfully parsed.
+    ///
+    /// [`kstrtol()`]: https://www.kernel.org/doc/html/latest/core-api/kernel-api.html#c.kstrtol
+    /// [`kstrtoul()`]: https://www.kernel.org/doc/html/latest/core-api/kernel-api.html#c.kstrtoul
+    ///
+    /// # Example
+    /// ```
+    /// use kernel::str::parse_int::ParseInt;
+    /// use kernel::b_str;
+    ///
+    /// assert_eq!(Ok(0), u8::from_str(b_str!("0")));
+    ///
+    /// assert_eq!(Ok(0xa2u8), u8::from_str(b_str!("0xa2")));
+    /// assert_eq!(Ok(-0xa2i32), i32::from_str(b_str!("-0xa2")));
+    ///
+    /// assert_eq!(Ok(-0o57i8), i8::from_str(b_str!("-0o57")));
+    /// assert_eq!(Ok(0o57i8), i8::from_str(b_str!("057")));
+    ///
+    /// assert_eq!(Ok(0b1001i16), i16::from_str(b_str!("0b1001")));
+    /// assert_eq!(Ok(-0b1001i16), i16::from_str(b_str!("-0b1001")));
+    ///
+    /// assert_eq!(Ok(127), i8::from_str(b_str!("127")));
+    /// assert!(i8::from_str(b_str!("128")).is_err());
+    /// assert_eq!(Ok(-128), i8::from_str(b_str!("-128")));
+    /// assert!(i8::from_str(b_str!("-129")).is_err());
+    /// assert_eq!(Ok(255), u8::from_str(b_str!("255")));
+    /// assert!(u8::from_str(b_str!("256")).is_err());
+    /// ```
+    pub trait ParseInt: private::FromStrRadix + TryFrom<u64> {
+        /// Parse a string according to the description in [`Self`].
+        fn from_str(src: &BStr) -> Result<Self> {
+            match src.deref() {
+                [b'-', rest @ ..] => {
+                    let (radix, digits) = strip_radix(rest.as_ref());
+                    // 2's complement values range from -2^(b-1) to 2^(b-1)-1.
+                    // So if we want to parse negative numbers as positive and
+                    // later multiply by -1, we have to parse into a larger
+                    // integer. We choose u64 as sufficiently large. NOTE: 128
+                    // bit integers are not available on all platforms, hence
+                    // the choice of 64 bit.
+                    let val = u64::from_str_radix(
+                        core::str::from_utf8(digits).map_err(|_| EINVAL)?,
+                        radix,
+                    )
+                    .map_err(|_| EINVAL)?;
+
+                    if val > Self::abs_min() {
+                        return Err(EINVAL);
+                    }
+
+                    // SAFETY: We checked that `val` will fit in `Self` above.
+                    let val: Self = unsafe { val.try_into().unwrap_unchecked() };
+
+                    Ok(val.complement())
+                }
+                _ => {
+                    let (radix, digits) = strip_radix(src);
+                    Self::from_str_radix(digits, radix).map_err(|_| EINVAL)
+                }
+            }
+        }
+    }
+
+    macro_rules! impl_parse_int {
+        ($ty:ty) => {
+            impl private::FromStrRadix for $ty {
+                fn from_str_radix(src: &BStr, radix: u32) -> Result<Self, crate::error::Error> {
+                    <$ty>::from_str_radix(core::str::from_utf8(src).map_err(|_| EINVAL)?, radix)
+                        .map_err(|_| EINVAL)
+                }
+
+                fn abs_min() -> u64 {
+                    #[allow(unused_comparisons)]
+                    if Self::MIN < 0 {
+                        1u64 << (Self::BITS - 1)
+                    } else {
+                        0
+                    }
+                }
+
+                fn complement(self) -> Self {
+                    (!self).wrapping_add((1 as $ty))
+                }
+            }
+
+            impl ParseInt for $ty {}
+        };
+    }
+
+    impl_parse_int!(i8);
+    impl_parse_int!(u8);
+    impl_parse_int!(i16);
+    impl_parse_int!(u16);
+    impl_parse_int!(i32);
+    impl_parse_int!(u32);
+    impl_parse_int!(i64);
+    impl_parse_int!(u64);
+    impl_parse_int!(isize);
+    impl_parse_int!(usize);
+}

-- 
2.47.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v8 6/7] rust: add parameter support to the `module!` macro
  2025-02-27 14:38 [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support Andreas Hindborg
                   ` (4 preceding siblings ...)
  2025-02-27 14:38 ` [PATCH v8 5/7] rust: str: add radix prefixed integer parsing functions Andreas Hindborg
@ 2025-02-27 14:38 ` Andreas Hindborg
  2025-02-27 14:38 ` [PATCH v8 7/7] modules: add rust modules files to MAINTAINERS Andreas Hindborg
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 19+ messages in thread
From: Andreas Hindborg @ 2025-02-27 14:38 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier, Luis Chamberlain
  Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
	linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
	linux-modules, Andreas Hindborg

Add support for module parameters to the `module!` macro. Implement read
only support for integer types without `sysfs` support.

Acked-by: Petr Pavlu <petr.pavlu@suse.com> # from modules perspective
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/lib.rs           |   1 +
 rust/kernel/module_param.rs  | 221 +++++++++++++++++++++++++++++++++++++++++++
 rust/macros/helpers.rs       |  25 +++++
 rust/macros/lib.rs           |  31 ++++++
 rust/macros/module.rs        | 191 +++++++++++++++++++++++++++++++++----
 samples/rust/rust_minimal.rs |  10 ++
 6 files changed, 461 insertions(+), 18 deletions(-)

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 496ed32b0911..aec04df2bac9 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 000000000000..0f153aa64dae
--- /dev/null
+++ b/rust/kernel/module_param.rs
@@ -0,0 +1,221 @@
+// 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;
+
+/// 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 it
+// from Rust module.
+unsafe impl Sync for RacyKernelParam {}
+
+/// Types that can be used for module parameters.
+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 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.
+///   The `arg` field of `param` must be an instance of `T`.
+/// - `param.arg` must be a pointer to valid `*mut T` as set up by the
+///   [`module!`] macro.
+///
+/// # 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 EINVAL.to_errno();
+    }
+
+    // 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) };
+
+    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 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.
+#[repr(transparent)]
+pub struct ModuleParamAccess<T> {
+    data: core::cell::UnsafeCell<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(value: T) -> Self {
+        Self {
+            data: core::cell::UnsafeCell::new(value),
+        }
+    }
+
+    /// 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 {
+        // SAFETY: As we only support read only parameters with no sysfs
+        // exposure, the kernel will not touch the parameter data after module
+        // initialization.
+        unsafe { &*self.data.get() }
+    }
+
+    /// Get a mutable pointer to the parameter value.
+    pub const fn as_mut_ptr(&self) -> *mut T {
+        self.data.get()
+    }
+}
+
+#[doc(hidden)]
+#[macro_export]
+/// 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) => {
+        ///
+        /// Static [`kernel_param_ops`](srctree/include/linux/moduleparam.h)
+        /// struct generated by `make_param_ops`
+        #[doc = concat!("for [`", stringify!($ty), "`].")]
+        pub static $ops: $crate::bindings::kernel_param_ops = $crate::bindings::kernel_param_ops {
+            flags: 0,
+            set: Some(set_param::<$ty>),
+            get: None,
+            free: Some(free::<$ty>),
+        };
+    };
+}
+
+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);
diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
index 563dcd2b7ace..ffc9f0cccddc 100644
--- a/rust/macros/helpers.rs
+++ b/rust/macros/helpers.rs
@@ -10,6 +10,17 @@ pub(crate) fn try_ident(it: &mut token_stream::IntoIter) -> Option<String> {
     }
 }
 
+pub(crate) fn try_sign(it: &mut token_stream::IntoIter) -> Option<char> {
+    let peek = it.clone().next();
+    match peek {
+        Some(TokenTree::Punct(punct)) if punct.as_char() == '-' => {
+            let _ = it.next();
+            Some(punct.as_char())
+        }
+        _ => None,
+    }
+}
+
 pub(crate) fn try_literal(it: &mut token_stream::IntoIter) -> Option<String> {
     if let Some(TokenTree::Literal(literal)) = it.next() {
         Some(literal.to_string())
@@ -107,6 +118,20 @@ pub(crate) struct Generics {
     pub(crate) ty_generics: Vec<TokenTree>,
 }
 
+/// Parse a token stream of the form `expected_name: "value",` and return the
+/// string in the position of "value".
+///
+/// # Panics
+///
+/// - On parse error.
+pub(crate) fn expect_string_field(it: &mut token_stream::IntoIter, expected_name: &str) -> String {
+    assert_eq!(expect_ident(it), expected_name);
+    assert_eq!(expect_punct(it), ':');
+    let string = expect_string(it);
+    assert_eq!(expect_punct(it), ',');
+    string
+}
+
 /// Parses the given `TokenStream` into `Generics` and the rest.
 ///
 /// The generics are not present in the rest, but a where clause might remain.
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index d61bc6a56425..2778292f8cee 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -24,6 +24,30 @@
 /// The `type` argument should be a type which implements the [`Module`]
 /// trait. Also accepts various forms of kernel metadata.
 ///
+/// The `params` field describe module parameters. Each entry has the form
+///
+/// ```ignore
+/// parameter_name: type {
+///     default: default_value,
+///     description: "Description",
+/// }
+/// ```
+///
+/// `type` may be one of
+///
+/// - [`i8`]
+/// - [`u8`]
+/// - [`i8`]
+/// - [`u8`]
+/// - [`i16`]
+/// - [`u16`]
+/// - [`i32`]
+/// - [`u32`]
+/// - [`i64`]
+/// - [`u64`]
+/// - [`isize`]
+/// - [`usize`]
+///
 /// C header: [`include/linux/moduleparam.h`](srctree/include/linux/moduleparam.h)
 ///
 /// [`Module`]: ../kernel/trait.Module.html
@@ -40,6 +64,12 @@
 ///     description: "My very own kernel module!",
 ///     license: "GPL",
 ///     alias: ["alternate_module_name"],
+///     params: {
+///         my_parameter: i64 {
+///             default: 1,
+///             description: "This parameter has a default of 1",
+///         },
+///     },
 /// }
 ///
 /// struct MyModule(i32);
@@ -48,6 +78,7 @@
 ///     fn init(_module: &'static ThisModule) -> Result<Self> {
 ///         let foo: i32 = 42;
 ///         pr_info!("I contain:  {}\n", foo);
+///         pr_info!("i32 param is:  {}\n", module_parameters::my_parameter.read());
 ///         Ok(Self(foo))
 ///     }
 /// }
diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index cdf94f4982df..e6af3ae5fe80 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -26,6 +26,7 @@ struct ModInfoBuilder<'a> {
     module: &'a str,
     counter: usize,
     buffer: String,
+    param_buffer: String,
 }
 
 impl<'a> ModInfoBuilder<'a> {
@@ -34,10 +35,11 @@ fn new(module: &'a str) -> Self {
             module,
             counter: 0,
             buffer: String::new(),
+            param_buffer: String::new(),
         }
     }
 
-    fn emit_base(&mut self, field: &str, content: &str, builtin: bool) {
+    fn emit_base(&mut self, field: &str, content: &str, builtin: bool, param: bool) {
         let string = if builtin {
             // Built-in modules prefix their modinfo strings by `module.`.
             format!(
@@ -51,8 +53,14 @@ fn emit_base(&mut self, field: &str, content: &str, builtin: bool) {
             format!("{field}={content}\0", field = field, content = content)
         };
 
+        let buffer = if param {
+            &mut self.param_buffer
+        } else {
+            &mut self.buffer
+        };
+
         write!(
-            &mut self.buffer,
+            buffer,
             "
                 {cfg}
                 #[doc(hidden)]
@@ -75,20 +83,116 @@ fn emit_base(&mut self, field: &str, content: &str, builtin: bool) {
         self.counter += 1;
     }
 
-    fn emit_only_builtin(&mut self, field: &str, content: &str) {
-        self.emit_base(field, content, true)
+    fn emit_only_builtin(&mut self, field: &str, content: &str, param: bool) {
+        self.emit_base(field, content, true, param)
     }
 
-    fn emit_only_loadable(&mut self, field: &str, content: &str) {
-        self.emit_base(field, content, false)
+    fn emit_only_loadable(&mut self, field: &str, content: &str, param: bool) {
+        self.emit_base(field, content, false, param)
     }
 
     fn emit(&mut self, field: &str, content: &str) {
-        self.emit_only_builtin(field, content);
-        self.emit_only_loadable(field, content);
+        self.emit_internal(field, content, false);
+    }
+
+    fn emit_internal(&mut self, field: &str, content: &str, param: bool) {
+        self.emit_only_builtin(field, content, param);
+        self.emit_only_loadable(field, content, param);
+    }
+
+    fn emit_param(&mut self, field: &str, param: &str, content: &str) {
+        let content = format!("{param}:{content}", param = param, content = content);
+        self.emit_internal(field, &content, true);
+    }
+
+    fn emit_params(&mut self, info: &ModuleInfo) {
+        let Some(params) = &info.params else {
+            return;
+        };
+
+        for param in params {
+            let ops = param_ops_path(&param.ptype);
+
+            // Note: The spelling of these fields is dictated by the user space
+            // tool `modinfo`.
+            self.emit_param("parmtype", &param.name, &param.ptype);
+            self.emit_param("parm", &param.name, &param.description);
+
+            write!(
+                self.param_buffer,
+                "
+                    pub(crate) static {param_name}:
+                        ::kernel::module_param::ModuleParamAccess<{param_type}> =
+                            ::kernel::module_param::ModuleParamAccess::new({param_default});
+
+                    #[link_section = \"__param\"]
+                    #[used]
+                    static __{module_name}_{param_name}_struct:
+                        ::kernel::module_param::RacyKernelParam =
+                        ::kernel::module_param::RacyKernelParam(::kernel::bindings::kernel_param {{
+                            name: if cfg!(MODULE) {{
+                                ::kernel::c_str!(\"{param_name}\").as_bytes_with_nul()
+                            }} else {{
+                                ::kernel::c_str!(\"{module_name}.{param_name}\").as_bytes_with_nul()
+                            }}.as_ptr(),
+                            // SAFETY: `__this_module` is constructed by the kernel at load time
+                            // and will not be freed until the module is unloaded.
+                            #[cfg(MODULE)]
+                            mod_: unsafe {{
+                                (&::kernel::bindings::__this_module
+                                    as *const ::kernel::bindings::module)
+                                    .cast_mut()
+                            }},
+                            #[cfg(not(MODULE))]
+                            mod_: ::core::ptr::null_mut(),
+                            ops: &{ops} as *const ::kernel::bindings::kernel_param_ops,
+                            perm: 0, // Will not appear in sysfs
+                            level: -1,
+                            flags: 0,
+                            __bindgen_anon_1:
+                                ::kernel::bindings::kernel_param__bindgen_ty_1 {{
+                                    arg: {param_name}.as_mut_ptr().cast()
+                                }},
+                        }});
+                ",
+                module_name = info.name,
+                param_type = param.ptype,
+                param_default = param.default,
+                param_name = param.name,
+                ops = ops,
+            )
+            .unwrap();
+        }
+    }
+}
+
+fn param_ops_path(param_type: &str) -> &'static str {
+    match param_type {
+        "i8" => "::kernel::module_param::PARAM_OPS_I8",
+        "u8" => "::kernel::module_param::PARAM_OPS_U8",
+        "i16" => "::kernel::module_param::PARAM_OPS_I16",
+        "u16" => "::kernel::module_param::PARAM_OPS_U16",
+        "i32" => "::kernel::module_param::PARAM_OPS_I32",
+        "u32" => "::kernel::module_param::PARAM_OPS_U32",
+        "i64" => "::kernel::module_param::PARAM_OPS_I64",
+        "u64" => "::kernel::module_param::PARAM_OPS_U64",
+        "isize" => "::kernel::module_param::PARAM_OPS_ISIZE",
+        "usize" => "::kernel::module_param::PARAM_OPS_USIZE",
+        t => panic!("Unsupported parameter type {}", t),
     }
 }
 
+fn expect_param_default(param_it: &mut token_stream::IntoIter) -> String {
+    assert_eq!(expect_ident(param_it), "default");
+    assert_eq!(expect_punct(param_it), ':');
+    let sign = try_sign(param_it);
+    let default = try_literal(param_it).expect("Expected default param value");
+    assert_eq!(expect_punct(param_it), ',');
+    let mut value = sign.map(String::from).unwrap_or_default();
+    value.push_str(&default);
+    value
+}
+
 #[derive(Debug, Default)]
 struct ModuleInfo {
     type_: String,
@@ -98,6 +202,50 @@ struct ModuleInfo {
     description: Option<String>,
     alias: Option<Vec<String>>,
     firmware: Option<Vec<String>>,
+    params: Option<Vec<Parameter>>,
+}
+
+#[derive(Debug)]
+struct Parameter {
+    name: String,
+    ptype: String,
+    default: String,
+    description: String,
+}
+
+fn expect_params(it: &mut token_stream::IntoIter) -> Vec<Parameter> {
+    let params = expect_group(it);
+    assert_eq!(params.delimiter(), Delimiter::Brace);
+    let mut it = params.stream().into_iter();
+    let mut parsed = Vec::new();
+
+    loop {
+        let param_name = match it.next() {
+            Some(TokenTree::Ident(ident)) => ident.to_string(),
+            Some(_) => panic!("Expected Ident or end"),
+            None => break,
+        };
+
+        assert_eq!(expect_punct(&mut it), ':');
+        let param_type = expect_ident(&mut it);
+        let group = expect_group(&mut it);
+        assert_eq!(group.delimiter(), Delimiter::Brace);
+        assert_eq!(expect_punct(&mut it), ',');
+
+        let mut param_it = group.stream().into_iter();
+        let param_default = expect_param_default(&mut param_it);
+        let param_description = expect_string_field(&mut param_it, "description");
+        expect_end(&mut param_it);
+
+        parsed.push(Parameter {
+            name: param_name,
+            ptype: param_type,
+            default: param_default,
+            description: param_description,
+        })
+    }
+
+    parsed
 }
 
 impl ModuleInfo {
@@ -112,6 +260,7 @@ fn parse(it: &mut token_stream::IntoIter) -> Self {
             "license",
             "alias",
             "firmware",
+            "params",
         ];
         const REQUIRED_KEYS: &[&str] = &["type", "name", "license"];
         let mut seen_keys = Vec::new();
@@ -140,6 +289,7 @@ fn parse(it: &mut token_stream::IntoIter) -> Self {
                 "license" => info.license = expect_string_ascii(it),
                 "alias" => info.alias = Some(expect_string_array(it)),
                 "firmware" => info.firmware = Some(expect_string_array(it)),
+                "params" => info.params = Some(expect_params(it)),
                 _ => panic!(
                     "Unknown key \"{}\". Valid keys are: {:?}.",
                     key, EXPECTED_KEYS
@@ -183,28 +333,30 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
     let info = ModuleInfo::parse(&mut it);
 
     let mut modinfo = ModInfoBuilder::new(info.name.as_ref());
-    if let Some(author) = info.author {
-        modinfo.emit("author", &author);
+    if let Some(author) = &info.author {
+        modinfo.emit("author", author);
     }
-    if let Some(description) = info.description {
-        modinfo.emit("description", &description);
+    if let Some(description) = &info.description {
+        modinfo.emit("description", description);
     }
     modinfo.emit("license", &info.license);
-    if let Some(aliases) = info.alias {
+    if let Some(aliases) = &info.alias {
         for alias in aliases {
-            modinfo.emit("alias", &alias);
+            modinfo.emit("alias", alias);
         }
     }
-    if let Some(firmware) = info.firmware {
+    if let Some(firmware) = &info.firmware {
         for fw in firmware {
-            modinfo.emit("firmware", &fw);
+            modinfo.emit("firmware", fw);
         }
     }
 
     // Built-in modules also export the `file` modinfo string.
     let file =
         std::env::var("RUST_MODFILE").expect("Unable to fetch RUST_MODFILE environmental variable");
-    modinfo.emit_only_builtin("file", &file);
+    modinfo.emit_only_builtin("file", &file, false);
+
+    modinfo.emit_params(&info);
 
     format!(
         "
@@ -362,14 +514,17 @@ unsafe fn __exit() {{
                             __MOD.assume_init_drop();
                         }}
                     }}
-
                     {modinfo}
                 }}
             }}
+            mod module_parameters {{
+                {params}
+            }}
         ",
         type_ = info.type_,
         name = info.name,
         modinfo = modinfo.buffer,
+        params = modinfo.param_buffer,
         initcall_section = ".initcall6.init"
     )
     .parse()
diff --git a/samples/rust/rust_minimal.rs b/samples/rust/rust_minimal.rs
index 4aaf117bf8e3..cbe8c6346648 100644
--- a/samples/rust/rust_minimal.rs
+++ b/samples/rust/rust_minimal.rs
@@ -10,6 +10,12 @@
     author: "Rust for Linux Contributors",
     description: "Rust minimal sample",
     license: "GPL",
+    params: {
+        test_parameter: i64 {
+            default: 1,
+            description: "This parameter has a default of 1",
+        },
+    },
 }
 
 struct RustMinimal {
@@ -20,6 +26,10 @@ impl kernel::Module for RustMinimal {
     fn init(_module: &'static ThisModule) -> Result<Self> {
         pr_info!("Rust minimal sample (init)\n");
         pr_info!("Am I built-in? {}\n", !cfg!(MODULE));
+        pr_info!(
+            "test_parameter: {}\n",
+            *module_parameters::test_parameter.get()
+        );
 
         let mut numbers = KVec::new();
         numbers.push(72, GFP_KERNEL)?;

-- 
2.47.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* [PATCH v8 7/7] modules: add rust modules files to MAINTAINERS
  2025-02-27 14:38 [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support Andreas Hindborg
                   ` (5 preceding siblings ...)
  2025-02-27 14:38 ` [PATCH v8 6/7] rust: add parameter support to the `module!` macro Andreas Hindborg
@ 2025-02-27 14:38 ` Andreas Hindborg
  2025-03-20  8:54   ` Daniel Gomez
  2025-03-20  8:41 ` [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support Daniel Gomez
  2025-03-20 23:23 ` Miguel Ojeda
  8 siblings, 1 reply; 19+ messages in thread
From: Andreas Hindborg @ 2025-02-27 14:38 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier, Luis Chamberlain
  Cc: Trevor Gross, Adam Bratschi-Kaye, rust-for-linux, linux-kernel,
	linux-kbuild, Petr Pavlu, Sami Tolvanen, Daniel Gomez,
	Simona Vetter, Greg KH, Fiona Behrens, Daniel Almeida,
	linux-modules, Andreas Hindborg

The module subsystem people agreed to maintain rust support for modules
[1]. Thus, add entries for relevant files to modules entry in MAINTAINERS.

Link: https://lore.kernel.org/all/0d9e596a-5316-4e00-862b-fd77552ae4b5@suse.com/ [1]
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 896a307fa065..ff65178f6e06 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -15991,6 +15991,8 @@ F:	include/linux/kmod.h
 F:	include/linux/module*.h
 F:	kernel/module/
 F:	lib/test_kmod.c
+F:	rust/kernel/module_param.rs
+F:	rust/macros/module.rs
 F:	scripts/module*
 F:	tools/testing/selftests/kmod/
 

-- 
2.47.0



^ permalink raw reply related	[flat|nested] 19+ messages in thread

* Re: [PATCH v8 2/7] rust: str: implement `Index` for `BStr`
  2025-02-27 14:38 ` [PATCH v8 2/7] rust: str: implement `Index` " Andreas Hindborg
@ 2025-02-28  9:10   ` Alice Ryhl
  0 siblings, 0 replies; 19+ messages in thread
From: Alice Ryhl @ 2025-02-28  9:10 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier, Luis Chamberlain, Trevor Gross,
	Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
	Petr Pavlu, Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
	Fiona Behrens, Daniel Almeida, linux-modules

On Thu, Feb 27, 2025 at 3:39 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> The `Index` implementation on `BStr` was lost when we switched `BStr` from
> a type alias of `[u8]` to a newtype. Add back `Index` by implementing
> `Index` for `BStr` when `Index` would be implemented for `[u8]`.
>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Reviewed-by: Fiona Behrens <me@kloenk.dev>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v8 1/7] rust: str: implement `PartialEq` for `BStr`
  2025-02-27 14:38 ` [PATCH v8 1/7] rust: str: implement `PartialEq` for `BStr` Andreas Hindborg
@ 2025-03-04 11:07   ` Fiona Behrens
  0 siblings, 0 replies; 19+ messages in thread
From: Fiona Behrens @ 2025-03-04 11:07 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier, Luis Chamberlain, Trevor Gross,
	Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
	Petr Pavlu, Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
	Daniel Almeida, linux-modules

Andreas Hindborg <a.hindborg@kernel.org> writes:

> Implement `PartialEq` for `BStr` by comparing underlying byte slices.
>
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

Reviewed-by: Fiona Behrens <me@kloenk.dev>

> ---
>  rust/kernel/str.rs | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index 28e2201604d6..002dcddf7c76 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -108,6 +108,12 @@ fn deref(&self) -> &Self::Target {
>      }
>  }
>  
> +impl PartialEq for BStr {
> +    fn eq(&self, other: &Self) -> bool {
> +        self.deref().eq(other.deref())
> +    }
> +}
> +
>  /// Creates a new [`BStr`] from a string literal.
>  ///
>  /// `b_str!` converts the supplied string literal to byte string, so non-ASCII

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v8 3/7] rust: str: implement `AsRef<BStr>` for `[u8]` and `BStr`
  2025-02-27 14:38 ` [PATCH v8 3/7] rust: str: implement `AsRef<BStr>` for `[u8]` and `BStr` Andreas Hindborg
@ 2025-03-04 11:09   ` Fiona Behrens
  0 siblings, 0 replies; 19+ messages in thread
From: Fiona Behrens @ 2025-03-04 11:09 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier, Luis Chamberlain, Trevor Gross,
	Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
	Petr Pavlu, Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
	Daniel Almeida, linux-modules

Andreas Hindborg <a.hindborg@kernel.org> writes:

> Implement `AsRef<BStr>` for `[u8]` and `BStr` so these can be used
> interchangeably for operations on `BStr`.
>
> Reviewed-by: Gary Guo <gary@garyguo.net>
> Tested-by: Daniel Almeida <daniel.almeida@collabora.com>
> Reviewed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

Reviewed-by: Fiona Behrens <me@kloenk.dev>

> ---
>  rust/kernel/str.rs | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
> index ba6b1a5c4f99..c6bd2c69543d 100644
> --- a/rust/kernel/str.rs
> +++ b/rust/kernel/str.rs
> @@ -125,6 +125,18 @@ fn index(&self, index: Idx) -> &Self::Output {
>      }
>  }
>  
> +impl AsRef<BStr> for [u8] {
> +    fn as_ref(&self) -> &BStr {
> +        BStr::from_bytes(self)
> +    }
> +}
> +
> +impl AsRef<BStr> for BStr {
> +    fn as_ref(&self) -> &BStr {
> +        self
> +    }
> +}
> +
>  /// Creates a new [`BStr`] from a string literal.
>  ///
>  /// `b_str!` converts the supplied string literal to byte string, so non-ASCII

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support
  2025-02-27 14:38 [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support Andreas Hindborg
                   ` (6 preceding siblings ...)
  2025-02-27 14:38 ` [PATCH v8 7/7] modules: add rust modules files to MAINTAINERS Andreas Hindborg
@ 2025-03-20  8:41 ` Daniel Gomez
  2025-03-20 10:26   ` Andreas Hindborg
  2025-03-20 23:23 ` Miguel Ojeda
  8 siblings, 1 reply; 19+ messages in thread
From: Daniel Gomez @ 2025-03-20  8:41 UTC (permalink / raw)
  To: Andreas Hindborg, Miguel Ojeda, Petr Pavlu
  Cc: Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Benno Lossin, Alice Ryhl, Masahiro Yamada, Nathan Chancellor,
	Nicolas Schier, Luis Chamberlain, Trevor Gross,
	Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
	Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
	Fiona Behrens, Daniel Almeida, linux-modules

Hi, 
On Thu, Feb 27, 2025 at 03:38:06PM +0100, Andreas Hindborg wrote:
> Extend the `module!` macro with support module parameters. Also add some string
> to integer parsing functions and updates `BStr` with a method to strip a string
> prefix.
> 
> Based on code by Adam Bratschi-Kaye lifted from the original `rust` branch [1].
> 
> Link: https://github.com/Rust-for-Linux/linux/tree/bc22545f38d74473cfef3e9fd65432733435b79f [1]
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

I've tested this series including the following patches from Andreas' tree [1]
as dependency for module testing parameters with the Rust Null Block driver:

[1] https://web.git.kernel.org/pub/scm/linux/kernel/git/a.hindborg/linux.git/log/?h=rnull-v6.14-rc1

51d304103b7f rust: refactor: rename `RawWriter` to `BufferWriter`
544811b24574 LIST: [PATCH v2 1/3] rust: sync: change `<Arc<T> as ForeignOwnable>::PointedTo` to `T`
3f097abd58de LIST: [PATCH v15 1/3] rust: types: add `ForeignOwnable::PointedTo`
0525eda0ff8d LIST: [PATCH v7 3/14] rust: sync: add `Arc::as_ptr`
ce7343b48e63 LIST: [PATCH v2 2/3] rust: configfs: introduce rust support for configfs
6efae1a9a226 rust: rnull: add module parameter support
b6545e0eaf94 rust: rnull: enable configuration via `configfs`
6a3bc0dc31d0 rust: rnull: move driver to separate directory

* modinfo
sudo modinfo rnull_mod
filename:       /lib/modules/6.14.0-rc6-00015-g51d304103b7f/kernel/drivers/block/rnull/rnull_mod.ko
author:         Andreas Hindborg
description:    Rust implementation of the C null block driver
license:        GPL v2
name:           rnull_mod
intree:         Y
depends:
vermagic:       6.14.0-rc6-00015-g51d304103b7f mod_unload modversions
parm:           nr_devices:Number of devices to register (u64)
parm:           bs:Block size (in bytes) (u32)
parm:           rotational:Set the rotational feature for the device (0 for false, 1 for true). Default: 0 (u8)
parm:           gb:Device capacity in MiB (u64)

* Testing nr_devices parameter:
sudo modprobe rnull_mod nr_devices=100

sudo ls /dev/rnullb* | wc -l
100

* Testing block size and capacity parameters:
sudo rmmod rnull_mod
sudo modprobe rnull_mod nr_devices=1 bs=512 gb=1024

sudo fdisk -l /dev/rnullb0
Disk /dev/rnullb0: 1 GiB, 1073741824 bytes, 2097152 sectors
Units: sectors of 1 * 512 = 512 bytes
Sector size (logical/physical): 512 bytes / 512 bytes
I/O size (minimum/optimal): 512 bytes / 512 bytes

* Testing block size with fio and blkalgn [1] (tool for validating driver block
size):

[1] blkalgn is an eBPF-based tool for tracing block operations that also reports
block granularity and alignment histograms:
https://github.com/dkruces/bcc/tree/blkalgn

Install:
https://github.com/dkruces/bcc/releases/latest/download/blkalgn-$(uname -m) \
--output /usr/sbin/blkalgn \
&& sudo chmod +x /usr/sbin/blkalgn

sudo modprobe rnull_mod nr_devices=1 bs=1024 gb=1024
sudo curl --location \

sudo blkalgn --disk=rnullb0 --ops=Write
sudo fio --name=test --direct=1 --rw=write --bs=1024 --size=512k \
--filename=/dev/rnullb0 --loop=1000

I/O Granularity Histogram for Device rnullb0 (lbads: 10 - 1024 bytes)
Total I/Os: 10748
     Bytes         : count    distribution
        1024       : 10748    |****************************************|

I/O Alignment Histogram for Device rnullb0
     Bytes               : count    distribution
         0 -> 1          : 0        |                                        |
         2 -> 3          : 0        |                                        |
         4 -> 7          : 0        |                                        |
         8 -> 15         : 0        |                                        |
        16 -> 31         : 0        |                                        |
        32 -> 63         : 0        |                                        |
        64 -> 127        : 0        |                                        |
       128 -> 255        : 0        |                                        |
       256 -> 511        : 0        |                                        |
       512 -> 1023       : 0        |                                        |
      1024 -> 2047       : 10748    |****************************************|

Tested-by: Daniel Gomez <da.gomez@samsung.com>


Andreas, Petr, Miguel,

Based on the discussion in v7, it seems that all these patches will go through
the Rust tree. Is that correct? What would be missing from the module's side?

I agree with Petr in that thread that if the changes are mostly limited to
rust-module files, they can go through the module's tree. However, that is not
the case yet.

Daniel

> ---
> Changes in v8:
> - Change print statement in sample to better communicate parameter name.
> - Use imperative mode in commit messages.
> - Remove prefix path from `EINVAL`.
> - Change `try_from_param_arg` to accept `&BStr` rather than `&[u8]`.
> - Parse integers without 128 bit integer types.
> - Seal trait `FromStrRadix`.
> - Strengthen safety requirement of `set_param`.
> - Remove comment about Display and `PAGE_SIZE`.
> - Add note describing why `ModuleParamAccess` is pub.
> - Typo and grammar fixes for documentation.
> - Update MAINTAINERS with rust module files.
> - Link to v7: https://lore.kernel.org/r/20250218-module-params-v3-v7-0-5e1afabcac1b@kernel.org
> 
> Changes in v7:
> - Remove dependency on `pr_warn_once` patches, replace with TODO.
> - Rework `ParseInt::from_str` to avoid allocating.
> - Add a comment explaining how we parse "0".
> - Change trait bound on `Index` impl for `BStr` to match std library approach.
> - Link to v6: https://lore.kernel.org/r/20250211-module-params-v3-v6-0-24b297ddc43d@kernel.org
> 
> Changes in v6:
> - Fix a bug that prevented parsing of negative default values for
>   parameters in the `module!` macro.
> - Fix a bug that prevented parsing zero in `strip_radix`. Also add a
>   test case for this.
> - Add `AsRef<BStr>` for `[u8]` and `BStr`.
> - Use `impl AsRef<BStr>` as type of prefix in `BStr::strip_prefix`.
> - Link to v5: https://lore.kernel.org/r/20250204-module-params-v3-v5-0-bf5ec2041625@kernel.org
> 
> Changes in v5:
> - Fix a typo in a safety comment in `set_param`.
> - Use a match statement in `parse_int::strip_radix`.
> - Add an implementation of `Index` for `BStr`.
> - Fix a logic inversion bug where parameters would not be parsed.
> - Use `kernel::ffi::c_char` in `set_param` rather than the one in `core`.
> - Use `kernel::c_str!` rather than `c"..."` literal in module macro.
> - Rebase on v6.14-rc1.
> - Link to v4: https://lore.kernel.org/r/20250109-module-params-v3-v4-0-c208bcfbe11f@kernel.org
> 
> Changes in v4:
> - Add module maintainers to Cc list (sorry)
> - Add a few missing [`doc_links`]
> - Add panic section to `expect_string_field`
> - Fix a typo in safety requirement of `module_params::free`
> - Change `assert!` to `pr_warn_once!` in `module_params::set_param`
> - Remove `module_params::get_param` and install null pointer instead
> - Remove use of the unstable feature `sync_unsafe_cell`
> - Link to v3: https://lore.kernel.org/r/20241213-module-params-v3-v3-0-485a015ac2cf@kernel.org
> 
> Changes in v3:
> - use `SyncUnsafeCell` rather than `static mut` and simplify parameter access
> - remove `Display` bound from `ModuleParam`
> - automatically generate documentation for `PARAM_OPS_.*`
> - remove `as *const _ as *mut_` phrasing
> - inline parameter name in struct instantiation in  `emit_params`
> - move `RacyKernelParam` out of macro template
> - use C string literals rather than byte string literals with explicit null
> - template out `__{name}_{param_name}` in `emit_param`
> - indent template in `emit_params`
> - use let-else expression in `emit_params` to get rid of an indentation level
> - document `expect_string_field`
> - move invication of `impl_int_module_param` to be closer to macro def
> - move attributes after docs in `make_param_ops`
> - rename `impl_module_param` to impl_int_module_param`
> - use `ty` instead of `ident` in `impl_parse_int`
> - use `BStr` instead of `&str` for string manipulation
> - move string parsing functions to seperate patch and add examples, fix bugs
> - degrade comment about future support from doc comment to regular comment
> - remove std lib path from `Sized` marker
> - update documentation for `trait ModuleParam`
> - Link to v2: https://lore.kernel.org/all/20240819133345.3438739-1-nmi@metaspace.dk/
> 
> Changes in v2:
> - Remove support for params without values (`NOARG_ALLOWED`).
> - Improve documentation for `try_from_param_arg`.
> - Use prelude import.
> - Refactor `try_from_param_arg` to return `Result`.
> - Refactor `ParseInt::from_str` to return `Result`.
> - Move C callable functions out of `ModuleParam` trait.
> - Rename literal string field parser to `expect_string_field`.
> - Move parameter parsing from generation to parsing stage.
> - Use absolute type paths in macro code.
> - Inline `kparam`and `read_func` values.
> - Resolve TODO regarding alignment attributes.
> - Remove unnecessary unsafe blocks in macro code.
> - Improve error message for unrecognized parameter types.
> - Do not use `self` receiver when reading parameter value.
> - Add parameter documentation to `module!` macro.
> - Use empty `enum` for parameter type.
> - Use `addr_of_mut` to get address of parameter value variable.
> - Enabled building of docs for for `module_param` module.
> - Link to v1: https://lore.kernel.org/rust-for-linux/20240705111455.142790-1-nmi@metaspace.dk/
> 
> ---
> Andreas Hindborg (7):
>       rust: str: implement `PartialEq` for `BStr`
>       rust: str: implement `Index` for `BStr`
>       rust: str: implement `AsRef<BStr>` for `[u8]` and `BStr`
>       rust: str: implement `strip_prefix` for `BStr`
>       rust: str: add radix prefixed integer parsing functions
>       rust: add parameter support to the `module!` macro
>       modules: add rust modules files to MAINTAINERS
> 
>  MAINTAINERS                  |   2 +
>  rust/kernel/lib.rs           |   1 +
>  rust/kernel/module_param.rs  | 221 +++++++++++++++++++++++++++++++++++++++++++
>  rust/kernel/str.rs           | 200 +++++++++++++++++++++++++++++++++++++++
>  rust/macros/helpers.rs       |  25 +++++
>  rust/macros/lib.rs           |  31 ++++++
>  rust/macros/module.rs        | 191 +++++++++++++++++++++++++++++++++----
>  samples/rust/rust_minimal.rs |  10 ++
>  8 files changed, 663 insertions(+), 18 deletions(-)
> ---
> base-commit: 379487e17ca406b47392e7ab6cf35d1c3bacb371
> change-id: 20241211-module-params-v3-ae7e5c8d8b5a
> 
> Best regards,
> -- 
> Andreas Hindborg <a.hindborg@kernel.org>
> 
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v8 7/7] modules: add rust modules files to MAINTAINERS
  2025-02-27 14:38 ` [PATCH v8 7/7] modules: add rust modules files to MAINTAINERS Andreas Hindborg
@ 2025-03-20  8:54   ` Daniel Gomez
  0 siblings, 0 replies; 19+ messages in thread
From: Daniel Gomez @ 2025-03-20  8:54 UTC (permalink / raw)
  To: Andreas Hindborg, R
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier, Luis Chamberlain, Trevor Gross,
	Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
	Petr Pavlu, Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
	Fiona Behrens, Daniel Almeida, linux-modules

On Thu, Feb 27, 2025 at 03:38:13PM +0100, Andreas Hindborg wrote:
> The module subsystem people agreed to maintain rust support for modules
> [1]. Thus, add entries for relevant files to modules entry in MAINTAINERS.
> 
> Link: https://lore.kernel.org/all/0d9e596a-5316-4e00-862b-fd77552ae4b5@suse.com/ [1]
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

And this is what we signed up for here:

https://lore.kernel.org/all/ZsPANzx4-5DrOl5m@bombadil.infradead.org/

Acked-by: Daniel Gomez <da.gomez@samsung.com>

> ---
>  MAINTAINERS | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 896a307fa065..ff65178f6e06 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -15991,6 +15991,8 @@ F:	include/linux/kmod.h
>  F:	include/linux/module*.h
>  F:	kernel/module/
>  F:	lib/test_kmod.c
> +F:	rust/kernel/module_param.rs
> +F:	rust/macros/module.rs
>  F:	scripts/module*
>  F:	tools/testing/selftests/kmod/
>  
> 
> -- 
> 2.47.0
> 
> 

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support
  2025-03-20  8:41 ` [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support Daniel Gomez
@ 2025-03-20 10:26   ` Andreas Hindborg
  2025-03-20 12:00     ` Miguel Ojeda
  0 siblings, 1 reply; 19+ messages in thread
From: Andreas Hindborg @ 2025-03-20 10:26 UTC (permalink / raw)
  To: Daniel Gomez
  Cc: Miguel Ojeda, Petr Pavlu, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier, Luis Chamberlain, Trevor Gross,
	Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
	Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
	Fiona Behrens, Daniel Almeida, linux-modules

"Daniel Gomez" <da.gomez@kernel.org> writes:

> Hi,
> On Thu, Feb 27, 2025 at 03:38:06PM +0100, Andreas Hindborg wrote:
>> Extend the `module!` macro with support module parameters. Also add some string
>> to integer parsing functions and updates `BStr` with a method to strip a string
>> prefix.
>>
>> Based on code by Adam Bratschi-Kaye lifted from the original `rust` branch [1].
>>
>> Link: https://github.com/Rust-for-Linux/linux/tree/bc22545f38d74473cfef3e9fd65432733435b79f [1]
>> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
>
> I've tested this series including the following patches from Andreas' tree [1]
> as dependency for module testing parameters with the Rust Null Block driver:
>
> [1] https://web.git.kernel.org/pub/scm/linux/kernel/git/a.hindborg/linux.git/log/?h=rnull-v6.14-rc1
>
> 51d304103b7f rust: refactor: rename `RawWriter` to `BufferWriter`
> 544811b24574 LIST: [PATCH v2 1/3] rust: sync: change `<Arc<T> as ForeignOwnable>::PointedTo` to `T`
> 3f097abd58de LIST: [PATCH v15 1/3] rust: types: add `ForeignOwnable::PointedTo`
> 0525eda0ff8d LIST: [PATCH v7 3/14] rust: sync: add `Arc::as_ptr`
> ce7343b48e63 LIST: [PATCH v2 2/3] rust: configfs: introduce rust support for configfs
> 6efae1a9a226 rust: rnull: add module parameter support
> b6545e0eaf94 rust: rnull: enable configuration via `configfs`
> 6a3bc0dc31d0 rust: rnull: move driver to separate directory
>
> * modinfo
> sudo modinfo rnull_mod
> filename:       /lib/modules/6.14.0-rc6-00015-g51d304103b7f/kernel/drivers/block/rnull/rnull_mod.ko
> author:         Andreas Hindborg
> description:    Rust implementation of the C null block driver
> license:        GPL v2
> name:           rnull_mod
> intree:         Y
> depends:
> vermagic:       6.14.0-rc6-00015-g51d304103b7f mod_unload modversions
> parm:           nr_devices:Number of devices to register (u64)
> parm:           bs:Block size (in bytes) (u32)
> parm:           rotational:Set the rotational feature for the device (0 for false, 1 for true). Default: 0 (u8)
> parm:           gb:Device capacity in MiB (u64)
>
> * Testing nr_devices parameter:
> sudo modprobe rnull_mod nr_devices=100
>
> sudo ls /dev/rnullb* | wc -l
> 100
>
> * Testing block size and capacity parameters:
> sudo rmmod rnull_mod
> sudo modprobe rnull_mod nr_devices=1 bs=512 gb=1024
>
> sudo fdisk -l /dev/rnullb0
> Disk /dev/rnullb0: 1 GiB, 1073741824 bytes, 2097152 sectors
> Units: sectors of 1 * 512 = 512 bytes
> Sector size (logical/physical): 512 bytes / 512 bytes
> I/O size (minimum/optimal): 512 bytes / 512 bytes
>
> * Testing block size with fio and blkalgn [1] (tool for validating driver block
> size):
>
> [1] blkalgn is an eBPF-based tool for tracing block operations that also reports
> block granularity and alignment histograms:
> https://github.com/dkruces/bcc/tree/blkalgn
>
> Install:
> https://github.com/dkruces/bcc/releases/latest/download/blkalgn-$(uname -m) \
> --output /usr/sbin/blkalgn \
> && sudo chmod +x /usr/sbin/blkalgn
>
> sudo modprobe rnull_mod nr_devices=1 bs=1024 gb=1024
> sudo curl --location \
>
> sudo blkalgn --disk=rnullb0 --ops=Write
> sudo fio --name=test --direct=1 --rw=write --bs=1024 --size=512k \
> --filename=/dev/rnullb0 --loop=1000
>
> I/O Granularity Histogram for Device rnullb0 (lbads: 10 - 1024 bytes)
> Total I/Os: 10748
>      Bytes         : count    distribution
>         1024       : 10748    |****************************************|
>
> I/O Alignment Histogram for Device rnullb0
>      Bytes               : count    distribution
>          0 -> 1          : 0        |                                        |
>          2 -> 3          : 0        |                                        |
>          4 -> 7          : 0        |                                        |
>          8 -> 15         : 0        |                                        |
>         16 -> 31         : 0        |                                        |
>         32 -> 63         : 0        |                                        |
>         64 -> 127        : 0        |                                        |
>        128 -> 255        : 0        |                                        |
>        256 -> 511        : 0        |                                        |
>        512 -> 1023       : 0        |                                        |
>       1024 -> 2047       : 10748    |****************************************|
>
> Tested-by: Daniel Gomez <da.gomez@samsung.com>
>
>
> Andreas, Petr, Miguel,
>
> Based on the discussion in v7, it seems that all these patches will go through
> the Rust tree. Is that correct? What would be missing from the module's side?
>
> I agree with Petr in that thread that if the changes are mostly limited to
> rust-module files, they can go through the module's tree. However, that is not
> the case yet.

As far as I understand, Miguel would take patch 1-5 for v6.15 and
modules would take patch 6-7 for v6.16. At least that is my
understanding from [1], @Petr and @Miguel please correct me if I am
wrong.


Best regards,
Andreas Hindborg

[1] https://lore.kernel.org/all/CANiq72mW94Y-bsJFMHqF8fbXhvAizEn7-NnxawTW+5brbxJHBg@mail.gmail.com



^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support
  2025-03-20 10:26   ` Andreas Hindborg
@ 2025-03-20 12:00     ` Miguel Ojeda
  2025-03-20 12:47       ` Petr Pavlu
  0 siblings, 1 reply; 19+ messages in thread
From: Miguel Ojeda @ 2025-03-20 12:00 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Daniel Gomez, Miguel Ojeda, Petr Pavlu, Alex Gaynor, Boqun Feng,
	Gary Guo, Björn Roy Baron, Benno Lossin, Alice Ryhl,
	Masahiro Yamada, Nathan Chancellor, Nicolas Schier,
	Luis Chamberlain, Trevor Gross, Adam Bratschi-Kaye,
	rust-for-linux, linux-kernel, linux-kbuild, Sami Tolvanen,
	Daniel Gomez, Simona Vetter, Greg KH, Fiona Behrens,
	Daniel Almeida, linux-modules

On Thu, Mar 20, 2025 at 11:26 AM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> As far as I understand, Miguel would take patch 1-5 for v6.15 and
> modules would take patch 6-7 for v6.16. At least that is my
> understanding from [1], @Petr and @Miguel please correct me if I am
> wrong.

So I offered that as an option -- I assume it is OK since nobody said
anything (please correct me if I am wrong), and it would help get
things moving.

So I will take 1-5 later today or tomorrow unless someone shouts.

Thanks!

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support
  2025-03-20 12:00     ` Miguel Ojeda
@ 2025-03-20 12:47       ` Petr Pavlu
  0 siblings, 0 replies; 19+ messages in thread
From: Petr Pavlu @ 2025-03-20 12:47 UTC (permalink / raw)
  To: Miguel Ojeda, Andreas Hindborg
  Cc: Daniel Gomez, Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier, Luis Chamberlain, Trevor Gross,
	Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
	Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
	Fiona Behrens, Daniel Almeida, linux-modules

On 3/20/25 13:00, Miguel Ojeda wrote:
> On Thu, Mar 20, 2025 at 11:26 AM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> As far as I understand, Miguel would take patch 1-5 for v6.15 and
>> modules would take patch 6-7 for v6.16. At least that is my
>> understanding from [1], @Petr and @Miguel please correct me if I am
>> wrong.
> 
> So I offered that as an option -- I assume it is OK since nobody said
> anything (please correct me if I am wrong), and it would help get
> things moving.
> 
> So I will take 1-5 later today or tomorrow unless someone shouts.

Yep, looks good to me.

-- 
Thanks,
Petr

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v8 5/7] rust: str: add radix prefixed integer parsing functions
  2025-02-27 14:38 ` [PATCH v8 5/7] rust: str: add radix prefixed integer parsing functions Andreas Hindborg
@ 2025-03-20 20:21   ` Miguel Ojeda
  2025-03-21  7:53     ` Andreas Hindborg
  0 siblings, 1 reply; 19+ messages in thread
From: Miguel Ojeda @ 2025-03-20 20:21 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier, Luis Chamberlain, Trevor Gross,
	Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
	Petr Pavlu, Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
	Fiona Behrens, Daniel Almeida, linux-modules

On Thu, Feb 27, 2025 at 3:39 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> +                    // SAFETY: We checked that `val` will fit in `Self` above.
> +                    let val: Self = unsafe { val.try_into().unwrap_unchecked() };

This is wrong -- `val` can be the maximum, and thus it does not fit
since it is 2's complement, even if later the complement would.

In fact, it is caught by the doctest when run with debug assertions enabled:

   /// assert_eq!(Ok(-128), i8::from_str(b_str!("-128")));

We try to put 128 into `i8`, which of course does not work...

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support
  2025-02-27 14:38 [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support Andreas Hindborg
                   ` (7 preceding siblings ...)
  2025-03-20  8:41 ` [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support Daniel Gomez
@ 2025-03-20 23:23 ` Miguel Ojeda
  8 siblings, 0 replies; 19+ messages in thread
From: Miguel Ojeda @ 2025-03-20 23:23 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier, Luis Chamberlain, Trevor Gross,
	Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
	Petr Pavlu, Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
	Fiona Behrens, Daniel Almeida, linux-modules

On Thu, Feb 27, 2025 at 3:39 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Extend the `module!` macro with support module parameters. Also add some string
> to integer parsing functions and updates `BStr` with a method to strip a string
> prefix.
>
> Based on code by Adam Bratschi-Kaye lifted from the original `rust` branch [1].
>
> Link: https://github.com/Rust-for-Linux/linux/tree/bc22545f38d74473cfef3e9fd65432733435b79f [1]
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

Applied (1-4) to `rust-next` -- thanks everyone!

    [ Pluralized section name. Hid `use`. - Miguel ]

For the moment, I skipped 5 due to the UB I found.

Cheers,
Miguel

^ permalink raw reply	[flat|nested] 19+ messages in thread

* Re: [PATCH v8 5/7] rust: str: add radix prefixed integer parsing functions
  2025-03-20 20:21   ` Miguel Ojeda
@ 2025-03-21  7:53     ` Andreas Hindborg
  0 siblings, 0 replies; 19+ messages in thread
From: Andreas Hindborg @ 2025-03-21  7:53 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Benno Lossin, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Nicolas Schier, Luis Chamberlain, Trevor Gross,
	Adam Bratschi-Kaye, rust-for-linux, linux-kernel, linux-kbuild,
	Petr Pavlu, Sami Tolvanen, Daniel Gomez, Simona Vetter, Greg KH,
	Fiona Behrens, Daniel Almeida, linux-modules

"Miguel Ojeda" <miguel.ojeda.sandonis@gmail.com> writes:

> On Thu, Feb 27, 2025 at 3:39 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>>
>> +                    // SAFETY: We checked that `val` will fit in `Self` above.
>> +                    let val: Self = unsafe { val.try_into().unwrap_unchecked() };
>
> This is wrong -- `val` can be the maximum, and thus it does not fit
> since it is 2's complement, even if later the complement would.
>
> In fact, it is caught by the doctest when run with debug assertions enabled:
>
>    /// assert_eq!(Ok(-128), i8::from_str(b_str!("-128")));
>
> We try to put 128 into `i8`, which of course does not work...

Thanks for catching this! I have to start running tests with debug
assertions, it makes no sense not to do so.

I'll send a new version now, hope it will make the cut.


Best regards,
Andreas Hindborg




^ permalink raw reply	[flat|nested] 19+ messages in thread

end of thread, other threads:[~2025-03-21  7:54 UTC | newest]

Thread overview: 19+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-02-27 14:38 [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support Andreas Hindborg
2025-02-27 14:38 ` [PATCH v8 1/7] rust: str: implement `PartialEq` for `BStr` Andreas Hindborg
2025-03-04 11:07   ` Fiona Behrens
2025-02-27 14:38 ` [PATCH v8 2/7] rust: str: implement `Index` " Andreas Hindborg
2025-02-28  9:10   ` Alice Ryhl
2025-02-27 14:38 ` [PATCH v8 3/7] rust: str: implement `AsRef<BStr>` for `[u8]` and `BStr` Andreas Hindborg
2025-03-04 11:09   ` Fiona Behrens
2025-02-27 14:38 ` [PATCH v8 4/7] rust: str: implement `strip_prefix` for `BStr` Andreas Hindborg
2025-02-27 14:38 ` [PATCH v8 5/7] rust: str: add radix prefixed integer parsing functions Andreas Hindborg
2025-03-20 20:21   ` Miguel Ojeda
2025-03-21  7:53     ` Andreas Hindborg
2025-02-27 14:38 ` [PATCH v8 6/7] rust: add parameter support to the `module!` macro Andreas Hindborg
2025-02-27 14:38 ` [PATCH v8 7/7] modules: add rust modules files to MAINTAINERS Andreas Hindborg
2025-03-20  8:54   ` Daniel Gomez
2025-03-20  8:41 ` [PATCH v8 0/7] rust: extend `module!` macro with integer parameter support Daniel Gomez
2025-03-20 10:26   ` Andreas Hindborg
2025-03-20 12:00     ` Miguel Ojeda
2025-03-20 12:47       ` Petr Pavlu
2025-03-20 23:23 ` Miguel Ojeda

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).