The first patch removes a bunch of code from the asm/module.h which is
included in almost all drivers through linux/module.h. Next are two
patches to fix unaligned access when doing module relocations and do
proper range checks for auipc+jalr offsets.
I'm a little less confident about the following patches, so consider
this more of an RFC for those. The idea is to consolidate the RISC-V
instruction generation and manipulation similar to arm64's asm/insn.h
header.
/Emil
Emil Renner Berthing (7):
riscv: Avoid unaligned access when relocating modules
riscv: Fix auipc+jalr relocation range checks
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: kernel/modules.c simplification
arch/riscv/include/asm/insn.h | 121 +++++++++++
arch/riscv/kernel/jump_label.c | 12 +-
arch/riscv/kernel/module-sections.c | 27 +--
arch/riscv/kernel/module.c | 326 +++++++++++++---------------
4 files changed, 276 insertions(+), 210 deletions(-)
create mode 100644 arch/riscv/include/asm/insn.h
--
2.35.1
RISC-V can do PC-relative jumps with a 32bit range using the following
two instructions:
auipc t0, imm20 ; t0 = PC + imm20 * 2^12
jalr ra, t0, imm12 ; ra = PC + 4, PC = t0 + imm12,
Crucially both the 20bit immediate imm20 and the 12bit immediate imm12
are treated as two's-complement signed values. For this reason the
immediates are usually calculated like this:
imm20 = (offset + 0x800) >> 12
imm12 = offset & 0xfff
..where offset is the signed offset from the auipc instruction. When
the 11th bit of offset is 0 the addition of 0x800 doesn't change the top
20 bits and imm12 considered positive. When the 11th bit is 1 the carry
of the addition by 0x800 means imm20 is one higher, but since imm12 is
then considered negative the two's complement representation means it
all cancels out nicely.
However, this addition by 0x800 (2^11) means an offset greater than or
equal to 2^31 - 2^11 would overflow so imm20 is considered negative and
result in a backwards jump. Similarly the lower range of offset is also
moved down by 2^11 and hence the true 32bit range is
[-2^31 - 2^11, 2^31 - 2^11)
Signed-off-by: Emil Renner Berthing <[email protected]>
---
arch/riscv/kernel/module.c | 20 +++++++++++++++-----
1 file changed, 15 insertions(+), 5 deletions(-)
diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 3d33442226e7..a75ccf3a6ce8 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -13,6 +13,18 @@
#include <linux/pgtable.h>
#include <asm/sections.h>
+static inline bool riscv_insn_valid_32bit_offset(ptrdiff_t val)
+{
+ if (IS_ENABLED(CONFIG_32BIT))
+ return true;
+
+ /*
+ * auipc+jalr can reach any PC-relative offset in the range
+ * [-2^31 - 2^11, 2^31 - 2^11)
+ */
+ return (-(1L << 31) - (1L << 11)) <= val && val < ((1L << 31) - (1L << 11));
+}
+
static int riscv_insn_rmw(void *location, u32 keep, u32 set)
{
u16 *parcel = location;
@@ -111,7 +123,7 @@ static int apply_r_riscv_pcrel_hi20_rela(struct module *me, void *location,
{
ptrdiff_t offset = (void *)v - location;
- if (offset != (s32)offset) {
+ if (!riscv_insn_valid_32bit_offset(offset)) {
pr_err(
"%s: target %016llx can not be addressed by the 32-bit offset from PC = %p\n",
me->name, (long long)v, location);
@@ -201,10 +213,9 @@ static int apply_r_riscv_call_plt_rela(struct module *me, void *location,
Elf_Addr v)
{
ptrdiff_t offset = (void *)v - location;
- s32 fill_v = offset;
u32 hi20, lo12;
- if (offset != fill_v) {
+ 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 = (void *)module_emit_plt_entry(me, v) - location;
@@ -226,10 +237,9 @@ static int apply_r_riscv_call_rela(struct module *me, void *location,
Elf_Addr v)
{
ptrdiff_t offset = (void *)v - location;
- s32 fill_v = offset;
u32 hi20, lo12;
- if (offset != fill_v) {
+ if (!riscv_insn_valid_32bit_offset(offset)) {
pr_err(
"%s: target %016llx can not be addressed by the 32-bit offset from PC = %p\n",
me->name, (long long)v, location);
--
2.35.1
Signed-off-by: Emil Renner Berthing <[email protected]>
---
arch/riscv/kernel/module.c | 93 ++++++++++++++++----------------------
1 file changed, 39 insertions(+), 54 deletions(-)
diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 2212d88776e0..e371977aecfd 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -298,24 +298,23 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
unsigned int symindex, unsigned int relsec,
struct module *me)
{
- Elf_Rela *rel = (void *) sechdrs[relsec].sh_addr;
- int (*handler)(struct module *me, void *location, Elf_Addr v);
- Elf_Sym *sym;
- void *location;
- unsigned int i, type;
- Elf_Addr v;
- int res;
+ Elf_Rela *rel = (void *)sechdrs[relsec].sh_addr;
+ unsigned int entries = sechdrs[relsec].sh_size / sizeof(*rel);
+ unsigned int i;
pr_debug("Applying relocate section %u to %u\n", relsec,
sechdrs[relsec].sh_info);
- for (i = 0; i < sechdrs[relsec].sh_size / sizeof(*rel); i++) {
- /* This is where to make the change */
- location = (void *)sechdrs[sechdrs[relsec].sh_info].sh_addr
- + rel[i].r_offset;
- /* This is the symbol it is referring to */
- sym = (Elf_Sym *)sechdrs[symindex].sh_addr
+ for (i = 0; i < entries; i++) {
+ Elf_Sym *sym = (Elf_Sym *)sechdrs[symindex].sh_addr
+ ELF_RISCV_R_SYM(rel[i].r_info);
+ Elf_Addr loc = sechdrs[sechdrs[relsec].sh_info].sh_addr
+ + rel[i].r_offset;
+ unsigned int type = ELF_RISCV_R_TYPE(rel[i].r_info);
+ int (*handler)(struct module *me, void *location, Elf_Addr v);
+ Elf_Addr v;
+ int res;
+
if (IS_ERR_VALUE(sym->st_value)) {
/* Ignore unresolved weak symbol */
if (ELF_ST_BIND(sym->st_info) == STB_WEAK)
@@ -325,8 +324,6 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
return -ENOENT;
}
- type = ELF_RISCV_R_TYPE(rel[i].r_info);
-
if (type < ARRAY_SIZE(reloc_handlers_rela))
handler = reloc_handlers_rela[type];
else
@@ -343,48 +340,36 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
if (type == R_RISCV_PCREL_LO12_I || type == R_RISCV_PCREL_LO12_S) {
unsigned int j;
- for (j = 0; j < sechdrs[relsec].sh_size / sizeof(*rel); j++) {
- unsigned long hi20_loc =
- sechdrs[sechdrs[relsec].sh_info].sh_addr
+ /* find the corresponding HI20 entry */
+ for (j = 0; j < entries; j++) {
+ Elf_Sym *hi20_sym = (Elf_Sym *)sechdrs[symindex].sh_addr
+ + ELF_RISCV_R_SYM(rel[j].r_info);
+ Elf_Addr hi20_loc = sechdrs[sechdrs[relsec].sh_info].sh_addr
+ rel[j].r_offset;
- u32 hi20_type = ELF_RISCV_R_TYPE(rel[j].r_info);
-
- /* Find the corresponding HI20 relocation entry */
- if (hi20_loc == sym->st_value
- && (hi20_type == R_RISCV_PCREL_HI20
- || hi20_type == R_RISCV_GOT_HI20)) {
- s32 hi20, lo12;
- Elf_Sym *hi20_sym =
- (Elf_Sym *)sechdrs[symindex].sh_addr
- + ELF_RISCV_R_SYM(rel[j].r_info);
- unsigned long hi20_sym_val =
- hi20_sym->st_value
- + rel[j].r_addend;
-
- /* Calculate lo12 */
- size_t offset = hi20_sym_val - hi20_loc;
- if (IS_ENABLED(CONFIG_MODULE_SECTIONS)
- && hi20_type == R_RISCV_GOT_HI20) {
- offset = module_emit_got_entry(
- me, hi20_sym_val);
- offset = offset - hi20_loc;
- }
- hi20 = (offset + 0x800) & 0xfffff000;
- lo12 = offset - hi20;
- v = lo12;
-
- break;
- }
- }
- if (j == sechdrs[relsec].sh_size / sizeof(*rel)) {
- pr_err(
- "%s: Can not find HI20 relocation information\n",
- me->name);
- return -EINVAL;
+ unsigned int hi20_type = ELF_RISCV_R_TYPE(rel[j].r_info);
+
+ if (hi20_loc != sym->st_value ||
+ (hi20_type != R_RISCV_PCREL_HI20 &&
+ hi20_type != R_RISCV_GOT_HI20))
+ continue;
+
+ /* calculate relative offset */
+ v = hi20_sym->st_value + rel[j].r_addend;
+
+ if (IS_ENABLED(CONFIG_MODULE_SECTIONS) &&
+ hi20_type == R_RISCV_GOT_HI20)
+ v = module_emit_got_entry(me, v);
+
+ v -= hi20_loc;
+ goto handle_reloc;
}
- }
- res = handler(me, location, v);
+ pr_err("%s: Cannot find HI20 relocation information\n",
+ me->name);
+ return -EINVAL;
+ }
+handle_reloc:
+ res = handler(me, (void *)loc, v);
if (res)
return res;
}
--
2.35.1
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 | 138 +++++++++++++++----------------------
1 file changed, 56 insertions(+), 82 deletions(-)
diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index a75ccf3a6ce8..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,38 +12,27 @@
#include <linux/vmalloc.h>
#include <linux/sizes.h>
#include <linux/pgtable.h>
+#include <asm/insn.h>
#include <asm/sections.h>
-static inline bool riscv_insn_valid_32bit_offset(ptrdiff_t val)
-{
- if (IS_ENABLED(CONFIG_32BIT))
- return true;
-
- /*
- * auipc+jalr can reach any PC-relative offset in the range
- * [-2^31 - 2^11, 2^31 - 2^11)
- */
- return (-(1L << 31) - (1L << 11)) <= val && val < ((1L << 31) - (1L << 11));
-}
-
-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;
}
@@ -67,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,
@@ -130,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,
@@ -166,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,
@@ -206,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 */
@@ -227,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(
@@ -246,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