From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by mx.groups.io with SMTP id smtpd.web12.44321.1606173939199695854 for ; Mon, 23 Nov 2020 15:25:39 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@ibm.com header.s=pp1 header.b=dzOnUBqY; spf=pass (domain: linux.ibm.com, ip: 148.163.156.1, mailfrom: jejb@linux.ibm.com) Received: from pps.filterd (m0098396.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.0.42/8.16.0.42) with SMTP id 0ANN2vlk169132; Mon, 23 Nov 2020 18:25:37 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ibm.com; h=message-id : subject : from : reply-to : to : cc : date : in-reply-to : references : content-type : mime-version : content-transfer-encoding; s=pp1; bh=8Dft0wnrPbIXA8DktRBlRW2p56bvoyl2NsPwlEdB8g4=; b=dzOnUBqY7J9Wm/lMx//NaYeYx4ktz6dtMk5vhEDjPPtXJc5hdmjP+fJtNQqZHSuuhBYa hyzsCCh2Nre/xdr62HfqNoh0hi4gVN7BL36wpma+oMaNvSbVvqztmTGMdiva1KWcROXy 13WQ9sxhqFxuMBTzlLWUXgyFerxp8j3kl63SmUS0R3i4JOZkeax1NX3UbpnCQUE4TLFe vIrQNcsLpDkcr5tkUHQl2hbuKRy423GR487faPYkqNVcZTqiSNGuGZSxWdtndgXjtmNl KSS0qNVhX+VP03e+PeKhLp4qL5c4gRq/pRsecIFjv7l7BSBqLkJ7SLdJgedO4rcG24/S dg== Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com with ESMTP id 350n0ytsmt-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 23 Nov 2020 18:25:37 -0500 Received: from m0098396.ppops.net (m0098396.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.36/8.16.0.36) with SMTP id 0ANNFT7N023394; Mon, 23 Nov 2020 18:25:37 -0500 Received: from ppma01dal.us.ibm.com (83.d6.3fa9.ip4.static.sl-reverse.com [169.63.214.131]) by mx0a-001b2d01.pphosted.com with ESMTP id 350n0ytsmm-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 23 Nov 2020 18:25:36 -0500 Received: from pps.filterd (ppma01dal.us.ibm.com [127.0.0.1]) by ppma01dal.us.ibm.com (8.16.0.42/8.16.0.42) with SMTP id 0ANNLOmv017570; Mon, 23 Nov 2020 23:25:36 GMT Received: from b03cxnp07028.gho.boulder.ibm.com (b03cxnp07028.gho.boulder.ibm.com [9.17.130.15]) by ppma01dal.us.ibm.com with ESMTP id 34xth8ym9a-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Mon, 23 Nov 2020 23:25:35 +0000 Received: from b03ledav004.gho.boulder.ibm.com (b03ledav004.gho.boulder.ibm.com [9.17.130.235]) by b03cxnp07028.gho.boulder.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 0ANNPWST2622108 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Mon, 23 Nov 2020 23:25:32 GMT Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 76B1E78060; Mon, 23 Nov 2020 23:25:32 +0000 (GMT) Received: from b03ledav004.gho.boulder.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 59B017805C; Mon, 23 Nov 2020 23:25:30 +0000 (GMT) Received: from jarvis.int.hansenpartnership.com (unknown [9.85.194.234]) by b03ledav004.gho.boulder.ibm.com (Postfix) with ESMTP; Mon, 23 Nov 2020 23:25:30 +0000 (GMT) Message-ID: Subject: Re: [PATCH v2 1/6] OvmfPkg/Amdsev: Base commit to build encrypted boot specific OVMF From: "James Bottomley" Reply-To: jejb@linux.ibm.com To: Laszlo Ersek , 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" Date: Mon, 23 Nov 2020 15:25:29 -0800 In-Reply-To: <608a264b-b284-2512-a786-cab14a6a1237@redhat.com> References: <20201120184521.19437-1-jejb@linux.ibm.com> <20201120184521.19437-2-jejb@linux.ibm.com> <608a264b-b284-2512-a786-cab14a6a1237@redhat.com> User-Agent: Evolution 3.34.4 MIME-Version: 1.0 X-TM-AS-GCONF: 00 X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.312,18.0.737 definitions=2020-11-23_19:2020-11-23,2020-11-23 signatures=0 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 suspectscore=0 malwarescore=0 mlxlogscore=999 spamscore=0 clxscore=1015 phishscore=0 bulkscore=0 mlxscore=0 impostorscore=0 adultscore=0 priorityscore=1501 lowpriorityscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2009150000 definitions=main-2011230147 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit On Mon, 2020-11-23 at 19:01 +0100, Laszlo Ersek wrote: > 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/VariablePoli > cyLib.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: Yes, I've rebased and added the necessary new libraries to get it to build. > On 11/20/20 19:45, James Bottomley wrote: > > [...] > > > +[LibraryClasses.common.DXE_RUNTIME_DRIVER] > > [...] > > > + QemuFwCfgS3Lib|OvmfPkg/Library/QemuFwCfgS3Lib/DxeQemuFwCfgS3LibF > > wCfg.inf > > (2) the following (DXE_RUNTIME_DRIVER-specific) lib class resolution > is missing here: > > > VariablePolicyLib|MdeModulePkg/Library/VariablePolicyLib/VariablePoli > cyLibRuntimeDxe.inf > > > ... So I think that, for (1)+(2) together, simply the "OvmfXen.dsc" > hunks from 435a05aff54d should be incorporated into this patch. Yes, I based the fix on that. > [...] > > > +[PcdsFixedAtBuild] > > + gEfiMdeModulePkgTokenSpaceGuid.PcdStatusCodeMemorySize|1 > > + gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationCh > > ange|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|0x4000 > > 0 > > (3) The above two PCD settings are conditional on NETWORK_TLS_ENABLE > being TRUE, so please remove them together with the condition. I removed them and assumed "with the condition" meant with the condition you removed in the first iteration. > 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. Done. Thanks! James > The rest looks good to me. > > Thanks! > Laszlo >