From patchwork Thu Aug 17 12:25:46 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ard Biesheuvel X-Patchwork-Id: 110322 Delivered-To: patch@linaro.org Received: by 10.140.95.78 with SMTP id h72csp2120204qge; Thu, 17 Aug 2017 05:25:58 -0700 (PDT) X-Received: by 10.84.232.135 with SMTP id i7mr5560348plk.193.1502972758025; Thu, 17 Aug 2017 05:25:58 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1502972758; cv=none; d=google.com; s=arc-20160816; b=DzEdKo+WpJ4RAqJKdVecBB6V+kvx7zMgBwlW671IN+jyqy+034/R9tr8L/tcwIkEjw J8oQov/VvB23fb4sxQgAQkz7JUpH1Ytipg6Spa6FlBBTfBvmpuC0pkRltqf/RZRgDTcp bS+XUc5GX1MjADxl8O8T9S6CqyNkMgU/zVXiM2k34WrSiM9sZy323UFVp0AKnv4dTjFh fX94GR6emXwYN4TEAeGEFEdzcYH7tncBFWhgWDmgjlDhxNjM1PDQf0Y4EZQHZDOHTaDN 4TpiZKnM/U3SuNF0odVTDEir1He4ibFBFCU+z4v6HdRJlCGitTm8RoE9bbXS8o1EhH4V bYXg== 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=MsKFMLXtIrMaUFGMUo60JceqY+clcIcxcdGNo1nonBw=; b=BRKpoJ8H5t1gIsJRL4H24KqjTaej7YJubUtSaCWynf8iW2nBnuX1CZoUH2uM7oiqwp /FW3X/eBX77V7DxUzObTUjBGrUUvhO75Yp2ongySmGyYxUFDSq4vw8v4HBuFLvr38GBL k7tG+iohFXeBO8jn8EOoLR2BC5ST7XnTR4AWpHgEgVByAnzPMCrYqyZs4Dt5VVTSNEIX 7KXSdGTfyWNQwWigZfb5II7082rlJt3DXfVOAxGkU37f9Sdu9KSa3Qv0nDNMG49rpg7q Kjx4JN0RuaKsD8yLWNBaUu45zuRJmgVHPypwyzSUQY2KhhUPVAvplNPXxln6KYZYQRno CNAw== ARC-Authentication-Results: i=1; mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=AxX/EwlH; spf=pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 2001:19d0:306:5::1 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. [2001:19d0:306:5::1]) by mx.google.com with ESMTPS id p10si2047582pfj.460.2017.08.17.05.25.57 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 17 Aug 2017 05:25:58 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 2001:19d0:306:5::1 as permitted sender) client-ip=2001:19d0:306:5::1; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org header.s=google header.b=AxX/EwlH; spf=pass (google.com: best guess record for domain of edk2-devel-bounces@lists.01.org designates 2001:19d0:306:5::1 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 D1BA421CFA5F2; Thu, 17 Aug 2017 05:23:29 -0700 (PDT) X-Original-To: edk2-devel@lists.01.org Delivered-To: edk2-devel@lists.01.org Received: from mail-wr0-x231.google.com (mail-wr0-x231.google.com [IPv6:2a00:1450:400c:c0c::231]) (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 B9F902095B9D7 for ; Thu, 17 Aug 2017 05:23:27 -0700 (PDT) Received: by mail-wr0-x231.google.com with SMTP id b65so42201404wrd.0 for ; Thu, 17 Aug 2017 05:25:54 -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=/pBDIXWYVvygxMbyf3DRkEZYy5ynHqm8zhoOCL8g8Oc=; b=AxX/EwlHYHxKVwd2TMjP2MjdeVuvZejvUxjqPrQpUpShxo7oaqaMMdDbkWzDbXmc05 I24BTFku35VYJvaGCJV7xrfB1/FmgWTum2YZ+czL/kvj5E42kJc5l7lGy1Wxvey6VSbG ni7Dt92zK6XgJQZrHvfsOaYYm5mvLZLq0/Nig= 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=/pBDIXWYVvygxMbyf3DRkEZYy5ynHqm8zhoOCL8g8Oc=; b=itILBM8t1xkp/dLMkXSdTZAN7j6V+wzsqQc3oxucdsJTH8sC9PLyuFoNzhyz0V9EjZ nX0/05LCi0K4gRGXsVdWrBYK0lz8aviGqHP6oXCJZaRzZdNZns7Bg3SQYvKhuCSp+Thl 2ldpy0Gq8Nf6M2GUdZVHoisd6WVeDLW2sgZQ0yioZ0+GsWWbJNOk7t2niGajvYBByNbY WU9RkbhZOxyIy+DoV3VfZeqy9za0nureKtCR1TcgyRDOMQSE7X/zdsPVw2aefq/usj93 0srTkN03iH5CPHMzP3x2pl981rl8VXOZcFsrTjNmq2hhOUVpzw9oEaY6RIJywXpU+wzf zShQ== X-Gm-Message-State: AHYfb5jr10nXwDpRftz+vw47mQj+hzKy6J5oRjvjbwdi32Sb35P06qrA GgYTrP6xL+dfshOc1JzYzA== X-Received: by 10.28.94.19 with SMTP id s19mr1106250wmb.15.1502972752933; Thu, 17 Aug 2017 05:25:52 -0700 (PDT) Received: from localhost.localdomain ([154.146.161.128]) by smtp.gmail.com with ESMTPSA id 5sm3959480wre.5.2017.08.17.05.25.50 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 17 Aug 2017 05:25:52 -0700 (PDT) From: Ard Biesheuvel To: edk2-devel@lists.01.org, leif.lindholm@linaro.org, lersek@redhat.com Date: Thu, 17 Aug 2017 13:25:46 +0100 Message-Id: <20170817122546.17683-1-ard.biesheuvel@linaro.org> X-Mailer: git-send-email 2.11.0 Subject: [edk2] [PATCH] ArmPkg/ArmDmaLib: use double buffering only for bus master write 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" The ArmPkg implementation of DmaLib uses double buffering to ensure that any attempt to perform non-coherent DMA on unaligned buffers cannot corrupt adjacent unrelated data which happens to share cachelines with the data we are exchanging with the device. Such corruption can only occur on bus master read, in which case we have to invalidate the caches to ensure the CPU will see the data written to memory by the device. In the bus master write case, we can simply clean and invalidate at the same time, which may purge unrelated adjacent data from the caches, but will not corrupt its contents. Also, this double buffer does not necessarily have to be allocated from uncached memory: by the same reasoning, we can perform cache invalidation on an ordinary pool allocation as long as we take the same alignment constraints into account. So update our code accordingly: remove double buffering from the bus master read path, and switch to a pool allocation for the double buffer. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel --- ArmPkg/Library/ArmDmaLib/ArmDmaLib.c | 47 ++++++++++++-------- 1 file changed, 28 insertions(+), 19 deletions(-) -- 2.11.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel Reviewed-by: Leif Lindholm diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c index f4ee9e4c5ea2..61d70614bff0 100644 --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c @@ -80,6 +80,7 @@ DmaMap ( MAP_INFO_INSTANCE *Map; VOID *Buffer; EFI_GCD_MEMORY_SPACE_DESCRIPTOR GcdDescriptor; + UINTN AllocSize; if (HostAddress == NULL || NumberOfBytes == NULL || DeviceAddress == NULL || Mapping == NULL ) { return EFI_INVALID_PARAMETER; @@ -104,8 +105,9 @@ DmaMap ( return EFI_OUT_OF_RESOURCES; } - if ((((UINTN)HostAddress & (mCpu->DmaBufferAlignment - 1)) != 0) || - ((*NumberOfBytes & (mCpu->DmaBufferAlignment - 1)) != 0)) { + if (Operation != MapOperationBusMasterRead && + ((((UINTN)HostAddress & (mCpu->DmaBufferAlignment - 1)) != 0) || + ((*NumberOfBytes & (mCpu->DmaBufferAlignment - 1)) != 0))) { // Get the cacheability of the region Status = gDS->GetMemorySpaceDescriptor ((UINTN)HostAddress, &GcdDescriptor); @@ -129,21 +131,24 @@ DmaMap ( } // - // If the buffer does not fill entire cache lines we must double buffer into - // uncached memory. Device (PCI) address becomes uncached page. + // If the buffer does not fill entire cache lines we must double buffer + // into a suitably aligned allocation that allows us to invalidate the + // cache without running the risk of corrupting adjacent unrelated data. + // Note that pool allocations are guaranteed to be 8 byte aligned, so + // we only have to add (alignment - 8) worth of padding. // - Map->DoubleBuffer = TRUE; - Status = DmaAllocateBuffer (EfiBootServicesData, EFI_SIZE_TO_PAGES (*NumberOfBytes), &Buffer); - if (EFI_ERROR (Status)) { + Map->DoubleBuffer = TRUE; + AllocSize = ALIGN_VALUE (*NumberOfBytes, mCpu->DmaBufferAlignment) + + (mCpu->DmaBufferAlignment - 8); + Map->BufferAddress = AllocatePool (AllocSize); + if (Map->BufferAddress == NULL) { + Status = EFI_OUT_OF_RESOURCES; goto FreeMapInfo; } - if (Operation == MapOperationBusMasterRead) { - CopyMem (Buffer, HostAddress, *NumberOfBytes); - } - + Buffer = ALIGN_POINTER (Map->BufferAddress, mCpu->DmaBufferAlignment); *DeviceAddress = HostToDeviceAddress (ConvertToPhysicalAddress (Buffer)); - Map->BufferAddress = Buffer; + } else { Map->DoubleBuffer = FALSE; } @@ -207,6 +212,7 @@ DmaUnmap ( { MAP_INFO_INSTANCE *Map; EFI_STATUS Status; + VOID *Buffer; if (Mapping == NULL) { ASSERT (FALSE); @@ -217,17 +223,20 @@ DmaUnmap ( Status = EFI_SUCCESS; if (Map->DoubleBuffer) { - ASSERT (Map->Operation != MapOperationBusMasterCommonBuffer); + ASSERT (Map->Operation == MapOperationBusMasterWrite); - if (Map->Operation == MapOperationBusMasterCommonBuffer) { + if (Map->Operation != MapOperationBusMasterWrite) { Status = EFI_INVALID_PARAMETER; - } else if (Map->Operation == MapOperationBusMasterWrite) { - CopyMem ((VOID *)(UINTN)Map->HostAddress, Map->BufferAddress, - Map->NumberOfBytes); - } + } else { + Buffer = ALIGN_POINTER (Map->BufferAddress, mCpu->DmaBufferAlignment); + + mCpu->FlushDataCache (mCpu, (UINTN)Buffer, Map->NumberOfBytes, + EfiCpuFlushTypeInvalidate); - DmaFreeBuffer (EFI_SIZE_TO_PAGES (Map->NumberOfBytes), Map->BufferAddress); + CopyMem ((VOID *)(UINTN)Map->HostAddress, Buffer, Map->NumberOfBytes); + FreePool (Map->BufferAddress); + } } else { if (Map->Operation == MapOperationBusMasterWrite) { //