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.web08.11091.1639664733685288445 for ; Thu, 16 Dec 2021 06:25:33 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=JDwJr7J1; 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=1639664732; 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=TQ0IONLLdErXiQZdumke22eAVqK25EMizH4HDefvBGg=; b=JDwJr7J1Beh3JSO4xkh3Lw3RgTqG2fhtUl3YlGe9Vlj68ybeQ9VRDxdrhUaQO0vwrRoBaM 8NVMq9X0bHFXx8RPOUm0lTFsf4Ra81QEzqmb2tXXEC45BEQHOWcO1wkOV4qGTtPBq67N+6 yPX0XaQBn3dOcEeaoJUwht7VMGHd2Ng= 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-433-1wITjAt8NkSTzzl1apce9Q-1; Thu, 16 Dec 2021 09:25:29 -0500 X-MC-Unique: 1wITjAt8NkSTzzl1apce9Q-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C4A9E10144F0; Thu, 16 Dec 2021 14:25:27 +0000 (UTC) Received: from sirius.home.kraxel.org (unknown [10.39.192.14]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 69AAE7A228; Thu, 16 Dec 2021 14:25:27 +0000 (UTC) Received: by sirius.home.kraxel.org (Postfix, from userid 1000) id B1CCA18003A3; Thu, 16 Dec 2021 15:25:25 +0100 (CET) Date: Thu, 16 Dec 2021 15:25:25 +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: <20211216142525.pkaxszwaevlpg4ap@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> MIME-Version: 1.0 In-Reply-To: X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 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 Hi, > > Oh, wow. So you compile in PEI, then decide at runtime whenever you use it > > or not? > Yes. > In OvmfPkgX64.dsc above code will not be built into the image. So it follows the SEC->PEI->DXE flow. > In IntelTdxX64.dsc, it if is Tdx guest, it jumps from SEC to DXE (see TdxStartup ()). Otherwise, it follows the SEC->PEI->DXE flow (Legacy guest, SEV guest, etc). > > No. Please don't. That's just silly. If you don't want use PEI, ok, fine, but > > please go the way then, remove PEI from the build and take the PEI-less code > > path in all cases. > In the first version TDVF, we do remove the PEI from the image. The > image only contains the SEC and DXE, and only the components TDVF > needs. It's a slim image. Then the *ONE BINARY* requirement is > proposed. It requires to bring up Legacy guest and Tdx guest with the > same image. So PEI must be included in the build, Why? Booting non-tdx guests without PEI shouldn't be fundamentally different from a TDX guest. Memory detection needs fw_cfg instead of the td_hob, and you have to skip some tdx setup steps, but that should be it. Code for all that exists in PlatformPei, it only needs to be moved to a place where SEC can use it too. Yes, you can't include a number of features which depend on PEI into the build then. But config-b wants be a stripped down build anyway, right? One major advantage of having a single binary is that most aspects of the SEC->DXE boot workflow can also be tested without TDX. Easier for developers. Easier for CI coverage. Especially now where we talk about pre-production hardware support. When builing a frankenstein image which uses SEC->DXE with TDX and SEC->PEI->DXE without TDX you loose that advantage, because that is effectively a two-in-one binary. > and it probes Tdx > guest in run-time so that it decides to go to the legacy flow > (SEC->PEI->DXE) or Tdx flow (SEC->DXE). Ok, so the state with wave-2 merged will be: * We have the ovmf build, which supports native/sev/tdx guests, with basic tdx support (aka config-a). * We have the amdsev variant (supports native/sev/not-sure-about-tdx), which is largely identical to the normal build, only unwanted drivers removed (no network etc), grub boot loader added and its own PlatformBootManagerLib to have a more strict boot policy (all dxe phase changes). So, where to go from here? I still think the best way forward would be to model the inteltdx build (aka config-b) similar to the amdsev variant. Just disable the stuff you don't need, add support for the advanced tdx features (measurement etc), but otherwise continue to use the same SEC->PEI->DXE boot workflow. Advantages: * It should be relatively easy to unify amdsev + inteltdx into one binary. * No quirks needed due to SEC/PEI differences. SEC can't set PCDs, leading to patches like #9 of this series (and there was another similar one ...). The other route (as preferred by Jiewen) would be to not use PEI in inteltdx. Requires some reorganization of the qemu platform initialization code (probably move to lib) so we can run the same code (without using cut+paste programming) in both sec and pei phase. Clearly not my preference, but should work too. A better solution for the PCD issue (and possibly other simliar issues poping up later) would be good. Can't we handle that early in PlatformDxe? So we have one single place for those quirks, and the dxe drivers don't need to know about the SEC->DXE and SEC->PEI->DXE differences? take care, Gerd