From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: [PATCH v2 04/10] x86/misc: Early cleanup Date: Fri, 16 May 2014 11:39:36 +0100 Message-ID: <1400236782-28492-5-git-send-email-andrew.cooper3@citrix.com> References: <1400236782-28492-1-git-send-email-andrew.cooper3@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1400236782-28492-1-git-send-email-andrew.cooper3@citrix.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Xen-devel Cc: Andrew Cooper , Keir Fraser , Jan Beulich , Tim Deegan List-Id: xen-devel@lists.xenproject.org Various bits of cleanup without functional impact as far as the series goes, but make subsequent patches cleaner. * WARN_ON(1) is just WARN(). * Replace hand-crafted rolled stack printing with fatal_trap(). * 16 BSS bytes is overkill for an empty idtr to triple fault with. Construct it on the stack using an appropriate struct, and correct the asm memory constraint. * Fix watchdog asymmetry in panic(). machine_halt() needs just as much watchdog care as machine_restart(), but it should be up to the arch implementation of machine_{halt,restart}() to play with the watchdog. * unsigned and const correctness for trapstr(), along with whitespace cleanup. * Introduce BAD_VIRT_ADDR to cause #GP faults when used. Use in preference to a NULL pointer for double_fault's entry in the exception_table. Signed-off-by: Andrew Cooper CC: Keir Fraser CC: Jan Beulich CC: Tim Deegan -- v2: * More const correctness in trapstr(). * Reword commit message regarding panic() and watchdogs. * Reposition BAD_VIRT_ADDR in the series. --- xen/arch/x86/shutdown.c | 4 ++-- xen/arch/x86/traps.c | 44 +++++++++++++++++------------------------- xen/arch/x86/x86_64/entry.S | 2 +- xen/drivers/char/console.c | 5 ----- xen/include/asm-x86/config.h | 1 + 5 files changed, 22 insertions(+), 34 deletions(-) diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c index 44bcd7f..81dfadf 100644 --- a/xen/arch/x86/shutdown.c +++ b/xen/arch/x86/shutdown.c @@ -34,7 +34,6 @@ enum reboot_type { BOOT_CF9 = 'p', }; -static long no_idt[2]; static int reboot_mode; /* @@ -466,6 +465,7 @@ void machine_restart(unsigned int delay_millisecs) { unsigned int i, attempt; enum reboot_type orig_reboot_type = reboot_type; + const struct desc_ptr no_idt = { 0 }; watchdog_disable(); console_start_sync(); @@ -532,7 +532,7 @@ void machine_restart(unsigned int delay_millisecs) ? BOOT_ACPI : BOOT_TRIPLE); break; case BOOT_TRIPLE: - asm volatile ( "lidt %0 ; int3" : "=m" (no_idt) ); + asm volatile ("lidt %0; int3" : : "m" (no_idt)); reboot_type = BOOT_KBD; break; case BOOT_ACPI: diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index fcd5b00..33aa67f 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -375,21 +375,18 @@ void vcpu_show_execution_state(struct vcpu *v) vcpu_unpause(v); } -static char *trapstr(int trapnr) -{ - static char *strings[] = { - "divide error", "debug", "nmi", "bkpt", "overflow", "bounds", - "invalid opcode", "device not available", "double fault", - "coprocessor segment", "invalid tss", "segment not found", - "stack error", "general protection fault", "page fault", - "spurious interrupt", "coprocessor error", "alignment check", +static const char *trapstr(unsigned int trapnr) +{ + static const char * const strings[] = { + "divide error", "debug", "nmi", "bkpt", "overflow", "bounds", + "invalid opcode", "device not available", "double fault", + "coprocessor segment", "invalid tss", "segment not found", + "stack error", "general protection fault", "page fault", + "spurious interrupt", "coprocessor error", "alignment check", "machine check", "simd error" }; - if ( (trapnr < 0) || (trapnr >= ARRAY_SIZE(strings)) ) - return "???"; - - return strings[trapnr]; + return trapnr < ARRAY_SIZE(strings) ? strings[trapnr] : "???"; } /* @@ -1485,15 +1482,10 @@ void __init do_early_page_fault(struct cpu_user_regs *regs) if ( stuck++ == 1000 ) { - unsigned long *stk = (unsigned long *)regs; - printk("Early fatal page fault at %04x:%p (cr2=%p, ec=%04x)\n", + console_start_sync(); + printk("Early fatal page fault at %04x:%p (cr2=%p, ec=%04x)\n", regs->cs, _p(regs->eip), _p(cr2), regs->error_code); - show_page_walk(cr2); - printk("Stack dump: "); - while ( ((long)stk & ((PAGE_SIZE - 1) & ~(BYTES_PER_LONG - 1))) != 0 ) - printk("%p ", _p(*stk++)); - for ( ; ; ) - halt(); + fatal_trap(TRAP_page_fault, regs); } } @@ -3393,7 +3385,7 @@ void do_debug(struct cpu_user_regs *regs) } if ( !debugger_trap_fatal(TRAP_debug, regs) ) { - WARN_ON(1); + WARN(); regs->eflags &= ~X86_EFLAGS_TF; } } @@ -3508,11 +3500,11 @@ void __devinit percpu_traps_init(void) void __init trap_init(void) { /* - * Note that interrupt gates are always used, rather than trap gates. We - * must have interrupts disabled until DS/ES/FS/GS are saved because the - * first activation must have the "bad" value(s) for these registers and - * we may lose them if another activation is installed before they are - * saved. The page-fault handler also needs interrupts disabled until %cr2 + * Note that interrupt gates are always used, rather than trap gates. We + * must have interrupts disabled until DS/ES/FS/GS are saved because the + * first activation must have the "bad" value(s) for these registers and + * we may lose them if another activation is installed before they are + * saved. The page-fault handler also needs interrupts disabled until %cr2 * has been read and saved on the stack. */ set_intr_gate(TRAP_divide_error,÷_error); diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S index 4796e65..305054f 100644 --- a/xen/arch/x86/x86_64/entry.S +++ b/xen/arch/x86/x86_64/entry.S @@ -716,7 +716,7 @@ ENTRY(exception_table) .quad do_bounds .quad do_invalid_op .quad do_device_not_available - .quad 0 # double_fault + .quad BAD_VIRT_ADDR /* double_fault, IST entry */ .quad do_coprocessor_segment_overrun .quad do_invalid_TSS .quad do_segment_not_present diff --git a/xen/drivers/char/console.c b/xen/drivers/char/console.c index 50b4415..2f6c090 100644 --- a/xen/drivers/char/console.c +++ b/xen/drivers/char/console.c @@ -1124,14 +1124,9 @@ void panic(const char *fmt, ...) #endif if ( opt_noreboot ) - { machine_halt(); - } else - { - watchdog_disable(); machine_restart(5000); - } } void __bug(char *file, int line) diff --git a/xen/include/asm-x86/config.h b/xen/include/asm-x86/config.h index 02ab1fc..5df2fe2 100644 --- a/xen/include/asm-x86/config.h +++ b/xen/include/asm-x86/config.h @@ -106,6 +106,7 @@ /* Return value for zero-size _xmalloc(), distinguished from NULL. */ #define ZERO_BLOCK_PTR ((void *)0xBAD0BAD0BAD0BAD0UL) +#define BAD_VIRT_ADDR _AC(0x8000000000000000,UL) #ifndef __ASSEMBLY__ extern unsigned long trampoline_phys; -- 1.7.10.4