From patchwork Thu Aug 24 17:23:00 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ard Biesheuvel X-Patchwork-Id: 110937 Delivered-To: patch@linaro.org Received: by 10.37.128.210 with SMTP id c18csp8795348ybm; Thu, 24 Aug 2017 10:23:13 -0700 (PDT) X-Received: by 10.84.128.78 with SMTP id 72mr7671607pla.334.1503595393753; Thu, 24 Aug 2017 10:23:13 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1503595393; cv=none; d=google.com; s=arc-20160816; b=WhbqOUO/tt5PucM3BbcN3Eyt8wG9pycADq5LXYQFAb0RfjMMDk7f7V3mRJg9Nt2cLe izmr1Co/fR/B7iHfzXbQbgcJuuAmjEzNqsgSGU/OKSNNOnPvP+4/SkdQiX+ZTLbNWjte mmybIKxnQkp3kS5wYTiHzYTMF78X2eKnR4PMG/EQJ8RxUAJniIXTxHZ63e8SBfAEfPLt 7aCELwMV2GktpRThiv/Qi4+QelOT0oV7VGp79Am1qG7PZIVjB12tW4NJa3wNfLjpWMTH bnrts5es//s0mV1kxAiek5lHSASRgloj05y7KUjih6PhWmGJkYlz7zQQdYXD9kgs5R2v f82g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=sender:errors-to:content-transfer-encoding:mime-version:cc :list-subscribe:list-help:list-post:list-archive:list-unsubscribe :list-id:precedence:subject:message-id:date:to:from:dkim-signature :delivered-to:arc-authentication-results; bh=nR5h2rUiaTZ7LeW3+Tq1bI0sCNt+fgu1ZLMcd21ZeUI=; b=ulVU6NIeWXJ37fsDBuGLL9d0ouSY8b7Rb5Vsk9EzI1OqwPT0hGq42qQIhZkwkEzj10 AM1pOKkoECjveR3b/+ptllveuYFmgVMm7bwr0uf4OA8gKo+w+fAc299a5MIY7KgNPlfv qkxEzhS9muE3sW2diYtP9/UB57oD/2hEF55qAYJmHFemngiSyZ3V4KYaTHQgs0Xjc9Wm yisQjhbnrNXoYZJ65DIsmviKfC93WurrHLcPBKoYnN+sW+iA8HCdyn7Pv5mxQx0w4fKD FDuU50OFxR5IqyqseGrO7n/w2i7UF1L7IGGCDZIHeef+174o0v+DT7YTF0VZPS9tbrpN nRrA== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=NMZLzA7p; spf=pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 198.145.21.10 as permitted sender) smtp.mailfrom=edk2-devel-bounces@lists.01.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Return-Path: Received: from ml01.01.org (ml01.01.org. [198.145.21.10]) by mx.google.com with ESMTPS id h61si3281573pld.2.2017.08.24.10.23.13 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 24 Aug 2017 10:23:13 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 198.145.21.10 as permitted sender) client-ip=198.145.21.10; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=NMZLzA7p; spf=pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 198.145.21.10 as permitted sender) smtp.mailfrom=edk2-devel-bounces@lists.01.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 05D5D2095AE43; Thu, 24 Aug 2017 10:20:38 -0700 (PDT) X-Original-To: edk2-devel@lists.01.org Delivered-To: edk2-devel@lists.01.org Received: from mail-wm0-x235.google.com (mail-wm0-x235.google.com [IPv6:2a00:1450:400c:c09::235]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id A846D2095AE40 for ; Thu, 24 Aug 2017 10:20:36 -0700 (PDT) Received: by mail-wm0-x235.google.com with SMTP id b189so586133wmd.0 for ; Thu, 24 Aug 2017 10:23:11 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=1raWRdktQlUpe4KheDynuIyBS817xl6MpsFRN0DOHL0=; b=NMZLzA7pE5a1z2Z3H9a6o2MTRhkuP2bgtg71/FJ6m4yvzxj5UcF/Kx+JyIh75umzoo X3ev10UDWp06bLVg6bUBu6uBZQSKZ7tupzdGVhm7lYjdJaRlcqi4AQCMURecheOnXoBZ G7Pkp5Cl5Z/sHnjyUngdrqrFTtuP2De2IMesA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=1raWRdktQlUpe4KheDynuIyBS817xl6MpsFRN0DOHL0=; b=tYtNlWQlL27bUfesHZ0SF069LX7dR3JKVsBF2nBAbFFXqOBfrQJxzNHocFiresBiLW BoR/1eZUcnca/oMroXG0p95Lyx+0JaC8H7DIb5B/mAvqaJ1rP4FYrF6SXmJvdnhWGUWJ Q7YDT/QF4qVPn/g78McRHwJc8nk66Kvz3WiSw/KQWJBpFpjhBpNccaRq66c6NTlX/5FQ FeYzzmGAtQwTrM2HY+d4G9QrbKxqg2jJZ5uyluOwSJZRPxqYIZRz+dwHONITMztJiLVz b+eyBnwF2GfPLmacHHF6Kj+XBcq3+SdbD9zXsOsLqM2Pj9ftzMZ4gj40TweAgjnrGCJq eeRQ== X-Gm-Message-State: AHYfb5i5oma6YGX2fzcNwPE+upmaAcSrJ3fgYGPOQstY+B2PAH39bM7t MV/jBcvgxqUlfhnL+5CwDg== X-Received: by 10.28.105.217 with SMTP id z86mr3806091wmh.22.1503595390098; Thu, 24 Aug 2017 10:23:10 -0700 (PDT) Received: from localhost.localdomain ([196.71.110.206]) by smtp.gmail.com with ESMTPSA id 5sm3939047wre.5.2017.08.24.10.23.08 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 24 Aug 2017 10:23:09 -0700 (PDT) From: Ard Biesheuvel To: edk2-devel@lists.01.org, leif.lindholm@linaro.org Date: Thu, 24 Aug 2017 18:23:00 +0100 Message-Id: <20170824172300.2117-1-ard.biesheuvel@linaro.org> X-Mailer: git-send-email 2.11.0 Subject: [edk2] [PATCH] ArmPkg/ArmDmaLib: remove dependency on UncachedMemoryAllocationLib 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: Ard Biesheuvel MIME-Version: 1.0 Errors-To: edk2-devel-bounces@lists.01.org Sender: "edk2-devel" Now that ArmDmaLib no longer uses uncached mappings for short-lived bounce buffers used for streaming DMA, the only place we allocate uncached memory is in DmaAllocateBuffer (), which is used for static mappings shared between the host and the device, e.g., for packet descriptor rings etc. There is no performance concern around such long lived mappings, and so we can really do without the overhead of UncachedMemoryAllocationLib, which is a sizable chunk of poorly maintained code that never actually releases any memory, and despite the fact that it implements pool based routines, it always performs page based allocations anyway. So let's invoke the DXE services directly to manage memory attributes on allocations, and keep track of the allocations in a linked list so we can restore the attributes and free the memory properly after use. Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Ard Biesheuvel --- ArmPkg/Library/ArmDmaLib/ArmDmaLib.c | 141 ++++++++++++++++---- ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf | 1 - 2 files changed, 112 insertions(+), 30 deletions(-) -- 2.11.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c index 566f77d03f29..e12bda4c2d33 100644 --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c @@ -15,12 +15,12 @@ **/ #include +#include #include #include #include #include #include -#include #include #include @@ -35,16 +35,23 @@ typedef struct { } MAP_INFO_INSTANCE; +typedef struct { + LIST_ENTRY Link; + VOID *HostAddress; + UINTN NumPages; + UINT64 Attributes; +} UNCACHED_ALLOCATION; STATIC EFI_CPU_ARCH_PROTOCOL *mCpu; +STATIC LIST_ENTRY UncachedAllocationList; STATIC PHYSICAL_ADDRESS HostToDeviceAddress ( - IN PHYSICAL_ADDRESS HostAddress + IN VOID *Address ) { - return HostAddress + PcdGet64 (PcdArmDmaDeviceOffset); + return (PHYSICAL_ADDRESS)(UINTN)Address + PcdGet64 (PcdArmDmaDeviceOffset); } /** @@ -91,14 +98,7 @@ DmaMap ( return EFI_INVALID_PARAMETER; } - // - // The debug implementation of UncachedMemoryAllocationLib in ArmPkg returns - // a virtual uncached alias, and unmaps the cached ID mapping of the buffer, - // in order to catch inadvertent references to the cached mapping. - // Since HostToDeviceAddress () expects ID mapped input addresses, convert - // the host address to an ID mapped address first. - // - *DeviceAddress = HostToDeviceAddress (ConvertToPhysicalAddress (HostAddress)); + *DeviceAddress = HostToDeviceAddress (HostAddress); // Remember range so we can flush on the other side Map = AllocatePool (sizeof (MAP_INFO_INSTANCE)); @@ -148,7 +148,7 @@ DmaMap ( } Buffer = ALIGN_POINTER (Map->BufferAddress, mCpu->DmaBufferAlignment); - *DeviceAddress = HostToDeviceAddress (ConvertToPhysicalAddress (Buffer)); + *DeviceAddress = HostToDeviceAddress (Buffer); // // Get rid of any dirty cachelines covering the double buffer. This @@ -270,7 +270,7 @@ DmaUnmap ( @param HostAddress A pointer to store the base system memory address of the allocated range. - @retval EFI_SUCCESS The requested memory pages were allocated. + @retval EFI_SUCCESS The requested memory pages were allocated. @retval EFI_UNSUPPORTED Attributes is unsupported. The only legal attribute bits are MEMORY_WRITE_COMBINE and MEMORY_CACHED. @retval EFI_INVALID_PARAMETER One or more parameters are invalid. @@ -285,21 +285,20 @@ DmaAllocateBuffer ( OUT VOID **HostAddress ) { - VOID *Allocation; + EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor; + VOID *Allocation; + UINT64 MemType; + UNCACHED_ALLOCATION *Alloc; + EFI_STATUS Status; if (HostAddress == NULL) { return EFI_INVALID_PARAMETER; } - // - // The only valid memory types are EfiBootServicesData and EfiRuntimeServicesData - // - // We used uncached memory to keep coherency - // if (MemoryType == EfiBootServicesData) { - Allocation = UncachedAllocatePages (Pages); + Allocation = AllocatePages (Pages); } else if (MemoryType == EfiRuntimeServicesData) { - Allocation = UncachedAllocateRuntimePages (Pages); + Allocation = AllocateRuntimePages (Pages); } else { return EFI_INVALID_PARAMETER; } @@ -308,9 +307,60 @@ DmaAllocateBuffer ( return EFI_OUT_OF_RESOURCES; } + // Get the cacheability of the region + Status = gDS->GetMemorySpaceDescriptor ((UINTN)Allocation, &GcdDescriptor); + if (EFI_ERROR(Status)) { + goto FreeBuffer; + } + + // Choose a suitable uncached memory type that is supported by the region + if (GcdDescriptor.Capabilities & EFI_MEMORY_WC) { + MemType = EFI_MEMORY_WC; + } else if (GcdDescriptor.Capabilities & EFI_MEMORY_UC) { + MemType = EFI_MEMORY_UC; + } else { + Status = EFI_UNSUPPORTED; + goto FreeBuffer; + } + + Alloc = AllocatePool (sizeof *Alloc); + if (Alloc == NULL) { + goto FreeBuffer; + } + + Alloc->HostAddress = Allocation; + Alloc->NumPages = Pages; + Alloc->Attributes = GcdDescriptor.Attributes; + + InsertHeadList (&UncachedAllocationList, &Alloc->Link); + + // Remap the region with the new attributes + Status = gDS->SetMemorySpaceAttributes ((PHYSICAL_ADDRESS)(UINTN)Allocation, + EFI_PAGES_TO_SIZE (Pages), + MemType); + if (EFI_ERROR (Status)) { + goto FreeAlloc; + } + + Status = mCpu->FlushDataCache (mCpu, + (PHYSICAL_ADDRESS)(UINTN)Allocation, + EFI_PAGES_TO_SIZE (Pages), + EfiCpuFlushTypeInvalidate); + if (EFI_ERROR (Status)) { + goto FreeAlloc; + } + *HostAddress = Allocation; return EFI_SUCCESS; + +FreeAlloc: + RemoveEntryList (&Alloc->Link); + FreePool (Alloc); + +FreeBuffer: + FreePages (Allocation, Pages); + return Status; } @@ -332,12 +382,49 @@ DmaFreeBuffer ( IN VOID *HostAddress ) { + LIST_ENTRY *Link; + UNCACHED_ALLOCATION *Alloc; + BOOLEAN Found; + EFI_STATUS Status; + if (HostAddress == NULL) { return EFI_INVALID_PARAMETER; } - UncachedFreePages (HostAddress, Pages); - return EFI_SUCCESS; + for (Link = GetFirstNode (&UncachedAllocationList), Found = FALSE; + !IsNull (&UncachedAllocationList, Link); + Link = GetNextNode (&UncachedAllocationList, Link)) { + + Alloc = BASE_CR (Link, UNCACHED_ALLOCATION, Link); + if (Alloc->HostAddress == HostAddress && Alloc->NumPages == Pages) { + Found = TRUE; + break; + } + } + + if (!Found) { + ASSERT (FALSE); + return EFI_INVALID_PARAMETER; + } + + RemoveEntryList (&Alloc->Link); + + Status = gDS->SetMemorySpaceAttributes ((PHYSICAL_ADDRESS)(UINTN)HostAddress, + EFI_PAGES_TO_SIZE (Pages), + Alloc->Attributes); + if (EFI_ERROR (Status)) { + goto FreeAlloc; + } + + // + // If we fail to restore the original attributes, it is better to leak the + // memory than to return it to the heap + // + FreePages (HostAddress, Pages); + +FreeAlloc: + FreePool (Alloc); + return Status; } @@ -348,12 +435,8 @@ ArmDmaLibConstructor ( IN EFI_SYSTEM_TABLE *SystemTable ) { - EFI_STATUS Status; + InitializeListHead (&UncachedAllocationList); // Get the Cpu protocol for later use - Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&mCpu); - ASSERT_EFI_ERROR(Status); - - return Status; + return gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&mCpu); } - diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf index 9b7dad114b18..e33ed92c9d20 100644 --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.inf @@ -34,7 +34,6 @@ [LibraryClasses] DxeServicesTableLib UefiBootServicesTableLib MemoryAllocationLib - UncachedMemoryAllocationLib IoLib BaseMemoryLib