| Message ID | a0788e4744b04597fbd3e71c2bef0bd76843a066.1674226563.git.oleksii.kurochko@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | RISCV basic exception handling implementation | expand |
On 20.01.2023 15:59, Oleksii Kurochko wrote: > --- /dev/null > +++ b/xen/arch/riscv/include/asm/bug.h > @@ -0,0 +1,120 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > +/* > + * Copyright (C) 2012 Regents of the University of California > + * Copyright (C) 2021-2023 Vates > + * > + */ > + > +#ifndef _ASM_RISCV_BUG_H > +#define _ASM_RISCV_BUG_H > + > +#include <xen/stringify.h> > +#include <xen/types.h> > + > +#ifndef __ASSEMBLY__ > + > +struct bug_frame { > + signed int loc_disp; /* Relative address to the bug address */ > + signed int file_disp; /* Relative address to the filename */ > + signed int msg_disp; /* Relative address to the predicate (for ASSERT) */ > + uint16_t line; /* Line number */ > + uint32_t pad0:16; /* Padding for 8-bytes align */ > +}; > + > +#define bug_loc(b) ((const void *)(b) + (b)->loc_disp) > +#define bug_file(b) ((const void *)(b) + (b)->file_disp); > +#define bug_line(b) ((b)->line) > +#define bug_msg(b) ((const char *)(b) + (b)->msg_disp) > + > +#define BUGFRAME_run_fn 0 > +#define BUGFRAME_warn 1 > +#define BUGFRAME_bug 2 > +#define BUGFRAME_assert 3 > + > +#define BUGFRAME_NR 4 > + > +#define __INSN_LENGTH_MASK _UL(0x3) > +#define __INSN_LENGTH_32 _UL(0x3) > +#define __COMPRESSED_INSN_MASK _UL(0xffff) > + > +#define __BUG_INSN_32 _UL(0x00100073) /* ebreak */ > +#define __BUG_INSN_16 _UL(0x9002) /* c.ebreak */ May I suggest that you avoid double-underscore (or other reserved) names where possible? > +#define GET_INSN_LENGTH(insn) \ > +({ \ > + unsigned long __len; \ > + __len = ((insn & __INSN_LENGTH_MASK) == __INSN_LENGTH_32) ? \ > + 4UL : 2UL; \ > + __len; \ > +}) > + > +typedef u32 bug_insn_t; This is problematic beyond the u32 instead of uint32_t. You use it once, ... > +/* These are defined by the architecture */ > +int is_valid_bugaddr(bug_insn_t addr); ... in a call to this function, but you can't assume that you can access 32 bits when the insn you look at might be a compressed one. Just to be on the safe side I'd like to suggest to either avoid such a type, or to introduce two (32- and 16-bit) which then of course need using properly in respective contexts. > +#define BUG_FN_REG t0 > + > +/* Many versions of GCC doesn't support the asm %c parameter which would > + * be preferable to this unpleasantness. We use mergeable string > + * sections to avoid multiple copies of the string appearing in the > + * Xen image. BUGFRAME_run_fn needs to be handled separately. > + */ > +#define BUG_FRAME(type, line, file, has_msg, msg) do { \ > + asm ("1:ebreak\n" \ Something's odd with the padding here; looks like there might be hard tabs. > + ".pushsection .rodata.str, \"aMS\", %progbits, 1\n" \ > + "2:\t.asciz " __stringify(file) "\n" \ > + "3:\n" \ > + ".if " #has_msg "\n" \ > + "\t.asciz " #msg "\n" \ > + ".endif\n" \ > + ".popsection\n" \ > + ".pushsection .bug_frames." __stringify(type) ", \"a\", %progbits\n"\ > + "4:\n" \ > + ".p2align 2\n" \ > + ".long (1b - 4b)\n" \ > + ".long (2b - 4b)\n" \ > + ".long (3b - 4b)\n" \ > + ".hword " __stringify(line) ", 0\n" \ > + ".popsection"); \ > +} while (0) > + > +/* > + * GCC will not allow to use "i" when PIE is enabled (Xen doesn't set the > + * flag but instead rely on the default value from the compiler). So the > + * easiest way to implement run_in_exception_handler() is to pass the to > + * be called function in a fixed register. > + */ > +#define run_in_exception_handler(fn) do { \ With register void *fn_ asm(__stringify(BUG_FN_REG)) = (fn); you should be able to avoid ... > + asm ("mv " __stringify(BUG_FN_REG) ", %0\n" \ ... this and simply use ... > + "1:ebreak\n" \ > + ".pushsection .bug_frames." __stringify(BUGFRAME_run_fn) "," \ > + " \"a\", %%progbits\n" \ > + "2:\n" \ > + ".p2align 2\n" \ > + ".long (1b - 2b)\n" \ > + ".long 0, 0, 0\n" \ > + ".popsection" :: "r" (fn) : __stringify(BUG_FN_REG) ); \ :: "r" (fn_) ); here. See x86's alternative_callN() for similar examples. > @@ -107,7 +108,122 @@ static void do_unexpected_trap(const struct cpu_user_regs *regs) > wait_for_interrupt(); > } > > +void show_execution_state(const struct cpu_user_regs *regs) > +{ > + early_printk("implement show_execution_state(regs)\n"); > +} > + > +int do_bug_frame(struct cpu_user_regs *regs, vaddr_t pc) > +{ > + struct bug_frame *start, *end; > + struct bug_frame *bug = NULL; > + unsigned int id = 0; > + const char *filename, *predicate; > + int lineno; > + > + unsigned long bug_frames[] = { > + (unsigned long)&__start_bug_frames[0], > + (unsigned long)&__stop_bug_frames_0[0], > + (unsigned long)&__stop_bug_frames_1[0], > + (unsigned long)&__stop_bug_frames_2[0], > + (unsigned long)&__stop_bug_frames_3[0], > + }; > + > + for ( id = 0; id < BUGFRAME_NR; id++ ) > + { > + start = (struct bug_frame *)bug_frames[id]; > + end = (struct bug_frame *)bug_frames[id + 1]; > + > + while ( start != end ) > + { > + if ( (vaddr_t)bug_loc(start) == pc ) > + { > + bug = start; > + goto found; > + } > + > + start++; > + } > + } > + > +found: Please indent labels by at least one blank; see ./CODING_STYLE. > + if ( bug == NULL ) > + return -ENOENT; > + > + if ( id == BUGFRAME_run_fn ) > + { > + void (*fn)(const struct cpu_user_regs *) = (void *)regs->BUG_FN_REG; > + > + fn(regs); > + > + goto end; > + } > + > + /* WARN, BUG or ASSERT: decode the filename pointer and line number. */ > + filename = bug_file(bug); > + lineno = bug_line(bug); > + > + switch ( id ) > + { > + case BUGFRAME_warn: > + early_printk("Xen WARN at "); > + early_printk(filename); > + early_printk(":"); > + early_printk_hnum(lineno); > + > + show_execution_state(regs); > + > + goto end; > + > + case BUGFRAME_bug: > + early_printk("Xen BUG at "); > + early_printk(filename); > + early_printk(":"); > + early_printk_hnum(lineno); > + > + show_execution_state(regs); > + early_printk("change wait_for_interrupt to panic() when common is available\n"); > + wait_for_interrupt(); > + > + case BUGFRAME_assert: > + /* ASSERT: decode the predicate string pointer. */ > + predicate = bug_msg(bug); > + > + early_printk("Assertion \'"); > + early_printk(predicate); > + early_printk("\' failed at "); > + early_printk(filename); > + early_printk(":"); > + early_printk_hnum(lineno); > + > + show_execution_state(regs); > + early_printk("change wait_for_interrupt to panic() when common is available\n"); > + wait_for_interrupt(); > + } > + > + return -EINVAL; > +end: > + regs->sepc += GET_INSN_LENGTH(*(bug_insn_t *)pc); > + > + return 0; > +} > + > +int is_valid_bugaddr(bug_insn_t insn) > +{ > + if ((insn & __INSN_LENGTH_MASK) == __INSN_LENGTH_32) > + return (insn == __BUG_INSN_32); > + else > + return ((insn & __COMPRESSED_INSN_MASK) == __BUG_INSN_16); > +} > + > void __handle_exception(struct cpu_user_regs *cpu_regs) > { > + register_t pc = cpu_regs->sepc; > + uint32_t instr = *(bug_insn_t *)pc; > + > + if (is_valid_bugaddr(instr)) > + if (!do_bug_frame(cpu_regs, pc)) return; > + > +// die: Perhaps better to omit the label until it's actually needed? In any event you shouldn't be using C++-style comments (see ./CODING_STYLE again). Jan
diff --git a/xen/arch/riscv/include/asm/bug.h b/xen/arch/riscv/include/asm/bug.h new file mode 100644 index 0000000000..d17ffdcc4d --- /dev/null +++ b/xen/arch/riscv/include/asm/bug.h @@ -0,0 +1,120 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (C) 2012 Regents of the University of California + * Copyright (C) 2021-2023 Vates + * + */ + +#ifndef _ASM_RISCV_BUG_H +#define _ASM_RISCV_BUG_H + +#include <xen/stringify.h> +#include <xen/types.h> + +#ifndef __ASSEMBLY__ + +struct bug_frame { + signed int loc_disp; /* Relative address to the bug address */ + signed int file_disp; /* Relative address to the filename */ + signed int msg_disp; /* Relative address to the predicate (for ASSERT) */ + uint16_t line; /* Line number */ + uint32_t pad0:16; /* Padding for 8-bytes align */ +}; + +#define bug_loc(b) ((const void *)(b) + (b)->loc_disp) +#define bug_file(b) ((const void *)(b) + (b)->file_disp); +#define bug_line(b) ((b)->line) +#define bug_msg(b) ((const char *)(b) + (b)->msg_disp) + +#define BUGFRAME_run_fn 0 +#define BUGFRAME_warn 1 +#define BUGFRAME_bug 2 +#define BUGFRAME_assert 3 + +#define BUGFRAME_NR 4 + +#define __INSN_LENGTH_MASK _UL(0x3) +#define __INSN_LENGTH_32 _UL(0x3) +#define __COMPRESSED_INSN_MASK _UL(0xffff) + +#define __BUG_INSN_32 _UL(0x00100073) /* ebreak */ +#define __BUG_INSN_16 _UL(0x9002) /* c.ebreak */ + +#define GET_INSN_LENGTH(insn) \ +({ \ + unsigned long __len; \ + __len = ((insn & __INSN_LENGTH_MASK) == __INSN_LENGTH_32) ? \ + 4UL : 2UL; \ + __len; \ +}) + +typedef u32 bug_insn_t; + +/* These are defined by the architecture */ +int is_valid_bugaddr(bug_insn_t addr); + +#define BUG_FN_REG t0 + +/* Many versions of GCC doesn't support the asm %c parameter which would + * be preferable to this unpleasantness. We use mergeable string + * sections to avoid multiple copies of the string appearing in the + * Xen image. BUGFRAME_run_fn needs to be handled separately. + */ +#define BUG_FRAME(type, line, file, has_msg, msg) do { \ + asm ("1:ebreak\n" \ + ".pushsection .rodata.str, \"aMS\", %progbits, 1\n" \ + "2:\t.asciz " __stringify(file) "\n" \ + "3:\n" \ + ".if " #has_msg "\n" \ + "\t.asciz " #msg "\n" \ + ".endif\n" \ + ".popsection\n" \ + ".pushsection .bug_frames." __stringify(type) ", \"a\", %progbits\n"\ + "4:\n" \ + ".p2align 2\n" \ + ".long (1b - 4b)\n" \ + ".long (2b - 4b)\n" \ + ".long (3b - 4b)\n" \ + ".hword " __stringify(line) ", 0\n" \ + ".popsection"); \ +} while (0) + +/* + * GCC will not allow to use "i" when PIE is enabled (Xen doesn't set the + * flag but instead rely on the default value from the compiler). So the + * easiest way to implement run_in_exception_handler() is to pass the to + * be called function in a fixed register. + */ +#define run_in_exception_handler(fn) do { \ + asm ("mv " __stringify(BUG_FN_REG) ", %0\n" \ + "1:ebreak\n" \ + ".pushsection .bug_frames." __stringify(BUGFRAME_run_fn) "," \ + " \"a\", %%progbits\n" \ + "2:\n" \ + ".p2align 2\n" \ + ".long (1b - 2b)\n" \ + ".long 0, 0, 0\n" \ + ".popsection" :: "r" (fn) : __stringify(BUG_FN_REG) ); \ +} while (0) + +#define WARN() BUG_FRAME(BUGFRAME_warn, __LINE__, __FILE__, 0, "") + +#define BUG() do { \ + BUG_FRAME(BUGFRAME_bug, __LINE__, __FILE__, 0, ""); \ + unreachable(); \ +} while (0) + +#define assert_failed(msg) do { \ + BUG_FRAME(BUGFRAME_assert, __LINE__, __FILE__, 1, msg); \ + unreachable(); \ +} while (0) + +extern const struct bug_frame __start_bug_frames[], + __stop_bug_frames_0[], + __stop_bug_frames_1[], + __stop_bug_frames_2[], + __stop_bug_frames_3[]; + +#endif /* !__ASSEMBLY__ */ + +#endif /* _ASM_RISCV_BUG_H */ diff --git a/xen/arch/riscv/traps.c b/xen/arch/riscv/traps.c index fc25138a4b..8b719a5ef5 100644 --- a/xen/arch/riscv/traps.c +++ b/xen/arch/riscv/traps.c @@ -4,6 +4,7 @@ * * RISC-V Trap handlers */ +#include <asm/bug.h> #include <asm/csr.h> #include <asm/early_printk.h> #include <asm/processor.h> @@ -107,7 +108,122 @@ static void do_unexpected_trap(const struct cpu_user_regs *regs) wait_for_interrupt(); } +void show_execution_state(const struct cpu_user_regs *regs) +{ + early_printk("implement show_execution_state(regs)\n"); +} + +int do_bug_frame(struct cpu_user_regs *regs, vaddr_t pc) +{ + struct bug_frame *start, *end; + struct bug_frame *bug = NULL; + unsigned int id = 0; + const char *filename, *predicate; + int lineno; + + unsigned long bug_frames[] = { + (unsigned long)&__start_bug_frames[0], + (unsigned long)&__stop_bug_frames_0[0], + (unsigned long)&__stop_bug_frames_1[0], + (unsigned long)&__stop_bug_frames_2[0], + (unsigned long)&__stop_bug_frames_3[0], + }; + + for ( id = 0; id < BUGFRAME_NR; id++ ) + { + start = (struct bug_frame *)bug_frames[id]; + end = (struct bug_frame *)bug_frames[id + 1]; + + while ( start != end ) + { + if ( (vaddr_t)bug_loc(start) == pc ) + { + bug = start; + goto found; + } + + start++; + } + } + +found: + if ( bug == NULL ) + return -ENOENT; + + if ( id == BUGFRAME_run_fn ) + { + void (*fn)(const struct cpu_user_regs *) = (void *)regs->BUG_FN_REG; + + fn(regs); + + goto end; + } + + /* WARN, BUG or ASSERT: decode the filename pointer and line number. */ + filename = bug_file(bug); + lineno = bug_line(bug); + + switch ( id ) + { + case BUGFRAME_warn: + early_printk("Xen WARN at "); + early_printk(filename); + early_printk(":"); + early_printk_hnum(lineno); + + show_execution_state(regs); + + goto end; + + case BUGFRAME_bug: + early_printk("Xen BUG at "); + early_printk(filename); + early_printk(":"); + early_printk_hnum(lineno); + + show_execution_state(regs); + early_printk("change wait_for_interrupt to panic() when common is available\n"); + wait_for_interrupt(); + + case BUGFRAME_assert: + /* ASSERT: decode the predicate string pointer. */ + predicate = bug_msg(bug); + + early_printk("Assertion \'"); + early_printk(predicate); + early_printk("\' failed at "); + early_printk(filename); + early_printk(":"); + early_printk_hnum(lineno); + + show_execution_state(regs); + early_printk("change wait_for_interrupt to panic() when common is available\n"); + wait_for_interrupt(); + } + + return -EINVAL; +end: + regs->sepc += GET_INSN_LENGTH(*(bug_insn_t *)pc); + + return 0; +} + +int is_valid_bugaddr(bug_insn_t insn) +{ + if ((insn & __INSN_LENGTH_MASK) == __INSN_LENGTH_32) + return (insn == __BUG_INSN_32); + else + return ((insn & __COMPRESSED_INSN_MASK) == __BUG_INSN_16); +} + void __handle_exception(struct cpu_user_regs *cpu_regs) { + register_t pc = cpu_regs->sepc; + uint32_t instr = *(bug_insn_t *)pc; + + if (is_valid_bugaddr(instr)) + if (!do_bug_frame(cpu_regs, pc)) return; + +// die: do_unexpected_trap(cpu_regs); } diff --git a/xen/arch/riscv/xen.lds.S b/xen/arch/riscv/xen.lds.S index ca57cce75c..139e2d04cb 100644 --- a/xen/arch/riscv/xen.lds.S +++ b/xen/arch/riscv/xen.lds.S @@ -39,6 +39,16 @@ SECTIONS . = ALIGN(PAGE_SIZE); .rodata : { _srodata = .; /* Read-only data */ + /* Bug frames table */ + __start_bug_frames = .; + *(.bug_frames.0) + __stop_bug_frames_0 = .; + *(.bug_frames.1) + __stop_bug_frames_1 = .; + *(.bug_frames.2) + __stop_bug_frames_2 = .; + *(.bug_frames.3) + __stop_bug_frames_3 = .; *(.rodata) *(.rodata.*) *(.data.rel.ro)
The patch introduces macros: BUG(), WARN(), run_in_exception(), assert_failed. The implementation uses "ebreak" instruction in combination with diffrent bug frame tables (for each type) which contains useful information. Signed-off-by: Oleksii Kurochko <oleksii.kurochko@gmail.com> --- xen/arch/riscv/include/asm/bug.h | 120 +++++++++++++++++++++++++++++++ xen/arch/riscv/traps.c | 116 ++++++++++++++++++++++++++++++ xen/arch/riscv/xen.lds.S | 10 +++ 3 files changed, 246 insertions(+) create mode 100644 xen/arch/riscv/include/asm/bug.h