Re: [RFC PATCH 1/5] arm64: Move instruction encoder/decoder under lib/

From: Julien Thierry
Date: Wed Feb 03 2021 - 03:28:22 EST


Hi Mark,

On 2/2/21 11:17 AM, Mark Rutland wrote:
Hi Julien,

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).

Or do you suggest that I still create the aarch64-insn.* files next to the insn.* files?


2) Moving insn.h and insn.c out to arch/arm64/lib/, updating includes
> With that, the second patch might be easier for git to identify as a
rename (if using `git format-patch -M -C`), and it'd be easier to see
that there are no unintended changes.


But it is more a split than a rename (at least in this patch). But I'm happy to do this in another way.

Thanks,

--
Julien Thierry