From: "Laszlo Ersek" <lersek@redhat.com>
To: "Kinney, Michael D" <michael.d.kinney@intel.com>,
"devel@edk2.groups.io" <devel@edk2.groups.io>,
"spbrogan@outlook.com" <spbrogan@outlook.com>,
"Wang, Jian J" <jian.j.wang@intel.com>,
"Zurcher, Christopher J" <christopher.j.zurcher@intel.com>,
"Yao, Jiewen" <jiewen.yao@intel.com>
Cc: "Lu, XiaoyuX" <xiaoyux.lu@intel.com>,
Ard Biesheuvel <ard.biesheuvel@linaro.org>
Subject: Re: [edk2-devel] [PATCH v2 2/2] CryptoPkg/OpensslLib: Commit the auto-generated assembly files for X64
Date: Wed, 19 Aug 2020 19:21:59 +0200 [thread overview]
Message-ID: <24820bba-a45b-79fe-0643-3b865470885e@redhat.com> (raw)
In-Reply-To: <MN2PR11MB4461B99AE3D6F6F3290A9A9CD25D0@MN2PR11MB4461.namprd11.prod.outlook.com>
On 08/19/20 18:08, Kinney, Michael D wrote:
> Laszlo,
>
> The current CryptoPkg DCS file with use of the CRYPTO_SERVICES define is cumbersome.
>
> #
> # Flavor of PEI, DXE, SMM modules to build.
> # Must be one of ALL, NONE, MIN_PEI, MIN_DXE_MIN_SMM.
> # Default is ALL that is used for package build verification.
> # PACKAGE - Package verification build of all components. Null
> # versions of libraries are used to minimize build times.
> # ALL - Build PEIM, DXE, and SMM drivers. Protocols and PPIs
> # publish all services.
> # NONE - Build PEIM, DXE, and SMM drivers. Protocols and PPIs
> # publish no services. Used to verify compiler/linker
> # optimizations are working correctly.
> # MIN_PEI - Build PEIM with PPI that publishes minimum required
> # services.
> # MIN_DXE_MIN_SMM - Build DXE and SMM drivers with Protocols that publish
> # minimum required services.
> #
> DEFINE CRYPTO_SERVICES = PACKAGE
>
> There is a known limitation for using structured PCDs in a module scope and
> that limitation is what resulted in the use of this define. Bob Feng
> has provided a BaseTools patch that attempts to address this limitation.
>
> https://edk2.groups.io/g/devel/message/63906
>
> This patch is functional, but has one open issue around the PCD report. Once
> this issue is resolved we will be able to specify structured PCD field values
> in the scope of a single module. I have a branch that simplifies the DSC and
> allows all flavors of the crypto modules to be built in a single invocation
> of the build command. There is more cleanup of the DSC possible, but I
> wanted to share a quick test case for Bob's patch.
>
> https://github.com/mdkinney/edk2/tree/Bug_xxx_CryptoPkg_UseModuleScopedPcds
>
> This feature supports both the generation of standard flavors of the crypto
> modules that a platform could consume as a pre-built binary and also allows
> a platform to choose their own profile by specifying the specific crypto APIs
> needed in PEI, DXE, SMM when building crypto modules from sources.
Yes, this resolves my first problem entirely!
Thanks!
Laszlo
>
> Best regards,
>
> Mike
>
>> -----Original Message-----
>> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Laszlo Ersek
>> Sent: Wednesday, August 19, 2020 3:44 AM
>> To: devel@edk2.groups.io; spbrogan@outlook.com; Kinney, Michael D <michael.d.kinney@intel.com>; Wang, Jian J
>> <jian.j.wang@intel.com>; Zurcher, Christopher J <christopher.j.zurcher@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
>> Cc: Lu, XiaoyuX <xiaoyux.lu@intel.com>; Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Subject: Re: [edk2-devel] [PATCH v2 2/2] CryptoPkg/OpensslLib: Commit the auto-generated assembly files for X64
>>
>> Hi,
>>
>> On 08/18/20 23:33, Sean wrote:
>>> Mike,
>>>
>>> I am not technically a basetool maintainer but as an active user/dev in
>>> basetools, i would be opposed to bringing in perl as an edk2 dependency.
>>> Also introducing another language is counter to the goal of aligning on
>>> python and improving the python used within edk2. From my perspective
>>> the openssl config case isn't strong enough to counter the above goal.
>>> In fact as you know we are trying to change the paradigm for
>>> Crypto/OpenSSL with the Crypto Driver
>>> (https://github.com/tianocore/edk2/tree/master/CryptoPkg/Driver) and
>>> BaseCryptLibOnProtocolPpi
>>> (https://github.com/tianocore/edk2/tree/master/CryptoPkg/Library/BaseCryptLibOnProtocolPpi)
>>> work so that everyday development doesn't need to compile openssl in
>>> their edk2 builds.
>>
>> Here I'd only like to comment on this one aspect (= build OpenSSL as a
>> statically linked library vs. as a crypto service driver / PEIM).
>>
>> Recently I tried to evaluate the crypto driver for OVMF. I started with
>> the PEI phase. The configuration interface (the PCD) is baroque, *BUT*
>> that is a direct consequence of OpenSSL offering a huge range of
>> interfaces. So no complaints about the config interface.
>>
>> I also reviewed the CryptoPkg.dsc "pre-sets" (i.e., CRYPTO_SERVICES
>> being one of PACKAGE / ALL / NONE / MIN_PEI / MIN_DXE_MIN_SMM). I had
>> two problems:
>>
>> - These pre-sets are supremely suitable for a platform that is composed
>> of multiple build runs; that is, build the crypto PEIM, build the DXE /
>> SMM protocol drivers, package up the resultant binaries, and *then*
>> build the actual platforms (which will then include the crypto service
>> drivers in *binary* form). On the other hand, the pre-sets are not
>> useful to a platform that is supposed to be built in a single-shot.
>> Importantly, I'm not saying that the pre-sets are *detrimental* to such
>> platforms -- they aren't. It's just that the pre-sets target a different
>> use case.
>>
>> - The other problem I had was the one that we had discussed when the
>> crypto service driver was being introduced. Namely, selecting the
>> OpenSSL interfaces (interface families) that the platform actually consumes.
>>
>> Now, I carefully tracked down the modules in OVMF that needed crypto
>> support, by *not* resolving SmmCryptLib, RuntimeCryptLib, TlsLib in
>> general [LibraryClasses] sections in the OVMF DSC files. Then I re-added
>> those lib-class resolutions as module-scoped <LibraryClasses> overrides
>> to the actual modules that needed them.
>>
>> However, I didn't know how to even *begin* evaluating the specific "API
>> needs" of the modules identified thusly. On a Windows or Linux OS, when
>> you have a dynamically linked executable, and it doesn't find a symbol
>> in a shared library, you get a nice error message, and the application
>> doesn't start. On the other hand, if a crypto protocol call fails in SMM
>> because we missed a feature bit in the config PCD, the results are
>> somewhat less user-friendly.
>>
>> The expression "minimum required services" in CryptoPkg.dsc seems
>> relevant, but it didn't convince me that it would cover everything
>> needed by -- for example -- VariableSmm, VariableRuntimeDxe, and TlsDxe.
>>
>> So, given that I couldn't construct a "tight profile", I started my
>> investigation (for OVMF's PEI phase) by including the crypto service
>> PEIM with *all* interfaces enabled.
>>
>> This would be restricted to "TPM_ENABLE", because only that is when
>> OVMF's PEI phase needs crypto -- due to including the various TPM1 and
>> TPM2 PEIMs.
>>
>> So basically this check would replace the statically linked -- and
>> accordingly trimmed! -- "thick" OpenSSL library copies in the TPM1/PTM2
>> PEIMs, with the thin wrapper lib
>> (BaseCryptLibOnProtocolPpi/PeiCryptLib.inf) *plus* the full-blown crypto
>> service PEIM.
>>
>> The result was a *violent* size explosion in PEIFV; at least in the
>> NOOPT build. Before:
>>
>>> PEIFV [64%Full] 917504 total, 592456 used, 325048 free
>>
>> after:
>>
>>> the required fv image size 0x132968 exceeds the set fv image size
>>> 0xe0000
>>
>> The PEIFV footprint more than doubled, from 592,456 bytes to 1,255,784
>> bytes.
>>
>> I gave up there. Until the "crypto profile" construction is not
>> automated for platforms, somehow, I don't know how I could maintain OVMF
>> consuming the crypto service PEIMs/drivers.
>>
>> (I wonder if we should maintain a "required crypto services" bitmap for
>> each individual PEIM / DXE / SMM driver inside edk2. And then, when a
>> platform includes any one such PEIM or driver, they'd know to "OR" the
>> bitmap for that particular module into their platform PCD setting.)
>>
>>> So I support leaving it as is which means if you have to change
>>> something in openssl config you deal with it and a special one off.
>>
>> (OK, I guess I can comment on this too, after all.)
>>
>> I agree.
>>
>> While perl is readily available on Linux build hosts, I remember:
>> - accommodating the python3 BaseTools requirements on my RHEL7 laptop,
>> - (almost) bumping the NASM version so we could compile the VMGEXIT
>> instruction for IA32,
>> - the python virtual environment discussions for running CI locally.
>>
>> So I agree that new build dependencies should be avoided as much as
>> possible.
>>
>> Thanks
>> Laszlo
>>
>>
>>
>
next prev parent reply other threads:[~2020-08-19 17:22 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-08-04 0:24 [PATCH v2 0/2] CryptoPkg/OpensslLib: Add native instruction support for X64 Zurcher, Christopher J
2020-08-04 0:24 ` [PATCH v2 1/2] " Zurcher, Christopher J
2020-08-11 11:46 ` [edk2-devel] " Guomin Jiang
2020-08-13 15:03 ` Yao, Jiewen
2020-08-18 22:49 ` Zurcher, Christopher J
[not found] ` <162C7E6ED8CEF542.12673@groups.io>
2020-08-24 21:25 ` [edk2-devel] " Zurcher, Christopher J
2020-08-24 23:35 ` Yao, Jiewen
2020-09-16 9:17 ` Guomin Jiang
2020-09-22 15:21 ` Zurcher, Christopher J
2020-09-23 2:35 ` Yao, Jiewen
2020-09-25 0:28 ` Zurcher, Christopher J
2020-09-25 0:49 ` 回复: " gaoliming
2020-09-25 1:06 ` Zurcher, Christopher J
2020-09-25 1:11 ` Yao, Jiewen
2020-09-25 1:14 ` Zurcher, Christopher J
[not found] ` <1637E1D4851CF309.11037@groups.io>
2020-09-25 2:28 ` Zurcher, Christopher J
[not found] ` <1637E5DD452A46F1.2312@groups.io>
2020-09-25 8:01 ` Zurcher, Christopher J
2020-09-29 21:08 ` Zurcher, Christopher J
2020-10-01 12:58 ` Laszlo Ersek
2020-10-08 19:56 ` Zurcher, Christopher J
2020-10-09 11:37 ` Laszlo Ersek
2020-10-09 19:27 ` Zurcher, Christopher J
2020-10-15 7:14 ` Laszlo Ersek
2020-08-04 0:24 ` [PATCH v2 2/2] CryptoPkg/OpensslLib: Commit the auto-generated assembly files " Zurcher, Christopher J
2020-08-13 15:24 ` Yao, Jiewen
2020-08-13 15:37 ` Michael D Kinney
2020-08-13 15:45 ` Yao, Jiewen
2020-08-14 19:34 ` Zurcher, Christopher J
2020-08-18 2:36 ` Wang, Jian J
2020-08-18 16:15 ` Michael D Kinney
2020-08-18 21:33 ` [edk2-devel] " Sean
2020-08-18 23:29 ` Andrew Fish
2020-08-19 16:33 ` Liming Gao
2020-08-19 10:43 ` Laszlo Ersek
2020-08-19 16:08 ` Michael D Kinney
2020-08-19 17:21 ` Laszlo Ersek [this message]
2020-08-18 16:15 ` Michael D Kinney
2020-08-18 21:22 ` Zurcher, Christopher J
2020-08-19 15:37 ` [edk2-devel] " Andrew Fish
2020-08-19 18:06 ` Laszlo Ersek
2020-08-25 0:55 ` Andrew Fish
2020-08-26 10:05 ` Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=24820bba-a45b-79fe-0643-3b865470885e@redhat.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox