[Linaro-uefi] Platforms/ARM: get rid of A PRIORI declaration for Dxe/Pcd.inf

Message ID 1461052172-17196-1-git-send-email-ard.biesheuvel@linaro.org
State Accepted
Commit ca8b09a0d6c31db5bf5ee955dc21b783d14157eb
Headers show

Commit Message

Ard Biesheuvel April 19, 2016, 7:49 a.m.
APRIORI declarations are evil. They force a driver to be dispatched
before any other drivers, while completely ignoring normal precedence
rules or protocol dependencies.

In this particular case, the DXE version of Pcd.inf is loaded a priori to
work around the problem that the default PcdLib resolution introduces a
a protocol dependency on gPcdProtocolGuid, which provides dynamic PCD
handling for other drivers, and is implemented by Pcd.inf. Since Pcd.inf
depends on PcdLib as well, it can never be dispatched in the ordinary way
if it inherits the default PcdLib resolution (since that will make it depend
on itself), and so it must be made to depend on the Null implementation of
PcdLib explicitly.

So add this explicit override, and drop the A PRIORI declarations.

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---

Note that the same reasoning applies to the PEI version of Pcd.inf, but we
don't normally use that, and in fact, it already has a similar Null library
class override in ArmVExpress-FVP-AArch64.dsc

 Platforms/ARM/Juno/ArmJuno.fdf                     | 4 ----
 Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf | 4 ----
 Platforms/ARM/VExpress/ArmVExpress.dsc.inc         | 5 ++++-
 3 files changed, 4 insertions(+), 9 deletions(-)

Comments

Ryan Harkin May 3, 2016, 11:09 a.m. | #1
On 19 April 2016 at 08:49, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> APRIORI declarations are evil. They force a driver to be dispatched
> before any other drivers, while completely ignoring normal precedence
> rules or protocol dependencies.
>
> In this particular case, the DXE version of Pcd.inf is loaded a priori to
> work around the problem that the default PcdLib resolution introduces a
> a protocol dependency on gPcdProtocolGuid, which provides dynamic PCD
> handling for other drivers, and is implemented by Pcd.inf. Since Pcd.inf
> depends on PcdLib as well, it can never be dispatched in the ordinary way
> if it inherits the default PcdLib resolution (since that will make it depend
> on itself), and so it must be made to depend on the Null implementation of
> PcdLib explicitly.
>
> So add this explicit override, and drop the A PRIORI declarations.
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Tested-by: Ryan Harkin <ryan.harkin@linaro.org>

Tested on Juno R0, R1 and R2, FVP Base AEMv8 and FVP Foundation models.

> ---
>
> Note that the same reasoning applies to the PEI version of Pcd.inf, but we
> don't normally use that, and in fact, it already has a similar Null library
> class override in ArmVExpress-FVP-AArch64.dsc
>
>  Platforms/ARM/Juno/ArmJuno.fdf                     | 4 ----
>  Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf | 4 ----
>  Platforms/ARM/VExpress/ArmVExpress.dsc.inc         | 5 ++++-
>  3 files changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/Platforms/ARM/Juno/ArmJuno.fdf b/Platforms/ARM/Juno/ArmJuno.fdf
> index a05151086910..3983c20de8e0 100644
> --- a/Platforms/ARM/Juno/ArmJuno.fdf
> +++ b/Platforms/ARM/Juno/ArmJuno.fdf
> @@ -86,10 +86,6 @@ READ_LOCK_CAP      = TRUE
>  READ_LOCK_STATUS   = TRUE
>  FvNameGuid         = B73FE497-B92E-416e-8326-45AD0D270092
>
> -  APRIORI DXE {
> -    INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
> -  }
> -
>    INF MdeModulePkg/Core/Dxe/DxeMain.inf
>    INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
>
> diff --git a/Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf b/Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf
> index 79c074c834d8..3bcdb1ca43f1 100644
> --- a/Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf
> +++ b/Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf
> @@ -73,10 +73,6 @@ READ_LOCK_CAP      = TRUE
>  READ_LOCK_STATUS   = TRUE
>  FvNameGuid         = 87940482-fc81-41c3-87e6-399cf85ac8a0
>
> -  APRIORI DXE {
> -    INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
> -  }
> -
>    INF MdeModulePkg/Core/Dxe/DxeMain.inf
>    INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
>
> diff --git a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
> index 9bb45c82a5e9..d898aef490f9 100644
> --- a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
> +++ b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
> @@ -443,7 +443,10 @@
>  !endif
>
>  [Components.common]
> -  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
> +  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
> +    <LibraryClasses>
> +      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> +  }
>
>    # Versatile Express FileSystem
>    ArmPlatformPkg/FileSystem/BootMonFs/BootMonFs.inf
> --
> 2.5.0
>

Patch

diff --git a/Platforms/ARM/Juno/ArmJuno.fdf b/Platforms/ARM/Juno/ArmJuno.fdf
index a05151086910..3983c20de8e0 100644
--- a/Platforms/ARM/Juno/ArmJuno.fdf
+++ b/Platforms/ARM/Juno/ArmJuno.fdf
@@ -86,10 +86,6 @@  READ_LOCK_CAP      = TRUE
 READ_LOCK_STATUS   = TRUE
 FvNameGuid         = B73FE497-B92E-416e-8326-45AD0D270092
 
-  APRIORI DXE {
-    INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
-  }
-
   INF MdeModulePkg/Core/Dxe/DxeMain.inf
   INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
 
diff --git a/Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf b/Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf
index 79c074c834d8..3bcdb1ca43f1 100644
--- a/Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf
+++ b/Platforms/ARM/VExpress/ArmVExpress-FVP-AArch64.fdf
@@ -73,10 +73,6 @@  READ_LOCK_CAP      = TRUE
 READ_LOCK_STATUS   = TRUE
 FvNameGuid         = 87940482-fc81-41c3-87e6-399cf85ac8a0
 
-  APRIORI DXE {
-    INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
-  }
-
   INF MdeModulePkg/Core/Dxe/DxeMain.inf
   INF MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
 
diff --git a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
index 9bb45c82a5e9..d898aef490f9 100644
--- a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
+++ b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
@@ -443,7 +443,10 @@ 
 !endif
 
 [Components.common]
-  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf
+  MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
+    <LibraryClasses>
+      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
+  }
 
   # Versatile Express FileSystem
   ArmPlatformPkg/FileSystem/BootMonFs/BootMonFs.inf