Laszlo, I also agree this is a good solution.
Thanks, Mike -----Original Message----- From: edk2-devel [mailto:edk2-devel-boun...@lists.01.org] On Behalf Of Laszlo Ersek Sent: Monday, November 16, 2015 10:38 AM To: Yao, Jiewen <jiewen....@intel.com> Cc: Kinney, Michael D <michael.d.kin...@intel.com>; Justen, Jordan L <jordan.l.jus...@intel.com>; edk2-de...@ml01.01.org Subject: Re: [edk2] [PATCH] MdeModulePkg: SmmLockBoxPeiLib: work without EFI_PEI_SMM_COMMUNICATION_PPI On 11/14/15 00:26, Yao, Jiewen wrote: > Good. Thanks! > Reviewed by: jiewen....@intel.com Thank you very much. I saw your reply soon after you posted it (which was "Friday night" in my timezone: Central European Time), but I figured I'd leave some time for Mike and Jordan to comment. It seems like it was 12:51 PM (= Friday early afternoon) in the Portland area when I posted the patch, and now it should be ~ 10:31 AM. I think for such a simple patch those hours should suffice. Plus, I did my best to provide the background info (based upon your original help, thanks again for that) that Jordan and Mike asked for. Thus, I've now committed this: SVN r18823. Thank you again for the good idea and your flexibility to address this in SmmLockBoxPeiLib. Laszlo > > > > -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Saturday, November 14, 2015 4:52 AM > To: edk2-de...@ml01.01.org > Cc: Yao, Jiewen; Kinney, Michael D; Justen, Jordan L > Subject: [PATCH] MdeModulePkg: SmmLockBoxPeiLib: work without > EFI_PEI_SMM_COMMUNICATION_PPI > > The RestoreLockBox() and RestoreAllLockBoxInPlace() functions handle > the case when EFI_PEI_SMM_COMMUNICATION_PPI.Communicate() returns > EFI_NOT_STARTED: they access the SMRAM directly, for restoring LockBox data. > > This occurs if a PEIM needs to restore LockBox data *before* the SMBASE is > relocated and the SMI handler is installed for all processors. > > One such PEIM is UefiCpuPkg/Universal/Acpi/S3Resume2Pei. On the S3 > resume path, in function S3RestoreConfig2(), LockBox data are restored > *before* the SmmRestoreCpu() function of UefiCpuPkg/PiSmmCpuDxeSmm is > called via > SmmS3ResumeState->SmmS3ResumeEntryPoint. (The latter SmmRestoreCpu() > function is responsible for the SMBASE relocation.) > > If a platform knows that its PEIMs restore LockBox data *only* before SMBASE > relocation -- e.g., due to S3Resume2Pei being the platform's only > SmmLockBoxPeiLib client --, then the platform might not want to include > "UefiCpuPkg/PiSmmCommunication/PiSmmCommunicationPei.inf" at all (hence not > provide EFI_PEI_SMM_COMMUNICATION_PPI) -- because all of those restores would > be serviced by direct SMRAM access anyway. > > Currently the absence of EFI_PEI_SMM_COMMUNICATION_PPI is not supported by > SmmLockBoxPeiLib, but it's not hard to implement. Handle it the same as when > EFI_PEI_SMM_COMMUNICATION_PPI.Communicate() returns EFI_NOT_STARTED: > restore LockBox data directly from SMRAM. > > Suggested-by: Jiewen Yao <jiewen....@intel.com> > Cc: Jiewen Yao <jiewen....@intel.com> > Cc: Michael Kinney <michael.d.kin...@intel.com> > Cc: Jordan Justen <jordan.l.jus...@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > --- > > Notes: > This allows me to drop the null implementation of > EFI_PEI_SMM_COMMUNICATION_PPI from the patch "OvmfPkg: add PEIM for > providing TSEG-as-SMRAM during PEI". > > MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c | 10 ++++++++-- > 1 file changed, 8 insertions(+), 2 deletions(-) > > diff --git a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c > b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c > index bd3204b..cea1fed 100644 > --- a/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c > +++ b/MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.c > @@ -563,7 +563,10 @@ RestoreLockBox ( > (VOID **)&SmmCommunicationPpi > ); > if (EFI_ERROR (Status)) { > - return EFI_NOT_STARTED; > + DEBUG ((EFI_D_INFO, "SmmLockBoxPeiLib LocatePpi - (%r)\n", Status)); > + Status = InternalRestoreLockBoxFromSmram (Guid, Buffer, Length); > + DEBUG ((EFI_D_INFO, "SmmLockBoxPeiLib RestoreLockBox - Exit (%r)\n", > Status)); > + return Status; > } > > // > @@ -682,7 +685,10 @@ RestoreAllLockBoxInPlace ( > (VOID **)&SmmCommunicationPpi > ); > if (EFI_ERROR (Status)) { > - return EFI_NOT_STARTED; > + DEBUG ((EFI_D_INFO, "SmmLockBoxPeiLib LocatePpi - (%r)\n", Status)); > + Status = InternalRestoreAllLockBoxInPlaceFromSmram (); > + DEBUG ((EFI_D_INFO, "SmmLockBoxPeiLib RestoreAllLockBoxInPlace - Exit > (%r)\n", Status)); > + return Status; > } > > // > -- > 1.8.3.1 > > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.01.org > https://lists.01.org/mailman/listinfo/edk2-devel > _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
