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

[Yazen Ghannam]

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?

Thanks,
Yazen