Date: Sat, 28 Aug 2010 23:22:59 +0300 From: Andriy Gapon <avg@freebsd.org> To: Jung-uk Kim <jkim@freebsd.org>, freebsd-stable@freebsd.org Cc: pluknet <pluknet@gmail.com> Subject: Re: 8.1-PRERELEASE: CPU packages not detected correctly Message-ID: <4C797023.8020206@freebsd.org> In-Reply-To: <4C7835E6.6070309@icyb.net.ua> References: <201007141414.o6EEEUx9014690@lurza.secnetix.de> <AANLkTinYUz0V%2B2nSWBMYLf2fL8HnUQ-fvXT0q-5WY4bb@mail.gmail.com> <4C782D3B.6020407@icyb.net.ua> <201008271743.29393.jkim@FreeBSD.org> <4C7835E6.6070309@icyb.net.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
This is a multi-part message in MIME format.
--------------000301090401000705080907
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
on 28/08/2010 01:02 Andriy Gapon said the following:
> BTW, it may be not that hard.
> It seems that 0x4 topology building involves knowing the masks and we already
> have that data (just interpreted differently), and APIC IDs of the CPUs and it
> seems that we also have that. We don't need to bind to CPUs to learn their IDs,
> we can just iterate over cpu_apic_ids[].
>
> The only problem is that currently topo_probe() is called before
> assign_cpu_ids() which populates cpu_apic_ids.
> assign_cpu_ids depends on topo_probe to know hyperthreading_cpus value.
> So, either cpu_apic_ids could be split out or alternatively we could use
> cpu_info[] similarly to how it's done in topo_probe_0xb (skipping !cpu_present
> and cpu_disabled entries).
So, here is my take on the problem.
The attached patch is against sources in FreeBSD tree, it should be applied
either to sys/amd64/amd64/mp_machdep.c or sys/i386/i386/mp_machdep.c depending
on the desired architecture.
The patch is substantially based on the Junk-uk's patch, but with some changes
and additions:
- topo_probo_0x4() is rewritten so that it does APIC ID matching against masks
as described in the Intel article. The code still heavily depends on the
assumption of the uniform topology, it discovers number of cores in BSP package
and number of threads in BSP core and extrapolates that to global topology.
The difference with current code and Junk-uk's patch is that actual APIC ID
matching is done as opposed to deriving counts purely from max. values.
- added a few comments that describe uniformity assumption, plus couple other
useful things.
- changed "final fallback" code, so that each logical CPU is considered to be in
its own physical package as opposed to current code placing all logical CPUs as
cores of a single package.
The rest is Junk-uk's work.
Concerns:
- about my code: ilog2_round_pow2 name is ugly; looking for suggestions on a
better name or re-arranging/writing that code, so that the function is not needed.
- about current code: logical_cpus variable (don't confuse with cpu_logical)
doesn't seem to be consistently used; e.g. it is not set at all by
topo_probo_0xb(); also, the method of using it for setting logical_cpus_mask
doesn't seem to be reliable - BSP may be missed.
Reviews, comments and test reports are very welcome!
Please test the patch if you have any problems with how CPU topology is reported
by the current code. Please test even if everything is OK, to avoid regressions.
Thanks!
--
Andriy Gapon
--------------000301090401000705080907
Content-Type: text/plain;
name="intel-cpu-topo.diff"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename="intel-cpu-topo.diff"
diff --git a/sys/amd64/amd64/mp_machdep.c b/sys/amd64/amd64/mp_machdep.c
index e2f82ec..47a5c0b 100644
--- a/sys/amd64/amd64/mp_machdep.c
+++ b/sys/amd64/amd64/mp_machdep.c
@@ -177,24 +177,101 @@ mem_range_AP_init(void)
}
static void
+topo_probe_amd(void)
+{
+
+ /* AMD processors do not support HTT. */
+ cpu_cores = (amd_feature2 & AMDID2_CMP) != 0 ?
+ (cpu_procinfo2 & AMDID_CMP_CORES) + 1 : 1;
+ cpu_logical = 1;
+}
+
+/*
+ * Round up to the next power of two, if necessary, and then
+ * take log2.
+ */
+static __inline int
+ilog2_round_pow2(u_int x)
+{
+
+ return fls(x << (1 - powerof2(x)));
+}
+
+static void
+topo_probe_0x4(void)
+{
+ u_int p[4];
+ int core_id_bits;
+ int smt_id_bits;
+ int max_cores;
+ int max_logical;
+ int id;
+
+ max_logical = (cpu_feature & CPUID_HTT) != 0 ?
+ (cpu_procinfo & CPUID_HTT_CORES) >> 16 : 1;
+ if (max_logical == 1)
+ return;
+
+ /*
+ * Because of uniformity assumption we examine only
+ * those logical processors that belong to the same
+ * package as BSP. Further, we count number of
+ * logical processors that belong to the same core
+ * as BSP thus deducing number of threads per core.
+ */
+ cpuid_count(0x04, 0, p);
+ max_cores = ((p[0] >> 26) & 0x3f) + 1;
+ smt_id_bits = ilog2_round_pow2(max_logical/max_cores) - 1;
+ if (smt_id_bits < 0)
+ return;
+ core_id_bits = smt_id_bits + fls(max_cores) - 1;
+
+ for (id = 0; id <= MAX_APIC_ID; id++) {
+ if (!cpu_info[id].cpu_present || cpu_info[id].cpu_disabled)
+ continue;
+ if ((id >> core_id_bits) != (boot_cpu_id >> core_id_bits))
+ continue;
+ cpu_cores++;
+ if ((id >> smt_id_bits) == (boot_cpu_id >> smt_id_bits))
+ cpu_logical++;
+ }
+
+ cpu_cores /= cpu_logical;
+ if (cpu_logical > 1)
+ hyperthreading_cpus = logical_cpus = cpu_logical;
+}
+
+static void
topo_probe_0xb(void)
{
- int logical;
- int p[4];
+ u_int p[4];
int bits;
- int type;
int cnt;
int i;
+ int logical;
+ int type;
int x;
- /* We only support two levels for now. */
+ /* We only support three levels for now. */
for (i = 0; i < 3; i++) {
- cpuid_count(0x0B, i, p);
+ cpuid_count(0x0b, i, p);
+
+ /* Fall back if CPU leaf 11 doesn't really exist. */
+ if (i == 0 && p[1] == 0) {
+ topo_probe_0x4();
+ return;
+ }
+
bits = p[0] & 0x1f;
logical = p[1] &= 0xffff;
type = (p[2] >> 8) & 0xff;
if (type == 0 || logical == 0)
break;
+ /*
+ * Because of uniformity assumption we examine only
+ * those logical processors that belong to the same
+ * package as BSP.
+ */
for (cnt = 0, x = 0; x <= MAX_APIC_ID; x++) {
if (!cpu_info[x].cpu_present ||
cpu_info[x].cpu_disabled)
@@ -212,76 +289,16 @@ topo_probe_0xb(void)
cpu_cores /= cpu_logical;
}
-static void
-topo_probe_0x4(void)
-{
- u_int threads_per_cache, p[4];
- u_int htt, cmp;
- int i;
-
- htt = cmp = 1;
- /*
- * If this CPU supports HTT or CMP then mention the
- * number of physical/logical cores it contains.
- */
- if (cpu_feature & CPUID_HTT)
- htt = (cpu_procinfo & CPUID_HTT_CORES) >> 16;
- if (cpu_vendor_id == CPU_VENDOR_AMD && (amd_feature2 & AMDID2_CMP))
- cmp = (cpu_procinfo2 & AMDID_CMP_CORES) + 1;
- else if (cpu_vendor_id == CPU_VENDOR_INTEL && (cpu_high >= 4)) {
- cpuid_count(4, 0, p);
- if ((p[0] & 0x1f) != 0)
- cmp = ((p[0] >> 26) & 0x3f) + 1;
- }
- cpu_cores = cmp;
- cpu_logical = htt / cmp;
-
- /* Setup the initial logical CPUs info. */
- if (cpu_feature & CPUID_HTT)
- logical_cpus = (cpu_procinfo & CPUID_HTT_CORES) >> 16;
-
- /*
- * Work out if hyperthreading is *really* enabled. This
- * is made really ugly by the fact that processors lie: Dual
- * core processors claim to be hyperthreaded even when they're
- * not, presumably because they want to be treated the same
- * way as HTT with respect to per-cpu software licensing.
- * At the time of writing (May 12, 2005) the only hyperthreaded
- * cpus are from Intel, and Intel's dual-core processors can be
- * identified via the "deterministic cache parameters" cpuid
- * calls.
- */
- /*
- * First determine if this is an Intel processor which claims
- * to have hyperthreading support.
- */
- if ((cpu_feature & CPUID_HTT) && cpu_vendor_id == CPU_VENDOR_INTEL) {
- /*
- * If the "deterministic cache parameters" cpuid calls
- * are available, use them.
- */
- if (cpu_high >= 4) {
- /* Ask the processor about the L1 cache. */
- for (i = 0; i < 1; i++) {
- cpuid_count(4, i, p);
- threads_per_cache = ((p[0] & 0x3ffc000) >> 14) + 1;
- if (hyperthreading_cpus < threads_per_cache)
- hyperthreading_cpus = threads_per_cache;
- if ((p[0] & 0x1f) == 0)
- break;
- }
- }
-
- /*
- * If the deterministic cache parameters are not
- * available, or if no caches were reported to exist,
- * just accept what the HTT flag indicated.
- */
- if (hyperthreading_cpus == 0)
- hyperthreading_cpus = logical_cpus;
- }
-}
-
+/*
+ * Both topology discovery code and code that consumes topology
+ * information assume top-down uniformity of the topology.
+ * That is, all physical packages must be identical and each
+ * core in a package must have the same number of threads.
+ * Topology information is queried only on BSP, on which this
+ * code runs and for which it can query CPUID information.
+ * Then topology is extrapolated on all packages using the
+ * uniformity assumption.
+ */
static void
topo_probe(void)
{
@@ -291,12 +308,26 @@ topo_probe(void)
return;
logical_cpus = logical_cpus_mask = 0;
- if (cpu_high >= 0xb)
- topo_probe_0xb();
- else if (cpu_high)
- topo_probe_0x4();
+ if (cpu_vendor_id == CPU_VENDOR_AMD)
+ topo_probe_amd();
+ else if (cpu_vendor_id == CPU_VENDOR_INTEL) {
+ /*
+ * See Intel(R) 64 Architecture Processor
+ * Topology Enumeration article for details.
+ */
+ if (cpu_high >= 0xb)
+ topo_probe_0xb();
+ else if (cpu_high >= 0x4)
+ topo_probe_0x4();
+ }
+
+ /*
+ * Fallback: assume each logical CPU is in separate
+ * physical package. That is, no multi-core,
+ * no SMT.
+ */
if (cpu_cores == 0)
- cpu_cores = mp_ncpus > 0 ? mp_ncpus : 1;
+ cpu_cores = 1;
if (cpu_logical == 0)
cpu_logical = 1;
cpu_topo_probed = 1;
--------------000301090401000705080907--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4C797023.8020206>