Linux-Crypto Archive on lore.kernel.org
 help / color / Atom feed
From: Nick Desaulniers <ndesaulniers@google.com>
To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: "open list:HARDWARE RANDOM NUMBER GENERATOR CORE" 
	<linux-crypto@vger.kernel.org>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	Eric Biggers <ebiggers@kernel.org>
Subject: Re: [PATCH v2 3/3] crypto: arm64/aegis128 - implement plain NEON version
Date: Mon, 12 Aug 2019 10:31:24 -0700
Message-ID: <CAKwvOd=9KP9j7SkyCJ6xBWmVQn8nSsP78PasdtBO5aDFcSm2Rg@mail.gmail.com> (raw)
In-Reply-To: <CAKv+Gu9KKvCXAm=1hJ-owkL4BTi=hmCXA8ag_rTvdnUgn_zvUg@mail.gmail.com>

On Mon, Aug 12, 2019 at 10:22 AM Ard Biesheuvel
<ard.biesheuvel@linaro.org> wrote:
>
> On Mon, 12 Aug 2019 at 19:50, Nick Desaulniers <ndesaulniers@google.com> wrote:
> >
> > On Sun, Aug 11, 2019 at 3:59 PM Ard Biesheuvel
> > <ard.biesheuvel@linaro.org> wrote:
> > > diff --git a/crypto/Makefile b/crypto/Makefile
> > > index 99a9fa9087d1..0d2cdd523fd9 100644
> > > --- a/crypto/Makefile
> > > +++ b/crypto/Makefile
> > > @@ -98,7 +98,14 @@ CFLAGS_aegis128-neon-inner.o += -mfpu=crypto-neon-fp-armv8
> > >  aegis128-$(CONFIG_CRYPTO_AEGIS128_SIMD) += aegis128-neon.o aegis128-neon-inner.o
> > >  endif
> > >  ifeq ($(ARCH),arm64)
> > > -CFLAGS_aegis128-neon-inner.o += -ffreestanding -mcpu=generic+crypto
> > > +aegis128-cflags-y := -ffreestanding -mcpu=generic+crypto
> > > +aegis128-cflags-$(CONFIG_CC_IS_GCC) += -ffixed-q16 -ffixed-q17 -ffixed-q18 \
> > > +                                      -ffixed-q19 -ffixed-q20 -ffixed-q21 \
> > > +                                      -ffixed-q22 -ffixed-q23 -ffixed-q24 \
> > > +                                      -ffixed-q25 -ffixed-q26 -ffixed-q27 \
> > > +                                      -ffixed-q28 -ffixed-q29 -ffixed-q30 \
> > > +                                      -ffixed-q31
> >
> > I've filed https://bugs.llvm.org/show_bug.cgi?id=42974 for a feature
> > request for this in Clang.
> >
>
> Good. But even GCC has issues here. Most notably, something like
>
> register uint8x16_t foo asm ("v16");
>
> should permit a register that is excluded from general allocation to
> be used explicitly, but this throws a warning on GCC and an error with
> Clang.

Consider filing bugs against GCC's issue tracker so that they're aware
of the issue if you think there's more that can be improved on their
end (for bugs in Clang, I'm always happy to help submit bug reports).
What is the warning?

for -ffixed-q* and `asm ("v16")`, on aarch64, what are the q registers
and v registers?  I assume they're related to NEON, but I'd only even
worked with w* and x* GPRs.  I *think* the explicit register syntax
works for GPRs in Clang; maybe the v* and q* registers being broken is
just oversight and can be fixed.

> > With those 2 recommendations:
> > Acked-by: Nick Desaulniers <ndesaulniers@google.com>
> > in regards to compiling w/ Clang.  Someone else should review the
> > implementation of this crypto routine.
-- 
Thanks,
~Nick Desaulniers

  reply index

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-11 22:59 [PATCH v2 0/3] crypto: aegis128 followup Ard Biesheuvel
2019-08-11 22:59 ` [PATCH v2 1/3] crypto: aegis128 - add support for SIMD acceleration Ard Biesheuvel
2019-08-11 22:59 ` [PATCH v2 2/3] crypto: aegis128 - provide a SIMD implementation based on NEON intrinsics Ard Biesheuvel
2019-08-11 22:59 ` [PATCH v2 3/3] crypto: arm64/aegis128 - implement plain NEON version Ard Biesheuvel
2019-08-12 16:50   ` Nick Desaulniers
2019-08-12 17:22     ` Ard Biesheuvel
2019-08-12 17:31       ` Nick Desaulniers [this message]
2019-08-12 18:14         ` Ard Biesheuvel
2019-08-15 12:08 ` [PATCH v2 0/3] crypto: aegis128 followup Herbert Xu

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAKwvOd=9KP9j7SkyCJ6xBWmVQn8nSsP78PasdtBO5aDFcSm2Rg@mail.gmail.com' \
    --to=ndesaulniers@google.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=ebiggers@kernel.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=linux-crypto@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Crypto Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-crypto/0 linux-crypto/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-crypto linux-crypto/ https://lore.kernel.org/linux-crypto \
		linux-crypto@vger.kernel.org
	public-inbox-index linux-crypto

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-crypto


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git