linux-kernel - Re: [PATCH v2] of: WARN on deprecated #address-cells/#size-cells handling

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] [day] [month] [year] [list]

Message-ID: <CAL_JsqL5FHPNQWGdBEz9UpD7cq3We-czPV8OmwD=0w5Eu10=kA@mail.gmail.com>
Date: Mon, 2 Dec 2024 16:55:58 -0600
From: Rob Herring <robh@...nel.org>
To: Segher Boessenkool <segher@...nel.crashing.org>
Cc: Michael Ellerman <mpe@...erman.id.au>, Saravana Kannan <saravanak@...gle.com>, 
	linuxppc-dev@...ts.ozlabs.org, Conor Dooley <conor@...nel.org>, 
	devicetree@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] of: WARN on deprecated #address-cells/#size-cells handling

On Mon, Dec 2, 2024 at 4:09 PM Segher Boessenkool
<segher@...nel.crashing.org> wrote:
>
> On Mon, Dec 02, 2024 at 08:18:22AM -0600, Rob Herring wrote:
> > On Wed, Nov 27, 2024 at 3:47 PM Segher Boessenkool
> > <segher@...nel.crashing.org> wrote:
> > > On Tue, Nov 26, 2024 at 02:36:32PM +1100, Michael Ellerman wrote:
> > > > Michael Ellerman <mpe@...erman.id.au> writes:
> > > > > "Rob Herring (Arm)" <robh@...nel.org> writes:
> > > > >> While OpenFirmware originally allowed walking parent nodes and default
> > > > >> root values for #address-cells and #size-cells, FDT has long required
> > > > >> explicit values. It's been a warning in dtc for the root node since the
> > > > >> beginning (2005) and for any parent node since 2007. Of course, not all
> > > > >> FDT uses dtc, but that should be the majority by far. The various
> > > > >> extracted OF devicetrees I have dating back to the 1990s (various
> > > > >> PowerMac, OLPC, PASemi Nemo) all have explicit root node properties.
> > > > >
> > > > > I have various old device trees that have been given to me over the
> > > > > years, and as far as I can tell they all have these properties (some of
> > > > > them are partial trees so it's hard to be 100% sure).
> > > > >
> > > > > So LGTM.
> > > >
> > > > Turns out I was wrong.
> > > >
> > > > The warning about #size-cells hits on some powermacs, possible fixup
> > > > patch here:
> > > >
> > > >   https://lore.kernel.org/linuxppc-dev/20241126025710.591683-1-mpe@ellerman.id.au/
> > >
> > > The Open Firmware specification is extremely clear that a "missing"
> > > "#size-cells" property means this bus has the default value of 1.
> >
> > And the default for #address-cells is 2, but yet every architecture
> > except Sparc has that wrong.
>
> ?
>
> Almost all architectures (that run Linux) use 64-bit addressing, both
> 32-bit and 64-bit architectures.

I'm just telling you what Linux uses for defaults for at least 20 years.

> > If I have a node without #size-cells, is the default of 1 used or do
> > we check parent nodes? My read of the spec would be the former, but
> > the kernel does the latter.
>
> The former is correct.  The latter makes no sense at all!  The whole
> point of the "bus" abstraction is that you get a new addressing domain
> there.

I agree, but that's what the kernel does (again, for 20+ years).
Walking the parents is really what I want to get rid of here. My
choices were drop that behavior and see who I break, or add a warning
and see who notices. I went the nicer route of a warning.

> Yes, these days you numerically find it most often with PCI sub-domains,
> but those are boring.  In most cases you *do* have different adressing
> on your child busses, and even if the addressing is the same, addresses
> on the child bus are not normally a subset of those on the parent bus.
>
> > > https://www.openfirmware.info/data/docs/of1275.pdf (page 186).
> > >
> > > DTC or FDT might want to do things differently, but expecting decades
> > > older stuff to conform to its ill-conceived unnecessarily super wordy
> > > stuff is, well, not a plan that is likely to work very well :-)
> >
> > That is not the intention. The intention is to identify what doesn't
> > conform and exclude those systems from this check (or apply a fixup if
> > that works).
>
> So *always* use the OF definition, at least on OF systems?  Where
> everything is meant to conform, but conform to OF, not conform to this
> "OF-like-but-very-different-in-crucial-spots" thing :-)

I'm pretty sure there are OF systems that don't conform, so it is not
that simple. There's this comment in of_irq_parse_raw() for example:

        /* Look for this #address-cells. We have to implement the old linux
         * trick of looking for the parent here as some device-trees rely on it
         */

Maybe that's from some system long dropped and we don't need it
anymore. I have no idea. That's what I'm trying to find out with this
patch.

We also don't really have a way to distinguish OF from FDT (where we'd
need to). It is somewhat just by arch, but PPC always passes an FDT to
the kernel for both FDT and OF systems.

Rob