Re: [PATCH 1/6 v14] ARM: Handle a device tree in lowmem

From: Ard Biesheuvel <ardb@kernel.org>
To: Linus Walleij <linus.walleij@linaro.org>
Cc: Florian Fainelli <f.fainelli@gmail.com>,
	Arnd Bergmann <arnd@arndb.de>,
	Abbott Liu <liuwenliang@huawei.com>,
	Russell King <linux@armlinux.org.uk>,
	Mike Rapoport <rppt@linux.ibm.com>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	Linux ARM <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH 1/6 v14] ARM: Handle a device tree in lowmem
Date: Mon, 5 Oct 2020 15:36:00 +0200	[thread overview]
Message-ID: <CAMj1kXHNsnv93SzJh8PNkhz8aPOE1NTibCU6ioBRJgdzFYxofw@mail.gmail.com> (raw)
In-Reply-To: <CACRpkdbzJQ2njOVNMUYX=BjQy+625_yHeMc47BvoVe-uhmryTQ@mail.gmail.com>

On Mon, 5 Oct 2020 at 15:27, Linus Walleij <linus.walleij@linaro.org> wrote:
>
> On Mon, Oct 5, 2020 at 11:14 AM Ard Biesheuvel <ardb@kernel.org> wrote:
> > On Mon, 5 Oct 2020 at 09:14, Ard Biesheuvel <ardb@kernel.org> wrote:
>
> > > Let me see if I can code up a PoC
> >
> > I pushed a branch to
> >
> > https://git.kernel.org/pub/scm/linux/kernel/git/ardb/linux.git/log/?h=arm-dt-mapping
> >
> > that moves the DT mapping to a read-only region at the top of the
> > kernel VA space: there happened to be a 4 MB hole there (between
> > VMALLOC_END and FIXADDR_START) that we can use, even if the purpose of
> > that hole was as a guard region, as a read-only mapping still catches
> > stray writes.
>
> I will test it when I'm back at the hardware.
> I tried to do this thing as well but couldn't figure out a good
> place to map it, putting it between VMALLOC_END
> and FIXADDR_START seems like a good idea!
>
> But this is going to be a problem:
>
> + map.type = MT_ROM;
>
> Because the current code calls unflatten_device_tree() which
> will unflatten the device tree right where it is.
> So then the memory needs to be RW.
>

I don't think this is the case. Note that arm64 has been using r/o
mappings for the device tree for a long time, and it calls
unflatten_device_tree() without any problems.

> This is why in my patch I change that to
> unflatten_and_copy_device_tree() so I can treat
> it as a ROM, unflatten and copy that and then
> ditch the memory where the device tree is so the
> kernel does not need to work around that.
>
> (unflatten_and_copy_device_tree()
> will not delete the memblock around the device
> tree, so that would need to be fixed in my patch.)
>
> With your patch, if we call
> unflatten_and_copy_device_tree() we can use
> MT_ROM but then we would want to get rid of the
> remapped memory and memblock
> for the device tree after copying and unflattening
> it, but since there is no delete_mapping()
> counterpart to create_mapping() I guess that
> is going to be hard?
>
> > What I don't get is why the DT *contents* get clobbered -
> > arm_memblock_init() memblock_reserve's the DT contents, and wiping
> > reserved memblocks is something we really shouldn't be doing.
>
> The contents are fine on my system, just the two section
> mappings get wiped.
>

Ah ok.

> I hope my previous mail explains that, the code in
> prepare_page_table() simply just wipes the lowmem
> PMDs without any regard for any reserved memblocks being
> in that range.
>

In that case, mapping the DT outside of the linear region should solve
this entirely.

Note that this code boots fine for me. The only question I have is
whether the ATAGS based systems require the ability to make changes to
the data structure at runtime.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel