From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f45.google.com (mail-wr1-f45.google.com [209.85.221.45]) by mx.groups.io with SMTP id smtpd.web11.32492.1585209008463449925 for ; Thu, 26 Mar 2020 00:50:09 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=z4/fMOPU; spf=pass (domain: linaro.org, ip: 209.85.221.45, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wr1-f45.google.com with SMTP id 65so6541596wrl.1 for ; Thu, 26 Mar 2020 00:50:08 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=geMPP3XVyIOacPDIrHBf82jP6bR2GIQtyVEln27TI1c=; b=z4/fMOPU2i2OB4+Ic089jHBD5QGroUgJv446NHjkdslRBTNmJz1ErtV1gHUIqdXmKY c17V7F81MsJyeyu2XsLfvCFC0dCkL8DKKde+trllZHq4oizOOylw56hTdIzjqZpv0gDm E7ar5iU5Cu0g2C2iomYKN5XiI3Qw8RXjPttt9NRzZA6VL1MotlJK/jfg5yf/z6KRxvev 7rXGfRNWgSeTVzxD+4DJEsqvkKVf3+VlO1ugrCQ+feIn44pk5FaB+PwuXPw+XFVm5BVu bCHOJwtGiAOdZvHYqyho1ZM0wKv3O1+a3WTBNELepNvyz7Ao3R/nFEmAF6eLv7581hlq WppQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=geMPP3XVyIOacPDIrHBf82jP6bR2GIQtyVEln27TI1c=; b=cK7jQ5XeokbvfHthZdP1Er00RQ7QjI9NEnzkyTP79MF6CWQNGq4OCPnGhLbKARYKMK 6PeKNYDq7l9aODSDJWtNCcCnWE8KkWx0aCIxpjsgYv0k7s+PUcnTDglHddFc8Gu/uxbj PHRuGGE3DlppoVxWrU22+Gd+VmNo4y/RwYTqI/flfdYBXOtEmFQVRxFh1ghD5BXM/v8h PdY/XAhZ573SPAbp80h4aUal/OUwx0w/U+1mR5LDXHFNQgU3m1yBTa31VrH+ui+HSdFO Twkv1gO+R71lPBcvbbzt9Ui2YAXo+O+FcmjEXBJmJTG5dBPSZUn9qARRWZFbAsdkshCc j4mg== X-Gm-Message-State: ANhLgQ2ghaKMuZZHKN5IrikuGEVU6qCXHW5vU87CAqhViPU4/Fr9niFz PCI7sCYHJSzPGmRDkkmG602o4+iiZCZ+vI5Jwoekbw== X-Google-Smtp-Source: ADFU+vsEW3neiDGYhYId2fBg0daoScOF/Olh+r5a6n4yw/gmNMTmCdFcUH9PxppTaJciOrIA1G154Z3QTKtjIv345qw= X-Received: by 2002:a5d:4fcf:: with SMTP id h15mr7736211wrw.262.1585209006987; Thu, 26 Mar 2020 00:50:06 -0700 (PDT) MIME-Version: 1.0 References: <20200317102656.20032-1-christopher.j.zurcher@intel.com> In-Reply-To: From: "Ard Biesheuvel" Date: Thu, 26 Mar 2020 08:49:56 +0100 Message-ID: Subject: Re: [edk2-devel] [PATCH 0/1] CryptoPkg/OpensslLib: Add native instruction support for IA32 and X64 To: "Zurcher, Christopher J" Cc: "devel@edk2.groups.io" , "Wang, Jian J" , "Lu, XiaoyuX" , "david.harris4@hp.com" Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Thu, 26 Mar 2020 at 02:04, Zurcher, Christopher J wrote: > > > -----Original Message----- > > From: devel@edk2.groups.io On Behalf Of Ard Bies= heuvel > > Sent: Wednesday, March 25, 2020 11:40 > > To: Zurcher, Christopher J > > Cc: edk2-devel-groups-io ; Wang, Jian J > > ; Lu, XiaoyuX ; Eugene Coh= en > > > > Subject: Re: [edk2-devel] [PATCH 0/1] CryptoPkg/OpensslLib: Add native > > instruction support for IA32 and X64 > > > > On Tue, 17 Mar 2020 at 11:26, Christopher J Zurcher > > wrote: > > > > > > BZ: https://bugzilla.tianocore.org/show_bug.cgi?id=3D2507 > > > > > > This patch adds support for building the native instruction algorithm= s for > > > IA32 and X64 versions of OpensslLib. The process_files.pl script was > > modified > > > to parse the .asm file targets from the OpenSSL build config data str= uct, > > and > > > generate the necessary assembly files for the EDK2 build environment. > > > > > > For the X64 variant, OpenSSL includes calls to a Windows error handli= ng > > API, > > > and that function has been stubbed out in ApiHooks.c. > > > > > > For all variants, a constructor was added to call the required CPUID > > function > > > within OpenSSL to facilitate processor capability checks in the nativ= e > > > algorithms. > > > > > > Additional native architecture variants should be simple to add by > > following > > > the changes made for these two architectures. > > > > > > The OpenSSL assembly files are traditionally generated at build time = using > > a > > > perl script. To avoid that burden on EDK2 users, these end-result ass= embly > > > files are generated during the configuration steps performed by the p= ackage > > > maintainer (through process_files.pl). The perl generator scripts ins= ide > > > OpenSSL do not parse file comments as they are only meant to create > > > intermediate build files, so process_files.pl contains additional hoo= ks to > > > preserve the copyright headers as well as clean up tabs and line endi= ngs to > > > comply with EDK2 coding standards. The resulting file headers align w= ith > > > the generated .h files which are already included in the EDK2 reposit= ory. > > > > > > Cc: Jian J Wang > > > Cc: Xiaoyu Lu > > > Cc: Eugene Cohen > > > Cc: Ard Biesheuvel > > > > > > Christopher J Zurcher (1): > > > CryptoPkg/OpensslLib: Add native instruction support for IA32 and X= 64 > > > > > > > Hello Christopher, > > > > It would be helpful to understand the purpose of all this. Also, I > > think we could be more picky about which algorithms we enable - DES > > and MD5 don't seem highly useful, and even if they were, what do we > > gain by using a faster (or smaller?) implementation? > > > > The selection of algorithms comes from the default OpenSSL assembly targe= ts; this combination is validated and widely used, and I don't know all the= consequences of picking and choosing which ones to include. If necessary I= could look into reducing the list. > > The primary driver for this change is enabling the Full Flash Update (FFU= ) OS provisioning flow for Windows as described here: > https://docs.microsoft.com/en-us/windows-hardware/manufacture/desktop/wim= -vs-ffu-image-file-formats > This item under "Reliability" is what we are speeding up: "Includes a cat= alog and hash table to validate a signature upfront before flashing onto a = device. The hash table is generated during capture, and validated when appl= ying the image." > This provisioning flow can be performed within the UEFI environment, and = the native algorithms allow significant time savings in a factory setting (= minutes per device). > > We also had a BZ which requested these changes and the specific need was = not provided. Maybe David @HP can provide further insight? > https://bugzilla.tianocore.org/show_bug.cgi?id=3D2507 > > There have been additional platform-specific benefits identified, for exa= mple speeding up HMAC authentication of communication with other HW/FW comp= onents. > OK, so in summary, there is one particular hash that you need to speed up, and this is why you enable every single asm implementation in OpenSSL for X86. I'm not sure I am convinced that this is justified. OpenSSL can easily deal with having just a couple of these accelerated implementations enabled. To me, it seems like a more sensible approach to only enable algorithms based on special instructions (such as AES-NI and SHA-NI), which typically give a speedup in the 10x-20x range, and to leave the other ones disabled. E.g., accelerated montgomery multiplication for RSA is really not worth it, since the speedup is around 50% and RSA is hardly a bottleneck in UEFI. Same applies to deprecated ciphers such as DES or MD5 - they are not based on special instructions, and shouldn't be used in the first place.