Re: [PATCH v2 1/8] x86/CPU/AMD: Save NodeId on AMD-based systems

[Yazen Ghannam <yazen.ghannam@amd.com>]

On Thu, Sep 17, 2020 at 12:37:20PM +0200, Borislav Petkov wrote:
> On Wed, Sep 16, 2020 at 02:51:52PM -0500, Yazen Ghannam wrote:
> > What do you think?
> 
> Yeah, forget logical_proc_id - the galactic senate of x86 maintainers
> said that we're keeping that for when BIOS vendors f*ck up with the
> phys_proc_id enumeration on AMD. Then we'll need that as a workaround.
> 
> Look instead at:
> 
> struct cpuinfo_x86 {
> 
> 	...
> 
>         u16                     cpu_die_id;
>         u16                     logical_die_id;
> 
> and
> 
> 7745f03eb395 ("x86/topology: Add CPUID.1F multi-die/package support")
> 
> "Some new systems have multiple software-visible die within each
> package."
> 
> and you could map the AMD packages to those dies. And if you guys
> implement CPUID.1F to enumerate those packages the same way, then all
> should just work (famous last words).
>
> Because Intel dies is basically AMD packages consisting of a CCX, caches
> and DF.
> 
> We would have to update the documentation in the end to denote that but
> let's see if this should work for you too first. Because the concepts
> sound very similar, if not identical...
>

Yep, we could ask the hardware folks to implement CPUID Leaf 0x1F, but
that'll be in some future products. 

I actually tried using cpu_die_id, but I ran into an issue on newer
systems.

On older systems, there is no CPUID Leaf 0xB or 0x1F, and cpu_die_id
doesn't get explicitly set. So setting cpu_die_id equal to AMD NodeId
would work. But newer systems support CPUID Leaf 0xB, so cpu_die_id
will get explicitly set by detect_extended_topology(). The value set is
different from the AMD NodeId. And at that point I shied away from
doing any override or fixup.

Thanks,
Yazen