Ard:
I think this change is OK. Ack-by: Liming Gao <liming....@intel.com>
Thanks
Liming
> -----Original Message-----
> From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Ard Biesheuvel
> Sent: Friday, February 28, 2020 1:11 AM
> To: edk2-devel-groups-io <devel@edk2.groups.io>; Wu, Hao A
> <hao.a...@intel.com>; Wang, Jian J <jian.j.w...@intel.com>; Gao,
> Zhichao <zhichao....@intel.com>; Ni, Ray <ray...@intel.com>
> Cc: Laszlo Ersek <ler...@redhat.com>; Leif Lindholm <l...@nuviainc.com>;
> Kinney, Michael D <michael.d.kin...@intel.com>
> Subject: Re: [edk2-devel] [PATCH v3 5/6] MdeModulePkg/DxeCore: defer PE/COFF
> emulator registration to StartImage
>
> On Wed, 26 Feb 2020 at 20:43, Ard Biesheuvel <ard.biesheu...@linaro.org>
> wrote:
> >
> > EDK2's implementation of the LoadImage() boot service permits non-native
> > binaries to be loaded (i.e., X64 images on IA32 firmware), but any
> > attempts to start such an image using StartImage() will return
> > EFI_UNSUPPORTED.
> >
> > The integration of the PE/COFF emulator protocol into the DXE core
> > deviates slightly from this paradigm, given that its IsImageSupported
> > hook as well as its RegisterImage hook are invoked from LoadImage,
> > and by the time StartImage is called, no opportunity is given to the
> > provider of the PE/COFF emulator protocol to prevent an image from
> > being started if it only supports loading it.
> >
> > To address this disparity, let's move the invocation of RegisterImage()
> > to the implementation of the StartImage() boot service, allowing the
> > emulator to permit LoadImage() but reject StartImage() on images that
> > turn out not to meet the requirements of the emulator as it is being
> > started.
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org>
>
> @MdeModulePkg maintainers: do you have any comments on this patch?
>
>
> > ---
> > MdeModulePkg/Core/Dxe/Image/Image.c | 24 +++++++++++---------
> > 1 file changed, 13 insertions(+), 11 deletions(-)
> >
> > diff --git a/MdeModulePkg/Core/Dxe/Image/Image.c
> > b/MdeModulePkg/Core/Dxe/Image/Image.c
> > index 22a87ecf6d7c..d86da89ee704 100644
> > --- a/MdeModulePkg/Core/Dxe/Image/Image.c
> > +++ b/MdeModulePkg/Core/Dxe/Image/Image.c
> > @@ -756,17 +756,6 @@ CoreLoadPeImage (
> > // Get the image entry point.
> > //
> > Image->EntryPoint =
> > (EFI_IMAGE_ENTRY_POINT)(UINTN)Image->ImageContext.EntryPoint;
> > - if (Image->PeCoffEmu != NULL) {
> > - Status = Image->PeCoffEmu->RegisterImage (Image->PeCoffEmu,
> > - Image->ImageBasePage,
> > - EFI_PAGES_TO_SIZE (Image->NumberOfPages),
> > - &Image->EntryPoint);
> > - if (EFI_ERROR (Status)) {
> > - DEBUG ((DEBUG_LOAD | DEBUG_ERROR,
> > - "CoreLoadPeImage: Failed to register foreign image with
> > emulator.\n"));
> > - goto Done;
> > - }
> > - }
> >
> > //
> > // Fill in the image information for the Loaded Image Protocol
> > @@ -1603,6 +1592,19 @@ CoreStartImage (
> > return EFI_UNSUPPORTED;
> > }
> >
> > + if (Image->PeCoffEmu != NULL) {
> > + Status = Image->PeCoffEmu->RegisterImage (Image->PeCoffEmu,
> > + Image->ImageBasePage,
> > + EFI_PAGES_TO_SIZE (Image->NumberOfPages),
> > + &Image->EntryPoint);
> > + if (EFI_ERROR (Status)) {
> > + DEBUG ((DEBUG_LOAD | DEBUG_ERROR,
> > + "CoreLoadPeImage: Failed to register foreign image with emulator -
> > %r\n",
> > + Status));
> > + return Status;
> > + }
> > + }
> > +
> > PERF_START_IMAGE_BEGIN (Handle);
> >
> >
> > --
> > 2.17.1
> >
>
>
-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.
View/Reply Online (#55089): https://edk2.groups.io/g/devel/message/55089
Mute This Topic: https://groups.io/mt/71572296/21656
Group Owner: devel+ow...@edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub [arch...@mail-archive.com]
-=-=-=-=-=-=-=-=-=-=-=-
