From nobody Fri Dec 27 17:01:53 2024 Delivered-To: importer@patchew.org Received-SPF: none (zoho.com: 198.145.21.10 is neither permitted nor denied by domain of lists.01.org) client-ip=198.145.21.10; envelope-from=edk2-devel-bounces@lists.01.org; helo=ml01.01.org; Authentication-Results: mx.zohomail.com; spf=none (zoho.com: 198.145.21.10 is neither permitted nor denied by domain of lists.01.org) smtp.mailfrom=edk2-devel-bounces@lists.01.org Return-Path: Received: from ml01.01.org (ml01.01.org [198.145.21.10]) by mx.zohomail.com with SMTPS id 1501709131803508.5576961646615; Wed, 2 Aug 2017 14:25:31 -0700 (PDT) Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id D9E42209589DE; Wed, 2 Aug 2017 14:23:10 -0700 (PDT) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 64C1D209589D0 for ; Wed, 2 Aug 2017 14:23:09 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BA44F5AFC8; Wed, 2 Aug 2017 21:25:19 +0000 (UTC) Received: from lacos-laptop-7.usersys.redhat.com (ovpn-116-47.phx2.redhat.com [10.3.116.47]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6A97617B57; Wed, 2 Aug 2017 21:25:18 +0000 (UTC) X-Original-To: edk2-devel@lists.01.org DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com BA44F5AFC8 Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx10.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com From: Laszlo Ersek To: edk2-devel-01 Date: Wed, 2 Aug 2017 23:24:50 +0200 Message-Id: <20170802212453.19221-10-lersek@redhat.com> In-Reply-To: <20170802212453.19221-1-lersek@redhat.com> References: <20170802212453.19221-1-lersek@redhat.com> X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.39]); Wed, 02 Aug 2017 21:25:19 +0000 (UTC) Subject: [edk2] [PATCH 09/12] OvmfPkg/IoMmuDxe: rework setup of "MapInfo->PlainTextAddress" in Map() X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Jordan Justen , Tom Lendacky , Ard Biesheuvel MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Errors-To: edk2-devel-bounces@lists.01.org Sender: "edk2-devel" X-ZohoMail: RSF_4 Z_629925259 SPT_0 Content-Type: text/plain; charset="utf-8" There are three issues with the current calculations: - The initial logic that sets up "DmaMemoryTop" and "AllocateType" checks for the BusMasterCommonBuffer64 operation in two places. The inner check for BusMasterCommonBuffer64 will never evaluate to TRUE however, because the outer check excludes BusMasterCommonBuffer64. - In order to lower "DmaMemoryTop" to (SIZE_4GB - 1), the outer check requires that the encrypted (original) buffer cross the 4GB mark. This is wrong: for BusMasterRead[64] and BusMasterWrite[64] operations, we unconditionally need a bounce buffer (a decrypted memory area), and for the 32-bit variants, "DmaMemoryTop" should be lowered regardless of the location of the original (encrypted) buffer. - The current logic would be hard to extend for the in-place decryption that we'll implement in the next patch. Therefore rework the "MapInfo->PlainTextAddress" setup. No functional changes beyond said bugfixes. Cc: Ard Biesheuvel Cc: Brijesh Singh Cc: Jordan Justen Cc: Tom Lendacky Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Laszlo Ersek --- OvmfPkg/IoMmuDxe/AmdSevIoMmu.c | 156 +++++++++++--------- 1 file changed, 87 insertions(+), 69 deletions(-) diff --git a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c index d899b0ab9e41..0a85ee6559e7 100644 --- a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c +++ b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c @@ -65,160 +65,178 @@ EFIAPI IoMmuMap ( IN EDKII_IOMMU_PROTOCOL *This, IN EDKII_IOMMU_OPERATION Operation, IN VOID *HostAddress, IN OUT UINTN *NumberOfBytes, OUT EFI_PHYSICAL_ADDRESS *DeviceAddress, OUT VOID **Mapping ) { EFI_STATUS Status; - EFI_PHYSICAL_ADDRESS PhysicalAddress; MAP_INFO *MapInfo; - EFI_PHYSICAL_ADDRESS DmaMemoryTop; EFI_ALLOCATE_TYPE AllocateType; =20 if (HostAddress =3D=3D NULL || NumberOfBytes =3D=3D NULL || DeviceAddres= s =3D=3D NULL || Mapping =3D=3D NULL) { return EFI_INVALID_PARAMETER; } =20 // - // Make sure that Operation is valid - // - if ((UINT32) Operation >=3D EdkiiIoMmuOperationMaximum) { - return EFI_INVALID_PARAMETER; - } - PhysicalAddress =3D (EFI_PHYSICAL_ADDRESS) (UINTN) HostAddress; - - DmaMemoryTop =3D (UINTN)-1; - AllocateType =3D AllocateAnyPages; - - if (((Operation !=3D EdkiiIoMmuOperationBusMasterRead64 && - Operation !=3D EdkiiIoMmuOperationBusMasterWrite64 && - Operation !=3D EdkiiIoMmuOperationBusMasterCommonBuffer64)) && - ((PhysicalAddress + *NumberOfBytes) > SIZE_4GB)) { - // - // If the root bridge or the device cannot handle performing DMA above - // 4GB but any part of the DMA transfer being mapped is above 4GB, then - // map the DMA transfer to a buffer below 4GB. - // - DmaMemoryTop =3D SIZE_4GB - 1; - AllocateType =3D AllocateMaxAddress; - - if (Operation =3D=3D EdkiiIoMmuOperationBusMasterCommonBuffer || - Operation =3D=3D EdkiiIoMmuOperationBusMasterCommonBuffer64) { - // - // Common Buffer operations can not be remapped. If the common bu= ffer - // if above 4GB, then it is not possible to generate a mapping, so - // return an error. - // - return EFI_UNSUPPORTED; - } - } - - // - // CommandBuffer was allocated by us (AllocateBuffer) and is already in - // unencryted buffer so no need to create bounce buffer - // - if (Operation =3D=3D EdkiiIoMmuOperationBusMasterCommonBuffer || - Operation =3D=3D EdkiiIoMmuOperationBusMasterCommonBuffer64) { - *Mapping =3D NO_MAPPING; - *DeviceAddress =3D PhysicalAddress; - - return EFI_SUCCESS; - } - - // // Allocate a MAP_INFO structure to remember the mapping when Unmap() is // called later. // MapInfo =3D AllocatePool (sizeof (MAP_INFO)); if (MapInfo =3D=3D NULL) { - *NumberOfBytes =3D 0; - return EFI_OUT_OF_RESOURCES; + Status =3D EFI_OUT_OF_RESOURCES; + goto Failed; } =20 // - // Initialize the MAP_INFO structure + // Initialize the MAP_INFO structure, except the PlainTextAddress field // MapInfo->Operation =3D Operation; MapInfo->NumberOfBytes =3D *NumberOfBytes; MapInfo->NumberOfPages =3D EFI_SIZE_TO_PAGES (MapInfo->NumberOfBytes= ); - MapInfo->CryptedAddress =3D PhysicalAddress; - MapInfo->PlainTextAddress =3D DmaMemoryTop; + MapInfo->CryptedAddress =3D (UINTN)HostAddress; =20 // - // Allocate a buffer to map the transfer to. + // In the switch statement below, we point "MapInfo->PlainTextAddress" t= o the + // plaintext buffer, according to Operation. // - Status =3D gBS->AllocatePages ( - AllocateType, - EfiBootServicesData, - MapInfo->NumberOfPages, - &MapInfo->PlainTextAddress - ); - if (EFI_ERROR (Status)) { + MapInfo->PlainTextAddress =3D MAX_ADDRESS; + AllocateType =3D AllocateAnyPages; + switch (Operation) { + // + // For BusMasterRead[64] and BusMasterWrite[64] operations, a bounce buf= fer + // is necessary regardless of whether the original (crypted) buffer cros= ses + // the 4GB limit or not -- we have to allocate a separate plaintext buff= er. + // The only variable is whether the plaintext buffer should be under 4GB. + // + case EdkiiIoMmuOperationBusMasterRead: + case EdkiiIoMmuOperationBusMasterWrite: + MapInfo->PlainTextAddress =3D BASE_4GB - 1; + AllocateType =3D AllocateMaxAddress; + // + // fall through + // + case EdkiiIoMmuOperationBusMasterRead64: + case EdkiiIoMmuOperationBusMasterWrite64: + // + // Allocate the implicit plaintext bounce buffer. + // + Status =3D gBS->AllocatePages ( + AllocateType, + EfiBootServicesData, + MapInfo->NumberOfPages, + &MapInfo->PlainTextAddress + ); + if (EFI_ERROR (Status)) { + goto FreeMapInfo; + } + break; + + // + // For BusMasterCommonBuffer[64] operations, a plaintext buffer has been + // allocated already, with AllocateBuffer(). We only check whether the + // address is low enough for the requested operation. + // + case EdkiiIoMmuOperationBusMasterCommonBuffer: + if ((MapInfo->CryptedAddress > BASE_4GB) || + (EFI_PAGES_TO_SIZE (MapInfo->NumberOfPages) > + BASE_4GB - MapInfo->CryptedAddress)) { + // + // CommonBuffer operations cannot be remapped. If the common buffer = is + // above 4GB, then it is not possible to generate a mapping, so retu= rn an + // error. + // + Status =3D EFI_UNSUPPORTED; + goto FreeMapInfo; + } + // + // fall through + // + case EdkiiIoMmuOperationBusMasterCommonBuffer64: + // + // The buffer at MapInfo->CryptedAddress comes from AllocateBuffer(), + // and it is already decrypted. + // + MapInfo->PlainTextAddress =3D MapInfo->CryptedAddress; + + // + // Therefore no mapping is necessary. + // + *DeviceAddress =3D MapInfo->PlainTextAddress; + *Mapping =3D NO_MAPPING; FreePool (MapInfo); - *NumberOfBytes =3D 0; - return Status; + return EFI_SUCCESS; + + default: + // + // Operation is invalid + // + Status =3D EFI_INVALID_PARAMETER; + goto FreeMapInfo; } =20 // - // Clear the memory encryption mask from the device buffer + // Clear the memory encryption mask on the plaintext buffer. // Status =3D MemEncryptSevClearPageEncMask ( 0, MapInfo->PlainTextAddress, MapInfo->NumberOfPages, TRUE ); ASSERT_EFI_ERROR(Status); =20 // // If this is a read operation from the Bus Master's point of view, // then copy the contents of the real buffer into the mapped buffer // so the Bus Master can read the contents of the real buffer. // if (Operation =3D=3D EdkiiIoMmuOperationBusMasterRead || Operation =3D=3D EdkiiIoMmuOperationBusMasterRead64) { CopyMem ( (VOID *) (UINTN) MapInfo->PlainTextAddress, (VOID *) (UINTN) MapInfo->CryptedAddress, MapInfo->NumberOfBytes ); } =20 // - // The DeviceAddress is the address of the maped buffer below 4GB + // Populate output parameters. // *DeviceAddress =3D MapInfo->PlainTextAddress; - - // - // Return a pointer to the MAP_INFO structure in Mapping - // *Mapping =3D MapInfo; =20 DEBUG (( DEBUG_VERBOSE, "%a PlainText 0x%Lx Crypted 0x%Lx Pages 0x%Lx Bytes 0x%Lx\n", __FUNCTION__, MapInfo->PlainTextAddress, MapInfo->CryptedAddress, (UINT64)MapInfo->NumberOfPages, (UINT64)MapInfo->NumberOfBytes )); =20 return EFI_SUCCESS; + +FreeMapInfo: + FreePool (MapInfo); + +Failed: + *NumberOfBytes =3D 0; + return Status; } =20 /** Completes the Map() operation and releases any corresponding resources. =20 @param This The protocol instance pointer. @param Mapping The mapping value returned from Map(). =20 @retval EFI_SUCCESS The range was unmapped. @retval EFI_INVALID_PARAMETER Mapping is not a value that was returned by Map(). @retval EFI_DEVICE_ERROR The data was not committed to the target s= ystem memory. **/ --=20 2.13.1.3.g8be5a757fa67 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel