Message-ID: <Ybtsm084H0g+cIOd@kernel.org>
Date:   Thu, 16 Dec 2021 18:43:07 +0200
From:   Mike Rapoport <rppt@...nel.org>
To:     Stephen Boyd <swboyd@...omium.org>
Cc:     Rob Herring <robh+dt@...nel.org>, linux-kernel@...r.kernel.org,
        devicetree@...r.kernel.org,
        Douglas Anderson <dianders@...omium.org>,
        Nicolas Boichat <drinkcat@...omium.org>,
        Quentin Perret <qperret@...gle.com>,
        Jan Kiszka <jan.kiszka@...mens.com>
Subject: Re: [PATCH v2] of/fdt: Don't worry about non-memory region overlap
 for no-map

On Wed, Dec 15, 2021 at 11:28:14AM -0800, Stephen Boyd wrote:
> 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.

Except the creation of page tables, data in memblock is used to populate
the memory map and the free lists, so it's important to know where there is
memory, where there are holes and what memory is in use.

For now, any used memory will be listed in memblock.reserved, no matter if
it's used by firwmare, marked reserved in DT, occupied by the kernel code
or allocated early during boot before "real" mm is setup.

If we track the types of the memory reservations in memblock.reserved we'll
know which regions are /memreserve/, which are nomap and which are used by
kernel itself and so we'll have more deterministic view on what can be
allowed and what not. E.g it's ok to set nomap for a region that is already
reserved by firmware, but we cannot set nomap on memory used by the kernel.

For now this is a theory, I haven't even got to experiment with this.

> > > 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.

-- 
Sincerely yours,
Mike.

Powered by blists - more mailing lists