Re: [PATCH v3] RISC-V: Ignore V from the riscv,isa DT property on older T-Head CPUs

From: Guo Ren <guoren@kernel.org>
To: Conor Dooley <conor@kernel.org>
Cc: linux-riscv@lists.infradead.org,
	Palmer Dabbelt <palmer@rivosinc.com>,
	 Conor Dooley <conor.dooley@microchip.com>,
	charlie@rivosinc.com, palmer@dabbelt.com
Subject: Re: [PATCH v3] RISC-V: Ignore V from the riscv,isa DT property on older T-Head CPUs
Date: Fri, 23 Feb 2024 19:38:17 +0800	[thread overview]
Message-ID: <CAJF2gTRs-vycC1yTWiKrpy6kUw5uyjFFngESWe9EFdX-UZA+8w@mail.gmail.com> (raw)
In-Reply-To: <20240223-tidings-shabby-607f086cb4d7@spud>

On Fri, Feb 23, 2024 at 7:31 PM Conor Dooley <conor@kernel.org> wrote:
>
> From: Palmer Dabbelt <palmer@rivosinc.com>
>
> Before attempting to support the pre-ratification version of vector
> found on older T-Head CPUs, disallow "v" in riscv,isa on these
> platforms. The deprecated property has no clear way to communicate
> the specific version of vector that is supported and much of the vendor
> provided software puts "v" in the isa string. riscv,isa-extensions
> should be used instead. This should not be too much of a burden for
> these systems, as the vendor shipped devicetrees and firmware do not
> work with a mainline kernel and will require updating.
>
> We can limit this restriction to only ignore v in riscv,isa on CPUs
> that report T-Head's vendor ID and a zero marchid. Newer T-Head CPUs
> that support the ratified version of vector should report non-zero
> marchid, according to Guo Ren [1].
Acked-by: Guo Ren <guoren@kernel.org>

Thx, this helps the newest application disable vector_10 when on the
old XuanTie processor.

>
> Link: https://lore.kernel.org/linux-riscv/CAJF2gTRy5eK73=d6s7CVy9m9pB8p4rAoMHM3cZFwzg=AuF7TDA@mail.gmail.com/ [1]
> 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>
> ---
> I'd forgotten about this patch, the plan had been to sit on it until
> someone actually reported encountering this, but Palmer asked me to
> resend this as part of reviving the attempt to mainline the basic
> support for vector on these devices.
>
> I guess the riscv,isa-extensions support actually landed after the v2
> was written, so v3 is less complicated since there's full support for
> parsing the new property and all we have to do is block V in the !acpi
> case when parsing riscv,isa now. As Guo Ren suggested, I decreased the
> size of the hammer further, so that the c908, that does implment the
> ratified V extension, can set "v" in riscv,isa. Other than that, what
> changed is just a rebase on top of the changes since July. v2 is in the
> Link: tag above.
>
> cc: charlie@rivosinc.com
> cc: conor.dooley@microchip.com
> cc: linux-riscv@lists.infradead.org
> cc: palmer@dabbelt.com
> cc: guoren@kernel.org
> ---
>  arch/riscv/kernel/cpufeature.c | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index 89920f84d0a3..3a05af7be510 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -24,6 +24,7 @@
>  #include <asm/hwprobe.h>
>  #include <asm/patch.h>
>  #include <asm/processor.h>
> +#include <asm/sbi.h>
>  #include <asm/vector.h>
>
>  #include "copy-unaligned.h"
> @@ -538,6 +539,20 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap)
>                         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.
> +                * CPU cores with the ratified spec will contain non-zero
> +                * marchid.
> +                */
> +               if (acpi_disabled && riscv_cached_mvendorid(cpu) == THEAD_VENDOR_ID &&
> +                   riscv_cached_marchid(cpu) == 0x0) {
> +                       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.43.0
>

-- 
Best Regards
 Guo Ren

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