Re: [edk2-devel] [PATCH] OvmfPkg/X86QemuLoadImageLib: fix "unused variable" error in X64 DXE builds

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

On 03/07/20 00:04, Laszlo Ersek wrote:
> When the MDE_CPU_IA32 macro is not defined, there is no access to the
> "KernelImageHandle" local variable in QemuStartKernelImage(). This breaks
> the OvmfPkgIa32X64 and OvmfPkgX64 platform builds, at least with gcc-8.
> 
> Move the local variable to the inner scope, where declaration and usage
> are inseparable.
> 
> (Note that such inner-scope declarations are frowned upon in the wider
> edk2 codebase, but we use them liberally in ArmVirtPkg and OvmfPkg anyway,
> because they help us reason about variable lifetime and visibility.)
> 
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Jordan Justen <jordan.l.justen@intel.com>
> Cc: Philippe Mathieu-Daudé <philmd@redhat.com>
> Fixes: 7c47d89003a6f8f7f6f0ce8ca7d3e87c630d14cc
> Ref: https://bugzilla.tianocore.org/show_bug.cgi?id=2572
> Signed-off-by: Laszlo Ersek <lersek@redhat.com>
> ---
> 
> Notes:
>     Ard, if you get to it first, feel free to push this in my stead. Thanks!
>     
>     Repo:   https://pagure.io/lersek/edk2.git
>     Branch: x86qlil_build_fix
> 
>  OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
> index c5bd6862b265..1868c9fcafdf 100644
> --- a/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
> +++ b/OvmfPkg/Library/X86QemuLoadImageLib/X86QemuLoadImageLib.c
> @@ -457,67 +457,68 @@ EFIAPI
>  QemuStartKernelImage (
>    IN  OUT EFI_HANDLE            *ImageHandle
>    )
>  {
>    EFI_STATUS                    Status;
>    OVMF_LOADED_X86_LINUX_KERNEL  *LoadedImage;
> -  EFI_HANDLE                    KernelImageHandle;
>  
>    Status = gBS->OpenProtocol (
>                    *ImageHandle,
>                    &gOvmfLoadedX86LinuxKernelProtocolGuid,
>                    (VOID **)&LoadedImage,
>                    gImageHandle,                  // AgentHandle
>                    NULL,                          // ControllerHandle
>                    EFI_OPEN_PROTOCOL_GET_PROTOCOL
>                    );
>    if (!EFI_ERROR (Status)) {
>      return QemuStartLegacyImage (*ImageHandle);
>    }
>  
>    Status = gBS->StartImage (
>                    *ImageHandle,
>                    NULL,              // ExitDataSize
>                    NULL               // ExitData
>                    );
>  #ifdef MDE_CPU_IA32
>    if (Status == EFI_UNSUPPORTED) {
> +    EFI_HANDLE KernelImageHandle;
> +
>      //
>      // On IA32, EFI_UNSUPPORTED means that the image's machine type is X64 while
>      // we are expecting a IA32 one, and the StartImage () boot service is unable
>      // to handle it, either because the image does not have the special .compat
>      // PE/COFF section that Linux specifies for mixed mode capable images, or
>      // because we are running without the support code for that. So load the
>      // image again, using the legacy loader, and unload the normally loaded
>      // image before starting the legacy one.
>      //
>      Status = QemuLoadLegacyImage (&KernelImageHandle);
>      if (EFI_ERROR (Status)) {
>        //
>        // Note: no change to (*ImageHandle), the caller will release it.
>        //
>        return Status;
>      }
>      //
>      // Swap in the legacy-loaded image.
>      //
>      QemuUnloadKernelImage (*ImageHandle);
>      *ImageHandle = KernelImageHandle;
>      return QemuStartLegacyImage (KernelImageHandle);
>    }
>  #endif
>    return Status;
>  }
>  
>  /**
>    Unloads an image loaded with QemuLoadKernelImage ().
>  
>    @param  ImageHandle             Handle that identifies the image to be
>                                    unloaded.
>  
>    @retval EFI_SUCCESS             The image has been unloaded.
>    @retval EFI_UNSUPPORTED         The image has been started, and does not
>                                    support unload.
>    @retval EFI_INVALID_PARAMETER   ImageHandle is not a valid image handle.
>  
>    @return                         Exit code from the image's unload function.
>  **/
> 

Commit a3e25cc8a1dd.

Thanks
Laszlo