public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: Laszlo Ersek <lersek@redhat.com>
To: edk2-devel@lists.01.org
Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Leif Lindholm <leif.lindholm@linaro.org>,
	Steve Capper <steve.capper@linaro.org>,
	Supreeth Venkatesh <Supreeth.Venkatesh@arm.com>
Subject: [PATCH 05/10] ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly
Date: Thu, 12 Apr 2018 02:55:35 +0200	[thread overview]
Message-ID: <20180412005540.26651-6-lersek@redhat.com> (raw)
In-Reply-To: <20180412005540.26651-1-lersek@redhat.com>

The lazy initialization of the varstore FVB makes no longer sense at this
point:

- "mNorFlashInstanceTemplate.Initialize" is NULL;

- in NorFlashCreateInstance(), we only set Instance->Initialize to
  non-NULL -- namely NorFlashFvbInitialize() -- if the FVB stands for the
  variable store (see "ContainVariableStorage" / "SupportFvb");

- we call Instance->Initialize() from three places:

  - from NorFlashWriteSingleBlock(), which is too late for the variable
    read service ("variable write" depends on "variable read");

  - from InitializeFvAndVariableStoreHeaders(), but that is only reachable
    from NorFlashFvbInitialize(), i.e. recursively from
    Instance->Initialize() itself;

  - and from FvbRead(), which is never called by the variable driver, only
    by the FTW driver. However, the variable driver may read (not write)
    the memory-mapped varstore flash chip before the FTW driver is
    dispatched.

Therefore the lazy initialization is both superfluous and insufficient.
Initialize the varstore headers eagerly, before we install the FVB
protocol interface.

Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Cc: Leif Lindholm <leif.lindholm@linaro.org>
Cc: Steve Capper <steve.capper@linaro.org>
Cc: Supreeth Venkatesh <Supreeth.Venkatesh@arm.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h    |  6 ------
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c    | 13 +------------
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c |  9 ---------
 3 files changed, 1 insertion(+), 27 deletions(-)

diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h
index c24680098f62..5c07694fbfaa 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h
@@ -119,36 +119,30 @@
 #define INSTANCE_FROM_FVB_THIS(a)                 CR(a, NOR_FLASH_INSTANCE, FvbProtocol, NOR_FLASH_SIGNATURE)
 #define INSTANCE_FROM_BLKIO_THIS(a)               CR(a, NOR_FLASH_INSTANCE, BlockIoProtocol, NOR_FLASH_SIGNATURE)
 #define INSTANCE_FROM_DISKIO_THIS(a)              CR(a, NOR_FLASH_INSTANCE, DiskIoProtocol, NOR_FLASH_SIGNATURE)
 
 typedef struct _NOR_FLASH_INSTANCE                NOR_FLASH_INSTANCE;
 
-typedef EFI_STATUS (*NOR_FLASH_INITIALIZE)        (NOR_FLASH_INSTANCE* Instance);
-
 typedef struct {
   VENDOR_DEVICE_PATH                  Vendor;
   EFI_DEVICE_PATH_PROTOCOL            End;
 } NOR_FLASH_DEVICE_PATH;
 
 struct _NOR_FLASH_INSTANCE {
   UINT32                              Signature;
   EFI_HANDLE                          Handle;
 
-  BOOLEAN                             Initialized;
-  NOR_FLASH_INITIALIZE                Initialize;
-
   UINTN                               DeviceBaseAddress;
   UINTN                               RegionBaseAddress;
   UINTN                               Size;
   EFI_LBA                             StartLba;
 
   EFI_BLOCK_IO_PROTOCOL               BlockIoProtocol;
   EFI_BLOCK_IO_MEDIA                  Media;
   EFI_DISK_IO_PROTOCOL                DiskIoProtocol;
 
-  BOOLEAN                             SupportFvb;
   EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL FvbProtocol;
   VOID*                               ShadowBuffer;
 
   NOR_FLASH_DEVICE_PATH               DevicePath;
 };
 
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
index 1098d9501cc7..46e815beb343 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
@@ -29,15 +29,12 @@ NOR_FLASH_INSTANCE **mNorFlashInstances;
 UINT32               mNorFlashDeviceCount;
 
 NOR_FLASH_INSTANCE  mNorFlashInstanceTemplate = {
   NOR_FLASH_SIGNATURE, // Signature
   NULL, // Handle ... NEED TO BE FILLED
 
-  FALSE, // Initialized
-  NULL, // Initialize
-
   0, // DeviceBaseAddress ... NEED TO BE FILLED
   0, // RegionBaseAddress ... NEED TO BE FILLED
   0, // Size ... NEED TO BE FILLED
   0, // StartLba
 
   {
@@ -66,13 +63,12 @@ NOR_FLASH_INSTANCE  mNorFlashInstanceTemplate = {
   {
     EFI_DISK_IO_PROTOCOL_REVISION, // Revision
     NorFlashDiskIoReadDisk,        // ReadDisk
     NorFlashDiskIoWriteDisk        // WriteDisk
   },
 
-  FALSE, // SupportFvb ... NEED TO BE FILLED
   {
     FvbGetAttributes, // GetAttributes
     FvbSetAttributes, // SetAttributes
     FvbGetPhysicalAddress,  // GetPhysicalAddress
     FvbGetBlockSize,  // GetBlockSize
     FvbRead,  // Read
@@ -134,14 +130,13 @@ NorFlashCreateInstance (
   Instance->ShadowBuffer = AllocateRuntimePool (BlockSize);;
   if (Instance->ShadowBuffer == NULL) {
     return EFI_OUT_OF_RESOURCES;
   }
 
   if (SupportFvb) {
-    Instance->SupportFvb = TRUE;
-    Instance->Initialize = NorFlashFvbInitialize;
+    NorFlashFvbInitialize (Instance);
 
     Status = gBS->InstallMultipleProtocolInterfaces (
                   &Instance->Handle,
                   &gEfiDevicePathProtocolGuid, &Instance->DevicePath,
                   &gEfiBlockIoProtocolGuid,  &Instance->BlockIoProtocol,
                   &gEfiFirmwareVolumeBlockProtocolGuid, &Instance->FvbProtocol,
@@ -149,14 +144,12 @@ NorFlashCreateInstance (
                   );
     if (EFI_ERROR(Status)) {
       FreePool (Instance);
       return Status;
     }
   } else {
-    Instance->Initialized = TRUE;
-
     Status = gBS->InstallMultipleProtocolInterfaces (
                     &Instance->Handle,
                     &gEfiDevicePathProtocolGuid, &Instance->DevicePath,
                     &gEfiBlockIoProtocolGuid,  &Instance->BlockIoProtocol,
                     &gEfiDiskIoProtocolGuid, &Instance->DiskIoProtocol,
                     NULL
@@ -921,16 +914,12 @@ NorFlashWriteSingleBlock (
   UINTN       BlockSize;
   UINTN       BlockAddress;
   UINTN       PrevBlockAddress;
 
   PrevBlockAddress = 0;
 
-  if (!Instance->Initialized && Instance->Initialize) {
-    Instance->Initialize(Instance);
-  }
-
   DEBUG ((DEBUG_BLKIO, "NorFlashWriteSingleBlock(Parameters: Lba=%ld, Offset=0x%x, *NumBytes=0x%x, Buffer @ 0x%08x)\n", Lba, Offset, *NumBytes, Buffer));
 
   // Detect WriteDisabled state
   if (Instance->Media.ReadOnly == TRUE) {
     DEBUG ((EFI_D_ERROR, "NorFlashWriteSingleBlock: ERROR - Can not write: Device is in WriteDisabled state.\n"));
     // It is in WriteDisabled state, return an error right away
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
index 3ea858f46ffb..c3e6489f398f 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
@@ -56,16 +56,12 @@ InitializeFvAndVariableStoreHeaders (
   EFI_STATUS                          Status;
   VOID*                               Headers;
   UINTN                               HeadersLength;
   EFI_FIRMWARE_VOLUME_HEADER          *FirmwareVolumeHeader;
   VARIABLE_STORE_HEADER               *VariableStoreHeader;
 
-  if (!Instance->Initialized && Instance->Initialize) {
-    Instance->Initialize (Instance);
-  }
-
   HeadersLength = sizeof(EFI_FIRMWARE_VOLUME_HEADER) + sizeof(EFI_FV_BLOCK_MAP_ENTRY) + sizeof(VARIABLE_STORE_HEADER);
   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));
@@ -428,16 +424,12 @@ FvbRead (
   NOR_FLASH_INSTANCE *Instance;
 
   Instance = INSTANCE_FROM_FVB_THIS(This);
 
   DEBUG ((DEBUG_BLKIO, "FvbRead(Parameters: Lba=%ld, Offset=0x%x, *NumBytes=0x%x, Buffer @ 0x%08x)\n", Instance->StartLba + Lba, Offset, *NumBytes, Buffer));
 
-  if (!Instance->Initialized && Instance->Initialize) {
-    Instance->Initialize(Instance);
-  }
-
   TempStatus = EFI_SUCCESS;
 
   // Cache the block size to avoid de-referencing pointers all the time
   BlockSize = Instance->Media.BlockSize;
 
   DEBUG ((DEBUG_BLKIO, "FvbRead: Check if (Offset=0x%x + NumBytes=0x%x) <= BlockSize=0x%x\n", Offset, *NumBytes, BlockSize ));
@@ -746,13 +738,12 @@ NorFlashFvbInitialize (
 
   Status = gDS->SetMemorySpaceAttributes (
       Instance->DeviceBaseAddress, RuntimeMmioRegionSize,
       EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);
   ASSERT_EFI_ERROR (Status);
 
-  Instance->Initialized = TRUE;
   mFlashNvStorageVariableBase = FixedPcdGet32 (PcdFlashNvStorageVariableBase);
 
   // Set the index of the first LBA for the FVB
   Instance->StartLba = (PcdGet32 (PcdFlashNvStorageVariableBase) - Instance->RegionBaseAddress) / Instance->Media.BlockSize;
 
   BootMode = GetBootModeHob ();
-- 
2.14.1.3.gb7cf6e02401b




  parent reply	other threads:[~2018-04-12  0:55 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-12  0:55 [PATCH 00/10] ArmPkg, ArmPlatformPkg, ArmVirtPkg, EmbeddedPkg, Omap35xxPkg: depex fixes Laszlo Ersek
2018-04-12  0:55 ` [PATCH 01/10] Omap35xxPkg/InterruptDxe: replace CPU Arch Protocol depex with notify Laszlo Ersek
2018-04-12  0:55 ` [PATCH 02/10] ArmPkg/ArmGicDxe: annotate protocol usage in "ArmGicDxe.inf" Laszlo Ersek
2018-04-12  0:55 ` [PATCH 03/10] ArmPkg/CpuDxe: order CpuDxe after ArmGicDxe via protocol depex Laszlo Ersek
2018-04-12  0:55 ` [PATCH 04/10] EmbeddedPkg: introduce NvVarStoreFormattedLib Laszlo Ersek
2018-04-12  0:55 ` Laszlo Ersek [this message]
2018-04-12  0:55 ` [PATCH 06/10] ArmPlatformPkg/NorFlashDxe: cue the variable driver with NvVarStoreFormatted Laszlo Ersek
2018-04-12  0:55 ` [PATCH 07/10] ArmPlatformPkg/NorFlashDxe: depend on gEfiCpuArchProtocolGuid Laszlo Ersek
2018-04-12  0:55 ` [PATCH 08/10] ArmPlatformPkg/PL031RealTimeClockLib: " Laszlo Ersek
2018-04-12  0:55 ` [PATCH 09/10] ArmVirtPkg/PlatformHasAcpiDtDxe: depend on gEfiVariableArchProtocolGuid Laszlo Ersek
2018-04-12  6:28   ` Ard Biesheuvel
2018-04-12  9:05     ` Laszlo Ersek
2018-04-12 10:06       ` Ard Biesheuvel
2018-04-12 15:16       ` Gao, Liming
2018-04-12 16:53         ` Laszlo Ersek
2018-04-12  0:55 ` [PATCH 10/10] ArmVirtPkg/ArmVirtQemu: hook NvVarStoreFormattedLib into VariableRuntimeDxe Laszlo Ersek
2018-04-12 10:09 ` [PATCH 00/10] ArmPkg, ArmPlatformPkg, ArmVirtPkg, EmbeddedPkg, Omap35xxPkg: depex fixes Ard Biesheuvel
2018-04-12 13:39   ` Steve Capper
2018-04-12 16:49     ` Laszlo Ersek
2018-04-12 16:44   ` Laszlo Ersek
2018-04-12 17:23   ` Leif Lindholm
2018-04-12 17:45     ` Laszlo Ersek
2018-04-12 18:13       ` derailing into patch style discussion Leif Lindholm
2018-04-12 18:48         ` Laszlo Ersek
2018-04-12 16:51 ` [PATCH 00/10] ArmPkg, ArmPlatformPkg, ArmVirtPkg, EmbeddedPkg, Omap35xxPkg: depex fixes Supreeth Venkatesh
2018-04-12 17:46   ` Laszlo Ersek
2018-04-12 19:29 ` Laszlo Ersek

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=20180412005540.26651-6-lersek@redhat.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