* [PATCH v2 01/10] x86/traps: Mnemonics for system descriptor types
2014-05-16 10:39 [PATCH v2 00/10] x86: Improvements to trap handling Andrew Cooper
@ 2014-05-16 10:39 ` Andrew Cooper
2014-05-16 10:39 ` [PATCH v2 02/10] x86/traps: Make panic and reboot paths safe during early boot Andrew Cooper
` (8 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Andrew Cooper @ 2014-05-16 10:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Keir Fraser, Jan Beulich
Avoids some particularly obscure magic numbers
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
---
v2:
* DESC_TYPE_xxx -> SYS_DESC_xxx
* Drop paretheses
* Style tweaks while changing code
---
xen/arch/x86/crash.c | 3 ++-
xen/arch/x86/traps.c | 8 ++++----
xen/arch/x86/x86_64/traps.c | 10 ++++------
xen/include/asm-x86/desc.h | 8 ++++++++
xen/include/asm-x86/ldt.h | 2 +-
5 files changed, 19 insertions(+), 12 deletions(-)
diff --git a/xen/arch/x86/crash.c b/xen/arch/x86/crash.c
index aed3b3e..c0b83df 100644
--- a/xen/arch/x86/crash.c
+++ b/xen/arch/x86/crash.c
@@ -149,7 +149,8 @@ static void nmi_shootdown_cpus(void)
* This update is safe from a security point of view, as this pcpu
* is never going to try to sysret back to a PV vcpu.
*/
- _set_gate_lower(&idt_tables[i][TRAP_nmi], 14, 0, &trap_nop);
+ _set_gate_lower(&idt_tables[i][TRAP_nmi],
+ SYS_DESC_irq_gate, 0, &trap_nop);
set_ist(&idt_tables[i][TRAP_machine_check], IST_NONE);
}
else
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 2cb3174..46cb48e 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -3431,8 +3431,8 @@ static void __set_intr_gate(unsigned int n, uint32_t dpl, void *addr)
/* Keep secondary tables in sync with IRQ updates. */
for ( i = 1; i < nr_cpu_ids; i++ )
if ( idt_tables[i] != NULL )
- _set_gate(&idt_tables[i][n], 14, dpl, addr);
- _set_gate(&idt_table[n], 14, dpl, addr);
+ _set_gate(&idt_tables[i][n], SYS_DESC_irq_gate, dpl, addr);
+ _set_gate(&idt_table[n], SYS_DESC_irq_gate, dpl, addr);
}
static void set_swint_gate(unsigned int n, void *addr)
@@ -3457,12 +3457,12 @@ void load_TR(void)
this_cpu(gdt_table) + TSS_ENTRY - FIRST_RESERVED_GDT_ENTRY,
(unsigned long)tss,
offsetof(struct tss_struct, __cacheline_filler) - 1,
- 9);
+ SYS_DESC_tss_avail);
_set_tssldt_desc(
this_cpu(compat_gdt_table) + TSS_ENTRY - FIRST_RESERVED_GDT_ENTRY,
(unsigned long)tss,
offsetof(struct tss_struct, __cacheline_filler) - 1,
- 11);
+ SYS_DESC_tss_busy);
/* Switch to non-compat GDT (which has B bit clear) to execute LTR. */
asm volatile (
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index 3a48478..8328a2c 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -390,14 +390,12 @@ void __devinit subarch_percpu_traps_init(void)
set_ist(&idt_table[TRAP_nmi], IST_NMI);
set_ist(&idt_table[TRAP_machine_check], IST_MCE);
- /*
- * The 32-on-64 hypercall entry vector is only accessible from ring 1.
- * Also note that this is a trap gate, not an interrupt gate.
- */
- _set_gate(idt_table+HYPERCALL_VECTOR, 15, 1, &compat_hypercall);
+ /* The 32-on-64 hypercall vector is only accessible from ring 1. */
+ _set_gate(idt_table + HYPERCALL_VECTOR,
+ SYS_DESC_trap_gate, 1, &compat_hypercall);
/* Fast trap for int80 (faster than taking the #GP-fixup path). */
- _set_gate(idt_table+0x80, 15, 3, &int80_direct_trap);
+ _set_gate(idt_table + 0x80, SYS_DESC_trap_gate, 3, &int80_direct_trap);
}
stack_bottom = (char *)get_stack_bottom();
diff --git a/xen/include/asm-x86/desc.h b/xen/include/asm-x86/desc.h
index 4edb834..8257167 100644
--- a/xen/include/asm-x86/desc.h
+++ b/xen/include/asm-x86/desc.h
@@ -98,6 +98,14 @@
#ifndef __ASSEMBLY__
+/* System Descriptor types for GDT and IDT entries. */
+#define SYS_DESC_ldt 2
+#define SYS_DESC_tss_avail 9
+#define SYS_DESC_tss_busy 11
+#define SYS_DESC_call_gate 12
+#define SYS_DESC_irq_gate 14
+#define SYS_DESC_trap_gate 15
+
struct desc_struct {
u32 a, b;
};
diff --git a/xen/include/asm-x86/ldt.h b/xen/include/asm-x86/ldt.h
index b6f7beb..aa77368 100644
--- a/xen/include/asm-x86/ldt.h
+++ b/xen/include/asm-x86/ldt.h
@@ -18,7 +18,7 @@ static inline void load_LDT(struct vcpu *v)
desc = (!is_pv_32on64_vcpu(v)
? this_cpu(gdt_table) : this_cpu(compat_gdt_table))
+ LDT_ENTRY - FIRST_RESERVED_GDT_ENTRY;
- _set_tssldt_desc(desc, LDT_VIRT_START(v), ents*8-1, 2);
+ _set_tssldt_desc(desc, LDT_VIRT_START(v), ents*8-1, SYS_DESC_ldt);
__asm__ __volatile__ ( "lldt %%ax" : : "a" (LDT_ENTRY << 3) );
}
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 02/10] x86/traps: Make panic and reboot paths safe during early boot
2014-05-16 10:39 [PATCH v2 00/10] x86: Improvements to trap handling Andrew Cooper
2014-05-16 10:39 ` [PATCH v2 01/10] x86/traps: Mnemonics for system descriptor types Andrew Cooper
@ 2014-05-16 10:39 ` Andrew Cooper
2014-05-16 10:39 ` [PATCH v2 03/10] x86/traps: Make the main trap handlers safe for use early during Xen boot Andrew Cooper
` (7 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Andrew Cooper @ 2014-05-16 10:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Keir Fraser, Jan Beulich, Tim Deegan
Reverse two conditions in show_registers(). For an early crash, it is not
safe to dereference 'current' for its HVM status before knowing that it is a
guest vcpu.
Introduce SYS_STATE_smp_boot to distinguish the point at which APs need
considering before boot is complete. There is one code change required as a
result; .init.text symbols are still in use before Xen is active, so alter its
predicate in is_active_kernel_text().
Make use of SYS_STATE_smp_boot in machine_{halt,restart}(). Before Xen starts
booting the APs, any execution here is certainly the BSP.
When halting or rebooting particularly early, this avoids the risks of a #PF
or #GP when accessing the LAPIC before generic_apic_probe(), as well as trying
to enable interrupts before init_IRQ() is complete.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
---
v2:
* Drop changes to map_domain_page_global() locking
* Rewrite patch description for clarity
---
xen/arch/x86/setup.c | 2 ++
xen/arch/x86/shutdown.c | 38 +++++++++++++++++++++++---------------
xen/arch/x86/x86_64/traps.c | 2 +-
xen/common/symbols.c | 2 +-
xen/include/xen/kernel.h | 1 +
5 files changed, 28 insertions(+), 17 deletions(-)
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index a2fe7e0..68f4bcc 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -1319,6 +1319,8 @@ void __init noreturn __start_xen(unsigned long mbi_p)
console_init_postirq();
+ system_state = SYS_STATE_smp_boot;
+
do_presmp_initcalls();
for_each_present_cpu ( i )
diff --git a/xen/arch/x86/shutdown.c b/xen/arch/x86/shutdown.c
index 827515d..44bcd7f 100644
--- a/xen/arch/x86/shutdown.c
+++ b/xen/arch/x86/shutdown.c
@@ -96,8 +96,13 @@ void machine_halt(void)
{
watchdog_disable();
console_start_sync();
- local_irq_enable();
- smp_call_function(__machine_halt, NULL, 0);
+
+ if ( system_state >= SYS_STATE_smp_boot )
+ {
+ local_irq_enable();
+ smp_call_function(__machine_halt, NULL, 0);
+ }
+
__machine_halt(NULL);
}
@@ -466,18 +471,6 @@ void machine_restart(unsigned int delay_millisecs)
console_start_sync();
spin_debug_disable();
- local_irq_enable();
-
- /* Ensure we are the boot CPU. */
- if ( get_apic_id() != boot_cpu_physical_apicid )
- {
- /* Send IPI to the boot CPU (logical cpu 0). */
- on_selected_cpus(cpumask_of(0), __machine_restart,
- &delay_millisecs, 0);
- for ( ; ; )
- halt();
- }
-
/*
* We may be called from an interrupt context, and various functions we
* may need to call (alloc_domheap_pages, map_domain_page, ...) assert that
@@ -485,7 +478,22 @@ void machine_restart(unsigned int delay_millisecs)
*/
local_irq_count(0) = 0;
- smp_send_stop();
+ if ( system_state >= SYS_STATE_smp_boot )
+ {
+ local_irq_enable();
+
+ /* Ensure we are the boot CPU. */
+ if ( get_apic_id() != boot_cpu_physical_apicid )
+ {
+ /* Send IPI to the boot CPU (logical cpu 0). */
+ on_selected_cpus(cpumask_of(0), __machine_restart,
+ &delay_millisecs, 0);
+ for ( ; ; )
+ halt();
+ }
+
+ smp_send_stop();
+ }
mdelay(delay_millisecs);
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index 8328a2c..af20e0b 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -86,7 +86,7 @@ void show_registers(struct cpu_user_regs *regs)
enum context context;
struct vcpu *v = current;
- if ( has_hvm_container_vcpu(v) && guest_mode(regs) )
+ if ( guest_mode(regs) && has_hvm_container_vcpu(v) )
{
struct segment_register sreg;
context = CTXT_hvm_guest;
diff --git a/xen/common/symbols.c b/xen/common/symbols.c
index 45941e1..bc2fde6 100644
--- a/xen/common/symbols.c
+++ b/xen/common/symbols.c
@@ -96,7 +96,7 @@ static unsigned int get_symbol_offset(unsigned long pos)
bool_t is_active_kernel_text(unsigned long addr)
{
return (is_kernel_text(addr) ||
- (system_state == SYS_STATE_boot && is_kernel_inittext(addr)));
+ (system_state < SYS_STATE_active && is_kernel_inittext(addr)));
}
const char *symbols_lookup(unsigned long addr,
diff --git a/xen/include/xen/kernel.h b/xen/include/xen/kernel.h
index 54e88dd..2c6d448 100644
--- a/xen/include/xen/kernel.h
+++ b/xen/include/xen/kernel.h
@@ -92,6 +92,7 @@ extern char _sinittext[], _einittext[];
extern enum system_state {
SYS_STATE_early_boot,
SYS_STATE_boot,
+ SYS_STATE_smp_boot,
SYS_STATE_active,
SYS_STATE_suspend,
SYS_STATE_resume
--
1.7.10.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 03/10] x86/traps: Make the main trap handlers safe for use early during Xen boot
2014-05-16 10:39 [PATCH v2 00/10] x86: Improvements to trap handling Andrew Cooper
2014-05-16 10:39 ` [PATCH v2 01/10] x86/traps: Mnemonics for system descriptor types Andrew Cooper
2014-05-16 10:39 ` [PATCH v2 02/10] x86/traps: Make panic and reboot paths safe during early boot Andrew Cooper
@ 2014-05-16 10:39 ` Andrew Cooper
2014-05-16 10:39 ` [PATCH v2 04/10] x86/misc: Early cleanup Andrew Cooper
` (6 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Andrew Cooper @ 2014-05-16 10:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Keir Fraser, Tim Deegan
Most of this patch is an analysis of the safety of the trap handlers.
Traps 0, 4, 5, 9-12, 16, 17 and 19 all end up in do_trap(). do_trap() is
mostly safe, performing an exception table search and possibly panic()s.
There is one complication with traps 16 and 19 which will see about calling
the fpu_exception_callback. This involves following current which is not
valid early on boot. The has_hvm_container_vcpu(curr) check is preceded with
a system_state check, so in the exceedingly unlikely case that Xen takes an
x87/SIMD trap while booting, it will panic() instead of following a bogus
current vcpu.
Traps 1, 3, 6-8, 13 and 15 are completely safe with respect to running during
early boot. They all have well formed and obvious differences between faults
in Xen and faults in guests, with the Xen faults doing little more than
exception table walks or panic()s.
Trap 2 is a complicated codepath, but appears safe. For the possible
injection of NMIs into dom0 there is a NULL domain pointer check. The
possible softirq raised for PCI SERR will be delivered until we start the idle
vcpu, but is safe.
Trap 14 is very complicated. The code is certainly unsafe for boot as
fixup_page_fault() will dereference current to find the running domain. There
exists an explicit do_early_page_fault() handler which shall continue to be
used.
Trap 18 has a default handler before the MCE infrastructure is set up, which
has always been unsafe and liable to deadlock itself with the console lock.
As it is expected never to trigger, and if it did we would be in serious
problems, the simple printk() is replaced with a fatal error path.
Trap 20 (Virtualisation Exception) is currently not implemented. It is fatal
one way or another, and will become more explicitly so with later changes.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
Reviewed-by: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
---
xen/arch/x86/cpu/mcheck/mce.c | 5 +++--
xen/arch/x86/traps.c | 3 ++-
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/xen/arch/x86/cpu/mcheck/mce.c b/xen/arch/x86/cpu/mcheck/mce.c
index a81952c..5488411 100644
--- a/xen/arch/x86/cpu/mcheck/mce.c
+++ b/xen/arch/x86/cpu/mcheck/mce.c
@@ -72,8 +72,9 @@ custom_param("mce_verbosity", mce_set_verbosity);
/* Handle unconfigured int18 (should never happen) */
static void unexpected_machine_check(struct cpu_user_regs *regs, long error_code)
{
- printk(XENLOG_ERR "CPU#%d: Unexpected int18 (Machine Check).\n",
- smp_processor_id());
+ console_force_unlock();
+ printk("Unexpected Machine Check Exception");
+ fatal_trap(TRAP_machine_check, regs);
}
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 46cb48e..fcd5b00 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -561,7 +561,8 @@ static void do_trap(struct cpu_user_regs *regs, int use_error_code)
}
if ( ((trapnr == TRAP_copro_error) || (trapnr == TRAP_simd_error)) &&
- has_hvm_container_vcpu(curr) && curr->arch.hvm_vcpu.fpu_exception_callback )
+ system_state >= SYS_STATE_active && has_hvm_container_vcpu(curr) &&
+ curr->arch.hvm_vcpu.fpu_exception_callback )
{
curr->arch.hvm_vcpu.fpu_exception_callback(
curr->arch.hvm_vcpu.fpu_exception_callback_arg, regs);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 04/10] x86/misc: Early cleanup
2014-05-16 10:39 [PATCH v2 00/10] x86: Improvements to trap handling Andrew Cooper
` (2 preceding siblings ...)
2014-05-16 10:39 ` [PATCH v2 03/10] x86/traps: Make the main trap handlers safe for use early during Xen boot Andrew Cooper
@ 2014-05-16 10:39 ` Andrew Cooper
2014-05-16 10:39 ` [PATCH v2 05/10] x86/traps: Functional prep work Andrew Cooper
` (5 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Andrew Cooper @ 2014-05-16 10:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Keir Fraser, Jan Beulich, Tim Deegan
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 <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
--
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
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 05/10] x86/traps: Functional prep work
2014-05-16 10:39 [PATCH v2 00/10] x86: Improvements to trap handling Andrew Cooper
` (3 preceding siblings ...)
2014-05-16 10:39 ` [PATCH v2 04/10] x86/misc: Early cleanup Andrew Cooper
@ 2014-05-16 10:39 ` Andrew Cooper
2014-05-16 10:39 ` [PATCH v2 06/10] x86/boot: Install trap handlers much earlier on boot Andrew Cooper
` (4 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Andrew Cooper @ 2014-05-16 10:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Keir Fraser, Jan Beulich, Tim Deegan
* Promote certain actions to earlier in __start_xen().
* Declare double_fault and early_page_fault as standard trap handlers.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
---
It would be nice if the linker could sort the exception tables for us, and the
Xen check could simply assert that it was sorted. Making this happen is not a
job for now.
v2:
* Don't introduce an explicitly latent this_cpu() bug before
percpu_area_init(), and implicit latent bug with subsequent changes.
---
xen/arch/x86/setup.c | 14 +++++++-------
xen/arch/x86/x86_64/traps.c | 1 -
xen/include/asm-x86/processor.h | 2 ++
xen/include/asm-x86/setup.h | 1 -
4 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 68f4bcc..82ce344 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -558,8 +558,15 @@ void __init noreturn __start_xen(unsigned long mbi_p)
.stop_bits = 1
};
+ set_processor_id(0);
+ set_current((struct vcpu *)0xfffff000); /* debug sanity. */
+ idle_vcpu[0] = current;
+
percpu_init_areas();
+ smp_prepare_boot_cpu();
+ sort_exception_tables();
+
set_intr_gate(TRAP_page_fault, &early_page_fault);
loader = (mbi->flags & MBI_LOADERNAME)
@@ -588,15 +595,10 @@ void __init noreturn __start_xen(unsigned long mbi_p)
parse_video_info();
- set_current((struct vcpu *)0xfffff000); /* debug sanity */
- idle_vcpu[0] = current;
- set_processor_id(0); /* needed early, for smp_processor_id() */
if ( cpu_has_efer )
rdmsrl(MSR_EFER, this_cpu(efer));
asm volatile ( "mov %%cr4,%0" : "=r" (this_cpu(cr4)) );
- smp_prepare_boot_cpu();
-
/* We initialise the serial devices very early so we can get debugging. */
ns16550.io_base = 0x3f8;
ns16550.irq = 4;
@@ -1212,8 +1214,6 @@ void __init noreturn __start_xen(unsigned long mbi_p)
if ( opt_watchdog )
nmi_watchdog = NMI_LOCAL_APIC;
- sort_exception_tables();
-
find_smp_config();
dmi_scan_machine();
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index af20e0b..898f9a0 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -223,7 +223,6 @@ void show_page_walk(unsigned long addr)
l1_table_offset(addr), l1e_get_intpte(l1e), pfn);
}
-void double_fault(void);
void do_double_fault(struct cpu_user_regs *regs)
{
unsigned int cpu;
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 35b2433..c9051be 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -500,12 +500,14 @@ DECLARE_TRAP_HANDLER(overflow);
DECLARE_TRAP_HANDLER(bounds);
DECLARE_TRAP_HANDLER(invalid_op);
DECLARE_TRAP_HANDLER(device_not_available);
+DECLARE_TRAP_HANDLER(double_fault);
DECLARE_TRAP_HANDLER(coprocessor_segment_overrun);
DECLARE_TRAP_HANDLER(invalid_TSS);
DECLARE_TRAP_HANDLER(segment_not_present);
DECLARE_TRAP_HANDLER(stack_segment);
DECLARE_TRAP_HANDLER(general_protection);
DECLARE_TRAP_HANDLER(page_fault);
+DECLARE_TRAP_HANDLER(early_page_fault);
DECLARE_TRAP_HANDLER(coprocessor_error);
DECLARE_TRAP_HANDLER(simd_coprocessor_error);
DECLARE_TRAP_HANDLER(machine_check);
diff --git a/xen/include/asm-x86/setup.h b/xen/include/asm-x86/setup.h
index 3039e1b..8f8c6f3 100644
--- a/xen/include/asm-x86/setup.h
+++ b/xen/include/asm-x86/setup.h
@@ -7,7 +7,6 @@ extern unsigned long xenheap_initial_phys_start;
void early_cpu_init(void);
void early_time_init(void);
-void early_page_fault(void);
int intel_cpu_init(void);
int amd_init_cpu(void);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 06/10] x86/boot: Install trap handlers much earlier on boot
2014-05-16 10:39 [PATCH v2 00/10] x86: Improvements to trap handling Andrew Cooper
` (4 preceding siblings ...)
2014-05-16 10:39 ` [PATCH v2 05/10] x86/traps: Functional prep work Andrew Cooper
@ 2014-05-16 10:39 ` Andrew Cooper
2014-05-16 10:39 ` [PATCH v2 07/10] x86/boot: Correct CR4 setup on APs Andrew Cooper
` (3 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Andrew Cooper @ 2014-05-16 10:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Keir Fraser, Jan Beulich, Tim Deegan
Patch the trap handlers into the master idt very early on boot, and setup &
load the GDT, IDT, TR and LDT. Load the IDT before the TR so we stand a chance
of catching an invalid TSS exception rather than triple faulting.
This provides full exception support far earlier on boot than previously.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
---
v2:
* Load IDT before TR to catch #TS rather than triple faulting.
* spaces/tabs, comment formatting.
---
xen/arch/x86/cpu/common.c | 70 +++++++++++++++++++++++++++++++++---------
xen/arch/x86/setup.c | 7 ++++-
xen/arch/x86/smpboot.c | 21 ++++---------
xen/arch/x86/traps.c | 23 ++++++++++++--
xen/arch/x86/x86_64/traps.c | 26 ----------------
xen/include/asm-x86/system.h | 1 +
xen/include/xen/sched.h | 1 +
7 files changed, 90 insertions(+), 59 deletions(-)
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index 4122684..dcd2ca1 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -515,6 +515,61 @@ void __init early_cpu_init(void)
centaur_init_cpu();
early_cpu_detect();
}
+
+/*
+ * Sets up system tables and descriptors.
+ *
+ * - Sets up TSS with stack pointers, including ISTs
+ * - Inserts TSS selector into regular and compat GDTs
+ * - Loads GDT, IDT, TR then null LDT
+ */
+void __cpuinit load_system_tables(void)
+{
+ unsigned int cpu = smp_processor_id();
+ unsigned long stack_bottom = get_stack_bottom(),
+ stack_top = stack_bottom & ~(STACK_SIZE - 1);
+
+ struct tss_struct *tss = &this_cpu(init_tss);
+ struct desc_struct *gdt =
+ this_cpu(gdt_table) - FIRST_RESERVED_GDT_ENTRY;
+ struct desc_struct *compat_gdt =
+ this_cpu(compat_gdt_table) - FIRST_RESERVED_GDT_ENTRY;
+
+ const struct desc_ptr gdtr = {
+ .base = (unsigned long)gdt,
+ .limit = LAST_RESERVED_GDT_BYTE,
+ };
+ const struct desc_ptr idtr = {
+ .base = (unsigned long)idt_tables[cpu],
+ .limit = (IDT_ENTRIES * sizeof(idt_entry_t)) - 1,
+ };
+
+ /* Main stack for interrupts/exceptions. */
+ tss->rsp0 = stack_bottom;
+ tss->bitmap = IOBMP_INVALID_OFFSET;
+
+ /* MCE, NMI and Double Fault handlers get their own stacks. */
+ tss->ist[IST_MCE - 1] = stack_top + IST_MCE * PAGE_SIZE;
+ tss->ist[IST_DF - 1] = stack_top + IST_DF * PAGE_SIZE;
+ tss->ist[IST_NMI - 1] = stack_top + IST_NMI * PAGE_SIZE;
+
+ _set_tssldt_desc(
+ gdt + TSS_ENTRY,
+ (unsigned long)tss,
+ offsetof(struct tss_struct, __cacheline_filler) - 1,
+ SYS_DESC_tss_avail);
+ _set_tssldt_desc(
+ compat_gdt + TSS_ENTRY,
+ (unsigned long)tss,
+ offsetof(struct tss_struct, __cacheline_filler) - 1,
+ SYS_DESC_tss_busy);
+
+ asm volatile ("lgdt %0" : : "m" (gdtr) );
+ asm volatile ("lidt %0" : : "m" (idtr) );
+ asm volatile ("ltr %w0" : : "rm" (TSS_ENTRY << 3) );
+ asm volatile ("lldt %w0" : : "rm" (0) );
+}
+
/*
* cpu_init() initializes state that is per-CPU. Some data is already
* initialized (naturally) in the bootstrap process, such as the GDT
@@ -524,11 +579,6 @@ void __init early_cpu_init(void)
void __cpuinit cpu_init(void)
{
int cpu = smp_processor_id();
- struct tss_struct *t = &this_cpu(init_tss);
- struct desc_ptr gdt_desc = {
- .base = (unsigned long)(this_cpu(gdt_table) - FIRST_RESERVED_GDT_ENTRY),
- .limit = LAST_RESERVED_GDT_BYTE
- };
if (cpumask_test_and_set_cpu(cpu, &cpu_initialized)) {
printk(KERN_WARNING "CPU#%d already initialized!\n", cpu);
@@ -543,22 +593,12 @@ void __cpuinit cpu_init(void)
/* Install correct page table. */
write_ptbase(current);
- asm volatile ( "lgdt %0" : : "m" (gdt_desc) );
-
/* No nested task. */
asm volatile ("pushf ; andw $0xbfff,(%"__OP"sp) ; popf" );
/* Ensure FPU gets initialised for each domain. */
stts();
- /* Set up and load the per-CPU TSS and LDT. */
- t->bitmap = IOBMP_INVALID_OFFSET;
- /* Bottom-of-stack must be 16-byte aligned! */
- BUG_ON((get_stack_bottom() & 15) != 0);
- t->rsp0 = get_stack_bottom();
- load_TR();
- asm volatile ( "lldt %%ax" : : "a" (0) );
-
/* Clear all 6 debug registers: */
#define CD(register) asm volatile ( "mov %0,%%db" #register : : "r"(0UL) );
CD(0); CD(1); CD(2); CD(3); /* no db4 and db5 */; CD(6); CD(7);
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 82ce344..5fc71d5 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -558,16 +558,21 @@ void __init noreturn __start_xen(unsigned long mbi_p)
.stop_bits = 1
};
+ /* Critical region without IDT or TSS. Any fault is deadly! */
+
set_processor_id(0);
set_current((struct vcpu *)0xfffff000); /* debug sanity. */
idle_vcpu[0] = current;
percpu_init_areas();
+ init_idt_traps();
+ load_system_tables();
+
smp_prepare_boot_cpu();
sort_exception_tables();
- set_intr_gate(TRAP_page_fault, &early_page_fault);
+ /* Full exception support from here on in. */
loader = (mbi->flags & MBI_LOADERNAME)
? (char *)__va(mbi->boot_loader_name) : "unknown";
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 5014397..c2c8752 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -303,15 +303,6 @@ static void set_cpu_sibling_map(int cpu)
}
}
-static void construct_percpu_idt(unsigned int cpu)
-{
- unsigned char idt_load[10];
-
- *(unsigned short *)(&idt_load[0]) = (IDT_ENTRIES*sizeof(idt_entry_t))-1;
- *(unsigned long *)(&idt_load[2]) = (unsigned long)idt_tables[cpu];
- __asm__ __volatile__ ( "lidt %0" : "=m" (idt_load) );
-}
-
void start_secondary(void *unused)
{
/*
@@ -320,6 +311,8 @@ void start_secondary(void *unused)
*/
unsigned int cpu = booting_cpu;
+ /* Critical region without IDT or TSS. Any fault is deadly! */
+
set_processor_id(cpu);
set_current(idle_vcpu[cpu]);
this_cpu(curr_vcpu) = idle_vcpu[cpu];
@@ -345,6 +338,10 @@ void start_secondary(void *unused)
*/
spin_debug_disable();
+ load_system_tables();
+
+ /* Full exception support from here on in. */
+
percpu_traps_init();
init_percpu_time();
@@ -353,12 +350,6 @@ void start_secondary(void *unused)
smp_callin();
- /*
- * At this point, boot CPU has fully initialised the IDT. It is
- * now safe to make ourselves a private copy.
- */
- construct_percpu_idt(cpu);
-
setup_secondary_APIC_clock();
/*
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 33aa67f..136821f 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -3497,7 +3497,7 @@ void __devinit percpu_traps_init(void)
ler_enable();
}
-void __init trap_init(void)
+void __init init_idt_traps(void)
{
/*
* Note that interrupt gates are always used, rather than trap gates. We
@@ -3515,23 +3515,42 @@ void __init trap_init(void)
set_intr_gate(TRAP_bounds,&bounds);
set_intr_gate(TRAP_invalid_op,&invalid_op);
set_intr_gate(TRAP_no_device,&device_not_available);
+ set_intr_gate(TRAP_double_fault,&double_fault);
set_intr_gate(TRAP_copro_seg,&coprocessor_segment_overrun);
set_intr_gate(TRAP_invalid_tss,&invalid_TSS);
set_intr_gate(TRAP_no_segment,&segment_not_present);
set_intr_gate(TRAP_stack_error,&stack_segment);
set_intr_gate(TRAP_gp_fault,&general_protection);
- set_intr_gate(TRAP_page_fault,&page_fault);
+ set_intr_gate(TRAP_page_fault,&early_page_fault);
set_intr_gate(TRAP_spurious_int,&spurious_interrupt_bug);
set_intr_gate(TRAP_copro_error,&coprocessor_error);
set_intr_gate(TRAP_alignment_check,&alignment_check);
set_intr_gate(TRAP_machine_check,&machine_check);
set_intr_gate(TRAP_simd_error,&simd_coprocessor_error);
+ /* Specify dedicated interrupt stacks for NMI, #DF, and #MC. */
+ set_ist(&idt_table[TRAP_double_fault], IST_DF);
+ set_ist(&idt_table[TRAP_nmi], IST_NMI);
+ set_ist(&idt_table[TRAP_machine_check], IST_MCE);
+
/* CPU0 uses the master IDT. */
idt_tables[0] = idt_table;
this_cpu(gdt_table) = boot_cpu_gdt_table;
this_cpu(compat_gdt_table) = boot_cpu_compat_gdt_table;
+}
+
+void __init trap_init(void)
+{
+ /* Replace early pagefault with real pagefault handler. */
+ set_intr_gate(TRAP_page_fault, &page_fault);
+
+ /* The 32-on-64 hypercall vector is only accessible from ring 1. */
+ _set_gate(idt_table + HYPERCALL_VECTOR,
+ SYS_DESC_trap_gate, 1, &compat_hypercall);
+
+ /* Fast trap for int80 (faster than taking the #GP-fixup path). */
+ _set_gate(idt_table + 0x80, SYS_DESC_trap_gate, 3, &int80_direct_trap);
percpu_traps_init();
diff --git a/xen/arch/x86/x86_64/traps.c b/xen/arch/x86/x86_64/traps.c
index 898f9a0..d09b6b6 100644
--- a/xen/arch/x86/x86_64/traps.c
+++ b/xen/arch/x86/x86_64/traps.c
@@ -379,23 +379,6 @@ static int write_stack_trampoline(
void __devinit subarch_percpu_traps_init(void)
{
char *stack_bottom, *stack;
- int cpu = smp_processor_id();
-
- if ( cpu == 0 )
- {
- /* Specify dedicated interrupt stacks for NMI, #DF, and #MC. */
- set_intr_gate(TRAP_double_fault, &double_fault);
- set_ist(&idt_table[TRAP_double_fault], IST_DF);
- set_ist(&idt_table[TRAP_nmi], IST_NMI);
- set_ist(&idt_table[TRAP_machine_check], IST_MCE);
-
- /* The 32-on-64 hypercall vector is only accessible from ring 1. */
- _set_gate(idt_table + HYPERCALL_VECTOR,
- SYS_DESC_trap_gate, 1, &compat_hypercall);
-
- /* Fast trap for int80 (faster than taking the #GP-fixup path). */
- _set_gate(idt_table + 0x80, SYS_DESC_trap_gate, 3, &int80_direct_trap);
- }
stack_bottom = (char *)get_stack_bottom();
stack = (char *)((unsigned long)stack_bottom & ~(STACK_SIZE - 1));
@@ -403,15 +386,6 @@ void __devinit subarch_percpu_traps_init(void)
/* IST_MAX IST pages + 1 syscall page + 1 guard page + primary stack. */
BUILD_BUG_ON((IST_MAX + 2) * PAGE_SIZE + PRIMARY_STACK_SIZE > STACK_SIZE);
- /* Machine Check handler has its own per-CPU 4kB stack. */
- this_cpu(init_tss).ist[IST_MCE-1] = (unsigned long)&stack[IST_MCE * PAGE_SIZE];
-
- /* Double-fault handler has its own per-CPU 4kB stack. */
- this_cpu(init_tss).ist[IST_DF-1] = (unsigned long)&stack[IST_DF * PAGE_SIZE];
-
- /* NMI handler has its own per-CPU 4kB stack. */
- this_cpu(init_tss).ist[IST_NMI-1] = (unsigned long)&stack[IST_NMI * PAGE_SIZE];
-
/* Trampoline for SYSCALL entry from long mode. */
stack = &stack[IST_MAX * PAGE_SIZE]; /* Skip the IST stacks. */
wrmsrl(MSR_LSTAR, (unsigned long)stack);
diff --git a/xen/include/asm-x86/system.h b/xen/include/asm-x86/system.h
index e9602aa..c5e482a 100644
--- a/xen/include/asm-x86/system.h
+++ b/xen/include/asm-x86/system.h
@@ -179,6 +179,7 @@ static inline int local_irq_is_enabled(void)
#define BROKEN_INIT_AFTER_S1 0x0002
void trap_init(void);
+void init_idt_traps(void);
void percpu_traps_init(void);
void subarch_percpu_traps_init(void);
diff --git a/xen/include/xen/sched.h b/xen/include/xen/sched.h
index 44851ae..acbe117 100644
--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -770,6 +770,7 @@ void domain_unpause(struct domain *d);
void domain_pause_by_systemcontroller(struct domain *d);
void domain_unpause_by_systemcontroller(struct domain *d);
void cpu_init(void);
+void load_system_tables(void);
struct scheduler;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 07/10] x86/boot: Correct CR4 setup on APs
2014-05-16 10:39 [PATCH v2 00/10] x86: Improvements to trap handling Andrew Cooper
` (5 preceding siblings ...)
2014-05-16 10:39 ` [PATCH v2 06/10] x86/boot: Install trap handlers much earlier on boot Andrew Cooper
@ 2014-05-16 10:39 ` Andrew Cooper
2014-05-16 10:39 ` [PATCH v2 08/10] x86/boot: Drop pre-C IDT patching Andrew Cooper
` (2 subsequent siblings)
9 siblings, 0 replies; 15+ messages in thread
From: Andrew Cooper @ 2014-05-16 10:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Keir Fraser, Jan Beulich, Tim Deegan
It is not safe to load mmu_cr4_features into cr4 early on AP start. Features
such as MCE require an int 0x18 handler to be set up.
Instead, load the minimum Xen CR4 features early but defer loading the full
'mmu_cr4_features' set until after the IDT has been set up.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
---
xen/arch/x86/boot/x86_64.S | 4 ++--
xen/arch/x86/setup.c | 3 +--
xen/arch/x86/smpboot.c | 4 +++-
xen/include/asm-x86/processor.h | 2 ++
4 files changed, 8 insertions(+), 5 deletions(-)
diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
index 67dfef9..417623f 100644
--- a/xen/arch/x86/boot/x86_64.S
+++ b/xen/arch/x86/boot/x86_64.S
@@ -9,8 +9,8 @@
mov %ecx,%gs
mov %ecx,%ss
- /* Enable full CR4 features. */
- mov mmu_cr4_features(%rip),%rcx
+ /* Enable minimal CR4 features. */
+ mov $XEN_MINIMAL_CR4,%rcx
mov %rcx,%cr4
mov stack_start(%rip),%rsp
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index 5fc71d5..b2a808a 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -100,8 +100,7 @@ char __attribute__ ((__section__(".bss.stack_aligned"))) cpu0_stack[STACK_SIZE];
struct cpuinfo_x86 __read_mostly boot_cpu_data = { 0, 0, 0, 0, -1 };
-unsigned long __read_mostly mmu_cr4_features =
- X86_CR4_PSE | X86_CR4_PGE | X86_CR4_PAE;
+unsigned long __read_mostly mmu_cr4_features = XEN_MINIMAL_CR4;
bool_t __initdata acpi_disabled;
bool_t __initdata acpi_force;
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index c2c8752..84f2d25 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -318,7 +318,6 @@ void start_secondary(void *unused)
this_cpu(curr_vcpu) = idle_vcpu[cpu];
if ( cpu_has_efer )
rdmsrl(MSR_EFER, this_cpu(efer));
- asm volatile ( "mov %%cr4,%0" : "=r" (this_cpu(cr4)) );
/*
* Just as during early bootstrap, it is convenient here to disable
@@ -342,6 +341,9 @@ void start_secondary(void *unused)
/* Full exception support from here on in. */
+ /* Safe to enable feature such as CR4.MCE with the IDT set up now. */
+ write_cr4(mmu_cr4_features);
+
percpu_traps_init();
init_percpu_time();
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index c9051be..805ec34 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -141,6 +141,8 @@
#define PFEC_page_paged (1U<<5)
#define PFEC_page_shared (1U<<6)
+#define XEN_MINIMAL_CR4 (X86_CR4_PSE | X86_CR4_PGE | X86_CR4_PAE)
+
#define XEN_SYSCALL_MASK (X86_EFLAGS_AC|X86_EFLAGS_VM|X86_EFLAGS_RF| \
X86_EFLAGS_NT|X86_EFLAGS_DF|X86_EFLAGS_IF| \
X86_EFLAGS_TF)
--
1.7.10.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 08/10] x86/boot: Drop pre-C IDT patching
2014-05-16 10:39 [PATCH v2 00/10] x86: Improvements to trap handling Andrew Cooper
` (6 preceding siblings ...)
2014-05-16 10:39 ` [PATCH v2 07/10] x86/boot: Correct CR4 setup on APs Andrew Cooper
@ 2014-05-16 10:39 ` Andrew Cooper
2014-05-16 10:39 ` [PATCH v2 09/10] x86/irqs: Move interrupt-stub generation out of C Andrew Cooper
2014-05-16 10:39 ` [PATCH v2 10/10] x86/misc: Post cleanup Andrew Cooper
9 siblings, 0 replies; 15+ messages in thread
From: Andrew Cooper @ 2014-05-16 10:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Keir Fraser, Jan Beulich, Tim Deegan
It is not needed now that __start_xen sets itself up with complete trap
handlers as its first action. This fixes a potential issue introduced in
c/s 7e510a7b874
"x86/boot: move some __high_start code and data into init sections"
which would leave ignore_int (in the .init section) patched into the reserved
exceptions in all IDTs.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
---
xen/arch/x86/boot/x86_64.S | 56 +-------------------------------------------
1 file changed, 1 insertion(+), 55 deletions(-)
diff --git a/xen/arch/x86/boot/x86_64.S b/xen/arch/x86/boot/x86_64.S
index 417623f..bfbafd2 100644
--- a/xen/arch/x86/boot/x86_64.S
+++ b/xen/arch/x86/boot/x86_64.S
@@ -25,64 +25,15 @@
leaq 1f(%rip),%rax
pushq %rax
lretq
-1: lidt idt_descr(%rip)
-
+1:
test %ebx,%ebx
jnz start_secondary
- jmp start_bsp
-
- .section .init.text, "ax", @progbits
-
-start_bsp:
- /* Initialise IDT with simple error defaults. */
- leaq ignore_int(%rip),%rcx
- movl %ecx,%eax
- andl $0xFFFF0000,%eax
- orl $0x00008E00,%eax
- shlq $32,%rax
- movl %ecx,%edx
- andl $0x0000FFFF,%edx
- orl $(__HYPERVISOR_CS64<<16),%edx
- orq %rdx,%rax
- shrq $32,%rcx
- movl %ecx,%edx
- leaq idt_table(%rip),%rdi
- movl $256,%ecx
-1: movq %rax,(%rdi)
- movq %rdx,8(%rdi)
- addq $16,%rdi
- loop 1b
/* Pass off the Multiboot info structure to C land. */
mov multiboot_ptr(%rip),%edi
call __start_xen
ud2 /* Force a panic (invalid opcode). */
-/* This is the default interrupt handler. */
-ignore_int:
- SAVE_ALL CLAC
- movq %cr2,%rsi
- leaq int_msg(%rip),%rdi
- xorl %eax,%eax
- call printk
- movq %rsp,%rbp
-0: movq (%rbp),%rsi
- addq $8,%rbp
- leaq hex_msg(%rip),%rdi
- xorl %eax,%eax
- call printk
- testq $0xff8,%rbp
- jnz 0b
-1: hlt
- jmp 1b
-
- .section .init.rodata, "a", @progbits
-
-int_msg:
- .asciz "Unknown interrupt (cr2=%016lx)\n"
-hex_msg:
- .asciz " %016lx"
-
/*** DESCRIPTOR TABLES ***/
.data
@@ -95,11 +46,6 @@ GLOBAL(gdt_descr)
.word LAST_RESERVED_GDT_BYTE
.quad boot_cpu_gdt_table - FIRST_RESERVED_GDT_BYTE
- .word 0,0,0
-GLOBAL(idt_descr)
- .word 256*16-1
- .quad idt_table
-
GLOBAL(stack_start)
.quad cpu0_stack
--
1.7.10.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH v2 09/10] x86/irqs: Move interrupt-stub generation out of C
2014-05-16 10:39 [PATCH v2 00/10] x86: Improvements to trap handling Andrew Cooper
` (7 preceding siblings ...)
2014-05-16 10:39 ` [PATCH v2 08/10] x86/boot: Drop pre-C IDT patching Andrew Cooper
@ 2014-05-16 10:39 ` Andrew Cooper
2014-05-16 15:06 ` Jan Beulich
2014-05-16 10:39 ` [PATCH v2 10/10] x86/misc: Post cleanup Andrew Cooper
9 siblings, 1 reply; 15+ messages in thread
From: Andrew Cooper @ 2014-05-16 10:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Feng Wu, Keir Fraser, Jan Beulich, Tim Deegan
Also generate stubs for reserved exceptions, which find their way to
do_reserved_exception() along with their entry vector for informative
purposes. Currently there are no reserved exceptions which have an error
code automatically pushed onto the stack.
* Move all automatic stub generation out of i8259.c and into irqgen.S.
* Move patching of the master IDT into trap_init(), and provide ASSERT()s to
ensure we have fully populated the IDT, and don't accidentally clobbered
any preexisting traps.
* Demote TRAP_copro_seg and TRAP_spurious_int to being reserved exceptions,
and remove their custom entry points.
* Acquaint Xen with #VE but leave it reserved.
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
CC: Feng Wu <feng.wu@intel.com>
---
v2:
* Far less .macro hackary, inspired by Feng's patch of a similar nature. On
consideration, having literal symbol names is very little use, either in
the disassembly, or running hypervisor. In hindsight this is new version is
substantially more likely to compile under clang, although I don't have a
compiler to hand.
---
xen/arch/x86/i8259.c | 69 +--------------------------------------
xen/arch/x86/traps.c | 36 +++++++++++++++-----
xen/arch/x86/x86_64/Makefile | 1 +
xen/arch/x86/x86_64/entry.S | 20 +++++-------
xen/arch/x86/x86_64/irqgen.S | 52 +++++++++++++++++++++++++++++
xen/include/asm-x86/asm_defns.h | 5 ---
xen/include/asm-x86/processor.h | 4 +--
7 files changed, 92 insertions(+), 95 deletions(-)
create mode 100644 xen/arch/x86/x86_64/irqgen.S
diff --git a/xen/arch/x86/i8259.c b/xen/arch/x86/i8259.c
index 9fec490..9f99995 100644
--- a/xen/arch/x86/i8259.c
+++ b/xen/arch/x86/i8259.c
@@ -23,65 +23,6 @@
#include <io_ports.h>
/*
- * Common place to define all x86 IRQ vectors
- *
- * This builds up the IRQ handler stubs using some ugly macros in irq.h
- *
- * These macros create the low-level assembly IRQ routines that save
- * register context and call do_IRQ(). do_IRQ() then does all the
- * operations that are needed to keep the AT (or SMP IOAPIC)
- * interrupt-controller happy.
- */
-
-__asm__(".section .text");
-
-#define IRQ_NAME(nr) VEC##nr##_interrupt
-
-#define BI(nr) \
-void IRQ_NAME(nr)(void); \
-__asm__( \
-".if " STR(0x##nr) " >= " STR(FIRST_DYNAMIC_VECTOR) "\n" \
-__ALIGN_STR "\n" \
-STR(IRQ_NAME(nr)) ":\n\t" \
-BUILD_IRQ(0x##nr) "\n" \
-".else\n" \
-".equ " STR(IRQ_NAME(nr)) ", 0\n" \
-".endif\n")
-
-#define BUILD_16_IRQS(x) \
- BI(x##0); BI(x##1); BI(x##2); BI(x##3); \
- BI(x##4); BI(x##5); BI(x##6); BI(x##7); \
- BI(x##8); BI(x##9); BI(x##a); BI(x##b); \
- BI(x##c); BI(x##d); BI(x##e); BI(x##f)
-
-BUILD_16_IRQS(0); BUILD_16_IRQS(1); BUILD_16_IRQS(2); BUILD_16_IRQS(3);
-BUILD_16_IRQS(4); BUILD_16_IRQS(5); BUILD_16_IRQS(6); BUILD_16_IRQS(7);
-BUILD_16_IRQS(8); BUILD_16_IRQS(9); BUILD_16_IRQS(a); BUILD_16_IRQS(b);
-BUILD_16_IRQS(c); BUILD_16_IRQS(d); BUILD_16_IRQS(e); BUILD_16_IRQS(f);
-
-#undef BUILD_16_IRQS
-#undef BI
-
-
-#define IRQ(x,y) IRQ_NAME(x##y)
-
-#define IRQLIST_16(x) \
- IRQ(x,0), IRQ(x,1), IRQ(x,2), IRQ(x,3), \
- IRQ(x,4), IRQ(x,5), IRQ(x,6), IRQ(x,7), \
- IRQ(x,8), IRQ(x,9), IRQ(x,a), IRQ(x,b), \
- IRQ(x,c), IRQ(x,d), IRQ(x,e), IRQ(x,f)
-
-static void (*__initdata interrupt[NR_VECTORS])(void) = {
- IRQLIST_16(0), IRQLIST_16(1), IRQLIST_16(2), IRQLIST_16(3),
- IRQLIST_16(4), IRQLIST_16(5), IRQLIST_16(6), IRQLIST_16(7),
- IRQLIST_16(8), IRQLIST_16(9), IRQLIST_16(a), IRQLIST_16(b),
- IRQLIST_16(c), IRQLIST_16(d), IRQLIST_16(e), IRQLIST_16(f)
-};
-
-#undef IRQ
-#undef IRQLIST_16
-
-/*
* This is the 'legacy' 8259A Programmable Interrupt Controller,
* present in the majority of PC/AT boxes.
* plus some generic x86 specific things if generic specifics makes
@@ -395,7 +336,7 @@ static struct irqaction __read_mostly cascade = { no_action, "cascade", NULL};
void __init init_IRQ(void)
{
- int vector, irq, cpu = smp_processor_id();
+ int irq, cpu = smp_processor_id();
init_bsp_APIC();
@@ -403,14 +344,6 @@ void __init init_IRQ(void)
BUG_ON(init_irq_data() < 0);
- for ( vector = FIRST_DYNAMIC_VECTOR; vector < NR_VECTORS; vector++ )
- {
- if (vector == HYPERCALL_VECTOR || vector == LEGACY_SYSCALL_VECTOR)
- continue;
- BUG_ON(!interrupt[vector]);
- set_intr_gate(vector, interrupt[vector]);
- }
-
for (irq = 0; platform_legacy_irq(irq); irq++) {
struct irq_desc *desc = irq_to_desc(irq);
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 136821f..2928743 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -383,7 +383,7 @@ static const char *trapstr(unsigned int trapnr)
"coprocessor segment", "invalid tss", "segment not found",
"stack error", "general protection fault", "page fault",
"spurious interrupt", "coprocessor error", "alignment check",
- "machine check", "simd error"
+ "machine check", "simd error", "virtualisation exception"
};
return trapnr < ARRAY_SIZE(strings) ? strings[trapnr] : "???";
@@ -534,6 +534,15 @@ int set_guest_nmi_trapbounce(void)
return !null_trap_bounce(v, tb);
}
+void do_reserved_exception(struct cpu_user_regs *regs)
+{
+ unsigned int trapnr = regs->entry_vector;
+
+ DEBUGGER_trap_fatal(trapnr, regs);
+ show_execution_state(regs);
+ panic("FATAL RESERVED TRAP %#x: %s", trapnr, trapstr(trapnr));
+}
+
static void do_trap(struct cpu_user_regs *regs, int use_error_code)
{
struct vcpu *curr = current;
@@ -589,7 +598,6 @@ void do_##name(struct cpu_user_regs *regs) \
DO_ERROR_NOCODE(divide_error)
DO_ERROR_NOCODE(overflow)
DO_ERROR_NOCODE(bounds)
-DO_ERROR_NOCODE(coprocessor_segment_overrun)
DO_ERROR( invalid_TSS)
DO_ERROR( segment_not_present)
DO_ERROR( stack_segment)
@@ -3414,10 +3422,6 @@ void do_debug(struct cpu_user_regs *regs)
return;
}
-void do_spurious_interrupt_bug(struct cpu_user_regs *regs)
-{
-}
-
static void __set_intr_gate(unsigned int n, uint32_t dpl, void *addr)
{
int i;
@@ -3516,13 +3520,11 @@ void __init init_idt_traps(void)
set_intr_gate(TRAP_invalid_op,&invalid_op);
set_intr_gate(TRAP_no_device,&device_not_available);
set_intr_gate(TRAP_double_fault,&double_fault);
- set_intr_gate(TRAP_copro_seg,&coprocessor_segment_overrun);
set_intr_gate(TRAP_invalid_tss,&invalid_TSS);
set_intr_gate(TRAP_no_segment,&segment_not_present);
set_intr_gate(TRAP_stack_error,&stack_segment);
set_intr_gate(TRAP_gp_fault,&general_protection);
set_intr_gate(TRAP_page_fault,&early_page_fault);
- set_intr_gate(TRAP_spurious_int,&spurious_interrupt_bug);
set_intr_gate(TRAP_copro_error,&coprocessor_error);
set_intr_gate(TRAP_alignment_check,&alignment_check);
set_intr_gate(TRAP_machine_check,&machine_check);
@@ -3540,8 +3542,11 @@ void __init init_idt_traps(void)
this_cpu(compat_gdt_table) = boot_cpu_compat_gdt_table;
}
+extern void (*__initconst autogen_entrypoints[NR_VECTORS])(void);
void __init trap_init(void)
{
+ unsigned int vector;
+
/* Replace early pagefault with real pagefault handler. */
set_intr_gate(TRAP_page_fault, &page_fault);
@@ -3552,6 +3557,21 @@ void __init trap_init(void)
/* Fast trap for int80 (faster than taking the #GP-fixup path). */
_set_gate(idt_table + 0x80, SYS_DESC_trap_gate, 3, &int80_direct_trap);
+ for ( vector = 0; vector < NR_VECTORS; ++vector )
+ {
+ if ( autogen_entrypoints[vector] )
+ {
+ /* Found autogen entry: check we won't clobber an existing trap. */
+ ASSERT(idt_table[vector].b == 0);
+ set_intr_gate(vector, autogen_entrypoints[vector]);
+ }
+ else
+ {
+ /* No entry point: confirm we have an existing trap in place. */
+ ASSERT(idt_table[vector].b != 0);
+ }
+ }
+
percpu_traps_init();
cpu_init();
diff --git a/xen/arch/x86/x86_64/Makefile b/xen/arch/x86/x86_64/Makefile
index 7f8fb3d..4b23d89 100644
--- a/xen/arch/x86/x86_64/Makefile
+++ b/xen/arch/x86/x86_64/Makefile
@@ -1,6 +1,7 @@
subdir-y += compat
obj-bin-y += entry.o
+obj-bin-y += irqgen.o
obj-bin-y += gpr_switch.o
obj-y += mm.o
obj-y += traps.o
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 305054f..c3b4474 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -475,6 +475,12 @@ ENTRY(common_interrupt)
callq do_IRQ
jmp ret_from_intr
+ENTRY(reserved_exception)
+ SAVE_ALL CLAC
+ movq %rsp,%rdi
+ callq do_reserved_exception
+ jmp ret_from_intr
+
/* No special register assumptions. */
ENTRY(ret_from_intr)
GET_CURRENT(%rbx)
@@ -586,11 +592,6 @@ ENTRY(invalid_op)
movl $TRAP_invalid_op,4(%rsp)
jmp handle_exception
-ENTRY(coprocessor_segment_overrun)
- pushq $0
- movl $TRAP_copro_seg,4(%rsp)
- jmp handle_exception
-
ENTRY(invalid_TSS)
movl $TRAP_invalid_tss,4(%rsp)
jmp handle_exception
@@ -611,11 +612,6 @@ ENTRY(alignment_check)
movl $TRAP_alignment_check,4(%rsp)
jmp handle_exception
-ENTRY(spurious_interrupt_bug)
- pushq $0
- movl $TRAP_spurious_int,4(%rsp)
- jmp handle_exception
-
ENTRY(double_fault)
movl $TRAP_double_fault,4(%rsp)
/* Set AC to reduce chance of further SMAP faults */
@@ -717,13 +713,13 @@ ENTRY(exception_table)
.quad do_invalid_op
.quad do_device_not_available
.quad BAD_VIRT_ADDR /* double_fault, IST entry */
- .quad do_coprocessor_segment_overrun
+ .quad BAD_VIRT_ADDR /* coproc_seg_overrun, reserved */
.quad do_invalid_TSS
.quad do_segment_not_present
.quad do_stack_segment
.quad do_general_protection
.quad do_page_fault
- .quad do_spurious_interrupt_bug
+ .quad BAD_VIRT_ADDR /* PIC IRQ7 spurious, reserved */
.quad do_coprocessor_error
.quad do_alignment_check
.quad do_machine_check
diff --git a/xen/arch/x86/x86_64/irqgen.S b/xen/arch/x86/x86_64/irqgen.S
new file mode 100644
index 0000000..1d1f249
--- /dev/null
+++ b/xen/arch/x86/x86_64/irqgen.S
@@ -0,0 +1,52 @@
+/* Automatically generated interrupt entry points */
+#include <asm/processor.h>
+#include <irq_vectors.h>
+
+.section ".init.rodata", "a", @progbits
+
+/* Table of automatically generated entry points. One per vector. */
+GLOBAL(autogen_entrypoints)
+
+/* pop into the .init.rodata section and record an entry point. */
+.macro entrypoint ent
+ .pushsection ".init.rodata", "a", @progbits
+ .quad \ent
+ .popsection
+.endm
+
+.text
+
+/* Automatically generate stub entry points. */
+autogen_stubs:
+vec = 0
+.rept NR_VECTORS
+ ALIGN
+
+ /* Common interrupts, heading towards do_IRQ(). */
+ .if vec >= FIRST_DYNAMIC_VECTOR && vec != HYPERCALL_VECTOR && vec != LEGACY_SYSCALL_VECTOR
+
+1: pushq $0
+ movb $vec,4(%rsp)
+ jmp common_interrupt
+
+ entrypoint 1b
+
+ /* Reserved exceptions, heading towards do_reserved_exception(). */
+ .elseif vec == TRAP_copro_seg || vec == TRAP_spurious_int || (vec > TRAP_simd_error && vec <= TRAP_last_reserved)
+
+1: pushq $0
+ movb $vec,4(%rsp)
+ jmp reserved_exception
+
+ entrypoint 1b
+
+ /* Hand generated in entry.S - NULL out entrypoint. */
+ .else
+ entrypoint 0
+ .endif
+
+ vec = vec + 1
+.endr
+
+.section ".init.rodata", "a", @progbits
+.size autogen_entrypoints, . - autogen_entrypoints
diff --git a/xen/include/asm-x86/asm_defns.h b/xen/include/asm-x86/asm_defns.h
index df4873b..87a462f 100644
--- a/xen/include/asm-x86/asm_defns.h
+++ b/xen/include/asm-x86/asm_defns.h
@@ -357,9 +357,4 @@ static inline void stac(void)
#define REX64_PREFIX "rex64/"
#endif
-#define BUILD_IRQ(nr) \
- "pushq $0\n\t" \
- "movl $"#nr",4(%rsp)\n\t" \
- "jmp common_interrupt"
-
#endif /* __X86_ASM_DEFNS_H__ */
diff --git a/xen/include/asm-x86/processor.h b/xen/include/asm-x86/processor.h
index 805ec34..9135860 100644
--- a/xen/include/asm-x86/processor.h
+++ b/xen/include/asm-x86/processor.h
@@ -113,6 +113,7 @@
#define TRAP_alignment_check 17
#define TRAP_machine_check 18
#define TRAP_simd_error 19
+#define TRAP_virtualisation 20
#define TRAP_last_reserved 31
/* Set for entry via SYSCALL. Informs return code to use SYSRETQ not IRETQ. */
@@ -503,7 +504,6 @@ DECLARE_TRAP_HANDLER(bounds);
DECLARE_TRAP_HANDLER(invalid_op);
DECLARE_TRAP_HANDLER(device_not_available);
DECLARE_TRAP_HANDLER(double_fault);
-DECLARE_TRAP_HANDLER(coprocessor_segment_overrun);
DECLARE_TRAP_HANDLER(invalid_TSS);
DECLARE_TRAP_HANDLER(segment_not_present);
DECLARE_TRAP_HANDLER(stack_segment);
@@ -514,12 +514,12 @@ DECLARE_TRAP_HANDLER(coprocessor_error);
DECLARE_TRAP_HANDLER(simd_coprocessor_error);
DECLARE_TRAP_HANDLER(machine_check);
DECLARE_TRAP_HANDLER(alignment_check);
-DECLARE_TRAP_HANDLER(spurious_interrupt_bug);
#undef DECLARE_TRAP_HANDLER
void trap_nop(void);
void enable_nmis(void);
void noreturn do_nmi_crash(struct cpu_user_regs *regs);
+void do_reserved_exception(struct cpu_user_regs *regs);
void syscall_enter(void);
void sysenter_entry(void);
--
1.7.10.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 09/10] x86/irqs: Move interrupt-stub generation out of C
2014-05-16 10:39 ` [PATCH v2 09/10] x86/irqs: Move interrupt-stub generation out of C Andrew Cooper
@ 2014-05-16 15:06 ` Jan Beulich
2014-05-16 15:22 ` Andrew Cooper
0 siblings, 1 reply; 15+ messages in thread
From: Jan Beulich @ 2014-05-16 15:06 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Keir Fraser, Tim Deegan, Feng Wu, Xen-devel
>>> On 16.05.14 at 12:39, <andrew.cooper3@citrix.com> wrote:
> v2:
> * Far less .macro hackary, inspired by Feng's patch of a similar nature. On
> consideration, having literal symbol names is very little use, either in
> the disassembly, or running hypervisor. In hindsight this is new version is
> substantially more likely to compile under clang, although I don't have a
> compiler to hand.
Iirc assembly code gets translated by gas even when clang is used for C.
> --- a/xen/arch/x86/x86_64/entry.S
> +++ b/xen/arch/x86/x86_64/entry.S
> @@ -475,6 +475,12 @@ ENTRY(common_interrupt)
> callq do_IRQ
> jmp ret_from_intr
>
> +ENTRY(reserved_exception)
> + SAVE_ALL CLAC
> + movq %rsp,%rdi
> + callq do_reserved_exception
> + jmp ret_from_intr
Sadly this still doesn't take care of auto-detecting whether an error
code got pushed.
Also - is there a particular reason you don't have this go though
handle_exception?
> @@ -717,13 +713,13 @@ ENTRY(exception_table)
> .quad do_invalid_op
> .quad do_device_not_available
> .quad BAD_VIRT_ADDR /* double_fault, IST entry */
> - .quad do_coprocessor_segment_overrun
> + .quad BAD_VIRT_ADDR /* coproc_seg_overrun, reserved */
> .quad do_invalid_TSS
> .quad do_segment_not_present
> .quad do_stack_segment
> .quad do_general_protection
> .quad do_page_fault
> - .quad do_spurious_interrupt_bug
> + .quad BAD_VIRT_ADDR /* PIC IRQ7 spurious, reserved */
And the comment here didn't get much (if at all) adjusted either.
> --- /dev/null
> +++ b/xen/arch/x86/x86_64/irqgen.S
> @@ -0,0 +1,52 @@
> +/* Automatically generated interrupt entry points */
> +#include <asm/processor.h>
> +#include <irq_vectors.h>
> +
> +.section ".init.rodata", "a", @progbits
> +
> +/* Table of automatically generated entry points. One per vector. */
> +GLOBAL(autogen_entrypoints)
> +
> +/* pop into the .init.rodata section and record an entry point. */
> +.macro entrypoint ent
> + .pushsection ".init.rodata", "a", @progbits
Actually it's quite pointless to repeat the attributes - they can't be
different than in the first section declaration anyway.
Also I think this more legible version would now better go into entry.S
rather than having a new file for it.
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 09/10] x86/irqs: Move interrupt-stub generation out of C
2014-05-16 15:06 ` Jan Beulich
@ 2014-05-16 15:22 ` Andrew Cooper
2014-05-16 15:30 ` Jan Beulich
0 siblings, 1 reply; 15+ messages in thread
From: Andrew Cooper @ 2014-05-16 15:22 UTC (permalink / raw)
To: Jan Beulich; +Cc: Keir Fraser, Tim Deegan, Feng Wu, Xen-devel
On 16/05/14 16:06, Jan Beulich wrote:
>>>> On 16.05.14 at 12:39, <andrew.cooper3@citrix.com> wrote:
>>>>
>>>>
>> --- a/xen/arch/x86/x86_64/entry.S
>> +++ b/xen/arch/x86/x86_64/entry.S
>> @@ -475,6 +475,12 @@ ENTRY(common_interrupt)
>> callq do_IRQ
>> jmp ret_from_intr
>>
>> +ENTRY(reserved_exception)
>> + SAVE_ALL CLAC
>> + movq %rsp,%rdi
>> + callq do_reserved_exception
>> + jmp ret_from_intr
> Sadly this still doesn't take care of auto-detecting whether an error
> code got pushed.
It is not supposed to.
Each reserved exception has its own automatically generated stub which
pushes its trap value, so do_reserved_exception() can identify which
trap has been unexpectedly triggered.
Currently there are no reserved exceptions which push an error code. I
will leave a warning comment by the generation code to take care with
the first pushq $0.
>
> Also - is there a particular reason you don't have this go though
> handle_exception?
I hadn't considered that. Its a good idea. We can:
* Automatically generate the reserved entry point, directed at
handle_exception
* Fully populate the exception_table, with several labels going towards
do_reserved_exception.
* Treat the double fault entry in exception_table as reserved. We
certainly don't want to enter through it.
>
>> @@ -717,13 +713,13 @@ ENTRY(exception_table)
>> .quad do_invalid_op
>> .quad do_device_not_available
>> .quad BAD_VIRT_ADDR /* double_fault, IST entry */
>> - .quad do_coprocessor_segment_overrun
>> + .quad BAD_VIRT_ADDR /* coproc_seg_overrun, reserved */
>> .quad do_invalid_TSS
>> .quad do_segment_not_present
>> .quad do_stack_segment
>> .quad do_general_protection
>> .quad do_page_fault
>> - .quad do_spurious_interrupt_bug
>> + .quad BAD_VIRT_ADDR /* PIC IRQ7 spurious, reserved */
> And the comment here didn't get much (if at all) adjusted either.
What about "Default PIC spurious interrupt, architecturally reserved" ?
>
>> --- /dev/null
>> +++ b/xen/arch/x86/x86_64/irqgen.S
>> @@ -0,0 +1,52 @@
>> +/* Automatically generated interrupt entry points */
>> +#include <asm/processor.h>
>> +#include <irq_vectors.h>
>> +
>> +.section ".init.rodata", "a", @progbits
>> +
>> +/* Table of automatically generated entry points. One per vector. */
>> +GLOBAL(autogen_entrypoints)
>> +
>> +/* pop into the .init.rodata section and record an entry point. */
>> +.macro entrypoint ent
>> + .pushsection ".init.rodata", "a", @progbits
> Actually it's quite pointless to repeat the attributes - they can't be
> different than in the first section declaration anyway.
>
> Also I think this more legible version would now better go into entry.S
> rather than having a new file for it.
>
> Jan
>
Sure.
~Andrew
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v2 09/10] x86/irqs: Move interrupt-stub generation out of C
2014-05-16 15:22 ` Andrew Cooper
@ 2014-05-16 15:30 ` Jan Beulich
0 siblings, 0 replies; 15+ messages in thread
From: Jan Beulich @ 2014-05-16 15:30 UTC (permalink / raw)
To: Andrew Cooper; +Cc: KeirFraser, Tim Deegan, Feng Wu, Xen-devel
>>> On 16.05.14 at 17:22, <andrew.cooper3@citrix.com> wrote:
> On 16/05/14 16:06, Jan Beulich wrote:
>>>>> On 16.05.14 at 12:39, <andrew.cooper3@citrix.com> wrote:
>>>>>
>>>>>
>>> --- a/xen/arch/x86/x86_64/entry.S
>>> +++ b/xen/arch/x86/x86_64/entry.S
>>> @@ -475,6 +475,12 @@ ENTRY(common_interrupt)
>>> callq do_IRQ
>>> jmp ret_from_intr
>>>
>>> +ENTRY(reserved_exception)
>>> + SAVE_ALL CLAC
>>> + movq %rsp,%rdi
>>> + callq do_reserved_exception
>>> + jmp ret_from_intr
>> Sadly this still doesn't take care of auto-detecting whether an error
>> code got pushed.
>
> It is not supposed to.
>
> Each reserved exception has its own automatically generated stub which
> pushes its trap value, so do_reserved_exception() can identify which
> trap has been unexpectedly triggered.
>
> Currently there are no reserved exceptions which push an error code. I
> will leave a warning comment by the generation code to take care with
> the first pushq $0.
The nature of reserved exceptions is that you don't know whether
they would push an error code. For instance, a year or two ago
(unless you were Intel) you wouldn't have known whether #VE
would have one.
>> Also - is there a particular reason you don't have this go though
>> handle_exception?
>
> I hadn't considered that. Its a good idea. We can:
>
> * Automatically generate the reserved entry point, directed at
> handle_exception
> * Fully populate the exception_table, with several labels going towards
> do_reserved_exception.
> * Treat the double fault entry in exception_table as reserved. We
> certainly don't want to enter through it.
That sounds nice.
>>> @@ -717,13 +713,13 @@ ENTRY(exception_table)
>>> .quad do_invalid_op
>>> .quad do_device_not_available
>>> .quad BAD_VIRT_ADDR /* double_fault, IST entry */
>>> - .quad do_coprocessor_segment_overrun
>>> + .quad BAD_VIRT_ADDR /* coproc_seg_overrun, reserved */
>>> .quad do_invalid_TSS
>>> .quad do_segment_not_present
>>> .quad do_stack_segment
>>> .quad do_general_protection
>>> .quad do_page_fault
>>> - .quad do_spurious_interrupt_bug
>>> + .quad BAD_VIRT_ADDR /* PIC IRQ7 spurious, reserved */
>> And the comment here didn't get much (if at all) adjusted either.
>
> What about "Default PIC spurious interrupt, architecturally reserved" ?
Fine with me.
Jan
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2 10/10] x86/misc: Post cleanup
2014-05-16 10:39 [PATCH v2 00/10] x86: Improvements to trap handling Andrew Cooper
` (8 preceding siblings ...)
2014-05-16 10:39 ` [PATCH v2 09/10] x86/irqs: Move interrupt-stub generation out of C Andrew Cooper
@ 2014-05-16 10:39 ` Andrew Cooper
2014-05-16 15:10 ` Jan Beulich
9 siblings, 1 reply; 15+ messages in thread
From: Andrew Cooper @ 2014-05-16 10:39 UTC (permalink / raw)
To: Xen-devel; +Cc: Andrew Cooper, Keir Fraser, Jan Beulich, Tim Deegan
* panic() now works on early boot. Replace EARLY_FAIL()
* Cleanup __set_intr_gate() & friends. The master IDT is fully constructed on
early boot, and only subsequently altered on the crash path. Make them
private to traps.c, move them into .init, and remove the loop over all idts,
as __set_intr_gate() will never find an AP to patch. (For some reason,
leaving out the noinline causes ~1.5k of code bloat from GCC inlining
everything)
* No need to clear X86_EFLAGS_NT in cpu_init(). This is covered by the eflags
reset in __high_start().
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
CC: Keir Fraser <keir@xen.org>
CC: Jan Beulich <JBeulich@suse.com>
CC: Tim Deegan <tim@xen.org>
---
v2:
* Don't page align idt_table.
* Forgo clearing NT.
---
xen/arch/x86/cpu/common.c | 3 ---
xen/arch/x86/setup.c | 20 ++++++--------------
xen/arch/x86/traps.c | 11 +++--------
xen/include/asm-x86/desc.h | 1 -
4 files changed, 9 insertions(+), 26 deletions(-)
diff --git a/xen/arch/x86/cpu/common.c b/xen/arch/x86/cpu/common.c
index dcd2ca1..18112f2 100644
--- a/xen/arch/x86/cpu/common.c
+++ b/xen/arch/x86/cpu/common.c
@@ -593,9 +593,6 @@ void __cpuinit cpu_init(void)
/* Install correct page table. */
write_ptbase(current);
- /* No nested task. */
- asm volatile ("pushf ; andw $0xbfff,(%"__OP"sp) ; popf" );
-
/* Ensure FPU gets initialised for each domain. */
stts();
diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c
index b2a808a..508649d 100644
--- a/xen/arch/x86/setup.c
+++ b/xen/arch/x86/setup.c
@@ -133,11 +133,6 @@ static void __init parse_acpi_param(char *s)
}
}
-#define EARLY_FAIL(f, a...) do { \
- printk( f , ## a ); \
- for ( ; ; ) halt(); \
-} while (0)
-
static const module_t *__initdata initial_images;
static unsigned int __initdata nr_initial_images;
@@ -669,11 +664,10 @@ void __init noreturn __start_xen(unsigned long mbi_p)
/* Check that we have at least one Multiboot module. */
if ( !(mbi->flags & MBI_MODULES) || (mbi->mods_count == 0) )
- EARLY_FAIL("dom0 kernel not specified. "
- "Check bootloader configuration.\n");
+ panic("dom0 kernel not specified. Check bootloader configuration.");
if ( ((unsigned long)cpu0_stack & (STACK_SIZE-1)) != 0 )
- EARLY_FAIL("Misaligned CPU0 stack.\n");
+ panic("Misaligned CPU0 stack.");
if ( efi_enabled )
{
@@ -754,9 +748,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
e820_raw_nr = 2;
}
else
- {
- EARLY_FAIL("Bootloader provided no memory information.\n");
- }
+ panic("Bootloader provided no memory information.");
/* Sanitise the raw E820 map to produce a final clean version. */
max_page = raw_max_page = init_e820(memmap_type, e820_raw, &e820_raw_nr);
@@ -791,7 +783,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
for ( i = 0; !efi_enabled && i < mbi->mods_count; i++ )
{
if ( mod[i].mod_start & (PAGE_SIZE - 1) )
- EARLY_FAIL("Bootloader didn't honor module alignment request.\n");
+ panic("Bootloader didn't honor module alignment request.");
mod[i].mod_end -= mod[i].mod_start;
mod[i].mod_start >>= PAGE_SHIFT;
mod[i].reserved = 0;
@@ -964,7 +956,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
}
if ( modules_headroom && !mod->reserved )
- EARLY_FAIL("Not enough memory to relocate the dom0 kernel image.\n");
+ panic("Not enough memory to relocate the dom0 kernel image.");
for ( i = 0; i < mbi->mods_count; ++i )
{
uint64_t s = (uint64_t)mod[i].mod_start << PAGE_SHIFT;
@@ -973,7 +965,7 @@ void __init noreturn __start_xen(unsigned long mbi_p)
}
if ( !xen_phys_start )
- EARLY_FAIL("Not enough memory to relocate Xen.\n");
+ panic("Not enough memory to relocate Xen.");
reserve_e820_ram(&boot_e820, efi_enabled ? mbi->mem_upper : __pa(&_start),
__pa(&_end));
diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c
index 2928743..413b97d 100644
--- a/xen/arch/x86/traps.c
+++ b/xen/arch/x86/traps.c
@@ -3422,22 +3422,17 @@ void do_debug(struct cpu_user_regs *regs)
return;
}
-static void __set_intr_gate(unsigned int n, uint32_t dpl, void *addr)
+static void __init noinline __set_intr_gate(unsigned int n, uint32_t dpl, void *addr)
{
- int i;
- /* Keep secondary tables in sync with IRQ updates. */
- for ( i = 1; i < nr_cpu_ids; i++ )
- if ( idt_tables[i] != NULL )
- _set_gate(&idt_tables[i][n], SYS_DESC_irq_gate, dpl, addr);
_set_gate(&idt_table[n], SYS_DESC_irq_gate, dpl, addr);
}
-static void set_swint_gate(unsigned int n, void *addr)
+static void __init set_swint_gate(unsigned int n, void *addr)
{
__set_intr_gate(n, 3, addr);
}
-void set_intr_gate(unsigned int n, void *addr)
+static void __init set_intr_gate(unsigned int n, void *addr)
{
__set_intr_gate(n, 0, addr);
}
diff --git a/xen/include/asm-x86/desc.h b/xen/include/asm-x86/desc.h
index 8257167..225913a 100644
--- a/xen/include/asm-x86/desc.h
+++ b/xen/include/asm-x86/desc.h
@@ -199,7 +199,6 @@ DECLARE_PER_CPU(struct desc_struct *, gdt_table);
extern struct desc_struct boot_cpu_compat_gdt_table[];
DECLARE_PER_CPU(struct desc_struct *, compat_gdt_table);
-extern void set_intr_gate(unsigned int irq, void * addr);
extern void load_TR(void);
#endif /* !__ASSEMBLY__ */
--
1.7.10.4
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH v2 10/10] x86/misc: Post cleanup
2014-05-16 10:39 ` [PATCH v2 10/10] x86/misc: Post cleanup Andrew Cooper
@ 2014-05-16 15:10 ` Jan Beulich
0 siblings, 0 replies; 15+ messages in thread
From: Jan Beulich @ 2014-05-16 15:10 UTC (permalink / raw)
To: Andrew Cooper; +Cc: Tim Deegan, Keir Fraser, Xen-devel
>>> On 16.05.14 at 12:39, <andrew.cooper3@citrix.com> wrote:
> * panic() now works on early boot. Replace EARLY_FAIL()
> * Cleanup __set_intr_gate() & friends. The master IDT is fully constructed on
> early boot, and only subsequently altered on the crash path. Make them
> private to traps.c, move them into .init, and remove the loop over all idts,
> as __set_intr_gate() will never find an AP to patch. (For some reason,
> leaving out the noinline causes ~1.5k of code bloat from GCC inlining
> everything)
> * No need to clear X86_EFLAGS_NT in cpu_init(). This is covered by the eflags
> reset in __high_start().
>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
^ permalink raw reply [flat|nested] 15+ messages in thread