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