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.web12.9914.1609947504411352994 for ; Wed, 06 Jan 2021 07:38:24 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b=JBYW7Uy0; 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=1609947503; 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=cFWg/7eRKSWgPlMfzPFeSpq6n2d7+Kt4VqvWNm5zWrA=; b=JBYW7Uy0++66/KidcOxU3lpPj9we4PsCsCe+/gGlKKSuL/y2B0peKiVhTtXI+L6jZ96CvU djNzXxzyVf1FXIE39KHU+1Wr3jd+f1ir1G2sPFtNUHx0JiMpHRfMkliYenHPX57IPG++p4 rRSl3eaQC0Jeb8QpdNwHWZBxtpcE6dI= 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-466-T1AL02NLPjyaEn1cWziSVg-1; Wed, 06 Jan 2021 10:38:19 -0500 X-MC-Unique: T1AL02NLPjyaEn1cWziSVg-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 9BA3F107ACE4; Wed, 6 Jan 2021 15:38:18 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-113-198.ams2.redhat.com [10.36.113.198]) by smtp.corp.redhat.com (Postfix) with ESMTP id 2500E100AE2D; Wed, 6 Jan 2021 15:38:16 +0000 (UTC) Subject: Re: [edk2-devel] [PATCH 06/12] OvmfPkg/AmdSevDxe: Clear encryption bit on PCIe MMCONFIG range To: Tom Lendacky , devel@edk2.groups.io Cc: Brijesh Singh , James Bottomley , Jordan Justen , Ard Biesheuvel References: <90152d0505354d270cc3af9e5838010e4dcbe114.1608065471.git.thomas.lendacky@amd.com> <5daddb82-ad8e-7de7-49df-f3d18907bfe1@redhat.com> <946164e0-38f4-aff6-b2dc-3f2348c0d97d@amd.com> From: "Laszlo Ersek" Message-ID: Date: Wed, 6 Jan 2021 16:38:16 +0100 MIME-Version: 1.0 In-Reply-To: <946164e0-38f4-aff6-b2dc-3f2348c0d97d@amd.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 01/05/21 23:48, Tom Lendacky wrote: > On 1/4/21 3:04 PM, Laszlo Ersek wrote: >> On 12/15/20 21:51, Lendacky, Thomas wrote: >>> From: Tom Lendacky >>> >>> BZ: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.tianocore.org%2Fshow_bug.cgi%3Fid%3D3108&data=04%7C01%7Cthomas.lendacky%40amd.com%7Cf35ac4fb20264b713aa108d8b0f45717%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637453910773208310%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=wZg2UIdJ%2FZ2HLGbWcfli3SVzl1cSMkyI%2FvREVldOB9M%3D&reserved=0 >>> >>> The PCIe MMCONFIG range should be treated as an MMIO range. However, >>> there is a comment in the code explaining why AddIoMemoryBaseSizeHob() >>> is not called. The AmdSevDxe walks the GCD map looking for MemoryMappedIo >>> or NonExistent type memory and will clear the encryption bit for these >>> ranges. >>> >>> Since the MMCONFIG range does not have one of these types, the encryption >>> bit is not cleared for this range. Add support to detect the presence of >>> the MMCONFIG range and clear the encryption bit. This will be needed for >>> follow-on support that will validate MMIO under SEV-ES. >>> >>> Cc: Jordan Justen >>> Cc: Laszlo Ersek >>> Cc: Ard Biesheuvel >>> Cc: Brijesh Singh >>> Signed-off-by: Tom Lendacky >>> --- >>> OvmfPkg/AmdSevDxe/AmdSevDxe.inf | 8 +++++++- >>> OvmfPkg/AmdSevDxe/AmdSevDxe.c | 20 +++++++++++++++++++- >>> 2 files changed, 26 insertions(+), 2 deletions(-) >>> >>> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf >>> index dd9ecc789a20..0676fcc5b6a4 100644 >>> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.inf >>> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.inf >>> @@ -2,7 +2,7 @@ >>> # >>> # Driver clears the encryption attribute from MMIO regions when SEV is enabled >>> # >>> -# Copyright (c) 2017, AMD Inc. All rights reserved.
>>> +# Copyright (c) 2017 - 2020, AMD Inc. All rights reserved.
>>> # >>> # SPDX-License-Identifier: BSD-2-Clause-Patent >>> # >>> @@ -39,3 +39,9 @@ [Depex] >>> >>> [FeaturePcd] >>> gUefiOvmfPkgTokenSpaceGuid.PcdSmmSmramRequire >>> + >>> +[FixedPcd] >>> + gEfiMdePkgTokenSpaceGuid.PcdPciExpressBaseAddress >>> + >>> +[Pcd] >>> + gUefiOvmfPkgTokenSpaceGuid.PcdOvmfHostBridgePciDevId >>> diff --git a/OvmfPkg/AmdSevDxe/AmdSevDxe.c b/OvmfPkg/AmdSevDxe/AmdSevDxe.c >>> index 595586617882..ed516fcdf956 100644 >>> --- a/OvmfPkg/AmdSevDxe/AmdSevDxe.c >>> +++ b/OvmfPkg/AmdSevDxe/AmdSevDxe.c >>> @@ -4,7 +4,7 @@ >>> in APRIORI. It clears C-bit from MMIO and NonExistent Memory space when SEV >>> is enabled. >>> >>> - Copyright (c) 2017, AMD Inc. All rights reserved.
>>> + Copyright (c) 2017 - 2020, AMD Inc. All rights reserved.
>>> >>> SPDX-License-Identifier: BSD-2-Clause-Patent >>> >>> @@ -17,6 +17,7 @@ >>> #include >>> #include >>> #include >>> +#include >> >> (1) Please keep the #include list alphabetically sorted. > > Will fix. > >> >>> >>> EFI_STATUS >>> EFIAPI >>> @@ -65,6 +66,23 @@ AmdSevDxeEntryPoint ( >>> FreePool (AllDescMap); >>> } >>> >>> + // >>> + // If PCI Express is enabled, the MMCONFIG area has been reserved, rather >>> + // than marked as MMIO, and so the C-bit won't be cleared by the above walk >>> + // through the GCD map. Check for the MMCONFIG area and clear the C-bit for >>> + // the range. >>> + // >>> + if (PcdGet16 (PcdOvmfHostBridgePciDevId) == INTEL_Q35_MCH_DEVICE_ID) { >>> + Status = MemEncryptSevClearPageEncMask ( >>> + 0, >>> + FixedPcdGet64 (PcdPciExpressBaseAddress), >>> + EFI_SIZE_TO_PAGES (SIZE_256MB), >>> + FALSE >>> + ); >>> + >>> + ASSERT_EFI_ERROR (Status); >>> + } >>> + >>> // >>> // When SMM is enabled, clear the C-bit from SMM Saved State Area >>> // >>> >> >> Very interesting. One wonders why, without this change, MMCONFIG >> accesses work at all on SEV. >> >> But then... this guest phys area is not backed by RAM in the first >> place. Whenever the guest accesses it, we trap to QEMU unconditionally. >> And so memory encryption plays no role in practice, I must think. >> >> It's different for the flash, because the flash is backed by RAM, and >> whether an access to it traps to QEMU or not depends on both the access >> (r/w/x) and the mode the flash is in (programming mode on vs. off). >> >> I now wonder whether the comment in the leading context (not visible >> above), namely the one that references the root bridge MMIO aperture, >> from which the PCI MMIO BARs are allocated, is accurate. Perhaps that >> area would work in fact even if we didn't clear the C bit for them >> (considering just the accesses themselves under SEV; not SEV-ES). >> >> (2) Please include a sentence in the commit message about the fact that >> MMCONFIG is not backed by a KVM memory slot, and so actual memory >> encryption does not take place, and that's why MMCONFIG accesses do not >> break currently under SEV / SEV-ES. (This is at least what I think happens.) > > Since that address range is marked as MMIO in the nested page tables by > KVM (reserved bits set), accessing that address range will always trigger > a nested page fault (NPF). > > For SEV, the hardware clears the encryption bit from the GPA provided for > the NPF, so KVM/Qemu see the base address and everything just works. > > For SEV-ES, the NPF triggers a #VC. Since we run identity mapped (VA == > PA), I use the virtual address in the VMGEXIT, which doesn't contain the > encryption bit, so, again, everything just works. The SEV-ES check for the > encryption bit being set is what uncovered this condition. > > I'll write that up in the commit. Thank you! Laszlo