From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by mx.groups.io with SMTP id smtpd.web08.12282.1605124659668440995 for ; Wed, 11 Nov 2020 11:57:39 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=VpKirebC; spf=pass (domain: redhat.com, ip: 63.128.21.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1605124658; 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=ktvZiKS8d9SvmEVqwnUEHC7ogI7JOsVEpSlnPMX7mjk=; b=VpKirebCr6FnKSaPUWhhPZSy+tkYoKhIOb4lIyatrOa+/GAazBekaQ6KR27zeCZWfNU8he 6UNMfQ5b9dTcAonIwIC2+s62Gy7pOH1Q+TMnu863p8bO7iYOsuN8cdiebJOczy54zC/6zY /NqJSeL70oSvuuaEYxuvAqFn/CDNllQ= 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-290-dPLIDIk7PqWLQRYLDAD8FA-1; Wed, 11 Nov 2020 14:57:35 -0500 X-MC-Unique: dPLIDIk7PqWLQRYLDAD8FA-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 8A93D64081; Wed, 11 Nov 2020 19:57:34 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-85.ams2.redhat.com [10.36.113.85]) by smtp.corp.redhat.com (Postfix) with ESMTP id CD4675DA6A; Wed, 11 Nov 2020 19:57:32 +0000 (UTC) Subject: Re: [PATCH] OvmfPkg/Bhyve: Update Bhyve following changes to OVMF To: Rebecca Cran Cc: devel@edk2.groups.io, Jordan Justen , Ard Biesheuvel , Peter Grehan , Tom Lendacky References: <20201111031006.33564-1-rebecca@bsdio.com> From: "Laszlo Ersek" Message-ID: <28151b00-7744-ff98-ddd0-52ec527c6f1c@redhat.com> Date: Wed, 11 Nov 2020 20:57:31 +0100 MIME-Version: 1.0 In-Reply-To: <20201111031006.33564-1-rebecca@bsdio.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=lersek@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit +Tom On 11/11/20 04:10, Rebecca Cran wrote: > Fix BhyveX64.dsc and BhyveX64.fdf to follow breaking > changes in OVMF. > > Signed-off-by: Rebecca Cran > --- > OvmfPkg/Bhyve/BhyveX64.dsc | 1 + > OvmfPkg/Bhyve/BhyveX64.fdf | 6 ++++++ > 2 files changed, 7 insertions(+) Ouch, I'm sorry. > > diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc > index 16d2233d77..868338b460 100644 > --- a/OvmfPkg/Bhyve/BhyveX64.dsc > +++ b/OvmfPkg/Bhyve/BhyveX64.dsc > @@ -225,6 +225,7 @@ > > [LibraryClasses.common] > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf > + VmgExitLib|UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf > > [LibraryClasses.common.SEC] > !ifdef $(DEBUG_ON_SERIAL_PORT) Yep, makes sense. > diff --git a/OvmfPkg/Bhyve/BhyveX64.fdf b/OvmfPkg/Bhyve/BhyveX64.fdf > index 5d2586ae14..8776aaf7ac 100644 > --- a/OvmfPkg/Bhyve/BhyveX64.fdf > +++ b/OvmfPkg/Bhyve/BhyveX64.fdf > @@ -76,6 +76,12 @@ gUefiOvmfPkgTokenSpaceGuid.PcdOvmfLockBoxStorageBase|gUefiOvmfPkgTokenSpaceGuid. > 0x007000|0x001000 > gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|gUefiOvmfPkgTokenSpaceGuid.PcdGuidedExtractHandlerTableSize > > +0x008000|0x001000 > +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbPageTableSize > + > +0x009000|0x002000 > +gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecGhcbSize > + > 0x010000|0x010000 > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfSecPeiTempRamSize > > Hmm, this, on the other hand, makes me wonder. All four PCDs are [PcdsFixedAtBuild], from "OvmfPkg.dec", so the platform DSC/FDF files *should not* be required to override defaults. .... Ah, wait, you're hitting the exact PCD value checks (%error directives) in "OvmfPkg/ResetVector/ResetVector.nasmb". During the SEV-ES review, I completely lost track of Bhyve consuming "OvmfPkg/ResetVector/ResetVector.inf". Sorry about that. So the following list of commits: (1) 6995a1b79bab OvmfPkg: Create a GHCB page for use during Sec phase (2) 8a2732186a53 OvmfPkg/ResetVector: Add support for a 32-bit SEV check (3) 30937f2f98c4 OvmfPkg: Use the SEV-ES work area for the SEV-ES AP reset vector causes a problem for the Bhyve platform. I don't like the "OvmfPkg/Bhyve/BhyveX64.fdf" hack as presented above, because, while it makes the symptom go away, it causes "BhyveX64.fdf" appear as if it had anything to do with SEV-ES -- which it doesn't. Here's what I suggest: * patch#1: Subject: OvmfPkg/Bhyve: detach ResetVector from before the SEV-ES changes Commit message: Commits 6995a1b79bab, 8a2732186a53 and 30937f2f98c4 modified all four regular files under "OvmfPkg/ResetVector" with SEV-ES dependencies. These are not relevant for Bhyve. Detach the pre-SEV-ES version of ResetVector for Bhyve. Composing the patch: $ git checkout -b bhyve_reset_vector master $ rm -r OvmfPkg/ResetVector/ $ git checkout 6995a1b79bab^ -- OvmfPkg/ResetVector/ $ mv OvmfPkg/ResetVector/ OvmfPkg/Bhyve/ $ git checkout master -- OvmfPkg/ResetVector/ # add your (C) notices to all files under OvmfPkg/Bhyve/ResetVector/ # namely "PageTables64.asm", "ResetVector.inf", "ResetVector.nasmb" # do *not* re-generate the FILE_GUID in the new INF file (this is a # well-known GUID, namely "gEfiFirmwareVolumeTopFileGuid") $ git add OvmfPkg/Bhyve/ResetVector/ $ git commit * patch#2: Subject: OvmfPkg/Bhyve: fix build breakage after SEV-ES changes Commit message: Consume the SEV-ES-independent reset vector restored in the previous patch. Use the Null instance of VmgExitLib. Body: > diff --git a/OvmfPkg/Bhyve/BhyveX64.dsc b/OvmfPkg/Bhyve/BhyveX64.dsc > index 16d2233d7788..ba79ceef5563 100644 > --- a/OvmfPkg/Bhyve/BhyveX64.dsc > +++ b/OvmfPkg/Bhyve/BhyveX64.dsc > @@ -225,6 +225,7 @@ [LibraryClasses] > > [LibraryClasses.common] > BaseCryptLib|CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf > + VmgExitLib|UefiCpuPkg/Library/VmgExitLibNull/VmgExitLibNull.inf > > [LibraryClasses.common.SEC] > !ifdef $(DEBUG_ON_SERIAL_PORT) > @@ -571,7 +572,7 @@ [PcdsDynamicHii] > # > ################################################################################ > [Components] > - OvmfPkg/ResetVector/ResetVector.inf > + OvmfPkg/Bhyve/ResetVector/ResetVector.inf > > # > # SEC Phase modules > diff --git a/OvmfPkg/Bhyve/BhyveX64.fdf b/OvmfPkg/Bhyve/BhyveX64.fdf > index 5d2586ae141a..f4050c4934b7 100644 > --- a/OvmfPkg/Bhyve/BhyveX64.fdf > +++ b/OvmfPkg/Bhyve/BhyveX64.fdf > @@ -117,7 +117,7 @@ [FV.SECFV] > # > INF OvmfPkg/Sec/SecMain.inf > > -INF RuleOverride=RESET_VECTOR OvmfPkg/ResetVector/ResetVector.inf > +INF RuleOverride=RESET_VECTOR OvmfPkg/Bhyve/ResetVector/ResetVector.inf > > ################################################################################ > [FV.PEIFV] Optimally, these changes should have been part of the SEV-ES feature series, but we didn't realize. Sorry about the regression! Thanks Laszlo