linux-kernel - Re: [PATCH v2 2/2] x86/of: add support for reserved memory defined by DT

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: <CAGptq8GOX0EDFPd+YdZfX76vsD0w0PyX8Y60CKwp3n7abRT1=g@mail.gmail.com>
Date: Wed, 30 Apr 2025 10:55:31 +0200
From: Grzegorz Jaszczyk <jaszczyk@...omium.org>
To: Rob Herring <robh@...nel.org>
Cc: tglx@...utronix.de, mingo@...hat.com, bp@...en8.de, 
	dave.hansen@...ux.intel.com, x86@...nel.org, hpa@...or.com, 
	saravanak@...gle.com, dmaluka@...omium.org, bgrzesik@...gle.com, 
	jaszczyk@...gle.com, ilpo.jarvinen@...ux.intel.com, usamaarif642@...il.com, 
	linux-kernel@...r.kernel.org, devicetree@...r.kernel.org, tnowicki@...gle.com, 
	mazurekm@...gle.com, vineethrp@...gle.com
Subject: Re: [PATCH v2 2/2] x86/of: add support for reserved memory defined by DT

On Fri, Apr 25, 2025 at 9:18 PM Rob Herring <robh@...nel.org> wrote:
>
> On Thu, Apr 24, 2025 at 08:06:33PM +0200, Grzegorz Jaszczyk wrote:
> > On Wed, Apr 23, 2025 at 4:09 PM Rob Herring <robh@...nel.org> wrote:
> > >
> > > On Fri, Apr 18, 2025 at 12:47:18PM +0000, Grzegorz Jaszczyk wrote:
> > > > From: Grzegorz Jaszczyk <jaszczyk@...gle.com>
> > > >
> > > > The DT reserved-memory nodes can be present in DT as described in
> > > > Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml.
> > > > Similar to other architecture, which supports DT, there is a need to
> > > > create reserved memory regions for such nodes.
> > > >
> > > > Additionally, the x86 architecture builds its memory map based on E820
> > > > description passed by bootloader and not on DT. Since x86 already has
> > > > some DT support and allows booting with both ACPI and DT at the same
> > > > time, let's register an arch specific hook which will validate if a
> > > > reserved-memory region passed by DT is valid (covered by E820 reserved
> > > > region entry).
> > > >
> > > > Without this check, the reserved memory from DT could be successfully
> > > > registered, even though such a region could conflict with e820
> > > > description e.g. it could be described as E820_RAM and could be already
> > > > used at early x86 boot stage for memblock initialization (which happens
> > > > before DT parsing).
> > >
> > > Sorry, I don't get how it conflicts. Wouldn't the E820_RAM be registered
> > > with memblock and memblock then handles the conflict (or should).
> > >
> >
> > On x86, early memblock setup is performed by e820__memblock_setup()
> > and regions which are marked as E820_RAM are added to the memblock
> > "memory" type and such regions can be later on used for memblock
> > allocation on early x86 setup. If memblock allocation is performed
> > after e820__memblock_setup and before x86_flattree_get_config,  the
> > reserved region described in DT (but described as RAM in e820) could
> > be silently used before we scan DT for reserved memory regions.
> >
> > Additionally there are more reasons why we want to make sure that e820
> > reserved regions are in sync with DT reserved memory: resource tree
> > building and setup pci gap based on e820.
> > On the x86 resource tree is built taking into account e820 entries
> > (e820__reserve_resources()) while on other arch like e.g. arm64, which
> > relies on DT, the resource tree is built taking into account
> > information from DT(request_standard_resources). Mixing both on x86
> > seems problematic and at first glance could be achieved by e.g.
> > patching e820_table via e820__range_update so other part of the early
> > x86 kernel setup such as e820__setup_pci_gap() will also not use
> > region which is described in DT as reserved-memory. But it is not
> > straight-forward (initially I've tried to go through this path) e.g.
> > it will require handling DT earlier (x86_flattree_get_config) but at
> > the same time x86_flattree_get_config relies on the memblock being set
> > up. Therefore it seems that making a requirement that the e820
> > reserved region should be in sync with DT reserved-memory on x86 is
> > reasonable.
>
> x86_flattree_get_config() is a bit odd in that the DT is mapped and
> unflattened in one shot. Usually the flat DT is mapped and scanned
> early, and then only unflattened once memblock is up. You will be better
> off moving the early mapping and scanning earlier. Then the next thing
> you want from the DT early will be there. For example, the console or
> handling for kexec (which is its own reserved regions).

But reserved memory scanning relies on memblcok being already setup
(see: early_init_fdt_scan_reserved_mem->__reserved_mem_reserve_reg->early_init_dt_reserve_memory()
which uses:
memblock_overlaps_region, memblock_is_region_reserved,
memblock_mark_nomap and memblock_reserve and therefore we can't move
scanning earlier than e820__memblock_setup(). We can move early
mapping and reserved memory scanning part(actually entire
x86_flattree_get_config) at the end of  e820__memblock_setup but there
will be still remaining issue with e820 not being with sync which will
affect mentioned earlier e820 based assumptions for e.g.
e820__setup_pci_gap and e820__reserve_resources.

I've prepared v3 which moves x86_flattree_get_config earlier in the
setup process and prepared patch, which updates e820 table
accordingly: https://lore.kernel.org/all/20250430084138.2287031-1-jaszczyk@chromium.org/.
Could you please take a look?

Best regards,
Grzegorz