From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [205.139.110.120]) by mx.groups.io with SMTP id smtpd.web12.8246.1580392449434756101 for ; Thu, 30 Jan 2020 05:54:09 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=LBPGl5iH; spf=pass (domain: redhat.com, ip: 205.139.110.120, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1580392448; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=n19JoCFxI8B/qtaupvES+3Oo8KgsMeP3uG5P+6YTt/M=; b=LBPGl5iHWdFFKqEgfEqnna3/p4qlkJBeT9o7I6p6sW7fUB+Asnd+F3jSQHwPxGynVu4xms ca/J2xOG1Li0svGKXba1SloF/8qaCG6AWsRURz/bVMMkBV8dp0vZZ2dZDhUU4QmqopOG17 cVvsGZamVTcv4tffESucng3SknbqPns= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-158-r22MDagoOSCtUGz2qWEHog-1; Thu, 30 Jan 2020 08:54:01 -0500 Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A321B13EC; Thu, 30 Jan 2020 13:53:59 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-117-35.ams2.redhat.com [10.36.117.35]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3B0C987B2D; Thu, 30 Jan 2020 13:53:58 +0000 (UTC) Subject: Re: [edk2-devel] [Patch 3/5] CryptoPkg/Driver: Add Crypto PEIM, DXE, and SMM modules To: devel@edk2.groups.io, michael.d.kinney@intel.com Cc: Jian J Wang , Xiaoyu Lu , Ard Biesheuvel References: <20200130070037.8516-1-michael.d.kinney@intel.com> <20200130070037.8516-4-michael.d.kinney@intel.com> From: "Laszlo Ersek" Message-ID: Date: Thu, 30 Jan 2020 14:53:57 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.9.1 MIME-Version: 1.0 In-Reply-To: <20200130070037.8516-4-michael.d.kinney@intel.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-MC-Unique: r22MDagoOSCtUGz2qWEHog-1 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit On 01/30/20 08:00, Michael D Kinney wrote: > https://bugzilla.tianocore.org/show_bug.cgi?id=2420 > > Based on the following package with changes to merge into > CryptoPkg. > > https://github.com/microsoft/mu_plus/tree/dev/201908/SharedCryptoPkg > > Add the CryptoPei, CryptoDxe, and CryptoSmm modules that produce > EDK II Crypto Protocols/PPIs that provide the same services as > the BaseCryptLib class. > > In order to optimize the size of CryptoPei, CryptoDxe, and > CryptoSmm modules for a specific platform, the FixedAtBuild > PCD gEfiCryptoPkgTokenSpaceGuid.PcdCryptoServiceFamilyEnable > is used to determine if a specific service is enabled or > disabled. If a service is enabled, then a call is made to > the BaseCryptLib service. If the service is disabled, then > a DEBUG() message and ASSERT() are performed and a default > return value is returned. This provides simple detection > of a service that is disabled but is used by another module > when DEBUG()/ASSERT() macros are enabled. > > The use of a FixedAtBuild PCD is required so the compiler > and linker know each services enable/disable setting at > build time and allows disabled services to be optimized away. I wonder how platforms are expected to determine their minimal PCD bitmaps. In the static linking case, the linker sees all sites that (a) call the crypto functions, (b) take the addresses of crypto functions. Therefore the linker can eliminate unused functions with certainty. If we switch to dynamic linking, we save space between independent modules, but we will not easily know what crypto functions are globally used (and, un-used) between all modules. Dynamic coverage testing seems risky. If we miss something, then in a RELEASE build, the missing function will not even crash the system, but pretend some action and return a default value (IIUC). That might allow the issue to spread out, and cause symptoms at distant locations. (This sounds really dangerous for crypto, so I think I'd prefer a hard crash, on the spot). Either way, safety requirements appear to conflict with space saving goals. I tried to find a method for statically deriving the necessary set of functions (basically, ask the linker or the object files about the *wrapper* crypto APIs that are actually called), but I came up empty. And then there's the other direction too -- we don't just want to enable a service that we initially missed (and caught with an ASSERT() in a DEBUG build), we also want to disable a service that is not used *anymore*, after e.g. removing a module from a firmware build, or after slimming down a module. How will we notice such opportunities for "trimming" the protocol? I think the static PCD is a great mechanism, but we need some automated way ("policy") too, for calculating the PCD for a given platform. This is not to say that the series should not go in as-is -- it's huge, and I can't offer a review better than this mere email, so I'm not trying to say anything like go/no-go. It's just that in OVMF I wouldn't even attempt to come up with the right bitmaps, I'd just set "give me everything" for safety -- and that might actually consume more space in the flash than statically linking just a handful (?) of functions into 5 modules. (To clarify: by "we", I don't mean "Red Hat", I mean the upstream community. I.e. the above is not a vendor-side evaluation, but a general platform perspective. OVMF is used just as an example that I'm familiar with.) With the DevicePathLib and PrintLib instances that defer to a protocol, I think the *proportions* are a bit different. I feel it's not difficult for a platform firmware to put most of the DevicePathLib / PrintLib APIs to use (i.e. the duplication due to static linking may be significant), plus the protocol implementations themselves can be "complete", because they are not huge (i.e., even if the protocols are provided at some net loss in space, the loss is not serious). Again: this is not a review either way, I'm just asking: how is a platform supposed to calculate the minimal service bitmaps for the PCD? Thanks Laszlo