From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=66.187.233.73; helo=mx1.redhat.com; envelope-from=lersek@redhat.com; receiver=edk2-devel@lists.01.org Received: from mx1.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 7EB0E223CDC38 for ; Mon, 5 Mar 2018 11:18:56 -0800 (PST) Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id E0B32404084A; Mon, 5 Mar 2018 19:25:08 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-64.rdu2.redhat.com [10.10.120.64]) by smtp.corp.redhat.com (Postfix) with ESMTP id 7267B213AEF8; Mon, 5 Mar 2018 19:25:07 +0000 (UTC) To: =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= Cc: edk2-devel@lists.01.org, Peter Jones , Jiewen Yao , QEMU , Javier Martinez Canillas References: <20180223132311.26555-1-marcandre.lureau@redhat.com> <20180223132311.26555-6-marcandre.lureau@redhat.com> <53a8a301-0719-aca9-7b4c-90df90ccfda0@redhat.com> From: Laszlo Ersek Message-ID: <1ea18af6-9878-edd2-cee1-6f90077f1774@redhat.com> Date: Mon, 5 Mar 2018 20:25:06 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.6.0 MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.78 on 10.11.54.6 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Mon, 05 Mar 2018 19:25:08 +0000 (UTC) X-Greylist: inspected by milter-greylist-4.5.16 (mx1.redhat.com [10.11.55.5]); Mon, 05 Mar 2018 19:25:08 +0000 (UTC) for IP:'10.11.54.6' DOMAIN:'int-mx06.intmail.prod.int.rdu2.redhat.com' HELO:'smtp.corp.redhat.com' FROM:'lersek@redhat.com' RCPT:'' Subject: Re: [PATCH 5/7] ovmf: link with Tcg2Dxe module X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 05 Mar 2018 19:18:58 -0000 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 8bit On 03/05/18 16:45, Marc-André Lureau wrote: > Hi > > On Mon, Feb 26, 2018 at 10:50 AM, Laszlo Ersek wrote: >> On 02/23/18 14:23, marcandre.lureau@redhat.com wrote: >>> + SecurityPkg/Tcg/Tcg2Dxe/Tcg2Dxe.inf { >>> + >>> + Tpm2DeviceLib|SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRouterDxe.inf >> >> Can you please explain why Tpm2DeviceLib has to be resolved differently >> for DXE_DRIVER modules in general (see above) and for "Tcg2Dxe.inf" >> specifically? >> >> Hmmm... Looks like "Tpm2DeviceLibTcg2.inf" implements the APIs via the >> TPM2 protocol. Whereas "Tcg2Dxe.inf" itself provides that protocol, so >> obviously it cannot rely on the protocol; it has to access the device, >> which is done with the help of "Tpm2DeviceLibRouterDxe.inf" and the >> "Tpm2InstanceLibDTpm.inf" that is plugged in via NULL resolution below. >> Is that about correct? >> >> If so, can you please document it in the commit message? > > I followed the SecurityPkg.dsc, and tried some variants. This router > pattern can be found in other places, unfortunately, I can't explain > it. I can copy&paste your explanation if that helps. Yes, I think we should capture it in the commit message. I'd also like to make another attempt at explaining it to you (and me as well :) ). * We have a library class called Tpm2DeviceLib -- this is basically the set of APIs declared in "SecurityPkg/Include/Library/Tpm2DeviceLib.h". Its leading comment says "This library abstract how to access TPM2 hardware device". There are two *sets* of APIs in "Tpm2DeviceLib.h": (a) functions that deal with the TPM2 device: - Tpm2RequestUseTpm(), - Tpm2SubmitCommand() This set of APIs is supposed to be used by clients that *consume* the TPM2 device abstraction. (b) the function Tpm2RegisterTpm2DeviceLib(), which is supposed to be used by *providers* of various TPM2 device abstractions. * Then, we have two implementations (instances) of the Tpm2DeviceLib class: (1) SecurityPkg/Library/Tpm2DeviceLibTcg2/Tpm2DeviceLibTcg2.inf (2) SecurityPkg/Library/Tpm2DeviceLibRouter/Tpm2DeviceLibRouterDxe.inf (1) The first library instance ("Tpm2DeviceLibTcg2.inf") implements the APIs listed under (a), and it does not implement (b) -- see EFI_UNSUPPORTED. In other words, this lib instance is strictly meant for drivers that *consume* the TPM2 device abstraction. And, the (a) group of APIs is implemented by forwarding the requests to the TCG2 protocol. The idea here is that all the drivers that consume the TPM2 abstraction do not have to be statically linked with a large TPM2 device library instance; instead they are only linked (statically) with this "thin" library instance, and all the actual work is delegated to whichever driver that provides the singleton TCG2 protocol. (2) The second library instance ("Tpm2DeviceLibRouterDxe.inf") is meant for the driver that offers (produces) the TCG2 protocol. This lib instance implements both (a) and (b) API groups. * Here's how things fit together: (i) The "SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf" library instance (which has no lib class) is linked into "Tcg2Dxe.inf" via NULL class resolution. This simply means that before the "Tcg2Dxe.inf" entry point function is entered, the constructor function of "Tpm2InstanceLibDTpm.inf" will be called. (ii) This Tpm2InstanceLibDTpmConstructor() function calls API (b), and registers its own actual TPM2 command implementation with the "Tpm2DeviceLibRouter" library instance (also linked into the Tcg2Dxe driver). This provides the back-end for the API set (a). TCG2 protocol provider (Tcg2Dxe.inf driver) launches | v NULL class: Tpm2InstanceLibDTpm instance construction | v Tpm2DeviceLib class: Tpm2DeviceLibRouter instance backend registration for API set (a) (iii) The Tcg2Dxe driver exposes the TCG2 protocol. (iv) A TPM2 consumer calls API set (a) via lib instance (1). Such calls land in Tcg2Dxe, via the protocol. (v) Tcg2Dxe serves the protocol request by forwarding it to API set (a) from lib instance (2). (vi) Those functions call the "backend" functions registered by Tpm2DeviceLibDTpm in step (ii). TPM 2 consumer driver | v Tpm2DeviceLib class: Tpm2DeviceLibTcg2 instance | v TCG2 protocol interface | v TCG2 protocol provider: Tcg2Dxe.inf driver | v Tpm2DeviceLib class: Tpm2DeviceLibRouter instance | v NULL class: Tpm2InstanceLibDTpm instance (via earlier registration) | v TPM2 chip (actual hardware) * So that is the "router" pattern in edk2. Namely, - Consumers of an abstraction use a thin library instance. - The thin library instance calls a firmware-global (singleton) service, i.e. a PPI (in the PEI phase) or protocol (in the DXE phase). - The PEIM providing the PPI, or the DXE driver providing the protocol, don't themselves implement the actual service either. Instead they offer a "registration" service too, and they only connect the incoming "consumer" calls to the earlier registered back-end(s). - The "registration service", for back-ends to use, may take various forms. It can be exposed globally to the rest of the firmware, as another member function of the PPI / protocol structure. Then backends can be provided by separate PEIMs / DXE drivers. Or else, the registration service can be exposed as just another library API. In this case, the backends are provided as NULL class library instances, and a platform DSC file links them into the PEIM / DXE driver via NULL class resolutions. The backend lib instances call the registration service in their own respective constructor functions. >> >>> + NULL|SecurityPkg/Library/Tpm2DeviceLibDTpm/Tpm2InstanceLibDTpm.inf >>> + NULL|SecurityPkg/Library/HashInstanceLibSha1/HashInstanceLibSha1.inf >> >> Again -- is SHA1 required? > > The linux kernel doesn't yet read the EFI_TCG2_EVENT_LOG_FORMAT_TCG_2, > which is required for crypto-agile log. In fact, only upcoming 4.16 > adds support EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2... > > Any major drawback in keeping sha1 enabled? (same for Pei) Likely not a major drawback. I just remember that, for a while now, it's been "time to walk, but not run, to the fire exits. You don't see smoke, but the fire alarms have gone off" -- . Note the year. :) So I prefer avoiding SHA1 in new things, for general hygiene. If we can't avoid SHA1, I'm OK with pulling it in, though. Thanks! Laszlo