| Message ID | 1446127645-4115-1-git-send-email-ard.biesheuvel@linaro.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 10/29/2015 09:07 AM, Ard Biesheuvel wrote: > The current arm64 Image relocation code in the UEFI stub assumes that > the dram_base argument it receives is always a multiple of 2 MB. In > reality, it is simply the lowest start address of all RAM entries in > the UEFI memory map, which means it could be any multiple of 4 KB. > > Since the arm64 kernel Image needs to reside TEXT_OFFSET bytes beyond > a 2 MB aligned base, or it will fail to boot, make sure we round dram_base > to 2 MB before using it to calculate the relocation address. > > Reported-by: Timur Tabi<timur@codeaurora.org> > Acked-by: Mark Rutland<mark.rutland@arm.com> > Signed-off-by: Ard Biesheuvel<ard.biesheuvel@linaro.org> This works for me Tested-by: Timur Tabi<timur@codeaurora.org> On my system, dram_base is 0x4000820000, and *image_addr is 0x43956db000, preferred_offset is set to 0x4000880000.
On Thu, Oct 29, 2015 at 03:07:25PM +0100, Ard Biesheuvel wrote: > The current arm64 Image relocation code in the UEFI stub assumes that > the dram_base argument it receives is always a multiple of 2 MB. In > reality, it is simply the lowest start address of all RAM entries in > the UEFI memory map, which means it could be any multiple of 4 KB. > > Since the arm64 kernel Image needs to reside TEXT_OFFSET bytes beyond > a 2 MB aligned base, or it will fail to boot, make sure we round dram_base > to 2 MB before using it to calculate the relocation address. > > Reported-by: Timur Tabi <timur@codeaurora.org> > Acked-by: Mark Rutland <mark.rutland@arm.com> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > --- > > If deemed acceptable, could we either get this into v4.3 or tag it for > stable (v4.3 only) please? So is this fixing a regression introduced in 4.3? If so, can I have a Fixes: tag too, please? (I was under the impression that this was a longstanding issue, but the thread hasn't been easy to follow). Will
On 10/29/2015 09:57 AM, Will Deacon wrote: > So is this fixing a regression introduced in 4.3? If so, can I have a > Fixes: tag too, please? (I was under the impression that this was a > longstanding issue, but the thread hasn't been easy to follow). It fixes a bug introduced by: Author: Ard Biesheuvel <ard.biesheuvel@linaro.org> 2015-07-24 06:38:27 Committer: Will Deacon <will.deacon@arm.com> 2015-07-27 05:08:41 Parent: ddeeefe2dfbe1fa6b116b9362b1bec465b64c873 (arm64/efi: efistub: Apply __init annotation) Child: 739fa11938bec53a849b77b2042237a0e17c70ae (arm64/efi: do not assume DRAM base is aligned to 2 MB) Branches: dcc, master, qserver, qserver2, remotes/origin/master, tty Follows: v4.2-rc4 Precedes: v4.3-rc1 arm64: efi: prefer AllocatePages() over efi_low_alloc() for vmlinux
On Thu, Oct 29, 2015 at 10:00:14AM -0500, Timur Tabi wrote: > On 10/29/2015 09:57 AM, Will Deacon wrote: > >So is this fixing a regression introduced in 4.3? If so, can I have a > >Fixes: tag too, please? (I was under the impression that this was a > >longstanding issue, but the thread hasn't been easy to follow). > > It fixes a bug introduced by: > > Author: Ard Biesheuvel <ard.biesheuvel@linaro.org> 2015-07-24 06:38:27 > Committer: Will Deacon <will.deacon@arm.com> 2015-07-27 05:08:41 > Parent: ddeeefe2dfbe1fa6b116b9362b1bec465b64c873 (arm64/efi: > efistub: Apply __init annotation) > Child: 739fa11938bec53a849b77b2042237a0e17c70ae (arm64/efi: do not > assume DRAM base is aligned to 2 MB) > Branches: dcc, master, qserver, qserver2, remotes/origin/master, tty > Follows: v4.2-rc4 > Precedes: v4.3-rc1 > > arm64: efi: prefer AllocatePages() over efi_low_alloc() for vmlinux As it wasn't in the context, that's commit ID e38457c361b30c5a, which was introduced in v4.3-rc1. However, the check on the base address was always buggy since its introduction in v3.16 in commit 3c7f255039a2ad6e ("arm64: efi: add EFI stub"), so that portion needs to be backported further. Thanks, Mark.
> On 29 okt. 2015, at 16:00, Timur Tabi <timur@codeaurora.org> wrote: > >> On 10/29/2015 09:57 AM, Will Deacon wrote: >> So is this fixing a regression introduced in 4.3? If so, can I have a >> Fixes: tag too, please? (I was under the impression that this was a >> longstanding issue, but the thread hasn't been easy to follow). > > It fixes a bug introduced by: > > Author: Ard Biesheuvel <ard.biesheuvel@linaro.org> 2015-07-24 06:38:27 > Committer: Will Deacon <will.deacon@arm.com> 2015-07-27 05:08:41 > Parent: ddeeefe2dfbe1fa6b116b9362b1bec465b64c873 (arm64/efi: efistub: Apply __init annotation) > Child: 739fa11938bec53a849b77b2042237a0e17c70ae (arm64/efi: do not assume DRAM base is aligned to 2 MB) > Branches: dcc, master, qserver, qserver2, remotes/origin/master, tty > Follows: v4.2-rc4 > Precedes: v4.3-rc1 > > arm64: efi: prefer AllocatePages() over efi_low_alloc() for vmlinux > > The code that existed before that also assumes incorrectly that dram_base is a multiple of 2MB, but since the patch won't apply cleanly, and nobody reported any problems, I think we only need to worry about 4.3 Ard.
diff --git a/arch/arm64/kernel/efi-stub.c b/arch/arm64/kernel/efi-stub.c index 816120ece6bc..78dfbd34b6bf 100644 --- a/arch/arm64/kernel/efi-stub.c +++ b/arch/arm64/kernel/efi-stub.c @@ -25,10 +25,20 @@ efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table_arg, unsigned long kernel_size, kernel_memsize = 0; unsigned long nr_pages; void *old_image_addr = (void *)*image_addr; + unsigned long preferred_offset; + + /* + * The preferred offset of the kernel Image is TEXT_OFFSET bytes beyond + * a 2 MB aligned base, which itself may be lower than dram_base, as + * long as the resulting offset equals or exceeds it. + */ + preferred_offset = round_down(dram_base, SZ_2M) + TEXT_OFFSET; + if (preferred_offset < dram_base) + preferred_offset += SZ_2M; /* Relocate the image, if required. */ kernel_size = _edata - _text; - if (*image_addr != (dram_base + TEXT_OFFSET)) { + if (*image_addr != preferred_offset) { kernel_memsize = kernel_size + (_end - _edata); /* @@ -42,7 +52,7 @@ efi_status_t __init handle_kernel_image(efi_system_table_t *sys_table_arg, * Mustang), we can still place the kernel at the address * 'dram_base + TEXT_OFFSET'. */ - *image_addr = *reserve_addr = dram_base + TEXT_OFFSET; + *image_addr = *reserve_addr = preferred_offset; nr_pages = round_up(kernel_memsize, EFI_ALLOC_ALIGN) / EFI_PAGE_SIZE; status = efi_call_early(allocate_pages, EFI_ALLOCATE_ADDRESS,