Re: [PATCH v6 5/5] riscv: errata: picoheart: Add workaround for AMOCASQ errata

[Jisheng Zhang <jszhang@kernel.org>]

On Wed, Nov 04, 2026 at 03:57:06PM +0800, Yicong Yang wrote:
> Some Picoheart CPUs implement Zacas extension but lack support
> for AMOCASQ PMA attribute. Thus makes the CASQ instruction de
> facto unavailable. Since currently no ways to retrieve the PMA
> information and the riscv kernel will declare cmpxchg128 support
> if platform declare support of Zacas, the use of cmpxchg128 will
> lead to PMA violation and crash the kernel. Add the errata
> workaround to disable the use of cmpxchg128 on the affected CPUs.
> 
> Tested-by: Han Gao <gaohan@iscas.ac.cn>
> Signed-off-by: Yicong Yang <yang.yicong@picoheart.com>
> ---
>  arch/riscv/Kconfig.errata                    | 12 ++++++++++
>  arch/riscv/errata/picoheart/errata.c         | 25 +++++++++++++++++++-
>  arch/riscv/include/asm/cmpxchg.h             | 18 +++++++++++++-
>  arch/riscv/include/asm/errata_list_vendors.h |  3 ++-
>  4 files changed, 55 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig.errata b/arch/riscv/Kconfig.errata
> index c75dec4b0ba7..4e6516ba3a42 100644
> --- a/arch/riscv/Kconfig.errata
> +++ b/arch/riscv/Kconfig.errata
> @@ -202,4 +202,16 @@ config ERRATA_PICOHEART_CBO_CLEAN
>  
>  	   If you don't know what to do here, say "Y".
>  
> +config ERRATA_PICOHEART_AMOCASQ
> +	bool "Apply Picoheart AMOCASQ errata"
> +	depends on ERRATA_PICOHEART && RISCV_ISA_ZACAS
> +	default y
> +	help
> +	  Some Picoheart CPUs implement the Zacas extension but lack support
> +	  for AMOCASQ PMA attribute. Thus makes the CASQ instruction de facto
> +	  unavailable. Enable this errata workaround to disable the use of
> +	  CASQ in the kernel (cmpxchg128).
> +
> +	  If you don't know what to do here, say "Y".
> +
>  endmenu # "CPU errata selection"
> diff --git a/arch/riscv/errata/picoheart/errata.c b/arch/riscv/errata/picoheart/errata.c
> index c945ae694396..a658f38cf4de 100644
> --- a/arch/riscv/errata/picoheart/errata.c
> +++ b/arch/riscv/errata/picoheart/errata.c
> @@ -174,11 +174,34 @@ static void picoheart_errata_probe_cbo_clean(unsigned int stage,
>  	static_branch_enable(&has_picoheart_cbo_clean_errata);
>  }
>  
> +static bool picoheart_errata_probe_amocasq(unsigned int stage,
> +					   unsigned long archid,
> +					   unsigned long impid)
> +{
> +	if (!IS_ENABLED(CONFIG_ERRATA_PICOHEART_AMOCASQ))
> +		return false;
> +
> +	if (!IS_ENABLED(CONFIG_RISCV_ISA_ZACAS) ||
> +	    !riscv_isa_extension_available(NULL, ZACAS))
> +		return false;
> +
> +	if (archid != 0x804a555049544552 || impid != 0x100)
> +		return false;
> +
> +	return true;
> +}
> +
>  static u32 picoheart_errata_probe(unsigned int stage, unsigned long archid,
>  				  unsigned long impid)
>  {
> +	u32 cpu_req_errata = 0;
> +
>  	picoheart_errata_probe_cbo_clean(stage, archid, impid);
> -	return 0;
> +
> +	if (picoheart_errata_probe_amocasq(stage, archid, impid))
> +		cpu_req_errata |= BIT(ERRATA_PICOHEART_AMOCASQ);
> +
> +	return cpu_req_errata;
>  }
>  
>  void picoheart_errata_patch_func(struct alt_entry *begin, struct alt_entry *end,
> diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
> index 662e160b0522..0488b9314c13 100644
> --- a/arch/riscv/include/asm/cmpxchg.h
> +++ b/arch/riscv/include/asm/cmpxchg.h
> @@ -329,7 +329,23 @@
>  
>  #if defined(CONFIG_64BIT) && defined(CONFIG_RISCV_ISA_ZACAS) && defined(CONFIG_TOOLCHAIN_HAS_ZACAS)
>  
> -#define system_has_cmpxchg128()        riscv_has_extension_unlikely(RISCV_ISA_EXT_ZACAS)
> +#define system_has_cmpxchg128 system_has_cmpxchg128
> +static __always_inline bool system_has_cmpxchg128(void)
> +{
> +	/* The vendor erratum overrides the standard Zacas alternative. */
> +	asm goto(ALTERNATIVE_2(
> +			"nop",
> +			"j %l[l_yes]",
> +			STANDARD_EXT, RISCV_ISA_EXT_ZACAS, 1,
> +			"nop",
> +			PICOHEART_VENDOR_ID, ERRATA_PICOHEART_AMOCASQ,
> +			CONFIG_ERRATA_PICOHEART_AMOCASQ)
> +		: : : : l_yes);

This makes the generic code ugly, adds some maintain effort.
why not discarding "zacas" in the dts? It's much simpler

> +
> +	return false;
> +l_yes:
> +	return true;
> +}
>  
>  union __u128_halves {
>  	u128 full;
> diff --git a/arch/riscv/include/asm/errata_list_vendors.h b/arch/riscv/include/asm/errata_list_vendors.h
> index 8160afed8d21..97b7bbdc4984 100644
> --- a/arch/riscv/include/asm/errata_list_vendors.h
> +++ b/arch/riscv/include/asm/errata_list_vendors.h
> @@ -29,7 +29,8 @@
>  #endif
>  
>  #ifdef CONFIG_ERRATA_PICOHEART
> -#define ERRATA_PICOHEART_NUMBER	0
> +#define ERRATA_PICOHEART_AMOCASQ	0
> +#define ERRATA_PICOHEART_NUMBER		1
>  #endif
>  
>  #endif /* ASM_ERRATA_LIST_VENDORS_H */
> -- 
> 2.54.0 (Apple Git-157)
> 
> _______________________________________________
> 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