Re: [RFC PATCH 1/5] arm64: Move instruction encoder/decoder under lib/
From: Julien Thierry
Date: Wed Feb 03 2021 - 12:33:14 EST
On 2/3/21 12:12 PM, Mark Rutland wrote:
On Wed, Feb 03, 2021 at 09:26:45AM +0100, Julien Thierry wrote:
On 2/2/21 11:17 AM, Mark Rutland wrote:
On Wed, Jan 20, 2021 at 06:17:41PM +0100, Julien Thierry wrote:
Aarch64 instruction set encoding and decoding logic can prove useful
for some features/tools both part of the kernel and outside the kernel.
Isolate the function dealing only with encoding/decoding instructions,
with minimal dependency on kernel utilities in order to be able to reuse
that code.
Code was only moved, no code should have been added, removed nor
modifier.
Signed-off-by: Julien Thierry <jthierry@xxxxxxxxxx>
This looks sound, but the diff is a little hard to check.
Yes, I was expecting this change to be hard to digest.
Would it be possible to split this into two patches, where:
1) Refactoring definitions into insn.h and insn.c, leaving those files
in their current locations.
I'm not quite sure I understand the suggestions. After this patch insn.h and
insn.c still contain some definitions that can't really be used outside of
kernel code which is why I split them into insn.* and aarch64-insn.* (the
latter containing what could be used by tools).
Sorry; I hadn't appreciated what was getting left behind. With the
series applied I see that's some kernel patching logic, and AArch32 insn
bits.
How about we invert the move, and split those bits out of insn.c first,
then move the rest in one go, i.e.
1) Factor the patching bits out into new patching.{c,h} files.
2) Move insn.c to arch/arm64/lib/
3) Split insn.* into aarch64-insn.* and aarch32-insn.*
... if that makes any sense?
We might not even need to split the aarch32 bits out given they all have
an aarch32_* prefix.
Yes, that approach sounds good. I'll if the aarchxx-insn is necessary
but as you say, all in the same file shouldn't cause trouble.
Thanks,
--
Julien Thierry