public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Ard Biesheuvel" <ard.biesheuvel@arm.com>
To: devel@edk2.groups.io
Cc: leif@nuviainc.com, Ard Biesheuvel <ard.biesheuvel@arm.com>,
	Vijayenthiran Subramaniam <vijayenthiran.subramaniam@arm.com>,
	Masahisa Kojima <masahisa.kojima@linaro.org>,
	Sami Mujawar <sami.mujawar@arm.com>
Subject: [PATCH] ArmPlatformPkg/NorFlashDxe: use correct PCD accessors
Date: Mon, 11 Jan 2021 11:57:19 +0100	[thread overview]
Message-ID: <20210111105719.18700-1-ard.biesheuvel@arm.com> (raw)

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@arm.com>
Cc: Masahisa Kojima <masahisa.kojima@linaro.org>
Cc: Sami Mujawar <sami.mujawar@arm.com>
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@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(-)

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);
   }
 
-- 
2.17.1


             reply	other threads:[~2021-01-11 10:57 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-11 10:57 Ard Biesheuvel [this message]
2021-01-11 15:54 ` [edk2-devel] [PATCH] ArmPlatformPkg/NorFlashDxe: use correct PCD accessors Laszlo Ersek
2021-01-12 11:13 ` Vijayenthiran Subramaniam
2021-01-12 11:42 ` Philippe Mathieu-Daudé
2021-01-12 12:10   ` Ard Biesheuvel
2021-01-12 12:20     ` Sami Mujawar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210111105719.18700-1-ard.biesheuvel@arm.com \
    --to=devel@edk2.groups.io \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox