On Tue, Oct 16, 2018 at 03:09:16PM +0200, Alberto Garcia wrote:
> On Tue 16 Oct 2018 12:09:14 PM CEST, Daniel P. Berrangé wrote:
>
> > @@ -110,20 +111,34 @@ void xts_decrypt(const void *datactx,
> > /* encrypt the iv */
> > encfunc(tweakctx, XTS_BLOCK_SIZE, T.b, iv);
> >
> > - for (i = 0; i < lim; i++) {
> > - xts_tweak_encdec(datactx, decfunc, src, dst, T.b);
> > -
> > - src += XTS_BLOCK_SIZE;
> > - dst += XTS_BLOCK_SIZE;
> > + if (QEMU_PTR_IS_ALIGNED(src, sizeof(uint64_t)) &&
> > + QEMU_PTR_IS_ALIGNED(dst, sizeof(uint64_t))) {
> > + xts_uint128 *S = (xts_uint128 *)src;
> > + xts_uint128 *D = (xts_uint128 *)dst;
> > + for (i = 0; i < lim; i++, S++, D++) {
> > + xts_tweak_encdec(datactx, decfunc, S, D, &T);
> > + }
> > + } else {
> > + xts_uint128 S, D;
> > +
> > + for (i = 0; i < lim; i++) {
> > + memcpy(&S, src, XTS_BLOCK_SIZE);
> > + xts_tweak_encdec(datactx, decfunc, &S, &D, &T);
> > + memcpy(dst, &D, XTS_BLOCK_SIZE);
> > + src += XTS_BLOCK_SIZE;
> > + dst += XTS_BLOCK_SIZE;
> > + }
>
> The patch looks good to me, but a couple of comments:
>
> - As far as I can see xts_tweak_encdec() works the same regardless of
> whether src and dst point to the same address or not. As a matter of
> fact both qcrypto_block_decrypt() and qcrypto_block_encrypt() do the
> decryption and encryption in place, and as you can see the
> qcrypto_cipher_*crypt() calls in crypto/block.c pass the same buffer
> as input and output.
>
> So instead of having S and D you should be fine with just one of them.
Yes, I could do that in the 2nd loop.
>
> - I think this is just a matter of style preference, but in the first
> for loop you can remove the comma operator (i++, S++, D++) and use
> S[i] and D[I] instead in the line after that. I'm fine if you prefer
> the current style, though.
The syntax I used results in slightly more efficient asm code.
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 :|
