[v1] ArmPlatformPkg: Enable support for flash in 64-bit address space

[edk2-devel] [PATCH] ArmPlatformPkg: Enable support for flash in 64-bit address space

Posted by Vijayenthiran Subramaniam 3 years, 5 months ago

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.

Signed-off-by: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>
---
 .../Drivers/NorFlashDxe/NorFlashDxe.c         | 13 ++++++--
 .../Drivers/NorFlashDxe/NorFlashDxe.inf       |  3 ++
 .../Drivers/NorFlashDxe/NorFlashFvbDxe.c      | 31 ++++++++++++++-----
 3 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
index d9e196cbf1..f3fbbafb7d 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
@@ -1298,9 +1298,16 @@ 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/NorFlashDxe.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
index a647c01687..b2a941d672 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
@@ -54,10 +54,13 @@
   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/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
index 9cdd85096a..ecbe009495 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
@@ -58,8 +58,17 @@ InitializeFvAndVariableStoreHeaders (
   Headers = AllocateZeroPool(HeadersLength);
 
   // 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 (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
+    ASSERT(PcdGet64(PcdFlashNvStorageVariableBase64) + PcdGet32(PcdFlashNvStorageVariableSize) == PcdGet64(PcdFlashNvStorageFtwWorkingBase64));
+  } else {
+    ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) + PcdGet32(PcdFlashNvStorageVariableSize) == PcdGet32(PcdFlashNvStorageFtwWorkingBase));
+  }
+
+  if (PcdGet64 (PcdFlashNvStorageFtwWorkingBase64) != 0) {
+    ASSERT(PcdGet64(PcdFlashNvStorageFtwWorkingBase64) + PcdGet32(PcdFlashNvStorageFtwWorkingSize) == PcdGet64(PcdFlashNvStorageFtwSpareBase64));
+  } else {
+    ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) + PcdGet32(PcdFlashNvStorageFtwWorkingSize) == PcdGet32(PcdFlashNvStorageFtwSpareBase));
+  }
 
   // Check if the size of the area is at least one block size
   ASSERT((PcdGet32(PcdFlashNvStorageVariableSize) > 0) && (PcdGet32(PcdFlashNvStorageVariableSize) / Instance->Media.BlockSize > 0));
@@ -67,9 +76,16 @@ InitializeFvAndVariableStoreHeaders (
   ASSERT((PcdGet32(PcdFlashNvStorageFtwSpareSize) > 0) && (PcdGet32(PcdFlashNvStorageFtwSpareSize) / Instance->Media.BlockSize > 0));
 
   // 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 (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
+    ASSERT(PcdGet64(PcdFlashNvStorageVariableBase64) % Instance->Media.BlockSize == 0);
+    ASSERT(PcdGet64(PcdFlashNvStorageFtwWorkingBase64) % Instance->Media.BlockSize == 0);
+    ASSERT(PcdGet64(PcdFlashNvStorageFtwSpareBase64) % Instance->Media.BlockSize == 0);
+  }
+  else {
+    ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) % Instance->Media.BlockSize == 0);
+    ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) % Instance->Media.BlockSize == 0);
+    ASSERT(PcdGet32(PcdFlashNvStorageFtwSpareBase) % Instance->Media.BlockSize == 0);
+  }
 
   //
   // EFI_FIRMWARE_VOLUME_HEADER
@@ -736,10 +752,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



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67973): https://edk2.groups.io/g/devel/message/67973
Mute This Topic: https://groups.io/mt/78499161/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] ArmPlatformPkg: Enable support for flash in 64-bit address space

Posted by Sami Mujawar 3 years, 5 months ago

Hi Vijay,

Thank you for this patch.

Please see my response inline marked [SAMI].

Regards,

Sami Mujawar

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Vijayenthiran Subramaniam via groups.io
Sent: 25 November 2020 01:30 PM
To: devel@edk2.groups.io; leif@nuviainc.com; Ard Biesheuvel <Ard.Biesheuvel@arm.com>
Cc: Thomas Abraham <thomas.abraham@arm.com>; Aditya Angadi <Aditya.Angadi@arm.com>
Subject: [edk2-devel] [PATCH] 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.

Signed-off-by: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>
---
 .../Drivers/NorFlashDxe/NorFlashDxe.c         | 13 ++++++--
 .../Drivers/NorFlashDxe/NorFlashDxe.inf       |  3 ++
 .../Drivers/NorFlashDxe/NorFlashFvbDxe.c      | 31 ++++++++++++++-----
 3 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
index d9e196cbf1..f3fbbafb7d 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
@@ -1298,9 +1298,16 @@ 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);
+  }
[SAMI] Since you are updating the code here, can you limit the maximum line length, please? 
I prefer 80 chars but coding guidelines allows up to 120 see https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/3_quick_reference#3-3-files-general-rules
[/SAMI]
 
     Status = NorFlashCreateInstance (
       NorFlashDevices[Index].DeviceBaseAddress,
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
index a647c01687..b2a941d672 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
@@ -54,10 +54,13 @@
   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/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
index 9cdd85096a..ecbe009495 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
@@ -58,8 +58,17 @@ InitializeFvAndVariableStoreHeaders (
   Headers = AllocateZeroPool(HeadersLength);
 
   // 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 (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
+    ASSERT(PcdGet64(PcdFlashNvStorageVariableBase64) + PcdGet32(PcdFlashNvStorageVariableSize) == PcdGet64(PcdFlashNvStorageFtwWorkingBase64));

[SAMI] Please put a space between PcdGetxx and '('. Same applies to ASSERT () as well.
[/SAMI] 

+  } else {
+    ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) + PcdGet32(PcdFlashNvStorageVariableSize) == PcdGet32(PcdFlashNvStorageFtwWorkingBase));
+  }
+
+  if (PcdGet64 (PcdFlashNvStorageFtwWorkingBase64) != 0) {
+    ASSERT(PcdGet64(PcdFlashNvStorageFtwWorkingBase64) + PcdGet32(PcdFlashNvStorageFtwWorkingSize) == PcdGet64(PcdFlashNvStorageFtwSpareBase64));
+  } else {
+    ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) + PcdGet32(PcdFlashNvStorageFtwWorkingSize) == PcdGet32(PcdFlashNvStorageFtwSpareBase));
+  }


   // Check if the size of the area is at least one block size
   ASSERT((PcdGet32(PcdFlashNvStorageVariableSize) > 0) && (PcdGet32(PcdFlashNvStorageVariableSize) / Instance->Media.BlockSize > 0));
@@ -67,9 +76,16 @@ InitializeFvAndVariableStoreHeaders (
   ASSERT((PcdGet32(PcdFlashNvStorageFtwSpareSize) > 0) && (PcdGet32(PcdFlashNvStorageFtwSpareSize) / Instance->Media.BlockSize > 0));
 
   // 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 (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
+    ASSERT(PcdGet64(PcdFlashNvStorageVariableBase64) % Instance->Media.BlockSize == 0);
+    ASSERT(PcdGet64(PcdFlashNvStorageFtwWorkingBase64) % Instance->Media.BlockSize == 0);
+    ASSERT(PcdGet64(PcdFlashNvStorageFtwSpareBase64) % Instance->Media.BlockSize == 0);
+  }
+  else {

[SAMI] Move else to previous line e.g. '} else {' 
[/SAMI]

+    ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) % Instance->Media.BlockSize == 0);
+    ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) % Instance->Media.BlockSize == 0);
+    ASSERT(PcdGet32(PcdFlashNvStorageFtwSpareBase) % Instance->Media.BlockSize == 0);
+  }

[SAMI]  This entire code block from line 60 to 90 is asserting when an invalid parameter is detected so could ideally be in DEBUG_CODE ().
But more importantly, in release builds the asserts would be turned off leading to incorrect values being used.  I think this code can be improved to return an error code if a parameter is invalid. This would be useful as some of these PCDs are dynamic. 
[/SAMI]

 
   //
   // EFI_FIRMWARE_VOLUME_HEADER
@@ -736,10 +752,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








-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67980): https://edk2.groups.io/g/devel/message/67980
Mute This Topic: https://groups.io/mt/78499161/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] ArmPlatformPkg: Enable support for flash in 64-bit address space

Posted by Vijayenthiran Subramaniam 3 years, 4 months ago

Hi Sami,

On Wed, Nov 25, 2020 at 2:45 PM Sami Mujawar <sami.mujawar@arm.com> wrote:
>
> Hi Vijay,
>
> Thank you for this patch.
>
> Please see my response inline marked [SAMI].
>
> Regards,
>
> Sami Mujawar
>
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Vijayenthiran Subramaniam via groups.io
> Sent: 25 November 2020 01:30 PM
> To: devel@edk2.groups.io; leif@nuviainc.com; Ard Biesheuvel <Ard.Biesheuvel@arm.com>
> Cc: Thomas Abraham <thomas.abraham@arm.com>; Aditya Angadi <Aditya.Angadi@arm.com>
> Subject: [edk2-devel] [PATCH] 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.
>
> Signed-off-by: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>
> ---
>  .../Drivers/NorFlashDxe/NorFlashDxe.c         | 13 ++++++--
>  .../Drivers/NorFlashDxe/NorFlashDxe.inf       |  3 ++
>  .../Drivers/NorFlashDxe/NorFlashFvbDxe.c      | 31 ++++++++++++++-----
>  3 files changed, 37 insertions(+), 10 deletions(-)
>
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> index d9e196cbf1..f3fbbafb7d 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> @@ -1298,9 +1298,16 @@ 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);
> +  }
> [SAMI] Since you are updating the code here, can you limit the maximum line length, please?
> I prefer 80 chars but coding guidelines allows up to 120 see https://edk2-docs.gitbook.io/edk-ii-c-coding-standards-specification/3_quick_reference#3-3-files-general-rules
> [/SAMI]
>
>      Status = NorFlashCreateInstance (
>        NorFlashDevices[Index].DeviceBaseAddress,
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> index a647c01687..b2a941d672 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> @@ -54,10 +54,13 @@
>    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/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> index 9cdd85096a..ecbe009495 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> @@ -58,8 +58,17 @@ InitializeFvAndVariableStoreHeaders (
>    Headers = AllocateZeroPool(HeadersLength);
>
>    // 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 (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
> +    ASSERT(PcdGet64(PcdFlashNvStorageVariableBase64) + PcdGet32(PcdFlashNvStorageVariableSize) == PcdGet64(PcdFlashNvStorageFtwWorkingBase64));
>
> [SAMI] Please put a space between PcdGetxx and '('. Same applies to ASSERT () as well.
> [/SAMI]
>
> +  } else {
> +    ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) + PcdGet32(PcdFlashNvStorageVariableSize) == PcdGet32(PcdFlashNvStorageFtwWorkingBase));
> +  }
> +
> +  if (PcdGet64 (PcdFlashNvStorageFtwWorkingBase64) != 0) {
> +    ASSERT(PcdGet64(PcdFlashNvStorageFtwWorkingBase64) + PcdGet32(PcdFlashNvStorageFtwWorkingSize) == PcdGet64(PcdFlashNvStorageFtwSpareBase64));
> +  } else {
> +    ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) + PcdGet32(PcdFlashNvStorageFtwWorkingSize) == PcdGet32(PcdFlashNvStorageFtwSpareBase));
> +  }
>
>
>    // Check if the size of the area is at least one block size
>    ASSERT((PcdGet32(PcdFlashNvStorageVariableSize) > 0) && (PcdGet32(PcdFlashNvStorageVariableSize) / Instance->Media.BlockSize > 0));
> @@ -67,9 +76,16 @@ InitializeFvAndVariableStoreHeaders (
>    ASSERT((PcdGet32(PcdFlashNvStorageFtwSpareSize) > 0) && (PcdGet32(PcdFlashNvStorageFtwSpareSize) / Instance->Media.BlockSize > 0));
>
>    // 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 (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
> +    ASSERT(PcdGet64(PcdFlashNvStorageVariableBase64) % Instance->Media.BlockSize == 0);
> +    ASSERT(PcdGet64(PcdFlashNvStorageFtwWorkingBase64) % Instance->Media.BlockSize == 0);
> +    ASSERT(PcdGet64(PcdFlashNvStorageFtwSpareBase64) % Instance->Media.BlockSize == 0);
> +  }
> +  else {
>
> [SAMI] Move else to previous line e.g. '} else {'
> [/SAMI]
>
> +    ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) % Instance->Media.BlockSize == 0);
> +    ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) % Instance->Media.BlockSize == 0);
> +    ASSERT(PcdGet32(PcdFlashNvStorageFtwSpareBase) % Instance->Media.BlockSize == 0);
> +  }
>
> [SAMI]  This entire code block from line 60 to 90 is asserting when an invalid parameter is detected so could ideally be in DEBUG_CODE ().
> But more importantly, in release builds the asserts would be turned off leading to incorrect values being used.  I think this code can be improved to return an error code if a parameter is invalid. This would be useful as some of these PCDs are dynamic.
> [/SAMI]
>
>
>    //
>    // EFI_FIRMWARE_VOLUME_HEADER
> @@ -736,10 +752,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
>

Thanks for the review. I will address the changes in the next version
of the patch.

Regards,
Vijay


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#68078): https://edk2.groups.io/g/devel/message/68078
Mute This Topic: https://groups.io/mt/78499161/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] ArmPlatformPkg: Enable support for flash in 64-bit address space

Posted by Ard Biesheuvel 3 years, 5 months ago

On 11/25/20 2:29 PM, 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.
> 

Why?

> Signed-off-by: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>
> ---
>   .../Drivers/NorFlashDxe/NorFlashDxe.c         | 13 ++++++--
>   .../Drivers/NorFlashDxe/NorFlashDxe.inf       |  3 ++
>   .../Drivers/NorFlashDxe/NorFlashFvbDxe.c      | 31 ++++++++++++++-----
>   3 files changed, 37 insertions(+), 10 deletions(-)
> 
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> index d9e196cbf1..f3fbbafb7d 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> @@ -1298,9 +1298,16 @@ 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/NorFlashDxe.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> index a647c01687..b2a941d672 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> @@ -54,10 +54,13 @@
>     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/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> index 9cdd85096a..ecbe009495 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> @@ -58,8 +58,17 @@ InitializeFvAndVariableStoreHeaders (
>     Headers = AllocateZeroPool(HeadersLength);
>   
>     // 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 (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
> +    ASSERT(PcdGet64(PcdFlashNvStorageVariableBase64) + PcdGet32(PcdFlashNvStorageVariableSize) == PcdGet64(PcdFlashNvStorageFtwWorkingBase64));
> +  } else {
> +    ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) + PcdGet32(PcdFlashNvStorageVariableSize) == PcdGet32(PcdFlashNvStorageFtwWorkingBase));
> +  }
> +
> +  if (PcdGet64 (PcdFlashNvStorageFtwWorkingBase64) != 0) {
> +    ASSERT(PcdGet64(PcdFlashNvStorageFtwWorkingBase64) + PcdGet32(PcdFlashNvStorageFtwWorkingSize) == PcdGet64(PcdFlashNvStorageFtwSpareBase64));
> +  } else {
> +    ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) + PcdGet32(PcdFlashNvStorageFtwWorkingSize) == PcdGet32(PcdFlashNvStorageFtwSpareBase));
> +  }
>   
>     // Check if the size of the area is at least one block size
>     ASSERT((PcdGet32(PcdFlashNvStorageVariableSize) > 0) && (PcdGet32(PcdFlashNvStorageVariableSize) / Instance->Media.BlockSize > 0));
> @@ -67,9 +76,16 @@ InitializeFvAndVariableStoreHeaders (
>     ASSERT((PcdGet32(PcdFlashNvStorageFtwSpareSize) > 0) && (PcdGet32(PcdFlashNvStorageFtwSpareSize) / Instance->Media.BlockSize > 0));
>   
>     // 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 (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
> +    ASSERT(PcdGet64(PcdFlashNvStorageVariableBase64) % Instance->Media.BlockSize == 0);
> +    ASSERT(PcdGet64(PcdFlashNvStorageFtwWorkingBase64) % Instance->Media.BlockSize == 0);
> +    ASSERT(PcdGet64(PcdFlashNvStorageFtwSpareBase64) % Instance->Media.BlockSize == 0);
> +  }
> +  else {
> +    ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) % Instance->Media.BlockSize == 0);
> +    ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) % Instance->Media.BlockSize == 0);
> +    ASSERT(PcdGet32(PcdFlashNvStorageFtwSpareBase) % Instance->Media.BlockSize == 0);
> +  }
>   
>     //
>     // EFI_FIRMWARE_VOLUME_HEADER
> @@ -736,10 +752,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) {
> 



-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#67982): https://edk2.groups.io/g/devel/message/67982
Mute This Topic: https://groups.io/mt/78499161/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-

Re: [edk2-devel] [PATCH] ArmPlatformPkg: Enable support for flash in 64-bit address space

Posted by Vijayenthiran Subramaniam 3 years, 4 months ago

Hi Ard,

On Wed, Nov 25, 2020 at 2:59 PM Ard Biesheuvel <ard.biesheuvel@arm.com> wrote:
>
> On 11/25/20 2:29 PM, 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.
> >
>
> Why?

There’s a derivative SGI/RD platform which has NOR flash connected to the 64-bit
MMIO space. We are planning to upstream support for the platform to
edk2-platform’s SgiPkg in the upcoming days. As a preparation, this patch
updates the NorFlashDxe to support 64-bit addresses.

Regards,
Vijayenthiran

>
> > Signed-off-by: Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>
> > ---
> >   .../Drivers/NorFlashDxe/NorFlashDxe.c         | 13 ++++++--
> >   .../Drivers/NorFlashDxe/NorFlashDxe.inf       |  3 ++
> >   .../Drivers/NorFlashDxe/NorFlashFvbDxe.c      | 31 ++++++++++++++-----
> >   3 files changed, 37 insertions(+), 10 deletions(-)
> >
> > diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> > index d9e196cbf1..f3fbbafb7d 100644
> > --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> > +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> > @@ -1298,9 +1298,16 @@ 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/NorFlashDxe.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> > index a647c01687..b2a941d672 100644
> > --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> > +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> > @@ -54,10 +54,13 @@
> >     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/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> > index 9cdd85096a..ecbe009495 100644
> > --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> > +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
> > @@ -58,8 +58,17 @@ InitializeFvAndVariableStoreHeaders (
> >     Headers = AllocateZeroPool(HeadersLength);
> >
> >     // 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 (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
> > +    ASSERT(PcdGet64(PcdFlashNvStorageVariableBase64) + PcdGet32(PcdFlashNvStorageVariableSize) == PcdGet64(PcdFlashNvStorageFtwWorkingBase64));
> > +  } else {
> > +    ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) + PcdGet32(PcdFlashNvStorageVariableSize) == PcdGet32(PcdFlashNvStorageFtwWorkingBase));
> > +  }
> > +
> > +  if (PcdGet64 (PcdFlashNvStorageFtwWorkingBase64) != 0) {
> > +    ASSERT(PcdGet64(PcdFlashNvStorageFtwWorkingBase64) + PcdGet32(PcdFlashNvStorageFtwWorkingSize) == PcdGet64(PcdFlashNvStorageFtwSpareBase64));
> > +  } else {
> > +    ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) + PcdGet32(PcdFlashNvStorageFtwWorkingSize) == PcdGet32(PcdFlashNvStorageFtwSpareBase));
> > +  }
> >
> >     // Check if the size of the area is at least one block size
> >     ASSERT((PcdGet32(PcdFlashNvStorageVariableSize) > 0) && (PcdGet32(PcdFlashNvStorageVariableSize) / Instance->Media.BlockSize > 0));
> > @@ -67,9 +76,16 @@ InitializeFvAndVariableStoreHeaders (
> >     ASSERT((PcdGet32(PcdFlashNvStorageFtwSpareSize) > 0) && (PcdGet32(PcdFlashNvStorageFtwSpareSize) / Instance->Media.BlockSize > 0));
> >
> >     // 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 (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
> > +    ASSERT(PcdGet64(PcdFlashNvStorageVariableBase64) % Instance->Media.BlockSize == 0);
> > +    ASSERT(PcdGet64(PcdFlashNvStorageFtwWorkingBase64) % Instance->Media.BlockSize == 0);
> > +    ASSERT(PcdGet64(PcdFlashNvStorageFtwSpareBase64) % Instance->Media.BlockSize == 0);
> > +  }
> > +  else {
> > +    ASSERT(PcdGet32(PcdFlashNvStorageVariableBase) % Instance->Media.BlockSize == 0);
> > +    ASSERT(PcdGet32(PcdFlashNvStorageFtwWorkingBase) % Instance->Media.BlockSize == 0);
> > +    ASSERT(PcdGet32(PcdFlashNvStorageFtwSpareBase) % Instance->Media.BlockSize == 0);
> > +  }
> >
> >     //
> >     // EFI_FIRMWARE_VOLUME_HEADER
> > @@ -736,10 +752,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) {
> >


-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#68079): https://edk2.groups.io/g/devel/message/68079
Mute This Topic: https://groups.io/mt/78499161/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-