[PATCH v1 4/7] RISC-V: validate riscv,isa at boot, not during ISA string parsing

[Conor Dooley]

On Fri, May 05, 2023 at 08:40:22PM +0800, Yangyu Chen wrote:
> On 5/5/23 2:14 AM, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley at microchip.com>
> >
> > Since riscv_fill_hwcap() now only iterates over possible cpus, the
> > basic validation of whether riscv,isa contains "rv<width>" can be moved
> > to riscv_early_of_processor_hartid().
> >
> > Further, "ima" support is required by the kernel, so reject any CPU not
> > fitting the bill.
> >
> > Signed-off-by: Conor Dooley <conor.dooley at microchip.com>
> > ---
> >  arch/riscv/kernel/cpu.c        |  8 +++++---
> >  arch/riscv/kernel/cpufeature.c | 12 ++++++------
> >  2 files changed, 11 insertions(+), 9 deletions(-)
> >
> > diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> > index 7030a5004f8e..b0c3ec0f2f5b 100644
> > --- a/arch/riscv/kernel/cpu.c
> > +++ b/arch/riscv/kernel/cpu.c
> > @@ -63,10 +63,12 @@ int riscv_early_of_processor_hartid(struct device_node *node, unsigned long *har
> >  		pr_warn("CPU with hartid=%lu has no \"riscv,isa\" property\n", *hart);
> >  		return -ENODEV;
> >  	}
> > -	if (tolower(isa[0]) != 'r' || tolower(isa[1]) != 'v') {
> > -		pr_warn("CPU with hartid=%lu has an invalid ISA of \"%s\"\n", *hart, isa);
> > +
> > +	if (IS_ENABLED(CONFIG_32BIT) && strncasecmp(isa, "rv32ima", 7))
> > +		return -ENODEV;
> > +
> > +	if (IS_ENABLED(CONFIG_64BIT) && strncasecmp(isa, "rv64ima", 7))
> >  		return -ENODEV;
> > -	}
> >  
> >  	return 0;
> >  }
> 
> What about reducing the compare string to "rv32" and "rv64" only?

I explicitly wanted the "ima" to rule out harts that might've been
left enabled but we cannot run on.

> Linux kernel is able to run on RV32IA or RV64IA CPUs theoretically as
> someone did before[1]. In this case, M extension is not required here.
> Although it's an invalid DT for now, reducing the compare string will
> make the porting easier. And M extension does not provide additional
> ISA-level registers that need the kernel to care about during context
> switch.
> 
> In my opinion, we should concern about if someday linux will support
> RV32E or RV64E. It's not necessary to validate if "ima" presents here
> for better forward compatibility.
> 
> [1] https://www.facebook.com/groups/riscv.tw/posts/1611033709104137/

I had a look, and as expected, this requires a makefile change to remove
the M extension from -march.
If that change ever lands in mainline the check here could be modified
too, but otherwise I don't think we need to be concerned about what some
out of tree code is doing.

Cheers,
Conor.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-riscv/attachments/20230505/62b76712/attachment.sig>