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/[email protected]/
[2]https://lore.kernel.org/linux-arm-kernel/[email protected]/
[3]https://lore.kernel.org/linux-riscv/[email protected]/
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
make the riscv_cpufeature_patch_func() scan all ISA extensions rather
than limited feature macros.
Signed-off-by: Jisheng Zhang <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
Reviewed-by: Heiko Stuebner <[email protected]>
---
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
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 <[email protected]>
Reviewed-by: Heiko Stuebner <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
---
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
Make it possible to use alternatives in the vDSO, so that better
implementations can be used if possible.
Signed-off-by: Jisheng Zhang <[email protected]>
---
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
Switch has_fpu() from statich branch to the new helper
riscv_has_extension_likely().
Signed-off-by: Jisheng Zhang <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
Reviewed-by: Heiko Stuebner <[email protected]>
---
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
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 <[email protected]> wrote:
>
> Make it possible to use alternatives in the vDSO, so that better
> implementations can be used if possible.
>
> Signed-off-by: Jisheng Zhang <[email protected]>
> ---
> 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
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 <[email protected]> wrote:
> >
> > Make it possible to use alternatives in the vDSO, so that better
> > implementations can be used if possible.
> >
> > Signed-off-by: Jisheng Zhang <[email protected]>
> > ---
> > 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
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 <[email protected]>
> Reviewed-by: Heiko Stuebner <[email protected]>
> Reviewed-by: Andrew Jones <[email protected]>
> ---
> 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
>
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 <[email protected]>
> Reviewed-by: Andrew Jones <[email protected]>
> Reviewed-by: Heiko Stuebner <[email protected]>
> ---
> 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.
On Mon, Dec 5, 2022 at 11:33 PM Jisheng Zhang <[email protected]> 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 <[email protected]>
>
> Thanks
>
> >
> > On Mon, Dec 5, 2022 at 1:57 AM Jisheng Zhang <[email protected]> wrote:
> > >
> > > Make it possible to use alternatives in the vDSO, so that better
> > > implementations can be used if possible.
> > >
> > > Signed-off-by: Jisheng Zhang <[email protected]>
> > > ---
> > > 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
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 <[email protected]>
> > Reviewed-by: Heiko Stuebner <[email protected]>
> > Reviewed-by: Andrew Jones <[email protected]>
> > ---
> > 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
> >
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 <[email protected]>
> ---
> 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 <[email protected]>