2022-02-24 16:06:52

by Emil Renner Berthing

[permalink] [raw]
Subject: [PATCH v3 0/8] Add RISC-V asm/insn.h header

This series adds a RISC-V asm/insn.h header to consolidate instruction
generation and manipulation similar to arm64's asm/insn.h.

The first three patches are preparatory patches fixing unaligned access
in module relocations and moving unneded definitions out of asm/module.h
and asm/ftrace.h.
Patch 4 adds the header and the remaining patches converts module
relocation, plt entry, jump label and dynamic ftrace code to use it.

The only obvious code left to convert is the BPF JIT(s).

This series depends on the module relocation range check fix here:
https://lore.kernel.org/linux-riscv/[email protected]/

Changes since v2:
- Split the range check fix out and send it separately.
- Convert "if (IS_DEFINED(CONFIG_32BIT))" to #ifdef to avoid compiler
warning on rv32. Thanks kernel test robot!
- Add sp register, load/store instructions and RISCV_INSN_REG_L,
RISCV_INSN_REG_S and RISC_INSN_SZREG macros to work on both rv32 and
rv64 to the asm/insn.h header.
- Add patch moving unneded definitions out of out asm/ftrace.h and a
patch converting kernel/ftrace.c to use the header.

Changes since v1:
- Send the right patches.

Emil Renner Berthing (8):
riscv: Avoid unaligned access when relocating modules
riscv: Remove unneeded definitions from asm/module.h
riscv: Remove unneeded definitions from asm/ftrace.h
riscv: Add asm/insn.h header
riscv: Use asm/insn.h for module relocations
riscv: Use asm/insn.h to generate plt entries
riscv: Use asm/insn.h for jump labels
riscv: Use asm/insn.h for dynamic ftrace

arch/riscv/include/asm/ftrace.h | 35 +----
arch/riscv/include/asm/insn.h | 151 ++++++++++++++++++
arch/riscv/include/asm/module.h | 87 ----------
arch/riscv/kernel/ftrace.c | 56 ++++---
arch/riscv/kernel/jump_label.c | 12 +-
arch/riscv/kernel/module-sections.c | 71 +++++++++
arch/riscv/kernel/module.c | 236 +++++++++++++---------------
7 files changed, 364 insertions(+), 284 deletions(-)
create mode 100644 arch/riscv/include/asm/insn.h

--
2.35.1


2022-02-24 16:09:46

by Emil Renner Berthing

[permalink] [raw]
Subject: [PATCH v3 1/8] riscv: Avoid unaligned access when relocating modules

With the C-extension regular 32bit instructions are not
necessarily aligned on 4-byte boundaries. RISC-V instructions
are in fact an ordered list of 16bit native-endian
"parcels", so access the instruction as such.

This should also make the code work in case someone builds
a big-endian RISC-V machine.

Fix rcv -> rvc typo while we're at it.

Signed-off-by: Emil Renner Berthing <[email protected]>
---
arch/riscv/kernel/module.c | 151 +++++++++++++++++++------------------
1 file changed, 76 insertions(+), 75 deletions(-)

diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 4a48287513c3..8d6a16d74b5b 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -26,68 +26,86 @@ static bool riscv_insn_valid_32bit_offset(ptrdiff_t val)
#endif
}

-static int apply_r_riscv_32_rela(struct module *me, u32 *location, Elf_Addr v)
+static int riscv_insn_rmw(void *location, u32 keep, u32 set)
+{
+ u16 *parcel = location;
+ u32 insn = (u32)parcel[0] | (u32)parcel[1] << 16;
+
+ insn &= keep;
+ insn |= set;
+
+ parcel[0] = insn;
+ parcel[1] = insn >> 16;
+ return 0;
+}
+
+static int riscv_insn_rvc_rmw(void *location, u16 keep, u16 set)
+{
+ u16 *parcel = location;
+
+ *parcel = (*parcel & keep) | set;
+ return 0;
+}
+
+static int apply_r_riscv_32_rela(struct module *me, void *location, Elf_Addr v)
{
if (v != (u32)v) {
pr_err("%s: value %016llx out of range for 32-bit field\n",
me->name, (long long)v);
return -EINVAL;
}
- *location = v;
+ *(u32 *)location = v;
return 0;
}

-static int apply_r_riscv_64_rela(struct module *me, u32 *location, Elf_Addr v)
+static int apply_r_riscv_64_rela(struct module *me, void *location, Elf_Addr v)
{
*(u64 *)location = v;
return 0;
}

-static int apply_r_riscv_branch_rela(struct module *me, u32 *location,
+static int apply_r_riscv_branch_rela(struct module *me, void *location,
Elf_Addr v)
{
- ptrdiff_t offset = (void *)v - (void *)location;
+ ptrdiff_t offset = (void *)v - location;
u32 imm12 = (offset & 0x1000) << (31 - 12);
u32 imm11 = (offset & 0x800) >> (11 - 7);
u32 imm10_5 = (offset & 0x7e0) << (30 - 10);
u32 imm4_1 = (offset & 0x1e) << (11 - 4);

- *location = (*location & 0x1fff07f) | imm12 | imm11 | imm10_5 | imm4_1;
- return 0;
+ return riscv_insn_rmw(location, 0x1fff07f, imm12 | imm11 | imm10_5 | imm4_1);
}

-static int apply_r_riscv_jal_rela(struct module *me, u32 *location,
+static int apply_r_riscv_jal_rela(struct module *me, void *location,
Elf_Addr v)
{
- ptrdiff_t offset = (void *)v - (void *)location;
+ ptrdiff_t offset = (void *)v - location;
u32 imm20 = (offset & 0x100000) << (31 - 20);
u32 imm19_12 = (offset & 0xff000);
u32 imm11 = (offset & 0x800) << (20 - 11);
u32 imm10_1 = (offset & 0x7fe) << (30 - 10);

- *location = (*location & 0xfff) | imm20 | imm19_12 | imm11 | imm10_1;
- return 0;
+ return riscv_insn_rmw(location, 0xfff, imm20 | imm19_12 | imm11 | imm10_1);
}

-static int apply_r_riscv_rcv_branch_rela(struct module *me, u32 *location,
+static int apply_r_riscv_rvc_branch_rela(struct module *me, void *location,
Elf_Addr v)
{
- ptrdiff_t offset = (void *)v - (void *)location;
+ ptrdiff_t offset = (void *)v - location;
u16 imm8 = (offset & 0x100) << (12 - 8);
u16 imm7_6 = (offset & 0xc0) >> (6 - 5);
u16 imm5 = (offset & 0x20) >> (5 - 2);
u16 imm4_3 = (offset & 0x18) << (12 - 5);
u16 imm2_1 = (offset & 0x6) << (12 - 10);

- *(u16 *)location = (*(u16 *)location & 0xe383) |
- imm8 | imm7_6 | imm5 | imm4_3 | imm2_1;
- return 0;
+ return riscv_insn_rvc_rmw(location, 0xe383,
+ imm8 | imm7_6 | imm5 | imm4_3 | imm2_1);
}

-static int apply_r_riscv_rvc_jump_rela(struct module *me, u32 *location,
+static int apply_r_riscv_rvc_jump_rela(struct module *me, void *location,
Elf_Addr v)
{
- ptrdiff_t offset = (void *)v - (void *)location;
+ ptrdiff_t offset = (void *)v - location;
u16 imm11 = (offset & 0x800) << (12 - 11);
u16 imm10 = (offset & 0x400) >> (10 - 8);
u16 imm9_8 = (offset & 0x300) << (12 - 11);
@@ -97,16 +115,14 @@ static int apply_r_riscv_rvc_jump_rela(struct module *me, u32 *location,
u16 imm4 = (offset & 0x10) << (12 - 5);
u16 imm3_1 = (offset & 0xe) << (12 - 10);

- *(u16 *)location = (*(u16 *)location & 0xe003) |
- imm11 | imm10 | imm9_8 | imm7 | imm6 | imm5 | imm4 | imm3_1;
- return 0;
+ return riscv_insn_rvc_rmw(location, 0xe003,
+ imm11 | imm10 | imm9_8 | imm7 | imm6 | imm5 | imm4 | imm3_1);
}

-static int apply_r_riscv_pcrel_hi20_rela(struct module *me, u32 *location,
+static int apply_r_riscv_pcrel_hi20_rela(struct module *me, void *location,
Elf_Addr v)
{
- ptrdiff_t offset = (void *)v - (void *)location;
- s32 hi20;
+ ptrdiff_t offset = (void *)v - location;

if (!riscv_insn_valid_32bit_offset(offset)) {
pr_err(
@@ -115,23 +131,20 @@ static int apply_r_riscv_pcrel_hi20_rela(struct module *me, u32 *location,
return -EINVAL;
}

- hi20 = (offset + 0x800) & 0xfffff000;
- *location = (*location & 0xfff) | hi20;
- return 0;
+ return riscv_insn_rmw(location, 0xfff, (offset + 0x800) & 0xfffff000);
}

-static int apply_r_riscv_pcrel_lo12_i_rela(struct module *me, u32 *location,
+static int apply_r_riscv_pcrel_lo12_i_rela(struct module *me, void *location,
Elf_Addr v)
{
/*
* v is the lo12 value to fill. It is calculated before calling this
* handler.
*/
- *location = (*location & 0xfffff) | ((v & 0xfff) << 20);
- return 0;
+ return riscv_insn_rmw(location, 0xfffff, (v & 0xfff) << 20);
}

-static int apply_r_riscv_pcrel_lo12_s_rela(struct module *me, u32 *location,
+static int apply_r_riscv_pcrel_lo12_s_rela(struct module *me, void *location,
Elf_Addr v)
{
/*
@@ -141,15 +154,12 @@ static int apply_r_riscv_pcrel_lo12_s_rela(struct module *me, u32 *location,
u32 imm11_5 = (v & 0xfe0) << (31 - 11);
u32 imm4_0 = (v & 0x1f) << (11 - 4);

- *location = (*location & 0x1fff07f) | imm11_5 | imm4_0;
- return 0;
+ return riscv_insn_rmw(location, 0x1fff07f, imm11_5 | imm4_0);
}

-static int apply_r_riscv_hi20_rela(struct module *me, u32 *location,
+static int apply_r_riscv_hi20_rela(struct module *me, void *location,
Elf_Addr v)
{
- s32 hi20;
-
if (IS_ENABLED(CONFIG_CMODEL_MEDLOW)) {
pr_err(
"%s: target %016llx can not be addressed by the 32-bit offset from PC = %p\n",
@@ -157,22 +167,20 @@ static int apply_r_riscv_hi20_rela(struct module *me, u32 *location,
return -EINVAL;
}

- hi20 = ((s32)v + 0x800) & 0xfffff000;
- *location = (*location & 0xfff) | hi20;
- return 0;
+ return riscv_insn_rmw(location, 0xfff, ((s32)v + 0x800) & 0xfffff000);
}

-static int apply_r_riscv_lo12_i_rela(struct module *me, u32 *location,
+static int apply_r_riscv_lo12_i_rela(struct module *me, void *location,
Elf_Addr v)
{
/* Skip medlow checking because of filtering by HI20 already */
s32 hi20 = ((s32)v + 0x800) & 0xfffff000;
s32 lo12 = ((s32)v - hi20);
- *location = (*location & 0xfffff) | ((lo12 & 0xfff) << 20);
- return 0;
+
+ return riscv_insn_rmw(location, 0xfffff, (lo12 & 0xfff) << 20);
}

-static int apply_r_riscv_lo12_s_rela(struct module *me, u32 *location,
+static int apply_r_riscv_lo12_s_rela(struct module *me, void *location,
Elf_Addr v)
{
/* Skip medlow checking because of filtering by HI20 already */
@@ -180,20 +188,18 @@ static int apply_r_riscv_lo12_s_rela(struct module *me, u32 *location,
s32 lo12 = ((s32)v - hi20);
u32 imm11_5 = (lo12 & 0xfe0) << (31 - 11);
u32 imm4_0 = (lo12 & 0x1f) << (11 - 4);
- *location = (*location & 0x1fff07f) | imm11_5 | imm4_0;
- return 0;
+
+ return riscv_insn_rmw(location, 0x1fff07f, imm11_5 | imm4_0);
}

-static int apply_r_riscv_got_hi20_rela(struct module *me, u32 *location,
+static int apply_r_riscv_got_hi20_rela(struct module *me, void *location,
Elf_Addr v)
{
- ptrdiff_t offset = (void *)v - (void *)location;
- s32 hi20;
+ ptrdiff_t offset = (void *)v - location;

/* Always emit the got entry */
if (IS_ENABLED(CONFIG_MODULE_SECTIONS)) {
- offset = module_emit_got_entry(me, v);
- offset = (void *)offset - (void *)location;
+ offset = (void *)module_emit_got_entry(me, v) - location;
} else {
pr_err(
"%s: can not generate the GOT entry for symbol = %016llx from PC = %p\n",
@@ -201,22 +207,19 @@ static int apply_r_riscv_got_hi20_rela(struct module *me, u32 *location,
return -EINVAL;
}

- hi20 = (offset + 0x800) & 0xfffff000;
- *location = (*location & 0xfff) | hi20;
- return 0;
+ return riscv_insn_rmw(location, 0xfff, (offset + 0x800) & 0xfffff000);
}

-static int apply_r_riscv_call_plt_rela(struct module *me, u32 *location,
+static int apply_r_riscv_call_plt_rela(struct module *me, void *location,
Elf_Addr v)
{
- ptrdiff_t offset = (void *)v - (void *)location;
+ ptrdiff_t offset = (void *)v - location;
u32 hi20, lo12;

if (!riscv_insn_valid_32bit_offset(offset)) {
/* Only emit the plt entry if offset over 32-bit range */
if (IS_ENABLED(CONFIG_MODULE_SECTIONS)) {
- offset = module_emit_plt_entry(me, v);
- offset = (void *)offset - (void *)location;
+ offset = (void *)module_emit_plt_entry(me, v) - location;
} else {
pr_err(
"%s: target %016llx can not be addressed by the 32-bit offset from PC = %p\n",
@@ -227,15 +230,14 @@ static int apply_r_riscv_call_plt_rela(struct module *me, u32 *location,

hi20 = (offset + 0x800) & 0xfffff000;
lo12 = (offset - hi20) & 0xfff;
- *location = (*location & 0xfff) | hi20;
- *(location + 1) = (*(location + 1) & 0xfffff) | (lo12 << 20);
- return 0;
+ riscv_insn_rmw(location, 0xfff, hi20);
+ return riscv_insn_rmw(location + 4, 0xfffff, lo12 << 20);
}

-static int apply_r_riscv_call_rela(struct module *me, u32 *location,
+static int apply_r_riscv_call_rela(struct module *me, void *location,
Elf_Addr v)
{
- ptrdiff_t offset = (void *)v - (void *)location;
+ ptrdiff_t offset = (void *)v - location;
u32 hi20, lo12;

if (!riscv_insn_valid_32bit_offset(offset)) {
@@ -247,18 +249,17 @@ static int apply_r_riscv_call_rela(struct module *me, u32 *location,

hi20 = (offset + 0x800) & 0xfffff000;
lo12 = (offset - hi20) & 0xfff;
- *location = (*location & 0xfff) | hi20;
- *(location + 1) = (*(location + 1) & 0xfffff) | (lo12 << 20);
- return 0;
+ riscv_insn_rmw(location, 0xfff, hi20);
+ return riscv_insn_rmw(location + 4, 0xfffff, lo12 << 20);
}

-static int apply_r_riscv_relax_rela(struct module *me, u32 *location,
+static int apply_r_riscv_relax_rela(struct module *me, void *location,
Elf_Addr v)
{
return 0;
}

-static int apply_r_riscv_align_rela(struct module *me, u32 *location,
+static int apply_r_riscv_align_rela(struct module *me, void *location,
Elf_Addr v)
{
pr_err(
@@ -267,41 +268,41 @@ static int apply_r_riscv_align_rela(struct module *me, u32 *location,
return -EINVAL;
}

-static int apply_r_riscv_add32_rela(struct module *me, u32 *location,
+static int apply_r_riscv_add32_rela(struct module *me, void *location,
Elf_Addr v)
{
*(u32 *)location += (u32)v;
return 0;
}

-static int apply_r_riscv_add64_rela(struct module *me, u32 *location,
+static int apply_r_riscv_add64_rela(struct module *me, void *location,
Elf_Addr v)
{
*(u64 *)location += (u64)v;
return 0;
}

-static int apply_r_riscv_sub32_rela(struct module *me, u32 *location,
+static int apply_r_riscv_sub32_rela(struct module *me, void *location,
Elf_Addr v)
{
*(u32 *)location -= (u32)v;
return 0;
}

-static int apply_r_riscv_sub64_rela(struct module *me, u32 *location,
+static int apply_r_riscv_sub64_rela(struct module *me, void *location,
Elf_Addr v)
{
*(u64 *)location -= (u64)v;
return 0;
}

-static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
+static int (*reloc_handlers_rela[]) (struct module *me, void *location,
Elf_Addr v) = {
[R_RISCV_32] = apply_r_riscv_32_rela,
[R_RISCV_64] = apply_r_riscv_64_rela,
[R_RISCV_BRANCH] = apply_r_riscv_branch_rela,
[R_RISCV_JAL] = apply_r_riscv_jal_rela,
- [R_RISCV_RVC_BRANCH] = apply_r_riscv_rcv_branch_rela,
+ [R_RISCV_RVC_BRANCH] = apply_r_riscv_rvc_branch_rela,
[R_RISCV_RVC_JUMP] = apply_r_riscv_rvc_jump_rela,
[R_RISCV_PCREL_HI20] = apply_r_riscv_pcrel_hi20_rela,
[R_RISCV_PCREL_LO12_I] = apply_r_riscv_pcrel_lo12_i_rela,
@@ -325,9 +326,9 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
struct module *me)
{
Elf_Rela *rel = (void *) sechdrs[relsec].sh_addr;
- int (*handler)(struct module *me, u32 *location, Elf_Addr v);
+ int (*handler)(struct module *me, void *location, Elf_Addr v);
Elf_Sym *sym;
- u32 *location;
+ void *location;
unsigned int i, type;
Elf_Addr v;
int res;
--
2.35.1

2022-02-24 16:22:58

by Emil Renner Berthing

[permalink] [raw]
Subject: [PATCH v3 8/8] riscv: Use asm/insn.h for dynamic ftrace

This converts kernel/ftrace.c to use asm/insn.h to generate the
instructions for dynamic ftrace. This also converts the make_call macro
into a regular static function.

Signed-off-by: Emil Renner Berthing <[email protected]>
---
arch/riscv/kernel/ftrace.c | 69 ++++++++++++--------------------------
1 file changed, 22 insertions(+), 47 deletions(-)

diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
index 2cc15dc45ce0..7dd3aafa17aa 100644
--- a/arch/riscv/kernel/ftrace.c
+++ b/arch/riscv/kernel/ftrace.c
@@ -9,6 +9,7 @@
#include <linux/uaccess.h>
#include <linux/memory.h>
#include <asm/cacheflush.h>
+#include <asm/insn.h>
#include <asm/patch.h>

#ifdef CONFIG_DYNAMIC_FTRACE
@@ -21,31 +22,13 @@
* Dynamic ftrace generates probes to call sites, so we must deal with
* both auipc and jalr at the same time.
*/
+static void make_call(unsigned long caller, unsigned long callee, unsigned int call[2])
+{
+ u32 offset = callee - caller;

-#define JALR_SIGN_MASK (0x00000800)
-#define JALR_OFFSET_MASK (0x00000fff)
-#define AUIPC_OFFSET_MASK (0xfffff000)
-#define AUIPC_PAD (0x00001000)
-#define JALR_SHIFT 20
-#define JALR_BASIC (0x000080e7)
-#define AUIPC_BASIC (0x00000097)
-#define NOP4 (0x00000013)
-
-#define make_call(caller, callee, call) \
-do { \
- call[0] = to_auipc_insn((unsigned int)((unsigned long)callee - \
- (unsigned long)caller)); \
- call[1] = to_jalr_insn((unsigned int)((unsigned long)callee - \
- (unsigned long)caller)); \
-} while (0)
-
-#define to_jalr_insn(offset) \
- (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_BASIC)
-
-#define to_auipc_insn(offset) \
- ((offset & JALR_SIGN_MASK) ? \
- (((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) | AUIPC_BASIC) : \
- ((offset & AUIPC_OFFSET_MASK) | AUIPC_BASIC))
+ call[0] = RISCV_INSN_AUIPC | RISCV_INSN_RD_RA | riscv_insn_u_imm(offset + 0x800);
+ call[1] = RISCV_INSN_JALR | RISCV_INSN_RD_RA | RISCV_INSN_RS1_RA | riscv_insn_i_imm(offset);
+}

int ftrace_arch_code_modify_prepare(void) __acquires(&text_mutex)
{
@@ -63,7 +46,7 @@ static int ftrace_check_current_call(unsigned long hook_pos,
unsigned int *expected)
{
unsigned int replaced[2];
- unsigned int nops[2] = {NOP4, NOP4};
+ unsigned int nops[2] = { RISCV_INSN_NOP, RISCV_INSN_NOP };

/* we expect nops at the hook position */
if (!expected)
@@ -95,7 +78,7 @@ static int __ftrace_modify_call(unsigned long hook_pos, unsigned long target,
bool enable)
{
unsigned int call[2];
- unsigned int nops[2] = {NOP4, NOP4};
+ unsigned int nops[2] = { RISCV_INSN_NOP, RISCV_INSN_NOP };

make_call(hook_pos, target, call);

@@ -108,39 +91,31 @@ static int __ftrace_modify_call(unsigned long hook_pos, unsigned long target,
}

/*
- * Put 5 instructions with 16 bytes at the front of function within
+ * Put 4 instructions with 16 bytes at the front of function within
* patchable function entry nops' area.
*
* 0: REG_S ra, -SZREG(sp)
- * 1: auipc ra, 0x?
- * 2: jalr -?(ra)
+ * 1: auipc ra, ?
+ * 2: jalr ra, ra, ?
* 3: REG_L ra, -SZREG(sp)
- *
- * So the opcodes is:
- * 0: 0xfe113c23 (sd)/0xfe112e23 (sw)
- * 1: 0x???????? -> auipc
- * 2: 0x???????? -> jalr
- * 3: 0xff813083 (ld)/0xffc12083 (lw)
*/
-#if __riscv_xlen == 64
-#define INSN0 0xfe113c23
-#define INSN3 0xff813083
-#elif __riscv_xlen == 32
-#define INSN0 0xfe112e23
-#define INSN3 0xffc12083
-#endif
-
#define FUNC_ENTRY_SIZE 16
#define FUNC_ENTRY_JMP 4

int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
{
- unsigned int call[4] = {INSN0, 0, 0, INSN3};
+ unsigned int call[4] = {
+ RISCV_INSN_REG_S | RISCV_INSN_RS2_RA | RISCV_INSN_RS1_SP |
+ riscv_insn_s_imm(-RISCV_INSN_SZREG),
+ 0,
+ 0,
+ RISCV_INSN_REG_L | RISCV_INSN_RD_RA | RISCV_INSN_RS1_SP |
+ riscv_insn_i_imm(-RISCV_INSN_SZREG),
+ };
unsigned long target = addr;
unsigned long caller = rec->ip + FUNC_ENTRY_JMP;

- call[1] = to_auipc_insn((unsigned int)(target - caller));
- call[2] = to_jalr_insn((unsigned int)(target - caller));
+ make_call(caller, target, &call[1]);

if (patch_text_nosync((void *)rec->ip, call, FUNC_ENTRY_SIZE))
return -EPERM;
@@ -151,7 +126,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
unsigned long addr)
{
- unsigned int nops[4] = {NOP4, NOP4, NOP4, NOP4};
+ unsigned int nops[4] = { RISCV_INSN_NOP, RISCV_INSN_NOP, RISCV_INSN_NOP, RISCV_INSN_NOP };

if (patch_text_nosync((void *)rec->ip, nops, FUNC_ENTRY_SIZE))
return -EPERM;
--
2.35.1

2022-02-24 16:24:43

by Emil Renner Berthing

[permalink] [raw]
Subject: [PATCH v3 5/8] riscv: Use asm/insn.h for module relocations

This converts the module relocations in kernel/module.c to use
asm/insn.h for instruction manipulation.

Also RISC-V has a number of instruction pairs to
generate 32bit immediates or jump/call offsets. Eg.:

lui rd, hi20
addi rd, rd, lo12

..where hi20 is the upper 20bits to load into register rd and lo12 is
the lower 12bits. However both immediates are interpreted as two's
complement signed values. Hence the old code calculates hi20 and lo12
for 32bit immediates imm like this:

hi20 = (imm + 0x800) & 0xfffff000;
lo12 = (imm - hi20) & 0xfff;

This patch simplifies it to:

hi20 = (imm + 0x800) & 0xfffff000;
lo12 = imm & 0xfff;

..which amounts to the same: imm - hi20 may be become
negative/underflow, but it doesn't change the lower 12 bits.

Signed-off-by: Emil Renner Berthing <[email protected]>
---
arch/riscv/kernel/module.c | 139 +++++++++++++++----------------------
1 file changed, 56 insertions(+), 83 deletions(-)

diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 8d6a16d74b5b..2212d88776e0 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -2,6 +2,7 @@
/*
*
* Copyright (C) 2017 Zihao Yu
+ * Copyright (C) 2020 Emil Renner Berthing
*/

#include <linux/elf.h>
@@ -11,39 +12,27 @@
#include <linux/vmalloc.h>
#include <linux/sizes.h>
#include <linux/pgtable.h>
+#include <asm/insn.h>
#include <asm/sections.h>

-/*
- * The auipc+jalr instruction pair can reach any PC-relative offset
- * in the range [-2^31 - 2^11, 2^31 - 2^11)
- */
-static bool riscv_insn_valid_32bit_offset(ptrdiff_t val)
-{
-#ifdef CONFIG_32BIT
- return true;
-#else
- return (-(1L << 31) - (1L << 11)) <= val && val < ((1L << 31) - (1L << 11));
-#endif
-}
-
-static int riscv_insn_rmw(void *location, u32 keep, u32 set)
+static int riscv_insn_rmw(void *location, u32 mask, u32 value)
{
u16 *parcel = location;
u32 insn = (u32)parcel[0] | (u32)parcel[1] << 16;

- insn &= keep;
- insn |= set;
+ insn &= ~mask;
+ insn |= value;

parcel[0] = insn;
parcel[1] = insn >> 16;
return 0;
}

-static int riscv_insn_rvc_rmw(void *location, u16 keep, u16 set)
+static int riscv_insn_rvc_rmw(void *location, u16 mask, u16 value)
{
u16 *parcel = location;

- *parcel = (*parcel & keep) | set;
+ *parcel = (*parcel & ~mask) | value;
return 0;
}

@@ -68,55 +57,40 @@ static int apply_r_riscv_branch_rela(struct module *me, void *location,
Elf_Addr v)
{
ptrdiff_t offset = (void *)v - location;
- u32 imm12 = (offset & 0x1000) << (31 - 12);
- u32 imm11 = (offset & 0x800) >> (11 - 7);
- u32 imm10_5 = (offset & 0x7e0) << (30 - 10);
- u32 imm4_1 = (offset & 0x1e) << (11 - 4);

- return riscv_insn_rmw(location, 0x1fff07f, imm12 | imm11 | imm10_5 | imm4_1);
+ return riscv_insn_rmw(location,
+ RISCV_INSN_B_IMM_MASK,
+ riscv_insn_b_imm(offset));
}

static int apply_r_riscv_jal_rela(struct module *me, void *location,
Elf_Addr v)
{
ptrdiff_t offset = (void *)v - location;
- u32 imm20 = (offset & 0x100000) << (31 - 20);
- u32 imm19_12 = (offset & 0xff000);
- u32 imm11 = (offset & 0x800) << (20 - 11);
- u32 imm10_1 = (offset & 0x7fe) << (30 - 10);

- return riscv_insn_rmw(location, 0xfff, imm20 | imm19_12 | imm11 | imm10_1);
+ return riscv_insn_rmw(location,
+ RISCV_INSN_J_IMM_MASK,
+ riscv_insn_j_imm(offset));
}

static int apply_r_riscv_rvc_branch_rela(struct module *me, void *location,
Elf_Addr v)
{
ptrdiff_t offset = (void *)v - location;
- u16 imm8 = (offset & 0x100) << (12 - 8);
- u16 imm7_6 = (offset & 0xc0) >> (6 - 5);
- u16 imm5 = (offset & 0x20) >> (5 - 2);
- u16 imm4_3 = (offset & 0x18) << (12 - 5);
- u16 imm2_1 = (offset & 0x6) << (12 - 10);
-
- return riscv_insn_rvc_rmw(location, 0xe383,
- imm8 | imm7_6 | imm5 | imm4_3 | imm2_1);
+
+ return riscv_insn_rvc_rmw(location,
+ RISCV_INSN_CB_IMM_MASK,
+ riscv_insn_rvc_branch_imm(offset));
}

static int apply_r_riscv_rvc_jump_rela(struct module *me, void *location,
Elf_Addr v)
{
ptrdiff_t offset = (void *)v - location;
- u16 imm11 = (offset & 0x800) << (12 - 11);
- u16 imm10 = (offset & 0x400) >> (10 - 8);
- u16 imm9_8 = (offset & 0x300) << (12 - 11);
- u16 imm7 = (offset & 0x80) >> (7 - 6);
- u16 imm6 = (offset & 0x40) << (12 - 11);
- u16 imm5 = (offset & 0x20) >> (5 - 2);
- u16 imm4 = (offset & 0x10) << (12 - 5);
- u16 imm3_1 = (offset & 0xe) << (12 - 10);
-
- return riscv_insn_rvc_rmw(location, 0xe003,
- imm11 | imm10 | imm9_8 | imm7 | imm6 | imm5 | imm4 | imm3_1);
+
+ return riscv_insn_rvc_rmw(location,
+ RISCV_INSN_CJ_IMM_MASK,
+ riscv_insn_rvc_jump_imm(offset));
}

static int apply_r_riscv_pcrel_hi20_rela(struct module *me, void *location,
@@ -131,30 +105,27 @@ static int apply_r_riscv_pcrel_hi20_rela(struct module *me, void *location,
return -EINVAL;
}

- return riscv_insn_rmw(location, 0xfff, (offset + 0x800) & 0xfffff000);
+ return riscv_insn_rmw(location,
+ RISCV_INSN_U_IMM_MASK,
+ riscv_insn_u_imm(offset + 0x800));
}

static int apply_r_riscv_pcrel_lo12_i_rela(struct module *me, void *location,
Elf_Addr v)
{
- /*
- * v is the lo12 value to fill. It is calculated before calling this
- * handler.
- */
- return riscv_insn_rmw(location, 0xfffff, (v & 0xfff) << 20);
+ /* v is already the relative offset */
+ return riscv_insn_rmw(location,
+ RISCV_INSN_I_IMM_MASK,
+ riscv_insn_i_imm(v));
}

static int apply_r_riscv_pcrel_lo12_s_rela(struct module *me, void *location,
Elf_Addr v)
{
- /*
- * v is the lo12 value to fill. It is calculated before calling this
- * handler.
- */
- u32 imm11_5 = (v & 0xfe0) << (31 - 11);
- u32 imm4_0 = (v & 0x1f) << (11 - 4);
-
- return riscv_insn_rmw(location, 0x1fff07f, imm11_5 | imm4_0);
+ /* v is already the relative offset */
+ return riscv_insn_rmw(location,
+ RISCV_INSN_S_IMM_MASK,
+ riscv_insn_s_imm(v));
}

static int apply_r_riscv_hi20_rela(struct module *me, void *location,
@@ -167,29 +138,27 @@ static int apply_r_riscv_hi20_rela(struct module *me, void *location,
return -EINVAL;
}

- return riscv_insn_rmw(location, 0xfff, ((s32)v + 0x800) & 0xfffff000);
+ return riscv_insn_rmw(location,
+ RISCV_INSN_U_IMM_MASK,
+ riscv_insn_u_imm(v + 0x800));
}

static int apply_r_riscv_lo12_i_rela(struct module *me, void *location,
Elf_Addr v)
{
/* Skip medlow checking because of filtering by HI20 already */
- s32 hi20 = ((s32)v + 0x800) & 0xfffff000;
- s32 lo12 = ((s32)v - hi20);
-
- return riscv_insn_rmw(location, 0xfffff, (lo12 & 0xfff) << 20);
+ return riscv_insn_rmw(location,
+ RISCV_INSN_I_IMM_MASK,
+ riscv_insn_i_imm(v));
}

static int apply_r_riscv_lo12_s_rela(struct module *me, void *location,
Elf_Addr v)
{
/* Skip medlow checking because of filtering by HI20 already */
- s32 hi20 = ((s32)v + 0x800) & 0xfffff000;
- s32 lo12 = ((s32)v - hi20);
- u32 imm11_5 = (lo12 & 0xfe0) << (31 - 11);
- u32 imm4_0 = (lo12 & 0x1f) << (11 - 4);
-
- return riscv_insn_rmw(location, 0x1fff07f, imm11_5 | imm4_0);
+ return riscv_insn_rmw(location,
+ RISCV_INSN_S_IMM_MASK,
+ riscv_insn_s_imm(v));
}

static int apply_r_riscv_got_hi20_rela(struct module *me, void *location,
@@ -207,14 +176,15 @@ static int apply_r_riscv_got_hi20_rela(struct module *me, void *location,
return -EINVAL;
}

- return riscv_insn_rmw(location, 0xfff, (offset + 0x800) & 0xfffff000);
+ return riscv_insn_rmw(location,
+ RISCV_INSN_U_IMM_MASK,
+ riscv_insn_u_imm(offset + 0x800));
}

static int apply_r_riscv_call_plt_rela(struct module *me, void *location,
Elf_Addr v)
{
ptrdiff_t offset = (void *)v - location;
- u32 hi20, lo12;

if (!riscv_insn_valid_32bit_offset(offset)) {
/* Only emit the plt entry if offset over 32-bit range */
@@ -228,17 +198,18 @@ static int apply_r_riscv_call_plt_rela(struct module *me, void *location,
}
}

- hi20 = (offset + 0x800) & 0xfffff000;
- lo12 = (offset - hi20) & 0xfff;
- riscv_insn_rmw(location, 0xfff, hi20);
- return riscv_insn_rmw(location + 4, 0xfffff, lo12 << 20);
+ riscv_insn_rmw(location,
+ RISCV_INSN_U_IMM_MASK,
+ riscv_insn_u_imm(offset + 0x800));
+ return riscv_insn_rmw(location + 4,
+ RISCV_INSN_I_IMM_MASK,
+ riscv_insn_i_imm(offset));
}

static int apply_r_riscv_call_rela(struct module *me, void *location,
Elf_Addr v)
{
ptrdiff_t offset = (void *)v - location;
- u32 hi20, lo12;

if (!riscv_insn_valid_32bit_offset(offset)) {
pr_err(
@@ -247,10 +218,12 @@ static int apply_r_riscv_call_rela(struct module *me, void *location,
return -EINVAL;
}

- hi20 = (offset + 0x800) & 0xfffff000;
- lo12 = (offset - hi20) & 0xfff;
- riscv_insn_rmw(location, 0xfff, hi20);
- return riscv_insn_rmw(location + 4, 0xfffff, lo12 << 20);
+ riscv_insn_rmw(location,
+ RISCV_INSN_U_IMM_MASK,
+ riscv_insn_u_imm(offset + 0x800));
+ return riscv_insn_rmw(location + 4,
+ RISCV_INSN_I_IMM_MASK,
+ riscv_insn_i_imm(offset));
}

static int apply_r_riscv_relax_rela(struct module *me, void *location,
--
2.35.1

2022-02-24 16:28:49

by Emil Renner Berthing

[permalink] [raw]
Subject: [PATCH v3 3/8] riscv: Remove unneeded definitions from asm/ftrace.h

The macros for generating the auipc + jalr instruction pair is only ever
used in kernel/ftrace.c, so move the definitions there.

Signed-off-by: Emil Renner Berthing <[email protected]>
---
arch/riscv/include/asm/ftrace.h | 35 +--------------------------------
arch/riscv/kernel/ftrace.c | 35 +++++++++++++++++++++++++++++++++
2 files changed, 36 insertions(+), 34 deletions(-)

diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
index 04dad3380041..585714993749 100644
--- a/arch/riscv/include/asm/ftrace.h
+++ b/arch/riscv/include/asm/ftrace.h
@@ -36,41 +36,8 @@ struct dyn_arch_ftrace {
#endif

#ifdef CONFIG_DYNAMIC_FTRACE
-/*
- * A general call in RISC-V is a pair of insts:
- * 1) auipc: setting high-20 pc-related bits to ra register
- * 2) jalr: setting low-12 offset to ra, jump to ra, and set ra to
- * return address (original pc + 4)
- *
- * Dynamic ftrace generates probes to call sites, so we must deal with
- * both auipc and jalr at the same time.
- */
-
-#define MCOUNT_ADDR ((unsigned long)MCOUNT_NAME)
-#define JALR_SIGN_MASK (0x00000800)
-#define JALR_OFFSET_MASK (0x00000fff)
-#define AUIPC_OFFSET_MASK (0xfffff000)
-#define AUIPC_PAD (0x00001000)
-#define JALR_SHIFT 20
-#define JALR_BASIC (0x000080e7)
-#define AUIPC_BASIC (0x00000097)
-#define NOP4 (0x00000013)
-
-#define make_call(caller, callee, call) \
-do { \
- call[0] = to_auipc_insn((unsigned int)((unsigned long)callee - \
- (unsigned long)caller)); \
- call[1] = to_jalr_insn((unsigned int)((unsigned long)callee - \
- (unsigned long)caller)); \
-} while (0)
-
-#define to_jalr_insn(offset) \
- (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_BASIC)

-#define to_auipc_insn(offset) \
- ((offset & JALR_SIGN_MASK) ? \
- (((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) | AUIPC_BASIC) : \
- ((offset & AUIPC_OFFSET_MASK) | AUIPC_BASIC))
+#define MCOUNT_ADDR ((unsigned long)MCOUNT_NAME)

/*
* Let auipc+jalr be the basic *mcount unit*, so we make it 8 bytes here.
diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
index 4716f4cdc038..2cc15dc45ce0 100644
--- a/arch/riscv/kernel/ftrace.c
+++ b/arch/riscv/kernel/ftrace.c
@@ -12,6 +12,41 @@
#include <asm/patch.h>

#ifdef CONFIG_DYNAMIC_FTRACE
+/*
+ * A general call in RISC-V is a pair of insts:
+ * 1) auipc: setting high-20 pc-related bits to ra register
+ * 2) jalr: setting low-12 offset to ra, jump to ra, and set ra to
+ * return address (original pc + 4)
+ *
+ * Dynamic ftrace generates probes to call sites, so we must deal with
+ * both auipc and jalr at the same time.
+ */
+
+#define JALR_SIGN_MASK (0x00000800)
+#define JALR_OFFSET_MASK (0x00000fff)
+#define AUIPC_OFFSET_MASK (0xfffff000)
+#define AUIPC_PAD (0x00001000)
+#define JALR_SHIFT 20
+#define JALR_BASIC (0x000080e7)
+#define AUIPC_BASIC (0x00000097)
+#define NOP4 (0x00000013)
+
+#define make_call(caller, callee, call) \
+do { \
+ call[0] = to_auipc_insn((unsigned int)((unsigned long)callee - \
+ (unsigned long)caller)); \
+ call[1] = to_jalr_insn((unsigned int)((unsigned long)callee - \
+ (unsigned long)caller)); \
+} while (0)
+
+#define to_jalr_insn(offset) \
+ (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_BASIC)
+
+#define to_auipc_insn(offset) \
+ ((offset & JALR_SIGN_MASK) ? \
+ (((offset & AUIPC_OFFSET_MASK) + AUIPC_PAD) | AUIPC_BASIC) : \
+ ((offset & AUIPC_OFFSET_MASK) | AUIPC_BASIC))
+
int ftrace_arch_code_modify_prepare(void) __acquires(&text_mutex)
{
mutex_lock(&text_mutex);
--
2.35.1

2022-02-24 16:31:31

by Emil Renner Berthing

[permalink] [raw]
Subject: [PATCH v3 2/8] riscv: Remove unneeded definitions from asm/module.h

The inline functions previously defined here are only ever
used in kernel/module-sections.c, so there is no need to
include them in every user of asm/module.h. Through
linux/module.h this is just about every driver.

Now that these functions are static in a single file
remove the inline marker to allow the compiler to make
its own decisions.

Signed-off-by: Emil Renner Berthing <[email protected]>
---
arch/riscv/include/asm/module.h | 87 ----------------------------
arch/riscv/kernel/module-sections.c | 90 +++++++++++++++++++++++++++++
2 files changed, 90 insertions(+), 87 deletions(-)

diff --git a/arch/riscv/include/asm/module.h b/arch/riscv/include/asm/module.h
index 76aa96a9fc08..570cd025f220 100644
--- a/arch/riscv/include/asm/module.h
+++ b/arch/riscv/include/asm/module.h
@@ -22,93 +22,6 @@ struct mod_arch_specific {
struct mod_section plt;
struct mod_section got_plt;
};
-
-struct got_entry {
- unsigned long symbol_addr; /* the real variable address */
-};
-
-static inline struct got_entry emit_got_entry(unsigned long val)
-{
- return (struct got_entry) {val};
-}
-
-static inline struct got_entry *get_got_entry(unsigned long val,
- const struct mod_section *sec)
-{
- struct got_entry *got = (struct got_entry *)(sec->shdr->sh_addr);
- int i;
- for (i = 0; i < sec->num_entries; i++) {
- if (got[i].symbol_addr == val)
- return &got[i];
- }
- return NULL;
-}
-
-struct plt_entry {
- /*
- * Trampoline code to real target address. The return address
- * should be the original (pc+4) before entring plt entry.
- */
- u32 insn_auipc; /* auipc t0, 0x0 */
- u32 insn_ld; /* ld t1, 0x10(t0) */
- u32 insn_jr; /* jr t1 */
-};
-
-#define OPC_AUIPC 0x0017
-#define OPC_LD 0x3003
-#define OPC_JALR 0x0067
-#define REG_T0 0x5
-#define REG_T1 0x6
-
-static inline struct plt_entry emit_plt_entry(unsigned long val,
- unsigned long plt,
- unsigned long got_plt)
-{
- /*
- * U-Type encoding:
- * +------------+----------+----------+
- * | imm[31:12] | rd[11:7] | opc[6:0] |
- * +------------+----------+----------+
- *
- * I-Type encoding:
- * +------------+------------+--------+----------+----------+
- * | imm[31:20] | rs1[19:15] | funct3 | rd[11:7] | opc[6:0] |
- * +------------+------------+--------+----------+----------+
- *
- */
- unsigned long offset = got_plt - plt;
- u32 hi20 = (offset + 0x800) & 0xfffff000;
- u32 lo12 = (offset - hi20);
- return (struct plt_entry) {
- OPC_AUIPC | (REG_T0 << 7) | hi20,
- OPC_LD | (lo12 << 20) | (REG_T0 << 15) | (REG_T1 << 7),
- OPC_JALR | (REG_T1 << 15)
- };
-}
-
-static inline int get_got_plt_idx(unsigned long val, const struct mod_section *sec)
-{
- struct got_entry *got_plt = (struct got_entry *)sec->shdr->sh_addr;
- int i;
- for (i = 0; i < sec->num_entries; i++) {
- if (got_plt[i].symbol_addr == val)
- return i;
- }
- return -1;
-}
-
-static inline struct plt_entry *get_plt_entry(unsigned long val,
- const struct mod_section *sec_plt,
- const struct mod_section *sec_got_plt)
-{
- struct plt_entry *plt = (struct plt_entry *)sec_plt->shdr->sh_addr;
- int got_plt_idx = get_got_plt_idx(val, sec_got_plt);
- if (got_plt_idx >= 0)
- return plt + got_plt_idx;
- else
- return NULL;
-}
-
#endif /* CONFIG_MODULE_SECTIONS */

#endif /* _ASM_RISCV_MODULE_H */
diff --git a/arch/riscv/kernel/module-sections.c b/arch/riscv/kernel/module-sections.c
index e264e59e596e..39d4ac681c2a 100644
--- a/arch/riscv/kernel/module-sections.c
+++ b/arch/riscv/kernel/module-sections.c
@@ -10,6 +10,28 @@
#include <linux/module.h>
#include <linux/moduleloader.h>

+struct got_entry {
+ unsigned long symbol_addr; /* the real variable address */
+};
+
+static struct got_entry emit_got_entry(unsigned long val)
+{
+ return (struct got_entry) {val};
+}
+
+static struct got_entry *get_got_entry(unsigned long val,
+ const struct mod_section *sec)
+{
+ struct got_entry *got = (struct got_entry *)(sec->shdr->sh_addr);
+ int i;
+
+ for (i = 0; i < sec->num_entries; i++) {
+ if (got[i].symbol_addr == val)
+ return &got[i];
+ }
+ return NULL;
+}
+
unsigned long module_emit_got_entry(struct module *mod, unsigned long val)
{
struct mod_section *got_sec = &mod->arch.got;
@@ -29,6 +51,74 @@ unsigned long module_emit_got_entry(struct module *mod, unsigned long val)
return (unsigned long)&got[i];
}

+struct plt_entry {
+ /*
+ * Trampoline code to real target address. The return address
+ * should be the original (pc+4) before entring plt entry.
+ */
+ u32 insn_auipc; /* auipc t0, 0x0 */
+ u32 insn_ld; /* ld t1, 0x10(t0) */
+ u32 insn_jr; /* jr t1 */
+};
+
+#define OPC_AUIPC 0x0017
+#define OPC_LD 0x3003
+#define OPC_JALR 0x0067
+#define REG_T0 0x5
+#define REG_T1 0x6
+
+static struct plt_entry emit_plt_entry(unsigned long val,
+ unsigned long plt,
+ unsigned long got_plt)
+{
+ /*
+ * U-Type encoding:
+ * +------------+----------+----------+
+ * | imm[31:12] | rd[11:7] | opc[6:0] |
+ * +------------+----------+----------+
+ *
+ * I-Type encoding:
+ * +------------+------------+--------+----------+----------+
+ * | imm[31:20] | rs1[19:15] | funct3 | rd[11:7] | opc[6:0] |
+ * +------------+------------+--------+----------+----------+
+ *
+ */
+ unsigned long offset = got_plt - plt;
+ u32 hi20 = (offset + 0x800) & 0xfffff000;
+ u32 lo12 = (offset - hi20);
+
+ return (struct plt_entry) {
+ OPC_AUIPC | (REG_T0 << 7) | hi20,
+ OPC_LD | (lo12 << 20) | (REG_T0 << 15) | (REG_T1 << 7),
+ OPC_JALR | (REG_T1 << 15)
+ };
+}
+
+static int get_got_plt_idx(unsigned long val, const struct mod_section *sec)
+{
+ struct got_entry *got_plt = (struct got_entry *)sec->shdr->sh_addr;
+ int i;
+
+ for (i = 0; i < sec->num_entries; i++) {
+ if (got_plt[i].symbol_addr == val)
+ return i;
+ }
+ return -1;
+}
+
+static struct plt_entry *get_plt_entry(unsigned long val,
+ const struct mod_section *sec_plt,
+ const struct mod_section *sec_got_plt)
+{
+ struct plt_entry *plt = (struct plt_entry *)sec_plt->shdr->sh_addr;
+ int got_plt_idx = get_got_plt_idx(val, sec_got_plt);
+
+ if (got_plt_idx >= 0)
+ return plt + got_plt_idx;
+ else
+ return NULL;
+}
+
unsigned long module_emit_plt_entry(struct module *mod, unsigned long val)
{
struct mod_section *got_plt_sec = &mod->arch.got_plt;
--
2.35.1

2022-02-24 16:32:49

by Emil Renner Berthing

[permalink] [raw]
Subject: [PATCH v3 6/8] riscv: Use asm/insn.h to generate plt entries

This converts kernel/module-sections.c to use asm/insn.h to generate
the instructions in the plt entries.

Signed-off-by: Emil Renner Berthing <[email protected]>
---
arch/riscv/kernel/module-sections.c | 27 ++++-----------------------
1 file changed, 4 insertions(+), 23 deletions(-)

diff --git a/arch/riscv/kernel/module-sections.c b/arch/riscv/kernel/module-sections.c
index 39d4ac681c2a..cb73399c3603 100644
--- a/arch/riscv/kernel/module-sections.c
+++ b/arch/riscv/kernel/module-sections.c
@@ -9,6 +9,7 @@
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/moduleloader.h>
+#include <asm/insn.h>

struct got_entry {
unsigned long symbol_addr; /* the real variable address */
@@ -61,36 +62,16 @@ struct plt_entry {
u32 insn_jr; /* jr t1 */
};

-#define OPC_AUIPC 0x0017
-#define OPC_LD 0x3003
-#define OPC_JALR 0x0067
-#define REG_T0 0x5
-#define REG_T1 0x6
-
static struct plt_entry emit_plt_entry(unsigned long val,
unsigned long plt,
unsigned long got_plt)
{
- /*
- * U-Type encoding:
- * +------------+----------+----------+
- * | imm[31:12] | rd[11:7] | opc[6:0] |
- * +------------+----------+----------+
- *
- * I-Type encoding:
- * +------------+------------+--------+----------+----------+
- * | imm[31:20] | rs1[19:15] | funct3 | rd[11:7] | opc[6:0] |
- * +------------+------------+--------+----------+----------+
- *
- */
unsigned long offset = got_plt - plt;
- u32 hi20 = (offset + 0x800) & 0xfffff000;
- u32 lo12 = (offset - hi20);

return (struct plt_entry) {
- OPC_AUIPC | (REG_T0 << 7) | hi20,
- OPC_LD | (lo12 << 20) | (REG_T0 << 15) | (REG_T1 << 7),
- OPC_JALR | (REG_T1 << 15)
+ RISCV_INSN_AUIPC | RISCV_INSN_RD_T0 | riscv_insn_u_imm(offset + 0x800),
+ RISCV_INSN_LD | RISCV_INSN_RD_T1 | RISCV_INSN_RS1_T0 | riscv_insn_i_imm(offset),
+ RISCV_INSN_JALR | RISCV_INSN_RS1_T1,
};
}

--
2.35.1

2022-02-24 16:36:23

by Emil Renner Berthing

[permalink] [raw]
Subject: [PATCH v3 7/8] riscv: Use asm/insn.h for jump labels

This converts kernel/jump_label.c to use asm/insn.h to generate the
jump/nop instructions.

Signed-off-by: Emil Renner Berthing <[email protected]>
---
arch/riscv/kernel/jump_label.c | 12 +++---------
1 file changed, 3 insertions(+), 9 deletions(-)

diff --git a/arch/riscv/kernel/jump_label.c b/arch/riscv/kernel/jump_label.c
index 20e09056d141..b5b4892c3e9e 100644
--- a/arch/riscv/kernel/jump_label.c
+++ b/arch/riscv/kernel/jump_label.c
@@ -9,11 +9,9 @@
#include <linux/memory.h>
#include <linux/mutex.h>
#include <asm/bug.h>
+#include <asm/insn.h>
#include <asm/patch.h>

-#define RISCV_INSN_NOP 0x00000013U
-#define RISCV_INSN_JAL 0x0000006fU
-
void arch_jump_label_transform(struct jump_entry *entry,
enum jump_label_type type)
{
@@ -23,14 +21,10 @@ void arch_jump_label_transform(struct jump_entry *entry,
if (type == JUMP_LABEL_JMP) {
long offset = jump_entry_target(entry) - jump_entry_code(entry);

- if (WARN_ON(offset & 1 || offset < -524288 || offset >= 524288))
+ if (WARN_ON(!riscv_insn_valid_20bit_offset(offset)))
return;

- insn = RISCV_INSN_JAL |
- (((u32)offset & GENMASK(19, 12)) << (12 - 12)) |
- (((u32)offset & GENMASK(11, 11)) << (20 - 11)) |
- (((u32)offset & GENMASK(10, 1)) << (21 - 1)) |
- (((u32)offset & GENMASK(20, 20)) << (31 - 20));
+ insn = RISCV_INSN_JAL | riscv_insn_j_imm(offset);
} else {
insn = RISCV_INSN_NOP;
}
--
2.35.1

2022-02-28 02:08:05

by Samuel Bronson

[permalink] [raw]
Subject: Re: [PATCH v3 1/8] riscv: Avoid unaligned access when relocating modules

Emil Renner Berthing <[email protected]> writes:

> With the C-extension regular 32bit instructions are not
> necessarily aligned on 4-byte boundaries. RISC-V instructions
> are in fact an ordered list of 16bit native-endian
> "parcels", so access the instruction as such.

Hold on a minute, this is what it says in my copy of the Unprivileged
ISA:

,----
| RISC-V base ISAs have either little-endian or big-endian memory systems,
| with the privileged architecture further defining bi-endian operation.
| Instructions are stored in memory as a sequence of 16-bit *little-endian*
| parcels, regardless of memory system endianness. Parcels forming one
| instruction are stored at increasing halfword addresses, with the
| *lowest-addressed parcel holding the lowest-numbered bits* in the
| instruction specification.
`----
[Emphasis mine.]

In other words, the parcels are little endian, and they're arranged in
little-endian order. System endianness doesn't matter, it collapses to
plain old little-endian.

(I'm really not sure why they describe the ordering in such a
round-about way; I assume that's the source of the confusion here?)

2022-02-28 13:57:38

by Emil Renner Berthing

[permalink] [raw]
Subject: Re: [PATCH v3 1/8] riscv: Avoid unaligned access when relocating modules

On Mon, 28 Feb 2022 at 01:45, Samuel Bronson <[email protected]> wrote:
>
> Emil Renner Berthing <[email protected]> writes:
>
> > With the C-extension regular 32bit instructions are not
> > necessarily aligned on 4-byte boundaries. RISC-V instructions
> > are in fact an ordered list of 16bit native-endian
> > "parcels", so access the instruction as such.
>
> Hold on a minute, this is what it says in my copy of the Unprivileged
> ISA:
>
> ,----
> | RISC-V base ISAs have either little-endian or big-endian memory systems,
> | with the privileged architecture further defining bi-endian operation.
> | Instructions are stored in memory as a sequence of 16-bit *little-endian*
> | parcels, regardless of memory system endianness. Parcels forming one
> | instruction are stored at increasing halfword addresses, with the
> | *lowest-addressed parcel holding the lowest-numbered bits* in the
> | instruction specification.
> `----
> [Emphasis mine.]
>
> In other words, the parcels are little endian, and they're arranged in
> little-endian order. System endianness doesn't matter, it collapses to
> plain old little-endian.

Wow, this actually changed since I wrote this code, nice catch. In
specs up until the latest it used to say "Instructions are stored in
memory with each 16-bit parcel
stored in a memory halfword according to the implementation’s natural
endianness". I hope noone designed a big-endian core before they
changed it.

> (I'm really not sure why they describe the ordering in such a
> round-about way; I assume that's the source of the confusion here?)

I guess the round-about way is exactly because the parcels used to be
native-endian, but the order of the parcels alsways little-endian.

/Emil