Currently, isa string is read and checked for correctness at multiple
places.
Consolidate them into one function and use it only during early bootup.
In case of a incorrect isa string, the cpu shouldn't boot at all.
Signed-off-by: Atish Patra <[email protected]>
---
arch/riscv/include/asm/processor.h | 1 +
arch/riscv/kernel/cpu.c | 40 ++++++++++++++++++++++--------
arch/riscv/kernel/cpufeature.c | 4 +--
arch/riscv/kernel/smpboot.c | 4 +++
4 files changed, 36 insertions(+), 13 deletions(-)
diff --git a/arch/riscv/include/asm/processor.h b/arch/riscv/include/asm/processor.h
index f539149d04c2..189bf98f9a3f 100644
--- a/arch/riscv/include/asm/processor.h
+++ b/arch/riscv/include/asm/processor.h
@@ -74,6 +74,7 @@ static inline void wait_for_interrupt(void)
}
struct device_node;
+int riscv_read_check_isa(struct device_node *node, const char **isa);
int riscv_of_processor_hartid(struct device_node *node);
extern void riscv_fill_hwcap(void);
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
index 40a3c442ac5f..95ef5c91823d 100644
--- a/arch/riscv/kernel/cpu.c
+++ b/arch/riscv/kernel/cpu.c
@@ -8,13 +8,42 @@
#include <linux/of.h>
#include <asm/smp.h>
+int riscv_read_check_isa(struct device_node *node, const char **isa)
+{
+ u32 hart;
+
+ if (of_property_read_u32(node, "reg", &hart)) {
+ pr_warn("Found CPU without hart ID\n");
+ return -ENODEV;
+ }
+
+ if (of_property_read_string(node, "riscv,isa", isa)) {
+ pr_warn("CPU with hartid=%d has no \"riscv,isa\" property\n",
+ hart);
+ return -ENODEV;
+ }
+
+ /*
+ * Linux doesn't support rv32e or rv128i, and we only support booting
+ * kernels on harts with the same ISA that the kernel is compiled for.
+ */
+#if defined(CONFIG_32BIT)
+ if (strncmp(*isa, "rv32i", 5) != 0)
+ return -ENODEV;
+#elif defined(CONFIG_64BIT)
+ if (strncmp(*isa, "rv64i", 5) != 0)
+ return -ENODEV;
+#endif
+
+ return 0;
+}
+
/*
* Returns the hart ID of the given device tree node, or -ENODEV if the node
* isn't an enabled and valid RISC-V hart node.
*/
int riscv_of_processor_hartid(struct device_node *node)
{
- const char *isa;
u32 hart;
if (!of_device_is_compatible(node, "riscv")) {
@@ -32,15 +61,6 @@ int riscv_of_processor_hartid(struct device_node *node)
return -ENODEV;
}
- if (of_property_read_string(node, "riscv,isa", &isa)) {
- pr_warn("CPU with hartid=%d has no \"riscv,isa\" property\n", hart);
- return -ENODEV;
- }
- if (isa[0] != 'r' || isa[1] != 'v') {
- pr_warn("CPU with hartid=%d has an invalid ISA of \"%s\"\n", hart, isa);
- return -ENODEV;
- }
-
return hart;
}
diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
index b1ade9a49347..eaad5aa07403 100644
--- a/arch/riscv/kernel/cpufeature.c
+++ b/arch/riscv/kernel/cpufeature.c
@@ -38,10 +38,8 @@ void riscv_fill_hwcap(void)
if (riscv_of_processor_hartid(node) < 0)
continue;
- if (of_property_read_string(node, "riscv,isa", &isa)) {
- pr_warn("Unable to find \"riscv,isa\" devicetree entry\n");
+ if (riscv_read_check_isa(node, &isa) < 0)
continue;
- }
for (i = 0; i < strlen(isa); ++i)
this_hwcap |= isa2hwcap[(unsigned char)(isa[i])];
diff --git a/arch/riscv/kernel/smpboot.c b/arch/riscv/kernel/smpboot.c
index 18ae6da5115e..15ee71297abf 100644
--- a/arch/riscv/kernel/smpboot.c
+++ b/arch/riscv/kernel/smpboot.c
@@ -60,12 +60,16 @@ void __init setup_smp(void)
int hart;
bool found_boot_cpu = false;
int cpuid = 1;
+ const char *isa;
for_each_of_cpu_node(dn) {
hart = riscv_of_processor_hartid(dn);
if (hart < 0)
continue;
+ if (riscv_read_check_isa(dn, &isa) < 0)
+ continue;
+
if (hart == cpuid_to_hartid_map(0)) {
BUG_ON(found_boot_cpu);
found_boot_cpu = 1;
--
2.21.0
> +int riscv_read_check_isa(struct device_node *node, const char **isa)
> +{
> + u32 hart;
> +
> + if (of_property_read_u32(node, "reg", &hart)) {
> + pr_warn("Found CPU without hart ID\n");
> + return -ENODEV;
> + }
> +
> + if (of_property_read_string(node, "riscv,isa", isa)) {
> + pr_warn("CPU with hartid=%d has no \"riscv,isa\" property\n",
> + hart);
> + return -ENODEV;
> + }
> +
> + /*
> + * Linux doesn't support rv32e or rv128i, and we only support booting
> + * kernels on harts with the same ISA that the kernel is compiled for.
> + */
> +#if defined(CONFIG_32BIT)
> + if (strncmp(*isa, "rv32i", 5) != 0)
> + return -ENODEV;
> +#elif defined(CONFIG_64BIT)
> + if (strncmp(*isa, "rv64i", 5) != 0)
> + return -ENODEV;
> +#endif
Using IS_ENABLED here would clean the checks up a bit.
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index b1ade9a49347..eaad5aa07403 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -38,10 +38,8 @@ void riscv_fill_hwcap(void)
> if (riscv_of_processor_hartid(node) < 0)
> continue;
>
> - if (of_property_read_string(node, "riscv,isa", &isa)) {
> - pr_warn("Unable to find \"riscv,isa\" devicetree entry\n");
> + if (riscv_read_check_isa(node, &isa) < 0)
> continue;
Do we really get rid of warnings if we didn't find anything proper?
On Tue, 2019-10-08 at 08:44 -0700, Christoph Hellwig wrote:
> > +int riscv_read_check_isa(struct device_node *node, const char
> > **isa)
> > +{
> > + u32 hart;
> > +
> > + if (of_property_read_u32(node, "reg", &hart)) {
> > + pr_warn("Found CPU without hart ID\n");
> > + return -ENODEV;
> > + }
> > +
> > + if (of_property_read_string(node, "riscv,isa", isa)) {
> > + pr_warn("CPU with hartid=%d has no \"riscv,isa\"
> > property\n",
> > + hart);
> > + return -ENODEV;
> > + }
> > +
> > + /*
> > + * Linux doesn't support rv32e or rv128i, and we only support
> > booting
> > + * kernels on harts with the same ISA that the kernel is
> > compiled for.
> > + */
> > +#if defined(CONFIG_32BIT)
> > + if (strncmp(*isa, "rv32i", 5) != 0)
> > + return -ENODEV;
> > +#elif defined(CONFIG_64BIT)
> > + if (strncmp(*isa, "rv64i", 5) != 0)
> > + return -ENODEV;
> > +#endif
>
> Using IS_ENABLED here would clean the checks up a bit.
>
> > diff --git a/arch/riscv/kernel/cpufeature.c
> > b/arch/riscv/kernel/cpufeature.c
> > index b1ade9a49347..eaad5aa07403 100644
> > --- a/arch/riscv/kernel/cpufeature.c
> > +++ b/arch/riscv/kernel/cpufeature.c
> > @@ -38,10 +38,8 @@ void riscv_fill_hwcap(void)
> > if (riscv_of_processor_hartid(node) < 0)
> > continue;
> >
> > - if (of_property_read_string(node, "riscv,isa", &isa)) {
> > - pr_warn("Unable to find \"riscv,isa\"
> > devicetree entry\n");
> > + if (riscv_read_check_isa(node, &isa) < 0)
> > continue;
>
> Do we really get rid of warnings if we didn't find anything proper?
Ok. Added back the warnings and IS_ENABLED in v2.
--
Regards,
Atish