From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751218AbdJVMsN (ORCPT ); Sun, 22 Oct 2017 08:48:13 -0400 Received: from pandora.armlinux.org.uk ([78.32.30.218]:55250 "EHLO pandora.armlinux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750966AbdJVMsM (ORCPT ); Sun, 22 Oct 2017 08:48:12 -0400 Date: Sun, 22 Oct 2017 13:47:57 +0100 From: Russell King - ARM Linux To: Ard Biesheuvel Cc: Jeffy Chen , "linux-kernel@vger.kernel.org" , chris.zhong@rock-chips.com, Ingo Molnar , "linux-arm-kernel@lists.infradead.org" Subject: Re: [PATCH] ARM: Fix zImage file size not aligned with CONFIG_EFI_STUB enabled Message-ID: <20171022124757.GL20805@n2100.armlinux.org.uk> References: <20171018050108.10352-1-jeffy.chen@rock-chips.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Oct 22, 2017 at 12:01:13PM +0100, Ard Biesheuvel wrote: > On 18 October 2017 at 06:01, Jeffy Chen wrote: > > The zImage file size should be aligned. > > > > Fixes: e4bae4d0b5f3 ("arm/efi: Split zImage code and data into separate PE/COFF sections") > > Signed-off-by: Jeffy Chen > > --- > > > > arch/arm/boot/compressed/vmlinux.lds.S | 8 ++++---- > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > diff --git a/arch/arm/boot/compressed/vmlinux.lds.S b/arch/arm/boot/compressed/vmlinux.lds.S > > index b38dcef90756..1636fa259577 100644 > > --- a/arch/arm/boot/compressed/vmlinux.lds.S > > +++ b/arch/arm/boot/compressed/vmlinux.lds.S > > @@ -70,10 +70,6 @@ SECTIONS > > .got : { *(.got) } > > _got_end = .; > > > > - /* ensure the zImage file size is always a multiple of 64 bits */ > > - /* (without a dummy byte, ld just ignores the empty section) */ > > - .pad : { BYTE(0); . = ALIGN(8); } > > - > > #ifdef CONFIG_EFI_STUB > > .data : ALIGN(4096) { > > __pecoff_data_start = .; > > @@ -93,6 +89,10 @@ SECTIONS > > __pecoff_data_rawsize = . - ADDR(.data); > > #endif > > > > + /* ensure the zImage file size is always a multiple of 64 bits */ > > + /* (without a dummy byte, ld just ignores the empty section) */ > > + .pad : { BYTE(0); . = ALIGN(8); } > > + > > _edata = .; > > > > _magic_sig = ZIMAGE_MAGIC(0x016f2818); > > -- > > 2.11.0 > > > > This is not the right fix. If CONFIG_EFI_STUB is enabled, the zImage > filesize should be rounded up to 512 bytes not 8 bytes. The '. = > ALIGN(512);' in the .data section appears to ensure that, but for some > reason, that appears not to be working. Actually, the existing .pad section is totally and utterly bogus when EFI is enabled: . = ALIGN(4); _etext = .; .got.plt : { *(.got.plt) } _got_start = .; .got : { *(.got) } _got_end = .; The .got.plt and .got are always word-based. This is then followed by .pad, which does nothing but pad out to a multiple of 64 bit: /* ensure the zImage file size is always a multiple of 64 bits */ /* (without a dummy byte, ld just ignores the empty section) */ .pad : { BYTE(0); . = ALIGN(8); } So this may add zero or 4 bytes of padding. This is then followed by the EFI data: .data : ALIGN(4096) { ... . = ALIGN(512); } which is aligned to 4K but aligns the end of itself to 512. So, we have the end of .got aligned to 4, followed by .pad that tries to align to 8, followed by an optional .data section. This is pointless. A sane patch would be to choose between the EFI .data section and the .pad section. So, it should be: #ifdef CONFIG_EFI_STUB .data : ALIGN(4096) { ... . = ALIGN(512); } #else .pad : { BYTE(0); . = ALIGN(8); } #endif -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up According to speedtest.net: 8.21Mbps down 510kbps up