Skip to contentSkip to conversation

riscv: improve boot time isa extensions handling

PATCH Superseded Jisheng Zhang 5 participants 24 messages Jan 15, 2023 – Jan 28, 2023

Subsystems (4)

v4 of 5 Superseded 1 of 13 applied Latest is v5 →

lore
Share & tools
lore
#0 lore raw

v4 cover riscv: improve boot time isa extensions handling

Generally, riscv ISA extensions are fixed for any specific hardware platform, so a hart's features won't change after booting, this chacteristic makes it straightforward to use a static branch to check a specific ISA extension is supported or not to optimize performance.

However, some ISA extensions such as SVPBMT and ZICBOM are handled via. the alternative sequences.

Basically, for ease of maintenance, we prefer to use static branches in C code, but recently, Samuel found that the static branch usage in cpu_relax() breaks building with CONFIG_CC_OPTIMIZE_FOR_SIZE[1]. As Samuel pointed out, "Having a static branch in cpu_relax() is problematic because that function is widely inlined, including in some quite complex functions like in the VDSO. A quick measurement shows this static branch is responsible by itself for around 40% of the jump table."

Samuel's findings pointed out one of a few downsides of static branches usage in C code to handle ISA extensions detected at boot time:
static branch's metadata in the __jump_table section, which is not discarded after ISA extensions are finalized, wastes some space.

I want to try to solve the issue for all possible dynamic handling of ISA extensions at boot time. Inspired by Mark[2], this patch introduces riscv_has_extension_*() helpers, which work like static branches but are patched using alternatives, thus the metadata can be freed after patching.

Since v3
- collect Reviewed-by tag and remove Heiko's reviewed-by from patch5
- address Conor and Andrew comments
- fix two building errors of !MMU and RV32

Since v2
- rebase on riscv-next
- collect Reviewed-by tag
- fix jal imm construction
- combine Heiko's code and my code for jal patching, thus add Co-developed-by tag
- address comments from Conor

Since v1
- rebase on v6.1-rc7 + Heiko's alternative improvements[3]
- collect Reviewed-by tag
- add one patch to update jal offsets in patched alternatives
- add one patch to switch to relative alternative entries
- add patches to patch vdso

[1]https://lore.kernel.org/linux-riscv/20220922060958.44203-1-samuel at sholland.org/ [2]https://lore.kernel.org/linux-arm-kernel/20220912162210.3626215-8-mark.rutland at arm.com/ [3]https://lore.kernel.org/linux-riscv/20221130225614.1594256-1-heiko at sntech.de/

Andrew Jones (1):
riscv: KVM: Switch has_svinval() to riscv_has_extension_unlikely()

Jisheng Zhang (12):
riscv: fix jal offsets in patched alternatives
riscv: move riscv_noncoherent_supported() out of ZICBOM probe
riscv: cpufeature: detect RISCV_ALTERNATIVES_EARLY_BOOT earlier
riscv: hwcap: make ISA extension ids can be used in asm
riscv: cpufeature: extend riscv_cpufeature_patch_func to all ISA
extensions
riscv: introduce riscv_has_extension_[un]likely()
riscv: fpu: switch has_fpu() to riscv_has_extension_likely()
riscv: module: move find_section to module.h
riscv: switch to relative alternative entries
riscv: alternative: patch alternatives in the vDSO
riscv: cpu_relax: switch to riscv_has_extension_likely()
riscv: remove riscv_isa_ext_keys[] array and related usage

 arch/riscv/errata/sifive/errata.c           |  3 +-
 arch/riscv/errata/thead/errata.c            | 11 ++-
 arch/riscv/include/asm/alternative-macros.h | 20 ++---
 arch/riscv/include/asm/alternative.h        | 17 ++--
 arch/riscv/include/asm/errata_list.h        |  9 +-
 arch/riscv/include/asm/hwcap.h              | 97 +++++++++++----------
 arch/riscv/include/asm/insn.h               | 27 ++++++
 arch/riscv/include/asm/module.h             | 16 ++++
 arch/riscv/include/asm/switch_to.h          |  3 +-
 arch/riscv/include/asm/vdso.h               |  4 +
 arch/riscv/include/asm/vdso/processor.h     |  2 +-
 arch/riscv/kernel/alternative.c             | 56 ++++++++++++
 arch/riscv/kernel/cpufeature.c              | 78 +++--------------
 arch/riscv/kernel/module.c                  | 15 ----
 arch/riscv/kernel/setup.c                   |  3 +
 arch/riscv/kernel/vdso.c                    |  5 --
 arch/riscv/kernel/vdso/vdso.lds.S           |  7 ++
 arch/riscv/kvm/tlb.c                        |  3 +-
 18 files changed, 214 insertions(+), 162 deletions(-)
Signature · 1 line
2.38.1
#1 lore raw

PATCH v4 1/13 riscv: fix jal offsets in patched alternatives

Alternatives live in a different section, so offsets used by jal instruction will point to wrong locations after the patch got applied.

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

---
 arch/riscv/include/asm/insn.h   | 27 +++++++++++++++++++++++++++
 arch/riscv/kernel/alternative.c | 27 +++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)
arch/riscv/include/asm/insn.h +27 -0
index 98453535324a..25ef9c0b19e7 100644
@@ -291,6 +291,33 @@ static __always_inline bool riscv_insn_is_branch(u32 code)
 	(RVC_X(x_, RVC_B_IMM_7_6_OPOFF, RVC_B_IMM_7_6_MASK) << RVC_B_IMM_7_6_OFF) | \
 	(RVC_IMM_SIGN(x_) << RVC_B_IMM_SIGN_OFF); })
 
+/*
+ * Get the immediate from a J-type instruction.
+ *
+ * @insn: instruction to process
+ * Return: immediate
+ */
+static inline s32 riscv_insn_extract_jtype_imm(u32 insn)
+{
+	return RV_EXTRACT_JTYPE_IMM(insn);
+}
+
+/*
+ * Update a J-type instruction with an immediate value.
+ *
+ * @insn: pointer to the jtype instruction
+ * @imm: the immediate to insert into the instruction
+ */
+static inline void riscv_insn_insert_jtype_imm(u32 *insn, s32 imm)
+{
+	/* drop the old IMMs, all jal IMM bits sit@31:12 */
+	*insn &= ~GENMASK(31, 12);
+	*insn |= (RV_X(imm, RV_J_IMM_10_1_OFF, RV_J_IMM_10_1_MASK) << RV_J_IMM_10_1_OPOFF) |
+		 (RV_X(imm, RV_J_IMM_11_OFF, RV_J_IMM_11_MASK) << RV_J_IMM_11_OPOFF) |
+		 (RV_X(imm, RV_J_IMM_19_12_OFF, RV_J_IMM_19_12_MASK) << RV_J_IMM_19_12_OPOFF) |
+		 (RV_X(imm, RV_J_IMM_SIGN_OFF, 1) << RV_J_IMM_SIGN_OPOFF);
+}
+
 /*
  * Put together one immediate from a U-type and I-type instruction pair.
  *
arch/riscv/kernel/alternative.c +27 -0
index 6212ea0eed72..3d4f1f32c7f6 100644
@@ -79,6 +79,21 @@ static void riscv_alternative_fix_auipc_jalr(void *ptr, u32 auipc_insn,
 	patch_text_nosync(ptr, call, sizeof(u32) * 2);
 }
 
+static void riscv_alternative_fix_jal(void *ptr, u32 jal_insn, int patch_offset)
+{
+	s32 imm;
+
+	/* get and adjust new target address */
+	imm = riscv_insn_extract_jtype_imm(jal_insn);
+	imm -= patch_offset;
+
+	/* update instruction */
+	riscv_insn_insert_jtype_imm(&jal_insn, imm);
+
+	/* patch the call place again */
+	patch_text_nosync(ptr, &jal_insn, sizeof(u32));
+}
+
 void riscv_alternative_fix_offsets(void *alt_ptr, unsigned int len,
 				      int patch_offset)
 {
@@ -106,6 +121,18 @@ void riscv_alternative_fix_offsets(void *alt_ptr, unsigned int len,
 			riscv_alternative_fix_auipc_jalr(alt_ptr + i * sizeof(u32),
 							 insn, insn2, patch_offset);
 		}
+
+		if (riscv_insn_is_jal(insn)) {
+			s32 imm = riscv_insn_extract_jtype_imm(insn);
+
+			/* Don't modify jumps inside the alternative block */
+			if ((alt_ptr + i * sizeof(u32) + imm) >= alt_ptr &&
+			    (alt_ptr + i * sizeof(u32) + imm) < (alt_ptr + len))
+				continue;
+
+			riscv_alternative_fix_jal(alt_ptr + i * sizeof(u32),
+						  insn, patch_offset);
+		}
 	}
 }
 
Signature · 1 line
2.38.1
#2 lore raw

PATCH v4 2/13 riscv: move riscv_noncoherent_supported() out of ZICBOM probe

In reply to Jisheng Zhang

It's a bit weird to call riscv_noncoherent_supported() each time when insmoding a module. Move the calling out of feature patch func.

---
 arch/riscv/kernel/cpufeature.c | 1 -
 arch/riscv/kernel/setup.c      | 3 +++
 2 files changed, 3 insertions(+), 1 deletion(-)
arch/riscv/kernel/cpufeature.c +0 -1
index 205bbd6b1fce..421b3d9578cc 100644
@@ -297,7 +297,6 @@ static bool __init_or_module cpufeature_probe_zicbom(unsigned int stage)
 	if (!riscv_isa_extension_available(NULL, ZICBOM))
 		return false;
 
-	riscv_noncoherent_supported();
 	return true;
 }
 
arch/riscv/kernel/setup.c +3 -0
index 86acd690d529..376d2827e736 100644
@@ -300,6 +300,9 @@ void __init setup_arch(char **cmdline_p)
 	riscv_init_cbom_blocksize();
 	riscv_fill_hwcap();
 	apply_boot_alternatives();
+	if (IS_ENABLED(CONFIG_RISCV_ISA_ZICBOM) &&
+	    riscv_isa_extension_available(NULL, ZICBOM))
+		riscv_noncoherent_supported();
 }
 
 static int __init topology_init(void)
Signature · 1 line
2.38.1
#3 lore raw

PATCH v4 3/13 riscv: cpufeature: detect RISCV_ALTERNATIVES_EARLY_BOOT earlier

In reply to Jisheng Zhang

Currently riscv_cpufeature_patch_func() does nothing at the RISCV_ALTERNATIVES_EARLY_BOOT stage. Add a check to detect whether we are in this stage and exit early. This will allow us to use riscv_cpufeature_patch_func() for scanning of all ISA extensions.

---
 arch/riscv/kernel/cpufeature.c | 3 +++
 1 file changed, 3 insertions(+)
arch/riscv/kernel/cpufeature.c +3 -0
index 421b3d9578cc..37e8c5e69754 100644
@@ -328,6 +328,9 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
 	struct alt_entry *alt;
 	u32 tmp;
 
+	if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
+		return;
+
 	for (alt = begin; alt < end; alt++) {
 		if (alt->vendor_id != 0)
 			continue;
Signature · 1 line
2.38.1
#4 lore raw

PATCH v4 4/13 riscv: hwcap: make ISA extension ids can be used in asm

In reply to Jisheng Zhang

So that ISA extensions can be used in assembly files, convert the multi-letter RISC-V ISA extension IDs enums to macros. In order to make them visible, move the #ifndef __ASSEMBLY__ guard to a later point in the header

---
 arch/riscv/include/asm/hwcap.h | 45 ++++++++++++++++------------------
 1 file changed, 21 insertions(+), 24 deletions(-)
arch/riscv/include/asm/hwcap.h +21 -24
index 86328e3acb02..09a7767723f6 100644
@@ -12,20 +12,6 @@
 #include <linux/bits.h>
 #include <uapi/asm/hwcap.h>
 
-#ifndef __ASSEMBLY__
-#include <linux/jump_label.h>
-/*
- * This yields a mask that user programs can use to figure out what
- * instruction set this cpu supports.
- */
-#define ELF_HWCAP		(elf_hwcap)
-
-enum {
-	CAP_HWCAP = 1,
-};
-
-extern unsigned long elf_hwcap;
-
 #define RISCV_ISA_EXT_a		('a' - 'a')
 #define RISCV_ISA_EXT_c		('c' - 'a')
 #define RISCV_ISA_EXT_d		('d' - 'a')
@@ -46,22 +32,33 @@ extern unsigned long elf_hwcap;
 #define RISCV_ISA_EXT_BASE 26
 
 /*
- * This enum represent the logical ID for each multi-letter RISC-V ISA extension.
+ * These macros represent the logical ID for each multi-letter RISC-V ISA extension.
  * The logical ID should start from RISCV_ISA_EXT_BASE and must not exceed
  * RISCV_ISA_EXT_MAX. 0-25 range is reserved for single letter
  * extensions while all the multi-letter extensions should define the next
  * available logical extension id.
  */
-enum riscv_isa_ext_id {
-	RISCV_ISA_EXT_SSCOFPMF = RISCV_ISA_EXT_BASE,
-	RISCV_ISA_EXT_SVPBMT,
-	RISCV_ISA_EXT_ZICBOM,
-	RISCV_ISA_EXT_ZIHINTPAUSE,
-	RISCV_ISA_EXT_SSTC,
-	RISCV_ISA_EXT_SVINVAL,
-	RISCV_ISA_EXT_ID_MAX
+#define RISCV_ISA_EXT_SSCOFPMF		26
+#define RISCV_ISA_EXT_SVPBMT		27
+#define RISCV_ISA_EXT_ZICBOM		28
+#define RISCV_ISA_EXT_ZIHINTPAUSE	29
+#define RISCV_ISA_EXT_SSTC		30
+#define RISCV_ISA_EXT_SVINVAL		31
+
+#ifndef __ASSEMBLY__
+#include <linux/jump_label.h>
+/*
+ * This yields a mask that user programs can use to figure out what
+ * instruction set this cpu supports.
+ */
+#define ELF_HWCAP		(elf_hwcap)
+
+enum {
+	CAP_HWCAP = 1,
 };
-static_assert(RISCV_ISA_EXT_ID_MAX <= RISCV_ISA_EXT_MAX);
+
+extern unsigned long elf_hwcap;
+
 
 /*
  * This enum represents the logical ID for each RISC-V ISA extension static
Signature · 1 line
2.38.1
#5 lore raw

PATCH v4 5/13 riscv: cpufeature: extend riscv_cpufeature_patch_func to all ISA extensions

In reply to Jisheng Zhang

riscv_cpufeature_patch_func() currently only scans a limited set of cpufeatures, explicitly defined with macros. Extend it to probe for all ISA extensions.

---
 arch/riscv/include/asm/errata_list.h |  9 ++--
 arch/riscv/kernel/cpufeature.c       | 63 ++++------------------------
 2 files changed, 11 insertions(+), 61 deletions(-)
arch/riscv/include/asm/errata_list.h +3 -6
index 4180312d2a70..274c6f889602 100644
@@ -7,6 +7,7 @@
 
 #include <asm/alternative.h>
 #include <asm/csr.h>
+#include <asm/hwcap.h>
 #include <asm/vendorid_list.h>
 
 #ifdef CONFIG_ERRATA_SIFIVE
@@ -22,10 +23,6 @@
 #define	ERRATA_THEAD_NUMBER 3
 #endif
 
-#define	CPUFEATURE_SVPBMT 0
-#define	CPUFEATURE_ZICBOM 1
-#define	CPUFEATURE_NUMBER 2
-
 #ifdef __ASSEMBLY__
 
 #define ALT_INSN_FAULT(x)						\
@@ -55,7 +52,7 @@ asm(ALTERNATIVE("sfence.vma %0", "sfence.vma", SIFIVE_VENDOR_ID,	\
 #define ALT_SVPBMT(_val, prot)						\
 asm(ALTERNATIVE_2("li %0, 0\t\nnop",					\
 		  "li %0, %1\t\nslli %0,%0,%3", 0,			\
-			CPUFEATURE_SVPBMT, CONFIG_RISCV_ISA_SVPBMT,	\
+			RISCV_ISA_EXT_SVPBMT, CONFIG_RISCV_ISA_SVPBMT,	\
 		  "li %0, %2\t\nslli %0,%0,%4", THEAD_VENDOR_ID,	\
 			ERRATA_THEAD_PBMT, CONFIG_ERRATA_THEAD_PBMT)	\
 		: "=r"(_val)						\
@@ -129,7 +126,7 @@ asm volatile(ALTERNATIVE_2(						\
 	"add a0, a0, %0\n\t"						\
 	"2:\n\t"							\
 	"bltu a0, %2, 3b\n\t"						\
-	"nop", 0, CPUFEATURE_ZICBOM, CONFIG_RISCV_ISA_ZICBOM,		\
+	"nop", 0, RISCV_ISA_EXT_ZICBOM, CONFIG_RISCV_ISA_ZICBOM,	\
 	"mv a0, %1\n\t"							\
 	"j 2f\n\t"							\
 	"3:\n\t"							\
arch/riscv/kernel/cpufeature.c +8 -55

84 lines not on this page · raw message

Signature · 1 line
2.38.1
Reviewed-by on v4 5/13
#6 lore raw

Re: [PATCH v4 05/13] riscv: cpufeature: extend riscv_cpufeature_patch_func to all ISA extensions

Quoted 6 lines · Jisheng Zhang

riscv_cpufeature_patch_func() currently only scans a limited set of cpufeatures, explicitly defined with macros. Extend it to probe for all ISA extensions.

With the same rationale as 6/13:

Thanks,
Conor.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/kvm-riscv/attachments/20230118/375056eb/attachment.sig>

#7 lore raw

PATCH v4 6/13 riscv: introduce riscv_has_extension_[un]likely()

In reply to Jisheng Zhang

Generally, riscv ISA extensions are fixed for any specific hardware platform, so a hart's features won't change after booting. This chacteristic makes it straightforward to use a static branch to check if a specific ISA extension is supported or not to optimize performance.

However, some ISA extensions such as SVPBMT and ZICBOM are handled via. the alternative sequences.

Basically, for ease of maintenance, we prefer to use static branches in C code, but recently, Samuel found that the static branch usage in cpu_relax() breaks building with CONFIG_CC_OPTIMIZE_FOR_SIZE[1]. As Samuel pointed out, "Having a static branch in cpu_relax() is problematic because that function is widely inlined, including in some quite complex functions like in the VDSO. A quick measurement shows this static branch is responsible by itself for around 40% of the jump table."

Samuel's findings pointed out one of a few downsides of static branches usage in C code to handle ISA extensions detected at boot time:
static branch's metadata in the __jump_table section, which is not discarded after ISA extensions are finalized, wastes some space.

I want to try to solve the issue for all possible dynamic handling of ISA extensions at boot time. Inspired by Mark[2], this patch introduces riscv_has_extension_*() helpers, which work like static branches but are patched using alternatives, thus the metadata can be freed after patching.

---
 arch/riscv/include/asm/hwcap.h | 37 ++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
arch/riscv/include/asm/hwcap.h +37 -0
index 09a7767723f6..1767a9ce1a04 100644
@@ -8,6 +8,7 @@
 #ifndef _ASM_RISCV_HWCAP_H
 #define _ASM_RISCV_HWCAP_H
 
+#include <asm/alternative-macros.h>
 #include <asm/errno.h>
 #include <linux/bits.h>
 #include <uapi/asm/hwcap.h>
@@ -97,6 +98,42 @@ static __always_inline int riscv_isa_ext2key(int num)
 	}
 }
 
+static __always_inline bool
+riscv_has_extension_likely(const unsigned long ext)
+{
+	compiletime_assert(ext < RISCV_ISA_EXT_MAX,
+			   "ext must be < RISCV_ISA_EXT_MAX");
+
+	asm_volatile_goto(
+	ALTERNATIVE("j	%l[l_no]", "nop", 0, %[ext], 1)
+	:
+	: [ext] "i" (ext)
+	:
+	: l_no);
+
+	return true;
+l_no:
+	return false;
+}
+
+static __always_inline bool
+riscv_has_extension_unlikely(const unsigned long ext)
+{
+	compiletime_assert(ext < RISCV_ISA_EXT_MAX,
+			   "ext must be < RISCV_ISA_EXT_MAX");
+
+	asm_volatile_goto(
+	ALTERNATIVE("nop", "j	%l[l_yes]", 0, %[ext], 1)
+	:
+	: [ext] "i" (ext)
+	:
+	: l_yes);
+
+	return false;
+l_yes:
+	return true;
+}
+
 unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);
 
 #define riscv_isa_extension_mask(ext) BIT_MASK(RISCV_ISA_EXT_##ext)
Signature · 1 line
2.38.1
#8 lore raw

Re: [PATCH v4 06/13] riscv: introduce riscv_has_extension_[un]likely()

Hey Jisheng,

Quoted 29 lines · Jisheng Zhang

Generally, riscv ISA extensions are fixed for any specific hardware platform, so a hart's features won't change after booting. This chacteristic makes it straightforward to use a static branch to check if a specific ISA extension is supported or not to optimize performance.

However, some ISA extensions such as SVPBMT and ZICBOM are handled via. the alternative sequences.

Basically, for ease of maintenance, we prefer to use static branches in C code, but recently, Samuel found that the static branch usage in cpu_relax() breaks building with CONFIG_CC_OPTIMIZE_FOR_SIZE[1]. As Samuel pointed out, "Having a static branch in cpu_relax() is problematic because that function is widely inlined, including in some quite complex functions like in the VDSO. A quick measurement shows this static branch is responsible by itself for around 40% of the jump table."

Samuel's findings pointed out one of a few downsides of static branches usage in C code to handle ISA extensions detected at boot time:
static branch's metadata in the __jump_table section, which is not discarded after ISA extensions are finalized, wastes some space.

I want to try to solve the issue for all possible dynamic handling of ISA extensions at boot time. Inspired by Mark[2], this patch introduces riscv_has_extension_*() helpers, which work like static branches but are patched using alternatives, thus the metadata can be freed after patching.

Quoted 4 lines

It'd be great, if, in the future, you would hold off on sending new versions of patchsets where the previous version is still being discussed [3].
~3 days between versions is not very much, especially when that includes a weekend!
I know you replied there earlier today with your opinion, but please give people a chance to read and respond, before resubmitting, so as not to split discussion between several threads.

3 - https://lore.kernel.org/linux-riscv/2398293.3Lj2Plt8kZ at diego/

Thanks,
Conor.

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/kvm-riscv/attachments/20230115/7c8c8389/attachment-0001.sig>

Acked-by on v4 6/13
#9 lore raw

Re: [PATCH v4 06/13] riscv: introduce riscv_has_extension_[un]likely()

In reply to Jisheng Zhang
Quoted 29 lines · Jisheng Zhang

Generally, riscv ISA extensions are fixed for any specific hardware platform, so a hart's features won't change after booting. This chacteristic makes it straightforward to use a static branch to check if a specific ISA extension is supported or not to optimize performance.

However, some ISA extensions such as SVPBMT and ZICBOM are handled via. the alternative sequences.

Basically, for ease of maintenance, we prefer to use static branches in C code, but recently, Samuel found that the static branch usage in cpu_relax() breaks building with CONFIG_CC_OPTIMIZE_FOR_SIZE[1]. As Samuel pointed out, "Having a static branch in cpu_relax() is problematic because that function is widely inlined, including in some quite complex functions like in the VDSO. A quick measurement shows this static branch is responsible by itself for around 40% of the jump table."

Samuel's findings pointed out one of a few downsides of static branches usage in C code to handle ISA extensions detected at boot time:
static branch's metadata in the __jump_table section, which is not discarded after ISA extensions are finalized, wastes some space.

I want to try to solve the issue for all possible dynamic handling of ISA extensions at boot time. Inspired by Mark[2], this patch introduces riscv_has_extension_*() helpers, which work like static branches but are patched using alternatives, thus the metadata can be freed after patching.

Quoted 4 lines

I think I just didn't leave an R-b last time because of the ongoing discussion & the functions naming.
Given the conversation I attempted to summarise at [1], I'm satisfied that we're not just introducing something that'd conflict with trying to use this mechanism for non-extension capabilities:

1 - https://lore.kernel.org/linux-riscv/Y8hqptFcUgjhns4F at spud/

Quoted 67 lines
---
 arch/riscv/include/asm/hwcap.h | 37 ++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)
arch/riscv/include/asm/hwcap.h +37 -0
index 09a7767723f6..1767a9ce1a04 100644
@@ -8,6 +8,7 @@
 #ifndef _ASM_RISCV_HWCAP_H
 #define _ASM_RISCV_HWCAP_H
 
+#include <asm/alternative-macros.h>
 #include <asm/errno.h>
 #include <linux/bits.h>
 #include <uapi/asm/hwcap.h>
@@ -97,6 +98,42 @@ static __always_inline int riscv_isa_ext2key(int num)
 	}
 }
 
+static __always_inline bool
+riscv_has_extension_likely(const unsigned long ext)
+{
+	compiletime_assert(ext < RISCV_ISA_EXT_MAX,
+			   "ext must be < RISCV_ISA_EXT_MAX");
+
+	asm_volatile_goto(
+	ALTERNATIVE("j	%l[l_no]", "nop", 0, %[ext], 1)
+	:
+	: [ext] "i" (ext)
+	:
+	: l_no);
+
+	return true;
+l_no:
+	return false;
+}
+
+static __always_inline bool
+riscv_has_extension_unlikely(const unsigned long ext)
+{
+	compiletime_assert(ext < RISCV_ISA_EXT_MAX,
+			   "ext must be < RISCV_ISA_EXT_MAX");
+
+	asm_volatile_goto(
+	ALTERNATIVE("nop", "j	%l[l_yes]", 0, %[ext], 1)
+	:
+	: [ext] "i" (ext)
+	:
+	: l_yes);
+
+	return false;
+l_yes:
+	return true;
+}
+
 unsigned long riscv_isa_extension_base(const unsigned long *isa_bitmap);
 
 #define riscv_isa_extension_mask(ext) BIT_MASK(RISCV_ISA_EXT_##ext)
Signature · 7 lines
2.38.1


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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/kvm-riscv/attachments/20230118/6e4b12d5/attachment-0001.sig>

#10 lore raw

PATCH v4 7/13 riscv: fpu: switch has_fpu() to riscv_has_extension_likely()

In reply to Jisheng Zhang

Switch has_fpu() from static branch to the new helper riscv_has_extension_likely().

---
 arch/riscv/include/asm/switch_to.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arch/riscv/include/asm/switch_to.h +2 -1
index 11463489fec6..60f8ca01d36e 100644
@@ -59,7 +59,8 @@ static inline void __switch_to_aux(struct task_struct *prev,
 
 static __always_inline bool has_fpu(void)
 {
-	return static_branch_likely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_FPU]);
+	return riscv_has_extension_likely(RISCV_ISA_EXT_f) ||
+		riscv_has_extension_likely(RISCV_ISA_EXT_d);
 }
 #else
 static __always_inline bool has_fpu(void) { return false; }
Signature · 1 line
2.38.1
#11 lore raw

PATCH v4 8/13 riscv: module: move find_section to module.h

In reply to Jisheng Zhang

Move find_section() to module.h so that the implementation can be shared by the alternatives code. This will allow us to use alternatives in the vdso.

---
 arch/riscv/include/asm/module.h | 16 ++++++++++++++++
 arch/riscv/kernel/module.c      | 15 ---------------
 2 files changed, 16 insertions(+), 15 deletions(-)
arch/riscv/include/asm/module.h +16 -0
index 76aa96a9fc08..0f3baaa6a9a8 100644
@@ -5,6 +5,7 @@
 #define _ASM_RISCV_MODULE_H
 
 #include <asm-generic/module.h>
+#include <linux/elf.h>
 
 struct module;
 unsigned long module_emit_got_entry(struct module *mod, unsigned long val);
@@ -111,4 +112,19 @@ static inline struct plt_entry *get_plt_entry(unsigned long val,
 
 #endif /* CONFIG_MODULE_SECTIONS */
 
+static inline const Elf_Shdr *find_section(const Elf_Ehdr *hdr,
+					   const Elf_Shdr *sechdrs,
+					   const char *name)
+{
+	const Elf_Shdr *s, *se;
+	const char *secstrs = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
+
+	for (s = sechdrs, se = sechdrs + hdr->e_shnum; s < se; s++) {
+		if (strcmp(name, secstrs + s->sh_name) == 0)
+			return s;
+	}
+
+	return NULL;
+}
+
 #endif /* _ASM_RISCV_MODULE_H */
arch/riscv/kernel/module.c +0 -15
index 91fe16bfaa07..76f4b9c2ec5b 100644
@@ -429,21 +429,6 @@ void *module_alloc(unsigned long size)
 }
 #endif
 
-static const Elf_Shdr *find_section(const Elf_Ehdr *hdr,
-				    const Elf_Shdr *sechdrs,
-				    const char *name)
-{
-	const Elf_Shdr *s, *se;
-	const char *secstrs = (void *)hdr + sechdrs[hdr->e_shstrndx].sh_offset;
-
-	for (s = sechdrs, se = sechdrs + hdr->e_shnum; s < se; s++) {
-		if (strcmp(name, secstrs + s->sh_name) == 0)
-			return s;
-	}
-
-	return NULL;
-}
-
 int module_finalize(const Elf_Ehdr *hdr,
 		    const Elf_Shdr *sechdrs,
 		    struct module *me)
Signature · 1 line
2.38.1
#12 lore raw

PATCH v4 9/13 riscv: switch to relative alternative entries

In reply to Jisheng Zhang

Instead of using absolute addresses for both the old instrucions and the alternative instructions, use offsets relative to the alt_entry values. So this not only cuts the size of the alternative entry, but also meets the prerequisite for patching alternatives in the vDSO, since absolute alternative entries are subject to dynamic relocation, which is incompatible with the vDSO building.

---
 arch/riscv/errata/sifive/errata.c           |  3 ++-
 arch/riscv/errata/thead/errata.c            | 11 ++++++++---
 arch/riscv/include/asm/alternative-macros.h | 20 ++++++++++----------
 arch/riscv/include/asm/alternative.h        | 17 +++++++++++------
 arch/riscv/kernel/cpufeature.c              |  8 +++++---
 5 files changed, 36 insertions(+), 23 deletions(-)
arch/riscv/errata/sifive/errata.c +2 -1
index 1031038423e7..ef9a4eec0dba 100644
@@ -107,7 +107,8 @@ void __init_or_module sifive_errata_patch_func(struct alt_entry *begin,
 
 		tmp = (1U << alt->errata_id);
 		if (cpu_req_errata & tmp) {
-			patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
+			patch_text_nosync(ALT_OLD_PTR(alt), ALT_ALT_PTR(alt),
+					  alt->alt_len);
 			cpu_apply_errata |= tmp;
 		}
 	}
arch/riscv/errata/thead/errata.c +8 -3
index fac5742d1c1e..c0bea5c94128 100644
@@ -87,6 +87,7 @@ void __init_or_module thead_errata_patch_func(struct alt_entry *begin, struct al
 	struct alt_entry *alt;
 	u32 cpu_req_errata = thead_errata_probe(stage, archid, impid);
 	u32 tmp;
+	void *oldptr, *altptr;
 
 	for (alt = begin; alt < end; alt++) {
 		if (alt->vendor_id != THEAD_VENDOR_ID)
@@ -96,12 +97,16 @@ void __init_or_module thead_errata_patch_func(struct alt_entry *begin, struct al
 
 		tmp = (1U << alt->errata_id);
 		if (cpu_req_errata & tmp) {
+			oldptr = ALT_OLD_PTR(alt);
+			altptr = ALT_ALT_PTR(alt);
+
 			/* On vm-alternatives, the mmu isn't running yet */
 			if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
-				memcpy((void *)__pa_symbol(alt->old_ptr),
-				       (void *)__pa_symbol(alt->alt_ptr), alt->alt_len);
+				memcpy((void *)__pa_symbol(oldptr),
+				       (void *)__pa_symbol(altptr),
+				       alt->alt_len);
 			else
-				patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
+				patch_text_nosync(oldptr, altptr, alt->alt_len);
 		}
 	}
 
arch/riscv/include/asm/alternative-macros.h +10 -10
index 7226e2462584..cc6a81c00f2f 100644
@@ -7,11 +7,11 @@
 #ifdef __ASSEMBLY__
 
 .macro ALT_ENTRY oldptr newptr vendor_id errata_id new_len
-	RISCV_PTR \oldptr
-	RISCV_PTR \newptr
-	REG_ASM \vendor_id
-	REG_ASM \new_len
-	.word	\errata_id
+	.4byte \oldptr - .
+	.4byte \newptr - .
+	.2byte \vendor_id
+	.2byte \new_len
+	.4byte \errata_id
 .endm
 
 .macro ALT_NEW_CONTENT vendor_id, errata_id, enable = 1, new_c : vararg
@@ -59,11 +59,11 @@
 #include <linux/stringify.h>
 
 #define ALT_ENTRY(oldptr, newptr, vendor_id, errata_id, newlen)		\
-	RISCV_PTR " " oldptr "\n"					\
-	RISCV_PTR " " newptr "\n"					\
-	REG_ASM " " vendor_id "\n"					\
-	REG_ASM " " newlen "\n"						\
-	".word " errata_id "\n"
+	".4byte	((" oldptr ") - .) \n"					\
+	".4byte	((" newptr ") - .) \n"					\
+	".2byte	" vendor_id "\n"					\
+	".2byte " newlen "\n"						\
+	".4byte	" errata_id "\n"
 
 #define ALT_NEW_CONTENT(vendor_id, errata_id, enable, new_c)		\
 	".if " __stringify(enable) " == 1\n"				\
arch/riscv/include/asm/alternative.h +11 -6
index 1bd4027d34ca..b8648d4f2ac1 100644
@@ -23,6 +23,11 @@
 #define RISCV_ALTERNATIVES_MODULE	1 /* alternatives applied during module-init */
 #define RISCV_ALTERNATIVES_EARLY_BOOT	2 /* alternatives applied before mmu start */
 
+/* add the relative offset to the address of the offset to get the absolute address */
+#define __ALT_PTR(a, f)			((void *)&(a)->f + (a)->f)
+#define ALT_OLD_PTR(a)			__ALT_PTR(a, old_offset)
+#define ALT_ALT_PTR(a)			__ALT_PTR(a, alt_offset)
+
 void __init apply_boot_alternatives(void);
 void __init apply_early_boot_alternatives(void);
 void apply_module_alternatives(void *start, size_t length);
@@ -31,12 +36,12 @@ void riscv_alternative_fix_offsets(void *alt_ptr, unsigned int len,
 				   int patch_offset);
 
 struct alt_entry {
-	void *old_ptr;		 /* address of original instruciton or data  */
-	void *alt_ptr;		 /* address of replacement instruction or data */
-	unsigned long vendor_id; /* cpu vendor id */
-	unsigned long alt_len;   /* The replacement size */
-	unsigned int errata_id;  /* The errata id */
-} __packed;
+	s32 old_offset;		/* offset relative to original instruction or data  */
+	s32 alt_offset;		/* offset relative to replacement instruction or data */
+	u16 vendor_id;		/* cpu vendor id */
+	u16 alt_len;		/* The replacement size */
+	u32 errata_id;		/* The errata id */
+};
 
 struct errata_checkfunc_id {
 	unsigned long vendor_id;
arch/riscv/kernel/cpufeature.c +5 -3
index 6db8b31d9149..50cfbbbecaa7 100644
@@ -280,6 +280,7 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
 						  unsigned int stage)
 {
 	struct alt_entry *alt;
+	void *oldptr, *altptr;
 
 	if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
 		return;
@@ -293,12 +294,13 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
 			continue;
 		}
 
+		oldptr = ALT_OLD_PTR(alt);
+		altptr = ALT_ALT_PTR(alt);
 		if (!__riscv_isa_extension_available(NULL, alt->errata_id))
 			continue;
 
-		patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
-		riscv_alternative_fix_offsets(alt->old_ptr, alt->alt_len,
-					      alt->old_ptr - alt->alt_ptr);
+		patch_text_nosync(oldptr, altptr, alt->alt_len);
+		riscv_alternative_fix_offsets(oldptr, alt->alt_len, oldptr - altptr);
 	}
 }
 #endif
Signature · 1 line
2.38.1
Reviewed-by on v4 9/13
#13 lore raw

Re: [PATCH v4 09/13] riscv: switch to relative alternative entries

Quoted 10 lines · Jisheng Zhang

Instead of using absolute addresses for both the old instrucions and the alternative instructions, use offsets relative to the alt_entry values. So this not only cuts the size of the alternative entry, but also meets the prerequisite for patching alternatives in the vDSO, since absolute alternative entries are subject to dynamic relocation, which is incompatible with the vDSO building.

arch/riscv/include/asm/alternative.h line 29

Quoted 4 lines
arch/riscv/include/asm/alternative.h +4 -0
+/* add the relative offset to the address of the offset to get the absolute address */
+#define __ALT_PTR(a, f)			((void *)&(a)->f + (a)->f)
+#define ALT_OLD_PTR(a)			__ALT_PTR(a, old_offset)
+#define ALT_ALT_PTR(a)			__ALT_PTR(a, alt_offset)

LGTM, thanks for doing that! Certainly makes things more understandable.

arch/riscv/kernel/cpufeature.c void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin, · line 300

Quoted 5 lines
arch/riscv/kernel/cpufeature.c +2 -0
+		oldptr = ALT_OLD_PTR(alt);
+		altptr = ALT_ALT_PTR(alt);
 		if (!__riscv_isa_extension_available(NULL, alt->errata_id))
 			continue;

One minor nit, the oldptr/altptr assignments could go down here, below the if/continue, right? Obviously don't respin for that though!

arch/riscv/kernel/cpufeature.c void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin, · line 306

Quoted 8 lines
arch/riscv/kernel/cpufeature.c +2 -3
-		patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
-		riscv_alternative_fix_offsets(alt->old_ptr, alt->alt_len,
-					      alt->old_ptr - alt->alt_ptr);
+		patch_text_nosync(oldptr, altptr, alt->alt_len);
+		riscv_alternative_fix_offsets(oldptr, alt->alt_len, oldptr - altptr);
 	}
 }
 #endif

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/kvm-riscv/attachments/20230118/a18e39cb/attachment-0001.sig>

#14 lore raw

Re: [PATCH v4 09/13] riscv: switch to relative alternative entries

In reply to Jisheng Zhang

On Sun, Jan 15, 2023 at 11:49:49PM +0800, Jisheng Zhang wrote:
...

Review

Quoted 12 lines

#define ALT_ENTRY(oldptr, newptr, vendor_id, errata_id, newlen) \

(quoted diff) +5 -5
-	RISCV_PTR " " oldptr "\n"					\
-	RISCV_PTR " " newptr "\n"					\
-	REG_ASM " " vendor_id "\n"					\
-	REG_ASM " " newlen "\n"						\
-	".word " errata_id "\n"
+	".4byte	((" oldptr ") - .) \n"					\
+	".4byte	((" newptr ") - .) \n"					\
+	".2byte	" vendor_id "\n"					\
+	".2byte " newlen "\n"						\
+	".4byte	" errata_id "\n"

Hi Jisheng,

This patch breaks loading the KVM module for me. I got "kvm: Unknown relocation type 34". My guess is that these 2 byte fields are inspiring the compiler to emit 16-bit relocation types. The patch below fixes things for me. If you agree with fixing it this way, rather than changing something in alternatives, like not using 2 byte fields, then please pick the below patch up in your series.

Thanks,
drew

From 4d203697aa745a0cd3a9217d547a9fb7fa2a87c7 Mon Sep 17 00:00:00 2001
From: Andrew Jones <[email protected]>
Date: Fri, 20 Jan 2023 19:05:44 +0100
Subject: [PATCH] riscv: module: Add ADD16 and SUB16 rela types
Content-type: text/plain

To prepare for 16-bit relocation types to be emitted in alternatives add support for ADD16 and SUB16.

---
 arch/riscv/kernel/module.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
arch/riscv/kernel/module.c +16 -0
index 76f4b9c2ec5b..7c651d55fcbd 100644
@@ -268,6 +268,13 @@ static int apply_r_riscv_align_rela(struct module *me, u32 *location,
 	return -EINVAL;
 }
 
+static int apply_r_riscv_add16_rela(struct module *me, u32 *location,
+				    Elf_Addr v)
+{
+	*(u16 *)location += (u16)v;
+	return 0;
+}
+
 static int apply_r_riscv_add32_rela(struct module *me, u32 *location,
 				    Elf_Addr v)
 {
@@ -282,6 +289,13 @@ static int apply_r_riscv_add64_rela(struct module *me, u32 *location,
 	return 0;
 }
 
+static int apply_r_riscv_sub16_rela(struct module *me, u32 *location,
+				    Elf_Addr v)
+{
+	*(u16 *)location -= (u16)v;
+	return 0;
+}
+
 static int apply_r_riscv_sub32_rela(struct module *me, u32 *location,
 				    Elf_Addr v)
 {
@@ -315,8 +329,10 @@ 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_ALIGN]			= apply_r_riscv_align_rela,
+	[R_RISCV_ADD16]			= apply_r_riscv_add16_rela,
 	[R_RISCV_ADD32]			= apply_r_riscv_add32_rela,
 	[R_RISCV_ADD64]			= apply_r_riscv_add64_rela,
+	[R_RISCV_SUB16]			= apply_r_riscv_sub16_rela,
 	[R_RISCV_SUB32]			= apply_r_riscv_sub32_rela,
 	[R_RISCV_SUB64]			= apply_r_riscv_sub64_rela,
 };
Signature · 1 line
2.39.0
#15 lore raw

Re: [PATCH v4 09/13] riscv: switch to relative alternative entries

Quoted 17 lines · Andrew Jones

On Sun, Jan 15, 2023 at 11:49:49PM +0800, Jisheng Zhang wrote:
...

Review

Quoted 12 lines

#define ALT_ENTRY(oldptr, newptr, vendor_id, errata_id, newlen) \

(quoted diff) +5 -5
-	RISCV_PTR " " oldptr "\n"					\
-	RISCV_PTR " " newptr "\n"					\
-	REG_ASM " " vendor_id "\n"					\
-	REG_ASM " " newlen "\n"						\
-	".word " errata_id "\n"
+	".4byte	((" oldptr ") - .) \n"					\
+	".4byte	((" newptr ") - .) \n"					\
+	".2byte	" vendor_id "\n"					\
+	".2byte " newlen "\n"						\
+	".4byte	" errata_id "\n"

Hi Jisheng,

Quoted 6 lines

This patch breaks loading the KVM module for me. I got "kvm: Unknown relocation type 34". My guess is that these 2 byte fields are inspiring the compiler to emit 16-bit relocation types. The patch below fixes things for me. If you agree with fixing it this way, rather than changing something in alternatives, like not using 2 byte fields, then please pick the below patch up in your series.

Hi Jisheng,

I'm poking again on this as I see this series is now working its way to be merged into for-next. I'd rather avoid the bisection breakage which will be present if we fix this issue afterwards by having a v5 merged which addresses the issue in the correct patch order.

Thanks,
drew

Quoted 61 lines

From 4d203697aa745a0cd3a9217d547a9fb7fa2a87c7 Mon Sep 17 00:00:00 2001
From: Andrew Jones <[email protected]>
Date: Fri, 20 Jan 2023 19:05:44 +0100
Subject: [PATCH] riscv: module: Add ADD16 and SUB16 rela types
Content-type: text/plain

To prepare for 16-bit relocation types to be emitted in alternatives add support for ADD16 and SUB16.

---
 arch/riscv/kernel/module.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
arch/riscv/kernel/module.c +16 -0
index 76f4b9c2ec5b..7c651d55fcbd 100644
@@ -268,6 +268,13 @@ static int apply_r_riscv_align_rela(struct module *me, u32 *location,
 	return -EINVAL;
 }
 
+static int apply_r_riscv_add16_rela(struct module *me, u32 *location,
+				    Elf_Addr v)
+{
+	*(u16 *)location += (u16)v;
+	return 0;
+}
+
 static int apply_r_riscv_add32_rela(struct module *me, u32 *location,
 				    Elf_Addr v)
 {
@@ -282,6 +289,13 @@ static int apply_r_riscv_add64_rela(struct module *me, u32 *location,
 	return 0;
 }
 
+static int apply_r_riscv_sub16_rela(struct module *me, u32 *location,
+				    Elf_Addr v)
+{
+	*(u16 *)location -= (u16)v;
+	return 0;
+}
+
 static int apply_r_riscv_sub32_rela(struct module *me, u32 *location,
 				    Elf_Addr v)
 {
@@ -315,8 +329,10 @@ 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_ALIGN]			= apply_r_riscv_align_rela,
+	[R_RISCV_ADD16]			= apply_r_riscv_add16_rela,
 	[R_RISCV_ADD32]			= apply_r_riscv_add32_rela,
 	[R_RISCV_ADD64]			= apply_r_riscv_add64_rela,
+	[R_RISCV_SUB16]			= apply_r_riscv_sub16_rela,
 	[R_RISCV_SUB32]			= apply_r_riscv_sub32_rela,
 	[R_RISCV_SUB64]			= apply_r_riscv_sub64_rela,
 };
Signature · 1 line
2.39.0
#16 lore raw

Re: [PATCH v4 09/13] riscv: switch to relative alternative entries

Quoted 25 lines · Andrew Jones
Quoted 17 lines · Andrew Jones

On Sun, Jan 15, 2023 at 11:49:49PM +0800, Jisheng Zhang wrote:
...

Review

Quoted 12 lines

#define ALT_ENTRY(oldptr, newptr, vendor_id, errata_id, newlen) \

(quoted diff) +5 -5
-	RISCV_PTR " " oldptr "\n"					\
-	RISCV_PTR " " newptr "\n"					\
-	REG_ASM " " vendor_id "\n"					\
-	REG_ASM " " newlen "\n"						\
-	".word " errata_id "\n"
+	".4byte	((" oldptr ") - .) \n"					\
+	".4byte	((" newptr ") - .) \n"					\
+	".2byte	" vendor_id "\n"					\
+	".2byte " newlen "\n"						\
+	".4byte	" errata_id "\n"

Hi Jisheng,

Quoted 6 lines

This patch breaks loading the KVM module for me. I got "kvm: Unknown relocation type 34". My guess is that these 2 byte fields are inspiring the compiler to emit 16-bit relocation types. The patch below fixes things for me. If you agree with fixing it this way, rather than changing something in alternatives, like not using 2 byte fields, then please pick the below patch up in your series.

Quoted 6 lines

Hi Jisheng,

I'm poking again on this as I see this series is now working its way to be merged into for-next. I'd rather avoid the bisection breakage which will be present if we fix this issue afterwards by having a v5 merged which addresses the issue in the correct patch order.

Hi Andrew,

Sorry for being late. I was on holiday in the past few days. I'm cooking v5 and will send out it soon.

Thanks so much

Quoted 65 lines

Thanks,
drew

Quoted 61 lines

From 4d203697aa745a0cd3a9217d547a9fb7fa2a87c7 Mon Sep 17 00:00:00 2001
From: Andrew Jones <[email protected]>
Date: Fri, 20 Jan 2023 19:05:44 +0100
Subject: [PATCH] riscv: module: Add ADD16 and SUB16 rela types
Content-type: text/plain

To prepare for 16-bit relocation types to be emitted in alternatives add support for ADD16 and SUB16.

---
 arch/riscv/kernel/module.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
arch/riscv/kernel/module.c +16 -0
index 76f4b9c2ec5b..7c651d55fcbd 100644
@@ -268,6 +268,13 @@ static int apply_r_riscv_align_rela(struct module *me, u32 *location,
 	return -EINVAL;
 }
 
+static int apply_r_riscv_add16_rela(struct module *me, u32 *location,
+				    Elf_Addr v)
+{
+	*(u16 *)location += (u16)v;
+	return 0;
+}
+
 static int apply_r_riscv_add32_rela(struct module *me, u32 *location,
 				    Elf_Addr v)
 {
@@ -282,6 +289,13 @@ static int apply_r_riscv_add64_rela(struct module *me, u32 *location,
 	return 0;
 }
 
+static int apply_r_riscv_sub16_rela(struct module *me, u32 *location,
+				    Elf_Addr v)
+{
+	*(u16 *)location -= (u16)v;
+	return 0;
+}
+
 static int apply_r_riscv_sub32_rela(struct module *me, u32 *location,
 				    Elf_Addr v)
 {
@@ -315,8 +329,10 @@ 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_ALIGN]			= apply_r_riscv_align_rela,
+	[R_RISCV_ADD16]			= apply_r_riscv_add16_rela,
 	[R_RISCV_ADD32]			= apply_r_riscv_add32_rela,
 	[R_RISCV_ADD64]			= apply_r_riscv_add64_rela,
+	[R_RISCV_SUB16]			= apply_r_riscv_sub16_rela,
 	[R_RISCV_SUB32]			= apply_r_riscv_sub32_rela,
 	[R_RISCV_SUB64]			= apply_r_riscv_sub64_rela,
 };
Signature · 1 line
2.39.0
Reviewed-by
#17 lore raw

Re: [PATCH v4 09/13] riscv: switch to relative alternative entries

In reply to Andrew Jones
Quoted 33 lines · Andrew Jones

On Sun, Jan 15, 2023 at 11:49:49PM +0800, Jisheng Zhang wrote:
...

Review

Quoted 12 lines

#define ALT_ENTRY(oldptr, newptr, vendor_id, errata_id, newlen) \

(quoted diff) +5 -5
-	RISCV_PTR " " oldptr "\n"					\
-	RISCV_PTR " " newptr "\n"					\
-	REG_ASM " " vendor_id "\n"					\
-	REG_ASM " " newlen "\n"						\
-	".word " errata_id "\n"
+	".4byte	((" oldptr ") - .) \n"					\
+	".4byte	((" newptr ") - .) \n"					\
+	".2byte	" vendor_id "\n"					\
+	".2byte " newlen "\n"						\
+	".4byte	" errata_id "\n"

Hi Jisheng,

This patch breaks loading the KVM module for me. I got "kvm: Unknown relocation type 34". My guess is that these 2 byte fields are inspiring the compiler to emit 16-bit relocation types. The patch below fixes things for me. If you agree with fixing it this way, rather than changing something in alternatives, like not using 2 byte fields, then please pick the below patch up in your series.

Thanks,
drew

From 4d203697aa745a0cd3a9217d547a9fb7fa2a87c7 Mon Sep 17 00:00:00 2001
From: Andrew Jones <[email protected]>
Date: Fri, 20 Jan 2023 19:05:44 +0100
Subject: [PATCH] riscv: module: Add ADD16 and SUB16 rela types
Content-type: text/plain

Quoted 4 lines

To prepare for 16-bit relocation types to be emitted in alternatives add support for ADD16 and SUB16.

For the fixup:

Thanks!

Quoted 50 lines
---
 arch/riscv/kernel/module.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
arch/riscv/kernel/module.c +16 -0
index 76f4b9c2ec5b..7c651d55fcbd 100644
@@ -268,6 +268,13 @@ static int apply_r_riscv_align_rela(struct module *me, u32 *location,
 	return -EINVAL;
 }
 
+static int apply_r_riscv_add16_rela(struct module *me, u32 *location,
+				    Elf_Addr v)
+{
+	*(u16 *)location += (u16)v;
+	return 0;
+}
+
 static int apply_r_riscv_add32_rela(struct module *me, u32 *location,
 				    Elf_Addr v)
 {
@@ -282,6 +289,13 @@ static int apply_r_riscv_add64_rela(struct module *me, u32 *location,
 	return 0;
 }
 
+static int apply_r_riscv_sub16_rela(struct module *me, u32 *location,
+				    Elf_Addr v)
+{
+	*(u16 *)location -= (u16)v;
+	return 0;
+}
+
 static int apply_r_riscv_sub32_rela(struct module *me, u32 *location,
 				    Elf_Addr v)
 {
@@ -315,8 +329,10 @@ 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_ALIGN]			= apply_r_riscv_align_rela,
+	[R_RISCV_ADD16]			= apply_r_riscv_add16_rela,
 	[R_RISCV_ADD32]			= apply_r_riscv_add32_rela,
 	[R_RISCV_ADD64]			= apply_r_riscv_add64_rela,
+	[R_RISCV_SUB16]			= apply_r_riscv_sub16_rela,
 	[R_RISCV_SUB32]			= apply_r_riscv_sub32_rela,
 	[R_RISCV_SUB64]			= apply_r_riscv_sub64_rela,
 };
Signature · 1 line
2.39.0

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/kvm-riscv/attachments/20230126/94bf811f/attachment-0001.sig>

#18 lore raw

PATCH v4 10/13 riscv: alternative: patch alternatives in the vDSO

In reply to Jisheng Zhang

Make it possible to use alternatives in the vDSO, so that better implementations can be used if possible.

---
 arch/riscv/include/asm/vdso.h     |  4 ++++
 arch/riscv/kernel/alternative.c   | 29 +++++++++++++++++++++++++++++
 arch/riscv/kernel/vdso.c          |  5 -----
 arch/riscv/kernel/vdso/vdso.lds.S |  7 +++++++
 4 files changed, 40 insertions(+), 5 deletions(-)
arch/riscv/include/asm/vdso.h +4 -0
index a7644f46d0e5..f891478829a5 100644
@@ -28,8 +28,12 @@
 #define COMPAT_VDSO_SYMBOL(base, name)						\
 	(void __user *)((unsigned long)(base) + compat__vdso_##name##_offset)
 
+extern char compat_vdso_start[], compat_vdso_end[];
+
 #endif /* CONFIG_COMPAT */
 
+extern char vdso_start[], vdso_end[];
+
 #endif /* !__ASSEMBLY__ */
 
 #endif /* CONFIG_MMU */
arch/riscv/kernel/alternative.c +29 -0
index 3d4f1f32c7f6..fc341b69bf62 100644
@@ -11,7 +11,9 @@
 #include <linux/cpu.h>
 #include <linux/uaccess.h>
 #include <asm/alternative.h>
+#include <asm/module.h>
 #include <asm/sections.h>
+#include <asm/vdso.h>
 #include <asm/vendorid_list.h>
 #include <asm/sbi.h>
 #include <asm/csr.h>
@@ -160,6 +162,31 @@ static void __init_or_module _apply_alternatives(struct alt_entry *begin,
 				stage);
 }
 
+#ifdef CONFIG_MMU
+static void __init apply_vdso_alternatives(void)
+{
+	const Elf_Ehdr *hdr;
+	const Elf_Shdr *shdr;
+	const Elf_Shdr *alt;
+	struct alt_entry *begin, *end;
+
+	hdr = (Elf_Ehdr *)vdso_start;
+	shdr = (void *)hdr + hdr->e_shoff;
+	alt = find_section(hdr, shdr, ".alternative");
+	if (!alt)
+		return;
+
+	begin = (void *)hdr + alt->sh_offset,
+	end = (void *)hdr + alt->sh_offset + alt->sh_size,
+
+	_apply_alternatives((struct alt_entry *)begin,
+			    (struct alt_entry *)end,
+			    RISCV_ALTERNATIVES_BOOT);
+}
+#else
+static void __init apply_vdso_alternatives(void) { }
+#endif
+
 void __init apply_boot_alternatives(void)
 {
 	/* If called on non-boot cpu things could go wrong */
@@ -168,6 +195,8 @@ void __init apply_boot_alternatives(void)
 	_apply_alternatives((struct alt_entry *)__alt_start,
 			    (struct alt_entry *)__alt_end,
 			    RISCV_ALTERNATIVES_BOOT);
+
+	apply_vdso_alternatives();
 }
 
 /*
arch/riscv/kernel/vdso.c +0 -5
index e410275918ac..4e631c098f4d 100644
@@ -22,11 +22,6 @@ struct vdso_data {
 };
 #endif
 
-extern char vdso_start[], vdso_end[];
-#ifdef CONFIG_COMPAT
-extern char compat_vdso_start[], compat_vdso_end[];
-#endif
-
 enum vvar_pages {
 	VVAR_DATA_PAGE_OFFSET,
 	VVAR_TIMENS_PAGE_OFFSET,
arch/riscv/kernel/vdso/vdso.lds.S +7 -0
index 150b1a572e61..4a0606633290 100644
@@ -40,6 +40,13 @@ SECTIONS
 	. = 0x800;
 	.text		: { *(.text .text.*) }		:text
 
+	. = ALIGN(4);
+	.alternative : {
+		__alt_start = .;
+		*(.alternative)
+		__alt_end = .;
+	}
+
 	.data		: {
 		*(.got.plt) *(.got)
 		*(.data .data.* .gnu.linkonce.d.*)
Signature · 1 line
2.38.1
#19 lore raw

PATCH v4 11/13 riscv: cpu_relax: switch to riscv_has_extension_likely()

In reply to Jisheng Zhang

Switch cpu_relax() from static branch to the new helper riscv_has_extension_likely()

---
 arch/riscv/include/asm/vdso/processor.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
arch/riscv/include/asm/vdso/processor.h +1 -1
index fa70cfe507aa..edf0e25e43d1 100644
@@ -10,7 +10,7 @@
 
 static inline void cpu_relax(void)
 {
-	if (!static_branch_likely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_ZIHINTPAUSE])) {
+	if (!riscv_has_extension_likely(RISCV_ISA_EXT_ZIHINTPAUSE)) {
 #ifdef __riscv_muldiv
 		int dummy;
 		/* In lieu of a halt instruction, induce a long-latency stall. */
Signature · 1 line
2.38.1
#20 lore raw

PATCH v4 12/13 riscv: KVM: Switch has_svinval() to riscv_has_extension_unlikely()

In reply to Jisheng Zhang

From: Andrew Jones <[email protected]>

Switch has_svinval() from static branch to the new helper riscv_has_extension_unlikely().

---
 arch/riscv/kvm/tlb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
arch/riscv/kvm/tlb.c +1 -2
index 309d79b3e5cd..aa3da18ad873 100644
@@ -15,8 +15,7 @@
 #include <asm/hwcap.h>
 #include <asm/insn-def.h>
 
-#define has_svinval()	\
-	static_branch_unlikely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_SVINVAL])
+#define has_svinval()	riscv_has_extension_unlikely(RISCV_ISA_EXT_SVINVAL)
 
 void kvm_riscv_local_hfence_gvma_vmid_gpa(unsigned long vmid,
 					  gpa_t gpa, gpa_t gpsz,
Signature · 1 line
2.38.1
maintainer Acked-by on v4 12/13
#21 lore raw

Re: [PATCH v4 12/13] riscv: KVM: Switch has_svinval() to riscv_has_extension_unlikely()

Quoted 8 lines · Jisheng Zhang

From: Andrew Jones <[email protected]>

Switch has_svinval() from static branch to the new helper riscv_has_extension_unlikely().

For KVM RISC-V:

Regards,
Anup

Quoted 21 lines
---
 arch/riscv/kvm/tlb.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
arch/riscv/kvm/tlb.c +1 -3
index 309d79b3e5cd..aa3da18ad873 100644
@@ -15,8 +15,7 @@
 #include <asm/hwcap.h>
 #include <asm/insn-def.h>
 
-#define has_svinval()  \
-       static_branch_unlikely(&riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_SVINVAL])
+#define has_svinval()  riscv_has_extension_unlikely(RISCV_ISA_EXT_SVINVAL)
 
 void kvm_riscv_local_hfence_gvma_vmid_gpa(unsigned long vmid,
                                          gpa_t gpa, gpa_t gpsz,
--

2.38.1

#22 lore raw

PATCH v4 13/13 riscv: remove riscv_isa_ext_keys[] array and related usage

In reply to Jisheng Zhang

All users have switched to riscv_has_extension_*, remove unused definitions, vars and related setting code.

---
 arch/riscv/include/asm/hwcap.h | 31 -------------------------------
 arch/riscv/kernel/cpufeature.c |  9 ---------
 2 files changed, 40 deletions(-)
arch/riscv/include/asm/hwcap.h +0 -31
index 1767a9ce1a04..e3749bee5c24 100644
@@ -60,19 +60,6 @@ enum {
 
 extern unsigned long elf_hwcap;
 
-
-/*
- * This enum represents the logical ID for each RISC-V ISA extension static
- * keys. We can use static key to optimize code path if some ISA extensions
- * are available.
- */
-enum riscv_isa_ext_key {
-	RISCV_ISA_EXT_KEY_FPU,		/* For 'F' and 'D' */
-	RISCV_ISA_EXT_KEY_ZIHINTPAUSE,
-	RISCV_ISA_EXT_KEY_SVINVAL,
-	RISCV_ISA_EXT_KEY_MAX,
-};
-
 struct riscv_isa_ext_data {
 	/* Name of the extension displayed to userspace via /proc/cpuinfo */
 	char uprop[RISCV_ISA_EXT_NAME_LEN_MAX];
@@ -80,24 +67,6 @@ struct riscv_isa_ext_data {
 	unsigned int isa_ext_id;
 };
 
-extern struct static_key_false riscv_isa_ext_keys[RISCV_ISA_EXT_KEY_MAX];
-
-static __always_inline int riscv_isa_ext2key(int num)
-{
-	switch (num) {
-	case RISCV_ISA_EXT_f:
-		return RISCV_ISA_EXT_KEY_FPU;
-	case RISCV_ISA_EXT_d:
-		return RISCV_ISA_EXT_KEY_FPU;
-	case RISCV_ISA_EXT_ZIHINTPAUSE:
-		return RISCV_ISA_EXT_KEY_ZIHINTPAUSE;
-	case RISCV_ISA_EXT_SVINVAL:
-		return RISCV_ISA_EXT_KEY_SVINVAL;
-	default:
-		return -EINVAL;
-	}
-}
-
 static __always_inline bool
 riscv_has_extension_likely(const unsigned long ext)
 {
arch/riscv/kernel/cpufeature.c +0 -9
index 50cfbbbecaa7..636dabfd35d0 100644
@@ -29,9 +29,6 @@ unsigned long elf_hwcap __read_mostly;
 /* Host ISA bitmap */
 static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly;
 
-DEFINE_STATIC_KEY_ARRAY_FALSE(riscv_isa_ext_keys, RISCV_ISA_EXT_KEY_MAX);
-EXPORT_SYMBOL(riscv_isa_ext_keys);
-
 /**
  * riscv_isa_extension_base() - Get base extension word
  *
@@ -266,12 +263,6 @@ void __init riscv_fill_hwcap(void)
 		if (elf_hwcap & BIT_MASK(i))
 			print_str[j++] = (char)('a' + i);
 	pr_info("riscv: ELF capabilities %s\n", print_str);
-
-	for_each_set_bit(i, riscv_isa, RISCV_ISA_EXT_MAX) {
-		j = riscv_isa_ext2key(i);
-		if (j >= 0)
-			static_branch_enable(&riscv_isa_ext_keys[j]);
-	}
 }
 
 #ifdef CONFIG_RISCV_ALTERNATIVE
Signature · 1 line
2.38.1
#23 lore raw
In reply to Jisheng Zhang
This series was applied to riscv/linux.git (for-next) by Palmer Dabbelt <[email protected]>:

Hello:

This series was applied to riscv/linux.git (for-next) by Palmer Dabbelt <[email protected]>:

Quoted 9 lines · On Sun, 15 Jan 2023 23:49:40 +0800 you wrote:

Generally, riscv ISA extensions are fixed for any specific hardware platform, so a hart's features won't change after booting, this chacteristic makes it straightforward to use a static branch to check a specific ISA extension is supported or not to optimize performance.

However, some ISA extensions such as SVPBMT and ZICBOM are handled via. the alternative sequences.

[...]

Here is the summary with links:
- [v4,01/13] riscv: fix jal offsets in patched alternatives

    https://git.kernel.org/riscv/c/9d5567ccf96f

- [v4,02/13] riscv: move riscv_noncoherent_supported() out of ZICBOM probe (no matching commit)
- [v4,03/13] riscv: cpufeature: detect RISCV_ALTERNATIVES_EARLY_BOOT earlier (no matching commit)
- [v4,04/13] riscv: hwcap: make ISA extension ids can be used in asm
(no matching commit)
- [v4,05/13] riscv: cpufeature: extend riscv_cpufeature_patch_func to all ISA extensions (no matching commit)
- [v4,06/13] riscv: introduce riscv_has_extension_[un]likely()
(no matching commit)
- [v4,07/13] riscv: fpu: switch has_fpu() to riscv_has_extension_likely() (no matching commit)
- [v4,08/13] riscv: module: move find_section to module.h
(no matching commit)
- [v4,09/13] riscv: switch to relative alternative entries
(no matching commit)
- [v4,10/13] riscv: alternative: patch alternatives in the vDSO
(no matching commit)
- [v4,11/13] riscv: cpu_relax: switch to riscv_has_extension_likely()
(no matching commit)
- [v4,12/13] riscv: KVM: Switch has_svinval() to riscv_has_extension_unlikely() (no matching commit)
- [v4,13/13] riscv: remove riscv_isa_ext_keys[] array and related usage (no matching commit)

You are awesome, thank you!

Signature · 2 lines
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html