linux-kernel - Re: [PATCH v4 6/7] riscv: Add tools support for xmipsexectl

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: <20250626-af013235ad8d22421b2fe5b1@orel>
Date: Thu, 26 Jun 2025 12:49:37 +0200
From: Andrew Jones <ajones@...tanamicro.com>
To: Aleksa Paunovic via B4 Relay <devnull+aleksa.paunovic.htecgroup.com@...nel.org>
Cc: Rob Herring <robh@...nel.org>, 
	Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley <conor+dt@...nel.org>, 
	Paul Walmsley <paul.walmsley@...ive.com>, Palmer Dabbelt <palmer@...belt.com>, 
	Albert Ou <aou@...s.berkeley.edu>, Alexandre Ghiti <alex@...ti.fr>, 
	Jonathan Corbet <corbet@....net>, Palmer Dabbelt <palmer@...ive.com>, 
	Conor Dooley <conor@...nel.org>, devicetree@...r.kernel.org, linux-riscv@...ts.infradead.org, 
	linux-kernel@...r.kernel.org, linux-doc@...r.kernel.org, 
	Aleksa Paunovic <aleksa.paunovic@...cgroup.com>
Subject: Re: [PATCH v4 6/7] riscv: Add tools support for xmipsexectl

On Thu, Jun 26, 2025 at 11:34:21AM +0200, Andrew Jones wrote:
> On Thu, Jun 26, 2025 at 11:21:10AM +0200, Andrew Jones wrote:
> > On Wed, Jun 25, 2025 at 04:21:01PM +0200, Aleksa Paunovic via B4 Relay wrote:
> > > From: Aleksa Paunovic <aleksa.paunovic@...cgroup.com>
> > > 
> > > Use the hwprobe syscall to decide which PAUSE instruction to execute in
> > > userspace code.
> > > 
> > > Signed-off-by: Aleksa Paunovic <aleksa.paunovic@...cgroup.com>
> > > ---
> > >  tools/arch/riscv/include/asm/vdso/processor.h | 27 +++++++++++++++++----------
> > >  1 file changed, 17 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/tools/arch/riscv/include/asm/vdso/processor.h b/tools/arch/riscv/include/asm/vdso/processor.h
> > > index 662aca03984817f9c69186658b19e9dad9e4771c..027219a486b7b93814888190f8224af29498707c 100644
> > > --- a/tools/arch/riscv/include/asm/vdso/processor.h
> > > +++ b/tools/arch/riscv/include/asm/vdso/processor.h
> > > @@ -4,26 +4,33 @@
> > >  
> > >  #ifndef __ASSEMBLY__
> > >  
> > > +#include <asm/hwprobe.h>
> > > +#include <sys/hwprobe.h>
> > > +#include <asm/vendor/mips.h>
> > >  #include <asm-generic/barrier.h>
> > >  
> > >  static inline void cpu_relax(void)
> > >  {
> > > +	struct riscv_hwprobe pair;
> > > +	bool has_mipspause;
> > >  #ifdef __riscv_muldiv
> > >  	int dummy;
> > >  	/* In lieu of a halt instruction, induce a long-latency stall. */
> > >  	__asm__ __volatile__ ("div %0, %0, zero" : "=r" (dummy));
> > >  #endif
> > >  
> > > -#ifdef CONFIG_TOOLCHAIN_HAS_ZIHINTPAUSE
> > > -	/*
> > > -	 * Reduce instruction retirement.
> > > -	 * This assumes the PC changes.
> > > -	 */
> > > -	__asm__ __volatile__ ("pause");
> > > -#else
> > > -	/* Encoding of the pause instruction */
> > > -	__asm__ __volatile__ (".4byte 0x100000F");
> > > -#endif
> > > +	pair.key = RISCV_HWPROBE_KEY_VENDOR_EXT_MIPS_0;
> > > +	__riscv_hwprobe(&pair, 1, 0, NULL, 0);
> > > +	has_mipspause = pair.value & RISCV_HWPROBE_VENDOR_EXT_XMIPSEXECTL;
> > > +
> > > +	if (has_mipspause) {
> > > +		/* Encoding of the mips pause instruction */
> > > +		__asm__ __volatile__(".4byte 0x00501013");
> > > +	} else {
> > > +		/* Encoding of the pause instruction */
> > > +		__asm__ __volatile__(".4byte 0x100000F");
> > > +	}
> > > +
> > 
> > cpu_relax() is used in places where we cannot afford the overhead nor call
> > arbitrary functions which may take locks, etc. We've even had trouble
> > using a static key here in the past since this is inlined and it bloated
> > the size too much. You'll need to use ALTERNATIVE().
> 
> Oh, I see now that the next patch is handling the kernel cpu_relax with
> ALTERNATIVE and this was just the tools cpu_relax. We don't want to make
> a syscall inside cpu_relax though either, since it gets called in loops.

(Another follow up to myself...)

I guess with the vdso cached result it should only be a handful of
instructions, but it still seems odd to embed a call in cpu_relax.

Thanks,
drew

> It'd be better to just call the standard pause (0x100000F) even if it
> does nothing. Or maybe there's some define that can be added/used to
> select the correct instruction?
> 
> Thanks,
> drew