Re: [edk2-devel] [Patch 1/3] EmulatorPkg: Fix VS20xx IA32 boot failure

["Michael D Kinney" <michael.d.kinney@intel.com>]

Hao Wu,

I could not reproduce the exception case today.

There must have been something incorrect about my
build environment.  I have verified an error message
with normal exit always occurs.

Mike

> -----Original Message-----
> From: Kinney, Michael D
> Sent: Wednesday, August 7, 2019 8:52 AM
> To: Wu, Hao A <hao.a.wu@intel.com>;
> devel@edk2.groups.io; Kinney, Michael D
> <michael.d.kinney@intel.com>
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>;
> Andrew Fish <afish@apple.com>; Ni, Ray
> <ray.ni@intel.com>
> Subject: RE: [edk2-devel] [Patch 1/3] EmulatorPkg: Fix
> VS20xx IA32 boot failure
> 
> Hao Wu,
> 
> I agree that the exception is not an expected result at
> all.  I will debug a bit more to see why the mapping
> was allowed to pass.  The expected result should be an
> error message with normal app exit if the address can
> not be mapped.
> 
> Mike
> 
> > -----Original Message-----
> > From: Wu, Hao A
> > Sent: Wednesday, August 7, 2019 1:15 AM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>;
> > devel@edk2.groups.io
> > Cc: Justen, Jordan L <jordan.l.justen@intel.com>;
> Andrew Fish
> > <afish@apple.com>; Ni, Ray <ray.ni@intel.com>
> > Subject: RE: [edk2-devel] [Patch 1/3] EmulatorPkg:
> Fix VS20xx IA32
> > boot failure
> >
> > > -----Original Message-----
> > > From: Kinney, Michael D
> > > Sent: Wednesday, August 07, 2019 3:43 PM
> > > To: Wu, Hao A; devel@edk2.groups.io; Kinney,
> Michael
> > D
> > > Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> > > Subject: RE: [edk2-devel] [Patch 1/3] EmulatorPkg:
> > Fix VS20xx IA32
> > > boot failure
> > >
> > > I observed 2 failure modes
> > >
> > > 1) The range could not be mapped at the request
> > address
> > >    And an error message was displayed and the app
> > exited.
> > > 2) The mapping passed, but when it was accessed by
> > the
> > >    Sec module, the app generated an exception.
> >
> >
> > Thanks for the additional information.
> >
> > For case 2), even though it might not be directly
> related with this
> > issue.
> > I think it would be the best to ensure
> WinNtOpenFile() returns with
> > success only when the map operation takes effect.
> >
> > For this patch:
> > Reviewed-by: Hao A Wu <hao.a.wu@intel.com>
> >
> > Best Regards,
> > Hao Wu
> >
> >
> > >
> > > Mike
> > >
> > > > -----Original Message-----
> > > > From: Wu, Hao A
> > > > Sent: Tuesday, August 6, 2019 11:19 PM
> > > > To: devel@edk2.groups.io; Kinney, Michael D
> > > > <michael.d.kinney@intel.com>
> > > > Cc: Justen, Jordan L <jordan.l.justen@intel.com>;
> > Andrew Fish
> > > > <afish@apple.com>; Ni, Ray <ray.ni@intel.com>
> > > > Subject: RE: [edk2-devel] [Patch 1/3]
> EmulatorPkg:
> > Fix VS20xx IA32
> > > > boot failure
> > > >
> > > > > -----Original Message-----
> > > > > From: devel@edk2.groups.io
> > > > [mailto:devel@edk2.groups.io] On Behalf Of
> > > > > Michael D Kinney
> > > > > Sent: Wednesday, August 07, 2019 12:20 PM
> > > > > To: devel@edk2.groups.io
> > > > > Cc: Justen, Jordan L; Andrew Fish; Ni, Ray
> > > > > Subject: [edk2-devel] [Patch 1/3] EmulatorPkg:
> > Fix
> > > > VS20xx IA32 boot
> > > > > failure
> > > > >
> > > > >
> > https://bugzilla.tianocore.org/show_bug.cgi?id=2056
> > > > >
> > > > > The IA32 build of the EmulatorPkg for VS20xx
> does
> > not
> > > > boot because the
> > > > > default value of PCD PcdPeiServicesTablePage is
> > set
> > > > for X64 Windows
> > > > > Host environments.  If the EmulatorPkg is build
> > for
> > > > an IA32 Windows
> > > > > Host environment, then set this PCD to
> 0x13000000
> > > > that can be mapped
> > > > > into a 32-bit user process.
> > > >
> > > >
> > > > Hello Mike,
> > > >
> > > > I have a question for this patch.
> > > >
> > > > For the main() function within file
> > > > EmulatorPkg/Win/Host/WinHost.c, I saw the below
> > codes:
> > > >
> > > >   EmuMagicPage = (VOID *)(UINTN)(FixedPcdGet64
> > > > (PcdPeiServicesTablePage) & MAX_UINTN);
> > > >   if (EmuMagicPage != NULL) {
> > > >     UINT64  Size;
> > > >     Status = WinNtOpenFile (
> > > >               NULL,
> > > >               SIZE_4KB,
> > > >               0,
> > > >               &EmuMagicPage,
> > > >               &Size
> > > >               );
> > > >     if (EFI_ERROR (Status)) {
> > > >       SecPrint ("ERROR : Could not allocate
> > PeiServicesTablePage @
> > > > %p\n", EmuMagicPage);
> > > >       return EFI_DEVICE_ERROR;
> > > >     }
> > > >   }
> > > >
> > > > My understanding is that the WinNtOpenFile()
> > function call is
> > > > attempting to map the address specified by
> > 'EmuMagicPage'. For IA32
> > > > case, the value in 'EmuMagicPage' here has
> already
> > been truncated
> > > > (0x03000000). If the
> > > > WinNtOpenFile() call returned successfully, the
> > address should be
> > > > mapped.
> > > >
> > > > Is it the case that even if WinNtOpenFile()
> returns
> > with no error,
> > > > the specified address (0x03000000) is not
> actually
> > being mapped?
> > > >
> > > > Best Regards,
> > > > Hao Wu
> > > >
> > > >
> > > > >
> > > > > Cc: Jordan Justen <jordan.l.justen@intel.com>
> > > > > Cc: Andrew Fish <afish@apple.com>
> > > > > Cc: Ray Ni <ray.ni@intel.com>
> > > > > Signed-off-by: Michael D Kinney
> > > > <michael.d.kinney@intel.com>
> > > > > ---
> > > > >  EmulatorPkg/EmulatorPkg.dsc | 6 +++++-
> > > > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/EmulatorPkg/EmulatorPkg.dsc
> > > > b/EmulatorPkg/EmulatorPkg.dsc
> > > > > index ea8b6ce76e..c4ec10d1d8 100644
> > > > > --- a/EmulatorPkg/EmulatorPkg.dsc
> > > > > +++ b/EmulatorPkg/EmulatorPkg.dsc
> > > > > @@ -4,7 +4,7 @@
> > > > >  # The Emulation Platform can be used to debug
> > > > individual modules,
> > > > > prior to creating  # a real platform. This also
> > > > provides an example
> > > > > for how an DSC is created.
> > > > >  #
> > > > > -# Copyright (c) 2006 - 2018, Intel
> Corporation.
> > All
> > > > rights
> > > > > reserved.<BR>
> > > > > +# Copyright (c) 2006 - 2019, Intel
> Corporation.
> > All
> > > > rights
> > > > > +reserved.<BR>
> > > > >  # Portions copyright (c) 2010 - 2011, Apple
> Inc.
> > All
> > > > rights
> > > > > reserved.<BR>  #  # SPDX-License-Identifier:
> BSD-
> > 2-
> > > > Clause-Patent @@
> > > > > -225,6 +225,10 @@ [PcdsFixedAtBuild]
> > > > >    #  0-PCANSI, 1-VT100, 2-VT00+, 3-UTF8, 4-
> > TTYTERM
> > > > >
> > gEfiMdePkgTokenSpaceGuid.PcdDefaultTerminalType|1
> > > > >
> > > > > +!if "IA32" in $(ARCH) && "MSFT" in $(FAMILY)
> > > > > +
> > > >
> >
> gEmulatorPkgTokenSpaceGuid.PcdPeiServicesTablePage|0x13
> > > > 000000
> > > > > +!endif
> > > > > +
> > > > >  [PcdsDynamicDefault.common.DEFAULT]
> > > > >
> > > >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwSpar
> > > > eBase64|0
> > > > >
> > > > >
> > > >
> >
> gEfiMdeModulePkgTokenSpaceGuid.PcdFlashNvStorageFtwWork
> > > > ingBase64|
> > > > > 0
> > > > > --
> > > > > 2.21.0.windows.1
> > > > >
> > > > >
> > > > >