public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH v5] ArmPlatformPkg: Enable support for flash in 64-bit address space
@ 2021-01-06  9:04 Vijayenthiran Subramaniam
  2021-01-07  3:00 ` Masahisa Kojima
  0 siblings, 1 reply; 3+ messages in thread
From: Vijayenthiran Subramaniam @ 2021-01-06  9:04 UTC (permalink / raw)
  To: devel, masahisa.kojima, Ard.Biesheuvel, sami.mujawar; +Cc: leif, thomas.abraham

The existing NOR Flash DXE and StandaloneMm driver supports NOR flash
devices connected in the 32-bit address space. Extend these drivers to
allow NOR flash devices connected to 64-bit address space to be usable
as well. Also, convert the base address and size sanity check from
ASSERT() to if condition so that even if the firmware is build in
release mode, it can return error if the parameter(s) is/are invalid.

Signed-off-by: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>
---

Changes since v4:
  - Update NorFlashStandaloneMm.c to be usable on 64-bit address space

Changes since v3:
  - Add 64-bit PCDs to NorFlashStandaloneMm.inf to build for StandaloneMm

Changes since v2:
  - Rebased to latest edk2 master branch and update copyright year
  - Retaining Sami's R-by from
    https://edk2.groups.io/g/devel/message/69214

 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf          |  5 +-
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf |  5 +-
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c            | 22 +++++--
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c            | 61 +++++++++++++++++---
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c   | 22 +++++--
 5 files changed, 92 insertions(+), 23 deletions(-)

diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
index 8b5078497fff..f8d4c2703143 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
@@ -2,7 +2,7 @@
 #
 #  Component description file for NorFlashDxe module
 #
-#  Copyright (c) 2011 - 2014, ARM Ltd. All rights reserved.<BR>
+#  Copyright (c) 2011 - 2021, Arm Limited. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
 #
@@ -55,10 +55,13 @@ [Protocols]
   gEfiDiskIoProtocolGuid
 
 [Pcd.common]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
 
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
index f788472406b7..b2f72fb4de20 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
@@ -2,7 +2,7 @@
 #
 #  Component description file for NorFlashStandaloneMm module
 #
-#  Copyright (c) 2011 - 2014, ARM Ltd. All rights reserved.<BR>
+#  Copyright (c) 2011 - 2021, Arm Limited. All rights reserved.<BR>
 #  Copyright (c) 2020, Linaro, Ltd. All rights reserved.<BR>
 #
 #  SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -50,10 +50,13 @@ [Protocols]
   gEfiSmmFirmwareVolumeBlockProtocolGuid
 
 [Pcd.common]
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase64
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingBase
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWorkingSize
+  gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase64
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
 
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
index 41cdd1cbd397..28dc8e125c78 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
@@ -1,6 +1,6 @@
 /** @file  NorFlashDxe.c
 
-  Copyright (c) 2011 - 2020, Arm Limited. All rights reserved.<BR>
+  Copyright (c) 2011 - 2021, Arm Limited. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -343,9 +343,18 @@ NorFlashInitialise (
 
   for (Index = 0; Index < mNorFlashDeviceCount; Index++) {
     // Check if this NOR Flash device contain the variable storage region
-    ContainVariableStorage =
-        (NorFlashDevices[Index].RegionBaseAddress <= PcdGet32 (PcdFlashNvStorageVariableBase)) &&
-        (PcdGet32 (PcdFlashNvStorageVariableBase) + PcdGet32 (PcdFlashNvStorageVariableSize) <= NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
+
+   if (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
+     ContainVariableStorage =
+       (NorFlashDevices[Index].RegionBaseAddress <= PcdGet64 (PcdFlashNvStorageVariableBase64)) &&
+       (PcdGet64 (PcdFlashNvStorageVariableBase64) + PcdGet32 (PcdFlashNvStorageVariableSize) <=
+        NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
+   } else {
+     ContainVariableStorage =
+       (NorFlashDevices[Index].RegionBaseAddress <= PcdGet32 (PcdFlashNvStorageVariableBase)) &&
+       (PcdGet32 (PcdFlashNvStorageVariableBase) + PcdGet32 (PcdFlashNvStorageVariableSize) <=
+        NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
+  }
 
     Status = NorFlashCreateInstance (
       NorFlashDevices[Index].DeviceBaseAddress,
@@ -413,10 +422,11 @@ NorFlashFvbInitialize (
       EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
   ASSERT_EFI_ERROR (Status);
 
-  mFlashNvStorageVariableBase = PcdGet32 (PcdFlashNvStorageVariableBase);
+  mFlashNvStorageVariableBase = (FixedPcdGet64 (PcdFlashNvStorageVariableBase64) != 0) ?
+    FixedPcdGet64 (PcdFlashNvStorageVariableBase64) : FixedPcdGet32 (PcdFlashNvStorageVariableBase);
 
   // Set the index of the first LBA for the FVB
-  Instance->StartLba = (PcdGet32 (PcdFlashNvStorageVariableBase) - Instance->RegionBaseAddress) / Instance->Media.BlockSize;
+  Instance->StartLba = (mFlashNvStorageVariableBase - Instance->RegionBaseAddress) / Instance->Media.BlockSize;
 
   BootMode = GetBootModeHob ();
   if (BootMode == BOOT_WITH_DEFAULT_SETTINGS) {
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c
index a332b5e98be7..db8eb595f4b8 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvb.c
@@ -1,6 +1,6 @@
 /*++ @file  NorFlashFvbDxe.c
 
- Copyright (c) 2011 - 2020, ARM Ltd. All rights reserved.<BR>
+ Copyright (c) 2011 - 2021, Arm Limited. All rights reserved.<BR>
 
  SPDX-License-Identifier: BSD-2-Clause-Patent
 
@@ -48,23 +48,66 @@ InitializeFvAndVariableStoreHeaders (
   UINTN                               HeadersLength;
   EFI_FIRMWARE_VOLUME_HEADER          *FirmwareVolumeHeader;
   VARIABLE_STORE_HEADER               *VariableStoreHeader;
+  UINT32                              NvStorageFtwSpareSize;
+  UINT32                              NvStorageFtwWorkingSize;
+  UINT32                              NvStorageVariableSize;
+  UINT64                              NvStorageFtwSpareBase;
+  UINT64                              NvStorageFtwWorkingBase;
+  UINT64                              NvStorageVariableBase;
 
   HeadersLength = sizeof(EFI_FIRMWARE_VOLUME_HEADER) + sizeof(EFI_FV_BLOCK_MAP_ENTRY) + sizeof(VARIABLE_STORE_HEADER);
   Headers = AllocateZeroPool(HeadersLength);
 
+  NvStorageFtwWorkingSize = PcdGet32 (PcdFlashNvStorageFtwWorkingSize);
+  NvStorageFtwSpareSize = PcdGet32 (PcdFlashNvStorageFtwSpareSize);
+  NvStorageVariableSize = PcdGet32 (PcdFlashNvStorageVariableSize);
+
+  NvStorageFtwSpareBase = (PcdGet64 (PcdFlashNvStorageFtwSpareBase64) != 0) ?
+    PcdGet64 (PcdFlashNvStorageFtwSpareBase64) : PcdGet32 (PcdFlashNvStorageFtwSpareBase);
+  NvStorageFtwWorkingBase = (PcdGet64 (PcdFlashNvStorageFtwWorkingBase64) != 0) ?
+    PcdGet64 (PcdFlashNvStorageFtwWorkingBase64) : PcdGet32 (PcdFlashNvStorageFtwWorkingBase);
+  NvStorageVariableBase = (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) ?
+    PcdGet64 (PcdFlashNvStorageVariableBase64) : PcdGet32 (PcdFlashNvStorageVariableBase);
+
   // FirmwareVolumeHeader->FvLength is declared to have the Variable area AND the FTW working area AND the FTW Spare contiguous.
-  ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) + PcdGet32(PcdFlashNvStorageVariableSize) == PcdGet32(PcdFlashNvStorageFtwWorkingBase));
-  ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) + PcdGet32(PcdFlashNvStorageFtwWorkingSize) == PcdGet32(PcdFlashNvStorageFtwSpareBase));
+  if ((NvStorageVariableBase + NvStorageVariableSize) != NvStorageFtwWorkingBase) {
+    DEBUG ((DEBUG_ERROR, "%a: NvStorageFtwWorkingBase is not contiguous with NvStorageVariableBase region\n",
+          __FUNCTION__));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if ((NvStorageFtwWorkingBase + NvStorageFtwWorkingSize) != NvStorageFtwSpareBase) {
+    DEBUG ((DEBUG_ERROR, "%a: NvStorageFtwSpareBase is not contiguous with NvStorageFtwWorkingBase region\n",
+        __FUNCTION__));
+    return EFI_INVALID_PARAMETER;
+  }
 
   // Check if the size of the area is at least one block size
-  ASSERT((PcdGet32(PcdFlashNvStorageVariableSize) > 0) && (PcdGet32(PcdFlashNvStorageVariableSize) / Instance->Media.BlockSize > 0));
-  ASSERT((PcdGet32(PcdFlashNvStorageFtwWorkingSize) > 0) && (PcdGet32(PcdFlashNvStorageFtwWorkingSize) / Instance->Media.BlockSize > 0));
-  ASSERT((PcdGet32(PcdFlashNvStorageFtwSpareSize) > 0) && (PcdGet32(PcdFlashNvStorageFtwSpareSize) / Instance->Media.BlockSize > 0));
+  if ((NvStorageVariableSize <= 0) || (NvStorageVariableSize / Instance->Media.BlockSize <= 0)) {
+    DEBUG ((DEBUG_ERROR, "%a: NvStorageVariableSize is 0x%x, should be atleast one block size\n", __FUNCTION__,
+          NvStorageVariableSize));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if ((NvStorageFtwWorkingSize <= 0) || (NvStorageFtwWorkingSize / Instance->Media.BlockSize <= 0)) {
+    DEBUG ((DEBUG_ERROR, "%a: NvStorageFtwWorkingSize is 0x%x, should be atleast one block size\n", __FUNCTION__,
+          NvStorageFtwWorkingSize));
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if ((NvStorageFtwSpareSize <= 0) || (NvStorageFtwSpareSize / Instance->Media.BlockSize <= 0)) {
+    DEBUG ((DEBUG_ERROR, "%a: NvStorageFtwSpareSize is 0x%x, should be atleast one block size\n", __FUNCTION__,
+          NvStorageFtwSpareSize));
+    return EFI_INVALID_PARAMETER;
+  }
 
   // Ensure the Variable area Base Addresses are aligned on a block size boundaries
-  ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) % Instance->Media.BlockSize == 0);
-  ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) % Instance->Media.BlockSize == 0);
-  ASSERT(PcdGet32(PcdFlashNvStorageFtwSpareBase) % Instance->Media.BlockSize == 0);
+  if ((NvStorageVariableBase % Instance->Media.BlockSize != 0) ||
+      (NvStorageFtwWorkingBase % Instance->Media.BlockSize != 0) ||
+      (NvStorageFtwSpareBase % Instance->Media.BlockSize != 0)) {
+    DEBUG ((DEBUG_ERROR, "%a: NvStorage Base addresses must be aligned to block size boundaries", __FUNCTION__));
+    return EFI_INVALID_PARAMETER;
+  }
 
   //
   // EFI_FIRMWARE_VOLUME_HEADER
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
index 1ebf6d6ba70b..8a4fb395d286 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
@@ -1,6 +1,6 @@
 /** @file  NorFlashStandaloneMm.c
 
-  Copyright (c) 2011 - 2020, Arm Limited. All rights reserved.<BR>
+  Copyright (c) 2011 - 2021, Arm Limited. All rights reserved.<BR>
   Copyright (c) 2020, Linaro, Ltd. All rights reserved.<BR>
 
   SPDX-License-Identifier: BSD-2-Clause-Patent
@@ -298,9 +298,18 @@ NorFlashInitialise (
 
   for (Index = 0; Index < mNorFlashDeviceCount; Index++) {
     // Check if this NOR Flash device contain the variable storage region
-    ContainVariableStorage =
-        (NorFlashDevices[Index].RegionBaseAddress <= PcdGet32 (PcdFlashNvStorageVariableBase)) &&
-        (PcdGet32 (PcdFlashNvStorageVariableBase) + PcdGet32 (PcdFlashNvStorageVariableSize) <= NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
+
+   if (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
+     ContainVariableStorage =
+       (NorFlashDevices[Index].RegionBaseAddress <= PcdGet64 (PcdFlashNvStorageVariableBase64)) &&
+       (PcdGet64 (PcdFlashNvStorageVariableBase64) + PcdGet32 (PcdFlashNvStorageVariableSize) <=
+        NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
+   } else {
+     ContainVariableStorage =
+       (NorFlashDevices[Index].RegionBaseAddress <= PcdGet32 (PcdFlashNvStorageVariableBase)) &&
+       (PcdGet32 (PcdFlashNvStorageVariableBase) + PcdGet32 (PcdFlashNvStorageVariableSize) <=
+        NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
+  }
 
     Status = NorFlashCreateInstance (
       NorFlashDevices[Index].DeviceBaseAddress,
@@ -330,10 +339,11 @@ NorFlashFvbInitialize (
 
   ASSERT((Instance != NULL));
 
-  mFlashNvStorageVariableBase = PcdGet32 (PcdFlashNvStorageVariableBase);
 
+  mFlashNvStorageVariableBase = (FixedPcdGet64 (PcdFlashNvStorageVariableBase64) != 0) ?
+    FixedPcdGet64 (PcdFlashNvStorageVariableBase64) : FixedPcdGet32 (PcdFlashNvStorageVariableBase);
   // Set the index of the first LBA for the FVB
-  Instance->StartLba = (PcdGet32 (PcdFlashNvStorageVariableBase) - Instance->RegionBaseAddress) / Instance->Media.BlockSize;
+  Instance->StartLba = (mFlashNvStorageVariableBase - Instance->RegionBaseAddress) / Instance->Media.BlockSize;
 
   // Determine if there is a valid header at the beginning of the NorFlash
   Status = ValidateFvHeader (Instance);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH v5] ArmPlatformPkg: Enable support for flash in 64-bit address space
  2021-01-06  9:04 [PATCH v5] ArmPlatformPkg: Enable support for flash in 64-bit address space Vijayenthiran Subramaniam
@ 2021-01-07  3:00 ` Masahisa Kojima
  2021-01-07 15:26   ` Ard Biesheuvel
  0 siblings, 1 reply; 3+ messages in thread
From: Masahisa Kojima @ 2021-01-07  3:00 UTC (permalink / raw)
  To: Vijayenthiran Subramaniam
  Cc: edk2-devel-groups-io, Ard Biesheuvel, Sami Mujawar, Leif Lindholm,
	Thomas Abraham

Hi Vijay,

I have verified this patch on the StandaloneMM platform(SBSA-QEMU).
Both 32bit(PcdFlashNvStorageVariableBase) and
64bit(PcdFlashNvStorageVariableBase64) cases work fine.

Tested-by: Masahisa Kojima <masahisa.kojima@linaro.org>


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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH v5] ArmPlatformPkg: Enable support for flash in 64-bit address space
  2021-01-07  3:00 ` Masahisa Kojima
@ 2021-01-07 15:26   ` Ard Biesheuvel
  0 siblings, 0 replies; 3+ messages in thread
From: Ard Biesheuvel @ 2021-01-07 15:26 UTC (permalink / raw)
  To: Masahisa Kojima, Vijayenthiran Subramaniam
  Cc: edk2-devel-groups-io, Sami Mujawar, Leif Lindholm, Thomas Abraham

On 1/7/21 4:00 AM, Masahisa Kojima wrote:
> Hi Vijay,
> 
> I have verified this patch on the StandaloneMM platform(SBSA-QEMU).
> Both 32bit(PcdFlashNvStorageVariableBase) and
> 64bit(PcdFlashNvStorageVariableBase64) cases work fine.
> 
> Tested-by: Masahisa Kojima <masahisa.kojima@linaro.org>
> 

Merged as #1315

Thanks all,

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


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-01-07 15:26 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-06  9:04 [PATCH v5] ArmPlatformPkg: Enable support for flash in 64-bit address space Vijayenthiran Subramaniam
2021-01-07  3:00 ` Masahisa Kojima
2021-01-07 15:26   ` Ard Biesheuvel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox