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.
**/
--
2.19.1.3.g30247aa5d201
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#55622): https://edk2.groups.io/g/devel/message/55622
Mute This Topic: https://groups.io/mt/71785477/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 3/7/20 12:04 AM, 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>
Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daude <philmd@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.
> **/
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#55623): https://edk2.groups.io/g/devel/message/55623
Mute This Topic: https://groups.io/mt/71785477/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 03/07/20 01:01, Philippe Mathieu-Daudé wrote: > On 3/7/20 12:04 AM, 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> > > Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com> > Tested-by: Philippe Mathieu-Daude <philmd@redhat.com> Thank you, Phil, this really helps -- I want to push the patch as quickly as possible, due to it averting a build break. So I've submitted <https://github.com/tianocore/edk2/pull/428> with your tags applied... but mergify is apparently not picking up the "push" label again... I have no idea what's going on. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#55625): https://edk2.groups.io/g/devel/message/55625 Mute This Topic: https://groups.io/mt/71785477/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Sat, 7 Mar 2020 at 02:00, Laszlo Ersek <lersek@redhat.com> wrote: > > On 03/07/20 01:01, Philippe Mathieu-Daudé wrote: > > On 3/7/20 12:04 AM, 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> > > > > Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com> > > Tested-by: Philippe Mathieu-Daude <philmd@redhat.com> > > Thank you, Phil, this really helps -- I want to push the patch as > quickly as possible, due to it averting a build break. > > So I've submitted <https://github.com/tianocore/edk2/pull/428> with your > tags applied... but mergify is apparently not picking up the "push" > label again... I have no idea what's going on. > I queued it here as well: https://github.com/tianocore/edk2/pull/427 -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#55628): https://edk2.groups.io/g/devel/message/55628 Mute This Topic: https://groups.io/mt/71785477/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 03/07/20 07:10, Ard Biesheuvel wrote: > On Sat, 7 Mar 2020 at 02:00, Laszlo Ersek <lersek@redhat.com> wrote: >> >> On 03/07/20 01:01, Philippe Mathieu-Daudé wrote: >>> On 3/7/20 12:04 AM, 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> >>> >>> Reviewed-by: Philippe Mathieu-Daude <philmd@redhat.com> >>> Tested-by: Philippe Mathieu-Daude <philmd@redhat.com> >> >> Thank you, Phil, this really helps -- I want to push the patch as >> quickly as possible, due to it averting a build break. >> >> So I've submitted <https://github.com/tianocore/edk2/pull/428> with your >> tags applied... but mergify is apparently not picking up the "push" >> label again... I have no idea what's going on. >> > > I queued it here as well: > https://github.com/tianocore/edk2/pull/427 Nope, still stuck, both #427 and #428. "We are currently investigating a delay in notification delivery." "We are monitoring service recovery. We are processing a backlog of notifications." https://www.githubstatus.com/incidents/lcgmxy9pmgm4 The last update says "resolved" (Mar 06, 2020 - 00:22 UTC), but I don't really believe that... I've contacted github about this. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#55630): https://edk2.groups.io/g/devel/message/55630 Mute This Topic: https://groups.io/mt/71785477/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On 03/07/20 08:28, Laszlo Ersek wrote: > On 03/07/20 07:10, Ard Biesheuvel wrote: >> I queued it here as well: >> https://github.com/tianocore/edk2/pull/427 > > > Nope, still stuck, both #427 and #428. > > "We are currently investigating a delay in notification delivery." > > "We are monitoring service recovery. We are processing a backlog of > notifications." > > https://www.githubstatus.com/incidents/lcgmxy9pmgm4 > > The last update says "resolved" (Mar 06, 2020 - 00:22 UTC), but I don't > really believe that... > > I've contacted github about this. Ticket ID: 592107 Thanks Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#55632): https://edk2.groups.io/g/devel/message/55632 Mute This Topic: https://groups.io/mt/71785477/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
On Sat, 7 Mar 2020 at 00:04, Laszlo Ersek <lersek@redhat.com> 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>
Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
(in case you still need it)
> ---
>
> 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.
> **/
> --
> 2.19.1.3.g30247aa5d201
>
>
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#55647): https://edk2.groups.io/g/devel/message/55647
Mute This Topic: https://groups.io/mt/71785477/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
On 03/07/20 15:34, Ard Biesheuvel wrote: > On Sat, 7 Mar 2020 at 00:04, Laszlo Ersek <lersek@redhat.com> 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> > > Reviewed-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > (in case you still need it) Thanks. If the workaround for the current github / mergify outage turns out to be "resubmit pending PRs", then I'll likely pick up your R-b too. Seeing how your latest <https://github.com/tianocore/edk2/pull/430> is also stuck, I must wonder what is going on at github. They have not responded to my ticket (592107) and their status page claims "All Systems Operational": https://www.githubstatus.com/ Which is not the case. Mergify have their own status page: https://www.notion.so/Mergify-Status-Page-7803a762235d4ee6bed1f9976d17bd83 and that also claims "Dashboard Operational" and "Engine Operational". I've now sent an email to <support@mergify.io> too. Laszlo -=-=-=-=-=-=-=-=-=-=-=- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#55663): https://edk2.groups.io/g/devel/message/55663 Mute This Topic: https://groups.io/mt/71785477/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=-=-=-=-=-=-=-=-=-=-=-
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
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#55700): https://edk2.groups.io/g/devel/message/55700
Mute This Topic: https://groups.io/mt/71785477/1787277
Group Owner: devel+owner@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org]
-=-=-=-=-=-=-=-=-=-=-=-
© 2016 - 2024 Red Hat, Inc.