All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] RISC-V: Make modules loadable.
@ 2018-02-23  3:12 Shea Levy
  2018-02-23  3:12 ` [PATCH 1/9] RISC-V: Build non position-independent modules Shea Levy
                   ` (9 more replies)
  0 siblings, 10 replies; 24+ messages in thread
From: Shea Levy @ 2018-02-23  3:12 UTC (permalink / raw)
  To: linux-riscv

This was enough to load enough modules to make virtio_scsi devices
appear on the qemu virt machine on a highly modular distro kernel.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 1/9] RISC-V: Build non position-independent modules.
  2018-02-23  3:12 [PATCH 0/9] RISC-V: Make modules loadable Shea Levy
@ 2018-02-23  3:12 ` Shea Levy
  2018-02-23  3:12 ` [PATCH 2/9] RISC-V: Load modules within relative jump range of the kernel text Shea Levy
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 24+ messages in thread
From: Shea Levy @ 2018-02-23  3:12 UTC (permalink / raw)
  To: linux-riscv

The relocations we have to handle for no-PIC are much simpler, and we
can't take advantage of GOT/PLT sharing anyway.

Signed-off-by: Shea Levy <shea@shealevy.com>
---
 arch/riscv/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/Makefile b/arch/riscv/Makefile
index 26892daefa05..e0b04a0b5b82 100644
--- a/arch/riscv/Makefile
+++ b/arch/riscv/Makefile
@@ -11,8 +11,8 @@
 LDFLAGS         :=
 OBJCOPYFLAGS    := -O binary
 LDFLAGS_vmlinux :=
-KBUILD_AFLAGS_MODULE += -fPIC
-KBUILD_CFLAGS_MODULE += -fPIC
+KBUILD_AFLAGS_MODULE += -fno-PIC -mcmodel=medany
+KBUILD_CFLAGS_MODULE += -fno-PIC -mcmodel=medany
 
 KBUILD_DEFCONFIG = defconfig
 
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 2/9] RISC-V: Load modules within relative jump range of the kernel text.
  2018-02-23  3:12 [PATCH 0/9] RISC-V: Make modules loadable Shea Levy
  2018-02-23  3:12 ` [PATCH 1/9] RISC-V: Build non position-independent modules Shea Levy
@ 2018-02-23  3:12 ` Shea Levy
  2018-02-27  0:35   ` Palmer Dabbelt
  2018-02-23  3:12 ` [PATCH 3/9] RISC-V: Support RISCV_CALL relocations for non-PIC modules Shea Levy
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Shea Levy @ 2018-02-23  3:12 UTC (permalink / raw)
  To: linux-riscv

Signed-off-by: Shea Levy <shea@shealevy.com>
---
 arch/riscv/include/asm/pgtable.h |  9 +++++++++
 arch/riscv/kernel/module.c       | 11 +++++++++++
 2 files changed, 20 insertions(+)

diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
index 16301966d65b..b08ded13364a 100644
--- a/arch/riscv/include/asm/pgtable.h
+++ b/arch/riscv/include/asm/pgtable.h
@@ -25,6 +25,7 @@
 #include <asm/page.h>
 #include <asm/tlbflush.h>
 #include <linux/mm_types.h>
+#include <linux/sizes.h>
 
 #ifdef CONFIG_64BIT
 #include <asm/pgtable-64.h>
@@ -425,6 +426,14 @@ static inline void pgtable_cache_init(void)
 #define TASK_SIZE VMALLOC_START
 #endif
 
+/*
+ * The module space lives between the addresses given by TASK_SIZE
+ * and PAGE_OFFSET - it must be within 2G of the kernel text.
+ */
+#define MODULES_SIZE		(SZ_128M)
+#define MODULES_VADDR		(PAGE_OFFSET - MODULES_SIZE)
+#define MODULES_END		(VMALLOC_END)
+
 #include <asm-generic/pgtable.h>
 
 #endif /* !__ASSEMBLY__ */
diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index e0f05034fc21..8ad24bdf69de 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -16,6 +16,8 @@
 #include <linux/err.h>
 #include <linux/errno.h>
 #include <linux/moduleloader.h>
+#include <linux/vmalloc.h>
+#include <asm/pgtable.h>
 
 static int apply_r_riscv_64_rela(struct module *me, u32 *location, Elf_Addr v)
 {
@@ -215,3 +217,12 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
 
 	return 0;
 }
+
+void *module_alloc(unsigned long size)
+{
+	return __vmalloc_node_range(size, 1, MODULES_VADDR,
+				    MODULES_END, GFP_KERNEL,
+				    PAGE_KERNEL_EXEC, 0,
+				    NUMA_NO_NODE,
+				    __builtin_return_address(0));
+}
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 3/9] RISC-V: Support RISCV_CALL relocations for non-PIC modules.
  2018-02-23  3:12 [PATCH 0/9] RISC-V: Make modules loadable Shea Levy
  2018-02-23  3:12 ` [PATCH 1/9] RISC-V: Build non position-independent modules Shea Levy
  2018-02-23  3:12 ` [PATCH 2/9] RISC-V: Load modules within relative jump range of the kernel text Shea Levy
@ 2018-02-23  3:12 ` Shea Levy
  2018-02-27  0:35   ` Palmer Dabbelt
  2018-02-23  3:12 ` [PATCH 4/9] RISC-V: Support RISCV_RVC_BRANCH relocations in modules Shea Levy
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 24+ messages in thread
From: Shea Levy @ 2018-02-23  3:12 UTC (permalink / raw)
  To: linux-riscv

Only PIC objects have RISCV_CALL_PLT relocations.

Signed-off-by: Shea Levy <shea@shealevy.com>
---
 arch/riscv/kernel/module.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 8ad24bdf69de..0db01486a357 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -94,7 +94,7 @@ static int apply_r_riscv_pcrel_lo12_s_rela(struct module *me, u32 *location,
 	return 0;
 }
 
-static int apply_r_riscv_call_plt_rela(struct module *me, u32 *location,
+static int apply_r_riscv_call_rela(struct module *me, u32 *location,
 				       Elf_Addr v)
 {
 	s64 offset = (void *)v - (void *)location;
@@ -129,7 +129,7 @@ static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
 	[R_RISCV_PCREL_HI20]		= apply_r_riscv_pcrel_hi20_rela,
 	[R_RISCV_PCREL_LO12_I]		= apply_r_riscv_pcrel_lo12_i_rela,
 	[R_RISCV_PCREL_LO12_S]		= apply_r_riscv_pcrel_lo12_s_rela,
-	[R_RISCV_CALL_PLT]		= apply_r_riscv_call_plt_rela,
+	[R_RISCV_CALL]			= apply_r_riscv_call_rela,
 	[R_RISCV_RELAX]			= apply_r_riscv_relax_rela,
 };
 
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 4/9] RISC-V: Support RISCV_RVC_BRANCH relocations in modules.
  2018-02-23  3:12 [PATCH 0/9] RISC-V: Make modules loadable Shea Levy
                   ` (2 preceding siblings ...)
  2018-02-23  3:12 ` [PATCH 3/9] RISC-V: Support RISCV_CALL relocations for non-PIC modules Shea Levy
@ 2018-02-23  3:12 ` Shea Levy
  2018-02-23  3:12 ` [PATCH 5/9] RISC-V: Support RISCV_RVC_JUMP " Shea Levy
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 24+ messages in thread
From: Shea Levy @ 2018-02-23  3:12 UTC (permalink / raw)
  To: linux-riscv

Signed-off-by: Shea Levy <shea@shealevy.com>
---
 arch/riscv/kernel/module.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 0db01486a357..49ebfd4a7026 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -38,6 +38,22 @@ static int apply_r_riscv_branch_rela(struct module *me, u32 *location,
 	return 0;
 }
 
+static int apply_r_riscv_rvc_branch_rela(struct module *me, u32 *location,
+					 Elf_Addr v)
+{
+	s64 offset = (void *)v - (void *)location;
+	u32 imm8 = (offset & 0x100) << (12 - 8);
+	u32 imm7_6 = (offset & 0xc0) >> (7 - 6);
+	u32 imm5 = (offset & 0x20) >> (5 - 2);
+	u32 imm4_3 = (offset & 0x18) << (11 - 4);
+	u32 imm2_1 = (offset & 0x6) << (4 - 2);
+
+	u16 *short_location = (u16 *) location;
+
+	*short_location = (*short_location & 0xe383) | imm8 | imm7_6 | imm5 | imm4_3 | imm2_1;
+	return 0;
+}
+
 static int apply_r_riscv_jal_rela(struct module *me, u32 *location,
 				  Elf_Addr v)
 {
@@ -125,6 +141,7 @@ static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
 				Elf_Addr v) = {
 	[R_RISCV_64]			= apply_r_riscv_64_rela,
 	[R_RISCV_BRANCH]		= apply_r_riscv_branch_rela,
+	[R_RISCV_RVC_BRANCH]		= apply_r_riscv_rvc_branch_rela,
 	[R_RISCV_JAL]			= apply_r_riscv_jal_rela,
 	[R_RISCV_PCREL_HI20]		= apply_r_riscv_pcrel_hi20_rela,
 	[R_RISCV_PCREL_LO12_I]		= apply_r_riscv_pcrel_lo12_i_rela,
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 5/9] RISC-V: Support RISCV_RVC_JUMP relocations in modules.
  2018-02-23  3:12 [PATCH 0/9] RISC-V: Make modules loadable Shea Levy
                   ` (3 preceding siblings ...)
  2018-02-23  3:12 ` [PATCH 4/9] RISC-V: Support RISCV_RVC_BRANCH relocations in modules Shea Levy
@ 2018-02-23  3:12 ` Shea Levy
  2018-02-23  3:12 ` [PATCH 6/9] RISC-V: Support RISCV_ADD32 " Shea Levy
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 24+ messages in thread
From: Shea Levy @ 2018-02-23  3:12 UTC (permalink / raw)
  To: linux-riscv

Signed-off-by: Shea Levy <shea@shealevy.com>
---
 arch/riscv/kernel/module.c | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 49ebfd4a7026..30b29c5b6600 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -67,6 +67,25 @@ static int apply_r_riscv_jal_rela(struct module *me, u32 *location,
 	return 0;
 }
 
+static int apply_r_riscv_rvc_jump_rela(struct module *me, u32 *location,
+				       Elf_Addr v)
+{
+	s64 offset = (void *)v - (void *)location;
+	u32 imm11 = (offset & 0x800) << (12 - 11);
+	u32 imm10 = (offset & 0x400) >> (10 - 8);
+	u32 imm9_8 = (offset & 0x300) << (10 - 9);
+	u32 imm7 = (offset & 0x80) >> (7 - 6);
+	u32 imm6 = (offset & 0x40) << (7 - 6);
+	u32 imm5 = (offset & 0x20) >> (5 - 2);
+	u32 imm4 = (offset & 0x10) << (11 - 4);
+	u32 imm3_1 = (offset & 0xe) << (5 - 3);
+
+	u16 *short_location = (u16 *) location;
+
+	*short_location = (*short_location & 0xe003) | imm11 | imm10 | imm9_8 | imm7 | imm6 | imm5 | imm4 | imm3_1;
+	return 0;
+}
+
 static int apply_r_riscv_pcrel_hi20_rela(struct module *me, u32 *location,
 					 Elf_Addr v)
 {
@@ -143,6 +162,7 @@ static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
 	[R_RISCV_BRANCH]		= apply_r_riscv_branch_rela,
 	[R_RISCV_RVC_BRANCH]		= apply_r_riscv_rvc_branch_rela,
 	[R_RISCV_JAL]			= apply_r_riscv_jal_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,
 	[R_RISCV_PCREL_LO12_S]		= apply_r_riscv_pcrel_lo12_s_rela,
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 6/9] RISC-V: Support RISCV_ADD32 relocations in modules.
  2018-02-23  3:12 [PATCH 0/9] RISC-V: Make modules loadable Shea Levy
                   ` (4 preceding siblings ...)
  2018-02-23  3:12 ` [PATCH 5/9] RISC-V: Support RISCV_RVC_JUMP " Shea Levy
@ 2018-02-23  3:12 ` Shea Levy
  2018-02-23  3:12 ` [PATCH 7/9] RISC-V: Support RISCV_SUB32 " Shea Levy
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 24+ messages in thread
From: Shea Levy @ 2018-02-23  3:12 UTC (permalink / raw)
  To: linux-riscv

Signed-off-by: Shea Levy <shea@shealevy.com>
---
 arch/riscv/kernel/module.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 30b29c5b6600..25b4aae5b8de 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -156,6 +156,13 @@ static int apply_r_riscv_relax_rela(struct module *me, u32 *location,
 	return 0;
 }
 
+static int apply_r_riscv_add32_rela(struct module *me, u32 *location,
+				    Elf_Addr v)
+{
+	*location += v;
+	return 0;
+}
+
 static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
 				Elf_Addr v) = {
 	[R_RISCV_64]			= apply_r_riscv_64_rela,
@@ -168,6 +175,7 @@ static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
 	[R_RISCV_PCREL_LO12_S]		= apply_r_riscv_pcrel_lo12_s_rela,
 	[R_RISCV_CALL]			= apply_r_riscv_call_rela,
 	[R_RISCV_RELAX]			= apply_r_riscv_relax_rela,
+	[R_RISCV_ADD32]			= apply_r_riscv_add32_rela,
 };
 
 int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 7/9] RISC-V: Support RISCV_SUB32 relocations in modules.
  2018-02-23  3:12 [PATCH 0/9] RISC-V: Make modules loadable Shea Levy
                   ` (5 preceding siblings ...)
  2018-02-23  3:12 ` [PATCH 6/9] RISC-V: Support RISCV_ADD32 " Shea Levy
@ 2018-02-23  3:12 ` Shea Levy
  2018-02-23  3:12 ` [PATCH 8/9] RISC-V: Support RISCV_ALIGN " Shea Levy
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 24+ messages in thread
From: Shea Levy @ 2018-02-23  3:12 UTC (permalink / raw)
  To: linux-riscv

Signed-off-by: Shea Levy <shea@shealevy.com>
---
 arch/riscv/kernel/module.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 25b4aae5b8de..4f3e15e7995b 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -163,6 +163,13 @@ static int apply_r_riscv_add32_rela(struct module *me, u32 *location,
 	return 0;
 }
 
+static int apply_r_riscv_sub32_rela(struct module *me, u32 *location,
+				    Elf_Addr v)
+{
+	*location -= v;
+	return 0;
+}
+
 static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
 				Elf_Addr v) = {
 	[R_RISCV_64]			= apply_r_riscv_64_rela,
@@ -176,6 +183,7 @@ static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
 	[R_RISCV_CALL]			= apply_r_riscv_call_rela,
 	[R_RISCV_RELAX]			= apply_r_riscv_relax_rela,
 	[R_RISCV_ADD32]			= apply_r_riscv_add32_rela,
+	[R_RISCV_SUB32]			= apply_r_riscv_sub32_rela,
 };
 
 int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 8/9] RISC-V: Support RISCV_ALIGN relocations in modules.
  2018-02-23  3:12 [PATCH 0/9] RISC-V: Make modules loadable Shea Levy
                   ` (6 preceding siblings ...)
  2018-02-23  3:12 ` [PATCH 7/9] RISC-V: Support RISCV_SUB32 " Shea Levy
@ 2018-02-23  3:12 ` Shea Levy
  2018-02-27  0:35   ` Palmer Dabbelt
  2018-02-23  3:12 ` [PATCH 9/9] RISC-V: Build a module-enabled kernel by default Shea Levy
  2018-02-27  0:35 ` [PATCH 0/9] RISC-V: Make modules loadable Palmer Dabbelt
  9 siblings, 1 reply; 24+ messages in thread
From: Shea Levy @ 2018-02-23  3:12 UTC (permalink / raw)
  To: linux-riscv

While legal, this noop implementation may be inefficient.

Signed-off-by: Shea Levy <shea@shealevy.com>
---
 arch/riscv/kernel/module.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 4f3e15e7995b..827cf1211360 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -150,8 +150,8 @@ static int apply_r_riscv_call_rela(struct module *me, u32 *location,
 	return 0;
 }
 
-static int apply_r_riscv_relax_rela(struct module *me, u32 *location,
-				    Elf_Addr v)
+static int apply_r_riscv_noop_rela(struct module *me, u32 *location,
+				   Elf_Addr v)
 {
 	return 0;
 }
@@ -181,7 +181,8 @@ static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
 	[R_RISCV_PCREL_LO12_I]		= apply_r_riscv_pcrel_lo12_i_rela,
 	[R_RISCV_PCREL_LO12_S]		= apply_r_riscv_pcrel_lo12_s_rela,
 	[R_RISCV_CALL]			= apply_r_riscv_call_rela,
-	[R_RISCV_RELAX]			= apply_r_riscv_relax_rela,
+	[R_RISCV_RELAX]			= apply_r_riscv_noop_rela,
+	[R_RISCV_ALIGN]			= apply_r_riscv_noop_rela,
 	[R_RISCV_ADD32]			= apply_r_riscv_add32_rela,
 	[R_RISCV_SUB32]			= apply_r_riscv_sub32_rela,
 };
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 9/9] RISC-V: Build a module-enabled kernel by default.
  2018-02-23  3:12 [PATCH 0/9] RISC-V: Make modules loadable Shea Levy
                   ` (7 preceding siblings ...)
  2018-02-23  3:12 ` [PATCH 8/9] RISC-V: Support RISCV_ALIGN " Shea Levy
@ 2018-02-23  3:12 ` Shea Levy
  2018-02-27  0:35 ` [PATCH 0/9] RISC-V: Make modules loadable Palmer Dabbelt
  9 siblings, 0 replies; 24+ messages in thread
From: Shea Levy @ 2018-02-23  3:12 UTC (permalink / raw)
  To: linux-riscv

Signed-off-by: Shea Levy <shea@shealevy.com>
---
 arch/riscv/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/riscv/configs/defconfig b/arch/riscv/configs/defconfig
index 47dacf06c679..56ad83919b42 100644
--- a/arch/riscv/configs/defconfig
+++ b/arch/riscv/configs/defconfig
@@ -73,3 +73,4 @@ CONFIG_NFS_V4_2=y
 CONFIG_ROOT_NFS=y
 # CONFIG_RCU_TRACE is not set
 CONFIG_CRYPTO_USER_API_HASH=y
+CONFIG_MODULES=y
-- 
2.16.1

^ permalink raw reply related	[flat|nested] 24+ messages in thread

* [PATCH 3/9] RISC-V: Support RISCV_CALL relocations for non-PIC modules.
  2018-02-23  3:12 ` [PATCH 3/9] RISC-V: Support RISCV_CALL relocations for non-PIC modules Shea Levy
@ 2018-02-27  0:35   ` Palmer Dabbelt
  0 siblings, 0 replies; 24+ messages in thread
From: Palmer Dabbelt @ 2018-02-27  0:35 UTC (permalink / raw)
  To: linux-riscv

On Thu, 22 Feb 2018 19:12:12 PST (-0800), shea at shealevy.com wrote:
> Only PIC objects have RISCV_CALL_PLT relocations.
>
> Signed-off-by: Shea Levy <shea@shealevy.com>
> ---
>  arch/riscv/kernel/module.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
> index 8ad24bdf69de..0db01486a357 100644
> --- a/arch/riscv/kernel/module.c
> +++ b/arch/riscv/kernel/module.c
> @@ -94,7 +94,7 @@ static int apply_r_riscv_pcrel_lo12_s_rela(struct module *me, u32 *location,
>  	return 0;
>  }
>
> -static int apply_r_riscv_call_plt_rela(struct module *me, u32 *location,
> +static int apply_r_riscv_call_rela(struct module *me, u32 *location,
>  				       Elf_Addr v)

This is misaligned, but don't bother submitting another patch set just for 
this.  I'll fix it up.

>  {
>  	s64 offset = (void *)v - (void *)location;
> @@ -129,7 +129,7 @@ static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
>  	[R_RISCV_PCREL_HI20]		= apply_r_riscv_pcrel_hi20_rela,
>  	[R_RISCV_PCREL_LO12_I]		= apply_r_riscv_pcrel_lo12_i_rela,
>  	[R_RISCV_PCREL_LO12_S]		= apply_r_riscv_pcrel_lo12_s_rela,
> -	[R_RISCV_CALL_PLT]		= apply_r_riscv_call_plt_rela,
> +	[R_RISCV_CALL]			= apply_r_riscv_call_rela,
>  	[R_RISCV_RELAX]			= apply_r_riscv_relax_rela,
>  };

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 2/9] RISC-V: Load modules within relative jump range of the kernel text.
  2018-02-23  3:12 ` [PATCH 2/9] RISC-V: Load modules within relative jump range of the kernel text Shea Levy
@ 2018-02-27  0:35   ` Palmer Dabbelt
  2018-03-01 11:23     ` Shea Levy
  0 siblings, 1 reply; 24+ messages in thread
From: Palmer Dabbelt @ 2018-02-27  0:35 UTC (permalink / raw)
  To: linux-riscv

On Thu, 22 Feb 2018 19:12:11 PST (-0800), shea at shealevy.com wrote:
> Signed-off-by: Shea Levy <shea@shealevy.com>
> ---
>  arch/riscv/include/asm/pgtable.h |  9 +++++++++
>  arch/riscv/kernel/module.c       | 11 +++++++++++
>  2 files changed, 20 insertions(+)
>
> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
> index 16301966d65b..b08ded13364a 100644
> --- a/arch/riscv/include/asm/pgtable.h
> +++ b/arch/riscv/include/asm/pgtable.h
> @@ -25,6 +25,7 @@
>  #include <asm/page.h>
>  #include <asm/tlbflush.h>
>  #include <linux/mm_types.h>
> +#include <linux/sizes.h>
>
>  #ifdef CONFIG_64BIT
>  #include <asm/pgtable-64.h>
> @@ -425,6 +426,14 @@ static inline void pgtable_cache_init(void)
>  #define TASK_SIZE VMALLOC_START
>  #endif
>
> +/*
> + * The module space lives between the addresses given by TASK_SIZE
> + * and PAGE_OFFSET - it must be within 2G of the kernel text.
> + */
> +#define MODULES_SIZE		(SZ_128M)
> +#define MODULES_VADDR		(PAGE_OFFSET - MODULES_SIZE)
> +#define MODULES_END		(VMALLOC_END)
> +
>  #include <asm-generic/pgtable.h>
>
>  #endif /* !__ASSEMBLY__ */
> diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
> index e0f05034fc21..8ad24bdf69de 100644
> --- a/arch/riscv/kernel/module.c
> +++ b/arch/riscv/kernel/module.c
> @@ -16,6 +16,8 @@
>  #include <linux/err.h>
>  #include <linux/errno.h>
>  #include <linux/moduleloader.h>
> +#include <linux/vmalloc.h>
> +#include <asm/pgtable.h>
>
>  static int apply_r_riscv_64_rela(struct module *me, u32 *location, Elf_Addr v)
>  {
> @@ -215,3 +217,12 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
>
>  	return 0;
>  }
> +
> +void *module_alloc(unsigned long size)
> +{
> +	return __vmalloc_node_range(size, 1, MODULES_VADDR,
> +				    MODULES_END, GFP_KERNEL,
> +				    PAGE_KERNEL_EXEC, 0,
> +				    NUMA_NO_NODE,
> +				    __builtin_return_address(0));
> +}

arm64 aligns their modules to a page boundary.  I think we need to align them 
to at least whatever the linker is expecting, which should be recorded in each 
segment in the ELF that's provided.

This code otherwise looks exactly teh same as MIPS, maybe we should make it 
generic somewhere?  It's not that much, though, but it looks like it'll get 
bigger as things support KASLR and such.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 8/9] RISC-V: Support RISCV_ALIGN relocations in modules.
  2018-02-23  3:12 ` [PATCH 8/9] RISC-V: Support RISCV_ALIGN " Shea Levy
@ 2018-02-27  0:35   ` Palmer Dabbelt
  2018-03-01 11:22     ` Shea Levy
  0 siblings, 1 reply; 24+ messages in thread
From: Palmer Dabbelt @ 2018-02-27  0:35 UTC (permalink / raw)
  To: linux-riscv

On Thu, 22 Feb 2018 19:12:17 PST (-0800), shea at shealevy.com wrote:
> While legal, this noop implementation may be inefficient.
>
> Signed-off-by: Shea Levy <shea@shealevy.com>
> ---
>  arch/riscv/kernel/module.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
> index 4f3e15e7995b..827cf1211360 100644
> --- a/arch/riscv/kernel/module.c
> +++ b/arch/riscv/kernel/module.c
> @@ -150,8 +150,8 @@ static int apply_r_riscv_call_rela(struct module *me, u32 *location,
>  	return 0;
>  }
>
> -static int apply_r_riscv_relax_rela(struct module *me, u32 *location,
> -				    Elf_Addr v)
> +static int apply_r_riscv_noop_rela(struct module *me, u32 *location,
> +				   Elf_Addr v)
>  {
>  	return 0;
>  }
> @@ -181,7 +181,8 @@ static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
>  	[R_RISCV_PCREL_LO12_I]		= apply_r_riscv_pcrel_lo12_i_rela,
>  	[R_RISCV_PCREL_LO12_S]		= apply_r_riscv_pcrel_lo12_s_rela,
>  	[R_RISCV_CALL]			= apply_r_riscv_call_rela,
> -	[R_RISCV_RELAX]			= apply_r_riscv_relax_rela,
> +	[R_RISCV_RELAX]			= apply_r_riscv_noop_rela,
> +	[R_RISCV_ALIGN]			= apply_r_riscv_noop_rela,
>  	[R_RISCV_ADD32]			= apply_r_riscv_add32_rela,
>  	[R_RISCV_SUB32]			= apply_r_riscv_sub32_rela,
>  };

The output of most RISC-V toolchains requires a relaxation pass for alignment 
for correctness.  Even if you're enforcing this somewhere in your module build 
flow you should at least check for alignment when loading modules so there 
aren't silent breakages, if R_RISCV_ALIGN is escaping to the kernel's loader 
then it probably needs to be handled.

For example, I did this (not the best example, but there should be some way to 
generate this from C I was lazy):

    diff --git a/crypto/authenc.c b/crypto/authenc.c
    index d3d6d72fe649..4715d7f63f3e 100644
    --- a/crypto/authenc.c
    +++ b/crypto/authenc.c
    @@ -42,6 +42,17 @@ struct authenc_request_ctx {
            char tail[];
     };
    
    +__asm__ (
    +".align 2"
    +"\n\t.global align2"
    +"\n\talign2:"
    +"\n\tret"
    +"\n\t.align 3"
    +"\n\t.global align3"
    +"\n\talign3:"
    +"\n\tret"
    +);
    +
     static void authenc_request_complete(struct aead_request *req, int err)
     {
            if (err != -EINPROGRESS)

and ended up with some misaligned stuff:

    $ riscv64-unknown-elf-objdump -d -r crypto/authenc.ko | head -n25
    
    crypto/authenc.ko:     file format elf64-littleriscv
    
    
    Disassembly of section .text:
    
    0000000000000000 <align2-0x2>:
       0:	0001                	nop
    			0: R_RISCV_ALIGN	*ABS*+0x2
    
    0000000000000002 <align2>:
       2:	8082                	ret
       4:	0001                	nop
    			4: R_RISCV_ALIGN	*ABS*+0x6
       6:	00000013          	nop
    
    000000000000000a <align3>:
       a:	8082                	ret
    
    000000000000000c <crypto_authenc_extractkeys>:
       c:	1141                	addi	sp,sp,-16
       e:	e422                	sd	s0,8(sp)
      10:	0800                	addi	s0,sp,16
      12:	470d                	li	a4,3
      14:	08c77063          	bleu	a2,a4,94 <.L4>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/9] RISC-V: Make modules loadable.
  2018-02-23  3:12 [PATCH 0/9] RISC-V: Make modules loadable Shea Levy
                   ` (8 preceding siblings ...)
  2018-02-23  3:12 ` [PATCH 9/9] RISC-V: Build a module-enabled kernel by default Shea Levy
@ 2018-02-27  0:35 ` Palmer Dabbelt
  2018-02-27  5:49   ` Stef O'Rear
  9 siblings, 1 reply; 24+ messages in thread
From: Palmer Dabbelt @ 2018-02-27  0:35 UTC (permalink / raw)
  To: linux-riscv

On Thu, 22 Feb 2018 19:12:09 PST (-0800), shea at shealevy.com wrote:
> This was enough to load enough modules to make virtio_scsi devices
> appear on the qemu virt machine on a highly modular distro kernel.

Thanks for this, but there's a bit of an issue with R_RISCV_ALIGN.  I'd be OK 
merging the change set if you emit errors on R_RISCV_ALIGN instances that 
actually require alignment, as IIRC it's possible to coerce GCC to emit code 
that doesn't require relaxation of alignment sequences.  This should probably go 
in the module CFLAGS as well.

There are also some minor comments, see the per-patch messages.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/9] RISC-V: Make modules loadable.
  2018-02-27  0:35 ` [PATCH 0/9] RISC-V: Make modules loadable Palmer Dabbelt
@ 2018-02-27  5:49   ` Stef O'Rear
  2018-02-27 18:21     ` Palmer Dabbelt
  0 siblings, 1 reply; 24+ messages in thread
From: Stef O'Rear @ 2018-02-27  5:49 UTC (permalink / raw)
  To: linux-riscv

On Mon, Feb 26, 2018 at 4:35 PM, Palmer Dabbelt <palmer@sifive.com> wrote:
> On Thu, 22 Feb 2018 19:12:09 PST (-0800), shea at shealevy.com wrote:
>>
>> This was enough to load enough modules to make virtio_scsi devices
>> appear on the qemu virt machine on a highly modular distro kernel.
>
>
> Thanks for this, but there's a bit of an issue with R_RISCV_ALIGN.  I'd be
> OK merging the change set if you emit errors on R_RISCV_ALIGN instances that
> actually require alignment, as IIRC it's possible to coerce GCC to emit code
> that doesn't require relaxation of alignment sequences.  This should
> probably go in the module CFLAGS as well.
>
> There are also some minor comments, see the per-patch messages.

The question of whether processing R_RISCV_ALIGN is mandatory came up
during lld patch review,
https://lists.llvm.org/pipermail/llvm-dev/2017-July/115407.html .   My
position is that if gas is generating instances that require
alignment, that is a bug, although I'm quite fine with hardening this.

-s

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/9] RISC-V: Make modules loadable.
  2018-02-27  5:49   ` Stef O'Rear
@ 2018-02-27 18:21     ` Palmer Dabbelt
  2018-02-27 18:26       ` Stef O'Rear
  0 siblings, 1 reply; 24+ messages in thread
From: Palmer Dabbelt @ 2018-02-27 18:21 UTC (permalink / raw)
  To: linux-riscv

On Mon, 26 Feb 2018 21:49:40 PST (-0800), sorear2 at gmail.com wrote:
> On Mon, Feb 26, 2018 at 4:35 PM, Palmer Dabbelt <palmer@sifive.com> wrote:
>> On Thu, 22 Feb 2018 19:12:09 PST (-0800), shea at shealevy.com wrote:
>>>
>>> This was enough to load enough modules to make virtio_scsi devices
>>> appear on the qemu virt machine on a highly modular distro kernel.
>>
>>
>> Thanks for this, but there's a bit of an issue with R_RISCV_ALIGN.  I'd be
>> OK merging the change set if you emit errors on R_RISCV_ALIGN instances that
>> actually require alignment, as IIRC it's possible to coerce GCC to emit code
>> that doesn't require relaxation of alignment sequences.  This should
>> probably go in the module CFLAGS as well.
>>
>> There are also some minor comments, see the per-patch messages.
>
> The question of whether processing R_RISCV_ALIGN is mandatory came up
> during lld patch review,
> https://lists.llvm.org/pipermail/llvm-dev/2017-July/115407.html .   My
> position is that if gas is generating instances that require
> alignment, that is a bug, although I'm quite fine with hardening this.

We'd need to add a new relocation in order to make that happen, as 
R_RISCV_ALIGN assumes.  I wouldn't be opposed to it, but I thought the 
conclusion we came to was that there were two reasons to avoid processing 
R_RISCV_ALIGN:

* The linker doesn't support relaxation.  We consider this a bug in the linker, 
  and I don't want to add a relocation just for incomplete linkers.
* Relaxation is too time consuming.  In this case you want to disable all 
  relaxation, and GAS will already emit code without R_RISCV_ALIGN when run 
  with relaxation disabled.

IIRC there was an email thread or a github issue about this.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/9] RISC-V: Make modules loadable.
  2018-02-27 18:21     ` Palmer Dabbelt
@ 2018-02-27 18:26       ` Stef O'Rear
  2018-02-27 21:21         ` Palmer Dabbelt
  0 siblings, 1 reply; 24+ messages in thread
From: Stef O'Rear @ 2018-02-27 18:26 UTC (permalink / raw)
  To: linux-riscv

On Tue, Feb 27, 2018 at 10:21 AM, Palmer Dabbelt <palmer@sifive.com> wrote:
> * Relaxation is too time consuming.  In this case you want to disable all
> relaxation, and GAS will already emit code without R_RISCV_ALIGN when run
> with relaxation disabled.

Tangentially, I described a linear time algorithm for relaxation a
while ago, but binutils is still using a quadratic time one.  Would
patches be accepted?

-s

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 0/9] RISC-V: Make modules loadable.
  2018-02-27 18:26       ` Stef O'Rear
@ 2018-02-27 21:21         ` Palmer Dabbelt
  0 siblings, 0 replies; 24+ messages in thread
From: Palmer Dabbelt @ 2018-02-27 21:21 UTC (permalink / raw)
  To: linux-riscv

On Tue, 27 Feb 2018 10:26:48 PST (-0800), sorear2 at gmail.com wrote:
> On Tue, Feb 27, 2018 at 10:21 AM, Palmer Dabbelt <palmer@sifive.com> wrote:
>> * Relaxation is too time consuming.  In this case you want to disable all
>> relaxation, and GAS will already emit code without R_RISCV_ALIGN when run
>> with relaxation disabled.
>
> Tangentially, I described a linear time algorithm for relaxation a
> while ago, but binutils is still using a quadratic time one.  Would
> patches be accepted?

Yes.  There's a tiny bit of support in there for it via R_RISCV_DELETE, I think 
we just need to convert the rest of the relaxations over to that and then flip 
around the pass that handles R_RISCV_DELETE and we should be good.

Of course, R_RISCV_DELETE isn't a public relocation so if you have a better way 
to handle it then I'm all ears :).

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 8/9] RISC-V: Support RISCV_ALIGN relocations in modules.
  2018-02-27  0:35   ` Palmer Dabbelt
@ 2018-03-01 11:22     ` Shea Levy
  2018-03-01 21:54       ` Palmer Dabbelt
  0 siblings, 1 reply; 24+ messages in thread
From: Shea Levy @ 2018-03-01 11:22 UTC (permalink / raw)
  To: linux-riscv

Hi Palmer,

Palmer Dabbelt <palmer@sifive.com> writes:

> On Thu, 22 Feb 2018 19:12:17 PST (-0800), shea at shealevy.com wrote:
>> While legal, this noop implementation may be inefficient.
>>
>> Signed-off-by: Shea Levy <shea@shealevy.com>
>> ---
>>  arch/riscv/kernel/module.c | 7 ++++---
>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
>> index 4f3e15e7995b..827cf1211360 100644
>> --- a/arch/riscv/kernel/module.c
>> +++ b/arch/riscv/kernel/module.c
>> @@ -150,8 +150,8 @@ static int apply_r_riscv_call_rela(struct module *me, u32 *location,
>>  	return 0;
>>  }
>>
>> -static int apply_r_riscv_relax_rela(struct module *me, u32 *location,
>> -				    Elf_Addr v)
>> +static int apply_r_riscv_noop_rela(struct module *me, u32 *location,
>> +				   Elf_Addr v)
>>  {
>>  	return 0;
>>  }
>> @@ -181,7 +181,8 @@ static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
>>  	[R_RISCV_PCREL_LO12_I]		= apply_r_riscv_pcrel_lo12_i_rela,
>>  	[R_RISCV_PCREL_LO12_S]		= apply_r_riscv_pcrel_lo12_s_rela,
>>  	[R_RISCV_CALL]			= apply_r_riscv_call_rela,
>> -	[R_RISCV_RELAX]			= apply_r_riscv_relax_rela,
>> +	[R_RISCV_RELAX]			= apply_r_riscv_noop_rela,
>> +	[R_RISCV_ALIGN]			= apply_r_riscv_noop_rela,
>>  	[R_RISCV_ADD32]			= apply_r_riscv_add32_rela,
>>  	[R_RISCV_SUB32]			= apply_r_riscv_sub32_rela,
>>  };
>
> The output of most RISC-V toolchains requires a relaxation pass for alignment 
> for correctness.  Even if you're enforcing this somewhere in your module build 
> flow you should at least check for alignment when loading modules so there 
> aren't silent breakages, if R_RISCV_ALIGN is escaping to the kernel's loader 
> then it probably needs to be handled.
>

Well, I guess the question is: Do we want to do relaxations at all at
load time? If so, I'll work on that and include RISCV_ALIGN. If not, is
there any way to configure gcc and/or ld not to emit any relaxations? In
the latter case we can just explicitly not list RISCV_ALIGN here and the
module loader will fail with an unsupported relocation.

>
> For example, I did this (not the best example, but there should be some way to 
> generate this from C I was lazy):
>
>     diff --git a/crypto/authenc.c b/crypto/authenc.c
>     index d3d6d72fe649..4715d7f63f3e 100644
>     --- a/crypto/authenc.c
>     +++ b/crypto/authenc.c
>     @@ -42,6 +42,17 @@ struct authenc_request_ctx {
>             char tail[];
>      };
>     
>     +__asm__ (
>     +".align 2"
>     +"\n\t.global align2"
>     +"\n\talign2:"
>     +"\n\tret"
>     +"\n\t.align 3"
>     +"\n\t.global align3"
>     +"\n\talign3:"
>     +"\n\tret"
>     +);
>     +
>      static void authenc_request_complete(struct aead_request *req, int err)
>      {
>             if (err != -EINPROGRESS)
>
> and ended up with some misaligned stuff:
>
>     $ riscv64-unknown-elf-objdump -d -r crypto/authenc.ko | head -n25
>     
>     crypto/authenc.ko:     file format elf64-littleriscv
>     
>     
>     Disassembly of section .text:
>     
>     0000000000000000 <align2-0x2>:
>        0:	0001                	nop
>     			0: R_RISCV_ALIGN	*ABS*+0x2
>     
>     0000000000000002 <align2>:
>        2:	8082                	ret
>        4:	0001                	nop
>     			4: R_RISCV_ALIGN	*ABS*+0x6
>        6:	00000013          	nop
>     
>     000000000000000a <align3>:
>        a:	8082                	ret
>     
>     000000000000000c <crypto_authenc_extractkeys>:
>        c:	1141                	addi	sp,sp,-16
>        e:	e422                	sd	s0,8(sp)
>       10:	0800                	addi	s0,sp,16
>       12:	470d                	li	a4,3
>       14:	08c77063          	bleu	a2,a4,94 <.L4>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-riscv/attachments/20180301/8416b6ce/attachment.sig>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 2/9] RISC-V: Load modules within relative jump range of the kernel text.
  2018-02-27  0:35   ` Palmer Dabbelt
@ 2018-03-01 11:23     ` Shea Levy
  0 siblings, 0 replies; 24+ messages in thread
From: Shea Levy @ 2018-03-01 11:23 UTC (permalink / raw)
  To: linux-riscv

Hi Palmer,

Palmer Dabbelt <palmer@sifive.com> writes:

> On Thu, 22 Feb 2018 19:12:11 PST (-0800), shea at shealevy.com wrote:
>> Signed-off-by: Shea Levy <shea@shealevy.com>
>> ---
>>  arch/riscv/include/asm/pgtable.h |  9 +++++++++
>>  arch/riscv/kernel/module.c       | 11 +++++++++++
>>  2 files changed, 20 insertions(+)
>>
>> diff --git a/arch/riscv/include/asm/pgtable.h b/arch/riscv/include/asm/pgtable.h
>> index 16301966d65b..b08ded13364a 100644
>> --- a/arch/riscv/include/asm/pgtable.h
>> +++ b/arch/riscv/include/asm/pgtable.h
>> @@ -25,6 +25,7 @@
>>  #include <asm/page.h>
>>  #include <asm/tlbflush.h>
>>  #include <linux/mm_types.h>
>> +#include <linux/sizes.h>
>>
>>  #ifdef CONFIG_64BIT
>>  #include <asm/pgtable-64.h>
>> @@ -425,6 +426,14 @@ static inline void pgtable_cache_init(void)
>>  #define TASK_SIZE VMALLOC_START
>>  #endif
>>
>> +/*
>> + * The module space lives between the addresses given by TASK_SIZE
>> + * and PAGE_OFFSET - it must be within 2G of the kernel text.
>> + */
>> +#define MODULES_SIZE		(SZ_128M)
>> +#define MODULES_VADDR		(PAGE_OFFSET - MODULES_SIZE)
>> +#define MODULES_END		(VMALLOC_END)
>> +
>>  #include <asm-generic/pgtable.h>
>>
>>  #endif /* !__ASSEMBLY__ */
>> diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
>> index e0f05034fc21..8ad24bdf69de 100644
>> --- a/arch/riscv/kernel/module.c
>> +++ b/arch/riscv/kernel/module.c
>> @@ -16,6 +16,8 @@
>>  #include <linux/err.h>
>>  #include <linux/errno.h>
>>  #include <linux/moduleloader.h>
>> +#include <linux/vmalloc.h>
>> +#include <asm/pgtable.h>
>>
>>  static int apply_r_riscv_64_rela(struct module *me, u32 *location, Elf_Addr v)
>>  {
>> @@ -215,3 +217,12 @@ int apply_relocate_add(Elf_Shdr *sechdrs, const char *strtab,
>>
>>  	return 0;
>>  }
>> +
>> +void *module_alloc(unsigned long size)
>> +{
>> +	return __vmalloc_node_range(size, 1, MODULES_VADDR,
>> +				    MODULES_END, GFP_KERNEL,
>> +				    PAGE_KERNEL_EXEC, 0,
>> +				    NUMA_NO_NODE,
>> +				    __builtin_return_address(0));
>> +}
>
> arm64 aligns their modules to a page boundary.  I think we need to align them 
> to at least whatever the linker is expecting, which should be recorded in each 
> segment in the ELF that's provided.
>
> This code otherwise looks exactly teh same as MIPS, maybe we should make it 
> generic somewhere?  It's not that much, though, but it looks like it'll get 
> bigger as things support KASLR and such.

Thanks, I'll include this fix when I resolve the RISCV_ALIGN issue, and
see if it makes sense to make generic.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-riscv/attachments/20180301/56fddeab/attachment.sig>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 8/9] RISC-V: Support RISCV_ALIGN relocations in modules.
  2018-03-01 11:22     ` Shea Levy
@ 2018-03-01 21:54       ` Palmer Dabbelt
  2018-03-01 23:53         ` Shea Levy
  0 siblings, 1 reply; 24+ messages in thread
From: Palmer Dabbelt @ 2018-03-01 21:54 UTC (permalink / raw)
  To: linux-riscv

On Thu, 01 Mar 2018 03:22:27 PST (-0800), shea at shealevy.com wrote:
> Hi Palmer,
>
> Palmer Dabbelt <palmer@sifive.com> writes:
>
>> On Thu, 22 Feb 2018 19:12:17 PST (-0800), shea at shealevy.com wrote:
>>> While legal, this noop implementation may be inefficient.
>>>
>>> Signed-off-by: Shea Levy <shea@shealevy.com>
>>> ---
>>>  arch/riscv/kernel/module.c | 7 ++++---
>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
>>> index 4f3e15e7995b..827cf1211360 100644
>>> --- a/arch/riscv/kernel/module.c
>>> +++ b/arch/riscv/kernel/module.c
>>> @@ -150,8 +150,8 @@ static int apply_r_riscv_call_rela(struct module *me, u32 *location,
>>>  	return 0;
>>>  }
>>>
>>> -static int apply_r_riscv_relax_rela(struct module *me, u32 *location,
>>> -				    Elf_Addr v)
>>> +static int apply_r_riscv_noop_rela(struct module *me, u32 *location,
>>> +				   Elf_Addr v)
>>>  {
>>>  	return 0;
>>>  }
>>> @@ -181,7 +181,8 @@ static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
>>>  	[R_RISCV_PCREL_LO12_I]		= apply_r_riscv_pcrel_lo12_i_rela,
>>>  	[R_RISCV_PCREL_LO12_S]		= apply_r_riscv_pcrel_lo12_s_rela,
>>>  	[R_RISCV_CALL]			= apply_r_riscv_call_rela,
>>> -	[R_RISCV_RELAX]			= apply_r_riscv_relax_rela,
>>> +	[R_RISCV_RELAX]			= apply_r_riscv_noop_rela,
>>> +	[R_RISCV_ALIGN]			= apply_r_riscv_noop_rela,
>>>  	[R_RISCV_ADD32]			= apply_r_riscv_add32_rela,
>>>  	[R_RISCV_SUB32]			= apply_r_riscv_sub32_rela,
>>>  };
>>
>> The output of most RISC-V toolchains requires a relaxation pass for alignment 
>> for correctness.  Even if you're enforcing this somewhere in your module build 
>> flow you should at least check for alignment when loading modules so there 
>> aren't silent breakages, if R_RISCV_ALIGN is escaping to the kernel's loader 
>> then it probably needs to be handled.
>>
>
> Well, I guess the question is: Do we want to do relaxations at all at
> load time? If so, I'll work on that and include RISCV_ALIGN. If not, is
> there any way to configure gcc and/or ld not to emit any relaxations? In
> the latter case we can just explicitly not list RISCV_ALIGN here and the
> module loader will fail with an unsupported relocation.

Well, this is awkward: I remembered doing this (and telling people about 
"-mno-relax"), but apparently that was all just in my head.  Here's a patch to 
add "-mno-relax" to GCC in the real world :) 
https://gcc.gnu.org/ml/gcc-patches/2018-03/msg00062.html

I think the best thing to do is:

* Check if the complier supports "-mno-relax" and add it to the module build 
  command-line.
* Change R_RISCV_ALIGN support in the kernel to check if the address happens to 
  already be aligned (which happens sometimes), and if it's not then provide a 
  helpful error message along the lines of "upgrade to GCC 8".

I guess we eventually want to support relaxation in the kernel, but only if 
there's a real performance problem that someone can point out.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 8/9] RISC-V: Support RISCV_ALIGN relocations in modules.
  2018-03-01 21:54       ` Palmer Dabbelt
@ 2018-03-01 23:53         ` Shea Levy
  2018-03-02  9:12           ` Stef O'Rear
  0 siblings, 1 reply; 24+ messages in thread
From: Shea Levy @ 2018-03-01 23:53 UTC (permalink / raw)
  To: linux-riscv

Palmer Dabbelt <palmer@sifive.com> writes:

> On Thu, 01 Mar 2018 03:22:27 PST (-0800), shea at shealevy.com wrote:
>> Hi Palmer,
>>
>> Palmer Dabbelt <palmer@sifive.com> writes:
>>
>>> On Thu, 22 Feb 2018 19:12:17 PST (-0800), shea at shealevy.com wrote:
>>>> While legal, this noop implementation may be inefficient.
>>>>
>>>> Signed-off-by: Shea Levy <shea@shealevy.com>
>>>> ---
>>>>  arch/riscv/kernel/module.c | 7 ++++---
>>>>  1 file changed, 4 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
>>>> index 4f3e15e7995b..827cf1211360 100644
>>>> --- a/arch/riscv/kernel/module.c
>>>> +++ b/arch/riscv/kernel/module.c
>>>> @@ -150,8 +150,8 @@ static int apply_r_riscv_call_rela(struct module *me, u32 *location,
>>>>  	return 0;
>>>>  }
>>>>
>>>> -static int apply_r_riscv_relax_rela(struct module *me, u32 *location,
>>>> -				    Elf_Addr v)
>>>> +static int apply_r_riscv_noop_rela(struct module *me, u32 *location,
>>>> +				   Elf_Addr v)
>>>>  {
>>>>  	return 0;
>>>>  }
>>>> @@ -181,7 +181,8 @@ static int (*reloc_handlers_rela[]) (struct module *me, u32 *location,
>>>>  	[R_RISCV_PCREL_LO12_I]		= apply_r_riscv_pcrel_lo12_i_rela,
>>>>  	[R_RISCV_PCREL_LO12_S]		= apply_r_riscv_pcrel_lo12_s_rela,
>>>>  	[R_RISCV_CALL]			= apply_r_riscv_call_rela,
>>>> -	[R_RISCV_RELAX]			= apply_r_riscv_relax_rela,
>>>> +	[R_RISCV_RELAX]			= apply_r_riscv_noop_rela,
>>>> +	[R_RISCV_ALIGN]			= apply_r_riscv_noop_rela,
>>>>  	[R_RISCV_ADD32]			= apply_r_riscv_add32_rela,
>>>>  	[R_RISCV_SUB32]			= apply_r_riscv_sub32_rela,
>>>>  };
>>>
>>> The output of most RISC-V toolchains requires a relaxation pass for alignment 
>>> for correctness.  Even if you're enforcing this somewhere in your module build 
>>> flow you should at least check for alignment when loading modules so there 
>>> aren't silent breakages, if R_RISCV_ALIGN is escaping to the kernel's loader 
>>> then it probably needs to be handled.
>>>
>>
>> Well, I guess the question is: Do we want to do relaxations at all at
>> load time? If so, I'll work on that and include RISCV_ALIGN. If not, is
>> there any way to configure gcc and/or ld not to emit any relaxations? In
>> the latter case we can just explicitly not list RISCV_ALIGN here and the
>> module loader will fail with an unsupported relocation.
>
> Well, this is awkward: I remembered doing this (and telling people about 
> "-mno-relax"), but apparently that was all just in my head.  Here's a patch to 
> add "-mno-relax" to GCC in the real world :) 
> https://gcc.gnu.org/ml/gcc-patches/2018-03/msg00062.html
>
> I think the best thing to do is:
>
> * Check if the complier supports "-mno-relax" and add it to the module build 
>   command-line.
> * Change R_RISCV_ALIGN support in the kernel to check if the address happens to 
>   already be aligned (which happens sometimes), and if it's not then provide a 
>   helpful error message along the lines of "upgrade to GCC 8".
>
> I guess we eventually want to support relaxation in the kernel, but only if 
> there's a real performance problem that someone can point out.

OK, thanks! I'll look into that.

Actually, other than the initial implementation cost, is there any
potential *problem* with implementing relaxations? If I have time I
might want to look into that just for my own education.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 832 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-riscv/attachments/20180301/9c5d9289/attachment.sig>

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 8/9] RISC-V: Support RISCV_ALIGN relocations in modules.
  2018-03-01 23:53         ` Shea Levy
@ 2018-03-02  9:12           ` Stef O'Rear
  2018-03-06 23:45             ` Palmer Dabbelt
  0 siblings, 1 reply; 24+ messages in thread
From: Stef O'Rear @ 2018-03-02  9:12 UTC (permalink / raw)
  To: linux-riscv

On Thu, Mar 1, 2018 at 3:53 PM, Shea Levy <shea@shealevy.com> wrote:
> Palmer Dabbelt <palmer@sifive.com> writes:
>> Well, this is awkward: I remembered doing this (and telling people about
>> "-mno-relax"), but apparently that was all just in my head.  Here's a patch to
>> add "-mno-relax" to GCC in the real world :)
>> https://gcc.gnu.org/ml/gcc-patches/2018-03/msg00062.html
>>
>> I think the best thing to do is:
>>
>> * Check if the complier supports "-mno-relax" and add it to the module build
>>   command-line.
>> * Change R_RISCV_ALIGN support in the kernel to check if the address happens to
>>   already be aligned (which happens sometimes), and if it's not then provide a
>>   helpful error message along the lines of "upgrade to GCC 8".
>>
>> I guess we eventually want to support relaxation in the kernel, but only if
>> there's a real performance problem that someone can point out.
>
> OK, thanks! I'll look into that.
>
> Actually, other than the initial implementation cost, is there any
> potential *problem* with implementing relaxations? If I have time I
> might want to look into that just for my own education.

It might be a reasonable exercise but I'd rather not upstream it.

-mno-relax .o files ought to be order of 2x smaller, so assuming you
care about initramfs size we want to use that instead of complicating
the loader.

(I'd be interested in real numbers for a larger module, e.g. btrfs.)

-s

^ permalink raw reply	[flat|nested] 24+ messages in thread

* [PATCH 8/9] RISC-V: Support RISCV_ALIGN relocations in modules.
  2018-03-02  9:12           ` Stef O'Rear
@ 2018-03-06 23:45             ` Palmer Dabbelt
  0 siblings, 0 replies; 24+ messages in thread
From: Palmer Dabbelt @ 2018-03-06 23:45 UTC (permalink / raw)
  To: linux-riscv

On Fri, 02 Mar 2018 01:12:26 PST (-0800), sorear2 at gmail.com wrote:
> On Thu, Mar 1, 2018 at 3:53 PM, Shea Levy <shea@shealevy.com> wrote:
>> Palmer Dabbelt <palmer@sifive.com> writes:
>>> Well, this is awkward: I remembered doing this (and telling people about
>>> "-mno-relax"), but apparently that was all just in my head.  Here's a patch to
>>> add "-mno-relax" to GCC in the real world :)
>>> https://gcc.gnu.org/ml/gcc-patches/2018-03/msg00062.html
>>>
>>> I think the best thing to do is:
>>>
>>> * Check if the complier supports "-mno-relax" and add it to the module build
>>>   command-line.
>>> * Change R_RISCV_ALIGN support in the kernel to check if the address happens to
>>>   already be aligned (which happens sometimes), and if it's not then provide a
>>>   helpful error message along the lines of "upgrade to GCC 8".
>>>
>>> I guess we eventually want to support relaxation in the kernel, but only if
>>> there's a real performance problem that someone can point out.
>>
>> OK, thanks! I'll look into that.
>>
>> Actually, other than the initial implementation cost, is there any
>> potential *problem* with implementing relaxations? If I have time I
>> might want to look into that just for my own education.
>
> It might be a reasonable exercise but I'd rather not upstream it.
>
> -mno-relax .o files ought to be order of 2x smaller, so assuming you
> care about initramfs size we want to use that instead of complicating
> the loader.
>
> (I'd be interested in real numbers for a larger module, e.g. btrfs.)

It's actually kind of an odd one: using "-mrelax" instead of "-mno-relax" would 
result in:

* Larger initramfs sizes (the kernel modules would still have all the 
  pre-relaxed instructions, plus the extra relocation info required to relax 
  and the most pessimistic alignment possible).
* Larger, slower, more complicated module loading code (to handle all the 
  relaxations).
* Smaller, faster modules once they've been loaded into the kernel.

I guess we'd need to see some numbers as to how much can actually be relaxed, 
as I suspect there won't be a whole lot of opportunities for relaxation in 
module code.  I think this is probably not that high priority, but it would be 
fun :).

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2018-03-06 23:45 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-23  3:12 [PATCH 0/9] RISC-V: Make modules loadable Shea Levy
2018-02-23  3:12 ` [PATCH 1/9] RISC-V: Build non position-independent modules Shea Levy
2018-02-23  3:12 ` [PATCH 2/9] RISC-V: Load modules within relative jump range of the kernel text Shea Levy
2018-02-27  0:35   ` Palmer Dabbelt
2018-03-01 11:23     ` Shea Levy
2018-02-23  3:12 ` [PATCH 3/9] RISC-V: Support RISCV_CALL relocations for non-PIC modules Shea Levy
2018-02-27  0:35   ` Palmer Dabbelt
2018-02-23  3:12 ` [PATCH 4/9] RISC-V: Support RISCV_RVC_BRANCH relocations in modules Shea Levy
2018-02-23  3:12 ` [PATCH 5/9] RISC-V: Support RISCV_RVC_JUMP " Shea Levy
2018-02-23  3:12 ` [PATCH 6/9] RISC-V: Support RISCV_ADD32 " Shea Levy
2018-02-23  3:12 ` [PATCH 7/9] RISC-V: Support RISCV_SUB32 " Shea Levy
2018-02-23  3:12 ` [PATCH 8/9] RISC-V: Support RISCV_ALIGN " Shea Levy
2018-02-27  0:35   ` Palmer Dabbelt
2018-03-01 11:22     ` Shea Levy
2018-03-01 21:54       ` Palmer Dabbelt
2018-03-01 23:53         ` Shea Levy
2018-03-02  9:12           ` Stef O'Rear
2018-03-06 23:45             ` Palmer Dabbelt
2018-02-23  3:12 ` [PATCH 9/9] RISC-V: Build a module-enabled kernel by default Shea Levy
2018-02-27  0:35 ` [PATCH 0/9] RISC-V: Make modules loadable Palmer Dabbelt
2018-02-27  5:49   ` Stef O'Rear
2018-02-27 18:21     ` Palmer Dabbelt
2018-02-27 18:26       ` Stef O'Rear
2018-02-27 21:21         ` Palmer Dabbelt

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.