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

["Ard Biesheuvel" <ard.biesheuvel@arm.com>]

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) {
>