* [PATCH v2] ArmPlatformPkg: Enable support for flash in 64-bit address space
@ 2020-12-10 13:53 Vijayenthiran Subramaniam
2020-12-18 18:13 ` Sami Mujawar
0 siblings, 1 reply; 4+ messages in thread
From: Vijayenthiran Subramaniam @ 2020-12-10 13:53 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>
---
Changes since v1:
- Address Sami Mujawar from edk2.groups.io/g/devel/message/67980
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf | 5 +-
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 15 ++++-
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c | 66 ++++++++++++++++----
3 files changed, 71 insertions(+), 15 deletions(-)
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
index a647c016878d..992ae32f4c3b 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 - 2020, Arm Limited. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -54,10 +54,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/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
index 20134094badc..de9f3d0d7edb 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
@@ -1300,9 +1300,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,
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
index 9cdd85096a46..e2a1ad5ece50 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
@@ -1,6 +1,6 @@
/*++ @file NorFlashFvbDxe.c
- Copyright (c) 2011 - 2020, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2011 - 2020, Arm Limited. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -53,23 +53,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
@@ -736,10 +779,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) {
--
2.17.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] ArmPlatformPkg: Enable support for flash in 64-bit address space
2020-12-10 13:53 [PATCH v2] ArmPlatformPkg: Enable support for flash in 64-bit address space Vijayenthiran Subramaniam
@ 2020-12-18 18:13 ` Sami Mujawar
2021-01-04 18:36 ` Ard Biesheuvel
0 siblings, 1 reply; 4+ messages in thread
From: Sami Mujawar @ 2020-12-18 18:13 UTC (permalink / raw)
To: Vijayenthiran Subramaniam, devel@edk2.groups.io,
leif@nuviainc.com, Ard Biesheuvel, Thomas Abraham
This patch looks good to me.
Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
Regards,
Sami Mujawar
-----Original Message-----
From: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>
Sent: 10 December 2020 01:53 PM
To: devel@edk2.groups.io; leif@nuviainc.com; Ard Biesheuvel <Ard.Biesheuvel@arm.com>; Thomas Abraham <thomas.abraham@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com>
Subject: [PATCH v2] ArmPlatformPkg: Enable support for flash in 64-bit address space
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>
---
Changes since v1:
- Address Sami Mujawar from edk2.groups.io/g/devel/message/67980
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf | 5 +-
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 15 ++++-
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c | 66 ++++++++++++++++----
3 files changed, 71 insertions(+), 15 deletions(-)
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
index a647c016878d..992ae32f4c3b 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 - 2020, Arm Limited. All rights reserved.<BR>
#
# SPDX-License-Identifier: BSD-2-Clause-Patent
#
@@ -54,10 +54,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/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
index 20134094badc..de9f3d0d7edb 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
@@ -1300,9 +1300,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,
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
index 9cdd85096a46..e2a1ad5ece50 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
@@ -1,6 +1,6 @@
/*++ @file NorFlashFvbDxe.c
- Copyright (c) 2011 - 2020, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2011 - 2020, Arm Limited. All rights reserved.<BR>
SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -53,23 +53,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
@@ -736,10 +779,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) {
--
2.17.1
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2] ArmPlatformPkg: Enable support for flash in 64-bit address space
2020-12-18 18:13 ` Sami Mujawar
@ 2021-01-04 18:36 ` Ard Biesheuvel
2021-01-05 6:34 ` [edk2-devel] " Vijayenthiran Subramaniam
0 siblings, 1 reply; 4+ messages in thread
From: Ard Biesheuvel @ 2021-01-04 18:36 UTC (permalink / raw)
To: Sami Mujawar, Vijayenthiran Subramaniam, devel@edk2.groups.io,
leif@nuviainc.com, Thomas Abraham
On 12/18/20 7:13 PM, Sami Mujawar wrote:
> This patch looks good to me.
>
> Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
>
Thanks Sami.
Vijay, could you please rebase this onto the latest edk2? There have
been some changes to the NorFlashDxe driver.
Thanks,
Ard.
> -----Original Message-----
> From: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>
> Sent: 10 December 2020 01:53 PM
> To: devel@edk2.groups.io; leif@nuviainc.com; Ard Biesheuvel <Ard.Biesheuvel@arm.com>; Thomas Abraham <thomas.abraham@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com>
> Subject: [PATCH v2] ArmPlatformPkg: Enable support for flash in 64-bit address space
>
> 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>
> ---
>
> Changes since v1:
> - Address Sami Mujawar from edk2.groups.io/g/devel/message/67980
>
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf | 5 +-
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 15 ++++-
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c | 66 ++++++++++++++++----
> 3 files changed, 71 insertions(+), 15 deletions(-)
>
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> index a647c016878d..992ae32f4c3b 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 - 2020, Arm Limited. All rights reserved.<BR>
> #
> # SPDX-License-Identifier: BSD-2-Clause-Patent
> #
> @@ -54,10 +54,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/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> index 20134094badc..de9f3d0d7edb 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> @@ -1300,9 +1300,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,
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> index 9cdd85096a46..e2a1ad5ece50 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> @@ -1,6 +1,6 @@
> /*++ @file NorFlashFvbDxe.c
>
> - Copyright (c) 2011 - 2020, ARM Ltd. All rights reserved.<BR>
> + Copyright (c) 2011 - 2020, Arm Limited. All rights reserved.<BR>
>
> SPDX-License-Identifier: BSD-2-Clause-Patent
>
> @@ -53,23 +53,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
> @@ -736,10 +779,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) {
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [edk2-devel] [PATCH v2] ArmPlatformPkg: Enable support for flash in 64-bit address space
2021-01-04 18:36 ` Ard Biesheuvel
@ 2021-01-05 6:34 ` Vijayenthiran Subramaniam
0 siblings, 0 replies; 4+ messages in thread
From: Vijayenthiran Subramaniam @ 2021-01-05 6:34 UTC (permalink / raw)
To: devel, Ard Biesheuvel
Cc: Sami Mujawar, Vijayenthiran Subramaniam, leif@nuviainc.com,
Thomas Abraham
Hi Ard,
On Mon, Jan 4, 2021 at 6:36 PM Ard Biesheuvel <ard.biesheuvel@arm.com> wrote:
>
> On 12/18/20 7:13 PM, Sami Mujawar wrote:
> > This patch looks good to me.
> >
> > Reviewed-by: Sami Mujawar <sami.mujawar@arm.com>
> >
>
> Thanks Sami.
>
> Vijay, could you please rebase this onto the latest edk2? There have
> been some changes to the NorFlashDxe driver.
>
> Thanks,
> Ard.
Done. Posted here: https://edk2.groups.io/g/devel/message/69677
Happy New Year and Regards,
Vijay
>
> > -----Original Message-----
> > From: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>
> > Sent: 10 December 2020 01:53 PM
> > To: devel@edk2.groups.io; leif@nuviainc.com; Ard Biesheuvel <Ard.Biesheuvel@arm.com>; Thomas Abraham <thomas.abraham@arm.com>; Sami Mujawar <Sami.Mujawar@arm.com>
> > Subject: [PATCH v2] ArmPlatformPkg: Enable support for flash in 64-bit address space
> >
> > 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>
> > ---
> >
> > Changes since v1:
> > - Address Sami Mujawar from edk2.groups.io/g/devel/message/67980
> >
> > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf | 5 +-
> > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 15 ++++-
> > ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c | 66 ++++++++++++++++----
> > 3 files changed, 71 insertions(+), 15 deletions(-)
> >
> > diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> > index a647c016878d..992ae32f4c3b 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 - 2020, Arm Limited. All rights reserved.<BR>
> > #
> > # SPDX-License-Identifier: BSD-2-Clause-Patent
> > #
> > @@ -54,10 +54,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/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> > index 20134094badc..de9f3d0d7edb 100644
> > --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> > +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> > @@ -1300,9 +1300,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,
> > diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> > index 9cdd85096a46..e2a1ad5ece50 100644
> > --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> > +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> > @@ -1,6 +1,6 @@
> > /*++ @file NorFlashFvbDxe.c
> >
> > - Copyright (c) 2011 - 2020, ARM Ltd. All rights reserved.<BR>
> > + Copyright (c) 2011 - 2020, Arm Limited. All rights reserved.<BR>
> >
> > SPDX-License-Identifier: BSD-2-Clause-Patent
> >
> > @@ -53,23 +53,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
> > @@ -736,10 +779,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) {
> > --
> > 2.17.1
> >
>
>
>
>
>
>
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-01-05 6:34 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-12-10 13:53 [PATCH v2] ArmPlatformPkg: Enable support for flash in 64-bit address space Vijayenthiran Subramaniam
2020-12-18 18:13 ` Sami Mujawar
2021-01-04 18:36 ` Ard Biesheuvel
2021-01-05 6:34 ` [edk2-devel] " Vijayenthiran Subramaniam
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox