intr: Further delay MachIntrABI.finalize()
authorSepherosa Ziehau <sephe@dragonflybsd.org>
Wed, 2 Feb 2011 15:55:43 +0000 (23:55 +0800)
committerSepherosa Ziehau <sephe@dragonflybsd.org>
Wed, 2 Feb 2011 16:10:35 +0000 (00:10 +0800)
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.

12 files changed:
sys/kern/init_main.c
sys/platform/pc32/apic/ioapic_abi.c
sys/platform/pc32/i386/machdep.c
sys/platform/pc32/i386/mp_machdep.c
sys/platform/pc32/icu/icu_abi.c
sys/platform/pc64/apic/ioapic_abi.c
sys/platform/pc64/icu/icu_abi.c
sys/platform/pc64/x86_64/machdep.c
sys/platform/pc64/x86_64/mp_machdep.c
sys/platform/vkernel/platform/machintr.c
sys/platform/vkernel64/platform/machintr.c
sys/sys/machintr.h

index deceac6..a65de3f 100644 (file)
@@ -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();
index fdce657..f2b963d 100644 (file)
@@ -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)
 {
index 1fc7c6f..a06da81 100644 (file)
@@ -366,6 +366,8 @@ again:
         */
        mp_start();                     /* fire up the APs and APICs */
        mp_announce();
+#else
+       MachIntrABI.finalize();
 #endif  /* SMP */
        cpu_setregs();
 }
index a875566..ff97213 100644 (file)
@@ -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);
 
index 9c99530..81abb1e 100644 (file)
@@ -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
index d8f7d10..7e35fd4 100644 (file)
@@ -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)
index 6312c43..849834a 100644 (file)
@@ -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
index f83e42a..3396e4d 100644 (file)
@@ -387,6 +387,8 @@ again:
         */
        mp_start();                     /* fire up the APs and APICs */
        mp_announce();
+#else
+       MachIntrABI.finalize();
 #endif  /* SMP */
        cpu_setregs();
 }
index f7fef5e..36f3692 100644 (file)
@@ -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 */
index f5f76b9..fb2bb4e 100644 (file)
@@ -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
  */
index 06cc1a4..08184a2 100644 (file)
@@ -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
  */
index bc338ee..950e300 100644 (file)
@@ -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)