From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-x229.google.com (mail-wm0-x229.google.com [IPv6:2a00:1450:400c:c09::229]) (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 94E741A1E14 for ; Fri, 9 Sep 2016 05:18:30 -0700 (PDT) Received: by mail-wm0-x229.google.com with SMTP id 1so29582485wmz.1 for ; Fri, 09 Sep 2016 05:18:30 -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=xa2/WLX4XgRFhoqkkTd/Y88TJGhyo6qkE1KDrgNdKm4=; b=hMSAhYtDdv9PNMJUjEKE7lm/YXB2AP7DoFWbCm7CCd0H0wuuO8KsaqwEKasr/V/SF0 B8cxPJelDw3ATA8ukgmGHuYCo+d/CVRnohiUY+AVpZh/BuOQUdontE8EWqIBhiPUkX4W W39SmODpoiYTpn4cosFIeEJcsWb6HqeE3Iyfo= 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=xa2/WLX4XgRFhoqkkTd/Y88TJGhyo6qkE1KDrgNdKm4=; b=SL/O9yU4tk0AH8IdOmvkkS1PQVL1DlrCRt7JmgB+XNT1hmLgXrEXxxy0y5AxKziRnZ TgKAB0qWxUpJzP4b5ew4xFBiApj/nIrMDkNz5RdHUQdUQKtAuJHo7+e4rEc4uBQ191TO 2jiaxZdMWajfsdSSSM9GndOKkZGU3ZJ8eFEXX9VjII/ALd/Ls58nltRTb0jI+FQ+MPJx KH9EPT2rvhvai98kmSIaBfPlWb75loN9l6LcOcL/P8W9Dz503UMk6lC2gCxJ0lIWojpU sqznb/tOXOjNAbdEr5b9qG45nI/YXgm/o2xtDnbVmARk3YXnTAtIHj2bOAE4fp+mpHb4 4ACA== X-Gm-Message-State: AE9vXwOj0HpIXrsjZkkIok0jZLdkOZlpYeI/ix6Yb+qUpVPmlebzVilv/orTePXLM3Q7NbXg X-Received: by 10.194.192.195 with SMTP id hi3mr2811293wjc.108.1473423509159; Fri, 09 Sep 2016 05:18:29 -0700 (PDT) Received: from localhost.localdomain ([105.190.180.180]) by smtp.gmail.com with ESMTPSA id gf4sm3112584wjb.47.2016.09.09.05.18.27 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Fri, 09 Sep 2016 05:18:28 -0700 (PDT) From: Ard Biesheuvel To: edk2-devel@lists.01.org, leif.lindholm@linaro.org Cc: Ard Biesheuvel Date: Fri, 9 Sep 2016 13:18:22 +0100 Message-Id: <1473423502-7061-1-git-send-email-ard.biesheuvel@linaro.org> X-Mailer: git-send-email 2.7.4 Subject: [PATCH v2] 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: , X-List-Received-Date: Fri, 09 Sep 2016 12:18:31 -0000 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 --- v2: - simplify the AlignedCopyMem () implementation, given that we do not require memmove() semantics for copying between NOR flash and memory - document the above - add macro to perform alignment check - drop redundant/unnecessary casts and parentheses ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 63 +++++++++++++++++++- 1 file changed, 61 insertions(+), 2 deletions(-) diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c index 3abbe5cb32bc..ca61ac5e1983 100644 --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c @@ -744,6 +744,65 @@ NorFlashWriteBlocks ( return Status; } +#define BOTH_ALIGNED(a, b, align) ((((UINTN)(a) | (UINTN)(b)) & ((align) - 1)) == 0) + +/** + Copy Length bytes from Source to Destination, using aligned accesses only. + Note that this implementation uses memcpy() semantics rather then memmove() + semantics, i.e., SourceBuffer and DestinationBuffer should not overlap. + + @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; + + if (BOTH_ALIGNED(DestinationBuffer, SourceBuffer, 8) && Length >= 8) { + Destination64 = DestinationBuffer; + Source64 = SourceBuffer; + while (Length >= 8) { + *Destination64++ = *Source64++; + Length -= 8; + } + + Destination8 = (UINT8 *)Destination64; + Source8 = (CONST UINT8 *)Source64; + } else if (BOTH_ALIGNED(DestinationBuffer, SourceBuffer, 4) && Length >= 4) { + Destination32 = DestinationBuffer; + Source32 = SourceBuffer; + while (Length >= 4) { + *Destination32++ = *Source32++; + Length -= 4; + } + + Destination8 = (UINT8 *)Destination32; + Source8 = (CONST UINT8 *)Source32; + } else { + Destination8 = DestinationBuffer; + Source8 = SourceBuffer; + } + while (Length-- != 0) { + *Destination8++ = *Source8++; + } + return DestinationBuffer; +} + EFI_STATUS NorFlashReadBlocks ( IN NOR_FLASH_INSTANCE *Instance, @@ -791,7 +850,7 @@ NorFlashReadBlocks ( SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY); // Readout the data - CopyMem(Buffer, (UINTN *)StartAddress, BufferSizeInBytes); + AlignedCopyMem (Buffer, (VOID *)StartAddress, BufferSizeInBytes); return EFI_SUCCESS; } @@ -832,7 +891,7 @@ NorFlashRead ( SEND_NOR_COMMAND (Instance->DeviceBaseAddress, 0, P30_CMD_READ_ARRAY); // Readout the data - CopyMem (Buffer, (UINTN *)(StartAddress + Offset), BufferSizeInBytes); + AlignedCopyMem (Buffer, (VOID *)StartAddress + Offset, BufferSizeInBytes); return EFI_SUCCESS; } -- 2.7.4