On 09/17/08 18:02, Li, Aubrey wrote: > 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 >
Can I push the fix into hpet_pcplusmp along with the fix to properly check/initialize the hpet? Bill
