Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
MIME-Version: 1.0
References: <20220517101410.3493781-1-andre.przywara@arm.com>
 <20220517153444.GA1057027-robh@kernel.org> <CAFEAcA8sE8Rj0GmF71ox4BdDr0UcaS4QwiLUVUUFH5oj+hDhfA@mail.gmail.com>
 <CAL_JsqLRvEn2E7cpTzQJRCJ=aeLjUtKhDimWat=nPtm3QP+cfA@mail.gmail.com>
 <CAFEAcA_DRoJmnFdyEEcSvCxtPYignZFqQFnHyWkcpyijCBSrCg@mail.gmail.com>
 <20220518165421.GF3302100-robh@kernel.org> <CAFEAcA80B2aGdaxK2pm7AK84KK_UqwD-KCMKtK6b8fF41MeKRg@mail.gmail.com>
In-Reply-To: <CAFEAcA80B2aGdaxK2pm7AK84KK_UqwD-KCMKtK6b8fF41MeKRg@mail.gmail.com>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Wed, 18 May 2022 22:46:42 +0200
Message-ID: <CAMj1kXFgyg7CFNoPomeucN4BCCo-PMO8w8smjmxFWKRvOY0ePg@mail.gmail.com>
Subject: Re: [PATCH] of/fdt: Ignore disabled memory nodes
To:     Peter Maydell <peter.maydell@linaro.org>
Cc:     Rob Herring <robh@kernel.org>,
        Andre Przywara <andre.przywara@arm.com>,
        Frank Rowand <frowand.list@gmail.com>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@vger.kernel.org>,
        linux-arm-kernel <linux-arm-kernel@lists.infradead.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        Ross Burton <ross.burton@arm.com>,
        Catalin Marinas <catalin.marinas@arm.com>,
        Will Deacon <will@kernel.org>,
        Russell King <linux@armlinux.org.uk>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, 18 May 2022 at 19:54, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Wed, 18 May 2022 at 17:54, Rob Herring <robh@kernel.org> wrote:
> >
> > On Tue, May 17, 2022 at 08:19:47PM +0100, Peter Maydell wrote:
> > > We generate the DTB with libfdt, so source-only information
> > > isn't something we can put in, I think. (The quoted DT fragment
> > > in this patch's commit message is the result of decompiling
> > > the runtime generated DT binary blob with dtc.)
> >
> > Given the runtime aspect with overlays, it's conceivable that libfdt
> > could support setting labels some day and then dts output maintaining
> > them.
> >
> > We could also consider a standard node name such as 'secure-memory'.
> > It's a whole can of worms though on how secure vs. non-secure memory
> > (and other things) are represented.
>
> Mmm. We put in the very basic parts years ago in
> Documentation/devicetree/bindings/arm/secure.txt
> which is (and has remained) generally sufficient for the QEMU->Trusted
> Firmware-> maybe uboot->Linux stack, which is pretty much the only use
> case I think. (My intention when we wrote that up was that memory
> that's S-only would be indicated the same way as S-only devices,
> with the secure-status and status properties.)
>
> > > Are we just stuck with what we have for historical reasons ?
> >
> > Yes. If we were designing this, we'd probably have 'compatible =
> > "memory"'. We're likely just stuck with things how they are. Mostly node
> > names haven't been an ABI and we're just trying to be consistent in
> > naming and use of unit-addresses.
>
> So, do you think it's worthwhile/a good idea for me to rename
> the DT node that QEMU is currently calling "secmem" to be
> "memory" ? My default is "leave it as it is", for economy of
> effort reasons :-) -- but it's an easy enough change to make.
> Though EDK2's dtb reading code just looks for the first
> "memory" node and assumes it's the big one, so changing the node
> name would make us reliant on the order of the two nodes in the
> DTB unless we fixed EDK2 (which we should probably do anyway, tbh).
>

Agreed. The referenced code is only one of two implementations (one
for the firmware version that executes in place from NOR flash, and
one for the version that executes from DRAM), which don't follow the
same strategy (the other one enumerates all device_type="memory" nodes
in the tree, and picks the one with the lowest starting address, but
also ignores status properties or node names). The PrePi version
linked here should simply choose the node that covers its own image in
memory. The XIP version uses a stack in DRAM, so it could do something
similar.