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

[Jisheng Zhang <jszhang@kernel.org>]

On Sat, Nov 07, 2026 at 11:10:59PM +0800, Yicong Yang wrote:
> On 11/7/26 9:34 PM, Jisheng Zhang wrote:
> > 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
> 
> it'll prevent the usage of other more widely used CAS instructions
> in both user and kernel space. we'd like to retain the performance
> benefit of CAS over LR/SC for primitives such as cmpxchg() on our
> server as well.

If the Zacas can't be safely exposed on this platform, the ISA extension
should be disabled, rather than adding vendor specific exceptions to
generic cap detecion which adds unnecessary complexity and maintenance
burden to generic code. It also sets an undesirable precedent for
handling similar errata in the future. Imagine if various vendors don't
implement various extensions correctly, then the riscv arch code will
be full of this kind of specific exceptions here and there, which make
thes fragmentation in RISC-V land situation worse.

> 
> since kernel already provides system_has_cmpxchg128() as a
> separate capability check, disabling 128-bit CAS here lets us
> limit the workaround to cmpxhg128 only and could be a reasonable

The workaround only affects kernel's cap check, but doesn't prevent
userspace from executing the affected instruction if userspace uses
hwprobe etc. to dynamically detect the underlying hw supported
extensions. Therefore, this workaround does not fully address the
errata and could leave userspace exposed to the same issue.

Disabling the affected extension at the platform level is a more
consistent approach.

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