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

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

Laszlo:
  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. 

Thanks
Liming
>-----Original Message-----
>From: Laszlo Ersek [mailto:lersek@redhat.com]
>Sent: Sunday, July 02, 2017 4:36 AM
>To: Gao, Liming <liming.gao@intel.com>; edk2-devel-01 <edk2-
>devel@lists.01.org>
>Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>; Justen, Jordan L
><jordan.l.justen@intel.com>
>Subject: Re: [PATCH 1/2] OvmfPkg: disable build-time relocation for DXEFV
>modules
>
>Liming,
>
>On 06/29/17 05:32, Gao, Liming wrote:	
>> Laszlo:
>>   LMFA feature doesn't do PE image rebase at build time. Only XIP
>>   module needs to be rebased at build time.
>
>Thank you for the clarification.
>
>In this case, BaseTools/GenFv has a bug.
>
>Namely, if LMFA does not need build-time relocation for DXE-phase
>modules, then *what exactly* needs build-time relocation for DXE-phase
>modules?
>
>This is the code in "BaseTools/Source/C/GenFv/GenFvInternalLib.c":
>
>  3537        case EFI_FV_FILETYPE_DRIVER:
>  3538        case EFI_FV_FILETYPE_DXE_CORE:
>  3539          //
>  3540          // Check if section-alignment and file-alignment match or not
>  3541          //
>  3542          if ((ImgHdr->Pe32.OptionalHeader.SectionAlignment != ImgHdr-
>>Pe32.OptionalHeader.FileAlignment)) {
>  3543            //
>  3544            // Xip module has the same section alignment and file alignment.
>  3545            //
>  3546            Error (NULL, 0, 3000, "Invalid", "Section-Alignment and File-
>Alignment do not match : %s.", FileName);
>  3547            return EFI_ABORTED;
>  3548          }
>  3549          NewPe32BaseAddress = XipBase + (UINTN)
>CurrentPe32Section.Pe32Section + CurSecHdrSize - (UINTN)FfsFile;
>  3550          break;
>
>According to the Platform Init 1.6 spec,
>
>- 2.1.4.1 Firmware File Types:
>
>  [...] the type EFI_FV_FILETYPE_DRIVER indicates that the file is a DXE
>  driver and is interesting to the DXE Dispatcher.
>
>- 2.1.4.1.4 EFI_FV_FILETYPE_DRIVER:
>
>  The file type EFI_FV_FILETYPE_DRIVER denotes a file that contains a
>  PE32 image that can be dispatched by the DXE Dispatcher.
>
>So, *when exactly* is it the case that a module dispatched by the DXE
>core *needs* build-time relocation?
>
>I claim that the answer is "never", and therefore the above code is a
>bug in BaseTools/GenFv.
>
>I thought that this build-time relocation was needed by the LMFA
>feature, but you confirmed above that LMFA does not need it. So, it
>looks like *nothing at all* needs build-time relocation for
>EFI_FV_FILETYPE_DRIVER, because the modules dispatched by the DXE core
>are *never* XIP modules.
>
>Do you agree?
>
>>   For this case, OvmfPkg DXEFV doesn't require to run as XIP. So, it
>>   doesn't require rebase. I agree this change.
>
>This change (for the OVMF FDF files) is only valid if the above
>BaseTools/GenFv code is also valid. In other words, *if* there is at
>least one valid reason for rebasing DXE modules at build-time.
>
>If there is *no* such reason, then the OVMF FDF files do not need this
>change, and GenFv must be fixed instead.
>
>So: if LMFA is *not* the one reason that justifies the rebasing for
>EFI_FV_FILETYPE_DRIVER, then *what* is the reason? In my commit message
>for the OVMF FDF change, I have to refer to that exact reason.
>
>Thanks
>Laszlo