linux-kernel - Re: [PATCH] LoongArch: Provide __lshrti3, __ashrti3, and _

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 for Android: free password hash cracker in your pocket

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

Date: Sat, 27 Apr 2024 15:20:55 +0800
From: Huacai Chen <chenhuacai@...nel.org>
To: Xi Ruoyao <xry111@...111.site>
Cc: WANG Xuerui <kernel@...0n.name>, loongarch@...ts.linux.dev, 
	linux-kernel@...r.kernel.org, Tiezhu Yang <yangtiezhu@...ngson.cn>
Subject: Re: [PATCH] LoongArch: Provide __lshrti3, __ashrti3, and __ashrti3

On Sat, Apr 27, 2024 at 12:13 PM Xi Ruoyao <xry111@...111.site> wrote:
>
> On Sat, 2024-04-27 at 12:00 +0800, Xi Ruoyao wrote:
> > On Sat, 2024-04-27 at 10:50 +0800, Huacai Chen wrote:
> > > Hi, Ruoyao,
> > >
> > > I don't think #ifdef CONFIG_ARCH_SUPPORTS_INT128 is needed here.
> > > S390/ARM64/RISCV all built it unconditionally.
> >
> > The problem here is RISCV and ARM64 are using an incorrect prototype for
> > these functions in asm-prototypes.h:
> >
> > long long __lshrti3(long long a, int b);
> > long long __ashrti3(long long a, int b);
> > long long __ashlti3(long long a, int b);
> >
> > where "long long" is not 128-bit.  Despite this seems working for RISC-V
> > and ARM64 I really dislike it.
> >
> > S390 seems assuming CONFIG_ARCH_SUPPORTS_INT128 is always true, but I
> > don't think we can assume it too (at least it'll likely to be false for
> > LA32, so doing so will cause trouble when we add LA32 support).
> >
> > So if we don't want to check CONFIG_ARCH_SUPPORTS_INT128 and still use a
> > correct prototype, we'll do:
> >
> > diff --git a/arch/loongarch/include/asm/asm-prototypes.h b/arch/loongarch/include/asm/asm-prototypes.h
> > index 51f224bcfc65..0a57db01116d 100644
> > --- a/arch/loongarch/include/asm/asm-prototypes.h
> > +++ b/arch/loongarch/include/asm/asm-prototypes.h
> > @@ -7,8 +7,6 @@
> >  #include <asm/ftrace.h>
> >  #include <asm-generic/asm-prototypes.h>
> >
> > -#ifdef CONFIG_ARCH_SUPPORTS_INT128
> > -__int128_t __ashlti3(__int128_t a, int b);
> > -__int128_t __ashrti3(__int128_t a, int b);
> > -__int128_t __lshrti3(__int128_t a, int b);
> > -#endif
> > +struct { u64 lo, hi; } __ashlti3(u64 lo, u64 hi, int b);
> > +struct { u64 lo, hi; } __ashrti3(u64 lo, u64 hi, int b);
> > +struct { u64 lo, hi; } __lshrti3(u64 lo, u64 hi, int b);
>
> Whoops.  This is still incorrect for LA32.  On LA32 an "int128" (if it
> ever exists) should be passed as a pointer, but this is passing it in 4
> GPRs.  So if we want to keep the prototype correct we need to either use
> "struct { u64 lo, hi; }" in the parameter list too, or guard it with
> #ifdef CONFIG_64BIT.
>
> So to me checking CONFIG_ARCH_SUPPORTS_INT128 is just easier.
>
> If you insists on not checking CONFIG_ARCH_SUPPORTS_INT128 I'll just use
> an incorrect prototype like RISC-V but put a comment here, like:
>
> /* The prototypes are incorrect but this file is only used by
>    modpost which does not care.  */
> long long __ashlti3(long long a, int b);
> long long __ashrti3(long long a, int b);
> long long __lshrti3(long long a, int b);
>
> How do you think?
OK, then just keep the original status.


Huacai
>
> --
> Xi Ruoyao <xry111@...111.site>
> School of Aerospace Science and Technology, Xidian University
>