| Message ID | 20230818121504.60492-1-cuiyunhui@bytedance.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v4] riscv: add userland instruction dump to RISC-V splats | expand |
| Context | Check | Description |
|---|---|---|
| conchuod/cover_letter | success | Single patches do not need cover letters |
| conchuod/tree_selection | success | Guessed tree name to be for-next at HEAD 174e8ac0272d |
| conchuod/fixes_present | success | Fixes tag not required for -next series |
| conchuod/maintainers_pattern | success | MAINTAINERS pattern errors before the patch: 4 and now 4 |
| conchuod/verify_signedoff | success | Signed-off-by tag matches author and committer |
| conchuod/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| conchuod/build_rv64_clang_allmodconfig | success | Errors and warnings before: 9 this patch: 9 |
| conchuod/module_param | success | Was 0 now: 0 |
| conchuod/build_rv64_gcc_allmodconfig | fail | Errors and warnings before: 9 this patch: 10 |
| conchuod/build_rv32_defconfig | success | Build OK |
| conchuod/dtb_warn_rv64 | success | Errors and warnings before: 12 this patch: 12 |
| conchuod/header_inline | success | No static functions without inline keyword in header files |
| conchuod/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 48 lines checked |
| conchuod/build_rv64_nommu_k210_defconfig | success | Build OK |
| conchuod/verify_fixes | success | No Fixes tag |
| conchuod/build_rv64_nommu_virt_defconfig | success | Build OK |
Yunhui Cui <cuiyunhui@bytedance.com> writes: > Add userland instruction dump and rename dump_kernel_instr() > to dump_instr(). > > An example: > [ 0.822439] Freeing unused kernel image (initmem) memory: 6916K > [ 0.823817] Run /init as init process > [ 0.839411] init[1]: unhandled signal 4 code 0x1 at 0x000000000005be18 in bb[10000+5fb000] > [ 0.840751] CPU: 0 PID: 1 Comm: init Not tainted 5.14.0-rc4-00049-gbd644290aa72-dirty #187 > [ 0.841373] Hardware name: , BIOS > [ 0.841743] epc : 000000000005be18 ra : 0000000000079e74 sp : 0000003fffcafda0 > [ 0.842271] gp : ffffffff816e9dc8 tp : 0000000000000000 t0 : 0000000000000000 > [ 0.842947] t1 : 0000003fffc9fdf0 t2 : 0000000000000000 s0 : 0000000000000000 > [ 0.843434] s1 : 0000000000000000 a0 : 0000003fffca0190 a1 : 0000003fffcafe18 > [ 0.843891] a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000 > [ 0.844357] a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000000000000 > [ 0.844803] s2 : 0000000000000000 s3 : 0000000000000000 s4 : 0000000000000000 > [ 0.845253] s5 : 0000000000000000 s6 : 0000000000000000 s7 : 0000000000000000 > [ 0.845722] s8 : 0000000000000000 s9 : 0000000000000000 s10: 0000000000000000 > [ 0.846180] s11: 0000000000d144e0 t3 : 0000000000000000 t4 : 0000000000000000 > [ 0.846616] t5 : 0000000000000000 t6 : 0000000000000000 > [ 0.847204] status: 0000000200000020 badaddr: 00000000f0028053 cause: 0000000000000002 > [ 0.848219] Code: f06f ff5f 3823 fa11 0113 fb01 2e23 0201 0293 0000 (8053) f002 > [ 0.851016] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000004 > > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com> Nice! Reviewed-by: Björn Töpel <bjorn@rivosinc.com>
Hi Björn， On Fri, Aug 18, 2023 at 9:33 PM Björn Töpel <bjorn@kernel.org> wrote: > > Yunhui Cui <cuiyunhui@bytedance.com> writes: > > > Add userland instruction dump and rename dump_kernel_instr() > > to dump_instr(). > > > > An example: > > [ 0.822439] Freeing unused kernel image (initmem) memory: 6916K > > [ 0.823817] Run /init as init process > > [ 0.839411] init[1]: unhandled signal 4 code 0x1 at 0x000000000005be18 in bb[10000+5fb000] > > [ 0.840751] CPU: 0 PID: 1 Comm: init Not tainted 5.14.0-rc4-00049-gbd644290aa72-dirty #187 > > [ 0.841373] Hardware name: , BIOS > > [ 0.841743] epc : 000000000005be18 ra : 0000000000079e74 sp : 0000003fffcafda0 > > [ 0.842271] gp : ffffffff816e9dc8 tp : 0000000000000000 t0 : 0000000000000000 > > [ 0.842947] t1 : 0000003fffc9fdf0 t2 : 0000000000000000 s0 : 0000000000000000 > > [ 0.843434] s1 : 0000000000000000 a0 : 0000003fffca0190 a1 : 0000003fffcafe18 > > [ 0.843891] a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000 > > [ 0.844357] a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000000000000 > > [ 0.844803] s2 : 0000000000000000 s3 : 0000000000000000 s4 : 0000000000000000 > > [ 0.845253] s5 : 0000000000000000 s6 : 0000000000000000 s7 : 0000000000000000 > > [ 0.845722] s8 : 0000000000000000 s9 : 0000000000000000 s10: 0000000000000000 > > [ 0.846180] s11: 0000000000d144e0 t3 : 0000000000000000 t4 : 0000000000000000 > > [ 0.846616] t5 : 0000000000000000 t6 : 0000000000000000 > > [ 0.847204] status: 0000000200000020 badaddr: 00000000f0028053 cause: 0000000000000002 > > [ 0.848219] Code: f06f ff5f 3823 fa11 0113 fb01 2e23 0201 0293 0000 (8053) f002 > > [ 0.851016] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000004 > > > > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com> > > Nice! > > Reviewed-by: Björn Töpel <bjorn@rivosinc.com> When can this patch be applied to the linux-next branch? Thanks, Yunhui
yunhui cui <cuiyunhui@bytedance.com> writes: > Hi Björn， > > On Fri, Aug 18, 2023 at 9:33 PM Björn Töpel <bjorn@kernel.org> wrote: >> >> Yunhui Cui <cuiyunhui@bytedance.com> writes: >> >> > Add userland instruction dump and rename dump_kernel_instr() >> > to dump_instr(). >> > >> > An example: >> > [ 0.822439] Freeing unused kernel image (initmem) memory: 6916K >> > [ 0.823817] Run /init as init process >> > [ 0.839411] init[1]: unhandled signal 4 code 0x1 at 0x000000000005be18 in bb[10000+5fb000] >> > [ 0.840751] CPU: 0 PID: 1 Comm: init Not tainted 5.14.0-rc4-00049-gbd644290aa72-dirty #187 >> > [ 0.841373] Hardware name: , BIOS >> > [ 0.841743] epc : 000000000005be18 ra : 0000000000079e74 sp : 0000003fffcafda0 >> > [ 0.842271] gp : ffffffff816e9dc8 tp : 0000000000000000 t0 : 0000000000000000 >> > [ 0.842947] t1 : 0000003fffc9fdf0 t2 : 0000000000000000 s0 : 0000000000000000 >> > [ 0.843434] s1 : 0000000000000000 a0 : 0000003fffca0190 a1 : 0000003fffcafe18 >> > [ 0.843891] a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000 >> > [ 0.844357] a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000000000000 >> > [ 0.844803] s2 : 0000000000000000 s3 : 0000000000000000 s4 : 0000000000000000 >> > [ 0.845253] s5 : 0000000000000000 s6 : 0000000000000000 s7 : 0000000000000000 >> > [ 0.845722] s8 : 0000000000000000 s9 : 0000000000000000 s10: 0000000000000000 >> > [ 0.846180] s11: 0000000000d144e0 t3 : 0000000000000000 t4 : 0000000000000000 >> > [ 0.846616] t5 : 0000000000000000 t6 : 0000000000000000 >> > [ 0.847204] status: 0000000200000020 badaddr: 00000000f0028053 cause: 0000000000000002 >> > [ 0.848219] Code: f06f ff5f 3823 fa11 0113 fb01 2e23 0201 0293 0000 (8053) f002 >> > [ 0.851016] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000004 >> > >> > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com> >> >> Nice! >> >> Reviewed-by: Björn Töpel <bjorn@rivosinc.com> > > When can this patch be applied to the linux-next branch? It looks like you're getting a sparse warning on the patch [1], and that's probably why it hasn't been considered. You need something like: -- diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c index 69b5d18b5ae9..c5364131b8bd 100644 --- a/arch/riscv/kernel/traps.c +++ b/arch/riscv/kernel/traps.c @@ -34,7 +34,7 @@ int show_unhandled_signals = 1; static DEFINE_SPINLOCK(die_lock); #define get_user_nofault(val, ptr) ({ \ - const typeof(val) *__gk_ptr = (ptr); \ + const typeof(val) __user *__gk_ptr = (__force const typeof(val) __user *)(ptr); \ copy_from_user_nofault(&(val), __gk_ptr, sizeof(val));\ }) -- IDK, maybe it worth removing the define all together, and just open code it. When you're submitting patches, it's a good thing to track patchwork for warnings/errors. Björn [1] https://patchwork.kernel.org/project/linux-riscv/patch/20230818121504.60492-1-cuiyunhui@bytedance.com/
Hi Björn, On Fri, Sep 1, 2023 at 5:06 PM Björn Töpel <bjorn@kernel.org> wrote: > > yunhui cui <cuiyunhui@bytedance.com> writes: > > > Hi Björn， > > > > On Fri, Aug 18, 2023 at 9:33 PM Björn Töpel <bjorn@kernel.org> wrote: > >> > >> Yunhui Cui <cuiyunhui@bytedance.com> writes: > >> > >> > Add userland instruction dump and rename dump_kernel_instr() > >> > to dump_instr(). > >> > > >> > An example: > >> > [ 0.822439] Freeing unused kernel image (initmem) memory: 6916K > >> > [ 0.823817] Run /init as init process > >> > [ 0.839411] init[1]: unhandled signal 4 code 0x1 at 0x000000000005be18 in bb[10000+5fb000] > >> > [ 0.840751] CPU: 0 PID: 1 Comm: init Not tainted 5.14.0-rc4-00049-gbd644290aa72-dirty #187 > >> > [ 0.841373] Hardware name: , BIOS > >> > [ 0.841743] epc : 000000000005be18 ra : 0000000000079e74 sp : 0000003fffcafda0 > >> > [ 0.842271] gp : ffffffff816e9dc8 tp : 0000000000000000 t0 : 0000000000000000 > >> > [ 0.842947] t1 : 0000003fffc9fdf0 t2 : 0000000000000000 s0 : 0000000000000000 > >> > [ 0.843434] s1 : 0000000000000000 a0 : 0000003fffca0190 a1 : 0000003fffcafe18 > >> > [ 0.843891] a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000 > >> > [ 0.844357] a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000000000000 > >> > [ 0.844803] s2 : 0000000000000000 s3 : 0000000000000000 s4 : 0000000000000000 > >> > [ 0.845253] s5 : 0000000000000000 s6 : 0000000000000000 s7 : 0000000000000000 > >> > [ 0.845722] s8 : 0000000000000000 s9 : 0000000000000000 s10: 0000000000000000 > >> > [ 0.846180] s11: 0000000000d144e0 t3 : 0000000000000000 t4 : 0000000000000000 > >> > [ 0.846616] t5 : 0000000000000000 t6 : 0000000000000000 > >> > [ 0.847204] status: 0000000200000020 badaddr: 00000000f0028053 cause: 0000000000000002 > >> > [ 0.848219] Code: f06f ff5f 3823 fa11 0113 fb01 2e23 0201 0293 0000 (8053) f002 > >> > [ 0.851016] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000004 > >> > > >> > Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com> > >> > >> Nice! > >> > >> Reviewed-by: Björn Töpel <bjorn@rivosinc.com> > > > > When can this patch be applied to the linux-next branch? > > It looks like you're getting a sparse warning on the patch [1], and > that's probably why it hasn't been considered. You need something like: > > -- > diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c > index 69b5d18b5ae9..c5364131b8bd 100644 > --- a/arch/riscv/kernel/traps.c > +++ b/arch/riscv/kernel/traps.c > @@ -34,7 +34,7 @@ int show_unhandled_signals = 1; > static DEFINE_SPINLOCK(die_lock); > > #define get_user_nofault(val, ptr) ({ \ > - const typeof(val) *__gk_ptr = (ptr); \ > + const typeof(val) __user *__gk_ptr = (__force const typeof(val) __user *)(ptr); \ > copy_from_user_nofault(&(val), __gk_ptr, sizeof(val));\ > }) > -- > > IDK, maybe it worth removing the define all together, and just open code > it. > > When you're submitting patches, it's a good thing to track patchwork for > warnings/errors. > > > Björn > > [1] https://patchwork.kernel.org/project/linux-riscv/patch/20230818121504.60492-1-cuiyunhui@bytedance.com/ Okay, I might as well remove the macro, after all, the definition is only here. I'll update it v5 like: +static int copy_code(struct pt_regs *regs, u16 *val, const u16 *insns) +{ + if (!user_mode(regs)) + return get_kernel_nofault(*val, insns); + + /* The user space code from other tasks cannot be accessed. */ + if (regs != task_pt_regs(current)) + return -EPERM; + + return copy_from_user_nofault(val, insns, sizeof(*val)); +} Thanks, Yunhui
diff --git a/arch/riscv/kernel/traps.c b/arch/riscv/kernel/traps.c index f798c853bede..69b5d18b5ae9 100644 --- a/arch/riscv/kernel/traps.c +++ b/arch/riscv/kernel/traps.c @@ -33,7 +33,24 @@ int show_unhandled_signals = 1; static DEFINE_SPINLOCK(die_lock); -static void dump_kernel_instr(const char *loglvl, struct pt_regs *regs) +#define get_user_nofault(val, ptr) ({ \ + const typeof(val) *__gk_ptr = (ptr); \ + copy_from_user_nofault(&(val), __gk_ptr, sizeof(val));\ +}) + +static int copy_code(struct pt_regs *regs, u16 *val, const u16 *insns) +{ + if (!user_mode(regs)) + return get_kernel_nofault(*val, insns); + + /* The user space code from other tasks cannot be accessed. */ + if (regs != task_pt_regs(current)) + return -EPERM; + + return get_user_nofault(*val, insns); +} + +static void dump_instr(const char *loglvl, struct pt_regs *regs) { char str[sizeof("0000 ") * 12 + 2 + 1], *p = str; const u16 *insns = (u16 *)instruction_pointer(regs); @@ -42,7 +59,7 @@ static void dump_kernel_instr(const char *loglvl, struct pt_regs *regs) int i; for (i = -10; i < 2; i++) { - bad = get_kernel_nofault(val, &insns[i]); + bad = copy_code(regs, &val, &insns[i]); if (!bad) { p += sprintf(p, i == 0 ? "(%04hx) " : "%04hx ", val); } else { @@ -71,7 +88,7 @@ void die(struct pt_regs *regs, const char *str) print_modules(); if (regs) { show_regs(regs); - dump_kernel_instr(KERN_EMERG, regs); + dump_instr(KERN_EMERG, regs); } cause = regs ? regs->cause : -1; @@ -104,6 +121,7 @@ void do_trap(struct pt_regs *regs, int signo, int code, unsigned long addr) print_vma_addr(KERN_CONT " in ", instruction_pointer(regs)); pr_cont("\n"); __show_regs(regs); + dump_instr(KERN_EMERG, regs); } force_sig_fault(signo, code, (void __user *)addr);
Add userland instruction dump and rename dump_kernel_instr() to dump_instr(). An example: [ 0.822439] Freeing unused kernel image (initmem) memory: 6916K [ 0.823817] Run /init as init process [ 0.839411] init[1]: unhandled signal 4 code 0x1 at 0x000000000005be18 in bb[10000+5fb000] [ 0.840751] CPU: 0 PID: 1 Comm: init Not tainted 5.14.0-rc4-00049-gbd644290aa72-dirty #187 [ 0.841373] Hardware name: , BIOS [ 0.841743] epc : 000000000005be18 ra : 0000000000079e74 sp : 0000003fffcafda0 [ 0.842271] gp : ffffffff816e9dc8 tp : 0000000000000000 t0 : 0000000000000000 [ 0.842947] t1 : 0000003fffc9fdf0 t2 : 0000000000000000 s0 : 0000000000000000 [ 0.843434] s1 : 0000000000000000 a0 : 0000003fffca0190 a1 : 0000003fffcafe18 [ 0.843891] a2 : 0000000000000000 a3 : 0000000000000000 a4 : 0000000000000000 [ 0.844357] a5 : 0000000000000000 a6 : 0000000000000000 a7 : 0000000000000000 [ 0.844803] s2 : 0000000000000000 s3 : 0000000000000000 s4 : 0000000000000000 [ 0.845253] s5 : 0000000000000000 s6 : 0000000000000000 s7 : 0000000000000000 [ 0.845722] s8 : 0000000000000000 s9 : 0000000000000000 s10: 0000000000000000 [ 0.846180] s11: 0000000000d144e0 t3 : 0000000000000000 t4 : 0000000000000000 [ 0.846616] t5 : 0000000000000000 t6 : 0000000000000000 [ 0.847204] status: 0000000200000020 badaddr: 00000000f0028053 cause: 0000000000000002 [ 0.848219] Code: f06f ff5f 3823 fa11 0113 fb01 2e23 0201 0293 0000 (8053) f002 [ 0.851016] Kernel panic - not syncing: Attempted to kill init! exitcode=0x00000004 Signed-off-by: Yunhui Cui <cuiyunhui@bytedance.com> --- arch/riscv/kernel/traps.c | 24 +++++++++++++++++++++--- 1 file changed, 21 insertions(+), 3 deletions(-)