From patchwork Wed Apr 20 08:40:28 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ard Biesheuvel X-Patchwork-Id: 66183 Delivered-To: patch@linaro.org Received: by 10.140.93.198 with SMTP id d64csp2319441qge; Wed, 20 Apr 2016 01:40:42 -0700 (PDT) X-Received: by 10.66.254.74 with SMTP id ag10mr10536402pad.106.1461141642506; Wed, 20 Apr 2016 01:40:42 -0700 (PDT) Return-Path: Received: from ml01.01.org (ml01.01.org. [198.145.21.10]) by mx.google.com with ESMTPS id xs13si139183pac.140.2016.04.20.01.40.42 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Wed, 20 Apr 2016 01:40:42 -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; 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 dis=NONE) header.from=linaro.org Received: from [127.0.0.1] (localhost [IPv6:::1]) by ml01.01.org (Postfix) with ESMTP id 824131A21A0; Wed, 20 Apr 2016 01:40:41 -0700 (PDT) X-Original-To: edk2-devel@lists.01.org Delivered-To: edk2-devel@lists.01.org Received: from mail-wm0-x233.google.com (mail-wm0-x233.google.com [IPv6:2a00:1450:400c:c09::233]) (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 646951A2182 for ; Wed, 20 Apr 2016 01:40:40 -0700 (PDT) Received: by mail-wm0-x233.google.com with SMTP id u206so68857595wme.1 for ; Wed, 20 Apr 2016 01:40:40 -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=W7fVPdZGn4+8ALDrvxyVxEX2Q29WlrOFXwhuZhODzXA=; b=eIzwgIQp5b/mzm6GmwODjViL5MOpcj4vihY5/ckuqbn2Rl7FppW/pj/7VnUsvn1bVr DEaH8JAzjMtLyNZDsIW0ysKemVn35y+BZFDFSiVXTxWFQZ8WEraMlSByXLoeQJZPReHn yVrME1RwS+aYilEJknvdvEqblPrQYari2IHQ0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=W7fVPdZGn4+8ALDrvxyVxEX2Q29WlrOFXwhuZhODzXA=; b=loh/GuThcYJT2hYTvCr59d6NR3RJUp5MGFRisLKBWUhSnagQxTTwPzdZa9Y15Yl83b yZDYAM8Gr4wRzU51TAk3FRkh1vp/q6hOO9h9YTQHQUIlIKQYIRmek+WuZ0RHs4mpbd7C Ge2qm2kDG6IstlcEF0OiWAl3V2jd9Yek+lN9wZcIlq10PUjutmiVUmm2wMUOMYH4g7tu aVGiahC49NqcFlscoFNhUN3hIbeoUHywRPeCU152OyRbC4Pf/gqWuwvgCdN/Wi5oydby XQiSzL5jR02aSycIQS1Yos503cojG/FOhcc1M7w+zGtJOqgmLLD7kd1jLcdjV4Ssotrz JfrQ== X-Gm-Message-State: AOPr4FXwfvxnUfgVF8f8j9BTdUhVdHZmghv1sCpKP3vOp1qJ6iHcXR+xAPHHK7JbPOEEUYlU X-Received: by 10.28.54.148 with SMTP id y20mr7863499wmh.68.1461141638981; Wed, 20 Apr 2016 01:40:38 -0700 (PDT) Received: from localhost.localdomain ([195.55.142.58]) by smtp.gmail.com with ESMTPSA id r2sm4398582wjm.8.2016.04.20.01.40.37 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Wed, 20 Apr 2016 01:40:38 -0700 (PDT) From: Ard Biesheuvel To: edk2-devel@lists.01.org, leif.lindholm@linaro.org, ryan.harkin@linaro.org Date: Wed, 20 Apr 2016 10:40:28 +0200 Message-Id: <1461141628-20896-1-git-send-email-ard.biesheuvel@linaro.org> X-Mailer: git-send-email 2.5.0 Subject: [edk2] [PATCH] ArmPkg/ArmDmaLib: assert that consistent mappings are uncached X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Cc: heyi.guo@linaro.org, Ard Biesheuvel MIME-Version: 1.0 Errors-To: edk2-devel-bounces@lists.01.org Sender: "edk2-devel" DmaMap () only allows uncached mappings to be used for creating consistent mappings with operation type MapOperationBusMasterCommonBuffer. However, if the buffer passed to DmaMap () happens to be aligned to the CWG, there is no need for a bounce buffer, and we perform the cache maintenance directly without ever checking if the memory attributes of the buffer adhere to the API. So add some debug code that asserts that the operation type and the memory attributes are consistent. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel --- This applies on top of the ArmDmaLib patches that I sent out yesterday. Based on the original code which contained some dodgy looking workarounds, I expect this to actually break something, but Olivier is not around anymore to tell me what he was trying to fix (and the broken 'promote cache mainantenance by VA to set/way above a certain threshold' may well have been the culprit here) Since the PCI emulation for non-coherent devices relies on this DMA code, any non-coherent real platform that uses the SATA or EHCI/UCHI code could be used to test this, but I don't have access to any of those. Suggestions are welcome, as are donations of a Juno or a TC2. Thanks. ArmPkg/Library/ArmDmaLib/ArmDmaLib.c | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) -- 2.5.0 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel Tested-by: Ryan Harkin Reviewed-by: Leif Lindholm diff --git a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c index 83f4d38a8a60..329756efc268 100644 --- a/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c +++ b/ArmPkg/Library/ArmDmaLib/ArmDmaLib.c @@ -135,6 +135,23 @@ DmaMap ( } else { Map->DoubleBuffer = FALSE; + DEBUG_CODE_BEGIN (); + + // + // The operation type check above only executes if the buffer happens to be + // misaligned with respect to CWG, but even if it is aligned, we should not + // allow arbitrary buffers to be used for creating consistent mappings. + // So duplicate the check here when running in DEBUG mode, just to assert + // that we are not trying to create a consistent mapping for cached memory. + // + Status = gDS->GetMemorySpaceDescriptor (*DeviceAddress, &GcdDescriptor); + ASSERT_EFI_ERROR(Status); + + ASSERT (Operation != MapOperationBusMasterCommonBuffer || + (GcdDescriptor.Attributes & (EFI_MEMORY_WB | EFI_MEMORY_WT)) == 0); + + DEBUG_CODE_END (); + // Flush the Data Cache (should not have any effect if the memory region is uncached) gCpu->FlushDataCache (gCpu, *DeviceAddress, *NumberOfBytes, EfiCpuFlushTypeWriteBackInvalidate); }