> Date: Wed, 20 Jun 2018 20:29:51 +1000
> From: Jonathan Matthew <jonat...@d14n.org>
>
> On Wed, Jun 20, 2018 at 10:38:41AM +0100, Stuart Henderson wrote:
> > On 2018/06/20 05:53, Leo Unglaub wrote:
> > > Hi,
> > > I applied your code on my AMD Ryzen 7 1700X. Below is the dmesg. I hope
> > > this
> > > helps, if you have any other AMD Ryzen related stuff that needs testing
> > > please let me know.
> >
> > So there's no convenient "smt id" returned..
> >
> >
> > The reacharound is a bit ugly, but would something like this do the trick?
> > I don't have a suitable AMD to test, but have tested the mechanism on an
> > i7 with HT and it does what's expected.
>
> Here's what this does on a single socket epyc 7551p:
>
> cpu0: smt 0, core 0, package 0
> cpu1: smt 0, core 8, package 1
> cpu2: smt 0, core 16, package 2
> cpu3: smt 0, core 24, package 3
> cpu4: smt 0, core 4, package 0
> cpu5: smt 0, core 12, package 1
> cpu6: smt 0, core 20, package 2
> cpu7: smt 0, core 28, package 3
> cpu8: smt 0, core 1, package 0
> cpu9: smt 0, core 9, package 1
> cpu10: smt 0, core 17, package 2
> cpu11: smt 0, core 25, package 3
> cpu12: smt 0, core 5, package 0
> cpu13: smt 0, core 13, package 1
> cpu14: smt 0, core 21, package 2
> cpu15: smt 0, core 29, package 3
> cpu16: smt 0, core 2, package 0
> cpu17: smt 0, core 10, package 1
> cpu18: smt 0, core 18, package 2
> cpu19: smt 0, core 26, package 3
> cpu20: smt 0, core 6, package 0
> cpu21: smt 0, core 14, package 1
> cpu22: smt 0, core 22, package 2
> cpu23: smt 0, core 30, package 3
> cpu24: smt 0, core 3, package 0
> cpu25: smt 0, core 11, package 1
> cpu26: smt 0, core 19, package 2
> cpu27: smt 0, core 27, package 3
> cpu28: smt 0, core 7, package 0
> cpu29: smt 0, core 15, package 1
> cpu30: smt 0, core 23, package 2
> cpu31: smt 0, core 31, package 3
> cpu32: smt 1, core 0, package 0
> cpu33: smt 1, core 8, package 1
> cpu34: smt 1, core 16, package 2
> cpu35: smt 1, core 24, package 3
> cpu36: smt 1, core 4, package 0
> cpu37: smt 1, core 12, package 1
> cpu38: smt 1, core 20, package 2
> cpu39: smt 1, core 28, package 3
> cpu40: smt 1, core 1, package 0
> cpu41: smt 1, core 9, package 1
> cpu42: smt 1, core 17, package 2
> cpu43: smt 1, core 25, package 3
> cpu44: smt 1, core 5, package 0
> cpu45: smt 1, core 13, package 1
> cpu46: smt 1, core 21, package 2
> cpu47: smt 1, core 29, package 3
> cpu48: smt 1, core 2, package 0
> cpu49: smt 1, core 10, package 1
> cpu50: smt 1, core 18, package 2
> cpu51: smt 1, core 26, package 3
> cpu52: smt 1, core 6, package 0
> cpu53: smt 1, core 14, package 1
> cpu54: smt 1, core 22, package 2
> cpu55: smt 1, core 30, package 3
> cpu56: smt 1, core 3, package 0
> cpu57: smt 1, core 11, package 1
> cpu58: smt 1, core 19, package 2
> cpu59: smt 1, core 27, package 3
> cpu60: smt 1, core 7, package 0
> cpu61: smt 1, core 15, package 1
> cpu62: smt 1, core 23, package 2
> cpu63: smt 1, core 31, package 3
>
> It's probably reasonable to think of nodes as being like separate
> pacakges. I'm not sure what happens in two socket systems though.
According to the documentation, a node corresponds to a die and there
are 4 dies in a package. So package is a bit of a misnomer here, but
since each die has its own memory controller you could argue that
treating dies as separate packages is a reasonable description of the
topology. Although dies in a single package do seem to be coupled
somewhat tighter as dies within a package are directly connected
whereas dies in different packages may not be.
My expectation is that on a dual socket machine the nodes in the 2nd
socket will be numbered 4-7.
> If I turn SMT off, I get this, which doesn't quite look right:
>
> cpu0: smt 0, core 0, package 0
> cpu1: smt 0, core 16, package 1
> cpu2: smt 0, core 32, package 2
> cpu3: smt 0, core 48, package 3
> cpu4: smt 0, core 8, package 0
> cpu5: smt 0, core 24, package 1
> cpu6: smt 0, core 40, package 2
> cpu7: smt 0, core 56, package 3
> cpu8: smt 0, core 1, package 0
> cpu9: smt 0, core 17, package 1
> cpu10: smt 0, core 33, package 2
> cpu11: smt 0, core 49, package 3
> cpu12: smt 0, core 9, package 0
> cpu13: smt 0, core 25, package 1
> cpu14: smt 0, core 41, package 2
> cpu15: smt 0, core 57, package 3
> cpu16: smt 0, core 2, package 0
> cpu17: smt 0, core 18, package 1
> cpu18: smt 0, core 34, package 2
> cpu19: smt 0, core 50, package 3
> cpu20: smt 0, core 10, package 0
> cpu21: smt 0, core 26, package 1
> cpu22: smt 0, core 42, package 2
> cpu23: smt 0, core 58, package 3
> cpu24: smt 0, core 3, package 0
> cpu25: smt 0, core 19, package 1
> cpu26: smt 0, core 35, package 2
> cpu27: smt 0, core 51, package 3
> cpu28: smt 0, core 11, package 0
> cpu29: smt 0, core 27, package 1
> cpu30: smt 0, core 43, package 2
> cpu31: smt 0, core 59, package 3
Well, it doesn't look wrong to me. What's odd is that it seems the
cores got renumbered. Cores 4-7, 12-15, etc. are no longer there.
But there now are cores with numbers in the 32-63 range. As long as
we treat ci_core_id as just a number, that shouldn't be an issue
though.
So I think the patch is fine.
ok kettenis@
> > Index: identcpu.c
> > ===================================================================
> > RCS file: /cvs/src/sys/arch/amd64/amd64/identcpu.c,v
> > retrieving revision 1.96
> > diff -u -p -r1.96 identcpu.c
> > --- identcpu.c 7 Jun 2018 04:07:28 -0000 1.96
> > +++ identcpu.c 20 Jun 2018 09:26:08 -0000
> > @@ -792,17 +792,32 @@ cpu_topology(struct cpu_info *ci)
> > if (ci->ci_pnfeatset < 0x80000008)
> > goto no_topology;
> >
> > - CPUID(0x80000008, eax, ebx, ecx, edx);
> > - core_bits = (ecx >> 12) & 0xf;
> > - if (core_bits == 0)
> > - goto no_topology;
> > - /* So coreidsize 2 gives 3, 3 gives 7... */
> > - core_mask = (1 << core_bits) - 1;
> > - /* Core id is the least significant considering mask */
> > - ci->ci_core_id = apicid & core_mask;
> > - /* Pkg id is the upper remaining bits */
> > - ci->ci_pkg_id = apicid & ~core_mask;
> > - ci->ci_pkg_id >>= core_bits;
> > + if (ci->ci_pnfeatset >= 0x8000001e) {
> > + struct cpu_info *ci_other;
> > + CPU_INFO_ITERATOR cii;
> > +
> > + CPUID(0x8000001e, eax, ebx, ecx, edx);
> > + ci->ci_core_id = ebx & 0xff;
> > + ci->ci_pkg_id = ecx & 0xff;
> > + ci->ci_smt_id = 0;
> > + CPU_INFO_FOREACH(cii, ci_other) {
> > + if (ci != ci_other &&
> > + ci_other->ci_core_id == ci->ci_core_id)
> > + ci->ci_smt_id++;
> > + }
> > + } else {
> > + CPUID(0x80000008, eax, ebx, ecx, edx);
> > + core_bits = (ecx >> 12) & 0xf;
> > + if (core_bits == 0)
> > + goto no_topology;
> > + /* So coreidsize 2 gives 3, 3 gives 7... */
> > + core_mask = (1 << core_bits) - 1;
> > + /* Core id is the least significant considering mask */
> > + ci->ci_core_id = apicid & core_mask;
> > + /* Pkg id is the upper remaining bits */
> > + ci->ci_pkg_id = apicid & ~core_mask;
> > + ci->ci_pkg_id >>= core_bits;
> > + }
> > } else if (strcmp(cpu_vendor, "GenuineIntel") == 0) {
> > /* We only support leaf 1/4 detection */
> > if (cpuid_level < 4)
> >
>
>
