* [PATCH v4] ArmPlatformPkg: Enable support for flash in 64-bit address space
@ 2021-01-05 10:57 Vijayenthiran Subramaniam
2021-01-05 11:07 ` Ard Biesheuvel
0 siblings, 1 reply; 5+ messages in thread
From: Vijayenthiran Subramaniam @ 2021-01-05 10:57 UTC (permalink / raw)
To: devel, leif, Ard.Biesheuvel, thomas.abraham, sami.mujawar
The existing NOR Flash dxe driver supports NOR flash devices connected
in the 32-bit address space. Extend this driver 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@arm.com>
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
---
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 +++++++++++++++++---
4 files changed, 76 insertions(+), 17 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
--
2.17.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v4] ArmPlatformPkg: Enable support for flash in 64-bit address space
2021-01-05 10:57 [PATCH v4] ArmPlatformPkg: Enable support for flash in 64-bit address space Vijayenthiran Subramaniam
@ 2021-01-05 11:07 ` Ard Biesheuvel
2021-01-05 11:17 ` [edk2-devel] " Vijayenthiran Subramaniam
0 siblings, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2021-01-05 11:07 UTC (permalink / raw)
To: Vijayenthiran Subramaniam, devel, leif, thomas.abraham,
sami.mujawar, Masahisa Kojima
On 1/5/21 11:57 AM, Vijayenthiran Subramaniam wrote:
> The existing NOR Flash dxe driver supports NOR flash devices connected
> in the 32-bit address space. Extend this driver 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@arm.com>
> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> ---
Thanks Vijay.
But adding the new PCDs to
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf only makes
sense if the driver uses their values. So it appears to me its version
of NorFlashInitialise needs to be modified as well.
Also, please cc Masahisa (cc'ed) on your next revision - he should be
able to test your patch on a standalone MM based platform.
Thanks,
Ard.
>
> 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 +++++++++++++++++---
> 4 files changed, 76 insertions(+), 17 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
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH v4] ArmPlatformPkg: Enable support for flash in 64-bit address space
2021-01-05 11:07 ` Ard Biesheuvel
@ 2021-01-05 11:17 ` Vijayenthiran Subramaniam
2021-01-06 9:07 ` Vijayenthiran Subramaniam
0 siblings, 1 reply; 5+ messages in thread
From: Vijayenthiran Subramaniam @ 2021-01-05 11:17 UTC (permalink / raw)
To: devel, Ard Biesheuvel
Cc: Vijayenthiran Subramaniam, leif, Thomas Abraham, Sami Mujawar,
Masahisa Kojima
Hi Ard,
On Tue, Jan 5, 2021 at 11:07 AM Ard Biesheuvel <ard.biesheuvel@arm.com> wrote:
>
> On 1/5/21 11:57 AM, Vijayenthiran Subramaniam wrote:
> > The existing NOR Flash dxe driver supports NOR flash devices connected
> > in the 32-bit address space. Extend this driver 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@arm.com>
> > Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> > ---
>
> Thanks Vijay.
>
> But adding the new PCDs to
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf only makes
> sense if the driver uses their values. So it appears to me its version
> of NorFlashInitialise needs to be modified as well.
Right. I will do that and post a new version.
> Also, please cc Masahisa (cc'ed) on your next revision - he should be
> able to test your patch on a standalone MM based platform.
Sure :)
> Thanks,
> Ard.
>
>
> >
> > 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 +++++++++++++++++---
> > 4 files changed, 76 insertions(+), 17 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
> >
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH v4] ArmPlatformPkg: Enable support for flash in 64-bit address space
2021-01-05 11:17 ` [edk2-devel] " Vijayenthiran Subramaniam
@ 2021-01-06 9:07 ` Vijayenthiran Subramaniam
2021-01-06 12:13 ` Masahisa Kojima
0 siblings, 1 reply; 5+ messages in thread
From: Vijayenthiran Subramaniam @ 2021-01-06 9:07 UTC (permalink / raw)
To: Masahisa Kojima
Cc: devel, Ard Biesheuvel, Vijayenthiran Subramaniam, leif,
Thomas Abraham, Sami Mujawar
Hi Masahisa,
On Tue, Jan 5, 2021 at 11:17 AM Vijayenthiran Subramanian
<vijayenthiran.subramaniam@arm.com> wrote:
>
> Hi Ard,
>
> On Tue, Jan 5, 2021 at 11:07 AM Ard Biesheuvel <ard.biesheuvel@arm.com> wrote:
> >
> > On 1/5/21 11:57 AM, Vijayenthiran Subramaniam wrote:
> > > The existing NOR Flash dxe driver supports NOR flash devices connected
> > > in the 32-bit address space. Extend this driver 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@arm.com>
> > > Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> > > ---
> >
> > Thanks Vijay.
> >
> > But adding the new PCDs to
> > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf only makes
> > sense if the driver uses their values. So it appears to me its version
> > of NorFlashInitialise needs to be modified as well.
>
> Right. I will do that and post a new version.
>
> > Also, please cc Masahisa (cc'ed) on your next revision - he should be
> > able to test your patch on a standalone MM based platform.
>
> Sure :)
Can you test the v5 of this patch on the StandaloneMm based platform?
(posted here: https://edk2.groups.io/g/devel/message/69798)
Regards,
Vijay
>
> > Thanks,
> > Ard.
> >
> >
> > >
> > > 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 +++++++++++++++++---
> > > 4 files changed, 76 insertions(+), 17 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
> > >
> >
> >
> >
> >
> >
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH v4] ArmPlatformPkg: Enable support for flash in 64-bit address space
2021-01-06 9:07 ` Vijayenthiran Subramaniam
@ 2021-01-06 12:13 ` Masahisa Kojima
0 siblings, 0 replies; 5+ messages in thread
From: Masahisa Kojima @ 2021-01-06 12:13 UTC (permalink / raw)
To: Vijayenthiran Subramanian
Cc: edk2-devel-groups-io, Ard Biesheuvel, Leif Lindholm,
Thomas Abraham, Sami Mujawar
Hi Vijay,
> Can you test the v5 of this patch on the StandaloneMm based platform?
> (posted here: https://edk2.groups.io/g/devel/message/69798)
OK.
I will test the patch tomorrow(JST).
Regards,
Masahisa
On Wed, 6 Jan 2021 at 18:07, Vijayenthiran Subramanian
<vijayenthiran.subramaniam@arm.com> wrote:
>
> Hi Masahisa,
>
> On Tue, Jan 5, 2021 at 11:17 AM Vijayenthiran Subramanian
> <vijayenthiran.subramaniam@arm.com> wrote:
> >
> > Hi Ard,
> >
> > On Tue, Jan 5, 2021 at 11:07 AM Ard Biesheuvel <ard.biesheuvel@arm.com> wrote:
> > >
> > > On 1/5/21 11:57 AM, Vijayenthiran Subramaniam wrote:
> > > > The existing NOR Flash dxe driver supports NOR flash devices connected
> > > > in the 32-bit address space. Extend this driver 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@arm.com>
> > > > Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> > > > ---
> > >
> > > Thanks Vijay.
> > >
> > > But adding the new PCDs to
> > > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf only makes
> > > sense if the driver uses their values. So it appears to me its version
> > > of NorFlashInitialise needs to be modified as well.
> >
> > Right. I will do that and post a new version.
> >
> > > Also, please cc Masahisa (cc'ed) on your next revision - he should be
> > > able to test your patch on a standalone MM based platform.
> >
> > Sure :)
>
> Can you test the v5 of this patch on the StandaloneMm based platform?
> (posted here: https://edk2.groups.io/g/devel/message/69798)
>
> Regards,
> Vijay
>
>
>
> >
> > > Thanks,
> > > Ard.
> > >
> > >
> > > >
> > > > 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 +++++++++++++++++---
> > > > 4 files changed, 76 insertions(+), 17 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
> > > >
> > >
> > >
> > >
> > >
> > >
> > >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2021-01-06 12:13 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-05 10:57 [PATCH v4] ArmPlatformPkg: Enable support for flash in 64-bit address space Vijayenthiran Subramaniam
2021-01-05 11:07 ` Ard Biesheuvel
2021-01-05 11:17 ` [edk2-devel] " Vijayenthiran Subramaniam
2021-01-06 9:07 ` Vijayenthiran Subramaniam
2021-01-06 12:13 ` Masahisa Kojima
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox