[edk2-devel] [PATCH] ArmPlatformPkg/NorFlashDxe: use correct PCD accessors
Laszlo Ersek
lersek at redhat.com
Mon Jan 11 15:54:23 UTC 2021
On 01/11/21 11:57, Ard Biesheuvel wrote:
> Commit 8015f3f6d4005d83 ("ArmPlatformPkg: Enable support for flash in
> 64-bit address space") updated the NorFlash DXE and StMM drivers to
> take alternate PCDs into account when discovering the base of the
> NOR flash regions.
>
> This introduced a disparity between the declarations of the PCD references
> in the .INF files, which permits the use of dynamic PCDs, and the code
> itself, which now uses FixedPcdGet() accessors. On platforms that actually
> use dynamic PCDs, this results in a build error.
>
> So let's clean this up:
> - for the DXE version, use the generic PcdGet() accessors, so dynamic PCDs
> are permitted
> - for the standalone MM version, redeclare the PCDs as [FixedPcd] in the
> .INF description, and switch to the FixedPcdGet() accessors.
>
> Cc: Vijayenthiran Subramaniam <vijayenthiran.subramaniam at arm.com>
> Cc: Masahisa Kojima <masahisa.kojima at linaro.org>
> Cc: Sami Mujawar <sami.mujawar at arm.com>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel at arm.com>
> ---
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf | 3 ++-
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c | 4 ++--
> ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c | 10 +++++-----
> 3 files changed, 9 insertions(+), 8 deletions(-)
Looks justified to me.
Acked-by: Laszlo Ersek <lersek at redhat.com>
Thanks,
Laszlo
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> index b2f72fb4de20..56347a8bc7c1 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.inf
> @@ -49,7 +49,7 @@ [Guids]
> [Protocols]
> gEfiSmmFirmwareVolumeBlockProtocolGuid
>
> -[Pcd.common]
> +[FixedPcd]
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase64
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableBase
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageVariableSize
> @@ -60,6 +60,7 @@ [Pcd.common]
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
>
> +[FeaturePcd]
> gArmPlatformTokenSpaceGuid.PcdNorFlashCheckBlockLocked
>
> [Depex]
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> index 28dc8e125c78..f412731200cf 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
> @@ -422,8 +422,8 @@ NorFlashFvbInitialize (
> EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
> ASSERT_EFI_ERROR (Status);
>
> - mFlashNvStorageVariableBase = (FixedPcdGet64 (PcdFlashNvStorageVariableBase64) != 0) ?
> - FixedPcdGet64 (PcdFlashNvStorageVariableBase64) : FixedPcdGet32 (PcdFlashNvStorageVariableBase);
> + mFlashNvStorageVariableBase = (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) ?
> + PcdGet64 (PcdFlashNvStorageVariableBase64) : PcdGet32 (PcdFlashNvStorageVariableBase);
>
> // Set the index of the first LBA for the FVB
> Instance->StartLba = (mFlashNvStorageVariableBase - Instance->RegionBaseAddress) / Instance->Media.BlockSize;
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
> index 8a4fb395d286..4ebbc06e1de3 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashStandaloneMm.c
> @@ -299,15 +299,15 @@ NorFlashInitialise (
> for (Index = 0; Index < mNorFlashDeviceCount; Index++) {
> // Check if this NOR Flash device contain the variable storage region
>
> - if (PcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
> + if (FixedPcdGet64 (PcdFlashNvStorageVariableBase64) != 0) {
> ContainVariableStorage =
> - (NorFlashDevices[Index].RegionBaseAddress <= PcdGet64 (PcdFlashNvStorageVariableBase64)) &&
> - (PcdGet64 (PcdFlashNvStorageVariableBase64) + PcdGet32 (PcdFlashNvStorageVariableSize) <=
> + (NorFlashDevices[Index].RegionBaseAddress <= FixedPcdGet64 (PcdFlashNvStorageVariableBase64)) &&
> + (FixedPcdGet64 (PcdFlashNvStorageVariableBase64) + FixedPcdGet32 (PcdFlashNvStorageVariableSize) <=
> NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
> } else {
> ContainVariableStorage =
> - (NorFlashDevices[Index].RegionBaseAddress <= PcdGet32 (PcdFlashNvStorageVariableBase)) &&
> - (PcdGet32 (PcdFlashNvStorageVariableBase) + PcdGet32 (PcdFlashNvStorageVariableSize) <=
> + (NorFlashDevices[Index].RegionBaseAddress <= FixedPcdGet32 (PcdFlashNvStorageVariableBase)) &&
> + (FixedPcdGet32 (PcdFlashNvStorageVariableBase) + FixedPcdGet32 (PcdFlashNvStorageVariableSize) <=
> NorFlashDevices[Index].RegionBaseAddress + NorFlashDevices[Index].Size);
> }
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#70122): https://edk2.groups.io/g/devel/message/70122
Mute This Topic: https://groups.io/mt/79593039/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-
More information about the edk2-devel-archive
mailing list