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

[Kristian Amlie]

On 02/09/2021 10:47, Ard Biesheuvel wrote:
> On Thu, 2 Sept 2021 at 10:43, Kristian Amlie
>  wrote:
>>
>> On 31/08/2021 13:12, Kristian Amlie wrote:
>>> On 31/08/2021 12:46, Heinrich Schuchardt wrote:


 
 *Von:* Ard Biesheuvel 
 *Gesendet:* 31. August 2021 12:33:56 MESZ
 *An:* Heinrich Schuchardt 
 *CC:* Kristian Amlie 
 *Betreff:* Re: [PATCH] efi_loader: Omit memory with "no-map" when
 returning memory map.

 On Tue, 31 Aug 2021 at 08:41, Heinrich Schuchardt  
 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!
>>
>> The result have been a bit inconclusive. I *think* it works, but the
>> system later crashes because init is killed. Which may be a problem with
>> that kernel in general, I don't know. I would require more time to
>> figure out exactly what's causing it. The early boot and all the memory
>> initialization seems to work just fine though.
>>
>> I have pasted the log below if you want to look at it, but to me the
>> error looks unrelated.
>>
> 
> Agreed. systemd-init crashes for some reason, but this is highly
> unlikely to be caused by the EFI memory map containing reserved memory
> regions.

Thanks for checking! Then from my perspective you can consider this
issue resolved, and drop the patch.

-- 
Kristian



OpenPGP_signature
Description: OpenPGP digital signature

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

[Ard Biesheuvel]

On Thu, 2 Sept 2021 at 10:43, Kristian Amlie
 wrote:
>
> On 31/08/2021 13:12, Kristian Amlie wrote:
> > On 31/08/2021 12:46, Heinrich Schuchardt wrote:
> >>
> >>
> >> 
> >> *Von:* Ard Biesheuvel 
> >> *Gesendet:* 31. August 2021 12:33:56 MESZ
> >> *An:* Heinrich Schuchardt 
> >> *CC:* Kristian Amlie 
> >> *Betreff:* Re: [PATCH] efi_loader: Omit memory with "no-map" when
> >> returning memory map.
> >>
> >> On Tue, 31 Aug 2021 at 08:41, Heinrich Schuchardt  
> >> 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!
>
> The result have been a bit inconclusive. I *think* it works, but the
> system later crashes because init is killed. Which may be a problem with
> that kernel in general, I don't know. I would require more time to
> figure out exactly what's causing it. The early boot and all the memory
> initialization seems to work just fine though.
>
> I have pasted the log below if you want to look at it, but to me the
> error looks unrelated.
>

Agreed. systemd-init crashes for some reason, but this is highly
unlikely to be caused by the EFI memory map containing reserved memory
regions.

-- 
Ard.

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

[Kristian Amlie]

On 31/08/2021 13:12, Kristian Amlie wrote:
> On 31/08/2021 12:46, Heinrich Schuchardt wrote:
>>
>>
>> 
>> *Von:* Ard Biesheuvel 
>> *Gesendet:* 31. August 2021 12:33:56 MESZ
>> *An:* Heinrich Schuchardt 
>> *CC:* Kristian Amlie 
>> *Betreff:* Re: [PATCH] efi_loader: Omit memory with "no-map" when
>> returning memory map.
>>
>> On Tue, 31 Aug 2021 at 08:41, Heinrich Schuchardt  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!

The result have been a bit inconclusive. I *think* it works, but the
system later crashes because init is killed. Which may be a problem with
that kernel in general, I don't know. I would require more time to
figure out exactly what's causing it. The early boot and all the memory
initialization seems to work just fine though.

I have pasted the log below if you want to look at it, but to me the
error looks unrelated.

--- BOOT LOG ---

U-Boot 2021.07 (Jul 05 2021 - 15:11:28 +)

DRAM:  256 MiB
WARNING: Caches not enabled
Flash: 64 MiB
MMC:   sysctl@1000 - probe failed: -19
aaci@4000 - probe failed: -19
uart@9000 - probe failed: -19
uart@a000 - probe failed: -19
uart@b000 - probe failed: -19
uart@c000 - probe failed: -19
timer@11000 - probe failed: -19
timer@12000 - probe failed: -19
rtc@17000 - probe failed: -19
clcd@1f000 - probe failed: -19
clcd@1002 - probe failed: -19
memory-controller@100e - probe failed: -19
memory-controller@100e1000 - probe failed: -19
watchdog@100e5000 - probe failed: -19

Loading Environment from Flash... *** Warning - bad CRC, using default
environment

In:serial
Out:   serial
Err:   serial
Net:   eth0: ethernet@3,0200
Hit any key to stop autoboot:  0
no mmc device at slot 1
switch to partitions #0, OK
mmc0 is current device
Scanning mmc 0:1...
14163 bytes read in 16 ms (864.3 KiB/s)
Scanning disk sys...@1000.blk...
Disk sys...@1000.blk not ready
Scanning disk a...@4000.blk...
Disk a...@4000.blk not ready
Scanning disk m...@5000.blk...
Scanning disk u...@9000.blk...
Disk u...@9000.blk not ready
Scanning disk u...@a000.blk...
Disk u...@a000.blk not ready
Scanning disk u...@b000.blk...
Disk u...@b000.blk not ready
Scanning disk u...@c000.blk...
Disk u...@c000.blk not ready
Scanning disk ti...@11000.blk...
Disk ti...@11000.blk not ready
Scanning disk ti...@12000.blk...
Disk ti...@12000.blk not ready
Scanning disk r...@17000.blk...
Disk r...@17000.blk not ready
Scanning disk c...@1f000.blk...
Disk c...@1f000.blk not ready
Scanning disk c...@1002.blk...
Disk c...@1002.blk not ready
Scanning disk memory-control...@100e.bl...
Disk memory-control...@100e.bl not ready
Scanning disk memory-control...@100e1000.bl...
Disk memory-control...@100e1000.bl not ready
Scanning disk watch...@100e5000.blk...
Disk watch...@100e5000.blk not ready
Found 5 disks
** Unable to read file ubootefi.var **
Failed to load EFI variables
BootOrder not defined
EFI boot manager: Cannot load any image
Found EFI removable media binary efi/boot/bootarm.efi
536576 bytes read 

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

[Ard Biesheuvel]

On Tue, 31 Aug 2021 at 18:59, Heinrich Schuchardt  wrote:
>
>
>
> On 8/31/21 1:12 PM, Kristian Amlie wrote:
> > On 31/08/2021 12:46, Heinrich Schuchardt wrote:
> >>
> >>
> >> 
> >> *Von:* Ard Biesheuvel 
> >> *Gesendet:* 31. August 2021 12:33:56 MESZ
> >> *An:* Heinrich Schuchardt 
> >> *CC:* Kristian Amlie 
> >> *Betreff:* Re: [PATCH] efi_loader: Omit memory with "no-map" when
> >> returning memory map.
> >>
> >> On Tue, 31 Aug 2021 at 08:41, Heinrich Schuchardt  
> >> 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.

No, not at all. The way Linux/ARM defines its linear map is tied to
the placement of the kernel image, and any memory below it cannot be
used by the OS. IOW, placing the kernel at the very top of memory
would result in zero MB of lowmem being available, and therefore no
successful boot.

Formerly, the decompressor would simply round down the decompressor's
load address to 128 MB, and use the resulting value as the load
address of the decompressed kernel. This meant that
a) systems where the DRAM banks are not on a 128 MB boundary were
difficult to support
b) having reserved regions at the start of memory was problematic,
because the decompressor did not look at the device tree at all (this
is why we have all these TEXT_OFFSET hacks in the ARM kernel)
c) the EFI stub was forced to relocate itself into the first 128 MB of
memory, or the decompressor would misidentify the start of DRAM.

This has been fixed now: we can find the start of DRAM in the device
tree when necessary, but for EFI boot, we use the memory map to define
that kernel load address and pass it directly. This means we no longer
need to move the decompressor before invoking it
(d0f9ca9be11f25ef4151195eab7ea36d136084f6)

This uncovered another issue, though: the minimal relative alignment
of physical vs. virtual addresses was 16 MB, to ensure that the
PA-to-VA and vice versa routines worked correctly. So the tiniest
memory reservation at the start of DRAM would mean losing ~16 MB of
memory (given that DRAM below the kernel is not usable)

I fixed this in 9443076e4330a14ae2c6114307668b98a8293b77, and so now,
the kernel is loaded at the lowest available 2 MB boundary.

> 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 }
>
> 0x1F00 = efi_get_max_initrd_addr(0xff00);
>
> And below 0x1f00 there may be no RAM at all.
>

Yeah, that function is broken in more ways than one: the 512 MB is an
approximation, but you could actually boot the kernel with a larger
vmalloc space, in which case the lowmem region could be smaller than
512 MB.

But 

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

[Heinrich Schuchardt]

On 8/31/21 1:12 PM, Kristian Amlie wrote:

On 31/08/2021 12:46, Heinrich Schuchardt wrote:




*Von:* Ard Biesheuvel 
*Gesendet:* 31. August 2021 12:33:56 MESZ
*An:* Heinrich Schuchardt 
*CC:* Kristian Amlie 
*Betreff:* Re: [PATCH] efi_loader: Omit memory with "no-map" when
returning memory map.

On Tue, 31 Aug 2021 at 08:41, Heinrich Schuchardt  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 }

0x1F00 = efi_get_max_initrd_addr(0xff00);

And below 0x1f00 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
 


 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=c79e89ecaa246c880292ba68cbe08c9c30db77e3
 


 Cc Ard, maintainer of the kernel EFI stub.

 Best regards

 Heinrich


 Signed-off-by: Kristian Amlie 
 

 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, _mem)
 + list_for_each(lhandle, _mem) {
 + struct efi_mem_list *lmem;
 +
 + lmem = list_entry(lhandle, struct efi_mem_list, link);
 +
 + if (lmem->desc.type == EFI_RESERVED_MEMORY_TYPE)
 

Re: 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 
> *Gesendet:* 31. August 2021 12:33:56 MESZ
> *An:* Heinrich Schuchardt 
> *CC:* Kristian Amlie 
> *Betreff:* Re: [PATCH] efi_loader: Omit memory with "no-map" when
> returning memory map.
> 
> On Tue, 31 Aug 2021 at 08:41, Heinrich Schuchardt  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
> 
> 
> 
> 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=c79e89ecaa246c880292ba68cbe08c9c30db77e3
> 
> 
> 
> Cc Ard, maintainer of the kernel EFI stub.
> 
> Best regards
> 
> Heinrich
> 
> 
> Signed-off-by: Kristian Amlie 
> 
> 
> 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, _mem)
> + list_for_each(lhandle, _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);
>  

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

[Heinrich Schuchardt]

 Ursprüngliche Nachricht 
Von: Ard Biesheuvel 
Gesendet: 31. August 2021 12:33:56 MESZ
An: Heinrich Schuchardt 
CC: Kristian Amlie 
Betreff: Re: [PATCH] efi_loader: Omit memory with "no-map" when returning 
memory map.

On Tue, 31 Aug 2021 at 08:41, Heinrich Schuchardt  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.

> > 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.


> 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
>
> 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=c79e89ecaa246c880292ba68cbe08c9c30db77e3
>
> Cc Ard, maintainer of the kernel EFI stub.
>
> Best regards
>
> Heinrich
>
> >
> > Signed-off-by: Kristian Amlie 
> > ---
> >   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, _mem)
> > + list_for_each(lhandle, _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--;
> >   }
> >
>