On Fri, Apr 10, 2015 at 10:14 AM, Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote: > On 10 April 2015 at 18:43, Roy Franz <roy.franz@xxxxxxxxxx> wrote: >> On Fri, Apr 10, 2015 at 5:56 AM, Ard Biesheuvel >> <ard.biesheuvel@xxxxxxxxxx> wrote: >>> This changes the allocation for the ASCII-converted command >>> line to use an ordinary memory pool rather than a separate >>> page based allocation. >>> >>> Pool allocations are generally preferred over page based >>> allocations due to the fact that they cause less fragmentation, >>> but in the particular case of arm64, where page allocations are >>> rounded up to 64 KB and where this allocation happens to be the >>> only explicit low allocation, it results in the lowest 64 KB of >>> memory to always be taken up by this particular allocation. >>> >>> So allocate from the EFI_LOADER_DATA pool instead. >>> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> >>> --- >>> drivers/firmware/efi/libstub/efi-stub-helper.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c >>> index f07d4a67fa76..c95a567ca132 100644 >>> --- a/drivers/firmware/efi/libstub/efi-stub-helper.c >>> +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c >>> @@ -684,7 +684,8 @@ char *efi_convert_cmdline(efi_system_table_t *sys_table_arg, >>> >>> options_bytes++; /* NUL termination */ >>> >>> - status = efi_low_alloc(sys_table_arg, options_bytes, 0, &cmdline_addr); >>> + status = efi_call_early(allocate_pool, EFI_LOADER_DATA, >>> + options_bytes, (void **)&cmdline_addr); >>> if (status != EFI_SUCCESS) >>> return NULL; >>> >>> -- >>> 1.8.3.2 >>> >> >> Hi Ard, >> >> We can't do this without also changing the frees on the error paths in >> arm-stub.c (line 289) >> and eboot.c (line 1148) to be a pool free as well. >> Also, for x86/x86_64 the address of the command line is put into a >> boot_params structure, >> which only has a __u32 field for the address of the command line, so >> we could get an address >> that won't fit if we use a pool allocation. >> Looking at that bit of if it, I don't think we handle the case of no >> 32 bit addressable memory >> existing at the time of the efi_low_alloc() for x86_64 systems, so if >> a >32 bit address is returned here, >> this won't be detected as a failure, and the cmdline address will be >> the 32 bit truncated address. I'm working on a patch for this, and it also looks like efi_low_alloc() is not correct for 32 bit builds, as internally it can allocate a >32 bit address, and then will truncate that to return an 'unsigned long'. I should have something out next week. >> >> I don't think that there is a way to control the address range >> returned by pool allocations, >> so I think we are stuck with the page based allocations if we have >> address restrictions. >> > > Ah yes, I wondered about the reason for the low_alloc(). I guess I > could have looked a bit further myself :-) > > It is not such a big deal: the memory is reclaimed anyway, I was just > trying to reduce the fragmentation a bit, and trying to avoid > efi_xxx_alloc() which are substantially heavier than calling > allocate_pool() or allocate_pages() directly. > > I'll drop this patch then. It's not really worth the effort as its > primarily cosmetics anyway. > > -- > Ard. -- To unsubscribe from this list: send the line "unsubscribe linux-efi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
