Re: [PATCH 2/3] RISC-V: Introduce riscv_isa_extension_check

From: Conor Dooley <conor@kernel.org>
To: Andrew Jones <ajones@ventanamicro.com>
Cc: linux-riscv@lists.infradead.org,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Albert Ou <aou@eecs.berkeley.edu>,
	Conor Dooley <conor.dooley@microchip.com>,
	Heiko Stuebner <heiko@sntech.de>,
	Anup Patel <apatel@ventanamicro.com>,
	Atish Patra <atishp@rivosinc.com>
Subject: Re: [PATCH 2/3] RISC-V: Introduce riscv_isa_extension_check
Date: Sun, 23 Oct 2022 20:32:38 +0100	[thread overview]
Message-ID: <Y1WW1v5T6IYSGN6G@spud> (raw)
In-Reply-To: <20221021105905.206385-3-ajones@ventanamicro.com>

On Fri, Oct 21, 2022 at 12:59:04PM +0200, Andrew Jones wrote:
> Currently any isa extension found in the isa string is set in the
> isa bitmap. An isa extension set in the bitmap indicates that the
> extension is present and may be used (a.k.a is enabled). However,
> when an extension cannot be used due to missing dependencies or
> errata it should not be added to the bitmap. Introduce a function
> where additional checks may be placed in order to determine if an
> extension should be enabled or not.
> 
> Note, the checks may simply indicate an issue with the DT, but,
> since extensions may be used in early boot, it's not always possible
> to simply produce an error at the point the issue is determined.
> It's best to keep the extension disabled and produce an error.

Aye, agreed. Could be pretty nasty chasing down something like this...
Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

> 
> No functional change intended, as the function is only introduced
> and always returns true. A later patch will provide checks for an
> isa extension.
> 
> Signed-off-by: Andrew Jones <ajones@ventanamicro.com>
> ---
>  arch/riscv/kernel/cpufeature.c | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 4677320d7e31..220be7222129 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -68,6 +68,11 @@ bool __riscv_isa_extension_available(const unsigned long *isa_bitmap, int bit)
>  }
>  EXPORT_SYMBOL_GPL(__riscv_isa_extension_available);
>  
> +static bool riscv_isa_extension_check(int id)
> +{
> +	return true;
> +}
> +
>  void __init riscv_fill_hwcap(void)
>  {
>  	struct device_node *node;
> @@ -189,7 +194,8 @@ void __init riscv_fill_hwcap(void)
>  #define SET_ISA_EXT_MAP(name, bit)						\
>  			do {							\
>  				if ((ext_end - ext == sizeof(name) - 1) &&	\
> -				     !memcmp(ext, name, sizeof(name) - 1))	\
> +				     !memcmp(ext, name, sizeof(name) - 1) &&	\
> +				     riscv_isa_extension_check(bit))		\
>  					set_bit(bit, this_isa);			\
>  			} while (false)						\
>  
> @@ -198,8 +204,10 @@ void __init riscv_fill_hwcap(void)
>  			if (!ext_long) {
>  				int nr = *ext - 'a';
>  
> -				this_hwcap |= isa2hwcap[nr];
> -				set_bit(nr, this_isa);
> +				if (riscv_isa_extension_check(nr)) {
> +					this_hwcap |= isa2hwcap[nr];
> +					set_bit(nr, this_isa);
> +				}
>  			} else {
>  				SET_ISA_EXT_MAP("sscofpmf", RISCV_ISA_EXT_SSCOFPMF);
>  				SET_ISA_EXT_MAP("svpbmt", RISCV_ISA_EXT_SVPBMT);
> -- 
> 2.37.3
> 
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv