ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly
authorLaszlo Ersek <lersek@redhat.com>
Wed, 11 Apr 2018 20:18:24 +0000 (22:18 +0200)
committerLaszlo Ersek <lersek@redhat.com>
Thu, 12 Apr 2018 19:24:32 +0000 (21:24 +0200)
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>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Tested-by: Steve Capper <steve.capper@linaro.org>
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h
ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c

index 1098d9501cc7ddd4ebde01995100da6e6ae86f6e..46e815beb34306eb51d25e7922ed5555a6bcfeec 100644 (file)
@@ -32,9 +32,6 @@ NOR_FLASH_INSTANCE  mNorFlashInstanceTemplate = {
   NOR_FLASH_SIGNATURE, // Signature\r
   NULL, // Handle ... NEED TO BE FILLED\r
 \r
-  FALSE, // Initialized\r
-  NULL, // Initialize\r
-\r
   0, // DeviceBaseAddress ... NEED TO BE FILLED\r
   0, // RegionBaseAddress ... NEED TO BE FILLED\r
   0, // Size ... NEED TO BE FILLED\r
@@ -69,7 +66,6 @@ NOR_FLASH_INSTANCE  mNorFlashInstanceTemplate = {
     NorFlashDiskIoWriteDisk        // WriteDisk\r
   },\r
 \r
-  FALSE, // SupportFvb ... NEED TO BE FILLED\r
   {\r
     FvbGetAttributes, // GetAttributes\r
     FvbSetAttributes, // SetAttributes\r
@@ -137,8 +133,7 @@ NorFlashCreateInstance (
   }\r
 \r
   if (SupportFvb) {\r
-    Instance->SupportFvb = TRUE;\r
-    Instance->Initialize = NorFlashFvbInitialize;\r
+    NorFlashFvbInitialize (Instance);\r
 \r
     Status = gBS->InstallMultipleProtocolInterfaces (\r
                   &Instance->Handle,\r
@@ -152,8 +147,6 @@ NorFlashCreateInstance (
       return Status;\r
     }\r
   } else {\r
-    Instance->Initialized = TRUE;\r
-\r
     Status = gBS->InstallMultipleProtocolInterfaces (\r
                     &Instance->Handle,\r
                     &gEfiDevicePathProtocolGuid, &Instance->DevicePath,\r
@@ -924,10 +917,6 @@ NorFlashWriteSingleBlock (
 \r
   PrevBlockAddress = 0;\r
 \r
-  if (!Instance->Initialized && Instance->Initialize) {\r
-    Instance->Initialize(Instance);\r
-  }\r
-\r
   DEBUG ((DEBUG_BLKIO, "NorFlashWriteSingleBlock(Parameters: Lba=%ld, Offset=0x%x, *NumBytes=0x%x, Buffer @ 0x%08x)\n", Lba, Offset, *NumBytes, Buffer));\r
 \r
   // Detect WriteDisabled state\r
index c24680098f629541fe823a7356c778e7819f7907..5c07694fbfaa18140730a60171e60b4df43d635e 100644 (file)
 \r
 typedef struct _NOR_FLASH_INSTANCE                NOR_FLASH_INSTANCE;\r
 \r
-typedef EFI_STATUS (*NOR_FLASH_INITIALIZE)        (NOR_FLASH_INSTANCE* Instance);\r
-\r
 typedef struct {\r
   VENDOR_DEVICE_PATH                  Vendor;\r
   EFI_DEVICE_PATH_PROTOCOL            End;\r
@@ -133,9 +131,6 @@ struct _NOR_FLASH_INSTANCE {
   UINT32                              Signature;\r
   EFI_HANDLE                          Handle;\r
 \r
-  BOOLEAN                             Initialized;\r
-  NOR_FLASH_INITIALIZE                Initialize;\r
-\r
   UINTN                               DeviceBaseAddress;\r
   UINTN                               RegionBaseAddress;\r
   UINTN                               Size;\r
@@ -145,7 +140,6 @@ struct _NOR_FLASH_INSTANCE {
   EFI_BLOCK_IO_MEDIA                  Media;\r
   EFI_DISK_IO_PROTOCOL                DiskIoProtocol;\r
 \r
-  BOOLEAN                             SupportFvb;\r
   EFI_FIRMWARE_VOLUME_BLOCK2_PROTOCOL FvbProtocol;\r
   VOID*                               ShadowBuffer;\r
 \r
index 3ea858f46ffba8244b98cce7feba32dd3d741ed9..c3e6489f398f896811d16999cb3a09292425329f 100644 (file)
@@ -59,10 +59,6 @@ InitializeFvAndVariableStoreHeaders (
   EFI_FIRMWARE_VOLUME_HEADER          *FirmwareVolumeHeader;\r
   VARIABLE_STORE_HEADER               *VariableStoreHeader;\r
 \r
-  if (!Instance->Initialized && Instance->Initialize) {\r
-    Instance->Initialize (Instance);\r
-  }\r
-\r
   HeadersLength = sizeof(EFI_FIRMWARE_VOLUME_HEADER) + sizeof(EFI_FV_BLOCK_MAP_ENTRY) + sizeof(VARIABLE_STORE_HEADER);\r
   Headers = AllocateZeroPool(HeadersLength);\r
 \r
@@ -431,10 +427,6 @@ FvbRead (
 \r
   DEBUG ((DEBUG_BLKIO, "FvbRead(Parameters: Lba=%ld, Offset=0x%x, *NumBytes=0x%x, Buffer @ 0x%08x)\n", Instance->StartLba + Lba, Offset, *NumBytes, Buffer));\r
 \r
-  if (!Instance->Initialized && Instance->Initialize) {\r
-    Instance->Initialize(Instance);\r
-  }\r
-\r
   TempStatus = EFI_SUCCESS;\r
 \r
   // Cache the block size to avoid de-referencing pointers all the time\r
@@ -749,7 +741,6 @@ NorFlashFvbInitialize (
       EFI_MEMORY_UC | EFI_MEMORY_RUNTIME);\r
   ASSERT_EFI_ERROR (Status);\r
 \r
-  Instance->Initialized = TRUE;\r
   mFlashNvStorageVariableBase = FixedPcdGet32 (PcdFlashNvStorageVariableBase);\r
 \r
   // Set the index of the first LBA for the FVB\r