Re: [PATCH] arm64/efi: efistub: jump to 'stext' directly, not through the header

[Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>]

On Tue, Jul 15, 2014 at 12:49:07PM +0100, Ard Biesheuvel wrote:
> On 15 July 2014 13:31, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
> >> >> diff --git a/arch/arm64/kernel/efi-entry.S b/arch/arm64/kernel/efi-entry.S
> >> >> index 619b1dd7bcde..6ef541731d9e 100644
> >> >> --- a/arch/arm64/kernel/efi-entry.S
> >> >> +++ b/arch/arm64/kernel/efi-entry.S
> >> >> @@ -61,7 +61,7 @@ ENTRY(efi_stub_entry)
> >> >>        */
> >> >>       mov     x20, x0         // DTB address
> >> >>       ldr     x0, [sp, #16]   // relocated _text address
> >> >> -     mov     x21, x0
> >> >> +     add     x21, x0, #:lo12:stext_offset
> >> >
> >> > I think we can drop the :lo12: here, which will allow us to have a
> >> > warning if stext_offset is unexpectedly large (I believe this will
> >> > currently silently mask bits were that to happen?).
> >> >
> >>
> >> There is no alternative lo12 relocation that errors out when the value
> >> does not fit, so it would have to use a literal instead.
> >
> > Ah, that's a shame. What happens when the value doesn't fit if the
> > linker / assembler don't error out?
> >
> 
> Obviously, it will jump into the wrong place if stext ever gets pushed
> beyond a 4k boundary, which is not that likely (current value is
> 0x160, we may want to up it to 0x200 at some point if we need to
> increase the PE/COFF section alignment, which some versions of the
> (poor) PE/COFF documentation say should be 0x200 at the minimum)
> 
> However, doing ldr x21, =stext_offset works fine as well
> 
> > That sounds like a toolchain bug if they're silently doing the wrong
> > thing.
> >
> 
> Meh, don't think so, really. You get what you pay for, and :lo12: just
> isn't supposed to care. (It's mainly used with adrp to get a +/- 4gb
> PC relative reach)

We appear to have a misunderstanding.

I was suggesting we use:

	add	x21, x0, #stext_offset

... and I thought you meant that wouldn't produce an error if the
immediate was out of range. I understand that :lo12: cuts the top bits
off, and for its intended use that makes sense.

>From testing I see see that gas isn't happy to accept an undefined
symbol as an immediate without :lo12:, and I can see why that makes
sense. My suggestion was bad, sorry.

Thanks,
Mark.