Commit 4488f90c authored by Mark Rutland's avatar Mark Rutland Committed by Will Deacon
Browse files

arm64: insn: simplify insn group identification 



The only code which needs to check for an entire instruction group is
the aarch64_insn_is_steppable() helper function used by kprobes, which
must not be instrumented, and only needs to check for the "Branch,
exception generation and system instructions" class.

Currently we have an out-of-line helper in insn.c which must be marked
as __kprobes, which indexes a table with some bits extracted from the
instruction. In aarch64_insn_is_steppable() we then need to compare the
result with an expected enum value.

It would be simpler to have a predicate for this, as with the other
aarch64_insn_is_*() helpers, which would be always inlined to prevent
inadvertent instrumentation, and would permit better code generation.

This patch adds a predicate function for this instruction group using
the existing __AARCH64_INSN_FUNCS() helpers, and removes the existing
out-of-line helper. As the only class we currently care about is the
branch+exception+sys class, I have only added helpers for this, and left
the other classes unimplemented for now.

There should be no functional change as a result of this patch.

Signed-off-by: default avatarMark Rutland <mark.rutland@arm.com>
Cc: Catalin Marinas <catalin.marinas@arm.com>
Cc: Joey Gouly <joey.gouly@arm.com>
Cc: Will Deacon <will@kernel.org>
Reviewed-by: default avatarJoey Gouly <joey.gouly@arm.com>
Link: https://lore.kernel.org/r/20221114135928.3000571-4-mark.rutland@arm.com


Signed-off-by: default avatarWill Deacon <will@kernel.org>
parent 9b948e79

arch/arm64/include/asm/insn.h

+17 −25
Original line number Diff line number Diff line
@@ -13,31 +13,6 @@ 
#include <asm/insn-def.h>



#ifndef __ASSEMBLY__

/*

 * ARM Architecture Reference Manual for ARMv8 Profile-A, Issue A.a

 * Section C3.1 "A64 instruction index by encoding":

 * AArch64 main encoding table

 *  Bit position

 *   28 27 26 25	Encoding Group

 *   0  0  -  -		Unallocated

 *   1  0  0  -		Data processing, immediate

 *   1  0  1  -		Branch, exception generation and system instructions

 *   -  1  -  0		Loads and stores

 *   -  1  0  1		Data processing - register

 *   0  1  1  1		Data processing - SIMD and floating point

 *   1  1  1  1		Data processing - SIMD and floating point

 * "-" means "don't care"

 */

enum aarch64_insn_encoding_class {

	AARCH64_INSN_CLS_UNKNOWN,	/* UNALLOCATED */

	AARCH64_INSN_CLS_SVE,		/* SVE instructions */

	AARCH64_INSN_CLS_DP_IMM,	/* Data processing - immediate */

	AARCH64_INSN_CLS_DP_REG,	/* Data processing - register */

	AARCH64_INSN_CLS_DP_FPSIMD,	/* Data processing - SIMD and FP */

	AARCH64_INSN_CLS_LDST,		/* Loads and stores */

	AARCH64_INSN_CLS_BR_SYS,	/* Branch, exception generation and

					 * system instructions */

};



enum aarch64_insn_hint_cr_op {

	AARCH64_INSN_HINT_NOP	= 0x0 << 5,
@@ -326,6 +301,23 @@ static __always_inline u32 aarch64_insn_get_##abbr##_value(void) \ 
	return (val);							\

}



/*

 * ARM Architecture Reference Manual for ARMv8 Profile-A, Issue A.a

 * Section C3.1 "A64 instruction index by encoding":

 * AArch64 main encoding table

 *  Bit position

 *   28 27 26 25	Encoding Group

 *   0  0  -  -		Unallocated

 *   1  0  0  -		Data processing, immediate

 *   1  0  1  -		Branch, exception generation and system instructions

 *   -  1  -  0		Loads and stores

 *   -  1  0  1		Data processing - register

 *   0  1  1  1		Data processing - SIMD and floating point

 *   1  1  1  1		Data processing - SIMD and floating point

 * "-" means "don't care"

 */

__AARCH64_INSN_FUNCS(class_branch_sys,	0x1c000000, 0x14000000)



__AARCH64_INSN_FUNCS(adr,	0x9F000000, 0x10000000)

__AARCH64_INSN_FUNCS(adrp,	0x9F000000, 0x90000000)

__AARCH64_INSN_FUNCS(prfm,	0x3FC00000, 0x39800000)

arch/arm64/kernel/probes/decode-insn.c

+1 −1
Original line number Diff line number Diff line
@@ -24,7 +24,7 @@ static bool __kprobes aarch64_insn_is_steppable(u32 insn) 
	 * currently safe. Lastly, MSR instructions can do any number of nasty

	 * things we can't handle during single-stepping.

	 */

	if (aarch64_get_insn_class(insn) == AARCH64_INSN_CLS_BR_SYS) {

	if (aarch64_insn_is_class_branch_sys(insn)) {

		if (aarch64_insn_is_branch(insn) ||

		    aarch64_insn_is_msr_imm(insn) ||

		    aarch64_insn_is_msr_reg(insn) ||

arch/arm64/lib/insn.c

+0 −24
Original line number Diff line number Diff line
@@ -20,30 +20,6 @@ 
#define AARCH64_INSN_N_BIT	BIT(22)

#define AARCH64_INSN_LSL_12	BIT(22)



static const int aarch64_insn_encoding_class[] = {

	AARCH64_INSN_CLS_UNKNOWN,

	AARCH64_INSN_CLS_UNKNOWN,

	AARCH64_INSN_CLS_SVE,

	AARCH64_INSN_CLS_UNKNOWN,

	AARCH64_INSN_CLS_LDST,

	AARCH64_INSN_CLS_DP_REG,

	AARCH64_INSN_CLS_LDST,

	AARCH64_INSN_CLS_DP_FPSIMD,

	AARCH64_INSN_CLS_DP_IMM,

	AARCH64_INSN_CLS_DP_IMM,

	AARCH64_INSN_CLS_BR_SYS,

	AARCH64_INSN_CLS_BR_SYS,

	AARCH64_INSN_CLS_LDST,

	AARCH64_INSN_CLS_DP_REG,

	AARCH64_INSN_CLS_LDST,

	AARCH64_INSN_CLS_DP_FPSIMD,

};



enum aarch64_insn_encoding_class __kprobes aarch64_get_insn_class(u32 insn)

{

	return aarch64_insn_encoding_class[(insn >> 25) & 0xf];

}



static int __kprobes aarch64_get_imm_shift_mask(enum aarch64_insn_imm_type type,

						u32 *maskp, int *shiftp)

{