[edk2-devel] [PATCH v5] ArmPlatformPkg: Enable support for flash in 64-bit address space
Masahisa Kojima
masahisa.kojima at linaro.org
Thu Jan 7 03:00:42 UTC 2021
Hi Vijay,
I have verified this patch on the StandaloneMM platform(SBSA-QEMU).
Both 32bit(PcdFlashNvStorageVariableBase) and
64bit(PcdFlashNvStorageVariableBase64) cases work fine.
Tested-by: Masahisa Kojima <masahisa.kojima at linaro.org>
On Wed, 6 Jan 2021 at 18:04, Vijayenthiran Subramaniam
<vijayenthiran.subramaniam at arm.com> wrote:
>
> The existing NOR Flash DXE and StandaloneMm driver supports NOR flash
> devices connected in the 32-bit address space. Extend these drivers to
> allow NOR flash devices connected to 64-bit address space to be usable
> as well. Also, convert the base address and size sanity check from
> ASSERT() to if condition so that even if the firmware is build in
> release mode, it can return error if the parameter(s) is/are invalid.
>
> Signed-off-by: Vijayenthiran Subramaniam <vijayenthiran.subramaniam at arm.com>
> ---
>
> Changes since v4:
> - Update NorFlashStandaloneMm.c to be usable on 64-bit address space
>
> Changes since v3:
> - Add 64-bit PCDs to NorFlashStandaloneMm.inf to build for StandaloneMm
>
> Changes since v2:
> - Rebased to latest edk2 master branch and update copyright year
> - Retaining Sami's R-by from
> https://edk2.groups.io/g/devel/message/69214
>
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf | 5 +-
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf | 5 +-
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 22 +++++--
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c | 61 +++++++++++++++++---
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c | 22 +++++--
> 5 files changed, 92 insertions(+), 23 deletions(-)
>
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> index 8b5078497fff..f8d4c2703143 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> @@ -2,7 +2,7 @@
> #
> # Component description file for NorFlashDxe module
> #
> -# Copyright (c) 2011 - 2014, ARM Ltd. All rights reserved.<BR>
> +# Copyright (c) 2011 - 2021, Arm Limited. All rights reserved.<BR>
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> @@ -55,10 +55,13 @@ [Protocols]
> gEfiDiskIoProtocolGuid
>
> [Pcd.common]
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
>
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> index f788472406b7..b2f72fb4de20 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> @@ -2,7 +2,7 @@
> #
> # Component description file for NorFlashStandaloneMm module
> #
> -# Copyright (c) 2011 - 2014, ARM Ltd. All rights reserved.<BR>
> +# Copyright (c) 2011 - 2021, Arm Limited. All rights reserved.<BR>
> # Copyright (c) 2020, Linaro, Ltd. All rights reserved.<BR>
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -50,10 +50,13 @@ [Protocols]
> gEfiSmmFirmwareVolumeBlockProtocolGuid
>
> [Pcd.common]
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
> + gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
>
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> index 41cdd1cbd397..28dc8e125c78 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> @@ -1,6 +1,6 @@
> /** @file NorFlashDxe.c
>
> - Copyright (c) 2011 - 2020, Arm Limited. All rights reserved.<BR>
> + Copyright (c) 2011 - 2021, Arm Limited. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -343,9 +343,18 @@ NorFlashInitialise (
>
> for (Index = 0; Index < mNorFlashDeviceCount; Index++) {
> // Check if this NOR Flash device contain the variable storage region
> - ContainVariableStorage =
> - (NorFlashDevices[Index].RegionBaseAddress <= PcdGet32 (PcdFlashNvStorageVariableBase)) &&
> - (PcdGet32 (PcdFlashNvStorageVariableBase) + PcdGet32 (PcdFlashNvStorageVariableSize) <= NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
> +
> + if (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
> + ContainVariableStorage =
> + (NorFlashDevices[Index].RegionBaseAddress <= PcdGet64 (PcdFlashNvStorageVariableBase64)) &&
> + (PcdGet64 (PcdFlashNvStorageVariableBase64) + PcdGet32 (PcdFlashNvStorageVariableSize) <=
> + NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
> + } else {
> + ContainVariableStorage =
> + (NorFlashDevices[Index].RegionBaseAddress <= PcdGet32 (PcdFlashNvStorageVariableBase)) &&
> + (PcdGet32 (PcdFlashNvStorageVariableBase) + PcdGet32 (PcdFlashNvStorageVariableSize) <=
> + NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
> + }
>
> Status = NorFlashCreateInstance (
> NorFlashDevices[Index].DeviceBaseAddress,
> @@ -413,10 +422,11 @@ NorFlashFvbInitialize (
> EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
> ASSERT_EFI_ERROR (Status);
>
> - mFlashNvStorageVariableBase = PcdGet32 (PcdFlashNvStorageVariableBase);
> + mFlashNvStorageVariableBase = (FixedPcdGet64 (PcdFlashNvStorageVariableBase64) != 0) ?
> + FixedPcdGet64 (PcdFlashNvStorageVariableBase64) : FixedPcdGet32 (PcdFlashNvStorageVariableBase);
>
> // Set the index of the first LBA for the FVB
> - Instance->StartLba = (PcdGet32 (PcdFlashNvStorageVariableBase) - Instance->RegionBaseAddress) / Instance->Media.BlockSize;
> + Instance->StartLba = (mFlashNvStorageVariableBase - Instance->RegionBaseAddress) / Instance->Media.BlockSize;
>
> BootMode = GetBootModeHob ();
> if (BootMode == BOOT_WITH_DEFAULT_SETTINGS) {
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c
> index a332b5e98be7..db8eb595f4b8 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c
> @@ -1,6 +1,6 @@
> /*++ @file NorFlashFvbDxe.c
>
> - Copyright (c) 2011 - 2020, ARM Ltd. All rights reserved.<BR>
> + Copyright (c) 2011 - 2021, Arm Limited. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -48,23 +48,66 @@ InitializeFvAndVariableStoreHeaders (
> UINTN HeadersLength;
> EFI_FIRMWARE_VOLUME_HEADER *FirmwareVolumeHeader;
> VARIABLE_STORE_HEADER *VariableStoreHeader;
> + UINT32 NvStorageFtwSpareSize;
> + UINT32 NvStorageFtwWorkingSize;
> + UINT32 NvStorageVariableSize;
> + UINT64 NvStorageFtwSpareBase;
> + UINT64 NvStorageFtwWorkingBase;
> + UINT64 NvStorageVariableBase;
>
> HeadersLength = sizeof(EFI_FIRMWARE_VOLUME_HEADER) + sizeof(EFI_FV_BLOCK_MAP_ENTRY) + sizeof(VARIABLE_STORE_HEADER);
> Headers = AllocateZeroPool(HeadersLength);
>
> + NvStorageFtwWorkingSize = PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
> + NvStorageFtwSpareSize = PcdGet32 (PcdFlashNvStorageFtwSpareSize);
> + NvStorageVariableSize = PcdGet32 (PcdFlashNvStorageVariableSize);
> +
> + NvStorageFtwSpareBase = (PcdGet64 (PcdFlashNvStorageFtwSpareBase64) != 0) ?
> + PcdGet64 (PcdFlashNvStorageFtwSpareBase64) : PcdGet32 (PcdFlashNvStorageFtwSpareBase);
> + NvStorageFtwWorkingBase = (PcdGet64 (PcdFlashNvStorageFtwWorkingBase64) != 0) ?
> + PcdGet64 (PcdFlashNvStorageFtwWorkingBase64) : PcdGet32 (PcdFlashNvStorageFtwWorkingBase);
> + NvStorageVariableBase = (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) ?
> + PcdGet64 (PcdFlashNvStorageVariableBase64) : PcdGet32 (PcdFlashNvStorageVariableBase);
> +
> // FirmwareVolumeHeader->FvLength is declared to have the Variable area AND the FTW working area AND the FTW Spare contiguous.
> - ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) + PcdGet32(PcdFlashNvStorageVariableSize) == PcdGet32(PcdFlashNvStorageFtwWorkingBase));
> - ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) + PcdGet32(PcdFlashNvStorageFtwWorkingSize) == PcdGet32(PcdFlashNvStorageFtwSpareBase));
> + if ((NvStorageVariableBase + NvStorageVariableSize) != NvStorageFtwWorkingBase) {
> + DEBUG ((DEBUG_ERROR, "%a: NvStorageFtwWorkingBase is not contiguous with NvStorageVariableBase region\n",
> + __FUNCTION__));
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + if ((NvStorageFtwWorkingBase + NvStorageFtwWorkingSize) != NvStorageFtwSpareBase) {
> + DEBUG ((DEBUG_ERROR, "%a: NvStorageFtwSpareBase is not contiguous with NvStorageFtwWorkingBase region\n",
> + __FUNCTION__));
> + return EFI_INVALID_PARAMETER;
> + }
>
> // Check if the size of the area is at least one block size
> - ASSERT((PcdGet32(PcdFlashNvStorageVariableSize) > 0) && (PcdGet32(PcdFlashNvStorageVariableSize) / Instance->Media.BlockSize > 0));
> - ASSERT((PcdGet32(PcdFlashNvStorageFtwWorkingSize) > 0) && (PcdGet32(PcdFlashNvStorageFtwWorkingSize) / Instance->Media.BlockSize > 0));
> - ASSERT((PcdGet32(PcdFlashNvStorageFtwSpareSize) > 0) && (PcdGet32(PcdFlashNvStorageFtwSpareSize) / Instance->Media.BlockSize > 0));
> + if ((NvStorageVariableSize <= 0) || (NvStorageVariableSize / Instance->Media.BlockSize <= 0)) {
> + DEBUG ((DEBUG_ERROR, "%a: NvStorageVariableSize is 0x%x, should be atleast one block size\n", __FUNCTION__,
> + NvStorageVariableSize));
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + if ((NvStorageFtwWorkingSize <= 0) || (NvStorageFtwWorkingSize / Instance->Media.BlockSize <= 0)) {
> + DEBUG ((DEBUG_ERROR, "%a: NvStorageFtwWorkingSize is 0x%x, should be atleast one block size\n", __FUNCTION__,
> + NvStorageFtwWorkingSize));
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + if ((NvStorageFtwSpareSize <= 0) || (NvStorageFtwSpareSize / Instance->Media.BlockSize <= 0)) {
> + DEBUG ((DEBUG_ERROR, "%a: NvStorageFtwSpareSize is 0x%x, should be atleast one block size\n", __FUNCTION__,
> + NvStorageFtwSpareSize));
> + return EFI_INVALID_PARAMETER;
> + }
>
> // Ensure the Variable area Base Addresses are aligned on a block size boundaries
> - ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) % Instance->Media.BlockSize == 0);
> - ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) % Instance->Media.BlockSize == 0);
> - ASSERT(PcdGet32(PcdFlashNvStorageFtwSpareBase) % Instance->Media.BlockSize == 0);
> + if ((NvStorageVariableBase % Instance->Media.BlockSize != 0) ||
> + (NvStorageFtwWorkingBase % Instance->Media.BlockSize != 0) ||
> + (NvStorageFtwSpareBase % Instance->Media.BlockSize != 0)) {
> + DEBUG ((DEBUG_ERROR, "%a: NvStorage Base addresses must be aligned to block size boundaries", __FUNCTION__));
> + return EFI_INVALID_PARAMETER;
> + }
>
> //
> // EFI_FIRMWARE_VOLUME_HEADER
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
> index 1ebf6d6ba70b..8a4fb395d286 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
> @@ -1,6 +1,6 @@
> /** @file NorFlashStandaloneMm.c
>
> - Copyright (c) 2011 - 2020, Arm Limited. All rights reserved.<BR>
> + Copyright (c) 2011 - 2021, Arm Limited. All rights reserved.<BR>
> Copyright (c) 2020, Linaro, Ltd. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
> @@ -298,9 +298,18 @@ NorFlashInitialise (
>
> for (Index = 0; Index < mNorFlashDeviceCount; Index++) {
> // Check if this NOR Flash device contain the variable storage region
> - ContainVariableStorage =
> - (NorFlashDevices[Index].RegionBaseAddress <= PcdGet32 (PcdFlashNvStorageVariableBase)) &&
> - (PcdGet32 (PcdFlashNvStorageVariableBase) + PcdGet32 (PcdFlashNvStorageVariableSize) <= NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
> +
> + if (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
> + ContainVariableStorage =
> + (NorFlashDevices[Index].RegionBaseAddress <= PcdGet64 (PcdFlashNvStorageVariableBase64)) &&
> + (PcdGet64 (PcdFlashNvStorageVariableBase64) + PcdGet32 (PcdFlashNvStorageVariableSize) <=
> + NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
> + } else {
> + ContainVariableStorage =
> + (NorFlashDevices[Index].RegionBaseAddress <= PcdGet32 (PcdFlashNvStorageVariableBase)) &&
> + (PcdGet32 (PcdFlashNvStorageVariableBase) + PcdGet32 (PcdFlashNvStorageVariableSize) <=
> + NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
> + }
>
> Status = NorFlashCreateInstance (
> NorFlashDevices[Index].DeviceBaseAddress,
> @@ -330,10 +339,11 @@ NorFlashFvbInitialize (
>
> ASSERT((Instance != NULL));
>
> - mFlashNvStorageVariableBase = PcdGet32 (PcdFlashNvStorageVariableBase);
>
> + mFlashNvStorageVariableBase = (FixedPcdGet64 (PcdFlashNvStorageVariableBase64) != 0) ?
> + FixedPcdGet64 (PcdFlashNvStorageVariableBase64) : FixedPcdGet32 (PcdFlashNvStorageVariableBase);
> // Set the index of the first LBA for the FVB
> - Instance->StartLba = (PcdGet32 (PcdFlashNvStorageVariableBase) - Instance->RegionBaseAddress) / Instance->Media.BlockSize;
> + Instance->StartLba = (mFlashNvStorageVariableBase - Instance->RegionBaseAddress) / Instance->Media.BlockSize;
>
> // Determine if there is a valid header at the beginning of the NorFlash
> Status = ValidateFvHeader (Instance);
> --
> 2.17.1
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#69888): https://edk2.groups.io/g/devel/message/69888
Mute This Topic: https://groups.io/mt/79471264/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list