From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from NAM02-DM3-obe.outbound.protection.outlook.com (NAM02-DM3-obe.outbound.protection.outlook.com [40.107.95.88]) by mx.groups.io with SMTP id smtpd.web08.7534.1619446889185518674 for ; Mon, 26 Apr 2021 07:21:29 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@amd.com header.s=selector1 header.b=JgH4Ahwi; 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.95.88, mailfrom: thomas.lendacky@amd.com) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=l2uXtUc4w+vKmyJcbkoH/BzLs9IvziZNdmY49ONzNvzBQtgg7y2DJeoHDmVr2o2p0dTOzGPE0OmhXbeF3TuZio8pYspM2q7I2RdXuGGZjmxvDkDkK69Q5Dpi8uNqAca0FBQfH2d4QroOZO0OBMQfZaHW92Dk1jmmDg1K6Gll7UDmQVuQohLQ8pXWstIrufrVHeo9Y7bQ//yMH5s/lj06t5jaIiZWpoA1VDRLJQ97M71LKPFOlibeqevPGhhAXP5WRa/sbtebDZtTxMeG/EzRu1ahZMNsSyQy8Db3pfhdO01He8elj5gRMFJneU+NUmAD5YmCcgWeCA7VcDUAEI4AYg== 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=+QNIHEYvGcsAdX6ReH9n0cN+V3fGlSRlZP7sUnXPB1U=; b=SPiLtalWa1wsMvfDmEE6KGa78ftKGaC0Yebs1xq3zojnfSTXmNa+3DRKuKs4veCHUnIMKnWUUSVU/2Ro9X2ZStXJwpemGV0OGaEDKK6WVZ3ASR5cqFg47znIIjwQn6fcVjLZPmOeqmGCl7dUYGVTJz/H7tdzR9ATVKCrKGOGxiMTPY1qnOVGB6B4sVd3eI4p0+/5BspuqzIpLmbfiP3U1EGC6/LY9fX0IN6Rt6ivWHlgk/1askmgyl4DSw7+CsvsrOEyJVki+IIP9pCM1kNpVAro1gueVDoq/ot6c2jeN8sfQMQWOIcjTjRHxyOYMxKhkz/81VLuus17phI7+kt+Jg== 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=+QNIHEYvGcsAdX6ReH9n0cN+V3fGlSRlZP7sUnXPB1U=; b=JgH4AhwiuBinEgV9FXx8xjNeEtu5LLseIJbwIxy15D0vb1fxuzvdJdPq+QfLzmFaScHxR7WZ/Br3OoHfbBasWhxMFByX0NZC5SuHTpBzuCMLSB9waV9OHX2MeNOsX/ROoGupk9IEUj0CF1oq8SkN56hapHypfplV2FgG9dGSndk= Authentication-Results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=amd.com; Received: from DM5PR12MB1355.namprd12.prod.outlook.com (2603:10b6:3:6e::7) by DM5PR1201MB0026.namprd12.prod.outlook.com (2603:10b6:4:52::19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4065.25; Mon, 26 Apr 2021 14:21:27 +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; Mon, 26 Apr 2021 14:21:27 +0000 Subject: Re: [edk2-devel] [PATCH 3/3] OvmfPkg/PlatformPei: Mark TPM MMIO range as unencrypted for SEV To: Laszlo Ersek , devel@edk2.groups.io Cc: Joerg Roedel , Borislav Petkov , Ard Biesheuvel , Jordan Justen , Brijesh Singh , James Bottomley , Jiewen Yao , Min Xu References: <1f64ca5689ec86c427e4db8c41da598896dca4ba.1618959281.git.thomas.lendacky@amd.com> <8417023b-b3d6-5268-e92a-0c6f5ebfff6b@redhat.com> <4261e15a-84a0-11a7-2981-9eeb538f6da9@redhat.com> <311cdc78-d818-bea4-16a1-01077bfc1140@amd.com> <21f40236-88f6-5886-1345-5a8b9ac1f732@amd.com> From: "Lendacky, Thomas" Message-ID: <0ad9369d-d863-5bfc-1326-ff4b631bd04e@amd.com> Date: Mon, 26 Apr 2021 09:21:24 -0500 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 In-Reply-To: X-Originating-IP: [67.79.209.213] X-ClientProxiedBy: SN6PR08CA0019.namprd08.prod.outlook.com (2603:10b6:805:66::32) 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 SN6PR08CA0019.namprd08.prod.outlook.com (2603:10b6:805:66::32) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4065.21 via Frontend Transport; Mon, 26 Apr 2021 14:21:25 +0000 X-MS-PublicTrafficType: Email X-MS-Office365-Filtering-Correlation-Id: f72e50ac-a5e5-4eed-2114-08d908be93d5 X-MS-TrafficTypeDiagnostic: DM5PR1201MB0026: X-MS-Exchange-Transport-Forked: True X-Microsoft-Antispam-PRVS: X-MS-Oob-TLC-OOBClassifiers: OLM:9508; X-MS-Exchange-SenderADCheck: 1 X-Microsoft-Antispam: BCL:0; X-Microsoft-Antispam-Message-Info: brSwoHdwf44hVNpOmwO6Hb837jYnlUFrpPEcBOm6cRtyOZRg5MBOccqVOh7E3BZDXUo0LXbyJGA0PiKJUiLue9WWLU04Ftj6ZjZC4WFCc1vP7AV8Idx2rhQLXjj5AsU6Cor4Cje8skFpaEelpMzAekAhCgRgdbPd0Zba/ihWYp7f1Y4dSRMQ59vMJRlUNoWRPTfGJGEFk5/VQAEWs+8ksN8GKpm08XzwR+0AHSYNxsU7fPChw2bWNU/jKnW7CjUb3UJ04ZxO5YpnmuGkHjMqn2phUgSvFV36wicgfZFerQFZ3JhjW7jBCc8XF9LE9gH6/hP4yDrOyULdzGnOYT4aQDHIf+1Vyc7Xs1XrkD0GHAh6GJkKFGKGRvFNVjtycjtiFtxjSWSj11MYOfWzAsRaZ4oM56fhjQ09ZnF5y7hF29BeUXc6XNOQ0y7yBAB+gnQWa7QOzXem53I18+A/mcrryhmtGtwDv2RRwi8PTvJFqvtTprnxkMQhAK9Bs0KC69meM6n0VEJnt/bvGkYKkaMdxQrBt4AVAyjSY5VMkoTGjbLHqqlmKdMhfkbALxGrsD9NYmnVULO0VwfyFaCx3efCbk1mzWX7R6mIW9CpEYzWVEf9BGHncFLt2FO9rCmIm4bQLWkET6IuaSMOrv2ZxNsmHCBV9+nw1hmaVsbIDRCv6ynmkhPU8lWgQu03nfX2eC36QQPPixuJ9jCe7Xu5fJ54BJGwVGNrvL8pMTa92swZEYG8ohgShJOFqJkAtW01t/VeqkkIL7sd20eY+jsH6+Vt4w== 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)(39860400002)(136003)(346002)(376002)(366004)(396003)(83380400001)(4326008)(30864003)(966005)(66946007)(31686004)(956004)(6512007)(478600001)(36756003)(38100700002)(45080400002)(6486002)(2906002)(16526019)(186003)(54906003)(8936002)(53546011)(6506007)(66476007)(66556008)(26005)(5660300002)(2616005)(316002)(31696002)(8676002)(86362001)(43740500002)(45980500001);DIR:OUT;SFP:1101; X-MS-Exchange-AntiSpam-MessageData: =?utf-8?B?cWpodENWY1YwN0RHNDIvUFFQbHVMNXA5bFJDQmp2K3BwVGRzZzNPRTFPWHpW?= =?utf-8?B?cmJMTktyV0t1KytHZHpmeTU1WGtzakRWb0Q3SDlVS2UyMkhBS2QwQ3hZbzRu?= =?utf-8?B?V3ZUSkgrYmVMZ0cvSmRlRFdrWnJuSG01QkpnV2RRVWtTQlV6MG1FYnFTZHkr?= =?utf-8?B?NlV6WWNsaUFSUFdRV2FNM0J3UEVFRHJGQWRwaEZmc2E2bEpYQVZ4eHVTLzFI?= =?utf-8?B?UFE5VnpaVmcyT1k0d3ZVT0daY2dSbE1oNFVIU2NvVEI0VEJnVzdvclREOERE?= =?utf-8?B?S29zV0dzTzRLOFYwaXF5cTl3T2F5OE5RWmlIV0RPbWJEZ0lwZXpVQTFFSDJH?= =?utf-8?B?eWoyUEVma25udlphYmZHZ2pHOFo3ZjU0dk0yTnQxSnhCSGNhR3hmNkVTLys4?= =?utf-8?B?OWNTTmhLMWswMnRPOE5yb1cxb0xNR1BHczdNMURYNlgvSWRlQTB5QlNuUkJp?= =?utf-8?B?RHBGNXNHOEEzOTQ4RzdZcHdJN3BkaXhwMkdUZjBlcnZHb1VwMzlmNE5aSU84?= =?utf-8?B?U1dDTVBaOEFiR01RVkFiTko0K1dqZTRIL1BUM2wzcFFreFBDc2xlekhHMzVJ?= =?utf-8?B?YTRtNDlMUmRHNDhnQklIbUJwWWhQNXg0ODFMOTF5WFpSdHBhSWg3d3dWL055?= =?utf-8?B?b2pXTVRMSm00bzBiaTBJc1Y1a2JmQUJFTVJ1SUY4NVJGRlRBMkw0dENGY3ZZ?= =?utf-8?B?MUl1aW12cVExYitIdmQ2UFdhWGhkL1Y0cjk1UU1CT244dlMxY1ozYlpTL1Ni?= =?utf-8?B?dGEvcDZ3RHZNSUpVQllJMlgvVndEV2VLNXJ3MURPS1JZdHJPck9HdkMxVUxY?= =?utf-8?B?bWl5K0wyQVlhcGNwWm01cUdodXRmVGJybkVEaE11MVFZZzFHYTJ0dTk5K1dL?= =?utf-8?B?M2gvYzVsRlhIM1BzMmdKamk1V3dCLys1YnRGdTRFT01ZU2prenVyb1FBKzNm?= =?utf-8?B?aDI3UTJpVzhYcHFKczVrZllVSUkrZ2pPM2VXYkVaY2JkekVHUkxrVFNCRmZl?= =?utf-8?B?bk5mYTFWd2ZKeVR1cnMwTTNaYTBNd3ZVeDZCU0FEMnRvOGFkVVB1UEVablF0?= =?utf-8?B?TlJnbHV5ZGt1ODlsYWhqazJEaTJmM1I0V0w0eG9zblBKUHNoM0RxcmMrR25q?= =?utf-8?B?UVdYVmJjTjV6c2JKL0xUazUrU04wOVRHQUlOeCs2WXc1UVhoSkEweGtKcHM0?= =?utf-8?B?b2ErVXh1MHVHOUJINlREeGZDdXovTS9RTWt6ODhYM1pCTXpybk5DcnFkWitp?= =?utf-8?B?dURmaGdHWVljN3hrQTNLU2p5RzRGRVlpNzl6R0FKUEM3amNKOXM1TFpIakNp?= =?utf-8?B?dkZUMTNXQUxLa01BRVhWZTcrbHNyUjV3VnZCTVpPNHMwOGxIR2lOSi9udjhU?= =?utf-8?B?M3NZZkxxbTJIbWZ6Tyt0bnJOZTlyRjVTMDRDYTczeEhEU1pDVlFSWC9aekVO?= =?utf-8?B?a1B2R3owNGhjTjNEYmRLNk1namg5YnpkRTV4aXQxZVZuSnBDcDZ6UXU4a3No?= =?utf-8?B?RzF2OGdRcnBiNGFuOVV3Rm44TzFxQ3VsdytORFRsbGp6cFZnY3BqYWVhMTF5?= =?utf-8?B?aDNEeTA4Y2tyeVVhaC9oaWt2MHkrd2dXWm15L2NvUXF0UDBpZXdZR2VkZTZF?= =?utf-8?B?Snp0N2xWOThCQ21yVlk3aytGcnNOMjA1b1M1TENLdFUydzgvYTVPVXIzNDg2?= =?utf-8?B?Q0R3MldnUE00Um9FQlplR1NJNzNtdVZZTW5xeXo0SGpNR0JUeGxSQ24zWUtB?= =?utf-8?Q?VTt3HheU+pZ7DuRq+gmgBwPiOOdtSYefja4WNGo?= X-OriginatorOrg: amd.com X-MS-Exchange-CrossTenant-Network-Message-Id: f72e50ac-a5e5-4eed-2114-08d908be93d5 X-MS-Exchange-CrossTenant-AuthSource: DM5PR12MB1355.namprd12.prod.outlook.com X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Apr 2021 14:21:26.8761 (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: /YPhbA+NmuDSWmFo5WN0gL/B+4qmfsgotPGC3Y5wAc5aqNBWAdoZbR/pZ1xz98+L6WU5oV7BmqUUFGeSYW9ymQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR1201MB0026 Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit On 4/26/21 7:07 AM, Laszlo Ersek wrote: > On 04/23/21 22:02, Tom Lendacky wrote: >> On 4/23/21 12:41 PM, Tom Lendacky wrote: >>> On 4/23/21 8:04 AM, Laszlo Ersek wrote: >>>> On 04/23/21 12:26, Laszlo Ersek wrote: >>>>> review#2 from scratch: >>>>> >>>>> On 04/21/21 00:54, Tom Lendacky wrote: >>>>>> From: Tom Lendacky >>>>>> >>>>>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3345&data=04%7C01%7Cthomas.lendacky%40amd.com%7Cc8a12e7c1e6a4282963508d908abe333%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637550356650588901%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=3SeLVc3SscAozGX62BSAyWseujfwxzm6qIB%2Fh8ALyq0%3D&reserved=0 >>>>>> >>>>>> The TPM support in OVMF performs MMIO accesses during the PEI phase. At >>>>>> this point, MMIO ranges have not been marked un-encyrpted, so an SEV-ES >>>>>> guest will fail attempting to perform MMIO to an encrypted address. >>>>> >>>>> (1) As discussed, please update the commit message, for more clarify >>>>> about SEV vs. SEV-ES. >>>>> >>>>>> >>>>>> Read the PcdTpmBaseAddress and mark the specification defined range >>>>>> (0x5000 in length) as un-encrypted, to allow an SEV-ES guest to process >>>>>> the MMIO requests. >>>>>> >>>>>> Cc: Laszlo Ersek >>>>>> Cc: Ard Biesheuvel >>>>>> Cc: Jordan Justen >>>>>> Cc: Brijesh Singh >>>>>> Cc: James Bottomley >>>>>> Cc: Jiewen Yao >>>>>> Cc: Min Xu >>>>>> Signed-off-by: Tom Lendacky >>>>>> --- >>>>>> OvmfPkg/PlatformPei/PlatformPei.inf | 1 + >>>>>> OvmfPkg/PlatformPei/AmdSev.c | 19 +++++++++++++++++++ >>>>>> 2 files changed, 20 insertions(+) >>>>>> >>>>>> diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf >>>>>> index 6ef77ba7bb21..de60332e9390 100644 >>>>>> --- a/OvmfPkg/PlatformPei/PlatformPei.inf >>>>>> +++ b/OvmfPkg/PlatformPei/PlatformPei.inf >>>>>> @@ -113,6 +113,7 @@ [Pcd] >>>>>> >>>>>> [FixedPcd] >>>>>> gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress >>>>>> + gEfiSecurityPkgTokenSpaceGuid.PcdTpmBaseAddress >>>>>> gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIMemoryNVS >>>>>> gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiACPIReclaimMemory >>>>>> gEmbeddedTokenSpaceGuid.PcdMemoryTypeEfiReservedMemoryType >>>>>> diff --git a/OvmfPkg/PlatformPei/AmdSev.c b/OvmfPkg/PlatformPei/AmdSev.c >>>>>> index dddffdebda4b..d524929f9e10 100644 >>>>>> --- a/OvmfPkg/PlatformPei/AmdSev.c >>>>>> +++ b/OvmfPkg/PlatformPei/AmdSev.c >>>>>> @@ -141,6 +141,7 @@ AmdSevInitialize ( >>>>>> ) >>>>>> { >>>>>> UINT64 EncryptionMask; >>>>>> + UINT64 TpmBaseAddress; >>>>>> RETURN_STATUS PcdStatus; >>>>>> >>>>>> // >>>>>> @@ -206,6 +207,24 @@ AmdSevInitialize ( >>>>>> } >>>>>> } >>>>>> >>>>>> + // >>>>>> + // PEI TPM support will perform MMIO accesses, be sure this range is not >>>>>> + // marked encrypted. >>>>>> + // >>>>>> + TpmBaseAddress = PcdGet64 (PcdTpmBaseAddress); >>>>>> + if (TpmBaseAddress != 0) { >>>>> >>>>> It's OK to keep this as a sanity check, yes. >>>>> >>>>>> + RETURN_STATUS DecryptStatus; >>>>>> + >>>>>> + DecryptStatus = MemEncryptSevClearPageEncMask ( >>>>>> + 0, >>>>>> + TpmBaseAddress, >>>>>> + EFI_SIZE_TO_PAGES (0x5000), >>>>> >>>>> (2) Should be (UINTN)0x5000, as discussed earlier. >>>>> >>>>>> + FALSE >>>>>> + ); >>>>>> + >>>>>> + ASSERT_RETURN_ERROR (DecryptStatus); >>>>> >>>>> (3) So this is where the mess begins. >>>>> >>>>> The idea is to delay the dispatch of Tcg2ConfigPei until after >>>>> PlatformPei determines if SEV is active, and (in case SEV is active) >>>>> PlatformPei decrypts the MMIO range of the TPM. >>>>> >>>>> For this, we need to change the IA32 / X64 DEPEX of Tcg2ConfigPei, from >>>>> the current TRUE, to some PPI GUID. >>>>> >>>>> There are two choices for that PPI: >>>>> >>>>> (a) gEfiPeiMemoryDiscoveredPpiGuid >>>>> >>>>> Advantages: >>>>> >>>>> - no new PPI definition needed, >>>>> >>>>> - no new PPI installation needed, >>>>> >>>>> - OvmfPkg/Bhyve/PlatformPei needs no separate change >>>>> >>>>> Disadvantages: >>>>> >>>>> - total abuse of gEfiPeiMemoryDiscoveredPpiGuid >>>>> >>>>> >>>>> (b) gOvmfTpmMmioAccessiblePpiGuid >>>>> >>>>> Disadvantages: >>>>> >>>>> - this new GUID must be defined in "OvmfPkg.dec", in the [Ppis] section, >>>>> in a separate patch; its comment should say "this PPI signals that >>>>> accessing the MMIO range of the TPM is possible in the PEI phase, >>>>> regardless of memory encryption". The PPI definitions should be kept >>>>> alphabetically ordered. >>>>> >>>>> - OvmfPkg/PlatformPei must install this new PPI, with a NULL interface. >>>>> (See "mPpiBootMode" as a technical example.) OvmfPkg/PlatformPei must >>>>> install this new PPI either when the SEV check at the top of >>>>> AmdSevInitialize() fails, or when MemEncryptSevClearPageEncMask() succeeds. >>>>> >>>>> - OvmfPkg/Bhyve/PlatformPei must receive the same update, in a separate >>>>> patch. That's because "OvmfPkg/Bhyve/BhyveX64.fdf" includes the same >>>>> "Tcg2ConfigPei", but consumes "OvmfPkg/Bhyve/PlatformPei" rather than >>>>> "OvmfPkg/PlatformPei". Tcg2ConfigPei will receive the same >>>>> stricter-than-before depex, so something on the bhyve platform too must >>>>> produce the new PPI. >>>>> >>>>> Advantages: >>>>> >>>>> - more or less palatable as a concept, with the new PPI precisely >>>>> expressing the dependency we have. >>>>> >>>>> >>>>> In approach (b), the "OvmfPkg/Bhyve/PlatformPei" patch needs to be CC'd >>>>> to the Bhyve reviewers. If the Bhyve reviewers determine that such an >>>>> update is actually unnecessary, because on Bhyve, there is no TPM >>>>> support and/or no SEV support in fact, then *first* we have to create an >>>>> independent Bhyve cleanup series, that rips out the TPM and/or SEV >>>>> remnants from the OvmfPkg/Bhyve sub-tree. >>>>> >>>>> >>>>> I prefer approach (b). I'm sorry that it means extra work wrt. Bhyve, >>>>> but I strongly believe in keeping all platforms in the tree, and that >>>>> means we need to spend time on such changes. >>>>> >>>>> I'm not CC'ing Rebecca and Peter on this message -- we're deep into this >>>>> patch review thread, and they'd have no useful context. I suggest simply >>>>> including the "OvmfPkg/Bhyve/PlatformPei" patch in the next version of >>>>> this series, with a proper explanation in the blurb (patch#0) and on the >>>>> "OvmfPkg/Bhyve/PlatformPei" patch. That should give them enough context >>>>> to evaluate whether the change is necessary, or whether we should purge >>>>> the TPM and/or the SEV bits from Bhyve. You could also ask them just >>>>> this question in advance, in a separate email on the list (with >>>>> distilled context). Personally I'm unsure if the TPM and SEV bits >>>>> survived into Bhyve because those bits are actually put to use there, or >>>>> because the initial platform creation / cloning wasn't as minimal as it >>>>> could have been. >>>>> >>>>> Note that in case TPM makes sense on bhyve but SEV doesn't, then >>>>> "OvmfPkg/Bhyve/PlatformPei" will have to install the new PPI >>>>> unconditionally. >>>> >>>> I've had a further idea on this. >>>> >>>> You could add an entirely new PEIM just for this. The entry point >>>> function of the PEIM would check for SEV, decrypt the TPM range if SEV >>>> were active, and then install gOvmfTpmMmioAccessiblePpiGuid >>>> (unconditionally). The exit status of the PEIM would always be >>>> EFI_ABORTED, because there would be no need to keep the PEIM resident. >>>> >>>> The new PEIM would have a DEPEX on gEfiPeiMemoryDiscoveredPpiGuid, to >>>> make sure that potential page table splitting for the potential MMIO >>>> range decryption could be satisfied from permanent PEI RAM. >>>> >>>> The new PEIM would be included in the DSC and FDF files of the usual >>>> three OVMF platforms, and in the Bhyve platform -- dependent on the >>>> TPM_ENABLE build flag. >>>> >>>> There are several advantages to such a separate PEIM: >>>> >>>> - For Bhyve, the update is minimal. Just include one line in each of the >>>> FDF and the DSC files. No need to customize an existent >>>> platform-specific PEIM, no code duplication between two PlatformPei modules. >>>> >>>> - The new PEIM would depend on the TPM_ENABLE build flag, so it would >>>> only be included in the firmware binaries if and only if Tcg2ConfigPei >>>> were. No useless PPI installation would occur in the absence of TPM_ENABLE. >>>> >>>> - No need to check PcdTpmBaseAddress for nullity in the new PEIM, before >>>> the decryption, as TPM_ENABLE guarantees (on IA32/X64) that the PCD >>>> already has the right value. >>>> >>>> - The new logic would be properly ordered between PlatformPei and >>>> Tcg2ConfigPei, namely due to the use of two such PPI GUIDs in DEPEXes >>>> that actually make sense. PlatformPei -> TPM MMIO decryptor PEIM ordered >>>> via "memory discovered" (needed for potential page table splitting), TPM >>>> MMIO decryptor PEIM -> Tcg2ConfigPei ordered via "TPM MMIO decrypted". >>>> >>>> You could place the new PEIM at: >>>> >>>> OvmfPkg/Tcg/TpmMmioSevDecryptPei >>>> >>>> If you haven't lost your patience with me yet, I'd really appreciate if >>>> you could investigate this! >>>> >>> >>> So far, this appears to be working nicely. I'm new at the whole PEIM >>> thing, so hopefully I haven't missed anything. I should be submitting the >>> patches soon for review. >> >> So one thing I failed to do before submitting my previous patch was to >> complete my testing against the IA32 and X64 combination build. In this >> build, PEI is built as Ia32, and MemEncryptSevClearPageEncMask() will >> return UNSUPPORTED causing an ASSERT (since I check the return code). So >> there are a few options: >> >> 1. SEV works with the current encrypted mapping, it is only the SEV-ES >> support that fails because of the ValidateMmioMemory() check. I can do >> the mapping change just for SEV-ES since it is X64 only. This works, >> because MemEncryptSevClearPageEncMask() will not return UNSUPPORTED >> when running in 64-bit. > > Can we really say "SEV works" though? Because, even using an X64 PEI > phase, and enabling only SEV (not SEV-ES), TPM access will be broken in > the PEI phase. Is my understanding correct? Because the memory range is marked as MMIO, we'll take a nested page fault (NPF). The GPA passed as part of the NPF does not include the c-bit. So we do in fact work properly with a TPM in SEV. SEV-ES would also work properly if the mitigation for accessing an encrypted address was removed from the #VC handler. It is only this added mitigation to protect MMIO that results in an issue with the TPM in PEI. > > I think the behavior you currently see is actually what we want, we > should double down on it -- if MemEncryptSevClearPageEncMask() fails, > report an explicit DEBUG_ERROR, and call CpuDeadLoop(). If the firmware > is built with TPM_ENABLE, and SEV is active, then an IA32 PEI phase is > simply unusable. Silently pretending that the TPM is not there, even > though it may have been configured on the QEMU command line, we just > failed to communicate with it, is not a good idea, IMO. However, because the c-bit is not part of the NPF, we do communicate successfully with the TPM. So we could actually do following: - For IA32: - Remove the Depex on gOvmfTpmMmioAccessiblePpiGuid - Do not add OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf - For X64: - Add the Depex on gOvmfTpmMmioAccessiblePpiGuid - Add OvmfPkg/Tcg/TpmMmioSevDecryptPei/TpmMmioSevDecryptPei.inf That might be confusing, though. So we could just do option #3 below. Thanks, Tom > > This is somewhat similar IMO to the S3Verification() function in > "OvmfPkg/PlatformPei/Platform.c". > > TPM_ENABLE, SEV, IA32 PEI phase: pick any two. > > Thanks, > Laszlo > >> >> 2. Call MemEncryptSevClearPageEncMask() for SEV or SEV-ES, but don't check >> the return status. >> >> 3. Create Ia32 and X64 versions of internal functions, where the Ia32 >> version simply returns SUCCESS because it can't do anything and the X64 >> version calls MemEncryptSevClearPageEncMask(), allowing the main code >> to ASSERT on any errors. >> >> I'm leaning towards #1, because this is an SEV-ES only issue. Thoughts? >> >> Thanks, >> Tom >> >>> >>> One thing I found is that the Bhyve package makes reference to the >>> OvmfPkg/Bhyve/Tcg directory, but that directory does not exist. So I don't >>> think that TPM enablement has been tested. I didn't update the Bhyve >>> support for that reason. >>> >>> Thanks, >>> Tom >>> >>>> Thanks! >>>> Laszlo >>>> >> >