From: Grzegorz Jaszczyk <jaszczyk@google.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).

Co-developed-by: Bartłomiej Grzesik <bgrzesik@google.com>
Signed-off-by: Bartłomiej Grzesik <bgrzesik@google.com>
Signed-off-by: Grzegorz Jaszczyk <jaszczyk@google.com>
---
 arch/x86/kernel/devicetree.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/arch/x86/kernel/devicetree.c b/arch/x86/kernel/devicetree.c
index dd8748c45529..9a93eddfcedb 100644
--- a/arch/x86/kernel/devicetree.c
+++ b/arch/x86/kernel/devicetree.c
@@ -18,6 +18,7 @@
 #include <linux/of_pci.h>
 #include <linux/initrd.h>
 
+#include <asm/e820/api.h>
 #include <asm/irqdomain.h>
 #include <asm/hpet.h>
 #include <asm/apic.h>
@@ -289,6 +290,11 @@ static void __init x86_dtb_parse_smp_config(void)
 	dtb_apic_setup();
 }
 
+static bool __init x86_is_region_reserved(phys_addr_t base, phys_addr_t size)
+{
+	return e820__mapped_all(base, base + size, E820_TYPE_RESERVED);
+}
+
 void __init x86_flattree_get_config(void)
 {
 #ifdef CONFIG_OF_EARLY_FLATTREE
@@ -307,6 +313,9 @@ void __init x86_flattree_get_config(void)
 		}
 
 		early_init_dt_verify(dt, __pa(dt));
+
+		early_init_set_rsv_region_verifier(x86_is_region_reserved);
+		early_init_fdt_scan_reserved_mem();
 	}
 
 	unflatten_and_copy_device_tree();
-- 
2.49.0.805.g082f7c87e0-goog

On Fri, Apr 18, 2025 at 12:47:18PM +0000, Grzegorz Jaszczyk wrote:
> From: Grzegorz Jaszczyk <jaszczyk@google.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).

Rob

On Wed, Apr 23, 2025 at 4:09 PM Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Apr 18, 2025 at 12:47:18PM +0000, Grzegorz Jaszczyk wrote:
> > From: Grzegorz Jaszczyk <jaszczyk@google.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.

Best regards,
Grzegorz

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@kernel.org> wrote:
> >
> > On Fri, Apr 18, 2025 at 12:47:18PM +0000, Grzegorz Jaszczyk wrote:
> > > From: Grzegorz Jaszczyk <jaszczyk@google.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).

Rob

On Fri, Apr 25, 2025 at 9:18 PM Rob Herring <robh@kernel.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@kernel.org> wrote:
> > >
> > > On Fri, Apr 18, 2025 at 12:47:18PM +0000, Grzegorz Jaszczyk wrote:
> > > > From: Grzegorz Jaszczyk <jaszczyk@google.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