On 5/11/22 14:34, Yuichiro NAITO wrote:
After applying your patch, cpu1 is not identified on my current kernel.
Dmesg shows as follows. I'll see it further more.

I found that LAPIC is necessary for `delay` function that is used in 
`idendifycpu` and
waiting loop for CPUF_IDENTIFY flag is set.

I partly reverted calling `lapic_enable` and `lapic_startclock` and it works 
for me.
Please see my following patch.

diff --git a/sys/arch/amd64/amd64/cpu.c b/sys/arch/amd64/amd64/cpu.c
index 4778347a12e..83c0297e7f1 100644
--- a/sys/arch/amd64/amd64/cpu.c
+++ b/sys/arch/amd64/amd64/cpu.c
@@ -844,50 +844,54 @@ cpu_start_secondary(struct cpu_info *ci)
         * wait for it to become ready
         */
        for (i = 100000; (!(ci->ci_flags & CPUF_PRESENT)) && i>0;i--) {
                delay(10);
        }
        if (! (ci->ci_flags & CPUF_PRESENT)) {
                printf("%s: failed to become ready\n", ci->ci_dev->dv_xname);
 #if defined(MPDEBUG) && defined(DDB)
                printf("dropping into debugger; continue from here to resume 
boot\n");
                db_enter();
 #endif
-       } else {
-               /*
-                * Synchronize time stamp counters. Invalidate cache and
-                * synchronize twice (in tsc_sync_bp) to minimize possible
-                * cache effects. Disable interrupts to try and rule out any
-                * external interference.
-                */
-               s = intr_disable();
-               wbinvd();
-               tsc_sync_bp(ci);
-               intr_restore(s);
-#ifdef TSC_DEBUG
-               printf("TSC skew=%lld\n", (long long)ci->ci_tsc_skew);
-#endif
+               goto cleanup;
        }

        if ((ci->ci_flags & CPUF_IDENTIFIED) == 0) {
                atomic_setbits_int(&ci->ci_flags, CPUF_IDENTIFY);

                /* wait for it to identify */
                for (i = 2000000; (ci->ci_flags & CPUF_IDENTIFY) && i > 0; i--)
                        delay(10);

-               if (ci->ci_flags & CPUF_IDENTIFY)
+               if (ci->ci_flags & CPUF_IDENTIFY) {
                        printf("%s: failed to identify\n",
                            ci->ci_dev->dv_xname);
+                       goto cleanup;
+               }
        }

+       /*
+        * Synchronize time stamp counters. Invalidate cache and
+        * synchronize twice (in tsc_sync_bp) to minimize possible
+        * cache effects. Disable interrupts to try and rule out any
+        * external interference.
+        */
+       s = intr_disable();
+       wbinvd();
+       tsc_sync_bp(ci);
+       intr_restore(s);
+#ifdef TSC_DEBUG
+       printf("TSC skew=%lld\n", (long long)ci->ci_tsc_skew);
+#endif
+
+cleanup:
        CPU_START_CLEANUP(ci);

        pmap_kremove(MP_TRAMPOLINE, PAGE_SIZE);
        pmap_kremove(MP_TRAMP_DATA, PAGE_SIZE);
 }

 void
 cpu_boot_secondary(struct cpu_info *ci)
 {
        int i;
        int64_t drift;
@@ -932,53 +936,53 @@ void
 cpu_hatch(void *v)
 {
        struct cpu_info *ci = (struct cpu_info *)v;
        int s;

        cpu_init_msrs(ci);

 #ifdef DEBUG
        if (ci->ci_flags & CPUF_PRESENT)
                panic("%s: already running!?", ci->ci_dev->dv_xname);
 #endif
-
-       /*
-        * Synchronize the TSC for the first time. Note that interrupts are
-        * off at this point.
-        */
-       wbinvd();
        ci->ci_flags |= CPUF_PRESENT;
-       ci->ci_tsc_skew = 0; /* reset on resume */
-       tsc_sync_ap(ci);

        lapic_enable();
        lapic_startclock();
        cpu_ucode_apply(ci);
        cpu_tsx_disable(ci);

        if ((ci->ci_flags & CPUF_IDENTIFIED) == 0) {
                /*
                 * We need to wait until we can identify, otherwise dmesg
                 * output will be messy.
                 */
                while ((ci->ci_flags & CPUF_IDENTIFY) == 0)
                        delay(10);

                identifycpu(ci);

                /* Signal we're done */
                atomic_clearbits_int(&ci->ci_flags, CPUF_IDENTIFY);
                /* Prevent identifycpu() from running again */
                atomic_setbits_int(&ci->ci_flags, CPUF_IDENTIFIED);
        }

+       /*
+        * Synchronize the TSC for the first time. Note that interrupts are
+        * off at this point.
+        */
+       wbinvd();
+       ci->ci_tsc_skew = 0; /* reset on resume */
+       tsc_sync_ap(ci);
+
        while ((ci->ci_flags & CPUF_GO) == 0)
                delay(10);
 #ifdef HIBERNATE
        if ((ci->ci_flags & CPUF_PARK) != 0) {
                atomic_clearbits_int(&ci->ci_flags, CPUF_PARK);
                hibernate_drop_to_real_mode();
        }
 #endif /* HIBERNATE */

 #ifdef DEBUG
        if (ci->ci_flags & CPUF_RUNNING)


--
Yuichiro NAITO (naito.yuich...@gmail.com)

Reply via email to