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

[Laszlo Ersek <lersek@redhat.com>]

On 07/04/17 13:54, Gao, Liming wrote:
> Reviewed-by: Liming Gao <liming.gao@intel.com>

Thanks for your help, Liming!
Laszlo

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