Re: [PATCH V2] efi_high_alloc: use EFI_ALLOCATE_MAX_ADDRESS

[Matt Fleming]

On Mon, 25 Aug, at 01:55:32PM, harald@xxxxxxxxxx wrote:
> From: Harald Hoyer <harald@xxxxxxxxxx>
> 
> On my Lenovo T420s with 4GB memory, efi_high_alloc() was checking the
> following memory regions:
> 
> 0x0000000000100000 - 0x0000000020000000
> 0x0000000020200000 - 0x0000000040000000
> 0x0000000040200000 - 0x00000000d2c02000
> 0x00000000d6e9f000 - 0x000000011e600000
> 
> and decided to allocate 2649 pages at address 0x11dba7000.
> ...
> [    0.000000] efi: mem53: type=2, attr=0xf, range=[0x000000011dba7000-0x000000011e600000) (10MB)
> ...
> [    0.000000] RAMDISK: [mem 0x11dba7000-0x11e5fffff]
> ...
> [    0.154933] Unpacking initramfs...
> [    0.160990] Initramfs unpacking failed: junk in compressed archive
> [    0.163436] Freeing initrd memory: 10596K (ffff88011dba7000 - ffff88011e600000)
> ...
> 
> Nevertheless, unpacking of the initramfs later on failed.
> This is maybe caused by my buggy EFI BIOS and
> commit 4bf7111f50167133a71c23530ca852a41355e739,
> which enables loading the initramfs above 4G addresses.
> 
> With this patch efi_high_alloc() now uses EFI_ALLOCATE_MAX_ADDRESS,
> which should do the same as before, but use the EFI logic to select the high memory range.
 
No, that's not correct. Your patch changes the semantics of
efi_high_alloc(). The original version allocates from the top of memory
down, so you always get the highest aligned address, that is no higher
than 'max_addr'.

Your version allocates some address that isn't above 'max_addr', but it
needn't necessarily be the highest possible address. The following is
taken from the AllocatePages() documentation in the UEFI spec,

  "Allocation requests of Type AllocateMaxAddress allocate any available
   range of pages whose uppermost address is less than or equal to the
   address pointed to by Memory on input."

Note the part about allocating *any* available range.

Furthermore, there are more callers of efi_high_alloc() than the initrd
loading case, and you've changed their behaviour with this patch.

I get where you're coming from, but this isn't the best way to solve
this problem, sorry. NAK.

-- 
Matt Fleming, Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/