On Wed, 9 Oct 2019, Atish Patra wrote:
> 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]>
Looks like riscv_read_check_isa() is called twice for each hart. Is there
any way to call it only once per hart?
- Paul
> ---
> arch/riscv/include/asm/processor.h | 1 +
> arch/riscv/kernel/cpu.c | 41 ++++++++++++++++++++++--------
> arch/riscv/kernel/cpufeature.c | 4 +--
> arch/riscv/kernel/smpboot.c | 4 +++
> 4 files changed, 37 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..6bd4c7176bf6 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -8,13 +8,43 @@
> #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 (IS_ENABLED(CONFIG_32BIT) && (strncmp(*isa, "rv32i", 5) != 0)) {
> + pr_warn("hartid=%d has an invalid ISA \"%s\" for 32bit
> config\n",
> + hart, *isa);
> + return -ENODEV;
> + } else if (IS_ENABLED(CONFIG_64BIT) &&
> + (strncmp(*isa, "rv64i", 5) != 0)) {
> + pr_warn("hartid=%d has an invalid ISA \"%s\" for 64bit
> config\n",
> + hart, *isa);
> + return -ENODEV;
> + }
> + 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 +62,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
>
>
- Paul
