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.
Regards,
Bill
