Re: [PATCH] arm64: acpi: reenumerate topology ids

From: Andrew Jones
Date: Fri Jun 29 2018 - 07:42:36 EST


On Fri, Jun 29, 2018 at 11:53:34AM +0100, Sudeep Holla wrote:
> On Thu, Jun 28, 2018 at 12:12:00PM -0500, Jeremy Linton wrote:
> > Hi,
> >
> > On 06/28/2018 11:30 AM, Sudeep Holla wrote:
>
> [...]
>
> > >I am not sure if we can ever guarantee that DT and ACPI will get the
> > >same ids whatever counter we use as it depends on the order presented in
> > >the firmware(DT or ACPI). So I am not for generating ids for core and
> > >threads in that way.
> > >
> > >So I would like to keep it simple and just have this counters for
> > >package ids as demonstrated in Shunyong's patch.
> >
> > So, currently on a non threaded system, the core id's look nice because they
> > are just the ACPI ids. Its the package id's that look strange, we could just
> > fix the package ids, but on threaded machines the threads have the nice acpi
> > ids, and the core ids are then funny numbers. So, I suspect that is driving
> > this as much as the strange package ids.
> >
>
> Yes, I know that and that's what made be look at topology_get_acpi_cpu_tag
> For me, if the PPTT has valid ID, we should use that. Just becuase DT lacks
> it and uses counter doesn't mean ACPI also needs to follow that.

AFAIK, a valid ACPI UID doesn't need to be something derivable directly
from the hardware, so it's just as arbitrary as the CPU phandle that is
in the DT cpu-map, i.e. DT *does* have an analogous leaf node integer.

>
> I am sure some vendor will put valid UID and expect that to be in the
> sysfs.

I can't think of any reason that would be useful, especially when the
UID is for a thread, which isn't even displayed by sysfs.

>
> > (and as a side, I actually like the PE has a acpi id behavior, but for
> > threads its being lost with this patch...)
> >
> > Given i've seen odd package/core ids on x86s a few years ago, it never

So this inspired me to grep some x86 topology code. I found
arch/x86/kernel/smpboot.c:topology_update_package_map(), which uses
a counter to set the logical package id and Documentation/x86/topology.txt
states

"""
- cpuinfo_x86.logical_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.
"""

Which I see as x86 precedent for the consistency argument I made in my
other reply.

Thanks,
drew