On 30 July 2015 at 16:01, Matt Fleming <matt@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxx> >> --- >> 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. -- 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