On 05/11/15 09:07, Tian, Feng wrote: > It looks good to me, Laszlo > > Just a minor comment, How about moving the local variable statement up to the > beginning of the function for better coding style? > > If you agree, I will do it at check-in this patch and sign: > > Reviewed-by: Feng Tian <feng.t...@intel.com>
Works for me, thank you. Laszlo > Thanks > Feng > > -----Original Message----- > From: Laszlo Ersek [mailto:ler...@redhat.com] > Sent: Saturday, May 09, 2015 02:52 > To: edk2-devel@lists.sourceforge.net > Cc: Tian, Feng > Subject: [PATCH 07/11] MdeModulePkg: SmmIplEntry(): don't suppress SMM core > startup failure > > When the ExecuteSmmCoreFromSmram() function fails, SmmIplEntry() restores the > SMRAM range to EFI_MEMORY_UC. However, it saves the return value of > gDS->SetMemorySpaceAttributes() in the same Status variable that > gDS->contains > the return value of ExecuteSmmCoreFromSmram(). Therefore, if > gDS->SetMemorySpaceAttributes() succeeds, the failure of > ExecuteSmmCoreFromSmram() is masked, and Bad Things Happen (TM). > > Introduce a temporary variable just for the return value of > gDS->SetMemorySpaceAttributes(). > > Cc: Feng Tian <feng.t...@intel.com> > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Laszlo Ersek <ler...@redhat.com> > --- > MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c | 14 ++++++++------ > 1 file changed, 8 insertions(+), 6 deletions(-) > > diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c > b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c > index 4759579..35da454 100644 > --- a/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c > +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmIpl.c > @@ -1213,12 +1213,14 @@ SmmIplEntry ( > // Attempt to reset SMRAM cacheability to UC > // > if (CpuArch != NULL) { > - Status = gDS->SetMemorySpaceAttributes( > - mSmramCacheBase, > - mSmramCacheSize, > - EFI_MEMORY_UC > - ); > - if (EFI_ERROR (Status)) { > + EFI_STATUS SetAttrStatus; > + > + SetAttrStatus = gDS->SetMemorySpaceAttributes( > + mSmramCacheBase, > + mSmramCacheSize, > + EFI_MEMORY_UC > + ); > + if (EFI_ERROR (SetAttrStatus)) { > DEBUG ((DEBUG_WARN, "SMM IPL failed to reset SMRAM window to > EFI_MEMORY_UC\n")); > } > } > -- > 1.8.3.1 > > ------------------------------------------------------------------------------ One dashboard for servers and applications across Physical-Virtual-Cloud Widest out-of-the-box monitoring support with 50+ applications Performance metrics, stats and reports that give you Actionable Insights Deep dive visibility with transaction tracing using APM Insight. http://ad.doubleclick.net/ddm/clk/290420510;117567292;y _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
