From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756307AbcHWJka (ORCPT ); Tue, 23 Aug 2016 05:40:30 -0400 Received: from foss.arm.com ([217.140.101.70]:35106 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754825AbcHWJju (ORCPT ); Tue, 23 Aug 2016 05:39:50 -0400 Subject: Re: [PATCH 5/8] arm64: alternative: Add support for patching adrp instructions To: Will Deacon References: <1471525832-21209-1-git-send-email-suzuki.poulose@arm.com> <1471525832-21209-6-git-send-email-suzuki.poulose@arm.com> <20160822111921.GC14680@arm.com> Cc: linux-arm-kernel@lists.infradead.org, linux-kernel@vger.kernel.org, catalin.marinas@arm.com, mark.rutland@arm.com, andre.przywara@arm.com, Marc Zyngier From: Suzuki K Poulose Message-ID: <74075792-8e10-2b6f-7df2-47c0f9e91fcf@arm.com> Date: Tue, 23 Aug 2016 10:39:01 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20160822111921.GC14680@arm.com> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 22/08/16 12:19, Will Deacon wrote: > On Thu, Aug 18, 2016 at 02:10:29PM +0100, Suzuki K Poulose wrote: >> adrp uses PC-relative address offset to a page (of 4K size) of >> a symbol. If it appears in an alternative code patched in, we >> should adjust the offset to reflect the address where it will >> be run from. This patch adds support for fixing the offset >> for adrp instructions. >> >> Cc: Will Deacon >> Cc: Marc Zyngier >> Cc: Andre Przywara >> Cc: Mark Rutland >> Signed-off-by: Suzuki K Poulose >> --- >> arch/arm64/kernel/alternative.c | 13 +++++++++++++ >> 1 file changed, 13 insertions(+) >> >> diff --git a/arch/arm64/kernel/alternative.c b/arch/arm64/kernel/alternative.c >> index d2ee1b2..71c6962 100644 >> --- a/arch/arm64/kernel/alternative.c >> +++ b/arch/arm64/kernel/alternative.c >> @@ -80,6 +80,19 @@ static u32 get_alt_insn(struct alt_instr *alt, u32 *insnptr, u32 *altinsnptr) >> offset = target - (unsigned long)insnptr; >> insn = aarch64_set_branch_offset(insn, offset); >> } >> + } else if (aarch64_insn_is_adrp(insn)) { >> + s32 orig_offset, new_offset; >> + unsigned long target; >> + >> + /* >> + * If we're replacing an adrp instruction, which uses PC-relative >> + * immediate addressing, adjust the offset to reflect the new >> + * PC. adrp operates on 4K aligned addresses. >> + */ >> + orig_offset = aarch64_insn_adrp_get_offset(insn); >> + target = ((unsigned long)altinsnptr & ~0xfffUL) + orig_offset; >> + new_offset = target - ((unsigned long)insnptr & ~0xfffUL); > > The masking with ~0xfffUL might be nicer if you write it as > align_down(ptr, SZ_4K); Right, that definitely looks better. Will change it. > >> + insn = aarch64_insn_adrp_set_offset(insn, new_offset); >> } >> >> return insn; > > I wonder if we shouldn't have a catch-all for any instructions performing > PC-relative operations here, because silent corruption of the instruction Correct, which is what happened initially when I didn't have the adrp handling ;-). > stream is pretty horrible. What other instructions are there? ADR, LDR > (literal), ... ? From a quick look, all the instructions under "Load register (literal)" : i.e, LDR (literal) for GPR/FP_SIMD/32bit/64bit LDRSW (literal) PRFM (literal) and Data processing instructions - immediate group with PC-relative addressing: ADR, ADRP I will add a check to catch the unsupported instructions in the alternative code. Thanks Suzuki