* [PATCH v2 00/13] riscv: improve boot time isa extensions handling
@ 2022-12-04 17:46 Jisheng Zhang
2022-12-04 17:46 ` [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives Jisheng Zhang
` (12 more replies)
0 siblings, 13 replies; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-04 17:46 UTC (permalink / raw)
To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones
Cc: linux-riscv, linux-kernel, kvm, kvm-riscv
Generally, riscv ISA extensions are fixed for any specific hardware
platform, that's to say, the hart features won't change any more
after booting, this chacteristic make it straightforward to use
static branch to check one 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 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@sholland.org/
[2]https://lore.kernel.org/linux-arm-kernel/20220912162210.3626215-8-mark.rutland@arm.com/
[3]https://lore.kernel.org/linux-riscv/20221130225614.1594256-1-heiko@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 | 4 +-
arch/riscv/errata/thead/errata.c | 11 ++-
arch/riscv/include/asm/alternative-macros.h | 20 ++---
arch/riscv/include/asm/alternative.h | 14 +--
arch/riscv/include/asm/errata_list.h | 9 +-
arch/riscv/include/asm/hwcap.h | 96 +++++++++++----------
arch/riscv/include/asm/module.h | 15 ++++
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 | 63 ++++++++++++++
arch/riscv/kernel/cpufeature.c | 82 +++---------------
arch/riscv/kernel/module.c | 15 ----
arch/riscv/kernel/setup.c | 2 +
arch/riscv/kernel/vdso.c | 5 --
arch/riscv/kernel/vdso/vdso.lds.S | 7 ++
arch/riscv/kvm/tlb.c | 3 +-
17 files changed, 191 insertions(+), 164 deletions(-)
--
2.37.2
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives
2022-12-04 17:46 [PATCH v2 00/13] riscv: improve boot time isa extensions handling Jisheng Zhang
@ 2022-12-04 17:46 ` Jisheng Zhang
2022-12-05 14:57 ` Andrew Jones
2022-12-05 15:31 ` Heiko Stübner
2022-12-04 17:46 ` [PATCH v2 02/13] riscv: move riscv_noncoherent_supported() out of ZICBOM probe Jisheng Zhang
` (11 subsequent siblings)
12 siblings, 2 replies; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-04 17:46 UTC (permalink / raw)
To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones
Cc: linux-riscv, linux-kernel, kvm, kvm-riscv
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.
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
arch/riscv/include/asm/alternative.h | 2 ++
arch/riscv/kernel/alternative.c | 38 ++++++++++++++++++++++++++++
arch/riscv/kernel/cpufeature.c | 3 +++
3 files changed, 43 insertions(+)
diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
index c58ec3cc4bc3..33eae9541684 100644
--- a/arch/riscv/include/asm/alternative.h
+++ b/arch/riscv/include/asm/alternative.h
@@ -29,6 +29,8 @@ void apply_module_alternatives(void *start, size_t length);
void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
int patch_offset);
+void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
+ int patch_offset);
struct alt_entry {
void *old_ptr; /* address of original instruciton or data */
diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
index 292cc42dc3be..9d88375624b5 100644
--- a/arch/riscv/kernel/alternative.c
+++ b/arch/riscv/kernel/alternative.c
@@ -125,6 +125,44 @@ void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
}
}
+#define to_jal_imm(value) \
+ (((value & (RV_J_IMM_10_1_MASK << RV_J_IMM_10_1_OFF)) << RV_I_IMM_11_0_OPOFF) | \
+ ((value & (RV_J_IMM_11_MASK << RV_J_IMM_11_OFF)) << RV_J_IMM_11_OPOFF) | \
+ ((value & (RV_J_IMM_19_12_OPOFF << RV_J_IMM_19_12_OFF)) << RV_J_IMM_19_12_OPOFF) | \
+ ((value & (1 << RV_J_IMM_SIGN_OFF)) << RV_J_IMM_SIGN_OPOFF))
+
+void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
+ int patch_offset)
+{
+ int num_instr = len / sizeof(u32);
+ unsigned int call;
+ int i;
+ int imm;
+
+ for (i = 0; i < num_instr; i++) {
+ u32 inst = riscv_instruction_at(alt_ptr, i);
+
+ if (!riscv_insn_is_jal(inst))
+ continue;
+
+ /* get and adjust new target address */
+ imm = RV_EXTRACT_JTYPE_IMM(inst);
+ imm -= patch_offset;
+
+ /* pick the original jal */
+ call = inst;
+
+ /* drop the old IMMs, all jal imm bits sit at 31:12 */
+ call &= ~GENMASK(31, 12);
+
+ /* add the adapted IMMs */
+ call |= to_jal_imm(imm);
+
+ /* patch the call place again */
+ patch_text_nosync(alt_ptr + i * sizeof(u32), &call, 4);
+ }
+}
+
/*
* This is called very early in the boot process (directly after we run
* a feature detect on the boot CPU). No need to worry about other CPUs
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index ba62a4ff5ccd..c743f0adc794 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -324,6 +324,9 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
riscv_alternative_fix_auipc_jalr(alt->old_ptr,
alt->alt_len,
alt->old_ptr - alt->alt_ptr);
+ riscv_alternative_fix_jal(alt->old_ptr,
+ alt->alt_len,
+ alt->old_ptr - alt->alt_ptr);
}
}
}
--
2.37.2
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v2 02/13] riscv: move riscv_noncoherent_supported() out of ZICBOM probe
2022-12-04 17:46 [PATCH v2 00/13] riscv: improve boot time isa extensions handling Jisheng Zhang
2022-12-04 17:46 ` [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives Jisheng Zhang
@ 2022-12-04 17:46 ` Jisheng Zhang
2022-12-04 21:52 ` Heiko Stübner
2022-12-04 17:46 ` [PATCH v2 03/13] riscv: cpufeature: detect RISCV_ALTERNATIVES_EARLY_BOOT earlier Jisheng Zhang
` (10 subsequent siblings)
12 siblings, 1 reply; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-04 17:46 UTC (permalink / raw)
To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones
Cc: linux-riscv, linux-kernel, kvm, kvm-riscv, Conor Dooley
It's a bit weird to call riscv_noncoherent_supported() each time when
insmoding a module. Move the calling out of feature patch func.
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
---
arch/riscv/kernel/cpufeature.c | 1 -
arch/riscv/kernel/setup.c | 2 ++
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index c743f0adc794..364d1fe86bea 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -274,7 +274,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;
}
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index 86acd690d529..6eea40bf8c6b 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -300,6 +300,8 @@ void __init setup_arch(char **cmdline_p)
riscv_init_cbom_blocksize();
riscv_fill_hwcap();
apply_boot_alternatives();
+ if (riscv_isa_extension_available(NULL, ZICBOM))
+ riscv_noncoherent_supported();
}
static int __init topology_init(void)
--
2.37.2
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v2 03/13] riscv: cpufeature: detect RISCV_ALTERNATIVES_EARLY_BOOT earlier
2022-12-04 17:46 [PATCH v2 00/13] riscv: improve boot time isa extensions handling Jisheng Zhang
2022-12-04 17:46 ` [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives Jisheng Zhang
2022-12-04 17:46 ` [PATCH v2 02/13] riscv: move riscv_noncoherent_supported() out of ZICBOM probe Jisheng Zhang
@ 2022-12-04 17:46 ` Jisheng Zhang
2022-12-05 19:09 ` Conor Dooley
2022-12-04 17:46 ` [PATCH v2 04/13] riscv: hwcap: make ISA extension ids can be used in asm Jisheng Zhang
` (9 subsequent siblings)
12 siblings, 1 reply; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-04 17:46 UTC (permalink / raw)
To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones
Cc: linux-riscv, linux-kernel, kvm, kvm-riscv
Now, the riscv_cpufeature_patch_func() do nothing in the stage of
RISCV_ALTERNATIVES_EARLY_BOOT. We can move the detection of "early
boot" stage earlier.
In following patch, we will make riscv_cpufeature_patch_func() scans
all ISA extensions.
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
---
arch/riscv/kernel/cpufeature.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 364d1fe86bea..a4d2af67e05c 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -305,6 +305,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;
--
2.37.2
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v2 04/13] riscv: hwcap: make ISA extension ids can be used in asm
2022-12-04 17:46 [PATCH v2 00/13] riscv: improve boot time isa extensions handling Jisheng Zhang
` (2 preceding siblings ...)
2022-12-04 17:46 ` [PATCH v2 03/13] riscv: cpufeature: detect RISCV_ALTERNATIVES_EARLY_BOOT earlier Jisheng Zhang
@ 2022-12-04 17:46 ` Jisheng Zhang
2022-12-05 18:53 ` Conor Dooley
2022-12-04 17:46 ` [PATCH v2 05/13] riscv: cpufeature: extend riscv_cpufeature_patch_func to all ISA extensions Jisheng Zhang
` (8 subsequent siblings)
12 siblings, 1 reply; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-04 17:46 UTC (permalink / raw)
To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones
Cc: linux-riscv, linux-kernel, kvm, kvm-riscv
We will make use of ISA extension in asm files, so make the multi-letter
RISC-V ISA extension IDs macros rather than enums and move them and
those base ISA extension IDs to suitable place.
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
arch/riscv/include/asm/hwcap.h | 43 ++++++++++++++++------------------
1 file changed, 20 insertions(+), 23 deletions(-)
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index b22525290073..996884986fea 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -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 = RISCV_ISA_EXT_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,
};
+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
--
2.37.2
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v2 05/13] riscv: cpufeature: extend riscv_cpufeature_patch_func to all ISA extensions
2022-12-04 17:46 [PATCH v2 00/13] riscv: improve boot time isa extensions handling Jisheng Zhang
` (3 preceding siblings ...)
2022-12-04 17:46 ` [PATCH v2 04/13] riscv: hwcap: make ISA extension ids can be used in asm Jisheng Zhang
@ 2022-12-04 17:46 ` Jisheng Zhang
2022-12-05 19:37 ` Conor Dooley
2022-12-04 17:46 ` [PATCH v2 06/13] riscv: introduce riscv_has_extension_[un]likely() Jisheng Zhang
` (7 subsequent siblings)
12 siblings, 1 reply; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-04 17:46 UTC (permalink / raw)
To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones
Cc: linux-riscv, linux-kernel, kvm, kvm-riscv
make the riscv_cpufeature_patch_func() scan all ISA extensions rather
than limited feature macros.
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
---
arch/riscv/include/asm/errata_list.h | 9 ++--
arch/riscv/kernel/cpufeature.c | 73 +++++-----------------------
2 files changed, 15 insertions(+), 67 deletions(-)
diff --git a/arch/riscv/include/asm/errata_list.h b/arch/riscv/include/asm/errata_list.h
index 19a771085781..722525f4fc96 100644
--- a/arch/riscv/include/asm/errata_list.h
+++ b/arch/riscv/include/asm/errata_list.h
@@ -6,6 +6,7 @@
#define ASM_ERRATA_LIST_H
#include <asm/alternative.h>
+#include <asm/hwcap.h>
#include <asm/vendorid_list.h>
#ifdef CONFIG_ERRATA_SIFIVE
@@ -20,10 +21,6 @@
#define ERRATA_THEAD_NUMBER 2
#endif
-#define CPUFEATURE_SVPBMT 0
-#define CPUFEATURE_ZICBOM 1
-#define CPUFEATURE_NUMBER 2
-
#ifdef __ASSEMBLY__
#define ALT_INSN_FAULT(x) \
@@ -53,7 +50,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) \
@@ -127,7 +124,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" \
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index a4d2af67e05c..6244be5cd94a 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -252,58 +252,11 @@ void __init riscv_fill_hwcap(void)
}
#ifdef CONFIG_RISCV_ALTERNATIVE
-static bool __init_or_module cpufeature_probe_svpbmt(unsigned int stage)
-{
- if (!IS_ENABLED(CONFIG_RISCV_ISA_SVPBMT))
- return false;
-
- if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
- return false;
-
- return riscv_isa_extension_available(NULL, SVPBMT);
-}
-
-static bool __init_or_module cpufeature_probe_zicbom(unsigned int stage)
-{
- if (!IS_ENABLED(CONFIG_RISCV_ISA_ZICBOM))
- return false;
-
- if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
- return false;
-
- if (!riscv_isa_extension_available(NULL, ZICBOM))
- return false;
-
- return true;
-}
-
-/*
- * Probe presence of individual extensions.
- *
- * This code may also be executed before kernel relocation, so we cannot use
- * addresses generated by the address-of operator as they won't be valid in
- * this context.
- */
-static u32 __init_or_module cpufeature_probe(unsigned int stage)
-{
- u32 cpu_req_feature = 0;
-
- if (cpufeature_probe_svpbmt(stage))
- cpu_req_feature |= BIT(CPUFEATURE_SVPBMT);
-
- if (cpufeature_probe_zicbom(stage))
- cpu_req_feature |= BIT(CPUFEATURE_ZICBOM);
-
- return cpu_req_feature;
-}
-
void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
struct alt_entry *end,
unsigned int stage)
{
- u32 cpu_req_feature = cpufeature_probe(stage);
struct alt_entry *alt;
- u32 tmp;
if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
return;
@@ -311,25 +264,23 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
for (alt = begin; alt < end; alt++) {
if (alt->vendor_id != 0)
continue;
- if (alt->errata_id >= CPUFEATURE_NUMBER) {
- WARN(1, "This feature id:%d is not in kernel cpufeature list",
+ if (alt->errata_id >= RISCV_ISA_EXT_MAX) {
+ WARN(1, "This extension id:%d is not in ISA extension list",
alt->errata_id);
continue;
}
- tmp = (1U << alt->errata_id);
- if (cpu_req_feature & tmp) {
- /* do the basic patching */
- patch_text_nosync(alt->old_ptr, alt->alt_ptr,
- alt->alt_len);
+ if (!__riscv_isa_extension_available(NULL, alt->errata_id))
+ continue;
- riscv_alternative_fix_auipc_jalr(alt->old_ptr,
- alt->alt_len,
- alt->old_ptr - alt->alt_ptr);
- riscv_alternative_fix_jal(alt->old_ptr,
- alt->alt_len,
- alt->old_ptr - alt->alt_ptr);
- }
+ /* do the basic patching */
+ patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
+ riscv_alternative_fix_auipc_jalr(alt->old_ptr,
+ alt->alt_len,
+ alt->old_ptr - alt->alt_ptr);
+ riscv_alternative_fix_jal(alt->old_ptr,
+ alt->alt_len,
+ alt->old_ptr - alt->alt_ptr);
}
}
#endif
--
2.37.2
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v2 06/13] riscv: introduce riscv_has_extension_[un]likely()
2022-12-04 17:46 [PATCH v2 00/13] riscv: improve boot time isa extensions handling Jisheng Zhang
` (4 preceding siblings ...)
2022-12-04 17:46 ` [PATCH v2 05/13] riscv: cpufeature: extend riscv_cpufeature_patch_func to all ISA extensions Jisheng Zhang
@ 2022-12-04 17:46 ` Jisheng Zhang
2022-12-06 20:25 ` Conor Dooley
2022-12-04 17:46 ` [PATCH v2 07/13] riscv: fpu: switch has_fpu() to riscv_has_extension_likely() Jisheng Zhang
` (6 subsequent siblings)
12 siblings, 1 reply; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-04 17:46 UTC (permalink / raw)
To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones
Cc: linux-riscv, linux-kernel, kvm, kvm-riscv
Generally, riscv ISA extensions are fixed for any specific hardware
platform, that's to say, the hart features won't change any more
after booting, this chacteristic make it straightforward to use
static branch to check one 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.
[1]https://lore.kernel.org/linux-riscv/20220922060958.44203-1-samuel@sholland.org/
[2]https://lore.kernel.org/linux-arm-kernel/20220912162210.3626215-8-mark.rutland@arm.com/
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
---
arch/riscv/include/asm/hwcap.h | 37 ++++++++++++++++++++++++++++++++++
1 file changed, 37 insertions(+)
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index 996884986fea..e2d3f6df7701 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -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>
@@ -96,6 +97,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)
--
2.37.2
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v2 07/13] riscv: fpu: switch has_fpu() to riscv_has_extension_likely()
2022-12-04 17:46 [PATCH v2 00/13] riscv: improve boot time isa extensions handling Jisheng Zhang
` (5 preceding siblings ...)
2022-12-04 17:46 ` [PATCH v2 06/13] riscv: introduce riscv_has_extension_[un]likely() Jisheng Zhang
@ 2022-12-04 17:46 ` Jisheng Zhang
2022-12-04 17:46 ` [PATCH v2 08/13] riscv: module: move find_section to module.h Jisheng Zhang
` (5 subsequent siblings)
12 siblings, 0 replies; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-04 17:46 UTC (permalink / raw)
To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones
Cc: linux-riscv, linux-kernel, kvm, kvm-riscv
Switch has_fpu() from statich branch to the new helper
riscv_has_extension_likely().
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
---
arch/riscv/include/asm/switch_to.h | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/riscv/include/asm/switch_to.h b/arch/riscv/include/asm/switch_to.h
index 11463489fec6..60f8ca01d36e 100644
--- a/arch/riscv/include/asm/switch_to.h
+++ b/arch/riscv/include/asm/switch_to.h
@@ -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; }
--
2.37.2
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v2 08/13] riscv: module: move find_section to module.h
2022-12-04 17:46 [PATCH v2 00/13] riscv: improve boot time isa extensions handling Jisheng Zhang
` (6 preceding siblings ...)
2022-12-04 17:46 ` [PATCH v2 07/13] riscv: fpu: switch has_fpu() to riscv_has_extension_likely() Jisheng Zhang
@ 2022-12-04 17:46 ` Jisheng Zhang
2022-12-05 15:25 ` Andrew Jones
2022-12-06 20:44 ` Conor Dooley
2022-12-04 17:46 ` [PATCH v2 09/13] riscv: switch to relative alternative entries Jisheng Zhang
` (4 subsequent siblings)
12 siblings, 2 replies; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-04 17:46 UTC (permalink / raw)
To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones
Cc: linux-riscv, linux-kernel, kvm, kvm-riscv
Move it to the header so that the implementation can be shared
by the alternatives code.
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
arch/riscv/include/asm/module.h | 15 +++++++++++++++
arch/riscv/kernel/module.c | 15 ---------------
2 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/arch/riscv/include/asm/module.h b/arch/riscv/include/asm/module.h
index 76aa96a9fc08..78722a79fc59 100644
--- a/arch/riscv/include/asm/module.h
+++ b/arch/riscv/include/asm/module.h
@@ -111,4 +111,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 */
diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
index 91fe16bfaa07..76f4b9c2ec5b 100644
--- a/arch/riscv/kernel/module.c
+++ b/arch/riscv/kernel/module.c
@@ -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)
--
2.37.2
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v2 09/13] riscv: switch to relative alternative entries
2022-12-04 17:46 [PATCH v2 00/13] riscv: improve boot time isa extensions handling Jisheng Zhang
` (7 preceding siblings ...)
2022-12-04 17:46 ` [PATCH v2 08/13] riscv: module: move find_section to module.h Jisheng Zhang
@ 2022-12-04 17:46 ` Jisheng Zhang
2022-12-05 0:51 ` Guo Ren
2022-12-04 17:46 ` [PATCH v2 10/13] riscv: alternative: patch alternatives in the vDSO Jisheng Zhang
` (3 subsequent siblings)
12 siblings, 1 reply; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-04 17:46 UTC (permalink / raw)
To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones
Cc: linux-riscv, linux-kernel, kvm, kvm-riscv
Instead of using absolute addresses for both the old instrucions and
the alternative instructions, use offsets relative to the alt_entry
values. So we can not only cut the size of the alternative entry, but
also meet the prerequisite for patching alternatives in the vDSO,
since absolute alternative entries are subject to dynamic relocation,
which is incompatible with the vDSO building.
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
arch/riscv/errata/sifive/errata.c | 4 +++-
arch/riscv/errata/thead/errata.c | 11 ++++++++---
arch/riscv/include/asm/alternative-macros.h | 20 ++++++++++----------
arch/riscv/include/asm/alternative.h | 12 ++++++------
arch/riscv/kernel/cpufeature.c | 13 ++++++-------
5 files changed, 33 insertions(+), 27 deletions(-)
diff --git a/arch/riscv/errata/sifive/errata.c b/arch/riscv/errata/sifive/errata.c
index 1031038423e7..0e537cdfd324 100644
--- a/arch/riscv/errata/sifive/errata.c
+++ b/arch/riscv/errata/sifive/errata.c
@@ -107,7 +107,9 @@ 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((void *)&alt->old_offset + alt->old_offset,
+ (void *)&alt->alt_offset + alt->alt_offset,
+ alt->alt_len);
cpu_apply_errata |= tmp;
}
}
diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
index 21546937db39..2a6e335b5a32 100644
--- a/arch/riscv/errata/thead/errata.c
+++ b/arch/riscv/errata/thead/errata.c
@@ -68,6 +68,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, *updptr;
for (alt = begin; alt < end; alt++) {
if (alt->vendor_id != THEAD_VENDOR_ID)
@@ -77,12 +78,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 = (void *)&alt->old_offset + alt->old_offset;
+ updptr = (void *)&alt->alt_offset + alt->alt_offset;
+
/* 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(updptr),
+ alt->alt_len);
else
- patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
+ patch_text_nosync(oldptr, updptr, alt->alt_len);
}
}
diff --git a/arch/riscv/include/asm/alternative-macros.h b/arch/riscv/include/asm/alternative-macros.h
index ec2f3f1b836f..dd40727bc859 100644
--- a/arch/riscv/include/asm/alternative-macros.h
+++ b/arch/riscv/include/asm/alternative-macros.h
@@ -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
+ .long \oldptr - .
+ .long \newptr - .
+ .short \vendor_id
+ .short \new_len
+ .long \errata_id
.endm
.macro ALT_NEW_CONTENT vendor_id, errata_id, enable = 1, new_c : vararg
@@ -75,11 +75,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"
+ ".long ((" oldptr ") - .) \n" \
+ ".long ((" newptr ") - .) \n" \
+ ".short " vendor_id "\n" \
+ ".short " newlen "\n" \
+ ".long " errata_id "\n"
#define ALT_NEW_CONTENT(vendor_id, errata_id, enable, new_c) \
".if " __stringify(enable) " == 1\n" \
diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
index 33eae9541684..3baf32e05b46 100644
--- a/arch/riscv/include/asm/alternative.h
+++ b/arch/riscv/include/asm/alternative.h
@@ -33,12 +33,12 @@ void riscv_alternative_fix_jal(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 to original instruciton or data */
+ s32 alt_offset; /* offset 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;
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 6244be5cd94a..adeac90b1d8e 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -257,6 +257,7 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
unsigned int stage)
{
struct alt_entry *alt;
+ void *oldptr, *updptr;
if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
return;
@@ -270,17 +271,15 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
continue;
}
+ oldptr = (void *)&alt->old_offset + alt->old_offset;
+ updptr = (void *)&alt->alt_offset + alt->alt_offset;
if (!__riscv_isa_extension_available(NULL, alt->errata_id))
continue;
/* do the basic patching */
- patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
- riscv_alternative_fix_auipc_jalr(alt->old_ptr,
- alt->alt_len,
- alt->old_ptr - alt->alt_ptr);
- riscv_alternative_fix_jal(alt->old_ptr,
- alt->alt_len,
- alt->old_ptr - alt->alt_ptr);
+ patch_text_nosync(oldptr, updptr, alt->alt_len);
+ riscv_alternative_fix_auipc_jalr(oldptr, alt->alt_len, oldptr - updptr);
+ riscv_alternative_fix_jal(oldptr, alt->alt_len, oldptr - updptr);
}
}
#endif
--
2.37.2
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v2 10/13] riscv: alternative: patch alternatives in the vDSO
2022-12-04 17:46 [PATCH v2 00/13] riscv: improve boot time isa extensions handling Jisheng Zhang
` (8 preceding siblings ...)
2022-12-04 17:46 ` [PATCH v2 09/13] riscv: switch to relative alternative entries Jisheng Zhang
@ 2022-12-04 17:46 ` Jisheng Zhang
2022-12-05 1:56 ` Guo Ren
2023-01-11 14:12 ` Andrew Jones
2022-12-04 17:46 ` [PATCH v2 11/13] riscv: cpu_relax: switch to riscv_has_extension_likely() Jisheng Zhang
` (2 subsequent siblings)
12 siblings, 2 replies; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-04 17:46 UTC (permalink / raw)
To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones
Cc: linux-riscv, linux-kernel, kvm, kvm-riscv
Make it possible to use alternatives in the vDSO, so that better
implementations can be used if possible.
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
---
arch/riscv/include/asm/vdso.h | 4 ++++
arch/riscv/kernel/alternative.c | 25 +++++++++++++++++++++++++
arch/riscv/kernel/vdso.c | 5 -----
arch/riscv/kernel/vdso/vdso.lds.S | 7 +++++++
4 files changed, 36 insertions(+), 5 deletions(-)
diff --git a/arch/riscv/include/asm/vdso.h b/arch/riscv/include/asm/vdso.h
index af981426fe0f..b6ff7473fb8a 100644
--- a/arch/riscv/include/asm/vdso.h
+++ b/arch/riscv/include/asm/vdso.h
@@ -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 */
diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
index 9d88375624b5..eaf7ddaba54c 100644
--- a/arch/riscv/kernel/alternative.c
+++ b/arch/riscv/kernel/alternative.c
@@ -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>
@@ -187,6 +189,27 @@ static void __init_or_module _apply_alternatives(struct alt_entry *begin,
stage);
}
+static void __init apply_vdso_alternatives(void)
+{
+ const struct elf64_hdr *hdr;
+ const struct elf64_shdr *shdr;
+ const struct elf64_shdr *alt;
+ struct alt_entry *begin, *end;
+
+ hdr = (struct elf64_hdr *)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);
+}
+
void __init apply_boot_alternatives(void)
{
/* If called on non-boot cpu things could go wrong */
@@ -195,6 +218,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();
}
/*
diff --git a/arch/riscv/kernel/vdso.c b/arch/riscv/kernel/vdso.c
index 123d05255fcf..1f47bc6566cf 100644
--- a/arch/riscv/kernel/vdso.c
+++ b/arch/riscv/kernel/vdso.c
@@ -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,
diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S
index 150b1a572e61..4a0606633290 100644
--- a/arch/riscv/kernel/vdso/vdso.lds.S
+++ b/arch/riscv/kernel/vdso/vdso.lds.S
@@ -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.*)
--
2.37.2
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v2 11/13] riscv: cpu_relax: switch to riscv_has_extension_likely()
2022-12-04 17:46 [PATCH v2 00/13] riscv: improve boot time isa extensions handling Jisheng Zhang
` (9 preceding siblings ...)
2022-12-04 17:46 ` [PATCH v2 10/13] riscv: alternative: patch alternatives in the vDSO Jisheng Zhang
@ 2022-12-04 17:46 ` Jisheng Zhang
2022-12-05 0:52 ` Guo Ren
2022-12-06 22:04 ` Conor Dooley
2022-12-04 17:46 ` [PATCH v2 12/13] riscv: KVM: Switch has_svinval() to riscv_has_extension_unlikely() Jisheng Zhang
2022-12-04 17:46 ` [PATCH v2 13/13] riscv: remove riscv_isa_ext_keys[] array and related usage Jisheng Zhang
12 siblings, 2 replies; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-04 17:46 UTC (permalink / raw)
To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones
Cc: linux-riscv, linux-kernel, kvm, kvm-riscv
Switch cpu_relax() from statich branch to the new helper
riscv_has_extension_likely()
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
---
arch/riscv/include/asm/vdso/processor.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h
index fa70cfe507aa..edf0e25e43d1 100644
--- a/arch/riscv/include/asm/vdso/processor.h
+++ b/arch/riscv/include/asm/vdso/processor.h
@@ -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. */
--
2.37.2
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v2 12/13] riscv: KVM: Switch has_svinval() to riscv_has_extension_unlikely()
2022-12-04 17:46 [PATCH v2 00/13] riscv: improve boot time isa extensions handling Jisheng Zhang
` (10 preceding siblings ...)
2022-12-04 17:46 ` [PATCH v2 11/13] riscv: cpu_relax: switch to riscv_has_extension_likely() Jisheng Zhang
@ 2022-12-04 17:46 ` Jisheng Zhang
2022-12-05 0:52 ` Guo Ren
2022-12-04 17:46 ` [PATCH v2 13/13] riscv: remove riscv_isa_ext_keys[] array and related usage Jisheng Zhang
12 siblings, 1 reply; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-04 17:46 UTC (permalink / raw)
To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones
Cc: linux-riscv, linux-kernel, kvm, kvm-riscv
From: Andrew Jones <ajones@ventanamicro.com>
Switch has_svinval() from static branch to the new helper
riscv_has_extension_unlikely().
Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
---
arch/riscv/kvm/tlb.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/arch/riscv/kvm/tlb.c b/arch/riscv/kvm/tlb.c
index 309d79b3e5cd..aa3da18ad873 100644
--- a/arch/riscv/kvm/tlb.c
+++ b/arch/riscv/kvm/tlb.c
@@ -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.37.2
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v2 13/13] riscv: remove riscv_isa_ext_keys[] array and related usage
2022-12-04 17:46 [PATCH v2 00/13] riscv: improve boot time isa extensions handling Jisheng Zhang
` (11 preceding siblings ...)
2022-12-04 17:46 ` [PATCH v2 12/13] riscv: KVM: Switch has_svinval() to riscv_has_extension_unlikely() Jisheng Zhang
@ 2022-12-04 17:46 ` Jisheng Zhang
2022-12-05 0:53 ` Guo Ren
2022-12-06 22:16 ` Conor Dooley
12 siblings, 2 replies; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-04 17:46 UTC (permalink / raw)
To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones
Cc: linux-riscv, linux-kernel, kvm, kvm-riscv
All users have switched to riscv_has_extension_*, removed unused
definitions, vars and related setting code.
Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Reviewed-by: Heiko Stuebner <heiko@sntech.de>
---
arch/riscv/include/asm/hwcap.h | 30 ------------------------------
arch/riscv/kernel/cpufeature.c | 9 ---------
2 files changed, 39 deletions(-)
diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
index e2d3f6df7701..be00a4337578 100644
--- a/arch/riscv/include/asm/hwcap.h
+++ b/arch/riscv/include/asm/hwcap.h
@@ -60,18 +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];
@@ -79,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)
{
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index adeac90b1d8e..3240a2915bf1 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -28,9 +28,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
*
@@ -243,12 +240,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
--
2.37.2
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: [PATCH v2 02/13] riscv: move riscv_noncoherent_supported() out of ZICBOM probe
2022-12-04 17:46 ` [PATCH v2 02/13] riscv: move riscv_noncoherent_supported() out of ZICBOM probe Jisheng Zhang
@ 2022-12-04 21:52 ` Heiko Stübner
2022-12-05 15:16 ` Jisheng Zhang
0 siblings, 1 reply; 52+ messages in thread
From: Heiko Stübner @ 2022-12-04 21:52 UTC (permalink / raw)
To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Andrew Jones, Jisheng Zhang
Cc: linux-riscv, linux-kernel, kvm, kvm-riscv, Conor Dooley
Am Sonntag, 4. Dezember 2022, 18:46:21 CET schrieb 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.
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> arch/riscv/kernel/cpufeature.c | 1 -
> arch/riscv/kernel/setup.c | 2 ++
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index c743f0adc794..364d1fe86bea 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -274,7 +274,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;
> }
>
> diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> index 86acd690d529..6eea40bf8c6b 100644
> --- a/arch/riscv/kernel/setup.c
> +++ b/arch/riscv/kernel/setup.c
> @@ -300,6 +300,8 @@ void __init setup_arch(char **cmdline_p)
> riscv_init_cbom_blocksize();
> riscv_fill_hwcap();
> apply_boot_alternatives();
> + if (riscv_isa_extension_available(NULL, ZICBOM))
> + riscv_noncoherent_supported();
hmm, this changes the behaviour slightly. In the probe function there
is the
if (!IS_ENABLED(CONFIG_RISCV_ISA_ZICBOM))
return false;
at the top, so with this change the second WARN_TAINT in arch_setup_dma_ops
will behave differently
Heiko
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 09/13] riscv: switch to relative alternative entries
2022-12-04 17:46 ` [PATCH v2 09/13] riscv: switch to relative alternative entries Jisheng Zhang
@ 2022-12-05 0:51 ` Guo Ren
2022-12-05 15:18 ` Jisheng Zhang
0 siblings, 1 reply; 52+ messages in thread
From: Guo Ren @ 2022-12-05 0:51 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
On Mon, Dec 5, 2022 at 1:57 AM Jisheng Zhang <jszhang@kernel.org> wrote:
>
> Instead of using absolute addresses for both the old instrucions and
> the alternative instructions, use offsets relative to the alt_entry
> values. So we can not only cut the size of the alternative entry, but
> also meet the prerequisite for patching alternatives in the vDSO,
> since absolute alternative entries are subject to dynamic relocation,
> which is incompatible with the vDSO building.
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
> arch/riscv/errata/sifive/errata.c | 4 +++-
> arch/riscv/errata/thead/errata.c | 11 ++++++++---
> arch/riscv/include/asm/alternative-macros.h | 20 ++++++++++----------
> arch/riscv/include/asm/alternative.h | 12 ++++++------
> arch/riscv/kernel/cpufeature.c | 13 ++++++-------
> 5 files changed, 33 insertions(+), 27 deletions(-)
>
> diff --git a/arch/riscv/errata/sifive/errata.c b/arch/riscv/errata/sifive/errata.c
> index 1031038423e7..0e537cdfd324 100644
> --- a/arch/riscv/errata/sifive/errata.c
> +++ b/arch/riscv/errata/sifive/errata.c
> @@ -107,7 +107,9 @@ 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((void *)&alt->old_offset + alt->old_offset,
> + (void *)&alt->alt_offset + alt->alt_offset,
(void *)&alt->alt_offset + alt->alt_offset. ??!!
> + alt->alt_len);
> cpu_apply_errata |= tmp;
> }
> }
> diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
> index 21546937db39..2a6e335b5a32 100644
> --- a/arch/riscv/errata/thead/errata.c
> +++ b/arch/riscv/errata/thead/errata.c
> @@ -68,6 +68,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, *updptr;
>
> for (alt = begin; alt < end; alt++) {
> if (alt->vendor_id != THEAD_VENDOR_ID)
> @@ -77,12 +78,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 = (void *)&alt->old_offset + alt->old_offset;
> + updptr = (void *)&alt->alt_offset + alt->alt_offset;
> +
> /* 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(updptr),
> + alt->alt_len);
> else
> - patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> + patch_text_nosync(oldptr, updptr, alt->alt_len);
> }
> }
>
> diff --git a/arch/riscv/include/asm/alternative-macros.h b/arch/riscv/include/asm/alternative-macros.h
> index ec2f3f1b836f..dd40727bc859 100644
> --- a/arch/riscv/include/asm/alternative-macros.h
> +++ b/arch/riscv/include/asm/alternative-macros.h
> @@ -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
> + .long \oldptr - .
> + .long \newptr - .
> + .short \vendor_id
> + .short \new_len
> + .long \errata_id
> .endm
>
> .macro ALT_NEW_CONTENT vendor_id, errata_id, enable = 1, new_c : vararg
> @@ -75,11 +75,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"
> + ".long ((" oldptr ") - .) \n" \
> + ".long ((" newptr ") - .) \n" \
> + ".short " vendor_id "\n" \
> + ".short " newlen "\n" \
> + ".long " errata_id "\n"
>
> #define ALT_NEW_CONTENT(vendor_id, errata_id, enable, new_c) \
> ".if " __stringify(enable) " == 1\n" \
> diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> index 33eae9541684..3baf32e05b46 100644
> --- a/arch/riscv/include/asm/alternative.h
> +++ b/arch/riscv/include/asm/alternative.h
> @@ -33,12 +33,12 @@ void riscv_alternative_fix_jal(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 to original instruciton or data */
> + s32 alt_offset; /* offset 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;
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 6244be5cd94a..adeac90b1d8e 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -257,6 +257,7 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> unsigned int stage)
> {
> struct alt_entry *alt;
> + void *oldptr, *updptr;
>
> if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> return;
> @@ -270,17 +271,15 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> continue;
> }
>
> + oldptr = (void *)&alt->old_offset + alt->old_offset;
> + updptr = (void *)&alt->alt_offset + alt->alt_offset;
> if (!__riscv_isa_extension_available(NULL, alt->errata_id))
> continue;
>
> /* do the basic patching */
> - patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> - riscv_alternative_fix_auipc_jalr(alt->old_ptr,
> - alt->alt_len,
> - alt->old_ptr - alt->alt_ptr);
> - riscv_alternative_fix_jal(alt->old_ptr,
> - alt->alt_len,
> - alt->old_ptr - alt->alt_ptr);
> + patch_text_nosync(oldptr, updptr, alt->alt_len);
> + riscv_alternative_fix_auipc_jalr(oldptr, alt->alt_len, oldptr - updptr);
> + riscv_alternative_fix_jal(oldptr, alt->alt_len, oldptr - updptr);
> }
> }
> #endif
> --
> 2.37.2
>
--
Best Regards
Guo Ren
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 11/13] riscv: cpu_relax: switch to riscv_has_extension_likely()
2022-12-04 17:46 ` [PATCH v2 11/13] riscv: cpu_relax: switch to riscv_has_extension_likely() Jisheng Zhang
@ 2022-12-05 0:52 ` Guo Ren
2022-12-06 22:04 ` Conor Dooley
1 sibling, 0 replies; 52+ messages in thread
From: Guo Ren @ 2022-12-05 0:52 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
Reviewed-by: Guo Ren <guoren@kernel.org>
On Mon, Dec 5, 2022 at 1:57 AM Jisheng Zhang <jszhang@kernel.org> wrote:
>
> Switch cpu_relax() from statich branch to the new helper
> riscv_has_extension_likely()
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> ---
> arch/riscv/include/asm/vdso/processor.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h
> index fa70cfe507aa..edf0e25e43d1 100644
> --- a/arch/riscv/include/asm/vdso/processor.h
> +++ b/arch/riscv/include/asm/vdso/processor.h
> @@ -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. */
> --
> 2.37.2
>
--
Best Regards
Guo Ren
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 12/13] riscv: KVM: Switch has_svinval() to riscv_has_extension_unlikely()
2022-12-04 17:46 ` [PATCH v2 12/13] riscv: KVM: Switch has_svinval() to riscv_has_extension_unlikely() Jisheng Zhang
@ 2022-12-05 0:52 ` Guo Ren
0 siblings, 0 replies; 52+ messages in thread
From: Guo Ren @ 2022-12-05 0:52 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
Reviewed-by: Guo Ren <guoren@kernel.org>
On Mon, Dec 5, 2022 at 1:57 AM Jisheng Zhang <jszhang@kernel.org> wrote:
>
> From: Andrew Jones <ajones@ventanamicro.com>
>
> Switch has_svinval() from static branch to the new helper
> riscv_has_extension_unlikely().
>
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
> arch/riscv/kvm/tlb.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/arch/riscv/kvm/tlb.c b/arch/riscv/kvm/tlb.c
> index 309d79b3e5cd..aa3da18ad873 100644
> --- a/arch/riscv/kvm/tlb.c
> +++ b/arch/riscv/kvm/tlb.c
> @@ -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.37.2
>
--
Best Regards
Guo Ren
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 13/13] riscv: remove riscv_isa_ext_keys[] array and related usage
2022-12-04 17:46 ` [PATCH v2 13/13] riscv: remove riscv_isa_ext_keys[] array and related usage Jisheng Zhang
@ 2022-12-05 0:53 ` Guo Ren
2022-12-06 22:16 ` Conor Dooley
1 sibling, 0 replies; 52+ messages in thread
From: Guo Ren @ 2022-12-05 0:53 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
On Mon, Dec 5, 2022 at 1:57 AM Jisheng Zhang <jszhang@kernel.org> wrote:
>
> All users have switched to riscv_has_extension_*, removed unused
> definitions, vars and related setting code.
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> ---
> arch/riscv/include/asm/hwcap.h | 30 ------------------------------
> arch/riscv/kernel/cpufeature.c | 9 ---------
> 2 files changed, 39 deletions(-)
>
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index e2d3f6df7701..be00a4337578 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -60,18 +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];
> @@ -79,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;
> - }
> -}
> -
Reviewed-by: Guo Ren <guoren@kernel.org>
> static __always_inline bool
> riscv_has_extension_likely(const unsigned long ext)
> {
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index adeac90b1d8e..3240a2915bf1 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -28,9 +28,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
> *
> @@ -243,12 +240,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]);
> - }
Reviewed-by: Guo Ren <guoren@kernel.org>
> }
>
> #ifdef CONFIG_RISCV_ALTERNATIVE
> --
> 2.37.2
>
--
Best Regards
Guo Ren
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 10/13] riscv: alternative: patch alternatives in the vDSO
2022-12-04 17:46 ` [PATCH v2 10/13] riscv: alternative: patch alternatives in the vDSO Jisheng Zhang
@ 2022-12-05 1:56 ` Guo Ren
2022-12-05 15:23 ` Jisheng Zhang
2023-01-11 14:12 ` Andrew Jones
1 sibling, 1 reply; 52+ messages in thread
From: Guo Ren @ 2022-12-05 1:56 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
Are there any patches that depend on it? Any existing utilization? My
first idea is to let __vdso_flush_icache use it, the standard
implementation is so heavy for user space JIT scenario, maybe vendors
could give a custom one.
On Mon, Dec 5, 2022 at 1:57 AM Jisheng Zhang <jszhang@kernel.org> wrote:
>
> Make it possible to use alternatives in the vDSO, so that better
> implementations can be used if possible.
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
> arch/riscv/include/asm/vdso.h | 4 ++++
> arch/riscv/kernel/alternative.c | 25 +++++++++++++++++++++++++
> arch/riscv/kernel/vdso.c | 5 -----
> arch/riscv/kernel/vdso/vdso.lds.S | 7 +++++++
> 4 files changed, 36 insertions(+), 5 deletions(-)
>
> diff --git a/arch/riscv/include/asm/vdso.h b/arch/riscv/include/asm/vdso.h
> index af981426fe0f..b6ff7473fb8a 100644
> --- a/arch/riscv/include/asm/vdso.h
> +++ b/arch/riscv/include/asm/vdso.h
> @@ -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 */
> diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
> index 9d88375624b5..eaf7ddaba54c 100644
> --- a/arch/riscv/kernel/alternative.c
> +++ b/arch/riscv/kernel/alternative.c
> @@ -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>
> @@ -187,6 +189,27 @@ static void __init_or_module _apply_alternatives(struct alt_entry *begin,
> stage);
> }
>
> +static void __init apply_vdso_alternatives(void)
> +{
> + const struct elf64_hdr *hdr;
> + const struct elf64_shdr *shdr;
> + const struct elf64_shdr *alt;
> + struct alt_entry *begin, *end;
> +
> + hdr = (struct elf64_hdr *)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);
> +}
> +
> void __init apply_boot_alternatives(void)
> {
> /* If called on non-boot cpu things could go wrong */
> @@ -195,6 +218,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();
> }
>
> /*
> diff --git a/arch/riscv/kernel/vdso.c b/arch/riscv/kernel/vdso.c
> index 123d05255fcf..1f47bc6566cf 100644
> --- a/arch/riscv/kernel/vdso.c
> +++ b/arch/riscv/kernel/vdso.c
> @@ -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,
> diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S
> index 150b1a572e61..4a0606633290 100644
> --- a/arch/riscv/kernel/vdso/vdso.lds.S
> +++ b/arch/riscv/kernel/vdso/vdso.lds.S
> @@ -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.*)
> --
> 2.37.2
>
--
Best Regards
Guo Ren
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives
2022-12-04 17:46 ` [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives Jisheng Zhang
@ 2022-12-05 14:57 ` Andrew Jones
2022-12-05 15:34 ` Jisheng Zhang
2022-12-05 16:42 ` Jisheng Zhang
2022-12-05 15:31 ` Heiko Stübner
1 sibling, 2 replies; 52+ messages in thread
From: Andrew Jones @ 2022-12-05 14:57 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, linux-riscv, linux-kernel, kvm,
kvm-riscv
On Mon, Dec 05, 2022 at 01:46:20AM +0800, Jisheng Zhang wrote:
> 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.
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
> arch/riscv/include/asm/alternative.h | 2 ++
> arch/riscv/kernel/alternative.c | 38 ++++++++++++++++++++++++++++
> arch/riscv/kernel/cpufeature.c | 3 +++
> 3 files changed, 43 insertions(+)
>
> diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> index c58ec3cc4bc3..33eae9541684 100644
> --- a/arch/riscv/include/asm/alternative.h
> +++ b/arch/riscv/include/asm/alternative.h
> @@ -29,6 +29,8 @@ void apply_module_alternatives(void *start, size_t length);
>
> void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
> int patch_offset);
> +void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
> + int patch_offset);
>
> struct alt_entry {
> void *old_ptr; /* address of original instruciton or data */
> diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
> index 292cc42dc3be..9d88375624b5 100644
> --- a/arch/riscv/kernel/alternative.c
> +++ b/arch/riscv/kernel/alternative.c
> @@ -125,6 +125,44 @@ void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
> }
> }
>
> +#define to_jal_imm(value) \
> + (((value & (RV_J_IMM_10_1_MASK << RV_J_IMM_10_1_OFF)) << RV_I_IMM_11_0_OPOFF) | \
^ RV_J_IMM_10_1_OPOFF
> + ((value & (RV_J_IMM_11_MASK << RV_J_IMM_11_OFF)) << RV_J_IMM_11_OPOFF) | \
> + ((value & (RV_J_IMM_19_12_OPOFF << RV_J_IMM_19_12_OFF)) << RV_J_IMM_19_12_OPOFF) | \
> + ((value & (1 << RV_J_IMM_SIGN_OFF)) << RV_J_IMM_SIGN_OPOFF))
Should put () around value
> +
> +void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
> + int patch_offset)
> +{
> + int num_instr = len / sizeof(u32);
> + unsigned int call;
> + int i;
> + int imm;
> +
> + for (i = 0; i < num_instr; i++) {
> + u32 inst = riscv_instruction_at(alt_ptr, i);
> +
> + if (!riscv_insn_is_jal(inst))
> + continue;
> +
> + /* get and adjust new target address */
> + imm = RV_EXTRACT_JTYPE_IMM(inst);
> + imm -= patch_offset;
> +
> + /* pick the original jal */
> + call = inst;
> +
> + /* drop the old IMMs, all jal imm bits sit at 31:12 */
> + call &= ~GENMASK(31, 12);
It'd be nice if this had a define.
> +
> + /* add the adapted IMMs */
> + call |= to_jal_imm(imm);
> +
> + /* patch the call place again */
> + patch_text_nosync(alt_ptr + i * sizeof(u32), &call, 4);
> + }
> +}
> +
> /*
> * This is called very early in the boot process (directly after we run
> * a feature detect on the boot CPU). No need to worry about other CPUs
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index ba62a4ff5ccd..c743f0adc794 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -324,6 +324,9 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> riscv_alternative_fix_auipc_jalr(alt->old_ptr,
> alt->alt_len,
> alt->old_ptr - alt->alt_ptr);
> + riscv_alternative_fix_jal(alt->old_ptr,
> + alt->alt_len,
> + alt->old_ptr - alt->alt_ptr);
> }
> }
> }
> --
> 2.37.2
>
Thanks,
drew
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 02/13] riscv: move riscv_noncoherent_supported() out of ZICBOM probe
2022-12-04 21:52 ` Heiko Stübner
@ 2022-12-05 15:16 ` Jisheng Zhang
2022-12-05 15:31 ` Conor Dooley
0 siblings, 1 reply; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-05 15:16 UTC (permalink / raw)
To: Heiko Stübner
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Andrew Jones, linux-riscv, linux-kernel, kvm,
kvm-riscv, Conor Dooley
On Sun, Dec 04, 2022 at 10:52:03PM +0100, Heiko Stübner wrote:
> Am Sonntag, 4. Dezember 2022, 18:46:21 CET schrieb 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.
> >
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> > Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
> > ---
> > arch/riscv/kernel/cpufeature.c | 1 -
> > arch/riscv/kernel/setup.c | 2 ++
> > 2 files changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index c743f0adc794..364d1fe86bea 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -274,7 +274,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;
> > }
> >
> > diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
> > index 86acd690d529..6eea40bf8c6b 100644
> > --- a/arch/riscv/kernel/setup.c
> > +++ b/arch/riscv/kernel/setup.c
> > @@ -300,6 +300,8 @@ void __init setup_arch(char **cmdline_p)
> > riscv_init_cbom_blocksize();
> > riscv_fill_hwcap();
> > apply_boot_alternatives();
> > + if (riscv_isa_extension_available(NULL, ZICBOM))
> > + riscv_noncoherent_supported();
>
> hmm, this changes the behaviour slightly. In the probe function there
> is the
> if (!IS_ENABLED(CONFIG_RISCV_ISA_ZICBOM))
> return false;
> at the top, so with this change the second WARN_TAINT in arch_setup_dma_ops
> will behave differently
thanks for the information. below code can keep the behavior:
if (IS_ENABLED(CONFIG_RISCV_ISA_ZICBOM) &&
riscv_isa_extension_available(NULL, ZICBOM))
riscv_noncoherent_supported();
will wait for one more day for more review comments, then will send out
a v3
>
> Heiko
>
>
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 09/13] riscv: switch to relative alternative entries
2022-12-05 0:51 ` Guo Ren
@ 2022-12-05 15:18 ` Jisheng Zhang
2022-12-06 4:34 ` Guo Ren
0 siblings, 1 reply; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-05 15:18 UTC (permalink / raw)
To: Guo Ren
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
On Mon, Dec 05, 2022 at 08:51:41AM +0800, Guo Ren wrote:
> On Mon, Dec 5, 2022 at 1:57 AM Jisheng Zhang <jszhang@kernel.org> wrote:
> >
> > Instead of using absolute addresses for both the old instrucions and
> > the alternative instructions, use offsets relative to the alt_entry
> > values. So we can not only cut the size of the alternative entry, but
> > also meet the prerequisite for patching alternatives in the vDSO,
> > since absolute alternative entries are subject to dynamic relocation,
> > which is incompatible with the vDSO building.
> >
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > ---
> > arch/riscv/errata/sifive/errata.c | 4 +++-
> > arch/riscv/errata/thead/errata.c | 11 ++++++++---
> > arch/riscv/include/asm/alternative-macros.h | 20 ++++++++++----------
> > arch/riscv/include/asm/alternative.h | 12 ++++++------
> > arch/riscv/kernel/cpufeature.c | 13 ++++++-------
> > 5 files changed, 33 insertions(+), 27 deletions(-)
> >
> > diff --git a/arch/riscv/errata/sifive/errata.c b/arch/riscv/errata/sifive/errata.c
> > index 1031038423e7..0e537cdfd324 100644
> > --- a/arch/riscv/errata/sifive/errata.c
> > +++ b/arch/riscv/errata/sifive/errata.c
> > @@ -107,7 +107,9 @@ 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((void *)&alt->old_offset + alt->old_offset,
> > + (void *)&alt->alt_offset + alt->alt_offset,
> (void *)&alt->alt_offset + alt->alt_offset. ??!!
Hi Guo,
what's the problem? I can't catch your meaning, could you please proide
more details?
Thanks
>
> > + alt->alt_len);
> > cpu_apply_errata |= tmp;
> > }
> > }
> > diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
> > index 21546937db39..2a6e335b5a32 100644
> > --- a/arch/riscv/errata/thead/errata.c
> > +++ b/arch/riscv/errata/thead/errata.c
> > @@ -68,6 +68,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, *updptr;
> >
> > for (alt = begin; alt < end; alt++) {
> > if (alt->vendor_id != THEAD_VENDOR_ID)
> > @@ -77,12 +78,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 = (void *)&alt->old_offset + alt->old_offset;
> > + updptr = (void *)&alt->alt_offset + alt->alt_offset;
> > +
> > /* 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(updptr),
> > + alt->alt_len);
> > else
> > - patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> > + patch_text_nosync(oldptr, updptr, alt->alt_len);
> > }
> > }
> >
> > diff --git a/arch/riscv/include/asm/alternative-macros.h b/arch/riscv/include/asm/alternative-macros.h
> > index ec2f3f1b836f..dd40727bc859 100644
> > --- a/arch/riscv/include/asm/alternative-macros.h
> > +++ b/arch/riscv/include/asm/alternative-macros.h
> > @@ -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
> > + .long \oldptr - .
> > + .long \newptr - .
> > + .short \vendor_id
> > + .short \new_len
> > + .long \errata_id
> > .endm
> >
> > .macro ALT_NEW_CONTENT vendor_id, errata_id, enable = 1, new_c : vararg
> > @@ -75,11 +75,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"
> > + ".long ((" oldptr ") - .) \n" \
> > + ".long ((" newptr ") - .) \n" \
> > + ".short " vendor_id "\n" \
> > + ".short " newlen "\n" \
> > + ".long " errata_id "\n"
> >
> > #define ALT_NEW_CONTENT(vendor_id, errata_id, enable, new_c) \
> > ".if " __stringify(enable) " == 1\n" \
> > diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> > index 33eae9541684..3baf32e05b46 100644
> > --- a/arch/riscv/include/asm/alternative.h
> > +++ b/arch/riscv/include/asm/alternative.h
> > @@ -33,12 +33,12 @@ void riscv_alternative_fix_jal(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 to original instruciton or data */
> > + s32 alt_offset; /* offset 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;
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index 6244be5cd94a..adeac90b1d8e 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -257,6 +257,7 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> > unsigned int stage)
> > {
> > struct alt_entry *alt;
> > + void *oldptr, *updptr;
> >
> > if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> > return;
> > @@ -270,17 +271,15 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> > continue;
> > }
> >
> > + oldptr = (void *)&alt->old_offset + alt->old_offset;
> > + updptr = (void *)&alt->alt_offset + alt->alt_offset;
> > if (!__riscv_isa_extension_available(NULL, alt->errata_id))
> > continue;
> >
> > /* do the basic patching */
> > - patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> > - riscv_alternative_fix_auipc_jalr(alt->old_ptr,
> > - alt->alt_len,
> > - alt->old_ptr - alt->alt_ptr);
> > - riscv_alternative_fix_jal(alt->old_ptr,
> > - alt->alt_len,
> > - alt->old_ptr - alt->alt_ptr);
> > + patch_text_nosync(oldptr, updptr, alt->alt_len);
> > + riscv_alternative_fix_auipc_jalr(oldptr, alt->alt_len, oldptr - updptr);
> > + riscv_alternative_fix_jal(oldptr, alt->alt_len, oldptr - updptr);
> > }
> > }
> > #endif
> > --
> > 2.37.2
> >
>
>
> --
> Best Regards
> Guo Ren
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 10/13] riscv: alternative: patch alternatives in the vDSO
2022-12-05 1:56 ` Guo Ren
@ 2022-12-05 15:23 ` Jisheng Zhang
2022-12-06 4:29 ` Guo Ren
0 siblings, 1 reply; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-05 15:23 UTC (permalink / raw)
To: Guo Ren
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
On Mon, Dec 05, 2022 at 09:56:37AM +0800, Guo Ren wrote:
> Are there any patches that depend on it? Any existing utilization? My
> first idea is to let __vdso_flush_icache use it, the standard
> implementation is so heavy for user space JIT scenario, maybe vendors
> could give a custom one.
Hi Guo,
the 11th patch of swtiching cpu_relax() to riscv_has_extension_likely()
depends on this patch, the gettimeofday implementation calls cpu_relax()
which may use ZIHINTPAUSE extension, so we need to patch vDSO.
Thanks
>
> On Mon, Dec 5, 2022 at 1:57 AM Jisheng Zhang <jszhang@kernel.org> wrote:
> >
> > Make it possible to use alternatives in the vDSO, so that better
> > implementations can be used if possible.
> >
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > ---
> > arch/riscv/include/asm/vdso.h | 4 ++++
> > arch/riscv/kernel/alternative.c | 25 +++++++++++++++++++++++++
> > arch/riscv/kernel/vdso.c | 5 -----
> > arch/riscv/kernel/vdso/vdso.lds.S | 7 +++++++
> > 4 files changed, 36 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/vdso.h b/arch/riscv/include/asm/vdso.h
> > index af981426fe0f..b6ff7473fb8a 100644
> > --- a/arch/riscv/include/asm/vdso.h
> > +++ b/arch/riscv/include/asm/vdso.h
> > @@ -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 */
> > diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
> > index 9d88375624b5..eaf7ddaba54c 100644
> > --- a/arch/riscv/kernel/alternative.c
> > +++ b/arch/riscv/kernel/alternative.c
> > @@ -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>
> > @@ -187,6 +189,27 @@ static void __init_or_module _apply_alternatives(struct alt_entry *begin,
> > stage);
> > }
> >
> > +static void __init apply_vdso_alternatives(void)
> > +{
> > + const struct elf64_hdr *hdr;
> > + const struct elf64_shdr *shdr;
> > + const struct elf64_shdr *alt;
> > + struct alt_entry *begin, *end;
> > +
> > + hdr = (struct elf64_hdr *)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);
> > +}
> > +
> > void __init apply_boot_alternatives(void)
> > {
> > /* If called on non-boot cpu things could go wrong */
> > @@ -195,6 +218,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();
> > }
> >
> > /*
> > diff --git a/arch/riscv/kernel/vdso.c b/arch/riscv/kernel/vdso.c
> > index 123d05255fcf..1f47bc6566cf 100644
> > --- a/arch/riscv/kernel/vdso.c
> > +++ b/arch/riscv/kernel/vdso.c
> > @@ -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,
> > diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S
> > index 150b1a572e61..4a0606633290 100644
> > --- a/arch/riscv/kernel/vdso/vdso.lds.S
> > +++ b/arch/riscv/kernel/vdso/vdso.lds.S
> > @@ -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.*)
> > --
> > 2.37.2
> >
>
>
> --
> Best Regards
> Guo Ren
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 08/13] riscv: module: move find_section to module.h
2022-12-04 17:46 ` [PATCH v2 08/13] riscv: module: move find_section to module.h Jisheng Zhang
@ 2022-12-05 15:25 ` Andrew Jones
2022-12-06 20:44 ` Conor Dooley
1 sibling, 0 replies; 52+ messages in thread
From: Andrew Jones @ 2022-12-05 15:25 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, linux-riscv, linux-kernel, kvm,
kvm-riscv
On Mon, Dec 05, 2022 at 01:46:27AM +0800, Jisheng Zhang wrote:
> Move it to the header so that the implementation can be shared
> by the alternatives code.
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
> arch/riscv/include/asm/module.h | 15 +++++++++++++++
> arch/riscv/kernel/module.c | 15 ---------------
> 2 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/arch/riscv/include/asm/module.h b/arch/riscv/include/asm/module.h
> index 76aa96a9fc08..78722a79fc59 100644
> --- a/arch/riscv/include/asm/module.h
> +++ b/arch/riscv/include/asm/module.h
> @@ -111,4 +111,19 @@ static inline struct plt_entry *get_plt_entry(unsigned long val,
>
> #endif /* CONFIG_MODULE_SECTIONS */
Should probably add an explicit #include <linux/elf.h>
Otherwise
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Thanks,
drew
>
> +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 */
> diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
> index 91fe16bfaa07..76f4b9c2ec5b 100644
> --- a/arch/riscv/kernel/module.c
> +++ b/arch/riscv/kernel/module.c
> @@ -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)
> --
> 2.37.2
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives
2022-12-04 17:46 ` [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives Jisheng Zhang
2022-12-05 14:57 ` Andrew Jones
@ 2022-12-05 15:31 ` Heiko Stübner
2022-12-05 15:40 ` Jisheng Zhang
1 sibling, 1 reply; 52+ messages in thread
From: Heiko Stübner @ 2022-12-05 15:31 UTC (permalink / raw)
To: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Andrew Jones, Jisheng Zhang
Cc: linux-riscv, linux-kernel, kvm, kvm-riscv
Am Sonntag, 4. Dezember 2022, 18:46:20 CET schrieb Jisheng Zhang:
> 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.
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
> arch/riscv/include/asm/alternative.h | 2 ++
> arch/riscv/kernel/alternative.c | 38 ++++++++++++++++++++++++++++
> arch/riscv/kernel/cpufeature.c | 3 +++
> 3 files changed, 43 insertions(+)
>
> diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> index c58ec3cc4bc3..33eae9541684 100644
> --- a/arch/riscv/include/asm/alternative.h
> +++ b/arch/riscv/include/asm/alternative.h
> @@ -29,6 +29,8 @@ void apply_module_alternatives(void *start, size_t length);
>
> void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
> int patch_offset);
> +void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
> + int patch_offset);
>
> struct alt_entry {
> void *old_ptr; /* address of original instruciton or data */
> diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
> index 292cc42dc3be..9d88375624b5 100644
> --- a/arch/riscv/kernel/alternative.c
> +++ b/arch/riscv/kernel/alternative.c
> @@ -125,6 +125,44 @@ void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
> }
> }
>
> +#define to_jal_imm(value) \
> + (((value & (RV_J_IMM_10_1_MASK << RV_J_IMM_10_1_OFF)) << RV_I_IMM_11_0_OPOFF) | \
> + ((value & (RV_J_IMM_11_MASK << RV_J_IMM_11_OFF)) << RV_J_IMM_11_OPOFF) | \
> + ((value & (RV_J_IMM_19_12_OPOFF << RV_J_IMM_19_12_OFF)) << RV_J_IMM_19_12_OPOFF) | \
> + ((value & (1 << RV_J_IMM_SIGN_OFF)) << RV_J_IMM_SIGN_OPOFF))
> +
> +void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
> + int patch_offset)
> +{
I think we might want to unfiy this into a common function like
riscv_alternative_fix_offsets(...)
so that we only run through the code block once
for (i = 0; i < num_instr; i++) {
if (riscv_insn_is_auipc_jalr(inst1, inst2)) {
riscv_alternative_fix_auipc_jalr(...)
continue;
}
if (riscv_insn_is_jal(inst)) {
riscv_alternative_fix_jal(...)
continue;
}
}
This would also remove the need from calling multiple functions
after patching alternatives.
Thoughts?
Heiko
> + int num_instr = len / sizeof(u32);
> + unsigned int call;
> + int i;
> + int imm;
> +
> + for (i = 0; i < num_instr; i++) {
> + u32 inst = riscv_instruction_at(alt_ptr, i);
> +
> + if (!riscv_insn_is_jal(inst))
> + continue;
> +
> + /* get and adjust new target address */
> + imm = RV_EXTRACT_JTYPE_IMM(inst);
> + imm -= patch_offset;
> +
> + /* pick the original jal */
> + call = inst;
> +
> + /* drop the old IMMs, all jal imm bits sit at 31:12 */
> + call &= ~GENMASK(31, 12);
> +
> + /* add the adapted IMMs */
> + call |= to_jal_imm(imm);
> +
> + /* patch the call place again */
> + patch_text_nosync(alt_ptr + i * sizeof(u32), &call, 4);
> + }
> +}
> +
> /*
> * This is called very early in the boot process (directly after we run
> * a feature detect on the boot CPU). No need to worry about other CPUs
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index ba62a4ff5ccd..c743f0adc794 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -324,6 +324,9 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> riscv_alternative_fix_auipc_jalr(alt->old_ptr,
> alt->alt_len,
> alt->old_ptr - alt->alt_ptr);
> + riscv_alternative_fix_jal(alt->old_ptr,
> + alt->alt_len,
> + alt->old_ptr - alt->alt_ptr);
> }
> }
> }
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 02/13] riscv: move riscv_noncoherent_supported() out of ZICBOM probe
2022-12-05 15:16 ` Jisheng Zhang
@ 2022-12-05 15:31 ` Conor Dooley
0 siblings, 0 replies; 52+ messages in thread
From: Conor Dooley @ 2022-12-05 15:31 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Heiko Stübner, Palmer Dabbelt, Paul Walmsley, Albert Ou,
Anup Patel, Atish Patra, Andrew Jones, linux-riscv, linux-kernel,
kvm, kvm-riscv
[-- Attachment #1: Type: text/plain, Size: 298 bytes --]
On Mon, Dec 05, 2022 at 11:16:30PM +0800, Jisheng Zhang wrote:
> will wait for one more day for more review comments, then will send out
> a v3
AFAICT, the patches were only sent yesterday? It'd be nice if you could
give us more than 2 days between versions of something please :/
Thanks
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives
2022-12-05 14:57 ` Andrew Jones
@ 2022-12-05 15:34 ` Jisheng Zhang
2022-12-05 16:42 ` Jisheng Zhang
1 sibling, 0 replies; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-05 15:34 UTC (permalink / raw)
To: Andrew Jones
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, linux-riscv, linux-kernel, kvm,
kvm-riscv
On Mon, Dec 05, 2022 at 03:57:10PM +0100, Andrew Jones wrote:
> On Mon, Dec 05, 2022 at 01:46:20AM +0800, Jisheng Zhang wrote:
> > 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.
> >
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > ---
> > arch/riscv/include/asm/alternative.h | 2 ++
> > arch/riscv/kernel/alternative.c | 38 ++++++++++++++++++++++++++++
> > arch/riscv/kernel/cpufeature.c | 3 +++
> > 3 files changed, 43 insertions(+)
> >
> > diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> > index c58ec3cc4bc3..33eae9541684 100644
> > --- a/arch/riscv/include/asm/alternative.h
> > +++ b/arch/riscv/include/asm/alternative.h
> > @@ -29,6 +29,8 @@ void apply_module_alternatives(void *start, size_t length);
> >
> > void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
> > int patch_offset);
> > +void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
> > + int patch_offset);
> >
> > struct alt_entry {
> > void *old_ptr; /* address of original instruciton or data */
> > diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
> > index 292cc42dc3be..9d88375624b5 100644
> > --- a/arch/riscv/kernel/alternative.c
> > +++ b/arch/riscv/kernel/alternative.c
> > @@ -125,6 +125,44 @@ void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
> > }
> > }
> >
> > +#define to_jal_imm(value) \
> > + (((value & (RV_J_IMM_10_1_MASK << RV_J_IMM_10_1_OFF)) << RV_I_IMM_11_0_OPOFF) | \
> ^ RV_J_IMM_10_1_OPOFF
Good catch! I was lucky when testing the whole series ;) will fix it in
newer version.
>
> > + ((value & (RV_J_IMM_11_MASK << RV_J_IMM_11_OFF)) << RV_J_IMM_11_OPOFF) | \
> > + ((value & (RV_J_IMM_19_12_OPOFF << RV_J_IMM_19_12_OFF)) << RV_J_IMM_19_12_OPOFF) | \
> > + ((value & (1 << RV_J_IMM_SIGN_OFF)) << RV_J_IMM_SIGN_OPOFF))
>
> Should put () around value
good idea, previously, I just follow to_jalr_imm(), will update it in
newer version.
>
> > +
> > +void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
> > + int patch_offset)
> > +{
> > + int num_instr = len / sizeof(u32);
> > + unsigned int call;
> > + int i;
> > + int imm;
> > +
> > + for (i = 0; i < num_instr; i++) {
> > + u32 inst = riscv_instruction_at(alt_ptr, i);
> > +
> > + if (!riscv_insn_is_jal(inst))
> > + continue;
> > +
> > + /* get and adjust new target address */
> > + imm = RV_EXTRACT_JTYPE_IMM(inst);
> > + imm -= patch_offset;
> > +
> > + /* pick the original jal */
> > + call = inst;
> > +
> > + /* drop the old IMMs, all jal imm bits sit at 31:12 */
> > + call &= ~GENMASK(31, 12);
>
> It'd be nice if this had a define.
>
> > +
> > + /* add the adapted IMMs */
> > + call |= to_jal_imm(imm);
> > +
> > + /* patch the call place again */
> > + patch_text_nosync(alt_ptr + i * sizeof(u32), &call, 4);
> > + }
> > +}
> > +
> > /*
> > * This is called very early in the boot process (directly after we run
> > * a feature detect on the boot CPU). No need to worry about other CPUs
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index ba62a4ff5ccd..c743f0adc794 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -324,6 +324,9 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> > riscv_alternative_fix_auipc_jalr(alt->old_ptr,
> > alt->alt_len,
> > alt->old_ptr - alt->alt_ptr);
> > + riscv_alternative_fix_jal(alt->old_ptr,
> > + alt->alt_len,
> > + alt->old_ptr - alt->alt_ptr);
> > }
> > }
> > }
> > --
> > 2.37.2
> >
>
> Thanks,
> drew
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives
2022-12-05 15:31 ` Heiko Stübner
@ 2022-12-05 15:40 ` Jisheng Zhang
2022-12-05 18:36 ` Conor Dooley
0 siblings, 1 reply; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-05 15:40 UTC (permalink / raw)
To: Heiko Stübner
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Andrew Jones, linux-riscv, linux-kernel, kvm,
kvm-riscv
On Mon, Dec 05, 2022 at 04:31:08PM +0100, Heiko Stübner wrote:
> Am Sonntag, 4. Dezember 2022, 18:46:20 CET schrieb Jisheng Zhang:
> > 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.
> >
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > ---
> > arch/riscv/include/asm/alternative.h | 2 ++
> > arch/riscv/kernel/alternative.c | 38 ++++++++++++++++++++++++++++
> > arch/riscv/kernel/cpufeature.c | 3 +++
> > 3 files changed, 43 insertions(+)
> >
> > diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> > index c58ec3cc4bc3..33eae9541684 100644
> > --- a/arch/riscv/include/asm/alternative.h
> > +++ b/arch/riscv/include/asm/alternative.h
> > @@ -29,6 +29,8 @@ void apply_module_alternatives(void *start, size_t length);
> >
> > void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
> > int patch_offset);
> > +void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
> > + int patch_offset);
> >
> > struct alt_entry {
> > void *old_ptr; /* address of original instruciton or data */
> > diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
> > index 292cc42dc3be..9d88375624b5 100644
> > --- a/arch/riscv/kernel/alternative.c
> > +++ b/arch/riscv/kernel/alternative.c
> > @@ -125,6 +125,44 @@ void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
> > }
> > }
> >
> > +#define to_jal_imm(value) \
> > + (((value & (RV_J_IMM_10_1_MASK << RV_J_IMM_10_1_OFF)) << RV_I_IMM_11_0_OPOFF) | \
> > + ((value & (RV_J_IMM_11_MASK << RV_J_IMM_11_OFF)) << RV_J_IMM_11_OPOFF) | \
> > + ((value & (RV_J_IMM_19_12_OPOFF << RV_J_IMM_19_12_OFF)) << RV_J_IMM_19_12_OPOFF) | \
> > + ((value & (1 << RV_J_IMM_SIGN_OFF)) << RV_J_IMM_SIGN_OPOFF))
> > +
> > +void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
> > + int patch_offset)
> > +{
>
> I think we might want to unfiy this into a common function like
>
> riscv_alternative_fix_offsets(...)
>
> so that we only run through the code block once
>
> for (i = 0; i < num_instr; i++) {
> if (riscv_insn_is_auipc_jalr(inst1, inst2)) {
> riscv_alternative_fix_auipc_jalr(...)
> continue;
> }
>
> if (riscv_insn_is_jal(inst)) {
> riscv_alternative_fix_jal(...)
> continue;
> }
> }
>
> This would also remove the need from calling multiple functions
> after patching alternatives.
Yesterday, I also wanted to unify the two instruction fix into
one. But that would need to roll back the
riscv_alternative_fix_auipc_jalr() to your v1 version. And IMHO,
it's better if you can split the Zbb string optimizations series
into two: one for alternative improvements, another for Zbb. Then
we may get the alternative improvements and this inst extension
series merged in v6.2-rc1.
>
> Thoughts?
>
>
> Heiko
>
> > + int num_instr = len / sizeof(u32);
> > + unsigned int call;
> > + int i;
> > + int imm;
> > +
> > + for (i = 0; i < num_instr; i++) {
> > + u32 inst = riscv_instruction_at(alt_ptr, i);
> > +
> > + if (!riscv_insn_is_jal(inst))
> > + continue;
> > +
> > + /* get and adjust new target address */
> > + imm = RV_EXTRACT_JTYPE_IMM(inst);
> > + imm -= patch_offset;
> > +
> > + /* pick the original jal */
> > + call = inst;
> > +
> > + /* drop the old IMMs, all jal imm bits sit at 31:12 */
> > + call &= ~GENMASK(31, 12);
> > +
> > + /* add the adapted IMMs */
> > + call |= to_jal_imm(imm);
> > +
> > + /* patch the call place again */
> > + patch_text_nosync(alt_ptr + i * sizeof(u32), &call, 4);
> > + }
> > +}
> > +
> > /*
> > * This is called very early in the boot process (directly after we run
> > * a feature detect on the boot CPU). No need to worry about other CPUs
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index ba62a4ff5ccd..c743f0adc794 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -324,6 +324,9 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> > riscv_alternative_fix_auipc_jalr(alt->old_ptr,
> > alt->alt_len,
> > alt->old_ptr - alt->alt_ptr);
> > + riscv_alternative_fix_jal(alt->old_ptr,
> > + alt->alt_len,
> > + alt->old_ptr - alt->alt_ptr);
> > }
> > }
> > }
> >
>
>
>
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives
2022-12-05 14:57 ` Andrew Jones
2022-12-05 15:34 ` Jisheng Zhang
@ 2022-12-05 16:42 ` Jisheng Zhang
2022-12-05 16:49 ` Jisheng Zhang
1 sibling, 1 reply; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-05 16:42 UTC (permalink / raw)
To: Andrew Jones
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, linux-riscv, linux-kernel, kvm,
kvm-riscv
On Mon, Dec 05, 2022 at 03:57:10PM +0100, Andrew Jones wrote:
> On Mon, Dec 05, 2022 at 01:46:20AM +0800, Jisheng Zhang wrote:
> > 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.
> >
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > ---
> > arch/riscv/include/asm/alternative.h | 2 ++
> > arch/riscv/kernel/alternative.c | 38 ++++++++++++++++++++++++++++
> > arch/riscv/kernel/cpufeature.c | 3 +++
> > 3 files changed, 43 insertions(+)
> >
> > diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> > index c58ec3cc4bc3..33eae9541684 100644
> > --- a/arch/riscv/include/asm/alternative.h
> > +++ b/arch/riscv/include/asm/alternative.h
> > @@ -29,6 +29,8 @@ void apply_module_alternatives(void *start, size_t length);
> >
> > void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
> > int patch_offset);
> > +void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
> > + int patch_offset);
> >
> > struct alt_entry {
> > void *old_ptr; /* address of original instruciton or data */
> > diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
> > index 292cc42dc3be..9d88375624b5 100644
> > --- a/arch/riscv/kernel/alternative.c
> > +++ b/arch/riscv/kernel/alternative.c
> > @@ -125,6 +125,44 @@ void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
> > }
> > }
> >
> > +#define to_jal_imm(value) \
> > + (((value & (RV_J_IMM_10_1_MASK << RV_J_IMM_10_1_OFF)) << RV_I_IMM_11_0_OPOFF) | \
> ^ RV_J_IMM_10_1_OPOFF
>
> > + ((value & (RV_J_IMM_11_MASK << RV_J_IMM_11_OFF)) << RV_J_IMM_11_OPOFF) | \
> > + ((value & (RV_J_IMM_19_12_OPOFF << RV_J_IMM_19_12_OFF)) << RV_J_IMM_19_12_OPOFF) | \
> > + ((value & (1 << RV_J_IMM_SIGN_OFF)) << RV_J_IMM_SIGN_OPOFF))
Hi all,
I believe there's bug in the to_jal_imm() macro implementation, the
correct one should be like this:
#define to_jal_imm(value) \
((RV_X(value, RV_J_IMM_10_1_OFF, RV_J_IMM_10_1_MASK) << RV_J_IMM_10_1_OPOFF) | \
(RV_X(value, RV_J_IMM_11_OFF, RV_J_IMM_11_MASK) << RV_J_IMM_11_OPOFF) | \
(RV_X(value, RV_J_IMM_19_12_OFF, RV_J_IMM_19_12_MASK) << RV_J_IMM_19_12_OPOFF) | \
(RV_X(value, RV_J_IMM_SIGN_OFF, 1) << RV_J_IMM_SIGN_OPOFF))
Will fix it in next version.
Thanks
>
> Should put () around value
>
> > +
> > +void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
> > + int patch_offset)
> > +{
> > + int num_instr = len / sizeof(u32);
> > + unsigned int call;
> > + int i;
> > + int imm;
> > +
> > + for (i = 0; i < num_instr; i++) {
> > + u32 inst = riscv_instruction_at(alt_ptr, i);
> > +
> > + if (!riscv_insn_is_jal(inst))
> > + continue;
> > +
> > + /* get and adjust new target address */
> > + imm = RV_EXTRACT_JTYPE_IMM(inst);
> > + imm -= patch_offset;
> > +
> > + /* pick the original jal */
> > + call = inst;
> > +
> > + /* drop the old IMMs, all jal imm bits sit at 31:12 */
> > + call &= ~GENMASK(31, 12);
>
> It'd be nice if this had a define.
>
> > +
> > + /* add the adapted IMMs */
> > + call |= to_jal_imm(imm);
> > +
> > + /* patch the call place again */
> > + patch_text_nosync(alt_ptr + i * sizeof(u32), &call, 4);
> > + }
> > +}
> > +
> > /*
> > * This is called very early in the boot process (directly after we run
> > * a feature detect on the boot CPU). No need to worry about other CPUs
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index ba62a4ff5ccd..c743f0adc794 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -324,6 +324,9 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> > riscv_alternative_fix_auipc_jalr(alt->old_ptr,
> > alt->alt_len,
> > alt->old_ptr - alt->alt_ptr);
> > + riscv_alternative_fix_jal(alt->old_ptr,
> > + alt->alt_len,
> > + alt->old_ptr - alt->alt_ptr);
> > }
> > }
> > }
> > --
> > 2.37.2
> >
>
> Thanks,
> drew
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives
2022-12-05 16:42 ` Jisheng Zhang
@ 2022-12-05 16:49 ` Jisheng Zhang
2022-12-06 5:50 ` Andrew Jones
0 siblings, 1 reply; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-05 16:49 UTC (permalink / raw)
To: Andrew Jones
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, linux-riscv, linux-kernel, kvm,
kvm-riscv
On Tue, Dec 06, 2022 at 12:42:16AM +0800, Jisheng Zhang wrote:
> On Mon, Dec 05, 2022 at 03:57:10PM +0100, Andrew Jones wrote:
> > On Mon, Dec 05, 2022 at 01:46:20AM +0800, Jisheng Zhang wrote:
> > > 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.
> > >
> > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > > ---
> > > arch/riscv/include/asm/alternative.h | 2 ++
> > > arch/riscv/kernel/alternative.c | 38 ++++++++++++++++++++++++++++
> > > arch/riscv/kernel/cpufeature.c | 3 +++
> > > 3 files changed, 43 insertions(+)
> > >
> > > diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> > > index c58ec3cc4bc3..33eae9541684 100644
> > > --- a/arch/riscv/include/asm/alternative.h
> > > +++ b/arch/riscv/include/asm/alternative.h
> > > @@ -29,6 +29,8 @@ void apply_module_alternatives(void *start, size_t length);
> > >
> > > void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
> > > int patch_offset);
> > > +void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
> > > + int patch_offset);
> > >
> > > struct alt_entry {
> > > void *old_ptr; /* address of original instruciton or data */
> > > diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
> > > index 292cc42dc3be..9d88375624b5 100644
> > > --- a/arch/riscv/kernel/alternative.c
> > > +++ b/arch/riscv/kernel/alternative.c
> > > @@ -125,6 +125,44 @@ void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
> > > }
> > > }
> > >
> > > +#define to_jal_imm(value) \
> > > + (((value & (RV_J_IMM_10_1_MASK << RV_J_IMM_10_1_OFF)) << RV_I_IMM_11_0_OPOFF) | \
> > ^ RV_J_IMM_10_1_OPOFF
> >
> > > + ((value & (RV_J_IMM_11_MASK << RV_J_IMM_11_OFF)) << RV_J_IMM_11_OPOFF) | \
> > > + ((value & (RV_J_IMM_19_12_OPOFF << RV_J_IMM_19_12_OFF)) << RV_J_IMM_19_12_OPOFF) | \
> > > + ((value & (1 << RV_J_IMM_SIGN_OFF)) << RV_J_IMM_SIGN_OPOFF))
>
> Hi all,
>
> I believe there's bug in the to_jal_imm() macro implementation, the
> correct one should be like this:
>
> #define to_jal_imm(value) \
> ((RV_X(value, RV_J_IMM_10_1_OFF, RV_J_IMM_10_1_MASK) << RV_J_IMM_10_1_OPOFF) | \
> (RV_X(value, RV_J_IMM_11_OFF, RV_J_IMM_11_MASK) << RV_J_IMM_11_OPOFF) | \
> (RV_X(value, RV_J_IMM_19_12_OFF, RV_J_IMM_19_12_MASK) << RV_J_IMM_19_12_OPOFF) | \
> (RV_X(value, RV_J_IMM_SIGN_OFF, 1) << RV_J_IMM_SIGN_OPOFF))
And I just tested to_jal_imm() vs RV_EXTRACT_JTYPE_IMM(), they match perfectly.
E.g:
RV_EXTRACT_JTYPE_IMM(to_jal_imm(imm)) == imm is alway true when imm is a jal
valid offset.
>
> Will fix it in next version.
>
> Thanks
> >
> > Should put () around value
> >
> > > +
> > > +void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
> > > + int patch_offset)
> > > +{
> > > + int num_instr = len / sizeof(u32);
> > > + unsigned int call;
> > > + int i;
> > > + int imm;
> > > +
> > > + for (i = 0; i < num_instr; i++) {
> > > + u32 inst = riscv_instruction_at(alt_ptr, i);
> > > +
> > > + if (!riscv_insn_is_jal(inst))
> > > + continue;
> > > +
> > > + /* get and adjust new target address */
> > > + imm = RV_EXTRACT_JTYPE_IMM(inst);
> > > + imm -= patch_offset;
> > > +
> > > + /* pick the original jal */
> > > + call = inst;
> > > +
> > > + /* drop the old IMMs, all jal imm bits sit at 31:12 */
> > > + call &= ~GENMASK(31, 12);
> >
> > It'd be nice if this had a define.
> >
> > > +
> > > + /* add the adapted IMMs */
> > > + call |= to_jal_imm(imm);
> > > +
> > > + /* patch the call place again */
> > > + patch_text_nosync(alt_ptr + i * sizeof(u32), &call, 4);
> > > + }
> > > +}
> > > +
> > > /*
> > > * This is called very early in the boot process (directly after we run
> > > * a feature detect on the boot CPU). No need to worry about other CPUs
> > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > index ba62a4ff5ccd..c743f0adc794 100644
> > > --- a/arch/riscv/kernel/cpufeature.c
> > > +++ b/arch/riscv/kernel/cpufeature.c
> > > @@ -324,6 +324,9 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> > > riscv_alternative_fix_auipc_jalr(alt->old_ptr,
> > > alt->alt_len,
> > > alt->old_ptr - alt->alt_ptr);
> > > + riscv_alternative_fix_jal(alt->old_ptr,
> > > + alt->alt_len,
> > > + alt->old_ptr - alt->alt_ptr);
> > > }
> > > }
> > > }
> > > --
> > > 2.37.2
> > >
> >
> > Thanks,
> > drew
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives
2022-12-05 15:40 ` Jisheng Zhang
@ 2022-12-05 18:36 ` Conor Dooley
2022-12-05 18:49 ` Heiko Stübner
0 siblings, 1 reply; 52+ messages in thread
From: Conor Dooley @ 2022-12-05 18:36 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Heiko Stübner, Palmer Dabbelt, Paul Walmsley, Albert Ou,
Anup Patel, Atish Patra, Andrew Jones, linux-riscv, linux-kernel,
kvm, kvm-riscv
[-- Attachment #1: Type: text/plain, Size: 5277 bytes --]
Heiko, Jisheng,
On Mon, Dec 05, 2022 at 11:40:44PM +0800, Jisheng Zhang wrote:
> On Mon, Dec 05, 2022 at 04:31:08PM +0100, Heiko Stübner wrote:
> > Am Sonntag, 4. Dezember 2022, 18:46:20 CET schrieb Jisheng Zhang:
> > > 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.
> > >
> > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > > ---
> > > arch/riscv/include/asm/alternative.h | 2 ++
> > > arch/riscv/kernel/alternative.c | 38 ++++++++++++++++++++++++++++
> > > arch/riscv/kernel/cpufeature.c | 3 +++
> > > 3 files changed, 43 insertions(+)
> > >
> > > diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> > > index c58ec3cc4bc3..33eae9541684 100644
> > > --- a/arch/riscv/include/asm/alternative.h
> > > +++ b/arch/riscv/include/asm/alternative.h
> > > @@ -29,6 +29,8 @@ void apply_module_alternatives(void *start, size_t length);
> > >
> > > void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
> > > int patch_offset);
> > > +void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
> > > + int patch_offset);
> > >
> > > struct alt_entry {
> > > void *old_ptr; /* address of original instruciton or data */
> > > diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
> > > index 292cc42dc3be..9d88375624b5 100644
> > > --- a/arch/riscv/kernel/alternative.c
> > > +++ b/arch/riscv/kernel/alternative.c
> > > @@ -125,6 +125,44 @@ void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
> > > }
> > > }
> > >
> > > +#define to_jal_imm(value) \
> > > + (((value & (RV_J_IMM_10_1_MASK << RV_J_IMM_10_1_OFF)) << RV_I_IMM_11_0_OPOFF) | \
> > > + ((value & (RV_J_IMM_11_MASK << RV_J_IMM_11_OFF)) << RV_J_IMM_11_OPOFF) | \
> > > + ((value & (RV_J_IMM_19_12_OPOFF << RV_J_IMM_19_12_OFF)) << RV_J_IMM_19_12_OPOFF) | \
> > > + ((value & (1 << RV_J_IMM_SIGN_OFF)) << RV_J_IMM_SIGN_OPOFF))
> > > +
> > > +void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
> > > + int patch_offset)
> > > +{
> >
> > I think we might want to unfiy this into a common function like
> >
> > riscv_alternative_fix_offsets(...)
> >
> > so that we only run through the code block once
> >
> > for (i = 0; i < num_instr; i++) {
> > if (riscv_insn_is_auipc_jalr(inst1, inst2)) {
> > riscv_alternative_fix_auipc_jalr(...)
> > continue;
> > }
> >
> > if (riscv_insn_is_jal(inst)) {
> > riscv_alternative_fix_jal(...)
> > continue;
> > }
> > }
> >
> > This would also remove the need from calling multiple functions
> > after patching alternatives.
>
> Yesterday, I also wanted to unify the two instruction fix into
> one. But that would need to roll back the
> riscv_alternative_fix_auipc_jalr() to your v1 version. And IMHO,
> it's better if you can split the Zbb string optimizations series
> into two: one for alternative improvements, another for Zbb. Then
> we may get the alternative improvements and this inst extension
> series merged in v6.2-rc1.
Heiko, perhaps you can correct me here:
Last Wednesday you & Palmer agreed that it was too late in the cycle to
apply any of the stuff touching alternatives?
If I do recall correctly, gives plenty of time to sort out any
interdependent changes here.
Could easily be misremembering, wouldn't be the first time!
Thanks,
Conor.
> > > + int num_instr = len / sizeof(u32);
> > > + unsigned int call;
> > > + int i;
> > > + int imm;
> > > +
> > > + for (i = 0; i < num_instr; i++) {
> > > + u32 inst = riscv_instruction_at(alt_ptr, i);
> > > +
> > > + if (!riscv_insn_is_jal(inst))
> > > + continue;
> > > +
> > > + /* get and adjust new target address */
> > > + imm = RV_EXTRACT_JTYPE_IMM(inst);
> > > + imm -= patch_offset;
> > > +
> > > + /* pick the original jal */
> > > + call = inst;
> > > +
> > > + /* drop the old IMMs, all jal imm bits sit at 31:12 */
> > > + call &= ~GENMASK(31, 12);
> > > +
> > > + /* add the adapted IMMs */
> > > + call |= to_jal_imm(imm);
> > > +
> > > + /* patch the call place again */
> > > + patch_text_nosync(alt_ptr + i * sizeof(u32), &call, 4);
> > > + }
> > > +}
> > > +
> > > /*
> > > * This is called very early in the boot process (directly after we run
> > > * a feature detect on the boot CPU). No need to worry about other CPUs
> > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > index ba62a4ff5ccd..c743f0adc794 100644
> > > --- a/arch/riscv/kernel/cpufeature.c
> > > +++ b/arch/riscv/kernel/cpufeature.c
> > > @@ -324,6 +324,9 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> > > riscv_alternative_fix_auipc_jalr(alt->old_ptr,
> > > alt->alt_len,
> > > alt->old_ptr - alt->alt_ptr);
> > > + riscv_alternative_fix_jal(alt->old_ptr,
> > > + alt->alt_len,
> > > + alt->old_ptr - alt->alt_ptr);
> > > }
> > > }
> > > }
> > >
> >
> >
> >
> >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives
2022-12-05 18:36 ` Conor Dooley
@ 2022-12-05 18:49 ` Heiko Stübner
2022-12-05 19:49 ` Conor Dooley
0 siblings, 1 reply; 52+ messages in thread
From: Heiko Stübner @ 2022-12-05 18:49 UTC (permalink / raw)
To: Jisheng Zhang, Conor Dooley
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Andrew Jones, linux-riscv, linux-kernel, kvm,
kvm-riscv
Am Montag, 5. Dezember 2022, 19:36:45 CET schrieb Conor Dooley:
> Heiko, Jisheng,
>
> On Mon, Dec 05, 2022 at 11:40:44PM +0800, Jisheng Zhang wrote:
> > On Mon, Dec 05, 2022 at 04:31:08PM +0100, Heiko Stübner wrote:
> > > Am Sonntag, 4. Dezember 2022, 18:46:20 CET schrieb Jisheng Zhang:
> > > > 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.
> > > >
> > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > > > ---
> > > > arch/riscv/include/asm/alternative.h | 2 ++
> > > > arch/riscv/kernel/alternative.c | 38 ++++++++++++++++++++++++++++
> > > > arch/riscv/kernel/cpufeature.c | 3 +++
> > > > 3 files changed, 43 insertions(+)
> > > >
> > > > diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> > > > index c58ec3cc4bc3..33eae9541684 100644
> > > > --- a/arch/riscv/include/asm/alternative.h
> > > > +++ b/arch/riscv/include/asm/alternative.h
> > > > @@ -29,6 +29,8 @@ void apply_module_alternatives(void *start, size_t length);
> > > >
> > > > void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
> > > > int patch_offset);
> > > > +void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
> > > > + int patch_offset);
> > > >
> > > > struct alt_entry {
> > > > void *old_ptr; /* address of original instruciton or data */
> > > > diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
> > > > index 292cc42dc3be..9d88375624b5 100644
> > > > --- a/arch/riscv/kernel/alternative.c
> > > > +++ b/arch/riscv/kernel/alternative.c
> > > > @@ -125,6 +125,44 @@ void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
> > > > }
> > > > }
> > > >
> > > > +#define to_jal_imm(value) \
> > > > + (((value & (RV_J_IMM_10_1_MASK << RV_J_IMM_10_1_OFF)) << RV_I_IMM_11_0_OPOFF) | \
> > > > + ((value & (RV_J_IMM_11_MASK << RV_J_IMM_11_OFF)) << RV_J_IMM_11_OPOFF) | \
> > > > + ((value & (RV_J_IMM_19_12_OPOFF << RV_J_IMM_19_12_OFF)) << RV_J_IMM_19_12_OPOFF) | \
> > > > + ((value & (1 << RV_J_IMM_SIGN_OFF)) << RV_J_IMM_SIGN_OPOFF))
> > > > +
> > > > +void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
> > > > + int patch_offset)
> > > > +{
> > >
> > > I think we might want to unfiy this into a common function like
> > >
> > > riscv_alternative_fix_offsets(...)
> > >
> > > so that we only run through the code block once
> > >
> > > for (i = 0; i < num_instr; i++) {
> > > if (riscv_insn_is_auipc_jalr(inst1, inst2)) {
> > > riscv_alternative_fix_auipc_jalr(...)
> > > continue;
> > > }
> > >
> > > if (riscv_insn_is_jal(inst)) {
> > > riscv_alternative_fix_jal(...)
> > > continue;
> > > }
> > > }
> > >
> > > This would also remove the need from calling multiple functions
> > > after patching alternatives.
> >
> > Yesterday, I also wanted to unify the two instruction fix into
> > one. But that would need to roll back the
> > riscv_alternative_fix_auipc_jalr() to your v1 version. And IMHO,
> > it's better if you can split the Zbb string optimizations series
> > into two: one for alternative improvements, another for Zbb. Then
> > we may get the alternative improvements and this inst extension
> > series merged in v6.2-rc1.
>
> Heiko, perhaps you can correct me here:
>
> Last Wednesday you & Palmer agreed that it was too late in the cycle to
> apply any of the stuff touching alternatives?
> If I do recall correctly, gives plenty of time to sort out any
> interdependent changes here.
>
> Could easily be misremembering, wouldn't be the first time!
You slightly misremembered, but are still correct with the above ;-) .
I.e. what we talked about was stuff for fixes for 6.1-rc, were Palmers
wisely wanted to limit additions to really easy fixes for the remaining
last rc, to not upset any existing boards.
But you are still correct that we also shouldn't target the 6.2 merge window
anymore :-) .
We're after -rc8 now (which is in itself uncommon) and in his -rc7
announcement [0], Linus stated
"[...] the usual rule is that things that I get sent for the
merge window should have been all ready _before_ the merge window
opened. But with the merge window happening largely during the holiday
season, I'll just be enforcing that pretty strictly."
That means new stuff should be reviewed and in linux-next _way before_ the
merge window opens next weekend. Taking into account that people need
to review stuff (and maybe the series needing another round), I really don't
see this happening this week and everything else will get us shouted at
from atop a christmas tree ;-) .
That's the reason most maintainer-trees stop accepting stuff after -rc7
Heiko
[0] https://lkml.org/lkml/2022/11/27/278
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 04/13] riscv: hwcap: make ISA extension ids can be used in asm
2022-12-04 17:46 ` [PATCH v2 04/13] riscv: hwcap: make ISA extension ids can be used in asm Jisheng Zhang
@ 2022-12-05 18:53 ` Conor Dooley
2022-12-22 22:58 ` Conor Dooley
0 siblings, 1 reply; 52+ messages in thread
From: Conor Dooley @ 2022-12-05 18:53 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
[-- Attachment #1: Type: text/plain, Size: 3133 bytes --]
Hey Jisheng,
On Mon, Dec 05, 2022 at 01:46:23AM +0800, Jisheng Zhang wrote:
> We will make use of ISA extension in asm files, so make the multi-letter
> RISC-V ISA extension IDs macros rather than enums and move them and
> those base ISA extension IDs to suitable place.
Which base ISA extension IDs? Changelog should match the patch contents,
and it's a little unclear here since the base ISA extension IDs are
visible here but in the context not the diff.
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> ---
> arch/riscv/include/asm/hwcap.h | 43 ++++++++++++++++------------------
> 1 file changed, 20 insertions(+), 23 deletions(-)
>
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index b22525290073..996884986fea 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -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 = RISCV_ISA_EXT_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
Could you re-order these alphabetically when you move them please?
Thanks,
Conor.
> +
> +#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;
> +
> /*
> * 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
> --
> 2.37.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 03/13] riscv: cpufeature: detect RISCV_ALTERNATIVES_EARLY_BOOT earlier
2022-12-04 17:46 ` [PATCH v2 03/13] riscv: cpufeature: detect RISCV_ALTERNATIVES_EARLY_BOOT earlier Jisheng Zhang
@ 2022-12-05 19:09 ` Conor Dooley
0 siblings, 0 replies; 52+ messages in thread
From: Conor Dooley @ 2022-12-05 19:09 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
[-- Attachment #1: Type: text/plain, Size: 1575 bytes --]
On Mon, Dec 05, 2022 at 01:46:22AM +0800, Jisheng Zhang wrote:
> Now, the riscv_cpufeature_patch_func() do nothing in the stage of
> RISCV_ALTERNATIVES_EARLY_BOOT. We can move the detection of "early
> boot" stage earlier.
>
> In following patch, we will make riscv_cpufeature_patch_func() scans
> all ISA extensions.
I'm not 100% on the motivation here. Let me try and regurgitate the
changelog in a way that makes more sense to me and maybe you can confirm
it.
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.
Would that be a correct summary?
Thanks,
Conor.
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> ---
> arch/riscv/kernel/cpufeature.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 364d1fe86bea..a4d2af67e05c 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -305,6 +305,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;
> --
> 2.37.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 05/13] riscv: cpufeature: extend riscv_cpufeature_patch_func to all ISA extensions
2022-12-04 17:46 ` [PATCH v2 05/13] riscv: cpufeature: extend riscv_cpufeature_patch_func to all ISA extensions Jisheng Zhang
@ 2022-12-05 19:37 ` Conor Dooley
0 siblings, 0 replies; 52+ messages in thread
From: Conor Dooley @ 2022-12-05 19:37 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
[-- Attachment #1: Type: text/plain, Size: 2221 bytes --]
On Mon, Dec 05, 2022 at 01:46:24AM +0800, Jisheng Zhang wrote:
> make the riscv_cpufeature_patch_func() scan all ISA extensions rather
> than limited feature macros.
Certainly looks like a nice cleanup. Perhaps for the changelog,
something along the lines of:
"riscv_cpufeature_patch_func() currently only scans a limited set of
cpufeatures, explicitly defined with macros. Extend it to probe for all
ISA extensions"
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> ---
> arch/riscv/include/asm/errata_list.h | 9 ++--
> arch/riscv/kernel/cpufeature.c | 73 +++++-----------------------
> 2 files changed, 15 insertions(+), 67 deletions(-)
> @@ -311,25 +264,23 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> for (alt = begin; alt < end; alt++) {
> if (alt->vendor_id != 0)
> continue;
> - if (alt->errata_id >= CPUFEATURE_NUMBER) {
> - WARN(1, "This feature id:%d is not in kernel cpufeature list",
> + if (alt->errata_id >= RISCV_ISA_EXT_MAX) {
> + WARN(1, "This extension id:%d is not in ISA extension list",
> alt->errata_id);
> continue;
> }
>
> - tmp = (1U << alt->errata_id);
> - if (cpu_req_feature & tmp) {
> - /* do the basic patching */
> - patch_text_nosync(alt->old_ptr, alt->alt_ptr,
> - alt->alt_len);
> + if (!__riscv_isa_extension_available(NULL, alt->errata_id))
> + continue;
>
> - riscv_alternative_fix_auipc_jalr(alt->old_ptr,
> - alt->alt_len,
> - alt->old_ptr - alt->alt_ptr);
> - riscv_alternative_fix_jal(alt->old_ptr,
> - alt->alt_len,
> - alt->old_ptr - alt->alt_ptr);
> - }
> + /* do the basic patching */
> + patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> + riscv_alternative_fix_auipc_jalr(alt->old_ptr,
> + alt->alt_len,
> + alt->old_ptr - alt->alt_ptr);
> + riscv_alternative_fix_jal(alt->old_ptr,
> + alt->alt_len,
> + alt->old_ptr - alt->alt_ptr);
nit:
Now that you've dropped a level of indent, can alt->alt_len move up a
line?
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives
2022-12-05 18:49 ` Heiko Stübner
@ 2022-12-05 19:49 ` Conor Dooley
2022-12-06 0:39 ` Heiko Stübner
0 siblings, 1 reply; 52+ messages in thread
From: Conor Dooley @ 2022-12-05 19:49 UTC (permalink / raw)
To: Heiko Stübner
Cc: Jisheng Zhang, Palmer Dabbelt, Paul Walmsley, Albert Ou,
Anup Patel, Atish Patra, Andrew Jones, linux-riscv, linux-kernel,
kvm, kvm-riscv
[-- Attachment #1: Type: text/plain, Size: 3080 bytes --]
On Mon, Dec 05, 2022 at 07:49:01PM +0100, Heiko Stübner wrote:
> Am Montag, 5. Dezember 2022, 19:36:45 CET schrieb Conor Dooley:
> > Heiko, Jisheng,
> > On Mon, Dec 05, 2022 at 11:40:44PM +0800, Jisheng Zhang wrote:
> > > Yesterday, I also wanted to unify the two instruction fix into
> > > one. But that would need to roll back the
> > > riscv_alternative_fix_auipc_jalr() to your v1 version. And IMHO,
> > > it's better if you can split the Zbb string optimizations series
> > > into two: one for alternative improvements, another for Zbb. Then
> > > we may get the alternative improvements and this inst extension
> > > series merged in v6.2-rc1.
> >
> > Heiko, perhaps you can correct me here:
> >
> > Last Wednesday you & Palmer agreed that it was too late in the cycle to
> > apply any of the stuff touching alternatives?
> > If I do recall correctly, gives plenty of time to sort out any
> > interdependent changes here.
> >
> > Could easily be misremembering, wouldn't be the first time!
>
> You slightly misremembered, but are still correct with the above ;-) .
>
> I.e. what we talked about was stuff for fixes for 6.1-rc, were Palmers
> wisely wanted to limit additions to really easy fixes for the remaining
> last rc, to not upset any existing boards.
Ahh right. I was 50-50 on whether something like that was said so at
least I am not going crazy.
> But you are still correct that we also shouldn't target the 6.2 merge window
> anymore :-) .
>
> We're after -rc8 now (which is in itself uncommon) and in his -rc7
> announcement [0], Linus stated
>
> "[...] the usual rule is that things that I get sent for the
> merge window should have been all ready _before_ the merge window
> opened. But with the merge window happening largely during the holiday
> season, I'll just be enforcing that pretty strictly."
Yah, of all the windows to land patchsets that are being re-spun a few
days before it opens this probably isn't the best one to pick!
> That means new stuff should be reviewed and in linux-next _way before_ the
> merge window opens next weekend. Taking into account that people need
> to review stuff (and maybe the series needing another round), I really don't
> see this happening this week and everything else will get us shouted at
> from atop a christmas tree ;-) .
>
> That's the reason most maintainer-trees stop accepting stuff after -rc7
Aye, in RISC-V land maybe we will get there one day :)
For the original question though, breaking them up into 3 or 4 smaller
bits that could get applied on their own is probably a good idea?
Between yourselves, Drew and Prabhakar there's a couple series touching
the same bits. Certainly don't want to seem like I am speaking for the
Higher Powers here, but some sort of logical ordering would probably be
a good idea so as not to hold each other up?
The non-string bit of your series has been fairly well reviewed & would,
in theory, be mergeable once the tree re-opens? Timing aside, Jisheng's
idea seems like a good one, no?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives
2022-12-05 19:49 ` Conor Dooley
@ 2022-12-06 0:39 ` Heiko Stübner
2022-12-06 15:02 ` Jisheng Zhang
0 siblings, 1 reply; 52+ messages in thread
From: Heiko Stübner @ 2022-12-06 0:39 UTC (permalink / raw)
To: Conor Dooley
Cc: Jisheng Zhang, Palmer Dabbelt, Paul Walmsley, Albert Ou,
Anup Patel, Atish Patra, Andrew Jones, linux-riscv, linux-kernel,
kvm, kvm-riscv
Am Montag, 5. Dezember 2022, 20:49:26 CET schrieb Conor Dooley:
> On Mon, Dec 05, 2022 at 07:49:01PM +0100, Heiko Stübner wrote:
> > Am Montag, 5. Dezember 2022, 19:36:45 CET schrieb Conor Dooley:
> > > Heiko, Jisheng,
> > > On Mon, Dec 05, 2022 at 11:40:44PM +0800, Jisheng Zhang wrote:
> > > > Yesterday, I also wanted to unify the two instruction fix into
> > > > one. But that would need to roll back the
> > > > riscv_alternative_fix_auipc_jalr() to your v1 version. And IMHO,
> > > > it's better if you can split the Zbb string optimizations series
> > > > into two: one for alternative improvements, another for Zbb. Then
> > > > we may get the alternative improvements and this inst extension
> > > > series merged in v6.2-rc1.
> > >
> > > Heiko, perhaps you can correct me here:
> > >
> > > Last Wednesday you & Palmer agreed that it was too late in the cycle to
> > > apply any of the stuff touching alternatives?
> > > If I do recall correctly, gives plenty of time to sort out any
> > > interdependent changes here.
> > >
> > > Could easily be misremembering, wouldn't be the first time!
> >
> > You slightly misremembered, but are still correct with the above ;-) .
> >
> > I.e. what we talked about was stuff for fixes for 6.1-rc, were Palmers
> > wisely wanted to limit additions to really easy fixes for the remaining
> > last rc, to not upset any existing boards.
>
> Ahh right. I was 50-50 on whether something like that was said so at
> least I am not going crazy.
>
> > But you are still correct that we also shouldn't target the 6.2 merge window
> > anymore :-) .
> >
> > We're after -rc8 now (which is in itself uncommon) and in his -rc7
> > announcement [0], Linus stated
> >
> > "[...] the usual rule is that things that I get sent for the
> > merge window should have been all ready _before_ the merge window
> > opened. But with the merge window happening largely during the holiday
> > season, I'll just be enforcing that pretty strictly."
>
> Yah, of all the windows to land patchsets that are being re-spun a few
> days before it opens this probably isn't the best one to pick!
>
> > That means new stuff should be reviewed and in linux-next _way before_ the
> > merge window opens next weekend. Taking into account that people need
> > to review stuff (and maybe the series needing another round), I really don't
> > see this happening this week and everything else will get us shouted at
> > from atop a christmas tree ;-) .
> >
> > That's the reason most maintainer-trees stop accepting stuff after -rc7
>
> Aye, in RISC-V land maybe we will get there one day :)
>
> For the original question though, breaking them up into 3 or 4 smaller
> bits that could get applied on their own is probably a good idea?
>
> Between yourselves, Drew and Prabhakar there's a couple series touching
> the same bits. Certainly don't want to seem like I am speaking for the
> Higher Powers here, but some sort of logical ordering would probably be
> a good idea so as not to hold each other up?
> The non-string bit of your series has been fairly well reviewed & would,
> in theory, be mergeable once the tree re-opens? Timing aside, Jisheng's
> idea seems like a good one, no?
yeah, I had that same thought over the weekend - with the generic
part being pretty good in the review and only the string part needing
more work and thus ideally splitting the series [0] .
Jisheng's series just made that even more important to do :-)
Heiko
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 10/13] riscv: alternative: patch alternatives in the vDSO
2022-12-05 15:23 ` Jisheng Zhang
@ 2022-12-06 4:29 ` Guo Ren
0 siblings, 0 replies; 52+ messages in thread
From: Guo Ren @ 2022-12-06 4:29 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
On Mon, Dec 5, 2022 at 11:33 PM Jisheng Zhang <jszhang@kernel.org> wrote:
>
> On Mon, Dec 05, 2022 at 09:56:37AM +0800, Guo Ren wrote:
> > Are there any patches that depend on it? Any existing utilization? My
> > first idea is to let __vdso_flush_icache use it, the standard
> > implementation is so heavy for user space JIT scenario, maybe vendors
> > could give a custom one.
>
> Hi Guo,
>
> the 11th patch of swtiching cpu_relax() to riscv_has_extension_likely()
> depends on this patch, the gettimeofday implementation calls cpu_relax()
> which may use ZIHINTPAUSE extension, so we need to patch vDSO.
Oh, I missed that. Thx.
Reviewed-by: Guo Ren <guoren@kernel.org>
>
> Thanks
>
> >
> > On Mon, Dec 5, 2022 at 1:57 AM Jisheng Zhang <jszhang@kernel.org> wrote:
> > >
> > > Make it possible to use alternatives in the vDSO, so that better
> > > implementations can be used if possible.
> > >
> > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > > ---
> > > arch/riscv/include/asm/vdso.h | 4 ++++
> > > arch/riscv/kernel/alternative.c | 25 +++++++++++++++++++++++++
> > > arch/riscv/kernel/vdso.c | 5 -----
> > > arch/riscv/kernel/vdso/vdso.lds.S | 7 +++++++
> > > 4 files changed, 36 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/arch/riscv/include/asm/vdso.h b/arch/riscv/include/asm/vdso.h
> > > index af981426fe0f..b6ff7473fb8a 100644
> > > --- a/arch/riscv/include/asm/vdso.h
> > > +++ b/arch/riscv/include/asm/vdso.h
> > > @@ -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 */
> > > diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
> > > index 9d88375624b5..eaf7ddaba54c 100644
> > > --- a/arch/riscv/kernel/alternative.c
> > > +++ b/arch/riscv/kernel/alternative.c
> > > @@ -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>
> > > @@ -187,6 +189,27 @@ static void __init_or_module _apply_alternatives(struct alt_entry *begin,
> > > stage);
> > > }
> > >
> > > +static void __init apply_vdso_alternatives(void)
> > > +{
> > > + const struct elf64_hdr *hdr;
> > > + const struct elf64_shdr *shdr;
> > > + const struct elf64_shdr *alt;
> > > + struct alt_entry *begin, *end;
> > > +
> > > + hdr = (struct elf64_hdr *)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);
> > > +}
> > > +
> > > void __init apply_boot_alternatives(void)
> > > {
> > > /* If called on non-boot cpu things could go wrong */
> > > @@ -195,6 +218,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();
> > > }
> > >
> > > /*
> > > diff --git a/arch/riscv/kernel/vdso.c b/arch/riscv/kernel/vdso.c
> > > index 123d05255fcf..1f47bc6566cf 100644
> > > --- a/arch/riscv/kernel/vdso.c
> > > +++ b/arch/riscv/kernel/vdso.c
> > > @@ -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,
> > > diff --git a/arch/riscv/kernel/vdso/vdso.lds.S b/arch/riscv/kernel/vdso/vdso.lds.S
> > > index 150b1a572e61..4a0606633290 100644
> > > --- a/arch/riscv/kernel/vdso/vdso.lds.S
> > > +++ b/arch/riscv/kernel/vdso/vdso.lds.S
> > > @@ -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.*)
> > > --
> > > 2.37.2
> > >
> >
> >
> > --
> > Best Regards
> > Guo Ren
--
Best Regards
Guo Ren
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 09/13] riscv: switch to relative alternative entries
2022-12-05 15:18 ` Jisheng Zhang
@ 2022-12-06 4:34 ` Guo Ren
2022-12-06 14:50 ` Jisheng Zhang
0 siblings, 1 reply; 52+ messages in thread
From: Guo Ren @ 2022-12-06 4:34 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
On Mon, Dec 5, 2022 at 11:28 PM Jisheng Zhang <jszhang@kernel.org> wrote:
>
> On Mon, Dec 05, 2022 at 08:51:41AM +0800, Guo Ren wrote:
> > On Mon, Dec 5, 2022 at 1:57 AM Jisheng Zhang <jszhang@kernel.org> wrote:
> > >
> > > Instead of using absolute addresses for both the old instrucions and
> > > the alternative instructions, use offsets relative to the alt_entry
> > > values. So we can not only cut the size of the alternative entry, but
> > > also meet the prerequisite for patching alternatives in the vDSO,
> > > since absolute alternative entries are subject to dynamic relocation,
> > > which is incompatible with the vDSO building.
> > >
> > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > > ---
> > > arch/riscv/errata/sifive/errata.c | 4 +++-
> > > arch/riscv/errata/thead/errata.c | 11 ++++++++---
> > > arch/riscv/include/asm/alternative-macros.h | 20 ++++++++++----------
> > > arch/riscv/include/asm/alternative.h | 12 ++++++------
> > > arch/riscv/kernel/cpufeature.c | 13 ++++++-------
> > > 5 files changed, 33 insertions(+), 27 deletions(-)
> > >
> > > diff --git a/arch/riscv/errata/sifive/errata.c b/arch/riscv/errata/sifive/errata.c
> > > index 1031038423e7..0e537cdfd324 100644
> > > --- a/arch/riscv/errata/sifive/errata.c
> > > +++ b/arch/riscv/errata/sifive/errata.c
> > > @@ -107,7 +107,9 @@ 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((void *)&alt->old_offset + alt->old_offset,
> > > + (void *)&alt->alt_offset + alt->alt_offset,
> > (void *)&alt->alt_offset + alt->alt_offset. ??!!
>
> Hi Guo,
>
> what's the problem? I can't catch your meaning, could you please proide
> more details?
Can you explain why:
alt->old_ptr = (void *)&alt->old_offset + alt->old_offset
| offset | <- &offset
| ... |
| value | <- ptr = &offset + offset
I don't make sense of the above.
>
> Thanks
>
> >
> > > + alt->alt_len);
> > > cpu_apply_errata |= tmp;
> > > }
> > > }
> > > diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
> > > index 21546937db39..2a6e335b5a32 100644
> > > --- a/arch/riscv/errata/thead/errata.c
> > > +++ b/arch/riscv/errata/thead/errata.c
> > > @@ -68,6 +68,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, *updptr;
> > >
> > > for (alt = begin; alt < end; alt++) {
> > > if (alt->vendor_id != THEAD_VENDOR_ID)
> > > @@ -77,12 +78,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 = (void *)&alt->old_offset + alt->old_offset;
> > > + updptr = (void *)&alt->alt_offset + alt->alt_offset;
> > > +
> > > /* 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(updptr),
> > > + alt->alt_len);
> > > else
> > > - patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> > > + patch_text_nosync(oldptr, updptr, alt->alt_len);
> > > }
> > > }
> > >
> > > diff --git a/arch/riscv/include/asm/alternative-macros.h b/arch/riscv/include/asm/alternative-macros.h
> > > index ec2f3f1b836f..dd40727bc859 100644
> > > --- a/arch/riscv/include/asm/alternative-macros.h
> > > +++ b/arch/riscv/include/asm/alternative-macros.h
> > > @@ -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
> > > + .long \oldptr - .
> > > + .long \newptr - .
> > > + .short \vendor_id
> > > + .short \new_len
> > > + .long \errata_id
> > > .endm
> > >
> > > .macro ALT_NEW_CONTENT vendor_id, errata_id, enable = 1, new_c : vararg
> > > @@ -75,11 +75,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"
> > > + ".long ((" oldptr ") - .) \n" \
> > > + ".long ((" newptr ") - .) \n" \
> > > + ".short " vendor_id "\n" \
> > > + ".short " newlen "\n" \
> > > + ".long " errata_id "\n"
> > >
> > > #define ALT_NEW_CONTENT(vendor_id, errata_id, enable, new_c) \
> > > ".if " __stringify(enable) " == 1\n" \
> > > diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> > > index 33eae9541684..3baf32e05b46 100644
> > > --- a/arch/riscv/include/asm/alternative.h
> > > +++ b/arch/riscv/include/asm/alternative.h
> > > @@ -33,12 +33,12 @@ void riscv_alternative_fix_jal(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 to original instruciton or data */
> > > + s32 alt_offset; /* offset 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;
> > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > index 6244be5cd94a..adeac90b1d8e 100644
> > > --- a/arch/riscv/kernel/cpufeature.c
> > > +++ b/arch/riscv/kernel/cpufeature.c
> > > @@ -257,6 +257,7 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> > > unsigned int stage)
> > > {
> > > struct alt_entry *alt;
> > > + void *oldptr, *updptr;
> > >
> > > if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> > > return;
> > > @@ -270,17 +271,15 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> > > continue;
> > > }
> > >
> > > + oldptr = (void *)&alt->old_offset + alt->old_offset;
> > > + updptr = (void *)&alt->alt_offset + alt->alt_offset;
> > > if (!__riscv_isa_extension_available(NULL, alt->errata_id))
> > > continue;
> > >
> > > /* do the basic patching */
> > > - patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> > > - riscv_alternative_fix_auipc_jalr(alt->old_ptr,
> > > - alt->alt_len,
> > > - alt->old_ptr - alt->alt_ptr);
> > > - riscv_alternative_fix_jal(alt->old_ptr,
> > > - alt->alt_len,
> > > - alt->old_ptr - alt->alt_ptr);
> > > + patch_text_nosync(oldptr, updptr, alt->alt_len);
> > > + riscv_alternative_fix_auipc_jalr(oldptr, alt->alt_len, oldptr - updptr);
> > > + riscv_alternative_fix_jal(oldptr, alt->alt_len, oldptr - updptr);
> > > }
> > > }
> > > #endif
> > > --
> > > 2.37.2
> > >
> >
> >
> > --
> > Best Regards
> > Guo Ren
--
Best Regards
Guo Ren
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives
2022-12-05 16:49 ` Jisheng Zhang
@ 2022-12-06 5:50 ` Andrew Jones
0 siblings, 0 replies; 52+ messages in thread
From: Andrew Jones @ 2022-12-06 5:50 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, linux-riscv, linux-kernel, kvm,
kvm-riscv
On Tue, Dec 06, 2022 at 12:49:51AM +0800, Jisheng Zhang wrote:
> On Tue, Dec 06, 2022 at 12:42:16AM +0800, Jisheng Zhang wrote:
> > On Mon, Dec 05, 2022 at 03:57:10PM +0100, Andrew Jones wrote:
> > > On Mon, Dec 05, 2022 at 01:46:20AM +0800, Jisheng Zhang wrote:
> > > > 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.
> > > >
> > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > > > ---
> > > > arch/riscv/include/asm/alternative.h | 2 ++
> > > > arch/riscv/kernel/alternative.c | 38 ++++++++++++++++++++++++++++
> > > > arch/riscv/kernel/cpufeature.c | 3 +++
> > > > 3 files changed, 43 insertions(+)
> > > >
> > > > diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> > > > index c58ec3cc4bc3..33eae9541684 100644
> > > > --- a/arch/riscv/include/asm/alternative.h
> > > > +++ b/arch/riscv/include/asm/alternative.h
> > > > @@ -29,6 +29,8 @@ void apply_module_alternatives(void *start, size_t length);
> > > >
> > > > void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
> > > > int patch_offset);
> > > > +void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
> > > > + int patch_offset);
> > > >
> > > > struct alt_entry {
> > > > void *old_ptr; /* address of original instruciton or data */
> > > > diff --git a/arch/riscv/kernel/alternative.c b/arch/riscv/kernel/alternative.c
> > > > index 292cc42dc3be..9d88375624b5 100644
> > > > --- a/arch/riscv/kernel/alternative.c
> > > > +++ b/arch/riscv/kernel/alternative.c
> > > > @@ -125,6 +125,44 @@ void riscv_alternative_fix_auipc_jalr(void *alt_ptr, unsigned int len,
> > > > }
> > > > }
> > > >
> > > > +#define to_jal_imm(value) \
> > > > + (((value & (RV_J_IMM_10_1_MASK << RV_J_IMM_10_1_OFF)) << RV_I_IMM_11_0_OPOFF) | \
> > > ^ RV_J_IMM_10_1_OPOFF
> > >
> > > > + ((value & (RV_J_IMM_11_MASK << RV_J_IMM_11_OFF)) << RV_J_IMM_11_OPOFF) | \
> > > > + ((value & (RV_J_IMM_19_12_OPOFF << RV_J_IMM_19_12_OFF)) << RV_J_IMM_19_12_OPOFF) | \
> > > > + ((value & (1 << RV_J_IMM_SIGN_OFF)) << RV_J_IMM_SIGN_OPOFF))
> >
> > Hi all,
> >
> > I believe there's bug in the to_jal_imm() macro implementation, the
> > correct one should be like this:
> >
> > #define to_jal_imm(value) \
> > ((RV_X(value, RV_J_IMM_10_1_OFF, RV_J_IMM_10_1_MASK) << RV_J_IMM_10_1_OPOFF) | \
> > (RV_X(value, RV_J_IMM_11_OFF, RV_J_IMM_11_MASK) << RV_J_IMM_11_OPOFF) | \
> > (RV_X(value, RV_J_IMM_19_12_OFF, RV_J_IMM_19_12_MASK) << RV_J_IMM_19_12_OPOFF) | \
> > (RV_X(value, RV_J_IMM_SIGN_OFF, 1) << RV_J_IMM_SIGN_OPOFF))
>
> And I just tested to_jal_imm() vs RV_EXTRACT_JTYPE_IMM(), they match perfectly.
> E.g:
> RV_EXTRACT_JTYPE_IMM(to_jal_imm(imm)) == imm is alway true when imm is a jal
> valid offset.
I think we should define deposit() functions for each type, as well as the
extract() functions, (and I'd prefer we use static inlines to macros to
get some type checking). See [1] for my proposal.
[1] https://lore.kernel.org/all/20221205145323.l2dro6dt7muumqpn@kamzik/
Thanks,
drew
>
> >
> > Will fix it in next version.
> >
> > Thanks
> > >
> > > Should put () around value
> > >
> > > > +
> > > > +void riscv_alternative_fix_jal(void *alt_ptr, unsigned int len,
> > > > + int patch_offset)
> > > > +{
> > > > + int num_instr = len / sizeof(u32);
> > > > + unsigned int call;
> > > > + int i;
> > > > + int imm;
> > > > +
> > > > + for (i = 0; i < num_instr; i++) {
> > > > + u32 inst = riscv_instruction_at(alt_ptr, i);
> > > > +
> > > > + if (!riscv_insn_is_jal(inst))
> > > > + continue;
> > > > +
> > > > + /* get and adjust new target address */
> > > > + imm = RV_EXTRACT_JTYPE_IMM(inst);
> > > > + imm -= patch_offset;
> > > > +
> > > > + /* pick the original jal */
> > > > + call = inst;
> > > > +
> > > > + /* drop the old IMMs, all jal imm bits sit at 31:12 */
> > > > + call &= ~GENMASK(31, 12);
> > >
> > > It'd be nice if this had a define.
> > >
> > > > +
> > > > + /* add the adapted IMMs */
> > > > + call |= to_jal_imm(imm);
> > > > +
> > > > + /* patch the call place again */
> > > > + patch_text_nosync(alt_ptr + i * sizeof(u32), &call, 4);
> > > > + }
> > > > +}
> > > > +
> > > > /*
> > > > * This is called very early in the boot process (directly after we run
> > > > * a feature detect on the boot CPU). No need to worry about other CPUs
> > > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > > index ba62a4ff5ccd..c743f0adc794 100644
> > > > --- a/arch/riscv/kernel/cpufeature.c
> > > > +++ b/arch/riscv/kernel/cpufeature.c
> > > > @@ -324,6 +324,9 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> > > > riscv_alternative_fix_auipc_jalr(alt->old_ptr,
> > > > alt->alt_len,
> > > > alt->old_ptr - alt->alt_ptr);
> > > > + riscv_alternative_fix_jal(alt->old_ptr,
> > > > + alt->alt_len,
> > > > + alt->old_ptr - alt->alt_ptr);
> > > > }
> > > > }
> > > > }
> > > > --
> > > > 2.37.2
> > > >
> > >
> > > Thanks,
> > > drew
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 09/13] riscv: switch to relative alternative entries
2022-12-06 4:34 ` Guo Ren
@ 2022-12-06 14:50 ` Jisheng Zhang
2022-12-06 21:43 ` Conor Dooley
0 siblings, 1 reply; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-06 14:50 UTC (permalink / raw)
To: Guo Ren
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
On Tue, Dec 06, 2022 at 12:34:40PM +0800, Guo Ren wrote:
> On Mon, Dec 5, 2022 at 11:28 PM Jisheng Zhang <jszhang@kernel.org> wrote:
> >
> > On Mon, Dec 05, 2022 at 08:51:41AM +0800, Guo Ren wrote:
> > > On Mon, Dec 5, 2022 at 1:57 AM Jisheng Zhang <jszhang@kernel.org> wrote:
> > > >
> > > > Instead of using absolute addresses for both the old instrucions and
> > > > the alternative instructions, use offsets relative to the alt_entry
> > > > values. So we can not only cut the size of the alternative entry, but
> > > > also meet the prerequisite for patching alternatives in the vDSO,
> > > > since absolute alternative entries are subject to dynamic relocation,
> > > > which is incompatible with the vDSO building.
> > > >
> > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > > > ---
> > > > arch/riscv/errata/sifive/errata.c | 4 +++-
> > > > arch/riscv/errata/thead/errata.c | 11 ++++++++---
> > > > arch/riscv/include/asm/alternative-macros.h | 20 ++++++++++----------
> > > > arch/riscv/include/asm/alternative.h | 12 ++++++------
> > > > arch/riscv/kernel/cpufeature.c | 13 ++++++-------
> > > > 5 files changed, 33 insertions(+), 27 deletions(-)
> > > >
> > > > diff --git a/arch/riscv/errata/sifive/errata.c b/arch/riscv/errata/sifive/errata.c
> > > > index 1031038423e7..0e537cdfd324 100644
> > > > --- a/arch/riscv/errata/sifive/errata.c
> > > > +++ b/arch/riscv/errata/sifive/errata.c
> > > > @@ -107,7 +107,9 @@ 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((void *)&alt->old_offset + alt->old_offset,
> > > > + (void *)&alt->alt_offset + alt->alt_offset,
> > > (void *)&alt->alt_offset + alt->alt_offset. ??!!
> >
> > Hi Guo,
> >
> > what's the problem? I can't catch your meaning, could you please proide
> > more details?
> Can you explain why:
>
> alt->old_ptr = (void *)&alt->old_offset + alt->old_offset
Hi,
when constructing the alt entry, we save the offset in
then entry as below:
.long \oldptr - .
So we can restore the old_ptr by &alt->old_offset + alt->old_offset
Thanks
>
> | offset | <- &offset
> | ... |
> | value | <- ptr = &offset + offset
>
> I don't make sense of the above.
>
> >
> > Thanks
> >
> > >
> > > > + alt->alt_len);
> > > > cpu_apply_errata |= tmp;
> > > > }
> > > > }
> > > > diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
> > > > index 21546937db39..2a6e335b5a32 100644
> > > > --- a/arch/riscv/errata/thead/errata.c
> > > > +++ b/arch/riscv/errata/thead/errata.c
> > > > @@ -68,6 +68,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, *updptr;
> > > >
> > > > for (alt = begin; alt < end; alt++) {
> > > > if (alt->vendor_id != THEAD_VENDOR_ID)
> > > > @@ -77,12 +78,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 = (void *)&alt->old_offset + alt->old_offset;
> > > > + updptr = (void *)&alt->alt_offset + alt->alt_offset;
> > > > +
> > > > /* 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(updptr),
> > > > + alt->alt_len);
> > > > else
> > > > - patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> > > > + patch_text_nosync(oldptr, updptr, alt->alt_len);
> > > > }
> > > > }
> > > >
> > > > diff --git a/arch/riscv/include/asm/alternative-macros.h b/arch/riscv/include/asm/alternative-macros.h
> > > > index ec2f3f1b836f..dd40727bc859 100644
> > > > --- a/arch/riscv/include/asm/alternative-macros.h
> > > > +++ b/arch/riscv/include/asm/alternative-macros.h
> > > > @@ -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
> > > > + .long \oldptr - .
> > > > + .long \newptr - .
> > > > + .short \vendor_id
> > > > + .short \new_len
> > > > + .long \errata_id
> > > > .endm
> > > >
> > > > .macro ALT_NEW_CONTENT vendor_id, errata_id, enable = 1, new_c : vararg
> > > > @@ -75,11 +75,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"
> > > > + ".long ((" oldptr ") - .) \n" \
> > > > + ".long ((" newptr ") - .) \n" \
> > > > + ".short " vendor_id "\n" \
> > > > + ".short " newlen "\n" \
> > > > + ".long " errata_id "\n"
> > > >
> > > > #define ALT_NEW_CONTENT(vendor_id, errata_id, enable, new_c) \
> > > > ".if " __stringify(enable) " == 1\n" \
> > > > diff --git a/arch/riscv/include/asm/alternative.h b/arch/riscv/include/asm/alternative.h
> > > > index 33eae9541684..3baf32e05b46 100644
> > > > --- a/arch/riscv/include/asm/alternative.h
> > > > +++ b/arch/riscv/include/asm/alternative.h
> > > > @@ -33,12 +33,12 @@ void riscv_alternative_fix_jal(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 to original instruciton or data */
> > > > + s32 alt_offset; /* offset 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;
> > > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > > index 6244be5cd94a..adeac90b1d8e 100644
> > > > --- a/arch/riscv/kernel/cpufeature.c
> > > > +++ b/arch/riscv/kernel/cpufeature.c
> > > > @@ -257,6 +257,7 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> > > > unsigned int stage)
> > > > {
> > > > struct alt_entry *alt;
> > > > + void *oldptr, *updptr;
> > > >
> > > > if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> > > > return;
> > > > @@ -270,17 +271,15 @@ void __init_or_module riscv_cpufeature_patch_func(struct alt_entry *begin,
> > > > continue;
> > > > }
> > > >
> > > > + oldptr = (void *)&alt->old_offset + alt->old_offset;
> > > > + updptr = (void *)&alt->alt_offset + alt->alt_offset;
> > > > if (!__riscv_isa_extension_available(NULL, alt->errata_id))
> > > > continue;
> > > >
> > > > /* do the basic patching */
> > > > - patch_text_nosync(alt->old_ptr, alt->alt_ptr, alt->alt_len);
> > > > - riscv_alternative_fix_auipc_jalr(alt->old_ptr,
> > > > - alt->alt_len,
> > > > - alt->old_ptr - alt->alt_ptr);
> > > > - riscv_alternative_fix_jal(alt->old_ptr,
> > > > - alt->alt_len,
> > > > - alt->old_ptr - alt->alt_ptr);
> > > > + patch_text_nosync(oldptr, updptr, alt->alt_len);
> > > > + riscv_alternative_fix_auipc_jalr(oldptr, alt->alt_len, oldptr - updptr);
> > > > + riscv_alternative_fix_jal(oldptr, alt->alt_len, oldptr - updptr);
> > > > }
> > > > }
> > > > #endif
> > > > --
> > > > 2.37.2
> > > >
> > >
> > >
> > > --
> > > Best Regards
> > > Guo Ren
>
>
>
> --
> Best Regards
> Guo Ren
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives
2022-12-06 0:39 ` Heiko Stübner
@ 2022-12-06 15:02 ` Jisheng Zhang
2022-12-06 16:12 ` Conor Dooley
0 siblings, 1 reply; 52+ messages in thread
From: Jisheng Zhang @ 2022-12-06 15:02 UTC (permalink / raw)
To: Heiko Stübner
Cc: Conor Dooley, Palmer Dabbelt, Paul Walmsley, Albert Ou,
Anup Patel, Atish Patra, Andrew Jones, linux-riscv, linux-kernel,
kvm, kvm-riscv
On Tue, Dec 06, 2022 at 01:39:50AM +0100, Heiko Stübner wrote:
> Am Montag, 5. Dezember 2022, 20:49:26 CET schrieb Conor Dooley:
> > On Mon, Dec 05, 2022 at 07:49:01PM +0100, Heiko Stübner wrote:
> > > Am Montag, 5. Dezember 2022, 19:36:45 CET schrieb Conor Dooley:
> > > > Heiko, Jisheng,
> > > > On Mon, Dec 05, 2022 at 11:40:44PM +0800, Jisheng Zhang wrote:
> > > > > Yesterday, I also wanted to unify the two instruction fix into
> > > > > one. But that would need to roll back the
> > > > > riscv_alternative_fix_auipc_jalr() to your v1 version. And IMHO,
> > > > > it's better if you can split the Zbb string optimizations series
> > > > > into two: one for alternative improvements, another for Zbb. Then
> > > > > we may get the alternative improvements and this inst extension
> > > > > series merged in v6.2-rc1.
> > > >
> > > > Heiko, perhaps you can correct me here:
> > > >
> > > > Last Wednesday you & Palmer agreed that it was too late in the cycle to
> > > > apply any of the stuff touching alternatives?
> > > > If I do recall correctly, gives plenty of time to sort out any
> > > > interdependent changes here.
> > > >
> > > > Could easily be misremembering, wouldn't be the first time!
> > >
> > > You slightly misremembered, but are still correct with the above ;-) .
> > >
> > > I.e. what we talked about was stuff for fixes for 6.1-rc, were Palmers
> > > wisely wanted to limit additions to really easy fixes for the remaining
> > > last rc, to not upset any existing boards.
> >
> > Ahh right. I was 50-50 on whether something like that was said so at
> > least I am not going crazy.
> >
> > > But you are still correct that we also shouldn't target the 6.2 merge window
> > > anymore :-) .
> > >
> > > We're after -rc8 now (which is in itself uncommon) and in his -rc7
> > > announcement [0], Linus stated
> > >
> > > "[...] the usual rule is that things that I get sent for the
> > > merge window should have been all ready _before_ the merge window
> > > opened. But with the merge window happening largely during the holiday
> > > season, I'll just be enforcing that pretty strictly."
> >
> > Yah, of all the windows to land patchsets that are being re-spun a few
> > days before it opens this probably isn't the best one to pick!
> >
> > > That means new stuff should be reviewed and in linux-next _way before_ the
> > > merge window opens next weekend. Taking into account that people need
> > > to review stuff (and maybe the series needing another round), I really don't
> > > see this happening this week and everything else will get us shouted at
> > > from atop a christmas tree ;-) .
> > >
> > > That's the reason most maintainer-trees stop accepting stuff after -rc7
Thanks for the information, then we have more time to test and review
this series.
> >
> > Aye, in RISC-V land maybe we will get there one day :)
> >
> > For the original question though, breaking them up into 3 or 4 smaller
> > bits that could get applied on their own is probably a good idea?
> >
> > Between yourselves, Drew and Prabhakar there's a couple series touching
> > the same bits. Certainly don't want to seem like I am speaking for the
Because alternative is the best solution to riscv extensions while still
keep one unified kernel Image ;)
> > Higher Powers here, but some sort of logical ordering would probably be
> > a good idea so as not to hold each other up?
> > The non-string bit of your series has been fairly well reviewed & would,
> > in theory, be mergeable once the tree re-opens? Timing aside, Jisheng's
> > idea seems like a good one, no?
IMHO, it will be better if Palmer can merge Heiko's alternative improvements
into riscv-next once well reviewed and the window is reopen. Then Drew,
Prabhakar and I can rebase on that tree.
>
> yeah, I had that same thought over the weekend - with the generic
> part being pretty good in the review and only the string part needing
> more work and thus ideally splitting the series [0] .
>
> Jisheng's series just made that even more important to do :-)
>
>
> Heiko
>
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives
2022-12-06 15:02 ` Jisheng Zhang
@ 2022-12-06 16:12 ` Conor Dooley
2022-12-19 21:32 ` Conor Dooley
0 siblings, 1 reply; 52+ messages in thread
From: Conor Dooley @ 2022-12-06 16:12 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Heiko Stübner, Palmer Dabbelt, Paul Walmsley, Albert Ou,
Anup Patel, Atish Patra, Andrew Jones, linux-riscv, linux-kernel,
kvm, kvm-riscv
[-- Attachment #1: Type: text/plain, Size: 680 bytes --]
On Tue, Dec 06, 2022 at 11:02:35PM +0800, Jisheng Zhang wrote:
> > > Higher Powers here, but some sort of logical ordering would probably be
> > > a good idea so as not to hold each other up?
> > > The non-string bit of your series has been fairly well reviewed & would,
> > > in theory, be mergeable once the tree re-opens? Timing aside, Jisheng's
> > > idea seems like a good one, no?
>
> IMHO, it will be better if Palmer can merge Heiko's alternative improvements
> into riscv-next once well reviewed and the window is reopen. Then Drew,
> Prabhakar and I can rebase on that tree.
Unless I missed something, we're saying the same thing in different ways
:)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 06/13] riscv: introduce riscv_has_extension_[un]likely()
2022-12-04 17:46 ` [PATCH v2 06/13] riscv: introduce riscv_has_extension_[un]likely() Jisheng Zhang
@ 2022-12-06 20:25 ` Conor Dooley
0 siblings, 0 replies; 52+ messages in thread
From: Conor Dooley @ 2022-12-06 20:25 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
[-- Attachment #1: Type: text/plain, Size: 4023 bytes --]
Hey Jisheng,
Just a couple really minor bits here...
On Mon, Dec 05, 2022 at 01:46:25AM +0800, Jisheng Zhang wrote:
> Generally, riscv ISA extensions are fixed for any specific hardware
> platform, that's to say, the hart features won't change any more
s/that's to say, the hart/so a hart's/
s/any more//
> after booting, this chacteristic make it straightforward to use
"booting. This characteristic makes it"
> static branch to check one specific ISA extension is supported or not
"a static branch to check if a"
> 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.
>
> [1]https://lore.kernel.org/linux-riscv/20220922060958.44203-1-samuel@sholland.org/
> [2]https://lore.kernel.org/linux-arm-kernel/20220912162210.3626215-8-mark.rutland@arm.com/
Can you make these into Link: tags please (and drop the line between the
and the SoB)? So:
Link: https://lore.kernel.org/linux-riscv/20220922060958.44203-1-samuel@sholland.org/ [1]
Link: https://lore.kernel.org/linux-arm-kernel/20220912162210.3626215-8-mark.rutland@arm.com/ [2]
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Changes themselves look grand, no comments there :)
Thanks!
Conor.
> ---
> arch/riscv/include/asm/hwcap.h | 37 ++++++++++++++++++++++++++++++++++
> 1 file changed, 37 insertions(+)
>
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index 996884986fea..e2d3f6df7701 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -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>
> @@ -96,6 +97,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)
> --
> 2.37.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 08/13] riscv: module: move find_section to module.h
2022-12-04 17:46 ` [PATCH v2 08/13] riscv: module: move find_section to module.h Jisheng Zhang
2022-12-05 15:25 ` Andrew Jones
@ 2022-12-06 20:44 ` Conor Dooley
1 sibling, 0 replies; 52+ messages in thread
From: Conor Dooley @ 2022-12-06 20:44 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
[-- Attachment #1: Type: text/plain, Size: 2743 bytes --]
On Mon, Dec 05, 2022 at 01:46:27AM +0800, Jisheng Zhang wrote:
> Move it to the header so that the implementation can be shared
> by the alternatives code.
I'm not a super big fan of these perfunctory commit messages.
Maybe I'm being a bit ornery, but a few words about why the alternatives
could benefit from this would be nice. Also, s/it/find_section()/ please.
How about:
> 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.
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
> arch/riscv/include/asm/module.h | 15 +++++++++++++++
> arch/riscv/kernel/module.c | 15 ---------------
Silly question maybe, but is module.h the right place to put this?
But I have nothing better to suggest, and I hate bikeshedding stuff when
I have no suggestion to make.
Rationale behind the movement seems grand to me though, so I suppose:
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Thanks,
Conor.
> 2 files changed, 15 insertions(+), 15 deletions(-)
>
> diff --git a/arch/riscv/include/asm/module.h b/arch/riscv/include/asm/module.h
> index 76aa96a9fc08..78722a79fc59 100644
> --- a/arch/riscv/include/asm/module.h
> +++ b/arch/riscv/include/asm/module.h
> @@ -111,4 +111,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 */
> diff --git a/arch/riscv/kernel/module.c b/arch/riscv/kernel/module.c
> index 91fe16bfaa07..76f4b9c2ec5b 100644
> --- a/arch/riscv/kernel/module.c
> +++ b/arch/riscv/kernel/module.c
> @@ -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)
> --
> 2.37.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 09/13] riscv: switch to relative alternative entries
2022-12-06 14:50 ` Jisheng Zhang
@ 2022-12-06 21:43 ` Conor Dooley
0 siblings, 0 replies; 52+ messages in thread
From: Conor Dooley @ 2022-12-06 21:43 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Guo Ren, Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
[-- Attachment #1: Type: text/plain, Size: 6192 bytes --]
Hey Jisheng, Guo Ren,
On Tue, Dec 06, 2022 at 10:50:37PM +0800, Jisheng Zhang wrote:
> On Tue, Dec 06, 2022 at 12:34:40PM +0800, Guo Ren wrote:
> > On Mon, Dec 5, 2022 at 11:28 PM Jisheng Zhang <jszhang@kernel.org> wrote:
> > > On Mon, Dec 05, 2022 at 08:51:41AM +0800, Guo Ren wrote:
> > > > On Mon, Dec 5, 2022 at 1:57 AM Jisheng Zhang <jszhang@kernel.org> wrote:
> > > > >
> > > > > Instead of using absolute addresses for both the old instrucions and
> > > > > the alternative instructions, use offsets relative to the alt_entry
> > > > > values. So we can not only cut the size of the alternative entry, but
"This not only cuts"
> > > > > also meet the prerequisite for patching alternatives in the vDSO,
> > > > > since absolute alternative entries are subject to dynamic relocation,
> > > > > which is incompatible with the vDSO building.
I do this this is in the wrong order though, saving on size is
secondary to enabling their use in the vdso?
> > > > >
> > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > > > > ---
> > > > > arch/riscv/errata/sifive/errata.c | 4 +++-
> > > > > arch/riscv/errata/thead/errata.c | 11 ++++++++---
> > > > > arch/riscv/include/asm/alternative-macros.h | 20 ++++++++++----------
> > > > > arch/riscv/include/asm/alternative.h | 12 ++++++------
> > > > > arch/riscv/kernel/cpufeature.c | 13 ++++++-------
> > > > > 5 files changed, 33 insertions(+), 27 deletions(-)
> > > > >
> > > > > diff --git a/arch/riscv/errata/sifive/errata.c b/arch/riscv/errata/sifive/errata.c
> > > > > index 1031038423e7..0e537cdfd324 100644
> > > > > --- a/arch/riscv/errata/sifive/errata.c
> > > > > +++ b/arch/riscv/errata/sifive/errata.c
> > > > > @@ -107,7 +107,9 @@ 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((void *)&alt->old_offset + alt->old_offset,
> > > > > + (void *)&alt->alt_offset + alt->alt_offset,
> > > > (void *)&alt->alt_offset + alt->alt_offset. ??!!
> > >
> > > Hi Guo,
> > >
> > > what's the problem? I can't catch your meaning, could you please proide
> > > more details?
> > Can you explain why:
> >
> > alt->old_ptr = (void *)&alt->old_offset + alt->old_offset
>
> Hi,
>
> when constructing the alt entry, we save the offset in
> then entry as below:
>
> .long \oldptr - .
>
> So we can restore the old_ptr by &alt->old_offset + alt->old_offset
Please correct me if I have misunderstood, but for stuff like this I
find it useful to kinda summarise a bit and figure out for myself what
is going on..
As things stand, we have absolute "locations" for the alternative and
"old" instructions/data/functions. Your commit is converting us over to
use offsets. The code that patches in the alternatives needs to have
absolute addresses though, so you need to be able to, effectively,
reverse engineer those from the offset.
You do this by taking the address of the offset & adding the offset to
the address before casting to (void *). This works, because the offset
is the offset from the alt_entry data structure to the alternative?
I hope I am in the right ballpark there haha, but I do think that this
really needs a comment explaining what it is doing. Maybe extract that
operation into some sort of macro in alternatives.h so the operation is
done in a central location & you can leave the comment there?
That'd make it at least more manageable for us mere mortals who can just
do something like
patch_text_nosync(ALT_OFFSET_ADDRESS(alt->old_offset),
ALT_OFFSET_ADDRESS(alt->alt_offset),
alt->alt_len);
when we have to go an add some alternatives..
> >
> > | offset | <- &offset
> > | ... |
> > | value | <- ptr = &offset + offset
> >
> > I don't make sense of the above.
> >
> > >
> > > Thanks
> > >
> > > >
> > > > > + alt->alt_len);
> > > > > cpu_apply_errata |= tmp;
> > > > > }
> > > > > }
> > > > > diff --git a/arch/riscv/errata/thead/errata.c b/arch/riscv/errata/thead/errata.c
> > > > > index 21546937db39..2a6e335b5a32 100644
> > > > > --- a/arch/riscv/errata/thead/errata.c
> > > > > +++ b/arch/riscv/errata/thead/errata.c
> > > > > @@ -68,6 +68,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, *updptr;
Why mix the terminology with "upd" instead of "alt"?
> > > > >
> > > > > for (alt = begin; alt < end; alt++) {
> > > > > if (alt->vendor_id != THEAD_VENDOR_ID)
> > > > > 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 to original instruciton or data */
> > > > > + s32 alt_offset; /* offset to replacement instruction or data */
Perhaps also this comment could be expanded on to specify what it is an
offset *from* as well as to?
> > > > > + u16 vendor_id; /* cpu vendor id */
> > > > > + u16 alt_len; /* The replacement size */
> > > > > + u32 errata_id; /* The errata id */
> > > > > +};
I hope I didn't make a hames of trying to understand what you were
doing, but please let me know what I have undoubtedly got mixed up on!
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 11/13] riscv: cpu_relax: switch to riscv_has_extension_likely()
2022-12-04 17:46 ` [PATCH v2 11/13] riscv: cpu_relax: switch to riscv_has_extension_likely() Jisheng Zhang
2022-12-05 0:52 ` Guo Ren
@ 2022-12-06 22:04 ` Conor Dooley
1 sibling, 0 replies; 52+ messages in thread
From: Conor Dooley @ 2022-12-06 22:04 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
[-- Attachment #1: Type: text/plain, Size: 1057 bytes --]
On Mon, Dec 05, 2022 at 01:46:30AM +0800, Jisheng Zhang wrote:
> Switch cpu_relax() from statich branch to the new helper
The tiniest nit: static
> riscv_has_extension_likely()
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> ---
> arch/riscv/include/asm/vdso/processor.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/riscv/include/asm/vdso/processor.h b/arch/riscv/include/asm/vdso/processor.h
> index fa70cfe507aa..edf0e25e43d1 100644
> --- a/arch/riscv/include/asm/vdso/processor.h
> +++ b/arch/riscv/include/asm/vdso/processor.h
> @@ -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. */
> --
> 2.37.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 13/13] riscv: remove riscv_isa_ext_keys[] array and related usage
2022-12-04 17:46 ` [PATCH v2 13/13] riscv: remove riscv_isa_ext_keys[] array and related usage Jisheng Zhang
2022-12-05 0:53 ` Guo Ren
@ 2022-12-06 22:16 ` Conor Dooley
1 sibling, 0 replies; 52+ messages in thread
From: Conor Dooley @ 2022-12-06 22:16 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
[-- Attachment #1: Type: text/plain, Size: 3230 bytes --]
On Mon, Dec 05, 2022 at 01:46:32AM +0800, Jisheng Zhang wrote:
> All users have switched to riscv_has_extension_*, removed unused
minor nit: remove
> definitions, vars and related setting code.
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> Reviewed-by: Heiko Stuebner <heiko@sntech.de>
May as well join the R-b club here, the removal of 2 places where
extensions need to be kept ordered is especially appreciated!
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>
Thanks!
> ---
> arch/riscv/include/asm/hwcap.h | 30 ------------------------------
> arch/riscv/kernel/cpufeature.c | 9 ---------
> 2 files changed, 39 deletions(-)
>
> diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> index e2d3f6df7701..be00a4337578 100644
> --- a/arch/riscv/include/asm/hwcap.h
> +++ b/arch/riscv/include/asm/hwcap.h
> @@ -60,18 +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];
> @@ -79,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)
> {
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index adeac90b1d8e..3240a2915bf1 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -28,9 +28,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
> *
> @@ -243,12 +240,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
> --
> 2.37.2
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives
2022-12-06 16:12 ` Conor Dooley
@ 2022-12-19 21:32 ` Conor Dooley
0 siblings, 0 replies; 52+ messages in thread
From: Conor Dooley @ 2022-12-19 21:32 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Heiko Stübner, Palmer Dabbelt, Paul Walmsley, Albert Ou,
Anup Patel, Atish Patra, Andrew Jones, linux-riscv, linux-kernel,
kvm, kvm-riscv
[-- Attachment #1: Type: text/plain, Size: 1058 bytes --]
On Tue, Dec 06, 2022 at 04:12:35PM +0000, Conor Dooley wrote:
> On Tue, Dec 06, 2022 at 11:02:35PM +0800, Jisheng Zhang wrote:
>
> > > > Higher Powers here, but some sort of logical ordering would probably be
> > > > a good idea so as not to hold each other up?
> > > > The non-string bit of your series has been fairly well reviewed & would,
> > > > in theory, be mergeable once the tree re-opens? Timing aside, Jisheng's
> > > > idea seems like a good one, no?
> >
> > IMHO, it will be better if Palmer can merge Heiko's alternative improvements
> > into riscv-next once well reviewed and the window is reopen. Then Drew,
> > Prabhakar and I can rebase on that tree.
>
> Unless I missed something, we're saying the same thing in different ways
> :)
Hey Jisheng,
FYI I'm gonna mark this version of the patchset as "Changes Requested"
in patchwork. Palmer's said he'll apply Heiko's patchset that this
depends on once rc1 is out so I am expecting that you'll rebase on top
of that with the various comments fixed.
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 04/13] riscv: hwcap: make ISA extension ids can be used in asm
2022-12-05 18:53 ` Conor Dooley
@ 2022-12-22 22:58 ` Conor Dooley
0 siblings, 0 replies; 52+ messages in thread
From: Conor Dooley @ 2022-12-22 22:58 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, Andrew Jones, linux-riscv,
linux-kernel, kvm, kvm-riscv
[-- Attachment #1: Type: text/plain, Size: 3840 bytes --]
On Mon, Dec 05, 2022 at 06:53:53PM +0000, Conor Dooley wrote:
> Hey Jisheng,
>
> On Mon, Dec 05, 2022 at 01:46:23AM +0800, Jisheng Zhang wrote:
> > We will make use of ISA extension in asm files, so make the multi-letter
> > RISC-V ISA extension IDs macros rather than enums and move them and
> > those base ISA extension IDs to suitable place.
>
> Which base ISA extension IDs? Changelog should match the patch contents,
> and it's a little unclear here since the base ISA extension IDs are
> visible here but in the context not the diff.
>
> >
> > Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> > Reviewed-by: Heiko Stuebner <heiko@sntech.de>
> > Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
> > ---
> > arch/riscv/include/asm/hwcap.h | 43 ++++++++++++++++------------------
> > 1 file changed, 20 insertions(+), 23 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/hwcap.h b/arch/riscv/include/asm/hwcap.h
> > index b22525290073..996884986fea 100644
> > --- a/arch/riscv/include/asm/hwcap.h
> > +++ b/arch/riscv/include/asm/hwcap.h
> > @@ -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 = RISCV_ISA_EXT_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
>
> Could you re-order these alphabetically when you move them please?
On reflection, this is a horrific idea - don't bother. It'd only be
temporary anyway as it'd need massaging when the next extension comes
along.
Either people will have a slightly harder seeing if something is added,
or a "slightly" harder time finding which is the next free number or
have to reshuffle the whole thing.
The latter sounds like a prime bug breeding ground, while the former is
just a search away.
So yeah, don't bother & apologies for the noise!
> > +
> > +#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;
> > +
> > /*
> > * 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
> > --
> > 2.37.2
> >
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v2 10/13] riscv: alternative: patch alternatives in the vDSO
2022-12-04 17:46 ` [PATCH v2 10/13] riscv: alternative: patch alternatives in the vDSO Jisheng Zhang
2022-12-05 1:56 ` Guo Ren
@ 2023-01-11 14:12 ` Andrew Jones
1 sibling, 0 replies; 52+ messages in thread
From: Andrew Jones @ 2023-01-11 14:12 UTC (permalink / raw)
To: Jisheng Zhang
Cc: Palmer Dabbelt, Paul Walmsley, Albert Ou, Anup Patel,
Atish Patra, Heiko Stuebner, linux-riscv, linux-kernel, kvm,
kvm-riscv
On Mon, Dec 05, 2022 at 01:46:29AM +0800, Jisheng Zhang wrote:
> Make it possible to use alternatives in the vDSO, so that better
> implementations can be used if possible.
>
> Signed-off-by: Jisheng Zhang <jszhang@kernel.org>
> ---
> arch/riscv/include/asm/vdso.h | 4 ++++
> arch/riscv/kernel/alternative.c | 25 +++++++++++++++++++++++++
> arch/riscv/kernel/vdso.c | 5 -----
> arch/riscv/kernel/vdso/vdso.lds.S | 7 +++++++
> 4 files changed, 36 insertions(+), 5 deletions(-)
>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply [flat|nested] 52+ messages in thread
end of thread, other threads:[~2023-01-11 14:12 UTC | newest]
Thread overview: 52+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-04 17:46 [PATCH v2 00/13] riscv: improve boot time isa extensions handling Jisheng Zhang
2022-12-04 17:46 ` [PATCH v2 01/13] riscv: fix jal offsets in patched alternatives Jisheng Zhang
2022-12-05 14:57 ` Andrew Jones
2022-12-05 15:34 ` Jisheng Zhang
2022-12-05 16:42 ` Jisheng Zhang
2022-12-05 16:49 ` Jisheng Zhang
2022-12-06 5:50 ` Andrew Jones
2022-12-05 15:31 ` Heiko Stübner
2022-12-05 15:40 ` Jisheng Zhang
2022-12-05 18:36 ` Conor Dooley
2022-12-05 18:49 ` Heiko Stübner
2022-12-05 19:49 ` Conor Dooley
2022-12-06 0:39 ` Heiko Stübner
2022-12-06 15:02 ` Jisheng Zhang
2022-12-06 16:12 ` Conor Dooley
2022-12-19 21:32 ` Conor Dooley
2022-12-04 17:46 ` [PATCH v2 02/13] riscv: move riscv_noncoherent_supported() out of ZICBOM probe Jisheng Zhang
2022-12-04 21:52 ` Heiko Stübner
2022-12-05 15:16 ` Jisheng Zhang
2022-12-05 15:31 ` Conor Dooley
2022-12-04 17:46 ` [PATCH v2 03/13] riscv: cpufeature: detect RISCV_ALTERNATIVES_EARLY_BOOT earlier Jisheng Zhang
2022-12-05 19:09 ` Conor Dooley
2022-12-04 17:46 ` [PATCH v2 04/13] riscv: hwcap: make ISA extension ids can be used in asm Jisheng Zhang
2022-12-05 18:53 ` Conor Dooley
2022-12-22 22:58 ` Conor Dooley
2022-12-04 17:46 ` [PATCH v2 05/13] riscv: cpufeature: extend riscv_cpufeature_patch_func to all ISA extensions Jisheng Zhang
2022-12-05 19:37 ` Conor Dooley
2022-12-04 17:46 ` [PATCH v2 06/13] riscv: introduce riscv_has_extension_[un]likely() Jisheng Zhang
2022-12-06 20:25 ` Conor Dooley
2022-12-04 17:46 ` [PATCH v2 07/13] riscv: fpu: switch has_fpu() to riscv_has_extension_likely() Jisheng Zhang
2022-12-04 17:46 ` [PATCH v2 08/13] riscv: module: move find_section to module.h Jisheng Zhang
2022-12-05 15:25 ` Andrew Jones
2022-12-06 20:44 ` Conor Dooley
2022-12-04 17:46 ` [PATCH v2 09/13] riscv: switch to relative alternative entries Jisheng Zhang
2022-12-05 0:51 ` Guo Ren
2022-12-05 15:18 ` Jisheng Zhang
2022-12-06 4:34 ` Guo Ren
2022-12-06 14:50 ` Jisheng Zhang
2022-12-06 21:43 ` Conor Dooley
2022-12-04 17:46 ` [PATCH v2 10/13] riscv: alternative: patch alternatives in the vDSO Jisheng Zhang
2022-12-05 1:56 ` Guo Ren
2022-12-05 15:23 ` Jisheng Zhang
2022-12-06 4:29 ` Guo Ren
2023-01-11 14:12 ` Andrew Jones
2022-12-04 17:46 ` [PATCH v2 11/13] riscv: cpu_relax: switch to riscv_has_extension_likely() Jisheng Zhang
2022-12-05 0:52 ` Guo Ren
2022-12-06 22:04 ` Conor Dooley
2022-12-04 17:46 ` [PATCH v2 12/13] riscv: KVM: Switch has_svinval() to riscv_has_extension_unlikely() Jisheng Zhang
2022-12-05 0:52 ` Guo Ren
2022-12-04 17:46 ` [PATCH v2 13/13] riscv: remove riscv_isa_ext_keys[] array and related usage Jisheng Zhang
2022-12-05 0:53 ` Guo Ren
2022-12-06 22:16 ` Conor Dooley
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).