Skip to contentSkip to conversation

riscv: Add GENERIC_ENTRY, IRQ_STACKS support

PATCH In review [email protected] 3 participants 17 messages Sep 4, 2022 – Sep 6, 2022

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

v2 cover riscv: Add GENERIC_ENTRY, IRQ_STACKS support

From: Guo Ren <[email protected]>

The patches convert riscv to use the generic entry infrastructure from kernel/entry/*. Add independent irq stacks (IRQ_STACKS) for percpu to prevent kernel stack overflows. Add the HAVE_SOFTIRQ_ON_OWN_STACK feature for the IRQ_STACKS config. You can try it directly with [1].

[1] https://github.com/guoren83/linux/tree/generic_entry_v2

Changes in V2:
- Fixup compile error by include "riscv: ptrace: Remove duplicate operation"
https://lore.kernel.org/linux-riscv/[email protected]/T/#u
- Fixup compile warning

- Add test repo link in cover letter

Guo Ren (6):
riscv: ptrace: Remove duplicate operation
riscv: convert to generic entry
riscv: Support HAVE_IRQ_EXIT_ON_IRQ_STACK
riscv: Support HAVE_SOFTIRQ_ON_OWN_STACK
riscv: elf_kexec: Fixup compile warning
riscv: compat_syscall_table: Fixup compile warning

 arch/riscv/Kconfig                    |  10 +
 arch/riscv/include/asm/csr.h          |   1 -
 arch/riscv/include/asm/entry-common.h |   8 +
 arch/riscv/include/asm/irq.h          |   3 +
 arch/riscv/include/asm/ptrace.h       |  10 +-
 arch/riscv/include/asm/stacktrace.h   |   5 +
 arch/riscv/include/asm/syscall.h      |   6 +
 arch/riscv/include/asm/thread_info.h  |  15 +-
 arch/riscv/include/asm/vmap_stack.h   |  28 +++
 arch/riscv/kernel/Makefile            |   1 +
 arch/riscv/kernel/elf_kexec.c         |   4 +
 arch/riscv/kernel/entry.S             | 255 +++++---------------------
 arch/riscv/kernel/irq.c               |  75 ++++++++
 arch/riscv/kernel/ptrace.c            |  41 -----
 arch/riscv/kernel/signal.c            |  21 +--
 arch/riscv/kernel/sys_riscv.c         |  26 +++
 arch/riscv/kernel/traps.c             |  11 ++
 arch/riscv/mm/fault.c                 |  12 +-
 18 files changed, 250 insertions(+), 282 deletions(-)
 create mode 100644 arch/riscv/include/asm/entry-common.h
 create mode 100644 arch/riscv/include/asm/vmap_stack.h
Signature · 1 line
2.36.1
#1 lore raw

PATCH v2 1/6 riscv: ptrace: Remove duplicate operation

From: Guo Ren <[email protected]>

The TIF_SYSCALL_TRACE is controlled by a common code, see kernel/ptrace.c and include/linux/thread.h.

clear_task_syscall_work(child, SYSCALL_TRACE);

---
 arch/riscv/kernel/ptrace.c | 1 -
 1 file changed, 1 deletion(-)
arch/riscv/kernel/ptrace.c +0 -1
index 2ae8280ae475..44f4b1ca315d 100644
@@ -212,7 +212,6 @@ unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs, unsigned int n)
 
 void ptrace_disable(struct task_struct *child)
 {
-	clear_tsk_thread_flag(child, TIF_SYSCALL_TRACE);
 }
 
 long arch_ptrace(struct task_struct *child, long request,
Signature · 1 line
2.36.1
#2 lore raw

PATCH v2 2/6 riscv: convert to generic entry

In reply to [email protected]

From: Guo Ren <[email protected]>

This patch converts riscv to use the generic entry infrastructure from kernel/entry/*. The generic entry makes maintainers' work easier and codes more elegant. Here are the changes than before:

- More clear entry.S with handle_exception and ret_from_exception
- Get rid of complex custom signal implementation
- More readable syscall procedure
- Little modification on ret_from_fork & ret_from_kernel_thread
- Wrap with irqentry_enter/exit and syscall_enter/exit_from_user_mode
- Use the standard preemption code instead of custom

---
 arch/riscv/Kconfig                    |   1 +
 arch/riscv/include/asm/csr.h          |   1 -
 arch/riscv/include/asm/entry-common.h |   8 +
 arch/riscv/include/asm/ptrace.h       |  10 +-
 arch/riscv/include/asm/stacktrace.h   |   5 +
 arch/riscv/include/asm/syscall.h      |   6 +
 arch/riscv/include/asm/thread_info.h  |  13 +-
 arch/riscv/kernel/entry.S             | 228 +++-----------------------
 arch/riscv/kernel/irq.c               |  15 ++
 arch/riscv/kernel/ptrace.c            |  40 -----
 arch/riscv/kernel/signal.c            |  21 +--
 arch/riscv/kernel/sys_riscv.c         |  26 +++
 arch/riscv/kernel/traps.c             |  11 ++
 arch/riscv/mm/fault.c                 |  12 +-
 14 files changed, 116 insertions(+), 281 deletions(-)
 create mode 100644 arch/riscv/include/asm/entry-common.h
arch/riscv/Kconfig +1 -0
index ed66c31e4655..a07bb3b73b5b 100644
@@ -56,6 +56,7 @@ config RISCV
 	select GENERIC_ATOMIC64 if !64BIT
 	select GENERIC_CLOCKEVENTS_BROADCAST if SMP
 	select GENERIC_EARLY_IOREMAP
+	select GENERIC_ENTRY
 	select GENERIC_GETTIMEOFDAY if HAVE_GENERIC_VDSO
 	select GENERIC_IDLE_POLL_SETUP
 	select GENERIC_IOREMAP if MMU
arch/riscv/include/asm/csr.h +0 -1
index 0e571f6483d9..7c2b8cdb7b77 100644
@@ -40,7 +40,6 @@
 #define SR_UXL		_AC(0x300000000, UL) /* XLEN mask for U-mode */
 #define SR_UXL_32	_AC(0x100000000, UL) /* XLEN = 32 for U-mode */
 #define SR_UXL_64	_AC(0x200000000, UL) /* XLEN = 64 for U-mode */
-#define SR_UXL_SHIFT	32
 #endif
 
 /* SATP flags */
arch/riscv/include/asm/entry-common.h new file +8 -0
new file mode 100644
index 000000000000..1636ac2af28e
@@ -0,0 +1,8 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+
+#ifndef _ASM_RISCV_ENTRY_COMMON_H
+#define _ASM_RISCV_ENTRY_COMMON_H
+
+#include <asm/stacktrace.h>
+
+#endif /* _ASM_RISCV_ENTRY_COMMON_H */
arch/riscv/include/asm/ptrace.h +8 -2
index 6ecd461129d2..4e46a611f255 100644
@@ -53,6 +53,9 @@ struct pt_regs {
 	unsigned long orig_a0;
 };
 
+#define PTRACE_SYSEMU			0x1f
+#define PTRACE_SYSEMU_SINGLESTEP	0x20
+
 #ifdef CONFIG_64BIT
 #define REG_FMT "%016lx"
 #else
@@ -121,8 +124,6 @@ extern unsigned long regs_get_kernel_stack_nth(struct pt_regs *regs,
 
 void prepare_ftrace_return(unsigned long *parent, unsigned long self_addr,
 			   unsigned long frame_pointer);
-int do_syscall_trace_enter(struct pt_regs *regs);
-void do_syscall_trace_exit(struct pt_regs *regs);
 
 /**
  * regs_get_register() - get register value from its offset
@@ -172,6 +173,11 @@ static inline unsigned long regs_get_kernel_argument(struct pt_regs *regs,
 	return 0;
 }
 
+static inline int regs_irqs_disabled(struct pt_regs *regs)
+{
+	return !(regs->status & SR_IE);
+}
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_RISCV_PTRACE_H */
arch/riscv/include/asm/stacktrace.h +5 -0
index 3450c1912afd..f7e8ef2418b9 100644
@@ -16,4 +16,9 @@ extern void notrace walk_stackframe(struct task_struct *task, struct pt_regs *re
 extern void dump_backtrace(struct pt_regs *regs, struct task_struct *task,
 			   const char *loglvl);
 
+static inline bool on_thread_stack(void)
+{
+	return !(((unsigned long)(current->stack) ^ current_stack_pointer) & ~(THREAD_SIZE - 1));
+}
+
 #endif /* _ASM_RISCV_STACKTRACE_H */
arch/riscv/include/asm/syscall.h +6 -0
index 384a63b86420..6c573f18030b 100644
@@ -74,5 +74,11 @@ static inline int syscall_get_arch(struct task_struct *task)
 #endif
 }
 
+static inline bool arch_syscall_is_vdso_sigreturn(struct pt_regs *regs)
+{
+	return false;
+}
+
 asmlinkage long sys_riscv_flush_icache(uintptr_t, uintptr_t, uintptr_t);
+asmlinkage void do_sys_ecall_u(struct pt_regs *regs);
 #endif	/* _ASM_RISCV_SYSCALL_H */
arch/riscv/include/asm/thread_info.h +1 -12
index 67322f878e0d..7de4fb96f0b5 100644
@@ -66,6 +66,7 @@ struct thread_info {
 	long			kernel_sp;	/* Kernel stack pointer */
 	long			user_sp;	/* User stack pointer */
 	int			cpu;
+	unsigned long		syscall_work;	/* SYSCALL_WORK_ flags */
 };
 
 /*
@@ -88,26 +89,18 @@ struct thread_info {
  * - pending work-to-be-done flags are in lowest half-word
  * - other flags in upper half-word(s)
  */
-#define TIF_SYSCALL_TRACE	0	/* syscall trace active */
 #define TIF_NOTIFY_RESUME	1	/* callback before returning to user */
 #define TIF_SIGPENDING		2	/* signal pending */
 #define TIF_NEED_RESCHED	3	/* rescheduling necessary */
 #define TIF_RESTORE_SIGMASK	4	/* restore signal mask in do_signal() */
 #define TIF_MEMDIE		5	/* is terminating due to OOM killer */
-#define TIF_SYSCALL_TRACEPOINT  6       /* syscall tracepoint instrumentation */
-#define TIF_SYSCALL_AUDIT	7	/* syscall auditing */
-#define TIF_SECCOMP		8	/* syscall secure computing */
 #define TIF_NOTIFY_SIGNAL	9	/* signal notifications exist */
 #define TIF_UPROBE		10	/* uprobe breakpoint or singlestep */
 #define TIF_32BIT		11	/* compat-mode 32bit process */
 
-#define _TIF_SYSCALL_TRACE	(1 << TIF_SYSCALL_TRACE)
 #define _TIF_NOTIFY_RESUME	(1 << TIF_NOTIFY_RESUME)
 #define _TIF_SIGPENDING		(1 << TIF_SIGPENDING)
 #define _TIF_NEED_RESCHED	(1 << TIF_NEED_RESCHED)
-#define _TIF_SYSCALL_TRACEPOINT	(1 << TIF_SYSCALL_TRACEPOINT)
-#define _TIF_SYSCALL_AUDIT	(1 << TIF_SYSCALL_AUDIT)
-#define _TIF_SECCOMP		(1 << TIF_SECCOMP)
 #define _TIF_NOTIFY_SIGNAL	(1 << TIF_NOTIFY_SIGNAL)
 #define _TIF_UPROBE		(1 << TIF_UPROBE)
 
@@ -115,8 +108,4 @@ struct thread_info {
 	(_TIF_NOTIFY_RESUME | _TIF_SIGPENDING | _TIF_NEED_RESCHED | \
 	 _TIF_NOTIFY_SIGNAL | _TIF_UPROBE)
 
-#define _TIF_SYSCALL_WORK \
-	(_TIF_SYSCALL_TRACE | _TIF_SYSCALL_TRACEPOINT | _TIF_SYSCALL_AUDIT | \
-	 _TIF_SECCOMP)
-
 #endif /* _ASM_RISCV_THREAD_INFO_H */
arch/riscv/kernel/entry.S +22 -206

312 lines not on this page · raw message

arch/riscv/kernel/irq.c +15 -0
index 7207fa08d78f..24c2e1bd756a 100644
@@ -5,6 +5,7 @@
  * Copyright (C) 2018 Christoph Hellwig
  */
 
+#include <linux/entry-common.h>
 #include <linux/interrupt.h>
 #include <linux/irqchip.h>
 #include <linux/seq_file.h>
@@ -22,3 +23,17 @@ void __init init_IRQ(void)
 	if (!handle_arch_irq)
 		panic("No interrupt controller found.");
 }
+
+asmlinkage void noinstr do_riscv_irq(struct pt_regs *regs)
+{
+	struct pt_regs *old_regs;
+	irqentry_state_t state = irqentry_enter(regs);
+
+	irq_enter_rcu();
+	old_regs = set_irq_regs(regs);
+	handle_arch_irq(regs);
+	set_irq_regs(old_regs);
+	irq_exit_rcu();
+
+	irqentry_exit(regs, state);
+}
arch/riscv/kernel/ptrace.c +0 -40
index 44f4b1ca315d..4caed6c683e4 100644
@@ -228,46 +228,6 @@ long arch_ptrace(struct task_struct *child, long request,
 	return ret;
 }
 
-/*
- * Allows PTRACE_SYSCALL to work.  These are called from entry.S in
- * {handle,ret_from}_syscall.
- */
-__visible int do_syscall_trace_enter(struct pt_regs *regs)
-{
-	if (test_thread_flag(TIF_SYSCALL_TRACE))
-		if (ptrace_report_syscall_entry(regs))
-			return -1;
-
-	/*
-	 * Do the secure computing after ptrace; failures should be fast.
-	 * If this fails we might have return value in a0 from seccomp
-	 * (via SECCOMP_RET_ERRNO/TRACE).
-	 */
-	if (secure_computing() == -1)
-		return -1;
-
-#ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS
-	if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
-		trace_sys_enter(regs, syscall_get_nr(current, regs));
-#endif
-
-	audit_syscall_entry(regs->a7, regs->a0, regs->a1, regs->a2, regs->a3);
-	return 0;
-}
-
-__visible void do_syscall_trace_exit(struct pt_regs *regs)
-{
-	audit_syscall_exit(regs);
-
-	if (test_thread_flag(TIF_SYSCALL_TRACE))
-		ptrace_report_syscall_exit(regs, 0);
-
-#ifdef CONFIG_HAVE_SYSCALL_TRACEPOINTS
-	if (test_thread_flag(TIF_SYSCALL_TRACEPOINT))
-		trace_sys_exit(regs, regs_return_value(regs));
-#endif
-}
-
 #ifdef CONFIG_COMPAT
 static int compat_riscv_gpr_get(struct task_struct *target,
 				const struct user_regset *regset,
arch/riscv/kernel/signal.c +2 -19
index 5a2de6b6f882..5871eccbbd94 100644
@@ -12,6 +12,7 @@
 #include <linux/syscalls.h>
 #include <linux/resume_user_mode.h>
 #include <linux/linkage.h>
+#include <linux/entry-common.h>
 
 #include <asm/ucontext.h>
 #include <asm/vdso.h>
@@ -272,7 +273,7 @@ static void handle_signal(struct ksignal *ksig, struct pt_regs *regs)
 	signal_setup_done(ret, ksig, 0);
 }
 
-static void do_signal(struct pt_regs *regs)
+void arch_do_signal_or_restart(struct pt_regs *regs)
 {
 	struct ksignal ksig;
 
@@ -309,21 +310,3 @@ static void do_signal(struct pt_regs *regs)
 	 */
 	restore_saved_sigmask();
 }
-
-/*
- * notification of userspace execution resumption
- * - triggered by the _TIF_WORK_MASK flags
- */
-asmlinkage __visible void do_notify_resume(struct pt_regs *regs,
-					   unsigned long thread_info_flags)
-{
-	if (thread_info_flags & _TIF_UPROBE)
-		uprobe_notify_resume(regs);
-
-	/* Handle pending signal delivery */
-	if (thread_info_flags & (_TIF_SIGPENDING | _TIF_NOTIFY_SIGNAL))
-		do_signal(regs);
-
-	if (thread_info_flags & _TIF_NOTIFY_RESUME)
-		resume_user_mode_work(regs);
-}
arch/riscv/kernel/sys_riscv.c +26 -0
index 571556bb9261..1194a1b960ef 100644
@@ -5,8 +5,10 @@
  * Copyright (C) 2017 SiFive
  */
 
+#include <linux/entry-common.h>
 #include <linux/syscalls.h>
 #include <asm/unistd.h>
+#include <asm/syscall.h>
 #include <asm/cacheflush.h>
 #include <asm-generic/mman-common.h>
 
@@ -72,3 +74,27 @@ SYSCALL_DEFINE3(riscv_flush_icache, uintptr_t, start, uintptr_t, end,
 
 	return 0;
 }
+
+typedef long (*syscall_t)(ulong, ulong, ulong, ulong, ulong, ulong, ulong);
+
+asmlinkage void do_sys_ecall_u(struct pt_regs *regs)
+{
+	syscall_t syscall;
+	ulong sr_uxl = regs->status & SR_UXL;
+	ulong nr = regs->a7;
+
+	regs->epc += 4;
+	regs->orig_a0 = regs->a0;
+	regs->a0 = -ENOSYS;
+
+	nr = syscall_enter_from_user_mode(regs, nr);
+	if (sr_uxl == SR_UXL_32)
+		syscall = compat_sys_call_table[nr];
+	else
+		syscall = sys_call_table[nr];
+
+	if (nr < NR_syscalls)
+		regs->a0 = syscall(regs->orig_a0, regs->a1, regs->a2,
+				   regs->a3, regs->a4, regs->a5, regs->a6);
+	syscall_exit_to_user_mode(regs);
+}
arch/riscv/kernel/traps.c +11 -0
index 635e6ec26938..751745de10c5 100644
@@ -17,6 +17,7 @@
 #include <linux/module.h>
 #include <linux/irq.h>
 #include <linux/kexec.h>
+#include <linux/entry-common.h>
 
 #include <asm/asm-prototypes.h>
 #include <asm/bug.h>
@@ -99,7 +100,9 @@ static void do_trap_error(struct pt_regs *regs, int signo, int code,
 #define DO_ERROR_INFO(name, signo, code, str)				\
 asmlinkage __visible __trap_section void name(struct pt_regs *regs)	\
 {									\
+	irqentry_state_t state = irqentry_enter(regs);			\
 	do_trap_error(regs, signo, code, regs->epc, "Oops - " str);	\
+	irqentry_exit(regs, state);					\
 }
 
 DO_ERROR_INFO(do_trap_unknown,
@@ -123,18 +126,22 @@ int handle_misaligned_store(struct pt_regs *regs);
 
 asmlinkage void __trap_section do_trap_load_misaligned(struct pt_regs *regs)
 {
+	irqentry_state_t state = irqentry_enter(regs);
 	if (!handle_misaligned_load(regs))
 		return;
 	do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
 		      "Oops - load address misaligned");
+	irqentry_exit(regs, state);
 }
 
 asmlinkage void __trap_section do_trap_store_misaligned(struct pt_regs *regs)
 {
+	irqentry_state_t state = irqentry_enter(regs);
 	if (!handle_misaligned_store(regs))
 		return;
 	do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
 		      "Oops - store (or AMO) address misaligned");
+	irqentry_exit(regs, state);
 }
 #endif
 DO_ERROR_INFO(do_trap_store_fault,
@@ -158,6 +165,8 @@ static inline unsigned long get_break_insn_length(unsigned long pc)
 
 asmlinkage __visible __trap_section void do_trap_break(struct pt_regs *regs)
 {
+	irqentry_state_t state = irqentry_enter(regs);
+
 #ifdef CONFIG_KPROBES
 	if (kprobe_single_step_handler(regs))
 		return;
@@ -185,6 +194,8 @@ asmlinkage __visible __trap_section void do_trap_break(struct pt_regs *regs)
 		regs->epc += get_break_insn_length(regs->epc);
 	else
 		die(regs, "Kernel BUG");
+
+	irqentry_exit(regs, state);
 }
 NOKPROBE_SYMBOL(do_trap_break);
 
arch/riscv/mm/fault.c +11 -1
index f2fbd1400b7c..45cd200114b1 100644
@@ -15,6 +15,7 @@
 #include <linux/uaccess.h>
 #include <linux/kprobes.h>
 #include <linux/kfence.h>
+#include <linux/entry-common.h>
 
 #include <asm/ptrace.h>
 #include <asm/tlbflush.h>
@@ -203,7 +204,7 @@ static inline bool access_error(unsigned long cause, struct vm_area_struct *vma)
  * This routine handles page faults.  It determines the address and the
  * problem, and then passes it off to one of the appropriate routines.
  */
-asmlinkage void do_page_fault(struct pt_regs *regs)
+static void __do_page_fault(struct pt_regs *regs)
 {
 	struct task_struct *tsk;
 	struct vm_area_struct *vma;
@@ -350,4 +351,13 @@ asmlinkage void do_page_fault(struct pt_regs *regs)
 	}
 	return;
 }
+
+asmlinkage void do_page_fault(struct pt_regs *regs)
+{
+	irqentry_state_t state = irqentry_enter(regs);
+
+	__do_page_fault(regs);
+
+	irqentry_exit(regs, state);
+}
 NOKPROBE_SYMBOL(do_page_fault);
Signature · 1 line
2.36.1
#3 lore raw

PATCH v2 3/6 riscv: Support HAVE_IRQ_EXIT_ON_IRQ_STACK

In reply to [email protected]

From: Guo Ren <[email protected]>

Add independent irq stacks for percpu to prevent kernel stack overflows. It is also compatible with VMAP_STACK by implementing arch_alloc_vmap_stack. Many architectures have supported HAVE_IRQ_EXIT_ON_IRQ_STACK, riscv should follow up.

---
 arch/riscv/Kconfig                   |  8 +++++
 arch/riscv/include/asm/irq.h         |  3 ++
 arch/riscv/include/asm/thread_info.h |  2 ++
 arch/riscv/include/asm/vmap_stack.h  | 28 ++++++++++++++++
 arch/riscv/kernel/entry.S            | 27 ++++++++++++++++
 arch/riscv/kernel/irq.c              | 48 ++++++++++++++++++++++++++--
 6 files changed, 114 insertions(+), 2 deletions(-)
 create mode 100644 arch/riscv/include/asm/vmap_stack.h
arch/riscv/Kconfig +8 -0
index a07bb3b73b5b..a8a12b4ba1a9 100644
@@ -433,6 +433,14 @@ config FPU
 
 	  If you don't know what to do here, say Y.
 
+config IRQ_STACKS
+	bool "Independent irq stacks"
+	default y
+	select HAVE_IRQ_EXIT_ON_IRQ_STACK
+	help
+	  Add independent irq stacks for percpu to prevent kernel stack overflows.
+	  We may save some memory footprint by disabling IRQ_STACKS.
+
 endmenu # "Platform type"
 
 menu "Kernel features"
arch/riscv/include/asm/irq.h +3 -0
index e4c435509983..205e1c693dfd 100644
@@ -13,5 +13,8 @@
 #include <asm-generic/irq.h>
 
 extern void __init init_IRQ(void);
+asmlinkage void call_on_stack(struct pt_regs *regs, ulong *sp,
+				     void (*fn)(struct pt_regs *), ulong tmp);
+asmlinkage void noinstr do_riscv_irq(struct pt_regs *regs);
 
 #endif /* _ASM_RISCV_IRQ_H */
arch/riscv/include/asm/thread_info.h +2 -0
index 7de4fb96f0b5..043da8ccc7e6 100644
@@ -40,6 +40,8 @@
 #define OVERFLOW_STACK_SIZE     SZ_4K
 #define SHADOW_OVERFLOW_STACK_SIZE (1024)
 
+#define IRQ_STACK_SIZE		THREAD_SIZE
+
 #ifndef __ASSEMBLY__
 
 extern long shadow_stack[SHADOW_OVERFLOW_STACK_SIZE / sizeof(long)];
arch/riscv/include/asm/vmap_stack.h new file +28 -0
new file mode 100644
index 000000000000..3fbf481abf4f
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+// Copied from arch/arm64/include/asm/vmap_stack.h.
+#ifndef _ASM_RISCV_VMAP_STACK_H
+#define _ASM_RISCV_VMAP_STACK_H
+
+#include <linux/bug.h>
+#include <linux/gfp.h>
+#include <linux/kconfig.h>
+#include <linux/vmalloc.h>
+#include <linux/pgtable.h>
+#include <asm/thread_info.h>
+
+/*
+ * To ensure that VMAP'd stack overflow detection works correctly, all VMAP'd
+ * stacks need to have the same alignment.
+ */
+static inline unsigned long *arch_alloc_vmap_stack(size_t stack_size, int node)
+{
+	void *p;
+
+	BUILD_BUG_ON(!IS_ENABLED(CONFIG_VMAP_STACK));
+
+	p = __vmalloc_node(stack_size, THREAD_ALIGN, THREADINFO_GFP, node,
+			__builtin_return_address(0));
+	return kasan_reset_tag(p);
+}
+
+#endif /* _ASM_RISCV_VMAP_STACK_H */
arch/riscv/kernel/entry.S +27 -0
index 5f49517cd3a2..551ce4c25698 100644
@@ -332,6 +332,33 @@ ENTRY(ret_from_kernel_thread)
 	tail syscall_exit_to_user_mode
 ENDPROC(ret_from_kernel_thread)
 
+#ifdef CONFIG_IRQ_STACKS
+ENTRY(call_on_stack)
+	/* Create a frame record to save our ra and fp */
+	addi	sp, sp, -RISCV_SZPTR
+	REG_S	ra, (sp)
+	addi	sp, sp, -RISCV_SZPTR
+	REG_S	fp, (sp)
+
+	/* Save sp in fp */
+	move	fp, sp
+
+	/* Move to the new stack and call the function there */
+	la	a3, IRQ_STACK_SIZE
+	add	sp, a1, a3
+	jalr	a2
+
+	/*
+	 * Restore sp from prev fp, and fp, ra from the frame
+	 */
+	move	sp, fp
+	REG_L	fp, (sp)
+	addi	sp, sp, RISCV_SZPTR
+	REG_L	ra, (sp)
+	addi	sp, sp, RISCV_SZPTR
+	ret
+ENDPROC(call_on_stack)
+#endif
 
 /*
  * Integer register context switch
arch/riscv/kernel/irq.c +46 -2

73 lines not on this page · raw message

Signature · 7 lines
2.36.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
#4 lore raw

PATCH v2 4/6 riscv: Support HAVE_SOFTIRQ_ON_OWN_STACK

In reply to [email protected]

From: Guo Ren <[email protected]>

Add the HAVE_SOFTIRQ_ON_OWN_STACK feature for the IRQ_STACKS config. The irq and softirq use the same independent irq_stack of percpu by time division multiplexing.

---
 arch/riscv/Kconfig      |  7 ++++---
 arch/riscv/kernel/irq.c | 16 ++++++++++++++++
 2 files changed, 20 insertions(+), 3 deletions(-)
arch/riscv/Kconfig +4 -3
index a8a12b4ba1a9..da548ed7d107 100644
@@ -434,12 +434,13 @@ config FPU
 	  If you don't know what to do here, say Y.
 
 config IRQ_STACKS
-	bool "Independent irq stacks"
+	bool "Independent irq & softirq stacks"
 	default y
 	select HAVE_IRQ_EXIT_ON_IRQ_STACK
+	select HAVE_SOFTIRQ_ON_OWN_STACK
 	help
-	  Add independent irq stacks for percpu to prevent kernel stack overflows.
-	  We may save some memory footprint by disabling IRQ_STACKS.
+	  Add independent irq & softirq stacks for percpu to prevent kernel stack
+	  overflows. We may save some memory footprint by disabling IRQ_STACKS.
 
 endmenu # "Platform type"
 
arch/riscv/kernel/irq.c +16 -0
index 3ac16391ae2e..e2196b22f6f4 100644
@@ -11,6 +11,7 @@
 #include <linux/seq_file.h>
 #include <asm/smp.h>
 #include <asm/vmap_stack.h>
+#include <asm/softirq_stack.h>
 
 #ifdef CONFIG_IRQ_STACKS
 DEFINE_PER_CPU(ulong *, irq_stack_ptr);
@@ -38,6 +39,21 @@ static void init_irq_stacks(void)
 		per_cpu(irq_stack_ptr, cpu) = per_cpu(irq_stack, cpu);
 }
 #endif /* CONFIG_VMAP_STACK */
+
+#ifndef CONFIG_PREEMPT_RT
+static void do_riscv_softirq(struct pt_regs *regs)
+{
+	__do_softirq();
+}
+
+void do_softirq_own_stack(void)
+{
+	ulong *sp = per_cpu(irq_stack_ptr, smp_processor_id());
+
+	call_on_stack(NULL, sp, do_riscv_softirq, 0);
+}
+#endif /* CONFIG_PREEMPT_RT */
+
 #else
 static void init_irq_stacks(void) {}
 #endif /* CONFIG_IRQ_STACKS */
Signature · 7 lines
2.36.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
#5 lore raw

PATCH v2 5/6 riscv: elf_kexec: Fixup compile warning

In reply to [email protected]

From: Guo Ren <[email protected]>

COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash arch/riscv/

../arch/riscv/kernel/elf_kexec.c: In function 'elf_kexec_load':
../arch/riscv/kernel/elf_kexec.c:185:23: warning: variable 'kernel_start' set but not used [-Wunused-but-set-variable]

  185 |         unsigned long kernel_start;
      |                       ^~~~~~~~~~~~
---
 arch/riscv/kernel/elf_kexec.c | 4 ++++
 1 file changed, 4 insertions(+)
arch/riscv/kernel/elf_kexec.c +4 -0
index 0cb94992c15b..bba3723a0914 100644
@@ -182,7 +182,9 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
 	unsigned long new_kernel_pbase = 0UL;
 	unsigned long initrd_pbase = 0UL;
 	unsigned long headers_sz;
+#ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY
 	unsigned long kernel_start;
+#endif /* CONFIG_ARCH_HAS_KEXEC_PURGATORY */
 	void *fdt, *headers;
 	struct elfhdr ehdr;
 	struct kexec_buf kbuf;
@@ -197,7 +199,9 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
 			     &old_kernel_pbase, &new_kernel_pbase);
 	if (ret)
 		goto out;
+#ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY
 	kernel_start = image->start;
+#endif /* CONFIG_ARCH_HAS_KEXEC_PURGATORY */
 	pr_notice("The entry point of kernel at 0x%lx\n", image->start);
 
 	/* Add the kernel binary to the image */
Signature · 7 lines
2.36.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
#6 lore raw

Re: [PATCH V2 5/6] riscv: elf_kexec: Fixup compile warning

Quoted 10 lines · On 04/09/2022 08:26, [email protected] wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

From: Guo Ren <[email protected]>

COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash arch/riscv/

../arch/riscv/kernel/elf_kexec.c: In function 'elf_kexec_load':
../arch/riscv/kernel/elf_kexec.c:185:23: warning: variable 'kernel_start' set but not used [-Wunused-but-set-variable]

Quoted 6 lines
  185 |         unsigned long kernel_start;
      |                       ^~~~~~~~~~~~

Is this then a

Fixes 838b3e28488f ("RISC-V: Load purgatory in kexec_file")

?

Could you also add something like:
"If CONFIG_CRYTPO is not enabled, ...." to explain why this may be unused?

Quoted 19 lines
---
 arch/riscv/kernel/elf_kexec.c | 4 ++++
 1 file changed, 4 insertions(+)
arch/riscv/kernel/elf_kexec.c +2 -0
index 0cb94992c15b..bba3723a0914 100644
@@ -182,7 +182,9 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
        unsigned long new_kernel_pbase = 0UL;
        unsigned long initrd_pbase = 0UL;
        unsigned long headers_sz;
+#ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY
        unsigned long kernel_start;
+#endif /* CONFIG_ARCH_HAS_KEXEC_PURGATORY */
        void *fdt, *headers;
        struct elfhdr ehdr;
        struct kexec_buf kbuf;

arch/riscv/kernel/elf_kexec.c static void *elf_kexec_load(struct kimage *image, char *kernel_buf, · line 204

Quoted 6 lines
arch/riscv/kernel/elf_kexec.c +2 -0
@@ -197,7 +199,9 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
                             &old_kernel_pbase, &new_kernel_pbase);
        if (ret)
                goto out;
+#ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY
        kernel_start = image->start;
+#endif /* CONFIG_ARCH_HAS_KEXEC_PURGATORY */

Instead of adding more #ifdefs to the file, could we instead just drop the kernel_start variable? For the sake of compilation coverage, we could then also do the following (build-tested only):

-- >8 --
From: Conor Dooley <[email protected]>
Date: Sun, 4 Sep 2022 11:27:07 +0100
Subject: [PATCH] riscv: elf_kexec: replace ifdef with IS_ENABLED()

IS_ENABLED() gives better compile time coverage than #ifdef. Replace the ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY in elf_kexec_load() since none of the code it guards uses a symbol that's missing if it is not set.

---
 arch/riscv/kernel/elf_kexec.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)
arch/riscv/kernel/elf_kexec.c +14 -14
index 0cb94992c15b..29cbf655c474 100644
@@ -248,21 +248,21 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
 		cmdline = modified_cmdline;
 	}
 
-#ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY
-	/* Add purgatory to the image */
-	kbuf.top_down = true;
-	kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
-	ret = kexec_load_purgatory(image, &kbuf);
-	if (ret) {
-		pr_err("Error loading purgatory ret=%d\n", ret);
-		goto out;
+	if (IS_ENABLED(CONFIG_ARCH_HAS_KEXEC_PURGATORY)) {
+		/* Add purgatory to the image */
+		kbuf.top_down = true;
+		kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
+		ret = kexec_load_purgatory(image, &kbuf);
+		if (ret) {
+			pr_err("Error loading purgatory ret=%d\n", ret);
+			goto out;
+		}
+		ret = kexec_purgatory_get_set_symbol(image, "riscv_kernel_entry",
+						     &kernel_start,
+						     sizeof(kernel_start), 0);
+		if (ret)
+			pr_err("Error update purgatory ret=%d\n", ret);
 	}
-	ret = kexec_purgatory_get_set_symbol(image, "riscv_kernel_entry",
-					     &kernel_start,
-					     sizeof(kernel_start), 0);
-	if (ret)
-		pr_err("Error update purgatory ret=%d\n", ret);
-#endif /* CONFIG_ARCH_HAS_KEXEC_PURGATORY */
 
 	/* Add the initrd to the image */
 	if (initrd != NULL) {
Signature · 1 line
2.37.1
author
#7 lore raw

Re: [PATCH V2 5/6] riscv: elf_kexec: Fixup compile warning

Quoted 48 lines · On Sun, Sep 4, 2022 at 6:36 PM <[email protected]> wrote:
Quoted 10 lines · On 04/09/2022 08:26, [email protected] wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

From: Guo Ren <[email protected]>

COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash arch/riscv/

../arch/riscv/kernel/elf_kexec.c: In function 'elf_kexec_load':
../arch/riscv/kernel/elf_kexec.c:185:23: warning: variable 'kernel_start' set but not used [-Wunused-but-set-variable]

Quoted 6 lines
  185 |         unsigned long kernel_start;
      |                       ^~~~~~~~~~~~

Is this then a

Fixes 838b3e28488f ("RISC-V: Load purgatory in kexec_file")

?

Could you also add something like:
"If CONFIG_CRYTPO is not enabled, ...." to explain why this may be unused?

Quoted 21 lines
---
 arch/riscv/kernel/elf_kexec.c | 4 ++++
 1 file changed, 4 insertions(+)
arch/riscv/kernel/elf_kexec.c +2 -0
index 0cb94992c15b..bba3723a0914 100644
@@ -182,7 +182,9 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
        unsigned long new_kernel_pbase = 0UL;
        unsigned long initrd_pbase = 0UL;
        unsigned long headers_sz;
+#ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY
        unsigned long kernel_start;
+#endif /* CONFIG_ARCH_HAS_KEXEC_PURGATORY */
        void *fdt, *headers;
        struct elfhdr ehdr;
        struct kexec_buf kbuf;
@@ -197,7 +199,9 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
                             &old_kernel_pbase, &new_kernel_pbase);
        if (ret)
Quoted 8 lines

arch/riscv/kernel/elf_kexec.c static void *elf_kexec_load(struct kimage *image, char *kernel_buf, · line 204

Quoted 4 lines
arch/riscv/kernel/elf_kexec.c +2 -0
                goto out;
+#ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY
        kernel_start = image->start;
+#endif /* CONFIG_ARCH_HAS_KEXEC_PURGATORY */

Instead of adding more #ifdefs to the file, could we instead just drop the kernel_start variable? For the sake of compilation coverage, we could then also do the following (build-tested only):

Em... I prefer:

arch/riscv/kernel/elf_kexec.c +0 -0
index 0cb94992c15b..4b9264340b78 100644
@@ -198,7 +198,7 @@ static void *elf_kexec_load(struct kimage *image,

char *kernel_buf,

        if (ret)
                goto out;
        kernel_start = image->start;

- pr_notice("The entry point of kernel at 0x%lx\n", image->start);
+ pr_notice("The entry point of kernel at 0x%lx\n", kernel_start);

/* Add the kernel binary to the image */ ret = riscv_kexec_elf_load(image, &ehdr, &elf_info,

Quoted 59 lines

-- >8 --
From: Conor Dooley <[email protected]>
Date: Sun, 4 Sep 2022 11:27:07 +0100
Subject: [PATCH] riscv: elf_kexec: replace ifdef with IS_ENABLED()

IS_ENABLED() gives better compile time coverage than #ifdef. Replace the ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY in elf_kexec_load() since none of the code it guards uses a symbol that's missing if it is not set.

---
 arch/riscv/kernel/elf_kexec.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)
arch/riscv/kernel/elf_kexec.c +14 -15
index 0cb94992c15b..29cbf655c474 100644
@@ -248,21 +248,21 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
                cmdline = modified_cmdline;
        }
 
-#ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY
-       /* Add purgatory to the image */
-       kbuf.top_down = true;
-       kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
-       ret = kexec_load_purgatory(image, &kbuf);
-       if (ret) {
-               pr_err("Error loading purgatory ret=%d\n", ret);
-               goto out;
+       if (IS_ENABLED(CONFIG_ARCH_HAS_KEXEC_PURGATORY)) {
+               /* Add purgatory to the image */
+               kbuf.top_down = true;
+               kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
+               ret = kexec_load_purgatory(image, &kbuf);
+               if (ret) {
+                       pr_err("Error loading purgatory ret=%d\n", ret);
+                       goto out;
+               }
+               ret = kexec_purgatory_get_set_symbol(image, "riscv_kernel_entry",
+                                                    &kernel_start,
+                                                    sizeof(kernel_start), 0);
+               if (ret)
+                       pr_err("Error update purgatory ret=%d\n", ret);
        }
-       ret = kexec_purgatory_get_set_symbol(image, "riscv_kernel_entry",
-                                            &kernel_start,
-                                            sizeof(kernel_start), 0);
-       if (ret)
-               pr_err("Error update purgatory ret=%d\n", ret);
-#endif /* CONFIG_ARCH_HAS_KEXEC_PURGATORY */
 
        /* Add the initrd to the image */
        if (initrd != NULL) {
--

2.37.1

Signature · 2 lines
Best Regards
 Guo Ren
Reviewed-by
#8 lore raw

Re: [PATCH V2 5/6] riscv: elf_kexec: Fixup compile warning

Quoted 57 lines · Guo Ren
Quoted 48 lines · On Sun, Sep 4, 2022 at 6:36 PM <[email protected]> wrote:
Quoted 10 lines · On 04/09/2022 08:26, [email protected] wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

From: Guo Ren <[email protected]>

COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash arch/riscv/

../arch/riscv/kernel/elf_kexec.c: In function 'elf_kexec_load':
../arch/riscv/kernel/elf_kexec.c:185:23: warning: variable 'kernel_start' set but not used [-Wunused-but-set-variable]

Quoted 6 lines
  185 |         unsigned long kernel_start;
      |                       ^~~~~~~~~~~~

Is this then a

Fixes 838b3e28488f ("RISC-V: Load purgatory in kexec_file")

?

Could you also add something like:
"If CONFIG_CRYTPO is not enabled, ...." to explain why this may be unused?

Quoted 21 lines
---
 arch/riscv/kernel/elf_kexec.c | 4 ++++
 1 file changed, 4 insertions(+)
arch/riscv/kernel/elf_kexec.c +2 -0
index 0cb94992c15b..bba3723a0914 100644
@@ -182,7 +182,9 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
        unsigned long new_kernel_pbase = 0UL;
        unsigned long initrd_pbase = 0UL;
        unsigned long headers_sz;
+#ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY
        unsigned long kernel_start;
+#endif /* CONFIG_ARCH_HAS_KEXEC_PURGATORY */
        void *fdt, *headers;
        struct elfhdr ehdr;
        struct kexec_buf kbuf;
@@ -197,7 +199,9 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
                             &old_kernel_pbase, &new_kernel_pbase);
        if (ret)
Quoted 8 lines

arch/riscv/kernel/elf_kexec.c static void *elf_kexec_load(struct kimage *image, char *kernel_buf, · line 204

Quoted 4 lines
arch/riscv/kernel/elf_kexec.c +2 -0
                goto out;
+#ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY
        kernel_start = image->start;
+#endif /* CONFIG_ARCH_HAS_KEXEC_PURGATORY */

Instead of adding more #ifdefs to the file, could we instead just drop the kernel_start variable? For the sake of compilation coverage, we could then also do the following (build-tested only):

Quoted 1 line

Em... I prefer:

Uh, that works for me. The below diff is:

Quoted 14 lines
arch/riscv/kernel/elf_kexec.c +1 -1
index 0cb94992c15b..4b9264340b78 100644
@@ -198,7 +198,7 @@ static void *elf_kexec_load(struct kimage *image,
 char *kernel_buf,
        if (ret)
                goto out;
        kernel_start = image->start;
-       pr_notice("The entry point of kernel at 0x%lx\n", image->start);
+       pr_notice("The entry point of kernel at 0x%lx\n", kernel_start);
 
        /* Add the kernel binary to the image */
Quoted 4 lines

ret = riscv_kexec_elf_load(image, &ehdr, &elf_info,

Quoted 1 line

Feel free to include this patch in your v3 then:

Quoted 60 lines
Quoted 58 lines

-- >8 --
From: Conor Dooley <[email protected]>
Date: Sun, 4 Sep 2022 11:27:07 +0100
Subject: [PATCH] riscv: elf_kexec: replace ifdef with IS_ENABLED()

IS_ENABLED() gives better compile time coverage than #ifdef. Replace the ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY in elf_kexec_load() since none of the code it guards uses a symbol that's missing if it is not set.

---
 arch/riscv/kernel/elf_kexec.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)
arch/riscv/kernel/elf_kexec.c +14 -15
index 0cb94992c15b..29cbf655c474 100644
@@ -248,21 +248,21 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
                cmdline = modified_cmdline;
        }
 
-#ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY
-       /* Add purgatory to the image */
-       kbuf.top_down = true;
-       kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
-       ret = kexec_load_purgatory(image, &kbuf);
-       if (ret) {
-               pr_err("Error loading purgatory ret=%d\n", ret);
-               goto out;
+       if (IS_ENABLED(CONFIG_ARCH_HAS_KEXEC_PURGATORY)) {
+               /* Add purgatory to the image */
+               kbuf.top_down = true;
+               kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
+               ret = kexec_load_purgatory(image, &kbuf);
+               if (ret) {
+                       pr_err("Error loading purgatory ret=%d\n", ret);
+                       goto out;
+               }
+               ret = kexec_purgatory_get_set_symbol(image, "riscv_kernel_entry",
+                                                    &kernel_start,
+                                                    sizeof(kernel_start), 0);
+               if (ret)
+                       pr_err("Error update purgatory ret=%d\n", ret);
        }
-       ret = kexec_purgatory_get_set_symbol(image, "riscv_kernel_entry",
-                                            &kernel_start,
-                                            sizeof(kernel_start), 0);
-       if (ret)
-               pr_err("Error update purgatory ret=%d\n", ret);
-#endif /* CONFIG_ARCH_HAS_KEXEC_PURGATORY */
 
        /* Add the initrd to the image */
        if (initrd != NULL) {
--

2.37.1

author
#9 lore raw

Re: [PATCH V2 5/6] riscv: elf_kexec: Fixup compile warning

Quoted 59 lines · On Sun, Sep 4, 2022 at 7:13 PM <[email protected]> wrote:
Quoted 57 lines · Guo Ren
Quoted 56 lines · On Sun, Sep 4, 2022 at 6:36 PM <[email protected]> wrote:
Quoted 10 lines · On 04/09/2022 08:26, [email protected] wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

From: Guo Ren <[email protected]>

COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-12.1.0 make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash arch/riscv/

../arch/riscv/kernel/elf_kexec.c: In function 'elf_kexec_load':
../arch/riscv/kernel/elf_kexec.c:185:23: warning: variable 'kernel_start' set but not used [-Wunused-but-set-variable]

Quoted 6 lines
  185 |         unsigned long kernel_start;
      |                       ^~~~~~~~~~~~

Is this then a

Fixes 838b3e28488f ("RISC-V: Load purgatory in kexec_file")

?

Could you also add something like:
"If CONFIG_CRYTPO is not enabled, ...." to explain why this may be unused?

Quoted 19 lines
---
 arch/riscv/kernel/elf_kexec.c | 4 ++++
 1 file changed, 4 insertions(+)
arch/riscv/kernel/elf_kexec.c +2 -0
index 0cb94992c15b..bba3723a0914 100644
@@ -182,7 +182,9 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
        unsigned long new_kernel_pbase = 0UL;
        unsigned long initrd_pbase = 0UL;
        unsigned long headers_sz;
+#ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY
        unsigned long kernel_start;
+#endif /* CONFIG_ARCH_HAS_KEXEC_PURGATORY */
        void *fdt, *headers;
        struct elfhdr ehdr;
        struct kexec_buf kbuf;

arch/riscv/kernel/elf_kexec.c static void *elf_kexec_load(struct kimage *image, char *kernel_buf, · line 204

Quoted 6 lines
arch/riscv/kernel/elf_kexec.c +2 -0
@@ -197,7 +199,9 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
                             &old_kernel_pbase, &new_kernel_pbase);
        if (ret)
                goto out;
+#ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY
        kernel_start = image->start;
+#endif /* CONFIG_ARCH_HAS_KEXEC_PURGATORY */

Instead of adding more #ifdefs to the file, could we instead just drop the kernel_start variable? For the sake of compilation coverage, we could then also do the following (build-tested only):

Quoted 4 lines
Quoted 1 line

Em... I prefer:

Uh, that works for me. The below diff is:

Thx, your idea is also awesome which let me know the benefit of IS_ENABLED() than ifdef.

Quoted 81 lines
Quoted 15 lines
arch/riscv/kernel/elf_kexec.c +1 -1
index 0cb94992c15b..4b9264340b78 100644
@@ -198,7 +198,7 @@ static void *elf_kexec_load(struct kimage *image,
 char *kernel_buf,
        if (ret)
                goto out;
        kernel_start = image->start;
-       pr_notice("The entry point of kernel at 0x%lx\n", image->start);
+       pr_notice("The entry point of kernel at 0x%lx\n", kernel_start);
 
        /* Add the kernel binary to the image */
Quoted 4 lines

ret = riscv_kexec_elf_load(image, &ehdr, &elf_info,

Quoted 1 line

Feel free to include this patch in your v3 then:

Quoted 60 lines
Quoted 58 lines

-- >8 --
From: Conor Dooley <[email protected]>
Date: Sun, 4 Sep 2022 11:27:07 +0100
Subject: [PATCH] riscv: elf_kexec: replace ifdef with IS_ENABLED()

IS_ENABLED() gives better compile time coverage than #ifdef. Replace the ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY in elf_kexec_load() since none of the code it guards uses a symbol that's missing if it is not set.

---
 arch/riscv/kernel/elf_kexec.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)
arch/riscv/kernel/elf_kexec.c +14 -15
index 0cb94992c15b..29cbf655c474 100644
@@ -248,21 +248,21 @@ static void *elf_kexec_load(struct kimage *image, char *kernel_buf,
                cmdline = modified_cmdline;
        }
 
-#ifdef CONFIG_ARCH_HAS_KEXEC_PURGATORY
-       /* Add purgatory to the image */
-       kbuf.top_down = true;
-       kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
-       ret = kexec_load_purgatory(image, &kbuf);
-       if (ret) {
-               pr_err("Error loading purgatory ret=%d\n", ret);
-               goto out;
+       if (IS_ENABLED(CONFIG_ARCH_HAS_KEXEC_PURGATORY)) {
+               /* Add purgatory to the image */
+               kbuf.top_down = true;
+               kbuf.mem = KEXEC_BUF_MEM_UNKNOWN;
+               ret = kexec_load_purgatory(image, &kbuf);
+               if (ret) {
+                       pr_err("Error loading purgatory ret=%d\n", ret);
+                       goto out;
+               }
+               ret = kexec_purgatory_get_set_symbol(image, "riscv_kernel_entry",
+                                                    &kernel_start,
+                                                    sizeof(kernel_start), 0);
+               if (ret)
+                       pr_err("Error update purgatory ret=%d\n", ret);
        }
-       ret = kexec_purgatory_get_set_symbol(image, "riscv_kernel_entry",
-                                            &kernel_start,
-                                            sizeof(kernel_start), 0);
-       if (ret)
-               pr_err("Error update purgatory ret=%d\n", ret);
-#endif /* CONFIG_ARCH_HAS_KEXEC_PURGATORY */
 
        /* Add the initrd to the image */
        if (initrd != NULL) {
--

2.37.1

Signature · 2 lines
Best Regards
 Guo Ren
#10 lore raw

PATCH v2 6/6 riscv: compat_syscall_table: Fixup compile warning

In reply to [email protected]

From: Guo Ren <[email protected]>

../arch/riscv/kernel/compat_syscall_table.c:12:41: warning: initialized field overwritten [-Woverride-init]
12 | #define __SYSCALL(nr, call) [nr] = (call),
| ^
../include/uapi/asm-generic/unistd.h:567:1: note: in expansion of macro '__SYSCALL'
567 | __SYSCALL(__NR_semget, sys_semget)

---
 arch/riscv/kernel/Makefile | 1 +
 1 file changed, 1 insertion(+)
arch/riscv/kernel/Makefile +1 -0
index 33bb60a354cd..01da14e21019 100644
@@ -9,6 +9,7 @@ CFLAGS_REMOVE_patch.o	= $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_sbi.o	= $(CC_FLAGS_FTRACE)
 endif
 CFLAGS_syscall_table.o	+= $(call cc-option,-Wno-override-init,)
+CFLAGS_compat_syscall_table.o += $(call cc-option,-Wno-override-init,)
 
 ifdef CONFIG_KEXEC
 AFLAGS_kexec_relocate.o := -mcmodel=medany $(call cc-option,-mno-relax)
Signature · 7 lines
2.36.1


_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv
Reviewed-by on v2 6/6
#11 lore raw

Re: [PATCH V2 6/6] riscv: compat_syscall_table: Fixup compile warning

Quoted 12 lines · On 04/09/2022 08:26, [email protected] wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

From: Guo Ren <[email protected]>

../arch/riscv/kernel/compat_syscall_table.c:12:41: warning: initialized field overwritten [-Woverride-init]
12 | #define __SYSCALL(nr, call) [nr] = (call),
| ^
../include/uapi/asm-generic/unistd.h:567:1: note: in expansion of macro '__SYSCALL'
567 | __SYSCALL(__NR_semget, sys_semget)

Quoted 3 lines

I **love** this patch.. I really wanted to get rid of these warnings since they seemed to be false positives..

Quoted 24 lines
---
 arch/riscv/kernel/Makefile | 1 +
 1 file changed, 1 insertion(+)
arch/riscv/kernel/Makefile +1 -1
index 33bb60a354cd..01da14e21019 100644
@@ -9,6 +9,7 @@ CFLAGS_REMOVE_patch.o   = $(CC_FLAGS_FTRACE)
 CFLAGS_REMOVE_sbi.o    = $(CC_FLAGS_FTRACE)
 endif
 CFLAGS_syscall_table.o += $(call cc-option,-Wno-override-init,)
+CFLAGS_compat_syscall_table.o += $(call cc-option,-Wno-override-init,)
 
 ifdef CONFIG_KEXEC
 AFLAGS_kexec_relocate.o := -mcmodel=medany $(call cc-option,-mno-relax)
--

2.36.1

_______________________________________________
linux-riscv mailing list
[email protected]
http://lists.infradead.org/mailman/listinfo/linux-riscv

#12 lore raw
In reply to [email protected]

Hey Guo Ren,
(off topic: is Guo or Ren your given name?)

This series seems to introduce a build warning:

arch/riscv/kernel/irq.c:17:1: warning: symbol 'irq_stack_ptr' was not declared. Should it be static?

One more comment below:

Quoted 9 lines · On 04/09/2022 08:26, [email protected] wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

From: Guo Ren <[email protected]>

The patches convert riscv to use the generic entry infrastructure from kernel/entry/*. Add independent irq stacks (IRQ_STACKS) for percpu to prevent kernel stack overflows. Add the HAVE_SOFTIRQ_ON_OWN_STACK feature for the IRQ_STACKS config. You can try it directly with [1].

Quoted 6 lines

[1] https://github.com/guoren83/linux/tree/generic_entry_v2

Changes in V2:
- Fixup compile error by include "riscv: ptrace: Remove duplicate operation"
https://lore.kernel.org/linux-riscv/[email protected]/T/#u

I find this really confusing. The same patch is in two different series? Is the above series no longer required & this is a different approach? Thanks,
Conor.

Quoted 37 lines

- Fixup compile warning

- Add test repo link in cover letter

Guo Ren (6):
riscv: ptrace: Remove duplicate operation
riscv: convert to generic entry
riscv: Support HAVE_IRQ_EXIT_ON_IRQ_STACK
riscv: Support HAVE_SOFTIRQ_ON_OWN_STACK
riscv: elf_kexec: Fixup compile warning
riscv: compat_syscall_table: Fixup compile warning

 arch/riscv/Kconfig                    |  10 +
 arch/riscv/include/asm/csr.h          |   1 -
 arch/riscv/include/asm/entry-common.h |   8 +
 arch/riscv/include/asm/irq.h          |   3 +
 arch/riscv/include/asm/ptrace.h       |  10 +-
 arch/riscv/include/asm/stacktrace.h   |   5 +
 arch/riscv/include/asm/syscall.h      |   6 +
 arch/riscv/include/asm/thread_info.h  |  15 +-
 arch/riscv/include/asm/vmap_stack.h   |  28 +++
 arch/riscv/kernel/Makefile            |   1 +
 arch/riscv/kernel/elf_kexec.c         |   4 +
 arch/riscv/kernel/entry.S             | 255 +++++---------------------
 arch/riscv/kernel/irq.c               |  75 ++++++++
 arch/riscv/kernel/ptrace.c            |  41 -----
 arch/riscv/kernel/signal.c            |  21 +--
 arch/riscv/kernel/sys_riscv.c         |  26 +++
 arch/riscv/kernel/traps.c             |  11 ++
 arch/riscv/mm/fault.c                 |  12 +-
 18 files changed, 250 insertions(+), 282 deletions(-)
 create mode 100644 arch/riscv/include/asm/entry-common.h
 create mode 100644 arch/riscv/include/asm/vmap_stack.h

--
2.36.1

author
#13 lore raw
Quoted 20 lines · On Sun, Sep 4, 2022 at 6:17 PM <[email protected]> wrote:

Hey Guo Ren,
(off topic: is Guo or Ren your given name?)

This series seems to introduce a build warning:

arch/riscv/kernel/irq.c:17:1: warning: symbol 'irq_stack_ptr' was not declared. Should it be static?

One more comment below:

Quoted 9 lines · On 04/09/2022 08:26, [email protected] wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

From: Guo Ren <[email protected]>

The patches convert riscv to use the generic entry infrastructure from kernel/entry/*. Add independent irq stacks (IRQ_STACKS) for percpu to prevent kernel stack overflows. Add the HAVE_SOFTIRQ_ON_OWN_STACK feature for the IRQ_STACKS config. You can try it directly with [1].

Quoted 8 lines
Quoted 6 lines

[1] https://github.com/guoren83/linux/tree/generic_entry_v2

Changes in V2:
- Fixup compile error by include "riscv: ptrace: Remove duplicate operation"
https://lore.kernel.org/linux-riscv/20220903162328 [email protected]/T/#u

I find this really confusing. The same patch is in two different series?

Generic entry needn't TIF_SYSCALL_TRACE. So it depends on "riscv:
ptrace: Remove duplicate"

Quoted 1 line

Is the above series no longer required & this is a different approach?

Above series cleanup ptrace_disable, we still need that.

Quoted 41 lines

Thanks,
Conor.

Quoted 37 lines

- Fixup compile warning

- Add test repo link in cover letter

Guo Ren (6):
riscv: ptrace: Remove duplicate operation
riscv: convert to generic entry
riscv: Support HAVE_IRQ_EXIT_ON_IRQ_STACK
riscv: Support HAVE_SOFTIRQ_ON_OWN_STACK
riscv: elf_kexec: Fixup compile warning
riscv: compat_syscall_table: Fixup compile warning

 arch/riscv/Kconfig                    |  10 +
 arch/riscv/include/asm/csr.h          |   1 -
 arch/riscv/include/asm/entry-common.h |   8 +
 arch/riscv/include/asm/irq.h          |   3 +
 arch/riscv/include/asm/ptrace.h       |  10 +-
 arch/riscv/include/asm/stacktrace.h   |   5 +
 arch/riscv/include/asm/syscall.h      |   6 +
 arch/riscv/include/asm/thread_info.h  |  15 +-
 arch/riscv/include/asm/vmap_stack.h   |  28 +++
 arch/riscv/kernel/Makefile            |   1 +
 arch/riscv/kernel/elf_kexec.c         |   4 +
 arch/riscv/kernel/entry.S             | 255 +++++---------------------
 arch/riscv/kernel/irq.c               |  75 ++++++++
 arch/riscv/kernel/ptrace.c            |  41 -----
 arch/riscv/kernel/signal.c            |  21 +--
 arch/riscv/kernel/sys_riscv.c         |  26 +++
 arch/riscv/kernel/traps.c             |  11 ++
 arch/riscv/mm/fault.c                 |  12 +-
 18 files changed, 250 insertions(+), 282 deletions(-)
 create mode 100644 arch/riscv/include/asm/entry-common.h
 create mode 100644 arch/riscv/include/asm/vmap_stack.h

--
2.36.1

Signature · 2 lines
Best Regards
 Guo Ren
author
#14 lore raw
In reply to [email protected]
Quoted 3 lines · On Sun, Sep 4, 2022 at 6:17 PM <[email protected]> wrote:

Hey Guo Ren,
(off topic: is Guo or Ren your given name?)

My name is written in Guo Ren / guoren. Don't separate them. In China, no one calls me Guo or Ren separately, it makes me strange.

Quoted 4 lines

This series seems to introduce a build warning:

arch/riscv/kernel/irq.c:17:1: warning: symbol 'irq_stack_ptr' was not declared. Should it be static?

I don't have that warning. But you are right, it should be static. Thank you. I will fix it in the next version of the patch.

Quoted 63 lines

One more comment below:

Quoted 9 lines · On 04/09/2022 08:26, [email protected] wrote:

EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe

From: Guo Ren <[email protected]>

The patches convert riscv to use the generic entry infrastructure from kernel/entry/*. Add independent irq stacks (IRQ_STACKS) for percpu to prevent kernel stack overflows. Add the HAVE_SOFTIRQ_ON_OWN_STACK feature for the IRQ_STACKS config. You can try it directly with [1].

Quoted 6 lines

[1] https://github.com/guoren83/linux/tree/generic_entry_v2

Changes in V2:
- Fixup compile error by include "riscv: ptrace: Remove duplicate operation"
https://lore.kernel.org/linux-riscv/[email protected]/T/#u

I find this really confusing. The same patch is in two different series? Is the above series no longer required & this is a different approach? Thanks,
Conor.

Quoted 37 lines

- Fixup compile warning

- Add test repo link in cover letter

Guo Ren (6):
riscv: ptrace: Remove duplicate operation
riscv: convert to generic entry
riscv: Support HAVE_IRQ_EXIT_ON_IRQ_STACK
riscv: Support HAVE_SOFTIRQ_ON_OWN_STACK
riscv: elf_kexec: Fixup compile warning
riscv: compat_syscall_table: Fixup compile warning

 arch/riscv/Kconfig                    |  10 +
 arch/riscv/include/asm/csr.h          |   1 -
 arch/riscv/include/asm/entry-common.h |   8 +
 arch/riscv/include/asm/irq.h          |   3 +
 arch/riscv/include/asm/ptrace.h       |  10 +-
 arch/riscv/include/asm/stacktrace.h   |   5 +
 arch/riscv/include/asm/syscall.h      |   6 +
 arch/riscv/include/asm/thread_info.h  |  15 +-
 arch/riscv/include/asm/vmap_stack.h   |  28 +++
 arch/riscv/kernel/Makefile            |   1 +
 arch/riscv/kernel/elf_kexec.c         |   4 +
 arch/riscv/kernel/entry.S             | 255 +++++---------------------
 arch/riscv/kernel/irq.c               |  75 ++++++++
 arch/riscv/kernel/ptrace.c            |  41 -----
 arch/riscv/kernel/signal.c            |  21 +--
 arch/riscv/kernel/sys_riscv.c         |  26 +++
 arch/riscv/kernel/traps.c             |  11 ++
 arch/riscv/mm/fault.c                 |  12 +-
 18 files changed, 250 insertions(+), 282 deletions(-)
 create mode 100644 arch/riscv/include/asm/entry-common.h
 create mode 100644 arch/riscv/include/asm/vmap_stack.h

--
2.36.1

Signature · 2 lines
Best Regards
 Guo Ren
#15 lore raw
In reply to [email protected]
Quoted 4 lines · On Sun, Sep 04, 2022 at 03:26:31AM -0400, [email protected] wrote:

From: Guo Ren <[email protected]>

The patches convert riscv to use the generic entry infrastructure from kernel/entry/*. Add independent irq stacks (IRQ_STACKS) for percpu to

Amazing! You read my mind. I planed to do similar series this week, as can be seen, I didn't RESEND the riscv irqstack patch, I planed to add irqstack after generic entry. Thanks for this series.

Will read and test your patches soon. A minor comments below.

Quoted 13 lines

prevent kernel stack overflows. Add the HAVE_SOFTIRQ_ON_OWN_STACK feature for the IRQ_STACKS config. You can try it directly with [1].

[1] https://github.com/guoren83/linux/tree/generic_entry_v2

Changes in V2:
- Fixup compile error by include "riscv: ptrace: Remove duplicate operation"
https://lore.kernel.org/linux-riscv/[email protected]/T/#u
- Fixup compile warning

- Add test repo link in cover letter

Quoted 7 lines

Guo Ren (6):
riscv: ptrace: Remove duplicate operation
riscv: convert to generic entry
riscv: Support HAVE_IRQ_EXIT_ON_IRQ_STACK
riscv: Support HAVE_SOFTIRQ_ON_OWN_STACK
riscv: elf_kexec: Fixup compile warning
riscv: compat_syscall_table: Fixup compile warning

It's better to move these two patches ahead of patch2.

Quoted 26 lines
 arch/riscv/Kconfig                    |  10 +
 arch/riscv/include/asm/csr.h          |   1 -
 arch/riscv/include/asm/entry-common.h |   8 +
 arch/riscv/include/asm/irq.h          |   3 +
 arch/riscv/include/asm/ptrace.h       |  10 +-
 arch/riscv/include/asm/stacktrace.h   |   5 +
 arch/riscv/include/asm/syscall.h      |   6 +
 arch/riscv/include/asm/thread_info.h  |  15 +-
 arch/riscv/include/asm/vmap_stack.h   |  28 +++
 arch/riscv/kernel/Makefile            |   1 +
 arch/riscv/kernel/elf_kexec.c         |   4 +
 arch/riscv/kernel/entry.S             | 255 +++++---------------------
 arch/riscv/kernel/irq.c               |  75 ++++++++
 arch/riscv/kernel/ptrace.c            |  41 -----
 arch/riscv/kernel/signal.c            |  21 +--
 arch/riscv/kernel/sys_riscv.c         |  26 +++
 arch/riscv/kernel/traps.c             |  11 ++
 arch/riscv/mm/fault.c                 |  12 +-
 18 files changed, 250 insertions(+), 282 deletions(-)
 create mode 100644 arch/riscv/include/asm/entry-common.h
 create mode 100644 arch/riscv/include/asm/vmap_stack.h
Signature · 1 line
2.36.1
author
#16 lore raw
Quoted 8 lines · Jisheng Zhang
Quoted 4 lines · On Sun, Sep 04, 2022 at 03:26:31AM -0400, [email protected] wrote:

From: Guo Ren <[email protected]>

The patches convert riscv to use the generic entry infrastructure from kernel/entry/*. Add independent irq stacks (IRQ_STACKS) for percpu to

Amazing! You read my mind. I planed to do similar series this week, as

I'm happy you liked it.

Quoted 4 lines

can be seen, I didn't RESEND the riscv irqstack patch, I planed to add irqstack after generic entry. Thanks for this series.

Will read and test your patches soon. A minor comments below.

Thx, any tests and reviews are helpful.

Quoted 22 lines
Quoted 14 lines

prevent kernel stack overflows. Add the HAVE_SOFTIRQ_ON_OWN_STACK feature for the IRQ_STACKS config. You can try it directly with [1].

[1] https://github.com/guoren83/linux/tree/generic_entry_v2

Changes in V2:
- Fixup compile error by include "riscv: ptrace: Remove duplicate operation"
https://lore.kernel.org/linux-riscv/[email protected]/T/#u
- Fixup compile warning

- Add test repo link in cover letter

Quoted 7 lines

Guo Ren (6):
riscv: ptrace: Remove duplicate operation
riscv: convert to generic entry
riscv: Support HAVE_IRQ_EXIT_ON_IRQ_STACK
riscv: Support HAVE_SOFTIRQ_ON_OWN_STACK
riscv: elf_kexec: Fixup compile warning
riscv: compat_syscall_table: Fixup compile warning

Quoted 1 line

It's better to move these two patches ahead of patch2.

Okay.

Quoted 27 lines
Quoted 27 lines

 arch/riscv/Kconfig                    |  10 +
 arch/riscv/include/asm/csr.h          |   1 -
 arch/riscv/include/asm/entry-common.h |   8 +
 arch/riscv/include/asm/irq.h          |   3 +
 arch/riscv/include/asm/ptrace.h       |  10 +-
 arch/riscv/include/asm/stacktrace.h   |   5 +
 arch/riscv/include/asm/syscall.h      |   6 +
 arch/riscv/include/asm/thread_info.h  |  15 +-
 arch/riscv/include/asm/vmap_stack.h   |  28 +++
 arch/riscv/kernel/Makefile            |   1 +
 arch/riscv/kernel/elf_kexec.c         |   4 +
 arch/riscv/kernel/entry.S             | 255 +++++---------------------
 arch/riscv/kernel/irq.c               |  75 ++++++++
 arch/riscv/kernel/ptrace.c            |  41 -----
 arch/riscv/kernel/signal.c            |  21 +--
 arch/riscv/kernel/sys_riscv.c         |  26 +++
 arch/riscv/kernel/traps.c             |  11 ++
 arch/riscv/mm/fault.c                 |  12 +-
 18 files changed, 250 insertions(+), 282 deletions(-)
 create mode 100644 arch/riscv/include/asm/entry-common.h
 create mode 100644 arch/riscv/include/asm/vmap_stack.h

--
2.36.1

Signature · 2 lines
Best Regards
 Guo Ren