linux-kernel - Re: [PATCH v3] riscv: Fix the PAUSE Opcode for MIPS P8700.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250129-museum-slider-3bb634d124de@spud>
Date: Wed, 29 Jan 2025 16:19:58 +0000
From: Conor Dooley <conor@...nel.org>
To: Aleksandar Rikalo <arikalo@...il.com>
Cc: linux-riscv@...ts.infradead.org,
	Paul Walmsley <paul.walmsley@...ive.com>,
	Palmer Dabbelt <palmer@...belt.com>,
	Albert Ou <aou@...s.berkeley.edu>,
	Andrew Jones <ajones@...tanamicro.com>,
	Christoph Müllner <christoph.muellner@...ll.eu>,
	linux-kernel@...r.kernel.org,
	Djordje Todorovic <djordje.todorovic@...cgroup.com>
Subject: Re: [PATCH v3] riscv: Fix the PAUSE Opcode for MIPS P8700.

On Wed, Jan 29, 2025 at 02:17:03PM +0100, Aleksandar Rikalo wrote:
> From: Djordje Todorovic <djordje.todorovic@...cgroup.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.

You say p8700, but the erratum will be applied on all systems that are
identified as using a mips cpu. Why's that?

> +void mips_errata_patch_func(struct alt_entry *begin,
> +					     struct alt_entry *end,
> +					     unsigned long archid,
> +					     unsigned long impid,
> +					     unsigned int stage)
> +{
> +	struct alt_entry *alt;
> +
> +	BUILD_BUG_ON(ERRATA_MIPS_NUMBER >= RISCV_VENDOR_EXT_ALTERNATIVES_BASE);
> +
> +	if (stage == RISCV_ALTERNATIVES_EARLY_BOOT)
> +		return;
> +
> +	for (alt = begin; alt < end; alt++) {
> +		if (alt->vendor_id != MIPS_VENDOR_ID)
> +			continue;
> +
> +		if (alt->patch_id >= ERRATA_MIPS_NUMBER) {
> +			WARN(1, "MIPS errata id:%d not in kernel errata list\n",
> +			     alt->patch_id);
> +			continue;
> +		}
> +
> +		mutex_lock(&text_mutex);
> +		patch_text_nosync(ALT_OLD_PTR(alt), ALT_ALT_PTR(alt), alt->alt_len);
> +		mutex_unlock(&text_mutex);
> +	}
> +}

> diff --git a/tools/arch/riscv/include/asm/vdso/processor.h b/tools/arch/riscv/include/asm/vdso/processor.h
> index 662aca039848..880f26a24f69 100644
> --- a/tools/arch/riscv/include/asm/vdso/processor.h
> +++ b/tools/arch/riscv/include/asm/vdso/processor.h
> @@ -14,7 +14,10 @@ static inline void cpu_relax(void)
>  	__asm__ __volatile__ ("div %0, %0, zero" : "=r" (dummy));
>  #endif
>  
> -#ifdef CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE
> +#ifdef CONFIG_ERRATA_MIPS_P8700_PAUSE_OPCODE
> +    /* MIPS P8700 pause opcode */
> +    __asm__ __volatile__ (".4byte 0x00501013");
> +#elif CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE
>  	/*
>  	 * Reduce instruction retirement.
>  	 * This assumes the PC changes.

What about when the erratum is enabled and the toolchain supports
Zihintpause? Why don't you use the same implementation as the !tools
copy of the header? (I'm not sure why they're different in the first
place).

Download attachment "signature.asc" of type "application/pgp-signature" (229 bytes)