On Tue, Oct 16, 2018 at 03:35:01PM +0200, Alberto Garcia wrote:
> On Tue 16 Oct 2018 12:09:15 PM CEST, Daniel P. Berrangé wrote:
> > Using 64-bit arithmetic increases the performance for xts-aes-128
> > when built with gcrypt:
> >
> > Encrypt: 355 MB/s -> 545 MB/s
> > Decrypt: 362 MB/s -> 568 MB/s
> >
> > Signed-off-by: Daniel P. Berrangé <berra...@redhat.com>
>
> This patch is also fine, but I have a couple of minor comments:
>
> > +static void xts_mult_x(xts_uint128 *I)
> > +{
> > + uint64_t tt;
> > +
> > + xts_uint128_cpu_to_les(I);
> > +
> > + tt = I->u[0] >> 63;
> > + I->u[0] = I->u[0] << 1;
>
> Perhaps I->u[0] <<= 1 , for clarity and consistency with the following
> line (I->u[0] ^= 0x87) ? But I don't mind if you prefer to keep it as is
> now.
In fact I could do the following:
@@ -59,12 +59,13 @@ static void xts_mult_x(xts_uint128 *I)
xts_uint128_cpu_to_les(I);
tt = I->u[0] >> 63;
- I->u[0] = I->u[0] << 1;
+ I->u[0] <<= 1;
if (I->u[1] >> 63) {
I->u[0] ^= 0x87;
}
- I->u[1] = (I->u[1] << 1) | tt;
+ I->u[1] <<= 1;
+ I->u[1] |= tt;
xts_uint128_le_to_cpus(I);
}
either way it generates the exact same asm code
>
> > + if (I->u[1] >> 63) {
> > + I->u[0] ^= 0x87;
> > }
> > + I->u[1] = (I->u[1] << 1) | tt;
> > +
> > + xts_uint128_le_to_cpus(I);
>
> I think both endianness conversion calls should be flipped. First you
> convert from the buffer byte order (LE) to the CPU byte order so you can
> do the bit shifts, then back to the original byte order (LE).
>
> Changing this doesn't have any practical effect because both calls
> perform the exact same operation, but it documents better what's going
> on.
Yep, ok
> With this changed,
>
> Reviewed-by: Alberto Garcia <be...@igalia.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
