Hi Niels,
Here is another revised patch testing with NETTLE_FAT_OVERRIDE. Same
performance as last version. I also added a new test vector with 917 bytes for
AES/GCM tests to test multiple blocks and partial. Attached are the patch and
benchmark for AES.
Thanks.
-Danny
On Dec 12, 2023, at 9:01 AM, Danny Tsen wrote:
On Dec 11, 2023, at 10:32 AM, Niels Möller wrote:
Danny Tsen writes:
Here is the version 2 for AES/GCM stitched patch. The stitched code is
in all assembly and m4 macros are used. The overall performance
improved around ~110% and 120% for encrypt and decrypt respectably.
Please see the attached patch and aes benchmark.
Thanks, comments below.
--- a/fat-ppc.c
+++ b/fat-ppc.c
@@ -226,6 +231,8 @@ fat_init (void)
_nettle_ghash_update_arm64() */
_nettle_ghash_set_key_vec = _nettle_ghash_set_key_ppc64;
_nettle_ghash_update_vec = _nettle_ghash_update_ppc64;
+ _nettle_ppc_gcm_aes_encrypt_vec = _nettle_ppc_gcm_aes_encrypt_ppc64;
+ _nettle_ppc_gcm_aes_decrypt_vec = _nettle_ppc_gcm_aes_decrypt_ppc64;
}
else
{
Fat setup is a bit tricky, here it looks like
_nettle_ppc_gcm_aes_decrypt_vec is left undefined by the else clause. I
would suspect that breaks when the extensions aren't available. You can
test that with NETTLE_FAT_OVERRIDE=none.
Sure. I’ll test and see to fix it.
gcm_aes128_encrypt(struct gcm_aes128_ctx *ctx,
size_t length, uint8_t *dst, const uint8_t *src)
{
+#if defined(HAVE_NATIVE_AES_GCM_STITCH)
+ if (length >= 128) {
+PPC_GCM_CRYPT(1, _AES128_ROUNDS, ctx, length, dst, src);
+if (length == 0) {
+ return;
+}
+ }
+#endif /* HAVE_NATIVE_AES_GCM_STITCH */
+
GCM_ENCRYPT(ctx, aes128_encrypt, length, dst, src);
}
In a non-fat build, it's right with a compile-time #if to select if the
optimized code should be called. But in a fat build, we'de need a valid
function in all cases, but doing different things depending on the
runtime fat initialization. One could do that with two versions of
gcm_aes128_encrypt (which is likely preferable if we do something
similar for other archs that has separate assembly for aes128, aes192,
etc). Or we would need to call some function unconditionally, which
would be a nop if the extensions are not available. E.g, do something
like
#if HAVE_NATIVE_fat_aes_gcm_encrypt
void
gcm_aes128_encrypt(struct gcm_aes128_ctx *ctx,
size_t length, uint8_t *dst, const uint8_t *src)
{
size_t done = _gcm_aes_encrypt (>key, >gcm.x, >gcm.ctr,
_AES128_ROUNDS, >cipher.keys, length, dst, src);
ctx->data_size += done;
length -= done;
if (length > 0)
{
src += done;
dst += done;
GCM_ENCRYPT(ctx, aes128_encrypt, length, dst, src);
}
}
#endif
where the C-implementation of _gcm_aes_encrypt could just return 0.
And it's preferable that te same interface could be used on other archs,
even if they don't do 8 blocks at a time like your ppc code.
--- a/gcm.h
+++ b/gcm.h
@@ -195,6 +195,47 @@ gcm_digest(struct gcm_ctx *ctx, const struct gcm_key *key,
(nettle_cipher_func *) (encrypt), \
(length), (digest)))
+#if defined(HAVE_NATIVE_AES_GCM_STITCH)
+#define _ppc_gcm_aes_encrypt _nettle_ppc_gcm_aes_encrypt
+#define _ppc_gcm_aes_decrypt _nettle_ppc_gcm_aes_decrypt
+void
+_ppc_gcm_aes_encrypt (void *ctx, size_t rounds, uint8_t *ctr,
+ size_t len, uint8_t *dst, const uint8_t *src);
+void
+_ppc_gcm_aes_decrypt (void *ctx, size_t rounds, uint8_t *ctr,
+ size_t len, uint8_t *dst, const uint8_t *src);
+struct ppc_gcm_aes_context {
+ uint8_t *x;
+ uint8_t *htable;
+ struct aes128_ctx *rkeys;
+};
+#define GET_PPC_CTX(gcm_aes_ctx, ctx, key, cipher) \
+ { \
+(gcm_aes_ctx)->x = (uint8_t *) &(ctx)->x; \
+(gcm_aes_ctx)->htable = (uint8_t *) (key); \
+(gcm_aes_ctx)->rkeys = (struct aes128_ctx *) (cipher)->keys; \
+ }
+
+#define PPC_GCM_CRYPT(encrypt, rounds, ctx, length, dst, src) \
+ { \
+size_t rem_len = 0; \
+struct ppc_gcm_aes_context c; \
+struct gcm_ctx *gctx = &(ctx)->gcm; \
+GET_PPC_CTX(, gctx, &(ctx)->key, &(ctx)->cipher); \
+if ((encrypt)) { \
+ _ppc_gcm_aes_encrypt(, (rounds), (&(ctx)->gcm)->ctr.b, (length),
(dst), (src)); \
+} else { \
+ _ppc_gcm_aes_decrypt(, (rounds), (&(ctx)->gcm)->ctr.b, (length),
(dst), (src)); \
+} \
+rem_len = (length) % (GCM_BLOCK_SIZE * 8); \
+(length) -= rem_len; \
+gctx->data_size += (length); \
+(dst) += (length); \
+(src) += (length); \
+(length) = rem_len; \
+ }
+#endif /* HAVE_NATIVE_AES_GCM_STITCH */
This looks a little awkward. I think it would be better to pass the
various pointers needed by the assembly implementation as separate
(register) arguments. Or pass the pointer to the struct gcm_aesxxx_ctx
directly (with the disadvantage that assembly code needs to know
corresponding offsets).
I’ll see what’s a better options.
--- a/powerpc64/machine.m4
+++ b/powerpc64/machine.m4
@@ -63,3 +63,40 @@ C INC_VR(VR, INC)
