On Thu, Apr 14, 2016 at 01:10:35PM +0200, Ard Biesheuvel wrote: > On 14 April 2016 at 13:02, Steve Capper <steve.capper@xxxxxxx> wrote: > > On Fri, Apr 08, 2016 at 03:50:23PM -0700, David Daney wrote: > >> From: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> > >> > >> There are two problems with the UEFI stub DT memory node removal > >> routine: > >> - it deletes nodes as it traverses the tree, which happens to work > >> but is not supported, as deletion invalidates the node iterator; > >> - deleting memory nodes entirely may discard annotations in the form > >> of additional properties on the nodes. > >> > >> Since the discovery of DT memory nodes occurs strictly before the > >> UEFI init sequence, we can simply clear the memblock memory table > >> before parsing the UEFI memory map. This way, it is no longer > >> necessary to remove the nodes, so we can remove that logic from the > >> stub as well. > >> > >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> > >> Signed-off-by: David Daney <david.daney@xxxxxxxxxx> > >> --- > >> drivers/firmware/efi/arm-init.c | 8 ++++++++ > >> drivers/firmware/efi/libstub/fdt.c | 24 +----------------------- > >> 2 files changed, 9 insertions(+), 23 deletions(-) > >> > >> diff --git a/drivers/firmware/efi/arm-init.c b/drivers/firmware/efi/arm-init.c > >> index aa1f743..5d6945b 100644 > >> --- a/drivers/firmware/efi/arm-init.c > >> +++ b/drivers/firmware/efi/arm-init.c > >> @@ -143,6 +143,14 @@ static __init void reserve_regions(void) > >> if (efi_enabled(EFI_DBG)) > >> pr_info("Processing EFI memory map:\n"); > >> > >> + /* > >> + * Discard memblocks discovered so far: if there are any at this > >> + * point, they originate from memory nodes in the DT, and UEFI > >> + * uses its own memory map instead. > >> + */ > >> + memblock_dump_all(); > >> + memblock_remove(0, ULLONG_MAX); > >> + > > > > Does this change need to be applied to any other architectures given > > that deletion code has been removed from libstub below? > > > > The 'generic' libstub code below is only used by ARM, so we're safe > here in that regard. Thanks Ard, In that case, FWIW: Acked-by: Steve Capper <steve.capper@xxxxxxx> Cheers, -- Steve > > > >> for_each_efi_memory_desc(&memmap, md) { > >> paddr = md->phys_addr; > >> npages = md->num_pages; > >> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c > >> index 6dba78a..e58abfa 100644 > >> --- a/drivers/firmware/efi/libstub/fdt.c > >> +++ b/drivers/firmware/efi/libstub/fdt.c > >> @@ -24,7 +24,7 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt, > >> unsigned long map_size, unsigned long desc_size, > >> u32 desc_ver) > >> { > >> - int node, prev, num_rsv; > >> + int node, num_rsv; > >> int status; > >> u32 fdt_val32; > >> u64 fdt_val64; > >> @@ -54,28 +54,6 @@ efi_status_t update_fdt(efi_system_table_t *sys_table, void *orig_fdt, > >> goto fdt_set_fail; > >> > >> /* > >> - * Delete any memory nodes present. We must delete nodes which > >> - * early_init_dt_scan_memory may try to use. > >> - */ > >> - prev = 0; > >> - for (;;) { > >> - const char *type; > >> - int len; > >> - > >> - node = fdt_next_node(fdt, prev, NULL); > >> - if (node < 0) > >> - break; > >> - > >> - type = fdt_getprop(fdt, node, "device_type", &len); > >> - if (type && strncmp(type, "memory", len) == 0) { > >> - fdt_del_node(fdt, node); > >> - continue; > >> - } > >> - > >> - prev = node; > >> - } > >> - > >> - /* > >> * Delete all memory reserve map entries. When booting via UEFI, > >> * kernel will use the UEFI memory map to find reserved regions. > >> */ > >> -- > >> 1.8.3.1 > >> > >> > >> _______________________________________________ > >> linux-arm-kernel mailing list > >> linux-arm-kernel@xxxxxxxxxxxxxxxxxxx > >> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > >> > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html