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

[Lorenzo Pieralisi]

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 at linaro.org> wrote:
> > > On 27 October 2014 21:12, Laura Abbott <lauraa at 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 at chromium.org>
> > >> Signed-off-by: Laura Abbott <lauraa at 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.

Please respin the patch with the above changes, I tested by applying
local changes myself, I will add my tags to the final version.

Thanks,
Lorenzo

> I gave all of that a go locally and my Juno happily booted to userspace
> with cpuidle running. I had to apply Marc's timer fix to get cpuidle to
> work at all with v3.18-rc2, but that should be fixed for -rc3.
> 
> So with those changes:
> 
> Reviewed-by: Mark Rutland <mark.rutland at arm.com>
> Tested-by: Mark Rutland <mark.rutland at arm.com>