Re: [PATCH 4/7] x86: Add SSSE3 optimized chacha20

[Noah Goldstein <goldstein.w.n@gmail.com>]

On Thu, Apr 14, 2022 at 12:03 PM Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>
>
>
> On 13/04/2022 20:12, Noah Goldstein wrote:
> > On Wed, Apr 13, 2022 at 1:27 PM Adhemerval Zanella via Libc-alpha
> > <libc-alpha@sourceware.org> wrote:
> >>
> >> +
> >> +       /* eax zeroed by round loop. */
> >> +       leave;
> >> +       cfi_adjust_cfa_offset(-8)
> >> +       cfi_def_cfa_register(%rsp);
> >> +       ret;
> >> +       int3;
> > why int3?
>
> It was originally added on libgcrypt by 11ade08efbfbc36dbf3571f1026946269950bc40,
> as a straight-line speculation hardening.  It is was is emitted by clang 14 and
> gcc 12 with -mharden-sls=return.
>
> I am not sure if we need that kind of hardening, but I would prefer to the first
> version be in sync with libgcrypt as much as possible so the future optimizations
> would be simpler to keep localized to glibc (if libgcrypt does not want to
> backport it).

Okay, can keep for now. Any thoughts on changing it to sse2?


>
> >> +END (__chacha20_ssse3_blocks8)
> >> diff --git a/sysdeps/x86_64/chacha20_arch.h b/sysdeps/x86_64/chacha20_arch.h
> >> new file mode 100644
> >> index 0000000000..37a4fdfb1f
> >> --- /dev/null
> >> +++ b/sysdeps/x86_64/chacha20_arch.h
> >> @@ -0,0 +1,42 @@
> >> +/* Chacha20 implementation, used on arc4random.
> >> +   Copyright (C) 2022 Free Software Foundation, Inc.
> >> +   This file is part of the GNU C Library.
> >> +
> >> +   The GNU C Library is free software; you can redistribute it and/or
> >> +   modify it under the terms of the GNU Lesser General Public
> >> +   License as published by the Free Software Foundation; either
> >> +   version 2.1 of the License, or (at your option) any later version.
> >> +
> >> +   The GNU C Library is distributed in the hope that it will be useful,
> >> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> >> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> >> +   Lesser General Public License for more details.
> >> +
> >> +   You should have received a copy of the GNU Lesser General Public
> >> +   License along with the GNU C Library; if not, see
> >> +   <http://www.gnu.org/licenses/>.  */
> >> +
> >> +#include <ldsodefs.h>
> >> +#include <cpu-features.h>
> >> +#include <sys/param.h>
> >> +
> >> +unsigned int __chacha20_ssse3_blocks8 (uint32_t *state, uint8_t *dst,
> >> +                                      const uint8_t *src, size_t nblks);
> >> +
> >> +static inline void
> >> +chacha20_crypt (struct chacha20_state *state, uint8_t *dst, const uint8_t *src,
> >> +               size_t bytes)
> >> +{
> >> +  if (CPU_FEATURE_USABLE_P (cpu_features, SSSE3) && bytes >= CHACHA20_BLOCK_SIZE * 4)
> >
> > Can we make this an ifunc?
>
> I though about it, but if you check on arc4random implementation the
> chacha20_crypt is called for the whole internal buf once it is exhausted.
> Assuming a 1 cycle per byte (as indicated by bench-slope libgrcypt on
> my machine), it will be at least 1k cycles to encrypt each block.  I
> am not sure if setting up an internal PLT call to save a couple of cycles
> on a internal function will really show anything significant here (assuming
> that the PLT call won't add more overhead in fact).
>
> Besides that the code boilerplate to setup the internal ifunc is also
> way more complex.

Okay for now as long as open to changing later (not that we will but that
this isn't locking us into the decision).