Linux Modules
 help / color / mirror / Atom feed
* Re: [PATCH 0/2] Restrict module namespace to in-tree modules and rename macro
From: Christian Brauner @ 2025-07-08  7:40 UTC (permalink / raw)
  To: Vlastimil Babka
  Cc: Matthias Maennich, Jonathan Corbet, Luis Chamberlain, Petr Pavlu,
	Sami Tolvanen, Daniel Gomez, Masahiro Yamada, Nathan Chancellor,
	Nicolas Schier, Alexander Viro, Jan Kara, Christoph Hellwig,
	Peter Zijlstra, David Hildenbrand, Shivank Garg,
	Jiri Slaby (SUSE), Stephen Rothwell, linux-doc, linux-kernel,
	linux-modules, linux-kbuild, linux-fsdevel
In-Reply-To: <20250708-export_modules-v1-0-fbf7a282d23f@suse.cz>

On Tue, Jul 08, 2025 at 09:28:56AM +0200, Vlastimil Babka wrote:
> Christian asked [1] for EXPORT_SYMBOL_FOR_MODULES() without the _GPL_
> part to avoid controversy converting selected existing EXPORT_SYMBOL().
> Christoph argued [2] that the _FOR_MODULES() export is intended for
> in-tree modules and thus GPL is implied anyway and can be simply dropped
> from the export macro name. Peter agreed [3] about the intention for
> in-tree modules only, although nothing currently enforces it.
> 
> It seems straightforward to add this enforcement, so patch 1 does that.
> Patch 2 then drops the _GPL_ from the name and so we're left with
> EXPORT_SYMBOL_FOR_MODULES() restricted to in-tree modules only.
> 
> Current -next has some new instances of EXPORT_SYMBOL_GPL_FOR_MODULES()
> in drivers/tty/serial/8250/8250_rsa.c by commit b20d6576cdb3 ("serial:
> 8250: export RSA functions"). Hopefully it's resolvable by a merge
> commit fixup and we don't need to provide a temporary alias.
> 
> [1] https://lore.kernel.org/all/20250623-warmwasser-giftig-ff656fce89ad@brauner/
> [2] https://lore.kernel.org/all/aFleJN_fE-RbSoFD@infradead.org/
> [3] https://lore.kernel.org/all/20250623142836.GT1613200@noisy.programming.kicks-ass.net/
> 
> Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
> ---

Love this. It'd be great to get this in as a bugfix,
Acked-by: Christian Brauner <brauner@kernel.org>

^ permalink raw reply

* [PATCH 1/2] module: Restrict module namespace access to in-tree modules
From: Vlastimil Babka @ 2025-07-08  7:28 UTC (permalink / raw)
  To: Matthias Maennich, Jonathan Corbet, Luis Chamberlain, Petr Pavlu,
	Sami Tolvanen, Daniel Gomez, Masahiro Yamada, Nathan Chancellor,
	Nicolas Schier, Alexander Viro, Christian Brauner, Jan Kara
  Cc: Christoph Hellwig, Peter Zijlstra, David Hildenbrand,
	Shivank Garg, Jiri Slaby (SUSE), Stephen Rothwell, linux-doc,
	linux-kernel, linux-modules, linux-kbuild, linux-fsdevel,
	Vlastimil Babka
In-Reply-To: <20250708-export_modules-v1-0-fbf7a282d23f@suse.cz>

The module namespace support has been introduced to allow restricting
exports to specific modules only, and intended for in-tree modules such
as kvm. Make this intention explicit by disallowing out of tree modules
both for the module loader and modpost.

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 Documentation/core-api/symbol-namespaces.rst | 5 +++--
 kernel/module/main.c                         | 3 ++-
 scripts/mod/modpost.c                        | 6 +++++-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/Documentation/core-api/symbol-namespaces.rst b/Documentation/core-api/symbol-namespaces.rst
index 32fc73dc5529e8844c2ce2580987155bcd13cd09..dc228ac738a5cdc49cc736c29170ca96df6a28dc 100644
--- a/Documentation/core-api/symbol-namespaces.rst
+++ b/Documentation/core-api/symbol-namespaces.rst
@@ -83,13 +83,14 @@ Symbols exported using this macro are put into a module namespace. This
 namespace cannot be imported.
 
 The macro takes a comma separated list of module names, allowing only those
-modules to access this symbol. Simple tail-globs are supported.
+modules to access this symbol. The access is restricted to in-tree modules.
+Simple tail-globs are supported.
 
 For example::
 
   EXPORT_SYMBOL_GPL_FOR_MODULES(preempt_notifier_inc, "kvm,kvm-*")
 
-will limit usage of this symbol to modules whoes name matches the given
+will limit usage of this symbol to in-tree modules whoes name matches the given
 patterns.
 
 How to use Symbols exported in Namespaces
diff --git a/kernel/module/main.c b/kernel/module/main.c
index 413ac6ea37021bc8ae260f624ca2745ed85333fc..ec7d8daa0347e3b65713396d6b6d14c2cb0270d3 100644
--- a/kernel/module/main.c
+++ b/kernel/module/main.c
@@ -1157,7 +1157,8 @@ static int verify_namespace_is_imported(const struct load_info *info,
 	namespace = kernel_symbol_namespace(sym);
 	if (namespace && namespace[0]) {
 
-		if (verify_module_namespace(namespace, mod->name))
+		if (get_modinfo(info, "intree") &&
+		    verify_module_namespace(namespace, mod->name))
 			return 0;
 
 		for_each_modinfo_entry(imported_namespace, info, "import_ns") {
diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c
index 5ca7c268294ebb65acb0ba52a671eddca9279c61..d78be9834ed75f4b6ddb9af02a300a9bcc9234cc 100644
--- a/scripts/mod/modpost.c
+++ b/scripts/mod/modpost.c
@@ -1695,7 +1695,8 @@ void buf_write(struct buffer *buf, const char *s, int len)
  * @modname: module name
  *
  * If @namespace is prefixed with "module:" to indicate it is a module namespace
- * then test if @modname matches any of the comma separated patterns.
+ * then test if @modname matches any of the comma separated patterns. Access to
+ * module namespaces is restricted to in-tree modules only.
  *
  * The patterns only support tail-glob.
  */
@@ -1706,6 +1707,9 @@ static bool verify_module_namespace(const char *namespace, const char *modname)
 	const char *sep;
 	bool glob;
 
+	if (external_module)
+		return false;
+
 	if (!strstarts(namespace, prefix))
 		return false;
 

-- 
2.50.0


^ permalink raw reply related

* [PATCH 2/2] module: Rename EXPORT_SYMBOL_GPL_FOR_MODULES to EXPORT_SYMBOL_FOR_MODULES
From: Vlastimil Babka @ 2025-07-08  7:28 UTC (permalink / raw)
  To: Matthias Maennich, Jonathan Corbet, Luis Chamberlain, Petr Pavlu,
	Sami Tolvanen, Daniel Gomez, Masahiro Yamada, Nathan Chancellor,
	Nicolas Schier, Alexander Viro, Christian Brauner, Jan Kara
  Cc: Christoph Hellwig, Peter Zijlstra, David Hildenbrand,
	Shivank Garg, Jiri Slaby (SUSE), Stephen Rothwell, linux-doc,
	linux-kernel, linux-modules, linux-kbuild, linux-fsdevel,
	Vlastimil Babka
In-Reply-To: <20250708-export_modules-v1-0-fbf7a282d23f@suse.cz>

With module namespace access restricted to in-tree modules, the GPL
requirement becomes implied. Drop it from the name of the export helper.

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
 Documentation/core-api/symbol-namespaces.rst | 6 +++---
 fs/anon_inodes.c                             | 2 +-
 include/linux/export.h                       | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/core-api/symbol-namespaces.rst b/Documentation/core-api/symbol-namespaces.rst
index dc228ac738a5cdc49cc736c29170ca96df6a28dc..aafbc0469cd6a4b76225e0e96a86025de512008e 100644
--- a/Documentation/core-api/symbol-namespaces.rst
+++ b/Documentation/core-api/symbol-namespaces.rst
@@ -76,8 +76,8 @@ A second option to define the default namespace is directly in the compilation
 within the corresponding compilation unit before the #include for
 <linux/export.h>. Typically it's placed before the first #include statement.
 
-Using the EXPORT_SYMBOL_GPL_FOR_MODULES() macro
------------------------------------------------
+Using the EXPORT_SYMBOL_FOR_MODULES() macro
+-------------------------------------------
 
 Symbols exported using this macro are put into a module namespace. This
 namespace cannot be imported.
@@ -88,7 +88,7 @@ Simple tail-globs are supported.
 
 For example::
 
-  EXPORT_SYMBOL_GPL_FOR_MODULES(preempt_notifier_inc, "kvm,kvm-*")
+  EXPORT_SYMBOL_FOR_MODULES(preempt_notifier_inc, "kvm,kvm-*")
 
 will limit usage of this symbol to in-tree modules whoes name matches the given
 patterns.
diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 1d847a939f29a41356af3f12e5f61372ec2fb550..180a458fc4f74249d674ec3c6e01277df1d9e743 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -129,7 +129,7 @@ struct inode *anon_inode_make_secure_inode(struct super_block *sb, const char *n
 	}
 	return inode;
 }
-EXPORT_SYMBOL_GPL_FOR_MODULES(anon_inode_make_secure_inode, "kvm");
+EXPORT_SYMBOL_FOR_MODULES(anon_inode_make_secure_inode, "kvm");
 
 static struct file *__anon_inode_getfile(const char *name,
 					 const struct file_operations *fops,
diff --git a/include/linux/export.h b/include/linux/export.h
index f35d03b4113b19798036d2993d67eb932ad8ce6f..a686fd0ba406509da5f397e3a415d05c5a051c0d 100644
--- a/include/linux/export.h
+++ b/include/linux/export.h
@@ -91,6 +91,6 @@
 #define EXPORT_SYMBOL_NS(sym, ns)	__EXPORT_SYMBOL(sym, "", ns)
 #define EXPORT_SYMBOL_NS_GPL(sym, ns)	__EXPORT_SYMBOL(sym, "GPL", ns)
 
-#define EXPORT_SYMBOL_GPL_FOR_MODULES(sym, mods) __EXPORT_SYMBOL(sym, "GPL", "module:" mods)
+#define EXPORT_SYMBOL_FOR_MODULES(sym, mods) __EXPORT_SYMBOL(sym, "GPL", "module:" mods)
 
 #endif /* _LINUX_EXPORT_H */

-- 
2.50.0


^ permalink raw reply related

* [PATCH 0/2] Restrict module namespace to in-tree modules and rename macro
From: Vlastimil Babka @ 2025-07-08  7:28 UTC (permalink / raw)
  To: Matthias Maennich, Jonathan Corbet, Luis Chamberlain, Petr Pavlu,
	Sami Tolvanen, Daniel Gomez, Masahiro Yamada, Nathan Chancellor,
	Nicolas Schier, Alexander Viro, Christian Brauner, Jan Kara
  Cc: Christoph Hellwig, Peter Zijlstra, David Hildenbrand,
	Shivank Garg, Jiri Slaby (SUSE), Stephen Rothwell, linux-doc,
	linux-kernel, linux-modules, linux-kbuild, linux-fsdevel,
	Vlastimil Babka

Christian asked [1] for EXPORT_SYMBOL_FOR_MODULES() without the _GPL_
part to avoid controversy converting selected existing EXPORT_SYMBOL().
Christoph argued [2] that the _FOR_MODULES() export is intended for
in-tree modules and thus GPL is implied anyway and can be simply dropped
from the export macro name. Peter agreed [3] about the intention for
in-tree modules only, although nothing currently enforces it.

It seems straightforward to add this enforcement, so patch 1 does that.
Patch 2 then drops the _GPL_ from the name and so we're left with
EXPORT_SYMBOL_FOR_MODULES() restricted to in-tree modules only.

Current -next has some new instances of EXPORT_SYMBOL_GPL_FOR_MODULES()
in drivers/tty/serial/8250/8250_rsa.c by commit b20d6576cdb3 ("serial:
8250: export RSA functions"). Hopefully it's resolvable by a merge
commit fixup and we don't need to provide a temporary alias.

[1] https://lore.kernel.org/all/20250623-warmwasser-giftig-ff656fce89ad@brauner/
[2] https://lore.kernel.org/all/aFleJN_fE-RbSoFD@infradead.org/
[3] https://lore.kernel.org/all/20250623142836.GT1613200@noisy.programming.kicks-ass.net/

Signed-off-by: Vlastimil Babka <vbabka@suse.cz>
---
Vlastimil Babka (2):
      module: Restrict module namespace access to in-tree modules
      module: Rename EXPORT_SYMBOL_GPL_FOR_MODULES to EXPORT_SYMBOL_FOR_MODULES

 Documentation/core-api/symbol-namespaces.rst | 11 ++++++-----
 fs/anon_inodes.c                             |  2 +-
 include/linux/export.h                       |  2 +-
 kernel/module/main.c                         |  3 ++-
 scripts/mod/modpost.c                        |  6 +++++-
 5 files changed, 15 insertions(+), 9 deletions(-)
---
base-commit: d7b8f8e20813f0179d8ef519541a3527e7661d3a
change-id: 20250708-export_modules-12908fa41006

Best regards,
-- 
Vlastimil Babka <vbabka@suse.cz>


^ permalink raw reply

* Re: [PATCH 3/8] execmem: rework execmem_cache_free()
From: Peter Zijlstra @ 2025-07-08  7:26 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Liam R. Howlett, Andrew Morton, Andy Lutomirski, Borislav Petkov,
	Daniel Gomez, Dave Hansen, Ingo Molnar, Luis Chamberlain,
	Mark Rutland, Masami Hiramatsu, H. Peter Anvin, Petr Pavlu,
	Sami Tolvanen, Steven Rostedt, Thomas Gleixner, linux-kernel,
	linux-mm, linux-modules, linux-trace-kernel, x86
In-Reply-To: <aGvj2gUHntuuOp4H@kernel.org>

On Mon, Jul 07, 2025 at 06:12:26PM +0300, Mike Rapoport wrote:
> On Mon, Jul 07, 2025 at 11:06:25AM -0400, Liam R. Howlett wrote:
> > * Mike Rapoport <rppt@kernel.org> [250707 07:32]:
> > > On Mon, Jul 07, 2025 at 01:11:02PM +0200, Peter Zijlstra wrote:
> > > > 
> > > > 	err = __execmem_cache_free(&mas, ptr, GFP_KERNEL | __GFP_NORETRY);
> > > > 	if (err) {
> > > > 		mas_store_gfp(&mas, pending_free_set(ptr), GFP_KERNEL);
> > > > 		execmem_cache.pending_free_cnt++;
> > > > 		schedule_delayed_work(&execmem_cache_free_work, FREE_DELAY);
> > > > 		return true;
> > > > 	}
> > > > 
> > > > 	schedule_work(&execmem_cache_clean_work);
> > > > 	return true;
> > > > }
> > > > 
> > > > And now I have to ask what happens if mas_store_gfp() returns an error?
> > > 
> > > AFAIU it won't. mas points to exact slot we've got the area from, nothing else
> > > can modify the tree because of the mutex, so that mas_store_gfp()
> > > essentially updates the value at an existing entry.
> > > 
> > > I'll add a comment about it.
> > > 
> > > Added @Liam to make sure I'm not saying nonsense :)
> > > 
> > 
> > Yes, if there is already a node with a value with the same range, there
> > will be no allocations that will happen, so it'll just change the
> > pointer for you.  This is a slot store operation.
> > 
> > But, if it's possible to have no entries (an empty tree, or a single
> > value at 0), you will most likely allocate a node to store it, which is
> > 256B.
> > 
> > I don't think this is a concern in this particular case though as you
> > are searching for an entry and storing, so it needs to exist.  So
> > really, the only scenario here is if you store 1 - ULONG_MAX (without
> > having expanded a root node) or 0 - ULONG_MAX, and that seems invalid.
> 
> Thanks for clarification, Liam!
> The tree cannot be empty at that point and if it has a single value, it
> won't be at 0, I'm quite sure no architecture has execmem areas at 0.

Would it make sense to have something like GFP_NO_ALLOC to pass to
functions like this where we know it won't actually allocate -- and
which when it does reach the allocator generates a WARN and returns NULL
?

^ permalink raw reply

* Re: [PATCH 3/8] execmem: rework execmem_cache_free()
From: Mike Rapoport @ 2025-07-08  7:10 UTC (permalink / raw)
  To: Yann Ylavic
  Cc: Andrew Morton, Andy Lutomirski, Borislav Petkov, Daniel Gomez,
	Dave Hansen, Ingo Molnar, Luis Chamberlain, Mark Rutland,
	Masami Hiramatsu, H. Peter Anvin, Peter Zijlstra, Petr Pavlu,
	Sami Tolvanen, Steven Rostedt, Thomas Gleixner, linux-kernel,
	linux-mm, linux-modules, linux-trace-kernel, x86
In-Reply-To: <CAKQ1sVN9KZYt=M5cst+BonDMVL_wO3sbhgZ+JPzc=Rw9Qfz9tw@mail.gmail.com>

On Mon, Jul 07, 2025 at 05:32:11PM +0200, Yann Ylavic wrote:
> On Fri, Jul 4, 2025 at 3:54 PM Mike Rapoport <rppt@kernel.org> wrote:
> > +
> > +static void execmem_cache_free_slow(struct work_struct *work)
> > +{
> > +       struct maple_tree *busy_areas = &execmem_cache.busy_areas;
> > +       MA_STATE(mas, busy_areas, 0, ULONG_MAX);
> > +       void *area;
> > +
> > +       guard(mutex)(&execmem_cache.mutex);
> > +
> > +       if (!execmem_cache.pending_free_cnt)
> > +               return;
> > +
> > +       mas_for_each(&mas, area, ULONG_MAX) {
> > +               if (!is_pending_free(area))
> > +                       continue;
> > +
> > +               pending_free_clear(area);
> 
> Probably:
>                   area = pending_free_clear(area);
> ?

Right, thanks!
 
> > +               if (__execmem_cache_free(&mas, area, GFP_KERNEL))
> > +                       continue;
> > +
> > +               execmem_cache.pending_free_cnt--;
> > +       }
> > +
> > +       if (execmem_cache.pending_free_cnt)
> > +               schedule_delayed_work(&execmem_cache_free_work, FREE_DELAY);
> > +       else
> > +               schedule_work(&execmem_cache_clean_work);
> > +}
> 
> 
> Regards;
> Yann.

-- 
Sincerely yours,
Mike.

^ permalink raw reply

* Re: [PATCH 2/3] module: make structure definitions always visible
From: Daniel Gomez @ 2025-07-07 19:11 UTC (permalink / raw)
  To: Thomas Weißschuh, Luis Chamberlain, Petr Pavlu,
	Sami Tolvanen, Daniel Gomez, Brendan Higgins, David Gow, Rae Moar
  Cc: linux-modules, linux-kernel, linux-kselftest, kunit-dev
In-Reply-To: <20250612-kunit-ifdef-modules-v1-2-fdccd42dcff8@linutronix.de>

On 12/06/2025 16.53, Thomas WeiÃschuh wrote:
> To write code that works with both CONFIG_MODULES=y and CONFIG_MODULES=n
> it is convenient to use "if (IS_ENABLED(CONFIG_MODULES))" over raw #ifdef.
> The code will still fully typechecked but the unreachable parts are
> discarded by the compiler. This prevents accidental breakage when a certain
> kconfig combination was not specifically tested by the developer.
> This pattern is already supported to some extend by module.h defining
> empty stub functions if CONFIG_MODULES=n.
> However some users of module.h work on the structured defined by module.h.
> 
> Therefore these structure definitions need to be visible, too.

We are missing here which structures are needed. + we are making more things
visible than what we actually need.

> 
> Many structure members are still gated by specific configuration settings.
> The assumption for those is that the code using them will be gated behind
> the same configuration setting anyways.

I think code and kconfig need to reflect the actual dependencies. For example,
if CONFIG_LIVEPATCH depends on CONFIG_MODULES, we need to specify that in
Kconfig with depends on, as well as keep the code gated by these 2 configs with
ifdef/IS_ENABLED.

> 
> Signed-off-by: Thomas Weißschuh <thomas.weissschuh@linutronix.de>
> ---
>  include/linux/module.h | 23 ++++++++++++-----------
>  1 file changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/module.h b/include/linux/module.h
> index 52f7b0487a2733c56e2531a434887e56e1bf45b2..7f783e71636542b99db3dd869a9387d14992df45 100644
> --- a/include/linux/module.h
> +++ b/include/linux/module.h
> @@ -302,17 +302,6 @@ static typeof(name) __mod_device_table__##type##__##name		\
>  
>  struct notifier_block;
>  
> -#ifdef CONFIG_MODULES
> -
> -extern int modules_disabled; /* for sysctl */
> -/* Get/put a kernel symbol (calls must be symmetric) */
> -void *__symbol_get(const char *symbol);
> -void *__symbol_get_gpl(const char *symbol);
> -#define symbol_get(x)	({ \
> -	static const char __notrim[] \
> -		__used __section(".no_trim_symbol") = __stringify(x); \
> -	(typeof(&x))(__symbol_get(__stringify(x))); })
> -
>  enum module_state {
>  	MODULE_STATE_LIVE,	/* Normal state. */
>  	MODULE_STATE_COMING,	/* Full formed, running module_init. */
> @@ -598,6 +587,18 @@ struct module {
>  	struct _ddebug_info dyndbg_info;
>  #endif
>  } ____cacheline_aligned __randomize_layout;
> +
> +#ifdef CONFIG_MODULES
> +
> +extern int modules_disabled; /* for sysctl */
> +/* Get/put a kernel symbol (calls must be symmetric) */
> +void *__symbol_get(const char *symbol);
> +void *__symbol_get_gpl(const char *symbol);
> +#define symbol_get(x)	({ \
> +	static const char __notrim[] \
> +		__used __section(".no_trim_symbol") = __stringify(x); \
> +	(typeof(&x))(__symbol_get(__stringify(x))); })
> +

The patch exposes data structures that are not needed. + breaks the
config dependencies.

For example, before this patch:

#ifdef CONFIG_MODULES

{...}

struct mod_tree_node {

{...}

struct module_memory {
	void *base;
	bool is_rox;
	unsigned int size;

#ifdef CONFIG_MODULES_TREE_LOOKUP
	struct mod_tree_node mtn;
#endif
};

{...}
#endif /* CONFIG_MODULES */

After the patch, mod_tree_node is not needed externally. And the mtn field
in module_memory is exposed only under MODULES_TREE_LOOKUP and not MODULES
+ MODULES_TREE_LOOKUP.

I general, I see the issues I mentioned with LIVEPATCH, mod_tree_node, macros,
and LOOKUP.

>  #define MODULE_ARCH_INIT {}
>  #endif
> 

^ permalink raw reply

* Re: [PATCH 3/8] execmem: rework execmem_cache_free()
From: Yann Ylavic @ 2025-07-07 15:43 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Andrew Morton, Andy Lutomirski, Borislav Petkov, Daniel Gomez,
	Dave Hansen, Ingo Molnar, Luis Chamberlain, Mark Rutland,
	Masami Hiramatsu, H. Peter Anvin, Peter Zijlstra, Petr Pavlu,
	Sami Tolvanen, Steven Rostedt, Thomas Gleixner, linux-kernel,
	linux-mm, linux-modules, linux-trace-kernel, x86
In-Reply-To: <CAKQ1sVN9KZYt=M5cst+BonDMVL_wO3sbhgZ+JPzc=Rw9Qfz9tw@mail.gmail.com>

On Mon, Jul 7, 2025 at 5:32 PM Yann Ylavic <ylavic.dev@gmail.com> wrote:
>
> On Fri, Jul 4, 2025 at 3:54 PM Mike Rapoport <rppt@kernel.org> wrote:
> > +
> > +static void execmem_cache_free_slow(struct work_struct *work)
> > +{
> > +       struct maple_tree *busy_areas = &execmem_cache.busy_areas;
> > +       MA_STATE(mas, busy_areas, 0, ULONG_MAX);
> > +       void *area;
> > +
> > +       guard(mutex)(&execmem_cache.mutex);
> > +
> > +       if (!execmem_cache.pending_free_cnt)
> > +               return;
> > +
> > +       mas_for_each(&mas, area, ULONG_MAX) {
> > +               if (!is_pending_free(area))
> > +                       continue;
> > +
> > +               pending_free_clear(area);
>
> Probably:
>                   area = pending_free_clear(area);
> ?

Likewise in execmem_cache_free_slow() btw.

>
> > +               if (__execmem_cache_free(&mas, area, GFP_KERNEL))
> > +                       continue;
> > +
> > +               execmem_cache.pending_free_cnt--;
> > +       }
> > +
> > +       if (execmem_cache.pending_free_cnt)
> > +               schedule_delayed_work(&execmem_cache_free_work, FREE_DELAY);
> > +       else
> > +               schedule_work(&execmem_cache_clean_work);
> > +}
>
>
> Regards;
> Yann.

^ permalink raw reply

* Re: [PATCH 3/8] execmem: rework execmem_cache_free()
From: Yann Ylavic @ 2025-07-07 15:32 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Andrew Morton, Andy Lutomirski, Borislav Petkov, Daniel Gomez,
	Dave Hansen, Ingo Molnar, Luis Chamberlain, Mark Rutland,
	Masami Hiramatsu, H. Peter Anvin, Peter Zijlstra, Petr Pavlu,
	Sami Tolvanen, Steven Rostedt, Thomas Gleixner, linux-kernel,
	linux-mm, linux-modules, linux-trace-kernel, x86
In-Reply-To: <20250704134943.3524829-4-rppt@kernel.org>

On Fri, Jul 4, 2025 at 3:54 PM Mike Rapoport <rppt@kernel.org> wrote:
> +
> +static void execmem_cache_free_slow(struct work_struct *work)
> +{
> +       struct maple_tree *busy_areas = &execmem_cache.busy_areas;
> +       MA_STATE(mas, busy_areas, 0, ULONG_MAX);
> +       void *area;
> +
> +       guard(mutex)(&execmem_cache.mutex);
> +
> +       if (!execmem_cache.pending_free_cnt)
> +               return;
> +
> +       mas_for_each(&mas, area, ULONG_MAX) {
> +               if (!is_pending_free(area))
> +                       continue;
> +
> +               pending_free_clear(area);

Probably:
                  area = pending_free_clear(area);
?

> +               if (__execmem_cache_free(&mas, area, GFP_KERNEL))
> +                       continue;
> +
> +               execmem_cache.pending_free_cnt--;
> +       }
> +
> +       if (execmem_cache.pending_free_cnt)
> +               schedule_delayed_work(&execmem_cache_free_work, FREE_DELAY);
> +       else
> +               schedule_work(&execmem_cache_clean_work);
> +}


Regards;
Yann.

^ permalink raw reply

* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
From: Boqun Feng @ 2025-07-07 15:13 UTC (permalink / raw)
  To: Alice Ryhl
  Cc: Andreas Hindborg, Miguel Ojeda, Alex Gaynor, Gary Guo,
	Björn Roy Baron, Masahiro Yamada, Nathan Chancellor,
	Luis Chamberlain, Danilo Krummrich, Benno Lossin, Nicolas Schier,
	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
In-Reply-To: <CAH5fLgiKo=jN_V5cAe_AJqxxp7mQWqhKx7knkEj6js3yiU9sqA@mail.gmail.com>

On Mon, Jul 07, 2025 at 03:38:58PM +0200, Alice Ryhl wrote:
> On Mon, Jul 7, 2025 at 3:32 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
> >
> > Introduce the `SetOnce` type, a container that can only be written once.
> > The container uses an internal atomic to synchronize writes to the internal
> > value.
> >
> > Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> 
> LGTM:
> Reviewed-by: Alice Ryhl <aliceryhl@google.com>
> 
> > +impl<T> Drop for SetOnce<T> {
> > +    fn drop(&mut self) {
> > +        if self.init.load(Acquire) == 2 {
> > +            // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
> > +            // contains a valid value. We have exclusive access, as we hold a `mut` reference to
> > +            // `self`.
> > +            unsafe { drop_in_place(self.value.get()) };
> 
> This load does not need to be Acquire. It can be a Relaxed load or
> even an unsynchronized one since the access is exclusive.

Right, I think we can do the similar as Revocable here:

        if *self.init.get_mut() == 2 { }

Further, with my following Benno's suggestion and making `Atomic<T>` an
`UnsafeCell<T>:

	https://lore.kernel.org/rust-for-linux/aGhh-TvNOWhkt0JG@Mac.home/

compiler can generate a noalias reference here, which allows further
optimization.

Regards,
Boqun

> 
> Alice

^ permalink raw reply

* Re: [PATCH 3/8] execmem: rework execmem_cache_free()
From: Mike Rapoport @ 2025-07-07 15:12 UTC (permalink / raw)
  To: Liam R. Howlett, Peter Zijlstra, Andrew Morton, Andy Lutomirski,
	Borislav Petkov, Daniel Gomez, Dave Hansen, Ingo Molnar,
	Luis Chamberlain, Mark Rutland, Masami Hiramatsu, H. Peter Anvin,
	Petr Pavlu, Sami Tolvanen, Steven Rostedt, Thomas Gleixner,
	linux-kernel, linux-mm, linux-modules, linux-trace-kernel, x86
In-Reply-To: <oez2aoegd2dfq4h4fg2on2rsgwp36aumpedmobxkj7dlmaoeyr@sqz27uhgf3f7>

On Mon, Jul 07, 2025 at 11:06:25AM -0400, Liam R. Howlett wrote:
> * Mike Rapoport <rppt@kernel.org> [250707 07:32]:
> > On Mon, Jul 07, 2025 at 01:11:02PM +0200, Peter Zijlstra wrote:
> > > 
> > > 	err = __execmem_cache_free(&mas, ptr, GFP_KERNEL | __GFP_NORETRY);
> > > 	if (err) {
> > > 		mas_store_gfp(&mas, pending_free_set(ptr), GFP_KERNEL);
> > > 		execmem_cache.pending_free_cnt++;
> > > 		schedule_delayed_work(&execmem_cache_free_work, FREE_DELAY);
> > > 		return true;
> > > 	}
> > > 
> > > 	schedule_work(&execmem_cache_clean_work);
> > > 	return true;
> > > }
> > > 
> > > And now I have to ask what happens if mas_store_gfp() returns an error?
> > 
> > AFAIU it won't. mas points to exact slot we've got the area from, nothing else
> > can modify the tree because of the mutex, so that mas_store_gfp()
> > essentially updates the value at an existing entry.
> > 
> > I'll add a comment about it.
> > 
> > Added @Liam to make sure I'm not saying nonsense :)
> > 
> 
> Yes, if there is already a node with a value with the same range, there
> will be no allocations that will happen, so it'll just change the
> pointer for you.  This is a slot store operation.
> 
> But, if it's possible to have no entries (an empty tree, or a single
> value at 0), you will most likely allocate a node to store it, which is
> 256B.
> 
> I don't think this is a concern in this particular case though as you
> are searching for an entry and storing, so it needs to exist.  So
> really, the only scenario here is if you store 1 - ULONG_MAX (without
> having expanded a root node) or 0 - ULONG_MAX, and that seems invalid.

Thanks for clarification, Liam!
The tree cannot be empty at that point and if it has a single value, it
won't be at 0, I'm quite sure no architecture has execmem areas at 0.

 
> Thanks,
> Liam

-- 
Sincerely yours,
Mike.

^ permalink raw reply

* Re: [PATCH 3/8] execmem: rework execmem_cache_free()
From: Liam R. Howlett @ 2025-07-07 15:06 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Peter Zijlstra, Andrew Morton, Andy Lutomirski, Borislav Petkov,
	Daniel Gomez, Dave Hansen, Ingo Molnar, Luis Chamberlain,
	Mark Rutland, Masami Hiramatsu, H. Peter Anvin, Petr Pavlu,
	Sami Tolvanen, Steven Rostedt, Thomas Gleixner, linux-kernel,
	linux-mm, linux-modules, linux-trace-kernel, x86
In-Reply-To: <aGuwMtxsouXvdiCK@kernel.org>

* Mike Rapoport <rppt@kernel.org> [250707 07:32]:
> On Mon, Jul 07, 2025 at 01:11:02PM +0200, Peter Zijlstra wrote:
> > On Fri, Jul 04, 2025 at 04:49:38PM +0300, Mike Rapoport wrote:
> > >  static bool execmem_cache_free(void *ptr)
> > >  {
> > >  	struct maple_tree *busy_areas = &execmem_cache.busy_areas;
> > >  	unsigned long addr = (unsigned long)ptr;
> > >  	MA_STATE(mas, busy_areas, addr, addr);
> > >  	void *area;
> > > +	int err;
> > > +
> > > +	guard(mutex)(&execmem_cache.mutex);
> > >  
> > >  	area = mas_walk(&mas);
> > > +	if (!area)
> > >  		return false;
> > >  
> > > +	err = __execmem_cache_free(&mas, ptr, GFP_KERNEL | __GFP_NORETRY);
> > > +	if (err)
> > > +		goto err_slowpath;
> > >  
> > >  	schedule_work(&execmem_cache_clean_work);
> > >  
> > >  	return true;
> > > +
> > > +err_slowpath:
> > > +	mas_store_gfp(&mas, pending_free_set(ptr), GFP_KERNEL);
> > > +	execmem_cache.pending_free_cnt++;
> > > +	schedule_delayed_work(&execmem_cache_free_work, FREE_DELAY);
> > > +	return true;
> > >  }
> > 
> > This is a bit if an anti-pattern, using guard() and error goto. Since
> 
> Good to know :)
> 
> > there is only the one site, its best to write it like so:
> > 
> > static bool execmem_cache_free(void *ptr)
> > {
> > 	struct maple_tree *busy_areas = &execmem_cache.busy_areas;
> > 	unsigned long addr = (unsigned long)ptr;
> > 	MA_STATE(mas, busy_areas, addr, addr);
> > 	void *area;
> > 	int err;
> > 
> > 	guard(mutex)(&execmem_cache.mutex);
> > 
> > 	area = mas_walk(&mas);
> > 	if (!area)
> > 		return false;
> > 
> > 	err = __execmem_cache_free(&mas, ptr, GFP_KERNEL | __GFP_NORETRY);
> > 	if (err) {
> > 		mas_store_gfp(&mas, pending_free_set(ptr), GFP_KERNEL);
> > 		execmem_cache.pending_free_cnt++;
> > 		schedule_delayed_work(&execmem_cache_free_work, FREE_DELAY);
> > 		return true;
> > 	}
> > 
> > 	schedule_work(&execmem_cache_clean_work);
> > 	return true;
> > }
> > 
> > And now I have to ask what happens if mas_store_gfp() returns an error?
> 
> AFAIU it won't. mas points to exact slot we've got the area from, nothing else
> can modify the tree because of the mutex, so that mas_store_gfp()
> essentially updates the value at an existing entry.
> 
> I'll add a comment about it.
> 
> Added @Liam to make sure I'm not saying nonsense :)
> 

Yes, if there is already a node with a value with the same range, there
will be no allocations that will happen, so it'll just change the
pointer for you.  This is a slot store operation.

But, if it's possible to have no entries (an empty tree, or a single
value at 0), you will most likely allocate a node to store it, which is
256B.

I don't think this is a concern in this particular case though as you
are searching for an entry and storing, so it needs to exist.  So
really, the only scenario here is if you store 1 - ULONG_MAX (without
having expanded a root node) or 0 - ULONG_MAX, and that seems invalid.

Thanks,
Liam

^ permalink raw reply

* Re: [PATCH v15 0/7] rust: extend `module!` macro with integer parameter support
From: Miguel Ojeda @ 2025-07-07 14:05 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
	Benno Lossin, Nicolas Schier, 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
In-Reply-To: <20250707-module-params-v3-v15-0-c1f4269a57b9@kernel.org>

On Mon, Jul 7, 2025 at 3:31 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Based on code by Adam Bratschi-Kaye lifted from the original `rust` branch [1].

I would suggest adding this sort of note to the commit messages,
especially since the commits have no Co-developed-by/Link tags
otherwise.

From our discussion in Zulip: the code itself is older than that the
merge above. I think you wanted an example sentence for this --
something simple could be e.g.

    Based on the original module parameter support by Miguel [1],
later extended and generalized by Adam for more types [2][3].
Originally tracked at [4].

    Link: https://github.com/Rust-for-Linux/linux/pull/7 [1]
    Link: https://github.com/Rust-for-Linux/linux/pull/82 [2]
    Link: https://github.com/Rust-for-Linux/linux/pull/87 [3]
    Link: https://github.com/Rust-for-Linux/linux/issues/11 [4]

By the way, I guess you should inherit that issue in the last link :)
It had some details about things we may or may not want to support
etc. that I looked up back then. If you prefer that we close it or
that we create sub-issues, that is fine -- up to you!

Finally, if you end up adding strings, please link to Adam's
https://github.com/Rust-for-Linux/linux/pull/110.

Thanks!

Cheers,
Miguel

^ permalink raw reply

* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
From: Alice Ryhl @ 2025-07-07 13:38 UTC (permalink / raw)
  To: Andreas Hindborg
  Cc: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Masahiro Yamada, Nathan Chancellor,
	Luis Chamberlain, Danilo Krummrich, Benno Lossin, Nicolas Schier,
	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
In-Reply-To: <20250707-module-params-v3-v15-1-c1f4269a57b9@kernel.org>

On Mon, Jul 7, 2025 at 3:32 PM Andreas Hindborg <a.hindborg@kernel.org> wrote:
>
> Introduce the `SetOnce` type, a container that can only be written once.
> The container uses an internal atomic to synchronize writes to the internal
> value.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>

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

> +impl<T> Drop for SetOnce<T> {
> +    fn drop(&mut self) {
> +        if self.init.load(Acquire) == 2 {
> +            // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
> +            // contains a valid value. We have exclusive access, as we hold a `mut` reference to
> +            // `self`.
> +            unsafe { drop_in_place(self.value.get()) };

This load does not need to be Acquire. It can be a Relaxed load or
even an unsynchronized one since the access is exclusive.

Alice

^ permalink raw reply

* Re: [PATCH v15 1/7] rust: sync: add `SetOnce`
From: Andreas Hindborg @ 2025-07-07 13:35 UTC (permalink / raw)
  To: Miguel Ojeda
  Cc: Alex Gaynor, Boqun Feng, Gary Guo, Björn Roy Baron,
	Alice Ryhl, Masahiro Yamada, Nathan Chancellor, Luis Chamberlain,
	Danilo Krummrich, Benno Lossin, Nicolas Schier, 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
In-Reply-To: <20250707-module-params-v3-v15-1-c1f4269a57b9@kernel.org>

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

> Introduce the `SetOnce` type, a container that can only be written once.
> The container uses an internal atomic to synchronize writes to the internal
> value.
>
> Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
> ---
>  rust/kernel/sync.rs          |   2 +
>  rust/kernel/sync/set_once.rs | 125 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 127 insertions(+)
>
> diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
> index 81e3a806e57e2..13e6bc7fa87ac 100644
> --- a/rust/kernel/sync.rs
> +++ b/rust/kernel/sync.rs
> @@ -18,6 +18,7 @@
>  mod locked_by;
>  pub mod poll;
>  pub mod rcu;
> +mod set_once;
>  
>  pub use arc::{Arc, ArcBorrow, UniqueArc};
>  pub use completion::Completion;
> @@ -26,6 +27,7 @@
>  pub use lock::mutex::{new_mutex, Mutex, MutexGuard};
>  pub use lock::spinlock::{new_spinlock, SpinLock, SpinLockGuard};
>  pub use locked_by::LockedBy;
> +pub use set_once::SetOnce;
>  
>  /// Represents a lockdep class. It's a wrapper around C's `lock_class_key`.
>  #[repr(transparent)]
> diff --git a/rust/kernel/sync/set_once.rs b/rust/kernel/sync/set_once.rs
> new file mode 100644
> index 0000000000000..e1e31f5faed09
> --- /dev/null
> +++ b/rust/kernel/sync/set_once.rs
> @@ -0,0 +1,125 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! A container that can be initialized at most once.
> +
> +use super::atomic::ordering::Acquire;
> +use super::atomic::ordering::Relaxed;
> +use super::atomic::ordering::Release;
> +use super::atomic::Atomic;
> +use core::ptr::drop_in_place;
> +use kernel::types::Opaque;
> +
> +/// A container that can be populated at most once. Thread safe.
> +///
> +/// Once the a [`SetOnce`] is populated, it remains populated by the same object for the
> +/// lifetime `Self`.
> +///
> +/// # Invariants
> +///
> +/// - `init` may only increase in value.
> +/// - `init` may only assume values in the range `0..=2`.
> +/// - `init == 0` if and only if the container is empty.
> +/// - `init == 1` if and only if being initialized.
> +/// - `init == 2` if and only if the container is populated and valid for shared access.
> +///
> +/// # Example
> +///
> +/// ```
> +/// # use kernel::sync::SetOnce;
> +/// let value = SetOnce::new();
> +/// assert_eq!(None, value.as_ref());
> +///
> +/// let status = value.populate(42u8);
> +/// assert_eq!(true, status);
> +/// assert_eq!(Some(&42u8), value.as_ref());
> +/// assert_eq!(Some(42u8), value.copy());
> +///
> +/// let status = value.populate(101u8);
> +/// assert_eq!(false, status);
> +/// assert_eq!(Some(&42u8), value.as_ref());
> +/// assert_eq!(Some(42u8), value.copy());
> +/// ```
> +pub struct SetOnce<T> {
> +    init: Atomic<u32>,
> +    value: Opaque<T>,
> +}
> +
> +impl<T> Default for SetOnce<T> {
> +    fn default() -> Self {
> +        Self::new()
> +    }
> +}
> +
> +// TODO: change names

I just saw that this line decided to stick around, that was obviously
not intentional. Just disregard this line.


Best regards,
Andreas Hindborg



^ permalink raw reply

* [PATCH v15 3/7] rust: introduce module_param module
From: Andreas Hindborg @ 2025-07-07 13:29 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
	Benno Lossin, Benno Lossin, Nicolas Schier
  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
In-Reply-To: <20250707-module-params-v3-v15-0-c1f4269a57b9@kernel.org>

Add types and traits for interfacing the C moduleparam API.

Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/lib.rs          |   1 +
 rust/kernel/module_param.rs | 181 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 182 insertions(+)

diff --git a/rust/kernel/lib.rs b/rust/kernel/lib.rs
index 6b4774b2b1c37..2b439ea061850 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 0000000000000..9b187ed1d3513
--- /dev/null
+++ b/rust/kernel/module_param.rs
@@ -0,0 +1,181 @@
+// 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::SetOnce;
+
+/// Newtype to make `bindings::kernel_param` [`Sync`].
+#[repr(transparent)]
+#[doc(hidden)]
+pub struct KernelParam(bindings::kernel_param);
+
+impl KernelParam {
+    #[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 KernelParam {}
+
+/// 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 {
+    /// 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 SetOnce<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 {
+            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: SetOnce<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: SetOnce::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 value(&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 {
+        core::ptr::from_ref(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.2



^ permalink raw reply related

* [PATCH v15 4/7] rust: module: use a reference in macros::module::module
From: Andreas Hindborg @ 2025-07-07 13:29 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
	Benno Lossin, Benno Lossin, Nicolas Schier
  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
In-Reply-To: <20250707-module-params-v3-v15-0-c1f4269a57b9@kernel.org>

When we add parameter support to the module macro, we want to be able to
pass a reference to `ModuleInfo` to a helper function. That is not possible
when we move out of the local `modinfo`. So change the function to access
the local via reference rather than value.

Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/macros/module.rs | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/rust/macros/module.rs b/rust/macros/module.rs
index 2ddd2eeb28521..1a867a1e787ed 100644
--- a/rust/macros/module.rs
+++ b/rust/macros/module.rs
@@ -179,26 +179,26 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
     // Rust does not allow hyphens in identifiers, use underscore instead.
     let ident = info.name.replace('-', "_");
     let mut modinfo = ModInfoBuilder::new(ident.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(authors) = info.authors {
+    if let Some(authors) = &info.authors {
         for author in authors {
-            modinfo.emit("author", &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);
         }
     }
 

-- 
2.47.2



^ permalink raw reply related

* [PATCH v15 7/7] modules: add rust modules files to MAINTAINERS
From: Andreas Hindborg @ 2025-07-07 13:29 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
	Benno Lossin, Benno Lossin, Nicolas Schier
  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
In-Reply-To: <20250707-module-params-v3-v15-0-c1f4269a57b9@kernel.org>

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]

Acked-by: Daniel Gomez <da.gomez@samsung.com>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 MAINTAINERS | 2 ++
 1 file changed, 2 insertions(+)

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

-- 
2.47.2



^ permalink raw reply related

* [PATCH v15 2/7] rust: str: add radix prefixed integer parsing functions
From: Andreas Hindborg @ 2025-07-07 13:29 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
	Benno Lossin, Benno Lossin, Nicolas Schier
  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
In-Reply-To: <20250707-module-params-v3-v15-0-c1f4269a57b9@kernel.org>

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 Gomez <da.gomez@samsung.com>
Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Suggested-by: Benno Lossin <benno.lossin@proton.me>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/str.rs           |   2 +
 rust/kernel/str/parse_int.rs | 148 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 150 insertions(+)

diff --git a/rust/kernel/str.rs b/rust/kernel/str.rs
index a927db8e079c3..2b6c8b4a0ae4b 100644
--- a/rust/kernel/str.rs
+++ b/rust/kernel/str.rs
@@ -8,6 +8,8 @@
 
 use crate::prelude::*;
 
+pub mod parse_int;
+
 /// Byte string without UTF-8 validity guarantee.
 #[repr(transparent)]
 pub struct BStr([u8]);
diff --git a/rust/kernel/str/parse_int.rs b/rust/kernel/str/parse_int.rs
new file mode 100644
index 0000000000000..48eb4c202984c
--- /dev/null
+++ b/rust/kernel/str/parse_int.rs
@@ -0,0 +1,148 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! Integer parsing functions.
+//!
+//! 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::prelude::*;
+    use crate::str::BStr;
+
+    /// Trait that allows parsing a [`&BStr`] to an integer with a radix.
+    pub trait FromStrRadix: Sized {
+        /// Parse `src` to [`Self`] using radix `radix`.
+        fn from_str_radix(src: &BStr, radix: u32) -> Result<Self>;
+
+        /// Tries to convert `value` into [`Self`] and negates the resulting value.
+        fn from_u64_negated(value: u64) -> Result<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://docs.kernel.org/core-api/kernel-api.html#c.kstrtol
+/// [`kstrtoul()`]: https://docs.kernel.org/core-api/kernel-api.html#c.kstrtoul
+///
+/// # Examples
+///
+/// ```
+/// # use kernel::str::parse_int::ParseInt;
+/// # use kernel::b_str;
+///
+/// assert_eq!(Ok(0u8), 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(127i8), i8::from_str(b_str!("127")));
+/// assert!(i8::from_str(b_str!("128")).is_err());
+/// assert_eq!(Ok(-128i8), i8::from_str(b_str!("-128")));
+/// assert!(i8::from_str(b_str!("-129")).is_err());
+/// assert_eq!(Ok(255u8), 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 bits.
+                let val =
+                    u64::from_str_radix(core::str::from_utf8(digits).map_err(|_| EINVAL)?, radix)
+                        .map_err(|_| EINVAL)?;
+                Self::from_u64_negated(val)
+            }
+            _ => {
+                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> {
+                    <$ty>::from_str_radix(core::str::from_utf8(src).map_err(|_| EINVAL)?, radix)
+                        .map_err(|_| EINVAL)
+                }
+
+                fn from_u64_negated(value: u64) -> Result<Self> {
+                    const ABS_MIN: u64 = {
+                        #[allow(unused_comparisons)]
+                        if <$ty>::MIN < 0 {
+                            1u64 << (<$ty>::BITS - 1)
+                        } else {
+                            0
+                        }
+                    };
+
+                    if value > ABS_MIN {
+                        return Err(EINVAL);
+                    }
+
+                    if value == ABS_MIN {
+                        return Ok(<$ty>::MIN);
+                    }
+
+                    // SAFETY: The above checks guarantee that `value` fits into `Self`:
+                    // - if `Self` is unsigned, then `ABS_MIN == 0` and thus we have returned above
+                    //   (either `EINVAL` or `MIN`).
+                    // - if `Self` is signed, then we have that `0 <= value < ABS_MIN`. And since
+                    //   `ABS_MIN - 1` fits into `Self` by construction, `value` also does.
+                    let value: Self = unsafe { value.try_into().unwrap_unchecked() };
+
+                    Ok((!value).wrapping_add(1))
+                }
+            }
+
+            impl ParseInt for $ty {}
+        )*
+    };
+}
+
+impl_parse_int![i8, u8, i16, u16, i32, u32, i64, u64, isize, usize];

-- 
2.47.2



^ permalink raw reply related

* [PATCH v15 5/7] rust: module: update the module macro with module parameter support
From: Andreas Hindborg @ 2025-07-07 13:29 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
	Benno Lossin, Benno Lossin, Nicolas Schier
  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
In-Reply-To: <20250707-module-params-v3-v15-0-c1f4269a57b9@kernel.org>

Allow module parameters to be declared in the rust `module!` macro.

Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/macros/helpers.rs |  25 +++++++
 rust/macros/lib.rs     |  31 +++++++++
 rust/macros/module.rs  | 178 ++++++++++++++++++++++++++++++++++++++++++++++---
 3 files changed, 224 insertions(+), 10 deletions(-)

diff --git a/rust/macros/helpers.rs b/rust/macros/helpers.rs
index e2602be402c10..365d7eb499c08 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())
@@ -103,3 +114,17 @@ pub(crate) fn file() -> String {
         proc_macro::Span::call_site().file()
     }
 }
+
+/// 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
+}
diff --git a/rust/macros/lib.rs b/rust/macros/lib.rs
index fa847cf3a9b5f..2fb520dc930af 100644
--- a/rust/macros/lib.rs
+++ b/rust/macros/lib.rs
@@ -28,6 +28,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
@@ -44,6 +68,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);
@@ -52,6 +82,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 1a867a1e787ed..c1400597774a5 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")
         };
 
+        let buffer = if param {
+            &mut self.param_buffer
+        } else {
+            &mut self.buffer
+        };
+
         write!(
-            &mut self.buffer,
+            buffer,
             "
                 {cfg}
                 #[doc(hidden)]
@@ -75,20 +83,119 @@ 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});
+
+                const _: () = {{
+                    #[link_section = \"__param\"]
+                    #[used]
+                    static __{module_name}_{param_name}_struct:
+                        ::kernel::module_param::KernelParam =
+                        ::kernel::module_param::KernelParam::new(
+                            ::kernel::bindings::kernel_param {{
+                                name: if ::core::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 {{
+                                    core::ptr::from_ref(&::kernel::bindings::__this_module)
+                                        .cast_mut()
+                                }},
+                                #[cfg(not(MODULE))]
+                                mod_: ::core::ptr::null_mut(),
+                                ops: core::ptr::from_ref(&{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_void_ptr()
+                                }},
+                            }}
+                        );
+                }};
+                ",
+                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,
@@ -99,6 +206,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 {
@@ -114,6 +265,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 +292,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 \"{key}\". Valid keys are: {EXPECTED_KEYS:?}."),
             }
 
@@ -205,7 +358,9 @@ pub(crate) fn module(ts: TokenStream) -> TokenStream {
     // 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!(
         "
@@ -369,15 +524,18 @@ unsafe fn __exit() {{
                             __MOD.assume_init_drop();
                         }}
                     }}
-
                     {modinfo}
                 }}
             }}
+            mod module_parameters {{
+                {params}
+            }}
         ",
         type_ = info.type_,
         name = info.name,
         ident = ident,
         modinfo = modinfo.buffer,
+        params = modinfo.param_buffer,
         initcall_section = ".initcall6.init"
     )
     .parse()

-- 
2.47.2



^ permalink raw reply related

* [PATCH v15 1/7] rust: sync: add `SetOnce`
From: Andreas Hindborg @ 2025-07-07 13:29 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
	Benno Lossin, Benno Lossin, Nicolas Schier
  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
In-Reply-To: <20250707-module-params-v3-v15-0-c1f4269a57b9@kernel.org>

Introduce the `SetOnce` type, a container that can only be written once.
The container uses an internal atomic to synchronize writes to the internal
value.

Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 rust/kernel/sync.rs          |   2 +
 rust/kernel/sync/set_once.rs | 125 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 127 insertions(+)

diff --git a/rust/kernel/sync.rs b/rust/kernel/sync.rs
index 81e3a806e57e2..13e6bc7fa87ac 100644
--- a/rust/kernel/sync.rs
+++ b/rust/kernel/sync.rs
@@ -18,6 +18,7 @@
 mod locked_by;
 pub mod poll;
 pub mod rcu;
+mod set_once;
 
 pub use arc::{Arc, ArcBorrow, UniqueArc};
 pub use completion::Completion;
@@ -26,6 +27,7 @@
 pub use lock::mutex::{new_mutex, Mutex, MutexGuard};
 pub use lock::spinlock::{new_spinlock, SpinLock, SpinLockGuard};
 pub use locked_by::LockedBy;
+pub use set_once::SetOnce;
 
 /// Represents a lockdep class. It's a wrapper around C's `lock_class_key`.
 #[repr(transparent)]
diff --git a/rust/kernel/sync/set_once.rs b/rust/kernel/sync/set_once.rs
new file mode 100644
index 0000000000000..e1e31f5faed09
--- /dev/null
+++ b/rust/kernel/sync/set_once.rs
@@ -0,0 +1,125 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! A container that can be initialized at most once.
+
+use super::atomic::ordering::Acquire;
+use super::atomic::ordering::Relaxed;
+use super::atomic::ordering::Release;
+use super::atomic::Atomic;
+use core::ptr::drop_in_place;
+use kernel::types::Opaque;
+
+/// A container that can be populated at most once. Thread safe.
+///
+/// Once the a [`SetOnce`] is populated, it remains populated by the same object for the
+/// lifetime `Self`.
+///
+/// # Invariants
+///
+/// - `init` may only increase in value.
+/// - `init` may only assume values in the range `0..=2`.
+/// - `init == 0` if and only if the container is empty.
+/// - `init == 1` if and only if being initialized.
+/// - `init == 2` if and only if the container is populated and valid for shared access.
+///
+/// # Example
+///
+/// ```
+/// # use kernel::sync::SetOnce;
+/// let value = SetOnce::new();
+/// assert_eq!(None, value.as_ref());
+///
+/// let status = value.populate(42u8);
+/// assert_eq!(true, status);
+/// assert_eq!(Some(&42u8), value.as_ref());
+/// assert_eq!(Some(42u8), value.copy());
+///
+/// let status = value.populate(101u8);
+/// assert_eq!(false, status);
+/// assert_eq!(Some(&42u8), value.as_ref());
+/// assert_eq!(Some(42u8), value.copy());
+/// ```
+pub struct SetOnce<T> {
+    init: Atomic<u32>,
+    value: Opaque<T>,
+}
+
+impl<T> Default for SetOnce<T> {
+    fn default() -> Self {
+        Self::new()
+    }
+}
+
+// TODO: change names
+
+impl<T> SetOnce<T> {
+    /// Create a new [`SetOnce`].
+    ///
+    /// The returned instance will be empty.
+    pub const fn new() -> Self {
+        // INVARIANT: The container is empty and we initialize `init` to `0`.
+        Self {
+            value: Opaque::uninit(),
+            init: Atomic::new(0),
+        }
+    }
+
+    /// Get a reference to the contained object.
+    ///
+    /// Returns [`None`] if this [`SetOnce`] is empty.
+    pub fn as_ref(&self) -> Option<&T> {
+        if self.init.load(Acquire) == 2 {
+            // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
+            // contains a valid value.
+            Some(unsafe { &*self.value.get() })
+        } else {
+            None
+        }
+    }
+
+    /// Populate the [`SetOnce`].
+    ///
+    /// Returns `true` if the [`SetOnce`] was successfully populated.
+    pub fn populate(&self, value: T) -> bool {
+        // INVARIANT: If the swap succeeds:
+        //  - We increase `init`.
+        //  - We write the valid value `1` to `init`.
+        //  - Only one thread can succeed in this write, so we have exclusive access after the
+        //    write.
+        if let Ok(0) = self.init.cmpxchg(0, 1, Relaxed) {
+            // SAFETY: By the type invariants of `Self`, the fact that we succeeded in writing `1`
+            // to `self.init` means we obtained exclusive access to the contained object.
+            unsafe { core::ptr::write(self.value.get(), value) };
+            // INVARIANT:
+            //  - We increase `init`.
+            //  - We write the valid value `2` to `init`.
+            //  - We release our exclusive access to the contained object and the object is now
+            //    valid for shared access.
+            self.init.store(2, Release);
+            true
+        } else {
+            false
+        }
+    }
+
+    /// Get a copy of the contained object.
+    ///
+    /// Returns [`None`] if the [`SetOnce`] is empty.
+    pub fn copy(&self) -> Option<T>
+    where
+        T: Copy,
+    {
+        self.as_ref().copied()
+    }
+}
+
+impl<T> Drop for SetOnce<T> {
+    fn drop(&mut self) {
+        if self.init.load(Acquire) == 2 {
+            // SAFETY: By the type invariants of `Self`, `self.init == 2` means that `self.value`
+            // contains a valid value. We have exclusive access, as we hold a `mut` reference to
+            // `self`.
+            unsafe { drop_in_place(self.value.get()) };
+        }
+    }
+}

-- 
2.47.2



^ permalink raw reply related

* [PATCH v15 0/7] rust: extend `module!` macro with integer parameter support
From: Andreas Hindborg @ 2025-07-07 13:29 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
	Benno Lossin, Benno Lossin, Nicolas Schier
  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 v15:
- Rebase on v6.16-rc5.
- Dedent code in module macro for better formatting.
- Rename `OnceLock` to `SetOnce`.
- Use "being initialized" rather than "being mutably accessed" when
  describing initialization state of `OnceLock`.
- Use `Relaxed` ordering when transitioning to exclusive access in
  `OnceLock`.
- Add drop implementation for `OnceLock`.
- Re-export `OnceLock` from `kernel::sync` module.
- Improve indentation of in macro code. Prefix `cfg` to `::core::cfg` in
  macro code.
- Use `core::ptr::from_ref` rather than `as` casts.
- Hide `KernelParam` instances behind `const _: ()` blocks.
- Rename `ModuleParamAccess::get` to `ModuleParamAccess::value`.
- Rename `RacyKernelParam` to `KernelParam`.
- Remove `ModuleParam::Value`.
- Move `copy` implementation of `OnceLock`.
- Update safety comments and invariants of `OnceLock`.
- Link to v14: https://lore.kernel.org/r/20250702-module-params-v3-v14-0-5b1cc32311af@kernel.org

Note: This series now depends on the atomics series [1].

[1] https://lore.kernel.org/all/20250618164934.19817-1-boqun.feng@gmail.com

Changes in v14:
- Remove unnecessary `crate::` prefix from `module_param::set_param`.
- Make `FromStrRadix` safe again by moving unsafe blocks to macro implementation (thanks Benno).
- Use `core::ptr::write` in `set_param` and drop safety requirement regarding initialization.
- Add a TODO to use `SyncUnsafeCell` for `ModuleParamAccess` when available.
- Add a NOTE regarding `Copy` bound on `ModuleParam`.
- Remove `'static` lifetime qualifier from `ModuleParam::try_from_param_arg` argument.
- Fix a typo in the safety requirements for `set_param`.
- Remove unused `#[macro_export]` attribute.
- Remove obsolete documentation for `ModuleParam::try_from_param_arg`.
- Make `RacyKernelParam` tuple field private.
- Introduce `OnceLock` and use that to synchronize population of parameter values.
- Link to v13: https://lore.kernel.org/r/20250612-module-params-v3-v13-0-bc219cd1a3f8@kernel.org

Changes in v13:
- remove absolute path for `ffi` types.
- Split patch 2 into 4 separate patches.
- Overhaul safety framework for `set_param`.
- Remove generated docs for `kernel_param_ops`.
- Move `parse_int` to separate file.
- Rebase on v6.16-rc1
- Link to v12: https://lore.kernel.org/r/20250506-module-params-v3-v12-0-c04d80c8a2b1@kernel.org

Changes in v12:
- Assign through pointer rather than using `core::ptr::replace`.
- Prevent a potential use-after-free during module teardown.
- Link to v11: https://lore.kernel.org/r/20250502-module-params-v3-v11-0-6096875a2b78@kernel.org

Changes in v11:
- Apply a few nits from Miguel.
- Link to v10: https://lore.kernel.org/r/20250501-module-params-v3-v10-0-4da485d343d5@kernel.org

Changes in v10:
- Apply fixups from Miguel:
  - Add integer type suffixes to `assert!` in tests.
  - Fix links to docs.kernel.org.
  - Applyy markdown and intra-doc links where possible.
  - Change to `///` for `mod` docs.
  - Slightly reword a comment.
  - Pluralize "Examples" section name.
  - Hide `use`s in example.
  - Removed `#[expect]` for the `rusttest` target.
- Link to v9: https://lore.kernel.org/r/20250321-module-params-v3-v9-0-28b905f2e345@kernel.org

Changes in v9:
- Remove UB when parsing the minimum integer values.
- Make `FromStr` trait unsafe, since wrong implementations can cause UB.
- Drop patches that were applied to rust-next.
- Link to v8: https://lore.kernel.org/r/20250227-module-params-v3-v8-0-ceeee85d9347@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: sync: add `SetOnce`
      rust: str: add radix prefixed integer parsing functions
      rust: introduce module_param module
      rust: module: use a reference in macros::module::module
      rust: module: update the module macro with module parameter support
      rust: samples: add a module parameter to the rust_minimal sample
      modules: add rust modules files to MAINTAINERS

 MAINTAINERS                  |   2 +
 rust/kernel/lib.rs           |   1 +
 rust/kernel/module_param.rs  | 181 +++++++++++++++++++++++++++++++++++++++
 rust/kernel/str.rs           |   2 +
 rust/kernel/str/parse_int.rs | 148 ++++++++++++++++++++++++++++++++
 rust/kernel/sync.rs          |   2 +
 rust/kernel/sync/set_once.rs | 125 +++++++++++++++++++++++++++
 rust/macros/helpers.rs       |  25 ++++++
 rust/macros/lib.rs           |  31 +++++++
 rust/macros/module.rs        | 198 ++++++++++++++++++++++++++++++++++++++-----
 samples/rust/rust_minimal.rs |  10 +++
 11 files changed, 705 insertions(+), 20 deletions(-)
---
base-commit: 47633099a672fc7bfe604ef454e4f116e2c954b1
change-id: 20241211-module-params-v3-ae7e5c8d8b5a
prerequisite-message-id: 20250618164934.19817-1-boqun.feng@gmail.com
prerequisite-patch-id: 997f74bd63d81dc66725d7f84ffcf5a3517da1eb
prerequisite-patch-id: 3e3f8bdada027406a356a2b35aebe13edbda0a94
prerequisite-patch-id: 468b093532aa503c23302142b8cfe7989e3cb8f1
prerequisite-patch-id: 96ca162a1e95a29d3aca2e748d600a31417b83e5
prerequisite-patch-id: 8f78abede975c0c9690ee0f4cf4e7af3cb145191
prerequisite-patch-id: a716ee7a51184d224ef60812ecfffadb51980153
prerequisite-patch-id: e54f74896a75147b66e509c1281804047d41f08b
prerequisite-patch-id: 3f77e2179901ae664a2d2e58965a9725ad13189e
prerequisite-patch-id: e964a9576f9c3970bb7a34b4d2891ffb92939d0e
prerequisite-patch-id: bba35ecdf783653e0fa46015bc289706508fac55

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



^ permalink raw reply

* [PATCH v15 6/7] rust: samples: add a module parameter to the rust_minimal sample
From: Andreas Hindborg @ 2025-07-07 13:29 UTC (permalink / raw)
  To: Miguel Ojeda, Alex Gaynor, Boqun Feng, Gary Guo,
	Björn Roy Baron, Alice Ryhl, Masahiro Yamada,
	Nathan Chancellor, Luis Chamberlain, Danilo Krummrich,
	Benno Lossin, Benno Lossin, Nicolas Schier
  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
In-Reply-To: <20250707-module-params-v3-v15-0-c1f4269a57b9@kernel.org>

Showcase the rust module parameter support by adding a module parameter to
the `rust_minimal` sample.

Reviewed-by: Benno Lossin <lossin@kernel.org>
Signed-off-by: Andreas Hindborg <a.hindborg@kernel.org>
---
 samples/rust/rust_minimal.rs | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/samples/rust/rust_minimal.rs b/samples/rust/rust_minimal.rs
index 1fc7a1be6b6d7..8eb9583571d72 100644
--- a/samples/rust/rust_minimal.rs
+++ b/samples/rust/rust_minimal.rs
@@ -10,6 +10,12 @@
     authors: ["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.value()
+        );
 
         let mut numbers = KVec::new();
         numbers.push(72, GFP_KERNEL)?;

-- 
2.47.2



^ permalink raw reply related

* Re: [PATCH 1/8] execmem: drop unused execmem_update_copy()
From: Christophe Leroy @ 2025-07-07 13:02 UTC (permalink / raw)
  To: Mike Rapoport
  Cc: Andrew Morton, Andy Lutomirski, Borislav Petkov, Daniel Gomez,
	Dave Hansen, Ingo Molnar, Luis Chamberlain, Mark Rutland,
	Masami Hiramatsu, H. Peter Anvin, Peter Zijlstra, Petr Pavlu,
	Sami Tolvanen, Steven Rostedt, Thomas Gleixner, linux-kernel,
	linux-mm, linux-modules, linux-trace-kernel, x86
In-Reply-To: <aGu0Yj08EZvpL5Xv@kernel.org>



Le 07/07/2025 à 13:49, Mike Rapoport a écrit :
> On Mon, Jul 07, 2025 at 12:10:43PM +0200, Christophe Leroy wrote:
>>
>> Le 04/07/2025 à 15:49, Mike Rapoport a écrit :
>>> From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
>>>
>>> The execmem_update_copy() that used text poking was required when memory
>>> allocated from ROX cache was always read-only. Since now its permissions
>>> can be switched to read-write there is no need in a function that updates
>>> memory with text poking.
>>
>> Erm. Looks like I missed the patch that introduced this change.
>>
>> On some variant of powerpc, namely book3s/32, this is not feasible.
> 
> The only user of EXECMEM_ROX_CACHE for now is x86-64, we can always revisit
> when powerpc book3s/32 would want to opt in to cache usage.
> 
> And it seems that [MODULES_VADDR, MODULES_END] is already mapped with
> "large pages", isn't it?

I don't think so. It uses execmem_alloc() which sets VM_ALLOW_HUGE_VMAP 
only when using EXECMEM_ROX_CACHE. And book3s/32 doesn't have large pages.

Only 8xx has large pages but they are not PMD aligned (PMD_SIZE is 4M 
while large pages are 512k and 8M) so it wouldn't work well with 
existing execmem_vmalloc().

> 
>> The granularity for setting the NX (non exec) bit is 256 Mbytes sections.
>> So the area dedicated to execmem [MODULES_VADDR; MODULES_END[ always have
>> the NX bit unset.
>>
>> You can change any page within this area from ROX to RWX but you can't make
>> it RW without X. If you want RW without X you must map it in the VMALLOC
>> area, as VMALLOC area have NX bit always set.
> 
> So what will happen when one callse
> 
> 	set_memory_nx()
> 	set_memory_rw()
> 
> in such areas?

Nothing will happen. It will successfully unset the X bit on the PTE but 
that will be ignored by the HW which only relies on the segment's NX bit 
which is set for the entire VMALLOC area and unset for the entire MODULE 
area.

That's one of the reasons why it has 
CONFIG_ARCH_WANTS_MODULES_DATA_IN_VMALLOC , to make sure text is 
allocated in exec area and data in no-exec area.

Christophe

^ permalink raw reply

* Re: [PATCH 1/8] execmem: drop unused execmem_update_copy()
From: Mike Rapoport @ 2025-07-07 11:49 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: Andrew Morton, Andy Lutomirski, Borislav Petkov, Daniel Gomez,
	Dave Hansen, Ingo Molnar, Luis Chamberlain, Mark Rutland,
	Masami Hiramatsu, H. Peter Anvin, Peter Zijlstra, Petr Pavlu,
	Sami Tolvanen, Steven Rostedt, Thomas Gleixner, linux-kernel,
	linux-mm, linux-modules, linux-trace-kernel, x86
In-Reply-To: <7e52f721-1d8e-4c50-af33-bee3f0d2ac6e@csgroup.eu>

On Mon, Jul 07, 2025 at 12:10:43PM +0200, Christophe Leroy wrote:
> 
> Le 04/07/2025 à 15:49, Mike Rapoport a écrit :
> > From: "Mike Rapoport (Microsoft)" <rppt@kernel.org>
> > 
> > The execmem_update_copy() that used text poking was required when memory
> > allocated from ROX cache was always read-only. Since now its permissions
> > can be switched to read-write there is no need in a function that updates
> > memory with text poking.
> 
> Erm. Looks like I missed the patch that introduced this change.
> 
> On some variant of powerpc, namely book3s/32, this is not feasible.

The only user of EXECMEM_ROX_CACHE for now is x86-64, we can always revisit
when powerpc book3s/32 would want to opt in to cache usage.

And it seems that [MODULES_VADDR, MODULES_END] is already mapped with
"large pages", isn't it?

> The granularity for setting the NX (non exec) bit is 256 Mbytes sections.
> So the area dedicated to execmem [MODULES_VADDR; MODULES_END[ always have
> the NX bit unset.
> 
> You can change any page within this area from ROX to RWX but you can't make
> it RW without X. If you want RW without X you must map it in the VMALLOC
> area, as VMALLOC area have NX bit always set.

So what will happen when one callse

	set_memory_nx()
	set_memory_rw()

in such areas?

> Christophe

-- 
Sincerely yours,
Mike.

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox