On 12 May 2015 at 16:31, Leif Lindholm <leif.lindholm@xxxxxxxxxx> wrote:
> On Mon, May 11, 2015 at 08:41:58AM +0200, Ard Biesheuvel wrote:
>> 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.
>>
>> Now that the UEFI initialization has moved to an earlier stage, we can
>> actually just ignore any memblocks that are installed after we have
>> processed 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.
>
> So ... I don't have an issue with this patch as such, but keeping the
> memory nodes around for NUMA topology when the UEFI memory map is
> the one being used for memblock leads to a spooky redefinition of what
> that memory node _is_; it would now describe a region which may only
> partially be populated with RAM.
>
How does that not apply to ACPI? It is still the responsibility of the
firmware to ensure that all information it supplies is consistent with
itself.
But I agree that we should carefully consider this interaction before
accepting any NUMA changes to the memnode bindings and implementation.
--
Ard.
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
>> ---
>> arch/arm64/mm/init.c | 7 +++++++
>> drivers/firmware/efi/libstub/fdt.c | 24 +-----------------------
>> 2 files changed, 8 insertions(+), 23 deletions(-)
>>
>> diff --git a/arch/arm64/mm/init.c b/arch/arm64/mm/init.c
>> index 64480b65ef17..95c239f43384 100644
>> --- a/arch/arm64/mm/init.c
>> +++ b/arch/arm64/mm/init.c
>> @@ -386,6 +386,13 @@ __setup("keepinitrd", keepinitrd_setup);
>>
>> void __init early_init_dt_add_memory_arch(u64 base, u64 size)
>> {
>> + /*
>> + * EFI_MEMMAP will be set /after/ UEFI has installed all memory based
>> + * on the content of the UEFI memory map.
>> + */
>> + if (efi_enabled(EFI_MEMMAP))
>> + return;
>> +
>> if (!PAGE_ALIGNED(base)) {
>> if (size < PAGE_SIZE - (base & ~PAGE_MASK)) {
>> pr_warn("Ignoring memory block 0x%llx - 0x%llx\n",
>> diff --git a/drivers/firmware/efi/libstub/fdt.c b/drivers/firmware/efi/libstub/fdt.c
>> index ef5d764e2a27..343e7992bd8f 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.9.1
>>
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
