Re: [PATCH] ARM: head-common.S: Clear lr before jumping to start_kernel()

[Nicolas Pitre]

On Sat, 14 Oct 2017, Russell King - ARM Linux wrote:

> On Sat, Oct 14, 2017 at 03:14:05PM +0100, Russell King - ARM Linux wrote:
> > On Thu, Oct 12, 2017 at 11:25:50AM -0400, Nicolas Pitre wrote:
> > > It should go into your devel-testing branch as this must be applied on 
> > > top of my xip_zdata branch that you merged there.
> > 
> > Thanks, it would've been good to have known that ahead of time.
> > 
> > It's why the patch system has the KernelVersion: tag:
> > 
> >  6. Kernel version.
> > On a separate line, add a tag "KernelVersion: " followed by the kernel
> > version that the patch was generated against. This should be formatted
> > as "KernelVersion: 2.6.0-rmk1"
> > 
> > This is because that information is relevant for knowing where it should
> > be applied, and to which branch.  Having it be something else means I
> > have to guess, and that can result in the patch being discarded in this
> > manner if I don't find where it's supposed to be applied.
> > 
> > Yes, I know it's a pain to have to supply this information, but giving
> > accurate information there makes things a lot easier and quicker when
> > applying patches, rather than playing a game of "guess where it needs
> > to be applied, nope, doesn't apply there, try somewhere else."
> > 
> > Various people in the kernel community have different solutions to this.
> > For example, on netdev, it is preferred to state whether you want your
> > patch to be applied to "net" or "net-next" by adding that into the
> > "[PATCH ...]" tag in the subject line.  It's really about streamlining
> > the patch submission and application process.
> 
> The tag can take a plain kernel version and a kernel version suffixed
> with a shortened git hash (please avoid the full hash, it doesn't
> display well with the web presentation, and will probably be truncated
> when it's inserted into the database.)

Probably what you want is the output of 'git describe'.


Nicolas

Re: [PATCH] ARM: head-common.S: Clear lr before jumping to start_kernel()

[Nicolas Pitre]

On Tue, 3 Oct 2017, Geert Uytterhoeven wrote:

> If CONFIG_DEBUG_LOCK_ALLOC=y, the kernel log is spammed with a few
> hundred identical messages:
> 
> unwind: Unknown symbol address c0800300
> unwind: Index not found c0800300
> 
> c0800300 is the return address from the last subroutine call (to
> __memzero()) in __mmap_switched().  Apparently having this address in
> the link register confuses the unwinder.
> 
> To fix this, reset the link register to zero before jumping to
> start_kernel().
> 
> Fixes: 9520b1a1b5f7a348 ("ARM: head-common.S: speed up startup code")
> Suggested-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
> Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be>

Good! This fixes another similar case I had but I didn't come around to 
look at closely yet.

Acked-by: Nicolas Pitre <n...@linaro.org>

Please send it to RMK's patch system.


> ---
> Commit 9520b1a1b5f7a348 is in arm/for-next.
> 
>  arch/arm/kernel/head-common.S | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/arch/arm/kernel/head-common.S b/arch/arm/kernel/head-common.S
> index a25027b87a6024b9..21dde771a7dd8d14 100644
> --- a/arch/arm/kernel/head-common.S
> +++ b/arch/arm/kernel/head-common.S
> @@ -114,6 +114,7 @@ __mmap_switched:
>   str r8, [r2]@ Save atags pointer
>   cmp r3, #0
>   strne   r10, [r3]   @ Save control register values
> + mov lr, #0
>   b   start_kernel
>  ENDPROC(__mmap_switched)
>  
> -- 
> 2.7.4
> 
> 

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

[Nicolas Pitre]

On Fri, 11 Aug 2017, Ard Biesheuvel wrote:

> On 11 August 2017 at 21:06, Nicolas Pitre <nicolas.pi...@linaro.org> wrote:
> > On Fri, 11 Aug 2017, Ard Biesheuvel wrote:
> >
> >> On 11 August 2017 at 20:58, Nicolas Pitre <nicolas.pi...@linaro.org> wrote:
> >> > On Fri, 11 Aug 2017, Ard Biesheuvel wrote:
> >> >
> >> >> On 11 August 2017 at 16:13, Tony Lindgren <t...@atomide.com> wrote:
> >> >> > * Ard Biesheuvel <ard.biesheu...@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.biesheu...@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

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

[Nicolas Pitre]

On Fri, 11 Aug 2017, Ard Biesheuvel wrote:

> On 11 August 2017 at 20:58, Nicolas Pitre <nicolas.pi...@linaro.org> wrote:
> > On Fri, 11 Aug 2017, Ard Biesheuvel wrote:
> >
> >> On 11 August 2017 at 16:13, Tony Lindgren <t...@atomide.com> wrote:
> >> > * Ard Biesheuvel <ard.biesheu...@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.biesheu...@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

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

[Nicolas Pitre]

On Fri, 11 Aug 2017, Ard Biesheuvel wrote:

> On 11 August 2017 at 16:13, Tony Lindgren  wrote:
> > * Ard Biesheuvel  [170805 13:54]:
> >> Replace the open coded PC relative offset calculations with a pair
> >> of adr_l invocations.
> >>
> >> Signed-off-by: Ard Biesheuvel 
> >> ---
> >>  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

Re: [PATCH 01/15] ARM: assembler: introduce adr_l, ldr_l and str_l macros

[Nicolas Pitre]

On Tue, 8 Aug 2017, Ard Biesheuvel wrote:

> On 8 August 2017 at 16:10, Nicolas Pitre <nicolas.pi...@linaro.org> wrote:
> > On Sat, 5 Aug 2017, Ard Biesheuvel wrote:
> >
> >> Like arm64, ARM supports position independent code sequences that
> >> produce symbol references with a greater reach than the ordinary
> >> adr/ldr instructions.
> >>
> >> Currently, we use open coded instruction sequences involving literals
> >> and arithmetic operations. Instead, we can use movw/movt pairs on v7
> >> CPUs, circumventing the D-cache entirely. For older CPUs, we can emit
> >> the literal into a subsection, allowing it to be emitted out of line
> >> while retaining the ability to perform arithmetic on label offsets.
> >>
> >> E.g., on pre-v7 CPUs, we can emit a PC-relative reference as follows:
> >>
> >>ldr  , 222f
> >>   111: add  , , pc
> >>.subsection  1
> >>   222: .long - (111b + 8)
> >>.previous
> >>
> >> This is allowed by the assembler because, unlike ordinary sections,
> >> subsections are combined into a single section into the object file,
> >> and so the label references are not true cross-section references that
> >> are visible as relocations. Note that we could even do something like
> >>
> >>add  , pc, #(222f - 111f) & ~0xfff
> >>ldr  , [, #(222f - 111f) & 0xfff]
> >>   111: add  , , pc
> >>.subsection  1
> >>   222: .long - (111b + 8)
> >>.previous
> >>
> >> if it turns out that the 4 KB range of the ldr instruction is insufficient
> >> to reach the literal in the subsection, although this is currently not a
> >> problem (of the 98 objects built from .S files in a multi_v7_defconfig
> >> build, only 11 have .text sections that are over 1 KB, and the largest one
> >> [entry-armv.o] is 3308 bytes)
> >>
> >> Subsections have been available in binutils since 2004 at least, so
> >> they should not cause any issues with older toolchains.
> >>
> >> So use the above to implement the macros mov_l, adr_l, adrm_l (using ldm
> >> to load multiple literals at once), ldr_l and str_l, all of which will
> >> use movw/movt pairs on v7 and later CPUs, and use PC-relative literals
> >> otherwise.
> >
> > There is no adrm_l definition in this patch.
> >
> 
> Ah yes, I played around with it but it becomes a bit clunky so I removed it:
> 
> adrl , 222f
> ldm  , {, }
>111: add  , , pc
> add  , , pc
> .subsection  1
>222: .long - (111b + 8)
> .long - (111b + 12)
> .previous
> 
> The adrl pseudo op always assembles to two instructions, so you need 5
> instructions while using adr_l twice uses only 4. I am not sure if
> eliminating one of the loads would make a huge difference, given that
> there are no use cases for adrm_l on hot paths, at least not in this
> series.

I'd suggest you keep it to a minimum.  Using adr_l twice is clear and 
obvious.

> > Also, might it be better to change mov_l to movl? Tthis looks similar to
> > the ARM64 movl pseudo-instruction, and unlike all the other _l variants,
> > this is not producing a pc relative result.
> >
> 
> On arm64, we have mov_q for a 64-bit absolute load, and I thought
> mov_l was less confusing than mov_w. In general, I like the underscore
> in the middle because on the one hand, it looks like a ordinary
> mnemonic but on the other hand, it is obvious that it is not a true
> instruction. mov_abs perhaps?
> 
> > Talking about the _l suffix: I wonder if this could be more meaningful,
> > like _rel maybe? At least in the adr_l case, this could easily be
> > confused with adrl.
> >
> 
> On arm64, we have ldr_l, str_l and adr_l as well, and I usually try to
> align between ARM and arm64 if I can.

OK. I'm much less versed into ARM64 assembly so I'll defer to your 
judgment.  It's good if this mnemonic scheme already exists there with 
a similar meaning.


Nicolas

Re: [PATCH 01/15] ARM: assembler: introduce adr_l, ldr_l and str_l macros

[Nicolas Pitre]

On Sat, 5 Aug 2017, Ard Biesheuvel wrote:

> Like arm64, ARM supports position independent code sequences that
> produce symbol references with a greater reach than the ordinary
> adr/ldr instructions.
> 
> Currently, we use open coded instruction sequences involving literals
> and arithmetic operations. Instead, we can use movw/movt pairs on v7
> CPUs, circumventing the D-cache entirely. For older CPUs, we can emit
> the literal into a subsection, allowing it to be emitted out of line
> while retaining the ability to perform arithmetic on label offsets.
> 
> E.g., on pre-v7 CPUs, we can emit a PC-relative reference as follows:
> 
>ldr  , 222f
>   111: add  , , pc
>.subsection  1
>   222: .long - (111b + 8)
>.previous
> 
> This is allowed by the assembler because, unlike ordinary sections,
> subsections are combined into a single section into the object file,
> and so the label references are not true cross-section references that
> are visible as relocations. Note that we could even do something like
> 
>add  , pc, #(222f - 111f) & ~0xfff
>ldr  , [, #(222f - 111f) & 0xfff]
>   111: add  , , pc
>.subsection  1
>   222: .long - (111b + 8)
>.previous
> 
> if it turns out that the 4 KB range of the ldr instruction is insufficient
> to reach the literal in the subsection, although this is currently not a
> problem (of the 98 objects built from .S files in a multi_v7_defconfig
> build, only 11 have .text sections that are over 1 KB, and the largest one
> [entry-armv.o] is 3308 bytes)
> 
> Subsections have been available in binutils since 2004 at least, so
> they should not cause any issues with older toolchains.
> 
> So use the above to implement the macros mov_l, adr_l, adrm_l (using ldm
> to load multiple literals at once), ldr_l and str_l, all of which will
> use movw/movt pairs on v7 and later CPUs, and use PC-relative literals
> otherwise.

There is no adrm_l definition in this patch.

Also, might it be better to change mov_l to movl? Tthis looks similar to 
the ARM64 movl pseudo-instruction, and unlike all the other _l variants, 
this is not producing a pc relative result.

Talking about the _l suffix: I wonder if this could be more meaningful, 
like _rel maybe? At least in the adr_l case, this could easily be 
confused with adrl.

Otherwise I like it pretty much.


> Signed-off-by: Ard Biesheuvel 
> ---
>  arch/arm/include/asm/assembler.h | 70 
>  1 file changed, 70 insertions(+)
> 
> diff --git a/arch/arm/include/asm/assembler.h 
> b/arch/arm/include/asm/assembler.h
> index ad301f107dd2..cedf59a7f853 100644
> --- a/arch/arm/include/asm/assembler.h
> +++ b/arch/arm/include/asm/assembler.h
> @@ -518,4 +518,74 @@ THUMB(   orr \reg , \reg , #PSR_T_BIT)
>  #endif
>   .endm
>  
> +#ifdef CONFIG_THUMB2_KERNEL
> +#define  ARM_PC_BIAS 4
> +#else
> +#define  ARM_PC_BIAS 8
> +#endif
> +
> + .macro  __adldst_l, op, reg, sym, tmp
> + .if __LINUX_ARM_ARCH__ < 7
> + ldr \tmp, 111f
> + .subsection 1
> + .align  2
> +111: .long   \sym - (222f + ARM_PC_BIAS)
> + .previous
> + .else
> + movw\tmp, #:lower16:\sym - (222f + ARM_PC_BIAS)
> + movt\tmp, #:upper16:\sym - (222f + ARM_PC_BIAS)
> + .endif
> +222:;.ifc\op, add
> + add \reg, \tmp, pc
> + .elseif CONFIG_THUMB2_KERNEL == 1
> + add \tmp, \tmp, pc
> + \op \reg, [\tmp]
> + .else
> + \op \reg, [pc, \tmp]
> + .endif
> + .endm
> +
> + /*
> +  * mov_l - move a constant value or [relocated] address into a register
> +  */
> + .macro  mov_l, dst:req, imm:req
> + .if __LINUX_ARM_ARCH__ < 7
> + ldr \dst, =\imm
> + .else
> + movw\dst, #:lower16:\imm
> + movt\dst, #:upper16:\imm
> + .endif
> + .endm
> +
> + /*
> +  * adr_l - adr pseudo-op with unlimited range
> +  *
> +  * @dst: destination register
> +  * @sym: name of the symbol
> +  */
> + .macro  adr_l, dst:req, sym:req
> + __adldst_l  add, \dst, \sym, \dst
> + .endm
> +
> + /*
> +  * ldr_l - ldr  pseudo-op with unlimited range
> +  *
> +  * @dst: destination register
> +  * @sym: name of the symbol
> +  */
> + .macro  ldr_l, dst:req, sym:req
> + __adldst_l  ldr, \dst, \sym, \dst
> + .endm
> +
> + /*
> +  * str_l - str  pseudo-op with unlimited range
> +  *
> +  * @src: source register
> +  * @sym: name of the symbol
> +  * @tmp: mandatory scratch register
> +  */
> + .macro  str_l, src:req, sym:req, tmp:req
> + __adldst_l  str, \src, \sym, \tmp
> + .endm
> +
> 

Re: [PATCH v2 2/2] ARM: shmobile: Remove shmobile_boot_arg

[Nicolas Pitre]

On Wed, 10 Feb 2016, Geert Uytterhoeven wrote:

> CPU boot configuration writes to shmobile_boot_arg, which is located in
> the .text section, and thus should not be written to.
> 
> As of commit 1d33a354bbb618ba ("ARM: shmobile: Per-CPU SMP boot / sleep
> code for SCU SoCs"), and ignoring accidental remainings,
> shmobile_boot_arg is always set to MPIDR_HWID_BITMASK by C code.
> Hence we can just hardcode this in the assembler code, and remove the
> variable, and thus also remove the need to write to this variable.
> 
> Signed-off-by: Geert Uytterhoeven <geert+rene...@glider.be>

Acked-by: Nicolas Pitre <n...@linaro.org>



> ---
> v2:
>   - New.
> ---
>  arch/arm/mach-shmobile/common.h   | 1 -
>  arch/arm/mach-shmobile/headsmp.S  | 8 ++--
>  arch/arm/mach-shmobile/platsmp-apmu.c | 1 -
>  arch/arm/mach-shmobile/platsmp-scu.c  | 1 -
>  4 files changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/arch/arm/mach-shmobile/common.h b/arch/arm/mach-shmobile/common.h
> index 225c12bb3de91e76..5464b7a75e3028a7 100644
> --- a/arch/arm/mach-shmobile/common.h
> +++ b/arch/arm/mach-shmobile/common.h
> @@ -4,7 +4,6 @@
>  extern void shmobile_init_delay(void);
>  extern void shmobile_boot_vector(void);
>  extern unsigned long shmobile_boot_fn;
> -extern unsigned long shmobile_boot_arg;
>  extern unsigned long shmobile_boot_size;
>  extern void shmobile_smp_boot(void);
>  extern void shmobile_smp_sleep(void);
> diff --git a/arch/arm/mach-shmobile/headsmp.S 
> b/arch/arm/mach-shmobile/headsmp.S
> index 4cf7f0b016b0b0fe..ede974b4fd67d8cd 100644
> --- a/arch/arm/mach-shmobile/headsmp.S
> +++ b/arch/arm/mach-shmobile/headsmp.S
> @@ -24,7 +24,6 @@
>   .arm
>   .align  12
>  ENTRY(shmobile_boot_vector)
> - ldr r0, 2f
>   ldr r1, 1f
>   bx  r1
>  
> @@ -34,9 +33,6 @@ ENDPROC(shmobile_boot_vector)
>   .globl  shmobile_boot_fn
>  shmobile_boot_fn:
>  1:   .space  4
> - .globl  shmobile_boot_arg
> -shmobile_boot_arg:
> -2:   .space  4
>   .globl  shmobile_boot_size
>  shmobile_boot_size:
>   .long   . - shmobile_boot_vector
> @@ -46,9 +42,9 @@ shmobile_boot_size:
>   */
>  
>  ENTRY(shmobile_smp_boot)
> - @ r0 = MPIDR_HWID_BITMASK
>   mrc p15, 0, r1, c0, c0, 5   @ r1 = MPIDR
> - and r0, r1, r0  @ r0 = cpu_logical_map() value
> + and r0, r1, #0xff   @ MPIDR_HWID_BITMASK
> + @ r0 = cpu_logical_map() value
>   mov r1, #0  @ r1 = CPU index
>   adr r2, 1f
>   ldmia   r2, {r5, r6, r7}
> diff --git a/arch/arm/mach-shmobile/platsmp-apmu.c 
> b/arch/arm/mach-shmobile/platsmp-apmu.c
> index 911884f7e28b174c..aba75c89f9c1c5eb 100644
> --- a/arch/arm/mach-shmobile/platsmp-apmu.c
> +++ b/arch/arm/mach-shmobile/platsmp-apmu.c
> @@ -123,7 +123,6 @@ void __init shmobile_smp_apmu_prepare_cpus(unsigned int 
> max_cpus,
>  {
>   /* install boot code shared by all CPUs */
>   shmobile_boot_fn = virt_to_phys(shmobile_smp_boot);
> - shmobile_boot_arg = MPIDR_HWID_BITMASK;
>  
>   /* perform per-cpu setup */
>   apmu_parse_cfg(apmu_init_cpu, apmu_config, num);
> diff --git a/arch/arm/mach-shmobile/platsmp-scu.c 
> b/arch/arm/mach-shmobile/platsmp-scu.c
> index 4c7d2caf3730f644..8d478f1da265d55e 100644
> --- a/arch/arm/mach-shmobile/platsmp-scu.c
> +++ b/arch/arm/mach-shmobile/platsmp-scu.c
> @@ -46,7 +46,6 @@ void __init shmobile_smp_scu_prepare_cpus(phys_addr_t 
> scu_base_phys,
>  {
>   /* install boot code shared by all CPUs */
>   shmobile_boot_fn = virt_to_phys(shmobile_smp_boot);
> - shmobile_boot_arg = MPIDR_HWID_BITMASK;
>  
>   /* enable SCU and cache coherency on booting CPU */
>   shmobile_scu_base_phys = scu_base_phys;
> -- 
> 1.9.1
> 
>