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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id 1982AC433FE for ; Tue, 22 Feb 2022 20:06:47 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S235401AbiBVUHL (ORCPT ); Tue, 22 Feb 2022 15:07:11 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:55420 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S235396AbiBVUHK (ORCPT ); Tue, 22 Feb 2022 15:07:10 -0500 Received: from ams.source.kernel.org (ams.source.kernel.org [145.40.68.75]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 8CA58A41BD; Tue, 22 Feb 2022 12:06:44 -0800 (PST) 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 ams.source.kernel.org (Postfix) with ESMTPS id 0499DB81C5F; Tue, 22 Feb 2022 20:06:43 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 56812C340E8; Tue, 22 Feb 2022 20:06:41 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1645560401; bh=uI0Fg70JQsRjF8A280UxeWyrTx+BSIhEvWBXFTuCqvc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=oV94Lm4dBzwnapWO4YqkF1+FiDKUOgNPNsAaPRtqEp7aUERF9fx5An9GvUGCIbjdO ml75/8hKPJIpMkKR/BBixoM9MlWszX9BQJEKLvkbdOzd5Y/MFnIaVoZs8X+7cHzmIR KVog4RCIGsRxnnU5t+0SXRfTXag3ZmA4C7G2mVIhYTQtQpmD8Fi2lnPAPAjGORG1/H NXOsNZ6yLBw/2p5WTdH9a/+NLBV7UFhzavqHlsVPAk3ftgHAe79bNPhSvhWaqizg+P i+q/0tgKBideH8K1N+4kBi0MLYEDFCy0cUuJC5JX6ZbkLDR4FOp4iDlw0bKrCJ4VBZ EIzYowpTp8JVA== Date: Tue, 22 Feb 2022 12:06:39 -0800 From: Eric Biggers To: Keith Busch Cc: David Laight , "linux-nvme@lists.infradead.org" , "linux-block@vger.kernel.org" , "linux-crypto@vger.kernel.org" , "x86@kernel.org" , "linux-kernel@vger.kernel.org" , "axboe@kernel.dk" , "hch@lst.de" , "martin.petersen@oracle.com" , "colyli@suse.de" Subject: Re: [PATCHv3 10/10] x86/crypto: add pclmul acceleration for crc64 Message-ID: References: <20220222163144.1782447-1-kbusch@kernel.org> <20220222163144.1782447-11-kbusch@kernel.org> <20220222171405.GA1782521@dhcp-10-100-145-180.wdc.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220222171405.GA1782521@dhcp-10-100-145-180.wdc.com> Precedence: bulk List-ID: X-Mailing-List: linux-crypto@vger.kernel.org On Tue, Feb 22, 2022 at 09:14:05AM -0800, Keith Busch wrote: > On Tue, Feb 22, 2022 at 05:02:16PM +0000, David Laight wrote: > > From: Keith Busch > > > Sent: 22 February 2022 16:32 > > > > > > The crc64 table lookup method is inefficient, using a significant number > > > of CPU cycles in the block stack per IO. If available on x86, use a > > > PCLMULQDQ implementation to accelerate the calculation. > > > > > > The assembly from this patch was mostly generated by gcc from a C > > > program using library functions provided by x86 intrinsics, and measures > > > ~20x faster than the table lookup. > > > > I think I'd like to see the C code and compiler options used to > > generate the assembler as comments in the committed source file. > > Either that or reasonable comments in the assembler. > > The C code, compiled as "gcc -O3 -msse4 -mpclmul -S", was adapted from > this found on the internet: > > https://github.com/rawrunprotected/crc/blob/master/crc64.c > > I just ported it to linux, changed the poly parameters and removed the > unnecessary stuff. > > I'm okay with dropping this patch from the series for now since I don't > think I'm qualified to write it. :) I just needed something to test the > crytpo module registration. Is the license of that code compatible with the kernel's license? In any case, adding uncommented generated assembly isn't acceptable. The most common convention for Linux kernel crypto is to use hand-written assembly that is properly commented. There is some precedent for using compiler intrinsics instead, e.g. crypto/aegis128-neon-inner.c. (I'm not sure why they aren't used more often.) There are also some files where a Perl script generates the assembly code. (This is a bit ugly IMO, but it's what the author of much of OpenSSL's crypto assembly code does, and it was desired to reuse that code.) Anyway, those are the available options. Checking in some uncommented generated assembly isn't one of them. - Eric