2022-09-04 07:28:59

by Guo Ren

[permalink] [raw]
Subject: [PATCH V2 0/6] 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
Reported-by: kernel test robot <[email protected]>
- 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


2022-09-04 07:29:33

by Guo Ren

[permalink] [raw]
Subject: [PATCH V2 4/6] riscv: Support HAVE_SOFTIRQ_ON_OWN_STACK

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.

Signed-off-by: Guo Ren <[email protected]>
Signed-off-by: Guo Ren <[email protected]>
---
arch/riscv/Kconfig | 7 ++++---
arch/riscv/kernel/irq.c | 16 ++++++++++++++++
2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index a8a12b4ba1a9..da548ed7d107 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -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"

diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
index 3ac16391ae2e..e2196b22f6f4 100644
--- a/arch/riscv/kernel/irq.c
+++ b/arch/riscv/kernel/irq.c
@@ -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 */
--
2.36.1

2022-09-04 07:41:09

by Guo Ren

[permalink] [raw]
Subject: [PATCH V2 6/6] riscv: compat_syscall_table: Fixup compile warning

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)

Signed-off-by: Guo Ren <[email protected]>
Signed-off-by: Guo Ren <[email protected]>
Reported-by: kernel test robot <[email protected]>
---
arch/riscv/kernel/Makefile | 1 +
1 file changed, 1 insertion(+)

diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
index 33bb60a354cd..01da14e21019 100644
--- a/arch/riscv/kernel/Makefile
+++ b/arch/riscv/kernel/Makefile
@@ -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

2022-09-04 07:48:44

by Guo Ren

[permalink] [raw]
Subject: [PATCH V2 3/6] riscv: Support HAVE_IRQ_EXIT_ON_IRQ_STACK

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.

Signed-off-by: Guo Ren <[email protected]>
Signed-off-by: Guo Ren <[email protected]>
---
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

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index a07bb3b73b5b..a8a12b4ba1a9 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -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"
diff --git a/arch/riscv/include/asm/irq.h b/arch/riscv/include/asm/irq.h
index e4c435509983..205e1c693dfd 100644
--- a/arch/riscv/include/asm/irq.h
+++ b/arch/riscv/include/asm/irq.h
@@ -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 */
diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
index 7de4fb96f0b5..043da8ccc7e6 100644
--- a/arch/riscv/include/asm/thread_info.h
+++ b/arch/riscv/include/asm/thread_info.h
@@ -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)];
diff --git a/arch/riscv/include/asm/vmap_stack.h b/arch/riscv/include/asm/vmap_stack.h
new file mode 100644
index 000000000000..3fbf481abf4f
--- /dev/null
+++ b/arch/riscv/include/asm/vmap_stack.h
@@ -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 */
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index 5f49517cd3a2..551ce4c25698 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -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
diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
index 24c2e1bd756a..3ac16391ae2e 100644
--- a/arch/riscv/kernel/irq.c
+++ b/arch/riscv/kernel/irq.c
@@ -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
+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,34 @@ 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
+ ulong *sp = per_cpu(irq_stack_ptr, smp_processor_id());
+
+ if (on_thread_stack())
+ call_on_stack(regs, sp, handle_riscv_irq, 0);
+ else
+#endif
+ handle_riscv_irq(regs);

irqentry_exit(regs, state);
}
--
2.36.1

2022-09-04 07:52:59

by Guo Ren

[permalink] [raw]
Subject: [PATCH V2 2/6] riscv: convert to generic entry

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

Signed-off-by: Guo Ren <[email protected]>
Signed-off-by: Guo Ren <[email protected]>
Suggested-by: Huacai Chen <[email protected]>
---
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

diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index ed66c31e4655..a07bb3b73b5b 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -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
diff --git a/arch/riscv/include/asm/csr.h b/arch/riscv/include/asm/csr.h
index 0e571f6483d9..7c2b8cdb7b77 100644
--- a/arch/riscv/include/asm/csr.h
+++ b/arch/riscv/include/asm/csr.h
@@ -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 */
diff --git a/arch/riscv/include/asm/entry-common.h b/arch/riscv/include/asm/entry-common.h
new file mode 100644
index 000000000000..1636ac2af28e
--- /dev/null
+++ b/arch/riscv/include/asm/entry-common.h
@@ -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 */
diff --git a/arch/riscv/include/asm/ptrace.h b/arch/riscv/include/asm/ptrace.h
index 6ecd461129d2..4e46a611f255 100644
--- a/arch/riscv/include/asm/ptrace.h
+++ b/arch/riscv/include/asm/ptrace.h
@@ -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 */
diff --git a/arch/riscv/include/asm/stacktrace.h b/arch/riscv/include/asm/stacktrace.h
index 3450c1912afd..f7e8ef2418b9 100644
--- a/arch/riscv/include/asm/stacktrace.h
+++ b/arch/riscv/include/asm/stacktrace.h
@@ -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 */
diff --git a/arch/riscv/include/asm/syscall.h b/arch/riscv/include/asm/syscall.h
index 384a63b86420..6c573f18030b 100644
--- a/arch/riscv/include/asm/syscall.h
+++ b/arch/riscv/include/asm/syscall.h
@@ -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 */
diff --git a/arch/riscv/include/asm/thread_info.h b/arch/riscv/include/asm/thread_info.h
index 67322f878e0d..7de4fb96f0b5 100644
--- a/arch/riscv/include/asm/thread_info.h
+++ b/arch/riscv/include/asm/thread_info.h
@@ -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 */
diff --git a/arch/riscv/kernel/entry.S b/arch/riscv/kernel/entry.S
index b9eda3fcbd6d..5f49517cd3a2 100644
--- a/arch/riscv/kernel/entry.S
+++ b/arch/riscv/kernel/entry.S
@@ -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
-
-#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

/*
* MSB of cause differentiates between
@@ -126,134 +111,26 @@ skip_context_tracking:
*/
bge s4, zero, 1f

- la ra, ret_from_exception
-
/* Handle interrupts */
- move a0, sp /* pt_regs */
- la a1, generic_handle_arch_irq
- jr a1
+ tail do_riscv_irq
1:
- /*
- * Exceptions run with interrupts enabled or disabled depending on the
- * state of SR_PIE in m/sstatus.
- */
- andi t0, s1, SR_PIE
- beqz t0, 1f
- /* kprobes, entered via ebreak, must have interrupts disabled. */
- li t0, EXC_BREAKPOINT
- beq s4, t0, 1f
-#ifdef CONFIG_TRACE_IRQFLAGS
- call __trace_hardirqs_on
-#endif
- csrs CSR_STATUS, SR_IE
-
-1:
- la ra, ret_from_exception
- /* Handle syscalls */
- li t0, EXC_SYSCALL
- beq s4, t0, handle_syscall
-
/* Handle other exceptions */
slli t0, s4, RISCV_LGPTR
la t1, excp_vect_table
la t2, excp_vect_table_end
- move a0, sp /* pt_regs */
add t0, t1, t0
/* Check if exception code lies within bounds */
- bgeu t0, t2, 1f
+ bgeu t0, t2, 2f
REG_L t0, 0(t0)
jr t0
-1:
+2:
tail do_trap_unknown
+END(handle_exception)

-handle_syscall:
-#ifdef CONFIG_RISCV_M_MODE
- /*
- * When running is M-Mode (no MMU config), MPIE does not get set.
- * As a result, we need to force enable interrupts here because
- * handle_exception did not do set SR_IE as it always sees SR_PIE
- * being cleared.
- */
- csrs CSR_STATUS, SR_IE
-#endif
-#if defined(CONFIG_TRACE_IRQFLAGS) || defined(CONFIG_CONTEXT_TRACKING_USER)
- /* Recover a0 - a7 for system calls */
- REG_L a0, PT_A0(sp)
- REG_L a1, PT_A1(sp)
- REG_L a2, PT_A2(sp)
- REG_L a3, PT_A3(sp)
- REG_L a4, PT_A4(sp)
- REG_L a5, PT_A5(sp)
- REG_L a6, PT_A6(sp)
- REG_L a7, PT_A7(sp)
-#endif
- /* save the initial A0 value (needed in signal handlers) */
- REG_S a0, PT_ORIG_A0(sp)
- /*
- * Advance SEPC to avoid executing the original
- * scall instruction on sret
- */
- addi s2, s2, 0x4
- REG_S s2, PT_EPC(sp)
- /* Trace syscalls, but only if requested by the user. */
- REG_L t0, TASK_TI_FLAGS(tp)
- andi t0, t0, _TIF_SYSCALL_WORK
- bnez t0, handle_syscall_trace_enter
-check_syscall_nr:
- /* Check to make sure we don't jump to a bogus syscall number. */
- li t0, __NR_syscalls
- la s0, sys_ni_syscall
- /*
- * Syscall number held in a7.
- * If syscall number is above allowed value, redirect to ni_syscall.
- */
- bgeu a7, t0, 3f
-#ifdef CONFIG_COMPAT
+ENTRY(ret_from_exception)
REG_L s0, PT_STATUS(sp)
- srli s0, s0, SR_UXL_SHIFT
- andi s0, s0, (SR_UXL >> SR_UXL_SHIFT)
- li t0, (SR_UXL_32 >> SR_UXL_SHIFT)
- sub t0, s0, t0
- bnez t0, 1f
-
- /* Call compat_syscall */
- la s0, compat_sys_call_table
- j 2f
-1:
-#endif
- /* Call syscall */
- la s0, sys_call_table
-2:
- slli t0, a7, RISCV_LGPTR
- add s0, s0, t0
- REG_L s0, 0(s0)
-3:
- jalr s0

-ret_from_syscall:
- /* Set user a0 to kernel a0 */
- REG_S a0, PT_A0(sp)
- /*
- * We didn't execute the actual syscall.
- * Seccomp already set return value for the current task pt_regs.
- * (If it was configured with SECCOMP_RET_ERRNO/TRACE)
- */
-ret_from_syscall_rejected:
-#ifdef CONFIG_DEBUG_RSEQ
- move a0, sp
- call rseq_syscall
-#endif
- /* Trace syscalls, but only if requested by the user. */
- REG_L t0, TASK_TI_FLAGS(tp)
- andi t0, t0, _TIF_SYSCALL_WORK
- bnez t0, handle_syscall_trace_exit
-
-ret_from_exception:
- REG_L s0, PT_STATUS(sp)
csrc CSR_STATUS, SR_IE
-#ifdef CONFIG_TRACE_IRQFLAGS
- call __trace_hardirqs_off
-#endif
#ifdef CONFIG_RISCV_M_MODE
/* the MPP value is too large to be used as an immediate arg for addi */
li t0, SR_MPP
@@ -261,17 +138,7 @@ ret_from_exception:
#else
andi s0, s0, SR_SPP
#endif
- bnez s0, resume_kernel
-
-resume_userspace:
- /* Interrupts must be disabled here so flags are checked atomically */
- REG_L s0, TASK_TI_FLAGS(tp) /* current_thread_info->flags */
- andi s1, s0, _TIF_WORK_MASK
- bnez s1, work_pending
-
-#ifdef CONFIG_CONTEXT_TRACKING_USER
- call user_enter_callable
-#endif
+ bnez s0, 1f

/* Save unwound kernel stack pointer in thread_info */
addi s0, sp, PT_SIZE_ON_STACK
@@ -282,19 +149,7 @@ resume_userspace:
* structures again.
*/
csrw CSR_SCRATCH, tp
-
-restore_all:
-#ifdef CONFIG_TRACE_IRQFLAGS
- REG_L s1, PT_STATUS(sp)
- andi t0, s1, SR_PIE
- beqz t0, 1f
- call __trace_hardirqs_on
- j 2f
1:
- call __trace_hardirqs_off
-2:
-#endif
- REG_L a0, PT_STATUS(sp)
/*
* The current load reservation is effectively part of the processor's
* state, in the sense that load reservations cannot be shared between
@@ -315,9 +170,11 @@ restore_all:
REG_L a2, PT_EPC(sp)
REG_SC x0, a2, PT_EPC(sp)

- csrw CSR_STATUS, a0
csrw CSR_EPC, a2

+ REG_L a0, PT_STATUS(sp)
+ csrw CSR_STATUS, a0
+
REG_L x1, PT_RA(sp)
REG_L x3, PT_GP(sp)
REG_L x4, PT_TP(sp)
@@ -356,54 +213,10 @@ restore_all:
#else
sret
#endif
-
-#if IS_ENABLED(CONFIG_PREEMPTION)
-resume_kernel:
- REG_L s0, TASK_TI_PREEMPT_COUNT(tp)
- bnez s0, restore_all
- REG_L s0, TASK_TI_FLAGS(tp)
- andi s0, s0, _TIF_NEED_RESCHED
- beqz s0, restore_all
- call preempt_schedule_irq
- j restore_all
-#endif
-
-work_pending:
- /* Enter slow path for supplementary processing */
- la ra, ret_from_exception
- andi s1, s0, _TIF_NEED_RESCHED
- bnez s1, work_resched
-work_notifysig:
- /* Handle pending signals and notify-resume requests */
- csrs CSR_STATUS, SR_IE /* Enable interrupts for do_notify_resume() */
- move a0, sp /* pt_regs */
- move a1, s0 /* current_thread_info->flags */
- tail do_notify_resume
-work_resched:
- tail schedule
-
-/* Slow paths for ptrace. */
-handle_syscall_trace_enter:
- move a0, sp
- call do_syscall_trace_enter
- move t0, a0
- REG_L a0, PT_A0(sp)
- REG_L a1, PT_A1(sp)
- REG_L a2, PT_A2(sp)
- REG_L a3, PT_A3(sp)
- REG_L a4, PT_A4(sp)
- REG_L a5, PT_A5(sp)
- REG_L a6, PT_A6(sp)
- REG_L a7, PT_A7(sp)
- bnez t0, ret_from_syscall_rejected
- j check_syscall_nr
-handle_syscall_trace_exit:
- move a0, sp
- call do_syscall_trace_exit
- j ret_from_exception
+END(ret_from_exception)

#ifdef CONFIG_VMAP_STACK
-handle_kernel_stack_overflow:
+ENTRY(handle_kernel_stack_overflow)
la sp, shadow_stack
addi sp, sp, SHADOW_OVERFLOW_STACK_SIZE

@@ -499,21 +312,24 @@ restore_caller_reg:
REG_S s5, PT_TP(sp)
move a0, sp
tail handle_bad_stack
+END(handle_kernel_stack_overflow)
#endif

-END(handle_exception)
-
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)


@@ -582,7 +398,7 @@ ENTRY(excp_vect_table)
RISCV_PTR do_trap_load_fault
RISCV_PTR do_trap_store_misaligned
RISCV_PTR do_trap_store_fault
- RISCV_PTR do_trap_ecall_u /* system call, gets intercepted */
+ RISCV_PTR do_sys_ecall_u /* system call */
RISCV_PTR do_trap_ecall_s
RISCV_PTR do_trap_unknown
RISCV_PTR do_trap_ecall_m
diff --git a/arch/riscv/kernel/irq.c b/arch/riscv/kernel/irq.c
index 7207fa08d78f..24c2e1bd756a 100644
--- a/arch/riscv/kernel/irq.c
+++ b/arch/riscv/kernel/irq.c
@@ -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);
+}
diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
index 44f4b1ca315d..4caed6c683e4 100644
--- a/arch/riscv/kernel/ptrace.c
+++ b/arch/riscv/kernel/ptrace.c
@@ -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,
diff --git a/arch/riscv/kernel/signal.c b/arch/riscv/kernel/signal.c
index 5a2de6b6f882..5871eccbbd94 100644
--- a/arch/riscv/kernel/signal.c
+++ b/arch/riscv/kernel/signal.c
@@ -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);
-}
diff --git a/arch/riscv/kernel/sys_riscv.c b/arch/riscv/kernel/sys_riscv.c
index 571556bb9261..1194a1b960ef 100644
--- a/arch/riscv/kernel/sys_riscv.c
+++ b/arch/riscv/kernel/sys_riscv.c
@@ -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);
+}
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c
index 635e6ec26938..751745de10c5 100644
--- a/arch/riscv/kernel/traps.c
+++ b/arch/riscv/kernel/traps.c
@@ -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);

diff --git a/arch/riscv/mm/fault.c b/arch/riscv/mm/fault.c
index f2fbd1400b7c..45cd200114b1 100644
--- a/arch/riscv/mm/fault.c
+++ b/arch/riscv/mm/fault.c
@@ -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);
--
2.36.1

2022-09-04 07:54:32

by Guo Ren

[permalink] [raw]
Subject: [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);

Signed-off-by: Guo Ren <[email protected]>
Signed-off-by: Guo Ren <[email protected]>
Reviewed-by: Oleg Nesterov <[email protected]>
---
arch/riscv/kernel/ptrace.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/arch/riscv/kernel/ptrace.c b/arch/riscv/kernel/ptrace.c
index 2ae8280ae475..44f4b1ca315d 100644
--- a/arch/riscv/kernel/ptrace.c
+++ b/arch/riscv/kernel/ptrace.c
@@ -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,
--
2.36.1

2022-09-04 07:54:37

by Guo Ren

[permalink] [raw]
Subject: [PATCH V2 5/6] riscv: elf_kexec: Fixup compile warning

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;
| ^~~~~~~~~~~~

Signed-off-by: Guo Ren <[email protected]>
Signed-off-by: Guo Ren <[email protected]>
Reported-by: kernel test robot <[email protected]>
---
arch/riscv/kernel/elf_kexec.c | 4 ++++
1 file changed, 4 insertions(+)

diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
index 0cb94992c15b..bba3723a0914 100644
--- a/arch/riscv/kernel/elf_kexec.c
+++ b/arch/riscv/kernel/elf_kexec.c
@@ -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 */
--
2.36.1

2022-09-04 10:10:33

by Conor Dooley

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

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)
>
> Signed-off-by: Guo Ren <[email protected]>
> Signed-off-by: Guo Ren <[email protected]>
> Reported-by: kernel test robot <[email protected]>

I **love** this patch.. I really wanted to get rid of these warnings
since they seemed to be false positives..
Reviewed-by: Conor Dooley <[email protected]>

> ---
> arch/riscv/kernel/Makefile | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile
> index 33bb60a354cd..01da14e21019 100644
> --- a/arch/riscv/kernel/Makefile
> +++ b/arch/riscv/kernel/Makefile
> @@ -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

2022-09-04 10:49:31

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH V2 0/6] riscv: Add GENERIC_ENTRY, IRQ_STACKS support

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:
>
> 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].
> >
> > [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"

> Is the above series no longer required & this is a different approach?
Above series cleanup ptrace_disable, we still need that.

> Thanks,
> Conor.
>
> > - Fixup compile warning
> > Reported-by: kernel test robot <[email protected]>
> > - 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
> >
>


--
Best Regards
Guo Ren

2022-09-04 10:59:42

by Conor Dooley

[permalink] [raw]
Subject: Re: [PATCH V2 0/6] riscv: Add GENERIC_ENTRY, IRQ_STACKS support

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:

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].
>
> [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.

> - Fixup compile warning
> Reported-by: kernel test robot <[email protected]>
> - 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
>

2022-09-04 11:23:39

by Guo Ren

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

On Sun, Sep 4, 2022 at 6:36 PM <[email protected]> wrote:
>
> 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]
> > 185 | unsigned long kernel_start;
> > | ^~~~~~~~~~~~
> >
> > Signed-off-by: Guo Ren <[email protected]>
> > Signed-off-by: Guo Ren <[email protected]>
> > Reported-by: kernel test robot <[email protected]>
>
> 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?
>
> > ---
> > arch/riscv/kernel/elf_kexec.c | 4 ++++
> > 1 file changed, 4 insertions(+)
> >
> > diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
> > index 0cb94992c15b..bba3723a0914 100644
> > --- a/arch/riscv/kernel/elf_kexec.c
> > +++ b/arch/riscv/kernel/elf_kexec.c
> > @@ -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 */
>
> 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:

diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
index 0cb94992c15b..4b9264340b78 100644
--- a/arch/riscv/kernel/elf_kexec.c
+++ b/arch/riscv/kernel/elf_kexec.c
@@ -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,


>
> -- >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.
>
> Signed-off-by: Conor Dooley <[email protected]>
> ---
> arch/riscv/kernel/elf_kexec.c | 28 ++++++++++++++--------------
> 1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
> index 0cb94992c15b..29cbf655c474 100644
> --- a/arch/riscv/kernel/elf_kexec.c
> +++ b/arch/riscv/kernel/elf_kexec.c
> @@ -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
>


--
Best Regards
Guo Ren

2022-09-04 11:30:27

by Conor Dooley

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

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]
> 185 | unsigned long kernel_start;
> | ^~~~~~~~~~~~
>
> Signed-off-by: Guo Ren <[email protected]>
> Signed-off-by: Guo Ren <[email protected]>
> Reported-by: kernel test robot <[email protected]>

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?

> ---
> arch/riscv/kernel/elf_kexec.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
> index 0cb94992c15b..bba3723a0914 100644
> --- a/arch/riscv/kernel/elf_kexec.c
> +++ b/arch/riscv/kernel/elf_kexec.c
> @@ -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 */

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.

Signed-off-by: Conor Dooley <[email protected]>
---
arch/riscv/kernel/elf_kexec.c | 28 ++++++++++++++--------------
1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
index 0cb94992c15b..29cbf655c474 100644
--- a/arch/riscv/kernel/elf_kexec.c
+++ b/arch/riscv/kernel/elf_kexec.c
@@ -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

2022-09-04 11:36:37

by Conor Dooley

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

On 04/09/2022 11:56, Guo Ren wrote:
> On Sun, Sep 4, 2022 at 6:36 PM <[email protected]> wrote:
>>
>> 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]
>>> 185 | unsigned long kernel_start;
>>> | ^~~~~~~~~~~~
>>>
>>> Signed-off-by: Guo Ren <[email protected]>
>>> Signed-off-by: Guo Ren <[email protected]>
>>> Reported-by: kernel test robot <[email protected]>
>>
>> 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?
>>
>>> ---
>>> arch/riscv/kernel/elf_kexec.c | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
>>> index 0cb94992c15b..bba3723a0914 100644
>>> --- a/arch/riscv/kernel/elf_kexec.c
>>> +++ b/arch/riscv/kernel/elf_kexec.c
>>> @@ -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 */
>>
>> 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:

Uh, that works for me. The below diff is:
Reviewed-by: Conor Dooley <[email protected]>

>
> diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
> index 0cb94992c15b..4b9264340b78 100644
> --- a/arch/riscv/kernel/elf_kexec.c
> +++ b/arch/riscv/kernel/elf_kexec.c
> @@ -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,
>
>
>>

Feel free to include this patch in your v3 then:
>> -- >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.
>>
>> Signed-off-by: Conor Dooley <[email protected]>
>> ---
>> arch/riscv/kernel/elf_kexec.c | 28 ++++++++++++++--------------
>> 1 file changed, 14 insertions(+), 14 deletions(-)
>>
>> diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
>> index 0cb94992c15b..29cbf655c474 100644
>> --- a/arch/riscv/kernel/elf_kexec.c
>> +++ b/arch/riscv/kernel/elf_kexec.c
>> @@ -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
>>
>
>

2022-09-04 11:47:37

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH V2 0/6] riscv: Add GENERIC_ENTRY, IRQ_STACKS support

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.

>
> 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.

>
> One more comment below:
>
> 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].
> >
> > [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.
>
> > - Fixup compile warning
> > Reported-by: kernel test robot <[email protected]>
> > - 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
> >
>


--
Best Regards
Guo Ren

2022-09-04 12:23:39

by Guo Ren

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

On Sun, Sep 4, 2022 at 7:13 PM <[email protected]> wrote:
>
> On 04/09/2022 11:56, Guo Ren wrote:
> > On Sun, Sep 4, 2022 at 6:36 PM <[email protected]> wrote:
> >>
> >> 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]
> >>> 185 | unsigned long kernel_start;
> >>> | ^~~~~~~~~~~~
> >>>
> >>> Signed-off-by: Guo Ren <[email protected]>
> >>> Signed-off-by: Guo Ren <[email protected]>
> >>> Reported-by: kernel test robot <[email protected]>
> >>
> >> 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?
> >>
> >>> ---
> >>> arch/riscv/kernel/elf_kexec.c | 4 ++++
> >>> 1 file changed, 4 insertions(+)
> >>>
> >>> diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
> >>> index 0cb94992c15b..bba3723a0914 100644
> >>> --- a/arch/riscv/kernel/elf_kexec.c
> >>> +++ b/arch/riscv/kernel/elf_kexec.c
> >>> @@ -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 */
> >>
> >> 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:
>
> Uh, that works for me. The below diff is:
> Reviewed-by: Conor Dooley <[email protected]>
Thx, your idea is also awesome which let me know the benefit of
IS_ENABLED() than ifdef.

>
> >
> > diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
> > index 0cb94992c15b..4b9264340b78 100644
> > --- a/arch/riscv/kernel/elf_kexec.c
> > +++ b/arch/riscv/kernel/elf_kexec.c
> > @@ -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,
> >
> >
> >>
>
> Feel free to include this patch in your v3 then:
> >> -- >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.
> >>
> >> Signed-off-by: Conor Dooley <[email protected]>
> >> ---
> >> arch/riscv/kernel/elf_kexec.c | 28 ++++++++++++++--------------
> >> 1 file changed, 14 insertions(+), 14 deletions(-)
> >>
> >> diff --git a/arch/riscv/kernel/elf_kexec.c b/arch/riscv/kernel/elf_kexec.c
> >> index 0cb94992c15b..29cbf655c474 100644
> >> --- a/arch/riscv/kernel/elf_kexec.c
> >> +++ b/arch/riscv/kernel/elf_kexec.c
> >> @@ -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
> >>
> >
> >



--
Best Regards
Guo Ren

2022-09-05 13:48:53

by Jisheng Zhang

[permalink] [raw]
Subject: Re: [PATCH V2 0/6] riscv: Add GENERIC_ENTRY, IRQ_STACKS support

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.

> 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
> Reported-by: kernel test robot <[email protected]>
> - 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

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

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

2022-09-06 02:11:26

by Guo Ren

[permalink] [raw]
Subject: Re: [PATCH V2 0/6] riscv: Add GENERIC_ENTRY, IRQ_STACKS support

On Mon, Sep 5, 2022 at 9:27 PM Jisheng Zhang <[email protected]> wrote:
>
> 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.

> 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.

>
> > 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
> > Reported-by: kernel test robot <[email protected]>
> > - 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
>
> It's better to move these two patches ahead of patch2.
Okay.

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



--
Best Regards
Guo Ren