This series contains two minor fixes for the extension parsing in
cpufeature.c.
Some T-Head boards without vector 1.0 support report "v" in the isa
string in their DT which will cause the kernel to run vector code. The
code to blacklist "v" from these boards was doing so by using
riscv_cached_mvendorid() which has not been populated at the time of
extension parsing. This fix instead greedily reads the mvendorid CSR of
the boot hart to determine if the cpu is from T-Head.
The other fix is for an incorrect indexing bug. riscv extensions
sometimes imply other extensions. When adding these "subset" extensions
to the harware capabilities array, they need to be checked if they are
valid. The current code only checks if the extension that is including
other extensions is valid and not the subset extensions.
These patches were previously included in:
https://lore.kernel.org/lkml/20240420-dev-charlie-support_thead_vector_6_9-v3-0-67cff4271d1d@rivosinc.com/
Signed-off-by: Charlie Jenkins <[email protected]>
---
Charlie Jenkins (2):
riscv: cpufeature: Fix thead vector hwcap removal
riscv: cpufeature: Fix extension subset checking
arch/riscv/include/asm/sbi.h | 2 ++
arch/riscv/kernel/cpu.c | 40 ++++++++++++++++++++++++++++++++++++----
arch/riscv/kernel/cpufeature.c | 14 +++++++++++---
3 files changed, 49 insertions(+), 7 deletions(-)
---
base-commit: ed30a4a51bb196781c8058073ea720133a65596f
change-id: 20240424-cpufeature_fixes-91bec4d793e7
--
- Charlie
This loop is supposed to check if ext->subset_ext_ids[j] is valid, rather
than if ext->subset_ext_ids[i] is valid, before setting the extension
id ext->subset_ext_ids[j] in isainfo->isa.
Signed-off-by: Charlie Jenkins <[email protected]>
Reviewed-by: Conor Dooley <[email protected]>
Reviewed-by: Alexandre Ghiti <[email protected]>
Fixes: 0d8295ed975b ("riscv: add ISA extension parsing for scalar crypto")
---
arch/riscv/kernel/cpufeature.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index c6e27b45e192..6dff7bb1db3f 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -607,7 +607,7 @@ static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap)
if (ext->subset_ext_size) {
for (int j = 0; j < ext->subset_ext_size; j++) {
- if (riscv_isa_extension_check(ext->subset_ext_ids[i]))
+ if (riscv_isa_extension_check(ext->subset_ext_ids[j]))
set_bit(ext->subset_ext_ids[j], isainfo->isa);
}
}
--
2.44.0
The riscv_cpuinfo struct that contains mvendorid and marchid is not
populated until all harts are booted which happens after the DT parsing.
Use the vendorid/archid values from the DT if available or assume all
harts have the same values as the boot hart as a fallback.
Fixes: d82f32202e0d ("RISC-V: Ignore V from the riscv,isa DT property on older T-Head CPUs")
Signed-off-by: Charlie Jenkins <[email protected]>
Reviewed-by: Conor Dooley <[email protected]>
---
arch/riscv/include/asm/sbi.h | 2 ++
arch/riscv/kernel/cpu.c | 40 ++++++++++++++++++++++++++++++++++++----
arch/riscv/kernel/cpufeature.c | 12 ++++++++++--
3 files changed, 48 insertions(+), 6 deletions(-)
diff --git a/arch/riscv/include/asm/sbi.h b/arch/riscv/include/asm/sbi.h
index 6e68f8dff76b..0fab508a65b3 100644
--- a/arch/riscv/include/asm/sbi.h
+++ b/arch/riscv/include/asm/sbi.h
@@ -370,6 +370,8 @@ static inline int sbi_remote_fence_i(const struct cpumask *cpu_mask) { return -1
static inline void sbi_init(void) {}
#endif /* CONFIG_RISCV_SBI */
+unsigned long riscv_get_mvendorid(void);
+unsigned long riscv_get_marchid(void);
unsigned long riscv_cached_mvendorid(unsigned int cpu_id);
unsigned long riscv_cached_marchid(unsigned int cpu_id);
unsigned long riscv_cached_mimpid(unsigned int cpu_id);
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index d11d6320fb0d..c1f3655238fd 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -139,6 +139,34 @@ int riscv_of_parent_hartid(struct device_node *node, unsigned long *hartid)
return -1;
}
+unsigned long __init riscv_get_marchid(void)
+{
+ struct riscv_cpuinfo *ci = this_cpu_ptr(&riscv_cpuinfo);
+
+#if IS_ENABLED(CONFIG_RISCV_SBI)
+ ci->marchid = sbi_spec_is_0_1() ? 0 : sbi_get_marchid();
+#elif IS_ENABLED(CONFIG_RISCV_M_MODE)
+ ci->marchid = csr_read(CSR_MARCHID);
+#else
+ ci->marchid = 0;
+#endif
+ return ci->marchid;
+}
+
+unsigned long __init riscv_get_mvendorid(void)
+{
+ struct riscv_cpuinfo *ci = this_cpu_ptr(&riscv_cpuinfo);
+
+#if IS_ENABLED(CONFIG_RISCV_SBI)
+ ci->mvendorid = sbi_spec_is_0_1() ? 0 : sbi_get_mvendorid();
+#elif IS_ENABLED(CONFIG_RISCV_M_MODE)
+ ci->mvendorid = csr_read(CSR_MVENDORID);
+#else
+ ci->mvendorid = 0;
+#endif
+ return ci->mvendorid;
+}
+
DEFINE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo);
unsigned long riscv_cached_mvendorid(unsigned int cpu_id)
@@ -170,12 +198,16 @@ static int riscv_cpuinfo_starting(unsigned int cpu)
struct riscv_cpuinfo *ci = this_cpu_ptr(&riscv_cpuinfo);
#if IS_ENABLED(CONFIG_RISCV_SBI)
- ci->mvendorid = sbi_spec_is_0_1() ? 0 : sbi_get_mvendorid();
- ci->marchid = sbi_spec_is_0_1() ? 0 : sbi_get_marchid();
+ if (!ci->mvendorid)
+ ci->mvendorid = sbi_spec_is_0_1() ? 0 : sbi_get_mvendorid();
+ if (!ci->marchid)
+ ci->marchid = sbi_spec_is_0_1() ? 0 : sbi_get_marchid();
ci->mimpid = sbi_spec_is_0_1() ? 0 : sbi_get_mimpid();
#elif IS_ENABLED(CONFIG_RISCV_M_MODE)
- ci->mvendorid = csr_read(CSR_MVENDORID);
- ci->marchid = csr_read(CSR_MARCHID);
+ if (!ci->mvendorid)
+ ci->mvendorid = csr_read(CSR_MVENDORID);
+ if (!ci->marchid)
+ ci->marchid = csr_read(CSR_MARCHID);
ci->mimpid = csr_read(CSR_MIMPID);
#else
ci->mvendorid = 0;
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 3ed2359eae35..c6e27b45e192 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -490,6 +490,8 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap)
struct acpi_table_header *rhct;
acpi_status status;
unsigned int cpu;
+ u64 boot_vendorid;
+ u64 boot_archid;
if (!acpi_disabled) {
status = acpi_get_table(ACPI_SIG_RHCT, 0, &rhct);
@@ -497,6 +499,13 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap)
return;
}
+ /*
+ * Naively assume that all harts have the same mvendorid/marchid as the
+ * boot hart.
+ */
+ boot_vendorid = riscv_get_mvendorid();
+ boot_archid = riscv_get_marchid();
+
for_each_possible_cpu(cpu) {
struct riscv_isainfo *isainfo = &hart_isa[cpu];
unsigned long this_hwcap = 0;
@@ -544,8 +553,7 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap)
* CPU cores with the ratified spec will contain non-zero
* marchid.
*/
- if (acpi_disabled && riscv_cached_mvendorid(cpu) == THEAD_VENDOR_ID &&
- riscv_cached_marchid(cpu) == 0x0) {
+ if (acpi_disabled && boot_vendorid == THEAD_VENDOR_ID && boot_archid == 0x0) {
this_hwcap &= ~isa2hwcap[RISCV_ISA_EXT_v];
clear_bit(RISCV_ISA_EXT_v, isainfo->isa);
}
--
2.44.0
On Wed, Apr 24, 2024 at 09:19:54AM GMT, Charlie Jenkins wrote:
..
> + /*
> + * Naively assume that all harts have the same mvendorid/marchid as the
> + * boot hart.
This comment isn't necessary, since we don't have to assume all harts have
the same IDs. This information is currently being collected specifically
for thead. So, we can state in a comment below, where the information is
used, that we assume when the boot hart is thead, then we don't want to
enable V on any hart (whatever their IDs are).
Thanks,
drew
> + */
> + boot_vendorid = riscv_get_mvendorid();
> + boot_archid = riscv_get_marchid();
> +
> for_each_possible_cpu(cpu) {
> struct riscv_isainfo *isainfo = &hart_isa[cpu];
> unsigned long this_hwcap = 0;
> @@ -544,8 +553,7 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap)
> * CPU cores with the ratified spec will contain non-zero
> * marchid.
> */
> - if (acpi_disabled && riscv_cached_mvendorid(cpu) == THEAD_VENDOR_ID &&
> - riscv_cached_marchid(cpu) == 0x0) {
> + if (acpi_disabled && boot_vendorid == THEAD_VENDOR_ID && boot_archid == 0x0) {
> this_hwcap &= ~isa2hwcap[RISCV_ISA_EXT_v];
> clear_bit(RISCV_ISA_EXT_v, isainfo->isa);
> }
>
> --
> 2.44.0
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Wed, Apr 24, 2024 at 09:19:55AM GMT, Charlie Jenkins wrote:
> This loop is supposed to check if ext->subset_ext_ids[j] is valid, rather
> than if ext->subset_ext_ids[i] is valid, before setting the extension
> id ext->subset_ext_ids[j] in isainfo->isa.
>
> Signed-off-by: Charlie Jenkins <[email protected]>
> Reviewed-by: Conor Dooley <[email protected]>
> Reviewed-by: Alexandre Ghiti <[email protected]>
> Fixes: 0d8295ed975b ("riscv: add ISA extension parsing for scalar crypto")
> ---
> arch/riscv/kernel/cpufeature.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index c6e27b45e192..6dff7bb1db3f 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -607,7 +607,7 @@ static int __init riscv_fill_hwcap_from_ext_list(unsigned long *isa2hwcap)
>
> if (ext->subset_ext_size) {
> for (int j = 0; j < ext->subset_ext_size; j++) {
> - if (riscv_isa_extension_check(ext->subset_ext_ids[i]))
> + if (riscv_isa_extension_check(ext->subset_ext_ids[j]))
> set_bit(ext->subset_ext_ids[j], isainfo->isa);
> }
> }
>
> --
> 2.44.0
>
Reviewed-by: Andrew Jones <[email protected]>
On Fri, Apr 26, 2024 at 11:02:30AM +0200, Andrew Jones wrote:
> On Wed, Apr 24, 2024 at 09:19:54AM GMT, Charlie Jenkins wrote:
> ...
> > + /*
> > + * Naively assume that all harts have the same mvendorid/marchid as the
> > + * boot hart.
>
> This comment isn't necessary, since we don't have to assume all harts have
> the same IDs. This information is currently being collected specifically
> for thead. So, we can state in a comment below, where the information is
> used, that we assume when the boot hart is thead, then we don't want to
> enable V on any hart (whatever their IDs are).
Yes you are correct, I will do as you suggest.
- Charlie
>
> Thanks,
> drew
>
> > + */
> > + boot_vendorid = riscv_get_mvendorid();
> > + boot_archid = riscv_get_marchid();
> > +
> > for_each_possible_cpu(cpu) {
> > struct riscv_isainfo *isainfo = &hart_isa[cpu];
> > unsigned long this_hwcap = 0;
> > @@ -544,8 +553,7 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap)
> > * CPU cores with the ratified spec will contain non-zero
> > * marchid.
> > */
> > - if (acpi_disabled && riscv_cached_mvendorid(cpu) == THEAD_VENDOR_ID &&
> > - riscv_cached_marchid(cpu) == 0x0) {
> > + if (acpi_disabled && boot_vendorid == THEAD_VENDOR_ID && boot_archid == 0x0) {
> > this_hwcap &= ~isa2hwcap[RISCV_ISA_EXT_v];
> > clear_bit(RISCV_ISA_EXT_v, isainfo->isa);
> > }
> >
> > --
> > 2.44.0
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > [email protected]
> > http://lists.infradead.org/mailman/listinfo/linux-riscv