From 7bf5fa56b57c6864fef14bc71b211a265244d886 Mon Sep 17 00:00:00 2001 From: Sepherosa Ziehau Date: Wed, 2 Feb 2011 23:55:43 +0800 Subject: [PATCH] intr: Further delay MachIntrABI.finalize() It only affects SMP case. For ICU, it will be better if finalize() is called after IMCR detection is done, though on most modern systems IMCR does not exist. For I/O APIC, finalize() _should_ be called after BSP's LAPIC is initialized, since it alters BSP LAPIC's LINT0 and LINT1 configuration. Add stabilize() ABI to MachIntrABI which is only implemented by ICU currently and this ABI is called in the place where finalize() used to be called. --- sys/kern/init_main.c | 2 +- sys/platform/pc32/apic/ioapic_abi.c | 40 ++++++++++--- sys/platform/pc32/i386/machdep.c | 2 + sys/platform/pc32/i386/mp_machdep.c | 3 + sys/platform/pc32/icu/icu_abi.c | 67 +++++++++++++++------- sys/platform/pc64/apic/ioapic_abi.c | 42 ++++++++++---- sys/platform/pc64/icu/icu_abi.c | 67 +++++++++++++++------- sys/platform/pc64/x86_64/machdep.c | 2 + sys/platform/pc64/x86_64/mp_machdep.c | 3 + sys/platform/vkernel/platform/machintr.c | 9 ++- sys/platform/vkernel64/platform/machintr.c | 9 ++- sys/sys/machintr.h | 1 + 12 files changed, 185 insertions(+), 62 deletions(-) diff --git a/sys/kern/init_main.c b/sys/kern/init_main.c index deceac68a0..a65de3ffa9 100644 --- a/sys/kern/init_main.c +++ b/sys/kern/init_main.c @@ -299,7 +299,7 @@ SYSINIT(announce, SI_BOOT1_COPYRIGHT, SI_ORDER_FIRST, print_caddr_t, copyright) static void leavecrit(void *dummy __unused) { - MachIntrABI.finalize(); + MachIntrABI.stabilize(); cpu_enable_intr(); MachIntrABI.cleanup(); crit_exit(); diff --git a/sys/platform/pc32/apic/ioapic_abi.c b/sys/platform/pc32/apic/ioapic_abi.c index fdce657e29..f2b963d95f 100644 --- a/sys/platform/pc32/apic/ioapic_abi.c +++ b/sys/platform/pc32/apic/ioapic_abi.c @@ -458,6 +458,7 @@ static int ioapic_vectorctl(int, int, int); static void ioapic_finalize(void); static void ioapic_cleanup(void); static void ioapic_setdefault(void); +static void ioapic_stabilize(void); static int ioapic_imcr_present; @@ -470,7 +471,8 @@ struct machintr_abi MachIntrABI_IOAPIC = { .getvar = ioapic_getvar, .finalize = ioapic_finalize, .cleanup = ioapic_cleanup, - .setdefault = ioapic_setdefault + .setdefault = ioapic_setdefault, + .stabilize = ioapic_stabilize }; static int @@ -508,16 +510,18 @@ ioapic_getvar(int varid, void *buf) } /* - * Called before interrupts are physically enabled, this routine does the - * final configuration of the BSP's local APIC: + * Called from ICU's finalize if I/O APIC is enabled, after BSP's LAPIC + * is initialized; some of the BSP's LAPIC configuration are adjusted. * - * - disable 'pic mode'. - * - disable 'virtual wire mode'. - * - enable NMI. + * - disable 'pic mode'. + * - switch MachIntrABI. + * - disable 'virtual wire mode'. + * - enable NMI. */ static void ioapic_finalize(void) { + u_long ef; uint32_t temp; KKASSERT(MachIntrABI.type == MACHINTR_ICU); @@ -534,7 +538,7 @@ ioapic_finalize(void) } /* - * Setup lint0 (the 8259 'virtual wire' connection). We + * Setup LINT0 (the 8259 'virtual wire' connection). We * mask the interrupt, completing the disconnection of the * 8259. */ @@ -542,6 +546,11 @@ ioapic_finalize(void) temp |= APIC_LVT_MASKED; lapic.lvt_lint0 = temp; + crit_enter(); + + ef = read_eflags(); + cpu_disable_intr(); + /* * 8259 is completely disconnected; switch to IOAPIC MachIntrABI * and reconfigure the default IDT entries. @@ -549,15 +558,21 @@ ioapic_finalize(void) MachIntrABI = MachIntrABI_IOAPIC; MachIntrABI.setdefault(); + write_eflags(ef); + + MachIntrABI.cleanup(); + + crit_exit(); + /* - * Setup lint1 to handle NMI + * Setup LINT1 to handle NMI */ temp = lapic.lvt_lint1; temp &= ~APIC_LVT_MASKED; lapic.lvt_lint1 = temp; if (bootverbose) - apic_dump("bsp_apic_configure()"); + apic_dump("ioapic_finalize()"); } /* @@ -571,6 +586,13 @@ ioapic_cleanup(void) bzero(mdcpu->gd_ipending, sizeof(mdcpu->gd_ipending)); } +/* Must never be called */ +static void +ioapic_stabilize(void) +{ + panic("ioapic_stabilize() is called\n"); +} + static int ioapic_vectorctl(int op, int intr, int flags) { diff --git a/sys/platform/pc32/i386/machdep.c b/sys/platform/pc32/i386/machdep.c index 1fc7c6f02f..a06da81eaa 100644 --- a/sys/platform/pc32/i386/machdep.c +++ b/sys/platform/pc32/i386/machdep.c @@ -366,6 +366,8 @@ again: */ mp_start(); /* fire up the APs and APICs */ mp_announce(); +#else + MachIntrABI.finalize(); #endif /* SMP */ cpu_setregs(); } diff --git a/sys/platform/pc32/i386/mp_machdep.c b/sys/platform/pc32/i386/mp_machdep.c index a875566bc2..ff9721363b 100644 --- a/sys/platform/pc32/i386/mp_machdep.c +++ b/sys/platform/pc32/i386/mp_machdep.c @@ -2168,6 +2168,9 @@ start_all_aps(u_int boot_addr) /* Initialize BSP's local APIC */ apic_initialize(TRUE); + /* Finalize PIC */ + MachIntrABI.finalize(); + /* install the AP 1st level boot code */ install_ap_tramp(boot_addr); diff --git a/sys/platform/pc32/icu/icu_abi.c b/sys/platform/pc32/icu/icu_abi.c index 9c9953013c..81abb1e563 100644 --- a/sys/platform/pc32/icu/icu_abi.c +++ b/sys/platform/pc32/icu/icu_abi.c @@ -89,6 +89,7 @@ static int icu_getvar(int, void *); static void icu_finalize(void); static void icu_cleanup(void); static void icu_setdefault(void); +static void icu_stabilize(void); struct machintr_abi MachIntrABI_ICU = { MACHINTR_ICU, @@ -99,7 +100,8 @@ struct machintr_abi MachIntrABI_ICU = { .getvar = icu_getvar, .finalize = icu_finalize, .cleanup = icu_cleanup, - .setdefault = icu_setdefault + .setdefault = icu_setdefault, + .stabilize = icu_stabilize }; static int icu_imcr_present; @@ -145,25 +147,49 @@ icu_getvar(int varid, void *buf) * Called before interrupts are physically enabled */ static void -icu_finalize(void) +icu_stabilize(void) { int intr; + for (intr = 0; intr < ICU_HWI_VECTORS; ++intr) + machintr_intrdis(intr); + machintr_intren(ICU_IRQ_SLAVE); +} + +/* + * Called after interrupts physically enabled but before the + * critical section is released. + */ +static void +icu_cleanup(void) +{ + bzero(mdcpu->gd_ipending, sizeof(mdcpu->gd_ipending)); +} + +/* + * Called after stablize and cleanup; critical section is not + * held and interrupts are not physically disabled. + * + * For SMP: + * Further delayed after BSP's LAPIC is initialized + */ +static void +icu_finalize(void) +{ + KKASSERT(MachIntrABI.type == MACHINTR_ICU); + #ifdef SMP if (apic_io_enable) { - KKASSERT(MachIntrABI.type == MACHINTR_ICU); + /* + * MachIntrABI switching will happen in + * MachIntrABI_IOAPIC.finalize() + */ MachIntrABI_IOAPIC.setvar(MACHINTR_VAR_IMCR_PRESENT, &icu_imcr_present); MachIntrABI_IOAPIC.finalize(); return; } -#endif - - for (intr = 0; intr < ICU_HWI_VECTORS; ++intr) - machintr_intrdis(intr); - machintr_intren(ICU_IRQ_SLAVE); -#ifdef SMP /* * If an IMCR is present, programming bit 0 disconnects the 8259 * from the BSP. The 8259 may still be connected to LINT0 on the @@ -174,20 +200,21 @@ icu_finalize(void) * in addition to the 8259. */ if (icu_imcr_present) { + u_long ef; + + crit_enter(); + + ef = read_eflags(); + cpu_disable_intr(); + outb(0x22, 0x70); outb(0x23, 0x01); - } -#endif -} -/* - * Called after interrupts physically enabled but before the - * critical section is released. - */ -static void -icu_cleanup(void) -{ - bzero(mdcpu->gd_ipending, sizeof(mdcpu->gd_ipending)); + write_eflags(ef); + + crit_exit(); + } +#endif /* SMP */ } static int diff --git a/sys/platform/pc64/apic/ioapic_abi.c b/sys/platform/pc64/apic/ioapic_abi.c index d8f7d10070..7e35fd45bd 100644 --- a/sys/platform/pc64/apic/ioapic_abi.c +++ b/sys/platform/pc64/apic/ioapic_abi.c @@ -458,6 +458,7 @@ static int ioapic_vectorctl(int, int, int); static void ioapic_finalize(void); static void ioapic_cleanup(void); static void ioapic_setdefault(void); +static void ioapic_stabilize(void); static int ioapic_imcr_present; @@ -470,7 +471,8 @@ struct machintr_abi MachIntrABI_IOAPIC = { .getvar = ioapic_getvar, .finalize = ioapic_finalize, .cleanup = ioapic_cleanup, - .setdefault = ioapic_setdefault + .setdefault = ioapic_setdefault, + .stabilize = ioapic_stabilize }; static int @@ -508,16 +510,18 @@ ioapic_getvar(int varid, void *buf) } /* - * Called before interrupts are physically enabled, this routine does the - * final configuration of the BSP's local APIC: + * Called from ICU's finalize if I/O APIC is enabled, after BSP's LAPIC + * is initialized; some of the BSP's LAPIC configuration are adjusted. * - * - disable 'pic mode'. - * - disable 'virtual wire mode'. - * - enable NMI. + * - disable 'pic mode'. + * - disable 'virtual wire mode'. + * - switch MachIntrABI + * - enable NMI. */ static void ioapic_finalize(void) { + register_t ef; uint32_t temp; KKASSERT(MachIntrABI.type == MACHINTR_ICU); @@ -534,7 +538,7 @@ ioapic_finalize(void) } /* - * Setup lint0 (the 8259 'virtual wire' connection). We + * Setup LINT0 (the 8259 'virtual wire' connection). We * mask the interrupt, completing the disconnection of the * 8259. */ @@ -542,6 +546,11 @@ ioapic_finalize(void) temp |= APIC_LVT_MASKED; lapic->lvt_lint0 = temp; + crit_enter(); + + ef = read_rflags(); + cpu_disable_intr(); + /* * 8259 is completely disconnected; switch to IOAPIC MachIntrABI * and reconfigure the default IDT entries. @@ -549,15 +558,21 @@ ioapic_finalize(void) MachIntrABI = MachIntrABI_IOAPIC; MachIntrABI.setdefault(); + write_rflags(ef); + + MachIntrABI.cleanup(); + + crit_exit(); + /* - * Setup lint1 to handle an NMI + * Setup LINT1 to handle an NMI */ temp = lapic->lvt_lint1; temp &= ~APIC_LVT_MASKED; lapic->lvt_lint1 = temp; if (bootverbose) - apic_dump("bsp_apic_configure()"); + apic_dump("ioapic_finalize()"); } /* @@ -571,6 +586,13 @@ ioapic_cleanup(void) bzero(mdcpu->gd_ipending, sizeof(mdcpu->gd_ipending)); } +/* Must never be called */ +static void +ioapic_stabilize(void) +{ + panic("ioapic_stabilize is called\n"); +} + static int ioapic_vectorctl(int op, int intr, int flags) { @@ -578,7 +600,7 @@ ioapic_vectorctl(int op, int intr, int flags) int vector; int select; uint32_t value; - u_long ef; + register_t ef; if (intr < 0 || intr >= IOAPIC_HWI_VECTORS || intr == IDT_OFFSET_SYSCALL - IDT_OFFSET) diff --git a/sys/platform/pc64/icu/icu_abi.c b/sys/platform/pc64/icu/icu_abi.c index 6312c43202..849834af0e 100644 --- a/sys/platform/pc64/icu/icu_abi.c +++ b/sys/platform/pc64/icu/icu_abi.c @@ -89,6 +89,7 @@ static int icu_getvar(int, void *); static void icu_finalize(void); static void icu_cleanup(void); static void icu_setdefault(void); +static void icu_stabilize(void); struct machintr_abi MachIntrABI_ICU = { MACHINTR_ICU, @@ -99,7 +100,8 @@ struct machintr_abi MachIntrABI_ICU = { .getvar = icu_getvar, .finalize = icu_finalize, .cleanup = icu_cleanup, - .setdefault = icu_setdefault + .setdefault = icu_setdefault, + .stabilize = icu_stabilize }; static int icu_imcr_present; @@ -145,25 +147,49 @@ icu_getvar(int varid, void *buf) * Called before interrupts are physically enabled */ static void -icu_finalize(void) +icu_stabilize(void) { int intr; + for (intr = 0; intr < ICU_HWI_VECTORS; ++intr) + machintr_intrdis(intr); + machintr_intren(ICU_IRQ_SLAVE); +} + +/* + * Called after interrupts physically enabled but before the + * critical section is released. + */ +static void +icu_cleanup(void) +{ + bzero(mdcpu->gd_ipending, sizeof(mdcpu->gd_ipending)); +} + +/* + * Called after stablize and cleanup; critical section is not + * held and interrupts are not physically disabled. + * + * For SMP: + * Further delayed after BSP's LAPIC is initialized + */ +static void +icu_finalize(void) +{ + KKASSERT(MachIntrABI.type == MACHINTR_ICU); + #ifdef SMP if (apic_io_enable) { - KKASSERT(MachIntrABI.type == MACHINTR_ICU); + /* + * MachIntrABI switching will happen in + * MachIntrABI_IOAPIC.finalize() + */ MachIntrABI_IOAPIC.setvar(MACHINTR_VAR_IMCR_PRESENT, &icu_imcr_present); MachIntrABI_IOAPIC.finalize(); return; } -#endif - - for (intr = 0; intr < ICU_HWI_VECTORS; ++intr) - machintr_intrdis(intr); - machintr_intren(ICU_IRQ_SLAVE); -#if defined(SMP) /* * If an IMCR is present, programming bit 0 disconnects the 8259 * from the BSP. The 8259 may still be connected to LINT0 on the @@ -174,20 +200,21 @@ icu_finalize(void) * in addition to the 8259. */ if (icu_imcr_present) { + register_t ef; + + crit_enter(); + + ef = read_rflags(); + cpu_disable_intr(); + outb(0x22, 0x70); outb(0x23, 0x01); - } -#endif -} -/* - * Called after interrupts physically enabled but before the - * critical section is released. - */ -static void -icu_cleanup(void) -{ - bzero(mdcpu->gd_ipending, sizeof(mdcpu->gd_ipending)); + write_rflags(ef); + + crit_exit(); + } +#endif /* SMP */ } static int diff --git a/sys/platform/pc64/x86_64/machdep.c b/sys/platform/pc64/x86_64/machdep.c index f83e42ad57..3396e4d54b 100644 --- a/sys/platform/pc64/x86_64/machdep.c +++ b/sys/platform/pc64/x86_64/machdep.c @@ -387,6 +387,8 @@ again: */ mp_start(); /* fire up the APs and APICs */ mp_announce(); +#else + MachIntrABI.finalize(); #endif /* SMP */ cpu_setregs(); } diff --git a/sys/platform/pc64/x86_64/mp_machdep.c b/sys/platform/pc64/x86_64/mp_machdep.c index f7fef5e398..36f3692da4 100644 --- a/sys/platform/pc64/x86_64/mp_machdep.c +++ b/sys/platform/pc64/x86_64/mp_machdep.c @@ -2165,6 +2165,9 @@ start_all_aps(u_int boot_addr) /* Initialize BSP's local APIC */ apic_initialize(TRUE); + /* Finalize PIC */ + MachIntrABI.finalize(); + /* install the AP 1st level boot code */ pmap_kenter(va, boot_address); cpu_invlpg((void *)va); /* JG XXX */ diff --git a/sys/platform/vkernel/platform/machintr.c b/sys/platform/vkernel/platform/machintr.c index f5f76b9f78..fb2bb4e503 100644 --- a/sys/platform/vkernel/platform/machintr.c +++ b/sys/platform/vkernel/platform/machintr.c @@ -59,6 +59,7 @@ static int dummy_setvar(int, const void *); static int dummy_getvar(int, void *); static void dummy_finalize(void); static void dummy_intrcleanup(void); +static void dummy_stabilize(void); struct machintr_abi MachIntrABI = { MACHINTR_GENERIC, @@ -68,7 +69,8 @@ struct machintr_abi MachIntrABI = { .setvar = dummy_setvar, .getvar = dummy_getvar, .finalize = dummy_finalize, - .cleanup = dummy_intrcleanup + .cleanup = dummy_intrcleanup, + .stabilize = dummy_stabilize }; static void @@ -110,6 +112,11 @@ dummy_intrcleanup(void) { } +static void +dummy_stabilize(void) +{ +} + /* * Process pending interrupts */ diff --git a/sys/platform/vkernel64/platform/machintr.c b/sys/platform/vkernel64/platform/machintr.c index 06cc1a4712..08184a23c3 100644 --- a/sys/platform/vkernel64/platform/machintr.c +++ b/sys/platform/vkernel64/platform/machintr.c @@ -59,6 +59,7 @@ static int dummy_setvar(int, const void *); static int dummy_getvar(int, void *); static void dummy_finalize(void); static void dummy_intrcleanup(void); +static void dummy_stabilize(void); struct machintr_abi MachIntrABI = { MACHINTR_GENERIC, @@ -68,7 +69,8 @@ struct machintr_abi MachIntrABI = { .setvar = dummy_setvar, .getvar = dummy_getvar, .finalize = dummy_finalize, - .cleanup = dummy_intrcleanup + .cleanup = dummy_intrcleanup, + .stabilize = dummy_stabilize }; static void @@ -110,6 +112,11 @@ dummy_intrcleanup(void) { } +static void +dummy_stabilize(void) +{ +} + /* * Process pending interrupts */ diff --git a/sys/sys/machintr.h b/sys/sys/machintr.h index bc338ee64b..950e300a97 100644 --- a/sys/sys/machintr.h +++ b/sys/sys/machintr.h @@ -64,6 +64,7 @@ struct machintr_abi { void (*finalize)(void); /* final before ints enabled */ void (*cleanup)(void); /* cleanup */ void (*setdefault)(void); /* set default vectors */ + void (*stabilize)(void); /* stable before ints enabled */ }; #define machintr_intren(intr) MachIntrABI.intren(intr) -- 2.41.0