Fwd: Re: [PATCH] efi_loader: Omit memory with "no-map" when returning memory map.

[Kristian Amlie]

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.

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--;
>         }
> 
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20210831/240eea8f/attachment.sig>