All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mark Rutland <mark.rutland@arm.com>
To: linux-arm-kernel@lists.infradead.org
Cc: catalin.marinas@arm.com, joey.gouly@arm.com,
	mark.rutland@arm.com, will@kernel.org
Subject: [PATCH 3/4] arm64: insn: simplify insn group identification
Date: Mon, 14 Nov 2022 13:59:27 +0000	[thread overview]
Message-ID: <20221114135928.3000571-4-mark.rutland@arm.com> (raw)
In-Reply-To: <20221114135928.3000571-1-mark.rutland@arm.com>

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: Mark 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>
---
 arch/arm64/include/asm/insn.h          | 42 +++++++++++---------------
 arch/arm64/kernel/probes/decode-insn.c |  2 +-
 arch/arm64/lib/insn.c                  | 24 ---------------
 3 files changed, 18 insertions(+), 50 deletions(-)

diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h
index de5bc71b4b1d..876cacad103e 100644
--- a/arch/arm64/include/asm/insn.h
+++ b/arch/arm64/include/asm/insn.h
@@ -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)
diff --git a/arch/arm64/kernel/probes/decode-insn.c b/arch/arm64/kernel/probes/decode-insn.c
index 104101f633b1..968d5fffe233 100644
--- a/arch/arm64/kernel/probes/decode-insn.c
+++ b/arch/arm64/kernel/probes/decode-insn.c
@@ -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) ||
diff --git a/arch/arm64/lib/insn.c b/arch/arm64/lib/insn.c
index 67c1be804cc3..99194464d675 100644
--- a/arch/arm64/lib/insn.c
+++ b/arch/arm64/lib/insn.c
@@ -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)
 {
-- 
2.30.2


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

  parent reply	other threads:[~2022-11-14 14:01 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-11-14 13:59 [PATCH 0/4] arm64: insn: cleanups Mark Rutland
2022-11-14 13:59 ` [PATCH 1/4] arm64: insn: remove aarch64_insn_gen_prefetch() Mark Rutland
2022-11-14 13:59 ` [PATCH 2/4] arm64: insn: always inline predicates Mark Rutland
2022-11-14 13:59 ` Mark Rutland [this message]
2022-11-14 13:59 ` [PATCH 4/4] arm64: insn: always inline hint generation Mark Rutland
2022-11-15 11:09 ` [PATCH 0/4] arm64: insn: cleanups Joey Gouly
2022-11-15 18:32 ` Will Deacon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221114135928.3000571-4-mark.rutland@arm.com \
    --to=mark.rutland@arm.com \
    --cc=catalin.marinas@arm.com \
    --cc=joey.gouly@arm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=will@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.