[05/15] ARM: head: use PC-relative insn sequence for __smp_alt

Message ID 20170805205222.19868-6-ard.biesheuvel@linaro.org
State New
Headers show
Series
  • ARM: add and use convenience macros for PC relative references
Related show

Commit Message

Ard Biesheuvel Aug. 5, 2017, 8:52 p.m.
Replace the open coded PC relative offset calculations with a pair
of adr_l invocations.

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 arch/arm/kernel/head.S | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

-- 
2.11.0

--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Tony Lindgren Aug. 11, 2017, 3:13 p.m. | #1
* Ard Biesheuvel <ard.biesheuvel@linaro.org> [170805 13:54]:
> Replace the open coded PC relative offset calculations with a pair

> of adr_l invocations.

> 

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> ---

>  arch/arm/kernel/head.S | 12 ++----------

>  1 file changed, 2 insertions(+), 10 deletions(-)

> 

> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S

> index 6e9df3663a57..aed341e0f530 100644

> --- a/arch/arm/kernel/head.S

> +++ b/arch/arm/kernel/head.S

> @@ -523,19 +523,11 @@ ARM_BE8(rev	r0, r0)			@ byteswap if big endian

>  	retne	lr

>  

>  __fixup_smp_on_up:

> -	adr	r0, 1f

> -	ldmia	r0, {r3 - r5}

> -	sub	r3, r0, r3

> -	add	r4, r4, r3

> -	add	r5, r5, r3

> +	adr_l	r4, __smpalt_begin

> +	adr_l	r5, __smpalt_end

>  	b	__do_fixup_smp_on_up

>  ENDPROC(__fixup_smp)

>  

> -	.align

> -1:	.word	.

> -	.word	__smpalt_begin

> -	.word	__smpalt_end

> -

>  	.pushsection .data

>  	.globl	smp_on_up

>  smp_on_up:


Ard, it's this one that cause boot to fail on omap3.
The rest of your set works for me with just this one
left out.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ard Biesheuvel Aug. 11, 2017, 7:37 p.m. | #2
On 11 August 2017 at 16:13, Tony Lindgren <tony@atomide.com> wrote:
> * Ard Biesheuvel <ard.biesheuvel@linaro.org> [170805 13:54]:

>> Replace the open coded PC relative offset calculations with a pair

>> of adr_l invocations.

>>

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> ---

>>  arch/arm/kernel/head.S | 12 ++----------

>>  1 file changed, 2 insertions(+), 10 deletions(-)

>>

>> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S

>> index 6e9df3663a57..aed341e0f530 100644

>> --- a/arch/arm/kernel/head.S

>> +++ b/arch/arm/kernel/head.S

>> @@ -523,19 +523,11 @@ ARM_BE8(rev     r0, r0)                 @ byteswap if big endian

>>       retne   lr

>>

>>  __fixup_smp_on_up:

>> -     adr     r0, 1f

>> -     ldmia   r0, {r3 - r5}

>> -     sub     r3, r0, r3

>> -     add     r4, r4, r3

>> -     add     r5, r5, r3

>> +     adr_l   r4, __smpalt_begin

>> +     adr_l   r5, __smpalt_end

>>       b       __do_fixup_smp_on_up

>>  ENDPROC(__fixup_smp)

>>

>> -     .align

>> -1:   .word   .

>> -     .word   __smpalt_begin

>> -     .word   __smpalt_end

>> -

>>       .pushsection .data

>>       .globl  smp_on_up

>>  smp_on_up:

>

> Ard, it's this one that cause boot to fail on omap3.

> The rest of your set works for me with just this one

> left out.

>


I'm completely puzzled. The change is so simple that it is difficult
to reduce it in parts, but if you still have the time to spare, mind
trying the diff below?


        .globl  smp_on_up

The main point of these changes is to eliminate absolute references,
not to use the new macros, so if this does work, I will go with this
instead.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.htmldiff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 6e9df3663a57..c0361e608210 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -524,17 +524,15 @@ ARM_BE8(rev       r0, r0)                 @
byteswap if big endian

 __fixup_smp_on_up:
        adr     r0, 1f
-       ldmia   r0, {r3 - r5}
-       sub     r3, r0, r3
-       add     r4, r4, r3
-       add     r5, r5, r3
+       ldmia   r0, {r4 - r5}
+       add     r4, r4, r0
+       add     r5, r5, r0
        b       __do_fixup_smp_on_up
 ENDPROC(__fixup_smp)

        .align
-1:     .word   .
-       .word   __smpalt_begin
-       .word   __smpalt_end
+1:     .word   __smpalt_begin - 1b
+       .word   __smpalt_end - 1b

        .pushsection .data

Nicolas Pitre Aug. 11, 2017, 7:58 p.m. | #3
On Fri, 11 Aug 2017, Ard Biesheuvel wrote:

> On 11 August 2017 at 16:13, Tony Lindgren <tony@atomide.com> wrote:

> > * Ard Biesheuvel <ard.biesheuvel@linaro.org> [170805 13:54]:

> >> Replace the open coded PC relative offset calculations with a pair

> >> of adr_l invocations.

> >>

> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> >> ---

> >>  arch/arm/kernel/head.S | 12 ++----------

> >>  1 file changed, 2 insertions(+), 10 deletions(-)

> >>

> >> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S

> >> index 6e9df3663a57..aed341e0f530 100644

> >> --- a/arch/arm/kernel/head.S

> >> +++ b/arch/arm/kernel/head.S

> >> @@ -523,19 +523,11 @@ ARM_BE8(rev     r0, r0)                 @ byteswap if big endian

> >>       retne   lr

> >>

> >>  __fixup_smp_on_up:

> >> -     adr     r0, 1f

> >> -     ldmia   r0, {r3 - r5}

> >> -     sub     r3, r0, r3

> >> -     add     r4, r4, r3

> >> -     add     r5, r5, r3

> >> +     adr_l   r4, __smpalt_begin

> >> +     adr_l   r5, __smpalt_end

> >>       b       __do_fixup_smp_on_up

> >>  ENDPROC(__fixup_smp)

> >>

> >> -     .align

> >> -1:   .word   .

> >> -     .word   __smpalt_begin

> >> -     .word   __smpalt_end

> >> -

> >>       .pushsection .data

> >>       .globl  smp_on_up

> >>  smp_on_up:

> >

> > Ard, it's this one that cause boot to fail on omap3.

> > The rest of your set works for me with just this one

> > left out.

> >

> 

> I'm completely puzzled.


Found it.

You replaced:

-       adr     r0, 1f
-       ldmia   r0, {r3 - r5}
-       sub     r3, r0, r3
-       add     r4, r4, r3
-       add     r5, r5, r3
+       adr_l   r4, __smpalt_begin
+       adr_l   r5, __smpalt_end
        b       __do_fixup_smp_on_up

Notice that r3 is now uninitialized.

Now have a look at the code for __do_fixup_smp_on_up.


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ard Biesheuvel Aug. 11, 2017, 8:01 p.m. | #4
On 11 August 2017 at 20:58, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> On Fri, 11 Aug 2017, Ard Biesheuvel wrote:

>

>> On 11 August 2017 at 16:13, Tony Lindgren <tony@atomide.com> wrote:

>> > * Ard Biesheuvel <ard.biesheuvel@linaro.org> [170805 13:54]:

>> >> Replace the open coded PC relative offset calculations with a pair

>> >> of adr_l invocations.

>> >>

>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> >> ---

>> >>  arch/arm/kernel/head.S | 12 ++----------

>> >>  1 file changed, 2 insertions(+), 10 deletions(-)

>> >>

>> >> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S

>> >> index 6e9df3663a57..aed341e0f530 100644

>> >> --- a/arch/arm/kernel/head.S

>> >> +++ b/arch/arm/kernel/head.S

>> >> @@ -523,19 +523,11 @@ ARM_BE8(rev     r0, r0)                 @ byteswap if big endian

>> >>       retne   lr

>> >>

>> >>  __fixup_smp_on_up:

>> >> -     adr     r0, 1f

>> >> -     ldmia   r0, {r3 - r5}

>> >> -     sub     r3, r0, r3

>> >> -     add     r4, r4, r3

>> >> -     add     r5, r5, r3

>> >> +     adr_l   r4, __smpalt_begin

>> >> +     adr_l   r5, __smpalt_end

>> >>       b       __do_fixup_smp_on_up

>> >>  ENDPROC(__fixup_smp)

>> >>

>> >> -     .align

>> >> -1:   .word   .

>> >> -     .word   __smpalt_begin

>> >> -     .word   __smpalt_end

>> >> -

>> >>       .pushsection .data

>> >>       .globl  smp_on_up

>> >>  smp_on_up:

>> >

>> > Ard, it's this one that cause boot to fail on omap3.

>> > The rest of your set works for me with just this one

>> > left out.

>> >

>>

>> I'm completely puzzled.

>

> Found it.

>

> You replaced:

>

> -       adr     r0, 1f

> -       ldmia   r0, {r3 - r5}

> -       sub     r3, r0, r3

> -       add     r4, r4, r3

> -       add     r5, r5, r3

> +       adr_l   r4, __smpalt_begin

> +       adr_l   r5, __smpalt_end

>         b       __do_fixup_smp_on_up

>

> Notice that r3 is now uninitialized.

>

> Now have a look at the code for __do_fixup_smp_on_up.

>


I still don't see it :-)

__do_fixup_smp_on_up() mentions r3 in the comment block, but it does
not actually refer to it in the code, does it?
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nicolas Pitre Aug. 11, 2017, 8:06 p.m. | #5
On Fri, 11 Aug 2017, Ard Biesheuvel wrote:

> On 11 August 2017 at 20:58, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:

> > On Fri, 11 Aug 2017, Ard Biesheuvel wrote:

> >

> >> On 11 August 2017 at 16:13, Tony Lindgren <tony@atomide.com> wrote:

> >> > * Ard Biesheuvel <ard.biesheuvel@linaro.org> [170805 13:54]:

> >> >> Replace the open coded PC relative offset calculations with a pair

> >> >> of adr_l invocations.

> >> >>

> >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> >> >> ---

> >> >>  arch/arm/kernel/head.S | 12 ++----------

> >> >>  1 file changed, 2 insertions(+), 10 deletions(-)

> >> >>

> >> >> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S

> >> >> index 6e9df3663a57..aed341e0f530 100644

> >> >> --- a/arch/arm/kernel/head.S

> >> >> +++ b/arch/arm/kernel/head.S

> >> >> @@ -523,19 +523,11 @@ ARM_BE8(rev     r0, r0)                 @ byteswap if big endian

> >> >>       retne   lr

> >> >>

> >> >>  __fixup_smp_on_up:

> >> >> -     adr     r0, 1f

> >> >> -     ldmia   r0, {r3 - r5}

> >> >> -     sub     r3, r0, r3

> >> >> -     add     r4, r4, r3

> >> >> -     add     r5, r5, r3

> >> >> +     adr_l   r4, __smpalt_begin

> >> >> +     adr_l   r5, __smpalt_end

> >> >>       b       __do_fixup_smp_on_up

> >> >>  ENDPROC(__fixup_smp)

> >> >>

> >> >> -     .align

> >> >> -1:   .word   .

> >> >> -     .word   __smpalt_begin

> >> >> -     .word   __smpalt_end

> >> >> -

> >> >>       .pushsection .data

> >> >>       .globl  smp_on_up

> >> >>  smp_on_up:

> >> >

> >> > Ard, it's this one that cause boot to fail on omap3.

> >> > The rest of your set works for me with just this one

> >> > left out.

> >> >

> >>

> >> I'm completely puzzled.

> >

> > Found it.

> >

> > You replaced:

> >

> > -       adr     r0, 1f

> > -       ldmia   r0, {r3 - r5}

> > -       sub     r3, r0, r3

> > -       add     r4, r4, r3

> > -       add     r5, r5, r3

> > +       adr_l   r4, __smpalt_begin

> > +       adr_l   r5, __smpalt_end

> >         b       __do_fixup_smp_on_up

> >

> > Notice that r3 is now uninitialized.

> >

> > Now have a look at the code for __do_fixup_smp_on_up.

> >

> 

> I still don't see it :-)

> 

> __do_fixup_smp_on_up() mentions r3 in the comment block, but it does

> not actually refer to it in the code, does it?


__do_fixup_smp_on_up:
        cmp     r4, r5
        reths   lr
        ldmia   r4!, {r0, r6}
 ARM(   str     r6, [r0, r3]    )	<<============
 THUMB( add     r0, r0, r3      )	<<============
[...]


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ard Biesheuvel Aug. 11, 2017, 8:07 p.m. | #6
On 11 August 2017 at 21:06, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:
> On Fri, 11 Aug 2017, Ard Biesheuvel wrote:

>

>> On 11 August 2017 at 20:58, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:

>> > On Fri, 11 Aug 2017, Ard Biesheuvel wrote:

>> >

>> >> On 11 August 2017 at 16:13, Tony Lindgren <tony@atomide.com> wrote:

>> >> > * Ard Biesheuvel <ard.biesheuvel@linaro.org> [170805 13:54]:

>> >> >> Replace the open coded PC relative offset calculations with a pair

>> >> >> of adr_l invocations.

>> >> >>

>> >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> >> >> ---

>> >> >>  arch/arm/kernel/head.S | 12 ++----------

>> >> >>  1 file changed, 2 insertions(+), 10 deletions(-)

>> >> >>

>> >> >> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S

>> >> >> index 6e9df3663a57..aed341e0f530 100644

>> >> >> --- a/arch/arm/kernel/head.S

>> >> >> +++ b/arch/arm/kernel/head.S

>> >> >> @@ -523,19 +523,11 @@ ARM_BE8(rev     r0, r0)                 @ byteswap if big endian

>> >> >>       retne   lr

>> >> >>

>> >> >>  __fixup_smp_on_up:

>> >> >> -     adr     r0, 1f

>> >> >> -     ldmia   r0, {r3 - r5}

>> >> >> -     sub     r3, r0, r3

>> >> >> -     add     r4, r4, r3

>> >> >> -     add     r5, r5, r3

>> >> >> +     adr_l   r4, __smpalt_begin

>> >> >> +     adr_l   r5, __smpalt_end

>> >> >>       b       __do_fixup_smp_on_up

>> >> >>  ENDPROC(__fixup_smp)

>> >> >>

>> >> >> -     .align

>> >> >> -1:   .word   .

>> >> >> -     .word   __smpalt_begin

>> >> >> -     .word   __smpalt_end

>> >> >> -

>> >> >>       .pushsection .data

>> >> >>       .globl  smp_on_up

>> >> >>  smp_on_up:

>> >> >

>> >> > Ard, it's this one that cause boot to fail on omap3.

>> >> > The rest of your set works for me with just this one

>> >> > left out.

>> >> >

>> >>

>> >> I'm completely puzzled.

>> >

>> > Found it.

>> >

>> > You replaced:

>> >

>> > -       adr     r0, 1f

>> > -       ldmia   r0, {r3 - r5}

>> > -       sub     r3, r0, r3

>> > -       add     r4, r4, r3

>> > -       add     r5, r5, r3

>> > +       adr_l   r4, __smpalt_begin

>> > +       adr_l   r5, __smpalt_end

>> >         b       __do_fixup_smp_on_up

>> >

>> > Notice that r3 is now uninitialized.

>> >

>> > Now have a look at the code for __do_fixup_smp_on_up.

>> >

>>

>> I still don't see it :-)

>>

>> __do_fixup_smp_on_up() mentions r3 in the comment block, but it does

>> not actually refer to it in the code, does it?

>

> __do_fixup_smp_on_up:

>         cmp     r4, r5

>         reths   lr

>         ldmia   r4!, {r0, r6}

>  ARM(   str     r6, [r0, r3]    )       <<============

>  THUMB( add     r0, r0, r3      )       <<============

> [...]

>


Aahhh

Looking at the wrong version of the file.

Thanks for spotting that.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Nicolas Pitre Aug. 11, 2017, 8:12 p.m. | #7
On Fri, 11 Aug 2017, Ard Biesheuvel wrote:

> On 11 August 2017 at 21:06, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:

> > On Fri, 11 Aug 2017, Ard Biesheuvel wrote:

> >

> >> On 11 August 2017 at 20:58, Nicolas Pitre <nicolas.pitre@linaro.org> wrote:

> >> > On Fri, 11 Aug 2017, Ard Biesheuvel wrote:

> >> >

> >> >> On 11 August 2017 at 16:13, Tony Lindgren <tony@atomide.com> wrote:

> >> >> > * Ard Biesheuvel <ard.biesheuvel@linaro.org> [170805 13:54]:

> >> >> >> Replace the open coded PC relative offset calculations with a pair

> >> >> >> of adr_l invocations.

> >> >> >>

> >> >> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> >> >> >> ---

> >> >> >>  arch/arm/kernel/head.S | 12 ++----------

> >> >> >>  1 file changed, 2 insertions(+), 10 deletions(-)

> >> >> >>

> >> >> >> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S

> >> >> >> index 6e9df3663a57..aed341e0f530 100644

> >> >> >> --- a/arch/arm/kernel/head.S

> >> >> >> +++ b/arch/arm/kernel/head.S

> >> >> >> @@ -523,19 +523,11 @@ ARM_BE8(rev     r0, r0)                 @ byteswap if big endian

> >> >> >>       retne   lr

> >> >> >>

> >> >> >>  __fixup_smp_on_up:

> >> >> >> -     adr     r0, 1f

> >> >> >> -     ldmia   r0, {r3 - r5}

> >> >> >> -     sub     r3, r0, r3

> >> >> >> -     add     r4, r4, r3

> >> >> >> -     add     r5, r5, r3

> >> >> >> +     adr_l   r4, __smpalt_begin

> >> >> >> +     adr_l   r5, __smpalt_end

> >> >> >>       b       __do_fixup_smp_on_up

> >> >> >>  ENDPROC(__fixup_smp)

> >> >> >>

> >> >> >> -     .align

> >> >> >> -1:   .word   .

> >> >> >> -     .word   __smpalt_begin

> >> >> >> -     .word   __smpalt_end

> >> >> >> -

> >> >> >>       .pushsection .data

> >> >> >>       .globl  smp_on_up

> >> >> >>  smp_on_up:

> >> >> >

> >> >> > Ard, it's this one that cause boot to fail on omap3.

> >> >> > The rest of your set works for me with just this one

> >> >> > left out.

> >> >> >

> >> >>

> >> >> I'm completely puzzled.

> >> >

> >> > Found it.

> >> >

> >> > You replaced:

> >> >

> >> > -       adr     r0, 1f

> >> > -       ldmia   r0, {r3 - r5}

> >> > -       sub     r3, r0, r3

> >> > -       add     r4, r4, r3

> >> > -       add     r5, r5, r3

> >> > +       adr_l   r4, __smpalt_begin

> >> > +       adr_l   r5, __smpalt_end

> >> >         b       __do_fixup_smp_on_up

> >> >

> >> > Notice that r3 is now uninitialized.

> >> >

> >> > Now have a look at the code for __do_fixup_smp_on_up.

> >> >

> >>

> >> I still don't see it :-)

> >>

> >> __do_fixup_smp_on_up() mentions r3 in the comment block, but it does

> >> not actually refer to it in the code, does it?

> >

> > __do_fixup_smp_on_up:

> >         cmp     r4, r5

> >         reths   lr

> >         ldmia   r4!, {r0, r6}

> >  ARM(   str     r6, [r0, r3]    )       <<============

> >  THUMB( add     r0, r0, r3      )       <<============

> > [...]

> >

> 

> Aahhh

> 

> Looking at the wrong version of the file.

> 

> Thanks for spotting that.


No problem. Can't attribute that to my sharp eyes though.  :-)


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Tony Lindgren Aug. 14, 2017, 4:19 p.m. | #8
* Ard Biesheuvel <ard.biesheuvel@linaro.org> [170811 12:37]:
> On 11 August 2017 at 16:13, Tony Lindgren <tony@atomide.com> wrote:

> > * Ard Biesheuvel <ard.biesheuvel@linaro.org> [170805 13:54]:

> >> Replace the open coded PC relative offset calculations with a pair

> >> of adr_l invocations.

> >>

> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> >> ---

> >>  arch/arm/kernel/head.S | 12 ++----------

> >>  1 file changed, 2 insertions(+), 10 deletions(-)

> >>

> >> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S

> >> index 6e9df3663a57..aed341e0f530 100644

> >> --- a/arch/arm/kernel/head.S

> >> +++ b/arch/arm/kernel/head.S

> >> @@ -523,19 +523,11 @@ ARM_BE8(rev     r0, r0)                 @ byteswap if big endian

> >>       retne   lr

> >>

> >>  __fixup_smp_on_up:

> >> -     adr     r0, 1f

> >> -     ldmia   r0, {r3 - r5}

> >> -     sub     r3, r0, r3

> >> -     add     r4, r4, r3

> >> -     add     r5, r5, r3

> >> +     adr_l   r4, __smpalt_begin

> >> +     adr_l   r5, __smpalt_end

> >>       b       __do_fixup_smp_on_up

> >>  ENDPROC(__fixup_smp)

> >>

> >> -     .align

> >> -1:   .word   .

> >> -     .word   __smpalt_begin

> >> -     .word   __smpalt_end

> >> -

> >>       .pushsection .data

> >>       .globl  smp_on_up

> >>  smp_on_up:

> >

> > Ard, it's this one that cause boot to fail on omap3.

> > The rest of your set works for me with just this one

> > left out.

> >

> 

> I'm completely puzzled. The change is so simple that it is difficult

> to reduce it in parts, but if you still have the time to spare, mind

> trying the diff below?

> 

> 

> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S

> index 6e9df3663a57..c0361e608210 100644

> --- a/arch/arm/kernel/head.S

> +++ b/arch/arm/kernel/head.S

> @@ -524,17 +524,15 @@ ARM_BE8(rev       r0, r0)                 @

> byteswap if big endian

> 

>  __fixup_smp_on_up:

>         adr     r0, 1f

> -       ldmia   r0, {r3 - r5}

> -       sub     r3, r0, r3

> -       add     r4, r4, r3

> -       add     r5, r5, r3

> +       ldmia   r0, {r4 - r5}

> +       add     r4, r4, r0

> +       add     r5, r5, r0

>         b       __do_fixup_smp_on_up

>  ENDPROC(__fixup_smp)

> 

>         .align

> -1:     .word   .

> -       .word   __smpalt_begin

> -       .word   __smpalt_end

> +1:     .word   __smpalt_begin - 1b

> +       .word   __smpalt_end - 1b

> 

>         .pushsection .data

>         .globl  smp_on_up

> 

> The main point of these changes is to eliminate absolute references,

> not to use the new macros, so if this does work, I will go with this

> instead.


For reference, I manually did this changes as it did not apply
after reverting your earlier version of this patch. No luck with
this change, so I'll test again when you you have updated patches
available.

Regards,

Tony
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ard Biesheuvel Aug. 14, 2017, 4:20 p.m. | #9
On 14 August 2017 at 17:19, Tony Lindgren <tony@atomide.com> wrote:
> * Ard Biesheuvel <ard.biesheuvel@linaro.org> [170811 12:37]:

>> On 11 August 2017 at 16:13, Tony Lindgren <tony@atomide.com> wrote:

>> > * Ard Biesheuvel <ard.biesheuvel@linaro.org> [170805 13:54]:

>> >> Replace the open coded PC relative offset calculations with a pair

>> >> of adr_l invocations.

>> >>

>> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>> >> ---

>> >>  arch/arm/kernel/head.S | 12 ++----------

>> >>  1 file changed, 2 insertions(+), 10 deletions(-)

>> >>

>> >> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S

>> >> index 6e9df3663a57..aed341e0f530 100644

>> >> --- a/arch/arm/kernel/head.S

>> >> +++ b/arch/arm/kernel/head.S

>> >> @@ -523,19 +523,11 @@ ARM_BE8(rev     r0, r0)                 @ byteswap if big endian

>> >>       retne   lr

>> >>

>> >>  __fixup_smp_on_up:

>> >> -     adr     r0, 1f

>> >> -     ldmia   r0, {r3 - r5}

>> >> -     sub     r3, r0, r3

>> >> -     add     r4, r4, r3

>> >> -     add     r5, r5, r3

>> >> +     adr_l   r4, __smpalt_begin

>> >> +     adr_l   r5, __smpalt_end

>> >>       b       __do_fixup_smp_on_up

>> >>  ENDPROC(__fixup_smp)

>> >>

>> >> -     .align

>> >> -1:   .word   .

>> >> -     .word   __smpalt_begin

>> >> -     .word   __smpalt_end

>> >> -

>> >>       .pushsection .data

>> >>       .globl  smp_on_up

>> >>  smp_on_up:

>> >

>> > Ard, it's this one that cause boot to fail on omap3.

>> > The rest of your set works for me with just this one

>> > left out.

>> >

>>

>> I'm completely puzzled. The change is so simple that it is difficult

>> to reduce it in parts, but if you still have the time to spare, mind

>> trying the diff below?

>>

>>

>> diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S

>> index 6e9df3663a57..c0361e608210 100644

>> --- a/arch/arm/kernel/head.S

>> +++ b/arch/arm/kernel/head.S

>> @@ -524,17 +524,15 @@ ARM_BE8(rev       r0, r0)                 @

>> byteswap if big endian

>>

>>  __fixup_smp_on_up:

>>         adr     r0, 1f

>> -       ldmia   r0, {r3 - r5}

>> -       sub     r3, r0, r3

>> -       add     r4, r4, r3

>> -       add     r5, r5, r3

>> +       ldmia   r0, {r4 - r5}

>> +       add     r4, r4, r0

>> +       add     r5, r5, r0

>>         b       __do_fixup_smp_on_up

>>  ENDPROC(__fixup_smp)

>>

>>         .align

>> -1:     .word   .

>> -       .word   __smpalt_begin

>> -       .word   __smpalt_end

>> +1:     .word   __smpalt_begin - 1b

>> +       .word   __smpalt_end - 1b

>>

>>         .pushsection .data

>>         .globl  smp_on_up

>>

>> The main point of these changes is to eliminate absolute references,

>> not to use the new macros, so if this does work, I will go with this

>> instead.

>

> For reference, I manually did this changes as it did not apply

> after reverting your earlier version of this patch. No luck with

> this change, so I'll test again when you you have updated patches

> available.

>


Thanks Tony,

But Nico already found the issue: I was looking at the code with
another patch on top, which removed the reference to r3 in the
followup code.

In the KASLR series I just posted, I reordered that patch with this
one, so everything should be good.
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch hide | download patch | download mbox

diff --git a/arch/arm/kernel/head.S b/arch/arm/kernel/head.S
index 6e9df3663a57..aed341e0f530 100644
--- a/arch/arm/kernel/head.S
+++ b/arch/arm/kernel/head.S
@@ -523,19 +523,11 @@  ARM_BE8(rev	r0, r0)			@ byteswap if big endian
 	retne	lr
 
 __fixup_smp_on_up:
-	adr	r0, 1f
-	ldmia	r0, {r3 - r5}
-	sub	r3, r0, r3
-	add	r4, r4, r3
-	add	r5, r5, r3
+	adr_l	r4, __smpalt_begin
+	adr_l	r5, __smpalt_end
 	b	__do_fixup_smp_on_up
 ENDPROC(__fixup_smp)
 
-	.align
-1:	.word	.
-	.word	__smpalt_begin
-	.word	__smpalt_end
-
 	.pushsection .data
 	.globl	smp_on_up
 smp_on_up: