| Message ID | 1407256336-11839-1-git-send-email-julien.grall@linaro.org |
|---|---|
| State | Superseded, archived |
| Headers | show |
On Tue, 2014-08-05 at 17:32 +0100, Julien Grall wrote: > Currently the hypervisor will hang if it hits a WARN_ON. > > The implemention uses an undefined instruction, made ourself because ARM > doesn't provide one, to implement BUG/ASSERT/WARN_ON, and sets up the > different tables (one for each type) which contain useful information. > > This is based on the x86 implementation (include/asm-x86/bug.h). Unfortunately > the structure can't be shared because many ARM{32,64} gcc versions doesn't > correctly support %c. The support of executing a function in an exception handler > is also keep unimplemented on ARM. Therefore, dump_execution_state is > implement as WARN() > > The current opcode used to go in exception mode may not be undefined on ARM64. > Use the instruction "brk" to generate a software debug exception. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> > > --- > Changes in v2: > - Missing static in do_bug_frame prototype > - Add support for ARM64 by using the instruction brk > - Implement dump_execution_state as WARN > --- > xen/arch/arm/arm32/traps.c | 22 ++++++++- > xen/arch/arm/traps.c | 103 +++++++++++++++++++++++++++++++++++++++ > xen/arch/arm/xen.lds.S | 8 +++ > xen/include/asm-arm/arm32/bug.h | 13 +++++ > xen/include/asm-arm/arm64/bug.h | 10 ++++ > xen/include/asm-arm/bug.h | 77 +++++++++++++++++++++++++++-- > xen/include/asm-arm/debugger.h | 2 +- > xen/include/asm-arm/processor.h | 17 ++++++- > 8 files changed, 246 insertions(+), 6 deletions(-) > create mode 100644 xen/include/asm-arm/arm32/bug.h > create mode 100644 xen/include/asm-arm/arm64/bug.h > > diff --git a/xen/arch/arm/arm32/traps.c b/xen/arch/arm/arm32/traps.c > index ff0b945..baf8c41 100644 > --- a/xen/arch/arm/arm32/traps.c > +++ b/xen/arch/arm/arm32/traps.c > @@ -18,6 +18,7 @@ > > #include <xen/config.h> > #include <xen/lib.h> > +#include <xen/kernel.h> > > #include <public/xen.h> > > @@ -25,7 +26,26 @@ > > asmlinkage void do_trap_undefined_instruction(struct cpu_user_regs *regs) > { > - do_unexpected_trap("Undefined Instruction", regs); > + uint64_t pc = regs->pc; > + uint32_t instr; > + > + if ( !is_kernel_text(pc) && > + (system_state >= SYS_STATE_active || !is_kernel_inittext(pc)) ) > + goto die; > + > + /* PC is always 4-byte align, as Xen is using ARM instruction set */ "aligned" Is it worth a check here? I presume the nested fault if PC were misaligned would be pretty exciting, print+goto die would seem appropriate. > + instr = *((uint32_t *)regs->pc); > + if ( instr != BUG_OPCODE ) > + goto die; > + > + if ( do_bug_frame(regs, regs->pc) ) > + goto die; > + > + regs->pc += 4; > + return; > + > +die: > + do_unexpected_trap("undefined instruction", regs); No need to change the case of this message. > }> +#ifdef CONFIG_ARM_64 > +static void do_trap_brk(struct cpu_user_regs *regs, union hsr hsr) > +{ > + /* HCR_EL2.TGE and MDCR_EL2.TDE are not set so we never receive > + * software breakpoint exception for EL1 and EL0 here > + */ BUG_ON? ;-O > +die: > + do_unexpected_trap("undefined breakpoint value", regs); Please can you capitilise this to match the other callers. > +/* Many version of GCC doesn't support the asm %c parameter which would "versions". > + * be preferable to this unpleasantness. We use mergeable string > + * sections to avoid multiple copies of the string appearing in the > + * Xen image. OOI what is the size increase of the final (stripped) binary with this patch? Ian.
Hi Ian, On 08/09/14 06:01, Ian Campbell wrote: >> + /* PC is always 4-byte align, as Xen is using ARM instruction set */ > > "aligned" Ok. > Is it worth a check here? I presume the nested fault if PC were > misaligned would be pretty exciting, print+goto die would seem > appropriate. I don't think so. The "undefined instruction" is only used when the processor is unable to decode the instruction. If it fails to load the instruction (because a wrong memory address, pc misaligned), Xen would have received a "prefetch abort". >> + instr = *((uint32_t *)regs->pc); >> + if ( instr != BUG_OPCODE ) >> + goto die; >> + >> + if ( do_bug_frame(regs, regs->pc) ) >> + goto die; >> + >> + regs->pc += 4; >> + return; >> + >> +die: >> + do_unexpected_trap("undefined instruction", regs); > > No need to change the case of this message. Ok. >> }> +#ifdef CONFIG_ARM_64 >> +static void do_trap_brk(struct cpu_user_regs *regs, union hsr hsr) >> +{ >> + /* HCR_EL2.TGE and MDCR_EL2.TDE are not set so we never receive >> + * software breakpoint exception for EL1 and EL0 here >> + */ > > BUG_ON? ;-O Sounds a good plan. >> +die: >> + do_unexpected_trap("undefined breakpoint value", regs); > > Please can you capitilise this to match the other callers. Will do. > >> +/* Many version of GCC doesn't support the asm %c parameter which would > > "versions". > >> + * be preferable to this unpleasantness. We use mergeable string >> + * sections to avoid multiple copies of the string appearing in the >> + * Xen image. > > OOI what is the size increase of the final (stripped) binary with this patch? I compared the file xen/xen and on both arm32/arm64 the binary is smaller of about 2%. I think this is because previously gcc wasn't merge the string when BUG_ON was used in inline function. Regards,
On 08/09/14 12:19, Julien Grall wrote: >> >> OOI what is the size increase of the final (stripped) binary with this >> patch? > > I compared the file xen/xen and on both arm32/arm64 the binary is > smaller of about 2%. > > I think this is because previously gcc wasn't merge the string when > BUG_ON was used in inline function. BTW this series won't work correctly because of XSA-102 on ARM64 when WARN_ON/BUG_ON/ASSERT is used in startup code. This is because the "current" is set to a debug sanity value. Do you plan to send the series to add support for 32-bit userspace on 64-bits kernels for Xen 4.5? Regards,
On Mon, 2014-09-08 at 12:19 -0700, Julien Grall wrote: > Hi Ian, > > On 08/09/14 06:01, Ian Campbell wrote: > >> + /* PC is always 4-byte align, as Xen is using ARM instruction set */ > > > > "aligned" > > Ok. > > > Is it worth a check here? I presume the nested fault if PC were > > misaligned would be pretty exciting, print+goto die would seem > > appropriate. > > I don't think so. The "undefined instruction" is only used when the > processor is unable to decode the instruction. If it fails to load the > instruction (because a wrong memory address, pc misaligned), Xen would > have received a "prefetch abort". I was thinking of something either wrongly or rightly transitioning to thumb mode. It'd have to be something horrible like a broken firmware returning from a PSCI call in the wrong mode or something like that though. > >> }> +#ifdef CONFIG_ARM_64 > >> +static void do_trap_brk(struct cpu_user_regs *regs, union hsr hsr) > >> +{ > >> + /* HCR_EL2.TGE and MDCR_EL2.TDE are not set so we never receive > >> + * software breakpoint exception for EL1 and EL0 here > >> + */ > > > > BUG_ON? ;-O > > Sounds a good plan. Just be careful that it won't recurse ;-) > > > >> +/* Many version of GCC doesn't support the asm %c parameter which would > > > > "versions". > > > >> + * be preferable to this unpleasantness. We use mergeable string > >> + * sections to avoid multiple copies of the string appearing in the > >> + * Xen image. > > > > OOI what is the size increase of the final (stripped) binary with this patch? > > I compared the file xen/xen and on both arm32/arm64 the binary is > smaller of about 2%. > > I think this is because previously gcc wasn't merge the string when > BUG_ON was used in inline function. Not what I expected, but great! Ian.
On Mon, 2014-09-08 at 12:24 -0700, Julien Grall wrote: > On 08/09/14 12:19, Julien Grall wrote: > >> > >> OOI what is the size increase of the final (stripped) binary with this > >> patch? > > > > I compared the file xen/xen and on both arm32/arm64 the binary is > > smaller of about 2%. > > > > I think this is because previously gcc wasn't merge the string when > > BUG_ON was used in inline function. > > BTW this series won't work correctly because of XSA-102 on ARM64 when > WARN_ON/BUG_ON/ASSERT is used in startup code. > This is because the "current" is set to a debug sanity value. > > Do you plan to send the series to add support for 32-bit userspace on > 64-bits kernels for Xen 4.5? I must confess I'd forgotten about it. I'll dig it out and brush the cobwebs off ASAP. In the meantime can the test at the head of do_trap_hypervisor be extended to DTRT with traps from EL2 (i.e. skip the rest of the expression)? Ian.
Hi Ian, On 09/09/14 02:17, Ian Campbell wrote: > On Mon, 2014-09-08 at 12:19 -0700, Julien Grall wrote: >> Hi Ian, >> >> On 08/09/14 06:01, Ian Campbell wrote: >>>> + /* PC is always 4-byte align, as Xen is using ARM instruction set */ >>> >>> "aligned" >> >> Ok. >> >>> Is it worth a check here? I presume the nested fault if PC were >>> misaligned would be pretty exciting, print+goto die would seem >>> appropriate. >> >> I don't think so. The "undefined instruction" is only used when the >> processor is unable to decode the instruction. If it fails to load the >> instruction (because a wrong memory address, pc misaligned), Xen would >> have received a "prefetch abort". > > I was thinking of something either wrongly or rightly transitioning to > thumb mode. It'd have to be something horrible like a broken firmware > returning from a PSCI call in the wrong mode or something like that > though. The thumb mode is stored in SPSR which is a banked registers. Your use case can only happen if the secure mode decides to modify the registers of the hypervisor. It would be very stupid from the firmware and other bad things could happen. >>>> }> +#ifdef CONFIG_ARM_64 >>>> +static void do_trap_brk(struct cpu_user_regs *regs, union hsr hsr) >>>> +{ >>>> + /* HCR_EL2.TGE and MDCR_EL2.TDE are not set so we never receive >>>> + * software breakpoint exception for EL1 and EL0 here >>>> + */ >>> >>> BUG_ON? ;-O >> >> Sounds a good plan. > > Just be careful that it won't recurse ;-) Panic should never go in exception mode, so I will use it instead. Regards,
Hi Ian, On 09/09/14 02:19, Ian Campbell wrote: > On Mon, 2014-09-08 at 12:24 -0700, Julien Grall wrote: >> On 08/09/14 12:19, Julien Grall wrote: >>>> >>>> OOI what is the size increase of the final (stripped) binary with this >>>> patch? >>> >>> I compared the file xen/xen and on both arm32/arm64 the binary is >>> smaller of about 2%. >>> >>> I think this is because previously gcc wasn't merge the string when >>> BUG_ON was used in inline function. >> >> BTW this series won't work correctly because of XSA-102 on ARM64 when >> WARN_ON/BUG_ON/ASSERT is used in startup code. >> This is because the "current" is set to a debug sanity value. >> >> Do you plan to send the series to add support for 32-bit userspace on >> 64-bits kernels for Xen 4.5? > > I must confess I'd forgotten about it. I'll dig it out and brush the > cobwebs off ASAP. Thanks, I saw the series in my inbox. I will give a look today or tomorrow. > In the meantime can the test at the head of do_trap_hypervisor be > extended to DTRT with traps from EL2 (i.e. skip the rest of the > expression)? I plan to do smth like: if ( guest_mode(regs) && ... ) Regards,
On Tue, 2014-09-09 at 11:31 -0700, Julien Grall wrote: > Hi Ian, > > On 09/09/14 02:17, Ian Campbell wrote: > > On Mon, 2014-09-08 at 12:19 -0700, Julien Grall wrote: > >> Hi Ian, > >> > >> On 08/09/14 06:01, Ian Campbell wrote: > >>>> + /* PC is always 4-byte align, as Xen is using ARM instruction set */ > >>> > >>> "aligned" > >> > >> Ok. > >> > >>> Is it worth a check here? I presume the nested fault if PC were > >>> misaligned would be pretty exciting, print+goto die would seem > >>> appropriate. > >> > >> I don't think so. The "undefined instruction" is only used when the > >> processor is unable to decode the instruction. If it fails to load the > >> instruction (because a wrong memory address, pc misaligned), Xen would > >> have received a "prefetch abort". > > > > I was thinking of something either wrongly or rightly transitioning to > > thumb mode. It'd have to be something horrible like a broken firmware > > returning from a PSCI call in the wrong mode or something like that > > though. > > The thumb mode is stored in SPSR which is a banked registers. Your use > case can only happen if the secure mode decides to modify the registers > of the hypervisor. It would be very stupid from the firmware and other > bad things could happen. Yes, as I said it would need to be horribly broken firmware. I wouldn't rule out the ability of firmware to do horribly broken things though. > >>>> }> +#ifdef CONFIG_ARM_64 > >>>> +static void do_trap_brk(struct cpu_user_regs *regs, union hsr hsr) > >>>> +{ > >>>> + /* HCR_EL2.TGE and MDCR_EL2.TDE are not set so we never receive > >>>> + * software breakpoint exception for EL1 and EL0 here > >>>> + */ > >>> > >>> BUG_ON? ;-O > >> > >> Sounds a good plan. > > > > Just be careful that it won't recurse ;-) > > Panic should never go in exception mode, so I will use it instead. > > Regards, >
Hi Ian, On 10/09/14 02:35, Ian Campbell wrote: > On Tue, 2014-09-09 at 11:31 -0700, Julien Grall wrote: >> Hi Ian, >> >> On 09/09/14 02:17, Ian Campbell wrote: >>> On Mon, 2014-09-08 at 12:19 -0700, Julien Grall wrote: >>>> Hi Ian, >>>> >>>> On 08/09/14 06:01, Ian Campbell wrote: >>>>>> + /* PC is always 4-byte align, as Xen is using ARM instruction set */ >>>>> >>>>> "aligned" >>>> >>>> Ok. >>>> >>>>> Is it worth a check here? I presume the nested fault if PC were >>>>> misaligned would be pretty exciting, print+goto die would seem >>>>> appropriate. >>>> >>>> I don't think so. The "undefined instruction" is only used when the >>>> processor is unable to decode the instruction. If it fails to load the >>>> instruction (because a wrong memory address, pc misaligned), Xen would >>>> have received a "prefetch abort". >>> >>> I was thinking of something either wrongly or rightly transitioning to >>> thumb mode. It'd have to be something horrible like a broken firmware >>> returning from a PSCI call in the wrong mode or something like that >>> though. >> >> The thumb mode is stored in SPSR which is a banked registers. Your use >> case can only happen if the secure mode decides to modify the registers >> of the hypervisor. It would be very stupid from the firmware and other >> bad things could happen. > > Yes, as I said it would need to be horribly broken firmware. I wouldn't > rule out the ability of firmware to do horribly broken things though. Ok. I will add a check and directly go to die. Regards,
diff --git a/xen/arch/arm/arm32/traps.c b/xen/arch/arm/arm32/traps.c index ff0b945..baf8c41 100644 --- a/xen/arch/arm/arm32/traps.c +++ b/xen/arch/arm/arm32/traps.c @@ -18,6 +18,7 @@ #include <xen/config.h> #include <xen/lib.h> +#include <xen/kernel.h> #include <public/xen.h> @@ -25,7 +26,26 @@ asmlinkage void do_trap_undefined_instruction(struct cpu_user_regs *regs) { - do_unexpected_trap("Undefined Instruction", regs); + uint64_t pc = regs->pc; + uint32_t instr; + + if ( !is_kernel_text(pc) && + (system_state >= SYS_STATE_active || !is_kernel_inittext(pc)) ) + goto die; + + /* PC is always 4-byte align, as Xen is using ARM instruction set */ + instr = *((uint32_t *)regs->pc); + if ( instr != BUG_OPCODE ) + goto die; + + if ( do_bug_frame(regs, regs->pc) ) + goto die; + + regs->pc += 4; + return; + +die: + do_unexpected_trap("undefined instruction", regs); } asmlinkage void do_trap_supervisor_call(struct cpu_user_regs *regs) diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c index 76a9586..a97617e 100644 --- a/xen/arch/arm/traps.c +++ b/xen/arch/arm/traps.c @@ -32,6 +32,7 @@ #include <xen/domain_page.h> #include <public/sched.h> #include <public/xen.h> +#include <asm/debugger.h> #include <asm/event.h> #include <asm/regs.h> #include <asm/cpregs.h> @@ -1002,6 +1003,101 @@ void do_unexpected_trap(const char *msg, struct cpu_user_regs *regs) panic("CPU%d: Unexpected Trap: %s\n", smp_processor_id(), msg); } +int do_bug_frame(struct cpu_user_regs *regs, vaddr_t pc) +{ + const struct bug_frame *bug; + const char *prefix = "", *filename, *predicate; + unsigned long fixup; + int id, lineno; + static const struct bug_frame *const stop_frames[] = { + __stop_bug_frames_0, + __stop_bug_frames_1, + __stop_bug_frames_2, + NULL + }; + + for ( bug = __start_bug_frames, id = 0; stop_frames[id]; ++bug ) + { + while ( unlikely(bug == stop_frames[id]) ) + ++id; + + if ( ((vaddr_t)bug_loc(bug)) == pc ) + break; + } + + if ( !stop_frames[id] ) + return -ENOENT; + + /* WARN, BUG or ASSERT: decode the filename pointer and line number. */ + filename = bug_file(bug); + if ( !is_kernel(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 0; + + case BUGFRAME_bug: + printk("Xen BUG at %s%s:%d\n", prefix, filename, lineno); + + if ( debugger_trap_fatal(TRAP_invalid_op, regs) ) + return 0; + + show_execution_state(regs); + panic("Xen BUG at %s%s:%d", prefix, filename, lineno); + + case BUGFRAME_assert: + /* ASSERT: decode the predicate string pointer. */ + predicate = bug_msg(bug); + if ( !is_kernel(predicate) ) + predicate = "<unknown>"; + + printk("Assertion '%s' failed at %s%s:%d\n", + predicate, prefix, filename, lineno); + if ( debugger_trap_fatal(TRAP_invalid_op, regs) ) + return 0; + show_execution_state(regs); + panic("Assertion '%s' failed at %s%s:%d", + predicate, prefix, filename, lineno); + } + + return -EINVAL; +} + +#ifdef CONFIG_ARM_64 +static void do_trap_brk(struct cpu_user_regs *regs, union hsr hsr) +{ + /* HCR_EL2.TGE and MDCR_EL2.TDE are not set so we never receive + * software breakpoint exception for EL1 and EL0 here + */ + + switch (hsr.brk.comment) + { + case BRK_BUG_FRAME: + if ( do_bug_frame(regs, regs->pc) ) + goto die; + + regs->pc += 4; + + break; + + default: +die: + do_unexpected_trap("undefined breakpoint value", regs); + } +} +#endif + typedef register_t (*arm_hypercall_fn_t)( register_t, register_t, register_t, register_t, register_t); @@ -1875,6 +1971,13 @@ asmlinkage void do_trap_hypervisor(struct cpu_user_regs *regs) case HSR_EC_DATA_ABORT_LOWER_EL: do_trap_data_abort_guest(regs, hsr); break; + +#ifdef CONFIG_ARM_64 + case HSR_EC_BRK: + do_trap_brk(regs, hsr); + break; +#endif + default: bad_trap: printk("Hypervisor Trap. HSR=0x%x EC=0x%x IL=%x Syndrome=0x%"PRIx32"\n", diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S index 079e085..cca1d8c 100644 --- a/xen/arch/arm/xen.lds.S +++ b/xen/arch/arm/xen.lds.S @@ -40,6 +40,14 @@ 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 = .; *(.rodata) *(.rodata.*) _erodata = .; /* End of read-only data */ diff --git a/xen/include/asm-arm/arm32/bug.h b/xen/include/asm-arm/arm32/bug.h new file mode 100644 index 0000000..155b420 --- /dev/null +++ b/xen/include/asm-arm/arm32/bug.h @@ -0,0 +1,13 @@ +#ifndef __ARM_ARM32_BUG_H__ +#define __ARM_ARM32_BUG_H__ + +#include <xen/stringify.h> + +/* ARMv7 provides a list of undefined opcode (see A8.8.247 DDI 0406C.b) + * Use one them encoding A1 to go in exception mode + */ +#define BUG_OPCODE 0xe7f00f0 + +#define BUG_INSTR ".word " __stringify(BUG_OPCODE) + +#endif /* __ARM_ARM32_BUG_H__ */ diff --git a/xen/include/asm-arm/arm64/bug.h b/xen/include/asm-arm/arm64/bug.h new file mode 100644 index 0000000..42b0e4f --- /dev/null +++ b/xen/include/asm-arm/arm64/bug.h @@ -0,0 +1,10 @@ +#ifndef __ARM_ARM64_BUG_H__ +#define __ARM_ARM64_BUG_H__ + +#include <xen/stringify.h> + +#define BRK_BUG_FRAME 1 + +#define BUG_INSTR "brk " __stringify(BRK_BUG_FRAME) + +#endif /* __ARM_ARM64_BUG_H__ */ diff --git a/xen/include/asm-arm/bug.h b/xen/include/asm-arm/bug.h index 458c818..57442df 100644 --- a/xen/include/asm-arm/bug.h +++ b/xen/include/asm-arm/bug.h @@ -1,10 +1,81 @@ #ifndef __ARM_BUG_H__ #define __ARM_BUG_H__ -#define BUG() __bug(__FILE__, __LINE__) -#define WARN() __warn(__FILE__, __LINE__) +#include <asm/processor.h> -#endif /* __X86_BUG_H__ */ +#if defined(CONFIG_ARM_32) +# include <asm/arm32/bug.h> +#elif defined(CONFIG_ARM_64) +# include <asm/arm64/bug.h> +#else +# error "unknown ARM variant" +#endif + +#define BUG_DISP_WIDTH 24 +#define BUG_LINE_LO_WIDTH (31 - BUG_DISP_WIDTH) +#define BUG_LINE_HI_WIDTH (31 - BUG_DISP_WIDTH) + +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_warn 0 +#define BUGFRAME_bug 1 +#define BUGFRAME_assert 2 + +/* Many version 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. + */ +#define BUG_FRAME(type, line, file, has_msg, msg) do { \ + BUILD_BUG_ON((line) >> 16); \ + asm ("1:"BUG_INSTR"\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" \ + ".long (1b - 4b)\n" \ + ".long (2b - 4b)\n" \ + ".long (3b - 4b)\n" \ + ".hword " __stringify(line) ", 0\n" \ + ".popsection"); \ +} 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[]; + +int do_bug_frame(struct cpu_user_regs *regs, vaddr_t pc); + +#endif /* __ARM_BUG_H__ */ /* * Local variables: * mode: C diff --git a/xen/include/asm-arm/debugger.h b/xen/include/asm-arm/debugger.h index 916860b..ac776ef 100644 --- a/xen/include/asm-arm/debugger.h +++ b/xen/include/asm-arm/debugger.h @@ -1,7 +1,7 @@ #ifndef __ARM_DEBUGGER_H__ #define __ARM_DEBUGGER_H__ -#define debugger_trap_fatal(v, r) ((void) 0) +#define debugger_trap_fatal(v, r) (0) #define debugger_trap_immediate() ((void) 0) #endif /* __ARM_DEBUGGER_H__ */ diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h index 9d230f3..acaf566 100644 --- a/xen/include/asm-arm/processor.h +++ b/xen/include/asm-arm/processor.h @@ -3,6 +3,7 @@ #include <asm/cpregs.h> #include <asm/sysregs.h> +#include <public/arch-arm.h> /* MIDR Main ID Register */ #define MIDR_MASK 0xff0ffff0 @@ -122,6 +123,9 @@ #define HSR_EC_INSTR_ABORT_CURR_EL 0x21 #define HSR_EC_DATA_ABORT_LOWER_EL 0x24 #define HSR_EC_DATA_ABORT_CURR_EL 0x25 +#ifdef CONFIG_ARM_64 +#define HSR_EC_BRK 0x3c +#endif /* FSR format, common */ #define FSR_LPAE (_AC(1,UL)<<9) @@ -358,6 +362,17 @@ union hsr { unsigned long len:1; /* Instruction length */ unsigned long ec:6; /* Exception Class */ } dabt; /* HSR_EC_DATA_ABORT_* */ + +#ifdef CONFIG_ARM_64 + struct hsr_brk { + unsigned long comment:16; /* Comment */ + unsigned long res0:9; + unsigned long len:1; /* Instruction length */ + unsigned long ec:6; /* Exception Class */ + } brk; +#endif + + }; #endif @@ -487,7 +502,7 @@ void panic_PAR(uint64_t par); void show_execution_state(struct cpu_user_regs *regs); void show_registers(struct cpu_user_regs *regs); //#define dump_execution_state() run_in_exception_handler(show_execution_state) -#define dump_execution_state() asm volatile (".word 0xe7f000f0\n"); /* XXX */ +#define dump_execution_state() WARN() #define cpu_relax() barrier() /* Could yield? */
Currently the hypervisor will hang if it hits a WARN_ON. The implemention uses an undefined instruction, made ourself because ARM doesn't provide one, to implement BUG/ASSERT/WARN_ON, and sets up the different tables (one for each type) which contain useful information. This is based on the x86 implementation (include/asm-x86/bug.h). Unfortunately the structure can't be shared because many ARM{32,64} gcc versions doesn't correctly support %c. The support of executing a function in an exception handler is also keep unimplemented on ARM. Therefore, dump_execution_state is implement as WARN() The current opcode used to go in exception mode may not be undefined on ARM64. Use the instruction "brk" to generate a software debug exception. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- Changes in v2: - Missing static in do_bug_frame prototype - Add support for ARM64 by using the instruction brk - Implement dump_execution_state as WARN --- xen/arch/arm/arm32/traps.c | 22 ++++++++- xen/arch/arm/traps.c | 103 +++++++++++++++++++++++++++++++++++++++ xen/arch/arm/xen.lds.S | 8 +++ xen/include/asm-arm/arm32/bug.h | 13 +++++ xen/include/asm-arm/arm64/bug.h | 10 ++++ xen/include/asm-arm/bug.h | 77 +++++++++++++++++++++++++++-- xen/include/asm-arm/debugger.h | 2 +- xen/include/asm-arm/processor.h | 17 ++++++- 8 files changed, 246 insertions(+), 6 deletions(-) create mode 100644 xen/include/asm-arm/arm32/bug.h create mode 100644 xen/include/asm-arm/arm64/bug.h