8.1-PRERELEASE: CPU packages not detected correctly

[Jung-uk Kim]

On Thursday 15 July 2010 01:56 pm, Andriy Gapon wrote:
> on 15/07/2010 19:57 Oliver Fromme said the following:
> > In topo_probe(), cpu_high is 0xd, so topo_probe_0xb() is
> > called.  But the cpuid 0xb instruction doesn't seem to
> > return useful data:  All values are zero already in the
> > first level, so cpu_cores remains 0.
> >
> > Back in topo_probe(), there is a fallback if cpu_cores is
> > stil 0:  It assigns mp_ncpu to cpu_cores, so it gets 8
> > which is wrong.
> >
> > I patched topo_probe() so it calls topo_probe_0x4() after
> > topo_probe_0xb() if cpu_cores is still 0.  I think this
> > is a better fallback procedure.  With this patch, cpu_cores
> > gets the value 4 which is the correct one, finally:
> >
> > FreeBSD/SMP: Multiprocessor System Detected: 8 CPUs
> > FreeBSD/SMP: 2 package(s) x 4 core(s)
>
> Thank you for debugging this issue!
> Not sure if this is the best patch that there can be, but its
> direction is definitely correct.
> As the Intel document says (translated to our x86 mp_machdep.c
> terms): if cpu_high >= 0xb then we should execute cpuid_count(0xb,
> 0, p) and examine EBX value (p[1]), only if it's non-zero should we
> proceed with topo_probe_0xb(), otherwise we should fall back to
> topo_probe_0x4, etc.
>
> I think that your addition achieves this effect, perhaps just not
> as explicitly as I would preferred.
>
> Jung-uk, what do you think?

Yes, you're right.  Please try new patch:

http://people.freebsd.org/~jkim/mp_machdep2.diff

Thanks,

Jung-uk Kim

> > This is the patch, it's against jkim's patched version:
> > @@ -265,7 +268,7 @@
> >  	else if (cpu_vendor_id == CPU_VENDOR_INTEL) {
> >  		if (cpu_high >= 0xb)
> >  			topo_probe_0xb();
> > -		else if (cpu_high >= 0x4)
> > +		if (cpu_high >= 0x4 && cpu_cores == 0)
> >  			topo_probe_0x4();
> >  	}
> >  	if (cpu_cores == 0)