> -----Original Message----- > From: Justen, Jordan L <jordan.l.jus...@intel.com> > Sent: Thursday, February 21, 2019 1:44 AM > To: Gao, Liming <liming....@intel.com>; Ni, Ray <ray...@intel.com>; edk2- > de...@lists.01.org > Cc: Wu, Hao A <hao.a...@intel.com>; Anthony Perard > <anthony.per...@citrix.com>; Laszlo Ersek <ler...@redhat.com>; Zeng, > Star <star.z...@intel.com> > Subject: Re: [PATCH 00/10] Fix PEI Core issue during TemporaryRamMigration > > On 2019-02-20 05:27:21, Ni, Ray wrote: > > On 2/19/2019 9:25 PM, Gao, Liming wrote: > > > Ray: > > > > > > Now, real platform has no side effect. So, only TempRamDone > > > PPI is produced. For emulator platform, is there any side > > > effect when both Temporary RAM and Permanent RAM are enabled? > > > > > > > No side effect when both of T-RAM and P-RAM are enabled. > > Which means no side effect when neither of the PPIs is produced. > > But for demo purpose, I think producing TemporaryRamDone PPI makes > sense. > > In addition to being a demo/sample, it also provides a check that no modules > are accessing temp-ram after temp-ram should no longer be used. > > > I will work out patches for EmulatorPkg to produce TemoraryRamDone. > > I think we should first fix TemporaryRamSupport usage. Otherwise, we are > just hiding the bug. > > Have you tried these patches to verify that they fix the issue in your setup? > Have you taken a look at the patches to see what problem is being fixed?
I feel the change to PeiCore is a bit complex and introduce additional deps (assembly). Behavior of ARM and x86 becomes different. (The patch only fixes x86 issue.) Given there is no real requirement on this, I prefer to not change PeiCore. > > -Jordan > > > > > >> -----Original Message----- > > >> From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf > > >> Of Ni, Ray > > >> Sent: Tuesday, February 19, 2019 10:46 AM > > >> To: Justen, Jordan L <jordan.l.jus...@intel.com>; > > >> edk2-devel@lists.01.org > > >> Cc: Wu, Hao A <hao.a...@intel.com>; Anthony Perard > > >> <anthony.per...@citrix.com>; Laszlo Ersek <ler...@redhat.com>; > > >> Zeng, Star <star.z...@intel.com> > > >> Subject: Re: [edk2] [PATCH 00/10] Fix PEI Core issue during > > >> TemporaryRamMigration > > >> > > >> Jordan, > > >> I find many real platforms do not implement the temporary ram > > >> migration PPI and rely on the PeiCore migration logic. > > >> So perhaps TemporaryRamMigration PPI was added to help platform to > > >> destroy the temporary RAM (CAR in x86 platform). > > >> But with the introduction of TemporaryRamDone PPI, maybe the > > >> TemporaryRamMigration PPI can be retired. > > >> The logic in PeiCore to call TemporaryRamMigration is just for > > >> backward compatibility. > > >> If that's true, do you still need to enhance PeiCore? > > >> > > >> For the Emulator case, I already found without > > >> TemporaryRamMigration the platform can still boot. > > >> > > >> Does OVMF hard-depend on TemporaryRamMigration? Or it can reply > on > > >> the PeiCore migration logic + TemporaryDone PPI? > > >> > > >> Thanks, > > >> Ray > > >> > > >>> -----Original Message----- > > >>> From: Justen, Jordan L <jordan.l.jus...@intel.com> > > >>> Sent: Monday, February 18, 2019 12:12 PM > > >>> To: edk2-devel@lists.01.org > > >>> Cc: Justen, Jordan L <jordan.l.jus...@intel.com>; Liu Yu > > >>> <pedroa....@outlook.com>; Andrew Fish <af...@apple.com>; > Anthony > > >>> Perard <anthony.per...@citrix.com>; Ard Biesheuvel > > >>> <ard.biesheu...@linaro.org>; Wu, Hao A <hao.a...@intel.com>; > Wang, > > >>> Jian J <jian.j.w...@intel.com>; Julien Grall > > >>> <julien.gr...@linaro.org>; Laszlo Ersek <ler...@redhat.com>; Ni, > > >>> Ray <ray...@intel.com>; Zeng, Star <star.z...@intel.com> > > >>> Subject: [PATCH 00/10] Fix PEI Core issue during > > >>> TemporaryRamMigration > > >>> > > >>> https://github.com/jljusten/edk2.git temp-ram-support > > >>> > > >>> This series fixes an issue that, while rare, is possible based on > > >>> the way the TemporaryRamSupport PPI is defined along with how it > > >>> is used by the PEI Core. > > >>> > > >>> Liu Yu reported a boot issue with EmulatorPkg, which I believe was > > >>> caused by this issue. > > >>> > > >>> The details of the issue are described in the commit message of > > >>> the > > >>> "MdeModePkg/Core/Pei: Add code path to allow assembly temp-ram > > >>> migration" patch. > > >>> > > >>> Along with this, I added a few Temporary RAM patches for > > >>> EmulatorPkg and OvmfPkg. > > >>> > > >>> Cc: Liu Yu <pedroa....@outlook.com> > > >>> Cc: Andrew Fish <af...@apple.com> > > >>> Cc: Anthony Perard <anthony.per...@citrix.com> > > >>> Cc: Ard Biesheuvel <ard.biesheu...@linaro.org> > > >>> Cc: Hao Wu <hao.a...@intel.com> > > >>> Cc: Jian J Wang <jian.j.w...@intel.com> > > >>> Cc: Julien Grall <julien.gr...@linaro.org> > > >>> Cc: Laszlo Ersek <ler...@redhat.com> > > >>> Cc: Ray Ni <ray...@intel.com> > > >>> Cc: Star Zeng <star.z...@intel.com> > > >>> > > >>> Jordan Justen (10): > > >>> EmulatorPkg/build.sh: Fix missing usage of -b BUILDTARGET > parameter > > >>> EmulatorPkg/Unix/Host: Use PcdInitValueInTempStack to init temp- > ram > > >>> EmulatorPkg/Sec: Replace assembly temp-ram support with C code > > >>> EmulatorPkg/Sec: Disable optimizations for TemporaryRamMigration > > >>> function > > >>> OvmfPkg/Sec: Swap TemporaryRam Stack and Heap locations > > >>> OvmfPkg/Sec: Disable optimizations for TemporaryRamMigration > > >>> MdeModePkg/Core/Pei: Add code path to allow assembly temp-ram > > >>> migration > > >>> MdeModulePkg/Core/Pei: Use assembly for X64 > TemporaryRamMigration > > >>> MdeModulePkg/Core/Pei: Use assembly for IA32 > TemporaryRamMigration > > >>> OvmfPkg/Sec: Fill Temp Ram after TemporaryRamMigration > > >>> > > >>> EmulatorPkg/Sec/Ia32/SwitchRam.S | 95 ------------------- > > >>> EmulatorPkg/Sec/Ia32/SwitchRam.asm | 94 ------------------ > > >>> EmulatorPkg/Sec/Ia32/TempRam.c | 65 ------------- > > >>> EmulatorPkg/Sec/Sec.c | 76 ++++++++++++++- > > >>> EmulatorPkg/Sec/Sec.inf | 13 +-- > > >>> EmulatorPkg/Sec/X64/SwitchRam.S | 72 -------------- > > >>> EmulatorPkg/Sec/X64/SwitchRam.asm | 76 --------------- > > >>> EmulatorPkg/Unix/Host/Host.c | 2 +- > > >>> EmulatorPkg/Unix/Host/Host.inf | 1 + > > >>> EmulatorPkg/build.sh | 10 +- > > >>> MdeModulePkg/Core/Pei/Dispatcher/Dispatcher.c | 59 ++++++++---- > > >>> .../Dispatcher/Ia32/TemporaryRamMigration.S | 72 > ++++++++++++++ > > >>> .../Ia32/TemporaryRamMigration.nasm | 78 +++++++++++++++ > > >>> .../Pei/Dispatcher/TemporaryRamMigration.c | 52 ++++++++++ > > >>> .../Dispatcher/X64/TemporaryRamMigration.S | 69 > ++++++++++++++ > > >>> .../Dispatcher/X64/TemporaryRamMigration.nasm | 75 > +++++++++++++++ > > >>> MdeModulePkg/Core/Pei/PeiMain.h | 52 ++++++++++ > > >>> MdeModulePkg/Core/Pei/PeiMain.inf | 14 +++ > > >>> OvmfPkg/Sec/Ia32/SecEntry.nasm | 2 +- > > >>> OvmfPkg/Sec/SecMain.c | 59 ++++++++---- > > >>> OvmfPkg/Sec/X64/SecEntry.nasm | 2 +- > > >>> 21 files changed, 577 insertions(+), 461 deletions(-) delete > > >>> mode 100644 EmulatorPkg/Sec/Ia32/SwitchRam.S delete mode > 100644 > > >>> EmulatorPkg/Sec/Ia32/SwitchRam.asm > > >>> delete mode 100644 EmulatorPkg/Sec/Ia32/TempRam.c delete mode > > >>> 100644 EmulatorPkg/Sec/X64/SwitchRam.S delete mode 100644 > > >>> EmulatorPkg/Sec/X64/SwitchRam.asm create mode 100644 > > >>> MdeModulePkg/Core/Pei/Dispatcher/Ia32/TemporaryRamMigration.S > > >>> create mode 100644 > > >>> > MdeModulePkg/Core/Pei/Dispatcher/Ia32/TemporaryRamMigration.nasm > > >>> create mode 100644 > > >>> MdeModulePkg/Core/Pei/Dispatcher/TemporaryRamMigration.c > > >>> create mode 100644 > > >>> MdeModulePkg/Core/Pei/Dispatcher/X64/TemporaryRamMigration.S > > >>> create mode 100644 > > >>> > MdeModulePkg/Core/Pei/Dispatcher/X64/TemporaryRamMigration.nasm > > >>> > > >>> -- > > >>> 2.20.0.rc1 > > >> > > >> _______________________________________________ > > >> edk2-devel mailing list > > >> edk2-devel@lists.01.org > > >> https://lists.01.org/mailman/listinfo/edk2-devel > > > > > > -- > > Thanks, > > Ray _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
