On 31/08/2021 12:46, Heinrich Schuchardt wrote: > > > ------------------------------------------------------------------------ > *Von:* Ard Biesheuvel <a...@kernel.org> > *Gesendet:* 31. August 2021 12:33:56 MESZ > *An:* Heinrich Schuchardt <xypron.g...@gmx.de> > *CC:* Kristian Amlie <kristian.am...@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.g...@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.am...@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--; > } > >
OpenPGP_signature
Description: OpenPGP digital signature