Re: [PATCH] ArmVirtPkg: use protocol-based DevicePathLib instance for most DXE modules

[Ard Biesheuvel <ard.biesheuvel@linaro.org>]

On 24 April 2018 at 02:50, Laszlo Ersek <lersek@redhat.com> wrote:
> Port OvmfPkg commit 5c3481b0b611e to ArmVirtPkg. Some explanation should
> be in order (because 5c3481b0b611e doesn't offer any):
>
> - The UefiDevicePathLibDevicePathProtocol instance uses the Device Path
>   Utilities Protocol, produced by DevicePathDxe, for formatting and
>   parsing the textual device path representation. This allows for a
>   lighter weight lib instance that gets linked into several DXE modules.
>   In comparison, the more standalone UefiDevicePathLib instance includes
>   the formatting and parsing routines in every client module.
>
> - The DXE core needs DevicePathLib before it dispatches DevicePathDxe, so
>   it needs to stick with the standalone instance.
>
> - DevicePathDxe itself also needs the standalone instance, for
>   implementing the protocol.
>
> - The DXE-phase PCD driver, "MdeModulePkg/Universal/PCD/Dxe/Pcd.inf",
>   depends on DevicePathLib via UefiLib and DxeServicesLib at the least; so
>   with this update, it inherits a dependency on the protocol. In reverse,
>   DevicePathDxe depends on the PCD Protocol, via PcdLib. The cycle is
>   broken by using BasePcdLibNull in DevicePathDxe. That restricts it to
>   FixedAtBuild, Patch, and FeatureFlag PCDs, but that's fine.
>
> Example space savings (using ArmVirtQemu and the GCC5 toolchain):
> - NOOPT:   187KB in FVMAIN, 12KB in FVMAIN_COMPACT
> - DEBUG:   147KB in FVMAIN, 20KB in FVMAIN_COMPACT
> - RELEASE: 123KB in FVMAIN, 17KB in FVMAIN_COMPACT
>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Julien Grall <julien.grall@linaro.org>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=940
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>

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

> ---
>
> Notes:
>     Repo:   https://github.com/lersek/edk2.git
>     Branch: armvirt_devpathlib
>
>  ArmVirtPkg/ArmVirt.dsc.inc       | 2 +-
>  ArmVirtPkg/ArmVirtQemu.dsc       | 7 ++++++-
>  ArmVirtPkg/ArmVirtQemuKernel.dsc | 7 ++++++-
>  ArmVirtPkg/ArmVirtXen.dsc        | 7 ++++++-
>  4 files changed, 19 insertions(+), 4 deletions(-)
>
> diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc
> index cde514958da2..35bccb3dc1f4 100644
> --- a/ArmVirtPkg/ArmVirt.dsc.inc
> +++ b/ArmVirtPkg/ArmVirt.dsc.inc
> @@ -48,7 +48,7 @@ [LibraryClasses.common]
>    UefiLib|MdePkg/Library/UefiLib/UefiLib.inf
>    HobLib|ArmVirtPkg/Library/ArmVirtDxeHobLib/ArmVirtDxeHobLib.inf
>    UefiRuntimeServicesTableLib|MdePkg/Library/UefiRuntimeServicesTableLib/UefiRuntimeServicesTableLib.inf
> -  DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
> +  DevicePathLib|MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLibDevicePathProtocol.inf
>    UefiBootServicesTableLib|MdePkg/Library/UefiBootServicesTableLib/UefiBootServicesTableLib.inf
>    DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableLib.inf
>    DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
> diff --git a/ArmVirtPkg/ArmVirtQemu.dsc b/ArmVirtPkg/ArmVirtQemu.dsc
> index 824070edc2a9..d74feb709cd1 100644
> --- a/ArmVirtPkg/ArmVirtQemu.dsc
> +++ b/ArmVirtPkg/ArmVirtQemu.dsc
> @@ -243,6 +243,7 @@ [Components.common]
>    MdeModulePkg/Core/Dxe/DxeMain.inf {
>      <LibraryClasses>
>        NULL|MdeModulePkg/Library/DxeCrc32GuidedSectionExtractLib/DxeCrc32GuidedSectionExtractLib.inf
> +      DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
>    }
>    MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
>      <LibraryClasses>
> @@ -319,7 +320,11 @@ [Components.common]
>    #
>    # Bds
>    #
> -  MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
> +  MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf {
> +    <LibraryClasses>
> +      DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
> +      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> +  }
>    MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
>    MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
>    MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
> diff --git a/ArmVirtPkg/ArmVirtQemuKernel.dsc b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> index 2368ba283bff..1e823aeab7c0 100644
> --- a/ArmVirtPkg/ArmVirtQemuKernel.dsc
> +++ b/ArmVirtPkg/ArmVirtQemuKernel.dsc
> @@ -232,6 +232,7 @@ [Components.common]
>    MdeModulePkg/Core/Dxe/DxeMain.inf {
>      <LibraryClasses>
>        NULL|MdeModulePkg/Library/DxeCrc32GuidedSectionExtractLib/DxeCrc32GuidedSectionExtractLib.inf
> +      DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
>    }
>    MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
>      <LibraryClasses>
> @@ -308,7 +309,11 @@ [Components.common]
>    #
>    # Bds
>    #
> -  MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
> +  MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf {
> +    <LibraryClasses>
> +      DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
> +      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> +  }
>    MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
>    MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
>    MdeModulePkg/Universal/DriverHealthManagerDxe/DriverHealthManagerDxe.inf
> diff --git a/ArmVirtPkg/ArmVirtXen.dsc b/ArmVirtPkg/ArmVirtXen.dsc
> index 175b56d10c8f..e0fb4b47cf63 100644
> --- a/ArmVirtPkg/ArmVirtXen.dsc
> +++ b/ArmVirtPkg/ArmVirtXen.dsc
> @@ -153,6 +153,7 @@ [Components.common]
>    MdeModulePkg/Core/Dxe/DxeMain.inf {
>      <LibraryClasses>
>        NULL|MdeModulePkg/Library/DxeCrc32GuidedSectionExtractLib/DxeCrc32GuidedSectionExtractLib.inf
> +      DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
>    }
>    MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
>      <LibraryClasses>
> @@ -205,7 +206,11 @@ [Components.common]
>    #
>    # Bds
>    #
> -  MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf
> +  MdeModulePkg/Universal/DevicePathDxe/DevicePathDxe.inf {
> +    <LibraryClasses>
> +      DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLib.inf
> +      PcdLib|MdePkg/Library/BasePcdLibNull/BasePcdLibNull.inf
> +  }
>    MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
>    MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
>    MdeModulePkg/Universal/BdsDxe/BdsDxe.inf
> --
> 2.14.1.3.gb7cf6e02401b
>