| Message ID | 20230912090051.4014114-25-ardb@google.com |
|---|---|
| State | New |
| Headers | show |
| Series | x86/boot: Rework PE header generation | expand |
* Ard Biesheuvel <ardb@google.com> wrote: > From: Ard Biesheuvel <ardb@kernel.org> > > The x86 boot image generation tool assign a default value to startup_64 > and subsequently parses the actual value from zoffset.h but it never > actually uses the value anywhere. So remove this code. > > This change has no impact on the resulting bzImage binary. > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > --- > arch/x86/boot/Makefile | 2 +- > arch/x86/boot/tools/build.c | 3 --- > 2 files changed, 1 insertion(+), 4 deletions(-) Note that this patch conflicted with a recent upstream cleanup commit: e78d334a5470 ("x86/boot: Mark global variables as static") It was trivial to resolve, but please double-check the result once I push out the new tip:x86/boot tree. Thanks, Ingo
On Fri, 15 Sept 2023 at 11:15, Ingo Molnar <mingo@kernel.org> wrote: > > > * Ard Biesheuvel <ardb@google.com> wrote: > > > From: Ard Biesheuvel <ardb@kernel.org> > > > > The x86 boot image generation tool assign a default value to startup_64 > > and subsequently parses the actual value from zoffset.h but it never > > actually uses the value anywhere. So remove this code. > > > > This change has no impact on the resulting bzImage binary. > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > --- > > arch/x86/boot/Makefile | 2 +- > > arch/x86/boot/tools/build.c | 3 --- > > 2 files changed, 1 insertion(+), 4 deletions(-) > > Note that this patch conflicted with a recent upstream cleanup commit: > > e78d334a5470 ("x86/boot: Mark global variables as static") > > It was trivial to resolve, but please double-check the result once I push > out the new tip:x86/boot tree. > Ehm, I suspect something is going on with your workflow - did you apply my patches out of order perhaps? (/me notes that you seem to have omitted patches #7 and #9) The patch you refer to is commit e78d334a5470ead861590ec83158f3b17bd6c807 Author: Arvind Sankar <nivedita@alum.mit.edu> AuthorDate: Mon May 11 18:58:49 2020 -0400 Commit: Ard Biesheuvel <ardb@kernel.org> CommitDate: Thu May 14 11:11:20 2020 +0200 x86/boot: Mark global variables as static which went into v5.7 as a late fix via the EFI tree.
* Ard Biesheuvel <ardb@kernel.org> wrote: > On Fri, 15 Sept 2023 at 11:15, Ingo Molnar <mingo@kernel.org> wrote: > > > > > > * Ard Biesheuvel <ardb@google.com> wrote: > > > > > From: Ard Biesheuvel <ardb@kernel.org> > > > > > > The x86 boot image generation tool assign a default value to startup_64 > > > and subsequently parses the actual value from zoffset.h but it never > > > actually uses the value anywhere. So remove this code. > > > > > > This change has no impact on the resulting bzImage binary. > > > > > > Signed-off-by: Ard Biesheuvel <ardb@kernel.org> > > > --- > > > arch/x86/boot/Makefile | 2 +- > > > arch/x86/boot/tools/build.c | 3 --- > > > 2 files changed, 1 insertion(+), 4 deletions(-) > > > > Note that this patch conflicted with a recent upstream cleanup commit: > > > > e78d334a5470 ("x86/boot: Mark global variables as static") > > > > It was trivial to resolve, but please double-check the result once I push > > out the new tip:x86/boot tree. > > > > Ehm, I suspect something is going on with your workflow - did you > apply my patches out of order perhaps? (/me notes that you seem to > have omitted patches #7 Indeed: patch #7 was not in my inbox - nor is it in my lkml folder: 664225 Sep 12 Ard Biesheuvel | ├─>[PATCH v2 04/15] x86/boot: Remove the 'bugger off' message 664226 Sep 12 Ard Biesheuvel | ├─>[PATCH v2 05/15] x86/boot: Omit compression buffer from PE/COFF image memory footprint 664227 Sep 12 Ard Biesheuvel | ├─>[PATCH v2 06/15] x86/boot: Drop redundant code setting the root device 664228 Sep 12 Ard Biesheuvel | ├─>[PATCH v2 08/15] x86/boot: Drop references to startup_64 664229 Sep 12 Ard Biesheuvel | ├─>[PATCH v2 10/15] x86/boot: Define setup size in linker script 664230 Sep 12 Ard Biesheuvel | ├─>[PATCH v2 12/15] x86/boot: Construct PE/COFF .text section from assembler 664231 Sep 12 Ard Biesheuvel | ├─>[PATCH v2 13/15] x86/boot: Drop PE/COFF .reloc section 664232 Sep 12 Ard Biesheuvel | ├─>[PATCH v2 14/15] x86/boot: Split off PE/COFF .data section :-/ Very weird - could it have gotten lost in the sending process, on your side? Thanks, Ingo
* Ingo Molnar <mingo@kernel.org> wrote: > Very weird - could it have gotten lost in the sending process, on your > side? In any case I've dropped patch #8 from tip:x86/boot as well - the first 6 patches arrived fine and are in the intended order. Once the boot problem has been resolved, mind resending the rest? There are no changes in -tip that should be interfering: tip:x86/boot has an upstream base. Thanks, Ingo
On Fri, 15 Sept 2023 at 17:45, Ingo Molnar <mingo@kernel.org> wrote: > > > * Ingo Molnar <mingo@kernel.org> wrote: > > > Very weird - could it have gotten lost in the sending process, on your > > side? > Lore does have them but I have no idea whose end caused this - I recently switched to Google's internal infrastructure for outgoing GIT email as our network does not permit outgoing SMTP to kernel.org but this is the first time I heard of a potential issue of this nature. https://lore.kernel.org/linux-efi/20230912090051.4014114-24-ardb@google.com/T/#u https://lore.kernel.org/linux-efi/20230912090051.4014114-26-ardb@google.com/T/#u > In any case I've dropped patch #8 from tip:x86/boot as well - the first 6 > patches arrived fine and are in the intended order. > > Once the boot problem has been resolved, mind resending the rest? > > There are no changes in -tip that should be interfering: tip:x86/boot has > an upstream base. > Yeah I'll respin and resend. Thanks.
diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile index f33e45ed1437..0e98bc503699 100644 --- a/arch/x86/boot/Makefile +++ b/arch/x86/boot/Makefile @@ -89,7 +89,7 @@ $(obj)/vmlinux.bin: $(obj)/compressed/vmlinux FORCE SETUP_OBJS = $(addprefix $(obj)/,$(setup-y)) -sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [a-zA-Z] \(startup_32\|startup_64\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|efi32_pe_entry\|input_data\|kernel_info\|_end\|_ehead\|_text\|z_.*\)$$/\#define ZO_\2 0x\1/p' +sed-zoffset := -e 's/^\([0-9a-fA-F]*\) [a-zA-Z] \(startup_32\|efi32_stub_entry\|efi64_stub_entry\|efi_pe_entry\|efi32_pe_entry\|input_data\|kernel_info\|_end\|_ehead\|_text\|z_.*\)$$/\#define ZO_\2 0x\1/p' quiet_cmd_zoffset = ZOFFSET $@ cmd_zoffset = $(NM) $< | sed -n $(sed-zoffset) > $@ diff --git a/arch/x86/boot/tools/build.c b/arch/x86/boot/tools/build.c index 660627ea6cbb..14ef13fe7ab0 100644 --- a/arch/x86/boot/tools/build.c +++ b/arch/x86/boot/tools/build.c @@ -59,7 +59,6 @@ static unsigned long efi32_stub_entry; static unsigned long efi64_stub_entry; static unsigned long efi_pe_entry; static unsigned long efi32_pe_entry; -static unsigned long startup_64; static unsigned long _end; /*----------------------------------------------------------------------*/ @@ -263,7 +262,6 @@ static void efi_stub_defaults(void) efi_pe_entry = 0x10; #else efi_pe_entry = 0x210; - startup_64 = 0x200; #endif } @@ -338,7 +336,6 @@ static void parse_zoffset(char *fname) PARSE_ZOFS(p, efi64_stub_entry); PARSE_ZOFS(p, efi_pe_entry); PARSE_ZOFS(p, efi32_pe_entry); - PARSE_ZOFS(p, startup_64); PARSE_ZOFS(p, _end); p = strchr(p, '\n');