* [PATCH] riscv: correct riscv_insn_is_c_jr() and riscv_insn_is_c_jalr()
@ 2023-07-31 18:39 Nam Cao
2023-08-09 0:04 ` Charlie Jenkins
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Nam Cao @ 2023-07-31 18:39 UTC (permalink / raw)
To: Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv,
linux-kernel; +Cc: Nam Cao
The instructions c.jr and c.jalr must have rs1 != 0, but
riscv_insn_is_c_jr() and riscv_insn_is_c_jalr() do not check for this. So,
riscv_insn_is_c_jr() can match a reserved encoding, while
riscv_insn_is_c_jalr() can match the c.ebreak instruction.
Rewrite them with check for rs1 != 0.
Signed-off-by: Nam Cao <namcaov@gmail.com>
---
arch/riscv/include/asm/insn.h | 15 +++++++++++++--
1 file changed, 13 insertions(+), 2 deletions(-)
diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
index 4e1505cef8aa..fce00400c9bc 100644
--- a/arch/riscv/include/asm/insn.h
+++ b/arch/riscv/include/asm/insn.h
@@ -110,6 +110,7 @@
#define RVC_INSN_FUNCT4_OPOFF 12
#define RVC_INSN_FUNCT3_MASK GENMASK(15, 13)
#define RVC_INSN_FUNCT3_OPOFF 13
+#define RVC_INSN_J_RS1_MASK GENMASK(11, 7)
#define RVC_INSN_J_RS2_MASK GENMASK(6, 2)
#define RVC_INSN_OPCODE_MASK GENMASK(1, 0)
#define RVC_ENCODE_FUNCT3(f_) (RVC_FUNCT3_##f_ << RVC_INSN_FUNCT3_OPOFF)
@@ -245,8 +246,6 @@ __RISCV_INSN_FUNCS(c_jal, RVC_MASK_C_JAL, RVC_MATCH_C_JAL)
__RISCV_INSN_FUNCS(auipc, RVG_MASK_AUIPC, RVG_MATCH_AUIPC)
__RISCV_INSN_FUNCS(jalr, RVG_MASK_JALR, RVG_MATCH_JALR)
__RISCV_INSN_FUNCS(jal, RVG_MASK_JAL, RVG_MATCH_JAL)
-__RISCV_INSN_FUNCS(c_jr, RVC_MASK_C_JR, RVC_MATCH_C_JR)
-__RISCV_INSN_FUNCS(c_jalr, RVC_MASK_C_JALR, RVC_MATCH_C_JALR)
__RISCV_INSN_FUNCS(c_j, RVC_MASK_C_J, RVC_MATCH_C_J)
__RISCV_INSN_FUNCS(beq, RVG_MASK_BEQ, RVG_MATCH_BEQ)
__RISCV_INSN_FUNCS(bne, RVG_MASK_BNE, RVG_MATCH_BNE)
@@ -273,6 +272,18 @@ static __always_inline bool riscv_insn_is_branch(u32 code)
return (code & RV_INSN_OPCODE_MASK) == RVG_OPCODE_BRANCH;
}
+static __always_inline bool riscv_insn_is_c_jr(u32 code)
+{
+ return (code & RVC_MASK_C_JR) == RVC_MATCH_C_JR &&
+ (code & RVC_INSN_J_RS1_MASK) != 0;
+}
+
+static __always_inline bool riscv_insn_is_c_jalr(u32 code)
+{
+ return (code & RVC_MASK_C_JALR) == RVC_MATCH_C_JALR &&
+ (code & RVC_INSN_J_RS1_MASK) != 0;
+}
+
#define RV_IMM_SIGN(x) (-(((x) >> 31) & 1))
#define RVC_IMM_SIGN(x) (-(((x) >> 12) & 1))
#define RV_X(X, s, mask) (((X) >> (s)) & (mask))
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH] riscv: correct riscv_insn_is_c_jr() and riscv_insn_is_c_jalr()
2023-07-31 18:39 [PATCH] riscv: correct riscv_insn_is_c_jr() and riscv_insn_is_c_jalr() Nam Cao
@ 2023-08-09 0:04 ` Charlie Jenkins
2023-08-14 12:07 ` Björn Töpel
2023-08-17 15:20 ` patchwork-bot+linux-riscv
2 siblings, 0 replies; 5+ messages in thread
From: Charlie Jenkins @ 2023-08-09 0:04 UTC (permalink / raw)
To: Nam Cao; +Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv,
linux-kernel
On Mon, Jul 31, 2023 at 08:39:25PM +0200, Nam Cao wrote:
> The instructions c.jr and c.jalr must have rs1 != 0, but
> riscv_insn_is_c_jr() and riscv_insn_is_c_jalr() do not check for this. So,
> riscv_insn_is_c_jr() can match a reserved encoding, while
> riscv_insn_is_c_jalr() can match the c.ebreak instruction.
>
> Rewrite them with check for rs1 != 0.
>
> Signed-off-by: Nam Cao <namcaov@gmail.com>
> ---
> arch/riscv/include/asm/insn.h | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
> index 4e1505cef8aa..fce00400c9bc 100644
> --- a/arch/riscv/include/asm/insn.h
> +++ b/arch/riscv/include/asm/insn.h
> @@ -110,6 +110,7 @@
> #define RVC_INSN_FUNCT4_OPOFF 12
> #define RVC_INSN_FUNCT3_MASK GENMASK(15, 13)
> #define RVC_INSN_FUNCT3_OPOFF 13
> +#define RVC_INSN_J_RS1_MASK GENMASK(11, 7)
> #define RVC_INSN_J_RS2_MASK GENMASK(6, 2)
> #define RVC_INSN_OPCODE_MASK GENMASK(1, 0)
> #define RVC_ENCODE_FUNCT3(f_) (RVC_FUNCT3_##f_ << RVC_INSN_FUNCT3_OPOFF)
> @@ -245,8 +246,6 @@ __RISCV_INSN_FUNCS(c_jal, RVC_MASK_C_JAL, RVC_MATCH_C_JAL)
> __RISCV_INSN_FUNCS(auipc, RVG_MASK_AUIPC, RVG_MATCH_AUIPC)
> __RISCV_INSN_FUNCS(jalr, RVG_MASK_JALR, RVG_MATCH_JALR)
> __RISCV_INSN_FUNCS(jal, RVG_MASK_JAL, RVG_MATCH_JAL)
> -__RISCV_INSN_FUNCS(c_jr, RVC_MASK_C_JR, RVC_MATCH_C_JR)
> -__RISCV_INSN_FUNCS(c_jalr, RVC_MASK_C_JALR, RVC_MATCH_C_JALR)
> __RISCV_INSN_FUNCS(c_j, RVC_MASK_C_J, RVC_MATCH_C_J)
> __RISCV_INSN_FUNCS(beq, RVG_MASK_BEQ, RVG_MATCH_BEQ)
> __RISCV_INSN_FUNCS(bne, RVG_MASK_BNE, RVG_MATCH_BNE)
> @@ -273,6 +272,18 @@ static __always_inline bool riscv_insn_is_branch(u32 code)
> return (code & RV_INSN_OPCODE_MASK) == RVG_OPCODE_BRANCH;
> }
>
> +static __always_inline bool riscv_insn_is_c_jr(u32 code)
> +{
> + return (code & RVC_MASK_C_JR) == RVC_MATCH_C_JR &&
> + (code & RVC_INSN_J_RS1_MASK) != 0;
> +}
> +
> +static __always_inline bool riscv_insn_is_c_jalr(u32 code)
> +{
> + return (code & RVC_MASK_C_JALR) == RVC_MATCH_C_JALR &&
> + (code & RVC_INSN_J_RS1_MASK) != 0;
> +}
> +
> #define RV_IMM_SIGN(x) (-(((x) >> 31) & 1))
> #define RVC_IMM_SIGN(x) (-(((x) >> 12) & 1))
> #define RV_X(X, s, mask) (((X) >> (s)) & (mask))
> --
> 2.34.1
>
Good catch. You can add:
Reviewed-by: Charlie Jenkins <charlie@rivosinc.com>
>
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] riscv: correct riscv_insn_is_c_jr() and riscv_insn_is_c_jalr()
2023-07-31 18:39 [PATCH] riscv: correct riscv_insn_is_c_jr() and riscv_insn_is_c_jalr() Nam Cao
2023-08-09 0:04 ` Charlie Jenkins
@ 2023-08-14 12:07 ` Björn Töpel
2023-08-14 14:03 ` Nam Cao
2023-08-17 15:20 ` patchwork-bot+linux-riscv
2 siblings, 1 reply; 5+ messages in thread
From: Björn Töpel @ 2023-08-14 12:07 UTC (permalink / raw)
To: Nam Cao, Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv,
linux-kernel
Cc: Nam Cao, charlie
Nam Cao <namcaov@gmail.com> writes:
> The instructions c.jr and c.jalr must have rs1 != 0, but
> riscv_insn_is_c_jr() and riscv_insn_is_c_jalr() do not check for this. So,
> riscv_insn_is_c_jr() can match a reserved encoding, while
> riscv_insn_is_c_jalr() can match the c.ebreak instruction.
>
> Rewrite them with check for rs1 != 0.
>
> Signed-off-by: Nam Cao <namcaov@gmail.com>
> ---
> arch/riscv/include/asm/insn.h | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
> index 4e1505cef8aa..fce00400c9bc 100644
> --- a/arch/riscv/include/asm/insn.h
> +++ b/arch/riscv/include/asm/insn.h
> @@ -110,6 +110,7 @@
> #define RVC_INSN_FUNCT4_OPOFF 12
> #define RVC_INSN_FUNCT3_MASK GENMASK(15, 13)
> #define RVC_INSN_FUNCT3_OPOFF 13
> +#define RVC_INSN_J_RS1_MASK GENMASK(11, 7)
> #define RVC_INSN_J_RS2_MASK GENMASK(6, 2)
> #define RVC_INSN_OPCODE_MASK GENMASK(1, 0)
> #define RVC_ENCODE_FUNCT3(f_) (RVC_FUNCT3_##f_ << RVC_INSN_FUNCT3_OPOFF)
> @@ -245,8 +246,6 @@ __RISCV_INSN_FUNCS(c_jal, RVC_MASK_C_JAL, RVC_MATCH_C_JAL)
> __RISCV_INSN_FUNCS(auipc, RVG_MASK_AUIPC, RVG_MATCH_AUIPC)
> __RISCV_INSN_FUNCS(jalr, RVG_MASK_JALR, RVG_MATCH_JALR)
> __RISCV_INSN_FUNCS(jal, RVG_MASK_JAL, RVG_MATCH_JAL)
> -__RISCV_INSN_FUNCS(c_jr, RVC_MASK_C_JR, RVC_MATCH_C_JR)
> -__RISCV_INSN_FUNCS(c_jalr, RVC_MASK_C_JALR, RVC_MATCH_C_JALR)
> __RISCV_INSN_FUNCS(c_j, RVC_MASK_C_J, RVC_MATCH_C_J)
> __RISCV_INSN_FUNCS(beq, RVG_MASK_BEQ, RVG_MATCH_BEQ)
> __RISCV_INSN_FUNCS(bne, RVG_MASK_BNE, RVG_MATCH_BNE)
> @@ -273,6 +272,18 @@ static __always_inline bool riscv_insn_is_branch(u32 code)
> return (code & RV_INSN_OPCODE_MASK) == RVG_OPCODE_BRANCH;
> }
>
> +static __always_inline bool riscv_insn_is_c_jr(u32 code)
> +{
> + return (code & RVC_MASK_C_JR) == RVC_MATCH_C_JR &&
> + (code & RVC_INSN_J_RS1_MASK) != 0;
> +}
> +
> +static __always_inline bool riscv_insn_is_c_jalr(u32 code)
> +{
> + return (code & RVC_MASK_C_JALR) == RVC_MATCH_C_JALR &&
> + (code & RVC_INSN_J_RS1_MASK) != 0;
> +}
> +
Nice one!
In the future, add a Fixes-tag for these kind of changes!
Fixes: ec5f90877516 ("RISC-V: Move riscv_insn_is_* macros into a common header")
(No need for a respin, b4 will pick up the Fixes above.)
Björn
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH] riscv: correct riscv_insn_is_c_jr() and riscv_insn_is_c_jalr()
2023-08-14 12:07 ` Björn Töpel
@ 2023-08-14 14:03 ` Nam Cao
0 siblings, 0 replies; 5+ messages in thread
From: Nam Cao @ 2023-08-14 14:03 UTC (permalink / raw)
To: Björn Töpel
Cc: Paul Walmsley, Palmer Dabbelt, Albert Ou, linux-riscv,
linux-kernel, charlie
On Mon, Aug 14, 2023 at 02:07:50PM +0200, Björn Töpel wrote:
> In the future, add a Fixes-tag for these kind of changes!
> Fixes: ec5f90877516 ("RISC-V: Move riscv_insn_is_* macros into a common header")
>
> (No need for a respin, b4 will pick up the Fixes above.)
Thanks! I will keep that in mind in the future.
Best regards,
Nam
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] riscv: correct riscv_insn_is_c_jr() and riscv_insn_is_c_jalr()
2023-07-31 18:39 [PATCH] riscv: correct riscv_insn_is_c_jr() and riscv_insn_is_c_jalr() Nam Cao
2023-08-09 0:04 ` Charlie Jenkins
2023-08-14 12:07 ` Björn Töpel
@ 2023-08-17 15:20 ` patchwork-bot+linux-riscv
2 siblings, 0 replies; 5+ messages in thread
From: patchwork-bot+linux-riscv @ 2023-08-17 15:20 UTC (permalink / raw)
To: Nam Cao; +Cc: linux-riscv, paul.walmsley, palmer, aou, linux-kernel
Hello:
This patch was applied to riscv/linux.git (fixes)
by Palmer Dabbelt <palmer@rivosinc.com>:
On Mon, 31 Jul 2023 20:39:25 +0200 you wrote:
> The instructions c.jr and c.jalr must have rs1 != 0, but
> riscv_insn_is_c_jr() and riscv_insn_is_c_jalr() do not check for this. So,
> riscv_insn_is_c_jr() can match a reserved encoding, while
> riscv_insn_is_c_jalr() can match the c.ebreak instruction.
>
> Rewrite them with check for rs1 != 0.
>
> [...]
Here is the summary with links:
- riscv: correct riscv_insn_is_c_jr() and riscv_insn_is_c_jalr()
https://git.kernel.org/riscv/c/79bc3f85c51f
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-08-17 15:21 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-07-31 18:39 [PATCH] riscv: correct riscv_insn_is_c_jr() and riscv_insn_is_c_jalr() Nam Cao
2023-08-09 0:04 ` Charlie Jenkins
2023-08-14 12:07 ` Björn Töpel
2023-08-14 14:03 ` Nam Cao
2023-08-17 15:20 ` patchwork-bot+linux-riscv
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox