Re: [PATCH v2 10/10] RISC-V: provide a Kconfig option to disable parsing "riscv,isa"

[Andrew Jones]

On Thu, Jun 29, 2023 at 09:28:56AM +0100, Conor Dooley wrote:
> As it says on the tin, provide a Kconfig option to disabling parsing the
> "riscv,isa" devicetree property. Hide the option behind NONPORTABLE so
> that only those willing to keep the pieces enable it, and make sure the
> default kernel contains the fallback code.
> 
> Suggested-by: Palmer Dabbelt <palmer@xxxxxxxxxxxx>
> Signed-off-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> ---
>  arch/riscv/Kconfig             | 16 ++++++++++++++++
>  arch/riscv/kernel/cpu.c        |  3 +++
>  arch/riscv/kernel/cpufeature.c |  2 +-
>  3 files changed, 20 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index 1d39efe2b940..0e1909ac5947 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -291,6 +291,22 @@ config NONPORTABLE
>  
>  	  If unsure, say N.
>  
> +config NO_RISCV_ISA_FALLBACK
> +	bool "Permit falling back to parsing riscv,isa for extension support"
> +	depends on NONPORTABLE
> +	help
> +	  Parsing the "riscv,isa" devicetree property has been deprecated and
> +	  replaced by a list of explicitly defined strings. For compatibility
> +	  with existing platforms, the kernel will fall back to parsing the
> +	  "riscv,isa" property if the replacements are not found.
> +
> +	  Selecting Y here will result in a kernel without this fallback, and
> +	  will not work on platforms where the devicetree does not contain the
> +	  replacement 	  properties of "riscv,isa-base" and
                     ^ spacing issue

> +	  "riscv,isa-extensions". Please see the dt-binding, located at
> +	  Documentation/devicetree/bindings/riscv/extensions.yaml for details
> +	  on the replacement properties.
> +
>  choice
>  	prompt "Base ISA"
>  	default ARCH_RV64I
> diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c
> index 9a4f4a23afcd..86a1d98b8b3b 100644
> --- a/arch/riscv/kernel/cpu.c
> +++ b/arch/riscv/kernel/cpu.c
> @@ -81,6 +81,9 @@ int riscv_early_of_processor_hartid(struct device_node *node, unsigned long *har
>  	return 0;
>  
>  old_interface:
> +	if (IS_ENABLED(CONFIG_NO_RISCV_ISA_FALLBACK))
> +		return -ENODEV;
> +
>  	if (of_property_read_string(node, "riscv,isa", &isa)) {
>  		pr_warn("CPU with hartid=%lu has no \"riscv,isa-base\" or \"riscv,isa\" property\n",
>  			*hart);
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 2c4503fa984f..f6fb18d2af84 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -490,7 +490,7 @@ void __init riscv_fill_hwcap(void)
>  	} else {
>  		int ret = riscv_fill_hwcap_from_ext_list(isa2hwcap);
>  
> -		if (ret) {
> +		if (ret && !IS_ENABLED(CONFIG_NO_RISCV_ISA_FALLBACK)) {
>  			pr_info("Falling back to deprecated \"riscv,isa\"\n");
>  			riscv_fill_hwcap_from_isa_string(isa2hwcap);
>  		}
> -- 
> 2.40.1
>

Should we also have a kernel command line option, 'isa_fallback', where
without this config the command line option is not necessary to fallback,
but, with this config, no fallback will be done unless 'isa_fallback' is
provided?

Thanks,
drew