Re: [edk2] [PATCH 00/10] Fix PEI Core issue during TemporaryRamMigration

2019-02-20 Thread Ni, Ray



> -Original Message-
> From: Justen, Jordan L 
> Sent: Thursday, February 21, 2019 9:03 AM
> To: Gao, Liming ; Ni, Ray ; edk2-
> de...@lists.01.org
> Cc: Wu, Hao A ; Anthony Perard
> ; Laszlo Ersek ; Zeng,
> Star ; Andrew Fish 
> Subject: RE: [PATCH 00/10] Fix PEI Core issue during TemporaryRamMigration
> 
> On 2019-02-20 16:15:59, Ni, Ray wrote:
> > > -Original Message-
> > > From: Justen, Jordan L 
> > > Sent: Thursday, February 21, 2019 1:44 AM
> > > To: Gao, Liming ; Ni, Ray ;
> > > edk2- de...@lists.01.org
> > > Cc: Wu, Hao A ; Anthony Perard
> > > ; Laszlo Ersek ; Zeng,
> > > Star 
> > > 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).
> 
> I asked about this last November, and as I recall, Andrew said we can (and
> should) add assembly to PEI Core if it is required to meet the specs.
> 
> > Behavior of ARM and x86 becomes different. (The patch only fixes x86
> > issue.)
> 
> Yes. Similar code should be written for ARM, but I don't have experience
> with ARM assembly code.
> 
> > Given there is no real requirement on this,
> 
> Isn't the requirement to be compatible with the PI specification?
> 
> It seems that at least you and Liu Yu have encountered a build environment
> that produces code for PEI Core that isn't compatible with the PI 
> specification.
> 
> It doesn't seem like the best response to this is to just change the platform
> boot path and ignore the bug.

The environment Liu Yu and I encountered is the same Emulator environment.
I think the code change you did to PeiCore is great. It re-wrote the C code in
assembly to make sure the implementation doesn't rely on C compiler's
behavior.
But looking back, if a PI spec interface should depend on assembly
Implementation, I will pursue to change the PI spec interface directly.
Given the fact that no platform is producing this RamMigration PPI,
changing PI spec then changing the PeiCore to use new PPI interface
has no impact.

To be honest, I don't want to introduce complexity to PeiCore or edk2. That's
my only concern.

> 
> I do agree that after this issue is fixed, we can then consider changing the
> platform. The downside to changing it then will be to leave an untested code
> path, but at least we won't leave it with a known bug present.
> 
> -Jordan
> 
> > I prefer to not change PeiCore.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 00/10] Fix PEI Core issue during TemporaryRamMigration

2019-02-20 Thread Jordan Justen
On 2019-02-20 16:15:59, Ni, Ray wrote:
> > -Original Message-
> > From: Justen, Jordan L 
> > Sent: Thursday, February 21, 2019 1:44 AM
> > To: Gao, Liming ; Ni, Ray ; edk2-
> > de...@lists.01.org
> > Cc: Wu, Hao A ; Anthony Perard
> > ; Laszlo Ersek ; Zeng,
> > Star 
> > 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).

I asked about this last November, and as I recall, Andrew said we can
(and should) add assembly to PEI Core if it is required to meet the
specs.

> Behavior of ARM and x86 becomes different. (The patch only fixes x86 issue.)

Yes. Similar code should be written for ARM, but I don't have
experience with ARM assembly code.

> Given there is no real requirement on this,

Isn't the requirement to be compatible with the PI specification?

It seems that at least you and Liu Yu have encountered a build
environment that produces code for PEI Core that isn't compatible with
the PI specification.

It doesn't seem like the best response to this is to just change the
platform boot path and ignore the bug.

I do agree that after this issue is fixed, we can then consider
changing the platform. The downside to changing it then will be to
leave an untested code path, but at least we won't leave it with a
known bug present.

-Jordan

> I prefer to not change PeiCore.
___
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel


Re: [edk2] [PATCH 00/10] Fix PEI Core issue during TemporaryRamMigration

2019-02-20 Thread Ni, Ray
> -Original Message-
> From: Justen, Jordan L 
> Sent: Thursday, February 21, 2019 1:44 AM
> To: Gao, Liming ; Ni, Ray ; edk2-
> de...@lists.01.org
> Cc: Wu, Hao A ; Anthony Perard
> ; Laszlo Ersek ; Zeng,
> Star 
> 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 ;
> > >> edk2-devel@lists.01.org
> > >> Cc: Wu, Hao A ; Anthony Perard
> > >> ; Laszlo Ersek ;
> > >> Zeng, Star 
> > >> 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 
> > >>> Sent: Monday, February 18, 2019 12:12 PM
> > >>> To: edk2-devel@lists.01.org
> > >>> Cc: Justen, Jordan L ; Liu Yu
> > >>> ; Andrew Fish ;
> Anthony
> > >>> Perard ; Ard Biesheuvel
> > >>> ; Wu, Hao A ;
> Wang,
> > >>> Jian J ; Julien Grall
> > >>> ; Laszlo Ersek ; Ni,
> > >>> Ray ; Zeng, Star 
> > >>> 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 an

Re: [edk2] [PATCH 00/10] Fix PEI Core issue during TemporaryRamMigration

2019-02-20 Thread Jordan Justen
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?

-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 ; edk2-devel@lists.01.org
> >> Cc: Wu, Hao A ; Anthony Perard 
> >> ; Laszlo Ersek ; Zeng, Star
> >> 
> >> 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 
> >>> Sent: Monday, February 18, 2019 12:12 PM
> >>> To: edk2-devel@lists.01.org
> >>> Cc: Justen, Jordan L ; Liu Yu
> >>> ; Andrew Fish ; Anthony
> >>> Perard ; Ard Biesheuvel
> >>> ; Wu, Hao A ; Wang, Jian
> >>> J ; Julien Grall ; Laszlo 
> >>> Ersek
> >>> ; Ni, Ray ; Zeng, Star
> >>> 
> >>> 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 
> >>> Cc: Andrew Fish 
> >>> Cc: Anthony Perard 
> >>> Cc: Ard Biesheuvel 
> >>> Cc: Hao Wu 
> >>> Cc: Jian J Wang 
> >>> Cc: Julien Grall 
> >>> Cc: Laszlo Ersek 
> >>> Cc: Ray Ni 
> >>> Cc: Star Zeng 
> >>>
> >>> 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 pa

Re: [edk2] [PATCH 00/10] Fix PEI Core issue during TemporaryRamMigration

2019-02-20 Thread Ni, Ray

On 2/19/2019 9:25 PM, Gao, Liming wrote:

Ray:
In PI spec, TEMPORARY_RAM_SUPPORT_PPI is described as an optional PPI 
that is only required for platforms that may have side effects when both 
Temporary RAM and Permanent RAM are enabled. If a platform does not have any 
side effects when both Temporary RAM and Permanent RAM are enabled, and the 
platform is required to disable the use of Temporary RAM, then 
EFI_PEI_TEMPORARY_RAM_DONE should be produced. If a platform does not have any 
side effects when both Temporary RAM and Permanent RAM are enabled, and the 
platform is not required to disable the use of Temporary RAM, then neither 
EFI_PEI_TEMPORARY_RAM_DONE nor EFI_PEI_TEMPORARY_RAM_SUPPORT_PPI should be 
produced.

 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.

I will work out patches for EmulatorPkg to produce TemoraryRamDone.



Thanks
Liming

-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 ; edk2-devel@lists.01.org
Cc: Wu, Hao A ; Anthony Perard ; 
Laszlo Ersek ; Zeng, Star

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 
Sent: Monday, February 18, 2019 12:12 PM
To: edk2-devel@lists.01.org
Cc: Justen, Jordan L ; Liu Yu
; Andrew Fish ; Anthony
Perard ; Ard Biesheuvel
; Wu, Hao A ; Wang, Jian
J ; Julien Grall ; Laszlo Ersek
; Ni, Ray ; Zeng, Star

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 
Cc: Andrew Fish 
Cc: Anthony Perard 
Cc: Ard Biesheuvel 
Cc: Hao Wu 
Cc: Jian J Wang 
Cc: Julien Grall 
Cc: Laszlo Ersek 
Cc: Ray Ni 
Cc: Star Zeng 

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

Re: [edk2] [PATCH 00/10] Fix PEI Core issue during TemporaryRamMigration

2019-02-19 Thread Jordan Justen
On 2019-02-18 18:46:24, Ni, Ray wrote:
> 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).

I guess since it is in the PI spec, we can't be sure it is only used
for this case, or that it might not be used in the future.

> 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?

I checked the PI 1.4 spec, and I didn't see anything indicating that
TemporaryRamSupport PPI is deprecated.

Since it is not deprecated, should we ignore the known issue?

> 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?

This is a good question. If it is true that TemporaryRamSupport is
rarely used, then maybe it is better to have the sample platforms use
the more commonly used path.

Personally, I think we should still address the issue with
TemporaryRamSupport, and leave the question of whether to test
TemporaryRamSupport code paths in the sample platforms as a separate
task.

At the least, I think we should still continue to use TemporaryRamDone
to reset the temp-ram contents to help make sure nothing accidentally
depends on a temp-ram pointer. Unfortunately, this would mean that the
TemporaryRamSupport path is not really being tested, but it might be
the better choise if TemporaryRamSupport is never used in real
platforms.

-Jordan

> 
> > -Original Message-
> > From: Justen, Jordan L 
> > Sent: Monday, February 18, 2019 12:12 PM
> > To: edk2-devel@lists.01.org
> > Cc: Justen, Jordan L ; Liu Yu
> > ; Andrew Fish ; Anthony
> > Perard ; Ard Biesheuvel
> > ; Wu, Hao A ; Wang, Jian
> > J ; Julien Grall ; Laszlo 
> > Ersek
> > ; Ni, Ray ; Zeng, Star
> > 
> > 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 
> > Cc: Andrew Fish 
> > Cc: Anthony Perard 
> > Cc: Ard Biesheuvel 
> > Cc: Hao Wu 
> > Cc: Jian J Wang 
> > Cc: Julien Grall 
> > Cc: Laszlo Ersek 
> > Cc: Ray Ni 
> > Cc: Star Zeng 
> > 
> > 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 

Re: [edk2] [PATCH 00/10] Fix PEI Core issue during TemporaryRamMigration

2019-02-19 Thread Gao, Liming
Ray:
In PI spec, TEMPORARY_RAM_SUPPORT_PPI is described as an optional PPI 
that is only required for platforms that may have side effects when both 
Temporary RAM and Permanent RAM are enabled. If a platform does not have any 
side effects when both Temporary RAM and Permanent RAM are enabled, and the 
platform is required to disable the use of Temporary RAM, then 
EFI_PEI_TEMPORARY_RAM_DONE should be produced. If a platform does not have any 
side effects when both Temporary RAM and Permanent RAM are enabled, and the 
platform is not required to disable the use of Temporary RAM, then neither 
EFI_PEI_TEMPORARY_RAM_DONE nor EFI_PEI_TEMPORARY_RAM_SUPPORT_PPI should be 
produced.

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? 

Thanks
Liming
> -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 ; edk2-devel@lists.01.org
> Cc: Wu, Hao A ; Anthony Perard 
> ; Laszlo Ersek ; Zeng, Star
> 
> 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 
> > Sent: Monday, February 18, 2019 12:12 PM
> > To: edk2-devel@lists.01.org
> > Cc: Justen, Jordan L ; Liu Yu
> > ; Andrew Fish ; Anthony
> > Perard ; Ard Biesheuvel
> > ; Wu, Hao A ; Wang, Jian
> > J ; Julien Grall ; Laszlo 
> > Ersek
> > ; Ni, Ray ; Zeng, Star
> > 
> > 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 
> > Cc: Andrew Fish 
> > Cc: Anthony Perard 
> > Cc: Ard Biesheuvel 
> > Cc: Hao Wu 
> > Cc: Jian J Wang 
> > Cc: Julien Grall 
> > Cc: Laszlo Ersek 
> > Cc: Ray Ni 
> > Cc: Star Zeng 
> >
> > 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

Re: [edk2] [PATCH 00/10] Fix PEI Core issue during TemporaryRamMigration

2019-02-18 Thread Ni, Ray
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 
> Sent: Monday, February 18, 2019 12:12 PM
> To: edk2-devel@lists.01.org
> Cc: Justen, Jordan L ; Liu Yu
> ; Andrew Fish ; Anthony
> Perard ; Ard Biesheuvel
> ; Wu, Hao A ; Wang, Jian
> J ; Julien Grall ; Laszlo 
> Ersek
> ; Ni, Ray ; Zeng, Star
> 
> 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 
> Cc: Andrew Fish 
> Cc: Anthony Perard 
> Cc: Ard Biesheuvel 
> Cc: Hao Wu 
> Cc: Jian J Wang 
> Cc: Julien Grall 
> Cc: Laszlo Ersek 
> Cc: Ray Ni 
> Cc: Star Zeng 
> 
> 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


[edk2] [PATCH 00/10] Fix PEI Core issue during TemporaryRamMigration

2019-02-17 Thread Jordan Justen
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 
Cc: Andrew Fish 
Cc: Anthony Perard 
Cc: Ard Biesheuvel 
Cc: Hao Wu 
Cc: Jian J Wang 
Cc: Julien Grall 
Cc: Laszlo Ersek 
Cc: Ray Ni 
Cc: Star Zeng 

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