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

Andrew Jones
2018-06-29T11:24:02Z

On Fri, Jun 29, 2018 at 11:29:27AM +0100, Sudeep Holla wrote:
> On Thu, Jun 28, 2018 at 07:32:43PM +0200, Andrew Jones wrote:
> > On Thu, Jun 28, 2018 at 05:30:51PM +0100, 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.
> >
> > I don't believe we have to guarantee that the exact (package,core,thread)
> > triplet describing a PE with DT matches ACPI. We just need to guarantee
> > that each triplet we select properly puts a PE in the same group as its
> > peers. So, as long as we keep the grouping described by DT or ACPI, then
> > the (package,core,thread) IDs assigned are pretty arbitrary.
> >
>
> If that's the requirement, we already do that. The IDs are just too
> arbitrary :)

Right. The patch doesn't fix anything, it just makes the IDs less weird
looking to humans, and brings some consistency to IDs across systems
and hardware descriptions.

>
> > I could change the commit message to state we can generate IDs *like*
> > DT does (i.e. with counters), even if they may not result in identical
> > triplet to PE mappings.
> >
>
> Why we need to make it *like DT* ?

Because the DT parser considers human readers, which, IMHO, is nicer.

The consistency remapping to counters brings shouldn't be overlooked. If
both ACPI and DT present the CPUs in the same order (i.e. cpu0 is mpidr0,
cpu1 is mpidr1, ...) using their respective descriptions, then this patch
will ensure topology IDs are the same. Also, when going from machine to
machine, it's nice to expect package/core/thread-id to be within
the same range and to vary by a set difference (1), rather than have
IDs that increment by 20 on one system and by 160 on another,
depending on how the ACPI table nodes are laid out.

>
> > >
> > > So I would like to keep it simple and just have this counters for
> > > package ids as demonstrated in Shunyong's patch.
> > >
> >
> > If we don't also handle cores when there are threads, then the cores
> > will also end up having weird IDs.
> >
>
> Yes, but if PPTT says it has valid ID, I would prefer that over DT like
> generated.

Valid *ACPI* ID, which just means it's a guaranteed unique ACPI UID,
which isn't likely going to be anything useful to a user.

Thanks,
drew