2023-09-12 10:27:11

by Ard Biesheuvel

[permalink] [raw]
Subject: [PATCH v2 08/15] x86/boot: Drop references to startup_64

From: Ard Biesheuvel <[email protected]>

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 <[email protected]>
---
arch/x86/boot/Makefile | 2 +-
arch/x86/boot/tools/build.c | 3 ---
2 files changed, 1 insertion(+), 4 deletions(-)

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');
--
2.42.0.283.g2d96d420d3-goog


2023-09-15 10:38:18

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 08/15] x86/boot: Drop references to startup_64


* Ard Biesheuvel <[email protected]> wrote:

> From: Ard Biesheuvel <[email protected]>
>
> 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 <[email protected]>
> ---
> 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

2023-09-15 13:52:22

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v2 08/15] x86/boot: Drop references to startup_64

On Fri, 15 Sept 2023 at 11:15, Ingo Molnar <[email protected]> wrote:
>
>
> * Ard Biesheuvel <[email protected]> wrote:
>
> > From: Ard Biesheuvel <[email protected]>
> >
> > 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 <[email protected]>
> > ---
> > 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 <[email protected]>
AuthorDate: Mon May 11 18:58:49 2020 -0400
Commit: Ard Biesheuvel <[email protected]>
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.

Subject: [tip: x86/boot] x86/boot: Drop references to startup_64

The following commit has been merged into the x86/boot branch of tip:

Commit-ID: b618d31f112bea3d2daea19190d63e567f32a4db
Gitweb: https://git.kernel.org/tip/b618d31f112bea3d2daea19190d63e567f32a4db
Author: Ard Biesheuvel <[email protected]>
AuthorDate: Tue, 12 Sep 2023 09:00:59
Committer: Ingo Molnar <[email protected]>
CommitterDate: Fri, 15 Sep 2023 11:18:42 +02:00

x86/boot: Drop references to startup_64

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 <[email protected]>
Signed-off-by: Ingo Molnar <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
---
arch/x86/boot/Makefile | 2 +-
arch/x86/boot/tools/build.c | 3 ---
2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/arch/x86/boot/Makefile b/arch/x86/boot/Makefile
index f33e45e..0e98bc5 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 efa4e9c..10b0207 100644
--- a/arch/x86/boot/tools/build.c
+++ b/arch/x86/boot/tools/build.c
@@ -60,7 +60,6 @@ static unsigned long efi64_stub_entry;
static unsigned long efi_pe_entry;
static unsigned long efi32_pe_entry;
static unsigned long kernel_info;
-static unsigned long startup_64;
static unsigned long _end;

/*----------------------------------------------------------------------*/
@@ -264,7 +263,6 @@ static void efi_stub_defaults(void)
efi_pe_entry = 0x10;
#else
efi_pe_entry = 0x210;
- startup_64 = 0x200;
#endif
}

@@ -340,7 +338,6 @@ static void parse_zoffset(char *fname)
PARSE_ZOFS(p, efi_pe_entry);
PARSE_ZOFS(p, efi32_pe_entry);
PARSE_ZOFS(p, kernel_info);
- PARSE_ZOFS(p, startup_64);
PARSE_ZOFS(p, _end);

p = strchr(p, '\n');

2023-09-15 15:50:40

by Ard Biesheuvel

[permalink] [raw]
Subject: Re: [PATCH v2 08/15] x86/boot: Drop references to startup_64

On Fri, 15 Sept 2023 at 17:45, Ingo Molnar <[email protected]> wrote:
>
>
> * Ingo Molnar <[email protected]> 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/[email protected]/T/#u
https://lore.kernel.org/linux-efi/[email protected]/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.

2023-09-15 18:42:12

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 08/15] x86/boot: Drop references to startup_64


* Ard Biesheuvel <[email protected]> wrote:

> On Fri, 15 Sept 2023 at 11:15, Ingo Molnar <[email protected]> wrote:
> >
> >
> > * Ard Biesheuvel <[email protected]> wrote:
> >
> > > From: Ard Biesheuvel <[email protected]>
> > >
> > > 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 <[email protected]>
> > > ---
> > > 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

2023-09-15 22:39:39

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH v2 08/15] x86/boot: Drop references to startup_64


* Ingo Molnar <[email protected]> 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