Bill.Holler wrote: > On 09/17/08 13:25, Bill Holler wrote: >> On 09/17/08 13:08, Bill Holler wrote: >> >>> On 09/17/08 02:41, Li, Aubrey wrote: >>> >>> >>>> Hi Bill, >>>> >>>> Bill.Holler wrote: >>>> >>>> >>>> >>>> >>>>> updated 3301 Should check for HPET support before enabling Deep >>>>> >>>>> >>>>> >>>> It doesn't work on my side. >>>> In case something wrong in hpet gate, I uses onnv_97 as my kernel >>>> build. And I got: >>>> ======================================================= $ cat >>>> mdb/cpi_xmaxeax.txt >>>>>> walk cpu | ::print cpu_t cpu_m | ::print -t "struct > machcpu" mcpu_cpi >>>>>>> print -t "struct cpuid_info" cpi_xmaxeax >>>> $ cat mdb/cpi_xmaxeax.txt | pfexec mdb -k >>>> uint_t cpi_xmaxeax = 0 >>>> uint_t cpi_xmaxeax = 0 >>>> uint_t cpi_xmaxeax = 0 >>>> uint_t cpi_xmaxeax = 0 >>>> uint_t cpi_xmaxeax = 0 >>>> uint_t cpi_xmaxeax = 0 >>>> uint_t cpi_xmaxeax = 0 >>>> uint_t cpi_xmaxeax = 0 >>>> ======================================================= >>>> This is weird, because on onnv_95, cpi_xmaxeax = 0x80000008 on my >>>> side. I also tried the head(Rev7623) of onnv-gate, this issue >>>> remains. I posted it here to see if it's a known bug. >>>> >>>> Thanks, >>>> -Aubrey >>>> _______________________________________________ >>>> tesla-dev mailing list >>>> tesla-dev at opensolaris.org >>>> http://mail.opensolaris.org/mailman/listinfo/tesla-dev >>>> >>>> >>>> >>> Something stepped on cpi_xmaxeax. :-( >>> A memory write breakpoint on cpuid_info0.cpi_xmaxeax fires >>> in boot in cpuid_pass2: >>> [0]> :c >>> kmdb: stop on write of [cpuid_info0+0xc0, cpuid_info0+0xc1) >>> kmdb: target stopped at: >>> cpuid_pass2+0x23a: movl $0x0,0x8(%r12) >>> >>> Instruction cpuid_pass2+0x232 triggered the breakpoint. >>> mdb reports the instruction *after* the instruction that hit >>> triggered the break point. >>> >>> cpuid_pass2+0x22c: jne +0x10b <cpuid_pass2+0x33d> >>> cpuid_pass2+0x232: movl $0xb,(%r12) >>> cpuid_pass2+0x23a: movl $0x0,0x8(%r12) >>> >>> %r12 contains the address of cpi_xmaxeax. >>> >>> None of the C source for cpuid_pass2 writes to cpi_xmaxeax. >>> I am looking into the assembly to match this line with the C code. >>> >>> >>> It looks like we also need to add code in cpuid_pass1's extended- >>> cpuid section to get 0x80000007 TSC info. >>> >>> Regards, >>> Bill >>> _______________________________________________ >>> tesla-dev mailing list >>> tesla-dev at opensolaris.org >>> http://mail.opensolaris.org/mailman/listinfo/tesla-dev >>> >>> >> >> This code in cpuid_pass2() steps on cpi->xmaxeax: >> >> if (cpi->cpi_maxeax >= 0xB && cpi->cpi_vendor == >> X86_VENDOR_Intel) { cp->cp_eax = 0xB; >> cp->cp_ecx = 0; >> >> (void) __cpuid_insn(cp); >> >> >> Apparently we walked off the end of the cpi->cpi_std[] array. >> Here is the code above that went too far: >> >> if ((nmax = cpi->cpi_maxeax + 1) > NMAX_CPI_STD) >> nmax = NMAX_CPI_STD; >> /* >> * (We already handled n == 0 and n == 1 in pass 1) >> */ for (n = 2, cp = &cpi->cpi_std[2]; n < nmax; n++, cp++) { >> cp->cp_eax = n; >> >> ... >> } >> >> When this for loop is done, cp is pointing at >> &cpi->cpi_std[NMAX_CPI_STD]. But the array is only NMAX_CPI_STD >> elements, so this points at xmaxeax instead of a valid array >> element. :-( >> >> Regards, >> Bill >> >> >> _______________________________________________ >> tesla-dev mailing list >> tesla-dev at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/tesla-dev >> > > > A possible fix is to have this clause in cpuid_pass2(0 allocate a > struct cpuid_regs and point cp at it instead of using the stale cp > pointer which points one passed the end of cpi->cpi_std[]. > It does not look like this clause cares to save the results from > cpuid after parsing it. > > if (cpi->cpi_maxeax >= 0xB && cpi->cpi_vendor == > X86_VENDOR_Intel) { > > struct cpuid_regs regs; <--- possible fix > > cp = ®s; <--- possible fix > cp->cp_eax = 0xB; > cp->cp_ecx = 0; > > (void) __cpuid_insn(cp); > > /* > * Check CPUID.EAX=0BH, ECX=0H:EBX is non-zero, which > * indicates that the extended topology > enumeration leaf is > * available. > */ > if (cp->cp_ebx) { > uint32_t x2apic_id; > uint_t coreid_shift = 0; > uint_t ncpu_per_core = 1; > uint_t chipid_shift = 0; > uint_t ncpu_per_chip = 1; > uint_t i; > uint_t level; > > for (i = 0; i < CPI_FNB_ECX_MAX; i++) { > cp->cp_eax = 0xB; > cp->cp_ecx = i; > > (void) __cpuid_insn(cp); > level = CPI_CPU_LEVEL_TYPE(cp); > > if (level == 1) { > x2apic_id = cp->cp_edx; > coreid_shift = BITX(cp->cp_eax, > 4, 0); > ncpu_per_core = > BITX(cp->cp_ebx, 15, 0); > } else if (level == 2) { > x2apic_id = cp->cp_edx; > chipid_shift = BITX(cp->cp_eax, > 4, 0); > ncpu_per_chip = > BITX(cp->cp_ebx, 15, 0); > } > } > > cpi->cpi_apicid = x2apic_id; > cpi->cpi_ncpu_per_chip = ncpu_per_chip; > cpi->cpi_ncore_per_chip = ncpu_per_chip / > ncpu_per_core; > cpi->cpi_chipid = x2apic_id >> chipid_shift; > cpi->cpi_clogid = x2apic_id & ((1 << > chipid_shift) - 1); cpi->cpi_coreid = > x2apic_id >> coreid_shift; cpi->cpi_pkgcoreid > = cpi->cpi_clogid >> coreid_shift; } > } > > > I will file a CR against this. > We probably need to fix this first in our gate? Or just fix it locally to make the test further?
Thanks, -Aubrey
