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

[Peter Smith]

________________________________________
From: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
Sent: 25 September 2019 17:35
To: Borislav Petkov
Cc: H. Peter Anvin; Jarkko Sakkinen; Thomas Gleixner; Ingo Molnar; clang-built-linux; maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT); Tri Vo; Masahiro Yamada; Rob Herring; George Rimar; LKML; Fangrui Song; Peter Smith; Rui Ueyama
Subject: Re: [PATCH v2] x86, realmode: explicitly set entry via command line

+ Fangrui, Peter, Rui, George (LLD)

On Wed, Sep 25, 2019 at 3:20 AM Borislav Petkov <bp@xxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > 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

If I've understood the thread correctly, sorry jumping in late.
- LLD will set the entry point to the start of the .text section in absence of any of the other ways to communicate an entry point. It gives a warning in this case.
- Setting the entry point to an address that is the current start of the .text section silences the warning, but is potentially fragile.

I think LLD is on balance right to give a warning as in many cases the start of the .text section is not going to coincide with the desired entry point.

I recommend doing this via using a symbol defined at the entry point, for example Nick's last suggestion. This will be most resistant to changes such as the .text section changing address or the entry point isn't first in the .text section.

Peter