All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ard Biesheuvel <ardb@kernel.org>
To: "Horia Geantă" <horia.geanta@nxp.com>
Cc: Herbert Xu <herbert@gondor.apana.org.au>,
	"Andrei Botila (OSS)" <andrei.botila@oss.nxp.com>,
	Aymen Sghaier <aymen.sghaier@nxp.com>,
	"David S. Miller" <davem@davemloft.net>,
	"linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH RESEND 1/9] crypto: caam/jr - add fallback for XTS with more than 8B IV
Date: Mon, 14 Sep 2020 21:20:00 +0300	[thread overview]
Message-ID: <CAMj1kXH0jOQms9y1MywORywoKjxQ2p8ttv+Xf9KTOkfORX5XWw@mail.gmail.com> (raw)
In-Reply-To: <38f9904b-5bf7-ea99-ed8a-27cb49f405bd@nxp.com>

On Mon, 14 Sep 2020 at 20:12, Horia Geantă <horia.geanta@nxp.com> wrote:
>
> On 9/14/2020 7:28 PM, Ard Biesheuvel wrote:
> > On Mon, 14 Sep 2020 at 19:24, Horia Geantă <horia.geanta@nxp.com> wrote:
> >>
> >> On 9/9/2020 1:10 AM, Herbert Xu wrote:
> >>> On Tue, Sep 08, 2020 at 01:35:04PM +0300, Horia Geantă wrote:
> >>>>
> >>>>> Just go with the get_unaligned unconditionally.
> >>>>
> >>>> Won't this lead to sub-optimal code for ARMv7
> >>>> in case the IV is aligned?
> >>>
> >>> If this should be optimised in ARMv7 then that should be done
> >>> in get_unaligned itself and not open-coded.
> >>>
> >> I am not sure what's wrong with avoiding using the unaligned accessors
> >> in case data is aligned.
> >>
> >> Documentation/core-api/unaligned-memory-access.rst clearly states:
> >> These macros work for memory accesses of any length (not just 32 bits as
> >> in the examples above). Be aware that when compared to standard access of
> >> aligned memory, using these macros to access unaligned memory can be costly in
> >> terms of performance.
> >>
> >> So IMO it makes sense to use get_unaligned() only when needed.
> >> There are several cases of users doing this, e.g. siphash.
> >>
> >
> > For ARMv7 code, using the unaligned accessors unconditionally is fine,
> > and it will not affect performance.
> >
> > In general, when CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS is defined,
> > you can use the unaligned accessors. If it is not, it helps to have
> > different code paths.
> >
> arch/arm/include/asm/unaligned.h doesn't make use of
> linux/unaligned/access_ok.h, even if CONFIG_HAVE_EFFICIENT_UNALIGNED_ACCESS
> is set.
>
> I understand the comment in the file, however using get_unaligned()
> unconditionally takes away the opportunity to generate optimized code
> (using ldrd/ldm) when data is aligned.
>

But the minimal optimization that is possible here (one ldrd/ldm
instruction vs two ldr instructions) is defeated by the fact that you
are using a conditional branch to select between the two. And this is
not even a hot path to begin with,

> > This is a bit murky, and through the years, the interpretation of
> > unaligned-memory-access.rst has shifted a bit, but in this case, it
> > makes no sense to make the distinction.
> >
>
> Thanks,
> Horia

  reply	other threads:[~2020-09-14 18:20 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-06 16:35 [PATCH RESEND 0/9] crypto: caam - xts(aes) updates Andrei Botila
2020-08-06 16:35 ` [PATCH RESEND 1/9] crypto: caam/jr - add fallback for XTS with more than 8B IV Andrei Botila
2020-08-11 14:30   ` Horia Geantă
2020-08-21  3:47     ` Herbert Xu
2020-08-21 12:02       ` Horia Geantă
2020-08-19 23:56   ` Sasha Levin
2020-08-21  3:46   ` Herbert Xu
2020-09-08 10:35     ` Horia Geantă
2020-09-08 22:10       ` Herbert Xu
2020-09-14 16:23         ` Horia Geantă
2020-09-14 16:28           ` Ard Biesheuvel
2020-09-14 17:12             ` Horia Geantă
2020-09-14 18:20               ` Ard Biesheuvel [this message]
2020-09-15 10:02                 ` Horia Geantă
2020-09-15 10:26                   ` Ard Biesheuvel
2020-09-15 12:44                     ` Horia Geantă
2020-09-15 12:51                       ` Ard Biesheuvel
2020-08-06 16:35 ` [PATCH RESEND 2/9] crypto: caam/qi " Andrei Botila
2020-08-19 23:56   ` Sasha Levin
2020-08-06 16:35 ` [PATCH RESEND 3/9] crypto: caam/qi2 " Andrei Botila
2020-08-19 23:56   ` Sasha Levin
2020-08-06 16:35 ` [PATCH RESEND 4/9] crypto: caam/jr - add support for more XTS key lengths Andrei Botila
2020-08-11 14:36   ` Horia Geantă
2020-08-19 23:56   ` Sasha Levin
2020-08-06 16:35 ` [PATCH RESEND 5/9] crypto: caam/qi " Andrei Botila
2020-08-19 23:56   ` Sasha Levin
2020-08-06 16:35 ` [PATCH RESEND 6/9] crypto: caam/qi2 " Andrei Botila
2020-08-19 23:56   ` Sasha Levin
2020-08-06 16:35 ` [PATCH RESEND 7/9] crypto: caam/jr - add support for XTS with 16B IV Andrei Botila
2020-08-06 16:35 ` [PATCH RESEND 8/9] crypto: caam/qi " Andrei Botila
2020-08-06 16:35 ` [PATCH RESEND 9/9] crypto: caam/qi2 " Andrei Botila

Reply instructions:

You may reply publicly 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=CAMj1kXH0jOQms9y1MywORywoKjxQ2p8ttv+Xf9KTOkfORX5XWw@mail.gmail.com \
    --to=ardb@kernel.org \
    --cc=andrei.botila@oss.nxp.com \
    --cc=aymen.sghaier@nxp.com \
    --cc=davem@davemloft.net \
    --cc=herbert@gondor.apana.org.au \
    --cc=horia.geanta@nxp.com \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-kernel@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
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.