linux-efi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Heinrich Schuchardt <xypron.glpk@gmx.de>
To: Ard Biesheuvel <ardb@kernel.org>
Cc: U-Boot Mailing List <u-boot@lists.denx.de>,
	linux-efi@vger.kernel.org,
	Kristian Amlie <kristian.amlie@northern.tech>
Subject: Re: Fwd: Re: [PATCH] efi_loader: Omit memory with "no-map" when returning memory map.
Date: Tue, 31 Aug 2021 18:59:40 +0200	[thread overview]
Message-ID: <341cc376-540b-0c96-a2db-728cc276d983@gmx.de> (raw)
In-Reply-To: <5fafbb62-b93d-1f08-0e52-54b892f2ad30@northern.tech>



On 8/31/21 1:12 PM, Kristian Amlie wrote:
> On 31/08/2021 12:46, Heinrich Schuchardt wrote:
>>
>>
>> ------------------------------------------------------------------------
>> *Von:* Ard Biesheuvel <ardb@kernel.org>
>> *Gesendet:* 31. August 2021 12:33:56 MESZ
>> *An:* Heinrich Schuchardt <xypron.glpk@gmx.de>
>> *CC:* Kristian Amlie <kristian.amlie@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@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@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--;
>>          }
>>
>>
>
>

       reply	other threads:[~2021-08-31 16:59 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <CAMj1kXFAniHs=a8d1t-3KxH8uGYmeCDKm8DOAb_HiRQNeehw6g@mail.gmail.com>
     [not found] ` <FD131CA7-415E-48D5-8BBD-F236AC6D4055@gmx.de>
     [not found]   ` <5fafbb62-b93d-1f08-0e52-54b892f2ad30@northern.tech>
2021-08-31 16:59     ` Heinrich Schuchardt [this message]
2021-09-01  7:34       ` Ard Biesheuvel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=341cc376-540b-0c96-a2db-728cc276d983@gmx.de \
    --to=xypron.glpk@gmx.de \
    --cc=ardb@kernel.org \
    --cc=kristian.amlie@northern.tech \
    --cc=linux-efi@vger.kernel.org \
    --cc=u-boot@lists.denx.de \
    --subject='Re: Fwd: Re: [PATCH] efi_loader: Omit memory with "no-map" when returning memory map.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).