From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM12-MW2-obe.outbound.protection.outlook.com (NAM12-MW2-obe.outbound.protection.outlook.com [40.107.244.41]) by mx.groups.io with SMTP id smtpd.web08.1619.1619638995366152592 for ; Wed, 28 Apr 2021 12:43:15 -0700 Authentication-Results: mx.groups.io; dkim=fail reason="body hash did not verify" header.i=@amd.com header.s=selector1 header.b=jkpocsWa; spf=permerror, err=parse error for token &{10 18 %{i}._ip.%{h}._ehlo.%{d}._spf.vali.email}: invalid domain name (domain: amd.com, ip: 40.107.244.41, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=B4+W7E7APCL9y25HpYzs9098uFfizdstKkjxlwAm87gP01Ng0AYufJmx+QWRA1KydvZwb9ORC83lSTsZSDIg13HDWoZjrmmp9sQAJuMI7DMsz3ETiOxs6fJHvDUEF1yjUQsK/OWMetlMu5+m2T8+Ny94sJ5RyIEfFzIjNIUf0gKbbAiCNzvHmPmFWg0/SuIoZ8vgs9B2e2METfsJDxWBIbx1LdnHqN73y0SBMY/zk06I2Ojgaz7peeJG1UoLDrlQhEyCYZkIoXV100YZOLPID5ilkVqjrDKqeV5YPeJ/oYKxuw1ROUpdXjNQvJIXGqBpu3EWKqHH1SCrBu22kGPyBw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=kxYCNT0vDq3hgWJ9nsr1FlzeaHKd6w5nquk306pFBRM=; b=esCE6hw20DT6VRNQB9cTHh/xfsRbKgo9Fg8kaEANVs50xV1aoKJrUVL5KZuRtQiAttFJ18oLKkSzGAZfychjUNe7AStJ2RulN/7bcVEhBjGY9YUWTh/DTC7MZt1aqaKnHR0IRyMvv5UVbSy0Oi/Pn7JEICyphwt6FQ041j4Pn1UoKXYbnBSTGxN0XudzvU+46ULp5lx+rSenxRi3icgem+CMnpwLF2I4Qt8dpN2p3WViEdDqKxRq3T5w5+RUMdoxWLu4zsaDEkSIYtgxkZQJzYx7YUC4BRjMV1XLFNmQ8jruf/T9ahvdw9eQ2WelzTVLaDPRnuODHGLrJ/9X5vB+GA== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amd.com; s=selector1; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=kxYCNT0vDq3hgWJ9nsr1FlzeaHKd6w5nquk306pFBRM=; b=jkpocsWacquA+auwyEstJSP28ZsiOD5LyW165PB0nJ65ctfVjui7enhVVaVIbPPuhMDUrtp3I74b6jX+0csSju1TZB+QTJcfUoKQ+gGmDZ6STKNxGJBJ1gBWsYYh/L4qYk8VFUNjahIt/TzYyrFI1b7tv4BMNVjafeD1R6eEIKQ= Authentication-Results: linux.ibm.com; dkim=none (message not signed) header.d=none;linux.ibm.com; dmarc=none action=none header.from=amd.com; Received: from DM5PR12MB1355.namprd12.prod.outlook.com (2603:10b6:3:6e::7) by DM6PR12MB4987.namprd12.prod.outlook.com (2603:10b6:5:163::31) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4042.21; Wed, 28 Apr 2021 19:43:11 +0000 Received: from DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::b914:4704:ad6f:aba9]) by DM5PR12MB1355.namprd12.prod.outlook.com ([fe80::b914:4704:ad6f:aba9%12]) with mapi id 15.20.4065.026; Wed, 28 Apr 2021 19:43:11 +0000 Subject: Re: [edk2-devel] [PATCH v2 4/4] OvmfPkg/Tcg2ConfigPei: Mark TPM MMIO range as unencrypted for SEV-ES To: devel@edk2.groups.io, lersek@redhat.com CC: Joerg Roedel , Borislav Petkov , Ard Biesheuvel , Jordan Justen , Brijesh Singh , Erdem Aktas , James Bottomley , Jiewen Yao , Min Xu , =?UTF-8?Q?Marc-Andr=c3=a9_Lureau?= , Stefan Berger References: <00ff47c80f180b5b9054890de0ce5e1975fe2b1f.1619540470.git.thomas.lendacky@amd.com> <6807464e-823e-3a16-cf1c-24f612a43936@redhat.com> From: "Lendacky, Thomas" Message-ID: <096090a1-6fd4-6364-fc88-733a0b3ef422@amd.com> Date: Wed, 28 Apr 2021 14:43:09 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 In-Reply-To: <6807464e-823e-3a16-cf1c-24f612a43936@redhat.com> X-Originating-IP: [67.79.209.213] X-ClientProxiedBy: SN2PR01CA0053.prod.exchangelabs.com (2603:10b6:800::21) To DM5PR12MB1355.namprd12.prod.outlook.com (2603:10b6:3:6e::7) Return-Path: thomas.lendacky@amd.com MIME-Version: 1.0 X-MS-Exchange-MessageSentRepresentingType: 1 Received: from office-linux.texastahm.com (67.79.209.213) by SN2PR01CA0053.prod.exchangelabs.com (2603:10b6:800::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4087.27 via Frontend Transport; Wed, 28 Apr 2021 19:43:10 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: 5bf43492-8a27-4711-d027-08d90a7ddb48 X-MS-TrafficTypeDiagnostic: DM6PR12MB4987: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:10000; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: RQ/Z7gTZi2OWE+HPtHgkV6OfZCFTbLJZcIjCqrrVIUtHy9fGJeN26Uwfh9jRv910WLJV9kbM5vAjfwtKjWq9xqEGhXWLJ2OftA+OJBpnKra66ZdNra/fr/W0IdGT0Smv6IpyY2t6+/eqetj0pV2ao/KB1W+okJ8Cz8i6sJZq/0+YAKH9JZSKClrE0s6gXiMglyuAqj6UUZDMdYX0J3RFkqIxqh/JviQw3RzeEoYaQ9AP07QzvvxafW0vNlT7VucNlkh/r8TsD1vsE1Z8dqX0TE8CSTau8eUeu2QkNjHITfsDjBcPa9aGEO++t2jJXKowVzAmWmG2sSCuZmdFQpDsUukeqAM61S+PiBGjkVuTFoAOZ80A1kXXqSLuDODxsxm7bpjGkIgw92NJjF9HOKNUzTh2pWohNqp9DzLU3lplqFB7AND2dFhmQM+ArLSS44S1bw6LFER87HHNRYEyRLA/x1MIhy5zyjjlHRktOA47xH478kDuW+VCJMrRhd/X1eQqjAeJ5Mrifnf+kfcQgDWsaw/5WaWqEdQOFW01LRH8YQJ+GIPcR1Ofbm1tAIfDge+rqLOYABncMi3P05reh1lYD99MRP8YSB0dzyvgHHks6/HPXHtoFOUQhMHcoa7h1IZk5slYMUCT8IPNNy1HjMUaUe5GhVaR2xazgtVgza/Qmf6iMvRDfAZLzu8bS9qQgwFvwDIYP8VzK4R/rmWeSUtoGPYiHm/oBWgaACdZbKP+nNM= X-Forefront-Antispam-Report: CIP:255.255.255.255;CTRY:;LANG:en;SCL:1;SRV:;IPV:NLI;SFV:NSPM;H:DM5PR12MB1355.namprd12.prod.outlook.com;PTR:;CAT:NONE;SFS:(4636009)(346002)(396003)(39860400002)(366004)(136003)(376002)(83380400001)(54906003)(16526019)(6506007)(8676002)(26005)(4326008)(8936002)(31686004)(2906002)(66476007)(66946007)(186003)(19627235002)(31696002)(2616005)(5660300002)(956004)(36756003)(38100700002)(66556008)(6486002)(7416002)(478600001)(45080400002)(53546011)(6512007)(966005)(30864003)(86362001)(316002)(43740500002);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?us-ascii?Q?nKRlu2l93hzz5GC8hMUWitFO/3iHxQLUje/rbQ4hO8IWWFh7SiO+sSYwJwmT?= =?us-ascii?Q?hnQfq9LCX+/X2gAfQ/dLeWHVXEyzEmzQZoYYvSZg/e3h4nASfd21gOMj0lwD?= =?us-ascii?Q?YVJBN0MUV9S0Qx0MRDk+UQGGrrfVvHA+5RYKtRmD74hR7bXu1tiEsNyhP8mc?= =?us-ascii?Q?7+OntjuYk/172BcPcTEhJaMJocWS0LJq9OnFBMWUK8fOcNadMlyIFq0DBC0I?= =?us-ascii?Q?r8foDIsnClVRXJ28+CJ5U6P6vVlZ2W5VCaWPJBbOttcJzbBbTg5f3kB9+ax8?= =?us-ascii?Q?6X0pFlFP915DjuMz/oqpAF817WCkUPqQ4tQln4d3UhXC2XhriMWAL4hRHWFn?= =?us-ascii?Q?FRtFwxMt6VV211M/jbYva8rExR4/R1c4Wr8a/3wM+xih1M2XLMINu6CTbsrX?= =?us-ascii?Q?Fu1CmiYMrUe7aL0EbtIw7w524bw7vMQZiUTeAxmk8xTGZ6UA6WRHH0gxRYLI?= =?us-ascii?Q?Zvt5y/b0lqX5K9sV2v8/QzWBctQq/s5WgumBlXvTmoK+h+zRt8lB5JmM/mDM?= =?us-ascii?Q?ydifqN9CRrHQ3xppTlZo3HcE4T8FDyhxY1/wlTYKA31spqhvLs7o3ghhYsB7?= =?us-ascii?Q?lT6vzBZWgwjOs7kTA+inMRLQEovDnb+YiAjox/ZnAOVa6qzjn8YfxZ9yKbfm?= =?us-ascii?Q?7itzLdug4smggYKcfYHklCXdtf3cNP6Hlxb4pPa4yeifcSxYnqedWbWg2vA3?= =?us-ascii?Q?29BBns0OkbLdDwb5h4bQOpl9xbnjC2UrDyUdkgjSYVrA7uj02rcJXgZm8m0h?= =?us-ascii?Q?Bwt84LETF4ZXjWanEcu2K4vMO8QDrJC0VTLsehKP+2uwDqWpFLm62y4+y/FK?= =?us-ascii?Q?AJNlM33C7+HGTu0E+BKm0XEvqtmDL8CuOugUuzM3OvuRWPR77vMIG2AbbNk7?= =?us-ascii?Q?fN8za5zLmpur52gOA3DaZu6j7FgkOA3/ETfBZFimfNTNo/QfJsvTmDt6JPRN?= =?us-ascii?Q?jQ433vBs5MWg4Cx8VoCpLtGpuIaz5jXQM+VEgC/Whv64wVk0fQsbni78+aPC?= =?us-ascii?Q?Lk2ZZrB2bClD7FEaBzc1wNk7PbWFvwQS+9g9RWmaAAkslhky/61BKEiTdgaM?= =?us-ascii?Q?/pYGMureoh3E2x9fP8x75Yy6XCDpodjbnaIG8spNzOqeN0G8OchkM9jnIAEl?= =?us-ascii?Q?9KY5AoHmDeTIgXIUZJQ9oAQLetUcP9LUCXLlu1P8V83AbFXj/YYU0cEVRnfg?= =?us-ascii?Q?Uv0yhYZVw7pxCtbZdoRN3tUWl17wauQjVn4uq5zmfdJIF1hQVZ/4+WBKJN11?= =?us-ascii?Q?3kBTqm+s+H/wN6p41GfD9kbomgh36lzeujLpKQMXy+0GtdgzsvJOYjHnFITP?= =?us-ascii?Q?XWGoegpfkVbGH7wN8bJmRoSq?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: 5bf43492-8a27-4711-d027-08d90a7ddb48 X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB1355.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 28 Apr 2021 19:43:11.7457 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d X-MS-Exchange-CrossTenant-MailboxType: HOSTED X-MS-Exchange-CrossTenant-UserPrincipalName: Nth8+2u3/qt7L7VjFWT7ShYAd/yTCorysi6p1wleGKs+9WZVcq5sHT8r1XREhRAGQsHQqNo40fuPWC/MWPqZhQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4987 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: quoted-printable On 4/28/21 12:51 PM, Laszlo Ersek via groups.io wrote: > I'm going to ask for v3 after all: >=20 > On 04/27/21 18:21, Lendacky, Thomas wrote: >> From: Tom Lendacky >> >> BZ: https://nam11.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2= Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3345&data=3D04%7C01%7Ctho= mas.lendacky%40amd.com%7C3c65ebfe044e4f3eb5b808d90a6e5455%7C3dd8961fe4884e6= 08e11a82d994e183d%7C0%7C0%7C637552291252644310%7CUnknown%7CTWFpbGZsb3d8eyJW= IjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&s= data=3DG1GwQc6sZqRuNHWC5vbdb78gCOl4YkAq%2BHi0F0ceucg%3D&reserved=3D0 >> >> During PEI, the MMIO range for the TPM is marked as encrypted when runn= ing >> as an SEV guest. While this isn't an issue for an SEV guest because of >> the way the nested page fault is handled, it does result in an SEV-ES >> guest terminating because of a mitigation check in the #VC handler to >> prevent MMIO to an encrypted address. For an SEV-ES guest, this range >> must be marked as unencrypted. >> >> Create a new x86 PEIM for TPM support that will map the TPM MMIO range = as >> unencrypted when SEV-ES is active. The gOvmfTpmMmioAccessiblePpiGuid PP= I >> will be unconditionally installed before exiting. The PEIM will exit wi= th >> the EFI_ABORTED status so that the PEIM does not stay resident. >=20 > (1) Please spell out that the new PEIM will depend on the installation > of the permanent PEI RAM, by PlatformPei -- the reason being that, in > case page table splitting proves necessary for clearing the C-bit, the > new page table(s) should be allocated from permanent PEI RAM. Will do. >=20 >=20 >> >> The OVMF Tcg2Config PEIM will add the gOvmfTpmMmioAccessiblePpiGuid as = a >> Depex for IA32 and X64 builds so that the MMIO range is properly mapped >> for SEV-ES before the Tcg2Config PEIM is loaded. >=20 > (2) The Tcg2Config depex change should be a separate patch -- the last > patch in the series. That covers both the Tcg2Config hunk in the patch > body, and this commit message paragraph right here. Ok, I'll split that out. >=20 >=20 > (3) While at it: those parts of this patch that are *not* related to the > Tcg2Config PEIM can be squashed into the previous patch -- if you prefer > that. >=20 > I'm certainly happy with three separate patches though: for the DEC > file, for TpmMmioSevDecryptPei + the DSC/FDF files, and finally the > Tcg2Config PEIM. So in total the series should include 4 or 5 patches > (up to you). I'll do it as 5 patches. >=20 >=20 >> >> Update all OVMF Ia32 and X64 build packages to include this new PEIM. >> >> Cc: Laszlo Ersek >> Cc: Ard Biesheuvel >> Cc: Jordan Justen >> Cc: Brijesh Singh >> Cc: Erdem Aktas >> Cc: James Bottomley >> Cc: Jiewen Yao >> Cc: Min Xu >> Cc: Marc-Andr?? Lureau >=20 > (4) Marc-Andr=C3=A9's name is garbled here too. >=20 > The following git config options are related: >=20 > - For encoding non-ASCII characters in git commits, the > "i18n.commitencoding" knob is relevant. Almost universally, this should > be "UTF-8" (assuming your text editor used for composing commit messages > produces UTF-8-encoded files). >=20 > - For formatting commits to patch emails, "i18n.logOutputEncoding" > matters. This should *always* be "UTF-8", when git-format-patch is invok= ed. We were having problems with sending patches via git-format-patch and git-send-email and our email system. Likely some left over .gitconfig entries that are causing the problem. I'll double check everything. >=20 >=20 >> Cc: Stefan Berger >> Signed-off-by: Tom Lendacky >> --- >> OvmfPkg/AmdSev/AmdSevX64.dsc | 1 + >> OvmfPkg/OvmfPkgIa32.dsc | 1 + >> OvmfPkg/OvmfPkgIa32X64.dsc | 1 + >> OvmfPkg/OvmfPkgX64.dsc | 1 + >> OvmfPkg/AmdSev/AmdSevX64.fdf | 1 + >> OvmfPkg/OvmfPkgIa32.fdf | 1 + >> OvmfPkg/OvmfPkgIa32X64.fdf | 1 + >> OvmfPkg/OvmfPkgX64.fdf | 1 + >> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf | 2 +- >> OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf | 40 +++++++= ++++ >> OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPeim.c | 76 +++++++= +++++++++++++ >> 11 files changed, 125 insertions(+), 1 deletion(-) >=20 > Right, skipping Bhyve is justified, per your previous report / > . >=20 >> >> diff --git a/OvmfPkg/AmdSev/AmdSevX64.dsc b/OvmfPkg/AmdSev/AmdSevX64.ds= c >> index cdb29d53142d..5a5246c64bf7 100644 >> --- a/OvmfPkg/AmdSev/AmdSevX64.dsc >> +++ b/OvmfPkg/AmdSev/AmdSevX64.dsc >> @@ -627,6 +627,7 @@ [Components] >> >> !if $(TPM_ENABLE) =3D=3D TRUE >> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf >> + OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf >> SecurityPkg/Tcg/TcgPei/TcgPei.inf >> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf { >> >=20 > (5) Functionally correct, but it reads more nicely (from a logical > dependency POV) if we place the new PEIM first. >=20 > (Please apply to the rest of the DSC files, and the FDF files too.) Ok, I was going with the alphabetical placement. I'll switch it up. >=20 >=20 >> diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc >> index 1730b6558b5c..a33c14c673a0 100644 >> --- a/OvmfPkg/OvmfPkgIa32.dsc >> +++ b/OvmfPkg/OvmfPkgIa32.dsc >> @@ -707,6 +707,7 @@ [Components] >> >> !if $(TPM_ENABLE) =3D=3D TRUE >> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf >> + OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf >> SecurityPkg/Tcg/TcgPei/TcgPei.inf >> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf { >> >> diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc >> index 78a559da0d0b..a4ff7ed44705 100644 >> --- a/OvmfPkg/OvmfPkgIa32X64.dsc >> +++ b/OvmfPkg/OvmfPkgIa32X64.dsc >> @@ -720,6 +720,7 @@ [Components.IA32] >> >> !if $(TPM_ENABLE) =3D=3D TRUE >> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf >> + OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf >> SecurityPkg/Tcg/TcgPei/TcgPei.inf >> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf { >> >> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc >> index a7d747f6b4ab..3fb56b3f9ff9 100644 >> --- a/OvmfPkg/OvmfPkgX64.dsc >> +++ b/OvmfPkg/OvmfPkgX64.dsc >> @@ -719,6 +719,7 @@ [Components] >> >> !if $(TPM_ENABLE) =3D=3D TRUE >> OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf >> + OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf >> SecurityPkg/Tcg/TcgPei/TcgPei.inf >> SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf { >> >> diff --git a/OvmfPkg/AmdSev/AmdSevX64.fdf b/OvmfPkg/AmdSev/AmdSevX64.fd= f >> index c0098502aa90..ab58a9c0b4da 100644 >> --- a/OvmfPkg/AmdSev/AmdSevX64.fdf >> +++ b/OvmfPkg/AmdSev/AmdSevX64.fdf >> @@ -148,6 +148,7 @@ [FV.PEIFV] >> >> !if $(TPM_ENABLE) =3D=3D TRUE >> INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf >> +INF OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf >> INF SecurityPkg/Tcg/TcgPei/TcgPei.inf >> INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf >> !endif >> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf >> index f400c845b9c9..fc0ae1f280df 100644 >> --- a/OvmfPkg/OvmfPkgIa32.fdf >> +++ b/OvmfPkg/OvmfPkgIa32.fdf >> @@ -163,6 +163,7 @@ [FV.PEIFV] >> >> !if $(TPM_ENABLE) =3D=3D TRUE >> INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf >> +INF OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf >> INF SecurityPkg/Tcg/TcgPei/TcgPei.inf >> INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf >> !endif >> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf >> index d055552fd09f..306fc5a9b60d 100644 >> --- a/OvmfPkg/OvmfPkgIa32X64.fdf >> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf >> @@ -163,6 +163,7 @@ [FV.PEIFV] >> >> !if $(TPM_ENABLE) =3D=3D TRUE >> INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf >> +INF OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf >> INF SecurityPkg/Tcg/TcgPei/TcgPei.inf >> INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf >> !endif >> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf >> index d519f8532822..22c8664427d6 100644 >> --- a/OvmfPkg/OvmfPkgX64.fdf >> +++ b/OvmfPkg/OvmfPkgX64.fdf >> @@ -175,6 +175,7 @@ [FV.PEIFV] >> >> !if $(TPM_ENABLE) =3D=3D TRUE >> INF OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf >> +INF OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf >> INF SecurityPkg/Tcg/TcgPei/TcgPei.inf >> INF SecurityPkg/Tcg/Tcg2Pei/Tcg2Pei.inf >> !endif >> diff --git a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf b/OvmfPkg/Tcg/Tcg= 2Config/Tcg2ConfigPei.inf >> index 6776ec931ce0..39d1deeed16b 100644 >> --- a/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf >> +++ b/OvmfPkg/Tcg/Tcg2Config/Tcg2ConfigPei.inf >> @@ -57,7 +57,7 @@ [Pcd] >> gEfiSecurityPkgTokenSpaceGuid.PcdTpmInstanceGuid ## = PRODUCES >> >> [Depex.IA32, Depex.X64] >> - TRUE >> + gOvmfTpmMmioAccessiblePpiGuid >> >> [Depex.ARM, Depex.AARCH64] >> gOvmfTpmDiscoveredPpiGuid >> diff --git a/OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf = b/OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf >> new file mode 100644 >> index 000000000000..926113b8ffb0 >> --- /dev/null >> +++ b/OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf >> @@ -0,0 +1,40 @@ >> +## @file >> +# Map TPM MMIO range unencrypted when SEV is active >=20 > (6) Please add another sentence here: "Install > gOvmfTpmMmioAccessiblePpiGuid unconditionally". Will do. >=20 >=20 >> +# >> +# Copyright (C) 2021, Advanced Micro Devices, Inc. >> +# >> +# SPDX-License-Identifier: BSD-2-Clause-Patent >> +## >> + >> +[Defines] >> + INF_VERSION =3D 0x00010005 >=20 > (7) The latest INF spec version is 1.29: >=20 > https://nam11.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Fgith= ub.com%2Ftianocore%2Ftianocore.github.io%2Fwiki%2FEDK-II-Draft-Specificatio= n&data=3D04%7C01%7Cthomas.lendacky%40amd.com%7C3c65ebfe044e4f3eb5b808d9= 0a6e5455%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637552291252644310%7C= Unknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiL= CJXVCI6Mn0%3D%7C1000&sdata=3DAXuQkvUSwLjEyZiwivQQaUwTaY7Mo0wLSHUf8QKNLC= 8%3D&reserved=3D0 > https://nam11.safelinks.protection.outlook.com/?url=3Dhttps%3A%2F%2Ftian= ocore-docs.github.io%2Fedk2-InfSpecification%2Fdraft%2F&data=3D04%7C01%= 7Cthomas.lendacky%40amd.com%7C3c65ebfe044e4f3eb5b808d90a6e5455%7C3dd8961fe4= 884e608e11a82d994e183d%7C0%7C0%7C637552291252644310%7CUnknown%7CTWFpbGZsb3d= 8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&= amp;sdata=3DYUHs6g5aMPWBjCNjcZPKnTSEs2gBazDX094nqj9qpnE%3D&reserved=3D0 >=20 > plus INF_VERSION no longer requires a binary-only (hex-only) format. So > please just write "1.29". Will do. >=20 >=20 >> + BASE_NAME =3D TpmMmioSevDecryptPei >> + FILE_GUID =3D F12F698A-E506-4A1B-B32E-6920E55DA= 1C4 >> + MODULE_TYPE =3D PEIM >> + VERSION_STRING =3D 1.0 >> + ENTRY_POINT =3D TpmMmioSevDecryptPeimEntryPoint >> + >> +[Sources] >> + TpmMmioSevDecryptPeim.c >> + >> +[Packages] >> + MdePkg/MdePkg.dec >> + MdeModulePkg/MdeModulePkg.dec >> + OvmfPkg/OvmfPkg.dec >> + SecurityPkg/SecurityPkg.dec >=20 > (8) Is MdeModulePkg necessary? I don't think so. Let me double check it. >=20 >=20 >> + >> +[LibraryClasses] >> + BaseLib >> + DebugLib >> + MemEncryptSevLib >> + PeimEntryPoint >> + PeiServicesLib >> + >> +[Ppis] >> + gOvmfTpmMmioAccessiblePpiGuid ## PRODUCES >> + >> +[FixedPcd] >> + gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress ## CONSUMES >> + >> +[Depex] >> + gEfiPeiMemoryDiscoveredPpiGuid >> diff --git a/OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPeim.c b= /OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPeim.c >> new file mode 100644 >> index 000000000000..dd1f1a80b5b0 >> --- /dev/null >> +++ b/OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPeim.c >> @@ -0,0 +1,76 @@ >> +/** @file >> + Map TPM MMIO range unencrypted when SEV is active >=20 > (9) Same request as (6) -- please add another sentence: "Install > gOvmfTpmMmioAccessiblePpiGuid unconditionally". >=20 Will do. >=20 >> + >> + Copyright (C) 2021, Advanced Micro Devices, Inc. >> + >> + SPDX-License-Identifier: BSD-2-Clause-Patent >> +**/ >> + >> + >> +#include >> + >> +#include >> +#include >> +#include >=20 > (10) This Library #include list does not match the [LibraryClasses] > section of the INF file (the PeimEntryPoint class apart, which should > indeed only be in the INF file). In other words, BaseLib appears > superfluous in the INF file. Ok, let me check on that and fix as appropriate. >=20 >=20 >> + >> +STATIC CONST EFI_PEI_PPI_DESCRIPTOR mTpmMmioRangeAccessible =3D { >> + EFI_PEI_PPI_DESCRIPTOR_PPI | EFI_PEI_PPI_DESCRIPTOR_TERMINATE_LIST, >> + &gOvmfTpmMmioAccessiblePpiGuid, >> + NULL >> +}; >> + >> +/** >> + The entry point for TPM MMIO range mapping driver. >> + >> + @param[in] FileHandle Handle of the file being invoked. >> + @param[in] PeiServices Describes the list of possible PEI Services= . >> + >> + @retval EFI_ABORTED No need to keep this PEIM resident >> +**/ >> +EFI_STATUS >> +EFIAPI >> +TpmMmioSevDecryptPeimEntryPoint ( >> + IN EFI_PEI_FILE_HANDLE FileHandle, >> + IN CONST EFI_PEI_SERVICES **PeiServices >> + ) >> +{ >> + RETURN_STATUS DecryptStatus; >> + EFI_STATUS Status; >> + >> + DEBUG ((DEBUG_INFO, "%a\n", __FUNCTION__)); >> + >> + // >> + // If SEV or SEV-ES is active, MMIO succeeds against an encrypted ph= ysical >> + // address because the nested page fault (NPF) that occurs on access= does not >> + // include the encryption bit in the guest physical address provided= to the >> + // hypervisor. >> + // >> + // However, if SEV-ES is active, before performing the actual MMIO, = an >> + // additional MMIO mitigation check is performed in the #VC handler = to ensure >> + // that MMIO is being done to an unencrypted address. To prevent gue= st >> + // termination in this scenario, mark the range unencrypted ahead of= access. >> + // >=20 > Lovely comment, thanks! >=20 >> + if (MemEncryptSevEsIsEnabled ()) { >> + DEBUG ((DEBUG_INFO, "%a: mapping TPM MMIO address range unencrypte= d\n", __FUNCTION__)); >> + >> + DecryptStatus =3D MemEncryptSevClearPageEncMask ( >> + 0, >> + PcdGet64 (PcdTpmBaseAddress), >=20 > (11) The INF file says [FixedPcd], so it would be cleanest to say > FixedPcdGet64() here. Will do. >=20 >=20 > (12) PcdLib is missing from both the [LibraryClasses] section and the > #include directives. Right, I'll update that. >=20 >=20 >> + EFI_SIZE_TO_PAGES ((UINTN) 0x5000), >> + FALSE >> + ); >> + >> + if (RETURN_ERROR (DecryptStatus)) { >> + DEBUG ((DEBUG_INFO, "%a: failed to map TPM MMIO address range un= encrypted\n", __FUNCTION__)); >=20 > (13) Overlong line. Ok, I'll change that. I though that was ok now since PatchCheck.py didn't complain. >=20 >=20 > (14) Please report errors with DEBUG_ERROR. Yup, will change. Thanks, Tom >=20 >=20 >> + ASSERT_RETURN_ERROR (DecryptStatus); >> + } >> + } >> + >> + // >> + // MMIO range available >> + // >> + Status =3D PeiServicesInstallPpi (&mTpmMmioRangeAccessible); >> + ASSERT_EFI_ERROR (Status); >> + >> + return EFI_ABORTED; >> +} >> >=20 > Thanks! > Laszlo >=20 >=20 >=20 >=20 >=20 >=20