From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [216.205.24.124]) by mx.groups.io with SMTP id smtpd.web10.39035.1606154531660242807 for ; Mon, 23 Nov 2020 10:02:11 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=f69V8YCO; spf=pass (domain: redhat.com, ip: 216.205.24.124, mailfrom: lersek@redhat.com) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1606154530; 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=Pd3ijB85K9MWloSgwKFkgSmAjewUw/nFld+4FQEa4h4=; b=f69V8YCO8kx52ACX3yYczs3vXRIVtaL0+LdMpWRJ8NJE9yWBrY3BRNuFCiltz8IR8yhLJc /KxFuD7tAUDhsRbmbQ5HsgwOy2tipuC0v/Mmzk5N/YvkY/EO6H2cbV/rPho2kG3Yt//QaG mg2qfH0py+Fdn6Oapf9BhprM2qyjamE= 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-153-2TsU75dUMIigk0BV9krfyA-1; Mon, 23 Nov 2020 13:01:58 -0500 X-MC-Unique: 2TsU75dUMIigk0BV9krfyA-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 4819518C43C2; Mon, 23 Nov 2020 18:01:56 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-112-230.ams2.redhat.com [10.36.112.230]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6532810013BD; Mon, 23 Nov 2020 18:01:53 +0000 (UTC) Subject: Re: [PATCH v2 1/6] OvmfPkg/Amdsev: Base commit to build encrypted boot specific OVMF To: James Bottomley , devel@edk2.groups.io Cc: dovmurik@linux.vnet.ibm.com, Dov.Murik1@il.ibm.com, ashish.kalra@amd.com, brijesh.singh@amd.com, tobin@ibm.com, david.kaplan@amd.com, jon.grimm@amd.com, thomas.lendacky@amd.com, frankeh@us.ibm.com, "Dr . David Alan Gilbert" References: <20201120184521.19437-1-jejb@linux.ibm.com> <20201120184521.19437-2-jejb@linux.ibm.com> From: "Laszlo Ersek" Message-ID: <608a264b-b284-2512-a786-cab14a6a1237@redhat.com> Date: Mon, 23 Nov 2020 19:01:52 +0100 MIME-Version: 1.0 In-Reply-To: <20201120184521.19437-2-jejb@linux.ibm.com> X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 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 On 11/20/20 19:45, James Bottomley wrote: > This commit represents the file copied from OvmfPkgX64 with minor > changes to change the build name. > > This package will form the basis for adding Sev specific features. > Since everything must go into a single rom file for attestation, the > separated build of code and variables is eliminated. > > Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=3077 > Signed-off-by: James Bottomley > > --- > > v2: remove secure boot, smm and networking > --- > OvmfPkg/AmdSev/AmdSevX64.dsc | 867 +++++++++++++++++++++++++++++++++++ > OvmfPkg/AmdSev/AmdSevX64.fdf | 461 +++++++++++++++++++ > 2 files changed, 1328 insertions(+) > create mode 100644 OvmfPkg/AmdSev/AmdSevX64.dsc > create mode 100644 OvmfPkg/AmdSev/AmdSevX64.fdf > > diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.dsc > new file mode 100644 > index 000000000000..852be757bfbe > --- /dev/null > +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc > @@ -0,0 +1,867 @@ [...] > +[LibraryClasses] [...] > + VarCheckLib|MdeModulePkg/Library/VarCheckLib/VarCheckLib.inf (1) The following two lib class resolutions are missing here: VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLib.inf VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf They were added to the other OVMF DSC files in commit 435a05aff54d ("OvmfPkg: Add VariablePolicy engine to OvmfPkg platform", 2020-11-17). This dependency didn't exist at the time of your v1 posting (Nov 12th, in my time zone). But now the new platform doesn't seem to build without them: > OvmfPkg/AmdSev/AmdSevX64.dsc(...): error 4000: Instance of library class [VariablePolicyLib] is not found > in [MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf] [X64] > consumed by module [MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf] They are required by "MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf" too, not just by "MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf" (which is correctly removed by this patch). ... actually, for VariableRuntimeDxe: On 11/20/20 19:45, James Bottomley wrote: [...] > +[LibraryClasses.common.DXE_RUNTIME_DRIVER] [...] > + QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibFwCfg.inf (2) the following (DXE_RUNTIME_DRIVER-specific) lib class resolution is missing here: VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePolicyLibRuntimeDxe.inf ... So I think that, for (1)+(2) together, simply the "OvmfXen.dsc" hunks from 435a05aff54d should be incorporated into this patch. [...] > +[PcdsFixedAtBuild] > + gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 > + gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE > + gEfiMdePkgTokenSpaceGuid.PcdMaximumGuidedExtractHandler|0x10 > +!if ($(FD_SIZE_IN_KB) == 1024) || ($(FD_SIZE_IN_KB) == 2048) > + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x2000 > + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x2800 > + # match PcdFlashNvStorageVariableSize purely for convenience > + gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0xe000 > +!endif > +!if $(FD_SIZE_IN_KB) == 4096 > + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVariableSize|0x8400 > + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxAuthVariableSize|0x8400 > + # match PcdFlashNvStorageVariableSize purely for convenience > + gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x40000 > +!endif > + gEfiMdeModulePkgTokenSpaceGuid.PcdVariableStoreSize|0x80000 > + gEfiMdeModulePkgTokenSpaceGuid.PcdMaxVolatileVariableSize|0x40000 (3) The above two PCD settings are conditional on NETWORK_TLS_ENABLE being TRUE, so please remove them together with the condition. Right now, they override the settings in the just preceding blocks (which depend on FD_SIZE_IN_KB being 1M / 2M / 4M). [...] > +[PcdsDynamicDefault] > + # only set when > + # ($(SMM_REQUIRE) == FALSE) (4) please remove the comment (only the comment) > + gEfiMdeModulePkgTokenSpaceGuid.PcdEmuVariableNvStoreReserved|0 [...] > +[Components] [...] > + OvmfPkg/VirtioNetDxe/VirtioNet.inf (5) Please remove this driver from the DSC file too (you correctly removed it from the FDF file). Right now, the driver is built, just not included. We shouldn't build it. The rest looks good to me. Thanks! Laszlo