[RFT/RFC PATCH 1/6] ARM: replace PROCINFO embedded branch with relative offset

Ard Biesheuvel ard.biesheuvel at linaro.org
Thu Mar 12 14:00:08 PDT 2015


On 12 March 2015 at 21:50, Russell King - ARM Linux
<linux at arm.linux.org.uk> wrote:
> On Thu, Mar 12, 2015 at 06:38:07PM +0100, Ard Biesheuvel wrote:
>> @@ -138,8 +138,9 @@ ENTRY(stext)
>>                                               @ mmu has been enabled
>>       adr     lr, BSYM(1f)                    @ return (PIC) address
>>       mov     r8, r4                          @ set TTBR1 to swapper_pg_dir
>> - ARM(        add     pc, r10, #PROCINFO_INITFUNC     )
>> - THUMB(      add     r12, r10, #PROCINFO_INITFUNC    )
>> +     ldr     r12, [r10, #PROCINFO_INITFUNC]
>> + ARM(        add     pc, r12, r10                    )
>> + THUMB(      add     r12, r12, r10                   )
>>   THUMB(      ret     r12                             )
>
> Given this change, I'd prefer a slightly different result:
>
>         ldr     r12, [r10, #PROCINFO_INITFUNC]
>         add     r12, r12, r10
>         ret     r12
>

Good idea

>>  1:   b       __enable_mmu
>>  ENDPROC(stext)
>> @@ -386,10 +387,11 @@ ENTRY(secondary_startup)
>>       ldr     r8, [r7, lr]                    @ get secondary_data.swapper_pg_dir
>>       adr     lr, BSYM(__enable_mmu)          @ return address
>>       mov     r13, r12                        @ __secondary_switched address
>> - ARM(        add     pc, r10, #PROCINFO_INITFUNC     ) @ initialise processor
>> -                                               @ (return control reg)
>> - THUMB(      add     r12, r10, #PROCINFO_INITFUNC    )
>> - THUMB(      ret     r12                             )
>> +     ldr     r12, [r10, #PROCINFO_INITFUNC]
>> + ARM(        add     pc, r12, r10            )       @ initialise processor
>> +                                             @ (return control reg)
>> + THUMB(      add     r12, r12, r10           )
>> + THUMB(      ret     r12                     )
>
> and same here.  It means that we have less code to look at, at the expense
> of one additional ARM instruction.
>
>> +
>> +.macro       initfn, initfunc
>> +     .long   \initfunc - . + PROCINFO_INITFUNC
>> +.endm
>
> The more I look at this, the more I find it hard to decide whether this
> is correct or not, and that means it's bad.  It is correct, but it needs
> some thought to confirm that.  I'd prefer a different solution.
>
> The value which we want to place into this location is the difference
> between the start of the procinfo structure and the target symbol.  So
> let's do that - we have a symbol for each procinfo structure, let's
> make "initfn" take that symbol and do the computation using that.
>

I agree. I'll respin with these changes



More information about the linux-arm-kernel mailing list