linux-kernel - Re: [PATCH] of: skip disabled CPU nodes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 09 Sep 2020 10:57:54 +0200
From:   Matthias Schiffer <matthias.schiffer@...tq-group.com>
To:     Rob Herring <robh+dt@...nel.org>
Cc:     devicetree@...r.kernel.org,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Frank Rowand <frowand.list@...il.com>
Subject: Re: [PATCH] of: skip disabled CPU nodes

On Thu, 2020-08-27 at 09:10 +0200, Matthias Schiffer wrote:
> On Wed, 2020-08-26 at 13:26 -0600, Rob Herring wrote:
> > On Wed, Aug 26, 2020 at 8:47 AM Frank Rowand <
> > frowand.list@...il.com>
> > wrote:
> > > 
> > > Hi Rob,
> > > 
> > > On 2020-08-26 08:54, Matthias Schiffer wrote:
[snip]
> > > > 
> > > > Hmm, I see. This difference in behaviour is quite unfortunate,
> > > > as
> > > > I'm
> > > > currently looking for a way to *really* disable a CPU core.
> > > > 
> > > > In arch/arm64/boot/dts/freescale/imx8mn.dtsi (and other
> > > > variants
> > > > of the
> > > > i.MX8M), there are 4 CPU nodes for the full-featured quad-core
> > > > version.
> > > > The reduced single- and dual-core versions are currently
> > > > handled
> > > > in
> > > > NXP's U-Boot fork by deleting the additional nodes.
> > > > 
> > > > Not doing so causes the kernel to hang for a while when trying
> > > > to
> > > > online the non-existent cores during boot (at least in linux-
> > > > imx
> > > > 5.4 -
> > > > I have not checked a more recent mainline kernel yet), but the
> > > > deletion
> > > > is non-trivial to do without leaving dangling phandle
> > > > references.
> > > 
> > > Any thoughts on implementing another universal property that
> > > means
> > > something like "the hardware described by this node does not
> > > exist
> > > or is so broken that you better not use it".
> > 
> > There's a couple of options:
> > 
> > The DT spec defines 'fail' value for status. We could use that
> > instead
> > of 'disabled'.
> > 
> > The spec behavior with cpu 'disabled' is only on PPC AFAIK. On
> > arm/arm64 (probably riscv now too) we've never followed it where we
> > online 'disabled' CPUs. So we could just make the check conditional
> > on
> > !IS_ENABLED(CONFIG_PPC). This would need some spec update.
> 
> On ARM(64), the "disabled" status on CPUs doesn't have any effect. I
> assume this changed with the mentioned commit c961cb3be906 "of: Fix
> cpu
> node iterator to not ignore disabled cpu nodes", as reverting it
> gives
> me the desired behaviour of considering the disabled CPUs non-
> existent.
> 
> So it seems that we already changed the interpretation in a non-
> compatible way once (back in v4.20), and somehow PPC has yet another
> different behaviour?
> 
> How do we get out of this mess? Is going back to the v4.19 logic for
> non-PPC platforms an acceptable regression fix, or would this be
> considered another breaking change?
> 

Any new insights on this? I'll gladly provide patches or proposals for
a spec update if we can decide where we want to go with this.

Kind regards,
Matthias