[PATCH 00/10] ArmPkg, ArmPlatformPkg, ArmVirtPkg, EmbeddedPkg, Omap35xxPkg: depex fixes

[PATCH 00/10] ArmPkg, ArmPlatformPkg, ArmVirtPkg, EmbeddedPkg, Omap35xxPkg: depex fixes

[Laszlo Ersek]

Repo:   https://github.com/lersek/edk2.git
Branch: depex_fixes

ArmVirtQemu boots again, it just took a few more patches than I expected
:)

Some of these patches will have to be ported to edk2-platforms, I think.

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>

Thanks,
Laszlo

Laszlo Ersek (10):
  Omap35xxPkg/InterruptDxe: replace CPU Arch Protocol depex with notify
  ArmPkg/ArmGicDxe: annotate protocol usage in "ArmGicDxe.inf"
  ArmPkg/CpuDxe: order CpuDxe after ArmGicDxe via protocol depex
  EmbeddedPkg: introduce NvVarStoreFormattedLib
  ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly
  ArmPlatformPkg/NorFlashDxe: cue the variable driver with
    NvVarStoreFormatted
  ArmPlatformPkg/NorFlashDxe: depend on gEfiCpuArchProtocolGuid
  ArmPlatformPkg/PL031RealTimeClockLib: depend on
    gEfiCpuArchProtocolGuid
  ArmVirtPkg/PlatformHasAcpiDtDxe: depend on
    gEfiVariableArchProtocolGuid
  ArmVirtPkg/ArmVirtQemu: hook NvVarStoreFormattedLib into
    VariableRuntimeDxe

 ArmPkg/ArmPkg.dec                                                      |  2 -
 ArmPkg/Drivers/ArmGic/ArmGicDxe.inf                                    |  8 +-
 ArmPkg/Drivers/CpuDxe/CpuDxe.inf                                       |  2 +-
 ArmPlatformPkg/ArmPlatformPkg.dec                                      |  4 -
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c                       | 13 +---
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h                       |  6 --
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf                     |  7 +-
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c                    | 22 +++---
 ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf |  5 +-
 ArmVirtPkg/ArmVirtQemu.dsc                                             |  1 +
 ArmVirtPkg/ArmVirtQemuKernel.dsc                                       |  1 +
 ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf               |  2 +-
 EmbeddedPkg/EmbeddedPkg.dec                                            |  3 +
 EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h                         | 39 ++++++++++
 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c    | 41 ++++++++++
 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf  | 52 +++++++++++++
 Omap35xxPkg/InterruptDxe/HardwareInterrupt.c                           | 81 +++++++++++++++-----
 Omap35xxPkg/InterruptDxe/InterruptDxe.inf                              |  6 +-
 18 files changed, 230 insertions(+), 65 deletions(-)
 create mode 100644 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
 create mode 100644 EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h
 create mode 100644 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c

-- 
2.14.1.3.gb7cf6e02401b

[PATCH 01/10] Omap35xxPkg/InterruptDxe: replace CPU Arch Protocol depex with notify

[Laszlo Ersek]

In a later patch, we'll modify the depex of
"ArmPkg/Drivers/CpuDxe/CpuDxe.inf" (currently "AFTER gArmGicDxeFileGuid")
to "gHardwareInterruptProtocolGuid OR gHardwareInterrupt2ProtocolGuid".

Considering platforms that include "ArmPkg/Drivers/CpuDxe/CpuDxe.inf",
there are two classes:

(1) The platform gets its gHardwareInterruptProtocolGuid or
    gHardwareInterrupt2ProtocolGuid instance from
    "ArmPkg/Drivers/ArmGic/ArmGicDxe.inf". For such platforms, the
    upcoming CpuDxe change is not a problem, because commit 61a7b0ec634f
    made ArmGicDxe wait for the CPU Arch Protocol with a protocol notify.

(2) The platform gets its hardware interrupt protocol(s) from a different
    driver that has a hard depex on the CPU Arch Protocol. The upcoming
    CpuDxe change would lead to a loop in the DXE dispatch order.

In the edk2 tree, only "BeagleBoardPkg/BeagleBoardPkg.dsc" falls in class
(2), and the driver in question is "Omap35xxPkg/InterruptDxe". Port (most
of) commit 61a7b0ec634f to it.

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

Notes:
    build-tested only, as part of BeagleBoardPkg

 Omap35xxPkg/InterruptDxe/InterruptDxe.inf    |  6 +-
 Omap35xxPkg/InterruptDxe/HardwareInterrupt.c | 81 +++++++++++++++-----
 2 files changed, 66 insertions(+), 21 deletions(-)

diff --git a/Omap35xxPkg/InterruptDxe/InterruptDxe.inf b/Omap35xxPkg/InterruptDxe/InterruptDxe.inf
index 6deb8c3f675c..61ad89af2758 100644
--- a/Omap35xxPkg/InterruptDxe/InterruptDxe.inf
+++ b/Omap35xxPkg/InterruptDxe/InterruptDxe.inf
@@ -41,14 +41,14 @@ [LibraryClasses]
   PrintLib
   UefiDriverEntryPoint
   IoLib
   ArmLib
 
 [Protocols]
-  gHardwareInterruptProtocolGuid
-  gEfiCpuArchProtocolGuid
+  gHardwareInterruptProtocolGuid  ## PRODUCES
+  gEfiCpuArchProtocolGuid         ## CONSUMES ## NOTIFY
 
 [FixedPcd.common]
   gEmbeddedTokenSpaceGuid.PcdInterruptBaseAddress
 
 [Depex]
-  gEfiCpuArchProtocolGuid
+  TRUE
diff --git a/Omap35xxPkg/InterruptDxe/HardwareInterrupt.c b/Omap35xxPkg/InterruptDxe/HardwareInterrupt.c
index 09e22b5921b0..2ddc7c0566d0 100644
--- a/Omap35xxPkg/InterruptDxe/HardwareInterrupt.c
+++ b/Omap35xxPkg/InterruptDxe/HardwareInterrupt.c
@@ -293,12 +293,60 @@ EFI_HARDWARE_INTERRUPT_PROTOCOL gHardwareInterruptProtocol = {
   EnableInterruptSource,
   DisableInterruptSource,
   GetInterruptSourceState,
   EndOfInterrupt
 };
 
+STATIC VOID *mCpuArchProtocolNotifyEventRegistration;
+
+STATIC
+VOID
+EFIAPI
+CpuArchEventProtocolNotify (
+  IN  EFI_EVENT       Event,
+  IN  VOID            *Context
+  )
+{
+  EFI_CPU_ARCH_PROTOCOL   *Cpu;
+  EFI_STATUS              Status;
+
+  //
+  // Get the CPU protocol that this driver requires.
+  //
+  Status = gBS->LocateProtocol (&gEfiCpuArchProtocolGuid, NULL, (VOID **)&Cpu);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a: gBS->LocateProtocol() - %r\n", __FUNCTION__,
+      Status));
+    ASSERT (FALSE);
+    return;
+  }
+
+  //
+  // Unregister the default exception handler.
+  //
+  Status = Cpu->RegisterInterruptHandler (Cpu, EXCEPT_ARM_IRQ, NULL);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a: Cpu->RegisterInterruptHandler() - %r\n",
+      __FUNCTION__, Status));
+    ASSERT (FALSE);
+    return;
+  }
+
+  //
+  // Register to receive interrupts
+  //
+  Status = Cpu->RegisterInterruptHandler (Cpu, EXCEPT_ARM_IRQ,
+                  IrqInterruptHandler);
+  if (EFI_ERROR (Status)) {
+    DEBUG ((DEBUG_ERROR, "%a: Cpu->RegisterInterruptHandler() - %r\n",
+      __FUNCTION__, Status));
+    ASSERT (FALSE);
+    return;
+  }
+}
+
 /**
   Initialize the state information for the CPU Architectural Protocol
 
   @param  ImageHandle   of the loaded driver
   @param  SystemTable   Pointer to the System Table
 
@@ -311,13 +359,13 @@ EFI_STATUS
 InterruptDxeInitialize (
   IN EFI_HANDLE         ImageHandle,
   IN EFI_SYSTEM_TABLE   *SystemTable
   )
 {
   EFI_STATUS  Status;
-  EFI_CPU_ARCH_PROTOCOL   *Cpu;
+  EFI_EVENT   CpuArchEvent;
 
   // Make sure the Interrupt Controller Protocol is not already installed in the system.
   ASSERT_PROTOCOL_ALREADY_INSTALLED (NULL, &gHardwareInterruptProtocolGuid);
 
   // Make sure all interrupts are disabled by default.
   MmioWrite32 (INTCPS_MIR(0), 0xFFFFFFFF);
@@ -328,30 +376,27 @@ InterruptDxeInitialize (
   Status = gBS->InstallMultipleProtocolInterfaces(&gHardwareInterruptHandle,
                                                   &gHardwareInterruptProtocolGuid,   &gHardwareInterruptProtocol,
                                                   NULL);
   ASSERT_EFI_ERROR(Status);
 
   //
-  // Get the CPU protocol that this driver requires.
-  //
-  Status = gBS->LocateProtocol(&gEfiCpuArchProtocolGuid, NULL, (VOID **)&Cpu);
-  ASSERT_EFI_ERROR(Status);
-
-  //
-  // Unregister the default exception handler.
-  //
-  Status = Cpu->RegisterInterruptHandler(Cpu, EXCEPT_ARM_IRQ, NULL);
-  ASSERT_EFI_ERROR(Status);
-
-  //
-  // Register to receive interrupts
-  //
-  Status = Cpu->RegisterInterruptHandler(Cpu, EXCEPT_ARM_IRQ, IrqInterruptHandler);
-  ASSERT_EFI_ERROR(Status);
+  // Install the interrupt handler as soon as the CPU arch protocol appears.
+  //
+  CpuArchEvent = EfiCreateProtocolNotifyEvent (
+                   &gEfiCpuArchProtocolGuid,
+                   TPL_CALLBACK,
+                   CpuArchEventProtocolNotify,
+                   NULL,
+                   &mCpuArchProtocolNotifyEventRegistration
+                   );
+  ASSERT (CpuArchEvent != NULL);
 
   // Register for an ExitBootServicesEvent
   Status = gBS->CreateEvent(EVT_SIGNAL_EXIT_BOOT_SERVICES, TPL_NOTIFY, ExitBootServicesEvent, NULL, &EfiExitBootServicesEvent);
-  ASSERT_EFI_ERROR(Status);
+  if (EFI_ERROR (Status)) {
+    ASSERT_EFI_ERROR (Status);
+    gBS->CloseEvent (CpuArchEvent);
+  }
 
   return Status;
 }
 
-- 
2.14.1.3.gb7cf6e02401b

[PATCH 02/10] ArmPkg/ArmGicDxe: annotate protocol usage in "ArmGicDxe.inf"

[Laszlo Ersek]

"ArmGicDxe.inf" currently does not document how the protocols in the
[Protocols] section are used. Such comments help us analyze behavior, so
let's add them now.

- gHardwareInterruptProtocolGuid and gHardwareInterrupt2ProtocolGuid are
  always produced on the InterruptDxeInitialize() -> (GicV2DxeInitialize()
  | GicV3DxeInitialize()) -> InstallAndRegisterInterruptService() call
  path.

- gEfiCpuArchProtocolGuid is consumed in the CpuArchEventProtocolNotify()
  protocol notify callback. (Technically this is "conditional"; however
  the firmware cannot work without architectural protocols, so we can call
  it unconditional.)

While at it, drop the gArmGicDxeFileGuid comment from FILE_GUID; we're
going to make that GUID uninteresting soon.

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>
---
 ArmPkg/Drivers/ArmGic/ArmGicDxe.inf | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
index 24b02ef30e83..9c85b1e069dc 100644
--- a/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
+++ b/ArmPkg/Drivers/ArmGic/ArmGicDxe.inf
@@ -13,13 +13,13 @@
 #
 #**/
 
 [Defines]
   INF_VERSION                    = 0x00010005
   BASE_NAME                      = ArmGicDxe
-  FILE_GUID                      = DE371F7C-DEC4-4D21-ADF1-593ABCC15882 # gArmGicDxeFileGuid
+  FILE_GUID                      = DE371F7C-DEC4-4D21-ADF1-593ABCC15882
   MODULE_TYPE                    = DXE_DRIVER
   VERSION_STRING                 = 1.0
 
   ENTRY_POINT                    = InterruptDxeInitialize
 
 [Sources.common]
@@ -45,15 +45,15 @@ [LibraryClasses]
   UefiDriverEntryPoint
   IoLib
   PcdLib
   UefiLib
 
 [Protocols]
-  gHardwareInterruptProtocolGuid
-  gHardwareInterrupt2ProtocolGuid
-  gEfiCpuArchProtocolGuid
+  gHardwareInterruptProtocolGuid  ## PRODUCES
+  gHardwareInterrupt2ProtocolGuid ## PRODUCES
+  gEfiCpuArchProtocolGuid         ## CONSUMES ## NOTIFY
 
 [Pcd.common]
   gArmTokenSpaceGuid.PcdGicDistributorBase
   gArmTokenSpaceGuid.PcdGicRedistributorsBase
   gArmTokenSpaceGuid.PcdGicInterruptInterfaceBase
   gArmTokenSpaceGuid.PcdArmGicV3WithV2Legacy
-- 
2.14.1.3.gb7cf6e02401b

[PATCH 03/10] ArmPkg/CpuDxe: order CpuDxe after ArmGicDxe via protocol depex

[Laszlo Ersek]

Commit 61a7b0ec634f ("ArmPkg/Gic: force GIC driver to run before CPU arch
protocol driver", 2018-02-06) explains why CpuDxe should be dispatched
after ArmGicDxe.

To implement the ordering, we should use a regular protocol depex rather
than the less flexible AFTER opcode. ArmGicDxe installs
gHardwareInterruptProtocolGuid and gHardwareInterrupt2ProtocolGuid as one
of the last actions on its entry point stack; either of those is OK for
CpuDxe to wait for.

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>
---
 ArmPkg/ArmPkg.dec                | 2 --
 ArmPkg/Drivers/CpuDxe/CpuDxe.inf | 2 +-
 2 files changed, 1 insertion(+), 3 deletions(-)

diff --git a/ArmPkg/ArmPkg.dec b/ArmPkg/ArmPkg.dec
index a55b6268ff85..5dbd019e2966 100644
--- a/ArmPkg/ArmPkg.dec
+++ b/ArmPkg/ArmPkg.dec
@@ -45,14 +45,12 @@ [Guids.common]
   gArmTokenSpaceGuid       = { 0xBB11ECFE, 0x820F, 0x4968, { 0xBB, 0xA6, 0xF7, 0x6A, 0xFE, 0x30, 0x25, 0x96 } }
 
   ## ARM MPCore table
   # Include/Guid/ArmMpCoreInfo.h
   gArmMpCoreInfoGuid = { 0xa4ee0728, 0xe5d7, 0x4ac5,  {0xb2, 0x1e, 0x65, 0x8e, 0xd8, 0x57, 0xe8, 0x34} }
 
-  gArmGicDxeFileGuid = { 0xde371f7c, 0xdec4, 0x4d21, { 0xad, 0xf1, 0x59, 0x3a, 0xbc, 0xc1, 0x58, 0x82 } }
-
 [Ppis]
   ## Include/Ppi/ArmMpCoreInfo.h
   gArmMpCoreInfoPpiGuid = { 0x6847cc74, 0xe9ec, 0x4f8f, {0xa2, 0x9d, 0xab, 0x44, 0xe7, 0x54, 0xa8, 0xfc} }
 
 [PcdsFeatureFlag.common]
   gArmTokenSpaceGuid.PcdCpuDxeProduceDebugSupport|FALSE|BOOLEAN|0x00000001
diff --git a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
index b748f0344537..c32d2cb9c7d4 100644
--- a/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
+++ b/ArmPkg/Drivers/CpuDxe/CpuDxe.inf
@@ -71,7 +71,7 @@ [Pcd.common]
   gArmTokenSpaceGuid.PcdVFPEnabled
 
 [FeaturePcd.common]
   gArmTokenSpaceGuid.PcdDebuggerExceptionSupport
 
 [Depex]
-  AFTER gArmGicDxeFileGuid
+  gHardwareInterruptProtocolGuid OR gHardwareInterrupt2ProtocolGuid
-- 
2.14.1.3.gb7cf6e02401b

[PATCH 04/10] EmbeddedPkg: introduce NvVarStoreFormattedLib

[Laszlo Ersek]

Some platforms don't format a variable store template at build time;
instead they format the non-volatile varstore flash chip during boot,
dynamically. Introduce NvVarStoreFormattedLib to enable such platforms to
delay the "variable read" service drivers until the platform specific
module(s) report that the variable store has been formatted.

The platform-specific module that performs the formatting during startup
is usually an FVB or MM FVB driver. Under the proposed scheme, it becomes
responsible for installing gEdkiiNvVarStoreFormattedGuid with a NULL
interface in the protocol database. In turn, the platform DSC will hook
NvVarStoreFormattedLib into the variable service driver, to make the
latter wait for the FVB driver. Platforms that need not delay the variable
service driver like this may still use the same FVB driver;
gEdkiiNvVarStoreFormattedGuid will simply be ignored.

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>
---
 EmbeddedPkg/EmbeddedPkg.dec                                           |  3 ++
 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf | 52 ++++++++++++++++++++
 EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h                        | 39 +++++++++++++++
 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c   | 41 +++++++++++++++
 4 files changed, 135 insertions(+)

diff --git a/EmbeddedPkg/EmbeddedPkg.dec b/EmbeddedPkg/EmbeddedPkg.dec
index e48ce2e95bf7..c7b134dd34e6 100644
--- a/EmbeddedPkg/EmbeddedPkg.dec
+++ b/EmbeddedPkg/EmbeddedPkg.dec
@@ -64,12 +64,15 @@ [Guids.common]
   # File GUID for default DTB image embedded in the firmware volume
   gDtPlatformDefaultDtbFileGuid = { 0x25462cda, 0x221f, 0x47df, { 0xac, 0x1d, 0x25, 0x9c, 0xfa, 0xa4, 0xe3, 0x26 } }
 
   # HII form set GUID for ConsolePrefDxe driver
   gConsolePrefFormSetGuid = { 0x2d2358b4, 0xe96c, 0x484d, { 0xb2, 0xdd, 0x7c, 0x2e, 0xdf, 0xc7, 0xd5, 0x6f } }
 
+  ## Include/Guid/NvVarStoreFormatted.h
+  gEdkiiNvVarStoreFormattedGuid = { 0xd1a86e3f, 0x0707, 0x4c35, { 0x83, 0xcd, 0xdc, 0x2c, 0x29, 0xc8, 0x91, 0xa3 } }
+
 [Protocols.common]
   gHardwareInterruptProtocolGuid =  { 0x2890B3EA, 0x053D, 0x1643, { 0xAD, 0x0C, 0xD6, 0x48, 0x08, 0xDA, 0x3F, 0xF1 } }
   gHardwareInterrupt2ProtocolGuid = { 0x32898322, 0x2da1, 0x474a, { 0xba, 0xaa, 0xf3, 0xf7, 0xcf, 0x56, 0x94, 0x70 } }
   gEmbeddedDeviceGuid =   { 0xbf4b9d10, 0x13ec, 0x43dd, { 0x88, 0x80, 0xe9, 0xb, 0x71, 0x8f, 0x27, 0xde } }
   gEmbeddedExternalDeviceProtocolGuid = { 0x735F8C64, 0xD696, 0x44D0, { 0xBD, 0xF2, 0x44, 0x7F, 0xD0, 0x5A, 0x54, 0x06 }}
   gEmbeddedGpioProtocolGuid           = { 0x17a0a3d7, 0xc0a5, 0x4635, { 0xbb, 0xd5, 0x07, 0x21, 0x87, 0xdf, 0xe2, 0xee }}
diff --git a/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf b/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
new file mode 100644
index 000000000000..b02fc7810948
--- /dev/null
+++ b/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
@@ -0,0 +1,52 @@
+## @file
+# A hook-in library for:
+# - MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
+# - MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+# - MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
+#
+# Plugging this library instance into one of the above modules makes that
+# variable service backend wait for another platform module to dynamically
+# initialize or verify EFI_FIRMWARE_VOLUME_HEADER and VARIABLE_STORE_HEADER in
+# the non-volatile variable store FVB device. The initialization / verification
+# is signaled by installing gEdkiiNvVarStoreFormattedGuid into the
+# phase-matching PPI or protocol database, with a NULL interface. (Note that
+# installing gEdkiiNvVarStoreFormattedGuid into either the DXE or the MM
+# protocol database will unblock VariableSmm -- refer to EFI_SECTION_MM_DEPEX
+# in the PI spec.)
+#
+# Copyright (C) 2018, Red Hat, Inc.
+#
+# This program and the accompanying materials are licensed and made available
+# under the terms and conditions of the BSD License which accompanies this
+# distribution. The full text of the license may be found at
+# http://opensource.org/licenses/bsd-license.php
+#
+# THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+# WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+##
+
+[Defines]
+  INF_VERSION                    = 1.27
+  BASE_NAME                      = NvVarStoreFormattedLib
+  FILE_GUID                      = 78f76ae8-ae62-4455-8148-c3a7ebaaa3f3
+  MODULE_TYPE                    = BASE
+  VERSION_STRING                 = 1.0
+  LIBRARY_CLASS                  = NvVarStoreFormattedLib|PEIM DXE_RUNTIME_DRIVER DXE_SMM_DRIVER
+  CONSTRUCTOR                    = NvVarStoreFormattedInitialize
+
+[Sources]
+  NvVarStoreFormattedLib.c
+
+[Packages]
+  EmbeddedPkg/EmbeddedPkg.dec
+  MdeModulePkg/MdeModulePkg.dec
+  MdePkg/MdePkg.dec
+
+#
+# The matching DEPEX section below will generate the EFI_SECTION_PEI_DEPEX,
+# EFI_SECTION_DXE_DEPEX or EFI_SECTION_MM_DEPEX leaf section for the PEIM
+# (EFI_FV_FILETYPE_PEIM), DXE_RUNTIME_DRIVER (EFI_FV_FILETYPE_DRIVER), or
+# DXE_SMM_DRIVER (EFI_FV_FILETYPE_MM) module, respectively.
+#
+[Depex.common.PEIM, Depex.common.DXE_RUNTIME_DRIVER, Depex.common.DXE_SMM_DRIVER]
+  gEdkiiNvVarStoreFormattedGuid
diff --git a/EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h b/EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h
new file mode 100644
index 000000000000..4e057e4d894d
--- /dev/null
+++ b/EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h
@@ -0,0 +1,39 @@
+/** @file
+  EDKII NvVarStore Formatted GUID
+
+  A NULL protocol instance with this GUID in the DXE and/or MM protocol
+  databases, and/or a NULL PPI with this GUID in the PPI database, implies that
+  a DXE or MM driver, or a PEIM, has verified (or dynamically ensured) that the
+  non-volatile variable store has valid and consistent headers
+  (EFI_FIRMWARE_VOLUME_HEADER and VARIABLE_STORE_HEADER).
+
+  Said predicate is required by the read-only variable PEIM, and the read side
+  of the runtime variable DXE and MM drivers, immediately after they are
+  dispatched. This GUID presents platforms with one way to coordinate between
+  their module(s) that format the variable store FVB device and the variable
+  service drivers.
+
+  Copyright (C) 2018, Red Hat, Inc.
+
+  This program and the accompanying materials are licensed and made available
+  under the terms and conditions of the BSD License that accompanies this
+  distribution. The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php.
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+**/
+
+
+#ifndef __EDKII_NV_VAR_STORE_FORMATTED_H__
+#define __EDKII_NV_VAR_STORE_FORMATTED_H__
+
+#define EDKII_NV_VAR_STORE_FORMATTED_GUID \
+  { \
+    0xd1a86e3f, 0x0707, 0x4c35, \
+    { 0x83, 0xcd, 0xdc, 0x2c, 0x29, 0xc8, 0x91, 0xa3 } \
+  }
+
+extern EFI_GUID gEdkiiNvVarStoreFormattedGuid;
+
+#endif
diff --git a/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c b/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c
new file mode 100644
index 000000000000..001e47cfe87c
--- /dev/null
+++ b/EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c
@@ -0,0 +1,41 @@
+/** @file
+  A hook-in library for:
+  - MdeModulePkg/Universal/Variable/Pei/VariablePei.inf
+  - MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
+  - MdeModulePkg/Universal/Variable/RuntimeDxe/VariableSmm.inf
+
+  Plugging this library instance into one of the above modules makes that
+  variable service backend wait for another platform module to dynamically
+  initialize or verify EFI_FIRMWARE_VOLUME_HEADER and VARIABLE_STORE_HEADER in
+  the non-volatile variable store FVB device. The initialization / verification
+  is signaled by installing gEdkiiNvVarStoreFormattedGuid into the
+  phase-matching PPI or protocol database, with a NULL interface. (Note that
+  installing gEdkiiNvVarStoreFormattedGuid into either the DXE or the MM
+  protocol database will unblock VariableSmm -- refer to EFI_SECTION_MM_DEPEX
+  in the PI spec.)
+
+  Copyright (C) 2018, Red Hat, Inc.
+
+  This program and the accompanying materials are licensed and made available
+  under the terms and conditions of the BSD License which accompanies this
+  distribution. The full text of the license may be found at
+  http://opensource.org/licenses/bsd-license.php
+
+  THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, WITHOUT
+  WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED.
+**/
+
+#include <Base.h>
+
+RETURN_STATUS
+EFIAPI
+NvVarStoreFormattedInitialize (
+  VOID
+  )
+{
+  //
+  // Do nothing, just imbue VariablePei / VariableRuntimeDxe / VariableSmm with
+  // a PPI or protocol dependency on EDKII_NV_VAR_STORE_FORMATTED_GUID.
+  //
+  return RETURN_SUCCESS;
+}
-- 
2.14.1.3.gb7cf6e02401b

[PATCH 05/10] ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly

[Laszlo Ersek]

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

[PATCH 06/10] ArmPlatformPkg/NorFlashDxe: cue the variable driver with NvVarStoreFormatted

[Laszlo Ersek]

The BEFORE depex opcode that we currently use to force ourselves in front
of the variable driver cannot be combined with other depex opcodes.
Replace the depex with TRUE, and signal NvVarStoreFormattedLib through the
installation of "gEdkiiNvVarStoreFormattedGuid".

Platforms that rely on NorFlashDxe to format the variable store (as
opposed to formatting a variable store template through an FDF file, as
part of the build) should hook NvVarStoreFormattedLib into the variable
drivers they use, so that the latter await our cue.

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/ArmPlatformPkg.dec                   |  4 ----
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf  |  7 +++----
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c | 13 +++++++++++++
 3 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/ArmPlatformPkg/ArmPlatformPkg.dec b/ArmPlatformPkg/ArmPlatformPkg.dec
index 7cec775abeee..8f32a5e2c14d 100644
--- a/ArmPlatformPkg/ArmPlatformPkg.dec
+++ b/ArmPlatformPkg/ArmPlatformPkg.dec
@@ -37,16 +37,12 @@ [LibraryClasses]
   LcdPlatformLib|Include/Library/LcdPlatformLib.h
   NorFlashPlatformLib|Include/Library/NorFlashPlatformLib.h
   PL011UartLib|Include/Library/PL011UartLib.h
 
 [Guids.common]
   gArmPlatformTokenSpaceGuid   = { 0x9c0aaed4, 0x74c5, 0x4043, { 0xb4, 0x17, 0xa3, 0x22, 0x38, 0x14, 0xce, 0x76 } }
-  #
-  # Following Guid must match FILE_GUID in MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf
-  #
-  gVariableRuntimeDxeFileGuid = { 0xcbd2e4d5, 0x7068, 0x4ff5, { 0xb4, 0x62, 0x98, 0x22, 0xb4, 0xad, 0x8d, 0x60 } }
 
 [PcdsFeatureFlag.common]
   gArmPlatformTokenSpaceGuid.PcdSendSgiToBringUpSecondaryCores|FALSE|BOOLEAN|0x00000004
 
   gArmPlatformTokenSpaceGuid.PcdNorFlashCheckBlockLocked|FALSE|BOOLEAN|0x0000003C
 
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
index 812dafd065b2..c40ac27a6599 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
@@ -28,12 +28,13 @@ [Sources.common]
   NorFlashBlockIoDxe.c
 
 [Packages]
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
   ArmPlatformPkg/ArmPlatformPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec
 
 [LibraryClasses]
   IoLib
   BaseLib
   DebugLib
   HobLib
@@ -46,12 +47,13 @@ [LibraryClasses]
 
 [Guids]
   gEfiSystemNvDataFvGuid
   gEfiVariableGuid
   gEfiAuthenticatedVariableGuid
   gEfiEventVirtualAddressChangeGuid
+  gEdkiiNvVarStoreFormattedGuid     ## PRODUCES ## PROTOCOL
 
 [Protocols]
   gEfiBlockIoProtocolGuid
   gEfiDevicePathProtocolGuid
   gEfiFirmwareVolumeBlockProtocolGuid
   gEfiDiskIoProtocolGuid
@@ -64,10 +66,7 @@ [Pcd.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
 
   gArmPlatformTokenSpaceGuid.PcdNorFlashCheckBlockLocked
 
 [Depex]
-  #
-  # NorFlashDxe must be loaded before VariableRuntimeDxe in case empty flash needs populating with default values
-  #
-  BEFORE gVariableRuntimeDxeFileGuid
+  TRUE
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
index c3e6489f398f..e62ffbb433d0 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c
@@ -22,12 +22,13 @@
 #include <Library/MemoryAllocationLib.h>
 #include <Library/DxeServicesTableLib.h>
 #include <Library/UefiBootServicesTableLib.h>
 
 #include <Guid/VariableFormat.h>
 #include <Guid/SystemNvDataGuid.h>
+#include <Guid/NvVarStoreFormatted.h>
 
 #include "NorFlashDxe.h"
 
 STATIC EFI_EVENT mFvbVirtualAddrChangeEvent;
 STATIC UINTN     mFlashNvStorageVariableBase;
 
@@ -773,12 +774,24 @@ NorFlashFvbInitialize (
     Status = InitializeFvAndVariableStoreHeaders (Instance);
     if (EFI_ERROR(Status)) {
       return Status;
     }
   }
 
+  //
+  // The driver implementing the variable read service can now be dispatched;
+  // the varstore headers are in place.
+  //
+  Status = gBS->InstallProtocolInterface (
+                  &gImageHandle,
+                  &gEdkiiNvVarStoreFormattedGuid,
+                  EFI_NATIVE_INTERFACE,
+                  NULL
+                  );
+  ASSERT_EFI_ERROR (Status);
+
   //
   // Register for the virtual address change event
   //
   Status = gBS->CreateEventEx (
                   EVT_NOTIFY_SIGNAL,
                   TPL_NOTIFY,
-- 
2.14.1.3.gb7cf6e02401b

[PATCH 07/10] ArmPlatformPkg/NorFlashDxe: depend on gEfiCpuArchProtocolGuid

[Laszlo Ersek]

NorFlashFvbInitialize() calls gDS->SetMemorySpaceAttributes() to mark the
varstore flash region as uncached. This DXE service depends on the CPU
Architectural protocol, and the DXE core is allowed to return
EFI_NOT_AVAILABLE_YET if it hasn't dispatched ArmPkg/Drivers/CpuDxe
earlier. Make the dependency explicit.

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.inf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
index c40ac27a6599..a59a21a03e0a 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
@@ -66,7 +66,7 @@ [Pcd.common]
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareBase
   gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpareSize
 
   gArmPlatformTokenSpaceGuid.PcdNorFlashCheckBlockLocked
 
 [Depex]
-  TRUE
+  gEfiCpuArchProtocolGuid
-- 
2.14.1.3.gb7cf6e02401b

[PATCH 08/10] ArmPlatformPkg/PL031RealTimeClockLib: depend on gEfiCpuArchProtocolGuid

[Laszlo Ersek]

The RealTimeClockLib class is declared under EmbeddedPkg, so that
platforms can provide the internals for the
EmbeddedPkg/RealTimeClockRuntimeDxe driver. In turn the driver produces
the Real Time Clock Arch Protocol, without which UEFI drivers cannot be
dispatched.

The PL031RealTimeClockLib instance calls gDS->SetMemorySpaceAttributes()
in the LibRtcInitialize() public function. This DXE service depends on the
CPU Arch Protocol. Add it to the depex.

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/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
index a3e4f73e7d05..3d62aeabd670 100644
--- a/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
+++ b/ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf
@@ -16,13 +16,13 @@
 [Defines]
   INF_VERSION                    = 0x00010005
   BASE_NAME                      = PL031RealTimeClockLib
   FILE_GUID                      = 470DFB96-E205-4515-A75E-2E60F853E79D
   MODULE_TYPE                    = BASE
   VERSION_STRING                 = 1.0
-  LIBRARY_CLASS                  = RealTimeClockLib
+  LIBRARY_CLASS                  = RealTimeClockLib|DXE_RUNTIME_DRIVER
 
 [Sources.common]
   PL031RealTimeClockLib.c
 
 [Packages]
   MdePkg/MdePkg.dec
@@ -41,6 +41,9 @@ [LibraryClasses]
 [Guids]
   gEfiEventVirtualAddressChangeGuid
 
 [Pcd]
   gArmPlatformTokenSpaceGuid.PcdPL031RtcBase
   gArmPlatformTokenSpaceGuid.PcdPL031RtcPpmAccuracy
+
+[Depex.common.DXE_RUNTIME_DRIVER]
+  gEfiCpuArchProtocolGuid
-- 
2.14.1.3.gb7cf6e02401b

[PATCH 09/10] ArmVirtPkg/PlatformHasAcpiDtDxe: depend on gEfiVariableArchProtocolGuid

[Laszlo Ersek]

PlatformHasAcpiDtDxe consumes the DynamicHii PCD called
"gArmVirtTokenSpaceGuid.PcdForceNoAcpi". The PcdGetBool() library call
terminates in gRT->GetVariable(), in the MdeModulePkg/Universal/PCD/Dxe
driver. Put "gEfiVariableArchProtocolGuid" on PlatformHasAcpiDtDxe's DEPEX
so that we not attempt the call before the PCD driver can successfully
read non-volatile variables.

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>
---
 ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
index 5351e741aa41..8346f7f24b32 100644
--- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
+++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
@@ -44,7 +44,7 @@ [Guids]
   gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
 
 [Pcd]
   gArmVirtTokenSpaceGuid.PcdForceNoAcpi
 
 [Depex]
-  TRUE
+  gEfiVariableArchProtocolGuid
-- 
2.14.1.3.gb7cf6e02401b

[PATCH 10/10] ArmVirtPkg/ArmVirtQemu: hook NvVarStoreFormattedLib into VariableRuntimeDxe

[Laszlo Ersek]

In spite of both ArmVirtQemu and ArmVirtQemuKernel formatting the variable
store template at build time, link NvVarStoreFormattedLib into
VariableRuntimeDxe via NULL class resolution on both platforms. This lets
us test the depexes implemented in the previous patches.

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>
---
 ArmVirtPkg/ArmVirtQemu.dsc       | 1 +
 ArmVirtPkg/ArmVirtQemuKernel.dsc | 1 +
 2 files changed, 2 insertions(+)

diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
index 76a83b99a303..824070edc2a9 100644
--- a/ArmVirtPkg/ArmVirtQemu.dsc
+++ b/ArmVirtPkg/ArmVirtQemu.dsc
@@ -254,12 +254,13 @@ [Components.common]
   #
   ArmPkg/Drivers/CpuDxe/CpuDxe.inf
   MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
     <LibraryClasses>
       NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
+      NULL|EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
       # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
       BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
   }
 !if $(SECURE_BOOT_ENABLE) == TRUE
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
     <LibraryClasses>
diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
index 7d9a9196d640..2368ba283bff 100644
--- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
+++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
@@ -243,12 +243,13 @@ [Components.common]
   #
   ArmPkg/Drivers/CpuDxe/CpuDxe.inf
   MdeModulePkg/Core/RuntimeDxe/RuntimeDxe.inf
   MdeModulePkg/Universal/Variable/RuntimeDxe/VariableRuntimeDxe.inf {
     <LibraryClasses>
       NULL|MdeModulePkg/Library/VarCheckUefiLib/VarCheckUefiLib.inf
+      NULL|EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
       # don't use unaligned CopyMem () on the UEFI varstore NOR flash region
       BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf
   }
 !if $(SECURE_BOOT_ENABLE) == TRUE
   MdeModulePkg/Universal/SecurityStubDxe/SecurityStubDxe.inf {
     <LibraryClasses>
-- 
2.14.1.3.gb7cf6e02401b

Re: [PATCH 09/10] ArmVirtPkg/PlatformHasAcpiDtDxe: depend on gEfiVariableArchProtocolGuid

[Ard Biesheuvel]

On 12 April 2018 at 02:55, Laszlo Ersek <lersek@redhat.com> wrote:
> PlatformHasAcpiDtDxe consumes the DynamicHii PCD called
> "gArmVirtTokenSpaceGuid.PcdForceNoAcpi". The PcdGetBool() library call
> terminates in gRT->GetVariable(), in the MdeModulePkg/Universal/PCD/Dxe
> driver. Put "gEfiVariableArchProtocolGuid" on PlatformHasAcpiDtDxe's DEPEX
> so that we not attempt the call before the PCD driver can successfully
> read non-volatile variables.
>

OK, so does this mean that UefiRuntimeServicesTableLib deliberately
does not depex on gEfiVariableArchProtocolGuid, and it will return
EFI_NOT_AVAILABLE_YET when called too early? If that is the case, I
would assume it is the responsibility of the PcdLib implementation to
deal with that, not the PcdLib clients.


> 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>
> ---
>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
> index 5351e741aa41..8346f7f24b32 100644
> --- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
> +++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
> @@ -44,7 +44,7 @@ [Guids]
>    gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
>
>  [Pcd]
>    gArmVirtTokenSpaceGuid.PcdForceNoAcpi
>
>  [Depex]
> -  TRUE
> +  gEfiVariableArchProtocolGuid
> --
> 2.14.1.3.gb7cf6e02401b
>
>

Re: [PATCH 09/10] ArmVirtPkg/PlatformHasAcpiDtDxe: depend on gEfiVariableArchProtocolGuid

[Laszlo Ersek]

On 04/12/18 08:28, Ard Biesheuvel wrote:
> On 12 April 2018 at 02:55, Laszlo Ersek <lersek@redhat.com> wrote:
>> PlatformHasAcpiDtDxe consumes the DynamicHii PCD called
>> "gArmVirtTokenSpaceGuid.PcdForceNoAcpi". The PcdGetBool() library call
>> terminates in gRT->GetVariable(), in the MdeModulePkg/Universal/PCD/Dxe
>> driver. Put "gEfiVariableArchProtocolGuid" on PlatformHasAcpiDtDxe's DEPEX
>> so that we not attempt the call before the PCD driver can successfully
>> read non-volatile variables.
>>
> 
> OK, so does this mean that UefiRuntimeServicesTableLib deliberately
> does not depex on gEfiVariableArchProtocolGuid,

Yes, I do think so; the runtime services are expected to become
available gradually. For example, "MdePkg/Include/Protocol/Variable.h"
has a great long comment on this.

> and it will return
> EFI_NOT_AVAILABLE_YET when called too early? If that is the case, I
> would assume it is the responsibility of the PcdLib implementation to
> deal with that, not the PcdLib clients.

Yes, this idea occurred to me as well, but I think it is not correct.
The PcdLib instance for the DXE phase is responsible for talking to the
PCD protocol, and indeed it has a depex on that protocol
(MdePkg/Library/DxePcdLib/DxePcdLib.inf). However, if a module uses
dynamic PCDs, but none of those dynamic PCDs are HII (i.e., backed by
non-volatile UEFI variables), then IMO neither the PcdLib instance, nor
the PCD DXE driver, should depend on the variable read service
(gEfiVariableArchProtocolGuid).

(Well the PCD DXE driver is shared / used by all of the DXE modules, so
it couldn't depend on gEfiVariableArchProtocolGuid in any case at all.)

Ultimately, in the optimal case, the PcdLib instance for DXE should
"know" if any PCDs used by the main driver module are DynamicHII, and in
that case, the PcdLib instance should create a dependency on the
variable read (and possibly write) services. I think edk2 simply doesn't
support this use case at this moment (it is ultimately a DSC decision
what type we give to a PCD, choosing from the possibilities listed in
the DEC), so we have to take care of it ourselves. I do see that this
limits the reusability of individual drivers across different DSC files
-- if another DSC makes the same PCD fixed, for example, then the
gEfiVariableArchProtocolGuid dependency becomes useless / needlessly
restrictive in the driver.

We could factor out just the gEfiVariableArchProtocolGuid depex into
another NULL class lib, and hook that into PlatformHasAcpiDtDxe in the
DSC file, because in the DSC we do know that the PCD is DynamicHII. If
you feel strongly about it, it's doable, but I certainly think it's
overkill. PlatformHasAcpiDtDxe got "Platform" in its name; it's pretty
unlikely to be reused in any other DSC. (Even ArmVirtXen has a different
implementation, which does not use the PCD.) I feel the proposed patch
should be fine -- what's your take?

Thanks!
Laszlo

> 
> 
>> 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>
>> ---
>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
>> index 5351e741aa41..8346f7f24b32 100644
>> --- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
>> +++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
>> @@ -44,7 +44,7 @@ [Guids]
>>    gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
>>
>>  [Pcd]
>>    gArmVirtTokenSpaceGuid.PcdForceNoAcpi
>>
>>  [Depex]
>> -  TRUE
>> +  gEfiVariableArchProtocolGuid
>> --
>> 2.14.1.3.gb7cf6e02401b
>>
>>

Re: [PATCH 09/10] ArmVirtPkg/PlatformHasAcpiDtDxe: depend on gEfiVariableArchProtocolGuid

[Ard Biesheuvel]

On 12 April 2018 at 11:05, Laszlo Ersek <lersek@redhat.com> wrote:
> On 04/12/18 08:28, Ard Biesheuvel wrote:
>> On 12 April 2018 at 02:55, Laszlo Ersek <lersek@redhat.com> wrote:
>>> PlatformHasAcpiDtDxe consumes the DynamicHii PCD called
>>> "gArmVirtTokenSpaceGuid.PcdForceNoAcpi". The PcdGetBool() library call
>>> terminates in gRT->GetVariable(), in the MdeModulePkg/Universal/PCD/Dxe
>>> driver. Put "gEfiVariableArchProtocolGuid" on PlatformHasAcpiDtDxe's DEPEX
>>> so that we not attempt the call before the PCD driver can successfully
>>> read non-volatile variables.
>>>
>>
>> OK, so does this mean that UefiRuntimeServicesTableLib deliberately
>> does not depex on gEfiVariableArchProtocolGuid,
>
> Yes, I do think so; the runtime services are expected to become
> available gradually. For example, "MdePkg/Include/Protocol/Variable.h"
> has a great long comment on this.
>
>> and it will return
>> EFI_NOT_AVAILABLE_YET when called too early? If that is the case, I
>> would assume it is the responsibility of the PcdLib implementation to
>> deal with that, not the PcdLib clients.
>
> Yes, this idea occurred to me as well, but I think it is not correct.
> The PcdLib instance for the DXE phase is responsible for talking to the
> PCD protocol, and indeed it has a depex on that protocol
> (MdePkg/Library/DxePcdLib/DxePcdLib.inf). However, if a module uses
> dynamic PCDs, but none of those dynamic PCDs are HII (i.e., backed by
> non-volatile UEFI variables), then IMO neither the PcdLib instance, nor
> the PCD DXE driver, should depend on the variable read service
> (gEfiVariableArchProtocolGuid).
>
> (Well the PCD DXE driver is shared / used by all of the DXE modules, so
> it couldn't depend on gEfiVariableArchProtocolGuid in any case at all.)
>
> Ultimately, in the optimal case, the PcdLib instance for DXE should
> "know" if any PCDs used by the main driver module are DynamicHII, and in
> that case, the PcdLib instance should create a dependency on the
> variable read (and possibly write) services. I think edk2 simply doesn't
> support this use case at this moment (it is ultimately a DSC decision
> what type we give to a PCD, choosing from the possibilities listed in
> the DEC), so we have to take care of it ourselves. I do see that this
> limits the reusability of individual drivers across different DSC files
> -- if another DSC makes the same PCD fixed, for example, then the
> gEfiVariableArchProtocolGuid dependency becomes useless / needlessly
> restrictive in the driver.
>
> We could factor out just the gEfiVariableArchProtocolGuid depex into
> another NULL class lib, and hook that into PlatformHasAcpiDtDxe in the
> DSC file, because in the DSC we do know that the PCD is DynamicHII. If
> you feel strongly about it, it's doable, but I certainly think it's
> overkill. PlatformHasAcpiDtDxe got "Platform" in its name; it's pretty
> unlikely to be reused in any other DSC. (Even ArmVirtXen has a different
> implementation, which does not use the PCD.) I feel the proposed patch
> should be fine -- what's your take?
>

I missed the bit where gRT->GetVariable() is only called for HII style
dynamic PCDs, so I agree that it makes sense to deal with things as
you are doing here.

Re: [PATCH 00/10] ArmPkg, ArmPlatformPkg, ArmVirtPkg, EmbeddedPkg, Omap35xxPkg: depex fixes

[Ard Biesheuvel]

On 12 April 2018 at 02:55, Laszlo Ersek <lersek@redhat.com> wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: depex_fixes
>
> ArmVirtQemu boots again, it just took a few more patches than I expected
> :)
>
> Some of these patches will have to be ported to edk2-platforms, I think.
>
> 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>
>
> Thanks,
> Laszlo
>
> Laszlo Ersek (10):
>   Omap35xxPkg/InterruptDxe: replace CPU Arch Protocol depex with notify
>   ArmPkg/ArmGicDxe: annotate protocol usage in "ArmGicDxe.inf"
>   ArmPkg/CpuDxe: order CpuDxe after ArmGicDxe via protocol depex
>   EmbeddedPkg: introduce NvVarStoreFormattedLib
>   ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly
>   ArmPlatformPkg/NorFlashDxe: cue the variable driver with
>     NvVarStoreFormatted
>   ArmPlatformPkg/NorFlashDxe: depend on gEfiCpuArchProtocolGuid
>   ArmPlatformPkg/PL031RealTimeClockLib: depend on
>     gEfiCpuArchProtocolGuid
>   ArmVirtPkg/PlatformHasAcpiDtDxe: depend on
>     gEfiVariableArchProtocolGuid
>   ArmVirtPkg/ArmVirtQemu: hook NvVarStoreFormattedLib into
>     VariableRuntimeDxe
>

Laszlo,

Thanks a lot for taking care of this. I am glad we finally got rid of
the BEFORE depex in the NOR flash driver, which has been problematic
for years.

For the series,

Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

but please allow some time for Leif to chime in as well.

Thanks,
Ard.



>  ArmPkg/ArmPkg.dec                                                      |  2 -
>  ArmPkg/Drivers/ArmGic/ArmGicDxe.inf                                    |  8 +-
>  ArmPkg/Drivers/CpuDxe/CpuDxe.inf                                       |  2 +-
>  ArmPlatformPkg/ArmPlatformPkg.dec                                      |  4 -
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c                       | 13 +---
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h                       |  6 --
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf                     |  7 +-
>  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c                    | 22 +++---
>  ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf |  5 +-
>  ArmVirtPkg/ArmVirtQemu.dsc                                             |  1 +
>  ArmVirtPkg/ArmVirtQemuKernel.dsc                                       |  1 +
>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf               |  2 +-
>  EmbeddedPkg/EmbeddedPkg.dec                                            |  3 +
>  EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h                         | 39 ++++++++++
>  EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c    | 41 ++++++++++
>  EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf  | 52 +++++++++++++
>  Omap35xxPkg/InterruptDxe/HardwareInterrupt.c                           | 81 +++++++++++++++-----
>  Omap35xxPkg/InterruptDxe/InterruptDxe.inf                              |  6 +-
>  18 files changed, 230 insertions(+), 65 deletions(-)
>  create mode 100644 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
>  create mode 100644 EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h
>  create mode 100644 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c
>
> --
> 2.14.1.3.gb7cf6e02401b
>

Re: [PATCH 00/10] ArmPkg, ArmPlatformPkg, ArmVirtPkg, EmbeddedPkg, Omap35xxPkg: depex fixes

[Steve Capper]

On 12 April 2018 at 11:09, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 12 April 2018 at 02:55, Laszlo Ersek <lersek@redhat.com> wrote:
>> Repo:   https://github.com/lersek/edk2.git
>> Branch: depex_fixes
>>
>> ArmVirtQemu boots again, it just took a few more patches than I expected
>> :)
>>
>> Some of these patches will have to be ported to edk2-platforms, I think.
>>
>> 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>
>>
>> Thanks,
>> Laszlo
>>
>> Laszlo Ersek (10):
>>   Omap35xxPkg/InterruptDxe: replace CPU Arch Protocol depex with notify
>>   ArmPkg/ArmGicDxe: annotate protocol usage in "ArmGicDxe.inf"
>>   ArmPkg/CpuDxe: order CpuDxe after ArmGicDxe via protocol depex
>>   EmbeddedPkg: introduce NvVarStoreFormattedLib
>>   ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly
>>   ArmPlatformPkg/NorFlashDxe: cue the variable driver with
>>     NvVarStoreFormatted
>>   ArmPlatformPkg/NorFlashDxe: depend on gEfiCpuArchProtocolGuid
>>   ArmPlatformPkg/PL031RealTimeClockLib: depend on
>>     gEfiCpuArchProtocolGuid
>>   ArmVirtPkg/PlatformHasAcpiDtDxe: depend on
>>     gEfiVariableArchProtocolGuid
>>   ArmVirtPkg/ArmVirtQemu: hook NvVarStoreFormattedLib into
>>     VariableRuntimeDxe
>>
>
> Laszlo,
>
> Thanks a lot for taking care of this. I am glad we finally got rid of
> the BEFORE depex in the NOR flash driver, which has been problematic
> for years.
>
> For the series,
>
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> but please allow some time for Leif to chime in as well.
>
> Thanks,
> Ard.
>
>
>

Hi Laszlo,
Thanks for this!

I've booted with both DEBUG and RELEASE configurations on an FVP model
using this series.
I built this on Debian Stretch (using the AArch64 cross compiler).

In order to test on FVP (which is defined in edk2-platforms), I made a
simple modification to:
edk2-platforms/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
(Basically mirroring what you did in patch #10)

FWIW, feel free to add:
Tested-by: Steve Capper <steve.capper@linaro.org>

Cheers,
--
Steve

Re: [PATCH 09/10] ArmVirtPkg/PlatformHasAcpiDtDxe: depend on gEfiVariableArchProtocolGuid

[Gao, Liming]

Laszlo:
  We also have the similar idea that auto appends Variable read or Variable write protocol. Build tool can know which PCD is configured as DynamicHii. But, it doesn't know whether this PCD is consumed or produced in the driver entry point. If PCD is used in driver entry point, Build tool needs to append Variable arch protocol depex. Otherwise, BaseTool doesn't need do it. Further, I think BaseTools can use PCD usage described in Pcd section. CONSUMES means PCD read in the driver entry point, PRODUCES means PCD write in the driver entry point. Seemly, this solution can work well. But, it depends on the correct PCD usage in module INF. If PCD usage is not described or wrongly described, it may cause system boot failure. Consider its impact, we have not added this support. 

[Pcd]
  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate        ## CONSUMES
  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits         ## SOMETIMES_CONSUMES
  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity           ## PRODUCES
  gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits         ## SOMETIMES_PRODUCES

Thanks
Liming
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
> Sent: Thursday, April 12, 2018 5:06 PM
> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>
> Subject: Re: [edk2] [PATCH 09/10] ArmVirtPkg/PlatformHasAcpiDtDxe: depend on gEfiVariableArchProtocolGuid
> 
> On 04/12/18 08:28, Ard Biesheuvel wrote:
> > On 12 April 2018 at 02:55, Laszlo Ersek <lersek@redhat.com> wrote:
> >> PlatformHasAcpiDtDxe consumes the DynamicHii PCD called
> >> "gArmVirtTokenSpaceGuid.PcdForceNoAcpi". The PcdGetBool() library call
> >> terminates in gRT->GetVariable(), in the MdeModulePkg/Universal/PCD/Dxe
> >> driver. Put "gEfiVariableArchProtocolGuid" on PlatformHasAcpiDtDxe's DEPEX
> >> so that we not attempt the call before the PCD driver can successfully
> >> read non-volatile variables.
> >>
> >
> > OK, so does this mean that UefiRuntimeServicesTableLib deliberately
> > does not depex on gEfiVariableArchProtocolGuid,
> 
> Yes, I do think so; the runtime services are expected to become
> available gradually. For example, "MdePkg/Include/Protocol/Variable.h"
> has a great long comment on this.
> 
> > and it will return
> > EFI_NOT_AVAILABLE_YET when called too early? If that is the case, I
> > would assume it is the responsibility of the PcdLib implementation to
> > deal with that, not the PcdLib clients.
> 
> Yes, this idea occurred to me as well, but I think it is not correct.
> The PcdLib instance for the DXE phase is responsible for talking to the
> PCD protocol, and indeed it has a depex on that protocol
> (MdePkg/Library/DxePcdLib/DxePcdLib.inf). However, if a module uses
> dynamic PCDs, but none of those dynamic PCDs are HII (i.e., backed by
> non-volatile UEFI variables), then IMO neither the PcdLib instance, nor
> the PCD DXE driver, should depend on the variable read service
> (gEfiVariableArchProtocolGuid).
> 
> (Well the PCD DXE driver is shared / used by all of the DXE modules, so
> it couldn't depend on gEfiVariableArchProtocolGuid in any case at all.)
> 
> Ultimately, in the optimal case, the PcdLib instance for DXE should
> "know" if any PCDs used by the main driver module are DynamicHII, and in
> that case, the PcdLib instance should create a dependency on the
> variable read (and possibly write) services. I think edk2 simply doesn't
> support this use case at this moment (it is ultimately a DSC decision
> what type we give to a PCD, choosing from the possibilities listed in
> the DEC), so we have to take care of it ourselves. I do see that this
> limits the reusability of individual drivers across different DSC files
> -- if another DSC makes the same PCD fixed, for example, then the
> gEfiVariableArchProtocolGuid dependency becomes useless / needlessly
> restrictive in the driver.
> 
> We could factor out just the gEfiVariableArchProtocolGuid depex into
> another NULL class lib, and hook that into PlatformHasAcpiDtDxe in the
> DSC file, because in the DSC we do know that the PCD is DynamicHII. If
> you feel strongly about it, it's doable, but I certainly think it's
> overkill. PlatformHasAcpiDtDxe got "Platform" in its name; it's pretty
> unlikely to be reused in any other DSC. (Even ArmVirtXen has a different
> implementation, which does not use the PCD.) I feel the proposed patch
> should be fine -- what's your take?
> 
> Thanks!
> Laszlo
> 
> >
> >
> >> 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>
> >> ---
> >>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
> b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
> >> index 5351e741aa41..8346f7f24b32 100644
> >> --- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
> >> +++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
> >> @@ -44,7 +44,7 @@ [Guids]
> >>    gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
> >>
> >>  [Pcd]
> >>    gArmVirtTokenSpaceGuid.PcdForceNoAcpi
> >>
> >>  [Depex]
> >> -  TRUE
> >> +  gEfiVariableArchProtocolGuid
> >> --
> >> 2.14.1.3.gb7cf6e02401b
> >>
> >>
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 00/10] ArmPkg, ArmPlatformPkg, ArmVirtPkg, EmbeddedPkg, Omap35xxPkg: depex fixes

[Laszlo Ersek]

On 04/12/18 12:09, Ard Biesheuvel wrote:
> On 12 April 2018 at 02:55, Laszlo Ersek <lersek@redhat.com> wrote:
>> Repo:   https://github.com/lersek/edk2.git
>> Branch: depex_fixes
>>
>> ArmVirtQemu boots again, it just took a few more patches than I expected
>> :)
>>
>> Some of these patches will have to be ported to edk2-platforms, I think.
>>
>> 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>
>>
>> Thanks,
>> Laszlo
>>
>> Laszlo Ersek (10):
>>   Omap35xxPkg/InterruptDxe: replace CPU Arch Protocol depex with notify
>>   ArmPkg/ArmGicDxe: annotate protocol usage in "ArmGicDxe.inf"
>>   ArmPkg/CpuDxe: order CpuDxe after ArmGicDxe via protocol depex
>>   EmbeddedPkg: introduce NvVarStoreFormattedLib
>>   ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly
>>   ArmPlatformPkg/NorFlashDxe: cue the variable driver with
>>     NvVarStoreFormatted
>>   ArmPlatformPkg/NorFlashDxe: depend on gEfiCpuArchProtocolGuid
>>   ArmPlatformPkg/PL031RealTimeClockLib: depend on
>>     gEfiCpuArchProtocolGuid
>>   ArmVirtPkg/PlatformHasAcpiDtDxe: depend on
>>     gEfiVariableArchProtocolGuid
>>   ArmVirtPkg/ArmVirtQemu: hook NvVarStoreFormattedLib into
>>     VariableRuntimeDxe
>>
> 
> Laszlo,
> 
> Thanks a lot for taking care of this. I am glad we finally got rid of
> the BEFORE depex in the NOR flash driver, which has been problematic
> for years.
> 
> For the series,
> 
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Thank you!

> 
> but please allow some time for Leif to chime in as well.

Will certainly do!

Cheers!
Laszlo

Re: [PATCH 00/10] ArmPkg, ArmPlatformPkg, ArmVirtPkg, EmbeddedPkg, Omap35xxPkg: depex fixes

[Laszlo Ersek]

On 04/12/18 15:39, Steve Capper wrote:
> On 12 April 2018 at 11:09, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> On 12 April 2018 at 02:55, Laszlo Ersek <lersek@redhat.com> wrote:
>>> Repo:   https://github.com/lersek/edk2.git
>>> Branch: depex_fixes
>>>
>>> ArmVirtQemu boots again, it just took a few more patches than I expected
>>> :)
>>>
>>> Some of these patches will have to be ported to edk2-platforms, I think.
>>>
>>> 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>
>>>
>>> Thanks,
>>> Laszlo
>>>
>>> Laszlo Ersek (10):
>>>   Omap35xxPkg/InterruptDxe: replace CPU Arch Protocol depex with notify
>>>   ArmPkg/ArmGicDxe: annotate protocol usage in "ArmGicDxe.inf"
>>>   ArmPkg/CpuDxe: order CpuDxe after ArmGicDxe via protocol depex
>>>   EmbeddedPkg: introduce NvVarStoreFormattedLib
>>>   ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly
>>>   ArmPlatformPkg/NorFlashDxe: cue the variable driver with
>>>     NvVarStoreFormatted
>>>   ArmPlatformPkg/NorFlashDxe: depend on gEfiCpuArchProtocolGuid
>>>   ArmPlatformPkg/PL031RealTimeClockLib: depend on
>>>     gEfiCpuArchProtocolGuid
>>>   ArmVirtPkg/PlatformHasAcpiDtDxe: depend on
>>>     gEfiVariableArchProtocolGuid
>>>   ArmVirtPkg/ArmVirtQemu: hook NvVarStoreFormattedLib into
>>>     VariableRuntimeDxe
>>>
>>
>> Laszlo,
>>
>> Thanks a lot for taking care of this. I am glad we finally got rid of
>> the BEFORE depex in the NOR flash driver, which has been problematic
>> for years.
>>
>> For the series,
>>
>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> but please allow some time for Leif to chime in as well.
>>
>> Thanks,
>> Ard.
>>
>>
>>
> 
> Hi Laszlo,
> Thanks for this!
> 
> I've booted with both DEBUG and RELEASE configurations on an FVP model
> using this series.
> I built this on Debian Stretch (using the AArch64 cross compiler).
> 
> In order to test on FVP (which is defined in edk2-platforms), I made a
> simple modification to:
> edk2-platforms/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc
> (Basically mirroring what you did in patch #10)

That's great; I'm relieved it wasn't more complex than that. :)

> FWIW, feel free to add:
> Tested-by: Steve Capper <steve.capper@linaro.org>

Thank you -- I'll add your T-b to the following patches: #2, #3, #4, #5,
#6, #7, #8. As those patches affect modules that seem to be used by
"edk2-platforms/Platform/ARM/VExpressPkg/ArmVExpress-FVP-AArch64.dsc".

Cheers!
Laszlo

Re: [PATCH 00/10] ArmPkg, ArmPlatformPkg, ArmVirtPkg, EmbeddedPkg, Omap35xxPkg: depex fixes

[Supreeth Venkatesh]

Thanks Laszlo.
It works for me too.

Supreeth

-----Original Message-----
From: Laszlo Ersek <lersek@redhat.com>
Sent: Wednesday, April 11, 2018 7:56 PM
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 00/10] ArmPkg, ArmPlatformPkg, ArmVirtPkg, EmbeddedPkg, Omap35xxPkg: depex fixes

Repo:   https://github.com/lersek/edk2.git
Branch: depex_fixes

ArmVirtQemu boots again, it just took a few more patches than I expected
:)

Some of these patches will have to be ported to edk2-platforms, I think.

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>

Thanks,
Laszlo

Laszlo Ersek (10):
  Omap35xxPkg/InterruptDxe: replace CPU Arch Protocol depex with notify
  ArmPkg/ArmGicDxe: annotate protocol usage in "ArmGicDxe.inf"
  ArmPkg/CpuDxe: order CpuDxe after ArmGicDxe via protocol depex
  EmbeddedPkg: introduce NvVarStoreFormattedLib
  ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly
  ArmPlatformPkg/NorFlashDxe: cue the variable driver with
    NvVarStoreFormatted
  ArmPlatformPkg/NorFlashDxe: depend on gEfiCpuArchProtocolGuid
  ArmPlatformPkg/PL031RealTimeClockLib: depend on
    gEfiCpuArchProtocolGuid
  ArmVirtPkg/PlatformHasAcpiDtDxe: depend on
    gEfiVariableArchProtocolGuid
  ArmVirtPkg/ArmVirtQemu: hook NvVarStoreFormattedLib into
    VariableRuntimeDxe

 ArmPkg/ArmPkg.dec                                                      |  2 -
 ArmPkg/Drivers/ArmGic/ArmGicDxe.inf                                    |  8 +-
 ArmPkg/Drivers/CpuDxe/CpuDxe.inf                                       |  2 +-
 ArmPlatformPkg/ArmPlatformPkg.dec                                      |  4 -
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c                       | 13 +---
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h                       |  6 --
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf                     |  7 +-
 ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c                    | 22 +++---
 ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf |  5 +-
 ArmVirtPkg/ArmVirtQemu.dsc                                             |  1 +
 ArmVirtPkg/ArmVirtQemuKernel.dsc                                       |  1 +
 ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf               |  2 +-
 EmbeddedPkg/EmbeddedPkg.dec                                            |  3 +
 EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h                         | 39 ++++++++++
 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c    | 41 ++++++++++
 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf  | 52 +++++++++++++
 Omap35xxPkg/InterruptDxe/HardwareInterrupt.c                           | 81 +++++++++++++++-----
 Omap35xxPkg/InterruptDxe/InterruptDxe.inf                              |  6 +-
 18 files changed, 230 insertions(+), 65 deletions(-)  create mode 100644 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
 create mode 100644 EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h
 create mode 100644 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c

--
2.14.1.3.gb7cf6e02401b

IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.

Re: [PATCH 09/10] ArmVirtPkg/PlatformHasAcpiDtDxe: depend on gEfiVariableArchProtocolGuid

[Laszlo Ersek]

On 04/12/18 17:16, Gao, Liming wrote:
> Laszlo:
>   We also have the similar idea that auto appends Variable read or Variable write protocol. Build tool can know which PCD is configured as DynamicHii. But, it doesn't know whether this PCD is consumed or produced in the driver entry point. If PCD is used in driver entry point, Build tool needs to append Variable arch protocol depex. Otherwise, BaseTool doesn't need do it. Further, I think BaseTools can use PCD usage described in Pcd section. CONSUMES means PCD read in the driver entry point, PRODUCES means PCD write in the driver entry point. Seemly, this solution can work well. But, it depends on the correct PCD usage in module INF. If PCD usage is not described or wrongly described, it may cause system boot failure. Consider its impact, we have not added this support. 
> 
> [Pcd]
>   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultBaudRate        ## CONSUMES
>   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultDataBits         ## SOMETIMES_CONSUMES
>   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultParity           ## PRODUCES
>   gEfiMdePkgTokenSpaceGuid.PcdUartDefaultStopBits         ## SOMETIMES_PRODUCES

Right, I agree with everything you said. The usage comments are really
helpful (and not just for PCDs but also for protocols, guids, ...) but
in their current form they are indeed a bit too lax for basing
build-time decisions on them. If we ever want to support this in
BaseTools, then I think it will take a first-class extension to the
current INF file format; i.e., with syntax and semantic checking, error
messages, build failures etc. I think we can manage the current
situation, we just have to be aware of it.

Thank you!
Laszlo

> 
> Thanks
> Liming
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Laszlo Ersek
>> Sent: Thursday, April 12, 2018 5:06 PM
>> To: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> Cc: edk2-devel@lists.01.org; Leif Lindholm <leif.lindholm@linaro.org>
>> Subject: Re: [edk2] [PATCH 09/10] ArmVirtPkg/PlatformHasAcpiDtDxe: depend on gEfiVariableArchProtocolGuid
>>
>> On 04/12/18 08:28, Ard Biesheuvel wrote:
>>> On 12 April 2018 at 02:55, Laszlo Ersek <lersek@redhat.com> wrote:
>>>> PlatformHasAcpiDtDxe consumes the DynamicHii PCD called
>>>> "gArmVirtTokenSpaceGuid.PcdForceNoAcpi". The PcdGetBool() library call
>>>> terminates in gRT->GetVariable(), in the MdeModulePkg/Universal/PCD/Dxe
>>>> driver. Put "gEfiVariableArchProtocolGuid" on PlatformHasAcpiDtDxe's DEPEX
>>>> so that we not attempt the call before the PCD driver can successfully
>>>> read non-volatile variables.
>>>>
>>>
>>> OK, so does this mean that UefiRuntimeServicesTableLib deliberately
>>> does not depex on gEfiVariableArchProtocolGuid,
>>
>> Yes, I do think so; the runtime services are expected to become
>> available gradually. For example, "MdePkg/Include/Protocol/Variable.h"
>> has a great long comment on this.
>>
>>> and it will return
>>> EFI_NOT_AVAILABLE_YET when called too early? If that is the case, I
>>> would assume it is the responsibility of the PcdLib implementation to
>>> deal with that, not the PcdLib clients.
>>
>> Yes, this idea occurred to me as well, but I think it is not correct.
>> The PcdLib instance for the DXE phase is responsible for talking to the
>> PCD protocol, and indeed it has a depex on that protocol
>> (MdePkg/Library/DxePcdLib/DxePcdLib.inf). However, if a module uses
>> dynamic PCDs, but none of those dynamic PCDs are HII (i.e., backed by
>> non-volatile UEFI variables), then IMO neither the PcdLib instance, nor
>> the PCD DXE driver, should depend on the variable read service
>> (gEfiVariableArchProtocolGuid).
>>
>> (Well the PCD DXE driver is shared / used by all of the DXE modules, so
>> it couldn't depend on gEfiVariableArchProtocolGuid in any case at all.)
>>
>> Ultimately, in the optimal case, the PcdLib instance for DXE should
>> "know" if any PCDs used by the main driver module are DynamicHII, and in
>> that case, the PcdLib instance should create a dependency on the
>> variable read (and possibly write) services. I think edk2 simply doesn't
>> support this use case at this moment (it is ultimately a DSC decision
>> what type we give to a PCD, choosing from the possibilities listed in
>> the DEC), so we have to take care of it ourselves. I do see that this
>> limits the reusability of individual drivers across different DSC files
>> -- if another DSC makes the same PCD fixed, for example, then the
>> gEfiVariableArchProtocolGuid dependency becomes useless / needlessly
>> restrictive in the driver.
>>
>> We could factor out just the gEfiVariableArchProtocolGuid depex into
>> another NULL class lib, and hook that into PlatformHasAcpiDtDxe in the
>> DSC file, because in the DSC we do know that the PCD is DynamicHII. If
>> you feel strongly about it, it's doable, but I certainly think it's
>> overkill. PlatformHasAcpiDtDxe got "Platform" in its name; it's pretty
>> unlikely to be reused in any other DSC. (Even ArmVirtXen has a different
>> implementation, which does not use the PCD.) I feel the proposed patch
>> should be fine -- what's your take?
>>
>> Thanks!
>> Laszlo
>>
>>>
>>>
>>>> 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>
>>>> ---
>>>>  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf | 2 +-
>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
>> b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
>>>> index 5351e741aa41..8346f7f24b32 100644
>>>> --- a/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
>>>> +++ b/ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf
>>>> @@ -44,7 +44,7 @@ [Guids]
>>>>    gEdkiiPlatformHasDeviceTreeGuid ## SOMETIMES_PRODUCES ## PROTOCOL
>>>>
>>>>  [Pcd]
>>>>    gArmVirtTokenSpaceGuid.PcdForceNoAcpi
>>>>
>>>>  [Depex]
>>>> -  TRUE
>>>> +  gEfiVariableArchProtocolGuid
>>>> --
>>>> 2.14.1.3.gb7cf6e02401b
>>>>
>>>>
>>
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel

Re: [PATCH 00/10] ArmPkg, ArmPlatformPkg, ArmVirtPkg, EmbeddedPkg, Omap35xxPkg: depex fixes

[Leif Lindholm]

On Thu, Apr 12, 2018 at 12:09:46PM +0200, Ard Biesheuvel wrote:
> On 12 April 2018 at 02:55, Laszlo Ersek <lersek@redhat.com> wrote:
> > Repo:   https://github.com/lersek/edk2.git
> > Branch: depex_fixes
> >
> > ArmVirtQemu boots again, it just took a few more patches than I expected
> > :)
> >
> > Some of these patches will have to be ported to edk2-platforms, I think.
> >
> > 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>
> >
> > Thanks,
> > Laszlo
> >
> > Laszlo Ersek (10):
> >   Omap35xxPkg/InterruptDxe: replace CPU Arch Protocol depex with notify
> >   ArmPkg/ArmGicDxe: annotate protocol usage in "ArmGicDxe.inf"
> >   ArmPkg/CpuDxe: order CpuDxe after ArmGicDxe via protocol depex
> >   EmbeddedPkg: introduce NvVarStoreFormattedLib
> >   ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly
> >   ArmPlatformPkg/NorFlashDxe: cue the variable driver with
> >     NvVarStoreFormatted
> >   ArmPlatformPkg/NorFlashDxe: depend on gEfiCpuArchProtocolGuid
> >   ArmPlatformPkg/PL031RealTimeClockLib: depend on
> >     gEfiCpuArchProtocolGuid
> >   ArmVirtPkg/PlatformHasAcpiDtDxe: depend on
> >     gEfiVariableArchProtocolGuid
> >   ArmVirtPkg/ArmVirtQemu: hook NvVarStoreFormattedLib into
> >     VariableRuntimeDxe
> >
> 
> Laszlo,
> 
> Thanks a lot for taking care of this. I am glad we finally got rid of
> the BEFORE depex in the NOR flash driver, which has been problematic
> for years.
> 
> For the series,
> 
> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> but please allow some time for Leif to chime in as well.

Well, there are a couple of places where I could nitpick on
alphabetical sorting of includes, but none in files that weren't
already disorderly. And like Ard, I am very grateful for this quite
invasive (yet clean) bugfix.

So, for 1-9/10:
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

> 
> Thanks,
> Ard.
> 
> 
> 
> >  ArmPkg/ArmPkg.dec                                                      |  2 -
> >  ArmPkg/Drivers/ArmGic/ArmGicDxe.inf                                    |  8 +-
> >  ArmPkg/Drivers/CpuDxe/CpuDxe.inf                                       |  2 +-
> >  ArmPlatformPkg/ArmPlatformPkg.dec                                      |  4 -
> >  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.c                       | 13 +---
> >  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.h                       |  6 --
> >  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf                     |  7 +-
> >  ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashFvbDxe.c                    | 22 +++---
> >  ArmPlatformPkg/Library/PL031RealTimeClockLib/PL031RealTimeClockLib.inf |  5 +-
> >  ArmVirtPkg/ArmVirtQemu.dsc                                             |  1 +
> >  ArmVirtPkg/ArmVirtQemuKernel.dsc                                       |  1 +
> >  ArmVirtPkg/PlatformHasAcpiDtDxe/PlatformHasAcpiDtDxe.inf               |  2 +-
> >  EmbeddedPkg/EmbeddedPkg.dec                                            |  3 +
> >  EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h                         | 39 ++++++++++
> >  EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c    | 41 ++++++++++
> >  EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf  | 52 +++++++++++++
> >  Omap35xxPkg/InterruptDxe/HardwareInterrupt.c                           | 81 +++++++++++++++-----
> >  Omap35xxPkg/InterruptDxe/InterruptDxe.inf                              |  6 +-
> >  18 files changed, 230 insertions(+), 65 deletions(-)
> >  create mode 100644 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.inf
> >  create mode 100644 EmbeddedPkg/Include/Guid/NvVarStoreFormatted.h
> >  create mode 100644 EmbeddedPkg/Library/NvVarStoreFormattedLib/NvVarStoreFormattedLib.c
> >
> > --
> > 2.14.1.3.gb7cf6e02401b
> >

Re: [PATCH 00/10] ArmPkg, ArmPlatformPkg, ArmVirtPkg, EmbeddedPkg, Omap35xxPkg: depex fixes

[Laszlo Ersek]

On 04/12/18 19:23, Leif Lindholm wrote:
> On Thu, Apr 12, 2018 at 12:09:46PM +0200, Ard Biesheuvel wrote:
>> On 12 April 2018 at 02:55, Laszlo Ersek <lersek@redhat.com> wrote:
>>> Repo:   https://github.com/lersek/edk2.git
>>> Branch: depex_fixes
>>>
>>> ArmVirtQemu boots again, it just took a few more patches than I expected
>>> :)
>>>
>>> Some of these patches will have to be ported to edk2-platforms, I think.
>>>
>>> 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>
>>>
>>> Thanks,
>>> Laszlo
>>>
>>> Laszlo Ersek (10):
>>>   Omap35xxPkg/InterruptDxe: replace CPU Arch Protocol depex with notify
>>>   ArmPkg/ArmGicDxe: annotate protocol usage in "ArmGicDxe.inf"
>>>   ArmPkg/CpuDxe: order CpuDxe after ArmGicDxe via protocol depex
>>>   EmbeddedPkg: introduce NvVarStoreFormattedLib
>>>   ArmPlatformPkg/NorFlashDxe: initialize varstore headers eagerly
>>>   ArmPlatformPkg/NorFlashDxe: cue the variable driver with
>>>     NvVarStoreFormatted
>>>   ArmPlatformPkg/NorFlashDxe: depend on gEfiCpuArchProtocolGuid
>>>   ArmPlatformPkg/PL031RealTimeClockLib: depend on
>>>     gEfiCpuArchProtocolGuid
>>>   ArmVirtPkg/PlatformHasAcpiDtDxe: depend on
>>>     gEfiVariableArchProtocolGuid
>>>   ArmVirtPkg/ArmVirtQemu: hook NvVarStoreFormattedLib into
>>>     VariableRuntimeDxe
>>>
>>
>> Laszlo,
>>
>> Thanks a lot for taking care of this. I am glad we finally got rid of
>> the BEFORE depex in the NOR flash driver, which has been problematic
>> for years.
>>
>> For the series,
>>
>> Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> but please allow some time for Leif to chime in as well.
> 
> Well, there are a couple of places where I could nitpick on
> alphabetical sorting of includes,

And, believe me, you would have my total agreement :), but in such
cases, there's always a fork in the road: (a) add a separate patch that
first sorts the includes and [LibraryClasses], without functional
changes, or (b) just stick with the existing disorder, and get in and
out as surgically as possible. My personal preference is (a), but it has
drawn disagreement -- even accusations of pedantry :) :) --, and/or
suggestions to squash such patches with functional changes, in the past,
so I trod more lightly now. Rest assured, I didn't *miss* those places,
I just elected to close my eyes! ;)

> but none in files that weren't
> already disorderly. And like Ard, I am very grateful for this quite
> invasive (yet clean) bugfix.
> 
> So, for 1-9/10:
> Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

Thank you! I shall get my TODO list (or, should I say, TODO "hash
table", considering its cleanliness) in order, and then I'll get to
pushing stuff.

I *very* much appreciate the quick feedback on this!

Cheers,
Laszlo

Re: [PATCH 00/10] ArmPkg, ArmPlatformPkg, ArmVirtPkg, EmbeddedPkg, Omap35xxPkg: depex fixes

[Laszlo Ersek]

On 04/12/18 18:51, Supreeth Venkatesh wrote:
> Thanks Laszlo.
> It works for me too.

Appreciate the testing!
Laszlo

derailing into patch style discussion

[Leif Lindholm]

Since you already have my r-b on the set, I'll pick up the style
topic, partly because I'm not sure if I've ever explained my
thinking publicly in words that anyone other than Ard understands.

On Thu, Apr 12, 2018 at 07:45:19PM +0200, Laszlo Ersek wrote:
> > Well, there are a couple of places where I could nitpick on
> > alphabetical sorting of includes,
> 
> And, believe me, you would have my total agreement :), but in such
> cases, there's always a fork in the road: (a) add a separate patch that
> first sorts the includes and [LibraryClasses], without functional
> changes, or (b) just stick with the existing disorder, and get in and
> out as surgically as possible. My personal preference is (a), but it has
> drawn disagreement -- even accusations of pedantry :) :) --, and/or
> suggestions to squash such patches with functional changes, in the past,
> so I trod more lightly now. Rest assured, I didn't *miss* those places,
> I just elected to close my eyes! ;)

Right :)

So, yes - I do strongly agree with the idea of keeping functionality
and style changes separate (ask Evan), so that wasn't exactly what I
was referring to.

In general my internal "optimal" situation is one where the purely
functional diff leaves the code in a (quite subjectively, since it's
still not conformant) better state on average than it was.

My subjective mark of optimal is that which would minimise any
subsequent patch that was _only_ a style cleanup.

To pick an example from this set (1/10):
diff --git a/Omap35xxPkg/InterruptDxe/InterruptDxe.inf
b/Omap35xxPkg/InterruptDxe/InterruptDxe.inf
index 6deb8c3f675c..61ad89af2758 100644
--- a/Omap35xxPkg/InterruptDxe/InterruptDxe.inf
+++ b/Omap35xxPkg/InterruptDxe/InterruptDxe.inf
@@ -41,14 +41,14 @@ [LibraryClasses]
   PrintLib
   UefiDriverEntryPoint
   IoLib
   ArmLib

 [Protocols]
 -  gHardwareInterruptProtocolGuid
 -  gEfiCpuArchProtocolGuid
 +  gHardwareInterruptProtocolGuid  ## PRODUCES
 +  gEfiCpuArchProtocolGuid         ## CONSUMES ## NOTIFY

---
This one is pretty straightforward - without touching any non-modified
lines, these _could_ be reordered alphabetically.

(Yes, that change may have functional side-effects, but only on
undefined behaviour, so no different from rebasing to a version with
newer BaseTools can give you.)

Where the minimum diff logic comes in is in situations like (6/10):
diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
index 812dafd065b2..c40ac27a6599 100644
--- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
+++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
@@ -28,12 +28,13 @@ [Sources.common]
   NorFlashBlockIoDxe.c

 [Packages]
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
   ArmPlatformPkg/ArmPlatformPkg.dec
+  EmbeddedPkg/EmbeddedPkg.dec

---
Applying my "optimal" rule here would have meant inserting the new
line before MdePkg/MdeModulePkg .decs instead. That way a cleanup
patch would end up doing

[Packages]
+  ArmPlatformPkg/ArmPlatformPkg.dec
   EmbeddedPkg/EmbeddedPkg.dec
   MdePkg/MdePkg.dec
   MdeModulePkg/MdeModulePkg.dec
-  ArmPlatformPkg/ArmPlatformPkg.dec

instead of

[Packages]
-  MdePkg/MdePkg.dec
-  MdeModulePkg/MdeModulePkg.dec
   ArmPlatformPkg/ArmPlatformPkg.dec
   EmbeddedPkg/EmbeddedPkg.dec
+  MdePkg/MdePkg.dec
+  MdeModulePkg/MdeModulePkg.dec


Anyway, this is all an aside - I just thought I would give you an
insight into the mind of Leif.

/
    Leif

Re: derailing into patch style discussion

[Laszlo Ersek]

On 04/12/18 20:13, Leif Lindholm wrote:
> Since you already have my r-b on the set, I'll pick up the style
> topic, partly because I'm not sure if I've ever explained my
> thinking publicly in words that anyone other than Ard understands.
> 
> On Thu, Apr 12, 2018 at 07:45:19PM +0200, Laszlo Ersek wrote:
>>> Well, there are a couple of places where I could nitpick on
>>> alphabetical sorting of includes,
>>
>> And, believe me, you would have my total agreement :), but in such
>> cases, there's always a fork in the road: (a) add a separate patch that
>> first sorts the includes and [LibraryClasses], without functional
>> changes, or (b) just stick with the existing disorder, and get in and
>> out as surgically as possible. My personal preference is (a), but it has
>> drawn disagreement -- even accusations of pedantry :) :) --, and/or
>> suggestions to squash such patches with functional changes, in the past,
>> so I trod more lightly now. Rest assured, I didn't *miss* those places,
>> I just elected to close my eyes! ;)
> 
> Right :)
> 
> So, yes - I do strongly agree with the idea of keeping functionality
> and style changes separate (ask Evan), so that wasn't exactly what I
> was referring to.
> 
> In general my internal "optimal" situation is one where the purely
> functional diff leaves the code in a (quite subjectively, since it's
> still not conformant) better state on average than it was.
> 
> My subjective mark of optimal is that which would minimise any
> subsequent patch that was _only_ a style cleanup.
> 
> To pick an example from this set (1/10):
> diff --git a/Omap35xxPkg/InterruptDxe/InterruptDxe.inf
> b/Omap35xxPkg/InterruptDxe/InterruptDxe.inf
> index 6deb8c3f675c..61ad89af2758 100644
> --- a/Omap35xxPkg/InterruptDxe/InterruptDxe.inf
> +++ b/Omap35xxPkg/InterruptDxe/InterruptDxe.inf
> @@ -41,14 +41,14 @@ [LibraryClasses]
>    PrintLib
>    UefiDriverEntryPoint
>    IoLib
>    ArmLib
> 
>  [Protocols]
>  -  gHardwareInterruptProtocolGuid
>  -  gEfiCpuArchProtocolGuid
>  +  gHardwareInterruptProtocolGuid  ## PRODUCES
>  +  gEfiCpuArchProtocolGuid         ## CONSUMES ## NOTIFY
> 
> ---
> This one is pretty straightforward - without touching any non-modified
> lines, these _could_ be reordered alphabetically.
> 
> (Yes, that change may have functional side-effects, but only on
> undefined behaviour, so no different from rebasing to a version with
> newer BaseTools can give you.)
> 
> Where the minimum diff logic comes in is in situations like (6/10):
> diff --git a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> index 812dafd065b2..c40ac27a6599 100644
> --- a/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> +++ b/ArmPlatformPkg/Drivers/NorFlashDxe/NorFlashDxe.inf
> @@ -28,12 +28,13 @@ [Sources.common]
>    NorFlashBlockIoDxe.c
> 
>  [Packages]
>    MdePkg/MdePkg.dec
>    MdeModulePkg/MdeModulePkg.dec
>    ArmPlatformPkg/ArmPlatformPkg.dec
> +  EmbeddedPkg/EmbeddedPkg.dec
> 
> ---
> Applying my "optimal" rule here would have meant inserting the new
> line before MdePkg/MdeModulePkg .decs instead. That way a cleanup
> patch would end up doing
> 
> [Packages]
> +  ArmPlatformPkg/ArmPlatformPkg.dec
>    EmbeddedPkg/EmbeddedPkg.dec
>    MdePkg/MdePkg.dec
>    MdeModulePkg/MdeModulePkg.dec
> -  ArmPlatformPkg/ArmPlatformPkg.dec
> 
> instead of
> 
> [Packages]
> -  MdePkg/MdePkg.dec
> -  MdeModulePkg/MdeModulePkg.dec
>    ArmPlatformPkg/ArmPlatformPkg.dec
>    EmbeddedPkg/EmbeddedPkg.dec
> +  MdePkg/MdePkg.dec
> +  MdeModulePkg/MdeModulePkg.dec
> 
> 
> Anyway, this is all an aside - I just thought I would give you an
> insight into the mind of Leif.

I do see the point -- basically, even if we don't imlement separate
cleanups, and just add the functional changes on top of whatever we
already have, if there are multiple ways to keep the functional changes
purely functional and focused, we had better select the one that at
least doesn't make the style worse. Using your [Packages] example, it's
possible *not* to increase the disorder, without actually sorting those
lines. I fully admit this eluded me -- I considered "messy" all-or-nothing.

I'll try to remember this next time. (It's honestly a bit difficult for
me, because if I take the time & effort to be considerate like this, I'd
mostly (though perhaps not always) just write the patch that cleans up
first.)

Thanks!
Laszlo

Re: [PATCH 00/10] ArmPkg, ArmPlatformPkg, ArmVirtPkg, EmbeddedPkg, Omap35xxPkg: depex fixes

[Laszlo Ersek]

On 04/12/18 02:55, Laszlo Ersek wrote:
> Repo:   https://github.com/lersek/edk2.git
> Branch: depex_fixes
> 
> ArmVirtQemu boots again, it just took a few more patches than I expected
> :)
> 
> Some of these patches will have to be ported to edk2-platforms, I think.
> 
> 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>

Pushed as commit range 153f5c7a93be..bf453d581ecf.

Thank you all!
Laszlo