On Fri, May 03, 2024 at 10:13:33AM -0700, Evan Green wrote:
On Fri, May 3, 2024 at 10:08 AM Charlie Jenkins charlie@rivosinc.com wrote:
On Fri, May 03, 2024 at 09:28:24AM -0700, Evan Green wrote:
On Thu, May 2, 2024 at 9:46 PM Charlie Jenkins charlie@rivosinc.com wrote:
Separate vendor extensions out into one struct per vendor instead of adding vendor extensions onto riscv_isa_ext.
Add a hidden config RISCV_ISA_VENDOR_EXT to conditionally include this code.
The xtheadvector vendor extension is added using these changes.
Signed-off-by: Charlie Jenkins charlie@rivosinc.com
arch/riscv/Kconfig | 2 + arch/riscv/Kconfig.vendor | 19 +++++ arch/riscv/include/asm/cpufeature.h | 18 +++++ arch/riscv/include/asm/vendor_extensions.h | 34 +++++++++ arch/riscv/include/asm/vendor_extensions/thead.h | 16 ++++ arch/riscv/kernel/Makefile | 2 + arch/riscv/kernel/cpufeature.c | 93 +++++++++++++++++++----- arch/riscv/kernel/vendor_extensions.c | 18 +++++ arch/riscv/kernel/vendor_extensions/Makefile | 3 + arch/riscv/kernel/vendor_extensions/thead.c | 18 +++++ 10 files changed, 203 insertions(+), 20 deletions(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig index be09c8836d56..fec86fba3acd 100644 --- a/arch/riscv/Kconfig +++ b/arch/riscv/Kconfig @@ -759,6 +759,8 @@ config RISCV_EFFICIENT_UNALIGNED_ACCESS
endchoice
+source "arch/riscv/Kconfig.vendor"
endmenu # "Platform type"
menu "Kernel features" diff --git a/arch/riscv/Kconfig.vendor b/arch/riscv/Kconfig.vendor new file mode 100644 index 000000000000..85ac30496b0e --- /dev/null +++ b/arch/riscv/Kconfig.vendor @@ -0,0 +1,19 @@ +menu "Vendor extensions"
+config RISCV_ISA_VENDOR_EXT
bool+menu "T-Head" +config RISCV_ISA_VENDOR_EXT_THEAD
bool "T-Head vendor extension support"select RISCV_ISA_VENDOR_EXTdefault yhelpSay N here to disable detection of and support for all T-Head vendorextensions. Without this option enabled, T-Head vendor extensions willnot be detected at boot and their presence not reported to userspace.If you don't know what to do here, say Y.+endmenu
+endmenu diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h index 0c4f08577015..fedd479ccfd1 100644 --- a/arch/riscv/include/asm/cpufeature.h +++ b/arch/riscv/include/asm/cpufeature.h @@ -35,6 +35,24 @@ extern u32 riscv_vlenb_of;
void riscv_user_isa_enable(void);
+#define _RISCV_ISA_EXT_DATA(_name, _id, _subset_exts, _subset_exts_size) { \
.name = #_name, \.property = #_name, \.id = _id, \.subset_ext_ids = _subset_exts, \.subset_ext_size = _subset_exts_size \+}
+#define __RISCV_ISA_EXT_DATA(_name, _id) _RISCV_ISA_EXT_DATA(_name, _id, NULL, 0)
+/* Used to declare pure "lasso" extension (Zk for instance) */ +#define __RISCV_ISA_EXT_BUNDLE(_name, _bundled_exts) \
_RISCV_ISA_EXT_DATA(_name, RISCV_ISA_EXT_INVALID, _bundled_exts, ARRAY_SIZE(_bundled_exts))+/* Used to declare extensions that are a superset of other extensions (Zvbb for instance) */ +#define __RISCV_ISA_EXT_SUPERSET(_name, _id, _sub_exts) \
_RISCV_ISA_EXT_DATA(_name, _id, _sub_exts, ARRAY_SIZE(_sub_exts))#if defined(CONFIG_RISCV_MISALIGNED) bool check_unaligned_access_emulated_all_cpus(void); void unaligned_emulation_finish(void); diff --git a/arch/riscv/include/asm/vendor_extensions.h b/arch/riscv/include/asm/vendor_extensions.h new file mode 100644 index 000000000000..bf4dac66e6e6 --- /dev/null +++ b/arch/riscv/include/asm/vendor_extensions.h @@ -0,0 +1,34 @@ +/* SPDX-License-Identifier: GPL-2.0-only */ +/*
- Copyright 2024 Rivos, Inc
- */
+#ifndef _ASM_VENDOR_EXTENSIONS_H +#define _ASM_VENDOR_EXTENSIONS_H
+#include <asm/cpufeature.h>
+#include <linux/array_size.h> +#include <linux/types.h>
+/*
- The extension keys of each vendor must be strictly less than this value.
- */
+#define RISCV_ISA_VENDOR_EXT_MAX 32
+struct riscv_isavendorinfo {
DECLARE_BITMAP(isa, RISCV_ISA_VENDOR_EXT_MAX);+};
Nice, I think this was a good compromise: being honest with the compiler about the fixed array sizes, with the tradeoff that all vendors have to use the same ceiling for the number of bits. If one vendor raises this ceiling absurdly and starts creating huge amounts of waste we can revisit.
+struct riscv_isa_vendor_ext_data_list {
const size_t ext_data_count;const struct riscv_isa_ext_data *ext_data;struct riscv_isavendorinfo per_hart_isa_bitmap[NR_CPUS];struct riscv_isavendorinfo all_harts_isa_bitmap;+};
+extern struct riscv_isa_vendor_ext_data_list *riscv_isa_vendor_ext_list[];
+extern const size_t riscv_isa_vendor_ext_list_size;
+#endif /* _ASM_VENDOR_EXTENSIONS_H */ diff --git a/arch/riscv/include/asm/vendor_extensions/thead.h b/arch/riscv/include/asm/vendor_extensions/thead.h new file mode 100644 index 000000000000..48421d1553ad --- /dev/null +++ b/arch/riscv/include/asm/vendor_extensions/thead.h @@ -0,0 +1,16 @@ +/* SPDX-License-Identifier: GPL-2.0 */ +#ifndef _ASM_RISCV_VENDOR_EXTENSIONS_THEAD_H +#define _ASM_RISCV_VENDOR_EXTENSIONS_THEAD_H
+#include <asm/vendor_extensions.h>
+#include <linux/types.h>
+/*
- Extension keys must be strictly less than RISCV_ISA_VENDOR_EXT_MAX.
- */
+#define RISCV_ISA_VENDOR_EXT_XTHEADVECTOR 0
+extern struct riscv_isa_vendor_ext_data_list riscv_isa_vendor_ext_list_thead;
+#endif diff --git a/arch/riscv/kernel/Makefile b/arch/riscv/kernel/Makefile index 81d94a8ee10f..53361c50fb46 100644 --- a/arch/riscv/kernel/Makefile +++ b/arch/riscv/kernel/Makefile @@ -58,6 +58,8 @@ obj-y += riscv_ksyms.o obj-y += stacktrace.o obj-y += cacheinfo.o obj-y += patch.o +obj-y += vendor_extensions.o +obj-y += vendor_extensions/ obj-y += probes/ obj-y += tests/ obj-$(CONFIG_MMU) += vdso.o vdso/ diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index 12c79db0b0bb..cc9ec393c8f6 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -24,6 +24,7 @@ #include <asm/processor.h> #include <asm/sbi.h> #include <asm/vector.h> +#include <asm/vendor_extensions.h>
#define NUM_ALPHA_EXTS ('z' - 'a' + 1)
@@ -102,24 +103,6 @@ static bool riscv_isa_extension_check(int id) return true; }
-#define _RISCV_ISA_EXT_DATA(_name, _id, _subset_exts, _subset_exts_size) { \
.name = #_name, \.property = #_name, \.id = _id, \.subset_ext_ids = _subset_exts, \.subset_ext_size = _subset_exts_size \-}
-#define __RISCV_ISA_EXT_DATA(_name, _id) _RISCV_ISA_EXT_DATA(_name, _id, NULL, 0)
-/* Used to declare pure "lasso" extension (Zk for instance) */ -#define __RISCV_ISA_EXT_BUNDLE(_name, _bundled_exts) \
_RISCV_ISA_EXT_DATA(_name, RISCV_ISA_EXT_INVALID, _bundled_exts, ARRAY_SIZE(_bundled_exts))-/* Used to declare extensions that are a superset of other extensions (Zvbb for instance) */ -#define __RISCV_ISA_EXT_SUPERSET(_name, _id, _sub_exts) \
_RISCV_ISA_EXT_DATA(_name, _id, _sub_exts, ARRAY_SIZE(_sub_exts))static const unsigned int riscv_zk_bundled_exts[] = { RISCV_ISA_EXT_ZBKB, RISCV_ISA_EXT_ZBKC, @@ -353,6 +336,21 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc bool ext_long = false, ext_err = false;
switch (*ext) {
case 'x':case 'X':if (acpi_disabled)pr_warn_once("Vendor extensions are ignored in riscv,isa. Use riscv,isa-extensions instead.");/** To skip an extension, we find its end.* As multi-letter extensions must be split from other multi-letter* extensions with an "_", the end of a multi-letter extension will* either be the null character or the "_" at the start of the next* multi-letter extension.*/for (; *isa && *isa != '_'; ++isa);ext_err = true;break; case 's': /* * Workaround for invalid single-letter 's' & 'u' (QEMU).@@ -368,8 +366,6 @@ static void __init riscv_parse_isa_string(unsigned long *this_hwcap, struct risc } fallthrough; case 'S':
case 'x':case 'X': case 'z': case 'Z': /*@@ -572,6 +568,59 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap) acpi_put_table((struct acpi_table_header *)rhct); }
+static void __init riscv_fill_cpu_vendor_ext(struct device_node *cpu_node, int cpu) +{
if (!IS_ENABLED(CONFIG_RISCV_ISA_VENDOR_EXT))return;for (int i = 0; i < riscv_isa_vendor_ext_list_size; i++) {struct riscv_isa_vendor_ext_data_list *ext_list = riscv_isa_vendor_ext_list[i];for (int j = 0; j < ext_list->ext_data_count; j++) {const struct riscv_isa_ext_data ext = ext_list->ext_data[j];struct riscv_isavendorinfo *isavendorinfo = &ext_list->per_hart_isa_bitmap[cpu];if (of_property_match_string(cpu_node, "riscv,isa-extensions",ext.property) < 0)continue;/** Assume that subset extensions are all members of the* same vendor.*/if (ext.subset_ext_size)for (int k = 0; k < ext.subset_ext_size; k++)set_bit(ext.subset_ext_ids[k], isavendorinfo->isa);set_bit(ext.id, isavendorinfo->isa);}}+}
+static void __init riscv_fill_vendor_ext_list(int cpu) +{
bool first = true;if (!IS_ENABLED(CONFIG_RISCV_ISA_VENDOR_EXT))return;for (int i = 0; i < riscv_isa_vendor_ext_list_size; i++) {struct riscv_isa_vendor_ext_data_list *ext_list = riscv_isa_vendor_ext_list[i];if (first) {bitmap_copy(ext_list->all_harts_isa_bitmap.isa,ext_list->per_hart_isa_bitmap[cpu].isa,RISCV_ISA_VENDOR_EXT_MAX);first = false;I think this is still not quite right. This behaves properly when called on the first cpu (let's say 0), but then we call it again with cpu 1, first gets set to true, and we clobber the old work we did for cpu 0. If we knew that cpu 0 was always called first (this looks true since both calls are within a for_each_possible_cpu() loop), and that this was only called once at boot for cpu 0 (looks true, and riscv_fill_hwcap() is __init), then it could be bool first = cpu == 0.
Assuming that the first cpu is 0 should be safe, but to eliminate that assumption we can pass in a "first" parameter into this function that will only be true the first time this function is called. We also keep "first = false" in this function so the copy only happens on the first iteration of the first cpu.
Yeah, though then you still have to maintain that the function passing true as the first parameter really is the first invocation.
static bool first = true would also fix it, maybe more reliably. Are static locals allowed in the kernel?
That's even better! I did a search and saw many uses of static locals, so let's go with that solution. I also tested it locally with a DT with two CPUs that have two different vendor extensions from the same vendor and verified that the all_hart_isa_bitmap did not contain either of the vendor extensions. I don't think it's possible to have an automated test case like that in the kernel.
- Charlie
- Charlie