Received-SPF: pass (google.com: domain of linux-crypto-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20200626080429.155450-1-giovanni.cabiddu@intel.com>
 <20200626080429.155450-5-giovanni.cabiddu@intel.com> <CAMj1kXGu4_Fp=0i9FUJuRUknsUrf0Ci=r9gMb5+Zf+hVXN4-rw@mail.gmail.com>
 <20200629170353.GA2750@silpixa00400314>
In-Reply-To: <20200629170353.GA2750@silpixa00400314>
From:   Ard Biesheuvel <ardb@kernel.org>
Date:   Tue, 30 Jun 2020 12:09:46 +0200
Message-ID: <CAMj1kXFW00GxG1dzqKMAqRjLT=6u4sWQAmft5k+f0E7++LcD=A@mail.gmail.com>
Subject: Re: [PATCH v2 4/4] crypto: qat - fallback for xts with 192 bit keys
To:     Giovanni Cabiddu <giovanni.cabiddu@intel.com>
Cc:     Herbert Xu <herbert@gondor.apana.org.au>,
        Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
        qat-linux@intel.com
Content-Type: text/plain; charset="UTF-8"
Sender: linux-crypto-owner@vger.kernel.org
Precedence: bulk

On Mon, 29 Jun 2020 at 19:05, Giovanni Cabiddu
<giovanni.cabiddu@intel.com> wrote:
>
> Thanks for your feedback Ard.
>
> On Fri, Jun 26, 2020 at 08:15:16PM +0200, Ard Biesheuvel wrote:
> > On Fri, 26 Jun 2020 at 10:04, Giovanni Cabiddu
> > <giovanni.cabiddu@intel.com> wrote:
> > >
> > > +static int qat_alg_skcipher_init_xts_tfm(struct crypto_skcipher *tfm)
> > > +{
> > > +       struct qat_alg_skcipher_ctx *ctx = crypto_skcipher_ctx(tfm);
> > > +       int reqsize;
> > > +
> > > +       ctx->ftfm = crypto_alloc_skcipher("xts(aes)", 0, CRYPTO_ALG_ASYNC);
> >
> > Why are you only permitting synchronous fallbacks? If the logic above
> > is sound, and copies the base.complete and base.data fields as well,
> > the fallback can complete asynchronously without problems.
> > Note that SIMD s/w implementations of XTS(AES) are asynchronous as
> > well, as they use the crypto_simd helper which queues requests for
> > asynchronous completion if the context from which the request was
> > issued does not permit access to the SIMD register file (e.g., softirq
> > context on some architectures, if the interrupted context is also
> > using SIMD)
> I did it this way since I though I didn't have a way to test it with an
> asynchronous sw implementation.
> I changed this line to avoid masking the asynchronous implementations
> and test it by forcing simd.c to use always cryptd (don't know if there
> is a simpler way to do it).
>

This is exactly how I tested it in the past, but note that the
extended testing that Eric implemented will also run from a context
where SIMD is disabled artificially, and so you should be getting this
behavior in any case.

> Also, I added to the mask CRYPTO_ALG_NEED_FALLBACK so I don't get another
> implementation that requires a fallback.
>
> I'm going to send a v3.
>
> Regards,
>
> --
> Giovanni