diff mbox series

[v4,1/5] RISC-V: move some stray __RISCV_INSN_FUNCS definitions from kprobes

Message ID 20230109181755.2383085-2-heiko@sntech.de (mailing list archive)
State Superseded
Delegated to: Palmer Dabbelt
Headers show
Series Zbb string optimizations and call support in alternatives | expand

Commit Message

Heiko Stuebner Jan. 9, 2023, 6:17 p.m. UTC 
From: Heiko Stuebner <heiko.stuebner@vrull.eu>

The __RISCV_INSN_FUNCS originally declared riscv_insn_is_* functions inside
the kprobes implementation. This got moved into a central header in
commit ec5f90877516 ("RISC-V: Move riscv_insn_is_* macros into a common header").

Though it looks like I overlooked two of them, so fix that. FENCE itself is
an instruction defined directly by its own opcode, while the created
riscv_isn_is_system function covers all instructions defined under the SYSTEM
opcode.

Fixes: ec5f90877516 ("RISC-V: Move riscv_insn_is_* macros into a common header")
Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
 arch/riscv/include/asm/insn.h            | 10 ++++++++++
 arch/riscv/kernel/probes/simulate-insn.h |  3 ---
 2 files changed, 10 insertions(+), 3 deletions(-)

Comments

Conor Dooley Jan. 9, 2023, 8:53 p.m. UTC | #1 
On Mon, Jan 09, 2023 at 07:17:51PM +0100, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> 
> The __RISCV_INSN_FUNCS originally declared riscv_insn_is_* functions inside
> the kprobes implementation. This got moved into a central header in
> commit ec5f90877516 ("RISC-V: Move riscv_insn_is_* macros into a common header").
> 
> Though it looks like I overlooked two of them, so fix that. FENCE itself is
> an instruction defined directly by its own opcode, while the created
> riscv_isn_is_system function covers all instructions defined under the SYSTEM
> opcode.
> 
> Fixes: ec5f90877516 ("RISC-V: Move riscv_insn_is_* macros into a common header")

Not quite sure why it needs a fixes tag, nothing was actually broken
previously, right?

Not that I really care, but Sasha is likely gonna pick it up for
backporting as a result.

Either way, looks grand.
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/insn.h            | 10 ++++++++++
>  arch/riscv/kernel/probes/simulate-insn.h |  3 ---
>  2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
> index 98453535324a..0455b4dcb0a7 100644
> --- a/arch/riscv/include/asm/insn.h
> +++ b/arch/riscv/include/asm/insn.h
> @@ -128,6 +128,7 @@
>  #define RVC_C2_RD_OPOFF		7
>  
>  /* parts of opcode for RVG*/
> +#define RVG_OPCODE_FENCE	0x0f
>  #define RVG_OPCODE_AUIPC	0x17
>  #define RVG_OPCODE_BRANCH	0x63
>  #define RVG_OPCODE_JALR		0x67
> @@ -163,6 +164,7 @@
>  #define RVG_MATCH_AUIPC		(RVG_OPCODE_AUIPC)
>  #define RVG_MATCH_JALR		(RV_ENCODE_FUNCT3(JALR) | RVG_OPCODE_JALR)
>  #define RVG_MATCH_JAL		(RVG_OPCODE_JAL)
> +#define RVG_MATCH_FENCE		(RVG_OPCODE_FENCE)
>  #define RVG_MATCH_BEQ		(RV_ENCODE_FUNCT3(BEQ) | RVG_OPCODE_BRANCH)
>  #define RVG_MATCH_BNE		(RV_ENCODE_FUNCT3(BNE) | RVG_OPCODE_BRANCH)
>  #define RVG_MATCH_BLT		(RV_ENCODE_FUNCT3(BLT) | RVG_OPCODE_BRANCH)
> @@ -182,6 +184,7 @@
>  #define RVG_MASK_AUIPC		(RV_INSN_OPCODE_MASK)
>  #define RVG_MASK_JALR		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
>  #define RVG_MASK_JAL		(RV_INSN_OPCODE_MASK)
> +#define RVG_MASK_FENCE		(RV_INSN_OPCODE_MASK)
>  #define RVC_MASK_C_JALR		(RVC_INSN_FUNCT4_MASK | RVC_INSN_J_RS2_MASK | RVC_INSN_OPCODE_MASK)
>  #define RVC_MASK_C_JR		(RVC_INSN_FUNCT4_MASK | RVC_INSN_J_RS2_MASK | RVC_INSN_OPCODE_MASK)
>  #define RVC_MASK_C_JAL		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
> @@ -233,6 +236,13 @@ __RISCV_INSN_FUNCS(c_bnez, RVC_MASK_C_BNEZ, RVC_MATCH_C_BNEZ)
>  __RISCV_INSN_FUNCS(c_ebreak, RVC_MASK_C_EBREAK, RVC_MATCH_C_EBREAK)
>  __RISCV_INSN_FUNCS(ebreak, RVG_MASK_EBREAK, RVG_MATCH_EBREAK)
>  __RISCV_INSN_FUNCS(sret, RVG_MASK_SRET, RVG_MATCH_SRET)
> +__RISCV_INSN_FUNCS(fence, RVG_MASK_FENCE, RVG_MATCH_FENCE);
> +
> +/* special case to catch _any_ system instruction */
> +static __always_inline bool riscv_insn_is_system(u32 code)
> +{
> +	return (code & RV_INSN_OPCODE_MASK) == RVG_OPCODE_SYSTEM;
> +}
>  
>  /* special case to catch _any_ branch instruction */
>  static __always_inline bool riscv_insn_is_branch(u32 code)
> diff --git a/arch/riscv/kernel/probes/simulate-insn.h b/arch/riscv/kernel/probes/simulate-insn.h
> index a19aaa0feb44..61e35db31001 100644
> --- a/arch/riscv/kernel/probes/simulate-insn.h
> +++ b/arch/riscv/kernel/probes/simulate-insn.h
> @@ -12,9 +12,6 @@
>  		}							\
>  	} while (0)
>  
> -__RISCV_INSN_FUNCS(system,	0x7f, 0x73);
> -__RISCV_INSN_FUNCS(fence,	0x7f, 0x0f);
> -
>  #define RISCV_INSN_SET_SIMULATE(name, code)				\
>  	do {								\
>  		if (riscv_insn_is_##name(code)) {			\
> -- 
> 2.35.1
>
Andrew Jones Jan. 10, 2023, 8:32 a.m. UTC | #2 
On Mon, Jan 09, 2023 at 07:17:51PM +0100, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> 
> The __RISCV_INSN_FUNCS originally declared riscv_insn_is_* functions inside
> the kprobes implementation. This got moved into a central header in
> commit ec5f90877516 ("RISC-V: Move riscv_insn_is_* macros into a common header").
> 
> Though it looks like I overlooked two of them, so fix that. FENCE itself is
> an instruction defined directly by its own opcode, while the created
> riscv_isn_is_system function covers all instructions defined under the SYSTEM
> opcode.
> 
> Fixes: ec5f90877516 ("RISC-V: Move riscv_insn_is_* macros into a common header")
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/insn.h            | 10 ++++++++++
>  arch/riscv/kernel/probes/simulate-insn.h |  3 ---
>  2 files changed, 10 insertions(+), 3 deletions(-)
>

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Heiko Stuebner Jan. 11, 2023, 3:14 p.m. UTC | #3 
Am Montag, 9. Januar 2023, 21:53:27 CET schrieb Conor Dooley:
> On Mon, Jan 09, 2023 at 07:17:51PM +0100, Heiko Stuebner wrote:
> > From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> > 
> > The __RISCV_INSN_FUNCS originally declared riscv_insn_is_* functions inside
> > the kprobes implementation. This got moved into a central header in
> > commit ec5f90877516 ("RISC-V: Move riscv_insn_is_* macros into a common header").
> > 
> > Though it looks like I overlooked two of them, so fix that. FENCE itself is
> > an instruction defined directly by its own opcode, while the created
> > riscv_isn_is_system function covers all instructions defined under the SYSTEM
> > opcode.
> > 
> > Fixes: ec5f90877516 ("RISC-V: Move riscv_insn_is_* macros into a common header")
> 
> Not quite sure why it needs a fixes tag, nothing was actually broken
> previously, right?

I was going with the thought of "this should've been in the original patch
already" / "the original patch does not fully do what it says it's doing"
hence the Fixes, but you're right - it's just cosmetics at this point and
would just incur more work for the stable people.

So I've dropped the Fixes

Heiko
diff mbox series

Patch

diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
index 98453535324a..0455b4dcb0a7 100644
--- a/arch/riscv/include/asm/insn.h
+++ b/arch/riscv/include/asm/insn.h
@@ -128,6 +128,7 @@ 
 #define RVC_C2_RD_OPOFF		7
 
 /* parts of opcode for RVG*/
+#define RVG_OPCODE_FENCE	0x0f
 #define RVG_OPCODE_AUIPC	0x17
 #define RVG_OPCODE_BRANCH	0x63
 #define RVG_OPCODE_JALR		0x67
@@ -163,6 +164,7 @@ 
 #define RVG_MATCH_AUIPC		(RVG_OPCODE_AUIPC)
 #define RVG_MATCH_JALR		(RV_ENCODE_FUNCT3(JALR) | RVG_OPCODE_JALR)
 #define RVG_MATCH_JAL		(RVG_OPCODE_JAL)
+#define RVG_MATCH_FENCE		(RVG_OPCODE_FENCE)
 #define RVG_MATCH_BEQ		(RV_ENCODE_FUNCT3(BEQ) | RVG_OPCODE_BRANCH)
 #define RVG_MATCH_BNE		(RV_ENCODE_FUNCT3(BNE) | RVG_OPCODE_BRANCH)
 #define RVG_MATCH_BLT		(RV_ENCODE_FUNCT3(BLT) | RVG_OPCODE_BRANCH)
@@ -182,6 +184,7 @@ 
 #define RVG_MASK_AUIPC		(RV_INSN_OPCODE_MASK)
 #define RVG_MASK_JALR		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
 #define RVG_MASK_JAL		(RV_INSN_OPCODE_MASK)
+#define RVG_MASK_FENCE		(RV_INSN_OPCODE_MASK)
 #define RVC_MASK_C_JALR		(RVC_INSN_FUNCT4_MASK | RVC_INSN_J_RS2_MASK | RVC_INSN_OPCODE_MASK)
 #define RVC_MASK_C_JR		(RVC_INSN_FUNCT4_MASK | RVC_INSN_J_RS2_MASK | RVC_INSN_OPCODE_MASK)
 #define RVC_MASK_C_JAL		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
@@ -233,6 +236,13 @@  __RISCV_INSN_FUNCS(c_bnez, RVC_MASK_C_BNEZ, RVC_MATCH_C_BNEZ)
 __RISCV_INSN_FUNCS(c_ebreak, RVC_MASK_C_EBREAK, RVC_MATCH_C_EBREAK)
 __RISCV_INSN_FUNCS(ebreak, RVG_MASK_EBREAK, RVG_MATCH_EBREAK)
 __RISCV_INSN_FUNCS(sret, RVG_MASK_SRET, RVG_MATCH_SRET)
+__RISCV_INSN_FUNCS(fence, RVG_MASK_FENCE, RVG_MATCH_FENCE);
+
+/* special case to catch _any_ system instruction */
+static __always_inline bool riscv_insn_is_system(u32 code)
+{
+	return (code & RV_INSN_OPCODE_MASK) == RVG_OPCODE_SYSTEM;
+}
 
 /* special case to catch _any_ branch instruction */
 static __always_inline bool riscv_insn_is_branch(u32 code)
diff --git a/arch/riscv/kernel/probes/simulate-insn.h b/arch/riscv/kernel/probes/simulate-insn.h
index a19aaa0feb44..61e35db31001 100644
--- a/arch/riscv/kernel/probes/simulate-insn.h
+++ b/arch/riscv/kernel/probes/simulate-insn.h
@@ -12,9 +12,6 @@ 
 		}							\
 	} while (0)
 
-__RISCV_INSN_FUNCS(system,	0x7f, 0x73);
-__RISCV_INSN_FUNCS(fence,	0x7f, 0x0f);
-
 #define RISCV_INSN_SET_SIMULATE(name, code)				\
 	do {								\
 		if (riscv_insn_is_##name(code)) {			\