Hi, Scott.
After applying your patch, cpu1 is not identified on my current kernel.
Dmesg shows as follows. I'll see it further more.
OpenBSD 7.1-current (GENERIC.MP) #3: Wed May 11 12:27:53 JST 2022
yuich...@yuichiro-obsd.soum.co.jp:/usr/src/sys/arch/amd64/compile/GENERIC.MP
real mem = 2129526784 (2030MB)
avail mem = 2047676416 (1952MB)
random: good seed from bootblocks
mpath0 at root
scsibus0 at mpath0: 256 targets
mainbus0 at root
bios0 at mainbus0: SMBIOS rev. 2.7 @ 0xe0010 (242 entries)
bios0: vendor Phoenix Technologies LTD version "6.00" date 12/12/2018
bios0: VMware, Inc. VMware Virtual Platform
acpi0 at bios0: ACPI 4.0
acpi0: sleep states S0 S1 S4 S5
acpi0: tables DSDT FACP BOOT APIC MCFG SRAT HPET WAET
acpi0: wakeup devices PCI0(S3) USB_(S1) P2P0(S3) S1F0(S3) S2F0(S3) S8F0(S3)
S16F(S3) S17F(S3) S18F(S3) S22F(S3) S23F(S3) S24F(S3) S25F(S3) PE40(S3)
S1F0(S3) PE50(S3) [...]
acpitimer0 at acpi0: 3579545 Hz, 24 bits
acpimadt0 at acpi0 addr 0xfee00000: PC-AT compat
cpu0 at mainbus0: apid 0 (boot processor)
cpu0: AMD Ryzen 7 5700G with Radeon Graphics, 3792.28 MHz, 19-50-00
cpu0:
FPU,VME,DE,PSE,TSC,MSR,PAE,MCE,CX8,APIC,SEP,MTRR,PGE,MCA,CMOV,PAT,PSE36,CFLUSH,MMX,FXSR,SSE,SSE2,SSE3,PCLMUL,SSSE3,FMA3,CX16,SSE4.1,SSE4.2,x2APIC,MOVBE,POPCNT,AES,XSAVE,AVX,F16C,RDRAND,HV,NXE,MMXX,FFXSR,PAGE1GB,RDTSCP,LONG,LAHF,EAPICSP,AMCR8,ABM,SSE4A,MASSE,3DNOWP,OSVW,ITSC,FSGSBASE,BMI1,AVX2,SMEP,BMI2,ERMS,INVPCID,RDSEED,ADX,SMAP,CLFLUSHOPT,CLWB,SHA,PKU,IBPB,XSAVEOPT,XSAVEC,XSAVES
cpu0: 32KB 64b/line 8-way I-cache, 32KB 64b/line 8-way D-cache, 512KB 64b/line
8-way L2 cache
cpu0: ITLB 64 4KB entries fully associative, 64 4MB entries fully associative
cpu0: DTLB 64 4KB entries fully associative, 64 4MB entries fully associative
cpu0: smt 0, core 0, package 0
mtrr: Pentium Pro MTRR support, 8 var ranges, 88 fixed ranges
cpu0: apic clock running at 65MHz
cpu1 at mainbus0: apid 2 (application processor)
cpu1: failed to identify
On 5/11/22 11:04, Scott Cheloha wrote:
On Tue, Mar 29, 2022 at 10:24:03AM -0500, Scott Cheloha wrote:
On Tue, Mar 29, 2022 at 03:26:49PM +1100, Jonathan Gray wrote:
On Mon, Mar 28, 2022 at 10:52:09PM -0500, Scott Cheloha wrote:
I want to use the IA32_TSC_ADJUST MSR where available when testing TSC
synchronization. We note if it's available during CPU identification.
Can we do CPU identification earlier in cpu_hatch() and
cpu_start_secondary(), before we do the TSC sync testing?
This can wait until after release. I'm just trying to suss out
whether there is an order dependency I'm not seeing. My laptop
appears to boot and resume no differently with this patch.
Thoughts?
The rest aside, moving the cpu_ucode_apply() call to after the
identifycpu() call is wrong as microcode can add cpuid bits.
I would keep cpu_tsx_disable() before it as well.
Okay, moved them up.
I'm sure I've had problems trying to change the sequencing
of lapic, tsc freq and identify in the past. It caused problems
only on certain machines.
[...]
6 week bump + rebase.
Once again, I want to do CPU identification before the TSC sync test
so we can check for and use the IA32_TSC_ADJUST MSR during the sync
test.
Does anyone understand amd64 CPU startup well enough to say whether
this rearrangement is going to break something?
Is this ok?
Index: cpu.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/amd64/cpu.c,v
retrieving revision 1.156
diff -u -p -r1.156 cpu.c
--- cpu.c 26 Apr 2022 08:35:30 -0000 1.156
+++ cpu.c 11 May 2022 02:00:37 -0000
@@ -852,20 +852,7 @@ cpu_start_secondary(struct cpu_info *ci)
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) {
@@ -875,11 +862,28 @@ cpu_start_secondary(struct cpu_info *ci)
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);
@@ -940,18 +944,8 @@ cpu_hatch(void *v)
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);
@@ -970,6 +964,17 @@ cpu_hatch(void *v)
/* 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);
+
+ lapic_enable();
+ lapic_startclock();
while ((ci->ci_flags & CPUF_GO) == 0)
delay(10);
--
Yuichiro NAITO (naito.yuich...@gmail.com)
