| Message ID | 20240420-dev-charlie-support_thead_vector_6_9-v3-4-67cff4271d1d@rivosinc.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | riscv: Support vendor extensions and xtheadvector | expand |
On Sat, Apr 20, 2024 at 06:04:36PM -0700, Charlie Jenkins wrote: > If vlenb is provided in the device tree, prefer that over reading the > vlenb csr. > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > --- > arch/riscv/include/asm/cpufeature.h | 2 ++ > arch/riscv/kernel/cpufeature.c | 26 ++++++++++++++++++++++++++ > arch/riscv/kernel/vector.c | 13 +++++++++---- > 3 files changed, 37 insertions(+), 4 deletions(-) > > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h > index 347805446151..809f61ffb667 100644 > --- a/arch/riscv/include/asm/cpufeature.h > +++ b/arch/riscv/include/asm/cpufeature.h > @@ -31,6 +31,8 @@ DECLARE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo); > /* Per-cpu ISA extensions. */ > extern struct riscv_isainfo hart_isa[NR_CPUS]; > > +extern u32 riscv_vlenb_dt[NR_CPUS]; > + > void riscv_user_isa_enable(void); > > #if defined(CONFIG_RISCV_MISALIGNED) > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c > index c6e27b45e192..48874aac4871 100644 > --- a/arch/riscv/kernel/cpufeature.c > +++ b/arch/riscv/kernel/cpufeature.c > @@ -35,6 +35,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly; > /* Per-cpu ISA extensions. */ > struct riscv_isainfo hart_isa[NR_CPUS]; > > +u32 riscv_vlenb_dt[NR_CPUS]; > + > /** > * riscv_isa_extension_base() - Get base extension word > * > @@ -656,6 +658,28 @@ static int __init riscv_isa_fallback_setup(char *__unused) > early_param("riscv_isa_fallback", riscv_isa_fallback_setup); > #endif > > +static void riscv_set_vlenb_from_dt(void) I'd expect to see a name here that had "of" in it, not "dt". > +{ > + int cpu; > + > + for_each_possible_cpu(cpu) { > + struct device_node *cpu_node; > + > + cpu_node = of_cpu_device_node_get(cpu); > + if (!cpu_node) { > + pr_warn("Unable to find cpu node\n"); > + continue; > + } > + > + if (!of_property_read_u32(cpu_node, "riscv,vlenb", &riscv_vlenb_dt[cpu])) { > + of_node_put(cpu_node); > + continue; > + } > + > + of_node_put(cpu_node); > + } > +} > + > void __init riscv_fill_hwcap(void) > { > char print_str[NUM_ALPHA_EXTS + 1]; > @@ -675,6 +699,8 @@ void __init riscv_fill_hwcap(void) > } else { > int ret = riscv_fill_hwcap_from_ext_list(isa2hwcap); > > + riscv_set_vlenb_from_dt(); Hmm, I think we can go a step further here. We know all of the CPUs widths by the time we get to the first call to riscv_v_setup_vsize(), can we examine the whole list and decide not to enable vector if they do not match, rather than continuing and failing to online CPUs that having the mismatched size? I guess that can go into the `if (elf_hwcap & COMPAT_HWCAP_ISA_V)` condition we already have, and would require clearing the bit from the mask we have at the moment. Cheers, Conor. > + > if (ret && riscv_isa_fallback) { > pr_info("Falling back to deprecated \"riscv,isa\"\n"); > riscv_fill_hwcap_from_isa_string(isa2hwcap); > diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c > index 6727d1d3b8f2..fb7f3ca80d9e 100644 > --- a/arch/riscv/kernel/vector.c > +++ b/arch/riscv/kernel/vector.c > @@ -32,11 +32,16 @@ EXPORT_SYMBOL_GPL(riscv_v_vsize); > int riscv_v_setup_vsize(void) > { > unsigned long this_vsize; > + int cpu = smp_processor_id(); > > - /* There are 32 vector registers with vlenb length. */ > - riscv_v_enable(); > - this_vsize = csr_read(CSR_VLENB) * 32; > - riscv_v_disable(); > + if (riscv_vlenb_dt[cpu]) { > + this_vsize = riscv_vlenb_dt[cpu]; > + } else { > + /* There are 32 vector registers with vlenb length. */ > + riscv_v_enable(); > + this_vsize = csr_read(CSR_VLENB) * 32; > + riscv_v_disable(); > + } > > if (!riscv_v_vsize) { > riscv_v_vsize = this_vsize; > > -- > 2.44.0 >
On Fri, Apr 26, 2024 at 04:17:52PM +0100, Conor Dooley wrote: > On Sat, Apr 20, 2024 at 06:04:36PM -0700, Charlie Jenkins wrote: > > If vlenb is provided in the device tree, prefer that over reading the > > vlenb csr. > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > --- > > arch/riscv/include/asm/cpufeature.h | 2 ++ > > arch/riscv/kernel/cpufeature.c | 26 ++++++++++++++++++++++++++ > > arch/riscv/kernel/vector.c | 13 +++++++++---- > > 3 files changed, 37 insertions(+), 4 deletions(-) > > > > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h > > index 347805446151..809f61ffb667 100644 > > --- a/arch/riscv/include/asm/cpufeature.h > > +++ b/arch/riscv/include/asm/cpufeature.h > > @@ -31,6 +31,8 @@ DECLARE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo); > > /* Per-cpu ISA extensions. */ > > extern struct riscv_isainfo hart_isa[NR_CPUS]; > > > > +extern u32 riscv_vlenb_dt[NR_CPUS]; > > + > > void riscv_user_isa_enable(void); > > > > #if defined(CONFIG_RISCV_MISALIGNED) > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c > > index c6e27b45e192..48874aac4871 100644 > > --- a/arch/riscv/kernel/cpufeature.c > > +++ b/arch/riscv/kernel/cpufeature.c > > @@ -35,6 +35,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly; > > /* Per-cpu ISA extensions. */ > > struct riscv_isainfo hart_isa[NR_CPUS]; > > > > +u32 riscv_vlenb_dt[NR_CPUS]; > > + > > /** > > * riscv_isa_extension_base() - Get base extension word > > * > > @@ -656,6 +658,28 @@ static int __init riscv_isa_fallback_setup(char *__unused) > > early_param("riscv_isa_fallback", riscv_isa_fallback_setup); > > #endif > > > > +static void riscv_set_vlenb_from_dt(void) > > I'd expect to see a name here that had "of" in it, not "dt". Also, "set" - I think "get" is more suitable here given that this doesn't actually set the vlen, we only do any setting later on in riscv_v_set_vsize(). > > > +{ > > + int cpu; > > + > > + for_each_possible_cpu(cpu) { > > + struct device_node *cpu_node; > > + > > + cpu_node = of_cpu_device_node_get(cpu); > > + if (!cpu_node) { > > + pr_warn("Unable to find cpu node\n"); > > + continue; > > + } > > + > > + if (!of_property_read_u32(cpu_node, "riscv,vlenb", &riscv_vlenb_dt[cpu])) { > > + of_node_put(cpu_node); > > + continue; > > + } > > + > > + of_node_put(cpu_node); > > + } > > +} > > + > > void __init riscv_fill_hwcap(void) > > { > > char print_str[NUM_ALPHA_EXTS + 1]; > > @@ -675,6 +699,8 @@ void __init riscv_fill_hwcap(void) > > } else { > > int ret = riscv_fill_hwcap_from_ext_list(isa2hwcap); > > > > + riscv_set_vlenb_from_dt(); > > Hmm, I think we can go a step further here. We know all of the CPUs > widths by the time we get to the first call to riscv_v_setup_vsize(), can > we examine the whole list and decide not to enable vector if they do > not match, rather than continuing and failing to online CPUs that having > the mismatched size? > > I guess that can go into the `if (elf_hwcap & COMPAT_HWCAP_ISA_V)` > condition we already have, and would require clearing the bit from the > mask we have at the moment. > > Cheers, > Conor. > > > + > > if (ret && riscv_isa_fallback) { > > pr_info("Falling back to deprecated \"riscv,isa\"\n"); > > riscv_fill_hwcap_from_isa_string(isa2hwcap); > > diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c > > index 6727d1d3b8f2..fb7f3ca80d9e 100644 > > --- a/arch/riscv/kernel/vector.c > > +++ b/arch/riscv/kernel/vector.c > > @@ -32,11 +32,16 @@ EXPORT_SYMBOL_GPL(riscv_v_vsize); > > int riscv_v_setup_vsize(void) > > { > > unsigned long this_vsize; > > + int cpu = smp_processor_id(); > > > > - /* There are 32 vector registers with vlenb length. */ > > - riscv_v_enable(); > > - this_vsize = csr_read(CSR_VLENB) * 32; > > - riscv_v_disable(); > > + if (riscv_vlenb_dt[cpu]) { > > + this_vsize = riscv_vlenb_dt[cpu]; > > > + } else { > > + /* There are 32 vector registers with vlenb length. */ > > + riscv_v_enable(); > > + this_vsize = csr_read(CSR_VLENB) * 32; > > + riscv_v_disable(); > > + } > > > > if (!riscv_v_vsize) { > > riscv_v_vsize = this_vsize; > > > > -- > > 2.44.0 > >
On Fri, Apr 26, 2024 at 05:21:16PM +0100, Conor Dooley wrote: > On Fri, Apr 26, 2024 at 04:17:52PM +0100, Conor Dooley wrote: > > On Sat, Apr 20, 2024 at 06:04:36PM -0700, Charlie Jenkins wrote: > > > If vlenb is provided in the device tree, prefer that over reading the > > > vlenb csr. > > > > > > Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> > > > --- > > > arch/riscv/include/asm/cpufeature.h | 2 ++ > > > arch/riscv/kernel/cpufeature.c | 26 ++++++++++++++++++++++++++ > > > arch/riscv/kernel/vector.c | 13 +++++++++---- > > > 3 files changed, 37 insertions(+), 4 deletions(-) > > > > > > diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h > > > index 347805446151..809f61ffb667 100644 > > > --- a/arch/riscv/include/asm/cpufeature.h > > > +++ b/arch/riscv/include/asm/cpufeature.h > > > @@ -31,6 +31,8 @@ DECLARE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo); > > > /* Per-cpu ISA extensions. */ > > > extern struct riscv_isainfo hart_isa[NR_CPUS]; > > > > > > +extern u32 riscv_vlenb_dt[NR_CPUS]; > > > + > > > void riscv_user_isa_enable(void); > > > > > > #if defined(CONFIG_RISCV_MISALIGNED) > > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c > > > index c6e27b45e192..48874aac4871 100644 > > > --- a/arch/riscv/kernel/cpufeature.c > > > +++ b/arch/riscv/kernel/cpufeature.c > > > @@ -35,6 +35,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly; > > > /* Per-cpu ISA extensions. */ > > > struct riscv_isainfo hart_isa[NR_CPUS]; > > > > > > +u32 riscv_vlenb_dt[NR_CPUS]; > > > + > > > /** > > > * riscv_isa_extension_base() - Get base extension word > > > * > > > @@ -656,6 +658,28 @@ static int __init riscv_isa_fallback_setup(char *__unused) > > > early_param("riscv_isa_fallback", riscv_isa_fallback_setup); > > > #endif > > > > > > +static void riscv_set_vlenb_from_dt(void) > > > > I'd expect to see a name here that had "of" in it, not "dt". > > Also, "set" - I think "get" is more suitable here given that this > doesn't actually set the vlen, we only do any setting later on in > riscv_v_set_vsize(). > > > > > > +{ > > > + int cpu; > > > + > > > + for_each_possible_cpu(cpu) { > > > + struct device_node *cpu_node; > > > + > > > + cpu_node = of_cpu_device_node_get(cpu); > > > + if (!cpu_node) { > > > + pr_warn("Unable to find cpu node\n"); > > > + continue; > > > + } > > > + > > > + if (!of_property_read_u32(cpu_node, "riscv,vlenb", &riscv_vlenb_dt[cpu])) { > > > + of_node_put(cpu_node); > > > + continue; > > > + } > > > + > > > + of_node_put(cpu_node); > > > + } > > > +} > > > + > > > void __init riscv_fill_hwcap(void) > > > { > > > char print_str[NUM_ALPHA_EXTS + 1]; > > > @@ -675,6 +699,8 @@ void __init riscv_fill_hwcap(void) > > > } else { > > > int ret = riscv_fill_hwcap_from_ext_list(isa2hwcap); > > > > > > + riscv_set_vlenb_from_dt(); > > > > Hmm, I think we can go a step further here. We know all of the CPUs > > widths by the time we get to the first call to riscv_v_setup_vsize(), can > > we examine the whole list and decide not to enable vector if they do > > not match, rather than continuing and failing to online CPUs that having > > the mismatched size? > > > > I guess that can go into the `if (elf_hwcap & COMPAT_HWCAP_ISA_V)` > > condition we already have, and would require clearing the bit from the > > mask we have at the moment. Good point, thank you. Since this is not supported with ACPI, I will clear the COMPAT_HWCAP_ISA_V bit from elf_hwcap if riscv_set_vlenb_from_dt() (which will be renamed) determines that the the riscv,vlenb field changes between CPU entries. - Charlie > > > > Cheers, > > Conor. > > > > > + > > > if (ret && riscv_isa_fallback) { > > > pr_info("Falling back to deprecated \"riscv,isa\"\n"); > > > riscv_fill_hwcap_from_isa_string(isa2hwcap); > > > diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c > > > index 6727d1d3b8f2..fb7f3ca80d9e 100644 > > > --- a/arch/riscv/kernel/vector.c > > > +++ b/arch/riscv/kernel/vector.c > > > @@ -32,11 +32,16 @@ EXPORT_SYMBOL_GPL(riscv_v_vsize); > > > int riscv_v_setup_vsize(void) > > > { > > > unsigned long this_vsize; > > > + int cpu = smp_processor_id(); > > > > > > - /* There are 32 vector registers with vlenb length. */ > > > - riscv_v_enable(); > > > - this_vsize = csr_read(CSR_VLENB) * 32; > > > - riscv_v_disable(); > > > + if (riscv_vlenb_dt[cpu]) { > > > + this_vsize = riscv_vlenb_dt[cpu]; > > > > > + } else { > > > + /* There are 32 vector registers with vlenb length. */ > > > + riscv_v_enable(); > > > + this_vsize = csr_read(CSR_VLENB) * 32; > > > + riscv_v_disable(); > > > + } > > > > > > if (!riscv_v_vsize) { > > > riscv_v_vsize = this_vsize; > > > > > > -- > > > 2.44.0 > > > > >
diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h index 347805446151..809f61ffb667 100644 --- a/arch/riscv/include/asm/cpufeature.h +++ b/arch/riscv/include/asm/cpufeature.h @@ -31,6 +31,8 @@ DECLARE_PER_CPU(struct riscv_cpuinfo, riscv_cpuinfo); /* Per-cpu ISA extensions. */ extern struct riscv_isainfo hart_isa[NR_CPUS]; +extern u32 riscv_vlenb_dt[NR_CPUS]; + void riscv_user_isa_enable(void); #if defined(CONFIG_RISCV_MISALIGNED) diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c index c6e27b45e192..48874aac4871 100644 --- a/arch/riscv/kernel/cpufeature.c +++ b/arch/riscv/kernel/cpufeature.c @@ -35,6 +35,8 @@ static DECLARE_BITMAP(riscv_isa, RISCV_ISA_EXT_MAX) __read_mostly; /* Per-cpu ISA extensions. */ struct riscv_isainfo hart_isa[NR_CPUS]; +u32 riscv_vlenb_dt[NR_CPUS]; + /** * riscv_isa_extension_base() - Get base extension word * @@ -656,6 +658,28 @@ static int __init riscv_isa_fallback_setup(char *__unused) early_param("riscv_isa_fallback", riscv_isa_fallback_setup); #endif +static void riscv_set_vlenb_from_dt(void) +{ + int cpu; + + for_each_possible_cpu(cpu) { + struct device_node *cpu_node; + + cpu_node = of_cpu_device_node_get(cpu); + if (!cpu_node) { + pr_warn("Unable to find cpu node\n"); + continue; + } + + if (!of_property_read_u32(cpu_node, "riscv,vlenb", &riscv_vlenb_dt[cpu])) { + of_node_put(cpu_node); + continue; + } + + of_node_put(cpu_node); + } +} + void __init riscv_fill_hwcap(void) { char print_str[NUM_ALPHA_EXTS + 1]; @@ -675,6 +699,8 @@ void __init riscv_fill_hwcap(void) } else { int ret = riscv_fill_hwcap_from_ext_list(isa2hwcap); + riscv_set_vlenb_from_dt(); + if (ret && riscv_isa_fallback) { pr_info("Falling back to deprecated \"riscv,isa\"\n"); riscv_fill_hwcap_from_isa_string(isa2hwcap); diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c index 6727d1d3b8f2..fb7f3ca80d9e 100644 --- a/arch/riscv/kernel/vector.c +++ b/arch/riscv/kernel/vector.c @@ -32,11 +32,16 @@ EXPORT_SYMBOL_GPL(riscv_v_vsize); int riscv_v_setup_vsize(void) { unsigned long this_vsize; + int cpu = smp_processor_id(); - /* There are 32 vector registers with vlenb length. */ - riscv_v_enable(); - this_vsize = csr_read(CSR_VLENB) * 32; - riscv_v_disable(); + if (riscv_vlenb_dt[cpu]) { + this_vsize = riscv_vlenb_dt[cpu]; + } else { + /* There are 32 vector registers with vlenb length. */ + riscv_v_enable(); + this_vsize = csr_read(CSR_VLENB) * 32; + riscv_v_disable(); + } if (!riscv_v_vsize) { riscv_v_vsize = this_vsize;
If vlenb is provided in the device tree, prefer that over reading the vlenb csr. Signed-off-by: Charlie Jenkins <charlie@rivosinc.com> --- arch/riscv/include/asm/cpufeature.h | 2 ++ arch/riscv/kernel/cpufeature.c | 26 ++++++++++++++++++++++++++ arch/riscv/kernel/vector.c | 13 +++++++++---- 3 files changed, 37 insertions(+), 4 deletions(-)