On ACPI based systems, the information about the hart
like ISA is provided by the RISC-V Hart Capabilities Table (RHCT).
Enable filling up hwcap structure based on the information in RHCT.
Signed-off-by: Sunil V L <[email protected]>
Acked-by: Rafael J. Wysocki <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
---
arch/riscv/kernel/cpufeature.c | 39 ++++++++++++++++++++++++++++++----
1 file changed, 35 insertions(+), 4 deletions(-)
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index 63e56ce04162..5d2065b937e5 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -6,6 +6,7 @@
* Copyright (C) 2017 SiFive
*/
+#include <linux/acpi.h>
#include <linux/bitmap.h>
#include <linux/ctype.h>
#include <linux/libfdt.h>
@@ -13,6 +14,8 @@
#include <linux/memory.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_device.h>
+#include <asm/acpi.h>
#include <asm/alternative.h>
#include <asm/cacheflush.h>
#include <asm/errata_list.h>
@@ -91,6 +94,9 @@ void __init riscv_fill_hwcap(void)
char print_str[NUM_ALPHA_EXTS + 1];
int i, j, rc;
unsigned long isa2hwcap[26] = {0};
+ struct acpi_table_header *rhct;
+ acpi_status status;
+ unsigned int cpu;
isa2hwcap['i' - 'a'] = COMPAT_HWCAP_ISA_I;
isa2hwcap['m' - 'a'] = COMPAT_HWCAP_ISA_M;
@@ -103,14 +109,36 @@ void __init riscv_fill_hwcap(void)
bitmap_zero(riscv_isa, RISCV_ISA_EXT_MAX);
- for_each_of_cpu_node(node) {
+ if (!acpi_disabled) {
+ status = acpi_get_table(ACPI_SIG_RHCT, 0, &rhct);
+ if (ACPI_FAILURE(status))
+ return;
+ }
+
+ for_each_possible_cpu(cpu) {
unsigned long this_hwcap = 0;
DECLARE_BITMAP(this_isa, RISCV_ISA_EXT_MAX);
const char *temp;
- if (of_property_read_string(node, "riscv,isa", &isa)) {
- pr_warn("Unable to find \"riscv,isa\" devicetree entry\n");
- continue;
+ if (acpi_disabled) {
+ node = of_cpu_device_node_get(cpu);
+ if (node) {
+ rc = of_property_read_string(node, "riscv,isa", &isa);
+ of_node_put(node);
+ if (rc) {
+ pr_warn("Unable to find \"riscv,isa\" devicetree entry\n");
+ continue;
+ }
+ } else {
+ pr_warn("Unable to find cpu node\n");
+ continue;
+ }
+ } else {
+ rc = acpi_get_riscv_isa(rhct, cpu, &isa);
+ if (rc < 0) {
+ pr_warn("Unable to get ISA for the hart - %d\n", cpu);
+ continue;
+ }
}
temp = isa;
@@ -243,6 +271,9 @@ void __init riscv_fill_hwcap(void)
bitmap_and(riscv_isa, riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
}
+ if (!acpi_disabled && rhct)
+ acpi_put_table((struct acpi_table_header *)rhct);
+
/* We don't support systems with F but without D, so mask those out
* here. */
if ((elf_hwcap & COMPAT_HWCAP_ISA_F) && !(elf_hwcap & COMPAT_HWCAP_ISA_D)) {
--
2.34.1
On Tue, Apr 04, 2023 at 11:50:27PM +0530, Sunil V L wrote:
> On ACPI based systems, the information about the hart
> like ISA is provided by the RISC-V Hart Capabilities Table (RHCT).
> Enable filling up hwcap structure based on the information in RHCT.
>
> Signed-off-by: Sunil V L <[email protected]>
> Acked-by: Rafael J. Wysocki <[email protected]>
> Reviewed-by: Andrew Jones <[email protected]>
> ---
> arch/riscv/kernel/cpufeature.c | 39 ++++++++++++++++++++++++++++++----
> 1 file changed, 35 insertions(+), 4 deletions(-)
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 63e56ce04162..5d2065b937e5 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -6,6 +6,7 @@
> * Copyright (C) 2017 SiFive
> */
>
> +#include <linux/acpi.h>
> #include <linux/bitmap.h>
> #include <linux/ctype.h>
> #include <linux/libfdt.h>
> @@ -13,6 +14,8 @@
> #include <linux/memory.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_device.h>
> +#include <asm/acpi.h>
> #include <asm/alternative.h>
> #include <asm/cacheflush.h>
> #include <asm/errata_list.h>
> @@ -91,6 +94,9 @@ void __init riscv_fill_hwcap(void)
> char print_str[NUM_ALPHA_EXTS + 1];
> int i, j, rc;
> unsigned long isa2hwcap[26] = {0};
> + struct acpi_table_header *rhct;
> + acpi_status status;
> + unsigned int cpu;
>
> isa2hwcap['i' - 'a'] = COMPAT_HWCAP_ISA_I;
> isa2hwcap['m' - 'a'] = COMPAT_HWCAP_ISA_M;
> @@ -103,14 +109,36 @@ void __init riscv_fill_hwcap(void)
>
> bitmap_zero(riscv_isa, RISCV_ISA_EXT_MAX);
>
> - for_each_of_cpu_node(node) {
> + if (!acpi_disabled) {
> + status = acpi_get_table(ACPI_SIG_RHCT, 0, &rhct);
> + if (ACPI_FAILURE(status))
> + return;
> + }
> +
> + for_each_possible_cpu(cpu) {
> unsigned long this_hwcap = 0;
> DECLARE_BITMAP(this_isa, RISCV_ISA_EXT_MAX);
> const char *temp;
>
> - if (of_property_read_string(node, "riscv,isa", &isa)) {
> - pr_warn("Unable to find \"riscv,isa\" devicetree entry\n");
> - continue;
> + if (acpi_disabled) {
> + node = of_cpu_device_node_get(cpu);
> + if (node) {
> + rc = of_property_read_string(node, "riscv,isa", &isa);
Hmm, after digging in the previous patch, I think this is actually not
possible to fail? We already validated it when setting up the mask of
possible cpus, but I think leaving the error handling here makes things
a lot more obvious.
I'd swear I gave you a (conditional) R-b on v3 though, no?
Reviewed-by: Conor Dooley <[email protected]>
Cheers,
Conor.
> + of_node_put(node);
> + if (rc) {
> + pr_warn("Unable to find \"riscv,isa\" devicetree entry\n");
> + continue;
> + }
> + } else {
> + pr_warn("Unable to find cpu node\n");
> + continue;
> + }
> + } else {
> + rc = acpi_get_riscv_isa(rhct, cpu, &isa);
> + if (rc < 0) {
> + pr_warn("Unable to get ISA for the hart - %d\n", cpu);
> + continue;
> + }
> }
>
> temp = isa;
> @@ -243,6 +271,9 @@ void __init riscv_fill_hwcap(void)
> bitmap_and(riscv_isa, riscv_isa, this_isa, RISCV_ISA_EXT_MAX);
> }
>
> + if (!acpi_disabled && rhct)
> + acpi_put_table((struct acpi_table_header *)rhct);
> +
> /* We don't support systems with F but without D, so mask those out
> * here. */
> if ((elf_hwcap & COMPAT_HWCAP_ISA_F) && !(elf_hwcap & COMPAT_HWCAP_ISA_D)) {
> --
> 2.34.1
>
>
> _______________________________________________
> linux-riscv mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-riscv
On Tue, Apr 04, 2023 at 09:57:19PM +0100, Conor Dooley wrote:
> On Tue, Apr 04, 2023 at 11:50:27PM +0530, Sunil V L wrote:
> > On ACPI based systems, the information about the hart
> > like ISA is provided by the RISC-V Hart Capabilities Table (RHCT).
> > Enable filling up hwcap structure based on the information in RHCT.
> >
> > Signed-off-by: Sunil V L <[email protected]>
> > Acked-by: Rafael J. Wysocki <[email protected]>
> > Reviewed-by: Andrew Jones <[email protected]>
> > ---
> > arch/riscv/kernel/cpufeature.c | 39 ++++++++++++++++++++++++++++++----
> > 1 file changed, 35 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > index 63e56ce04162..5d2065b937e5 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -6,6 +6,7 @@
> > * Copyright (C) 2017 SiFive
> > */
> >
> > +#include <linux/acpi.h>
> > #include <linux/bitmap.h>
> > #include <linux/ctype.h>
> > #include <linux/libfdt.h>
> > @@ -13,6 +14,8 @@
> > #include <linux/memory.h>
> > #include <linux/module.h>
> > #include <linux/of.h>
> > +#include <linux/of_device.h>
> > +#include <asm/acpi.h>
> > #include <asm/alternative.h>
> > #include <asm/cacheflush.h>
> > #include <asm/errata_list.h>
> > @@ -91,6 +94,9 @@ void __init riscv_fill_hwcap(void)
> > char print_str[NUM_ALPHA_EXTS + 1];
> > int i, j, rc;
> > unsigned long isa2hwcap[26] = {0};
> > + struct acpi_table_header *rhct;
> > + acpi_status status;
> > + unsigned int cpu;
> >
> > isa2hwcap['i' - 'a'] = COMPAT_HWCAP_ISA_I;
> > isa2hwcap['m' - 'a'] = COMPAT_HWCAP_ISA_M;
> > @@ -103,14 +109,36 @@ void __init riscv_fill_hwcap(void)
> >
> > bitmap_zero(riscv_isa, RISCV_ISA_EXT_MAX);
> >
> > - for_each_of_cpu_node(node) {
> > + if (!acpi_disabled) {
> > + status = acpi_get_table(ACPI_SIG_RHCT, 0, &rhct);
> > + if (ACPI_FAILURE(status))
> > + return;
> > + }
> > +
> > + for_each_possible_cpu(cpu) {
> > unsigned long this_hwcap = 0;
> > DECLARE_BITMAP(this_isa, RISCV_ISA_EXT_MAX);
> > const char *temp;
> >
> > - if (of_property_read_string(node, "riscv,isa", &isa)) {
> > - pr_warn("Unable to find \"riscv,isa\" devicetree entry\n");
> > - continue;
> > + if (acpi_disabled) {
> > + node = of_cpu_device_node_get(cpu);
> > + if (node) {
> > + rc = of_property_read_string(node, "riscv,isa", &isa);
>
> Hmm, after digging in the previous patch, I think this is actually not
> possible to fail? We already validated it when setting up the mask of
> possible cpus, but I think leaving the error handling here makes things
> a lot more obvious.
>
Yeah, do you prefer to merge these patches again since only in this
patch, we change the loop to for_each_possible_cpu() from
for_each_of_cpu_node() which actually makes riscv_of_processor_hartid()
not useful?
> I'd swear I gave you a (conditional) R-b on v3 though, no?
> Reviewed-by: Conor Dooley <[email protected]>
>
Thanks,
Sunil
On Wed, Apr 05, 2023 at 07:05:42PM +0530, Sunil V L wrote:
> On Tue, Apr 04, 2023 at 09:57:19PM +0100, Conor Dooley wrote:
> > On Tue, Apr 04, 2023 at 11:50:27PM +0530, Sunil V L wrote:
> > > On ACPI based systems, the information about the hart
> > > like ISA is provided by the RISC-V Hart Capabilities Table (RHCT).
> > > Enable filling up hwcap structure based on the information in RHCT.
> > >
> > > Signed-off-by: Sunil V L <[email protected]>
> > > Acked-by: Rafael J. Wysocki <[email protected]>
> > > Reviewed-by: Andrew Jones <[email protected]>
> > > ---
> > > arch/riscv/kernel/cpufeature.c | 39 ++++++++++++++++++++++++++++++----
> > > 1 file changed, 35 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> > > index 63e56ce04162..5d2065b937e5 100644
> > > --- a/arch/riscv/kernel/cpufeature.c
> > > +++ b/arch/riscv/kernel/cpufeature.c
> > > @@ -6,6 +6,7 @@
> > > * Copyright (C) 2017 SiFive
> > > */
> > >
> > > +#include <linux/acpi.h>
> > > #include <linux/bitmap.h>
> > > #include <linux/ctype.h>
> > > #include <linux/libfdt.h>
> > > @@ -13,6 +14,8 @@
> > > #include <linux/memory.h>
> > > #include <linux/module.h>
> > > #include <linux/of.h>
> > > +#include <linux/of_device.h>
> > > +#include <asm/acpi.h>
> > > #include <asm/alternative.h>
> > > #include <asm/cacheflush.h>
> > > #include <asm/errata_list.h>
> > > @@ -91,6 +94,9 @@ void __init riscv_fill_hwcap(void)
> > > char print_str[NUM_ALPHA_EXTS + 1];
> > > int i, j, rc;
> > > unsigned long isa2hwcap[26] = {0};
> > > + struct acpi_table_header *rhct;
> > > + acpi_status status;
> > > + unsigned int cpu;
> > >
> > > isa2hwcap['i' - 'a'] = COMPAT_HWCAP_ISA_I;
> > > isa2hwcap['m' - 'a'] = COMPAT_HWCAP_ISA_M;
> > > @@ -103,14 +109,36 @@ void __init riscv_fill_hwcap(void)
> > >
> > > bitmap_zero(riscv_isa, RISCV_ISA_EXT_MAX);
> > >
> > > - for_each_of_cpu_node(node) {
> > > + if (!acpi_disabled) {
> > > + status = acpi_get_table(ACPI_SIG_RHCT, 0, &rhct);
> > > + if (ACPI_FAILURE(status))
> > > + return;
> > > + }
> > > +
> > > + for_each_possible_cpu(cpu) {
> > > unsigned long this_hwcap = 0;
> > > DECLARE_BITMAP(this_isa, RISCV_ISA_EXT_MAX);
> > > const char *temp;
> > >
> > > - if (of_property_read_string(node, "riscv,isa", &isa)) {
> > > - pr_warn("Unable to find \"riscv,isa\" devicetree entry\n");
> > > - continue;
> > > + if (acpi_disabled) {
> > > + node = of_cpu_device_node_get(cpu);
> > > + if (node) {
> > > + rc = of_property_read_string(node, "riscv,isa", &isa);
> >
> > Hmm, after digging in the previous patch, I think this is actually not
> > possible to fail? We already validated it when setting up the mask of
> > possible cpus, but I think leaving the error handling here makes things
> > a lot more obvious.
> >
> Yeah, do you prefer to merge these patches again since only in this
> patch, we change the loop to for_each_possible_cpu() from
> for_each_of_cpu_node() which actually makes riscv_of_processor_hartid()
> not useful?
Yah, all 3 of us mistakenly thought that that was an unrelated cleanup
on the last revision, but clearly it is not.
Squash it back IMO, sorry for my part in the extra work generated.
Cheers,
Conor.
>
> > I'd swear I gave you a (conditional) R-b on v3 though, no?
> > Reviewed-by: Conor Dooley <[email protected]>
On Wed, Apr 05, 2023 at 03:31:24PM +0100, Conor Dooley wrote:
> On Wed, Apr 05, 2023 at 07:05:42PM +0530, Sunil V L wrote:
> > On Tue, Apr 04, 2023 at 09:57:19PM +0100, Conor Dooley wrote:
> > > On Tue, Apr 04, 2023 at 11:50:27PM +0530, Sunil V L wrote:
...
> > > > - if (of_property_read_string(node, "riscv,isa", &isa)) {
> > > > - pr_warn("Unable to find \"riscv,isa\" devicetree entry\n");
> > > > - continue;
> > > > + if (acpi_disabled) {
> > > > + node = of_cpu_device_node_get(cpu);
> > > > + if (node) {
> > > > + rc = of_property_read_string(node, "riscv,isa", &isa);
> > >
> > > Hmm, after digging in the previous patch, I think this is actually not
> > > possible to fail? We already validated it when setting up the mask of
> > > possible cpus, but I think leaving the error handling here makes things
> > > a lot more obvious.
> > >
> > Yeah, do you prefer to merge these patches again since only in this
> > patch, we change the loop to for_each_possible_cpu() from
> > for_each_of_cpu_node() which actually makes riscv_of_processor_hartid()
> > not useful?
>
> Yah, all 3 of us mistakenly thought that that was an unrelated cleanup
> on the last revision, but clearly it is not.
> Squash it back IMO, sorry for my part in the extra work generated.
Yup, please squash back in. Sorry about that, Sunil!
drew
On Sat, Apr 29, 2023 at 11:31:20AM +0100, Conor Dooley wrote:
> Hey Sunil,
>
> On Tue, Apr 04, 2023 at 11:50:27PM +0530, Sunil V L wrote:
>
> > @@ -103,14 +109,36 @@ void __init riscv_fill_hwcap(void)
> >
> > bitmap_zero(riscv_isa, RISCV_ISA_EXT_MAX);
> >
> > - for_each_of_cpu_node(node) {
> > + if (!acpi_disabled) {
> > + status = acpi_get_table(ACPI_SIG_RHCT, 0, &rhct);
> > + if (ACPI_FAILURE(status))
> > + return;
> > + }
> > +
> > + for_each_possible_cpu(cpu) {
> > unsigned long this_hwcap = 0;
> > DECLARE_BITMAP(this_isa, RISCV_ISA_EXT_MAX);
> > const char *temp;
> >
> > - if (of_property_read_string(node, "riscv,isa", &isa)) {
> > - pr_warn("Unable to find \"riscv,isa\" devicetree entry\n");
> > - continue;
> > + if (acpi_disabled) {
> > + node = of_cpu_device_node_get(cpu);
> > + if (node) {
> > + rc = of_property_read_string(node, "riscv,isa", &isa);
> > + of_node_put(node);
> > + if (rc) {
> > + pr_warn("Unable to find \"riscv,isa\" devicetree entry\n");
> > + continue;
> > + }
> > + } else {
> > + pr_warn("Unable to find cpu node\n");
> > + continue;
>
> I was poking at this the last few days and went back to look at the ACPI
> code again. Is there a reason we don't do early-return here? IOW:
>
> node = of_cpu_device_node_get(cpu);
> if (!node) {
> pr_warn()
> continue;
> }
>
> rc = of_property_read_string(node, "riscv,isa", &isa);
> of_node_put(node);
> if (rc) {
> pr_warn();
> continue;
> }
>
This looks better. Will update when I send the next revision of the
series. Thank you!, Conor.