Fwd: Re: [PATCH] efi_loader: Omit memory with "no-map" when returning memory map.
Heinrich Schuchardt
xypron.glpk at gmx.de
Tue Aug 31 18:59:40 CEST 2021
On 8/31/21 1:12 PM, Kristian Amlie wrote:
> On 31/08/2021 12:46, Heinrich Schuchardt wrote:
>>
>>
>> ------------------------------------------------------------------------
>> *Von:* Ard Biesheuvel <ardb at kernel.org>
>> *Gesendet:* 31. August 2021 12:33:56 MESZ
>> *An:* Heinrich Schuchardt <xypron.glpk at gmx.de>
>> *CC:* Kristian Amlie <kristian.amlie at northern.tech>
>> *Betreff:* Re: [PATCH] efi_loader: Omit memory with "no-map" when
>> returning memory map.
>>
>> On Tue, 31 Aug 2021 at 08:41, Heinrich Schuchardt <xypron.glpk at gmx.de> wrote:
>>
>>
>> On 8/27/21 9:55 AM, Kristian Amlie wrote:
>>
>> You can use scripts/get_maintainer.pl to find the right addressees for
>> your patches.
>>
>> efi_reserve_memory() states that memory marked with "no-map"
>> shall not
>> be accessed by the UEFI payload. Make sure efi_get_memory_map()
>> honors
>> this.
>>
>>
>> Accessing memory and describing memory are two different things.
>> Describing reserved memory in the memory map is important, because it
>> helps us distinguish it from MMIO regions.
>
> Ok, my mistake, I thought the kernel would deduce this separately
> through the DTB.
>
>>
>> This helps the case when booting vexpress_ca9x4 under QEMU. Because
>> the kernel wants to use an address in the lowest 128MiB of the
>> range,
>> but this range is reserved with "no-map", the kernel complains
>> that it
>> can not allocate the low memory it needs. In reality the actual
>> usable
>> memory starts much higher, which is reflected correctly in the
>> memory
>> map after this fix.
>>
>>
>>
>> This is a u-boot patch right? (I cannot tell from the context, as
>> there are no mailing lists on cc)
>>
>> It is u-boot's job to describe all the memory, no matter how it is
>> used. Even if the kernel itself may not use it as system memory, there
>> are cases where kernel infrastructure is used to map these regions:
>> for instance, the ACPI core may need to map a SystemMemory OpRegion,
>> and we need the EFI memory map to tell us whether to use memory or I/O
>> semantics.
>>
>> As for the 128 MB issue: can you reproduce this with a recent kernel?
>> We made some changes recently to the EFI stub as well as the
>> decompressor code to prevent the decompressor code from relocating
>> itself to the base of DRAM, and to allow the decompressed kernel to
>> reside at any 2 MB aligned offset in memory.
This would allow putting the kernel at the very top of memory. But
consider this function that misbehaves:
arch/arm/include/asm/efi.h:
76 /* on ARM, the initrd should be loaded in a lowmem region */
77 static inline unsigned long efi_get_max_initrd_addr(unsigned long
image_addr)
78 {
79 return round_down(image_addr, SZ_4M) + SZ_512M;
80 }
0x1F000000 = efi_get_max_initrd_addr(0xff000000);
And below 0x1f000000 there may be no RAM at all.
Best regards
Heinrich
>
> I'll try this and get back to you!
>
> --
> Kristian
>
>>
>>
>> The 'no-map' requirement needs to be fulfilled by the kernel.
>>
>> The GetMemoryMap() boot time service must comply to the UEFI
>> specification.
>>
>> The Devicetree Specification has this clarification:
>>
>> "Reserved regions with the no-map property must be listed in the memory
>> map with type EfiReservedMemoryType. All other reserved regions must be
>> listed with type EfiBootServicesData."
>> https://devicetree-specification.readthedocs.io/en/latest/chapter3-devicenodes.html
>> <https://devicetree-specification.readthedocs.io/en/latest/chapter3-devicenodes.html>
>>
>> Should the kernel calculate its internal 128 MiB requirement incorrectly
>> this needs be fixed in the kernel EFI stub. Does the problem exist with
>> kernel 5.14?
>>
>> I wonder if the 128 MiB requirement of the kernel can be lifted for
>> 32bit ARM as we already did for RISC-V. Cf.
>>
>>
>> As mentioned above, this should already be fixed, in v5.11 or later
>>
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.14&id=c79e89ecaa246c880292ba68cbe08c9c30db77e3
>> <https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v5.14&id=c79e89ecaa246c880292ba68cbe08c9c30db77e3>
>>
>> Cc Ard, maintainer of the kernel EFI stub.
>>
>> Best regards
>>
>> Heinrich
>>
>>
>> Signed-off-by: Kristian Amlie <kristian.amlie at northern.tech>
>> ------------------------------------------------------------------------
>> lib/efi_loader/efi_memory.c | 14 +++++++++++++-
>> 1 file changed, 13 insertions(+), 1 deletion(-)
>>
>> diff --git a/lib/efi_loader/efi_memory.c
>> b/lib/efi_loader/efi_memory.c
>> index f4acbee4f9..7f8543143a 100644
>> --- a/lib/efi_loader/efi_memory.c
>> +++ b/lib/efi_loader/efi_memory.c
>> @@ -646,8 +646,16 @@ efi_status_t efi_get_memory_map(efi_uintn_t
>> *memory_map_size,
>>
>> provided_map_size = *memory_map_size;
>>
>> - list_for_each(lhandle, &efi_mem)
>> + list_for_each(lhandle, &efi_mem) {
>> + struct efi_mem_list *lmem;
>> +
>> + lmem = list_entry(lhandle, struct efi_mem_list, link);
>> +
>> + if (lmem->desc.type == EFI_RESERVED_MEMORY_TYPE)
>> + continue;
>> +
>> map_entries++;
>> + }
>>
>> map_size = map_entries * sizeof(struct efi_mem_desc);
>>
>> @@ -672,6 +680,10 @@ efi_status_t efi_get_memory_map(efi_uintn_t
>> *memory_map_size,
>> struct efi_mem_list *lmem;
>>
>> lmem = list_entry(lhandle, struct efi_mem_list, link);
>> +
>> + if (lmem->desc.type == EFI_RESERVED_MEMORY_TYPE)
>> + continue;
>> +
>> *memory_map = lmem->desc;
>> memory_map--;
>> }
>>
>>
>
>
More information about the U-Boot
mailing list