[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v3 1/4] xen: introduce CONFIG_GENERIC_BUG_FRAME
On Mon, 2023-02-27 at 15:23 +0100, Jan Beulich wrote: > On 24.02.2023 12:31, Oleksii Kurochko wrote: > > --- /dev/null > > +++ b/xen/common/bug.c > > @@ -0,0 +1,109 @@ > > +#include <xen/bug.h> > > +#include <xen/debugger.h> > > +#include <xen/errno.h> > > +#include <xen/kernel.h> > > +#include <xen/livepatch.h> > > +#include <xen/string.h> > > +#include <xen/types.h> > > +#include <xen/virtual_region.h> > > + > > +#include <asm/processor.h> > > + > > +/* Set default value for TRAP_invalid_op as it is defined only for > > X86 now */ > > +#ifndef TRAP_invalid_op > > +#define TRAP_invalid_op 0 > > +#endif > > + > > +int do_bug_frame(const struct cpu_user_regs *regs, unsigned long > > pc) > > +{ > > + const struct bug_frame *bug = NULL; > > + const struct virtual_region *region; > > + const char *prefix = "", *filename, *predicate; > > + unsigned long fixup; > > + unsigned int id = BUGFRAME_NR, lineno; > > + > > + region = find_text_region(pc); > > + if ( region ) > > + { > > + for ( id = 0; id < BUGFRAME_NR; id++ ) > > + { > > + const struct bug_frame *b; > > + unsigned int i; > > + > > + for ( i = 0, b = region->frame[id].bugs; > > + i < region->frame[id].n_bugs; b++, i++ ) > > + { > > + if ( bug_loc(b) == pc ) > > + { > > + bug = b; > > + goto found; > > + } > > + } > > + } > > + } > > + > > + found: > > + if ( !bug ) > > + return -EINVAL; > > + > > + if ( id == BUGFRAME_run_fn ) > > + { > > +#ifdef BUG_FN_REG > > + void (*fn)(const struct cpu_user_regs *) = (void *)regs- > > >BUG_FN_REG; > > +#else > > + void (*fn)(const struct cpu_user_regs *) = bug_ptr(bug); > > +#endif > > + > > + fn(regs); > > + > > + return id; > > + } > > + > > + /* WARN, BUG or ASSERT: decode the filename pointer and line > > number. */ > > + filename = bug_ptr(bug); > > + if ( !is_kernel(filename) && !is_patch(filename) ) > > + return -EINVAL; > > + fixup = strlen(filename); > > + if ( fixup > 50 ) > > + { > > + filename += fixup - 47; > > + prefix = "..."; > > + } > > + lineno = bug_line(bug); > > + > > + switch ( id ) > > + { > > + case BUGFRAME_warn: > > + printk("Xen WARN at %s%s:%d\n", prefix, filename, lineno); > > + show_execution_state(regs); > > + > > + return id; > > + > > + case BUGFRAME_bug: > > + printk("Xen BUG at %s%s:%d\n", prefix, filename, lineno); > > + > > + if ( debugger_trap_fatal(TRAP_invalid_op, regs) ) > > TRAP_invalid_op is, as said, about to disappear on x86 as well. I > think > this construct wants abstracting by another asm/bug.h provided macro > (taking just regs). > Thanks for the link. Nice idea to abstract 'debugger_trap_fatal(TRAP_invalid_op, regs)'. Actually we have to options here: 1. As you proposed abstract in <asm/bug.h>: x86: #define DEBUG_TRAP_FATAL(regs) debugger_trap_fatal(X86_EXC_GP, regs) ARM: #define DEBUG_TRAP_FATAL(regs) 0 RISC-V: #define DEBUG_TRAP_FATAL(regs) 0 For ARM and RISC-V it doesn't use so we can skip the check if ( DEBUG_TRAP_FATAL ). 2. Abstract only TRAP_invalid_op in <asm/bug.h> x86: #define TRAP_invalud_op X86_EXC_GP RISC-V: #define TRAP_invalid_op 0 ARN: #define TRAP_invalid_op 0 I am not sure if we have to provide real invalid opcodes for RISC-V and ARM as it looks like debug_trap_fatal() isn't used in ARM&RISC-V now. Could you please suggest which one option is better? ~ Oleksii > Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |