| Message ID | 1438164259-14470-2-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | New |
| Headers | show |
On 30 July 2015 at 16:01, Matt Fleming <matt@codeblueprint.co.uk> wrote: > On Wed, 29 Jul, at 12:04:18PM, Ard Biesheuvel wrote: >> In some cases, e.g., when allocating memory for the arm64 kernel, >> we need memory at a certain offset from an aligned boundary. So add >> an offset parameter to efi_low_alloc(), and update the existing >> callers to pass zero by default. >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> --- >> arch/arm64/kernel/efi-stub.c | 2 +- >> arch/x86/boot/compressed/eboot.c | 4 ++-- >> drivers/firmware/efi/libstub/efi-stub-helper.c | 20 +++++++++++++++----- >> include/linux/efi.h | 2 +- >> 4 files changed, 19 insertions(+), 9 deletions(-) > > [...] > >> @@ -269,10 +269,19 @@ efi_status_t efi_low_alloc(efi_system_table_t *sys_table_arg, >> * checks pointers against NULL. Skip the first 8 >> * bytes so we start at a nice even number. >> */ >> - if (start == 0x0) >> + if (start + offset == 0x0) >> start += 8; >> >> - start = round_up(start, align); >> + /* >> + * Check if the offset exceeds the misalignment of this region. >> + * In that case, we can round down instead of up, and the >> + * resulting start value will be correctly aligned and still >> + * point past the start of the region. >> + */ >> + if (offset >= (start & (align - 1))) >> + start = round_down(start, align) + offset; >> + else >> + start = round_up(start, align) + offset; >> if ((start + size) > end) >> continue; > > Aha, now I see what you mean. Thanks for doing this Ard, these are much > more polished than what I was expecting. > > I'm gonna have to NAK this because it's just too much of a special case > to support directly in efi_low_alloc(), which I think was the exact > point that you made originally, and which I was too tired/dumb to > understand. Sorry. > No worries. Will has already queued the original patch, which solves all know issues regarding the placement of the kernel image by the EFI stub. > In particular, the fact that you can use the offset argument to violate > the requested alignment seems like it would trip up most users. > Yes. We could always rename this enhanced efi_low_alloc() to efi_low_alloc_with_offset() and introduce a new efi_low_alloc() which calls it using an offset of zero. But only if you insist.
diff --git a/arch/arm64/kernel/efi-stub.c b/arch/arm64/kernel/efi-stub.c index f5374065ad53..d85a0b2098b3 100644 --- a/arch/arm64/kernel/efi-stub.c +++ b/arch/arm64/kernel/efi-stub.c @@ -29,7 +29,7 @@ efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table, if (*image_addr != (dram_base + TEXT_OFFSET)) { kernel_memsize = kernel_size + (_end - _edata); status = efi_low_alloc(sys_table, kernel_memsize + TEXT_OFFSET, - SZ_2M, reserve_addr); + SZ_2M, 0, reserve_addr); if (status != EFI_SUCCESS) { pr_efi_err(sys_table, "Failed to relocate kernel\n"); return status; diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c index 2c82bd150d43..33d1a72f4266 100644 --- a/arch/x86/boot/compressed/eboot.c +++ b/arch/x86/boot/compressed/eboot.c @@ -1077,7 +1077,7 @@ struct boot_params *make_boot_params(struct efi_config *c) return NULL; } - status = efi_low_alloc(sys_table, 0x4000, 1, + status = efi_low_alloc(sys_table, 0x4000, 1, 0, (unsigned long *)&boot_params); if (status != EFI_SUCCESS) { efi_printk(sys_table, "Failed to alloc lowmem for boot params\n"); @@ -1424,7 +1424,7 @@ struct boot_params *efi_main(struct efi_config *c, } gdt->size = 0x800; - status = efi_low_alloc(sys_table, gdt->size, 8, + status = efi_low_alloc(sys_table, gdt->size, 8, 0, (unsigned long *)&gdt->address); if (status != EFI_SUCCESS) { efi_printk(sys_table, "Failed to alloc mem for gdt\n"); diff --git a/drivers/firmware/efi/libstub/efi-stub-helper.c b/drivers/firmware/efi/libstub/efi-stub-helper.c index f07d4a67fa76..67d1759781c5 100644 --- a/drivers/firmware/efi/libstub/efi-stub-helper.c +++ b/drivers/firmware/efi/libstub/efi-stub-helper.c @@ -226,7 +226,7 @@ fail: */ efi_status_t efi_low_alloc(efi_system_table_t *sys_table_arg, unsigned long size, unsigned long align, - unsigned long *addr) + unsigned long offset, unsigned long *addr) { unsigned long map_size, desc_size; efi_memory_desc_t *map; @@ -269,10 +269,19 @@ efi_status_t efi_low_alloc(efi_system_table_t *sys_table_arg, * checks pointers against NULL. Skip the first 8 * bytes so we start at a nice even number. */ - if (start == 0x0) + if (start + offset == 0x0) start += 8; - start = round_up(start, align); + /* + * Check if the offset exceeds the misalignment of this region. + * In that case, we can round down instead of up, and the + * resulting start value will be correctly aligned and still + * point past the start of the region. + */ + if (offset >= (start & (align - 1))) + start = round_down(start, align) + offset; + else + start = round_up(start, align) + offset; if ((start + size) > end) continue; @@ -580,7 +589,7 @@ efi_status_t efi_relocate_kernel(efi_system_table_t *sys_table_arg, * possible. */ if (status != EFI_SUCCESS) { - status = efi_low_alloc(sys_table_arg, alloc_size, alignment, + status = efi_low_alloc(sys_table_arg, alloc_size, alignment, 0, &new_addr); } if (status != EFI_SUCCESS) { @@ -684,7 +693,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_low_alloc(sys_table_arg, options_bytes, 0, 0, + &cmdline_addr); if (status != EFI_SUCCESS) return NULL; diff --git a/include/linux/efi.h b/include/linux/efi.h index faafa1ad6ea7..e738e97632ba 100644 --- a/include/linux/efi.h +++ b/include/linux/efi.h @@ -1245,7 +1245,7 @@ efi_status_t efi_get_memory_map(efi_system_table_t *sys_table_arg, efi_status_t efi_low_alloc(efi_system_table_t *sys_table_arg, unsigned long size, unsigned long align, - unsigned long *addr); + unsigned long offset, unsigned long *addr); efi_status_t efi_high_alloc(efi_system_table_t *sys_table_arg, unsigned long size, unsigned long align,
In some cases, e.g., when allocating memory for the arm64 kernel, we need memory at a certain offset from an aligned boundary. So add an offset parameter to efi_low_alloc(), and update the existing callers to pass zero by default. Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- arch/arm64/kernel/efi-stub.c | 2 +- arch/x86/boot/compressed/eboot.c | 4 ++-- drivers/firmware/efi/libstub/efi-stub-helper.c | 20 +++++++++++++++----- include/linux/efi.h | 2 +- 4 files changed, 19 insertions(+), 9 deletions(-)