From mboxrd@z Thu Jan 1 00:00:00 1970 From: mark.rutland@arm.com (Mark Rutland) Date: Tue, 28 Oct 2014 15:31:18 +0000 Subject: [PATCHv4 3/7] arm64: Move cpu_resume into the text section In-Reply-To: <20141028152656.GB24445@red-moon> References: <1414440752-9411-1-git-send-email-lauraa@codeaurora.org> <1414440752-9411-4-git-send-email-lauraa@codeaurora.org> <20141028124314.GD9796@leverpostej> <20141028152656.GB24445@red-moon> Message-ID: <20141028153118.GL9796@leverpostej> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org 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 wrote: > > > > On 27 October 2014 21:12, Laura Abbott 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 > > > >> Signed-off-by: Laura Abbott > > > >> --- > > > >> 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.