From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: 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 6AB9321D492FD for ; Thu, 7 Sep 2017 15:38:43 -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 93C7E806A1; Thu, 7 Sep 2017 22:41:34 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 93C7E806A1 Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx02.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=lersek@redhat.com Received: from lacos-laptop-7.usersys.redhat.com (ovpn-120-54.rdu2.redhat.com [10.10.120.54]) by smtp.corp.redhat.com (Postfix) with ESMTP id 5006C5D967; Thu, 7 Sep 2017 22:41:33 +0000 (UTC) From: Laszlo Ersek To: edk2-devel-01 Cc: Ard Biesheuvel , Brijesh Singh , Jiewen Yao , Jordan Justen Date: Fri, 8 Sep 2017 00:41:14 +0200 Message-Id: <20170907224116.895-9-lersek@redhat.com> In-Reply-To: <20170907224116.895-1-lersek@redhat.com> References: <20170907224116.895-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.26]); Thu, 07 Sep 2017 22:41:34 +0000 (UTC) Subject: [PATCH 08/10] OvmfPkg/IoMmuDxe: track all mappings 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: , X-List-Received-Date: Thu, 07 Sep 2017 22:38:43 -0000 The "mRecycledMapInfos" list implements an internal pool of unused MAP_INFO structures between the IoMmuUnmap() and IoMmuMap() functions. The original goal was to allow IoMmuUnmap() to tear down CommonBuffer mappings without releasing any memory: IoMmuUnmap() would recycle the MAP_INFO structure to the list, and IoMmuMap() would always check the list first, before allocating a brand new MAP_INFO structure. In one of the following patches, we'll change OvmfPkg/IoMmuDxe so that it unmaps all existent bus master operations (CommonBuffer, Read, Write) at ExitBootServices(), strictly after the individual device drivers abort pending DMA on the devices they manage, in their own ExitBootServices() notification functions. For this, rename and repurpose the list to track all live mappings. This means that IoMmuUnmap() will always release a MAP_INFO structure (even when cleaning up a CommonBuffer operation). That's fine (for now), because device drivers are no longer expected to call Unmap() in their ExitBootServices() notification functions. In theory, we could also move the allocation and freeing of the stash buffer from IoMmuAllocateBuffer() and IoMmuFreeBuffer(), respectively, to IoMmuMap() and IoMmuUnmap(). However, this would require allocating and freeing a stash buffer in *both* IoMmuMap() and IoMmuUnmap(), as IoMmuMap() performs in-place decryption for CommonBuffer operations, and IoMmuUnmap() performs in-place encryption for the same. By keeping the stash buffer allocation as-is, not only do we keep the code almost fully undisturbed, but - we also continue to guarantee that IoMmuUnmap() succeeds: allocating a stash buffer in IoMmuUnmap(), for in-place encryption after a CommonBuffer operation, could fail; - we also keep IoMmuUnmap() largely reusable for ExitBootServices() callback context: allocating a stash buffer in IoMmuUnmap() would simply be forbidden in that context. Cc: Ard Biesheuvel Cc: Brijesh Singh Cc: Jiewen Yao Cc: Jordan Justen Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Laszlo Ersek --- OvmfPkg/IoMmuDxe/AmdSevIoMmu.c | 49 +++++++------------- 1 file changed, 18 insertions(+), 31 deletions(-) diff --git a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c index bc57de5b572b..c86e73498555 100644 --- a/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c +++ b/OvmfPkg/IoMmuDxe/AmdSevIoMmu.c @@ -31,18 +31,15 @@ typedef struct { EFI_PHYSICAL_ADDRESS CryptedAddress; EFI_PHYSICAL_ADDRESS PlainTextAddress; } MAP_INFO; // -// List of MAP_INFO structures recycled by Unmap(). +// List of the MAP_INFO structures that have been set up by IoMmuMap() and not +// yet torn down by IoMmuUnmap(). The list represents the full set of mappings +// currently in effect. // -// Recycled MAP_INFO structures are equally good for future recycling and -// freeing. -// -STATIC LIST_ENTRY mRecycledMapInfos = INITIALIZE_LIST_HEAD_VARIABLE ( - mRecycledMapInfos - ); +STATIC LIST_ENTRY mMapInfos = INITIALIZE_LIST_HEAD_VARIABLE (mMapInfos); #define COMMON_BUFFER_SIG SIGNATURE_64 ('C', 'M', 'N', 'B', 'U', 'F', 'F', 'R') // // ASCII names for EDKII_IOMMU_OPERATION constants, for debug logging. @@ -121,11 +118,10 @@ IoMmuMap ( OUT EFI_PHYSICAL_ADDRESS *DeviceAddress, OUT VOID **Mapping ) { EFI_STATUS Status; - LIST_ENTRY *RecycledMapInfo; MAP_INFO *MapInfo; EFI_ALLOCATE_TYPE AllocateType; COMMON_BUFFER_HEADER *CommonBufferHeader; VOID *DecryptionSource; @@ -148,23 +144,14 @@ IoMmuMap ( // // Allocate a MAP_INFO structure to remember the mapping when Unmap() is // called later. // - RecycledMapInfo = GetFirstNode (&mRecycledMapInfos); - if (RecycledMapInfo == &mRecycledMapInfos) { - // - // No recycled MAP_INFO structure, allocate a new one. - // - MapInfo = AllocatePool (sizeof (MAP_INFO)); - if (MapInfo == NULL) { - Status = EFI_OUT_OF_RESOURCES; - goto Failed; - } - } else { - MapInfo = CR (RecycledMapInfo, MAP_INFO, Link, MAP_INFO_SIG); - RemoveEntryList (RecycledMapInfo); + MapInfo = AllocatePool (sizeof (MAP_INFO)); + if (MapInfo == NULL) { + Status = EFI_OUT_OF_RESOURCES; + goto Failed; } // // Initialize the MAP_INFO structure, except the PlainTextAddress field // @@ -296,10 +283,14 @@ IoMmuMap ( DecryptionSource, MapInfo->NumberOfBytes ); } + // + // Track all MAP_INFO structures. + // + InsertHeadList (&mMapInfos, &MapInfo->Link); // // Populate output parameters. // *DeviceAddress = MapInfo->PlainTextAddress; *Mapping = MapInfo; @@ -428,28 +419,24 @@ IoMmuUnmap ( CopyMem ( (VOID *)(UINTN)MapInfo->CryptedAddress, CommonBufferHeader->StashBuffer, MapInfo->NumberOfBytes ); - - // - // Recycle the MAP_INFO structure. - // - InsertTailList (&mRecycledMapInfos, &MapInfo->Link); } else { ZeroMem ( (VOID *)(UINTN)MapInfo->PlainTextAddress, EFI_PAGES_TO_SIZE (MapInfo->NumberOfPages) ); gBS->FreePages (MapInfo->PlainTextAddress, MapInfo->NumberOfPages); - - // - // Free the MAP_INFO structure. - // - FreePool (MapInfo); } + // + // Forget and free the MAP_INFO structure. + // + RemoveEntryList (&MapInfo->Link); + FreePool (MapInfo); + return EFI_SUCCESS; } /** Allocates pages that are suitable for an OperationBusMasterCommonBuffer or -- 2.14.1.3.gb7cf6e02401b