Re: [PATCH] riscv: Rewrite AMO instructions via lr and sc.

[Conor Dooley]

Hey,

On Fri, Nov 29, 2024 at 03:43:19PM +0100, Aleksandar Rikalo wrote:
> From: Chao-ying Fu <cfu@xxxxxxxx>
> 
> Use lr and sc to implement all atomic functions. Some CPUs have
> native support for lr and sc, but emulate AMO instructions through
> trap handlers that are slow.
> 
> Add config RISCV_ISA_ZALRSC_ONLY.
> 
> Signed-off-by: Chao-ying Fu <cfu@xxxxxxxx>
> Signed-off-by: Aleksandar Rikalo <arikalo@xxxxxxxxx>
> ---
>  arch/riscv/Kconfig               | 10 ++++++
>  arch/riscv/include/asm/atomic.h  | 52 +++++++++++++++++++++++++++++++-
>  arch/riscv/include/asm/bitops.h  | 45 +++++++++++++++++++++++++++
>  arch/riscv/include/asm/cmpxchg.h | 16 ++++++++++
>  arch/riscv/include/asm/futex.h   | 46 ++++++++++++++++++++++++++++
>  5 files changed, 168 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index cc63aef41e94..767538c27875 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -715,6 +715,16 @@ config RISCV_ISA_ZACAS
>  
>  	  If you don't know what to do here, say Y.
>  
> +config RISCV_ISA_ZALRSC_ONLY
> +	bool "Zalrsc extension support only"

I don't agree with the naming of and description for this option.
From the description below I'd imagine that you only care about a
platform where use of AMO instructions is only undesirable, but overall
the option implies that it can be used to build a kernel that can be
used on systems that only implement Zalrsc, which, even with your patch
applied, it cannot. I think, if we are going to merge something like this,
we should go the whole way and permit platforms that don't even emulate
the a extension. If not, the Kconfig option should be explicitly clear
that the a extension is still mandatory.

> +	default n

"default n" is the default, you shouldn't need this line.

> +	help
> +	   Use lr and sc to build all atomic functions. Some CPUs have
> +	   native support for lr and sc, but emulate amo instructions through
> +	   trap handlers that are slow.

Since you mention trap handlers here, it sounds like it may not be the
CPU itself that emulates it, but rather firmware?

Cheers,
Conor.

Attachment: signature.asc
Description: PGP signature