diff mbox series

[v7,2/2] arch/riscv: Enable kprobes when CONFIG_MODULES=n

Message ID 20240326134616.7691-2-jarkko@kernel.org (mailing list archive)
State New, archived
Headers show
Series [v7,1/2] kprobes: Implement trampoline memory allocator for tracing | expand

Commit Message

Jarkko Sakkinen March 26, 2024, 1:46 p.m. UTC
Tacing with kprobes while running a monolithic kernel is currently
impossible due the kernel module allocator dependency.

Address the issue by implementing textmem API for RISC-V.

Link: https://www.sochub.fi # for power on testing new SoC's with a minimal stack
Link: https://lore.kernel.org/all/20220608000014.3054333-1-jarkko@profian.com/ # continuation
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v5-v7:
- No changes.
v4:
- Include linux/execmem.h.
v3:
- Architecture independent parts have been split to separate patches.
- Do not change arch/riscv/kernel/module.c as it is out of scope for
  this patch set now.
v2:
- Better late than never right? :-)
- Focus only to RISC-V for now to make the patch more digestable. This
  is the arch where I use the patch on a daily basis to help with QA.
- Introduce HAVE_KPROBES_ALLOC flag to help with more gradual migration.
---
 arch/riscv/Kconfig          |  1 +
 arch/riscv/kernel/Makefile  |  3 +++
 arch/riscv/kernel/execmem.c | 22 ++++++++++++++++++++++
 3 files changed, 26 insertions(+)
 create mode 100644 arch/riscv/kernel/execmem.c

Comments

Conor Dooley March 26, 2024, 6:42 p.m. UTC | #1
On Tue, Mar 26, 2024 at 03:46:16PM +0200, Jarkko Sakkinen wrote:
> Tacing with kprobes while running a monolithic kernel is currently
> impossible due the kernel module allocator dependency.
> 
> Address the issue by implementing textmem API for RISC-V.

This doesn't compile for nommu:
  /build/tmp.3xucsBhqDV/arch/riscv/kernel/execmem.c:10:46: error: 'MODULES_VADDR' undeclared (first use in this function)
  /build/tmp.3xucsBhqDV/arch/riscv/kernel/execmem.c:11:37: error: 'MODULES_END' undeclared (first use in this function)
  /build/tmp.3xucsBhqDV/arch/riscv/kernel/execmem.c:14:1: error: control reaches end of non-void function [-Werror=return-type]
Clang builds also report:
../arch/riscv/kernel/execmem.c:8:56: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]

> 
> Link: https://www.sochub.fi # for power on testing new SoC's with a minimal stack
> Link: https://lore.kernel.org/all/20220608000014.3054333-1-jarkko@profian.com/ # continuation
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> v5-v7:
> - No changes.
> v4:
> - Include linux/execmem.h.
> v3:
> - Architecture independent parts have been split to separate patches.
> - Do not change arch/riscv/kernel/module.c as it is out of scope for
>   this patch set now.

Meta comment. I dunno when v1 was sent, but versions can you please
relax with submitting new versions of your patches? There's conversations
ongoing on v5 at the moment, while this is a more recent version. v2
seems to have been sent on the 23rd and there's been 5 versions in the
last day:
https://patchwork.kernel.org/project/linux-riscv/list/?submitter=195059&state=*

Could you please also try and use a cover letter for patchsets, ideally
with a consistent subject? Otherwise I have to manually mark stuff as
superseded.

Thanks,
Conor.

> v2:
> - Better late than never right? :-)
> - Focus only to RISC-V for now to make the patch more digestable. This
>   is the arch where I use the patch on a daily basis to help with QA.
> - Introduce HAVE_KPROBES_ALLOC flag to help with more gradual migration.
Jarkko Sakkinen March 27, 2024, 5 p.m. UTC | #2
On Tue Mar 26, 2024 at 8:42 PM EET, Conor Dooley wrote:
> On Tue, Mar 26, 2024 at 03:46:16PM +0200, Jarkko Sakkinen wrote:
> > Tacing with kprobes while running a monolithic kernel is currently
> > impossible due the kernel module allocator dependency.
> > 
> > Address the issue by implementing textmem API for RISC-V.
>
> This doesn't compile for nommu:
>   /build/tmp.3xucsBhqDV/arch/riscv/kernel/execmem.c:10:46: error: 'MODULES_VADDR' undeclared (first use in this function)
>   /build/tmp.3xucsBhqDV/arch/riscv/kernel/execmem.c:11:37: error: 'MODULES_END' undeclared (first use in this function)
>   /build/tmp.3xucsBhqDV/arch/riscv/kernel/execmem.c:14:1: error: control reaches end of non-void function [-Werror=return-type]
> Clang builds also report:
> ../arch/riscv/kernel/execmem.c:8:56: warning: omitting the parameter name in a function definition is a C2x extension [-Wc2x-extensions]
>
> > 
> > Link: https://www.sochub.fi # for power on testing new SoC's with a minimal stack
> > Link: https://lore.kernel.org/all/20220608000014.3054333-1-jarkko@profian.com/ # continuation
> > Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> > ---
> > v5-v7:
> > - No changes.
> > v4:
> > - Include linux/execmem.h.
> > v3:
> > - Architecture independent parts have been split to separate patches.
> > - Do not change arch/riscv/kernel/module.c as it is out of scope for
> >   this patch set now.
>
> Meta comment. I dunno when v1 was sent, but versions can you please
> relax with submitting new versions of your patches? There's conversations
> ongoing on v5 at the moment, while this is a more recent version. v2
> seems to have been sent on the 23rd and there's been 5 versions in the
> last day:
> https://patchwork.kernel.org/project/linux-riscv/list/?submitter=195059&state=*
>
> Could you please also try and use a cover letter for patchsets, ideally
> with a consistent subject? Otherwise I have to manually mark stuff as
> superseded.

Point taken but the work has been taken over by Mark and relevant
changes are now sucked into that patch set.

> Thanks,
> Conor.

BR, Jarkko
Christophe Leroy March 28, 2024, 12:50 p.m. UTC | #3
Le 26/03/2024 à 14:46, Jarkko Sakkinen a écrit :
> Tacing with kprobes while running a monolithic kernel is currently
> impossible due the kernel module allocator dependency.
> 
> Address the issue by implementing textmem API for RISC-V.
> 
> Link: https://www.sochub.fi # for power on testing new SoC's with a minimal stack
> Link: https://lore.kernel.org/all/20220608000014.3054333-1-jarkko@profian.com/ # continuation
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> v5-v7:
> - No changes.
> v4:
> - Include linux/execmem.h.
> v3:
> - Architecture independent parts have been split to separate patches.
> - Do not change arch/riscv/kernel/module.c as it is out of scope for
>    this patch set now.
> v2:
> - Better late than never right? :-)
> - Focus only to RISC-V for now to make the patch more digestable. This
>    is the arch where I use the patch on a daily basis to help with QA.
> - Introduce HAVE_KPROBES_ALLOC flag to help with more gradual migration.
> ---
>   arch/riscv/Kconfig          |  1 +
>   arch/riscv/kernel/Makefile  |  3 +++
>   arch/riscv/kernel/execmem.c | 22 ++++++++++++++++++++++
>   3 files changed, 26 insertions(+)
>   create mode 100644 arch/riscv/kernel/execmem.c
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index e3142ce531a0..499512fb17ff 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -132,6 +132,7 @@ config RISCV
>   	select HAVE_KPROBES if !XIP_KERNEL
>   	select HAVE_KPROBES_ON_FTRACE if !XIP_KERNEL
>   	select HAVE_KRETPROBES if !XIP_KERNEL
> +	select HAVE_ALLOC_EXECMEM if !XIP_KERNEL
>   	# https://github.com/ClangBuiltLinux/linux/issues/1881
>   	select HAVE_LD_DEAD_CODE_DATA_ELIMINATION if !LD_IS_LLD
>   	select HAVE_MOVE_PMD
> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index 604d6bf7e476..337797f10d3e 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -73,6 +73,9 @@ obj-$(CONFIG_SMP)		+= cpu_ops.o
>   
>   obj-$(CONFIG_RISCV_BOOT_SPINWAIT) += cpu_ops_spinwait.o
>   obj-$(CONFIG_MODULES)		+= module.o
> +ifeq ($(CONFIG_ALLOC_EXECMEM),y)
> +obj-y				+= execmem.o

Why not just :

obj-$(CONFIG_ALLOC_EXECMEM)		+= execmem.o

> +endif
>   obj-$(CONFIG_MODULE_SECTIONS)	+= module-sections.o
>   
>   obj-$(CONFIG_CPU_PM)		+= suspend_entry.o suspend.o
> diff --git a/arch/riscv/kernel/execmem.c b/arch/riscv/kernel/execmem.c
> new file mode 100644
> index 000000000000..3e52522ead32
> --- /dev/null
> +++ b/arch/riscv/kernel/execmem.c
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +
> +#include <linux/mm.h>
> +#include <linux/execmem.h>
> +#include <linux/vmalloc.h>
> +#include <asm/sections.h>
> +
> +void *alloc_execmem(unsigned long size, gfp_t /* gfp */)
> +{
> +	return __vmalloc_node_range(size, 1, MODULES_VADDR,
> +				    MODULES_END, GFP_KERNEL,

Why not use gfp argument ?

> +				    PAGE_KERNEL, 0, NUMA_NO_NODE,
> +				    __builtin_return_address(0));
> +}
> +
> +void free_execmem(void *region)
> +{
> +	if (in_interrupt())
> +		pr_warn("In interrupt context: vmalloc may not work.\n");

Do you expect that to happen ? module_memfree() has a WARN_ON() meaning 
this should never happen and if it really does it is not just a poor 
dmesg warning.

> +
> +	vfree(region);
> +}
diff mbox series

Patch

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index e3142ce531a0..499512fb17ff 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -132,6 +132,7 @@  config RISCV
 	select HAVE_KPROBES if !XIP_KERNEL
 	select HAVE_KPROBES_ON_FTRACE if !XIP_KERNEL
 	select HAVE_KRETPROBES if !XIP_KERNEL
+	select HAVE_ALLOC_EXECMEM if !XIP_KERNEL
 	# https://github.com/ClangBuiltLinux/linux/issues/1881
 	select HAVE_LD_DEAD_CODE_DATA_ELIMINATION if !LD_IS_LLD
 	select HAVE_MOVE_PMD
diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index 604d6bf7e476..337797f10d3e 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -73,6 +73,9 @@  obj-$(CONFIG_SMP)		+= cpu_ops.o
 
 obj-$(CONFIG_RISCV_BOOT_SPINWAIT) += cpu_ops_spinwait.o
 obj-$(CONFIG_MODULES)		+= module.o
+ifeq ($(CONFIG_ALLOC_EXECMEM),y)
+obj-y				+= execmem.o
+endif
 obj-$(CONFIG_MODULE_SECTIONS)	+= module-sections.o
 
 obj-$(CONFIG_CPU_PM)		+= suspend_entry.o suspend.o
diff --git a/arch/riscv/kernel/execmem.c b/arch/riscv/kernel/execmem.c
new file mode 100644
index 000000000000..3e52522ead32
--- /dev/null
+++ b/arch/riscv/kernel/execmem.c
@@ -0,0 +1,22 @@ 
+// SPDX-License-Identifier: GPL-2.0-or-later
+
+#include <linux/mm.h>
+#include <linux/execmem.h>
+#include <linux/vmalloc.h>
+#include <asm/sections.h>
+
+void *alloc_execmem(unsigned long size, gfp_t /* gfp */)
+{
+	return __vmalloc_node_range(size, 1, MODULES_VADDR,
+				    MODULES_END, GFP_KERNEL,
+				    PAGE_KERNEL, 0, NUMA_NO_NODE,
+				    __builtin_return_address(0));
+}
+
+void free_execmem(void *region)
+{
+	if (in_interrupt())
+		pr_warn("In interrupt context: vmalloc may not work.\n");
+
+	vfree(region);
+}