Skip to contentSkip to conversation

riscv: Add GENERIC_ENTRY support and related features

PATCH Superseded [email protected] 2 participants 26 messages Dec 8, 2022 – Dec 10, 2022

Subsystems (3)

v10 of 17 Superseded Latest is v17 →

lore
Share & tools
lore
#0 lore raw

v10 cover riscv: Add GENERIC_ENTRY support and related features

From: Guo Ren <[email protected]>

The patches convert riscv to use the generic entry infrastructure from kernel/entry/*. Additionally, add independent irq stacks (IRQ_STACKS) for percpu to prevent kernel stack overflows. Add generic_entry based STACKLEAK support. Some optimization for entry.S with new .macro and merge ret_from_kernel_thread into ret_from_fork.

The 1,2 are the preparation of generic entry. 3~7 are the main part of generic entry. 8~10 are separate-irq-stack optimizations based on generic entry.

All tested with rv64, rv32, rv64 + 32rootfs, all are passed.

You can directly try it with:
[1] https://github.com/guoren83/linux/tree/generic_entry_v10

Any reviews and tests are helpful.

v10:
- Rebase on palmer/for-next branch (20221208)
- Remove unrelated patches from the series (Suggested-by: Bjorn)
- Fixup Typos.

v9:
https://lore.kernel.org/linux-riscv/[email protected]/
- Fixup NR_syscalls check (by Ben Hutchings)
- Add Tested-by: Jisheng Zhang

v8:
https://lore.kernel.org/linux-riscv/[email protected]/
- Rebase on palmer/for-next branch (20221102)
- Add save/restore_from_x5_to_x31 .macro (JishengZhang)
- Consolidate ret_from_kernel_thread into ret_from_fork (JishengZhang)
- Optimize __noinstr_section comment (JiangshanLai)

v7:
https://lore.kernel.org/linux-riscv/[email protected]/
- Fixup regs_irqs_disabled with SR_PIE
- Optimize stackleak_erase -> stackleak_erase_on_task_stack (Thx Mark Rutland)
- Add BUG_ON(!irqs_disabled()) in trap handlers
- Using regs_irqs_disabled in __do_page_fault
- Remove unnecessary irq disable in ret_from_exception and add comment

v6:
https://lore.kernel.org/linux-riscv/[email protected]/
- Use THEAD_SIZE_ORDER for thread size adjustment in kconfig (Thx Arnd)
- Move call_on_stack to inline style (Thx Peter Zijlstra)
- Fixup fp chain broken (Thx Chen Zhongjin)
- Remove common entry modification, and fixup page_fault entry (Thx Peter Zijlstra)
- Treat some traps as nmi entry (Thx Peter Zijlstra)

v5:
https://lore.kernel.org/linux-riscv/[email protected]/
- Add riscv own stackleak patch instead of generic entry modification (by Mark Rutland)
- Add EXPERT dependency for THREAD_SIZE (by Arnd)
- Add EXPERT dependency for IRQ_STACK (by Sebastian, David Laight)
- Corrected __trap_section (by Peter Zijlstra)
- Add Tested-by (Yipeng Zou)
- Use CONFIG_SOFTIRQ_ON_OWN_STACK replace "#ifndef CONFIG_PREEMPT_RT"
- Fixup systrace_enter compile error
- Fixup exit_to_user_mode_prepare preempt_disable warning

V4:
https://lore.kernel.org/linux-riscv/[email protected]/
- Fixup entry.S with "la" bug (by Conor.Dooley)
- Fixup missing noinstr bug (by Peter Zijlstra)

V3:
https://lore.kernel.org/linux-riscv/[email protected]/
- Fixup CONFIG_COMPAT=n compile error
- Add THREAD_SIZE_ORDER config
- Optimize elf_kexec.c warning fixup
- Add static to irq_stack_ptr definition

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

- Add test repo link in cover letter

V1:
https://lore.kernel.org/linux-riscv/[email protected]/

Guo Ren (6):
riscv: ptrace: Remove duplicate operation
riscv: entry: Add noinstr to prevent instrumentation inserted
riscv: entry: Convert to generic entry
riscv: stack: Support HAVE_IRQ_EXIT_ON_IRQ_STACK
riscv: stack: Support HAVE_SOFTIRQ_ON_OWN_STACK
riscv: stack: Add config of thread stack size

Jisheng Zhang (3):
riscv: entry: Remove extra level wrappers of trace_hardirqs_{on,off}
riscv: entry: Consolidate ret_from_kernel_thread into ret_from_fork
riscv: entry: Consolidate general regs saving/restoring

Lai Jiangshan (1):
compiler_types.h: Add __noinstr_section() for noinstr

 arch/riscv/Kconfig                    |  20 ++
 arch/riscv/include/asm/asm.h          |  63 +++++
 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  |  27 +--
 arch/riscv/include/asm/vmap_stack.h   |  28 +++
 arch/riscv/kernel/Makefile            |   2 -
 arch/riscv/kernel/entry.S             | 325 +++-----------------------
 arch/riscv/kernel/irq.c               | 110 +++++++++
 arch/riscv/kernel/mcount-dyn.S        |  56 +----
 arch/riscv/kernel/process.c           |   5 +-
 arch/riscv/kernel/ptrace.c            |  44 ----
 arch/riscv/kernel/signal.c            |  21 +-
 arch/riscv/kernel/sys_riscv.c         |  29 +++
 arch/riscv/kernel/trace_irq.c         |  27 ---
 arch/riscv/kernel/trace_irq.h         |  11 -
 arch/riscv/kernel/traps.c             |  74 ++++--
 arch/riscv/mm/fault.c                 |  16 +-
 include/linux/compiler_types.h        |  15 +-
 22 files changed, 402 insertions(+), 501 deletions(-)
 create mode 100644 arch/riscv/include/asm/entry-common.h
 create mode 100644 arch/riscv/include/asm/vmap_stack.h
 delete mode 100644 arch/riscv/kernel/trace_irq.c
 delete mode 100644 arch/riscv/kernel/trace_irq.h
Signature · 1 line
2.36.1
#1 lore raw

PATCH v10 1/10 compiler_types.h: Add __noinstr_section() for noinstr

From: Lai Jiangshan <[email protected]>

Using __noinstr_section() doesn't automatically disable all instrumentations on the section. Inhibition for some instrumentations requires extra code. I.E. KPROBES explicitly avoids instrumenting on .noinstr.text.

---
 include/linux/compiler_types.h | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)
include/linux/compiler_types.h +11 -4
index eb0466236661..41e4faa4cd95 100644
@@ -230,12 +230,19 @@ struct ftrace_likely_data {
 #define __no_sanitize_or_inline __always_inline
 #endif
 
-/* Section for code which can't be instrumented at all */
-#define noinstr								\
-	noinline notrace __attribute((__section__(".noinstr.text")))	\
-	__no_kcsan __no_sanitize_address __no_profile __no_sanitize_coverage \
+/*
+ * Using __noinstr_section() doesn't automatically disable all instrumentations
+ * on the section.  Inhibition for some instrumentations requires extra code.
+ * I.E. KPROBES explicitly avoids instrumenting on .noinstr.text.
+ */
+#define __noinstr_section(section)				\
+	noinline notrace __section(section) __no_profile	\
+	__no_kcsan __no_sanitize_address __no_sanitize_coverage	\
 	__no_sanitize_memory
 
+/* Section for code which can't be instrumented at all */
+#define noinstr __noinstr_section(".noinstr.text")
+
 #endif /* __KERNEL__ */
 
 #endif /* __ASSEMBLY__ */
Signature · 1 line
2.36.1
#2 lore raw

PATCH v10 2/10 riscv: ptrace: Remove duplicate operation

In reply to [email protected]

From: Guo Ren <[email protected]>

The TIF_SYSCALL_TRACE is controlled by a common code, see kernel/ptrace.c and include/linux/thread_info.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 · 7 lines
2.36.1


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

PATCH v10 3/10 riscv: entry: Add noinstr to prevent instrumentation inserted

In reply to [email protected]

From: Guo Ren <[email protected]>

Without noinstr the compiler is free to insert instrumentation (think all the k*SAN, KCov, GCov, ftrace etc..) which can call code we're not yet ready to run this early in the entry path, for instance it could rely on RCU which isn't on yet, or expect lockdep state. (by peterz)

---
 arch/riscv/kernel/traps.c | 4 ++--
 arch/riscv/mm/fault.c     | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
arch/riscv/kernel/traps.c +2 -2
index f3e96d60a2ff..f7fa973558bc 100644
@@ -95,9 +95,9 @@ static void do_trap_error(struct pt_regs *regs, int signo, int code,
 }
 
 #if defined(CONFIG_XIP_KERNEL) && defined(CONFIG_RISCV_ALTERNATIVE)
-#define __trap_section		__section(".xip.traps")
+#define __trap_section __noinstr_section(".xip.traps")
 #else
-#define __trap_section
+#define __trap_section noinstr
 #endif
 #define DO_ERROR_INFO(name, signo, code, str)				\
 asmlinkage __visible __trap_section void name(struct pt_regs *regs)	\
arch/riscv/mm/fault.c +1 -1
index d86f7cebd4a7..b26f68eac61c 100644
@@ -204,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)
+asmlinkage void noinstr do_page_fault(struct pt_regs *regs)
 {
 	struct task_struct *tsk;
 	struct vm_area_struct *vma;
Signature · 7 lines
2.36.1


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

PATCH v10 4/10 riscv: entry: 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             | 237 ++++----------------------
 arch/riscv/kernel/irq.c               |  15 ++
 arch/riscv/kernel/ptrace.c            |  43 -----
 arch/riscv/kernel/signal.c            |  21 +--
 arch/riscv/kernel/sys_riscv.c         |  29 ++++
 arch/riscv/kernel/traps.c             |  70 ++++++--
 arch/riscv/mm/fault.c                 |  16 +-
 14 files changed, 175 insertions(+), 300 deletions(-)
 create mode 100644 arch/riscv/include/asm/entry-common.h
arch/riscv/Kconfig +1 -0
index ef8d66de5f38..518e8523d41d 100644
@@ -57,6 +57,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..b5b0adcc85c1 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_PIE);
+}
+
 #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 +29 -208

320 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 -43

55 lines not on this page · raw message

arch/riscv/kernel/signal.c +2 -19
index 5c591123c440..2e365084417e 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>
@@ -274,7 +275,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;
 
@@ -311,21 +312,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 +29 -0
index 5d3f2fbeb33c..459e24ed61fa 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>
 
@@ -69,3 +71,30 @@ 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 nr = regs->a7;
+
+	regs->epc += 4;
+	regs->orig_a0 = regs->a0;
+	regs->a0 = -ENOSYS;
+
+	nr = syscall_enter_from_user_mode(regs, nr);
+
+	if (nr < NR_syscalls) {
+#ifdef CONFIG_COMPAT
+		if ((regs->status & SR_UXL) == SR_UXL_32)
+			syscall = compat_sys_call_table[nr];
+		else
+#endif
+			syscall = sys_call_table[nr];
+
+		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 +57 -13

104 lines not on this page · raw message

arch/riscv/mm/fault.c +14 -2
index b26f68eac61c..5cbea6c55a59 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>
@@ -204,7 +205,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 noinstr 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;
@@ -251,7 +252,7 @@ asmlinkage void noinstr do_page_fault(struct pt_regs *regs)
 	}
 #endif
 	/* Enable interrupts if they were enabled in the parent context. */
-	if (likely(regs->status & SR_PIE))
+	if (!regs_irqs_disabled(regs))
 		local_irq_enable();
 
 	/*
@@ -351,4 +352,15 @@ asmlinkage void noinstr do_page_fault(struct pt_regs *regs)
 	}
 	return;
 }
+
+asmlinkage void noinstr do_page_fault(struct pt_regs *regs)
+{
+	irqentry_state_t state = irqentry_enter(regs);
+
+	__do_page_fault(regs);
+
+	local_irq_disable();
+
+	irqentry_exit(regs, state);
+}
 NOKPROBE_SYMBOL(do_page_fault);
Signature · 7 lines
2.36.1


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

Re: [PATCH -next V10 04/10] riscv: entry: Convert to generic entry

[email protected] writes:

The RISC-V entry.S is much more paletable after this patch! :-)

Some minor things...

Quoted 5 lines

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:

s/changes than before/changes/

Quoted 3 lines

- More clear entry.S with handle_exception and ret_from_exception
- Get rid of complex custom signal implementation
- More readable syscall procedure

Maybe reword this a bit? It's a move from assembly to C (which, is much more readable!).

Quoted 1 line

- Little modification on ret_from_fork & ret_from_kernel_thread

What changes?

Quoted 26 lines

- 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             | 237 ++++----------------------
 arch/riscv/kernel/irq.c               |  15 ++
 arch/riscv/kernel/ptrace.c            |  43 -----
 arch/riscv/kernel/signal.c            |  21 +--
 arch/riscv/kernel/sys_riscv.c         |  29 ++++
 arch/riscv/kernel/traps.c             |  70 ++++++--
 arch/riscv/mm/fault.c                 |  16 +-
 14 files changed, 175 insertions(+), 300 deletions(-)
 create mode 100644 arch/riscv/include/asm/entry-common.h

[...]

Quoted 24 lines
arch/riscv/kernel/entry.S +0 -9
index da44fe2d0d82..69097dfffdc9 100644
@@ -14,10 +14,6 @@
 #include <asm/asm-offsets.h>
 #include <asm/errata_list.h>
 
-#if !IS_ENABLED(CONFIG_PREEMPTION)
-.set resume_kernel, restore_all
-#endif
-
 ENTRY(handle_exception)
 	/*
 	 * If coming from userspace, preserve the user thread pointer and load
@@ -106,19 +102,8 @@ _save_context:
 .option norelax
 	la gp, __global_pointer$
 .option pop
-
-#ifdef CONFIG_TRACE_IRQFLAGS
-	call __trace_hardirqs_off
-#endif
-

arch/riscv/kernel/entry.S _save_context: · line 106

Quoted 10 lines
arch/riscv/kernel/entry.S +2 -8
-#ifdef CONFIG_CONTEXT_TRACKING_USER
-	/* If previous state is in user mode, call user_exit_callable(). */
-	li   a0, SR_PP
-	and a0, s1, a0
-	bnez a0, skip_context_tracking
-	call user_exit_callable
-skip_context_tracking:
-#endif
+	move a0, sp /* pt_regs */
+	la ra, ret_from_exception

Not for this series, but at some point it would be nice to get rid of the "move" pseudoinsn in favor of "mv".

[...]

Quoted 42 lines
arch/riscv/kernel/traps.c +14 -6
index f7fa973558bc..ee9a0ef672e9 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,10 +100,19 @@ static void do_trap_error(struct pt_regs *regs, int signo, int code,
 #else
 #define __trap_section noinstr
 #endif
-#define DO_ERROR_INFO(name, signo, code, str)				\
-asmlinkage __visible __trap_section void name(struct pt_regs *regs)	\
-{									\
-	do_trap_error(regs, signo, code, regs->epc, "Oops - " str);	\
+#define DO_ERROR_INFO(name, signo, code, str)					\
+asmlinkage __visible __trap_section void name(struct pt_regs *regs)		\
+{										\
+	if (user_mode(regs)) {							\
+		irqentry_enter_from_user_mode(regs);				\
+		do_trap_error(regs, signo, code, regs->epc, "Oops - " str);	\
+		irqentry_exit_to_user_mode(regs);				\
+	} else {								\
+		irqentry_state_t irq_state = irqentry_nmi_enter(regs);		\
+		do_trap_error(regs, signo, code, regs->epc, "Oops - " str);	\
+		irqentry_nmi_exit(regs, irq_state);				\
+	}									\
+	BUG_ON(!irqs_disabled());						\
 }
 
 DO_ERROR_INFO(do_trap_unknown,
@@ -126,18 +136,38 @@ int handle_misaligned_store(struct pt_regs *regs);
 
 asmlinkage void __trap_section do_trap_load_misaligned(struct pt_regs *regs)
 {
-	if (!handle_misaligned_load(regs))
-		return;

arch/riscv/kernel/traps.c int handle_misaligned_store(struct pt_regs *regs); · line 146

Quoted 10 lines
arch/riscv/kernel/traps.c +8 -2
-	do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
-		      "Oops - load address misaligned");
+	if (user_mode(regs)) {
+		irqentry_enter_from_user_mode(regs);
+		if (handle_misaligned_load(regs))
+			do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
+			      "Oops - load address misaligned");
+		irqentry_exit_to_user_mode(regs);
+	} else {
+		irqentry_state_t irq_state = irqentry_nmi_enter(regs);

Please add a newline.

Quoted 12 lines
arch/riscv/kernel/traps.c +6 -2
+		if (handle_misaligned_load(regs))
+			do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
+			      "Oops - load address misaligned");
+		irqentry_nmi_exit(regs, irq_state);
+	}
+	BUG_ON(!irqs_disabled());
 }
 
 asmlinkage void __trap_section do_trap_store_misaligned(struct pt_regs *regs)
 {
-	if (!handle_misaligned_store(regs))
-		return;

arch/riscv/kernel/traps.c int handle_misaligned_store(struct pt_regs *regs); · line 164

Quoted 10 lines
arch/riscv/kernel/traps.c +8 -2
-	do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
-		      "Oops - store (or AMO) address misaligned");
+	if (user_mode(regs)) {
+		irqentry_enter_from_user_mode(regs);
+		if (handle_misaligned_store(regs))
+			do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
+				"Oops - store (or AMO) address misaligned");
+		irqentry_exit_to_user_mode(regs);
+	} else {
+		irqentry_state_t irq_state = irqentry_nmi_enter(regs);

Please add a newline.

Quoted 21 lines
arch/riscv/kernel/traps.c +7 -1
+		if (handle_misaligned_store(regs))
+			do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
+				"Oops - store (or AMO) address misaligned");
+		irqentry_nmi_exit(regs, irq_state);
+	}
+	BUG_ON(!irqs_disabled());
 }
 #endif
 DO_ERROR_INFO(do_trap_store_fault,
@@ -159,7 +189,7 @@ static inline unsigned long get_break_insn_length(unsigned long pc)
 	return GET_INSN_LENGTH(insn);
 }
 
-asmlinkage __visible __trap_section void do_trap_break(struct pt_regs *regs)
+static void __do_trap_break(struct pt_regs *regs)
 {
 #ifdef CONFIG_KPROBES
 	if (kprobe_single_step_handler(regs))
@@ -189,6 +219,20 @@ asmlinkage __visible __trap_section void do_trap_break(struct pt_regs *regs)
 	else
 		die(regs, "Kernel BUG");

arch/riscv/kernel/traps.c asmlinkage __visible __trap_section void do_trap_break(struct pt_regs *regs) · line 230

Quoted 10 lines
arch/riscv/kernel/traps.c +9 -0
 }
+
+asmlinkage __visible __trap_section void do_trap_break(struct pt_regs *regs)
+{
+	if (user_mode(regs)) {
+		irqentry_enter_from_user_mode(regs);
+		__do_trap_break(regs);
+		irqentry_exit_to_user_mode(regs);
+	} else {
+		irqentry_state_t irq_state = irqentry_nmi_enter(regs);

Please add a newline.

Björn

author
#6 lore raw

Re: [PATCH -next V10 04/10] riscv: entry: Convert to generic entry

Quoted 7 lines · Björn Töpel

[email protected] writes:

The RISC-V entry.S is much more paletable after this patch! :-)

Some minor things...

Quoted 7 lines
Quoted 5 lines

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:

s/changes than before/changes/

Okay

Quoted 7 lines
Quoted 4 lines

- More clear entry.S with handle_exception and ret_from_exception
- Get rid of complex custom signal implementation
- More readable syscall procedure

Maybe reword this a bit? It's a move from assembly to C (which, is much more readable!).

Okay.

Quoted 4 lines
Quoted 2 lines

- Little modification on ret_from_fork & ret_from_kernel_thread

What changes?

ENTRY(ret_from_fork)
+ call schedule_tail
+ move a0, sp /* pt_regs */
la ra, ret_from_exception
- tail schedule_tail
+ tail syscall_exit_to_user_mode
ENDPROC(ret_from_fork)

ENTRY(ret_from_kernel_thread)

        call schedule_tail
        /* Call fn(arg) */
-       la ra, ret_from_exception
        move a0, s1
-       jr s0
+       jalr s0
+       move a0, sp /* pt_regs */
+       la ra, ret_from_exception
+       tail syscall_exit_to_user_mode

ENDPROC(ret_from_kernel_thread)

Quoted 108 lines
Quoted 27 lines

- 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             | 237 ++++----------------------
 arch/riscv/kernel/irq.c               |  15 ++
 arch/riscv/kernel/ptrace.c            |  43 -----
 arch/riscv/kernel/signal.c            |  21 +--
 arch/riscv/kernel/sys_riscv.c         |  29 ++++
 arch/riscv/kernel/traps.c             |  70 ++++++--
 arch/riscv/mm/fault.c                 |  16 +-
 14 files changed, 175 insertions(+), 300 deletions(-)
 create mode 100644 arch/riscv/include/asm/entry-common.h

[...]

Quoted 24 lines
arch/riscv/kernel/entry.S +0 -9
index da44fe2d0d82..69097dfffdc9 100644
@@ -14,10 +14,6 @@
 #include <asm/asm-offsets.h>
 #include <asm/errata_list.h>
 
-#if !IS_ENABLED(CONFIG_PREEMPTION)
-.set resume_kernel, restore_all
-#endif
-
 ENTRY(handle_exception)
      /*
       * If coming from userspace, preserve the user thread pointer and load
@@ -106,19 +102,8 @@ _save_context:
 .option norelax
      la gp, __global_pointer$
 .option pop
-
-#ifdef CONFIG_TRACE_IRQFLAGS
-     call __trace_hardirqs_off
-#endif
-

arch/riscv/kernel/entry.S _save_context: · line 106

Quoted 10 lines
arch/riscv/kernel/entry.S +2 -8
-#ifdef CONFIG_CONTEXT_TRACKING_USER
-     /* If previous state is in user mode, call user_exit_callable(). */
-     li   a0, SR_PP
-     and a0, s1, a0
-     bnez a0, skip_context_tracking
-     call user_exit_callable
-skip_context_tracking:
-#endif
+     move a0, sp /* pt_regs */
+     la ra, ret_from_exception

Not for this series, but at some point it would be nice to get rid of the "move" pseudoinsn in favor of "mv".

[...]

Quoted 38 lines
arch/riscv/kernel/traps.c +14 -4
index f7fa973558bc..ee9a0ef672e9 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,10 +100,19 @@ static void do_trap_error(struct pt_regs *regs, int signo, int code,
 #else
 #define __trap_section noinstr
 #endif
-#define DO_ERROR_INFO(name, signo, code, str)                                \
-asmlinkage __visible __trap_section void name(struct pt_regs *regs)  \
-{                                                                    \
-     do_trap_error(regs, signo, code, regs->epc, "Oops - " str);     \
+#define DO_ERROR_INFO(name, signo, code, str)                                        \
+asmlinkage __visible __trap_section void name(struct pt_regs *regs)          \
+{                                                                            \
+     if (user_mode(regs)) {                                                  \
+             irqentry_enter_from_user_mode(regs);                            \
+             do_trap_error(regs, signo, code, regs->epc, "Oops - " str);     \
+             irqentry_exit_to_user_mode(regs);                               \
+     } else {                                                                \
+             irqentry_state_t irq_state = irqentry_nmi_enter(regs);          \
+             do_trap_error(regs, signo, code, regs->epc, "Oops - " str);     \
+             irqentry_nmi_exit(regs, irq_state);                             \
+     }                                                                       \
+     BUG_ON(!irqs_disabled());                                               \
 }
 
 DO_ERROR_INFO(do_trap_unknown,
@@ -126,18 +136,38 @@ int handle_misaligned_store(struct pt_regs *regs);
 
Quoted 16 lines
Quoted 4 lines
arch/riscv/kernel/traps.c +0 -2
 asmlinkage void __trap_section do_trap_load_misaligned(struct pt_regs *regs)
 {
-     if (!handle_misaligned_load(regs))
-             return;

arch/riscv/kernel/traps.c int handle_misaligned_store(struct pt_regs *regs); · line 146

Quoted 10 lines
arch/riscv/kernel/traps.c +8 -2
-     do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
-                   "Oops - load address misaligned");
+     if (user_mode(regs)) {
+             irqentry_enter_from_user_mode(regs);
+             if (handle_misaligned_load(regs))
+                     do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
+                           "Oops - load address misaligned");
+             irqentry_exit_to_user_mode(regs);
+     } else {
+             irqentry_state_t irq_state = irqentry_nmi_enter(regs);

Please add a newline.

okay

Quoted 24 lines
Quoted 13 lines
arch/riscv/kernel/traps.c +6 -2
+             if (handle_misaligned_load(regs))
+                     do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
+                           "Oops - load address misaligned");
+             irqentry_nmi_exit(regs, irq_state);
+     }
+     BUG_ON(!irqs_disabled());
 }
 
 asmlinkage void __trap_section do_trap_store_misaligned(struct pt_regs *regs)
 {
-     if (!handle_misaligned_store(regs))
-             return;

arch/riscv/kernel/traps.c int handle_misaligned_store(struct pt_regs *regs); · line 164

Quoted 10 lines
arch/riscv/kernel/traps.c +8 -2
-     do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
-                   "Oops - store (or AMO) address misaligned");
+     if (user_mode(regs)) {
+             irqentry_enter_from_user_mode(regs);
+             if (handle_misaligned_store(regs))
+                     do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
+                             "Oops - store (or AMO) address misaligned");
+             irqentry_exit_to_user_mode(regs);
+     } else {
+             irqentry_state_t irq_state = irqentry_nmi_enter(regs);
Quoted 1 line

Please add a newline.

okay

Quoted 26 lines
Quoted 26 lines
arch/riscv/kernel/traps.c +10 -1
+             if (handle_misaligned_store(regs))
+                     do_trap_error(regs, SIGBUS, BUS_ADRALN, regs->epc,
+                             "Oops - store (or AMO) address misaligned");
+             irqentry_nmi_exit(regs, irq_state);
+     }
+     BUG_ON(!irqs_disabled());
 }
 #endif
 DO_ERROR_INFO(do_trap_store_fault,
@@ -159,7 +189,7 @@ static inline unsigned long get_break_insn_length(unsigned long pc)
      return GET_INSN_LENGTH(insn);
 }
 
-asmlinkage __visible __trap_section void do_trap_break(struct pt_regs *regs)
+static void __do_trap_break(struct pt_regs *regs)
 {
 #ifdef CONFIG_KPROBES
      if (kprobe_single_step_handler(regs))
@@ -189,6 +219,20 @@ asmlinkage __visible __trap_section void do_trap_break(struct pt_regs *regs)
      else
              die(regs, "Kernel BUG");
 }
+
+asmlinkage __visible __trap_section void do_trap_break(struct pt_regs *regs)
+{
Quoted 8 lines

arch/riscv/kernel/traps.c asmlinkage __visible __trap_section void do_trap_break(struct pt_regs *regs) · line 230

Quoted 6 lines
arch/riscv/kernel/traps.c +6 -0
+     if (user_mode(regs)) {
+             irqentry_enter_from_user_mode(regs);
+             __do_trap_break(regs);
+             irqentry_exit_to_user_mode(regs);
+     } else {
+             irqentry_state_t irq_state = irqentry_nmi_enter(regs);

Please add a newline.

okay

Quoted 3 lines

Björn

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

Re: [PATCH -next V10 04/10] riscv: entry: Convert to generic entry

Quoted 12 lines · Guo Ren
Quoted 3 lines
Quoted 1 line

- Little modification on ret_from_fork & ret_from_kernel_thread

What changes?

ENTRY(ret_from_fork)

arch/riscv/kernel/entry.S +3 -1
+       call schedule_tail
+       move a0, sp /* pt_regs */
        la ra, ret_from_exception
-       tail schedule_tail
+       tail syscall_exit_to_user_mode
 ENDPROC(ret_from_fork)
 
 ENTRY(ret_from_kernel_thread)

arch/riscv/kernel/entry.S restore_caller_reg: · line 339

Quoted 10 lines
arch/riscv/kernel/entry.S +4 -2
        call schedule_tail
        /* Call fn(arg) */
-       la ra, ret_from_exception
        move a0, s1
-       jr s0
+       jalr s0
+       move a0, sp /* pt_regs */
+       la ra, ret_from_exception
+       tail syscall_exit_to_user_mode
 ENDPROC(ret_from_kernel_thread)

Thanks for clearing that up! It's more useful to have a descriptive text, than just "these functions were changed". (Why instead of what)

Cheers,
Björn

#8 lore raw

PATCH v10 5/10 riscv: entry: Remove extra level wrappers of trace_hardirqs_{on,off}

In reply to [email protected]

From: Jisheng Zhang <[email protected]>

Since riscv is converted to generic entry, there's no need for the extra wrappers of trace_hardirqs_{on,off}.

Tested with llvm + irqsoff.

---
 arch/riscv/kernel/Makefile    |  2 --
 arch/riscv/kernel/trace_irq.c | 27 ---------------------------
 arch/riscv/kernel/trace_irq.h | 11 -----------
 3 files changed, 40 deletions(-)
 delete mode 100644 arch/riscv/kernel/trace_irq.c
 delete mode 100644 arch/riscv/kernel/trace_irq.h
arch/riscv/kernel/Makefile +0 -2
index 4cf303a779ab..392fa6e35d4a 100644
@@ -68,8 +68,6 @@ obj-$(CONFIG_CPU_PM)		+= suspend_entry.o suspend.o
 obj-$(CONFIG_FUNCTION_TRACER)	+= mcount.o ftrace.o
 obj-$(CONFIG_DYNAMIC_FTRACE)	+= mcount-dyn.o
 
-obj-$(CONFIG_TRACE_IRQFLAGS)	+= trace_irq.o
-
 obj-$(CONFIG_PERF_EVENTS)	+= perf_callchain.o
 obj-$(CONFIG_HAVE_PERF_REGS)	+= perf_regs.o
 obj-$(CONFIG_RISCV_SBI)		+= sbi.o
arch/riscv/kernel/trace_irq.c deleted +0 -27
deleted file mode 100644
index 095ac976d7da..000000000000
@@ -1,27 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * Copyright (C) 2022 Changbin Du <[email protected]>
- */
-
-#include <linux/irqflags.h>
-#include <linux/kprobes.h>
-#include "trace_irq.h"
-
-/*
- * trace_hardirqs_on/off require the caller to setup frame pointer properly.
- * Otherwise, CALLER_ADDR1 might trigger an pagging exception in kernel.
- * Here we add one extra level so they can be safely called by low
- * level entry code which $fp is used for other purpose.
- */
-
-void __trace_hardirqs_on(void)
-{
-	trace_hardirqs_on();
-}
-NOKPROBE_SYMBOL(__trace_hardirqs_on);
-
-void __trace_hardirqs_off(void)
-{
-	trace_hardirqs_off();
-}
-NOKPROBE_SYMBOL(__trace_hardirqs_off);
arch/riscv/kernel/trace_irq.h deleted +0 -11
deleted file mode 100644
index 99fe67377e5e..000000000000
@@ -1,11 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/*
- * Copyright (C) 2022 Changbin Du <[email protected]>
- */
-#ifndef __TRACE_IRQ_H
-#define __TRACE_IRQ_H
-
-void __trace_hardirqs_on(void);
-void __trace_hardirqs_off(void);
-
-#endif /* __TRACE_IRQ_H */
Signature · 7 lines
2.36.1


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

Re: [PATCH -next V10 05/10] riscv: entry: Remove extra level wrappers of trace_hardirqs_{on,off}

Quoted 6 lines · [email protected] writes:

From: Jisheng Zhang <[email protected]>

Since riscv is converted to generic entry, there's no need for the extra wrappers of trace_hardirqs_{on,off}.

Tested with llvm + irqsoff.

What does this mean?

Björn

author
#10 lore raw

Re: [PATCH -next V10 05/10] riscv: entry: Remove extra level wrappers of trace_hardirqs_{on,off}

Quoted 11 lines · Björn Töpel
Quoted 6 lines · [email protected] writes:

From: Jisheng Zhang <[email protected]>

Since riscv is converted to generic entry, there's no need for the extra wrappers of trace_hardirqs_{on,off}.

Tested with llvm + irqsoff.

What does this mean?

It's just a tested environment description. This is covered by the generic entry. This patch removes unused code.

I would remove the "Tested with llvm + irqsoff." sentence; it's unnecessary.

Quoted 3 lines

Björn

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

PATCH v10 6/10 riscv: entry: Consolidate ret_from_kernel_thread into ret_from_fork

In reply to [email protected]

From: Jisheng Zhang <[email protected]>

The ret_from_kernel_thread() behaves similarly with ret_from_fork(), the only difference is whether call the fn(arg) or not, this can be achieved by testing fn is NULL or not, I.E s0 is 0 or not. Many architectures have done the same thing, it make entry.S more clean.

---
 arch/riscv/kernel/entry.S   | 12 +++---------
 arch/riscv/kernel/process.c |  5 ++---
 2 files changed, 5 insertions(+), 12 deletions(-)
arch/riscv/kernel/entry.S +3 -9
index 69097dfffdc9..e4a9140a5b99 100644
@@ -132,7 +132,6 @@ END(handle_exception)
  * caller list:
  *  - handle_exception
  *  - ret_from_fork
- *  - ret_from_kernel_thread
  */
 SYM_CODE_START_NOALIGN(ret_from_exception)
 	REG_L s0, PT_STATUS(sp)
@@ -323,20 +322,15 @@ END(handle_kernel_stack_overflow)
 
 ENTRY(ret_from_fork)
 	call schedule_tail
-	move a0, sp /* pt_regs */
-	la ra, ret_from_exception
-	tail syscall_exit_to_user_mode
-ENDPROC(ret_from_fork)
-
-ENTRY(ret_from_kernel_thread)
-	call schedule_tail
+	beqz s0, 1f	/* not from kernel thread */
 	/* Call fn(arg) */
 	move a0, s1
 	jalr s0
+1:
 	move a0, sp /* pt_regs */
 	la ra, ret_from_exception
 	tail syscall_exit_to_user_mode
-ENDPROC(ret_from_kernel_thread)
+ENDPROC(ret_from_fork)
 
 
 /*
arch/riscv/kernel/process.c +2 -3
index b0c63e8e867e..5108c76a14dd 100644
@@ -34,7 +34,6 @@ EXPORT_SYMBOL(__stack_chk_guard);
 #endif
 
 extern asmlinkage void ret_from_fork(void);
-extern asmlinkage void ret_from_kernel_thread(void);
 
 void arch_cpu_idle(void)
 {
@@ -172,7 +171,6 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 		/* Supervisor/Machine, irqs on: */
 		childregs->status = SR_PP | SR_PIE;
 
-		p->thread.ra = (unsigned long)ret_from_kernel_thread;
 		p->thread.s[0] = (unsigned long)args->fn;
 		p->thread.s[1] = (unsigned long)args->fn_arg;
 	} else {
@@ -182,8 +180,9 @@ int copy_thread(struct task_struct *p, const struct kernel_clone_args *args)
 		if (clone_flags & CLONE_SETTLS)
 			childregs->tp = tls;
 		childregs->a0 = 0; /* Return value of fork() */
-		p->thread.ra = (unsigned long)ret_from_fork;
+		p->thread.s[0] = 0;
 	}
+	p->thread.ra = (unsigned long)ret_from_fork;
 	p->thread.sp = (unsigned long)childregs; /* kernel sp */
 	return 0;
 }
Signature · 7 lines
2.36.1


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

Re: [PATCH -next V10 06/10] riscv: entry: Consolidate ret_from_kernel_thread into ret_from_fork

Quoted 6 lines · [email protected] writes:

From: Jisheng Zhang <[email protected]>

The ret_from_kernel_thread() behaves similarly with ret_from_fork(), the only difference is whether call the fn(arg) or not, this can be achieved by testing fn is NULL or not, I.E s0 is 0 or not. Many architectures have done the same thing, it make entry.S more clean.

Nit: "it makes".

Björn

author
#13 lore raw

Re: [PATCH -next V10 06/10] riscv: entry: Consolidate ret_from_kernel_thread into ret_from_fork

Quoted 11 lines · Björn Töpel
Quoted 6 lines · [email protected] writes:

From: Jisheng Zhang <[email protected]>

The ret_from_kernel_thread() behaves similarly with ret_from_fork(), the only difference is whether call the fn(arg) or not, this can be achieved by testing fn is NULL or not, I.E s0 is 0 or not. Many architectures have done the same thing, it make entry.S more clean.

Nit: "it makes".

Okay.

Quoted 3 lines

Björn

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

PATCH v10 7/10 riscv: entry: Consolidate general regs saving/restoring

In reply to [email protected]

From: Jisheng Zhang <[email protected]>

Consolidate the saving/restoring GPs(except zero, ra, sp, gp and tp) into save_from_x5_to_x31/restore_from_x5_to_x31 macros.

No functional change intended.

---
 arch/riscv/include/asm/asm.h   | 63 +++++++++++++++++++++++++
 arch/riscv/kernel/entry.S      | 84 ++--------------------------------
 arch/riscv/kernel/mcount-dyn.S | 56 +----------------------
 3 files changed, 68 insertions(+), 135 deletions(-)
arch/riscv/include/asm/asm.h +63 -0
index 1b471ff73178..bf5247aa317d 100644
@@ -68,6 +68,7 @@
 #endif
 
 #ifdef __ASSEMBLY__
+#include <asm/asm-offsets.h>
 
 /* Common assembly source macros */
 
@@ -80,6 +81,68 @@
 	.endr
 .endm
 
+	/* save all GPs except zero, ra, sp, gp and tp */
+	.macro save_from_x5_to_x31
+	REG_S x5,  PT_T0(sp)
+	REG_S x6,  PT_T1(sp)
+	REG_S x7,  PT_T2(sp)
+	REG_S x8,  PT_S0(sp)
+	REG_S x9,  PT_S1(sp)
+	REG_S x10, PT_A0(sp)
+	REG_S x11, PT_A1(sp)
+	REG_S x12, PT_A2(sp)
+	REG_S x13, PT_A3(sp)
+	REG_S x14, PT_A4(sp)
+	REG_S x15, PT_A5(sp)
+	REG_S x16, PT_A6(sp)
+	REG_S x17, PT_A7(sp)
+	REG_S x18, PT_S2(sp)
+	REG_S x19, PT_S3(sp)
+	REG_S x20, PT_S4(sp)
+	REG_S x21, PT_S5(sp)
+	REG_S x22, PT_S6(sp)
+	REG_S x23, PT_S7(sp)
+	REG_S x24, PT_S8(sp)
+	REG_S x25, PT_S9(sp)
+	REG_S x26, PT_S10(sp)
+	REG_S x27, PT_S11(sp)
+	REG_S x28, PT_T3(sp)
+	REG_S x29, PT_T4(sp)
+	REG_S x30, PT_T5(sp)
+	REG_S x31, PT_T6(sp)
+	.endm
+
+	/* restore all GPs except zero, ra, sp, gp and tp */
+	.macro restore_from_x5_to_x31
+	REG_L x5,  PT_T0(sp)
+	REG_L x6,  PT_T1(sp)
+	REG_L x7,  PT_T2(sp)
+	REG_L x8,  PT_S0(sp)
+	REG_L x9,  PT_S1(sp)
+	REG_L x10, PT_A0(sp)
+	REG_L x11, PT_A1(sp)
+	REG_L x12, PT_A2(sp)
+	REG_L x13, PT_A3(sp)
+	REG_L x14, PT_A4(sp)
+	REG_L x15, PT_A5(sp)
+	REG_L x16, PT_A6(sp)
+	REG_L x17, PT_A7(sp)
+	REG_L x18, PT_S2(sp)
+	REG_L x19, PT_S3(sp)
+	REG_L x20, PT_S4(sp)
+	REG_L x21, PT_S5(sp)
+	REG_L x22, PT_S6(sp)
+	REG_L x23, PT_S7(sp)
+	REG_L x24, PT_S8(sp)
+	REG_L x25, PT_S9(sp)
+	REG_L x26, PT_S10(sp)
+	REG_L x27, PT_S11(sp)
+	REG_L x28, PT_T3(sp)
+	REG_L x29, PT_T4(sp)
+	REG_L x30, PT_T5(sp)
+	REG_L x31, PT_T6(sp)
+	.endm
+
 #endif /* __ASSEMBLY__ */
 
 #endif /* _ASM_RISCV_ASM_H */
arch/riscv/kernel/entry.S +3 -81

102 lines not on this page · raw message

arch/riscv/kernel/mcount-dyn.S +2 -54

68 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
#15 lore raw

PATCH v10 8/10 riscv: stack: 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/thread_info.h |  2 +
 arch/riscv/include/asm/vmap_stack.h  | 28 ++++++++++++
 arch/riscv/kernel/irq.c              | 66 +++++++++++++++++++++++++++-
 4 files changed, 102 insertions(+), 2 deletions(-)
 create mode 100644 arch/riscv/include/asm/vmap_stack.h
arch/riscv/Kconfig +8 -0
index 518e8523d41d..0a9d4bdc0338 100644
@@ -446,6 +446,14 @@ config FPU
 
 	  If you don't know what to do here, say Y.
 
+config IRQ_STACKS
+	bool "Independent irq stacks" if EXPERT
+	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/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/irq.c +64 -2

91 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
#16 lore raw

Re: [PATCH -next V10 08/10] riscv: stack: Support HAVE_IRQ_EXIT_ON_IRQ_STACK

Quoted 7 lines · [email protected] writes:

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.

I still would like to see this as a separate series, and that the generic entry series ended with the previous patch. It's already a lot of moving pieces in this series. Now add stack changes? Is this really required for generic entry support?

Some comments below.

Quoted 17 lines
---
 arch/riscv/Kconfig                   |  8 ++++
 arch/riscv/include/asm/thread_info.h |  2 +
 arch/riscv/include/asm/vmap_stack.h  | 28 ++++++++++++
 arch/riscv/kernel/irq.c              | 66 +++++++++++++++++++++++++++-
 4 files changed, 102 insertions(+), 2 deletions(-)
 create mode 100644 arch/riscv/include/asm/vmap_stack.h
arch/riscv/Kconfig +0 -0
index 518e8523d41d..0a9d4bdc0338 100644
@@ -446,6 +446,14 @@ config FPU
 

arch/riscv/Kconfig config FPU · line 456

Quoted 10 lines
arch/riscv/Kconfig +8 -0
 	  If you don't know what to do here, say Y.
 
+config IRQ_STACKS
+	bool "Independent irq stacks" if EXPERT
+	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.
+

Other archs uses CONFIG_IRQSTACKS. Let's use that for riscv as well, and also use the same Kconfig wording as the other archs.

Quoted 131 lines
arch/riscv/Kconfig +0 -0
 endmenu # "Platform type"
 
 menu "Kernel features"
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/irq.c +51 -2
index 24c2e1bd756a..5d77f692b198 100644
@@ -10,6 +10,37 @@
 #include <linux/irqchip.h>
 #include <linux/seq_file.h>
 #include <asm/smp.h>
+#include <asm/vmap_stack.h>
+
+#ifdef CONFIG_IRQ_STACKS
+static DEFINE_PER_CPU(ulong *, irq_stack_ptr);
+
+#ifdef CONFIG_VMAP_STACK
+static void init_irq_stacks(void)
+{
+	int cpu;
+	ulong *p;
+
+	for_each_possible_cpu(cpu) {
+		p = arch_alloc_vmap_stack(IRQ_STACK_SIZE, cpu_to_node(cpu));
+		per_cpu(irq_stack_ptr, cpu) = p;
+	}
+}
+#else
+/* irq stack only needs to be 16 byte aligned - not IRQ_STACK_SIZE aligned. */
+DEFINE_PER_CPU_ALIGNED(ulong [IRQ_STACK_SIZE/sizeof(ulong)], irq_stack);
+
+static void init_irq_stacks(void)
+{
+	int cpu;
+
+	for_each_possible_cpu(cpu)
+		per_cpu(irq_stack_ptr, cpu) = per_cpu(irq_stack, cpu);
+}
+#endif /* CONFIG_VMAP_STACK */
+#else
+static void init_irq_stacks(void) {}
+#endif /* CONFIG_IRQ_STACKS */
 
 int arch_show_interrupts(struct seq_file *p, int prec)
 {
@@ -19,21 +50,52 @@ int arch_show_interrupts(struct seq_file *p, int prec)
 
 void __init init_IRQ(void)
 {
+	init_irq_stacks();
 	irqchip_init();
 	if (!handle_arch_irq)
 		panic("No interrupt controller found.");
 }
 
-asmlinkage void noinstr do_riscv_irq(struct pt_regs *regs)
+static void noinstr handle_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();
+}
+
+asmlinkage void noinstr do_riscv_irq(struct pt_regs *regs)
+{
+	irqentry_state_t state = irqentry_enter(regs);
+#ifdef CONFIG_IRQ_STACKS
+	if (on_thread_stack()) {
+		ulong *sp = per_cpu(irq_stack_ptr, smp_processor_id())
+					+ IRQ_STACK_SIZE/sizeof(ulong);
+		__asm__ __volatile(
+		"addi	sp, sp, -"RISCV_SZPTR  "\n"
+		REG_S"  ra, (sp)		\n"
+		"addi	sp, sp, -"RISCV_SZPTR  "\n"
+		REG_S"  s0, (sp)		\n"
+		"addi	s0, sp, 2*"RISCV_SZPTR "\n"
+		"move	sp, %[sp]		\n"
+		"move	a0, %[regs]		\n"
+		"call	handle_riscv_irq	\n"

arch/riscv/kernel/irq.c int arch_show_interrupts(struct seq_file *p, int prec) · line 95

Quoted 10 lines
arch/riscv/kernel/irq.c +10 -0
+		"addi	sp, s0, -2*"RISCV_SZPTR"\n"
+		REG_L"  s0, (sp)		\n"
+		"addi	sp, sp, "RISCV_SZPTR   "\n"
+		REG_L"  ra, (sp)		\n"
+		"addi	sp, sp, "RISCV_SZPTR   "\n"
+		:
+		: [sp] "r" (sp), [regs] "r" (regs)
+		: "a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7",
+		  "t0", "t1", "t2", "t3", "t4", "t5", "t6",
+		  "memory");

This whole assembly thing will be C&P in later commits. Can we please do something like x86 does here (call_on_stack -- arch/x86/include/asm/irq_stack.h), which will hurt our eyes a bit less, and make it more maintainable?

Björn

author
#17 lore raw

Re: [PATCH -next V10 08/10] riscv: stack: Support HAVE_IRQ_EXIT_ON_IRQ_STACK

Quoted 11 lines · Björn Töpel
Quoted 7 lines · [email protected] writes:

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.

Quoted 4 lines

I still would like to see this as a separate series, and that the generic entry series ended with the previous patch. It's already a lot of moving pieces in this series. Now add stack changes? Is this really required for generic entry support?

Okay, Let the generic entry merge first.

Quoted 22 lines

Some comments below.

Quoted 19 lines
---
 arch/riscv/Kconfig                   |  8 ++++
 arch/riscv/include/asm/thread_info.h |  2 +
 arch/riscv/include/asm/vmap_stack.h  | 28 ++++++++++++
 arch/riscv/kernel/irq.c              | 66 +++++++++++++++++++++++++++-
 4 files changed, 102 insertions(+), 2 deletions(-)
 create mode 100644 arch/riscv/include/asm/vmap_stack.h
arch/riscv/Kconfig +0 -0
index 518e8523d41d..0a9d4bdc0338 100644
@@ -446,6 +446,14 @@ config FPU
 
        If you don't know what to do here, say Y.
 
Quoted 11 lines

arch/riscv/Kconfig config FPU · line 456

Quoted 8 lines
arch/riscv/Kconfig +8 -0
+config IRQ_STACKS
+     bool "Independent irq stacks" if EXPERT
+     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.
+

Other archs uses CONFIG_IRQSTACKS. Let's use that for riscv as well, and also use the same Kconfig wording as the other archs.

Okay

Quoted 139 lines
Quoted 139 lines
arch/riscv/Kconfig +0 -0
 endmenu # "Platform type"
 
 menu "Kernel features"
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/irq.c +58 -2
index 24c2e1bd756a..5d77f692b198 100644
@@ -10,6 +10,37 @@
 #include <linux/irqchip.h>
 #include <linux/seq_file.h>
 #include <asm/smp.h>
+#include <asm/vmap_stack.h>
+
+#ifdef CONFIG_IRQ_STACKS
+static DEFINE_PER_CPU(ulong *, irq_stack_ptr);
+
+#ifdef CONFIG_VMAP_STACK
+static void init_irq_stacks(void)
+{
+     int cpu;
+     ulong *p;
+
+     for_each_possible_cpu(cpu) {
+             p = arch_alloc_vmap_stack(IRQ_STACK_SIZE, cpu_to_node(cpu));
+             per_cpu(irq_stack_ptr, cpu) = p;
+     }
+}
+#else
+/* irq stack only needs to be 16 byte aligned - not IRQ_STACK_SIZE aligned. */
+DEFINE_PER_CPU_ALIGNED(ulong [IRQ_STACK_SIZE/sizeof(ulong)], irq_stack);
+
+static void init_irq_stacks(void)
+{
+     int cpu;
+
+     for_each_possible_cpu(cpu)
+             per_cpu(irq_stack_ptr, cpu) = per_cpu(irq_stack, cpu);
+}
+#endif /* CONFIG_VMAP_STACK */
+#else
+static void init_irq_stacks(void) {}
+#endif /* CONFIG_IRQ_STACKS */
 
 int arch_show_interrupts(struct seq_file *p, int prec)
 {
@@ -19,21 +50,52 @@ int arch_show_interrupts(struct seq_file *p, int prec)
 
 void __init init_IRQ(void)
 {
+     init_irq_stacks();
      irqchip_init();
      if (!handle_arch_irq)
              panic("No interrupt controller found.");
 }
 
-asmlinkage void noinstr do_riscv_irq(struct pt_regs *regs)
+static void noinstr handle_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();
+}
+
+asmlinkage void noinstr do_riscv_irq(struct pt_regs *regs)
+{
+     irqentry_state_t state = irqentry_enter(regs);
+#ifdef CONFIG_IRQ_STACKS
+     if (on_thread_stack()) {
+             ulong *sp = per_cpu(irq_stack_ptr, smp_processor_id())
+                                     + IRQ_STACK_SIZE/sizeof(ulong);
+             __asm__ __volatile(
+             "addi   sp, sp, -"RISCV_SZPTR  "\n"
+             REG_S"  ra, (sp)                \n"
+             "addi   sp, sp, -"RISCV_SZPTR  "\n"
+             REG_S"  s0, (sp)                \n"
+             "addi   s0, sp, 2*"RISCV_SZPTR "\n"
+             "move   sp, %[sp]               \n"
+             "move   a0, %[regs]             \n"
+             "call   handle_riscv_irq        \n"
+             "addi   sp, s0, -2*"RISCV_SZPTR"\n"
+             REG_L"  s0, (sp)                \n"
+             "addi   sp, sp, "RISCV_SZPTR   "\n"
+             REG_L"  ra, (sp)                \n"
+             "addi   sp, sp, "RISCV_SZPTR   "\n"
+             :
+             : [sp] "r" (sp), [regs] "r" (regs)
Quoted 8 lines

arch/riscv/kernel/irq.c int arch_show_interrupts(struct seq_file *p, int prec) · line 95

Quoted 3 lines
arch/riscv/kernel/irq.c +3 -0
+             : "a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7",
+               "t0", "t1", "t2", "t3", "t4", "t5", "t6",
+               "memory");

This whole assembly thing will be C&P in later commits. Can we please do something like x86 does here (call_on_stack -- arch/x86/include/asm/irq_stack.h), which will hurt our eyes a bit less, and make it more maintainable?

Okay.

Quoted 3 lines

Björn

Signature · 7 lines
Best Regards
 Guo Ren

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

PATCH v10 9/10 riscv: stack: 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 | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 3 deletions(-)
arch/riscv/Kconfig +4 -3
index 0a9d4bdc0338..bd4c4ae4cdc9 100644
@@ -447,12 +447,13 @@ config FPU
 	  If you don't know what to do here, say Y.
 
 config IRQ_STACKS
-	bool "Independent irq stacks" if EXPERT
+	bool "Independent irq & softirq stacks" if EXPERT
 	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 +33 -0
index 5d77f692b198..a6406da34937 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
 static DEFINE_PER_CPU(ulong *, irq_stack_ptr);
@@ -38,6 +39,38 @@ static void init_irq_stacks(void)
 		per_cpu(irq_stack_ptr, cpu) = per_cpu(irq_stack, cpu);
 }
 #endif /* CONFIG_VMAP_STACK */
+
+#ifdef CONFIG_HAVE_SOFTIRQ_ON_OWN_STACK
+void do_softirq_own_stack(void)
+{
+#ifdef CONFIG_IRQ_STACKS
+	if (on_thread_stack()) {
+		ulong *sp = per_cpu(irq_stack_ptr, smp_processor_id())
+					+ IRQ_STACK_SIZE/sizeof(ulong);
+		__asm__ __volatile(
+		"addi	sp, sp, -"RISCV_SZPTR  "\n"
+		REG_S"  ra, (sp)		\n"
+		"addi	sp, sp, -"RISCV_SZPTR  "\n"
+		REG_S"  s0, (sp)		\n"
+		"addi	s0, sp, 2*"RISCV_SZPTR "\n"
+		"move	sp, %[sp]		\n"
+		"call	__do_softirq		\n"
+		"addi	sp, s0, -2*"RISCV_SZPTR"\n"
+		REG_L"  s0, (sp)		\n"
+		"addi	sp, sp, "RISCV_SZPTR   "\n"
+		REG_L"  ra, (sp)		\n"
+		"addi	sp, sp, "RISCV_SZPTR   "\n"
+		:
+		: [sp] "r" (sp)
+		: "a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7",
+		  "t0", "t1", "t2", "t3", "t4", "t5", "t6",
+		  "memory");
+	} else
+#endif
+		__do_softirq();
+}
+#endif /* CONFIG_HAVE_SOFTIRQ_ON_OWN_STACK */
+
 #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
#19 lore raw

Re: [PATCH -next V10 09/10] riscv: stack: Support HAVE_SOFTIRQ_ON_OWN_STACK

Quoted 19 lines · [email protected] writes:

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 | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 3 deletions(-)
arch/riscv/Kconfig +0 -0
index 0a9d4bdc0338..bd4c4ae4cdc9 100644

arch/riscv/Kconfig config FPU · line 453

Quoted 8 lines
arch/riscv/Kconfig +2 -1
@@ -447,12 +447,13 @@ config FPU
 	  If you don't know what to do here, say Y.
 
 config IRQ_STACKS
-	bool "Independent irq stacks" if EXPERT
+	bool "Independent irq & softirq stacks" if EXPERT
 	default y
 	select HAVE_IRQ_EXIT_ON_IRQ_STACK
+	select HAVE_SOFTIRQ_ON_OWN_STACK

HAVE_IRQ_EXIT_ON_IRQ_STACK is used by softirq.c Shouldn't that be selected introduced in this patch, instead of the previous one?

arch/riscv/Kconfig config FPU · line 456

Quoted 5 lines
arch/riscv/Kconfig +2 -2
 	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.

Same comment from previous patch. Please use the same wording/config as other archs.

Quoted 34 lines
arch/riscv/Kconfig +0 -0
 endmenu # "Platform type"
 
arch/riscv/kernel/irq.c +17 -0
index 5d77f692b198..a6406da34937 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
 static DEFINE_PER_CPU(ulong *, irq_stack_ptr);
@@ -38,6 +39,38 @@ static void init_irq_stacks(void)
 		per_cpu(irq_stack_ptr, cpu) = per_cpu(irq_stack, cpu);
 }
 #endif /* CONFIG_VMAP_STACK */
+
+#ifdef CONFIG_HAVE_SOFTIRQ_ON_OWN_STACK
+void do_softirq_own_stack(void)
+{
+#ifdef CONFIG_IRQ_STACKS
+	if (on_thread_stack()) {
+		ulong *sp = per_cpu(irq_stack_ptr, smp_processor_id())
+					+ IRQ_STACK_SIZE/sizeof(ulong);
+		__asm__ __volatile(
+		"addi	sp, sp, -"RISCV_SZPTR  "\n"
+		REG_S"  ra, (sp)		\n"
+		"addi	sp, sp, -"RISCV_SZPTR  "\n"
+		REG_S"  s0, (sp)		\n"
+		"addi	s0, sp, 2*"RISCV_SZPTR "\n"
+		"move	sp, %[sp]		\n"
+		"call	__do_softirq		\n"

arch/riscv/kernel/irq.c static void init_irq_stacks(void) · line 67

Quoted 10 lines
arch/riscv/kernel/irq.c +10 -0
+		"addi	sp, s0, -2*"RISCV_SZPTR"\n"
+		REG_L"  s0, (sp)		\n"
+		"addi	sp, sp, "RISCV_SZPTR   "\n"
+		REG_L"  ra, (sp)		\n"
+		"addi	sp, sp, "RISCV_SZPTR   "\n"
+		:
+		: [sp] "r" (sp)
+		: "a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7",
+		  "t0", "t1", "t2", "t3", "t4", "t5", "t6",
+		  "memory");

Same as previous patch. Please avoid C&P and have a look at how call_on_stack is done on x86.

Björn

author
#20 lore raw

Re: [PATCH -next V10 09/10] riscv: stack: Support HAVE_SOFTIRQ_ON_OWN_STACK

Quoted 24 lines · Björn Töpel
Quoted 21 lines · [email protected] writes:

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 | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 3 deletions(-)
arch/riscv/Kconfig +0 -0
index 0a9d4bdc0338..bd4c4ae4cdc9 100644
@@ -447,12 +447,13 @@ config FPU
        If you don't know what to do here, say Y.
 
Quoted 9 lines

arch/riscv/Kconfig config FPU · line 453

Quoted 6 lines
arch/riscv/Kconfig +2 -1
 config IRQ_STACKS
-     bool "Independent irq stacks" if EXPERT
+     bool "Independent irq & softirq stacks" if EXPERT
      default y
      select HAVE_IRQ_EXIT_ON_IRQ_STACK
+     select HAVE_SOFTIRQ_ON_OWN_STACK

HAVE_IRQ_EXIT_ON_IRQ_STACK is used by softirq.c Shouldn't that be selected introduced in this patch, instead of the previous one?

This patch depends on the previous one. And the previous one could work separately.

Quoted 49 lines

arch/riscv/Kconfig config FPU · line 456

Quoted 6 lines
arch/riscv/Kconfig +2 -2
      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.

Same comment from previous patch. Please use the same wording/config as other archs.

Quoted 39 lines
arch/riscv/Kconfig +0 -0
 endmenu # "Platform type"
 
arch/riscv/kernel/irq.c +22 -0
index 5d77f692b198..a6406da34937 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
 static DEFINE_PER_CPU(ulong *, irq_stack_ptr);
@@ -38,6 +39,38 @@ static void init_irq_stacks(void)
              per_cpu(irq_stack_ptr, cpu) = per_cpu(irq_stack, cpu);
 }
 #endif /* CONFIG_VMAP_STACK */
+
+#ifdef CONFIG_HAVE_SOFTIRQ_ON_OWN_STACK
+void do_softirq_own_stack(void)
+{
+#ifdef CONFIG_IRQ_STACKS
+     if (on_thread_stack()) {
+             ulong *sp = per_cpu(irq_stack_ptr, smp_processor_id())
+                                     + IRQ_STACK_SIZE/sizeof(ulong);
+             __asm__ __volatile(
+             "addi   sp, sp, -"RISCV_SZPTR  "\n"
+             REG_S"  ra, (sp)                \n"
+             "addi   sp, sp, -"RISCV_SZPTR  "\n"
+             REG_S"  s0, (sp)                \n"
+             "addi   s0, sp, 2*"RISCV_SZPTR "\n"
+             "move   sp, %[sp]               \n"
+             "call   __do_softirq            \n"
+             "addi   sp, s0, -2*"RISCV_SZPTR"\n"
+             REG_L"  s0, (sp)                \n"
+             "addi   sp, sp, "RISCV_SZPTR   "\n"
+             REG_L"  ra, (sp)                \n"
+             "addi   sp, sp, "RISCV_SZPTR   "\n"
Quoted 8 lines

arch/riscv/kernel/irq.c static void init_irq_stacks(void) · line 67

Quoted 5 lines
arch/riscv/kernel/irq.c +5 -0
+             :
+             : [sp] "r" (sp)
+             : "a0", "a1", "a2", "a3", "a4", "a5", "a6", "a7",
+               "t0", "t1", "t2", "t3", "t4", "t5", "t6",
+               "memory");

Same as previous patch. Please avoid C&P and have a look at how call_on_stack is done on x86.

Okay.

Quoted 3 lines

Björn

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

Re: [PATCH -next V10 09/10] riscv: stack: Support HAVE_SOFTIRQ_ON_OWN_STACK

Quoted 34 lines · Guo Ren
Quoted 24 lines · Björn Töpel
Quoted 21 lines · [email protected] writes:

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 | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 3 deletions(-)
arch/riscv/Kconfig +0 -0
index 0a9d4bdc0338..bd4c4ae4cdc9 100644
@@ -447,12 +447,13 @@ config FPU
        If you don't know what to do here, say Y.
 
Quoted 9 lines

arch/riscv/Kconfig config FPU · line 453

Quoted 6 lines
arch/riscv/Kconfig +2 -1
 config IRQ_STACKS
-     bool "Independent irq stacks" if EXPERT
+     bool "Independent irq & softirq stacks" if EXPERT
      default y
      select HAVE_IRQ_EXIT_ON_IRQ_STACK
+     select HAVE_SOFTIRQ_ON_OWN_STACK

HAVE_IRQ_EXIT_ON_IRQ_STACK is used by softirq.c Shouldn't that be selected introduced in this patch, instead of the previous one?

Quoted 2 lines

This patch depends on the previous one. And the previous one could work separately.

Let me try to be more clear: IRQ_STACKS should be introduced in the previous patch, which adds per-cpu stacks to hardirq. This patch adds per-cpu stacks for softirq, and the softirq related selects:

select HAVE_IRQ_EXIT_ON_IRQ_STACK
select HAVE_SOFTIRQ_ON_OWN_STACK

Hence, the "HAVE_IRQ_EXIT_ON_IRQ_STACK" select should be part of *this* patch, not the previous.

Or am I missing something?

Björn

author
#22 lore raw

Re: [PATCH -next V10 09/10] riscv: stack: Support HAVE_SOFTIRQ_ON_OWN_STACK

Quoted 44 lines · Björn Töpel
Quoted 34 lines · Guo Ren
Quoted 24 lines · Björn Töpel
Quoted 21 lines · [email protected] writes:

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 | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+), 3 deletions(-)
arch/riscv/Kconfig +0 -0
index 0a9d4bdc0338..bd4c4ae4cdc9 100644
@@ -447,12 +447,13 @@ config FPU
        If you don't know what to do here, say Y.
 
Quoted 9 lines

arch/riscv/Kconfig config FPU · line 453

Quoted 6 lines
arch/riscv/Kconfig +2 -1
 config IRQ_STACKS
-     bool "Independent irq stacks" if EXPERT
+     bool "Independent irq & softirq stacks" if EXPERT
      default y
      select HAVE_IRQ_EXIT_ON_IRQ_STACK
+     select HAVE_SOFTIRQ_ON_OWN_STACK

HAVE_IRQ_EXIT_ON_IRQ_STACK is used by softirq.c Shouldn't that be selected introduced in this patch, instead of the previous one?

Quoted 2 lines

This patch depends on the previous one. And the previous one could work separately.

Let me try to be more clear: IRQ_STACKS should be introduced in the previous patch, which adds per-cpu stacks to hardirq. This patch adds per-cpu stacks for softirq, and the softirq related selects:

Quoted 7 lines

select HAVE_IRQ_EXIT_ON_IRQ_STACK
select HAVE_SOFTIRQ_ON_OWN_STACK

Hence, the "HAVE_IRQ_EXIT_ON_IRQ_STACK" select should be part of *this* patch, not the previous.

Or am I missing something?

You are right, HAVE_IRQ_EXIT_ON_IRQ_STACK is belong to SOFTIRQ:
static inline void invoke_softirq(void)
{
...

        if (!force_irqthreads() || !__this_cpu_read(ksoftirqd)) {
#ifdef CONFIG_HAVE_IRQ_EXIT_ON_IRQ_STACK
...
                __do_softirq();
#else
...
                do_softirq_own_stack();

#endif
...
}

I would fix that in the next version.

Quoted 3 lines

Björn

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

PATCH v10 10/10 riscv: stack: Add config of thread stack size

In reply to [email protected]

From: Guo Ren <[email protected]>

0cac21b02ba5 ("risc v: use 16KB kernel stack on 64-bit") increase the thread size mandatory, but some scenarios, such as D1 with a small memory footprint, would suffer from that. After independent irq stack support, let's give users a choice to determine their custom stack size.

---
 arch/riscv/Kconfig                   | 10 ++++++++++
 arch/riscv/include/asm/thread_info.h | 12 +-----------
 2 files changed, 11 insertions(+), 11 deletions(-)
arch/riscv/Kconfig +10 -0
index bd4c4ae4cdc9..60202cd5c5ae 100644
@@ -455,6 +455,16 @@ config IRQ_STACKS
 	  Add independent irq & softirq stacks for percpu to prevent kernel stack
 	  overflows. We may save some memory footprint by disabling IRQ_STACKS.
 
+config THREAD_SIZE_ORDER
+	int "Kernel stack size (in power-of-two numbers of page size)" if VMAP_STACK && EXPERT
+	range 0 4
+	default 1 if 32BIT && !KASAN
+	default 3 if 64BIT && KASAN
+	default 2
+	help
+	  Specify the Pages of thread stack size (from 4KB to 64KB), which also
+	  affects irq stack size, which is equal to thread stack size.
+
 endmenu # "Platform type"
 
 menu "Kernel features"
arch/riscv/include/asm/thread_info.h +1 -11
index 043da8ccc7e6..c970d41dc4c6 100644
@@ -11,18 +11,8 @@
 #include <asm/page.h>
 #include <linux/const.h>
 
-#ifdef CONFIG_KASAN
-#define KASAN_STACK_ORDER 1
-#else
-#define KASAN_STACK_ORDER 0
-#endif
-
 /* thread information allocation */
-#ifdef CONFIG_64BIT
-#define THREAD_SIZE_ORDER	(2 + KASAN_STACK_ORDER)
-#else
-#define THREAD_SIZE_ORDER	(1 + KASAN_STACK_ORDER)
-#endif
+#define THREAD_SIZE_ORDER	CONFIG_THREAD_SIZE_ORDER
 #define THREAD_SIZE		(PAGE_SIZE << THREAD_SIZE_ORDER)
 
 /*
Signature · 7 lines
2.36.1


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

Re: [PATCH -next V10 10/10] riscv: stack: Add config of thread stack size

Quoted 3 lines · [email protected] writes:

From: Guo Ren <[email protected]>

0cac21b02ba5 ("risc v: use 16KB kernel stack on 64-bit") increase the

checkpatch complains here: Use "commit SHA...".

Quoted 3 lines

thread size mandatory, but some scenarios, such as D1 with a small memory footprint, would suffer from that. After independent irq stack support, let's give users a choice to determine their custom stack size.

...and again, my "why is this in the generic entry" series rant. :-)

Björn

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

author
#25 lore raw

Re: [PATCH -next V10 10/10] riscv: stack: Add config of thread stack size

Quoted 8 lines · Björn Töpel
Quoted 3 lines · [email protected] writes:

From: Guo Ren <[email protected]>

0cac21b02ba5 ("risc v: use 16KB kernel stack on 64-bit") increase the

checkpatch complains here: Use "commit SHA...".

Okay, I would check that.

Quoted 6 lines
Quoted 4 lines

thread size mandatory, but some scenarios, such as D1 with a small memory footprint, would suffer from that. After independent irq stack support, let's give users a choice to determine their custom stack size.

...and again, my "why is this in the generic entry" series rant. :-)

I would remove it from the generic entry series.

Quoted 3 lines

Björn

Signature · 2 lines
Best Regards
 Guo Ren