Re: [RFC v5 00/57] objtool: Add support for arm64

From: Julien Thierry <jthierry@redhat.com>
To: Will Deacon <will@kernel.org>
Cc: peterz@infradead.org, catalin.marinas@arm.com,
	linux-kernel@vger.kernel.org, raphael.gault@arm.com,
	jpoimboe@redhat.com, linux-arm-kernel@lists.infradead.org
Subject: Re: [RFC v5 00/57] objtool: Add support for arm64
Date: Thu, 23 Jan 2020 15:11:20 +0000	[thread overview]
Message-ID: <ad6f3a12-60b9-f2d8-b0c9-0de59c0e0c2c@redhat.com> (raw)
In-Reply-To: <20200123143503.GA19649@willie-the-truck>

On 1/23/20 2:35 PM, Will Deacon wrote:
> On Thu, Jan 23, 2020 at 01:52:17PM +0000, Julien Thierry wrote:
>>
>>
>> On 1/21/20 10:30 AM, Will Deacon wrote:
>>> On Thu, Jan 09, 2020 at 04:02:03PM +0000, Julien Thierry wrote:
>>>> This patch series is the continuation of Raphael's work [1]. All the
>>>> patches can be retrieved from:
>>>> git clone -b arm64-objtool-v5 https://github.com/julien-thierry/linux.git
>>>
>>> [...]
>>>
>>>>     objtool: arm64: Decode unknown instructions
>>>>     objtool: arm64: Decode simple data processing instructions
>>>>     objtool: arm64: Decode add/sub immediate instructions
>>>>     objtool: arm64: Decode logical data processing instructions
>>>>     objtool: arm64: Decode system instructions not affecting the flow
>>>>     objtool: arm64: Decode calls to higher EL
>>>>     objtool: arm64: Decode brk instruction
>>>>     objtool: arm64: Decode instruction triggering context switch
>>>>     objtool: arm64: Decode branch instructions with PC relative immediates
>>>>     objtool: arm64: Decode branch to register instruction
>>>>     objtool: arm64: Decode basic load/stores
>>>>     objtool: arm64: Decode load/store with register offset
>>>>     objtool: arm64: Decode load/store register pair instructions
>>>>     objtool: arm64: Decode FP/SIMD load/store instructions
>>>>     objtool: arm64: Decode load/store exclusive
>>>>     objtool: arm64: Decode atomic load/store
>>>>     objtool: arm64: Decode pointer auth load instructions
>>>>     objtool: arm64: Decode load acquire/store release
>>>>     objtool: arm64: Decode load/store with memory tag
>>>>     objtool: arm64: Decode load literal
>>>>     objtool: arm64: Decode register data processing instructions
>>>>     objtool: arm64: Decode FP/SIMD data processing instructions
>>>>     objtool: arm64: Decode SVE instructions
>>>
>>> That's a lot of decoding logic which we already have in
>>> arch/arm64/{kernel/insn.c,include/asm/insn.h}. I'd prefer to see this stuff
>>> reused or generated from a single source, since it's really easy to get it
>>> wrong, has a tendency to bitrot and is nasty to debug.
>>>
>>
>> The thing is that the code in those files is mostly encoding logic
>> (motivated by BPF) rather than decoding (except for the instruction that
>> might be trapped, but these rarely overlap with instructions that objtools
>> cares about). I agree that ideally the decoding/encoding should be under
>> arch/arm64/lib, I was just a bit weary introducing a lot of decoding code
>> under arch/arm64 that wouldn't even be used in kernel code.
> 
> Hmm, but kprobes decodes instructions somehow :p
> 
> Not saying you have to refactor everything, but I'd hope you could reuse
> some of the aarch64_insn_is* and aarch64_insn_extract* functions at least.
> 

Oh, you're right, there seem to be more than what I remembered. There is 
probably a bunch of things I can reuse (and I'll feel more at ease with 
that rather than introducing a whole bunch of new code :D ).

Thanks,

-- 
Julien Thierry

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel