[PATCHv4 3/7] arm64: Move cpu_resume into the text section

[mark.rutland@arm.com (Mark Rutland)]

On Tue, Oct 28, 2014 at 03:26:56PM +0000, Lorenzo Pieralisi wrote:
> On Tue, Oct 28, 2014 at 12:43:15PM +0000, Mark Rutland wrote:
> > On Tue, Oct 28, 2014 at 08:22:00AM +0000, Ard Biesheuvel wrote:
> > > On 28 October 2014 09:10, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > > > On 27 October 2014 21:12, Laura Abbott <lauraa@codeaurora.org> wrote:
> > > >> The function cpu_resume currently lives in the .data
> > > >> section. There's no reason for it to be there since
> > > >> we can use relative instructions without a problem.
> > > >> Move it to the text section where it belongs.
> > > >>
> > > >> Reviewed-by: Kees Cook <keescook@chromium.org>
> > > >> Signed-off-by: Laura Abbott <lauraa@codeaurora.org>
> > > >> ---
> > > >>  arch/arm64/kernel/sleep.S | 12 +++++++-----
> > > >>  1 file changed, 7 insertions(+), 5 deletions(-)
> > > >>
> > > >
> > > > Hi Laura,
> > > >
> > > > Apologies for waiting until v4 to bring this up, but I have some minor comments.
> > > >
> > > >> diff --git a/arch/arm64/kernel/sleep.S b/arch/arm64/kernel/sleep.S
> > > >> index a564b44..5762b16 100644
> > > >> --- a/arch/arm64/kernel/sleep.S
> > > >> +++ b/arch/arm64/kernel/sleep.S
> > > >> @@ -147,12 +147,12 @@ cpu_resume_after_mmu:
> > > >>         ret
> > > >>  ENDPROC(cpu_resume_after_mmu)
> > > >>
> > > >> -       .data
> > > >>  ENTRY(cpu_resume)
> > > >>         bl      el2_setup               // if in EL2 drop to EL1 cleanly
> > > >>  #ifdef CONFIG_SMP
> > > >>         mrs     x1, mpidr_el1
> > > >> -       adr     x4, mpidr_hash_ptr
> > > >> +       adrp    x4, mpidr_hash_ptr
> > > >> +       add     x4, x4, #:lo12:mpidr_hash_ptr
> > > >
> > > > Instead of this change, you could put mpidr_hash_ptr itself in .text as well.
> > > >
> > > 
> > > Actually, looking more closely, it appears mpidr_hash_ptr can be
> > > dropped completely:
> > > the address obtained by adrp/add is PC relative, so the whole sequence
> > > could just be
> > > 
> > > adrp x8, mpidr_hash
> > > add x8, x8, #:lo12:mpidr_hash // x8 = struct mpidr_hash phys address
> > > 
> > > (and the ldr and add below can be dropped, and so can mpidr_hash_ptr at the end)
> > 
> > Also, if you update arch/arm64/kernel/suspend.c to drop the extern from
> > sleep_save_sp and sleep_idmap_phys, you can drop all of the .data from
> > sleep.S
> 
> If with that you also mean that all adr references to those data structures
> in the resume path should become adrp I agree otherwise we might have an
> offset issue.

Yes. The original patch had those modifications already, so I assumed
they would remain.

Mark.