* [PATCH] ArmPlatformPkg: Enable support for flash in 64-bit address space
@ 2020-11-25 13:29 Vijayenthiran Subramaniam
2020-11-25 14:44 ` [edk2-devel] " Sami Mujawar
2020-11-25 14:59 ` Ard Biesheuvel
0 siblings, 2 replies; 5+ messages in thread
From: Vijayenthiran Subramaniam @ 2020-11-25 13:29 UTC (permalink / raw)
To: devel, leif, Ard.Biesheuvel; +Cc: thomas.abraham, Aditya.Angadi
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH] ArmPlatformPkg: Enable support for flash in 64-bit address space
2020-11-25 13:29 [PATCH] ArmPlatformPkg: Enable support for flash in 64-bit address space Vijayenthiran Subramaniam
@ 2020-11-25 14:44 ` Sami Mujawar
2020-11-30 13:43 ` Vijayenthiran Subramaniam
2020-11-25 14:59 ` Ard Biesheuvel
1 sibling, 1 reply; 5+ messages in thread
From: Sami Mujawar @ 2020-11-25 14:44 UTC (permalink / raw)
To: devel@edk2.groups.io, Vijayenthiran Subramaniam,
leif@nuviainc.com, Ard Biesheuvel
Cc: Thomas Abraham, Aditya Angadi, nd
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
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] ArmPlatformPkg: Enable support for flash in 64-bit address space
2020-11-25 13:29 [PATCH] ArmPlatformPkg: Enable support for flash in 64-bit address space Vijayenthiran Subramaniam
2020-11-25 14:44 ` [edk2-devel] " Sami Mujawar
@ 2020-11-25 14:59 ` Ard Biesheuvel
2020-11-30 13:48 ` [edk2-devel] " Vijayenthiran Subramaniam
1 sibling, 1 reply; 5+ messages in thread
From: Ard Biesheuvel @ 2020-11-25 14:59 UTC (permalink / raw)
To: Vijayenthiran Subramaniam, devel, leif; +Cc: thomas.abraham, Aditya.Angadi
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) {
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH] ArmPlatformPkg: Enable support for flash in 64-bit address space
2020-11-25 14:44 ` [edk2-devel] " Sami Mujawar
@ 2020-11-30 13:43 ` Vijayenthiran Subramaniam
0 siblings, 0 replies; 5+ messages in thread
From: Vijayenthiran Subramaniam @ 2020-11-30 13:43 UTC (permalink / raw)
To: devel, sami.mujawar
Cc: Vijayenthiran Subramaniam, leif@nuviainc.com, Ard Biesheuvel,
Thomas Abraham, Aditya Angadi
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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [edk2-devel] [PATCH] ArmPlatformPkg: Enable support for flash in 64-bit address space
2020-11-25 14:59 ` Ard Biesheuvel
@ 2020-11-30 13:48 ` Vijayenthiran Subramaniam
0 siblings, 0 replies; 5+ messages in thread
From: Vijayenthiran Subramaniam @ 2020-11-30 13:48 UTC (permalink / raw)
To: devel, ard.biesheuvel
Cc: Vijayenthiran Subramaniam, leif, thomas.abraham, Aditya.Angadi,
sami.mujawar
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) {
> >
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-11-30 13:48 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-11-25 13:29 [PATCH] ArmPlatformPkg: Enable support for flash in 64-bit address space Vijayenthiran Subramaniam
2020-11-25 14:44 ` [edk2-devel] " Sami Mujawar
2020-11-30 13:43 ` Vijayenthiran Subramaniam
2020-11-25 14:59 ` Ard Biesheuvel
2020-11-30 13:48 ` [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