From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id DC7C2C433F5 for ; Sun, 1 May 2022 21:32:45 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=s28Bj7ExDwLp/GQy8Pym3rs8KxPHObezqYtzynI4at4=; b=MFne5cCroCHqY7 9Bzfn0egsS0YN3OBs7z2MT9nkPCqnWBV4o982TGR8GlsBZA2/EdMjMy3007uhVlJp/eJd0RUJUWlr jomOg7eb3u3fffbj+hOJOn/RKt3GpuNGunN8KBv3IHWQvbZ0Fm5FlBYtr8MjovJpu8+1IVWcztO9n S60eBHGeAnHexnnnv/lc2U7xRnlidn/NIEQLzc0qfFlSxTHuUg+Hw1/TpbVlYTWj7b9pS0FR+N0cS vx4qRy5eSAwJpUrURDS88VoQjNNQKV15o7SXVZB215dDpxOvcmK8AQrvDH3/m1qziYEGG86SaLZrM A2aMZxDuo0qc86+jfayQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nlHAM-00GvCd-AA; Sun, 01 May 2022 21:31:42 +0000 Received: from dfw.source.kernel.org ([139.178.84.217]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nlHAJ-00GvC7-Iy for linux-arm-kernel@lists.infradead.org; Sun, 01 May 2022 21:31:41 +0000 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id 9F6E161026; Sun, 1 May 2022 21:31:38 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id BA8DAC385A9; Sun, 1 May 2022 21:31:37 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1651440698; bh=WeRRG4BLMn2nheT0tIBFSxNPYhUfchErXx3IZQQ2mlc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=sb67Us74Gi8vk5fVPHEycB1rGg2Sgh5Rkx2TmCP9jhv5Nx+rWiYlSehdtst2MJOaY KfD1Y2T8c6UYSzx+THZ7sxldLko0wmKyNGFXdcafJ8UvrgNZkkdmFDXdLUOp9VhJQy bsKNl4aKKwl/KkwD5kT8NJYkhfMVLRS+wDnnEUf3OVox8XSYbKwHz0IdYX3nvo0wOt zyzFV/VxFCOrp5Lb3nue6Ftu5+8Js/QfFM72HcsobW60s71/k3kgsIySTkYodV4pXR Qa+59HRiQLea0di18r4DKVLgqDdFy9lKrStoWjGxBUSCFrdbQoYQ9bxyCCyZseUy22 Ee3XERXvE4k9w== Date: Sun, 1 May 2022 14:31:32 -0700 From: Eric Biggers To: Nathan Huckleberry Cc: linux-crypto@vger.kernel.org, linux-fscrypt.vger.kernel.org@google.com, Herbert Xu , "David S. Miller" , linux-arm-kernel@lists.infradead.org, Paul Crowley , Sami Tolvanen , Ard Biesheuvel Subject: Re: [PATCH v5 4/8] crypto: x86/aesni-xctr: Add accelerated implementation of XCTR Message-ID: References: <20220427003759.1115361-1-nhuck@google.com> <20220427003759.1115361-5-nhuck@google.com> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20220427003759.1115361-5-nhuck@google.com> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220501_143139_840454_912553DE X-CRM114-Status: GOOD ( 22.35 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Wed, Apr 27, 2022 at 12:37:55AM +0000, Nathan Huckleberry wrote: > Add hardware accelerated versions of XCTR for x86-64 CPUs with AESNI > support. These implementations are modified versions of the CTR > implementations found in aesni-intel_asm.S and aes_ctrby8_avx-x86_64.S. Just one implementation now, using aes_ctrby8_avx-x86_64.S. > +/* Note: the "x" prefix in these aliases means "this is an xmm register". The > + * alias prefixes have no relation to XCTR where the "X" prefix means "XOR > + * counter". > + */ Block comments look like: /* * text */ > + .if !\xctr > + vpshufb xbyteswap, xcounter, xdata0 > + .set i, 1 > + .rept (by - 1) > + club XDATA, i > + vpaddq (ddq_add_1 + 16 * (i - 1))(%rip), xcounter, var_xdata > + vptest ddq_low_msk(%rip), var_xdata > + jnz 1f > + vpaddq ddq_high_add_1(%rip), var_xdata, var_xdata > + vpaddq ddq_high_add_1(%rip), xcounter, xcounter > + 1: > + vpshufb xbyteswap, var_xdata, var_xdata > + .set i, (i +1) > + .endr > + .else > + movq counter, xtmp > + .set i, 0 > + .rept (by) > + club XDATA, i > + vpaddq (ddq_add_1 + 16 * i)(%rip), xtmp, var_xdata > + .set i, (i +1) > + .endr > + .set i, 0 > + .rept (by) > + club XDATA, i > + vpxor xiv, var_xdata, var_xdata > + .set i, (i +1) > + .endr > + .endif I'm not a fan of 'if !condition ... else ...', as the else clause is double-negated. It's more straightforward to do 'if condition ... else ...'. > + .if !\xctr > + vmovdqa byteswap_const(%rip), xbyteswap > + vmovdqu (p_iv), xcounter > + vpshufb xbyteswap, xcounter, xcounter > + .else > + andq $(~0xf), num_bytes > + shr $4, counter > + vmovdqu (p_iv), xiv > + .endif Isn't the 'andq $(~0xf), num_bytes' instruction unnecessary? If it is necessary, I'd expect it to be necessary for CTR too. Otherwise this file looks good. Note, the macros in this file all expand to way too much code, especially due to the separate cases for AES-128, AES-192, and AES-256, and for each one every partial stride length 1..7. Of course, this is true for the existing CTR code too, so I don't think you have to fix this... But maybe think about addressing this later. Changing the handling of partial strides might be the easiest way to save a lot of code without hurting any micro-benchmarks too much. Also maybe some or all of the AES key sizes could be combined. > +#ifdef CONFIG_X86_64 > +/* > + * XCTR does not have a non-AVX implementation, so it must be enabled > + * conditionally. > + */ > +static struct skcipher_alg aesni_xctr = { > + .base = { > + .cra_name = "__xctr(aes)", > + .cra_driver_name = "__xctr-aes-aesni", > + .cra_priority = 400, > + .cra_flags = CRYPTO_ALG_INTERNAL, > + .cra_blocksize = 1, > + .cra_ctxsize = CRYPTO_AES_CTX_SIZE, > + .cra_module = THIS_MODULE, > + }, > + .min_keysize = AES_MIN_KEY_SIZE, > + .max_keysize = AES_MAX_KEY_SIZE, > + .ivsize = AES_BLOCK_SIZE, > + .chunksize = AES_BLOCK_SIZE, > + .setkey = aesni_skcipher_setkey, > + .encrypt = xctr_crypt, > + .decrypt = xctr_crypt, > +}; > + > +static struct simd_skcipher_alg *aesni_simd_xctr; > +#endif Comment the #endif above: #endif /* CONFIG_X86_64 */ > @@ -1180,8 +1274,19 @@ static int __init aesni_init(void) > if (err) > goto unregister_skciphers; > > +#ifdef CONFIG_X86_64 > + if (boot_cpu_has(X86_FEATURE_AVX)) > + err = simd_register_skciphers_compat(&aesni_xctr, 1, > + &aesni_simd_xctr); > + if (err) > + goto unregister_aeads; > +#endif > + > return 0; > > +unregister_aeads: > + simd_unregister_aeads(aesni_aeads, ARRAY_SIZE(aesni_aeads), > + aesni_simd_aeads); This will cause a compiler warning in 32-bit builds because the 'unregister_aeads' label won't be used. - Eric _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel