[PATCH v4 00/12] Allow calls in alternatives

[PATCH v4 00/12] Allow calls in alternatives

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@vrull.eu>

This series is split out of my work on optimizing string functions
and provides the basics to:

- actually allowing calls in alternatives
  Function calls use auipc + jalr to reach those 32bit relative
  addresses but when they're compiled the offset will be wrong
  as alternatives live in a different section. So when the patch
  gets applied the address will point to the wrong location.

  So similar to arm64 the target addresses need to be updated.

  This is probably also helpful for other things needing more
  complex code in alternatives.


For v2 I got into some sort of cleanup spree for the general instruction
parsing that already existed. A number of places do their own
instruction parsing and I tried consolidating some of them.

Noteable, the kvm parts still do, but I had to stop somewhere :-)

The series is based on v6.1-rc7 right now.

changes since v3:
- separate allowing calls in alternatives from string work
- move the immediate handling for auipc+jalr into the insn.h header
  This allows other parts of the kernel to reuse this, instead of
  duplicating the code in a number or areas
- adjust the riscv_alternative_fix_auipc_jalr function to be called
  from a central _fix_offsets function, so that other offsets can
  get fixed from the same loop in the future (jal, etc)

  I've removed Conor's Reviewed-by: from that last patch, as it
  changed so much since v3.

changes since v2:
- add patch fixing the c.jalr funct4 value
- reword some commit messages
- fix position of auipc addition patch (earlier)
- fix compile errors from patch-reordering gone wrong
  (worked at the end of v2, but compiling individual patches
   caused issues) - patches are now tested individually
- limit Zbb variants for GNU as for now
  (LLVM support for .option arch is still under review)
- prevent str-functions from getting optimized to builtin-variants

changes since v1:
- a number of generalizations/cleanups for instruction parsing
- use accessor function to access instructions (Emil)
- actually patch the correct location when having more than one
  instruction in an alternative block
- string function cleanups (comments etc) (Conor)
- move zbb extension above s* extensions in cpu.c lists

changes since rfc:
- make Zbb code actually work
- drop some unneeded patches
- a lot of cleanups

Heiko Stuebner (12):
  RISC-V: fix funct4 definition for c.jalr in parse_asm.h
  RISC-V: add prefix to all constants/macros in parse_asm.h
  RISC-V: detach funct-values from their offset
  RISC-V: add ebreak instructions to definitions
  RISC-V: add auipc elements to parse_asm header
  RISC-V: Move riscv_insn_is_* macros into a common header
  RISC-V: rename parse_asm.h to insn.h
  RISC-V: kprobes: use central defined funct3 constants
  RISC-V: add U-type imm parsing to insn.h header
  RISC-V: add rd reg parsing to insn.h header
  RISC-V: add helpers for handling immediates in U-type and I-type pairs
  RISC-V: fix auipc-jalr addresses in patched alternatives

 arch/riscv/include/asm/alternative.h     |   3 +
 arch/riscv/include/asm/insn.h            | 339 +++++++++++++++++++++++
 arch/riscv/include/asm/parse_asm.h       | 219 ---------------
 arch/riscv/kernel/alternative.c          |  56 ++++
 arch/riscv/kernel/cpufeature.c           |   5 +-
 arch/riscv/kernel/kgdb.c                 |  63 ++---
 arch/riscv/kernel/probes/simulate-insn.c |  19 +-
 arch/riscv/kernel/probes/simulate-insn.h |  26 +-
 8 files changed, 435 insertions(+), 295 deletions(-)
 create mode 100644 arch/riscv/include/asm/insn.h
 delete mode 100644 arch/riscv/include/asm/parse_asm.h

-- 
2.35.1


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

[PATCH v4 01/12] RISC-V: fix funct4 definition for c.jalr in parse_asm.h

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@vrull.eu>

The opcode definition for c.jalr is
    c.jalr c_rs1_n0  1..0=2 15..13=4 12=1 6..2=0

This means funct4 consisting of bit [15:12] is 1001b, so the value is 0x9.

Fixes: edde5584c7ab ("riscv: Add SW single-step support for KDB")
Reported-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
 arch/riscv/include/asm/parse_asm.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/parse_asm.h b/arch/riscv/include/asm/parse_asm.h
index f36368de839f..7fee806805c1 100644
--- a/arch/riscv/include/asm/parse_asm.h
+++ b/arch/riscv/include/asm/parse_asm.h
@@ -125,7 +125,7 @@
 #define FUNCT3_C_J		0xa000
 #define FUNCT3_C_JAL		0x2000
 #define FUNCT4_C_JR		0x8000
-#define FUNCT4_C_JALR		0xf000
+#define FUNCT4_C_JALR		0x9000
 
 #define FUNCT12_SRET		0x10200000
 
-- 
2.35.1


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

[PATCH v4 02/12] RISC-V: add prefix to all constants/macros in parse_asm.h

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@vrull.eu>

Some of the constants and macros already have suitable RV_, RVG_ or
RVC_ prefixes.

Extend this to the rest of the file as well, as we want to use these
things in a broader scope soon.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
 arch/riscv/include/asm/parse_asm.h | 194 ++++++++++++++---------------
 arch/riscv/kernel/kgdb.c           |  40 +++---
 2 files changed, 117 insertions(+), 117 deletions(-)

diff --git a/arch/riscv/include/asm/parse_asm.h b/arch/riscv/include/asm/parse_asm.h
index 7fee806805c1..ea51542e0c65 100644
--- a/arch/riscv/include/asm/parse_asm.h
+++ b/arch/riscv/include/asm/parse_asm.h
@@ -6,37 +6,37 @@
 #include <linux/bits.h>
 
 /* The bit field of immediate value in I-type instruction */
-#define I_IMM_SIGN_OPOFF	31
-#define I_IMM_11_0_OPOFF	20
-#define I_IMM_SIGN_OFF		12
-#define I_IMM_11_0_OFF		0
-#define I_IMM_11_0_MASK		GENMASK(11, 0)
+#define RV_I_IMM_SIGN_OPOFF	31
+#define RV_I_IMM_11_0_OPOFF	20
+#define RV_I_IMM_SIGN_OFF	12
+#define RV_I_IMM_11_0_OFF	0
+#define RV_I_IMM_11_0_MASK	GENMASK(11, 0)
 
 /* The bit field of immediate value in J-type instruction */
-#define J_IMM_SIGN_OPOFF	31
-#define J_IMM_10_1_OPOFF	21
-#define J_IMM_11_OPOFF		20
-#define J_IMM_19_12_OPOFF	12
-#define J_IMM_SIGN_OFF		20
-#define J_IMM_10_1_OFF		1
-#define J_IMM_11_OFF		11
-#define J_IMM_19_12_OFF		12
-#define J_IMM_10_1_MASK		GENMASK(9, 0)
-#define J_IMM_11_MASK		GENMASK(0, 0)
-#define J_IMM_19_12_MASK	GENMASK(7, 0)
+#define RV_J_IMM_SIGN_OPOFF	31
+#define RV_J_IMM_10_1_OPOFF	21
+#define RV_J_IMM_11_OPOFF	20
+#define RV_J_IMM_19_12_OPOFF	12
+#define RV_J_IMM_SIGN_OFF	20
+#define RV_J_IMM_10_1_OFF	1
+#define RV_J_IMM_11_OFF		11
+#define RV_J_IMM_19_12_OFF	12
+#define RV_J_IMM_10_1_MASK	GENMASK(9, 0)
+#define RV_J_IMM_11_MASK	GENMASK(0, 0)
+#define RV_J_IMM_19_12_MASK	GENMASK(7, 0)
 
 /* The bit field of immediate value in B-type instruction */
-#define B_IMM_SIGN_OPOFF	31
-#define B_IMM_10_5_OPOFF	25
-#define B_IMM_4_1_OPOFF		8
-#define B_IMM_11_OPOFF		7
-#define B_IMM_SIGN_OFF		12
-#define B_IMM_10_5_OFF		5
-#define B_IMM_4_1_OFF		1
-#define B_IMM_11_OFF		11
-#define B_IMM_10_5_MASK		GENMASK(5, 0)
-#define B_IMM_4_1_MASK		GENMASK(3, 0)
-#define B_IMM_11_MASK		GENMASK(0, 0)
+#define RV_B_IMM_SIGN_OPOFF	31
+#define RV_B_IMM_10_5_OPOFF	25
+#define RV_B_IMM_4_1_OPOFF	8
+#define RV_B_IMM_11_OPOFF	7
+#define RV_B_IMM_SIGN_OFF	12
+#define RV_B_IMM_10_5_OFF	5
+#define RV_B_IMM_4_1_OFF	1
+#define RV_B_IMM_11_OFF		11
+#define RV_B_IMM_10_5_MASK	GENMASK(5, 0)
+#define RV_B_IMM_4_1_MASK	GENMASK(3, 0)
+#define RV_B_IMM_11_MASK	GENMASK(0, 0)
 
 /* The register offset in RVG instruction */
 #define RVG_RS1_OPOFF		15
@@ -100,71 +100,71 @@
 #define RVC_C2_RD_OPOFF		7
 
 /* parts of opcode for RVG*/
-#define OPCODE_BRANCH		0x63
-#define OPCODE_JALR		0x67
-#define OPCODE_JAL		0x6f
-#define OPCODE_SYSTEM		0x73
+#define RVG_OPCODE_BRANCH	0x63
+#define RVG_OPCODE_JALR		0x67
+#define RVG_OPCODE_JAL		0x6f
+#define RVG_OPCODE_SYSTEM	0x73
 
 /* parts of opcode for RVC*/
-#define OPCODE_C_0		0x0
-#define OPCODE_C_1		0x1
-#define OPCODE_C_2		0x2
+#define RVC_OPCODE_C0		0x0
+#define RVC_OPCODE_C1		0x1
+#define RVC_OPCODE_C2		0x2
 
 /* parts of funct3 code for I, M, A extension*/
-#define FUNCT3_JALR		0x0
-#define FUNCT3_BEQ		0x0
-#define FUNCT3_BNE		0x1000
-#define FUNCT3_BLT		0x4000
-#define FUNCT3_BGE		0x5000
-#define FUNCT3_BLTU		0x6000
-#define FUNCT3_BGEU		0x7000
+#define RVG_FUNCT3_JALR		0x0
+#define RVG_FUNCT3_BEQ		0x0
+#define RVG_FUNCT3_BNE		0x1000
+#define RVG_FUNCT3_BLT		0x4000
+#define RVG_FUNCT3_BGE		0x5000
+#define RVG_FUNCT3_BLTU		0x6000
+#define RVG_FUNCT3_BGEU		0x7000
 
 /* parts of funct3 code for C extension*/
-#define FUNCT3_C_BEQZ		0xc000
-#define FUNCT3_C_BNEZ		0xe000
-#define FUNCT3_C_J		0xa000
-#define FUNCT3_C_JAL		0x2000
-#define FUNCT4_C_JR		0x8000
-#define FUNCT4_C_JALR		0x9000
-
-#define FUNCT12_SRET		0x10200000
-
-#define MATCH_JALR		(FUNCT3_JALR | OPCODE_JALR)
-#define MATCH_JAL		(OPCODE_JAL)
-#define MATCH_BEQ		(FUNCT3_BEQ | OPCODE_BRANCH)
-#define MATCH_BNE		(FUNCT3_BNE | OPCODE_BRANCH)
-#define MATCH_BLT		(FUNCT3_BLT | OPCODE_BRANCH)
-#define MATCH_BGE		(FUNCT3_BGE | OPCODE_BRANCH)
-#define MATCH_BLTU		(FUNCT3_BLTU | OPCODE_BRANCH)
-#define MATCH_BGEU		(FUNCT3_BGEU | OPCODE_BRANCH)
-#define MATCH_SRET		(FUNCT12_SRET | OPCODE_SYSTEM)
-#define MATCH_C_BEQZ		(FUNCT3_C_BEQZ | OPCODE_C_1)
-#define MATCH_C_BNEZ		(FUNCT3_C_BNEZ | OPCODE_C_1)
-#define MATCH_C_J		(FUNCT3_C_J | OPCODE_C_1)
-#define MATCH_C_JAL		(FUNCT3_C_JAL | OPCODE_C_1)
-#define MATCH_C_JR		(FUNCT4_C_JR | OPCODE_C_2)
-#define MATCH_C_JALR		(FUNCT4_C_JALR | OPCODE_C_2)
-
-#define MASK_JALR		0x707f
-#define MASK_JAL		0x7f
-#define MASK_C_JALR		0xf07f
-#define MASK_C_JR		0xf07f
-#define MASK_C_JAL		0xe003
-#define MASK_C_J		0xe003
-#define MASK_BEQ		0x707f
-#define MASK_BNE		0x707f
-#define MASK_BLT		0x707f
-#define MASK_BGE		0x707f
-#define MASK_BLTU		0x707f
-#define MASK_BGEU		0x707f
-#define MASK_C_BEQZ		0xe003
-#define MASK_C_BNEZ		0xe003
-#define MASK_SRET		0xffffffff
+#define RVC_FUNCT3_C_BEQZ	0xc000
+#define RVC_FUNCT3_C_BNEZ	0xe000
+#define RVC_FUNCT3_C_J		0xa000
+#define RVC_FUNCT3_C_JAL	0x2000
+#define RVC_FUNCT4_C_JR		0x8000
+#define RVC_FUNCT4_C_JALR	0x9000
+
+#define RVG_FUNCT12_SRET	0x10200000
+
+#define RVG_MATCH_JALR		(RVG_FUNCT3_JALR | RVG_OPCODE_JALR)
+#define RVG_MATCH_JAL		(RVG_OPCODE_JAL)
+#define RVG_MATCH_BEQ		(RVG_FUNCT3_BEQ | RVG_OPCODE_BRANCH)
+#define RVG_MATCH_BNE		(RVG_FUNCT3_BNE | RVG_OPCODE_BRANCH)
+#define RVG_MATCH_BLT		(RVG_FUNCT3_BLT | RVG_OPCODE_BRANCH)
+#define RVG_MATCH_BGE		(RVG_FUNCT3_BGE | RVG_OPCODE_BRANCH)
+#define RVG_MATCH_BLTU		(RVG_FUNCT3_BLTU | RVG_OPCODE_BRANCH)
+#define RVG_MATCH_BGEU		(RVG_FUNCT3_BGEU | RVG_OPCODE_BRANCH)
+#define RVG_MATCH_SRET		(RVG_FUNCT12_SRET | RVG_OPCODE_SYSTEM)
+#define RVC_MATCH_C_BEQZ	(RVC_FUNCT3_C_BEQZ | RVC_OPCODE_C1)
+#define RVC_MATCH_C_BNEZ	(RVC_FUNCT3_C_BNEZ | RVC_OPCODE_C1)
+#define RVC_MATCH_C_J		(RVC_FUNCT3_C_J | RVC_OPCODE_C1)
+#define RVC_MATCH_C_JAL		(RVC_FUNCT3_C_JAL | RVC_OPCODE_C1)
+#define RVC_MATCH_C_JR		(RVC_FUNCT4_C_JR | RVC_OPCODE_C2)
+#define RVC_MATCH_C_JALR	(RVC_FUNCT4_C_JALR | RVC_OPCODE_C2)
+
+#define RVG_MASK_JALR		0x707f
+#define RVG_MASK_JAL		0x7f
+#define RVC_MASK_C_JALR		0xf07f
+#define RVC_MASK_C_JR		0xf07f
+#define RVC_MASK_C_JAL		0xe003
+#define RVC_MASK_C_J		0xe003
+#define RVG_MASK_BEQ		0x707f
+#define RVG_MASK_BNE		0x707f
+#define RVG_MASK_BLT		0x707f
+#define RVG_MASK_BGE		0x707f
+#define RVG_MASK_BLTU		0x707f
+#define RVG_MASK_BGEU		0x707f
+#define RVC_MASK_C_BEQZ		0xe003
+#define RVC_MASK_C_BNEZ		0xe003
+#define RVG_MASK_SRET		0xffffffff
 
 #define __INSN_LENGTH_MASK	_UL(0x3)
 #define __INSN_LENGTH_GE_32	_UL(0x3)
 #define __INSN_OPCODE_MASK	_UL(0x7F)
-#define __INSN_BRANCH_OPCODE	_UL(OPCODE_BRANCH)
+#define __INSN_BRANCH_OPCODE	_UL(RVG_OPCODE_BRANCH)
 
 /* Define a series of is_XXX_insn functions to check if the value INSN
  * is an instance of instruction XXX.
@@ -180,26 +180,26 @@ static inline bool is_ ## INSN_NAME ## _insn(long insn) \
 #define RV_X(X, s, mask)  (((X) >> (s)) & (mask))
 #define RVC_X(X, s, mask) RV_X(X, s, mask)
 
-#define EXTRACT_JTYPE_IMM(x) \
+#define RV_EXTRACT_JTYPE_IMM(x) \
 	({typeof(x) x_ = (x); \
-	(RV_X(x_, J_IMM_10_1_OPOFF, J_IMM_10_1_MASK) << J_IMM_10_1_OFF) | \
-	(RV_X(x_, J_IMM_11_OPOFF, J_IMM_11_MASK) << J_IMM_11_OFF) | \
-	(RV_X(x_, J_IMM_19_12_OPOFF, J_IMM_19_12_MASK) << J_IMM_19_12_OFF) | \
-	(RV_IMM_SIGN(x_) << J_IMM_SIGN_OFF); })
+	(RV_X(x_, RV_J_IMM_10_1_OPOFF, RV_J_IMM_10_1_MASK) << RV_J_IMM_10_1_OFF) | \
+	(RV_X(x_, RV_J_IMM_11_OPOFF, RV_J_IMM_11_MASK) << RV_J_IMM_11_OFF) | \
+	(RV_X(x_, RV_J_IMM_19_12_OPOFF, RV_J_IMM_19_12_MASK) << RV_J_IMM_19_12_OFF) | \
+	(RV_IMM_SIGN(x_) << RV_J_IMM_SIGN_OFF); })
 
-#define EXTRACT_ITYPE_IMM(x) \
+#define RV_EXTRACT_ITYPE_IMM(x) \
 	({typeof(x) x_ = (x); \
-	(RV_X(x_, I_IMM_11_0_OPOFF, I_IMM_11_0_MASK)) | \
-	(RV_IMM_SIGN(x_) << I_IMM_SIGN_OFF); })
+	(RV_X(x_, RV_I_IMM_11_0_OPOFF, RV_I_IMM_11_0_MASK)) | \
+	(RV_IMM_SIGN(x_) << RV_I_IMM_SIGN_OFF); })
 
-#define EXTRACT_BTYPE_IMM(x) \
+#define RV_EXTRACT_BTYPE_IMM(x) \
 	({typeof(x) x_ = (x); \
-	(RV_X(x_, B_IMM_4_1_OPOFF, B_IMM_4_1_MASK) << B_IMM_4_1_OFF) | \
-	(RV_X(x_, B_IMM_10_5_OPOFF, B_IMM_10_5_MASK) << B_IMM_10_5_OFF) | \
-	(RV_X(x_, B_IMM_11_OPOFF, B_IMM_11_MASK) << B_IMM_11_OFF) | \
-	(RV_IMM_SIGN(x_) << B_IMM_SIGN_OFF); })
+	(RV_X(x_, RV_B_IMM_4_1_OPOFF, RV_B_IMM_4_1_MASK) << RV_B_IMM_4_1_OFF) | \
+	(RV_X(x_, RV_B_IMM_10_5_OPOFF, RV_B_IMM_10_5_MASK) << RV_B_IMM_10_5_OFF) | \
+	(RV_X(x_, RV_B_IMM_11_OPOFF, RV_B_IMM_11_MASK) << RV_B_IMM_11_OFF) | \
+	(RV_IMM_SIGN(x_) << RV_B_IMM_SIGN_OFF); })
 
-#define EXTRACT_RVC_J_IMM(x) \
+#define RVC_EXTRACT_JTYPE_IMM(x) \
 	({typeof(x) x_ = (x); \
 	(RVC_X(x_, RVC_J_IMM_3_1_OPOFF, RVC_J_IMM_3_1_MASK) << RVC_J_IMM_3_1_OFF) | \
 	(RVC_X(x_, RVC_J_IMM_4_OPOFF, RVC_J_IMM_4_MASK) << RVC_J_IMM_4_OFF) | \
@@ -210,7 +210,7 @@ static inline bool is_ ## INSN_NAME ## _insn(long insn) \
 	(RVC_X(x_, RVC_J_IMM_10_OPOFF, RVC_J_IMM_10_MASK) << RVC_J_IMM_10_OFF) | \
 	(RVC_IMM_SIGN(x_) << RVC_J_IMM_SIGN_OFF); })
 
-#define EXTRACT_RVC_B_IMM(x) \
+#define RVC_EXTRACT_BTYPE_IMM(x) \
 	({typeof(x) x_ = (x); \
 	(RVC_X(x_, RVC_B_IMM_2_1_OPOFF, RVC_B_IMM_2_1_MASK) << RVC_B_IMM_2_1_OFF) | \
 	(RVC_X(x_, RVC_B_IMM_4_3_OPOFF, RVC_B_IMM_4_3_MASK) << RVC_B_IMM_4_3_OFF) | \
diff --git a/arch/riscv/kernel/kgdb.c b/arch/riscv/kernel/kgdb.c
index 963ed7edcff2..137c6d870d58 100644
--- a/arch/riscv/kernel/kgdb.c
+++ b/arch/riscv/kernel/kgdb.c
@@ -29,20 +29,20 @@ DECLARE_INSN(c_jal, MATCH_C_JAL, MASK_C_JAL)
 #else
 #define is_c_jal_insn(opcode) 0
 #endif
-DECLARE_INSN(jalr, MATCH_JALR, MASK_JALR)
-DECLARE_INSN(jal, MATCH_JAL, MASK_JAL)
-DECLARE_INSN(c_jr, MATCH_C_JR, MASK_C_JR)
-DECLARE_INSN(c_jalr, MATCH_C_JALR, MASK_C_JALR)
-DECLARE_INSN(c_j, MATCH_C_J, MASK_C_J)
-DECLARE_INSN(beq, MATCH_BEQ, MASK_BEQ)
-DECLARE_INSN(bne, MATCH_BNE, MASK_BNE)
-DECLARE_INSN(blt, MATCH_BLT, MASK_BLT)
-DECLARE_INSN(bge, MATCH_BGE, MASK_BGE)
-DECLARE_INSN(bltu, MATCH_BLTU, MASK_BLTU)
-DECLARE_INSN(bgeu, MATCH_BGEU, MASK_BGEU)
-DECLARE_INSN(c_beqz, MATCH_C_BEQZ, MASK_C_BEQZ)
-DECLARE_INSN(c_bnez, MATCH_C_BNEZ, MASK_C_BNEZ)
-DECLARE_INSN(sret, MATCH_SRET, MASK_SRET)
+DECLARE_INSN(jalr, RVG_MATCH_JALR, RVG_MASK_JALR)
+DECLARE_INSN(jal, RVG_MATCH_JAL, RVG_MASK_JAL)
+DECLARE_INSN(c_jr, RVC_MATCH_C_JR, RVC_MASK_C_JR)
+DECLARE_INSN(c_jalr, RVC_MATCH_C_JALR, RVC_MASK_C_JALR)
+DECLARE_INSN(c_j, RVC_MATCH_C_J, RVC_MASK_C_J)
+DECLARE_INSN(beq, RVG_MATCH_BEQ, RVG_MASK_BEQ)
+DECLARE_INSN(bne, RVG_MATCH_BNE, RVG_MASK_BNE)
+DECLARE_INSN(blt, RVG_MATCH_BLT, RVG_MASK_BLT)
+DECLARE_INSN(bge, RVG_MATCH_BGE, RVG_MASK_BGE)
+DECLARE_INSN(bltu, RVG_MATCH_BLTU, RVG_MASK_BLTU)
+DECLARE_INSN(bgeu, RVG_MATCH_BGEU, RVG_MASK_BGEU)
+DECLARE_INSN(c_beqz, RVC_MATCH_C_BEQZ, RVC_MASK_C_BEQZ)
+DECLARE_INSN(c_bnez, RVC_MATCH_C_BNEZ, RVC_MASK_C_BNEZ)
+DECLARE_INSN(sret, RVG_MATCH_SRET, RVG_MASK_SRET)
 
 static int decode_register_index(unsigned long opcode, int offset)
 {
@@ -69,19 +69,19 @@ static int get_step_address(struct pt_regs *regs, unsigned long *next_addr)
 			rs1_num = decode_register_index(op_code, RVC_C2_RS1_OPOFF);
 			*next_addr = regs_ptr[rs1_num];
 		} else if (is_c_j_insn(op_code) || is_c_jal_insn(op_code)) {
-			*next_addr = EXTRACT_RVC_J_IMM(op_code) + pc;
+			*next_addr = RVC_EXTRACT_JTYPE_IMM(op_code) + pc;
 		} else if (is_c_beqz_insn(op_code)) {
 			rs1_num = decode_register_index_short(op_code,
 							      RVC_C1_RS1_OPOFF);
 			if (!rs1_num || regs_ptr[rs1_num] == 0)
-				*next_addr = EXTRACT_RVC_B_IMM(op_code) + pc;
+				*next_addr = RVC_EXTRACT_BTYPE_IMM(op_code) + pc;
 			else
 				*next_addr = pc + 2;
 		} else if (is_c_bnez_insn(op_code)) {
 			rs1_num =
 			    decode_register_index_short(op_code, RVC_C1_RS1_OPOFF);
 			if (rs1_num && regs_ptr[rs1_num] != 0)
-				*next_addr = EXTRACT_RVC_B_IMM(op_code) + pc;
+				*next_addr = RVC_EXTRACT_BTYPE_IMM(op_code) + pc;
 			else
 				*next_addr = pc + 2;
 		} else {
@@ -90,7 +90,7 @@ static int get_step_address(struct pt_regs *regs, unsigned long *next_addr)
 	} else {
 		if ((op_code & __INSN_OPCODE_MASK) == __INSN_BRANCH_OPCODE) {
 			bool result = false;
-			long imm = EXTRACT_BTYPE_IMM(op_code);
+			long imm = RV_EXTRACT_BTYPE_IMM(op_code);
 			unsigned long rs1_val = 0, rs2_val = 0;
 
 			rs1_num = decode_register_index(op_code, RVG_RS1_OPOFF);
@@ -121,12 +121,12 @@ static int get_step_address(struct pt_regs *regs, unsigned long *next_addr)
 			else
 				*next_addr = pc + 4;
 		} else if (is_jal_insn(op_code)) {
-			*next_addr = EXTRACT_JTYPE_IMM(op_code) + pc;
+			*next_addr = RV_EXTRACT_JTYPE_IMM(op_code) + pc;
 		} else if (is_jalr_insn(op_code)) {
 			rs1_num = decode_register_index(op_code, RVG_RS1_OPOFF);
 			if (rs1_num)
 				*next_addr = ((unsigned long *)regs)[rs1_num];
-			*next_addr += EXTRACT_ITYPE_IMM(op_code);
+			*next_addr += RV_EXTRACT_ITYPE_IMM(op_code);
 		} else if (is_sret_insn(op_code)) {
 			*next_addr = pc;
 		} else {
-- 
2.35.1


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

[PATCH v4 03/12] RISC-V: detach funct-values from their offset

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@vrull.eu>

Rather than defining funct3, funct4, etc values pre-shifted to their
target-position in an instruction, define the values themselves and
only shift them where needed.

This allows using these funct-values in other places as well, for example
when decoding functions.

At the same time also reduces the use of magic numbers, one would need
a spec manual to understand.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
 arch/riscv/include/asm/parse_asm.h | 100 +++++++++++++++++------------
 1 file changed, 59 insertions(+), 41 deletions(-)

diff --git a/arch/riscv/include/asm/parse_asm.h b/arch/riscv/include/asm/parse_asm.h
index ea51542e0c65..e3f87da108f4 100644
--- a/arch/riscv/include/asm/parse_asm.h
+++ b/arch/riscv/include/asm/parse_asm.h
@@ -5,6 +5,15 @@
 
 #include <linux/bits.h>
 
+#define RV_INSN_FUNCT3_MASK	GENMASK(14, 12)
+#define RV_INSN_FUNCT3_OPOFF	12
+#define RV_INSN_OPCODE_MASK	GENMASK(6, 0)
+#define RV_INSN_OPCODE_OPOFF	0
+#define RV_INSN_FUNCT12_OPOFF	20
+
+#define RV_ENCODE_FUNCT3(f_)	(RVG_FUNCT3_##f_ << RV_INSN_FUNCT3_OPOFF)
+#define RV_ENCODE_FUNCT12(f_)	(RVG_FUNCT12_##f_ << RV_INSN_FUNCT12_OPOFF)
+
 /* The bit field of immediate value in I-type instruction */
 #define RV_I_IMM_SIGN_OPOFF	31
 #define RV_I_IMM_11_0_OPOFF	20
@@ -84,6 +93,15 @@
 #define RVC_B_IMM_2_1_MASK	GENMASK(1, 0)
 #define RVC_B_IMM_5_MASK	GENMASK(0, 0)
 
+#define RVC_INSN_FUNCT4_MASK	GENMASK(15, 12)
+#define RVC_INSN_FUNCT4_OPOFF	12
+#define RVC_INSN_FUNCT3_MASK	GENMASK(15, 13)
+#define RVC_INSN_FUNCT3_OPOFF	13
+#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)
+#define RVC_ENCODE_FUNCT4(f_)	(RVC_FUNCT4_##f_ << RVC_INSN_FUNCT4_OPOFF)
+
 /* The register offset in RVC op=C0 instruction */
 #define RVC_C0_RS1_OPOFF	7
 #define RVC_C0_RS2_OPOFF	2
@@ -113,52 +131,52 @@
 /* parts of funct3 code for I, M, A extension*/
 #define RVG_FUNCT3_JALR		0x0
 #define RVG_FUNCT3_BEQ		0x0
-#define RVG_FUNCT3_BNE		0x1000
-#define RVG_FUNCT3_BLT		0x4000
-#define RVG_FUNCT3_BGE		0x5000
-#define RVG_FUNCT3_BLTU		0x6000
-#define RVG_FUNCT3_BGEU		0x7000
+#define RVG_FUNCT3_BNE		0x1
+#define RVG_FUNCT3_BLT		0x4
+#define RVG_FUNCT3_BGE		0x5
+#define RVG_FUNCT3_BLTU		0x6
+#define RVG_FUNCT3_BGEU		0x7
 
 /* parts of funct3 code for C extension*/
-#define RVC_FUNCT3_C_BEQZ	0xc000
-#define RVC_FUNCT3_C_BNEZ	0xe000
-#define RVC_FUNCT3_C_J		0xa000
-#define RVC_FUNCT3_C_JAL	0x2000
-#define RVC_FUNCT4_C_JR		0x8000
-#define RVC_FUNCT4_C_JALR	0x9000
+#define RVC_FUNCT3_C_BEQZ	0x6
+#define RVC_FUNCT3_C_BNEZ	0x7
+#define RVC_FUNCT3_C_J		0x5
+#define RVC_FUNCT3_C_JAL	0x1
+#define RVC_FUNCT4_C_JR		0x8
+#define RVC_FUNCT4_C_JALR	0x9
 
-#define RVG_FUNCT12_SRET	0x10200000
+#define RVG_FUNCT12_SRET	0x102
 
-#define RVG_MATCH_JALR		(RVG_FUNCT3_JALR | RVG_OPCODE_JALR)
+#define RVG_MATCH_JALR		(RV_ENCODE_FUNCT3(JALR) | RVG_OPCODE_JALR)
 #define RVG_MATCH_JAL		(RVG_OPCODE_JAL)
-#define RVG_MATCH_BEQ		(RVG_FUNCT3_BEQ | RVG_OPCODE_BRANCH)
-#define RVG_MATCH_BNE		(RVG_FUNCT3_BNE | RVG_OPCODE_BRANCH)
-#define RVG_MATCH_BLT		(RVG_FUNCT3_BLT | RVG_OPCODE_BRANCH)
-#define RVG_MATCH_BGE		(RVG_FUNCT3_BGE | RVG_OPCODE_BRANCH)
-#define RVG_MATCH_BLTU		(RVG_FUNCT3_BLTU | RVG_OPCODE_BRANCH)
-#define RVG_MATCH_BGEU		(RVG_FUNCT3_BGEU | RVG_OPCODE_BRANCH)
-#define RVG_MATCH_SRET		(RVG_FUNCT12_SRET | RVG_OPCODE_SYSTEM)
-#define RVC_MATCH_C_BEQZ	(RVC_FUNCT3_C_BEQZ | RVC_OPCODE_C1)
-#define RVC_MATCH_C_BNEZ	(RVC_FUNCT3_C_BNEZ | RVC_OPCODE_C1)
-#define RVC_MATCH_C_J		(RVC_FUNCT3_C_J | RVC_OPCODE_C1)
-#define RVC_MATCH_C_JAL		(RVC_FUNCT3_C_JAL | RVC_OPCODE_C1)
-#define RVC_MATCH_C_JR		(RVC_FUNCT4_C_JR | RVC_OPCODE_C2)
-#define RVC_MATCH_C_JALR	(RVC_FUNCT4_C_JALR | RVC_OPCODE_C2)
-
-#define RVG_MASK_JALR		0x707f
-#define RVG_MASK_JAL		0x7f
-#define RVC_MASK_C_JALR		0xf07f
-#define RVC_MASK_C_JR		0xf07f
-#define RVC_MASK_C_JAL		0xe003
-#define RVC_MASK_C_J		0xe003
-#define RVG_MASK_BEQ		0x707f
-#define RVG_MASK_BNE		0x707f
-#define RVG_MASK_BLT		0x707f
-#define RVG_MASK_BGE		0x707f
-#define RVG_MASK_BLTU		0x707f
-#define RVG_MASK_BGEU		0x707f
-#define RVC_MASK_C_BEQZ		0xe003
-#define RVC_MASK_C_BNEZ		0xe003
+#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)
+#define RVG_MATCH_BGE		(RV_ENCODE_FUNCT3(BGE) | RVG_OPCODE_BRANCH)
+#define RVG_MATCH_BLTU		(RV_ENCODE_FUNCT3(BLTU) | RVG_OPCODE_BRANCH)
+#define RVG_MATCH_BGEU		(RV_ENCODE_FUNCT3(BGEU) | RVG_OPCODE_BRANCH)
+#define RVG_MATCH_SRET		(RV_ENCODE_FUNCT12(SRET) | RVG_OPCODE_SYSTEM)
+#define RVC_MATCH_C_BEQZ	(RVC_ENCODE_FUNCT3(C_BEQZ) | RVC_OPCODE_C1)
+#define RVC_MATCH_C_BNEZ	(RVC_ENCODE_FUNCT3(C_BNEZ) | RVC_OPCODE_C1)
+#define RVC_MATCH_C_J		(RVC_ENCODE_FUNCT3(C_J) | RVC_OPCODE_C1)
+#define RVC_MATCH_C_JAL		(RVC_ENCODE_FUNCT3(C_JAL) | RVC_OPCODE_C1)
+#define RVC_MATCH_C_JR		(RVC_ENCODE_FUNCT4(C_JR) | RVC_OPCODE_C2)
+#define RVC_MATCH_C_JALR	(RVC_ENCODE_FUNCT4(C_JALR) | RVC_OPCODE_C2)
+
+#define RVG_MASK_JALR		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
+#define RVG_MASK_JAL		(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)
+#define RVC_MASK_C_J		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
+#define RVG_MASK_BEQ		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
+#define RVG_MASK_BNE		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
+#define RVG_MASK_BLT		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
+#define RVG_MASK_BGE		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
+#define RVG_MASK_BLTU		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
+#define RVG_MASK_BGEU		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
+#define RVC_MASK_C_BEQZ		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
+#define RVC_MASK_C_BNEZ		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
 #define RVG_MASK_SRET		0xffffffff
 
 #define __INSN_LENGTH_MASK	_UL(0x3)
-- 
2.35.1


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

[PATCH v4 04/12] RISC-V: add ebreak instructions to definitions

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@vrull.eu>

kprobes need to match ebreak instructions, so add the necessary
data to enable us to centralize that functionality.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
 arch/riscv/include/asm/parse_asm.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/riscv/include/asm/parse_asm.h b/arch/riscv/include/asm/parse_asm.h
index e3f87da108f4..e8303250f598 100644
--- a/arch/riscv/include/asm/parse_asm.h
+++ b/arch/riscv/include/asm/parse_asm.h
@@ -144,7 +144,9 @@
 #define RVC_FUNCT3_C_JAL	0x1
 #define RVC_FUNCT4_C_JR		0x8
 #define RVC_FUNCT4_C_JALR	0x9
+#define RVC_FUNCT4_C_EBREAK	0x9
 
+#define RVG_FUNCT12_EBREAK	0x1
 #define RVG_FUNCT12_SRET	0x102
 
 #define RVG_MATCH_JALR		(RV_ENCODE_FUNCT3(JALR) | RVG_OPCODE_JALR)
@@ -155,6 +157,7 @@
 #define RVG_MATCH_BGE		(RV_ENCODE_FUNCT3(BGE) | RVG_OPCODE_BRANCH)
 #define RVG_MATCH_BLTU		(RV_ENCODE_FUNCT3(BLTU) | RVG_OPCODE_BRANCH)
 #define RVG_MATCH_BGEU		(RV_ENCODE_FUNCT3(BGEU) | RVG_OPCODE_BRANCH)
+#define RVG_MATCH_EBREAK	(RV_ENCODE_FUNCT12(EBREAK) | RVG_OPCODE_SYSTEM)
 #define RVG_MATCH_SRET		(RV_ENCODE_FUNCT12(SRET) | RVG_OPCODE_SYSTEM)
 #define RVC_MATCH_C_BEQZ	(RVC_ENCODE_FUNCT3(C_BEQZ) | RVC_OPCODE_C1)
 #define RVC_MATCH_C_BNEZ	(RVC_ENCODE_FUNCT3(C_BNEZ) | RVC_OPCODE_C1)
@@ -162,6 +165,7 @@
 #define RVC_MATCH_C_JAL		(RVC_ENCODE_FUNCT3(C_JAL) | RVC_OPCODE_C1)
 #define RVC_MATCH_C_JR		(RVC_ENCODE_FUNCT4(C_JR) | RVC_OPCODE_C2)
 #define RVC_MATCH_C_JALR	(RVC_ENCODE_FUNCT4(C_JALR) | RVC_OPCODE_C2)
+#define RVC_MATCH_C_EBREAK	(RVC_ENCODE_FUNCT4(C_EBREAK) | RVC_OPCODE_C2)
 
 #define RVG_MASK_JALR		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
 #define RVG_MASK_JAL		(RV_INSN_OPCODE_MASK)
@@ -177,6 +181,8 @@
 #define RVG_MASK_BGEU		(RV_INSN_FUNCT3_MASK | RV_INSN_OPCODE_MASK)
 #define RVC_MASK_C_BEQZ		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
 #define RVC_MASK_C_BNEZ		(RVC_INSN_FUNCT3_MASK | RVC_INSN_OPCODE_MASK)
+#define RVC_MASK_C_EBREAK	0xffff
+#define RVG_MASK_EBREAK		0xffffffff
 #define RVG_MASK_SRET		0xffffffff
 
 #define __INSN_LENGTH_MASK	_UL(0x3)
-- 
2.35.1


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

[PATCH v4 05/12] RISC-V: add auipc elements to parse_asm header

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@vrull.eu>

Instruction parsing should not be done in individual code, but instead
supported by central

Right now kgdb and kprobes parse instructions and at least kprobes (and
the upcoming auipc+jalr alternative fixer-function) need the auipc
instruction.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
 arch/riscv/include/asm/parse_asm.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/riscv/include/asm/parse_asm.h b/arch/riscv/include/asm/parse_asm.h
index e8303250f598..28742eb19034 100644
--- a/arch/riscv/include/asm/parse_asm.h
+++ b/arch/riscv/include/asm/parse_asm.h
@@ -118,6 +118,7 @@
 #define RVC_C2_RD_OPOFF		7
 
 /* parts of opcode for RVG*/
+#define RVG_OPCODE_AUIPC	0x17
 #define RVG_OPCODE_BRANCH	0x63
 #define RVG_OPCODE_JALR		0x67
 #define RVG_OPCODE_JAL		0x6f
@@ -149,6 +150,7 @@
 #define RVG_FUNCT12_EBREAK	0x1
 #define RVG_FUNCT12_SRET	0x102
 
+#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_BEQ		(RV_ENCODE_FUNCT3(BEQ) | RVG_OPCODE_BRANCH)
@@ -167,6 +169,7 @@
 #define RVC_MATCH_C_JALR	(RVC_ENCODE_FUNCT4(C_JALR) | RVC_OPCODE_C2)
 #define RVC_MATCH_C_EBREAK	(RVC_ENCODE_FUNCT4(C_EBREAK) | RVC_OPCODE_C2)
 
+#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 RVC_MASK_C_JALR		(RVC_INSN_FUNCT4_MASK | RVC_INSN_J_RS2_MASK | RVC_INSN_OPCODE_MASK)
-- 
2.35.1


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

[PATCH v4 06/12] RISC-V: Move riscv_insn_is_* macros into a common header

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@vrull.eu>

Right now the riscv kernel has (at least) two independent sets
of functions to check if an encoded instruction is of a specific
type. One in kgdb and one kprobes simulate-insn code.

More parts of the kernel will probably need this in the future,
so instead of allowing this duplication to go on further,
move macros that do the function declaration in a common header,
similar to at least aarch64.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
 arch/riscv/include/asm/parse_asm.h       | 42 ++++++++++++++++----
 arch/riscv/kernel/kgdb.c                 | 49 ++++++++----------------
 arch/riscv/kernel/probes/simulate-insn.h | 26 +++----------
 3 files changed, 55 insertions(+), 62 deletions(-)

diff --git a/arch/riscv/include/asm/parse_asm.h b/arch/riscv/include/asm/parse_asm.h
index 28742eb19034..50c899cf4de5 100644
--- a/arch/riscv/include/asm/parse_asm.h
+++ b/arch/riscv/include/asm/parse_asm.h
@@ -193,13 +193,41 @@
 #define __INSN_OPCODE_MASK	_UL(0x7F)
 #define __INSN_BRANCH_OPCODE	_UL(RVG_OPCODE_BRANCH)
 
-/* Define a series of is_XXX_insn functions to check if the value INSN
- * is an instance of instruction XXX.
- */
-#define DECLARE_INSN(INSN_NAME, INSN_MATCH, INSN_MASK) \
-static inline bool is_ ## INSN_NAME ## _insn(long insn) \
-{ \
-	return (insn & (INSN_MASK)) == (INSN_MATCH); \
+#define __RISCV_INSN_FUNCS(name, mask, val)				\
+static __always_inline bool riscv_insn_is_##name(u32 code)		\
+{									\
+	BUILD_BUG_ON(~(mask) & (val));					\
+	return (code & (mask)) == (val);				\
+}									\
+
+#if __riscv_xlen == 32
+/* C.JAL is an RV32C-only instruction */
+__RISCV_INSN_FUNCS(c_jal, RVC_MASK_C_JAL, RVC_MATCH_C_JAL)
+#else
+#define riscv_insn_is_c_jal(opcode) 0
+#endif
+__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)
+__RISCV_INSN_FUNCS(blt, RVG_MASK_BLT, RVG_MATCH_BLT)
+__RISCV_INSN_FUNCS(bge, RVG_MASK_BGE, RVG_MATCH_BGE)
+__RISCV_INSN_FUNCS(bltu, RVG_MASK_BLTU, RVG_MATCH_BLTU)
+__RISCV_INSN_FUNCS(bgeu, RVG_MASK_BGEU, RVG_MATCH_BGEU)
+__RISCV_INSN_FUNCS(c_beqz, RVC_MASK_C_BEQZ, RVC_MATCH_C_BEQZ)
+__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)
+
+/* special case to catch _any_ branch instruction */
+static __always_inline bool riscv_insn_is_branch(u32 code)
+{
+	return (code & RV_INSN_OPCODE_MASK) == RVG_OPCODE_BRANCH;
 }
 
 #define RV_IMM_SIGN(x) (-(((x) >> 31) & 1))
diff --git a/arch/riscv/kernel/kgdb.c b/arch/riscv/kernel/kgdb.c
index 137c6d870d58..61237aeb493c 100644
--- a/arch/riscv/kernel/kgdb.c
+++ b/arch/riscv/kernel/kgdb.c
@@ -23,27 +23,6 @@ enum {
 static unsigned long stepped_address;
 static unsigned int stepped_opcode;
 
-#if __riscv_xlen == 32
-/* C.JAL is an RV32C-only instruction */
-DECLARE_INSN(c_jal, MATCH_C_JAL, MASK_C_JAL)
-#else
-#define is_c_jal_insn(opcode) 0
-#endif
-DECLARE_INSN(jalr, RVG_MATCH_JALR, RVG_MASK_JALR)
-DECLARE_INSN(jal, RVG_MATCH_JAL, RVG_MASK_JAL)
-DECLARE_INSN(c_jr, RVC_MATCH_C_JR, RVC_MASK_C_JR)
-DECLARE_INSN(c_jalr, RVC_MATCH_C_JALR, RVC_MASK_C_JALR)
-DECLARE_INSN(c_j, RVC_MATCH_C_J, RVC_MASK_C_J)
-DECLARE_INSN(beq, RVG_MATCH_BEQ, RVG_MASK_BEQ)
-DECLARE_INSN(bne, RVG_MATCH_BNE, RVG_MASK_BNE)
-DECLARE_INSN(blt, RVG_MATCH_BLT, RVG_MASK_BLT)
-DECLARE_INSN(bge, RVG_MATCH_BGE, RVG_MASK_BGE)
-DECLARE_INSN(bltu, RVG_MATCH_BLTU, RVG_MASK_BLTU)
-DECLARE_INSN(bgeu, RVG_MATCH_BGEU, RVG_MASK_BGEU)
-DECLARE_INSN(c_beqz, RVC_MATCH_C_BEQZ, RVC_MASK_C_BEQZ)
-DECLARE_INSN(c_bnez, RVC_MATCH_C_BNEZ, RVC_MASK_C_BNEZ)
-DECLARE_INSN(sret, RVG_MATCH_SRET, RVG_MASK_SRET)
-
 static int decode_register_index(unsigned long opcode, int offset)
 {
 	return (opcode >> offset) & 0x1F;
@@ -65,19 +44,21 @@ static int get_step_address(struct pt_regs *regs, unsigned long *next_addr)
 	if (get_kernel_nofault(op_code, (void *)pc))
 		return -EINVAL;
 	if ((op_code & __INSN_LENGTH_MASK) != __INSN_LENGTH_GE_32) {
-		if (is_c_jalr_insn(op_code) || is_c_jr_insn(op_code)) {
+		if (riscv_insn_is_c_jalr(op_code) ||
+		    riscv_insn_is_c_jr(op_code)) {
 			rs1_num = decode_register_index(op_code, RVC_C2_RS1_OPOFF);
 			*next_addr = regs_ptr[rs1_num];
-		} else if (is_c_j_insn(op_code) || is_c_jal_insn(op_code)) {
+		} else if (riscv_insn_is_c_j(op_code) ||
+			   riscv_insn_is_c_jal(op_code)) {
 			*next_addr = RVC_EXTRACT_JTYPE_IMM(op_code) + pc;
-		} else if (is_c_beqz_insn(op_code)) {
+		} else if (riscv_insn_is_c_beqz(op_code)) {
 			rs1_num = decode_register_index_short(op_code,
 							      RVC_C1_RS1_OPOFF);
 			if (!rs1_num || regs_ptr[rs1_num] == 0)
 				*next_addr = RVC_EXTRACT_BTYPE_IMM(op_code) + pc;
 			else
 				*next_addr = pc + 2;
-		} else if (is_c_bnez_insn(op_code)) {
+		} else if (riscv_insn_is_c_bnez(op_code)) {
 			rs1_num =
 			    decode_register_index_short(op_code, RVC_C1_RS1_OPOFF);
 			if (rs1_num && regs_ptr[rs1_num] != 0)
@@ -100,34 +81,34 @@ static int get_step_address(struct pt_regs *regs, unsigned long *next_addr)
 			if (rs2_num)
 				rs2_val = regs_ptr[rs2_num];
 
-			if (is_beq_insn(op_code))
+			if (riscv_insn_is_beq(op_code))
 				result = (rs1_val == rs2_val) ? true : false;
-			else if (is_bne_insn(op_code))
+			else if (riscv_insn_is_bne(op_code))
 				result = (rs1_val != rs2_val) ? true : false;
-			else if (is_blt_insn(op_code))
+			else if (riscv_insn_is_blt(op_code))
 				result =
 				    ((long)rs1_val <
 				     (long)rs2_val) ? true : false;
-			else if (is_bge_insn(op_code))
+			else if (riscv_insn_is_bge(op_code))
 				result =
 				    ((long)rs1_val >=
 				     (long)rs2_val) ? true : false;
-			else if (is_bltu_insn(op_code))
+			else if (riscv_insn_is_bltu(op_code))
 				result = (rs1_val < rs2_val) ? true : false;
-			else if (is_bgeu_insn(op_code))
+			else if (riscv_insn_is_bgeu(op_code))
 				result = (rs1_val >= rs2_val) ? true : false;
 			if (result)
 				*next_addr = imm + pc;
 			else
 				*next_addr = pc + 4;
-		} else if (is_jal_insn(op_code)) {
+		} else if (riscv_insn_is_jal(op_code)) {
 			*next_addr = RV_EXTRACT_JTYPE_IMM(op_code) + pc;
-		} else if (is_jalr_insn(op_code)) {
+		} else if (riscv_insn_is_jalr(op_code)) {
 			rs1_num = decode_register_index(op_code, RVG_RS1_OPOFF);
 			if (rs1_num)
 				*next_addr = ((unsigned long *)regs)[rs1_num];
 			*next_addr += RV_EXTRACT_ITYPE_IMM(op_code);
-		} else if (is_sret_insn(op_code)) {
+		} else if (riscv_insn_is_sret(op_code)) {
 			*next_addr = pc;
 		} else {
 			*next_addr = pc + 4;
diff --git a/arch/riscv/kernel/probes/simulate-insn.h b/arch/riscv/kernel/probes/simulate-insn.h
index cb6ff7dccb92..29fb16cd335c 100644
--- a/arch/riscv/kernel/probes/simulate-insn.h
+++ b/arch/riscv/kernel/probes/simulate-insn.h
@@ -3,14 +3,7 @@
 #ifndef _RISCV_KERNEL_PROBES_SIMULATE_INSN_H
 #define _RISCV_KERNEL_PROBES_SIMULATE_INSN_H
 
-#define __RISCV_INSN_FUNCS(name, mask, val)				\
-static __always_inline bool riscv_insn_is_##name(probe_opcode_t code)	\
-{									\
-	BUILD_BUG_ON(~(mask) & (val));					\
-	return (code & (mask)) == (val);				\
-}									\
-bool simulate_##name(u32 opcode, unsigned long addr,			\
-		     struct pt_regs *regs)
+#include <asm/parse_asm.h>
 
 #define RISCV_INSN_REJECTED(name, code)					\
 	do {								\
@@ -30,18 +23,9 @@ __RISCV_INSN_FUNCS(fence,	0x7f, 0x0f);
 		}							\
 	} while (0)
 
-__RISCV_INSN_FUNCS(c_j,		0xe003, 0xa001);
-__RISCV_INSN_FUNCS(c_jr,	0xf007, 0x8002);
-__RISCV_INSN_FUNCS(c_jal,	0xe003, 0x2001);
-__RISCV_INSN_FUNCS(c_jalr,	0xf007, 0x9002);
-__RISCV_INSN_FUNCS(c_beqz,	0xe003, 0xc001);
-__RISCV_INSN_FUNCS(c_bnez,	0xe003, 0xe001);
-__RISCV_INSN_FUNCS(c_ebreak,	0xffff, 0x9002);
-
-__RISCV_INSN_FUNCS(auipc,	0x7f, 0x17);
-__RISCV_INSN_FUNCS(branch,	0x7f, 0x63);
-
-__RISCV_INSN_FUNCS(jal,		0x7f, 0x6f);
-__RISCV_INSN_FUNCS(jalr,	0x707f, 0x67);
+bool simulate_auipc(u32 opcode, unsigned long addr, struct pt_regs *regs);
+bool simulate_branch(u32 opcode, unsigned long addr, struct pt_regs *regs);
+bool simulate_jal(u32 opcode, unsigned long addr, struct pt_regs *regs);
+bool simulate_jalr(u32 opcode, unsigned long addr, struct pt_regs *regs);
 
 #endif /* _RISCV_KERNEL_PROBES_SIMULATE_INSN_H */
-- 
2.35.1


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

[PATCH v4 07/12] RISC-V: rename parse_asm.h to insn.h

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@vrull.eu>

The current parse_asm header should become a more centralized place
for everything concerning parsing and constructing instructions.

We already have a header insn-def.h similar to aarch64, so rename
parse_asm.h to insn.h (again similar to aarch64) to show that it's
meant for more than simple instruction parsing.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
 arch/riscv/include/asm/{parse_asm.h => insn.h} | 0
 arch/riscv/kernel/kgdb.c                       | 2 +-
 arch/riscv/kernel/probes/simulate-insn.h       | 2 +-
 3 files changed, 2 insertions(+), 2 deletions(-)
 rename arch/riscv/include/asm/{parse_asm.h => insn.h} (100%)

diff --git a/arch/riscv/include/asm/parse_asm.h b/arch/riscv/include/asm/insn.h
similarity index 100%
rename from arch/riscv/include/asm/parse_asm.h
rename to arch/riscv/include/asm/insn.h
diff --git a/arch/riscv/kernel/kgdb.c b/arch/riscv/kernel/kgdb.c
index 61237aeb493c..2e0266ae6bd7 100644
--- a/arch/riscv/kernel/kgdb.c
+++ b/arch/riscv/kernel/kgdb.c
@@ -11,7 +11,7 @@
 #include <linux/string.h>
 #include <asm/cacheflush.h>
 #include <asm/gdb_xml.h>
-#include <asm/parse_asm.h>
+#include <asm/insn.h>
 
 enum {
 	NOT_KGDB_BREAK = 0,
diff --git a/arch/riscv/kernel/probes/simulate-insn.h b/arch/riscv/kernel/probes/simulate-insn.h
index 29fb16cd335c..a19aaa0feb44 100644
--- a/arch/riscv/kernel/probes/simulate-insn.h
+++ b/arch/riscv/kernel/probes/simulate-insn.h
@@ -3,7 +3,7 @@
 #ifndef _RISCV_KERNEL_PROBES_SIMULATE_INSN_H
 #define _RISCV_KERNEL_PROBES_SIMULATE_INSN_H
 
-#include <asm/parse_asm.h>
+#include <asm/insn.h>
 
 #define RISCV_INSN_REJECTED(name, code)					\
 	do {								\
-- 
2.35.1


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

[PATCH v4 08/12] RISC-V: kprobes: use central defined funct3 constants

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@vrull.eu>

Don't redefine values that are already available in the central header
asm/insn.h . Use the values from there instead.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
 arch/riscv/kernel/probes/simulate-insn.c | 19 ++++++-------------
 1 file changed, 6 insertions(+), 13 deletions(-)

diff --git a/arch/riscv/kernel/probes/simulate-insn.c b/arch/riscv/kernel/probes/simulate-insn.c
index d73e96f6ed7c..330afe9331a8 100644
--- a/arch/riscv/kernel/probes/simulate-insn.c
+++ b/arch/riscv/kernel/probes/simulate-insn.c
@@ -136,13 +136,6 @@ bool __kprobes simulate_auipc(u32 opcode, unsigned long addr, struct pt_regs *re
 #define branch_offset(opcode) \
 	sign_extend32((branch_imm(opcode)), 12)
 
-#define BRANCH_BEQ	0x0
-#define BRANCH_BNE	0x1
-#define BRANCH_BLT	0x4
-#define BRANCH_BGE	0x5
-#define BRANCH_BLTU	0x6
-#define BRANCH_BGEU	0x7
-
 bool __kprobes simulate_branch(u32 opcode, unsigned long addr, struct pt_regs *regs)
 {
 	/*
@@ -169,22 +162,22 @@ bool __kprobes simulate_branch(u32 opcode, unsigned long addr, struct pt_regs *r
 
 	offset_tmp = branch_offset(opcode);
 	switch (branch_funct3(opcode)) {
-	case BRANCH_BEQ:
+	case RVG_FUNCT3_BEQ:
 		offset = (rs1_val == rs2_val) ? offset_tmp : 4;
 		break;
-	case BRANCH_BNE:
+	case RVG_FUNCT3_BNE:
 		offset = (rs1_val != rs2_val) ? offset_tmp : 4;
 		break;
-	case BRANCH_BLT:
+	case RVG_FUNCT3_BLT:
 		offset = ((long)rs1_val < (long)rs2_val) ? offset_tmp : 4;
 		break;
-	case BRANCH_BGE:
+	case RVG_FUNCT3_BGE:
 		offset = ((long)rs1_val >= (long)rs2_val) ? offset_tmp : 4;
 		break;
-	case BRANCH_BLTU:
+	case RVG_FUNCT3_BLTU:
 		offset = (rs1_val < rs2_val) ? offset_tmp : 4;
 		break;
-	case BRANCH_BGEU:
+	case RVG_FUNCT3_BGEU:
 		offset = (rs1_val >= rs2_val) ? offset_tmp : 4;
 		break;
 	default:
-- 
2.35.1


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

[PATCH v4 09/12] RISC-V: add U-type imm parsing to insn.h header

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@vrull.eu>

Similar to other existing types, allow extracting the immediate
for a U-type instruction.

U-type immediates are special in that regard, that the value
in the instruction in bits [31:12] already represents the same
bits of the immediate, so no shifting is required.

U-type immediates are for example used in the auipc instruction,
so these constants make it easier to parse such instructions.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
 arch/riscv/include/asm/insn.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
index 50c899cf4de5..21ec817abec1 100644
--- a/arch/riscv/include/asm/insn.h
+++ b/arch/riscv/include/asm/insn.h
@@ -34,6 +34,15 @@
 #define RV_J_IMM_11_MASK	GENMASK(0, 0)
 #define RV_J_IMM_19_12_MASK	GENMASK(7, 0)
 
+/*
+ * U-type IMMs contain the upper 20bits [31:20] of an immediate with
+ * the rest filled in by zeros, so no shifting required. Similarly,
+ * bit31 contains the signed state, so no sign extension necessary.
+ */
+#define RV_U_IMM_SIGN_OPOFF	31
+#define RV_U_IMM_31_12_OPOFF	0
+#define RV_U_IMM_31_12_MASK	GENMASK(31, 12)
+
 /* The bit field of immediate value in B-type instruction */
 #define RV_B_IMM_SIGN_OPOFF	31
 #define RV_B_IMM_10_5_OPOFF	25
@@ -235,6 +244,10 @@ static __always_inline bool riscv_insn_is_branch(u32 code)
 #define RV_X(X, s, mask)  (((X) >> (s)) & (mask))
 #define RVC_X(X, s, mask) RV_X(X, s, mask)
 
+#define RV_EXTRACT_UTYPE_IMM(x) \
+	({typeof(x) x_ = (x); \
+	(RV_X(x_, RV_U_IMM_31_12_OPOFF, RV_U_IMM_31_12_MASK)); })
+
 #define RV_EXTRACT_JTYPE_IMM(x) \
 	({typeof(x) x_ = (x); \
 	(RV_X(x_, RV_J_IMM_10_1_OPOFF, RV_J_IMM_10_1_MASK) << RV_J_IMM_10_1_OFF) | \
-- 
2.35.1


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

[PATCH v4 10/12] RISC-V: add rd reg parsing to insn.h header

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@vrull.eu>

Add a macro to allow parsing of the rd register from an instruction.

Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
 arch/riscv/include/asm/insn.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
index 21ec817abec1..2a23890b4577 100644
--- a/arch/riscv/include/asm/insn.h
+++ b/arch/riscv/include/asm/insn.h
@@ -60,6 +60,7 @@
 #define RVG_RS1_OPOFF		15
 #define RVG_RS2_OPOFF		20
 #define RVG_RD_OPOFF		7
+#define RVG_RD_MASK		GENMASK(4, 0)
 
 /* The bit field of immediate value in RVC J instruction */
 #define RVC_J_IMM_SIGN_OPOFF	12
@@ -244,6 +245,10 @@ static __always_inline bool riscv_insn_is_branch(u32 code)
 #define RV_X(X, s, mask)  (((X) >> (s)) & (mask))
 #define RVC_X(X, s, mask) RV_X(X, s, mask)
 
+#define RV_EXTRACT_RD_REG(x) \
+	({typeof(x) x_ = (x); \
+	(RV_X(x_, RVG_RD_OPOFF, RVG_RD_MASK)); })
+
 #define RV_EXTRACT_UTYPE_IMM(x) \
 	({typeof(x) x_ = (x); \
 	(RV_X(x_, RV_U_IMM_31_12_OPOFF, RV_U_IMM_31_12_MASK)); })
-- 
2.35.1


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

[PATCH v4 11/12] RISC-V: add helpers for handling immediates in U-type and I-type pairs

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@vrull.eu>

Used together U-type and I-type instructions can for example be used to
generate bigger jumps (i.e. in auipc+jalr pairs) by splitting the value
into an upper immediate (i.e. auipc) and a 12bit immediate (i.e. jalr).

Due to both immediates being considered signed this creates some corner
cases, so add some helper to prevent this from getting duplicated in
different places.

Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
 arch/riscv/include/asm/insn.h | 47 +++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
index 2a23890b4577..bb1e6120a560 100644
--- a/arch/riscv/include/asm/insn.h
+++ b/arch/riscv/include/asm/insn.h
@@ -290,3 +290,50 @@ static __always_inline bool riscv_insn_is_branch(u32 code)
 	(RVC_X(x_, RVC_B_IMM_5_OPOFF, RVC_B_IMM_5_MASK) << RVC_B_IMM_5_OFF) | \
 	(RVC_X(x_, RVC_B_IMM_7_6_OPOFF, RVC_B_IMM_7_6_MASK) << RVC_B_IMM_7_6_OFF) | \
 	(RVC_IMM_SIGN(x_) << RVC_B_IMM_SIGN_OFF); })
+
+/*
+ * Put together one immediate from a U-type and I-type instruction pair.
+ *
+ * The U-type contains an upper immediate, meaning bits[31:12] with [11:0]
+ * being zero, while the I-type contains a 12bit immediate.
+ * Combined these can encode larger 32bit values and are used for example
+ * in auipc + jalr pairs to allow larger jumps.
+ *
+ * @utype_insn: instruction containing the upper immediate
+ * @itype_insn: instruction
+ * Return: combined immediate
+ */
+static inline s32 riscv_insn_extract_utype_itype_imm(u32 utype_insn, u32 itype_insn)
+{
+	s32 imm;
+
+	imm = RV_EXTRACT_UTYPE_IMM(utype_insn);
+	imm += RV_EXTRACT_ITYPE_IMM(itype_insn);
+
+	return imm;
+}
+
+/*
+ * Update a set of two instructions (U-type + I-type) with an immediate value.
+ *
+ * Used for example in auipc+jalrs pairs the U-type instructions contains
+ * a 20bit upper immediate representing bits[31:12], while the I-type
+ * instruction contains a 12bit immediate representing bits[11:0].
+ *
+ * This also takes into account that both separate immediates are
+ * considered as signed values, so if the I-type immediate becomes
+ * negative (BIT(11) set) the U-type part gets adjusted.
+ *
+ * @insn: pointer to a set of two instructions
+ * @imm: the immediate to insert into the two instructions
+ */
+static inline void riscv_insn_insert_utype_itype_imm(u32 *insn, s32 imm)
+{
+	/* drop possible old IMM values */
+	insn[0] &= ~(RV_U_IMM_31_12_MASK);
+	insn[1] &= ~(RV_I_IMM_11_0_MASK << RV_I_IMM_11_0_OPOFF);
+
+	/* add the adapted IMMs */
+	insn[0] |= ((imm & RV_U_IMM_31_12_MASK) + ((imm & BIT(11)) << 1));
+	insn[1] |= ((imm & RV_I_IMM_11_0_MASK) << RV_I_IMM_11_0_OPOFF);
+}
-- 
2.35.1


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

[PATCH v4 12/12] RISC-V: fix auipc-jalr addresses in patched alternatives

[Heiko Stuebner]

From: Heiko Stuebner <heiko.stuebner@vrull.eu>

Alternatives live in a different section, so addresses used by call
functions will point to wrong locations after the patch got applied.

Similar to arm64, adjust the location to consider that offset.

Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
---
 arch/riscv/include/asm/alternative.h |  3 ++
 arch/riscv/kernel/alternative.c      | 56 ++++++++++++++++++++++++++++
 arch/riscv/kernel/cpufeature.c       |  5 ++-
 3 files changed, 63 insertions(+), 1 deletion(-)

diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
index 6511dd73e812..1bd4027d34ca 100644
--- a/arch/riscv/include/asm/alternative.h
+++ b/arch/riscv/include/asm/alternative.h
@@ -27,6 +27,9 @@ void __init apply_boot_alternatives(void);
 void __init apply_early_boot_alternatives(void);
 void apply_module_alternatives(void *start, size_t length);
 
+void riscv_alternative_fix_offsets(void *alt_ptr, unsigned int len,
+				   int patch_offset);
+
 struct alt_entry {
 	void *old_ptr;		 /* address of original instruciton or data  */
 	void *alt_ptr;		 /* address of replacement instruction or data */
diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
index a7d26a00beea..c29e198ed9df 100644
--- a/arch/riscv/kernel/alternative.c
+++ b/arch/riscv/kernel/alternative.c
@@ -15,6 +15,8 @@
 #include <asm/vendorid_list.h>
 #include <asm/sbi.h>
 #include <asm/csr.h>
+#include <asm/insn.h>
+#include <asm/patch.h>
 
 struct cpu_manufacturer_info_t {
 	unsigned long vendor_id;
@@ -53,6 +55,60 @@ static void __init_or_module riscv_fill_cpu_mfr_info(struct cpu_manufacturer_inf
 	}
 }
 
+static u32 riscv_instruction_at(void *p)
+{
+	u16 *parcel = p;
+
+	return (unsigned int)parcel[0] | (unsigned int)parcel[1] << 16;
+}
+
+static void riscv_alternative_fix_auipc_jalr(void *ptr, u32 insn1, u32 insn2, int patch_offset)
+{
+	/* pick the original auipc + jalr */
+	u32 call[2] = { insn1, insn2 };
+	s32 imm;
+
+	/* get and adjust new target address */
+	imm = riscv_insn_extract_utype_itype_imm(insn1, insn2);
+	imm -= patch_offset;
+
+	/* update instructions */
+	riscv_insn_insert_utype_itype_imm(call, imm);
+
+	/* patch the call place again */
+	patch_text_nosync(ptr, call, sizeof(u32) * 2);
+}
+
+void riscv_alternative_fix_offsets(void *alt_ptr, unsigned int len,
+				      int patch_offset)
+{
+	int num_instr = len / sizeof(u32);
+	int i;
+
+	/*
+	 * stop one instruction before the end, as we're checking
+	 * for auipc + jalr
+	 */
+	for (i = 0; i < num_instr; i++) {
+		u32 inst = riscv_instruction_at(alt_ptr + i * sizeof(u32));
+
+		/* may be the start of an auipc + jalr pair */
+		if (riscv_insn_is_auipc(inst) && i < num_instr - 1) {
+			u32 inst2 = riscv_instruction_at(alt_ptr + (i + 1) * sizeof(u32));
+
+			if (!riscv_insn_is_jalr(inst2))
+				continue;
+
+			/* call will use ra register */
+			if (RV_EXTRACT_RD_REG(inst) != 1)
+				continue;
+
+			riscv_alternative_fix_auipc_jalr(alt_ptr + i * sizeof(u32),
+							 inst, inst2, patch_offset);
+		}
+	}
+}
+
 /*
  * This is called very early in the boot process (directly after we run
  * a feature detect on the boot CPU). No need to worry about other CPUs
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 694267d1fe81..e91ec3d8e240 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -316,8 +316,11 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
 		}
 
 		tmp = (1U << alt->errata_id);
-		if (cpu_req_feature & tmp)
+		if (cpu_req_feature & tmp) {
 			patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
+			riscv_alternative_fix_offsets(alt->old_ptr, alt->alt_len,
+						      alt->old_ptr - alt->alt_ptr);
+		}
 	}
 }
 #endif
-- 
2.35.1


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

Re: [PATCH v4 11/12] RISC-V: add helpers for handling immediates in U-type and I-type pairs

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 3418 bytes --]

On Wed, Dec 07, 2022 at 07:08:20PM +0100, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> 
> Used together U-type and I-type instructions can for example be used to
> generate bigger jumps (i.e. in auipc+jalr pairs) by splitting the value
> into an upper immediate (i.e. auipc) and a 12bit immediate (i.e. jalr).
> 
> Due to both immediates being considered signed this creates some corner
> cases, so add some helper to prevent this from getting duplicated in
> different places.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/insn.h | 47 +++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
> index 2a23890b4577..bb1e6120a560 100644
> --- a/arch/riscv/include/asm/insn.h
> +++ b/arch/riscv/include/asm/insn.h
> @@ -290,3 +290,50 @@ static __always_inline bool riscv_insn_is_branch(u32 code)
>  	(RVC_X(x_, RVC_B_IMM_5_OPOFF, RVC_B_IMM_5_MASK) << RVC_B_IMM_5_OFF) | \
>  	(RVC_X(x_, RVC_B_IMM_7_6_OPOFF, RVC_B_IMM_7_6_MASK) << RVC_B_IMM_7_6_OFF) | \
>  	(RVC_IMM_SIGN(x_) << RVC_B_IMM_SIGN_OFF); })
> +
> +/*
> + * Put together one immediate from a U-type and I-type instruction pair.
> + *
> + * The U-type contains an upper immediate, meaning bits[31:12] with [11:0]
> + * being zero, while the I-type contains a 12bit immediate.
> + * Combined these can encode larger 32bit values and are used for example
> + * in auipc + jalr pairs to allow larger jumps.
> + *
> + * @utype_insn: instruction containing the upper immediate
> + * @itype_insn: instruction
> + * Return: combined immediate
> + */
> +static inline s32 riscv_insn_extract_utype_itype_imm(u32 utype_insn, u32 itype_insn)
> +{
> +	s32 imm;
> +
> +	imm = RV_EXTRACT_UTYPE_IMM(utype_insn);
> +	imm += RV_EXTRACT_ITYPE_IMM(itype_insn);
> +
> +	return imm;
> +}
> +
> +/*
> + * Update a set of two instructions (U-type + I-type) with an immediate value.
> + *
> + * Used for example in auipc+jalrs pairs the U-type instructions contains
> + * a 20bit upper immediate representing bits[31:12], while the I-type
> + * instruction contains a 12bit immediate representing bits[11:0].
> + *
> + * This also takes into account that both separate immediates are
> + * considered as signed values, so if the I-type immediate becomes
> + * negative (BIT(11) set) the U-type part gets adjusted.
> + *
> + * @insn: pointer to a set of two instructions
> + * @imm: the immediate to insert into the two instructions
> + */
> +static inline void riscv_insn_insert_utype_itype_imm(u32 *insn, s32 imm)
> +{
> +	/* drop possible old IMM values */
> +	insn[0] &= ~(RV_U_IMM_31_12_MASK);
> +	insn[1] &= ~(RV_I_IMM_11_0_MASK << RV_I_IMM_11_0_OPOFF);
> +
> +	/* add the adapted IMMs */
> +	insn[0] |= ((imm & RV_U_IMM_31_12_MASK) + ((imm & BIT(11)) << 1));
> +	insn[1] |= ((imm & RV_I_IMM_11_0_MASK) << RV_I_IMM_11_0_OPOFF);

If you send a v5, could you drop the extra ()s around some of these?
Otherwise:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
I do like the comments a lot :) Saves going off to read specs...

> +}
> -- 
> 2.35.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

Re: [PATCH v4 12/12] RISC-V: fix auipc-jalr addresses in patched alternatives

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 5555 bytes --]

On Wed, Dec 07, 2022 at 07:08:21PM +0100, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> 
> Alternatives live in a different section, so addresses used by call
> functions will point to wrong locations after the patch got applied.
> 
> Similar to arm64, adjust the location to consider that offset.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/alternative.h |  3 ++
>  arch/riscv/kernel/alternative.c      | 56 ++++++++++++++++++++++++++++
>  arch/riscv/kernel/cpufeature.c       |  5 ++-
>  3 files changed, 63 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> index 6511dd73e812..1bd4027d34ca 100644
> --- a/arch/riscv/include/asm/alternative.h
> +++ b/arch/riscv/include/asm/alternative.h
> @@ -27,6 +27,9 @@ void __init apply_boot_alternatives(void);
>  void __init apply_early_boot_alternatives(void);
>  void apply_module_alternatives(void *start, size_t length);
>  
> +void riscv_alternative_fix_offsets(void *alt_ptr, unsigned int len,
> +				   int patch_offset);
> +
>  struct alt_entry {
>  	void *old_ptr;		 /* address of original instruciton or data  */
>  	void *alt_ptr;		 /* address of replacement instruction or data */
> diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
> index a7d26a00beea..c29e198ed9df 100644
> --- a/arch/riscv/kernel/alternative.c
> +++ b/arch/riscv/kernel/alternative.c
> @@ -15,6 +15,8 @@
>  #include <asm/vendorid_list.h>
>  #include <asm/sbi.h>
>  #include <asm/csr.h>
> +#include <asm/insn.h>
> +#include <asm/patch.h>
>  
>  struct cpu_manufacturer_info_t {
>  	unsigned long vendor_id;
> @@ -53,6 +55,60 @@ static void __init_or_module riscv_fill_cpu_mfr_info(struct cpu_manufacturer_inf
>  	}
>  }
>  
> +static u32 riscv_instruction_at(void *p)
> +{
> +	u16 *parcel = p;
> +
> +	return (unsigned int)parcel[0] | (unsigned int)parcel[1] << 16;

I feel bad for not mentioning this before - can we replace this magic 16
with something self documenting?

> +}
> +
> +static void riscv_alternative_fix_auipc_jalr(void *ptr, u32 insn1, u32 insn2, int patch_offset)
> +{
> +	/* pick the original auipc + jalr */
> +	u32 call[2] = { insn1, insn2 };
> +	s32 imm;
> +
> +	/* get and adjust new target address */
> +	imm = riscv_insn_extract_utype_itype_imm(insn1, insn2);
> +	imm -= patch_offset;
> +
> +	/* update instructions */
> +	riscv_insn_insert_utype_itype_imm(call, imm);
> +
> +	/* patch the call place again */
> +	patch_text_nosync(ptr, call, sizeof(u32) * 2);
> +}

Obv. I have not left R-b tags on stuff without trying to understand
what's going on, but from a lay-observer point of view, I think these
function names & flow does a good job of explaining some of the black
magic in this neck of the woods.

> +
> +void riscv_alternative_fix_offsets(void *alt_ptr, unsigned int len,
> +				      int patch_offset)
> +{
> +	int num_instr = len / sizeof(u32);

instr...

> +	int i;
> +
> +	/*
> +	 * stop one instruction before the end, as we're checking
> +	 * for auipc + jalr
> +	 */
> +	for (i = 0; i < num_instr; i++) {
> +		u32 inst = riscv_instruction_at(alt_ptr + i * sizeof(u32));

...inst...

> +
> +		/* may be the start of an auipc + jalr pair */
> +		if (riscv_insn_is_auipc(inst) && i < num_instr - 1) {

...insn.

Is there a reason for that?

Also, I've gotten myself slightly confused about the loop. You "stop one
instruction before the end" but the main loop goes from 0 -> num_instr.
The inner loop then checks for i < num_instr - 1. What am I missing that
prevents the outer loop from stopping at num_instr - 1 instead?
> +			u32 inst2 = riscv_instruction_at(alt_ptr + (i + 1) * sizeof(u32));
> +
> +			if (!riscv_insn_is_jalr(inst2))
> +				continue;
> +
> +			/* call will use ra register */

Super minor, but "call will use ra register" is a little unclear. As
written, it makes perfect sense when you've been staring at this code,
but not so much if you're passing through.. How about:
/* if this instruction pair is a call, it will use the ra register */

> +			if (RV_EXTRACT_RD_REG(inst) != 1)
> +				continue;
>

All minor stuff though, so you can re-add my R-b either way:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

Thanks,
Conor.

> +			riscv_alternative_fix_auipc_jalr(alt_ptr + i * sizeof(u32),
> +							 inst, inst2, patch_offset);
> +		}
> +	}
> +}
> +
>  /*
>   * This is called very early in the boot process (directly after we run
>   * a feature detect on the boot CPU). No need to worry about other CPUs
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 694267d1fe81..e91ec3d8e240 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -316,8 +316,11 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
>  		}
>  
>  		tmp = (1U << alt->errata_id);
> -		if (cpu_req_feature & tmp)
> +		if (cpu_req_feature & tmp) {
>  			patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> +			riscv_alternative_fix_offsets(alt->old_ptr, alt->alt_len,
> +						      alt->old_ptr - alt->alt_ptr);
> +		}
>  	}
>  }
>  #endif
> -- 
> 2.35.1
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

Re: [PATCH v4 12/12] RISC-V: fix auipc-jalr addresses in patched alternatives

[Jessica Clarke]

On 7 Dec 2022, at 20:48, Conor Dooley <conor@kernel.org> wrote:
> 
> On Wed, Dec 07, 2022 at 07:08:21PM +0100, Heiko Stuebner wrote:
>> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>> 
>> Alternatives live in a different section, so addresses used by call
>> functions will point to wrong locations after the patch got applied.
>> 
>> Similar to arm64, adjust the location to consider that offset.
>> 
>> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
>> ---
>> arch/riscv/include/asm/alternative.h |  3 ++
>> arch/riscv/kernel/alternative.c      | 56 ++++++++++++++++++++++++++++
>> arch/riscv/kernel/cpufeature.c       |  5 ++-
>> 3 files changed, 63 insertions(+), 1 deletion(-)
>> 
>> diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
>> index 6511dd73e812..1bd4027d34ca 100644
>> --- a/arch/riscv/include/asm/alternative.h
>> +++ b/arch/riscv/include/asm/alternative.h
>> @@ -27,6 +27,9 @@ void __init apply_boot_alternatives(void);
>> void __init apply_early_boot_alternatives(void);
>> void apply_module_alternatives(void *start, size_t length);
>> 
>> +void riscv_alternative_fix_offsets(void *alt_ptr, unsigned int len,
>> +				   int patch_offset);
>> +
>> struct alt_entry {
>> 	void *old_ptr;		 /* address of original instruciton or data  */
>> 	void *alt_ptr;		 /* address of replacement instruction or data */
>> diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
>> index a7d26a00beea..c29e198ed9df 100644
>> --- a/arch/riscv/kernel/alternative.c
>> +++ b/arch/riscv/kernel/alternative.c
>> @@ -15,6 +15,8 @@
>> #include <asm/vendorid_list.h>
>> #include <asm/sbi.h>
>> #include <asm/csr.h>
>> +#include <asm/insn.h>
>> +#include <asm/patch.h>
>> 
>> struct cpu_manufacturer_info_t {
>> 	unsigned long vendor_id;
>> @@ -53,6 +55,60 @@ static void __init_or_module riscv_fill_cpu_mfr_info(struct cpu_manufacturer_inf
>> 	}
>> }
>> 
>> +static u32 riscv_instruction_at(void *p)
>> +{
>> +	u16 *parcel = p;
>> +
>> +	return (unsigned int)parcel[0] | (unsigned int)parcel[1] << 16;
> 
> I feel bad for not mentioning this before - can we replace this magic 16
> with something self documenting?

I mean, it’s a u16 * two lines above, that seems a bit unnecessary... I'd
argue giving it a name would be worse than not.

Though these unsigned ints should be u32 to match the return type.

Jess

>> +}
>> +
>> +static void riscv_alternative_fix_auipc_jalr(void *ptr, u32 insn1, u32 insn2, int patch_offset)
>> +{
>> +	/* pick the original auipc + jalr */
>> +	u32 call[2] = { insn1, insn2 };
>> +	s32 imm;
>> +
>> +	/* get and adjust new target address */
>> +	imm = riscv_insn_extract_utype_itype_imm(insn1, insn2);
>> +	imm -= patch_offset;
>> +
>> +	/* update instructions */
>> +	riscv_insn_insert_utype_itype_imm(call, imm);
>> +
>> +	/* patch the call place again */
>> +	patch_text_nosync(ptr, call, sizeof(u32) * 2);
>> +}
> 
> Obv. I have not left R-b tags on stuff without trying to understand
> what's going on, but from a lay-observer point of view, I think these
> function names & flow does a good job of explaining some of the black
> magic in this neck of the woods.
> 
>> +
>> +void riscv_alternative_fix_offsets(void *alt_ptr, unsigned int len,
>> +				      int patch_offset)
>> +{
>> +	int num_instr = len / sizeof(u32);
> 
> instr...
> 
>> +	int i;
>> +
>> +	/*
>> +	 * stop one instruction before the end, as we're checking
>> +	 * for auipc + jalr
>> +	 */
>> +	for (i = 0; i < num_instr; i++) {
>> +		u32 inst = riscv_instruction_at(alt_ptr + i * sizeof(u32));
> 
> ...inst...
> 
>> +
>> +		/* may be the start of an auipc + jalr pair */
>> +		if (riscv_insn_is_auipc(inst) && i < num_instr - 1) {
> 
> ...insn.
> 
> Is there a reason for that?
> 
> Also, I've gotten myself slightly confused about the loop. You "stop one
> instruction before the end" but the main loop goes from 0 -> num_instr.
> The inner loop then checks for i < num_instr - 1. What am I missing that
> prevents the outer loop from stopping at num_instr - 1 instead?
>> +			u32 inst2 = riscv_instruction_at(alt_ptr + (i + 1) * sizeof(u32));
>> +
>> +			if (!riscv_insn_is_jalr(inst2))
>> +				continue;
>> +
>> +			/* call will use ra register */
> 
> Super minor, but "call will use ra register" is a little unclear. As
> written, it makes perfect sense when you've been staring at this code,
> but not so much if you're passing through.. How about:
> /* if this instruction pair is a call, it will use the ra register */
> 
>> +			if (RV_EXTRACT_RD_REG(inst) != 1)
>> +				continue;
>> 
> 
> All minor stuff though, so you can re-add my R-b either way:
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> 
> Thanks,
> Conor.
> 
>> +			riscv_alternative_fix_auipc_jalr(alt_ptr + i * sizeof(u32),
>> +							 inst, inst2, patch_offset);
>> +		}
>> +	}
>> +}
>> +
>> /*
>>  * This is called very early in the boot process (directly after we run
>>  * a feature detect on the boot CPU). No need to worry about other CPUs
>> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
>> index 694267d1fe81..e91ec3d8e240 100644
>> --- a/arch/riscv/kernel/cpufeature.c
>> +++ b/arch/riscv/kernel/cpufeature.c
>> @@ -316,8 +316,11 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
>> 		}
>> 
>> 		tmp = (1U << alt->errata_id);
>> -		if (cpu_req_feature & tmp)
>> +		if (cpu_req_feature & tmp) {
>> 			patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
>> +			riscv_alternative_fix_offsets(alt->old_ptr, alt->alt_len,
>> +						      alt->old_ptr - alt->alt_ptr);
>> +		}
>> 	}
>> }
>> #endif
>> -- 
>> 2.35.1
>> 
>> 
>> _______________________________________________
>> linux-riscv mailing list
>> linux-riscv@lists.infradead.org
>> http://lists.infradead.org/mailman/listinfo/linux-riscv
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv


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

Re: [PATCH v4 12/12] RISC-V: fix auipc-jalr addresses in patched alternatives

[Conor Dooley]

[-- Attachment #1.1: Type: text/plain, Size: 527 bytes --]

On Wed, Dec 07, 2022 at 10:00:13PM +0000, Jessica Clarke wrote:

> >> +static u32 riscv_instruction_at(void *p)
> >> +{
> >> +	u16 *parcel = p;
> >> +
> >> +	return (unsigned int)parcel[0] | (unsigned int)parcel[1] << 16;
> > 
> > I feel bad for not mentioning this before - can we replace this magic 16
> > with something self documenting?
> 
> I mean, it’s a u16 * two lines above, that seems a bit unnecessary...

I guess that's why I didn't mention it before... Can safely disregard
that request so Heiko!

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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

Re: [PATCH v4 12/12] RISC-V: fix auipc-jalr addresses in patched alternatives

[Heiko Stuebner]

Am Mittwoch, 7. Dezember 2022, 21:48:08 CET schrieb Conor Dooley:
> On Wed, Dec 07, 2022 at 07:08:21PM +0100, Heiko Stuebner wrote:
> > From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> > 
> > Alternatives live in a different section, so addresses used by call
> > functions will point to wrong locations after the patch got applied.
> > 
> > Similar to arm64, adjust the location to consider that offset.
> > 
> > Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> > ---
> >  arch/riscv/include/asm/alternative.h |  3 ++
> >  arch/riscv/kernel/alternative.c      | 56 ++++++++++++++++++++++++++++
> >  arch/riscv/kernel/cpufeature.c       |  5 ++-
> >  3 files changed, 63 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> > index 6511dd73e812..1bd4027d34ca 100644
> > --- a/arch/riscv/include/asm/alternative.h
> > +++ b/arch/riscv/include/asm/alternative.h
> > @@ -27,6 +27,9 @@ void __init apply_boot_alternatives(void);
> >  void __init apply_early_boot_alternatives(void);
> >  void apply_module_alternatives(void *start, size_t length);
> >  
> > +void riscv_alternative_fix_offsets(void *alt_ptr, unsigned int len,
> > +				   int patch_offset);
> > +
> >  struct alt_entry {
> >  	void *old_ptr;		 /* address of original instruciton or data  */
> >  	void *alt_ptr;		 /* address of replacement instruction or data */
> > diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
> > index a7d26a00beea..c29e198ed9df 100644
> > --- a/arch/riscv/kernel/alternative.c
> > +++ b/arch/riscv/kernel/alternative.c
> > @@ -15,6 +15,8 @@
> >  #include <asm/vendorid_list.h>
> >  #include <asm/sbi.h>
> >  #include <asm/csr.h>
> > +#include <asm/insn.h>
> > +#include <asm/patch.h>
> >  
> >  struct cpu_manufacturer_info_t {
> >  	unsigned long vendor_id;
> > @@ -53,6 +55,60 @@ static void __init_or_module riscv_fill_cpu_mfr_info(struct cpu_manufacturer_inf
> >  	}
> >  }
> >  
> > +static u32 riscv_instruction_at(void *p)
> > +{
> > +	u16 *parcel = p;
> > +
> > +	return (unsigned int)parcel[0] | (unsigned int)parcel[1] << 16;
> 
> I feel bad for not mentioning this before - can we replace this magic 16
> with something self documenting?
> 
> > +}
> > +
> > +static void riscv_alternative_fix_auipc_jalr(void *ptr, u32 insn1, u32 insn2, int patch_offset)
> > +{
> > +	/* pick the original auipc + jalr */
> > +	u32 call[2] = { insn1, insn2 };
> > +	s32 imm;
> > +
> > +	/* get and adjust new target address */
> > +	imm = riscv_insn_extract_utype_itype_imm(insn1, insn2);
> > +	imm -= patch_offset;
> > +
> > +	/* update instructions */
> > +	riscv_insn_insert_utype_itype_imm(call, imm);
> > +
> > +	/* patch the call place again */
> > +	patch_text_nosync(ptr, call, sizeof(u32) * 2);
> > +}
> 
> Obv. I have not left R-b tags on stuff without trying to understand
> what's going on, but from a lay-observer point of view, I think these
> function names & flow does a good job of explaining some of the black
> magic in this neck of the woods.
> 
> > +
> > +void riscv_alternative_fix_offsets(void *alt_ptr, unsigned int len,
> > +				      int patch_offset)
> > +{
> > +	int num_instr = len / sizeof(u32);
> 
> instr...
> 
> > +	int i;
> > +
> > +	/*
> > +	 * stop one instruction before the end, as we're checking
> > +	 * for auipc + jalr
> > +	 */
> > +	for (i = 0; i < num_instr; i++) {
> > +		u32 inst = riscv_instruction_at(alt_ptr + i * sizeof(u32));
> 
> ...inst...
> 
> > +
> > +		/* may be the start of an auipc + jalr pair */
> > +		if (riscv_insn_is_auipc(inst) && i < num_instr - 1) {
> 
> ...insn.
> 
> Is there a reason for that?

I guess more of a generational issue - with the code spanning
too much time :-)

So poll question, what would be preferred?
I think I remember seeing all of them somewhere, so I'm unsure what
to standardize on.

> 
> Also, I've gotten myself slightly confused about the loop. You "stop one
> instruction before the end" but the main loop goes from 0 -> num_instr.
> The inner loop then checks for i < num_instr - 1. What am I missing that
> prevents the outer loop from stopping at num_instr - 1 instead?

The idea with this is to allow a
	if(riscv_insn_is_jal(inst))
		riscv_alternative_fix_jal(...)

etc, and everything else is a single instruction so needs one more
loop iteration, only for auipc+jalr do we want to stop one earlier.

So to get this alternatives_fix_offsets() main entry point I
made the loop do the one iteration more again.


> > +			u32 inst2 = riscv_instruction_at(alt_ptr + (i + 1) * sizeof(u32));
> > +
> > +			if (!riscv_insn_is_jalr(inst2))
> > +				continue;
> > +
> > +			/* call will use ra register */
> 
> Super minor, but "call will use ra register" is a little unclear. As
> written, it makes perfect sense when you've been staring at this code,
> but not so much if you're passing through.. How about:
> /* if this instruction pair is a call, it will use the ra register */

sure :-)

> 
> > +			if (RV_EXTRACT_RD_REG(inst) != 1)
> > +				continue;
> >
> 
> All minor stuff though, so you can re-add my R-b either way:
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

thanks :-)

Heiko



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

Re: [PATCH v4 11/12] RISC-V: add helpers for handling immediates in U-type and I-type pairs

[Heiko Stuebner]

Am Mittwoch, 7. Dezember 2022, 21:07:39 CET schrieb Conor Dooley:
> On Wed, Dec 07, 2022 at 07:08:20PM +0100, Heiko Stuebner wrote:
> > From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> > 
> > Used together U-type and I-type instructions can for example be used to
> > generate bigger jumps (i.e. in auipc+jalr pairs) by splitting the value
> > into an upper immediate (i.e. auipc) and a 12bit immediate (i.e. jalr).
> > 
> > Due to both immediates being considered signed this creates some corner
> > cases, so add some helper to prevent this from getting duplicated in
> > different places.
> > 
> > Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> > ---
> >  arch/riscv/include/asm/insn.h | 47 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> > 
> > diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
> > index 2a23890b4577..bb1e6120a560 100644
> > --- a/arch/riscv/include/asm/insn.h
> > +++ b/arch/riscv/include/asm/insn.h
> > @@ -290,3 +290,50 @@ static __always_inline bool riscv_insn_is_branch(u32 code)
> >  	(RVC_X(x_, RVC_B_IMM_5_OPOFF, RVC_B_IMM_5_MASK) << RVC_B_IMM_5_OFF) | \
> >  	(RVC_X(x_, RVC_B_IMM_7_6_OPOFF, RVC_B_IMM_7_6_MASK) << RVC_B_IMM_7_6_OFF) | \
> >  	(RVC_IMM_SIGN(x_) << RVC_B_IMM_SIGN_OFF); })
> > +
> > +/*
> > + * Put together one immediate from a U-type and I-type instruction pair.
> > + *
> > + * The U-type contains an upper immediate, meaning bits[31:12] with [11:0]
> > + * being zero, while the I-type contains a 12bit immediate.
> > + * Combined these can encode larger 32bit values and are used for example
> > + * in auipc + jalr pairs to allow larger jumps.
> > + *
> > + * @utype_insn: instruction containing the upper immediate
> > + * @itype_insn: instruction
> > + * Return: combined immediate
> > + */
> > +static inline s32 riscv_insn_extract_utype_itype_imm(u32 utype_insn, u32 itype_insn)
> > +{
> > +	s32 imm;
> > +
> > +	imm = RV_EXTRACT_UTYPE_IMM(utype_insn);
> > +	imm += RV_EXTRACT_ITYPE_IMM(itype_insn);
> > +
> > +	return imm;
> > +}
> > +
> > +/*
> > + * Update a set of two instructions (U-type + I-type) with an immediate value.
> > + *
> > + * Used for example in auipc+jalrs pairs the U-type instructions contains
> > + * a 20bit upper immediate representing bits[31:12], while the I-type
> > + * instruction contains a 12bit immediate representing bits[11:0].
> > + *
> > + * This also takes into account that both separate immediates are
> > + * considered as signed values, so if the I-type immediate becomes
> > + * negative (BIT(11) set) the U-type part gets adjusted.
> > + *
> > + * @insn: pointer to a set of two instructions
> > + * @imm: the immediate to insert into the two instructions
> > + */
> > +static inline void riscv_insn_insert_utype_itype_imm(u32 *insn, s32 imm)
> > +{
> > +	/* drop possible old IMM values */
> > +	insn[0] &= ~(RV_U_IMM_31_12_MASK);
> > +	insn[1] &= ~(RV_I_IMM_11_0_MASK << RV_I_IMM_11_0_OPOFF);
> > +
> > +	/* add the adapted IMMs */
> > +	insn[0] |= ((imm & RV_U_IMM_31_12_MASK) + ((imm & BIT(11)) << 1));
> > +	insn[1] |= ((imm & RV_I_IMM_11_0_MASK) << RV_I_IMM_11_0_OPOFF);
> 
> If you send a v5, could you drop the extra ()s around some of these?

will do

> Otherwise:
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> I do like the comments a lot :) Saves going off to read specs...

That was the intention with the big comments :-) .

And self-preservation ... Once my mind does a cache-flush of these things,
I don't want to have to dig through things to re-understand this again.


Heiko



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

Re: [PATCH v4 12/12] RISC-V: fix auipc-jalr addresses in patched alternatives

[Conor.Dooley]

On 07/12/2022 22:37, Heiko Stuebner wrote:
> Am Mittwoch, 7. Dezember 2022, 21:48:08 CET schrieb Conor Dooley:
>> On Wed, Dec 07, 2022 at 07:08:21PM +0100, Heiko Stuebner wrote:
>>> From: Heiko Stuebner <heiko.stuebner@vrull.eu>

>>> +void riscv_alternative_fix_offsets(void *alt_ptr, unsigned int len,
>>> +                                 int patch_offset)
>>> +{
>>> +   int num_instr = len / sizeof(u32);
>>
>> instr...
>>
>>> +   int i;
>>> +
>>> +   /*
>>> +    * stop one instruction before the end, as we're checking
>>> +    * for auipc + jalr
>>> +    */
>>> +   for (i = 0; i < num_instr; i++) {
>>> +           u32 inst = riscv_instruction_at(alt_ptr + i * sizeof(u32));
>>
>> ...inst...
>>
>>> +
>>> +           /* may be the start of an auipc + jalr pair */
>>> +           if (riscv_insn_is_auipc(inst) && i < num_instr - 1) {
>>
>> ...insn.
>>
>> Is there a reason for that?
> 
> I guess more of a generational issue - with the code spanning
> too much time :-)
> 
> So poll question, what would be preferred?
> I think I remember seeing all of them somewhere, so I'm unsure what
> to standardize on.

I'd vote for insn, since it's used in the functions you're calling.

>> Also, I've gotten myself slightly confused about the loop. You "stop one
>> instruction before the end" but the main loop goes from 0 -> num_instr.
>> The inner loop then checks for i < num_instr - 1. What am I missing that
>> prevents the outer loop from stopping at num_instr - 1 instead?
> 
> The idea with this is to allow a
>          if(riscv_insn_is_jal(inst))
>                  riscv_alternative_fix_jal(...)
> 
> etc, and everything else is a single instruction so needs one more
> loop iteration, only for auipc+jalr do we want to stop one earlier.
> 
> So to get this alternatives_fix_offsets() main entry point I
> made the loop do the one iteration more again.

Right, fair enough in my book :)

Thanks,
Conor.



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

Re: [PATCH v4 11/12] RISC-V: add helpers for handling immediates in U-type and I-type pairs

[Andrew Jones]

On Wed, Dec 07, 2022 at 07:08:20PM +0100, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> 
> Used together U-type and I-type instructions can for example be used to
> generate bigger jumps (i.e. in auipc+jalr pairs) by splitting the value
> into an upper immediate (i.e. auipc) and a 12bit immediate (i.e. jalr).
> 
> Due to both immediates being considered signed this creates some corner
> cases, so add some helper to prevent this from getting duplicated in
> different places.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/insn.h | 47 +++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
> index 2a23890b4577..bb1e6120a560 100644
> --- a/arch/riscv/include/asm/insn.h
> +++ b/arch/riscv/include/asm/insn.h
> @@ -290,3 +290,50 @@ static __always_inline bool riscv_insn_is_branch(u32 code)
>  	(RVC_X(x_, RVC_B_IMM_5_OPOFF, RVC_B_IMM_5_MASK) << RVC_B_IMM_5_OFF) | \
>  	(RVC_X(x_, RVC_B_IMM_7_6_OPOFF, RVC_B_IMM_7_6_MASK) << RVC_B_IMM_7_6_OFF) | \
>  	(RVC_IMM_SIGN(x_) << RVC_B_IMM_SIGN_OFF); })
> +
> +/*
> + * Put together one immediate from a U-type and I-type instruction pair.
> + *
> + * The U-type contains an upper immediate, meaning bits[31:12] with [11:0]
> + * being zero, while the I-type contains a 12bit immediate.
> + * Combined these can encode larger 32bit values and are used for example
> + * in auipc + jalr pairs to allow larger jumps.
> + *
> + * @utype_insn: instruction containing the upper immediate
> + * @itype_insn: instruction
> + * Return: combined immediate
> + */
> +static inline s32 riscv_insn_extract_utype_itype_imm(u32 utype_insn, u32 itype_insn)

Might be nice to keep the interfaces of these two complementing functions
consistent by taking a u32 *insn here, like below. Or, below, taking
&utype_insn and &itype_insn, like here.

> +{
> +	s32 imm;
> +
> +	imm = RV_EXTRACT_UTYPE_IMM(utype_insn);
> +	imm += RV_EXTRACT_ITYPE_IMM(itype_insn);
> +
> +	return imm;
> +}
> +
> +/*
> + * Update a set of two instructions (U-type + I-type) with an immediate value.
> + *
> + * Used for example in auipc+jalrs pairs the U-type instructions contains
> + * a 20bit upper immediate representing bits[31:12], while the I-type
> + * instruction contains a 12bit immediate representing bits[11:0].
> + *
> + * This also takes into account that both separate immediates are
> + * considered as signed values, so if the I-type immediate becomes
> + * negative (BIT(11) set) the U-type part gets adjusted.
> + *
> + * @insn: pointer to a set of two instructions
> + * @imm: the immediate to insert into the two instructions
> + */
> +static inline void riscv_insn_insert_utype_itype_imm(u32 *insn, s32 imm)
> +{
> +	/* drop possible old IMM values */
> +	insn[0] &= ~(RV_U_IMM_31_12_MASK);
> +	insn[1] &= ~(RV_I_IMM_11_0_MASK << RV_I_IMM_11_0_OPOFF);
> +
> +	/* add the adapted IMMs */
> +	insn[0] |= ((imm & RV_U_IMM_31_12_MASK) + ((imm & BIT(11)) << 1));
> +	insn[1] |= ((imm & RV_I_IMM_11_0_MASK) << RV_I_IMM_11_0_OPOFF);
> +}
> -- 
> 2.35.1
>

Otherwise

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

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

Re: [PATCH v4 12/12] RISC-V: fix auipc-jalr addresses in patched alternatives

[Andrew Jones]

On Wed, Dec 07, 2022 at 07:08:21PM +0100, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> 
> Alternatives live in a different section, so addresses used by call
> functions will point to wrong locations after the patch got applied.
> 
> Similar to arm64, adjust the location to consider that offset.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/alternative.h |  3 ++
>  arch/riscv/kernel/alternative.c      | 56 ++++++++++++++++++++++++++++
>  arch/riscv/kernel/cpufeature.c       |  5 ++-
>  3 files changed, 63 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> index 6511dd73e812..1bd4027d34ca 100644
> --- a/arch/riscv/include/asm/alternative.h
> +++ b/arch/riscv/include/asm/alternative.h
> @@ -27,6 +27,9 @@ void __init apply_boot_alternatives(void);
>  void __init apply_early_boot_alternatives(void);
>  void apply_module_alternatives(void *start, size_t length);
>  
> +void riscv_alternative_fix_offsets(void *alt_ptr, unsigned int len,
> +				   int patch_offset);
> +
>  struct alt_entry {
>  	void *old_ptr;		 /* address of original instruciton or data  */
>  	void *alt_ptr;		 /* address of replacement instruction or data */
> diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
> index a7d26a00beea..c29e198ed9df 100644
> --- a/arch/riscv/kernel/alternative.c
> +++ b/arch/riscv/kernel/alternative.c
> @@ -15,6 +15,8 @@
>  #include <asm/vendorid_list.h>
>  #include <asm/sbi.h>
>  #include <asm/csr.h>
> +#include <asm/insn.h>
> +#include <asm/patch.h>
>  
>  struct cpu_manufacturer_info_t {
>  	unsigned long vendor_id;
> @@ -53,6 +55,60 @@ static void __init_or_module riscv_fill_cpu_mfr_info(struct cpu_manufacturer_inf
>  	}
>  }
>  
> +static u32 riscv_instruction_at(void *p)
> +{
> +	u16 *parcel = p;
> +
> +	return (unsigned int)parcel[0] | (unsigned int)parcel[1] << 16;
> +}
> +
> +static void riscv_alternative_fix_auipc_jalr(void *ptr, u32 insn1, u32 insn2, int patch_offset)

Maybe instead of 'insn1' and 'insn2' names we should use the names
'auipc' and 'jalr' to self-document which is which.

> +{
> +	/* pick the original auipc + jalr */

If we change the names, then the comment above could be removed.

> +	u32 call[2] = { insn1, insn2 };
> +	s32 imm;
> +
> +	/* get and adjust new target address */
> +	imm = riscv_insn_extract_utype_itype_imm(insn1, insn2);
> +	imm -= patch_offset;
> +
> +	/* update instructions */
> +	riscv_insn_insert_utype_itype_imm(call, imm);
> +
> +	/* patch the call place again */
> +	patch_text_nosync(ptr, call, sizeof(u32) * 2);
> +}
> +
> +void riscv_alternative_fix_offsets(void *alt_ptr, unsigned int len,
> +				      int patch_offset)
> +{
> +	int num_instr = len / sizeof(u32);
> +	int i;
> +
> +	/*
> +	 * stop one instruction before the end, as we're checking
> +	 * for auipc + jalr
> +	 */
> +	for (i = 0; i < num_instr; i++) {
> +		u32 inst = riscv_instruction_at(alt_ptr + i * sizeof(u32));
> +
> +		/* may be the start of an auipc + jalr pair */
> +		if (riscv_insn_is_auipc(inst) && i < num_instr - 1) {
> +			u32 inst2 = riscv_instruction_at(alt_ptr + (i + 1) * sizeof(u32));
> +
> +			if (!riscv_insn_is_jalr(inst2))
> +				continue;
> +
> +			/* call will use ra register */
> +			if (RV_EXTRACT_RD_REG(inst) != 1)
> +				continue;
> +
> +			riscv_alternative_fix_auipc_jalr(alt_ptr + i * sizeof(u32),
> +							 inst, inst2, patch_offset);
> +		}
> +	}
> +}
> +
>  /*
>   * This is called very early in the boot process (directly after we run
>   * a feature detect on the boot CPU). No need to worry about other CPUs
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 694267d1fe81..e91ec3d8e240 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -316,8 +316,11 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
>  		}
>  
>  		tmp = (1U << alt->errata_id);
> -		if (cpu_req_feature & tmp)
> +		if (cpu_req_feature & tmp) {
>  			patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> +			riscv_alternative_fix_offsets(alt->old_ptr, alt->alt_len,
> +						      alt->old_ptr - alt->alt_ptr);
> +		}
>  	}
>  }
>  #endif
> -- 
> 2.35.1
>

Besides the naming nit which probably isn't worth respinning for

Reviewed-by: Andrew Jones <ajones@ventanamicro.com>

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

Re: [PATCH v4 01/12] RISC-V: fix funct4 definition for c.jalr in parse_asm.h

[Lad, Prabhakar]

Hi Heiko,

Thank you for the patch.

On Wed, Dec 7, 2022 at 6:08 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
> The opcode definition for c.jalr is
>     c.jalr c_rs1_n0  1..0=2 15..13=4 12=1 6..2=0
>
> This means funct4 consisting of bit [15:12] is 1001b, so the value is 0x9.
>
I was struggling to find the details for this, finally found it here
https://msyksphinz-self.github.io/riscv-isadoc/html/rvc.html#c-jalr

> Fixes: edde5584c7ab ("riscv: Add SW single-step support for KDB")
> Reported-by: Andrew Jones <ajones@ventanamicro.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/parse_asm.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

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

Re: [PATCH v4 02/12] RISC-V: add prefix to all constants/macros in parse_asm.h

[Lad, Prabhakar]

Hi Heiko,

Thank you for the patch.

On Wed, Dec 7, 2022 at 6:08 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
> Some of the constants and macros already have suitable RV_, RVG_ or
> RVC_ prefixes.
>
> Extend this to the rest of the file as well, as we want to use these
> things in a broader scope soon.
>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/parse_asm.h | 194 ++++++++++++++---------------
>  arch/riscv/kernel/kgdb.c           |  40 +++---
>  2 files changed, 117 insertions(+), 117 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

> diff --git a/arch/riscv/include/asm/parse_asm.h b/arch/riscv/include/asm/parse_asm.h
> index 7fee806805c1..ea51542e0c65 100644
> --- a/arch/riscv/include/asm/parse_asm.h
> +++ b/arch/riscv/include/asm/parse_asm.h
> @@ -6,37 +6,37 @@
>  #include <linux/bits.h>
>
>  /* The bit field of immediate value in I-type instruction */
> -#define I_IMM_SIGN_OPOFF       31
> -#define I_IMM_11_0_OPOFF       20
> -#define I_IMM_SIGN_OFF         12
> -#define I_IMM_11_0_OFF         0
> -#define I_IMM_11_0_MASK                GENMASK(11, 0)
> +#define RV_I_IMM_SIGN_OPOFF    31
> +#define RV_I_IMM_11_0_OPOFF    20
> +#define RV_I_IMM_SIGN_OFF      12
> +#define RV_I_IMM_11_0_OFF      0
> +#define RV_I_IMM_11_0_MASK     GENMASK(11, 0)
>
>  /* The bit field of immediate value in J-type instruction */
> -#define J_IMM_SIGN_OPOFF       31
> -#define J_IMM_10_1_OPOFF       21
> -#define J_IMM_11_OPOFF         20
> -#define J_IMM_19_12_OPOFF      12
> -#define J_IMM_SIGN_OFF         20
> -#define J_IMM_10_1_OFF         1
> -#define J_IMM_11_OFF           11
> -#define J_IMM_19_12_OFF                12
> -#define J_IMM_10_1_MASK                GENMASK(9, 0)
> -#define J_IMM_11_MASK          GENMASK(0, 0)
> -#define J_IMM_19_12_MASK       GENMASK(7, 0)
> +#define RV_J_IMM_SIGN_OPOFF    31
> +#define RV_J_IMM_10_1_OPOFF    21
> +#define RV_J_IMM_11_OPOFF      20
> +#define RV_J_IMM_19_12_OPOFF   12
> +#define RV_J_IMM_SIGN_OFF      20
> +#define RV_J_IMM_10_1_OFF      1
> +#define RV_J_IMM_11_OFF                11
> +#define RV_J_IMM_19_12_OFF     12
> +#define RV_J_IMM_10_1_MASK     GENMASK(9, 0)
> +#define RV_J_IMM_11_MASK       GENMASK(0, 0)
> +#define RV_J_IMM_19_12_MASK    GENMASK(7, 0)
>
>  /* The bit field of immediate value in B-type instruction */
> -#define B_IMM_SIGN_OPOFF       31
> -#define B_IMM_10_5_OPOFF       25
> -#define B_IMM_4_1_OPOFF                8
> -#define B_IMM_11_OPOFF         7
> -#define B_IMM_SIGN_OFF         12
> -#define B_IMM_10_5_OFF         5
> -#define B_IMM_4_1_OFF          1
> -#define B_IMM_11_OFF           11
> -#define B_IMM_10_5_MASK                GENMASK(5, 0)
> -#define B_IMM_4_1_MASK         GENMASK(3, 0)
> -#define B_IMM_11_MASK          GENMASK(0, 0)
> +#define RV_B_IMM_SIGN_OPOFF    31
> +#define RV_B_IMM_10_5_OPOFF    25
> +#define RV_B_IMM_4_1_OPOFF     8
> +#define RV_B_IMM_11_OPOFF      7
> +#define RV_B_IMM_SIGN_OFF      12
> +#define RV_B_IMM_10_5_OFF      5
> +#define RV_B_IMM_4_1_OFF       1
> +#define RV_B_IMM_11_OFF                11
> +#define RV_B_IMM_10_5_MASK     GENMASK(5, 0)
> +#define RV_B_IMM_4_1_MASK      GENMASK(3, 0)
> +#define RV_B_IMM_11_MASK       GENMASK(0, 0)
>
>  /* The register offset in RVG instruction */
>  #define RVG_RS1_OPOFF          15
> @@ -100,71 +100,71 @@
>  #define RVC_C2_RD_OPOFF                7
>
>  /* parts of opcode for RVG*/
not related to your changes, it needs a space while closing the
comment here and below.

Cheers,
Prabhakar

> -#define OPCODE_BRANCH          0x63
> -#define OPCODE_JALR            0x67
> -#define OPCODE_JAL             0x6f
> -#define OPCODE_SYSTEM          0x73
> +#define RVG_OPCODE_BRANCH      0x63
> +#define RVG_OPCODE_JALR                0x67
> +#define RVG_OPCODE_JAL         0x6f
> +#define RVG_OPCODE_SYSTEM      0x73
>
>  /* parts of opcode for RVC*/
> -#define OPCODE_C_0             0x0
> -#define OPCODE_C_1             0x1
> -#define OPCODE_C_2             0x2
> +#define RVC_OPCODE_C0          0x0
> +#define RVC_OPCODE_C1          0x1
> +#define RVC_OPCODE_C2          0x2
>
>  /* parts of funct3 code for I, M, A extension*/
> -#define FUNCT3_JALR            0x0
> -#define FUNCT3_BEQ             0x0
> -#define FUNCT3_BNE             0x1000
> -#define FUNCT3_BLT             0x4000
> -#define FUNCT3_BGE             0x5000
> -#define FUNCT3_BLTU            0x6000
> -#define FUNCT3_BGEU            0x7000
> +#define RVG_FUNCT3_JALR                0x0
> +#define RVG_FUNCT3_BEQ         0x0
> +#define RVG_FUNCT3_BNE         0x1000
> +#define RVG_FUNCT3_BLT         0x4000
> +#define RVG_FUNCT3_BGE         0x5000
> +#define RVG_FUNCT3_BLTU                0x6000
> +#define RVG_FUNCT3_BGEU                0x7000
>
>  /* parts of funct3 code for C extension*/
> -#define FUNCT3_C_BEQZ          0xc000
> -#define FUNCT3_C_BNEZ          0xe000
> -#define FUNCT3_C_J             0xa000
> -#define FUNCT3_C_JAL           0x2000
> -#define FUNCT4_C_JR            0x8000
> -#define FUNCT4_C_JALR          0x9000
> -
> -#define FUNCT12_SRET           0x10200000
> -
> -#define MATCH_JALR             (FUNCT3_JALR | OPCODE_JALR)
> -#define MATCH_JAL              (OPCODE_JAL)
> -#define MATCH_BEQ              (FUNCT3_BEQ | OPCODE_BRANCH)
> -#define MATCH_BNE              (FUNCT3_BNE | OPCODE_BRANCH)
> -#define MATCH_BLT              (FUNCT3_BLT | OPCODE_BRANCH)
> -#define MATCH_BGE              (FUNCT3_BGE | OPCODE_BRANCH)
> -#define MATCH_BLTU             (FUNCT3_BLTU | OPCODE_BRANCH)
> -#define MATCH_BGEU             (FUNCT3_BGEU | OPCODE_BRANCH)
> -#define MATCH_SRET             (FUNCT12_SRET | OPCODE_SYSTEM)
> -#define MATCH_C_BEQZ           (FUNCT3_C_BEQZ | OPCODE_C_1)
> -#define MATCH_C_BNEZ           (FUNCT3_C_BNEZ | OPCODE_C_1)
> -#define MATCH_C_J              (FUNCT3_C_J | OPCODE_C_1)
> -#define MATCH_C_JAL            (FUNCT3_C_JAL | OPCODE_C_1)
> -#define MATCH_C_JR             (FUNCT4_C_JR | OPCODE_C_2)
> -#define MATCH_C_JALR           (FUNCT4_C_JALR | OPCODE_C_2)
> -
> -#define MASK_JALR              0x707f
> -#define MASK_JAL               0x7f
> -#define MASK_C_JALR            0xf07f
> -#define MASK_C_JR              0xf07f
> -#define MASK_C_JAL             0xe003
> -#define MASK_C_J               0xe003
> -#define MASK_BEQ               0x707f
> -#define MASK_BNE               0x707f
> -#define MASK_BLT               0x707f
> -#define MASK_BGE               0x707f
> -#define MASK_BLTU              0x707f
> -#define MASK_BGEU              0x707f
> -#define MASK_C_BEQZ            0xe003
> -#define MASK_C_BNEZ            0xe003
> -#define MASK_SRET              0xffffffff
> +#define RVC_FUNCT3_C_BEQZ      0xc000
> +#define RVC_FUNCT3_C_BNEZ      0xe000
> +#define RVC_FUNCT3_C_J         0xa000
> +#define RVC_FUNCT3_C_JAL       0x2000
> +#define RVC_FUNCT4_C_JR                0x8000
> +#define RVC_FUNCT4_C_JALR      0x9000
> +
> +#define RVG_FUNCT12_SRET       0x10200000
> +
> +#define RVG_MATCH_JALR         (RVG_FUNCT3_JALR | RVG_OPCODE_JALR)
> +#define RVG_MATCH_JAL          (RVG_OPCODE_JAL)
> +#define RVG_MATCH_BEQ          (RVG_FUNCT3_BEQ | RVG_OPCODE_BRANCH)
> +#define RVG_MATCH_BNE          (RVG_FUNCT3_BNE | RVG_OPCODE_BRANCH)
> +#define RVG_MATCH_BLT          (RVG_FUNCT3_BLT | RVG_OPCODE_BRANCH)
> +#define RVG_MATCH_BGE          (RVG_FUNCT3_BGE | RVG_OPCODE_BRANCH)
> +#define RVG_MATCH_BLTU         (RVG_FUNCT3_BLTU | RVG_OPCODE_BRANCH)
> +#define RVG_MATCH_BGEU         (RVG_FUNCT3_BGEU | RVG_OPCODE_BRANCH)
> +#define RVG_MATCH_SRET         (RVG_FUNCT12_SRET | RVG_OPCODE_SYSTEM)
> +#define RVC_MATCH_C_BEQZ       (RVC_FUNCT3_C_BEQZ | RVC_OPCODE_C1)
> +#define RVC_MATCH_C_BNEZ       (RVC_FUNCT3_C_BNEZ | RVC_OPCODE_C1)
> +#define RVC_MATCH_C_J          (RVC_FUNCT3_C_J | RVC_OPCODE_C1)
> +#define RVC_MATCH_C_JAL                (RVC_FUNCT3_C_JAL | RVC_OPCODE_C1)
> +#define RVC_MATCH_C_JR         (RVC_FUNCT4_C_JR | RVC_OPCODE_C2)
> +#define RVC_MATCH_C_JALR       (RVC_FUNCT4_C_JALR | RVC_OPCODE_C2)
> +
> +#define RVG_MASK_JALR          0x707f
> +#define RVG_MASK_JAL           0x7f
> +#define RVC_MASK_C_JALR                0xf07f
> +#define RVC_MASK_C_JR          0xf07f
> +#define RVC_MASK_C_JAL         0xe003
> +#define RVC_MASK_C_J           0xe003
> +#define RVG_MASK_BEQ           0x707f
> +#define RVG_MASK_BNE           0x707f
> +#define RVG_MASK_BLT           0x707f
> +#define RVG_MASK_BGE           0x707f
> +#define RVG_MASK_BLTU          0x707f
> +#define RVG_MASK_BGEU          0x707f
> +#define RVC_MASK_C_BEQZ                0xe003
> +#define RVC_MASK_C_BNEZ                0xe003
> +#define RVG_MASK_SRET          0xffffffff
>
>  #define __INSN_LENGTH_MASK     _UL(0x3)
>  #define __INSN_LENGTH_GE_32    _UL(0x3)
>  #define __INSN_OPCODE_MASK     _UL(0x7F)
> -#define __INSN_BRANCH_OPCODE   _UL(OPCODE_BRANCH)
> +#define __INSN_BRANCH_OPCODE   _UL(RVG_OPCODE_BRANCH)
>
>  /* Define a series of is_XXX_insn functions to check if the value INSN
>   * is an instance of instruction XXX.
> @@ -180,26 +180,26 @@ static inline bool is_ ## INSN_NAME ## _insn(long insn) \
>  #define RV_X(X, s, mask)  (((X) >> (s)) & (mask))
>  #define RVC_X(X, s, mask) RV_X(X, s, mask)
>
> -#define EXTRACT_JTYPE_IMM(x) \
> +#define RV_EXTRACT_JTYPE_IMM(x) \
>         ({typeof(x) x_ = (x); \
> -       (RV_X(x_, J_IMM_10_1_OPOFF, J_IMM_10_1_MASK) << J_IMM_10_1_OFF) | \
> -       (RV_X(x_, J_IMM_11_OPOFF, J_IMM_11_MASK) << J_IMM_11_OFF) | \
> -       (RV_X(x_, J_IMM_19_12_OPOFF, J_IMM_19_12_MASK) << J_IMM_19_12_OFF) | \
> -       (RV_IMM_SIGN(x_) << J_IMM_SIGN_OFF); })
> +       (RV_X(x_, RV_J_IMM_10_1_OPOFF, RV_J_IMM_10_1_MASK) << RV_J_IMM_10_1_OFF) | \
> +       (RV_X(x_, RV_J_IMM_11_OPOFF, RV_J_IMM_11_MASK) << RV_J_IMM_11_OFF) | \
> +       (RV_X(x_, RV_J_IMM_19_12_OPOFF, RV_J_IMM_19_12_MASK) << RV_J_IMM_19_12_OFF) | \
> +       (RV_IMM_SIGN(x_) << RV_J_IMM_SIGN_OFF); })
>
> -#define EXTRACT_ITYPE_IMM(x) \
> +#define RV_EXTRACT_ITYPE_IMM(x) \
>         ({typeof(x) x_ = (x); \
> -       (RV_X(x_, I_IMM_11_0_OPOFF, I_IMM_11_0_MASK)) | \
> -       (RV_IMM_SIGN(x_) << I_IMM_SIGN_OFF); })
> +       (RV_X(x_, RV_I_IMM_11_0_OPOFF, RV_I_IMM_11_0_MASK)) | \
> +       (RV_IMM_SIGN(x_) << RV_I_IMM_SIGN_OFF); })
>
> -#define EXTRACT_BTYPE_IMM(x) \
> +#define RV_EXTRACT_BTYPE_IMM(x) \
>         ({typeof(x) x_ = (x); \
> -       (RV_X(x_, B_IMM_4_1_OPOFF, B_IMM_4_1_MASK) << B_IMM_4_1_OFF) | \
> -       (RV_X(x_, B_IMM_10_5_OPOFF, B_IMM_10_5_MASK) << B_IMM_10_5_OFF) | \
> -       (RV_X(x_, B_IMM_11_OPOFF, B_IMM_11_MASK) << B_IMM_11_OFF) | \
> -       (RV_IMM_SIGN(x_) << B_IMM_SIGN_OFF); })
> +       (RV_X(x_, RV_B_IMM_4_1_OPOFF, RV_B_IMM_4_1_MASK) << RV_B_IMM_4_1_OFF) | \
> +       (RV_X(x_, RV_B_IMM_10_5_OPOFF, RV_B_IMM_10_5_MASK) << RV_B_IMM_10_5_OFF) | \
> +       (RV_X(x_, RV_B_IMM_11_OPOFF, RV_B_IMM_11_MASK) << RV_B_IMM_11_OFF) | \
> +       (RV_IMM_SIGN(x_) << RV_B_IMM_SIGN_OFF); })
>
> -#define EXTRACT_RVC_J_IMM(x) \
> +#define RVC_EXTRACT_JTYPE_IMM(x) \
>         ({typeof(x) x_ = (x); \
>         (RVC_X(x_, RVC_J_IMM_3_1_OPOFF, RVC_J_IMM_3_1_MASK) << RVC_J_IMM_3_1_OFF) | \
>         (RVC_X(x_, RVC_J_IMM_4_OPOFF, RVC_J_IMM_4_MASK) << RVC_J_IMM_4_OFF) | \
> @@ -210,7 +210,7 @@ static inline bool is_ ## INSN_NAME ## _insn(long insn) \
>         (RVC_X(x_, RVC_J_IMM_10_OPOFF, RVC_J_IMM_10_MASK) << RVC_J_IMM_10_OFF) | \
>         (RVC_IMM_SIGN(x_) << RVC_J_IMM_SIGN_OFF); })
>
> -#define EXTRACT_RVC_B_IMM(x) \
> +#define RVC_EXTRACT_BTYPE_IMM(x) \
>         ({typeof(x) x_ = (x); \
>         (RVC_X(x_, RVC_B_IMM_2_1_OPOFF, RVC_B_IMM_2_1_MASK) << RVC_B_IMM_2_1_OFF) | \
>         (RVC_X(x_, RVC_B_IMM_4_3_OPOFF, RVC_B_IMM_4_3_MASK) << RVC_B_IMM_4_3_OFF) | \
> diff --git a/arch/riscv/kernel/kgdb.c b/arch/riscv/kernel/kgdb.c
> index 963ed7edcff2..137c6d870d58 100644
> --- a/arch/riscv/kernel/kgdb.c
> +++ b/arch/riscv/kernel/kgdb.c
> @@ -29,20 +29,20 @@ DECLARE_INSN(c_jal, MATCH_C_JAL, MASK_C_JAL)
>  #else
>  #define is_c_jal_insn(opcode) 0
>  #endif
> -DECLARE_INSN(jalr, MATCH_JALR, MASK_JALR)
> -DECLARE_INSN(jal, MATCH_JAL, MASK_JAL)
> -DECLARE_INSN(c_jr, MATCH_C_JR, MASK_C_JR)
> -DECLARE_INSN(c_jalr, MATCH_C_JALR, MASK_C_JALR)
> -DECLARE_INSN(c_j, MATCH_C_J, MASK_C_J)
> -DECLARE_INSN(beq, MATCH_BEQ, MASK_BEQ)
> -DECLARE_INSN(bne, MATCH_BNE, MASK_BNE)
> -DECLARE_INSN(blt, MATCH_BLT, MASK_BLT)
> -DECLARE_INSN(bge, MATCH_BGE, MASK_BGE)
> -DECLARE_INSN(bltu, MATCH_BLTU, MASK_BLTU)
> -DECLARE_INSN(bgeu, MATCH_BGEU, MASK_BGEU)
> -DECLARE_INSN(c_beqz, MATCH_C_BEQZ, MASK_C_BEQZ)
> -DECLARE_INSN(c_bnez, MATCH_C_BNEZ, MASK_C_BNEZ)
> -DECLARE_INSN(sret, MATCH_SRET, MASK_SRET)
> +DECLARE_INSN(jalr, RVG_MATCH_JALR, RVG_MASK_JALR)
> +DECLARE_INSN(jal, RVG_MATCH_JAL, RVG_MASK_JAL)
> +DECLARE_INSN(c_jr, RVC_MATCH_C_JR, RVC_MASK_C_JR)
> +DECLARE_INSN(c_jalr, RVC_MATCH_C_JALR, RVC_MASK_C_JALR)
> +DECLARE_INSN(c_j, RVC_MATCH_C_J, RVC_MASK_C_J)
> +DECLARE_INSN(beq, RVG_MATCH_BEQ, RVG_MASK_BEQ)
> +DECLARE_INSN(bne, RVG_MATCH_BNE, RVG_MASK_BNE)
> +DECLARE_INSN(blt, RVG_MATCH_BLT, RVG_MASK_BLT)
> +DECLARE_INSN(bge, RVG_MATCH_BGE, RVG_MASK_BGE)
> +DECLARE_INSN(bltu, RVG_MATCH_BLTU, RVG_MASK_BLTU)
> +DECLARE_INSN(bgeu, RVG_MATCH_BGEU, RVG_MASK_BGEU)
> +DECLARE_INSN(c_beqz, RVC_MATCH_C_BEQZ, RVC_MASK_C_BEQZ)
> +DECLARE_INSN(c_bnez, RVC_MATCH_C_BNEZ, RVC_MASK_C_BNEZ)
> +DECLARE_INSN(sret, RVG_MATCH_SRET, RVG_MASK_SRET)
>
>  static int decode_register_index(unsigned long opcode, int offset)
>  {
> @@ -69,19 +69,19 @@ static int get_step_address(struct pt_regs *regs, unsigned long *next_addr)
>                         rs1_num = decode_register_index(op_code, RVC_C2_RS1_OPOFF);
>                         *next_addr = regs_ptr[rs1_num];
>                 } else if (is_c_j_insn(op_code) || is_c_jal_insn(op_code)) {
> -                       *next_addr = EXTRACT_RVC_J_IMM(op_code) + pc;
> +                       *next_addr = RVC_EXTRACT_JTYPE_IMM(op_code) + pc;
>                 } else if (is_c_beqz_insn(op_code)) {
>                         rs1_num = decode_register_index_short(op_code,
>                                                               RVC_C1_RS1_OPOFF);
>                         if (!rs1_num || regs_ptr[rs1_num] == 0)
> -                               *next_addr = EXTRACT_RVC_B_IMM(op_code) + pc;
> +                               *next_addr = RVC_EXTRACT_BTYPE_IMM(op_code) + pc;
>                         else
>                                 *next_addr = pc + 2;
>                 } else if (is_c_bnez_insn(op_code)) {
>                         rs1_num =
>                             decode_register_index_short(op_code, RVC_C1_RS1_OPOFF);
>                         if (rs1_num && regs_ptr[rs1_num] != 0)
> -                               *next_addr = EXTRACT_RVC_B_IMM(op_code) + pc;
> +                               *next_addr = RVC_EXTRACT_BTYPE_IMM(op_code) + pc;
>                         else
>                                 *next_addr = pc + 2;
>                 } else {
> @@ -90,7 +90,7 @@ static int get_step_address(struct pt_regs *regs, unsigned long *next_addr)
>         } else {
>                 if ((op_code & __INSN_OPCODE_MASK) == __INSN_BRANCH_OPCODE) {
>                         bool result = false;
> -                       long imm = EXTRACT_BTYPE_IMM(op_code);
> +                       long imm = RV_EXTRACT_BTYPE_IMM(op_code);
>                         unsigned long rs1_val = 0, rs2_val = 0;
>
>                         rs1_num = decode_register_index(op_code, RVG_RS1_OPOFF);
> @@ -121,12 +121,12 @@ static int get_step_address(struct pt_regs *regs, unsigned long *next_addr)
>                         else
>                                 *next_addr = pc + 4;
>                 } else if (is_jal_insn(op_code)) {
> -                       *next_addr = EXTRACT_JTYPE_IMM(op_code) + pc;
> +                       *next_addr = RV_EXTRACT_JTYPE_IMM(op_code) + pc;
>                 } else if (is_jalr_insn(op_code)) {
>                         rs1_num = decode_register_index(op_code, RVG_RS1_OPOFF);
>                         if (rs1_num)
>                                 *next_addr = ((unsigned long *)regs)[rs1_num];
> -                       *next_addr += EXTRACT_ITYPE_IMM(op_code);
> +                       *next_addr += RV_EXTRACT_ITYPE_IMM(op_code);
>                 } else if (is_sret_insn(op_code)) {
>                         *next_addr = pc;
>                 } else {
> --
> 2.35.1
>

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

Re: [PATCH v4 03/12] RISC-V: detach funct-values from their offset

[Lad, Prabhakar]

On Wed, Dec 7, 2022 at 6:08 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
> Rather than defining funct3, funct4, etc values pre-shifted to their
> target-position in an instruction, define the values themselves and
> only shift them where needed.
>
> This allows using these funct-values in other places as well, for example
> when decoding functions.
>
> At the same time also reduces the use of magic numbers, one would need
> a spec manual to understand.
>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/parse_asm.h | 100 +++++++++++++++++------------
>  1 file changed, 59 insertions(+), 41 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

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

Re: [PATCH v4 04/12] RISC-V: add ebreak instructions to definitions

[Lad, Prabhakar]

On Wed, Dec 7, 2022 at 6:08 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
> kprobes need to match ebreak instructions, so add the necessary
> data to enable us to centralize that functionality.
>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/parse_asm.h | 6 ++++++
>  1 file changed, 6 insertions(+)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

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

Re: [PATCH v4 05/12] RISC-V: add auipc elements to parse_asm header

[Lad, Prabhakar]

On Wed, Dec 7, 2022 at 6:08 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
> Instruction parsing should not be done in individual code, but instead
> supported by central
>
> Right now kgdb and kprobes parse instructions and at least kprobes (and
> the upcoming auipc+jalr alternative fixer-function) need the auipc
> instruction.
>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/parse_asm.h | 3 +++
>  1 file changed, 3 insertions(+)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

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

Re: [PATCH v4 06/12] RISC-V: Move riscv_insn_is_* macros into a common header

[Lad, Prabhakar]

On Wed, Dec 7, 2022 at 6:08 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
> Right now the riscv kernel has (at least) two independent sets
> of functions to check if an encoded instruction is of a specific
> type. One in kgdb and one kprobes simulate-insn code.
>
> More parts of the kernel will probably need this in the future,
> so instead of allowing this duplication to go on further,
> move macros that do the function declaration in a common header,
> similar to at least aarch64.
>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/parse_asm.h       | 42 ++++++++++++++++----
>  arch/riscv/kernel/kgdb.c                 | 49 ++++++++----------------
>  arch/riscv/kernel/probes/simulate-insn.h | 26 +++----------
>  3 files changed, 55 insertions(+), 62 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

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

Re: [PATCH v4 07/12] RISC-V: rename parse_asm.h to insn.h

[Lad, Prabhakar]

On Wed, Dec 7, 2022 at 6:08 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
> The current parse_asm header should become a more centralized place
> for everything concerning parsing and constructing instructions.
>
> We already have a header insn-def.h similar to aarch64, so rename
> parse_asm.h to insn.h (again similar to aarch64) to show that it's
> meant for more than simple instruction parsing.
>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/{parse_asm.h => insn.h} | 0
>  arch/riscv/kernel/kgdb.c                       | 2 +-
>  arch/riscv/kernel/probes/simulate-insn.h       | 2 +-
>  3 files changed, 2 insertions(+), 2 deletions(-)
>  rename arch/riscv/include/asm/{parse_asm.h => insn.h} (100%)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

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

Re: [PATCH v4 08/12] RISC-V: kprobes: use central defined funct3 constants

[Lad, Prabhakar]

On Wed, Dec 7, 2022 at 6:08 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
> Don't redefine values that are already available in the central header
> asm/insn.h . Use the values from there instead.
>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/kernel/probes/simulate-insn.c | 19 ++++++-------------
>  1 file changed, 6 insertions(+), 13 deletions(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

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

Re: [PATCH v4 09/12] RISC-V: add U-type imm parsing to insn.h header

[Lad, Prabhakar]

On Wed, Dec 7, 2022 at 6:08 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
> Similar to other existing types, allow extracting the immediate
> for a U-type instruction.
>
> U-type immediates are special in that regard, that the value
> in the instruction in bits [31:12] already represents the same
> bits of the immediate, so no shifting is required.
>
> U-type immediates are for example used in the auipc instruction,
> so these constants make it easier to parse such instructions.
>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/insn.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

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

Re: [PATCH v4 10/12] RISC-V: add rd reg parsing to insn.h header

[Lad, Prabhakar]

On Wed, Dec 7, 2022 at 6:09 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
> Add a macro to allow parsing of the rd register from an instruction.
>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/insn.h | 5 +++++
>  1 file changed, 5 insertions(+)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

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

Re: [PATCH v4 11/12] RISC-V: add helpers for handling immediates in U-type and I-type pairs

[Lad, Prabhakar]

On Wed, Dec 7, 2022 at 6:09 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
> Used together U-type and I-type instructions can for example be used to
> generate bigger jumps (i.e. in auipc+jalr pairs) by splitting the value
> into an upper immediate (i.e. auipc) and a 12bit immediate (i.e. jalr).
>
> Due to both immediates being considered signed this creates some corner
> cases, so add some helper to prevent this from getting duplicated in
> different places.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/insn.h | 47 +++++++++++++++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

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

Re: [PATCH v4 12/12] RISC-V: fix auipc-jalr addresses in patched alternatives

[Lad, Prabhakar]

On Wed, Dec 7, 2022 at 6:08 PM Heiko Stuebner <heiko@sntech.de> wrote:
>
> From: Heiko Stuebner <heiko.stuebner@vrull.eu>
>
> Alternatives live in a different section, so addresses used by call
> functions will point to wrong locations after the patch got applied.
>
> Similar to arm64, adjust the location to consider that offset.
>
> Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> ---
>  arch/riscv/include/asm/alternative.h |  3 ++
>  arch/riscv/kernel/alternative.c      | 56 ++++++++++++++++++++++++++++
>  arch/riscv/kernel/cpufeature.c       |  5 ++-
>  3 files changed, 63 insertions(+), 1 deletion(-)
>
Reviewed-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>
Tested-by: Lad Prabhakar <prabhakar.mahadev-lad.rj@bp.renesas.com>

Cheers,
Prabhakar

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

Re: [PATCH v4 11/12] RISC-V: add helpers for handling immediates in U-type and I-type pairs

[Heiko Stübner]

Am Donnerstag, 8. Dezember 2022, 15:38:11 CET schrieb Andrew Jones:
> On Wed, Dec 07, 2022 at 07:08:20PM +0100, Heiko Stuebner wrote:
> > From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> > 
> > Used together U-type and I-type instructions can for example be used to
> > generate bigger jumps (i.e. in auipc+jalr pairs) by splitting the value
> > into an upper immediate (i.e. auipc) and a 12bit immediate (i.e. jalr).
> > 
> > Due to both immediates being considered signed this creates some corner
> > cases, so add some helper to prevent this from getting duplicated in
> > different places.
> > 
> > Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> > ---
> >  arch/riscv/include/asm/insn.h | 47 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 47 insertions(+)
> > 
> > diff --git a/arch/riscv/include/asm/insn.h b/arch/riscv/include/asm/insn.h
> > index 2a23890b4577..bb1e6120a560 100644
> > --- a/arch/riscv/include/asm/insn.h
> > +++ b/arch/riscv/include/asm/insn.h
> > @@ -290,3 +290,50 @@ static __always_inline bool riscv_insn_is_branch(u32 code)
> >  	(RVC_X(x_, RVC_B_IMM_5_OPOFF, RVC_B_IMM_5_MASK) << RVC_B_IMM_5_OFF) | \
> >  	(RVC_X(x_, RVC_B_IMM_7_6_OPOFF, RVC_B_IMM_7_6_MASK) << RVC_B_IMM_7_6_OFF) | \
> >  	(RVC_IMM_SIGN(x_) << RVC_B_IMM_SIGN_OFF); })
> > +
> > +/*
> > + * Put together one immediate from a U-type and I-type instruction pair.
> > + *
> > + * The U-type contains an upper immediate, meaning bits[31:12] with [11:0]
> > + * being zero, while the I-type contains a 12bit immediate.
> > + * Combined these can encode larger 32bit values and are used for example
> > + * in auipc + jalr pairs to allow larger jumps.
> > + *
> > + * @utype_insn: instruction containing the upper immediate
> > + * @itype_insn: instruction
> > + * Return: combined immediate
> > + */
> > +static inline s32 riscv_insn_extract_utype_itype_imm(u32 utype_insn, u32 itype_insn)
> 
> Might be nice to keep the interfaces of these two complementing functions
> consistent by taking a u32 *insn here, like below. Or, below, taking
> &utype_insn and &itype_insn, like here.

I did go with separating the instructions in the insert function, simply
because I think it's way nicer to not impose the fixed structure
(2 u32 array) on every possible caller .

Heiko



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