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

[Laszlo Ersek <lersek@redhat.com>]

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