linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Sudeep Holla <sudeep.holla@arm.com>
To: Andrew Jones <drjones@redhat.com>
Cc: Jeremy Linton <jeremy.linton@arm.com>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, ard.biesheuvel@linaro.org,
	shunyong.yang@hxt-semitech.com, yu.zheng@hxt-semitech.com,
	catalin.marinas@arm.com, will.deacon@arm.com
Subject: Re: [PATCH] arm64: acpi: reenumerate topology ids
Date: Fri, 29 Jun 2018 14:48:22 +0100	[thread overview]
Message-ID: <20180629134822.GC16282@e107155-lin> (raw)
In-Reply-To: <20180629115539.w7lgjy2bmucgz7gm@kamzik.brq.redhat.com>

On Fri, Jun 29, 2018 at 01:55:39PM +0200, Andrew Jones wrote:
> On Fri, Jun 29, 2018 at 01:42:27PM +0200, Andrew Jones wrote:
> > 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.
> > """
> 
> Eh, x86 does seem to display the physical, rather than logical (linear)
> IDs in sysfs though,
> 
> arch/x86/include/asm/topology.h:#define topology_physical_package_id(cpu)       (cpu_data(cpu).phys_proc_id)
> 
> """
>   - cpuinfo_x86.phys_proc_id:
> 
>     The physical ID of the package. This information is retrieved via CPUID
>     and deduced from the APIC IDs of the cores in the package.
> """
> 
> So, hmmm...
> 
> But, I think we should either be looking for a hardware derived ID to use
> (like x86), or remap to counters. I don't believe the current scheme of
> using ACPI offsets can be better than counters, and it has consistency and
> human readability issues.
> 

UID was added for the same reason and we *have* to use it if present.
If not, OS can have it's own policy and I am fine with offset. So if
offset hurts eyes, better even the absence of UID in the PPTT. As we
don't have architectural way to derive it, we *have* to rely on platform
providing UID. If it doesn't, why should OS ? I really don't think
counter is the solution as this is user ABI, better be consistent rather
than human readable especially if platforms don't care to provide one.

--
Regards,
Sudeep

  reply	other threads:[~2018-06-29 13:48 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-28 14:51 [PATCH] arm64: acpi: reenumerate topology ids Andrew Jones
2018-06-28 16:30 ` Sudeep Holla
2018-06-28 17:12   ` Jeremy Linton
2018-06-29 10:53     ` Sudeep Holla
2018-06-29 11:42       ` Andrew Jones
2018-06-29 11:55         ` Andrew Jones
2018-06-29 13:48           ` Sudeep Holla [this message]
2018-06-29 13:38         ` Sudeep Holla
2018-06-29 16:03           ` Andrew Jones
2018-06-28 17:32   ` Andrew Jones
2018-06-29 10:29     ` Sudeep Holla
2018-06-29 11:23       ` Andrew Jones
2018-06-29 13:29         ` Sudeep Holla
2018-06-29 15:46           ` Andrew Jones
2018-06-29 15:55             ` Sudeep Holla
2018-06-29 16:48             ` Jeremy Linton
2018-06-29 17:03               ` Andrew Jones
2018-06-29 17:23                 ` Sudeep Holla
2018-06-29 18:03                   ` Andrew Jones
2018-07-02 14:58             ` Jeffrey Hugo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180629134822.GC16282@e107155-lin \
    --to=sudeep.holla@arm.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=catalin.marinas@arm.com \
    --cc=drjones@redhat.com \
    --cc=jeremy.linton@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=shunyong.yang@hxt-semitech.com \
    --cc=will.deacon@arm.com \
    --cc=yu.zheng@hxt-semitech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).