diff mbox series

[v2] x86, realmode: explicitly set entry via command line

Message ID 20190924193310.132104-1-ndesaulniers@google.com
State New
Headers show
Series [v2] x86, realmode: explicitly set entry via command line | expand

Commit Message

Nick Desaulniers Sept. 24, 2019, 7:33 p.m. UTC 
Linking with ld.lld via $ make LD=ld.lld produces the warning:
ld.lld: warning: cannot find entry symbol _start; defaulting to 0x1000

Linking with ld.bfd shows the default entry is 0x1000:
$ readelf -h arch/x86/realmode/rm/realmode.elf | grep Entry
  Entry point address:               0x1000

While ld.lld is being pedantic, just set the entry point explicitly,
instead of depending on the implicit default.

Link: https://github.com/ClangBuiltLinux/linux/issues/216
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

---
Changes V1 -> V2:
* Use command line flag, rather than linker script, as ld.bfd produces a
  syntax error for `ENTRY(0x1000)` but is happy with `-e 0x1000`

 arch/x86/realmode/rm/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.23.0.351.gc4317032e6-goog

Comments

Nick Desaulniers Sept. 24, 2019, 7:37 p.m. UTC | #1 
On Tue, Sep 24, 2019 at 12:33 PM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>

> Linking with ld.lld via $ make LD=ld.lld produces the warning:

> ld.lld: warning: cannot find entry symbol _start; defaulting to 0x1000

>

> Linking with ld.bfd shows the default entry is 0x1000:

> $ readelf -h arch/x86/realmode/rm/realmode.elf | grep Entry

>   Entry point address:               0x1000

>

> While ld.lld is being pedantic, just set the entry point explicitly,

> instead of depending on the implicit default.

>

> Link: https://github.com/ClangBuiltLinux/linux/issues/216

> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>


I meant to pick up Sedat's reported by tag:
Reported-by: Sedat Dilek <sedat.dilek@gmail.com>

> ---

> Changes V1 -> V2:

> * Use command line flag, rather than linker script, as ld.bfd produces a

>   syntax error for `ENTRY(0x1000)` but is happy with `-e 0x1000`

>

>  arch/x86/realmode/rm/Makefile | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/arch/x86/realmode/rm/Makefile b/arch/x86/realmode/rm/Makefile

> index f60501a384f9..338a00c5257f 100644

> --- a/arch/x86/realmode/rm/Makefile

> +++ b/arch/x86/realmode/rm/Makefile

> @@ -46,7 +46,7 @@ $(obj)/pasyms.h: $(REALMODE_OBJS) FORCE

>  targets += realmode.lds

>  $(obj)/realmode.lds: $(obj)/pasyms.h

>

> -LDFLAGS_realmode.elf := -m elf_i386 --emit-relocs -T

> +LDFLAGS_realmode.elf := -m elf_i386 --emit-relocs -e 0x1000 -T

>  CPPFLAGS_realmode.lds += -P -C -I$(objtree)/$(obj)

>

>  targets += realmode.elf

> --

> 2.23.0.351.gc4317032e6-goog

>



-- 
Thanks,
~Nick Desaulniers
Borislav Petkov Sept. 25, 2019, 10:20 a.m. UTC | #2 
+ some more people who did the unified realmode thing.

On Tue, Sep 24, 2019 at 12:33:08PM -0700, Nick Desaulniers wrote:
> Linking with ld.lld via $ make LD=ld.lld produces the warning:

> ld.lld: warning: cannot find entry symbol _start; defaulting to 0x1000

> 

> Linking with ld.bfd shows the default entry is 0x1000:

> $ readelf -h arch/x86/realmode/rm/realmode.elf | grep Entry

>   Entry point address:               0x1000

> 

> While ld.lld is being pedantic, just set the entry point explicitly,

> instead of depending on the implicit default.

> 

> Link: https://github.com/ClangBuiltLinux/linux/issues/216

> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

> ---

> Changes V1 -> V2:

> * Use command line flag, rather than linker script, as ld.bfd produces a

>   syntax error for `ENTRY(0x1000)` but is happy with `-e 0x1000`

> 

>  arch/x86/realmode/rm/Makefile | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

> 

> diff --git a/arch/x86/realmode/rm/Makefile b/arch/x86/realmode/rm/Makefile

> index f60501a384f9..338a00c5257f 100644

> --- a/arch/x86/realmode/rm/Makefile

> +++ b/arch/x86/realmode/rm/Makefile

> @@ -46,7 +46,7 @@ $(obj)/pasyms.h: $(REALMODE_OBJS) FORCE

>  targets += realmode.lds

>  $(obj)/realmode.lds: $(obj)/pasyms.h

>  

> -LDFLAGS_realmode.elf := -m elf_i386 --emit-relocs -T

> +LDFLAGS_realmode.elf := -m elf_i386 --emit-relocs -e 0x1000 -T


So looking at arch/x86/realmode/rm/realmode.lds.S: what's stopping
people from adding more sections before the first

. = ALIGN(PAGE_SIZE);

which, with enough bytes to go above the first 4K, would cause that
alignment to go to 0x2000 and then your hardcoded address would be
wrong, all of a sudden.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
Nick Desaulniers Sept. 25, 2019, 4:35 p.m. UTC | #3 
+ Fangrui, Peter, Rui, George (LLD)

On Wed, Sep 25, 2019 at 3:20 AM Borislav Petkov <bp@alien8.de> wrote:
>

> + some more people who did the unified realmode thing.

>

> On Tue, Sep 24, 2019 at 12:33:08PM -0700, Nick Desaulniers wrote:

> > Linking with ld.lld via $ make LD=ld.lld produces the warning:

> > ld.lld: warning: cannot find entry symbol _start; defaulting to 0x1000

> >

> > Linking with ld.bfd shows the default entry is 0x1000:

> > $ readelf -h arch/x86/realmode/rm/realmode.elf | grep Entry

> >   Entry point address:               0x1000

> >

> > While ld.lld is being pedantic, just set the entry point explicitly,

> > instead of depending on the implicit default.

> >

> > Link: https://github.com/ClangBuiltLinux/linux/issues/216

> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

> > ---

> > Changes V1 -> V2:

> > * Use command line flag, rather than linker script, as ld.bfd produces a

> >   syntax error for `ENTRY(0x1000)` but is happy with `-e 0x1000`

> >

> >  arch/x86/realmode/rm/Makefile | 2 +-

> >  1 file changed, 1 insertion(+), 1 deletion(-)

> >

> > diff --git a/arch/x86/realmode/rm/Makefile b/arch/x86/realmode/rm/Makefile

> > index f60501a384f9..338a00c5257f 100644

> > --- a/arch/x86/realmode/rm/Makefile

> > +++ b/arch/x86/realmode/rm/Makefile

> > @@ -46,7 +46,7 @@ $(obj)/pasyms.h: $(REALMODE_OBJS) FORCE

> >  targets += realmode.lds

> >  $(obj)/realmode.lds: $(obj)/pasyms.h

> >

> > -LDFLAGS_realmode.elf := -m elf_i386 --emit-relocs -T

> > +LDFLAGS_realmode.elf := -m elf_i386 --emit-relocs -e 0x1000 -T

>

> So looking at arch/x86/realmode/rm/realmode.lds.S: what's stopping

> people from adding more sections before the first

>

> . = ALIGN(PAGE_SIZE);

>

> which, with enough bytes to go above the first 4K, would cause that

> alignment to go to 0x2000 and then your hardcoded address would be

> wrong, all of a sudden.


Thanks for the consideration Boris.  So IIUC if the preceding sections
are larger than 0x1000 altogether, setting the entry there will be
wrong?

Currently, .text looks like it's currently at 0x1000 for a defconfig,
and I assume that could move in the case I stated above?
$ readelf -S arch/x86/realmode/rm/realmode.elf | grep text
  [ 3] .text             PROGBITS        00001000 201000 000f51 00  AX
 0   0 4096
...

In that case, it seems that maybe I should set the ENTRY in the linker
script as:
diff --git a/arch/x86/realmode/rm/realmode.lds.S
b/arch/x86/realmode/rm/realmode.lds.S
index 3bb980800c58..64d135d1ee63 100644
--- a/arch/x86/realmode/rm/realmode.lds.S
+++ b/arch/x86/realmode/rm/realmode.lds.S
@@ -11,6 +11,7 @@

 OUTPUT_FORMAT("elf32-i386")
 OUTPUT_ARCH(i386)
+ENTRY(pa_text_start)

 SECTIONS
 {

-- 
Thanks,
~Nick Desaulniers
Borislav Petkov Sept. 25, 2019, 5:10 p.m. UTC | #4 
On Wed, Sep 25, 2019 at 09:35:24AM -0700, Nick Desaulniers wrote:
> Thanks for the consideration Boris.  So IIUC if the preceding sections

> are larger than 0x1000 altogether, setting the entry there will be

> wrong?


Well, I spent some time this morning grepping to find out whether PA
0x1000 was magical but didn't find anything. Perhaps hpa can refresh my
memory...

> Currently, .text looks like it's currently at 0x1000 for a defconfig,

> and I assume that could move in the case I stated above?


Yes, I think we shouldn't hardcode.

> $ readelf -S arch/x86/realmode/rm/realmode.elf | grep text

>   [ 3] .text             PROGBITS        00001000 201000 000f51 00  AX

>  0   0 4096

> ...

> 

> In that case, it seems that maybe I should set the ENTRY in the linker

> script as:

> diff --git a/arch/x86/realmode/rm/realmode.lds.S

> b/arch/x86/realmode/rm/realmode.lds.S

> index 3bb980800c58..64d135d1ee63 100644

> --- a/arch/x86/realmode/rm/realmode.lds.S

> +++ b/arch/x86/realmode/rm/realmode.lds.S

> @@ -11,6 +11,7 @@

> 

>  OUTPUT_FORMAT("elf32-i386")

>  OUTPUT_ARCH(i386)

> +ENTRY(pa_text_start)


Well, looking at arch/x86/boot/setup.ld, it does do:

ENTRY(_start)

for the global _start symbol in .../boot/header.S.

So you doing the respective thing in that linker script would make
sense...

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette
diff mbox series

Patch

diff --git a/arch/x86/realmode/rm/Makefile b/arch/x86/realmode/rm/Makefile
index f60501a384f9..338a00c5257f 100644
--- a/arch/x86/realmode/rm/Makefile
+++ b/arch/x86/realmode/rm/Makefile
@@ -46,7 +46,7 @@  $(obj)/pasyms.h: $(REALMODE_OBJS) FORCE
 targets += realmode.lds
 $(obj)/realmode.lds: $(obj)/pasyms.h
 
-LDFLAGS_realmode.elf := -m elf_i386 --emit-relocs -T
+LDFLAGS_realmode.elf := -m elf_i386 --emit-relocs -e 0x1000 -T
 CPPFLAGS_realmode.lds += -P -C -I$(objtree)/$(obj)
 
 targets += realmode.elf