Bill.Holler at Sun.COM wrote: > 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
Yeah, totally agree. -Aubrey
