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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 8/31/21 1:12 PM, Kristian Amlie wrote:
On 31/08/2021 12:46, Heinrich Schuchardt wrote:


------------------------------------------------------------------------
*Von:* Ard Biesheuvel <ardb@xxxxxxxxxx>
*Gesendet:* 31. August 2021 12:33:56 MESZ
*An:* Heinrich Schuchardt <xypron.glpk@xxxxxx>
*CC:* Kristian Amlie <kristian.amlie@xxxxxxxxxxxxx>
*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@xxxxxx> 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@xxxxxxxxxxxxx>
         ------------------------------------------------------------------------
         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--;
         }








[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux