Re: [PATCH v2] RISC-V: Don't trust V from the riscv,isa DT property on T-Head CPUs

[Jisheng Zhang <jszhang@kernel.org>]

On Wed, Jul 12, 2023 at 06:48:02PM +0100, Conor Dooley wrote:
> From: Palmer Dabbelt <palmer@rivosinc.com>
> 
> The last merge window contained both V support and the deprecation of
> the riscv,isa DT property, with the V implementation reading riscv,isa
> to determine the presence of the V extension.  At the time that was the
> only way to do it, but there's a lot of ambiguity around V in ISA
> strings.  In particular, there is a lot of firmware in the wild that
> uses "v" in the riscv,isa DT property to communicate support for the
> 0.7.1 version of the Vector specification implemented by T-Head CPU
> cores.

Add Guo

Hi Conor, Palmer,

FWICT, new T-HEAD's riscv cores such as C908 support standard RVV-1.0,
this patch looks like a big hammer for T-HEAD. I do understand why
this patch is provided, but can we mitigate the situation by carefully
review the DTs? Per my understanding, dts is also part of linux kernel.

Thanks

> 
> Rather than forcing use of the newly added interface that has strict
> meanings for extensions to detect the presence of vector support, as
> that would penalise those who have behaved, only ignore v in riscv,isa
> on CPUs that report T-Head's vendor ID.
> 
> Fixes: dc6667a4e7e3 ("riscv: Extending cpufeature.c to detect V-extension")
> Signed-off-by: Palmer Dabbelt <palmer@rivosinc.com>
> Co-developed-by: Conor Dooley <conor.dooley@microchip.com>
> Signed-off-by: Conor Dooley <conor.dooley@microchip.com>
> ---
> Changes in v2:
> - Use my version of the patch that touches hwcap and isainfo uniformly
> - Don't penalise those who behaved
> ---
>  arch/riscv/kernel/cpufeature.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index bdcf460ea53d..05362715e1b7 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -21,6 +21,7 @@
>  #include <asm/hwcap.h>
>  #include <asm/patch.h>
>  #include <asm/processor.h>
> +#include <asm/sbi.h>
>  #include <asm/vector.h>
>  
>  #define NUM_ALPHA_EXTS ('z' - 'a' + 1)
> @@ -334,6 +335,27 @@ void __init riscv_fill_hwcap(void)
>  			set_bit(RISCV_ISA_EXT_ZIHPM, isainfo->isa);
>  		}
>  
> +		/*
> +		 * "V" in ISA strings is ambiguous in practice: it should mean
> +		 * just the standard V-1.0 but vendors aren't well behaved.
> +		 * Many vendors with T-Head CPU cores which implement the 0.7.1
> +		 * version of the vector specification put "v" into their DTs
> +		 * and no T-Head CPU cores with the standard version of vector
> +		 * are in circulation yet.
> +		 * Platforms with T-Head CPU cores that support the standard
> +		 * version of vector must provide the explicit V property,
> +		 * which is well defined.
> +		 */
> +		if (acpi_disabled && riscv_cached_mvendorid(cpu) == THEAD_VENDOR_ID) {
> +			if (of_property_match_string(node, "riscv,isa-extensions", "v") >= 0) {
> +				this_hwcap |= isa2hwcap[RISCV_ISA_EXT_v];
> +				set_bit(RISCV_ISA_EXT_v, isainfo->isa);
> +			} else {
> +				this_hwcap &= ~isa2hwcap[RISCV_ISA_EXT_v];
> +				clear_bit(RISCV_ISA_EXT_v, isainfo->isa);
> +			}
> +		}
> +
>  		/*
>  		 * All "okay" hart should have same isa. Set HWCAP based on
>  		 * common capabilities of every "okay" hart, in case they don't
> -- 
> 2.39.2
> 
> 
> _______________________________________________
> 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