From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mx.groups.io with SMTP id smtpd.web10.28981.1641801346709859631 for ; Sun, 09 Jan 2022 23:55:47 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=FGFRhjau; spf=pass (domain: redhat.com, ip: 170.10.129.124, mailfrom: kraxel@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1641801345; 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: in-reply-to:in-reply-to:references:references; bh=ZKsYHykb9CDgFdDuaIX6WGJWvF0Nnz94j09Z5lwW4TU=; b=FGFRhjaupUL9xRLdCkizRtvIMQcXZdsjddlSblCioQBpax4sWRkV6vubmIRacMWPKJmqVA qN3eKH8XLZiuuEONx9kfxrgudDvf7QVSBiSXzcLy51fogEq6CtfL+m7tdVLgu4e0KPqVg+ C0nCzksTq1dsx8ufF54mN+EIa54HazQ= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-147-qsEwMETePIa1FbkF2QN3bA-1; Mon, 10 Jan 2022 02:55:43 -0500 X-MC-Unique: qsEwMETePIa1FbkF2QN3bA-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 731A11083F62; Mon, 10 Jan 2022 07:55:39 +0000 (UTC) Received: from sirius.home.kraxel.org (unknown [10.39.193.24]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 1DBD5104B4C6; Mon, 10 Jan 2022 07:55:39 +0000 (UTC) Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id 6883B18003A0; Mon, 10 Jan 2022 08:55:37 +0100 (CET) Date: Mon, 10 Jan 2022 08:55:37 +0100 From: "Gerd Hoffmann" To: "Xu, Min M" Cc: "devel@edk2.groups.io" , "Kinney, Michael D" , Brijesh Singh , "Aktas, Erdem" , James Bottomley , "Yao, Jiewen" , Tom Lendacky Subject: Re: [edk2-devel] [PATCH 08/10] OvmfPkg: Update Sec to support Tdvf Config-B Message-ID: <20220110075537.2dxghysjlz5rmwhm@sirius.home.kraxel.org> References: <20211214134126.869-1-min.m.xu@intel.com> <20211214134126.869-9-min.m.xu@intel.com> <20211215102753.m4bp56bdxzgmdzkr@sirius.home.kraxel.org> <20211216142525.pkaxszwaevlpg4ap@sirius.home.kraxel.org> <20211220121145.aiqcqs6vd2hb2sb4@sirius.home.kraxel.org> <20220103080218.ap7tktgh4fuvw6sf@sirius.home.kraxel.org> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=kraxel@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Fri, Jan 07, 2022 at 06:13:37AM +0000, Xu, Min M wrote: > On January 3, 2022 4:02 PM, Gerd Hoffmann wrote: > > > > > PCDs cannot be set in SEC phase, so the values should be saved in a > > > Hob (for example, PLATFORM_INFO_HOB). In early DXE phase these values > > > are set to the PCDs. This is how TdxDxe does today. > > > > > > Other tasks can be done in SEC phase. I think there should be a lib > > > (for example, PlatformPeiLib) to wrap these functions so that they can > > > be re-used by OvmfPkg/PlatformPei. > > > > Yes, I think we need a PlatformLib for the platform initialization code. With > > PEI we would simply link the lib into PlatformPei, without PEI we would link > > parts of the lib into SEC and parts of the lib into DXE. > After carefully study the PlatformPei code and a quick PoC > (PlatformInitLib which wraps the basic functions in PlatformPei), I > found it's not a easy task for such a lib which can be used in both > PlatformPei and Pei-less boot. > 1. PlatformInitLib should work both in SEC and PEI. So it cannot use > global variables between different functions. mHostBridgeDevId and > mPhysMemAddressWidth are the examples. So these variables must be > provided by the caller thru the input function parameters. > 2. PlatformInitLib cannot set PCDs in the code. So a Guid hob should > be created to store the PCDs and pass them to DXE phase. Then these > PCDs will be set at the very beginning of DXE phase. Yes. Your patches add a PlatformInitHob because of that. I think right now it only has some tdx-specific variables, but we can move more variables into the hob to allow platform init code run in both SEC and PEI phase. I think it makes sense to have the hob in both PEI and PEI-less mode to minimize the code differences. > 4. In PlatformPei there are many if-else to check if it is > SMM/S3/Microvm/Cloud-Hypervisor/SEV/TDX. There are also Bhyve and Xen > PlatformPei variants. In the current PlatformPei those if-else check > depends on the PCDs and global variables. Because of (1) it needs > input parameters for all these if-else check. Maybe a big environment > variable data structure is needed. Use PlatformInitHob? > But anyway a complete functional PlatformInitLib is a big task. My > suggestion is that in TDVF-Config-B we first propose a basic > functional PlatformInitLib. This lib can boot up Tdx guest and legacy > OVMF guest in TDVF-Config-B. OvmfPkg/PlatformPei is not refactored by > this basic PlatformInitLib this time. Well. The whole point of adding PlatformInitLib is to move over (and refactor if needed) existing code in PlatformPei so we can avoid code duplication. Now you want add PlatformInitLib without touching PlatformPei, probably by copying code. That doesn't make sense at all. > This is because PlatformPei serves > SMM/S3/Microvm/Cloud-Hypervisor/SEV/TDX. It is a big risk for such > refactor. We can revisit PlatformPei in the future. Well, if you want avoid the refactoring because of the risk there is still the option to have tdx config-b use the normal PEI boot flow. Then revisit refactoring and adding support for PEI-less boot later. take care, Gerd