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: <b579eb63-c465-45df-9558-a11c02fdb02b@htecgroup.com>
Date: Fri, 27 Jun 2025 08:40:53 +0000
From: Aleksa Paunovic <aleksa.paunovic@...cgroup.com>
To: "ajones@...tanamicro.com" <ajones@...tanamicro.com>
CC: Aleksa Paunovic <aleksa.paunovic@...cgroup.com>, "alex@...ti.fr"
	<alex@...ti.fr>, "aou@...s.berkeley.edu" <aou@...s.berkeley.edu>,
	"conor+dt@...nel.org" <conor+dt@...nel.org>, "conor@...nel.org"
	<conor@...nel.org>, "corbet@....net" <corbet@....net>,
	"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
	"devnull+aleksa.paunovic.htecgroup.com@...nel.org"
	<devnull+aleksa.paunovic.htecgroup.com@...nel.org>, "krzk+dt@...nel.org"
	<krzk+dt@...nel.org>, "linux-doc@...r.kernel.org"
	<linux-doc@...r.kernel.org>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-riscv@...ts.infradead.org"
	<linux-riscv@...ts.infradead.org>, "palmer@...belt.com" <palmer@...belt.com>,
	"palmer@...ive.com" <palmer@...ive.com>, "paul.walmsley@...ive.com"
	<paul.walmsley@...ive.com>, "robh@...nel.org" <robh@...nel.org>
Subject: Re: [PATCH v4 6/7] riscv: Add tools support for xmipsexectl

On 26. 6. 25. 12:49, Andrew Jones wrote:> 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.
>
  
Hi Andrew,

Thank you for your comments!

> 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?
>>

We did try using an ifdef/else in v3, but since that would have to be marked
non-portable, we decided to go with a hwprobe call. 
Since the MIPS pause should behave as a nop on other CPUs,
would leaving both the standard pause and the MIPS pause calls be an acceptable solution?

That said, I am not sure how this would behave on future MIPS CPUs in case they support both encodings.

Best regards,
Aleksa

>> Thanks,
>> drew