| Message ID | 20250317192023.568432-5-andrew.cooper3@citrix.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86/traps: Simplify exception setup | expand |
On 17.03.2025 20:20, Andrew Cooper wrote: > Something I overlooked when last cleaning up exception handling is that a TSS > is not necessary if IST isn't configured, and IST isn't necessary until we're > running guest code. > > Introduce early_traps_init() which is far more minimal than init_idt_traps(); > bsp_ist[] is constructed without IST settings, so all early_traps_init() needs > to do is load the IDT, and invalidate TR/LDTR. > > Put the remaining logic into traps_init(), later on boot. Note that > load_system_tables() already contains enable_each_ist(), so the call is simply > dropped. > > This removes some complexity prior to having exception support, and lays the > groundwork to not even allocate a TSS when using FRED. > > No practical change. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Given the present state of thing: Reviewed-by: Jan Beulich <jbeulich@suse.com> What worries me slightly is that ... > @@ -63,6 +88,13 @@ void __init traps_init(void) > /* Replace early pagefault with real pagefault handler. */ > _update_gate_addr_lower(&bsp_idt[X86_EXC_PF], entry_PF); > > + this_cpu(idt) = bsp_idt; > + this_cpu(gdt) = boot_gdt; > + if ( IS_ENABLED(CONFIG_PV32) ) > + this_cpu(compat_gdt) = boot_compat_gdt; ... this being done later now requires more care with e.g. play_dead(). Yet if and when needed, this setting up could of course be moved earlier again. It's not entirely clear to me why you specifically want it here and not in early_traps_init(). The sole dependency is percpu_init_areas(), which runs - as even visible from patch context here - ahead of early_traps_init(). My take is - the earlier we do things that can be done easily, the less we have to worry about ordering and dependencies. Jan
diff --git a/xen/arch/x86/include/asm/system.h b/xen/arch/x86/include/asm/system.h index 1bf6bd026852..8ceaaf45d1a0 100644 --- a/xen/arch/x86/include/asm/system.h +++ b/xen/arch/x86/include/asm/system.h @@ -263,7 +263,6 @@ static inline int local_irq_is_enabled(void) #define BROKEN_ACPI_Sx 0x0001 #define BROKEN_INIT_AFTER_S1 0x0002 -void init_idt_traps(void); void load_system_tables(void); void subarch_percpu_traps_init(void); diff --git a/xen/arch/x86/include/asm/traps.h b/xen/arch/x86/include/asm/traps.h index 3d30aa6738d4..72c33a33e283 100644 --- a/xen/arch/x86/include/asm/traps.h +++ b/xen/arch/x86/include/asm/traps.h @@ -7,6 +7,7 @@ #ifndef ASM_TRAP_H #define ASM_TRAP_H +void early_traps_init(void); void traps_init(void); void percpu_traps_init(void); diff --git a/xen/arch/x86/setup.c b/xen/arch/x86/setup.c index 5e2411a008f5..6a6ddc569500 100644 --- a/xen/arch/x86/setup.c +++ b/xen/arch/x86/setup.c @@ -1093,8 +1093,7 @@ void asmlinkage __init noreturn __start_xen(void) percpu_init_areas(); - init_idt_traps(); - load_system_tables(); + early_traps_init(); smp_prepare_boot_cpu(); sort_exception_tables(); diff --git a/xen/arch/x86/traps-setup.c b/xen/arch/x86/traps-setup.c index 1a7b42c14bf2..67a6fda89ad9 100644 --- a/xen/arch/x86/traps-setup.c +++ b/xen/arch/x86/traps-setup.c @@ -55,6 +55,31 @@ static void __init init_ler(void) setup_force_cpu_cap(X86_FEATURE_XEN_LBR); } +/* + * Configure basic exception handling. This is prior to parsing the command + * line or configuring a console, and needs to be as simple as possible. + * + * boot_gdt is already loaded, and bsp_idt[] is constructed without IST + * settings, so we don't need a TSS configured yet. + * + * Load bsp_idt[], and invalidate the TSS and LDT. + */ +void __init early_traps_init(void) +{ + const struct desc_ptr idtr = { + .base = (unsigned long)bsp_idt, + .limit = sizeof(bsp_idt) - 1, + }; + + lidt(&idtr); + + _set_tssldt_desc(boot_gdt + TSS_ENTRY - FIRST_RESERVED_GDT_ENTRY, + 0, 0, SYS_DESC_tss_avail); + + ltr(TSS_SELECTOR); + lldt(0); +} + /* * Configure complete exception, interrupt and syscall handling. */ @@ -63,6 +88,13 @@ void __init traps_init(void) /* Replace early pagefault with real pagefault handler. */ _update_gate_addr_lower(&bsp_idt[X86_EXC_PF], entry_PF); + this_cpu(idt) = bsp_idt; + this_cpu(gdt) = boot_gdt; + if ( IS_ENABLED(CONFIG_PV32) ) + this_cpu(compat_gdt) = boot_compat_gdt; + + load_system_tables(); + init_ler(); /* Cache {,compat_}gdt_l1e now that physically relocation is done. */ diff --git a/xen/arch/x86/traps.c b/xen/arch/x86/traps.c index 79d92f21acf5..25e0d5777e6e 100644 --- a/xen/arch/x86/traps.c +++ b/xen/arch/x86/traps.c @@ -1858,19 +1858,6 @@ void asmlinkage do_entry_CP(struct cpu_user_regs *regs) panic("CONTROL-FLOW PROTECTION FAULT: #CP[%04x] %s\n", ec, err); } -void __init init_idt_traps(void) -{ - /* Specify dedicated interrupt stacks for NMI, #DF, and #MC. */ - enable_each_ist(bsp_idt); - - /* CPU0 uses the master IDT. */ - this_cpu(idt) = bsp_idt; - - this_cpu(gdt) = boot_gdt; - if ( IS_ENABLED(CONFIG_PV32) ) - this_cpu(compat_gdt) = boot_compat_gdt; -} - void asm_domain_crash_synchronous(unsigned long addr) { /*
Something I overlooked when last cleaning up exception handling is that a TSS is not necessary if IST isn't configured, and IST isn't necessary until we're running guest code. Introduce early_traps_init() which is far more minimal than init_idt_traps(); bsp_ist[] is constructed without IST settings, so all early_traps_init() needs to do is load the IDT, and invalidate TR/LDTR. Put the remaining logic into traps_init(), later on boot. Note that load_system_tables() already contains enable_each_ist(), so the call is simply dropped. This removes some complexity prior to having exception support, and lays the groundwork to not even allocate a TSS when using FRED. No practical change. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- CC: Jan Beulich <JBeulich@suse.com> CC: Roger Pau Monné <roger.pau@citrix.com> --- xen/arch/x86/include/asm/system.h | 1 - xen/arch/x86/include/asm/traps.h | 1 + xen/arch/x86/setup.c | 3 +-- xen/arch/x86/traps-setup.c | 32 +++++++++++++++++++++++++++++++ xen/arch/x86/traps.c | 13 ------------- 5 files changed, 34 insertions(+), 16 deletions(-)