Re: [PATCH] riscv: Fix the PAUSE Opcode for MIPS P8700.

From: Olof Johansson <olof@lixom.net>
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>,
	"Andrew Jones" <ajones@ventanamicro.com>,
	"Christoph Müllner" <christoph.muellner@vrull.eu>,
	linux-kernel@vger.kernel.org,
	"Djordje Todorovic" <djordje.todorovic@htecgroup.com>
Subject: Re: [PATCH] riscv: Fix the PAUSE Opcode for MIPS P8700.
Date: Thu, 19 Dec 2024 10:45:46 -0800	[thread overview]
Message-ID: <Z2Rp2q2HCAMbD0WA@chonkvm.lixom.net> (raw)
In-Reply-To: <20241218133340.51003-1-arikalo@gmail.com>

On Wed, Dec 18, 2024 at 02:33:40PM +0100, Aleksandar Rikalo wrote:
> From: Raj Vishwanathan4 <rvishwanathan@mips.com>
> 
> The riscv MIPS P8700 uses a different opcode for PAUSE.
> It is a ‘hint’ encoding of the SLLI instruction, with rd=0, rs1=0 and
> imm=5. It will behave as a NOP instruction if no additional behavior
> beyond that of SLLI is implemented.
> 
> Signed-off-by: Raj Vishwanathan4 <rvishwanathan@mips.com>
> Signed-off-by: Aleksandar Rikalo <arikalo@gmail.com>
> ---
>  arch/riscv/Kconfig                | 12 ++++++++++++
>  arch/riscv/include/asm/insn-def.h |  5 +++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
> index d4a7ca0388c0..07a77fa0a014 100644
> --- a/arch/riscv/Kconfig
> +++ b/arch/riscv/Kconfig
> @@ -716,6 +716,18 @@ config RISCV_ISA_ZACAS
>  
>  	  If you don't know what to do here, say Y.
>  
> +config RISCV_MIPS_P8700_PAUSE_OPCODE_FIX
> +	bool "Fix the PAUSE Opcode for MIPS P8700"
> +	default n
> +	help
> +	   The RISCV MIPS P8700 uses a different opcode for PAUSE.
> +	   It is a 'hint' encoding of the SLLI instruction,
> +	   with rd=0, rs1=0 and imm=5. It will behave as a NOP
> +	   instruction if no additional behavior beyond that of
> +	   SLLI is implemented.
> +
> +	   If you are not using the P8700 processor, say n.
> +
>  config TOOLCHAIN_HAS_ZBB
>  	bool
>  	default y
> diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
> index 9a913010cdd9..793eedde3516 100644
> --- a/arch/riscv/include/asm/insn-def.h
> +++ b/arch/riscv/include/asm/insn-def.h
> @@ -196,7 +196,12 @@
>  	INSN_I(OPCODE_MISC_MEM, FUNC3(2), __RD(0),		\
>  	       RS1(base), SIMM12(4))
>  
> +#ifndef CONFIG_RISCV_MIPS_P8700_PAUSE_OPCODE_FIX
>  #define RISCV_PAUSE	".4byte 0x100000f"
> +#else
> +#define RISCV_PAUSE	".4byte 0x00501013"
> +#endif
> +
>  #define ZAWRS_WRS_NTO	".4byte 0x00d00073"
>  #define ZAWRS_WRS_STO	".4byte 0x01d00073"

This means that a kernel that's built for MIPS P8700 won't do the right
thing on other platforms, i.e. single platform builds. That's not the
right approach.

It would make more sense to treat this as an errata with boot time
patchup.

-Olof

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