Re: [PATCH v2 1/2] OvmfPkg: disable build-time relocation for DXEFV modules

["Gao, Liming" <liming.gao@intel.com>]

Reviewed-by: Liming Gao <liming.gao@intel.com>

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Tuesday, July 4, 2017 6:00 PM
> To: edk2-devel-01 <edk2-devel@lists.01.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Justen, Jordan L <jordan.l.justen@intel.com>; Gao, Liming <liming.gao@intel.com>
> Subject: [PATCH v2 1/2] OvmfPkg: disable build-time relocation for DXEFV modules
> 
> When the GenFv utility from BaseTools composes a firmware volume, it
> checks whether modules in the firmware volume are subject to build-time
> relocation. The primary indication for relocation is whether the firmware
> volume has a nonzero base address, according to the [FD] section(s) in the
> FDF file that refer to the firmware volume.
> 
> The idea behind build-time relocation is that XIP (execute in place)
> modules will not be relocated at boot-time:
> 
> - Pre-DXE phase modules generally execute in place.
> 
>   (OVMF is no exception, despite the fact that we have writeable memory
>   even in SEC: PEI_CORE and PEIMs run in-place from PEIFV, after SEC
>   decompresses PEIFV and DXEFV from FVMAIN_COMPACT (flash) to RAM.
>   PEI_CORE and the PEIMs are relocated at boot-time only after PlatformPei
>   installs the permanent PEI RAM, and the RAM migration occurs.)
> 
> - Modules dispatched by the DXE Core are generally relocated at boot-time.
>   However, this is not necessarily so. Quoting Liming from
>   <https://lists.01.org/pipermail/edk2-devel/2017-July/012053.html>:
> 
> > PI spec has no limitation that XIP is for PEIM only. DXE driver may be
> > built as XIP for other purpose. For example, if DXE driver image address
> > is not zero, DxeCore will try allocating the preferred address and load
> > it. In another case, once DXE driver is relocated at build time, DxeCore
> > will dispatch it and start it directly without loading, it may save boot
> > performance.
> 
> Therefore GenFv relocates even DXE and UEFI driver modules if the
> containing firmware volume has a nonzero base address.
> 
> In OVMF, this is the case for both PEIV and DXEFV:
> 
> > [FD.MEMFD]
> > BaseAddress   = $(MEMFD_BASE_ADDRESS)
> > Size          = 0xB00000
> > ErasePolarity = 1
> > BlockSize     = 0x10000
> > NumBlocks     = 0xB0
> > ...
> > 0x020000|0x0E0000
> > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfPeiMemFvSize
> > FV = PEIFV
> >
> > 0x100000|0xA00000
> > gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvBase|gUefiOvmfPkgTokenSpaceGuid.PcdOvmfDxeMemFvSize
> > FV = DXEFV
> 
> While the build-time relocation certainly makes sense for PEIFV (see
> above), the reasons for which we specify DXEFV under [FD.MEMFD] are
> weaker:
> 
> - we set the PcdOvmfDxeMemFvBase and PcdOvmfDxeMemFvSize PCDs here,
> 
> - and we ascertain that DXEFV, when decompressed by SEC from
>   FVMAIN_COMPACT, will fit into the area allotted here, at build time.
> 
> In other words, the build-time relocation of the modules in DXEFV is a
> waste of resources. But, it gets worse:
> 
> Build-time relocation of an executable is only possible if the on-disk and
> in-memory layouts are identical, i.e., if the sections of the PE/COFF
> image adhere to the same alignment on disk and in memory. Put differently,
> the FileAlignment and SectionAlignment headers must be equal.
> 
> For boot-time modules that we build as part of edk2, both alignment values
> are 0x20 bytes. For runtime modules that we build as part of edk2, both
> alignment values are 0x1000 bytes. This is why the DXEFV relocation,
> albeit wasteful, is also successful every time.
> 
> Unfortunately, if we try to include a PE/COFF binary in DXEFV that
> originates from outside of edk2, the DXEFV relocation can fail due to the
> binary having unmatched FileAlignment and SectionAlignment headers. This
> is precisely the case with the E3522X2.EFI network driver for the e1000
> NIC, from Intel's BootUtil / PREBOOT.EXE distribution.
> 
> The solution is to use the FvForceRebase=FALSE override under [FV.DXEFV].
> This tells GenFv not to perform build-time relocation on the firmware
> volume, despite the FV having a nonzero base address.
> 
> In DXEFV we also have SMM drivers. Those are relocated at boot-time (into
> SMRAM) unconditionally; SMRAM is always discovered at boot-time.
> 
> Kudos to Ard and Liming for the PE/COFF sections & relocations
> explanation, and for the FvForceRebase=FALSE tip.
> 
> I regression-tested this change in the following configurations (all with
> normal boot and S3 suspend/resume):
> 
>   IA32,     q35,     SMM,     Linux
>   IA32X64,  q35,     SMM,     Linux
>   IA32X64,  q35,     SMM,     Windows-8.1
>   X64,      i440fx,  no-SMM,  Linux
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=613
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=615
> Suggested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Suggested-by: Liming Gao <liming.gao@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     v2:
>     - in the commit message, replace LMFA references with Liming's
>       explanation from the mailing list
>     - no code changes
> 
>  OvmfPkg/OvmfPkgIa32.fdf    | 1 +
>  OvmfPkg/OvmfPkgIa32X64.fdf | 1 +
>  OvmfPkg/OvmfPkgX64.fdf     | 1 +
>  3 files changed, 3 insertions(+)
> 
> diff --git a/OvmfPkg/OvmfPkgIa32.fdf b/OvmfPkg/OvmfPkgIa32.fdf
> index 09c165882c3f..859457e9aae5 100644
> --- a/OvmfPkg/OvmfPkgIa32.fdf
> +++ b/OvmfPkg/OvmfPkgIa32.fdf
> @@ -168,6 +168,7 @@ [FV.PEIFV]
>  ################################################################################
> 
>  [FV.DXEFV]
> +FvForceRebase      = FALSE
>  FvNameGuid         = 7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1
>  BlockSize          = 0x10000
>  FvAlignment        = 16
> diff --git a/OvmfPkg/OvmfPkgIa32X64.fdf b/OvmfPkg/OvmfPkgIa32X64.fdf
> index 5233314139bc..2a0ed8313786 100644
> --- a/OvmfPkg/OvmfPkgIa32X64.fdf
> +++ b/OvmfPkg/OvmfPkgIa32X64.fdf
> @@ -168,6 +168,7 @@ [FV.PEIFV]
>  ################################################################################
> 
>  [FV.DXEFV]
> +FvForceRebase      = FALSE
>  FvNameGuid         = 7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1
>  BlockSize          = 0x10000
>  FvAlignment        = 16
> diff --git a/OvmfPkg/OvmfPkgX64.fdf b/OvmfPkg/OvmfPkgX64.fdf
> index 36150101e784..ca61fa125795 100644
> --- a/OvmfPkg/OvmfPkgX64.fdf
> +++ b/OvmfPkg/OvmfPkgX64.fdf
> @@ -168,6 +168,7 @@ [FV.PEIFV]
>  ################################################################################
> 
>  [FV.DXEFV]
> +FvForceRebase      = FALSE
>  FvNameGuid         = 7CB8BDC9-F8EB-4F34-AAEA-3EE4AF6516A1
>  BlockSize          = 0x10000
>  FvAlignment        = 16
> --
> 2.13.1.3.g8be5a757fa67
>