Date:   Tue, 21 Dec 2021 06:57:37 -0800
From:   Jakub Kicinski <kuba@kernel.org>
To:     Ard Biesheuvel <ardb@kernel.org>
Cc:     Herbert Xu <herbert@gondor.apana.org.au>,
        Linux Crypto Mailing List <linux-crypto@vger.kernel.org>,
        Sabrina Dubroca <sd@queasysnail.net>
Subject: Re: x86 AES crypto alignment
Message-ID: <20211221065737.0db2a746@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
In-Reply-To: <CAMj1kXGV+DHZSOw7t8NgZojMsA6bq-VENz-WxQH+rb8yFj0zyA@mail.gmail.com>
References: <20211207113252.162701ed@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
        <20211208044037.GA11399@gondor.apana.org.au>
        <20211207212907.6e91821b@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
        <20211220150343.4e12a4d2@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
        <20211220161125.78bc4d66@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
        <20211220165251.400813dc@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
        <CAMj1kXG+FGBHr=+vUwVz-u5n7oHpRxikLsOogVW0bOvNow3jHQ@mail.gmail.com>
        <CAMj1kXGV+DHZSOw7t8NgZojMsA6bq-VENz-WxQH+rb8yFj0zyA@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Tue, 21 Dec 2021 08:24:48 +0100 Ard Biesheuvel wrote:
> > Could you check whether this means that gcm_context_data in
> > gcmaes_crypt_by_sg() does not have to be aligned either? It would be
> > nice if we could drop that horrible hack as well.
> 
> I guess you meant by "we take care of the meta-data (key, iv etc.)
> alignment anyway" that we have these hacks for gcm_context_data (which
> carries the key) and the IV, using oversized buffers on the stack and
> open coded realignment.
> 
> It would be really nice if we could just get rid of all of that as
> well, and just use {v}movdqu to load those items.

Yup, exactly. I did something close to s/movdqa/movdqu/ initially,
but doing a competent job removing the alignment assumption would
be more effort. Let's see if I can see the copy if any perf profile...

FWIW there is a comment up top in arch/x86/crypto/aesni-intel_asm.S
which explains the aligned operations were chosen because they have
a shorter encoding. Seems like an intentional choice.