Re: [edk2-devel] [PATCH 2/2] OvmfPkg: Add EsrtFmpDxe in OVMF X64 Builds.

["Laszlo Ersek" <lersek@redhat.com>]

On 11/09/20 12:03, Sandeep Dhanvada wrote:
> This will allow testing of FMP Capsule update on a PCI device with OVMF.
>
> DxeRuntimeCapsuleLib from DxeCapsuleLibFmp enables capsule update
> support in OVMF.
>
> Inclusion of EsrtFmpDxe in OVMF X64 builds will enable dynamic creation
> of ESRT using FMP produced by UEFI device driver.
>
> Testing these changes with CapsuleApp.efi and with FMP support added in
> UEFI device driver shows that, dump ESRT using -E option displays ESRT
> table and using this efi with a capsule file as argument, is initiating
> the firmware update process using UpdateCapsule API.
>
> Signed-off-by: Sandeep Dhanvada <sandeep.dhanvada@xilinx.com>
> ---
>  OvmfPkg/OvmfPkgX64.dsc | 4 +++-
>  OvmfPkg/OvmfPkgX64.fdf | 1 +
>  2 files changed, 4 insertions(+), 1 deletion(-)


Please see the previous discussion here:

* [edk2-devel] [PATCH v2]
  OvmfPkg: Use DxeRuntimeCapsuleLib from DxeCapsuleLibFmp in X64 builds

  https://edk2.groups.io/g/devel/message/42752
  http://mid.mail-archive.com/6ef09714-fd1f-2f7b-5a1d-fdf5e1a609fb@solarflare.com

(1) Please address the requests that I made in that thread. In
particular:

(1a) Unless this is specific to X64, the same modifications should be
replicated to the IA32 and IA32X64 DSC/FDF files.

(1b) Please make these changes dependent on a new build flag (default
value: FALSE), in the DSC file(s). I suggest:

> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 2637dfaf2de3..232e9ce2726a 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -33,6 +33,7 @@ [Defines]
>    DEFINE SOURCE_DEBUG_ENABLE     = FALSE
>    DEFINE TPM_ENABLE              = FALSE
>    DEFINE TPM_CONFIG_ENABLE       = FALSE
> +  DEFINE PCI_DEV_CAPSULE_ENABLE  = FALSE
>
>    #
>    # Network definition

(1c) Please spell out the role of "CapsuleRuntimeDxe" in the commit
message.

Some new comments:

>
> diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc
> index 7a8bdb8..07cc167 100644
> --- a/OvmfPkg/OvmfPkgX64.dsc
> +++ b/OvmfPkg/OvmfPkgX64.dsc
> @@ -138,7 +138,7 @@

(2) Please run "BaseTools/Scripts/SetupGit.py" in your edk2 clone, or
else manually implement the settings at
<https://github.com/tianocore/tianocore.github.io/wiki/Laszlo%27s-unkempt-git-guide-for-edk2-contributors-and-maintainers#contrib-05>.

I'm asking specifically because the "@@" hunk header above should show
which section of the DSC file is being modified.

>    UefiBootManagerLib|MdeModulePkg/Library/UefiBootManagerLib/UefiBootManagerLib.inf
>    BootLogoLib|MdeModulePkg/Library/BootLogoLib/BootLogoLib.inf
>    FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf
> -  CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibNull/DxeCapsuleLibNull.inf
> +  CapsuleLib|MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf
>    DxeServicesLib|MdePkg/Library/DxeServicesLib/DxeServicesLib.inf
>    DxeServicesTableLib|MdePkg/Library/DxeServicesTableLib/DxeServicesTableLib.inf
>    PeCoffGetEntryPointLib|MdePkg/Library/BasePeCoffGetEntryPointLib/BasePeCoffGetEntryPointLib.inf
> @@ -796,6 +796,8 @@
>    MdeModulePkg/Universal/WatchdogTimerDxe/WatchdogTimer.inf
>    MdeModulePkg/Universal/MonotonicCounterRuntimeDxe/MonotonicCounterRuntimeDxe.inf
>    MdeModulePkg/Universal/CapsuleRuntimeDxe/CapsuleRuntimeDxe.inf
> +  MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmpDxe.inf
> +  MdeModulePkg/Library/DxeCapsuleLibFmp/DxeRuntimeCapsuleLib.inf

(3) Listing a library instance in the [Components] section of a DSC file
makes no sense in the OVMF DSC files. (It may make sense for in other
packages' DSC files, but not for OVMF.)

>    MdeModulePkg/Universal/Console/ConPlatformDxe/ConPlatformDxe.inf
>    MdeModulePkg/Universal/Console/ConSplitterDxe/ConSplitterDxe.inf
>    MdeModulePkg/Universal/Console/GraphicsConsoleDxe/GraphicsConsoleDxe.inf {
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index 17ba9e1..97405e8 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -281,6 +281,7 @@ INF  MdeModulePkg/Universal/HiiDatabaseDxe/HiiDatabaseDxe.inf
>  INF  MdeModulePkg/Universal/SetupBrowserDxe/SetupBrowserDxe.inf
>  INF  MdeModulePkg/Universal/DisplayEngineDxe/DisplayEngineDxe.inf
>  INF  MdeModulePkg/Universal/MemoryTest/NullMemoryTestDxe/NullMemoryTestDxe.inf
> +NF  MdeModulePkg/Universal/EsrtFmpDxe/EsrtFmpDxe.inf

(4) This change seems corrupt.

(5) Please try to list modules in the DSC and FDF files in the same
order. If you insert EsrtFmpDxe just after CapsuleRuntimeDxe in the DSC
file, then please attempt to follow suit in the FDF file.

Thanks
Laszlo

>
>  INF  OvmfPkg/SioBusDxe/SioBusDxe.inf
>  !if $(SOURCE_DEBUG_ENABLE) == FALSE
> --
> 2.1.1