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

[Conor Dooley <conor@kernel.org>]

[-- Attachment #1.1: Type: text/plain, Size: 6651 bytes --]

On Mon, Sep 07, 2026 at 03:21:49PM +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.
> 
> Signed-off-by: Yicong Yang <yang.yicong@picoheart.com>
> ---
>  arch/riscv/Kconfig.errata            | 12 ++++++++++++
>  arch/riscv/errata/picoheart/errata.c | 22 ++++++++++++++++++++++
>  arch/riscv/include/asm/cmpxchg.h     |  2 +-
>  arch/riscv/include/asm/cpufeature.h  |  8 ++++++++
>  arch/riscv/kernel/cpufeature.c       | 13 ++++++++++++-
>  5 files changed, 55 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/riscv/Kconfig.errata b/arch/riscv/Kconfig.errata
> index 8ce5c0314321..16e37391d596 100644
> --- a/arch/riscv/Kconfig.errata
> +++ b/arch/riscv/Kconfig.errata
> @@ -176,4 +176,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 21480d19c084..0fb4cb41ab03 100644
> --- a/arch/riscv/errata/picoheart/errata.c
> +++ b/arch/riscv/errata/picoheart/errata.c
> @@ -174,10 +174,32 @@ static void picoheart_errata_probe_cbo_clean(unsigned int stage,
>  	static_branch_enable(&has_picoheart_cbo_clean_errata);
>  }
>  
> +static void picoheart_errata_probe_amocasq(unsigned int stage,
> +					   unsigned long archid,
> +					   unsigned long impid)
> +{
> +	if (!IS_ENABLED(CONFIG_ERRATA_PICOHEART_AMOCASQ))
> +		return;
> +
> +	if (stage != RISCV_ALTERNATIVES_BOOT)
> +		return;
> +
> +	if (!IS_ENABLED(CONFIG_RISCV_ISA_ZACAS) ||
> +	    !riscv_isa_extension_available(NULL, ZACAS))
> +		return;
> +
> +	if (archid != 0x804a555049544552 || impid != 0x100)
> +		return;
> +
> +	static_branch_disable(&cpus_support_cmpxchg128);
> +}
> +
>  static u32 picoheart_errata_probe(unsigned int stage, unsigned long archid,
>  				  unsigned long impid)
>  {
>  	picoheart_errata_probe_cbo_clean(stage, archid, impid);
> +	picoheart_errata_probe_amocasq(stage, archid, impid);
> +
>  	return 0;
>  }
>  
> diff --git a/arch/riscv/include/asm/cmpxchg.h b/arch/riscv/include/asm/cmpxchg.h
> index 662e160b0522..e231e9619eee 100644
> --- a/arch/riscv/include/asm/cmpxchg.h
> +++ b/arch/riscv/include/asm/cmpxchg.h
> @@ -329,7 +329,7 @@
>  
>  #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
>  
>  union __u128_halves {
>  	u128 full;
> diff --git a/arch/riscv/include/asm/cpufeature.h b/arch/riscv/include/asm/cpufeature.h
> index 739fcc84bf7b..f9d95cbf5abd 100644
> --- a/arch/riscv/include/asm/cpufeature.h
> +++ b/arch/riscv/include/asm/cpufeature.h
> @@ -164,4 +164,12 @@ static inline bool cpu_supports_indirect_br_lp_instr(void)
>  		riscv_has_extension_unlikely(RISCV_ISA_EXT_ZICFILP));
>  }
>  
> +DECLARE_STATIC_KEY_FALSE(cpus_support_cmpxchg128);
> +
> +static inline bool system_has_cmpxchg128(void)
> +{
> +	return riscv_has_extension_unlikely(RISCV_ISA_EXT_ZACAS) &&
> +	       static_branch_likely(&cpus_support_cmpxchg128);
> +}
> +
>  #endif
> diff --git a/arch/riscv/kernel/cpufeature.c b/arch/riscv/kernel/cpufeature.c
> index b56d149cf4ed..f73ce43df626 100644
> --- a/arch/riscv/kernel/cpufeature.c
> +++ b/arch/riscv/kernel/cpufeature.c
> @@ -46,6 +46,9 @@ struct riscv_isainfo hart_isa[NR_CPUS];
>  
>  u32 thead_vlenb_of;
>  
> +/* All the CPUs supports cmpxchg128 (CASQ*). */
> +DEFINE_STATIC_KEY_FALSE(cpus_support_cmpxchg128);
> +
>  /**
>   * riscv_isa_extension_base() - Get base extension word
>   *
> @@ -317,6 +320,14 @@ static int riscv_cfiss_validate(const struct riscv_isa_ext_data *data,
>  	return 0;
>  }
>  
> +static int riscv_ext_zacas_validate(const struct riscv_isa_ext_data *data,
> +				    const unsigned long *isa_bitmap)
> +{
> +	static_branch_enable(&cpus_support_cmpxchg128);

A validate callback is meant to determine whether or not the extension
can be used, it's not there for people to add arbitrary code, partially
because each validate callback runs multiple times and may change state
depending on what happens with other extensions. E.g. a validate
callback may pass on the first iteration but fail on the second because
a dependant extension later in the list fails. In practice, this might
not ever affect Zacas, and doesn't at the moment, but this sort of code
should not be added to validate callbacks.

To be honest, I'd rather just turn the extension off entirely on this
platform rather than make everyone suffer.
The errata probe functions run after extension detection but before
patching, so could we just clear the Zacas bit in the isa bitmap?

Cheers,
Conor.

> +
> +	return 0;
> +}
> +
>  static const unsigned int riscv_a_exts[] = {
>  	RISCV_ISA_EXT_ZAAMO,
>  	RISCV_ISA_EXT_ZALRSC,
> @@ -544,7 +555,7 @@ const struct riscv_isa_ext_data riscv_isa_ext[] = {
>  	__RISCV_ISA_EXT_DATA(za64rs, RISCV_ISA_EXT_ZA64RS),
>  	__RISCV_ISA_EXT_DATA(zaamo, RISCV_ISA_EXT_ZAAMO),
>  	__RISCV_ISA_EXT_DATA(zabha, RISCV_ISA_EXT_ZABHA),
> -	__RISCV_ISA_EXT_DATA(zacas, RISCV_ISA_EXT_ZACAS),
> +	__RISCV_ISA_EXT_DATA_VALIDATE(zacas, RISCV_ISA_EXT_ZACAS, riscv_ext_zacas_validate),
>  	__RISCV_ISA_EXT_DATA(zalasr, RISCV_ISA_EXT_ZALASR),
>  	__RISCV_ISA_EXT_DATA(zalrsc, RISCV_ISA_EXT_ZALRSC),
>  	__RISCV_ISA_EXT_DATA(zawrs, RISCV_ISA_EXT_ZAWRS),
> -- 
> 2.50.1 (Apple Git-155)
> 
> _______________________________________________
> linux-riscv mailing list
> linux-riscv@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-riscv

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

[-- Attachment #2: Type: text/plain, Size: 161 bytes --]

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