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

From: Conor Dooley <conor@kernel.org>
To: Aleksandar Rikalo <arikalo@gmail.com>
Cc: linux-riscv@lists.infradead.org,
	Paul Walmsley <paul.walmsley@sifive.com>,
	Palmer Dabbelt <palmer@dabbelt.com>,
	Albert Ou <aou@eecs.berkeley.edu>, Will Deacon <will@kernel.org>,
	Peter Zijlstra <peterz@infradead.org>,
	Boqun Feng <boqun.feng@gmail.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Yury Norov <yury.norov@gmail.com>,
	Rasmus Villemoes <linux@rasmusvillemoes.dk>,
	Andrea Parri <parri.andrea@gmail.com>,
	Leonardo Bras <leobras@redhat.com>, Guo Ren <guoren@kernel.org>,
	Samuel Holland <samuel.holland@sifive.com>,
	Eric Chan <ericchancf@google.com>,
	linux-kernel@vger.kernel.org,
	Djordje Todorovic <djordje.todorovic@htecgroup.com>
Subject: Re: [PATCH] riscv: Rewrite AMO instructions via lr and sc.
Date: Fri, 29 Nov 2024 16:29:14 +0000	[thread overview]
Message-ID: <20241129-antivirus-blunderer-efcbd49cfeb6@spud> (raw)
In-Reply-To: <20241129144319.74257-1-arikalo@gmail.com>

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

Hey,

On Fri, Nov 29, 2024 at 03:43:19PM +0100, Aleksandar Rikalo wrote:
> From: Chao-ying Fu <cfu@mips.com>
> 
> 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@mips.com>
> Signed-off-by: Aleksandar Rikalo <arikalo@gmail.com>
> ---
>  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 #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