Re: [PATCH v2] of/fdt: Don't worry about non-memory region overlap for no-map

From: Stephen Boyd <swboyd@chromium.org>
To: Mike Rapoport <rppt@kernel.org>
Cc: Rob Herring <robh+dt@kernel.org>,
	linux-kernel@vger.kernel.org, devicetree@vger.kernel.org,
	Douglas Anderson <dianders@chromium.org>,
	Nicolas Boichat <drinkcat@chromium.org>,
	Quentin Perret <qperret@google.com>,
	Jan Kiszka <jan.kiszka@siemens.com>
Subject: Re: [PATCH v2] of/fdt: Don't worry about non-memory region overlap for no-map
Date: Wed, 15 Dec 2021 11:28:14 -0800	[thread overview]
Message-ID: <CAE-0n53sMjF7-STAyn=snT5NSAMYSXhkBXJ4ay_fPiB_DbV_gg@mail.gmail.com> (raw)
In-Reply-To: <Ybm6KQiS7B28QOSW@kernel.org>

Quoting Mike Rapoport (2021-12-15 01:49:29)
> Hi,
>
> On Tue, Dec 14, 2021 at 11:20:11PM -0800, Stephen Boyd wrote:
> > In commit 8a5a75e5e9e5 ("of/fdt: Make sure no-map does not remove
> > already reserved regions") we returned -EBUSY when trying to mark
> > regions as no-map when they're in the reserved memory node. This if
> > condition will still trigger though if the DT has a /memreserve/ that
> > completely subsumes the no-map memory carveouts in the reserved memory
> > node. Let's only consider this to be a problem if we're trying to mark a
> > region as no-map and it is actually memory. If it isn't memory,
> > presumably it was removed from the memory map via /memreserve/ and thus
> > can't be mapped anyway.
>
> I have no objections for this patch, but I afraid that this is a never
> ending story of reservation vs nomap ordering and this won't be the last
> fix in the area.

Ugh ok

>
> I was toying with the idea to use flags in memblock.reserved to have
> clearer view of how the reserved memory was used and then we won't need
> to guess firmware intentions.
> Thoughts?

My understanding of the commit being fixed was that it tried to detect
bad DT where two reserved regions overlapped and different reserved
memory regions stomped on each other. It certainly seems like that could
be improved by recording what reserved memory region it belongs to, but
within memblock I don't know if it cares. I thought memblock just cared
to find out what is memory and what is supposed to be mapped into the
page tables.

>
> > This silences a warning seen at boot on sc7180-trogdor.dtsi boards that
> > have /memreserve/ populated by the bootloader where those reserved
> > regions overlap with the reserved-memory carveouts that we have in DT
> > for other purposes like communicating with remote processors.
>
> Do you mind adding the relevant pats of the device tree to the changelog?

Sure. Let me add the reserved memory snippet.