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

From: Yazen Ghannam
Date: Wed Sep 16 2020 - 15:52:34 EST

On Tue, Sep 15, 2020 at 10:35:15AM +0200, Borislav Petkov wrote:
> > Yeah, I think example 4b works here. The mismatch though is with
> > phys_proc_id and package on AMD systems. You can see above that
> > phys_proc_id gives a socket number, and the AMD NodeId gives a package
> > number.
> Ok, now looka here:
> " - cpuinfo_x86.logical_proc_id:
> The logical ID of the package. As we do not trust BIOSes to enumerate the
> packages in a consistent way, we introduced the concept of logical package
> ID so we can sanely calculate the number of maximum possible packages in
> the system and have the packages enumerated linearly."
> Doesn't that sound like exactly what you need?
> Because that DF ID *is* practically the package ID as there's 1:1
> mapping between DF and a package, as you say above.
> Right?
> Now, it says
> [ 7.670791] smpboot: Max logical packages: 2
> on my Rome box but what you want sounds very much like the logical
> package ID and if we define that on AMD to be that and document it this
> way, I guess that should work too, provided there are no caveats like
> sched is using this info for proper task placement and so on. That would
> need code audit, of course...

The only use of logical_proc_id seems to be in hswep_uncore_cpu_init().
So I think maybe we can use this.

However, I think there are two issues.

1) The logical_proc_id seems like it should refer to the same type of
structure as phys_proc_id. In our case, this won't be true as
phys_proc_id would refer to the "socket" on AMD and logical_proc_id
would refer to the package/AMD NodeId.

2) The AMD NodeId is read during c_init()/init_amd(), so logical_proc_id
can be set here. But then logical_proc_id will get overwritten later in
topology_update_package_map(). I don't know if it'd be good to modify
the generic flow to support this vendor-specific behavior.

What do you think?