From patchwork Thu Sep 8 17:32:05 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ard Biesheuvel X-Patchwork-Id: 75812 Delivered-To: patch@linaro.org Received: by 10.140.106.11 with SMTP id d11csp965523qgf; Thu, 8 Sep 2016 10:32:14 -0700 (PDT) X-Received: by 10.98.39.193 with SMTP id n184mr1407587pfn.164.1473355934692; Thu, 08 Sep 2016 10:32:14 -0700 (PDT) Return-Path: Received: from ml01.01.org (ml01.01.org. [198.145.21.10]) by mx.google.com with ESMTPS id rx9si47887875pab.57.2016.09.08.10.32.14 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 08 Sep 2016 10:32:14 -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 1B6111A1E55; Thu, 8 Sep 2016 10:32:14 -0700 (PDT) X-Original-To: edk2-devel@lists.01.org Delivered-To: edk2-devel@lists.01.org Received: from mail-wm0-x22a.google.com (mail-wm0-x22a.google.com [IPv6:2a00:1450:400c:c09::22a]) (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 9CC731A1E4C for ; Thu, 8 Sep 2016 10:32:12 -0700 (PDT) Received: by mail-wm0-x22a.google.com with SMTP id w12so48448267wmf.0 for ; Thu, 08 Sep 2016 10:32:12 -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=7Y0q9B3OauCgb2sReePaYie0yihFJSjnZVy7C0kDxcA=; b=P2AFQcAogVuaPHLmO0+nl7JLj65talfzUB8/vBTaEjgzPK/Pz70WRu+Dd7LVDH2twW bswmwNwGkt4MqPA/bGYtByNTd5iZJt/pmRZNVOGKGzTXY9XDb7t5IHBhXS14KLkaKtQW hOAX6DQeJ0mM4Lyh7zR8OmfzrD9cjb6Gn6Y6Y= 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=7Y0q9B3OauCgb2sReePaYie0yihFJSjnZVy7C0kDxcA=; b=TOhTvRA3UHUbc3g58uoTI+NyW85Sy1xDkr4t9LXS/zMzZ4osCXsZjQOr//k1joMH4S Mko2T/jPkjY+W5/CjTvdhcGgSzpZpRjKCziYVAXhKTSpogqMiD4Q2HSv0HpJLlKu/vIC 0rwUwm+Tw5bjLApbhUw8GY4eMqBYOYNkgcBeRlGknz16DViwll4BOLBP2Ix3Lr/IdbJh pyuQ3snUSNG/ahJFVCfczGDbAQz6CMKsjMCIn2it/A1/tjttYzW2CVVBZ6amMxkfY5zm ULySDXsdAwZdi6ArWZuiHSZE8yfesRjcFu55NrdEKd1XbE5liZ1vSb3SegIv4SGBC203 GhDw== X-Gm-Message-State: AE9vXwNDuxBUjGSA4IIAFA2vxTXQfsHEaAudzo2BN0IwEDGe3eqoEYw3qpphwiZ02OzArbMJ X-Received: by 10.194.206.68 with SMTP id lm4mr868928wjc.106.1473355930680; Thu, 08 Sep 2016 10:32:10 -0700 (PDT) Received: from localhost.localdomain ([197.130.133.164]) by smtp.gmail.com with ESMTPSA id ub8sm45442627wjc.39.2016.09.08.10.32.08 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 08 Sep 2016 10:32:09 -0700 (PDT) From: Ard Biesheuvel To: edk2-devel@lists.01.org, leif.lindholm@linaro.org Date: Thu, 8 Sep 2016 18:32:05 +0100 Message-Id: <1473355925-26678-1-git-send-email-ard.biesheuvel@linaro.org> X-Mailer: git-send-email 2.7.4 Subject: [edk2] [PATCH] ArmPlatformPkg/NorFlashDxe: use strictly aligned CopyMem() X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.21 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 UEFI spec stipulates that unaligned accesses should be enabled on CPUs that support them, which means all of them, given that we no longer support pre-v7 ARM cores, and the AARCH64 bindings mandate support for unaligned accesses unconditionally. This means that one should not assume that CopyMem () is safe to call on regions that may be mapped using device attributes, which is the case for the NOR flash. Since we have no control over the mappings when running under the OS, and given that write accesses require device mappings, we should not call CopyMem () in the read path either, but use our own implementation that is guaranteed to take alignment into account. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel --- ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 130 +++++++++++++++++++- 1 file changed, 128 insertions(+), 2 deletions(-) -- 2.7.4 _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c index 3abbe5cb32bc..45d1f0c20d52 100644 --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c @@ -744,6 +744,132 @@ NorFlashWriteBlocks ( return Status; } +/** + Copy Length bytes from Source to Destination, using aligned accesses only + + @param DestinationBuffer The target of the copy request. + @param SourceBuffer The place to copy from. + @param Length The number of bytes to copy. + + @return Destination + +**/ +STATIC +VOID * +AlignedCopyMem ( + OUT VOID *DestinationBuffer, + IN CONST VOID *SourceBuffer, + IN UINTN Length + ) +{ + UINT8 *Destination8; + CONST UINT8 *Source8; + UINT32 *Destination32; + CONST UINT32 *Source32; + UINT64 *Destination64; + CONST UINT64 *Source64; + UINTN Alignment; + + if ((((UINTN)DestinationBuffer & 0x7) == 0) && (((UINTN)SourceBuffer & 0x7) == 0) && (Length >= 8)) { + if (SourceBuffer > DestinationBuffer) { + Destination64 = (UINT64*)DestinationBuffer; + Source64 = (CONST UINT64*)SourceBuffer; + while (Length >= 8) { + *(Destination64++) = *(Source64++); + Length -= 8; + } + + // Finish if there are still some bytes to copy + Destination8 = (UINT8*)Destination64; + Source8 = (CONST UINT8*)Source64; + while (Length-- != 0) { + *(Destination8++) = *(Source8++); + } + } else if (SourceBuffer < DestinationBuffer) { + Destination64 = (UINT64*)((UINTN)DestinationBuffer + Length); + Source64 = (CONST UINT64*)((UINTN)SourceBuffer + Length); + + // Destination64 and Source64 were aligned on a 64-bit boundary + // but if length is not a multiple of 8 bytes then they won't be + // anymore. + + Alignment = Length & 0x7; + if (Alignment != 0) { + Destination8 = (UINT8*)Destination64; + Source8 = (CONST UINT8*)Source64; + + while (Alignment-- != 0) { + *(--Destination8) = *(--Source8); + --Length; + } + Destination64 = (UINT64*)Destination8; + Source64 = (CONST UINT64*)Source8; + } + + while (Length > 0) { + *(--Destination64) = *(--Source64); + Length -= 8; + } + } + } else if ((((UINTN)DestinationBuffer & 0x3) == 0) && (((UINTN)SourceBuffer & 0x3) == 0) && (Length >= 4)) { + if (SourceBuffer > DestinationBuffer) { + Destination32 = (UINT32*)DestinationBuffer; + Source32 = (CONST UINT32*)SourceBuffer; + while (Length >= 4) { + *(Destination32++) = *(Source32++); + Length -= 4; + } + + // Finish if there are still some bytes to copy + Destination8 = (UINT8*)Destination32; + Source8 = (CONST UINT8*)Source32; + while (Length-- != 0) { + *(Destination8++) = *(Source8++); + } + } else if (SourceBuffer < DestinationBuffer) { + Destination32 = (UINT32*)((UINTN)DestinationBuffer + Length); + Source32 = (CONST UINT32*)((UINTN)SourceBuffer + Length); + + // Destination32 and Source32 were aligned on a 32-bit boundary + // but if length is not a multiple of 4 bytes then they won't be + // anymore. + + Alignment = Length & 0x3; + if (Alignment != 0) { + Destination8 = (UINT8*)Destination32; + Source8 = (CONST UINT8*)Source32; + + while (Alignment-- != 0) { + *(--Destination8) = *(--Source8); + --Length; + } + Destination32 = (UINT32*)Destination8; + Source32 = (CONST UINT32*)Source8; + } + + while (Length > 0) { + *(--Destination32) = *(--Source32); + Length -= 4; + } + } + } else { + if (SourceBuffer > DestinationBuffer) { + Destination8 = (UINT8*)DestinationBuffer; + Source8 = (CONST UINT8*)SourceBuffer; + while (Length-- != 0) { + *(Destination8++) = *(Source8++); + } + } else if (SourceBuffer < DestinationBuffer) { + Destination8 = (UINT8*)DestinationBuffer + Length; + Source8 = (CONST UINT8*)SourceBuffer + Length; + while (Length-- != 0) { + *(--Destination8) = *(--Source8); + } + } + } + return DestinationBuffer; +} + EFI_STATUS NorFlashReadBlocks ( IN NOR_FLASH_INSTANCE *Instance, @@ -791,7 +917,7 @@ NorFlashReadBlocks ( SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY); // Readout the data - CopyMem(Buffer, (UINTN *)StartAddress, BufferSizeInBytes); + AlignedCopyMem (Buffer, (UINTN *)StartAddress, BufferSizeInBytes); return EFI_SUCCESS; } @@ -832,7 +958,7 @@ NorFlashRead ( SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY); // Readout the data - CopyMem (Buffer, (UINTN *)(StartAddress + Offset), BufferSizeInBytes); + AlignedCopyMem (Buffer, (UINTN *)(StartAddress + Offset), BufferSizeInBytes); return EFI_SUCCESS; }